From f7da15bae3e41e1a3fe30918887928c8908ccbe3 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 6 Sep 2017 23:09:41 +0900 Subject: Use before_save :set_protected --- app/models/ci/build.rb | 6 +++++- app/models/ci/pipeline.rb | 6 +++++- app/services/ci/create_pipeline_service.rb | 3 +-- lib/gitlab/ci/stage/seed.rb | 9 +-------- spec/services/ci/retry_build_service_spec.rb | 11 +++++++++++ 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ba3156154ac..11390d3aa0d 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -27,7 +27,6 @@ module Ci validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true - validates :protected, inclusion: { in: [true, false], unless: :importing? }, on: :create scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } @@ -47,6 +46,7 @@ module Ci before_save :update_artifacts_size, if: :artifacts_file_changed? before_save :ensure_token + before_save :set_protected before_destroy { unscoped_project } after_create do |build| @@ -461,6 +461,10 @@ module Ci end end + def set_protected + self.protected = pipeline.protected + end + def erase_trace! trace.erase! end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 35d14b6e297..f51b7d7d2a7 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -36,9 +36,9 @@ module Ci validates :sha, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } validates :status, presence: { unless: :importing? } - validates :protected, inclusion: { in: [true, false], unless: :importing? }, on: :create validate :valid_commit_sha, unless: :importing? + # before_save :set_protected after_create :keep_around_commits, unless: :importing? enum source: { @@ -445,6 +445,10 @@ module Ci statuses.latest.status || 'skipped' end + def set_protected + self.protected = project.protected_for?(self.ref) + end + def keep_around_commits return unless project diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 414c01b2546..de2cd7e87be 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -12,8 +12,7 @@ module Ci tag: tag?, trigger_requests: Array(trigger_request), user: current_user, - pipeline_schedule: schedule, - protected: project.protected_for?(ref) + pipeline_schedule: schedule ) result = validate(current_user, diff --git a/lib/gitlab/ci/stage/seed.rb b/lib/gitlab/ci/stage/seed.rb index e19aae35a81..f81f9347b4d 100644 --- a/lib/gitlab/ci/stage/seed.rb +++ b/lib/gitlab/ci/stage/seed.rb @@ -28,8 +28,7 @@ module Gitlab attributes.merge(project: project, ref: pipeline.ref, tag: pipeline.tag, - trigger_request: trigger, - protected: protected_ref?) + trigger_request: trigger) end end @@ -44,12 +43,6 @@ module Gitlab end end end - - private - - def protected_ref? - @protected_ref ||= project.protected_for?(pipeline.ref) - end end end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index f5ed9ff608f..f0eb700d342 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -52,6 +52,17 @@ describe Ci::RetryBuildService do expect(new_build.send(attribute)).to eq build.send(attribute) end end + + context 'when job has nullified protected' do + before do + build.update_attribute(:protected, nil) + end + + it "clones protected build attribute" do + expect(new_build.protected).not_to be_nil + expect(new_build.protected).to eq build.protected + end + end end describe 'reject acessors' do -- cgit v1.2.1 From 4b8c52f2fc13711ca54bd5274bc1041f69607eb5 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 6 Sep 2017 23:19:23 +0900 Subject: Fix /models/ci/pipeline.rb --- app/models/ci/pipeline.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index f51b7d7d2a7..fe8f12df70b 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -38,7 +38,7 @@ module Ci validates :status, presence: { unless: :importing? } validate :valid_commit_sha, unless: :importing? - # before_save :set_protected + before_save :set_protected after_create :keep_around_commits, unless: :importing? enum source: { -- cgit v1.2.1 From c4c383e6d5e38de260434fe414fbdbe02d0e3763 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 7 Sep 2017 00:30:44 +0900 Subject: Revert set_protected --- app/models/ci/build.rb | 6 +----- app/models/ci/pipeline.rb | 6 +----- app/services/ci/create_pipeline_service.rb | 3 ++- lib/gitlab/ci/stage/seed.rb | 9 ++++++++- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 11390d3aa0d..ba3156154ac 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -27,6 +27,7 @@ module Ci validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true + validates :protected, inclusion: { in: [true, false], unless: :importing? }, on: :create scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } @@ -46,7 +47,6 @@ module Ci before_save :update_artifacts_size, if: :artifacts_file_changed? before_save :ensure_token - before_save :set_protected before_destroy { unscoped_project } after_create do |build| @@ -461,10 +461,6 @@ module Ci end end - def set_protected - self.protected = pipeline.protected - end - def erase_trace! trace.erase! end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fe8f12df70b..35d14b6e297 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -36,9 +36,9 @@ module Ci validates :sha, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } validates :status, presence: { unless: :importing? } + validates :protected, inclusion: { in: [true, false], unless: :importing? }, on: :create validate :valid_commit_sha, unless: :importing? - before_save :set_protected after_create :keep_around_commits, unless: :importing? enum source: { @@ -445,10 +445,6 @@ module Ci statuses.latest.status || 'skipped' end - def set_protected - self.protected = project.protected_for?(self.ref) - end - def keep_around_commits return unless project diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index de2cd7e87be..414c01b2546 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -12,7 +12,8 @@ module Ci tag: tag?, trigger_requests: Array(trigger_request), user: current_user, - pipeline_schedule: schedule + pipeline_schedule: schedule, + protected: project.protected_for?(ref) ) result = validate(current_user, diff --git a/lib/gitlab/ci/stage/seed.rb b/lib/gitlab/ci/stage/seed.rb index f81f9347b4d..e19aae35a81 100644 --- a/lib/gitlab/ci/stage/seed.rb +++ b/lib/gitlab/ci/stage/seed.rb @@ -28,7 +28,8 @@ module Gitlab attributes.merge(project: project, ref: pipeline.ref, tag: pipeline.tag, - trigger_request: trigger) + trigger_request: trigger, + protected: protected_ref?) end end @@ -43,6 +44,12 @@ module Gitlab end end end + + private + + def protected_ref? + @protected_ref ||= project.protected_for?(pipeline.ref) + end end end end -- cgit v1.2.1 From 608cd406076dadc641815dca0fb917317fa80b39 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 7 Sep 2017 00:31:22 +0900 Subject: Remove only validation --- app/models/ci/build.rb | 1 - app/models/ci/pipeline.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ba3156154ac..db4ecb8f8d0 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -27,7 +27,6 @@ module Ci validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true - validates :protected, inclusion: { in: [true, false], unless: :importing? }, on: :create scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 35d14b6e297..46e5c344fdc 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -36,7 +36,6 @@ module Ci validates :sha, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } validates :status, presence: { unless: :importing? } - validates :protected, inclusion: { in: [true, false], unless: :importing? }, on: :create validate :valid_commit_sha, unless: :importing? after_create :keep_around_commits, unless: :importing? -- cgit v1.2.1 From 6c012c3b4c9a8b9a89c14403fc7f415c81b90722 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 7 Sep 2017 01:13:49 +0900 Subject: Fix spec --- spec/services/ci/retry_build_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index f0eb700d342..bbc3a8c79f5 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -59,7 +59,7 @@ describe Ci::RetryBuildService do end it "clones protected build attribute" do - expect(new_build.protected).not_to be_nil + expect(new_build.protected).to be_nil expect(new_build.protected).to eq build.protected end end -- cgit v1.2.1