Delegate Model Processing to parent implementation

[jycr]

By default, Eclipse Sisu IoC implementation knows all candidates of org.apache.maven.model.building.ModelProcessor implementation. So, there is always org.apache.maven.model.building.DefaultModelProcessor in this list (as well as the current me.qoomon.maven.gitversioning.GitVersioningModelProcessor).

When a project uses another custom ModelProcessor like the polyglot extension (see Maven Website), this list also contains this alternative implementation.

This change allows delegating core methods (File locatePom( File projectDirectory ) and Model read(...)) to the "parent implementation" (defaults to org.apache.maven.model.building. ModelProcessor or a custom implementation if applicable).

This PR fixes #268

[qoomon]

@jycr thx for your PR I came up with basically the same idea :-D, however I went for setter injection of all Processors. I tend to like setter injection a little more, because it is easier to read. WDYT?

    @Inject
    private void setDelegatedModelProcessor(List<ModelProcessor> modelProcessors) {
        this.delegatedModelProcessor = modelProcessors.stream()
                .filter(it -> it != this)
                // There is normally always at least one implementation available: org.apache.maven.model.building.DefaultModelProcessor
                .findFirst().orElseThrow(() -> new NoSuchElementException("Unable to find default ModelProcessor"));
    }

[jycr]

@jycr thx for your PR I came up with basically the same idea :-D, however I went for setter injection of all Processors. I tend to like setter injection a little more, because it is easier to read. WDYT?

    @Inject
    private void setDelegatedModelProcessor(List<ModelProcessor> modelProcessors) {
        this.delegatedModelProcessor = modelProcessors.stream()
                .filter(it -> it != this)
                // There is normally always at least one implementation available: org.apache.maven.model.building.DefaultModelProcessor
                .findFirst().orElseThrow(() -> new NoSuchElementException("Unable to find default ModelProcessor"));
    }

Why not :) For my part, I prefer dependency injection by constructor. I'll push a new commit to follow your idea.

[qoomon]

There is one more issue, we need to tackle or at least throw an appropriate error. At following line I need to write the artificial pom.xml to do that I first read the original one, however in case of polyglot-yaml extension the source is a yaml file. See projectModel.getPomFile() returns pom.yaml https://github.com/qoomon/maven-git-versioning-extension/blob/59c80d2dfabbc6b44f0291ad6c2c17f3b12a7040/src/main/java/me/qoomon/maven/gitversioning/GitVersioningModelProcessor.java#L1166

I think, if the pom file is not an xml file I should skip the pom.xml file processing and just generate a pom.xml from the projectModel

How is it intended to work for the polyglot extensions e.g. if you release something to maven central. Will polyglot extensions generate an pom.xml as well?

[jycr]

There is one more issue, we need to tackle or at least throw an appropriate error. At following line I need to write the artificial pom.xml to do that I first read the original one, however in case of polyglot-yaml extension the source is a yaml file. See projectModel.getPomFile() returns pom.yaml

https://github.com/qoomon/maven-git-versioning-extension/blob/59c80d2dfabbc6b44f0291ad6c2c17f3b12a7040/src/main/java/me/qoomon/maven/gitversioning/GitVersioningModelProcessor.java#L1166

I think, if the pom file is not an xml file I should skip the pom.xml file processing and just generate a pom.xml from the projectModel

How is it intended to work for the polyglot extensions e.g. if you release something to maven central. Will polyglot extensions generate an pom.xml as well?

Indeed, I call File locatePom(File projectDirectory) instead of calling projectModel.getPomFile() to prevent this issue

[qoomon]

...and obviously the update pom feature is not supported as well

[jycr]

...and obviously the update pom feature is not supported as well

I think that this is indeed a limitation that remains for the use of several ModelProcessors (in particular for the use of Maven Polyglot). However, I don't think this option is used by the majority of users of this extension?

[qoomon]

