Re: [eclipselink-dev] Hello and question about Connector

["Wonseok Kim" <guruwons@xxxxxxxxx>]

Hi Tom,

I filed an issue for this.
https://bugs.eclipse.org/bugs/show_bug.cgi?id=224083

Actually I was curious that at predeploy phase why DefaultConnector is always set to Login object and not used until it's replaced with real Connector(JNDIConnector or DefaultConnector with driver, url setting) at deploy time. I guess it's just dummy Connector which is not actually used at predeploy time.

Anyway to address this issue I got two suggestions. One is checking JDBC URL property when setting this to Connector like below. This will throw proper error message rather than NPE.

EntityManagerSetupImpl:
    protected void updateLoginDefaultConnector(DatasourceLogin login, Map m){
        String connectionString = getConfigPropertyAsStringLogDebug(PersistenceUnitProperties.JDBC_URL, m, session);
        if(connectionString == null)
            throw new javax.persistence.PersistenceException(EntityManagerSetupException.propertyIsMissing(PersistenceUnitProperties.JDBC_URL));
        
        String jdbcDriver = getConfigPropertyAsStringLogDebug(PersistenceUnitProperties.JDBC_DRIVER, m, session);
        // jdbcDriver is optional(could be null) because driver might be already loaded 
        
        DefaultConnector connector = new DefaultConnector(jdbcDriver, "", connectionString);
        login.setConnector(connector);
    }

Another one is modifying DefaultConnector not to have default driver class name (JdbcOdbc). Even if driver class name is not given, DriverManager can use already loaded drivers for the URL. Also JDBC 4.0 drivers are loaded automatically(4.0 change) if they are in the classpath. Therefore eclipselink.jdbc.driver property can be optional.

For this DefaultConnector can to be modified like below.

Index: /home/wons/works/eclipselink/foundation/eclipselink.core/src/org/eclipse/persistence/sessions/DefaultConnector.java
===================================================================
--- /home/wons/works/eclipselink/foundation/eclipselink.core/src/org/eclipse/persistence/sessions/DefaultConnector.java    (revision 13786)
+++ /home/wons/works/eclipselink/foundation/eclipselink.core/src/org/eclipse/persistence/sessions/DefaultConnector.java    (working copy)
@@ -47,11 +47,10 @@
     
     /**
      * PUBLIC:
-     * Construct a Connector with default settings (Sun JDBC-ODBC bridge).
+     * Construct a Connector with default settings.
      * The database URL will still need to be set.
      */
     public DefaultConnector() {
-        this("sun.jdbc.odbc.JdbcOdbcDriver", "jdbc:odbc:", "");
     }
 
     /**
@@ -80,8 +79,8 @@
      * @return java.sql.Connection
      */
     public Connection connect(Properties properties, Session session) throws DatabaseException {
-        // ensure the driver has been loaded and registered
-        if(this.driverClass == null) {
+        // if driver class name is given, ensure the driver has been loaded and registered
+        if(this.driverClassName != null && this.driverClass == null) {
             this.loadDriverClass(session);
         }
         
@@ -96,7 +95,12 @@
                 }
             }
         }
-
+        
+        if(this.driverClass == null) {
+            throw DatabaseException.sqlException(driverManagerException, (org.eclipse.persistence.internal.sessions.AbstractSession) session, true);
+        }
+        
+        // try driver class directly 
         // Save secondary exception state and don't clear original exception
         boolean wrongDriverExceptionOccurred = false;
         try {

I'll attach this fix to the issue soon.

Cheers,
-Wonseok

On Wed, Mar 26, 2008 at 2:36 AM, Tom Ware <tom.ware@xxxxxxxxxx> wrote:

'Just to add to what James has said.

In the "predeploy" phase we are trying to build a Project in such a way that it
references a minimal number of classes - ideally only java classes and
EclipseLink classes.  We do that because this we use this phase to figure out
how we are going to weave classes and we do not want any classes hanging around
that were loaded by the temporary class loader.

In the deploy phase we end up loading those classes with the correct loader.

-Tom

James Sutherland wrote:
> Welcome Wonseok,
>
>   The default driver is the JDBC ODBC driver because this is (well was)
> the only JDBC part of JDK (at least the sun jdk).  It is odd that your
> JVM is getting an error trying to load the class.
>
>
>
>   In general I agree, the sun JDBC ODBC driver is rarely used, perhaps
> the default should be changed to Derby, or Oracle or MySQL, or perhaps
> just have no default.  Technically the driver class is not always
> required, if the driver has already been loaded in the JVM simply using
> the URL should be enough to work.  If you did not set either a driver or
> a URL, then we should probably be throwing an error.
>
>
>
>   I'm not sure on the deployment stuff.  My guess is that predeployment
> is called before the application is deployed, such as through the
> agent.  The application classes (and possibly JDBC driver) do not yet
> exist, so cannot be referred to.  Deployment in an application server
> may also pass a default database to connect to, so we may not know the
> database until deployment.
>
>
>
>
>
> -----Original Message-----
> *From:* eclipselink-dev-bounces@xxxxxxxxxxx
> [mailto:eclipselink-dev-bounces@xxxxxxxxxxx]*On Behalf Of *Wonseok Kim
> *Sent:* Tuesday, March 25, 2008 11:12 AM
> *To:* eclipselink-dev@xxxxxxxxxxx
> *Subject:* [eclipselink-dev] Hello and question about Connector
>
>
>
> Hello! EclipseLink commiters
>
> Some guys already know me, but I'd like to introduce me first.
> I am Wonseok working at TmaxSoft (South Korea) and also a committer of
> TopLink Essentials.
> Finally I got time to look into EclipseLink code and I would like to
> participate and contribute to EclipseLink project. Also our AS
> product(JEUS) which now includes TLE has plan to incorporate/support
> EclipseLink as a default provider. I'm happy to see this project is
> already mature (documentation also).
>
> Apparently I will have numerous questions about code or anything. So
> please, please shed light on me even I ask silly questions. :-)
>
> Okay, here my first question goes.
> While I'm setting up my test environment (transition from TLE), I got
> the following NPE and this is definitely my mistake of not setting
> eclipselink.jdbc.driver correctly - it was toplink.jdbc.driver :-)
>
> Exception in thread "main" java.lang.NullPointerException
>     at sun.jdbc.odbc.JdbcOdbcDriver.initialize(JdbcOdbcDriver.java:436)
>     at sun.jdbc.odbc.JdbcOdbcDriver.connect(JdbcOdbcDriver.java:153)
>     at java.sql.DriverManager.getConnection(DriverManager.java:582)
>     at java.sql.DriverManager.getConnection(DriverManager.java:154)
>     at
> org.eclipse.persistence.sessions.DefaultConnector.connect(DefaultConnector.java:91)
>     at
> org.eclipse.persistence.sessions.DatasourceLogin.connectToDatasource(DatasourceLogin.java:164)
>     at
> org.eclipse.persistence.internal.sessions.DatabaseSessionImpl.loginAndDetectDatasource(DatabaseSessionImpl.java:578)
>     at
> org.eclipse.persistence.internal.jpa.EntityManagerFactoryProvider.login(EntityManagerFactoryProvider.java:214)
>     at
> org.eclipse.persistence.internal.jpa.EntityManagerSetupImpl.deploy(EntityManagerSetupImpl.java:234)
>     at
> org.eclipse.persistence.internal.jpa.EntityManagerFactoryImpl.getServerSession(EntityManagerFactoryImpl.java:69)
>     at
> org.eclipse.persistence.internal.jpa.EntityManagerFactoryImpl.createEntityManagerImpl(EntityManagerFactoryImpl.java:118)
>     at
> org.eclipse.persistence.internal.jpa.EntityManagerFactoryImpl.createEntityManagerImpl(EntityManagerFactoryImpl.java:112)
>     at
> org.eclipse.persistence.internal.jpa.EntityManagerFactoryImpl.createEntityManager(EntityManagerFactoryImpl.java:100)
>
> This is misleading error(I met this error several time before). Looking
> into Connector code, I don't understand two things.
> First, why is JDBC-ODBC bridge driver used when there is no driver
> setting? I don't believe this is useful default. IMO just throwing
> proper error is more helpful.
>
> In addition, why is a dummy DefaultConnector(with default ODBC driver
> setting) set to DatasourceLogin at predeployment time? It doen't seem
> being used at all. Then the DefaultConnector is updated with real
> properties with EntityManagerSetupImpl#updateLogins and
> updateLoginDefaultConnector() at deployment. Isn't it clear and better
> to create and set DefaultConnector at deployment if JDBC properties are
> used while checking them(error could be thrown at this time)? I had no
> problem when I modified code like this. Is this right way?
>
> P.S. I really appreciate your commitment and efforts toward open source!
> It's a really win-win to everyone.
>
> Best Regards
> -Wonseok
>
>

> ------------------------------------------------------------------------
>
> _______________________________________________
> eclipselink-dev mailing list
> eclipselink-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev