Merge branch 'es_deletes_fix' into 'master'

[ES] Record deletion fix

Specs for deletion are in MR https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/654/diffs that is built on top of this MR. I've made it in a separate MR because there is complex stuff left to do. We need another docker image where elastic has plugin `delete-by-query`

No changelog item required because the bug was introduced in master a few days ago by https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/615

See merge request !653

app/models/concerns/elastic/application_search.rb

@@ -45,7 +45,12 @@ module Elastic
 
       after_commit on: :destroy do
         if current_application_settings.elasticsearch_indexing? && self.searchable?
-          ElasticIndexerWorker.perform_async(:delete, self.class.to_s, self.id)
+          ElasticIndexerWorker.perform_async(
+            :delete,
+            self.class.to_s,
+            self.id,
+            project_id: self.es_parent
+          )
         end
       end
 
@@ -55,11 +60,16 @@ module Elastic
       end
 
       def es_parent
-        project_id
+        project_id if respond_to?(:project_id)
       end
     end
 
     module ClassMethods
+      # Should be overridden for all nested models
+      def nested?
+        false
+      end
+
       def highlight_options(fields)
         es_fields = fields.map { |field| field.split('^').first }.inject({}) do |memo, field|
           memo[field.to_sym] = {}

app/models/concerns/elastic/issues_search.rb

@@ -33,6 +33,10 @@ module Elastic
         data
       end
 
+      def self.nested?
+        true
+      end
+
       def self.elastic_search(query, options: {})
         if query =~ /#(\d+)\z/
           query_hash = iid_query_hash(query_hash, $1)

app/models/concerns/elastic/merge_requests_search.rb

@@ -55,6 +55,10 @@ module Elastic
         target_project_id
       end
 
+      def self.nested?
+        true
+      end
+
       def self.elastic_search(query, options: {})
         if query =~ /#(\d+)\z/
           query_hash = iid_query_hash(query_hash, $1)

app/models/concerns/elastic/milestones_search.rb

@@ -22,6 +22,10 @@ module Elastic
         )
       end
 
+      def self.nested?
+        true
+      end
+
       def self.elastic_search(query, options: {})
         options[:in] = %w(title^2 description)
 

app/models/concerns/elastic/notes_search.rb

@@ -40,6 +40,10 @@ module Elastic
         data
       end
 
+      def self.nested?
+        true
+      end
+
       def self.elastic_search(query, options: {})
         options[:in] = ['note']
 

app/workers/elastic_indexer_worker.rb

@@ -14,15 +14,24 @@ class ElasticIndexerWorker
       record = klass.find(record_id)
       record.__elasticsearch__.client = client
 
-      if [Project, PersonalSnippet, ProjectSnippet, Snippet].include?(klass)
-        record.__elasticsearch__.__send__ "#{operation}_document"
-      else
+      if klass.nested?
         record.__elasticsearch__.__send__ "#{operation}_document", parent: record.es_parent
+      else
+        record.__elasticsearch__.__send__ "#{operation}_document"
       end
 
       update_issue_notes(record, options["changed_fields"]) if klass == Issue
     when /delete/
-      client.delete index: klass.index_name, type: klass.document_type, id: record_id
+      if klass.nested?
+        client.delete(
+          index: klass.index_name,
+          type: klass.document_type,
+          id: record_id,
+          parent: options["project_id"]
+        )
+      else
+        client.delete index: klass.index_name, type: klass.document_type, id: record_id
+      end
 
       clear_project_indexes(record_id) if klass == Project
     end
@@ -30,6 +39,8 @@ class ElasticIndexerWorker
     true # Less work to do!
   end
 
+  private
+
   def update_issue_notes(record, changed_fields)
     if changed_fields && (changed_fields & ISSUE_TRACKED_FIELDS).any?
       Note.import_with_parent query: -> { where(noteable: record) }
@@ -37,7 +48,12 @@ class ElasticIndexerWorker
   end
 
   def clear_project_indexes(record_id)
