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.

Sign up for GitHub

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

Jump to bottom

Improve ResourceFactory and Resource list handling #5133

Closed
joakime opened this issue Aug 10, 2020 · 3 comments · Fixed by #5142
Closed

Improve ResourceFactory and Resource list handling #5133

joakime opened this issue Aug 10, 2020 · 3 comments · Fixed by #5142
Assignees
@joakime

Comments

@joakime
Copy link
Contributor

joakime commented Aug 10, 2020

Jetty version
10.0.x

Description
While reviewing PR #5131 it was discovered that the WebAppClassLoader constructor automatically added the WebAppContext.getExtraClassPath() entries on it's own.
While at the same time WebInfConfiguration was processing the WebAppContext.getExtraClassPath() in it's own Servlet specific ways.

https://github.com/eclipse/jetty.project/blob/e117fbe8281bd43ec5601a30d39f50a09cbcef22/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java#L195-L196

This behavior in the WebAppClassLoader constructor should be removed.
@joakime
Copy link
Contributor Author

joakime commented Aug 10, 2020

@janbartel if I remove these 2 lines from the jetty-9.4.x branch then the WebAppClassLoader is not updated from the extraClasspath.

Should WebInfConfiguration set the WebAppClassLoader? or is that handled in a later Configuration from the MetaData?

@janbartel
Copy link
Contributor

@joakime I think that WebInfConfiguration should do the parse of the extraClassPath and then poke the results into the WebApPClassLoader, because it is already poking the WEB-INF/classes and WEB-INF/lib jars into it.

WebInfConfiguration should not create the WebAppClassLoader, that is handled by the WebAppContext in it's startup sequence, and allows for the user to supply their own.

@joakime joakime changed the title WebAppClassLoader should not add context.extraClassPath on its own, let WebInfConfiguration do it WebAppContext.extraClassPath should be managed by MetaInfConfiguration, not WebAppClassLoader Aug 12, 2020
joakime added a commit that referenced this issue Aug 12, 2020
@joakime
Issue #5133 - Reworking WebAppContext.extraClasspath
0b0d7d3 
+ Now parsed by WebAppContext into List<Resource>
+ Reintroduced Resource.fromList
+ Refactored ResourceFactory to never return null
  and always throw an exception if unable to
  get/create/resolve the Resource

Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Aug 12, 2020
@joakime
Issue #5133 - Reworking WebAppContext.extraClasspath
6848403 
+ Reverting name ResourceFactory.newResource(String)
  to .getResource(String)
+ Reintroducing Resource.getResource(String)
+ ResourceHandler.getResource(String) cleaned up
  in light of Exception handling requirement
+ Resource.addPath(String) implementations can
  never return null now

Signed-off-by: Joakim Erdfelt <[email protected]>
@joakime joakime linked a pull request Aug 12, 2020 that will close this issue
Issue #5133 - Improve ResourceFactory and Resource list handling #5142
Merged
@joakime
Copy link
Contributor Author

joakime commented Aug 12, 2020

Opened PR #5142

joakime added a commit that referenced this issue Aug 20, 2020
@joakime
Issue #5133 - Fixing ResourceCollection(String) when no entries provi…
b35c433 
…ded.

Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Aug 26, 2020
@joakime
Issue #5133 - Updates based on review
ccc8637 
Signed-off-by: Joakim Erdfelt <[email protected]>
joakime added a commit that referenced this issue Sep 21, 2020
@joakime
Issue #5133 - Changes from Review with jan & greg
f6bcbda 
Signed-off-by: Joakim Erdfelt <[email protected]>
@gregw gregw changed the title WebAppContext.extraClassPath should be managed by MetaInfConfiguration, not WebAppClassLoader Improve ResourceFactory and Resource list handling Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@joakime joakime

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue #5133 - Improve ResourceFactory and Resource list handling jetty/jetty.project
2 participants
@joakime @janbartel