I came up with following solution that works on my machine

 private File writePomFile(Model projectModel) throws IOException {
        File gitVersionedPomFile = new File(projectModel.getProjectDirectory(), GIT_VERSIONING_POM_NAME);
        logger.debug("generate " + gitVersionedPomFile);

        if(projectModel.getPomFile().getName().endsWith(".xml")) {
            // read original pom file
            Document gitVersionedPomDocument = readXml(projectModel.getPomFile());
            Element projectElement = gitVersionedPomDocument.getChild("project");

            // update project
            updateParentVersion(projectElement, projectModel.getParent());
            updateVersion(projectElement, projectModel);
            updatePropertyValues(projectElement, projectModel);
            updateDependencyVersions(projectElement, projectModel);
            updatePluginVersions(projectElement, projectModel.getBuild(), projectModel.getReporting());

            updateProfiles(projectElement, projectModel.getProfiles());

            writeXml(gitVersionedPomFile, gitVersionedPomDocument);
        } else {
            writeModel(gitVersionedPomFile, projectModel);
        }

        return gitVersionedPomFile;
    }

[qoomon]

Indeed, I call File locatePom(File projectDirectory) instead of calling projectModel.getPomFile() to prevent this issue

I think this also only works by accident, because the method documentation does not say anything about what kind of pom file it should return.

[jycr]

I came up with following solution that works on my machine

 private File writePomFile(Model projectModel) throws IOException {
        File gitVersionedPomFile = new File(projectModel.getProjectDirectory(), GIT_VERSIONING_POM_NAME);
        logger.debug("generate " + gitVersionedPomFile);

        if(projectModel.getPomFile().getName().endsWith(".xml")) {
            // read original pom file
            Document gitVersionedPomDocument = readXml(projectModel.getPomFile());
            Element projectElement = gitVersionedPomDocument.getChild("project");

            // update project
            updateParentVersion(projectElement, projectModel.getParent());
            updateVersion(projectElement, projectModel);
            updatePropertyValues(projectElement, projectModel);
            updateDependencyVersions(projectElement, projectModel);
            updatePluginVersions(projectElement, projectModel.getBuild(), projectModel.getReporting());

            updateProfiles(projectElement, projectModel.getProfiles());

            writeXml(gitVersionedPomFile, gitVersionedPomDocument);
        } else {
            writeModel(gitVersionedPomFile, projectModel);
        }

        return gitVersionedPomFile;
    }

I don't think that this modification is needed.

You can only write POM in XML langage.

So, I will modify:

        if (updatePom) {
            logger.debug("updating original POM file");
            Files.copy(
                    gitVersionedPomFile.toPath(),
                    projectModel.getPomFile().toPath(),
                    StandardCopyOption.REPLACE_EXISTING);
        }

by

        if (updatePom) {
            logger.debug("updating original POM file");
            Files.copy(
                    gitVersionedPomFile.toPath(),
                    locatePom(projectModel.getProjectDirectory()).toPath(),
                    StandardCopyOption.REPLACE_EXISTING);
        }

[qoomon]

You need to handle the yaml pom on two places (in writePomFile(), and in case of updatePom feature is used)

        File gitVersionedPomFile = writePomFile(projectModel);
        if (updatePom) {
            if(projectModel.getPomFile().getName().endsWith(gitVersionedPomFile.getName().replaceAll(".*(?=\\.)", ""))) {
                logger.debug("updating original POM file");
                Files.copy(
                        gitVersionedPomFile.toPath(),
                        projectModel.getPomFile().toPath(),
                        StandardCopyOption.REPLACE_EXISTING);
            } else {
               logger.warn("updating original POM file only works for pom.xml files");
            }
        }

[jycr]

In writePomFile I get XML translated File (if Polyglot extension used) or regular pom.xml (if no other ModelProcessor used):

Document gitVersionedPomDocument = readXml(this.locatePom(projectModel.getProjectDirectory()));

See: https://github.com/qoomon/maven-git-versioning-extension/pull/269/files#diff-c3f0a423d7388864cf22420318507613b6d071eff5d724dc50127ad35e08be84R1197

if updatePom feature is used, it updates the XML translated File (if Polyglot extension used) or regular pom.xml (if no other ModelProcessor used):

Files.copy(
    gitVersionedPomFile.toPath(),
    locatePom(projectModel.getProjectDirectory()).toPath(),
    StandardCopyOption.REPLACE_EXISTING);

See: https://github.com/qoomon/maven-git-versioning-extension/pull/269/files#diff-c3f0a423d7388864cf22420318507613b6d071eff5d724dc50127ad35e08be84R321

[qoomon]

Well, lets try