Merge "Simplify documentation indexer"
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
index bd5b512..41bc5af 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -32,6 +32,7 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gwtorm.server.OrmException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -76,6 +77,7 @@
@Assisted("changeId") String changeId);
}
+ private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil;
private final ReviewDb db;
private final PersonIdent i;
@@ -87,24 +89,27 @@
private String tagName;
@AssistedInject
- PushOneCommit(ApprovalsUtil approvalsUtil,
+ PushOneCommit(ChangeNotes.Factory notesFactory,
+ ApprovalsUtil approvalsUtil,
@Assisted ReviewDb db,
@Assisted PersonIdent i) {
- this(approvalsUtil, db, i, SUBJECT, FILE_NAME, FILE_CONTENT);
+ this(notesFactory, approvalsUtil, db, i, SUBJECT, FILE_NAME, FILE_CONTENT);
}
@AssistedInject
- PushOneCommit(ApprovalsUtil approvalsUtil,
+ PushOneCommit(ChangeNotes.Factory notesFactory,
+ ApprovalsUtil approvalsUtil,
@Assisted ReviewDb db,
@Assisted PersonIdent i,
@Assisted("subject") String subject,
@Assisted("fileName") String fileName,
@Assisted("content") String content) {
- this(approvalsUtil, db, i, subject, fileName, content, null);
+ this(notesFactory, approvalsUtil, db, i, subject, fileName, content, null);
}
@AssistedInject
- PushOneCommit(ApprovalsUtil approvalsUtil,
+ PushOneCommit(ChangeNotes.Factory notesFactory,
+ ApprovalsUtil approvalsUtil,
@Assisted ReviewDb db,
@Assisted PersonIdent i,
@Assisted("subject") String subject,
@@ -112,6 +117,7 @@
@Assisted("content") String content,
@Assisted("changeId") String changeId) {
this.db = db;
+ this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.i = i;
this.subject = subject;
@@ -133,26 +139,21 @@
if (tagName != null) {
git.tag().setName(tagName).setAnnotated(false).call();
}
- return new Result(db, approvalsUtil, ref,
- pushHead(git, ref, tagName != null), c, subject);
+ return new Result(ref, pushHead(git, ref, tagName != null), c, subject);
}
public void setTag(final String tagName) {
this.tagName = tagName;
}
- public static class Result {
- private final ReviewDb db;
- private final ApprovalsUtil approvalsUtil;
+ public class Result {
private final String ref;
private final PushResult result;
private final Commit commit;
private final String subject;
- private Result(ReviewDb db, ApprovalsUtil approvalsUtil, String ref,
- PushResult result, Commit commit, String subject) {
- this.db = db;
- this.approvalsUtil = approvalsUtil;
+ private Result(String ref, PushResult result, Commit commit,
+ String subject) {
this.ref = ref;
this.result = result;
this.commit = commit;
@@ -199,7 +200,7 @@
}));
for (Account.Id accountId
- : approvalsUtil.getReviewers(db, c.getId()).values()) {
+ : approvalsUtil.getReviewers(db, notesFactory.create(c)).values()) {
assertTrue("unexpected reviewer " + accountId,
expectedReviewerIds.remove(accountId));
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
index 5eff748..c9d3ffd 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
@@ -42,6 +42,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
@@ -87,6 +88,9 @@
private GroupCache groupCache;
@Inject
+ private ChangeNotes.Factory changeNotesFactory;
+
+ @Inject
private @GerritPersonIdent PersonIdent serverIdent;
@Inject
@@ -258,7 +262,9 @@
}
private void assertSubmitApproval(PatchSet.Id patchSetId) throws OrmException {
- PatchSetApproval a = approvalsUtil.getSubmitter(db, patchSetId);
+ Change c = db.changes().get(patchSetId.getParentKey());
+ ChangeNotes notes = changeNotesFactory.create(c).load();
+ PatchSetApproval a = approvalsUtil.getSubmitter(db, notes, patchSetId);
assertTrue(a.isSubmit());
assertEquals(1, a.getValue());
assertEquals(admin.id, a.getAccountId());
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
index 408ad39..f379b7c 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
@@ -34,13 +34,18 @@
}
public static String checkName(String name) {
- if (name == null || name.isEmpty()) {
- throw new IllegalArgumentException("Empty label name");
- }
+ checkNameInternal(name);
if ("SUBM".equals(name)) {
throw new IllegalArgumentException(
"Reserved label name \"" + name + "\"");
}
+ return name;
+ }
+
+ public static String checkNameInternal(String name) {
+ if (name == null || name.isEmpty()) {
+ throw new IllegalArgumentException("Empty label name");
+ }
for (int i = 0; i < name.length(); i++) {
char c = name.charAt(i);
if ((i == 0 && c == '-') ||
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
index ce0ea63..aad2692 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
@@ -14,7 +14,6 @@
package com.google.gerrit.client.diff;
-import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.GWT;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index 25654d2..dd2c32b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -18,10 +18,14 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
+import com.google.common.base.Objects;
+import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.common.collect.SetMultimap;
@@ -36,6 +40,9 @@
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeKind;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
@@ -57,7 +64,7 @@
* mark the "no score" case, a dummy approval, which may live in any of
* the available categories, with a score of 0 is used.
* <p>
- * The methods in this class do not begin/commit transactions.
+ * The methods in this class only modify the gwtorm database.
*/
public class ApprovalsUtil {
private static Ordering<PatchSetApproval> SORT_APPROVALS = Ordering.natural()
@@ -73,24 +80,40 @@
return SORT_APPROVALS.sortedCopy(approvals);
}
+ private static Iterable<PatchSetApproval> filterApprovals(
+ Iterable<PatchSetApproval> psas, final Account.Id accountId) {
+ return Iterables.filter(psas, new Predicate<PatchSetApproval>() {
+ @Override
+ public boolean apply(PatchSetApproval input) {
+ return Objects.equal(input.getAccountId(), accountId);
+ }
+ });
+ }
+
+ private final NotesMigration migration;
+
@VisibleForTesting
@Inject
- public ApprovalsUtil() {
+ public ApprovalsUtil(NotesMigration migration) {
+ this.migration = migration;
}
/**
* Get all reviewers for a change.
*
* @param db review database.
- * @param changeId change ID.
+ * @param notes change notes.
* @return multimap of reviewers keyed by state, where each account appears
* exactly once in {@link SetMultimap#values()}, and
* {@link ReviewerState#REMOVED} is not present.
* @throws OrmException if reviewers for the change could not be read.
*/
public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers(
- ReviewDb db, Change.Id changeId) throws OrmException {
- return getReviewers(db.patchSetApprovals().byChange(changeId));
+ ReviewDb db, ChangeNotes notes) throws OrmException {
+ if (!migration.readPatchSetApprovals()) {
+ return getReviewers(db.patchSetApprovals().byChange(notes.getChangeId()));
+ }
+ return notes.load().getReviewers();
}
/**
@@ -132,12 +155,11 @@
*
* @throws OrmException
*/
- public void copyLabels(ReviewDb db, LabelTypes labelTypes,
+ public void copyLabels(ReviewDb db, ChangeNotes notes, LabelTypes labelTypes,
PatchSet.Id source, PatchSet dest, ChangeKind changeKind)
throws OrmException {
- Iterable<PatchSetApproval> sourceApprovals =
- db.patchSetApprovals().byPatchSet(source);
- copyLabels(db, labelTypes, sourceApprovals, source, dest, changeKind);
+ copyLabels(db, labelTypes, byPatchSet(db, notes, source), source, dest,
+ changeKind);
}
/**
@@ -170,27 +192,24 @@
db.patchSetApprovals().insert(copied);
}
- public List<PatchSetApproval> addReviewers(ReviewDb db, LabelTypes labelTypes,
- Change change, PatchSet ps, PatchSetInfo info,
- Iterable<Account.Id> wantReviewers,
+ public List<PatchSetApproval> addReviewers(ReviewDb db,
+ ChangeUpdate update, LabelTypes labelTypes, Change change, PatchSet ps,
+ PatchSetInfo info, Iterable<Account.Id> wantReviewers,
Collection<Account.Id> existingReviewers) throws OrmException {
- return addReviewers(db, labelTypes, change, ps.getId(), ps.isDraft(),
- info.getAuthor().getAccount(), info.getCommitter().getAccount(),
- wantReviewers, existingReviewers);
+ return addReviewers(db, update, labelTypes, change, ps.getId(),
+ ps.isDraft(), info.getAuthor().getAccount(),
+ info.getCommitter().getAccount(), wantReviewers, existingReviewers);
}
- public List<PatchSetApproval> addReviewers(ReviewDb db, LabelTypes labelTypes,
- Change change, Iterable<Account.Id> wantReviewers) throws OrmException {
+ public List<PatchSetApproval> addReviewers(ReviewDb db, ChangeNotes notes,
+ ChangeUpdate update, LabelTypes labelTypes, Change change,
+ Iterable<Account.Id> wantReviewers) throws OrmException {
PatchSet.Id psId = change.currentPatchSetId();
- Set<Account.Id> existing = Sets.newHashSet();
- for (PatchSetApproval psa : db.patchSetApprovals().byPatchSet(psId)) {
- existing.add(psa.getAccountId());
- }
- return addReviewers(db, labelTypes, change, psId, false, null, null,
- wantReviewers, existing);
+ return addReviewers(db, update, labelTypes, change, psId, false, null, null,
+ wantReviewers, getReviewers(db, notes).values());
}
- private List<PatchSetApproval> addReviewers(ReviewDb db,
+ private List<PatchSetApproval> addReviewers(ReviewDb db, ChangeUpdate update,
LabelTypes labelTypes, Change change, PatchSet.Id psId, boolean isDraft,
Account.Id authorId, Account.Id committerId,
Iterable<Account.Id> wantReviewers,
@@ -220,23 +239,53 @@
cells.add(new PatchSetApproval(
new PatchSetApproval.Key(psId, account, labelId),
(short) 0, TimeUtil.nowTs()));
+ update.putReviewer(account, ReviewerState.REVIEWER);
}
db.patchSetApprovals().insert(cells);
return Collections.unmodifiableList(cells);
}
- public List<PatchSetApproval> byPatchSet(ReviewDb db, PatchSet.Id psId)
- throws OrmException {
- return sortApprovals(db.patchSetApprovals().byPatchSet(psId));
+ public ListMultimap<PatchSet.Id, PatchSetApproval> byChange(ReviewDb db,
+ ChangeNotes notes) throws OrmException {
+ if (!migration.readPatchSetApprovals()) {
+ ImmutableListMultimap.Builder<PatchSet.Id, PatchSetApproval> result =
+ ImmutableListMultimap.builder();
+ for (PatchSetApproval psa
+ : db.patchSetApprovals().byChange(notes.getChangeId())) {
+ result.put(psa.getPatchSetId(), psa);
+ }
+ return result.build();
+ }
+ return notes.load().getApprovals();
}
- public PatchSetApproval getSubmitter(ReviewDb db, PatchSet.Id c) {
+ public List<PatchSetApproval> byPatchSet(ReviewDb db, ChangeNotes notes,
+ PatchSet.Id psId) throws OrmException {
+ if (!migration.readPatchSetApprovals()) {
+ return sortApprovals(db.patchSetApprovals().byPatchSet(psId));
+ }
+ return notes.load().getApprovals().get(psId);
+ }
+
+ public List<PatchSetApproval> byPatchSetUser(ReviewDb db,
+ ChangeNotes notes, PatchSet.Id psId, Account.Id accountId)
+ throws OrmException {
+ if (!migration.readPatchSetApprovals()) {
+ return sortApprovals(
+ db.patchSetApprovals().byPatchSetUser(psId, accountId));
+ }
+ return ImmutableList.copyOf(
+ filterApprovals(byPatchSet(db, notes, psId), accountId));
+ }
+
+ public PatchSetApproval getSubmitter(ReviewDb db, ChangeNotes notes,
+ PatchSet.Id c) {
if (c == null) {
return null;
}
PatchSetApproval submitter = null;
try {
- for (PatchSetApproval a : db.patchSetApprovals().byPatchSet(c)) {
+ for (PatchSetApproval a : byPatchSet(db, notes, c)) {
if (a.getValue() > 0 && a.isSubmit()) {
if (submitter == null
|| a.getGranted().compareTo(submitter.getGranted()) > 0) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
index a17c34d..e57dc7f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
@@ -489,6 +489,7 @@
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.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(patchSetId));
db.patchSetAncestors().delete(db.patchSetAncestors().byPatchSet(patchSetId));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index 18cfefd..42f5c23 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -28,8 +28,10 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.mail.CreateChangeSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.RefControl;
import com.google.gwtorm.server.OrmException;
@@ -55,6 +57,7 @@
LoggerFactory.getLogger(ChangeInserter.class);
private final Provider<ReviewDb> dbProvider;
+ private final ChangeUpdate.Factory updateFactory;
private final GitReferenceUpdated gitRefUpdated;
private final ChangeHooks hooks;
private final ApprovalsUtil approvalsUtil;
@@ -75,6 +78,7 @@
@Inject
ChangeInserter(Provider<ReviewDb> dbProvider,
+ ChangeUpdate.Factory updateFactory,
Provider<ApprovalsUtil> approvals,
PatchSetInfoFactory patchSetInfoFactory,
GitReferenceUpdated gitRefUpdated,
@@ -86,6 +90,7 @@
@Assisted Change change,
@Assisted RevCommit commit) {
this.dbProvider = dbProvider;
+ this.updateFactory = updateFactory;
this.gitRefUpdated = gitRefUpdated;
this.hooks = hooks;
this.approvalsUtil = approvalsUtil;
@@ -154,14 +159,16 @@
public Change insert() throws OrmException, IOException {
ReviewDb db = dbProvider.get();
+ ChangeUpdate update = updateFactory.create(change, change.getCreatedOn(),
+ (IdentifiedUser) refControl.getCurrentUser());
db.changes().beginTransaction(change.getId());
try {
ChangeUtil.insertAncestors(db, patchSet.getId(), commit);
db.patchSets().insert(Collections.singleton(patchSet));
db.changes().insert(Collections.singleton(change));
LabelTypes labelTypes = refControl.getProjectControl().getLabelTypes();
- approvalsUtil.addReviewers(db, labelTypes, change, patchSet,
- patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
+ approvalsUtil.addReviewers(db, update, labelTypes, change,
+ patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
if (messageIsForChange()) {
insertMessage(db);
}
@@ -170,6 +177,7 @@
db.rollback();
}
+ update.commit();
CheckedFuture<?, IOException> f =
mergeabilityChecker.updateAndIndexAsync(change);
if (!messageIsForChange()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
index 37553b2..fb8397f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
@@ -23,6 +23,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.TypeLiteral;
@@ -51,6 +52,10 @@
return getControl().getChange();
}
+ public ChangeNotes getNotes() {
+ return getControl().getNotes();
+ }
+
@Override
public String getETag() {
CurrentUser user = control.getCurrentUser();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
index 18eec41..c396eaa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
@@ -26,10 +26,12 @@
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.DeleteReviewer.Input;
import com.google.gerrit.server.index.ChangeIndexer;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
@@ -45,13 +47,20 @@
}
private final Provider<ReviewDb> dbProvider;
+ private final ChangeUpdate.Factory updateFactory;
+ private final ApprovalsUtil approvalsUtil;
private final ChangeIndexer indexer;
private final IdentifiedUser.GenericFactory userFactory;
@Inject
- DeleteReviewer(Provider<ReviewDb> dbProvider, ChangeIndexer indexer,
+ DeleteReviewer(Provider<ReviewDb> dbProvider,
+ ChangeUpdate.Factory updateFactory,
+ ApprovalsUtil approvalsUtil,
+ ChangeIndexer indexer,
IdentifiedUser.GenericFactory userFactory) {
this.dbProvider = dbProvider;
+ this.updateFactory = updateFactory;
+ this.approvalsUtil = approvalsUtil;
this.indexer = indexer;
this.userFactory = userFactory;
}
@@ -63,6 +72,8 @@
ChangeControl control = rsrc.getControl();
Change.Id changeId = rsrc.getChange().getId();
ReviewDb db = dbProvider.get();
+ ChangeUpdate update = updateFactory.create(rsrc.getChange());
+
StringBuilder msg = new StringBuilder();
db.changes().beginTransaction(changeId);
try {
@@ -88,6 +99,7 @@
}
ChangeUtil.bumpRowVersionNotLastUpdatedOn(rsrc.getChange().getId(), db);
db.patchSetApprovals().delete(del);
+ update.removeReviewer(rsrc.getUser().getAccountId());
if (msg.length() > 0) {
ChangeMessage changeMessage =
@@ -103,6 +115,7 @@
} finally {
db.rollback();
}
+ update.commit();
indexer.index(db, rsrc.getChange());
return Response.none();
}
@@ -119,7 +132,7 @@
ReviewerResource rsrc) throws OrmException {
final Account.Id user = rsrc.getUser().getAccountId();
return Iterables.filter(
- db.patchSetApprovals().byChange(rsrc.getChange().getId()),
+ approvalsUtil.byChange(db, rsrc.getNotes()).values(),
new Predicate<PatchSetApproval>() {
@Override
public boolean apply(PatchSetApproval input) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
index ddc1c12..25d4e2c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
@@ -17,7 +17,6 @@
import com.google.common.collect.Maps;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
@@ -49,9 +48,8 @@
public List<ReviewerInfo> apply(ChangeResource rsrc) throws OrmException {
Map<Account.Id, ReviewerResource> reviewers = Maps.newLinkedHashMap();
ReviewDb db = dbProvider.get();
- Change.Id changeId = rsrc.getChange().getId();
for (Account.Id accountId
- : approvalsUtil.getReviewers(db, changeId).values()) {
+ : approvalsUtil.getReviewers(db, rsrc.getNotes()).values()) {
if (!reviewers.containsKey(accountId)) {
reviewers.put(accountId, resourceFactory.create(rsrc, accountId));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index ddfb955..e90a0eb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -35,6 +35,7 @@
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
@@ -81,6 +82,7 @@
private final ChangeHooks hooks;
private final PatchSetInfoFactory patchSetInfoFactory;
private final ReviewDb db;
+ private final ChangeUpdate.Factory updateFactory;
private final GitReferenceUpdated gitRefUpdated;
private final CommitValidators.Factory commitValidatorsFactory;
private final MergeabilityChecker mergeabilityChecker;
@@ -106,12 +108,13 @@
@Inject
public PatchSetInserter(ChangeHooks hooks,
ReviewDb db,
+ ChangeUpdate.Factory updateFactory,
+ ApprovalsUtil approvalsUtil,
PatchSetInfoFactory patchSetInfoFactory,
GitReferenceUpdated gitRefUpdated,
CommitValidators.Factory commitValidatorsFactory,
MergeabilityChecker mergeabilityChecker,
ReplacePatchSetSender.Factory replacePatchSetFactory,
- ApprovalsUtil approvalsUtil,
ChangeKindCache changeKindCache,
@Assisted Repository git,
@Assisted RevWalk revWalk,
@@ -122,12 +125,13 @@
ctl.getChange().getId());
this.hooks = hooks;
this.db = db;
+ this.updateFactory = updateFactory;
+ this.approvalsUtil = approvalsUtil;
this.patchSetInfoFactory = patchSetInfoFactory;
this.gitRefUpdated = gitRefUpdated;
this.commitValidatorsFactory = commitValidatorsFactory;
this.mergeabilityChecker = mergeabilityChecker;
this.replacePatchSetFactory = replacePatchSetFactory;
- this.approvalsUtil = approvalsUtil;
this.changeKindCache = changeKindCache;
this.git = git;
@@ -221,6 +225,9 @@
final PatchSet.Id currentPatchSetId = c.currentPatchSetId();
+ ChangeUpdate update = updateFactory.create(c, patchSet.getCreatedOn(),
+ (IdentifiedUser) ctl.getCurrentUser());
+
db.changes().beginTransaction(c.getId());
try {
if (!db.changes().get(c.getId()).getStatus().isOpen()) {
@@ -232,7 +239,7 @@
db.patchSets().insert(Collections.singleton(patchSet));
SetMultimap<ReviewerState, Account.Id> oldReviewers = sendMail
- ? approvalsUtil.getReviewers(db, c.getId())
+ ? approvalsUtil.getReviewers(db, ctl.getNotes())
: null;
updatedChange =
@@ -273,11 +280,12 @@
ctl.getProjectControl().getProjectState();
ChangeKind changeKind = changeKindCache.getChangeKind(
projectState, git, priorCommit, commit);
-
- approvalsUtil.copyLabels(db, ctl.getProjectControl() .getLabelTypes(),
- currentPatchSetId, patchSet, changeKind);
+ approvalsUtil.copyLabels(db, ctl.getNotes(),
+ ctl.getProjectControl().getLabelTypes(), currentPatchSetId,
+ patchSet, changeKind);
}
db.commit();
+ update.commit();
if (!messageIsForChange()) {
insertMessage(db);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 0203e3b..4828558 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -44,10 +44,12 @@
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
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.project.ChangeControl;
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.TimeUtil;
@@ -74,6 +76,8 @@
private final Provider<ReviewDb> db;
private final ChangesCollection changes;
+ private final ChangeUpdate.Factory updateFactory;
+ private final ApprovalsUtil approvalsUtil;
private final ChangeIndexer indexer;
private final AccountsCollection accounts;
private final EmailReviewComments.Factory email;
@@ -89,12 +93,16 @@
@Inject
PostReview(Provider<ReviewDb> db,
ChangesCollection changes,
+ ChangeUpdate.Factory updateFactory,
+ ApprovalsUtil approvalsUtil,
ChangeIndexer indexer,
AccountsCollection accounts,
EmailReviewComments.Factory email,
ChangeHooks hooks) {
this.db = db;
this.changes = changes;
+ this.updateFactory = updateFactory;
+ this.approvalsUtil = approvalsUtil;
this.indexer = indexer;
this.accounts = accounts;
this.email = email;
@@ -119,6 +127,7 @@
input.notify = NotifyHandling.NONE;
}
+ ChangeUpdate update = null;
db.get().changes().beginTransaction(revision.getChange().getId());
boolean dirty = false;
try {
@@ -126,8 +135,9 @@
ChangeUtil.updated(change);
timestamp = change.getLastUpdatedOn();
+ update = updateFactory.create(change, timestamp, revision.getUser());
dirty |= insertComments(revision, input.comments, input.drafts);
- dirty |= updateLabels(revision, input.labels);
+ dirty |= updateLabels(revision, update, input.labels);
dirty |= insertMessage(revision, input.message);
if (dirty) {
db.get().changes().update(Collections.singleton(change));
@@ -136,6 +146,9 @@
} finally {
db.get().rollback();
}
+ if (update != null) {
+ update.commit();
+ }
CheckedFuture<?, IOException> indexWrite;
if (dirty) {
@@ -365,8 +378,8 @@
return drafts;
}
- private boolean updateLabels(RevisionResource rsrc, Map<String, Short> labels)
- throws OrmException {
+ private boolean updateLabels(RevisionResource rsrc, ChangeUpdate update,
+ Map<String, Short> labels) throws OrmException {
if (labels == null) {
labels = Collections.emptyMap();
}
@@ -394,6 +407,7 @@
addLabelDelta(normName, (short) 0);
}
del.add(c);
+ update.putApproval(ent.getKey(), (short) 0);
}
} else if (c != null && c.getValue() != ent.getValue()) {
c.setValue(ent.getValue());
@@ -401,6 +415,7 @@
upd.add(c);
addLabelDelta(normName, c.getValue());
categories.put(normName, c.getValue());
+ update.putApproval(ent.getKey(), ent.getValue());
} else if (c != null && c.getValue() == ent.getValue()) {
current.put(normName, c);
} else if (c == null) {
@@ -413,6 +428,7 @@
ins.add(c);
addLabelDelta(normName, c.getValue());
categories.put(normName, c.getValue());
+ update.putApproval(ent.getKey(), ent.getValue());
}
}
@@ -455,8 +471,10 @@
List<PatchSetApproval> del) throws OrmException {
LabelTypes labelTypes = rsrc.getControl().getLabelTypes();
Map<String, PatchSetApproval> current = Maps.newHashMap();
- for (PatchSetApproval a : db.get().patchSetApprovals().byPatchSetUser(
- rsrc.getPatchSet().getId(), rsrc.getAccountId())) {
+
+ for (PatchSetApproval a : approvalsUtil.byPatchSetUser(
+ db.get(), rsrc.getNotes(), rsrc.getPatchSet().getId(),
+ rsrc.getAccountId())) {
if (a.isSubmit()) {
continue;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index 46893df..16a65b2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -48,6 +48,7 @@
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.AddReviewerSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException;
@@ -79,6 +80,7 @@
private final GroupMembers.Factory groupMembersFactory;
private final AccountInfo.Loader.Factory accountLoaderFactory;
private final Provider<ReviewDb> dbProvider;
+ private final ChangeUpdate.Factory updateFactory;
private final IdentifiedUser currentUser;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final Config cfg;
@@ -96,6 +98,7 @@
GroupMembers.Factory groupMembersFactory,
AccountInfo.Loader.Factory accountLoaderFactory,
Provider<ReviewDb> db,
+ ChangeUpdate.Factory updateFactory,
IdentifiedUser currentUser,
IdentifiedUser.GenericFactory identifiedUserFactory,
@GerritServerConfig Config cfg,
@@ -111,6 +114,7 @@
this.groupMembersFactory = groupMembersFactory;
this.accountLoaderFactory = accountLoaderFactory;
this.dbProvider = db;
+ this.updateFactory = updateFactory;
this.currentUser = currentUser;
this.identifiedUserFactory = identifiedUserFactory;
this.cfg = cfg;
@@ -217,27 +221,30 @@
Map<Account.Id, ChangeControl> reviewers)
throws OrmException, EmailException, IOException {
ReviewDb db = dbProvider.get();
+ ChangeUpdate update = updateFactory.create(rsrc.getChange());
List<PatchSetApproval> added;
db.changes().beginTransaction(rsrc.getChange().getId());
try {
ChangeUtil.bumpRowVersionNotLastUpdatedOn(rsrc.getChange().getId(), db);
- added = approvalsUtil.addReviewers(db, rsrc.getControl().getLabelTypes(),
- rsrc.getChange(), reviewers.keySet());
+ added = approvalsUtil.addReviewers(db, rsrc.getNotes(), update,
+ rsrc.getControl().getLabelTypes(), rsrc.getChange(),
+ reviewers.keySet());
db.commit();
} finally {
db.rollback();
}
+ update.commit();
CheckedFuture<?, IOException> indexFuture =
indexer.indexAsync(rsrc.getChange().getId());
result.reviewers = Lists.newArrayListWithCapacity(added.size());
for (PatchSetApproval psa : added) {
result.reviewers.add(json.format(
new ReviewerInfo(psa.getAccountId()),
+ rsrc.getNotes(),
reviewers.get(psa.getAccountId()),
ImmutableList.of(psa)));
}
-
accountLoaderFactory.create(true).fill(result.reviewers);
postAdd(rsrc.getChange(), added);
indexFuture.checkedGet();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java
index f522de5..2f33ae7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.change.Publish.Input;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.PatchSetNotificationSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
@@ -43,16 +44,19 @@
}
private final Provider<ReviewDb> dbProvider;
+ private final ChangeUpdate.Factory updateFactory;
private final PatchSetNotificationSender sender;
private final ChangeHooks hooks;
private final ChangeIndexer indexer;
@Inject
public Publish(Provider<ReviewDb> dbProvider,
+ ChangeUpdate.Factory updateFactory,
PatchSetNotificationSender sender,
ChangeHooks hooks,
ChangeIndexer indexer) {
this.dbProvider = dbProvider;
+ this.updateFactory = updateFactory;
this.sender = sender;
this.hooks = hooks;
this.indexer = indexer;
@@ -72,6 +76,8 @@
PatchSet updatedPatchSet = updateDraftPatchSet(rsrc);
Change updatedChange = updateDraftChange(rsrc);
+ ChangeUpdate update = updateFactory.create(rsrc.getChange(),
+ updatedChange.getLastUpdatedOn());
try {
if (!updatedPatchSet.isDraft()
@@ -79,7 +85,8 @@
CheckedFuture<?, IOException> indexFuture =
indexer.indexAsync(updatedChange.getId());
hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, dbProvider.get());
- sender.send(rsrc.getChange().getStatus() == Change.Status.DRAFT,
+ sender.send(rsrc.getNotes(), update,
+ rsrc.getChange().getStatus() == Change.Status.DRAFT,
rsrc.getUser(), updatedChange, updatedPatchSet,
rsrc.getControl().getLabelTypes());
indexFuture.checkedGet();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
index a951d51..f44aeca 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.git.LabelNormalizer;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -44,16 +45,19 @@
public class ReviewerJson {
private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory;
+ private final ApprovalsUtil approvalsUtil;
private final LabelNormalizer labelNormalizer;
private final AccountInfo.Loader.Factory accountLoaderFactory;
@Inject
ReviewerJson(Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
+ ApprovalsUtil approvalsUtil,
LabelNormalizer labelNormalizer,
AccountInfo.Loader.Factory accountLoaderFactory) {
this.db = db;
this.changeDataFactory = changeDataFactory;
+ this.approvalsUtil = approvalsUtil;
this.labelNormalizer = labelNormalizer;
this.accountLoaderFactory = accountLoaderFactory;
}
@@ -75,14 +79,12 @@
return format(ImmutableList.<ReviewerResource> of(rsrc));
}
- public ReviewerInfo format(ReviewerInfo out, ChangeControl ctl,
- List<PatchSetApproval> approvals) throws OrmException {
+ public ReviewerInfo format(ReviewerInfo out, ChangeNotes changeNotes,
+ ChangeControl ctl, List<PatchSetApproval> approvals) throws OrmException {
PatchSet.Id psId = ctl.getChange().currentPatchSetId();
- if (approvals == null) {
- approvals = ApprovalsUtil.sortApprovals(db.get().patchSetApprovals()
- .byPatchSetUser(psId, out._id));
- }
+ approvals =
+ approvalsUtil.byPatchSetUser(db.get(), changeNotes, psId, out._id);
approvals = labelNormalizer.normalize(ctl, approvals);
LabelTypes labelTypes = ctl.getLabelTypes();
@@ -129,7 +131,7 @@
private ReviewerInfo format(ReviewerResource rsrc,
List<PatchSetApproval> approvals) throws OrmException {
return format(new ReviewerInfo(rsrc.getUser().getAccountId()),
- rsrc.getUserControl(), approvals);
+ rsrc.getNotes(), rsrc.getUserControl(), approvals);
}
public static class ReviewerInfo extends AccountInfo {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
index 76a194d..e7c913a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
@@ -81,6 +81,6 @@
private Collection<Account.Id> fetchAccountIds(ChangeResource rsrc)
throws OrmException {
return approvalsUtil.getReviewers(
- dbProvider.get(), rsrc.getChange().getId()).values();
+ dbProvider.get(), rsrc.getNotes()).values();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
index b15719f..eb9f4ea 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
@@ -21,6 +21,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.TypeLiteral;
@@ -53,6 +54,10 @@
return getControl().getChange();
}
+ public ChangeNotes getNotes() {
+ return getChangeResource().getNotes();
+ }
+
public PatchSet getPatchSet() {
return ps;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
index 8857f23..dd85fef 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
@@ -32,6 +32,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ProjectUtil;
@@ -39,6 +40,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeQueue;
import com.google.gerrit.server.index.ChangeIndexer;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.AtomicUpdate;
@@ -76,16 +78,22 @@
private final Provider<ReviewDb> dbProvider;
private final GitRepositoryManager repoManager;
+ private final ChangeUpdate.Factory updateFactory;
+ private final ApprovalsUtil approvalsUtil;
private final MergeQueue mergeQueue;
private final ChangeIndexer indexer;
@Inject
Submit(Provider<ReviewDb> dbProvider,
GitRepositoryManager repoManager,
+ ChangeUpdate.Factory updateFactory,
+ ApprovalsUtil approvalsUtil,
MergeQueue mergeQueue,
ChangeIndexer indexer) {
this.dbProvider = dbProvider;
this.repoManager = repoManager;
+ this.updateFactory = updateFactory;
+ this.approvalsUtil = approvalsUtil;
this.mergeQueue = mergeQueue;
this.indexer = indexer;
}
@@ -180,10 +188,11 @@
throws OrmException, IOException {
final Timestamp timestamp = TimeUtil.nowTs();
Change change = rsrc.getChange();
+ ChangeUpdate update = updateFactory.create(change, timestamp, caller);
ReviewDb db = dbProvider.get();
db.changes().beginTransaction(change.getId());
try {
- approve(rsrc.getPatchSet(), caller, timestamp);
+ approve(rsrc, update, caller, timestamp);
change = db.changes().atomicUpdate(
change.getId(),
new AtomicUpdate<Change>() {
@@ -209,27 +218,23 @@
return change;
}
- private void approve(PatchSet rev, IdentifiedUser caller, Timestamp timestamp)
- throws OrmException {
- PatchSetApproval submit = Iterables.getFirst(Iterables.filter(
- dbProvider.get().patchSetApprovals()
- .byPatchSetUser(rev.getId(), caller.getAccountId()),
- new Predicate<PatchSetApproval>() {
- @Override
- public boolean apply(PatchSetApproval input) {
- return input.isSubmit();
- }
- }), null);
- if (submit == null) {
+ private void approve(RevisionResource rsrc, ChangeUpdate update,
+ IdentifiedUser caller, Timestamp timestamp) throws OrmException {
+ PatchSetApproval submit = approvalsUtil.getSubmitter(
+ dbProvider.get(), rsrc.getNotes(), rsrc.getPatchSet().getId());
+ if (submit == null || submit.getAccountId() != caller.getAccountId()) {
submit = new PatchSetApproval(
new PatchSetApproval.Key(
- rev.getId(),
+ rsrc.getPatchSet().getId(),
caller.getAccountId(),
LabelId.SUBMIT),
(short) 1, TimeUtil.nowTs());
}
submit.setValue((short) 1);
submit.setGranted(timestamp);
+ // TODO(dborowitz): Don't use a label in notedb; just check when status
+ // change happened.
+ update.putApproval(submit.getLabel(), submit.getValue());
dbProvider.get().patchSetApprovals().upsert(Collections.singleton(submit));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index c5327b4..827ccd0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -101,6 +101,7 @@
import com.google.gerrit.server.mail.RegisterNewEmailSender;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
import com.google.gerrit.server.mail.VelocityRuntimeProvider;
+import com.google.gerrit.server.notedb.NoteDbModule;
import com.google.gerrit.server.patch.PatchListCacheImpl;
import com.google.gerrit.server.patch.PatchScriptFactory;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -166,6 +167,7 @@
install(new CmdLineParserModule());
install(new EmailModule());
install(new GitModule());
+ install(new NoteDbModule());
install(new PrologModule());
install(new SshAddressesModule());
install(ThreadLocalRequestContext.module());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
index f578a9e..9d48574 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
@@ -48,6 +48,7 @@
import com.google.gerrit.server.data.SubmitLabelAttribute;
import com.google.gerrit.server.data.SubmitRecordAttribute;
import com.google.gerrit.server.data.TrackingIdAttribute;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListEntry;
@@ -167,12 +168,12 @@
* Add allReviewers to an existing ChangeAttribute.
*
* @param a
- * @param change
+ * @param notes
*/
- public void addAllReviewers(ChangeAttribute a, Change change)
+ public void addAllReviewers(ChangeAttribute a, ChangeNotes notes)
throws OrmException {
Collection<Account.Id> reviewers =
- approvalsUtil.getReviewers(db.get(), change.getId()).values();
+ approvalsUtil.getReviewers(db.get(), notes).values();
if (!reviewers.isEmpty()) {
a.allReviewers = Lists.newArrayListWithCapacity(reviewers.size());
for (Account.Id id : reviewers) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CherryPick.java
index 18b862c..a72297d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CherryPick.java
@@ -132,8 +132,7 @@
args.rw.parseBody(n);
- final PatchSetApproval submitAudit =
- args.mergeUtil.getSubmitter(n.change.currentPatchSetId());
+ final PatchSetApproval submitAudit = args.mergeUtil.getSubmitter(n);
PersonIdent cherryPickCommitterIdent;
if (submitAudit != null) {
@@ -157,7 +156,7 @@
}
PatchSet.Id id =
- ChangeUtil.nextPatchSetId(args.repo, n.change.currentPatchSetId());
+ ChangeUtil.nextPatchSetId(args.repo, n.getChange().currentPatchSetId());
final PatchSet ps = new PatchSet(id);
ps.setCreatedOn(TimeUtil.nowTs());
ps.setUploader(submitAudit.getAccountId());
@@ -165,19 +164,22 @@
final RefUpdate ru;
- args.db.changes().beginTransaction(n.change.getId());
+ args.db.changes().beginTransaction(n.getChange().getId());
try {
insertAncestors(args.db, ps.getId(), newCommit);
args.db.patchSets().insert(Collections.singleton(ps));
- n.change
+ n.getChange()
.setCurrentPatchSet(patchSetInfoFactory.get(newCommit, ps.getId()));
- args.db.changes().update(Collections.singletonList(n.change));
+ args.db.changes().update(Collections.singletonList(n.getChange()));
final List<PatchSetApproval> approvals = Lists.newArrayList();
for (PatchSetApproval a
- : args.approvalsUtil.byPatchSet(args.db, n.patchsetId)) {
+ : args.approvalsUtil.byPatchSet(args.db, n.notes, n.patchsetId)) {
approvals.add(new PatchSetApproval(ps.getId(), a));
}
+ // TODO(dborowitz): This doesn't copy labels in the notedb. We should
+ // stamp those in atomically with the same commit that describes the
+ // change being submitted.
args.db.patchSetApprovals().insert(approvals);
ru = args.repo.updateRef(ps.getRefName());
@@ -186,7 +188,7 @@
ru.disableRefLog();
if (ru.update(args.rw) != RefUpdate.Result.NEW) {
throw new IOException(String.format(
- "Failed to create ref %s in %s: %s", ps.getRefName(), n.change
+ "Failed to create ref %s in %s: %s", ps.getRefName(), n.getChange()
.getDest().getParentKey().get(), ru.getResult()));
}
@@ -195,7 +197,7 @@
args.db.rollback();
}
- gitRefUpdated.fire(n.change.getProject(), ru);
+ gitRefUpdated.fire(n.getChange().getProject(), ru);
newCommit.copyFrom(n);
newCommit.statusCode = CommitMergeStatus.CLEAN_PICK;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java
index 311856d..789be78 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java
@@ -16,6 +16,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.notedb.ChangeNotes;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
@@ -39,8 +40,8 @@
*/
PatchSet.Id patchsetId;
- /** The change containing {@link #patchsetId} . */
- Change change;
+ /** Change info loaded from notes. */
+ public ChangeNotes notes;
/**
* Ordinal position of this commit within the submit queue.
@@ -65,9 +66,13 @@
void copyFrom(final CodeReviewCommit src) {
patchsetId = src.patchsetId;
- change = src.change;
+ notes = src.notes;
originalOrder = src.originalOrder;
statusCode = src.statusCode;
missing = src.missing;
}
+
+ Change getChange() {
+ return notes.getChange();
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 987bfa5..92ea302 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -52,6 +52,8 @@
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.MergeFailSender;
import com.google.gerrit.server.mail.MergedSender;
+
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
@@ -127,6 +129,7 @@
private final GitRepositoryManager repoManager;
private final SchemaFactory<ReviewDb> schemaFactory;
+ private final ChangeNotes.Factory notesFactory;
private final ProjectCache projectCache;
private final LabelNormalizer labelNormalizer;
private final GitReferenceUpdated gitRefUpdated;
@@ -165,6 +168,7 @@
@Inject
MergeOp(final GitRepositoryManager grm, final SchemaFactory<ReviewDb> sf,
+ final ChangeNotes.Factory nf,
final ProjectCache pc, final LabelNormalizer fs,
final GitReferenceUpdated gru, final MergedSender.Factory msf,
final MergeFailSender.Factory mfsf,
@@ -182,6 +186,7 @@
final ApprovalsUtil approvalsUtil) {
repoManager = grm;
schemaFactory = sf;
+ notesFactory = nf;
labelNormalizer = fs;
projectCache = pc;
gitRefUpdated = gru;
@@ -278,8 +283,8 @@
for (final CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) {
final Capable capable = isSubmitStillPossible(commit);
if (capable != Capable.OK) {
- sendMergeFail(commit.change,
- message(commit.change, capable.getMessage()), false);
+ sendMergeFail(commit.notes,
+ message(commit.getChange(), capable.getMessage()), false);
}
}
} catch (NoSuchProjectException noProject) {
@@ -335,7 +340,7 @@
return false;
}
- if (!missingCommit.change.currentPatchSetId().equals(
+ if (!missingCommit.getChange().currentPatchSetId().equals(
missingCommit.patchsetId)) {
// If the missing commit is not the current patch set,
// the change must be rebased to use the proper parent.
@@ -518,7 +523,7 @@
continue;
}
- commit.change = chg;
+ commit.notes = notesFactory.create(chg);
commit.patchsetId = ps.getId();
commit.originalOrder = commitOrder++;
commits.put(changeId, commit);
@@ -621,8 +626,8 @@
gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate);
Account account = null;
- PatchSetApproval submitter =
- approvalsUtil.getSubmitter(db, mergeTip.patchsetId);
+ PatchSetApproval submitter = approvalsUtil.getSubmitter(
+ db, mergeTip.notes, mergeTip.patchsetId);
if (submitter != null) {
account = accountCache.get(submitter.getAccountId()).getAccount();
}
@@ -684,7 +689,7 @@
case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND:
case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT:
case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN:
- setNew(c, message(c, txt));
+ setNew(commit, message(c, txt));
break;
case MISSING_DEPENDENCY:
@@ -692,7 +697,7 @@
break;
default:
- setNew(c, message(c, "Unspecified merge failure: " + s.name()));
+ setNew(commit, message(c, "Unspecified merge failure: " + s.name()));
break;
}
} catch (OrmException err) {
@@ -720,7 +725,7 @@
private Capable isSubmitStillPossible(final CodeReviewCommit commit) {
final Capable capable;
- final Change c = commit.change;
+ final Change c = commit.getChange();
final boolean submitStillPossible = isSubmitForMissingCommitsStillPossible(commit);
final long now = TimeUtil.nowMs();
final long waitUntil = c.getLastUpdatedOn().getTime() + DEPENDENCY_DELAY;
@@ -745,7 +750,7 @@
m.append("\n");
for (CodeReviewCommit missingCommit : commit.missing) {
m.append("* ");
- m.append(missingCommit.change.getKey().get());
+ m.append(missingCommit.getChange().getKey().get());
m.append("\n");
}
capable = new Capable(m.toString());
@@ -764,10 +769,10 @@
m.append("* Depends on patch set ");
m.append(missingCommit.patchsetId.get());
m.append(" of ");
- m.append(missingCommit.change.getKey().abbreviate());
- if (missingCommit.patchsetId.get() != missingCommit.change.currentPatchSetId().get()) {
+ m.append(missingCommit.getChange().getKey().abbreviate());
+ if (missingCommit.patchsetId.get() != missingCommit.getChange().currentPatchSetId().get()) {
m.append(", however the current patch set is ");
- m.append(missingCommit.change.currentPatchSetId().get());
+ m.append(missingCommit.getChange().currentPatchSetId().get());
}
m.append(".\n");
@@ -786,14 +791,15 @@
}
private void loadChangeInfo(final CodeReviewCommit commit) {
- if (commit.patchsetId == null) {
+ if (commit.notes == null) {
try {
List<PatchSet> matches =
db.patchSets().byRevision(new RevId(commit.name())).toList();
if (matches.size() == 1) {
final PatchSet ps = matches.get(0);
commit.patchsetId = ps.getId();
- commit.change = db.changes().get(ps.getId().getParentKey());
+ commit.notes =
+ notesFactory.create(db.changes().get(ps.getId().getParentKey()));
}
} catch (OrmException e) {
}
@@ -821,9 +827,9 @@
// We must pull the patchset out of commits, because the patchset ID is
// modified when using the cherry-pick merge strategy.
CodeReviewCommit commit = commits.get(c.getId());
- PatchSet.Id merged = commit.change.currentPatchSetId();
+ PatchSet.Id merged = commit.getChange().currentPatchSetId();
c = setMergedPatchSet(c.getId(), merged);
- PatchSetApproval submitter = saveApprovals(c, merged);
+ PatchSetApproval submitter = saveApprovals(c, commit.notes, merged);
addMergedMessage(submitter, msg);
db.commit();
@@ -870,8 +876,8 @@
});
}
- private PatchSetApproval saveApprovals(Change c, PatchSet.Id merged)
- throws OrmException {
+ private PatchSetApproval saveApprovals(Change c, ChangeNotes notes,
+ PatchSet.Id merged) throws OrmException {
// Flatten out existing approvals for this patch set based upon the current
// permissions. Once the change is closed the approvals are not updated at
// presentation view time, except for zero votes used to indicate a reviewer
@@ -880,7 +886,7 @@
PatchSetApproval submitter = null;
try {
List<PatchSetApproval> approvals =
- db.patchSetApprovals().byPatchSet(merged).toList();
+ approvalsUtil.byPatchSet(db, notes, merged);
Set<PatchSetApproval.Key> toDelete =
Sets.newHashSetWithExpectedSize(approvals.size());
for (PatchSetApproval a : approvals) {
@@ -899,6 +905,7 @@
}
}
}
+ // TODO(dborowitz): Store normalized labels in notedb.
db.patchSetApprovals().update(approvals);
db.patchSetApprovals().deleteKeys(toDelete);
} catch (NoSuchChangeException err) {
@@ -956,8 +963,12 @@
}));
}
- private void setNew(Change c, ChangeMessage msg) {
- sendMergeFail(c, msg, true);
+ private void setNew(CodeReviewCommit c, ChangeMessage msg) {
+ sendMergeFail(c.notes, msg, true);
+ }
+
+ private void setNew(Change c, ChangeMessage msg) throws OrmException {
+ sendMergeFail(notesFactory.create(c), msg, true);
}
private enum RetryStatus {
@@ -993,11 +1004,12 @@
}
}
- private void sendMergeFail(final Change c, final ChangeMessage msg,
+ private void sendMergeFail(ChangeNotes notes, final ChangeMessage msg,
boolean makeNew) {
PatchSetApproval submitter = null;
try {
- submitter = approvalsUtil.getSubmitter(db, c.currentPatchSetId());
+ submitter = approvalsUtil.getSubmitter(
+ db, notes, notes.getChange().currentPatchSetId());
} catch (Exception e) {
log.error("Cannot get submitter", e);
}
@@ -1012,6 +1024,7 @@
}
final boolean setStatusNew = makeNew;
+ final Change c = notes.getChange();
Change change = null;
try {
db.changes().beginTransaction(c.getId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index 0faac22..09a0e68 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -24,7 +24,6 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
-import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -168,8 +167,8 @@
});
}
- public PatchSetApproval getSubmitter(final PatchSet.Id c) {
- return approvalsUtil.getSubmitter(db.get(), c);
+ PatchSetApproval getSubmitter(CodeReviewCommit c) {
+ return approvalsUtil.getSubmitter(db.get(), c.notes, c.patchsetId);
}
public RevCommit createCherryPickFromCommit(Repository repo,
@@ -218,10 +217,10 @@
msgbuf.append('\n');
}
- if (!contains(footers, CHANGE_ID, n.change.getKey().get())) {
+ if (!contains(footers, CHANGE_ID, n.getChange().getKey().get())) {
msgbuf.append(CHANGE_ID.getName());
msgbuf.append(": ");
- msgbuf.append(n.change.getKey().get());
+ msgbuf.append(n.getChange().getKey().get());
msgbuf.append('\n');
}
@@ -314,7 +313,7 @@
private List<PatchSetApproval> safeGetApprovals(CodeReviewCommit n) {
try {
- return approvalsUtil.byPatchSet(db.get(), n.patchsetId);
+ return approvalsUtil.byPatchSet(db.get(), n.notes, n.patchsetId);
} catch (OrmException e) {
log.error("Can't read approval records for " + n.patchsetId, e);
return Collections.emptyList();
@@ -344,7 +343,7 @@
final RevWalk rw, final List<CodeReviewCommit> codeReviewCommits) {
PatchSetApproval submitter = null;
for (final CodeReviewCommit c : codeReviewCommits) {
- PatchSetApproval s = getSubmitter(c.patchsetId);
+ PatchSetApproval s = getSubmitter(c);
if (submitter == null
|| (s != null && s.getGranted().compareTo(submitter.getGranted()) > 0)) {
submitter = s;
@@ -601,8 +600,8 @@
LinkedHashSet<String> topics = new LinkedHashSet<>(4);
for (CodeReviewCommit c : merged) {
- if (!Strings.isNullOrEmpty(c.change.getTopic())) {
- topics.add(c.change.getTopic());
+ if (!Strings.isNullOrEmpty(c.getChange().getTopic())) {
+ topics.add(c.getChange().getTopic());
}
}
@@ -617,7 +616,7 @@
new Function<CodeReviewCommit, String>() {
@Override
public String apply(CodeReviewCommit in) {
- return in.change.getKey().abbreviate();
+ return in.getChange().getKey().abbreviate();
}
})),
merged.size() > 5 ? ", ..." : "");
@@ -707,7 +706,7 @@
if (c.patchsetId != null) {
c.statusCode = CommitMergeStatus.CLEAN_MERGE;
if (submitApproval == null) {
- submitApproval = getSubmitter(c.patchsetId);
+ submitApproval = getSubmitter(c);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseIfNecessary.java
index cbd87fd..01d37dc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseIfNecessary.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseIfNecessary.java
@@ -14,13 +14,12 @@
package com.google.gerrit.server.git;
-import static com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
-
import com.google.common.collect.Lists;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.changedetail.PathConflictException;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -79,30 +78,34 @@
} else {
try {
- final IdentifiedUser uploader =
- args.identifiedUserFactory.create(
- args.mergeUtil.getSubmitter(n.patchsetId).getAccountId());
+ final IdentifiedUser uploader = args.identifiedUserFactory.create(
+ args.mergeUtil.getSubmitter(n).getAccountId());
final PatchSet newPatchSet =
rebaseChange.rebase(args.repo, args.rw, args.inserter,
- n.patchsetId, n.change, uploader,
+ n.patchsetId, n.getChange(), uploader,
newMergeTip, args.mergeUtil, committerIdent,
false, false, ValidatePolicy.NONE);
+
+ // TODO(dborowitz): This doesn't copy labels in the notedb. We
+ // should stamp those in atomically with the same commit that
+ // describes the change being submitted.
List<PatchSetApproval> approvals = Lists.newArrayList();
- for (PatchSetApproval a
- : args.approvalsUtil.byPatchSet(args.db, n.patchsetId)) {
+ for (PatchSetApproval a : args.approvalsUtil.byPatchSet(
+ args.db, n.notes, n.patchsetId)) {
approvals.add(new PatchSetApproval(newPatchSet.getId(), a));
}
args.db.patchSetApprovals().insert(approvals);
+
newMergeTip =
(CodeReviewCommit) args.rw.parseCommit(ObjectId
.fromString(newPatchSet.getRevision().get()));
newMergeTip.copyFrom(n);
newMergeTip.patchsetId = newPatchSet.getId();
- newMergeTip.change =
- args.db.changes().get(newPatchSet.getId().getParentKey());
+ newMergeTip.notes = args.notesFactory.create(
+ args.db.changes().get(newPatchSet.getId().getParentKey()));
newMergeTip.statusCode = CommitMergeStatus.CLEAN_REBASE;
newCommits.put(newPatchSet.getId().getParentKey(), newMergeTip);
- setRefLogIdent(args.mergeUtil.getSubmitter(n.patchsetId));
+ setRefLogIdent(args.mergeUtil.getSubmitter(n));
} catch (PathConflictException e) {
n.statusCode = CommitMergeStatus.PATH_CONFLICT;
} catch (NoSuchChangeException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 59431b5..bf98294 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromReviewers;
+
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
@@ -88,6 +89,8 @@
import com.google.gerrit.server.mail.MailUtil.MailRecipients;
import com.google.gerrit.server.mail.MergedSender;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
@@ -254,6 +257,8 @@
private final IdentifiedUser currentUser;
private final ReviewDb db;
+ private final ChangeNotes.Factory notesFactory;
+ private final ChangeUpdate.Factory updateFactory;
private final SchemaFactory<ReviewDb> schemaFactory;
private final AccountResolver accountResolver;
private final CmdLineParser.Factory optionParserFactory;
@@ -317,6 +322,8 @@
@Inject
ReceiveCommits(final ReviewDb db,
final SchemaFactory<ReviewDb> schemaFactory,
+ final ChangeNotes.Factory notesFactory,
+ final ChangeUpdate.Factory updateFactory,
final AccountResolver accountResolver,
final CmdLineParser.Factory optionParserFactory,
final CreateChangeSender.Factory createChangeSenderFactory,
@@ -352,6 +359,8 @@
final ChangeKindCache changeKindCache) throws IOException {
this.currentUser = (IdentifiedUser) projectControl.getCurrentUser();
this.db = db;
+ this.notesFactory = notesFactory;
+ this.updateFactory = updateFactory;
this.schemaFactory = schemaFactory;
this.accountResolver = accountResolver;
this.optionParserFactory = optionParserFactory;
@@ -1842,6 +1851,7 @@
recipients.add(getRecipientsFromFooters(accountResolver, newPatchSet, footerLines));
recipients.remove(me);
+ ChangeUpdate update = updateFactory.create(change, newPatchSet.getCreatedOn());
db.changes().beginTransaction(change.getId());
try {
change = db.changes().get(change.getId());
@@ -1858,15 +1868,15 @@
mergedIntoRef = mergedInto != null ? mergedInto.getName() : null;
}
- List<PatchSetApproval> oldChangeApprovals =
- db.patchSetApprovals().byChange(change.getId()).toList();
+ Collection<PatchSetApproval> oldChangeApprovals =
+ approvalsUtil.byChange(db, notesFactory.create(change)).values();
SetMultimap<ReviewerState, Account.Id> reviewers =
ApprovalsUtil.getReviewers(oldChangeApprovals);
MailRecipients oldRecipients = getRecipientsFromReviewers(reviewers);
approvalsUtil.copyLabels(db, labelTypes, oldChangeApprovals,
priorPatchSet, newPatchSet, changeKind);
- approvalsUtil.addReviewers(db, labelTypes, change, newPatchSet, info,
- recipients.getReviewers(), oldRecipients.getAll());
+ approvalsUtil.addReviewers(db, update, labelTypes, change, newPatchSet,
+ info, recipients.getReviewers(), oldRecipients.getAll());
recipients.add(oldRecipients);
msg =
@@ -1924,6 +1934,7 @@
} finally {
db.rollback();
}
+ update.commit();
if (mergedIntoRef != null) {
// Change was already submitted to a branch, close it.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategy.java
index 2e00e0c..8fc1c35 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategy.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategy.java
@@ -23,6 +23,9 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.index.ChangeIndexer;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate.Result;
@@ -49,6 +52,8 @@
protected final IdentifiedUser.GenericFactory identifiedUserFactory;
protected final PersonIdent myIdent;
protected final ReviewDb db;
+ protected final ChangeNotes.Factory notesFactory;
+ protected final ChangeUpdate.Factory updateFactory;
protected final Repository repo;
protected final RevWalk rw;
@@ -62,7 +67,9 @@
protected final MergeSorter mergeSorter;
Arguments(final IdentifiedUser.GenericFactory identifiedUserFactory,
- final PersonIdent myIdent, final ReviewDb db, final Repository repo,
+ final PersonIdent myIdent, final ReviewDb db,
+ final ChangeNotes.Factory notesFactory,
+ final ChangeUpdate.Factory updateFactory, final Repository repo,
final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Set<RevCommit> alreadyAccepted,
final Branch.NameKey destBranch, final ApprovalsUtil approvalsUtil,
@@ -70,6 +77,8 @@
this.identifiedUserFactory = identifiedUserFactory;
this.myIdent = myIdent;
this.db = db;
+ this.notesFactory = notesFactory;
+ this.updateFactory = updateFactory;
this.repo = repo;
this.rw = rw;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java
index ff4329d..4b183b8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java
@@ -23,6 +23,8 @@
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.index.ChangeIndexer;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
@@ -47,6 +49,8 @@
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final PersonIdent myIdent;
+ private final ChangeNotes.Factory notesFactory;
+ private final ChangeUpdate.Factory updateFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final GitReferenceUpdated gitRefUpdated;
private final RebaseChange rebaseChange;
@@ -59,6 +63,8 @@
SubmitStrategyFactory(
final IdentifiedUser.GenericFactory identifiedUserFactory,
@GerritPersonIdent final PersonIdent myIdent,
+ final ChangeNotes.Factory notesFactory,
+ final ChangeUpdate.Factory updateFactory,
final PatchSetInfoFactory patchSetInfoFactory,
final GitReferenceUpdated gitRefUpdated, final RebaseChange rebaseChange,
final ProjectCache projectCache,
@@ -67,6 +73,8 @@
final ChangeIndexer indexer) {
this.identifiedUserFactory = identifiedUserFactory;
this.myIdent = myIdent;
+ this.notesFactory = notesFactory;
+ this.updateFactory = updateFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.gitRefUpdated = gitRefUpdated;
this.rebaseChange = rebaseChange;
@@ -83,9 +91,10 @@
throws MergeException, NoSuchProjectException {
ProjectState project = getProject(destBranch);
final SubmitStrategy.Arguments args =
- new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db, repo,
- rw, inserter, canMergeFlag, alreadyAccepted, destBranch,
- approvalsUtil, mergeUtilFactory.create(project), indexer);
+ new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db,
+ notesFactory, updateFactory, repo, rw, inserter, canMergeFlag,
+ alreadyAccepted, destBranch,approvalsUtil,
+ mergeUtilFactory.create(project), indexer);
switch (submitType) {
case CHERRY_PICK:
return new CherryPick(args, patchSetInfoFactory, gitRefUpdated);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
index 88f9db1..880e5af 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
@@ -119,7 +119,8 @@
}
} else {
if (!oldParent.equals(newParent)) {
- PatchSetApproval psa = approvalsUtil.getSubmitter(db, patchSetId);
+ PatchSetApproval psa =
+ approvalsUtil.getSubmitter(db, commit.notes, patchSetId);
if (psa == null) {
throw new MergeValidationException(CommitMergeStatus.
SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index 09f9e42..4e51a73 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.mail;
-import com.google.common.collect.SetMultimap;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -57,21 +56,19 @@
private static final Logger log = LoggerFactory.getLogger(ChangeEmail.class);
protected final Change change;
+ protected final ChangeData changeData;
protected PatchSet patchSet;
protected PatchSetInfo patchSetInfo;
protected ChangeMessage changeMessage;
protected ProjectState projectState;
- protected ChangeData changeData;
protected Set<Account.Id> authors;
protected boolean emailOnlyAuthors;
- private SetMultimap<ReviewerState, Account.Id> reviewers;
-
protected ChangeEmail(EmailArguments ea, Change c, String mc) {
super(ea, mc, c.getProject(), c.getDest());
change = c;
- changeData = ea.changeDataFactory.create(ea.db.get(), change);
+ changeData = ea.changeDataFactory.create(ea.db.get(), c);
emailOnlyAuthors = false;
}
@@ -96,22 +93,13 @@
changeMessage = cm;
}
- private SetMultimap<ReviewerState, Account.Id> getReviewers()
- throws OrmException {
- if (reviewers == null) {
- reviewers =
- args.approvalsUtil.getReviewers(args.db.get(), change.getId());
- }
- return reviewers;
- }
-
/** Format the message body by calling {@link #appendText(String)}. */
protected void format() throws EmailException {
formatChange();
appendText(velocifyFile("ChangeFooter.vm"));
try {
TreeSet<String> names = new TreeSet<String>();
- for (Account.Id who : getReviewers().values()) {
+ for (Account.Id who : changeData.reviewers().values()) {
names.add(getNameEmailFor(who));
}
@@ -317,7 +305,7 @@
/** Any user who has published comments on this change. */
protected void ccAllApprovals() {
try {
- for (Account.Id id : getReviewers().values()) {
+ for (Account.Id id : changeData.reviewers().values()) {
add(RecipientType.CC, id);
}
} catch (OrmException err) {
@@ -328,7 +316,7 @@
/** Users who have non-zero approval codes on the change. */
protected void ccExistingReviewers() {
try {
- for (Account.Id id : getReviewers().get(ReviewerState.REVIEWER)) {
+ for (Account.Id id : changeData.reviewers().get(ReviewerState.REVIEWER)) {
add(RecipientType.CC, id);
}
} catch (OrmException err) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java
index ac73530..66ea1e0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ProjectCache;
@@ -54,6 +55,7 @@
final PatchSetInfoFactory patchSetInfoFactory;
final IdentifiedUser.GenericFactory identifiedUserFactory;
final CapabilityControl.Factory capabilityControlFactory;
+ final ChangeNotes.Factory changeNotesFactory;
final AnonymousUser anonymousUser;
final String anonymousCowardName;
final Provider<String> urlProvider;
@@ -76,6 +78,7 @@
EmailSender emailSender, PatchSetInfoFactory patchSetInfoFactory,
GenericFactory identifiedUserFactory,
CapabilityControl.Factory capabilityControlFactory,
+ ChangeNotes.Factory changeNotesFactory,
AnonymousUser anonymousUser,
@AnonymousCowardName String anonymousCowardName,
@CanonicalWebUrl @Nullable Provider<String> urlProvider,
@@ -98,6 +101,7 @@
this.patchSetInfoFactory = patchSetInfoFactory;
this.identifiedUserFactory = identifiedUserFactory;
this.capabilityControlFactory = capabilityControlFactory;
+ this.changeNotesFactory = changeNotesFactory;
this.anonymousUser = anonymousUser;
this.anonymousCowardName = anonymousCowardName;
this.urlProvider = urlProvider;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
index 37d800d..07a5f9a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
@@ -61,8 +61,8 @@
try {
Table<Account.Id, String, PatchSetApproval> pos = HashBasedTable.create();
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
- for (PatchSetApproval ca : args.db.get().patchSetApprovals()
- .byPatchSet(patchSet.getId())) {
+ for (PatchSetApproval ca : args.approvalsUtil.byPatchSet(
+ args.db.get(), changeData.notes(), patchSet.getId())) {
LabelType lt = labelTypes.byLabel(ca.getLabelId());
if (lt == null) {
continue;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java
index 6777fdd..93fc063 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java
@@ -31,6 +31,8 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.MailUtil.MailRecipients;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gwtorm.server.OrmException;
@@ -79,9 +81,10 @@
this.replacePatchSetFactory = replacePatchSetFactory;
}
- public void send(final boolean newChange,
- final IdentifiedUser currentUser, final Change updatedChange,
- final PatchSet updatedPatchSet, final LabelTypes labelTypes)
+ public void send(final ChangeNotes notes, final ChangeUpdate update,
+ final boolean newChange, final IdentifiedUser currentUser,
+ final Change updatedChange, final PatchSet updatedPatchSet,
+ final LabelTypes labelTypes)
throws OrmException, IOException, PatchSetInfoNotAvailableException {
final Repository git = repoManager.openRepository(updatedChange.getProject());
try {
@@ -101,9 +104,9 @@
recipients.remove(me);
if (newChange) {
- approvalsUtil.addReviewers(db, labelTypes,
- updatedChange, updatedPatchSet, info,
- recipients.getReviewers(), Collections.<Account.Id> emptySet());
+ approvalsUtil.addReviewers(db, update, labelTypes, updatedChange,
+ updatedPatchSet, info, recipients.getReviewers(),
+ Collections.<Account.Id> emptySet());
try {
CreateChangeSender cm = createChangeSenderFactory.create(updatedChange);
cm.setFrom(me);
@@ -115,9 +118,9 @@
log.error("Cannot send email for new change " + updatedChange.getId(), e);
}
} else {
- approvalsUtil.addReviewers(db, labelTypes, updatedChange,
+ approvalsUtil.addReviewers(db, update, labelTypes, updatedChange,
updatedPatchSet, info, recipients.getReviewers(),
- approvalsUtil.getReviewers(db, updatedChange.getId()).values());
+ approvalsUtil.getReviewers(db, notes).values());
final ChangeMessage msg =
new ChangeMessage(new ChangeMessage.Key(updatedChange.getId(),
ChangeUtil.messageUUID(db)), me,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 667e3f0..4a514df 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -73,8 +73,9 @@
public static class Factory {
private final GitRepositoryManager repoManager;
+ @VisibleForTesting
@Inject
- Factory(GitRepositoryManager repoManager) {
+ public Factory(GitRepositoryManager repoManager) {
this.repoManager = repoManager;
}
@@ -261,6 +262,10 @@
return this;
}
+ public Change.Id getChangeId() {
+ return change.getId();
+ }
+
public Change getChange() {
return change;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 144eb53..59d583a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -40,6 +40,7 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.FooterKey;
@@ -67,6 +68,7 @@
ChangeUpdate create(Change change, Date when, IdentifiedUser user);
}
+ private final NotesMigration migration;
private final GitRepositoryManager repoManager;
private final AccountCache accountCache;
private final MetaDataUpdate.User updateFactory;
@@ -84,40 +86,43 @@
ChangeUpdate(
@GerritPersonIdent PersonIdent serverIdent,
GitRepositoryManager repoManager,
+ NotesMigration migration,
AccountCache accountCache,
MetaDataUpdate.User updateFactory,
ProjectCache projectCache,
IdentifiedUser user,
@Assisted Change change) {
- this(serverIdent, repoManager, accountCache, updateFactory, projectCache,
- user, change, serverIdent.getWhen());
+ this(serverIdent, repoManager, migration, accountCache, updateFactory,
+ projectCache, user, change, serverIdent.getWhen());
}
@AssistedInject
ChangeUpdate(
@GerritPersonIdent PersonIdent serverIdent,
GitRepositoryManager repoManager,
+ NotesMigration migration,
AccountCache accountCache,
MetaDataUpdate.User updateFactory,
ProjectCache projectCache,
IdentifiedUser user,
@Assisted Change change,
@Assisted Date when) {
- this(serverIdent, repoManager, accountCache, updateFactory, projectCache,
- change, when, user);
+ this(serverIdent, repoManager, migration, accountCache, updateFactory,
+ projectCache, change, when, user);
}
@AssistedInject
ChangeUpdate(
@GerritPersonIdent PersonIdent serverIdent,
GitRepositoryManager repoManager,
+ NotesMigration migration,
AccountCache accountCache,
MetaDataUpdate.User updateFactory,
ProjectCache projectCache,
@Assisted Change change,
@Assisted Date when,
@Assisted IdentifiedUser user) {
- this(serverIdent, repoManager, accountCache, updateFactory,
+ this(serverIdent, repoManager, migration, accountCache, updateFactory,
projectCache.get(change.getDest().getParentKey()).getLabelTypes(),
change, when, user);
}
@@ -126,6 +131,7 @@
ChangeUpdate(
PersonIdent serverIdent,
GitRepositoryManager repoManager,
+ NotesMigration migration,
AccountCache accountCache,
MetaDataUpdate.User updateFactory,
LabelTypes labelTypes,
@@ -133,6 +139,7 @@
Date when,
IdentifiedUser user) {
this.repoManager = repoManager;
+ this.migration = migration;
this.accountCache = accountCache;
this.updateFactory = updateFactory;
this.labelTypes = labelTypes;
@@ -144,6 +151,10 @@
this.reviewers = Maps.newLinkedHashMap();
}
+ public Change getChange() {
+ return change;
+ }
+
public IdentifiedUser getUser() {
return user;
}
@@ -181,6 +192,9 @@
@Override
public RevCommit commit(MetaDataUpdate md) throws IOException {
+ if (!migration.write()) {
+ return null;
+ }
Repository repo = repoManager.openRepository(change.getProject());
try {
load(repo);
@@ -208,6 +222,44 @@
}
@Override
+ public BatchMetaDataUpdate openUpdate(MetaDataUpdate update) throws IOException {
+ if (migration.write()) {
+ return super.openUpdate(update);
+ }
+ return new BatchMetaDataUpdate() {
+ @Override
+ public void write(CommitBuilder commit) {
+ // Do nothing.
+ }
+
+ @Override
+ public void write(VersionedMetaData config, CommitBuilder commit) {
+ // Do nothing.
+ }
+
+ @Override
+ public RevCommit createRef(String refName) {
+ return null;
+ }
+
+ @Override
+ public RevCommit commit() {
+ return null;
+ }
+
+ @Override
+ public RevCommit commitAt(ObjectId revision) {
+ return null;
+ }
+
+ @Override
+ public void close() {
+ // Do nothing.
+ }
+ };
+ }
+
+ @Override
protected String getRefName() {
return ChangeNoteUtil.changeRefName(change.getId());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java
new file mode 100644
index 0000000..174997c
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2013 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.server.notedb;
+
+import com.google.gerrit.server.config.FactoryModule;
+
+public class NoteDbModule extends FactoryModule {
+ @Override
+ public void configure() {
+ factory(ChangeUpdate.Factory.class);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
new file mode 100644
index 0000000..c9ac8de
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
@@ -0,0 +1,62 @@
+// Copyright (C) 2013 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.server.notedb;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.lib.Config;
+
+/**
+ * Holds the current state of the notes DB migration.
+ * <p>
+ * During a transitional period, different subsets of the former gwtorm DB
+ * functionality may be enabled on the site, possibly only for reading or
+ * writing.
+ */
+@Singleton
+public class NotesMigration {
+ @VisibleForTesting
+ static NotesMigration allEnabled() {
+ Config cfg = new Config();
+ cfg.setBoolean("notedb", null, "write", true);
+ //cfg.setBoolean("notedb", "patchSetApprovals", "read", true);
+ return new NotesMigration(cfg);
+ }
+
+ private final boolean write;
+ private final boolean readPatchSetApprovals;
+
+ @Inject
+ NotesMigration(@GerritServerConfig Config cfg) {
+ write = cfg.getBoolean("notedb", null, "write", false);
+ readPatchSetApprovals =
+ cfg.getBoolean("notedb", "patchSetApprovals", "read", false);
+ checkArgument(!readPatchSetApprovals,
+ "notedb.readPatchSetApprovals not yet supported");
+ }
+
+ public boolean write() {
+ return write;
+ }
+
+ public boolean readPatchSetApprovals() {
+ return readPatchSetApprovals;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 4c2a30b..102a136 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -690,7 +690,7 @@
}
private ChangeData changeData(ReviewDb db, @Nullable ChangeData cd) {
- return cd != null ? cd : changeDataFactory.create(db, getChange());
+ return cd != null ? cd : changeDataFactory.create(db, this);
}
private void appliedBy(SubmitRecord.Label label, Term status)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 547cfeb..b6fc356 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -36,6 +36,8 @@
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
@@ -75,9 +77,16 @@
}
}
if (!missing.isEmpty()) {
- ReviewDb db = missing.values().iterator().next().db;
- for (Change change : db.changes().get(missing.keySet())) {
- missing.get(change.getId()).change = change;
+ ChangeData first = missing.values().iterator().next();
+ if (!first.notesMigration.readPatchSetApprovals()) {
+ ReviewDb db = missing.values().iterator().next().db;
+ for (Change change : db.changes().get(missing.keySet())) {
+ missing.get(change.getId()).change = change;
+ }
+ } else {
+ for (ChangeData cd : missing.values()) {
+ cd.change();
+ }
}
}
}
@@ -113,9 +122,13 @@
throws OrmException {
List<ResultSet<PatchSetApproval>> pending = Lists.newArrayList();
for (ChangeData cd : changes) {
- if (cd.currentApprovals == null && cd.limitedApprovals == null) {
- pending.add(cd.db.patchSetApprovals()
- .byPatchSet(cd.change().currentPatchSetId()));
+ if (!cd.notesMigration.readPatchSetApprovals()) {
+ if (cd.currentApprovals == null && cd.limitedApprovals == null) {
+ pending.add(cd.db.patchSetApprovals()
+ .byPatchSet(cd.change().currentPatchSetId()));
+ }
+ } else {
+ cd.currentApprovals();
}
}
if (!pending.isEmpty()) {
@@ -143,15 +156,19 @@
* @return instance for testing.
*/
static ChangeData createForTest(Change.Id id) {
- return new ChangeData(null, null, null, id);
+ return new ChangeData(null, null, null, null, null, null, id);
}
private final ReviewDb db;
private final GitRepositoryManager repoManager;
+ private final ChangeNotes.Factory notesFactory;
+ private final ApprovalsUtil approvalsUtil;
private final PatchListCache patchListCache;
+ private final NotesMigration notesMigration;
private final Change.Id legacyId;
private ChangeDataSource returnedBySource;
private Change change;
+ private ChangeNotes notes;
private String commitMessage;
private List<FooterLine> commitFooters;
private PatchSet currentPatchSet;
@@ -172,24 +189,36 @@
@AssistedInject
private ChangeData(
GitRepositoryManager repoManager,
+ ChangeNotes.Factory notesFactory,
+ ApprovalsUtil approvalsUtil,
PatchListCache patchListCache,
+ NotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted Change.Id id) {
this.db = db;
this.repoManager = repoManager;
+ this.notesFactory = notesFactory;
+ this.approvalsUtil = approvalsUtil;
this.patchListCache = patchListCache;
+ this.notesMigration = notesMigration;
legacyId = id;
}
@AssistedInject
private ChangeData(
GitRepositoryManager repoManager,
+ ChangeNotes.Factory notesFactory,
+ ApprovalsUtil approvalsUtil,
PatchListCache patchListCache,
+ NotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted Change c) {
this.db = db;
this.repoManager = repoManager;
+ this.notesFactory = notesFactory;
+ this.approvalsUtil = approvalsUtil;
this.patchListCache = patchListCache;
+ this.notesMigration = notesMigration;
legacyId = c.getId();
change = c;
}
@@ -197,15 +226,22 @@
@AssistedInject
private ChangeData(
GitRepositoryManager repoManager,
+ ChangeNotes.Factory notesFactory,
+ ApprovalsUtil approvalsUtil,
PatchListCache patchListCache,
+ NotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted ChangeControl c) {
this.db = db;
this.repoManager = repoManager;
+ this.notesFactory = notesFactory;
+ this.approvalsUtil = approvalsUtil;
this.patchListCache = patchListCache;
+ this.notesMigration = notesMigration;
legacyId = c.getChange().getId();
change = c.getChange();
changeControl = c;
+ notes = c.getNotes();
}
public boolean isFromSource(ChangeDataSource s) {
@@ -328,6 +364,13 @@
return change;
}
+ public ChangeNotes notes() throws OrmException {
+ if (notes == null) {
+ notes = notesFactory.create(change());
+ }
+ return notes;
+ }
+
public PatchSet currentPatchSet() throws OrmException {
if (currentPatchSet == null) {
Change c = change();
@@ -356,8 +399,8 @@
(limitedIds == null || limitedIds.contains(c.currentPatchSetId()))) {
return limitedApprovals.get(c.currentPatchSetId());
} else {
- currentApprovals = sortApprovals(db.patchSetApprovals()
- .byPatchSet(c.currentPatchSetId()));
+ currentApprovals = approvalsUtil.byPatchSet(
+ db, notes(), c.currentPatchSetId());
}
}
return currentApprovals;
@@ -457,8 +500,8 @@
limitedApprovals.putAll(id, allApprovals.get(id));
}
} else {
- for (PatchSetApproval psa : sortApprovals(
- db.patchSetApprovals().byChange(legacyId))) {
+ for (PatchSetApproval psa
+ : approvalsUtil.byChange(db, notes()).values()) {
if (limitedIds == null || limitedIds.contains(legacyId)) {
limitedApprovals.put(psa.getPatchSetId(), psa);
}
@@ -488,11 +531,7 @@
public ListMultimap<PatchSet.Id, PatchSetApproval> allApprovalsMap()
throws OrmException {
if (allApprovals == null) {
- allApprovals = ArrayListMultimap.create();
- for (PatchSetApproval psa : sortApprovals(
- db.patchSetApprovals().byChange(legacyId))) {
- allApprovals.put(psa.getPatchSetId(), psa);
- }
+ allApprovals = approvalsUtil.byChange(db, notes());
}
return allApprovals;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
index 00d8ce3..518907e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
@@ -317,7 +317,7 @@
}
if (includeAllReviewers) {
- eventFactory.addAllReviewers(c, d.change());
+ eventFactory.addAllReviewers(c, d.notes());
}
if (includeSubmitRecords) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
index 9032eca..52f528f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
@@ -60,7 +60,7 @@
private final short value;
public LabelVote(String name, short value) {
- this.name = LabelType.checkName(name);
+ this.name = LabelType.checkNameInternal(name);
this.value = value;
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
index dea4af8..eb92bf9 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
@@ -30,6 +30,8 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.SubmoduleSubscriptionAccess;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.ListResultSet;
@@ -79,6 +81,7 @@
private Provider<String> urlProvider;
private GitRepositoryManager repoManager;
private GitReferenceUpdated gitRefUpdated;
+ private ChangeNotes.Factory notesFactory;
@SuppressWarnings("unchecked")
@Override
@@ -92,6 +95,7 @@
urlProvider = createStrictMock(Provider.class);
repoManager = createStrictMock(GitRepositoryManager.class);
gitRefUpdated = createStrictMock(GitReferenceUpdated.class);
+ notesFactory = new ChangeNotes.Factory(repoManager);
}
private void doReplay() {
@@ -612,11 +616,11 @@
final Change submittedChange = new Change(
new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1),
new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs());
- codeReviewCommit.change = submittedChange;
+ codeReviewCommit.notes = notesFactory.create(submittedChange);
final Map<Change.Id, CodeReviewCommit> mergedCommits =
new HashMap<Change.Id, CodeReviewCommit>();
- mergedCommits.put(codeReviewCommit.change.getId(), codeReviewCommit);
+ mergedCommits.put(codeReviewCommit.notes.getChangeId(), codeReviewCommit);
final List<Change> submitted = new ArrayList<Change>();
submitted.add(submittedChange);
@@ -643,7 +647,7 @@
subscribers);
expect(repoManager.openRepository(targetBranchNameKey.getParentKey()))
- .andReturn(targetRepository);
+ .andReturn(targetRepository).anyTimes();
Capture<RefUpdate> ruCapture = new Capture<RefUpdate>();
gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()),
@@ -716,11 +720,11 @@
final Change submittedChange = new Change(
new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1),
new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs());
- codeReviewCommit.change = submittedChange;
+ codeReviewCommit.notes = notesFactory.create(submittedChange);
final Map<Change.Id, CodeReviewCommit> mergedCommits =
new HashMap<Change.Id, CodeReviewCommit>();
- mergedCommits.put(codeReviewCommit.change.getId(), codeReviewCommit);
+ mergedCommits.put(codeReviewCommit.notes.getChangeId(), codeReviewCommit);
final List<Change> submitted = new ArrayList<Change>();
submitted.add(submittedChange);
@@ -747,7 +751,7 @@
subscribers);
expect(repoManager.openRepository(targetBranchNameKey.getParentKey()))
- .andReturn(targetRepository);
+ .andReturn(targetRepository).anyTimes();
Capture<RefUpdate> ruCapture = new Capture<RefUpdate>();
gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()),
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 5f7a0b8..1e4bf59 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -211,7 +211,7 @@
update.putApproval("Verified", (short) 1);
commit(update);
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
assertEquals(1, notes.getApprovals().keySet().size());
List<PatchSetApproval> psas =
notes.getApprovals().get(c.currentPatchSetId());
@@ -244,7 +244,7 @@
commit(update);
PatchSet.Id ps2 = c.currentPatchSetId();
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
ListMultimap<PatchSet.Id, PatchSetApproval> psas = notes.getApprovals();
assertEquals(2, notes.getApprovals().keySet().size());
@@ -270,7 +270,7 @@
update.putApproval("Code-Review", (short) -1);
commit(update);
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
PatchSetApproval psa = Iterables.getOnlyElement(
notes.getApprovals().get(c.currentPatchSetId()));
assertEquals("Code-Review", psa.getLabel());
@@ -280,7 +280,7 @@
update.putApproval("Code-Review", (short) 1);
commit(update);
- notes = newChange(c);
+ notes = newNotes(c);
psa = Iterables.getOnlyElement(
notes.getApprovals().get(c.currentPatchSetId()));
assertEquals("Code-Review", psa.getLabel());
@@ -298,7 +298,7 @@
update.putApproval("Code-Review", (short) 1);
commit(update);
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
assertEquals(1, notes.getApprovals().keySet().size());
List<PatchSetApproval> psas =
notes.getApprovals().get(c.currentPatchSetId());
@@ -325,7 +325,7 @@
update.putReviewer(otherUser.getAccount().getId(), REVIEWER);
commit(update);
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
assertEquals(ImmutableSetMultimap.of(
REVIEWER, new Account.Id(1),
REVIEWER, new Account.Id(2)),
@@ -340,7 +340,7 @@
update.putReviewer(otherUser.getAccount().getId(), CC);
commit(update);
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
assertEquals(ImmutableSetMultimap.of(
REVIEWER, new Account.Id(1),
CC, new Account.Id(2)),
@@ -354,7 +354,7 @@
update.putReviewer(otherUser.getAccount().getId(), REVIEWER);
commit(update);
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
assertEquals(ImmutableSetMultimap.of(
REVIEWER, new Account.Id(2)),
notes.getReviewers());
@@ -363,7 +363,7 @@
update.putReviewer(otherUser.getAccount().getId(), CC);
commit(update);
- notes = newChange(c);
+ notes = newNotes(c);
assertEquals(ImmutableSetMultimap.of(
CC, new Account.Id(2)),
notes.getReviewers());
@@ -384,7 +384,7 @@
update.putApproval("Code-Review", (short) 1);
commit(update);
- ChangeNotes notes = newChange(c);
+ ChangeNotes notes = newNotes(c);
List<PatchSetApproval> psas =
notes.getApprovals().get(c.currentPatchSetId());
assertEquals(2, psas.size());
@@ -395,7 +395,7 @@
update.removeReviewer(otherUser.getAccount().getId());
commit(update);
- notes = newChange(c);
+ notes = newNotes(c);
psas = notes.getApprovals().get(c.currentPatchSetId());
assertEquals(1, psas.size());
assertEquals(changeOwner.getAccount().getId(), psas.get(0).getAccountId());
@@ -415,11 +415,12 @@
private ChangeUpdate newUpdate(Change c, IdentifiedUser user)
throws ConfigInvalidException, IOException {
- return new ChangeUpdate(SERVER_IDENT, repoManager, accountCache, null,
- LABEL_TYPES, c, TimeUtil.nowTs(), user);
+ return new ChangeUpdate(SERVER_IDENT, repoManager,
+ NotesMigration.allEnabled(), accountCache, null, LABEL_TYPES, c,
+ TimeUtil.nowTs(), user);
}
- private ChangeNotes newChange(Change c) throws OrmException {
+ private ChangeNotes newNotes(Change c) throws OrmException {
return new ChangeNotes(repoManager, c).load();
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 0e772f2..561219b 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -17,6 +17,7 @@
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
+
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
@@ -45,7 +46,6 @@
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.CreateProject;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.schema.SchemaCreator;
@@ -77,7 +77,6 @@
private static final TopLevelResource TLR = TopLevelResource.INSTANCE;
@Inject protected AccountManager accountManager;
- @Inject protected ChangeControl.GenericFactory changeControlFactory;
@Inject protected ChangeInserter.Factory changeFactory;
@Inject protected ChangesCollection changes;
@Inject protected CreateProject.Factory projectFactory;
@@ -375,13 +374,12 @@
TestRepository<InMemoryRepository> repo = createProject("repo");
ChangeInserter ins = newChange(repo, null, null, null, null);
Change change = ins.insert();
- ChangeControl ctl = changeControlFactory.controlFor(change, user);
ReviewInput input = new ReviewInput();
input.message = "toplevel";
input.labels = ImmutableMap.<String, Short> of("Code-Review", (short) 1);
postReview.apply(new RevisionResource(
- changes.parse(ctl), ins.getPatchSet()), input);
+ changes.parse(change.getId()), ins.getPatchSet()), input);
assertTrue(query("label:Code-Review=-2").isEmpty());
assertTrue(query("label:Code-Review-2").isEmpty());
@@ -481,7 +479,6 @@
TestRepository<InMemoryRepository> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.insert();
- ChangeControl ctl1 = changeControlFactory.controlFor(change1, user);
Change change2 = newChange(repo, null, null, null, null).insert();
assertTrue(lastUpdatedMs(change1) < lastUpdatedMs(change2));
@@ -495,7 +492,7 @@
ReviewInput input = new ReviewInput();
input.message = "toplevel";
postReview.apply(new RevisionResource(
- changes.parse(ctl1), ins1.getPatchSet()), input);
+ changes.parse(change1.getId()), ins1.getPatchSet()), input);
change1 = db.changes().get(change1.getId());
assertTrue(lastUpdatedMs(change1) > lastUpdatedMs(change2));
@@ -514,7 +511,6 @@
TestRepository<InMemoryRepository> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.insert();
- ChangeControl ctl1 = changeControlFactory.controlFor(change1, user);
Change change2 = newChange(repo, null, null, null, null).insert();
assertTrue(lastUpdatedMs(change1) < lastUpdatedMs(change2));
@@ -528,7 +524,7 @@
ReviewInput input = new ReviewInput();
input.message = "toplevel";
postReview.apply(new RevisionResource(
- changes.parse(ctl1), ins1.getPatchSet()), input);
+ changes.parse(change1.getId()), ins1.getPatchSet()), input);
change1 = db.changes().get(change1.getId());
assertTrue(lastUpdatedMs(change1) > lastUpdatedMs(change2));
@@ -650,7 +646,6 @@
TestRepository<InMemoryRepository> repo = createProject("repo");
ChangeInserter ins = newChange(repo, null, null, null, null);
Change change = ins.insert();
- ChangeControl ctl = changeControlFactory.controlFor(change, user);
ReviewInput input = new ReviewInput();
input.message = "toplevel";
@@ -660,7 +655,7 @@
input.comments = ImmutableMap.<String, List<ReviewInput.Comment>> of(
"Foo.java", ImmutableList.<ReviewInput.Comment> of(comment));
postReview.apply(new RevisionResource(
- changes.parse(ctl), ins.getPatchSet()), input);
+ changes.parse(change.getId()), ins.getPatchSet()), input);
assertTrue(query("comment:foo").isEmpty());
assertResultEquals(change, queryOne("comment:toplevel"));
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
index 7b33b99..bad87de 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
@@ -116,6 +116,7 @@
@Option(name = "--label", aliases = "-l", usage = "custom label(s) to assign", metaVar = "LABEL=VALUE")
void addLabel(final String token) {
LabelVote v = LabelVote.parse(token);
+ LabelType.checkName(v.getLabel()); // Disallow SUBM.
customLabels.put(v.getLabel(), v.getValue());
}
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 0b0419b..6df20f3 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 0b0419b4aadace82ae2732d7fd3adf238bf3331a
+Subproject commit 6df20f370c328a87946246dca08f5f166e69ac6b
