Merge pull request #144 from nebula-plugins/plugin-enhancements

Modernize plugin: remove technical debt and adopt Provider API

Author: rpalcolea

src/main/groovy/netflix/nebula/dependency/recommender/DependencyRecommendationsPlugin.java

@@ -43,7 +43,6 @@
 import org.gradle.api.provider.Provider;
 import org.gradle.util.GradleVersion;
 
-import java.lang.reflect.Method;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -129,9 +128,6 @@ public Unit invoke(ResolvableDependencies resolvableDependencies) {
 
                             for (Dependency dependency : resolvableDependencies.getDependencies()) {
                                 applyRecommendationToDependency(rsFactory, dependency, new ArrayList<ProjectDependency>(), project);
-
-                                // if project dependency, pull all first orders and apply recommendations if missing dependency versions
-                                // dependency.getProjectConfiguration().allDependencies iterate and inspect them as well
                             }
 
                             conf.getResolutionStrategy().eachDependency(new Action<DependencyResolveDetails>() {
@@ -156,7 +152,7 @@ public void execute(DependencyResolveDetails details) {
                                             details.because("Recommending version " + version + " for dependency " + coordinate + " via " + strategyText + "\n" +
                                                     "\twith reasons: " + StringUtils.join(getReasonsRecursive(project), ", "));
                                         } else {
-                                            if (recommendationProviderContainer.getStrictMode()) {
+                                            if (recommendationProviderContainer.getStrictMode().get()) {
                                                 String errorMessage = "Dependency " + details.getRequested().getGroup() + ":" + details.getRequested().getName() + " omitted version with no recommended version. General causes include a dependency being removed from the recommendation source or not applying a recommendation source to a project that depends on another project using a recommender.";
                                                 project.getLogger().error(errorMessage);
                                                 throw new GradleException(errorMessage);
@@ -174,11 +170,11 @@ public void execute(DependencyResolveDetails details) {
     }
 
     private boolean isExcludedConfiguration(String confName) {
-        if (recommendationProviderContainer.getExcludedConfigurations().contains(confName)) {
+        if (recommendationProviderContainer.getExcludedConfigurations().get().contains(confName)) {
             return true;
         }
 
-        for (String prefix : recommendationProviderContainer.getExcludedConfigurationPrefixes()) {
+        for (String prefix : recommendationProviderContainer.getExcludedConfigurationPrefixes().get()) {
             if (confName.startsWith(prefix)) {
                 return true;
             }
@@ -194,25 +190,11 @@ private void applyRecommendationToDependency(final RecommendationStrategyFactory
             ProjectDependency projectDependency = (ProjectDependency) dependency;
             if (!visited.contains(projectDependency)) {
                 visited.add(projectDependency);
-                final Configuration[] configuration = new Configuration[1];
-                try {
-                    ProjectDependency.class.getMethod("getTargetConfiguration");
-                    String targetConfiguration = projectDependency.getTargetConfiguration() == null ? Dependency.DEFAULT_CONFIGURATION : projectDependency.getTargetConfiguration();
-                    Project dependencyProject = rootProject.findProject(projectDependency.getPath());
-                    if (dependencyProject != null) {
-                        configuration[0] = dependencyProject.getConfigurations().getByName(targetConfiguration);
-                    }
-
-                } catch (NoSuchMethodException ignore) {
-                    try {
-                        Method method = ProjectDependency.class.getMethod("getProjectConfiguration");
-                        configuration[0] = (Configuration) method.invoke(dependency);
-                    } catch (Exception e) {
-                        throw new RuntimeException("Unable to retrieve configuration for project dependency", e);
-                    }
-                }
-                if (configuration[0] != null) {
-                    DependencySet dependencies = configuration[0].getAllDependencies();
+                String targetConfiguration = projectDependency.getTargetConfiguration() == null ? Dependency.DEFAULT_CONFIGURATION : projectDependency.getTargetConfiguration();
+                Project dependencyProject = rootProject.findProject(projectDependency.getPath());
+                if (dependencyProject != null) {
+                    Configuration configuration = dependencyProject.getConfigurations().getByName(targetConfiguration);
+                    DependencySet dependencies = configuration.getAllDependencies();
                     for (Dependency dep : dependencies) {
                         applyRecommendationToDependency(factory, dep, visited, rootProject);
                     }

src/main/groovy/netflix/nebula/dependency/recommender/ExtendRecommenderConfigurationAction.java

@@ -40,7 +40,7 @@ public ExtendRecommenderConfigurationAction(Configuration bom, Project project,
 
     @Override
     public void execute(Configuration configuration) {
-        if (!isClasspathConfiguration(configuration) || container.getExcludedConfigurations().contains(configuration.getName()) || isCopyOfBomConfiguration(configuration)) {
+        if (!isClasspathConfiguration(configuration) || container.getExcludedConfigurations().get().contains(configuration.getName()) || isCopyOfBomConfiguration(configuration)) {
             return;
         }
 
@@ -98,8 +98,14 @@ private DefaultConfiguration createCopy(Set<Dependency> dependencies, Set<Depend
 
     private String getNameWithCopySuffix() {
         int count = this.copyCount.incrementAndGet();
-        String copyName = bom.getName() + "Copy";
-        return count == 1 ? copyName : copyName + count;
+        // Include project path to ensure uniqueness across composite builds and subprojects
+        // Replace colons with underscores to avoid configuration naming issues
+        String projectPathSuffix = project.getPath().replace(":", "_");
+        if (projectPathSuffix.isEmpty() || projectPathSuffix.equals("_")) {
+            projectPathSuffix = "root";
+        }
+        String copyName = bom.getName() + "Copy_" + projectPathSuffix;
+        return count == 1 ? copyName : copyName + "_" + count;
     }
 
     //we want to apply recommendation only into final resolvable configurations like `compileClasspath` or `runtimeClasspath` across all source sets.

src/main/groovy/netflix/nebula/dependency/recommender/RecommendationStrategyFactory.java

@@ -21,7 +21,7 @@ public RecommendationStrategy getRecommendationStrategy() {
         if(recommendationStrategy == null) {
             try {
                 RecommendationProviderContainer recommendationProviderContainer = project.getExtensions().findByType(RecommendationProviderContainer.class);
-                recommendationStrategy = Objects.requireNonNull(recommendationProviderContainer).getStrategy().getStrategyClass().newInstance();
+                recommendationStrategy = Objects.requireNonNull(recommendationProviderContainer).getStrategy().get().getStrategyClass().newInstance();
             } catch (Exception e) {
                 throw new IllegalStateException(e);
             }

src/main/groovy/netflix/nebula/dependency/recommender/provider/RecommendationProviderContainer.java

@@ -24,6 +24,8 @@
 import org.gradle.api.artifacts.Dependency;
 import org.gradle.api.internal.ConfigureByMapAction;
 import org.gradle.api.model.ObjectFactory;
+import org.gradle.api.provider.Property;
+import org.gradle.api.provider.SetProperty;
 
 import java.io.File;
 import java.lang.reflect.InvocationTargetException;
@@ -36,13 +38,13 @@ public class RecommendationProviderContainer {
 
     private Project project;
     private NamedDomainObjectList<RecommendationProvider> providers;
-    private RecommendationStrategies strategy = RecommendationStrategies.ConflictResolved;
+    private final Property<RecommendationStrategies> strategy;
     private MavenBomRecommendationProvider mavenBomProvider;
-    private Boolean strictMode = false;
-    private Set<String> excludedConfigurations = new HashSet<>();
-    private Set<String> excludedConfigurationPrefixes = new HashSet<>();
-    private Set<String> reasons = new HashSet<>();
-    private Boolean eagerlyResolve = true;
+    private final Property<Boolean> strictMode;
+    private final SetProperty<String> excludedConfigurations;
+    private final SetProperty<String> excludedConfigurationPrefixes;
+    private Set<String> reasons = new HashSet<>(); // Keep as regular Set - it's an output/result collection
+    private final Property<Boolean> eagerlyResolve;
 
     // Make strategies available without import
     public static final RecommendationStrategies OverrideTransitives = RecommendationStrategies.OverrideTransitives;
@@ -51,6 +53,20 @@ public class RecommendationProviderContainer {
     public RecommendationProviderContainer(Project project) {
         createList(project);
         this.project = project;
+
+        // Initialize properties using ObjectFactory for proper Gradle integration
+        ObjectFactory objects = project.getObjects();
+        this.strategy = objects.property(RecommendationStrategies.class)
+                .convention(RecommendationStrategies.ConflictResolved);
+        this.strictMode = objects.property(Boolean.class)
+                .convention(false);
+        this.excludedConfigurations = objects.setProperty(String.class)
+                .convention(new HashSet<>());
+        this.excludedConfigurationPrefixes = objects.setProperty(String.class)
+                .convention(new HashSet<>());
+        this.eagerlyResolve = objects.property(Boolean.class)
+                .convention(true);
+
         this.mavenBomProvider = getMavenBomRecommendationProvider();
         providers.add(this.mavenBomProvider);
     }
@@ -214,83 +230,71 @@ public String getRecommendedVersion(String group, String name) {
         return null;
     }
 
-    public RecommendationStrategies getStrategy() {
+    public Property<RecommendationStrategies> getStrategy() {
         return strategy;
     }
 
-    public void setStrategy(RecommendationStrategies strategy) {
-        this.strategy = strategy;
-    }
-
+    /**
+     * @deprecated Use {@link #getStrategy()}.set() instead
+     */
     @Deprecated
-    public Boolean isStrictMode() {
-        return getStrictMode();
+    public void setStrategy(RecommendationStrategies value) {
+        strategy.set(value);
     }
 
-    public Boolean getStrictMode() {
+    public Property<Boolean> getStrictMode() {
         return strictMode;
     }
 
-    public void setStrictMode(Boolean strict) {
-        strictMode = strict;
+    /**
+     * @deprecated Use {@link #getStrictMode()}.set() instead
+     */
+    @Deprecated
+    public void setStrictMode(Boolean value) {
+        strictMode.set(value);
     }
 
     /**
-     * Sets whether BOM configurations should be resolved eagerly during the configuration phase.
-     * 
-     * <p>When set to {@code true} (default), BOM configurations will be resolved automatically 
-     * during the {@code afterEvaluate} phase to prevent configuration resolution lock conflicts 
+     * Returns the Property controlling whether BOM configurations should be resolved eagerly.
+     *
+     * <p>When set to {@code true} (default), BOM configurations will be resolved automatically
+     * during the {@code afterEvaluate} phase to prevent configuration resolution lock conflicts
      * in parallel builds with Gradle 9+.</p>
-     * 
+     *
      * <p>When set to {@code false}, external plugins can take control of BOM resolution timing
-     * by calling {@link netflix.nebula.dependency.recommender.util.BomResolutionUtil#eagerlyResolveBoms} 
+     * by calling {@link netflix.nebula.dependency.recommender.util.BomResolutionUtil#eagerlyResolveBoms}
      * manually after modifying BOM configurations.</p>
-     * 
-     * <p><strong>Usage by External Plugins:</strong></p>
-     * <pre>{@code
-     * // Disable automatic eager resolution
-     * dependencyRecommendations {
-     *     setEagerlyResolve(false)
-     *     
-     *     // Add initial BOMs
-     *     mavenBom module: 'com.example:base-bom:1.0.0'
-     * }
-     * 
-     * project.afterEvaluate { p ->
-     *     def container = p.extensions.getByType(RecommendationProviderContainer)
-     *     
-     *     // Add additional BOMs dynamically
-     *     container.mavenBom(module: 'com.example:dynamic-bom:2.0.0')
-     *     
-     *     // Manually trigger resolution
-     *     BomResolutionUtil.eagerlyResolveBoms(p, container, 'nebulaRecommenderBom')
-     * }
-     * }</pre>
-     * 
-     * @param eagerlyResolve {@code true} to enable automatic eager resolution, 
-     *                       {@code false} to disable it and allow manual control
+     *
+     * @return Property containing the eagerly resolve flag
      * @since 12.7.0
      * @see netflix.nebula.dependency.recommender.util.BomResolutionUtil#eagerlyResolveBoms
      */
-    public void setEagerlyResolve(Boolean eagerlyResolve) {
-        this.eagerlyResolve = eagerlyResolve;
+    public Property<Boolean> getEagerlyResolve() {
+        return eagerlyResolve;
     }
 
     /**
-     * Returns whether BOM configurations should be resolved eagerly during the configuration phase.
-     * 
-     * <p>This setting controls whether the dependency recommender plugin automatically resolves
-     * BOM configurations during {@code afterEvaluate}, or whether external plugins should
-     * handle resolution timing manually.</p>
-     * 
-     * @return {@code true} if BOMs should be resolved eagerly (default), 
+     * Convenience method to check if BOMs should be resolved eagerly.
+     *
+     * @return {@code true} if BOMs should be resolved eagerly (default),
      *         {@code false} if resolution should be handled manually
      * @since 12.7.0
-     * @see #setEagerlyResolve(Boolean)
-     * @see netflix.nebula.dependency.recommender.util.BomResolutionUtil#shouldEagerlyResolveBoms
      */
     public Boolean shouldEagerlyResolve() {
-        return eagerlyResolve;
+        return eagerlyResolve.get();
+    }
+
+    /**
+     * Convenience method to set whether BOMs should be resolved eagerly.
+     *
+     * @param value {@code true} to enable automatic eager resolution,
+     *              {@code false} to disable it and allow manual control
+     * @since 12.7.0
+     * @deprecated Use {@link #getEagerlyResolve()}.set() instead
+     */
+    @Deprecated
+    public void setEagerlyResolve(Boolean value) {
+        eagerlyResolve.set(value);
     }
 
     public void excludeConfigurations(String ... names) {
@@ -301,11 +305,11 @@ public void excludeConfigurationPrefixes(String ... names) {
         excludedConfigurationPrefixes.addAll(Arrays.asList(names));
     }
 
-    public Set<String> getExcludedConfigurations() {
+    public SetProperty<String> getExcludedConfigurations() {
         return excludedConfigurations;
     }
 
-    public Set<String> getExcludedConfigurationPrefixes() {
+    public SetProperty<String> getExcludedConfigurationPrefixes() {
         return excludedConfigurationPrefixes;
     }