Move job names to the end of URLs so that they can contain slashes

spinnaker · Dec 3, 2015 · da853ce · da853ce
1 parent 1a1fd14
commit da853ce
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 16 deletions.
diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/BuildController.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/BuildController.groovy
@@ -23,6 +23,7 @@ import com.netflix.spinnaker.igor.jenkins.client.model.JobConfig
 import com.netflix.spinnaker.igor.jenkins.client.model.QueuedJob
 import groovy.transform.InheritConstructors
 import groovy.util.logging.Slf4j
+import javax.servlet.http.HttpServletRequest
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.http.HttpStatus
 import org.springframework.web.bind.annotation.PathVariable
@@ -31,6 +32,7 @@ import org.springframework.web.bind.annotation.RequestMethod
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.ResponseStatus
 import org.springframework.web.bind.annotation.RestController
+import org.springframework.web.servlet.HandlerMapping
 import org.yaml.snakeyaml.Yaml
 
 import java.util.concurrent.ExecutorService
@@ -47,8 +49,10 @@ class BuildController {
     @Autowired
     ObjectMapper objectMapper
 
-    @RequestMapping(value = '/jobs/{master}/{job}/{buildNumber}')
-    Map getJobStatus(@PathVariable String master, @PathVariable String job, @PathVariable Integer buildNumber) {
+    @RequestMapping(value = '/builds/status/{buildNumber}/{master}/**')
+    Map getJobStatus(@PathVariable String master, @PathVariable Integer buildNumber, HttpServletRequest request) {
+        def job = (String) request.getAttribute(
+            HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(5).join('/')
         if (!masters.map.containsKey(master)) {
             throw new MasterNotFoundException()
         }
@@ -69,26 +73,30 @@ class BuildController {
         result
     }
 
-    @RequestMapping(value = '/jobs/{master}/queue/{item}')
+    @RequestMapping(value = '/builds/queue/{master}/{item}')
     QueuedJob getQueueLocation(@PathVariable String master, @PathVariable int item){
         if (!masters.map.containsKey(master)) {
             throw new MasterNotFoundException()
         }
         masters.map[master].getQueuedItem(item)
     }
 
-    @RequestMapping(value = '/jobs/{master}/{job}/builds')
-    List<Build> getBuilds(@PathVariable String master, @PathVariable String job) {
+    @RequestMapping(value = '/builds/all/{master}/**')
+    List<Build> getBuilds(@PathVariable String master, HttpServletRequest request) {
+        def job = (String) request.getAttribute(
+            HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(4).join('/')
         if (!masters.map.containsKey(master)) {
             throw new MasterNotFoundException()
         }
         masters.map[master].getBuilds(job).list
     }
 
-    @RequestMapping(value = '/masters/{master}/jobs/{job:.+}', method = RequestMethod.PUT)
+    @RequestMapping(value = '/masters/{name}/jobs/**', method = RequestMethod.PUT)
     String build(
-        @PathVariable("master") String master,
-        @PathVariable String job, @RequestParam Map<String, String> requestParams) {
+        @PathVariable("name") String master,
+        @RequestParam Map<String, String> requestParams, HttpServletRequest request) {
+        def job = (String) request.getAttribute(
+            HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(4).join('/')
         if (!masters.map.containsKey(master)) {
             throw new MasterNotFoundException()
         }
@@ -124,10 +132,12 @@ class BuildController {
         queuedLocation.split('/')[-1]
     }
 
-    @RequestMapping(value = '/jobs/{master}/{job}/{buildNumber}/properties/{fileName:.+}')
+    @RequestMapping(value = '/builds/properties/{buildNumber}/{fileName}/{master}/**')
     Map<String, Object> getProperties(
         @PathVariable String master,
-        @PathVariable String job, @PathVariable Integer buildNumber, @PathVariable String fileName) {
+        @PathVariable Integer buildNumber, @PathVariable String fileName, HttpServletRequest request) {
+        def job = (String) request.getAttribute(
+            HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(6).join('/')
         if (!masters.map.containsKey(master)) {
             throw new MasterNotFoundException()
         }

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/InfoController.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/InfoController.groovy
@@ -31,7 +31,6 @@ import org.springframework.web.bind.annotation.RequestMethod
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.ResponseStatus
 import org.springframework.web.bind.annotation.RestController
-import org.springframework.web.context.request.WebRequest
 import org.springframework.web.servlet.HandlerMapping
 
 import javax.ws.rs.QueryParam

diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildControllerSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildControllerSpec.groovy
@@ -61,7 +61,7 @@ class BuildControllerSpec extends Specification {
     final HTTP_201 = 201
     final BUILD_NUMBER = 123
     final QUEUED_JOB_NUMBER = 123456
-    final JOB_NAME = "job1"
+    final JOB_NAME = "job/name/can/have/slashes"
     final FILE_NAME = "test.yml"
 
     void cleanup() {
@@ -82,7 +82,7 @@ class BuildControllerSpec extends Specification {
         1 * client.getBuild(JOB_NAME, BUILD_NUMBER) >> new Build(number: BUILD_NUMBER)
 
         when:
-        MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/${JOB_NAME}/${BUILD_NUMBER}")
+        MockHttpServletResponse response = mockMvc.perform(get("/builds/status/${BUILD_NUMBER}/${MASTER}/${JOB_NAME}")
             .accept(MediaType.APPLICATION_JSON)).andReturn().response
 
         then:
@@ -94,7 +94,7 @@ class BuildControllerSpec extends Specification {
         1 * client.getQueuedItem(QUEUED_JOB_NUMBER) >> new QueuedJob(number: QUEUED_JOB_NUMBER)
 
         when:
-        MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/queue/${QUEUED_JOB_NUMBER}")
+        MockHttpServletResponse response = mockMvc.perform(get("/builds/queue/${MASTER}/${QUEUED_JOB_NUMBER}")
             .accept(MediaType.APPLICATION_JSON)).andReturn().response
 
         then:
@@ -106,7 +106,7 @@ class BuildControllerSpec extends Specification {
         1 * client.getBuilds(JOB_NAME) >> new BuildsList(list: [new Build(number: 111), new Build(number: 222)])
 
         when:
-        MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/${JOB_NAME}/builds")
+        MockHttpServletResponse response = mockMvc.perform(get("/builds/all/${MASTER}/${JOB_NAME}")
             .accept(MediaType.APPLICATION_JSON)).andReturn().response
 
         then:
@@ -119,7 +119,8 @@ class BuildControllerSpec extends Specification {
              number: BUILD_NUMBER, artifacts: [new BuildArtifact(fileName: FILE_NAME, relativePath: FILE_NAME)])
 
         when:
-        MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/${JOB_NAME}/${BUILD_NUMBER}/properties/${FILE_NAME}")
+        MockHttpServletResponse response = mockMvc.perform(
+            get("/builds/properties/${BUILD_NUMBER}/${FILE_NAME}/${MASTER}/${JOB_NAME}")
             .accept(MediaType.APPLICATION_JSON)).andReturn().response
 
         then: