Commit b78c8dda authored by Dylan Griffith's avatar Dylan Griffith

Extract generic elastic_index_dependant_association method

This method will be used in several places as we de-normalize more data
in the index. This method at the class level also helps us avoid typo
like bugs or association rename bugs which would have previously
only have been caught later in sidekiq worker raising an exception.
Raising early when defining the class provides a safer developer
experience.
parent e17eba61
...@@ -52,7 +52,7 @@ module Elastic ...@@ -52,7 +52,7 @@ module Elastic
::Elastic::ProcessBookkeepingService.track!(self) ::Elastic::ProcessBookkeepingService.track!(self)
associations_to_update = associations_needing_elasticsearch_update associations_to_update = associations_needing_elasticsearch_update
unless associations_to_update.blank? if associations_to_update.present?
ElasticAssociationIndexerWorker.perform_async(self.class.name, id, associations_to_update) ElasticAssociationIndexerWorker.perform_async(self.class.name, id, associations_to_update)
end end
end end
...@@ -64,13 +64,43 @@ module Elastic ...@@ -64,13 +64,43 @@ module Elastic
# Override in child object if there are associations that need to be # Override in child object if there are associations that need to be
# updated when specific fields are updated # updated when specific fields are updated
def associations_needing_elasticsearch_update def associations_needing_elasticsearch_update
[] self.class.elastic_index_dependants.map do |dependant|
association_name = dependant[:association_name]
on_change = dependant[:on_change]
next nil unless previous_changes.include?(on_change)
association_name.to_s
end.compact.uniq
end end
class_methods do class_methods do
def __elasticsearch__ def __elasticsearch__
@__elasticsearch__ ||= ::Elastic::MultiVersionClassProxy.new(self) @__elasticsearch__ ||= ::Elastic::MultiVersionClassProxy.new(self)
end end
# Mark a dependant association as needing to be updated when a specific
# field in this object changes. For example if you want to update
# project.issues in the index when project.visibility_level is changed
# then you can declare that as:
#
# elastic_index_dependant_association :issues, on_change: :visibility_level
#
def elastic_index_dependant_association(association_name, on_change:)
# This class is used for non ActiveRecord models but this method is not
# applicable for that so we raise.
raise "elastic_index_dependant_association is not applicable as this class is not an ActiveRecord model." unless self < ActiveRecord::Base
# Validate these are actually correct associations before sending to
# Sidekiq to avoid errors occuring when the job is picked up.
raise "Invalid association to index. \"#{association_name}\" is either not a collection or not an association. Hint: You must declare the has_many before declaring elastic_index_dependant_association." unless reflect_on_association(association_name)&.collection?
elastic_index_dependants << { association_name: association_name, on_change: on_change }
end
def elastic_index_dependants
@elastic_index_dependants ||= []
end
end end
end end
end end
...@@ -100,6 +100,8 @@ module EE ...@@ -100,6 +100,8 @@ module EE
has_many :incident_management_oncall_schedules, class_name: 'IncidentManagement::OncallSchedule', inverse_of: :project has_many :incident_management_oncall_schedules, class_name: 'IncidentManagement::OncallSchedule', inverse_of: :project
elastic_index_dependant_association :issues, on_change: :visibility_level
scope :with_shared_runners_limit_enabled, -> do scope :with_shared_runners_limit_enabled, -> do
if ::Ci::Runner.has_shared_runners_with_non_zero_public_cost? if ::Ci::Runner.has_shared_runners_with_non_zero_public_cost?
with_shared_runners with_shared_runners
...@@ -627,15 +629,6 @@ module EE ...@@ -627,15 +629,6 @@ module EE
ElasticCommitIndexerWorker.perform_async(id, nil, nil, true) if use_elasticsearch? && !forked? ElasticCommitIndexerWorker.perform_async(id, nil, nil, true) if use_elasticsearch? && !forked?
end end
override :associations_needing_elasticsearch_update
def associations_needing_elasticsearch_update
if previous_changes.include?(:visibility_level)
['issues']
else
[]
end
end
def log_geo_updated_events def log_geo_updated_events
repository.log_geo_updated_event repository.log_geo_updated_event
wiki.repository.log_geo_updated_event wiki.repository.log_geo_updated_event
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Elastic::ApplicationVersionedSearch do
let(:klass) do
Class.new(ApplicationRecord) do
include Elastic::ApplicationVersionedSearch
has_many :widgets
end
end
describe '.elastic_index_dependant_association' do
it 'adds the associations to elastic_index_dependants' do
klass.elastic_index_dependant_association(:widgets, on_change: :title)
expect(klass.elastic_index_dependants).to include({
association_name: :widgets,
on_change: :title
})
end
context 'when the association does not exist' do
it 'raises an error' do
expect { klass.elastic_index_dependant_association(:foo_bars, on_change: :bar) }
.to raise_error("Invalid association to index. \"foo_bars\" is either not a collection or not an association. Hint: You must declare the has_many before declaring elastic_index_dependant_association.")
end
end
context 'when the class is not an ApplicationRecord' do
let(:not_application_record) do
Class.new do
include Elastic::ApplicationVersionedSearch
end
end
it 'raises an error' do
expect { not_application_record.elastic_index_dependant_association(:widgets, on_change: :title) }
.to raise_error("elastic_index_dependant_association is not applicable as this class is not an ActiveRecord model.")
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment