You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by mi...@apache.org on 2010/06/08 00:09:30 UTC

svn commit: r952461 - in /openjpa/sandboxes/OpenJPA-1551-1.0.x: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-persisten...

Author: mikedd
Date: Mon Jun  7 22:09:30 2010
New Revision: 952461

URL: http://svn.apache.org/viewvc?rev=952461&view=rev
Log:
Added trace and check for read only config

Modified:
    openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
    openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java
    openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java
    openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java
    openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java

Modified: openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
URL: http://svn.apache.org/viewvc/openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java?rev=952461&r1=952460&r2=952461&view=diff
==============================================================================
--- openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java (original)
+++ openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java Mon Jun  7 22:09:30 2010
@@ -33,6 +33,7 @@ import java.util.Set;
 import javax.sql.DataSource;
 
 import org.apache.commons.lang.StringUtils;
+import org.apache.openjpa.conf.OpenJPAConfiguration;
 import org.apache.openjpa.event.OrphanedKeyAction;
 import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
 import org.apache.openjpa.jdbc.meta.ClassMapping;
@@ -63,6 +64,7 @@ import org.apache.openjpa.kernel.exps.Ex
 import org.apache.openjpa.lib.jdbc.DelegatingConnection;
 import org.apache.openjpa.lib.jdbc.DelegatingPreparedStatement;
 import org.apache.openjpa.lib.jdbc.DelegatingStatement;
+import org.apache.openjpa.lib.log.Log;
 import org.apache.openjpa.lib.rop.MergedResultObjectProvider;
 import org.apache.openjpa.lib.rop.ResultObjectProvider;
 import org.apache.openjpa.lib.util.Localizer;
@@ -112,12 +114,17 @@ public class JDBCStoreManager 
         _conf = (JDBCConfiguration) ctx.getConfiguration();
         _dict = _conf.getDBDictionaryInstance();
         _sql = _conf.getSQLFactoryInstance();
+        Log log = _conf.getLog(JDBCConfiguration.LOG_RUNTIME);
 
         LockManager lm = ctx.getLockManager();
         if (lm instanceof JDBCLockManager)
             _lm = (JDBCLockManager) lm;
 
         _ds = getDataSource(ctx);
+        
+        if(log != null && log.isTraceEnabled()) {
+            log.trace("JDBCStoreManager: " + this + " using DS: " + _ds  );
+        }
 
         if (_conf.getUpdateManagerInstance().orderDirty())
             ctx.setOrderDirtyObjects(true);
@@ -136,33 +143,55 @@ public class JDBCStoreManager 
         }
     }
     
-    private final DataSource getDataSourceFromConfig(StoreContext ctx) { 
+    private final DataSource getDataSourceFromConfig(StoreContext ctx) {
+        Log log = _conf.getLog(JDBCConfiguration.LOG_RUNTIME);
+        DataSource ds; 
         if (useConnectionFactory2(ctx)) {
-            return _conf.getDataSource2(ctx);
+            ds =_conf.getDataSource2(ctx);
+            if(log != null && log.isTraceEnabled()) {
+                log.trace("Found connectionFactory2 from config: " + ds );
+            }
         }
         else {
-            return _conf.getDataSource(ctx);
+            ds = _conf.getDataSource(ctx);
+            if(log != null && log.isTraceEnabled()) {
+                log.trace("Found connectionFactory from config: " + ds );
+            }
         }
+        return ds; 
     }
     
     private final DataSource getDataSourceFromContext(StoreContext ctx) {
         DataSource ds;
-        
+        Log log = _conf.getLog(JDBCConfiguration.LOG_RUNTIME); 
         if (useConnectionFactory2(ctx)) {
             ds = (DataSource) ctx.getConnectionFactory2();
+            if(log != null && log.isTraceEnabled()) {
+                log.trace("Found connectionFactory2 from context: " + ds );
+            }
         } else {
             ds = (DataSource) ctx.getConnectionFactory();
+            if(log != null && log.isTraceEnabled()) {
+                log.trace("Found connectionFactory from context: " + ds );
+            }
         }
         return DataSourceFactory.decorateDataSource(ds, _conf, false);
     }
     
     private boolean useContextToGetDataSource(StoreContext ctx) { 
-        // configuration check to enable goes here. 
-        if (StringUtils.isBlank(ctx.getConnectionFactoryName()) 
-                && StringUtils.isBlank(ctx.getConnectionFactory2Name())) {
-            return false;
-        }
-        return true;
+        boolean rval = false;
+        
+        // only check connectionFactoryName, connectionFactory2Name is always blank.
+        if(StringUtils.isNotBlank(ctx.getConnectionFactoryName())) { 
+            rval = true; 
+        }
+        Log log = _conf.getLog(JDBCConfiguration.LOG_RUNTIME); 
+        if(log != null && log.isTraceEnabled()) { 
+             log.trace("Found connectionFactoryName: " + ctx.getConnectionFactoryName() + " from context" );
+             log.trace("Found connectionFactory2Name: " + ctx.getConnectionFactory2Name() + " from context" );
+             log.trace("useContexToGetDataSource returns : " + rval);
+         }
+        return rval; 
     }
 
     public JDBCConfiguration getConfiguration() {

Modified: openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java
URL: http://svn.apache.org/viewvc/openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java?rev=952461&r1=952460&r2=952461&view=diff
==============================================================================
--- openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java (original)
+++ openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java Mon Jun  7 22:09:30 2010
@@ -38,6 +38,8 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.lang.StringUtils;
+import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
 import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
 import org.apache.openjpa.jdbc.kernel.exps.FilterValue;
 import org.apache.openjpa.jdbc.schema.Column;
@@ -47,8 +49,10 @@ import org.apache.openjpa.jdbc.schema.Pr
 import org.apache.openjpa.jdbc.schema.Sequence;
 import org.apache.openjpa.jdbc.schema.Table;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.StoreContext;
 import org.apache.openjpa.lib.jdbc.DelegatingDatabaseMetaData;
 import org.apache.openjpa.lib.jdbc.DelegatingPreparedStatement;
+import org.apache.openjpa.lib.log.Log;
 import org.apache.openjpa.lib.util.J2DoPrivHelper;
 import org.apache.openjpa.lib.util.Localizer;
 import org.apache.openjpa.util.StoreException;
@@ -217,6 +221,10 @@ public class OracleDictionary
                 driverVendor = VENDOR_OTHER;
         }
         cacheDriverBehavior(driverVendor);
+        
+        if(log != null && log.isTraceEnabled()) {
+            log.trace("Oracle driverVendor:" + driverVendor);
+        }
     }
 
     /**
@@ -236,10 +244,15 @@ public class OracleDictionary
             _driverBehavior = BEHAVE_OTHER;
     }
 
+    public void ensureDriverVendor() { 
+        ensureDriverVendor(null);
+    }
+    
     /**
      * Ensure that the driver vendor has been set, and if not, set it now.
      */
-    public void ensureDriverVendor() {
+    public void ensureDriverVendor(StoreContext sc) {
+        Log log = conf.getLog(JDBCConfiguration.LOG_RUNTIME); 
         if (driverVendor != null) {
             cacheDriverBehavior(driverVendor);
             return;
@@ -248,8 +261,23 @@ public class OracleDictionary
         if (log.isInfoEnabled())
             log.info(_loc.get("oracle-connecting-for-driver"));
         Connection conn = null;
+
         try {
-            conn = conf.getDataSource2(null).getConnection();
+            
+            // getting the connection from the SC is a bit kludgy - fix configuration upstream to handle better. 
+            if (sc == null || StringUtils.isBlank(sc.getConnectionFactoryName())) {
+                // get from the configuration if no connectionFactoryName was specified.
+                if(log.isTraceEnabled()) {
+                    log.trace("Using DataSource from configuration to obtain driver vendor");
+                }
+                conn = conf.getDataSource2(null).getConnection();
+            } else {
+                // get from the StoreContex if the connectionFactoryName is set
+                if(log.isTraceEnabled()) {
+                    log.trace("Using DataSource from StoreContext to obtain driver vendor");
+                }
+                conn = (Connection) sc.getConnection();
+            }
             connectedConfiguration(conn);
         } catch (SQLException se) {
             throw SQLExceptions.getStore(se, this);
@@ -362,16 +390,17 @@ public class OracleDictionary
         boolean distinct, boolean forUpdate, long start, long end,
         Select sel) {
         if (!_checkedUpdateBug) {
-            ensureDriverVendor();
+            ensureDriverVendor(fetch == null ? null : fetch.getContext());
             if (forUpdate && _driverBehavior == BEHAVE_DATADIRECT31)
                 log.warn(_loc.get("dd-lock-bug"));
             _checkedUpdateBug = true;
         }
 
         // if no range, use standard select
-        if (start == 0 && end == Long.MAX_VALUE)
+        if (start == 0 && end == Long.MAX_VALUE) {
             return super.toSelect(select, fetch, tables, where, group, having,
                 order, distinct, forUpdate, 0, Long.MAX_VALUE, sel);
+        }
 
         // if no skip, ordering, or distinct can use rownum directly
         SQLBuffer buf = new SQLBuffer(this);

Modified: openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java
URL: http://svn.apache.org/viewvc/openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java?rev=952461&r1=952460&r2=952461&view=diff
==============================================================================
--- openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java (original)
+++ openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java Mon Jun  7 22:09:30 2010
@@ -4898,6 +4898,11 @@ public class BrokerImpl
      */
     public void setConnectionFactoryName(String connectionFactoryName) {
         this._connectionFactoryName = StringUtils.trimToNull(connectionFactoryName);
+        
+        if (_log != null && _log.isTraceEnabled()) {
+            _log.trace("Broker: " + this + " using connectionFactoryName : " + _connectionFactoryName
+                + " value passed in: " + connectionFactoryName);
+        }
     }
 
     /**

Modified: openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java
URL: http://svn.apache.org/viewvc/openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java?rev=952461&r1=952460&r2=952461&view=diff
==============================================================================
--- openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java (original)
+++ openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java Mon Jun  7 22:09:30 2010
@@ -30,7 +30,9 @@ import java.util.Properties;
 import java.util.Set;
 import javax.persistence.EntityManagerFactory;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.openjpa.conf.OpenJPAConfiguration;
+import org.apache.openjpa.conf.OpenJPAConfigurationImpl;
 import org.apache.openjpa.enhance.Reflection;
 import org.apache.openjpa.kernel.AutoDetach;
 import org.apache.openjpa.kernel.Broker;
@@ -41,6 +43,7 @@ import org.apache.openjpa.kernel.FetchCo
 import org.apache.openjpa.lib.conf.Configurations;
 import org.apache.openjpa.lib.conf.ProductDerivations;
 import org.apache.openjpa.lib.conf.Value;
+import org.apache.openjpa.lib.log.Log;
 import org.apache.openjpa.lib.util.Localizer;
 import org.apache.openjpa.lib.util.Closeable;
 import org.apache.openjpa.util.OpenJPAException;
@@ -152,6 +155,7 @@ public class EntityManagerFactoryImpl
             props = new HashMap(props);
 
         OpenJPAConfiguration conf = getConfiguration();
+        Log log = conf.getLog(OpenJPAConfiguration.LOG_RUNTIME);
         String user = (String) Configurations.removeProperty
             ("ConnectionUserName", props);
         if (user == null)
@@ -188,12 +192,25 @@ public class EntityManagerFactoryImpl
                     new Throwable[]{ e }, obj, true);
             }
         }
-
+        
         String connectionFactoryName = (String) Configurations.removeProperty("ConnectionFactoryName", props);
-        String connectionFactory2Name = (String) Configurations.removeProperty("ConnectionFactoryName2", props);
+        if(log != null && log.isTraceEnabled()) {
+            log.trace("Found ConnectionFactoryName from props: " + connectionFactoryName );
+        }
         
+        if (!conf.isReadOnly()) {
+            // use the new settings.
+            if (StringUtils.isNotEmpty(connectionFactoryName)) {
+                if (log != null && log.isTraceEnabled()) {
+                    log.trace("Configuration has not been loaded - overriding configuration with connectionFactory " +
+                        		"from props: " + connectionFactoryName);
+                }
+                conf.setConnectionFactoryName(connectionFactoryName);
+            }
+        }
+
         Broker broker = _factory.newBroker(user, pass, managed, retainMode,
-            false, connectionFactoryName, connectionFactory2Name);
+            false, connectionFactoryName, "");
             
         // add autodetach for close and rollback conditions to the configuration
         broker.setAutoDetach(AutoDetach.DETACH_CLOSE, true);

Modified: openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
URL: http://svn.apache.org/viewvc/openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java?rev=952461&r1=952460&r2=952461&view=diff
==============================================================================
--- openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java (original)
+++ openjpa/sandboxes/OpenJPA-1551-1.0.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java Mon Jun  7 22:09:30 2010
@@ -35,6 +35,7 @@ import javax.persistence.Query;
 import javax.persistence.TemporalType;
 
 import org.apache.commons.collections.map.LinkedMap;
+import org.apache.openjpa.conf.OpenJPAConfiguration;
 import org.apache.openjpa.enhance.Reflection;
 import org.apache.openjpa.kernel.DelegatingQuery;
 import org.apache.openjpa.kernel.DelegatingResultList;
@@ -43,6 +44,7 @@ import org.apache.openjpa.kernel.QueryLa
 import org.apache.openjpa.kernel.QueryOperations;
 import org.apache.openjpa.kernel.exps.AggregateListener;
 import org.apache.openjpa.kernel.exps.FilterListener;
+import org.apache.openjpa.lib.log.Log;
 import org.apache.openjpa.lib.rop.ResultList;
 import org.apache.openjpa.lib.util.Localizer;
 import org.apache.openjpa.util.ImplHelper;



Re: svn commit: r952461 - in /openjpa/sandboxes/OpenJPA-1551-1.0.x

Posted by Pinaki Poddar <pp...@apache.org>.
We had the debate I agree .. but the debate is not resolved.
The TRACE logging level exists at thousands of places and they are user
focused and localized.  They are already in use by the existing
customers/users.
If anyone believes that tracing helps (I do not, I believe they simply
hamper code readability) for diagnosis and non-localized messages will
promote that practice -- define a DEBUG log level and use it.
I fail to see what is the counter rationale to such straightforward
solution.


-----
Pinaki 
-- 
View this message in context: http://openjpa.208410.n2.nabble.com/Re-svn-commit-r952461-in-openjpa-sandboxes-OpenJPA-1551-1-0-x-tp5153410p5153731.html
Sent from the OpenJPA Developers mailing list archive at Nabble.com.

Re: svn commit: r952461 - in /openjpa/sandboxes/OpenJPA-1551-1.0.x

Posted by Donald Woods <dw...@apache.org>.
We've had this debate before on dev@, that TRACE/DEBUG messages will not
be translated.....  They are only meant for problem determination, where
we (the OpenJPA developers) shouldn't have to spend a lot of our time
trying to translate trace messages which have no message id as part of
the output to facilitate quick lookups....


-Donald


On 6/7/10 11:32 PM, Pinaki Poddar wrote:
> 
> I am hoping that TRACE level messages will stay localized. But I admit
> non-localized messages can be useful. Why not define a DEBUG level finer
> than TRACE for these messages? 
> 
> -----
> Pinaki 

Re: svn commit: r952461 - in /openjpa/sandboxes/OpenJPA-1551-1.0.x: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-persisten...

Posted by Pinaki Poddar <pp...@apache.org>.
I am hoping that TRACE level messages will stay localized. But I admit
non-localized messages can be useful. Why not define a DEBUG level finer
than TRACE for these messages? 

-----
Pinaki 
-- 
View this message in context: http://openjpa.208410.n2.nabble.com/svn-commit-r952461-in-openjpa-sandboxes-OpenJPA-1551-1-0-x-openjpa-jdbc-src-main-java-org-apache-ope-tp5151360p5152090.html
Sent from the OpenJPA Commits mailing list archive at Nabble.com.