Is there a way to get access to this enhancement before 6.9 is released?

If you write a patch for it and submit it for review, then yes. Otherwise? Maybe, but probably not.

Created attachment 97582 [details]
Patch for default implementation and removed private implementation

Here is my first cut for this issue.  I think that a much larger refactoring and design change is necessary to make implementing project types easier and use less code.  This only includes projects that relate to java language projects (j2ee/j2se).  I looked at other projects but decided not to include them, since I don't think making other languages depend on java.api.common is desirable.

Thanks, but I don't see the source of ProjectInfoImpl in your patch. Perhaps you did not do 'hg add' so it is not included in the diff?

Right, need to wait for complete patch to review impl.


[JG01] Please avoid unnecessary changes in patches, such as j2ee.archive/src/org/netbeans/modules/j2ee/archive/project/UpdateHelper.java. (Source cleanup patches are welcome too, but should be kept separate from behavior changes.)


[JG02, minor] Avoid introducing a dummy variable like projectInfo for a value that is used just once; simpler to just put the expression inline in the call to Lookups.fixed.


[JG03] Change to EarProjectOperations looks suspicious; old code used 'project', new uses 'original'.

(BTW it is also recommended to use ProjectUtils.getInformation rather than directly asking a Project.lookup for ProjectInformation.)


[JG04] apichanges.xml//change@id should be descriptive whenever possible; suggest "ProjectInfoImpl".

Created attachment 97679 [details]
Default implementation of ProjectInformation

I have made the requested changes.  I also filed a feature request for Mercurial to have 'add file' from a menu as I could not find it anywhere.

I did not provide JavaDoc for implemented methods, but did for the non-api methods I created.