-    # Remove repository index
+    remove_repository_index(record_id)
+    remove_wiki_index(record_id)
+    remove_nested_content(record_id)
+  end
+
+  def remove_repository_index(record_id)
     client.delete_by_query({
       index: Repository.__elasticsearch__.index_name,
       body: {
@@ -49,8 +65,20 @@ class ElasticIndexerWorker
         }
       }
     })
+  end
+
+  def remove_nested_content(record_id)
+    client.delete_by_query({
+      index: Project.__elasticsearch__.index_name,
+      body: {
+        query: {
+          term: { "_parent" => record_id }
+        }
+      }
+    })
+  end
 
-    # Remove wiki index
+  def remove_wiki_index(record_id)
     client.delete_by_query({
       index: ProjectWiki.__elasticsearch__.index_name,
       body: {

spec/workers/elastic_indexer_worker_spec.rb

+require 'spec_helper'
+
+describe ElasticIndexerWorker, elastic: true do
+  include Gitlab::CurrentSettings
+  subject { described_class.new }
+
+  before do
+    Elasticsearch::Model.client = Elasticsearch::Client.new(
+      host: current_application_settings.elasticsearch_host,
+      port: current_application_settings.elasticsearch_port
+    )
+
+    Gitlab::Elastic::Helper.create_empty_index
+  end
+
+  after do
+    Gitlab::Elastic::Helper.delete_index
+  end
+
+  Sidekiq::Testing.disable! do
+    describe 'Indexing new records' do
+      it 'indexes a project' do
+        project = create :empty_project
+
+        expect do
+          subject.perform("index", "Project", project.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('*').records.size }.by(1)
+      end
+
+      it 'indexes an issue' do
+        issue = create :issue
+
+        expect do
+          subject.perform("index", "Issue", issue.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('*').records.size }.by(1)
+      end
+
+      it 'indexes a note' do
+        note = create :note
+
+        expect do
+          subject.perform("index", "Note", note.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('*').records.size }.by(1)
+      end
+
+      it 'indexes a milestone' do
+        milestone = create :milestone
+
+        expect do
+          subject.perform("index", "Milestone", milestone.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('*').records.size }.by(1)
+      end
+
+      it 'indexes a merge request' do
+        merge_request = create :merge_request
+
+        expect do
+          subject.perform("index", "MergeRequest", merge_request.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('*').records.size }.by(1)
+      end
+    end
+
+    describe 'Updating index' do
+      it 'updates a project' do
+        project = create :empty_project
+        subject.perform("index", "Project", project.id)
+        project.update(name: "new")
+
+        expect do
+          subject.perform("update", "Project", project.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('new').records.size }.by(1)
+      end
+
+      it 'updates an issue' do
+        issue = create :issue
+        subject.perform("index", "Issue", issue.id)
+        issue.update(title: "new")
+
+        expect do
+          subject.perform("update", "Issue", issue.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('new').records.size }.by(1)
+      end
+
+      it 'updates a note' do
+        note = create :note
+        subject.perform("index", "Note", note.id)
+        note.update(note: 'new')
+
+        expect do
+          subject.perform("update", "Note", note.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('new').records.size }.by(1)
+      end
+
+      it 'updates a milestone' do
+        milestone = create :milestone
+        subject.perform("index", "Milestone", milestone.id)
+        milestone.update(title: 'new')
+
+        expect do
+          subject.perform("update", "Milestone", milestone.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('new').records.size }.by(1)
+      end
+
+      it 'updates a merge request' do
+        merge_request = create :merge_request
+        subject.perform("index", "MergeRequest", merge_request.id)
+        merge_request.update(title: 'new')
+
+        expect do
+          subject.perform("index", "MergeRequest", merge_request.id)
+          Gitlab::Elastic::Helper.refresh_index
+        end.to change{ Elasticsearch::Model.search('new').records.size }.by(1)
+      end
+    end
+  end
+end