From 9d851eea684ae4395cdacb5612cb23723c66240b Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 13 Mar 2023 19:35:35 +0100 Subject: [PATCH] [MNG-7678] Settings (v3) and Settings.Builder() cannot unset a non-null field (#983) --- .../maven/api/model/ImmutableCollections.java | 6 +- .../api/settings/ImmutableCollections.java | 6 +- .../maven/api/settings/SettingsTest.java | 41 +++++++++++ .../api/toolchain/ImmutableCollections.java | 6 +- .../lifecycle/ImmutableCollections.java | 6 +- src/mdo/model.vm | 71 +++---------------- 6 files changed, 72 insertions(+), 64 deletions(-) create mode 100644 api/maven-api-settings/src/test/java/org/apache/maven/api/settings/SettingsTest.java diff --git a/api/maven-api-model/src/main/java/org/apache/maven/api/model/ImmutableCollections.java b/api/maven-api-model/src/main/java/org/apache/maven/api/model/ImmutableCollections.java index 6c70c7400d..2da8c291d9 100644 --- a/api/maven-api-model/src/main/java/org/apache/maven/api/model/ImmutableCollections.java +++ b/api/maven-api-model/src/main/java/org/apache/maven/api/model/ImmutableCollections.java @@ -485,7 +485,11 @@ class ImmutableCollections { private final Object[] entries; private MapN(Map map) { - entries = map != null ? map.entrySet().toArray() : new Object[0]; + entries = map != null + ? map.entrySet().stream() + .map(e -> new SimpleImmutableEntry<>(e.getKey(), e.getValue())) + .toArray() + : new Object[0]; } @Override diff --git a/api/maven-api-settings/src/main/java/org/apache/maven/api/settings/ImmutableCollections.java b/api/maven-api-settings/src/main/java/org/apache/maven/api/settings/ImmutableCollections.java index 78d5f2de67..cf6de272d8 100644 --- a/api/maven-api-settings/src/main/java/org/apache/maven/api/settings/ImmutableCollections.java +++ b/api/maven-api-settings/src/main/java/org/apache/maven/api/settings/ImmutableCollections.java @@ -485,7 +485,11 @@ class ImmutableCollections { private final Object[] entries; private MapN(Map map) { - entries = map != null ? map.entrySet().toArray() : new Object[0]; + entries = map != null + ? map.entrySet().stream() + .map(e -> new SimpleImmutableEntry<>(e.getKey(), e.getValue())) + .toArray() + : new Object[0]; } @Override diff --git a/api/maven-api-settings/src/test/java/org/apache/maven/api/settings/SettingsTest.java b/api/maven-api-settings/src/test/java/org/apache/maven/api/settings/SettingsTest.java new file mode 100644 index 0000000000..2f525094c7 --- /dev/null +++ b/api/maven-api-settings/src/test/java/org/apache/maven/api/settings/SettingsTest.java @@ -0,0 +1,41 @@ +/* + * 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. + */ +package org.apache.maven.api.settings; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +public class SettingsTest { + + @Test + void testSetLocalRepository() { + Settings s = Settings.newInstance(); + + s = s.withLocalRepository("xxx"); + assertEquals("xxx", s.getLocalRepository()); + + s = s.withLocalRepository("yyy"); + assertEquals("yyy", s.getLocalRepository()); + + s = s.withLocalRepository(null); + assertNull(s.getLocalRepository()); + } +} diff --git a/api/maven-api-toolchain/src/main/java/org/apache/maven/api/toolchain/ImmutableCollections.java b/api/maven-api-toolchain/src/main/java/org/apache/maven/api/toolchain/ImmutableCollections.java index 3f53f5d677..5fa99017e8 100644 --- a/api/maven-api-toolchain/src/main/java/org/apache/maven/api/toolchain/ImmutableCollections.java +++ b/api/maven-api-toolchain/src/main/java/org/apache/maven/api/toolchain/ImmutableCollections.java @@ -485,7 +485,11 @@ class ImmutableCollections { private final Object[] entries; private MapN(Map map) { - entries = map != null ? map.entrySet().toArray() : new Object[0]; + entries = map != null + ? map.entrySet().stream() + .map(e -> new SimpleImmutableEntry<>(e.getKey(), e.getValue())) + .toArray() + : new Object[0]; } @Override diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/lifecycle/ImmutableCollections.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/lifecycle/ImmutableCollections.java index e113d89c7e..405edc3f02 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/lifecycle/ImmutableCollections.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/lifecycle/ImmutableCollections.java @@ -485,7 +485,11 @@ class ImmutableCollections { private final Object[] entries; private MapN(Map map) { - entries = map != null ? map.entrySet().toArray() : new Object[0]; + entries = map != null + ? map.entrySet().stream() + .map(e -> new SimpleImmutableEntry<>(e.getKey(), e.getValue())) + .toArray() + : new Object[0]; } @Override diff --git a/src/mdo/model.vm b/src/mdo/model.vm index 54341e3d6a..c28fe29c41 100644 --- a/src/mdo/model.vm +++ b/src/mdo/model.vm @@ -125,15 +125,7 @@ public class ${class.name} #end #if ( $locationTracking ) #if ( ! $class.superClass ) - /** Location of the xml element for this object. */ - final InputLocation location; - #end - #foreach ( $field in $class.getFields($version) ) - /** Location of the xml element for the field ${field.name}. */ - final InputLocation ${field.name}Location; - #end - #if ( ! $class.superClass ) - /** Other locations */ + /** Locations */ final Map locations; #end #end @@ -155,13 +147,7 @@ public class ${class.name} $type $field.name${sep} #end #if ( $locationTracking ) - Map locations, - #set ( $sep = "#if(${allFields.size()}>0),#end" ) - InputLocation location${sep} - #foreach ( $field in $allFields ) - #set ( $sep = "#if(${locationTracking}&&$field!=${allFields[${allFields.size()} - 1]}),#end" ) - InputLocation ${field.name}Location${sep} - #end + Map locations #end ) { @@ -172,13 +158,7 @@ public class ${class.name} ${field.name}${sep} #end #if ( $locationTracking ) - locations, - #set ( $sep = "#if(${inheritedFields.size()}>0),#end" ) - location${sep} - #foreach ( $field in $inheritedFields ) - #set ( $sep = "#if(${locationTracking}&&$field!=${inheritedFields[${inheritedFields.size()} - 1]}),#end" ) - ${field.name}Location${sep} - #end + locations #end ); #end @@ -195,10 +175,6 @@ public class ${class.name} #if ( $locationTracking ) #if ( ! $class.superClass ) this.locations = ImmutableCollections.copy( locations ); - this.location = location; - #end - #foreach ( $field in $class.getFields($version) ) - this.${field.name}Location = ${field.name}Location; #end #end } @@ -258,31 +234,13 @@ public class ${class.name} } #end - #if ( $locationTracking ) + #if ( $locationTracking && !$class.superClass ) /** * Gets the location of the specified field in the input source. */ public InputLocation getLocation( Object key ) { - if ( key instanceof String ) - { - switch ( ( String ) key ) - { - #if ( ! $class.superClass ) - case "": - return location; - #end - #foreach ( $field in $class.getFields($version) ) - case "${field.name}": - return ${field.name}Location; - #end - } - } - #if ( $class.superClass ) - return super.getLocation( key ); - #else return locations != null ? locations.get( key ) : null; - #end } #end @@ -311,7 +269,7 @@ public class ${class.name} @Nonnull public ${class.name} with${cap}( $type $field.name ) { - return with().${field.name}( $field.name ).build(); + return newBuilder(this, true).${field.name}( $field.name ).build(); } #end @@ -453,6 +411,9 @@ public class ${class.name} { #foreach ( $field in $class.getFields($version) ) this.${field.name} = base.${field.name}; + #end + #if ( $locationTracking ) + this.locations = base.locations; #end } else @@ -490,9 +451,9 @@ public class ${class.name} { if ( location != null ) { - if ( this.locations == null ) + if ( !(this.locations instanceof HashMap) ) { - this.locations = new HashMap<>(); + this.locations = this.locations != null ? new HashMap<>( this.locations ) : new HashMap<>(); } this.locations.put( key, location ); } @@ -520,10 +481,6 @@ public class ${class.name} if ( this.locations != null ) { locations = this.locations; - location = locations.remove( "" ); - #foreach ( $field in $allFields ) - ${field.name}Location = locations.remove( "${field.name}" ); - #end } #end return new ${class.name}( @@ -539,13 +496,7 @@ public class ${class.name} #end #end #if ( $locationTracking ) - locations != null ? locations : ( base != null ? base.locations : null ), - #set ( $sep = "#if(${allFields.size()}>0),#end" ) - location != null ? location : ( base != null ? base.location : null )${sep} - #foreach ( $field in $allFields ) - #set ( $sep = "#if(${locationTracking}&&$field!=${allFields[${allFields.size()} - 1]}),#end" ) - ${field.name}Location != null ? ${field.name}Location : ( base != null ? base.${field.name}Location : null )${sep} - #end + locations != null ? locations : ( base != null ? base.locations : null ) #end ); }