commit c3e0717a03e3682495807fb42168d8f3de767ed6
author Dave Borowitz <dborowitz@google.com> Thu Apr 07 18:18:25 2016 -0400
committer Dave Borowitz <dborowitz@google.com> Sun Apr 10 21:51:30 2016 -0700
tree 09cbb050ad6ebbc9afac539fbdb75e1efd40fa52
parent 2b30b6241b8417a9a7e96d17e34593964ca3ebe1

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