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
JSP To Load Versions Dynamically #13173
Conversation
#build |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_tnO3wM7eEeqmIcsdsdUKbw Target locations of links might be accessible only to IBM employees. |
Your Open Liberty build results are ready for viewing.
|
The build volosied-13173-20200726-0233 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_tnO3wM7eEeqmIcsdsdUKbw |
dev/com.ibm.ws.jsp/src/com/ibm/ws/jsp/webcontainerext/JSPExtensionFactory.java
Outdated
Show resolved
Hide resolved
0acd226
to
497da86
Compare
#build |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_NBN1sCLOEeux8uC7tiSOag Target locations of links might be accessible only to IBM employees. |
Your Open Liberty build results are ready for viewing.
|
The build volosied-13173-20201109-2204 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_NBN1sCLOEeux8uC7tiSOag |
dev/com.ibm.ws.jsp/resources/com/ibm/ws/jsp/resources/messages.nlsprops
Outdated
Show resolved
Hide resolved
#build |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_xqK5wiT2Eeux8uC7tiSOag Target locations of links might be accessible only to IBM employees. |
Your Open Liberty build results are ready for viewing.
|
The build volosied-13173-20201112-1600 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_xqK5wiT2Eeux8uC7tiSOag |
dev/com.ibm.ws.jsp.2.3_fat/fat/src/com/ibm/ws/jsp23/fat/tests/JSP23JSP22ServerTest.java
Outdated
Show resolved
Hide resolved
dev/com.ibm.ws.jsp/src/com/ibm/ws/jsp/webcontainerext/AbstractJSPExtensionProcessor.java
Show resolved
Hide resolved
dev/com.ibm.ws.jsp/src/com/ibm/ws/jsp/webcontainerext/JSPExtensionFactory.java
Show resolved
Hide resolved
dev/com.ibm.ws.jsp/src/com/ibm/ws/jsp/webcontainerext/JSPExtensionProcessor.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we have a PagesVersion class now but we're referring to it in the code in areas such as loadedJSPVersion should we change to loadedPagesVersion?
69726f3
to
ff7098b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for making the updates!
ff7098b
to
cb6ed10
Compare
#build |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_YicVUG1NEeulAs018fCg4w Target locations of links might be accessible only to IBM employees. |
Your Open Liberty build results are ready for viewing.
|
The build volosied-13173-20210212-0921 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_YicVUG1NEeulAs018fCg4w |
All tests passed except for one failure: org.eclipse.microprofile.rest.client.tck.timeout.TimeoutTest.testConnectTimeout. However, it's an existing defect: 272083. |
#run-libby-bot |
Code analysis and actionsDO NOT DELETE THIS COMMENT.
|
Potential Fix For #12387 that models the ServletVersion/JavaEEVersion.
I had modeled the Service-Component: com.ibm.ws.jsp.vXX.dd changes after our servlet factory code.
When a feature is loaded, a new version of JSP is set through injection within the JSPExtensionFactory.
The other two approaches didn't seem appealing.
Splitting the the 2.3 factories just meant more duplicate code for 2.3 and 3.0, and we only need a new version for 3.0.
Transformer could work, but it would be more of a hack. For example, we could change the version variable to "JSP_VERSION_2.3", transform to "JSP_VERSION_3.0", and then the getVersion functions would return the substring 2.3/3.0 instead.
OSGI approach seems most fitting.
I copied and applied code from the web-container methods below:
open-liberty/dev/com.ibm.ws.webcontainer/src/com/ibm/ws/webcontainer/osgi/WebContainer.java
Lines 1568 to 1579 in 36aa803
The new getLoadedPagesSpecLevel uses timing code (from webcontainer) that was merged in this PR commit
open-liberty/dev/com.ibm.ws.webcontainer/src/com/ibm/ws/webcontainer/osgi/WebContainer.java
Lines 1591 to 1606 in 36aa803
New Error message was added:
jsp.feature.not.loaded.correctly=CWWJS0001: The JSP [Pages] feature did not load with an acceptable version.
Update: Feb 8th 2021
Code was updated to pass the version variable to AbstractJSPExtensionServletWrapper per the conversation with Tom Watson in the comments below.
JSPExtensionFactory#createProcessor passes the version to AbstractJSPExtensionProcessor. It’s then set as a variable in AbstractJSPExtensionProcessor, and later passed through AbstractJSPExtensionServletWrapper#initialize. Then the version is stored as loadedJSPVersion in AbstractJSPExtensionServletWrapper.
Sounds complicated as I reread it, but it does get to AbstractJSPExtensionServletWrapper.