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.

Bug 85405

Summary: The registered CompositeCategoryProvider is not notified when the project properties is closed
Product: projects Reporter: Tomas Zezula <tzezula>
Component: Generic Projects UIAssignee: Milos Kleint <mkleint>
Status: RESOLVED FIXED    
Severity: blocker CC: jfdenise
Priority: P3 Keywords: API, API_REVIEW_FAST
Version: 6.x   
Hardware: All   
OS: All   
Issue Type: DEFECT Exception Reporter:
Bug Depends on: 201282    
Bug Blocks:    
Attachments: spi in ant project type base module introducing a baseclass for properties modification in customizers..
changed contract for project customizer lookup and implementation thereof
new, simplified proposal

Description Tomas Zezula 2006-09-21 07:19:58 UTC
For a stand alone module which does not depend on the (j2se) project type module
by impl dependency it is impossible to find out that the project properties were
closed by OK button and it should store the new data. Inside the (j2se)project
module it works since the J2SEProjectProperties can be used.
Comment 1 Milos Kleint 2006-09-21 08:30:45 UTC
tomasi, I don't think this ought to be fixed on the project api level. We do not
want every panel to read and store the files on disk. 
I'd rather have this fixed on a project type by project type basis with the
model passed in, with friend contract possibly.

I'll look into this though and see if we can provide some generic helper class.
Comment 2 Tomas Zezula 2006-09-21 08:50:33 UTC
Friend dependency is not very nice and it implies tight coupling of the
CompositeCategoryProvider and J2SEProjectProperties. It would be nice to define
an interface which the J2SEProjectProperties, WebProjectProperties and others
implement. When the CompositeCategoryProvider extends more project types it
would need to depend on more project type modules and behave in different way
for each *ProjectProperties since they have no common interface. The interface
can have just putAdditionalProperty method.
Comment 3 Milos Kleint 2006-09-21 09:08:52 UTC
that's most probably not enough for a generic contract. Someone might want to
write to a completely different property file, build script etc.

on the projectuiapi level we probably want just an OK Button listener equivalent
for the panel to register.
on ant projects level we could introduce a supertype for the various properties
as you suggest (I haven't investigated if it really applies to all, maybe
freeforms are special)

Comment 4 Jean-francois Denise 2006-09-21 09:23:50 UTC
In our specific case, extensions of J2SE project properties, having an OK
listener would be enough.
The problem of writing in nbproject.properties is an interesting one. We are
adding entries in this file in a "custom way". I looked at the J2SE project code
and I did the same ==> replication.
Having an API to Get/Put all properties comming from nbproject.properties at
once would be very cool!
Comment 5 Milos Kleint 2006-10-03 09:51:43 UTC
Created attachment 34791 [details]
spi in ant project type base module introducing a baseclass for properties modification in customizers..
Comment 6 Milos Kleint 2006-10-03 09:52:43 UTC
Created attachment 34792 [details]
changed contract for project customizer lookup and implementation thereof
Comment 7 Milos Kleint 2006-10-03 10:01:26 UTC
I've examined the current implementations of
J2seprojectproperties/webprojectproperties/ejbprojectproperties classes that are
used in current customizers
Introduced a base class that contains the common ground for all of these and put
it into the org.netbeans.spi.project.support.ant.ui package. It encapsulates the
creation/access to and storing of StoreGroups and allows to set additional
properties (as is used by current webservices panels) next to StoreGroup based
models. Additionally it allows to attach/detach listeners that are to be called
upon saving the cutomizer changes in subclasses. That should allow more complex
modifications of the project metadata by a 3rd party plugin.

The actual customizer implementations are assumed to subclass this common
parent, provide the actual saving logic and make the class available to 3rd
party plugins in the lookup passed to CompositeCategoryProvider. That should
make the coupling between the project type implementation and the plugins that
implement customizer panels more loose.

please review, comments welcome.
Comment 8 Jean-francois Denise 2006-10-03 15:46:00 UTC
I didn't find any API to load properties when the customizer is diplayed. 
The customizer needs to initialize its state with what is currently in
properties files. Did I miss something? Do I still have to implement it myself?


If you can confirm that the API allows me to write the following code, I think
that your fix answer our properties storage problem.

// In my Customizer (My class tha implements
ProjectCustomizer.CompositeCategoryProvider
public JComponent createComponent(ProjectCustomizer.Category category, Lookup
context) {

     AntProjectProperties projectProperties =      
            context.lookup(AntProjectProperties.class);

      // MyPanel is an ActionListener. In its constructor it calls 
      // projectProperties.addStoreActionListener(this); and keeps a reference
      // on projectProperties to be used in eventHandler
     MyPanel myPanel = new MyPanel(projectProperties);

    return myPanel;
}

