Fixed issue #2742 - Removing a record drops whole index on Elasticsearch 6 and later.

.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

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

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