Merge "Update @open-wc/testing to 3.1.8"
diff --git a/WORKSPACE b/WORKSPACE
index 29266cd..ad34969 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -112,8 +112,8 @@
http_archive(
name = "build_bazel_rules_nodejs",
- sha256 = "c29944ba9b0b430aadcaf3bf2570fece6fc5ebfb76df145c6cdad40d65c20811",
- urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/5.7.0/rules_nodejs-5.7.0.tar.gz"],
+ sha256 = "94070eff79305be05b7699207fbac5d2608054dd53e6109f7d00d923919ff45a",
+ urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/5.8.2/rules_nodejs-5.8.2.tar.gz"],
)
load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies")
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/package.json b/package.json
index 362b9dc..7b310aa 100644
--- a/package.json
+++ b/package.json
@@ -1,12 +1,12 @@
{
"name": "gerrit",
- "version": "3.1.0-SNAPSHOT",
+ "version": "3.9.0-SNAPSHOT",
"description": "Gerrit Code Review",
"dependencies": {
- "@bazel/concatjs": "^5.5.0",
- "@bazel/rollup": "^5.5.0",
- "@bazel/terser": "^5.5.0",
- "@bazel/typescript": "^5.5.0"
+ "@bazel/concatjs": "^5.8.0",
+ "@bazel/rollup": "^5.8.0",
+ "@bazel/terser": "^5.8.0",
+ "@bazel/typescript": "^5.8.0"
},
"devDependencies": {
"@koa/cors": "^3.3.0",
@@ -18,7 +18,7 @@
"eslint-config-google": "^0.14.0",
"eslint-plugin-html": "^6.2.0",
"eslint-plugin-import": "^2.26.0",
- "eslint-plugin-jsdoc": "^39.6.4",
+ "eslint-plugin-jsdoc": "^44.2.4",
"eslint-plugin-lit": "^1.6.1",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^4.0.0",
diff --git a/plugins/.eslintrc.js b/plugins/.eslintrc.js
index 149a31e..920ec2a 100644
--- a/plugins/.eslintrc.js
+++ b/plugins/.eslintrc.js
@@ -186,8 +186,6 @@
'jsdoc/implements-on-classes': 2,
// https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-match-description
'jsdoc/match-description': 0,
- // https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-newline-after-description
- 'jsdoc/newline-after-description': 2,
// https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-no-types
'jsdoc/no-types': 0,
// https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-no-undefined-types
diff --git a/polygerrit-ui/app/.eslintrc.js b/polygerrit-ui/app/.eslintrc.js
index e18a3af..7abc658 100644
--- a/polygerrit-ui/app/.eslintrc.js
+++ b/polygerrit-ui/app/.eslintrc.js
@@ -176,8 +176,6 @@
'jsdoc/implements-on-classes': 2,
// https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-match-description
'jsdoc/match-description': 0,
- // https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-newline-after-description
- 'jsdoc/newline-after-description': 2,
// https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-no-types
'jsdoc/no-types': 0,
// https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-no-undefined-types
diff --git a/polygerrit-ui/app/api/package.json b/polygerrit-ui/app/api/package.json
index 7df755b..a125b8f 100644
--- a/polygerrit-ui/app/api/package.json
+++ b/polygerrit-ui/app/api/package.json
@@ -1,6 +1,6 @@
{
"name": "@gerritcodereview/typescript-api",
- "version": "3.8.0",
+ "version": "3.9.0-SNAPSHOT",
"description": "Gerrit Code Review - TypeScript API",
"homepage": "https://www.gerritcodereview.com/",
"browser": true,
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');
+}
diff --git a/yarn.lock b/yarn.lock
index d66270b..df4e166 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -51,53 +51,52 @@
dependencies:
regenerator-runtime "^0.13.4"
-"@bazel/concatjs@^5.5.0":
- version "5.5.0"
- resolved "https://registry.yarnpkg.com/@bazel/concatjs/-/concatjs-5.5.0.tgz#e6104ed70595cae59463ae6b0b5389252566221e"
- integrity sha512-hwG+ahivR20Z3iTOlkUz3OdwnW/PUaZyyz8BIX+GNUTg6U3rPHK51CavUirMupOU/LRJ5GyCwBNAAtjCyquo2g==
+"@bazel/concatjs@^5.8.0":
+ version "5.8.1"
+ resolved "https://registry.yarnpkg.com/@bazel/concatjs/-/concatjs-5.8.1.tgz#dd20882429e382cae79c08cbd3238dfc680d2d67"
+ integrity sha512-TkARsNUxgi3bjFeGwIGlffmQglNhuR9qK9uE7uKhdBZvQE5caAWVCjYiMTzo3viKDhwKn5QNRcHY5huuJMVFfA==
dependencies:
protobufjs "6.8.8"
source-map-support "0.5.9"
tsutils "3.21.0"
-"@bazel/rollup@^5.5.0":
- version "5.5.0"
- resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-5.5.0.tgz#1e152d6147ef5583ec9fd872756c9d0635db73c7"
- integrity sha512-8SRbgVfaYdNb6PyIypj8jzzJHhlIRyMH3s5KpXODsjD+mXECH4jQxJ8VcRkt0f0exsgB12gK5dmoUK/F2PDKCw==
+"@bazel/rollup@^5.8.0":
+ version "5.8.1"
+ resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-5.8.1.tgz#ee876c35595b456f700d258385412e4f0dd57c15"
+ integrity sha512-Ys+UWbRp1TY2j+z15N+SZgID/nuqAtJTgJDsz0NZVjm8F8KzmgXxLDnBb/cUKFVk83pNOAi84G/bq1tINjMSNA==
dependencies:
- "@bazel/worker" "5.5.0"
+ "@bazel/worker" "5.8.1"
-"@bazel/terser@^5.5.0":
- version "5.5.0"
- resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-5.5.0.tgz#3b2b582a417d99d59ae99b50d74576ca0719c03a"
- integrity sha512-aBjNmJ7TbcD7cKAdFErYQYXn4OqTvrmqrtN6Z6Wnv82d+23kbEsF427ixgdCO3GTQJDw7+x7K9TP2CGogaGtcg==
+"@bazel/terser@^5.8.0":
+ version "5.8.1"
+ resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-5.8.1.tgz#729a0ec6dcc83e99c4f6d3f2bebb0ff254c10c48"
+ integrity sha512-TPjSDhw1pSZt9P2hd/22IJwl8KCZiJL+u2gB5mghBTCFDVdC5Dgsx135pFtvlqc6LjjOvd3s6dzcQr0YJo2HSg==
-"@bazel/typescript@^5.5.0":
- version "5.5.0"
- resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-5.5.0.tgz#053c255acb1b3cac23d24984cd8d5d5542fe1f7c"
- integrity sha512-Ord0+nCj+B1M4NDbe0uqZf2FyOCzaDAlc4DAsr5UKJrArCipIbMTEAxlsEk+WAYBNAFGO/FS9/zlDtLceqpHqw==
+"@bazel/typescript@^5.8.0":
+ version "5.8.1"
+ resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-5.8.1.tgz#74a76af434fad7930893cf3e98b4cc201e52dc65"
+ integrity sha512-NAJ8WQHZL1WE1YmRoCrq/1hhG15Mvy/viWh6TkvFnBeEhNUiQUsA5GYyhU1ztnBIYW03nATO3vwhAEfO7Q0U5g==
dependencies:
- "@bazel/worker" "5.5.0"
- protobufjs "6.8.8"
+ "@bazel/worker" "5.8.1"
semver "5.6.0"
source-map-support "0.5.9"
tsutils "3.21.0"
-"@bazel/worker@5.5.0":
- version "5.5.0"
- resolved "https://registry.yarnpkg.com/@bazel/worker/-/worker-5.5.0.tgz#d30b75e46f2052d33bcda251b328d36655a5636f"
- integrity sha512-pYfjJKg4D1CQ/AJ1UGC5ySyH09gDqNiBrQJ0uMYVewIBW24uOAkKsJfTE2y4ES0UL1Ik758WO0la0mJeFOKScg==
+"@bazel/worker@5.8.1":
+ version "5.8.1"
+ resolved "https://registry.yarnpkg.com/@bazel/worker/-/worker-5.8.1.tgz#65af7a70dd2f1aaedd6c19330abd9a198f96e7b2"
+ integrity sha512-GMyZSNW3F34f9GjbJqvs1aHyed5BNrNeiDzNJhC1fIizo/UeBM21oBBONIYLBDoBtq936U85VyPZ76JaP/83hw==
dependencies:
google-protobuf "^3.6.1"
-"@es-joy/jsdoccomment@~0.36.1":
- version "0.36.1"
- resolved "https://registry.yarnpkg.com/@es-joy/jsdoccomment/-/jsdoccomment-0.36.1.tgz#c37db40da36e4b848da5fd427a74bae3b004a30f"
- integrity sha512-922xqFsTpHs6D0BUiG4toiyPOMc8/jafnWKxz1KWgS4XzKPy2qXf1Pe6UFuNSCQqt6tOuhAWXBNuuyUhJmw9Vg==
+"@es-joy/jsdoccomment@~0.39.3":
+ version "0.39.4"
+ resolved "https://registry.yarnpkg.com/@es-joy/jsdoccomment/-/jsdoccomment-0.39.4.tgz#6b8a62e9b3077027837728818d3c4389a898b392"
+ integrity sha512-Jvw915fjqQct445+yron7Dufix9A+m9j1fCJYlCo1FWlRvTxa3pjJelxdSTdaLWcTwRU6vbL+NYjO4YuNIS5Qg==
dependencies:
comment-parser "1.3.1"
- esquery "^1.4.0"
- jsdoc-type-pratt-parser "~3.1.0"
+ esquery "^1.5.0"
+ jsdoc-type-pratt-parser "~4.0.0"
"@esbuild/linux-loong64@0.14.54":
version "0.14.54"
@@ -690,6 +689,11 @@
normalize-path "^3.0.0"
picomatch "^2.0.4"
+are-docs-informative@^0.0.2:
+ version "0.0.2"
+ resolved "https://registry.yarnpkg.com/are-docs-informative/-/are-docs-informative-0.0.2.tgz#387f0e93f5d45280373d387a59d34c96db321963"
+ integrity sha512-ixiS0nLNNG5jNQzgZJNoUpBKdo9yTYZMGJ+QgT2jmjR7G7+QHRCc4v6LQ3NgE7EBJq+o0ams3waJwkrlBom8Ig==
+
argparse@^2.0.1:
version "2.0.1"
resolved "https://registry.yarnpkg.com/argparse/-/argparse-2.0.1.tgz#246f50f3ca78a3240f6c997e8a9bd1eac49e4b38"
@@ -1704,17 +1708,18 @@
resolve "^1.22.0"
tsconfig-paths "^3.14.1"
-eslint-plugin-jsdoc@^39.6.4:
- version "39.6.4"
- resolved "https://registry.yarnpkg.com/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-39.6.4.tgz#b940aebd3eea26884a0d341785d2dc3aba6a38a7"
- integrity sha512-fskvdLCfwmPjHb6e+xNGDtGgbF8X7cDwMtVLAP2WwSf9Htrx68OAx31BESBM1FAwsN2HTQyYQq7m4aW4Q4Nlag==
+eslint-plugin-jsdoc@^44.2.4:
+ version "44.2.4"
+ resolved "https://registry.yarnpkg.com/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-44.2.4.tgz#0bdc163771504ec7330414eda6a7dbae67156ddb"
+ integrity sha512-/EMMxCyRh1SywhCb66gAqoGX4Yv6Xzc4bsSkF1AiY2o2+bQmGMQ05QZ5+JjHbdFTPDZY9pfn+DsSNP0a5yQpIg==
dependencies:
- "@es-joy/jsdoccomment" "~0.36.1"
+ "@es-joy/jsdoccomment" "~0.39.3"
+ are-docs-informative "^0.0.2"
comment-parser "1.3.1"
debug "^4.3.4"
escape-string-regexp "^4.0.0"
- esquery "^1.4.0"
- semver "^7.3.8"
+ esquery "^1.5.0"
+ semver "^7.5.1"
spdx-expression-parse "^3.0.1"
eslint-plugin-lit@^1.6.1:
@@ -1859,6 +1864,13 @@
dependencies:
estraverse "^5.1.0"
+esquery@^1.5.0:
+ version "1.5.0"
+ resolved "https://registry.yarnpkg.com/esquery/-/esquery-1.5.0.tgz#6ce17738de8577694edd7361c57182ac8cb0db0b"
+ integrity sha512-YQLXUplAwJgCydQ78IMJywZCceoqk1oH01OERdSAJc/7U2AylwjhSCLDEtqwg811idIS/9fIU5GjG73IgjKMVg==
+ dependencies:
+ estraverse "^5.1.0"
+
esrecurse@^4.3.0:
version "4.3.0"
resolved "https://registry.yarnpkg.com/esrecurse/-/esrecurse-4.3.0.tgz#7ad7964d679abb28bee72cec63758b1c5d2c9921"
@@ -2918,10 +2930,10 @@
dependencies:
argparse "^2.0.1"
-jsdoc-type-pratt-parser@~3.1.0:
- version "3.1.0"
- resolved "https://registry.yarnpkg.com/jsdoc-type-pratt-parser/-/jsdoc-type-pratt-parser-3.1.0.tgz#a4a56bdc6e82e5865ffd9febc5b1a227ff28e67e"
- integrity sha512-MgtD0ZiCDk9B+eI73BextfRrVQl0oyzRG8B2BjORts6jbunj4ScKPcyXGTbB6eXL4y9TzxCm6hyeLq/2ASzNdw==
+jsdoc-type-pratt-parser@~4.0.0:
+ version "4.0.0"
+ resolved "https://registry.yarnpkg.com/jsdoc-type-pratt-parser/-/jsdoc-type-pratt-parser-4.0.0.tgz#136f0571a99c184d84ec84662c45c29ceff71114"
+ integrity sha512-YtOli5Cmzy3q4dP26GraSOeAhqecewG04hoO8DY56CH4KJ9Fvv5qKWUCCo3HZob7esJQHCv6/+bnTy72xZZaVQ==
json-buffer@3.0.0:
version "3.0.0"
@@ -4107,10 +4119,10 @@
dependencies:
lru-cache "^6.0.0"
-semver@^7.3.8:
- version "7.3.8"
- resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.8.tgz#07a78feafb3f7b32347d725e33de7e2a2df67798"
- integrity sha512-NB1ctGL5rlHrPJtFDVIVzTyQylMLu9N9VICA6HSFJo8MCGVTMW6gfpicwKmmK/dAjTOrqu5l63JJOpDSrAis3A==
+semver@^7.5.1:
+ version "7.5.1"
+ resolved "https://registry.yarnpkg.com/semver/-/semver-7.5.1.tgz#c90c4d631cf74720e46b21c1d37ea07edfab91ec"
+ integrity sha512-Wvss5ivl8TMRZXXESstBA4uR5iXgEN/VC5/sOcuXdVLzcdkz4HWetIoRfG5gb5X+ij/G9rw9YoGn3QoQ8OCSpw==
dependencies:
lru-cache "^6.0.0"
