Skip to content
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

[issue#211] Remove use of Jakarta Deployment and classes that use it (no replacement deployment interfaces added) #292

Merged
merged 2 commits into from
Jun 4, 2020

Conversation

scottmarlow
Copy link
Contributor

import javax.enterprise.deploy.model.*;

import java.io.*;
import java.util.*;

import javax.enterprise.deploy.shared.factories.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you planning to remove the pending occurrences of javax.enterprise.deploy also. Will that be done in another change ?

Copy link
Contributor Author

@scottmarlow scottmarlow May 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alwin-joseph
I thought I removed all of the references, I will add an additional commit to remove the missed ones.

I'll also remove the added wget of https://repo1.maven.org/maven2/javax/enterprise/deploy/javax.enterprise.deploy-api/1.7/javax.enterprise.deploy-api-1.7.jar in docker/build_jakartaeetck.sh, as we shouldn't reference those classes any more.

The same for docker/build_standalone-tcks.sh.

Copy link
Contributor Author

@scottmarlow scottmarlow May 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alwin-joseph according to jakartaee/src/main/jbake/content/commonappdeploy.adoc, there are additional tests that depend on Jakarta Deployment, however, we removed Jakarta Deployment, so these tests will not work.

|Directory Under `com/sun/ts/tests` |Directory Under `com/sun/ts/tests`
With Associated Common Applications
|`ejb/ee/tx/session` |`ejb/ee/tx/txbean`

|`ejb/ee/tx/entity/bmp` |`ejb/ee/tx/txEbean`

|`ejb/ee/tx/entity/cmp` |`ejb/ee/tx/txECMPbean`

|`ejb/ee/tx/entity/pm` |`ejb/ee/tx/txEPMbean`

|`connector/ee/localTx/msginflow` |`common/connector/whitebox`

|`connector/ee/mdb` |`connector/ee/localTx`

|`common/connector/whitebox` |`connector/ee/noTx`

|`common/connector/whitebox` |`connector/ee/xa`

|`common/connector/whitebox` |`connector/ee/connManager`

|`common/connector/whitebox` |`xa/ee`

|`compat13/connector/localTx` |`compat13/connector/whitebox`

|`compat13/connector/noTx` |`compat13/connector/whitebox`

|`compat13/connector/xa` |`compat13/connector/whitebox`

|`interop/tx/session` |`interop/tx/txbean`

|`interop/tx/entity` |`interop/tx/txEbean`

|`interop/tx/webclient` |`interop/tx/txbean`

|`tests/interop/csiv2` |`interop/csiv2/rionly`

|`ejb/ee/pm/ejbql` |`ejb/ee/pm/ejbql/schema`

|`ejb/ee/tx/session/stateful/bm/TxMDBMS_Direct`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanA`

|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanB`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanC`

|`tests/ejb/ee/tx/session/stateful/bm/TxMDBMS_Indirect`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanA`

|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanB`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanC`

|`ejb/ee/tx/session/stateful/bm/TxMDBSS_Direct`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanA`

|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanB`
|`tests/ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanC`

|`ejb/ee/tx/session/stateful/bm/TxMDBSS_Indirect`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanA`

|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanB`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanC`

src/com/sun/ts/lib/harness/commonarchives.properties seems to have a different list, so the above mentioned doc file may be out of date.

Copy link
Contributor Author

@scottmarlow scottmarlow Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking that the Common Applications Deployment will not work without Jakarta Deployment. If that is the case, we should remove the Common Applications Deployment tests, docs, support classes. In theory, we could introduce an alternative to Jakarta Deployment as discussed but we don't have that in place yet, so likely need to remove CAD.

Also, we already discussed removing Common Applications Deployment on the mailing list, so will do that next.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated plan, to try to get through the build with failures, will remove Common Applications Deployment in a separate pull request.

@scottmarlow
Copy link
Contributor Author

Will merge this now, further feedback is still appreciated! As mentioned above we still expect to remove the Common Application Deployment in a separate pull request.

@scottmarlow scottmarlow merged commit f055e36 into jakartaee:master Jun 4, 2020
deployManageruri.2=deployer:Sun:AppServer::${deployment_host.2}:${ri.admin.port}
deployManageruname.2=${ri.admin.user}
deployManagerpasswd.2=${ri.admin.passwd}
#deployManagerJarFile.2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Property fetching causing exception:

As of now there are exceptions while sample test run as below :
" com.sun.ts.lib.deliverable.PropertyNotSetException: No value found for deployManagerJarFile.1. Please check your jte file for an appropiate value " from ./src/com/sun/ts/lib/deliverable/AbstractPropertyManager.java
So looks like it does not allow null for these currently when fetched at src/com/sun/ts/lib/deliverable/cts/CTSPropertyManager.java

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alwin-joseph, I will look at this.

Copy link
Contributor Author

@scottmarlow scottmarlow Jun 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are now getting execStatus=Failed. Illegal access to test\: java.lang.NullPointerException

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alwin-joseph from the ./root/vi/glassfish6/glassfish/domains/domain1/logs/server.log from samples-results.tar.gz:

[2020-06-08T17:09:01.779+0000] [glassfish 6.0] [INFO] [NCLS-DEPLOYMENT-02027] [javax.enterprise.system.tools.deployment.autodeploy] [tid: _ThreadID=54 _ThreadName=AutoDeployer] [timeMillis: 1591636141779] [levelValue: 800] [[
  Selecting file /root/vi/glassfish6/glassfish/domains/domain1/autodeploy/ejb_sam_Hello.ear for autodeployment]]
...
[2020-06-08T17:09:03.332+0000] [glassfish 6.0] [SEVERE] [NCLS-CORE-00029] [javax.enterprise.system.core] [tid: _ThreadID=54 _ThreadName=AutoDeployer] [timeMillis: 1591636143332] [levelValue: 1000] [[
  Cannot start container ejb, exception: A MultiException has 7 exceptions.  They are:
1. com.sun.enterprise.module.ResolveError: Failed to start OSGiModuleImpl:: Bundle = [org.glassfish.main.orb.iiop [218]], State = [NEW]
2. java.lang.IllegalStateException: Could not load descriptor SystemDescriptor(
        implementation=org.glassfish.enterprise.iiop.impl.GlassFishORBFactoryImpl
        contracts={org.glassfish.enterprise.iiop.impl.GlassFishORBFactoryImpl,org.glassfish.enterprise.iiop.api.GlassFishORBFactory}
        scope=jakarta.inject.Singleton
        qualifiers={}
        descriptorType=CLASS
        descriptorVisibility=NORMAL
        metadata=Bundle-SymbolicName={org.glassfish.main.orb.iiop},Bundle-Version={6.0.0.SNAPSHOT}
        rank=0
        loader=OsgiPopulatorPostProcessor.HK2Loader(OSGiModuleImpl:: Bundle = [org.glassfish.main.orb.iiop [218]], State = [NEW],655682992)
        proxiable=null
        proxyForSameScope=null
        analysisName=null
        id=110
        locatorId=0
        identityHashCode=444705614
        reified=false)
3. java.lang.IllegalStateException: Unable to perform operation: post construct on org.glassfish.enterprise.iiop.api.GlassFishORBHelper
4. java.lang.IllegalArgumentException: While attempting to resolve the dependencies of com.sun.ejb.containers.EjbContainerUtilImpl errors were found
5. java.lang.IllegalStateException: Unable to perform operation: resolve on com.sun.ejb.containers.EjbContainerUtilImpl
6. java.lang.IllegalArgumentException: While attempting to resolve the dependencies of org.glassfish.ejb.startup.EjbContainerStarter errors were found
7. java.lang.IllegalStateException: Unable to perform operation: resolve on org.glassfish.ejb.startup.EjbContainerStarter

NPE call stack is

[runcts] OUT => [javatest.batch] java.lang.NullPointerException
   [runcts] OUT => [javatest.batch]     at com.sun.ts.lib.harness.TSScript.run(TSScript.java:320)
   [runcts] OUT => [javatest.batch]     at com.sun.javatest.Script.run(Script.java:241)
   [runcts] OUT => [javatest.batch]     at com.sun.javatest.DefaultTestRunner.runTest(DefaultTestRunner.java:174)
   [runcts] OUT => [javatest.batch]     at com.sun.javatest.DefaultTestRunner.access$100(DefaultTestRunner.java:43)
   [runcts] OUT => [javatest.batch]     at com.sun.javatest.DefaultTestRunner$1.run(DefaultTestRunner.java:66)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created minor NPE fix via #311.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed some possible changes to https://github.com/scottmarlow/jakartaee-tck/tree/moreDeploymentRemoval

This issue is fixed now in the latest build. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created minor NPE fix via #311.

Scott - From what I see the root problem is that the deployment fails due to some reason. I am not sure if the fix is addressing that. The deployment failures are there for several packages even for standalone tck runs and need to be looked into. We can use the tck bundles built and try them in local to check the server logs. It is also possible something is wrong with the deployment rar/war or with glassfish itself.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the server logs :

[2020-06-08T16:58:42.522+0000] [glassfish 6.0] [WARNING] [] [] [tid: _ThreadID=52 _ThreadName=AutoDeployer] [timeMillis: 1591635522522] [levelValue: 900] [[
Error occurred
org.xml.sax.SAXParseException; lineNumber: 52; columnNumber: 106; Deployment descriptor file META-INF/ra.xml in archive [whitebox-tx.rar]. cvc-enumeration-valid: Value 'jakarta.resource.spi.security.PasswordCredential' is not facet-valid with respect to enumeration '[javax.resource.spi.security.PasswordCredential, org.ietf.jgss.GSSCredential, javax.resource.spi.security.GenericCredential]'. It must be a value from the enumeration.

I see glassfish6/glassfish/lib/schemas/connector_1_7.xsd & glassfish6/glassfish/lib/dtds/connector_1_5.dtd having javax.resource.spi.security.PasswordCredential. I suspect these are to be changed to jakarta.* in glassfish ?

Copy link
Contributor Author

@scottmarlow scottmarlow Jun 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, yes, I agree that glassfish should update these.
I did find eclipse-ee4j/glassfish#23080 which is for the ORB failure, that we also need fixed I think.

Links for updated XSDs:
https://jakarta.ee/xml/ns/jakartaee/connector_2_0.xsd
https://github.com/eclipse-ee4j/jakartaee-schemas/blob/master/jakartaee9/src/connector_2_0.xsds

#266 is for updating the Platform TCK to use EE 9 XSDs

https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/common/connector/whitebox/permissiondd/ra.xml is one of the files that do need to switch to the EE 9 XSDs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants