fix(pr): Address review feedback + allow querying multiple statuses

spinnaker · Jul 21, 2020 · aaa4e62 · aaa4e62
1 parent af347c1
commit aaa4e62
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 30 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
@@ -46,10 +46,10 @@ public ArtifactController(ArtifactServices artifactServices) {
   public List<String> getVersions(
       @PathVariable("provider") String provider,
       @PathVariable("name") String name,
-      @RequestParam(value = "releaseStatus", required = false) String releaseStatus,
+      @RequestParam(value = "releaseStatus", required = false) List<String> releaseStatuses,
       @RequestParam(value = "type", required = false, defaultValue = "deb") String type) {
     ArtifactService artifactService = getService(provider);
-    return artifactService.getArtifactVersions(type, name, releaseStatus);
+    return artifactService.getArtifactVersions(type, name, releaseStatuses);
   }
 
   @GetMapping("/{provider}/{name}/{version:.+}")

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
@@ -34,25 +34,9 @@ public interface ArtifactService {
   /** Used to populate the manual trigger dropdown with options */
   @Nonnull
   List<String> getArtifactVersions(
-      @Nonnull String type, @Nonnull String name, String releaseStatus);
+      @Nonnull String type, @Nonnull String name, List<String> releaseStatuses);
 
   /** Used to fetch a specific artifact for decorating a trigger */
   @Nonnull
   Artifact getArtifact(@Nonnull String type, @Nonnull String name, @Nonnull String version);
-
-  /**
-   * Used to populate the manual trigger dropdown with options (backwards-compatibility for Debian)
-   */
-  @Nonnull
-  default List<String> getArtifactVersions(@Nonnull String name, String releaseStatus) {
-    return getArtifactVersions("deb", name, releaseStatus);
-  }
-
-  /**
-   * Used to fetch a specific artifact for decorating a trigger (backwards-compatibility for Debian)
-   */
-  @Nonnull
-  default Artifact getArtifact(@Nonnull String name, @Nonnull String version) {
-    return getArtifact("deb", name, 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
@@ -27,13 +27,13 @@ class ArtifactServiceSpec extends Specification {
 
   @Subject
   ArtifactServices artifactServices = new ArtifactServices()
-  
+
   void setup() {
     Map<String, ArtifactService> 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
@@ -31,18 +31,20 @@ public ArtifactServiceProvider artifactServiceProvider() {
   }
 
   @Override
-  public List<String> getArtifactVersions(String type, String name, String releaseStatus) {
+  public List<String> getArtifactVersions(String type, String name, List<String> releaseStatuses) {
     if (!name.equals("test")) {
       return Collections.emptyList();
     }
     List<String> 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;