Merge changes I01e2cfcc,Ib2a38f56,I2dcc82ac,I7300ba58
* changes:
Update rules_nodejs to 5.8.2
Bump version in package.json
Update @bazel/* npm packages to ^5.8.0
Update eslint-plugin-jsdoc to 44.2.4
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 728606d..b94e840 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -113,7 +113,7 @@
private static final String CHANGE_FIELD = ChangeField.CHANGE_SPEC.getName();
static Term idTerm(ChangeData cd) {
- return idTerm(cd.getId());
+ return idTerm(cd.getVirtualId());
}
static Term idTerm(Change.Id id) {
diff --git a/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java b/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java
index 9a75469..834a623 100644
--- a/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java
+++ b/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java
@@ -99,8 +99,8 @@
}
List<ChangeData> cds =
- InternalChangeQuery.byBranchGroups(
- queryProvider, indexConfig, changeData.change().getDest(), groups);
+ InternalChangeQuery.byProjectGroups(
+ queryProvider, indexConfig, changeData.project(), groups);
if (cds.isEmpty()) {
return Collections.emptyList();
}
diff --git a/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java b/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java
index f3f7645..2a74833 100644
--- a/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java
+++ b/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java
@@ -14,24 +14,24 @@
package com.google.gerrit.server.config;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.lib.Config;
-public class GerritImportedServerIdsProvider implements Provider<ImmutableSet<String>> {
+public class GerritImportedServerIdsProvider implements Provider<ImmutableList<String>> {
public static final String SECTION = "gerrit";
public static final String KEY = "importedServerId";
- private final ImmutableSet<String> importedIds;
+ private final ImmutableList<String> importedIds;
@Inject
public GerritImportedServerIdsProvider(@GerritServerConfig Config cfg) {
- importedIds = ImmutableSet.copyOf(cfg.getStringList(SECTION, null, KEY));
+ importedIds = ImmutableList.copyOf(cfg.getStringList(SECTION, null, KEY));
}
@Override
- public ImmutableSet<String> get() {
+ public ImmutableList<String> get() {
return importedIds;
}
}
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index 14aadf47..678f4d0 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -294,8 +294,8 @@
// Find changes in the same related group as this patch set, having a patch
// set whose parent matches this patch set's revision.
for (ChangeData cd :
- InternalChangeQuery.byBranchGroups(
- queryProvider, indexConfig, change.getDest(), currentPs.groups())) {
+ InternalChangeQuery.byProjectGroups(
+ queryProvider, indexConfig, change.getProject(), currentPs.groups())) {
PATCH_SETS:
for (PatchSet ps : cd.patchSets()) {
RevCommit commit = rw.parseCommit(ps.commitId());
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 1d5818a..7057ff7 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -128,7 +128,7 @@
.required()
// The numeric change id is integer in string form
.size(10)
- .build(cd -> String.valueOf(cd.getId().get()));
+ .build(cd -> String.valueOf(cd.getVirtualId().get()));
public static final IndexedField<ChangeData, String>.SearchSpec NUMERIC_ID_STR_SPEC =
NUMERIC_ID_STR_FIELD.exact("legacy_id_str");
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index 2edea26..5ffb5fb 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -17,7 +17,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Change;
@@ -53,7 +53,7 @@
public final AllUsersName allUsers;
public final NoteDbMetrics metrics;
public final String serverId;
- public final ImmutableSet<String> importedServerIds;
+ public final ImmutableList<String> importedServerIds;
// Providers required to avoid dependency cycles.
@@ -68,7 +68,7 @@
NoteDbMetrics metrics,
Provider<ChangeNotesCache> cache,
@GerritServerId String serverId,
- @GerritImportedServerIds ImmutableSet<String> importedServerIds) {
+ @GerritImportedServerIds ImmutableList<String> importedServerIds) {
this.failOnLoadForTest = new AtomicBoolean();
this.repoManager = repoManager;
this.allUsers = allUsers;
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 01c2708..80c8085 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -74,6 +74,7 @@
import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.change.PureRevert;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtilFactory;
@@ -262,15 +263,65 @@
* <p>Attempting to lazy load data will fail with NPEs. Callers may consider manually setting
* fields that can be set.
*
+ * @param project project name
* @param id change ID
+ * @param currentPatchSetId current patchset number
+ * @param commitId commit SHA1 of the current patchset
* @return instance for testing.
*/
public static ChangeData createForTest(
Project.NameKey project, Change.Id id, int currentPatchSetId, ObjectId commitId) {
+ return createForTest(project, id, currentPatchSetId, commitId, null, null, null);
+ }
+
+ /**
+ * Create an instance for testing only.
+ *
+ * <p>Attempting to lazy load data will fail with NPEs. Callers may consider manually setting
+ * fields that can be set.
+ *
+ * @param project project name
+ * @param id change ID
+ * @param currentPatchSetId current patchset number
+ * @param commitId commit SHA1 of the current patchset
+ * @param serverId Gerrit server id
+ * @param virtualIdAlgo algorithm for virtualising the Change number
+ * @param changeNotes notes associated with the Change
+ * @return instance for testing.
+ */
+ public static ChangeData createForTest(
+ Project.NameKey project,
+ Change.Id id,
+ int currentPatchSetId,
+ ObjectId commitId,
+ @Nullable String serverId,
+ @Nullable ChangeNumberVirtualIdAlgorithm virtualIdAlgo,
+ @Nullable ChangeNotes changeNotes) {
ChangeData cd =
new ChangeData(
- null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, project, id, null, null);
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ serverId,
+ virtualIdAlgo,
+ project,
+ id,
+ null,
+ changeNotes);
cd.currentPatchSet =
PatchSet.builder()
.id(PatchSet.id(id, currentPatchSetId))
@@ -363,6 +414,9 @@
private Optional<Instant> mergedOn;
private ImmutableSetMultimap<NameKey, RefState> refStates;
private ImmutableList<byte[]> refStatePatterns;
+ private String gerritServerId;
+ private String changeServerId;
+ private ChangeNumberVirtualIdAlgorithm virtualIdFunc;
@Inject
private ChangeData(
@@ -383,6 +437,8 @@
SubmitRequirementsEvaluator submitRequirementsEvaluator,
SubmitRequirementsUtil submitRequirementsUtil,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
+ @GerritServerId String gerritServerId,
+ ChangeNumberVirtualIdAlgorithm virtualIdFunc,
@Assisted Project.NameKey project,
@Assisted Change.Id id,
@Assisted @Nullable Change change,
@@ -410,6 +466,10 @@
this.change = change;
this.notes = notes;
+
+ this.changeServerId = notes == null ? null : notes.getServerId();
+ this.gerritServerId = gerritServerId;
+ this.virtualIdFunc = virtualIdFunc;
}
/**
@@ -530,6 +590,14 @@
return legacyId;
}
+ public Change.Id getVirtualId() {
+ if (virtualIdFunc == null || changeServerId == null || changeServerId.equals(gerritServerId)) {
+ return legacyId;
+ }
+
+ return Change.id(virtualIdFunc.apply(changeServerId, legacyId.get()));
+ }
+
public Project.NameKey project() {
return project;
}
@@ -560,6 +628,7 @@
throw new StorageException("Unable to load change " + legacyId, e);
}
change = notes.getChange();
+ changeServerId = notes.getServerId();
setPatchSets(null);
return change;
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java b/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java
new file mode 100644
index 0000000..726a376
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2022 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.query.change;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.server.config.GerritImportedServerIds;
+import com.google.inject.Inject;
+import com.google.inject.ProvisionException;
+import com.google.inject.Singleton;
+
+/**
+ * Dictionary-based encoding algorithm for combining a serverId/legacyChangeNum into a virtual
+ * numeric id
+ *
+ * <p>TODO: To be reverted on master and stable-3.8
+ */
+@Singleton
+public class ChangeNumberBitmapMaskAlgorithm implements ChangeNumberVirtualIdAlgorithm {
+ /*
+ * Bit-wise masks for representing the Change's VirtualId as combination of ServerId + ChangeNum:
+ */
+ private static final int CHANGE_NUM_BIT_LEN = 28; // Allows up to 268M changes
+ private static final int LEGACY_ID_BIT_MASK = (1 << CHANGE_NUM_BIT_LEN) - 1;
+ private static final int SERVER_ID_BIT_LEN =
+ Integer.BYTES * 8 - CHANGE_NUM_BIT_LEN; // Allows up to 64 ServerIds
+
+ private final ImmutableMap<String, Integer> serverIdCodes;
+
+ @Inject
+ public ChangeNumberBitmapMaskAlgorithm(
+ @GerritImportedServerIds ImmutableList<String> importedServerIds) {
+ if (importedServerIds.size() >= 1 << SERVER_ID_BIT_LEN) {
+ throw new ProvisionException(
+ String.format(
+ "Too many imported GerritServerIds (%d) to fit into the Change virtual id",
+ importedServerIds.size()));
+ }
+ ImmutableMap.Builder<String, Integer> serverIdCodesBuilder = new ImmutableMap.Builder<>();
+ for (int i = 0; i < importedServerIds.size(); i++) {
+ serverIdCodesBuilder.put(importedServerIds.get(i), i + 1);
+ }
+
+ serverIdCodes = serverIdCodesBuilder.build();
+ }
+
+ @Override
+ public int apply(String changeServerId, int changeNum) {
+ if ((changeNum & LEGACY_ID_BIT_MASK) != changeNum) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Change number %d is too large to be converted into a virtual id", changeNum));
+ }
+
+ Integer encodedServerId = serverIdCodes.get(changeServerId);
+ if (encodedServerId == null) {
+ throw new IllegalArgumentException(
+ String.format("ServerId %s is not part of the GerritImportedServerIds", changeServerId));
+ }
+ int virtualId = (changeNum & LEGACY_ID_BIT_MASK) | (encodedServerId << CHANGE_NUM_BIT_LEN);
+
+ return virtualId;
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java b/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java
new file mode 100644
index 0000000..ab21705
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java
@@ -0,0 +1,35 @@
+// Copyright (C) 2022 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.query.change;
+
+import com.google.inject.ImplementedBy;
+
+/**
+ * Algorithm for encoding a serverId/legacyChangeNum into a virtual numeric id
+ *
+ * <p>TODO: To be reverted on master and stable-3.8
+ */
+@ImplementedBy(ChangeNumberBitmapMaskAlgorithm.class)
+public interface ChangeNumberVirtualIdAlgorithm {
+
+ /**
+ * Convert a serverId/legacyChangeNum tuple into a virtual numeric id
+ *
+ * @param serverId Gerrit serverId
+ * @param legacyChangeNum legacy change number
+ * @return virtual id which combines serverId and legacyChangeNum together
+ */
+ int apply(String serverId, int legacyChangeNum);
+}
diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 3edad69..62c070c 100644
--- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -264,21 +264,21 @@
return query(ChangePredicates.submissionId(cs));
}
- private static Predicate<ChangeData> byBranchGroupsPredicate(
- IndexConfig indexConfig, BranchNameKey branchAndProject, Collection<String> groups) {
- int n = indexConfig.maxTerms() - 2;
+ private static Predicate<ChangeData> byProjectGroupsPredicate(
+ IndexConfig indexConfig, Project.NameKey project, Collection<String> groups) {
+ int n = indexConfig.maxTerms() - 1;
checkArgument(groups.size() <= n, "cannot exceed %s groups", n);
List<GroupPredicate> groupPredicates = new ArrayList<>(groups.size());
for (String g : groups) {
groupPredicates.add(new GroupPredicate(g));
}
- return and(project(branchAndProject.project()), ref(branchAndProject), or(groupPredicates));
+ return and(project(project), or(groupPredicates));
}
- public static ImmutableList<ChangeData> byBranchGroups(
+ public static ImmutableList<ChangeData> byProjectGroups(
Provider<InternalChangeQuery> queryProvider,
IndexConfig indexConfig,
- BranchNameKey branchAndProject,
+ Project.NameKey project,
Collection<String> groups) {
// These queries may be complex along multiple dimensions:
// * Many groups per change, if there are very many patch sets. This requires partitioning the
@@ -289,17 +289,16 @@
// InternalChangeQuery is single-use.
Supplier<InternalChangeQuery> querySupplier = () -> queryProvider.get().enforceVisibility(true);
- int batchSize = indexConfig.maxTerms() - 2;
+ int batchSize = indexConfig.maxTerms() - 1;
if (groups.size() <= batchSize) {
return queryExhaustively(
- querySupplier, byBranchGroupsPredicate(indexConfig, branchAndProject, groups));
+ querySupplier, byProjectGroupsPredicate(indexConfig, project, groups));
}
Set<Change.Id> seen = new HashSet<>();
ImmutableList.Builder<ChangeData> result = ImmutableList.builder();
for (List<String> part : Iterables.partition(groups, batchSize)) {
for (ChangeData cd :
- queryExhaustively(
- querySupplier, byBranchGroupsPredicate(indexConfig, branchAndProject, part))) {
+ queryExhaustively(querySupplier, byProjectGroupsPredicate(indexConfig, project, part))) {
if (!seen.add(cd.getId())) {
result.add(cd);
}
diff --git a/java/com/google/gerrit/server/schema/SchemaModule.java b/java/com/google/gerrit/server/schema/SchemaModule.java
index e0e64a3..9593522 100644
--- a/java/com/google/gerrit/server/schema/SchemaModule.java
+++ b/java/com/google/gerrit/server/schema/SchemaModule.java
@@ -16,7 +16,7 @@
import static com.google.inject.Scopes.SINGLETON;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -55,7 +55,7 @@
.toProvider(GerritServerIdProvider.class)
.in(SINGLETON);
- bind(new TypeLiteral<ImmutableSet<String>>() {})
+ bind(new TypeLiteral<ImmutableList<String>>() {})
.annotatedWith(GerritImportedServerIds.class)
.toProvider(GerritImportedServerIdsProvider.class)
.in(SINGLETON);
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index e86fd09..52ac58a 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -19,7 +19,7 @@
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.group.GroupOperationsImpl;
@@ -324,9 +324,9 @@
@Provides
@Singleton
@GerritImportedServerIds
- public ImmutableSet<String> createImportedServerIds() {
- ImmutableSet<String> serverIds =
- ImmutableSet.copyOf(
+ public ImmutableList<String> createImportedServerIds() {
+ ImmutableList<String> serverIds =
+ ImmutableList.copyOf(
cfg.getStringList(
GerritServerIdProvider.SECTION, null, GerritImportedServerIdsProvider.KEY));
return serverIds;
diff --git a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
index 3e03b2a..e15ed08 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
@@ -40,13 +40,11 @@
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
-import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.GetRelatedOption;
import com.google.gerrit.extensions.api.changes.RelatedChangeAndCommitInfo;
import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.index.IndexConfig;
@@ -71,8 +69,6 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.transport.PushResult;
-import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.junit.Test;
@NoHttpd
@@ -670,71 +666,6 @@
}
}
- @Test
- public void getRelatedLinearSameCommitPushedTwice() throws Exception {
- RevCommit base = projectOperations.project(project).getHead("master");
-
- // 1,1---2,1 on master
- PushOneCommit.Result r1 =
- createChange(
- testRepo,
- "master",
- "subject: 1",
- "a.txt",
- "1",
- /** topic= */
- null);
- RevCommit c1_1 = r1.getCommit();
- PatchSet.Id ps1_1 = r1.getPatchSetId();
-
- PushOneCommit.Result r2 =
- createChange(
- testRepo,
- "master",
- "subject: 2",
- "b.txt",
- "2",
- /** topic= */
- null);
- RevCommit c2_1 = r2.getCommit();
- PatchSet.Id ps2_1 = r2.getPatchSetId();
-
- // 3,1---4,1 on stable
- gApi.projects().name(project.get()).branch("stable").create(new BranchInput());
- testRepo.reset(c1_1);
- PushResult r3 = pushHead(testRepo, "refs/for/stable%base=" + base.getName());
- assertThat(r3.getRemoteUpdate("refs/for/stable%base=" + base.getName()).getStatus())
- .isEqualTo(RemoteRefUpdate.Status.OK);
- ChangeData change3 =
- Iterables.getOnlyElement(
- queryProvider
- .get()
- .byBranchCommit(BranchNameKey.create(project, "stable"), c1_1.getName()));
- assertThat(change3.currentPatchSet().commitId()).isEqualTo(c1_1);
- RevCommit c3_1 = c1_1;
- PatchSet.Id ps3_1 = change3.currentPatchSet().id();
-
- PushOneCommit.Result r4 =
- createChange(
- testRepo,
- "stable",
- "subject: 4",
- "d.txt",
- "4",
- /** topic= */
- null);
- RevCommit c4_1 = r4.getCommit();
- PatchSet.Id ps4_1 = r4.getPatchSetId();
-
- for (PatchSet.Id ps : ImmutableList.of(ps2_1, ps1_1)) {
- assertRelated(ps, changeAndCommit(ps2_1, c2_1, 1), changeAndCommit(ps1_1, c1_1, 1));
- }
-
- for (PatchSet.Id ps : ImmutableList.of(ps4_1, ps3_1)) {
- assertRelated(ps, changeAndCommit(ps4_1, c4_1, 1), changeAndCommit(ps3_1, c3_1, 1));
- }
- }
-
private static Correspondence<RelatedChangeAndCommitInfo, String>
getRelatedChangeToStatusCorrespondence() {
return Correspondence.transforming(
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
index 35077db..59b354c 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
@@ -157,14 +157,16 @@
@Test
public void tolerateNullValuesForInsertion() {
Project.NameKey project = Project.nameKey("project");
- ChangeData cd = ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId());
+ ChangeData cd =
+ ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId(), null, null, null);
assertThat(ChangeField.ADDED_LINES_SPEC.setIfPossible(cd, new FakeStoredValue(null))).isTrue();
}
@Test
public void tolerateNullValuesForDeletion() {
Project.NameKey project = Project.nameKey("project");
- ChangeData cd = ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId());
+ ChangeData cd =
+ ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId(), null, null, null);
assertThat(ChangeField.DELETED_LINES_SPEC.setIfPossible(cd, new FakeStoredValue(null)))
.isTrue();
}
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index bf6839d..a7ec4c6 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -19,7 +19,6 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
@@ -185,9 +184,9 @@
install(new DefaultRefLogIdentityProvider.Module());
bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
bind(String.class).annotatedWith(GerritServerId.class).toInstance(serverId);
- bind(new TypeLiteral<ImmutableSet<String>>() {})
+ bind(new TypeLiteral<ImmutableList<String>>() {})
.annotatedWith(GerritImportedServerIds.class)
- .toInstance(new ImmutableSet.Builder<String>().add(importedServerIds).build());
+ .toInstance(new ImmutableList.Builder<String>().add(importedServerIds).build());
bind(GitRepositoryManager.class).toInstance(repoManager);
bind(ProjectCache.class).to(NullProjectCache.class);
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(testConfig);
diff --git a/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java b/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
index f80a96d..0ce00eb 100644
--- a/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
+++ b/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
@@ -15,18 +15,29 @@
package com.google.gerrit.server.query.change;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.TestChanges;
+import java.util.UUID;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+@RunWith(MockitoJUnitRunner.class)
public class ChangeDataTest {
+ private static final String GERRIT_SERVER_ID = UUID.randomUUID().toString();
+
+ @Mock private ChangeNotes changeNotesMock;
+
@Test
public void setPatchSetsClearsCurrentPatchSet() throws Exception {
Project.NameKey project = Project.nameKey("project");
@@ -41,6 +52,26 @@
assertThat(curr2).isNotSameInstanceAs(curr1);
}
+ @Test
+ public void getChangeVirtualIdUsingAlgorithm() throws Exception {
+ Project.NameKey project = Project.nameKey("project");
+ final int encodedChangeNum = 12345678;
+
+ when(changeNotesMock.getServerId()).thenReturn(UUID.randomUUID().toString());
+
+ ChangeData cd =
+ ChangeData.createForTest(
+ project,
+ Change.id(1),
+ 1,
+ ObjectId.zeroId(),
+ GERRIT_SERVER_ID,
+ (s, c) -> encodedChangeNum,
+ changeNotesMock);
+
+ assertThat(cd.getVirtualId().get()).isEqualTo(encodedChangeNum);
+ }
+
private static PatchSet newPatchSet(Change.Id changeId, int num) {
return PatchSet.builder()
.id(PatchSet.id(changeId, num))
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 348ea3b..30576c8 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -74,7 +74,7 @@
import {createChangeUrl, createDiffUrl} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
import {highlightServiceToken} from '../../../services/highlight/highlight-service';
-import {waitUntil} from '../../../utils/async-util';
+import {noAwait, waitUntil} from '../../../utils/async-util';
declare global {
interface HTMLElementEventMap {
@@ -811,7 +811,7 @@
const newReply = createNewReply(replyingTo, content, unresolved);
if (userWantsToEdit) {
this.getCommentsModel().addNewDraft(newReply);
- this.editDraft();
+ noAwait(this.editDraft());
} else {
try {
this.saving = true;
diff --git a/polygerrit-ui/app/embed/diff-old/gr-diff-highlight/gr-annotation.ts b/polygerrit-ui/app/embed/diff-old/gr-diff-highlight/gr-annotation.ts
index 38bd707..5669bcf 100644
--- a/polygerrit-ui/app/embed/diff-old/gr-diff-highlight/gr-annotation.ts
+++ b/polygerrit-ui/app/embed/diff-old/gr-diff-highlight/gr-annotation.ts
@@ -19,7 +19,7 @@
*/
getLength(node: Node) {
if (node instanceof Comment) return 0;
- return this.getStringLength(node.textContent || '');
+ return GrAnnotation.getStringLength(node.textContent || '');
},
/**
@@ -65,7 +65,7 @@
const nestedNodes: Node[] = [];
for (let node of childNodes) {
- const initialNodeLength = this.getLength(node);
+ const initialNodeLength = GrAnnotation.getLength(node);
// If the current node is completely before the offset.
if (offset > 0 && initialNodeLength <= offset) {
offset -= initialNodeLength;
@@ -73,15 +73,15 @@
}
if (offset > 0) {
- node = this.splitNode(node, offset);
+ node = GrAnnotation.splitNode(node, offset);
offset = 0;
}
- if (this.getLength(node) > length) {
- this.splitNode(node, length);
+ if (GrAnnotation.getLength(node) > length) {
+ GrAnnotation.splitNode(node, length);
}
nestedNodes.push(node);
- length -= this.getLength(node);
+ length -= GrAnnotation.getLength(node);
if (!length) break;
}
@@ -116,7 +116,7 @@
let subLength;
for (const node of nodes) {
- nodeLength = this.getLength(node);
+ nodeLength = GrAnnotation.getLength(node);
// If the current node is completely before the offset.
if (nodeLength <= offset) {
@@ -128,9 +128,9 @@
subLength = Math.min(length, nodeLength - offset);
if (node instanceof Text) {
- this._annotateText(node, offset, subLength, cssClass);
+ GrAnnotation._annotateText(node, offset, subLength, cssClass);
} else if (node instanceof Element) {
- this.annotateElement(node, offset, subLength, cssClass);
+ GrAnnotation.annotateElement(node, offset, subLength, cssClass);
}
// If there is still more to annotate, then shift the indices, otherwise
@@ -172,19 +172,19 @@
firstPart?: boolean
) {
if (
- (this.getLength(node) === offset && firstPart) ||
+ (GrAnnotation.getLength(node) === offset && firstPart) ||
(offset === 0 && !firstPart)
) {
- return this.wrapInHighlight(node, cssClass);
+ return GrAnnotation.wrapInHighlight(node, cssClass);
}
if (firstPart) {
- this.splitNode(node, offset);
+ GrAnnotation.splitNode(node, offset);
// Node points to first part of the Text, second one is sibling.
} else {
// if node is Text then splitNode will return a Text
- node = this.splitNode(node, offset) as Text;
+ node = GrAnnotation.splitNode(node, offset) as Text;
}
- return this.wrapInHighlight(node, cssClass);
+ return GrAnnotation.wrapInHighlight(node, cssClass);
},
/**
@@ -193,7 +193,7 @@
*/
splitNode(element: Node, offset: number) {
if (element instanceof Text) {
- return this.splitTextNode(element, offset);
+ return GrAnnotation.splitTextNode(element, offset);
}
const tail = element.cloneNode(false);
@@ -203,13 +203,14 @@
let node = element.firstChild;
while (
node &&
- (this.getLength(node) <= offset || this.getLength(node) === 0)
+ (GrAnnotation.getLength(node) <= offset ||
+ GrAnnotation.getLength(node) === 0)
) {
- offset -= this.getLength(node);
+ offset -= GrAnnotation.getLength(node);
node = node.nextSibling;
}
- if (node && this.getLength(node) > offset) {
- tail.appendChild(this.splitNode(node, offset));
+ if (node && GrAnnotation.getLength(node) > offset) {
+ tail.appendChild(GrAnnotation.splitNode(node, offset));
}
while (node && node.nextSibling) {
tail.appendChild(node.nextSibling);
@@ -245,7 +246,7 @@
},
_annotateText(node: Text, offset: number, length: number, cssClass: string) {
- const nodeLength = this.getLength(node);
+ const nodeLength = GrAnnotation.getLength(node);
// There are four cases:
// 1) Entire node is highlighted.
@@ -255,17 +256,17 @@
if (offset === 0 && nodeLength === length) {
// Case 1.
- this.wrapInHighlight(node, cssClass);
+ GrAnnotation.wrapInHighlight(node, cssClass);
} else if (offset === 0) {
// Case 2.
- this.splitAndWrapInHighlight(node, length, cssClass, true);
+ GrAnnotation.splitAndWrapInHighlight(node, length, cssClass, true);
} else if (offset + length === nodeLength) {
// Case 3
- this.splitAndWrapInHighlight(node, offset, cssClass, false);
+ GrAnnotation.splitAndWrapInHighlight(node, offset, cssClass, false);
} else {
// Case 4
- this.splitAndWrapInHighlight(
- this.splitTextNode(node, offset),
+ GrAnnotation.splitAndWrapInHighlight(
+ GrAnnotation.splitTextNode(node, offset),
length,
cssClass,
true
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
index 38bd707..5669bcf 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
@@ -19,7 +19,7 @@
*/
getLength(node: Node) {
if (node instanceof Comment) return 0;
- return this.getStringLength(node.textContent || '');
+ return GrAnnotation.getStringLength(node.textContent || '');
},
/**
@@ -65,7 +65,7 @@
const nestedNodes: Node[] = [];
for (let node of childNodes) {
- const initialNodeLength = this.getLength(node);
+ const initialNodeLength = GrAnnotation.getLength(node);
// If the current node is completely before the offset.
if (offset > 0 && initialNodeLength <= offset) {
offset -= initialNodeLength;
@@ -73,15 +73,15 @@
}
if (offset > 0) {
- node = this.splitNode(node, offset);
+ node = GrAnnotation.splitNode(node, offset);
offset = 0;
}
- if (this.getLength(node) > length) {
- this.splitNode(node, length);
+ if (GrAnnotation.getLength(node) > length) {
+ GrAnnotation.splitNode(node, length);
}
nestedNodes.push(node);
- length -= this.getLength(node);
+ length -= GrAnnotation.getLength(node);
if (!length) break;
}
@@ -116,7 +116,7 @@
let subLength;
for (const node of nodes) {
- nodeLength = this.getLength(node);
+ nodeLength = GrAnnotation.getLength(node);
// If the current node is completely before the offset.
if (nodeLength <= offset) {
@@ -128,9 +128,9 @@
subLength = Math.min(length, nodeLength - offset);
if (node instanceof Text) {
- this._annotateText(node, offset, subLength, cssClass);
+ GrAnnotation._annotateText(node, offset, subLength, cssClass);
} else if (node instanceof Element) {
- this.annotateElement(node, offset, subLength, cssClass);
+ GrAnnotation.annotateElement(node, offset, subLength, cssClass);
}
// If there is still more to annotate, then shift the indices, otherwise
@@ -172,19 +172,19 @@
firstPart?: boolean
) {
if (
- (this.getLength(node) === offset && firstPart) ||
+ (GrAnnotation.getLength(node) === offset && firstPart) ||
(offset === 0 && !firstPart)
) {
- return this.wrapInHighlight(node, cssClass);
+ return GrAnnotation.wrapInHighlight(node, cssClass);
}
if (firstPart) {
- this.splitNode(node, offset);
+ GrAnnotation.splitNode(node, offset);
// Node points to first part of the Text, second one is sibling.
} else {
// if node is Text then splitNode will return a Text
- node = this.splitNode(node, offset) as Text;
+ node = GrAnnotation.splitNode(node, offset) as Text;
}
- return this.wrapInHighlight(node, cssClass);
+ return GrAnnotation.wrapInHighlight(node, cssClass);
},
/**
@@ -193,7 +193,7 @@
*/
splitNode(element: Node, offset: number) {
if (element instanceof Text) {
- return this.splitTextNode(element, offset);
+ return GrAnnotation.splitTextNode(element, offset);
}
const tail = element.cloneNode(false);
@@ -203,13 +203,14 @@
let node = element.firstChild;
while (
node &&
- (this.getLength(node) <= offset || this.getLength(node) === 0)
+ (GrAnnotation.getLength(node) <= offset ||
+ GrAnnotation.getLength(node) === 0)
) {
- offset -= this.getLength(node);
+ offset -= GrAnnotation.getLength(node);
node = node.nextSibling;
}
- if (node && this.getLength(node) > offset) {
- tail.appendChild(this.splitNode(node, offset));
+ if (node && GrAnnotation.getLength(node) > offset) {
+ tail.appendChild(GrAnnotation.splitNode(node, offset));
}
while (node && node.nextSibling) {
tail.appendChild(node.nextSibling);
@@ -245,7 +246,7 @@
},
_annotateText(node: Text, offset: number, length: number, cssClass: string) {
- const nodeLength = this.getLength(node);
+ const nodeLength = GrAnnotation.getLength(node);
// There are four cases:
// 1) Entire node is highlighted.
@@ -255,17 +256,17 @@
if (offset === 0 && nodeLength === length) {
// Case 1.
- this.wrapInHighlight(node, cssClass);
+ GrAnnotation.wrapInHighlight(node, cssClass);
} else if (offset === 0) {
// Case 2.
- this.splitAndWrapInHighlight(node, length, cssClass, true);
+ GrAnnotation.splitAndWrapInHighlight(node, length, cssClass, true);
} else if (offset + length === nodeLength) {
// Case 3
- this.splitAndWrapInHighlight(node, offset, cssClass, false);
+ GrAnnotation.splitAndWrapInHighlight(node, offset, cssClass, false);
} else {
// Case 4
- this.splitAndWrapInHighlight(
- this.splitTextNode(node, offset),
+ GrAnnotation.splitAndWrapInHighlight(
+ GrAnnotation.splitTextNode(node, offset),
length,
cssClass,
true
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 68ec76a..ba6e384 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -3084,37 +3084,48 @@
});
}
- async setInProjectLookup(changeNum: NumericChangeId, project: RepoName) {
- const lookupProject = await this._projectLookup[changeNum];
- if (lookupProject && lookupProject !== project) {
- console.warn(
- 'Change set with multiple project nums.' +
- 'One of them must be invalid.'
- );
- }
+ /**
+ * This can be called by the router, if the project can be determined from
+ * the URL. Or when handling a dashabord or a search response.
+ *
+ * Then we don't need to make a dedicated REST API call or have a fallback,
+ * if that fails.
+ */
+ setInProjectLookup(changeNum: NumericChangeId, project: RepoName) {
this._projectLookup[changeNum] = Promise.resolve(project);
}
getFromProjectLookup(
changeNum: NumericChangeId
): Promise<RepoName | undefined> {
- const project = this._projectLookup[`${changeNum}`];
- if (project) {
- return project;
- }
+ // Hopefully setInProjectLookup() has already been called. Then we don't
+ // have to make a dedicated REST API call to look up the project.
+ let projectPromise = this._projectLookup[changeNum];
+ if (projectPromise) return projectPromise;
- const onError = (response?: Response | null) => firePageError(response);
+ // Ignore errors, because we have some dedicated fallback logic, see below.
+ const onError = () => {};
+ projectPromise = this.getChange(changeNum, onError).then(change => {
+ if (change?.project) return change.project;
- const projectPromise = this.getChange(changeNum, onError).then(change => {
- if (!change || !change.project) {
- return;
+ // In the very rare case that the change index cannot provide an answer
+ // (e.g. stale index) we should check, if the router has called
+ // setInProjectLookup() in the meantime. Then we can fall back to that.
+ const currentProjectPromise = this._projectLookup[changeNum];
+ if (currentProjectPromise !== projectPromise) {
+ return currentProjectPromise;
}
- this.setInProjectLookup(changeNum, change.project);
- return change.project;
+
+ // No luck. Without knowing the project we cannot proceed at all.
+ firePageError(
+ new Response(
+ `Failed to lookup the repo for change number ${changeNum}`,
+ {status: 404}
+ )
+ );
+ return undefined;
});
-
this._projectLookup[changeNum] = projectPromise;
-
return projectPromise;
}
@@ -3304,10 +3315,6 @@
return this.getDocsBaseUrlCachedPromise;
}
- testOnly_clearDocsBaseUrlCache() {
- this.getDocsBaseUrlCachedPromise = undefined;
- }
-
getDocumentationSearches(filter: string): Promise<DocResult[] | undefined> {
filter = filter.trim();
const encodedFilter = encodeURIComponent(filter);
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
index d570163..764aa98 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
@@ -38,6 +38,7 @@
} from '../../constants/constants';
import {
BasePatchSetNum,
+ ChangeInfo,
ChangeMessageId,
CommentInfo,
DashboardId,
@@ -62,6 +63,7 @@
import {assert} from '@open-wc/testing';
import {AuthService} from '../gr-auth/gr-auth';
import {GrAuthMock} from '../gr-auth/gr-auth_mock';
+import {getBaseUrl} from '../../utils/url-util';
const EXPECTED_QUERY_OPTIONS = listChangesOptionsToHex(
ListChangesOption.CHANGE_ACTIONS,
@@ -356,7 +358,7 @@
assert.isTrue(fetchStub.calledOnce);
assert.equal(
fetchStub.firstCall.args[0].url,
- 'test52/accounts/?o=DETAILS&q=%22bro%22'
+ `${getBaseUrl()}/accounts/?o=DETAILS&q=%22bro%22`
);
});
@@ -365,7 +367,7 @@
assert.isTrue(fetchStub.calledOnce);
assert.equal(
fetchStub.firstCall.args[0].url,
- 'test53/accounts/?o=DETAILS&q=%22bro%22%20and%20cansee%3A341682'
+ `${getBaseUrl()}/accounts/?o=DETAILS&q=%22bro%22%20and%20cansee%3A341682`
);
});
@@ -379,8 +381,7 @@
assert.isTrue(fetchStub.calledOnce);
assert.equal(
fetchStub.firstCall.args[0].url,
- 'test54/accounts/?o=DETAILS&q=%22bro%22%20and%20' +
- 'cansee%3A341682%20and%20is%3Aactive'
+ `${getBaseUrl()}/accounts/?o=DETAILS&q=%22bro%22%20and%20cansee%3A341682%20and%20is%3Aactive`
);
});
});
@@ -1154,32 +1155,46 @@
});
test('setInProjectLookup', async () => {
- await element.setInProjectLookup(
- 555 as NumericChangeId,
- 'project' as RepoName
- );
+ element.setInProjectLookup(555 as NumericChangeId, 'project' as RepoName);
const project = await element.getFromProjectLookup(555 as NumericChangeId);
assert.deepEqual(project, 'project' as RepoName);
});
suite('getFromProjectLookup', () => {
- test('getChange succeeds, no project', async () => {
- sinon.stub(element, 'getChange').resolves(null);
- const val = await element.getFromProjectLookup(555 as NumericChangeId);
- assert.strictEqual(val, undefined);
+ const changeNum = 555 as NumericChangeId;
+ const repo = 'test-repo' as RepoName;
+
+ test('getChange fails to yield a project', async () => {
+ const promise = mockPromise<null>();
+ sinon.stub(element, 'getChange').returns(promise);
+
+ const projectLookup = element.getFromProjectLookup(changeNum);
+ promise.resolve(null);
+
+ assert.isUndefined(await projectLookup);
});
test('getChange succeeds with project', async () => {
- sinon
- .stub(element, 'getChange')
- .resolves({...createChange(), project: 'project' as RepoName});
- const projectLookup = element.getFromProjectLookup(
- 555 as NumericChangeId
- );
- const val = await projectLookup;
- assert.equal(val, 'project' as RepoName);
+ const promise = mockPromise<null | ChangeInfo>();
+ sinon.stub(element, 'getChange').returns(promise);
+
+ const projectLookup = element.getFromProjectLookup(changeNum);
+ promise.resolve({...createChange(), project: repo});
+
+ assert.equal(await projectLookup, repo);
assert.deepEqual(element._projectLookup, {'555': projectLookup});
});
+
+ test('getChange fails, but a setInProjectLookup() call is used as fallback', async () => {
+ const promise = mockPromise<null>();
+ sinon.stub(element, 'getChange').returns(promise);
+
+ const projectLookup = element.getFromProjectLookup(changeNum);
+ element.setInProjectLookup(changeNum, repo);
+ promise.resolve(null);
+
+ assert.equal(await projectLookup, repo);
+ });
});
suite('getChanges populates _projectLookup', () => {
@@ -1592,10 +1607,11 @@
test('null config', async () => {
const probePathMock = sinon.stub(element, 'probePath').resolves(true);
const docsBaseUrl = await element.getDocsBaseUrl(undefined);
- assert.isTrue(
- probePathMock.calledWith('test91/Documentation/index.html')
+ assert.equal(
+ probePathMock.lastCall.args[0],
+ `${getBaseUrl()}/Documentation/index.html`
);
- assert.equal(docsBaseUrl, 'test91/Documentation');
+ assert.equal(docsBaseUrl, `${getBaseUrl()}/Documentation`);
});
test('no doc config', async () => {
@@ -1605,10 +1621,11 @@
gerrit: createGerritInfo(),
};
const docsBaseUrl = await element.getDocsBaseUrl(config);
- assert.isTrue(
- probePathMock.calledWith('test92/Documentation/index.html')
+ assert.equal(
+ probePathMock.lastCall.args[0],
+ `${getBaseUrl()}/Documentation/index.html`
);
- assert.equal(docsBaseUrl, 'test92/Documentation');
+ assert.equal(docsBaseUrl, `${getBaseUrl()}/Documentation`);
});
test('has doc config', async () => {
@@ -1625,8 +1642,9 @@
test('no probe', async () => {
const probePathMock = sinon.stub(element, 'probePath').resolves(false);
const docsBaseUrl = await element.getDocsBaseUrl(undefined);
- assert.isTrue(
- probePathMock.calledWith('test94/Documentation/index.html')
+ assert.equal(
+ probePathMock.lastCall.args[0],
+ `${getBaseUrl()}/Documentation/index.html`
);
assert.isNotOk(docsBaseUrl);
});
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index 24ba0e5..f1fae3b 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -375,7 +375,6 @@
): Promise<string>;
getDocsBaseUrl(config?: ServerInfo): Promise<string | null>;
- testOnly_clearDocsBaseUrlCache(): void;
createChange(
repo: RepoName,
diff --git a/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts b/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
index 842dace..34187fe 100644
--- a/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
+++ b/polygerrit-ui/app/services/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
@@ -3,25 +3,24 @@
* Copyright 2019 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {
- getAccountDisplayName,
- getGroupDisplayName,
-} from '../../utils/display-name-util';
import {RestApiService} from '../gr-rest-api/gr-rest-api';
import {
- AccountInfo,
isReviewerAccountSuggestion,
isReviewerGroupSuggestion,
NumericChangeId,
ServerInfo,
+ SuggestedReviewerAccountInfo,
SuggestedReviewerInfo,
Suggestion,
} from '../../types/common';
-import {assertNever} from '../../utils/common-util';
import {AutocompleteSuggestion} from '../../elements/shared/gr-autocomplete/gr-autocomplete';
import {allSettled, isFulfilled} from '../../utils/async-util';
import {isDefined, ParsedChangeInfo} from '../../types/types';
-import {accountKey} from '../../utils/account-util';
+import {
+ accountKey,
+ getSuggestedReviewerName,
+ isAccountSuggestion,
+} from '../../utils/account-util';
import {
AccountId,
ChangeInfo,
@@ -96,30 +95,11 @@
makeSuggestionItem(
suggestion: Suggestion
): AutocompleteSuggestion<SuggestedReviewerInfo> {
- if (isReviewerAccountSuggestion(suggestion)) {
- // Reviewer is an account suggestion from getChangeSuggestedReviewers.
- return {
- name: getAccountDisplayName(this.config, suggestion.account),
- value: suggestion,
- };
- }
-
- if (isReviewerGroupSuggestion(suggestion)) {
- // Reviewer is a group suggestion from getChangeSuggestedReviewers.
- return {
- name: getGroupDisplayName(suggestion.group),
- value: suggestion,
- };
- }
-
- if (isAccountSuggestion(suggestion)) {
- // Reviewer is an account suggestion from getSuggestedAccounts.
- return {
- name: getAccountDisplayName(this.config, suggestion),
- value: {account: suggestion, count: 1},
- };
- }
- assertNever(suggestion, 'Received an incorrect suggestion');
+ const name = getSuggestedReviewerName(suggestion, this.config);
+ const value = isAccountSuggestion(suggestion)
+ ? ({account: suggestion, count: 1} as SuggestedReviewerAccountInfo)
+ : suggestion;
+ return {name, value};
}
private getSuggestionsForChange(
@@ -160,7 +140,3 @@
}
return undefined;
}
-
-function isAccountSuggestion(s: Suggestion): s is AccountInfo {
- return (s as AccountInfo)._account_id !== undefined;
-}
diff --git a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
index f8e4160..bfa881e 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -321,9 +321,6 @@
}
return Promise.resolve('');
},
- testOnly_clearDocsBaseUrlCache() {
- return;
- },
getDocumentationSearches(): Promise<DocResult[] | undefined> {
return Promise.resolve([]);
},
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index 7dc9a62..68f6b42 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -16,10 +16,17 @@
ReviewerInput,
ServerInfo,
UserId,
+ Suggestion,
+ isReviewerAccountSuggestion,
+ isReviewerGroupSuggestion,
} from '../types/common';
import {AccountTag, ReviewerState} from '../constants/constants';
import {assertNever, hasOwnProperty} from './common-util';
-import {getDisplayName} from './display-name-util';
+import {
+ getAccountDisplayName,
+ getDisplayName,
+ getGroupDisplayName,
+} from './display-name-util';
import {getApprovalInfo} from './label-util';
import {ParsedChangeInfo} from '../types/types';
@@ -220,3 +227,29 @@
}
throw new Error('Must be either an account or a group.');
}
+
+export function isAccountSuggestion(s: Suggestion): s is AccountInfo {
+ return (s as AccountInfo)._account_id !== undefined;
+}
+
+export function getSuggestedReviewerName(
+ suggestion: Suggestion,
+ config?: ServerInfo
+) {
+ if (isAccountSuggestion(suggestion)) {
+ // Reviewer is an account suggestion from getSuggestedAccounts.
+ return getAccountDisplayName(config, suggestion);
+ }
+
+ if (isReviewerAccountSuggestion(suggestion)) {
+ // Reviewer is an account suggestion from getChangeSuggestedReviewers.
+ return getAccountDisplayName(config, suggestion.account);
+ }
+
+ if (isReviewerGroupSuggestion(suggestion)) {
+ // Reviewer is a group suggestion from getChangeSuggestedReviewers.
+ return getGroupDisplayName(suggestion.group);
+ }
+
+ assertNever(suggestion, 'Received an incorrect suggestion');
+}