Skip to content

Commit 55af45b

Browse files
Empactkarmi
authored andcommitted
[MODEL] On #import, raise an exception if the index does not exists
This is to avoid the index being created based on the structure of the bulk import json. Extract #index_exists? to serve. Closes elastic#253
1 parent 55e50c6 commit 55af45b

File tree

4 files changed

+116
-24
lines changed

4 files changed

+116
-24
lines changed

elasticsearch-model/lib/elasticsearch/model/importing.rb

+3
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,9 @@ def import(options={}, &block)
114114

115115
if options.delete(:force)
116116
self.create_index! force: true, index: target_index
117+
elsif !self.index_exists? index: target_index
118+
raise ArgumentError,
119+
"#{target_index} does not exist to be imported into. Use create_index! or the :force option to create it."
117120
end
118121

119122
__find_in_batches(options) do |batch|

elasticsearch-model/lib/elasticsearch/model/indexing.rb

+17-1
Original file line numberDiff line numberDiff line change
@@ -224,14 +224,30 @@ def create_index!(options={})
224224

225225
delete_index!(options.merge index: target_index) if options[:force]
226226

227-
unless ( self.client.indices.exists(index: target_index) rescue false )
227+
unless index_exists?(index: target_index)
228228
self.client.indices.create index: target_index,
229229
body: {
230230
settings: self.settings.to_hash,
231231
mappings: self.mappings.to_hash }
232232
end
233233
end
234234

235+
# Returns true if the index exists
236+
#
237+
# @example Check whether the model's index exists
238+
#
239+
# Article.__elasticsearch__.index_exists?
240+
#
241+
# @example Check whether a specific index exists
242+
#
243+
# Article.__elasticsearch__.index_exists? index: 'my-index'
244+
#
245+
def index_exists?(options={})
246+
target_index = options[:index] || self.index_name
247+
248+
self.client.indices.exists(index: target_index) rescue false
249+
end
250+
235251
# Deletes the index with corresponding name
236252
#
237253
# @example Delete the index for the `Article` model

elasticsearch-model/test/unit/importing_test.rb

+39-12
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ def importing_mixin
4444
DummyImportingModel.expects(:client).returns(client)
4545
DummyImportingModel.expects(:index_name).returns('foo')
4646
DummyImportingModel.expects(:document_type).returns('foo')
47+
DummyImportingModel.stubs(:index_exists?).returns(true)
4748
DummyImportingModel.stubs(:__batch_to_bulk)
4849
assert_equal 0, DummyImportingModel.import
4950
end
@@ -61,6 +62,7 @@ def importing_mixin
6162
DummyImportingModel.stubs(:client).returns(client)
6263
DummyImportingModel.stubs(:index_name).returns('foo')
6364
DummyImportingModel.stubs(:document_type).returns('foo')
65+
DummyImportingModel.stubs(:index_exists?).returns(true)
6466
DummyImportingModel.stubs(:__batch_to_bulk)
6567

6668
assert_equal 1, DummyImportingModel.import
@@ -79,6 +81,7 @@ def importing_mixin
7981
DummyImportingModel.stubs(:client).returns(client)
8082
DummyImportingModel.stubs(:index_name).returns('foo')
8183
DummyImportingModel.stubs(:document_type).returns('foo')
84+
DummyImportingModel.stubs(:index_exists?).returns(true)
8285
DummyImportingModel.stubs(:__batch_to_bulk)
8386

8487
assert_equal [{'index' => {'error' => 'FAILED'}}], DummyImportingModel.import(return: 'errors')
@@ -97,29 +100,50 @@ def importing_mixin
97100
DummyImportingModel.stubs(:client).returns(client)
98101
DummyImportingModel.stubs(:index_name).returns('foo')
99102
DummyImportingModel.stubs(:document_type).returns('foo')
103+
DummyImportingModel.stubs(:index_exists?).returns(true)
100104
DummyImportingModel.stubs(:__batch_to_bulk)
101105

