From 49c9647b3a848d8b1b43317d7c802705ac14df7a Mon Sep 17 00:00:00 2001 From: Luis Pollo <1323478+luispollo@users.noreply.github.com> Date: Tue, 21 Jul 2020 17:21:03 -0700 Subject: [PATCH] feat(artifacts): Add type parameter to ArtifactService (#821) * feat(artifacts): Add type parameter to ArtifactService * fix(pr): Address review feedback + allow querying multiple statuses --- .../igor/artifacts/ArtifactController.java | 10 ++++++---- .../igor/artifacts/ArtifactService.java | 15 +++++++++++++-- .../igor/artifacts/ArtifactServiceSpec.groovy | 16 ++++++++-------- .../igor/artifacts/TestArtifactService.java | 10 ++++++---- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactController.java b/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactController.java index 35a3b60ab..206f70206 100644 --- a/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactController.java +++ b/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactController.java @@ -46,18 +46,20 @@ public ArtifactController(ArtifactServices artifactServices) { public List getVersions( @PathVariable("provider") String provider, @PathVariable("name") String name, - @RequestParam(value = "releaseStatus", required = false) String releaseStatus) { + @RequestParam(value = "releaseStatus", required = false) List releaseStatuses, + @RequestParam(value = "type", required = false, defaultValue = "deb") String type) { ArtifactService artifactService = getService(provider); - return artifactService.getArtifactVersions(name, releaseStatus); + return artifactService.getArtifactVersions(type, name, releaseStatuses); } @GetMapping("/{provider}/{name}/{version:.+}") public Artifact getArtifact( @PathVariable("provider") String provider, @PathVariable("name") String name, - @PathVariable("version") String version) { + @PathVariable("version") String version, + @RequestParam(value = "type", required = false, defaultValue = "deb") String type) { ArtifactService artifactService = getService(provider); - return artifactService.getArtifact(name, version); + return artifactService.getArtifact(type, name, version); } private ArtifactService getService(String serviceName) { diff --git a/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactService.java b/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactService.java index 6e22ceebd..b46c6690a 100644 --- a/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactService.java +++ b/igor-web/src/main/java/com/netflix/spinnaker/igor/artifacts/ArtifactService.java @@ -19,6 +19,7 @@ import com.netflix.spinnaker.igor.model.ArtifactServiceProvider; import com.netflix.spinnaker.kork.artifacts.model.Artifact; +import java.util.ArrayList; import java.util.List; import javax.annotation.Nonnull; @@ -33,9 +34,19 @@ public interface ArtifactService { /** Used to populate the manual trigger dropdown with options */ @Nonnull - List getArtifactVersions(@Nonnull String name, String releaseStatus); + default List getArtifactVersions( + @Nonnull String type, @Nonnull String name, String releaseStatus) { + List releaseStatuses = new ArrayList<>(); + releaseStatuses.add(releaseStatus); + return getArtifactVersions(type, name, releaseStatuses); + } + + /** Used to populate the manual trigger dropdown with options */ + @Nonnull + List getArtifactVersions( + @Nonnull String type, @Nonnull String name, List releaseStatuses); /** Used to fetch a specific artifact for decorating a trigger */ @Nonnull - Artifact getArtifact(@Nonnull String name, @Nonnull String version); + Artifact getArtifact(@Nonnull String type, @Nonnull String name, @Nonnull String version); } diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy index 69da8a650..e135b4eaf 100644 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy @@ -27,13 +27,13 @@ class ArtifactServiceSpec extends Specification { @Subject ArtifactServices artifactServices = new ArtifactServices() - + void setup() { Map services = new HashMap<>() services.put("artifactory", new TestArtifactService()) artifactServices.addServices(services) } - + def "finds matching service"() { when: def service = artifactServices.getService("artifactory") @@ -53,7 +53,7 @@ class ArtifactServiceSpec extends Specification { def "service finds artifact versions"() { when: def service = artifactServices.getService("artifactory") - def versions = service.getArtifactVersions("test", null) + def versions = service.getArtifactVersions("deb","test", null) then: assertThat(versions).isNotNull() @@ -64,7 +64,7 @@ class ArtifactServiceSpec extends Specification { def "service finds only snapshot artifacts"() { when: def service = artifactServices.getService("artifactory") - def versions = service.getArtifactVersions("test", "snapshot") + def versions = service.getArtifactVersions("deb","test", "snapshot") then: assertThat(versions).isNotNull() @@ -75,7 +75,7 @@ class ArtifactServiceSpec extends Specification { def "service finds artifact"() { when: def service = artifactServices.getService("artifactory") - def artifact = service.getArtifact("test", "v0.4.0") + def artifact = service.getArtifact("deb","test", "v0.4.0") then: assertThat(artifact).isNotNull() @@ -86,7 +86,7 @@ class ArtifactServiceSpec extends Specification { def "versions list is empty when no versions found"() { when: def service = artifactServices.getService("artifactory") - def versions = service.getArtifactVersions("blah", "") + def versions = service.getArtifactVersions("deb","blah", "") then: assertThat(versions).isNotNull() @@ -96,10 +96,10 @@ class ArtifactServiceSpec extends Specification { def "404 is thrown when artifact not found"() { when: def service = artifactServices.getService("artifactory") - service.getArtifact("blah","v0.0.1") + service.getArtifact("deb","blah","v0.0.1") then: thrown(NotFoundException) } - + } diff --git a/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/TestArtifactService.java b/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/TestArtifactService.java index 2b362a941..78349ab3d 100644 --- a/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/TestArtifactService.java +++ b/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/TestArtifactService.java @@ -31,25 +31,27 @@ public ArtifactServiceProvider artifactServiceProvider() { } @Override - public List getArtifactVersions(String name, String releaseStatus) { + public List getArtifactVersions(String type, String name, List releaseStatuses) { if (!name.equals("test")) { return Collections.emptyList(); } List versions = new ArrayList<>(); - if (releaseStatus == null || releaseStatus.isEmpty() || releaseStatus.contains("final")) { + if (releaseStatuses == null || releaseStatuses.isEmpty() || releaseStatuses.contains("final")) { versions.add("v0.1.0"); versions.add("v0.2.0"); versions.add("v0.3.0"); versions.add("v0.4.0"); } - if (releaseStatus == null || releaseStatus.isEmpty() || releaseStatus.contains("snapshot")) { + if (releaseStatuses == null + || releaseStatuses.isEmpty() + || releaseStatuses.contains("snapshot")) { versions.add("v0.5.0~SNAPSHOT"); } return versions; } @Override - public Artifact getArtifact(String name, String version) { + public Artifact getArtifact(String type, String name, String version) { if (!name.equals("test") && !version.equals("v0.4.0")) { throw new NotFoundException("Artifact not found"); }