674: session = new ServerSession(new Project(new
DatabaseLogin()));
->
public DatabaseLogin(DatabasePlatform databasePlatform) {
super(databasePlatform);
this.useDefaultDriverConnect();
}
->
public void useDefaultDriverConnect() {
setConnector(new DefaultConnector());
}
updateLoginDefaultConnector method is modified to set proper new
DefaultConnector always like below and this also checks if JDBC URL is
set properly.
protected void updateLoginDefaultConnector(DatasourceLogin login,
Map m){
- if ((login.getConnector() instanceof DefaultConnector)) {
- DatabaseLogin dbLogin = (DatabaseLogin)login;
- // Note: This call does not checked the stored
persistenceUnitInfo or extended properties because
- // the map passed into this method should represent the
full set of properties we expect to process
- String jdbcDriver =
getConfigPropertyAsStringLogDebug(PersistenceUnitProperties.JDBC_DRIVER,
m, session);
- String connectionString =
getConfigPropertyAsStringLogDebug(PersistenceUnitProperties.JDBC_URL, m,
session);
- if(connectionString != null) {
- dbLogin.setConnectionString(connectionString);
- }
- if(jdbcDriver != null) {
- dbLogin.setDriverClassName(jdbcDriver);
- }
- }
+ 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 drivers might
be already loaded
+
+ DefaultConnector connector = new DefaultConnector(jdbcDriver,
"", connectionString);
+ login.setConnector(connector);
}
I believe that proper DefaultConnector should be always set instead of
just updating it in case that it's already DefaultConnector. It's
cleaner and expected behavior.
Also DefaultConnector is updated not to have default driver class like
JDBC-ODBC as I said before. The default behavior of DefaultConnector is
now just using DriverManager with already loaded driver classes. This
is useful with JDBC 4.0 drivers which are automatically loaded and this
makes eclipselink.jdbc.driver property optional.
I'm running LRG, SRG tests against this patch(I need to be familar with
current test model).
Please review and let me know if you have a concern.
-Wonseok
On Thu, Mar 27, 2008 at 12:45 AM, Tom Ware <tom.ware@xxxxxxxxxx
<mailto:tom.ware@xxxxxxxxxx>> wrote:
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>
> <mailto: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>>
> > [mailto: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
<mailto: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
> > eclipselink-dev@xxxxxxxxxxx
<mailto:eclipselink-dev@xxxxxxxxxxx>
<mailto:eclipselink-dev@xxxxxxxxxxx
<mailto:eclipselink-dev@xxxxxxxxxxx>>
> > https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
> _______________________________________________
> eclipselink-dev mailing list
> eclipselink-dev@xxxxxxxxxxx
<mailto:eclipselink-dev@xxxxxxxxxxx>
<mailto:eclipselink-dev@xxxxxxxxxxx
<mailto:eclipselink-dev@xxxxxxxxxxx>>
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
>
>
>
>
------------------------------------------------------------------------
>
> _______________________________________________
> eclipselink-dev mailing list
> eclipselink-dev@xxxxxxxxxxx <mailto:eclipselink-dev@xxxxxxxxxxx>
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx <mailto: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
