opendev/gerrit
Code Issues Proposed changes

Merge changes from topic "mark-reviewed"

Browse Source 
* changes:
  Replace Mute/Unmute by Mark as Reviewed/Unreviewed (Part 3)
  Replace Mute/Unmute by Mark as Reviewed/Unreviewed (Part 2)
  Replace Mute/Unmute by Mark as Reviewed/Unreviewed (Part 1)
This commit is contained in:
Dave Borowitz
2017-10-01 13:31:30 +00:00
committed by Gerrit Code Review
parent 297bb9c95b 614f983a71
commit 83abcbf3f8
18 changed files with 331 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toSet;
import com.google.auto.value.AutoValue;
import com.google.common.base.CharMatcher;
@@ -26,6 +27,7 @@ import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Sets;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
@@ -49,6 +51,7 @@ import java.io.IOException;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
@@ -154,7 +157,8 @@ public class StarredChangesUtil {
public static final String DEFAULT_LABEL = "star";
public static final String IGNORE_LABEL = "ignore";
public static final String MUTE_LABEL = "mute";
public static final String REVIEWED_LABEL = "reviewed";
public static final String UNREVIEWED_LABEL = "unreviewed";
public static final ImmutableSortedSet<String> DEFAULT_LABELS =
ImmutableSortedSet.of(DEFAULT_LABEL);
@@ -330,37 +334,41 @@ public class StarredChangesUtil {
return isIgnoredBy(rsrc.getChange().getId(), rsrc.getUser().asIdentifiedUser().getAccountId());
}
private static String getMuteLabel(Change change) {
return MUTE_LABEL + "/" + change.currentPatchSetId().get();
private static String getReviewedLabel(Change change) {
return getReviewedLabel(change.currentPatchSetId().get());
}
public void mute(ChangeResource rsrc) throws OrmException, IllegalLabelException {
private static String getReviewedLabel(int ps) {
return REVIEWED_LABEL + "/" + ps;
}
private static String getUnreviewedLabel(Change change) {
return getUnreviewedLabel(change.currentPatchSetId().get());
}
private static String getUnreviewedLabel(int ps) {
return UNREVIEWED_LABEL + "/" + ps;
}
public void markAsReviewed(ChangeResource rsrc) throws OrmException, IllegalLabelException {
star(
rsrc.getUser().asIdentifiedUser().getAccountId(),
rsrc.getProject(),
rsrc.getChange().getId(),
ImmutableSet.of(getMuteLabel(rsrc.getChange())),
ImmutableSet.of());
ImmutableSet.of(getReviewedLabel(rsrc.getChange())),
ImmutableSet.of(getUnreviewedLabel(rsrc.getChange())));
}
public void unmute(ChangeResource rsrc) throws OrmException, IllegalLabelException {
public void markAsUnreviewed(ChangeResource rsrc) throws OrmException, IllegalLabelException {
star(
rsrc.getUser().asIdentifiedUser().getAccountId(),
rsrc.getProject(),
rsrc.getChange().getId(),
ImmutableSet.of(),
ImmutableSet.of(getMuteLabel(rsrc.getChange())));
ImmutableSet.of(getUnreviewedLabel(rsrc.getChange())),
ImmutableSet.of(getReviewedLabel(rsrc.getChange())));
}
public boolean isMutedBy(Change change, Account.Id accountId) throws OrmException {
return getLabels(accountId, change.getId()).contains(getMuteLabel(change));
}
public boolean isMuted(ChangeResource rsrc) throws OrmException {
return isMutedBy(rsrc.getChange(), rsrc.getUser().asIdentifiedUser().getAccountId());
}
private static StarRef readLabels(Repository repo, String refName) throws IOException {
public static StarRef readLabels(Repository repo, String refName) throws IOException {
Ref ref = repo.exactRef(refName);
if (ref == null) {
return StarRef.MISSING;
@@ -394,6 +402,25 @@ public class StarredChangesUtil {
if (labels.containsAll(ImmutableSet.of(DEFAULT_LABEL, IGNORE_LABEL))) {
throw new MutuallyExclusiveLabelsException(DEFAULT_LABEL, IGNORE_LABEL);
}
Set<Integer> reviewedPatchSets =
labels
.stream()
.filter(l -> l.startsWith(REVIEWED_LABEL))
.map(l -> Integer.valueOf(l.substring(REVIEWED_LABEL.length() + 1)))
.collect(toSet());
Set<Integer> unreviewedPatchSets =
labels
.stream()
.filter(l -> l.startsWith(UNREVIEWED_LABEL))
.map(l -> Integer.valueOf(l.substring(UNREVIEWED_LABEL.length() + 1)))
.collect(toSet());
Optional<Integer> ps =
Sets.intersection(reviewedPatchSets, unreviewedPatchSets).stream().findFirst();
if (ps.isPresent()) {
throw new MutuallyExclusiveLabelsException(
getReviewedLabel(ps.get()), getUnreviewedLabel(ps.get()));
}
}
private static void validateLabels(Collection<String> labels) throws InvalidLabelsException {

36
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
View File

@@ -69,8 +69,9 @@ import com.google.gerrit.server.change.Index;
import com.google.gerrit.server.change.ListChangeComments;
import com.google.gerrit.server.change.ListChangeDrafts;
import com.google.gerrit.server.change.ListChangeRobotComments;
import com.google.gerrit.server.change.MarkAsReviewed;
import com.google.gerrit.server.change.MarkAsUnreviewed;
import com.google.gerrit.server.change.Move;
import com.google.gerrit.server.change.Mute;
import com.google.gerrit.server.change.PostHashtags;
import com.google.gerrit.server.change.PostPrivate;
import com.google.gerrit.server.change.PostReviewers;
@@ -88,7 +89,6 @@ import com.google.gerrit.server.change.SetWorkInProgress;
import com.google.gerrit.server.change.SubmittedTogether;
import com.google.gerrit.server.change.SuggestChangeReviewers;
import com.google.gerrit.server.change.Unignore;
import com.google.gerrit.server.change.Unmute;
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -140,8 +140,8 @@ class ChangeApiImpl implements ChangeApi {
private final DeletePrivate deletePrivate;
private final Ignore ignore;
private final Unignore unignore;
private final Mute mute;
private final Unmute unmute;
private final MarkAsReviewed markAsReviewed;
private final MarkAsUnreviewed markAsUnreviewed;
private final SetWorkInProgress setWip;
private final SetReadyForReview setReady;
private final PutMessage putMessage;
@@ -185,8 +185,8 @@ class ChangeApiImpl implements ChangeApi {
DeletePrivate deletePrivate,
Ignore ignore,
Unignore unignore,
Mute mute,
Unmute unmute,
MarkAsReviewed markAsReviewed,
MarkAsUnreviewed markAsUnreviewed,
SetWorkInProgress setWip,
SetReadyForReview setReady,
PutMessage putMessage,
@@ -228,8 +228,8 @@ class ChangeApiImpl implements ChangeApi {
this.deletePrivate = deletePrivate;
this.ignore = ignore;
this.unignore = unignore;
this.mute = mute;
this.unmute = unmute;
this.markAsReviewed = markAsReviewed;
this.markAsUnreviewed = markAsUnreviewed;
this.setWip = setWip;
this.setReady = setReady;
this.putMessage = putMessage;
@@ -677,26 +677,18 @@ class ChangeApiImpl implements ChangeApi {
}
@Override
public void mute(boolean mute) throws RestApiException {
public void markAsReviewed(boolean reviewed) throws RestApiException {
// TODO(dborowitz): Convert to RetryingRestModifyView. Needs to plumb BatchUpdate.Factory into
// StarredChangesUtil.
try {
if (mute) {
this.mute.apply(change, new Mute.Input());
if (reviewed) {
markAsReviewed.apply(change, new MarkAsReviewed.Input());
} else {
unmute.apply(change, new Unmute.Input());
markAsUnreviewed.apply(change, new MarkAsUnreviewed.Input());
}
} catch (OrmException | IllegalLabelException e) {
throw asRestApiException("Cannot mute change", e);
}
}
@Override
public boolean muted() throws RestApiException {
try {
return stars.isMuted(change);
} catch (OrmException e) {
throw asRestApiException("Cannot check if muted", e);
throw asRestApiException(
"Cannot mark change as " + (reviewed ? "reviewed" : "unreviewed"), e);
}
}

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

@@ -551,22 +551,13 @@ public class ChangeJson {
if (user.isIdentifiedUser()) {
Collection<String> stars = cd.stars(user.getAccountId());
out.starred = stars.contains(StarredChangesUtil.DEFAULT_LABEL) ? true : null;
out.muted =
stars.contains(StarredChangesUtil.MUTE_LABEL + "/" + cd.currentPatchSet().getPatchSetId())
? true
: null;
if (!stars.isEmpty()) {
out.stars = stars;
}
}
if (in.getStatus().isOpen() && has(REVIEWED) && user.isIdentifiedUser()) {
Account.Id accountId = user.getAccountId();
if (out.muted != null) {
out.reviewed = true;
} else {
out.reviewed = cd.reviewedBy().contains(accountId) ? true : null;
}
out.reviewed = cd.isReviewedBy(user.getAccountId()) ? true : null;
}
out.labels = labelsFor(perm, cd, has(LABELS), has(DETAILED_LABELS));

48
gerrit-server/src/main/java/com/google/gerrit/server/change/Mute.java → gerrit-server/src/main/java/com/google/gerrit/server/change/MarkAsReviewed.java
View File

@@ -14,66 +14,64 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class Mute implements RestModifyView<ChangeResource, Mute.Input>, UiAction<ChangeResource> {
private static final Logger log = LoggerFactory.getLogger(Mute.class);
public class MarkAsReviewed
implements RestModifyView<ChangeResource, MarkAsReviewed.Input>, UiAction<ChangeResource> {
private static final Logger log = LoggerFactory.getLogger(MarkAsReviewed.class);
public static class Input {}
private final Provider<ReviewDb> dbProvider;
private final ChangeData.Factory changeDataFactory;
private final StarredChangesUtil stars;
@Inject
Mute(StarredChangesUtil stars) {
MarkAsReviewed(
Provider<ReviewDb> dbProvider,
ChangeData.Factory changeDataFactory,
StarredChangesUtil stars) {
this.dbProvider = dbProvider;
this.changeDataFactory = changeDataFactory;
this.stars = stars;
}
@Override
public Description getDescription(ChangeResource rsrc) {
return new UiAction.Description()
.setLabel("Mute")
.setTitle("Mute the change to unhighlight it in the dashboard")
.setVisible(isMuteable(rsrc));
.setLabel("Mark Reviewed")
.setTitle("Mark the change as reviewed to unhighlight it in the dashboard")
.setVisible(!isReviewed(rsrc));
}
@Override
public Response<String> apply(ChangeResource rsrc, Input input)
throws RestApiException, OrmException, IllegalLabelException {
if (rsrc.isUserOwner()) {
throw new BadRequestException("cannot mute own change");
}
if (!isMuted(rsrc)) {
stars.mute(rsrc);
}
stars.markAsReviewed(rsrc);
return Response.ok("");
}
private boolean isMuted(ChangeResource rsrc) {
private boolean isReviewed(ChangeResource rsrc) {
try {
return stars.isMuted(rsrc);
return changeDataFactory
.create(dbProvider.get(), rsrc.getNotes())
.isReviewedBy(rsrc.getUser().asIdentifiedUser().getAccountId());
} catch (OrmException e) {
log.error("failed to check muted star", e);
}
return false;
}
private boolean isMuteable(ChangeResource rsrc) {
try {
return !rsrc.isUserOwner() && !isMuted(rsrc) && !stars.isIgnored(rsrc);
} catch (OrmException e) {
log.error("failed to check ignored star", e);
log.error("failed to check if change is reviewed", e);
}
return false;
}

36
gerrit-server/src/main/java/com/google/gerrit/server/change/Unmute.java → gerrit-server/src/main/java/com/google/gerrit/server/change/MarkAsUnreviewed.java
View File

@@ -17,50 +17,60 @@ package com.google.gerrit.server.change;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class Unmute
implements RestModifyView<ChangeResource, Unmute.Input>, UiAction<ChangeResource> {
private static final Logger log = LoggerFactory.getLogger(Unmute.class);
public class MarkAsUnreviewed
implements RestModifyView<ChangeResource, MarkAsUnreviewed.Input>, UiAction<ChangeResource> {
private static final Logger log = LoggerFactory.getLogger(MarkAsUnreviewed.class);
public static class Input {}
private final Provider<ReviewDb> dbProvider;
private final ChangeData.Factory changeDataFactory;
private final StarredChangesUtil stars;
@Inject
Unmute(StarredChangesUtil stars) {
MarkAsUnreviewed(
Provider<ReviewDb> dbProvider,
ChangeData.Factory changeDataFactory,
StarredChangesUtil stars) {
this.dbProvider = dbProvider;
this.changeDataFactory = changeDataFactory;
this.stars = stars;
}
@Override
public Description getDescription(ChangeResource rsrc) {
return new UiAction.Description()
.setLabel("Unmute")
.setTitle("Unmute the change")
.setVisible(isMuted(rsrc));
.setLabel("Mark Unreviewed")
.setTitle("Mark the change as unreviewed to highlight it in the dashboard")
.setVisible(isReviewed(rsrc));
}
@Override
public Response<String> apply(ChangeResource rsrc, Input input)
throws OrmException, IllegalLabelException {
if (isMuted(rsrc)) {
stars.unmute(rsrc);
}
stars.markAsUnreviewed(rsrc);
return Response.ok("");
}
private boolean isMuted(ChangeResource rsrc) {
private boolean isReviewed(ChangeResource rsrc) {
try {
return stars.isMuted(rsrc);
return changeDataFactory
.create(dbProvider.get(), rsrc.getNotes())
.isReviewedBy(rsrc.getUser().asIdentifiedUser().getAccountId());
} catch (OrmException e) {
log.error("failed to check muted star", e);
log.error("failed to check if change is reviewed", e);
}
return false;
}

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

@@ -90,8 +90,8 @@ public class Module extends RestApiModule {
delete(CHANGE_KIND, "private").to(DeletePrivate.class);
put(CHANGE_KIND, "ignore").to(Ignore.class);
put(CHANGE_KIND, "unignore").to(Unignore.class);
put(CHANGE_KIND, "mute").to(Mute.class);
put(CHANGE_KIND, "unmute").to(Unmute.class);
put(CHANGE_KIND, "reviewed").to(MarkAsReviewed.class);
put(CHANGE_KIND, "unreviewed").to(MarkAsUnreviewed.class);
post(CHANGE_KIND, "wip").to(SetWorkInProgress.class);
post(CHANGE_KIND, "ready").to(SetReadyForReview.class);
put(CHANGE_KIND, "message").to(PutMessage.class);

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

@@ -91,7 +91,10 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
@Deprecated static final Schema<ChangeData> V46 = schema(V45);
// Removal of draft change workflow requires reindexing
static final Schema<ChangeData> V47 = schema(V46);
@Deprecated static final Schema<ChangeData> V47 = schema(V46);
// Rename of star label 'mute' to 'reviewed' requires reindexing
static final Schema<ChangeData> V48 = schema(V47);
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();

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

@@ -1084,6 +1084,22 @@ public class ChangeData {
return draftsByUser;
}
public boolean isReviewedBy(Account.Id accountId) throws OrmException {
Collection<String> stars = stars(accountId);
if (stars.contains(
StarredChangesUtil.REVIEWED_LABEL + "/" + currentPatchSet().getPatchSetId())) {
return true;
}
if (stars.contains(
StarredChangesUtil.UNREVIEWED_LABEL + "/" + currentPatchSet().getPatchSetId())) {
return false;
}
return reviewedBy().contains(accountId);
}
public Set<Account.Id> reviewedBy() throws OrmException {
if (reviewedBy == null) {
if (!lazyLoad) {

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

@@ -35,7 +35,7 @@ import java.util.concurrent.TimeUnit;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
public static final Class<Schema_160> C = Schema_160.class;
public static final Class<Schema_161> C = Schema_161.class;
public static int getBinaryVersion() {
return guessVersion(C);

76
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_161.java Normal file
View File

@@ -0,0 +1,76 @@
// Copyright (C) 2017 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.schema;
import static java.util.stream.Collectors.toList;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
import com.google.gerrit.server.StarredChangesUtil.StarRef;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TextProgressMonitor;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
public class Schema_161 extends SchemaVersion {
private static final String MUTE_LABEL = "mute";
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
@Inject
Schema_161(
Provider<Schema_160> prior, GitRepositoryManager repoManager, AllUsersName allUsersName) {
super(prior);
this.repoManager = repoManager;
this.allUsersName = allUsersName;
}
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
try (Repository git = repoManager.openRepository(allUsersName);
RevWalk rw = new RevWalk(git)) {
BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate();
for (Ref ref : git.getRefDatabase().getRefs(RefNames.REFS_STARRED_CHANGES).values()) {
StarRef starRef = StarredChangesUtil.readLabels(git, ref.getName());
if (starRef.labels().contains(MUTE_LABEL)) {
ObjectId id =
StarredChangesUtil.writeLabels(
git,
starRef
.labels()
.stream()
.map(l -> l.equals(MUTE_LABEL) ? StarredChangesUtil.REVIEWED_LABEL : l)
.collect(toList()));
bru.addCommand(new ReceiveCommand(ObjectId.zeroId(), id, ref.getName()));
}
}
bru.execute(rw, new TextProgressMonitor());
} catch (IOException | IllegalLabelException ex) {
throw new OrmException(ex);
}
}
}