Files
gerrit/gerrit-acceptance-tests
Dave Borowitz c3e0717a03 NoteDb: Avoid no-op commits when touching published comments
The implementation of ChangeUpdate eagerly creates a ChangeDraftUpdate
any time a published comment was added, for the purpose of deleting
the corresponding draft if present. NoteDbUpdateManager would happily
add and execute this ChangeDraftUpdate if it exists, even though it
was really a no-op.

This was not triggering ChangeDraftUpdate#isEmpty(), because there was
actually a key in the set of comments to delete. In fact isEmpty()
cannot actually know whether the update is empty, because it doesn't
have access to the original RevisionNotes to know whether the delete
set refers to any comments that actually exist.

Tweak the AbstractChangeUpdate interface to allow applyImpl to specify
that the update was really a no-op. Use a special sentinel return
value for this purpose; it's ugly, but restricted to the
tightly-coupled AbstractChangeUpdate hierarchy. Determine whether a
change is a no-op in ChangeDraftUpdate by checking the literal byte
array output in each of the notes.

Add regression tests to ChangeNotesTest for this case. Also add some
tests to ChangeRebuilderIT that touch this codepath, although
NoteDbChecker wouldn't actually flag the no-op commits.

Fix some bugs exposed in the ChangDraftUpdate pipeline by the new
codepaths and tests, mostly introduced by acda8b37. Use the correct
cached RevisionNoteMap object. Don't mutate comments from within
PutDraftComment, as those instances originally came from and would be
shared with the underlying ChangeNotes; make defensive copies
instead. Don't double-serialize the old version of an updated note
along with the updated version.

Change-Id: I50a42a4e8edc3390a02dc7e658ce6beef966e24e
2016-04-10 21:51:30 -07:00
..