Merge "Update Getting Started to Gerrit 3.7.8" into stable-3.7
diff --git a/.bazelversion b/.bazelversion
index 6abaeb2..91e4a9f 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-6.2.0
+6.3.2
diff --git a/Documentation/cmd-index-changes.txt b/Documentation/cmd-index-changes.txt
index 0ee7aab..1d4cbe34 100644
--- a/Documentation/cmd-index-changes.txt
+++ b/Documentation/cmd-index-changes.txt
@@ -16,8 +16,7 @@
supported by the REST API.
== ACCESS
-Caller must have the 'Maintain Server' capability, or be the owner of the change
-to be indexed.
+Caller must have the 'Maintain Server' capability.
== SCRIPTING
This command is intended to be used in scripts.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index b9de6ab..720ac5f 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6730,6 +6730,12 @@
|`_number` ||
The change number. (The underscore is just a relict of a prior
attempt to deprecate the change number.)
+|`virtual_id_number` ||
+The virtual id number is globally unique. For local changes, it is equal to the
+`_number` attribute. For imported changes, the original `_number` is processed
+through a function designed to prevent conflicts with local change numbers.
+Note that its usage is intended solely for Gerrit's internals and UI, and
+adoption outside these scenarios is not advised.
|`owner` ||
The owner of the change as an link:rest-api-accounts.html#account-info[
AccountInfo] entity.
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 01c4942..92c3e92 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1543,6 +1543,7 @@
configLabel(project, label, func, ImmutableList.of(), value);
}
+ @SuppressWarnings("deprecation")
private void configLabel(
Project.NameKey project,
String label,
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index a9ad3f2..3bbc74e 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -332,7 +332,7 @@
String configuredIndexBackend = cfg.getString("index", null, "type");
if (configuredIndexBackend == null) {
// Propagate index type to pgms that run off of the gerrit.config file on local disk.
- IndexType indexType = IndexType.fromEnvironment().orElse(new IndexType("fake"));
+ IndexType indexType = IndexType.fromEnvironment().orElseGet(() -> new IndexType("fake"));
gerritConfig.setString("index", null, "type", indexType.isLucene() ? "lucene" : "fake");
}
gerritConfig.save();
@@ -509,7 +509,7 @@
IndexType indexType =
(configuredIndexBackend != null)
? new IndexType(configuredIndexBackend)
- : IndexType.fromEnvironment().orElse(new IndexType("fake"));
+ : IndexType.fromEnvironment().orElseGet(() -> new IndexType("fake"));
daemon.setIndexModule(createIndexModule(indexType, baseConfig, testIndexModule));
daemon.setEnableHttpd(desc.httpd());
diff --git a/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java
index dcf1158..a37c2ba 100644
--- a/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java
@@ -78,7 +78,7 @@
private InternalGroupCreation toInternalGroupCreation(TestGroupCreation groupCreation) {
AccountGroup.Id groupId = AccountGroup.id(seq.nextGroupId());
- String groupName = groupCreation.name().orElse("group-with-id-" + groupId.get());
+ String groupName = groupCreation.name().orElseGet(() -> "group-with-id-" + groupId.get());
AccountGroup.UUID groupUuid = GroupUuid.make(groupName, serverIdent);
AccountGroup.NameKey nameKey = AccountGroup.nameKey(groupName);
return InternalGroupCreation.builder()
diff --git a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
index deeb843..1014a0f 100644
--- a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
@@ -85,7 +85,7 @@
}
private Project.NameKey createNewProject(TestProjectCreation projectCreation) throws Exception {
- String name = projectCreation.name().orElse(RandomStringUtils.randomAlphabetic(8));
+ String name = projectCreation.name().orElseGet(() -> RandomStringUtils.randomAlphabetic(8));
CreateProjectArgs args = new CreateProjectArgs();
args.setProjectName(name);
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index d381b68..75b3ef5 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -99,7 +99,7 @@
public Boolean containsGitConflicts;
public Integer _number;
- public Integer _virtualIdNumber;
+ public Integer virtualIdNumber;
public AccountInfo owner;
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index 15dcf42..1e76f21 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -242,7 +242,7 @@
@GerritServerConfig Config cfg,
GerritApi gerritApi,
ExperimentFeatures experimentFeatures) {
- String cdnPath = options.devCdn().orElse(cfg.getString("gerrit", null, "cdnPath"));
+ String cdnPath = options.devCdn().orElseGet(() -> cfg.getString("gerrit", null, "cdnPath"));
String faviconPath = cfg.getString("gerrit", null, "faviconPath");
return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi, experimentFeatures);
}
diff --git a/java/com/google/gerrit/index/QueryOptions.java b/java/com/google/gerrit/index/QueryOptions.java
index 29ab6d0..40677a18 100644
--- a/java/com/google/gerrit/index/QueryOptions.java
+++ b/java/com/google/gerrit/index/QueryOptions.java
@@ -128,4 +128,8 @@
limit(),
filter.apply(this));
}
+
+ public int getLimitBasedOnPaginationType() {
+ return PaginationType.NONE == config().paginationType() ? limit() : pageSize();
+ }
}
diff --git a/java/com/google/gerrit/index/query/IndexedQuery.java b/java/com/google/gerrit/index/query/IndexedQuery.java
index ee25ef9..cb98c06 100644
--- a/java/com/google/gerrit/index/query/IndexedQuery.java
+++ b/java/com/google/gerrit/index/query/IndexedQuery.java
@@ -87,6 +87,12 @@
}
@Override
+ public ResultSet<T> restart(int start) {
+ opts = opts.withStart(start);
+ return search();
+ }
+
+ @Override
public ResultSet<T> restart(int start, int pageSize) {
opts = opts.withStart(start).withPageSize(pageSize);
return search();
diff --git a/java/com/google/gerrit/index/query/Paginated.java b/java/com/google/gerrit/index/query/Paginated.java
index 5521990..1065019 100644
--- a/java/com/google/gerrit/index/query/Paginated.java
+++ b/java/com/google/gerrit/index/query/Paginated.java
@@ -19,6 +19,8 @@
public interface Paginated<T> {
QueryOptions getOptions();
+ ResultSet<T> restart(int start);
+
ResultSet<T> restart(int start, int pageSize);
ResultSet<T> restart(Object searchAfter, int pageSize);
diff --git a/java/com/google/gerrit/index/query/PaginatingSource.java b/java/com/google/gerrit/index/query/PaginatingSource.java
index 03d749a..84572d7 100644
--- a/java/com/google/gerrit/index/query/PaginatingSource.java
+++ b/java/com/google/gerrit/index/query/PaginatingSource.java
@@ -63,42 +63,55 @@
pageResultSize++;
}
- if (last != null
- && source instanceof Paginated
- // TODO: this fix is only for the stable branches and the real refactoring would be to
- // restore the logic
- // for the filtering in AndSource (L58 - 64) as per
- // https://gerrit-review.googlesource.com/c/gerrit/+/345634/9
- && !indexConfig.paginationType().equals(PaginationType.NONE)) {
+ if (last != null && source instanceof Paginated) {
// Restart source and continue if we have not filled the
// full limit the caller wants.
- //
@SuppressWarnings("unchecked")
Paginated<T> p = (Paginated<T>) source;
QueryOptions opts = p.getOptions();
final int limit = opts.limit();
- int pageSize = opts.pageSize();
- int pageSizeMultiplier = opts.pageSizeMultiplier();
- Object searchAfter = resultSet.searchAfter();
- int nextStart = pageResultSize;
- while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit
- pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
- ResultSet<T> next =
- indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
- ? p.restart(searchAfter, pageSize)
- : p.restart(nextStart, pageSize);
- pageResultSize = 0;
- for (T data : buffer(next)) {
- if (match(data)) {
- r.add(data);
+
+ // TODO: this fix is only for the stable branches and the real refactoring would be to
+ // restore the logic
+ // for the filtering in AndSource (L58 - 64) as per
+ // https://gerrit-review.googlesource.com/c/gerrit/+/345634/9
+ if (!indexConfig.paginationType().equals(PaginationType.NONE)) {
+ int pageSize = opts.pageSize();
+ int pageSizeMultiplier = opts.pageSizeMultiplier();
+ Object searchAfter = resultSet.searchAfter();
+ int nextStart = pageResultSize;
+ while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit
+ pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
+ ResultSet<T> next =
+ indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
+ ? p.restart(searchAfter, pageSize)
+ : p.restart(nextStart, pageSize);
+ pageResultSize = 0;
+ for (T data : buffer(next)) {
+ if (match(data)) {
+ r.add(data);
+ }
+ pageResultSize++;
+ if (r.size() > limit) {
+ break;
+ }
}
- pageResultSize++;
- if (r.size() > limit) {
- break;
- }
+ nextStart += pageResultSize;
+ searchAfter = next.searchAfter();
}
- nextStart += pageResultSize;
- searchAfter = next.searchAfter();
+ } else {
+ int nextStart = pageResultSize;
+ while (pageResultSize == limit && r.size() < limit) {
+ ResultSet<T> next = p.restart(nextStart);
+ pageResultSize = 0;
+ for (T data : buffer(next)) {
+ if (match(data)) {
+ r.add(data);
+ }
+ pageResultSize++;
+ }
+ nextStart += pageResultSize;
+ }
}
}
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index c790fe4..b4b4e1c 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -149,10 +149,14 @@
.findFirst()
.orElse(-1)
+ 1;
- int toIndex = Math.min(fromIndex + opts.pageSize(), valueList.size());
+ int toIndex = Math.min(fromIndex + opts.getLimitBasedOnPaginationType(), valueList.size());
results = valueList.subList(fromIndex, toIndex);
} else {
- results = valueStream.skip(opts.start()).limit(opts.pageSize()).collect(toImmutableList());
+ results =
+ valueStream
+ .skip(opts.start())
+ .limit(opts.getLimitBasedOnPaginationType())
+ .collect(toImmutableList());
}
queryCount++;
resultsSizes.add(results.size());
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index a47b87e..7ee99ac 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -394,11 +394,11 @@
IndexSearcher[] searchers = new IndexSearcher[indexes.size()];
Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex = new HashMap<>();
try {
- int realPageSize = opts.start() + opts.pageSize();
- if (Integer.MAX_VALUE - opts.pageSize() < opts.start()) {
- realPageSize = Integer.MAX_VALUE;
+ int pageLimit = AbstractLuceneIndex.getLimitBasedOnPaginationType(opts, opts.pageSize());
+ int queryLimit = opts.start() + pageLimit;
+ if (Integer.MAX_VALUE - pageLimit < opts.start()) {
+ queryLimit = Integer.MAX_VALUE;
}
- int queryLimit = AbstractLuceneIndex.getLimitBasedOnPaginationType(opts, realPageSize);
List<TopFieldDocs> hits = new ArrayList<>();
int searchAfterHitsCount = 0;
for (int i = 0; i < indexes.size(); i++) {
@@ -406,7 +406,7 @@
searchers[i] = subIndex.acquire();
if (isSearchAfterPagination) {
ScoreDoc searchAfter = getSearchAfter(subIndex);
- int maxRemainingHits = realPageSize - searchAfterHitsCount;
+ int maxRemainingHits = queryLimit - searchAfterHitsCount;
if (maxRemainingHits > 0) {
TopFieldDocs subIndexHits =
searchers[i].searchAfter(
@@ -558,7 +558,7 @@
}
for (SchemaField<ChangeData, ?> field : getSchema().getSchemaFields().values()) {
- if (fields.contains(field.getName()) && doc.get(field.getName()) != null) {
+ if (fields.contains(field.getName())) {
field.setIfPossible(cd, new LuceneStoredValue(doc.get(field.getName())));
}
}
diff --git a/java/com/google/gerrit/server/DeadlineChecker.java b/java/com/google/gerrit/server/DeadlineChecker.java
index f41b1e3..9b7ffe6 100644
--- a/java/com/google/gerrit/server/DeadlineChecker.java
+++ b/java/com/google/gerrit/server/DeadlineChecker.java
@@ -180,12 +180,14 @@
this.timeoutName =
clientedProvidedTimeout
.map(clientTimeout -> "client.timeout")
- .orElse(
- serverSideDeadline
- .map(serverDeadline -> serverDeadline.id() + ".timeout")
- .orElse("timeout"));
+ .orElseGet(
+ () ->
+ serverSideDeadline
+ .map(serverDeadline -> serverDeadline.id() + ".timeout")
+ .orElse("timeout"));
this.timeout =
- clientedProvidedTimeout.orElse(serverSideDeadline.map(ServerDeadline::timeout).orElse(0L));
+ clientedProvidedTimeout.orElseGet(
+ () -> serverSideDeadline.map(ServerDeadline::timeout).orElse(0L));
this.deadline = timeout > 0 ? Optional.of(start + timeout) : Optional.empty();
}
diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java
index 66a36f6..d306ad0 100644
--- a/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -98,7 +98,7 @@
@Override
public AccountState getEvenIfMissing(Account.Id accountId) {
- return get(accountId).orElse(missing(accountId));
+ return get(accountId).orElseGet(() -> missing(accountId));
}
@Override
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index 891a467..5edba08 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -195,7 +195,7 @@
"Unable to deactivate account %s",
authRequest
.getUserName()
- .orElse(" for external ID key " + authRequest.getExternalIdKey().get()));
+ .orElseGet(() -> " for external ID key " + authRequest.getExternalIdKey().get()));
}
}
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 928c8f8..7248836 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -773,7 +773,7 @@
.collect(toList());
}
- out._virtualIdNumber = cd.virtualId().get();
+ out.virtualIdNumber = cd.virtualId().get();
return out;
}
@@ -965,7 +965,7 @@
// repository only once
try (Repository allUsersRepo = repoManager.openRepository(allUsers)) {
List<Change.Id> changeIds =
- changeInfos.stream().map(c -> Change.id(c._virtualIdNumber)).collect(Collectors.toList());
+ changeInfos.stream().map(c -> Change.id(c.virtualIdNumber)).collect(Collectors.toList());
Set<Change.Id> starredChanges =
starredChangesUtil.areStarred(
allUsersRepo, changeIds, userProvider.get().asIdentifiedUser().getAccountId());
@@ -973,7 +973,7 @@
return;
}
changeInfos.stream()
- .forEach(c -> c.starred = starredChanges.contains(Change.id(c._virtualIdNumber)));
+ .forEach(c -> c.starred = starredChanges.contains(Change.id(c.virtualIdNumber)));
} catch (IOException e) {
logger.atWarning().withCause(e).log("Failed to open All-Users repo.");
}
diff --git a/java/com/google/gerrit/server/change/DeleteChangeOp.java b/java/com/google/gerrit/server/change/DeleteChangeOp.java
index ac75165..4ac27c1 100644
--- a/java/com/google/gerrit/server/change/DeleteChangeOp.java
+++ b/java/com/google/gerrit/server/change/DeleteChangeOp.java
@@ -125,7 +125,7 @@
}
private void cleanUpReferences(ChangeData cd) throws IOException {
- accountPatchReviewStore.run(s -> s.clearReviewed(cd.getId()));
+ accountPatchReviewStore.run(s -> s.clearReviewed(cd.virtualId()));
// Non-atomic operation on All-Users refs; not much we can do to make it atomic.
starredChangesUtil.unstarAllForChangeDeletion(cd.virtualId());
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 1199be5..84ea730 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -214,7 +214,7 @@
reviewerDeleted.fire(
ctx.getChangeData(currChange),
patchSet,
- accountCache.get(reviewer.id()).orElse(AccountState.forAccount(reviewer)),
+ accountCache.get(reviewer.id()).orElseGet(() -> AccountState.forAccount(reviewer)),
ctx.getAccount(),
mailMessage,
newApprovals,
diff --git a/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java b/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
index 4f6094e..58c3eb1 100644
--- a/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
+++ b/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
@@ -54,7 +54,7 @@
urlFormatter
.get()
.getChangeViewUrl(c.getProject(), c.getId())
- .orElse(c.getId().toString()));
+ .orElseGet(() -> c.getId().toString()));
}
protected String cropSubject(String subject) {
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 0f5e3bc..dbe0fa8 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2019,10 +2019,10 @@
magicBranch.dest = BranchNameKey.create(project.getNameKey(), ref);
magicBranch.perm = permissions.ref(ref);
- Optional<AuthException> err =
- checkRefPermission(magicBranch.perm, RefPermission.READ)
- .map(Optional::of)
- .orElse(checkRefPermission(magicBranch.perm, RefPermission.CREATE_CHANGE));
+ Optional<AuthException> err = checkRefPermission(magicBranch.perm, RefPermission.READ);
+ if (err.isEmpty()) {
+ err = checkRefPermission(magicBranch.perm, RefPermission.CREATE_CHANGE);
+ }
if (err.isPresent()) {
rejectProhibited(cmd, err.get());
return;
diff --git a/java/com/google/gerrit/server/group/db/AuditLogFormatter.java b/java/com/google/gerrit/server/group/db/AuditLogFormatter.java
index 235ca4f..3ba087e 100644
--- a/java/com/google/gerrit/server/group/db/AuditLogFormatter.java
+++ b/java/com/google/gerrit/server/group/db/AuditLogFormatter.java
@@ -167,7 +167,7 @@
.map(Account::getName)
// Historically, the database did not enforce relational integrity, so it is
// possible for groups to have non-existing members.
- .orElse("No Account for Id #" + accountId);
+ .orElseGet(() -> "No Account for Id #" + accountId);
}
private PersonIdent getParsableAuthorIdent(
diff --git a/java/com/google/gerrit/server/patch/PatchFile.java b/java/com/google/gerrit/server/patch/PatchFile.java
index 7a8180bd..c3a6807 100644
--- a/java/com/google/gerrit/server/patch/PatchFile.java
+++ b/java/com/google/gerrit/server/patch/PatchFile.java
@@ -61,7 +61,7 @@
.filter(f -> f.getKey().equals(fileName))
.map(Map.Entry::getValue)
.findFirst()
- .orElse(FileDiffOutput.empty(fileName, ObjectId.zeroId(), ObjectId.zeroId()));
+ .orElseGet(() -> FileDiffOutput.empty(fileName, ObjectId.zeroId(), ObjectId.zeroId()));
if (Patch.PATCHSET_LEVEL.equals(fileName)) {
aTree = null;
diff --git a/java/com/google/gerrit/server/project/Reachable.java b/java/com/google/gerrit/server/project/Reachable.java
index 342c2bc..c935adf 100644
--- a/java/com/google/gerrit/server/project/Reachable.java
+++ b/java/com/google/gerrit/server/project/Reachable.java
@@ -74,7 +74,7 @@
Collection<Ref> filtered =
optionalUserProvider
.map(permissionBackend::user)
- .orElse(permissionBackend.currentUser())
+ .orElseGet(() -> permissionBackend.currentUser())
.project(project)
.filter(refs, repo, RefFilterOptions.defaults());
Collection<RevCommit> visible = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index a69d837..82f85e9 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -1229,8 +1229,6 @@
mergeable = true;
} else if (c.isAbandoned()) {
return null;
- } else if (c.isWorkInProgress()) {
- return null;
} else {
if (!lazyload()) {
return null;
diff --git a/java/com/google/gerrit/server/query/change/PredicateArgs.java b/java/com/google/gerrit/server/query/change/PredicateArgs.java
index ebe4390..02d2ca6 100644
--- a/java/com/google/gerrit/server/query/change/PredicateArgs.java
+++ b/java/com/google/gerrit/server/query/change/PredicateArgs.java
@@ -71,7 +71,7 @@
*
* @param args arguments to be parsed
*/
- PredicateArgs(String args) throws QueryParseException {
+ public PredicateArgs(String args) throws QueryParseException {
positional = new ArrayList<>();
keyValue = new HashMap<>();
diff --git a/java/com/google/gerrit/server/restapi/project/CreateLabel.java b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
index ad32f4f..ad79a14 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
@@ -123,6 +123,7 @@
* @throws BadRequestException if there was invalid data in the input
* @throws ResourceConflictException if the label cannot be created due to a conflict
*/
+ @SuppressWarnings("deprecation")
public LabelType createLabel(ProjectConfig config, String label, LabelDefinitionInput input)
throws BadRequestException, ResourceConflictException {
if (config.getLabelSections().containsKey(label)) {
diff --git a/java/com/google/gerrit/server/restapi/project/SetLabel.java b/java/com/google/gerrit/server/restapi/project/SetLabel.java
index 10589cc..6c4976d 100644
--- a/java/com/google/gerrit/server/restapi/project/SetLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/SetLabel.java
@@ -115,6 +115,7 @@
* @throws BadRequestException if there was invalid data in the input
* @throws ResourceConflictException if the update cannot be applied due to a conflict
*/
+ @SuppressWarnings("deprecation")
public boolean updateLabel(ProjectConfig config, LabelType labelType, LabelDefinitionInput input)
throws BadRequestException, ResourceConflictException {
boolean dirty = false;
diff --git a/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java b/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java
new file mode 100644
index 0000000..5f09658
--- /dev/null
+++ b/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java
@@ -0,0 +1,44 @@
+// Copyright (C) 2024 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.sshd;
+
+import java.security.PublicKey;
+import java.security.spec.InvalidKeySpecException;
+
+public class InvalidKeyAlgorithmException extends InvalidKeySpecException {
+ private final String invalidKeyAlgo;
+ private final String expectedKeyAlgo;
+ private final PublicKey publicKey;
+
+ public InvalidKeyAlgorithmException(
+ String invalidKeyAlgo, String expectedKeyAlgo, PublicKey publicKey) {
+ super("Key algorithm mismatch: expected " + expectedKeyAlgo + " but got " + invalidKeyAlgo);
+ this.invalidKeyAlgo = invalidKeyAlgo;
+ this.expectedKeyAlgo = expectedKeyAlgo;
+ this.publicKey = publicKey;
+ }
+
+ public String getInvalidKeyAlgo() {
+ return invalidKeyAlgo;
+ }
+
+ public String getExpectedKeyAlgo() {
+ return expectedKeyAlgo;
+ }
+
+ public PublicKey getPublicKey() {
+ return publicKey;
+ }
+}
diff --git a/java/com/google/gerrit/sshd/SshKeyCacheImpl.java b/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
index 628a050..58e331b 100644
--- a/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
+++ b/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
@@ -19,6 +19,7 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.exceptions.InvalidSshKeyException;
import com.google.gerrit.server.account.AccountSshKey;
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -144,7 +145,16 @@
// to do with the key object, and instead we must abort this load.
//
throw e;
- } catch (Exception e) {
+ } catch (InvalidKeyAlgorithmException e) {
+ logger.atWarning().withCause(e).log(
+ "SSH key %d of account %s has an invalid algorithm %s: fixing the algorithm to %s",
+ k.seq(), k.accountId(), e.getInvalidKeyAlgo(), e.getExpectedKeyAlgo());
+ if (fixKeyAlgorithm(k, e.getExpectedKeyAlgo())) {
+ kl.add(new SshKeyCacheEntry(k.accountId(), e.getPublicKey()));
+ } else {
+ markInvalid(k);
+ }
+ } catch (Throwable e) {
markInvalid(k);
}
}
@@ -158,5 +168,20 @@
"Failed to mark SSH key %d of account %s invalid", k.seq(), k.accountId());
}
}
+
+ private boolean fixKeyAlgorithm(AccountSshKey k, String keyAlgo) {
+ try {
+ logger.atInfo().log(
+ "Fixing SSH key %d of account %s algorithm to %s", k.seq(), k.accountId(), keyAlgo);
+ authorizedKeys.deleteKey(k.accountId(), k.seq());
+ String sshKey = k.sshPublicKey();
+ authorizedKeys.addKey(k.accountId(), keyAlgo + sshKey.substring(sshKey.indexOf(' ')));
+ return true;
+ } catch (IOException | ConfigInvalidException | InvalidSshKeyException e) {
+ logger.atSevere().withCause(e).log(
+ "Failed to fix SSH key %d of account %s with algo %s", k.seq(), k.accountId(), keyAlgo);
+ return false;
+ }
+ }
}
}
diff --git a/java/com/google/gerrit/sshd/SshUtil.java b/java/com/google/gerrit/sshd/SshUtil.java
index abbd81d..29d0e90 100644
--- a/java/com/google/gerrit/sshd/SshUtil.java
+++ b/java/com/google/gerrit/sshd/SshUtil.java
@@ -57,7 +57,12 @@
throw new InvalidKeySpecException("No key string");
}
final byte[] bin = BaseEncoding.base64().decode(s);
- return new ByteArrayBuffer(bin).getRawPublicKey();
+ String publicKeyAlgo = new ByteArrayBuffer(bin).getString();
+ PublicKey publicKey = new ByteArrayBuffer(bin).getRawPublicKey();
+ if (!key.algorithm().equals(publicKeyAlgo)) {
+ throw new InvalidKeyAlgorithmException(key.algorithm(), publicKeyAlgo, publicKey);
+ }
+ return publicKey;
} catch (RuntimeException | SshException e) {
throw new InvalidKeySpecException("Cannot parse key", e);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index c9c5c2c..ceceaac 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4791,6 +4791,8 @@
public void changeQueryReturnsMergeableWhenGerritIndexMergeable() throws Exception {
String changeId = createChange().getChangeId();
assertThat(gApi.changes().query(changeId).get().get(0).mergeable).isTrue();
+ gApi.changes().id(changeId).setWorkInProgress();
+ assertThat(gApi.changes().query(changeId).get().get(0).mergeable).isTrue();
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index 2668d1f..6ba1498 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -1232,6 +1232,7 @@
.contains("Code-Review+1 by User");
}
+ @SuppressWarnings("deprecation")
@Test
public void sticky_copiedToLatestPatchSetFromSubmitRecords() throws Exception {
updateVerifiedLabel(b -> b.setFunction(LabelFunction.NO_BLOCK));
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index 242c278..c223562 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -1576,6 +1576,7 @@
// Clear SRs for the project and update code-review label to be non-blocking.
clearSubmitRequirements(project);
+ @SuppressWarnings("deprecation")
LabelType cr =
TestLabels.codeReview().toBuilder().setFunction(LabelFunction.NO_BLOCK).build();
try (ProjectConfigUpdate u = updateProject(project)) {
@@ -1622,6 +1623,7 @@
// Clear SRs for the project and update code-review label to be non-blocking.
clearSubmitRequirements(project);
+ @SuppressWarnings("deprecation")
LabelType cr =
TestLabels.codeReview().toBuilder().setFunction(LabelFunction.NO_BLOCK).build();
try (ProjectConfigUpdate u = updateProject(project)) {
@@ -2836,6 +2838,7 @@
.setAllowOverrideInChildProjects(true)
.build());
+ @SuppressWarnings("deprecation")
LabelType cr = TestLabels.codeReview().toBuilder().setFunction(LabelFunction.NO_BLOCK).build();
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig().upsertLabelType(cr);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java
index af95e7e..e7efb62 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java
@@ -98,6 +98,7 @@
assertThat(recordLabels).contains(needCustomLabel);
}
+ @SuppressWarnings("deprecation")
private void setupCustomBlockingLabel() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 0e4f212..f561a5a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -186,7 +186,7 @@
.create(
admin.newIdent(),
testRepo,
- "parent 2",
+ "parent 1",
ImmutableMap.of("foo", "foo-2", "bar", "bar-2"))
.to("refs/heads/master");
@@ -205,7 +205,7 @@
.create(
admin.newIdent(),
testRepo,
- "parent 1",
+ "parent 2",
ImmutableMap.of("foo", "foo-1", "bar", "bar-1"))
.to("refs/heads/stable");
@@ -564,6 +564,25 @@
}
@Test
+ public void submitParentIsWorkInProgressChange() throws Throwable {
+ PushOneCommit.Result parent = pushTo("refs/for/master%wip");
+ PushOneCommit.Result change = createChange();
+ Change.Id num = parent.getChange().getId();
+ if (getSubmitType() == SubmitType.CHERRY_PICK) {
+ submit(change.getChangeId());
+ } else {
+ submitWithConflict(
+ change.getChangeId(),
+ "Failed to submit 2 changes due to the following problems:\n"
+ + "Change "
+ + num
+ + ": Change "
+ + num
+ + " is work in progress");
+ }
+ }
+
+ @Test
public void submitWithHiddenBranchInSameTopic() throws Throwable {
assume().that(isSubmitWholeTopicEnabled()).isTrue();
PushOneCommit.Result visible = createChange("refs/for/master%topic=" + name("topic"));
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index ea52690..fe7f935 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -2566,8 +2566,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body()).doesNotContain("\nPatch Set 2: Code-Review+2\n");
assertThat(message.body())
.contains("The change is no longer submittable: Code-Review is unsatisfied now.\n");
@@ -2652,8 +2652,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body()).doesNotContain("\nPatch Set 2: Code-Review+2\n");
assertThat(message.body()).contains("\nPatch Set 2: Code-Review+1\n");
assertThat(message.body())
@@ -2740,8 +2740,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body()).contains("\nPatch Set 2: Code-Review-2\n");
assertThat(message.body())
.contains("The change is no longer submittable: Code-Review is unsatisfied now.\n");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeNoLongerSubmittableIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeNoLongerSubmittableIT.java
index 1094a42..5bb6dc4 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeNoLongerSubmittableIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeNoLongerSubmittableIT.java
@@ -83,8 +83,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body())
.contains("The change is no longer submittable: Code-Review is unsatisfied now.\n");
assertThat(message.htmlBody())
@@ -127,8 +127,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body())
.contains("The change is no longer submittable: Code-Review is unsatisfied now.\n");
assertThat(message.htmlBody())
@@ -172,8 +172,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body())
.contains("The change is no longer submittable: Code-Review is unsatisfied now.\n");
assertThat(message.htmlBody())
@@ -263,8 +263,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body())
.contains(
"The change is no longer submittable:"
@@ -315,8 +315,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body()).doesNotContain("The change is no longer submittable");
assertThat(message.htmlBody()).doesNotContain("The change is no longer submittable");
}
@@ -361,8 +361,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body()).doesNotContain("The change is no longer submittable");
assertThat(message.htmlBody()).doesNotContain("The change is no longer submittable");
}
@@ -398,8 +398,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body()).doesNotContain("The change is no longer submittable");
assertThat(message.htmlBody()).doesNotContain("The change is no longer submittable");
}
@@ -435,8 +435,8 @@
String.format(
"Attention is currently required from: %s, %s.\n"
+ "\n"
- + "%s has posted comments on this change.",
- admin.fullName(), user.fullName(), approver.fullName()));
+ + "%s has posted comments on this change by %s.",
+ admin.fullName(), user.fullName(), approver.fullName(), admin.fullName()));
assertThat(message.body()).doesNotContain("The change is no longer submittable");
assertThat(message.htmlBody()).doesNotContain("The change is no longer submittable");
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index 647e26b..985baba 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -402,7 +402,9 @@
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
- assertThat(m.body()).contains(admin.fullName() + " has posted comments on this change.");
+ assertThat(m.body())
+ .contains(
+ admin.fullName() + " has posted comments on this change by " + admin.fullName() + ".");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertThat(m.body()).contains("Patch Set 1: Code-Review+2");
diff --git a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index 576c7d0..7ffc0a4 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -76,6 +76,7 @@
.update();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelNoOp_NegativeVoteNotBlock() throws Exception {
saveLabelConfig(LABEL.toBuilder().setFunction(NO_OP));
@@ -91,6 +92,7 @@
assertThat(q.blocking).isNull();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelNoBlock_NegativeVoteNotBlock() throws Exception {
saveLabelConfig(LABEL.toBuilder().setFunction(NO_BLOCK));
@@ -106,6 +108,7 @@
assertThat(q.blocking).isNull();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelMaxNoBlock_NegativeVoteNotBlock() throws Exception {
saveLabelConfig(LABEL.toBuilder().setFunction(MAX_NO_BLOCK));
@@ -121,6 +124,7 @@
assertThat(q.blocking).isNull();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelMaxNoBlock_MaxVoteSubmittable() throws Exception {
saveLabelConfig(LABEL.toBuilder().setFunction(MAX_NO_BLOCK), P.toBuilder().setFunction(NO_OP));
@@ -139,6 +143,7 @@
assertThat(q.blocking).isNull();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelAnyWithBlock_NegativeVoteBlock() throws Exception {
saveLabelConfig(LABEL.toBuilder().setFunction(ANY_WITH_BLOCK));
@@ -163,6 +168,7 @@
}
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelAnyWithBlock_Addreviewer_ZeroVote() throws Exception {
TestListener testListener = new TestListener();
@@ -190,6 +196,7 @@
}
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelMaxWithBlock_NegativeVoteBlock() throws Exception {
saveLabelConfig(LABEL.toBuilder().setFunction(MAX_WITH_BLOCK));
@@ -205,6 +212,7 @@
assertThat(q.blocking).isTrue();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelMaxWithBlock_DeletedVoteDoesNotTriggerNegativeBlock() throws Exception {
saveLabelConfig(P.toBuilder().setFunction(MAX_WITH_BLOCK));
@@ -227,6 +235,7 @@
assertThat(labelInfo.blocking).isNull(); // label is not blocking the change submission
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelMaxWithBlock_MaxVoteSubmittable() throws Exception {
saveLabelConfig(
@@ -246,6 +255,7 @@
assertThat(q.blocking).isNull();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabelMaxWithBlock_MaxVoteNegativeVoteBlock() throws Exception {
saveLabelConfig(LABEL.toBuilder().setFunction(MAX_WITH_BLOCK));
@@ -262,6 +272,7 @@
assertThat(q.blocking).isTrue();
}
+ @SuppressWarnings("deprecation")
@Test
public void customLabel_DisallowPostSubmit() throws Exception {
saveLabelConfig(
diff --git a/javatests/com/google/gerrit/server/group/db/AbstractGroupTest.java b/javatests/com/google/gerrit/server/group/db/AbstractGroupTest.java
index bea5eaa..a9ceddb 100644
--- a/javatests/com/google/gerrit/server/group/db/AbstractGroupTest.java
+++ b/javatests/com/google/gerrit/server/group/db/AbstractGroupTest.java
@@ -141,7 +141,7 @@
return GroupConfig.loadForGroup(allUsersName, allUsersRepo, uuid)
.getLoadedGroup()
.map(InternalGroup::getName)
- .orElse("Group " + uuid);
+ .orElseGet(() -> "Group " + uuid);
} catch (IOException | ConfigInvalidException e) {
return "Group " + uuid;
}
diff --git a/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java b/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
index b05f3c7..180dd92 100644
--- a/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
+++ b/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
@@ -40,6 +40,7 @@
@Before
public void setup() {
+ @SuppressWarnings("deprecation")
LabelType codeReview =
LabelType.builder(
"Code-Review",
@@ -50,6 +51,7 @@
.setFunction(LabelFunction.MAX_WITH_BLOCK)
.build();
+ @SuppressWarnings("deprecation")
LabelType verified =
LabelType.builder(
"Verified",
@@ -60,6 +62,7 @@
.setFunction(LabelFunction.MAX_NO_BLOCK)
.build();
+ @SuppressWarnings("deprecation")
LabelType codeStyle =
LabelType.builder(
"Code-Style",
@@ -70,6 +73,7 @@
.setFunction(LabelFunction.ANY_WITH_BLOCK)
.build();
+ @SuppressWarnings("deprecation")
LabelType ignoreSelfApprovalLabel =
LabelType.builder(
"ISA-Label",
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index b1faf03..e1f324f 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -25,6 +25,8 @@
import static org.junit.Assume.assumeTrue;
import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -83,12 +85,49 @@
AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
newQuery("status:new").withLimit(5).get();
+ // Since the limit of the query (i.e. 5) is more than the total number of changes (i.e. 4),
+ // only 1 index search is expected.
assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
}
@Test
@UseClockStep
@SuppressWarnings("unchecked")
+ public void queryRightNumberOfTimes() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ Account.Id user2 =
+ accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
+
+ // create 1 visible change
+ Change visibleChange1 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+
+ // create 4 private changes
+ Change invisibleChange2 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW), user2);
+ Change invisibleChange3 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW), user2);
+ Change invisibleChange4 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW), user2);
+ Change invisibleChange5 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW), user2);
+ gApi.changes().id(invisibleChange2.getChangeId()).setPrivate(true, null);
+ gApi.changes().id(invisibleChange3.getChangeId()).setPrivate(true, null);
+ gApi.changes().id(invisibleChange4.getChangeId()).setPrivate(true, null);
+ gApi.changes().id(invisibleChange5.getChangeId()).setPrivate(true, null);
+
+ AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
+ idx.resetQueryCount();
+ List<ChangeInfo> queryResult = newQuery("status:new").withLimit(2).get();
+ assertThat(queryResult).hasSize(1);
+ assertThat(queryResult.get(0).changeId).isEqualTo(visibleChange1.getKey().get());
+
+ // Since the limit of the query (i.e. 2), 2 index searches are expected in fact:
+ // 1: The first query will return invisibleChange5, invisibleChange4 and invisibleChange3,
+ // 2: Another query is needed to back-fill the limit requested by the user.
+ // even if one result in the second query is skipped because it is not visible,
+ // there are no more results to query.
+ assertThatSearchQueryWasPaginated(idx.getQueryCount(), 2);
+ }
+
+ @Test
+ @UseClockStep
+ @SuppressWarnings("unchecked")
public void noLimitQueryPaginates() throws Exception {
assumeFalse(PaginationType.NONE == getCurrentPaginationType());
@@ -123,37 +162,7 @@
@Test
@UseClockStep
- public void invisibleChangesNotPaginatedWithNonePaginationType() throws Exception {
- assumeTrue(PaginationType.NONE == getCurrentPaginationType());
- AbstractFakeIndex idx = setupRepoWithFourChanges();
- final int LIMIT = 3;
-
- projectOperations
- .project(allProjectsName)
- .forUpdate()
- .removeAllAccessSections()
- .add(allow(Permission.READ).ref("refs/*").group(SystemGroupBackend.REGISTERED_USERS))
- .update();
-
- // Set queryLimit to 3
- projectOperations
- .project(allProjects)
- .forUpdate()
- .add(allowCapability(QUERY_LIMIT).group(ANONYMOUS_USERS).range(0, LIMIT))
- .update();
-
- requestContext.setContext(anonymousUserProvider::get);
- List<ChangeInfo> result = newQuery("status:new").withLimit(LIMIT).get();
- assertThat(result.size()).isEqualTo(0);
- assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
- assertThat(idx.getResultsSizes().get(0)).isEqualTo(LIMIT + 1);
- }
-
- @Test
- @UseClockStep
public void invisibleChangesPaginatedWithPagination() throws Exception {
- assumeFalse(PaginationType.NONE == getCurrentPaginationType());
-
AbstractFakeIndex idx = setupRepoWithFourChanges();
final int LIMIT = 3;
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
index 9717bfb..54d0c87 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.server.config.AllProjectsName;
@@ -98,4 +99,27 @@
Change[] expected = new Change[] {change6, change5, change4, change3, change2, change1};
assertQuery(newQuery("project:repo").withNoLimit(), expected);
}
+
+ @Test
+ public void skipChangesNotVisible() throws Exception {
+ // create 1 new change on a repo
+ TestRepository<Repo> repo = createProject("repo");
+ Change visibleChange = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+ Change[] expected = new Change[] {visibleChange};
+
+ // pagination does not need to restart the datasource, the request is fulfilled
+ assertQuery(newQuery("status:new").withLimit(1), expected);
+
+ // create 2 new private changes
+ Account.Id user2 =
+ accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
+
+ Change invisibleChange1 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW), user2);
+ Change invisibleChange2 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW), user2);
+ gApi.changes().id(invisibleChange1.getChangeId()).setPrivate(true, null);
+ gApi.changes().id(invisibleChange2.getChangeId()).setPrivate(true, null);
+
+ // pagination should back-fill when the results skipped because of the visibility
+ assertQuery(newQuery("status:new").withLimit(1), expected);
+ }
}
diff --git a/javatests/com/google/gerrit/sshd/BUILD b/javatests/com/google/gerrit/sshd/BUILD
index 3e11ff2..44b9c62 100644
--- a/javatests/com/google/gerrit/sshd/BUILD
+++ b/javatests/com/google/gerrit/sshd/BUILD
@@ -4,8 +4,11 @@
name = "sshd_tests",
srcs = glob(["**/*.java"]),
deps = [
+ "//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/server",
"//java/com/google/gerrit/sshd",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
"//lib/mina:sshd",
"//lib/truth",
],
diff --git a/javatests/com/google/gerrit/sshd/SshUtilTest.java b/javatests/com/google/gerrit/sshd/SshUtilTest.java
new file mode 100644
index 0000000..1585bc3
--- /dev/null
+++ b/javatests/com/google/gerrit/sshd/SshUtilTest.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2024 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.sshd;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.account.AccountSshKey;
+import java.security.spec.InvalidKeySpecException;
+import org.junit.Test;
+
+public class SshUtilTest {
+ private static final Account.Id TEST_ACCOUNT_ID = Account.id(1);
+ private static final int TEST_SSHKEY_SEQUENCE = 1;
+ private static final String INVALID_ALGO = "invalid-algo";
+ private static final String VALID_OPENSSH_RSA_KEY =
+ "AAAAB3NzaC1yc2EAAAABIwAAAIEA0R66EoZ7hFp81w9sAJqu34UFyE+w36H/mobUqnT5Lns7PcTOJh3sgMJAlswX2lFAWqvF2gd2PRMpMhbfEU4iq2SfY8x+RDCJ4ZQWESln/587T41BlQjOXzu3W1bqgmtHnRCte3DjyWDvM/fucnUMSwOgP+FVEZCLTrk3thLMWsU=";
+ private static final Object VALID_SSH_RSA_ALGO = "ssh-rsa";
+
+ @Test
+ public void shouldFailParsingOpenSshKeyWithInvalidAlgo() {
+ String sshKeyWithInvalidAlgo = String.format("%s %s", INVALID_ALGO, VALID_OPENSSH_RSA_KEY);
+ AccountSshKey sshKey =
+ AccountSshKey.create(TEST_ACCOUNT_ID, TEST_SSHKEY_SEQUENCE, sshKeyWithInvalidAlgo);
+ assertThrows(InvalidKeySpecException.class, () -> SshUtil.parse(sshKey));
+ }
+
+ @Test
+ public void shouldParseSshKeyWithAlgoMatchingKey() {
+ String sshKeyWithValidKeyAlgo =
+ String.format("%s %s", VALID_SSH_RSA_ALGO, VALID_OPENSSH_RSA_KEY);
+ AccountSshKey sshKey =
+ AccountSshKey.create(TEST_ACCOUNT_ID, TEST_SSHKEY_SEQUENCE, sshKeyWithValidKeyAlgo);
+ assertThat(sshKey).isNotNull();
+ }
+}
diff --git a/modules/jgit b/modules/jgit
index acf21c0..c0b415f 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit acf21c0bc6a63a3d20fca92757b992a1f2d55f41
+Subproject commit c0b415fb028b4c1f29b6df749323bbb11599495d
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 848948f..e1604cf 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -495,7 +495,7 @@
return html`
<div class="label ${status}">
<span>${label} ${valueStr}</span>
- <paper-tooltip offset="5" ?fitToVisibleBounds=${true}>
+ <paper-tooltip offset="5" .fitToVisibleBounds=${true}>
The check result has (probably) influenced this label vote.
</paper-tooltip>
</div>
@@ -607,7 +607,7 @@
@click=${(e: MouseEvent) => this.tagClick(e, tag.name)}
>
<span>${tag.name}</span>
- <paper-tooltip offset="5" ?fitToVisibleBounds=${true}>
+ <paper-tooltip offset="5" .fitToVisibleBounds=${true}>
${tag.tooltip ??
'A category tag for this check result. Click to filter.'}
</paper-tooltip>
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts b/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
index 934e958..15176b0 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
@@ -45,12 +45,7 @@
/* HTML */ `
<div class="approved label">
<span> test-label +1 </span>
- <paper-tooltip
- fittovisiblebounds=""
- offset="5"
- role="tooltip"
- tabindex="-1"
- >
+ <paper-tooltip offset="5" role="tooltip" tabindex="-1">
The check result has (probably) influenced this label vote.
</paper-tooltip>
</div>
@@ -92,7 +87,6 @@
<button class="tag">
<span> OBSOLETE </span>
<paper-tooltip
- fittovisiblebounds=""
offset="5"
role="tooltip"
tabindex="-1"
@@ -103,7 +97,6 @@
<button class="tag">
<span> E2E </span>
<paper-tooltip
- fittovisiblebounds=""
offset="5"
role="tooltip"
tabindex="-1"
diff --git a/resources/com/google/gerrit/server/mail/Comment.soy b/resources/com/google/gerrit/server/mail/Comment.soy
index 98ab4b2..6a67398 100644
--- a/resources/com/google/gerrit/server/mail/Comment.soy
+++ b/resources/com/google/gerrit/server/mail/Comment.soy
@@ -29,7 +29,7 @@
{@param unsatisfiedSubmitRequirements: ?}
{@param oldSubmitRequirements: ?}
{@param newSubmitRequirements: ?}
- {$fromName} has posted comments on this change.
+ {$fromName} has posted comments on this change by {$change.ownerName}.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{if $unsatisfiedSubmitRequirements}
{\n}
diff --git a/tools/deps.bzl b/tools/deps.bzl
index 9a17ca8..5ceae59 100644
--- a/tools/deps.bzl
+++ b/tools/deps.bzl
@@ -115,15 +115,15 @@
maven_jar(
name = "commons-codec",
- artifact = "commons-codec:commons-codec:1.10",
- sha1 = "4b95f4897fa13f2cd904aee711aeafc0c5295cd8",
+ artifact = "commons-codec:commons-codec:1.15",
+ sha1 = "49d94806b6e3dc933dacbd8acb0fdbab8ebd1e5d",
)
# When upgrading commons-compress, also upgrade tukaani-xz
maven_jar(
name = "commons-compress",
- artifact = "org.apache.commons:commons-compress:1.22",
- sha1 = "691a8b4e6cf4248c3bc72c8b719337d5cb7359fa",
+ artifact = "org.apache.commons:commons-compress:1.25.0",
+ sha1 = "9d35aec423da6c8a7f93d7e9e1c6b1d9fe14bb5e",
)
maven_jar(
