Refactor JobRequest response structure

10 files changed, 233 insertions, 45 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index f2989eff22d..0c0bbb9d41d 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -508,6 +508,35 @@ module Ci
       ]
     end
 
+    def steps
+      [
+          Gitlab::Ci::Build::Response::Step.from_commands(self),
+          Gitlab::Ci::Build::Response::Step.from_after_script(self)
+      ].compact
+    end
+
+    def image
+      image = Gitlab::Ci::Build::Response::Image.new(options[:image])
+      return unless image.valid?
+      image
+    end
+
+    def services
+      services = options[:services].map do |service|
+        Gitlab::Ci::Build::Response::Image.new(service)
+      end
+
+      services.select(&:valid?).compact
+    end
+
+    def artifacts
+      options[:artifacts]
+    end
+
+    def cache
+      options[:cache]
+    end
+
     def credentials
       Gitlab::Ci::Build::Credentials::Factory.new(self).create!
     end
diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_job_service.rb
index 5b52a0425de..0ab9042bf24 100644
--- a/app/services/ci/register_build_service.rb
+++ b/app/services/ci/register_job_service.rb
@@ -1,7 +1,7 @@
 module Ci
   # This class responsible for assigning
   # proper pending build to runner on runner API request
-  class RegisterBuildService
+  class RegisterJobService
     include Gitlab::CurrentSettings
 
     attr_reader :runner
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index cb3b409b8d0..7e07154600a 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -698,42 +698,82 @@ module API
       expose :active?, as: :active
     end
 
-    class ArtifactFile < Grape::Entity
-      expose :filename, :size
-    end
+    module JobRequest
+      class JobInfo < Grape::Entity
+        expose :name, :stage
+        expose :project_id, :project_name
+      end
+
+      class GitInfo < Grape::Entity
+        expose :repo_url, :ref, :sha, :before_sha
+        expose :ref_type do |model|
+          if model.tag
+            'tag'
+          else
+            'branch'
+          end
+        end
+      end
 
-    class JobCredentials < Grape::Entity
-      expose :type, :url, :username, :password
-    end
+      class RunnerInfo < Grape::Entity
+        expose :timeout
+      end
 
-    class JobResponse < Grape::Entity
-      expose :id, :ref, :tag, :sha, :status
-      expose :name, :token, :stage
-      expose :project_id
-      expose :project_name
-      expose :artifacts_file, using: ArtifactFile, if: ->(build, _) { build.artifacts? }
-    end
+      class Step < Grape::Entity
+        expose :name, :script, :timeout, :condition, :result
+      end
 
-    class RequestJobResponse < JobResponse
-      expose :commands
-      expose :repo_url
-      expose :before_sha
-      expose :allow_git_fetch
-      expose :token
-      expose :artifacts_expire_at, if: ->(build, _) { build.artifacts? }
+      class Image < Grape::Entity
+        expose :name
+      end
 
-      expose :options do |model|
-        model.options
+      class Artifacts < Grape::Entity
+        expose :name, :untracked, :paths, :when, :expire_in
       end
 
-      expose :timeout do |model|
-        model.timeout
+      class Cache < Grape::Entity
+        expose :key, :untracked, :paths
       end
 
-      expose :variables
-      expose :depends_on_builds, using: JobResponse
+      class Credentials < Grape::Entity
+        expose :type, :url, :username, :password
+      end
 
-      expose :credentials, using: JobCredentials
+      class ArtifactFile < Grape::Entity
+        expose :filename, :size
+      end
+
+      class Dependency < Grape::Entity
+        expose :id, :name
+        expose :artifacts_file, using: ArtifactFile, if: ->(job, _) { job.artifacts? }
+      end
+
+      class Response < Grape::Entity
+        expose :id
+        expose :token
+        expose :allow_git_fetch
+
+        expose :job_info, using: JobInfo do |model|
+          model
+        end
+
+        expose :git_info, using: GitInfo do |model|
+          model
+        end
+
+        expose :runner_info, using: RunnerInfo do |model|
+          model
+        end
+
+        expose :variables
+        expose :steps, using: Step
+        expose :image, using: Image
+        expose :services, using: Image
+        expose :artifacts, using: Artifacts
+        expose :cache, using: Cache
+        expose :credentials, using: Credentials
+        expose :depends_on_builds, as: :dependencies, using: Dependency
+      end
     end
   end
 end
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index 2e7b96e5169..8372b794739 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -51,7 +51,7 @@ module API
 
     resource :jobs do
       desc 'Request a job' do
-        success Entities::RequestJobResponse
+        success Entities::JobRequest::Response
       end
       params do
         requires :token, type: String, desc: %q(Runner's authentication token)
@@ -68,13 +68,13 @@ module API
         end
 
         new_update = current_runner.ensure_runner_queue_value
-        result = ::Ci::RegisterBuildService.new(current_runner).execute
+        result = ::Ci::RegisterJobService.new(current_runner).execute
 
         if result.valid?
           if result.build
             Gitlab::Metrics.add_event(:build_found,
                                       project: result.build.project.path_with_namespace)
-            present result.build, with: Entities::RequestJobResponse
+            present result.build, with: Entities::JobRequest::Response
           else
             Gitlab::Metrics.add_event(:build_not_found)
             header 'X-GitLab-Last-Update', new_update
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index b51e76d93f2..746e76a1b1f 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -24,7 +24,7 @@ module Ci
 
           new_update = current_runner.ensure_runner_queue_value
 
-          result = Ci::RegisterBuildService.new(current_runner).execute
+          result = Ci::RegisterJobService.new(current_runner).execute
 
           if result.valid?
             if result.build
diff --git a/lib/gitlab/ci/build/response/image.rb b/lib/gitlab/ci/build/response/image.rb
new file mode 100644
index 00000000000..342a249aee8
--- /dev/null
+++ b/lib/gitlab/ci/build/response/image.rb
@@ -0,0 +1,20 @@
+module Gitlab
+  module Ci
+    module Build
+      module Response
+        class Image
+          attr_reader :name
+
+          def initialize(image)
+            type = image.class
+            @name = image if type == String
+          end
+
+          def valid?
+            @name != nil
+          end
+        end
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/lib/gitlab/ci/build/response/step.rb b/lib/gitlab/ci/build/response/step.rb
new file mode 100644
index 00000000000..7b35852a510
--- /dev/null
+++ b/lib/gitlab/ci/build/response/step.rb
@@ -0,0 +1,46 @@
+module Gitlab
+  module Ci
+    module Build
+      module Response
+        class Step
+          CONDITION_IF_SUCCEEDED = 'run_if_succeeded'
+          CONDITION_ALWAYS = 'run_always'
+
+          RESULT_FAILS_JOB = 'fails_job'
+          RESULT_DOESNT_FAIL_JOB = 'doesnt_fail_job'
+
+          attr_reader :name, :script, :condition, :result, :timeout
+
+          class << self
+            def from_commands(build)
+              self.new(:script,
+                       build.commands,
+                       build.timeout,
+                       CONDITION_IF_SUCCEEDED,
+                       RESULT_FAILS_JOB)
+            end
+
+            def from_after_script(build)
+              after_script = build.options[:after_script]
+              return unless after_script
+
+              self.new(:after_script,
+                       after_script,
+                       build.timeout,
+                       CONDITION_ALWAYS,
+                       RESULT_DOESNT_FAIL_JOB)
+            end
+          end
+
+          def initialize(name, script, timeout, condition = CONDITION_IF_SUCCEEDED, result = RESULT_FAILS_JOB)
+            @name = name
+            @script = script.split("\n")
+            @timeout = timeout
+            @condition = condition
+            @result = result
+          end
+        end
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index a90534d10ba..a77b3356b9a 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -15,8 +15,8 @@ FactoryGirl.define do
 
     options do
       {
-        image: "ruby:2.1",
-        services: ["postgres"]
+        image: 'ruby:2.1',
+        services: ['postgres']
       }
     end
 
@@ -151,5 +151,27 @@ FactoryGirl.define do
         allow(build).to receive(:commit).and_return build(:commit)
       end
     end
+
+    trait :extended_options do
+      options do
+        {
+            image: 'ruby:2.1',
+            services: ['postgres'],
+            after_script: "ls\ndate",
+            artifacts: {
+                name: 'artifacts_file',
+                untracked: false,
+                paths: ['out/'],
+                when: 'always',
+                expire_in: '7d'
+            },
+            cache: {
+                key: 'cache_key',
+                untracked: false,
+                paths: ['vendor/*']
+            }
+        }
+      end
+    end
   end
 end
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 3b94be55974..bd9dc422703 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -152,8 +152,8 @@ describe API::Runner do
   describe '/api/v4/jobs' do
     let(:project) { create(:empty_project, shared_runners_enabled: false) }
     let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
-    let!(:job) { create(:ci_build, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) }
     let(:runner) { create(:ci_runner) }
+    let!(:job) { create(:ci_build, :artifacts, :extended_options, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate") }
 
     before { project.runners << runner }
 
@@ -271,14 +271,44 @@ describe API::Runner do
 
             expect(response).to have_http_status(201)
             expect(response.headers).not_to have_key('X-GitLab-Last-Update')
-            expect(json_response['sha']).to eq(job.sha)
-            expect(json_response['options']).to eq({'image' => 'ruby:2.1', 'services' => ['postgres']})
-            expect(json_response['variables']).to include(
-                                                      {'key' => 'CI_BUILD_NAME', 'value' => 'spinach', 'public' => true},
-                                                      {'key' => 'CI_BUILD_STAGE', 'value' => 'test', 'public' => true},
-                                                      {'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true}
-                                                  )
             expect(runner.reload.platform).to eq('darwin')
+
+            expect(json_response['id']).to eq(job.id)
+            expect(json_response['token']).to eq(job.token)
+            expect(json_response['job_info']).to include({ 'name' => job.name },
+                                                         { 'stage' => job.stage })
+            expect(json_response['git_info']).to include({ 'sha' => job.sha },
+                                                         { 'repo_url' => job.repo_url })
+            expect(json_response['image']).to include({ 'name' => 'ruby:2.1' })
+            expect(json_response['services']).to include({ 'name' => 'postgres' })
+            expect(json_response['steps']).to include({ 'name' => 'after_script',
+                                                        'script' => ['ls', 'date'],
+                                                        'timeout' => job.timeout,
+                                                        'condition' => Gitlab::Ci::Build::Response::Step::CONDITION_ALWAYS,
+                                                        'result' => Gitlab::Ci::Build::Response::Step::RESULT_DOESNT_FAIL_JOB })
+            expect(json_response['variables']).to include({ 'key' => 'CI_BUILD_NAME', 'value' => 'spinach', 'public' => true },
+                                                          { 'key' => 'CI_BUILD_STAGE', 'value' => 'test', 'public' => true },
+                                                          { 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true })
+            expect(json_response['artifacts']).to include({ 'name' => 'artifacts_file' },
+                                                          { 'paths' => ['out/'] })
+          end
+
+          context 'when job is made for tag' do
+            let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) }
+
+            it 'sets branch as ref_type' do
+              request_job
+              expect(response).to have_http_status(201)
+              expect(json_response['git_info']['ref_type']).to eq('tag')
+            end
+          end
+
+          context 'when job is made for branch' do
+            it 'sets tag as ref_type' do
+              request_job
+              expect(response).to have_http_status(201)
+              expect(json_response['git_info']['ref_type']).to eq('branch')
+            end
           end
 
           it 'updates runner info' do
@@ -322,8 +352,8 @@ describe API::Runner do
 
               expect(response).to have_http_status(201)
               expect(json_response['id']).to eq(test_job.id)
-              expect(json_response['depends_on_builds'].count).to eq(1)
-              expect(json_response['depends_on_builds'][0]).to include('id' => job.id, 'name' => 'spinach')
+              expect(json_response['dependencies'].count).to eq(1)
+              expect(json_response['dependencies'][0]).to include('id' => job.id, 'name' => 'spinach')
             end
           end
 
@@ -381,6 +411,7 @@ describe API::Runner do
 
               it 'sends registry credentials key' do
                 request_job
+
                 expect(json_response).to have_key('credentials')
                 expect(json_response['credentials']).to include(registry_credentials)
               end
diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_job_service_spec.rb
index cd7dd53025c..ea8b996d481 100644
--- a/spec/services/ci/register_build_service_spec.rb
+++ b/spec/services/ci/register_job_service_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 module Ci
-  describe RegisterBuildService, services: true do
+  describe RegisterJobService, services: true do
     let!(:project) { FactoryGirl.create :empty_project, shared_runners_enabled: false }
     let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project }
     let!(:pending_build) { FactoryGirl.create :ci_build, pipeline: pipeline }