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/22 14:58:41 UTC

[maven] branch MNG-6824 updated: [MNG-6824] ModelMerger is broken Fixing reportSet.reports, mailingList.otherArchives, contributor.roles, build.filters, execution.goals, patternSet.includes, patternSet.excludes

This is an automated email from the ASF dual-hosted git repository.

rfscholte pushed a commit to branch MNG-6824
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/MNG-6824 by this push:
     new eab6b37  [MNG-6824] ModelMerger is broken Fixing reportSet.reports, mailingList.otherArchives, contributor.roles, build.filters, execution.goals, patternSet.includes, patternSet.excludes
eab6b37 is described below

commit eab6b37e91ef366f6250c2ec901ba713e4886bb3
Author: rfscholte <rf...@apache.org>
AuthorDate: Sun Dec 22 15:58:31 2019 +0100

    [MNG-6824] ModelMerger is broken
    Fixing reportSet.reports, mailingList.otherArchives, contributor.roles, build.filters, execution.goals, patternSet.includes, patternSet.excludes
---
 .../org/apache/maven/model/merge/ModelMerger.java  |  88 ++----------------
 .../apache/maven/model/merge/ModelMergerTest.java  | 102 ++++++++++++++++++++-
 2 files changed, 106 insertions(+), 84 deletions(-)

