Skip to content

Commit 304615e

Browse files
committed
[MODEL] Fixed the incorrect protection for existing model methods (such as search)
Due to the different scope in the `class << self`, the `respond_to?` condition always returned false, even when the model had in fact defined the method in question (such as `search`). Instead of using `respond_to?`, `self.public_instance_methods.include?` has been used. The concrete example is eg. [Ransack's](https://github.com/activerecord-hackery/ransack) `search` method. Closes elastic#96
1 parent b75582f commit 304615e

File tree

2 files changed

+15
-7
lines changed

2 files changed

+15
-7
lines changed

elasticsearch-model/lib/elasticsearch/model.rb

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ module Elasticsearch
6262
# # ...
6363
#
6464
module Model
65+
METHODS = [:search, :mapping, :mappings, :settings, :index_name, :document_type, :import]
6566

6667
# Adds the `Elasticsearch::Model` functionality to the including class.
6768
#
@@ -107,13 +108,9 @@ def as_indexed_json(options={})
107108
# Delegate important methods to the `__elasticsearch__` proxy, unless they are defined already
108109
#
109110
class << self
110-
delegate :search, to: :__elasticsearch__ unless respond_to?(:search)
111-
delegate :mapping, to: :__elasticsearch__ unless respond_to?(:mapping)
112-
delegate :mappings, to: :__elasticsearch__ unless respond_to?(:mappings)
113-
delegate :settings, to: :__elasticsearch__ unless respond_to?(:settings)
114-
delegate :index_name, to: :__elasticsearch__ unless respond_to?(:index_name)
115-
delegate :document_type, to: :__elasticsearch__ unless respond_to?(:document_type)
116-
delegate :import, to: :__elasticsearch__ unless respond_to?(:import)
111+
METHODS.each do |method|
112+
delegate method, to: :__elasticsearch__ unless self.public_instance_methods.include?(method)
113+
end
117114
end
118115

119116
# Mix the importing module into the proxy

elasticsearch-model/test/unit/module_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ class Elasticsearch::Model::ModuleTest < Test::Unit::TestCase
2222

2323
context "when included in module/class, " do
2424
class ::DummyIncludingModel; end
25+
class ::DummyIncludingModelWithSearchMethodDefined
26+
def self.search(query, options={})
27+
"SEARCH"
28+
end
29+
end
2530

2631
should "include and set up the proxy" do
2732
DummyIncludingModel.__send__ :include, Elasticsearch::Model
@@ -40,6 +45,12 @@ class ::DummyIncludingModel; end
4045
assert_respond_to DummyIncludingModel, :document_type
4146
assert_respond_to DummyIncludingModel, :import
4247
end
48+
49+
should "not override existing method" do
50+
DummyIncludingModelWithSearchMethodDefined.__send__ :include, Elasticsearch::Model
51+
52+
assert_equal 'SEARCH', DummyIncludingModelWithSearchMethodDefined.search('foo')
53+
end
4354
end
4455

4556
end

0 commit comments

Comments
 (0)