Bug 193549

Created attachment 104500 [details]
@ProjectRegistration

There are two problems with the previous patch. There is no support for saveProject and no ProjectState is passed to the factory method.

Implementation that handles ProjectState and saveProject:
http://hg.netbeans.org/ergonomics/rev/185ba0da2385
Its usage in 
http://hg.netbeans.org/ergonomics/rev/ea89235cffb7

Is this enhancement suitable for 7.0?

[JG01] I see no purpose for the mimeType attribute. Delete it unless there is a clear use case.


[JG02] The overall structure of the annotation seems more complicated than necessary. The usage of Flushable is also unnecessarily novel. A simpler suggestion is for the client to still implement ProjectFactory{,2} but register the factory using a new annotation rather than @ServiceProvider:

@ProjectFactory.Registration(iconBase="...", relativeFile="pom.xml")
public class MavenFactory implements ProjectFactory2 {
  // loadProject, isProject, isProject2, saveProject as usual
}

This would just register a proxy PF2 which implements isProject{,2} based on the metadata and returns quickly from loadProject for inappropriate dirs. If and when loadProject is called with a matching dir, the real factory is loaded and henceforth all method calls delegate to it.

This style:

1. Bypasses the need for the Flushable trick.

2. Affords more flexibility once class loading has already been triggered. For example, you could return a different icon from isProject2 if doing so is cheap enough. (Compare @ActionRegistration on an always-enabled Action instance.)

3. Provides the quickest migration path for existing PF{,2} impls.

4. Makes for a trivial annotation processor: one call to instanceAttribute.

Re. JG01 the mime attribute would allow CND to recognize nbproject/project.xml file, and not get triggered until there is the necessary content - e.g. correct project type declaration.

Re. JG02 - I was hoping to get away from the current style of registration and simplify it a bit. However I agree that Flushable contract is not nice.

(In reply to comment #4)
> Re. JG01 the mime attribute would allow CND to recognize nbproject/project.xml
> file, and not get triggered until there is the necessary content - e.g. correct
> project type declaration.

What would the MIME type be other than text/xml?

There is a need to scan content of the files to find out whether it form a valid project of this type or not. This is certainly true for Maven's pom.xml (and the support for Java/JavaEE/Scala, etc.), for Ruby (as it uses nbproject/project.xml) and possibly also for cnd.

I can define my own way of looking into the file, but rather I'd like to reuse a standard.

We have one standard, efficient and declarative way of looking into any file. The one used during mime resolution. Thus I want to reuse it.

So to answer your question, the mime type would be something like text/xml+cnd-project. Of course only if CND defines appropriate mime resolver to scan the content of the file and recognize it as such mimetype.

(In reply to comment #6)
> There is a need to scan content of the files to find out whether it form a
> valid project of this type or not. This is certainly true for Maven's pom.xml
> (and the support for Java/JavaEE/Scala, etc.)

No, these all produce NbMavenProjectImpl; pom.xml need not be opened to recognize a Maven project.

> We have one standard, efficient and declarative way of looking into any file.
> The one used during mime resolution. Thus I want to reuse it.
> 
> So to answer your question, the mime type would be something like
> text/xml+cnd-project. Of course only if CND defines appropriate mime resolver
> to scan the content of the file and recognize it as such mimetype.

I guess in this case text/xml+cnd-project will not be used for anything else, so it will become a regular XMLDataObject and have the normal validate actions etc.?

Created attachment 114960 [details]
@ProjectFactory.Registration and its usage in maven

Simplified to refer only to requiredFile and be applicable only to ProjectFactory. I'd like to integrate on Jan 24, 2012.

[JG03] GenericProjectFactory.saveProject should call delegate(). Possibly does not matter, but more clearly right.


[JG04] GPF.isProject does not call delegate.isProject at all, which is rather confusing. In the case of NbMavenProjectFactory, it means that the customized isProject is ignored except when used internally as part of loadProject; NbMPF.iP could as well throw an assertion error or unconditionally return true, and move that logic into loadProject. I would recommend that GPF.isProject call delegate.iP when fo != null && delegate != null, but if not it should at least be clearly documented that PF.iP will never be called (contrary to current Javadoc "all operations are delegated to it").


[JG05] The default value of iconBase clashes with the fact that GPF.isProject2 expects iconBase to be non-null. PM.Result does support a null icon but we would not want to encourage this (ProjectChooserAccessory in this case simply forces loading of the project). The Javadoc says the factory may implement PF2 _or_ specify an iconBase, but this is broken: (a) the processor does not check that either is true; (b) when the delegate has not yet been loaded, iconBase is still needed (though a fix of JG04 may change that).


[JG06] Processor buglets: iconBase should be passed through validateResource; roundEnv.processingOver should be checked; pr == null should be checked; instanceFile or instanceAttribute should use long form with annotation name.


[JG07] Test should not call Lookup.getDefault().lookup(ModuleInfo.class). Use MockLookup.setLayersAndInstances().


[JG08] Would like to see more than one usage: AntBasedProjectFactorySingleton, MakeBasedProjectFactorySingleton, GrailsProjectFactory?

Seems like ergonomics are happy with .properties specification of important files and otherwise there was no need for this annotation. Closing for now.