From c974173e85a92bff602ebf8406464887d0be5b6e Mon Sep 17 00:00:00 2001 From: Adam Jordens Date: Mon, 30 Nov 2015 17:30:13 -0800 Subject: [PATCH] s/JenkinsClient/JenkinsService - Discovered a couple spots that were explicitly casting to JenkinsClient (and no @CompileStatic) - Trying to make the usage of JenkinsService over JenkinsClient consistent --- .../igor/jenkins/BuildController.groovy | 17 ++++----- .../igor/jenkins/InfoController.groovy | 12 +++--- .../igor/jenkins/BuildControllerSpec.groovy | 22 +++++------ .../jenkins/BuildMonitorSchedulingSpec.groovy | 14 +++---- .../igor/jenkins/BuildMonitorSpec.groovy | 37 +++++++++---------- .../igor/jenkins/InfoControllerSpec.groovy | 9 ++--- 6 files changed, 53 insertions(+), 58 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 index e9f2cb553..81e810aa8 100644 --- 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 @@ -17,7 +17,6 @@ package com.netflix.spinnaker.igor.jenkins import com.fasterxml.jackson.databind.ObjectMapper -import com.netflix.spinnaker.igor.jenkins.client.JenkinsClient import com.netflix.spinnaker.igor.jenkins.client.JenkinsMasters import com.netflix.spinnaker.igor.jenkins.client.model.Build import com.netflix.spinnaker.igor.jenkins.client.model.JobConfig @@ -96,16 +95,16 @@ class BuildController { } def response - JenkinsClient client = masters.map[master] - JobConfig jobConfig = client.getJobConfig(job) + def jenkinsService = masters.map[master] + JobConfig jobConfig = jenkinsService.getJobConfig(job) if (requestParams && jobConfig.parameterDefinitionList?.size() > 0) { - response = client.buildWithParameters(job, requestParams) + response = jenkinsService.buildWithParameters(job, requestParams) } else if (!requestParams && jobConfig.parameterDefinitionList?.size() > 0) { // account for when you just want to fire a job with the default parameter values by adding a dummy param - response = client.buildWithParameters(job, ['startedBy': "igor"]) + response = jenkinsService.buildWithParameters(job, ['startedBy': "igor"]) } else if (!requestParams && (!jobConfig.parameterDefinitionList || jobConfig.parameterDefinitionList.size() == 0)) { - response = client.build(job) + response = jenkinsService.build(job) } else { // Jenkins will reject the build, so don't even try log.error("job : ${job}, passing params to a job which doesn't need them") // we should throw a BuildJobError, but I get a bytecode error : java.lang.VerifyError: Bad method call from inside of a branch @@ -135,12 +134,12 @@ class BuildController { } Map map = [:] try { - JenkinsClient jenkinsClient = masters.map[master] - String path = jenkinsClient.getBuild(job, buildNumber).artifacts.find { + def jenkinsService = masters.map[master] + String path = jenkinsService.getBuild(job, buildNumber).artifacts.find { it.fileName == fileName }?.relativePath - def propertyStream = jenkinsClient.getPropertyFile(job, buildNumber, path).body.in() + def propertyStream = jenkinsService.getPropertyFile(job, buildNumber, path).body.in() if (fileName.endsWith('.yml') || fileName.endsWith('.yaml')) { Yaml yml = new Yaml() 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 index f864568e1..8e1219e7a 100644 --- 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 @@ -68,24 +68,24 @@ class InfoController { List getJobs(@PathVariable String master) { log.info('Getting list of jobs for master: {}', master) - def jenkinsClient = masters.map[master] - if (!jenkinsClient) { + def jenkinsService = masters.map[master] + if (!jenkinsService) { throw new MasterNotFoundException("Master '${master}' does not exist") } - jenkinsClient.jobs.list.collect { it.name } + jenkinsService.jobs.list.collect { it.name } } @RequestMapping(value = '/jobs/{master}/{job:.+}') JobConfig getJobConfig(@PathVariable String master, @PathVariable String job) { log.info('Getting the job config for {} at {}', job, master) - def jenkinsClient = masters.map[master] - if (!jenkinsClient) { + def jenkinsService = masters.map[master] + if (!jenkinsService) { throw new MasterNotFoundException("Master '${master}' does not exist") } - jenkinsClient.getJobConfig(job) + jenkinsService.getJobConfig(job) } static class MasterResults { 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 index fd197bb81..a2449719b 100644 --- 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 @@ -16,16 +16,14 @@ package com.netflix.spinnaker.igor.jenkins -import com.netflix.spinnaker.igor.jenkins.client.JenkinsClient import com.netflix.spinnaker.igor.jenkins.client.JenkinsMasters -import com.netflix.spinnaker.igor.jenkins.client.model.Build import com.netflix.spinnaker.igor.jenkins.client.model.JobConfig import com.netflix.spinnaker.igor.jenkins.client.model.ParameterDefinition +import com.netflix.spinnaker.igor.jenkins.service.JenkinsService import retrofit.client.Header import retrofit.client.Response import spock.lang.Specification -import java.util.concurrent.ExecutionException import java.util.concurrent.Executors /** @@ -35,7 +33,7 @@ import java.util.concurrent.Executors class BuildControllerSpec extends Specification { JenkinsCache cache = Mock(JenkinsCache) - JenkinsClient client = Mock(JenkinsClient) + JenkinsService jenkinsService = Mock(JenkinsService) BuildController controller final MASTER = 'MASTER' @@ -45,13 +43,13 @@ class BuildControllerSpec extends Specification { final JOB_NAME = "job1" void setup() { - controller = new BuildController(executor: Executors.newSingleThreadExecutor(), masters: new JenkinsMasters(map: [MASTER: client])) + controller = new BuildController(executor: Executors.newSingleThreadExecutor(), masters: new JenkinsMasters(map: [MASTER: jenkinsService])) } void 'trigger a build without parameters'() { given: - 1 * client.getJobConfig(JOB_NAME) >> new JobConfig() - 1 * client.build(JOB_NAME) >> new Response("http://test.com", HTTP_201, "", [new Header("Location","foo/${QUEUED_JOB_NUMBER}")], null) + 1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig() + 1 * jenkinsService.build(JOB_NAME) >> new Response("http://test.com", HTTP_201, "", [new Header("Location","foo/${QUEUED_JOB_NUMBER}")], null) expect: controller.build(MASTER,JOB_NAME,null) == QUEUED_JOB_NUMBER.toString() @@ -60,8 +58,8 @@ class BuildControllerSpec extends Specification { void 'trigger a build with parameters to a job with parameters'() { given: - 1 * client.getJobConfig(JOB_NAME) >> new JobConfig(parameterDefinitionList: [new ParameterDefinition(defaultName: "name", defaultValue: null, description: "description")]) - 1 * client.buildWithParameters(JOB_NAME,[name:"myName"]) >> new Response("http://test.com", HTTP_201, "", [new Header("Location","foo/${QUEUED_JOB_NUMBER}")], null) + 1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(parameterDefinitionList: [new ParameterDefinition(defaultName: "name", defaultValue: null, description: "description")]) + 1 * jenkinsService.buildWithParameters(JOB_NAME,[name:"myName"]) >> new Response("http://test.com", HTTP_201, "", [new Header("Location","foo/${QUEUED_JOB_NUMBER}")], null) expect: controller.build(MASTER,JOB_NAME, [name:"myName"]) == QUEUED_JOB_NUMBER.toString() @@ -69,8 +67,8 @@ class BuildControllerSpec extends Specification { void 'trigger a build without parameters to a job with parameters with default values'() { given: - 1 * client.getJobConfig(JOB_NAME) >> new JobConfig(parameterDefinitionList: [new ParameterDefinition(defaultName: "name", defaultValue: "value", description: "description")]) - 1 * client.buildWithParameters(JOB_NAME, ['startedBy' : "igor"]) >> new Response("http://test.com", HTTP_201, "", [new Header("Location","foo/${QUEUED_JOB_NUMBER}")], null) + 1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(parameterDefinitionList: [new ParameterDefinition(defaultName: "name", defaultValue: "value", description: "description")]) + 1 * jenkinsService.buildWithParameters(JOB_NAME, ['startedBy': "igor"]) >> new Response("http://test.com", HTTP_201, "", [new Header("Location","foo/${QUEUED_JOB_NUMBER}")], null) expect: controller.build(MASTER, JOB_NAME, null) == QUEUED_JOB_NUMBER.toString() @@ -78,7 +76,7 @@ class BuildControllerSpec extends Specification { void 'trigger a build with parameters to a job without parameters'() { given: - 1 * client.getJobConfig(JOB_NAME) >> new JobConfig() + 1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig() when: controller.build(MASTER, JOB_NAME, [foo:"bar"]) diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSchedulingSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSchedulingSpec.groovy index fe66423a9..8dd2b8520 100644 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSchedulingSpec.groovy +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSchedulingSpec.groovy @@ -16,9 +16,9 @@ package com.netflix.spinnaker.igor.jenkins -import com.netflix.spinnaker.igor.jenkins.client.JenkinsClient import com.netflix.spinnaker.igor.jenkins.client.JenkinsMasters import com.netflix.spinnaker.igor.jenkins.client.model.ProjectsList +import com.netflix.spinnaker.igor.jenkins.service.JenkinsService import org.springframework.context.event.ContextRefreshedEvent import rx.schedulers.TestScheduler import spock.lang.Specification @@ -32,7 +32,7 @@ import java.util.concurrent.TimeUnit class BuildMonitorSchedulingSpec extends Specification { JenkinsCache cache = Mock(JenkinsCache) - JenkinsClient client = Mock(JenkinsClient) + JenkinsService jenkinsService = Mock(JenkinsService) BuildMonitor monitor final MASTER = 'MASTER' @@ -42,7 +42,7 @@ class BuildMonitorSchedulingSpec extends Specification { void 'scheduller polls periodically'() { given: cache.getJobNames(MASTER) >> [] - monitor = new BuildMonitor(cache: cache, jenkinsMasters: new JenkinsMasters(map: [MASTER: client])) + monitor = new BuildMonitor(cache: cache, jenkinsMasters: new JenkinsMasters(map: [MASTER: jenkinsService])) monitor.worker = scheduler.createWorker() monitor.pollInterval = 1 @@ -51,25 +51,25 @@ class BuildMonitorSchedulingSpec extends Specification { scheduler.advanceTimeBy(1L, TimeUnit.SECONDS.MILLISECONDS) then: 'initial poll' - 1 * client.projects >> PROJECTS + 1 * jenkinsService.projects >> PROJECTS when: scheduler.advanceTimeBy(998L, TimeUnit.SECONDS.MILLISECONDS) then: - 0 * client.projects >> PROJECTS + 0 * jenkinsService.projects >> PROJECTS when: 'poll at 1 second' scheduler.advanceTimeBy(2L, TimeUnit.SECONDS.MILLISECONDS) then: - 1 * client.projects >> PROJECTS + 1 * jenkinsService.projects >> PROJECTS when: 'poll at 2 and 3 second' scheduler.advanceTimeBy(4000L, TimeUnit.SECONDS.MILLISECONDS) then: - 4 * client.projects >> PROJECTS + 4 * jenkinsService.projects >> PROJECTS cleanup: monitor.stop() diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSpec.groovy index 031d32eb9..037ded250 100644 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSpec.groovy +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildMonitorSpec.groovy @@ -17,14 +17,13 @@ package com.netflix.spinnaker.igor.jenkins import com.netflix.spinnaker.igor.history.EchoService -import com.netflix.spinnaker.igor.jenkins.client.JenkinsClient import com.netflix.spinnaker.igor.jenkins.client.JenkinsMasters import com.netflix.spinnaker.igor.jenkins.client.model.Build import com.netflix.spinnaker.igor.jenkins.client.model.BuildsList import com.netflix.spinnaker.igor.jenkins.client.model.Project import com.netflix.spinnaker.igor.jenkins.client.model.ProjectsList +import com.netflix.spinnaker.igor.jenkins.service.JenkinsService import spock.lang.Specification -import spock.lang.IgnoreRest /** * Tests for BuildMonitor @@ -33,19 +32,19 @@ import spock.lang.IgnoreRest class BuildMonitorSpec extends Specification { JenkinsCache cache = Mock(JenkinsCache) - JenkinsClient client = Mock(JenkinsClient) + JenkinsService jenkinsService = Mock(JenkinsService) BuildMonitor monitor final MASTER = 'MASTER' void setup() { - monitor = new BuildMonitor(cache: cache, jenkinsMasters: new JenkinsMasters(map: [MASTER: client])) + monitor = new BuildMonitor(cache: cache, jenkinsMasters: new JenkinsMasters(map: [MASTER: jenkinsService])) } void 'flag a new build not found in the cache'() { given: 1 * cache.getJobNames(MASTER) >> ['job1'] - 1 * client.projects >> new ProjectsList(list: [new Project(name: 'job2', lastBuild : new Build(number: 1))]) + 1 * jenkinsService.projects >> new ProjectsList(list: [new Project(name: 'job2', lastBuild : new Build(number: 1))]) when: List builds = monitor.changedBuilds(MASTER) @@ -62,7 +61,7 @@ class BuildMonitorSpec extends Specification { void 'flag existing build with a higher number as changed'() { given: 1 * cache.getJobNames(MASTER) >> ['job2'] - 1 * client.projects >> new ProjectsList(list: [new Project(name: 'job2', lastBuild : new Build(number: 5))]) + 1 * jenkinsService.projects >> new ProjectsList(list: [new Project(name: 'job2', lastBuild : new Build(number: 5))]) when: List builds = monitor.changedBuilds(MASTER) @@ -77,7 +76,7 @@ class BuildMonitorSpec extends Specification { void 'flag builds in a different state as changed'() { given: 1 * cache.getJobNames(MASTER) >> ['job3'] - 1 * client.projects >> new ProjectsList( + 1 * jenkinsService.projects >> new ProjectsList( list: [new Project(name: 'job3', lastBuild : new Build(number: 5, building: false))] ) @@ -96,7 +95,7 @@ class BuildMonitorSpec extends Specification { void 'stale builds are removed'() { given: 1 * cache.getJobNames(MASTER) >> ['job3', 'job4'] - 1 * client.projects >> new ProjectsList(list: []) + 1 * jenkinsService.projects >> new ProjectsList(list: []) when: monitor.changedBuilds(MASTER) @@ -109,10 +108,10 @@ class BuildMonitorSpec extends Specification { void 'sends an event for every intermediate build'(){ given: 1 * cache.getJobNames(MASTER) >> ['job'] - 1 * client.projects >> new ProjectsList( + 1 * jenkinsService.projects >> new ProjectsList( list: [new Project(name: 'job', lastBuild: new Build(number: 6, building: false))] ) - 1 * client.getBuilds('job') >> new BuildsList( + 1 * jenkinsService.getBuilds('job') >> new BuildsList( list: [ new Build(number: 1), new Build(number: 2), @@ -140,10 +139,10 @@ class BuildMonitorSpec extends Specification { void 'emits events only for builds in list'(){ given: 1 * cache.getJobNames(MASTER) >> ['job'] - 1 * client.projects >> new ProjectsList( + 1 * jenkinsService.projects >> new ProjectsList( list: [new Project(name: 'job', lastBuild: new Build(number: 6, building: false))] ) - 1 * client.getBuilds('job') >> new BuildsList( + 1 * jenkinsService.getBuilds('job') >> new BuildsList( list: [ new Build(number: 1), new Build(number: 3, building: false), @@ -166,7 +165,7 @@ class BuildMonitorSpec extends Specification { void 'does not send event for current unchanged build'(){ given: 1 * cache.getJobNames(MASTER) >> ['job'] - 1 * client.projects >> new ProjectsList( + 1 * jenkinsService.projects >> new ProjectsList( list: [new Project(name: 'job', lastBuild: new Build(number: 3, building: true))] ) @@ -177,17 +176,17 @@ class BuildMonitorSpec extends Specification { then: 1 * cache.getLastBuild(MASTER, 'job') >> [lastBuildLabel: 3, lastBuildBuilding: true] - 0 * client.getBuilds('job') + 0 * jenkinsService.getBuilds('job') 0 * monitor.echoService.postBuild(_) } void 'does not send event for past build with already sent event'(){ given: 1 * cache.getJobNames(MASTER) >> ['job'] - 1 * client.projects >> new ProjectsList( + 1 * jenkinsService.projects >> new ProjectsList( list: [new Project(name: 'job', lastBuild: new Build(number: 6, building: true))] ) - 1 * client.getBuilds('job') >> new BuildsList( + 1 * jenkinsService.getBuilds('job') >> new BuildsList( list: [ new Build(number: 5, building: false), new Build(number: 6) @@ -209,7 +208,7 @@ class BuildMonitorSpec extends Specification { void 'does not send event for same build'(){ given: 1 * cache.getJobNames(MASTER) >> ['job'] - 1 * client.projects >> new ProjectsList( + 1 * jenkinsService.projects >> new ProjectsList( list: [new Project(name: 'job', lastBuild: new Build(number: 6, building: true))] ) @@ -220,7 +219,7 @@ class BuildMonitorSpec extends Specification { then: 1 * cache.getLastBuild(MASTER, 'job') >> [lastBuildLabel: 6, lastBuildBuilding: true] - 0 * client.getBuilds('job') + 0 * jenkinsService.getBuilds('job') 0 * monitor.echoService.postBuild({_}) } @@ -228,7 +227,7 @@ class BuildMonitorSpec extends Specification { void 'sends event for same build that has finished'(){ given: 1 * cache.getJobNames(MASTER) >> ['job'] - 1 * client.projects >> new ProjectsList( + 1 * jenkinsService.projects >> new ProjectsList( list: [new Project(name: 'job', lastBuild: new Build(number: 6, building: true))] ) diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/InfoControllerSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/InfoControllerSpec.groovy index 221d8dd98..ea0800c69 100644 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/InfoControllerSpec.groovy +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/InfoControllerSpec.groovy @@ -17,7 +17,7 @@ package com.netflix.spinnaker.igor.jenkins import com.netflix.spinnaker.igor.config.JenkinsConfig -import com.netflix.spinnaker.igor.jenkins.client.JenkinsClient +import com.netflix.spinnaker.igor.jenkins.service.JenkinsService import com.squareup.okhttp.mockwebserver.MockResponse import com.squareup.okhttp.mockwebserver.MockWebServer import spock.lang.Shared @@ -30,7 +30,6 @@ import org.springframework.mock.web.MockHttpServletResponse import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.setup.MockMvcBuilders import spock.lang.Specification -import spock.lang.Unroll /** * tests for the info controller @@ -43,7 +42,7 @@ class InfoControllerSpec extends Specification { JenkinsMasters masters @Shared - JenkinsClient client + JenkinsService service @Shared MockWebServer server @@ -93,7 +92,7 @@ class InfoControllerSpec extends Specification { .setHeader('Content-Type', 'text/xml;charset=UTF-8') ) server.play() - client = new JenkinsConfig().jenkinsClient(server.getUrl('/').toString(), 'username', 'password') + service = new JenkinsConfig().jenkinsService("jenkins", new JenkinsConfig().jenkinsClient(server.getUrl('/').toString(), 'username', 'password')) } void 'is able to get a job config'() { @@ -105,7 +104,7 @@ class InfoControllerSpec extends Specification { .accept(MediaType.APPLICATION_JSON)).andReturn().response then: - 1 * masters.map >> ['master2': [], 'build.masters.blah': [], 'master1': client] + 1 * masters.map >> ['master2': [], 'build.masters.blah': [], 'master1': service] response.contentAsString == '{"description":null,"displayName":"My-Build","name":"My-Build","buildable":true,"color":"red","url":"http://jenkins.builds.net/job/My-Build/","parameterDefinitionList":[{"defaultName":"pullRequestSourceBranch","defaultValue":"master","name":"pullRequestSourceBranch","description":null,"type":"StringParameterDefinition"},{"defaultName":"generation","defaultValue":"4","name":"generation","description":null,"type":"StringParameterDefinition"}],"upstreamProjectList":[{"name":"Upstream-Build","url":"http://jenkins.builds.net/job/Upstream-Build/","color":"blue"}],"downstreamProjectList":[{"name":"First-Downstream-Build","url":"http://jenkins.builds.net/job/First-Downstream-Build/","color":"blue"},{"name":"Second-Downstream-Build","url":"http://jenkins.builds.net/job/Second-Downstream-Build/","color":"blue"},{"name":"Third-Downstream-Build","url":"http://jenkins.builds.net/job/Third-Downstream-Build/","color":"red"}],"concurrentBuild":false}' }