From b427f467252d1e6f38b7b07b1f156564a38bc864 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 28 Aug 2025 11:35:57 +0000 Subject: [PATCH] DefaultPluginPrefixResolver: reduce unnecessary plugin downloads by two-pass heuristic on artifactId; add unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change avoids loading plugin descriptors for non-candidate plugins by first filtering declared plugins using a conventional artifactId→prefix mapping (e.g., maven-compiler-plugin → compiler). If a match is not found, we fall back to the original full scan to preserve correctness for custom goalPrefix values. Adds DefaultPluginPrefixResolverTest to verify: - Only candidate plugin descriptor is loaded based on heuristic - Fallback full scan resolves custom goalPrefix correctly Fixes #11067 --- .../internal/DefaultPluginPrefixResolver.java | 70 +++++++- .../DefaultPluginPrefixResolverTest.java | 157 ++++++++++++++++++ 2 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 impl/maven-core/src/test/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolverTest.java diff --git a/impl/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java b/impl/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java index dd811f2c1eb4..b6f4a6bd2d95 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java +++ b/impl/maven-core/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java @@ -129,12 +129,42 @@ private PluginPrefixResult resolveFromProject(PluginPrefixRequest request) { } private PluginPrefixResult resolveFromProject(PluginPrefixRequest request, List plugins) { + // First pass: narrow down to likely candidates based on artifactId to avoid resolving all plugins. + List candidates = new ArrayList<>(); + String wanted = request.getPrefix(); for (Plugin plugin : plugins) { + if (isLikelyPrefixFor(plugin, wanted)) { + candidates.add(plugin); + } + } + + // Try likely candidates first (cheap filter, preserves behavior if match is found). + for (Plugin plugin : candidates) { try { PluginDescriptor pluginDescriptor = pluginManager.loadPlugin(plugin, request.getRepositories(), request.getRepositorySession()); + if (wanted.equals(pluginDescriptor.getGoalPrefix())) { + return new DefaultPluginPrefixResult(plugin); + } + } catch (Exception e) { + if (logger.isDebugEnabled()) { + logger.warn("Failed to retrieve plugin descriptor for {}: {}", plugin.getId(), e.getMessage(), e); + } else { + logger.warn("Failed to retrieve plugin descriptor for {}: {}", plugin.getId(), e.getMessage()); + } + } + } - if (request.getPrefix().equals(pluginDescriptor.getGoalPrefix())) { + // Fallback: full scan (preserves original behavior for custom/non-standard goal prefixes). + for (Plugin plugin : plugins) { + if (isLikelyPrefixFor(plugin, wanted)) { + // already attempted above + continue; + } + try { + PluginDescriptor pluginDescriptor = + pluginManager.loadPlugin(plugin, request.getRepositories(), request.getRepositorySession()); + if (wanted.equals(pluginDescriptor.getGoalPrefix())) { return new DefaultPluginPrefixResult(plugin); } } catch (Exception e) { @@ -149,6 +179,44 @@ private PluginPrefixResult resolveFromProject(PluginPrefixRequest request, List< return null; } + /** + * Heuristic to guess the goal prefix from plugin artifactId without resolving the plugin. + * This reduces descriptor loads for obviously non-matching plugins while keeping a full fallback scan. + */ + private boolean isLikelyPrefixFor(Plugin plugin, String wanted) { + if (plugin == null || wanted == null) { + return false; + } + String aid = plugin.getArtifactId(); + if (aid == null) { + return false; + } + // direct quick matches + if (aid.equals(wanted)) { + return true; + } + if (aid.equals("maven-" + wanted + "-plugin")) { + return true; + } + if (aid.equals(wanted + "-maven-plugin")) { + return true; + } + if (aid.equals(wanted + "-plugin")) { + return true; + } + // derive conventional prefix: strip leading "maven-" and one trailing "-maven-plugin" or "-plugin" + String derived = aid; + if (derived.startsWith("maven-")) { + derived = derived.substring("maven-".length()); + } + if (derived.endsWith("-maven-plugin")) { + derived = derived.substring(0, derived.length() - "-maven-plugin".length()); + } else if (derived.endsWith("-plugin")) { + derived = derived.substring(0, derived.length() - "-plugin".length()); + } + return !derived.isEmpty() && derived.equals(wanted); + } + private PluginPrefixResult resolveFromRepository(PluginPrefixRequest request) { RequestTrace trace = RequestTrace.newChild(null, request); diff --git a/impl/maven-core/src/test/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolverTest.java b/impl/maven-core/src/test/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolverTest.java new file mode 100644 index 000000000000..803e3465e2c3 --- /dev/null +++ b/impl/maven-core/src/test/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolverTest.java @@ -0,0 +1,157 @@ +/* + * 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.plugin.prefix.internal; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.maven.artifact.repository.metadata.io.MetadataReader; +import org.apache.maven.model.Build; +import org.apache.maven.model.Model; +import org.apache.maven.model.Plugin; +import org.apache.maven.plugin.BuildPluginManager; +import org.apache.maven.plugin.descriptor.PluginDescriptor; +import org.apache.maven.plugin.prefix.DefaultPluginPrefixRequest; +import org.apache.maven.plugin.prefix.PluginPrefixRequest; +import org.eclipse.aether.RepositorySystem; +import org.eclipse.aether.RepositorySystemSession; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DefaultPluginPrefixResolverTest { + + @Mock + BuildPluginManager pluginManager; + + @Mock + RepositorySystem repositorySystem; + + @Mock + MetadataReader metadataReader; + + DefaultPluginPrefixResolver resolver; + + @BeforeEach + void setUp() { + resolver = new DefaultPluginPrefixResolver(pluginManager, repositorySystem, metadataReader); + } + + private static PluginDescriptor descriptorWithPrefix(String prefix) { + PluginDescriptor pd = new PluginDescriptor(); + pd.setGoalPrefix(prefix); + return pd; + } + + private static Plugin plugin(String groupId, String artifactId) { + Plugin p = new Plugin(); + p.setGroupId(groupId); + p.setArtifactId(artifactId); + return p; + } + + private static PluginPrefixRequest request(String prefix, Model model) { + RepositorySystemSession repoSession = org.mockito.Mockito.mock(RepositorySystemSession.class); + return new DefaultPluginPrefixRequest() + .setPrefix(prefix) + .setPom(model) + .setRepositories(Collections.emptyList()) + .setRepositorySession(repoSession) + .setPluginGroups(Collections.emptyList()); + } + + @Test + void resolvesUsingHeuristicWithoutLoadingAllPlugins() throws Exception { + // Given a project with many plugins and one obvious candidate by artifactId + String wanted = "my-prefix"; + Plugin candidate = plugin("org.acme", "maven-" + wanted + "-plugin"); + + List plugins = new ArrayList<>(); + // add non-candidates + Plugin nc1 = plugin("org.foo", "random-plugin-1"); + Plugin nc2 = plugin("org.bar", "something-else"); + Plugin nc3 = plugin("org.baz", "another-plugin"); + plugins.add(nc1); + plugins.add(nc2); + plugins.add(candidate); + plugins.add(nc3); + + Model model = new Model(); + Build build = new Build(); + build.setPlugins(plugins); + model.setBuild(build); + + // Only the candidate plugin should be loaded + when(pluginManager.loadPlugin(eq(candidate), anyList(), any())).thenReturn(descriptorWithPrefix(wanted)); + + // When + var result = resolver.resolve(request(wanted, model)); + + // Then + assertNotNull(result); + assertEquals(candidate.getGroupId(), result.getGroupId()); + assertEquals(candidate.getArtifactId(), result.getArtifactId()); + + // Verify only the candidate was loaded + verify(pluginManager, times(1)).loadPlugin(eq(candidate), anyList(), any()); + verify(pluginManager, never()).loadPlugin(eq(nc1), anyList(), any()); + verify(pluginManager, never()).loadPlugin(eq(nc2), anyList(), any()); + verify(pluginManager, never()).loadPlugin(eq(nc3), anyList(), any()); + } + + @Test + void fallsBackToFullScanForCustomGoalPrefix() throws Exception { + // Given a plugin whose artifactId does not imply the requested prefix + String wanted = "custom"; + Plugin odd = plugin("org.acme", "strange-artifact"); + + Model model = new Model(); + Build build = new Build(); + build.setPlugins(Collections.singletonList(odd)); + model.setBuild(build); + + // Heuristic will not select this as candidate, but fallback full scan should load and match by descriptor + when(pluginManager.loadPlugin(eq(odd), anyList(), any())).thenReturn(descriptorWithPrefix(wanted)); + + // When + var result = resolver.resolve(request(wanted, model)); + + // Then + assertNotNull(result); + assertEquals(odd.getGroupId(), result.getGroupId()); + assertEquals(odd.getArtifactId(), result.getArtifactId()); + + // Verify it was loaded exactly once via fallback + verify(pluginManager, times(1)).loadPlugin(eq(odd), anyList(), any()); + } +}