From 81536dd3680048a94098a28017e98d5528dcacc6 Mon Sep 17 00:00:00 2001 From: vanboom Date: Thu, 9 Nov 2023 19:08:16 -0600 Subject: [PATCH 1/2] Addressing #838, STI issues when code is reloaded in development, fix index name to klass lookup methodology --- .../lib/elasticsearch/model/adapters/multiple.rb | 12 ++++++------ .../lib/elasticsearch/model/multimodel.rb | 7 ++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb b/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb index 30d851043..602f644bf 100644 --- a/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb +++ b/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb @@ -106,16 +106,16 @@ def __ids_by_type # @api private # def __type_for_hit(hit) - @@__types ||= {} key = "#{hit[:_index]}::#{hit[:_type]}" if hit[:_type] && hit[:_type] != '_doc' key = hit[:_index] unless key - @@__types[key] ||= begin - Registry.all.detect do |model| - (model.index_name == hit[:_index] && __no_type?(hit)) || - (model.index_name == hit[:_index] && model.document_type == hit[:_type]) - end + # DVB -- not sure the ramifications of this - but do not memoize the model/klass + # I do not think that caching the types is necessary, minimal processing savings + # at the expense of broken STI autoloading in development, see #848 + Registry.all.detect do |model| + (model.index_name == hit[:_index] && __no_type?(hit)) || + (model.index_name == hit[:_index] && model.document_type == hit[:_type]) end end diff --git a/elasticsearch-model/lib/elasticsearch/model/multimodel.rb b/elasticsearch-model/lib/elasticsearch/model/multimodel.rb index 6b5fc2a81..79c9f316a 100644 --- a/elasticsearch-model/lib/elasticsearch/model/multimodel.rb +++ b/elasticsearch-model/lib/elasticsearch/model/multimodel.rb @@ -48,7 +48,12 @@ def self.all # Adds a model to the registry # def add(klass) - @models << klass + # Detect already loaded models and ensure that a duplicate is not stored + if i = @models.index{ |_class| _class.name == klass.name } + @models[i] = klass + else + @models << klass + end end # Returns a copy of the registered models From 11df40bd07a1e2ecbff8a2467061e97ee79ed238 Mon Sep 17 00:00:00 2001 From: vanboom Date: Sat, 11 Nov 2023 11:57:25 -0600 Subject: [PATCH 2/2] refactoring index to model map concern into Registry to address #838 and improve autoloader compatibility --- .../elasticsearch/model/adapters/multiple.rb | 16 +++---- .../lib/elasticsearch/model/multimodel.rb | 45 ++++++++++++++++++- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb b/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb index 602f644bf..7e91b6962 100644 --- a/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb +++ b/elasticsearch-model/lib/elasticsearch/model/adapters/multiple.rb @@ -107,16 +107,14 @@ def __ids_by_type # def __type_for_hit(hit) - key = "#{hit[:_index]}::#{hit[:_type]}" if hit[:_type] && hit[:_type] != '_doc' - key = hit[:_index] unless key - - # DVB -- not sure the ramifications of this - but do not memoize the model/klass - # I do not think that caching the types is necessary, minimal processing savings - # at the expense of broken STI autoloading in development, see #848 - Registry.all.detect do |model| - (model.index_name == hit[:_index] && __no_type?(hit)) || - (model.index_name == hit[:_index] && model.document_type == hit[:_type]) + key = if hit[:_type] && hit[:_type] != '_doc' + "#{hit[:_index]}::#{hit[:_type]}" + else + hit[:_index] end + + # DVB -- #838, refactor the lookup of index name to model into the Registry + Registry.lookup(key, hit[:_type]) end def __no_type?(hit) diff --git a/elasticsearch-model/lib/elasticsearch/model/multimodel.rb b/elasticsearch-model/lib/elasticsearch/model/multimodel.rb index 79c9f316a..d71c44552 100644 --- a/elasticsearch-model/lib/elasticsearch/model/multimodel.rb +++ b/elasticsearch-model/lib/elasticsearch/model/multimodel.rb @@ -19,10 +19,11 @@ module Elasticsearch module Model # Keeps a global registry of classes that include `Elasticsearch::Model` - # + # Keeps a global registry of index to class mappings class Registry def initialize @models = [] + @indexes = {} end # Returns the unique instance of the registry (Singleton) @@ -45,22 +46,64 @@ def self.all __instance.models end + def self.indexes + __instance.indexes + end + + def self.add_index(index, model) + __instance.add_index(index, model) + end + # Adds a model to the registry # def add(klass) # Detect already loaded models and ensure that a duplicate is not stored if i = @models.index{ |_class| _class.name == klass.name } @models[i] = klass + # clear the cached index map (autoloading in development causes this) + @indexes.clear else @models << klass end end + def add_index(index, model) + @indexes[index] = model + end + # Returns a copy of the registered models # def models @models.dup end + + def indexes + @indexes.dup + end + + ## + # Find the model matching the given index and document type from a search hit + # Cache the index->model mapping for performance + # Clear the index cache when models are reloaded + def self.lookup(index, type=nil) + if Registry.indexes.has_key?(index) + # Cache hit + Registry.indexes[index] + else + # Cache bust + model = if type.nil? or type == "_doc" + # lookup strictly by index for generic document types + Registry.all.detect{|m| m.index_name == index} + else + # lookup using index and type + Registry.all.detect{|m| m.index_name == index and model.document_type == type} + end + # cache the index to model mapping + Registry.add_index(index, model) + model + end + end + end # Wraps a collection of models when querying multiple indices