Add some fixes and refactoring after review

5 files changed, 92 insertions, 65 deletions

diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index 23db32c9b1f..caa330c7234 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -176,6 +176,7 @@ module API
       end
 
       desc 'Upload artifacts for job' do
+        success Entities::JobRequest::Response
         http_codes [[201, 'Artifact uploaded'],
                     [400, 'Bad request'],
                     [403, 'Forbidden'],
@@ -186,7 +187,7 @@ module API
         requires :id, type: Integer, desc: %q(Job's ID)
         optional :token, type: String, desc: %q(Job's authentication token)
         optional :expire_in, type: String, desc: %q(Specify when artifacts should expire)
-        optional 'file', type: File, desc: %q(Artifact's file)
+        optional :file, type: File, desc: %q(Artifact's file)
         optional 'file.path', type: String, desc: %q(path to locally stored body (generated by Workhorse))
         optional 'file.name', type: String, desc: %q(real filename as send in Content-Disposition (generated by Workhorse))
         optional 'file.type', type: String, desc: %q(real content type as send in Content-Type (generated by Workhorse))
diff --git a/lib/gitlab/ci/build/image.rb b/lib/gitlab/ci/build/image.rb
new file mode 100644
index 00000000000..c0663f1ae8a
--- /dev/null
+++ b/lib/gitlab/ci/build/image.rb
@@ -0,0 +1,34 @@
+module Gitlab
+  module Ci
+    module Build
+      class Image
+        attr_reader :name
+
+        class << self
+          def from_image(job)
+            image = Gitlab::Ci::Build::Image.new(job.options[:image])
+            return unless image.valid?
+            image
+          end
+
+          def from_services(job)
+            services = job.options[:services].to_a.map do |service|
+              Gitlab::Ci::Build::Image.new(service)
+            end
+
+            services.select(&:valid?).compact
+          end
+        end
+
+        def initialize(image)
+          type = image.class
+          @name = image if type == String
+        end
+
+        def valid?
+          @name.present?
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/build/response/image.rb b/lib/gitlab/ci/build/response/image.rb
deleted file mode 100644
index c160689a2e1..00000000000
--- a/lib/gitlab/ci/build/response/image.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-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
diff --git a/lib/gitlab/ci/build/response/step.rb b/lib/gitlab/ci/build/response/step.rb
deleted file mode 100644
index 98c531c1d99..00000000000
--- a/lib/gitlab/ci/build/response/step.rb
+++ /dev/null
@@ -1,44 +0,0 @@
-module Gitlab
-  module Ci
-    module Build
-      module Response
-        class Step
-          CONDITION_ON_FAILURE = 'on_failure'.freeze
-          CONDITION_ON_SUCCESS = 'on_success'.freeze
-          CONDITION_ALWAYS = 'always'.freeze
-
-          attr_reader :name, :script, :when, :allow_failure, :timeout
-
-          class << self
-            def from_commands(build)
-              self.new(:script,
-                       build.commands,
-                       build.timeout,
-                       CONDITION_ON_SUCCESS,
-                       false)
-            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,
-                       true)
-            end
-          end
-
-          def initialize(name, script, timeout, when_condition = CONDITION_ON_SUCCESS, allow_failure = true)
-            @name = name
-            @script = script.split("\n")
-            @timeout = timeout
-            @when = when_condition
-            @allow_failure = allow_failure
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/build/step.rb b/lib/gitlab/ci/build/step.rb
new file mode 100644
index 00000000000..f857ab6063a
--- /dev/null
+++ b/lib/gitlab/ci/build/step.rb
@@ -0,0 +1,56 @@
+module Gitlab
+  module Ci
+    module Build
+      class Step
+        WHEN_ON_FAILURE = 'on_failure'.freeze
+        WHEN_ON_SUCCESS = 'on_success'.freeze
+        WHEN_ALWAYS = 'always'.freeze
+
+        attr_reader :name, :script, :timeout, :when, :allow_failure
+
+        class << self
+          def from_commands(job)
+            self.new(:script).tap do |step|
+              step.script = job.commands
+              step.timeout = job.timeout
+              step.when = WHEN_ON_SUCCESS
+            end
+          end
+
+          def from_after_script(job)
+            after_script = job.options[:after_script]
+            return unless after_script
+
+            self.new(:after_script).tap do |step|
+              step.script = after_script
+              step.timeout = job.timeout
+              step.when = WHEN_ALWAYS
+              step.allow_failure_on
+            end
+          end
+        end
+
+        def initialize(name)
+          @name = name
+          @allow_failure = false
+        end
+
+        def script=(script)
+          @script = script.split("\n")
+        end
+
+        def timeout=(timeout)
+          @timeout = timeout
+        end
+
+        def when=(when_condition)
+          @when = when_condition
+        end
+
+        def allow_failure_on
+          @allow_failure = true
+        end
+      end
+    end
+  end
+end