opendev/gerrit
Code Issues Proposed changes

Start migration to notedb implementation of PatchSetApprovals

Browse Source 
Encapsulate the current state of the migration in a NotesMigration
class. For each database table (or broader group of functionality)
that we migrate to notedb, configure a boolean indicating whether that
data should be read from notes, defaulting to false.

Unlike reads, NotesMigration contains just a single boolean
indicating that data should be written. We don't attempt to write
just some types of data; as the migration continues we will just
rewrite history.

Since most existing reads of the PatchSetApprovals table have been
migrated to ApprovalsUtil, most implementation changes happen there.
There are a few other implementations scattered around, and some that
will require a bit more work (e.g. stamping normalized approvals at
submit time).

Change-Id: I5676267d4de607c385e8c9917a89333863b9c9e7
This commit is contained in:
Dave Borowitz
2013-12-09 11:58:38 -08:00
parent 498b19f419
commit d064abea4f
45 changed files with 616 additions and 245 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
View File

@@ -32,6 +32,7 @@ import com.google.gerrit.reviewdb.client.Change;
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 @@ public class PushOneCommit {
@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 @@ public class PushOneCommit {
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 @@ public class PushOneCommit {
@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 @@ public class PushOneCommit {
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 @@ public class PushOneCommit {
}));
for (Account.Id accountId
: approvalsUtil.getReviewers(db, c.getId()).values()) {
: approvalsUtil.getReviewers(db, notesFactory.create(c)).values()) {
assertTrue("unexpected reviewer " + accountId,
expectedReviewerIds.remove(accountId));
}

8
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
View File

@@ -42,6 +42,7 @@ import com.google.gerrit.server.git.CommitMergeStatus;
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;
@@ -86,6 +87,9 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
@Inject
private GroupCache groupCache;
@Inject
private ChangeNotes.Factory changeNotesFactory;
@Inject
private @GerritPersonIdent PersonIdent serverIdent;
@@ -258,7 +262,9 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
}
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());

11
gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
View File

@@ -34,13 +34,18 @@ public class LabelType {
}
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 == '-') ||

107
gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
View File

