Bug 200691

Created attachment 109810 [details]
proposed patch

proposed patch

TZ01: The new friend API does similar thing as the Category.setStoreListener(). What is the relation to it?
TZ02: Should not the save() method have the Project being saved as a parameter.

PS01 (Re TZ01): Category.setStoreListener() is successfully used in WebStart module, but this is only because all WebStart specific properties are modifyable from a single panel only. Thus either the user does not access it and nothing needs to be saved, or she accesses it and the panel gets created and the listener registered and everything works OK.
With JFX the situation is different, it is not possible to rely on a listener in any single category panel. JFX specific properties can be modified from several new category panels + modified JSE panels. Listeners in each of the new panels would cause the properties store() to be called redundantly, while in case of modified JSE panels the store listener is already occupied by JSE project.

With a project extension like JFX, where extension properties are spread across various category panels, it seems better to centralize the reaction to the save action (triggered by OK button) in one place.

PS02 (Re TZ02): Project indeed is needed to be passed to provider implementation. As the proposal above relies on @ProjectServiceProvider, the Project reference can be obtained through constructor parameter. On the other hand it is true that including Project in the save call makes things look clearer and avoids the need to store Project variable inside the provider. So I do not object to define:

public interface J2SECustomPropertySaver {
    void save(Project p);
}

OK, thanks.

Created attachment 109885 [details]
updated patch proposal

added Project parameter to save()

Y01 Re. "storeListener called redundantly" - can't you tighten the API specification to say that each instance of the listener will be called just once?

Y02 Try to write a test to permanently specify the new behavior.

PS03 (Re Y01) "storeListener called redundantly" - it is not the listeners that would be called redundantly. The API defining Category.setStoreListener() has itself a well defined meaning. I mentioned it only because it has been utilized in WebStart for the purpose of storing WebStart specific properties and as such it was worth investigating whether the same can be done for JavaFX2. The problem is that JavaFX2 specific properties are spread across several Categories. Clearly each affected Category would need a StoreListener() registered. The seemingly viable approach is then to have a store() method (in the class that maintains JFX specific properties), that would be called from each such StoreListener(). Such store() method is not part of any API and could be implemented to detect redundant calls of itself, but this still would not overcome the other problem - the problem with extended JSEProject categories. The property panel extension mechanism (#200704, being extension of previous SPI defined by J2SERunConfigProvider) makes it possible to add properties to JSEProject panels - but there seems to be no clear way of registering an additional StoreListener() for just the extended properties while not affecting the JSEProject panel code. Any possible workaround needed to achieve this together with the necessity to register listeners for several Categories seems much less straightforward than registering one central service provider as suggested here.

Created attachment 110009 [details]
test

Re Y02

If no other objections appear I plan to close this issue on We 17 Aug morning.

pushed to jet-main

http://hg.netbeans.org/jet-main/rev/b4d787206ac8

http://hg.netbeans.org/jet-main/rev/d007fbf157aa

Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/b4d787206ac8
User: Petr Somol <psomol@netbeans.org>
Log: #200691 SPI to enable J2SE Project extension properties to be stored properly