// MyPanel Actionlistener eventhandler
// Called when it is time to store properties 
public void actionPerformed(ActionEvent event) {

// I want to store all properties in project.properties
// I first create EditableProperties based on the user inputs. NOT DETAILED HERE
// Then store it in the public StoreGroup

projectProperties.getPublicStoreGroup().store(myEditableProperties);

}
Comment 9 Milos Kleint 2006-10-04 15:17:18 UTC
jfdenise:
there isnt't any handling for reading the current properties. My assumption was
that it's accessible through the project instance? is it not?

you have the first part of your sample correct. The final part with calling the
store() method on StoreGroup is wrong. it would not do anything I suppose.

Here are the 3 usecases that I envisioned.
1. After obtaining the AntProjectProperties instance you create GUI
models/documents through one of the the StoreGroup instance factory methods.
These are then automatically stored upon changing.
2. alternatively you use your own GUI models and upon change you write them
through the putAdditionalProperty method immediately.
1+2. gets saved by the AntProjectProperties subclass automatically, nothing to
care for on your side.
3. if you are not writing to project.properties or private.properties or have
some complex logic behind it, you attach the listener, read the project data (or
use some previously read) and store in your own custom way.

you should not be required to create/load EditableProperties instance. Writing
the content of public/private StoreGroup from APP could actually create a
dataloss situation.


Comment 10 Jean-francois Denise 2006-10-04 15:38:16 UTC
I currently access the project properties in this way that seems not perfect.
There is perhaps another way.
FileObject projectPropsFile =
project.getProjectDirectory().getFileObject(AntProjectHelper.PROJECT_PROPERTIES_PATH);

So StoreGroup.store() should not be public. 
The Interface of the class exposes services that are not really usable.

In my case I would have to call putAdditionalProperty for each property. Would
it be possible to put all properties in 1 call (by passing a Properties object).
 

Comment 11 Jesse Glick 2006-10-06 18:58:29 UTC
The proposed API will not work with j2seproject run configurations. Would prefer
that the project.properties/private.properties distinction not be hardcoded;
rather parametrize appropriate methods with a String path (as in
AntProjectHelper constants).


BTW please: echo 'diff -u' >> ~/.cvsrc


Prefer to use Map<String,String> to Properties where possible.
Comment 12 Milos Kleint 2006-10-12 13:50:52 UTC
jfdenise: re: SourceGroup.store() is already in a public api, no way to make it
non-public, I actually overlooked this method. It's indeed confusing who should
be calling store() on it and when. The StoreGroup related methods should be
probably pulled from the API.

jglick: after reading yur comments, I realized there already was an API proposal
for reading/storing properties and was deffered -
http://www.netbeans.org/issues/show_bug.cgi?id=73717 . Not sure I want to go
into that right now for the customizer.

Thanks for the review, the proposal was too hasty and was probably trying to do
too much. Let me rework it and submit again. I will limit the proposal to
notification of panels so that changes can be applied, how the changes are to be
read/applied is out of scope for now.

Comment 13 Milos Kleint 2006-10-16 08:22:58 UTC
Created attachment 35222 [details]
new, simplified  proposal
Comment 14 Milos Kleint 2006-10-16 08:43:18 UTC
please review the reworked api proposal.

The current api change involved allowing the
ProjectCustomizer.CompositeCategoryProvider to add a button listener on the
Category instance it creates. All the listeners get notified when the ok button
is pressed.

not sure if the listeners ought to be notified under project mutex or not,
currently they are not.
Comment 15 Jean-francois Denise 2006-10-17 10:32:40 UTC
Your API change allows us to store properties. 
Your sentence linked to Project mutex makes me unconfortable. 
In the OKButton handler we will access the Project properties file and update
it. Hope it integrates nicely with your internal project behavior.
Comment 16 Milos Kleint 2006-10-17 10:42:01 UTC
jfdenise: you should be writing the project data under write mutex in any case.
The comment was only about multiple small mutex locks compared to a big one
encompassing all the writes..
Comment 17 Jesse Glick 2006-10-19 23:27:16 UTC
Looks fine to me.


Javadoc could be cleaned up a bit - capitalization etc.


Listeners should probably be notified under project write mutex.
Comment 18 Milos Kleint 2006-10-23 12:25:57 UTC
thanks for comments, will notify listeners under write mutex.
about to integrate.
Comment 19 Milos Kleint 2006-10-23 15:22:21 UTC
done.