Created attachment 137463 [details]
The one and only API class

Created attachment 137464 [details]
Architecture description

Created attachment 137465 [details]
The whole patch

Please review the propose API changes.

Notes:
- there are no unit test yet but the aim is to have all API methods covered
- the module will not be in NetBeans 7.4 distribution and will be available from update center only

MK1: first of all I believe the api is fairly simplistic and as such of limited use. I've raised the objection when I was first approached and it stands. This api will only confuse potential users IMHO.

MK2: There's errors in implementation that have influence on the API signatures as well.

90             MavenProject prj = nbMaven.getMavenProject();
91	        String res = prj.getProperties().getProperty( propertyName );
92	        if( res == null ) {
93	            res = PluginPropertyUtils.getPluginProperty( prj, groupId, artifactId, propertyName, null, null );
94	        }

The problem here is that propertyName is used as both plugin parameter and property which doesn't hold in most cases. If you use PluginPropertyUtils.getPluginParameter() properly and fill out the last method parameter, it will do the getProperties() check for you as well, along with other things I believe..
See [1] as an example: while the propertyName is "target" here, the thing to look for in prj.getProperties() is "maven.compiler.target"

MK3: The set method (model operation) suffers from the same problem as MK2. The problem here is a bit less subtle though. You should not set both the property in getProperties() and the plugin parameter. One will effectively take over anyway so the other is redundant.

MK4: the modeloperation used will silently fail when the plugin is not yet defined in the project at the place you expect. So for example if you get the plugin configuration defined in the parent pom, the "get" method will resolve the value (as it works on top of resolved model), but any writing in "set" will be a noop. At the very least needs to be properly documented but in general leaves the potential users with the need to use the friend apis for any reasonably complex poms encountered.



[1] http://maven.apache.org/plugins/maven-compiler-plugin/compile-mojo.html#target

Y01 Add private constructor to the utility method class

Y02 "The main use case is implementing a panel to be added to Maven project properties window to adjust properties of a Maven plugin used in that project." - I'd like to see an example of how that can be done. A sample code snippet in arch.xml, references to other APIs one needs to use, etc. I guess one has to use @ProjectCustomizer.CompositeCategoryProvider.Registration(projectType = "org-netbeans-modules-maven", position = xyz) to register. I believe it is worth to tell that to users of the API.

Y03 I am not sure what performance team will say, but I'd like to see a scalable API which won't impact projects that don't use the plugin at all - e.g. don't use the above @ProjectCustomizer.CompositeCategoryProvider.Registration, but rather some new annotation to prevent loading of customizer classes for unused plugins.

Y04 Instance based API, rather than static methods API. Maybe: 
static PluginConfig find(pomFile, groupId, artefactId).

Re. MK01: I encourage Standa in his decision to create publicly available API. It may start simplistic, but it is a start and the direction is good. If designed with extensibility in mind, the API may grow as further use-cases are gathered. Re. confusion: We don't have any Maven API available to public (nothing at http://bits.netbeans.org/dev/javadoc/), just a habit to add more and more friends to an API that we are afraid to commit to and open up. Thus I am not afraid of any confusion.

I don't (want to) understand MK02-MK04, so I leave it up to you guys to sort that out.

Created attachment 138742 [details]
New patch

MK2-4 - will be addressed later
Y01-4 - fixed

Main changes - there's a new annotation to register category providers for Maven projects, the group id and artifact id are part of the annotation.
API customers must sub-class MavenCategoryProvider which has getPluginUtilities() method to get/set plugin properties.

Overall the patch is good and can be integrated as it is now. Here are some minor thoughts will might make things even better:

Y05 I don't think there is a need for multiple Registrations on a single CompositeCategoryProvider. No need to support that imho moreover it does not make much sense - would the same provider appeared there twice? Delete the code and the processor will be even simpler.

Y06 Utility classes usually contain static methods. That is no longer true. Thus I suggest to rename PluginUtilities to PluginConfigInfo or something like that.

Y07 I prefer less classes in API and as such I would remove MavenCategoryProvider (that requires to give the registration annotation full name) and accepted any CompositeCategoryProvider. The code would look like:

@MavenPluginCategoryRegistration(...)
public class MyCatPro implements CompositeCategoryProvider {
  private ProjectConfigInfo info;
  public MyCatPro(ProjectConfigInfo info) {
    this.info = info;
  }
}

or 

@MavenPluginCategoryRegistration(...)
static CompositeCategoryProvider factory(ProjectConfigInfo info ) { ... }

the annotation processor would have to check the type of factory method or constructor is valid (similar check is done in openide.awt/src/org/netbeans/modules/openide/awt/ActionProcessor.java). However whether Y07 should be implemented is a matter of taste, I don't insist on it.

Created attachment 139079 [details]
New patch

New patch to address MK2, MK3, MK4, Y05, Y06

If there are no more comments I'll integrate the changes on Monday.

core-main 9fed5f1d1739

I forgot to include the new files - core-main 153ee3e97c31

Integrated into 'main-silver', will be available in build *201308270001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/9fed5f1d1739
User: S. Aubrecht <saubrecht@netbeans.org>
Log: #233079 - Maven Plugin Params API

I successfully used this API and modules that depend on it in NetBeans 7.4 build. Thanks. I am glad we managed to expose this important Maven functionality without creating new external friend dependencies.