From e3aa406cc79a200dc0cbf58e52839d13c4074826 Mon Sep 17 00:00:00 2001 From: rfscholte Date: Sun, 22 Dec 2019 17:28:35 +0100 Subject: [PATCH] [MNG-6824] ModelMerger is broken Fixing reportSet.reports, mailingList.otherArchives, contributor.roles, build.filters, execution.goals, patternSet.includes, patternSet.excludes --- .../apache/maven/model/merge/ModelMerger.java | 91 ++-------------- .../maven/model/merge/ModelMergerTest.java | 102 +++++++++++++++++- 2 files changed, 109 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 4b687c9e23..956f67a796 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 @@ protected void mergeReportSet_Id( ReportSet target, ReportSet source, boolean so protected void mergeReportSet_Reports( ReportSet target, ReportSet source, boolean sourceDominant, Map context ) { - List src = source.getReports(); - if ( !src.isEmpty() ) - { - List tgt = target.getReports(); - List 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,10 @@ protected void mergeMailingList_Archive( MailingList target, MailingList source, protected void mergeMailingList_OtherArchives( MailingList target, MailingList source, boolean sourceDominant, Map context ) { - List src = source.getOtherArchives(); - if ( !src.isEmpty() ) - { - List tgt = target.getOtherArchives(); - List 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 +1621,7 @@ protected void mergeContributor_Timezone( Contributor target, Contributor source protected void mergeContributor_Roles( Contributor target, Contributor source, boolean sourceDominant, Map context ) { - List src = source.getRoles(); - if ( !src.isEmpty() ) - { - List tgt = target.getRoles(); - List 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 +2137,7 @@ protected void mergeBuildBase_FinalName( BuildBase target, BuildBase source, boo protected void mergeBuildBase_Filters( BuildBase target, BuildBase source, boolean sourceDominant, Map context ) { - List src = source.getFilters(); - if ( !src.isEmpty() ) - { - List tgt = target.getFilters(); - List 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 +2358,7 @@ protected void mergePluginExecution_Phase( PluginExecution target, PluginExecuti protected void mergePluginExecution_Goals( PluginExecution target, PluginExecution source, boolean sourceDominant, Map context ) { - List src = source.getGoals(); - if ( !src.isEmpty() ) - { - List tgt = target.getGoals(); - List 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 +2441,13 @@ protected void mergePatternSet( PatternSet target, PatternSet source, boolean so protected void mergePatternSet_Includes( PatternSet target, PatternSet source, boolean sourceDominant, Map context ) { - List src = source.getIncludes(); - if ( !src.isEmpty() ) - { - List tgt = target.getIncludes(); - List 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 context ) { - List src = source.getExcludes(); - if ( !src.isEmpty() ) - { - List tgt = target.getExcludes(); - List 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 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 ced553744f..2c8eb59498 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 @@ */ 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; @@ -131,6 +136,45 @@ public void mergeSameDevelopers() assertThat( target.getDevelopers(), contains( developer ) ); } + @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() { @@ -164,7 +208,20 @@ public void mergeInceptionYear() 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() { @@ -229,6 +286,19 @@ public void mergeName() assertThat( target.getName(), is( "TARGET" ) ); } + @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() { @@ -280,6 +350,19 @@ public void mergeSameProfiles() assertThat( target.getProfiles(), contains( profile ) ); } + @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() { @@ -297,6 +380,19 @@ public void mergeSameRepositories() assertThat( target.getRepositories(), contains( repository ) ); } + @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() {