102106
DummyImportingModel.import do |response|
103107
assert_equal 2, response['items'].size
104108
end
105109
end
106110

107-
should "delete and create the index with the force option" do
108-
DummyImportingModel.expects(:__find_in_batches).with do |options|
109-
assert_equal 'bar', options[:foo]
110-
assert_nil options[:force]
111-
true
112-
end
111+
context "when the index does not exist" do
112+
should "raise an exception" do
113+
Elasticsearch::Model::Adapter.expects(:from_class)
114+
.with(DummyImportingModel)
115+
.returns(DummyImportingAdapter)
116+
117+
DummyImportingModel.__send__ :include, Elasticsearch::Model::Importing
118+
119+
DummyImportingModel.expects(:index_name).returns('foo')
120+
DummyImportingModel.expects(:document_type).returns('foo')
121+
DummyImportingModel.expects(:index_exists?).returns(false)
113122

114-
DummyImportingModel.expects(:create_index!).with do |options|
115-
assert_equal true, options[:force]
116-
true
123+
assert_raise ArgumentError do
124+
DummyImportingModel.import
125+
end
117126
end
127+
end
118128

119-
DummyImportingModel.expects(:index_name).returns('foo')
120-
DummyImportingModel.expects(:document_type).returns('foo')
129+
context "with the force option" do
130+
should "delete and create the index" do
131+
DummyImportingModel.expects(:__find_in_batches).with do |options|
132+
assert_equal 'bar', options[:foo]
133+
assert_nil options[:force]
134+
true
135+
end
136+
137+
DummyImportingModel.expects(:create_index!).with do |options|
138+
assert_equal true, options[:force]
139+
true
140+
end
141+
142+
DummyImportingModel.expects(:index_name).returns('foo')
143+
DummyImportingModel.expects(:document_type).returns('foo')
121144

122-
DummyImportingModel.import force: true, foo: 'bar'
145+
DummyImportingModel.import force: true, foo: 'bar'
146+
end
123147
end
124148

125149
should "allow passing a different index / type" do
@@ -141,6 +165,7 @@ def importing_mixin
141165
.returns({'items' => [ {'index' => {} }]})
142166

143167
DummyImportingModel.stubs(:client).returns(client)
168+
DummyImportingModel.stubs(:index_exists?).returns(true)
144169
DummyImportingModel.stubs(:__batch_to_bulk)
145170

146171
DummyImportingModel.import index: 'my-new-index', type: 'my-other-type'
@@ -151,6 +176,7 @@ def importing_mixin
151176
transform = lambda {|a|}
152177

153178
DummyImportingModel.stubs(:client).returns(client)
179+
DummyImportingModel.stubs(:index_exists?).returns(true)
154180
DummyImportingModel.expects(:__transform).returns(transform)
155181
DummyImportingModel.expects(:__batch_to_bulk).with(anything, transform)
156182

@@ -162,6 +188,7 @@ def importing_mixin
162188
transform = lambda {|a|}
163189

164190
DummyImportingModel.stubs(:client).returns(client)
191+
DummyImportingModel.stubs(:index_exists?).returns(true)
165192
DummyImportingModel.expects(:__batch_to_bulk).with(anything, transform)
166193

167194
DummyImportingModel.import index: 'foo', type: 'bar', transform: transform

elasticsearch-model/test/unit/indexing_test.rb

+57-11
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,55 @@ def as_indexed_json(options={})
414414
end
415415
end
416416

