diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java index cdcb385a7b5d..118ba1ac1dbd 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java @@ -45,4 +45,21 @@ Model importManagement( List sources, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #importManagement}. + * + * @since 4.0.0 + */ + default void importManagement( + Model.Builder builder, + List sources, + ModelBuilderRequest request, + ModelProblemCollector problems) { + Model built = builder.build(); + Model result = importManagement(built, sources, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java index fb91b232cf60..fc29dc218e58 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java @@ -38,4 +38,24 @@ public interface DependencyManagementInjector { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant that operates on a {@link Model.Builder} directly, + * avoiding an intermediate {@code Model.build()} between pipeline stages. + *

+ * The default implementation bridges to {@link #injectManagement(Model, ModelBuilderRequest, ModelProblemCollector)} + * by building the model, processing it, and resetting the builder to the result. + * + * @param builder The model builder to modify in place, must not be {@code null}. + * @param request The model building request, must not be {@code null}. + * @param problems The container used to collect problems, must not be {@code null}. + * @since 4.0.0 + */ + default void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectManagement(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java index 59b24f370d04..3fdb1d3a6e81 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java @@ -42,4 +42,18 @@ public interface InheritanceAssembler { */ Model assembleModelInheritance( Model child, Model parent, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant that merges parent values into the child builder directly. + * + * @since 4.0.0 + */ + default void assembleModelInheritance( + Model.Builder childBuilder, Model parent, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = childBuilder.build(); + Model result = assembleModelInheritance(built, parent, request, problems); + if (result != built) { + childBuilder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java index c24a8a4d7f1e..4d678b21b4d3 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java @@ -51,4 +51,21 @@ Model interpolateModel( @Nullable Path projectDir, @Nonnull ModelBuilderRequest request, @Nonnull ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #interpolateModel}. + * + * @since 4.0.0 + */ + default void interpolateModel( + @Nonnull Model.Builder builder, + @Nullable Path projectDir, + @Nonnull ModelBuilderRequest request, + @Nonnull ModelProblemCollector problems) { + Model built = builder.build(); + Model result = interpolateModel(built, projectDir, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java index 50225fd417a5..a8b19a4cd9cb 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java @@ -48,4 +48,31 @@ public interface ModelNormalizer { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model injectDefaultValues(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #mergeDuplicates}. + * + * @since 4.0.0 + */ + default void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = mergeDuplicates(built, request, problems); + if (result != built) { + builder.reset(result); + } + } + + /** + * Builder-accepting variant of {@link #injectDefaultValues}. + * + * @since 4.0.0 + */ + default void injectDefaultValues( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectDefaultValues(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java index c2ec4ce522ac..eee49ecf486e 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java @@ -40,4 +40,17 @@ public interface ModelPathTranslator { * @since 4.0.0 */ Model alignToBaseDirectory(Model model, Path basedir, ModelBuilderRequest request); + + /** + * Builder-accepting variant of {@link #alignToBaseDirectory}. + * + * @since 4.0.0 + */ + default void alignToBaseDirectory(Model.Builder builder, Path basedir, ModelBuilderRequest request) { + Model built = builder.build(); + Model result = alignToBaseDirectory(built, basedir, request); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java index a216b99d7a86..f89cf80e9c1c 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java @@ -36,4 +36,17 @@ public interface ModelUrlNormalizer { * @param request The model building request that holds further settings, must not be {@code null}. */ Model normalize(Model model, ModelBuilderRequest request); + + /** + * Builder-accepting variant of {@link #normalize}. + * + * @since 4.0.0 + */ + default void normalize(Model.Builder builder, ModelBuilderRequest request) { + Model built = builder.build(); + Model result = normalize(built, request); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java index bdd249489df0..a5f15cb111ea 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java @@ -37,4 +37,18 @@ public interface PluginConfigurationExpander { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model expandPluginConfiguration(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #expandPluginConfiguration}. + * + * @since 4.0.0 + */ + default void expandPluginConfiguration( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = expandPluginConfiguration(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java index 36d7f7e19f86..a2e39c63ca40 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java @@ -38,4 +38,17 @@ public interface PluginManagementInjector { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #injectManagement}. + * + * @since 4.0.0 + */ + default void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectManagement(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java index 7361318f9801..256ecabdbf17 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java @@ -57,4 +57,21 @@ default Model injectProfile( */ Model injectProfiles( Model model, List profiles, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant that injects profile values into the model builder directly. + * + * @since 4.0.0 + */ + default void injectProfiles( + Model.Builder builder, + List profiles, + ModelBuilderRequest request, + ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectProfiles(built, profiles, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java b/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java index 640579cb8871..b5b785c7cb7f 100644 --- a/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java +++ b/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java @@ -40,6 +40,10 @@ class FileToRawModelMergerTest { void testOverriddenMergeMethods() { List methodNames = Stream.of(MavenMerger.class.getDeclaredMethods()) .filter(m -> m.getName().startsWith("merge")) + // Exclude *ToBuilder variants and void methods whose first parameter + // is a Builder — only the object-returning merge methods need overriding + .filter(m -> !m.getName().endsWith("ToBuilder")) + .filter(m -> !m.getParameterTypes()[0].getSimpleName().equals("Builder")) .filter(m -> { String baseName = m.getName().substring(5 /* merge */); String entity = baseName.substring(baseName.indexOf('_') + 1); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java index 36bdb746b59a..fe82c6d41990 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java @@ -52,26 +52,35 @@ public Model normalize(Model model, ModelBuilderRequest request) { } Model.Builder builder = Model.newBuilder(model); - builder.url(normalize(model.getUrl())); + normalizeBuilder(builder); + return builder.build(); + } + + @Override + public void normalize(Model.Builder builder, ModelBuilderRequest request) { + normalizeBuilder(builder); + } + + private void normalizeBuilder(Model.Builder builder) { + builder.url(normalize(builder.getUrl())); - Scm scm = model.getScm(); + Scm scm = builder.getScm(); if (scm != null) { - builder.scm(Scm.newBuilder(scm) + builder.getModifiableScm() .url(normalize(scm.getUrl())) .connection(normalize(scm.getConnection())) - .developerConnection(normalize(scm.getDeveloperConnection())) - .build()); + .developerConnection(normalize(scm.getDeveloperConnection())); } - DistributionManagement dist = model.getDistributionManagement(); + DistributionManagement dist = builder.getDistributionManagement(); if (dist != null) { Site site = dist.getSite(); if (site != null) { - builder.distributionManagement(dist.withSite(site.withUrl(normalize(site.getUrl())))); + builder.getModifiableDistributionManagement() + .getModifiableSite() + .url(normalize(site.getUrl())); } } - - return builder.build(); } private String normalize(String url) { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java index fc159b8ccd2a..95a4c2f9869c 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java @@ -44,6 +44,26 @@ @Singleton public class DefaultPluginConfigurationExpander implements PluginConfigurationExpander { + @Override + public void expandPluginConfiguration( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + + // Mutate the Build sub-builder in place to avoid intermediate immutable allocations + Build build = builder.getBuild(); + if (build != null) { + Build.Builder bb = builder.getModifiableBuild(); + bb.plugins(expandPlugin(build.getPlugins())); + PluginManagement pluginManagement = build.getPluginManagement(); + if (pluginManagement != null) { + bb.getModifiablePluginManagement().plugins(expandPlugin(pluginManagement.getPlugins())); + } + } + Reporting reporting = builder.getReporting(); + if (reporting != null) { + expandReport(reporting.getPlugins()); + } + } + @Override public Model expandPluginConfiguration(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Build build = model.getBuild(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java index 45de07f83a3c..314b6a57f409 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java @@ -56,43 +56,63 @@ public Model importManagement( ModelBuilderRequest request, ModelProblemCollector problems) { if (sources != null && !sources.isEmpty()) { - Map dependencies = new LinkedHashMap<>(); + Map dependencies = + collectDependencies(target.getDependencyManagement(), sources, request, problems); + return target.withDependencyManagement( + target.getDependencyManagement().withDependencies(dependencies.values())); + } + return target; + } + + @Override + public void importManagement( + Model.Builder builder, + List sources, + ModelBuilderRequest request, + ModelProblemCollector problems) { + if (sources != null && !sources.isEmpty()) { + Map dependencies = + collectDependencies(builder.getDependencyManagement(), sources, request, problems); + builder.getModifiableDependencyManagement().dependencies(dependencies.values()); + } + } - DependencyManagement depMgmt = target.getDependencyManagement(); + private Map collectDependencies( + DependencyManagement depMgmt, + List sources, + ModelBuilderRequest request, + ModelProblemCollector problems) { + Map dependencies = new LinkedHashMap<>(); - if (depMgmt != null) { - for (Dependency dependency : depMgmt.getDependencies()) { - dependencies.put(dependency.getManagementKey(), dependency); - } - } else { - depMgmt = DependencyManagement.newInstance(); + if (depMgmt != null) { + for (Dependency dependency : depMgmt.getDependencies()) { + dependencies.put(dependency.getManagementKey(), dependency); } + } - Set directDependencies = new HashSet<>(dependencies.keySet()); - - for (DependencyManagement source : sources) { - for (Dependency dependency : source.getDependencies()) { - String key = dependency.getManagementKey(); - Dependency present = dependencies.putIfAbsent(key, dependency); - if (present != null && !equals(dependency, present) && !directDependencies.contains(key)) { - // TODO: https://issues.apache.org/jira/browse/MNG-8004 - problems.add( - Severity.WARNING, - Version.V40, - "Ignored POM import for: " + toString(dependency) + " as already imported " - + toString(present) + ". Add the conflicting managed dependency directly " - + "to the dependencyManagement section of the POM."); - } - if (present == null && request.isLocationTracking()) { - Dependency updatedDependency = updateWithImportedFrom(dependency, source); - dependencies.put(key, updatedDependency); - } + Set directDependencies = new HashSet<>(dependencies.keySet()); + + for (DependencyManagement source : sources) { + for (Dependency dependency : source.getDependencies()) { + String key = dependency.getManagementKey(); + Dependency present = dependencies.putIfAbsent(key, dependency); + if (present != null && !equals(dependency, present) && !directDependencies.contains(key)) { + // TODO: https://issues.apache.org/jira/browse/MNG-8004 + problems.add( + Severity.WARNING, + Version.V40, + "Ignored POM import for: " + toString(dependency) + " as already imported " + + toString(present) + ". Add the conflicting managed dependency directly " + + "to the dependencyManagement section of the POM."); + } + if (present == null && request.isLocationTracking()) { + Dependency updatedDependency = updateWithImportedFrom(dependency, source); + dependencies.put(key, updatedDependency); } } - - return target.withDependencyManagement(depMgmt.withDependencies(dependencies.values())); } - return target; + + return dependencies; } private String toString(Dependency dependency) { @@ -167,6 +187,8 @@ static Dependency updateWithImportedFrom(Dependency dependency, DependencyManage if (dependencySource == null || bomSource == null || Objects.equals(dependencySource.getModelId(), bomSource.getModelId())) { + // Use forceCopy=true since we only set importedFrom (no field changes that would + // trigger copy-on-write), and build immediately as we need the immutable result. return Dependency.newBuilder(dependency, true) .importedFrom(bomLocation) .build(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java index d5c7d5d3e1b1..1de10c725700 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java @@ -45,6 +45,20 @@ public class DefaultDependencyManagementInjector implements DependencyManagement private ManagementModelMerger merger = new ManagementModelMerger(); + @Override + public void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + // Use builder getters instead of builder.build() to avoid materializing + // all model-object lists just to read Dependencies and DependencyManagement + DependencyManagement depMgmt = builder.getDependencyManagement(); + if (depMgmt != null) { + List deps = builder.getBuiltDependencies(); + List merged = merger.computeMergedDependencies(deps, depMgmt); + if (merged != null) { + builder.dependencies(merged); + } + } + } + @Override public Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { return merger.mergeManagedDependencies(model); @@ -55,41 +69,58 @@ public Model injectManagement(Model model, ModelBuilderRequest request, ModelPro */ protected static class ManagementModelMerger extends MavenModelMerger { - public Model mergeManagedDependencies(Model model) { + /** + * Computes the merged dependency list, or returns {@code null} if no dependencies were modified. + */ + List computeMergedDependencies(Model model) { DependencyManagement dependencyManagement = model.getDependencyManagement(); if (dependencyManagement != null) { - Map dependencies = new HashMap<>(); - Map context = Collections.emptyMap(); + return computeMergedDependencies(model.getDependencies(), dependencyManagement); + } + return null; + } - for (Dependency dependency : model.getDependencies()) { - Object key = getDependencyKey().apply(dependency); - dependencies.put(key, dependency); - } + /** + * Computes the merged dependency list from pre-extracted deps and dep management, + * or returns {@code null} if no dependencies were modified. + */ + List computeMergedDependencies( + List dependencies, DependencyManagement dependencyManagement) { + Map depsMap = new HashMap<>(); + Map context = Collections.emptyMap(); + + for (Dependency dependency : dependencies) { + Object key = getDependencyKey().apply(dependency); + depsMap.put(key, dependency); + } - boolean modified = false; - for (Dependency managedDependency : dependencyManagement.getDependencies()) { - Object key = getDependencyKey().apply(managedDependency); - Dependency dependency = dependencies.get(key); - if (dependency != null) { - Dependency merged = mergeDependency(dependency, managedDependency, false, context); - if (merged != dependency) { - dependencies.put(key, merged); - modified = true; - } + boolean modified = false; + for (Dependency managedDependency : dependencyManagement.getDependencies()) { + Object key = getDependencyKey().apply(managedDependency); + Dependency dependency = depsMap.get(key); + if (dependency != null) { + Dependency merged = mergeDependency(dependency, managedDependency, false, context); + if (merged != dependency) { + depsMap.put(key, merged); + modified = true; } } + } - if (modified) { - List newDeps = new ArrayList<>(dependencies.size()); - for (Dependency dep : model.getDependencies()) { - Object key = getDependencyKey().apply(dep); - Dependency dependency = dependencies.get(key); - newDeps.add(dependency); - } - return Model.newBuilder(model).dependencies(newDeps).build(); + if (modified) { + List newDeps = new ArrayList<>(depsMap.size()); + for (Dependency dep : dependencies) { + Object key = getDependencyKey().apply(dep); + newDeps.add(depsMap.get(key)); } + return newDeps; } - return model; + return null; + } + + public Model mergeManagedDependencies(Model model) { + List merged = computeMergedDependencies(model); + return merged != null ? Model.newBuilder(model).dependencies(merged).build() : model; } @Override diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java index b6d7230b4f41..69a060bb5aa3 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java @@ -66,11 +66,24 @@ public DefaultInheritanceAssembler(MavenMerger merger) { @Override public Model assembleModelInheritance( Model child, Model parent, ModelBuilderRequest request, ModelProblemCollector problems) { - Map hints = new HashMap<>(); + Map context = createContext(child, parent); + return merger.merge(child, parent, false, context); + } + + @Override + public void assembleModelInheritance( + Model.Builder childBuilder, Model parent, ModelBuilderRequest request, ModelProblemCollector problems) { + Model child = childBuilder.build(); + Map context = createContext(child, parent); + merger.merge(childBuilder, child, parent, false, context); + } + + private Map createContext(Model child, Model parent) { + Map context = new HashMap<>(); String childPath = child.getProperties().getOrDefault(CHILD_DIRECTORY_PROPERTY, child.getArtifactId()); - hints.put(CHILD_DIRECTORY, childPath); - hints.put(MavenModelMerger.CHILD_PATH_ADJUSTMENT, getChildPathAdjustment(child, parent, childPath)); - return merger.merge(child, parent, false, hints); + context.put(CHILD_DIRECTORY, childPath); + context.put(MavenModelMerger.CHILD_PATH_ADJUSTMENT, getChildPathAdjustment(child, parent, childPath)); + return context; } /** diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java index 496eb81ad156..ada763c6e06a 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java @@ -634,7 +634,7 @@ Model transformFileToRaw(Model model) { builder.dependencies(newDeps); } if (managedDepsChanged) { - builder.dependencyManagement(depMgmt.withDependencies(newManagedDeps)); + builder.getModifiableDependencyManagement().dependencies(newManagedDeps); } return builder.build(); } @@ -1007,35 +1007,44 @@ void buildEffectiveModel(Collection importIds) throws ModelBuilderExcept setSource(resultModel); setRootModel(resultModel); + // Thread remaining stages through a Model.Builder to avoid intermediate build() calls. + // The builder-accepting default methods on each interface bridge to the Model-accepting + // versions; Phase E overrides these defaults for the hot stages. + Model.Builder builder = Model.newBuilder(resultModel, false); + // model path translation - resultModel = - modelPathTranslator.alignToBaseDirectory(resultModel, resultModel.getProjectDirectory(), request); + modelPathTranslator.alignToBaseDirectory(builder, resultModel.getProjectDirectory(), request); // plugin management injection - resultModel = pluginManagementInjector.injectManagement(resultModel, request, this); + pluginManagementInjector.injectManagement(builder, request, this); - // lifecycle bindings injection + // lifecycle bindings injection (ModelTransformer API — no builder variant) if (request.getRequestType() != ModelBuilderRequest.RequestType.CONSUMER_DEPENDENCY) { org.apache.maven.api.services.ModelTransformer lifecycleBindingsInjector = request.getLifecycleBindingsInjector(); if (lifecycleBindingsInjector != null) { - resultModel = lifecycleBindingsInjector.transform(resultModel, request, this); + Model built = builder.build(); + Model transformed = lifecycleBindingsInjector.transform(built, request, this); + if (transformed != built) { + builder.reset(transformed); + } } } // dependency management import - resultModel = importDependencyManagement(resultModel, importIds); + importDependencyManagement(builder, importIds); // dependency management injection - resultModel = dependencyManagementInjector.injectManagement(resultModel, request, this); + dependencyManagementInjector.injectManagement(builder, request, this); - resultModel = modelNormalizer.injectDefaultValues(resultModel, request, this); + modelNormalizer.injectDefaultValues(builder, request, this); if (request.getRequestType() != ModelBuilderRequest.RequestType.CONSUMER_DEPENDENCY) { // plugins configuration - resultModel = pluginConfigurationExpander.expandPluginConfiguration(resultModel, request, this); + pluginConfigurationExpander.expandPluginConfiguration(builder, request, this); } + resultModel = builder.build(); for (var transformer : transformers) { resultModel = transformer.transformEffectiveModel(resultModel); } @@ -1431,11 +1440,12 @@ Model activateFileModel(Model inputModel) throws ModelBuilderException { List interpolatedActivations = getProfileActivations(inputModel); inputModel = injectProfileActivations(inputModel, interpolatedActivations); - // profile injection - inputModel = profileInjector.injectProfiles(inputModel, activePomProfiles, request, this); - inputModel = profileInjector.injectProfiles(inputModel, activeExternalProfiles, request, this); + // profile injection via builder to avoid intermediate build between injections + Model.Builder builder = Model.newBuilder(inputModel, false); + profileInjector.injectProfiles(builder, activePomProfiles, request, this); + profileInjector.injectProfiles(builder, activeExternalProfiles, request, this); - return inputModel; + return builder.build(); } @SuppressWarnings("checkstyle:methodlength") @@ -1485,25 +1495,34 @@ private Model readEffectiveModel() throws ModelBuilderException { inputModel = inputModel.withParent(inputModel.getParent().withRelativePath(relPath)); } - Model model = inheritanceAssembler.assembleModelInheritance(inputModel, parentModel, request, this); - - // Mixins - for (Mixin mixin : model.getMixins()) { - Model parent = resolveParent(model, mixin, profileActivationContext, parentChain); - // Merge mixin into model - model = inheritanceAssembler.assembleModelInheritance(model, parent, request, this); - // Ensure mixin properties override any previously inherited properties - // This is necessary because normal inheritance gives child precedence, but for mixins - // we want the mixin to take precedence over inherited parent properties - Map mergedProperties = new java.util.HashMap<>(model.getProperties()); - mergedProperties.putAll(parent.getProperties()); - model = model.withProperties(mergedProperties); + // Thread a single builder from inheritance assembly through the final build(). + // Each stage that needs an immutable snapshot calls builder.build() internally. + Model.Builder builder = Model.newBuilder(inputModel, false); + inheritanceAssembler.assembleModelInheritance(builder, parentModel, request, this); + + // Mixins — the mixin list is fixed by the child POM, so read it from the + // builder's base model. Each iteration needs a snapshot for resolveParent + // and assembleModelInheritance (which builds one internally anyway). + List mixins = builder.getBuiltMixins(); + if (!mixins.isEmpty()) { + for (Mixin mixin : mixins) { + Model snapshot = builder.build(); + Model parent = resolveParent(snapshot, mixin, profileActivationContext, parentChain); + inheritanceAssembler.assembleModelInheritance(builder, parent, request, this); + // Ensure mixin properties override any previously inherited properties + // This is necessary because normal inheritance gives child precedence, but for mixins + // we want the mixin to take precedence over inherited parent properties + Map mergedProperties = new java.util.HashMap<>(builder.getProperties()); + mergedProperties.putAll(parent.getProperties()); + builder.properties(mergedProperties); + } } // model normalization - model = modelNormalizer.mergeDuplicates(model, request, this); + modelNormalizer.mergeDuplicates(builder, request, this); - // profile activation + // profile activation — needs an immutable snapshot for the activation context + Model model = builder.build(); profileActivationContext.setModel(model); // Activate profiles from the input model (before inheritance) to get only local profiles @@ -1513,22 +1532,31 @@ private Model readEffectiveModel() throws ModelBuilderException { // profile injection - inject all profiles (local + inherited) into the model List activePomProfiles = getActiveProfiles(model.getProfiles(), profileActivationContext); - model = profileInjector.injectProfiles(model, activePomProfiles, request, this); - model = profileInjector.injectProfiles(model, activeExternalProfiles, request, this); + profileInjector.injectProfiles(builder, activePomProfiles, request, this); + profileInjector.injectProfiles(builder, activeExternalProfiles, request, this); // Track only the local profiles for this model - // Use ModelProblemUtils.toId() to get groupId:artifactId:version format (without packaging) - addActivePomProfiles(ModelProblemUtils.toId(model), localActivePomProfiles); + // Use the builder's getters to compute the id without an intermediate build() + String groupId = builder.getGroupId(); + if (groupId == null && builder.getParent() != null) { + groupId = builder.getParent().getGroupId(); + } + String version = builder.getVersion(); + if (version == null && builder.getParent() != null) { + version = builder.getParent().getVersion(); + } + addActivePomProfiles( + ModelProblemUtils.toId(groupId, builder.getArtifactId(), version), localActivePomProfiles); - // model interpolation - Model resultModel = model; - resultModel = interpolateModel(resultModel, request, this); + // model interpolation + normalization + url normalization via builder + interpolateModel(builder, request, this); // model normalization - resultModel = modelNormalizer.mergeDuplicates(resultModel, request, this); + modelNormalizer.mergeDuplicates(builder, request, this); // url normalization - resultModel = modelUrlNormalizer.normalize(resultModel, request); + modelUrlNormalizer.normalize(builder, request); + Model resultModel = builder.build(); // Now the fully interpolated model is available: reconfigure the resolver if (!resultModel.getRepositories().isEmpty()) { @@ -2048,14 +2076,14 @@ protected void mergeModel_Subprojects( return new ParentModelWithProfiles(injectedParentModel.withParent(null), parentActivePomProfiles); } - private Model importDependencyManagement(Model model, Collection importIds) { - DependencyManagement depMgmt = model.getDependencyManagement(); + private void importDependencyManagement(Model.Builder builder, Collection importIds) { + DependencyManagement depMgmt = builder.getDependencyManagement(); if (depMgmt == null) { - return model; + return; } - String importing = model.getGroupId() + ':' + model.getArtifactId() + ':' + model.getVersion(); + String importing = builder.getGroupId() + ':' + builder.getArtifactId() + ':' + builder.getVersion(); importIds.add(importing); @@ -2085,10 +2113,12 @@ private Model importDependencyManagement(Model model, Collection importI importIds.remove(importing); - model = model.withDependencyManagement( - model.getDependencyManagement().withDependencies(deps)); + // Use sub-builder mutation instead of immutable rebuild + builder.getModifiableDependencyManagement().dependencies(deps); - return dependencyManagementImporter.importManagement(model, importMgmts, request, this); + if (importMgmts != null) { + dependencyManagementImporter.importManagement(builder, importMgmts, request, this); + } } private DependencyManagement loadDependencyManagement(Dependency dependency, Collection importIds) { @@ -2419,6 +2449,33 @@ private Model injectProfileActivations(Model model, List activations return modified ? model.withProfiles(profiles) : model; } + private void interpolateModel(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + // The model interpolator transforms builder fields in place via visitBuilder, + // avoiding the intermediate Model materialization + reset the SPI default performs. + Path projectDir = builder.getPomFile() != null ? builder.getPomFile().getParent() : null; + modelInterpolator.interpolateModel(builder, projectDir, request, problems); + + // Parent version interpolation: use sub-builder mutation to avoid + // intermediate immutable Model/Parent rebuilds + Parent parent = builder.getParent(); + if (parent != null) { + Map map1 = request.getSession().getUserProperties(); + Map map2 = builder.getProperties(); + Map map3 = request.getSession().getSystemProperties(); + UnaryOperator cb = Interpolator.chain(map1::get, map2::get, map3::get); + try { + String interpolated = interpolator.interpolate(parent.getVersion(), cb); + builder.getModifiableParent().version(interpolated); + } catch (Exception e) { + problems.add( + Severity.ERROR, + Version.BASE, + "Failed to interpolate field: " + parent.getVersion() + " on class: ", + e); + } + } + } + private Model interpolateModel(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Model interpolatedModel = modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java index 1932f3fc5515..f86996a1a08a 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java @@ -103,6 +103,22 @@ interface InnerInterpolator { public Model interpolateModel( Model model, Path projectDir, ModelBuilderRequest request, ModelProblemCollector problems) { InnerInterpolator innerInterpolator = createInterpolator(model, projectDir, request, problems); + return newTransformer(innerInterpolator).visit(model); + } + + @Override + public void interpolateModel( + Model.Builder builder, Path projectDir, ModelBuilderRequest request, ModelProblemCollector problems) { + // Build a snapshot for consistent property resolution during interpolation. + // The per-field transform methods read from this immutable target and write + // changed values to the existing builder, avoiding the second Model + // materialization and reset that the SPI default method performs. + Model model = builder.build(); + InnerInterpolator innerInterpolator = createInterpolator(model, projectDir, request, problems); + newTransformer(innerInterpolator).visitBuilder(builder, model); + } + + private MavenTransformer newTransformer(InnerInterpolator innerInterpolator) { return new MavenTransformer(innerInterpolator::interpolate) { @Override protected String transform(String value) { @@ -115,7 +131,7 @@ protected String transform(String value) { } return super.transform(value); } - }.visit(model); + }; } private InnerInterpolator createInterpolator( diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java index f36918f770aa..5045c78cbb83 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java @@ -45,6 +45,41 @@ public class DefaultModelNormalizer implements ModelNormalizer { private DuplicateMerger merger = new DuplicateMerger(); + @Override + public void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + + // Use sub-builder mutation to avoid intermediate immutable allocations + Build build = builder.getBuild(); + if (build != null) { + List plugins = build.getPlugins(); + Map normalized = new LinkedHashMap<>(plugins.size() * 2); + + for (Plugin plugin : plugins) { + Object key = plugin.getKey(); + Plugin first = normalized.get(key); + if (first != null) { + plugin = merger.mergePlugin(plugin, first); + } + normalized.put(key, plugin); + } + + if (plugins.size() != normalized.size()) { + builder.getModifiableBuild().plugins(normalized.values()); + } + } + + List dependencies = builder.getBuiltDependencies(); + Map normalizedDeps = new LinkedHashMap<>(dependencies.size() * 2); + + for (Dependency dependency : dependencies) { + normalizedDeps.put(dependency.getManagementKey(), dependency); + } + + if (dependencies.size() != normalizedDeps.size()) { + builder.dependencies(normalizedDeps.values()); + } + } + @Override public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Model.Builder builder = Model.newBuilder(model); @@ -64,8 +99,7 @@ public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProb } if (plugins.size() != normalized.size()) { - builder.build( - Build.newBuilder(build).plugins(normalized.values()).build()); + builder.getModifiableBuild().plugins(normalized.values()); } } @@ -100,6 +134,23 @@ public Plugin mergePlugin(Plugin target, Plugin source) { } } + @Override + public void injectDefaultValues( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + + List newDeps = injectList(builder.getBuiltDependencies(), this::injectDependency); + if (newDeps != null) { + builder.dependencies(newDeps); + } + Build build = builder.getBuild(); + if (build != null) { + List newPlugins = injectList(build.getPlugins(), this::injectPlugin); + if (newPlugins != null) { + builder.getModifiableBuild().plugins(newPlugins); + } + } + } + @Override public Model injectDefaultValues(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Model.Builder builder = Model.newBuilder(model); @@ -107,10 +158,10 @@ public Model injectDefaultValues(Model model, ModelBuilderRequest request, Model builder.dependencies(injectList(model.getDependencies(), this::injectDependency)); Build build = model.getBuild(); if (build != null) { - Build newBuild = Build.newBuilder(build) - .plugins(injectList(build.getPlugins(), this::injectPlugin)) - .build(); - builder.build(newBuild != build ? newBuild : null); + List newPlugins = injectList(build.getPlugins(), this::injectPlugin); + if (newPlugins != null) { + builder.getModifiableBuild().plugins(newPlugins); + } } return builder.build(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java index bec34fe0fa76..7d1d756076aa 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java @@ -246,29 +246,29 @@ private static boolean dependenciesEqual( && Objects.equals(dep1.getSystemPath(), dep2.getSystemPath()) && Objects.equals(dep1.getExclusions(), dep2.getExclusions()) && Objects.equals(dep1.getOptional(), dep2.getOptional()) - && Objects.equals(dep1.getLocationKeys(), dep2.getLocationKeys()) && locationsEqual(dep1, dep2) && Objects.equals(dep1.getImportedFrom(), dep2.getImportedFrom()); } /** * Compare locations maps for two dependencies. + * Uses the direct getLocations() map instead of iterating through + * individual keys to avoid KeyValueHolder allocation overhead. + * Short-circuits on empty maps (common for imported dependencies). */ private static boolean locationsEqual( org.apache.maven.api.model.Dependency dep1, org.apache.maven.api.model.Dependency dep2) { - var keys1 = dep1.getLocationKeys(); - var keys2 = dep2.getLocationKeys(); - - if (!Objects.equals(keys1, keys2)) { - return false; + var locs1 = dep1.getLocations(); + var locs2 = dep2.getLocations(); + // Fast path: both empty (common for imported deps with location tracking off) + if (locs1.isEmpty() && locs2.isEmpty()) { + return true; } - - for (Object key : keys1) { - if (!Objects.equals(dep1.getLocation(key), dep2.getLocation(key))) { - return false; - } + // Use precomputed hash code for fast inequality check + if (dep1.getLocationsHashCode() != dep2.getLocationsHashCode()) { + return false; } - return true; + return locs1.equals(locs2); } /** @@ -283,33 +283,24 @@ private static int computeHashCode(Object obj) { /** * Custom hash code for Dependency objects based on all fields. + * Inlined to avoid the Object[] varargs allocation from Objects.hash(). + * Uses the precomputed locations hash code to avoid re-iterating + * the locations map entries. */ private static int dependencyHashCode(org.apache.maven.api.model.Dependency dep) { - return Objects.hash( - dep.getGroupId(), - dep.getArtifactId(), - dep.getVersion(), - dep.getType(), - dep.getClassifier(), - dep.getScope(), - dep.getSystemPath(), - dep.getExclusions(), - dep.getOptional(), - dep.getLocationKeys(), - locationsHashCode(dep), - dep.getImportedFrom()); - } - - /** - * Compute hash code for locations map. - */ - private static int locationsHashCode(org.apache.maven.api.model.Dependency dep) { - int hash = 1; - for (Object key : dep.getLocationKeys()) { - hash = 31 * hash + Objects.hashCode(key); - hash = 31 * hash + Objects.hashCode(dep.getLocation(key)); - } - return hash; + int h = 1; + h = 31 * h + Objects.hashCode(dep.getGroupId()); + h = 31 * h + Objects.hashCode(dep.getArtifactId()); + h = 31 * h + Objects.hashCode(dep.getVersion()); + h = 31 * h + Objects.hashCode(dep.getType()); + h = 31 * h + Objects.hashCode(dep.getClassifier()); + h = 31 * h + Objects.hashCode(dep.getScope()); + h = 31 * h + Objects.hashCode(dep.getSystemPath()); + h = 31 * h + Objects.hashCode(dep.getExclusions()); + h = 31 * h + Objects.hashCode(dep.getOptional()); + h = 31 * h + dep.getLocationsHashCode(); + h = 31 * h + Objects.hashCode(dep.getImportedFrom()); + return h; } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java index 52ebbbf9d9c6..2c5af59ce280 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java @@ -51,17 +51,58 @@ public DefaultModelPathTranslator(PathTranslator pathTranslator) { this.pathTranslator = pathTranslator; } + @Override + public void alignToBaseDirectory(Model.Builder builder, Path basedir, ModelBuilderRequest request) { + if (basedir == null) { + return; + } + // Mutate the Build and Reporting sub-builders in place, avoiding + // intermediate immutable object allocations + Build build = builder.getBuild(); + if (build != null) { + Build.Builder bb = builder.getModifiableBuild(); + bb.sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) + .directory(alignToBaseDirectory(build.getDirectory(), basedir)) + .sourceDirectory(alignToBaseDirectory(build.getSourceDirectory(), basedir)) + .testSourceDirectory(alignToBaseDirectory(build.getTestSourceDirectory(), basedir)) + .scriptSourceDirectory(alignToBaseDirectory(build.getScriptSourceDirectory(), basedir)) + .resources(map(build.getResources(), this::alignToBaseDirectory, basedir)) + .testResources(map(build.getTestResources(), this::alignToBaseDirectory, basedir)) + .filters(map(build.getFilters(), this::alignToBaseDirectory, basedir)) + .outputDirectory(alignToBaseDirectory(build.getOutputDirectory(), basedir)) + .testOutputDirectory(alignToBaseDirectory(build.getTestOutputDirectory(), basedir)); + } + Reporting reporting = builder.getReporting(); + if (reporting != null) { + builder.getModifiableReporting() + .outputDirectory(alignToBaseDirectory(reporting.getOutputDirectory(), basedir)); + } + } + @Override public Model alignToBaseDirectory(Model model, Path basedir, ModelBuilderRequest request) { if (model == null || basedir == null) { return model; } + Model.Builder builder = Model.newBuilder(model); + if (alignToBaseDirectory(model, basedir, builder)) { + return builder.build(); + } + return model; + } + + /** + * Shared logic: reads from {@code model}, writes modified fields to {@code builder}. + * Returns {@code true} if any field was modified. + */ + private boolean alignToBaseDirectory(Model model, Path basedir, Model.Builder builder) { + boolean modified = false; + Build build = model.getBuild(); - Build newBuild = null; if (build != null) { - newBuild = Build.newBuilder(build) - .sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) + Build.Builder bb = builder.getModifiableBuild(); + bb.sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) .directory(alignToBaseDirectory(build.getDirectory(), basedir)) .sourceDirectory(alignToBaseDirectory(build.getSourceDirectory(), basedir)) .testSourceDirectory(alignToBaseDirectory(build.getTestSourceDirectory(), basedir)) @@ -70,24 +111,17 @@ public Model alignToBaseDirectory(Model model, Path basedir, ModelBuilderRequest .testResources(map(build.getTestResources(), this::alignToBaseDirectory, basedir)) .filters(map(build.getFilters(), this::alignToBaseDirectory, basedir)) .outputDirectory(alignToBaseDirectory(build.getOutputDirectory(), basedir)) - .testOutputDirectory(alignToBaseDirectory(build.getTestOutputDirectory(), basedir)) - .build(); + .testOutputDirectory(alignToBaseDirectory(build.getTestOutputDirectory(), basedir)); + modified = true; } Reporting reporting = model.getReporting(); - Reporting newReporting = null; if (reporting != null) { - newReporting = Reporting.newBuilder(reporting) - .outputDirectory(alignToBaseDirectory(reporting.getOutputDirectory(), basedir)) - .build(); - } - if (newBuild != build || newReporting != reporting) { - model = Model.newBuilder(model) - .build(newBuild) - .reporting(newReporting) - .build(); + builder.getModifiableReporting() + .outputDirectory(alignToBaseDirectory(reporting.getOutputDirectory(), basedir)); + modified = true; } - return model; + return modified; } /** diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java index 4f10a4ca1f26..8219b5380ef8 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java @@ -47,6 +47,21 @@ public class DefaultPluginManagementInjector implements PluginManagementInjector private ManagementModelMerger merger = new ManagementModelMerger(); + @Override + public void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + // Use sub-builder mutation to avoid intermediate immutable allocations + Build build = builder.getBuild(); + if (build != null) { + PluginManagement pluginManagement = build.getPluginManagement(); + if (pluginManagement != null) { + List mergedPlugins = merger.mergeManagedPluginList(build, pluginManagement); + if (mergedPlugins != null) { + builder.getModifiableBuild().plugins(mergedPlugins); + } + } + } + } + @Override public Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { return merger.mergeManagedBuildPlugins(model); @@ -57,18 +72,12 @@ public Model injectManagement(Model model, ModelBuilderRequest request, ModelPro */ protected static class ManagementModelMerger extends MavenModelMerger { - public Model mergeManagedBuildPlugins(Model model) { - Build build = model.getBuild(); - if (build != null) { - PluginManagement pluginManagement = build.getPluginManagement(); - if (pluginManagement != null) { - return model.withBuild(mergePluginContainerPlugins(build, pluginManagement)); - } - } - return model; - } - - private Build mergePluginContainerPlugins(Build target, PluginContainer source) { + /** + * Returns the merged plugin list, or {@code null} if no changes were made. + * Used by the builder-accepting pipeline stage to set plugins directly on + * the Build sub-builder without creating intermediate immutable Build objects. + */ + List mergeManagedPluginList(Build target, PluginContainer source) { List src = source.getPlugins(); if (!src.isEmpty()) { Map managedPlugins = new LinkedHashMap<>(src.size() * 2); @@ -89,9 +98,23 @@ private Build mergePluginContainerPlugins(Build target, PluginContainer source) } newPlugins.add(element); } - return target.withPlugins(newPlugins); + return newPlugins; } - return target; + return null; + } + + public Model mergeManagedBuildPlugins(Model model) { + Build build = model.getBuild(); + if (build != null) { + PluginManagement pluginManagement = build.getPluginManagement(); + if (pluginManagement != null) { + List mergedPlugins = mergeManagedPluginList(build, pluginManagement); + if (mergedPlugins != null) { + return model.withBuild(build.withPlugins(mergedPlugins)); + } + } + } + return model; } @Override diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java index 206e8d4834c8..82a996f64752 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java @@ -70,6 +70,27 @@ public Model injectProfiles( return result == KEY ? model : result; } + @Override + public void injectProfiles( + Model.Builder builder, + List profiles, + ModelBuilderRequest request, + ModelProblemCollector problems) { + for (Profile profile : profiles) { + if (profile != null) { + Model model = builder.build(); + merger.mergeModelBase(builder, model, profile); + + if (profile.getBuild() != null) { + Build build = model.getBuild() != null ? model.getBuild() : Build.newInstance(); + Build.Builder bbuilder = Build.newBuilder(build); + merger.mergeBuildBase(bbuilder, build, profile.getBuild()); + builder.build(bbuilder.build()); + } + } + } + } + private Model doInjectProfiles(Model model, List profiles) { Model orgModel = model; for (Profile profile : profiles) { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java index b9f259cced1c..717d4a83aa94 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java @@ -73,6 +73,11 @@ public Model merge(Model target, Model source, boolean sourceDominant, Map return super.merge(target, source, sourceDominant, hints); } + @Override + public void merge(Model.Builder builder, Model target, Model source, boolean sourceDominant, Map hints) { + super.merge(builder, target, source, sourceDominant, hints); + } + @Override protected Model mergeModel(Model target, Model source, boolean sourceDominant, Map context) { context.put(ARTIFACT_ID, target.getArtifactId()); @@ -80,6 +85,14 @@ protected Model mergeModel(Model target, Model source, boolean sourceDominant, M return super.mergeModel(target, source, sourceDominant, context); } + @Override + protected void mergeModel( + Model.Builder builder, Model target, Model source, boolean sourceDominant, Map context) { + context.put(ARTIFACT_ID, target.getArtifactId()); + + super.mergeModel(builder, target, source, sourceDominant, context); + } + @Override protected void mergeModel_Name( Model.Builder builder, Model target, Model source, boolean sourceDominant, Map context) { diff --git a/src/mdo/merger.vm b/src/mdo/merger.vm index 6724b09742de..e5e5f9f41244 100644 --- a/src/mdo/merger.vm +++ b/src/mdo/merger.vm @@ -88,6 +88,29 @@ public class ${className} { return merge${root.name}(target, source, sourceDominant, context); } + /** + * Builder-accepting variant that merges the source into an existing builder, + * avoiding the intermediate immutable object allocation that {@link #merge} performs. + * + * @param builder The builder to merge into, must not be {@code null}. + * @param target The immutable snapshot to read current field values from, must not be {@code null}. + * @param source The (read-only) source object to merge from, may be {@code null}. + * @param sourceDominant Whether the source provides the dominant data. + * @param hints Domain-specific hints, may be {@code null}. + */ + public void merge(${root.name}.Builder builder, ${root.name} target, ${root.name} source, boolean sourceDominant, Map hints) { + Objects.requireNonNull(builder, "builder cannot be null"); + Objects.requireNonNull(target, "target cannot be null"); + if (source == null) { + return; + } + Map context = new HashMap<>(); + if (hints != null) { + context.putAll(hints); + } + merge${root.name}(builder, target, source, sourceDominant, context); + } + #foreach ( $class in $model.allClasses ) #if ( $class.name != "InputSource" && $class.name != "InputLocation" ) #set ( $ancestors = $Helper.ancestors( $class ) ) @@ -101,6 +124,17 @@ public class ${className} { return builder.build(); } + /** + * Merges the source into a builder based on the target, returning the Builder + * without calling build(). This allows callers to defer the build() call and + * avoid intermediate immutable object allocations in multi-stage pipelines. + */ + protected ${class.name}.Builder merge${class.name}ToBuilder(${class.name} target, ${class.name} source, boolean sourceDominant, Map context) { + ${class.name}.Builder builder = ${class.name}.newBuilder(target); + merge${class.name}(builder, target, source, sourceDominant, context); + return builder; + } + protected void merge${class.name}(${class.name}.Builder builder, ${class.name} target, ${class.name} source, boolean sourceDominant, Map context) { #if ( $class.superClass ) merge${class.superClass}(builder, target, source, sourceDominant, context); diff --git a/src/mdo/model.vm b/src/mdo/model.vm index 481ee0b4891a..78d73032f784 100644 --- a/src/mdo/model.vm +++ b/src/mdo/model.vm @@ -20,6 +20,11 @@ # #set ( $package = "${packageModelV4}" ) #set ( $root = $model.getClass( $model.getRoot($version), $version ) ) +## Build set of model class names for detecting list-of-model-object fields +#set ( $modelClassNames = [] ) +#foreach ( $c in $model.allClasses ) + #set ( $dummy = $modelClassNames.add($c.name) ) +#end #foreach ( $class in $model.allClasses ) #set ( $ancestors = $Helper.ancestors( $class ) ) #set ( $allFields = [] ) @@ -151,6 +156,8 @@ public class ${class.name} #if ( $locationTracking && ! $class.superClass ) /** Locations */ final Map locations; + /** Cached hash code for the locations map, precomputed at build time */ + final int locationsHashCode; /** Location tracking */ final InputLocation importedFrom; #end @@ -168,7 +175,28 @@ public class ${class.name} this.modelEncoding = builder.modelEncoding != null ? builder.modelEncoding : (builder.base != null ? builder.base.modelEncoding : "UTF-8"); #end #foreach ( $field in $class.getFields($version) ) - #if ( $field.type == "java.util.List" || $field.type == "java.util.Properties" || $field.type == "java.util.Map" ) + #set ( $cType = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $cIsModelObjList = false ) + #if ( $cType.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $cIsModelObjList = true ) + #end + #set ( $cIsModelObj = false ) + #if ( !$cType.startsWith("List<") && $modelClassNames.contains($cType) ) + #set ( $cIsModelObj = true ) + #end + #if ( $cIsModelObjList ) + if (builder.${field.name} != null) { + ArrayList<${field.to}> bl = new ArrayList<>(builder.${field.name}.size()); + for (${field.to}.Builder b : builder.${field.name}) { + bl.add(b.build()); + } + this.${field.name} = ImmutableCollections.copy(bl); + } else { + this.${field.name} = ImmutableCollections.copy(builder.base != null ? builder.base.${field.name} : null); + } + #elseif ( $cIsModelObj ) + this.${field.name} = builder.${field.name} != null ? builder.${field.name}.build() : (builder.base != null ? builder.base.${field.name} : null); + #elseif ( $field.type == "java.util.List" || $field.type == "java.util.Properties" || $field.type == "java.util.Map" ) this.${field.name} = ImmutableCollections.copy(builder.${field.name} != null ? builder.${field.name} : (builder.base != null ? builder.base.${field.name} : null)); #else #if ( $field.type == "boolean" || $field.type == "int" ) @@ -180,6 +208,7 @@ public class ${class.name} #end #if ( $locationTracking && ! $class.superClass ) this.locations = builder.computeLocations(); + this.locationsHashCode = this.locations.hashCode(); this.importedFrom = builder.importedFrom; #end } @@ -264,6 +293,26 @@ public class ${class.name} return locations.keySet().stream(); } + /** + * Gets the locations map. Provides direct access to avoid individual key lookups + * when comparing or hashing all locations at once. + * + * @return an unmodifiable map of locations, never {@code null} + */ + public Map getLocations() { + return locations; + } + + /** + * Gets the precomputed hash code for the locations map. + * This avoids re-iterating the map entries during pooling/interning operations. + * + * @return the cached hash code of the locations map + */ + public int getLocationsHashCode() { + return locationsHashCode; + } + /** * Gets the input location that caused this model to be read. */ @@ -391,8 +440,18 @@ public class ${class.name} #end #foreach ( $field in $class.getFields($version) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $isModelObjList = false ) + #set ( $isModelObj = false ) #if ( $type.startsWith("List<") ) - #set ( $type = ${type.replace('List<','Collection<')} ) + #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $isModelObjList = true ) + #set ( $type = "Collection<${field.to}.Builder>" ) + #else + #set ( $type = ${type.replace('List<','Collection<')} ) + #end + #elseif ( $modelClassNames.contains($type) ) + #set ( $isModelObj = true ) + #set ( $type = "${type}.Builder" ) #end #if ( $type == 'boolean' ) Boolean ${field.name}; @@ -435,7 +494,27 @@ public class ${class.name} #end if (forceCopy) { #foreach ( $field in $class.getFields($version) ) + #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $fcIsModelObjList = false ) + #set ( $fcIsModelObj = false ) + #if ( $type.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $fcIsModelObjList = true ) + #elseif ( !$type.startsWith("List<") && $modelClassNames.contains($type) ) + #set ( $fcIsModelObj = true ) + #end + #if ( $fcIsModelObjList ) + if (base.${field.name} != null && !base.${field.name}.isEmpty()) { + ArrayList<${field.to}.Builder> bl = new ArrayList<>(base.${field.name}.size()); + for (${field.to} v : base.${field.name}) { + bl.add(${field.to}.newBuilder(v, false)); + } + this.${field.name} = bl; + } + #elseif ( $fcIsModelObj ) + this.${field.name} = base.${field.name} != null ? ${type}.newBuilder(base.${field.name}, false) : null; + #else this.${field.name} = base.${field.name}; + #end #end #if ( $locationTracking ) this.locations = base.locations; @@ -461,19 +540,168 @@ public class ${class.name} #end #foreach ( $field in $allFields ) + #set ( $cap = $Helper.capitalise( $field.name ) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $sIsModelObjList = false ) + #set ( $sIsModelObj = false ) #if ( $type.startsWith("List<") ) - #set ( $type = ${type.replace('List<','Collection<')} ) + #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $sIsModelObjList = true ) + #else + #set ( $type = ${type.replace('List<','Collection<')} ) + #end + #elseif ( $modelClassNames.contains($type) ) + #set ( $sIsModelObj = true ) #end #foreach( $ann in ${field.annotations} ) ${ann} #end + #if ( $sIsModelObjList ) + /** + * Sets the {@code ${field.name}} for this builder. Each element is wrapped + * in a lightweight builder to support deferred materialization. + * + * @param ${field.name} the elements to set + * @return this builder + */ + @Nonnull + public Builder ${field.name}(Collection<${field.to}> ${field.name}) { + if (${field.name} != null) { + ArrayList<${field.to}.Builder> bl = new ArrayList<>(${field.name}.size()); + for (${field.to} v : ${field.name}) { + bl.add(${field.to}.newBuilder(v, false)); + } + this.${field.name} = bl; + } else { + this.${field.name} = null; + } + return this; + } + + /** + * Returns the mutable list of {@code ${field.to}.Builder} elements. + * If the builder has not been explicitly set, lazily wraps the base + * object's immutable list into builders and caches the result. + * + * @return a mutable list of element builders, never {@code null} + */ + @Nonnull + public List<${field.to}.Builder> getModifiable${cap}() { + if (${field.name} == null) { + if (base != null && !base.${field.name}.isEmpty()) { + ArrayList<${field.to}.Builder> bl = new ArrayList<>(base.${field.name}.size()); + for (${field.to} v : base.${field.name}) { + bl.add(${field.to}.newBuilder(v, false)); + } + this.${field.name} = bl; + } else { + this.${field.name} = new ArrayList<>(); + } + } + @SuppressWarnings("unchecked") + List<${field.to}.Builder> result = (List<${field.to}.Builder>) (List) ${field.name}; + return result; + } + + #elseif ( $sIsModelObj ) + /** + * Sets the {@code ${field.name}} for this builder, wrapping the immutable + * object in a lightweight builder to support deferred materialization. + * + * @param ${field.name} the {@code ${type}} to set, or {@code null} + * @return this builder + */ + @Nonnull + public Builder ${field.name}(${type} ${field.name}) { + this.${field.name} = ${field.name} != null ? ${type}.newBuilder(${field.name}, false) : null; + return this; + } + + /** + * Returns the mutable {@code ${type}.Builder} for the {@code ${field.name}} field. + * If the builder has not been explicitly set, lazily wraps the base + * object's immutable value into a builder and caches the result. + * + * @return a mutable builder for the ${field.name}, never {@code null} + */ + @Nonnull + public ${type}.Builder getModifiable${cap}() { + if (${field.name} == null) { + ${type} baseVal = base != null ? base.get${cap}() : null; + ${field.name} = baseVal != null ? ${type}.newBuilder(baseVal, false) : ${type}.newBuilder(false); + } + return ${field.name}; + } + + #else @Nonnull public Builder ${field.name}(${type} ${field.name}) { this.${field.name} = ${field.name}; return this; } + #end + #end + #foreach ( $field in $allFields ) + #set ( $cap = $Helper.capitalise( $field.name ) ) + #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $gIsModelObjList = false ) + #set ( $gIsModelObj = false ) + ## Builder stores List fields as Collection — match that type for the getter + #if ( $type.startsWith("List<") ) + #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $gIsModelObjList = true ) + #set ( $type = "Collection<${field.to}.Builder>" ) + #else + #set ( $type = ${type.replace('List<','Collection<')} ) + #end + #elseif ( $modelClassNames.contains($type) ) + #set ( $gIsModelObj = true ) + #end + #if ( $type == "boolean" || $type == "Boolean" ) + #set ( $pfx = "is" ) + #else + #set ( $pfx = "get" ) + #end + #if ( $gIsModelObjList ) + ## For model-object lists, return the raw field only (don't resolve through base + ## to avoid unwanted lazy-wrapping that would defeat the short-circuit optimization) + public ${type} ${pfx}${cap}() { + return ${field.name}; + } + ## Provide a method to get the built list, resolving through base when unmodified. + ## This avoids the overhead of a full model build() just to read one list field + ## in pipeline stages that only need to inspect a specific list. + public List<${field.to}> getBuilt${cap}() { + if (${field.name} == null) { + return base != null ? base.get${cap}() : List.of(); + } + ArrayList<${field.to}> result = new ArrayList<>(${field.name}.size()); + for (${field.to}.Builder b : ${field.name}) { + result.add(b.build()); + } + return result; + } + #elseif ( $gIsModelObj ) + ## For single model-object fields, resolve through .build() when the + ## sub-builder has been explicitly set; fall through to base otherwise. + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name}.build() : (base != null ? base.${pfx}${cap}() : null); + } + #elseif ( $type == "boolean" ) + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : ${field.defaultValue}); + } + #elseif ( $type == "int" ) + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : ${field.defaultValue}); + } + #else + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : null); + } + #end + #end #if ( $locationTracking ) @@ -494,13 +722,67 @@ public class ${class.name} return this; } + public InputLocation getLocation(Object key) { + if (locations != null && locations.containsKey(key)) { + return locations.get(key); + } + return base != null ? base.getLocation(key) : null; + } + + public InputLocation getImportedFrom() { + return importedFrom != null ? importedFrom : (base != null ? base.getImportedFrom() : null); + } + #end + /** + * Resets this builder to wrap the specified base object, clearing all + * explicitly set fields. After reset, all getters resolve through the + * new base. Used by pipeline stage default methods to replace the + * builder's state with a stage's result without allocating a new Builder. + * + * @param base the new base object to wrap + * @return this builder + */ + @Nonnull + public Builder reset(${class.name} base) { + #if ( $class.superClass ) + super.reset(base); + #end + this.base = base; + #if ( $class == $root ) + this.namespaceUri = null; + this.modelEncoding = null; + #end + #foreach ( $field in $class.getFields($version) ) + this.${field.name} = null; + #end + #if ( ! $class.superClass && $locationTracking ) + this.locations = null; + this.importedFrom = null; + #end + return this; + } + @Nonnull public ${class.name} build() { // this method should not contain any logic other than creating (or reusing) an object in order to ease subclassing if (base != null #foreach ( $field in $allFields ) + #set ( $bType = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $bIsModelObjList = false ) + #set ( $bIsModelObj = false ) + #if ( $bType.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $bIsModelObjList = true ) + #elseif ( !$bType.startsWith("List<") && $modelClassNames.contains($bType) ) + #set ( $bIsModelObj = true ) + #end + #if ( $bIsModelObjList ) + && (${field.name} == null || (${field.name}.isEmpty() && base.${field.name}.isEmpty())) + #elseif ( $bIsModelObj ) + && ${field.name} == null + #else && (${field.name} == null || ${field.name} == base.${field.name}) + #end #end ) { return base; @@ -518,14 +800,16 @@ public class ${class.name} Map newlocs = locations != null ? locations : Map.of(); Map oldlocs = base != null ? base.locations : Map.of(); if (newlocs.isEmpty()) { - return Map.copyOf(oldlocs); + return oldlocs; } if (oldlocs.isEmpty()) { return Map.copyOf(newlocs); } - return Stream.concat(newlocs.entrySet().stream(), oldlocs.entrySet().stream()) - // Keep value from newlocs in case of duplicates - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue, (v1, v2) -> v1)); + // Use HashMap.putAll instead of Stream.concat().collect() to avoid + // Stream allocation and intermediate Map.Entry iteration overhead + HashMap merged = new HashMap<>(oldlocs); + merged.putAll(newlocs); // newlocs entries override oldlocs (same semantics as before) + return Map.copyOf(merged); } #end } diff --git a/src/mdo/transformer.vm b/src/mdo/transformer.vm index e1655e15b850..0f4bd3da0de8 100644 --- a/src/mdo/transformer.vm +++ b/src/mdo/transformer.vm @@ -67,6 +67,25 @@ public class ${className} { return transform${root.name}(target); } + /** + * Transforms fields of the given {@code target} directly on the provided {@code builder}, + * avoiding the intermediate immutable model materialization that {@link #visit} performs. + * Each field is read from the immutable {@code target}, transformed, and any changed + * value is written to the existing {@code builder}. + * + * @param builder the existing builder to update in place + * @param target the immutable model to read field values from + */ + public void visitBuilder(${root.name}.Builder builder, ${root.name} target) { + Objects.requireNonNull(builder, "builder cannot be null"); + Objects.requireNonNull(target, "target cannot be null"); +#set ( $rootAllFields = $Helper.xmlFields( $root ) ) + Supplier<${root.name}.Builder> creator = () -> builder; +#foreach ( $field in $rootAllFields ) + transform${field.modelClass.name}_${Helper.capitalise($field.name)}(creator, builder, target); +#end + } + /** * The transformation function. */