Record reviewer state in notes database
Stop implicitly adding/removing reviewers by adding an arbitrary
PatchSetApprovals with a 0 vote. This is ugly and error-prone,
especially when LabelTypes change.
Instead, use the existing ReviewerState enum, adding a REMOVED type
as a tombstone. When parsing, record these during the walk, and prune
all REMOVED tombstones after the fact.
Change-Id: Iffb0517eb0162eb6cce66bf36d905a6eb60e75da
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/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/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/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/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 3438424..4d75d82 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -21,10 +21,12 @@
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.Multimaps;
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;
@@ -41,16 +43,20 @@
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 {
@@ -90,8 +96,154 @@
}
}
+ 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 Change change;
- private ListMultimap<PatchSet.Id, PatchSetApproval> approvals;
+ private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
+ private ImmutableSetMultimap<ReviewerState, Account.Id> reviewers;
@VisibleForTesting
ChangeNotes(Repository repo, Change change)
@@ -100,8 +252,12 @@
load(repo);
}
- public ListMultimap<PatchSet.Id, PatchSetApproval> getApprovals() {
- return Multimaps.unmodifiableListMultimap(approvals);
+ public ImmutableListMultimap<PatchSet.Id, PatchSetApproval> getApprovals() {
+ return approvals;
+ }
+
+ public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers() {
+ return reviewers;
}
@Override
@@ -115,89 +271,21 @@
if (rev == null) {
return;
}
- approvals = ArrayListMultimap.create();
RevWalk walk = new RevWalk(reader);
- walk.markStart(walk.parseCommit(rev));
- for (RevCommit commit : walk) {
- parse(commit);
- }
- 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);
- }
- }
- }
-
- 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(change.getId(), 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 {
- String email = commit.getCommitterIdent().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);
+ 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();
}
- throw parseException("invalid committer, expected <id>@%s: %s",
- GERRIT_PLACEHOLDER_HOST, email);
- }
-
- private ConfigInvalidException parseException(String fmt, Object... args) {
- return new ConfigInvalidException("Change " + change.getId() + ": "
- + String.format(fmt, args));
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 4c85435..ae7beac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -28,13 +28,14 @@
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.gerrit.server.util.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -47,8 +48,9 @@
import org.eclipse.jgit.revwalk.RevCommit;
import java.io.IOException;
-import java.sql.Timestamp;
+import java.util.Date;
import java.util.Map;
+import java.util.TimeZone;
/**
* A single delta to apply atomically to a change.
@@ -57,65 +59,90 @@
* 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) {
+ 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, TimeUtil.nowTs());
+ return create(change, serverIdent.getWhen());
}
- public ChangeUpdate create(Change change, Timestamp when) {
+ public ChangeUpdate create(Change change, Date when) {
return new ChangeUpdate(
- repoManager, updateFactory,
+ repoManager, accountCache, updateFactory,
projectCache.get(change.getProject()).getLabelTypes(),
- change, user.get().getAccountId(), when);
+ 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.Id accountId;
- private final Timestamp when;
+ 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, LabelTypes labelTypes,
- Change change, Account.Id accountId, @Nullable Timestamp when) {
- this(repoManager, null, labelTypes, change, accountId, when);
+ 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,
- @Nullable MetaDataUpdate.User updateFactory, LabelTypes labelTypes,
- Change change, Account.Id accountId, @Nullable Timestamp when) {
+ 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.accountId = accountId;
+ 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) {
@@ -131,6 +158,15 @@
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()));
@@ -149,18 +185,21 @@
md.setAllowEmpty(true);
CommitBuilder cb = md.getCommitBuilder();
- cb.setCommitter(new PersonIdent(
- cb.getAuthor().getName(),
- accountId.get() + "@" + GERRIT_PLACEHOLDER_HOST,
- when != null ? when : cb.getCommitter().getWhen(),
- cb.getCommitter().getTimeZone()));
- if (when != null) {
- md.getCommitBuilder().setAuthor(new PersonIdent(
- md.getCommitBuilder().getAuthor(), when));
- }
+ 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());
@@ -168,7 +207,7 @@
@Override
protected void onSave(CommitBuilder commit) {
- if (approvals.isEmpty()) {
+ if (approvals.isEmpty() && reviewers.isEmpty()) {
return;
}
int ps = psId != null ? psId.get() : change.currentPatchSetId().get();
@@ -180,6 +219,13 @@
}
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) {
@@ -190,9 +236,13 @@
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) {
- sb.append(footer.getName()).append(": ").append(value).append('\n');
+ addFooter(sb, footer).append(value).append('\n');
}
@Override
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/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 376d8c6..b21632a 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;
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index b614921..6d72456 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -16,15 +16,14 @@
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;
@@ -39,6 +38,7 @@
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 org.eclipse.jgit.errors.ConfigInvalidException;
@@ -63,7 +63,18 @@
private static final TimeZone TZ =
TimeZone.getTimeZone("America/Los_Angeles");
- private LabelTypes LABEL_TYPES = new LabelTypes(ImmutableList.of(
+ 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"),
@@ -76,6 +87,7 @@
private Project.NameKey project;
private InMemoryRepositoryManager repoManager;
private InMemoryRepository repo;
+ private FakeAccountCache accountCache;
private volatile long clockStepMs;
@Before
@@ -83,6 +95,9 @@
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
@@ -107,10 +122,12 @@
@Test
public void approvalsCommitFormat() throws Exception {
- Change c = newChange(5);
- ChangeUpdate update = newUpdate(c, c.getOwner());
+ 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());
@@ -121,20 +138,22 @@
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("Example User", author.getName());
- assertEquals("user@example.com", author.getEmailAddress());
+ 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("Example User", committer.getName());
- assertEquals("5@gerrit", committer.getEmailAddress());
+ assertEquals("Change Owner", committer.getName());
+ assertEquals("1@gerrit", committer.getEmailAddress());
assertEquals(author.getWhen(), committer.getWhen());
assertEquals(author.getTimeZone(), committer.getTimeZone());
} finally {
@@ -144,8 +163,8 @@
@Test
public void approvalsOnePatchSet() throws Exception {
- Change c = newChange(5);
- ChangeUpdate update = newUpdate(c, c.getOwner());
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
update.putApproval("Code-Review", (short) -1);
update.putApproval("Verified", (short) 1);
commit(update);
@@ -157,13 +176,13 @@
assertEquals(2, psas.size());
assertEquals(c.currentPatchSetId(), psas.get(0).getPatchSetId());
- assertEquals(5, psas.get(0).getAccountId().get());
+ 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(5, psas.get(1).getAccountId().get());
+ 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());
@@ -171,14 +190,14 @@
@Test
public void approvalsMultiplePatchSets() throws Exception {
- Change c = newChange(5);
- ChangeUpdate update = newUpdate(c, c.getOwner());
+ 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, c.getOwner());
+ update = newUpdate(c, CHANGE_OWNER);
update.putApproval("Code-Review", (short) 1);
commit(update);
PatchSet.Id ps2 = c.currentPatchSetId();
@@ -189,14 +208,14 @@
PatchSetApproval psa1 = Iterables.getOnlyElement(psas.get(ps1));
assertEquals(ps1, psa1.getPatchSetId());
- assertEquals(5, psa1.getAccountId().get());
+ 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(5, psa2.getAccountId().get());
+ assertEquals(1, psa2.getAccountId().get());
assertEquals("Code-Review", psa2.getLabel());
assertEquals((short) +1, psa2.getValue());
assertEquals(truncate(after(c, 2000)), psa2.getGranted());
@@ -204,8 +223,8 @@
@Test
public void approvalsMultipleApprovals() throws Exception {
- Change c = newChange(5);
- ChangeUpdate update = newUpdate(c, c.getOwner());
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
update.putApproval("Code-Review", (short) -1);
commit(update);
@@ -215,7 +234,7 @@
assertEquals("Code-Review", psa.getLabel());
assertEquals((short) -1, psa.getValue());
- update = newUpdate(c, c.getOwner());
+ update = newUpdate(c, CHANGE_OWNER);
update.putApproval("Code-Review", (short) 1);
commit(update);
@@ -228,12 +247,12 @@
@Test
public void approvalsMultipleUsers() throws Exception {
- Change c = newChange(5);
- ChangeUpdate update = newUpdate(c, c.getOwner());
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, CHANGE_OWNER);
update.putApproval("Code-Review", (short) -1);
commit(update);
- update = newUpdate(c, new Account.Id(6));
+ update = newUpdate(c, OTHER_ACCOUNT);
update.putApproval("Code-Review", (short) 1);
commit(update);
@@ -244,33 +263,118 @@
assertEquals(2, psas.size());
assertEquals(c.currentPatchSetId(), psas.get(0).getPatchSetId());
- assertEquals(5, psas.get(0).getAccountId().get());
+ 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(6, psas.get(1).getAccountId().get());
+ 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());
}
- private Change newChange(int accountId) {
+ @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,
- new Account.Id(accountId),
+ CHANGE_OWNER.getId(),
new Branch.NameKey(project, "master"),
TimeUtil.nowTs());
incrementPatchSet(c);
return c;
}
- private ChangeUpdate newUpdate(Change c, Account.Id owner)
+ private ChangeUpdate newUpdate(Change c, Account account)
throws ConfigInvalidException, IOException {
- return new ChangeUpdate(repoManager, LABEL_TYPES, c, owner, null);
+ return new ChangeUpdate(repoManager, accountCache, LABEL_TYPES, c, account,
+ TimeUtil.nowTs(), TZ);
}
private ChangeNotes newNotes(Change c)
@@ -297,11 +401,10 @@
private RevCommit commit(ChangeUpdate update) throws IOException {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED,
project, repo);
- Timestamp ts = TimeUtil.nowTs();
- md.getCommitBuilder().setAuthor(
- new PersonIdent("Example User", "user@example.com", ts, TZ));
- md.getCommitBuilder().setCommitter(
- new PersonIdent("Gerrit Test", "notthis@email.com", ts, TZ));
+ 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/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());
+ }
+}
