Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent creation of pipelines with duplicate names #87

Merged
merged 1 commit into from
May 12, 2016
Merged

prevent creation of pipelines with duplicate names #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions ...0-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/PipelineController.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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 {}
}
17 changes: 17 additions & 0 deletions ...0-web/src/main/groovy/com/netflix/spinnaker/front50/controllers/StrategyController.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
49 changes: 49 additions & 0 deletions ...eb/src/test/groovy/com/netflix/spinnaker/front50/controllers/PipelineControllerTck.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 {
Expand Down
49 changes: 49 additions & 0 deletions ...eb/src/test/groovy/com/netflix/spinnaker/front50/controllers/StrategyControllerTck.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 {
Expand Down