From 2a18fb20414b0d825235aaf6b35718b8f3ae257a Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Mon, 20 Oct 2025 20:02:00 +0200 Subject: [PATCH 1/8] Add ES DSL schema_versions implementation --- ee/lib/search/elastic/dsl/schema_versions.rb | 54 ++++++++++ .../elastic/dsl/schema_versions_spec.rb | 101 ++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 ee/lib/search/elastic/dsl/schema_versions.rb create mode 100644 ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb diff --git a/ee/lib/search/elastic/dsl/schema_versions.rb b/ee/lib/search/elastic/dsl/schema_versions.rb new file mode 100644 index 00000000000000..93412dbcf021db --- /dev/null +++ b/ee/lib/search/elastic/dsl/schema_versions.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Search + module Elastic + module Dsl + module SchemaVersions + extend ActiveSupport::Concern + + included do + # internal storage + class_attribute :_schema_versions, instance_accessor: false, default: [] + end + + class_methods do + # DSL entrypoint + def schema_versions(&block) + if block + begin + class_exec(&block) + rescue StandardError => e + ::Gitlab::ErrorTracking.track_exception(e, class: name) + end + end + + _schema_versions + end + + # register a version + def version(number, **opts) + _schema_versions << { version: number, condition: opts[:if] } + rescue StandardError => e + ::Gitlab::ErrorTracking.track_exception(e, class: name) + end + + # currently active version + def current_version + return if _schema_versions.blank? + + active = _schema_versions.filter_map do |entry| + entry[:version] if entry[:condition].nil? || entry[:condition].call + end + + active.max || _schema_versions.reverse.find { |v| v[:condition].nil? }&.dig(:version) + rescue StandardError => e + ::Gitlab::ErrorTracking.track_exception(e, class: name) + nil + end + + alias_method :schema_version, :current_version + end + end + end + end +end diff --git a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb new file mode 100644 index 00000000000000..35447ab5eeb9f6 --- /dev/null +++ b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Search::Elastic::Dsl::SchemaVersions, feature_category: :global_search do + let(:klass) do + Class.new do + include Search::Elastic::Dsl::SchemaVersions + end + end + + describe '.version' do + it 'registers a simple version' do + klass.schema_versions { version 1234 } + expect(klass.schema_versions).to include(version: 1234, condition: nil) + end + + it 'registers a version with a condition lambda' do + cond = -> { true } + klass.schema_versions { version 2000, if: cond } + expect(klass.schema_versions.last).to include(version: 2000, condition: cond) + end + + it 'logs errors if adding version fails' do + allow(Gitlab::ErrorTracking).to receive(:track_exception) + + def klass.version(_number, **_opts) + raise StandardError, 'boom' + rescue StandardError => e + ::Gitlab::ErrorTracking.track_exception(e, class: name) + end + + klass.version(3000) + + expect(Gitlab::ErrorTracking).to have_received(:track_exception).once + end + end + + describe '.current_version' do + it 'returns nil if no versions are defined' do + expect(klass.current_version).to be_nil + end + + it 'returns the latest version when all unconditional' do + klass.schema_versions do + version 1 + version 2 + end + expect(klass.current_version).to eq(2) + end + + it 'returns the highest active version when conditions pass' do + klass.schema_versions do + version 1, if: -> { false } + version 2, if: -> { true } + version 3, if: -> { true } + end + expect(klass.current_version).to eq(3) + end + + it 'falls back to the last unconditional version when none active' do + klass.schema_versions do + version 5, if: -> { false } + version 7, if: -> { false } + version 8 + end + expect(klass.current_version).to eq(8) + end + + it 'logs and returns nil on exception' do + allow(Gitlab::ErrorTracking).to receive(:track_exception) + klass.schema_versions { version 9, if: -> { raise 'fail' } } + + expect(klass.current_version).to be_nil + expect(Gitlab::ErrorTracking).to have_received(:track_exception).once + end + end + + describe '.schema_versions' do + it 'evaluates a block to define versions' do + klass.schema_versions do + version 100 + version 200 + end + + versions = klass.schema_versions.each_with_object([]) { |v, arr| arr << v[:version] } + expect(versions).to eq([100, 200]) + end + + it 'returns the versions list when called without a block' do + klass.schema_versions { version 11 } + expect(klass.schema_versions).to eq([{ version: 11, condition: nil }]) + end + + it 'logs error if block raises' do + allow(Gitlab::ErrorTracking).to receive(:track_exception) + expect { klass.schema_versions { raise 'error' } }.not_to raise_error + expect(Gitlab::ErrorTracking).to have_received(:track_exception).once + end + end +end -- GitLab From 04a18b6fc224479970d3f1d6bcef7a4856230acc Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Mon, 20 Oct 2025 20:24:01 +0200 Subject: [PATCH 2/8] Replace eq with match_array in specs --- ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb index 35447ab5eeb9f6..3ed381b041be65 100644 --- a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb +++ b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb @@ -84,12 +84,12 @@ def klass.version(_number, **_opts) end versions = klass.schema_versions.each_with_object([]) { |v, arr| arr << v[:version] } - expect(versions).to eq([100, 200]) + expect(versions).to match_array([100, 200]) end it 'returns the versions list when called without a block' do klass.schema_versions { version 11 } - expect(klass.schema_versions).to eq([{ version: 11, condition: nil }]) + expect(klass.schema_versions).to match_array([{ version: 11, condition: nil }]) end it 'logs error if block raises' do -- GitLab From c283b064a5bfc2738866c16cce98947934e3583a Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Mon, 20 Oct 2025 20:37:14 +0200 Subject: [PATCH 3/8] Add comments in SchemaVersions --- ee/lib/search/elastic/dsl/schema_versions.rb | 40 ++++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/ee/lib/search/elastic/dsl/schema_versions.rb b/ee/lib/search/elastic/dsl/schema_versions.rb index 93412dbcf021db..acc560ab841311 100644 --- a/ee/lib/search/elastic/dsl/schema_versions.rb +++ b/ee/lib/search/elastic/dsl/schema_versions.rb @@ -1,5 +1,16 @@ # frozen_string_literal: true +# -------------------------------------------------------------------------- +# Search::Elastic::Dsl::SchemaVersions +# -------------------------------------------------------------------------- +# Provides a DSL to declare schema versions for Elasticsearch models. +# Allows conditional versions that depend on migration or feature state. +# Example usage: +# schema_versions do +# version 2525 +# version 2526, if: -> { migration_has_finished?(:some_migration) } +# end +# -------------------------------------------------------------------------- module Search module Elastic module Dsl @@ -7,32 +18,51 @@ module SchemaVersions extend ActiveSupport::Concern included do - # internal storage + # Internal storage for schema version definitions + # Each entry: { version: , condition: } class_attribute :_schema_versions, instance_accessor: false, default: [] end class_methods do - # DSL entrypoint + # ------------------------------------------------------------------ + # DSL entrypoint: accepts a block to define multiple versions + # Example: + # schema_versions do + # version 2525 + # version 2526, if: -> { some_condition } + # end + # ------------------------------------------------------------------ def schema_versions(&block) if block begin + # Evaluate the block in the class context class_exec(&block) rescue StandardError => e + # Track errors to GitLab error tracking ::Gitlab::ErrorTracking.track_exception(e, class: name) end end + # Return the internal versions array _schema_versions end - # register a version + # ------------------------------------------------------------------ + # Register a single version + # number - version number (Integer) + # opts[:if] - optional Proc, only active if true + # ------------------------------------------------------------------ def version(number, **opts) _schema_versions << { version: number, condition: opts[:if] } rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) end - # currently active version + # ------------------------------------------------------------------ + # Return the currently active schema version + # Active versions are those without a condition or where condition.call == true + # Returns the maximum active version, or the last unconditional version + # ------------------------------------------------------------------ def current_version return if _schema_versions.blank? @@ -40,12 +70,14 @@ def current_version entry[:version] if entry[:condition].nil? || entry[:condition].call end + # Return highest active version, fallback to last unconditional version active.max || _schema_versions.reverse.find { |v| v[:condition].nil? }&.dig(:version) rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) nil end + # Alias for readability alias_method :schema_version, :current_version end end -- GitLab From 82e8d06578c829932a06f4ef104fdb50b32ccb8c Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Tue, 21 Oct 2025 20:23:09 +0200 Subject: [PATCH 4/8] Refine fallback logic for schema version selection --- ee/lib/search/elastic/dsl/schema_versions.rb | 5 ++++- ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/ee/lib/search/elastic/dsl/schema_versions.rb b/ee/lib/search/elastic/dsl/schema_versions.rb index acc560ab841311..5c531617475217 100644 --- a/ee/lib/search/elastic/dsl/schema_versions.rb +++ b/ee/lib/search/elastic/dsl/schema_versions.rb @@ -71,7 +71,10 @@ def current_version end # Return highest active version, fallback to last unconditional version - active.max || _schema_versions.reverse.find { |v| v[:condition].nil? }&.dig(:version) + return active.max if active.any? + + # Fallback: return the last unconditional version if no conditional versions are active + _schema_versions.reverse.find { |v| v[:condition].nil? }&.dig(:version) rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) nil diff --git a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb index 3ed381b041be65..6160c5a14bd2f1 100644 --- a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb +++ b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb @@ -67,6 +67,15 @@ def klass.version(_number, **_opts) expect(klass.current_version).to eq(8) end + it 'returns nil when all conditional versions evaluate to false and no unconditional versions exist' do + klass.schema_versions do + version 1, if: -> { false } + version 2, if: -> { false } + end + + expect(klass.current_version).to be_nil + end + it 'logs and returns nil on exception' do allow(Gitlab::ErrorTracking).to receive(:track_exception) klass.schema_versions { version 9, if: -> { raise 'fail' } } -- GitLab From 40f21674667b1e5dc7ab84c4bbef61dcb849f9b7 Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Wed, 22 Oct 2025 12:44:11 +0200 Subject: [PATCH 5/8] Fix comments in schema_versions --- ee/lib/search/elastic/dsl/schema_versions.rb | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/ee/lib/search/elastic/dsl/schema_versions.rb b/ee/lib/search/elastic/dsl/schema_versions.rb index 5c531617475217..477d15688c4809 100644 --- a/ee/lib/search/elastic/dsl/schema_versions.rb +++ b/ee/lib/search/elastic/dsl/schema_versions.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true -# -------------------------------------------------------------------------- # Search::Elastic::Dsl::SchemaVersions -# -------------------------------------------------------------------------- +# # Provides a DSL to declare schema versions for Elasticsearch models. # Allows conditional versions that depend on migration or feature state. # Example usage: @@ -10,7 +9,6 @@ # version 2525 # version 2526, if: -> { migration_has_finished?(:some_migration) } # end -# -------------------------------------------------------------------------- module Search module Elastic module Dsl @@ -24,45 +22,36 @@ module SchemaVersions end class_methods do - # ------------------------------------------------------------------ # DSL entrypoint: accepts a block to define multiple versions # Example: # schema_versions do # version 2525 # version 2526, if: -> { some_condition } # end - # ------------------------------------------------------------------ def schema_versions(&block) if block begin - # Evaluate the block in the class context class_exec(&block) rescue StandardError => e - # Track errors to GitLab error tracking ::Gitlab::ErrorTracking.track_exception(e, class: name) end end - # Return the internal versions array _schema_versions end - # ------------------------------------------------------------------ # Register a single version # number - version number (Integer) # opts[:if] - optional Proc, only active if true - # ------------------------------------------------------------------ def version(number, **opts) _schema_versions << { version: number, condition: opts[:if] } rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) end - # ------------------------------------------------------------------ # Return the currently active schema version # Active versions are those without a condition or where condition.call == true # Returns the maximum active version, or the last unconditional version - # ------------------------------------------------------------------ def current_version return if _schema_versions.blank? @@ -72,15 +61,12 @@ def current_version # Return highest active version, fallback to last unconditional version return active.max if active.any? - - # Fallback: return the last unconditional version if no conditional versions are active _schema_versions.reverse.find { |v| v[:condition].nil? }&.dig(:version) rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) nil end - # Alias for readability alias_method :schema_version, :current_version end end -- GitLab From 0a7d9752d68fab8fe90f067d348ceecdf65bc29e Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Wed, 22 Oct 2025 12:46:15 +0200 Subject: [PATCH 6/8] Fix specs by testing implicitly edge cases --- .../search/elastic/dsl/schema_versions_spec.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb index 6160c5a14bd2f1..2dacf3fd60e016 100644 --- a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb +++ b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb @@ -44,9 +44,10 @@ def klass.version(_number, **_opts) it 'returns the latest version when all unconditional' do klass.schema_versions do version 1 + version 3 version 2 end - expect(klass.current_version).to eq(2) + expect(klass.current_version).to eq(3) end it 'returns the highest active version when conditions pass' do @@ -62,9 +63,9 @@ def klass.version(_number, **_opts) klass.schema_versions do version 5, if: -> { false } version 7, if: -> { false } - version 8 + version 4 end - expect(klass.current_version).to eq(8) + expect(klass.current_version).to eq(4) end it 'returns nil when all conditional versions evaluate to false and no unconditional versions exist' do @@ -76,6 +77,15 @@ def klass.version(_number, **_opts) expect(klass.current_version).to be_nil end + it 'ignores unconditional versions if higher conditional versions are active' do + klass.schema_versions do + version 1 + version 2, if: -> { true } + end + + expect(klass.current_version).to eq(2) + end + it 'logs and returns nil on exception' do allow(Gitlab::ErrorTracking).to receive(:track_exception) klass.schema_versions { version 9, if: -> { raise 'fail' } } -- GitLab From 02244be40215e7047503d20a9c3f79446dc45316 Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Wed, 22 Oct 2025 13:06:12 +0200 Subject: [PATCH 7/8] Make version private and add remove it's specs --- ee/lib/search/elastic/dsl/models/base.rb | 21 ++++++++ .../elastic/dsl/models/vulnerability.rb | 22 ++++++++ ee/lib/search/elastic/dsl/schema_versions.rb | 38 ++++++++------ .../elastic/references/vulnerability.rb | 18 +------ .../elastic/dsl/models/vulnerability_spec.rb | 20 ++++++++ .../elastic/dsl/schema_versions_spec.rb | 40 +++++---------- .../dsl/shared/models_shared_examples.rb | 51 +++++++++++++++++++ 7 files changed, 150 insertions(+), 60 deletions(-) create mode 100644 ee/lib/search/elastic/dsl/models/base.rb create mode 100644 ee/lib/search/elastic/dsl/models/vulnerability.rb create mode 100644 ee/spec/lib/search/elastic/dsl/models/vulnerability_spec.rb create mode 100644 ee/spec/lib/search/elastic/dsl/shared/models_shared_examples.rb diff --git a/ee/lib/search/elastic/dsl/models/base.rb b/ee/lib/search/elastic/dsl/models/base.rb new file mode 100644 index 00000000000000..57e9c3146e27c2 --- /dev/null +++ b/ee/lib/search/elastic/dsl/models/base.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Search + module Elastic + module Dsl + module Models + class Base + include ::Search::Elastic::Dsl::SchemaVersions + + class << self + private + + def migration_has_finished?(name) + ::Elastic::DataMigrationService.migration_has_finished?(name) + end + end + end + end + end + end +end diff --git a/ee/lib/search/elastic/dsl/models/vulnerability.rb b/ee/lib/search/elastic/dsl/models/vulnerability.rb new file mode 100644 index 00000000000000..7fc8d2121ff817 --- /dev/null +++ b/ee/lib/search/elastic/dsl/models/vulnerability.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Search + module Elastic + module Dsl + module Models + class Vulnerability < Base + SCHEMA_VERSION = 25_43 + + schema_versions do + version 25_25 + version 25_26, if: -> { migration_has_finished?(:add_rechability_field_to_vulnerability) } + version 25_36, if: -> { migration_has_finished?(:add_resolved_at_dismissed_at_fields_to_vulnerability) } + version 25_37, if: -> { migration_has_finished?(:add_token_status_field_to_vulnerability) } + version 25_42, if: -> { migration_has_finished?(:add_policy_violations_field_to_vulnerability) } + version SCHEMA_VERSION, if: -> { migration_has_finished?(:add_risk_score_field_to_vulnerability) } + end + end + end + end + end +end diff --git a/ee/lib/search/elastic/dsl/schema_versions.rb b/ee/lib/search/elastic/dsl/schema_versions.rb index 477d15688c4809..0acd24d69296f0 100644 --- a/ee/lib/search/elastic/dsl/schema_versions.rb +++ b/ee/lib/search/elastic/dsl/schema_versions.rb @@ -19,6 +19,14 @@ module SchemaVersions # Internal storage for schema version definitions # Each entry: { version: , condition: } class_attribute :_schema_versions, instance_accessor: false, default: [] + + # Ensure subclasses get independent copies of schema versions + singleton_class.prepend(Module.new do + def inherited(subclass) + super + subclass._schema_versions = _schema_versions.dup + end + end) end class_methods do @@ -29,24 +37,12 @@ module SchemaVersions # version 2526, if: -> { some_condition } # end def schema_versions(&block) - if block - begin - class_exec(&block) - rescue StandardError => e - ::Gitlab::ErrorTracking.track_exception(e, class: name) - end - end - - _schema_versions - end + return _schema_versions unless block - # Register a single version - # number - version number (Integer) - # opts[:if] - optional Proc, only active if true - def version(number, **opts) - _schema_versions << { version: number, condition: opts[:if] } + class_exec(&block) rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) + _schema_versions end # Return the currently active schema version @@ -61,13 +57,23 @@ def current_version # Return highest active version, fallback to last unconditional version return active.max if active.any? + _schema_versions.reverse.find { |v| v[:condition].nil? }&.dig(:version) rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) nil end - alias_method :schema_version, :current_version + private + + # Register a single version + # number - version number (Integer) + # opts[:if] - optional Proc, only active if true + def version(number, **opts) + _schema_versions << { version: number, condition: opts[:if] } + rescue StandardError => e + ::Gitlab::ErrorTracking.track_exception(e, class: name) + end end end end diff --git a/ee/lib/search/elastic/references/vulnerability.rb b/ee/lib/search/elastic/references/vulnerability.rb index 3d426a210f0135..5cf0aee509a68d 100644 --- a/ee/lib/search/elastic/references/vulnerability.rb +++ b/ee/lib/search/elastic/references/vulnerability.rb @@ -7,7 +7,7 @@ class Vulnerability < Reference include Search::Elastic::Concerns::DatabaseReference include ::Gitlab::Utils::StrongMemoize - SCHEMA_VERSION = 25_43 + SCHEMA_VERSION = ::Search::Elastic::Dsl::Models::Vulnerability::SCHEMA_VERSION DOC_TYPE = 'vulnerability' INDEX_NAME = 'vulnerabilities' @@ -137,25 +137,11 @@ def fetch_record_attribute(record, attribute) def internal_es_fields { - schema_version: fetch_schema_version, + schema_version: ::Search::Elastic::Dsl::Models::Vulnerability.current_version, type: DOC_TYPE }.with_indifferent_access end - def fetch_schema_version - if risk_score_migration_completed? - SCHEMA_VERSION - elsif policy_violations_migration_finished? - 25_42 - elsif token_status_migration_finished? - 25_37 - elsif resolved_at_dismissed_at_migration_completed? - 25_36 - else - 25_26 - end - end - def token_status_migration_finished? ::Elastic::DataMigrationService.migration_has_finished?( :add_token_status_field_to_vulnerability diff --git a/ee/spec/lib/search/elastic/dsl/models/vulnerability_spec.rb b/ee/spec/lib/search/elastic/dsl/models/vulnerability_spec.rb new file mode 100644 index 00000000000000..70f8b30c200aad --- /dev/null +++ b/ee/spec/lib/search/elastic/dsl/models/vulnerability_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../shared/models_shared_examples' + +RSpec.describe Search::Elastic::Dsl::Models::Vulnerability, feature_category: :global_search do + it_behaves_like( + 'a model with schema version logic for migrations', + [ + [25_25, nil], + [25_26, :add_rechability_field_to_vulnerability], + [25_36, :add_resolved_at_dismissed_at_fields_to_vulnerability], + [25_37, :add_token_status_field_to_vulnerability], + [25_42, :add_policy_violations_field_to_vulnerability], + [25_43, :add_risk_score_field_to_vulnerability] + ], + current_version: 25_43, + fallback_version: 25_25 + ) +end diff --git a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb index 2dacf3fd60e016..41ade4216ca88f 100644 --- a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb +++ b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require_relative 'shared/models_shared_examples' RSpec.describe Search::Elastic::Dsl::SchemaVersions, feature_category: :global_search do let(:klass) do @@ -9,33 +10,6 @@ end end - describe '.version' do - it 'registers a simple version' do - klass.schema_versions { version 1234 } - expect(klass.schema_versions).to include(version: 1234, condition: nil) - end - - it 'registers a version with a condition lambda' do - cond = -> { true } - klass.schema_versions { version 2000, if: cond } - expect(klass.schema_versions.last).to include(version: 2000, condition: cond) - end - - it 'logs errors if adding version fails' do - allow(Gitlab::ErrorTracking).to receive(:track_exception) - - def klass.version(_number, **_opts) - raise StandardError, 'boom' - rescue StandardError => e - ::Gitlab::ErrorTracking.track_exception(e, class: name) - end - - klass.version(3000) - - expect(Gitlab::ErrorTracking).to have_received(:track_exception).once - end - end - describe '.current_version' do it 'returns nil if no versions are defined' do expect(klass.current_version).to be_nil @@ -88,7 +62,7 @@ def klass.version(_number, **_opts) it 'logs and returns nil on exception' do allow(Gitlab::ErrorTracking).to receive(:track_exception) - klass.schema_versions { version 9, if: -> { raise 'fail' } } + klass.schema_versions { version 9, if: -> { raise 'error' } } expect(klass.current_version).to be_nil expect(Gitlab::ErrorTracking).to have_received(:track_exception).once @@ -116,5 +90,15 @@ def klass.version(_number, **_opts) expect { klass.schema_versions { raise 'error' } }.not_to raise_error expect(Gitlab::ErrorTracking).to have_received(:track_exception).once end + + it 'returns existing versions if an error occurs in the block' do + allow(Gitlab::ErrorTracking).to receive(:track_exception) + + klass.schema_versions { version 1 } + result = klass.schema_versions { raise 'error' } + + expect(result).to match_array([{ version: 1, condition: nil }]) + expect(Gitlab::ErrorTracking).to have_received(:track_exception).once + end end end diff --git a/ee/spec/lib/search/elastic/dsl/shared/models_shared_examples.rb b/ee/spec/lib/search/elastic/dsl/shared/models_shared_examples.rb new file mode 100644 index 00000000000000..7374c176ad29fa --- /dev/null +++ b/ee/spec/lib/search/elastic/dsl/shared/models_shared_examples.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a model with schema version logic for migrations' do |expected_versions, + current_version:, fallback_version:| + describe '.current_version' do + subject(:version) { described_class.current_version } + + where(:expected_version, :migration) do + expected_versions + end + + with_them do + before do + allow(::Elastic::DataMigrationService) + .to receive(:migration_has_finished?).and_return(false) + + if migration + allow(::Elastic::DataMigrationService) + .to receive(:migration_has_finished?) + .with(migration) + .and_return(true) + end + end + + it( + "returns schema version #{params[:expected_version]} " \ + "when #{params[:migration] || 'no migration'} is finished" + ) do + expect(version).to eq(expected_version) + end + end + + context 'when no migrations are finished' do + it 'returns the fallback version' do + allow(::Elastic::DataMigrationService) + .to receive(:migration_has_finished?).and_return(false) + + expect(described_class.current_version).to eq(fallback_version) + end + end + + context 'when all migrations are finished' do + it 'returns the current (latest) schema version' do + allow(::Elastic::DataMigrationService) + .to receive(:migration_has_finished?).and_return(true) + + expect(described_class.current_version).to eq(current_version) + end + end + end +end -- GitLab From d0a5b2a0151ecd6a7fcf5d0d81ed49e27c0a3f5c Mon Sep 17 00:00:00 2001 From: Ugo Nnanna Okeadu Date: Thu, 23 Oct 2025 12:56:24 +0200 Subject: [PATCH 8/8] Let errors bubble up --- ee/lib/search/elastic/dsl/models/base.rb | 2 +- .../elastic/dsl/models/vulnerability.rb | 10 +++---- ee/lib/search/elastic/dsl/schema_versions.rb | 6 ++--- .../elastic/dsl/schema_versions_spec.rb | 27 ++++++++++++------- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/ee/lib/search/elastic/dsl/models/base.rb b/ee/lib/search/elastic/dsl/models/base.rb index 57e9c3146e27c2..79cedc9e73875d 100644 --- a/ee/lib/search/elastic/dsl/models/base.rb +++ b/ee/lib/search/elastic/dsl/models/base.rb @@ -10,7 +10,7 @@ class Base class << self private - def migration_has_finished?(name) + def mapped?(name) ::Elastic::DataMigrationService.migration_has_finished?(name) end end diff --git a/ee/lib/search/elastic/dsl/models/vulnerability.rb b/ee/lib/search/elastic/dsl/models/vulnerability.rb index 7fc8d2121ff817..951449ea327e63 100644 --- a/ee/lib/search/elastic/dsl/models/vulnerability.rb +++ b/ee/lib/search/elastic/dsl/models/vulnerability.rb @@ -9,11 +9,11 @@ class Vulnerability < Base schema_versions do version 25_25 - version 25_26, if: -> { migration_has_finished?(:add_rechability_field_to_vulnerability) } - version 25_36, if: -> { migration_has_finished?(:add_resolved_at_dismissed_at_fields_to_vulnerability) } - version 25_37, if: -> { migration_has_finished?(:add_token_status_field_to_vulnerability) } - version 25_42, if: -> { migration_has_finished?(:add_policy_violations_field_to_vulnerability) } - version SCHEMA_VERSION, if: -> { migration_has_finished?(:add_risk_score_field_to_vulnerability) } + version 25_26, if: -> { mapped?(:add_rechability_field_to_vulnerability) } + version 25_36, if: -> { mapped?(:add_resolved_at_dismissed_at_fields_to_vulnerability) } + version 25_37, if: -> { mapped?(:add_token_status_field_to_vulnerability) } + version 25_42, if: -> { mapped?(:add_policy_violations_field_to_vulnerability) } + version SCHEMA_VERSION, if: -> { mapped?(:add_risk_score_field_to_vulnerability) } end end end diff --git a/ee/lib/search/elastic/dsl/schema_versions.rb b/ee/lib/search/elastic/dsl/schema_versions.rb index 0acd24d69296f0..5272ebc29e3cbc 100644 --- a/ee/lib/search/elastic/dsl/schema_versions.rb +++ b/ee/lib/search/elastic/dsl/schema_versions.rb @@ -42,7 +42,7 @@ def schema_versions(&block) class_exec(&block) rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) - _schema_versions + raise e end # Return the currently active schema version @@ -61,7 +61,7 @@ def current_version _schema_versions.reverse.find { |v| v[:condition].nil? }&.dig(:version) rescue StandardError => e ::Gitlab::ErrorTracking.track_exception(e, class: name) - nil + raise e end private @@ -71,8 +71,6 @@ def current_version # opts[:if] - optional Proc, only active if true def version(number, **opts) _schema_versions << { version: number, condition: opts[:if] } - rescue StandardError => e - ::Gitlab::ErrorTracking.track_exception(e, class: name) end end end diff --git a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb index 41ade4216ca88f..1a1a059e7555f5 100644 --- a/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb +++ b/ee/spec/lib/search/elastic/dsl/schema_versions_spec.rb @@ -21,6 +21,7 @@ version 3 version 2 end + expect(klass.current_version).to eq(3) end @@ -30,6 +31,7 @@ version 2, if: -> { true } version 3, if: -> { true } end + expect(klass.current_version).to eq(3) end @@ -39,6 +41,7 @@ version 7, if: -> { false } version 4 end + expect(klass.current_version).to eq(4) end @@ -60,11 +63,11 @@ expect(klass.current_version).to eq(2) end - it 'logs and returns nil on exception' do + it 'logs and raises on exception' do allow(Gitlab::ErrorTracking).to receive(:track_exception) klass.schema_versions { version 9, if: -> { raise 'error' } } - expect(klass.current_version).to be_nil + expect { klass.current_version }.to raise_error(RuntimeError, 'error') expect(Gitlab::ErrorTracking).to have_received(:track_exception).once end end @@ -76,28 +79,34 @@ version 200 end - versions = klass.schema_versions.each_with_object([]) { |v, arr| arr << v[:version] } - expect(versions).to match_array([100, 200]) + expect(klass.schema_versions).to eq( + [ + { version: 100, condition: nil }, + { version: 200, condition: nil } + ] + ) end it 'returns the versions list when called without a block' do klass.schema_versions { version 11 } + expect(klass.schema_versions).to match_array([{ version: 11, condition: nil }]) end - it 'logs error if block raises' do + it 'logs and raises error if block raises' do allow(Gitlab::ErrorTracking).to receive(:track_exception) - expect { klass.schema_versions { raise 'error' } }.not_to raise_error + + expect { klass.schema_versions { raise 'error' } }.to raise_error(RuntimeError, 'error') expect(Gitlab::ErrorTracking).to have_received(:track_exception).once end - it 'returns existing versions if an error occurs in the block' do + it 'logs and raises but preserves previously defined versions' do allow(Gitlab::ErrorTracking).to receive(:track_exception) klass.schema_versions { version 1 } - result = klass.schema_versions { raise 'error' } - expect(result).to match_array([{ version: 1, condition: nil }]) + expect { klass.schema_versions { raise 'error' } }.to raise_error(RuntimeError, 'error') + expect(klass.schema_versions).to match_array([{ version: 1, condition: nil }]) expect(Gitlab::ErrorTracking).to have_received(:track_exception).once end end -- GitLab