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.
many of the plugins need/want to register a default instance. Most all of them have had to write code at the FileObject level into the server registry, since the registration of a default instance needs to take place before the registry is fully initialized. This change would create a clear place to code the default instance registration... that will be triggered AFTER the server registry is initialized.
Note this change would also allow plugin developers to stay inside the registry apis, instead of being forced to code below the registry abstraction at the FileObject level.
issue 138554 could be resolved by implementing a hack similar to those used in the Tomcat, JBoss and GlassFish plugins... so this isn't a strict "blocker"... but it would be better to implement this API and avoid implementing more of the same code that breaks the ServerRegistry abstraction. Once the API is in place, the hacks in the existing code could be re-evaluated by the module owners.
Created attachment 65441 [details] diffs to j2eeserver. includes usage of the API
Broken encapsulation examples http://hg.netbeans.org/main/file/tip/tomcat5/src/org/netbeans/modules/tomcat5/TomcatFactory.java : method registerServerInstanceFO http://hg.netbeans.org/main/file/tip/j2ee.jboss4/src/org/netbeans/modules/j2ee/jboss4/JBDeploymentFactory.java : method registerDefaultServerInstance
PH01: I have no idea how this API change would be used in your current code. I tried to design an alternative approach without callback (register method callable anytime), but it was impossible for me to incorporate it to the GFv3 workflow. It is even not clear how SPI change designed in this isuue would be used. Please post the patch that will include the real usage - such as GFv3 registration. Maybe I'm missing something but I'd really like to know how this will work with GFv3. PH02: The unit test is needed for any change to ServerRegistry (not actually needed right now, until PH01 is solved). PH03: Wrong module version.
http://www.netbeans.org/issues/show_bug.cgi?id=138554#desc7 contains additional information that supports the need for an API.
I'm not arguing about a need for API. It is just not clear to me how this SPI will solve the problem (I'm not saying it won't solve it). Any patch introducing the API/SPI should contain its usage. Nothing more. Using the API is also the way how to test the design of the API.
Created attachment 65607 [details] a second round of diffs to address some of phejl's comments
Most of the comments that follow are based on the context of the diffs in forReview2.txt... RE PH01: The additional spi that I am proposing is in OptionalDeploymentManagerFactory.java The additional method is boolean registerDefaultInstance(). This method returns true, if an instance is registered and false if an instance does not need to be registered. It throws an InstanceCreationException if there is some failure while attempting to register the instance. This is different from my original definition of the SPI. I figured folks would like to be able to detect problems on the "client side" of this SPI and "react" accordingly. An example of the usage of the spi is in ServerRegistry.java. This usage is clearly visible in the diff. An example implementation of the spi is in Hk2OptionalFactory.java. This implementation is clearly visible in the diff. RE PH02: I have extended the ServerRegistryTest.java to exercise the additional SPI. RE PH03: I have updated the module version... though I will obviously need to double check this BEFORE the API change is pushed to main.
PH01: I believe we should make the introduced SPI method more generic. It is also called after known instances are loaded - initialization code can manipulate them (check existence, delete etc.). Maybe it would be beneficial to let the method throw a checked exception. I didn't find usecase for return value. PH02: Unfortunately introduced test did not really test the registry but the mock test plugin. Fixed. Attaching the proposed patch.
Created attachment 65665 [details] serverplugin initialization patch
Created attachment 65669 [details] forgot to add one test file - fixed
in response to points raised in http://www.netbeans.org/issues/show_bug.cgi?id=141427#desc11 PH01: more generic -- great idea. push to after instance initialization -- seems ok... may not address issue 138554. I will double check. checked exception -- I could go either way... your version of the SPI doesn't use a checked exception. since the code that is going to catch the exception is in j2eeserver, I will defer to you.... PH02: yup. the test was weak... yours is better. Thanks for such a thorough review.
VK01: I did think of one thing that I would like to change... the name of the method added in ODMF... The name initializePlugin would lead people to think that the method is called very early in the lifecycle of the plugin. And it isn't. It is actually called pretty late in the lifecycle of the plugin. It may be as cearer to name the method finishInitialization()...
Re VK01: Method renamed to finishServerInitialization. Added checked exception to provide a clue to an implementor what to do under unexpected conditions. I suppose this could be the final patch.
Created attachment 65743 [details] updated patch
Yes. I think this can be the final patch.... Will you be doing the commit/push or do you want me to do it? If you do it... please do not check in the code changes that are in the glassfish.common module. I will commit those, since they are actually for a bug fix and not the API change.
As I'm on vacation next week, it would be great if you could commit/push this. Thanks.
will do. have a nice vacation.
http://hg.netbeans.org/main/rev/01260e21347f
Integrated into 'main-golden', available in build *200807311401* on http://bits.netbeans.org/dev/nightly/ Changeset: http://hg.netbeans.org/main/rev/01260e21347f User: vkraemer@netbeans.org Log: #141427 : api change to allow plugins to finish initialization.