Merge branch '1840-robustify-es-db-indexing' into 'master'

Resolve "Elasticsearch project indexing fails on bad data"

Closes #1840

See merge request !1365

app/models/concerns/elastic/application_search.rb

@@ -74,6 +74,16 @@ module Elastic
       def es_parent
         project_id if respond_to?(:project_id)
       end
+
+      # Some attributes are actually complicated methods. Bad data can cause
+      # them to raise exceptions. When this happens, we still want the remainder
+      # of the object to be saved, so silently swallow the errors
+      def safely_read_attribute_for_elasticsearch(attr_name)
+        send(attr_name)
+      rescue => err
+        logger.warn("Elasticsearch failed to read #{attr_name} for #{self.class} #{self.id}: #{err}")
+        nil
+      end
     end
 
     module ClassMethods

app/models/concerns/elastic/issues_search.rb

@@ -27,7 +27,7 @@ module Elastic
         # 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, :assignee_id, :confidential].each do |attr|
-          data[attr.to_s] = self.send(attr)
+          data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
         end
 
         data

app/models/concerns/elastic/merge_requests_search.rb

@@ -45,7 +45,7 @@ module Elastic
           :target_project_id,
           :author_id
         ].each do |attr|
-          data[attr.to_s] = self.send(attr)
+          data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
         end
 
         data

app/models/concerns/elastic/milestones_search.rb

@@ -17,9 +17,15 @@ module Elastic
       end
 
       def as_indexed_json(options = {})
-        as_json(
-          only: [:id, :title, :description, :project_id, :created_at, :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, :title, :description, :project_id, :created_at, :updated_at].each do |attr|
+          data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
+        end
+
+        data
       end
 
       def self.nested?

app/models/concerns/elastic/notes_search.rb

@@ -26,7 +26,7 @@ module Elastic
         # 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, :updated_at].each do |attr|
-          data[attr.to_s] = self.send(attr)
+          data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
         end
 
         if noteable.is_a?(Issue)

app/models/concerns/elastic/projects_search.rb

@@ -46,7 +46,7 @@ module Elastic
           :name_with_namespace,
           :path_with_namespace
         ].each do |attr|
-          data[attr.to_s] = self.send(attr)
+          data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
         end
 
         data

app/models/concerns/elastic/snippets_search.rb

@@ -22,20 +22,25 @@ module Elastic
       end
 
       def as_indexed_json(options = {})
-        as_json({
-          only: [
-            :id,
-            :title,
-            :file_name,
-            :content,
-            :created_at,
-            :updated_at,
-            :state,
-            :project_id,
-            :author_id,
-            :visibility_level
-          ]
-        })
+        # 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,
+          :title,
+          :file_name,
+          :content,
+          :created_at,
+          :updated_at,
+          :project_id,
+          :author_id,
+          :visibility_level
+        ].each do |attr|
+          data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
+        end
+
+        data
       end
 
       def self.elastic_search(query, options: {})

changelogs/unreleased-ee/1840-robustify-es-db-indexing.yml

+---
+title: Robustify reading attributes for elasticsearch
+merge_request: 1365
+author: