Merge "Extract logic from GetRelated to another class"
diff --git a/java/com/google/gerrit/server/restapi/change/GetRelated.java b/java/com/google/gerrit/server/restapi/change/GetRelated.java
index ba1a1dc..57291e8e 100644
--- a/java/com/google/gerrit/server/restapi/change/GetRelated.java
+++ b/java/com/google/gerrit/server/restapi/change/GetRelated.java
@@ -14,10 +14,6 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.common.base.Preconditions.checkArgument;
-import static java.util.stream.Collectors.toSet;
-
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
@@ -29,56 +25,37 @@
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommonConverters;
-import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
-import java.util.Set;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.revwalk.RevCommit;
@Singleton
public class GetRelated implements RestReadView<RevisionResource> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final Provider<InternalChangeQuery> queryProvider;
- private final PatchSetUtil psUtil;
- private final RelatedChangesSorter sorter;
- private final IndexConfig indexConfig;
private final ChangeData.Factory changeDataFactory;
+ private final GetRelatedUtil getRelatedUtil;
@Inject
- GetRelated(
- Provider<InternalChangeQuery> queryProvider,
- PatchSetUtil psUtil,
- RelatedChangesSorter sorter,
- IndexConfig indexConfig,
- ChangeData.Factory changeDataFactory) {
- this.queryProvider = queryProvider;
- this.psUtil = psUtil;
- this.sorter = sorter;
- this.indexConfig = indexConfig;
+ GetRelated(ChangeData.Factory changeDataFactory, GetRelatedUtil getRelatedUtil) {
this.changeDataFactory = changeDataFactory;
+ this.getRelatedUtil = getRelatedUtil;
}
@Override
public Response<RelatedChangesInfo> apply(RevisionResource rsrc)
- throws RepositoryNotFoundException, IOException, NoSuchProjectException,
- PermissionBackendException {
+ throws IOException, NoSuchProjectException, PermissionBackendException {
RelatedChangesInfo relatedChangesInfo = new RelatedChangesInfo();
relatedChangesInfo.changes = getRelated(rsrc);
return Response.ok(relatedChangesInfo);
@@ -86,30 +63,15 @@
public List<RelatedChangeAndCommitInfo> getRelated(RevisionResource rsrc)
throws IOException, PermissionBackendException {
- Set<String> groups = getAllGroups(rsrc.getNotes(), psUtil);
- logger.atFine().log("groups = %s", groups);
- if (groups.isEmpty()) {
- return Collections.emptyList();
- }
-
- List<ChangeData> cds =
- InternalChangeQuery.byProjectGroups(
- queryProvider, indexConfig, rsrc.getChange().getProject(), groups);
- if (cds.isEmpty()) {
- return Collections.emptyList();
- }
- if (cds.size() == 1 && cds.get(0).getId().equals(rsrc.getChange().getId())) {
- return Collections.emptyList();
- }
- List<RelatedChangeAndCommitInfo> result = new ArrayList<>(cds.size());
-
boolean isEdit = rsrc.getEdit().isPresent();
PatchSet basePs = isEdit ? rsrc.getEdit().get().getBasePatchSet() : rsrc.getPatchSet();
logger.atFine().log("isEdit = %s, basePs = %s", isEdit, basePs);
- cds = reloadChangeIfStale(cds, rsrc.getChange(), basePs);
+ List<RelatedChangesSorter.PatchSetData> sortedResult =
+ getRelatedUtil.getRelated(changeDataFactory.create(rsrc.getNotes()), basePs);
- for (RelatedChangesSorter.PatchSetData d : sorter.sort(cds, basePs)) {
+ List<RelatedChangeAndCommitInfo> result = new ArrayList<>(sortedResult.size());
+ for (RelatedChangesSorter.PatchSetData d : sortedResult) {
PatchSet ps = d.patchSet();
RevCommit commit;
if (isEdit && ps.id().equals(basePs.id())) {
@@ -134,37 +96,6 @@
return result;
}
- @VisibleForTesting
- public static Set<String> getAllGroups(ChangeNotes notes, PatchSetUtil psUtil) {
- return psUtil.byChange(notes).stream().flatMap(ps -> ps.groups().stream()).collect(toSet());
- }
-
- private List<ChangeData> reloadChangeIfStale(
- List<ChangeData> changeDatasFromIndex, Change wantedChange, PatchSet wantedPs) {
- checkArgument(
- wantedChange.getId().equals(wantedPs.id().changeId()),
- "change of wantedPs (%s) doesn't match wantedChange (%s)",
- wantedPs.id().changeId(),
- wantedChange.getId());
-
- List<ChangeData> changeDatas = new ArrayList<>(changeDatasFromIndex.size() + 1);
- changeDatas.addAll(changeDatasFromIndex);
-
- // Reload the change in case the patch set is absent.
- changeDatas.stream()
- .filter(
- cd -> cd.getId().equals(wantedPs.id().changeId()) && cd.patchSet(wantedPs.id()) == null)
- .forEach(ChangeData::reloadChange);
-
- if (changeDatas.stream().noneMatch(cd -> cd.getId().equals(wantedPs.id().changeId()))) {
- // The change of the wanted patch set is missing in the result from the index.
- // Load it from NoteDb and add it to the result.
- changeDatas.add(changeDataFactory.create(wantedChange));
- }
-
- return changeDatas;
- }
-
static RelatedChangeAndCommitInfo newChangeAndCommit(
Project.NameKey project, @Nullable Change change, @Nullable PatchSet ps, RevCommit c) {
RelatedChangeAndCommitInfo info = new RelatedChangeAndCommitInfo();
diff --git a/java/com/google/gerrit/server/restapi/change/GetRelatedUtil.java b/java/com/google/gerrit/server/restapi/change/GetRelatedUtil.java
new file mode 100644
index 0000000..c08e3ed
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/GetRelatedUtil.java
@@ -0,0 +1,119 @@
+// Copyright (C) 2021 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.restapi.change;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.stream.Collectors.toSet;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.index.IndexConfig;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+/** Utility class that gets the ancestor changes and the descendent changes of a specific change. */
+@Singleton
+public class GetRelatedUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final Provider<InternalChangeQuery> queryProvider;
+ private final RelatedChangesSorter sorter;
+ private final IndexConfig indexConfig;
+ private final ChangeData.Factory changeDataFactory;
+
+ @Inject
+ GetRelatedUtil(
+ Provider<InternalChangeQuery> queryProvider,
+ RelatedChangesSorter sorter,
+ IndexConfig indexConfig,
+ ChangeData.Factory changeDataFactory) {
+ this.queryProvider = queryProvider;
+ this.sorter = sorter;
+ this.indexConfig = indexConfig;
+ this.changeDataFactory = changeDataFactory;
+ }
+
+ /**
+ * Gets related changes of a specific change revision.
+ *
+ * @param changeData the change of the inputted revision.
+ * @param basePs the revision that the method checks for related changes.
+ * @return list of related changes, sorted via {@link RelatedChangesSorter}
+ */
+ public List<RelatedChangesSorter.PatchSetData> getRelated(ChangeData changeData, PatchSet basePs)
+ throws IOException, PermissionBackendException {
+ Set<String> groups = getAllGroups(changeData.patchSets());
+ logger.atFine().log("groups = %s", groups);
+ if (groups.isEmpty()) {
+ return Collections.emptyList();
+ }
+
+ List<ChangeData> cds =
+ InternalChangeQuery.byProjectGroups(
+ queryProvider, indexConfig, changeData.project(), groups);
+ if (cds.isEmpty()) {
+ return Collections.emptyList();
+ }
+ if (cds.size() == 1 && cds.get(0).getId().equals(changeData.getId())) {
+ return Collections.emptyList();
+ }
+
+ cds = reloadChangeIfStale(cds, changeData, basePs);
+
+ return sorter.sort(cds, basePs);
+ }
+
+ private List<ChangeData> reloadChangeIfStale(
+ List<ChangeData> changeDatasFromIndex, ChangeData wantedChange, PatchSet wantedPs) {
+ checkArgument(
+ wantedChange.getId().equals(wantedPs.id().changeId()),
+ "change of wantedPs (%s) doesn't match wantedChange (%s)",
+ wantedPs.id().changeId(),
+ wantedChange.getId());
+
+ List<ChangeData> changeDatas = new ArrayList<>(changeDatasFromIndex.size() + 1);
+ changeDatas.addAll(changeDatasFromIndex);
+
+ // Reload the change in case the patch set is absent.
+ changeDatas.stream()
+ .filter(
+ cd -> cd.getId().equals(wantedPs.id().changeId()) && cd.patchSet(wantedPs.id()) == null)
+ .forEach(ChangeData::reloadChange);
+
+ if (changeDatas.stream().noneMatch(cd -> cd.getId().equals(wantedPs.id().changeId()))) {
+ // The change of the wanted patch set is missing in the result from the index.
+ // Load it from NoteDb and add it to the result.
+ changeDatas.add(changeDataFactory.create(wantedChange.change()));
+ }
+
+ return changeDatas;
+ }
+
+ @VisibleForTesting
+ public static Set<String> getAllGroups(Collection<PatchSet> patchSets) {
+ return patchSets.stream().flatMap(ps -> ps.groups().stream()).collect(toSet());
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
index 74dfa04..b605594 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
@@ -47,7 +47,7 @@
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.restapi.change.ChangesCollection;
-import com.google.gerrit.server.restapi.change.GetRelated;
+import com.google.gerrit.server.restapi.change.GetRelatedUtil;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -573,7 +573,7 @@
ChangeData cd = getChange(last);
assertThat(cd.patchSets()).hasSize(n);
- assertThat(GetRelated.getAllGroups(cd.notes(), psUtil)).hasSize(n);
+ assertThat(GetRelatedUtil.getAllGroups(cd.notes().getPatchSets().values())).hasSize(n);
assertRelated(cd.change().currentPatchSetId());
}
