Split PluginRepositoryFactory from Registry

subprojects/core/src/main/java/org/gradle/api/internal/plugins/repositories/DefaultPluginRepositorySpec.java

@@ -24,26 +24,34 @@
  * Bridges between a global PluginRepositorySpec and a {@link org.gradle.api.Script}.
  */
 public class DefaultPluginRepositorySpec implements PluginRepositorySpec {
+    private final PluginRepositoryFactory pluginRepositoryFactory;
     private final PluginRepositoryRegistry pluginRepositoryRegistry;
     private final FileResolver fileResolver;
 
-    public DefaultPluginRepositorySpec(PluginRepositoryRegistry pluginRepositoryRegistry, FileResolver fileResolver) {
+    public DefaultPluginRepositorySpec(PluginRepositoryFactory pluginRepositoryFactory, PluginRepositoryRegistry pluginRepositoryRegistry, FileResolver fileResolver) {
+        this.pluginRepositoryFactory = pluginRepositoryFactory;
         this.pluginRepositoryRegistry = pluginRepositoryRegistry;
         this.fileResolver = fileResolver;
     }
 
     @Override
     public MavenPluginRepository maven(Action<? super MavenPluginRepository> action) {
-        return pluginRepositoryRegistry.maven(action, fileResolver);
+        MavenPluginRepository repo = pluginRepositoryFactory.maven(action, fileResolver);
+        pluginRepositoryRegistry.add(repo);
+        return repo;
     }
 
     @Override
     public IvyPluginRepository ivy(Action<? super IvyPluginRepository> action) {
-        return pluginRepositoryRegistry.ivy(action, fileResolver);
+        IvyPluginRepository repo = pluginRepositoryFactory.ivy(action, fileResolver);
+        pluginRepositoryRegistry.add(repo);
+        return repo;
     }
 
     @Override
     public GradlePluginPortal gradlePluginPortal() {
-        return pluginRepositoryRegistry.gradlePluginPortal();
+        GradlePluginPortal portal = pluginRepositoryFactory.gradlePluginPortal();
+        pluginRepositoryRegistry.add(portal);
+        return portal;
     }
 }

subprojects/core/src/main/java/org/gradle/api/internal/plugins/repositories/PluginRepositoryFactory.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2016 the original author or authors.
+ *
+ * Licensed 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.gradle.api.internal.plugins.repositories;
+
+import org.gradle.api.Action;
+import org.gradle.api.internal.file.FileResolver;
+
+/**
+ * Created by pepper on 5/4/16.
+ */
+public interface PluginRepositoryFactory {
+    /**
+     * Adds and configures a {@link MavenPluginRepository}.
+     *
+     * @param action The action to use to configure the repository.
+     * @param scriptFileResolver The {@link FileResolver} in the context of the Script.
+     * @return The added repository.
+     */
+    MavenPluginRepository maven(Action<? super MavenPluginRepository> action, FileResolver scriptFileResolver);
+
+    /**
+     * Adds and configures a {@link IvyPluginRepository}.
+     *
+     * @param action The action to use to configure the repository.
+     * @param scriptFileResolver The {@link FileResolver} in the context of the Script.
+     * @return the added repository.
+     */
+    IvyPluginRepository ivy(Action<? super IvyPluginRepository> action, FileResolver scriptFileResolver);
+
+    /**
+     * Adds the Gradle Plugin Portal (plugins.gradle.org) as a plugin repository.
+     * @return The added repository.
+     * @throws IllegalArgumentException if called more than once.
+     */
+    GradlePluginPortal gradlePluginPortal();
+}

subprojects/core/src/main/java/org/gradle/api/internal/plugins/repositories/PluginRepositoryRegistry.java

@@ -16,35 +16,12 @@
 
 package org.gradle.api.internal.plugins.repositories;
 
-import org.gradle.api.Action;
-import org.gradle.api.internal.file.FileResolver;
+import org.gradle.internal.HasInternalProtocol;
 
 /**
  * The {@link PluginRepository} instances used to resolve plugins.
  */
-public interface PluginRepositoryRegistry extends Iterable<PluginRepository> {
-    /**
-     * Adds and configures a {@link MavenPluginRepository}.
-     *
-     * @param action The action to use to configure the repository.
-     * @param scriptFileResolver The {@link FileResolver} in the context of the Script.
-     * @return The added repository.
-     */
-    MavenPluginRepository maven(Action<? super MavenPluginRepository> action, FileResolver scriptFileResolver);
-
-    /**
-     * Adds and configures a {@link IvyPluginRepository}.
-     *
-     * @param action The action to use to configure the repository.
-     * @param scriptFileResolver The {@link FileResolver} in the context of the Script.
-     * @return the added repository.
-     */
-    IvyPluginRepository ivy(Action<? super IvyPluginRepository> action, FileResolver scriptFileResolver);
-
-    /**
-     * Adds the Gradle Plugin Portal (plugins.gradle.org) as a plugin repository.
-     * @return The added repository.
-     * @throws IllegalArgumentException if called more than once.
-     */
-    GradlePluginPortal gradlePluginPortal();
+@HasInternalProtocol
+public interface PluginRepositoryRegistry {
+    void add(PluginRepository pluginRepository);
 }

subprojects/core/src/main/java/org/gradle/configuration/DefaultScriptPluginFactory.java

@@ -27,6 +27,7 @@
 import org.gradle.api.internal.initialization.ScriptHandlerFactory;
 import org.gradle.api.internal.initialization.ScriptHandlerInternal;
 import org.gradle.api.internal.plugins.PluginManagerInternal;
+import org.gradle.api.internal.plugins.repositories.PluginRepositoryFactory;
 import org.gradle.api.internal.plugins.repositories.PluginRepositoryRegistry;
 import org.gradle.api.internal.project.ProjectInternal;
 import org.gradle.api.tasks.util.PatternSet;
@@ -69,6 +70,7 @@ public class DefaultScriptPluginFactory implements ScriptPluginFactory {
     private final BuildScriptDataSerializer buildScriptDataSerializer = new BuildScriptDataSerializer();
     private final PluginRequestsSerializer pluginRequestsSerializer = new PluginRequestsSerializer();
     private final PluginRepositoryRegistry pluginRepositoryRegistry;
+    private final PluginRepositoryFactory pluginRepositoryFactory;
 
     public DefaultScriptPluginFactory(ScriptCompilerFactory scriptCompilerFactory,
                                       Factory<LoggingManagerInternal> loggingManagerFactory,
@@ -79,7 +81,8 @@ public DefaultScriptPluginFactory(ScriptCompilerFactory scriptCompilerFactory,
                                       DirectoryFileTreeFactory directoryFileTreeFactory,
                                       DocumentationRegistry documentationRegistry,
                                       ModelRuleSourceDetector modelRuleSourceDetector,
-                                      PluginRepositoryRegistry pluginRepositoryRegistry) {
+                                      PluginRepositoryRegistry pluginRepositoryRegistry,
+                                      PluginRepositoryFactory pluginRepositoryFactory) {
         this.scriptCompilerFactory = scriptCompilerFactory;
         this.loggingManagerFactory = loggingManagerFactory;
         this.instantiator = instantiator;
@@ -90,6 +93,7 @@ public DefaultScriptPluginFactory(ScriptCompilerFactory scriptCompilerFactory,
         this.documentationRegistry = documentationRegistry;
         this.modelRuleSourceDetector = modelRuleSourceDetector;
         this.pluginRepositoryRegistry = pluginRepositoryRegistry;
+        this.pluginRepositoryFactory = pluginRepositoryFactory;
     }
 
     public ScriptPlugin create(ScriptSource scriptSource, ScriptHandler scriptHandler, ClassLoaderScope targetScope, ClassLoaderScope baseScope, boolean topLevelScript) {
@@ -131,6 +135,7 @@ Factory<PatternSet> createPatternSetFactory() {
             services.add(DirectoryFileTreeFactory.class, directoryFileTreeFactory);
             services.add(ModelRuleSourceDetector.class, modelRuleSourceDetector);
             services.add(PluginRepositoryRegistry.class, pluginRepositoryRegistry);
+            services.add(PluginRepositoryFactory.class, pluginRepositoryFactory);
 
             final ScriptTarget initialPassScriptTarget = initialPassTarget(target);
 

subprojects/core/src/main/java/org/gradle/initialization/InitialPassSettingsScript.java

@@ -17,6 +17,7 @@
 
 import groovy.lang.Closure;
 import org.gradle.api.internal.plugins.dsl.PluginRepositorySpec;
+import org.gradle.api.internal.plugins.repositories.PluginRepositoryFactory;
 import org.gradle.api.internal.plugins.repositories.PluginRepositoryRegistry;
 import org.gradle.api.internal.plugins.repositories.DefaultPluginRepositorySpec;
 import org.gradle.internal.reflect.Instantiator;
@@ -29,9 +30,10 @@
 public abstract class InitialPassSettingsScript extends SettingsScript {
     private PluginRepositorySpec getPluginRepositorySpec() {
         Instantiator instantiator = __scriptServices.get(Instantiator.class);
+        PluginRepositoryFactory pluginRepositoryFactory = __scriptServices.get(PluginRepositoryFactory.class);
         PluginRepositoryRegistry pluginRepositoryRegistry = __scriptServices.get(PluginRepositoryRegistry.class);
         return instantiator.newInstance(
-            DefaultPluginRepositorySpec.class, pluginRepositoryRegistry, getFileResolver());
+            DefaultPluginRepositorySpec.class, pluginRepositoryFactory, pluginRepositoryRegistry, getFileResolver());
     }
 
     public void pluginRepositories(Closure config) {

subprojects/core/src/main/java/org/gradle/internal/service/scopes/BuildScopeServices.java

@@ -45,6 +45,7 @@
 import org.gradle.api.internal.plugins.DefaultPluginRegistry;
 import org.gradle.api.internal.plugins.PluginInspector;
 import org.gradle.api.internal.plugins.PluginRegistry;
+import org.gradle.api.internal.plugins.repositories.PluginRepositoryFactory;
 import org.gradle.api.internal.plugins.repositories.PluginRepositoryRegistry;
 import org.gradle.api.internal.project.DefaultProjectAccessListener;
 import org.gradle.api.internal.project.DefaultProjectRegistry;
@@ -287,7 +288,8 @@ private DefaultScriptPluginFactory defaultScriptPluginFactory() {
             get(DirectoryFileTreeFactory.class),
             get(DocumentationRegistry.class),
             get(ModelRuleSourceDetector.class),
-            get(PluginRepositoryRegistry.class));
+            get(PluginRepositoryRegistry.class),
+            get(PluginRepositoryFactory.class));
     }
 
     protected SettingsLoader createSettingsLoader(SettingsProcessor settingsProcessor, GradleLauncherFactory gradleLauncherFactory,

subprojects/core/src/test/groovy/org/gradle/configuration/DefaultScriptPluginFactoryTest.groovy

@@ -26,6 +26,7 @@ import org.gradle.api.internal.initialization.ScriptHandlerFactory
 import org.gradle.api.internal.initialization.ScriptHandlerInternal
 import org.gradle.api.internal.initialization.loadercache.ClassPathSnapshot
 import org.gradle.api.internal.initialization.loadercache.ClassPathSnapshotter
+import org.gradle.api.internal.plugins.repositories.PluginRepositoryFactory
 import org.gradle.api.internal.plugins.repositories.PluginRepositoryRegistry
 import org.gradle.api.internal.project.ProjectInternal
 import org.gradle.api.internal.project.ProjectScript
@@ -65,9 +66,10 @@ public class DefaultScriptPluginFactoryTest extends Specification {
     def documentationRegistry = Mock(DocumentationRegistry)
     def classPathSnapshotter = Mock(ClassPathSnapshotter)
     def pluginRepositoryRegistry = Mock(PluginRepositoryRegistry)
+    def pluginRepositoryFactory = Mock(PluginRepositoryFactory)
 
     def factory = new DefaultScriptPluginFactory(scriptCompilerFactory, loggingManagerFactory, instantiator, scriptHandlerFactory, pluginRequestApplicator, fileLookup,
-        directoryFileTreeFactory, documentationRegistry, new ModelRuleSourceDetector(), pluginRepositoryRegistry)
+        directoryFileTreeFactory, documentationRegistry, new ModelRuleSourceDetector(), pluginRepositoryRegistry, pluginRepositoryFactory)
 
     def setup() {
         def configurations = Mock(ConfigurationContainer)

subprojects/core/src/test/groovy/org/gradle/internal/service/scopes/BuildScopeServicesTest.groovy

@@ -28,6 +28,7 @@ import org.gradle.api.internal.file.FileResolver
 import org.gradle.api.internal.file.collections.DirectoryFileTreeFactory
 import org.gradle.api.internal.initialization.loadercache.ClassLoaderCache
 import org.gradle.api.internal.initialization.loadercache.ClassPathSnapshotter
+import org.gradle.api.internal.plugins.repositories.PluginRepositoryFactory
 import org.gradle.api.internal.plugins.repositories.PluginRepositoryRegistry
 import org.gradle.api.internal.project.*
 import org.gradle.api.internal.project.antbuilder.DefaultIsolatedAntBuilder
@@ -94,6 +95,7 @@ public class BuildScopeServicesTest extends Specification {
         sessionServices.get(CrossBuildInMemoryCachingScriptClassCache) >> Mock(CrossBuildInMemoryCachingScriptClassCache)
         sessionServices.get(InjectedPluginClasspath) >> Mock(InjectedPluginClasspath)
         sessionServices.get(PluginRepositoryRegistry) >> Mock(PluginRepositoryRegistry)
+        sessionServices.get(PluginRepositoryFactory) >> Mock(PluginRepositoryFactory)
         sessionServices.getAll(_) >> []
 
         registry = new BuildScopeServices(sessionServices, false)

subprojects/plugin-use/src/integTest/groovy/org/gradle/plugin/use/ResolvingFromMultipleCustomPluginRepositorySpec.groovy

@@ -199,6 +199,33 @@ class ResolvingFromMultipleCustomPluginRepositorySpec extends AbstractDependency
         repoType << [IVY, MAVEN]
     }
 
+    def "Prefers Plugin Repositories over buildscript ones."() {
+        given:
+        publishPlugins(MAVEN)
+        buildScript """
+          buildscript {
+              repositories {
+                  maven {
+                      url "${repoA.uri}"
+                  }
+              }
+              dependencies {
+                  classpath "org.example:pluginAB:1.0"
+              }
+          }
+          plugins {
+               id "$pluginAB" version "1.0"
+          }
+        """
+
+        when:
+        use(repoB)
+
+        then:
+        succeeds("pluginAB")
+        output.contains("fetched from $repoB.uri")
+    }
+
     @Requires(TestPrecondition.ONLINE)
     def "Can opt-in to plugin portal"() {
         given:

subprojects/plugin-use/src/main/java/org/gradle/plugin/use/internal/DefaultPluginRequestApplicator.java

@@ -17,7 +17,6 @@
 package org.gradle.plugin.use.internal;
 
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import org.gradle.api.Action;
@@ -35,20 +34,21 @@
 import org.gradle.api.internal.plugins.PluginManagerInternal;
 import org.gradle.api.internal.plugins.PluginRegistry;
 import org.gradle.api.internal.plugins.repositories.PluginRepository;
-import org.gradle.api.internal.plugins.repositories.PluginRepositoryRegistry;
 import org.gradle.api.plugins.InvalidPluginException;
 import org.gradle.api.plugins.UnknownPluginException;
 import org.gradle.api.specs.Spec;
 import org.gradle.internal.classpath.ClassPath;
 import org.gradle.internal.exceptions.LocationAwareException;
 import org.gradle.plugin.internal.PluginId;
 import org.gradle.plugin.use.repository.internal.BackedByArtifactRepository;
+import org.gradle.plugin.use.repository.internal.PluginRepositoryRegistryInternal;
 import org.gradle.plugin.use.resolve.internal.NotNonCorePluginOnClasspathCheckPluginResolver;
 import org.gradle.plugin.use.resolve.internal.PluginResolution;
 import org.gradle.plugin.use.resolve.internal.PluginResolutionResult;
 import org.gradle.plugin.use.resolve.internal.PluginResolveContext;
 import org.gradle.plugin.use.resolve.internal.PluginResolver;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Formatter;
 import java.util.LinkedList;
@@ -62,9 +62,9 @@
 public class DefaultPluginRequestApplicator implements PluginRequestApplicator {
     private final PluginRegistry pluginRegistry;
     private final PluginResolverFactory pluginResolverFactory;
-    private PluginRepositoryRegistry pluginRepositoryRegistry;
+    private PluginRepositoryRegistryInternal pluginRepositoryRegistry;
 
-    public DefaultPluginRequestApplicator(PluginRegistry pluginRegistry, PluginResolverFactory pluginResolver, PluginRepositoryRegistry pluginRepositoryRegistry) {
+    public DefaultPluginRequestApplicator(PluginRegistry pluginRegistry, PluginResolverFactory pluginResolver, PluginRepositoryRegistryInternal pluginRepositoryRegistry) {
         this.pluginRegistry = pluginRegistry;
         this.pluginResolverFactory = pluginResolver;
         this.pluginRepositoryRegistry = pluginRepositoryRegistry;
@@ -94,15 +94,17 @@ public Result transform(PluginRequest request) {
         final Map<Result, PluginImplementation<?>> pluginImplsFromOtherLoaders = Maps.newLinkedHashMap();
 
         if (!results.isEmpty()) {
+            final RepositoryHandler repositories = scriptHandler.getRepositories();
 
-            List<ArtifactRepository> pluginArtifactRepositories = Lists.newArrayList();
-            for (PluginRepository pluginRepository : pluginRepositoryRegistry) {
+            List<ArtifactRepository> pluginArtifactRepositories = new ArrayList<ArtifactRepository>();
+            pluginRepositoryRegistry.lock();
+            for (PluginRepository pluginRepository : pluginRepositoryRegistry.getPluginRepositories()) {
                 if (pluginRepository instanceof BackedByArtifactRepository) {
-                    pluginArtifactRepositories.add(((BackedByArtifactRepository) pluginRepository).getArtifactRepository());
+                    pluginArtifactRepositories.add(((BackedByArtifactRepository) pluginRepository).createArtifactRepository(repositories));
                 }
             }
-
-            final RepositoryHandler repositories = scriptHandler.getRepositories();
+            // The plugin repositories were prepended as they were added, but we want them at the front.
+            repositories.removeAll(pluginArtifactRepositories);
             repositories.addAll(0, pluginArtifactRepositories);
 
             final List<MavenArtifactRepository> mavenRepos = repositories.withType(MavenArtifactRepository.class);

subprojects/plugin-use/src/main/java/org/gradle/plugin/use/internal/PluginResolverFactory.java

@@ -20,9 +20,9 @@
 import org.gradle.api.internal.DocumentationRegistry;
 import org.gradle.api.internal.plugins.PluginRegistry;
 import org.gradle.api.internal.plugins.repositories.PluginRepository;
-import org.gradle.api.internal.plugins.repositories.PluginRepositoryRegistry;
 import org.gradle.internal.Factory;
 import org.gradle.plugin.use.repository.internal.PluginRepositoryInternal;
+import org.gradle.plugin.use.repository.internal.PluginRepositoryRegistryInternal;
 import org.gradle.plugin.use.resolve.internal.CompositePluginResolver;
 import org.gradle.plugin.use.resolve.internal.CorePluginResolver;
 import org.gradle.plugin.use.resolve.internal.NoopPluginResolver;
@@ -38,14 +38,14 @@ class PluginResolverFactory implements Factory<PluginResolver> {
     private final PluginRegistry pluginRegistry;
     private final DocumentationRegistry documentationRegistry;
     private final PluginResolutionServiceResolver pluginResolutionServiceResolver;
-    private final PluginRepositoryRegistry pluginRepositoryRegistry;
+    private final PluginRepositoryRegistryInternal pluginRepositoryRegistry;
     private final InjectedClasspathPluginResolver injectedClasspathPluginResolver;
 
     PluginResolverFactory(
         PluginRegistry pluginRegistry,
         DocumentationRegistry documentationRegistry,
         PluginResolutionServiceResolver pluginResolutionServiceResolver,
-        PluginRepositoryRegistry pluginRepositoryRegistry,
+        PluginRepositoryRegistryInternal pluginRepositoryRegistry,
         InjectedClasspathPluginResolver injectedClasspathPluginResolver
     ) {
         this.pluginRegistry = pluginRegistry;
@@ -86,12 +86,13 @@ private void addDefaultResolvers(List<PluginResolver> resolvers) {
             resolvers.add(injectedClasspathPluginResolver);
         }
 
-        for (PluginRepository pluginRepository : pluginRepositoryRegistry) {
+        pluginRepositoryRegistry.lock();
+        for (PluginRepository pluginRepository : pluginRepositoryRegistry.getPluginRepositories()) {
             PluginResolver resolver = ((PluginRepositoryInternal) pluginRepository).asResolver();
             resolvers.add(resolver);
         }
 
-        if (Iterables.isEmpty(pluginRepositoryRegistry)) {
+        if (Iterables.isEmpty(pluginRepositoryRegistry.getPluginRepositories())) {
             resolvers.add(pluginResolutionServiceResolver);
         }
     }