Merge "Move change id and status to left side of ChangeScreen2"
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java
index 1cbd20d..4d457ad 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java
@@ -20,6 +20,9 @@
String openChange();
String reviewedFileTitle();
+ String openLastFile();
+ String openCommitMessage();
+
String patchSet();
String commit();
String date();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties
index 71d0bde..289e9b4 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties
@@ -3,6 +3,9 @@
openChange = Open related change
reviewedFileTitle = Mark file as reviewed (Shortcut: r)
+openLastFile = Open last file
+openCommitMessage = Open commit message
+
patchSet = Patch Set
commit = Commit
date = Date
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java
index c53619b..da1b3dc 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java
@@ -239,6 +239,10 @@
keysNavigation.add(new OpenKeyCommand(0, KeyCodes.KEY_ENTER,
Util.C.patchTableOpenDiff()));
+ keysNavigation.add(
+ new OpenFileCommand(list.length() - 1, 0, '[', Resources.C.openLastFile()),
+ new OpenFileCommand(0, 0, ']', Resources.C.openCommitMessage()));
+
keysAction.add(new KeyCommand(0, 'r', PatchUtil.C.toggleReviewed()) {
@Override
public void onKeyPress(KeyPressEvent event) {
@@ -330,6 +334,20 @@
super.onCellSingleClick(event, row, column);
}
}
+
+ private class OpenFileCommand extends KeyCommand {
+ private final int index;
+
+ OpenFileCommand(int index, int modifiers, char c, String helpText) {
+ super(modifiers, c, helpText);
+ this.index = index;
+ }
+
+ @Override
+ public void onKeyPress(KeyPressEvent event) {
+ Gerrit.display(url(list.get(index)));
+ }
+ }
}
private final class DisplayCommand implements RepeatingCommand {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css
index 8efc9f7..67b86ed 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css
@@ -14,6 +14,8 @@
*/
.commentWidgets {
+ max-width: 650px;
+
font-family: sans-serif;
background-color: #fcfa96;
border: 1px solid black;
@@ -38,7 +40,6 @@
}
.header {
- max-width: 650px;
cursor: pointer;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
index 23630ea..3b712ad 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
@@ -148,11 +148,18 @@
lineWidget = null;
updateSelection();
}
- manager.clearLine(cm.side(), line);
+ manager.clearLine(cm.side(), line, this);
removeFromParent();
}
- void attach(DiffTable parent) {
+ void attachPair(DiffTable parent) {
+ if (lineWidget == null && peer.lineWidget == null) {
+ this.attach(parent);
+ peer.attach(parent);
+ }
+ }
+
+ private void attach(DiffTable parent) {
parent.add(this);
lineWidget = cm.addLineWidget(Math.max(0, line - 1), getElement(),
Configuration.create()
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
index 03f8e2a..fef52c6 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
@@ -134,10 +134,10 @@
setExpandAllComments(true);
}
for (CommentGroup g : sideA.values()) {
- g.attach(host.diffTable);
+ g.attachPair(host.diffTable);
}
for (CommentGroup g : sideB.values()) {
- g.attach(host.diffTable);
+ g.attachPair(host.diffTable);
g.handleRedraw();
}
attached = true;
@@ -280,8 +280,11 @@
}
}
- void clearLine(DisplaySide side, int line) {
- map(side).remove(line);
+ void clearLine(DisplaySide side, int line, CommentGroup group) {
+ SortedMap<Integer, CommentGroup> map = map(side);
+ if (map.get(line) == group) {
+ map.remove(line);
+ }
}
Runnable toggleOpenBox(final CodeMirror cm) {
@@ -398,8 +401,7 @@
sideB.put(lineB, b);
if (attached) {
- a.attach(host.diffTable);
- b.attach(host.diffTable);
+ a.attachPair(host.diffTable);
b.handleRedraw();
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
index aba6745..e4202fc 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
@@ -65,6 +65,7 @@
private Timer resizeTimer;
private int editAreaHeight;
private boolean autoClosed;
+ private CallbackGroup pendingGroup;
@UiField Widget header;
@UiField Element summary;
@@ -238,7 +239,7 @@
}
private void restoreSelection() {
- if (getFromTo() != null) {
+ if (getFromTo() != null && comment.in_reply_to() == null) {
getCm().setSelection(getFromTo().getFrom(), getFromTo().getTo());
}
}
@@ -262,10 +263,17 @@
@UiHandler("save")
void onSave(ClickEvent e) {
e.stopPropagation();
- save(null);
+ CallbackGroup group = new CallbackGroup();
+ save(group);
+ group.done();
}
void save(CallbackGroup group) {
+ if (pendingGroup != null) {
+ pendingGroup.addListener(group);
+ return;
+ }
+
String message = editArea.getValue().trim();
if (message.length() == 0) {
return;
@@ -275,10 +283,12 @@
input.message(message);
enableEdit(false);
+ pendingGroup = group;
GerritCallback<CommentInfo> cb = new GerritCallback<CommentInfo>() {
@Override
public void onSuccess(CommentInfo result) {
enableEdit(true);
+ pendingGroup = null;
set(result);
setEdit(false);
if (autoClosed) {
@@ -290,14 +300,14 @@
@Override
public void onFailure(Throwable e) {
enableEdit(true);
+ pendingGroup = null;
super.onFailure(e);
}
};
if (input.id() == null) {
- CommentApi.createDraft(psId, input, group == null ? cb : group.add(cb));
+ CommentApi.createDraft(psId, input, group.add(cb));
} else {
- CommentApi.updateDraft(
- psId, input.id(), input, group == null ? cb : group.add(cb));
+ CommentApi.updateDraft(psId, input.id(), input, group.add(cb));
}
getCm().focus();
}
@@ -332,13 +342,15 @@
restoreSelection();
} else {
setEdit(false);
+ pendingGroup = new CallbackGroup();
CommentApi.deleteDraft(psId, comment.id(),
- new GerritCallback<JavaScriptObject>() {
+ pendingGroup.addFinal(new GerritCallback<JavaScriptObject>() {
@Override
public void onSuccess(JavaScriptObject result) {
+ pendingGroup = null;
removeUI();
}
- });
+ }));
}
}
@@ -351,7 +363,9 @@
case 's':
case 'S':
e.preventDefault();
- save(null);
+ CallbackGroup group = new CallbackGroup();
+ save(group);
+ group.done();
return;
}
} else if (e.getNativeKeyCode() == KeyCodes.KEY_ESCAPE && !isDirty()) {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
index 182c5db..fc95480 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
@@ -251,7 +251,9 @@
removeKeyHandlerRegistrations();
if (commentManager != null) {
- commentManager.saveAllDrafts(null);
+ CallbackGroup group = new CallbackGroup();
+ commentManager.saveAllDrafts(group);
+ group.done();
}
if (resizeHandler != null) {
resizeHandler.removeHandler();
@@ -694,7 +696,8 @@
public void run() {
CallbackGroup group = new CallbackGroup();
commentManager.saveAllDrafts(group);
- group.addFinal(new GerritCallback<Void>() {
+ group.done();
+ group.addListener(new GerritCallback<Void>() {
@Override
public void onSuccess(Void result) {
String b = base != null ? String.valueOf(base.get()) : null;
@@ -703,7 +706,7 @@
PageLinks.toChange(changeId, rev),
new ChangeScreen2(changeId, b, rev, openReplyBox));
}
- }).onSuccess(null);
+ });
}
};
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/CallbackGroup.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/CallbackGroup.java
index 073c949..51a899d 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/CallbackGroup.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/CallbackGroup.java
@@ -82,6 +82,18 @@
applyAllSuccess();
}
+ public void addListener(AsyncCallback<Void> cb) {
+ if (!failed && finalAdded && remaining.isEmpty()) {
+ cb.onSuccess(null);
+ } else {
+ handleAdd(cb).onSuccess(null);
+ }
+ }
+
+ public void addListener(CallbackGroup group) {
+ addListener(group.add(CallbackGroup.<Void> emptyCallback()));
+ }
+
private void applyAllSuccess() {
if (!failed && finalAdded && remaining.isEmpty()) {
for (CallbackImpl<?> cb : callbacks) {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
index 6bd5429..7afded5 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
@@ -23,6 +23,7 @@
import com.google.gerrit.client.RpcStatus;
import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.core.client.Scheduler;
import com.google.gwt.http.client.Request;
import com.google.gwt.http.client.RequestBuilder;
import com.google.gwt.http.client.RequestBuilder.Method;
@@ -124,7 +125,8 @@
}
} else if (200 <= status && status < 300) {
- T data;
+ long start = System.currentTimeMillis();
+ final T data;
if (isTextBody(res)) {
data = NativeString.wrap(res.getText()).cast();
} else if (isJsonBody(res)) {
@@ -149,14 +151,25 @@
return;
}
- try {
- cb.onSuccess(data);
- } finally {
- if (!background) {
- RpcStatus.INSTANCE.onRpcComplete();
+ Scheduler.ScheduledCommand cmd = new Scheduler.ScheduledCommand() {
+ @Override
+ public void execute() {
+ try {
+ cb.onSuccess(data);
+ } finally {
+ if (!background) {
+ RpcStatus.INSTANCE.onRpcComplete();
+ }
+ }
}
- }
+ };
+ // Defer handling the response if the parse took a while.
+ if ((System.currentTimeMillis() - start) > 75) {
+ Scheduler.get().scheduleDeferred(cmd);
+ } else {
+ cmd.execute();
+ }
} else {
String msg;
if (isTextBody(res)) {
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index b92d1cb..73b1be3 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -234,7 +234,7 @@
Term id = QueryBuilder.idTerm(cd);
Document doc = toDocument(cd);
try {
- if (cd.getChange().getStatus().isOpen()) {
+ if (cd.change().getStatus().isOpen()) {
Futures.allAsList(
closedIndex.delete(id),
openIndex.insert(doc)).get();
@@ -243,9 +243,7 @@
openIndex.delete(id),
closedIndex.insert(doc)).get();
}
- } catch (ExecutionException e) {
- throw new IOException(e);
- } catch (InterruptedException e) {
+ } catch (OrmException | ExecutionException | InterruptedException e) {
throw new IOException(e);
}
}
@@ -256,7 +254,7 @@
Term id = QueryBuilder.idTerm(cd);
Document doc = toDocument(cd);
try {
- if (cd.getChange().getStatus().isOpen()) {
+ if (cd.change().getStatus().isOpen()) {
Futures.allAsList(
closedIndex.delete(id),
openIndex.replace(id, doc)).get();
@@ -265,9 +263,7 @@
openIndex.delete(id),
closedIndex.replace(id, doc)).get();
}
- } catch (ExecutionException e) {
- throw new IOException(e);
- } catch (InterruptedException e) {
+ } catch (OrmException | ExecutionException | InterruptedException e) {
throw new IOException(e);
}
}
@@ -280,9 +276,7 @@
Futures.allAsList(
openIndex.delete(id),
closedIndex.delete(id)).get();
- } catch (ExecutionException e) {
- throw new IOException(e);
- } catch (InterruptedException e) {
+ } catch (ExecutionException | InterruptedException e) {
throw new IOException(e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
index 3b3dbb8..356b7d5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
@@ -35,11 +35,11 @@
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.SystemException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
@@ -52,10 +52,19 @@
public static final StoredValue<PatchSet> PATCH_SET = create(PatchSet.class);
public static final StoredValue<ChangeControl> CHANGE_CONTROL = create(ChangeControl.class);
+ public static Change getChange(Prolog engine) throws SystemException {
+ ChangeData cd = CHANGE_DATA.get(engine);
+ try {
+ return cd.change();
+ } catch (OrmException e) {
+ throw new SystemException("Cannot load change " + cd.getId());
+ }
+ }
+
public static final StoredValue<PatchSetInfo> PATCH_SET_INFO = new StoredValue<PatchSetInfo>() {
@Override
public PatchSetInfo createValue(Prolog engine) {
- Change change = StoredValues.CHANGE_DATA.get(engine).getChange();
+ Change change = getChange(engine);
PatchSet ps = StoredValues.PATCH_SET.get(engine);
PrologEnvironment env = (PrologEnvironment) engine.control;
PatchSetInfoFactory patchInfoFactory =
@@ -74,7 +83,7 @@
PrologEnvironment env = (PrologEnvironment) engine.control;
PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
PatchListCache plCache = env.getArgs().getPatchListCache();
- Change change = StoredValues.CHANGE_DATA.get(engine).getChange();
+ Change change = getChange(engine);
Project.NameKey projectKey = change.getProject();
ObjectId a = null;
ObjectId b = ObjectId.fromString(psInfo.getRevId());
@@ -95,13 +104,11 @@
public Repository createValue(Prolog engine) {
PrologEnvironment env = (PrologEnvironment) engine.control;
GitRepositoryManager gitMgr = env.getArgs().getGitRepositoryManager();
- Change change = StoredValues.CHANGE_DATA.get(engine).getChange();
+ Change change = getChange(engine);
Project.NameKey projectKey = change.getProject();
final Repository repo;
try {
repo = gitMgr.openRepository(projectKey);
- } catch (RepositoryNotFoundException e) {
- throw new SystemException(e.getMessage());
} catch (IOException e) {
throw new SystemException(e.getMessage());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index 7b7476f..25654d2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -19,10 +19,10 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
-import com.google.common.collect.Multimaps;
import com.google.common.collect.Ordering;
import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
@@ -36,6 +36,7 @@
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.ReviewerState;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -77,21 +78,18 @@
public ApprovalsUtil() {
}
- public static enum ReviewerState {
- REVIEWER, CC;
- }
-
/**
* Get all reviewers for a change.
*
* @param db review database.
* @param changeId change ID.
* @return multimap of reviewers keyed by state, where each account appears
- * exactly once in {@link SetMultimap#values()}.
+ * 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 SetMultimap<ReviewerState, Account.Id> getReviewers(ReviewDb db,
- Change.Id changeId) throws OrmException {
+ public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers(
+ ReviewDb db, Change.Id changeId) throws OrmException {
return getReviewers(db.patchSetApprovals().byChange(changeId));
}
@@ -101,9 +99,10 @@
* @param allApprovals all approvals to consider; must all belong to the same
* change.
* @return multimap of reviewers keyed by state, where each account appears
- * exactly once in {@link SetMultimap#values()}.
+ * exactly once in {@link SetMultimap#values()}, and
+ * {@link ReviewerState#REMOVED} is not present.
*/
- public static SetMultimap<ReviewerState, Account.Id> getReviewers(
+ public static ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers(
Iterable<PatchSetApproval> allApprovals) {
PatchSetApproval first = null;
SetMultimap<ReviewerState, Account.Id> reviewers =
@@ -125,7 +124,7 @@
reviewers.put(ReviewerState.CC, id);
}
}
- return Multimaps.unmodifiableSetMultimap(reviewers);
+ return ImmutableSetMultimap.copyOf(reviewers);
}
/**
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 705bf6b..a1116a1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -384,7 +384,7 @@
}
LabelTypes labelTypes = ctl.getLabelTypes();
- if (cd.getChange().getStatus().isOpen()) {
+ if (cd.change().getStatus().isOpen()) {
return labelsForOpenChange(cd, labelTypes, standard, detailed);
} else {
return labelsForClosedChange(cd, labelTypes, standard, detailed);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index 99292f3..1840e76 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -28,7 +28,6 @@
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.ApprovalsUtil.ReviewerState;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.events.CommitReceivedEvent;
@@ -36,6 +35,7 @@
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
+import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
index 43e0975..01e191f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
@@ -97,6 +97,7 @@
private final Project.NameKey projectName;
private final Repository db;
private final CommitBuilder commit;
+ private boolean allowEmpty;
@Inject
public MetaDataUpdate(GitReferenceUpdated gitRefUpdated,
@@ -118,6 +119,10 @@
getCommitBuilder().getCommitter().getTimeZone()));
}
+ public void setAllowEmpty(boolean allowEmpty) {
+ this.allowEmpty = allowEmpty;
+ }
+
/** Close the cached Repository handle. */
public void close() {
getRepository().close();
@@ -131,6 +136,10 @@
return db;
}
+ boolean allowEmpty() {
+ return allowEmpty;
+ }
+
public CommitBuilder getCommitBuilder() {
return commit;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index a9710e1..59431b5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -18,7 +18,6 @@
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;
@@ -64,7 +63,6 @@
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.ApprovalsUtil.ReviewerState;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -90,6 +88,7 @@
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.ReviewerState;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
index 0719c7d..4cee593 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
@@ -213,7 +213,7 @@
doSave(config, commit);
final ObjectId res = newTree.writeTree(inserter);
- if (res.equals(srcTree)) {
+ if (res.equals(srcTree) && !update.allowEmpty()) {
// If there are no changes to the content, don't create the commit.
return;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index 47baa9e..09f9e42 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -25,9 +25,9 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.StarredChange;
-import com.google.gerrit.server.ApprovalsUtil.ReviewerState;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.mail.ProjectWatch.Watchers;
+import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
index 801e599..ac367ef 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
@@ -18,8 +18,8 @@
import com.google.gerrit.common.errors.NoSuchAccountException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.server.ApprovalsUtil.ReviewerState;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.revwalk.FooterKey;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
new file mode 100644
index 0000000..60d6f56
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -0,0 +1,45 @@
+// 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.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.RefNames;
+
+import org.eclipse.jgit.revwalk.FooterKey;
+
+public class ChangeNoteUtil {
+ static final String GERRIT_PLACEHOLDER_HOST = "gerrit";
+
+ static final FooterKey FOOTER_LABEL = new FooterKey("Label");
+ static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-Set");
+
+ public static String changeRefName(Change.Id id) {
+ StringBuilder r = new StringBuilder();
+ r.append(RefNames.REFS_CHANGES);
+ int n = id.get();
+ int m = n % 100;
+ if (m < 10) {
+ r.append('0');
+ }
+ r.append(m);
+ r.append('/');
+ r.append(n);
+ r.append("/meta");
+ return r.toString();
+ }
+
+ private ChangeNoteUtil() {
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
new file mode 100644
index 0000000..667e3f0
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -0,0 +1,309 @@
+// 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.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Ordering;
+import com.google.common.collect.Sets;
+import com.google.common.primitives.Ints;
+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.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.VersionedMetaData;
+import com.google.gerrit.server.util.LabelVote;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+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.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.util.RawParseUtils;
+
+import java.io.IOException;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/** View of a single {@link Change} based on the log of its notes branch. */
+public class ChangeNotes extends VersionedMetaData {
+ private static final Ordering<PatchSetApproval> PSA_BY_TIME =
+ Ordering.natural().onResultOf(
+ new Function<PatchSetApproval, Timestamp>() {
+ @Override
+ public Timestamp apply(PatchSetApproval input) {
+ return input.getGranted();
+ }
+ });
+
+ @Singleton
+ public static class Factory {
+ private final GitRepositoryManager repoManager;
+
+ @Inject
+ Factory(GitRepositoryManager repoManager) {
+ this.repoManager = repoManager;
+ }
+
+ public ChangeNotes create(Change change) {
+ return new ChangeNotes(repoManager, change);
+ }
+ }
+
+ private static class Parser {
+ private final Change.Id changeId;
+ private final ObjectId tip;
+ private final RevWalk walk;
+ private final ListMultimap<PatchSet.Id, PatchSetApproval> approvals;
+ private final Map<Account.Id, ReviewerState> reviewers;
+
+ private Parser(Change.Id changeId, ObjectId tip, RevWalk walk) {
+ this.changeId = changeId;
+ this.tip = tip;
+ this.walk = walk;
+ approvals = ArrayListMultimap.create();
+ reviewers = Maps.newLinkedHashMap();
+ }
+
+ private void parseAll() throws ConfigInvalidException, IOException {
+ walk.markStart(walk.parseCommit(tip));
+ for (RevCommit commit : walk) {
+ parse(commit);
+ }
+ pruneReviewers();
+ for (Collection<PatchSetApproval> v : approvals.asMap().values()) {
+ Collections.sort((List<PatchSetApproval>) v, PSA_BY_TIME);
+ }
+ }
+
+ private void parse(RevCommit commit) throws ConfigInvalidException {
+ PatchSet.Id psId = parsePatchSetId(commit);
+ Account.Id accountId = parseIdent(commit);
+ List<PatchSetApproval> psas = approvals.get(psId);
+
+ Map<String, PatchSetApproval> curr =
+ Maps.newHashMapWithExpectedSize(psas.size());
+ for (PatchSetApproval psa : psas) {
+ if (psa.getAccountId().equals(accountId)) {
+ curr.put(psa.getLabel(), psa);
+ }
+ }
+
+ for (String line : commit.getFooterLines(FOOTER_LABEL)) {
+ PatchSetApproval psa = parseApproval(psId, accountId, commit, line);
+ if (!curr.containsKey(psa.getLabel())) {
+ curr.put(psa.getLabel(), psa);
+ psas.add(psa);
+ }
+ }
+ for (ReviewerState state : ReviewerState.values()) {
+ for (String line : commit.getFooterLines(state.getFooterKey())) {
+ parseReviewer(state, line);
+ }
+ }
+ }
+
+ private PatchSet.Id parsePatchSetId(RevCommit commit)
+ throws ConfigInvalidException {
+ List<String> psIdLines = commit.getFooterLines(FOOTER_PATCH_SET);
+ if (psIdLines.size() != 1) {
+ throw parseException("missing or multiple %s: %s",
+ FOOTER_PATCH_SET, psIdLines);
+ }
+ Integer psId = Ints.tryParse(psIdLines.get(0));
+ if (psId == null) {
+ throw parseException("invalid %s: %s",
+ FOOTER_PATCH_SET, psIdLines.get(0));
+ }
+ return new PatchSet.Id(changeId, psId);
+ }
+
+ private PatchSetApproval parseApproval(PatchSet.Id psId, Account.Id accountId,
+ RevCommit commit, String line) throws ConfigInvalidException {
+ try {
+ LabelVote l = LabelVote.parseWithEquals(line);
+ return new PatchSetApproval(
+ new PatchSetApproval.Key(
+ psId, parseIdent(commit), new LabelId(l.getLabel())),
+ l.getValue(),
+ new Timestamp(commit.getCommitterIdent().getWhen().getTime()));
+ } catch (IllegalArgumentException e) {
+ ConfigInvalidException pe =
+ parseException("invalid %s: %s", FOOTER_LABEL, line);
+ pe.initCause(e);
+ throw pe;
+ }
+ }
+
+ private Account.Id parseIdent(RevCommit commit)
+ throws ConfigInvalidException {
+ return parseIdent(commit.getCommitterIdent());
+ }
+
+ private Account.Id parseIdent(PersonIdent ident)
+ throws ConfigInvalidException {
+ String email = ident.getEmailAddress();
+ int at = email.indexOf('@');
+ if (at >= 0) {
+ String host = email.substring(at + 1, email.length());
+ Integer id = Ints.tryParse(email.substring(0, at));
+ if (id != null && host.equals(GERRIT_PLACEHOLDER_HOST)) {
+ return new Account.Id(id);
+ }
+ }
+ throw parseException("invalid identity, expected <id>@%s: %s",
+ GERRIT_PLACEHOLDER_HOST, email);
+ }
+
+ private void parseReviewer(ReviewerState state, String line)
+ throws ConfigInvalidException {
+ PersonIdent ident = RawParseUtils.parsePersonIdent(line);
+ if (ident == null) {
+ throw parseException(
+ "invalid %s: %s", state.getFooterKey().getName(), line);
+ }
+ Account.Id accountId = parseIdent(ident);
+ if (!reviewers.containsKey(accountId)) {
+ reviewers.put(accountId, state);
+ }
+ }
+
+ private void pruneReviewers() {
+ Set<Account.Id> removed = Sets.newHashSetWithExpectedSize(reviewers.size());
+ Iterator<Map.Entry<Account.Id, ReviewerState>> rit =
+ reviewers.entrySet().iterator();
+ while (rit.hasNext()) {
+ Map.Entry<Account.Id, ReviewerState> e = rit.next();
+ if (e.getValue() == ReviewerState.REMOVED) {
+ removed.add(e.getKey());
+ rit.remove();
+ }
+ }
+
+ Iterator<Map.Entry<PatchSet.Id, PatchSetApproval>> ait =
+ approvals.entries().iterator();
+ while (ait.hasNext()) {
+ if (removed.contains(ait.next().getValue().getAccountId())) {
+ ait.remove();
+ }
+ }
+ }
+
+ private ConfigInvalidException parseException(String fmt, Object... args) {
+ return new ConfigInvalidException("Change " + changeId + ": "
+ + String.format(fmt, args));
+ }
+ }
+
+ private final GitRepositoryManager repoManager;
+ private final Change change;
+ private boolean loaded;
+ private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
+ private ImmutableSetMultimap<ReviewerState, Account.Id> reviewers;
+
+ @VisibleForTesting
+ ChangeNotes(GitRepositoryManager repoManager, Change change) {
+ this.repoManager = repoManager;
+ this.change = change;
+ }
+
+ // TODO(dborowitz): Wrap fewer exceptions if/when we kill gwtorm.
+ public ChangeNotes load() throws OrmException {
+ if (!loaded) {
+ Repository repo;
+ try {
+ repo = repoManager.openRepository(change.getProject());
+ } catch (IOException e) {
+ throw new OrmException(e);
+ }
+ try {
+ load(repo);
+ loaded = true;
+ } catch (ConfigInvalidException | IOException e) {
+ throw new OrmException(e);
+ } finally {
+ repo.close();
+ }
+ }
+ return this;
+ }
+
+ public Change getChange() {
+ return change;
+ }
+
+ public ImmutableListMultimap<PatchSet.Id, PatchSetApproval> getApprovals() {
+ return approvals;
+ }
+
+ public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers() {
+ return reviewers;
+ }
+
+ @Override
+ protected String getRefName() {
+ return ChangeNoteUtil.changeRefName(change.getId());
+ }
+
+ @Override
+ protected void onLoad() throws IOException, ConfigInvalidException {
+ ObjectId rev = getRevision();
+ if (rev == null) {
+ return;
+ }
+ RevWalk walk = new RevWalk(reader);
+ try {
+ Parser parser = new Parser(change.getId(), rev, walk);
+ parser.parseAll();
+ approvals = ImmutableListMultimap.copyOf(parser.approvals);
+ ImmutableSetMultimap.Builder<ReviewerState, Account.Id> reviewers =
+ ImmutableSetMultimap.builder();
+ for (Map.Entry<Account.Id, ReviewerState> e
+ : parser.reviewers.entrySet()) {
+ reviewers.put(e.getValue(), e.getKey());
+ }
+ this.reviewers = reviewers.build();
+ } finally {
+ walk.release();
+ }
+ }
+
+ @Override
+ protected void onSave(CommitBuilder commit) {
+ throw new UnsupportedOperationException(
+ getClass().getSimpleName() + " is read-only");
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
new file mode 100644
index 0000000..ae7beac
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -0,0 +1,252 @@
+// 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 static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Maps;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.LabelTypes;
+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.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.VersionedMetaData;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.util.LabelVote;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.FooterKey;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+import java.io.IOException;
+import java.util.Date;
+import java.util.Map;
+import java.util.TimeZone;
+
+/**
+ * A single delta to apply atomically to a change.
+ * <p>
+ * This delta becomes a single commit on the notes branch, so there are
+ * limitations on the set of modifications that can be handled in a single
+ * update. In particular, there is a single author and timestamp for each
+ * update.
+ * <p>
+ * This class is not thread-safe.
+ */
+public class ChangeUpdate extends VersionedMetaData {
+ @Singleton
+ public static class Factory {
+ private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
+ private final MetaDataUpdate.User updateFactory;
+ private final ProjectCache projectCache;
+ private final Provider<IdentifiedUser> user;
+ private final PersonIdent serverIdent;
+
+ @Inject
+ Factory(
+ GitRepositoryManager repoManager,
+ AccountCache accountCache,
+ MetaDataUpdate.User updateFactory,
+ ProjectCache projectCache,
+ Provider<IdentifiedUser> user,
+ @GerritPersonIdent PersonIdent serverIdent) {
+ this.repoManager = repoManager;
+ this.accountCache = accountCache;
+ this.updateFactory = updateFactory;
+ this.projectCache = projectCache;
+ this.user = user;
+ this.serverIdent = serverIdent;
+ }
+
+ public ChangeUpdate create(Change change) {
+ return create(change, serverIdent.getWhen());
+ }
+
+ public ChangeUpdate create(Change change, Date when) {
+ return new ChangeUpdate(
+ repoManager, accountCache, updateFactory,
+ projectCache.get(change.getProject()).getLabelTypes(),
+ change, user.get().getAccount(), when, serverIdent.getTimeZone());
+ }
+ }
+
+ private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
+ private final MetaDataUpdate.User updateFactory;
+ private final LabelTypes labelTypes;
+ private final Change change;
+ private final Account account;
+ private final Date when;
+ private final TimeZone tz;
+ private final Map<String, Short> approvals;
+ private final Map<Account.Id, ReviewerState> reviewers;
+ private String subject;
+ private PatchSet.Id psId;
+
+ @VisibleForTesting
+ ChangeUpdate(GitRepositoryManager repoManager, AccountCache accountCache,
+ LabelTypes labelTypes, Change change, Account account, Date when,
+ TimeZone tz) {
+ this(repoManager, accountCache, null, labelTypes, change, account, when,
+ tz);
+ }
+
+ private ChangeUpdate(GitRepositoryManager repoManager,
+ AccountCache accountCache, @Nullable MetaDataUpdate.User updateFactory,
+ LabelTypes labelTypes, Change change, Account account, Date when,
+ TimeZone tz) {
+ this.repoManager = repoManager;
+ this.accountCache = accountCache;
+ this.updateFactory = updateFactory;
+ this.labelTypes = labelTypes;
+ this.change = change;
+ this.account = account;
+ this.when = when;
+ this.tz = tz;
+ this.approvals = Maps.newTreeMap(labelTypes.nameComparator());
+ this.reviewers = Maps.newLinkedHashMap();
+ }
+
+ public Account getAccount() {
+ return account;
+ }
+
+ public Date getWhen() {
+ return when;
+ }
+
+ public void putApproval(String label, short value) {
+ approvals.put(label, value);
+ }
+
+ public void setSubject(String subject) {
+ this.subject = subject;
+ }
+
+ public void setPatchSetId(PatchSet.Id psId) {
+ checkArgument(psId == null || psId.getParentKey().equals(change.getKey()));
+ this.psId = psId;
+ }
+
+ public void putReviewer(Account.Id reviewer, ReviewerState type) {
+ checkArgument(type != ReviewerState.REMOVED, "invalid ReviewerType");
+ reviewers.put(reviewer, type);
+ }
+
+ public void removeReviewer(Account.Id reviewer) {
+ reviewers.put(reviewer, ReviewerState.REMOVED);
+ }
+
+ public RevCommit commit() throws IOException {
+ return commit(checkNotNull(updateFactory, "MetaDataUpdate.Factory")
+ .create(change.getProject()));
+ }
+
+ @Override
+ public RevCommit commit(MetaDataUpdate md) throws IOException {
+ Repository repo = repoManager.openRepository(change.getProject());
+ try {
+ load(repo);
+ } catch (ConfigInvalidException e) {
+ throw new IOException(e);
+ } finally {
+ repo.close();
+ }
+
+ md.setAllowEmpty(true);
+ CommitBuilder cb = md.getCommitBuilder();
+ cb.setCommitter(newCommitter());
+ return super.commit(md);
+ }
+
+ public PersonIdent newIdent(Account author) {
+ return new PersonIdent(
+ author.getFullName(),
+ author.getId().get() + "@" + GERRIT_PLACEHOLDER_HOST,
+ when, tz);
+ }
+
+ public PersonIdent newCommitter() {
+ return newIdent(account);
+ }
+
+ @Override
+ protected String getRefName() {
+ return ChangeNoteUtil.changeRefName(change.getId());
+ }
+
+ @Override
+ protected void onSave(CommitBuilder commit) {
+ if (approvals.isEmpty() && reviewers.isEmpty()) {
+ return;
+ }
+ int ps = psId != null ? psId.get() : change.currentPatchSetId().get();
+ StringBuilder msg = new StringBuilder();
+ if (subject != null) {
+ msg.append(subject);
+ } else {
+ msg.append("Update patch set ").append(ps);
+ }
+ msg.append("\n\n");
+ addFooter(msg, FOOTER_PATCH_SET, ps);
+ for (Map.Entry<Account.Id, ReviewerState> e : reviewers.entrySet()) {
+ Account account = accountCache.get(e.getKey()).getAccount();
+ PersonIdent ident = newIdent(account);
+ addFooter(msg, e.getValue().getFooterKey())
+ .append(ident.getName())
+ .append(" <").append(ident.getEmailAddress()).append(">\n");
+ }
+ for (Map.Entry<String, Short> e : approvals.entrySet()) {
+ LabelType lt = labelTypes.byLabel(e.getKey());
+ if (lt != null) {
+ addFooter(msg, FOOTER_LABEL,
+ new LabelVote(lt.getName(), e.getValue()).formatWithEquals());
+ }
+ }
+ commit.setMessage(msg.toString());
+ }
+
+ private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) {
+ return sb.append(footer.getName()).append(": ");
+ }
+
+ private static void addFooter(StringBuilder sb, FooterKey footer,
+ Object value) {
+ addFooter(sb, footer).append(value).append('\n');
+ }
+
+ @Override
+ protected void onLoad() throws IOException, ConfigInvalidException {
+ // Do nothing; just reads current revision.
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerState.java
new file mode 100644
index 0000000..b829a69
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerState.java
@@ -0,0 +1,39 @@
+// 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 org.eclipse.jgit.revwalk.FooterKey;
+
+/** State of a reviewer on a change. */
+public enum ReviewerState {
+ /** The user has contributed at least one nonzero vote on the change. */
+ REVIEWER(new FooterKey("Reviewer")),
+
+ /** The reviewer was added to the change, but has not voted. */
+ CC(new FooterKey("CC")),
+
+ /** The user was previously a reviewer on the change, but was removed. */
+ REMOVED(new FooterKey("Removed"));
+
+ private final FooterKey footerKey;
+
+ private ReviewerState(FooterKey footerKey) {
+ this.footerKey = footerKey;
+ }
+
+ FooterKey getFooterKey() {
+ return footerKey;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index f73a2f7..4c2a30b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -171,6 +172,7 @@
interface AssistedFactory {
ChangeControl create(RefControl refControl, Change change);
+ ChangeControl create(RefControl refControl, ChangeNotes notes);
}
/**
@@ -189,23 +191,34 @@
private final ApprovalsUtil approvalsUtil;
private final ChangeData.Factory changeDataFactory;
private final RefControl refControl;
- private final Change change;
+ private final ChangeNotes notes;
+
+ @AssistedInject
+ ChangeControl(
+ ApprovalsUtil approvalsUtil,
+ ChangeData.Factory changeDataFactory,
+ ChangeNotes.Factory notesFactory,
+ @Assisted RefControl refControl,
+ @Assisted Change change) {
+ this(approvalsUtil, changeDataFactory, refControl,
+ notesFactory.create(change));
+ }
@AssistedInject
ChangeControl(
ApprovalsUtil approvalsUtil,
ChangeData.Factory changeDataFactory,
@Assisted RefControl refControl,
- @Assisted Change change) {
+ @Assisted ChangeNotes notes) {
this.approvalsUtil = approvalsUtil;
this.changeDataFactory = changeDataFactory;
this.refControl = refControl;
- this.change = change;
+ this.notes = notes;
}
public ChangeControl forUser(final CurrentUser who) {
return new ChangeControl(approvalsUtil, changeDataFactory,
- getRefControl().forUser(who), change);
+ getRefControl().forUser(who), notes);
}
public RefControl getRefControl() {
@@ -225,12 +238,17 @@
}
public Change getChange() {
- return change;
+ return notes.getChange();
+ }
+
+ public ChangeNotes getNotes() {
+ return notes;
}
/** Can this user see this change? */
public boolean isVisible(ReviewDb db) throws OrmException {
- if (change.getStatus() == Change.Status.DRAFT && !isDraftVisible(db, null)) {
+ if (getChange().getStatus() == Change.Status.DRAFT
+ && !isDraftVisible(db, null)) {
return false;
}
return isRefVisible();
@@ -326,7 +344,7 @@
public boolean isOwner() {
if (getCurrentUser().isIdentifiedUser()) {
final IdentifiedUser i = (IdentifiedUser) getCurrentUser();
- return i.getAccountId().equals(change.getOwner());
+ return i.getAccountId().equals(getChange().getOwner());
}
return false;
}
@@ -384,7 +402,7 @@
/** Can this user edit the topic name? */
public boolean canEditTopicName() {
- if (change.getStatus().isOpen()) {
+ if (getChange().getStatus().isOpen()) {
return isOwner() // owner (aka creator) of the change can edit topic
|| getRefControl().isOwner() // branch owner can edit topic
|| getProjectControl().isOwner() // project owner can edit topic
@@ -411,18 +429,18 @@
public List<SubmitRecord> canSubmit(ReviewDb db, PatchSet patchSet,
@Nullable ChangeData cd, boolean fastEvalLabels, boolean allowClosed,
boolean allowDraft) {
- if (!allowClosed && change.getStatus().isClosed()) {
+ if (!allowClosed && getChange().getStatus().isClosed()) {
SubmitRecord rec = new SubmitRecord();
rec.status = SubmitRecord.Status.CLOSED;
return Collections.singletonList(rec);
}
- if (!patchSet.getId().equals(change.currentPatchSetId())) {
+ if (!patchSet.getId().equals(getChange().currentPatchSetId())) {
return ruleError("Patch set " + patchSet.getPatchSetId() + " is not current");
}
cd = changeData(db, cd);
- if ((change.getStatus() == Change.Status.DRAFT || patchSet.isDraft())
+ if ((getChange().getStatus() == Change.Status.DRAFT || patchSet.isDraft())
&& !allowDraft) {
return cannotSubmitDraft(db, patchSet, cd);
}
@@ -432,7 +450,7 @@
try {
evaluator = new SubmitRuleEvaluator(db, patchSet,
getProjectControl(),
- this, change, cd,
+ this, getChange(), cd,
fastEvalLabels,
"locate_submit_rule", "can_submit",
"locate_submit_filter", "filter_submit_results");
@@ -447,7 +465,7 @@
// required for this change to be submittable. Each label will indicate
// whether or not that is actually possible given the permissions.
log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change "
- + change.getId() + " of " + getProject().getName()
+ + getChange().getId() + " of " + getProject().getName()
+ " has no solution.");
return ruleError("Project submit rule has no solution");
}
@@ -569,7 +587,8 @@
@Nullable ChangeData cd) {
cd = changeData(db, cd);
try {
- if (change.getStatus() == Change.Status.DRAFT && !isDraftVisible(db, cd)) {
+ if (getChange().getStatus() == Change.Status.DRAFT
+ && !isDraftVisible(db, cd)) {
return typeRuleError("Patch set " + patchSet.getPatchSetId()
+ " not found");
}
@@ -586,7 +605,7 @@
SubmitRuleEvaluator evaluator;
try {
evaluator = new SubmitRuleEvaluator(db, patchSet,
- getProjectControl(), this, change, cd,
+ getProjectControl(), this, getChange(), cd,
false,
"locate_submit_type", "get_submit_type",
"locate_submit_type_filter", "filter_submit_type_results");
@@ -598,7 +617,7 @@
if (results.isEmpty()) {
// Should never occur for a well written rule
log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change "
- + change.getId() + " of " + getProject().getName()
+ + getChange().getId() + " of " + getProject().getName()
+ " has no solution.");
return typeRuleError("Project submit rule has no solution");
}
@@ -606,7 +625,7 @@
Term typeTerm = results.get(0);
if (!typeTerm.isSymbol()) {
log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change "
- + change.getId() + " of " + getProject().getName()
+ + getChange().getId() + " of " + getProject().getName()
+ " did not return a symbol.");
return typeRuleError("Project submit rule has invalid solution");
}
@@ -621,7 +640,7 @@
}
private List<SubmitRecord> logInvalidResult(Term rule, Term record, String reason) {
- return logRuleError("Submit rule " + rule + " for change " + change.getId()
+ return logRuleError("Submit rule " + rule + " for change " + getChange().getId()
+ " of " + getProject().getName() + " output invalid result: " + record
+ (reason == null ? "" : ". Reason: " + reason));
}
@@ -649,7 +668,7 @@
private SubmitTypeRecord logInvalidType(Term rule, String record) {
return logTypeRuleError("Submit type rule " + rule + " for change "
- + change.getId() + " of " + getProject().getName()
+ + getChange().getId() + " of " + getProject().getName()
+ " output invalid result: " + record);
}
@@ -671,7 +690,7 @@
}
private ChangeData changeData(ReviewDb db, @Nullable ChangeData cd) {
- return cd != null ? cd : changeDataFactory.create(db, change);
+ return cd != null ? cd : changeDataFactory.create(db, getChange());
}
private void appliedBy(SubmitRecord.Label label, Term status)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 8f8cb70..f228e79f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -34,9 +34,9 @@
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.ApprovalsUtil.ReviewerState;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListEntry;
@@ -133,6 +133,18 @@
ChangeData create(ReviewDb db, ChangeControl c);
}
+ /**
+ * Create an instance for testing only.
+ * <p>
+ * Attempting to lazy load data will fail with NPEs.
+ *
+ * @param id change ID
+ * @return instance for testing.
+ */
+ static ChangeData createForTest(Change.Id id) {
+ return new ChangeData(null, null, null, id);
+ }
+
private final ReviewDb db;
private final GitRepositoryManager repoManager;
private final PatchListCache patchListCache;
@@ -157,7 +169,7 @@
private boolean patchesLoaded;
@AssistedInject
- public ChangeData(
+ private ChangeData(
GitRepositoryManager repoManager,
PatchListCache patchListCache,
@Assisted ReviewDb db,
@@ -169,7 +181,7 @@
}
@AssistedInject
- ChangeData(
+ private ChangeData(
GitRepositoryManager repoManager,
PatchListCache patchListCache,
@Assisted ReviewDb db,
@@ -182,7 +194,7 @@
}
@AssistedInject
- ChangeData(
+ private ChangeData(
GitRepositoryManager repoManager,
PatchListCache patchListCache,
@Assisted ReviewDb db,
@@ -295,14 +307,6 @@
return legacyId;
}
- public Change getChange() {
- return change;
- }
-
- public boolean hasChange() {
- return change != null;
- }
-
boolean fastIsVisibleTo(CurrentUser user) {
return visibleTo == user;
}
@@ -323,10 +327,6 @@
return change;
}
- void setChange(Change c) {
- change = c;
- }
-
public PatchSet currentPatchSet() throws OrmException {
if (currentPatchSet == null) {
Change c = change();
@@ -385,7 +385,7 @@
IncorrectObjectTypeException {
PatchSet.Id psId = change().currentPatchSetId();
String sha1 = db.patchSets().get(psId).getRevision().get();
- Repository repo = repoManager.openRepository(change.getProject());
+ Repository repo = repoManager.openRepository(change().getProject());
try {
RevWalk walk = new RevWalk(repo);
try {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
index 65f71ae..00d8ce3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
@@ -308,8 +308,8 @@
}
LabelTypes labelTypes = cc.getLabelTypes();
- c = eventFactory.asChangeAttribute(d.getChange());
- eventFactory.extend(c, d.getChange());
+ c = eventFactory.asChangeAttribute(d.change());
+ eventFactory.extend(c, d.change());
if (!trackingFooters.isEmpty()) {
eventFactory.addTrackingIds(c,
@@ -317,11 +317,11 @@
}
if (includeAllReviewers) {
- eventFactory.addAllReviewers(c, d.getChange());
+ eventFactory.addAllReviewers(c, d.change());
}
if (includeSubmitRecords) {
- PatchSet.Id psId = d.getChange().currentPatchSetId();
+ PatchSet.Id psId = d.change().currentPatchSetId();
PatchSet patchSet = db.get().patchSets().get(psId);
List<SubmitRecord> submitResult = cc.canSubmit( //
db.get(), patchSet, null, false, true, true);
@@ -368,7 +368,7 @@
}
if (includeDependencies) {
- eventFactory.addDependencies(c, d.getChange());
+ eventFactory.addDependencies(c, d.change());
}
show(c);
diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java
index 67c2c93..b835b34 100644
--- a/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED_change_branch_1.java
@@ -15,7 +15,6 @@
package gerrit;
import com.google.gerrit.reviewdb.client.Branch;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.rules.StoredValues;
import com.googlecode.prolog_cafe.lang.Operation;
@@ -36,8 +35,7 @@
engine.setB0();
Term a1 = arg1.dereference();
- Change change = StoredValues.CHANGE_DATA.get(engine).getChange();
- Branch.NameKey name = change.getDest();
+ Branch.NameKey name = StoredValues.getChange(engine).getDest();
if (!a1.unify(SymbolTerm.create(name.get()), engine.trail)) {
return engine.fail();
diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java
index efe1c82..51502f8 100644
--- a/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED_change_owner_1.java
@@ -15,7 +15,6 @@
package gerrit;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.rules.StoredValues;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
@@ -40,8 +39,7 @@
engine.setB0();
Term a1 = arg1.dereference();
- Change change = StoredValues.CHANGE_DATA.get(engine).getChange();
- Account.Id ownerId = change.getOwner();
+ Account.Id ownerId = StoredValues.getChange(engine).getOwner();
if (!a1.unify(new StructureTerm(user, new IntegerTerm(ownerId.get())), engine.trail)) {
return engine.fail();
diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java
index 9a1d8b6..29c6704 100644
--- a/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED_change_project_1.java
@@ -14,7 +14,6 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.rules.StoredValues;
@@ -36,8 +35,7 @@
engine.setB0();
Term a1 = arg1.dereference();
- Change change = StoredValues.CHANGE_DATA.get(engine).getChange();
- Project.NameKey name = change.getProject();
+ Project.NameKey name = StoredValues.getChange(engine).getProject();
if (!a1.unify(SymbolTerm.create(name.get()), engine.trail)) {
return engine.fail();
diff --git a/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java b/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java
index e6748de..7ba648f 100644
--- a/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED_change_topic_1.java
@@ -36,7 +36,7 @@
Term a1 = arg1.dereference();
Term topicTerm = Prolog.Nil;
- Change change = StoredValues.CHANGE_DATA.get(engine).getChange();
+ Change change = StoredValues.getChange(engine);
String topic = change.getTopic();
if (topic != null) {
topicTerm = SymbolTerm.create(topic);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
new file mode 100644
index 0000000..01e01bf
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -0,0 +1,410 @@
+// 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.gerrit.server.project.Util.category;
+import static com.google.gerrit.server.project.Util.value;
+import static java.util.concurrent.TimeUnit.DAYS;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.Assert.assertEquals;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.ListMultimap;
+import com.google.gerrit.common.data.LabelTypes;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Branch;
+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.reviewdb.client.PatchSetInfo;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.util.TimeUtil;
+import com.google.gerrit.testutil.FakeAccountCache;
+import com.google.gerrit.testutil.InMemoryRepositoryManager;
+import com.google.gwtorm.server.OrmException;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.joda.time.DateTimeUtils;
+import org.joda.time.DateTimeUtils.MillisProvider;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.sql.Timestamp;
+import java.util.Date;
+import java.util.List;
+import java.util.TimeZone;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class ChangeNotesTest {
+ private static final TimeZone TZ =
+ TimeZone.getTimeZone("America/Los_Angeles");
+
+ private static final Account CHANGE_OWNER;
+ private static final Account OTHER_ACCOUNT;
+ static {
+ CHANGE_OWNER = new Account(new Account.Id(1), TimeUtil.nowTs());
+ CHANGE_OWNER.setFullName("Change Owner");
+ CHANGE_OWNER.setPreferredEmail("change@owner.com");
+ OTHER_ACCOUNT = new Account(new Account.Id(2), TimeUtil.nowTs());
+ OTHER_ACCOUNT.setFullName("Other Account");
+ OTHER_ACCOUNT.setPreferredEmail("other@account.com");
+ }
+
+ private static final LabelTypes LABEL_TYPES = new LabelTypes(ImmutableList.of(
+ category("Verified",
+ value(1, "Verified"),
+ value(0, "No score"),
+ value(-1, "Fails")),
+ category("Code-Review",
+ value(1, "Looks Good To Me"),
+ value(0, "No score"),
+ value(-1, "Do Not Submit"))));
+
+ private Project.NameKey project;
+ private InMemoryRepositoryManager repoManager;
+ private InMemoryRepository repo;
+ private FakeAccountCache accountCache;
+ private volatile long clockStepMs;
+
+ @Before
+ public void setUp() throws Exception {
+ project = new Project.NameKey("test-project");
+ repoManager = new InMemoryRepositoryManager();
+ repo = repoManager.createRepository(project);
+ accountCache = new FakeAccountCache();
+ accountCache.put(CHANGE_OWNER);
+ accountCache.put(OTHER_ACCOUNT);
+ }
+
+ @Before
+ public void setMillisProvider() {
+ clockStepMs = MILLISECONDS.convert(1, SECONDS);
+ final AtomicLong clockMs = new AtomicLong(
+ MILLISECONDS.convert(ChangeUtil.SORT_KEY_EPOCH_MINS, MINUTES)
+ + MILLISECONDS.convert(60, DAYS));
+
+ DateTimeUtils.setCurrentMillisProvider(new MillisProvider() {
+ @Override
+ public long getMillis() {
+ return clockMs.getAndAdd(clockStepMs);
+ }
+ });
+ }
+
+ @After
+ public void resetMillisProvider() {
+ DateTimeUtils.setCurrentMillisSystem();
+ }
+
+ @Test
+ public void approvalsCommitFormat() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) -1);
+ update.putApproval("Verified", (short) 1);
+ update.putReviewer(CHANGE_OWNER.getId(), ReviewerState.REVIEWER);
+ update.putReviewer(OTHER_ACCOUNT.getId(), ReviewerState.CC);
+ commit(update);
+ assertEquals("refs/changes/01/1/meta", update.getRefName());
+
+ RevWalk walk = new RevWalk(repo);
+ try {
+ RevCommit commit = walk.parseCommit(update.getRevision());
+ walk.parseBody(commit);
+ assertEquals("Update patch set 1\n"
+ + "\n"
+ + "Patch-Set: 1\n"
+ + "Reviewer: Change Owner <1@gerrit>\n"
+ + "CC: Other Account <2@gerrit>\n"
+ + "Label: Verified=+1\n"
+ + "Label: Code-Review=-1\n",
+ commit.getFullMessage());
+
+ PersonIdent author = commit.getAuthorIdent();
+ assertEquals("Change Owner", author.getName());
+ assertEquals("change@owner.com", author.getEmailAddress());
+ assertEquals(new Date(c.getCreatedOn().getTime() + 1000),
+ author.getWhen());
+ assertEquals(TimeZone.getTimeZone("GMT-8:00"), author.getTimeZone());
+
+ PersonIdent committer = commit.getCommitterIdent();
+ assertEquals("Change Owner", committer.getName());
+ assertEquals("1@gerrit", committer.getEmailAddress());
+ assertEquals(author.getWhen(), committer.getWhen());
+ assertEquals(author.getTimeZone(), committer.getTimeZone());
+ } finally {
+ walk.release();
+ }
+ }
+
+ @Test
+ public void approvalsOnePatchSet() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) -1);
+ update.putApproval("Verified", (short) 1);
+ commit(update);
+
+ ChangeNotes notes = newNotes(c);
+ assertEquals(1, notes.getApprovals().keySet().size());
+ List<PatchSetApproval> psas =
+ notes.getApprovals().get(c.currentPatchSetId());
+ assertEquals(2, psas.size());
+
+ assertEquals(c.currentPatchSetId(), psas.get(0).getPatchSetId());
+ assertEquals(1, psas.get(0).getAccountId().get());
+ assertEquals("Verified", psas.get(0).getLabel());
+ assertEquals((short) 1, psas.get(0).getValue());
+ assertEquals(truncate(after(c, 1000)), psas.get(0).getGranted());
+
+ assertEquals(c.currentPatchSetId(), psas.get(1).getPatchSetId());
+ assertEquals(1, psas.get(1).getAccountId().get());
+ assertEquals("Code-Review", psas.get(1).getLabel());
+ assertEquals((short) -1, psas.get(1).getValue());
+ assertEquals(psas.get(0).getGranted(), psas.get(1).getGranted());
+ }
+
+ @Test
+ public void approvalsMultiplePatchSets() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) -1);
+ commit(update);
+ PatchSet.Id ps1 = c.currentPatchSetId();
+
+ incrementPatchSet(c);
+ update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) 1);
+ commit(update);
+ PatchSet.Id ps2 = c.currentPatchSetId();
+
+ ChangeNotes notes = newNotes(c);
+ ListMultimap<PatchSet.Id, PatchSetApproval> psas = notes.getApprovals();
+ assertEquals(2, notes.getApprovals().keySet().size());
+
+ PatchSetApproval psa1 = Iterables.getOnlyElement(psas.get(ps1));
+ assertEquals(ps1, psa1.getPatchSetId());
+ assertEquals(1, psa1.getAccountId().get());
+ assertEquals("Code-Review", psa1.getLabel());
+ assertEquals((short) -1, psa1.getValue());
+ assertEquals(truncate(after(c, 1000)), psa1.getGranted());
+
+ PatchSetApproval psa2 = Iterables.getOnlyElement(psas.get(ps2));
+ assertEquals(ps2, psa2.getPatchSetId());
+ assertEquals(1, psa2.getAccountId().get());
+ assertEquals("Code-Review", psa2.getLabel());
+ assertEquals((short) +1, psa2.getValue());
+ assertEquals(truncate(after(c, 2000)), psa2.getGranted());
+ }
+
+ @Test
+ public void approvalsMultipleApprovals() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) -1);
+ commit(update);
+
+ ChangeNotes notes = newNotes(c);
+ PatchSetApproval psa = Iterables.getOnlyElement(
+ notes.getApprovals().get(c.currentPatchSetId()));
+ assertEquals("Code-Review", psa.getLabel());
+ assertEquals((short) -1, psa.getValue());
+
+ update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) 1);
+ commit(update);
+
+ notes = newNotes(c);
+ psa = Iterables.getOnlyElement(
+ notes.getApprovals().get(c.currentPatchSetId()));
+ assertEquals("Code-Review", psa.getLabel());
+ assertEquals((short) 1, psa.getValue());
+ }
+
+ @Test
+ public void approvalsMultipleUsers() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) -1);
+ commit(update);
+
+ update = newUpdate(c, OTHER_ACCOUNT);
+ update.putApproval("Code-Review", (short) 1);
+ commit(update);
+
+ ChangeNotes notes = newNotes(c);
+ assertEquals(1, notes.getApprovals().keySet().size());
+ List<PatchSetApproval> psas =
+ notes.getApprovals().get(c.currentPatchSetId());
+ assertEquals(2, psas.size());
+
+ assertEquals(c.currentPatchSetId(), psas.get(0).getPatchSetId());
+ assertEquals(1, psas.get(0).getAccountId().get());
+ assertEquals("Code-Review", psas.get(0).getLabel());
+ assertEquals((short) -1, psas.get(0).getValue());
+ assertEquals(truncate(after(c, 1000)), psas.get(0).getGranted());
+
+ assertEquals(c.currentPatchSetId(), psas.get(1).getPatchSetId());
+ assertEquals(2, psas.get(1).getAccountId().get());
+ assertEquals("Code-Review", psas.get(1).getLabel());
+ assertEquals((short) 1, psas.get(1).getValue());
+ assertEquals(truncate(after(c, 2000)), psas.get(1).getGranted());
+ }
+
+ @Test
+ public void multipleReviewers() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putReviewer(CHANGE_OWNER.getId(), ReviewerState.REVIEWER);
+ update.putReviewer(OTHER_ACCOUNT.getId(), ReviewerState.REVIEWER);
+ commit(update);
+
+ ChangeNotes notes = newNotes(c);
+ assertEquals(ImmutableSetMultimap.of(
+ ReviewerState.REVIEWER, new Account.Id(1),
+ ReviewerState.REVIEWER, new Account.Id(2)),
+ notes.getReviewers());
+ }
+
+ @Test
+ public void reviewerTypes() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putReviewer(CHANGE_OWNER.getId(), ReviewerState.REVIEWER);
+ update.putReviewer(OTHER_ACCOUNT.getId(), ReviewerState.CC);
+ commit(update);
+
+ ChangeNotes notes = newNotes(c);
+ assertEquals(ImmutableSetMultimap.of(
+ ReviewerState.REVIEWER, new Account.Id(1),
+ ReviewerState.CC, new Account.Id(2)),
+ notes.getReviewers());
+ }
+
+ @Test
+ public void oneReviewerMultipleTypes() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putReviewer(OTHER_ACCOUNT.getId(), ReviewerState.REVIEWER);
+ commit(update);
+
+ ChangeNotes notes = newNotes(c);
+ assertEquals(ImmutableSetMultimap.of(
+ ReviewerState.REVIEWER, new Account.Id(2)),
+ notes.getReviewers());
+
+ update = newUpdate(c, OTHER_ACCOUNT);
+ update.putReviewer(OTHER_ACCOUNT.getId(), ReviewerState.CC);
+ commit(update);
+
+ notes = newNotes(c);
+ assertEquals(ImmutableSetMultimap.of(
+ ReviewerState.CC, new Account.Id(2)),
+ notes.getReviewers());
+ }
+
+ @Test
+ public void removeReviewer() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
+ update.putReviewer(OTHER_ACCOUNT.getId(), ReviewerState.REVIEWER);
+ commit(update);
+
+ update = newUpdate(c, CHANGE_OWNER);
+ update.putApproval("Code-Review", (short) 1);
+ commit(update);
+
+ update = newUpdate(c, OTHER_ACCOUNT);
+ update.putApproval("Code-Review", (short) 1);
+ commit(update);
+
+ ChangeNotes notes = newNotes(c);
+ List<PatchSetApproval> psas =
+ notes.getApprovals().get(c.currentPatchSetId());
+ assertEquals(2, psas.size());
+ assertEquals(CHANGE_OWNER.getId(), psas.get(0).getAccountId());
+ assertEquals(OTHER_ACCOUNT.getId(), psas.get(1).getAccountId());
+
+ update = newUpdate(c, CHANGE_OWNER);
+ update.removeReviewer(OTHER_ACCOUNT.getId());
+ commit(update);
+
+ notes = newNotes(c);
+ psas = notes.getApprovals().get(c.currentPatchSetId());
+ assertEquals(1, psas.size());
+ assertEquals(CHANGE_OWNER.getId(), psas.get(0).getAccountId());
+ }
+
+ private Change newChange() {
+ Change.Id changeId = new Change.Id(1);
+ Change c = new Change(
+ new Change.Key("Iabcd1234abcd1234abcd1234abcd1234abcd1234"),
+ changeId,
+ CHANGE_OWNER.getId(),
+ new Branch.NameKey(project, "master"),
+ TimeUtil.nowTs());
+ incrementPatchSet(c);
+ return c;
+ }
+
+ private ChangeUpdate newUpdate(Change c, Account account)
+ throws ConfigInvalidException, IOException {
+ return new ChangeUpdate(repoManager, accountCache, LABEL_TYPES, c, account,
+ TimeUtil.nowTs(), TZ);
+ }
+
+ private ChangeNotes newNotes(Change c) throws OrmException {
+ return new ChangeNotes(repoManager, c).load();
+ }
+
+ private static void incrementPatchSet(Change change) {
+ PatchSet.Id curr = change.currentPatchSetId();
+ PatchSetInfo ps = new PatchSetInfo(new PatchSet.Id(
+ change.getId(), curr != null ? curr.get() + 1 : 1));
+ ps.setSubject("Change subject");
+ change.setCurrentPatchSet(ps);
+ }
+
+ private static Timestamp truncate(Timestamp ts) {
+ return new Timestamp((ts.getTime() / 1000) * 1000);
+ }
+
+ private static Timestamp after(Change c, long millis) {
+ return new Timestamp(c.getCreatedOn().getTime() + millis);
+ }
+
+ private RevCommit commit(ChangeUpdate update) throws IOException {
+ MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED,
+ project, repo);
+ md.getCommitBuilder().setAuthor(new PersonIdent(
+ update.getAccount().getFullName(),
+ update.getAccount().getPreferredEmail(),
+ update.getWhen(), TZ));
+ return update.commit(md);
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
index 6e59b7c..f09f2d8 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
@@ -244,7 +244,7 @@
return new ProjectControl(Collections.<AccountGroup.UUID> emptySet(),
Collections.<AccountGroup.UUID> emptySet(), projectCache,
- sectionSorter, null, changeControlFactory, canonicalWebUrl,
+ sectionSorter, repoManager, changeControlFactory, canonicalWebUrl,
new MockUser(name, memberOf), newProjectState(local));
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java
index 4e5dcdd..dd7ac56 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java
@@ -14,15 +14,16 @@
package com.google.gerrit.server.query.change;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
import com.google.gerrit.reviewdb.client.Change;
import com.google.gwtorm.server.OrmException;
+
import org.junit.Test;
import java.util.Arrays;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
public class RegexPathPredicateTest {
@Test
public void testPrefixOnlyOptimization() throws OrmException {
@@ -83,7 +84,7 @@
private static ChangeData change(String... files) {
Arrays.sort(files);
- ChangeData cd = new ChangeData(null, null, null, new Change.Id(1));
+ ChangeData cd = ChangeData.createForTest(new Change.Id(1));
cd.setCurrentFilePaths(Arrays.asList(files));
return cd;
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java
new file mode 100644
index 0000000..db1ed05
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java
@@ -0,0 +1,80 @@
+// 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.testutil;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountExternalId;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.util.TimeUtil;
+
+import java.util.Map;
+
+/** Fake implementation of {@link AccountCache} for testing. */
+public class FakeAccountCache implements AccountCache {
+ private final Map<Account.Id, AccountState> byId;
+ private final Map<String, AccountState> byUsername;
+
+ public FakeAccountCache() {
+ byId = Maps.newHashMap();
+ byUsername = Maps.newHashMap();
+ }
+
+ @Override
+ public synchronized AccountState get(Account.Id accountId) {
+ AccountState state = getIfPresent(accountId);
+ if (state != null) {
+ return state;
+ }
+ return newState(new Account(accountId, TimeUtil.nowTs()));
+ }
+
+ @Override
+ public synchronized AccountState getIfPresent(Account.Id accountId) {
+ return byId.get(accountId);
+ }
+
+ @Override
+ public synchronized AccountState getByUsername(String username) {
+ return byUsername.get(username);
+ }
+
+ @Override
+ public synchronized void evict(Account.Id accountId) {
+ byId.remove(accountId);
+ }
+
+ @Override
+ public synchronized void evictByUsername(String username) {
+ byUsername.remove(username);
+ }
+
+ public synchronized void put(Account account) {
+ AccountState state = newState(account);
+ byId.put(account.getId(), state);
+ if (account.getUserName() != null) {
+ byUsername.put(account.getUserName(), state);
+ }
+ }
+
+ private static AccountState newState(Account account) {
+ return new AccountState(
+ account, ImmutableSet.<AccountGroup.UUID> of(),
+ ImmutableSet.<AccountExternalId> of());
+ }
+}
diff --git a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
index 7101a4d..cf9c733 100644
--- a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
+++ b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java
@@ -148,14 +148,14 @@
String id = cd.getId().toString();
SolrInputDocument doc = toDocument(cd);
try {
- if (cd.getChange().getStatus().isOpen()) {
+ if (cd.change().getStatus().isOpen()) {
closedIndex.deleteById(id);
openIndex.add(doc);
} else {
openIndex.deleteById(id);
closedIndex.add(doc);
}
- } catch (SolrServerException e) {
+ } catch (OrmException | SolrServerException e) {
throw new IOException(e);
}
commit(openIndex);
@@ -167,14 +167,14 @@
String id = cd.getId().toString();
SolrInputDocument doc = toDocument(cd);
try {
- if (cd.getChange().getStatus().isOpen()) {
+ if (cd.change().getStatus().isOpen()) {
closedIndex.deleteById(id);
openIndex.add(doc);
} else {
openIndex.deleteById(id);
closedIndex.add(doc);
}
- } catch (SolrServerException e) {
+ } catch (OrmException | SolrServerException e) {
throw new IOException(e);
}
commit(openIndex);
@@ -185,14 +185,14 @@
public void delete(ChangeData cd) throws IOException {
String id = cd.getId().toString();
try {
- if (cd.getChange().getStatus().isOpen()) {
+ if (cd.change().getStatus().isOpen()) {
openIndex.deleteById(id);
commit(openIndex);
} else {
closedIndex.deleteById(id);
commit(closedIndex);
}
- } catch (SolrServerException e) {
+ } catch (OrmException | SolrServerException e) {
throw new IOException(e);
}
}
