diff --git a/app/models/import/source_user_placeholder_reference.rb b/app/models/import/source_user_placeholder_reference.rb index ab27dc7ff5e55ba4e0afaad648c7e9fd437b38e8..9338d47e680e8c20f00725e9381fed9be4971fbf 100644 --- a/app/models/import/source_user_placeholder_reference.rb +++ b/app/models/import/source_user_placeholder_reference.rb @@ -42,16 +42,16 @@ class SourceUserPlaceholderReference < ApplicationRecord SerializationError = Class.new(StandardError) def aliased_model - Import::PlaceholderReferenceAliasResolver.aliased_model(model) + PlaceholderReferences::AliasResolver.aliased_model(model) end def aliased_user_reference_column - Import::PlaceholderReferenceAliasResolver.aliased_column(model, user_reference_column) + PlaceholderReferences::AliasResolver.aliased_column(model, user_reference_column) end def aliased_composite_key composite_key.transform_keys do |key| - Import::PlaceholderReferenceAliasResolver.aliased_column(model, key) + PlaceholderReferences::AliasResolver.aliased_column(model, key) end end diff --git a/app/services/import/placeholder_references/base_service.rb b/app/services/import/placeholder_references/base_service.rb index 2aa083d796a1eb65fa60d3abd34480dbadbd3321..681298f1d99f890f23ee1404305afc52db568ad3 100644 --- a/app/services/import/placeholder_references/base_service.rb +++ b/app/services/import/placeholder_references/base_service.rb @@ -12,14 +12,10 @@ def initialize(import_source:, import_uid:) private - attr_reader :import_source, :import_uid, :reference + attr_reader :import_source, :import_uid - def cache - Gitlab::Cache::Import::Caching - end - - def cache_key - @cache_key ||= [:'placeholder-reference', import_source, import_uid].join(':') + def store + @store ||= PlaceholderReferences::Store.new(import_source: import_source, import_uid: import_uid) end def logger diff --git a/app/services/import/placeholder_references/load_service.rb b/app/services/import/placeholder_references/load_service.rb index c18a155a62216eaa06ef820be8794e99060b306d..6a382c033e10cc2eefc69ffa99a05218e954b888 100644 --- a/app/services/import/placeholder_references/load_service.rb +++ b/app/services/import/placeholder_references/load_service.rb @@ -37,7 +37,7 @@ def execute attr_accessor :error_count, :processed_count def next_batch - cache.limited_values_from_set(cache_key, limit: BATCH_LIMIT) + store.get(BATCH_LIMIT) end def load!(batch) @@ -79,7 +79,7 @@ def clear_batch!(batch) self.processed_count += processed_count - cache.set_remove(cache_key, batch) + store.remove(batch) end def log_error(item, exception) diff --git a/app/services/import/placeholder_references/push_service.rb b/app/services/import/placeholder_references/push_service.rb index 468d74c52a996bf319bb4be7e0b5469d74db976c..3d0de3e2c56856b2d85bc642c7f8af76ba2a2c94 100644 --- a/app/services/import/placeholder_references/push_service.rb +++ b/app/services/import/placeholder_references/push_service.rb @@ -48,7 +48,7 @@ def execute serialized_reference = reference.to_serialized - cache.set_add(cache_key, serialized_reference, timeout: cache_ttl) + store.add(serialized_reference) success(serialized_reference: serialized_reference) end @@ -57,10 +57,6 @@ def execute attr_reader :reference - def cache_ttl - Gitlab::Cache::Import::Caching::TIMEOUT - end - def track_error(reference) Gitlab::ErrorTracking.track_and_raise_for_dev_exception( InvalidReferenceError.new('Invalid placeholder user reference'), diff --git a/lib/gitlab/cache/import/caching.rb b/lib/gitlab/cache/import/caching.rb index 7692af6875f19c6ea8a211243ad20a090bcdeac2..c091d9909428e8ceb7d649ee4eac606afe2dacb3 100644 --- a/lib/gitlab/cache/import/caching.rb +++ b/lib/gitlab/cache/import/caching.rb @@ -143,6 +143,17 @@ def self.set_includes?(raw_key, value) end end + # Returns the number of values in the set. + # + # raw_key - The key of the set to check. + def self.set_count(raw_key) + key = cache_key_for(raw_key) + + with_redis do |redis| + redis.scard(key) + end + end + # Returns the values of the given set. # # raw_key - The key of the set to check. diff --git a/lib/import/placeholder_reference_alias_resolver.rb b/lib/import/placeholder_reference_alias_resolver.rb deleted file mode 100644 index fb5715826c7a17359a404536cd192afca9ad6254..0000000000000000000000000000000000000000 --- a/lib/import/placeholder_reference_alias_resolver.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module Import - module PlaceholderReferenceAliasResolver - MissingAlias = Class.new(StandardError) - - ALIASES = { - "Note" => { - model: Note, - columns: { - "author_id" => "author_id" - } - } - }.freeze - - def self.aliased_model(model) - aliased_model = ALIASES.dig(model, :model) - return aliased_model if aliased_model.present? - - message = "ALIASES must be extended to include #{model}" - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(MissingAlias.new(message)) - - model.safe_constantize - end - - def self.aliased_column(model, column) - aliased_column = ALIASES.dig(model, :columns, column) - return aliased_column if aliased_column.present? - - message = "ALIASES must be extended to include #{model} #{column}" - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(MissingAlias.new(message)) - - column - end - end -end diff --git a/lib/import/placeholder_references/alias_resolver.rb b/lib/import/placeholder_references/alias_resolver.rb new file mode 100644 index 0000000000000000000000000000000000000000..18a8539fca248e1c3f8b1ad8be757625430bc498 --- /dev/null +++ b/lib/import/placeholder_references/alias_resolver.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Import + module PlaceholderReferences + module AliasResolver + MissingAlias = Class.new(StandardError) + + ALIASES = { + "Note" => { + model: Note, + columns: { + "author_id" => "author_id" + } + } + }.freeze + + def self.aliased_model(model) + aliased_model = ALIASES.dig(model, :model) + return aliased_model if aliased_model.present? + + message = "ALIASES must be extended to include #{model}" + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(MissingAlias.new(message)) + + model.safe_constantize + end + + def self.aliased_column(model, column) + aliased_column = ALIASES.dig(model, :columns, column) + return aliased_column if aliased_column.present? + + message = "ALIASES must be extended to include #{model} #{column}" + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(MissingAlias.new(message)) + + column + end + end + end +end diff --git a/lib/import/placeholder_references/store.rb b/lib/import/placeholder_references/store.rb new file mode 100644 index 0000000000000000000000000000000000000000..b3d07c736b44e81dd2b2985ab3004daeccc850e7 --- /dev/null +++ b/lib/import/placeholder_references/store.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Import + module PlaceholderReferences + class Store + CACHE_TTL = 1.day + + def initialize(import_source:, import_uid:) + @import_source = import_source + @import_uid = import_uid + end + + def add(serialized_reference) + cache.set_add(cache_key, serialized_reference, timeout: CACHE_TTL) + end + + def get(limit = 1) + cache.limited_values_from_set(cache_key, limit: limit) + end + + def remove(values) + cache.set_remove(cache_key, values) + end + + def count + cache.set_count(cache_key) + end + + def empty? + count == 0 + end + + def any? + !empty? + end + + private + + attr_reader :import_source, :import_uid + + def cache + Gitlab::Cache::Import::Caching + end + + def cache_key + [:'placeholder-references', import_source, import_uid].join(':') + end + end + end +end diff --git a/spec/lib/gitlab/cache/import/caching_spec.rb b/spec/lib/gitlab/cache/import/caching_spec.rb index c84721eb7cb70aa6f863fc8250f44632254dfe42..6c813450a645ca3c831ca4204d50f5eaf4f716b0 100644 --- a/spec/lib/gitlab/cache/import/caching_spec.rb +++ b/spec/lib/gitlab/cache/import/caching_spec.rb @@ -193,6 +193,19 @@ end end + describe '.set_count' do + it 'returns 0 when the set does not exist' do + expect(described_class.set_count('foo')).to eq(0) + end + + it 'returns count of set' do + described_class.set_add('foo', 10) + described_class.set_add('foo', 20) + + expect(described_class.set_count('foo')).to eq(2) + end + end + describe '.hash_add' do it 'adds a value to a hash' do described_class.hash_add('foo', 1, 1) diff --git a/spec/lib/import/placeholder_reference_alias_resolver_spec.rb b/spec/lib/import/placeholder_references/alias_resolver_spec.rb similarity index 97% rename from spec/lib/import/placeholder_reference_alias_resolver_spec.rb rename to spec/lib/import/placeholder_references/alias_resolver_spec.rb index 1126cd657c315d64903b6527f0d22e8b123c0b32..6ed3383654b1e597b8bebb06188b2d07d663540e 100644 --- a/spec/lib/import/placeholder_reference_alias_resolver_spec.rb +++ b/spec/lib/import/placeholder_references/alias_resolver_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -RSpec.describe Import::PlaceholderReferenceAliasResolver, feature_category: :importers do +RSpec.describe Import::PlaceholderReferences::AliasResolver, feature_category: :importers do describe "ALIASES" do it "points to real columns" do def failure_message(model, column) diff --git a/spec/lib/import/placeholder_references/store_spec.rb b/spec/lib/import/placeholder_references/store_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5fed3c2dbcae6edb0f70ff9d988f049ab3f64804 --- /dev/null +++ b/spec/lib/import/placeholder_references/store_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Import::PlaceholderReferences::Store, :clean_gitlab_redis_shared_state, feature_category: :importers do + subject(:store) { described_class.new(import_source: 'source', import_uid: 'uid') } + + let(:cache_key) { [:'placeholder-references', 'source', 'uid'].join(':') } + + describe '#add' do + it 'adds to the set' do + store.add('foo') + + expect(cache.values_from_set(cache_key)).to eq(['foo']) + end + end + + describe '#get' do + before do + cache.set_add(cache_key, 'foo') + cache.set_add(cache_key, 'bar') + cache.set_add(cache_key, 'baz') + end + + it 'returns a member in the set' do + expect(store.get.size).to eq(1) + expect(store.get.first).to be_in(%w[foo bar baz]) + end + + it 'accepts an argument to return more members' do + expect(store.get(2).size).to eq(2) + end + end + + describe '#remove' do + it 'removes members from the set' do + cache.set_add(cache_key, 'foo') + cache.set_add(cache_key, 'bar') + cache.set_add(cache_key, 'baz') + + store.remove(%w[foo baz]) + + expect(cache.values_from_set(cache_key)).to eq(['bar']) + end + end + + describe '#count' do + it 'returns the count of members in the set' do + cache.set_add(cache_key, 'foo') + cache.set_add(cache_key, 'bar') + + expect(store.count).to eq(2) + end + end + + describe '#empty?' do + it 'returns true if the set is empty' do + expect(store.empty?).to eq(true) + end + + it 'returns false if the set is not empty' do + cache.set_add(cache_key, 'foo') + + expect(store.empty?).to eq(false) + end + end + + describe '#any?' do + it 'returns the inverse of #empty?' do + expect(store).to receive(:empty?).and_return(true) + expect(store.any?).to eq(false) + end + end + + def cache + Gitlab::Cache::Import::Caching + end +end diff --git a/spec/models/import/source_user_placeholder_reference_spec.rb b/spec/models/import/source_user_placeholder_reference_spec.rb index f4f000eef479acdb26fe42b55f44548da6254689..a23e4d1a75b2e04c4ab16022d4a9a524dd691696 100644 --- a/spec/models/import/source_user_placeholder_reference_spec.rb +++ b/spec/models/import/source_user_placeholder_reference_spec.rb @@ -136,7 +136,7 @@ def validation_errors(...) end before do - allow(Import::PlaceholderReferenceAliasResolver).to receive(:aliased_model).and_return(Note) + allow(Import::PlaceholderReferences::AliasResolver).to receive(:aliased_model).and_return(Note) end it "uses the new model" do @@ -158,7 +158,7 @@ def validation_errors(...) context "when the column name has changed" do before do - allow(Import::PlaceholderReferenceAliasResolver).to receive(:aliased_column).and_return("user_id") + allow(Import::PlaceholderReferences::AliasResolver).to receive(:aliased_column).and_return("user_id") end it "uses the new column" do @@ -177,8 +177,8 @@ def validation_errors(...) end before do - allow(Import::PlaceholderReferenceAliasResolver).to receive(:aliased_column).and_call_original - allow(Import::PlaceholderReferenceAliasResolver).to receive(:aliased_column) + allow(Import::PlaceholderReferences::AliasResolver).to receive(:aliased_column).and_call_original + allow(Import::PlaceholderReferences::AliasResolver).to receive(:aliased_column) .with("Note", "old_id").and_return("new_id") end diff --git a/spec/services/import/placeholder_references/load_service_spec.rb b/spec/services/import/placeholder_references/load_service_spec.rb index b12646aa4aebf7d0809a3e83ea9be12e35907aef..bfe57554711c5d1799fcb3c59262c197533fcc13 100644 --- a/spec/services/import/placeholder_references/load_service_spec.rb +++ b/spec/services/import/placeholder_references/load_service_spec.rb @@ -120,7 +120,7 @@ push(valid_reference_2) # We can't use `#push` because that validates, # so push directly to the set. - Gitlab::Cache::Import::Caching.set_add(cache_key, invalid_reference.values.to_json) + store.add(invalid_reference.values.to_json) push(valid_reference_3) end @@ -255,7 +255,8 @@ def push(reference) serialized = Import::SourceUserPlaceholderReference.new(reference).to_serialized - Gitlab::Cache::Import::Caching.set_add(cache_key, serialized) + + store.add(serialized) end def expect_log_message(type, **params) @@ -265,16 +266,14 @@ def expect_log_message(type, **params) end def set_members_count - Gitlab::Cache::Import::Caching.with_redis do |redis| - redis.scard(Gitlab::Cache::Import::Caching.cache_key_for(cache_key)) - end + store.count end - def cache_key - Import::PlaceholderReferences::BaseService.new( + def store + Import::PlaceholderReferences::Store.new( import_source: import_source, import_uid: import_uid - ).send(:cache_key) + ) end end end diff --git a/spec/services/import/placeholder_references/push_service_spec.rb b/spec/services/import/placeholder_references/push_service_spec.rb index b9ddb2acd56a1c2e938c4225f210103072d77415..89e3d66a9745b6a279d11d76149727cbd464a4f5 100644 --- a/spec/services/import/placeholder_references/push_service_spec.rb +++ b/spec/services/import/placeholder_references/push_service_spec.rb @@ -151,13 +151,9 @@ end def set - Gitlab::Cache::Import::Caching.values_from_set(cache_key) - end - - def cache_key - Import::PlaceholderReferences::BaseService.new( + Import::PlaceholderReferences::Store.new( import_source: import_source, import_uid: import_uid - ).send(:cache_key) + ).get end end