From b8b2928f77f0a76c9560f7138c5c26e112fdc787 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 28 Jan 2019 12:49:15 +0100 Subject: Add bridge variables trait to the factory --- spec/factories/ci/bridge.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/factories/ci/bridge.rb b/spec/factories/ci/bridge.rb index 39427f416a0..b1d82b98411 100644 --- a/spec/factories/ci/bridge.rb +++ b/spec/factories/ci/bridge.rb @@ -10,6 +10,10 @@ FactoryBot.define do pipeline factory: :ci_pipeline + trait :variables do + yaml_variables [{ key: 'BRIDGE', value: 'cross', public: true }] + end + transient { downstream nil } after(:build) do |bridge, evaluator| -- cgit v1.2.1 From d434af46f4e8f2f3d149bf15674098afe9710a4a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 28 Jan 2019 13:37:09 +0100 Subject: Extract processable metadata to a separate concern We extracted implementation of build/bridge metadata attributes to a separate concern, because in EE `Ci::Bridge` also has metadata attributes, and we want to build abstraction for storing values in build metadata table. --- app/models/ci/build.rb | 51 +------------------------- app/models/ci/build_metadata.rb | 4 +- app/models/concerns/ci/metadatable.rb | 69 +++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 51 deletions(-) create mode 100644 app/models/concerns/ci/metadatable.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 35cf4f8d277..84010e40ef4 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -4,6 +4,7 @@ module Ci class Build < CommitStatus prepend ArtifactMigratable include Ci::Processable + include Ci::Metadatable include TokenAuthenticatable include AfterCommitQueue include ObjectStorage::BackgroundMove @@ -37,12 +38,10 @@ module Ci has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id end - has_one :metadata, class_name: 'Ci::BuildMetadata', autosave: true has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build accepts_nested_attributes_for :runner_session - delegate :timeout, to: :metadata, prefix: true, allow_nil: true delegate :url, to: :runner_session, prefix: true, allow_nil: true delegate :terminal_specification, to: :runner_session, allow_nil: true delegate :gitlab_deploy_token, to: :project @@ -133,7 +132,6 @@ module Ci before_save :ensure_token before_destroy { unscoped_project } - before_create :ensure_metadata after_create unless: :importing? do |build| run_after_commit { BuildHooksWorker.perform_async(build.id) } end @@ -261,10 +259,6 @@ module Ci end end - def ensure_metadata - metadata || build_metadata(project: project) - end - def detailed_status(current_user) Gitlab::Ci::Status::Build::Factory .new(self, current_user) @@ -284,18 +278,6 @@ module Ci self.name == 'pages' end - # degenerated build is one that cannot be run by Runner - def degenerated? - self.options.blank? - end - - def degenerate! - Build.transaction do - self.update!(options: nil, yaml_variables: nil) - self.metadata&.destroy - end - end - def archived? return true if degenerated? @@ -639,22 +621,6 @@ module Ci super || project.try(:build_coverage_regex) end - def options - read_metadata_attribute(:options, :config_options, {}) - end - - def yaml_variables - read_metadata_attribute(:yaml_variables, :config_variables, []) - end - - def options=(value) - write_metadata_attribute(:options, :config_options, value) - end - - def yaml_variables=(value) - write_metadata_attribute(:yaml_variables, :config_variables, value) - end - def user_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| break variables if user.blank? @@ -956,20 +922,5 @@ module Ci def project_destroyed? project.pending_delete? end - - def read_metadata_attribute(legacy_key, metadata_key, default_value = nil) - read_attribute(legacy_key) || metadata&.read_attribute(metadata_key) || default_value - end - - def write_metadata_attribute(legacy_key, metadata_key, value) - # save to metadata or this model depending on the state of feature flag - if Feature.enabled?(:ci_build_metadata_config) - ensure_metadata.write_attribute(metadata_key, value) - write_attribute(legacy_key, nil) - else - write_attribute(legacy_key, value) - metadata&.write_attribute(metadata_key, nil) - end - end end end diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb index 38390f49217..06e4584863d 100644 --- a/app/models/ci/build_metadata.rb +++ b/app/models/ci/build_metadata.rb @@ -10,7 +10,9 @@ module Ci self.table_name = 'ci_builds_metadata' - belongs_to :build, class_name: 'Ci::Build' + belongs_to :build, class_name: 'CommitStatus', + polymorphic: true, # rubocop:disable Cop/PolymorphicAssociations + inverse_of: :metadata belongs_to :project before_create :set_build_project diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb new file mode 100644 index 00000000000..9eed9492b37 --- /dev/null +++ b/app/models/concerns/ci/metadatable.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module Ci + ## + # This module implements methods that need to read and write + # metadata for CI/CD entities. + # + module Metadatable + extend ActiveSupport::Concern + + included do + has_one :metadata, class_name: 'Ci::BuildMetadata', + foreign_key: :build_id, + inverse_of: :build, + autosave: true + + delegate :timeout, to: :metadata, prefix: true, allow_nil: true + before_create :ensure_metadata + end + + def ensure_metadata + metadata || build_metadata(project: project) + end + + def degenerated? + self.options.blank? + end + + def degenerate! + self.class.transaction do + self.update!(options: nil, yaml_variables: nil) + self.metadata&.destroy + end + end + + def options + read_metadata_attribute(:options, :config_options, {}) + end + + def yaml_variables + read_metadata_attribute(:yaml_variables, :config_variables, []) + end + + def options=(value) + write_metadata_attribute(:options, :config_options, value) + end + + def yaml_variables=(value) + write_metadata_attribute(:yaml_variables, :config_variables, value) + end + + private + + def read_metadata_attribute(legacy_key, metadata_key, default_value = nil) + read_attribute(legacy_key) || metadata&.read_attribute(metadata_key) || default_value + end + + def write_metadata_attribute(legacy_key, metadata_key, value) + # save to metadata or this model depending on the state of feature flag + if Feature.enabled?(:ci_build_metadata_config) + ensure_metadata.write_attribute(metadata_key, value) + write_attribute(legacy_key, nil) + else + write_attribute(legacy_key, value) + metadata&.write_attribute(metadata_key, nil) + end + end + end +end -- cgit v1.2.1 From 9f307421a7c6f18a9879598e107f9e36456abd83 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 29 Jan 2019 14:40:47 +0100 Subject: Simplify relation between a build and metadata This removes erroneously defined polymorphic association, because specifying `belongs_to` relationship with a class that already supports polymorphic associations works out-of-the-box. --- app/models/ci/build_metadata.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb index 06e4584863d..cd8eb774cf5 100644 --- a/app/models/ci/build_metadata.rb +++ b/app/models/ci/build_metadata.rb @@ -10,9 +10,7 @@ module Ci self.table_name = 'ci_builds_metadata' - belongs_to :build, class_name: 'CommitStatus', - polymorphic: true, # rubocop:disable Cop/PolymorphicAssociations - inverse_of: :metadata + belongs_to :build, class_name: 'CommitStatus' belongs_to :project before_create :set_build_project -- cgit v1.2.1