[Model] Fix naming with inheritance when using Proxy (elastic#887)

* Add naming inheritance tests when using a proxy

* Skip circular call to index_name/document_type when Proxy is used and inheritance is enabled

Author: tmandke

elasticsearch-model/lib/elasticsearch/model/naming.rb

@@ -79,7 +79,12 @@ def implicit(prop)
 
             if Elasticsearch::Model.settings[:inheritance_enabled]
               self.ancestors.each do |klass|
-                next if klass == self
+                # When Naming is included in Proxy::ClassMethods the actual model
+                # is among its ancestors. We don't want to call the actual model
+                # since it will result in the same call to the same instance of
+                # Proxy::ClassMethods. To prevent this we also skip the ancestor
+                # that is the target.
+                next if klass == self || self.respond_to?(:target) && klass == self.target
                 break if value = klass.respond_to?(prop) && klass.send(prop)
               end
             end

elasticsearch-model/spec/elasticsearch/model/naming_inheritance_spec.rb

@@ -2,22 +2,45 @@
 
 describe 'naming inheritance' do
 
-  before(:all) do
-    class ::TestBase
-      extend ActiveModel::Naming
+  context 'without using proxy' do
+    before(:all) do
+      TestBase = Class.new do
+        extend ActiveModel::Naming
 
-      extend  Elasticsearch::Model::Naming::ClassMethods
-      include Elasticsearch::Model::Naming::InstanceMethods
-    end
+        extend  Elasticsearch::Model::Naming::ClassMethods
+        include Elasticsearch::Model::Naming::InstanceMethods
+      end
+
+      Animal = Class.new TestBase do
+        extend ActiveModel::Naming
+
+        extend  Elasticsearch::Model::Naming::ClassMethods
+        include Elasticsearch::Model::Naming::InstanceMethods
+
+        index_name "mammals"
+        document_type "mammal"
+      end
+
+      Dog = Class.new Animal
 
-    class ::Animal < ::TestBase
-      extend ActiveModel::Naming
+      module ::MyNamespace
+        Dog = Class.new Animal
+      end
+
+      Cat = Class.new Animal do
+        extend ActiveModel::Naming
 
-      extend  Elasticsearch::Model::Naming::ClassMethods
-      include Elasticsearch::Model::Naming::InstanceMethods
+        extend  Elasticsearch::Model::Naming::ClassMethods
+        include Elasticsearch::Model::Naming::InstanceMethods
+
+        index_name "cats"
+        document_type "cat"
+      end
 
-      index_name "mammals"
-      document_type "mammal"
+    end
+
+    after(:all) do
+      remove_classes(TestBase, Animal, MyNamespace, Cat)
     end
 
     around(:all) do |example|
@@ -27,84 +50,141 @@ class ::Animal < ::TestBase
       Elasticsearch::Model.inheritance_enabled = original_value
     end
 
-    module ::MyNamespace
-      class Dog < ::Animal
+    describe '#index_name' do
+
+      it 'returns the default index name' do
+        expect(TestBase.index_name).to eq('test_bases')
+        expect(TestBase.new.index_name).to eq('test_bases')
       end
-    end
 
-    class ::Cat < ::Animal
-      extend ActiveModel::Naming
+      it 'returns the explicit index name' do
+        expect(Animal.index_name).to eq('mammals')
+        expect(Animal.new.index_name).to eq('mammals')
+
+        expect(Cat.index_name).to eq('cats')
+        expect(Cat.new.index_name).to eq('cats')
+      end
 
-      extend  Elasticsearch::Model::Naming::ClassMethods
-      include Elasticsearch::Model::Naming::InstanceMethods
+      it 'returns the ancestor index name' do
+        expect(Dog.index_name).to eq('mammals')
+        expect(Dog.new.index_name).to eq('mammals')
+      end
 
-      index_name "cats"
-      document_type "cat"
+      it 'returns the ancestor index name for namespaced models' do
+        expect(::MyNamespace::Dog.index_name).to eq('mammals')
+        expect(::MyNamespace::Dog.new.index_name).to eq('mammals')
+      end
     end
 
-  end
+    describe '#document_type' do
 
-  after(:all) do
-    remove_classes(TestBase, Animal, MyNamespace, Cat)
-  end
+      it 'returns nil' do
+        expect(TestBase.document_type).to be_nil
+        expect(TestBase.new.document_type).to be_nil
+      end
 
-  around(:all) do |example|
-    original_value = Elasticsearch::Model.settings[:inheritance_enabled]
-    Elasticsearch::Model.settings[:inheritance_enabled] = true
-    example.run
-    Elasticsearch::Model.settings[:inheritance_enabled] = original_value
-  end
+      it 'returns the explicit document type' do
+        expect(Animal.document_type).to eq('mammal')
+        expect(Animal.new.document_type).to eq('mammal')
 
+        expect(Cat.document_type).to eq('cat')
+        expect(Cat.new.document_type).to eq('cat')
+      end
 
-  describe '#index_name' do
+      it 'returns the ancestor document type' do
+        expect(Dog.document_type).to eq('mammal')
+        expect(Dog.new.document_type).to eq('mammal')
+      end
 
-    it 'returns the default index name' do
-      expect(TestBase.index_name).to eq('test_bases')
-      expect(TestBase.new.index_name).to eq('test_bases')
+      it 'returns the ancestor document type for namespaced models' do
+        expect(::MyNamespace::Dog.document_type).to eq('mammal')
+        expect(::MyNamespace::Dog.new.document_type).to eq('mammal')
+      end
     end
+  end
 
-    it 'returns the explicit index name' do
-      expect(Animal.index_name).to eq('mammals')
-      expect(Animal.new.index_name).to eq('mammals')
+  context 'when using proxy' do
+    before(:all) do
+      TestBase = Class.new do
+        extend ActiveModel::Naming
 
-      expect(Cat.index_name).to eq('cats')
-      expect(Cat.new.index_name).to eq('cats')
+        include Elasticsearch::Model
+      end
+
+      Animal = Class.new TestBase do
+        index_name "mammals"
+        document_type "mammal"
+      end
+
+      Dog = Class.new Animal
+
+      module MyNamespace
+        Dog = Class.new Animal
+      end
+
+      Cat = Class.new Animal do
+        index_name "cats"
+        document_type "cat"
+      end
     end
 
-    it 'returns the ancestor index name' do
-      expect(Dog.index_name).to eq('mammals')
-      expect(Dog.new.index_name).to eq('mammals')
+    after(:all) do
+      remove_classes(TestBase, Animal, MyNamespace, Cat)
     end
 
-    it 'returns the ancestor index name for namespaced models' do
-      expect(::MyNamespace::Dog.index_name).to eq('mammals')
-      expect(::MyNamespace::Dog.new.index_name).to eq('mammals')
+    around(:all) do |example|
+      original_value = Elasticsearch::Model.settings[:inheritance_enabled]
+      Elasticsearch::Model.settings[:inheritance_enabled] = true
+      example.run
+      Elasticsearch::Model.settings[:inheritance_enabled] = original_value
     end
-  end
 
-  describe '#document_type' do
 
     it 'returns the default document type' do
       expect(TestBase.document_type).to eq('_doc')
       expect(TestBase.new.document_type).to eq('_doc')
     end
 
-    it 'returns the explicit document type' do
-      expect(Animal.document_type).to eq('mammal')
-      expect(Animal.new.document_type).to eq('mammal')
+    describe '#index_name' do
 
-      expect(Cat.document_type).to eq('cat')
-      expect(Cat.new.document_type).to eq('cat')
-    end
+      it 'returns the default index name' do
+        expect(TestBase.index_name).to eq('test_bases')
+      end
+
+      it 'returns the explicit index name' do
+        expect(Animal.index_name).to eq('mammals')
 
-    it 'returns the ancestor document type' do
-      expect(Dog.document_type).to eq('mammal')
-      expect(Dog.new.document_type).to eq('mammal')
+        expect(Cat.index_name).to eq('cats')
+      end
+
+      it 'returns the ancestor index name' do
+        expect(Dog.index_name).to eq('mammals')
+      end
+
+      it 'returns the ancestor index name for namespaced models' do
+        expect(::MyNamespace::Dog.index_name).to eq('mammals')
+      end
     end
 
-    it 'returns the ancestor document type for namespaced models' do
-      expect(::MyNamespace::Dog.document_type).to eq('mammal')
-      expect(::MyNamespace::Dog.new.document_type).to eq('mammal')
+    describe '#document_type' do
+
+      it 'returns nil' do
+        expect(TestBase.document_type).to be_nil
+      end
+
+      it 'returns the explicit document type' do
+        expect(Animal.document_type).to eq('mammal')
+
+        expect(Cat.document_type).to eq('cat')
+      end
+
+      it 'returns the ancestor document type' do
+        expect(Dog.document_type).to eq('mammal')
+      end
+
+      it 'returns the ancestor document type for namespaced models' do
+        expect(::MyNamespace::Dog.document_type).to eq('mammal')
+      end
     end
   end
 end