Bugzilla – Full Text Bug Listing
| Summary: | OSGi Web Container RI: WAR extraction when installing a web application | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] Gemini.Web | Reporter: | Violeta Georgieva <milesg78> | ||||||||||||||||
| Component: | unknown | Assignee: | Glyn Normington <glyn.normington> | ||||||||||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||
| Priority: | P3 | CC: | glyn.normington, zteve.powell | ||||||||||||||||
| Version: | unspecified | ||||||||||||||||||
| Target Milestone: | 1.1.0.RC1-incubation | ||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||
| OS: | Windows XP | ||||||||||||||||||
| Whiteboard: | |||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
Created attachment 163269 [details]
test web application
The .cp directory is an Equinox implementation detail. It should be possible to explode the WAR into a directory before installing in order to satisfy this requirement. Note that Felix has apparently only added support for exploded bundle installation recently and if this is required, a suitable version of Felix should be used. (In reply to comment #2) > The .cp directory is an Equinox implementation detail. > It should be possible to explode the WAR into a directory before installing in > order to satisfy this requirement. > Note that Felix has apparently only added support for exploded bundle > installation recently and if this is required, a suitable version of Felix > should be used. Hi Glyn, I tried to install an extracted directory as you proposed. (install webbundle:file:<path-to-directory>) The problem is that the Manifest is not transformed for the regular web applications. I can see that in the WebBundleUrlStreamHandlerService you are using JarTransformer. I think that this is the cause for not transforming the Manifest when installing a directory. What do you think? Regards Violeta I haven't been in to the web container since it passed the TCK, but your observation seems spot on and my suggested workaround won't help. Hi, I made a DirTransformer based on the JarTransformer that you are using. There is one specific that I want to discuss with you. The DirTransformer makes the transformation directly in the source URL (i.e. source folder). This approach has some. Pros: - Fast deployment because all file copy operations are skipped - Deployment with reference is possible - Changes of the source folder are immediate visible, in some cases with zero downtime, depending on what was changed in the web application Cons: - There is no guarantee that server has write permissions for the source folder - It is not so good to change the source What do you think about this approach? Thanks Violeta Hi Violeta (In reply to comment #5) > Hi, > > I made a DirTransformer based on the JarTransformer that you are using. There > is one specific that I want to discuss with you. > > The DirTransformer makes the transformation directly in the source URL (i.e. > source folder). This approach has some. > > Pros: > - Fast deployment because all file copy operations are skipped > - Deployment with reference is possible > - Changes of the source folder are immediate visible, in some cases with zero > downtime, depending on what was changed in the web application > > Cons: > - There is no guarantee that server has write permissions for the source folder > - It is not so good to change the source > > What do you think about this approach? Changing the source is a no-no: * it will surprise users as we don't modify the source in any other cases * it may be impossible to recover from failures where the source is partially modified * it's complicated if you upgrade the server after the source has been modified - we'd have to make the modifications idempotent *across* releases of the server * it could cause interference problems if the source is shared between multiple server instances * the server may not have write permission, as you observe We should find a way of changing the copy, even though this is slower. > > Thanks > Violeta Regards, Glyn Created attachment 174009 [details]
Proposed patch
Hi Glyn,
I followed your recommendation for directory copying and not changing the original directory.
Now the installation of an exploded web application is working.
I want to propose a patch about this functionality if you are interested in.
The implementation is based on the JarTransformer that you are using.
I added two new classes (DirTransformer and DirTransformingURLConnection) in the package org.eclipse.gemini.web.internal.url, but if you think it is more appropriate to place them in org.eclipse.virgo.util.io, I'm OK.
Also the implementation uses a temporary directory for the transformations. If you think it is more appropriate another work directory, just tell me.
I will look forward to your comments.
Regards
Violeta
Hi Violeta (In reply to comment #7) > I will look forward to your comments. Sorry it is taking a while to get around to this one. I won't be able to look at it until later next week at the earliest, but the approach sounds promising. Meanwhile I would be grateful if you could do a couple of things. Firstly, please could you rework the patch so that I can apply it with the patch utility. You can do this by using git diff from the root directory of Gemini Web and piping the result into a patch file which you can then attach as a text file attachment (rather than a RAR). This produces a Unix patch file which I can then apply automatically to my directory. Secondly, please could you confirm that you wrote 100% of the patch, that you have the right to contribute it to Eclipse, and that any new files contain the appropriate License header. Thanks, Glyn (In reply to comment #8) Hi Glyn, Sure I will rework it. Also I want to introduce PathReference where it is appropriate. I値l try to use the proposal with git diff. > Secondly, please could you confirm that you wrote 100% of the patch, that you > have the right to contribute it to Eclipse, and that any new files contain the > appropriate License header. I confirm that I wrote 100% of the patch and I have the right to contribute it to Eclipse. I will introduce the appropriate License header to the new classes. Regards Violeta Adding Steve to the cc list for coverage during the holiday season. Created attachment 174863 [details]
Proposed patch
Hi Glyn,
I'm attaching the new patch with the changes mentioned in the previous comments.
I used the git diff for the patch. I hope that I created it correctly.
Regards
Violeta
(In reply to comment #11) Hi Violeta Please could you confirm that: you wrote 100% of the code, you have the right to contribute the code to Eclipse, and new Java file headers contain the appropriate License header. Please note that the legal process [1] requires a CQ to be raised for this contribution since it is over 250 LOC. Before I raise the CQ, please would you fix a thread safety bug (I noticed in a quick review of the code) in DirTransformingURLConnection: transformedURL is accessed without synchronisation. See [2] for concurrent programming guidelines which apply to Gemini Web as Virgo's sister project. It is probably simplest to introduce an internal monitor object and synchronise on that rather than bothering to document and worry about lack of thread safety. Note we can tweak the code after it has been through the CQ process, although any tweak larger than 250 LOC would need its own CQ. To keep this clear, I'll probably commit the updated patch to a branch and then any further changes can be made via separate patches. Regards, Glyn [1] http://www.eclipse.org/legal/EclipseLegalProcessPoster.pdf [2] http://wiki.eclipse.org/Virgo/Committers#Concurrent_Programming_Guidelines (In reply to comment #12) > (In reply to comment #11) > > Please could you confirm that: you wrote 100% of the code, you have the right > to contribute the code to Eclipse, and new Java file headers contain the > appropriate License header. Steve spotted you had already confirmed. Sorry for any confusion. Please would you simply fix the thread safety bug. Thanks, Glyn Created attachment 174968 [details]
Proposed patch
Hi,
Here is the new patch with fixed thread safety bug.
Regards
Violeta
(In reply to comment #14) Hi Violeta That's great - thanks for fixing that. I got a clean build so I'll go ahead and create a CQ. The most likely tweaks are to reduce the visibility of certain classes and methods (see [1]), although I haven't read the code in detail to be sure that is necessary. If you want to look at that in the next few days, please feel free, but in that case please attach a git diff of just your tweaks. Regards, Glyn [1] http://wiki.eclipse.org/Virgo/Committers#Code_Visibility (In reply to comment #15) CQ 4370 raised: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=4370 (In reply to comment #16) > (In reply to comment #15) > > CQ 4370 raised: > > https://dev.eclipse.org/ipzilla/show_bug.cgi?id=4370 CQ 4370 has been approved. The patch in attachment 174968 [details] is committed on branch bug307393. Thanks again for this contribution. Please would you tidy up the code visibility in a separate patch based on that branch. Created attachment 178388 [details]
Code visibility changed
Hi Glyn,
Here is the patch with changes in regard to the code visibility.
Regards
Violeta
Thanks - that looks great. The next step is for me to add this patch to the branch and merge the branch into master. Patch applied to branch and branch pushed to origin. I will test and merge this in due course. (Can't test now as a Virgo ripple is in progress and ports could clash.) Tested and merged into master. Hi Glyn, I added additional test case to the integration test in order to fully verify that the use case is working correctly. Proposed patch is attached. Regards Violeta Created attachment 178591 [details]
Additional integration test
Created attachment 178592 [details]
Additional integration test
Tested, committed, pushed. Thanks again. RC1 is planned instead of milestone 5. |
Build Identifier: Hi, I'm using OSGi Web Container RI. In my application I want to use the getRealPath(String) method from the Servlet API. Unfortunately when the application is not extracted on the file system, but loaded from the archive, this method returns NULL, this behavior is specified in the Servlet Specification and is correct, but most of the forums and wiki applications use actively the file system and the method specified above and cannot be rewritten in a way to escape the methods that depends on the file system. I can see in the OSGi repository that only the jar files from the WEB-INF/lib are extracted in ".cp" directory, but the other files are not extracted. In my application I have index.jsp that contains: config.getServletContext().getResourcePaths(/): <%=config.getServletContext().getResourcePaths("/")%> config.getServletContext().getRealPath(/): <%=config.getServletContext().getRealPath("/")%> If I request it: http://localhost:8080/app/index.jsp I'm receiving the following: config.getServletContext().getResourcePaths(/): [/index.jsp, /WEB-INF/, /META-INF/] config.getServletContext().getRealPath(/): null Is it possible to provide a file system extraction for the application in OSGi Web Container RI? Reproducible: Always Steps to Reproduce: 1.Deploy the attached application 2.Request http://localhost:8080/app/index.jsp 3.You will see that the result of getRealPath() is NULL