Merge "Show quick-approve button even when change is already approved"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 529718a..38720fe 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -869,6 +869,11 @@
private changes (even without having the `View Private Changes` access
right assigned).
+**NOTE**: If link:config-gerrit.html#auth.skipFullRefEvaluationIfAllRefsAreVisible[
+auth.skipFullRefEvaluationIfAllRefsAreVisible] is `true` (which is the case by
+default) privates changes and all change edit refs are also visible to users
+that have read access on `refs/*`.
+
[[category_toggle_work_in_progress_state]]
=== Toggle Work In Progress state
diff --git a/Documentation/concept-patch-sets.txt b/Documentation/concept-patch-sets.txt
index 8609afd..274fbb0 100644
--- a/Documentation/concept-patch-sets.txt
+++ b/Documentation/concept-patch-sets.txt
@@ -89,7 +89,7 @@
set description does not become a part of the project's history.
To add a patch set description, click *Add a patch set description*, located in
-the file list.
+the file list, or provide it link:user-upload.html#patch_set_description[on upload].
GERRIT
------
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index e7c0d13..ac5e3b7 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -684,8 +684,11 @@
[[auth.skipFullRefEvaluationIfAllRefsAreVisible]]auth.skipFullRefEvaluationIfAllRefsAreVisible::
+
-Whether to skip the full ref visibility checks as a performance shortcut when all refs are
-visible to a user. Full ref filtering would filter out things like pending edits.
+Whether to skip the full ref visibility checks as a performance shortcut when a
+user has READ permission for all refs.
++
+The full ref filtering would filter out refs for pending edits, private changes
+and auto merge commits.
+
By default, true.
@@ -742,6 +745,24 @@
+
Default is false.
+[[cache.openFiles]]cache.openFiles::
++
+The number of file descriptors to add to the limit set by the Gerrit daemon.
++
+Persistent caches are stored on the file system and as such participate in the
+file descriptors utilization. The number of file descriptors can vary depending
+on the cache configuration and the specific backend used.
++
+The additional file descriptors required by the cache should be accounted for
+via this setting, so that the Gerrit daemon can adjust the ulimit accordingly.
++
+If you increase this to a larger setting you may need to also adjust
+the ulimit on file descriptors for the host JVM, as Gerrit needs
+additional file descriptors available for network sockets and other
+repository data manipulation.
++
+Default is 0.
+
[[cache.name.maxAge]]cache.<name>.maxAge::
+
Maximum age to keep an entry in the cache. Entries are removed from
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index b2e1589..1db27d5 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2729,8 +2729,8 @@
[source, java]
----
import java.util.Optional;
-import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.common.data.SubmitRecord.Status;
+import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.entities.SubmitRecord.Status;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 3040348..8a95bab 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -66,6 +66,11 @@
* `caches/disk_hit_ratio`: Disk hit ratio for persistent cache.
* `caches/refresh_count`: The number of refreshes per cache with an indicator if a reload was necessary.
+Cache disk metrics are expensive to compute on larger installations and are not
+computed by default. They can be enabled via the
+link:config.gerrit.html#cache.enableDiskStatMetrics[`cache.enableDiskStatMetrics`]
+setting.
+
=== Change
* `change/submit_rule_evaluation`: Latency for evaluating submit rules on a change.
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index 926aa71..cdaf155 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -315,11 +315,11 @@
preference is set so the default behavior is to create `work-in-progress`
changes, this can be overridden with the `ready` option.
-[[message]]
-==== Message
+[[patch_set_description]]
+==== Patch Set Description
-A comment message can be applied to the change by using the `message` (or `m`)
-option:
+A link:concept-patch-sets.html#_description[patch set description] can be
+applied by using the `message` (or `m`) option:
----
git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/experimental%m=This_is_a_rebase_on_master%21
diff --git a/WORKSPACE b/WORKSPACE
index 3c30026..fff9235 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -51,10 +51,10 @@
http_archive(
name = "com_google_protobuf",
- sha256 = "71030a04aedf9f612d2991c1c552317038c3c5a2b578ac4745267a45e7037c29",
- strip_prefix = "protobuf-3.12.3",
+ sha256 = "d0f5f605d0d656007ce6c8b5a82df3037e1d8fe8b121ed42e536f569dec16113",
+ strip_prefix = "protobuf-3.14.0",
urls = [
- "https://github.com/protocolbuffers/protobuf/archive/v3.12.3.tar.gz",
+ "https://github.com/protocolbuffers/protobuf/archive/v3.14.0.tar.gz",
],
)
diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java
index 400861c..5595bc7 100644
--- a/java/com/google/gerrit/entities/RefNames.java
+++ b/java/com/google/gerrit/entities/RefNames.java
@@ -288,10 +288,16 @@
* Whether the ref is managed by Gerrit. Covers all Gerrit-internal refs like refs/cache-automerge
* and refs/meta as well as refs/changes. Does not cover user-created refs like branches or custom
* ref namespaces like refs/my-company.
+ *
+ * <p>Any ref for which this method evaluates to true will be served to users who have the {@code
+ * ACCESS_DATABASE} capability.
+ *
+ * <p><b>Caution</b>Any ref not in this list will be served if the user was granted a READ
+ * permission on it using Gerrit's permission model.
*/
public static boolean isGerritRef(String ref) {
return ref.startsWith(REFS_CHANGES)
- || ref.startsWith(REFS_META)
+ || ref.startsWith(REFS_EXTERNAL_IDS)
|| ref.startsWith(REFS_CACHE_AUTOMERGE)
|| ref.startsWith(REFS_DRAFT_COMMENTS)
|| ref.startsWith(REFS_DELETED_GROUPS)
@@ -299,7 +305,8 @@
|| ref.startsWith(REFS_GROUPS)
|| ref.startsWith(REFS_GROUPNAMES)
|| ref.startsWith(REFS_USERS)
- || ref.startsWith(REFS_STARRED_CHANGES);
+ || ref.startsWith(REFS_STARRED_CHANGES)
+ || ref.startsWith(REFS_REJECT_COMMITS);
}
static Integer parseShardedRefPart(String name) {
diff --git a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
index af00b20..1ad94be 100644
--- a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
+++ b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
@@ -391,11 +391,7 @@
}
private SMTPClient open() throws EmailException {
- final AuthSMTPClient client = new AuthSMTPClient(UTF_8.name());
-
- if (smtpEncryption == Encryption.SSL) {
- client.enableSSL(sslVerify);
- }
+ final AuthSMTPClient client = new AuthSMTPClient(smtpEncryption == Encryption.SSL, sslVerify);
client.setConnectTimeout(connectTimeout);
try {
@@ -411,7 +407,7 @@
}
if (smtpEncryption == Encryption.TLS) {
- if (!client.startTLS(smtpHost, smtpPort, sslVerify)) {
+ if (!client.execTLS()) {
throw new EmailException("SMTP server does not support TLS");
}
if (!client.login()) {
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 0b4828b..37c773a 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -61,11 +61,12 @@
}
/** Can this user see this change? */
- private boolean isVisible(ChangeData cd) {
- if (getChange().isPrivate() && !isPrivateVisible(cd)) {
+ boolean isVisible() {
+ if (getChange().isPrivate() && !isPrivateVisible(changeData)) {
return false;
}
- return refControl.isVisible();
+ // Does the user have READ permission on the destination?
+ return refControl.asForRef().testOrFalse(RefPermission.READ);
}
/** Can this user abandon this change? */
@@ -201,17 +202,13 @@
private ForChangeImpl() {}
- private ChangeData changeData() {
- return changeData;
- }
-
@Override
public String resourcePath() {
if (resourcePath == null) {
resourcePath =
String.format(
"/projects/%s/+changes/%s",
- getProjectControl().getProjectState().getName(), changeData().getId().get());
+ getProjectControl().getProjectState().getName(), changeData.getId().get());
}
return resourcePath;
}
@@ -256,7 +253,7 @@
try {
switch (perm) {
case READ:
- return isVisible(changeData());
+ return isVisible();
case ABANDON:
return canAbandon();
case DELETE:
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index defec4b..edd3cb1 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -17,10 +17,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.flogger.LazyArgs.lazy;
-import static com.google.gerrit.entities.RefNames.REFS_CACHE_AUTOMERGE;
import static com.google.gerrit.entities.RefNames.REFS_CONFIG;
-import static com.google.gerrit.entities.RefNames.REFS_USERS_SELF;
-import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toCollection;
import com.google.auto.value.AutoValue;
@@ -30,8 +27,6 @@
import com.google.common.collect.Maps;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
@@ -42,13 +37,10 @@
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TagMatcher;
-import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -80,8 +72,8 @@
private final TagCache tagCache;
private final ChangeNotes.Factory changeNotesFactory;
@Nullable private final SearchingChangeCacheImpl changeCache;
- private final GroupCache groupCache;
private final PermissionBackend permissionBackend;
+ private final RefVisibilityControl refVisibilityControl;
private final ProjectControl projectControl;
private final CurrentUser user;
private final ProjectState projectState;
@@ -97,16 +89,16 @@
TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
@Nullable SearchingChangeCacheImpl changeCache,
- GroupCache groupCache,
PermissionBackend permissionBackend,
+ RefVisibilityControl refVisibilityControl,
@GerritServerConfig Config config,
MetricMaker metricMaker,
@Assisted ProjectControl projectControl) {
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache;
- this.groupCache = groupCache;
this.permissionBackend = permissionBackend;
+ this.refVisibilityControl = refVisibilityControl;
this.skipFullRefEvaluationIfAllRefsAreVisible =
config.getBoolean("auth", "skipFullRefEvaluationIfAllRefsAreVisible", true);
this.projectControl = projectControl;
@@ -227,131 +219,56 @@
logger.atFinest().log("Doing full ref filtering");
fullFilterCount.increment();
- boolean viewMetadata;
- boolean isAdmin;
- Account.Id userId;
- IdentifiedUser identifiedUser;
- PermissionBackend.WithUser withUser = permissionBackend.user(user);
- if (user.isIdentifiedUser()) {
- viewMetadata = withUser.testOrFalse(GlobalPermission.ACCESS_DATABASE);
- isAdmin = withUser.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER);
- identifiedUser = user.asIdentifiedUser();
- userId = identifiedUser.getAccountId();
- logger.atFinest().log(
- "Account = %d; can view metadata = %s; is admin = %s",
- userId.get(), viewMetadata, isAdmin);
- } else {
- logger.atFinest().log("User is anonymous");
- viewMetadata = false;
- isAdmin = false;
- userId = null;
- identifiedUser = null;
- }
-
+ boolean hasAccessDatabase =
+ permissionBackend
+ .user(projectControl.getUser())
+ .testOrFalse(GlobalPermission.ACCESS_DATABASE);
List<Ref> resultRefs = new ArrayList<>(refs.size());
List<Ref> deferredTags = new ArrayList<>();
for (Ref ref : refs) {
- String name = ref.getName();
+ String refName = ref.getName();
Change.Id changeId;
- Account.Id accountId;
- AccountGroup.UUID accountGroupUuid;
- if (name.startsWith(REFS_CACHE_AUTOMERGE)) {
- continue;
- } else if (opts.filterMeta() && isMetadata(name)) {
- logger.atFinest().log("Filter out metadata ref %s", name);
- continue;
- } else if (RefNames.isRefsEdit(name)) {
- // Edits are visible only to the owning user, if change is visible.
- if (viewMetadata || visibleEdit(repo, name)) {
- logger.atFinest().log("Include edit ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out edit ref %s", name);
- }
- } else if ((changeId = Change.Id.fromRef(name)) != null) {
- // Change ref is visible only if the change is visible.
- if (viewMetadata || visible(repo, changeId)) {
- logger.atFinest().log("Include change ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out change ref %s", name);
- }
- } else if ((accountId = Account.Id.fromRef(name)) != null) {
- // Account ref is visible only to the corresponding account.
- if (viewMetadata || (accountId.equals(userId) && canReadRef(name))) {
- logger.atFinest().log("Include user ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out user ref %s", name);
- }
- } else if ((accountGroupUuid = AccountGroup.UUID.fromRef(name)) != null) {
- // Group ref is visible only to the corresponding owner group.
- InternalGroup group = groupCache.get(accountGroupUuid).orElse(null);
- if (viewMetadata
- || (group != null
- && isGroupOwner(group, identifiedUser, isAdmin)
- && canReadRef(name))) {
- logger.atFinest().log("Include group ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out group ref %s", name);
- }
+ if (opts.filterMeta() && isMetadata(refName)) {
+ logger.atFinest().log("Filter out metadata ref %s", refName);
} else if (isTag(ref)) {
if (hasReadOnRefsStar) {
- // The user has READ on refs/*. This is the broadest permission one can assign. There is
- // no way to grant access to (specific) tags in Gerrit, so we have to assume that these
- // users can see all tags because there could be tags that aren't reachable by any visible
- // ref while the user can see all non-Gerrit refs. This matches Gerrit's historic
- // behavior.
+ // The user has READ on refs/* with no effective block permission. This is the broadest
+ // permission one can assign. There is no way to grant access to (specific) tags in
+ // Gerrit,
+ // so we have to assume that these users can see all tags because there could be tags that
+ // aren't reachable by any visible ref while the user can see all non-Gerrit refs. This
+ // matches Gerrit's historic behavior.
// This makes it so that these users could see commits that they can't see otherwise
// (e.g. a private change ref) if a tag was attached to it. Tags are meant to be used on
// the regular Git tree that users interact with, not on any of the Gerrit trees, so this
// is a negligible risk.
- logger.atFinest().log("Include tag ref %s because user has read on refs/*", name);
+ logger.atFinest().log("Include tag ref %s because user has read on refs/*", refName);
resultRefs.add(ref);
} else {
// If its a tag, consider it later.
if (ref.getObjectId() != null) {
- logger.atFinest().log("Defer tag ref %s", name);
+ logger.atFinest().log("Defer tag ref %s", refName);
deferredTags.add(ref);
} else {
- logger.atFinest().log("Filter out tag ref %s that is not a tag", name);
+ logger.atFinest().log("Filter out tag ref %s that is not a tag", refName);
}
}
- } else if (name.startsWith(RefNames.REFS_SEQUENCES)) {
- // Sequences are internal database implementation details.
- if (viewMetadata) {
- logger.atFinest().log("Include sequence ref %s", name);
+ } else if ((changeId = Change.Id.fromRef(refName)) != null) {
+ // This is a mere performance optimization. RefVisibilityControl could determine the
+ // visibility of these refs just fine. But instead, we use highly-optimized logic that
+ // looks only on the last 10k most recent changes using the change index and a cache.
+ if (hasAccessDatabase) {
resultRefs.add(ref);
+ } else if (!visible(repo, changeId)) {
+ logger.atFinest().log("Filter out invisible change ref %s", refName);
+ } else if (RefNames.isRefsEdit(refName) && !visibleEdit(repo, refName)) {
+ logger.atFinest().log("Filter out invisible change edit ref %s", refName);
} else {
- logger.atFinest().log("Filter out sequence ref %s", name);
- }
- } else if (projectState.isAllUsers()
- && (name.equals(RefNames.REFS_EXTERNAL_IDS) || name.equals(RefNames.REFS_GROUPNAMES))) {
- // The notes branches with the external IDs / group names must not be exposed to normal
- // users.
- if (viewMetadata) {
- logger.atFinest().log("Include external IDs branch %s", name);
+ // Change is visible
resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out external IDs branch %s", name);
}
- } else if (canReadRef(ref.getLeaf().getName())) {
- // Use the leaf to lookup the control data. If the reference is
- // symbolic we want the control around the final target. If its
- // not symbolic then getLeaf() is a no-op returning ref itself.
- logger.atFinest().log(
- "Include ref %s because its leaf %s is readable", name, ref.getLeaf().getName());
+ } else if (refVisibilityControl.isVisible(projectControl, ref.getLeaf().getName())) {
resultRefs.add(ref);
- } else if (isRefsUsersSelf(ref)) {
- // viewMetadata allows to see all account refs, hence refs/users/self should be included as
- // well
- if (viewMetadata) {
- logger.atFinest().log("Include ref %s", REFS_USERS_SELF);
- resultRefs.add(ref);
- }
- } else {
- logger.atFinest().log("Filter out ref %s", name);
}
}
Result result = new AutoValue_DefaultRefFilter_Result(resultRefs, deferredTags);
@@ -374,7 +291,8 @@
r ->
!RefNames.isGerritRef(r.getName())
&& !r.getName().startsWith(RefNames.REFS_TAGS)
- && !r.isSymbolic())
+ && !r.isSymbolic()
+ && !r.getName().equals(RefNames.REFS_CONFIG))
// Don't use the default Java Collections.toList() as that is not size-aware and would
// expand an array list as new elements are added. Instead, provide a list that has the
// right size. This spares incremental list expansion which is quadratic in complexity.
@@ -520,10 +438,6 @@
return ref.getLeaf().getName().startsWith(Constants.R_TAGS);
}
- private static boolean isRefsUsersSelf(Ref ref) {
- return ref.getName().startsWith(REFS_USERS_SELF);
- }
-
private boolean canReadRef(String ref) throws PermissionBackendException {
try {
permissionBackendForProject.ref(ref).check(RefPermission.READ);
@@ -544,17 +458,6 @@
return true;
}
- private boolean isGroupOwner(
- InternalGroup group, @Nullable IdentifiedUser user, boolean isAdmin) {
- requireNonNull(group);
-
- // Keep this logic in sync with GroupControl#isOwner().
- boolean isGroupOwner =
- isAdmin || (user != null && user.getEffectiveGroups().contains(group.getOwnerGroupUUID()));
- logger.atFinest().log("User is owner of group %s = %s", group.getGroupUUID(), isGroupOwner);
- return isGroupOwner;
- }
-
/**
* Returns true if the user can see the provided change ref. Uses NoteDb for evaluation, hence
* does not suffer from the limitations documented in {@link SearchingChangeCacheImpl}.
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index 724017db..a92fde0 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -36,8 +36,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
@@ -68,11 +70,14 @@
private final Set<AccountGroup.UUID> uploadGroups;
private final Set<AccountGroup.UUID> receiveGroups;
private final PermissionBackend permissionBackend;
+ private final RefVisibilityControl refVisibilityControl;
+ private final GitRepositoryManager repositoryManager;
private final CurrentUser user;
private final ProjectState state;
private final PermissionCollection.Factory permissionFilter;
private final DefaultRefFilter.Factory refFilterFactory;
private final ChangeData.Factory changeDataFactory;
+ private final AllUsersName allUsersName;
private List<SectionMatcher> allSections;
private Map<String, RefControl> refControls;
@@ -84,16 +89,22 @@
@GitReceivePackGroups Set<AccountGroup.UUID> receiveGroups,
PermissionCollection.Factory permissionFilter,
PermissionBackend permissionBackend,
+ RefVisibilityControl refVisibilityControl,
+ GitRepositoryManager repositoryManager,
DefaultRefFilter.Factory refFilterFactory,
ChangeData.Factory changeDataFactory,
+ AllUsersName allUsersName,
@Assisted CurrentUser who,
@Assisted ProjectState ps) {
this.uploadGroups = uploadGroups;
this.receiveGroups = receiveGroups;
this.permissionFilter = permissionFilter;
this.permissionBackend = permissionBackend;
+ this.refVisibilityControl = refVisibilityControl;
+ this.repositoryManager = repositoryManager;
this.refFilterFactory = refFilterFactory;
this.changeDataFactory = changeDataFactory;
+ this.allUsersName = allUsersName;
user = who;
state = ps;
}
@@ -117,7 +128,9 @@
RefControl ctl = refControls.get(refName);
if (ctl == null) {
PermissionCollection relevant = permissionFilter.filter(access(), refName, user);
- ctl = new RefControl(changeDataFactory, this, refName, relevant);
+ ctl =
+ new RefControl(
+ changeDataFactory, refVisibilityControl, this, repositoryManager, refName, relevant);
refControls.put(refName, ctl);
}
return ctl;
@@ -164,7 +177,9 @@
}
boolean allRefsAreVisible(Set<String> ignore) {
- return user.isInternalUser() || canPerformOnAllRefs(Permission.READ, ignore);
+ return user.isInternalUser()
+ || (!getProject().getNameKey().equals(allUsersName)
+ && canPerformOnAllRefs(Permission.READ, ignore));
}
/** Can the user run upload pack? */
@@ -442,7 +457,7 @@
return canPushToAtLeastOneRef();
case READ_CONFIG:
- return controlForRef(RefNames.REFS_CONFIG).isVisible();
+ return controlForRef(RefNames.REFS_CONFIG).hasReadPermissionOnRef(false);
case BAN_COMMIT:
case READ_REFLOG:
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index e704a99..ad4188f 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkArgument;
+import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Permission;
@@ -28,6 +29,7 @@
import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.logging.LoggingContext;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -35,17 +37,24 @@
import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.MagicBranch;
+import java.io.IOException;
import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
/** Manages access control for Git references (aka branches, tags). */
class RefControl {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangeData.Factory changeDataFactory;
+ private final RefVisibilityControl refVisibilityControl;
private final ProjectControl projectControl;
+ private final GitRepositoryManager repositoryManager;
private final String refName;
/** All permissions that apply to this reference. */
@@ -58,15 +67,19 @@
private Boolean owner;
private Boolean canForgeAuthor;
private Boolean canForgeCommitter;
- private Boolean isVisible;
+ private Boolean hasReadPermissionOnRef;
RefControl(
ChangeData.Factory changeDataFactory,
+ RefVisibilityControl refVisibilityControl,
ProjectControl projectControl,
+ GitRepositoryManager repositoryManager,
String ref,
PermissionCollection relevant) {
this.changeDataFactory = changeDataFactory;
+ this.refVisibilityControl = refVisibilityControl;
this.projectControl = projectControl;
+ this.repositoryManager = repositoryManager;
this.refName = ref;
this.relevant = relevant;
this.callerFinder =
@@ -99,12 +112,27 @@
return owner;
}
- /** Can this user see this reference exists? */
- boolean isVisible() {
- if (isVisible == null) {
- isVisible = getUser().isInternalUser() || canPerform(Permission.READ);
+ /**
+ * Returns {@code true} if the user has permission to read the ref. This method evaluates {@link
+ * RefPermission#READ} only. Hence, it is not authoritative. For example, it does not tell if the
+ * user can see NoteDb refs such as {@code refs/meta/external-ids} which requires {@link
+ * GlobalPermission#ACCESS_DATABASE} and deny access in this case.
+ */
+ boolean hasReadPermissionOnRef(boolean allowNoteDbRefs) {
+ // Don't allow checking for NoteDb refs unless instructed otherwise.
+ if (!allowNoteDbRefs
+ && (refName.startsWith(Constants.R_TAGS) || RefNames.isGerritRef(refName))) {
+ logger.atWarning().atMostEvery(30, TimeUnit.SECONDS).log(
+ "%s: Can't determine visibility of %s in RefControl. Denying access. "
+ + "This case should have been handled before.",
+ projectControl.getProject().getName(), refName);
+ return false;
}
- return isVisible;
+
+ if (hasReadPermissionOnRef == null) {
+ hasReadPermissionOnRef = getUser().isInternalUser() || canPerform(Permission.READ);
+ }
+ return hasReadPermissionOnRef;
}
/** @return true if this user can add a new patch set to this ref */
@@ -591,7 +619,10 @@
private boolean can(RefPermission perm) throws PermissionBackendException {
switch (perm) {
case READ:
- return isVisible();
+ if (refName.startsWith(Constants.R_TAGS)) {
+ return isTagVisible();
+ }
+ return refVisibilityControl.isVisible(projectControl, refName);
case CREATE:
// TODO This isn't an accurate test.
return canPerform(refPermissionName(perm));
@@ -641,6 +672,38 @@
}
throw new PermissionBackendException(perm + " unsupported");
}
+
+ private boolean isTagVisible() throws PermissionBackendException {
+ if (projectControl.asForProject().test(ProjectPermission.READ)) {
+ // The user has READ on refs/* with no effective block permission. This is the broadest
+ // permission one can assign. There is no way to grant access to (specific) tags in Gerrit,
+ // so we have to assume that these users can see all tags because there could be tags that
+ // aren't reachable by any visible ref while the user can see all non-Gerrit refs. This
+ // matches Gerrit's historic behavior.
+ // This makes it so that these users could see commits that they can't see otherwise
+ // (e.g. a private change ref) if a tag was attached to it. Tags are meant to be used on
+ // the regular Git tree that users interact with, not on any of the Gerrit trees, so this
+ // is a negligible risk.
+ return true;
+ }
+
+ try (Repository repo =
+ repositoryManager.openRepository(projectControl.getProject().getNameKey())) {
+ // Tag visibility requires going through RefFilter because it entails loading all taggable
+ // refs and filtering them all by visibility.
+ Ref resolvedRef = repo.getRefDatabase().exactRef(refName);
+ if (resolvedRef == null) {
+ return false;
+ }
+ return projectControl.asForProject()
+ .filter(
+ ImmutableList.of(resolvedRef), repo, PermissionBackend.RefFilterOptions.defaults())
+ .stream()
+ .anyMatch(r -> refName.equals(r.getName()));
+ } catch (IOException e) {
+ throw new PermissionBackendException(e);
+ }
+ }
}
private static String refPermissionName(RefPermission refPermission) {
diff --git a/java/com/google/gerrit/server/permissions/RefVisibilityControl.java b/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
new file mode 100644
index 0000000..4744037
--- /dev/null
+++ b/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
@@ -0,0 +1,181 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.permissions;
+
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.entities.RefNames.REFS_CACHE_AUTOMERGE;
+
+import com.google.common.base.Throwables;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.NoSuchGroupException;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.GroupControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.query.change.ChangeData;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.eclipse.jgit.lib.Constants;
+
+/**
+ * This class is a component that is internal to {@link DefaultPermissionBackend}. It can
+ * authoritatively tell if a ref is accessible by a user.
+ */
+@Singleton
+class RefVisibilityControl {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final PermissionBackend permissionBackend;
+ private final GroupControl.GenericFactory groupControlFactory;
+ private final ChangeData.Factory changeDataFactory;
+
+ @Inject
+ RefVisibilityControl(
+ PermissionBackend permissionBackend,
+ GroupControl.GenericFactory groupControlFactory,
+ ChangeData.Factory changeDataFactory) {
+ this.permissionBackend = permissionBackend;
+ this.groupControlFactory = groupControlFactory;
+ this.changeDataFactory = changeDataFactory;
+ }
+
+ /**
+ * Returns an authoritative answer if the ref is visible to the user. Does not have support for
+ * tags and will throw a {@link PermissionBackendException} if asked for tags visibility.
+ */
+ boolean isVisible(ProjectControl projectControl, String refName)
+ throws PermissionBackendException {
+ if (refName.startsWith(Constants.R_TAGS)) {
+ throw new PermissionBackendException(
+ "can't check tags through RefVisibilityControl. Use PermissionBackend#filter instead.");
+ }
+ if (!RefNames.isGerritRef(refName)) {
+ // This is not a special Gerrit ref and not a NoteDb ref. Likely, it's just a ref under
+ // refs/heads or another ref the user created. Apply the regular permissions with inheritance.
+ return projectControl.controlForRef(refName).hasReadPermissionOnRef(false);
+ }
+
+ if (refName.startsWith(REFS_CACHE_AUTOMERGE)) {
+ // Internal cache state that is accessible to no one.
+ return false;
+ }
+
+ boolean hasAccessDatabase =
+ permissionBackend
+ .user(projectControl.getUser())
+ .testOrFalse(GlobalPermission.ACCESS_DATABASE);
+ if (hasAccessDatabase) {
+ return true;
+ }
+
+ // Change and change edit visibility
+ Change.Id changeId;
+ if ((changeId = Change.Id.fromRef(refName)) != null) {
+ // Change ref is visible only if the change is visible.
+ ChangeData cd;
+ try {
+ cd = changeDataFactory.create(projectControl.getProject().getNameKey(), changeId);
+ checkState(cd.change().getId().equals(changeId));
+ } catch (StorageException e) {
+ if (Throwables.getCausalChain(e).stream()
+ .anyMatch(e2 -> e2 instanceof NoSuchChangeException)) {
+ // The change was deleted or is otherwise not accessible anymore.
+ // If the caller can see all refs and is allowed to see private changes on refs/, allow
+ // access. This is an escape hatch for receivers of "ref deleted" events.
+ PermissionBackend.ForProject forProject = projectControl.asForProject();
+ return forProject.test(ProjectPermission.READ)
+ && forProject.ref("refs/").test(RefPermission.READ_PRIVATE_CHANGES);
+ }
+ throw new PermissionBackendException(e);
+ }
+ if (RefNames.isRefsEdit(refName)) {
+ // Edits are visible only to the owning user, if change is visible.
+ return visibleEdit(refName, projectControl, cd);
+ }
+ return projectControl.controlFor(cd).isVisible();
+ }
+
+ // Account visibility
+ CurrentUser user = projectControl.getUser();
+ Account.Id currentUserAccountId = user.isIdentifiedUser() ? user.getAccountId() : null;
+ Account.Id accountId;
+ if ((accountId = Account.Id.fromRef(refName)) != null) {
+ // Account ref is visible only to the corresponding account.
+ if (accountId.equals(currentUserAccountId)
+ && projectControl.controlForRef(refName).hasReadPermissionOnRef(true)) {
+ return true;
+ }
+ return false;
+ }
+
+ // Group visibility
+ AccountGroup.UUID accountGroupUuid;
+ if ((accountGroupUuid = AccountGroup.UUID.fromRef(refName)) != null) {
+ // Group ref is visible only to the corresponding owner group.
+ try {
+ return projectControl.controlForRef(refName).hasReadPermissionOnRef(true)
+ && groupControlFactory.controlFor(user, accountGroupUuid).isOwner();
+ } catch (NoSuchGroupException e) {
+ // The group is broken, but the ref is still around. Pretend the ref is not visible.
+ logger.atWarning().withCause(e).log("Found group ref %s but group isn't parsable", refName);
+ return false;
+ }
+ }
+
+ // We are done checking all cases where we would allow access to Gerrit-managed refs. Deny
+ // access in case we got this far.
+ logger.atFine().log(
+ "Denying access to %s because user doesn't have access to this Gerrit ref", refName);
+ return false;
+ }
+
+ private boolean visibleEdit(String refName, ProjectControl projectControl, ChangeData cd)
+ throws PermissionBackendException {
+ Change.Id id = Change.Id.fromEditRefPart(refName);
+ if (id == null) {
+ throw new IllegalStateException("unable to parse change id from edit ref " + refName);
+ }
+
+ if (!projectControl.controlFor(cd).isVisible()) {
+ // The user can't see the change so they can't see any edits.
+ return false;
+ }
+
+ if (projectControl.getUser().isIdentifiedUser()
+ && refName.startsWith(
+ RefNames.refsEditPrefix(projectControl.getUser().asIdentifiedUser().getAccountId()))) {
+ logger.atFinest().log("Own change edit ref is visible: %s", refName);
+ return true;
+ }
+
+ try {
+ // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
+ projectControl
+ .asForProject()
+ .ref(cd.change().getDest().branch())
+ .check(RefPermission.READ_PRIVATE_CHANGES);
+ logger.atFinest().log("Foreign change edit ref is visible: %s", refName);
+ return true;
+ } catch (AuthException e) {
+ logger.atFinest().log("Foreign change edit ref is not visible: %s", refName);
+ return false;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/util/ssl/BlindSSLSocketFactory.java b/java/com/google/gerrit/util/ssl/BlindSSLSocketFactory.java
index 6dc1006..88845ef 100644
--- a/java/com/google/gerrit/util/ssl/BlindSSLSocketFactory.java
+++ b/java/com/google/gerrit/util/ssl/BlindSSLSocketFactory.java
@@ -20,7 +20,6 @@
import java.net.UnknownHostException;
import java.security.GeneralSecurityException;
import java.security.SecureRandom;
-import java.security.cert.X509Certificate;
import javax.net.SocketFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSocketFactory;
@@ -32,19 +31,7 @@
private static final BlindSSLSocketFactory INSTANCE;
static {
- final X509TrustManager dummyTrustManager =
- new X509TrustManager() {
- @Override
- public X509Certificate[] getAcceptedIssuers() {
- return null;
- }
-
- @Override
- public void checkClientTrusted(X509Certificate[] chain, String authType) {}
-
- @Override
- public void checkServerTrusted(X509Certificate[] chain, String authType) {}
- };
+ final X509TrustManager dummyTrustManager = new BlindTrustManager();
try {
final SSLContext context = SSLContext.getInstance("SSL");
diff --git a/java/com/google/gerrit/util/ssl/BlindTrustManager.java b/java/com/google/gerrit/util/ssl/BlindTrustManager.java
new file mode 100644
index 0000000..2db091a
--- /dev/null
+++ b/java/com/google/gerrit/util/ssl/BlindTrustManager.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2020 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.util.ssl;
+
+import java.security.cert.X509Certificate;
+import javax.net.ssl.X509TrustManager;
+
+/** TrustManager implementation that accepts all certificates without validation. */
+public class BlindTrustManager implements X509TrustManager {
+
+ @Override
+ public X509Certificate[] getAcceptedIssuers() {
+ return null;
+ }
+
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType) {}
+
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType) {}
+}
diff --git a/java/org/apache/commons/net/smtp/AuthSMTPClient.java b/java/org/apache/commons/net/smtp/AuthSMTPClient.java
index 85e4dbf..0f8c1f4 100644
--- a/java/org/apache/commons/net/smtp/AuthSMTPClient.java
+++ b/java/org/apache/commons/net/smtp/AuthSMTPClient.java
@@ -17,68 +17,66 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.io.BaseEncoding;
-import com.google.gerrit.util.ssl.BlindSSLSocketFactory;
-import java.io.BufferedReader;
-import java.io.BufferedWriter;
+import com.google.gerrit.util.ssl.BlindTrustManager;
import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.OutputStreamWriter;
import java.io.UnsupportedEncodingException;
-import java.net.SocketException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.List;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
-import javax.net.ssl.SSLParameters;
-import javax.net.ssl.SSLSocket;
-import javax.net.ssl.SSLSocketFactory;
-public class AuthSMTPClient extends SMTPClient {
+/**
+ * SMTP Client with authentication support and optional SSL processing and verification. {@link
+ * org.apache.commons.net.smtp.SMTPSClient} is used for the SSL handshake and hostname verification.
+ *
+ * <p>If shouldHandshakeOnConnect mode is selected, SSL/TLS negotiation starts right after the
+ * connection has been established. Otherwise SSL/TLS negotiation will only occur if {@link
+ * AuthSMTPClient#execTLS} is explicitly called and the server accepts the command.
+ *
+ * <p>Examples:
+ *
+ * <ul>
+ * <li>For SSL connection:
+ * <pre>
+ * AuthSMTPClient c = new AuthSMTPClient(true, sslVerify);
+ * c.connect("127.0.0.1", 465);
+ * </pre>
+ * <li>For TLS connection:
+ * <pre>
+ * AuthSMTPClient c = new AuthSMTPClient(false, sslVerify);
+ * c.connect("127.0.0.1", 25);
+ * if (c.execTLS()) { /rest of the commands here/ }
+ * </pre>
+ * <li>If SSL encryption is not required:
+ * <pre>
+ * AuthSMTPClient c = new AuthSMTPClient(false, false);
+ * c.connect("127.0.0.1", port);
+ * </pre>
+ */
+public class AuthSMTPClient extends SMTPSClient {
+
private String authTypes;
- public AuthSMTPClient(String charset) {
- super(charset);
- }
-
- public void enableSSL(boolean verify) {
- _socketFactory_ = sslFactory(verify);
- }
-
- public boolean startTLS(String hostname, int port, boolean verify)
- throws SocketException, IOException {
- if (sendCommand("STARTTLS") != 220) {
- return false;
+ /**
+ * Constructs AuthSMTPClient.
+ *
+ * @param shouldHandshakeOnConnect the SSL processing mode, {@code true} if SSL negotiation should
+ * start right after connect, {@code false} if it will be started by the user explicitly or
+ * SSL negotiation is not required.
+ * @param sslVerificationEnabled {@code true} if the SMTP server's SSL certificate and hostname
+ * should be verified, {@code false} otherwise.
+ */
+ public AuthSMTPClient(boolean shouldHandshakeOnConnect, boolean sslVerificationEnabled) {
+ // If SSL Encryption is required, SMTPSClient is used for the handshake.
+ // Otherwise, use SMTPSClient in 'explicit' mode without calling execTLS().
+ // See SMTPSClient._connectAction_ in commons-net-3.6.
+ super("TLS", shouldHandshakeOnConnect, UTF_8.name());
+ this.setEndpointCheckingEnabled(sslVerificationEnabled);
+ if (!sslVerificationEnabled) {
+ this.setTrustManager(new BlindTrustManager());
}
-
- _socket_ = sslFactory(verify).createSocket(_socket_, hostname, port, true);
-
- if (verify) {
- SSLParameters sslParams = new SSLParameters();
- sslParams.setEndpointIdentificationAlgorithm("HTTPS");
- ((SSLSocket) _socket_).setSSLParameters(sslParams);
- }
-
- // XXX: Can't call _connectAction_() because SMTP server doesn't
- // give banner information again after STARTTLS, thus SMTP._connectAction_()
- // will wait on __getReply() forever, see source code of commons-net-2.2.
- //
- // The lines below are copied from SocketClient._connectAction_() and
- // SMTP._connectAction_() in commons-net-2.2.
- _socket_.setSoTimeout(_timeout_);
- _input_ = _socket_.getInputStream();
- _output_ = _socket_.getOutputStream();
- _reader = new BufferedReader(new InputStreamReader(_input_, UTF_8));
- _writer = new BufferedWriter(new OutputStreamWriter(_output_, UTF_8));
- return true;
- }
-
- private static SSLSocketFactory sslFactory(boolean verify) {
- if (verify) {
- return (SSLSocketFactory) SSLSocketFactory.getDefault();
- }
- return (SSLSocketFactory) BlindSSLSocketFactory.getDefault();
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index ccfa60e..f59fba0 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1061,6 +1061,11 @@
com.google.gerrit.acceptance.TestAccount deleteAs)
throws Exception {
try {
+ projectOperations
+ .project(projectName)
+ .forUpdate()
+ .add(allow(Permission.VIEW_PRIVATE_CHANGES).ref("refs/*").group(ANONYMOUS_USERS))
+ .update();
requestScopeOperations.setApiUser(owner.id());
ChangeInput in = new ChangeInput();
in.project = projectName.get();
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 436ad7c..e4bb73a 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -1194,6 +1194,11 @@
@Test
public void pushToDeletedGroupBranchIsRejectedForAllUsersRepo() throws Exception {
+ // refs/deleted-groups is only visible with ACCESS_DATABASE
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
String groupRef =
RefNames.refsDeletedGroups(AccountGroup.uuid(gApi.groups().create(name("foo")).get().id));
createBranch(allUsers, groupRef);
@@ -1386,6 +1391,11 @@
@Test
public void cannotDeleteDeletedGroupBranch() throws Exception {
+ // refs/deleted-groups is only visible with ACCESS_DATABASE
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
String groupRef = RefNames.refsDeletedGroups(AccountGroup.uuid(name("foo")));
createBranch(allUsers, groupRef);
testCannotDeleteGroupBranch(RefNames.REFS_DELETED_GROUPS + "*", groupRef);
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java b/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
index 191d5c5..531357a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
@@ -31,7 +31,9 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
@@ -52,6 +54,13 @@
}
}
+ @ConfigSuite.Config
+ public static Config skipFalse() {
+ Config config = new Config();
+ config.setBoolean("auth", null, "skipFullRefEvaluationIfAllRefsAreVisible", false);
+ return config;
+ }
+
@Inject private ProjectOperations projectOperations;
private RevCommit initialHead;
@@ -93,6 +102,20 @@
}
@Test
+ public void createTagForExistingCommit_withoutGlobalReadPermissions() throws Exception {
+ removeReadAccessOnRefsStar();
+ grantReadAccessOnRefsHeadsStar();
+ createTagForExistingCommit();
+ }
+
+ @Test
+ public void createTagForNewCommit_withoutGlobalReadPermissions() throws Exception {
+ removeReadAccessOnRefsStar();
+ grantReadAccessOnRefsHeadsStar();
+ createTagForNewCommit();
+ }
+
+ @Test
public void fastForward() throws Exception {
allowTagCreation();
String tagName = pushTagForExistingCommit(Status.OK);
@@ -109,6 +132,15 @@
fastForwardTagToExistingCommit(tagName, expectedStatus);
fastForwardTagToNewCommit(tagName, expectedStatus);
+ // Above we just fast-forwarded the tag to a new commit which is not part of any branch. By
+ // default this tag is not visible, as users can only see tags that point to commits that are
+ // part of visible branches, which is not the case for this tag. It's odd that we allow the user
+ // to create such a tag that is then not visible to the creator. Below we want to fast-forward
+ // this tag, but this is only possible if the tag is visible. To make it visible we must allow
+ // the user to read all tags, regardless of whether it points to a commit that is part of a
+ // visible branch.
+ allowReadingAllTag();
+
allowForcePushOnRefsTags();
fastForwardTagToExistingCommit(tagName, Status.OK);
fastForwardTagToNewCommit(tagName, Status.OK);
@@ -234,6 +266,49 @@
assertWithMessage(tagType.name()).that(refUpdate.getStatus()).isEqualTo(expectedStatus);
}
+ private void removeReadAccessOnRefsStar() {
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .remove(permissionKey(Permission.READ).ref("refs/*"))
+ .update();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .remove(permissionKey(Permission.READ).ref("refs/*"))
+ .update();
+ }
+
+ private void grantReadAccessOnRefsHeadsStar() {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/heads/*").group(REGISTERED_USERS))
+ .update();
+ }
+
+ private void allowReadingAllTag() throws Exception {
+ // Tags are only visible if the commits to which they point are part of a visible branch.
+ // To make all tags visible, including tags that point to commits that are not part of a visible
+ // branch, either auth.skipFullRefEvaluationIfAllRefsAreVisible in gerrit.config needs to be
+ // true, or the user must have READ access for all refs in the repository.
+
+ if (cfg.getBoolean("auth", "skipFullRefEvaluationIfAllRefsAreVisible", true)) {
+ return;
+ }
+
+ // By default READ access in the All-Projects project is granted to registered users on refs/*,
+ // which makes all refs, except refs/meta/config, visible to them. refs/meta/config is not
+ // visible since by default READ access to it is exclusively granted to the project owners only.
+ // This means to make all refs, and thus all tags, visible, we must allow registered users to
+ // see the refs/meta/config branch.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/meta/config").group(REGISTERED_USERS))
+ .update();
+ }
+
private void allowTagCreation() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
new file mode 100644
index 0000000..b4b1be0
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
@@ -0,0 +1,594 @@
+// Copyright (C) 2020 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.acceptance.rest.project;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.capabilityKey;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.api.projects.BranchInfo;
+import com.google.gerrit.extensions.api.projects.TagInfo;
+import com.google.gerrit.extensions.api.projects.TagInput;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.testing.ConfigSuite;
+import com.google.inject.Inject;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Test;
+
+public class GetBranchIT extends AbstractDaemonTest {
+ @Inject private ChangeOperations changeOperations;
+ @Inject private GroupOperations groupOperations;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private RequestScopeOperations requestScopeOperations;
+
+ @ConfigSuite.Config
+ public static Config skipFalse() {
+ Config config = new Config();
+ config.setBoolean("auth", null, "skipFullRefEvaluationIfAllRefsAreVisible", false);
+ return config;
+ }
+
+ @Test
+ public void cannotGetNonExistingBranch() {
+ assertBranchNotFound(project, RefNames.fullName("non-existing"));
+ }
+
+ @Test
+ public void getBranch() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, RefNames.fullName("master"));
+ }
+
+ @Test
+ public void getBranchByShortName() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, "master");
+ }
+
+ @Test
+ public void cannotGetNonVisibleBranch() {
+ String branchName = "master";
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, RefNames.fullName(branchName));
+ }
+
+ @Test
+ public void cannotGetNonVisibleBranchByShortName() {
+ String branchName = "master";
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, branchName);
+ }
+
+ @Test
+ public void getChangeRef() throws Exception {
+ // create a change
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+
+ // a user without the 'Access Database' capability can see the change ref
+ requestScopeOperations.setApiUser(user.id());
+ String changeRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+ assertBranchFound(project, changeRef);
+ }
+
+ @Test
+ public void getChangeRefOfNonVisibleChange() throws Exception {
+ // create a change
+ String branchName = "master";
+ Change.Id changeId = changeOperations.newChange().project(project).branch(branchName).create();
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ // a user without the 'Access Database' capability cannot see the change ref
+ requestScopeOperations.setApiUser(user.id());
+ String changeRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+ assertBranchNotFound(project, changeRef);
+
+ // a user with the 'Access Database' capability can see the change ref
+ testGetRefWithAccessDatabase(project, changeRef);
+ }
+
+ @Test
+ public void getChangeEditRef() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create a change
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+
+ // create a change edit by 'user'
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(changeId.get()).edit().create();
+
+ // every user can see their own change edit refs
+ String changeEditRef = RefNames.refsEdit(user.id(), changeId, PatchSet.id(changeId, 1));
+ assertBranchFound(project, changeEditRef);
+
+ // a user without the 'Access Database' capability cannot see the change edit ref of another
+ // user
+ requestScopeOperations.setApiUser(user2.id());
+ assertBranchNotFound(project, changeEditRef);
+
+ // a user with the 'Access Database' capability can see the change edit ref of another user
+ testGetRefWithAccessDatabase(project, changeEditRef);
+ }
+
+ @Test
+ public void cannotGetChangeEditRefOfNonVisibleChange() throws Exception {
+ // create a change
+ String branchName = "master";
+ Change.Id changeId = changeOperations.newChange().project(project).branch(branchName).create();
+
+ // create a change edit by 'user'
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(changeId.get()).edit().create();
+
+ // make the change non-visible by blocking read access on the destination
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ // user cannot see their own change edit refs if the change is no longer visible
+ String changeEditRef = RefNames.refsEdit(user.id(), changeId, PatchSet.id(changeId, 1));
+ assertBranchNotFound(project, changeEditRef);
+
+ // a user with the 'Access Database' capability can see the change edit ref
+ testGetRefWithAccessDatabase(project, changeEditRef);
+ }
+
+ @Test
+ public void getChangeMetaRef() throws Exception {
+ // create a change
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+
+ // A user without the 'Access Database' capability can see the change meta ref.
+ // This may be surprising, as 'Access Database' guards access to meta refs and the change meta
+ // ref is a meta ref, however change meta refs have been always visible to all users that can
+ // see the change and some tools rely on seeing these refs, so we have to keep the current
+ // behaviour.
+ requestScopeOperations.setApiUser(user.id());
+ String changeMetaRef = RefNames.changeMetaRef(changeId);
+ assertBranchFound(project, changeMetaRef);
+ }
+
+ @Test
+ public void getRefsMetaConfig() throws Exception {
+ // a non-project owner cannot get the refs/meta/config branch
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, RefNames.REFS_CONFIG);
+
+ // a non-project owner cannot get the refs/meta/config branch even with the 'Access Database'
+ // capability
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
+ try {
+ assertBranchNotFound(project, RefNames.REFS_CONFIG);
+ } finally {
+ projectOperations
+ .allProjectsForUpdate()
+ .remove(
+ capabilityKey(GlobalCapability.ACCESS_DATABASE)
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+ }
+
+ requestScopeOperations.setApiUser(user.id());
+
+ // a project owner can get the refs/meta/config branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.OWNER).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ assertBranchFound(project, RefNames.REFS_CONFIG);
+ }
+
+ @Test
+ public void getUserRefOfOtherUser() throws Exception {
+ String userRef = RefNames.refsUsers(admin.id());
+
+ // a user without the 'Access Database' capability cannot see the user ref of another user
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, userRef);
+
+ // a user with the 'Access Database' capability can see the user ref of another user
+ testGetRefWithAccessDatabase(allUsers, userRef);
+ }
+
+ @Test
+ public void getOwnUserRef() throws Exception {
+ // every user can see the own user ref
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(allUsers, RefNames.refsUsers(user.id()));
+
+ // TODO: every user can see the own user ref via the magic ref/users/self ref
+ // requestScopeOperations.setApiUser(user.id());
+ // assertBranchFound(allUsers, RefNames.REFS_USERS_SELF);
+ }
+
+ @Test
+ public void getExternalIdsRefs() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/meta/external-ids ref
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, RefNames.REFS_EXTERNAL_IDS);
+
+ // a user with the 'Access Database' capability can see the refs/meta/external-ids ref
+ testGetRefWithAccessDatabase(allUsers, RefNames.REFS_EXTERNAL_IDS);
+ }
+
+ @Test
+ public void getGroupRef() throws Exception {
+ // create a group
+ AccountGroup.UUID ownerGroupUuid =
+ groupOperations.newGroup().name("owner-group").addMember(admin.id()).create();
+ AccountGroup.UUID testGroupUuid =
+ groupOperations.newGroup().name("test-group").ownerGroupUuid(ownerGroupUuid).create();
+
+ // a non-group owner without the 'Access Database' capability cannot see the group ref
+ requestScopeOperations.setApiUser(user.id());
+ String groupRef = RefNames.refsGroups(testGroupUuid);
+ assertBranchNotFound(allUsers, groupRef);
+
+ // a non-group owner with the 'Access Database' capability can see the group ref
+ testGetRefWithAccessDatabase(allUsers, groupRef);
+
+ // a group owner can see the group ref if the group ref is visible
+ groupOperations.group(ownerGroupUuid).forUpdate().addMember(user.id()).update();
+ assertBranchFound(allUsers, groupRef);
+
+ // A group owner cannot see the group ref if the group ref is not visible.
+ // The READ access for refs/groups/* must be blocked on All-Projects rather than All-Users.
+ // This is because READ access for refs/groups/* on All-Users is by default granted to
+ // REGISTERED_USERS, and if an ALLOW rule and a BLOCK rule are on the same project and ref,
+ // the ALLOW rule takes precedence.
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/groups/*").group(ANONYMOUS_USERS))
+ .update();
+ assertBranchNotFound(allUsers, groupRef);
+ }
+
+ @Test
+ public void getGroupNamesRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/meta/group-names ref
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, RefNames.REFS_GROUPNAMES);
+
+ // a user with the 'Access Database' capability can see the refs/meta/group-names ref
+ testGetRefWithAccessDatabase(allUsers, RefNames.REFS_GROUPNAMES);
+ }
+
+ @Test
+ public void getDeletedGroupRef() throws Exception {
+ // Create a deleted group ref. We must create a directly in the repo, since group deletion is
+ // not supported yet.
+ String deletedGroupRef = RefNames.refsDeletedGroups(AccountGroup.uuid("deleted-group"));
+ try (TestRepository<Repository> testRepo =
+ new TestRepository<>(repoManager.openRepository(allUsers))) {
+ testRepo
+ .branch(deletedGroupRef)
+ .commit()
+ .message("Some Message")
+ .add("group.config", "content")
+ .create();
+ }
+
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, deletedGroupRef);
+
+ // a user with the 'Access Database' capability can see the deleted group ref
+ testGetRefWithAccessDatabase(allUsers, deletedGroupRef);
+ }
+
+ @Test
+ public void getDraftCommentsRef() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create a change
+ String fileName = "a.txt";
+ Change change = createChange("A Change", fileName, "content").getChange().change();
+
+ // create a draft comment by the by 'user'
+ requestScopeOperations.setApiUser(user.id());
+ DraftInput draftInput = new DraftInput();
+ draftInput.path = fileName;
+ draftInput.line = 0;
+ draftInput.message = "Some Comment";
+ gApi.changes().id(change.getChangeId()).current().createDraft(draftInput);
+
+ // every user can see their own draft comments refs
+ // TODO: is this a bug?
+ String draftCommentsRef = RefNames.refsDraftComments(change.getId(), user.id());
+ assertBranchFound(allUsers, draftCommentsRef);
+
+ // a user without the 'Access Database' capability cannot see the draft comments ref of another
+ // user
+ requestScopeOperations.setApiUser(user2.id());
+ assertBranchNotFound(allUsers, draftCommentsRef);
+
+ // a user with the 'Access Database' capability can see the draft comments ref of another user
+ testGetRefWithAccessDatabase(allUsers, draftCommentsRef);
+ }
+
+ @Test
+ public void getStarredChangesRef() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create a change
+ Change change = createChange().getChange().change();
+
+ // let user star the change
+ requestScopeOperations.setApiUser(user.id());
+ gApi.accounts().self().starChange(Integer.toString(change.getChangeId()));
+
+ // every user can see their own starred changes refs
+ // TODO: is this a bug?
+ String starredChangesRef = RefNames.refsStarredChanges(change.getId(), user.id());
+ assertBranchFound(allUsers, starredChangesRef);
+
+ // a user without the 'Access Database' capability cannot see the starred changes ref of another
+ // user
+ requestScopeOperations.setApiUser(user2.id());
+ assertBranchNotFound(allUsers, starredChangesRef);
+
+ // a user with the 'Access Database' capability can see the starred changes ref of another user
+ testGetRefWithAccessDatabase(allUsers, starredChangesRef);
+ }
+
+ @Test
+ public void getTagRef() throws Exception {
+ // create a tag
+ TagInput input = new TagInput();
+ input.message = "My Tag";
+ input.revision = projectOperations.project(project).getHead("master").name();
+ TagInfo tagInfo = gApi.projects().name(project.get()).tag("my-tag").create(input).get();
+
+ // any user who can see the project, can see the tag
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, tagInfo.ref);
+ }
+
+ @Test
+ public void cannotGetTagRefThatPointsToNonVisibleBranch() throws Exception {
+ // create a tag
+ TagInput input = new TagInput();
+ input.message = "My Tag";
+ input.revision = projectOperations.project(project).getHead("master").name();
+ TagInfo tagInfo = gApi.projects().name(project.get()).tag("my-tag").create(input).get();
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName("master")).group(ANONYMOUS_USERS))
+ .update();
+
+ // if the user cannot see the project, the tag is not visible
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, tagInfo.ref);
+ }
+
+ @Test
+ public void getSymbolicRef() throws Exception {
+ // 'HEAD' is visible since it points to 'master' that is visible
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, "HEAD");
+ }
+
+ @Test
+ public void cannotGetSymbolicRefThatPointsToNonVisibleBranch() {
+ // block read access to the branch to which HEAD points by default
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName("master")).group(ANONYMOUS_USERS))
+ .update();
+
+ // since 'master' is not visible, 'HEAD' which points to 'master' is also not visible
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, "HEAD");
+ }
+
+ @Test
+ public void getAccountSequenceRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/sequences/accounts ref
+ requestScopeOperations.setApiUser(user.id());
+ String accountSequenceRef = RefNames.REFS_SEQUENCES + Sequences.NAME_ACCOUNTS;
+ assertBranchNotFound(allUsers, accountSequenceRef);
+
+ // a user with the 'Access Database' capability can see the refs/sequences/accounts ref
+ testGetRefWithAccessDatabase(allUsers, accountSequenceRef);
+ }
+
+ @Test
+ public void getChangeSequenceRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/sequences/changes ref
+ requestScopeOperations.setApiUser(user.id());
+ String changeSequenceRef = RefNames.REFS_SEQUENCES + Sequences.NAME_CHANGES;
+ assertBranchNotFound(allProjects, changeSequenceRef);
+
+ // a user with the 'Access Database' capability can see the refs/sequences/changes ref
+ testGetRefWithAccessDatabase(allProjects, changeSequenceRef);
+ }
+
+ @Test
+ public void getGroupSequenceRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/sequences/groups ref
+ requestScopeOperations.setApiUser(user.id());
+ String groupSequenceRef = RefNames.REFS_SEQUENCES + Sequences.NAME_GROUPS;
+ assertBranchNotFound(allUsers, groupSequenceRef);
+
+ // a user with the 'Access Database' capability can see the refs/sequences/groups ref
+ testGetRefWithAccessDatabase(allUsers, groupSequenceRef);
+ }
+
+ @Test
+ public void getVersionMetaRef() throws Exception {
+ // TODO: a user without the 'Access Database' capability cannot see the refs/meta/version ref
+ // requestScopeOperations.setApiUser(user.id());
+ // assertBranchNotFound(allProjects, RefNames.REFS_VERSION);
+
+ // a user with the 'Access Database' capability can see the refs/meta/vaersion ref
+ testGetRefWithAccessDatabase(allProjects, RefNames.REFS_VERSION);
+ }
+
+ @Test
+ public void cannotGetAutoMergeRef() throws Exception {
+ String file = "foo/a.txt";
+
+ // Create a base change.
+ Change.Id baseChange =
+ changeOperations
+ .newChange()
+ .project(project)
+ .branch("master")
+ .file(file)
+ .content("base content")
+ .create();
+ approve(Integer.toString(baseChange.get()));
+ gApi.changes().id(baseChange.get()).current().submit();
+
+ // Create another branch
+ String branchName = "foo";
+ createBranchWithRevision(
+ BranchNameKey.create(project, branchName),
+ projectOperations.project(project).getHead("master").name());
+
+ // Create a change in master that touches the file.
+ Change.Id changeInMaster =
+ changeOperations
+ .newChange()
+ .project(project)
+ .branch("master")
+ .file(file)
+ .content("master content")
+ .create();
+ approve(Integer.toString(changeInMaster.get()));
+ gApi.changes().id(changeInMaster.get()).current().submit();
+
+ // Create a change in the other branch and that touches the file.
+ Change.Id changeInOtherBranch =
+ changeOperations
+ .newChange()
+ .project(project)
+ .branch(branchName)
+ .file(file)
+ .content("other content")
+ .create();
+ approve(Integer.toString(changeInOtherBranch.get()));
+ gApi.changes().id(changeInOtherBranch.get()).current().submit();
+
+ // Create a merge change with a conflict resolution for the file.
+ Change.Id mergeChange =
+ changeOperations
+ .newChange()
+ .project(project)
+ .branch("master")
+ .mergeOfButBaseOnFirst()
+ .tipOfBranch("master")
+ .and()
+ .tipOfBranch(branchName)
+ .file(file)
+ .content("merged content")
+ .create();
+
+ String mergeRevision =
+ changeOperations.change(mergeChange).currentPatchset().get().commitId().name();
+ assertBranchNotFound(project, RefNames.refsCacheAutomerge(mergeRevision));
+ }
+
+ private void testGetRefWithAccessDatabase(Project.NameKey project, String ref)
+ throws RestApiException {
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
+ try {
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, ref);
+ } finally {
+ projectOperations
+ .allProjectsForUpdate()
+ .remove(
+ capabilityKey(GlobalCapability.ACCESS_DATABASE)
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+ }
+ }
+
+ private void assertBranchNotFound(Project.NameKey project, String ref) {
+ ResourceNotFoundException exception =
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> gApi.projects().name(project.get()).branch(ref).get());
+ assertThat(exception).hasMessageThat().isEqualTo("Not found: " + ref);
+ }
+
+ private void assertBranchFound(Project.NameKey project, String ref) throws RestApiException {
+ BranchInfo branchInfo = gApi.projects().name(project.get()).branch(ref).get();
+ assertThat(branchInfo.ref).isEqualTo(RefNames.fullName(ref));
+ }
+}
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index ab2bb12..e269fc2 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -17,8 +17,11 @@
"//lib:junit",
"//lib/guice",
"//lib/httpcomponents:httpcore",
+ "//lib/jackson:jackson-annotations",
"//lib/log:api",
"//lib/testcontainers",
+ "//lib/testcontainers:docker-java-api",
+ "//lib/testcontainers:docker-java-transport",
"//lib/testcontainers:testcontainers-elasticsearch",
],
)
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 86829b9..48295ea 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -19,6 +19,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testcontainers.elasticsearch.ElasticsearchContainer;
+import org.testcontainers.utility.DockerImageName;
/* Helper class for running ES integration tests in docker container */
public class ElasticContainer extends ElasticsearchContainer {
@@ -39,7 +40,7 @@
private static String getImageName(ElasticVersion version) {
switch (version) {
case V6_8:
- return "blacktop/elasticsearch:6.8.12";
+ return "blacktop/elasticsearch:6.8.13";
case V7_0:
return "blacktop/elasticsearch:7.0.1";
case V7_1:
@@ -63,7 +64,9 @@
}
private ElasticContainer(ElasticVersion version) {
- super(getImageName(version));
+ super(
+ DockerImageName.parse(getImageName(version))
+ .asCompatibleSubstituteFor("docker.elastic.co/elasticsearch/elasticsearch"));
}
@Override
diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
index 81cb732..65196bf 100644
--- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
@@ -48,6 +48,7 @@
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.index.SingleVersionModule.SingleVersionListener;
import com.google.gerrit.server.project.ProjectCache;
@@ -63,6 +64,7 @@
import com.google.inject.Inject;
import com.google.inject.Injector;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Optional;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Repository;
@@ -90,6 +92,18 @@
assertWithMessage("not owner").that(u.isOwner()).isFalse();
}
+ private void assertAllRefsAreVisible(ProjectControl u) {
+ assertWithMessage("all refs visible")
+ .that(u.allRefsAreVisible(Collections.emptySet()))
+ .isTrue();
+ }
+
+ private void assertAllRefsAreNotVisible(ProjectControl u) {
+ assertWithMessage("all refs NOT visible")
+ .that(u.allRefsAreVisible(Collections.emptySet()))
+ .isFalse();
+ }
+
private void assertNotOwner(String ref, ProjectControl u) {
assertWithMessage("NOT OWN " + ref).that(u.controlForRef(ref).isOwner()).isFalse();
}
@@ -105,11 +119,21 @@
}
private void assertCanRead(String ref, ProjectControl u) {
- assertWithMessage("can read " + ref).that(u.controlForRef(ref).isVisible()).isTrue();
+ assertWithMessage("can read " + ref)
+ .that(
+ u.controlForRef(ref)
+ .hasReadPermissionOnRef(
+ true)) // This should be false but the test relies on inheritance into refs/tags
+ .isTrue();
}
private void assertCannotRead(String ref, ProjectControl u) {
- assertWithMessage("cannot read " + ref).that(u.controlForRef(ref).isVisible()).isFalse();
+ assertWithMessage("cannot read " + ref)
+ .that(
+ u.controlForRef(ref)
+ .hasReadPermissionOnRef(
+ true)) // This should be false but the test relies on inheritance into refs/tags
+ .isFalse();
}
private void assertCanSubmit(String ref, ProjectControl u) {
@@ -171,6 +195,7 @@
private final Project.NameKey parentKey = Project.nameKey("parent");
@Inject private AllProjectsName allProjectsName;
+ @Inject private AllUsersName allUsersName;
@Inject private InMemoryRepositoryManager repoManager;
@Inject private MetaDataUpdate.Server metaDataUpdateFactory;
@Inject private ProjectCache projectCache;
@@ -262,6 +287,32 @@
}
@Test
+ public void allRefsAreVisibleForRegularProject() throws Exception {
+ projectOperations
+ .project(localKey)
+ .forUpdate()
+ .add(allow(READ).ref("refs/*").group(DEVS))
+ .add(allow(READ).ref("refs/groups/*").group(DEVS))
+ .add(allow(READ).ref("refs/users/default").group(DEVS))
+ .update();
+
+ assertAllRefsAreVisible(user(localKey, DEVS));
+ }
+
+ @Test
+ public void allRefsAreNotVisibleForAllUsers() throws Exception {
+ projectOperations
+ .project(allUsersName)
+ .forUpdate()
+ .add(allow(READ).ref("refs/*").group(DEVS))
+ .add(allow(READ).ref("refs/groups/*").group(DEVS))
+ .add(allow(READ).ref("refs/users/default").group(DEVS))
+ .update();
+
+ assertAllRefsAreNotVisible(user(allUsersName, DEVS));
+ }
+
+ @Test
public void branchDelegation1() throws Exception {
projectOperations
.project(localKey)
diff --git a/lib/jackson/BUILD b/lib/jackson/BUILD
index d5253a0..f11b96d 100644
--- a/lib/jackson/BUILD
+++ b/lib/jackson/BUILD
@@ -1,6 +1,14 @@
load("@rules_java//java:defs.bzl", "java_library")
java_library(
+ name = "jackson-annotations",
+ testonly = True,
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = ["//visibility:public"],
+ exports = ["@jackson-annotations//jar"],
+)
+
+java_library(
name = "jackson-core",
data = ["//lib:LICENSE-Apache2.0"],
visibility = [
diff --git a/lib/nongoogle_test.sh b/lib/nongoogle_test.sh
index 0cdad1a..8369024 100755
--- a/lib/nongoogle_test.sh
+++ b/lib/nongoogle_test.sh
@@ -12,6 +12,8 @@
cat << EOF > $TMP/want
cglib-3_2
+docker-java-api
+docker-java-transport
dropwizard-core
duct-tape
eddsa
@@ -22,6 +24,7 @@
httpasyncclient
httpcore-nio
j2objc
+jackson-annotations
jackson-core
jna
jruby
diff --git a/lib/testcontainers/BUILD b/lib/testcontainers/BUILD
index a37b733..693a386 100644
--- a/lib/testcontainers/BUILD
+++ b/lib/testcontainers/BUILD
@@ -1,6 +1,22 @@
load("@rules_java//java:defs.bzl", "java_library")
java_library(
+ name = "docker-java-api",
+ testonly = True,
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = ["//visibility:public"],
+ exports = ["@docker-java-api//jar"],
+)
+
+java_library(
+ name = "docker-java-transport",
+ testonly = True,
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = ["//visibility:public"],
+ exports = ["@docker-java-transport//jar"],
+)
+
+java_library(
name = "duct-tape",
testonly = True,
data = ["//lib:LICENSE-testcontainers"],
diff --git a/modules/jgit b/modules/jgit
index dd16976..e2663a8 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit dd169769bf42115e1dee749efeecab84544b28c4
+Subproject commit e2663a8b85cf92f6a84d72834257243a84066e9d
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
index 9d40e28..44ab784 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
@@ -34,6 +34,7 @@
import {GroupId, GroupInfo, GroupName} from '../../../types/common';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {GrCreateGroupDialog} from '../gr-create-group-dialog/gr-create-group-dialog';
+import {fireTitleChange} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -100,13 +101,7 @@
attached() {
super.attached();
this._getCreateGroupCapability();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'Groups'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'Groups');
this._maybeOpenCreateOverlay(this.params);
}
diff --git a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
index bf987f9..e5a1586 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
@@ -37,7 +37,7 @@
EncodedGroupId,
GroupAuditEventInfo,
} from '../../../types/common';
-import {firePageError} from '../../../utils/event-util';
+import {firePageError, fireTitleChange} from '../../../utils/event-util';
const GROUP_EVENTS = ['ADD_GROUP', 'REMOVE_GROUP'];
@@ -66,13 +66,7 @@
/** @override */
attached() {
super.attached();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'Audit Log'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'Audit Log');
}
/** @override */
@@ -86,7 +80,9 @@
return '';
}
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getGroupAuditLog(this.groupId, errFn)
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
index af8edea..01571a2 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
@@ -46,7 +46,11 @@
import {AutocompleteQuery} from '../../shared/gr-autocomplete/gr-autocomplete';
import {PolymerDomRepeatEvent} from '../../../types/types';
import {hasOwnProperty} from '../../../utils/common-util';
-import {fireAlert, firePageError} from '../../../utils/event-util';
+import {
+ fireAlert,
+ firePageError,
+ fireTitleChange,
+} from '../../../utils/event-util';
const SUGGESTIONS_LIMIT = 15;
const SAVING_ERROR_TEXT =
@@ -126,13 +130,7 @@
super.attached();
this._loadGroupDetails();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'Members'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'Members');
}
_loadGroupDetails() {
@@ -142,7 +140,9 @@
const promises: Promise<void>[] = [];
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI.getGroupConfig(this.groupId, errFn).then(config => {
if (!config || !config.name) {
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
index b9854c2..4525543 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
@@ -38,7 +38,7 @@
RestApiService,
} from '../../../services/services/gr-rest-api/gr-rest-api';
import {hasOwnProperty} from '../../../utils/common-util';
-import {firePageError} from '../../../utils/event-util';
+import {firePageError, fireTitleChange} from '../../../utils/event-util';
const INTERNAL_GROUP_REGEX = /^[\da-f]{40}$/;
@@ -145,7 +145,9 @@
const promises: Promise<unknown>[] = [];
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI.getGroupConfig(this.groupId, errFn).then(config => {
if (!config || !config.name) {
@@ -176,13 +178,7 @@
}
this._groupConfig = config;
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: config.name},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, config.name);
return Promise.all(promises).then(() => {
this._loading = false;
diff --git a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
index 762d5fc..9337042 100644
--- a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
@@ -31,6 +31,7 @@
import {ErrorCallback} from '../../../services/services/gr-rest-api/gr-rest-api';
import {PluginInfo} from '../../../types/common';
import {firePageError} from '../../../utils/event-util';
+import {fireTitleChange} from '../../../utils/event-util';
interface PluginInfoWithName extends PluginInfo {
name: string;
@@ -85,13 +86,7 @@
/** @override */
attached() {
super.attached();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'Plugins'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'Plugins');
}
_paramsChanged(params: ListViewParams) {
@@ -103,7 +98,9 @@
}
_getPlugins(filter: string, pluginsPerPage: number, offset?: number) {
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getPlugins(filter, pluginsPerPage, offset, errFn)
.then(plugins => {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index fde7234..31e1f65 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -155,7 +155,9 @@
}
_reload(repo: RepoName) {
- const errFn = (response?: Response | null) => firePageError(this, response);
+ const errFn = (response?: Response | null) => {
+ firePageError(this, response);
+ };
this._editing = false;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
index 98c6db0..14cfedd 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
@@ -42,7 +42,11 @@
} from '../../../types/common';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {GrCreateChangeDialog} from '../gr-create-change-dialog/gr-create-change-dialog';
-import {fireAlert, firePageError} from '../../../utils/event-util';
+import {
+ fireAlert,
+ firePageError,
+ fireTitleChange,
+} from '../../../utils/event-util';
const GC_MESSAGE = 'Garbage collection completed successfully.';
const CONFIG_BRANCH = 'refs/meta/config' as BranchName;
@@ -96,13 +100,7 @@
super.attached();
this._loadRepo();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'Repo Commands'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'Repo Commands');
}
_loadRepo() {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
index f2735fb..99f85fa 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
@@ -62,7 +62,9 @@
return Promise.resolve();
}
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getRepoDashboards(repo, errFn)
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
index e4e9d34..17414c0 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
@@ -182,7 +182,9 @@
this._loading = true;
this._items = [];
flush();
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
if (detailType === RepoDetailView.BRANCHES) {
return this.$.restAPI
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
index ba2d850..a566eb0 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
@@ -34,6 +34,7 @@
import {RepoName, ProjectInfoWithName} from '../../../types/common';
import {GrCreateRepoDialog} from '../gr-create-repo-dialog/gr-create-repo-dialog';
import {ProjectState} from '../../../constants/constants';
+import {fireTitleChange} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -93,13 +94,7 @@
attached() {
super.attached();
this._getCreateRepoCapability();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'Repos'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'Repos');
this._maybeOpenCreateOverlay(this.params);
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
index ff4e6ef..426e512 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
@@ -48,7 +48,7 @@
import {ProjectState} from '../../../constants/constants';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {hasOwnProperty} from '../../../utils/common-util';
-import {firePageError} from '../../../utils/event-util';
+import {firePageError, fireTitleChange} from '../../../utils/event-util';
const STATES = {
active: {value: ProjectState.ACTIVE, label: 'Active'},
@@ -150,13 +150,7 @@
super.attached();
this._loadRepo();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: this.repo},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, `${this.repo}`);
}
_computePluginData(
@@ -182,7 +176,9 @@
const promises = [];
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
promises.push(
this._getLoggedIn().then(loggedIn => {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index 927d32f..3391901 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -41,6 +41,7 @@
import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
import {hasOwnProperty} from '../../../utils/common-util';
import {ChangeListViewState} from '../../../types/types';
+import {fireTitleChange} from '../../../utils/event-util';
const LookupQueryPatterns = {
CHANGE_ID: /^\s*i?[0-9a-f]{7,40}\s*$/i,
@@ -143,15 +144,7 @@
// NOTE: This method may be called before attachment. Fire title-change
// in an async so that attachment to the DOM can take place first.
- this.async(() =>
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: this._query},
- composed: true,
- bubbles: true,
- })
- )
- );
+ this.async(() => fireTitleChange(this, this._query));
this.$.restAPI
.getPreferences()
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
index 5f90169..ce3a811 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
@@ -58,7 +58,7 @@
import {ChangeListToggleReviewedDetail} from '../gr-change-list-item/gr-change-list-item';
import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
import {DashboardViewState} from '../../../types/types';
-import {firePageError} from '../../../utils/event-util';
+import {firePageError, fireTitleChange} from '../../../utils/event-util';
const PROJECT_PLACEHOLDER_PATTERN = /\$\{project\}/g;
@@ -150,7 +150,9 @@
project: RepoName,
dashboard: DashboardId
): Promise<UserDashboard | undefined> {
- const errFn = (response?: Response | null) => firePageError(this, response);
+ const errFn = (response?: Response | null) => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getDashboard(project, dashboard, errFn)
.then(response => {
@@ -221,13 +223,7 @@
return dashboardPromise
.then(res => {
if (res && res.title) {
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: res.title},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, res.title);
}
return this._fetchDashboardChanges(res, checkForNewUser);
})
@@ -236,15 +232,7 @@
this.reporting.dashboardDisplayed();
})
.catch(err => {
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {
- title: title || this._computeTitle(user),
- },
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, title || this._computeTitle(user));
console.warn(err);
})
.then(() => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index f8a5940..6b04153 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -76,6 +76,13 @@
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {GrEditableLabel} from '../../shared/gr-editable-label/gr-editable-label';
import {GrLinkedChip} from '../../shared/gr-linked-chip/gr-linked-chip';
+import {appContext} from '../../../services/app-context';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {
+ Metadata,
+ isSectionSet,
+ DisplayRules,
+} from '../../../utils/change-metadata-util';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -194,6 +201,25 @@
@property({type: Object})
_CHANGE_ROLE = ChangeRole;
+ @property({type: Object})
+ _SECTION = Metadata;
+
+ @property({type: Boolean})
+ _showAllSections = false;
+
+ @property({type: Boolean})
+ _isNewChangeSummaryUiEnabled = false;
+
+ flagsService = appContext.flagsService;
+
+ /** @override */
+ ready() {
+ super.ready();
+ this._isNewChangeSummaryUiEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.NEW_CHANGE_SUMMARY_UI
+ );
+ }
+
@observe('change.labels')
_labelsChanged(labels?: LabelNameToInfoMap) {
this.labels = {...labels} || null;
@@ -566,6 +592,35 @@
return this._getNonOwnerRole(change, role) ? '' : 'hideDisplay';
}
+ _computeDisplayState(
+ showAllSections: boolean,
+ change: ParsedChangeInfo | undefined,
+ section: Metadata
+ ) {
+ if (
+ !this._isNewChangeSummaryUiEnabled ||
+ showAllSections ||
+ DisplayRules.ALWAYS_SHOW.includes(section) ||
+ (DisplayRules.SHOW_IF_SET.includes(section) &&
+ isSectionSet(section, change))
+ ) {
+ return '';
+ }
+ return 'hideDisplay';
+ }
+
+ _computeShowAllLabelText(showAllSections: boolean) {
+ if (showAllSections) {
+ return 'Show less';
+ } else {
+ return 'Show all';
+ }
+ }
+
+ _onShowAllClick() {
+ this._showAllSections = !this._showAllSections;
+ }
+
/**
* Get the user with the specified role on the change. Returns null if the
* user with that role is the same as the owner.
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index f1d1127..88f7351 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -94,9 +94,31 @@
--account-max-length: 120px;
max-width: 285px;
}
+ .metadata-title {
+ font-weight: var(--font-weight-bold);
+ color: var(--deemphasized-text-color);
+ padding-left: var(--metadata-horizontal-padding);
+ }
+ .metadata-header {
+ display: flex;
+ justify-content: space-between;
+ }
</style>
<gr-external-style id="externalStyle" name="change-metadata">
- <section>
+ <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
+ <div class="metadata-header">
+ <h3 class="metadata-title">Change Info</h3>
+ <gr-button
+ class="show-all-button"
+ on-click="_onShowAllClick"
+ no-uppercase=""
+ >[[_computeShowAllLabelText(_showAllSections)]]</gr-button
+ >
+ </div>
+ </template>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.UPDATED)]]"
+ >
<span class="title">Updated</span>
<span class="value">
<gr-date-formatter
@@ -105,7 +127,9 @@
></gr-date-formatter>
</span>
</section>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.OWNER)]]"
+ >
<span class="title">Owner</span>
<span class="value">
<gr-account-chip
@@ -156,7 +180,9 @@
</span>
</section>
<template is="dom-if" if="[[_isAssigneeEnabled(serverConfig)]]">
- <section class="assignee">
+ <section
+ class$="assignee [[_computeDisplayState(_showAllSections, change, _SECTION.ASSIGNEE)]]"
+ >
<span class="title">Assignee</span>
<span class="value">
<gr-account-list
@@ -172,7 +198,9 @@
</span>
</section>
</template>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REVIEWERS)]]"
+ >
<span class="title">Reviewers</span>
<span class="value">
<gr-reviewer-list
@@ -183,7 +211,9 @@
></gr-reviewer-list>
</span>
</section>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.CC)]]"
+ >
<span class="title">CC</span>
<span class="value">
<gr-reviewer-list
@@ -198,7 +228,9 @@
is="dom-if"
if="[[_computeShowRepoBranchTogether(change.project, change.branch)]]"
>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REPO_BRANCH)]]"
+ >
<span class="title">Repo | Branch</span>
<span class="value">
<a href$="[[_computeProjectUrl(change.project)]]"
@@ -215,7 +247,9 @@
is="dom-if"
if="[[!_computeShowRepoBranchTogether(change.project, change.branch)]]"
>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REPO_BRANCH)]]"
+ >
<span class="title">Repo</span>
<span class="value">
<a href$="[[_computeProjectUrl(change.project)]]">
@@ -226,7 +260,9 @@
</a>
</span>
</section>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REPO_BRANCH)]]"
+ >
<span class="title">Branch</span>
<span class="value">
<a href$="[[_computeBranchUrl(change.project, change.branch)]]">
@@ -238,7 +274,9 @@
</span>
</section>
</template>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.PARENT)]]"
+ >
<span class="title">[[_computeParentsLabel(_currentParents)]]</span>
<span class="value">
<ol
@@ -262,7 +300,9 @@
</ol>
</span>
</section>
- <section class="topic">
+ <section
+ class$="topic [[_computeDisplayState(_showAllSections, change, _SECTION.TOPIC)]]"
+ >
<span class="title">Topic</span>
<span class="value">
<template is="dom-if" if="[[_showTopicChip(change.*, _settingTopic)]]">
@@ -288,7 +328,9 @@
</span>
</section>
<template is="dom-if" if="[[_showCherryPickOf(change.*)]]">
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.CHERRY_PICK_OF)]]"
+ >
<span class="title">Cherry pick of</span>
<span class="value">
<a
@@ -304,14 +346,16 @@
</section>
</template>
<section
- class="strategy"
+ class$="strategy [[_computeDisplayState(_showAllSections, change, _SECTION.STRATEGY)]]"
hidden$="[[_computeHideStrategy(change)]]"
hidden=""
>
<span class="title">Strategy</span>
<span class="value">[[_computeStrategy(change)]]</span>
</section>
- <section class="hashtag">
+ <section
+ class$="hashtag [[_computeDisplayState(_showAllSections, change, _SECTION.HASHTAGS)]]"
+ >
<span class="title">Hashtags</span>
<span class="value">
<template is="dom-repeat" items="[[change.hashtags]]">
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 c763aef..24841fc 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
@@ -146,7 +146,6 @@
CustomKeyboardEvent,
EditableContentSaveEvent,
OpenFixPreviewEvent,
- ShowAlertEventDetail,
SwitchTabEvent,
} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
@@ -155,6 +154,7 @@
import {PORTING_COMMENTS_CHANGE_LATENCY_LABEL} from '../../../services/gr-reporting/gr-reporting';
import {fireAlert, firePageError} from '../../../utils/event-util';
import {KnownExperimentId} from '../../../services/flags/flags';
+import {fireTitleChange} from '../../../utils/event-util';
const CHANGE_ID_ERROR = {
MISMATCH: 'mismatch',
@@ -1489,13 +1489,7 @@
this.set('_patchRange.basePatchNum', parent);
const title = change.subject + ' (' + change.change_id.substr(0, 9) + ')';
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, title);
}
/**
@@ -1698,16 +1692,7 @@
throw new Error('missing required _patchRange property');
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
if (patchNumEquals(this._patchRange.patchNum, latestPatchNum)) {
- const detail: ShowAlertEventDetail = {
- message: 'Latest is already selected.',
- };
- this.dispatchEvent(
- new CustomEvent('show-alert', {
- detail,
- composed: true,
- bubbles: true,
- })
- );
+ fireAlert(this, 'Latest is already selected.');
return;
}
GerritNav.navigateToChange(
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 108f9ed..7759b7b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -1558,7 +1558,7 @@
'changeComments, patchRange and diffPrefs must be set'
);
}
- diffElem.comments = this.changeComments.getCommentsBySideForFile(
+ diffElem.threads = this.changeComments.getThreadsBySideForFile(
file,
this.patchRange,
this.projectConfig
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
index 7a56d1c..4f3d7ce6 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
@@ -39,6 +39,7 @@
import {FetchRequest} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
import {ErrorType, FixIronA11yAnnouncer} from '../../../types/types';
import {AccountId} from '../../../types/common';
+import {EventType} from '../../../utils/event-util';
const HIDE_ALERT_TIMEOUT_MS = 5000;
const CHECK_SIGN_IN_INTERVAL_MS = 60 * 1000;
@@ -129,7 +130,7 @@
super.attached();
this.listen(document, 'server-error', '_handleServerError');
this.listen(document, 'network-error', '_handleNetworkError');
- this.listen(document, 'show-alert', '_handleShowAlert');
+ this.listen(document, EventType.SHOW_ALERT, '_handleShowAlert');
this.listen(document, 'hide-alert', '_hideAlert');
this.listen(document, 'show-error', '_handleShowErrorDialog');
this.listen(document, 'visibilitychange', '_handleVisibilityChange');
@@ -151,7 +152,7 @@
this._clearHideAlertHandle();
this.unlisten(document, 'server-error', '_handleServerError');
this.unlisten(document, 'network-error', '_handleNetworkError');
- this.unlisten(document, 'show-alert', '_handleShowAlert');
+ this.unlisten(document, EventType.SHOW_ALERT, '_handleShowAlert');
this.unlisten(document, 'hide-alert', '_hideAlert');
this.unlisten(document, 'show-error', '_handleShowErrorDialog');
this.unlisten(document, 'visibilitychange', '_handleVisibilityChange');
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index b01a110..683d887 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -38,7 +38,7 @@
RevisionId,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
-import {CommentSide} from '../../../constants/constants';
+import {CommentSide, Side} from '../../../constants/constants';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {
Comment,
@@ -313,6 +313,30 @@
return allDrafts;
}
+ _addCommentSide(comments: TwoSidesComments) {
+ const allComments = [];
+ for (const side of [Side.LEFT, Side.RIGHT]) {
+ // This is needed by the threading.
+ for (const comment of comments[side]) {
+ comment.__commentSide = side;
+ }
+ allComments.push(...comments[side]);
+ }
+ return allComments;
+ }
+
+ getThreadsBySideForPath(
+ path: string,
+ patchRange: PatchRange,
+ projectConfig?: ConfigInfo
+ ): CommentThread[] {
+ return createCommentThreads(
+ this._addCommentSide(
+ this.getCommentsBySideForPath(path, patchRange, projectConfig)
+ )
+ );
+ }
+
/**
* Get the comments (with drafts and robot comments) for a path and
* patch-range. Returns an object with left and right properties mapping to
@@ -371,6 +395,18 @@
};
}
+ getThreadsBySideForFile(
+ file: PatchSetFile,
+ patchRange: PatchRange,
+ projectConfig?: ConfigInfo
+ ): CommentThread[] {
+ return createCommentThreads(
+ this._addCommentSide(
+ this.getCommentsBySideForFile(file, patchRange, projectConfig)
+ )
+ );
+ }
+
/**
* Get the comments (with drafts and robot comments) for a file and
* patch-range. Returns an object with left and right properties mapping to
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 684a58a..43ed77f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -226,9 +226,10 @@
bubbles: true,
})
);
+ } else {
+ this.lastDisplayedNavigateToNextFileToast = Date.now();
+ fireAlert(this, 'Press n again to navigate to next unreviewed file');
}
- this.lastDisplayedNavigateToNextFileToast = Date.now();
- fireAlert(this, 'Press n again to navigate to next unreviewed file');
}
this._fixSide();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 8c08084..4b4f429 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -31,14 +31,7 @@
isMergeParent,
isNumber,
} from '../../../utils/patch-set-util';
-import {
- Comment,
- isDraft,
- UIComment,
- CommentThread,
- createCommentThreads,
-} from '../../../utils/comment-util';
-import {TwoSidesComments} from '../gr-comment-api/gr-comment-api';
+import {CommentThread} from '../../../utils/comment-util';
import {customElement, observe, property} from '@polymer/decorators';
import {
CommitRange,
@@ -198,8 +191,8 @@
@property({type: Boolean})
noRenderOnPrefsChange = false;
- @property({type: Object, observer: '_commentsChanged'})
- comments?: TwoSidesComments;
+ @property({type: Object, observer: '_threadsChanged'})
+ threads?: CommentThread[];
@property({type: Boolean})
lineWrapping = false;
@@ -276,11 +269,12 @@
'create-comment',
e => this._handleCreateComment(e)
);
- this.addEventListener('comment-discard', e =>
- this._handleCommentDiscard(e)
+ this.addEventListener('comment-discard', () =>
+ this._handleCommentSaveOrDiscard()
);
- this.addEventListener('comment-update', e => this._handleCommentUpdate(e));
- this.addEventListener('comment-save', e => this._handleCommentSave(e));
+ this.addEventListener('comment-save', () =>
+ this._handleCommentSaveOrDiscard()
+ );
this.addEventListener('render-start', () => this._handleRenderStart());
this.addEventListener('render-content', () => this._handleRenderContent());
this.addEventListener('normalize-range', event =>
@@ -679,21 +673,12 @@
return isImageDiff(diff);
}
- _commentsChanged(newComments: TwoSidesComments) {
- const allComments = [];
- for (const side of [Side.LEFT, Side.RIGHT]) {
- // This is needed by the threading.
- for (const comment of newComments[side]) {
- comment.__commentSide = side;
- }
- allComments.push(...newComments[side]);
- }
+ _threadsChanged(threads: CommentThread[]) {
// Currently, the only way this is ever changed here is when the initial
- // comments are loaded, so it's okay performance wise to clear the threads
+ // threads are loaded, so it's okay performance wise to clear the threads
// and recreate them. If this changes in future, we might want to reuse
// some DOM nodes here.
this._clearThreads();
- const threads = createCommentThreads(allComments);
for (const thread of threads) {
const threadEl = this._createThreadElement(thread);
this._attachThreadElement(threadEl);
@@ -932,79 +917,12 @@
: null;
}
- _handleCommentSave(e: CustomEvent) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- const idx = this._findDraftIndex(comment, side);
- this.set(['comments', side, idx], comment);
- this._handleCommentSaveOrDiscard();
- }
-
- _handleCommentDiscard(e: CustomEvent) {
- const comment = e.detail.comment;
- this._removeComment(comment);
- this._handleCommentSaveOrDiscard();
- }
-
- _handleCommentUpdate(e: CustomEvent) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) {
- // Update draft or comment.
- this.set(['comments', side, idx], comment);
- } else {
- // Create new draft.
- this.push(['comments', side], comment);
- }
- }
-
_handleCommentSaveOrDiscard() {
this.dispatchEvent(
new CustomEvent('diff-comments-modified', {bubbles: true, composed: true})
);
}
- _removeComment(comment: UIComment) {
- const side = comment.__commentSide;
- if (!side) throw new Error('Missing required "side" in comment.');
- this._removeCommentFromSide(comment, side);
- }
-
- _removeCommentFromSide(comment: Comment, side: Side) {
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) {
- this.splice('comments.' + side, idx, 1);
- }
- }
-
- _findCommentIndex(comment: Comment, side: Side) {
- if (!comment.id || !this.comments || !this.comments[side]) {
- return -1;
- }
- return this.comments[side].findIndex(item => item.id === comment.id);
- }
-
- _findDraftIndex(comment: Comment, side: Side) {
- if (
- !isDraft(comment) ||
- !comment.__draftID ||
- !this.comments ||
- !this.comments[side]
- ) {
- return -1;
- }
- return this.comments[side].findIndex(
- item => isDraft(item) && item.__draftID === comment.__draftID
- );
- }
-
_isSyntaxHighlightingEnabled(
preferenceChangeRecord?: PolymerDeepPropertyChange<
DiffPreferencesInfo,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index bf4d912..402cb52 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -62,198 +62,6 @@
});
});
- suite('handle comment-update', () => {
- setup(() => {
- sinon.stub(element, '_commentsChanged');
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
- });
-
- test('creating a draft', () => {
- const comment = {__draft: true, __draftID: 'tempID', side: 'PARENT',
- __commentSide: 'left'};
- element.dispatchEvent(
- new CustomEvent('comment-update', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- assert.include(element.comments.left, comment);
- });
-
- test('discarding a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sinon.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.dispatchEvent(
- new CustomEvent('comment-discard', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- const drafts = element.comments.left
- .filter(item => item.__draftID === draftID);
- assert.equal(drafts.length, 0);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
-
- test('saving a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sinon.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.dispatchEvent(
- new CustomEvent('comment-save', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- const drafts = element.comments.left
- .filter(item => item.__draftID === draftID);
- assert.equal(drafts.length, 1);
- assert.equal(drafts[0].id, id);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
- });
-
- test('remove comment', () => {
- sinon.stub(element, '_commentsChanged');
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
-
- // Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesn’t seem
- // to believe that one object deepEquals another even when they do :-/.
- assert.equal(JSON.stringify(element.comments), JSON.stringify({
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- }));
-
- element._removeComment({id: 'bc2', side: 'PARENT',
- __commentSide: 'left'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- });
-
- element._removeComment({id: 'd2', __commentSide: 'right'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- ],
- });
- });
-
test('thread-discard handling', () => {
const threads = createCommentThreads([
{
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 e1780f8..bb0ed61 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
@@ -65,11 +65,7 @@
GrDropdownList,
} from '../../shared/gr-dropdown-list/gr-dropdown-list';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
-import {
- ChangeComments,
- GrCommentApi,
- TwoSidesComments,
-} from '../gr-comment-api/gr-comment-api';
+import {ChangeComments, GrCommentApi} from '../gr-comment-api/gr-comment-api';
import {GrDiffModeSelector} from '../gr-diff-mode-selector/gr-diff-mode-selector';
import {
ChangeInfo,
@@ -101,7 +97,7 @@
import {AppElementParams} from '../../gr-app-types';
import {CustomKeyboardEvent, OpenFixPreviewEvent} from '../../../types/events';
import {PORTING_COMMENTS_DIFF_LATENCY_LABEL} from '../../../services/gr-reporting/gr-reporting';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireTitleChange} from '../../../utils/event-util';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
@@ -240,9 +236,6 @@
@property({type: Object})
_commentMap?: CommentMap;
- @property({type: Object})
- _commentsForDiff?: TwoSidesComments;
-
@property({
type: Object,
computed: '_computeCommentSkips(_commentMap, _fileList, _path)',
@@ -1013,12 +1006,6 @@
}
this._commentMap = this._getPaths(this._patchRange);
-
- this._commentsForDiff = this._getCommentsForPath(
- this._path,
- this._patchRange,
- this._projectConfig
- );
}
_isFileUnchanged(diff: DiffInfo) {
@@ -1087,7 +1074,13 @@
this._loading = false;
this._initPatchRange();
this._initCommitRange();
- this.$.diffHost.comments = this._commentsForDiff;
+ if (this._changeComments && this._path && this._patchRange) {
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForPath(
+ this._path,
+ this._patchRange,
+ this._projectConfig
+ );
+ }
portedCommentsPromise.then(() => {
this.reporting.timeEnd(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
});
@@ -1223,13 +1216,7 @@
_pathChanged(path: string) {
if (path) {
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: computeTruncatedPath(path)},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, computeTruncatedPath(path));
}
if (!this._fileList || this._fileList.length === 0) return;
@@ -1581,13 +1568,11 @@
const file = files[path];
if (file && file.old_path) {
- this._commentsForDiff = this._changeComments.getCommentsBySideForFile(
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForFile(
{path, basePath: file.old_path},
patchRange,
projectConfig
);
-
- this.$.diffHost.comments = this._commentsForDiff;
}
}
@@ -1596,22 +1581,6 @@
return this._changeComments.getPaths(patchRange);
}
- _getCommentsForPath(
- path?: string,
- patchRange?: PatchRange,
- projectConfig?: ConfigInfo
- ) {
- if (!path) return undefined;
- if (!patchRange) return undefined;
- if (!this._changeComments) return undefined;
-
- return this._changeComments.getCommentsBySideForPath(
- path,
- patchRange,
- projectConfig
- );
- }
-
_getDiffDrafts() {
if (!this._changeNum) throw new Error('Missing this._changeNum');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 7ef75ed..8e1e5c1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -151,8 +151,10 @@
computeCommentThreadCount: () => {},
computeUnresolvedNum: () => {},
getPaths: () => {},
- getCommentsBySideForPath: () => {},
+ getThreadsBySideForPath: () => {},
+ getThreadsBySideForFile: () => {},
findCommentById: _testOnly_findCommentById,
+
}));
await element._loadComments();
await flush();
@@ -200,11 +202,6 @@
commentLink: true,
commentId: 'c1',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
- sinon.stub(element, '_getCommentsForPath').returns({
- left: [{id: 'c1', __commentSide: 'left', line: 10}],
- right: [{id: 'c2', __commentSide: 'right', line: 11}],
- });
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -263,7 +260,6 @@
commentLink: true,
commentId: 'c1',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -293,7 +289,6 @@
commentLink: true,
commentId: 'c3',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -1455,7 +1450,6 @@
await flush();
});
test('empty', () => {
- sinon.stub(element, '_getCommentsForPath');
sinon.stub(element, '_getPaths').returns(new Map());
element._initPatchRange();
assert.equal(Object.keys(element._commentMap).length, 0);
@@ -1467,7 +1461,6 @@
'path/to/file/one.cpp': [{patch_set: 3, message: 'lorem'}],
'path-to/file/two.py': [{patch_set: 5, message: 'ipsum'}],
});
- sinon.stub(element, '_getCommentsForPath').returns({meta: {}});
element._changeNum = '42';
element._patchRange = {
basePatchNum: 3,
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
index 5967b03..79c4359 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
@@ -30,6 +30,7 @@
import {customElement, property} from '@polymer/decorators';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {DocResult} from '../../../types/common';
+import {fireTitleChange} from '../../../utils/event-util';
export interface GrDocumentationSearch {
$: {
@@ -62,9 +63,7 @@
/** @override */
attached() {
super.attached();
- this.dispatchEvent(
- new CustomEvent('title-change', {detail: {title: 'Documentation Search'}})
- );
+ fireTitleChange(this, 'Documentation Search');
}
_paramsChanged(params: ListViewParams) {
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index 5863ee3..60944d0 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -47,7 +47,7 @@
} from '../../../types/common';
import {GrStorage} from '../../shared/gr-storage/gr-storage';
import {HttpMethod, NotifyType} from '../../../constants/constants';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireTitleChange} from '../../../utils/event-util';
const RESTORED_MESSAGE = 'Content restored from a previous edit.';
const SAVING_MESSAGE = 'Saving changes...';
@@ -177,13 +177,7 @@
// has been queued, the event can bubble up to the handler in gr-app.
this.async(() => {
const title = `Editing ${computeTruncatedPath(value.path)}`;
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, title);
});
const promises = [];
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index c2fb124..2915acd 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -79,6 +79,7 @@
TitleChangeEventDetail,
} from '../types/events';
import {ViewState} from '../types/types';
+import {EventType} from '../utils/event-util';
interface ErrorInfo {
text: string;
@@ -209,8 +210,12 @@
created() {
super.created();
this._bindKeyboardShortcuts();
- this.addEventListener('page-error', e => this._handlePageError(e));
- this.addEventListener('title-change', e => this._handleTitleChange(e));
+ this.addEventListener(EventType.PAGE_ERROR, e => {
+ this._handlePageError(e);
+ });
+ this.addEventListener(EventType.TITLE_CHANGE, e => {
+ this._handleTitleChange(e);
+ });
this.addEventListener('location-change', e =>
this._handleLocationChange(e)
);
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
index 092c5de..891fdf6 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
@@ -32,7 +32,7 @@
GroupInfo,
ContributorAgreementInfo,
} from '../../../types/common';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireTitleChange} from '../../../utils/event-util';
export interface GrClaView {
$: {
@@ -80,13 +80,7 @@
super.attached();
this.loadData();
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'New Contributor Agreement'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'New Contributor Agreement');
}
loadData() {
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
index 84bdf78..4293a83 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
@@ -68,7 +68,7 @@
import {GerritView} from '../../core/gr-navigation/gr-navigation';
import {GrEmailEditor} from '../gr-email-editor/gr-email-editor';
import {CustomKeyboardEvent} from '../../../types/events';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireTitleChange} from '../../../utils/event-util';
const PREFS_SECTION_FIELDS: Array<keyof PreferencesInput> = [
'changes_per_page',
@@ -220,13 +220,7 @@
// Polymer 2: anchor tag won't work on shadow DOM
// we need to manually calling scrollIntoView when hash changed
this.listen(window, 'location-change', '_handleLocationChange');
- this.dispatchEvent(
- new CustomEvent('title-change', {
- detail: {title: 'Settings'},
- composed: true,
- bubbles: true,
- })
- );
+ fireTitleChange(this, 'Settings');
this._isDark = !!window.localStorage.getItem('dark-theme');
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index b3c0a85..b0b40dd 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -38,32 +38,23 @@
{value: '😊', match: 'smile :)'},
{value: '👍', match: 'thumbs up'},
{value: '😄', match: 'laugh :D'},
- {value: '🎉', match: 'party'},
- {value: '😞', match: 'sad :('},
+ {value: '❤️', match: 'heart <3'},
{value: '😂', match: "tears :')"},
- {value: '🙏', match: 'pray'},
+ {value: '🎉', match: 'party'},
+ {value: '😎', match: 'cool |;)'},
+ {value: '😞', match: 'sad :('},
{value: '😐', match: 'neutral :|'},
{value: '😮', match: 'shock :O'},
- {value: '👎', match: 'thumbs down'},
- {value: '😎', match: 'cool |;)'},
+ {value: '🙏', match: 'pray'},
{value: '😕', match: 'confused'},
{value: '👌', match: 'ok'},
{value: '🔥', match: 'fire'},
- {value: '👊', match: 'fistbump'},
{value: '💯', match: '100'},
- {value: '💔', match: 'broken heart'},
- {value: '🍺', match: 'beer'},
{value: '✔', match: 'check'},
{value: '😋', match: 'tongue'},
{value: '😭', match: "crying :'("},
- {value: '🐨', match: 'koala'},
{value: '🤓', match: 'glasses'},
- {value: '😆', match: 'grin'},
- {value: '💩', match: 'poop'},
{value: '😢', match: 'tear'},
- {value: '😒', match: 'unamused'},
- {value: '😉', match: 'wink ;)'},
- {value: '🍷', match: 'wine'},
{value: '😜', match: 'winking tongue ;)'},
];
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index e40412d..ba33954 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -27,4 +27,5 @@
PATCHSET_COMMENTS = 'UiFeature__patchset_comments',
NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks',
+ NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
}
diff --git a/polygerrit-ui/app/utils/change-metadata-util.ts b/polygerrit-ui/app/utils/change-metadata-util.ts
new file mode 100644
index 0000000..6ce1483
--- /dev/null
+++ b/polygerrit-ui/app/utils/change-metadata-util.ts
@@ -0,0 +1,75 @@
+/**
+ * @license
+ * Copyright (C) 2020 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.
+ */
+
+import {ParsedChangeInfo} from '../elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser';
+
+export enum Metadata {
+ OWNER = 'Owner',
+ REVIEWERS = 'Reviewers',
+ REPO_BRANCH = 'Repo | Branch',
+ SUBMITTED = 'Submitted',
+ PARENT = 'Parent',
+ STRATEGY = 'Strategy',
+ UPDATED = 'Updated',
+ CC = 'CC',
+ HASHTAGS = 'Hashtags',
+ TOPIC = 'Topic',
+ UPLOADER = 'Uploader',
+ AUTHOR = 'Author',
+ COMMITTER = 'Committer',
+ ASSIGNEE = 'Assignee',
+ CHERRY_PICK_OF = 'Cherry pick of',
+}
+
+export const DisplayRules = {
+ ALWAYS_SHOW: [
+ Metadata.OWNER,
+ Metadata.REVIEWERS,
+ Metadata.REPO_BRANCH,
+ Metadata.SUBMITTED,
+ ],
+ SHOW_IF_SET: [
+ Metadata.CC,
+ Metadata.HASHTAGS,
+ Metadata.TOPIC,
+ Metadata.UPLOADER,
+ Metadata.AUTHOR,
+ Metadata.COMMITTER,
+ Metadata.ASSIGNEE,
+ Metadata.CHERRY_PICK_OF,
+ ],
+ ALWAYS_HIDE: [Metadata.PARENT, Metadata.STRATEGY, Metadata.UPDATED],
+};
+
+export function isSectionSet(section: Metadata, change?: ParsedChangeInfo) {
+ switch (section) {
+ case Metadata.CC:
+ return !!change?.reviewers?.CC?.length;
+ case Metadata.HASHTAGS:
+ return !!change?.hashtags?.length;
+ case Metadata.TOPIC:
+ return !!change?.topic;
+ case Metadata.UPLOADER:
+ case Metadata.AUTHOR:
+ case Metadata.COMMITTER:
+ case Metadata.ASSIGNEE:
+ return false;
+ case Metadata.CHERRY_PICK_OF:
+ return !!change?.cherry_pick_of_change;
+ }
+ return true;
+}
diff --git a/polygerrit-ui/app/utils/event-util.ts b/polygerrit-ui/app/utils/event-util.ts
index 6587dfa..0af8fe2 100644
--- a/polygerrit-ui/app/utils/event-util.ts
+++ b/polygerrit-ui/app/utils/event-util.ts
@@ -18,6 +18,7 @@
export enum EventType {
SHOW_ALERT = 'show-alert',
PAGE_ERROR = 'page-error',
+ TITLE_CHANGE = 'title-change',
}
export function fireAlert(target: EventTarget, message: string) {
@@ -39,3 +40,13 @@
})
);
}
+
+export function fireTitleChange(target: EventTarget, title: string) {
+ target.dispatchEvent(
+ new CustomEvent(EventType.TITLE_CHANGE, {
+ detail: {title},
+ composed: true,
+ bubbles: true,
+ })
+ );
+}
diff --git a/resources/com/google/gerrit/pgm/init/gerrit.sh b/resources/com/google/gerrit/pgm/init/gerrit.sh
index ce858d5..87a6c05 100755
--- a/resources/com/google/gerrit/pgm/init/gerrit.sh
+++ b/resources/com/google/gerrit/pgm/init/gerrit.sh
@@ -296,6 +296,11 @@
GERRIT_FDS=`expr $FDS_MULTIPLIER \* $GERRIT_FDS`
test $GERRIT_FDS -lt 1024 && GERRIT_FDS=1024
+CACHE_FDS=`get_config --get cache.openFiles`
+if test -n "$CACHE_FDS"; then
+ GERRIT_FDS=`expr $CACHE_FDS \+ $GERRIT_FDS`
+fi
+
GERRIT_STARTUP_TIMEOUT=`get_config --get container.startupTimeout`
test -z "$GERRIT_STARTUP_TIMEOUT" && GERRIT_STARTUP_TIMEOUT=90 # seconds
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index a3cc66e..459143d 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -23,8 +23,8 @@
maven_jar(
name = "dropwizard-core",
- artifact = "io.dropwizard.metrics:metrics-core:4.1.14",
- sha1 = "14cf9dd67619a0390812dddb232df339e3383d35",
+ artifact = "io.dropwizard.metrics:metrics-core:4.1.12.1",
+ sha1 = "cb2f351bf4463751201f43bb99865235d5ba07ca",
)
SSHD_VERS = "2.4.0"
@@ -143,18 +143,40 @@
sha1 = "dc13ae4faca6df981fc7aeb5a522d9db446d5d50",
)
- TESTCONTAINERS_VERSION = "1.14.3"
+ DOCKER_JAVA_VERS = "3.2.5"
+
+ maven_jar(
+ name = "docker-java-api",
+ artifact = "com.github.docker-java:docker-java-api:" + DOCKER_JAVA_VERS,
+ sha1 = "8fe5c5e39f940ce58620e77cedc0a2a52d76f9d8",
+ )
+
+ maven_jar(
+ name = "docker-java-transport",
+ artifact = "com.github.docker-java:docker-java-transport:" + DOCKER_JAVA_VERS,
+ sha1 = "27af0ee7ebc2f5672e23ea64769497b5d55ce3ac",
+ )
+
+ # https://github.com/docker-java/docker-java/blob/3.2.5/pom.xml#L61
+ # <=> DOCKER_JAVA_VERS
+ maven_jar(
+ name = "jackson-annotations",
+ artifact = "com.fasterxml.jackson.core:jackson-annotations:2.10.3",
+ sha1 = "0f63b3b1da563767d04d2e4d3fc1ae0cdeffebe7",
+ )
+
+ TESTCONTAINERS_VERSION = "1.15.0"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
- sha1 = "071fc82ba663f469447a19434e7db90f3a872753",
+ sha1 = "b627535b444d88e7b14953bb953d80d9b7b3bd76",
)
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
- sha1 = "3709e2ebb0b6aa4e2ba2b6ca92ffdd3bf637a86c",
+ sha1 = "2bd79fd915e5c7bcf9b5d86cd8e0b7a0fff4b8ce",
)
maven_jar(
