Generate converters to move data from one model version to another

/modello-plugin-converters/src/main/org.codehaus.modello.plugin.converters.ConverterGenerator seems to do something like this:
"Generate a basic conversion class between two versions of a model."

Was that what you wanted?

I've tried to use ConvertorGenerator to generate basic nexus configuration upgrade code. Unfortunately, this did not work too well.

First, generated converter classes appear to assume that the "current" model version is generated into package both with and without version, i.e. the same v4 model classes expected to appear in o.c.m.test.maven.v4_0_0 and o.c.m.test.maven packages, which is not desired behaviour for nexus. As a side effect of this, there is a converter from one copy of the model to another.

Second, generated converter classes do not properly handle removal of attributes from newer model version.

And lastly, there is no way to disable generation of ConverterTool (and I could not make sense out of generated code).

Originally I tried to fix these problems in the existing generator but this proved to be impossible without changing existing clients of generated code (assuming there are any, of course) so I decided to create brand new generator. The new generator emphasizes upgrading model to the latest version (hence the "upgrade" name).

Let me know if this all makes sense.

PS: I am willing to support the new generator, if this makes any difference.

that sounds reasonable.

one place I've used it is here:
http://svn.apache.org/viewvc/maven/components/branches/maven-2.0.x-terse/maven-project/src/main/java/org/apache/maven/project/ModelReader.java?revision=620422&view=markup&pathrev=620422

would this continue to work in that revision with some changes?

I committed the code in a branch: see branches/MODELLO-4, and try to understand things.

I will need to write simple unit-tests to really grab the whole thing IMHO, since Maven model is too complex.

From the first work at it, I have some comments:

1. I like the new direct conversion to a model without package

2. Igor: the upgrade package is generated in toVersion instead of fromVersion. I suppose this is not the intended behaviour. Can you confirm this? I'll fix String packageName = objectModel.getDefaultPackageName( true, toVersion ) + ".upgrade";

3. I don't understand every method of ConvertTool: convertFromFile can be useful, but I don't understand other methods
Brett: do you know if this tool has really been used? Do we have to really care with compatibility?

4. Brett: for your maven-2.0.x-terse code, upgrade is exactly like convert on the used part: you should not have any problem to change from one to another

Brett,

I've added new generator/mojo but kept the old behaviour unchanged so your maven-2.0.x-terse code will continue work. You will need to change maven-2.0.x-terse ModelReader a bit if you want to switch to the new generator, to compensate for different package/class names and different handling of removed/readded elements.

Also, I think it is better to remove ConvertTool altogether, if no one is currently using it. New model versions means new/changed/removed elements. I do not believe it is possible to completely generate conversion code, but ConvertTool appear to assume it is.

ok, I see the logic.
But if I look at the Maven model example:

• the converter is in org.codehaus.modello.test.maven.v4_0_0.upgrade package, but there is no model in org.codehaus.modello.test.maven.v4_0_0
• if we want mutliple old versions supported (3.0.0 and 4.0.0 when we'll have 4.1.0), we'll need 2 converters: they'll need to be in different packages

I don't think this "old versions shouldn't know newer ones" logic is applicable here.
This is generated code: the last version, ie "current", is generated without its version in package name.
Other versions, either older or even newer, are generated just for conversion, in their own package with version: generating a converter to the current version is part of the package, IMHO.

I'm interested in unit tests to show every automatic conversion features. Showing limits of the conversion would be interesting too. If you have time to prepare it, I'm interested

Also, I think it is better to remove ConvertTool altogether, if no one is currently using it. New model versions means new/changed/removed elements. I do not believe it is possible to completely generate conversion code, but ConvertTool appear to assume it is.

Good point: +1
We'l do a better job with javadoc showing sample code of basic automatic transformation and explaining that user will propably extend BasicConverter to handle more subtle changes

As I mentioned, which package holds generated upgrade code does not matter much as long as the code works.

I do not have unit tests for "every automatic conversion features" at the moment but attached find simple project that demonstrates a problem with generated BasicVersionConverter when model attribute changes type from one model version to another (see compilation errors in org.codehaus.modello.test.modello0004.v1_0_1.convert.BasicVersionConverter). Hope this helps.

I tried to create a spec of this plugin, to help think about the code that should be generated: see http://modello.codehaus.org/modello-plugins/modello-plugin-converters/

apart from bug fixes, I am ok with 2 changes proposed by Igor:

• remove ConverterTool, since BasicVersionConverter should often be overriden by the developper to handle subtle model changes, which isn't possible with this generated class
• have a direct change to model without version

And I have other changes to propose:

• only convertRootClass() method should be public, other should be protected (don't need to expose internal methods),
• the target version should not be max(modello.all.versions) but should be modello.version: it's developer's choice to define the target version
• then modello-maven-plugin should invoke converters plugin only once, and converters plugin generate conversion classes from every modello.all.versions to modello.version

WDYT?

instead of removing ConverterTool, I finally think that it would be better to:
1. move it to a dedicated converter-tool plugin
2. add methods to set VersionConverter implementations, to be able to override BasicVersionConverter