Merge branch 'stable-3.12'
* stable-3.12:
Revert "Invoke changeServerId() function when calculating virtual Id"
Update git submodules
Fix ref existance check in CREATE ReceiveCommits
Add "@lezer/highlight" package
Find changes by change number only if imported server IDs are configured
AbtractFakeIndex: use changenumber instead of legacy_is_str
Update git submodules
Disable robot comments by default
Revert "Lookup imported change by change number in ChangeFinder::find"
Set version to 3.12.0-SNAPSHOT
Set version to 3.12.0-rc1
GitFileDiffCacheImpl: cancel tasks failing due to timeout or interrupt
Update git submodules
feat(Get AI Fix): Attach fixSuggestion to automatic reply when applying Ai Fix
Disable format warning on current editing line
AccountIndexerImpl: explicitly use AccountCacheImpl
Remove net.i2p.crypto:eddsa
Allow opening edit preference in editor view
Respect the `enableRobotComments` server config in the UI
Set version to 3.12.0-SNAPSHOT
Set version to 3.12.0-rc0
Set version to 3.11.3-SNAPSHOT
Set version to 3.11.2
Set version to 3.10.6-SNAPSHOT
Set version to 3.10.5
Set version to 3.9.11-SNAPSHOT
Set version to 3.9.10
Release-Notes: skip
Change-Id: Ied6c827d57a69adc1b6759104a57480ada9dcf0c
diff --git a/Documentation/config-validation.txt b/Documentation/config-validation.txt
index b0149fe..8b2c9d4 100644
--- a/Documentation/config-validation.txt
+++ b/Documentation/config-validation.txt
@@ -6,7 +6,6 @@
[[new-commit-validation]]
== New commit validation
-
Plugins implementing the `CommitValidationListener` interface can
perform additional validation checks against new commits.
@@ -21,6 +20,13 @@
Out of the box, Gerrit includes a plugin that checks the length of the
subject and body lines of commit messages on uploaded commits.
+[[push-options-validation]]
+== Push options validation
+
+Plugins implementing the `PushOptionsValidator` interface can validate push
+options. For example, they can reject options (or a combination of options) or
+emit a warning when a deprecated option is being used.
+
[plugin-push-options]]
=== Plugin push options
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 596ee96..2d48d81 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -8757,8 +8757,7 @@
|=================================
|Field Name ||Description
|`message` |optional|
-Message to be added as review comment to the change when reverting the
-change.
+Commit message of the revert commit. If not specified, a default commit message is set.
|`notify` |optional|
Notify handling that defines to whom email notifications should be sent
for reverting the change. +
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index 7cc9e29..c5bee19 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -219,6 +219,20 @@
Additional options may be specified when pushing changes.
+[[custom_keyed_values]]
+==== Custom Keyed Values
+
+Uploaders can specify custom keyed values for the change using this push option.
+The value can be set more than once to apply multiple custom keyed values.
+Each key,value pair must be separate by a colon (':') otherwise the push fails.
+
+----
+ git push ssh://bot@git.example.com:29418/kernel/common HEAD:refs/for/master -o custom-keyed-value=foo:bar custom-keyed-value=hello:world
+----
+
+Note: Setting the `custom-keyed-value` option in the refname
+(`%custom-keyed-values=foo:bar`) is **not** supported.
+
[[notify]]
==== Email Notifications
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index f094b48..4cbaea2 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -53,6 +53,7 @@
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.git.ChangeMessageModifier;
+import com.google.gerrit.server.git.receive.PushOptionsValidator;
import com.google.gerrit.server.git.validators.CommitValidationInfoListener;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
@@ -118,6 +119,7 @@
private final DynamicSet<ValidationOptionsListener> validationOptionsListeners;
private final DynamicSet<CommitValidationInfoListener> commitValidationInfoListeners;
private final DynamicSet<RetryListener> retryListeners;
+ private final DynamicSet<PushOptionsValidator> pushOptionsValidators;
private final DynamicMap<ChangeHasOperandFactory> hasOperands;
private final DynamicMap<ChangeIsOperandFactory> isOperands;
@@ -172,6 +174,7 @@
DynamicSet<ValidationOptionsListener> validationOptionsListeners,
DynamicSet<CommitValidationInfoListener> commitValidationInfoListeners,
DynamicSet<RetryListener> retryListeners,
+ DynamicSet<PushOptionsValidator> pushOptionsValidator,
DynamicMap<ReviewerSuggestion> reviewerSuggestions) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
@@ -219,6 +222,7 @@
this.validationOptionsListeners = validationOptionsListeners;
this.commitValidationInfoListeners = commitValidationInfoListeners;
this.retryListeners = retryListeners;
+ this.pushOptionsValidators = pushOptionsValidator;
this.reviewerSuggestions = reviewerSuggestions;
}
@@ -432,6 +436,11 @@
}
@CanIgnoreReturnValue
+ public Registration add(PushOptionsValidator pushOptionsValidator) {
+ return add(pushOptionsValidators, pushOptionsValidator);
+ }
+
+ @CanIgnoreReturnValue
public Registration add(CapabilityDefinition capabilityDefinition, String exportName) {
return add(capabilityDefinitions, capabilityDefinition, exportName);
}
diff --git a/java/com/google/gerrit/entities/Permission.java b/java/com/google/gerrit/entities/Permission.java
index 1f2f151..4d926b8 100644
--- a/java/com/google/gerrit/entities/Permission.java
+++ b/java/com/google/gerrit/entities/Permission.java
@@ -31,7 +31,9 @@
public abstract class Permission implements Comparable<Permission> {
public static final String ABANDON = "abandon";
public static final String ADD_PATCH_SET = "addPatchSet";
+ // https://gerrit-review.googlesource.com/Documentation/access-control.html#category_create
public static final String CREATE = "create";
+ // https://gerrit-review.googlesource.com/Documentation/access-control.html#category_create_signed
public static final String CREATE_SIGNED_TAG = "createSignedTag";
public static final String CREATE_TAG = "createTag";
public static final String DELETE = "delete";
@@ -47,8 +49,10 @@
public static final String LABEL_AS = "labelAs-";
public static final String REMOVE_LABEL = "removeLabel-";
public static final String OWNER = "owner";
+ // https://gerrit-review.googlesource.com/Documentation/access-control.html#category_push
public static final String PUSH = "push";
public static final String PUSH_MERGE = "pushMerge";
+ // https://gerrit-review.googlesource.com/Documentation/access-control.html#category_read
public static final String READ = "read";
public static final String REBASE = "rebase";
public static final String REMOVE_REVIEWER = "removeReviewer";
diff --git a/java/com/google/gerrit/extensions/api/projects/BranchInput.java b/java/com/google/gerrit/extensions/api/projects/BranchInput.java
index 1ccbd4f..63a118a 100644
--- a/java/com/google/gerrit/extensions/api/projects/BranchInput.java
+++ b/java/com/google/gerrit/extensions/api/projects/BranchInput.java
@@ -17,6 +17,35 @@
import com.google.gerrit.extensions.restapi.DefaultInput;
import java.util.Map;
+/**
+ * Input for creating a new branch.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#branch-input
+ *
+ * <p>This class holds the data needed to create a new branch, such as the revision to base the
+ * branch on and whether to create an empty commit.
+ *
+ * <p>The following properties can be set:
+ *
+ * <ul>
+ * <li>{@code revision}: The base revision of the new branch. If not set and create_empty_commit
+ * is true the branch is created with an empty initial commit. If not set and
+ * create_empty_commit is false or unset HEAD will be used as base revision.
+ * <li>{@code createEmptyCommit}: Whether the branch should be created with an empty initial
+ * commit. It is not possible to create a branch on a project with no commits. Cannot be used
+ * in combination with setting a revision. Can be used to review the initial content of a
+ * branch (create the branch with an empty initial commit, make a second commit with the
+ * initial content, e.g. by merging in another branch, and push the commit for review)..
+ * <li>{@code ref}: The name of the branch. The prefix refs/heads/ can be omitted. If set, must
+ * match the branch ID in the URL.
+ * <li>{@code validationOptions}: Map with key-value pairs that are forwarded as options to the
+ * ref operation validation listeners (e.g. can be used to skip certain validations). Which
+ * validation options are supported depends on the installed ref operation validation
+ * listeners. Gerrit core doesn’t support any validation options, but ref operation validation
+ * listeners that are implemented in plugins may. Please refer to the documentation of the
+ * installed plugins to learn whether they support validation options. Unknown validation
+ * options are silently ignored.
+ * </ul>
+ */
public class BranchInput {
@DefaultInput public String revision;
public boolean createEmptyCommit;
diff --git a/java/com/google/gerrit/server/account/AccountDelta.java b/java/com/google/gerrit/server/account/AccountDelta.java
index 14ced50..192d9d3 100644
--- a/java/com/google/gerrit/server/account/AccountDelta.java
+++ b/java/com/google/gerrit/server/account/AccountDelta.java
@@ -20,6 +20,8 @@
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.common.UsedAt.Project;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.NotifyConfig.NotifyType;
import com.google.gerrit.entities.ProjectWatchKey;
@@ -177,6 +179,11 @@
|| !this.getUpdatedExternalIds().isEmpty();
}
+ @UsedAt(Project.GOOGLE)
+ public boolean isEmpty() {
+ return this.equals(AccountDelta.builder().build());
+ }
+
/**
* Class to build an {@link AccountDelta}.
*
diff --git a/java/com/google/gerrit/server/change/ChangeMessages.java b/java/com/google/gerrit/server/change/ChangeMessages.java
index 787f036..c228ebe 100644
--- a/java/com/google/gerrit/server/change/ChangeMessages.java
+++ b/java/com/google/gerrit/server/change/ChangeMessages.java
@@ -14,25 +14,22 @@
package com.google.gerrit.server.change;
-import org.eclipse.jgit.nls.NLS;
-import org.eclipse.jgit.nls.TranslationBundle;
+public class ChangeMessages {
+ public static String revertChangeDefaultMessage = "Revert \"{0}\"\n\nThis reverts commit {1}.";
+ public static String revertSubmissionDefaultMessage = "This reverts commit {0}.";
+ public static String revertSubmissionUserMessage = "Revert \"{0}\"\n\n{1}";
+ public static String revertSubmissionOfRevertSubmissionUserMessage = "Revert^{0} \"{1}\"\n\n{2}";
-public class ChangeMessages extends TranslationBundle {
- public static ChangeMessages get() {
- return NLS.getBundleFor(ChangeMessages.class);
- }
+ public static String reviewerCantSeeChange = "{0} does not have permission to see this change";
+ public static String reviewerInvalid = "{0} is not a valid user identifier";
+ public static String reviewerNotFoundUserOrGroup =
+ "{0} does not identify a registered user or group";
- public String revertChangeDefaultMessage;
- public String revertSubmissionDefaultMessage;
- public String revertSubmissionUserMessage;
- public String revertSubmissionOfRevertSubmissionUserMessage;
-
- public String reviewerCantSeeChange;
- public String reviewerInvalid;
- public String reviewerNotFoundUserOrGroup;
-
- public String groupRemovalIsNotAllowed;
- public String groupIsNotAllowed;
- public String groupHasTooManyMembers;
- public String groupManyMembersConfirmation;
+ public static String groupRemovalIsNotAllowed =
+ "Groups can't be removed from reviewers, so can't remove {0}.";
+ public static String groupIsNotAllowed = "The group {0} cannot be added as reviewer.";
+ public static String groupHasTooManyMembers =
+ "The group {0} has too many members to add them all as reviewers.";
+ public static String groupManyMembersConfirmation =
+ "The group {0} has {1} members. Do you want to add them all as reviewers?";
}
diff --git a/java/com/google/gerrit/server/change/ReviewerModifier.java b/java/com/google/gerrit/server/change/ReviewerModifier.java
index 09bfcd3..6d454ae 100644
--- a/java/com/google/gerrit/server/change/ReviewerModifier.java
+++ b/java/com/google/gerrit/server/change/ReviewerModifier.java
@@ -314,7 +314,7 @@
return fail(
input,
FailureType.OTHER,
- MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, input.reviewer));
+ MessageFormat.format(ChangeMessages.reviewerCantSeeChange, input.reviewer));
}
@Nullable
@@ -340,7 +340,7 @@
return fail(
input,
FailureType.NOT_FOUND,
- MessageFormat.format(ChangeMessages.get().reviewerNotFoundUserOrGroup, input.reviewer));
+ MessageFormat.format(ChangeMessages.reviewerNotFoundUserOrGroup, input.reviewer));
}
return null;
}
@@ -349,14 +349,14 @@
return fail(
input,
FailureType.OTHER,
- MessageFormat.format(ChangeMessages.get().groupIsNotAllowed, group.getName()));
+ MessageFormat.format(ChangeMessages.groupIsNotAllowed, group.getName()));
}
if (input.state().equals(REMOVED)) {
return fail(
input,
FailureType.OTHER,
- MessageFormat.format(ChangeMessages.get().groupRemovalIsNotAllowed, group.getName()));
+ MessageFormat.format(ChangeMessages.groupRemovalIsNotAllowed, group.getName()));
}
Set<Account> reviewers = new HashSet<>();
@@ -376,7 +376,7 @@
return fail(
input,
FailureType.OTHER,
- MessageFormat.format(ChangeMessages.get().groupHasTooManyMembers, group.getName()));
+ MessageFormat.format(ChangeMessages.groupHasTooManyMembers, group.getName()));
}
// if maxWithoutCheck is set to 0, we never ask for confirmation
@@ -391,7 +391,7 @@
FailureType.OTHER,
true,
MessageFormat.format(
- ChangeMessages.get().groupManyMembersConfirmation, group.getName(), members.size()));
+ ChangeMessages.groupManyMembersConfirmation, group.getName(), members.size()));
}
for (Account member : members) {
@@ -413,7 +413,7 @@
return fail(
input,
FailureType.OTHER,
- MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, input.reviewer));
+ MessageFormat.format(ChangeMessages.reviewerCantSeeChange, input.reviewer));
}
Address adr = Address.tryParse(input.reviewer);
@@ -421,7 +421,7 @@
return fail(
input,
FailureType.NOT_FOUND,
- MessageFormat.format(ChangeMessages.get().reviewerInvalid, input.reviewer));
+ MessageFormat.format(ChangeMessages.reviewerInvalid, input.reviewer));
}
return new ReviewerModification(input, notes, user, null, ImmutableList.of(adr), true, false);
}
diff --git a/java/com/google/gerrit/server/config/CapabilityConstants.java b/java/com/google/gerrit/server/config/CapabilityConstants.java
index 43c4933..9448725 100644
--- a/java/com/google/gerrit/server/config/CapabilityConstants.java
+++ b/java/com/google/gerrit/server/config/CapabilityConstants.java
@@ -14,37 +14,30 @@
package com.google.gerrit.server.config;
-import org.eclipse.jgit.nls.NLS;
-import org.eclipse.jgit.nls.TranslationBundle;
-
-public class CapabilityConstants extends TranslationBundle {
- public static CapabilityConstants get() {
- return NLS.getBundleFor(CapabilityConstants.class);
- }
-
- public String accessDatabase;
- public String administrateServer;
- public String batchChangesLimit;
- public String createAccount;
- public String createGroup;
- public String deleteGroup;
- public String createProject;
- public String emailReviewers;
- public String flushCaches;
- public String killTask;
- public String maintainServer;
- public String modifyAccount;
- public String priority;
- public String readAs;
- public String queryLimit;
- public String runAs;
- public String runGC;
- public String streamEvents;
- public String viewAccess;
- public String viewAllAccounts;
- public String viewCaches;
- public String viewConnections;
- public String viewPlugins;
- public String viewQueue;
- public String viewSecondaryEmails;
+public class CapabilityConstants {
+ public static String accessDatabase = "Access Database";
+ public static String administrateServer = "Administrate Server";
+ public static String batchChangesLimit = "Batch Changes Limit";
+ public static String createAccount = "Create Account";
+ public static String createGroup = "Create Group";
+ public static String deleteGroup = "Delete Group";
+ public static String createProject = "Create Project";
+ public static String emailReviewers = "Email Reviewers";
+ public static String flushCaches = "Flush Caches";
+ public static String killTask = "Kill Task";
+ public static String maintainServer = "Maintain Server";
+ public static String modifyAccount = "Modify Account";
+ public static String priority = "Priority";
+ public static String readAs = "Read As";
+ public static String queryLimit = "Query Limit";
+ public static String runAs = "Run As";
+ public static String runGC = "Run Garbage Collection";
+ public static String streamEvents = "Stream Events";
+ public static String viewAccess = "View Access";
+ public static String viewAllAccounts = "View All Accounts";
+ public static String viewCaches = "View Caches";
+ public static String viewConnections = "View Connections";
+ public static String viewPlugins = "View Plugins";
+ public static String viewQueue = "View Queue";
+ public static String viewSecondaryEmails = "View Secondary Emails";
}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 3d480d9..d5fa605 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -145,6 +145,7 @@
import com.google.gerrit.server.git.ReceivePackInitializer;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TransferConfig;
+import com.google.gerrit.server.git.receive.PushOptionsValidator;
import com.google.gerrit.server.git.receive.ReceiveCommitsModule;
import com.google.gerrit.server.git.validators.CommentCountValidator;
import com.google.gerrit.server.git.validators.CommentCumulativeSizeValidator;
@@ -440,6 +441,7 @@
DynamicSet.setOf(binder(), ExternalIncludedIn.class);
DynamicSet.setOf(binder(), FilterIncludedIn.class);
DynamicMap.mapOf(binder(), ProjectConfigEntry.class);
+ DynamicSet.setOf(binder(), PushOptionsValidator.class);
DynamicSet.setOf(binder(), PluginPushOption.class);
DynamicSet.setOf(binder(), PatchSetWebLink.class);
DynamicSet.setOf(binder(), ResolveConflictsWebLink.class);
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
index cd91745..eb81ee1 100644
--- a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
@@ -60,4 +60,12 @@
/** Whether we allow fix suggestions in HumanComments. */
public static final String ALLOW_FIX_SUGGESTIONS_IN_COMMENTS =
"GerritBackendFeature__allow_fix_suggestions_in_comments";
+
+ /**
+ * Whether the submit operation that is executed to auto-merge a change on push (see
+ * https://gerrit-review.googlesource.com/Documentation/user-upload.html#auto_merge) should use a
+ * {@code RefUpdateContext} with type {@code DIRECT_PUSH}.
+ */
+ public static String GERRIT_BACKEND_FEATURE_USE_DIRECT_PUSH_CONTEXT_FOR_SUBMIT_ON_PUSH =
+ "GerritBackendFeature__use_direct_push_context_for_submit_on_push";
}
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 30f2ee8..cb5dc9d 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -288,7 +288,7 @@
if (message == null) {
message =
MessageFormat.format(
- ChangeMessages.get().revertChangeDefaultMessage, subject, patch.commitId().name());
+ ChangeMessages.revertChangeDefaultMessage, subject, patch.commitId().name());
}
if (generatedChangeId != null) {
message = ChangeIdUtil.insertId(message, generatedChangeId, true);
diff --git a/java/com/google/gerrit/server/git/receive/PushOptionsValidator.java b/java/com/google/gerrit/server/git/receive/PushOptionsValidator.java
new file mode 100644
index 0000000..59caab5
--- /dev/null
+++ b/java/com/google/gerrit/server/git/receive/PushOptionsValidator.java
@@ -0,0 +1,52 @@
+// Copyright (C) 2025 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.git.receive;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ListMultimap;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.server.git.validators.ValidationMessage;
+
+/**
+ * Extension point to validate push options.
+ *
+ * <p>Possible usages are:
+ *
+ * <ul>
+ * <li>Reject pushes that use a certain push option or a certain combination of push options.
+ * <li>Print a warning if a certain push option is used (e.g because the push option is
+ * deprecated).
+ * </ul>
+ */
+@ExtensionPoint
+public interface PushOptionsValidator {
+ /**
+ * Validate the push options that have been specified for a push to the given ref.
+ *
+ * @param projectName The name of the project to which the user is pushing.
+ * @param refName The target ref of the push.
+ * @param pushOptions The options that have been specified on push. This map includes the Git push
+ * options (specified by {@code -o <key>=<value>}) and options which have been specified as
+ * part of the target ref name (specified as {@code refs/for/master%<key>=<value>}).
+ * @return A list of validation messages. If any message with type {@link
+ * com.google.gerrit.server.git.validators.ValidationMessage.Type#ERROR} or {@link
+ * com.google.gerrit.server.git.validators.ValidationMessage.Type#FATAL} is returned the push
+ * is rejected. Validation messages with other type are returned to the client, but let the
+ * push proceed.
+ */
+ ImmutableList<ValidationMessage> validate(
+ Project.NameKey projectName, String refName, ListMultimap<String, String> pushOptions);
+}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index cef4497..86985f3 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -142,6 +142,8 @@
import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.ChangeReportFormatter;
import com.google.gerrit.server.git.GroupCollector;
@@ -285,6 +287,8 @@
public static final String DIRECT_PUSH_JUSTIFICATION_OPTION = "push-justification";
+ private static final String CUSTOM_KEYED_VALUE_OPTION = "custom-keyed-value";
+
interface Factory {
ReceiveCommits create(
ProjectState projectState,
@@ -413,7 +417,9 @@
private final CreateRefControl createRefControl;
private final DeadlineChecker.Factory deadlineCheckerFactory;
private final DiffOperationsForCommitValidation.Factory diffOperationsForCommitValidationFactory;
+ private final ExperimentFeatures experimentFeatures;
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
+ private final DynamicSet<PushOptionsValidator> pushOptionsValidators;
private final DynamicSet<PluginPushOption> pluginPushOptions;
private final PluginSetContext<ReceivePackInitializer> initializers;
private final MergedByPushOp.Factory mergedByPushOpFactory;
@@ -479,6 +485,7 @@
private boolean setChangeAsPrivate;
private Optional<NoteDbPushOption> noteDbPushOption;
private Optional<String> tracePushOption = Optional.empty();
+ private Map<String, String> customKeyedValues = new HashMap<>();
private final TraceIdConsumer traceIdConsumer;
private MessageSender messageSender;
@@ -513,7 +520,9 @@
CreateRefControl createRefControl,
DeadlineChecker.Factory deadlineCheckerFactory,
DiffOperationsForCommitValidation.Factory diffOperationsForCommitValidationFactory,
+ ExperimentFeatures experimentFeatures,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
+ DynamicSet<PushOptionsValidator> pushOptionsValidators,
DynamicSet<PluginPushOption> pluginPushOptions,
PluginSetContext<ReceivePackInitializer> initializers,
PluginSetContext<CommentValidator> commentValidators,
@@ -570,6 +579,7 @@
this.createGroupPermissionSyncer = createGroupPermissionSyncer;
this.deadlineCheckerFactory = deadlineCheckerFactory;
this.diffOperationsForCommitValidationFactory = diffOperationsForCommitValidationFactory;
+ this.experimentFeatures = experimentFeatures;
this.editUtil = editUtil;
this.exceptionHooks = exceptionHooks;
this.hashtagsFactory = hashtagsFactory;
@@ -586,6 +596,7 @@
this.patchSetInfoFactory = patchSetInfoFactory;
this.permissionBackend = permissionBackend;
this.pluginConfigEntries = pluginConfigEntries;
+ this.pushOptionsValidators = pushOptionsValidators;
this.pluginPushOptions = pluginPushOptions;
this.projectCache = projectCache;
this.psUtil = psUtil;
@@ -1169,11 +1180,11 @@
ObjectInserter ins,
ImmutableList<CreateRequest> newChanges,
Task replaceProgress) {
+ ReceiveCommand magicBranchCmd = magicBranch != null ? magicBranch.cmd : null;
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
try (TraceTimer traceTimer =
newTimer(
"insertChangesAndPatchSets", Metadata.builder().resourceCount(newChanges.size()))) {
- ReceiveCommand magicBranchCmd = magicBranch != null ? magicBranch.cmd : null;
if (magicBranchCmd != null && magicBranchCmd.getResult() != NOT_ATTEMPTED) {
logger.atWarning().log(
"Skipping change updates on %s because ref update failed: %s %s",
@@ -1217,8 +1228,26 @@
} catch (RestApiException | IOException | UpdateException e) {
throw new StorageException("Can't insert change/patch set for " + project.getName(), e);
}
+ }
+ }
- if (magicBranch != null && magicBranch.submit) {
+ if (magicBranch != null && magicBranch.submit) {
+ // Using the submit option submits the created change(s) immediately without checking labels
+ // nor submit rules. Since code review is bypassed, same as on direct push, use a direct push
+ // RefUpdateContext to do the direct submit.
+ Optional<String> justification =
+ pushOptions.get(DIRECT_PUSH_JUSTIFICATION_OPTION).stream().findFirst();
+ try (RefUpdateContext ctx =
+ experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_FEATURE_USE_DIRECT_PUSH_CONTEXT_FOR_SUBMIT_ON_PUSH,
+ project.getNameKey())
+ ? RefUpdateContext.openDirectPush(justification)
+ : RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ try (TraceTimer traceTimer =
+ newTimer(
+ "insertChangesAndPatchSets#submit",
+ Metadata.builder().resourceCount(newChanges.size()))) {
try {
submit(newChanges, replaceByChange.values());
} catch (ResourceConflictException e) {
@@ -1866,6 +1895,7 @@
Map<String, Short> labels = new HashMap<>();
String message;
List<RevCommit> baseCommit;
+ ListMultimap<String, String> optionMap;
CmdLineParser cmdLineParser;
Set<String> hashtags = new HashSet<>();
@@ -1873,6 +1903,12 @@
String trace;
@Option(
+ name = "--push-justification",
+ metaVar = "NAME",
+ usage = "justification for the push if the 'submit' option is used to submit on push")
+ String pushJustification;
+
+ @Option(
name = "--deadline",
metaVar = "NAME",
usage = "deadline after which the push should be aborted")
@@ -1881,6 +1917,16 @@
@Option(name = "--base", metaVar = "BASE", usage = "merge base of changes")
List<ObjectId> base;
+ // Custom keyed values need to be specified as a push option ('-o
+ // custom-keyed-value=<key>:<value'). Setting the 'custom-keyed-value' option in the refname is
+ // not allowed. This option here is only defined, so that the 'custom-keyed-value' option gets
+ // mentioned in the help output.
+ @Option(
+ name = "--custom-keyed-value",
+ metaVar = "CUSTOM_KEYED_VALUES",
+ usage = "custom keyed value in the format '<key>:<value>'")
+ List<String> customKeyedValues;
+
@Option(name = "--topic", metaVar = "NAME", usage = "attach topic to changes")
String topic;
@@ -2099,7 +2145,7 @@
String parse(ListMultimap<String, String> pushOptions) throws CmdLineException {
String ref = RefNames.fullName(MagicBranch.getDestBranchName(cmd.getRefName()));
- ListMultimap<String, String> options = LinkedListMultimap.create(pushOptions);
+ optionMap = LinkedListMultimap.create();
// Process and lop off the "%OPTION" suffix.
int optionStart = ref.indexOf('%');
@@ -2107,16 +2153,33 @@
for (String s : COMMAS.split(ref.substring(optionStart + 1))) {
int e = s.indexOf('=');
if (0 < e) {
- options.put(s.substring(0, e), s.substring(e + 1));
+ optionMap.put(s.substring(0, e), s.substring(e + 1));
} else {
- options.put(s, "");
+ optionMap.put(s, "");
}
}
ref = ref.substring(0, optionStart);
}
- if (!options.isEmpty()) {
- cmdLineParser.parseOptionMap(options);
+ // We cannot accept the 'custom-key-value' option in the refname (deprecated format), since
+ // key and value are separated by ':' which is a character that is not allowed in the refname.
+ // If a user tries to specify '%custom-key-value=<key>:<value>' in the refname the push fails
+ // due to ':' not being allowed in refnames. In this case the push fails much earlier and we
+ // do not reach here. However if someone tries to specify the '%custom-key-value' option with
+ // allowed characters in the value, we return a message here telling the user that they should
+ // use the push option instead.
+ if (optionMap.containsKey(CUSTOM_KEYED_VALUE_OPTION)) {
+ throw cmdLineParser.reject(
+ String.format(
+ "option '%s' cannot be specified as an option in the ref name, use a push option"
+ + " instead ('-o %s=<key>:<value>')",
+ CUSTOM_KEYED_VALUE_OPTION, CUSTOM_KEYED_VALUE_OPTION));
+ }
+
+ optionMap.putAll(pushOptions);
+
+ if (!optionMap.isEmpty()) {
+ cmdLineParser.parseOptionMap(optionMap);
}
return ref;
}
@@ -2173,6 +2236,22 @@
String ref;
magicBranch.cmdLineParser = optionParserFactory.create(magicBranch);
+ // Parse custom keyed values.
+ for (String keyValue : pushOptions.get(CUSTOM_KEYED_VALUE_OPTION)) {
+ List<String> keyValueList = Splitter.on(":").trimResults().splitToList(keyValue);
+ if (keyValueList.size() != 2) {
+ reject(
+ cmd,
+ RejectionReason.create(
+ MetricBucket.INVALID_OPTION,
+ String.format(
+ "the value for option '%s' must be given as '<key>:<format>'",
+ CUSTOM_KEYED_VALUE_OPTION)));
+ return;
+ }
+ customKeyedValues.put(keyValueList.get(0), keyValueList.get(1));
+ }
+
// Filter out plugin push options, as the parser would reject them as unknown.
ImmutableListMultimap<String, String> pushOptionsToParse =
pushOptions.entries().stream()
@@ -2189,6 +2268,16 @@
ref = null; // never happens
}
+ if (magicBranch.pushJustification != null && !magicBranch.submit) {
+ reject(
+ cmd,
+ RejectionReason.create(
+ MetricBucket.INVALID_OPTION,
+ "when pushing for a review a push justification can only be set when the"
+ + " 'submit' option is used"));
+ return;
+ }
+
if (magicBranch.skipValidation) {
reject(
cmd,
@@ -2404,6 +2493,8 @@
}
}
}
+
+ validatePushOptions(cmd, projectState.getNameKey(), ref, magicBranch.optionMap);
} catch (IOException e) {
throw new StorageException(
String.format("Error walking to %s in project %s", destBranch, project.getName()), e);
@@ -2416,6 +2507,28 @@
}
}
+ private void validatePushOptions(
+ ReceiveCommand cmd,
+ Project.NameKey projectName,
+ String refName,
+ ListMultimap<String, String> pushOptions) {
+ ImmutableList<ValidationMessage> validationMessages =
+ StreamSupport.stream(pushOptionsValidators.entries().spliterator(), /* parallel= */ false)
+ .flatMap(
+ pluginPushValidators ->
+ pluginPushValidators.get().validate(projectName, refName, pushOptions).stream())
+ .collect(toImmutableList());
+ for (ValidationMessage validationMessage : validationMessages) {
+ if (validationMessage.isError()) {
+ reject(
+ cmd,
+ RejectionReason.create(MetricBucket.INVALID_OPTION, validationMessage.getMessage()));
+ } else {
+ messages.add(validationMessage);
+ }
+ }
+ }
+
private boolean isPluginPushOption(String pushOptionName) {
if (transitionalPluginOptions.contains(pushOptionName)) {
return true;
@@ -3030,6 +3143,7 @@
.create(changeId, commit, refName)
.setTopic(magicBranch.topic)
.setPrivate(setChangeAsPrivate)
+ .setCustomKeyedValues(ImmutableMap.copyOf(customKeyedValues))
.setWorkInProgress(magicBranch.shouldSetWorkInProgressOnNewChanges())
// The commit has already been validated in
// selectNewAndReplacedChangesFromMagicBranch.
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java b/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java
index 5c9106c..484a064 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmailImpl.java
@@ -569,24 +569,18 @@
return this.authors = authors;
}
- @Override
- public void populateEmailContent() throws EmailException {
- BranchEmailUtils.addBranchData(email, args, branch);
- setThreadHeaders();
-
- email.addSoyParam("changeId", change.getKey().get());
- email.addSoyParam("coverLetter", getCoverLetter());
- email.addSoyParam("fromName", email.getNameFor(email.getFrom()));
- email.addSoyParam("fromEmail", email.getNameEmailFor(email.getFrom()));
- email.addSoyParam("diffLines", ChangeEmail.getDiffTemplateData(getUnifiedDiff()));
-
+ protected void addChangeRelatedSoyParams() {
+ // diffLines is the structured diff format for HTML emails.
+ // unifiedDiff is textual diff for plaintext emails.
+ email.addSoyParam("diffLines", ChangeEmail.getDiffTemplateData(getUnifiedDiff().trim()));
email.addSoyEmailDataParam("unifiedDiff", getUnifiedDiff());
- email.addSoyEmailDataParam("changeDetail", getChangeDetail());
- email.addSoyEmailDataParam("changeUrl", getChangeUrl());
email.addSoyEmailDataParam("includeDiff", getIncludeDiff());
- Map<String, String> changeData = new HashMap<>();
+ // changeDetail - is a plaintext summary of the change (message, files, delta)
+ email.addSoyEmailDataParam("changeDetail", getChangeDetail());
+ email.addSoyEmailDataParam("changeUrl", getChangeUrl());
+ Map<String, String> changeData = new HashMap<>();
String subject = change.getSubject();
String originalSubject = change.getOriginalSubject();
changeData.put("subject", subject);
@@ -611,7 +605,13 @@
patchSetInfoData.put("authorName", patchSetInfo.getAuthor().getName());
patchSetInfoData.put("authorEmail", patchSetInfo.getAuthor().getEmail());
email.addSoyParam("patchSetInfo", patchSetInfoData);
+ // We need names rather than account ids / emails to make it user readable.
+ email.addSoyParam(
+ "attentionSet",
+ currentAttentionSet.stream().map(email::getNameFor).sorted().collect(toImmutableList()));
+ }
+ protected void addEmailFooters() {
email.addFooter(MailHeader.CHANGE_ID.withDelimiter() + change.getKey().get());
email.addFooter(MailHeader.CHANGE_NUMBER.withDelimiter() + change.getChangeId());
email.addFooter(MailHeader.PATCH_SET.withDelimiter() + patchSet.number());
@@ -625,13 +625,24 @@
for (Account.Id attentionUser : currentAttentionSet) {
email.addFooter(MailHeader.ATTENTION.withDelimiter() + email.getNameEmailFor(attentionUser));
}
- // We need names rather than account ids / emails to make it user readable.
- email.addSoyParam(
- "attentionSet",
- currentAttentionSet.stream().map(email::getNameFor).sorted().collect(toImmutableList()));
+ }
+ @Override
+ public void populateEmailContent() throws EmailException {
+ BranchEmailUtils.addBranchData(email, args, branch);
+ setThreadHeaders();
+
+ email.addSoyParam("coverLetter", getCoverLetter());
+ email.addSoyParam("fromName", email.getNameFor(email.getFrom()));
+ email.addSoyParam("fromEmail", email.getNameEmailFor(email.getFrom()));
+
+ addChangeRelatedSoyParams();
+ addEmailFooters();
+
+ // Set email subject.
setChangeSubjectHeader();
+ // Set email content
if (email.useHtml()) {
email.appendHtml(email.soyHtmlTemplate("ChangeHeaderHtml"));
}
diff --git a/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
index 6a09f0f2..e0fc648 100644
--- a/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
+++ b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
@@ -197,24 +197,34 @@
* Git and not get deleted. These refs point to an empty tree. We delete such refs.
*/
@Override
- protected void deleteEmptyDraftsByKey(Collection<Ref> refs) throws IOException {
+ protected void deleteEmptyDraftsByKey(Collection<Ref> refs) {
long zombieRefsCnt = refs.size();
long deletedRefsCnt = 0;
long startTime = System.currentTimeMillis();
for (List<Ref> refsBatch : Iterables.partition(refs, CHUNK_SIZE)) {
- deleteZombieDraftsBatch(refsBatch);
- long elapsed = (System.currentTimeMillis() - startTime) / 1000;
- deletedRefsCnt += refsBatch.size();
- logProgress(deletedRefsCnt, zombieRefsCnt, elapsed);
+ try {
+ deleteZombieDraftsBatch(refsBatch);
+ long elapsed = (System.currentTimeMillis() - startTime) / 1000;
+ deletedRefsCnt += refsBatch.size();
+ logProgress(deletedRefsCnt, zombieRefsCnt, elapsed);
+ } catch (Exception error) {
+ logger.atWarning().withCause(error).log("Failed to delete drafts as a batch");
+ }
}
}
@Override
- protected void deleteZombieDrafts(ListMultimap<Ref, HumanComment> drafts) throws IOException {
+ protected void deleteZombieDrafts(ListMultimap<Ref, HumanComment> drafts) {
for (Map.Entry<Ref, Collection<HumanComment>> e : drafts.asMap().entrySet()) {
- deleteZombieDraftsForChange(
- getAccountId(e.getKey()), getChangeNotes(getChangeId(e.getKey())), e.getValue());
+ try {
+ deleteZombieDraftsForChange(
+ getAccountId(e.getKey()), getChangeNotes(getChangeId(e.getKey())), e.getValue());
+ } catch (Exception error) {
+ logger.atWarning().withCause(error).log(
+ "Failed to delete draft for change %s account %s",
+ getChangeId(e.getKey()), getAccountId(e.getKey()));
+ }
}
}
diff --git a/java/com/google/gerrit/server/permissions/AbstractChangeControl.java b/java/com/google/gerrit/server/permissions/AbstractChangeControl.java
new file mode 100644
index 0000000..863f6a6
--- /dev/null
+++ b/java/com/google/gerrit/server/permissions/AbstractChangeControl.java
@@ -0,0 +1,299 @@
+// Copyright (C) 2025 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.permissions;
+
+import static com.google.gerrit.server.permissions.AbstractLabelPermission.ForUser.ON_BEHALF_OF;
+import static com.google.gerrit.server.permissions.DefaultPermissionMappings.labelPermissionName;
+
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.entities.PermissionRange;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Abstract access control management for a user accessing a single change.
+ *
+ * <p>Contains all logic that is common for checking permissions on an existing change and on a
+ * change that is not created yet.
+ */
+abstract class AbstractChangeControl {
+ protected final ProjectControl projectControl;
+ protected final RefControl refControl;
+ protected final boolean isNew;
+
+ /** Is this user the owner of the change? */
+ protected final boolean isOwner;
+
+ private Map<String, PermissionRange> labels;
+
+ AbstractChangeControl(
+ ProjectControl projectControl, RefControl refControl, boolean isNew, boolean isOwner) {
+ this.projectControl = projectControl;
+ this.refControl = refControl;
+ this.isNew = isNew;
+ this.isOwner = isOwner;
+ }
+
+ protected abstract ForChange asForChange();
+
+ protected CurrentUser getUser() {
+ return refControl.getUser();
+ }
+
+ protected boolean can(ChangePermissionOrLabel perm) throws PermissionBackendException {
+ if (perm instanceof ChangePermission) {
+ return can((ChangePermission) perm);
+ } else if (perm instanceof AbstractLabelPermission) {
+ return can((AbstractLabelPermission) perm);
+ } else if (perm instanceof AbstractLabelPermission.WithValue) {
+ return can((AbstractLabelPermission.WithValue) perm);
+ }
+ throw new PermissionBackendException(perm + " unsupported");
+ }
+
+ protected boolean can(ChangePermission perm) throws PermissionBackendException {
+ try {
+ return switch (perm) {
+ case READ -> isVisible();
+ case ABANDON -> canAbandon();
+ case DELETE -> projectControl.isAdmin() || refControl.canDeleteChanges(isOwner);
+ case ADD_PATCH_SET -> canAddPatchSet();
+ case EDIT_DESCRIPTION -> canEditDescription();
+ case EDIT_HASHTAGS -> canEditHashtags();
+ case EDIT_CUSTOM_KEYED_VALUES -> canEditCustomKeyedValues();
+ case EDIT_TOPIC_NAME -> canEditTopicName();
+ case REBASE -> canRebase();
+ case REBASE_ON_BEHALF_OF_UPLOADER -> canRebaseOnBehalfOfUploader();
+ case RESTORE -> canRestore();
+ case REVERT -> canRevert();
+ case SUBMIT -> refControl.canSubmit(isOwner);
+ case TOGGLE_WORK_IN_PROGRESS_STATE -> canToggleWorkInProgressState();
+ case REMOVE_REVIEWER, SUBMIT_AS -> refControl.canPerform(changePermissionName(perm));
+ };
+ } catch (StorageException e) {
+ throw new PermissionBackendException("unavailable", e);
+ }
+ }
+
+ /** Can this user see this change? */
+ protected boolean isVisible() {
+ // Does the user have READ permission on the destination?
+ return refControl.asForRef().testOrFalse(RefPermission.READ);
+ }
+
+ /** Can this user abandon this change? */
+ private boolean canAbandon() {
+ return isOwner // owner (aka creator) of the change can abandon
+ || refControl.isOwner() // branch owner can abandon
+ || projectControl.isOwner() // project owner can abandon
+ || refControl.canPerform(Permission.ABANDON) // user can abandon a specific ref
+ || projectControl.isAdmin();
+ }
+
+ /** Can this user add a patch set to this change? */
+ private boolean canAddPatchSet() {
+ if (!refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)) {
+ return false;
+ }
+ if (isOwner) {
+ return true;
+ }
+ return refControl.canAddPatchSet();
+ }
+
+ /** Can this user edit the topic name? */
+ private boolean canEditTopicName() {
+ if (isNew) {
+ return isOwner // owner (aka creator) of the change can edit topic
+ || refControl.isOwner() // branch owner can edit topic
+ || projectControl.isOwner() // project owner can edit topic
+ || refControl.canPerform(
+ Permission.EDIT_TOPIC_NAME) // user can edit topic on a specific ref
+ || projectControl.isAdmin();
+ }
+ return refControl.canForceEditTopicName(isOwner);
+ }
+
+ /** Can this user edit the description? */
+ private boolean canEditDescription() {
+ if (isNew) {
+ return isOwner // owner (aka creator) of the change can edit desc
+ || refControl.isOwner() // branch owner can edit desc
+ || projectControl.isOwner() // project owner can edit desc
+ || projectControl.isAdmin();
+ }
+ return false;
+ }
+
+ /** Can this user edit the hashtag name? */
+ private boolean canEditHashtags() {
+ return isOwner // owner (aka creator) of the change can edit hashtags
+ || refControl.isOwner() // branch owner can edit hashtags
+ || projectControl.isOwner() // project owner can edit hashtags
+ || refControl.canPerform(
+ Permission.EDIT_HASHTAGS) // user can edit hashtag on a specific ref
+ || projectControl.isAdmin();
+ }
+
+ /** Can this user edit the custom keyed values? */
+ private boolean canEditCustomKeyedValues() {
+ return isOwner // owner (aka creator) of the change can edit custom keyed values
+ || projectControl.isAdmin();
+ }
+
+ /** Can this user rebase this change? */
+ private boolean canRebase() {
+ return (isOwner || refControl.canSubmit(isOwner) || refControl.canRebase())
+ && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
+ }
+
+ /**
+ * Can this user rebase this change on behalf of the uploader?
+ *
+ * <p>This only checks the permissions of the rebaser (aka the impersonating user).
+ *
+ * <p>In addition rebase on behalf of the uploader requires the uploader (aka the impersonated
+ * user) to have permissions to create the new patch set. These permissions need to be checked
+ * separately.
+ */
+ private boolean canRebaseOnBehalfOfUploader() {
+ return (isOwner || refControl.canSubmit(isOwner) || refControl.canRebase());
+ }
+
+ /** Can this user restore this change? */
+ private boolean canRestore() {
+ // Anyone who can abandon the change can restore it, as long as they can create changes.
+ return canAbandon() && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
+ }
+
+ /** Can this user revert this change? */
+ private boolean canRevert() {
+ return refControl.canRevert() && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
+ }
+
+ /** Can this user toggle WorkInProgress state? */
+ private boolean canToggleWorkInProgressState() {
+ return isOwner
+ || projectControl.isOwner()
+ || refControl.canPerform(Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
+ || projectControl.isAdmin();
+ }
+
+ private boolean can(AbstractLabelPermission perm) {
+ return !label(labelPermissionName(perm)).isEmpty();
+ }
+
+ private boolean can(AbstractLabelPermission.WithValue perm) {
+ PermissionRange r = label(labelPermissionName(perm));
+ if (perm.forUser() == ON_BEHALF_OF && r.isEmpty()) {
+ return false;
+ }
+ return r.contains(perm.value());
+ }
+
+ private PermissionRange label(String permission) {
+ if (labels == null) {
+ labels = Maps.newHashMapWithExpectedSize(4);
+ }
+ PermissionRange r = labels.get(permission);
+ if (r == null) {
+ r = getRange(permission);
+ labels.put(permission, r);
+ }
+ return r;
+ }
+
+ /** The range of permitted values associated with a label permission. */
+ private PermissionRange getRange(String permission) {
+ return refControl.getRange(permission, isOwner);
+ }
+
+ protected class ForChangeImpl extends ForChange {
+ @Nullable private final Change.Id changeId;
+
+ private String resourcePath;
+
+ protected ForChangeImpl(@Nullable Change.Id changeId) {
+ this.changeId = changeId;
+ }
+
+ @Override
+ public String resourcePath() {
+ if (resourcePath == null) {
+ resourcePath =
+ String.format(
+ "/projects/%s/+changes/%s",
+ projectControl.getProjectState().getName(), changeId != null ? changeId.get() : 0);
+ }
+ return resourcePath;
+ }
+
+ @Override
+ public void check(ChangePermissionOrLabel perm)
+ throws AuthException, PermissionBackendException {
+ if (!can(perm)) {
+ throw new AuthException(
+ perm.describeForException()
+ + " not permitted"
+ + perm.hintForException().map(hint -> " (" + hint + ")").orElse(""));
+ }
+ }
+
+ @Override
+ public <T extends ChangePermissionOrLabel> Set<T> test(Collection<T> permSet)
+ throws PermissionBackendException {
+ Set<T> ok = newSet(permSet);
+ for (T perm : permSet) {
+ if (can(perm)) {
+ ok.add(perm);
+ }
+ }
+ return ok;
+ }
+
+ @Override
+ public BooleanCondition testCond(ChangePermissionOrLabel perm) {
+ return new PermissionBackendCondition.ForChange(this, perm, getUser());
+ }
+ }
+
+ protected static <T extends ChangePermissionOrLabel> Set<T> newSet(Collection<T> permSet) {
+ if (permSet instanceof EnumSet) {
+ @SuppressWarnings({"unchecked", "rawtypes"})
+ Set<T> s = ((EnumSet) permSet).clone();
+ s.clear();
+ return s;
+ }
+ return Sets.newHashSetWithExpectedSize(permSet.size());
+ }
+
+ protected static String changePermissionName(ChangePermission changePermission) {
+ // Within this class, it's programmer error to call this method on a
+ // ChangePermission that isn't associated with a permission name.
+ return DefaultPermissionMappings.changePermissionName(changePermission)
+ .orElseThrow(() -> new IllegalStateException("no name for " + changePermission));
+ }
+}
diff --git a/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java b/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java
index e4a8160..ba7caed 100644
--- a/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java
+++ b/java/com/google/gerrit/server/permissions/AbstractLabelPermission.java
@@ -21,8 +21,7 @@
import com.google.gerrit.server.util.LabelVote;
/** Abstract permission representing a label. */
-public abstract class AbstractLabelPermission
- implements ChangePermissionOrLabel, RefPermissionOrLabel {
+public abstract class AbstractLabelPermission implements ChangePermissionOrLabel {
public enum ForUser {
SELF,
ON_BEHALF_OF
@@ -91,7 +90,7 @@
}
/** A {@link AbstractLabelPermission} at a specific value. */
- public abstract static class WithValue implements ChangePermissionOrLabel, RefPermissionOrLabel {
+ public abstract static class WithValue implements ChangePermissionOrLabel {
private final ForUser forUser;
private final LabelVote label;
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 39a6310..3fe4811 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -14,32 +14,18 @@
package com.google.gerrit.server.permissions;
-import static com.google.gerrit.server.permissions.AbstractLabelPermission.ForUser.ON_BEHALF_OF;
-import static com.google.gerrit.server.permissions.DefaultPermissionMappings.labelPermissionName;
-
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Permission;
-import com.google.gerrit.entities.PermissionRange;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.conditions.BooleanCondition;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.assistedinject.Assisted;
-import java.util.Collection;
-import java.util.EnumSet;
-import java.util.Map;
-import java.util.Set;
import javax.inject.Inject;
/** Access control management for a user accessing a single change. */
-public class ChangeControl {
+public class ChangeControl extends AbstractChangeControl {
public interface Factory {
ChangeControl create(
ProjectControl projectControl, RefControl refControl, ChangeData changeData);
@@ -47,8 +33,6 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final ProjectControl projectControl;
- private final RefControl refControl;
private final ChangeData changeData;
@Inject
@@ -56,155 +40,31 @@
@Assisted ProjectControl projectControl,
@Assisted RefControl refControl,
@Assisted ChangeData changeData) {
- this.projectControl = projectControl;
- this.refControl = refControl;
+ super(projectControl, refControl, changeData.change().isNew(), isOwner(refControl, changeData));
this.changeData = changeData;
}
- ForChange asForChange() {
- return new ForChangeImpl();
+ private static boolean isOwner(RefControl refControl, ChangeData changeData) {
+ CurrentUser user = refControl.getUser();
+ if (user.isIdentifiedUser()) {
+ Account.Id id = user.asIdentifiedUser().getAccountId();
+ return id.equals(changeData.change().getOwner());
+ }
+ return false;
}
- private CurrentUser getUser() {
- return refControl.getUser();
- }
-
- private ProjectControl getProjectControl() {
- return refControl.getProjectControl();
- }
-
- private Change getChange() {
- return changeData.change();
+ @Override
+ public ForChange asForChange() {
+ return new ForChangeImpl(changeData.getId());
}
/** Can this user see this change? */
- boolean isVisible() {
+ @Override
+ protected boolean isVisible() {
if (changeData.isPrivateOrThrow() && !isPrivateVisible(changeData)) {
return false;
}
- // Does the user have READ permission on the destination?
- return refControl.asForRef().testOrFalse(RefPermission.READ);
- }
-
- /** Can this user abandon this change? */
- private boolean canAbandon() {
- return isOwner() // owner (aka creator) of the change can abandon
- || refControl.isOwner() // branch owner can abandon
- || getProjectControl().isOwner() // project owner can abandon
- || refControl.canPerform(Permission.ABANDON) // user can abandon a specific ref
- || getProjectControl().isAdmin();
- }
-
- /** Can this user rebase this change? */
- private boolean canRebase() {
- return (isOwner() || refControl.canSubmit(isOwner()) || refControl.canRebase())
- && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
- }
-
- /**
- * Can this user rebase this change on behalf of the uploader?
- *
- * <p>This only checks the permissions of the rebaser (aka the impersonating user).
- *
- * <p>In addition rebase on behalf of the uploader requires the uploader (aka the impersonated
- * user) to have permissions to create the new patch set. These permissions need to be checked
- * separately.
- */
- private boolean canRebaseOnBehalfOfUploader() {
- return (isOwner() || refControl.canSubmit(isOwner()) || refControl.canRebase());
- }
-
- /** Can this user restore this change? */
- private boolean canRestore() {
- // Anyone who can abandon the change can restore it, as long as they can create changes.
- return canAbandon() && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
- }
-
- /** Can this user revert this change? */
- private boolean canRevert() {
- return refControl.canRevert() && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
- }
-
- /** The range of permitted values associated with a label permission. */
- private PermissionRange getRange(String permission) {
- return refControl.getRange(permission, isOwner());
- }
-
- /** Can this user add a patch set to this change? */
- private boolean canAddPatchSet() {
- if (!refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)) {
- return false;
- }
- if (isOwner()) {
- return true;
- }
- return refControl.canAddPatchSet();
- }
-
- /** Is this user the owner of the change? */
- private boolean isOwner() {
- if (getUser().isIdentifiedUser()) {
- Account.Id id = getUser().asIdentifiedUser().getAccountId();
- return id.equals(getChange().getOwner());
- }
- return false;
- }
-
- /** Is this user a reviewer for the change? */
- private boolean isReviewer(ChangeData cd) {
- if (getUser().isIdentifiedUser()) {
- ImmutableSet<Account.Id> results = cd.reviewers().all();
- return results.contains(getUser().getAccountId());
- }
- return false;
- }
-
- /** Can this user edit the topic name? */
- private boolean canEditTopicName() {
- if (getChange().isNew()) {
- return isOwner() // owner (aka creator) of the change can edit topic
- || refControl.isOwner() // branch owner can edit topic
- || getProjectControl().isOwner() // project owner can edit topic
- || refControl.canPerform(
- Permission.EDIT_TOPIC_NAME) // user can edit topic on a specific ref
- || getProjectControl().isAdmin();
- }
- return refControl.canForceEditTopicName(isOwner());
- }
-
- /** Can this user toggle WorkInProgress state? */
- private boolean canToggleWorkInProgressState() {
- return isOwner()
- || getProjectControl().isOwner()
- || refControl.canPerform(Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
- || getProjectControl().isAdmin();
- }
-
- /** Can this user edit the description? */
- private boolean canEditDescription() {
- if (getChange().isNew()) {
- return isOwner() // owner (aka creator) of the change can edit desc
- || refControl.isOwner() // branch owner can edit desc
- || getProjectControl().isOwner() // project owner can edit desc
- || getProjectControl().isAdmin();
- }
- return false;
- }
-
- /** Can this user edit the hashtag name? */
- private boolean canEditHashtags() {
- return isOwner() // owner (aka creator) of the change can edit hashtags
- || refControl.isOwner() // branch owner can edit hashtags
- || getProjectControl().isOwner() // project owner can edit hashtags
- || refControl.canPerform(
- Permission.EDIT_HASHTAGS) // user can edit hashtag on a specific ref
- || getProjectControl().isAdmin();
- }
-
- /** Can this user edit the custom keyed values? */
- private boolean canEditCustomKeyedValues() {
- return isOwner() // owner (aka creator) of the change can edit custom keyed values
- || getProjectControl().isAdmin();
+ return super.isVisible();
}
private boolean isPrivateVisible(ChangeData cd) {
@@ -215,7 +75,7 @@
return true;
}
- if (isOwner()) {
+ if (isOwner) {
logger.atFine().log(
"%s can see private change %s because this user is the change owner",
getUser().getLoggableName(), cd.getId());
@@ -247,125 +107,12 @@
return false;
}
- private class ForChangeImpl extends ForChange {
- private Map<String, PermissionRange> labels;
- private String resourcePath;
-
- private ForChangeImpl() {}
-
- @Override
- public String resourcePath() {
- if (resourcePath == null) {
- resourcePath =
- String.format(
- "/projects/%s/+changes/%s",
- getProjectControl().getProjectState().getName(), changeData.getId().get());
- }
- return resourcePath;
+ /** Is this user a reviewer for the change? */
+ private boolean isReviewer(ChangeData cd) {
+ if (getUser().isIdentifiedUser()) {
+ ImmutableSet<Account.Id> results = cd.reviewers().all();
+ return results.contains(getUser().getAccountId());
}
-
- @Override
- public void check(ChangePermissionOrLabel perm)
- throws AuthException, PermissionBackendException {
- if (!can(perm)) {
- throw new AuthException(
- perm.describeForException()
- + " not permitted"
- + perm.hintForException().map(hint -> " (" + hint + ")").orElse(""));
- }
- }
-
- @Override
- public <T extends ChangePermissionOrLabel> Set<T> test(Collection<T> permSet)
- throws PermissionBackendException {
- Set<T> ok = newSet(permSet);
- for (T perm : permSet) {
- if (can(perm)) {
- ok.add(perm);
- }
- }
- return ok;
- }
-
- @Override
- public BooleanCondition testCond(ChangePermissionOrLabel perm) {
- return new PermissionBackendCondition.ForChange(this, perm, getUser());
- }
-
- private boolean can(ChangePermissionOrLabel perm) throws PermissionBackendException {
- if (perm instanceof ChangePermission) {
- return can((ChangePermission) perm);
- } else if (perm instanceof AbstractLabelPermission) {
- return can((AbstractLabelPermission) perm);
- } else if (perm instanceof AbstractLabelPermission.WithValue) {
- return can((AbstractLabelPermission.WithValue) perm);
- }
- throw new PermissionBackendException(perm + " unsupported");
- }
-
- private boolean can(ChangePermission perm) throws PermissionBackendException {
- try {
- return switch (perm) {
- case READ -> isVisible();
- case ABANDON -> canAbandon();
- case DELETE -> getProjectControl().isAdmin() || refControl.canDeleteChanges(isOwner());
- case ADD_PATCH_SET -> canAddPatchSet();
- case EDIT_DESCRIPTION -> canEditDescription();
- case EDIT_HASHTAGS -> canEditHashtags();
- case EDIT_CUSTOM_KEYED_VALUES -> canEditCustomKeyedValues();
- case EDIT_TOPIC_NAME -> canEditTopicName();
- case REBASE -> canRebase();
- case REBASE_ON_BEHALF_OF_UPLOADER -> canRebaseOnBehalfOfUploader();
- case RESTORE -> canRestore();
- case REVERT -> canRevert();
- case SUBMIT -> refControl.canSubmit(isOwner());
- case TOGGLE_WORK_IN_PROGRESS_STATE -> canToggleWorkInProgressState();
- case REMOVE_REVIEWER, SUBMIT_AS -> refControl.canPerform(changePermissionName(perm));
- };
- } catch (StorageException e) {
- throw new PermissionBackendException("unavailable", e);
- }
- }
-
- private boolean can(AbstractLabelPermission perm) {
- return !label(labelPermissionName(perm)).isEmpty();
- }
-
- private boolean can(AbstractLabelPermission.WithValue perm) {
- PermissionRange r = label(labelPermissionName(perm));
- if (perm.forUser() == ON_BEHALF_OF && r.isEmpty()) {
- return false;
- }
- return r.contains(perm.value());
- }
-
- private PermissionRange label(String permission) {
- if (labels == null) {
- labels = Maps.newHashMapWithExpectedSize(4);
- }
- PermissionRange r = labels.get(permission);
- if (r == null) {
- r = getRange(permission);
- labels.put(permission, r);
- }
- return r;
- }
- }
-
- private static <T extends ChangePermissionOrLabel> Set<T> newSet(Collection<T> permSet) {
- if (permSet instanceof EnumSet) {
- @SuppressWarnings({"unchecked", "rawtypes"})
- Set<T> s = ((EnumSet) permSet).clone();
- s.clear();
- return s;
- }
- return Sets.newHashSetWithExpectedSize(permSet.size());
- }
-
- private static String changePermissionName(ChangePermission changePermission) {
- // Within this class, it's programmer error to call this method on a
- // ChangePermission that isn't associated with a permission name.
- return DefaultPermissionMappings.changePermissionName(changePermission)
- .orElseThrow(() -> new IllegalStateException("no name for " + changePermission));
+ return false;
}
}
diff --git a/java/com/google/gerrit/server/permissions/ChangeControlForChangeToBeCreated.java b/java/com/google/gerrit/server/permissions/ChangeControlForChangeToBeCreated.java
new file mode 100644
index 0000000..b8418f4
--- /dev/null
+++ b/java/com/google/gerrit/server/permissions/ChangeControlForChangeToBeCreated.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2025 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.permissions;
+
+import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
+
+/** Access control management for a user accessing a change that has not been created yet. */
+public class ChangeControlForChangeToBeCreated extends AbstractChangeControl {
+
+ public ChangeControlForChangeToBeCreated(
+ ProjectControl projectControl, RefControl refControl, boolean isOwner) {
+ super(projectControl, refControl, /* isNew= */ true, isOwner);
+ }
+
+ @Override
+ public ForChange asForChange() {
+ return new ForChangeImpl(/* changeId= */ null);
+ }
+}
diff --git a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
index 6a8c92b..0fc9807 100644
--- a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
@@ -173,18 +173,23 @@
}
@Override
- public void check(RefPermissionOrLabel perm) throws PermissionBackendException {
+ public ForChange changeToBeCreated(boolean isChangeOwner) {
+ return new FailedChange(message, cause);
+ }
+
+ @Override
+ public void check(RefPermission perm) throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
@Override
- public <T extends RefPermissionOrLabel> Set<T> test(Collection<T> permSet)
+ public Set<RefPermission> test(Collection<RefPermission> permSet)
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
@Override
- public BooleanCondition testCond(RefPermissionOrLabel perm) {
+ public BooleanCondition testCond(RefPermission perm) {
throw new UnsupportedOperationException(
"FailedPermissionBackend does not support conditions");
}
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 55cdc3d..8e2461d 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -394,33 +394,24 @@
/** Returns an instance scoped to change. */
public abstract ForChange change(ChangeNotes notes);
+ /** Returns an instance scoped to change that has not been created yet. */
+ public abstract ForChange changeToBeCreated(boolean isOwner);
+
/**
* Verify scoped user can {@code perm}, throwing if denied.
*
* <p>Should be used in REST API handlers where the thrown {@link AuthException} can be
* propagated. In business logic, where the exception would have to be caught, prefer using
- * {@link #test(RefPermissionOrLabel)}.
+ * {@link #test(RefPermission)}.
*/
- public abstract void check(RefPermissionOrLabel perm)
- throws AuthException, PermissionBackendException;
+ public abstract void check(RefPermission perm) throws AuthException, PermissionBackendException;
/** Filter {@code permSet} to permissions scoped user might be able to perform. */
- public abstract <T extends RefPermissionOrLabel> Set<T> test(Collection<T> permSet)
+ public abstract Set<RefPermission> test(Collection<RefPermission> permSet)
throws PermissionBackendException;
- public boolean test(RefPermissionOrLabel perm) throws PermissionBackendException {
- return test(Collections.singleton(perm)).contains(perm);
- }
-
- /**
- * Test which values of a label the user may be able to set.
- *
- * @param label definition of the label to test values of.
- * @return set containing values the user may be able to use; may be empty if none.
- * @throws PermissionBackendException if failure consulting backend configuration.
- */
- public Set<LabelPermission.WithValue> test(LabelType label) throws PermissionBackendException {
- return test(valuesOf(requireNonNull(label, "LabelType")));
+ public boolean test(RefPermission perm) throws PermissionBackendException {
+ return test(EnumSet.of(perm)).contains(perm);
}
/**
@@ -433,7 +424,7 @@
* @return true if the user might be able to perform the permission; false if the user may be
* missing the necessary grants or state, or if the backend threw an exception.
*/
- public boolean testOrFalse(RefPermissionOrLabel perm) {
+ public boolean testOrFalse(RefPermission perm) {
try {
return test(perm);
} catch (PermissionBackendException e) {
@@ -442,13 +433,7 @@
}
}
- public abstract BooleanCondition testCond(RefPermissionOrLabel perm);
-
- private static Set<LabelPermission.WithValue> valuesOf(LabelType label) {
- return label.getValues().stream()
- .map(v -> new LabelPermission.WithValue(label, v))
- .collect(toSet());
- }
+ public abstract BooleanCondition testCond(RefPermission perm);
}
/** PermissionBackend scoped to a user, project, reference and change. */
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java b/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java
index 7018fa0..a92e504 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java
@@ -148,10 +148,10 @@
public static class ForRef extends PermissionBackendCondition {
private final PermissionBackend.ForRef impl;
- private final RefPermissionOrLabel perm;
+ private final RefPermission perm;
private final CurrentUser user;
- public ForRef(PermissionBackend.ForRef impl, RefPermissionOrLabel perm, CurrentUser user) {
+ public ForRef(PermissionBackend.ForRef impl, RefPermission perm, CurrentUser user) {
this.impl = impl;
this.perm = perm;
this.user = user;
@@ -161,7 +161,7 @@
return impl;
}
- public RefPermissionOrLabel permission() {
+ public RefPermission permission() {
return perm;
}
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index 194179e..c86f8f6 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -123,6 +123,11 @@
return changeControlFactory.create(this, controlForRef(cd.branchOrThrow()), cd);
}
+ ChangeControlForChangeToBeCreated controlForChangeToBeCreated(
+ RefControl refControl, boolean isOwner) {
+ return new ChangeControlForChangeToBeCreated(this, refControl, isOwner);
+ }
+
RefControl controlForRef(BranchNameKey ref) {
return controlForRef(ref.branch());
}
@@ -453,7 +458,7 @@
throw new PermissionBackendException(perm.describeForException() + " unsupported");
}
- private boolean can(ProjectPermission perm) throws PermissionBackendException {
+ private boolean can(ProjectPermission perm) {
return switch (perm) {
case ACCESS -> user.isInternalUser() || isOwner() || canPerformOnAnyRef(Permission.READ);
case READ -> allRefsAreVisible(Collections.emptySet());
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index 88e53f7..5dd6be0 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -15,12 +15,8 @@
package com.google.gerrit.server.permissions;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.permissions.AbstractLabelPermission.ForUser.ON_BEHALF_OF;
-import static com.google.gerrit.server.permissions.DefaultPermissionMappings.labelPermissionName;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
@@ -47,7 +43,6 @@
import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
-import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Constants;
@@ -487,7 +482,6 @@
}
private class ForRefImpl extends ForRef {
- private Map<String, PermissionRange> labels;
private String resourcePath;
@Override
@@ -524,17 +518,14 @@
}
@Override
- public void check(RefPermissionOrLabel perm) throws AuthException, PermissionBackendException {
- if (perm instanceof RefPermission) {
- check((RefPermission) perm);
- } else {
- if (!can(perm)) {
- throw new AuthException(perm.describeForException() + " not permitted");
- }
- }
+ public ForChange changeToBeCreated(boolean isOwner) {
+ return getProjectControl()
+ .controlForChangeToBeCreated(RefControl.this, isOwner)
+ .asForChange();
}
- private void check(RefPermission perm) throws AuthException, PermissionBackendException {
+ @Override
+ public void check(RefPermission perm) throws AuthException, PermissionBackendException {
if (!can(perm)) {
PermissionDeniedException pde = new PermissionDeniedException(perm, refName);
switch (perm) {
@@ -601,10 +592,10 @@
}
@Override
- public <T extends RefPermissionOrLabel> Set<T> test(Collection<T> permSet)
+ public Set<RefPermission> test(Collection<RefPermission> permSet)
throws PermissionBackendException {
- Set<T> ok = newSet(permSet);
- for (T perm : permSet) {
+ EnumSet<RefPermission> ok = EnumSet.noneOf(RefPermission.class);
+ for (RefPermission perm : permSet) {
if (can(perm)) {
ok.add(perm);
}
@@ -613,44 +604,9 @@
}
@Override
- public BooleanCondition testCond(RefPermissionOrLabel perm) {
+ public BooleanCondition testCond(RefPermission perm) {
return new PermissionBackendCondition.ForRef(this, perm, getUser());
}
-
- private boolean can(RefPermissionOrLabel perm) throws PermissionBackendException {
- if (perm instanceof RefPermission) {
- return RefControl.this.can((RefPermission) perm);
- } else if (perm instanceof AbstractLabelPermission) {
- return can((AbstractLabelPermission) perm);
- } else if (perm instanceof AbstractLabelPermission.WithValue) {
- return can((AbstractLabelPermission.WithValue) perm);
- }
- throw new PermissionBackendException(perm + " unsupported");
- }
-
- private boolean can(AbstractLabelPermission perm) {
- return !label(labelPermissionName(perm)).isEmpty();
- }
-
- private boolean can(AbstractLabelPermission.WithValue perm) {
- PermissionRange r = label(labelPermissionName(perm));
- if (perm.forUser() == ON_BEHALF_OF && r.isEmpty()) {
- return false;
- }
- return r.contains(perm.value());
- }
-
- private PermissionRange label(String permission) {
- if (labels == null) {
- labels = Maps.newHashMapWithExpectedSize(4);
- }
- PermissionRange r = labels.get(permission);
- if (r == null) {
- r = getRange(permission);
- labels.put(permission, r);
- }
- return r;
- }
}
protected boolean can(RefPermission perm) throws PermissionBackendException {
@@ -773,15 +729,4 @@
return DefaultPermissionMappings.refPermissionName(refPermission)
.orElseThrow(() -> new IllegalStateException("no name for " + refPermission));
}
-
- // Same as in ChangeControl
- private static <T extends RefPermissionOrLabel> Set<T> newSet(Collection<T> permSet) {
- if (permSet instanceof EnumSet) {
- @SuppressWarnings({"unchecked", "rawtypes"})
- Set<T> s = ((EnumSet) permSet).clone();
- s.clear();
- return s;
- }
- return Sets.newHashSetWithExpectedSize(permSet.size());
- }
}
diff --git a/java/com/google/gerrit/server/permissions/RefPermission.java b/java/com/google/gerrit/server/permissions/RefPermission.java
index da5b745..34c46af 100644
--- a/java/com/google/gerrit/server/permissions/RefPermission.java
+++ b/java/com/google/gerrit/server/permissions/RefPermission.java
@@ -18,7 +18,7 @@
import com.google.gerrit.extensions.api.access.GerritPermission;
-public enum RefPermission implements RefPermissionOrLabel {
+public enum RefPermission implements GerritPermission {
READ,
CREATE,
diff --git a/java/com/google/gerrit/server/permissions/RefPermissionOrLabel.java b/java/com/google/gerrit/server/permissions/RefPermissionOrLabel.java
deleted file mode 100644
index 3ec74ba..0000000
--- a/java/com/google/gerrit/server/permissions/RefPermissionOrLabel.java
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright (C) 2025 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.permissions;
-
-import com.google.gerrit.extensions.api.access.GerritPermission;
-
-/** A {@link RefPermission} or a {@link AbstractLabelPermission}. */
-public interface RefPermissionOrLabel extends GerritPermission {}
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 9cd7908..77101ccb 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -392,21 +392,21 @@
if (initialMessage == null) {
initialMessage =
MessageFormat.format(
- ChangeMessages.get().revertSubmissionDefaultMessage,
+ ChangeMessages.revertSubmissionDefaultMessage,
changeNotes.getCurrentPatchSet().commitId().name());
}
// For performance purposes: Almost all cases will end here.
if (!subject.startsWith("Revert")) {
return MessageFormat.format(
- ChangeMessages.get().revertSubmissionUserMessage, subject, initialMessage);
+ ChangeMessages.revertSubmissionUserMessage, subject, initialMessage);
}
Matcher matcher = patternRevertSubjectWithNum.matcher(subject);
if (matcher.matches()) {
return MessageFormat.format(
- ChangeMessages.get().revertSubmissionOfRevertSubmissionUserMessage,
+ ChangeMessages.revertSubmissionOfRevertSubmissionUserMessage,
Integer.valueOf(matcher.group(1)) + 1,
matcher.group(2),
changeNotes.getCurrentPatchSet().commitId().name());
@@ -415,14 +415,14 @@
matcher = patternRevertSubject.matcher(subject);
if (matcher.matches()) {
return MessageFormat.format(
- ChangeMessages.get().revertSubmissionOfRevertSubmissionUserMessage,
+ ChangeMessages.revertSubmissionOfRevertSubmissionUserMessage,
2,
matcher.group(1),
changeNotes.getCurrentPatchSet().commitId().name());
}
return MessageFormat.format(
- ChangeMessages.get().revertSubmissionUserMessage, subject, initialMessage);
+ ChangeMessages.revertSubmissionUserMessage, subject, initialMessage);
}
/**
diff --git a/java/com/google/gerrit/server/restapi/config/ListCapabilities.java b/java/com/google/gerrit/server/restapi/config/ListCapabilities.java
index 6c8bf74..5b36950 100644
--- a/java/com/google/gerrit/server/restapi/config/ListCapabilities.java
+++ b/java/com/google/gerrit/server/restapi/config/ListCapabilities.java
@@ -68,8 +68,8 @@
private Map<String, CapabilityInfo> collectCoreCapabilities()
throws IllegalAccessException, NoSuchFieldException {
Map<String, CapabilityInfo> output = new HashMap<>();
- Class<? extends CapabilityConstants> bundleClass = CapabilityConstants.get().getClass();
- CapabilityConstants c = CapabilityConstants.get();
+ Class<? extends CapabilityConstants> bundleClass = new CapabilityConstants().getClass();
+ CapabilityConstants c = new CapabilityConstants();
for (String id : GlobalCapability.getAllNames()) {
String name = (String) bundleClass.getField(id).get(c);
output.put(id, new CapabilityInfo(id, name));
diff --git a/java/com/google/gerrit/server/restapi/project/ListLabels.java b/java/com/google/gerrit/server/restapi/project/ListLabels.java
index 8be2646..912360c 100644
--- a/java/com/google/gerrit/server/restapi/project/ListLabels.java
+++ b/java/com/google/gerrit/server/restapi/project/ListLabels.java
@@ -125,8 +125,7 @@
public List<LabelDefinitionInfo> filterLabelsThatUserCanVoteOnRef(
ProjectResource rsrc, List<LabelDefinitionInfo> allLabels)
- throws AuthException,
- PermissionBackendException,
+ throws PermissionBackendException,
ResourceConflictException,
RepositoryNotFoundException,
IOException {
@@ -161,11 +160,14 @@
continue;
}
+ // We assume that user is interested in labels that can be voted on if
+ // the user is the change owner.
Set<LabelPermission.WithValue> can =
permissionBackend
.currentUser()
.project(rsrc.getNameKey())
.ref(branchNameKey.branch())
+ .changeToBeCreated(/* isOwner= */ true)
.test(labelType);
for (LabelValue v : labelType.getValues()) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 83f36f9..5946725 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1828,7 +1828,7 @@
in.state = REMOVED;
ReviewerResult reviewerResult = gApi.changes().id(r.getChangeId()).addReviewer(in);
assertThat(reviewerResult.error)
- .isEqualTo(MessageFormat.format(ChangeMessages.get().groupRemovalIsNotAllowed, groupName));
+ .isEqualTo(MessageFormat.format(ChangeMessages.groupRemovalIsNotAllowed, groupName));
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/config/ListExperimentsIT.java b/javatests/com/google/gerrit/acceptance/api/config/ListExperimentsIT.java
index fe3cb00..19c1b81 100644
--- a/javatests/com/google/gerrit/acceptance/api/config/ListExperimentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/config/ListExperimentsIT.java
@@ -52,7 +52,9 @@
.GERRIT_BACKEND_FEATURE_ALWAYS_REJECT_IMPLICIT_MERGES_ON_MERGE,
ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_ATTACH_NONCE_TO_DOCUMENTATION,
ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_CHECK_IMPLICIT_MERGES_ON_MERGE,
- ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_REJECT_IMPLICIT_MERGES_ON_MERGE)
+ ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_REJECT_IMPLICIT_MERGES_ON_MERGE,
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_FEATURE_USE_DIRECT_PUSH_CONTEXT_FOR_SUBMIT_ON_PUSH)
.inOrder();
// "GerritBackendFeature__check_implicit_merges_on_merge",
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index fe488f5..1bf0f55 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -53,6 +53,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import com.google.common.collect.ListMultimap;
import com.google.common.collect.Streams;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
@@ -109,10 +110,12 @@
import com.google.gerrit.server.PluginPushOption;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.receive.NoteDbPushOption;
+import com.google.gerrit.server.git.receive.PushOptionsValidator;
import com.google.gerrit.server.git.receive.ReceiveConstants;
import com.google.gerrit.server.git.validators.CommitValidationInfo;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.CommitValidationMessage;
+import com.google.gerrit.server.git.validators.ValidationMessage;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.query.change.ChangeData;
@@ -3190,6 +3193,38 @@
}
@Test
+ public void pushWithMalformedCustomKeyedValuesRejects() throws Exception {
+ List<String> pushOptions = new ArrayList<>();
+ pushOptions.add("custom-keyed-value=foo-bar");
+
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ push.setPushOptions(pushOptions);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertErrorStatus(
+ "the value for option 'custom-keyed-value' must be given as '<key>:<format>'");
+ }
+
+ @Test
+ public void pushWithCustomKeyedValuesPasses() throws Exception {
+ List<String> pushOptions = new ArrayList<>();
+ pushOptions.add("custom-keyed-value=foo:bar");
+
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ push.setPushOptions(pushOptions);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertOkStatus();
+ assertThat(r.getChange().customKeyedValues()).isEqualTo(ImmutableMap.of("foo", "bar"));
+ }
+
+ @Test
+ public void cannotPushWithCustomKeyedValuesInRefName() throws Exception {
+ PushOneCommit.Result r = pushTo("refs/for/master%custom-keyed-value=foo-bar");
+ r.assertErrorStatus(
+ "option 'custom-keyed-value' cannot be specified as an option in the ref name, use a push"
+ + " option instead ('-o custom-keyed-value=<key>:<value>')");
+ }
+
+ @Test
public void pushForMasterWithUnknownOption() throws Exception {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
push.setPushOptions(ImmutableList.of("unknown=foo"));
@@ -3334,6 +3369,104 @@
}
}
+ @Test
+ public void rejectPushOptionFromValidator() throws Exception {
+ PushOptionsValidator pushOptionsValidator =
+ (Project.NameKey projectName, String refName, ListMultimap<String, String> pushOptions) ->
+ pushOptions.containsKey("topic")
+ ? ImmutableList.of(
+ new ValidationMessage("setting a topic is not allowed", /* isError= */ true))
+ : ImmutableList.of();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(pushOptionsValidator)) {
+ // Push with a push option that is being rejected.
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ List<String> pushOptions = new ArrayList<>();
+ pushOptions.add("topic=myTopic");
+ push.setPushOptions(pushOptions);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertErrorStatus("setting a topic is not allowed");
+
+ // Push with an option in the refname that is being rejected.
+ r = pushTo("refs/for/master%topic=myTopic");
+ r.assertErrorStatus("setting a topic is not allowed");
+
+ // Pushing with another push options is fine.
+ pushOptions = new ArrayList<>();
+ pushOptions.add("notify=NONE");
+ push.setPushOptions(pushOptions);
+ r = push.to("refs/for/master");
+ r.assertOkStatus();
+
+ // Pushing with another option in the refname is fine.
+ r = pushTo("refs/for/master%notify=NONE");
+ r.assertOkStatus();
+ }
+ }
+
+ @Test
+ public void printMessageForPushOptionFromValidator() throws Exception {
+ PushOptionsValidator pushOptionsValidator =
+ (Project.NameKey projectName, String refName, ListMultimap<String, String> pushOptions) ->
+ pushOptions.containsKey("topic")
+ ? ImmutableList.of(
+ new ValidationMessage(
+ "setting a topic is deprecated", ValidationMessage.Type.WARNING))
+ : ImmutableList.of();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(pushOptionsValidator)) {
+ // Push with a push option that is being rejected.
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ List<String> pushOptions = new ArrayList<>();
+ pushOptions.add("topic=myTopic");
+ push.setPushOptions(pushOptions);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertOkStatus();
+ r.assertMessage("warning: setting a topic is deprecated");
+
+ // Push with an option in the refname that is being rejected.
+ r = pushTo("refs/for/master%topic=myTopic");
+ r.assertOkStatus();
+ r.assertMessage("warning: setting a topic is deprecated");
+
+ // Pushing with another push options doesn't print the message.
+ push = pushFactory.create(admin.newIdent(), testRepo);
+ pushOptions = new ArrayList<>();
+ pushOptions.add("notify=NONE");
+ push.setPushOptions(pushOptions);
+ r = push.to("refs/for/master");
+ r.assertOkStatus();
+ r.assertNotMessage("warning: setting a topic is deprecated");
+
+ // Pushing with another option in the refname doesn't print the message.
+ r = pushTo("refs/for/master%notify=NONE");
+ r.assertOkStatus();
+ r.assertNotMessage("warning: setting a topic is deprecated");
+ }
+ }
+
+ @Test
+ public void cannotSetPushJustificationWhenSubmitOptionIsNotUsed() throws Exception {
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ List<String> pushOptions = new ArrayList<>();
+ pushOptions.add("push-justification=test justification");
+ push.setPushOptions(pushOptions);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertErrorStatus(
+ "when pushing for a review a push justification can only be set when the"
+ + " 'submit' option is used");
+
+ // pushing with the submit option works
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.SUBMIT).ref("refs/for/refs/heads/master").group(adminGroupUuid()))
+ .update();
+
+ r = push.to("refs/for/master%submit");
+ r.assertOkStatus();
+ }
+
private DraftInput newDraft(String path, int line, String message) {
DraftInput d = new DraftInput();
d.path = path;
diff --git a/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java b/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java
index c059ae4..c675e2b 100644
--- a/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/DirectPushRefUpdateContextIT.java
@@ -14,19 +14,32 @@
package com.google.gerrit.acceptance.git;
+import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static java.util.function.Function.identity;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.server.update.context.RefUpdateContext;
import com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType;
import com.google.gerrit.testing.RefUpdateContextCollector;
+import com.google.inject.Inject;
import java.util.Map.Entry;
import org.junit.Rule;
import org.junit.Test;
public class DirectPushRefUpdateContextIT extends AbstractDaemonTest {
+ @Inject private ProjectOperations projectOperations;
+
@Rule
public RefUpdateContextCollector refUpdateContextCollector = new RefUpdateContextCollector();
@@ -69,4 +82,80 @@
assertThat(ctx.getUpdateType()).isEqualTo(RefUpdateType.DIRECT_PUSH);
assertThat(ctx.getJustification()).hasValue("test justification");
}
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ values = {"GerritBackendFeature__use_direct_push_context_for_submit_on_push"})
+ public void submitOnPushWithoutJustification_emptyJustification() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.SUBMIT).ref("refs/for/refs/heads/master").group(adminGroupUuid()))
+ .update();
+
+ PushOneCommit push =
+ pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");
+ push.setPushOptions(ImmutableList.of("submit"));
+ PushOneCommit.Result result = push.to("refs/for/master");
+
+ result.assertOkStatus();
+ ImmutableList<Entry<String, ImmutableList<RefUpdateContext>>> updates =
+ refUpdateContextCollector.getContextsByUpdateType(RefUpdateType.DIRECT_PUSH);
+ ImmutableMap<String, ImmutableList<RefUpdateContext>> refUpdateContextsByRef =
+ updates.stream().collect(toImmutableMap(Entry::getKey, Entry::getValue));
+ assertThat(refUpdateContextsByRef.keySet())
+ .containsExactly("refs/heads/master", RefNames.changeMetaRef(result.getChange().getId()));
+
+ ImmutableList<RefUpdateContext> ctxts = refUpdateContextsByRef.get("refs/heads/master");
+ ImmutableListMultimap<RefUpdateContext.RefUpdateType, RefUpdateContext>
+ refUpdateContextsByType =
+ ctxts.stream()
+ .collect(toImmutableListMultimap(RefUpdateContext::getUpdateType, identity()));
+ assertThat(refUpdateContextsByType.keySet())
+ .containsExactly(RefUpdateType.DIRECT_PUSH, RefUpdateType.MERGE_CHANGE);
+ ImmutableList<RefUpdateContext> directPushRefUpdatesContexts =
+ refUpdateContextsByType.get(RefUpdateType.DIRECT_PUSH);
+ assertThat(directPushRefUpdatesContexts).hasSize(1);
+ RefUpdateContext directPushUpdateCtx = directPushRefUpdatesContexts.get(0);
+ assertThat(directPushUpdateCtx.getJustification()).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ values = {"GerritBackendFeature__use_direct_push_context_for_submit_on_push"})
+ public void submitOnPushWithJustification_justificationStoredInContext() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.SUBMIT).ref("refs/for/refs/heads/master").group(adminGroupUuid()))
+ .update();
+
+ PushOneCommit push =
+ pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");
+ push.setPushOptions(ImmutableList.of("push-justification=test justification", "submit"));
+ PushOneCommit.Result result = push.to("refs/for/master");
+
+ result.assertOkStatus();
+ ImmutableList<Entry<String, ImmutableList<RefUpdateContext>>> updates =
+ refUpdateContextCollector.getContextsByUpdateType(RefUpdateType.DIRECT_PUSH);
+ ImmutableMap<String, ImmutableList<RefUpdateContext>> refUpdateContextsByRef =
+ updates.stream().collect(toImmutableMap(Entry::getKey, Entry::getValue));
+ assertThat(refUpdateContextsByRef.keySet())
+ .containsExactly("refs/heads/master", RefNames.changeMetaRef(result.getChange().getId()));
+
+ ImmutableList<RefUpdateContext> ctxts = refUpdateContextsByRef.get("refs/heads/master");
+ ImmutableListMultimap<RefUpdateContext.RefUpdateType, RefUpdateContext>
+ refUpdateContextsByType =
+ ctxts.stream()
+ .collect(toImmutableListMultimap(RefUpdateContext::getUpdateType, identity()));
+ assertThat(refUpdateContextsByType.keySet())
+ .containsExactly(RefUpdateType.DIRECT_PUSH, RefUpdateType.MERGE_CHANGE);
+ ImmutableList<RefUpdateContext> directPushRefUpdatesContexts =
+ refUpdateContextsByType.get(RefUpdateType.DIRECT_PUSH);
+ assertThat(directPushRefUpdatesContexts).hasSize(1);
+ RefUpdateContext directPushUpdateCtx = directPushRefUpdatesContexts.get(0);
+ assertThat(directPushUpdateCtx.getJustification()).hasValue("test justification");
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
index 2667851..d544de1 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.toList;
@@ -318,6 +319,27 @@
}
@Test
+ public void voteableOnRefForChangeOwners() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ configLabel("bar", LabelFunction.NO_OP);
+
+ // Grant permissions to read config and vote on 'foo' label with full range (-2 to +2)
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref(RefNames.REFS_CONFIG).group(REGISTERED_USERS))
+ .add(allowLabel("foo").ref("refs/heads/master").group(CHANGE_OWNER).range(-2, 2))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+
+ List<LabelDefinitionInfo> labels =
+ gApi.projects().name(project.get()).labels().withVoteableOnRef("refs/heads/master").get();
+
+ assertThat(labelNames(labels)).containsExactly("foo");
+ }
+
+ @Test
public void voteableOnRefRespectsBranchRestrictions() throws Exception {
projectOperations
.project(project)
diff --git a/plugins/codemirror-editor b/plugins/codemirror-editor
index 5248349..331d172 160000
--- a/plugins/codemirror-editor
+++ b/plugins/codemirror-editor
@@ -1 +1 @@
-Subproject commit 5248349d072fb4cff7d4a2eeea63bb652836c1f1
+Subproject commit 331d1728d9a74fa9cdffbba3c85645a36419eff6
diff --git a/plugins/webhooks b/plugins/webhooks
index 8cf6f0c..788cf1a 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit 8cf6f0c18115f19ddaf6eb7aa2ba624d07ec107b
+Subproject commit 788cf1a335c25a96874b99b74d7e4183b111d2f1
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 000519b..261ca26 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -346,6 +346,13 @@
range: CommentRange | undefined;
}
+/** The detail of the 'range-selected-mouse-up' event dispatched by gr-diff. */
+export declare interface RangeSelectedEventDetail {
+ side: Side;
+ lineNum: LineNumber;
+ range: CommentRange | undefined;
+}
+
export declare interface ContentLoadNeededEventDetail {
lineRange: {
left: LineRange;
diff --git a/polygerrit-ui/app/api/plugin.ts b/polygerrit-ui/app/api/plugin.ts
index a952204..4e0fe9b 100644
--- a/polygerrit-ui/app/api/plugin.ts
+++ b/polygerrit-ui/app/api/plugin.ts
@@ -28,6 +28,8 @@
LABEL_CHANGE = 'labelchange',
SHOW_CHANGE = 'showchange',
SUBMIT_CHANGE = 'submitchange',
+ // Fires GerritView values such as 'change', 'dashboard', 'admin', ...
+ VIEW_CHANGE = 'view-change',
SHOW_REVISION_ACTIONS = 'show-revision-actions',
COMMIT_MSG_EDIT = 'commitmsgedit',
REVERT = 'revert',
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
index 58827fb..1e5736b 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
@@ -16,6 +16,7 @@
import '../gr-repo-commands/gr-repo-commands';
import '../gr-repo-dashboards/gr-repo-dashboards';
import '../gr-repo-detail-list/gr-repo-detail-list';
+import '../gr-repo-submit-requirements/gr-repo-submit-requirements';
import '../gr-repo-list/gr-repo-list';
import '../gr-server-info/gr-server-info';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
@@ -214,7 +215,7 @@
${this.renderGroupMembers()} ${this.renderGroupAuditLog()}
${this.renderRepoDetailList()} ${this.renderRepoCommands()}
${this.renderRepoAccess()} ${this.renderRepoDashboards()}
- ${this.renderServerInfo()}
+ ${this.renderRepoSubmitRequirements()} ${this.renderServerInfo()}
`;
}
@@ -449,6 +450,20 @@
`;
}
+ private renderRepoSubmitRequirements() {
+ if (this.view !== GerritView.REPO) return nothing;
+ if (this.repoViewState?.detail !== RepoDetailView.SUBMIT_REQUIREMENTS)
+ return nothing;
+
+ return html`
+ <div class="main table breadcrumbs">
+ <gr-repo-submit-requirements
+ .repo=${this.repoViewState.repo}
+ ></gr-repo-submit-requirements>
+ </div>
+ `;
+ }
+
private renderServerInfo() {
if (this.view !== GerritView.ADMIN) return nothing;
if (this.adminViewState?.adminView !== AdminChildView.SERVER_INFO)
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
index 34e8d24..f26610d 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
@@ -170,7 +170,7 @@
);
assert.equal(
queryAndAssert<GrDropdownList>(element, '#pageSelect').items!.length,
- 7
+ 8
);
});
@@ -370,6 +370,12 @@
detailType: RepoDetailView.DASHBOARDS,
url: '/admin/repos/my-repo,dashboards',
},
+ {
+ name: 'Submit Requirements',
+ view: GerritView.REPO,
+ detailType: RepoDetailView.SUBMIT_REQUIREMENTS,
+ url: '/admin/repos/my-repo,submit-requirements',
+ },
],
},
},
@@ -450,6 +456,14 @@
detailType: RepoDetailView.DASHBOARDS,
parent: 'my-repo' as RepoName,
},
+ {
+ text: 'Submit Requirements',
+ value: 'reposubmit-requirements',
+ view: GerritView.REPO,
+ url: '/admin/repos/my-repo,submit-requirements',
+ detailType: RepoDetailView.SUBMIT_REQUIREMENTS,
+ parent: 'my-repo' as RepoName,
+ },
];
const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
const selectedIsCurrentPageSpy = sinon.spy(
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
index e099184..de32b7f 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
@@ -10,11 +10,12 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {tableStyles} from '../../../styles/gr-table-styles';
import {LitElement, css, html, PropertyValues} from 'lit';
-import {customElement, property} from 'lit/decorators.js';
+import {customElement, property, state} from 'lit/decorators.js';
import {
DashboardType,
createDashboardUrl,
} from '../../../models/views/dashboard';
+import {when} from 'lit/directives/when.js';
interface DashboardRef {
section: string;
@@ -26,11 +27,11 @@
@property({type: String})
repo?: RepoName;
- @property({type: Boolean})
- _loading = true;
+ @state()
+ loading = true;
- @property({type: Array})
- _dashboards?: DashboardRef[];
+ @state()
+ dashboards?: DashboardRef[];
private readonly restApiService = getAppContext().restApiService;
@@ -43,23 +44,13 @@
display: block;
margin-bottom: var(--spacing-xxl);
}
- .loading #dashboards,
- #loadingContainer {
- display: none;
- }
- .loading #loadingContainer {
- display: block;
- }
`,
];
}
override render() {
- return html` <table
- id="list"
- class="genericList ${this._computeLoadingClass(this._loading)}"
- >
- <tbody>
+ return html` <table id="list" class="genericList">
+ <tbody id="dashboards">
<tr class="headerRow">
<th class="topHeader">Dashboard name</th>
<th class="topHeader">Dashboard title</th>
@@ -67,36 +58,39 @@
<th class="topHeader">Inherited from</th>
<th class="topHeader">Default</th>
</tr>
- <tr id="loadingContainer">
- <td>Loading...</td>
- </tr>
- </tbody>
- <tbody id="dashboards">
- ${(this._dashboards ?? []).map(
- item => html`
- <tr class="groupHeader">
- <td colspan="5">${item.section}</td>
- </tr>
- ${(item.dashboards ?? []).map(
- info => html`
- <tr class="table">
- <td class="name">
- <a href=${this._getUrl(info.project, info.id)}
- >${info.path}</a
- >
- </td>
- <td class="title">${info.title}</td>
- <td class="desc">${info.description}</td>
- <td class="inherited">
- ${this._computeInheritedFrom(
- info.project,
- info.defining_project
- )}
- </td>
- <td class="default">
- ${this._computeIsDefault(info.is_default)}
- </td>
+ ${when(
+ this.loading,
+ () => html`<tr id="loadingContainer">
+ <td>Loading...</td>
+ </tr>`,
+ () => html`
+ ${(this.dashboards ?? []).map(
+ item => html`
+ <tr class="groupHeader">
+ <td colspan="5">${item.section}</td>
</tr>
+ ${(item.dashboards ?? []).map(
+ info => html`
+ <tr class="table">
+ <td class="name">
+ <a href=${this.getUrl(info.project, info.id)}
+ >${info.path}</a
+ >
+ </td>
+ <td class="title">${info.title}</td>
+ <td class="desc">${info.description}</td>
+ <td class="inherited">
+ ${this.computeInheritedFrom(
+ info.project,
+ info.defining_project
+ )}
+ </td>
+ <td class="default">
+ ${this.computeIsDefault(info.is_default)}
+ </td>
+ </tr>
+ `
+ )}
`
)}
`
@@ -113,7 +107,7 @@
private repoChanged() {
const repo = this.repo;
- this._loading = true;
+ this.loading = true;
if (!repo) {
return Promise.resolve();
}
@@ -151,26 +145,22 @@
});
});
- this._dashboards = dashboardBuilder;
- this._loading = false;
+ this.dashboards = dashboardBuilder;
+ this.loading = false;
});
}
- _getUrl(project?: RepoName, dashboard?: DashboardId) {
+ private getUrl(project?: RepoName, dashboard?: DashboardId) {
if (!project || !dashboard) return '';
return createDashboardUrl({project, type: DashboardType.REPO, dashboard});
}
- _computeLoadingClass(loading: boolean) {
- return loading ? 'loading' : '';
- }
-
- _computeInheritedFrom(project: RepoName, definingProject: RepoName) {
+ private computeInheritedFrom(project: RepoName, definingProject: RepoName) {
return project === definingProject ? '' : definingProject;
}
- _computeIsDefault(isDefault?: boolean) {
+ private computeIsDefault(isDefault?: boolean) {
return isDefault ? '✓' : '';
}
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.ts
index 2bbc28b..b09212e 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.ts
@@ -9,7 +9,7 @@
import {
addListenerForTest,
mockPromise,
- queryAndAssert,
+ query,
stubRestApi,
waitEventLoop,
} from '../../../test/test-utils';
@@ -83,8 +83,8 @@
assert.shadowDom.equal(
element,
/* HTML */ `
- <table class="genericList loading" id="list">
- <tbody>
+ <table class="genericList" id="list">
+ <tbody id="dashboards">
<tr class="headerRow">
<th class="topHeader">Dashboard name</th>
<th class="topHeader">Dashboard title</th>
@@ -96,34 +96,20 @@
<td>Loading...</td>
</tr>
</tbody>
- <tbody id="dashboards"></tbody>
</table>
`
);
});
test('loading, sections, and ordering', async () => {
- assert.isTrue(element._loading);
- assert.notEqual(
- getComputedStyle(queryAndAssert(element, '#loadingContainer')).display,
- 'none'
- );
- assert.equal(
- getComputedStyle(queryAndAssert(element, '#dashboards')).display,
- 'none'
- );
+ assert.isTrue(element.loading);
+ assert.isOk(query(element, '#loadingContainer'));
+
element.repo = 'test' as RepoName;
await waitEventLoop();
- assert.equal(
- getComputedStyle(queryAndAssert(element, '#loadingContainer')).display,
- 'none'
- );
- assert.notEqual(
- getComputedStyle(queryAndAssert(element, '#dashboards')).display,
- 'none'
- );
+ assert.isNotOk(query(element, '#loadingContainer'));
- const dashboard = element._dashboards!;
+ const dashboard = element.dashboards!;
assert.equal(dashboard.length, 2);
assert.equal(dashboard[0].section, 'custom');
assert.equal(dashboard[1].section, 'default');
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-submit-requirements/gr-repo-submit-requirements.ts b/polygerrit-ui/app/elements/admin/gr-repo-submit-requirements/gr-repo-submit-requirements.ts
new file mode 100644
index 0000000..6de8705
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-repo-submit-requirements/gr-repo-submit-requirements.ts
@@ -0,0 +1,129 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {RepoName, SubmitRequirementInfo} from '../../../types/common';
+import {firePageError} from '../../../utils/event-util';
+import {getAppContext} from '../../../services/app-context';
+import {ErrorCallback} from '../../../api/rest';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {tableStyles} from '../../../styles/gr-table-styles';
+import {LitElement, css, html, PropertyValues} from 'lit';
+import {customElement, property, state} from 'lit/decorators.js';
+import {when} from 'lit/directives/when.js';
+
+@customElement('gr-repo-submit-requirements')
+export class GrRepoSubmitRequirements extends LitElement {
+ @property({type: String})
+ repo?: RepoName;
+
+ @state()
+ loading = true;
+
+ @state()
+ submitRequirements?: SubmitRequirementInfo[];
+
+ private readonly restApiService = getAppContext().restApiService;
+
+ static override get styles() {
+ return [
+ sharedStyles,
+ tableStyles,
+ css`
+ :host {
+ display: block;
+ margin-bottom: var(--spacing-xxl);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html` <table id="list" class="genericList">
+ <tbody>
+ <tr class="headerRow">
+ <th class="topHeader">Name</th>
+ <th class="topHeader">Description</th>
+ <th class="topHeader">Applicability Expression</th>
+ <th class="topHeader">Submittability Expression</th>
+ <th class="topHeader">Override Expression</th>
+ <th
+ class="topHeader"
+ title="Whether override is allowed in child projects"
+ >
+ Allow Override
+ </th>
+ </tr>
+ </tbody>
+ <tbody id="submit-requirements">
+ ${when(
+ this.loading,
+ () => html`<tr id="loadingContainer">
+ <td>Loading...</td>
+ </tr>`,
+ () =>
+ html` ${(this.submitRequirements ?? []).map(
+ item => html`
+ <tr class="table">
+ <td class="name">${item.name}</td>
+ <td class="desc">${item.description}</td>
+ <td class="applicability">
+ ${item.applicability_expression}
+ </td>
+ <td class="submittability">
+ ${item.submittability_expression}
+ </td>
+ <td class="override">${item.override_expression}</td>
+ <td class="allowOverride">
+ ${this.renderCheckmark(
+ item.allow_override_in_child_projects
+ )}
+ </td>
+ </tr>
+ `
+ )}`
+ )}
+ </tbody>
+ </table>`;
+ }
+
+ override updated(changedProperties: PropertyValues) {
+ if (changedProperties.has('repo')) {
+ this.repoChanged();
+ }
+ }
+
+ private repoChanged() {
+ const repo = this.repo;
+ this.loading = true;
+ if (!repo) {
+ return Promise.resolve();
+ }
+
+ const errFn: ErrorCallback = response => {
+ firePageError(response);
+ };
+
+ return this.restApiService
+ .getRepoSubmitRequirements(repo, errFn)
+ .then((res?: SubmitRequirementInfo[]) => {
+ if (!res) {
+ return;
+ }
+
+ this.submitRequirements = res;
+ this.loading = false;
+ });
+ }
+
+ private renderCheckmark(check?: boolean) {
+ return check ? '✓' : '';
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-repo-submit-requirements': GrRepoSubmitRequirements;
+ }
+}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-submit-requirements/gr-repo-submit-requirements_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-submit-requirements/gr-repo-submit-requirements_test.ts
new file mode 100644
index 0000000..f438a68
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-repo-submit-requirements/gr-repo-submit-requirements_test.ts
@@ -0,0 +1,106 @@
+/**
+ * @license
+ * Copyright 2017 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-repo-submit-requirements';
+import {GrRepoSubmitRequirements} from './gr-repo-submit-requirements';
+import {stubRestApi, waitEventLoop} from '../../../test/test-utils';
+import {RepoName, SubmitRequirementInfo} from '../../../types/common';
+import {fixture, html, assert} from '@open-wc/testing';
+
+suite('gr-repo-submit-requirements tests', () => {
+ let element: GrRepoSubmitRequirements;
+
+ setup(async () => {
+ element = await fixture(
+ html`<gr-repo-submit-requirements></gr-repo-submit-requirements>`
+ );
+ });
+
+ suite('submit requirements table', () => {
+ setup(() => {
+ stubRestApi('getRepoSubmitRequirements').returns(
+ Promise.resolve([
+ {
+ name: 'Verified',
+ description: 'CI result status for build and tests is passing',
+ submittability_expression:
+ 'label:Verified=MAX AND -label:Verified=MIN',
+ },
+ ] as SubmitRequirementInfo[])
+ );
+ element.repo = 'test' as RepoName;
+ });
+
+ test('render loading', () => {
+ element.repo = 'test2' as RepoName;
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `<table class="genericList" id="list">
+ <tbody>
+ <tr class="headerRow">
+ <th class="topHeader">Name</th>
+ <th class="topHeader">Description</th>
+ <th class="topHeader">Applicability Expression</th>
+ <th class="topHeader">Submittability Expression</th>
+ <th class="topHeader">Override Expression</th>
+ <th
+ class="topHeader"
+ title="Whether override is allowed in child projects"
+ >
+ Allow Override
+ </th>
+ </tr>
+ </tbody>
+ <tbody id="submit-requirements">
+ <tr id="loadingContainer">
+ <td>Loading...</td>
+ </tr>
+ </tbody>
+ </table>`
+ );
+ });
+
+ test('render', async () => {
+ await waitEventLoop();
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <table class="genericList" id="list">
+ <tbody>
+ <tr class="headerRow">
+ <th class="topHeader">Name</th>
+ <th class="topHeader">Description</th>
+ <th class="topHeader">Applicability Expression</th>
+ <th class="topHeader">Submittability Expression</th>
+ <th class="topHeader">Override Expression</th>
+ <th
+ class="topHeader"
+ title="Whether override is allowed in child projects"
+ >
+ Allow Override
+ </th>
+ </tr>
+ </tbody>
+ <tbody id="submit-requirements">
+ <tr class="table">
+ <td class="name">Verified</td>
+ <td class="desc">
+ CI result status for build and tests is passing
+ </td>
+ <td class="applicability"></td>
+ <td class="submittability">
+ label:Verified=MAX AND -label:Verified=MIN
+ </td>
+ <td class="override"></td>
+ <td class="allowOverride"></td>
+ </tr>
+ </tbody>
+ </table>
+ `
+ );
+ });
+ });
+});
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index ae254be..a26d37f8f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -1670,7 +1670,7 @@
let expectedMsg =
'Revert submission 199 0' +
'\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>' +
+ 'Reason for revert: <MUST SPECIFY REASON HERE>' +
'\n\n' +
'Reverted changes: /q/submissionid:199+0\n';
assert.equal(confirmRevertDialog.message, expectedMsg);
@@ -1683,7 +1683,7 @@
expectedMsg =
'Revert "random commit message"\n\nThis reverts ' +
'commit 2000.\n\nReason' +
- ' for revert: <INSERT REASONING HERE>\n';
+ ' for revert: <MUST SPECIFY REASON HERE>\n';
assert.equal(confirmRevertDialog.message, expectedMsg);
});
@@ -1733,13 +1733,13 @@
const revertSubmissionMsg =
'Revert submission 199 0' +
'\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>' +
+ 'Reason for revert: <MUST SPECIFY REASON HERE>' +
'\n\n' +
'Reverted changes: /q/submissionid:199+0\n';
const singleChangeMsg =
'Revert "random commit message"\n\nThis reverts ' +
'commit 2000.\n\nReason' +
- ' for revert: <INSERT REASONING HERE>\n';
+ ' for revert: <MUST SPECIFY REASON HERE>\n';
assert.equal(confirmRevertDialog.message, revertSubmissionMsg);
const newRevertMsg = revertSubmissionMsg + 'random';
const newSingleChangeMsg = singleChangeMsg + 'random';
@@ -1833,7 +1833,7 @@
const msg =
'Revert "random commit message"\n\n' +
'This reverts commit 2000.\n\nReason ' +
- 'for revert: <INSERT REASONING HERE>\n';
+ 'for revert: <MUST SPECIFY REASON HERE>\n';
assert.equal(confirmRevertDialog.message, msg);
let editedMsg = msg + 'hello';
confirmRevertDialog.message += 'hello';
@@ -1852,10 +1852,10 @@
// Contains generic template reason so doesn't submit
assert.isFalse(fireActionStub.called);
confirmRevertDialog.message = confirmRevertDialog.message.replace(
- '<INSERT REASONING HERE>',
+ '<MUST SPECIFY REASON HERE>',
''
);
- editedMsg = editedMsg.replace('<INSERT REASONING HERE>', '');
+ editedMsg = editedMsg.replace('<MUST SPECIFY REASON HERE>', '');
confirmButton.click();
await element.updateComplete;
assert.equal(fireActionStub.getCall(0).args[0], '/revert');
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index fb0b975..dec8def 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -563,7 +563,15 @@
subscribe(
this,
() => this.getViewModel().tab$,
- t => (this.activeTab = t)
+ t => {
+ // We don't want to jump during the initial page load. This scrolling
+ // works only when change is already loaded and user clicks on link
+ // to same page when tabs are not in view, but tabs needs to be rendered.
+ if (this.tabs) {
+ this.tabs.scrollIntoView({block: 'nearest'});
+ }
+ this.activeTab = t;
+ }
);
subscribe(
this,
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
index 3d69471..59ba256 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -24,9 +24,10 @@
import {ParsedChangeInfo} from '../../../types/types';
import {formStyles} from '../../../styles/form-styles';
import {GrValidationOptions} from '../gr-validation-options/gr-validation-options';
+import {parseCommitMessageString} from '../../../utils/commit-message-formatter-util';
const ERR_COMMIT_NOT_FOUND = 'Unable to find the commit hash of this change.';
-const INSERT_REASON_STRING = '<INSERT REASONING HERE>';
+const SPECIFY_REASON_STRING = '<MUST SPECIFY REASON HERE>';
// TODO(dhruvsri): clean up repeated definitions after moving to js modules
export enum RevertType {
@@ -230,27 +231,39 @@
commitMessage: string,
commitHash?: CommitId
) {
- // Figure out what the revert title should be.
- const originalTitle = (commitMessage || '').split('\n')[0];
- let revertTitle = `Revert "${originalTitle}"`;
- const match = originalTitle.match(/^Revert(?:\^([0-9]+))? "(.*)"$/);
- if (match) {
- let revertNum = 2;
- if (match[1]) {
- revertNum = Number(match[1]) + 1;
- }
- revertTitle = `Revert^${revertNum} "${match[2]}"`;
- }
-
if (!commitHash) {
fireAlert(this, ERR_COMMIT_NOT_FOUND);
return;
}
+
+ // Figure out what the revert title should be.
+ const originalTitle = (commitMessage || '').split('\n')[0];
+ let revertTitle = `Revert "${originalTitle}"`;
+ const revertTitleRegex = originalTitle.match(
+ /^Revert(?:\^([0-9]+))? "(.*)"$/
+ );
+
+ const footers = parseCommitMessageString(commitMessage).footer.filter(
+ f => f.startsWith('Issue: ') || f.startsWith('Bug: ')
+ );
+
+ if (revertTitleRegex) {
+ let revertNum = 2;
+ if (revertTitleRegex[1]) {
+ revertNum = Number(revertTitleRegex[1]) + 1;
+ }
+ revertTitle = `Revert^${revertNum} "${revertTitleRegex[2]}"`;
+ }
+
const revertCommitText = `This reverts commit ${commitHash}.`;
- const message =
+ let message =
`${revertTitle}\n\n${revertCommitText}\n\n` +
- `Reason for revert: ${INSERT_REASON_STRING}\n`;
+ `Reason for revert: ${SPECIFY_REASON_STRING}\n`;
+
+ if (footers.length > 0) {
+ message += '\n' + footers.join('\n'); // Empty line before the footers begin
+ }
// This is to give plugins a chance to update message
this.message = this.modifyRevertMsg(change, commitMessage, message);
this.revertType = RevertType.REVERT_SINGLE_CHANGE;
@@ -285,8 +298,7 @@
const message =
`Revert submission ${change.submission_id}` +
'\n\n' +
- 'Reason for revert: <INSERT ' +
- 'REASONING HERE>\n\n' +
+ `Reason for revert: ${SPECIFY_REASON_STRING}\n\n` +
'Reverted changes: ' +
createSearchUrl({query: `submissionid:${change.submission_id}`}) +
'\n';
@@ -326,7 +338,7 @@
e.stopPropagation();
if (
this.message === this.originalRevertMessages[this.revertType] ||
- this.message.includes(INSERT_REASON_STRING)
+ this.message.includes(SPECIFY_REASON_STRING)
) {
this.showErrorMessage = true;
return;
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
index f4abe0c..1e5a194 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
@@ -67,7 +67,7 @@
const expected =
'Revert "one line commit"\n\n' +
'This reverts commit abcd123.\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>\n';
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n';
assert.equal(element.message, expected);
});
@@ -81,7 +81,65 @@
const expected =
'Revert "many lines"\n\n' +
'This reverts commit abcd123.\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>\n';
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n';
+ assert.equal(element.message, expected);
+ });
+
+ test('populateRevertSingleChangeMessage parses Issues footer from commit message', () => {
+ assert.isNotOk(element.message);
+ element.populateRevertSingleChangeMessage(
+ createParsedChange(),
+ 'much lines\nvery\n\ncommit\nIssue: 1234567\nChange-Id: abcdefg\n',
+ 'abcd123' as CommitId
+ );
+ const expected =
+ 'Revert "much lines"\n\n' +
+ 'This reverts commit abcd123.\n\n' +
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n\n' +
+ 'Issue: 1234567';
+ assert.equal(element.message, expected);
+ });
+
+ test('populateRevertSingleChangeMessage does not parse Issue: from commit message body', () => {
+ assert.isNotOk(element.message);
+ element.populateRevertSingleChangeMessage(
+ createParsedChange(),
+ 'much lines\n\nIssue: 1234567very\n\ncommit\nChange-Id: abcdefg\n',
+ 'abcd123' as CommitId
+ );
+ const expected =
+ 'Revert "much lines"\n\n' +
+ 'This reverts commit abcd123.\n\n' +
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n';
+ assert.equal(element.message, expected);
+ });
+
+ test('populateRevertSingleChangeMessage parses Bug footer from commit message', () => {
+ assert.isNotOk(element.message);
+ element.populateRevertSingleChangeMessage(
+ createParsedChange(),
+ 'much lines\nvery\n\ncommit\nBug: 1234567\nChange-Id: abcdefg\n',
+ 'abcd123' as CommitId
+ );
+ const expected =
+ 'Revert "much lines"\n\n' +
+ 'This reverts commit abcd123.\n\n' +
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n\n' +
+ 'Bug: 1234567';
+ assert.equal(element.message, expected);
+ });
+
+ test('populateRevertSingleChangeMessage does not parse Bug: from commit message body', () => {
+ assert.isNotOk(element.message);
+ element.populateRevertSingleChangeMessage(
+ createParsedChange(),
+ 'much lines\n\nBug: 1234567very\n\ncommit\nChange-Id: abcdefg\n',
+ 'abcd123' as CommitId
+ );
+ const expected =
+ 'Revert "much lines"\n\n' +
+ 'This reverts commit abcd123.\n\n' +
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n';
assert.equal(element.message, expected);
});
@@ -95,7 +153,8 @@
const expected =
'Revert "much lines"\n\n' +
'This reverts commit abcd123.\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>\n';
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n\n' +
+ 'Bug: Issue 42';
assert.equal(element.message, expected);
});
@@ -109,7 +168,7 @@
const expected =
'Revert^2 "one line commit"\n\n' +
'This reverts commit abcd123.\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>\n';
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n';
assert.equal(element.message, expected);
});
@@ -123,7 +182,7 @@
const expected =
'Revert^3 "one line commit"\n\n' +
'This reverts commit abcd123.\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>\n';
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n';
assert.equal(element.message, expected);
});
@@ -140,7 +199,7 @@
const expected =
'Revert submission 5545\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>\n\n' +
+ 'Reason for revert: <MUST SPECIFY REASON HERE>\n\n' +
'Reverted changes: /q/submissionid:5545\n';
assert.equal(element.message, expected);
});
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 6a72d41..200db91 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -626,6 +626,11 @@
this.isOwner &&
// without fixes
!this.result?.fixes?.length &&
+ // disable on codepointer with range pointing to 0 line
+ this.result?.codePointers?.[0]?.range.start_line !== 0 &&
+ this.result?.codePointers?.[0]?.range.end_line !== 0 &&
+ this.result?.category !== Category.SUCCESS &&
+ this.result?.category !== Category.INFO &&
!fixAction
) {
getAiFixAction = createGetAiFixAction(this);
@@ -725,6 +730,8 @@
fireAlert(this, 'No suitable AI fix could be found');
return;
}
+ suggestion.description =
+ ReportSource.GET_AI_FIX_FOR_CHECK + ' ' + suggestion.description;
this.suggestion = suggestion;
this.toggleExpanded(/* setExpanded= */ true);
}
diff --git a/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts b/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
index 5a79b8d..0da5336 100644
--- a/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
+++ b/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
@@ -18,7 +18,7 @@
import './gr-checks-results';
import './gr-hovercard-run';
import {fontStyles} from '../../styles/gr-font-styles';
-import {Action} from '../../api/checks';
+import {Action, Category} from '../../api/checks';
import {assertIsDefined} from '../../utils/common-util';
import {resolve} from '../../models/dependency';
import {commentsModelToken} from '../../models/comments/comments-model';
@@ -298,6 +298,19 @@
if (this.result?.fixes?.length) {
return false;
}
+ if (
+ this.result?.category === Category.SUCCESS ||
+ this.result?.category === Category.INFO
+ ) {
+ return false;
+ }
+ // disable on codepointer with range pointing to 0 line
+ if (
+ this.result?.codePointers?.[0]?.range.start_line === 0 ||
+ this.result?.codePointers?.[0]?.range.end_line === 0
+ ) {
+ return false;
+ }
return this.isOwner;
}
@@ -383,6 +396,8 @@
fireAlert(this, 'No suitable AI fix could be found');
return;
}
+ suggestion.description =
+ ReportSource.GET_AI_FIX_FOR_CHECK + ' ' + suggestion.description;
this.suggestion = suggestion;
this.isExpanded = true;
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 1528374..9f87d99 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -167,6 +167,8 @@
// Matches /admin/repos/<repos>,access.
REPO_DASHBOARDS: /^\/admin\/repos\/(.+),dashboards$/,
+ REPO_SUBMIT_REQUIREMENTS: /^\/admin\/repos\/(.+),submit-requirements$/,
+
// Matches /admin/plugins with optional filter and offset.
PLUGIN_LIST: /^\/admin\/plugins\/?(?:\/q\/filter:(.*?))?(?:,(\d+))?$/,
// Matches /admin/groups with optional filter and offset.
@@ -786,6 +788,12 @@
ctx => this.handleRepoDashboardsRoute(ctx)
);
+ this.mapRoute(
+ RoutePattern.REPO_SUBMIT_REQUIREMENTS,
+ 'handleRepoSubmitRequirementsRoute',
+ ctx => this.handleRepoSubmitRequirementsRoute(ctx)
+ );
+
this.mapRoute(RoutePattern.BRANCH_LIST, 'handleBranchListRoute', ctx =>
this.handleBranchListRoute(ctx)
);
@@ -1215,6 +1223,19 @@
this.reporting.setRepoName(repo);
}
+ handleRepoSubmitRequirementsRoute(ctx: PageContext) {
+ const repo = ctx.params[0] as RepoName;
+ const state: RepoViewState = {
+ view: GerritView.REPO,
+ detail: RepoDetailView.SUBMIT_REQUIREMENTS,
+ repo,
+ };
+ // Note that router model view must be updated before view models.
+ this.setState(state);
+ this.repoViewModel.setState(state);
+ this.reporting.setRepoName(repo);
+ }
+
handleBranchListRoute(ctx: PageContext) {
const state: RepoViewState = {
view: GerritView.REPO,
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index 6d61f85..81b1616 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -191,6 +191,7 @@
'handleRepoGeneralRoute',
'handleRepoListRoute',
'handleRepoRoute',
+ 'handleRepoSubmitRequirementsRoute',
'handleQueryLegacySuffixRoute',
'handleQueryRoute',
'handleRegisterRoute',
@@ -727,6 +728,15 @@
});
});
+ test('REPO_SUBMIT_REQUIREMENTS', async () => {
+ // REPO_SUBMIT_REQUIREMENTS: /^\/admin\/repos\/(.+),submit-requirements$/,
+ await checkUrlToState('/admin/repos/4321,submit-requirements', {
+ ...createRepoViewState(),
+ detail: RepoDetailView.SUBMIT_REQUIREMENTS,
+ repo: '4321' as RepoName,
+ });
+ });
+
test('BRANCH_LIST', async () => {
await checkUrlToState('/admin/repos/4321,branches', {
...createRepoBranchesViewState(),
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 77b7e41..2b3be7f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -64,6 +64,7 @@
LineNumber,
LineSelectedEventDetail,
LOST,
+ RangeSelectedEventDetail,
RenderPreferences,
} from '../../../api/diff';
import {resolve} from '../../../models/dependency';
@@ -114,6 +115,7 @@
// prettier-ignore
'render': CustomEvent<{}>;
'create-comment': CustomEvent<CreateCommentEventDetail>;
+ 'range-selected-mouse-up': CustomEvent<RangeSelectedEventDetail>;
'is-blame-loaded-changed': ValueChangedEvent<boolean>;
'diff-changed': ValueChangedEvent<DiffInfo | undefined>;
'edit-weblinks-changed': ValueChangedEvent<WebLinkInfo[] | undefined>;
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 b08e9a5..89b2a6e 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
@@ -625,22 +625,6 @@
${
this.unresolved
? html`
- <gr-button
- id="ackBtn"
- link
- class="action ack"
- ?disabled=${this.saving}
- @click=${this.handleCommentAck}
- >Ack</gr-button
- >
- <gr-button
- id="doneBtn"
- link
- class="action done"
- ?disabled=${this.saving}
- @click=${this.handleCommentDone}
- >Done</gr-button
- >
${this.shouldShowAIFixButton()
? html`
<gr-button
@@ -657,6 +641,22 @@
>
`
: nothing}
+ <gr-button
+ id="ackBtn"
+ link
+ class="action ack"
+ ?disabled=${this.saving}
+ @click=${this.handleCommentAck}
+ >Ack</gr-button
+ >
+ <gr-button
+ id="doneBtn"
+ link
+ class="action done"
+ ?disabled=${this.saving}
+ @click=${this.handleCommentDone}
+ >Done</gr-button
+ >
`
: ''
}
@@ -1014,6 +1014,9 @@
fireAlert(this, 'No suitable AI fix could be found');
return;
}
+ // Description is used to identify the suggestion source in logs.
+ suggestion.description =
+ ReportSource.GET_AI_FIX_FOR_COMMENT + ' ' + suggestion.description;
this.suggestion = suggestion;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 44f4f84..461928e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -662,6 +662,16 @@
</gr-button>
<gr-button
aria-disabled="false"
+ class="action ai-fix"
+ id="aiFixBtn"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ Get AI Fix
+ </gr-button>
+ <gr-button
+ aria-disabled="false"
class="action ack"
id="ackBtn"
link=""
@@ -680,16 +690,6 @@
>
Done
</gr-button>
- <gr-button
- aria-disabled="false"
- class="action ai-fix"
- id="aiFixBtn"
- link=""
- role="button"
- tabindex="0"
- >
- Get AI Fix
- </gr-button>
<gr-icon
icon="link"
class="copy link-icon"
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.ts b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.ts
index d6a01ab..c8365c0 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.ts
@@ -99,6 +99,16 @@
);
});
+ test('Within one week', async () => {
+ await testDates(
+ '2015-07-22 03:34:14.985000000',
+ '2015-07-29 02:25:14.985000000',
+ 'Jul 29',
+ 'Jul 29 02:25',
+ 'Wednesday, Jul 29, 2015, 02:25:14'
+ );
+ });
+
test('More than 24 hours but less than six months', async () => {
await testDates(
'2015-07-29 20:34:14.985000000',
@@ -147,6 +157,16 @@
);
});
+ test('Within one week', async () => {
+ await testDates(
+ '2015-07-22 03:34:14.985000000',
+ '2015-07-29 02:25:14.985000000',
+ '07/29',
+ '07/29 02:25',
+ 'Wednesday, 07/29/15, 02:25:14'
+ );
+ });
+
test('More than 24 hours but less than six months', async () => {
await testDates(
'2015-07-29 20:34:14.985000000',
@@ -185,6 +205,16 @@
);
});
+ test('Within one week', async () => {
+ await testDates(
+ '2015-07-22 03:34:14.985000000',
+ '2015-07-29 02:25:14.985000000',
+ '07-29',
+ '07-29 02:25',
+ 'Wednesday, 2015-07-29, 02:25:14'
+ );
+ });
+
test('More than 24 hours but less than six months', async () => {
await testDates(
'2015-07-29 20:34:14.985000000',
@@ -223,6 +253,16 @@
);
});
+ test('Within one week', async () => {
+ await testDates(
+ '2015-07-22 03:34:14.985000000',
+ '2015-07-29 02:25:14.985000000',
+ '29. Jul',
+ '29. Jul 02:25',
+ 'Wednesday, 29.07.2015, 02:25:14'
+ );
+ });
+
test('More than 24 hours but less than six months', async () => {
await testDates(
'2015-07-29 20:34:14.985000000',
@@ -261,6 +301,16 @@
);
});
+ test('Within one week', async () => {
+ await testDates(
+ '2015-07-22 03:34:14.985000000',
+ '2015-07-29 02:25:14.985000000',
+ '29/07',
+ '29/07 02:25',
+ 'Wednesday, 29/07/2015, 02:25:14'
+ );
+ });
+
test('More than 24 hours but less than six months', async () => {
await testDates(
'2015-07-29 20:34:14.985000000',
@@ -270,6 +320,16 @@
'Monday, 15/06/2015, 03:25:14'
);
});
+
+ test('More than six months', async () => {
+ await testDates(
+ '2015-07-29 20:34:14.985000000',
+ '2016-08-15 03:25:14.985000000',
+ '15/08/2016',
+ '15/08/2016 03:25',
+ 'Monday, 15/08/2016, 03:25:14'
+ );
+ });
});
suite('STD + 12 hours time format preference', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
index 2ab8bc2..6a2df5f 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
@@ -82,6 +82,17 @@
}
}
+ async handleViewChange(view: string) {
+ await this.waitForPluginsToLoad();
+ for (const cb of this._getEventCallbacks(EventType.VIEW_CHANGE)) {
+ try {
+ cb(view);
+ } catch (err: unknown) {
+ this.reportError(err, EventType.VIEW_CHANGE);
+ }
+ }
+ }
+
async handleShowChange(detail: ShowChangeDetail) {
if (!detail.change) return;
await this.waitForPluginsToLoad();
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
index aa830d8..ba14fd2 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
@@ -49,6 +49,7 @@
handleShowChange(detail: ShowChangeDetail): Promise<void>;
handleShowRevisionActions(detail: ShowRevisionActionsDetail): void;
handleLabelChange(detail: {change?: ParsedChangeInfo}): void;
+ handleViewChange(view?: string): void;
modifyRevertMsg(
change: ChangeInfo,
revertMsg: string,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
index 75d6a57..7eff2f6e 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -393,6 +393,17 @@
return;
}
+ const range = {
+ start_line: start.line,
+ start_character: start.column,
+ end_line: end.line,
+ end_character: end.column,
+ };
+ const side = start.side;
+ this.selectedRange = {range, side};
+ if (isMouseUp) {
+ this.diffBuilder?.diffModel.fireRangeSelectedMouseUpEvent(side, range);
+ }
let actionBox = this.diffTable.querySelector('gr-selection-action-box');
if (!actionBox) {
actionBox = document.createElement('gr-selection-action-box');
@@ -403,15 +414,6 @@
if (hoverCardText) {
actionBox.setAttribute('hoverCardText', hoverCardText);
}
- this.selectedRange = {
- range: {
- start_line: start.line,
- start_character: start.column,
- end_line: end.line,
- end_character: end.column,
- },
- side: start.side,
- };
if (start.line === end.line) {
this.positionActionBox(actionBox, start.line, domRange);
} else if (start.node instanceof Text) {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts b/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
index c802c0c..6678078 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
@@ -16,6 +16,7 @@
DisplayLine,
LineNumber,
LineSelectedEventDetail,
+ RangeSelectedEventDetail,
RenderPreferences,
Side,
SyntaxBlock,
@@ -315,4 +316,13 @@
};
fire(this.eventTarget, 'create-comment', detail);
}
+
+ fireRangeSelectedMouseUpEvent(side: Side, range: CommentRange) {
+ const detail: RangeSelectedEventDetail = {
+ side,
+ lineNum: range.end_line,
+ range,
+ };
+ fire(this.eventTarget, 'range-selected-mouse-up', detail);
+ }
}
diff --git a/polygerrit-ui/app/models/views/admin.ts b/polygerrit-ui/app/models/views/admin.ts
index 164858d..880589a 100644
--- a/polygerrit-ui/app/models/views/admin.ts
+++ b/polygerrit-ui/app/models/views/admin.ts
@@ -275,6 +275,12 @@
detailType: RepoDetailView.DASHBOARDS,
url: createRepoUrl({repo, detail: RepoDetailView.DASHBOARDS}),
},
+ {
+ name: 'Submit Requirements',
+ view: GerritView.REPO,
+ detailType: RepoDetailView.SUBMIT_REQUIREMENTS,
+ url: createRepoUrl({repo, detail: RepoDetailView.SUBMIT_REQUIREMENTS}),
+ },
],
};
}
diff --git a/polygerrit-ui/app/models/views/admin_test.ts b/polygerrit-ui/app/models/views/admin_test.ts
index eab362a..675706f 100644
--- a/polygerrit-ui/app/models/views/admin_test.ts
+++ b/polygerrit-ui/app/models/views/admin_test.ts
@@ -63,7 +63,7 @@
if (expected.projectPageShown) {
assert.isOk(res.links[0].subsection);
- assert.equal(res.links[0].subsection!.children!.length, 6);
+ assert.equal(res.links[0].subsection!.children!.length, 7);
} else {
assert.isNotOk(res.links[0].subsection);
}
@@ -103,7 +103,7 @@
}
if (expected.projectPageShown) {
assert.equal(res.expandedSection!.name, 'my-repo');
- assert.equal(res.expandedSection!.children!.length, 6);
+ assert.equal(res.expandedSection!.children!.length, 7);
} else if (expected.groupPageShown) {
assert.equal(res.expandedSection!.name, 'my-group');
assert.equal(
diff --git a/polygerrit-ui/app/models/views/repo.ts b/polygerrit-ui/app/models/views/repo.ts
index 1629449..ded065d 100644
--- a/polygerrit-ui/app/models/views/repo.ts
+++ b/polygerrit-ui/app/models/views/repo.ts
@@ -17,6 +17,7 @@
COMMANDS = 'commands',
DASHBOARDS = 'dashboards',
TAGS = 'tags',
+ SUBMIT_REQUIREMENTS = 'submit-requirements',
}
export interface RepoViewState extends ViewState {
@@ -49,6 +50,8 @@
url += ',commands';
} else if (state.detail === RepoDetailView.DASHBOARDS) {
url += ',dashboards';
+ } else if (state.detail === RepoDetailView.SUBMIT_REQUIREMENTS) {
+ url += ',submit-requirements';
}
return getBaseUrl() + url;
}
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 9996c75..ab2ae52 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -109,7 +109,7 @@
): Map<DependencyToken<unknown>, Creator<unknown>> {
return new Map<DependencyToken<unknown>, Creator<unknown>>([
[authServiceToken, () => appContext.authService],
- [routerModelToken, () => new RouterModel()],
+ [routerModelToken, () => new RouterModel(resolver(pluginLoaderToken))],
[userModelToken, () => new UserModel(appContext.restApiService)],
[browserModelToken, () => new BrowserModel(resolver(userModelToken))],
[accountsModelToken, () => new AccountsModel(appContext.restApiService)],
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 6beb2aa..4601bd2 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
@@ -106,6 +106,7 @@
DraftInfo,
ListChangesOption,
ReviewResult,
+ SubmitRequirementInfo,
} from '../../types/common';
import {
DiffInfo,
@@ -1773,6 +1774,17 @@
});
}
+ getRepoSubmitRequirements(
+ repoName: RepoName,
+ errFn?: ErrorCallback
+ ): Promise<SubmitRequirementInfo[] | undefined> {
+ return this._restApiHelper.fetchJSON({
+ url: `/projects/${encodeURIComponent(repoName)}/submit_requirements`,
+ errFn,
+ anonymizedUrl: '/projects/*/submit_requirements',
+ }) as Promise<SubmitRequirementInfo[] | undefined>;
+ }
+
getRepoAccessRights(
repoName: RepoName,
errFn?: ErrorCallback
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 8dbf1e9..ea6451a 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
@@ -91,6 +91,7 @@
UserId,
DraftInfo,
ReviewResult,
+ SubmitRequirementInfo,
} from '../../types/common';
import {
DiffInfo,
@@ -285,6 +286,11 @@
errFn?: ErrorCallback
): Promise<string[] | undefined>;
+ getRepoSubmitRequirements(
+ repoName: RepoName,
+ errFn?: ErrorCallback
+ ): Promise<SubmitRequirementInfo[] | undefined>;
+
getRepoAccessRights(
repoName: RepoName,
errFn?: ErrorCallback
diff --git a/polygerrit-ui/app/services/router/router-model.ts b/polygerrit-ui/app/services/router/router-model.ts
index 09ba661..5506fb1 100644
--- a/polygerrit-ui/app/services/router/router-model.ts
+++ b/polygerrit-ui/app/services/router/router-model.ts
@@ -7,6 +7,7 @@
import {Model} from '../../models/base/model';
import {select} from '../../utils/observable-util';
import {define} from '../../models/dependency';
+import {PluginLoader} from '../../elements/shared/gr-js-api-interface/gr-plugin-loader';
export enum GerritView {
ADMIN = 'admin',
@@ -39,7 +40,12 @@
state => state.view
);
- constructor() {
+ constructor(private readonly pluginLoader: PluginLoader) {
super({});
+ this.subscriptions = [
+ this.routerView$.subscribe(view => {
+ this.pluginLoader.jsApiService.handleViewChange(view);
+ }),
+ ];
}
}
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 30bac0d..95f160d 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -60,6 +60,7 @@
NumericChangeId,
PreferencesInput,
DraftInfo,
+ SubmitRequirementInfo,
} from '../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../types/diff';
import {
@@ -393,6 +394,9 @@
getRepoAccess(): Promise<RepoAccessInfoMap | undefined> {
return Promise.resolve({});
},
+ getRepoSubmitRequirements(): Promise<SubmitRequirementInfo[] | undefined> {
+ return Promise.resolve([]);
+ },
getRepoAccessRights(): Promise<ProjectAccessInfo | undefined> {
return Promise.resolve(undefined);
},
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index c88c07b..47669f4 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -727,6 +727,19 @@
}
/**
+ * The SubmitRequirementInfo entity describes a submit requirement.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#submit-requirement-info
+ */
+export interface SubmitRequirementInfo {
+ name: string;
+ description?: string;
+ applicability_expression?: string;
+ submittability_expression: string;
+ override_expression?: string;
+ allow_override_in_child_projects: boolean;
+}
+
+/**
* The ProjectAccessInfo entity contains information about the access rights for
* a project.
* https://gerrit-review.googlesource.com/Documentation/rest-api-access.html#project-access-info
diff --git a/polygerrit-ui/app/utils/commit-message-formatter-util.ts b/polygerrit-ui/app/utils/commit-message-formatter-util.ts
index e1b3e27..02d8d2f 100644
--- a/polygerrit-ui/app/utils/commit-message-formatter-util.ts
+++ b/polygerrit-ui/app/utils/commit-message-formatter-util.ts
@@ -319,7 +319,7 @@
return errors;
}
-function parseCommitMessageString(messageString: string): CommitMessage {
+export function parseCommitMessageString(messageString: string): CommitMessage {
const lines = messageString.split('\n');
// Remove leading blank lines
while (lines.length > 0 && lines[0].trim() === '') {
diff --git a/polygerrit-ui/app/utils/date-util.ts b/polygerrit-ui/app/utils/date-util.ts
index d95d24a..b0222d9 100644
--- a/polygerrit-ui/app/utils/date-util.ts
+++ b/polygerrit-ui/app/utils/date-util.ts
@@ -61,7 +61,7 @@
* Return true if date is within 24 hours and on the same day.
*/
export function isWithinDay(now: Date, date: Date) {
- const diff = now.valueOf() - date.valueOf();
+ const diff = Math.abs(now.valueOf() - date.valueOf());
return diff < Duration.DAY && date.getDay() === now.getDay();
}
@@ -79,7 +79,7 @@
* Returns true if date is from one to six months.
*/
export function isWithinHalfYear(now: Date, date: Date) {
- const diff = now.valueOf() - date.valueOf();
+ const diff = Math.abs(now.valueOf() - date.valueOf());
return diff < 180 * Duration.DAY;
}
diff --git a/resources/com/google/gerrit/server/change/ChangeMessages.properties b/resources/com/google/gerrit/server/change/ChangeMessages.properties
deleted file mode 100644
index c78f5a3..0000000
--- a/resources/com/google/gerrit/server/change/ChangeMessages.properties
+++ /dev/null
@@ -1,13 +0,0 @@
-revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}.
-revertSubmissionDefaultMessage = This reverts commit {0}.
-revertSubmissionUserMessage = Revert \"{0}\"\n\n{1}
-revertSubmissionOfRevertSubmissionUserMessage = Revert^{0} \"{1}\"\n\n{2}
-
-reviewerCantSeeChange = {0} does not have permission to see this change
-reviewerInvalid = {0} is not a valid user identifier
-reviewerNotFoundUserOrGroup = {0} does not identify a registered user or group
-
-groupRemovalIsNotAllowed = Groups can't be removed from reviewers, so can't remove {0}.
-groupIsNotAllowed = The group {0} cannot be added as reviewer.
-groupHasTooManyMembers = The group {0} has too many members to add them all as reviewers.
-groupManyMembersConfirmation = The group {0} has {1} members. Do you want to add them all as reviewers?
diff --git a/resources/com/google/gerrit/server/config/CapabilityConstants.properties b/resources/com/google/gerrit/server/config/CapabilityConstants.properties
deleted file mode 100644
index 95206f9..0000000
--- a/resources/com/google/gerrit/server/config/CapabilityConstants.properties
+++ /dev/null
@@ -1,25 +0,0 @@
-accessDatabase = Access Database
-administrateServer = Administrate Server
-batchChangesLimit = Batch Changes Limit
-createAccount = Create Account
-createGroup = Create Group
-deleteGroup = Delete Group
-createProject = Create Project
-emailReviewers = Email Reviewers
-flushCaches = Flush Caches
-killTask = Kill Task
-maintainServer = Maintain Server
-modifyAccount = Modify Account
-priority = Priority
-readAs = Read As
-queryLimit = Query Limit
-runAs = Run As
-runGC = Run Garbage Collection
-streamEvents = Stream Events
-viewAccess = View Access
-viewAllAccounts = View All Accounts
-viewCaches = View Caches
-viewConnections = View Connections
-viewPlugins = View Plugins
-viewQueue = View Queue
-viewSecondaryEmails = View Secondary Emails
