Commit 92b943cb authored by Mark Chao's avatar Mark Chao

ES: For some write methods, require version to be specified

e.g. delegating delete_index! to 2 targets doesn't make sense.
parent 3ccf561f
...@@ -26,8 +26,12 @@ module Elastic ...@@ -26,8 +26,12 @@ module Elastic
end end
class_methods do class_methods do
def write_methods def methods_for_all_write_targets
%i(import create_index! delete_index! refresh_index!) %i(refresh_index!)
end
def methods_for_one_write_target
%i(import create_index! delete_index!)
end end
end end
end end
......
...@@ -26,9 +26,13 @@ module Elastic ...@@ -26,9 +26,13 @@ module Elastic
end end
class_methods do class_methods do
def write_methods def methods_for_all_write_targets
[:index_document, :delete_document, :update_document, :update_document_attributes] [:index_document, :delete_document, :update_document, :update_document_attributes]
end end
def methods_for_one_write_target
[]
end
end end
private private
......
...@@ -7,6 +7,9 @@ module Elastic ...@@ -7,6 +7,9 @@ module Elastic
attr_reader :data_class, :data_target attr_reader :data_class, :data_target
# TODO: remove once multi-version is functional https://gitlab.com/gitlab-org/gitlab-ee/issues/10156
TARGET_VERSION = 'V12p1'
# @params version [String, Module] can be a string "V12p1" or module (Elastic::V12p1) # @params version [String, Module] can be a string "V12p1" or module (Elastic::V12p1)
def version(version) def version(version)
version = Elastic.const_get(version) if version.is_a?(String) version = Elastic.const_get(version) if version.is_a?(String)
...@@ -18,7 +21,7 @@ module Elastic ...@@ -18,7 +21,7 @@ module Elastic
# TODO: load from db table https://gitlab.com/gitlab-org/gitlab-ee/issues/12555 # TODO: load from db table https://gitlab.com/gitlab-org/gitlab-ee/issues/12555
def elastic_reading_target def elastic_reading_target
strong_memoize(:elastic_reading_target) do strong_memoize(:elastic_reading_target) do
version('V12p1') version(TARGET_VERSION)
end end
end end
...@@ -34,14 +37,16 @@ module Elastic ...@@ -34,14 +37,16 @@ module Elastic
end end
def generate_forwarding def generate_forwarding
write_methods = elastic_writing_targets.first.real_class.write_methods methods_for_all_write_targets = elastic_writing_targets.first.real_class.methods_for_all_write_targets
methods_for_one_write_target = elastic_writing_targets.first.real_class.methods_for_one_write_target
write_methods.each do |method| methods_for_all_write_targets.each do |method|
self.class.forward_write_method(method) self.class.forward_to_all_write_targets(method)
end end
read_methods = elastic_reading_target.real_class.public_instance_methods read_methods = elastic_reading_target.real_class.public_instance_methods
read_methods -= write_methods read_methods -= methods_for_all_write_targets
read_methods -= methods_for_one_write_target
read_methods -= self.class.instance_methods read_methods -= self.class.instance_methods
read_methods.delete(:method_missing) read_methods.delete(:method_missing)
...@@ -57,7 +62,7 @@ module Elastic ...@@ -57,7 +62,7 @@ module Elastic
delegate method, to: :elastic_reading_target delegate method, to: :elastic_reading_target
end end
def forward_write_method(method) def forward_to_all_write_targets(method)
return if respond_to?(method) return if respond_to?(method)
define_method(method) do |*args| define_method(method) do |*args|
......
...@@ -26,8 +26,8 @@ describe Elastic::MultiVersionClassProxy do ...@@ -26,8 +26,8 @@ describe Elastic::MultiVersionClassProxy do
allow(subject).to receive(:elastic_writing_targets).and_return([old_target, new_target]) allow(subject).to receive(:elastic_writing_targets).and_return([old_target, new_target])
end end
it 'forwards write methods to all targets' do it 'forwards methods which should touch all write targets' do
Elastic::V12p1::SnippetClassProxy.write_methods.each do |method| Elastic::V12p1::SnippetClassProxy.methods_for_all_write_targets.each do |method|
expect(new_target).to receive(method).and_return(response) expect(new_target).to receive(method).and_return(response)
expect(old_target).to receive(method).and_return(response) expect(old_target).to receive(method).and_return(response)
...@@ -43,5 +43,11 @@ describe Elastic::MultiVersionClassProxy do ...@@ -43,5 +43,11 @@ describe Elastic::MultiVersionClassProxy do
expect(subject).not_to respond_to(:method_missing) expect(subject).not_to respond_to(:method_missing)
end end
it 'does not forward write methods which should touch specific version' do
Elastic::V12p1::SnippetClassProxy.methods_for_one_write_target.each do |method|
expect(subject).not_to respond_to(method)
end
end
end end
end end
...@@ -28,8 +28,8 @@ describe Elastic::MultiVersionInstanceProxy do ...@@ -28,8 +28,8 @@ describe Elastic::MultiVersionInstanceProxy do
allow(subject).to receive(:elastic_writing_targets).and_return([old_target, new_target]) allow(subject).to receive(:elastic_writing_targets).and_return([old_target, new_target])
end end
it 'forwards write methods to all targets' do it 'forwards methods which should touch all write targets' do
Elastic::V12p1::SnippetInstanceProxy.write_methods.each do |method| Elastic::V12p1::SnippetInstanceProxy.methods_for_all_write_targets.each do |method|
expect(new_target).to receive(method).and_return(response) expect(new_target).to receive(method).and_return(response)
expect(old_target).to receive(method).and_return(response) expect(old_target).to receive(method).and_return(response)
...@@ -45,5 +45,11 @@ describe Elastic::MultiVersionInstanceProxy do ...@@ -45,5 +45,11 @@ describe Elastic::MultiVersionInstanceProxy do
expect(subject).not_to respond_to(:method_missing) expect(subject).not_to respond_to(:method_missing)
end end
it 'does not forward write methods which should touch specific version' do
Elastic::V12p1::SnippetInstanceProxy.methods_for_one_write_target.each do |method|
expect(subject).not_to respond_to(method)
end
end
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