Merge "ChangeInfo: Add a field with the full branch name"
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 16dab80..e00749d 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -4150,68 +4150,6 @@
}
----
-[[migrate-labels]]
-=== Migrate label functions to submit requirements
---
-'POST /projects/link:#project-name[\{project-name\}]/migrate-labels'
---
-
-Migrates labels with functions to submit requirements. The migration result is
-committed into the `refs/meta/config` branch and thus immediately active. As a
-response it returns link:#migrate-labels-info[MigrateLabelsInfo] entity
-describing the outcome of the migration.
-
-The caller must be a project owner.
-
-.Request
-----
- POST /projects/testproj/migrate-labels HTTP/1.0
- Content-Type: application/json; charset=UTF-8
-----
-
-.Response
-----
- HTTP/1.1 200 OK
- Content-Type: application/json; charset=UTF-8
-
- )]}'
- {"status": "MIGRATED"}
-----
-
-
-[[migrate-labels-change]]
-=== Create change which migrate label functions to submit requirements
---
-'POST /projects/link:#project-name[\{project-name\}]/migrate-labels:review'
---
-
-Creates a change for review which migrates labels with functions to submit requirements.
-As a response it returns link:#migrate-labels-review-info[MigrageLabelsReviewInfo] entity
-describing the outcome of the migration.
-
-.Request
-----
- POST /projects/testproj/migrate-labels HTTP/1.0
- Content-Type: application/json; charset=UTF-8
-----
-
-.Response
-----
- HTTP/1.1 200 OK
- Content-Type: application/json; charset=UTF-8
-
- )]}'
- {
- "status": "MIGRATED",
- "change": {
- "id": "testproj~12345",
- ...
- }
- }
-----
-
-
-
[[ids]]
== IDs
@@ -5328,40 +5266,6 @@
a date in the future.
|=========================
-
-[[migrate-labels-info]]
-=== MigrateLabelsInfo
-The `MigrateLabelsInfo` entity contains information about an outcome of labels
-function migration.
-
-[options="header",cols="1,^2,4"]
-|=============================
-|Field Name ||Description
-|`status` ||The status of the migration. Takes one of the following values:
-`MIGRATED`,
-`HAS_PROLOG`,
-`PREVIOUSLY_MIGRATED`,
-`NO_CHANGE`
-|=============================
-
-[[migrate-labels-review-info]]
-=== MigrateLabelsReviewInfo
-The `MigrateLabelsReviewInfo` entity contains information about an outcome of creating
-a change for labels function migration.
-
-[options="header",cols="1,^2,4"]
-|=============================
-|Field Name ||Description
-|`status` ||The status of the migration. Takes one of the following values:
-`MIGRATED`,
-`HAS_PROLOG`,
-`PREVIOUSLY_MIGRATED`,
-`NO_CHANGE`
-|`change` |optional|The change created.
-It is a link:rest-api-changes.html#change-info[ChangeInfo] entity
-and is set only when the `status` value is `MIGRATED`.
-|=============================
-
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/contrib/maintenance/gerrit/site.py b/contrib/maintenance/gerrit/site.py
index 51d567f..faf6c02 100644
--- a/contrib/maintenance/gerrit/site.py
+++ b/contrib/maintenance/gerrit/site.py
@@ -33,7 +33,7 @@
def get_base_path(self):
if not self.base_path:
with GitConfigReader(
- os.path.join(self.get_etc_path(), "gerrit.config")
+ os.path.join(self.get_etc_path(), "gerrit.config"), []
) as cfg:
config_base_path = cfg.get("gerrit", None, "basePath", "git")
if os.path.isabs(config_base_path):
diff --git a/contrib/maintenance/git/config.py b/contrib/maintenance/git/config.py
index cdcb2b0..0a0f00b 100644
--- a/contrib/maintenance/git/config.py
+++ b/contrib/maintenance/git/config.py
@@ -27,14 +27,16 @@
class GitConfigReader:
- def __init__(self, config_path):
+ def __init__(self, config_path, cmd_options):
self.path = config_path
+ self.cmd_options = cmd_options
self.contents = {}
def __enter__(self):
LOG.debug("reader")
self.file = open(self.path, "r", encoding="utf-8")
self._parse()
+ self._parse_cmd_options()
return self
def __exit__(self, exc_type, exc_val, exc_tb):
@@ -109,6 +111,23 @@
]
LOG.debug("Parsed config: %s", self.contents)
+ def _parse_cmd_options(self):
+ for option in self.cmd_options:
+ key, value = option.split("=", 1)
+ key_parts = key.split(".")
+ if len(key_parts) == 2:
+ section = key_parts[0].lower()
+ subsection = DEFAULT_SUBSECTION
+ key = key_parts[1].lower()
+ elif len(key_parts) == 3:
+ section = key_parts[0].lower()
+ subsection = key_parts[1].lower()
+ key = key_parts[2].lower()
+ else:
+ raise GitConfigException(f"Invalid git config option: {option}")
+ self._ensure_full_section(section, subsection)
+ self.contents[section][subsection][key] = value
+
def _ensure_full_section(self, section, subsection):
if section not in self.contents:
self.contents[section] = {}
@@ -118,7 +137,7 @@
class GitConfigWriter(GitConfigReader):
def __init__(self, config_path):
- super().__init__(config_path)
+ super().__init__(config_path, [])
def __enter__(self):
self.file = open(self.path, "r+", encoding="utf-8")
diff --git a/contrib/maintenance/git/gc.py b/contrib/maintenance/git/gc.py
index fd671e2..546726c 100644
--- a/contrib/maintenance/git/gc.py
+++ b/contrib/maintenance/git/gc.py
@@ -41,6 +41,9 @@
class GCStep(abc.ABC):
+ def __init__(self, git_config: GitConfigReader):
+ self.git_config = git_config
+
@abc.abstractmethod
def run(self, repo_dir):
pass
@@ -62,7 +65,9 @@
class PreservePacksInitStep(GCStep):
def run(self, repo_dir):
- with GitConfigReader(os.path.join(repo_dir, "config")) as config_reader:
+ with GitConfigReader(
+ os.path.join(repo_dir, "config"), self.git_config
+ ) as config_reader:
is_prune_preserved = config_reader.get("gc", None, "prunepreserved", False)
is_preserve_old_packs = config_reader.get(
"gc", None, "preserveoldpacks", False
@@ -119,9 +124,6 @@
return f"{filename}.old-{ext[1:]}"
-DEFAULT_INIT_STEPS = [GCLockHandlingInitStep(), PreservePacksInitStep()]
-
-
class DeleteEmptyRefDirsCleanupStep(GCStep):
def run(self, repo_dir):
refs_path = os.path.join(repo_dir, "refs")
@@ -180,20 +182,20 @@
)
-DEFAULT_AFTER_STEPS = [
- DeleteEmptyRefDirsCleanupStep(),
- DeleteStaleIncomingPacksCleanupStep(),
-]
-
-
class GitGarbageCollectionProvider:
@staticmethod
def get(pack_refs=True, git_config=None):
- init_steps = DEFAULT_INIT_STEPS.copy()
- after_steps = DEFAULT_AFTER_STEPS.copy()
+ init_steps = [
+ GCLockHandlingInitStep(git_config),
+ PreservePacksInitStep(git_config),
+ ]
+ after_steps = [
+ DeleteEmptyRefDirsCleanupStep(git_config),
+ DeleteStaleIncomingPacksCleanupStep(git_config),
+ ]
if pack_refs:
- after_steps.append(PackAllRefsAfterStep())
+ after_steps.append(PackAllRefsAfterStep(git_config))
return GitGarbageCollection(init_steps, after_steps, git_config)
diff --git a/contrib/maintenance/tests/git/test_config.py b/contrib/maintenance/tests/git/test_config.py
index f8039d6..9f930a2 100644
--- a/contrib/maintenance/tests/git/test_config.py
+++ b/contrib/maintenance/tests/git/test_config.py
@@ -50,12 +50,12 @@
def test_list_config(repo_with_config):
- with GitConfigReader(os.path.join(repo_with_config, "config")) as reader:
+ with GitConfigReader(os.path.join(repo_with_config, "config"), []) as reader:
assert reader.list() == CONFIG_DICT
def test_get_config(repo_with_config):
- with GitConfigReader(os.path.join(repo_with_config, "config")) as reader:
+ with GitConfigReader(os.path.join(repo_with_config, "config"), []) as reader:
assert (
reader.get("section", None, "key")
== CONFIG_DICT["section"]["default"]["key"]
@@ -76,13 +76,24 @@
assert not reader.get("another_section", "default", "another_boolean")
+def test_get_config_with_override(repo_with_config):
+ with GitConfigReader(
+ os.path.join(repo_with_config, "config"), ["section.key=override"]
+ ) as reader:
+ assert reader.get("section", None, "key") == "override"
+ assert (
+ reader.get("section", "subsection", "another_key")
+ == CONFIG_DICT["section"]["subsection"]["another_key"]
+ )
+
+
def test_set_config(repo_with_config):
with GitConfigWriter(os.path.join(repo_with_config, "config")) as writer:
writer.set("new", None, "key", "value")
writer.set("new", "new_sub", "key", "val")
writer.write()
- with GitConfigReader(os.path.join(repo_with_config, "config")) as reader:
+ with GitConfigReader(os.path.join(repo_with_config, "config"), []) as reader:
assert reader.get("new", None, "key") == "value"
assert reader.get("new", "new_sub", "key") == "val"
@@ -94,7 +105,7 @@
writer.add("section", "subsection", "other_key", "val")
writer.write()
- with GitConfigReader(os.path.join(repo_with_config, "config")) as reader:
+ with GitConfigReader(os.path.join(repo_with_config, "config"), []) as reader:
assert reader.get("new", None, "key") == "value"
assert reader.get("section", None, "key") == "value2"
assert reader.get("section", "subsection", "other_key") == "val"
@@ -106,7 +117,7 @@
writer.unset("section", "subsection", "another_key")
writer.write()
- with GitConfigReader(os.path.join(repo_with_config, "config")) as reader:
+ with GitConfigReader(os.path.join(repo_with_config, "config"), []) as reader:
config = reader.list()
assert DEFAULT_SUBSECTION not in config["section"]
assert "another_key" not in config["section"]["subsection"]
@@ -117,6 +128,6 @@
writer.remove("section", "subsection", "other_key", "test")
writer.write()
- with GitConfigReader(os.path.join(repo_with_config, "config")) as reader:
+ with GitConfigReader(os.path.join(repo_with_config, "config"), []) as reader:
config = reader.list()
assert "test" not in config["section"]["subsection"]["other_key"]
diff --git a/contrib/maintenance/tests/git/test_gc.py b/contrib/maintenance/tests/git/test_gc.py
index 83c4fda..c8f231c 100644
--- a/contrib/maintenance/tests/git/test_gc.py
+++ b/contrib/maintenance/tests/git/test_gc.py
@@ -42,7 +42,7 @@
with open(lock_file, "w") as f:
f.write("1234")
- task = GCLockHandlingInitStep()
+ task = GCLockHandlingInitStep([])
task.run(repo)
assert os.path.exists(lock_file)
@@ -54,7 +54,7 @@
def test_PreservePacksInitStep(repo):
- task = PreservePacksInitStep()
+ task = PreservePacksInitStep([])
pack_path = os.path.join(repo, "objects", "pack")
preserved_pack_path = os.path.join(pack_path, "preserved")
@@ -101,6 +101,26 @@
assert not os.path.exists(fake_preserved_idx)
+def test_PreservePacksInitStepWithOverride(repo):
+ task = PreservePacksInitStep(["gc.preserveOldPacks=true"])
+
+ pack_path = os.path.join(repo, "objects", "pack")
+ preserved_pack_path = os.path.join(pack_path, "preserved")
+
+ fake_pack = os.path.join(pack_path, "pack-fake.pack")
+ fake_preserved_pack = os.path.join(preserved_pack_path, "pack-fake.old-pack")
+ fake_idx = os.path.join(pack_path, "pack-fake.idx")
+ fake_preserved_idx = os.path.join(preserved_pack_path, "pack-fake.old-idx")
+
+ Path(fake_pack).touch()
+ Path(fake_idx).touch()
+
+ task.run(repo)
+
+ assert os.path.exists(fake_preserved_pack)
+ assert os.path.exists(fake_preserved_idx)
+
+
def test_DeleteEmptyRefDirsCleanupStep(repo):
delete_path = os.path.join(repo, "refs", "heads", "delete")
os.makedirs(delete_path)
@@ -110,7 +130,7 @@
os.makedirs(keep_path)
Path(os.path.join(keep_path, "abcd1234")).touch()
- task = DeleteEmptyRefDirsCleanupStep()
+ task = DeleteEmptyRefDirsCleanupStep([])
task.run(repo)
assert os.path.exists(delete_path)
@@ -138,7 +158,7 @@
_modify_last_modified(tags_path, DOUBLE_MAX_AGE_EMPTY_REF_DIRS)
_modify_last_modified(delete_change_path, DOUBLE_MAX_AGE_EMPTY_REF_DIRS)
- task = DeleteEmptyRefDirsCleanupStep()
+ task = DeleteEmptyRefDirsCleanupStep([])
task.run(repo)
assert not os.path.exists(delete_change_path)
@@ -148,7 +168,7 @@
def test_DeleteStaleIncomingPacksCleanupStep(repo):
- task = DeleteStaleIncomingPacksCleanupStep()
+ task = DeleteStaleIncomingPacksCleanupStep([])
objects_path = os.path.join(repo, "objects")
pack_path = os.path.join(objects_path, "pack")
@@ -190,7 +210,7 @@
loose_ref_count += 1
git.repo.push(local_repo, "origin", f"HEAD:refs/heads/test{loose_ref_count}")
- task = PackAllRefsAfterStep()
+ task = PackAllRefsAfterStep([])
task.run(repo)
assert len(os.listdir(os.path.join(repo, "refs", "heads"))) == 0
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index d6df3d8..a6e659b 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -139,7 +139,6 @@
import com.google.gerrit.server.plugins.TestServerPlugin;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.restapi.change.Revisions;
@@ -269,7 +268,6 @@
@Inject protected FakeEmailSender sender;
@Inject protected GerritApi gApi;
@Inject protected GitRepositoryManager repoManager;
- @Inject protected RepoMetaDataUpdater repoMetaDataUpdater;
@Inject protected GroupBackend groupBackend;
@Inject protected GroupCache groupCache;
@Inject protected IdentifiedUser.GenericFactory identifiedUserFactory;
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index fb2db3a..d23c8b1 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -1026,11 +1026,14 @@
for (Account.Id id : removable) {
result.add(accountLoader.get(id));
}
- // Reviewers added by email are always removable
+ // Reviewers added by email
for (Collection<AccountInfo> infos : out.reviewers.values()) {
for (AccountInfo info : infos) {
if (info._accountId == null) {
- result.add(info);
+ if (canRemoveAnyReviewer
+ || removeReviewerControl.testRemoveReviewer(cd, userProvider.get(), null, 0)) {
+ result.add(info);
+ }
}
}
}
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java b/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
index 0fe526c..b1395b5 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
@@ -20,6 +20,7 @@
import com.google.gerrit.entities.Address;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.mail.EmailFactories;
import com.google.gerrit.server.mail.send.ChangeEmail;
@@ -27,6 +28,8 @@
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.OutgoingEmail;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.RemoveReviewerControl;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.PostUpdateContext;
import com.google.inject.Inject;
@@ -43,6 +46,7 @@
private final EmailFactories emailFactories;
private final MessageIdGenerator messageIdGenerator;
private final ChangeMessagesUtil changeMessagesUtil;
+ private final RemoveReviewerControl removeReviewerControl;
private final Address reviewer;
private String mailMessage;
@@ -53,15 +57,18 @@
EmailFactories emailFactories,
MessageIdGenerator messageIdGenerator,
ChangeMessagesUtil changeMessagesUtil,
+ RemoveReviewerControl removeReviewerControl,
@Assisted Address reviewer) {
this.emailFactories = emailFactories;
this.messageIdGenerator = messageIdGenerator;
this.changeMessagesUtil = changeMessagesUtil;
+ this.removeReviewerControl = removeReviewerControl;
this.reviewer = reviewer;
}
@Override
- public boolean updateChange(ChangeContext ctx) {
+ public boolean updateChange(ChangeContext ctx) throws PermissionBackendException, AuthException {
+ removeReviewerControl.checkRemoveReviewer(ctx.getNotes(), ctx.getUser(), null);
change = ctx.getChange();
PatchSet.Id psId = ctx.getChange().currentPatchSetId();
ChangeUpdate update = ctx.getUpdate(psId);
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 343d1a9..4087f84 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -161,7 +161,7 @@
boolean votesRemoved = false;
for (PatchSetApproval a : approvals(ctx, reviewerId)) {
// Check if removing this vote is OK
- removeReviewerControl.checkRemoveReviewer(ctx.getNotes(), ctx.getUser(), a);
+ removeReviewerControl.checkRemoveReviewerApproval(ctx.getNotes(), ctx.getUser(), a);
if (a.patchSetId().equals(patchSet.id()) && a.value() != 0) {
oldApprovals.put(a.label(), a.value());
removedVotesMsg
diff --git a/java/com/google/gerrit/server/project/RemoveReviewerControl.java b/java/com/google/gerrit/server/project/RemoveReviewerControl.java
index c87caac..ee8cf05 100644
--- a/java/com/google/gerrit/server/project/RemoveReviewerControl.java
+++ b/java/com/google/gerrit/server/project/RemoveReviewerControl.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.project;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSetApproval;
@@ -52,7 +53,7 @@
* @throws ResourceConflictException if the approval cannot be removed because the change is
* merged
*/
- public void checkRemoveReviewer(
+ public void checkRemoveReviewerApproval(
ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval)
throws PermissionBackendException, AuthException, ResourceConflictException {
if (notes.getChange().isMerged() && approval.value() != 0) {
@@ -66,10 +67,15 @@
* Checks if removing the given reviewer is OK. Does not check if removing any approvals the
* reviewer might have given is OK.
*
+ * <p>The `reviewer` parameter is only used for logging purposes and for checking whether the
+ * current user is equal to the reviewer. So it can be set to null. Then the permission check is
+ * generic.
+ *
* @throws AuthException if this user is not allowed to remove this approval.
* @throws PermissionBackendException on failure of permission checks.
*/
- public void checkRemoveReviewer(ChangeNotes notes, CurrentUser currentUser, Account.Id reviewer)
+ public void checkRemoveReviewer(
+ ChangeNotes notes, CurrentUser currentUser, @Nullable Account.Id reviewer)
throws PermissionBackendException, AuthException {
checkRemoveReviewer(notes, currentUser, reviewer, 0);
}
@@ -83,14 +89,14 @@
/** Returns true if the user is allowed to remove this reviewer. */
public boolean testRemoveReviewer(
- ChangeNotes notes, CurrentUser currentUser, Account.Id reviewer, int value)
+ ChangeNotes notes, CurrentUser currentUser, @Nullable Account.Id reviewer, int value)
throws PermissionBackendException {
return testRemoveReviewer(changeDataFactory.create(notes), currentUser, reviewer, value);
}
/** Returns true if the user is allowed to remove this reviewer. */
public boolean testRemoveReviewer(
- ChangeData cd, CurrentUser currentUser, Account.Id reviewer, int value)
+ ChangeData cd, CurrentUser currentUser, @Nullable Account.Id reviewer, int value)
throws PermissionBackendException {
if (cd.change().isMerged() && value != 0) {
return false;
@@ -110,11 +116,12 @@
}
private void checkRemoveReviewer(
- ChangeNotes notes, CurrentUser currentUser, Account.Id reviewer, int value)
+ ChangeNotes notes, CurrentUser currentUser, @Nullable Account.Id reviewer, int value)
throws PermissionBackendException, AuthException {
if (canRemoveReviewerWithoutPermissionCheck(notes.getChange(), currentUser, reviewer, value)) {
return;
}
+ String reviewerStr = reviewer != null ? reviewer.toString() : "by email";
// Users with the remove reviewer permission, the branch owner, project
// owner and site admin can remove anyone
@@ -125,7 +132,7 @@
"%s can remove reviewer %s from change %s since they are an owner of the destination"
+ " branch %s",
currentUser.getLoggableName(),
- reviewer,
+ reviewerStr,
notes.getChangeId(),
notes.getChange().getDest().branch());
return;
@@ -141,14 +148,14 @@
logger.atFine().log(
"%s can remove reviewer %s from change %s since they have the %s permission",
currentUser.getLoggableName(),
- reviewer,
+ reviewerStr,
notes.getChangeId(),
ChangePermission.REMOVE_REVIEWER.name());
} catch (AuthException e) {
logger.atFine().log(
"%s cannot remove reviewer %s from change %s since they don't have the %s permission",
currentUser.getLoggableName(),
- reviewer,
+ reviewerStr,
notes.getChangeId(),
ChangePermission.REMOVE_REVIEWER.name());
throw e;
@@ -156,28 +163,29 @@
}
public static boolean canRemoveReviewerWithoutPermissionCheck(
- Change change, CurrentUser currentUser, Account.Id reviewer, int value) {
+ Change change, CurrentUser currentUser, @Nullable Account.Id reviewer, int value) {
+ String reviewerStr = reviewer != null ? reviewer.toString() : "by email";
if (currentUser.isIdentifiedUser()) {
Account.Id aId = currentUser.getAccountId();
if (aId.equals(reviewer)) {
// A user can always remove themselves.
logger.atFine().log(
"%s can remove reviewer %s from change %s since they can always remove themselves",
- currentUser.getLoggableName(), reviewer, change.getId());
+ currentUser.getLoggableName(), reviewerStr, change.getId());
return true;
} else if (aId.equals(change.getOwner()) && 0 <= value) {
// The change owner may remove any zero or positive score.
logger.atFine().log(
"%s can remove reviewer %s from change %s since they own the change and the reviewer"
+ " scored with zero or a positive score",
- currentUser.getLoggableName(), reviewer, change.getId());
+ currentUser.getLoggableName(), reviewerStr, change.getId());
return true;
}
}
logger.atFine().log(
"%s cannot remove reviewer %s from change %s without permission check",
- currentUser.getLoggableName(), reviewer, change.getId());
+ currentUser.getLoggableName(), reviewerStr, change.getId());
return false;
}
}
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index 47c6abe..70ba6c5 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -27,7 +27,6 @@
"//java/com/google/gerrit/server/flow",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
- "//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/server/util/time",
"//lib:args4j",
"//lib:blame-cache",
diff --git a/java/com/google/gerrit/server/restapi/project/AbstractPostCollection.java b/java/com/google/gerrit/server/restapi/project/AbstractPostCollection.java
index 1680b21..6a99bed 100644
--- a/java/com/google/gerrit/server/restapi/project/AbstractPostCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/AbstractPostCollection.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.inject.Provider;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
diff --git a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
index 96090ed..5fc060a 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
@@ -28,8 +28,7 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigChangeCreator;
+import com.google.gerrit.server.restapi.project.RepoMetaDataUpdater.ConfigChangeCreator;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
diff --git a/java/com/google/gerrit/server/restapi/project/CreateLabel.java b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
index 3809929..35102d8 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
@@ -36,7 +36,6 @@
import com.google.gerrit.server.project.LabelResource;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
import com.google.inject.Inject;
import com.google.inject.Singleton;
diff --git a/java/com/google/gerrit/server/restapi/project/CreateSubmitRequirement.java b/java/com/google/gerrit/server/restapi/project/CreateSubmitRequirement.java
index 057f493..86d42cb 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateSubmitRequirement.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateSubmitRequirement.java
@@ -30,7 +30,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.gerrit.server.project.SubmitRequirementExpressionsValidator;
import com.google.gerrit.server.project.SubmitRequirementJson;
import com.google.gerrit.server.project.SubmitRequirementResource;
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteLabel.java b/java/com/google/gerrit/server/restapi/project/DeleteLabel.java
index 2482a28..b38aa29 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteLabel.java
@@ -25,7 +25,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.LabelResource;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteSubmitRequirement.java b/java/com/google/gerrit/server/restapi/project/DeleteSubmitRequirement.java
index ebc5461..64e2399 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteSubmitRequirement.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteSubmitRequirement.java
@@ -20,7 +20,6 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.gerrit.server.project.SubmitRequirementResource;
import com.google.inject.Inject;
import com.google.inject.Singleton;
diff --git a/java/com/google/gerrit/server/restapi/project/MigrateLabels.java b/java/com/google/gerrit/server/restapi/project/MigrateLabels.java
deleted file mode 100644
index 5fce7f9..0000000
--- a/java/com/google/gerrit/server/restapi/project/MigrateLabels.java
+++ /dev/null
@@ -1,83 +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.restapi.project;
-
-import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.ProjectPermission;
-import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.schema.MigrateLabelFunctionsToSubmitRequirement;
-import com.google.gerrit.server.schema.UpdateUI;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.util.Set;
-
-@Singleton
-public class MigrateLabels implements RestModifyView<ProjectResource, MigrateLabelsInput> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- private final MigrateLabelFunctionsToSubmitRequirement migrateLabelFunctionsToSubmitRequirement;
- private final PermissionBackend permissionBackend;
-
- @Inject
- MigrateLabels(
- MigrateLabelFunctionsToSubmitRequirement migrateLabelFunctionsToSubmitRequirement,
- PermissionBackend permissionBackend) {
- this.migrateLabelFunctionsToSubmitRequirement = migrateLabelFunctionsToSubmitRequirement;
- this.permissionBackend = permissionBackend;
- }
-
- @Override
- public Response<MigrateLabelsInfo> apply(ProjectResource rsrc, MigrateLabelsInput input)
- throws Exception {
- Project.NameKey project = rsrc.getNameKey();
- permissionBackend.currentUser().project(project).check(ProjectPermission.WRITE_CONFIG);
- MigrateLabelFunctionsToSubmitRequirement.Status status =
- migrateLabelFunctionsToSubmitRequirement.executeMigration(project, new LoggingUpdateUI());
-
- MigrateLabelsInfo info = new MigrateLabelsInfo();
- info.status = status;
- return Response.ok(info);
- }
-
- public static class LoggingUpdateUI implements UpdateUI {
-
- @Override
- public void message(String message) {
- logger.atInfo().log("%s", message);
- }
-
- @Override
- public boolean yesno(boolean defaultValue, String message) {
- return false;
- }
-
- @Override
- public void waitForUser() {}
-
- @Override
- public String readString(String defaultValue, Set<String> allowedValues, String message) {
- return null;
- }
-
- @Override
- public boolean isBatch() {
- return false;
- }
- }
-}
diff --git a/java/com/google/gerrit/server/restapi/project/MigrateLabelsInfo.java b/java/com/google/gerrit/server/restapi/project/MigrateLabelsInfo.java
deleted file mode 100644
index b6a2920..0000000
--- a/java/com/google/gerrit/server/restapi/project/MigrateLabelsInfo.java
+++ /dev/null
@@ -1,21 +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.restapi.project;
-
-import com.google.gerrit.server.schema.MigrateLabelFunctionsToSubmitRequirement;
-
-public class MigrateLabelsInfo {
- public MigrateLabelFunctionsToSubmitRequirement.Status status;
-}
diff --git a/java/com/google/gerrit/server/restapi/project/MigrateLabelsInput.java b/java/com/google/gerrit/server/restapi/project/MigrateLabelsInput.java
deleted file mode 100644
index d010a9d..0000000
--- a/java/com/google/gerrit/server/restapi/project/MigrateLabelsInput.java
+++ /dev/null
@@ -1,17 +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.restapi.project;
-
-public class MigrateLabelsInput {}
diff --git a/java/com/google/gerrit/server/restapi/project/MigrateLabelsReview.java b/java/com/google/gerrit/server/restapi/project/MigrateLabelsReview.java
deleted file mode 100644
index 0b31d63..0000000
--- a/java/com/google/gerrit/server/restapi/project/MigrateLabelsReview.java
+++ /dev/null
@@ -1,62 +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.restapi.project;
-
-import static com.google.gerrit.server.schema.MigrateLabelFunctionsToSubmitRequirement.Status.MIGRATED;
-
-import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigChangeCreator;
-import com.google.gerrit.server.schema.MigrateLabelFunctionsToSubmitRequirement;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-@Singleton
-public class MigrateLabelsReview implements RestModifyView<ProjectResource, MigrateLabelsInput> {
-
- private final RepoMetaDataUpdater repoMetaDataUpdater;
- private final MigrateLabelFunctionsToSubmitRequirement migrateLabelFunctionsToSubmitRequirement;
-
- @Inject
- MigrateLabelsReview(
- RepoMetaDataUpdater repoMetaDataUpdater,
- MigrateLabelFunctionsToSubmitRequirement migrateLabelFunctionsToSubmitRequirement) {
- this.repoMetaDataUpdater = repoMetaDataUpdater;
- this.migrateLabelFunctionsToSubmitRequirement = migrateLabelFunctionsToSubmitRequirement;
- }
-
- @Override
- public Response<MigrateLabelsReviewInfo> apply(ProjectResource rsrc, MigrateLabelsInput input)
- throws AuthException, BadRequestException, ResourceConflictException, Exception {
- try (ConfigChangeCreator creator =
- repoMetaDataUpdater.configChangeCreator(
- rsrc.getNameKey(), null, MigrateLabelFunctionsToSubmitRequirement.COMMIT_MSG)) {
- MigrateLabelFunctionsToSubmitRequirement.Status status =
- migrateLabelFunctionsToSubmitRequirement.updateConfig(
- rsrc.getProjectState().getNameKey(),
- creator.getConfig(),
- new MigrateLabels.LoggingUpdateUI());
- if (status == MIGRATED) {
- return Response.ok(new MigrateLabelsReviewInfo(MIGRATED, creator.createChange().value()));
- }
- return Response.ok(new MigrateLabelsReviewInfo(status));
- }
- }
-}
diff --git a/java/com/google/gerrit/server/restapi/project/MigrateLabelsReviewInfo.java b/java/com/google/gerrit/server/restapi/project/MigrateLabelsReviewInfo.java
deleted file mode 100644
index 01d9640..0000000
--- a/java/com/google/gerrit/server/restapi/project/MigrateLabelsReviewInfo.java
+++ /dev/null
@@ -1,33 +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.restapi.project;
-
-import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.server.schema.MigrateLabelFunctionsToSubmitRequirement;
-
-public class MigrateLabelsReviewInfo {
- public MigrateLabelFunctionsToSubmitRequirement.Status status;
- public ChangeInfo change;
-
- public MigrateLabelsReviewInfo(
- MigrateLabelFunctionsToSubmitRequirement.Status status, ChangeInfo change) {
- this.status = status;
- this.change = change;
- }
-
- public MigrateLabelsReviewInfo(MigrateLabelFunctionsToSubmitRequirement.Status status) {
- this(status, null);
- }
-}
diff --git a/java/com/google/gerrit/server/restapi/project/PostLabels.java b/java/com/google/gerrit/server/restapi/project/PostLabels.java
index 1900e28..2182784 100644
--- a/java/com/google/gerrit/server/restapi/project/PostLabels.java
+++ b/java/com/google/gerrit/server/restapi/project/PostLabels.java
@@ -24,7 +24,6 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.project.LabelResource;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
diff --git a/java/com/google/gerrit/server/restapi/project/PostLabelsReview.java b/java/com/google/gerrit/server/restapi/project/PostLabelsReview.java
index d0562c6..4e9d432 100644
--- a/java/com/google/gerrit/server/restapi/project/PostLabelsReview.java
+++ b/java/com/google/gerrit/server/restapi/project/PostLabelsReview.java
@@ -24,8 +24,7 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigChangeCreator;
+import com.google.gerrit.server.restapi.project.RepoMetaDataUpdater.ConfigChangeCreator;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import java.io.IOException;
diff --git a/java/com/google/gerrit/server/restapi/project/PostSubmitRequirements.java b/java/com/google/gerrit/server/restapi/project/PostSubmitRequirements.java
index 3848d75..71080a5 100644
--- a/java/com/google/gerrit/server/restapi/project/PostSubmitRequirements.java
+++ b/java/com/google/gerrit/server/restapi/project/PostSubmitRequirements.java
@@ -22,7 +22,6 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.gerrit.server.project.SubmitRequirementResource;
import com.google.inject.Provider;
import javax.inject.Inject;
diff --git a/java/com/google/gerrit/server/restapi/project/PostSubmitRequirementsReview.java b/java/com/google/gerrit/server/restapi/project/PostSubmitRequirementsReview.java
index 4e686fe..f0a371a 100644
--- a/java/com/google/gerrit/server/restapi/project/PostSubmitRequirementsReview.java
+++ b/java/com/google/gerrit/server/restapi/project/PostSubmitRequirementsReview.java
@@ -22,8 +22,7 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigChangeCreator;
+import com.google.gerrit.server.restapi.project.RepoMetaDataUpdater.ConfigChangeCreator;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import java.io.IOException;
diff --git a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
index adba60e..f5647ec 100644
--- a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
@@ -87,9 +87,6 @@
put(PROJECT_KIND, "config").to(PutConfig.class);
put(PROJECT_KIND, "config:review").to(PutConfigReview.class);
- post(PROJECT_KIND, "migrate-labels").to(MigrateLabels.class);
- post(PROJECT_KIND, "migrate-labels:review").to(MigrateLabelsReview.class);
-
post(PROJECT_KIND, "create.change").to(CreateChange.class);
child(PROJECT_KIND, "dashboards").to(DashboardsCollection.class);
diff --git a/java/com/google/gerrit/server/restapi/project/PutConfig.java b/java/com/google/gerrit/server/restapi/project/PutConfig.java
index 39ded77..afd07bb 100644
--- a/java/com/google/gerrit/server/restapi/project/PutConfig.java
+++ b/java/com/google/gerrit/server/restapi/project/PutConfig.java
@@ -55,8 +55,7 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigUpdater;
+import com.google.gerrit.server.restapi.project.RepoMetaDataUpdater.ConfigUpdater;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
diff --git a/java/com/google/gerrit/server/restapi/project/PutConfigReview.java b/java/com/google/gerrit/server/restapi/project/PutConfigReview.java
index 3e34227..7e6cc19 100644
--- a/java/com/google/gerrit/server/restapi/project/PutConfigReview.java
+++ b/java/com/google/gerrit/server/restapi/project/PutConfigReview.java
@@ -21,8 +21,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigChangeCreator;
+import com.google.gerrit.server.restapi.project.RepoMetaDataUpdater.ConfigChangeCreator;
import com.google.gerrit.server.update.UpdateException;
import java.io.IOException;
import javax.inject.Inject;
diff --git a/java/com/google/gerrit/server/restapi/project/PutDescription.java b/java/com/google/gerrit/server/restapi/project/PutDescription.java
index 0841d6b..ea685c8 100644
--- a/java/com/google/gerrit/server/restapi/project/PutDescription.java
+++ b/java/com/google/gerrit/server/restapi/project/PutDescription.java
@@ -26,7 +26,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
diff --git a/java/com/google/gerrit/server/project/RepoMetaDataUpdater.java b/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
similarity index 98%
rename from java/com/google/gerrit/server/project/RepoMetaDataUpdater.java
rename to java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
index e617f34..a34b7d8 100644
--- a/java/com/google/gerrit/server/project/RepoMetaDataUpdater.java
+++ b/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.project;
+package com.google.gerrit.server.restapi.project;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
@@ -42,6 +42,8 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.update.context.RefUpdateContext;
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccess.java b/java/com/google/gerrit/server/restapi/project/SetAccess.java
index 286cf90..65851c0 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccess.java
@@ -36,8 +36,7 @@
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigUpdater;
+import com.google.gerrit.server.restapi.project.RepoMetaDataUpdater.ConfigUpdater;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
diff --git a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
index d5b4a94..a46ee32 100644
--- a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
+++ b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.project.DashboardResource;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
diff --git a/java/com/google/gerrit/server/restapi/project/SetLabel.java b/java/com/google/gerrit/server/restapi/project/SetLabel.java
index b894b44..33d4640 100644
--- a/java/com/google/gerrit/server/restapi/project/SetLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/SetLabel.java
@@ -29,7 +29,6 @@
import com.google.gerrit.server.project.LabelDefinitionJson;
import com.google.gerrit.server.project.LabelResource;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
import com.google.inject.Inject;
import com.google.inject.Singleton;
diff --git a/java/com/google/gerrit/server/restapi/project/SetParent.java b/java/com/google/gerrit/server/restapi/project/SetParent.java
index 074e5f8..b94e911 100644
--- a/java/com/google/gerrit/server/restapi/project/SetParent.java
+++ b/java/com/google/gerrit/server/restapi/project/SetParent.java
@@ -47,7 +47,6 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
diff --git a/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java b/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
index 3d31e2b3..d3a251e 100644
--- a/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
+++ b/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
@@ -27,7 +27,6 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
import com.google.gerrit.server.project.SubmitRequirementExpressionsValidator;
import com.google.gerrit.server.project.SubmitRequirementJson;
import com.google.gerrit.server.project.SubmitRequirementResource;
diff --git a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
index bdc521e..fcd50f6 100644
--- a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
+++ b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
@@ -16,30 +16,35 @@
import static com.google.gerrit.server.project.ProjectConfig.RULES_PL_FILE;
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
-import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.LabelFunction;
import com.google.gerrit.entities.LabelType;
-import com.google.gerrit.entities.LabelValue;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.git.meta.VersionedConfigFile;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.RepoMetaDataUpdater;
-import com.google.gerrit.server.project.RepoMetaDataUpdater.ConfigUpdater;
import com.google.inject.Inject;
import java.io.IOException;
+import java.util.Arrays;
import java.util.Collection;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -78,9 +83,8 @@
*/
public class MigrateLabelFunctionsToSubmitRequirement {
public static final String COMMIT_MSG = "Migrate label functions to submit requirements";
-
- private final RepoMetaDataUpdater repoMetaDataUpdater;
private final GitRepositoryManager repoManager;
+ private final PersonIdent serverUser;
public enum Status {
/**
@@ -107,9 +111,9 @@
@Inject
public MigrateLabelFunctionsToSubmitRequirement(
- RepoMetaDataUpdater repoMetaDataUpdater, GitRepositoryManager repoManager) {
- this.repoMetaDataUpdater = repoMetaDataUpdater;
+ GitRepositoryManager repoManager, @GerritPersonIdent PersonIdent serverUser) {
this.repoManager = repoManager;
+ this.serverUser = serverUser;
}
/**
@@ -121,99 +125,176 @@
* @return {@link Status} reflecting the status of the migration.
*/
public Status executeMigration(Project.NameKey project, UpdateUI ui)
- throws IOException,
- ConfigInvalidException,
- MethodNotAllowedException,
- PermissionBackendException {
- try (ConfigUpdater updater =
- repoMetaDataUpdater.configUpdaterWithoutPermissionsCheck(project, null, COMMIT_MSG)) {
- Status result = updateConfig(project, updater.getConfig(), ui);
- if (result == Status.MIGRATED) {
- updater.commitConfigUpdate();
- }
- return result;
- }
- }
-
- public Status updateConfig(Project.NameKey project, ProjectConfig projectConfig, UpdateUI ui)
- throws IOException {
+ throws IOException, ConfigInvalidException {
boolean updated = false;
if (hasPrologRules(project)) {
ui.message(String.format("Skipping project %s because it has prolog rules", project));
return Status.HAS_PROLOG;
}
-
- if (hasMigrationAlreadyRun(project)) {
- ui.message(
- String.format(
- "Skipping migrating label functions to submit requirements for project '%s'"
- + " because it has been previously migrated",
- project));
- return Status.PREVIOUSLY_MIGRATED;
- }
-
- Map<String, LabelType> labelSections = projectConfig.getLabelSections();
- SubmitRequirementMap existingSubmitRequirements =
- new SubmitRequirementMap(projectConfig.getSubmitRequirementSections());
-
- for (Map.Entry<String, LabelType> section : labelSections.entrySet()) {
- String labelName = section.getKey();
- LabelType labelType = section.getValue();
-
- if (labelType.getFunction() == LabelFunction.PATCH_SET_LOCK) {
- // PATCH_SET_LOCK functions should be left as is
- continue;
+ VersionedConfigFile projectConfig = new VersionedConfigFile(ProjectConfig.PROJECT_CONFIG);
+ try (Repository repo = repoManager.openRepository(project);
+ MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, project, repo)) {
+ if (hasMigrationAlreadyRun(repo)) {
+ ui.message(
+ String.format(
+ "Skipping migrating label functions to submit requirements for project '%s'"
+ + " because it has been previously migrated",
+ project));
+ return Status.PREVIOUSLY_MIGRATED;
}
-
- // If the function is other than "NoBlock" we want to reset the label function regardless
- // of whether there exists a "submit requirement".
- if (labelType.getFunction() != LabelFunction.NO_BLOCK) {
- section.setValue(labelType.toBuilder().setNoBlockFunction().build());
- updated = true;
- }
-
- Optional<SubmitRequirement> sr = createSrFromLabelDef(labelType);
- if (!sr.isPresent()) {
- continue;
- }
- // Make the operation idempotent by skipping creating the submit-requirement if one was
- // already created or previously existed.
- if (existingSubmitRequirements.containsKey(labelName)) {
- SubmitRequirement existing = existingSubmitRequirements.get(labelName);
- if (!sr.get().equals(existing)) {
- ui.message(
- String.format(
- "Warning: Skipping creating a submit requirement for label '%s'. An existing "
- + "submit requirement is already present but its definition is not "
- + "identical to the existing label definition.",
- labelName));
+ projectConfig.load(project, repo);
+ Config cfg = projectConfig.getConfig();
+ Map<String, LabelAttributes> labelTypes = getLabelTypes(cfg);
+ Map<String, SubmitRequirement> existingSubmitRequirements = loadSubmitRequirements(cfg);
+ for (Map.Entry<String, LabelAttributes> lt : labelTypes.entrySet()) {
+ String labelName = lt.getKey();
+ LabelAttributes attributes = lt.getValue();
+ if (attributes.function().equals("PatchSetLock")) {
+ // PATCH_SET_LOCK functions should be left as is
+ continue;
}
- continue;
+ // If the function is other than "NoBlock" we want to reset the label function regardless
+ // of whether there exists a "submit requirement".
+ if (!attributes.function().equals("NoBlock")) {
+ updated = true;
+ writeLabelFunction(cfg, labelName, "NoBlock");
+ }
+ Optional<SubmitRequirement> sr = createSrFromLabelDef(labelName, attributes);
+ if (!sr.isPresent()) {
+ continue;
+ }
+ // Make the operation idempotent by skipping creating the submit-requirement if one was
+ // already created or previously existed.
+ if (existingSubmitRequirements.containsKey(labelName.toLowerCase(Locale.ROOT))) {
+ if (!sr.get()
+ .equals(existingSubmitRequirements.get(labelName.toLowerCase(Locale.ROOT)))) {
+ ui.message(
+ String.format(
+ "Warning: Skipping creating a submit requirement for label '%s'. An existing "
+ + "submit requirement is already present but its definition is not "
+ + "identical to the existing label definition.",
+ labelName));
+ }
+ continue;
+ }
+ updated = true;
+ ui.message(
+ String.format(
+ "Project %s: Creating a submit requirement for label %s", project, labelName));
+ writeSubmitRequirement(cfg, sr.get());
}
- updated = true;
- ui.message(
- String.format(
- "Project %s: Creating a submit requirement for label %s", project, labelName));
- existingSubmitRequirements.put(sr.get());
+ if (updated) {
+ commit(projectConfig, md);
+ }
}
return updated ? Status.MIGRATED : Status.NO_CHANGE;
}
- private static Optional<SubmitRequirement> createSrFromLabelDef(LabelType lt) {
- if (isLabelSkipped(lt)) {
- return Optional.of(createNonApplicableSr(lt));
- } else if (isBlockingOrRequiredLabel(lt)) {
- return Optional.of(createBlockingOrRequiredSr(lt));
+ /**
+ * Returns a Map containing label names as string in keys along with some of its attributes (that
+ * we need in the migration) like canOverride, ignoreSelfApproval and function in the value.
+ */
+ private Map<String, LabelAttributes> getLabelTypes(Config cfg) {
+ Map<String, LabelAttributes> labelTypes = new HashMap<>();
+ for (String labelName : cfg.getSubsections(ProjectConfig.LABEL)) {
+ String function = cfg.getString(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_FUNCTION);
+ boolean canOverride =
+ cfg.getBoolean(
+ ProjectConfig.LABEL,
+ labelName,
+ ProjectConfig.KEY_CAN_OVERRIDE,
+ /* defaultValue= */ true);
+ boolean ignoreSelfApproval =
+ cfg.getBoolean(
+ ProjectConfig.LABEL,
+ labelName,
+ ProjectConfig.KEY_IGNORE_SELF_APPROVAL,
+ /* defaultValue= */ false);
+ ImmutableList<String> values =
+ ImmutableList.<String>builder()
+ .addAll(
+ Arrays.asList(
+ cfg.getStringList(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_VALUE)))
+ .build();
+ ImmutableList<String> refPatterns =
+ ImmutableList.<String>builder()
+ .addAll(
+ Arrays.asList(
+ cfg.getStringList(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_BRANCH)))
+ .build();
+ LabelAttributes attributes =
+ LabelAttributes.create(
+ function == null ? "MaxWithBlock" : function,
+ canOverride,
+ ignoreSelfApproval,
+ values,
+ refPatterns);
+ labelTypes.put(labelName, attributes);
+ }
+ return labelTypes;
+ }
+
+ private void writeSubmitRequirement(Config cfg, SubmitRequirement sr) {
+ if (sr.description().isPresent()) {
+ cfg.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ sr.name(),
+ ProjectConfig.KEY_SR_DESCRIPTION,
+ sr.description().get());
+ }
+ if (sr.applicabilityExpression().isPresent()) {
+ cfg.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ sr.name(),
+ ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION,
+ sr.applicabilityExpression().get().expressionString());
+ }
+ cfg.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ sr.name(),
+ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+ sr.submittabilityExpression().expressionString());
+ if (sr.overrideExpression().isPresent()) {
+ cfg.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ sr.name(),
+ ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION,
+ sr.overrideExpression().get().expressionString());
+ }
+ cfg.setBoolean(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ sr.name(),
+ ProjectConfig.KEY_SR_OVERRIDE_IN_CHILD_PROJECTS,
+ sr.allowOverrideInChildProjects());
+ }
+
+ private void writeLabelFunction(Config cfg, String labelName, String function) {
+ cfg.setString(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_FUNCTION, function);
+ }
+
+ private void commit(VersionedConfigFile projectConfig, MetaDataUpdate md) throws IOException {
+ md.getCommitBuilder().setAuthor(serverUser);
+ md.getCommitBuilder().setCommitter(serverUser);
+ md.setMessage(COMMIT_MSG);
+ projectConfig.commit(md);
+ }
+
+ private static Optional<SubmitRequirement> createSrFromLabelDef(
+ String labelName, LabelAttributes attributes) {
+ if (isLabelSkipped(attributes.values())) {
+ return Optional.of(createNonApplicableSr(labelName, attributes.canOverride()));
+ } else if (isBlockingOrRequiredLabel(attributes.function())) {
+ return Optional.of(createBlockingOrRequiredSr(labelName, attributes));
}
return Optional.empty();
}
- private static SubmitRequirement createNonApplicableSr(LabelType lt) {
+ private static SubmitRequirement createNonApplicableSr(String labelName, boolean canOverride) {
return SubmitRequirement.builder()
- .setName(lt.getName())
+ .setName(labelName)
.setApplicabilityExpression(SubmitRequirementExpression.of("is:false"))
.setSubmittabilityExpression(SubmitRequirementExpression.create("is:true"))
- .setAllowOverrideInChildProjects(lt.isCanOverride())
+ .setAllowOverrideInChildProjects(canOverride)
.build();
}
@@ -221,49 +302,51 @@
* Create a "submit requirement" that is only satisfied if the label is voted with the max votes
* and/or not voted by the min vote, according to the label attributes.
*/
- private static SubmitRequirement createBlockingOrRequiredSr(LabelType lt) {
+ private static SubmitRequirement createBlockingOrRequiredSr(
+ String labelName, LabelAttributes attributes) {
SubmitRequirement.Builder builder =
SubmitRequirement.builder()
- .setName(lt.getName())
- .setAllowOverrideInChildProjects(lt.isCanOverride());
+ .setName(labelName)
+ .setAllowOverrideInChildProjects(attributes.canOverride());
String maxPart =
- String.format("label:%s=MAX", lt.getName())
- + (lt.isIgnoreSelfApproval() ? ",user=non_uploader" : "");
- switch (lt.getFunction()) {
- case MAX_WITH_BLOCK ->
+ String.format("label:%s=MAX", labelName)
+ + (attributes.ignoreSelfApproval() ? ",user=non_uploader" : "");
+ switch (attributes.function()) {
+ case "MaxWithBlock" ->
builder.setSubmittabilityExpression(
SubmitRequirementExpression.create(
- String.format("%s AND -label:%s=MIN", maxPart, lt.getName())));
- case ANY_WITH_BLOCK ->
+ String.format("%s AND -label:%s=MIN", maxPart, labelName)));
+ case "AnyWithBlock" ->
builder.setSubmittabilityExpression(
- SubmitRequirementExpression.create(String.format("-label:%s=MIN", lt.getName())));
- case MAX_NO_BLOCK ->
+ SubmitRequirementExpression.create(String.format("-label:%s=MIN", labelName)));
+ case "MaxNoBlock" ->
builder.setSubmittabilityExpression(SubmitRequirementExpression.create(maxPart));
default -> {}
}
- ImmutableList<String> refPatterns = lt.getRefPatterns();
- if (refPatterns != null && !refPatterns.isEmpty()) {
+ if (!attributes.refPatterns().isEmpty()) {
builder.setApplicabilityExpression(
SubmitRequirementExpression.of(
String.join(
" OR ",
- lt.getRefPatterns().stream()
+ attributes.refPatterns().stream()
.map(b -> "branch:\\\"" + b + "\\\"")
.collect(Collectors.toList()))));
}
return builder.build();
}
- private static boolean isBlockingOrRequiredLabel(LabelType lt) {
- return switch (lt.getFunction()) {
- case ANY_WITH_BLOCK, MAX_WITH_BLOCK, MAX_NO_BLOCK -> true;
- case NO_BLOCK, NO_OP, PATCH_SET_LOCK -> false;
- };
+ private static boolean isBlockingOrRequiredLabel(String function) {
+ return function.equals("AnyWithBlock")
+ || function.equals("MaxWithBlock")
+ || function.equals("MaxNoBlock");
}
- private static boolean isLabelSkipped(LabelType lt) {
- ImmutableList<LabelValue> values = lt.getValues();
- return values.isEmpty() || (values.size() == 1 && values.get(0).getValue() == 0);
+ /**
+ * Returns true if the label definition was skipped in the project, i.e. it had only one defined
+ * value: zero.
+ */
+ private static boolean isLabelSkipped(List<String> values) {
+ return values.isEmpty() || (values.size() == 1 && values.get(0).startsWith("0"));
}
public boolean anyProjectHasProlog(Collection<Project.NameKey> allProjects) throws IOException {
@@ -295,53 +378,85 @@
}
}
- private boolean hasMigrationAlreadyRun(Project.NameKey project) throws IOException {
- try (Repository repo = repoManager.openRepository(project)) {
- try (RevWalk revWalk = new RevWalk(repo)) {
- Ref refsMetaConfig = repo.exactRef(RefNames.REFS_CONFIG);
- if (refsMetaConfig == null) {
- return false;
- }
- revWalk.markStart(revWalk.parseCommit(refsMetaConfig.getObjectId()));
- RevCommit commit;
- while ((commit = revWalk.next()) != null) {
- if (COMMIT_MSG.equals(commit.getShortMessage())) {
- return true;
- }
- }
+ /**
+ * Returns a map containing submit requirement names in lower name as keys, with {@link
+ * com.google.gerrit.entities.SubmitRequirement} as value.
+ */
+ private Map<String, SubmitRequirement> loadSubmitRequirements(Config rc) {
+ Map<String, SubmitRequirement> allRequirements = new LinkedHashMap<>();
+ for (String name : rc.getSubsections(ProjectConfig.SUBMIT_REQUIREMENT)) {
+ String description =
+ rc.getString(ProjectConfig.SUBMIT_REQUIREMENT, name, ProjectConfig.KEY_SR_DESCRIPTION);
+ String applicabilityExpr =
+ rc.getString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ name,
+ ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION);
+ String submittabilityExpr =
+ rc.getString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ name,
+ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION);
+ String overrideExpr =
+ rc.getString(
+ ProjectConfig.SUBMIT_REQUIREMENT, name, ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION);
+ boolean canInherit =
+ rc.getBoolean(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ name,
+ ProjectConfig.KEY_SR_OVERRIDE_IN_CHILD_PROJECTS,
+ false);
+ SubmitRequirement submitRequirement =
+ SubmitRequirement.builder()
+ .setName(name)
+ .setDescription(Optional.ofNullable(description))
+ .setApplicabilityExpression(SubmitRequirementExpression.of(applicabilityExpr))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create(submittabilityExpr))
+ .setOverrideExpression(SubmitRequirementExpression.of(overrideExpr))
+ .setAllowOverrideInChildProjects(canInherit)
+ .build();
+ allRequirements.put(name.toLowerCase(Locale.ROOT), submitRequirement);
+ }
+ return allRequirements;
+ }
+
+ private static boolean hasMigrationAlreadyRun(Repository repo) throws IOException {
+ try (RevWalk revWalk = new RevWalk(repo)) {
+ Ref refsMetaConfig = repo.exactRef(RefNames.REFS_CONFIG);
+ if (refsMetaConfig == null) {
return false;
}
+ revWalk.markStart(revWalk.parseCommit(refsMetaConfig.getObjectId()));
+ RevCommit commit;
+ while ((commit = revWalk.next()) != null) {
+ if (COMMIT_MSG.equals(commit.getShortMessage())) {
+ return true;
+ }
+ }
+ return false;
}
}
- /**
- * Helper "Map" to of submit requirements with case-preserving keys and case-insensitive lookup
- */
- private static class SubmitRequirementMap {
- private final Map<String, SubmitRequirement> submitRequirements;
- private final Map<String, String> lowerCaseToOriginalNames;
+ @AutoValue
+ abstract static class LabelAttributes {
+ abstract String function();
- SubmitRequirementMap(Map<String, SubmitRequirement> submitRequirements) {
- this.submitRequirements = submitRequirements;
- this.lowerCaseToOriginalNames =
- submitRequirements.keySet().stream()
- .collect(Collectors.toMap(k -> k.toLowerCase(Locale.ROOT), k -> k));
- }
+ abstract boolean canOverride();
- boolean containsKey(String name) {
- return lowerCaseToOriginalNames.containsKey(name.toLowerCase(Locale.ROOT));
- }
+ abstract boolean ignoreSelfApproval();
- @Nullable
- SubmitRequirement get(String name) {
- String orig = lowerCaseToOriginalNames.get(name.toLowerCase(Locale.ROOT));
- return orig != null ? submitRequirements.get(orig) : null;
- }
+ abstract ImmutableList<String> values();
- void put(SubmitRequirement sr) {
- String name = sr.name();
- submitRequirements.put(name, sr);
- lowerCaseToOriginalNames.put(name.toLowerCase(Locale.ROOT), name);
+ abstract ImmutableList<String> refPatterns();
+
+ static LabelAttributes create(
+ String function,
+ boolean canOverride,
+ boolean ignoreSelfApproval,
+ ImmutableList<String> values,
+ ImmutableList<String> refPatterns) {
+ return new AutoValue_MigrateLabelFunctionsToSubmitRequirement_LabelAttributes(
+ function, canOverride, ignoreSelfApproval, values, refPatterns);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
index 7e237f5..8e1b86e 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
@@ -446,7 +446,7 @@
private TestUpdateUI runMigration(Status expectedResult) throws Exception {
TestUpdateUI updateUi = new TestUpdateUI();
MigrateLabelFunctionsToSubmitRequirement executor =
- new MigrateLabelFunctionsToSubmitRequirement(repoMetaDataUpdater, repoManager);
+ new MigrateLabelFunctionsToSubmitRequirement(repoManager, serverIdent.get());
Status status = executor.executeMigration(project, updateUi);
assertThat(status).isEqualTo(expectedResult);
projectCache.evictAndReindex(project);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index b38bf9d..51ca30a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.extensions.client.ReviewerState.CC;
import static com.google.gerrit.extensions.client.ReviewerState.REMOVED;
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -26,6 +27,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.Address;
@@ -38,10 +40,12 @@
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ReviewerUpdateInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import java.lang.reflect.Type;
+import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import org.junit.Before;
@@ -422,6 +426,35 @@
assertThat(gApi.changes().id(r.getChangeId()).addReviewer(input).ccs).isEmpty();
}
+ @Test
+ public void removeCcByEmailWithoutPermissionFails() throws Exception {
+ PushOneCommit.Result r = createChange();
+ TestAccount newUser = createAccounts(1, name("foo")).get(0);
+
+ ReviewerInput input = new ReviewerInput();
+ input.reviewer = "nonexisting@example.com";
+ input.state = ReviewerState.CC;
+ gApi.changes().id(r.getChangeId()).addReviewer(input);
+ requestScopeOperations.setApiUser(newUser.id());
+ assertThat(gApi.changes().id(r.getChangeId()).get().removableReviewers).isEmpty();
+
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () -> gApi.changes().id(r.getChangeId()).reviewer(input.reviewer).remove());
+ assertThat(thrown).hasMessageThat().contains("remove reviewer not permitted");
+ }
+
+ private List<TestAccount> createAccounts(int n, String emailPrefix) throws Exception {
+ List<TestAccount> result = new ArrayList<>(n);
+ for (int i = 0; i < n; i++) {
+ result.add(
+ accountCreator.create(
+ name("u" + i), emailPrefix + "-" + i + "@example.com", "Full Name " + i, null));
+ }
+ return result;
+ }
+
private static String toRfcAddressString(AccountInfo info) {
return Address.create(info.name, info.email).toString();
}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index 6653e1f..792e3b1 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -1327,6 +1327,7 @@
@Test
public void deleteReviewerByEmailFromWipChange() throws Exception {
StagedChange sc = stageWipChangeWithExtraReviewer();
+ requestScopeOperations.setApiUser(sc.owner.id());
gApi.changes().id(sc.changeId).reviewer(StagedUsers.REVIEWER_BY_EMAIL).remove();
assertThat(sender).didNotSend();
}
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 934fd65..37e7ccf 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
@@ -16,6 +16,7 @@
import '../../shared/gr-editable-content/gr-editable-content';
import '../../shared/gr-formatted-text/gr-formatted-text';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
+import '../../shared/gr-content-with-sidebar/gr-content-with-sidebar';
import '../gr-change-actions/gr-change-actions';
import '../gr-change-summary/gr-change-summary';
import '../gr-change-metadata/gr-change-metadata';
@@ -160,6 +161,7 @@
const TRAILING_WHITESPACE_REGEX = /[ \t]+$/gm;
const PREFIX = '#message-';
+
@customElement('gr-change-view')
export class GrChangeView extends LitElement {
/**
@@ -350,6 +352,9 @@
@state()
activeTab: Tab | string = Tab.FILES;
+ @state()
+ private showSidebarChat = false;
+
@property({type: Boolean})
unresolvedOnly = true;
@@ -790,6 +795,13 @@
sharedStyles,
modalStyles,
css`
+ :host {
+ --change-header-height: 38px;
+ --sidebar-top: calc(
+ var(--main-header-height) + var(--change-header-height)
+ );
+ --sidebar-bottom-overflow: var(--main-footer-height);
+ }
.tabs {
display: flex;
}
@@ -806,6 +818,10 @@
border-bottom: 2px solid var(--border-color);
display: flex;
padding: var(--spacing-s) var(--spacing-l);
+ position: sticky;
+ top: var(--main-header-height);
+ height: var(--change-header-height);
+ z-index: 110;
}
.header.active {
border-color: var(--status-active);
@@ -1114,12 +1130,18 @@
private renderMainContent() {
return html`
- <div id="mainContent" class="container" ?hidden=${!!this.loading}>
- ${this.renderChangeInfoSection()}
- <h2 class="assistive-tech-only">Files and Comments tabs</h2>
- ${this.renderTabHeaders()} ${this.renderTabContent()}
- ${this.renderChangeLog()}
- </div>
+ ${this.renderHeader()}
+ <gr-content-with-sidebar .hideSide=${!this.showSidebarChat}>
+ <div slot="main">
+ <div id="mainContent" class="container" ?hidden=${!!this.loading}>
+ ${this.renderChangeInfoSection()}
+ <h2 class="assistive-tech-only">Files and Comments tabs</h2>
+ ${this.renderTabHeaders()} ${this.renderTabContent()}
+ ${this.renderChangeLog()}
+ </div>
+ </div>
+ <div slot="side">${this.renderSidebar()}</div>
+ </gr-content-with-sidebar>
<gr-apply-fix-dialog id="applyFixDialog"></gr-apply-fix-dialog>
<dialog id="downloadModal" tabindex="-1">
<gr-download-dialog
@@ -1151,17 +1173,30 @@
`;
}
- private renderChangeInfoSection() {
- return html`<section class="changeInfoSection">
+ private renderHeader() {
+ if (this.loading) return;
+ return html`
<div class=${this.computeHeaderClass()}>
<h1 class="assistive-tech-only">
Change ${this.change?._number}: ${this.change?.subject}
</h1>
${this.renderHeaderTitle()} ${this.renderCommitActions()}
</div>
- <h2 class="assistive-tech-only">Change metadata</h2>
- ${this.renderChangeInfo()}
- </section>`;
+ `;
+ }
+
+ private renderSidebar() {
+ if (!this.showSidebarChat) return;
+ return html` <div>Hello, I am a fancy sidebar for AI Chat.</div> `;
+ }
+
+ private renderChangeInfoSection() {
+ return html`
+ <section class="changeInfoSection">
+ <h2 class="assistive-tech-only">Change metadata</h2>
+ ${this.renderChangeInfo()}
+ </section>
+ `;
}
private renderHeaderTitle() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 20bfeec..378d5eb 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -293,10 +293,6 @@
changeNum: TEST_NUMERIC_CHANGE_ID,
repo: 'gerrit' as RepoName,
};
- await element.updateComplete.then(() => {
- assertIsDefined(element.actions);
- sinon.stub(element.actions, 'reload').returns(Promise.resolve());
- });
userModel = testResolver(userModelToken);
changeModel = testResolver(changeModelToken);
});
@@ -310,147 +306,127 @@
element,
/* HTML */ `
<div class="container loading">Loading...</div>
- <div class="container" hidden="" id="mainContent">
- <section class="changeInfoSection">
- <div class="header">
- <h1 class="assistive-tech-only">Change :</h1>
- <div class="headerTitle">
- <div class="changeStatuses"></div>
- <div class="changeStarContainer">
- <gr-button
- aria-disabled="false"
- class="showCopyLinkDialogButton"
- down-arrow=""
- flatten=""
- id="copyLinkDialogButton"
- role="button"
+ <gr-content-with-sidebar>
+ <div slot="main">
+ <div class="container" hidden="" id="mainContent">
+ <section class="changeInfoSection">
+ <h2 class="assistive-tech-only">Change metadata</h2>
+ <div class="changeInfo">
+ <div class="changeInfo-column changeMetadata">
+ <gr-change-metadata id="metadata"> </gr-change-metadata>
+ </div>
+ <div
+ class="changeInfo-column mainChangeInfo"
+ id="mainChangeInfo"
+ >
+ <div id="commitAndRelated">
+ <div class="commitContainer">
+ <h3 class="assistive-tech-only">Commit Message</h3>
+ <div>
+ <gr-button
+ aria-disabled="false"
+ class="reply"
+ id="replyBtn"
+ primary=""
+ role="button"
+ tabindex="0"
+ title="Open reply dialog to publish comments and add reviewers (shortcut: a)"
+ >
+ Reply
+ </gr-button>
+ </div>
+ <div class="commitMessage" id="commitMessage">
+ <gr-editable-content
+ id="commitMessageEditor"
+ remove-zero-width-space=""
+ >
+ <gr-formatted-text> </gr-formatted-text>
+ </gr-editable-content>
+ </div>
+ <h3 class="assistive-tech-only">
+ Comments and Checks Summary
+ </h3>
+ <gr-change-summary> </gr-change-summary>
+ <gr-endpoint-decorator name="commit-container">
+ <gr-endpoint-param name="change"> </gr-endpoint-param>
+ <gr-endpoint-param name="revision">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </div>
+ <div class="relatedChanges">
+ <gr-related-changes-list> </gr-related-changes-list>
+ </div>
+ <div class="emptySpace"></div>
+ </div>
+ </div>
+ </div>
+ </section>
+ <h2 class="assistive-tech-only">Files and Comments tabs</h2>
+ <div class="tabs">
+ <md-tabs id="tabs">
+ <md-secondary-tab
+ active=""
+ data-name="files"
+ md-tab=""
tabindex="0"
>
- <gr-change-star id="changeStar"> </gr-change-star>
- <a aria-label="Change undefined" class="changeNumber"> </a>
- </gr-button>
- </div>
- <div class="headerSubject"></div>
- <gr-copy-clipboard
- class="changeCopyClipboard"
- hideinput=""
- text="undefined: undefined | http://localhost:9876undefined"
- >
- </gr-copy-clipboard>
- </div>
- <div class="commitActions">
- <gr-change-actions id="actions"> </gr-change-actions>
- </div>
- </div>
- <h2 class="assistive-tech-only">Change metadata</h2>
- <div class="changeInfo">
- <div class="changeInfo-column changeMetadata">
- <gr-change-metadata id="metadata"> </gr-change-metadata>
- </div>
- <div class="changeInfo-column mainChangeInfo" id="mainChangeInfo">
- <div id="commitAndRelated">
- <div class="commitContainer">
- <h3 class="assistive-tech-only">Commit Message</h3>
- <div>
- <gr-button
- aria-disabled="false"
- class="reply"
- id="replyBtn"
- primary=""
- role="button"
- tabindex="0"
- title="Open reply dialog to publish comments and add reviewers (shortcut: a)"
- >
- Reply
- </gr-button>
- </div>
- <div class="commitMessage" id="commitMessage">
- <gr-editable-content
- id="commitMessageEditor"
- remove-zero-width-space=""
- >
- <gr-formatted-text> </gr-formatted-text>
- </gr-editable-content>
- </div>
- <h3 class="assistive-tech-only">
- Comments and Checks Summary
- </h3>
- <gr-change-summary> </gr-change-summary>
- <gr-endpoint-decorator name="commit-container">
+ <span> Files </span>
+ </md-secondary-tab>
+ <md-secondary-tab
+ class="commentThreads"
+ data-name="comments"
+ md-tab=""
+ tabindex="-1"
+ >
+ <gr-tooltip-content has-tooltip="" title="">
+ <span> Comments </span>
+ </gr-tooltip-content>
+ </md-secondary-tab>
+ <md-secondary-tab
+ data-name="change-view-tab-header-url"
+ md-tab=""
+ tabindex="0"
+ >
+ <gr-endpoint-decorator name="change-view-tab-header-url">
<gr-endpoint-param name="change"> </gr-endpoint-param>
<gr-endpoint-param name="revision"> </gr-endpoint-param>
</gr-endpoint-decorator>
- </div>
- <div class="relatedChanges">
- <gr-related-changes-list> </gr-related-changes-list>
- </div>
- <div class="emptySpace"></div>
- </div>
+ </md-secondary-tab>
+ </md-tabs>
</div>
+ <section class="tabContent">
+ <div>
+ <gr-file-list-header id="fileListHeader">
+ </gr-file-list-header>
+ <gr-revision-parents> </gr-revision-parents>
+ <gr-file-list id="fileList"> </gr-file-list>
+ </div>
+ </section>
+ <gr-endpoint-decorator name="change-view-integration">
+ <gr-endpoint-param name="change"> </gr-endpoint-param>
+ <gr-endpoint-param name="revision"> </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ <div class="tabs">
+ <md-tabs>
+ <md-secondary-tab
+ active=""
+ class="changeLog"
+ data-name="_changeLog"
+ md-tab=""
+ tabindex="0"
+ >
+ Change Log
+ </md-secondary-tab>
+ </md-tabs>
+ </div>
+ <section class="changeLog">
+ <h2 class="assistive-tech-only">Change Log</h2>
+ <gr-messages-list> </gr-messages-list>
+ </section>
</div>
- </section>
- <h2 class="assistive-tech-only">Files and Comments tabs</h2>
- <div class="tabs">
- <md-tabs id="tabs">
- <md-secondary-tab
- active=""
- data-name="files"
- md-tab=""
- tabindex="0"
- >
- <span> Files </span>
- </md-secondary-tab>
- <md-secondary-tab
- class="commentThreads"
- data-name="comments"
- md-tab=""
- tabindex="-1"
- >
- <gr-tooltip-content has-tooltip="" title="">
- <span> Comments </span>
- </gr-tooltip-content>
- </md-secondary-tab>
- <md-secondary-tab
- data-name="change-view-tab-header-url"
- md-tab=""
- tabindex="0"
- >
- <gr-endpoint-decorator name="change-view-tab-header-url">
- <gr-endpoint-param name="change"> </gr-endpoint-param>
- <gr-endpoint-param name="revision"> </gr-endpoint-param>
- </gr-endpoint-decorator>
- </md-secondary-tab>
- </md-tabs>
</div>
- <section class="tabContent">
- <div>
- <gr-file-list-header id="fileListHeader"> </gr-file-list-header>
- <gr-revision-parents> </gr-revision-parents>
- <gr-file-list id="fileList"> </gr-file-list>
- </div>
- </section>
- <gr-endpoint-decorator name="change-view-integration">
- <gr-endpoint-param name="change"> </gr-endpoint-param>
- <gr-endpoint-param name="revision"> </gr-endpoint-param>
- </gr-endpoint-decorator>
- <div class="tabs">
- <md-tabs>
- <md-secondary-tab
- active=""
- class="changeLog"
- data-name="_changeLog"
- md-tab=""
- tabindex="0"
- >
- Change Log
- </md-secondary-tab>
- </md-tabs>
- </div>
- <section class="changeLog">
- <h2 class="assistive-tech-only">Change Log</h2>
- <gr-messages-list> </gr-messages-list>
- </section>
- </div>
+ <div slot="side"></div>
+ </gr-content-with-sidebar>
<gr-apply-fix-dialog id="applyFixDialog"> </gr-apply-fix-dialog>
<dialog id="downloadModal" tabindex="-1">
<gr-download-dialog id="downloadDialog" role="dialog">
@@ -647,7 +623,11 @@
suite('keyboard shortcuts', () => {
let clock: SinonFakeTimers;
- setup(() => {
+ setup(async () => {
+ element.change = createChangeViewChange();
+ element.loading = false;
+ await element.updateComplete;
+
clock = sinon.useFakeTimers();
});
@@ -706,7 +686,9 @@
assert.isTrue(loggedInErrorSpy.called);
});
- test('shift A does not open reply overlay', async () => {
+ test('shift A does not open reply overlay, if not logged in', async () => {
+ element.loggedIn = false;
+ await element.updateComplete;
pressKey(element, 'a', Modifier.SHIFT_KEY);
await element.updateComplete;
assertIsDefined(element.replyModal);
@@ -919,7 +901,11 @@
assert.equal(element.replyBtn.textContent, 'Sign in');
});
- test('download tap calls handleOpenDownloadDialog', () => {
+ test('download tap calls handleOpenDownloadDialog', async () => {
+ element.change = createChangeViewChange();
+ element.loading = false;
+ await element.updateComplete;
+
assertIsDefined(element.actions);
const openDialogStub = sinon.stub(element, 'handleOpenDownloadDialog');
element.actions.dispatchEvent(
@@ -1126,6 +1112,8 @@
labels: {},
actions: {},
};
+ element.loading = false;
+ await element.updateComplete;
sinon.stub(element, '_getUrlParameter').callsFake(param => {
assert.equal(param, 'revert');
@@ -1285,6 +1273,7 @@
const newChange = {...element.change};
newChange.revisions.rev2 = createRevision(EDIT);
element.change = newChange;
+ element.loading = false;
await element.updateComplete;
fireEdit();
@@ -1298,6 +1287,7 @@
newChange.revisions.rev2 = createRevision(2);
element.change = newChange;
element.viewModelPatchNum = 1 as RevisionPatchSetNum;
+ element.loading = false;
await element.updateComplete;
fireEdit();
@@ -1314,6 +1304,7 @@
newChange.revisions.rev2 = createRevision(2);
element.change = newChange;
element.patchNum = 2 as RevisionPatchSetNum;
+ element.loading = false;
await element.updateComplete;
fireEdit();
@@ -1329,6 +1320,7 @@
element.change = {
...createChangeViewChange(),
};
+ element.loading = false;
await element.updateComplete;
assertIsDefined(element.metadata);
assertIsDefined(element.actions);
@@ -1374,6 +1366,7 @@
starred: false,
};
element.loggedIn = true;
+ element.loading = false;
await element.updateComplete;
const stub = sinon.stub(element, 'handleToggleStar');
@@ -1458,6 +1451,7 @@
status: ChangeStatus.MERGED,
current_revision: sha,
};
+ element.loading = false;
await element.updateComplete;
const copyLinksDialog = queryAndAssert<GrCopyLinks>(
@@ -1471,6 +1465,7 @@
test('copy links without a base URL', async () => {
element.change = createChangeViewChange();
+ element.loading = false;
await element.updateComplete;
const copyLinksDialog = queryAndAssert<GrCopyLinks>(
@@ -1487,6 +1482,7 @@
test('copy links with a base URL having a path', async () => {
stubBaseUrl('/review');
element.change = createChangeViewChange();
+ element.loading = false;
await element.updateComplete;
const copyLinksDialog = queryAndAssert<GrCopyLinks>(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 80233b05..4b8ce40 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -539,7 +539,7 @@
.stickyHeader {
background-color: var(--view-background-color);
position: sticky;
- top: 0;
+ top: var(--main-header-height);
/* sidebar should outrank <footer> in GrAppElement */
z-index: 110;
box-shadow: var(--elevation-level-1);
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index acc8cf2..793ed14 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -282,6 +282,8 @@
display: flex;
flex-direction: column;
min-height: 100%;
+ --main-header-height: 48px;
+ --main-footer-height: 36px;
}
gr-main-header,
footer {
@@ -297,17 +299,21 @@
header should be shown on top of the sticky diff header, which has a
z-index of 110. */
z-index: 111;
+ position: sticky;
+ top: 0;
+ height: var(--main-header-height);
}
footer {
background: var(
--footer-background,
var(--footer-background-color, #eee)
);
+ height: var(--main-footer-height);
border-top: var(--footer-border-top);
display: flex;
justify-content: space-between;
padding: var(--spacing-m) var(--spacing-l);
- z-index: 100;
+ z-index: 10;
}
main {
flex: 1;
diff --git a/polygerrit-ui/app/elements/shared/gr-content-with-sidebar/gr-content-with-sidebar.ts b/polygerrit-ui/app/elements/shared/gr-content-with-sidebar/gr-content-with-sidebar.ts
new file mode 100644
index 0000000..b61a10a
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-content-with-sidebar/gr-content-with-sidebar.ts
@@ -0,0 +1,173 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {customElement, property, query, state} from 'lit/decorators.js';
+import {css, html, LitElement} from 'lit';
+import {styleMap} from 'lit/directives/style-map.js';
+
+const SIDEBAR_MIN_WIDTH = 300;
+
+/**
+ * A component that displays content in a main area and a resizable sidebar.
+ * The sidebar can be toggled between hidden and visible.
+ *
+ * slot main - The content to be displayed in the main area.
+ * slot side - The content to be displayed in the sidebar.
+ */
+@customElement('gr-content-with-sidebar')
+export class GrContentWithSidebar extends LitElement {
+ @query('.sidebar-wrapper') sidebarWrapper?: HTMLElement;
+
+ @state()
+ private sidebarWidthPx = SIDEBAR_MIN_WIDTH;
+
+ @property()
+ hideSide = true;
+
+ private isSidebarResizing = false;
+
+ private sidebarResizingStartPosPx = 0;
+
+ private sidebarResizingStartWidthPx = 0;
+
+ private readonly boundResizeSidebar = (e: MouseEvent) =>
+ this.resizeSidebar(e);
+
+ private readonly boundStopSidebarResize = () => this.stopSidebarResize();
+
+ static override get styles() {
+ return [
+ css`
+ :host {
+ display: block;
+ --sidebar-height: calc(100vh - var(--sidebar-top));
+ --sidebar-top: var(--sidebar-top, 0px);
+ --sidebar-bottom-overflow: var(--sidebar-bottom-overflow, 0px);
+ }
+ .sidebar-wrapper {
+ z-index: 50;
+ position: absolute;
+ display: flex;
+ top: 0;
+ bottom: calc(0px - var(--sidebar-bottom-overflow));
+ right: 0;
+ min-width: 300px;
+ max-width: 100%;
+ background-color: var(--background-color-secondary);
+ }
+ .sidebar {
+ position: sticky;
+ top: var(--sidebar-top);
+ height: var(--sidebar-height);
+ box-sizing: border-box;
+ overflow: auto;
+ flex-grow: 1;
+ padding: var(--spacing-l);
+ font-size: 14px;
+ }
+ .resizer-wrapper {
+ position: sticky;
+ top: var(--sidebar-top);
+ height: var(--sidebar-height);
+ z-index: 51;
+ }
+ .resizer {
+ background-color: var(--background-color-secondary);
+ width: 7px;
+ border-left: 1px solid var(--border-color);
+ cursor: ew-resize;
+ position: absolute;
+ top: 0;
+ bottom: 0;
+ left: -7px;
+ box-sizing: border-box;
+ }
+ .resizer:hover {
+ background-color: var(--background-color-tertiary);
+ width: 11px;
+ left: -9px;
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ const widthPx = this.hideSide ? 0 : this.sidebarWidthPx;
+ return html`
+ <div>
+ <div style=${styleMap({width: `calc(100% - ${widthPx}px)`})}>
+ <slot name="main"></slot>
+ </div>
+ ${this.renderSidebar()}
+ </div>
+ `;
+ }
+
+ private renderSidebar() {
+ if (this.hideSide) return;
+ return html`
+ <div
+ class="sidebar-wrapper"
+ style=${styleMap({width: `${this.sidebarWidthPx}px`})}
+ >
+ <div class="resizer-wrapper">
+ <div
+ class="resizer"
+ role="separator"
+ aria-orientation="vertical"
+ aria-valuenow=${this.sidebarWidthPx}
+ aria-label="Resize sidebar"
+ tabindex="0"
+ @mousedown=${this.startSidebarResize}
+ ></div>
+ </div>
+ <div class="sidebar">
+ <slot name="side"></slot>
+ </div>
+ </div>
+ `;
+ }
+
+ private startSidebarResize(event: MouseEvent) {
+ if (this.isSidebarResizing) return;
+
+ // Disable user selection while resizing.
+ document.body.style.setProperty('user-select', 'none');
+ this.isSidebarResizing = true;
+ this.sidebarResizingStartPosPx = event.clientX;
+ this.sidebarResizingStartWidthPx =
+ this.sidebarWrapper!.getBoundingClientRect().width;
+ window.addEventListener('mousemove', this.boundResizeSidebar);
+ window.addEventListener('mouseup', this.boundStopSidebarResize);
+ }
+
+ private stopSidebarResize() {
+ if (!this.isSidebarResizing) return;
+
+ // Re-enable user selection when resizing is done.
+ document.body.style.setProperty('user-select', 'auto');
+ this.isSidebarResizing = false;
+ this.sidebarResizingStartPosPx = 0;
+ this.sidebarResizingStartWidthPx = 0;
+ window.removeEventListener('mousemove', this.boundResizeSidebar);
+ window.removeEventListener('mouseup', this.boundStopSidebarResize);
+ }
+
+ private resizeSidebar(event: MouseEvent) {
+ if (!this.isSidebarResizing || event.buttons === 0) return;
+
+ const widthDiffPx = event.clientX - this.sidebarResizingStartPosPx;
+ this.sidebarWidthPx = Math.max(
+ this.sidebarResizingStartWidthPx - widthDiffPx,
+ SIDEBAR_MIN_WIDTH
+ );
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-content-with-sidebar': GrContentWithSidebar;
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-content-with-sidebar/gr-content-with-sidebar_test.ts b/polygerrit-ui/app/elements/shared/gr-content-with-sidebar/gr-content-with-sidebar_test.ts
new file mode 100644
index 0000000..1b09682
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-content-with-sidebar/gr-content-with-sidebar_test.ts
@@ -0,0 +1,67 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import {assert, fixture, html} from '@open-wc/testing';
+import './gr-content-with-sidebar';
+import {GrContentWithSidebar} from './gr-content-with-sidebar';
+
+suite('gr-content-with-sidebar tests', () => {
+ let element: GrContentWithSidebar;
+
+ setup(async () => {
+ element = await fixture<GrContentWithSidebar>(
+ html`<gr-content-with-sidebar></gr-content-with-sidebar>`
+ );
+ await element.updateComplete;
+ });
+
+ test('renders no sidebar', async () => {
+ element.hideSide = true;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <div>
+ <div style="width:calc(100% - 0px);">
+ <slot name="main"></slot>
+ </div>
+ </div>
+ `
+ );
+ });
+
+ test('renders sidebar', async () => {
+ element.hideSide = false;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <div>
+ <div style="width: calc(100% - 300px);">
+ <slot name="main"> </slot>
+ </div>
+ <div class="sidebar-wrapper" style="width:300px;">
+ <div class="resizer-wrapper">
+ <div
+ aria-label="Resize sidebar"
+ aria-orientation="vertical"
+ aria-valuenow="300"
+ class="resizer"
+ role="separator"
+ tabindex="0"
+ ></div>
+ </div>
+ <div class="sidebar">
+ <slot name="side"> </slot>
+ </div>
+ </div>
+ </div>
+ `
+ );
+ });
+});
diff --git a/polygerrit-ui/app/elements/shared/gr-page-nav/gr-page-nav.ts b/polygerrit-ui/app/elements/shared/gr-page-nav/gr-page-nav.ts
index ec79b5a..e66da3c 100644
--- a/polygerrit-ui/app/elements/shared/gr-page-nav/gr-page-nav.ts
+++ b/polygerrit-ui/app/elements/shared/gr-page-nav/gr-page-nav.ts
@@ -53,6 +53,7 @@
}
nav.pinned {
position: fixed;
+ top: var(--main-header-height);
}
@media only screen and (max-width: 53em) {
nav {
