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

Update DTDs that are < Java EE 8 #474

Merged
merged 11 commits into from Sep 5, 2020
Merged

Conversation

scottmarlow
Copy link
Contributor

@scottmarlow scottmarlow commented Aug 24, 2020

#266

Replaced ejb-jar_1_1.dtd, application_1_2.dtd, application-client_1_2.dtd, connector_1_0.dtd, application-client_1_3.dtd, application_1_3.dtd, ejb-jar_2_0.dtd, web-facesconfig_1_0.dtd, web-facesconfig_1_1.dtd with equivalent Jakarta EE 9 XSDs.

@scottmarlow
Copy link
Contributor Author

I reviewed the changes and they look fine to me, would be good if at least one more person reviews these changes. Thanks!

Copy link
Contributor

@RohitKumarJain RohitKumarJain left a comment

Choose a reason for hiding this comment

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

LGTM

@scottmarlow
Copy link
Contributor Author

scottmarlow commented Aug 25, 2020

Thanks @alwin-joseph for testing these changes via https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-alw/job/pr%252F474/2/testReport/

  1. It seems that more changes are needed, as you pointed out, some of the failures may be (J2EE) compat tests (like https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-alw/job/pr%252F474/2/testReport/com.sun.ts.tests.assembly.compat.cocktail.compat12_14/Client/jakartaeetck_run___assembly___test12DD/). I'm not sure if we should update the deployment descriptors to be compatible with EE 9 or remove/disable the failing (compat) tests.

  2. For the connector failures like https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-alw/job/pr%252F474/2/testReport/com.sun.ts.tests.connector.connManager/connManagerClient1/jakartaeetck_run___connector___testTransactionSupportLevels_from_jsp, the tests disagree with the https://jakarta.ee/xml/ns/jakartaee/connector_2_0.xsd. From src/com/sun/ts/tests/common/connector/whitebox/ra-compat-tx.xml:

<connector xmlns="https://jakarta.ee/xml/ns/jakartaee"
           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
           metadata-complete="false"
           xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee
           https://jakarta.ee/xml/ns/jakartaee/connector_2_0.xsd"
           version="2.0">
    <display-name>oldwhitebox-tx.rar</display-name>
    <vendor-name>Java Software</vendor-name>
    <spec-version>1.0</spec-version>
    <eis-type>TS EIS</eis-type>
    <version>1.6</version>
    <resourceadapter>
        <managedconnectionfactory-class>com.sun.ts.tests.common.connector.whitebox.LocalTxMCF</managedconnectionfactory-class>
        <connectionfactory-interface>com.sun.ts.tests.common.connector.whitebox.TSConnectionFactory</connectionfactory-interface>
        <connectionfactory-impl-class>com.sun.ts.tests.common.connector.whitebox.TSEISDataSource</connectionfactory-impl-class>
        <connection-interface>com.sun.ts.tests.common.connector.whitebox.TSConnection</connection-interface>
        <connection-impl-class>com.sun.ts.tests.common.connector.whitebox.TSEISConnection</connection-impl-class>
        <transaction-support>LocalTransaction</transaction-support>
        <authentication-mechanism>
            <authentication-mechanism-type>BasicPassword</authentication-mechanism-type>
            <credential-interface>jakarta.resource.spi.security.PasswordCredential</credential-interface>
        </authentication-mechanism>
        <reauthentication-support>false</reauthentication-support>
    </resourceadapter>
</connector>

However, from https://jakarta.ee/xml/ns/jakartaee/connector_2_0.xsd:

<xsd:restriction base="jakartaee:fully-qualified-classType">
<xsd:enumeration value="jakarta.resource.spi.security.PasswordCredential"/>
<xsd:enumeration value="org.ietf.jgss.GSSCredential"/>
<xsd:enumeration value="jakarta.resource.spi.security.GenericCredential"/>
</xsd:restriction>

<xsd:element name="connectionfactory-interface" type="jakartaee:fully-qualified-classType">
<xsd:annotation>
<xsd:documentation>
The element connectionfactory-interface specifies the fully qualified name of the ConnectionFactory interface supported by the resource adapter. Example: <connectionfactory-interface>com.wombat.ConnectionFactory </connectionfactory-interface> OR <connectionfactory-interface>jakarta.resource.cci.ConnectionFactory </connectionfactory-interface>
</xsd:documentation>
</xsd:annotation>
</xsd:element>

However, connectionfactory-interface in http://java.sun.com/dtd/connector_1_0.dtd didn't seem to be restricted. Either the connector tests that depend on http://java.sun.com/dtd/connector_1_0.dtd need to be updated or disabled or the https://jakarta.ee/xml/ns/jakartaee/connector_2_0.xsd updated to allow connectionfactory-interface to be of any type. Note that I didn't debug this, I'm just going by the test failure output java.lang.IllegalArgumentException: [com.sun.ts.tests.common.connector.whitebox.TSConnectionFactory] is not an allowed property value type which isn't exactly clear as to what the requirement is.

@smillidge
Copy link

My understanding is jakartaee:fully-qualified-classType just means a fully qualified class name and com.sun.ts.tests.common.connector.whitebox.TSConnectionFactory is a fully qualified classname?

@smillidge
Copy link

Looking at the exception there is an classloading problem. The error is being thrown by https://github.com/eclipse-ee4j/glassfish/blob/8ec34f93134c4857e8e6b2fba9914e897a426caf/appserver/deployment/dol/src/main/java/com/sun/enterprise/deployment/ResourceReferenceDescriptor.java#L520 so I don't think this is a schema check.

@scottmarlow

This comment has been minimized.

Signed-off-by: Scott Marlow <smarlow@redhat.com>
Signed-off-by: Scott Marlow <smarlow@redhat.com>
Signed-off-by: Scott Marlow <smarlow@redhat.com>
Signed-off-by: Scott Marlow <smarlow@redhat.com>
Signed-off-by: Scott Marlow <smarlow@redhat.com>
Signed-off-by: Scott Marlow <smarlow@redhat.com>
Signed-off-by: Scott Marlow <smarlow@redhat.com>
…nnector_2_0.xsd doesn't allow spec-version

Signed-off-by: Scott Marlow <smarlow@redhat.com>
Signed-off-by: Scott Marlow <smarlow@redhat.com>
…o update ts.jte files, glassfish/connector.xml + Connector1.6TestPlan.html

Signed-off-by: Scott Marlow <smarlow@redhat.com>
@scottmarlow
Copy link
Contributor Author

scottmarlow commented Sep 4, 2020

Summary of changes:

  1. application_1_2.dtd => application_9.xsd.
  2. application_1_3.dtd => application_9.xsd.
  3. ejb-jar_*.dtd => ejb-jar_4_0.xsd.
  4. application-client_1_*.dtd => application-client_9.xsd.
  5. web-facesconfig_1_*.dtd => web-facesconfig_3_0.xsd.
  6. Removed description from application client deployment descriptors.
  7. connector_1_0.dtd' => connector_2_0.xsd.
  8. Removed spec-version from ra.xml since https://jakarta.ee/xml/ns/jakartaee/connector_2_0.xsd doesn't allow spec-version.
  9. Removed Connector 1.0 compat tests, updated ts.jte files, glassfish/connector.xml, Connector1.6TestPlan.html.

@scottmarlow
Copy link
Contributor Author

scottmarlow commented Sep 4, 2020

One more change was needed for the older ejb deployment descriptors that contain <reentrant>False</reentrant> which must be lowercased.

Signed-off-by: Scott Marlow <smarlow@redhat.com>
Copy link
Contributor

@gurunrao gurunrao left a comment

Choose a reason for hiding this comment

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

LGTM

@scottmarlow scottmarlow merged commit e579846 into jakartaee:master Sep 5, 2020
@smillidge
Copy link

We now have TCK failures in GlassFish relating to parsing old deployment descriptors should those tests be removed?
https://ci.eclipse.org/jakartaee-tck/job/eftl-jakartaeetck-run-900/28/junit-reports-with-handlebars/test-summary/c43b6fb0-736e-4a01-a782-d3b4f6ecc0d6.html

@scottmarlow
Copy link
Contributor Author

I'm not sure @smillidge!

@dblevins please see above question about removing failing BMP compatibility tests that I think are for testing legacy EJB DTDs that are not supported in Jakarta EE 9:

Should we remove these tests from the Jakarta EE 9 Platform TCK? Another option would be to exclude them.

Perhaps we need more changes to https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/ejb/ee/deploy/entity/bmp/compat13_14/META-INF/ejb-jar.xml#L40 for the compat_13_14 tests to work.

From the TCK test output in run_jakartaeetck.log [runcts] OUT => [javatest.batch] 878: Error occurred during deployment: Exception while deploying the app [ejb_depEbmp_compat13_14] : org.xml.sax.SAXParseException; lineNumber: 37; columnNumber: 21; Deployment descriptor file META-INF/ejb-jar.xml in archive [ejb_depEbmp_compat13_14_ejb_jar]. cvc-complex-type.2.4.a: Invalid content was found starting with element 'resource-ref'. One of '{"https://jakarta.ee/xml/ns/jakartaee":query}' is expected.. Please see server.log for more details.]]

We do pass https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/ejb/ee/deploy/entity/bmp/compat12_14/Client.java.

@scottmarlow
Copy link
Contributor Author

scottmarlow commented Sep 8, 2020

Alwin also mentioned:

  1. https://ci.eclipse.org/jakartaee-tck/job/eftl-jakartaeetck-run-900/28/testReport/junit/com.sun.ts.tests.ejb.ee.deploy.entity.bmp.compat13_14/Client/test13DD/
  2. https://ci.eclipse.org/jakartaee-tck/job/eftl-jakartaeetck-run-900/28/testReport/junit/com.sun.ts.tests.ejb.ee.deploy.entity.cmp11.compat12_13/Client/test12DD/ &
  3. https://ci.eclipse.org/jakartaee-tck/job/eftl-jakartaeetck-run-900/28/testReport/junit/com.sun.ts.tests.ejb.ee.deploy.entity.cmp11.compat12_14/Client/test12DD/

1 fails due to “Invalid content was found starting with element ‘resource-ref’”
2 & 3 fails due to
” 15095: Caused by: java.lang.RuntimeException: JDO83002: Caught org.glassfish.deployment.common.DeploymentException while processing CMP beans for application [ejb_depEcmp11_compat12_13]; module [ejb_depEcmp11_compat12_13_ejb.jar]: 2.x CMP bean class com.sun.ts.tests.ejb.ee.deploy.entity.cmp11.compat12_13.TestBeanEJB must be decleared abstract or cmp-version for the corresponding bean must be set to 1.x.. ”

@scottmarlow scottmarlow deleted the dtds branch September 8, 2020 15:57
@scottmarlow
Copy link
Contributor Author

@smillidge @dblevins https://www.eclipse.org/lists/jakartaee-tck-dev/msg00957.html asks (on ejb-dev@eclipse.org + TCK ml) if we can remove the failing J2EE 1.2-1.4 EJB compatibility tests.

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.

None yet

5 participants