417+
context "Checking for index existence" do
418+
context "the index exists" do
419+
should "return true" do
420+
indices = mock('indices', exists: true)
421+
client = stub('client', indices: indices)
422+
423+
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
424+
425+
assert_equal true, DummyIndexingModelForRecreate.index_exists?
426+
end
427+
end
428+
429+
context "the index does not exists" do
430+
should "return false" do
431+
indices = mock('indices', exists: false)
432+
client = stub('client', indices: indices)
433+
434+
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
435+
436+
assert_equal false, DummyIndexingModelForRecreate.index_exists?
437+
end
438+
end
439+
440+
context "the indices raises" do
441+
should "return false" do
442+
client = stub('client')
443+
client.expects(:indices).raises(StandardError)
444+
445+
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
446+
447+
assert_equal false, DummyIndexingModelForRecreate.index_exists?
448+
end
449+
end
450+
451+
context "the indices raises" do
452+
should "return false" do
453+
indices = stub('indices')
454+
client = stub('client')
455+
client.expects(:indices).returns(indices)
456+
457+
indices.expects(:exists).raises(StandardError)
458+
459+
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
460+
461+
assert_equal false, DummyIndexingModelForRecreate.index_exists?
462+
end
463+
end
464+
end
465+
417466
context "Re-creating the index" do
418467
class ::DummyIndexingModelForRecreate
419468
extend ActiveModel::Naming
@@ -468,15 +517,14 @@ class ::DummyIndexingModelForRecreate
468517
indices = stub('indices')
469518
client.stubs(:indices).returns(indices)
470519

471-
indices.expects(:exists).returns(false)
472-
473520
indices.expects(:create).with do |payload|
474521
assert_equal 'dummy_indexing_model_for_recreates', payload[:index]
475522
assert_equal 1, payload[:body][:settings][:index][:number_of_shards]
476523
assert_equal 'keyword', payload[:body][:mappings][:dummy_indexing_model_for_recreate][:properties][:foo][:analyzer]
477524
true
478525
end.returns({})
479526

527+
DummyIndexingModelForRecreate.expects(:index_exists?).returns(false)
480528
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
481529

482530
assert_nothing_raised { DummyIndexingModelForRecreate.create_index! }
@@ -487,11 +535,10 @@ class ::DummyIndexingModelForRecreate
487535
indices = stub('indices')
488536
client.stubs(:indices).returns(indices)
489537

490-
indices.expects(:exists).returns(true)
491-
492538
indices.expects(:create).never
493539

494-
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
540+
DummyIndexingModelForRecreate.expects(:index_exists?).returns(true)
541+
DummyIndexingModelForRecreate.expects(:client).returns(client).never
495542

496543
assert_nothing_raised { DummyIndexingModelForRecreate.create_index! }
497544
end
@@ -502,9 +549,9 @@ class ::DummyIndexingModelForRecreate
502549
client.stubs(:indices).returns(indices)
503550

504551
indices.expects(:delete).returns({})
505-
indices.expects(:exists).returns(false)
506-
indices.expects(:create).raises(Exception)
552+
indices.expects(:create).raises(Exception).at_least_once
507553

554+
DummyIndexingModelForRecreate.expects(:index_exists?).returns(false)
508555
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
509556

510557
assert_raise(Exception) { DummyIndexingModelForRecreate.create_index! force: true }
@@ -516,9 +563,9 @@ class ::DummyIndexingModelForRecreate
516563
client.stubs(:indices).returns(indices)
517564

518565
indices.expects(:delete).returns({})
519-
indices.expects(:exists).returns(false)
520566
indices.expects(:create).returns({}).at_least_once
521567

568+
DummyIndexingModelForRecreate.expects(:index_exists?).returns(false)
522569
DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
523570

524571
assert_nothing_raised do
@@ -559,12 +606,11 @@ class ::DummyIndexingModelForRecreate
559606
end
560607

561608
should "create the custom index" do
562-
@indices.expects(:exists).with do |arguments|
609+
@indices.expects(:create).with do |arguments|
563610
assert_equal 'custom-foo', arguments[:index]
564611
true
565612
end
566-
567-
@indices.expects(:create).with do |arguments|
613+
DummyIndexingModelForRecreate.expects(:index_exists?).with do |arguments|
568614
assert_equal 'custom-foo', arguments[:index]
569615
true
570616
end

0 commit comments

Comments
 (0)