Commit 8f5cdb12 authored by Dylan Griffith's avatar Dylan Griffith Committed by Arturo Herrero

Delete any note from the index if noteable is nil

Prior to this we were specifically targetting missing commits because
this was a case we [observed in
staging](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55878)
but we also may have more cases we haven't run into yet for other
noteable types. The `noteable_id` column on the table does not have a
`NOT NULL` constraint nor does it have foreign key references so it's
totally conceivable there are other kinds of orphaned notes (eg. issues
that were hard deleted at some point) that we don't know about. I have
concerns that any one of these situations could again lead to the
migration to update permissions getting stuck forever. As such I think
the best course of action is to delete any of these. I also can't think
of any good reason to keep the orphaned notes in the index so this seems
quite safe.
parent 22669300
......@@ -6,10 +6,11 @@ module Elastic
delegate :noteable, :noteable_type, to: :target
def as_indexed_json(options = {})
# Notes on commits should return the commit object when `notable` is called. However, `noteable` can be null
# when a commit has been deleted so an error is raised to alert the caller that the document should be deleted
# from the index.
raise Elastic::Latest::DocumentShouldBeDeletedFromIndexError.new(target.class.name, target.id) if noteable_type == 'Commit' && noteable.nil?
# `noteable` can be sometimes be nil (eg. when a commit has been
# deleted) or somehow it was left orphaned in the database. In such
# cases we want to delete it from the index since there is no value in
# having orphaned notes be searchable.
raise Elastic::Latest::DocumentShouldBeDeletedFromIndexError.new(target.class.name, target.id) if noteable.nil?
data = {}
......
......@@ -121,8 +121,8 @@ RSpec.describe Note, :elastic do
expect(note.__elasticsearch__.as_indexed_json).to eq(expected_hash)
end
it 'does raises Elastic::Latest::DocumentShouldBeDeletedFromIndexError when commit is not found and noteable is null' do
note = create(:note_on_commit)
it 'raises Elastic::Latest::DocumentShouldBeDeletedFromIndexError when noteable is nil' do
note = create(:note_on_issue)
allow(note).to receive(:noteable).and_return(nil)
expect { note.__elasticsearch__.as_indexed_json }.to raise_error(::Elastic::Latest::DocumentShouldBeDeletedFromIndexError)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment