Skip to content

Commit

Permalink
refactor(artifacts): Rename ArtifactResolver to ArtifactUtils (spinna…
Browse files Browse the repository at this point in the history
…ker#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.
  • Loading branch information
ezimanyi authored and mergify[bot] committed Jan 17, 2020
1 parent 6ccdc92 commit bc2226c
Show file tree
Hide file tree
Showing 40 changed files with 346 additions and 314 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -51,7 +51,7 @@ class CreateBakeTask implements RetryableTask {
long timeout = 300000

@Autowired
ArtifactResolver artifactResolver
ArtifactUtils artifactUtils

@Autowired(required = false)
BakerySelector bakerySelector
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -48,7 +48,7 @@ class CreateBakeTaskSpec extends Specification {
Stage bakeStage
def mapper = OrcaObjectMapper.newInstance()

ArtifactResolver artifactResolver = Stub() {
ArtifactUtils artifactUtils = Stub() {
getAllArtifacts(_) >> []
}

Expand Down Expand Up @@ -211,7 +211,7 @@ class CreateBakeTaskSpec extends Specification {

def setup() {
task.mapper = mapper
task.artifactResolver = artifactResolver
task.artifactUtils = artifactUtils
bakeStage = pipeline.stages.first()
}

Expand Down Expand Up @@ -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:
Expand All @@ -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
}

Expand All @@ -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:
Expand All @@ -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
}

Expand All @@ -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:
Expand All @@ -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
}
}
1 change: 1 addition & 0 deletions orca-clouddriver/orca-clouddriver.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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 + "'");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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.");
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -35,7 +35,7 @@ class AppEngineServerGroupCreator implements ServerGroupCreator {
ObjectMapper objectMapper

@Autowired
ArtifactResolver artifactResolver
ArtifactUtils artifactUtils

@Override
List<Map> getOperations(Stage stage) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
Loading

0 comments on commit bc2226c

Please sign in to comment.