Merge changes from topic 'diff-summary-cache-2' * changes: Change indexing: avoid computing current file paths too early Store insertions/deletions in DiffSummary cache
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentTimestampAdapter.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentTimestampAdapter.java index 29c2077..2f47107 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentTimestampAdapter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentTimestampAdapter.java
@@ -40,6 +40,10 @@ * * <p>This adapter is mutually compatible with the old adapter: the old adapter is able to read the * UTC instant format, and this adapter can fall back to parsing the old format. + * + * <p>Older Gson versions are not able to parse milliseconds out of ISO 8601 instants, so this + * implementation truncates to seconds when writing. This is no worse than the truncation that + * happens to fit NoteDb timestamps into git commit formatting. */ class CommentTimestampAdapter extends TypeAdapter<Timestamp> { private static final DateTimeFormatter FALLBACK = @@ -47,7 +51,8 @@ @Override public void write(JsonWriter out, Timestamp ts) throws IOException { - out.value(ISO_INSTANT.format(ts.toInstant())); + Timestamp truncated = new Timestamp(ts.getTime() / 1000 * 1000); + out.value(ISO_INSTANT.format(truncated.toInstant())); } @Override
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java index aa3dfc4..aa37d51 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
@@ -35,9 +35,16 @@ /** Arbitrary time outside of a DST transition, as a reasonable Java 8 representation. */ private static final ZonedDateTime NON_DST = ZonedDateTime.parse(NON_DST_STR); + /** {@link #NON_DST_STR} truncated to seconds. */ + private static final String NON_DST_STR_TRUNC = "2017-02-07T10:20:30Z"; + /** Arbitrary time outside of a DST transition, as an unreasonable Timestamp representation. */ private static final Timestamp NON_DST_TS = Timestamp.from(NON_DST.toInstant()); + /** {@link #NON_DST_TS} truncated to seconds. */ + private static final Timestamp NON_DST_TS_TRUNC = + Timestamp.from(ZonedDateTime.parse(NON_DST_STR_TRUNC).toInstant()); + /** * Real live ms since epoch timestamp of a comment that was posted during the PDT to PST * transition in November 2013. @@ -100,8 +107,9 @@ @Test public void legacyAdapterCanParseOutputOfNewAdapter() { String instantJson = gson.toJson(NON_DST_TS); - assertThat(instantJson).isEqualTo('"' + NON_DST_STR + '"'); - assertThat(legacyGson.fromJson(instantJson, Timestamp.class)).isEqualTo(NON_DST_TS); + assertThat(instantJson).isEqualTo('"' + NON_DST_STR_TRUNC + '"'); + Timestamp result = legacyGson.fromJson(instantJson, Timestamp.class); + assertThat(result).isEqualTo(NON_DST_TS_TRUNC); } @Test @@ -128,8 +136,9 @@ @Test public void newAdapterRoundTrip() { String json = gson.toJson(NON_DST_TS); - assertThat(json).isEqualTo('"' + NON_DST_STR + '"'); - assertThat(gson.fromJson(json, Timestamp.class)).isEqualTo(NON_DST_TS); + // Round-trip lossily truncates ms, but that's ok. + assertThat(json).isEqualTo('"' + NON_DST_STR_TRUNC + '"'); + assertThat(gson.fromJson(json, Timestamp.class)).isEqualTo(NON_DST_TS_TRUNC); } @Test @@ -153,7 +162,12 @@ c.revId = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; String json = gson.toJson(c); - assertThat(json).contains("\"writtenOn\": \"" + NON_DST_STR + "\","); - assertThat(gson.fromJson(json, Comment.class)).isEqualTo(c); + assertThat(json).contains("\"writtenOn\": \"" + NON_DST_STR_TRUNC + "\","); + + Comment result = gson.fromJson(json, Comment.class); + // Round-trip lossily truncates ms, but that's ok. + assertThat(result.writtenOn).isEqualTo(NON_DST_TS_TRUNC); + result.writtenOn = NON_DST_TS; + assertThat(result).isEqualTo(c); } }
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js index e33458f..fbbaadb 100644 --- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js +++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -163,13 +163,14 @@ } var messages = this.messages || []; var index = message._index; - var authorId = message.author._account_id; + var authorId = message.author && message.author._account_id; var mDate = util.parseDate(message.date).getTime(); // NB: Messages array has oldest messages first. var nextMDate; if (index > 0) { for (var i = index - 1; i >= 0; i--) { - if (messages[i].author._account_id === authorId) { + if (messages[i] && messages[i].author && + messages[i].author._account_id === authorId) { nextMDate = util.parseDate(messages[i].date).getTime(); break; } @@ -179,7 +180,8 @@ for (var file in comments) { var fileComments = comments[file]; for (var i = 0; i < fileComments.length; i++) { - if (fileComments[i].author._account_id !== authorId) { + if (fileComments[i].author && + fileComments[i].author._account_id !== authorId) { continue; } var cDate = util.parseDate(fileComments[i].updated).getTime();
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html index 770da61..4c6ff36 100644 --- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
@@ -231,6 +231,37 @@ comments.file2.filter(isMarvin)); assert.deepEqual(messageElements[2].comments, {}); }); + + test('messages without author do not throw', function() { + var comments = { + file1: [ + { + message: 'message text', + updated: '2016-09-27 00:18:03.000000000', + in_reply_to: '6505d749_f0bec0aa', + line: 62, + id: '6505d749_10ed44b2', + patch_set: 2, + author: { + email: 'some@email.com', + _account_id: 123, + }, + }, + ]}; + var messages = [{ + _index: 5, + _revision_number: 4, + message: 'Uploaded patch set 4.', + date: '2016-09-28 13:36:33.000000000', + id: '8c19ccc949c6d482b061be6a28e10782abf0e7af', + }]; + element.messages = messages; + element.comments = comments + flushAsynchronousOperations(); + var messageEls = Polymer.dom(element.root).querySelectorAll('gr-message'); + assert.equal(messageEls.length, 1); + assert.equal(messageEls[0].message.message, messages[0].message); + }); }); suite('gr-messages-list automate tests', function() { @@ -311,5 +342,6 @@ //Autogenerated messages are now hidden. assert.isFalse(!!allHiddenMessageEls.length); }); - }); +}); + </script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js index 681e8a8..7d91e91 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -84,7 +84,8 @@ lastComment.id || lastComment.__draftID); commentEl.editing = true; } else { - this.addDraft(opt_lineNum, lastComment.range); + this.addDraft(opt_lineNum, + lastComment ? lastComment.range : undefined); } },
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html index 41c160f..ef4ab14 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -40,14 +40,20 @@ <script> suite('gr-diff-comment-thread tests', function() { var element; + var sandbox; setup(function() { stub('gr-rest-api-interface', { getLoggedIn: function() { return Promise.resolve(false); }, }); + sandbox = sinon.sandbox.create(); element = fixture('basic'); }); + teardown(function() { + sandbox.restore(); + }); + test('comments are sorted correctly', function() { var comments = [ { @@ -111,6 +117,36 @@ } ]); }); + + test('addOrEditDraft w/ edit draft', function() { + element.comments = [{ + id: 'jacks_reply', + message: 'i like you, too', + in_reply_to: 'sallys_confession', + updated: '2015-12-25 15:00:20.396000000', + __draft: true, + }]; + var commentElStub = sandbox.stub(element, '_commentElWithDraftID', + function() { return {}; }); + var addDraftStub = sandbox.stub(element, 'addDraft'); + + element.addOrEditDraft(123); + + assert.isTrue(commentElStub.called); + assert.isFalse(addDraftStub.called); + }); + + test('addOrEditDraft w/o edit draft', function() { + element.comments = []; + var commentElStub = sandbox.stub(element, '_commentElWithDraftID', + function() { return {}; }); + var addDraftStub = sandbox.stub(element, 'addDraft'); + + element.addOrEditDraft(123); + + assert.isFalse(commentElStub.called); + assert.isTrue(addDraftStub.called); + }); }); suite('comment action tests', function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js index 6d6227e..bc12b8f 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -372,7 +372,7 @@ _handleCancel: function(e) { e.preventDefault(); - if (this.comment.message === null || + if (!this.comment.message || this.comment.message.trim().length === 0) { this._fireDiscard(); return;