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:39 UTC

[maven] 03/06: [MNG-6656] Improve ModelValidator

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 d9cc0e475952bc88f1187e00a0f0ff142f4c6f2a
Author: rfscholte <rf...@apache.org>
AuthorDate: Wed Dec 18 23:00:34 2019 +0100

    [MNG-6656] Improve ModelValidator
---
 .../apache/maven/project/ProjectBuilderTest.java   |  2 ++
 .../maven/model/building/DefaultModelBuilder.java  |  4 +++
 .../building/DefaultModelBuildingRequest.java      | 14 ++++++++++
 .../model/building/FilterModelBuildingRequest.java | 13 +++++++++
 .../maven/model/building/ModelBuildingRequest.java | 15 +++++++++++
 .../model/validation/DefaultModelValidator.java    | 31 ++++++++++++++++------
 .../maven/model/validation/ModelValidator.java     | 16 +++++++++--
 .../validation/DefaultModelValidatorTest.java      | 31 +++++++++++-----------
 8 files changed, 101 insertions(+), 25 deletions(-)

diff --git a/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java b/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
index efa8a4c..d402f96 100644
--- a/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
+++ b/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
@@ -240,6 +240,7 @@ public class ProjectBuilderTest
         try
         {
             projectBuilder.build( pomFile, configuration );
+            fail();
         }
         catch ( InvalidArtifactRTException iarte )
         {
@@ -250,6 +251,7 @@ public class ProjectBuilderTest
         try
         {
             projectBuilder.build( Collections.singletonList( pomFile ), false, configuration );
+            fail();
         }
         catch ( ProjectBuildingException ex )
         {
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 8a46aed..5bfe617 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
@@ -660,6 +660,10 @@ public class DefaultModelBuilder
         model.setPomFile( pomFile );
 
         problems.setSource( model );
+
+        modelValidator.validateFileModel( model, request, problems );
+        request.setFileModel( model );
+        
         modelValidator.validateRawModel( model, request, problems );
 
         if ( hasFatalErrors( problems ) )
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java
index b7b54d8..3cf3a89 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java
@@ -38,6 +38,7 @@ import org.apache.maven.model.resolution.WorkspaceModelResolver;
 public class DefaultModelBuildingRequest
     implements ModelBuildingRequest
 {
+    private transient Model fileModel;
 
     private Model rawModel;
 
@@ -383,6 +384,19 @@ public class DefaultModelBuildingRequest
     }
 
     @Override
+    public Model getFileModel()
+    {
+        return fileModel;
+    }
+    
+    @Override
+    public ModelBuildingRequest setFileModel( Model fileModel )
+    {
+        this.fileModel = fileModel;
+        return this;
+    }
+    
+    @Override
     public Model getRawModel()
     {
         return rawModel;
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java
index 527a257..fb38f16 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/FilterModelBuildingRequest.java
@@ -257,6 +257,19 @@ class FilterModelBuildingRequest
     }
 
     @Override
+    public Model getFileModel()
+    {
+        return request.getFileModel();
+    }
+    
+    @Override
+    public ModelBuildingRequest setFileModel( Model fileModel )
+    {
+        request.setFileModel( fileModel );
+        return this;
+    }
+    
+    @Override
     public Model getRawModel()
     {
         return request.getRawModel();
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java
index b8ae4f8..1e0b3da 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java
@@ -63,6 +63,21 @@ public interface ModelBuildingRequest
      * Denotes strict validation as recommended by the current Maven version.
      */
     int VALIDATION_LEVEL_STRICT = VALIDATION_LEVEL_MAVEN_3_0;
+    
+    /**
+     * 
+     * @return the file model
+     * @since 3.7.0
+     */
+    Model getFileModel();
+    
+    /**
+     * 
+     * @param fileModel
+     * @return This request, never {@code null}.
+     * @since 3.7.0
+     */
+    ModelBuildingRequest setFileModel( Model fileModel );
 
     /**
      * Gets the raw model to build. If not set, model source will be used to load raw model.
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 86670eb..862c8ec 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
@@ -88,7 +88,7 @@ public class DefaultModelValidator
     private final Set<String> validIds = new HashSet<>();
 
     @Override
-    public void validateRawModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems )
+    public void validateFileModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems )
     {
         Parent parent = m.getParent();
         if ( parent != null )
@@ -99,13 +99,6 @@ public class DefaultModelValidator
             validateStringNotEmpty( "parent.artifactId", problems, Severity.FATAL, Version.BASE, parent.getArtifactId(),
                                     parent );
 
-            // resolvedModel will assign version based on relativePath
-            if ( !Features.buildConsumer().isActive() )
-            {
-                validateStringNotEmpty( "parent.version", problems, Severity.FATAL, Version.BASE, parent.getVersion(),
-                                        parent );
-            }
-
             if ( equals( parent.getGroupId(), m.getGroupId() ) && equals( parent.getArtifactId(), m.getArtifactId() ) )
             {
                 addViolation( problems, Severity.FATAL, Version.BASE, "parent.artifactId", null,
@@ -225,6 +218,28 @@ public class DefaultModelValidator
             }
         }
     }
+    
+    @Override
+    public void validateRawModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems )
+    {
+        Parent parent = m.getParent();
+        
+        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 );
+        }
+    }
 
     private void validate30RawProfileActivation( ModelProblemCollector problems, Activation activation,
                                                  String sourceHint, String prefix, String fieldName,
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java
index 84e3fad..198ba5a 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java
@@ -30,15 +30,27 @@ import org.apache.maven.model.building.ModelProblemCollector;
  */
 public interface ModelValidator
 {
-
     /**
-     * Checks the specified (raw) model for missing or invalid values. The raw model is directly created from the POM
+     * Checks the specified file model for missing or invalid values. This model is directly created from the POM
      * file and has not been subjected to inheritance, interpolation or profile/default injection.
      *
      * @param model The model to validate, must not be {@code null}.
      * @param request The model building request that holds further settings, must not be {@code null}.
      * @param problems The container used to collect problems that were encountered, must not be {@code null}.
      */
+    default void validateFileModel( Model model, ModelBuildingRequest request, ModelProblemCollector problems )
+    {
+        // do nothing
+    }
+
+    /**
+     * Checks the specified (raw) model for missing or invalid values. The raw model is the file model + buildpom filter
+     * transformation and has not been subjected to inheritance, interpolation or profile/default injection.
+     *
+     * @param model The model to validate, must not be {@code null}.
+     * @param request The model building request that holds further settings, must not be {@code null}.
+     * @param problems The container used to collect problems that were encountered, must not be {@code null}.
+     */
     void validateRawModel( Model model, ModelBuildingRequest request, ModelProblemCollector problems );
 
     /**
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 bf6b088..b90fcc6 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
@@ -22,7 +22,6 @@ package org.apache.maven.model.validation;
 import java.io.InputStream;
 import java.util.List;
 
-import org.apache.maven.feature.Features;
 import org.apache.maven.model.Model;
 import org.apache.maven.model.building.DefaultModelBuildingRequest;
 import org.apache.maven.model.building.ModelBuildingRequest;
@@ -65,10 +64,14 @@ public class DefaultModelValidatorTest
         throws Exception
     {
         ModelBuildingRequest request = new DefaultModelBuildingRequest().setValidationLevel( level );
+        
+        Model model =  read( pom );
 
-        SimpleProblemCollector problems = new SimpleProblemCollector( read( pom ) );
+        SimpleProblemCollector problems = new SimpleProblemCollector( model );
+        
+        request.setFileModel( model );
 
-        validator.validateEffectiveModel( problems.getModel(), request, problems );
+        validator.validateEffectiveModel( model, request, problems );
 
         return problems;
     }
@@ -78,9 +81,15 @@ public class DefaultModelValidatorTest
     {
         ModelBuildingRequest request = new DefaultModelBuildingRequest().setValidationLevel( level );
 
-        SimpleProblemCollector problems = new SimpleProblemCollector( read( pom ) );
+        Model model = read( pom );
+        
+        SimpleProblemCollector problems = new SimpleProblemCollector( model );
 
-        validator.validateRawModel( problems.getModel(), request, problems );
+        validator.validateFileModel( model, request, problems );
+        
+        request.setFileModel( model );
+        
+        validator.validateRawModel( model, request, problems );
 
         return problems;
     }
@@ -416,18 +425,10 @@ public class DefaultModelValidatorTest
     {
         SimpleProblemCollector result = validateRaw( "incomplete-parent.xml" );
 
-        if ( Features.buildConsumer().isActive() )
-        {
-            assertViolations( result, 2, 0, 0 );
-        }
-        else
-        {
-            assertViolations( result, 3, 0, 0 );
-            assertTrue( result.getFatals().get( 2 ).contains( "parent.version" ) );
-        }
-
+        assertViolations( result, 3, 0, 0 );
         assertTrue( result.getFatals().get( 0 ).contains( "parent.groupId" ) );
         assertTrue( result.getFatals().get( 1 ).contains( "parent.artifactId" ) );
+        assertTrue( result.getFatals().get( 2 ).contains( "parent.version" ) );
     }
 
     public void testHardCodedSystemPath()