Address review feedback

Author: breskeby

buildSrc/src/main/java/org/elasticsearch/gradle/DistributionDownloadPlugin.java

@@ -119,7 +119,7 @@ public static NamedDomainObjectContainer<DistributionResolution> getRegistration
         return (NamedDomainObjectContainer<DistributionResolution>) project.getExtensions().getByName(RESOLUTION_CONTAINER_NAME);
     }
 
-    private ElasticsearchDistribution finalizeDistributionDependencies(Project project, ElasticsearchDistribution distribution) {
+    private void finalizeDistributionDependencies(Project project, ElasticsearchDistribution distribution) {
         DependencyHandler dependencies = project.getDependencies();
         // for the distribution as a file, just depend on the artifact directly
         DistributionDependency distributionDependency = resolveDependencyNotation(project, distribution);
@@ -130,7 +130,6 @@ private ElasticsearchDistribution finalizeDistributionDependencies(Project proje
             // an artifact transform registered to resolve it as an unpacked folder.
             dependencies.add(distribution.getExtracted().getName(), distributionDependency.getExtractedNotation());
         }
-        return distribution;
     }
 
     private DistributionDependency resolveDependencyNotation(Project p, ElasticsearchDistribution distribution) {

buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchDistribution.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.gradle;
 
 import org.elasticsearch.gradle.docker.DockerSupportService;
+import org.gradle.api.Action;
 import org.gradle.api.Buildable;
 import org.gradle.api.artifacts.Configuration;
 import org.gradle.api.model.ObjectFactory;
@@ -31,8 +32,6 @@
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.Locale;
-import java.util.Optional;
-import java.util.function.Function;
 
 public class ElasticsearchDistribution implements Buildable, Iterable<File> {
 
@@ -105,7 +104,7 @@ public String toString() {
     private final Property<Boolean> bundledJdk;
     private final Property<Boolean> failIfUnavailable;
     private final Configuration extracted;
-    private Function<ElasticsearchDistribution, ElasticsearchDistribution> distributionFinalizer;
+    private Action<ElasticsearchDistribution> distributionFinalizer;
     private boolean froozen = false;
 
     ElasticsearchDistribution(
@@ -114,7 +113,7 @@ public String toString() {
         Provider<DockerSupportService> dockerSupport,
         Configuration fileConfiguration,
         Configuration extractedConfiguration,
-        Function<ElasticsearchDistribution, ElasticsearchDistribution> distributionFinalizer
+        Action<ElasticsearchDistribution> distributionFinalizer
     ) {
         this.name = name;
         this.dockerSupport = dockerSupport;
@@ -210,7 +209,7 @@ public String toString() {
     public ElasticsearchDistribution maybeFreeze() {
         if (!froozen) {
             finalizeValues();
-            distributionFinalizer.apply(this);
+            distributionFinalizer.execute(this);
             froozen = true;
         }
         return this;
@@ -238,26 +237,16 @@ public Configuration getExtracted() {
 
     @Override
     public TaskDependency getBuildDependencies() {
-        Optional<TaskDependency> dockerBuildDependencies = dockerBuildDependencies();
-        return dockerBuildDependencies.orElseGet(() -> {
+        if (skippingDockerDistributionBuild()) {
+            return task -> Collections.emptySet();
+        } else {
             maybeFreeze();
             return getType().shouldExtract() ? extracted.getBuildDependencies() : configuration.getBuildDependencies();
-        });
-    }
-
-    public TaskDependency getArchiveBuildDependencies() {
-        Optional<TaskDependency> dockerBuildDependencies = dockerBuildDependencies();
-        return dockerBuildDependencies.orElseGet(() -> {
-            maybeFreeze();
-            return configuration.getBuildDependencies();
-        });
+        }
     }
 
-    private Optional<TaskDependency> dockerBuildDependencies() {
-        // For non-required Docker distributions, skip building the distribution is Docker is unavailable
-        return (isDocker() && getFailIfUnavailable() == false && dockerSupport.get().getDockerAvailability().isAvailable == false)
-            ? Optional.of(task -> Collections.emptySet())
-            : Optional.empty();
+    private boolean skippingDockerDistributionBuild() {
+        return isDocker() && getFailIfUnavailable() == false && dockerSupport.get().getDockerAvailability().isAvailable == false;
     }
 
     @Override
@@ -329,4 +318,9 @@ void finalizeValues() {
         flavor.finalizeValue();
         bundledJdk.finalizeValue();
     }
+
+    public Configuration getArchive() {
+        maybeFreeze();
+        return configuration;
+    }
 }

buildSrc/src/main/java/org/elasticsearch/gradle/test/DistroTestPlugin.java

@@ -114,7 +114,7 @@ public void apply(Project project) {
             String taskname = destructiveDistroTestTaskName(distribution);
             TaskProvider<?> depsTask = project.getTasks().register(taskname + "#deps");
             // explicitly depend on the archive not on the implicit extracted distribution
-            depsTask.configure(t -> t.dependsOn(distribution.getArchiveBuildDependencies(), examplePlugin, quotaAwareFsPlugin));
+            depsTask.configure(t -> t.dependsOn(distribution.getArchive(), examplePlugin, quotaAwareFsPlugin));
             depsTasks.put(taskname, depsTask);
             TaskProvider<Test> destructiveTask = configureTestTask(project, taskname, distribution, t -> {
                 t.onlyIf(t2 -> distribution.isDocker() == false || dockerSupport.get().getDockerAvailability().isAvailable);