Skip to content

Commit

Permalink
refactor(artifacts): Use builder to create artifacts (#3328)
Browse files Browse the repository at this point in the history
The all-arg and no-arg constructors for Artifact and
ExpectedArtifact were deprecated in spinnaker/kork#429.
Replace their usages with a builder.
  • Loading branch information
ezimanyi authored and mergify[bot] committed Dec 4, 2019
1 parent cc10932 commit 268d1c0
Show file tree
Hide file tree
Showing 15 changed files with 142 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class CompletedBakeTaskSpec extends Specification {
region = "us-west-1"
bakeId = "b-5af233wjj78mwt2f420wt8ey3w"
ami = "ami-280c3b6d"
artifact = new Artifact(reference: ami)
artifact = Artifact.builder().reference(ami).build()
}

def "fails if the bake is not found"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ class CreateBakeTaskSpec extends Specification {
def bakeResult = task.bakeFromContext(stage, selectedBakeryService)

then:
2 * task.artifactResolver.getBoundArtifactForId(stage, _) >> new Artifact()
2 * task.artifactResolver.getBoundArtifactForId(stage, _) >> Artifact.builder().build()
1 * task.artifactResolver.getAllArtifacts(_) >> []
bakeResult.getPackageArtifacts().size() == 2
}
Expand Down Expand Up @@ -1135,7 +1135,7 @@ class CreateBakeTaskSpec extends Specification {
def bakeResult = task.bakeFromContext(stage, selectedBakeryService)

then:
0 * task.artifactResolver.getBoundArtifactForId(*_) >> new Artifact()
0 * task.artifactResolver.getBoundArtifactForId(*_) >> Artifact.builder().build()
1 * task.artifactResolver.getAllArtifacts(_) >> []
bakeResult.getPackageArtifacts().size() == 0
}
Expand Down Expand Up @@ -1164,7 +1164,7 @@ class CreateBakeTaskSpec extends Specification {

then:
noExceptionThrown()
2 * task.artifactResolver.getBoundArtifactForId(stage, _) >> new Artifact()
2 * task.artifactResolver.getBoundArtifactForId(stage, _) >> Artifact.builder().build()
1 * task.artifactResolver.getAllArtifacts(_) >> []
bakeResult.getPackageArtifacts().size() == 2
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ class FindImageFromClusterTask extends AbstractCloudProviderAwareTask implements
}.flatten()

List<Artifact> artifacts = imageSummaries.collect { placement, summaries ->
Artifact artifact = new Artifact()
Artifact artifact = Artifact.builder().build()
summaries.findResults { summary ->
if (config.imageNamePattern && !(summary.imageName ==~ config.imageNamePattern)) {
return null
Expand All @@ -282,12 +282,14 @@ class FindImageFromClusterTask extends AbstractCloudProviderAwareTask implements
log.error("Unable to merge server group image/build info (summary: ${summary})", e)
}

artifact.metadata = metadata
artifact.name = summary.imageName
artifact.location = location
artifact.type = "${cloudProvider}/image"
artifact.reference = "${summary.imageId}"
artifact.uuid = UUID.randomUUID().toString()
artifact = Artifact.builder()
.metadata(metadata)
.name(summary.imageName)
.location(location)
.type("${cloudProvider}/image")
.reference("${cloudProvider}/image")
.uuid(UUID.randomUUID().toString())
.build()
}
return artifact
}.flatten()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,14 @@ private Artifact generateArtifactFrom(
// This is either all or nothing
}

Artifact artifact = new Artifact();
artifact.setName(imageDetails.getImageName());
artifact.setReference(imageDetails.getImageId());
artifact.setLocation(imageDetails.getRegion());
artifact.setType(cloudProvider + "/image");
artifact.setMetadata(metadata);
artifact.setUuid(UUID.randomUUID().toString());

return artifact;
return Artifact.builder()
.name(imageDetails.getImageName())
.reference(imageDetails.getImageId())
.location(imageDetails.getRegion())
.type(cloudProvider + "/image")
.metadata(metadata)
.uuid(UUID.randomUUID().toString())
.build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ void composeFlowWithNoSourceAndManifestArtifactConvertsManifestToDirect() throws
List<Object> targetPercentageList = Stream.of(50, 75, 100).collect(Collectors.toList());
context.put("manifest", manifest);
context.put("targetPercentages", targetPercentageList);
Artifact boundArtifactForStage = new Artifact();
Artifact boundArtifactForStage = Artifact.builder().build();
Map<String, Object> application = new HashMap<>();
application.put("instances", "4");
application.put("memory", "64M");
Expand Down Expand Up @@ -325,7 +325,7 @@ void composeFlowWithNoSourceAndManifestArtifactConvertsManifestToDirect() throws
assertThat(deployServerGroupStage.getContext().get("capacity")).isEqualTo(zeroCapacity);
assertThat(deployServerGroupStage.getContext().get("manifest")).isEqualTo(expectedManifest);
verify(artifactResolver)
.getBoundArtifactForStage(deployServerGroupStage, artifactId, new Artifact());
.getBoundArtifactForStage(deployServerGroupStage, artifactId, Artifact.builder().build());
verify(oortService).fetchArtifact(boundArtifactForStage);
}

Expand All @@ -352,7 +352,7 @@ void composeFlowWithSourceAndManifestArtifactConvertsManifestToDirect() throws I
new ResizeStrategy.Capacity(initialSourceCapacity.getMax(), 0, 0);

Stage deployServerGroupStage = new Stage(new Execution(PIPELINE, "unit"), "type", context);
Artifact boundArtifactForStage = new Artifact();
Artifact boundArtifactForStage = Artifact.builder().build();
Map<String, Object> application = new HashMap<>();
application.put("instances", "4");
application.put("memory", "64M");
Expand Down Expand Up @@ -402,7 +402,7 @@ void composeFlowWithSourceAndManifestArtifactConvertsManifestToDirect() throws I
assertThat(deployServerGroupStage.getContext().get("capacity")).isEqualTo(zeroCapacity);
assertThat(deployServerGroupStage.getContext().get("manifest")).isEqualTo(expectedManifest);
verify(artifactResolver)
.getBoundArtifactForStage(deployServerGroupStage, artifactId, new Artifact());
.getBoundArtifactForStage(deployServerGroupStage, artifactId, Artifact.builder().build());
verify(oortService).fetchArtifact(boundArtifactForStage);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ConsumeArtifactTaskSpec extends Specification {

def oortService = Mock(OortService)
def artifactResolver = Stub(ArtifactResolver) {
getBoundArtifactForId(*_) >> new Artifact()
getBoundArtifactForId(*_) >> Artifact.builder().build()
}

@Subject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class GetPipelinesFromArtifactTaskSpec extends Specification {
def result = task.execute(new Stage(Execution.newPipeline("orca"), "whatever", context))

then:
1 * artifactResolver.getBoundArtifactForStage(_, '123', _) >> new Artifact().builder().type('http/file')
1 * artifactResolver.getBoundArtifactForStage(_, '123', _) >> Artifact.builder().type('http/file')
.reference('url1').build()
1 * oortService.fetchArtifact(_) >> new Response("url1", 200, "reason1", [],
new TypedString(pipelineJson))
Expand All @@ -66,7 +66,7 @@ class GetPipelinesFromArtifactTaskSpec extends Specification {
def result = task.execute(new Stage(Execution.newPipeline("orca"), "whatever", context))

then:
1 * artifactResolver.getBoundArtifactForStage(_, '123', _) >> new Artifact().builder().type('http/file')
1 * artifactResolver.getBoundArtifactForStage(_, '123', _) >> Artifact.builder().type('http/file')
.reference('url1').build()
1 * oortService.fetchArtifact(_) >> new Response("url1", 200, "reason1", [],
new TypedString(pipelineJson))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class DeployCloudFormationTaskSpec extends Specification {
def result = deployCloudFormationTask.execute(stage)

then:
(_..1) * artifactResolver.getBoundArtifactForStage(stage, 'id', null) >> new Artifact()
(_..1) * artifactResolver.getBoundArtifactForStage(stage, 'id', null) >> Artifact.builder().build()
(_..1) * oortService.fetchArtifact(_) >> new Response("url", 200, "reason", Collections.emptyList(), template)
thrown(expectedException)

Expand Down Expand Up @@ -141,7 +141,7 @@ class DeployCloudFormationTaskSpec extends Specification {
def result = deployCloudFormationTask.execute(stage)

then:
1 * artifactResolver.getBoundArtifactForStage(stage, stackArtifactId, _) >> new Artifact()
1 * artifactResolver.getBoundArtifactForStage(stage, stackArtifactId, _) >> Artifact.builder().build()
1 * oortService.fetchArtifact(_) >> new Response("url", 200, "reason", Collections.emptyList(), new TypedString(template))
1 * katoService.requestOperations("aws", {
it.get(0).get("deployCloudFormation").containsKey("templateBody")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class EcsServerGroupCreatorSpec extends Specification {
def taskDefArtifact = [
artifactId: testArtifactId
]
Artifact resolvedArtifact = new Artifact().builder().type('s3/object').name('s3://testfile.json').build()
Artifact resolvedArtifact = Artifact.builder().type('s3/object').name('s3://testfile.json').build()
mockResolver.getBoundArtifactForStage(stage, testArtifactId, null) >> resolvedArtifact
// define container mappings inputs
def (testReg,testRepo,testTag) = ["myregistry.io","myrepo","latest"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,7 @@ class GoogleServerGroupCreatorSpec extends Specification {
context.putAll(ctx)
}
artifactResolver.getBoundArtifactForId(*_) >> {
Artifact artifact = new Artifact();
artifact.setName("santaImage")
return artifact
return Artifact.builder().name("santaImage").build()
}
ops = new GoogleServerGroupCreator(artifactResolver: artifactResolver).getOperations(stage)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class KubernetesJobRunnerSpec extends Specification {
)
}
1 * artifactResolver.getBoundArtifactForStage(_, _, _) >> {
return new Artifact()
return Artifact.builder().build()
}
op.manifest == manifest
op.requiredArtifacts == []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import spock.lang.Subject
class FindArtifactFromExecutionTaskSpec extends Specification {
def PIPELINE = "my pipeline"
def EXECUTION_CRITERIA = new ExecutionRepository.ExecutionCriteria().setStatuses(Collections.singletonList("SUCCEEDED"))
def ARTIFACT_A = new Artifact(type: "kubernetes/replicaSet")
def ARTIFACT_B = new Artifact(type: "kubernetes/configMap")
def ARTIFACT_A = Artifact.builder().type("kubernetes/replicaSet").build()
def ARTIFACT_B = Artifact.builder().type("kubernetes/configMap").build()

ArtifactResolver artifactResolver = Mock(ArtifactResolver)
Execution execution = Mock(Execution)
Expand All @@ -41,7 +41,7 @@ class FindArtifactFromExecutionTaskSpec extends Specification {

def "finds a single artifact"() {
given:
def expectedArtifacts = [new ExpectedArtifact(matchArtifact: ARTIFACT_A)]
def expectedArtifacts = [ExpectedArtifact.builder().matchArtifact(ARTIFACT_A).build()]
def pipelineArtifacts = [ARTIFACT_A, ARTIFACT_B]
Set resolvedArtifacts = [ARTIFACT_A]
def stage = new Stage(execution, "findArtifactFromExecution", [
Expand All @@ -64,7 +64,7 @@ class FindArtifactFromExecutionTaskSpec extends Specification {

def "finds multiple artifacts"() {
given:
def expectedArtifacts = [new ExpectedArtifact(matchArtifact: ARTIFACT_A), new ExpectedArtifact(matchArtifact: ARTIFACT_B)]
def expectedArtifacts = [ExpectedArtifact.builder().matchArtifact(ARTIFACT_A).build(), ExpectedArtifact.builder().matchArtifact(ARTIFACT_B).build()]
def pipelineArtifacts = [ARTIFACT_A, ARTIFACT_B]
Set resolvedArtifacts = [ARTIFACT_A, ARTIFACT_B]
def stage = new Stage(execution, "findArtifactFromExecution", [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
class ArtifactExpressionFunctionProviderSpec extends Specification {
@Shared
def pipeline1 = pipeline {
def matchArtifact1 = new Artifact(type: "docker/image", "name": "artifact1")
def boundArtifact = new Artifact(type: "docker/image", "name": "artifact1", "reference": "artifact1")
def matchArtifact1 = Artifact.builder().type("docker/image").name("artifact1").build()
def boundArtifact = Artifact.builder().type("docker/image").name("artifact1").reference("artifact1").build()

trigger = new DefaultTrigger("manual", "artifact1")
trigger.resolvedExpectedArtifacts = [
new ExpectedArtifact(matchArtifact: matchArtifact1, boundArtifact: boundArtifact),
ExpectedArtifact.builder().matchArtifact(matchArtifact1).boundArtifact(boundArtifact).build()
]
}

Expand Down
Loading

0 comments on commit 268d1c0

Please sign in to comment.