From f806ac36119cabb7fd2a085430c9142b778dadf5 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 26 Sep 2019 13:41:20 +0200 Subject: [PATCH] Fixed issue #2742 - Removing a record drops whole index on Elasticsearch 6 and later. --- .gitlab-ci.yml | 2 +- lib/search_index_backend.rb | 51 +++++++++++++++++++--- spec/lib/search_index_backend_spec.rb | 62 ++++++++++++++++++++------- 3 files changed, 93 insertions(+), 22 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 29c6e9f32..073032091 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -121,7 +121,7 @@ pre:github: RAILS_ENV: "test" script: - bundle exec rake zammad:db:init - - bundle exec rspec -t ~type:system + - bundle exec rspec -t ~type:system --t ~searchindex test:rspec:mysql: stage: test diff --git a/lib/search_index_backend.rb b/lib/search_index_backend.rb index 26ea8100e..7e6477ed7 100644 --- a/lib/search_index_backend.rb +++ b/lib/search_index_backend.rb @@ -251,7 +251,12 @@ remove whole data from index =end def self.remove(type, o_id = nil) - url = build_url(type, o_id, false, false) + url = if o_id + build_url(type, o_id, false, true) + else + build_url(type, o_id, false, false) + end + return if url.blank? Rails.logger.info "# curl -X DELETE \"#{url}\"" @@ -737,7 +742,25 @@ return true if backend is configured "#{local_index}_#{index.underscore.tr('/', '_')}" end - def self.build_url(type = nil, o_id = nil, pipeline = true, with_type = true) +=begin + +generate url for index or document access (only for internal use) + + # url to access single document in index (in case with_pipeline or not) + url = SearchIndexBackend.build_url('User', 123, with_pipeline) + + # url to access whole index + url = SearchIndexBackend.build_url('User') + + # url to access document definition in index (only es6 and higher) + url = SearchIndexBackend.build_url('User', nil, false, true) + + # base url + url = SearchIndexBackend.build_url + +=end + + def self.build_url(type = nil, o_id = nil, with_pipeline = true, with_document_type = true) return if !SearchIndexBackend.enabled? # for elasticsearch 5.6 and lower @@ -745,7 +768,7 @@ return true if backend is configured if Setting.get('es_multi_index') == false url = Setting.get('es_url') url = if type - if pipeline == true + if with_pipeline == true url_pipline = Setting.get('es_pipeline') if url_pipline.present? url_pipline = "?pipeline=#{url_pipline}" @@ -764,7 +787,7 @@ return true if backend is configured # for elasticsearch 6.x and higher url = Setting.get('es_url') - if pipeline == true + if with_pipeline == true url_pipline = Setting.get('es_pipeline') if url_pipline.present? url_pipline = "?pipeline=#{url_pipline}" @@ -772,20 +795,36 @@ return true if backend is configured end if type index = build_index_name(type) - if with_type == false + + # access (e. g. creating or dropping) whole index + if with_document_type == false return "#{url}/#{index}" end + # access single document in index (e. g. drop or add document) if o_id return "#{url}/#{index}/_doc/#{o_id}#{url_pipline}" end + # access document type (e. g. creating or dropping document mapping) return "#{url}/#{index}/_doc#{url_pipline}" end "#{url}/" end - def self.build_search_url(index) +=begin + +generate url searchaccess (only for internal use) + + # url search access with single index + url = SearchIndexBackend.build_search_url('User') + + # url to access all over es + url = SearchIndexBackend.build_search_url + +=end + + def self.build_search_url(index = nil) # for elasticsearch 5.6 and lower if Setting.get('es_multi_index') == false diff --git a/spec/lib/search_index_backend_spec.rb b/spec/lib/search_index_backend_spec.rb index 2b12e60ba..9773e9bab 100644 --- a/spec/lib/search_index_backend_spec.rb +++ b/spec/lib/search_index_backend_spec.rb @@ -18,9 +18,26 @@ RSpec.describe SearchIndexBackend, searchindex: true do end describe '.search' do - subject(:search) { described_class.search(query, index, limit: 3000) } + + context 'query finds results' do + + let(:record_type) { 'Ticket'.freeze } + let(:record) { create :ticket } + + before do + described_class.add(record_type, record) + described_class.refresh + end + + it 'finds added records' do + result = described_class.search(record.number, record_type, sort_by: ['updated_at'], order_by: ['desc']) + expect(result).to eq([{ id: record.id.to_s, type: record_type }]) + end + end context 'for query with no results' do + subject(:search) { described_class.search(query, index, limit: 3000) } + let(:query) { 'preferences.notification_sound.enabled:*' } context 'on a single index' do @@ -107,26 +124,41 @@ RSpec.describe SearchIndexBackend, searchindex: true do end describe '.remove' do - context 'ticket' do - it 'from index after ticket delete' do + context 'record gets deleted' do - skip('No ES configured') if !described_class.enabled? + let(:record_type) { 'Ticket'.freeze } + let(:deleted_record) { create :ticket } - ticket = create :ticket - described_class.add('Ticket', ticket) + before do + described_class.add(record_type, deleted_record) + described_class.refresh + end - # give es time to rebuild index - sleep 2 - result = described_class.search(ticket.number, 'Ticket', sort_by: ['updated_at'], order_by: ['desc']) - expect(result).to eq([{ id: ticket.id.to_s, type: 'Ticket' }]) + it 'removes record from search index' do + described_class.remove(record_type, deleted_record.id) + described_class.refresh - described_class.remove('Ticket', ticket.id) - # give es time to rebuild index - sleep 2 - - result = described_class.search(ticket.number, 'Ticket', sort_by: ['updated_at'], order_by: ['desc']) + result = described_class.search(deleted_record.number, record_type, sort_by: ['updated_at'], order_by: ['desc']) expect(result).to eq([]) end + + context 'other records present' do + + let(:other_record) { create :ticket } + + before do + described_class.add(record_type, other_record) + described_class.refresh + end + + it "doesn't remove other records" do + described_class.remove(record_type, deleted_record.id) + described_class.refresh + + result = described_class.search(other_record.number, record_type, sort_by: ['updated_at'], order_by: ['desc']) + expect(result).to eq([{ id: other_record.id.to_s, type: record_type }]) + end + end end end end