Merge branch 'get_rid_of_as_json_es_indexer' into 'master'

[ES] More accurate as_indexed_json

fixes https://gitlab.com/gitlab-org/gitlab-ee/issues/349

It makes database indexer stable to corrupted data in database.

/cc @jnijhof 

See merge request !244

CHANGELOG-EE

@@ -7,6 +7,7 @@ v 8.6.0 (unreleased)
   - [Elastic] Removing repository and wiki index after removing project
   - [Elastic] Update index on push to wiki
   - [Elastic] Use subprocesses for ElasticSearch index jobs
+  - [Elastic] More accurate as_indexed_json (More stable database indexer)
 
 v 8.5.4
   - [Elastic][Security] Notes exposure

lib/elastic/issues_search.rb

@@ -27,12 +27,18 @@ module Elastic
       end
 
       def as_indexed_json(options = {})
-        as_json(
-          include: {
-            project:  { only: :id },
-            author:   { only: :id }
-          }
-        ).merge({ updated_at_sort: updated_at })
+        data = {}
+
+        # We don't use as_json(only: ...) because it calls all virtual and serialized attributtes
+        # https://gitlab.com/gitlab-org/gitlab-ee/issues/349
+        [:id, :iid, :title, :description, :created_at, :updated_at, :state, :project_id, :author_id].each do |attr|
+          data[attr.to_s] = self.send(attr)
+        end
+
+        data['project'] = { 'id' => project.id }
+        data['author'] = { 'id' => author.id }
+        data['updated_at_sort'] = updated_at
+        data
       end
 
       def self.elastic_search(query, options: {})

lib/elastic/merge_requests_search.rb

@@ -34,28 +34,33 @@ module Elastic
       end
 
       def as_indexed_json(options = {})
-        as_json({
-          only: [
-            :id,
-            :iid,
-            :target_branch,
-            :source_branch,
-            :title,
-            :description,
-            :created_at,
-            :updated_at,
-            :state,
-            :merge_status,
-            :source_project_id,
-            :target_project_id,
-            :author_id
-          ],
-          include: {
-            source_project: { only: :id },
-            target_project: { only: :id },
-            author:         { only: :id }
-          }
-        }).merge({ updated_at_sort: updated_at })
+        # We don't use as_json(only: ...) because it calls all virtual and serialized attributtes
+        # https://gitlab.com/gitlab-org/gitlab-ee/issues/349
+        data = {}
+
+        [
+          :id,
+          :iid,
+          :target_branch,
+          :source_branch,
+          :title,
+          :description,
+          :created_at,
+          :updated_at,
+          :state,
+          :merge_status,
+          :source_project_id,
+          :target_project_id,
+          :author_id
+        ].each do |attr|
+          data[attr.to_s] = self.send(attr)
+        end
+
+        data['source_project'] = { 'id' => source_project_id }
+        data['target_project'] = { 'id' => target_project_id }
+        data['author'] = { 'id' => author.id }
+        data['updated_at_sort'] = updated_at
+        data
       end
 
       def self.elastic_search(query, options: {})

lib/elastic/notes_search.rb

@@ -16,9 +16,16 @@ module Elastic
       end
 
       def as_indexed_json(options = {})
-        as_json(
-          only: [:id, :note, :project_id, :created_at]
-        ).merge({ updated_at_sort: updated_at })
+        data = {}
+
+        # We don't use as_json(only: ...) because it calls all virtual and serialized attributtes
+        # https://gitlab.com/gitlab-org/gitlab-ee/issues/349
+        [:id, :note, :project_id, :created_at].each do |attr|
+          data[attr.to_s] = self.send(attr)
+        end
+
+        data['updated_at_sort'] = updated_at
+        data
       end
 
       def self.elastic_search(query, options: {})

lib/elastic/projects_search.rb

@@ -29,23 +29,27 @@ module Elastic
       end
 
       def as_indexed_json(options = {})
-        as_json({
-          only: [
-            :id,
-            :name,
-            :path,
-            :description,
-            :namespace_id,
-            :created_at,
-            :archived,
-            :visibility_level,
-            :last_activity_at,
-            :last_pushed_at
-          ]
-        }).merge({
-          name_with_namespace: name_with_namespace,
-          path_with_namespace: path_with_namespace
-        })
+        # We don't use as_json(only: ...) because it calls all virtual and serialized attributtes
+        # https://gitlab.com/gitlab-org/gitlab-ee/issues/349
+        data = {}
+
+        [
+          :id,
+          :name,
+          :path,
+          :description,
+          :namespace_id,
+          :created_at,
+          :archived,
+          :visibility_level,
+          :last_activity_at,
+          :name_with_namespace,
+          :path_with_namespace
+        ].each do |attr|
+          data[attr.to_s] = self.send(attr)
+        end
+
+        data
       end
 
       def self.elastic_search(query, options: {})

spec/lib/elastic/issue_spec.rb

@@ -27,4 +27,18 @@ describe "Issue", elastic: true do
 
     expect(Issue.elastic_search('term', options: options).total_count).to eq(2)
   end
+
+  it "returns json with all needed elements" do
+    project = create :empty_project
+    issue = create :issue, project: project
+
+    expected_hash =  issue.attributes.extract!('id', 'iid', 'title', 'description', 'created_at',
+                                                'updated_at', 'state', 'project_id', 'author_id')
+
+    expected_hash['project'] = { "id" => project.id }
+    expected_hash['author'] = { "id" => issue.author_id }
+    expected_hash['updated_at_sort'] = issue.updated_at
+
+    expect(issue.as_indexed_json).to eq(expected_hash)
+  end
 end

spec/lib/elastic/merge_request_spec.rb

@@ -27,4 +27,31 @@ describe "MergeRequest", elastic: true do
 
     expect(MergeRequest.elastic_search('term', options: options).total_count).to eq(2)
   end
+
+  it "returns json with all needed elements" do
+    merge_request = create :merge_request
+
+    expected_hash =  merge_request.attributes.extract!(
+      'id',
+      'iid',
+      'target_branch',
+      'source_branch',
+      'title',
+      'description',
+      'created_at',
+      'updated_at',
+      'state',
+      'merge_status',
+      'source_project_id',
+      'target_project_id',
+      'author_id'
+    )
+
+    expected_hash['source_project'] = { 'id' => merge_request.source_project_id }
+    expected_hash['target_project'] = { 'id' => merge_request.target_project_id }
+    expected_hash['author']         = { 'id' => merge_request.author.id }
+    expected_hash['updated_at_sort'] = merge_request.updated_at
+
+    expect(merge_request.as_indexed_json).to eq(expected_hash)
+  end
 end

spec/lib/elastic/milestone_spec.rb

@@ -27,4 +27,20 @@ describe "Milestone", elastic: true do
 
     expect(Milestone.elastic_search('term', options: options).total_count).to eq(2)
   end
+
+  it "returns json with all needed elements" do
+    milestone = create :milestone
+
+    expected_hash =  milestone.attributes.extract!(
+      'id',
+      'title',
+      'description',
+      'project_id',
+      'created_at'
+    )
+
+    expected_hash[:updated_at_sort] = milestone.updated_at
+
+    expect(milestone.as_indexed_json).to eq(expected_hash)
+  end
 end

spec/lib/elastic/note_spec.rb

@@ -26,4 +26,19 @@ describe "Note", elastic: true do
 
     expect(Note.elastic_search('term', options: options).total_count).to eq(1)
   end
+
+  it "returns json with all needed elements" do
+    note = create :note
+
+    expected_hash =  note.attributes.extract!(
+      'id',
+      'note',
+      'project_id',
+      'created_at'
+    )
+
+    expected_hash['updated_at_sort'] = note.updated_at
+
+    expect(note.as_indexed_json).to eq(expected_hash)
+  end
 end

spec/lib/elastic/project_spec.rb

@@ -24,4 +24,25 @@ describe "Projects", elastic: true do
     expect(Project.elastic_search('test1', options: { pids: @project_ids }).total_count).to eq(1)
     expect(Project.elastic_search('someone_elses_project', options: { pids: @project_ids }).total_count).to eq(0)
   end
+
+  it "returns json with all needed elements" do
+    project = create :project
+
+    expected_hash = project.attributes.extract!(
+      'id',
+      'name',
+      'path',
+      'description',
+      'namespace_id',
+      'created_at',
+      'archived',
+      'visibility_level',
+      'last_activity_at'
+    )
+
+    expected_hash['name_with_namespace'] = project.name_with_namespace
+    expected_hash['path_with_namespace'] = project.path_with_namespace
+
+    expect(project.as_indexed_json).to eq(expected_hash)
+  end
 end

spec/lib/elastic/snippet_spec.rb

@@ -44,4 +44,25 @@ describe "Snippet", elastic: true do
     expect(Snippet.elastic_search('home', options: options).total_count).to eq(1)
     expect(Snippet.elastic_search('index.php', options:  options).total_count).to eq(1)
   end
+
+  it "returns json with all needed elements" do
+    snippet = create :project_snippet
+
+    expected_hash =  snippet.attributes.extract!(
+      'id',
+      'title',
+      'file_name',
+      'content',
+      'created_at',
+      'updated_at',
+      'state',
+      'project_id',
+      'author_id',
+    )
+
+    expected_hash['project'] = { 'id' => snippet.project.id }
+    expected_hash['author'] = { 'id' => snippet.author.id }
+
+    expect(snippet.as_indexed_json).to eq(expected_hash)
+  end
 end