opendev/gerrit
Code Issues Proposed changes

Merge changes Ibe956014,I4222b894,Iac4ea144,I803717e8,I39e97dcd, ...

Browse Source 
* changes:
  Support updating multiple VersionedMetaDatas in a BatchRefUpdate
  Refactor byChange() into two methods: drafts and published
  Add draft comments to PatchLineCommentsUtil
  Fix bug for comments with no range
  Resolve issue with naming of drafts ref
  Add method to parse an AccountId out of a RefName
  Improve PatchSet.Id.fromRef performance and avoid double-parsing
This commit is contained in:
Dave Borowitz
2014-08-07 22:28:16 +00:00
committed by Gerrit Code Review
parent dc9c0d03ff 55c93c991c
commit d1feb48787
39 changed files with 1270 additions and 221 deletions
Download Patch File Download Diff File Expand all files Collapse all files

198
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java Normal file
View File

@@ -0,0 +1,198 @@
// Copyright (C) 2014 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.acceptance.server.change;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.Comment;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gson.reflect.TypeToken;
import org.apache.http.HttpStatus;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
import java.io.IOException;
import java.lang.reflect.Type;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class CommentsIT extends AbstractDaemonTest {
@ConfigSuite.Config
public static Config noteDbEnabled() {
Config cfg = new Config();
cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "comments", "read", true);
return cfg;
}
@Test
public void createDraft() throws GitAPIException, IOException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
addDraft(changeId, revId, comment);
Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
assertEquals(1, result.size());
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
}
@Test
public void postComment() throws RestApiException, Exception {
String file = "file";
String contents = "contents";
PushOneCommit push = pushFactory.create(db, admin.getIdent(),
"first subject", file, contents);
PushOneCommit.Result r = push.to(git, "refs/for/master");
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput input = new ReviewInput();
ReviewInput.CommentInput comment = newCommentInfo(
file, Comment.Side.REVISION, 1, "comment 1");
input.comments = new HashMap<String, List<ReviewInput.CommentInput>>();
input.comments.put(comment.path, Lists.newArrayList(comment));
revision(r).review(input);
Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
assertTrue(!result.isEmpty());
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
}
@Test
public void putDraft() throws GitAPIException, IOException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
addDraft(changeId, revId, comment);
Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
String uuid = actual.id;
comment.message = "updated comment 1";
updateDraft(changeId, revId, comment, uuid);
result = getDraftComments(changeId, revId);
actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
}
@Test
public void getDraft() throws GitAPIException, IOException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
CommentInfo returned = addDraft(changeId, revId, comment);
CommentInfo actual = getDraftComment(changeId, revId, returned.id);
assertCommentInfo(comment, actual);
}
@Test
public void deleteDraft() throws IOException, GitAPIException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
CommentInfo returned = addDraft(changeId, revId, comment);
deleteDraft(changeId, revId, returned.id);
Map<String, List<CommentInfo>> drafts = getDraftComments(changeId, revId);
assertTrue(drafts.isEmpty());
}
private CommentInfo addDraft(String changeId, String revId,
ReviewInput.CommentInput c) throws IOException {
RestResponse r = userSession.put(
"/changes/" + changeId + "/revisions/" + revId + "/drafts", c);
assertEquals(HttpStatus.SC_CREATED, r.getStatusCode());
return newGson().fromJson(r.getReader(), CommentInfo.class);
}
private void updateDraft(String changeId, String revId,
ReviewInput.CommentInput c, String uuid) throws IOException {
RestResponse r = userSession.put(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid, c);
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
}
private void deleteDraft(String changeId, String revId, String uuid)
throws IOException {
RestResponse r = userSession.delete(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid);
assertEquals(HttpStatus.SC_NO_CONTENT, r.getStatusCode());
}
private Map<String, List<CommentInfo>> getPublishedComments(String changeId,
String revId) throws IOException {
RestResponse r = userSession.get(
"/changes/" + changeId + "/revisions/" + revId + "/comments/");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Type mapType = new TypeToken<Map<String, List<CommentInfo>>>() {}.getType();
return newGson().fromJson(r.getReader(), mapType);
}
private Map<String, List<CommentInfo>> getDraftComments(String changeId,
String revId) throws IOException {
RestResponse r = userSession.get(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Type mapType = new TypeToken<Map<String, List<CommentInfo>>>() {}.getType();
return newGson().fromJson(r.getReader(), mapType);
}
private CommentInfo getDraftComment(String changeId, String revId,
String uuid) throws IOException {
RestResponse r = userSession.get(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid);
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
return newGson().fromJson(r.getReader(), CommentInfo.class);
}
private static void assertCommentInfo(ReviewInput.CommentInput expected,
CommentInfo actual) {
assertEquals(expected.line, actual.line);
assertEquals(expected.message, actual.message);
assertEquals(expected.inReplyTo, actual.inReplyTo);
if (actual.side == null) {
assertEquals(expected.side, Comment.Side.REVISION);
}
}
private ReviewInput.CommentInput newCommentInfo(String path,
Comment.Side side, int line, String message) {
ReviewInput.CommentInput input = new ReviewInput.CommentInput();
input.path = path;
input.side = side;
input.line = line;
input.message = message;
return input;
}
}

3
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
View File

@@ -170,7 +170,8 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
// quickly locate where they have pending drafts, and review them.
//
final Account.Id me = ((IdentifiedUser) user).getAccountId();
for (final PatchLineComment c : db.patchComments().draftByPatchSetAuthor(psIdNew, me)) {
for (PatchLineComment c
: plcUtil.draftByPatchSetAuthor(db, psIdNew, me, notes)) {
final Patch p = byKey.get(c.getKey().getParentKey());
if (p != null) {
p.setDraftCount(p.getDraftCount() + 1);

13
gerrit-reviewdb/BUCK
View File

@@ -1,4 +1,5 @@
SRC = 'src/main/java/com/google/gerrit/reviewdb/'
TESTS = 'src/test/java/com/google/gerrit/reviewdb/'
gwt_module(
name = 'client',
@@ -22,3 +23,15 @@ java_library(
],
visibility = ['PUBLIC'],
)
java_test(
name = 'client_tests',
srcs = glob([TESTS + 'client/**/*.java']),
deps = [
':client',
'//lib:gwtorm',
'//lib:junit',
],
source_under_test = [':client'],
visibility = ['//tools/eclipse:classpath'],
)

49
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
View File

@@ -104,6 +104,55 @@ public final class Account {
r.fromString(str);
return r;
}
/**
* Parse an Account.Id out of a part of a ref-name.
*
* @param name a ref name with the following syntax: {@code "34/1234..."}.
* We assume that the caller has trimmed any prefix.
*/
public static Id fromRefPart(String name) {
if (name == null) {
return null;
}
String[] parts = name.split("/");
int n = parts.length;
if (n < 2) {
return null;
}
// Last 2 digits.
int le;
for (le = 0; le < parts[0].length(); le++) {
if (!Character.isDigit(parts[0].charAt(le))) {
return null;
}
}
if (le != 2) {
return null;
}
// Full ID.
int ie;
for (ie = 0; ie < parts[1].length(); ie++) {
if (!Character.isDigit(parts[1].charAt(ie))) {
if (ie == 0) {
return null;
} else {
break;
}
}
}
int shard = Integer.parseInt(parts[0]);
int id = Integer.parseInt(parts[1].substring(0, ie));
if (id % 100 != shard) {
return null;
}
return new Account.Id(id);
}
}
@Column(id = 1)

87
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java
View File

@@ -24,28 +24,8 @@ import java.sql.Timestamp;
/** A single revision of a {@link Change}. */
public final class PatchSet {
/** Is the reference name a change reference? */
public static boolean isRef(final String name) {
if (name == null || !name.startsWith(REFS_CHANGES)) {
return false;
}
boolean accepted = false;
int numsFound = 0;
for (int i = name.length() - 1; i >= REFS_CHANGES.length() - 1; i--) {
char c = name.charAt(i);
if (c >= '0' && c <= '9') {
accepted = (c != '0');
} else if (c == '/') {
if (accepted) {
if (++numsFound == 2) {
return true;
}
accepted = false;
}
} else {
return false;
}
}
return false;
public static boolean isRef(String name) {
return Id.fromRef(name) != null;
}
public static class Id extends IntKey<Change.Id> {
@@ -106,16 +86,63 @@ public final class PatchSet {
/** Parse a PatchSet.Id from a {@link PatchSet#getRefName()} result. */
public static Id fromRef(String name) {
if (!name.startsWith(REFS_CHANGES)) {
throw new IllegalArgumentException("Not a PatchSet.Id: " + name);
if (name == null || !name.startsWith(REFS_CHANGES)) {
return null;
}
final String[] parts = name.substring(REFS_CHANGES.length()).split("/");
final int n = parts.length;
if (n < 2) {
throw new IllegalArgumentException("Not a PatchSet.Id: " + name);
// Last 2 digits.
int ls = REFS_CHANGES.length();
int le;
for (le = ls; le < name.length() && name.charAt(le) != '/'; le++) {
if (name.charAt(le) < '0' || name.charAt(le) > '9') {
return null;
}
final int changeId = Integer.parseInt(parts[n - 2]);
final int patchSetId = Integer.parseInt(parts[n - 1]);
}
if (le - ls != 2) {
return null;
}
// Change ID.
int cs = le + 1;
if (cs >= name.length() || name.charAt(cs) == '0') {
return null;
}
int ce;
for (ce = cs; ce < name.length() && name.charAt(ce) != '/'; ce++) {
if (name.charAt(ce) < '0' || name.charAt(ce) > '9') {
return null;
}
}
switch (ce - cs) {
case 0:
return null;
case 1:
if (name.charAt(ls) != '0'
|| name.charAt(ls + 1) != name.charAt(cs)) {
return null;
}
break;
default:
if (name.charAt(ls) != name.charAt(ce - 2)
|| name.charAt(ls + 1) != name.charAt(ce - 1)) {
return null;
}
break;
}
// Patch set ID.
int ps = ce + 1;
if (ps >= name.length() || name.charAt(ps) == '0') {
return null;
}
for (int i = ps; i < name.length(); i++) {
if (name.charAt(i) < '0' || name.charAt(i) > '9') {
return null;
}
}
int changeId = Integer.parseInt(name.substring(cs, ce));
int patchSetId = Integer.parseInt(name.substring(ps));
return new PatchSet.Id(new Change.Id(changeId), patchSetId);
}
}

14
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
View File

@@ -31,6 +31,8 @@ public class RefNames {
/** Configurations of project-specific dashboards (canned search queries). */
public static final String REFS_DASHBOARDS = "refs/meta/dashboards/";
public static final String REFS_DRAFT_COMMENTS = "refs/draft-comments/";
/**
* Prefix applied to merge commit base nodes.
* <p>
@@ -43,8 +45,6 @@ public class RefNames {
*/
public static final String REFS_CACHE_AUTOMERGE = "refs/cache-automerge/";
public static final String REFS_DRAFT_PREFIX = "comments-";
public static String refsUsers(Account.Id accountId) {
StringBuilder r = new StringBuilder();
r.append(REFS_USER);
@@ -62,9 +62,15 @@ public class RefNames {
public static String refsDraftComments(Account.Id accountId,
Change.Id changeId) {
StringBuilder r = new StringBuilder();
r.append(refsUsers(accountId));
r.append(REFS_DRAFT_COMMENTS);
int n = accountId.get() % 100;
if (n < 10) {
r.append('0');
}
r.append(n);
r.append('/');
r.append(REFS_DRAFT_PREFIX);
r.append(accountId.get());
r.append('-');
r.append(changeId.get());
return r.toString();
}

55
gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java Normal file
View File

@@ -0,0 +1,55 @@
// Copyright (C) 2014 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.reviewdb.client;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import com.google.gerrit.reviewdb.client.Account;
import org.junit.Test;
public class AccountTest {
@Test
public void parseRefNameParts() {
assertRefPart(1, "01/1");
assertRefPart(1, "01/1-drafts");
assertRefPart(1, "01/1-drafts/2");
assertNotRefPart(null);
assertNotRefPart("");
// This method assumes that the common prefix "refs/users/" will be removed.
assertNotRefPart("refs/users/01/1");
// Invalid characters.
assertNotRefPart("01a/1");
assertNotRefPart("01/a1");
// Mismatched shard.
assertNotRefPart("01/23");
// Shard too short.
assertNotRefPart("1/1");
}
private static void assertRefPart(int accountId, String refName) {
assertEquals(new Account.Id(accountId), Account.Id.fromRefPart(refName));
}
private static void assertNotRefPart(String refName) {
assertNull(Account.Id.fromRefPart(refName));
}
}

69
gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetTest.java Normal file
View File

@@ -0,0 +1,69 @@
// Copyright (C) 2014 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.reviewdb.client;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
public class PatchSetTest {
@Test
public void parseRefNames() {
assertRef(1, 1, "refs/changes/01/1/1");
assertRef(1234, 56, "refs/changes/34/1234/56");
// Not even close.
assertNotRef(null);
assertNotRef("");
assertNotRef("01/1/1");
assertNotRef("HEAD");
assertNotRef("refs/tags/v1");
// Invalid characters.
assertNotRef("refs/changes/0x/1/1");
assertNotRef("refs/changes/01/x/1");
assertNotRef("refs/changes/01/1/x");
// Truncations.
assertNotRef("refs/changes/");
assertNotRef("refs/changes/1");
assertNotRef("refs/changes/01");
assertNotRef("refs/changes/01/");
assertNotRef("refs/changes/01/1/");
assertNotRef("refs/changes/01/1/1/");
assertNotRef("refs/changes/01//1/1");
// Leading zeroes.
assertNotRef("refs/changes/01/01/1");
assertNotRef("refs/changes/01/1/01");
// Mismatched last 2 digits.
assertNotRef("refs/changes/35/1234/56");
}
private static void assertRef(int changeId, int psId, String refName) {
assertTrue(PatchSet.isRef(refName));
assertEquals(new PatchSet.Id(new Change.Id(changeId), psId),
PatchSet.Id.fromRef(refName));
}
private static void assertNotRef(String refName) {
assertFalse(PatchSet.isRef(refName));
assertNull(PatchSet.Id.fromRef(refName));
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
View File

@@ -508,8 +508,8 @@ public class ChangeUtil {
ReviewDb db = this.db.get();
db.accountPatchReviews().delete(db.accountPatchReviews().byPatchSet(patchSetId));
db.changeMessages().delete(db.changeMessages().byPatchSet(patchSetId));
db.patchComments().delete(db.patchComments().byPatchSet(patchSetId));
// No need to delete from notedb; draft patch sets will be filtered out.
db.patchComments().delete(db.patchComments().byPatchSet(patchSetId));
db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(patchSetId));
db.patchSetAncestors().delete(db.patchSetAncestors().byPatchSet(patchSetId));

281
gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java
View File

@@ -12,25 +12,46 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.DraftCommentNotes;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
/**
* Utility functions to manipulate PatchLineComments.
@@ -40,45 +61,208 @@ import java.util.List;
*/
@Singleton
public class PatchLineCommentsUtil {
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final DraftCommentNotes.Factory draftFactory;
private final NotesMigration migration;
@VisibleForTesting
@Inject
public PatchLineCommentsUtil(NotesMigration migration) {
public PatchLineCommentsUtil(GitRepositoryManager repoManager,
AllUsersNameProvider allUsersProvider,
DraftCommentNotes.Factory draftFactory,
NotesMigration migration) {
this.repoManager = repoManager;
this.allUsers = allUsersProvider.get();
this.draftFactory = draftFactory;
this.migration = migration;
}
public Optional<PatchLineComment> get(ReviewDb db, ChangeNotes notes,
PatchLineComment.Key key) throws OrmException {
if (!migration.readComments()) {
return Optional.fromNullable(db.patchComments().get(key));
}
for (PatchLineComment c : publishedByChange(db, notes)) {
if (key.equals(c.getKey())) {
return Optional.of(c);
}
}
for (PatchLineComment c : draftByChange(db, notes)) {
if (key.equals(c.getKey())) {
return Optional.of(c);
}
}
return Optional.absent();
}
public List<PatchLineComment> publishedByChange(ReviewDb db,
ChangeNotes notes) throws OrmException {
if (!migration.readComments()) {
return byCommentStatus(db.patchComments().byChange(notes.getChangeId()),
Status.PUBLISHED);
}
notes.load();
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(notes.getBaseComments().values());
comments.addAll(notes.getPatchSetComments().values());
return comments;
}
public List<PatchLineComment> draftByChange(ReviewDb db,
ChangeNotes notes) throws OrmException {
if (!migration.readComments()) {
return byCommentStatus(db.patchComments().byChange(notes.getChangeId()),
Status.DRAFT);
}
List<PatchLineComment> comments = Lists.newArrayList();
Iterable<String> filtered = getDraftRefs(notes.getChangeId());
for (String refName : filtered) {
Account.Id account = Account.Id.fromRefPart(refName);
if (account != null) {
comments.addAll(draftByChangeAuthor(db, notes, account));
}
}
return comments;
}
private static List<PatchLineComment> byCommentStatus(
ResultSet<PatchLineComment> comments,
final PatchLineComment.Status status) {
return Lists.newArrayList(
Iterables.filter(comments, new Predicate<PatchLineComment>() {
@Override
public boolean apply(PatchLineComment input) {
return (input.getStatus() == status);
}
})
);
}
public List<PatchLineComment> byPatchSet(ReviewDb db,
ChangeNotes notes, PatchSet.Id psId) throws OrmException {
if (!migration.readComments()) {
return db.patchComments().byPatchSet(psId).toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(publishedByPatchSet(db, notes, psId));
Iterable<String> filtered = getDraftRefs(notes.getChangeId());
for (String refName : filtered) {
Account.Id account = Account.Id.fromRefPart(refName);
if (account != null) {
comments.addAll(draftByPatchSetAuthor(db, psId, account, notes));
}
}
return comments;
}
public List<PatchLineComment> publishedByChangeFile(ReviewDb db,
ChangeNotes notes, Change.Id changeId, String file) throws OrmException {
if (!migration.readPublishedComments()) {
if (!migration.readComments()) {
return db.patchComments().publishedByChangeFile(changeId, file).toList();
}
notes.load();
List<PatchLineComment> commentsOnFile = new ArrayList<PatchLineComment>();
List<PatchLineComment> comments = Lists.newArrayList();
// We must iterate through all comments to find the ones on this file.
addCommentsInFile(commentsOnFile, notes.getBaseComments().values(), file);
addCommentsInFile(commentsOnFile, notes.getPatchSetComments().values(),
addCommentsOnFile(comments, notes.getBaseComments().values(), file);
addCommentsOnFile(comments, notes.getPatchSetComments().values(),
file);
Collections.sort(commentsOnFile, ChangeNotes.PatchLineCommentComparator);
return commentsOnFile;
Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
return comments;
}
public List<PatchLineComment> publishedByPatchSet(ReviewDb db,
ChangeNotes notes, PatchSet.Id psId) throws OrmException {
if (!migration.readPublishedComments()) {
if (!migration.readComments()) {
return db.patchComments().publishedByPatchSet(psId).toList();
}
notes.load();
List<PatchLineComment> commentsOnPs = new ArrayList<PatchLineComment>();
commentsOnPs.addAll(notes.getPatchSetComments().get(psId));
commentsOnPs.addAll(notes.getBaseComments().get(psId));
return commentsOnPs;
List<PatchLineComment> comments = new ArrayList<PatchLineComment>();
comments.addAll(notes.getPatchSetComments().get(psId));
comments.addAll(notes.getBaseComments().get(psId));
return comments;
}
// TODO(yyonas): Delete drafts if they already existed.
public void addPublishedComments(ReviewDb db, ChangeUpdate update,
public List<PatchLineComment> draftByPatchSetAuthor(ReviewDb db,
PatchSet.Id psId, Account.Id author, ChangeNotes notes)
throws OrmException {
if (!migration.readComments()) {
return db.patchComments().draftByPatchSetAuthor(psId, author).toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(notes.getDraftBaseComments(author).row(psId).values());
comments.addAll(notes.getDraftPsComments(author).row(psId).values());
Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
return comments;
}
public List<PatchLineComment> draftByChangeFileAuthor(ReviewDb db,
ChangeNotes notes, String file, Account.Id author)
throws OrmException {
if (!migration.readComments()) {
return db.patchComments()
.draftByChangeFileAuthor(notes.getChangeId(), file, author)
.toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
addCommentsOnFile(comments, notes.getDraftBaseComments(author).values(),
file);
addCommentsOnFile(comments, notes.getDraftPsComments(author).values(),
file);
Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
return comments;
}
public List<PatchLineComment> draftByChangeAuthor(ReviewDb db,
ChangeNotes notes, Account.Id author)
throws OrmException {
if (!migration.readComments()) {
return db.patchComments().byChange(notes.getChangeId()).toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(notes.getDraftBaseComments(author).values());
comments.addAll(notes.getDraftPsComments(author).values());
return comments;
}
public List<PatchLineComment> draftByAuthor(ReviewDb db,
Account.Id author) throws OrmException {
if (!migration.readComments()) {
return db.patchComments().draftByAuthor(author).toList();
}
Set<String> refNames =
getRefNamesAllUsers(RefNames.REFS_DRAFT_COMMENTS);
List<PatchLineComment> comments = Lists.newArrayList();
for (String refName : refNames) {
Account.Id id = Account.Id.fromRefPart(refName);
if (!author.equals(id)) {
continue;
}
Change.Id changeId = Change.Id.parse(refName);
DraftCommentNotes draftNotes =
draftFactory.create(changeId, author).load();
comments.addAll(draftNotes.getDraftBaseComments().values());
comments.addAll(draftNotes.getDraftPsComments().values());
}
return comments;
}
public void insertComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.insertComment(c);
}
db.patchComments().insert(comments);
}
public void upsertComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.upsertComment(c);
@@ -86,7 +270,23 @@ public class PatchLineCommentsUtil {
db.patchComments().upsert(comments);
}
private static Collection<PatchLineComment> addCommentsInFile(
public void updateComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.updateComment(c);
}
db.patchComments().update(comments);
}
public void deleteComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.deleteComment(c);
}
db.patchComments().delete(comments);
}
private static Collection<PatchLineComment> addCommentsOnFile(
Collection<PatchLineComment> commentsOnFile,
Collection<PatchLineComment> allComments,
String file) {
@@ -98,4 +298,49 @@ public class PatchLineCommentsUtil {
}
return commentsOnFile;
}
public static void setCommentRevId(PatchLineComment c,
PatchListCache cache, Change change, PatchSet ps) throws OrmException {
if (c.getRevId() != null) {
return;
}
PatchList patchList;
try {
patchList = cache.get(change, ps);
} catch (PatchListNotAvailableException e) {
throw new OrmException(e);
}
c.setRevId((c.getSide() == (short) 0)
? new RevId(ObjectId.toString(patchList.getOldId()))
: new RevId(ObjectId.toString(patchList.getNewId())));
}
private Set<String> getRefNamesAllUsers(String prefix) throws OrmException {
Repository repo;
try {
repo = repoManager.openRepository(allUsers);
} catch (IOException e) {
throw new OrmException(e);
}
try {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
} catch (IOException e) {
throw new OrmException(e);
} finally {
repo.close();
}
}
private Iterable<String> getDraftRefs(final Change.Id changeId)
throws OrmException {
Set<String> refNames = getRefNamesAllUsers(RefNames.REFS_DRAFT_COMMENTS);
final String suffix = "-" + changeId.get();
return Iterables.filter(refNames, new Predicate<String>() {
@Override
public boolean apply(String input) {
return input.endsWith(suffix);
}
});
}
}

11
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -77,6 +77,7 @@ import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.extensions.webui.UiActions;
@@ -127,6 +128,7 @@ public class ChangeJson {
private final Provider<WebLinks> webLinks;
private final EnumSet<ListChangesOption> options;
private final ChangeMessagesUtil cmUtil;
private final PatchLineCommentsUtil plcUtil;
private AccountInfo.Loader accountLoader;
@@ -147,7 +149,8 @@ public class ChangeJson {
DynamicMap<RestView<ChangeResource>> changeViews,
Revisions revisions,
Provider<WebLinks> webLinks,
ChangeMessagesUtil cmUtil) {
ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil) {
this.db = db;
this.labelNormalizer = ln;
this.userProvider = user;
@@ -163,6 +166,7 @@ public class ChangeJson {
this.revisions = revisions;
this.webLinks = webLinks;
this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
options = EnumSet.noneOf(ListChangesOption.class);
}
@@ -821,9 +825,8 @@ public class ChangeJson {
&& userProvider.get().isIdentifiedUser()) {
IdentifiedUser user = (IdentifiedUser)userProvider.get();
out.hasDraftComments =
db.get().patchComments()
.draftByPatchSetAuthor(in.getId(), user.getAccountId())
.iterator().hasNext()
plcUtil.draftByPatchSetAuthor(db.get(), in.getId(),
user.getAccountId(), ctl.getNotes()).iterator().hasNext()
? true
: null;
}

29
gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java
View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.common.base.Strings;
import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -24,27 +26,41 @@ import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.PutDraft.Input;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.Collections;
@Singleton
class CreateDraft implements RestModifyView<RevisionResource, Input> {
private final Provider<ReviewDb> db;
private final ChangeUpdate.Factory updateFactory;
private final PatchLineCommentsUtil plcUtil;
private final PatchListCache patchListCache;
@Inject
CreateDraft(Provider<ReviewDb> db) {
CreateDraft(Provider<ReviewDb> db,
ChangeUpdate.Factory updateFactory,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache) {
this.db = db;
this.updateFactory = updateFactory;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
}
@Override
public Response<CommentInfo> apply(RevisionResource rsrc, Input in)
throws BadRequestException, OrmException {
throws BadRequestException, OrmException, IOException {
if (Strings.isNullOrEmpty(in.path)) {
throw new BadRequestException("path must be non-empty");
} else if (in.message == null || in.message.trim().isEmpty()) {
@@ -59,15 +75,20 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
? in.line
: in.range != null ? in.range.getEndLine() : 0;
Timestamp now = TimeUtil.nowTs();
ChangeUpdate update = updateFactory.create(rsrc.getControl(), now);
PatchLineComment c = new PatchLineComment(
new PatchLineComment.Key(
new Patch.Key(rsrc.getPatchSet().getId(), in.path),
ChangeUtil.messageUUID(db.get())),
line, rsrc.getAccountId(), Url.decode(in.inReplyTo), TimeUtil.nowTs());
line, rsrc.getAccountId(), Url.decode(in.inReplyTo), now);
c.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
c.setMessage(in.message.trim());
c.setRange(in.range);
db.get().patchComments().insert(Collections.singleton(c));
setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
plcUtil.insertComments(db.get(), update, Collections.singleton(c));
update.commit();
return Response.created(new CommentInfo(c, null));
}
}

27
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraft.java
View File

@@ -14,15 +14,22 @@
package com.google.gerrit.server.change;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.DeleteDraft.Input;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
@Singleton
@@ -31,16 +38,30 @@ class DeleteDraft implements RestModifyView<DraftResource, Input> {
}
private final Provider<ReviewDb> db;
private final PatchLineCommentsUtil plcUtil;
private final ChangeUpdate.Factory updateFactory;
private final PatchListCache patchListCache;
@Inject
DeleteDraft(Provider<ReviewDb> db) {
DeleteDraft(Provider<ReviewDb> db,
PatchLineCommentsUtil plcUtil,
ChangeUpdate.Factory updateFactory,
PatchListCache patchListCache) {
this.db = db;
this.plcUtil = plcUtil;
this.updateFactory = updateFactory;
this.patchListCache = patchListCache;
}
@Override
public Response<CommentInfo> apply(DraftResource rsrc, Input input)
throws OrmException {
db.get().patchComments().delete(Collections.singleton(rsrc.getComment()));
throws OrmException, IOException {
ChangeUpdate update = updateFactory.create(rsrc.getControl());
PatchLineComment c = rsrc.getComment();
setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
update.commit();
return Response.none();
}
}

12
gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -34,16 +35,19 @@ class Drafts implements ChildCollection<RevisionResource, DraftResource> {
private final Provider<CurrentUser> user;
private final ListDrafts list;
private final Provider<ReviewDb> dbProvider;
private final PatchLineCommentsUtil plcUtil;
@Inject
Drafts(DynamicMap<RestView<DraftResource>> views,
Provider<CurrentUser> user,
ListDrafts list,
Provider<ReviewDb> dbProvider) {
Provider<ReviewDb> dbProvider,
PatchLineCommentsUtil plcUtil) {
this.views = views;
this.user = user;
this.list = list;
this.dbProvider = dbProvider;
this.plcUtil = plcUtil;
}
@Override
@@ -62,10 +66,8 @@ class Drafts implements ChildCollection<RevisionResource, DraftResource> {
throws ResourceNotFoundException, OrmException, AuthException {
checkIdentifiedUser();
String uuid = id.get();
for (PatchLineComment c : dbProvider.get().patchComments()
.draftByPatchSetAuthor(
rev.getPatchSet().getId(),
rev.getAccountId())) {
for (PatchLineComment c : plcUtil.draftByPatchSetAuthor(dbProvider.get(),
rev.getPatchSet().getId(), rev.getAccountId(), rev.getNotes())) {
if (uuid.equals(c.getKey().get())) {
return new DraftResource(rev, c);
}

5
gerrit-server/src/main/java/com/google/gerrit/server/change/ListComments.java
View File

@@ -26,13 +26,10 @@ import com.google.inject.Singleton;
@Singleton
class ListComments extends ListDrafts {
private final PatchLineCommentsUtil plcUtil;
@Inject
ListComments(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf,
PatchLineCommentsUtil plcUtil) {
super(db, alf);
this.plcUtil = plcUtil;
super(db, alf, plcUtil);
}
@Override

12
gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
View File

@@ -22,6 +22,7 @@ import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -36,20 +37,21 @@ import java.util.Map;
@Singleton
class ListDrafts implements RestReadView<RevisionResource> {
protected final Provider<ReviewDb> db;
protected final PatchLineCommentsUtil plcUtil;
private final AccountInfo.Loader.Factory accountLoaderFactory;
@Inject
ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf) {
ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf,
PatchLineCommentsUtil plcUtil) {
this.db = db;
this.accountLoaderFactory = alf;
this.plcUtil = plcUtil;
}
protected Iterable<PatchLineComment> listComments(RevisionResource rsrc)
throws OrmException {
return db.get().patchComments()
.draftByPatchSetAuthor(
rsrc.getPatchSet().getId(),
rsrc.getAccountId());
return plcUtil.draftByPatchSetAuthor(db.get(), rsrc.getPatchSet().getId(),
rsrc.getAccountId(), rsrc.getNotes());
}
protected boolean includeAuthorInfo() {

29
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.common.base.Objects;
import com.google.common.base.Strings;
@@ -44,7 +45,6 @@ import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -54,9 +54,7 @@ import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.LabelVote;
@@ -65,7 +63,6 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.lib.ObjectId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -346,15 +343,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
List<PatchLineComment> del = Lists.newArrayList();
List<PatchLineComment> ups = Lists.newArrayList();
PatchList patchList = null;
try {
patchList = patchListCache.get(rsrc.getChange(), rsrc.getPatchSet());
} catch (PatchListNotAvailableException e) {
throw new OrmException("could not load PatchList for this patchset", e);
}
RevId patchSetCommit = new RevId(ObjectId.toString(patchList.getNewId()));
RevId baseCommit = new RevId(ObjectId.toString(patchList.getOldId()));
for (Map.Entry<String, List<CommentInput>> ent : in.entrySet()) {
String path = ent.getKey();
for (CommentInput c : ent.getValue()) {
@@ -374,7 +362,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
e.setStatus(PatchLineComment.Status.PUBLISHED);
e.setWrittenOn(timestamp);
e.setSide(c.side == Side.PARENT ? (short) 0 : (short) 1);
e.setRevId(c.side == Side.PARENT ? baseCommit : patchSetCommit);
setCommentRevId(e, patchListCache, rsrc.getChange(),
rsrc.getPatchSet());
e.setMessage(c.message);
if (c.range != null) {
e.setRange(new CommentRange(
@@ -398,13 +387,14 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
for (PatchLineComment e : drafts.values()) {
e.setStatus(PatchLineComment.Status.PUBLISHED);
e.setWrittenOn(timestamp);
e.setRevId(e.getSide() == (short) 0 ? baseCommit : patchSetCommit);
setCommentRevId(e, patchListCache, rsrc.getChange(),
rsrc.getPatchSet());
ups.add(e);
}
break;
}
db.get().patchComments().delete(del);
plcUtil.addPublishedComments(db.get(), update, ups);
plcUtil.deleteComments(db.get(), update, del);
plcUtil.upsertComments(db.get(), update, ups);
comments.addAll(ups);
return !del.isEmpty() || !ups.isEmpty();
}
@@ -412,9 +402,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
private Map<String, PatchLineComment> scanDraftComments(
RevisionResource rsrc) throws OrmException {
Map<String, PatchLineComment> drafts = Maps.newHashMap();
for (PatchLineComment c : db.get().patchComments().draftByPatchSetAuthor(
rsrc.getPatchSet().getId(),
rsrc.getAccountId())) {
for (PatchLineComment c : plcUtil.draftByPatchSetAuthor(db.get(),
rsrc.getPatchSet().getId(), rsrc.getAccountId(), rsrc.getNotes())) {
drafts.put(c.getKey().get(), c);
}
return drafts;

36
gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java
View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.DefaultInput;
@@ -24,13 +26,17 @@ import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.PutDraft.Input;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.Collections;
@@ -51,17 +57,28 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
private final Provider<ReviewDb> db;
private final DeleteDraft delete;
private final PatchLineCommentsUtil plcUtil;
private final ChangeUpdate.Factory updateFactory;
private final PatchListCache patchListCache;
@Inject
PutDraft(Provider<ReviewDb> db, DeleteDraft delete) {
PutDraft(Provider<ReviewDb> db,
DeleteDraft delete,
PatchLineCommentsUtil plcUtil,
ChangeUpdate.Factory updateFactory,
PatchListCache patchListCache) {
this.db = db;
this.delete = delete;
this.plcUtil = plcUtil;
this.updateFactory = updateFactory;
this.patchListCache = patchListCache;
}
@Override
public Response<CommentInfo> apply(DraftResource rsrc, Input in) throws
BadRequestException, OrmException {
BadRequestException, OrmException, IOException {
PatchLineComment c = rsrc.getComment();
ChangeUpdate update = updateFactory.create(rsrc.getControl());
if (in == null || in.message == null || in.message.trim().isEmpty()) {
return delete.apply(rsrc, null);
} else if (in.id != null && !rsrc.getId().equals(in.id)) {
@@ -76,7 +93,8 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
&& !in.path.equals(c.getKey().getParentKey().getFileName())) {
// Updating the path alters the primary key, which isn't possible.
// Delete then recreate the comment instead of an update.
db.get().patchComments().delete(Collections.singleton(c));
plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
c = new PatchLineComment(
new PatchLineComment.Key(
new Patch.Key(rsrc.getPatchSet().getId(), in.path),
@@ -84,10 +102,18 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
c.getLine(),
rsrc.getAuthorId(),
c.getParentUuid(), TimeUtil.nowTs());
db.get().patchComments().insert(Collections.singleton(update(c, in)));
setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
plcUtil.insertComments(db.get(), update,
Collections.singleton(update(c, in)));
} else {
db.get().patchComments().update(Collections.singleton(update(c, in)));
if (c.getRevId() == null) {
setCommentRevId(c, patchListCache, rsrc.getChange(),
rsrc.getPatchSet());
}
plcUtil.updateComments(db.get(), update,
Collections.singleton(update(c, in)));
}
update.commit();
return Response.ok(new CommentInfo(c, null));
}

49
gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.git;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -21,8 +22,10 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
@@ -59,7 +62,24 @@ public class MetaDataUpdate {
public MetaDataUpdate create(Project.NameKey name, IdentifiedUser user)
throws RepositoryNotFoundException, IOException {
MetaDataUpdate md = factory.create(name, mgr.openRepository(name));
return create(name, user, null);
}
/**
* Create an update using an existing batch ref update.
* <p>
* This allows batching together updates to multiple metadata refs. For making
* multiple commits to a single metadata ref, see
* {@link VersionedMetaData#openUpdate(MetaDataUpdate)}.
*
* @param name project name.
* @param user user for the update.
* @param batch batch update to use; the caller is responsible for committing
* the update.
*/
public MetaDataUpdate create(Project.NameKey name, IdentifiedUser user,
BatchRefUpdate batch) throws RepositoryNotFoundException, IOException {
MetaDataUpdate md = factory.create(name, mgr.openRepository(name), batch);
md.getCommitBuilder().setAuthor(createPersonIdent(user));
md.getCommitBuilder().setCommitter(serverIdent);
return md;
@@ -86,7 +106,13 @@ public class MetaDataUpdate {
public MetaDataUpdate create(Project.NameKey name)
throws RepositoryNotFoundException, IOException {
MetaDataUpdate md = factory.create(name, mgr.openRepository(name));
return create(name, null);
}
/** @see User#create(Project.NameKey, IdentifiedUser, BatchRefUpdate) */
public MetaDataUpdate create(Project.NameKey name, BatchRefUpdate batch)
throws RepositoryNotFoundException, IOException {
MetaDataUpdate md = factory.create(name, mgr.openRepository(name), batch);
md.getCommitBuilder().setAuthor(serverIdent);
md.getCommitBuilder().setCommitter(serverIdent);
return md;
@@ -95,24 +121,32 @@ public class MetaDataUpdate {
interface InternalFactory {
MetaDataUpdate create(@Assisted Project.NameKey projectName,
@Assisted Repository db);
@Assisted Repository db, @Assisted @Nullable BatchRefUpdate batch);
}
private final GitReferenceUpdated gitRefUpdated;
private final Project.NameKey projectName;
private final Repository db;
private final BatchRefUpdate batch;
private final CommitBuilder commit;
private boolean allowEmpty;
@Inject
@AssistedInject
public MetaDataUpdate(GitReferenceUpdated gitRefUpdated,
@Assisted Project.NameKey projectName, @Assisted Repository db) {
@Assisted Project.NameKey projectName, @Assisted Repository db,
@Assisted @Nullable BatchRefUpdate batch) {
this.gitRefUpdated = gitRefUpdated;
this.projectName = projectName;
this.db = db;
this.batch = batch;
this.commit = new CommitBuilder();
}
public MetaDataUpdate(GitReferenceUpdated gitRefUpdated,
Project.NameKey projectName, Repository db) {
this(gitRefUpdated, projectName, db, null);
}
/** Set the commit message used when committing the update. */
public void setMessage(String message) {
getCommitBuilder().setMessage(message);
@@ -128,6 +162,11 @@ public class MetaDataUpdate {
this.allowEmpty = allowEmpty;
}
/** @return batch in which to run the update, or {@code null} for no batch. */
BatchRefUpdate getBatch() {
return batch;
}
/** Close the cached Repository handle. */
public void close() {
getRepository().close();

7
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -2131,8 +2131,11 @@ public class ReceiveCommits {
allRefs.size() / estRefsPerChange,
estRefsPerChange);
for (Ref ref : allRefs.values()) {
if (ref.getObjectId() != null && PatchSet.isRef(ref.getName())) {
refsByChange.put(Change.Id.fromRef(ref.getName()), ref);
if (ref.getObjectId() != null) {
PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
if (psId != null) {
refsByChange.put(psId.getParentKey(), ref);
}
}
}
}

26
gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
View File

@@ -26,6 +26,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -40,6 +41,7 @@ import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.util.RawParseUtils;
@@ -181,6 +183,21 @@ public abstract class VersionedMetaData {
void close();
}
/**
* Open a batch of updates to the same metadata ref.
* <p>
* This allows making multiple commits to a single metadata ref, at the end of
* which is a single ref update. For batching together updates to multiple
* refs (each consisting of one or more commits against their respective
* refs), create the {@link MetaDataUpdate} with a {@link BatchRefUpdate}.
* <p>
* A ref update produced by this {@link BatchMetaDataUpdate} is only committed
* if there is no associated {@link BatchRefUpdate}. As a result, the
* configured ref updated event is not fired if there is an associated batch.
*
* @param update helper info about the update.
* @throws IOException if the update failed.
*/
public BatchMetaDataUpdate openUpdate(final MetaDataUpdate update) throws IOException {
final Repository db = update.getRepository();
@@ -302,6 +319,15 @@ public abstract class VersionedMetaData {
private RevCommit updateRef(AnyObjectId oldId, AnyObjectId newId,
String refName) throws IOException {
BatchRefUpdate bru = update.getBatch();
if (bru != null) {
bru.addCommand(new ReceiveCommand(
oldId.toObjectId(), newId.toObjectId(), refName));
inserter.flush();
revision = rw.parseCommit(newId);
return revision;
}
RefUpdate ru = db.updateRef(refName);
ru.setExpectedOldObjectId(oldId);
ru.setNewObjectId(src);

5
gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
View File

@@ -82,12 +82,13 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
final List<Ref> deferredTags = new ArrayList<>();
for (Ref ref : refs.values()) {
PatchSet.Id psId;
if (ref.getName().startsWith(RefNames.REFS_CACHE_AUTOMERGE)) {
continue;
} else if (PatchSet.isRef(ref.getName())) {
} else if ((psId = PatchSet.Id.fromRef(ref.getName())) != null) {
// Reference to a patch set is visible if the change is visible.
//
if (showChanges && visibleChanges.contains(Change.Id.fromRef(ref.getName()))) {
if (showChanges && visibleChanges.contains(psId.getParentKey())) {
result.put(ref.getName(), ref);
}

2
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
View File

@@ -408,7 +408,7 @@ public class ChangeField {
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
Set<String> r = Sets.newHashSet();
for (PatchLineComment c : input.comments()) {
for (PatchLineComment c : input.publishedComments()) {
r.add(c.getMessage());
}
for (ChangeMessage m : input.messages()) {

25
gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail;
import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.errors.EmailException;
@@ -24,6 +25,7 @@ import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.patch.PatchFile;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -53,13 +55,16 @@ public class CommentSender extends ReplyToChangeSender {
private final NotifyHandling notify;
private List<PatchLineComment> inlineComments = Collections.emptyList();
private final PatchLineCommentsUtil plcUtil;
@Inject
public CommentSender(EmailArguments ea,
@Assisted NotifyHandling notify,
@Assisted Change c) {
@Assisted Change c,
PatchLineCommentsUtil plcUtil) {
super(ea, c, "comment");
this.notify = notify;
this.plcUtil = plcUtil;
}
public void setPatchLineComments(final List<PatchLineComment> plc)
@@ -232,17 +237,19 @@ public class CommentSender extends ReplyToChangeSender {
private void appendQuotedParent(StringBuilder out, PatchLineComment child) {
if (child.getParentUuid() != null) {
PatchLineComment parent;
try {
parent = args.db.get().patchComments().get(
new PatchLineComment.Key(
Optional<PatchLineComment> parent;
PatchLineComment.Key key = new PatchLineComment.Key(
child.getKey().getParentKey(),
child.getParentUuid()));
child.getParentUuid());
try {
parent = plcUtil.get(args.db.get(), changeData.notes(), key);
} catch (OrmException e) {
parent = null;
log.warn("Could not find the parent of this comment: "
+ child.toString());
parent = Optional.absent();
}
if (parent != null) {
String msg = parent.getMessage().trim();
if (parent.isPresent()) {
String msg = parent.get().getMessage().trim();
if (msg.length() > 75) {
msg = msg.substring(0, 75);
}

8
gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
View File

@@ -29,6 +29,7 @@ import com.google.gerrit.server.git.VersionedMetaData;
import com.google.gerrit.server.project.ChangeControl;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
@@ -105,10 +106,15 @@ public abstract class AbstractChangeUpdate extends VersionedMetaData {
}
public BatchMetaDataUpdate openUpdate() throws IOException {
return openUpdateInBatch(null);
}
public BatchMetaDataUpdate openUpdateInBatch(BatchRefUpdate bru)
throws IOException {
if (migration.write()) {
load();
MetaDataUpdate md =
updateFactory.create(getProjectName(), getUser());
updateFactory.create(getProjectName(), getUser(), bru);
md.setAllowEmpty(true);
return super.openUpdate(md);
}

25
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
View File

@@ -105,9 +105,11 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
verifyComment(c);
checkArgument(c.getStatus() == Status.DRAFT,
"Cannot insert a published comment into a ChangeDraftUpdate");
if (migration.readComments()) {
checkArgument(!changeNotes.containsComment(c),
"A comment already exists with the same key,"
+ " so the following comment cannot be inserted: %s", c);
}
upsertComments.add(c);
}
@@ -122,17 +124,33 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
verifyComment(c);
checkArgument(c.getStatus() == Status.DRAFT,
"Cannot update a published comment into a ChangeDraftUpdate");
// Here, we check to see if this comment existed previously as a draft.
// However, this could cause a race condition if there is a delete and an
// update operation happening concurrently (or two deletes) and they both
// believe that the comment exists. If a delete happens first, then
// the update will fail. However, this is an acceptable risk since the
// caller wanted the comment deleted anyways, so the end result will be the
// same either way.
if (migration.readComments()) {
checkArgument(draftNotes.containsComment(c),
"Cannot update this comment because it didn't exist previously");
}
upsertComments.add(c);
}
public void deleteComment(PatchLineComment c) {
verifyComment(c);
// See the comment above about potential race condition.
if (migration.readComments()) {
checkArgument(draftNotes.containsComment(c), "Cannot delete this comment"
+ " because it didn't previously exist as a draft");
}
if (migration.write()) {
if (draftNotes.containsComment(c)) {
deleteComments.add(c);
}
}
}
/**
* Deletes a PatchLineComment from the list of drafts only if it existed
@@ -151,7 +169,9 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
checkArgument(getCommentPsId(comment).equals(psId),
"Comment on %s does not match configured patch set %s",
getCommentPsId(comment), psId);
if (migration.write()) {
checkArgument(comment.getRevId() != null);
}
checkArgument(comment.getAuthor().equals(accountId),
"The author for the following comment does not match the author of"
+ " this ChangeDraftUpdate (%s): %s", accountId, comment);
@@ -174,6 +194,8 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
Table<PatchSet.Id, String, PatchLineComment> psDrafts =
draftNotes.getDraftPsComments();
boolean draftsEmpty = baseDrafts.isEmpty() && psDrafts.isEmpty();
// There is no need to rewrite the note for one of the sides of the patch
// set if all of the modifications were made to the comments of one side,
// so we set these flags to potentially save that extra work.
@@ -219,7 +241,8 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
updateNoteMap(revisionSideChanged, noteMap, newPsDrafts,
psRevId);
removedAllComments.set(baseDrafts.isEmpty() && psDrafts.isEmpty());
removedAllComments.set(
baseDrafts.isEmpty() && psDrafts.isEmpty() && !draftsEmpty);
return noteMap.writeTree(inserter);
}

20
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@@ -239,9 +239,11 @@ public class ChangeUpdate extends AbstractChangeUpdate {
if (notes == null) {
notes = getChangeNotes().load();
}
if (migration.readComments()) {
checkArgument(!notes.containsComment(c),
"A comment already exists with the same key as the following comment,"
+ " so we cannot insert this comment: %s", c);
}
if (c.getSide() == 0) {
commentsForBase.add(c);
} else {
@@ -254,8 +256,19 @@ public class ChangeUpdate extends AbstractChangeUpdate {
draftUpdate.insertComment(c);
}
private void upsertPublishedComment(PatchLineComment c) {
private void upsertPublishedComment(PatchLineComment c) throws OrmException {
verifyComment(c);
if (notes == null) {
notes = getChangeNotes().load();
}
// This could allow callers to update a published comment if migration.write
// is on and migration.readComments is off because we will not be able to
// verify that the comment didn't already exist as a published comment
// since we don't have a ReviewDb.
if (migration.readComments()) {
checkArgument(!notes.containsCommentPublished(c),
"Cannot update a comment that has already been published and saved");
}
if (c.getSide() == 0) {
commentsForBase.add(c);
} else {
@@ -273,8 +286,12 @@ public class ChangeUpdate extends AbstractChangeUpdate {
if (notes == null) {
notes = getChangeNotes().load();
}
// See comment above in upsertPublishedComment() about potential risk with
// this check.
if (migration.readComments()) {
checkArgument(!notes.containsCommentPublished(c),
"Cannot update a comment that has already been published and saved");
}
if (c.getSide() == 0) {
commentsForBase.add(c);
} else {
@@ -298,7 +315,6 @@ public class ChangeUpdate extends AbstractChangeUpdate {
draftUpdate.deleteCommentIfPresent(c);
}
private void createDraftUpdateIfNull(PatchLineComment c) throws OrmException {
if (draftUpdate == null) {
draftUpdate = draftUpdateFactory.create(ctl, when);

1
gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java
View File

@@ -239,6 +239,7 @@ public class CommentsInNotesUtil {
if (note[ptr.value] == '\n') {
range.setEndLine(startLine);
ptr.value += 1;
return range;
} else if (note[ptr.value] == ':') {
range.setStartLine(startLine);

12
gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
View File

@@ -36,14 +36,14 @@ public class NotesMigration {
cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "patchSetApprovals", "read", true);
cfg.setBoolean("notedb", "changeMessages", "read", true);
cfg.setBoolean("notedb", "publishedComments", "read", true);
cfg.setBoolean("notedb", "comments", "read", true);
return new NotesMigration(cfg);
}
private final boolean write;
private final boolean readPatchSetApprovals;
private final boolean readChangeMessages;
private final boolean readPublishedComments;
private final boolean readComments;
@Inject
NotesMigration(@GerritServerConfig Config cfg) {
@@ -52,8 +52,8 @@ public class NotesMigration {
cfg.getBoolean("notedb", "patchSetApprovals", "read", false);
readChangeMessages =
cfg.getBoolean("notedb", "changeMessages", "read", false);
readPublishedComments =
cfg.getBoolean("notedb", "publishedComments", "read", false);
readComments =
cfg.getBoolean("notedb", "comments", "read", false);
}
public boolean write() {
@@ -68,7 +68,7 @@ public class NotesMigration {
return readChangeMessages;
}
public boolean readPublishedComments() {
return readPublishedComments;
public boolean readComments() {
return readComments;
}
}

3
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
View File

@@ -338,7 +338,8 @@ public class PatchScriptFactory implements Callable<PatchScript> {
private void loadDrafts(final Map<Patch.Key, Patch> byKey,
final AccountInfoCacheFactory aic, final Account.Id me, final String file)
throws OrmException {
for (PatchLineComment c : db.patchComments().draftByChangeFileAuthor(changeId, file, me)) {
for (PatchLineComment c :
plcUtil.draftByChangeFileAuthor(db, control.getNotes(), file, me)) {
if (comments.include(c)) {
aic.want(me);
}

3
gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java
View File

@@ -30,7 +30,8 @@ public class BasicChangeRewrites extends QueryRewriter<ChangeData> {
new InvalidProvider<ReviewDb>(), //
new InvalidProvider<ChangeQueryRewriter>(), //
null, null, null, null, null, null, null, //
null, null, null, null, null, null, null, null, null, null), null);
null, null, null, null, null, null, null, null, null, null, null),
null);
private static final QueryRewriter.Definition<ChangeData, BasicChangeRewrites> mydef =
new QueryRewriter.Definition<ChangeData, BasicChangeRewrites>(

20
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
@@ -154,7 +155,7 @@ public class ChangeData {
*/
static ChangeData createForTest(Change.Id id, int currentPatchSetId) {
ChangeData cd = new ChangeData(null, null, null, null, null,
null, null, null, null, id);
null, null, null, null, null, id);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd;
}
@@ -166,6 +167,7 @@ public class ChangeData {
private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
private final PatchLineCommentsUtil plcUtil;
private final PatchListCache patchListCache;
private final NotesMigration notesMigration;
private final Change.Id legacyId;
@@ -179,7 +181,7 @@ public class ChangeData {
private ListMultimap<PatchSet.Id, PatchSetApproval> allApprovals;
private List<PatchSetApproval> currentApprovals;
private Map<Integer, List<String>> files = new HashMap<>();
private Collection<PatchLineComment> comments;
private Collection<PatchLineComment> publishedComments;
private CurrentUser visibleTo;
private ChangeControl changeControl;
private List<ChangeMessage> messages;
@@ -194,6 +196,7 @@ public class ChangeData {
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -205,6 +208,7 @@ public class ChangeData {
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = id;
@@ -218,6 +222,7 @@ public class ChangeData {
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -229,6 +234,7 @@ public class ChangeData {
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = c.getId();
@@ -243,6 +249,7 @@ public class ChangeData {
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache,
NotesMigration notesMigration,
@Assisted ReviewDb db,
@@ -254,6 +261,7 @@ public class ChangeData {
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
legacyId = c.getChange().getId();
@@ -519,12 +527,12 @@ public class ChangeData {
return approvalsUtil.getReviewers(notes(), approvals().values());
}
public Collection<PatchLineComment> comments()
public Collection<PatchLineComment> publishedComments()
throws OrmException {
if (comments == null) {
comments = db.patchComments().byChange(legacyId).toList();
if (publishedComments == null) {
publishedComments = plcUtil.publishedByChange(db, notes());
}
return comments;
return publishedComments;
}
public List<ChangeMessage> messages()

4
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupBackend;
@@ -146,6 +147,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
final CapabilityControl.Factory capabilityControlFactory;
final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeData.Factory changeDataFactory;
final PatchLineCommentsUtil plcUtil;
final AccountResolver accountResolver;
final GroupBackend groupBackend;
final AllProjectsName allProjectsName;
@@ -168,6 +170,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
CapabilityControl.Factory capabilityControlFactory,
ChangeControl.GenericFactory changeControlGenericFactory,
ChangeData.Factory changeDataFactory,
PatchLineCommentsUtil plcUtil,
AccountResolver accountResolver,
GroupBackend groupBackend,
AllProjectsName allProjectsName,
@@ -187,6 +190,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
this.capabilityControlFactory = capabilityControlFactory;
this.changeControlGenericFactory = changeControlGenericFactory;
this.changeDataFactory = changeDataFactory;
this.plcUtil = plcUtil;
this.accountResolver = accountResolver;
this.groupBackend = groupBackend;
this.allProjectsName = allProjectsName;

17
gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByPredicate.java
View File

@@ -33,7 +33,8 @@ class HasDraftByPredicate extends OperatorPredicate<ChangeData> implements
private final Arguments args;
private final Account.Id accountId;
HasDraftByPredicate(Arguments args, Account.Id accountId) {
HasDraftByPredicate(Arguments args,
Account.Id accountId) {
super(ChangeQueryBuilder.FIELD_DRAFTBY, accountId.toString());
this.args = args;
this.accountId = accountId;
@@ -41,20 +42,16 @@ class HasDraftByPredicate extends OperatorPredicate<ChangeData> implements
@Override
public boolean match(final ChangeData object) throws OrmException {
for (PatchLineComment c : object.comments()) {
if (c.getStatus() == PatchLineComment.Status.DRAFT
&& c.getAuthor().equals(accountId)) {
return true;
}
}
return false;
return !args.plcUtil
.draftByChangeAuthor(args.db.get(), object.notes(), accountId)
.isEmpty();
}
@Override
public ResultSet<ChangeData> read() throws OrmException {
Set<Change.Id> ids = new HashSet<>();
for (PatchLineComment sc : args.db.get().patchComments()
.draftByAuthor(accountId)) {
for (PatchLineComment sc :
args.plcUtil.draftByAuthor(args.db.get(), accountId)) {
ids.add(sc.getKey().getParentKey().getParentKey().getParentKey());
}

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
View File

@@ -366,7 +366,7 @@ public class QueryProcessor {
eventFactory.addComments(c, d.messages());
if (includePatchSets) {
for (PatchSetAttribute attribute : c.patchSets) {
eventFactory.addPatchSetComments(attribute, d.comments());
eventFactory.addPatchSetComments(attribute, d.publishedComments());
}
}
}

166
gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java
View File

@@ -21,6 +21,7 @@ import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.replay;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import com.google.common.base.Objects;
@@ -43,6 +44,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
@@ -60,7 +62,6 @@ import com.google.gerrit.server.git.GitModule;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.TimeUtil;
@@ -74,6 +75,8 @@ import com.google.gwtorm.server.ResultSet;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.Provides;
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.util.Providers;
@@ -104,7 +107,7 @@ public class CommentsTest {
public static @GerritServerConfig Config noteDbEnabled() {
@GerritServerConfig Config cfg = new Config();
cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "publishedComments", "read", true);
cfg.setBoolean("notedb", "comments", "read", true);
return cfg;
}
@@ -118,15 +121,23 @@ public class CommentsTest {
private PatchLineComment plc1;
private PatchLineComment plc2;
private PatchLineComment plc3;
private PatchLineComment plc4;
private PatchLineComment plc5;
private IdentifiedUser changeOwner;
@Before
public void setUp() throws Exception {
@SuppressWarnings("unchecked")
final DynamicMap<RestView<CommentResource>> views =
final DynamicMap<RestView<CommentResource>> commentViews =
createMock(DynamicMap.class);
final TypeLiteral<DynamicMap<RestView<CommentResource>>> viewsType =
final TypeLiteral<DynamicMap<RestView<CommentResource>>> commentViewsType =
new TypeLiteral<DynamicMap<RestView<CommentResource>>>() {};
@SuppressWarnings("unchecked")
final DynamicMap<RestView<DraftResource>> draftViews =
createMock(DynamicMap.class);
final TypeLiteral<DynamicMap<RestView<DraftResource>>> draftViewsType =
new TypeLiteral<DynamicMap<RestView<DraftResource>>>() {};
final AccountInfo.Loader.Factory alf =
createMock(AccountInfo.Loader.Factory.class);
final ReviewDb db = createMock(ReviewDb.class);
@@ -139,10 +150,23 @@ public class CommentsTest {
@SuppressWarnings("unused")
InMemoryRepository repo = repoManager.createRepository(project);
Account co = new Account(new Account.Id(1), TimeUtil.nowTs());
co.setFullName("Change Owner");
co.setPreferredEmail("change@owner.com");
accountCache.put(co);
final Account.Id ownerId = co.getId();
Account ou = new Account(new Account.Id(2), TimeUtil.nowTs());
ou.setFullName("Other Account");
ou.setPreferredEmail("other@account.com");
accountCache.put(ou);
Account.Id otherUserId = ou.getId();
AbstractModule mod = new AbstractModule() {
@Override
protected void configure() {
bind(viewsType).toInstance(views);
bind(commentViewsType).toInstance(commentViews);
bind(draftViewsType).toInstance(draftViews);
bind(AccountInfo.Loader.Factory.class).toInstance(alf);
bind(ReviewDb.class).toProvider(Providers.<ReviewDb>of(db));
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
@@ -162,28 +186,16 @@ public class CommentsTest {
bind(PersonIdent.class).annotatedWith(GerritPersonIdent.class)
.toInstance(serverIdent);
}
@Provides
@Singleton
CurrentUser getCurrentUser(IdentifiedUser.GenericFactory userFactory) {
return userFactory.create(ownerId);
}
};
injector = Guice.createInjector(mod);
NotesMigration migration = injector.getInstance(NotesMigration.class);
allUsers = injector.getInstance(AllUsersNameProvider.class);
repoManager.createRepository(allUsers.get());
plcUtil = new PatchLineCommentsUtil(migration);
Account co = new Account(new Account.Id(1), TimeUtil.nowTs());
co.setFullName("Change Owner");
co.setPreferredEmail("change@owner.com");
accountCache.put(co);
Account.Id ownerId = co.getId();
Account ou = new Account(new Account.Id(2), TimeUtil.nowTs());
ou.setFullName("Other Account");
ou.setPreferredEmail("other@account.com");
accountCache.put(ou);
Account.Id otherUserId = ou.getId();
IdentifiedUser.GenericFactory userFactory =
injector.getInstance(IdentifiedUser.GenericFactory.class);
changeOwner = userFactory.create(ownerId);
@@ -199,6 +211,10 @@ public class CommentsTest {
expect(alf.create(true)).andReturn(accountLoader).anyTimes();
replay(accountLoader, alf);
allUsers = injector.getInstance(AllUsersNameProvider.class);
repoManager.createRepository(allUsers.get());
plcUtil = injector.getInstance(PatchLineCommentsUtil.class);
PatchLineCommentAccess plca = createMock(PatchLineCommentAccess.class);
expect(db.patchComments()).andReturn(plca).anyTimes();
@@ -208,7 +224,7 @@ public class CommentsTest {
PatchSet.Id psId2 = new PatchSet.Id(change.getId(), 2);
PatchSet ps2 = new PatchSet(psId2);
long timeBase = TimeUtil.nowMs();
long timeBase = TimeUtil.roundToSecond(TimeUtil.nowTs()).getTime();
plc1 = newPatchLineComment(psId1, "Comment1", null,
"FileOne.txt", Side.REVISION, 3, ownerId, timeBase,
"First Comment", new CommentRange(1, 2, 3, 4));
@@ -221,32 +237,56 @@ public class CommentsTest {
"FileOne.txt", Side.PARENT, 3, ownerId, timeBase + 2000,
"First Parent Comment", new CommentRange(1, 2, 3, 4));
plc3.setRevId(new RevId("CDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEF"));
plc4 = newPatchLineComment(psId2, "Comment4", null, "FileOne.txt",
Side.REVISION, 3, ownerId, timeBase + 3000, "Second Comment",
new CommentRange(1, 2, 3, 4), Status.DRAFT);
plc4.setRevId(new RevId("BCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDE"));
plc5 = newPatchLineComment(psId2, "Comment5", null, "FileOne.txt",
Side.REVISION, 5, ownerId, timeBase + 4000, "Third Comment",
new CommentRange(3, 4, 5, 6), Status.DRAFT);
plc5.setRevId(new RevId("BCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDE"));
List<PatchLineComment> commentsByOwner = Lists.newArrayList();
commentsByOwner.add(plc1);
commentsByOwner.add(plc3);
List<PatchLineComment> commentsByReviewer = Lists.newArrayList();
commentsByReviewer.add(plc2);
List<PatchLineComment> drafts = Lists.newArrayList();
drafts.add(plc4);
drafts.add(plc5);
plca.upsert(commentsByOwner);
expectLastCall().anyTimes();
plca.upsert(commentsByReviewer);
expectLastCall().anyTimes();
plca.upsert(drafts);
expectLastCall().anyTimes();
expect(plca.publishedByPatchSet(psId1))
.andAnswer(results(plc1, plc2, plc3)).anyTimes();
expect(plca.publishedByPatchSet(psId2))
.andAnswer(results()).anyTimes();
expect(plca.draftByPatchSetAuthor(psId1, ownerId))
.andAnswer(results()).anyTimes();
expect(plca.draftByPatchSetAuthor(psId2, ownerId))
.andAnswer(results(plc4, plc5)).anyTimes();
expect(plca.byChange(change.getId()))
.andAnswer(results(plc1, plc2, plc3, plc4, plc5)).anyTimes();
replay(db, plca);
ChangeUpdate update = newUpdate(change, changeOwner);
update.setPatchSetId(psId1);
plcUtil.addPublishedComments(db, update, commentsByOwner);
plcUtil.upsertComments(db, update, commentsByOwner);
update.commit();
update = newUpdate(change, otherUser);
update.setPatchSetId(psId1);
plcUtil.addPublishedComments(db, update, commentsByReviewer);
plcUtil.upsertComments(db, update, commentsByReviewer);
update.commit();
update = newUpdate(change, changeOwner);
update.setPatchSetId(psId2);
plcUtil.upsertComments(db, update, drafts);
update.commit();
ChangeControl ctl = stubChangeControl(change);
@@ -286,6 +326,37 @@ public class CommentsTest {
assertGetComment(injector, revRes1, null, "BadComment");
}
@Test
public void testListDrafts() throws Exception {
// test ListDrafts for patch set 1
assertListDrafts(injector, revRes1,
Collections.<String, ArrayList<PatchLineComment>> emptyMap());
// test ListDrafts for patch set 2
assertListDrafts(injector, revRes2, ImmutableMap.of(
"FileOne.txt", Lists.newArrayList(plc4, plc5)));
}
@Test
public void testPatchLineCommentsUtilByCommentStatus() throws OrmException {
List<PatchLineComment> publishedActual = plcUtil.publishedByChange(
injector.getInstance(ReviewDb.class), revRes2.getNotes());
List<PatchLineComment> draftActual = plcUtil.draftByChange(
injector.getInstance(ReviewDb.class), revRes2.getNotes());
List<PatchLineComment> publishedExpected =
Lists.newArrayList(plc1, plc2, plc3);
List<PatchLineComment> draftExpected =
Lists.newArrayList(plc4, plc5);
assertEquals(publishedExpected.size(), publishedActual.size());
assertEquals(draftExpected.size(), draftActual.size());
for (PatchLineComment c : draftExpected) {
assertTrue(draftActual.contains(c));
}
for (PatchLineComment c : publishedExpected) {
assertTrue(publishedActual.contains(c));
}
}
private static IAnswer<ResultSet<PatchLineComment>> results(
final PatchLineComment... comments) {
return new IAnswer<ResultSet<PatchLineComment>>() {
@@ -305,7 +376,7 @@ public class CommentsTest {
fail("Expected no comment");
}
CommentInfo actual = getComment.apply(commentRes);
assertComment(expected, actual);
assertComment(expected, actual, true);
} catch (ResourceNotFoundException e) {
if (expected != null) {
fail("Expected to find comment");
@@ -330,17 +401,42 @@ public class CommentsTest {
assertNotNull(actualComments);
assertEquals(expectedComments.size(), actualComments.size());
for (int i = 0; i < expectedComments.size(); i++) {
assertComment(expectedComments.get(i), actualComments.get(i));
assertComment(expectedComments.get(i), actualComments.get(i), true);
}
}
}
private static void assertComment(PatchLineComment plc, CommentInfo ci) {
private static void assertListDrafts(Injector inj, RevisionResource res,
Map<String, ArrayList<PatchLineComment>> expected) throws Exception {
Drafts drafts = inj.getInstance(Drafts.class);
RestReadView<RevisionResource> listView =
(RestReadView<RevisionResource>) drafts.list();
@SuppressWarnings("unchecked")
Map<String, List<CommentInfo>> actual =
(Map<String, List<CommentInfo>>) listView.apply(res);
assertNotNull(actual);
assertEquals(expected.size(), actual.size());
assertEquals(expected.keySet(), actual.keySet());
for (Map.Entry<String, ArrayList<PatchLineComment>> entry : expected.entrySet()) {
List<PatchLineComment> expectedComments = entry.getValue();
List<CommentInfo> actualComments = actual.get(entry.getKey());
assertNotNull(actualComments);
assertEquals(expectedComments.size(), actualComments.size());
for (int i = 0; i < expectedComments.size(); i++) {
assertComment(expectedComments.get(i), actualComments.get(i), false);
}
}
}
private static void assertComment(PatchLineComment plc, CommentInfo ci,
boolean isPublished) {
assertEquals(plc.getKey().get(), ci.id);
assertEquals(plc.getParentUuid(), ci.inReplyTo);
assertEquals(plc.getMessage(), ci.message);
if (isPublished) {
assertNotNull(ci.author);
assertEquals(plc.getAuthor(), ci.author._id);
}
assertEquals(plc.getLine(), (int) ci.line);
assertEquals(plc.getSide() == 0 ? Side.PARENT : Side.REVISION,
Objects.firstNonNull(ci.side, Side.REVISION));
@@ -351,7 +447,8 @@ public class CommentsTest {
private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
String uuid, String inReplyToUuid, String filename, Side side, int line,
Account.Id authorId, long millis, String message, CommentRange range) {
Account.Id authorId, long millis, String message, CommentRange range,
PatchLineComment.Status status) {
Patch.Key p = new Patch.Key(psId, filename);
PatchLineComment.Key id = new PatchLineComment.Key(p, uuid);
PatchLineComment plc =
@@ -359,8 +456,15 @@ public class CommentsTest {
plc.setMessage(message);
plc.setRange(range);
plc.setSide(side == Side.PARENT ? (short) 0 : (short) 1);
plc.setStatus(Status.PUBLISHED);
plc.setStatus(status);
plc.setWrittenOn(new Timestamp(millis));
return plc;
}
private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
String uuid, String inReplyToUuid, String filename, Side side, int line,
Account.Id authorId, long millis, String message, CommentRange range) {
return newPatchLineComment(psId, uuid, inReplyToUuid, filename, side, line,
authorId, millis, message, range, Status.PUBLISHED);
}
}

4
gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java
View File

@@ -26,8 +26,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
new FakeQueryBuilder.Definition<>(
FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
null, null, null, null, null, null, null, null, indexes, null, null,
null, null),
null, null, null, null, null, null, null, null, null, indexes, null,
null, null, null),
null);
}

127
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@@ -62,13 +62,12 @@ import com.google.gerrit.server.git.GitModule;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.CommentsInNotesUtil;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gerrit.testutil.TestChanges;
import com.google.gerrit.testutil.FakeAccountCache;
import com.google.gerrit.testutil.FakeRealm;
import com.google.gerrit.testutil.InMemoryRepositoryManager;
import com.google.gerrit.testutil.TestChanges;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.StandardKeyEncoder;
@@ -77,13 +76,16 @@ import com.google.inject.Injector;
import com.google.inject.util.Providers;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.joda.time.DateTime;
import org.joda.time.DateTimeUtils;
import org.joda.time.DateTimeUtils.MillisProvider;
@@ -694,6 +696,58 @@ public class ChangeNotesTest {
assertEquals((short) 2, psas.get(1).getValue());
}
@Test
public void multipleUpdatesAcrossRefs() throws Exception {
Change c1 = newChange();
ChangeUpdate update1 = newUpdate(c1, changeOwner);
update1.putApproval("Verified", (short) 1);
Change c2 = newChange();
ChangeUpdate update2 = newUpdate(c2, otherUser);
update2.putApproval("Code-Review", (short) 2);
BatchMetaDataUpdate batch1 = null;
BatchMetaDataUpdate batch2 = null;
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
try {
batch1 = update1.openUpdateInBatch(bru);
batch1.write(update1, new CommitBuilder());
batch1.commit();
assertNull(repo.getRef(update1.getRefName()));
batch2 = update2.openUpdateInBatch(bru);
batch2.write(update2, new CommitBuilder());
batch2.commit();
assertNull(repo.getRef(update2.getRefName()));
} finally {
if (batch1 != null) {
batch1.close();
}
if (batch2 != null) {
batch2.close();
}
}
List<ReceiveCommand> cmds = bru.getCommands();
assertEquals(2, cmds.size());
assertEquals(update1.getRefName(), cmds.get(0).getRefName());
assertEquals(update2.getRefName(), cmds.get(1).getRefName());
RevWalk rw = new RevWalk(repo);
try {
bru.execute(rw, NullProgressMonitor.INSTANCE);
} finally {
rw.release();
}
assertEquals(ReceiveCommand.Result.OK, cmds.get(0).getResult());
assertEquals(ReceiveCommand.Result.OK, cmds.get(1).getResult());
assertNotNull(repo.getRef(update1.getRefName()));
assertNotNull(repo.getRef(update2.getRefName()));
}
@Test
public void changeMessageOnePatchSet() throws Exception {
Change c = newChange();
@@ -895,7 +949,9 @@ public class ChangeNotesTest {
public void patchLineCommentNotesFormatSide1() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid";
String uuid1 = "uuid1";
String uuid2 = "uuid2";
String uuid3 = "uuid3";
String message1 = "comment 1";
String message2 = "comment 2";
String message3 = "comment 3";
@@ -906,7 +962,7 @@ public class ChangeNotesTest {
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
uuid, range1, range1.getEndLine(), otherUser, null, time1, message1,
uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment1);
@@ -915,7 +971,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
uuid, range2, range2.getEndLine(), otherUser, null, time2, message2,
uuid2, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment2);
@@ -924,7 +980,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
CommentRange range3 = new CommentRange(3, 1, 4, 1);
PatchLineComment comment3 = newPublishedPatchLineComment(psId, "file2",
uuid, range3, range3.getEndLine(), otherUser, null, time3, message3,
uuid3, range3, range3.getEndLine(), otherUser, null, time3, message3,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment3);
@@ -948,14 +1004,14 @@ public class ChangeNotesTest {
+ "1:1-2:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n"
+ "UUID: uuid1\n"
+ "Bytes: 9\n"
+ "comment 1\n"
+ "\n"
+ "2:1-3:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n"
+ "UUID: uuid2\n"
+ "Bytes: 9\n"
+ "comment 2\n"
+ "\n"
@@ -964,7 +1020,7 @@ public class ChangeNotesTest {
+ "3:1-4:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n"
+ "UUID: uuid3\n"
+ "Bytes: 9\n"
+ "comment 3\n"
+ "\n",
@@ -975,7 +1031,8 @@ public class ChangeNotesTest {
public void patchLineCommentNotesFormatSide0() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid";
String uuid1 = "uuid1";
String uuid2 = "uuid2";
String message1 = "comment 1";
String message2 = "comment 2";
CommentRange range1 = new CommentRange(1, 1, 2, 1);
@@ -984,7 +1041,7 @@ public class ChangeNotesTest {
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
uuid, range1, range1.getEndLine(), otherUser, null, time1, message1,
uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment1);
@@ -993,7 +1050,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
uuid, range2, range2.getEndLine(), otherUser, null, time2, message2,
uuid2, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment2);
@@ -1017,14 +1074,14 @@ public class ChangeNotesTest {
+ "1:1-2:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n"
+ "UUID: uuid1\n"
+ "Bytes: 9\n"
+ "comment 1\n"
+ "\n"
+ "2:1-3:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n"
+ "UUID: uuid2\n"
+ "Bytes: 9\n"
+ "comment 2\n"
+ "\n",
@@ -1037,7 +1094,8 @@ public class ChangeNotesTest {
throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid";
String uuid1 = "uuid1";
String uuid2 = "uuid2";
String messageForBase = "comment for base";
String messageForPS = "comment for ps";
CommentRange range = new CommentRange(1, 1, 2, 1);
@@ -1045,7 +1103,7 @@ public class ChangeNotesTest {
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment commentForBase =
newPublishedPatchLineComment(psId, "filename", uuid,
newPublishedPatchLineComment(psId, "filename", uuid1,
range, range.getEndLine(), otherUser, null, now, messageForBase,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
@@ -1054,7 +1112,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
PatchLineComment commentForPS =
newPublishedPatchLineComment(psId, "filename", uuid,
newPublishedPatchLineComment(psId, "filename", uuid2,
range, range.getEndLine(), otherUser, null, now, messageForPS,
(short) 1, "abcd4567abcd4567abcd4567abcd4567abcd4567");
update.setPatchSetId(psId);
@@ -1078,7 +1136,8 @@ public class ChangeNotesTest {
@Test
public void patchLineCommentMultipleOnePatchsetOneFile() throws Exception {
Change c = newChange();
String uuid = "uuid";
String uuid1 = "uuid1";
String uuid2 = "uuid2";
CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id psId = c.currentPatchSetId();
String filename = "filename";
@@ -1088,7 +1147,7 @@ public class ChangeNotesTest {
Timestamp timeForComment1 = TimeUtil.nowTs();
Timestamp timeForComment2 = TimeUtil.nowTs();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, filename,
uuid, range, range.getEndLine(), otherUser, null, timeForComment1,
uuid1, range, range.getEndLine(), otherUser, null, timeForComment1,
"comment 1", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment1);
@@ -1096,7 +1155,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, filename,
uuid, range, range.getEndLine(), otherUser, null, timeForComment2,
uuid2, range, range.getEndLine(), otherUser, null, timeForComment2,
"comment 2", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(comment2);
@@ -1370,6 +1429,34 @@ public class ChangeNotesTest {
assertTrue(notes.getDraftPsComments(otherUserId).values().isEmpty());
}
@Test
public void patchLineCommentNoRange() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid";
String messageForBase = "comment for base";
Timestamp now = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment commentForBase =
newPublishedPatchLineComment(psId, "filename", uuid,
null, 1, otherUser, null, now, messageForBase,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId);
update.upsertComment(commentForBase);
update.commit();
ChangeNotes notes = newNotes(c);
Multimap<PatchSet.Id, PatchLineComment> commentsForBase =
notes.getBaseComments();
Multimap<PatchSet.Id, PatchLineComment> commentsForPs =
notes.getPatchSetComments();
assertTrue(commentsForPs.isEmpty());
assertEquals(commentForBase,
Iterables.getOnlyElement(commentsForBase.get(psId)));
}
private Change newChange() {
return TestChanges.newChange(project, changeOwner);
}

1
tools/eclipse/BUCK
View File

@@ -11,6 +11,7 @@ java_library(
'//gerrit-main:main_lib',
'//gerrit-patch-jgit:jgit_patch_tests',
'//gerrit-plugin-gwtui:gwtui-api-lib',
'//gerrit-reviewdb:client_tests',
'//gerrit-server:server',
'//gerrit-server:server_tests',
'//lib/asciidoctor:asciidoc_lib',