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

The jersey-gf-ejb was adopted by the GlassFish as jersey-ejb-component-provider #4920

Open
wants to merge 1 commit into
base: 3.1
Choose a base branch
from

Conversation

dmatej
Copy link
Contributor

@dmatej dmatej commented Dec 1, 2021

  • it was a circular dependency between jersey and glassfish
  • jersey limited usage of glassfish to versions 4-6, but now started gf7 development
  • see commit a8cfbe9fc7cd4e662ea044c111569804b5174c2f in glassfish

@arjantijms
Copy link
Contributor

[INFO] jersey-examples-system-properties .................. SUCCESS [  2.171 s]
[INFO] jersey-examples-moxy ............................... SUCCESS [  2.305 s]
[INFO] jersey-test-framework-maven-custom-enforcer-rules .. SUCCESS [  8.542 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  43:17 min
[INFO] Finished at: 2021-12-01T14:11:58Z
[INFO] ------------------------------------------------------------------------
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] }
[Pipeline] // stage
[Pipeline] }
[Pipeline] // parallel
[Pipeline] }
[Pipeline] // stage
[Pipeline] End of Pipeline
ERROR: script returned exit code 1

GitHub has been notified of this commit’s build result

It looks like the build succeeds and everything passes, but it just out of the blue says fails anyway.

@dmatej
Copy link
Contributor Author

dmatej commented Dec 1, 2021

This is the failure from Jenkins:

ERROR: Bundle org.glassfish.jakarta.el [46] Error starting file:/tmp/1638367545154-0/pax-exam-downloads/org.glassfish.jakarta.el_4.0.0.jar (org.osgi.framework.BundleException: Unable to resolve org.glassfish.jakarta.el [46](R 46.0): missing requirement [org.glassfish.jakarta.el [46](R 46.0)] osgi.wiring.package; (&(osgi.wiring.package=jakarta.el)(version>=4.0.0)(!(version>=5.0.0))) Unresolved requirements: [[org.glassfish.jakarta.el [46](R 46.0)] osgi.wiring.package; (&(osgi.wiring.package=jakarta.el)(version>=4.0.0)(!(version>=5.0.0)))])
org.osgi.framework.BundleException: Unable to resolve org.glassfish.jakarta.el [46](R 46.0): missing requirement [org.glassfish.jakarta.el [46](R 46.0)] osgi.wiring.package; (&(osgi.wiring.package=jakarta.el)(version>=4.0.0)(!(version>=5.0.0))) Unresolved requirements: [[org.glassfish.jakarta.el [46](R 46.0)] osgi.wiring.package; (&(osgi.wiring.package=jakarta.el)(version>=4.0.0)(!(version>=5.0.0)))]
	at org.apache.felix.framework.Felix.resolveBundleRevision(Felix.java:4398)
	at org.apache.felix.framework.Felix.startBundle(Felix.java:2308)
	at org.apache.felix.framework.Felix.setActiveStartLevel(Felix.java:1566)
	at org.apache.felix.framework.FrameworkStartLevelImpl.run(FrameworkStartLevelImpl.java:308)
	at java.base/java.lang.Thread.run(Thread.java:831)
[org.ops4j.pax.exam.forked.ForkedTestContainer] ERROR : Bundle [id:46, url:mvn:org.glassfish/jakarta.el/4.0.0] is not resolved

It is not caused by this PR

@dmatej
Copy link
Contributor Author

dmatej commented Dec 1, 2021

Hmmmm, but the failure is caused by Read timeout when some test tried to download this. So it should be enough to rerun the build. Btw jakarta.el is downloaded several times, I don't know what the test is doing, but it is suspicious, especially with so extreme logging.

Caused by: shaded.org.eclipse.aether.resolution.ArtifactResolutionException: Error resolving artifact org.ops4j.pax.tipi:org.ops4j.pax.tipi.junit:jar:4.12.0.1
	at shaded.org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:413)
	at shaded.org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:215)
	at shaded.org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifact(DefaultArtifactResolver.java:192)
	at shaded.org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveArtifact(DefaultRepositorySystem.java:247)
	at org.ops4j.pax.url.mvn.internal.AetherBasedResolver.resolve(AetherBasedResolver.java:767)
	... 33 more
[org.ops4j.pax.exam.spi.reactors.ReactorManager] INFO : suite finished
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 8.883 s <<< FAILURE! - in org.glassfish.jersey.osgi.test.basic.BasicOsgiIntegrationTest
[ERROR] testSimpleResource(org.glassfish.jersey.osgi.test.basic.BasicOsgiIntegrationTest)  Time elapsed: 8.723 s  <<< ERROR!
shaded.org.eclipse.aether.resolution.ArtifactResolutionException: Error resolving artifact org.ops4j.pax.tipi:org.ops4j.pax.tipi.junit:jar:4.12.0.1

@jansupol
Copy link
Contributor

jansupol commented Dec 1, 2021

Dropping the EJB module looks suspicious. Will need to test this a bit.
A couple of concerns:

  • Need to run the TCK to see the EJB tests are passing. Jersey does not have EJB tests, the EJB tests were implemented when embedded GF did not support EJBs and hence the testing was done manually on full GF
  • Need to check the module is used just by GF

Pros:

  • Resolved the cyclic dependency
  • Dropped responsibility for EJB integration

At this point we probably do not need to drop the module, it just won't be integrated into the GF. But thank you @dmatej for letting us know the changes in GF. Jersey is planned for the 3.1.0.M1 release quite soonish, I'd like to keep the module for M1 at least.

@jansupol
Copy link
Contributor

I am not sure we can drop this at all. The Spec requirement is to contain EJB support. Dropping the module will de-facto break the requirement. It might be ok, to have the support in the glassfish repository, but it can render Jersey as non-spec compliant if any issue with GF....Keeping the module might prevent such a situation...no matter if no one would use it...

@dmatej
Copy link
Contributor Author

dmatej commented Mar 18, 2022

Hmm, so then there should be some detached project providing standalone EJB container, usable for both Jersey and GlassFish.
It would be useful even more ie. for testing ... and should already exist for maybe 19 years :-)
I will think about it ...

@dmatej
Copy link
Contributor Author

dmatej commented Jan 31, 2023

I am not sure we can drop this at all. The Spec requirement is to contain EJB support. Dropping the module will de-facto break the requirement. It might be ok, to have the support in the glassfish repository, but it can render Jersey as non-spec compliant if any issue with GF....Keeping the module might prevent such a situation...no matter if no one would use it...

Is the EJB support important for the JAX-RS specs? Or would be easier for JAX-RS and Jersey too if the spec would drop this requirement?

Also I believe this is not a task of Jersey (Jersey repo welcome page). Max it should verify that some basic scenarios work with several known use cases close to the real usage:

Track the JAX-RS API and provide regular releases of production quality Reference Implementations that ships with GlassFish;

At least for now I can rebase the PR ...

@jansupol
Copy link
Contributor

Is the EJB support important for the JAX-RS specs? Or would be easier for JAX-RS and Jersey too if the spec would drop this requirement?

Yes, the EJB support is mandatory by the JAX-RS Spec for the Jakarta EE environment where the EJB container is available.

@dmatej
Copy link
Contributor Author

dmatej commented Jan 31, 2023

Is the EJB support important for the JAX-RS specs? Or would be easier for JAX-RS and Jersey too if the spec would drop this requirement?

Yes, the EJB support is mandatory by the JAX-RS Spec for the Jakarta EE environment where the EJB container is available.

I am talking about the NEXT version ;)
About those already existing I know that from your previous comments.

…t-provider

- it was a circular dependency between jersey and glassfish
- jersey limited usage of glassfish to versions 4-6, but now started
  gf7 development
- see commit a8cfbe9fc7cd4e662ea044c111569804b5174c2f in glassfish

# Conflicts:
#	containers/glassfish/jersey-gf-ejb/pom.xml
#	containers/glassfish/jersey-gf-ejb/src/main/java/org/glassfish/jersey/gf/ejb/internal/EjbComponentInterceptor.java
#	containers/glassfish/jersey-gf-ejb/src/main/java/org/glassfish/jersey/gf/ejb/internal/EjbComponentProvider.java
#	containers/glassfish/jersey-gf-ejb/src/main/java/org/glassfish/jersey/gf/ejb/internal/EjbExceptionMapper.java
#	containers/glassfish/pom.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants