Skip to content
This repository was archived by the owner on May 28, 2018. It is now read-only.

JERSEY-2085 fixed nested jar problem on Jersey. #196

Open
wants to merge 7 commits into
base: 2.21.x
Choose a base branch
from

Conversation

wuqunfei
Copy link

@jerseyrobot
Copy link
Contributor

Can one of the admins verify this patch?

@wuqunfei
Copy link
Author

I test on tag(2.14) and I think this problem is still in 2.21.x branch. It works on my side, please veriy it. Thanks.

@AdamLindenthal
Copy link
Member

Hi and thanks for your contribution.

As you already probably know, before we can review your pull request, we need you to sign Oracle Contributor Agreement.

Also, with exception of changes in the documentation, we need every single patch to be backed by a test proving, that the described functionality was broken and was fixed (among other reasons to ensure, that we don't waste your valuable work by accidentally reverting the behaviour as a side effect of another change).

More information about contributing to Jersey can be found at: https://jersey.java.net/scm.html#/Submitting_Patches_and_Contribute_Code

Thanks,
Adam

@AdamLindenthal
Copy link
Member

Jenkins, please test this patch.

@wuqunfei
Copy link
Author

Hello @AdamLindenthal, I fix style check problem. Please build and test again, I will send Oralce Contributor Agreement later.

@AdamLindenthal
Copy link
Member

Thanks, I'll re-trigger it. But please note, that for now, the result of the build is rather informative for you. As I mentioned, we require the patches to be accompanied by at least one (relevant) test. So the build will have to be launched again anyway as soon as you provide one...

Regards,
Adam

@AdamLindenthal
Copy link
Member

Jenkins, please test this patch.

@wuqunfei
Copy link
Author

I have send my agreement to Oralce already, I can provide a unit test of this JarZipSchemeResourceFinderFactory. But before that, there is not any unit test on this class....

@wuqunfei
Copy link
Author

hello @AdamLindenthal, could you tell me which profiles you used of pom 2.21x branch, than I can make a unit test of this class. Before that I test on tag(2.14) with profile includes (default, jdk1.8 and release).

@AdamLindenthal
Copy link
Member

I see. We are repackaging (an minimizing) the guava library. Just checked quickly and to me it looks like the default profile should work fine. Have you tried to rebuild from the console (e.g. mvn clean install -Dmaven.test.skip=true to speed it up a little)?

Hope this helps,
Adam

@wuqunfei
Copy link
Author

Thanks Adam, mvn clean install -Dmaven.test.skip=true

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 03:36 min
[INFO] Finished at: 2015-09-14T18:09:29+02:00
[INFO] Final Memory: 409M/1835M

@wuqunfei
Copy link
Author

I will test on mvn test, but it spends more time. And I need to go home now.
Thanks @AdamLindenthal

@AdamLindenthal
Copy link
Member

Sure, or just run the entire build without the test skipping property. It takes quite long indeed (lots of tests...)

PS: I hope you don't feel offended, that I shortened your previous post. The entire reactor summary was too long and would make this thread less readable.

Qunfei Wu added 3 commits September 14, 2015 23:45
…eption and block whole application.It will print some warning messages.
… don't throw exception and block whole application.It will print some warning messages.
@wuqunfei
Copy link
Author

Don't worry, I understand it, too long log is not meaningful. I do mvn test now, it works
[INFO] jersey-core-server ................................. SUCCESS [ 29.946 s]
But there are others failed in other module, you can bulid by your jenkins.

@wuqunfei
Copy link
Author

Hello @AdamLindenthal, can you test it on your jenkins again. we are waiting for your response....
Best Regards
Qunfei

@AdamLindenthal
Copy link
Member

Hi Qunfei, sure, I can run the build. However, please be aware, that without the change being tested, we cannot merge it.
Also, I didn't mention it earlier - please be prepared, that the OCA processing takes usually quite a while (several weeks). On the other hand, that should give you enough time for additional changes of the patch.

Have a nice day,
Adam

@AdamLindenthal
Copy link
Member

Jenkins, please test this patch.

@AdamLindenthal
Copy link
Member

From the screenshot it seems you've sent it to the incorrect address - the correct one is oracle-ca_us, not just ca_us.

Cheers,
Adam

@wuqunfei
Copy link
Author

thanks, i will test later and waiting theire response

@AdamLindenthal
Copy link
Member

Hi, it's a typo in the address again. You are sending the OCA to oralce instead to oracle...

@wuqunfei
Copy link
Author

thanks , it's a very stupid misktake.

@wuqunfei
Copy link
Author

Hello @AdamLindenthal
I try to test on local with branch, I can't pass style check of the module jersey-bundles-jaxrs-ri.

[INFO] Reactor Summary:
[INFO]
[INFO] jersey ............................................. SUCCESS [ 12.075 s]
[INFO] jersey-archetypes .................................. SUCCESS [ 0.687 s]
[INFO] jersey-archetype-heroku-webapp ..................... SUCCESS [ 2.181 s]
[INFO] jersey-archetype-grizzly2 .......................... SUCCESS [ 0.741 s]
[INFO] jersey-archetype-webapp ............................ SUCCESS [ 0.390 s]
[INFO] jersey-example-java8-webapp ........................ SUCCESS [ 16.171 s]
[INFO] jersey-bom ......................................... SUCCESS [ 0.461 s]
[INFO] jersey-bundles ..................................... SUCCESS [ 0.464 s]
[INFO] jersey-repackaged .................................. SUCCESS [ 0.369 s]
[INFO] jersey-repackaged-guava ............................ SUCCESS [ 12.856 s]
[INFO] jersey-repackaged-jsr166e .......................... SUCCESS [ 10.877 s]
[INFO] jersey-core-common ................................. SUCCESS [ 45.646 s]
[INFO] jersey-media ....................................... SUCCESS [ 0.427 s]
[INFO] jersey-media-jaxb .................................. SUCCESS [ 7.641 s]
[INFO] jersey-core-client ................................. SUCCESS [ 12.331 s]
[INFO] jersey-core-server ................................. SUCCESS [ 51.771 s]
[INFO] jersey-containers .................................. SUCCESS [ 0.495 s]
[INFO] jersey-container-servlet-core ...................... SUCCESS [ 3.486 s]
[INFO] jersey-container-servlet ........................... SUCCESS [ 2.124 s]
[INFO] jersey-bundles-jaxrs-ri ............................ FAILURE [ 11.276 s]

BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 03:18 min
[INFO] Finished at: 2015-09-23T00:19:32+02:00
[INFO] Final Memory: 234M/1317M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (verify) on project jaxrs-ri: You have 32 Checkstyle violations. ->

Do I need to disable Checkstyle plugin?

Best Regards,
Qunfei Wu

@wuqunfei
Copy link
Author

got it, I will check it soon

@driverpt
Copy link

I'll take a look @ the code to see if I can help.

@pavelbucek
Copy link
Member

@driverpt feel free to share your issues with @wuqunfei , but please don't contribute any code unless you sign OCA. (I just checked and I don't see your name at http://www.oracle.com/technetwork/community/oca-486395.html; we won't be able to accept the patch if you do that).

@driverpt
Copy link

driverpt commented Aug 16, 2016

@wuqunfei ,

What about changing from

final String ssp = uri.getRawSchemeSpecificPart();

to this

final String ssp = uri.toString();

/cc @pavelbucek @henrik242

@driverpt
Copy link

Anybody?

@pavelbucek
Copy link
Member

@sheng-wang-i true, but @driverpt is not there.

read my message again (https://github.com/jersey/jersey/pull/196#issuecomment-239796496); I stated: X, please work with Y, but (X) don't do any code changes, since you don't have an OCA.

Please keep this conversation on topic.

@wuqunfei
Copy link
Author

I will figure out the problem this weekend

On 19 Aug 2016 5:06 p.m., "Pavel Bucek" [email protected] wrote:

@sheng-wang-i https://github.com/sheng-wang-i true, but @driverpt
https://github.com/driverpt is not there.

read my message again (#196 (comment)
https://github.com/jersey/jersey/pull/196#issuecomment-239796496); I
stated: X, please work with Y, but (X) don't do any code changes, since you
don't have an OCA.

Please keep this conversation on topic.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jersey/jersey/pull/196#issuecomment-241044002, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFId0kGLtzVzNkeDGYV6f3FrecoiDP8-ks5qhcZ-gaJpZM4F8x2a
.

@dragontree101
Copy link

does this problem have been solved?

@driverpt
Copy link

driverpt commented Sep 9, 2016

+1

@duddns
Copy link

duddns commented Sep 21, 2016

does this problem have been solved?

@wuqunfei
Copy link
Author

@driverpt, I took your idea and commit it. But I don't have the environment to test it now. If anyone still need it, please test it. The error is still the file Path.

@elinger
Copy link

elinger commented Sep 23, 2016

+1

3 similar comments
@auramirea
Copy link

+1

@sebastian-hauf
Copy link

+1

@gernd
Copy link

gernd commented Sep 23, 2016

+1

@kcmvp
Copy link

kcmvp commented Oct 20, 2016

How is the status ?
I run into the same issue

org.springframework.boot:spring-boot-starter-jersey:jar:1.4.0.RELEASE:compile
+- org.glassfish.jersey.core:jersey-server:jar:2.23.1:compile
| +- org.glassfish.jersey.core:jersey-client:jar:2.23.1:compile
| +- org.glassfish.jersey.media:jersey-media-jaxb:jar:2.23.1:compile
+- org.glassfish.jersey.containers:jersey-container-servlet:jar:2.23.1:compile
+- org.glassfish.jersey.ext:jersey-bean-validation:jar:2.23.1:compile
+- org.glassfish.jersey.ext:jersey-spring3:jar:2.23.1:compile

  • org.glassfish.jersey.media:jersey-media-json-jackson:jar:2.23.1:compile
    +- org.glassfish.jersey.ext:jersey-entity-filtering:jar:2.23.1:compile

@kcmvp
Copy link

kcmvp commented Oct 20, 2016

Caused by: org.glassfish.jersey.server.internal.scanning.ResourceFinderException: java.io.FileNotFoundException: C:\sandbox\playground\JKD\hjd.jar!\BOOT-INF\classes (The system cannot find the path specified)
at org.glassfish.jersey.server.internal.scanning.JarZipSchemeResourceFinderFactory.create(JarZipSchemeResourceFinderFactory.java:89)
at org.glassfish.jersey.server.internal.scanning.JarZipSchemeResourceFinderFactory.create(JarZipSchemeResourceFinderFactory.java:65)
at org.glassfish.jersey.server.internal.scanning.PackageNamesScanner.addResourceFinder(PackageNamesScanner.java:282)
at org.glassfish.jersey.server.internal.scanning.PackageNamesScanner.init(PackageNamesScanner.java:198)
at org.glassfish.jersey.server.internal.scanning.PackageNamesScanner.(PackageNamesScanner.java:154)
at org.glassfish.jersey.server.internal.scanning.PackageNamesScanner.(PackageNamesScanner.java:110)
at org.glassfish.jersey.server.ResourceConfig.packages(ResourceConfig.java:680)
at org.glassfish.jersey.server.ResourceConfig.packages(ResourceConfig.java:660)

@driverpt
Copy link

driverpt commented Oct 21, 2016

I believe it's not working. I can put up my Test Project on GitHub if you want

@andjarnic
Copy link

So.. after reading tons of posts..etc.. is this resolved? Is it merged? Does it work? I am a wee baffled why almost two years later this is still not in Jersey (or is it?). I honestly dont know what the primary issues is other than "jersey doesnt work in fat jars". I am using Spring boot in my IDE and it seems fine. Is it when I build my spring boot app in to app.jar and deploy it as a fat jar that I am going to run in to trouble?? Is there any possible way to please expedite this. Jersey is a core piece to spring boot micro services for MANY many projects. This should be the hottest priority right now if this is still an issue.

@pavelbucek
Copy link
Member

this change is not merged, since it failed verification. Note this is community contribution, not Jersey team initiative, so we do rely on the activity of the community.

Jersey can work in fat jars without any issues. We don't have any test in regards of integration with Spring Boot - we cannot test every framework which chooses to integrate Jersey, so I'm not sure whether presented issue is valid or not.

@divStar
Copy link

divStar commented Nov 20, 2016

Sadly I do not have a solution yet, but if jersey is supposed to work in fat jars, I believe there shouldn't be a big issue resolving this? I mean.. "user-service-0.0.1-SNAPSHOT.jar!\BOOT-INF\classes" states, that it is not able to find any classes in that subfolder of the jar - even though that jar very much does contain .class files in there.
I can imagine there might be problems with the BOOT-INF\lib folder as jersey might not be able to scan all the jar files inside the main jar file, but classes?

Sadly I have not worked with jersey enough to understand it to an extent where I could help out, but perhaps one could somehow use the same technique one would use if jersey without spring was being used?

@driverpt
Copy link

driverpt commented Nov 21, 2016

I think that this could be fixed by being able to specify the Root Path inside the Jar to where the Classes are. In this case, according to Spring Boot Documentation, it's /BOOT-INF/lib and /BOOT-INF/classes.

UPDATE: i think we need to use java.util.JarFile and JarEntry from the root to find the resources.

UPDATE 2: After some investigation, i don't think that the current ClassPath Scanning Implementation supports Jar Architectures outside the standards.

UPDATE 3: The only way to perform a ClassPath Scanning using the JarEntry, is convert the /BOOT-INF (Spring Boot Example) into a separate Jar Temp File and load it from there.

@driverpt
Copy link

Created #250 so that we can create a Custom Resource Finder for Spring Boot that works around this issue.

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

Successfully merging this pull request may close these issues.