| Re: [eclipselink-dev] Hello and question about Connector |
Hi Wonseok,
You are right about the connector being a dummy connector at that point.
Thanks for looking into this issue. I look forward to seeing your patch
attached to the bug.
-Tom
Wonseok Kim wrote:
> 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
> <mailto: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>
> > [mailto: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
> > eclipselink-dev@xxxxxxxxxxx <mailto:eclipselink-dev@xxxxxxxxxxx>> <mailto: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
> > https://dev.eclipse.org/mailman/listinfo/eclipselink-dev> eclipselink-dev@xxxxxxxxxxx <mailto:eclipselink-dev@xxxxxxxxxxx>
> _______________________________________________
> eclipselink-dev mailing list
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> 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