@@ -18,10 +18,14 @@ import static com.google.common.base.Preconditions.checkArgument;
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.PatchSetApproval.LabelId;
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 @@ import java.util.Set;
* 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 @@ public class ApprovalsUtil {
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 @@ public class ApprovalsUtil {
*
* @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 @@ public class ApprovalsUtil {
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 @@ public class ApprovalsUtil {
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) {

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

@@ -489,6 +489,7 @@ public class ChangeUtil {
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));

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

@@ -28,8 +28,10 @@ import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.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 @@ public class ChangeInserter {
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 @@ public class ChangeInserter {
@Inject
ChangeInserter(Provider<ReviewDb> dbProvider,
ChangeUpdate.Factory updateFactory,
Provider<ApprovalsUtil> approvals,
PatchSetInfoFactory patchSetInfoFactory,
GitReferenceUpdated gitRefUpdated,
@@ -86,6 +90,7 @@ public class ChangeInserter {
@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 class ChangeInserter {
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 @@ public class ChangeInserter {
db.rollback();
}
update.commit();
CheckedFuture<?, IOException> f =
mergeabilityChecker.updateAndIndexAsync(change);
if (!messageIsForChange()) {

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

@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.restapi.RestView;
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 @@ public class ChangeResource implements RestResource, HasETag {
return getControl().getChange();
}
public ChangeNotes getNotes() {
return getControl().getNotes();
}
@Override
public String getETag() {
CurrentUser user = control.getCurrentUser();

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

@@ -26,10 +26,12 @@ import com.google.gerrit.reviewdb.client.Change;
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 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
}
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 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
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 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
}
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 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
} finally {
db.rollback();
}
update.commit();
indexer.index(db, rsrc.getChange());
return Response.none();
}
@@ -119,7 +132,7 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
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) {

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

@@ -17,7 +17,6 @@ package com.google.gerrit.server.change;
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 @@ class ListReviewers implements RestReadView<ChangeResource> {
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));
}

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

@@ -35,6 +35,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
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 @@ public class PatchSetInserter {
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 @@ public class PatchSetInserter {
@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 @@ public class PatchSetInserter {
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 @@ public class PatchSetInserter {
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 @@ public class PatchSetInserter {
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 @@ public class PatchSetInserter {
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);

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

@@ -44,10 +44,12 @@ import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
@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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
input.notify = NotifyHandling.NONE;
}
ChangeUpdate update = null;
db.get().changes().beginTransaction(revision.getChange().getId());
boolean dirty = false;
try {
@@ -126,8 +135,9 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
} finally {
db.get().rollback();
}
if (update != null) {
update.commit();
}
CheckedFuture<?, IOException> indexWrite;
if (dirty) {
@@ -365,8 +378,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
ins.add(c);
addLabelDelta(normName, c.getValue());
categories.put(normName, c.getValue());
update.putApproval(ent.getKey(), ent.getValue());
}
}
@@ -455,8 +471,10 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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;
}

13
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@@ -48,6 +48,7 @@ import com.google.gerrit.server.group.GroupsCollection;
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 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
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 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
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 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
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 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
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();

9
gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java
View File

@@ -29,6 +29,7 @@ import com.google.gerrit.server.ChangeUtil;
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 @@ public class Publish implements RestModifyView<RevisionResource, Input>,
}
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 @@ public class Publish implements RestModifyView<RevisionResource, Input>,
PatchSet updatedPatchSet = updateDraftPatchSet(rsrc);
Change updatedChange = updateDraftChange(rsrc);
ChangeUpdate update = updateFactory.create(rsrc.getChange(),
updatedChange.getLastUpdatedOn());
try {
if (!updatedPatchSet.isDraft()
@@ -79,7 +85,8 @@ public class Publish implements RestModifyView<RevisionResource, Input>,
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();

16
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
View File

@@ -30,6 +30,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
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 @@ import java.util.TreeMap;
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 @@ public class ReviewerJson {
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 @@ public class ReviewerJson {
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 {

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

@@ -81,6 +81,6 @@ public class Reviewers implements
private Collection<Account.Id> fetchAccountIds(ChangeResource rsrc)
throws OrmException {
return approvalsUtil.getReviewers(
dbProvider.get(), rsrc.getChange().getId()).values();
dbProvider.get(), rsrc.getNotes()).values();
}
}

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

@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.client.Account;
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 @@ public class RevisionResource implements RestResource, HasETag {
return getControl().getChange();
}
public ChangeNotes getNotes() {
return getChangeResource().getNotes();
}
public PatchSet getPatchSet() {
return ps;
}

33
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@@ -32,6 +32,7 @@ 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;
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.change.ChangeJson.ChangeInfo;
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 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
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 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
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 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
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));
}

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

@@ -101,6 +101,7 @@ import com.google.gerrit.server.mail.MergedSender;
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 @@ public class GerritGlobalModule extends FactoryModule {
install(new CmdLineParserModule());
install(new EmailModule());
install(new GitModule());
install(new NoteDbModule());
install(new PrologModule());
install(new SshAddressesModule());
install(ThreadLocalRequestContext.module());

7
gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
View File

@@ -48,6 +48,7 @@ import com.google.gerrit.server.data.RefUpdateAttribute;
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 @@ public class EventFactory {
* 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) {

20
gerrit-server/src/main/java/com/google/gerrit/server/git/CherryPick.java
View File

@@ -132,8 +132,7 @@ public class CherryPick extends SubmitStrategy {
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 @@ public class CherryPick extends SubmitStrategy {
}
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 @@ public class CherryPick extends SubmitStrategy {
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 @@ public class CherryPick extends SubmitStrategy {
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 @@ public class CherryPick extends SubmitStrategy {
args.db.rollback();
}
gitRefUpdated.fire(n.change.getProject(), ru);
gitRefUpdated.fire(n.getChange().getProject(), ru);
newCommit.copyFrom(n);
newCommit.statusCode = CommitMergeStatus.CLEAN_PICK;

11
gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.git;
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 @@ public class CodeReviewCommit extends RevCommit {
*/
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 @@ public class CodeReviewCommit extends RevCommit {
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();
}
}

61
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -52,6 +52,8 @@ import com.google.gerrit.server.git.validators.MergeValidators;
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 @@ public class MergeOp {
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 @@ public class MergeOp {
@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 @@ public class MergeOp {
final ApprovalsUtil approvalsUtil) {
repoManager = grm;
schemaFactory = sf;
notesFactory = nf;
labelNormalizer = fs;
projectCache = pc;
gitRefUpdated = gru;
@@ -278,8 +283,8 @@ public class MergeOp {
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 @@ public class MergeOp {
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 @@ public class MergeOp {
continue;
}
commit.change = chg;
commit.notes = notesFactory.create(chg);
commit.patchsetId = ps.getId();
commit.originalOrder = commitOrder++;
commits.put(changeId, commit);
@@ -621,8 +626,8 @@ public class MergeOp {
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 @@ public class MergeOp {
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 @@ public class MergeOp {
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 @@ public class MergeOp {
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 @@ public class MergeOp {
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 @@ public class MergeOp {
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 @@ public class MergeOp {
}
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 @@ public class MergeOp {
// 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 @@ public class MergeOp {
});
}
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 @@ public class MergeOp {
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 @@ public class MergeOp {
}
}
}
// TODO(dborowitz): Store normalized labels in notedb.
db.patchSetApprovals().update(approvals);
db.patchSetApprovals().deleteKeys(toDelete);
} catch (NoSuchChangeException err) {
@@ -956,8 +963,12 @@ public class MergeOp {
}));
}
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 @@ public class MergeOp {
}
}
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 @@ public class MergeOp {
}
final boolean setStatusNew = makeNew;
final Change c = notes.getChange();
Change change = null;
try {
db.changes().beginTransaction(c.getId());

21
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@@ -24,7 +24,6 @@ import com.google.gerrit.common.Nullable;
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 class MergeUtil {
});
}
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 @@ public class MergeUtil {
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 @@ public class MergeUtil {
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 @@ public class MergeUtil {
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 @@ public class MergeUtil {
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 @@ public class MergeUtil {
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 @@ public class MergeUtil {
if (c.patchsetId != null) {
c.statusCode = CommitMergeStatus.CLEAN_MERGE;
if (submitApproval == null) {
submitApproval = getSubmitter(c.patchsetId);
submitApproval = getSubmitter(c);
}
}
}

25
gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseIfNecessary.java
View File

@@ -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 @@ public class RebaseIfNecessary extends SubmitStrategy {
} 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) {

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

@@ -18,6 +18,7 @@ import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
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.CreateChangeSender;
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 @@ public class ReceiveCommits {
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 @@ public class ReceiveCommits {
@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 @@ public class ReceiveCommits {
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 @@ public class ReceiveCommits {
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 @@ public class ReceiveCommits {
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 @@ public class ReceiveCommits {
} finally {
db.rollback();
}
update.commit();
if (mergedIntoRef != null) {
// Change was already submitted to a branch, close it.

11
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategy.java
View File

@@ -23,6 +23,9 @@ import com.google.gerrit.server.ApprovalsUtil;
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 @@ public abstract class SubmitStrategy {
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 @@ public abstract class SubmitStrategy {
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 @@ public abstract class SubmitStrategy {
this.identifiedUserFactory = identifiedUserFactory;
this.myIdent = myIdent;
this.db = db;
this.notesFactory = notesFactory;
this.updateFactory = updateFactory;
this.repo = repo;
this.rw = rw;

15
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmitStrategyFactory.java
View File

@@ -23,6 +23,8 @@ import com.google.gerrit.server.IdentifiedUser;
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 @@ public class SubmitStrategyFactory {
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 @@ public class SubmitStrategyFactory {
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 @@ public class SubmitStrategyFactory {
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 @@ public class SubmitStrategyFactory {
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);

3
gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
View File

@@ -119,7 +119,8 @@ public class MergeValidators {
}
} 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);

22
gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
View File

@@ -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 @@ public abstract class ChangeEmail extends NotificationEmail {
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 @@ public abstract class ChangeEmail extends NotificationEmail {
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 @@ public abstract class ChangeEmail extends NotificationEmail {
/** 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 @@ public abstract class ChangeEmail extends NotificationEmail {
/** 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) {

4
gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java
View File

@@ -28,6 +28,7 @@ import com.google.gerrit.server.config.AllProjectsName;
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 @@ class EmailArguments {
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 @@ class EmailArguments {
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 @@ class EmailArguments {
this.patchSetInfoFactory = patchSetInfoFactory;
this.identifiedUserFactory = identifiedUserFactory;
this.capabilityControlFactory = capabilityControlFactory;
this.changeNotesFactory = changeNotesFactory;
this.anonymousUser = anonymousUser;
this.anonymousCowardName = anonymousCowardName;
this.urlProvider = urlProvider;

4
gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
View File

@@ -61,8 +61,8 @@ public class MergedSender extends ReplyToChangeSender {
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;

19
gerrit-server/src/main/java/com/google/gerrit/server/mail/PatchSetNotificationSender.java
View File

@@ -31,6 +31,8 @@ import com.google.gerrit.server.account.AccountResolver;
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 @@ public class PatchSetNotificationSender {
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 @@ public class PatchSetNotificationSender {
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 @@ public class PatchSetNotificationSender {
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,

7
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
View File

@@ -73,8 +73,9 @@ public class ChangeNotes extends VersionedMetaData {
public static class Factory {
private final GitRepositoryManager repoManager;
@VisibleForTesting
@Inject
Factory(GitRepositoryManager repoManager) {
public Factory(GitRepositoryManager repoManager) {
this.repoManager = repoManager;
}
@@ -261,6 +262,10 @@ public class ChangeNotes extends VersionedMetaData {
return this;
}
public Change.Id getChangeId() {
return change.getId();
}
public Change getChange() {
return change;
}

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

@@ -40,6 +40,7 @@ import com.google.inject.assistedinject.AssistedInject;
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 @@ public class ChangeUpdate extends VersionedMetaData {
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 @@ public class ChangeUpdate extends VersionedMetaData {
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 @@ public class ChangeUpdate extends VersionedMetaData {
ChangeUpdate(
PersonIdent serverIdent,
GitRepositoryManager repoManager,
NotesMigration migration,
AccountCache accountCache,
MetaDataUpdate.User updateFactory,
LabelTypes labelTypes,
@@ -133,6 +139,7 @@ public class ChangeUpdate extends VersionedMetaData {
Date when,
IdentifiedUser user) {
this.repoManager = repoManager;
this.migration = migration;
this.accountCache = accountCache;
this.updateFactory = updateFactory;
this.labelTypes = labelTypes;
@@ -144,6 +151,10 @@ public class ChangeUpdate extends VersionedMetaData {
this.reviewers = Maps.newLinkedHashMap();
}
public Change getChange() {
return change;
}
public IdentifiedUser getUser() {
return user;
}
@@ -181,6 +192,9 @@ public class ChangeUpdate extends VersionedMetaData {
@Override
public RevCommit commit(MetaDataUpdate md) throws IOException {
if (!migration.write()) {
return null;
}
Repository repo = repoManager.openRepository(change.getProject());
try {
load(repo);
@@ -207,6 +221,44 @@ public class ChangeUpdate extends VersionedMetaData {
return newIdent(user.getAccount());
}
@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());

24
gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java Normal file
View File

@@ -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);
}
}

62
gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java Normal file
View File

@@ -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;
}
}

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

@@ -690,7 +690,7 @@ public class ChangeControl {
}
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)

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

@@ -36,6 +36,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
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 @@ public class ChangeData {
}
}
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 @@ public class ChangeData {
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 @@ public class ChangeData {
* @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 @@ public class ChangeData {
@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 @@ public class ChangeData {
@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 @@ public class ChangeData {
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 @@ public class ChangeData {
(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 @@ public class ChangeData {
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 class ChangeData {
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;
}

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

@@ -317,7 +317,7 @@ public class QueryProcessor {
}
if (includeAllReviewers) {
eventFactory.addAllReviewers(c, d.change());
eventFactory.addAllReviewers(c, d.notes());
}
if (includeSubmitRecords) {

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

@@ -60,7 +60,7 @@ public class LabelVote {
private final short value;
public LabelVote(String name, short value) {
this.name = LabelType.checkName(name);
this.name = LabelType.checkNameInternal(name);
this.value = value;
}

16
gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
View File

@@ -30,6 +30,8 @@ import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
private Provider<String> urlProvider;
private GitRepositoryManager repoManager;
private GitReferenceUpdated gitRefUpdated;
private ChangeNotes.Factory notesFactory;
@SuppressWarnings("unchecked")
@Override
@@ -92,6 +95,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
urlProvider = createStrictMock(Provider.class);
repoManager = createStrictMock(GitRepositoryManager.class);
gitRefUpdated = createStrictMock(GitReferenceUpdated.class);
notesFactory = new ChangeNotes.Factory(repoManager);
}
private void doReplay() {
@@ -612,11 +616,11 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
subscribers);
expect(repoManager.openRepository(targetBranchNameKey.getParentKey()))
.andReturn(targetRepository);
.andReturn(targetRepository).anyTimes();
Capture<RefUpdate> ruCapture = new Capture<RefUpdate>();
gitRefUpdated.fire(eq(targetBranchNameKey.getParentKey()),

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

@@ -211,7 +211,7 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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 @@ public class ChangeNotesTest {
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();
}

15
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.query.change;
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.ChangeJson.ChangeInfo;
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 @@ public abstract class AbstractQueryChangesTest {
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 @@ public abstract class AbstractQueryChangesTest {
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 @@ public abstract class AbstractQueryChangesTest {
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 @@ public abstract class AbstractQueryChangesTest {
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 @@ public abstract class AbstractQueryChangesTest {
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 @@ public abstract class AbstractQueryChangesTest {
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 @@ public abstract class AbstractQueryChangesTest {
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 @@ public abstract class AbstractQueryChangesTest {
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"));

1
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
View File

@@ -116,6 +116,7 @@ public class ReviewCommand extends SshCommand {
@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());
}

2
plugins/reviewnotes

Submodule plugins/reviewnotes updated: 0b0419b4aa...6df20f370c