opendev/gerrit
Code Issues Proposed changes

Replace ChangeJson.ChangeInfo with extensions.common.ChangeInfo

Browse Source 
These have essentially the same public fields, except for a few that
were not previously exposed through ChangeApiImpl for no good reason.

The most invasive change this requires is to hold on to the
SubmitRecord.Label.Status value during label initialization in
ChangeJson, which we now do by holding a pair of (LabelInfo, Status).
This is an implementation detail of ChangeJson, so does not need to
leak into the ChangeInfo class.

Additionally, although we can kill most of ChangeInfoMapper, we still
need the mapping of Change.Status to ChangeStatus, to avoid exposing
an internal implementation detail of the database storage layer to the
extension API.

Change-Id: I20f78ed0bd893560bce1b55d47583d6cba3e3fb2
This commit is contained in:
Dave Borowitz 2014-11-25 13:39:35 -05:00 committed by David Pursehouse
parent f74f440635
commit bc485ee2fc
28 changed files with 174 additions and 297 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -38,7 +38,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.OutputFormat;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
@ -197,17 +196,17 @@ public abstract class AbstractDaemonTest {
return push.to(git, "refs/for/master");
}
protected ChangeJson.ChangeInfo getChange(String changeId, ListChangesOption... options)
protected ChangeInfo getChange(String changeId, ListChangesOption... options)
throws IOException {
return getChange(adminSession, changeId, options);
}
protected ChangeJson.ChangeInfo getChange(RestSession session, String changeId,
protected ChangeInfo getChange(RestSession session, String changeId,
ListChangesOption... options) throws IOException {
String q = options.length > 0 ? "?o=" + Joiner.on("&o=").join(options) : "";
RestResponse r = session.get("/changes/" + changeId + q);
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
return newGson().fromJson(r.getReader(), ChangeJson.ChangeInfo.class);
return newGson().fromJson(r.getReader(), ChangeInfo.class);
}
protected ChangeInfo info(String id)

7
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
View File

@ -30,7 +30,10 @@ import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeStatus;
import com.google.gerrit.extensions.common.InheritableBoolean;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.SubmitType;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@ -38,8 +41,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.change.ChangeJson.LabelInfo;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.PutConfig;
import com.google.gson.reflect.TypeToken;
@ -179,7 +180,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
ChangeInfo change =
newGson().fromJson(r.getReader(),
new TypeToken<ChangeInfo>() {}.getType());
assertEquals(Change.Status.MERGED, change.status);
assertEquals(ChangeStatus.MERGED, change.status);
}
r.consume();
}

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ConflictsOperatorIT.java
View File

@ -24,7 +24,7 @@ import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gson.reflect.TypeToken;
import org.apache.http.HttpStatus;

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@ -21,7 +21,6 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeStatus;
import com.google.gerrit.server.change.ChangeJson;
import org.apache.http.HttpStatus;
import org.junit.Test;
@ -79,8 +78,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
RestResponse r = adminSession.post("/changes/", in);
assertEquals(HttpStatus.SC_CREATED, r.getStatusCode());
ChangeJson.ChangeInfo info = newGson().fromJson(r.getReader(),
ChangeJson.ChangeInfo.class);
ChangeInfo info = newGson().fromJson(r.getReader(), ChangeInfo.class);
ChangeInfo out = get(info.changeId);
assertEquals(in.branch, out.branch);

1
gerrit-extension-api/BUCK
View File

@ -5,6 +5,7 @@ gwt_module(
name = 'client',
srcs = glob([
SRC + 'api/projects/ProjectState.java',
SRC + 'common/ChangeStatus.java',
SRC + 'common/InheritableBoolean.java',
SRC + 'common/ListChangesOption.java',
SRC + 'common/SubmitType.java',

21
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
View File

@ -23,6 +23,7 @@ public class ChangeInfo {
public String project;
public String branch;
public String topic;
public Collection<String> hashtags;
public String changeId;
public String subject;
public ChangeStatus status;
@ -33,12 +34,20 @@ public class ChangeInfo {
public Boolean mergeable;
public Integer insertions;
public Integer deletions;
public AccountInfo owner;
public String currentRevision;
public Map<String, ActionInfo> actions;
public Map<String, LabelInfo> labels;
public Collection<ChangeMessageInfo> messages;
public Map<String, RevisionInfo> revisions;
public String _sortkey;
public String baseChange;
public int _number;
public AccountInfo owner;
public Map<String, ActionInfo> actions;
public Map<String, LabelInfo> labels;
public Map<String, Collection<String>> permittedLabels;
public Collection<AccountInfo> removableReviewers;
public Collection<ChangeMessageInfo> messages;
public String currentRevision;
public Map<String, RevisionInfo> revisions;
public Boolean _moreChanges;
}

2
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/LabelInfo.java
View File

@ -23,7 +23,9 @@ public class LabelInfo {
public AccountInfo recommended;
public AccountInfo disliked;
public List<ApprovalInfo> all;
public Map<String, String> values;
public Short value;
public Short defaultValue;
public Boolean optional;

44
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
View File

@ -16,12 +16,14 @@ package com.google.gerrit.reviewdb.client;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
import com.google.gerrit.extensions.common.ChangeStatus;
import com.google.gwtorm.client.Column;
import com.google.gwtorm.client.IntKey;
import com.google.gwtorm.client.RowVersion;
import com.google.gwtorm.client.StringKey;
import java.sql.Timestamp;
import java.util.Arrays;
/**
* A change proposed to be merged into a {@link Branch}.
@ -279,7 +281,7 @@ public final class Change {
* <li>{@link #ABANDONED} - when the Abandon action is used.
* </ul>
*/
NEW(STATUS_NEW),
NEW(STATUS_NEW, ChangeStatus.NEW),
/**
* Change is open, but has been submitted to the merge queue.
@ -306,7 +308,7 @@ public final class Change {
* <li>{@link #ABANDONED} - when the Abandon action is used.
* </ul>
*/
SUBMITTED(STATUS_SUBMITTED),
SUBMITTED(STATUS_SUBMITTED, ChangeStatus.SUBMITTED),
/**
* Change is a draft change that only consists of draft patchsets.
@ -324,7 +326,7 @@ public final class Change {
* <li>{@link #NEW} - when the change is published, it becomes a new change;
* </ul>
*/
DRAFT(STATUS_DRAFT),
DRAFT(STATUS_DRAFT, ChangeStatus.DRAFT),
/**
* Change is closed, and submitted to its destination branch.
@ -334,7 +336,7 @@ public final class Change {
* replacement patch set. Draft comments however may be published,
* supporting a post-submit review.
*/
MERGED(STATUS_MERGED),
MERGED(STATUS_MERGED, ChangeStatus.MERGED),
/**
* Change is closed, but was not submitted to its destination branch.
@ -344,14 +346,31 @@ public final class Change {
* a replacement patch set, and it cannot be merged. Draft comments however
* may be published, permitting reviewers to send constructive feedback.
*/
ABANDONED('A');
ABANDONED('A', ChangeStatus.ABANDONED);
static {
boolean ok = true;
if (Status.values().length != ChangeStatus.values().length) {
ok = false;
}
for (Status s : Status.values()) {
ok &= s.name().equals(s.changeStatus.name());
}
if (!ok) {
throw new IllegalStateException("Mismatched status mapping: "
+ Arrays.asList(Status.values()) + " != "
+ Arrays.asList(ChangeStatus.values()));
}
}
private final char code;
private final boolean closed;
private final ChangeStatus changeStatus;
private Status(final char c) {
private Status(char c, ChangeStatus cs) {
code = c;
closed = !(MIN_OPEN <= c && c <= MAX_OPEN);
changeStatus = cs;
}
public char getCode() {
@ -366,6 +385,10 @@ public final class Change {
return closed;
}
public ChangeStatus asChangeStatus() {
return changeStatus;
}
public static Status forCode(final char c) {
for (final Status s : Status.values()) {
if (s.code == c) {
@ -374,6 +397,15 @@ public final class Change {
}
return null;
}
public static Status forChangeStatus(ChangeStatus cs) {
for (Status s : Status.values()) {
if (s.changeStatus == cs) {
return s;
}
}
return null;
}
}
/** Locally assigned unique identifier of the change */

9
gerrit-server/BUCK
View File

@ -41,14 +41,12 @@ java_library(
'//lib:jsch',
'//lib:juniversalchardet',
'//lib:mime-util',
'//lib/ow2:ow2-asm',
'//lib/ow2:ow2-asm-tree',
'//lib/ow2:ow2-asm-util',
'//lib:parboiled-core',
'//lib:pegdown',
'//lib:protobuf',
'//lib:velocity',
'//lib/antlr:java_runtime',
'//lib/auto:auto-value',
'//lib/commons:codec',
'//lib/commons:dbcp',
'//lib/commons:lang',
@ -62,10 +60,13 @@ java_library(
'//lib/joda:joda-time',
'//lib/log:api',
'//lib/log:log4j',
'//lib/prolog:prolog-cafe',
'//lib/lucene:analyzers-common',
'//lib/lucene:core',
'//lib/lucene:query-parser',
'//lib/ow2:ow2-asm',
'//lib/ow2:ow2-asm-tree',
'//lib/ow2:ow2-asm-util',
'//lib/prolog:prolog-cafe',
],
provided_deps = [
'//lib:servlet-api-3_1',

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

@ -198,8 +198,7 @@ class ChangeApiImpl extends ChangeApi.NotImplemented implements ChangeApi {
public ChangeInfo get(EnumSet<ListChangesOption> s)
throws RestApiException {
try {
return ChangeInfoMapper.INSTANCE.apply(
changeJson.get().addOptions(s).format(change));
return changeJson.get().addOptions(s).format(change);
} catch (OrmException e) {
throw new RestApiException("Cannot retrieve change", e);
}

130
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java
View File

@ -1,130 +0,0 @@
// Copyright (C) 2014 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.api.changes;
import com.google.common.base.Function;
import com.google.common.collect.EnumBiMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.ChangeStatus;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.change.ChangeJson;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
public class ChangeInfoMapper
implements Function<ChangeJson.ChangeInfo, ChangeInfo> {
public static final ChangeInfoMapper INSTANCE = new ChangeInfoMapper();
private final static EnumBiMap<Change.Status, ChangeStatus> STATUS_MAP =
EnumBiMap.create(Change.Status.class, ChangeStatus.class);
static {
STATUS_MAP.put(Status.DRAFT, ChangeStatus.DRAFT);
STATUS_MAP.put(Status.NEW, ChangeStatus.NEW);
STATUS_MAP.put(Status.SUBMITTED, ChangeStatus.SUBMITTED);
STATUS_MAP.put(Status.MERGED, ChangeStatus.MERGED);
STATUS_MAP.put(Status.ABANDONED, ChangeStatus.ABANDONED);
}
public static Status changeStatus2Status(ChangeStatus status) {
if (status != null) {
return STATUS_MAP.inverse().get(status);
}
return Change.Status.NEW;
}
private ChangeInfoMapper() {
}
@Override
public ChangeInfo apply(ChangeJson.ChangeInfo i) {
ChangeInfo o = new ChangeInfo();
mapCommon(i, o);
mapLabels(i, o);
mapMessages(i, o);
o.revisions = i.revisions;
o.actions = i.actions;
return o;
}
private void mapCommon(ChangeJson.ChangeInfo i, ChangeInfo o) {
o.id = i.id;
o.project = i.project;
o.branch = i.branch;
o.topic = i.topic;
o.changeId = i.changeId;
o.subject = i.subject;
o.status = STATUS_MAP.get(i.status);
o.created = i.created;
o.updated = i.updated;
o.starred = i.starred;
o.reviewed = i.reviewed;
o.mergeable = i.mergeable;
o.insertions = i.insertions;
o.deletions = i.deletions;
o.owner = i.owner;
o.currentRevision = i.currentRevision;
o._number = i._number;
}
private void mapMessages(ChangeJson.ChangeInfo i, ChangeInfo o) {
if (i.messages == null) {
return;
}
List<ChangeMessageInfo> r =
Lists.newArrayListWithCapacity(i.messages.size());
for (ChangeJson.ChangeMessageInfo m : i.messages) {
ChangeMessageInfo cmi = new ChangeMessageInfo();
cmi.id = m.id;
cmi.author = m.author;
cmi.date = m.date;
cmi.message = m.message;
cmi._revisionNumber = m._revisionNumber;
r.add(cmi);
}
o.messages = r;
}
private void mapLabels(ChangeJson.ChangeInfo i, ChangeInfo o) {
if (i.labels == null) {
return;
}
Map<String, LabelInfo> r = Maps.newLinkedHashMap();
for (Map.Entry<String, ChangeJson.LabelInfo> e : i.labels.entrySet()) {
ChangeJson.LabelInfo li = e.getValue();
LabelInfo lo = new LabelInfo();
lo.approved = li.approved;
lo.rejected = li.rejected;
lo.recommended = li.recommended;
lo.disliked = li.disliked;
lo.value = li.value;
lo.defaultValue = li.defaultValue;
lo.optional = li.optional;
lo.blocking = li.blocking;
lo.values = li.values;
if (li.all != null) {
lo.all = new ArrayList<>(li.all);
}
r.put(e.getKey(), lo);
}
o.labels = r;
}
}

11
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
View File

@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.Changes;
import com.google.gerrit.extensions.common.ChangeInfo;
@ -30,7 +29,6 @@ import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.CreateChange;
import com.google.gerrit.server.project.InvalidChangeOperationException;
@ -89,7 +87,7 @@ class ChangesImpl implements Changes {
@Override
public ChangeApi create(ChangeInfo in) throws RestApiException {
try {
ChangeJson.ChangeInfo out = createChange.apply(
ChangeInfo out = createChange.apply(
TopLevelResource.INSTANCE, in).value();
return api.create(changes.parse(TopLevelResource.INSTANCE,
IdString.fromUrl(out.changeId)));
@ -133,12 +131,11 @@ class ChangesImpl implements Changes {
// Check type safety of result; the extension API should be safer than the
// REST API in this case, since it's intended to be used in Java.
Object first = checkNotNull(result.iterator().next());
checkState(first instanceof ChangeJson.ChangeInfo);
checkState(first instanceof ChangeInfo);
@SuppressWarnings("unchecked")
List<ChangeJson.ChangeInfo> infos = (List<ChangeJson.ChangeInfo>) result;
List<ChangeInfo> infos = (List<ChangeInfo>) result;
return ImmutableList.copyOf(
Lists.transform(infos, ChangeInfoMapper.INSTANCE));
return ImmutableList.copyOf(infos);
} catch (BadRequestException | AuthException | OrmException e) {
throw new RestApiException("Cannot query changes", e);
}

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

@ -18,6 +18,7 @@ import com.google.common.base.Strings;
import com.google.common.util.concurrent.CheckedFuture;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.extensions.api.changes.AbandonInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@ -28,7 +29,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.AbandonedSender;
import com.google.gerrit.server.mail.ReplyToChangeSender;

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

@ -30,6 +30,8 @@ import static com.google.gerrit.extensions.common.ListChangesOption.MESSAGES;
import static com.google.gerrit.extensions.common.ListChangesOption.REVIEWED;
import static com.google.gerrit.extensions.common.ListChangesOption.WEB_LINKS;
import com.google.auto.value.AutoValue;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Optional;
@ -37,6 +39,7 @@ import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
@ -45,6 +48,7 @@ import com.google.common.collect.Multimap;
import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
import com.google.common.collect.Table;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.LabelValue;
@ -54,9 +58,12 @@ import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.FetchInfo;
import com.google.gerrit.extensions.common.GitPerson;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
@ -113,6 +120,7 @@ import java.util.TreeMap;
public class ChangeJson {
private static final Logger log = LoggerFactory.getLogger(ChangeJson.class);
private static final List<ChangeMessage> NO_MESSAGES =
ImmutableList.of();
@ -285,7 +293,7 @@ public class ChangeJson {
out.deletions = changedLines.deletions;
}
out.subject = in.getSubject();
out.status = in.getStatus();
out.status = in.getStatus().asChangeStatus();
out.owner = accountLoader.get(in.getOwner());
out.created = in.getCreatedOn();
out.updated = in.getLastUpdatedOn();
@ -297,6 +305,7 @@ public class ChangeJson {
out.reviewed = in.getStatus().isOpen()
&& has(REVIEWED)
&& reviewed.contains(cd.getId()) ? true : null;
out.labels = labelsFor(ctl, cd, has(LABELS), has(DETAILED_LABELS));
if (out.labels != null && has(DETAILED_LABELS)) {
@ -313,7 +322,7 @@ public class ChangeJson {
if (has(MESSAGES)) {
out.messages = messages(ctl, cd, src);
}
out.finish();
finish(out);
if (has(ALL_REVISIONS)
|| has(CURRENT_REVISION)
@ -364,8 +373,8 @@ public class ChangeJson {
return cd.getSubmitRecords();
}
private Map<String, LabelInfo> labelsFor(ChangeControl ctl, ChangeData cd, boolean standard,
boolean detailed) throws OrmException {
private Map<String, LabelInfo> labelsFor(ChangeControl ctl,
ChangeData cd, boolean standard, boolean detailed) throws OrmException {
if (!standard && !detailed) {
return null;
}
@ -375,21 +384,21 @@ public class ChangeJson {
}
LabelTypes labelTypes = ctl.getLabelTypes();
if (cd.change().getStatus().isOpen()) {
return labelsForOpenChange(ctl, cd, labelTypes, standard, detailed);
} else {
return labelsForClosedChange(cd, labelTypes, standard, detailed);
}
Map<String, LabelWithStatus> withStatus = cd.change().getStatus().isOpen()
? labelsForOpenChange(ctl, cd, labelTypes, standard, detailed)
: labelsForClosedChange(cd, labelTypes, standard, detailed);
return ImmutableMap.copyOf(
Maps.transformValues(withStatus, LabelWithStatus.TO_LABEL_INFO));
}
private Map<String, LabelInfo> labelsForOpenChange(ChangeControl ctl,
private Map<String, LabelWithStatus> labelsForOpenChange(ChangeControl ctl,
ChangeData cd, LabelTypes labelTypes, boolean standard, boolean detailed)
throws OrmException {
Map<String, LabelInfo> labels = initLabels(cd, labelTypes, standard);
Map<String, LabelWithStatus> labels = initLabels(cd, labelTypes, standard);
if (detailed) {
setAllApprovals(ctl, cd, labels);
}
for (Map.Entry<String, LabelInfo> e : labels.entrySet()) {
for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
LabelType type = labelTypes.byLabel(e.getKey());
if (type == null) {
continue;
@ -410,19 +419,18 @@ public class ChangeJson {
return labels;
}
private Map<String, LabelInfo> initLabels(ChangeData cd,
private Map<String, LabelWithStatus> initLabels(ChangeData cd,
LabelTypes labelTypes, boolean standard) throws OrmException {
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
Map<String, LabelInfo> labels = new TreeMap<>(labelTypes.nameComparator());
Map<String, LabelWithStatus> labels = new TreeMap<>(labelTypes.nameComparator());
for (SubmitRecord rec : submitRecords(cd)) {
if (rec.labels == null) {
continue;
}
for (SubmitRecord.Label r : rec.labels) {
LabelInfo p = labels.get(r.label);
if (p == null || p._status.compareTo(r.status) < 0) {
LabelWithStatus p = labels.get(r.label);
if (p == null || p.status().compareTo(r.status) < 0) {
LabelInfo n = new LabelInfo();
n._status = r.status;
if (standard) {
switch (r.status) {
case OK:
@ -437,8 +445,8 @@ public class ChangeJson {
}
}
n.optional = n._status == SubmitRecord.Label.Status.MAY ? true : null;
labels.put(r.label, n);
n.optional = r.status == SubmitRecord.Label.Status.MAY ? true : null;
labels.put(r.label, LabelWithStatus.create(n, r.status));
}
}
}
@ -446,8 +454,8 @@ public class ChangeJson {
}
private void setLabelScores(LabelType type,
LabelInfo label, short score, Account.Id accountId) {
if (label.approved != null || label.rejected != null) {
LabelWithStatus l, short score, Account.Id accountId) {
if (l.label().approved != null || l.label().rejected != null) {
return;
}
@ -458,21 +466,21 @@ public class ChangeJson {
if (score != 0) {
if (score == type.getMin().getValue()) {
label.rejected = accountLoader.get(accountId);
l.label().rejected = accountLoader.get(accountId);
} else if (score == type.getMax().getValue()) {
label.approved = accountLoader.get(accountId);
l.label().approved = accountLoader.get(accountId);
} else if (score < 0) {
label.disliked = accountLoader.get(accountId);
label.value = score;
} else if (score > 0 && label.disliked == null) {
label.recommended = accountLoader.get(accountId);
label.value = score;
l.label().disliked = accountLoader.get(accountId);
l.label().value = score;
} else if (score > 0 && l.label().disliked == null) {
l.label().recommended = accountLoader.get(accountId);
l.label().value = score;
}
}
}
private void setAllApprovals(ChangeControl baseCtrl, ChangeData cd,
Map<String, LabelInfo> labels) throws OrmException {
Map<String, LabelWithStatus> labels) throws OrmException {
// Include a user in the output for this label if either:
// - They are an explicit reviewer.
// - They ever voted on this change.
@ -491,7 +499,7 @@ public class ChangeJson {
for (Account.Id accountId : allUsers) {
IdentifiedUser user = userFactory.create(accountId);
ChangeControl ctl = baseCtrl.forUser(user);
for (Map.Entry<String, LabelInfo> e : labels.entrySet()) {
for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
LabelType lt = ctl.getLabelTypes().byLabel(e.getKey());
if (lt == null) {
// Ignore submit record for undefined label; likely the submit rule
@ -510,12 +518,12 @@ public class ChangeJson {
// user can vote on this label.
value = labelNormalizer.canVote(ctl, lt, accountId) ? 0 : null;
}
e.getValue().addApproval(approvalInfo(accountId, value, date));
addApproval(e.getValue().label(), approvalInfo(accountId, value, date));
}
}
}
private Map<String, LabelInfo> labelsForClosedChange(ChangeData cd,
private Map<String, LabelWithStatus> labelsForClosedChange(ChangeData cd,
LabelTypes labelTypes, boolean standard, boolean detailed)
throws OrmException {
Set<Account.Id> allUsers = Sets.newHashSet();
@ -538,14 +546,15 @@ public class ChangeJson {
}
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
Map<String, LabelInfo> labels = new TreeMap<>(labelTypes.nameComparator());
Map<String, LabelWithStatus> labels =
new TreeMap<>(labelTypes.nameComparator());
for (String name : labelNames) {
LabelType type = labelTypes.byLabel(name);
LabelInfo li = new LabelInfo();
LabelWithStatus l = LabelWithStatus.create(new LabelInfo(), null);
if (detailed) {
setLabelValues(type, li);
setLabelValues(type, l);
}
labels.put(type.getName(), li);
labels.put(type.getName(), l);
}
for (Account.Id accountId : allUsers) {
@ -553,10 +562,10 @@ public class ChangeJson {
Maps.newHashMapWithExpectedSize(labels.size());
if (detailed) {
for (Map.Entry<String, LabelInfo> entry : labels.entrySet()) {
for (Map.Entry<String, LabelWithStatus> entry : labels.entrySet()) {
ApprovalInfo ai = approvalInfo(accountId, 0, null);
byLabel.put(entry.getKey(), ai);
entry.getValue().addApproval(ai);
addApproval(entry.getValue().label(), ai);
}
}
for (PatchSetApproval psa : current.get(accountId)) {
@ -571,13 +580,11 @@ public class ChangeJson {
info.value = Integer.valueOf(val);
info.date = psa.getGranted();
}
LabelInfo li = labels.get(type.getName());
if (!standard) {
continue;
}
setLabelScores(type, li, val, accountId);
setLabelScores(type, labels.get(type.getName()), val, accountId);
}
}
return labels;
@ -595,14 +602,14 @@ public class ChangeJson {
return values.isEmpty() || (values.size() == 1 && values.contains(" 0"));
}
private void setLabelValues(LabelType type, LabelInfo label) {
label.defaultValue = type.getDefaultValue();
label.values = Maps.newLinkedHashMap();
private void setLabelValues(LabelType type, LabelWithStatus l) {
l.label().defaultValue = type.getDefaultValue();
l.label().values = Maps.newLinkedHashMap();
for (LabelValue v : type.getValues()) {
label.values.put(v.formatValue(), v.getText());
l.label().values.put(v.formatValue(), v.getText());
}
if (isOnlyZero(label.values.keySet())) {
label.values = null;
if (isOnlyZero(l.label().values.keySet())) {
l.label().values = null;
}
}
@ -925,75 +932,36 @@ public class ChangeJson {
return p;
}
public static class ChangeInfo {
public String id;
public String project;
public String branch;
public String topic;
public Collection<String> hashtags;
public String changeId;
public String subject;
public Change.Status status;
public Timestamp created;
public Timestamp updated;
public Boolean starred;
public Boolean reviewed;
public Boolean mergeable;
public Integer insertions;
public Integer deletions;
public String _sortkey;
public int _number;
public AccountInfo owner;
public Map<String, ActionInfo> actions;
public Map<String, LabelInfo> labels;
public Map<String, Collection<String>> permittedLabels;
public Collection<AccountInfo> removableReviewers;
public Collection<ChangeMessageInfo> messages;
public String currentRevision;
public Map<String, RevisionInfo> revisions;
public Boolean _moreChanges;
void finish() {
id = Joiner.on('~').join(
Url.encode(project),
Url.encode(branch),
Url.encode(changeId));
}
static void finish(ChangeInfo info) {
info.id = Joiner.on('~').join(
Url.encode(info.project),
Url.encode(info.branch),
Url.encode(info.changeId));
}
public static class LabelInfo {
transient SubmitRecord.Label.Status _status;
public AccountInfo approved;
public AccountInfo rejected;
public AccountInfo recommended;
public AccountInfo disliked;
public List<ApprovalInfo> all;
public Map<String, String> values;
public Short value;
public Short defaultValue;
public Boolean optional;
public Boolean blocking;
void addApproval(ApprovalInfo ai) {
if (all == null) {
all = Lists.newArrayList();
}
all.add(ai);
private static void addApproval(LabelInfo label, ApprovalInfo approval) {
if (label.all == null) {
label.all = Lists.newArrayList();
}
label.all.add(approval);
}
public static class ChangeMessageInfo {
public String id;
public AccountInfo author;
public Timestamp date;
public String message;
public Integer _revisionNumber;
@AutoValue
abstract static class LabelWithStatus {
private static final Function<LabelWithStatus, LabelInfo> TO_LABEL_INFO =
new Function<LabelWithStatus, LabelInfo>() {
@Override
public LabelInfo apply(LabelWithStatus in) {
return in.label();
}
};
private static LabelWithStatus create(LabelInfo label,
SubmitRecord.Label.Status status) {
return new AutoValue_ChangeJson_LabelWithStatus(label, status);
}
abstract LabelInfo label();
@Nullable abstract SubmitRecord.Label.Status status();
}
}

6
gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java
View File

@ -15,9 +15,9 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -65,12 +65,12 @@ public class Check implements RestReadView<ChangeResource> {
info.topic = c.getTopic();
info.changeId = c.getKey().get();
info.subject = c.getSubject();
info.status = c.getStatus();
info.status = c.getStatus().asChangeStatus();
info.owner = new AccountInfo(c.getOwner().get());
info.created = c.getCreatedOn();
info.updated = c.getLastUpdatedOn();
info._number = c.getId().get();
info.finish();
ChangeJson.finish(info);
return info;
}
}

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

@ -14,7 +14,7 @@
package com.google.gerrit.server.change;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import java.util.List;

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

@ -16,6 +16,7 @@ package com.google.gerrit.server.change;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -25,7 +26,6 @@ import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;

6
gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
View File

@ -36,7 +36,6 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.api.changes.ChangeInfoMapper;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@ -107,7 +106,7 @@ public class CreateChange implements
}
@Override
public Response<ChangeJson.ChangeInfo> apply(TopLevelResource parent,
public Response<ChangeInfo> apply(TopLevelResource parent,
ChangeInfo input) throws AuthException, OrmException,
BadRequestException, UnprocessableEntityException, IOException,
InvalidChangeOperationException, ResourceNotFoundException {
@ -200,7 +199,8 @@ public class CreateChange implements
updateRef(git, rw, c, change, ins.getPatchSet());
change.setTopic(input.topic);
change.setStatus(ChangeInfoMapper.changeStatus2Status(input.status));
change.setStatus(input.status != null
? Change.Status.forChangeStatus(input.status) : Change.Status.NEW);
ins.insert();
return Response.created(json.format(change.getId()));

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

@ -17,6 +17,7 @@ package com.google.gerrit.server.change;
import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -26,7 +27,6 @@ import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.change.EditMessage.Input;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;

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

@ -14,11 +14,11 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.CacheControl;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;

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

@ -14,10 +14,10 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;

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

@ -14,10 +14,10 @@
package com.google.gerrit.server.change;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;

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

@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -24,7 +25,6 @@ import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.change.Rebase.Input;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.project.ChangeControl;

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

@ -18,6 +18,7 @@ import com.google.common.base.Strings;
import com.google.common.util.concurrent.CheckedFuture;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.extensions.api.changes.RestoreInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@ -29,7 +30,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.ReplyToChangeSender;
import com.google.gerrit.server.mail.RestoredSender;

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

@ -18,6 +18,7 @@ import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -28,7 +29,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;

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

@ -31,6 +31,7 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@ -51,7 +52,6 @@ import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LabelNormalizer;

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

@ -15,6 +15,7 @@
package com.google.gerrit.server.query.change;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@ -23,7 +24,6 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;

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

@ -35,6 +35,7 @@ import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.lifecycle.LifecycleManager;
@ -49,7 +50,6 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;