Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion elasticsearch-model/lib/elasticsearch/model/naming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,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
Expand Down
210 changes: 145 additions & 65 deletions elasticsearch-model/spec/elasticsearch/model/naming_inheritance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,105 +19,185 @@

describe 'naming inheritance' do

before(:all) do
class ::TestBase
extend ActiveModel::Naming
context 'without using proxy' do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just move d the tests into a context and changed the way the classes are defined so that they can be redefined with a different superclass in the other context.

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

class ::Dog < ::Animal
after(:all) do
remove_classes(TestBase, Animal, MyNamespace, Cat)
end

module ::MyNamespace
class Dog < ::Animal
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

class ::Cat < ::Animal
extend ActiveModel::Naming

extend Elasticsearch::Model::Naming::ClassMethods
include Elasticsearch::Model::Naming::InstanceMethods
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

index_name "cats"
document_type "cat"
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

it 'returns the ancestor index name' do
expect(Dog.index_name).to eq('mammals')
expect(Dog.new.index_name).to eq('mammals')
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')
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

context 'when using proxy' do
before(:all) do
TestBase = Class.new do
extend ActiveModel::Naming

include Elasticsearch::Model
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could just copy the required code from Elasticsearch::Model in here but that would result in tests passing even if the Elasticsearch::Model changed.

end

Animal = Class.new TestBase do
index_name "mammals"
document_type "mammal"
end

Dog = Class.new Animal

it 'returns the explicit index name' do
expect(Animal.index_name).to eq('mammals')
expect(Animal.new.index_name).to eq('mammals')
module MyNamespace
Dog = Class.new Animal
end

expect(Cat.index_name).to eq('cats')
expect(Cat.new.index_name).to eq('cats')
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 nil' do
expect(TestBase.document_type).to be_nil
expect(TestBase.new.document_type).to be_nil
end
describe '#index_name' do

it 'returns the explicit document type' do
expect(Animal.document_type).to eq('mammal')
expect(Animal.new.document_type).to eq('mammal')
it 'returns the default index name' do
expect(TestBase.index_name).to eq('test_bases')
end

expect(Cat.document_type).to eq('cat')
expect(Cat.new.document_type).to eq('cat')
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