diff --git a/maven-model/src/main/java/org/apache/maven/model/merge/ModelMerger.java b/maven-model/src/main/java/org/apache/maven/model/merge/ModelMerger.java
index 4b687c9..d115e6e 100644
--- a/maven-model/src/main/java/org/apache/maven/model/merge/ModelMerger.java
+++ b/maven-model/src/main/java/org/apache/maven/model/merge/ModelMerger.java
@@ -1232,33 +1232,7 @@ public class ModelMerger
     protected void mergeReportSet_Reports( ReportSet target, ReportSet source, boolean sourceDominant,
                                            Map<Object, Object> context )
     {
-        List<String> src = source.getReports();
-        if ( !src.isEmpty() )
-        {
-            List<String> tgt = target.getReports();
-            List<String> merged = new ArrayList<>( tgt.size() + src.size() );
-            merged.addAll( tgt );
-            merged.addAll( src );
-            target.setReports( merged );
-
-            InputLocation sourceLocation = source.getLocation( "reports" );
-            if ( sourceLocation != null )
-            {
-                InputLocation targetLocation = target.getLocation( "reports" );
-                if ( targetLocation == null )
-                {
-                    target.setLocation( "reports", sourceLocation );
-                }
-                else
-                {
-                    for ( int i = 0; i < src.size(); i++ )
-                    {
-                        targetLocation.setLocation( Integer.valueOf( tgt.size() + i ),
-                                                    sourceLocation.getLocation( Integer.valueOf( i ) ) );
-                    }
-                }
-            }
-        }
+        target.setReports( merge( target.getReports(), source.getReports(), sourceDominant, e -> e ) );
     }
 
     protected void mergeDependencyManagement( DependencyManagement target, DependencyManagement source,
@@ -1520,15 +1494,7 @@ public class ModelMerger
     protected void mergeMailingList_OtherArchives( MailingList target, MailingList source, boolean sourceDominant,
                                                    Map<Object, Object> context )
     {
-        List<String> src = source.getOtherArchives();
-        if ( !src.isEmpty() )
-        {
-            List<String> tgt = target.getOtherArchives();
-            List<String> merged = new ArrayList<>( tgt.size() + src.size() );
-            merged.addAll( tgt );
-            merged.addAll( src );
-            target.setOtherArchives( merged );
-        }
+        target.setOtherArchives( merge( target.getOtherArchives(), source.getOtherArchives(), sourceDominant, e -> e ) );
     }
 
     protected void mergeDeveloper( Developer target, Developer source, boolean sourceDominant,
@@ -1652,15 +1618,7 @@ public class ModelMerger
     protected void mergeContributor_Roles( Contributor target, Contributor source, boolean sourceDominant,
                                            Map<Object, Object> context )
     {
-        List<String> src = source.getRoles();
-        if ( !src.isEmpty() )
-        {
-            List<String> tgt = target.getRoles();
-            List<String> merged = new ArrayList<>( tgt.size() + src.size() );
-            merged.addAll( tgt );
-            merged.addAll( src );
-            target.setRoles( merged );
-        }
+        target.setRoles( merge( target.getRoles(), source.getRoles(), sourceDominant, e -> e ) );
     }
 
     protected void mergeContributor_Properties( Contributor target, Contributor source, boolean sourceDominant,
@@ -2176,15 +2134,7 @@ public class ModelMerger
     protected void mergeBuildBase_Filters( BuildBase target, BuildBase source, boolean sourceDominant,
                                            Map<Object, Object> context )
     {
-        List<String> src = source.getFilters();
-        if ( !src.isEmpty() )
-        {
-            List<String> tgt = target.getFilters();
-            List<String> merged = new ArrayList<>( tgt.size() + src.size() );
-            merged.addAll( tgt );
-            merged.addAll( src );
-            target.setFilters( merged );
-        }
+        target.setFilters( merge( target.getFilters(), source.getFilters(), sourceDominant, e -> e ) );
     }
 
     protected void mergeBuildBase_Resources( BuildBase target, BuildBase source, boolean sourceDominant,
@@ -2405,15 +2355,7 @@ public class ModelMerger
     protected void mergePluginExecution_Goals( PluginExecution target, PluginExecution source, boolean sourceDominant,
                                                Map<Object, Object> context )
     {
-        List<String> src = source.getGoals();
-        if ( !src.isEmpty() )
-        {
-            List<String> tgt = target.getGoals();
-            List<String> merged = new ArrayList<>( tgt.size() + src.size() );
-            merged.addAll( tgt );
-            merged.addAll( src );
-            target.setGoals( merged );
-        }
+        target.setGoals( merge( target.getGoals(), source.getGoals(), sourceDominant, e -> e ) );
     }
 
     protected void mergeResource( Resource target, Resource source, boolean sourceDominant,
@@ -2496,29 +2438,13 @@ public class ModelMerger
     protected void mergePatternSet_Includes( PatternSet target, PatternSet source, boolean sourceDominant,
                                              Map<Object, Object> context )
     {
-        List<String> src = source.getIncludes();
-        if ( !src.isEmpty() )
-        {
-            List<String> tgt = target.getIncludes();
-            List<String> merged = new ArrayList<>( tgt.size() + src.size() );
-            merged.addAll( tgt );
-            merged.addAll( src );
-            target.setIncludes( merged );
-        }
+        target.setIncludes( merge( target.getIncludes(), source.getIncludes(), sourceDominant, e -> e ) );
     }
 
     protected void mergePatternSet_Excludes( PatternSet target, PatternSet source, boolean sourceDominant,
                                              Map<Object, Object> context )
     {
-        List<String> src = source.getExcludes();
-        if ( !src.isEmpty() )
-        {
-            List<String> tgt = target.getExcludes();
-            List<String> merged = new ArrayList<>( tgt.size() + src.size() );
-            merged.addAll( tgt );
-            merged.addAll( src );
-            target.setExcludes( merged );
-        }
+        target.setExcludes( merge( target.getExcludes(), source.getExcludes(), sourceDominant, e -> e ) );
     }
 
     protected void mergeProfile( Profile target, Profile source, boolean sourceDominant, Map<Object, Object> context )
diff --git a/maven-model/src/test/java/org/apache/maven/model/merge/ModelMergerTest.java b/maven-model/src/test/java/org/apache/maven/model/merge/ModelMergerTest.java
index ced5537..2c8eb59 100644
--- a/maven-model/src/test/java/org/apache/maven/model/merge/ModelMergerTest.java
+++ b/maven-model/src/test/java/org/apache/maven/model/merge/ModelMergerTest.java
@@ -1,5 +1,8 @@
 package org.apache.maven.model.merge;
 
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.is;
+
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
@@ -20,17 +23,19 @@ package org.apache.maven.model.merge;
  */
 
 import static org.junit.Assert.assertThat;
-import static org.hamcrest.Matchers.contains;
-import static org.hamcrest.Matchers.is;
 
 import java.util.Arrays;
 
+import org.apache.maven.model.Build;
 import org.apache.maven.model.Contributor;
 import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Developer;
 import org.apache.maven.model.MailingList;
 import org.apache.maven.model.Model;
+import org.apache.maven.model.PatternSet;
+import org.apache.maven.model.PluginExecution;
 import org.apache.maven.model.Profile;
+import org.apache.maven.model.ReportSet;
 import org.apache.maven.model.Repository;
 import org.junit.Test;
 
@@ -132,6 +137,45 @@ public class ModelMergerTest
     }
 
     @Test
+    public void mergeSameExcludes()
+    {
+        PatternSet target = new PatternSet();
+        target.setExcludes( Arrays.asList( "first", "second", "third" ) );
+        PatternSet source = new PatternSet();
+        source.setExcludes( Arrays.asList( "first", "second", "third" ) );
+
+        modelMerger.mergePatternSet_Excludes( target, source, true, null );
+
+        assertThat( target.getExcludes(), contains( "first", "second", "third" ) );
+    }
+
+    @Test
+    public void mergeSameFilters()
+    {
+        Build target = new Build();
+        target.setFilters( Arrays.asList( "first", "second", "third" ) );
+        Build source = new Build();
+        source.setFilters( Arrays.asList( "first", "second", "third" ) );
+
+        modelMerger.mergeBuild( target, source, true, null );
+
+        assertThat( target.getFilters(), contains( "first", "second", "third" ) );
+    }
+
+    @Test
+    public void mergeSameGoals()
+    {
+        PluginExecution target = new PluginExecution();
+        target.setGoals( Arrays.asList( "first", "second", "third" ) );
+        PluginExecution source = new PluginExecution();
+        source.setGoals( Arrays.asList( "first", "second", "third" ) );
+
+        modelMerger.mergePluginExecution( target, source, true, null );
+
+        assertThat( target.getGoals(), contains( "first", "second", "third" ) );
+    }
+
+    @Test
     public void mergeGroupId()
     {
         Model target = new Model();
@@ -164,7 +208,20 @@ public class ModelMergerTest
         modelMerger.merge( target, source, false, null );
         assertThat( target.getInceptionYear(), is( "TARGET" ) );
     }
-    
+
+    @Test
+    public void mergeSameIncludes()
+    {
+        PatternSet target = new PatternSet();
+        target.setIncludes( Arrays.asList( "first", "second", "third" ) );
+        PatternSet source = new PatternSet();
+        source.setIncludes( Arrays.asList( "first", "second", "third" ) );
+
+        modelMerger.mergePatternSet_Includes( target, source, true, null );
+
+        assertThat( target.getIncludes(), contains( "first", "second", "third" ) );
+    }
+
     @Test
     public void mergeSameMailingLists()
     {
@@ -230,6 +287,19 @@ public class ModelMergerTest
     }
 
     @Test
+    public void mergeSameOtherArchives()
+    {
+        MailingList target = new MailingList();
+        target.setOtherArchives( Arrays.asList( "first", "second", "third" ) );
+        MailingList source = new MailingList();
+        source.setOtherArchives( Arrays.asList( "first", "second", "third" ) );
+
+        modelMerger.mergeMailingList( target, source, true, null );
+
+        assertThat( target.getOtherArchives(), contains( "first", "second", "third" ) );
+    }
+
+    @Test
     public void mergePackaging()
     {
         Model target = new Model();
@@ -281,6 +351,19 @@ public class ModelMergerTest
     }
 
     @Test
+    public void mergeSameReports()
+    {
+        ReportSet target = new ReportSet();
+        target.setReports( Arrays.asList( "first", "second", "third" ) );
+        ReportSet source = new ReportSet();
+        source.setReports( Arrays.asList( "first", "second", "third" ) );
+
+        modelMerger.mergeReportSet( target, source, true, null );
+
+        assertThat( target.getReports(), contains( "first", "second", "third" ) );
+    }
+
+    @Test
     public void mergeSameRepositories()
     {
         Repository repository = new Repository();
@@ -298,6 +381,19 @@ public class ModelMergerTest
     }
 
     @Test
+    public void mergeSameRoles()
+    {
+        Contributor target = new Contributor();
+        target.setRoles( Arrays.asList( "first", "second", "third" ) );
+        Contributor source = new Contributor();
+        source.setRoles( Arrays.asList( "first", "second", "third" ) );
+
+        modelMerger.mergeContributor_Roles( target, source, true, null );
+
+        assertThat( target.getRoles(), contains( "first", "second", "third" ) );
+    }
+
+    @Test
     public void mergeUrl()
     {
         Model target = new Model();