refactor(artifacts): Rename ArtifactResolver to ArtifactUtils (#3375)

* refactor(artifacts): Rename ArtifactResolver to ArtifactUtils This class does a lot more than resolve artifacts, and an upcoming commit will pull out the resolving-specific functionality into its own, smaller class. In order to leave the ArtifactResolver class name available for that new class, rename this to ArtifactUtils. In general, *Utils classes tend to get a lot of functionality that should live in better purpose-built classes so are often not a great idea. In this case, the class already has a bunch of functionality so the name accurately describes it; the upcoming commit will pull out one part of the functionality and hopefully future refactors will further pare down this class. * refactor(artifacts): Convert find artifacts test to java An upcoming commit will require some changes to this test; as it's easy to convert it to java (and makes the changes easier) just convert it to java here.
spinnaker · Jan 17, 2020 · bc2226c · bc2226c
1 parent 6ccdc92
commit bc2226c
Show file tree

Hide file tree

Showing 40 changed files with 346 additions and 314 deletions.
diff --git a/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTask.groovy b/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTask.groovy
@@ -29,7 +29,7 @@ import com.netflix.spinnaker.orca.bakery.api.BakeryService
 import com.netflix.spinnaker.orca.front50.Front50Service
 import com.netflix.spinnaker.orca.front50.model.Application
 import com.netflix.spinnaker.orca.pipeline.model.Stage
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils
 import com.netflix.spinnaker.orca.pipeline.util.OperatingSystem
 import com.netflix.spinnaker.orca.pipeline.util.PackageInfo
 import com.netflix.spinnaker.orca.pipeline.util.PackageType
@@ -51,7 +51,7 @@ class CreateBakeTask implements RetryableTask {
   long timeout = 300000
 
   @Autowired
-  ArtifactResolver artifactResolver
+  ArtifactUtils artifactUtils
 
   @Autowired(required = false)
   BakerySelector bakerySelector
@@ -155,7 +155,7 @@ class CreateBakeTask implements RetryableTask {
       packageType = new OperatingSystem(stage.context.baseOs as String).getPackageType()
     }
 
-    List<Artifact> artifacts = artifactResolver.getAllArtifacts(stage.getExecution())
+    List<Artifact> artifacts = artifactUtils.getAllArtifacts(stage.getExecution())
 
     PackageInfo packageInfo = new PackageInfo(stage,
       artifacts,
@@ -170,7 +170,7 @@ class CreateBakeTask implements RetryableTask {
     // if the field "packageArtifactIds" is present in the context, because it was set in the UI,
     // this will resolve those ids into real artifacts and then put them in List<Artifact> packageArtifacts
     requestMap.packageArtifacts = stage.context.packageArtifactIds.collect { String artifactId ->
-      artifactResolver.getBoundArtifactForId(stage, artifactId)
+      artifactUtils.getBoundArtifactForId(stage, artifactId)
     }
 
     // Workaround for deck/titusBakeStage.js historically injecting baseOs=trusty into stage definitions;

diff --git a/...main/groovy/com/netflix/spinnaker/orca/bakery/tasks/manifests/CreateBakeManifestTask.java b/...main/groovy/com/netflix/spinnaker/orca/bakery/tasks/manifests/CreateBakeManifestTask.java
@@ -27,7 +27,7 @@
 import com.netflix.spinnaker.orca.bakery.api.manifests.helm.HelmBakeManifestRequest;
 import com.netflix.spinnaker.orca.bakery.api.manifests.kustomize.KustomizeBakeManifestRequest;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver;
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils;
 import com.netflix.spinnaker.orca.pipeline.util.ContextParameterProcessor;
 import java.util.Collections;
 import java.util.HashMap;
@@ -57,16 +57,16 @@ public long getTimeout() {
 
   @Nullable private final BakeryService bakery;
 
-  private final ArtifactResolver artifactResolver;
+  private final ArtifactUtils artifactUtils;
 
   private final ContextParameterProcessor contextParameterProcessor;
 
   @Autowired
   public CreateBakeManifestTask(
-      ArtifactResolver artifactResolver,
+      ArtifactUtils artifactUtils,
       ContextParameterProcessor contextParameterProcessor,
       Optional<BakeryService> bakery) {
-    this.artifactResolver = artifactResolver;
+    this.artifactUtils = artifactUtils;
     this.contextParameterProcessor = contextParameterProcessor;
     this.bakery = bakery.orElse(null);
   }
@@ -91,7 +91,7 @@ public TaskResult execute(@Nonnull Stage stage) {
             .map(
                 p -> {
                   Artifact a =
-                      artifactResolver.getBoundArtifactForStage(stage, p.getId(), p.getArtifact());
+                      artifactUtils.getBoundArtifactForStage(stage, p.getId(), p.getArtifact());
                   if (a == null) {
                     throw new IllegalArgumentException(
                         String.format(

diff --git a/...-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTaskSpec.groovy b/...-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTaskSpec.groovy
@@ -24,7 +24,7 @@ import com.netflix.spinnaker.orca.bakery.api.BakeStatus
 import com.netflix.spinnaker.orca.bakery.api.BakeryService
 import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper
 import com.netflix.spinnaker.orca.pipeline.model.*
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils
 import retrofit.RetrofitError
 import retrofit.client.Response
 import retrofit.mime.TypedString
@@ -48,7 +48,7 @@ class CreateBakeTaskSpec extends Specification {
   Stage bakeStage
   def mapper = OrcaObjectMapper.newInstance()
 
-  ArtifactResolver artifactResolver = Stub() {
+  ArtifactUtils artifactUtils = Stub() {
     getAllArtifacts(_) >> []
   }
 
@@ -211,7 +211,7 @@ class CreateBakeTaskSpec extends Specification {
 
   def setup() {
     task.mapper = mapper
-    task.artifactResolver = artifactResolver
+    task.artifactUtils = artifactUtils
     bakeStage = pipeline.stages.first()
   }
 
@@ -1090,7 +1090,7 @@ class CreateBakeTaskSpec extends Specification {
       type = "bake"
       context = bakeConfigWithArtifacts
     }
-    task.artifactResolver = Mock(ArtifactResolver)
+    task.artifactUtils = Mock(ArtifactUtils)
     def bakery = Mock(BakeryService)
 
     and:
@@ -1107,8 +1107,8 @@ class CreateBakeTaskSpec extends Specification {
     def bakeResult = task.bakeFromContext(stage, selectedBakeryService)
 
     then:
-    2 * task.artifactResolver.getBoundArtifactForId(stage, _) >> Artifact.builder().build()
-    1 * task.artifactResolver.getAllArtifacts(_) >> []
+    2 * task.artifactUtils.getBoundArtifactForId(stage, _) >> Artifact.builder().build()
+    1 * task.artifactUtils.getAllArtifacts(_) >> []
     bakeResult.getPackageArtifacts().size() == 2
   }
 
@@ -1118,7 +1118,7 @@ class CreateBakeTaskSpec extends Specification {
       type = "bake"
       context = bakeConfig
     }
-    task.artifactResolver = Mock(ArtifactResolver)
+    task.artifactUtils = Mock(ArtifactUtils)
     def bakery = Mock(BakeryService)
 
     and:
@@ -1135,8 +1135,8 @@ class CreateBakeTaskSpec extends Specification {
     def bakeResult = task.bakeFromContext(stage, selectedBakeryService)
 
     then:
-    0 * task.artifactResolver.getBoundArtifactForId(*_) >> Artifact.builder().build()
-    1 * task.artifactResolver.getAllArtifacts(_) >> []
+    0 * task.artifactUtils.getBoundArtifactForId(*_) >> Artifact.builder().build()
+    1 * task.artifactUtils.getAllArtifacts(_) >> []
     bakeResult.getPackageArtifacts().size() == 0
   }
 
@@ -1146,7 +1146,7 @@ class CreateBakeTaskSpec extends Specification {
       type = "bake"
       context = bakeConfigWithoutOs
     }
-    task.artifactResolver = Mock(ArtifactResolver)
+    task.artifactUtils = Mock(ArtifactUtils)
     def bakery = Mock(BakeryService)
 
     and:
@@ -1164,8 +1164,8 @@ class CreateBakeTaskSpec extends Specification {
 
     then:
     noExceptionThrown()
-    2 * task.artifactResolver.getBoundArtifactForId(stage, _) >> Artifact.builder().build()
-    1 * task.artifactResolver.getAllArtifacts(_) >> []
+    2 * task.artifactUtils.getBoundArtifactForId(stage, _) >> Artifact.builder().build()
+    1 * task.artifactUtils.getAllArtifacts(_) >> []
     bakeResult.getPackageArtifacts().size() == 2
   }
 }
diff --git a/orca-clouddriver/orca-clouddriver.gradle b/orca-clouddriver/orca-clouddriver.gradle
@@ -45,6 +45,7 @@ dependencies {
   testImplementation("com.github.tomakehurst:wiremock:2.15.0")
   testImplementation("org.springframework:spring-test")
   testImplementation("org.junit.jupiter:junit-jupiter-api")
+  testImplementation("org.junit.platform:junit-platform-runner")
   testImplementation("org.assertj:assertj-core")
   testImplementation("org.mockito:mockito-core:2.25.0")
   testImplementation("org.mockito:mockito-junit-jupiter")

diff --git a/...spinnaker/orca/clouddriver/pipeline/servergroup/strategies/CFRollingRedBlackStrategy.java b/...spinnaker/orca/clouddriver/pipeline/servergroup/strategies/CFRollingRedBlackStrategy.java
@@ -35,7 +35,7 @@
 import com.netflix.spinnaker.orca.pipeline.model.Execution;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
 import com.netflix.spinnaker.orca.pipeline.model.SyntheticStageOwner;
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver;
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils;
 import groovy.util.logging.Slf4j;
 import java.io.IOException;
 import java.util.*;
@@ -68,7 +68,7 @@ public class CFRollingRedBlackStrategy implements Strategy, ApplicationContextAw
   public final String name = "cfrollingredblack";
 
   private ApplicationContext applicationContext;
-  private ArtifactResolver artifactResolver;
+  private ArtifactUtils artifactUtils;
   private Optional<PipelineStage> pipelineStage;
   private ResizeStrategySupport resizeStrategySupport;
   private TargetServerGroupResolver targetServerGroupResolver;
@@ -123,8 +123,7 @@ List<Stage> composeFlow(Stage stage) {
       Artifact artifact = objectMapper.convertValue(manifest.get("artifact"), Artifact.class);
       String artifactId =
           manifest.get("artifactId") != null ? manifest.get("artifactId").toString() : null;
-      Artifact boundArtifact =
-          artifactResolver.getBoundArtifactForStage(stage, artifactId, artifact);
+      Artifact boundArtifact = artifactUtils.getBoundArtifactForStage(stage, artifactId, artifact);
 
       if (boundArtifact == null) {
         throw new IllegalArgumentException("Unable to bind the manifest artifact");

diff --git a/...in/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/artifacts/ConsumeArtifactTask.java b/...in/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/artifacts/ConsumeArtifactTask.java
@@ -25,7 +25,7 @@
 import com.netflix.spinnaker.orca.TaskResult;
 import com.netflix.spinnaker.orca.clouddriver.OortService;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver;
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils;
 import java.io.InputStream;
 import java.util.Map;
 import java.util.Objects;
@@ -37,14 +37,14 @@
 public class ConsumeArtifactTask implements Task {
   public static final String TASK_NAME = "consumeArtifact";
 
-  private ArtifactResolver artifactResolver;
+  private ArtifactUtils artifactUtils;
   private OortService oort;
   private RetrySupport retrySupport;
   private ObjectMapper objectMapper = new ObjectMapper();
 
   public ConsumeArtifactTask(
-      ArtifactResolver artifactResolver, OortService oortService, RetrySupport retrySupport) {
-    this.artifactResolver = artifactResolver;
+      ArtifactUtils artifactUtils, OortService oortService, RetrySupport retrySupport) {
+    this.artifactUtils = artifactUtils;
     this.oort = oortService;
     this.retrySupport = retrySupport;
   }
@@ -54,7 +54,7 @@ public TaskResult execute(@NonNull Stage stage) {
     Map<String, Object> task = stage.getContext();
     String artifactId = (String) task.get("consumeArtifactId");
 
-    Artifact artifact = artifactResolver.getBoundArtifactForId(stage, artifactId);
+    Artifact artifact = artifactUtils.getBoundArtifactForId(stage, artifactId);
     if (artifact == null) {
       throw new IllegalArgumentException("No artifact could be bound to '" + artifactId + "'");
     }

diff --git a/...com/netflix/spinnaker/orca/clouddriver/tasks/artifacts/FindArtifactFromExecutionTask.java b/...com/netflix/spinnaker/orca/clouddriver/tasks/artifacts/FindArtifactFromExecutionTask.java
@@ -23,7 +23,7 @@
 import com.netflix.spinnaker.orca.Task;
 import com.netflix.spinnaker.orca.TaskResult;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver;
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -38,7 +38,7 @@
 public class FindArtifactFromExecutionTask implements Task {
   public static final String TASK_NAME = "findArtifactFromExecution";
 
-  private final ArtifactResolver artifactResolver;
+  private final ArtifactUtils artifactUtils;
 
   @Nonnull
   @Override
@@ -57,15 +57,15 @@ public TaskResult execute(@Nonnull Stage stage) {
         Optional.ofNullable(stage.getExecution().getPipelineConfigId()).orElse("");
     if (pipelineConfigId.equals(pipeline)) {
       priorArtifacts =
-          artifactResolver.getArtifactsForPipelineIdWithoutStageRef(
+          artifactUtils.getArtifactsForPipelineIdWithoutStageRef(
               pipeline, stage.getRefId(), executionOptions.toCriteria());
     } else {
       priorArtifacts =
-          artifactResolver.getArtifactsForPipelineId(pipeline, executionOptions.toCriteria());
+          artifactUtils.getArtifactsForPipelineId(pipeline, executionOptions.toCriteria());
     }
 
     Set<Artifact> matchingArtifacts =
-        artifactResolver.resolveExpectedArtifacts(expectedArtifacts, priorArtifacts, null, false);
+        artifactUtils.resolveExpectedArtifacts(expectedArtifacts, priorArtifacts, null, false);
 
     outputs.put("resolvedExpectedArtifacts", expectedArtifacts);
     outputs.put("artifacts", matchingArtifacts);

diff --git a/.../main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/manifest/ManifestEvaluator.java b/.../main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/manifest/ManifestEvaluator.java
@@ -31,7 +31,7 @@
 import com.netflix.spinnaker.orca.clouddriver.utils.CloudProviderAware;
 import com.netflix.spinnaker.orca.pipeline.expressions.PipelineExpressionEvaluator;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver;
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils;
 import com.netflix.spinnaker.orca.pipeline.util.ContextParameterProcessor;
 import java.io.InputStream;
 import java.util.*;
@@ -52,7 +52,7 @@ public class ManifestEvaluator implements CloudProviderAware {
   private static final ThreadLocal<Yaml> yamlParser =
       ThreadLocal.withInitial(() -> new Yaml(new SafeConstructor()));
 
-  private final ArtifactResolver artifactResolver;
+  private final ArtifactUtils artifactUtils;
   private final OortService oort;
   private final ObjectMapper objectMapper;
   private final ContextParameterProcessor contextParameterProcessor;
@@ -73,7 +73,7 @@ public Result evaluate(Stage stage, ManifestContext context) {
     List<Map<Object, Object>> manifests = Collections.emptyList();
     if (ManifestContext.Source.Artifact.equals(context.getSource())) {
       Artifact manifestArtifact =
-          artifactResolver.getBoundArtifactForStage(
+          artifactUtils.getBoundArtifactForStage(
               stage, context.getManifestArtifactId(), context.getManifestArtifact());
 
       if (manifestArtifact == null) {
@@ -145,7 +145,7 @@ public Result evaluate(Stage stage, ManifestContext context) {
 
     List<Artifact> requiredArtifacts = new ArrayList<>();
     for (String id : Optional.ofNullable(context.getRequiredArtifactIds()).orElse(emptyList())) {
-      Artifact requiredArtifact = artifactResolver.getBoundArtifactForId(stage, id);
+      Artifact requiredArtifact = artifactUtils.getBoundArtifactForId(stage, id);
       if (requiredArtifact == null) {
         throw new IllegalStateException(
             "No artifact with id '" + id + "' could be found in the pipeline context.");
@@ -158,7 +158,7 @@ public Result evaluate(Stage stage, ManifestContext context) {
     for (ManifestContext.BindArtifact artifact :
         Optional.ofNullable(context.getRequiredArtifacts()).orElse(emptyList())) {
       Artifact requiredArtifact =
-          artifactResolver.getBoundArtifactForStage(
+          artifactUtils.getBoundArtifactForStage(
               stage, artifact.getExpectedArtifactId(), artifact.getArtifact());
 
       if (requiredArtifact == null) {
@@ -173,7 +173,7 @@ public Result evaluate(Stage stage, ManifestContext context) {
 
     log.info("Artifacts {} are bound to the manifest", requiredArtifacts);
 
-    return new Result(manifests, requiredArtifacts, artifactResolver.getArtifacts(stage));
+    return new Result(manifests, requiredArtifacts, artifactUtils.getArtifacts(stage));
   }
 
   public TaskResult buildTaskResult(String taskName, Stage stage, Map<String, Object> task) {

diff --git a/...y/com/netflix/spinnaker/orca/clouddriver/tasks/pipeline/GetPipelinesFromArtifactTask.java b/...y/com/netflix/spinnaker/orca/clouddriver/tasks/pipeline/GetPipelinesFromArtifactTask.java
@@ -27,7 +27,7 @@
 import com.netflix.spinnaker.orca.clouddriver.OortService;
 import com.netflix.spinnaker.orca.front50.Front50Service;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver;
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils;
 import java.io.*;
 import java.util.*;
 import java.util.stream.Collectors;
@@ -50,17 +50,17 @@ public class GetPipelinesFromArtifactTask implements Task {
   private final Front50Service front50Service;
   private final OortService oort;
   private final ObjectMapper objectMapper;
-  private final ArtifactResolver artifactResolver;
+  private final ArtifactUtils artifactUtils;
 
   public GetPipelinesFromArtifactTask(
       Front50Service front50Service,
       OortService oort,
       ObjectMapper objectMapper,
-      ArtifactResolver artifactResolver) {
+      ArtifactUtils artifactUtils) {
     this.front50Service = front50Service;
     this.oort = oort;
     this.objectMapper = objectMapper;
-    this.artifactResolver = artifactResolver;
+    this.artifactUtils = artifactUtils;
   }
 
   @Getter
@@ -79,7 +79,7 @@ public static class PipelinesArtifactData {
   public TaskResult execute(Stage stage) {
     final PipelinesArtifactData pipelinesArtifact = stage.mapTo(PipelinesArtifactData.class);
     Artifact resolvedArtifact =
-        artifactResolver.getBoundArtifactForStage(
+        artifactUtils.getBoundArtifactForStage(
             stage, pipelinesArtifact.getId(), pipelinesArtifact.getInline());
     if (resolvedArtifact == null) {
       throw new IllegalArgumentException(

diff --git a/...x/spinnaker/orca/clouddriver/tasks/providers/appengine/AppEngineServerGroupCreator.groovy b/...x/spinnaker/orca/clouddriver/tasks/providers/appengine/AppEngineServerGroupCreator.groovy
@@ -21,7 +21,7 @@ import com.netflix.spinnaker.kork.artifacts.model.Artifact
 import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.ServerGroupCreator
 import com.netflix.spinnaker.orca.pipeline.model.Execution
 import com.netflix.spinnaker.orca.pipeline.model.Stage
-import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver
+import com.netflix.spinnaker.orca.pipeline.util.ArtifactUtils
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 import static com.netflix.spinnaker.orca.pipeline.model.Execution.ExecutionType.PIPELINE
@@ -35,7 +35,7 @@ class AppEngineServerGroupCreator implements ServerGroupCreator {
   ObjectMapper objectMapper
 
   @Autowired
-  ArtifactResolver artifactResolver
+  ArtifactUtils artifactUtils
 
   @Override
   List<Map> getOperations(Stage stage) {
@@ -65,7 +65,7 @@ class AppEngineServerGroupCreator implements ServerGroupCreator {
       String expectedId = operation.expectedArtifactId?.trim()
       Artifact expectedArtifact = operation.expectedArtifact
       if (expectedId || expectedArtifact) {
-        Artifact boundArtifact = artifactResolver.getBoundArtifactForStage(stage, expectedId, expectedArtifact)
+        Artifact boundArtifact = artifactUtils.getBoundArtifactForStage(stage, expectedId, expectedArtifact)
         if (boundArtifact) {
           operation.artifact = boundArtifact
         } else {
@@ -75,7 +75,7 @@ class AppEngineServerGroupCreator implements ServerGroupCreator {
       List<ArtifactAccountPair> configArtifacts = operation.configArtifacts
       if (configArtifacts != null && configArtifacts.size() > 0) {
         operation.configArtifacts = configArtifacts.collect { artifactAccountPair ->
-          def artifact = artifactResolver.getBoundArtifactForStage(stage, artifactAccountPair.id, artifactAccountPair.artifact)
+          def artifact = artifactUtils.getBoundArtifactForStage(stage, artifactAccountPair.id, artifactAccountPair.artifact)
           artifact.artifactAccount = artifactAccountPair.account
           return artifact
         }