Merge pull request #87 from anotherchrisberry/pipeline-name-dupe

prevent creation of pipelines with duplicate names
spinnaker · May 12, 2016 · 769f40a · 769f40a
2 parents 0a9bc1c + 9374ce9
commit 769f40a
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 0 deletions.
diff --git a/...0-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/PipelineController.groovy b/...0-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/PipelineController.groovy
@@ -19,10 +19,13 @@ package com.netflix.spinnaker.front50.controllers
 import com.netflix.spinnaker.front50.model.pipeline.Pipeline
 import com.netflix.spinnaker.front50.model.pipeline.PipelineDAO
 import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.http.HttpStatus
+import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.PathVariable
 import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RequestMethod
+import org.springframework.web.bind.annotation.ResponseStatus
 import org.springframework.web.bind.annotation.RestController
 
 /**
@@ -53,6 +56,9 @@ class PipelineController {
             triggers.findAll { it.type == "cron" }.each { Map trigger ->
                 trigger.id = UUID.randomUUID().toString()
             }
+            if (pipelineDAO.getPipelineId(pipeline.getApplication(), pipeline.getName())) {
+                throw new DuplicatePipelineNameException()
+            }
         }
 
         pipelineDAO.create(pipeline.id as String, pipeline)
@@ -77,6 +83,9 @@ class PipelineController {
 
     @RequestMapping(value = 'move', method = RequestMethod.POST)
     void rename(@RequestBody RenameCommand command) {
+        if (pipelineDAO.getPipelineId(command.application, command.to)) {
+            throw new DuplicatePipelineNameException()
+        }
         def pipelineId = pipelineDAO.getPipelineId(command.application, command.from)
         def pipeline = pipelineDAO.findById(pipelineId)
         pipeline.setName(command.to)
@@ -90,4 +99,11 @@ class PipelineController {
         String to
     }
 
+    @ExceptionHandler(DuplicatePipelineNameException)
+    @ResponseStatus(HttpStatus.BAD_REQUEST)
+    Map handleDuplicatePipelineNameException() {
+        return [error: "A pipeline with that name already exists in that application", status: HttpStatus.BAD_REQUEST]
+    }
+
+    static class DuplicatePipelineNameException extends Exception {}
 }
diff --git a/...0-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/StrategyController.groovy b/...0-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/StrategyController.groovy
@@ -19,10 +19,13 @@ package com.netflix.spinnaker.front50.controllers
 import com.netflix.spinnaker.front50.model.pipeline.Pipeline
 import com.netflix.spinnaker.front50.model.pipeline.PipelineStrategyDAO
 import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.http.HttpStatus
+import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.PathVariable
 import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RequestMethod
+import org.springframework.web.bind.annotation.ResponseStatus
 import org.springframework.web.bind.annotation.RestController
 
 /**
@@ -53,6 +56,9 @@ class StrategyController {
             triggers.findAll { it.type == "cron" }.each { Map trigger ->
                 trigger.id = UUID.randomUUID().toString()
             }
+            if (pipelineStrategyDAO.getPipelineId(strategy.getApplication(), strategy.getName())) {
+                throw new DuplicateStrategyException()
+            }
         }
 
         pipelineStrategyDAO.create(strategy.getId(), strategy)
@@ -77,13 +83,24 @@ class StrategyController {
 
     @RequestMapping(value = 'move', method = RequestMethod.POST)
     void rename(@RequestBody RenameCommand command) {
+        if (pipelineStrategyDAO.getPipelineId(command.application, command.to)) {
+            throw new DuplicateStrategyException()
+        }
         def pipelineId = pipelineStrategyDAO.getPipelineId(command.application, command.from)
         def pipeline = pipelineStrategyDAO.findById(pipelineId)
         pipeline.setName(command.to)
 
         pipelineStrategyDAO.update(pipelineId, pipeline)
     }
 
+    @ExceptionHandler(DuplicateStrategyException)
+    @ResponseStatus(HttpStatus.BAD_REQUEST)
+    Map handleDuplicateStrategyNameException() {
+        return [error: "A strategy with that name already exists in that application", status: HttpStatus.BAD_REQUEST]
+    }
+
+    static class DuplicateStrategyException extends Exception {}
+
     static class RenameCommand {
         String application
         String from

diff --git a/...eb/src/test/groovy/com/netflix/spinnaker/front50/controllers/PipelineControllerTck.groovy b/...eb/src/test/groovy/com/netflix/spinnaker/front50/controllers/PipelineControllerTck.groovy
@@ -44,6 +44,7 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders
 abstract class PipelineControllerTck extends Specification {
 
   static final int OK = 200
+  static final int BAD_REQUEST = 400
 
   MockMvc mockMvc
 
@@ -137,6 +138,54 @@ abstract class PipelineControllerTck extends Specification {
     response.status == OK
     pipelineDAO.all()*.name == ["pipeline2"]
   }
+
+  void 'should enforce unique names on save operations'() {
+    given:
+    pipelineDAO.create(null, new Pipeline([
+            name: "pipeline1", application: "test"
+    ]))
+    pipelineDAO.create(null, new Pipeline([
+            name: "pipeline2", application: "test"
+    ]))
+
+    when:
+    def allPipelines = pipelineDAO.all()
+    def allPipelinesForApplication = pipelineDAO.getPipelinesByApplication("test")
+
+    then:
+    allPipelines*.id.sort() == allPipelinesForApplication*.id.sort()
+    allPipelines.size() == 2
+
+    when:
+    def response = mockMvc.perform(post('/pipelines')
+                    .contentType(MediaType.APPLICATION_JSON)
+                    .content(new ObjectMapper().writeValueAsString([name: "pipeline1", application: "test"])))
+                  .andReturn().response
+
+    then:
+    response.status == BAD_REQUEST
+    response.contentAsString == '{"error":"A pipeline with that name already exists in that application","status":"BAD_REQUEST"}'
+  }
+
+  void 'should enforce unique names on rename operations'() {
+    given:
+    pipelineDAO.create(null, new Pipeline([
+            name: "pipeline1", application: "test"
+    ]))
+    pipelineDAO.create(null, new Pipeline([
+            name: "pipeline2", application: "test"
+    ]))
+
+    when:
+    def response = mockMvc.perform(post('/pipelines/move')
+            .contentType(MediaType.APPLICATION_JSON)
+            .content(new ObjectMapper().writeValueAsString([from: "pipeline2", to: "pipeline1", application: "test"])))
+            .andReturn().response
+
+    then:
+    response.status == BAD_REQUEST
+    response.contentAsString == '{"error":"A pipeline with that name already exists in that application","status":"BAD_REQUEST"}'
+  }
 }
 
 class CassandraPipelineControllerTck extends PipelineControllerTck {

diff --git a/...eb/src/test/groovy/com/netflix/spinnaker/front50/controllers/StrategyControllerTck.groovy b/...eb/src/test/groovy/com/netflix/spinnaker/front50/controllers/StrategyControllerTck.groovy
@@ -46,6 +46,7 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders
 abstract class StrategyControllerTck extends Specification {
 
   static final int OK = 200
+  static final int BAD_REQUEST = 400
 
   MockMvc mockMvc
 
@@ -140,6 +141,54 @@ abstract class StrategyControllerTck extends Specification {
     response.status == OK
     pipelineStrategyDAO.all()*.name == ["pipeline2"]
   }
+
+  void 'should enforce unique names on save operations'() {
+    given:
+    pipelineStrategyDAO.create(null, new Pipeline([
+            name: "pipeline1", application: "test"
+    ]))
+    pipelineStrategyDAO.create(null, new Pipeline([
+            name: "pipeline2", application: "test"
+    ]))
+
+    when:
+    def allPipelines = pipelineStrategyDAO.all()
+    def allPipelinesForApplication = pipelineStrategyDAO.getPipelinesByApplication("test")
+
+    then:
+    allPipelines*.id.sort() == allPipelinesForApplication*.id.sort()
+    allPipelines.size() == 2
+
+    when:
+    def response = mockMvc.perform(post('/strategies')
+            .contentType(MediaType.APPLICATION_JSON)
+            .content(new ObjectMapper().writeValueAsString([name: "pipeline1", application: "test"])))
+            .andReturn().response
+
+    then:
+    response.status == BAD_REQUEST
+    response.contentAsString == '{"error":"A strategy with that name already exists in that application","status":"BAD_REQUEST"}'
+  }
+
+  void 'should enforce unique names on rename operations'() {
+    given:
+    pipelineStrategyDAO.create(null, new Pipeline([
+            name: "pipeline1", application: "test"
+    ]))
+    pipelineStrategyDAO.create(null, new Pipeline([
+            name: "pipeline2", application: "test"
+    ]))
+
+    when:
+    def response = mockMvc.perform(post('/strategies/move')
+            .contentType(MediaType.APPLICATION_JSON)
+            .content(new ObjectMapper().writeValueAsString([from: "pipeline2", to: "pipeline1", application: "test"])))
+            .andReturn().response
+
+    then:
+    response.status == BAD_REQUEST
+    response.contentAsString == '{"error":"A strategy with that name already exists in that application","status":"BAD_REQUEST"}'
+  }
 }
 
 class CassandraStrategyControllerTck extends StrategyControllerTck {