Merge changes from topic "group-resolver"
* changes:
AddReviewersOp: Get user and ReviewDb from context
Move ReviewerAdder and its dependencies out of restapi package
Rename PostReviewers{Op,Email} to AddReviewers{Op,Email}
Move most methods from AccountsCollection to AccountResolver
Factor a non-restapi GroupResolver out of GroupsCollection
Move common reviewer addition code out of PostReviewers
PostReviewersOp: Pass correct to/CC arguments to emailReviewers
AbstractNotificationTest: Improve readability of failure message
Add push tests for auto-adding forged identities as reviewers
Add tests for re-adding the same reviewers
Add notification tests for adding reviewer/CC by email
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 4cece72..be50d3b 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1218,15 +1218,6 @@
+
Default is true.
-[[change.enableParallelFormatting]]change.enableParallelFormatting::
-+
-Whether or not changes can be formatted in parallel when requesting
-multiple changes at once. An example for this is Dashboards.
-+
-This setting is experimental.
-+
-Default is `false`.
-
[[change.showAssigneeInChangesTable]]change.showAssigneeInChangesTable::
+
Show assignee field in changes table. If set to false, assignees will
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 9dc9359..ad41920 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -14,55 +14,36 @@
package com.google.gerrit.server.change;
-import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_COMMITS;
-import static com.google.gerrit.extensions.client.ListChangesOption.ALL_FILES;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CHANGE_ACTIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CHECK;
import static com.google.gerrit.extensions.client.ListChangesOption.COMMIT_FOOTERS;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_ACTIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
-import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_FILES;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_ACCOUNTS;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
-import static com.google.gerrit.extensions.client.ListChangesOption.DOWNLOAD_COMMANDS;
import static com.google.gerrit.extensions.client.ListChangesOption.LABELS;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
-import static com.google.gerrit.extensions.client.ListChangesOption.PUSH_CERTIFICATES;
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWER_UPDATES;
import static com.google.gerrit.extensions.client.ListChangesOption.SKIP_MERGEABLE;
import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE;
import static com.google.gerrit.extensions.client.ListChangesOption.TRACKING_IDS;
-import static com.google.gerrit.extensions.client.ListChangesOption.WEB_LINKS;
import static com.google.gerrit.server.ChangeMessagesUtil.createChangeMessageInfo;
-import static com.google.gerrit.server.CommonConverters.toGitPerson;
import static java.util.stream.Collectors.toList;
-import com.google.auto.value.AutoValue;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Throwables;
-import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
-import com.google.common.collect.MultimapBuilder;
-import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
-import com.google.common.collect.Table;
import com.google.common.flogger.FluentLogger;
-import com.google.common.primitives.Ints;
-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;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRecord.Status;
import com.google.gerrit.common.data.SubmitRequirement;
@@ -74,22 +55,13 @@
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.LabelInfo;
import com.google.gerrit.extensions.common.ProblemInfo;
-import com.google.gerrit.extensions.common.PushCertificateInfo;
import com.google.gerrit.extensions.common.ReviewerUpdateInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.SubmitRequirementInfo;
import com.google.gerrit.extensions.common.TrackingIdInfo;
import com.google.gerrit.extensions.common.VotingRangeInfo;
-import com.google.gerrit.extensions.common.WebLinkInfo;
-import com.google.gerrit.extensions.config.DownloadCommand;
-import com.google.gerrit.extensions.config.DownloadScheme;
-import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.registration.Extension;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.index.query.QueryResult;
import com.google.gerrit.mail.Address;
@@ -100,40 +72,27 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
-import com.google.gerrit.reviewdb.client.Patch;
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.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.AnonymousUser;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.GpgException;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil;
-import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.AccountInfoComparator;
import com.google.gerrit.server.account.AccountLoader;
-import com.google.gerrit.server.account.GpgApiAdapter;
-import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.TrackingFooters;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.ChangePermission;
-import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
@@ -150,24 +109,11 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import java.util.TreeMap;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
import java.util.function.Supplier;
-import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
/**
* Produces {@link ChangeInfo} (which is serialized to JSON afterwards) from {@link ChangeData}.
@@ -184,7 +130,7 @@
public static final SubmitRuleOptions SUBMIT_RULE_OPTIONS_STRICT =
ChangeField.SUBMIT_RULE_OPTIONS_STRICT.toBuilder().build();
- public static final ImmutableSet<ListChangesOption> REQUIRE_LAZY_LOAD =
+ static final ImmutableSet<ListChangesOption> REQUIRE_LAZY_LOAD =
ImmutableSet.of(
ALL_COMMITS,
ALL_REVISIONS,
@@ -252,34 +198,21 @@
private final Provider<ReviewDb> db;
private final Provider<CurrentUser> userProvider;
- private final AnonymousUser anonymous;
private final PermissionBackend permissionBackend;
- private final GitRepositoryManager repoManager;
- private final ProjectCache projectCache;
- private final MergeUtil.Factory mergeUtilFactory;
- private final IdentifiedUser.GenericFactory userFactory;
private final ChangeData.Factory changeDataFactory;
- private final FileInfoJson fileInfoJson;
private final AccountLoader.Factory accountLoaderFactory;
- private final DynamicMap<DownloadScheme> downloadSchemes;
- private final DynamicMap<DownloadCommand> downloadCommands;
- private final WebLinks webLinks;
private final ImmutableSet<ListChangesOption> options;
private final ChangeMessagesUtil cmUtil;
private final Provider<ConsistencyChecker> checkerProvider;
private final ActionJson actionJson;
- private final GpgApiAdapter gpgApi;
private final ChangeNotes.Factory notesFactory;
- private final ChangeResource.Factory changeResourceFactory;
- private final ChangeKindCache changeKindCache;
- private final ApprovalsUtil approvalsUtil;
+ private final LabelsJson labelsJson;
private final RemoveReviewerControl removeReviewerControl;
private final TrackingFooters trackingFooters;
private final Metrics metrics;
- private final boolean enableParallelFormatting;
- private final ExecutorService fanOutExecutor;
+ private final RevisionJson revisionJson;
+ private final boolean lazyLoad;
- private boolean lazyLoad = true;
private AccountLoader accountLoader;
private FixInput fix;
private PluginDefinedAttributesFactory pluginDefinedAttributesFactory;
@@ -288,70 +221,50 @@
ChangeJson(
Provider<ReviewDb> db,
Provider<CurrentUser> user,
- AnonymousUser au,
PermissionBackend permissionBackend,
- GitRepositoryManager repoManager,
- ProjectCache projectCache,
- MergeUtil.Factory mergeUtilFactory,
- IdentifiedUser.GenericFactory uf,
ChangeData.Factory cdf,
- FileInfoJson fileInfoJson,
AccountLoader.Factory ailf,
- DynamicMap<DownloadScheme> downloadSchemes,
- DynamicMap<DownloadCommand> downloadCommands,
- WebLinks webLinks,
ChangeMessagesUtil cmUtil,
Provider<ConsistencyChecker> checkerProvider,
ActionJson actionJson,
- GpgApiAdapter gpgApi,
ChangeNotes.Factory notesFactory,
- ChangeResource.Factory changeResourceFactory,
- ChangeKindCache changeKindCache,
- ApprovalsUtil approvalsUtil,
+ LabelsJson.Factory labelsJsonFactory,
RemoveReviewerControl removeReviewerControl,
TrackingFooters trackingFooters,
Metrics metrics,
- @GerritServerConfig Config config,
- @FanOutExecutor ExecutorService fanOutExecutor,
+ RevisionJson.Factory revisionJsonFactory,
@Assisted Iterable<ListChangesOption> options) {
this.db = db;
this.userProvider = user;
- this.anonymous = au;
this.changeDataFactory = cdf;
this.permissionBackend = permissionBackend;
- this.repoManager = repoManager;
- this.userFactory = uf;
- this.projectCache = projectCache;
- this.mergeUtilFactory = mergeUtilFactory;
- this.fileInfoJson = fileInfoJson;
this.accountLoaderFactory = ailf;
- this.downloadSchemes = downloadSchemes;
- this.downloadCommands = downloadCommands;
- this.webLinks = webLinks;
this.cmUtil = cmUtil;
this.checkerProvider = checkerProvider;
this.actionJson = actionJson;
- this.gpgApi = gpgApi;
this.notesFactory = notesFactory;
- this.changeResourceFactory = changeResourceFactory;
- this.changeKindCache = changeKindCache;
- this.approvalsUtil = approvalsUtil;
+ this.labelsJson = labelsJsonFactory.create(options);
this.removeReviewerControl = removeReviewerControl;
this.trackingFooters = trackingFooters;
this.metrics = metrics;
- this.enableParallelFormatting = config.getBoolean("change", "enableParallelFormatting", false);
- this.fanOutExecutor = fanOutExecutor;
+ this.revisionJson = revisionJsonFactory.create(options);
this.options = Sets.immutableEnumSet(options);
+ this.lazyLoad = containsAnyOf(this.options, REQUIRE_LAZY_LOAD);
logger.atFine().log("options = %s", options);
}
- /**
- * See {@link ChangeData#setLazyLoad(boolean)}. If lazyLoad is set, converting data from
- * index-backed {@link ChangeData} will fail with an exception.
- */
- public ChangeJson lazyLoad(boolean load) {
- lazyLoad = load;
- return this;
+ public static ApprovalInfo getApprovalInfo(
+ Account.Id id,
+ Integer value,
+ VotingRangeInfo permittedVotingRange,
+ String tag,
+ Timestamp date) {
+ ApprovalInfo ai = new ApprovalInfo(id.get());
+ ai.value = value;
+ ai.permittedVotingRange = permittedVotingRange;
+ ai.date = date;
+ ai.tag = tag;
+ return ai;
}
public ChangeJson fix(FixInput fix) {
@@ -398,10 +311,68 @@
return format(cd, Optional.empty(), true, changeInfoSupplier);
}
- private ChangeInfo format(
- ChangeData cd, Optional<PatchSet.Id> limitToPsId, boolean fillAccountLoader)
- throws OrmException {
- return format(cd, limitToPsId, fillAccountLoader, ChangeInfo::new);
+ public ChangeInfo format(RevisionResource rsrc) throws OrmException {
+ ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
+ return format(cd, Optional.of(rsrc.getPatchSet().getId()), true, ChangeInfo::new);
+ }
+
+ public List<List<ChangeInfo>> formatQueryResults(List<QueryResult<ChangeData>> in)
+ throws PermissionBackendException {
+ try (Timer0.Context ignored = metrics.formatQueryResultsLatency.start()) {
+ accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
+ List<List<ChangeInfo>> res = new ArrayList<>(in.size());
+ Map<Change.Id, ChangeInfo> cache = Maps.newHashMapWithExpectedSize(in.size());
+ for (QueryResult<ChangeData> r : in) {
+ List<ChangeInfo> infos = toChangeInfos(r.entities(), cache);
+ infos.forEach(c -> cache.put(new Change.Id(c._number), c));
+ if (!infos.isEmpty() && r.more()) {
+ infos.get(infos.size() - 1)._moreChanges = true;
+ }
+ res.add(infos);
+ }
+ accountLoader.fill();
+ return res;
+ }
+ }
+
+ public List<ChangeInfo> formatChangeDatas(Collection<ChangeData> in)
+ throws OrmException, PermissionBackendException {
+ accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
+ ensureLoaded(in);
+ List<ChangeInfo> out = new ArrayList<>(in.size());
+ for (ChangeData cd : in) {
+ out.add(format(cd, Optional.empty(), false, ChangeInfo::new));
+ }
+ accountLoader.fill();
+ return out;
+ }
+
+ private static Collection<SubmitRequirementInfo> requirementsFor(ChangeData cd) {
+ Collection<SubmitRequirementInfo> reqInfos = new ArrayList<>();
+ for (SubmitRecord submitRecord : cd.submitRecords(SUBMIT_RULE_OPTIONS_STRICT)) {
+ if (submitRecord.requirements == null) {
+ continue;
+ }
+ for (SubmitRequirement requirement : submitRecord.requirements) {
+ reqInfos.add(requirementToInfo(requirement, submitRecord.status));
+ }
+ }
+ return reqInfos;
+ }
+
+ private static SubmitRequirementInfo requirementToInfo(SubmitRequirement req, Status status) {
+ return new SubmitRequirementInfo(status.name(), req.fallbackText(), req.type(), req.data());
+ }
+
+ private static void finish(ChangeInfo info) {
+ info.id =
+ Joiner.on('~')
+ .join(Url.encode(info.project), Url.encode(info.branch), Url.encode(info.changeId));
+ }
+
+ private static boolean containsAnyOf(
+ ImmutableSet<ListChangesOption> set, ImmutableSet<ListChangesOption> toFind) {
+ return !Sets.intersection(toFind, set).isEmpty();
}
private <I extends ChangeInfo> I format(
@@ -432,59 +403,6 @@
}
}
- public ChangeInfo format(RevisionResource rsrc) throws OrmException {
- ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
- return format(cd, Optional.of(rsrc.getPatchSet().getId()), true);
- }
-
- public List<List<ChangeInfo>> formatQueryResults(List<QueryResult<ChangeData>> in)
- throws PermissionBackendException {
- try (Timer0.Context ignored = metrics.formatQueryResultsLatency.start()) {
- accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
- List<List<ChangeInfo>> res = new ArrayList<>(in.size());
- Map<Change.Id, ChangeInfo> cache = Maps.newHashMapWithExpectedSize(in.size());
- for (QueryResult<ChangeData> r : in) {
- List<ChangeInfo> infos = toChangeInfos(r.entities(), cache);
- infos.forEach(c -> cache.put(new Change.Id(c._number), c));
- if (!infos.isEmpty() && r.more()) {
- infos.get(infos.size() - 1)._moreChanges = true;
- }
- res.add(infos);
- }
- accountLoader.fill();
- return res;
- }
- }
-
- public List<ChangeInfo> formatChangeDatas(Collection<ChangeData> in)
- throws OrmException, PermissionBackendException {
- accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
- ensureLoaded(in);
- List<ChangeInfo> out = new ArrayList<>(in.size());
- for (ChangeData cd : in) {
- out.add(format(cd, Optional.empty(), false));
- }
- accountLoader.fill();
- return out;
- }
-
- private static Collection<SubmitRequirementInfo> requirementsFor(ChangeData cd) {
- Collection<SubmitRequirementInfo> reqInfos = new ArrayList<>();
- for (SubmitRecord submitRecord : cd.submitRecords(SUBMIT_RULE_OPTIONS_STRICT)) {
- if (submitRecord.requirements == null) {
- continue;
- }
- for (SubmitRequirement requirement : submitRecord.requirements) {
- reqInfos.add(requirementToInfo(requirement, submitRecord.status));
- }
- }
- return reqInfos;
- }
-
- private static SubmitRequirementInfo requirementToInfo(SubmitRequirement req, Status status) {
- return new SubmitRequirementInfo(status.name(), req.fallbackText(), req.type(), req.data());
- }
-
private void ensureLoaded(Iterable<ChangeData> all) throws OrmException {
if (lazyLoad) {
ChangeData.ensureChangeLoaded(all);
@@ -511,51 +429,21 @@
private List<ChangeInfo> toChangeInfos(
List<ChangeData> changes, Map<Change.Id, ChangeInfo> cache) {
try (Timer0.Context ignored = metrics.toChangeInfosLatency.start()) {
- // Create a list of formatting calls that can be called sequentially or in parallel
- List<Callable<Optional<ChangeInfo>>> formattingCalls = new ArrayList<>(changes.size());
+ List<ChangeInfo> changeInfos = new ArrayList<>(changes.size());
for (ChangeData cd : changes) {
- formattingCalls.add(
- () -> {
- ChangeInfo i = cache.get(cd.getId());
- if (i != null) {
- return Optional.of(i);
- }
- try {
- ensureLoaded(Collections.singleton(cd));
- return Optional.of(format(cd, Optional.empty(), false));
- } catch (OrmException | RuntimeException e) {
- logger.atWarning().withCause(e).log(
- "Omitting corrupt change %s from results", cd.getId());
- return Optional.empty();
- }
- });
- }
-
- long numProjects = changes.stream().map(ChangeData::project).distinct().count();
- if (!enableParallelFormatting || !lazyLoad || changes.size() < 3 || numProjects < 2) {
- // Format these changes in the request thread as the multithreading overhead would be too
- // high.
- List<ChangeInfo> result = new ArrayList<>(changes.size());
- for (Callable<Optional<ChangeInfo>> c : formattingCalls) {
- try {
- c.call().ifPresent(result::add);
- } catch (Exception e) {
- logger.atWarning().withCause(e).log("Omitting change due to exception");
- }
+ ChangeInfo i = cache.get(cd.getId());
+ if (i != null) {
+ continue;
}
- return result;
- }
-
- // Format the changes in parallel on the executor
- List<ChangeInfo> result = new ArrayList<>(changes.size());
- try {
- for (Future<Optional<ChangeInfo>> f : fanOutExecutor.invokeAll(formattingCalls)) {
- f.get().ifPresent(result::add);
+ try {
+ ensureLoaded(Collections.singleton(cd));
+ changeInfos.add(format(cd, Optional.empty(), false, ChangeInfo::new));
+ } catch (OrmException | RuntimeException e) {
+ logger.atWarning().withCause(e).log(
+ "Omitting corrupt change %s from results", cd.getId());
}
- } catch (InterruptedException | ExecutionException e) {
- throw new IllegalStateException(e);
}
- return result;
+ return changeInfos;
}
}
@@ -674,7 +562,7 @@
out.reviewed = cd.isReviewedBy(user.getAccountId()) ? true : null;
}
- out.labels = labelsFor(cd, has(LABELS), has(DETAILED_LABELS));
+ out.labels = labelsJson.labelsFor(accountLoader, cd, has(LABELS), has(DETAILED_LABELS));
out.requirements = requirementsFor(cd);
if (out.labels != null && has(DETAILED_LABELS)) {
@@ -684,7 +572,7 @@
&& (!limitToPsId.isPresent() || limitToPsId.get().equals(in.currentPatchSetId()))) {
out.permittedLabels =
cd.change().getStatus() != Change.Status.ABANDONED
- ? permittedLabels(user.getAccountId(), cd)
+ ? labelsJson.permittedLabels(user.getAccountId(), cd)
: ImmutableMap.of();
}
@@ -719,7 +607,7 @@
// This block must come after the ChangeInfo is mostly populated, since
// it will be passed to ActionVisitors as-is.
if (needRevisions) {
- out.revisions = revisions(cd, src, limitToPsId, out);
+ out.revisions = revisionJson.getRevisions(accountLoader, cd, src, limitToPsId, out);
if (out.revisions != null) {
for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
if (entry.getValue().isCurrent) {
@@ -780,210 +668,6 @@
return SubmitRecord.allRecordsOK(cd.submitRecords(SUBMIT_RULE_OPTIONS_STRICT));
}
- private List<SubmitRecord> submitRecords(ChangeData cd) {
- return cd.submitRecords(SUBMIT_RULE_OPTIONS_LENIENT);
- }
-
- private Map<String, LabelInfo> labelsFor(ChangeData cd, boolean standard, boolean detailed)
- throws OrmException, PermissionBackendException {
- if (!standard && !detailed) {
- return null;
- }
-
- LabelTypes labelTypes = cd.getLabelTypes();
- Map<String, LabelWithStatus> withStatus =
- cd.change().getStatus() == Change.Status.MERGED
- ? labelsForSubmittedChange(cd, labelTypes, standard, detailed)
- : labelsForUnsubmittedChange(cd, labelTypes, standard, detailed);
- return ImmutableMap.copyOf(Maps.transformValues(withStatus, LabelWithStatus::label));
- }
-
- private Map<String, LabelWithStatus> labelsForUnsubmittedChange(
- ChangeData cd, LabelTypes labelTypes, boolean standard, boolean detailed)
- throws OrmException, PermissionBackendException {
- Map<String, LabelWithStatus> labels = initLabels(cd, labelTypes, standard);
- if (detailed) {
- setAllApprovals(cd, labels);
- }
- for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
- LabelType type = labelTypes.byLabel(e.getKey());
- if (type == null) {
- continue;
- }
- if (standard) {
- for (PatchSetApproval psa : cd.currentApprovals()) {
- if (type.matches(psa)) {
- short val = psa.getValue();
- Account.Id accountId = psa.getAccountId();
- setLabelScores(type, e.getValue(), val, accountId);
- }
- }
- }
- if (detailed) {
- setLabelValues(type, e.getValue());
- }
- }
- return labels;
- }
-
- private Map<String, LabelWithStatus> initLabels(
- ChangeData cd, LabelTypes labelTypes, boolean standard) {
- Map<String, LabelWithStatus> labels = new TreeMap<>(labelTypes.nameComparator());
- for (SubmitRecord rec : submitRecords(cd)) {
- if (rec.labels == null) {
- continue;
- }
- for (SubmitRecord.Label r : rec.labels) {
- LabelWithStatus p = labels.get(r.label);
- if (p == null || p.status().compareTo(r.status) < 0) {
- LabelInfo n = new LabelInfo();
- if (standard) {
- switch (r.status) {
- case OK:
- n.approved = accountLoader.get(r.appliedBy);
- break;
- case REJECT:
- n.rejected = accountLoader.get(r.appliedBy);
- n.blocking = true;
- break;
- case IMPOSSIBLE:
- case MAY:
- case NEED:
- default:
- break;
- }
- }
-
- n.optional = r.status == SubmitRecord.Label.Status.MAY ? true : null;
- labels.put(r.label, LabelWithStatus.create(n, r.status));
- }
- }
- }
- return labels;
- }
-
- private void setLabelScores(
- LabelType type, LabelWithStatus l, short score, Account.Id accountId) {
- if (l.label().approved != null || l.label().rejected != null) {
- return;
- }
-
- if (type.getMin() == null || type.getMax() == null) {
- // Can't set score for unknown or misconfigured type.
- return;
- }
-
- if (score != 0) {
- if (score == type.getMin().getValue()) {
- l.label().rejected = accountLoader.get(accountId);
- } else if (score == type.getMax().getValue()) {
- l.label().approved = accountLoader.get(accountId);
- } else if (score < 0) {
- 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(ChangeData cd, Map<String, LabelWithStatus> labels)
- throws OrmException, PermissionBackendException {
- Change.Status status = cd.change().getStatus();
- checkState(
- status != Change.Status.MERGED, "should not call setAllApprovals on %s change", status);
-
- // Include a user in the output for this label if either:
- // - They are an explicit reviewer.
- // - They ever voted on this change.
- Set<Account.Id> allUsers = new HashSet<>();
- allUsers.addAll(cd.reviewers().byState(ReviewerStateInternal.REVIEWER));
- for (PatchSetApproval psa : cd.approvals().values()) {
- allUsers.add(psa.getAccountId());
- }
-
- Table<Account.Id, String, PatchSetApproval> current =
- HashBasedTable.create(allUsers.size(), cd.getLabelTypes().getLabelTypes().size());
- for (PatchSetApproval psa : cd.currentApprovals()) {
- current.put(psa.getAccountId(), psa.getLabel(), psa);
- }
-
- LabelTypes labelTypes = cd.getLabelTypes();
- for (Account.Id accountId : allUsers) {
- PermissionBackend.ForChange perm = permissionBackendForChange(accountId, cd);
- Map<String, VotingRangeInfo> pvr = getPermittedVotingRanges(permittedLabels(accountId, cd));
- for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
- LabelType lt = labelTypes.byLabel(e.getKey());
- if (lt == null) {
- // Ignore submit record for undefined label; likely the submit rule
- // author didn't intend for the label to show up in the table.
- continue;
- }
- Integer value;
- VotingRangeInfo permittedVotingRange = pvr.getOrDefault(lt.getName(), null);
- String tag = null;
- Timestamp date = null;
- PatchSetApproval psa = current.get(accountId, lt.getName());
- if (psa != null) {
- value = Integer.valueOf(psa.getValue());
- if (value == 0) {
- // This may be a dummy approval that was inserted when the reviewer
- // was added. Explicitly check whether the user can vote on this
- // label.
- value = perm.test(new LabelPermission(lt)) ? 0 : null;
- }
- tag = psa.getTag();
- date = psa.getGranted();
- if (psa.isPostSubmit()) {
- logger.atWarning().log("unexpected post-submit approval on open change: %s", psa);
- }
- } else {
- // Either the user cannot vote on this label, or they were added as a
- // reviewer but have not responded yet. Explicitly check whether the
- // user can vote on this label.
- value = perm.test(new LabelPermission(lt)) ? 0 : null;
- }
- addApproval(
- e.getValue().label(), approvalInfo(accountId, value, permittedVotingRange, tag, date));
- }
- }
- }
-
- private Map<String, VotingRangeInfo> getPermittedVotingRanges(
- Map<String, Collection<String>> permittedLabels) {
- Map<String, VotingRangeInfo> permittedVotingRanges =
- Maps.newHashMapWithExpectedSize(permittedLabels.size());
- for (String label : permittedLabels.keySet()) {
- List<Integer> permittedVotingRange =
- permittedLabels
- .get(label)
- .stream()
- .map(this::parseRangeValue)
- .filter(java.util.Objects::nonNull)
- .sorted()
- .collect(toList());
-
- if (permittedVotingRange.isEmpty()) {
- permittedVotingRanges.put(label, null);
- } else {
- int minPermittedValue = permittedVotingRange.get(0);
- int maxPermittedValue = Iterables.getLast(permittedVotingRange);
- permittedVotingRanges.put(label, new VotingRangeInfo(minPermittedValue, maxPermittedValue));
- }
- }
- return permittedVotingRanges;
- }
-
- private Integer parseRangeValue(String value) {
- if (value.startsWith("+")) {
- value = value.substring(1);
- } else if (value.startsWith(" ")) {
- value = value.trim();
- }
- return Ints.tryParse(value);
- }
-
private void setSubmitter(ChangeData cd, ChangeInfo out) throws OrmException {
Optional<PatchSetApproval> s = cd.getSubmitApproval();
if (!s.isPresent()) {
@@ -993,214 +677,6 @@
out.submitter = accountLoader.get(s.get().getAccountId());
}
- private Map<String, LabelWithStatus> labelsForSubmittedChange(
- ChangeData cd, LabelTypes labelTypes, boolean standard, boolean detailed)
- throws OrmException, PermissionBackendException {
- Set<Account.Id> allUsers = new HashSet<>();
- if (detailed) {
- // Users expect to see all reviewers on closed changes, even if they
- // didn't vote on the latest patch set. If we don't need detailed labels,
- // we aren't including 0 votes for all users below, so we can just look at
- // the latest patch set (in the next loop).
- for (PatchSetApproval psa : cd.approvals().values()) {
- allUsers.add(psa.getAccountId());
- }
- }
-
- Set<String> labelNames = new HashSet<>();
- SetMultimap<Account.Id, PatchSetApproval> current =
- MultimapBuilder.hashKeys().hashSetValues().build();
- for (PatchSetApproval a : cd.currentApprovals()) {
- allUsers.add(a.getAccountId());
- LabelType type = labelTypes.byLabel(a.getLabelId());
- if (type != null) {
- labelNames.add(type.getName());
- // Not worth the effort to distinguish between votable/non-votable for 0
- // values on closed changes, since they can't vote anyway.
- current.put(a.getAccountId(), a);
- }
- }
-
- // Since voting on merged changes is allowed all labels which apply to
- // the change must be returned. All applying labels can be retrieved from
- // the submit records, which is what initLabels does.
- // It's not possible to only compute the labels based on the approvals
- // since merged changes may not have approvals for all labels (e.g. if not
- // all labels are required for submit or if the change was auto-closed due
- // to direct push or if new labels were defined after the change was
- // merged).
- Map<String, LabelWithStatus> labels;
- labels = initLabels(cd, labelTypes, standard);
-
- // Also include all labels for which approvals exists. E.g. there can be
- // approvals for labels that are ignored by a Prolog submit rule and hence
- // it wouldn't be included in the submit records.
- for (String name : labelNames) {
- if (!labels.containsKey(name)) {
- labels.put(name, LabelWithStatus.create(new LabelInfo(), null));
- }
- }
-
- if (detailed) {
- labels
- .entrySet()
- .stream()
- .filter(e -> labelTypes.byLabel(e.getKey()) != null)
- .forEach(e -> setLabelValues(labelTypes.byLabel(e.getKey()), e.getValue()));
- }
-
- for (Account.Id accountId : allUsers) {
- Map<String, ApprovalInfo> byLabel = Maps.newHashMapWithExpectedSize(labels.size());
- Map<String, VotingRangeInfo> pvr = Collections.emptyMap();
- if (detailed) {
- pvr = getPermittedVotingRanges(permittedLabels(accountId, cd));
- for (Map.Entry<String, LabelWithStatus> entry : labels.entrySet()) {
- ApprovalInfo ai = approvalInfo(accountId, 0, null, null, null);
- byLabel.put(entry.getKey(), ai);
- addApproval(entry.getValue().label(), ai);
- }
- }
- for (PatchSetApproval psa : current.get(accountId)) {
- LabelType type = labelTypes.byLabel(psa.getLabelId());
- if (type == null) {
- continue;
- }
-
- short val = psa.getValue();
- ApprovalInfo info = byLabel.get(type.getName());
- if (info != null) {
- info.value = Integer.valueOf(val);
- info.permittedVotingRange = pvr.getOrDefault(type.getName(), null);
- info.date = psa.getGranted();
- info.tag = psa.getTag();
- if (psa.isPostSubmit()) {
- info.postSubmit = true;
- }
- }
- if (!standard) {
- continue;
- }
-
- setLabelScores(type, labels.get(type.getName()), val, accountId);
- }
- }
- return labels;
- }
-
- private ApprovalInfo approvalInfo(
- Account.Id id,
- Integer value,
- VotingRangeInfo permittedVotingRange,
- String tag,
- Timestamp date) {
- ApprovalInfo ai = getApprovalInfo(id, value, permittedVotingRange, tag, date);
- accountLoader.put(ai);
- return ai;
- }
-
- public static ApprovalInfo getApprovalInfo(
- Account.Id id,
- Integer value,
- VotingRangeInfo permittedVotingRange,
- String tag,
- Timestamp date) {
- ApprovalInfo ai = new ApprovalInfo(id.get());
- ai.value = value;
- ai.permittedVotingRange = permittedVotingRange;
- ai.date = date;
- ai.tag = tag;
- return ai;
- }
-
- private static boolean isOnlyZero(Collection<String> values) {
- return values.isEmpty() || (values.size() == 1 && values.contains(" 0"));
- }
-
- private void setLabelValues(LabelType type, LabelWithStatus l) {
- l.label().defaultValue = type.getDefaultValue();
- l.label().values = new LinkedHashMap<>();
- for (LabelValue v : type.getValues()) {
- l.label().values.put(v.formatValue(), v.getText());
- }
- if (isOnlyZero(l.label().values.keySet())) {
- l.label().values = null;
- }
- }
-
- private Map<String, Collection<String>> permittedLabels(
- Account.Id filterApprovalsBy, ChangeData cd) throws OrmException, PermissionBackendException {
- boolean isMerged = cd.change().getStatus() == Change.Status.MERGED;
- LabelTypes labelTypes = cd.getLabelTypes();
- Map<String, LabelType> toCheck = new HashMap<>();
- for (SubmitRecord rec : submitRecords(cd)) {
- if (rec.labels != null) {
- for (SubmitRecord.Label r : rec.labels) {
- LabelType type = labelTypes.byLabel(r.label);
- if (type != null && (!isMerged || type.allowPostSubmit())) {
- toCheck.put(type.getName(), type);
- }
- }
- }
- }
-
- Map<String, Short> labels = null;
- Set<LabelPermission.WithValue> can =
- permissionBackendForChange(filterApprovalsBy, cd).testLabels(toCheck.values());
- SetMultimap<String, String> permitted = LinkedHashMultimap.create();
- for (SubmitRecord rec : submitRecords(cd)) {
- if (rec.labels == null) {
- continue;
- }
- for (SubmitRecord.Label r : rec.labels) {
- LabelType type = labelTypes.byLabel(r.label);
- if (type == null || (isMerged && !type.allowPostSubmit())) {
- continue;
- }
-
- for (LabelValue v : type.getValues()) {
- boolean ok = can.contains(new LabelPermission.WithValue(type, v));
- if (isMerged) {
- if (labels == null) {
- labels = currentLabels(filterApprovalsBy, cd);
- }
- short prev = labels.getOrDefault(type.getName(), (short) 0);
- ok &= v.getValue() >= prev;
- }
- if (ok) {
- permitted.put(r.label, v.formatValue());
- }
- }
- }
- }
-
- List<String> toClear = Lists.newArrayListWithCapacity(permitted.keySet().size());
- for (Map.Entry<String, Collection<String>> e : permitted.asMap().entrySet()) {
- if (isOnlyZero(e.getValue())) {
- toClear.add(e.getKey());
- }
- }
- for (String label : toClear) {
- permitted.removeAll(label);
- }
- return permitted.asMap();
- }
-
- private Map<String, Short> currentLabels(Account.Id accountId, ChangeData cd)
- throws OrmException {
- Map<String, Short> result = new HashMap<>();
- for (PatchSetApproval psa :
- approvalsUtil.byPatchSetUser(
- db.get(),
- lazyLoad ? cd.notes() : notesFactory.createFromIndexedChange(cd.change()),
- cd.change().currentPatchSetId(),
- accountId,
- null,
- null)) {
- result.put(psa.getLabel(), psa.getValue());
- }
- return result;
- }
-
private Collection<ChangeMessageInfo> messages(ChangeData cd) throws OrmException {
List<ChangeMessage> messages = cmUtil.byChange(db.get(), cd.notes());
if (messages.isEmpty()) {
@@ -1305,47 +781,6 @@
.collect(toList());
}
- @Nullable
- private Repository openRepoIfNecessary(Project.NameKey project) throws IOException {
- if (has(ALL_COMMITS) || has(CURRENT_COMMIT) || has(COMMIT_FOOTERS)) {
- return repoManager.openRepository(project);
- }
- return null;
- }
-
- @Nullable
- private RevWalk newRevWalk(@Nullable Repository repo) {
- return repo != null ? new RevWalk(repo) : null;
- }
-
- private Map<String, RevisionInfo> revisions(
- ChangeData cd,
- Map<PatchSet.Id, PatchSet> map,
- Optional<PatchSet.Id> limitToPsId,
- ChangeInfo changeInfo)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException,
- PermissionBackendException {
- Map<String, RevisionInfo> res = new LinkedHashMap<>();
- try (Repository repo = openRepoIfNecessary(cd.project());
- RevWalk rw = newRevWalk(repo)) {
- for (PatchSet in : map.values()) {
- PatchSet.Id id = in.getId();
- boolean want;
- if (has(ALL_REVISIONS)) {
- want = true;
- } else if (limitToPsId.isPresent()) {
- want = id.equals(limitToPsId.get());
- } else {
- want = id.equals(cd.change().currentPatchSetId());
- }
- if (want) {
- res.put(in.getRevision().get(), toRevisionInfo(cd, in, repo, rw, false, changeInfo));
- }
- }
- return res;
- }
- }
-
private Map<PatchSet.Id, PatchSet> loadPatchSets(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
throws OrmException {
Collection<PatchSet> src;
@@ -1373,197 +808,11 @@
return map;
}
- public RevisionInfo getRevisionInfo(ChangeData cd, PatchSet in)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException,
- PermissionBackendException {
- accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
- try (Repository repo = openRepoIfNecessary(cd.project());
- RevWalk rw = newRevWalk(repo)) {
- RevisionInfo rev = toRevisionInfo(cd, in, repo, rw, true, null);
- accountLoader.fill();
- return rev;
- }
- }
-
- private RevisionInfo toRevisionInfo(
- ChangeData cd,
- PatchSet in,
- @Nullable Repository repo,
- @Nullable RevWalk rw,
- boolean fillCommit,
- @Nullable ChangeInfo changeInfo)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException,
- PermissionBackendException {
- Change c = cd.change();
- RevisionInfo out = new RevisionInfo();
- out.isCurrent = in.getId().equals(c.currentPatchSetId());
- out._number = in.getId().get();
- out.ref = in.getRefName();
- out.created = in.getCreatedOn();
- out.uploader = accountLoader.get(in.getUploader());
- out.fetch = makeFetchMap(cd, in);
- out.kind = changeKindCache.getChangeKind(rw, repo != null ? repo.getConfig() : null, cd, in);
- out.description = in.getDescription();
-
- boolean setCommit = has(ALL_COMMITS) || (out.isCurrent && has(CURRENT_COMMIT));
- boolean addFooters = out.isCurrent && has(COMMIT_FOOTERS);
- if (setCommit || addFooters) {
- checkState(rw != null);
- checkState(repo != null);
- Project.NameKey project = c.getProject();
- String rev = in.getRevision().get();
- RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
- rw.parseBody(commit);
- if (setCommit) {
- out.commit = toCommit(project, rw, commit, has(WEB_LINKS), fillCommit);
- }
- if (addFooters) {
- Ref ref = repo.exactRef(cd.change().getDest().get());
- RevCommit mergeTip = null;
- if (ref != null) {
- mergeTip = rw.parseCommit(ref.getObjectId());
- rw.parseBody(mergeTip);
- }
- out.commitWithFooters =
- mergeUtilFactory
- .create(projectCache.get(project))
- .createCommitMessageOnSubmit(commit, mergeTip, cd.notes(), in.getId());
- }
- }
-
- if (has(ALL_FILES) || (out.isCurrent && has(CURRENT_FILES))) {
- out.files = fileInfoJson.toFileInfoMap(c, in);
- out.files.remove(Patch.COMMIT_MSG);
- out.files.remove(Patch.MERGE_LIST);
- }
-
- if (out.isCurrent && has(CURRENT_ACTIONS) && userProvider.get().isIdentifiedUser()) {
-
- actionJson.addRevisionActions(
- changeInfo,
- out,
- new RevisionResource(changeResourceFactory.create(cd.notes(), userProvider.get()), in));
- }
-
- if (gpgApi.isEnabled() && has(PUSH_CERTIFICATES)) {
- if (in.getPushCertificate() != null) {
- out.pushCertificate =
- gpgApi.checkPushCertificate(
- in.getPushCertificate(), userFactory.create(in.getUploader()));
- } else {
- out.pushCertificate = new PushCertificateInfo();
- }
- }
-
- return out;
- }
-
- public CommitInfo toCommit(
- Project.NameKey project, RevWalk rw, RevCommit commit, boolean addLinks, boolean fillCommit)
- throws IOException {
- CommitInfo info = new CommitInfo();
- if (fillCommit) {
- info.commit = commit.name();
- }
- info.parents = new ArrayList<>(commit.getParentCount());
- info.author = toGitPerson(commit.getAuthorIdent());
- info.committer = toGitPerson(commit.getCommitterIdent());
- info.subject = commit.getShortMessage();
- info.message = commit.getFullMessage();
-
- if (addLinks) {
- List<WebLinkInfo> links = webLinks.getPatchSetLinks(project, commit.name());
- info.webLinks = links.isEmpty() ? null : links;
- }
-
- for (RevCommit parent : commit.getParents()) {
- rw.parseBody(parent);
- CommitInfo i = new CommitInfo();
- i.commit = parent.name();
- i.subject = parent.getShortMessage();
- if (addLinks) {
- List<WebLinkInfo> parentLinks = webLinks.getParentLinks(project, parent.name());
- i.webLinks = parentLinks.isEmpty() ? null : parentLinks;
- }
- info.parents.add(i);
- }
- return info;
- }
-
- private Map<String, FetchInfo> makeFetchMap(ChangeData cd, PatchSet in)
- throws PermissionBackendException, OrmException, IOException {
- Map<String, FetchInfo> r = new LinkedHashMap<>();
- for (Extension<DownloadScheme> e : downloadSchemes) {
- String schemeName = e.getExportName();
- DownloadScheme scheme = e.getProvider().get();
- if (!scheme.isEnabled()
- || (scheme.isAuthRequired() && !userProvider.get().isIdentifiedUser())) {
- continue;
- }
- if (!scheme.isAuthSupported() && !isWorldReadable(cd)) {
- continue;
- }
-
- String projectName = cd.project().get();
- String url = scheme.getUrl(projectName);
- String refName = in.getRefName();
- FetchInfo fetchInfo = new FetchInfo(url, refName);
- r.put(schemeName, fetchInfo);
-
- if (has(DOWNLOAD_COMMANDS)) {
- populateFetchMap(scheme, downloadCommands, projectName, refName, fetchInfo);
- }
- }
-
- return r;
- }
-
- public static void populateFetchMap(
- DownloadScheme scheme,
- DynamicMap<DownloadCommand> commands,
- String projectName,
- String refName,
- FetchInfo fetchInfo) {
- for (Extension<DownloadCommand> e2 : commands) {
- String commandName = e2.getExportName();
- DownloadCommand command = e2.getProvider().get();
- String c = command.getCommand(scheme, projectName, refName);
- if (c != null) {
- addCommand(fetchInfo, commandName, c);
- }
- }
- }
-
- private static void addCommand(FetchInfo fetchInfo, String commandName, String c) {
- if (fetchInfo.commands == null) {
- fetchInfo.commands = new TreeMap<>();
- }
- fetchInfo.commands.put(commandName, c);
- }
-
- static void finish(ChangeInfo info) {
- info.id =
- Joiner.on('~')
- .join(Url.encode(info.project), Url.encode(info.branch), Url.encode(info.changeId));
- }
-
- private static void addApproval(LabelInfo label, ApprovalInfo approval) {
- if (label.all == null) {
- label.all = new ArrayList<>();
- }
- label.all.add(approval);
- }
-
private PermissionBackend.ForChange permissionBackendForChange(CurrentUser user, ChangeData cd)
throws OrmException {
return permissionBackendForChange(permissionBackend.user(user).database(db), cd);
}
- private PermissionBackend.ForChange permissionBackendForChange(Account.Id user, ChangeData cd)
- throws OrmException {
- return permissionBackendForChange(permissionBackend.absentUser(user).database(db), cd);
- }
-
/**
* @return {@link com.google.gerrit.server.permissions.PermissionBackend.ForChange} constructed
* from either an index-backed or a database-backed {@link ChangeData} depending on {@code
@@ -1575,31 +824,4 @@
? withUser.change(cd)
: withUser.indexedChange(cd, notesFactory.createFromIndexedChange(cd.change()));
}
-
- private boolean isWorldReadable(ChangeData cd)
- throws OrmException, PermissionBackendException, IOException {
- try {
- permissionBackendForChange(anonymous, cd).check(ChangePermission.READ);
- } catch (AuthException ae) {
- return false;
- }
- ProjectState projectState = projectCache.checkedGet(cd.project());
- if (projectState == null) {
- logger.atSevere().log("project state for project %s is null", cd.project());
- return false;
- }
- return projectState.statePermitsRead();
- }
-
- @AutoValue
- abstract static class LabelWithStatus {
- 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();
- }
}
diff --git a/java/com/google/gerrit/server/change/DownloadCommandsJson.java b/java/com/google/gerrit/server/change/DownloadCommandsJson.java
new file mode 100644
index 0000000..f56a16c
--- /dev/null
+++ b/java/com/google/gerrit/server/change/DownloadCommandsJson.java
@@ -0,0 +1,51 @@
+// Copyright (C) 2018 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.change;
+
+import com.google.gerrit.extensions.common.FetchInfo;
+import com.google.gerrit.extensions.config.DownloadCommand;
+import com.google.gerrit.extensions.config.DownloadScheme;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.Extension;
+import java.util.TreeMap;
+
+/** Populates the {@link FetchInfo} which is serialized to JSON afterwards. */
+public class DownloadCommandsJson {
+
+ private DownloadCommandsJson() {}
+
+ /**
+ * Populates the provided {@link FetchInfo} by calling all {@link DownloadCommand} extensions.
+ * Will mutate {@link FetchInfo#commands}.
+ */
+ public static void populateFetchMap(
+ DownloadScheme scheme,
+ DynamicMap<DownloadCommand> commands,
+ String projectName,
+ String refName,
+ FetchInfo fetchInfo) {
+ for (Extension<DownloadCommand> ext : commands) {
+ String commandName = ext.getExportName();
+ DownloadCommand command = ext.getProvider().get();
+ String c = command.getCommand(scheme, projectName, refName);
+ if (c != null) {
+ if (fetchInfo.commands == null) {
+ fetchInfo.commands = new TreeMap<>();
+ }
+ fetchInfo.commands.put(commandName, c);
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/change/LabelsJson.java b/java/com/google/gerrit/server/change/LabelsJson.java
new file mode 100644
index 0000000..17e45a9
--- /dev/null
+++ b/java/com/google/gerrit/server/change/LabelsJson.java
@@ -0,0 +1,557 @@
+// Copyright (C) 2018 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.change;
+
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.stream.Collectors.toList;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.MultimapBuilder;
+import com.google.common.collect.SetMultimap;
+import com.google.common.collect.Sets;
+import com.google.common.collect.Table;
+import com.google.common.flogger.FluentLogger;
+import com.google.common.primitives.Ints;
+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;
+import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ApprovalInfo;
+import com.google.gerrit.extensions.common.LabelInfo;
+import com.google.gerrit.extensions.common.VotingRangeInfo;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Account.Id;
+import com.google.gerrit.reviewdb.client.Change;
+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.account.AccountLoader;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
+import com.google.gerrit.server.permissions.LabelPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Provider;
+import com.google.inject.assistedinject.Assisted;
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import javax.inject.Inject;
+
+/**
+ * Produces label-related entities, like {@link LabelInfo}s, which is serialized to JSON afterwards.
+ */
+public class LabelsJson {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ public interface Factory {
+ LabelsJson create(Iterable<ListChangesOption> options);
+ }
+
+ private final Provider<ReviewDb> db;
+ private final ApprovalsUtil approvalsUtil;
+ private final ChangeNotes.Factory notesFactory;
+ private final PermissionBackend permissionBackend;
+ private final boolean lazyLoad;
+
+ @Inject
+ LabelsJson(
+ Provider<ReviewDb> db,
+ ApprovalsUtil approvalsUtil,
+ ChangeNotes.Factory notesFactory,
+ PermissionBackend permissionBackend,
+ @Assisted Iterable<ListChangesOption> options) {
+ this.db = db;
+ this.approvalsUtil = approvalsUtil;
+ this.notesFactory = notesFactory;
+ this.permissionBackend = permissionBackend;
+ this.lazyLoad = containsAnyOf(Sets.immutableEnumSet(options), ChangeJson.REQUIRE_LAZY_LOAD);
+ }
+
+ /**
+ * Returns all {@link LabelInfo}s for a single change. Uses the provided {@link AccountLoader} to
+ * lazily populate accounts. Callers have to call {@link AccountLoader#fill()} afterwards to
+ * populate all accounts in the returned {@link LabelInfo}s.
+ */
+ Map<String, LabelInfo> labelsFor(
+ AccountLoader accountLoader, ChangeData cd, boolean standard, boolean detailed)
+ throws OrmException, PermissionBackendException {
+ if (!standard && !detailed) {
+ return null;
+ }
+
+ LabelTypes labelTypes = cd.getLabelTypes();
+ Map<String, LabelWithStatus> withStatus =
+ cd.change().getStatus() == Change.Status.MERGED
+ ? labelsForSubmittedChange(accountLoader, cd, labelTypes, standard, detailed)
+ : labelsForUnsubmittedChange(accountLoader, cd, labelTypes, standard, detailed);
+ return ImmutableMap.copyOf(Maps.transformValues(withStatus, LabelWithStatus::label));
+ }
+
+ /** Returns all labels that the provided user has permission to vote on. */
+ Map<String, Collection<String>> permittedLabels(Account.Id filterApprovalsBy, ChangeData cd)
+ throws OrmException, PermissionBackendException {
+ boolean isMerged = cd.change().getStatus() == Change.Status.MERGED;
+ LabelTypes labelTypes = cd.getLabelTypes();
+ Map<String, LabelType> toCheck = new HashMap<>();
+ for (SubmitRecord rec : submitRecords(cd)) {
+ if (rec.labels != null) {
+ for (SubmitRecord.Label r : rec.labels) {
+ LabelType type = labelTypes.byLabel(r.label);
+ if (type != null && (!isMerged || type.allowPostSubmit())) {
+ toCheck.put(type.getName(), type);
+ }
+ }
+ }
+ }
+
+ Map<String, Short> labels = null;
+ Set<LabelPermission.WithValue> can =
+ permissionBackendForChange(filterApprovalsBy, cd).testLabels(toCheck.values());
+ SetMultimap<String, String> permitted = LinkedHashMultimap.create();
+ for (SubmitRecord rec : submitRecords(cd)) {
+ if (rec.labels == null) {
+ continue;
+ }
+ for (SubmitRecord.Label r : rec.labels) {
+ LabelType type = labelTypes.byLabel(r.label);
+ if (type == null || (isMerged && !type.allowPostSubmit())) {
+ continue;
+ }
+
+ for (LabelValue v : type.getValues()) {
+ boolean ok = can.contains(new LabelPermission.WithValue(type, v));
+ if (isMerged) {
+ if (labels == null) {
+ labels = currentLabels(filterApprovalsBy, cd);
+ }
+ short prev = labels.getOrDefault(type.getName(), (short) 0);
+ ok &= v.getValue() >= prev;
+ }
+ if (ok) {
+ permitted.put(r.label, v.formatValue());
+ }
+ }
+ }
+ }
+
+ List<String> toClear = Lists.newArrayListWithCapacity(permitted.keySet().size());
+ for (Map.Entry<String, Collection<String>> e : permitted.asMap().entrySet()) {
+ if (isOnlyZero(e.getValue())) {
+ toClear.add(e.getKey());
+ }
+ }
+ for (String label : toClear) {
+ permitted.removeAll(label);
+ }
+ return permitted.asMap();
+ }
+
+ private static boolean containsAnyOf(
+ ImmutableSet<ListChangesOption> set, ImmutableSet<ListChangesOption> toFind) {
+ return !Sets.intersection(toFind, set).isEmpty();
+ }
+
+ private static boolean isOnlyZero(Collection<String> values) {
+ return values.isEmpty() || (values.size() == 1 && values.contains(" 0"));
+ }
+
+ private static void addApproval(LabelInfo label, ApprovalInfo approval) {
+ if (label.all == null) {
+ label.all = new ArrayList<>();
+ }
+ label.all.add(approval);
+ }
+
+ private Map<String, LabelWithStatus> labelsForUnsubmittedChange(
+ AccountLoader accountLoader,
+ ChangeData cd,
+ LabelTypes labelTypes,
+ boolean standard,
+ boolean detailed)
+ throws OrmException, PermissionBackendException {
+ Map<String, LabelWithStatus> labels = initLabels(accountLoader, cd, labelTypes, standard);
+ if (detailed) {
+ setAllApprovals(accountLoader, cd, labels);
+ }
+ for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
+ LabelType type = labelTypes.byLabel(e.getKey());
+ if (type == null) {
+ continue;
+ }
+ if (standard) {
+ for (PatchSetApproval psa : cd.currentApprovals()) {
+ if (type.matches(psa)) {
+ short val = psa.getValue();
+ Account.Id accountId = psa.getAccountId();
+ setLabelScores(accountLoader, type, e.getValue(), val, accountId);
+ }
+ }
+ }
+ if (detailed) {
+ setLabelValues(type, e.getValue());
+ }
+ }
+ return labels;
+ }
+
+ private Integer parseRangeValue(String value) {
+ if (value.startsWith("+")) {
+ value = value.substring(1);
+ } else if (value.startsWith(" ")) {
+ value = value.trim();
+ }
+ return Ints.tryParse(value);
+ }
+
+ private ApprovalInfo approvalInfo(
+ AccountLoader accountLoader,
+ Account.Id id,
+ Integer value,
+ VotingRangeInfo permittedVotingRange,
+ String tag,
+ Timestamp date) {
+ ApprovalInfo ai = ChangeJson.getApprovalInfo(id, value, permittedVotingRange, tag, date);
+ accountLoader.put(ai);
+ return ai;
+ }
+
+ private void setLabelValues(LabelType type, LabelWithStatus l) {
+ l.label().defaultValue = type.getDefaultValue();
+ l.label().values = new LinkedHashMap<>();
+ for (LabelValue v : type.getValues()) {
+ l.label().values.put(v.formatValue(), v.getText());
+ }
+ if (isOnlyZero(l.label().values.keySet())) {
+ l.label().values = null;
+ }
+ }
+
+ private Map<String, Short> currentLabels(Account.Id accountId, ChangeData cd)
+ throws OrmException {
+ Map<String, Short> result = new HashMap<>();
+ for (PatchSetApproval psa :
+ approvalsUtil.byPatchSetUser(
+ db.get(),
+ lazyLoad ? cd.notes() : notesFactory.createFromIndexedChange(cd.change()),
+ cd.change().currentPatchSetId(),
+ accountId,
+ null,
+ null)) {
+ result.put(psa.getLabel(), psa.getValue());
+ }
+ return result;
+ }
+
+ private Map<String, LabelWithStatus> labelsForSubmittedChange(
+ AccountLoader accountLoader,
+ ChangeData cd,
+ LabelTypes labelTypes,
+ boolean standard,
+ boolean detailed)
+ throws OrmException, PermissionBackendException {
+ Set<Account.Id> allUsers = new HashSet<>();
+ if (detailed) {
+ // Users expect to see all reviewers on closed changes, even if they
+ // didn't vote on the latest patch set. If we don't need detailed labels,
+ // we aren't including 0 votes for all users below, so we can just look at
+ // the latest patch set (in the next loop).
+ for (PatchSetApproval psa : cd.approvals().values()) {
+ allUsers.add(psa.getAccountId());
+ }
+ }
+
+ Set<String> labelNames = new HashSet<>();
+ SetMultimap<Id, PatchSetApproval> current = MultimapBuilder.hashKeys().hashSetValues().build();
+ for (PatchSetApproval a : cd.currentApprovals()) {
+ allUsers.add(a.getAccountId());
+ LabelType type = labelTypes.byLabel(a.getLabelId());
+ if (type != null) {
+ labelNames.add(type.getName());
+ // Not worth the effort to distinguish between votable/non-votable for 0
+ // values on closed changes, since they can't vote anyway.
+ current.put(a.getAccountId(), a);
+ }
+ }
+
+ // Since voting on merged changes is allowed all labels which apply to
+ // the change must be returned. All applying labels can be retrieved from
+ // the submit records, which is what initLabels does.
+ // It's not possible to only compute the labels based on the approvals
+ // since merged changes may not have approvals for all labels (e.g. if not
+ // all labels are required for submit or if the change was auto-closed due
+ // to direct push or if new labels were defined after the change was
+ // merged).
+ Map<String, LabelWithStatus> labels;
+ labels = initLabels(accountLoader, cd, labelTypes, standard);
+
+ // Also include all labels for which approvals exists. E.g. there can be
+ // approvals for labels that are ignored by a Prolog submit rule and hence
+ // it wouldn't be included in the submit records.
+ for (String name : labelNames) {
+ if (!labels.containsKey(name)) {
+ labels.put(name, LabelWithStatus.create(new LabelInfo(), null));
+ }
+ }
+
+ if (detailed) {
+ labels
+ .entrySet()
+ .stream()
+ .filter(e -> labelTypes.byLabel(e.getKey()) != null)
+ .forEach(e -> setLabelValues(labelTypes.byLabel(e.getKey()), e.getValue()));
+ }
+
+ for (Account.Id accountId : allUsers) {
+ Map<String, ApprovalInfo> byLabel = Maps.newHashMapWithExpectedSize(labels.size());
+ Map<String, VotingRangeInfo> pvr = Collections.emptyMap();
+ if (detailed) {
+ pvr = getPermittedVotingRanges(permittedLabels(accountId, cd));
+ for (Map.Entry<String, LabelWithStatus> entry : labels.entrySet()) {
+ ApprovalInfo ai = approvalInfo(accountLoader, accountId, 0, null, null, null);
+ byLabel.put(entry.getKey(), ai);
+ addApproval(entry.getValue().label(), ai);
+ }
+ }
+ for (PatchSetApproval psa : current.get(accountId)) {
+ LabelType type = labelTypes.byLabel(psa.getLabelId());
+ if (type == null) {
+ continue;
+ }
+
+ short val = psa.getValue();
+ ApprovalInfo info = byLabel.get(type.getName());
+ if (info != null) {
+ info.value = Integer.valueOf(val);
+ info.permittedVotingRange = pvr.getOrDefault(type.getName(), null);
+ info.date = psa.getGranted();
+ info.tag = psa.getTag();
+ if (psa.isPostSubmit()) {
+ info.postSubmit = true;
+ }
+ }
+ if (!standard) {
+ continue;
+ }
+
+ setLabelScores(accountLoader, type, labels.get(type.getName()), val, accountId);
+ }
+ }
+ return labels;
+ }
+
+ private Map<String, LabelWithStatus> initLabels(
+ AccountLoader accountLoader, ChangeData cd, LabelTypes labelTypes, boolean standard) {
+ Map<String, LabelWithStatus> labels = new TreeMap<>(labelTypes.nameComparator());
+ for (SubmitRecord rec : submitRecords(cd)) {
+ if (rec.labels == null) {
+ continue;
+ }
+ for (SubmitRecord.Label r : rec.labels) {
+ LabelWithStatus p = labels.get(r.label);
+ if (p == null || p.status().compareTo(r.status) < 0) {
+ LabelInfo n = new LabelInfo();
+ if (standard) {
+ switch (r.status) {
+ case OK:
+ n.approved = accountLoader.get(r.appliedBy);
+ break;
+ case REJECT:
+ n.rejected = accountLoader.get(r.appliedBy);
+ n.blocking = true;
+ break;
+ case IMPOSSIBLE:
+ case MAY:
+ case NEED:
+ default:
+ break;
+ }
+ }
+
+ n.optional = r.status == SubmitRecord.Label.Status.MAY ? true : null;
+ labels.put(r.label, LabelWithStatus.create(n, r.status));
+ }
+ }
+ }
+ return labels;
+ }
+
+ private void setLabelScores(
+ AccountLoader accountLoader,
+ LabelType type,
+ LabelWithStatus l,
+ short score,
+ Account.Id accountId) {
+ if (l.label().approved != null || l.label().rejected != null) {
+ return;
+ }
+
+ if (type.getMin() == null || type.getMax() == null) {
+ // Can't set score for unknown or misconfigured type.
+ return;
+ }
+
+ if (score != 0) {
+ if (score == type.getMin().getValue()) {
+ l.label().rejected = accountLoader.get(accountId);
+ } else if (score == type.getMax().getValue()) {
+ l.label().approved = accountLoader.get(accountId);
+ } else if (score < 0) {
+ 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(
+ AccountLoader accountLoader, ChangeData cd, Map<String, LabelWithStatus> labels)
+ throws OrmException, PermissionBackendException {
+ Change.Status status = cd.change().getStatus();
+ checkState(
+ status != Change.Status.MERGED, "should not call setAllApprovals on %s change", status);
+
+ // Include a user in the output for this label if either:
+ // - They are an explicit reviewer.
+ // - They ever voted on this change.
+ Set<Id> allUsers = new HashSet<>();
+ allUsers.addAll(cd.reviewers().byState(ReviewerStateInternal.REVIEWER));
+ for (PatchSetApproval psa : cd.approvals().values()) {
+ allUsers.add(psa.getAccountId());
+ }
+
+ Table<Id, String, PatchSetApproval> current =
+ HashBasedTable.create(allUsers.size(), cd.getLabelTypes().getLabelTypes().size());
+ for (PatchSetApproval psa : cd.currentApprovals()) {
+ current.put(psa.getAccountId(), psa.getLabel(), psa);
+ }
+
+ LabelTypes labelTypes = cd.getLabelTypes();
+ for (Account.Id accountId : allUsers) {
+ PermissionBackend.ForChange perm = permissionBackendForChange(accountId, cd);
+ Map<String, VotingRangeInfo> pvr = getPermittedVotingRanges(permittedLabels(accountId, cd));
+ for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
+ LabelType lt = labelTypes.byLabel(e.getKey());
+ if (lt == null) {
+ // Ignore submit record for undefined label; likely the submit rule
+ // author didn't intend for the label to show up in the table.
+ continue;
+ }
+ Integer value;
+ VotingRangeInfo permittedVotingRange = pvr.getOrDefault(lt.getName(), null);
+ String tag = null;
+ Timestamp date = null;
+ PatchSetApproval psa = current.get(accountId, lt.getName());
+ if (psa != null) {
+ value = Integer.valueOf(psa.getValue());
+ if (value == 0) {
+ // This may be a dummy approval that was inserted when the reviewer
+ // was added. Explicitly check whether the user can vote on this
+ // label.
+ value = perm.test(new LabelPermission(lt)) ? 0 : null;
+ }
+ tag = psa.getTag();
+ date = psa.getGranted();
+ if (psa.isPostSubmit()) {
+ logger.atWarning().log("unexpected post-submit approval on open change: %s", psa);
+ }
+ } else {
+ // Either the user cannot vote on this label, or they were added as a
+ // reviewer but have not responded yet. Explicitly check whether the
+ // user can vote on this label.
+ value = perm.test(new LabelPermission(lt)) ? 0 : null;
+ }
+ addApproval(
+ e.getValue().label(),
+ approvalInfo(accountLoader, accountId, value, permittedVotingRange, tag, date));
+ }
+ }
+ }
+
+ /**
+ * @return {@link com.google.gerrit.server.permissions.PermissionBackend.ForChange} constructed
+ * from either an index-backed or a database-backed {@link ChangeData} depending on {@code
+ * lazyload}.
+ */
+ private PermissionBackend.ForChange permissionBackendForChange(Account.Id user, ChangeData cd)
+ throws OrmException {
+ PermissionBackend.WithUser withUser = permissionBackend.absentUser(user).database(db);
+ return lazyLoad
+ ? withUser.change(cd)
+ : withUser.indexedChange(cd, notesFactory.createFromIndexedChange(cd.change()));
+ }
+
+ private List<SubmitRecord> submitRecords(ChangeData cd) {
+ return cd.submitRecords(ChangeJson.SUBMIT_RULE_OPTIONS_LENIENT);
+ }
+
+ private Map<String, VotingRangeInfo> getPermittedVotingRanges(
+ Map<String, Collection<String>> permittedLabels) {
+ Map<String, VotingRangeInfo> permittedVotingRanges =
+ Maps.newHashMapWithExpectedSize(permittedLabels.size());
+ for (String label : permittedLabels.keySet()) {
+ List<Integer> permittedVotingRange =
+ permittedLabels
+ .get(label)
+ .stream()
+ .map(this::parseRangeValue)
+ .filter(java.util.Objects::nonNull)
+ .sorted()
+ .collect(toList());
+
+ if (permittedVotingRange.isEmpty()) {
+ permittedVotingRanges.put(label, null);
+ } else {
+ int minPermittedValue = permittedVotingRange.get(0);
+ int maxPermittedValue = Iterables.getLast(permittedVotingRange);
+ permittedVotingRanges.put(label, new VotingRangeInfo(minPermittedValue, maxPermittedValue));
+ }
+ }
+ return permittedVotingRanges;
+ }
+
+ @AutoValue
+ abstract static class LabelWithStatus {
+ private static LabelWithStatus create(LabelInfo label, SubmitRecord.Label.Status status) {
+ return new AutoValue_LabelsJson_LabelWithStatus(label, status);
+ }
+
+ abstract LabelInfo label();
+
+ @Nullable
+ abstract SubmitRecord.Label.Status status();
+ }
+}
diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
new file mode 100644
index 0000000..b67028d
--- /dev/null
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -0,0 +1,393 @@
+// Copyright (C) 2018 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.change;
+
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.extensions.client.ListChangesOption.ALL_COMMITS;
+import static com.google.gerrit.extensions.client.ListChangesOption.ALL_FILES;
+import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
+import static com.google.gerrit.extensions.client.ListChangesOption.COMMIT_FOOTERS;
+import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_ACTIONS;
+import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
+import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_FILES;
+import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_ACCOUNTS;
+import static com.google.gerrit.extensions.client.ListChangesOption.DOWNLOAD_COMMANDS;
+import static com.google.gerrit.extensions.client.ListChangesOption.PUSH_CERTIFICATES;
+import static com.google.gerrit.extensions.client.ListChangesOption.WEB_LINKS;
+import static com.google.gerrit.server.CommonConverters.toGitPerson;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.CommitInfo;
+import com.google.gerrit.extensions.common.FetchInfo;
+import com.google.gerrit.extensions.common.PushCertificateInfo;
+import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.common.WebLinkInfo;
+import com.google.gerrit.extensions.config.DownloadCommand;
+import com.google.gerrit.extensions.config.DownloadScheme;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.Extension;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Patch;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSet.Id;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.GpgException;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.WebLinks;
+import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.account.GpgApiAdapter;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
+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.assistedinject.Assisted;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/** Produces {@link RevisionInfo} and {@link CommitInfo} which are serialized to JSON afterwards. */
+public class RevisionJson {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ public interface Factory {
+ RevisionJson create(Iterable<ListChangesOption> options);
+ }
+
+ private final MergeUtil.Factory mergeUtilFactory;
+ private final IdentifiedUser.GenericFactory userFactory;
+ private final FileInfoJson fileInfoJson;
+ private final GpgApiAdapter gpgApi;
+ private final ChangeResource.Factory changeResourceFactory;
+ private final ChangeKindCache changeKindCache;
+ private final ActionJson actionJson;
+ private final DynamicMap<DownloadScheme> downloadSchemes;
+ private final DynamicMap<DownloadCommand> downloadCommands;
+ private final WebLinks webLinks;
+ private final Provider<CurrentUser> userProvider;
+ private final ProjectCache projectCache;
+ private final ImmutableSet<ListChangesOption> options;
+ private final AccountLoader.Factory accountLoaderFactory;
+ private final AnonymousUser anonymous;
+ private final GitRepositoryManager repoManager;
+ private final PermissionBackend permissionBackend;
+ private final ChangeNotes.Factory notesFactory;
+ private final boolean lazyLoad;
+
+ @Inject
+ RevisionJson(
+ Provider<CurrentUser> userProvider,
+ AnonymousUser anonymous,
+ ProjectCache projectCache,
+ IdentifiedUser.GenericFactory userFactory,
+ MergeUtil.Factory mergeUtilFactory,
+ FileInfoJson fileInfoJson,
+ AccountLoader.Factory accountLoaderFactory,
+ DynamicMap<DownloadScheme> downloadSchemes,
+ DynamicMap<DownloadCommand> downloadCommands,
+ WebLinks webLinks,
+ ActionJson actionJson,
+ GpgApiAdapter gpgApi,
+ ChangeResource.Factory changeResourceFactory,
+ ChangeKindCache changeKindCache,
+ GitRepositoryManager repoManager,
+ PermissionBackend permissionBackend,
+ ChangeNotes.Factory notesFactory,
+ @Assisted Iterable<ListChangesOption> options) {
+ this.userProvider = userProvider;
+ this.anonymous = anonymous;
+ this.projectCache = projectCache;
+ this.userFactory = userFactory;
+ this.mergeUtilFactory = mergeUtilFactory;
+ this.fileInfoJson = fileInfoJson;
+ this.accountLoaderFactory = accountLoaderFactory;
+ this.downloadSchemes = downloadSchemes;
+ this.downloadCommands = downloadCommands;
+ this.webLinks = webLinks;
+ this.actionJson = actionJson;
+ this.gpgApi = gpgApi;
+ this.changeResourceFactory = changeResourceFactory;
+ this.changeKindCache = changeKindCache;
+ this.permissionBackend = permissionBackend;
+ this.notesFactory = notesFactory;
+ this.repoManager = repoManager;
+ this.options = ImmutableSet.copyOf(options);
+ this.lazyLoad = containsAnyOf(this.options, ChangeJson.REQUIRE_LAZY_LOAD);
+ }
+
+ /**
+ * Returns a {@link RevisionInfo} based on a change and patch set. Reads from the repository
+ * depending on the options provided when constructing this instance.
+ */
+ public RevisionInfo getRevisionInfo(ChangeData cd, PatchSet in)
+ throws PatchListNotAvailableException, GpgException, OrmException, IOException,
+ PermissionBackendException {
+ AccountLoader accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
+ try (Repository repo = openRepoIfNecessary(cd.project());
+ RevWalk rw = newRevWalk(repo)) {
+ RevisionInfo rev = toRevisionInfo(accountLoader, cd, in, repo, rw, true, null);
+ accountLoader.fill();
+ return rev;
+ }
+ }
+
+ /**
+ * Returns a {@link CommitInfo} based on a commit and formatting options. Uses the provided
+ * RevWalk and assumes it is backed by an open repository.
+ */
+ public CommitInfo getCommitInfo(
+ Project.NameKey project, RevWalk rw, RevCommit commit, boolean addLinks, boolean fillCommit)
+ throws IOException {
+ CommitInfo info = new CommitInfo();
+ if (fillCommit) {
+ info.commit = commit.name();
+ }
+ info.parents = new ArrayList<>(commit.getParentCount());
+ info.author = toGitPerson(commit.getAuthorIdent());
+ info.committer = toGitPerson(commit.getCommitterIdent());
+ info.subject = commit.getShortMessage();
+ info.message = commit.getFullMessage();
+
+ if (addLinks) {
+ List<WebLinkInfo> links = webLinks.getPatchSetLinks(project, commit.name());
+ info.webLinks = links.isEmpty() ? null : links;
+ }
+
+ for (RevCommit parent : commit.getParents()) {
+ rw.parseBody(parent);
+ CommitInfo i = new CommitInfo();
+ i.commit = parent.name();
+ i.subject = parent.getShortMessage();
+ if (addLinks) {
+ List<WebLinkInfo> parentLinks = webLinks.getParentLinks(project, parent.name());
+ i.webLinks = parentLinks.isEmpty() ? null : parentLinks;
+ }
+ info.parents.add(i);
+ }
+ return info;
+ }
+
+ /**
+ * Returns multiple {@link RevisionInfo}s for a single change. Uses the provided {@link
+ * AccountLoader} to lazily populate accounts. Callers have to call {@link AccountLoader#fill()}
+ * afterwards to populate all accounts in the returned {@link RevisionInfo}s.
+ */
+ Map<String, RevisionInfo> getRevisions(
+ AccountLoader accountLoader,
+ ChangeData cd,
+ Map<PatchSet.Id, PatchSet> map,
+ Optional<Id> limitToPsId,
+ ChangeInfo changeInfo)
+ throws PatchListNotAvailableException, GpgException, OrmException, IOException,
+ PermissionBackendException {
+ Map<String, RevisionInfo> res = new LinkedHashMap<>();
+ try (Repository repo = openRepoIfNecessary(cd.project());
+ RevWalk rw = newRevWalk(repo)) {
+ for (PatchSet in : map.values()) {
+ PatchSet.Id id = in.getId();
+ boolean want;
+ if (has(ALL_REVISIONS)) {
+ want = true;
+ } else if (limitToPsId.isPresent()) {
+ want = id.equals(limitToPsId.get());
+ } else {
+ want = id.equals(cd.change().currentPatchSetId());
+ }
+ if (want) {
+ res.put(
+ in.getRevision().get(),
+ toRevisionInfo(accountLoader, cd, in, repo, rw, false, changeInfo));
+ }
+ }
+ return res;
+ }
+ }
+
+ private Map<String, FetchInfo> makeFetchMap(ChangeData cd, PatchSet in)
+ throws PermissionBackendException, OrmException, IOException {
+ Map<String, FetchInfo> r = new LinkedHashMap<>();
+ for (Extension<DownloadScheme> e : downloadSchemes) {
+ String schemeName = e.getExportName();
+ DownloadScheme scheme = e.getProvider().get();
+ if (!scheme.isEnabled()
+ || (scheme.isAuthRequired() && !userProvider.get().isIdentifiedUser())) {
+ continue;
+ }
+ if (!scheme.isAuthSupported() && !isWorldReadable(cd)) {
+ continue;
+ }
+
+ String projectName = cd.project().get();
+ String url = scheme.getUrl(projectName);
+ String refName = in.getRefName();
+ FetchInfo fetchInfo = new FetchInfo(url, refName);
+ r.put(schemeName, fetchInfo);
+
+ if (has(DOWNLOAD_COMMANDS)) {
+ DownloadCommandsJson.populateFetchMap(
+ scheme, downloadCommands, projectName, refName, fetchInfo);
+ }
+ }
+
+ return r;
+ }
+
+ private RevisionInfo toRevisionInfo(
+ AccountLoader accountLoader,
+ ChangeData cd,
+ PatchSet in,
+ @Nullable Repository repo,
+ @Nullable RevWalk rw,
+ boolean fillCommit,
+ @Nullable ChangeInfo changeInfo)
+ throws PatchListNotAvailableException, GpgException, OrmException, IOException,
+ PermissionBackendException {
+ Change c = cd.change();
+ RevisionInfo out = new RevisionInfo();
+ out.isCurrent = in.getId().equals(c.currentPatchSetId());
+ out._number = in.getId().get();
+ out.ref = in.getRefName();
+ out.created = in.getCreatedOn();
+ out.uploader = accountLoader.get(in.getUploader());
+ out.fetch = makeFetchMap(cd, in);
+ out.kind = changeKindCache.getChangeKind(rw, repo != null ? repo.getConfig() : null, cd, in);
+ out.description = in.getDescription();
+
+ boolean setCommit = has(ALL_COMMITS) || (out.isCurrent && has(CURRENT_COMMIT));
+ boolean addFooters = out.isCurrent && has(COMMIT_FOOTERS);
+ if (setCommit || addFooters) {
+ checkState(rw != null);
+ checkState(repo != null);
+ Project.NameKey project = c.getProject();
+ String rev = in.getRevision().get();
+ RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
+ rw.parseBody(commit);
+ if (setCommit) {
+ out.commit = getCommitInfo(project, rw, commit, has(WEB_LINKS), fillCommit);
+ }
+ if (addFooters) {
+ Ref ref = repo.exactRef(cd.change().getDest().get());
+ RevCommit mergeTip = null;
+ if (ref != null) {
+ mergeTip = rw.parseCommit(ref.getObjectId());
+ rw.parseBody(mergeTip);
+ }
+ out.commitWithFooters =
+ mergeUtilFactory
+ .create(projectCache.get(project))
+ .createCommitMessageOnSubmit(commit, mergeTip, cd.notes(), in.getId());
+ }
+ }
+
+ if (has(ALL_FILES) || (out.isCurrent && has(CURRENT_FILES))) {
+ out.files = fileInfoJson.toFileInfoMap(c, in);
+ out.files.remove(Patch.COMMIT_MSG);
+ out.files.remove(Patch.MERGE_LIST);
+ }
+
+ if (out.isCurrent && has(CURRENT_ACTIONS) && userProvider.get().isIdentifiedUser()) {
+ actionJson.addRevisionActions(
+ changeInfo,
+ out,
+ new RevisionResource(changeResourceFactory.create(cd.notes(), userProvider.get()), in));
+ }
+
+ if (gpgApi.isEnabled() && has(PUSH_CERTIFICATES)) {
+ if (in.getPushCertificate() != null) {
+ out.pushCertificate =
+ gpgApi.checkPushCertificate(
+ in.getPushCertificate(), userFactory.create(in.getUploader()));
+ } else {
+ out.pushCertificate = new PushCertificateInfo();
+ }
+ }
+
+ return out;
+ }
+
+ private boolean has(ListChangesOption option) {
+ return options.contains(option);
+ }
+
+ /**
+ * @return {@link com.google.gerrit.server.permissions.PermissionBackend.ForChange} constructed
+ * from either an index-backed or a database-backed {@link ChangeData} depending on {@code
+ * lazyload}.
+ */
+ private PermissionBackend.ForChange permissionBackendForChange(
+ PermissionBackend.WithUser withUser, ChangeData cd) throws OrmException {
+ return lazyLoad
+ ? withUser.change(cd)
+ : withUser.indexedChange(cd, notesFactory.createFromIndexedChange(cd.change()));
+ }
+
+ private boolean isWorldReadable(ChangeData cd)
+ throws OrmException, PermissionBackendException, IOException {
+ try {
+ permissionBackendForChange(permissionBackend.user(anonymous), cd)
+ .check(ChangePermission.READ);
+ } catch (AuthException ae) {
+ return false;
+ }
+ ProjectState projectState = projectCache.checkedGet(cd.project());
+ if (projectState == null) {
+ logger.atSevere().log("project state for project %s is null", cd.project());
+ return false;
+ }
+ return projectState.statePermitsRead();
+ }
+
+ @Nullable
+ private Repository openRepoIfNecessary(Project.NameKey project) throws IOException {
+ if (has(ALL_COMMITS) || has(CURRENT_COMMIT) || has(COMMIT_FOOTERS)) {
+ return repoManager.openRepository(project);
+ }
+ return null;
+ }
+
+ @Nullable
+ private RevWalk newRevWalk(@Nullable Repository repo) {
+ return repo != null ? new RevWalk(repo) : null;
+ }
+
+ private static boolean containsAnyOf(
+ ImmutableSet<ListChangesOption> set, ImmutableSet<ListChangesOption> toFind) {
+ return !Sets.intersection(toFind, set).isEmpty();
+ }
+}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 367fdb6..b4f9cc7 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -101,8 +101,10 @@
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeKindCacheImpl;
+import com.google.gerrit.server.change.LabelsJson;
import com.google.gerrit.server.change.MergeabilityCacheImpl;
import com.google.gerrit.server.change.ReviewerSuggestion;
+import com.google.gerrit.server.change.RevisionJson;
import com.google.gerrit.server.events.EventFactory;
import com.google.gerrit.server.events.EventListener;
import com.google.gerrit.server.events.EventsMetrics;
@@ -260,12 +262,14 @@
factory(ChangeData.AssistedFactory.class);
factory(ChangeJson.AssistedFactory.class);
factory(CreateChangeSender.Factory.class);
+ factory(LabelsJson.Factory.class);
factory(MergedSender.Factory.class);
factory(MergeUtil.Factory.class);
factory(PatchScriptFactory.Factory.class);
factory(ProjectState.Factory.class);
factory(RegisterNewEmailSender.Factory.class);
factory(ReplacePatchSetSender.Factory.class);
+ factory(RevisionJson.Factory.class);
factory(SetAssigneeSender.Factory.class);
factory(InboundEmailRejectionSender.Factory.class);
bind(PermissionCollection.Factory.class);
diff --git a/java/com/google/gerrit/server/edit/ChangeEditJson.java b/java/com/google/gerrit/server/edit/ChangeEditJson.java
index bd9c3a6..bf20404 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditJson.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditJson.java
@@ -23,7 +23,7 @@
import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.DownloadCommandsJson;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -97,7 +97,8 @@
FetchInfo fetchInfo = new FetchInfo(scheme.getUrl(projectName), refName);
r.put(schemeName, fetchInfo);
- ChangeJson.populateFetchMap(scheme, downloadCommands, projectName, refName, fetchInfo);
+ DownloadCommandsJson.populateFetchMap(
+ scheme, downloadCommands, projectName, refName, fetchInfo);
}
return r;
diff --git a/java/com/google/gerrit/server/extensions/events/EventUtil.java b/java/com/google/gerrit/server/extensions/events/EventUtil.java
index 3a10fc5..79876df 100644
--- a/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.RevisionJson;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
@@ -64,15 +65,18 @@
private final ChangeData.Factory changeDataFactory;
private final Provider<ReviewDb> db;
private final ChangeJson.Factory changeJsonFactory;
+ private final RevisionJson.Factory revisionJsonFactory;
@Inject
EventUtil(
ChangeJson.Factory changeJsonFactory,
+ RevisionJson.Factory revisionJsonFactory,
ChangeData.Factory changeDataFactory,
Provider<ReviewDb> db) {
this.changeDataFactory = changeDataFactory;
this.db = db;
this.changeJsonFactory = changeJsonFactory;
+ this.revisionJsonFactory = revisionJsonFactory;
}
public ChangeInfo changeInfo(Change change) throws OrmException {
@@ -89,7 +93,7 @@
throws OrmException, PatchListNotAvailableException, GpgException, IOException,
PermissionBackendException {
ChangeData cd = changeDataFactory.create(db.get(), project, ps.getId().getParentKey());
- return changeJsonFactory.create(CHANGE_OPTIONS).getRevisionInfo(cd, ps);
+ return revisionJsonFactory.create(CHANGE_OPTIONS).getRevisionInfo(cd, ps);
}
public AccountInfo accountInfo(AccountState accountState) {
diff --git a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index ab2203e..9fa1edc 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -230,8 +230,9 @@
String newNoteDbStateStr = change.getNoteDbState();
if (newNoteDbStateStr == null) {
throw new OrmException(
- "Rebuilding change %s produced no writes to NoteDb: "
- + bundleReader.fromReviewDb(db, changeId));
+ String.format(
+ "Rebuilding change %s produced no writes to NoteDb: %s",
+ changeId, bundleReader.fromReviewDb(db, changeId)));
}
NoteDbChangeState newNoteDbState =
checkNotNull(NoteDbChangeState.parse(changeId, newNoteDbStateStr));
@@ -290,8 +291,7 @@
// Can only rebuild a change if its primary storage is ReviewDb.
NoteDbChangeState s = NoteDbChangeState.parse(c);
if (s != null && s.getPrimaryStorage() != PrimaryStorage.REVIEW_DB) {
- throw new OrmException(
- String.format("cannot rebuild change " + c.getId() + " with state " + s));
+ throw new OrmException(String.format("cannot rebuild change %s with state %s", c.getId(), s));
}
return c;
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetCommit.java b/java/com/google/gerrit/server/restapi/change/GetCommit.java
index 645d7d1..29286cb 100644
--- a/java/com/google/gerrit/server/restapi/change/GetCommit.java
+++ b/java/com/google/gerrit/server/restapi/change/GetCommit.java
@@ -14,12 +14,13 @@
package com.google.gerrit.server.restapi.change;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.common.CommitInfo;
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.reviewdb.client.Project;
-import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.RevisionJson;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
@@ -33,12 +34,12 @@
public class GetCommit implements RestReadView<RevisionResource> {
private final GitRepositoryManager repoManager;
- private final ChangeJson.Factory json;
+ private final RevisionJson.Factory json;
private boolean addLinks;
@Inject
- GetCommit(GitRepositoryManager repoManager, ChangeJson.Factory json) {
+ GetCommit(GitRepositoryManager repoManager, RevisionJson.Factory json) {
this.repoManager = repoManager;
this.json = json;
}
@@ -57,7 +58,9 @@
String rev = rsrc.getPatchSet().getRevision().get();
RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
rw.parseBody(commit);
- CommitInfo info = json.noOptions().toCommit(rsrc.getProject(), rw, commit, addLinks, true);
+ CommitInfo info =
+ json.create(ImmutableSet.of())
+ .getCommitInfo(rsrc.getProject(), rw, commit, addLinks, true);
Response<CommitInfo> r = Response.ok(info);
if (rsrc.isCacheable()) {
r.caching(CacheControl.PRIVATE(7, TimeUnit.DAYS));
diff --git a/java/com/google/gerrit/server/restapi/change/GetMergeList.java b/java/com/google/gerrit/server/restapi/change/GetMergeList.java
index 2f3b536..8e7e693 100644
--- a/java/com/google/gerrit/server/restapi/change/GetMergeList.java
+++ b/java/com/google/gerrit/server/restapi/change/GetMergeList.java
@@ -15,13 +15,14 @@
package com.google.gerrit.server.restapi.change;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
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.reviewdb.client.Project;
-import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.RevisionJson;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.MergeListBuilder;
@@ -38,7 +39,7 @@
public class GetMergeList implements RestReadView<RevisionResource> {
private final GitRepositoryManager repoManager;
- private final ChangeJson.Factory json;
+ private final RevisionJson.Factory json;
@Option(name = "--parent", usage = "Uninteresting parent (1-based, default = 1)")
private int uninterestingParent = 1;
@@ -47,7 +48,7 @@
private boolean addLinks;
@Inject
- GetMergeList(GitRepositoryManager repoManager, ChangeJson.Factory json) {
+ GetMergeList(GitRepositoryManager repoManager, RevisionJson.Factory json) {
this.repoManager = repoManager;
this.json = json;
}
@@ -80,9 +81,9 @@
List<RevCommit> commits = MergeListBuilder.build(rw, commit, uninterestingParent);
List<CommitInfo> result = new ArrayList<>(commits.size());
- ChangeJson changeJson = json.noOptions();
+ RevisionJson changeJson = json.create(ImmutableSet.of());
for (RevCommit c : commits) {
- result.add(changeJson.toCommit(rsrc.getProject(), rw, c, addLinks, true));
+ result.add(changeJson.getCommitInfo(rsrc.getProject(), rw, c, addLinks, true));
}
return createResponse(rsrc, result);
}
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index b53c4e6..6610ebb 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -14,9 +14,7 @@
package com.google.gerrit.server.restapi.change;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -136,10 +134,7 @@
ChangeJson cjson = json.create(options);
cjson.setPluginDefinedAttributesFactory(this.imp);
- List<List<ChangeInfo>> res =
- cjson
- .lazyLoad(containsAnyOf(options, ChangeJson.REQUIRE_LAZY_LOAD))
- .formatQueryResults(results);
+ List<List<ChangeInfo>> res = cjson.formatQueryResults(results);
for (int n = 0; n < cnt; n++) {
List<ChangeInfo> info = res.get(n);
@@ -149,9 +144,4 @@
}
return res;
}
-
- private static boolean containsAnyOf(
- EnumSet<ListChangesOption> set, ImmutableSet<ListChangesOption> toFind) {
- return !Sets.intersection(toFind, set).isEmpty();
- }
}
diff --git a/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java b/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
index 4ced4c2..4ca986b 100644
--- a/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
+++ b/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
@@ -67,14 +67,11 @@
private final Provider<MergeSuperSet> mergeSuperSet;
private final Provider<WalkSorter> sorter;
- private boolean lazyLoad = false;
-
@Option(name = "-o", usage = "Output options")
void addOption(String option) {
for (ListChangesOption o : ListChangesOption.values()) {
if (o.name().equalsIgnoreCase(option)) {
jsonOpt.add(o);
- lazyLoad |= ChangeJson.REQUIRE_LAZY_LOAD.contains(o);
return;
}
}
@@ -150,7 +147,7 @@
cds = sort(cds, hidden);
SubmittedTogetherInfo info = new SubmittedTogetherInfo();
- info.changes = json.create(jsonOpt).lazyLoad(lazyLoad).formatChangeDatas(cds);
+ info.changes = json.create(jsonOpt).formatChangeDatas(cds);
info.nonVisibleChanges = hidden;
return info;
} catch (OrmException | IOException e) {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitmodulesIT.java b/javatests/com/google/gerrit/acceptance/git/GitmodulesIT.java
similarity index 100%
rename from gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/GitmodulesIT.java
rename to javatests/com/google/gerrit/acceptance/git/GitmodulesIT.java
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 0661466..0f394aa7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -35,7 +35,7 @@
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.RevisionJson;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
@@ -54,7 +54,7 @@
return submitWholeTopicEnabledConfig();
}
- @Inject private ChangeJson.Factory changeJsonFactory;
+ @Inject private RevisionJson.Factory revisionJsonFactory;
@Inject private DynamicSet<ActionVisitor> actionVisitors;
@@ -394,7 +394,7 @@
// ...via ChangeJson directly.
ChangeData cd = changeDataFactory.create(db, project, changeId);
- changeJsonFactory.create(opts).getRevisionInfo(cd, cd.patchSet(new PatchSet.Id(changeId, 1)));
+ revisionJsonFactory.create(opts).getRevisionInfo(cd, cd.patchSet(new PatchSet.Id(changeId, 1)));
}
private void visitedCurrentRevisionActionsAssertions(
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 543ae1f..4cea41a 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 543ae1f0a24dda61d4f36b173b9bddfd52ead3b9
+Subproject commit 4cea41a51ff7ce98bc18e4c01577f99a93931562
