You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by rf...@apache.org on 2019/12/30 14:23:42 UTC
[maven] 06/06: [MNG-6656] Introduce custom ModelMerger to keep
inputLocationTracker from fileModel in rawModel
This is an automated email from the ASF dual-hosted git repository.
rfscholte pushed a commit to branch MNG-6656
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 3d453bf25df69fecd645561f1d4b39732d52c5b1
Author: rfscholte <rf...@apache.org>
AuthorDate: Mon Dec 30 15:23:16 2019 +0100
[MNG-6656] Introduce custom ModelMerger to keep inputLocationTracker from fileModel in rawModel
---
.../maven/model/building/DefaultModelBuilder.java | 196 +++++++++++++++++++--
.../model/building/DefaultModelBuilderFactory.java | 1 +
.../model/validation/DefaultModelValidator.java | 35 ++--
.../model/building/FileToRawModelMergerTest.java | 82 +++++++++
.../validation/DefaultModelValidatorTest.java | 2 +-
5 files changed, 279 insertions(+), 37 deletions(-)
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
index 5bfe617..0e2f10a 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
@@ -57,15 +57,21 @@ import org.apache.maven.artifact.versioning.VersionRange;
import org.apache.maven.feature.Features;
import org.apache.maven.model.Activation;
import org.apache.maven.model.Build;
+import org.apache.maven.model.BuildBase;
+import org.apache.maven.model.CiManagement;
import org.apache.maven.model.Dependency;
import org.apache.maven.model.DependencyManagement;
import org.apache.maven.model.InputLocation;
import org.apache.maven.model.InputSource;
import org.apache.maven.model.Model;
+import org.apache.maven.model.ModelBase;
import org.apache.maven.model.Parent;
import org.apache.maven.model.Plugin;
+import org.apache.maven.model.PluginContainer;
import org.apache.maven.model.PluginManagement;
import org.apache.maven.model.Profile;
+import org.apache.maven.model.ReportPlugin;
+import org.apache.maven.model.Reporting;
import org.apache.maven.model.Repository;
import org.apache.maven.model.building.ModelProblem.Severity;
import org.apache.maven.model.building.ModelProblem.Version;
@@ -75,6 +81,7 @@ import org.apache.maven.model.interpolation.ModelInterpolator;
import org.apache.maven.model.io.ModelParseException;
import org.apache.maven.model.management.DependencyManagementInjector;
import org.apache.maven.model.management.PluginManagementInjector;
+import org.apache.maven.model.merge.ModelMerger;
import org.apache.maven.model.normalization.ModelNormalizer;
import org.apache.maven.model.path.ModelPathTranslator;
import org.apache.maven.model.path.ModelUrlNormalizer;
@@ -124,7 +131,7 @@ public class DefaultModelBuilder
@Inject
private ModelUrlNormalizer modelUrlNormalizer;
-
+
@Inject
private SuperPomProvider superPomProvider;
@@ -164,12 +171,13 @@ public class DefaultModelBuilder
private Provider<BuildPomXMLFilterFactory> buildPomXMLFilterFactory;
@Inject
- @Nullable
private ModelCacheManager modelCacheManager;
@Inject
@Nullable
private BuildPomXMLFilterListener xmlFilterListener;
+
+ private ModelMerger modelMerger = new FileToRawModelMerger();
public DefaultModelBuilder setModelProcessor( ModelProcessor modelProcessor )
{
@@ -278,6 +286,11 @@ public class DefaultModelBuilder
this.buildPomXMLFilterFactory = () -> buildPomXMLFilterFactory;
return this;
}
+
+ public void setModelCacheManager( ModelCacheManager modelCacheManager )
+ {
+ this.modelCacheManager = modelCacheManager;
+ }
@SuppressWarnings( "checkstyle:methodlength" )
@Override
@@ -451,7 +464,7 @@ public class DefaultModelBuilder
result.setEffectiveModel( resultModel );
- if ( modelCacheManager != null && request.getPomFile() != null )
+ if ( request.getPomFile() != null )
{
modelCacheManager.put( request.getPomFile().toPath(), resultModel );
}
@@ -664,27 +677,33 @@ public class DefaultModelBuilder
modelValidator.validateFileModel( model, request, problems );
request.setFileModel( model );
- modelValidator.validateRawModel( model, request, problems );
-
- if ( hasFatalErrors( problems ) )
- {
- throw problems.newModelBuildingException();
- }
-
- if ( !hasModelErrors( problems ) && Features.buildConsumer().isActive() && pomFile != null )
+ if ( Features.buildConsumer().isActive() && pomFile != null )
{
try
{
- model = modelProcessor.read( transformData( pomFile.toPath() ), null );
+ Model rawModel = modelProcessor.read( transformData( pomFile.toPath() ), null );
model.setPomFile( pomFile );
+
+ // model with locationTrackers, required for proper feedback during validations
+ model = request.getFileModel().clone();
+
+ // Apply enriched data
+ modelMerger.merge( model, rawModel, false, null );
}
catch ( IOException | TransformerException | SAXException | ParserConfigurationException e )
{
- problems.add( new ModelProblemCollectorRequest( Severity.FATAL, Version.V37 ).setException( e ) );
+ problems.add( new ModelProblemCollectorRequest( Severity.WARNING, Version.V37 ).setException( e ) );
}
}
+ modelValidator.validateRawModel( model, request, problems );
+
+ if ( hasFatalErrors( problems ) )
+ {
+ throw problems.newModelBuildingException();
+ }
+
return model;
}
@@ -1537,4 +1556,155 @@ public class DefaultModelBuilder
}
}
+ /**
+ * As long is Maven controls the BuildPomXMLFilter, the entities that need merging is known.
+ * All others can simply be copied from source to target to restore the locationTracker
+ *
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+ class FileToRawModelMerger extends ModelMerger
+ {
+ @Override
+ protected void mergeBuild_Extensions( Build target, Build source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+
+ @Override
+ protected void mergeBuildBase_Resources( BuildBase target, BuildBase source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeBuildBase_TestResources( BuildBase target, BuildBase source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeCiManagement_Notifiers( CiManagement target, CiManagement source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeDependencyManagement_Dependencies( DependencyManagement target, DependencyManagement source,
+ boolean sourceDominant, Map<Object, Object> context )
+ {
+ Iterator<Dependency> sourceIterator = source.getDependencies().iterator();
+ target.getDependencies().stream().forEach( t -> mergeDependency( t, sourceIterator.next(), sourceDominant,
+ context ) );
+ }
+
+ @Override
+ protected void mergeDependency_Exclusions( Dependency target, Dependency source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeModel_Contributors( Model target, Model source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeModel_Developers( Model target, Model source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeModel_Licenses( Model target, Model source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeModel_MailingLists( Model target, Model source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeModel_Profiles( Model target, Model source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ Iterator<Profile> sourceIterator = source.getProfiles().iterator();
+ target.getProfiles().stream().forEach( t -> mergeProfile( t, sourceIterator.next(), sourceDominant,
+ context ) );
+ }
+
+ @Override
+ protected void mergeModelBase_Dependencies( ModelBase target, ModelBase source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ Iterator<Dependency> sourceIterator = source.getDependencies().iterator();
+ target.getDependencies().stream().forEach( t -> mergeDependency( t, sourceIterator.next(), sourceDominant,
+ context ) );
+ }
+
+ @Override
+ protected void mergeModelBase_PluginRepositories( ModelBase target, ModelBase source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ target.setPluginRepositories( source.getPluginRepositories() );
+ }
+
+ @Override
+ protected void mergeModelBase_Repositories( ModelBase target, ModelBase source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergePlugin_Dependencies( Plugin target, Plugin source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ Iterator<Dependency> sourceIterator = source.getDependencies().iterator();
+ target.getDependencies().stream().forEach( t -> mergeDependency( t, sourceIterator.next(), sourceDominant,
+ context ) );
+ }
+
+ @Override
+ protected void mergePlugin_Executions( Plugin target, Plugin source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeReporting_Plugins( Reporting target, Reporting source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergeReportPlugin_ReportSets( ReportPlugin target, ReportPlugin source, boolean sourceDominant,
+ Map<Object, Object> context )
+ {
+ // don't merge
+ }
+
+ @Override
+ protected void mergePluginContainer_Plugins( PluginContainer target, PluginContainer source,
+ boolean sourceDominant, Map<Object, Object> context )
+ {
+ // don't merge
+ }
+ }
}
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilderFactory.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilderFactory.java
index cd499eb..82cd713 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilderFactory.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilderFactory.java
@@ -227,6 +227,7 @@ public class DefaultModelBuilderFactory
modelBuilder.setReportConfigurationExpander( newReportConfigurationExpander() );
modelBuilder.setReportingConverter( newReportingConverter() );
modelBuilder.setBuildPomXMLFilterFactory( new BuildPomXMLFilterFactory() );
+ modelBuilder.setModelCacheManager( new DefaultModelCacheManager() );
return modelBuilder;
}
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
index 1280a48..f789333 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
@@ -19,7 +19,6 @@ package org.apache.maven.model.validation;
* under the License.
*/
-import org.apache.maven.feature.Features;
import org.apache.maven.model.Activation;
import org.apache.maven.model.ActivationFile;
import org.apache.maven.model.Build;
@@ -118,6 +117,17 @@ public class DefaultModelValidator
if ( request.getValidationLevel() >= ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_2_0 )
{
+ Set<String> modules = new HashSet<>();
+ for ( int i = 0, n = m.getModules().size(); i < n; i++ )
+ {
+ String module = m.getModules().get( i );
+ if ( !modules.add( module ) )
+ {
+ addViolation( problems, Severity.ERROR, Version.V20, "modules.module[" + i + "]", null,
+ "specifies duplicate child module " + module, m.getLocation( "modules" ) );
+ }
+ }
+
Severity errOn30 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0 );
// [MNG-6074] Maven should produce an error if no model version has been set in a POM file used to build an
@@ -227,18 +237,8 @@ public class DefaultModelValidator
if ( parent != null )
{
- InputLocationTracker locationTracker;
- if ( Features.buildConsumer().isActive() )
- {
- locationTracker = request.getFileModel().getParent();
- }
- else
- {
- locationTracker = parent;
- }
-
validateStringNotEmpty( "parent.version", problems, Severity.FATAL, Version.BASE, parent.getVersion(),
- locationTracker );
+ parent );
}
}
@@ -396,17 +396,6 @@ public class DefaultModelValidator
if ( request.getValidationLevel() >= ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_2_0 )
{
- Set<String> modules = new HashSet<>();
- for ( int i = 0, n = m.getModules().size(); i < n; i++ )
- {
- String module = m.getModules().get( i );
- if ( !modules.add( module ) )
- {
- addViolation( problems, Severity.ERROR, Version.V20, "modules.module[" + i + "]", null,
- "specifies duplicate child module " + module, m.getLocation( "modules" ) );
- }
- }
-
Severity errOn31 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 );
validateBannedCharacters( EMPTY, "version", problems, errOn31, Version.V20, m.getVersion(), null, m,
diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java
new file mode 100644
index 0000000..485dc4c
--- /dev/null
+++ b/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java
@@ -0,0 +1,82 @@
+package org.apache.maven.model.building;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import static org.hamcrest.Matchers.hasItems;
+import static org.junit.Assert.assertThat;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.model.building.DefaultModelBuilder.FileToRawModelMerger;
+import org.apache.maven.model.merge.ModelMerger;
+import org.junit.Test;
+
+public class FileToRawModelMergerTest
+{
+
+ /**
+ * Ensures that all list-merge methods are overridden
+ */
+ @Test
+ public void testOverriddenMergeMethods()
+ {
+ List<String> methodNames =
+ Stream.of( ModelMerger.class.getDeclaredMethods() )
+ .filter( m -> m.getName().startsWith( "merge" ) )
+ .filter( m ->
+ {
+ String baseName = m.getName().substring( 5 /* merge */ );
+ String entity = baseName.substring( baseName.indexOf( '_' ) + 1 );
+ try
+ {
+ Type returnType = m.getParameterTypes()[0].getMethod( "get" + entity ).getGenericReturnType();
+ if ( returnType instanceof ParameterizedType )
+ {
+ return !( (ParameterizedType) returnType ).getActualTypeArguments()[0].equals( String.class );
+ }
+ else
+ {
+ return false;
+ }
+ }
+ catch ( ReflectiveOperationException | SecurityException e )
+ {
+ return false;
+ }
+ } )
+ .map( Method::getName )
+ .collect( Collectors.toList() );
+
+ List<String> overriddenMethods =
+ Stream.of( FileToRawModelMerger.class.getDeclaredMethods() )
+ .map( Method::getName )
+ .filter( m -> m.startsWith( "merge" ) )
+ .collect( Collectors.toList() );
+
+ assertThat( overriddenMethods, hasItems( methodNames.toArray( new String[0] ) ) );
+ }
+
+
+}
diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
index b90fcc6..d2a9e60 100644
--- a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
+++ b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
@@ -376,7 +376,7 @@ public class DefaultModelValidatorTest
public void testDuplicateModule()
throws Exception
{
- SimpleProblemCollector result = validate( "duplicate-module.xml" );
+ SimpleProblemCollector result = validateRaw( "duplicate-module.xml" );
assertViolations( result, 0, 1, 0 );