From 9d8dca08e440ccb730f20dcd79b3b47aef8aeb2e Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 29 Jun 2016 17:44:39 +0800
Subject: [PATCH] Use AR callbacks as suggested by:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4964#note_12744656
---
 app/models/ci/build.rb              | 6 +++++-
 lib/ci/api/builds.rb                | 1 -
 spec/requests/ci/api/builds_spec.rb | 8 ++++++--
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 0f8c9511ce1..2079d5a2178 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -19,6 +19,7 @@ module Ci
 
     acts_as_taggable
 
+    before_save :update_artifacts_size, if: :artifacts_file_changed?
     before_destroy { project }
 
     after_create :execute_hooks
@@ -340,7 +341,6 @@ module Ci
     def erase_artifacts!
       remove_artifacts_file!
       remove_artifacts_metadata!
-      self.artifacts_size = nil
       save
     end
 
@@ -381,6 +381,10 @@ module Ci
 
     private
 
+    def update_artifacts_size
+      self.artifacts_size = artifacts_file.size
+    end
+
     def erase_trace!
       self.trace = nil
     end
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index f6a8d907066..260ac81f5fa 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -147,7 +147,6 @@ module Ci
           build.artifacts_file = artifacts
           build.artifacts_metadata = metadata
           build.artifacts_expire_in = params['expire_in']
-          build.artifacts_size = artifacts.size
 
           if build.save
             present(build, with: Entities::BuildDetails)
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index de1ec8fd40d..64cb7dd12d0 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -476,13 +476,17 @@ describe Ci::API::API do
 
       describe 'DELETE /builds/:id/artifacts' do
         let(:build) { create(:ci_build, :artifacts) }
-        before { delete delete_url, token: build.token }
+
+        before do
+          delete delete_url, token: build.token
+          build.reload
+        end
 
         it 'should remove build artifacts' do
           expect(response).to have_http_status(200)
           expect(build.artifacts_file.exists?).to be_falsy
           expect(build.artifacts_metadata.exists?).to be_falsy
-          expect(build.artifacts_size).to be_falsy
+          expect(build.artifacts_size).to eq(0)
         end
       end