-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@DataSourceDefinition defined data source can't be used in persistence.xml #510
Comments
This appears to work in CargoTracker OK (at least for the web.xml defined DataSource) is there some specific condition where this fails? see code referenced below; |
I'll double check and retest with the latest Payara later today. Possibly I used an older version. |
We've not done anything special to fix it! I did notice after I posted the comment CargoTracker uses java:global whereas your examples use java:app. |
java:global is indeed handled distinctively different at places. This also shows up here: https://java.net/jira/browse/GLASSFISH-21447 In that case it's a matter of the context not being set up. Let's see if this indeed makes a difference here as well. |
I don't think we can take the code you've added to the GlassFish JIRA and incorporate it into Payara Server, until it ends up in the GlassFish source tree. We can though take a shot at reimplementing that fix or you can raise a pull request with a signed CLA. |
You mean because I posted it on the GF JIRA it's already licensed? It would have to be slightly re-implemented anyway, since the code as presented is just a quick POC. For the event being thrown I just took a random existing one that didn't have any further side-effects apart from setting the context. But the real fix should likely introduce a dedicated event, and the try/catch for the validateRequest method should be moved down a little, into the validate() method. But that fix is of course for JASPIC. The fix for the DataSourceDefinition problem (if it's indeed still a problem) could potentially be done via a fix that's similar in approach but ultimately different code. At any length I'll ask around for the GF people to incorporate the fix into the GlassFish source tree. |
Not that it is licensed by Oracle just not to us. |
I took another good look at your example, and I've spotted the main difference that causes the problem. Your code is using the (semi-external) derby database that ships with GlassFish/Payara. Notice how your pom.xml doesn't have a dependency for derby. The Java EE 7 samples test includes its own database/jdbc driver; it's embedded in the war (in this case it's h2). The app or global space appeared to be irrelevant here. With the following in web.xml:
and no dependency for derby in the pom.xml (hence no derby jar in the war), the test passes. I tested on both GlassFish (4.1.1) and Payara. Even tried an older Payara 4.1 version and there it worked too. So it's either that the combination {embedded database/driver, data-source in web.xml/annotation, persistence unit} is the problem, OR, it's simply that the h2 embedded database doesn't work with Payara in combination with JPA and/or transactions, I quickly tried to use another embedded database (went for HSQLDB), but unfortunately HSQLDB seems extraordinary complicated to setup for use with a data-source/jpa, so to rule out h2 as the guilty party I need some more time. |
Turned out HSQLDB wasn't that difficult to setup, just Payara throwing errors. I added the following data source in web.xml:
The following dependency in pom.xml
And the following in the Arquillian test class:
To be clear, it's this exact test: https://github.com/javaee-samples/javaee7-samples/tree/master/jpa/datasourcedefinition-webxml-pu This runs flawlessly on JBoss (WildFly 10.0 rc4) and even on WebLogic (12.2.1), but on Payara and GlassFish results in the following exceptions:
And
So there's definitely something fishy (no pun ;)) going on with Payara and GlassFish here. |
At what point is the first exception being thrown, during deployment or when the EJB is called from the second exception? I didn't realise you could package the driver jar into the war file. Is that part of the spec? I'd always assumed, but never checked, that you dropped the driver somewhere on the server's boot classpath. |
Both exceptions are thrown when the EJB is called. Because of the sheer size of the exceptions being printed it can be a little hard to follow. For some reason pretty much the same exception is printed 7 times. This is the full exception of a single print out:
I did however just notice that the first time after Payara/GlassFish is started and the test is deployed, the following appears in the log:
Especially the following stands out: "The web application [unknown] registered the JDBC driver [org.hsqldb.jdbc.JDBCDriver] but failed to unregister it when the web application was stopped. To prevent a memory leak, the JDBC Driver has been forcibly unregistered."
This was among others discussed in the following issue: https://java.net/jira/browse/JAVAEE_SPEC-7 After some discussion the spec lead said: "Note also that the defined way to package a JDBC driver with an application Still, it seems like putting the JDBC driver in WEB-INF/lib should also work. If it doesn't, please file a bug against GlassFish. That bug was filed as: https://java.net/jira/browse/GLASSFISH-19451 It initially looked like to be silently fixed, as it seems an embedded JDBC driver itself can be used since GlassFish 4, but apparently not with JPA. So basically we have the following 3 issues:
I think it's item 3 that is the one that fails now. |
Thanks for the detailed input. This is definitely something we will look to address. I would also like to get all your JASPIC tests to Green on Payara. |
You're welcome, would be great if this could be fixed.
I've got them all green locally, well, except for the JSF include but that's extremely likely to be more of a JSF issue. As I'm a Mojarra committer I'm going to look at that one soon. As for the EJB calls that fail; right now I'm trying to see what the best course of action is. I've asked around what it takes to get the fix committed in GlassFish, but I've not yet gotten a definite reply. Alternatively we could see if it can be fixed in Payara and then eventually contribute that back to GlassFish. |
A possible related exception was thrown here: https://travis-ci.org/javaee-samples/javaee7-samples/jobs/93063766 This test case is similar to the one I mentioned above, but it used the XA data source:
In this case it throws:
For the other containers hsqldb doesn't throw this exception, so I assume it's related to how GlassFish sets the username/password. |
The exception below is spurious and has no impact and has been raised with EclipseLink https://bugs.eclipse.org/bugs/show_bug.cgi?id=463629#c10 Warning: java.lang.NullPointerException
at org.eclipse.persistence.platform.server.ServerPlatformUtils.createServerPlatform(ServerPlatformUtils.java:99)
at org.eclipse.persistence.sessions.factories.SessionManager.init(SessionManager.java:77) |
Reproduced java.lang.IllegalStateException: This web container has not yet been started
at org.glassfish.web.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1674)
at org.glassfish.web.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1633)
at org.hsqldb.jdbc.JDBCResultSet.getMetaData(Unknown Source) |
Done some debugging on this and it looks like the DeploymentClassLoader is being used to try and load driver classes rather than the class loader of the web application. Not sure whether this is a specific H2 Driver issue or an issue with Payara. It looks like the TCCL is not being used to load classes in the driver. |
Okay, that's at least a step closer ;) To rule out H2 specifically is why I also tried with hsqldb. |
One other perhaps related issue; when the above shown data-source configuration is used, but the global namespace is used (e.g. When the application is subsequently redeployed, the data source will still exist, and when actually using it will lead to class loader problems at some point, for instance the "This web container has not yet been started" by the WebappClassLoader (See http://grepcode.com/file/repo1.maven.org/maven2/org.glassfish.main.web/war-util/4.1/org/glassfish/web/loader/WebappClassLoader.java#1674) I checked what some other servers do, and at least JBoss (WildFly 10rc4) does indeed unbind the application provided data source when that application is undeployed. |
Unbinding of the application datasource is now fixed see pull request #565 however I'm not sure this is the cause as it was just the JNDI entry floating around. I think there may be some interplay with JPA as this example deploys fine; https://github.com/payara/Payara-Examples/tree/master/Payara-Micro/datasource-example although it doesn't use JPA. I will extend this example to use JPA as a test case. |
I created an example application using MySQL and can deploy the datasource with a JPA PU no problem on current Payara snapshot. See https://github.com/payara/Payara-Examples/tree/master/Payara-Micro/jpa-datasource-example |
The example application seems to exercise all the parts that seemingly cause the failure. Did you try running the actual examples from the Java EE 7 samples project on the current Payara snapshot? If nothing else was fixed, then those "should" still fail. The differences with the jpa-datasource-example vs Java EE 7 samples are getting smaller though. The main difference is perhaps that the latter uses java:app and h2 + hsqldb. |
…nit as it loads driver classes This fixes payara#510
This was due to the classloader that initially loaded the Driver classes being a different classloader from the final application classloader. Resulting in a problem whereby when further classes needed to be loaded from the driver an incorrect classloader was used. The fix is to force the PU loading to use the final application classloader rather than the initial prepare phase deployment classloader. |
Also now added support via #586 for all custom Payara Datasource properties so you can enable connection validation etc. For example in web.xml; <data-source>
<name>java:global/JPAExampleDataSource</name>
<class-name>com.mysql.jdbc.jdbc2.optional.MysqlXADataSource</class-name>
<server-name>localhost</server-name>
<port-number>3306</port-number>
<database-name>mysql</database-name>
<user>test</user>
<!-- Example of using a Payara password alias in the datasource definition -->
<password>test</password>
<!-- Example of how to use a Payara specific custom connection pool setting -->
<property>
<name>fish.payara.slow-query-threshold-in-seconds</name>
<value>5</value>
</property>
<property>
<name>fish.payara.log-jdbc-calls</name>
<value>true</value>
</property>
<property>
<name>fish.payara.is-connection-validation-required</name>
<value>true</value>
</property>
<property>
<name>fish.payara.connection-validation-method</name>
<value>custom-validation</value>
</property>
<property>
<name>fish.payara.validation-classname</name>
<value>org.glassfish.api.jdbc.validation.MySQLConnectionValidation</value>
</property>
</data-source> |
Very nice! This is still something I hope that can be addressed better by the spec somehow. Currently it's a bit of a hit or miss whether the standard {{data-source}} element supports the same properties that the proprietary version of the same element supports. |
FISH-5824 Added Subresource Integrity for monitoring console
Defining a data source from within the application using either @DataSourceDefinition on a class or the data-source element in web.xml, and then using this in persistence.xml will cause a deployment failure.
See https://java.net/jira/browse/GLASSFISH-20944
The text was updated successfully, but these errors were encountered: