This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | standalone EJB (or AppClient) module cannot be deployed | ||
---|---|---|---|
Product: | javaee | Reporter: | David Konecny <dkonecny> |
Component: | EJB Project | Assignee: | David Konecny <dkonecny> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | AlainC, kganfield, phejl, pjiricka, thufir, vkraemer |
Priority: | P2 | Keywords: | API_REVIEW_FAST |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 163083 | ||
Attachments: |
patch
patch |
Description
David Konecny
2010-05-18 23:42:17 UTC
This should be fixed after NB 69 and most of work is likely in Server Plugings API/implementation. I'm requesting a waiver. We've always had this problem so it is not a blocker for 69. Created attachment 101073 [details]
patch
Vince, Petr, could you review this patch please and tell me what you think? It is first sketch. It fixes deployment of standalone EJB module by adding J2eeModuleImplementation3.getRequiredLibraries API method which is later called from GlassFish (in FastDeploy class) and passes these libraries to deployment command via "libraries" parameter.
Some questions:
* is it how you would fix this problem?
* I cannot figure out how to pass extra parameters to Hk2DeploymentManager.redeploy(). What am I missing?
(In reply to comment #3) > Created an attachment (id=101073) [details] > patch > > Vince, Petr, could you review this patch please and tell me what you think? It > is first sketch. It fixes deployment of standalone EJB module by adding > J2eeModuleImplementation3.getRequiredLibraries API method which is later called > from GlassFish (in FastDeploy class) and passes these libraries to deployment > command via "libraries" parameter. Looks good. > > Some questions: > * is it how you would fix this problem? I was thinking about (re)using ServerLibraryDependency with added factory method like forFile() that would represent such files - special case of library. I was not sure about the workflow (as we can't change DeploymentManager interface). Maybe you could consider ServerLibraryDependency instead of file (could be useful in future with version related stuff). Not sure just brainstorming... > * I cannot figure out how to pass extra parameters to > Hk2DeploymentManager.redeploy(). What am I missing? (In reply to comment #4) > Maybe you could consider ServerLibraryDependency instead of file (could be > useful in future with version related stuff). Not sure just brainstorming... I've tried to use ServerLibraryDependency but I'm not sure it is semantically correct. If J2eeModule.getRequiredLibraries returns list of ServerLibraryDependency (that is deployment of this standalone EE module depends on list of jar files) then calling ServerLibraryManager.deployLibraries() sounds like logical step to do but how would that be implemented in GlassFish for example? On GlassFish these libraries are deployed as part of EE module deployment and no as separate library deployment. I would be happy to use ServerLibraryDependency if you still think it make sense. Created attachment 101141 [details]
patch
New patch which solves remaining problems. I had to define new SPI which extends javax.enterprise.deploy.spi.DeploymentManager. I implemented all these new interfaces for GlassFish v3 plugin and the dpeloyment work (there are problems with AppClient execution but I will file that as separate issue). Please comment if you have an objection.
Vince, I did change several classes in your GF code so please tell me if you are OK with them or would rather want to implement it yourself differently. I needed to test my changes and I so I updated GF code. I am a bit confused by the use of Xyz2 and Abc3 classes... will the interfaces/impls get folded into Xyz and Abc at some point? (In reply to comment #8) > I am a bit confused by the use of Xyz2 and Abc3 classes... will the > interfaces/impls get folded into Xyz and Abc at some point? I do not think so. Generally speaking if you want your SPI interfaces to be backward compatible then Xyz2, Xyz3 is the way to go. If you know that your SPI is Friend API contract and you know all implementations and are willing to update them then you could introduce "incompatible change". To me number at the end is like a version of backward compatible interface and different clients are implementing different versions. Alternative could be for example to define instead of GlassfishModule2 something like GlassfishModuleStandaloneEEModuleDeploymentCapability which may or may not extends GlassfishModule. your code, your call... it is just that it is not a coding pattern that I have seen used often in the NB codebase. api reviewers, could you please review attached API change. Namely addition of org.netbeans.modules.j2ee.deployment.plugins.spi.DeploymentManager2, org.netbeans.modules.j2ee.deployment.devmodules.spi.J2eeModuleImplementation3 and org.netbeans.modules.glassfish.spi.GlassfishModule2. The patch enhances API/SPI of server plugins module to deploy standalone EE module and implements the new SPI by one of the plugins (GlassFish). Any objections/alternatives to appending version number to the end of SPI interface? Thanks. (In reply to comment #5) > (In reply to comment #4) Finally I get bunch of comments from issuezilla. I haven't investigated in more details as it is midnight :), anyway read bellow how it could work. > > Maybe you could consider ServerLibraryDependency instead of file (could be > > useful in future with version related stuff). Not sure just brainstorming... > > I've tried to use ServerLibraryDependency but I'm not sure it is semantically > correct. If J2eeModule.getRequiredLibraries returns list of > ServerLibraryDependency (that is deployment of this standalone EE module > depends on list of jar files) then calling > ServerLibraryManager.deployLibraries() sounds like logical step to do but how > would that be implemented in GlassFish for example? I was still trying to avoid DeploymentManager being involved - in long term this interface should be replaced. Anyway if we decide to enhance DeploymentManager via new DeploymentManager2 then instead of J2eeModule we can pass Set<ServerLibraryDependency> and plugin can decide when and how it will deploy the libs. For example it can deploy real *server* placed libs in current workflow while it can decide to deploy *file* only deps via on call to methods in DeploymentManager2. Of course such approach would require ServerLibraryDependecy.forFile() static constructor and @CheckForNull File getFile() getter. What do you think? > On GlassFish these > libraries are deployed as part of EE module deployment and no as separate > library deployment. I would be happy to use ServerLibraryDependency if you > still think it make sense. (In reply to comment #12) > I was still trying to avoid DeploymentManager being involved - in long term > this interface should be replaced. I agree that it should be replaced, but it may take a while for it to happen and in the meantime it blocks us. That's why I decided to just enhance it. > Anyway if we decide to enhance > DeploymentManager via new DeploymentManager2 then instead of J2eeModule we can > pass Set<ServerLibraryDependency> and plugin can decide when and how it will > deploy the libs. For example it can deploy real *server* placed libs in current > workflow while it can decide to deploy *file* only deps via on call to methods > in DeploymentManager2. Of course such approach would require > ServerLibraryDependecy.forFile() static constructor and @CheckForNull File > getFile() getter. What do you think? can you respond please to my comment #5 in this issue. I do not see how ServerLibraryManager.deployLibraries() would fit into this? Would we still need it or would it be only used for server libraries or would it be deprecated in favor of DeploymentManager2? ServerLibraryDependency would work for what I need. (In reply to comment #13) > (In reply to comment #12) > > I was still trying to avoid DeploymentManager being involved - in long term > > this interface should be replaced. > > I agree that it should be replaced, but it may take a while for it to happen > and in the meantime it blocks us. That's why I decided to just enhance it. I'm ok with that. > > > Anyway if we decide to enhance > > DeploymentManager via new DeploymentManager2 then instead of J2eeModule we can > > pass Set<ServerLibraryDependency> and plugin can decide when and how it will > > deploy the libs. For example it can deploy real *server* placed libs in current > > workflow while it can decide to deploy *file* only deps via on call to methods > > in DeploymentManager2. Of course such approach would require > > ServerLibraryDependecy.forFile() static constructor and @CheckForNull File > > getFile() getter. What do you think? > > can you respond please to my comment #5 in this issue. I do not see how > ServerLibraryManager.deployLibraries() would fit into this? Would we still need > it or would it be only used for server libraries or would it be deprecated in > favor of DeploymentManager2? ServerLibraryDependency would work for what I > need. It would be used for server libraries as it is now. In meantime I figured out that would not be that easy as we would need some getRequiredLibraries() method anyway. So let's forget it :) However I also realized I don't like an extension of J2eeModule API because nobody but plugin and infrustructure is interested in it. Perhaps it was the way how to propagate the required libraries through IncrementalDeployment spi. I would rather see getRequiredLibraries() somewhere in J2eeModuleProvider and IncrementalDeployment enhanced in similar way as DeploymentManager. I would also suggest to have a new final class DeploymentContext that would infrastructure create and pass to DeploymentManager2 or IncrementalDeployment2. Such class would have methods getModuleArchive(), getDeploymentPlan(), getRequiredLibraries(). In future we could add compatibly add some other information required by the server. This is not really related to the change, just an optional enhancement. I'll try to play with it and in case of success to provide a patch. A patch is worth a thousand words ;) Thanks for the review. I exchanged some ideas with Petr offline and as we are both in agreement I'm going to commit the change today. Fixed. fcc0b973be57 There is one remaining issue with AppClient which I filed and will investigate separately - bug 189227. Fixed also for Maven EJB Project: 15a2def6537d *** Bug 188744 has been marked as a duplicate of this bug. *** for someone learning EJB's this isn't a small problem, it's a huge stumbling block. thufir - please file a new issue with details how to reproduce the problem and what does not work. This issue should stay as is - it was resolved and closed several years ago. Thanks. |