From 298d8e77c99ca53729fc82fb417fcab7318064bb Mon Sep 17 00:00:00 2001 From: Marc Saleiko <msaleiko@gitlab.com> Date: Wed, 19 Feb 2025 06:14:59 +0000 Subject: [PATCH] Adds fixed items model/assoc to activerecord-gitlab gem Model concern allows classes with fixed items to have active record like methods (all, where, find_by, find). The has one association can be used on models to establish an association to a fixed items model. --- gems/activerecord-gitlab/README.md | 4 +- .../lib/active_record/fixed_items_model.rb | 4 + .../fixed_items_model/has_one.rb | 82 +++++++++ .../active_record/fixed_items_model/model.rb | 96 ++++++++++ .../lib/activerecord-gitlab.rb | 2 + .../fixed_items_model/has_one_spec.rb | 157 ++++++++++++++++ .../fixed_items_model/model_spec.rb | 167 ++++++++++++++++++ gems/activerecord-gitlab/spec/spec_helper.rb | 1 + 8 files changed, 511 insertions(+), 2 deletions(-) create mode 100644 gems/activerecord-gitlab/lib/active_record/fixed_items_model.rb create mode 100644 gems/activerecord-gitlab/lib/active_record/fixed_items_model/has_one.rb create mode 100644 gems/activerecord-gitlab/lib/active_record/fixed_items_model/model.rb create mode 100644 gems/activerecord-gitlab/spec/active_record/fixed_items_model/has_one_spec.rb create mode 100644 gems/activerecord-gitlab/spec/active_record/fixed_items_model/model_spec.rb diff --git a/gems/activerecord-gitlab/README.md b/gems/activerecord-gitlab/README.md index c5b56e367b996..021477bf15477 100644 --- a/gems/activerecord-gitlab/README.md +++ b/gems/activerecord-gitlab/README.md @@ -1,4 +1,4 @@ -# ActiveRecord::GitlabPatches +# activerecord-gitlab -This gem adds GitLab specific Active Record patches. +This gem adds GitLab specific Active Record patches and a fixed items model concern and association. We have patches as a separate gem to isolate complexity. diff --git a/gems/activerecord-gitlab/lib/active_record/fixed_items_model.rb b/gems/activerecord-gitlab/lib/active_record/fixed_items_model.rb new file mode 100644 index 0000000000000..f95ff85a7b71c --- /dev/null +++ b/gems/activerecord-gitlab/lib/active_record/fixed_items_model.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +require_relative "fixed_items_model/model" +require_relative "fixed_items_model/has_one" diff --git a/gems/activerecord-gitlab/lib/active_record/fixed_items_model/has_one.rb b/gems/activerecord-gitlab/lib/active_record/fixed_items_model/has_one.rb new file mode 100644 index 0000000000000..32484ea239dd7 --- /dev/null +++ b/gems/activerecord-gitlab/lib/active_record/fixed_items_model/has_one.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module ActiveRecord + module FixedItemsModel + # Realizes a has one association with a fixed items model. + # + # See ActiveRecord::FixedItemsModel::Model for reference + # for such a fixed items model. + # + # A minimal example is: + # + # class MyModel < ApplicationRecord + # include ActiveRecord::FixedItemsModel::HasOne + # + # belongs_to_fixed_items :static_model, fixed_items_class: StaticModel + # end + # + # The attribute `static_model_id` must exist for the model. + # + # Usage: + # + # m = MyModel.last + # m.static_model # Returns fixed items model instance + # m.static_model = StaticModel.find(1) + # m.static_model_id = 1 # still possible + # m.static_model? # Bool + # + module HasOne + extend ActiveSupport::Concern + + class_methods do + def belongs_to_fixed_items(association_name, fixed_items_class:, foreign_key: nil) + foreign_key ||= "#{association_name}_id" + + raise "Missing attribute #{foreign_key}" unless attribute_names.include?(foreign_key) + + # Getter method + define_method(association_name) do + current_id = read_attribute(foreign_key) + return if current_id.nil? + + @cached_static_associations ||= {} + + cached_association = @cached_static_associations[association_name] + + # Invalidate cache if the foreign key has changed + if cached_association && cached_association.id != current_id + @cached_static_associations.delete(association_name) + end + + @cached_static_associations[association_name] ||= fixed_items_class.find(current_id) + end + + # Setter method + define_method(:"#{association_name}=") do |static_object| + @cached_static_associations&.delete(association_name) + write_attribute(foreign_key, static_object&.id) + end + + # Query method + define_method(:"#{association_name}?") do + attribute_present?(foreign_key) + end + + # Clear cache on reset + if method_defined?(:reset) + after_reset = instance_method(:reset) + define_method(:reset) do |*args| + @cached_static_associations = nil + after_reset.bind_call(self, *args) + end + else + define_method(:reset) do + @cached_static_associations = nil + self + end + end + end + end + end + end +end diff --git a/gems/activerecord-gitlab/lib/active_record/fixed_items_model/model.rb b/gems/activerecord-gitlab/lib/active_record/fixed_items_model/model.rb new file mode 100644 index 0000000000000..1627c4082e1f9 --- /dev/null +++ b/gems/activerecord-gitlab/lib/active_record/fixed_items_model/model.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +module ActiveRecord + module FixedItemsModel + # Includes handy AR-like methods to models that have a fixed set + # of items that are stored in code instead of the database. + # + # See ActiveRecord::FixedItemsModel::HasOne for reference + # on how to build associations. + # + # A minimal example of such a model is: + # + # class StaticModel + # include ActiveModel::Model + # include ActiveModel::Attributes + # include ActiveRecord::FixedItemsModel::Model + # + # ITEMS = [ + # { + # id: 1, + # name: 'To do' + # } + # ] + # + # attribute :id, :integer + # attribute :name, :string + # end + # + # Usage: + # + # StaticModel.find(1) + # StaticModel.where(name: 'To do') + # StaticModel.find_by(name: 'To do') + # StaticModel.all + # + module Model + extend ActiveSupport::Concern + + class_methods do + # Caches created instances for fast retrieval used in associations. + def find(id) + find_instances[id] ||= self::ITEMS.find { |item| item[:id] == id }&.then do |item_data| + new(item_data) + end + end + + def all + self::ITEMS.map { |data| new(data) } + end + + def where(**conditions) + all.select { |item| item.matches?(conditions) } + end + + def find_by(**conditions) + all.find { |item| item.matches?(conditions) } + end + + private + + def find_instances + @find_instances ||= [] + end + end + + included do + def matches?(conditions) + conditions.all? do |attribute, value| + if value.is_a?(Array) + value.include?(read_attribute(attribute)) + else + read_attribute(attribute) == value + end + end + end + + def has_attribute?(key) + attribute_names.include?(key.to_s) + end + + def read_attribute(key) + return nil unless has_attribute?(key) + + # Passed attributes are actual attributes of the model + # rubocop:disable GitlabSecurity/PublicSend -- Reason above + public_send(key) + # rubocop:enable GitlabSecurity/PublicSend + end + + def inspect + "#<#{self.class} #{attributes.map { |k, v| "#{k}: #{v.inspect}" }.join(', ')}>" + end + end + end + end +end diff --git a/gems/activerecord-gitlab/lib/activerecord-gitlab.rb b/gems/activerecord-gitlab/lib/activerecord-gitlab.rb index 31cf29c70a157..bedc5a8a61ea9 100644 --- a/gems/activerecord-gitlab/lib/activerecord-gitlab.rb +++ b/gems/activerecord-gitlab/lib/activerecord-gitlab.rb @@ -2,4 +2,6 @@ # frozen_string_literal: true require_relative "active_record/gitlab_patches" +require_relative "active_record/fixed_items_model" + # rubocop:enable Naming/FileName diff --git a/gems/activerecord-gitlab/spec/active_record/fixed_items_model/has_one_spec.rb b/gems/activerecord-gitlab/spec/active_record/fixed_items_model/has_one_spec.rb new file mode 100644 index 0000000000000..adcb7190c425f --- /dev/null +++ b/gems/activerecord-gitlab/spec/active_record/fixed_items_model/has_one_spec.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActiveRecord::FixedItemsModel::HasOne, feature_category: :shared do + before do + stub_const('TestStaticModel', Class.new do + include ActiveModel::Model + include ActiveModel::Attributes + include ActiveRecord::FixedItemsModel::Model + + attribute :id, :integer + attribute :name, :string + end) + + stub_const('TestStaticModel::ITEMS', [ + { id: 1, name: 'Item 1' }, + { id: 2, name: 'Item 2' }, + { id: 3, name: 'Item 3' } + ].freeze) + + stub_const('TestRecord', Class.new do + include ActiveModel::Attributes + include ActiveRecord::FixedItemsModel::HasOne + + attribute :static_item_id, :integer + + # Mock AR methods + def read_attribute(attr_name) + send(attr_name) + end + + def write_attribute(attr_name, value) + send("#{attr_name}=", value) + end + + def attribute_present?(attr_name) + send(attr_name).present? + end + + # Mock reset method + def reset + @static_item = nil + self + end + + belongs_to_fixed_items :static_item, fixed_items_class: TestStaticModel + end) + end + + subject(:record) { TestRecord.new } + + describe '#belongs_to_fixed_items' do + it { is_expected.to respond_to(:static_item) } + it { is_expected.to respond_to(:static_item=) } + it { is_expected.to respond_to(:static_item?) } + + context 'when foreign key attribute does not exist' do + it 'raises runtime error' do + expect do + stub_const('TestRecord', Class.new do + include ActiveModel::Attributes + include ActiveRecord::FixedItemsModel::HasOne + + attribute :static_item_id, :integer + + # No need for mock methods because they're not called + # because of the guard raise + + belongs_to_fixed_items :doesnt_exist, fixed_items_class: TestStaticModel + end) + end.to raise_error(RuntimeError, "Missing attribute doesnt_exist_id") + end + end + end + + describe 'getter method' do + it 'returns nil when foreign key is nil' do + expect(record.static_item).to be_nil + end + + it 'returns the correct static item when foreign key is set' do + record.static_item_id = 2 + expect(record.static_item.name).to eq('Item 2') + # Ensure cache is invalidated when id is changed + record.static_item_id = 3 + expect(record.static_item.name).to eq('Item 3') + end + + it 'caches the result' do + record.static_item_id = 1 + expect(TestStaticModel).to receive(:find).once.and_call_original + 2.times { record.static_item } + end + end + + describe 'setter method' do + it 'sets the foreign key when assigning a static item' do + static_item = TestStaticModel.find(3) + record.static_item = static_item + expect(record.static_item_id).to eq(3) + end + + it 'sets the foreign key to nil when assigning nil' do + record.static_item_id = 1 + record.static_item = nil + expect(record.static_item_id).to be_nil + end + + it 'clears the cache when setting a new value' do + record.static_item_id = 1 + record.static_item # cache the value + record.static_item = TestStaticModel.find(2) + expect(TestStaticModel).to receive(:find).once.and_call_original + record.static_item # should refetch the object + end + end + + describe 'query method' do + it 'returns true when foreign key is present' do + record.static_item_id = 1 + expect(record.static_item?).to be true + end + + it 'returns false when foreign key is nil' do + record.static_item_id = nil + expect(record.static_item?).to be false + end + end + + describe '#reset' do + it 'clears the cache when reloading' do + record.static_item_id = 1 + record.static_item # cache the value + expect(TestStaticModel).to receive(:find).once.and_call_original + record.reset + record.static_item # should refetch the object + end + + context 'when original #reset is not defined' do + before do + stub_const('TestRecord', Class.new do + include ActiveModel::Attributes + include ActiveRecord::FixedItemsModel::HasOne + + attribute :static_item_id, :integer + + belongs_to_fixed_items :static_item, fixed_items_class: TestStaticModel + end) + end + + it 'does not raise an error' do + expect { record.reset }.not_to raise_error + end + end + end +end diff --git a/gems/activerecord-gitlab/spec/active_record/fixed_items_model/model_spec.rb b/gems/activerecord-gitlab/spec/active_record/fixed_items_model/model_spec.rb new file mode 100644 index 0000000000000..cb5d1195bc6bd --- /dev/null +++ b/gems/activerecord-gitlab/spec/active_record/fixed_items_model/model_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActiveRecord::FixedItemsModel::Model, feature_category: :shared do + before do + stub_const('TestStaticModel', Class.new do + include ActiveModel::Model + include ActiveModel::Attributes + include ActiveRecord::FixedItemsModel::Model + + attribute :id, :integer + attribute :name, :string + attribute :category + end) + + stub_const('TestStaticModel::ITEMS', [ + { id: 1, name: 'Item 1', category: :a }, + { id: 2, name: 'Item 2', category: :b }, + { id: 3, name: 'Item 3', category: :a } + ].freeze) + end + + describe '.find' do + it 'returns the correct item by id' do + item = TestStaticModel.find(2) + expect(item.name).to eq('Item 2') + end + + it 'returns nil for non-existent id' do + expect(TestStaticModel.find(999)).to be_nil + end + + it 'caches the found instance' do + item1 = TestStaticModel.find(1) + item2 = TestStaticModel.find(1) + expect(item1).to be(item2) + end + end + + describe '.all' do + it 'returns all items' do + expect(TestStaticModel.all.map(&:id)).to eq([1, 2, 3]) + end + end + + describe '.where' do + it 'returns items matching the conditions' do + items = TestStaticModel.where(category: :a) + expect(items.map(&:id)).to eq([1, 3]) + end + + it 'returns empty array when no items match' do + expect(TestStaticModel.where(category: :c)).to be_empty + end + + it 'handles multiple conditions' do + items = TestStaticModel.where(category: :a, name: 'Item 1') + expect(items.map(&:id)).to eq([1]) + end + + it 'handles array conditions' do + items = TestStaticModel.where(category: [:a, :b]) + expect(items.map(&:id)).to eq([1, 2, 3]) + end + end + + describe '.find_by' do + it 'returns the first item matching the conditions' do + item = TestStaticModel.find_by(category: :a) + expect(item.id).to eq(1) + end + + it 'returns nil when no items match' do + expect(TestStaticModel.find_by(category: :c)).to be_nil + end + end + + describe 'cache isolation' do + it 'creates new cache instances for each subclass' do + # Create a subclass of TestModelA + subclass = Class.new(TestStaticModel) + + # Modifying the subclass cache shouldn't affect the parent class data + # rubocop:disable GitlabSecurity/PublicSend -- Just used for mocking + subclass.send(:find_instances)[2] = 'test' + expect(subclass.send(:find_instances)[2]).to eq('test') + expect(TestStaticModel.send(:find_instances)[2]).to be_nil + # rubocop:enable GitlabSecurity/PublicSend + end + end + + describe '#matches?' do + let(:item) { TestStaticModel.find(1) } + + it 'returns true when all conditions match' do + expect(item.matches?(category: :a, name: 'Item 1')).to be true + end + + it 'returns false when any condition does not match' do + expect(item.matches?(category: :b, name: 'Item 1')).to be false + end + + it 'handles array conditions' do + expect(item.matches?(category: [:a, :b])).to be true + expect(item.matches?(category: [:b, :c])).to be false + end + + it 'does not match with unpermitted attribute' do + expect(item).not_to receive(:doesnt_exist) + expect(item.matches?(doesnt_exist: 'test', name: 'Item 1')).to be false + end + end + + describe '#has_attribute?' do + let(:item) { TestStaticModel.new(id: 1) } + + it 'returns true for valid attributes' do + expect(item.has_attribute?(:id)).to be true + end + + it 'returns false for invalid attributes' do + expect(item.has_attribute?(:non_existent)).to be false + end + + it 'handles both symbol and string keys' do + expect(item.has_attribute?(:id)).to be true + expect(item.has_attribute?('id')).to be true + end + + it 'returns false for nil or empty string keys' do + expect(item.has_attribute?(nil)).to be false + expect(item.has_attribute?('')).to be false + end + end + + describe '#read_attribute' do + let(:item) { TestStaticModel.new(id: 1, name: 'Test', category: :a) } + + it 'returns the value of a valid attribute' do + expect(item.read_attribute(:id)).to eq(1) + expect(item.read_attribute(:name)).to eq('Test') + expect(item.read_attribute(:category)).to eq(:a) + end + + it 'returns nil for an invalid attribute' do + expect(item.read_attribute(:non_existent)).to be_nil + end + + it 'handles both symbol and string keys' do + expect(item.read_attribute(:id)).to eq(1) + expect(item.read_attribute('id')).to eq(1) + end + + it 'returns nil for nil or empty string keys' do + expect(item.read_attribute(nil)).to be_nil + expect(item.read_attribute('')).to be_nil + end + end + + describe '#inspect' do + it 'returns a string representation of the object' do + item = TestStaticModel.find(1) + expect(item.inspect).to eq('#<TestStaticModel id: 1, name: "Item 1", category: :a>') + end + end +end diff --git a/gems/activerecord-gitlab/spec/spec_helper.rb b/gems/activerecord-gitlab/spec/spec_helper.rb index 548295b3f2cf7..5cc4faf2e37c8 100644 --- a/gems/activerecord-gitlab/spec/spec_helper.rb +++ b/gems/activerecord-gitlab/spec/spec_helper.rb @@ -3,6 +3,7 @@ ENV["RAILS_ENV"] = "test" require "active_record/gitlab_patches" +require "active_record/fixed_items_model" RSpec.configure do |config| # Enable flags like --only-failures and --next-failure -- GitLab