(In reply to comment #6)
> I did not provide JavaDoc for implemented methods, but did for the non-api
> methods I created.

That's fine.


[JG05] It is likely there is no reason to override ProjectInfoImpl; the one user in your patch is ArchiveProject, which is using updateHelper but could probably equally well use helper. Could omit configurationNameSpace from PII (pass null to XMLUtil.findElement to ignore namespace), thus working with any revision of project.xml's schema so long as it used <name>.

In that case PII would not need to be exposed as public at all - could just be a factory method returning ProjectInformation. (apichanges.xml would need to be changed correspondingly, of course.)


[JG06] Do not use ProjectInformation.PROP_NAME. That is a bean property name. It only coincidentally happens to be the name of the XML element <name>.

TZ01: Don't use finalize. For removing listeners use rather WeakListeners. When finalize is needed, things like reachability fence, finalize should be final to prevent to be overridden and not called or better finalizer guardian should be used.
TZ02: The ProjectInfoImpl has problems with races. The methods getName, getDisplayName and configurationXmlChanged should be guarded.
TZ03: The element name in the getPropertyFromConfiguration should be parametrized (passed by the client). There may be project types having different element name.
TZ04: The ProjectInfoImpl deserves an unit test.

TZ05: J2SEProject.Info() was using updateHelper but after rewrite it's using AntProjectHelper. This would not works for j2seproject/1 and j2seproject/2.

(In reply to comment #8)
> TZ03: The element name in the getPropertyFromConfiguration should be
> parametrized (passed by the client). There may be project types having
> different element name.

Seems unlikely, but would not hurt. See also JG06.

(In reply to comment #9)
> TZ05: J2SEProject.Info() was using updateHelper but after rewrite it's using
> AntProjectHelper. This would not works for j2seproject/1 and j2seproject/2.

I don't think this will be a problem if JG05 is accepted (see note about namespace==null).

For [JG05], what factory class would be used to create PII?

(In reply to comment #11)
> For [JG05], what factory class would be used to create PII?

Perhaps QuerySupport.

(This is a friend-only API so the decision could be changed in the future.)

>JG05:
I doubt it will work if implemented as proposed in the attached patch:

Element data = projectHelper.getPrimaryConfigurationData(shared)

where the APH.getPrimaryConfigurationData() does:

final String namespace = type.getPrimaryConfigurationDataElementNamespace(shared);
Element el = getConfigurationFragment(name, namespace, shared);

for older project types it will not find the data element. It will need to use getConfigurationFragment() with all versions or not to use NS at all.


TZ03: I know about at least one project type which uses displayName but it's not a java (ant) based. If this API should cover only J2SE project copies I am fine with this. If we want to move it in the future into generic project API, as it's not java specific, this will be a problem.

(In reply to comment #13)
> for older project types it will not find the data element. It will need to use
> getConfigurationFragment() with all versions or not to use NS at all.

You're right, this won't work.

Perhaps two variants of the factory method, one which takes AntProjectHelper, one which takes Updatehelper?

> TZ03: I know about at least one project type which uses displayName but it's
> not a java (ant) based.

Which? Does it use project.ant?

(In reply to comment #13)
> >JG05:
> I doubt it will work if implemented as proposed in the attached patch:
> 
> Element data = projectHelper.getPrimaryConfigurationData(shared)
> 
> where the APH.getPrimaryConfigurationData() does:
> 
> final String namespace =
> type.getPrimaryConfigurationDataElementNamespace(shared);
> Element el = getConfigurationFragment(name, namespace, shared);
> 
> for older project types it will not find the data element. It will need to use
> getConfigurationFragment() with all versions or not to use NS at all.
Does the namespace need to be used to find the data node?  Are there going to be more than one within the data node within configuration section?  If there is only 1 data section, changes to projectHelper.getPrimaryConfigurationData(shared) can be made to not require a namespace.

Created attachment 97957 [details]
Default implementation of ProjectInformation

I think I have addressed everything except for the unit test and factory creation.  I need to figure out how to create an AntProjectHelper.

I made the assumption that there will only be one configuration-data node in a project.xml and added a new method to AntProjectHelper that allows for that.  If this is true the AntProjectHelper API will need to be updated.  Would this be a different bug?

TZ06: I would prefer the Jesse's solution (2 versions of factory method APH and UH). The problem of the new method APH.getPrimaryConfigurationData is that it uses just the simple name and there can be more element having the same simple name (elements added by AuxiliaryConfiguration.putConfigurationFragment).
Otherwise OK, the test can be added later not a TCR.
Thanks

(In reply to comment #15)
> Does the namespace need to be used to find the data node?  Are there going to
> be more than one within the data node within configuration section?

There could be (with different namespaces). getPrimaryConfigurationData(true) always returns the "right" one, generally the most recent namespace.

(In reply to comment #16)
> I need to figure out how to create an AntProjectHelper.

You need to use ProjectGenerator to actually create a dummy project.

> added a new method to AntProjectHelper that allows for that

Not necessary and should not be there. Recommend use of either (unmodified) AntProjectHelper, or UpdateHelper, as suggested in comment #14.

Created attachment 98404 [details]
Default implementation of ProjectInformation

I implemented 2 factory methods in QuerySupport.  I made ProjectInfoImpl abstract with two static inner classes of QuerySupport implementing the AntProjectHelper and UpdateHelper specific logic.  The tests were added to QuerySupport.  I may have cheated on the UpdateHelper test case.

I also updated the API to reflect the QuerySupport changes.  If ProjectInfoImpl should not be exposed, let me know, and how to hide i.

Any comments?

Sorry, I was not receiving email there for a while. I will review your latest patch shortly.

Seems fine to me. Just add the javadoc to factory method in the QuerySupport.

(In reply to comment #19)
> If ProjectInfoImpl should not be exposed, let me know, and how to hide it.

I can do that; just make it package private and move to the same package as QuerySupport.

Committed with some other minor edits; thanks! core-main #1ac1517c44f1

Integrated into 'main-golden', will be available in build *201005192201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/1ac1517c44f1
User: mvfranz@netbeans.org
Log: #110886: default implementation of ProjectInformation.

Just found that web.project should have been included in this patch.  Do we need a new bug?

Yes, file a separate bug in javaee/web project blocking this one.

ant.freeform also has a comment referring to this bug.  Since ant.freeform does not already refer to java.api.common, the comment is wrong and should be removed.

(In reply to comment #27)
> ant.freeform also has a comment referring to this bug.  Since ant.freeform does
> not already refer to java.api.common, the comment is wrong and should be
> removed.

I don't think so. ant.freeform ought to use the same impl, since it uses project.ant and sets the project name from project.xml#//name like many other project types. Just because the impl happens to be in java.api.common now and ant.freeform cannot refer to that (since it is not specific to the Java language) does not mean that this situation will not be fixed in the future, say by promoting the API to project.ant.