Merge "Fix change message when change is created as WiP revert"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ce736c0..8aa5c7f 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -986,6 +986,10 @@
MergePatchSetInput and add a new patch set to the change corresponding
to the new merge commit.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the current patch set, the same email will be used as the committer email in the
+new patch set; otherwise, the user's preferred email will be used.
+
.Request
----
POST /changes/test~master~Ic5466d107c5294414710935a8ef3b0180fb848dc/merge HTTP/1.0
@@ -1039,6 +1043,10 @@
Creates a new patch set with a new commit message.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the current patch set, the same email will be used as the committer email in the
+new patch set; otherwise, the user's preferred email will be used.
+
The new commit message must be provided in the request body inside a
link:#commit-message-input[CommitMessageInput] entity. If a Change-Id
footer is specified, it must match the current Change-Id footer. If
@@ -1314,6 +1322,10 @@
Rebases a change.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the current patch set, the same email will be used as the committer email in the
+new patch set; otherwise, the user's preferred email will be used.
+
Optionally, the parent revision can be changed to another patch set through the
link:#rebase-input[RebaseInput] entity.
@@ -1440,6 +1452,10 @@
Rebases an ancestry chain of changes.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the current patch set, the same email will be used as the committer email in the
+new patch set; otherwise, the user's preferred email will be used.
+
The operated change is treated as the chain tip. All unsubmitted ancestors are rebased.
Requires a linear ancestry relation (single parenting throughout the chain).
@@ -1919,6 +1935,11 @@
Submits a change.
+If the submission results in a new patch set (due to a rebase or cherry-pick merge method), the
+committer email will remain the same as the one used in the previous commit, provided that one of
+the secondary emails associated with the user performing the operation was used as the committer
+email in the previous commit. Otherwise, the user's preferred email will be used.
+
The request body only needs to include a link:#submit-input[
SubmitInput] entity if submitting on behalf of another user.
@@ -2276,6 +2297,10 @@
Creates a new patch set on a destination change from the provided patch.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the current patch set, the same email will be used as the committer email in the
+new patch set; otherwise, the user's preferred email will be used.
+
The patch must be provided in the request body, inside a
link:#applypatchpatchset-input[ApplyPatchPatchSetInput] entity.
@@ -2891,6 +2916,11 @@
The custom keyed values to add or remove must be provided in the request body
inside a link:#custom-keyed-values-input[CustomKeyedValuesInput] entity.
+Note that custom keyed values are expected to be small in both key and value.
+A typical use-case would be storing the ID to some external system, in which
+case the key would be something unique to that system and the value would be
+the ID.
+
.Request
----
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/custom_keyed_values HTTP/1.0
@@ -3183,11 +3213,15 @@
[[put-edit-file]]
=== Change file content in Change Edit
--
-'PUT /changes/link:#change-id[\{change-id\}]/edit/path%2fto%2ffile
+'PUT /changes/link:#change-id[\{change-id\}]/edit/path%2fto%2ffile'
--
Put content of a file to a change edit.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the latest patch set, the same email will be used as the committer email in the
+new change edit commit; otherwise, the user's preferred email will be used.
+
.Request
----
PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit/foo HTTP/1.0
@@ -3234,7 +3268,7 @@
[[post-edit]]
=== Restore file content or rename files in Change Edit
--
-'POST /changes/link:#change-id[\{change-id\}]/edit
+'POST /changes/link:#change-id[\{change-id\}]/edit'
--
Creates empty change edit, restores file content or renames files in change
@@ -3242,6 +3276,10 @@
link:#change-edit-input[ChangeEditInput] entity when a file within change
edit should be restored or old and new file names to rename a file.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the latest patch set, the same email will be used as the committer email in the
+new change edit commit; otherwise, the user's preferred email will be used.
+
.Request
----
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit HTTP/1.0
@@ -3278,13 +3316,17 @@
[[put-change-edit-message]]
=== Change commit message in Change Edit
--
-'PUT /changes/link:#change-id[\{change-id\}]/edit:message
+'PUT /changes/link:#change-id[\{change-id\}]/edit:message'
--
Modify commit message. The request body needs to include a
link:#change-edit-message-input[ChangeEditMessageInput]
entity.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the latest patch set, the same email will be used as the committer email in the
+new change edit commit; otherwise, the user's preferred email will be used.
+
.Request
----
PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit:message HTTP/1.0
@@ -3313,6 +3355,10 @@
completely. This is not the same as reverting or restoring a file to its
previous contents.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the latest patch set, the same email will be used as the committer email in the
+new change edit commit; otherwise, the user's preferred email will be used.
+
.Request
----
DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit/foo HTTP/1.0
@@ -3480,11 +3526,15 @@
[[rebase-edit]]
=== Rebase Change Edit
--
-'POST /changes/link:#change-id[\{change-id\}]/edit:rebase
+'POST /changes/link:#change-id[\{change-id\}]/edit:rebase'
--
Rebases change edit on top of latest patch set.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the latest patch set, the same email will be used as the committer email in the
+new change edit commit; otherwise, the user's preferred email will be used.
+
.Request
----
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit:rebase HTTP/1.0
@@ -5594,6 +5644,10 @@
exists and the fix refers to the current patch set, or the fix refers to the
patch set on which the change edit is based.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the current patch set, the same email will be used as the committer email in the
+new change edit commit; otherwise, the user's preferred email will be used.
+
.Request
----
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/fixes/8f605a55_f6aa4ecc/apply HTTP/1.0
@@ -5660,6 +5714,10 @@
application of the fixes creates a new change edit. `Apply Provided Fix` can only be applied on the current
patchset.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the current patch set, the same email will be used as the committer email in the
+new change edit commit; otherwise, the user's preferred email will be used.
+
.Request
----
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/fix:apply HTTP/1.0
@@ -6315,6 +6373,10 @@
Cherry picks a revision to a destination branch.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the original revision, the same email will be used as the committer email
+in the new patch set; otherwise, the user's preferred email will be used.
+
To cherry pick a commit with no change-id associated with it, see
link:rest-api-projects.html#cherry-pick-commit[CherryPickCommit].
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 528be41..73fac68 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -2667,6 +2667,10 @@
Cherry-picks a commit of a project to a destination branch.
+If one of the secondary emails associated with the user performing the operation was used as the
+committer email in the original commit, the same email will be used as the committer email in the
+new patch set; otherwise, the user's preferred email will be used.
+
To cherry pick a change revision, see link:rest-api-changes.html#cherry-pick[CherryPick].
The destination branch must be provided in the request body inside a
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index 36d7888..d45d329 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -468,9 +468,7 @@
public PersonIdent newCommitterIdent(Instant when, ZoneId zoneId) {
final Account ua = getAccount();
- String name = ua.fullName();
String email = ua.preferredEmail();
-
if (email == null || email.isEmpty()) {
// No preferred email is configured. Use a generic identity so we
// don't leak an address the user may have given us, but doesn't
@@ -491,19 +489,18 @@
email = user + "@" + host;
}
-
- if (name == null || name.isEmpty()) {
- final int at = email.indexOf('@');
- if (0 < at) {
- name = email.substring(0, at);
- } else {
- name = anonymousCowardName;
- }
- }
-
+ String name = getCommitterName(ua, email);
return new PersonIdent(name, email, when, zoneId);
}
+ public Optional<PersonIdent> newCommitterIdent(String email, Instant when, ZoneId zoneId) {
+ if (!hasEmailAddress(email)) {
+ return Optional.empty();
+ }
+ String name = getCommitterName(getAccount(), email);
+ return Optional.of(new PersonIdent(name, email, when, zoneId));
+ }
+
@Override
public String toString() {
return "IdentifiedUser[account " + getAccountId() + "]";
@@ -551,4 +548,17 @@
public boolean hasSameAccountId(CurrentUser other) {
return getAccountId().get() == other.getAccountId().get();
}
+
+ protected String getCommitterName(Account ua, String email) {
+ String name = ua.fullName();
+ if (name == null || name.isEmpty()) {
+ final int at = email.indexOf('@');
+ if (0 < at) {
+ name = email.substring(0, at);
+ } else {
+ name = anonymousCowardName;
+ }
+ }
+ return name;
+ }
}
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 540e438..f46196f 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -58,6 +58,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import org.eclipse.jgit.diff.Sequence;
import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.lib.CommitBuilder;
@@ -504,7 +505,11 @@
if (committerIdent != null) {
cb.setCommitter(committerIdent);
} else {
- cb.setCommitter(ctx.newCommitterIdent());
+ PersonIdent committerIdent =
+ Optional.ofNullable(original.getCommitterIdent())
+ .map(ident -> ctx.newCommitterIdent(ident.getEmailAddress(), ctx.getIdentifiedUser()))
+ .orElseGet(ctx::newCommitterIdent);
+ cb.setCommitter(committerIdent);
}
if (matchAuthorToCommitterDate) {
cb.setAuthor(
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 7a50bdd..17d6212 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -177,9 +177,7 @@
import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs;
import com.google.gerrit.server.notedb.NoteDbModule;
import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
-import com.google.gerrit.server.patch.DiffFileSizeValidator;
import com.google.gerrit.server.patch.DiffOperationsImpl;
-import com.google.gerrit.server.patch.DiffValidator;
import com.google.gerrit.server.patch.PatchListCacheImpl;
import com.google.gerrit.server.patch.PatchScriptFactory;
import com.google.gerrit.server.patch.PatchScriptFactoryForAutoFix;
@@ -410,8 +408,6 @@
.to(SubmitRequirementConfigValidator.class);
DynamicSet.bind(binder(), CommitValidationListener.class).to(PrologRulesWarningValidator.class);
DynamicSet.setOf(binder(), CommentValidator.class);
- DynamicSet.setOf(binder(), DiffValidator.class);
- DynamicSet.bind(binder(), DiffValidator.class).to(DiffFileSizeValidator.class);
DynamicSet.setOf(binder(), ChangeMessageModifier.class);
DynamicSet.setOf(binder(), RefOperationValidationListener.class);
DynamicSet.setOf(binder(), OnSubmitValidationListener.class);
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 4c15a7e..68569f0 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -535,7 +535,7 @@
builder.setTreeId(tree);
builder.setParentIds(basePatchsetCommit.getParents());
builder.setAuthor(basePatchsetCommit.getAuthorIdent());
- builder.setCommitter(getCommitterIdent(timestamp));
+ builder.setCommitter(getCommitterIdent(basePatchsetCommit, timestamp));
builder.setMessage(commitMessage);
ObjectId newCommitId = objectInserter.insert(builder);
objectInserter.flush();
@@ -543,9 +543,14 @@
}
}
- private PersonIdent getCommitterIdent(Instant commitTimestamp) {
+ private PersonIdent getCommitterIdent(RevCommit basePatchsetCommit, Instant commitTimestamp) {
IdentifiedUser user = currentUser.get().asIdentifiedUser();
- return user.newCommitterIdent(commitTimestamp, zoneId);
+ return Optional.ofNullable(basePatchsetCommit.getCommitterIdent())
+ .map(
+ ident ->
+ user.newCommitterIdent(ident.getEmailAddress(), commitTimestamp, zoneId)
+ .orElseGet(() -> user.newCommitterIdent(commitTimestamp, zoneId)))
+ .orElseGet(() -> user.newCommitterIdent(commitTimestamp, zoneId));
}
/**
diff --git a/java/com/google/gerrit/server/git/LargeObjectException.java b/java/com/google/gerrit/server/git/LargeObjectException.java
index 145b631..04db42c 100644
--- a/java/com/google/gerrit/server/git/LargeObjectException.java
+++ b/java/com/google/gerrit/server/git/LargeObjectException.java
@@ -25,10 +25,6 @@
private static final long serialVersionUID = 1L;
- public LargeObjectException(String message) {
- super(message);
- }
-
public LargeObjectException(String message, org.eclipse.jgit.errors.LargeObjectException cause) {
super(message, cause);
}
diff --git a/java/com/google/gerrit/server/git/TagSet.java b/java/com/google/gerrit/server/git/TagSet.java
index a528c8f..bffc479 100644
--- a/java/com/google/gerrit/server/git/TagSet.java
+++ b/java/com/google/gerrit/server/git/TagSet.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.git;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableSet;
@@ -41,6 +43,7 @@
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevWalk;
import org.roaringbitmap.RoaringBitmap;
@@ -209,6 +212,7 @@
try (TagWalk rw = new TagWalk(git)) {
rw.setRetainBody(false);
+ RevFlag isTag = rw.newFlag("tag");
for (Ref ref :
git.getRefDatabase()
.getRefsByPrefixWithExclusions(RefDatabase.ALL, SKIPPABLE_REF_PREFIXES)) {
@@ -218,9 +222,9 @@
} else if (isTag(ref)) {
// For a tag, remember where it points to.
try {
- addTag(rw, git.getRefDatabase().peel(ref));
+ addTag(rw, git.getRefDatabase().peel(ref), isTag);
} catch (IOException e) {
- addTag(rw, ref);
+ addTag(rw, ref, isTag);
}
} else {
@@ -229,17 +233,10 @@
}
}
- // Traverse the complete history. Copy any flags from a commit to
- // all of its ancestors. This automatically updates any Tag object
- // as the TagCommit and the stored Tag object share the same
- // underlying bit set.
+ // Traverse the complete history and propagate reachability to parents.
TagCommit c;
while ((c = (TagCommit) rw.next()) != null) {
- RoaringBitmap mine = c.refFlags;
- int pCnt = c.getParentCount();
- for (int pIdx = 0; pIdx < pCnt; pIdx++) {
- ((TagCommit) c.getParent(pIdx)).refFlags.or(mine);
- }
+ c.propagateReachabilityToParents(isTag);
}
} catch (IOException e) {
logger.atWarning().withCause(e).log("Error building tags for repository %s", projectName);
@@ -356,9 +353,7 @@
refs.putAll(old.refs);
for (Tag srcTag : old.tags) {
- RoaringBitmap mine = new RoaringBitmap();
- mine.or(srcTag.refFlags);
- tags.add(new Tag(srcTag, mine));
+ tags.add(new Tag(srcTag));
}
for (TagMatcher.LostRef lost : m.lostRefs) {
@@ -369,7 +364,7 @@
}
}
- private void addTag(TagWalk rw, Ref ref) {
+ private void addTag(TagWalk rw, Ref ref, RevFlag isTag) {
ObjectId id = ref.getPeeledObjectId();
if (id == null) {
id = ref.getObjectId();
@@ -378,7 +373,12 @@
if (!tags.contains(id)) {
RoaringBitmap flags;
try {
- flags = ((TagCommit) rw.parseCommit(id)).refFlags;
+ TagCommit commit = ((TagCommit) rw.parseCommit(id));
+ commit.add(isTag);
+ if (commit.refFlags == null) {
+ commit.refFlags = new RoaringBitmap();
+ }
+ flags = commit.refFlags;
} catch (IncorrectObjectTypeException notCommit) {
flags = new RoaringBitmap();
} catch (IOException e) {
@@ -395,6 +395,9 @@
rw.markStart(commit);
int flag = refs.size();
+ if (commit.refFlags == null) {
+ commit.refFlags = new RoaringBitmap();
+ }
commit.refFlags.add(flag);
refs.put(ref.getName(), new CachedRef(ref, flag));
} catch (IncorrectObjectTypeException notCommit) {
@@ -432,8 +435,13 @@
// RoaringBitmap in TagCommit.refFlags.
@VisibleForTesting final RoaringBitmap refFlags;
+ Tag(Tag src) {
+ this(src, src.refFlags.clone());
+ }
+
Tag(AnyObjectId id, RoaringBitmap flags) {
super(id);
+ checkNotNull(flags);
this.refFlags = flags;
}
@@ -485,13 +493,50 @@
}
// TODO(hanwen): this would be better named as CommitWithReachability, as it also holds non-tags.
+ // However, non-tags will have a null refFlags field.
private static final class TagCommit extends RevCommit {
/** CachedRef.flag => isVisible, indicating if this commit is reachable from the ref. */
- final RoaringBitmap refFlags;
+ RoaringBitmap refFlags;
TagCommit(AnyObjectId id) {
super(id);
- refFlags = new RoaringBitmap();
+ }
+
+ /**
+ * Copy any flags from this commit to all of its ancestors.
+ *
+ * <p>Do not maintain a reference to the flags on non-tag commits after copying their flags to
+ * their ancestors. The flag copying automatically updates any Tag object as the TagCommit and
+ * the stored Tag object share the same underlying RoaringBitmap.
+ *
+ * @param isTag {@code RevFlag} indicating if this TagCommit is a tag
+ */
+ void propagateReachabilityToParents(RevFlag isTag) {
+ RoaringBitmap mine = refFlags;
+ if (mine != null) {
+ boolean canMoveBitmap = false;
+ if (!has(isTag)) {
+ refFlags = null;
+ canMoveBitmap = true;
+ }
+ int pCnt = getParentCount();
+ for (int pIdx = 0; pIdx < pCnt; pIdx++) {
+ TagCommit commit = (TagCommit) getParent(pIdx);
+ RoaringBitmap parentFlags = commit.refFlags;
+ if (parentFlags == null) {
+ if (canMoveBitmap) {
+ // This commit is not itself a Tag, so in order to reduce cloning overhead, migrate
+ // its refFlags object to its first parent with null refFlags
+ commit.refFlags = mine;
+ canMoveBitmap = false;
+ } else {
+ commit.refFlags = mine.clone();
+ }
+ } else {
+ parentFlags.or(mine);
+ }
+ }
+ }
}
}
}
diff --git a/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java b/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java
deleted file mode 100644
index 14a0f7b..0000000
--- a/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_CORE_SECTION;
-import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BIGFILE_THRESHOLD;
-import static org.eclipse.jgit.storage.pack.PackConfig.DEFAULT_BIG_FILE_THRESHOLD;
-
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-import com.google.inject.Inject;
-import org.eclipse.jgit.lib.Config;
-
-public class DiffFileSizeValidator implements DiffValidator {
- static final int DEFAULT_MAX_FILE_SIZE = DEFAULT_BIG_FILE_THRESHOLD;
- private static final String ERROR_MESSAGE =
- "File size for file %s exceeded the max file size threshold. Threshold = %d bytes, Actual size = %d bytes";
-
- final int maxFileSize;
-
- @Inject
- public DiffFileSizeValidator(@GerritServerConfig Config cfg) {
- this.maxFileSize =
- cfg.getInt(CONFIG_CORE_SECTION, CONFIG_KEY_BIGFILE_THRESHOLD, DEFAULT_MAX_FILE_SIZE);
- }
-
- @Override
- public void validate(FileDiffOutput fileDiff) throws LargeObjectException {
- if (fileDiff.size() > maxFileSize) {
- throw new LargeObjectException(
- String.format(ERROR_MESSAGE, fileDiff.getDefaultPath(), maxFileSize, fileDiff.size()));
- }
- if (fileDiff.sizeDelta() > maxFileSize) {
- throw new LargeObjectException(
- String.format(
- ERROR_MESSAGE, fileDiff.getDefaultPath(), maxFileSize, fileDiff.sizeDelta()));
- }
- }
-}
diff --git a/java/com/google/gerrit/server/patch/DiffValidator.java b/java/com/google/gerrit/server/patch/DiffValidator.java
deleted file mode 100644
index aee3c8b..0000000
--- a/java/com/google/gerrit/server/patch/DiffValidator.java
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import com.google.gerrit.extensions.annotations.ExtensionPoint;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-
-/** Interface to validate diff outputs. */
-@ExtensionPoint
-public interface DiffValidator {
- void validate(FileDiffOutput fileDiffOutput)
- throws LargeObjectException, DiffNotAvailableException;
-}
diff --git a/java/com/google/gerrit/server/patch/DiffValidators.java b/java/com/google/gerrit/server/patch/DiffValidators.java
deleted file mode 100644
index 964353d..0000000
--- a/java/com/google/gerrit/server/patch/DiffValidators.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-import com.google.inject.Inject;
-
-/** Validates {@link FileDiffOutput}(s) after they are computed by the {@link DiffOperations}. */
-public class DiffValidators {
- DynamicSet<DiffValidator> diffValidators;
-
- @Inject
- public DiffValidators(DynamicSet<DiffValidator> diffValidators) {
- this.diffValidators = diffValidators;
- }
-
- public void validate(FileDiffOutput fileDiffOutput)
- throws LargeObjectException, DiffNotAvailableException {
- for (DiffValidator validator : diffValidators) {
- validator.validate(fileDiffOutput);
- }
- }
-}
diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 5015c768..3baa3b1 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -94,7 +94,6 @@
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
private final DiffOperations diffOperations;
- private final DiffValidators diffValidators;
private final Change.Id changeId;
@@ -110,7 +109,6 @@
PermissionBackend permissionBackend,
ProjectCache projectCache,
DiffOperations diffOperations,
- DiffValidators diffValidators,
@Assisted ChangeNotes notes,
@Assisted String fileName,
@Assisted("patchSetA") @Nullable PatchSet.Id patchSetA,
@@ -126,7 +124,6 @@
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.diffOperations = diffOperations;
- this.diffValidators = diffValidators;
this.fileName = fileName;
this.psa = patchSetA;
@@ -147,7 +144,6 @@
PermissionBackend permissionBackend,
ProjectCache projectCache,
DiffOperations diffOperations,
- DiffValidators diffValidators,
@Assisted ChangeNotes notes,
@Assisted String fileName,
@Assisted int parentNum,
@@ -163,7 +159,6 @@
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.diffOperations = diffOperations;
- this.diffValidators = diffValidators;
this.fileName = fileName;
this.psa = null;
@@ -225,14 +220,13 @@
}
private PatchScript getPatchScript(Repository git, ObjectId aId, ObjectId bId)
- throws IOException, DiffNotAvailableException, LargeObjectException {
+ throws IOException, DiffNotAvailableException {
FileDiffOutput fileDiffOutput =
aId == null
? diffOperations.getModifiedFileAgainstParent(
notes.getProjectName(), bId, parentNum, fileName, diffPrefs.ignoreWhitespace)
: diffOperations.getModifiedFile(
notes.getProjectName(), aId, bId, fileName, diffPrefs.ignoreWhitespace);
- diffValidators.validate(fileDiffOutput);
return newBuilder().toPatchScript(git, fileDiffOutput);
}
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
index 9107dde..9286f47 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
@@ -64,10 +64,6 @@
*/
public abstract Optional<String> newPath();
- public String getDefaultPath() {
- return oldPath().isPresent() ? oldPath().get() : newPath().get();
- }
-
/**
* The file mode of the old file at the old git tree diff identified by {@link #oldCommitId()}
* ()}.
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index 6d4d74d..14e40bd 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -23,10 +23,12 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.DraftCommentsReader;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.HashtagsUtil;
import com.google.gerrit.server.index.change.ChangeField;
+import com.google.inject.ImplementedBy;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -61,12 +63,25 @@
return new ChangeIndexPredicate(ChangeField.COMMENTBY_SPEC, id.toString());
}
+ @ImplementedBy(IndexEditByPredicateProvider.class)
+ public interface EditByPredicateProvider {
+
+ /**
+ * Returns a predicate that matches changes where the provided {@link
+ * com.google.gerrit.entities.Account.Id} has a pending change edit.
+ */
+ Predicate<ChangeData> editBy(Account.Id id) throws QueryParseException;
+ }
+
/**
- * Returns a predicate that matches changes where the provided {@link
- * com.google.gerrit.entities.Account.Id} has a pending change edit.
+ * A default implementation of {@link EditByPredicateProvider}, based on th {@link
+ * ChangeField#EDITBY_SPEC} index field.
*/
- public static Predicate<ChangeData> editBy(Account.Id id) {
- return new ChangeIndexPredicate(ChangeField.EDITBY_SPEC, id.toString());
+ public static class IndexEditByPredicateProvider implements EditByPredicateProvider {
+ @Override
+ public Predicate<ChangeData> editBy(Account.Id id) {
+ return new ChangeIndexPredicate(ChangeField.EDITBY_SPEC, id.toString());
+ }
}
/**
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index b3fa087..f8a4a99 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -89,6 +89,7 @@
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ChildProjects;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.query.change.ChangePredicates.EditByPredicateProvider;
import com.google.gerrit.server.query.change.PredicateArgs.ValOp;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.submit.SubmitDryRun;
@@ -284,6 +285,8 @@
private final Provider<CurrentUser> self;
+ private final EditByPredicateProvider editByPredicateProvider;
+
@Inject
@VisibleForTesting
public Arguments(
@@ -318,7 +321,8 @@
ExperimentFeatures experimentFeatures,
HasOperandAliasConfig hasOperandAliasConfig,
ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory,
- PluginSetContext<SubmitRule> submitRules) {
+ PluginSetContext<SubmitRule> submitRules,
+ EditByPredicateProvider editByPredicateProvider) {
this(
queryProvider,
rewriter,
@@ -352,7 +356,8 @@
experimentFeatures,
hasOperandAliasConfig,
changeIsVisbleToPredicateFactory,
- submitRules);
+ submitRules,
+ editByPredicateProvider);
}
private Arguments(
@@ -388,7 +393,8 @@
ExperimentFeatures experimentFeatures,
HasOperandAliasConfig hasOperandAliasConfig,
ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory,
- PluginSetContext<SubmitRule> submitRules) {
+ PluginSetContext<SubmitRule> submitRules,
+ EditByPredicateProvider editByPredicateProvider) {
this.queryProvider = queryProvider;
this.rewriter = rewriter;
this.opFactories = opFactories;
@@ -422,6 +428,7 @@
this.experimentFeatures = experimentFeatures;
this.hasOperandAliasConfig = hasOperandAliasConfig;
this.submitRules = submitRules;
+ this.editByPredicateProvider = editByPredicateProvider;
}
public Arguments asUser(CurrentUser otherUser) {
@@ -458,7 +465,8 @@
experimentFeatures,
hasOperandAliasConfig,
changeIsVisbleToPredicateFactory,
- submitRules);
+ submitRules,
+ editByPredicateProvider);
}
Arguments asUser(Account.Id otherId) {
@@ -646,7 +654,7 @@
}
if ("edit".equalsIgnoreCase(value)) {
- return ChangePredicates.editBy(self());
+ return this.args.editByPredicateProvider.editBy(self());
}
if ("attention".equalsIgnoreCase(value)) {
@@ -1847,7 +1855,7 @@
}
/** Returns {@link com.google.gerrit.entities.Account.Id} of the identified calling user. */
- public Account.Id self() throws QueryParseException {
+ private Account.Id self() throws QueryParseException {
return args.getIdentifiedUser().getAccountId();
}
diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 3e471fb..5993c76 100644
--- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.index.query.Predicate.and;
import static com.google.gerrit.index.query.Predicate.not;
import static com.google.gerrit.index.query.Predicate.or;
+import static com.google.gerrit.server.query.change.ChangePredicates.EditByPredicateProvider;
import static com.google.gerrit.server.query.change.ChangeStatusPredicate.open;
import com.google.common.annotations.VisibleForTesting;
@@ -35,6 +36,7 @@
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.query.InternalQuery;
import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -75,8 +77,8 @@
return ChangeStatusPredicate.forStatus(status);
}
- private static Predicate<ChangeData> editBy(Account.Id accountId) {
- return ChangePredicates.editBy(accountId);
+ private Predicate<ChangeData> editBy(Account.Id accountId) throws QueryParseException {
+ return editByPredicateProvider.editBy(accountId);
}
private static Predicate<ChangeData> commit(String id) {
@@ -85,6 +87,7 @@
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
+ private final EditByPredicateProvider editByPredicateProvider;
@Inject
InternalChangeQuery(
@@ -92,10 +95,12 @@
ChangeIndexCollection indexes,
IndexConfig indexConfig,
ChangeData.Factory changeDataFactory,
- ChangeNotes.Factory notesFactory) {
+ ChangeNotes.Factory notesFactory,
+ EditByPredicateProvider editByPredicateProvider) {
super(queryProcessor, indexes, indexConfig);
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
+ this.editByPredicateProvider = editByPredicateProvider;
}
public List<ChangeData> byKey(Change.Key key) {
@@ -221,7 +226,7 @@
return query(and(ChangePredicates.exactTopic(topic), open()));
}
- public List<ChangeData> byOpenEditByUser(Account.Id accountId) {
+ public List<ChangeData> byOpenEditByUser(Account.Id accountId) throws QueryParseException {
return query(editBy(accountId));
}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteAccount.java b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
index 9b4c0a6..0831ff9 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java
@@ -26,6 +26,7 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.gpg.PublicKeyStoreUtil;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -166,7 +167,7 @@
}
}
- private void deleteChangeEdits(Account.Id accountId) throws IOException {
+ private void deleteChangeEdits(Account.Id accountId) throws IOException, QueryParseException {
// Note: in case of a stale index, the results of this query might be incomplete.
List<ChangeData> changesWithEdits = queryProvider.get().byOpenEditByUser(accountId);
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
index 75eaacf..6adde99 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
@@ -65,6 +65,7 @@
import java.time.Instant;
import java.time.ZoneId;
import java.util.List;
+import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
@@ -188,7 +189,14 @@
ObjectId treeId = applyResult.getTreeId();
Instant now = TimeUtil.now();
- PersonIdent committerIdent = user.get().newCommitterIdent(now, serverZoneId);
+ PersonIdent committerIdent =
+ Optional.ofNullable(latestPatchset.getCommitterIdent())
+ .map(
+ ident ->
+ user.get()
+ .newCommitterIdent(ident.getEmailAddress(), now, serverZoneId)
+ .orElseGet(() -> user.get().newCommitterIdent(now, serverZoneId)))
+ .orElseGet(() -> user.get().newCommitterIdent(now, serverZoneId));
PersonIdent authorIdent =
input.author == null
? committerIdent
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index 1bfb6bd..677279e 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -74,6 +74,7 @@
import java.time.ZoneId;
import java.util.HashSet;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
@@ -174,7 +175,8 @@
null,
null,
null,
- null);
+ null,
+ Optional.empty());
}
/**
@@ -205,7 +207,17 @@
throws IOException, InvalidChangeOperationException, UpdateException, RestApiException,
ConfigInvalidException, NoSuchProjectException {
return cherryPick(
- sourceChange, project, sourceCommit, input, dest, TimeUtil.now(), null, null, null, null);
+ sourceChange,
+ project,
+ sourceCommit,
+ input,
+ dest,
+ TimeUtil.now(),
+ null,
+ null,
+ null,
+ null,
+ Optional.empty());
}
/**
@@ -227,6 +239,9 @@
* @param idForNewChange The ID that the new change of the cherry pick will have. If provided and
* the cherry-pick doesn't result in creating a new change, then
* InvalidChangeOperationException is thrown.
+ * @param verifiedBaseCommit - base commit for the cherry-pick, which is guaranteed to be
+ * associated with exactly one change and belong to a {@code dest} branch. This is currently
+ * only used when this base commit was created in the same API call.
* @return Result object that describes the cherry pick.
* @throws IOException Unable to open repository or read from the database.
* @throws InvalidChangeOperationException Parent or branch don't exist, or two changes with same
@@ -247,7 +262,8 @@
@Nullable Change.Id revertedChange,
@Nullable ObjectId changeIdForNewChange,
@Nullable Change.Id idForNewChange,
- @Nullable Boolean workInProgress)
+ @Nullable Boolean workInProgress,
+ Optional<RevCommit> verifiedBaseCommit)
throws IOException, InvalidChangeOperationException, UpdateException, RestApiException,
ConfigInvalidException, NoSuchProjectException {
IdentifiedUser identifiedUser = user.get();
@@ -265,8 +281,9 @@
}
RevCommit baseCommit =
- CommitUtil.getBaseCommit(
- project.get(), queryProvider.get(), revWalk, destRef, input.base);
+ verifiedBaseCommit.orElse(
+ CommitUtil.getBaseCommit(
+ project.get(), queryProvider.get(), revWalk, destRef, input.base));
CodeReviewCommit commitToCherryPick = revWalk.parseCommit(sourceCommit);
@@ -306,8 +323,15 @@
CodeReviewCommit cherryPickCommit;
ProjectState projectState =
projectCache.get(dest.project()).orElseThrow(noSuchProject(dest.project()));
- PersonIdent committerIdent = identifiedUser.newCommitterIdent(timestamp, serverZoneId);
-
+ PersonIdent committerIdent =
+ Optional.ofNullable(commitToCherryPick.getCommitterIdent())
+ .map(
+ ident ->
+ identifiedUser
+ .newCommitterIdent(ident.getEmailAddress(), timestamp, serverZoneId)
+ .orElseGet(
+ () -> identifiedUser.newCommitterIdent(timestamp, serverZoneId)))
+ .orElseGet(() -> identifiedUser.newCommitterIdent(timestamp, serverZoneId));
try {
MergeUtil mergeUtil;
if (input.allowConflicts) {
diff --git a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
index ff21916..989dc7a 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
@@ -74,6 +74,7 @@
import java.time.Instant;
import java.time.ZoneId;
import java.util.List;
+import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
@@ -182,11 +183,18 @@
Instant now = TimeUtil.now();
IdentifiedUser me = user.get().asIdentifiedUser();
- PersonIdent committer = me.newCommitterIdent(now, serverZoneId);
PersonIdent author =
in.author == null
- ? committer
+ ? me.newCommitterIdent(now, serverZoneId)
: new PersonIdent(in.author.name, in.author.email, now, serverZoneId);
+ RevCommit commit = rw.parseCommit(ps.commitId());
+ PersonIdent committer =
+ Optional.ofNullable(commit.getCommitterIdent())
+ .map(
+ ident ->
+ me.newCommitterIdent(ident.getEmailAddress(), now, serverZoneId)
+ .orElseGet(() -> me.newCommitterIdent(now, serverZoneId)))
+ .orElseGet(() -> me.newCommitterIdent(now, serverZoneId));
CodeReviewCommit newCommit =
createMergeCommit(
in,
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 4eca1f3..3717e02 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
@@ -51,6 +52,7 @@
import java.io.IOException;
import java.time.Instant;
import java.time.ZoneId;
+import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
@@ -175,8 +177,15 @@
builder.setTreeId(basePatchSetCommit.getTree());
builder.setParentIds(basePatchSetCommit.getParents());
builder.setAuthor(basePatchSetCommit.getAuthorIdent());
- builder.setCommitter(
- userProvider.get().asIdentifiedUser().newCommitterIdent(timestamp, zoneId));
+ IdentifiedUser user = userProvider.get().asIdentifiedUser();
+ PersonIdent committer =
+ Optional.ofNullable(basePatchSetCommit.getCommitterIdent())
+ .map(
+ ident ->
+ user.newCommitterIdent(ident.getEmailAddress(), timestamp, zoneId)
+ .orElseGet(() -> user.newCommitterIdent(timestamp, zoneId)))
+ .orElseGet(() -> user.newCommitterIdent(timestamp, zoneId));
+ builder.setCommitter(committer);
builder.setMessage(commitMessage);
ObjectId newCommitId = objectInserter.insert(builder);
objectInserter.flush();
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 691fc75..3851e82 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -86,6 +86,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
+import java.util.Optional;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -310,6 +311,13 @@
cherryPickInput.message = revertInput.message;
ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
+ RevCommit baseCommit = null;
+ if (cherryPickInput.base != null) {
+ try (Repository git = repoManager.openRepository(changeNotes.getProjectName());
+ RevWalk revWalk = new RevWalk(git.newObjectReader())) {
+ baseCommit = revWalk.parseCommit(ObjectId.fromString(cherryPickInput.base));
+ }
+ }
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.now())) {
bu.setNotify(
@@ -323,7 +331,8 @@
generatedChangeId,
cherryPickRevertChangeId,
timestamp,
- revertInput.workInProgress));
+ revertInput.workInProgress,
+ baseCommit));
if (!revertInput.workInProgress) {
commitUtil.addChangeRevertedNotificationOps(
bu, changeNotes.getChangeId(), cherryPickRevertChangeId, generatedChangeId.name());
@@ -548,18 +557,21 @@
private final Change.Id cherryPickRevertChangeId;
private final Instant timestamp;
private final boolean workInProgress;
+ private final RevCommit baseCommit;
CreateCherryPickOp(
ObjectId revCommitId,
ObjectId computedChangeId,
Change.Id cherryPickRevertChangeId,
Instant timestamp,
- Boolean workInProgress) {
+ Boolean workInProgress,
+ RevCommit baseCommit) {
this.revCommitId = revCommitId;
this.computedChangeId = computedChangeId;
this.cherryPickRevertChangeId = cherryPickRevertChangeId;
this.timestamp = timestamp;
this.workInProgress = workInProgress;
+ this.baseCommit = baseCommit;
}
@Override
@@ -577,7 +589,8 @@
change.getId(),
computedChangeId,
cherryPickRevertChangeId,
- workInProgress);
+ workInProgress,
+ Optional.ofNullable(baseCommit));
// save the commit as base for next cherryPick of that branch
cherryPickInput.base =
changeNotesFactory
diff --git a/java/com/google/gerrit/server/submit/CherryPick.java b/java/com/google/gerrit/server/submit/CherryPick.java
index 0471b67..7fe5e69 100644
--- a/java/com/google/gerrit/server/submit/CherryPick.java
+++ b/java/com/google/gerrit/server/submit/CherryPick.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -102,8 +103,10 @@
RevCommit mergeTip = args.mergeTip.getCurrentTip();
args.rw.parseBody(mergeTip);
String cherryPickCmtMsg = args.mergeUtil.createCommitMessageOnSubmit(toMerge, mergeTip);
-
- PersonIdent committer = ctx.newCommitterIdent(args.caller);
+ PersonIdent committer =
+ Optional.ofNullable(toMerge.getCommitterIdent())
+ .map(ident -> ctx.newCommitterIdent(ident.getEmailAddress(), args.caller))
+ .orElseGet(() -> ctx.newCommitterIdent(args.caller));
try {
newCommit =
args.mergeUtil.createCherryPickFromCommit(
diff --git a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
index 5f58a74..87de810 100644
--- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
@@ -41,6 +41,7 @@
import java.io.IOException;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
@@ -167,7 +168,10 @@
RevCommit mergeTip = args.mergeTip.getCurrentTip();
args.rw.parseBody(mergeTip);
String cherryPickCmtMsg = args.mergeUtil.createCommitMessageOnSubmit(toMerge, mergeTip);
- PersonIdent committer = ctx.newCommitterIdent(args.caller);
+ PersonIdent committer =
+ Optional.ofNullable(toMerge.getCommitterIdent())
+ .map(ident -> ctx.newCommitterIdent(ident.getEmailAddress(), args.caller))
+ .orElseGet(() -> ctx.newCommitterIdent(args.caller));
try {
newCommit =
args.mergeUtil.createCherryPickFromCommit(
diff --git a/java/com/google/gerrit/server/update/Context.java b/java/com/google/gerrit/server/update/Context.java
index aa41d90..4e5d73f 100644
--- a/java/com/google/gerrit/server/update/Context.java
+++ b/java/com/google/gerrit/server/update/Context.java
@@ -164,4 +164,16 @@
default PersonIdent newCommitterIdent(IdentifiedUser user) {
return user.newCommitterIdent(getWhen(), getZoneId());
}
+
+ /**
+ * Creates a committer {@link PersonIdent} for the given user. The identity will be created with
+ * the given email if the user is allowed to use it, otherwise fallback to preferred email.
+ *
+ * @param user user for which a committer {@link PersonIdent} should be created
+ * @param email committer email of the source commit
+ * @return the created committer {@link PersonIdent}
+ */
+ default PersonIdent newCommitterIdent(String email, IdentifiedUser user) {
+ return user.newCommitterIdent(email, getWhen(), getZoneId()).orElseGet(this::newCommitterIdent);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
index b1cc866..d8c2aae 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
@@ -113,7 +113,7 @@
in.responseFormatOptions = ImmutableList.of(CURRENT_REVISION, CURRENT_COMMIT);
ChangeInfo result = gApi.changes().id(change.get()).applyPatch(in);
- assertThat(result.getCurrentRevision().commit.committer.email).isEqualTo(emailTwo);
+ assertThat(result.getCurrentRevision().commit.committer.email).isEqualTo(emailOne);
}
private static final String MODIFIED_FILE_NAME = "modified_file.txt";
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 3b0d240..3a835b7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3920,7 +3920,7 @@
gApi.changes().id(change.get()).setMessage(msg);
assertThat(gApi.changes().id(change.get()).get().getCurrentRevision().commit.committer.email)
- .isEqualTo(emailTwo);
+ .isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java b/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java
index 771935a..502f286 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java
@@ -188,7 +188,7 @@
gApi.changes().id(change.get()).createMergePatchSet(in);
assertThat(gApi.changes().id(change.get()).get().getCurrentRevision().commit.committer.email)
- .isEqualTo(emailTwo);
+ .isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
index 7fe79e4..7d1ddfc 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
@@ -257,7 +257,7 @@
.commit
.committer
.email)
- .isEqualTo(uploaderEmailTwo);
+ .isEqualTo(uploaderEmailOne);
assertThat(
gApi.changes()
.id(changeToBeRebased2.get())
@@ -266,7 +266,7 @@
.commit
.committer
.email)
- .isEqualTo(uploaderEmailTwo);
+ .isEqualTo(uploaderEmailOne);
assertThat(
gApi.changes()
.id(changeToBeRebased3.get())
@@ -275,7 +275,7 @@
.commit
.committer
.email)
- .isEqualTo(uploaderEmailTwo);
+ .isEqualTo(uploaderEmailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
index 152d9dd..d9b079a 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
@@ -237,7 +237,7 @@
// Rebase the second change
gApi.changes().id(c2.get()).rebase();
assertThat(gApi.changes().id(c2.get()).get().getCurrentRevision().commit.committer.email)
- .isEqualTo(emailTwo);
+ .isEqualTo(emailOne);
}
@Test
@@ -461,6 +461,12 @@
String changeId = r2.getChangeId();
requestScopeOperations.setApiUser(user.id());
rebaseCall.call(changeId);
+
+ // Verify that the committer has been updated
+ GitPerson committer =
+ gApi.changes().id(r2.getChangeId()).get().getCurrentRevision().commit.committer;
+ assertThat(committer.name).isEqualTo(user.fullName());
+ assertThat(committer.email).isEqualTo(user.email());
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
index 3456012..968c1f7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
@@ -282,7 +282,7 @@
.commit
.committer
.email)
- .isEqualTo(uploaderEmailTwo);
+ .isEqualTo(uploaderEmailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java
index e4dc0e83..84a4a40 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java
@@ -228,7 +228,7 @@
input.message = "cherry-pick to foo branch";
ChangeInfo cherryPickResult =
gApi.projects().name(project.get()).commit(commit).cherryPick(input).get();
- assertThat(cherryPickResult.getCurrentRevision().commit.committer.email).isEqualTo(emailTwo);
+ assertThat(cherryPickResult.getCurrentRevision().commit.committer.email).isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
index fa4f568..b9ef0bf 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
@@ -128,7 +128,7 @@
gApi.changes().id(change.get()).current().applyFix(applyProvidedFixInput);
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index cdcd044..b3db99f 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -385,7 +385,7 @@
input.message = "cherry-pick to foo branch";
ChangeInfo changeInfo =
gApi.changes().id(changeId.get()).revision(commit).cherryPick(input).get();
- assertThat(changeInfo.getCurrentRevision().commit.committer.email).isEqualTo(emailTwo);
+ assertThat(changeInfo.getCurrentRevision().commit.committer.email).isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 1ab74fb..b31d35c 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -796,7 +796,7 @@
gApi.changes().id(change.get()).current().applyFix(fixId);
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 6bff0be..9c691ae 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -291,7 +291,7 @@
gApi.changes().id(change.get()).edit().rebase();
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
@@ -365,7 +365,7 @@
gApi.changes().id(change.get()).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW));
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
@@ -530,7 +530,7 @@
gApi.changes().id(change.get()).edit().modifyCommitMessage(msg);
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
@@ -703,7 +703,7 @@
gApi.changes().id(change.get()).edit().deleteFile(FILE_NAME);
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
@@ -738,7 +738,7 @@
gApi.changes().id(change.get()).edit().renameFile(FILE_NAME, FILE_NAME3);
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
@@ -800,7 +800,7 @@
gApi.changes().id(change.get()).edit().restoreFile(FILE_NAME);
EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow();
- assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo);
+ assertThat(editInfo.commit.committer.email).isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 9e85d8c..c0531e5 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -875,6 +875,47 @@
"refs/tags/new-tag");
}
+ // rcMaster (c1 master master-tag) <- rcBranch (c2 branch branch-tag) <- rcBranch (c2 branch) <-
+ // newcommit1 <- newcommit2 (new-branch)
+ @Test
+ public void uploadPackReachableTagVisibleFromLeafBranch() throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ // rcBranch (c2 branch) <- newcommit1 (new-branch)
+ PushOneCommit.Result r =
+ pushFactory
+ .create(admin.newIdent(), testRepo)
+ .setParent(rcBranch)
+ .to("refs/heads/new-branch");
+ r.assertOkStatus();
+ RevCommit branchRc = r.getCommit();
+
+ // rcBranch (c2) <- newcommit1 <- newcommit2 (new-branch)
+ r =
+ pushFactory
+ .create(admin.newIdent(), testRepo)
+ .setParent(branchRc)
+ .to("refs/heads/new-branch");
+ r.assertOkStatus();
+ }
+
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(deny(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS))
+ .add(deny(Permission.READ).ref("refs/heads/branch").group(REGISTERED_USERS))
+ .add(allow(Permission.READ).ref("refs/heads/new-branch").group(REGISTERED_USERS))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ assertUploadPackRefs(
+ "refs/heads/new-branch",
+ // 'master' and 'branch' branches are not visible but 'master-tag' and 'branch-tag' are
+ // reachable from new-branch (since PushOneCommit always bases changes on each other).
+ "refs/tags/branch-tag",
+ "refs/tags/master-tag");
+ // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead.
+ }
+
// first ls-remote: rcBranch (c2 branch) <- newcommit1 (updated-tag)
// second ls-remote: rcBranch (c2 branch updated-tag)
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index 73e0d17..37684de 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -144,7 +144,7 @@
revision.review(ReviewInput.approve());
revision.submit();
assertThat(gApi.changes().id(changeId.get()).get().getCurrentRevision().commit.committer.email)
- .isEqualTo(emailTwo);
+ .isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
index a03a5b5..80fbe99 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
@@ -146,7 +146,7 @@
revision.review(ReviewInput.approve());
revision.submit();
assertThat(gApi.changes().id(changeId.get()).get().getCurrentRevision().commit.committer.email)
- .isEqualTo(emailTwo);
+ .isEqualTo(emailOne);
}
@Test
diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index ea8e0a7..90a9b9d 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -59,6 +59,7 @@
null,
null,
null,
+ null,
null));
}
diff --git a/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java b/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java
deleted file mode 100644
index fa1d09e..0000000
--- a/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java
+++ /dev/null
@@ -1,74 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.entities.Patch.ChangeType;
-import com.google.gerrit.entities.Patch.FileMode;
-import com.google.gerrit.entities.Patch.PatchType;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-import com.google.gerrit.testing.InMemoryModule;
-import com.google.inject.Guice;
-import com.google.inject.Inject;
-import com.google.inject.Injector;
-import java.util.Optional;
-import org.eclipse.jgit.lib.ObjectId;
-import org.junit.Before;
-import org.junit.Test;
-
-/** Test class for {@link DiffValidators}. */
-public class DiffValidatorsTest {
- @Inject private DiffValidators diffValidators;
-
- @Before
- public void setUpInjector() throws Exception {
- Injector injector = Guice.createInjector(new InMemoryModule());
- injector.injectMembers(this);
- }
-
- @Test
- public void fileSizeExceeded() {
- int largeSize = 100000000;
- FileDiffOutput fileDiff =
- FileDiffOutput.builder()
- .oldCommitId(ObjectId.zeroId())
- .newCommitId(ObjectId.zeroId())
- .comparisonType(ComparisonType.againstRoot())
- .changeType(ChangeType.ADDED)
- .patchType(Optional.of(PatchType.UNIFIED))
- .oldPath(Optional.empty())
- .newPath(Optional.of("f.txt"))
- .oldMode(Optional.empty())
- .newMode(Optional.of(FileMode.REGULAR_FILE))
- .headerLines(ImmutableList.of())
- .edits(ImmutableList.of())
- .size(largeSize)
- .sizeDelta(largeSize)
- .build();
- Exception thrown =
- assertThrows(LargeObjectException.class, () -> diffValidators.validate(fileDiff));
- assertThat(thrown)
- .hasMessageThat()
- .isEqualTo(
- String.format(
- "File size for file f.txt exceeded the max file size threshold."
- + " Threshold = %d bytes, Actual size = %d bytes",
- DiffFileSizeValidator.DEFAULT_MAX_FILE_SIZE, largeSize));
- }
-}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 57bb875..aa569ad 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -1025,23 +1025,17 @@
}
private actionsChanged() {
- this.hidden =
- Object.keys(this.actions).length === 0 &&
- Object.keys(this.revisionActions).length === 0 &&
- this.additionalActions.length === 0;
this.actionLoadingMessage = '';
this.disabledMenuActions = [];
- if (Object.keys(this.revisionActions).length !== 0) {
- if (!this.revisionActions.download) {
- this.revisionActions = {
- ...this.revisionActions,
- download: DOWNLOAD_ACTION,
- };
- fire(this, 'revision-actions-changed', {
- value: this.revisionActions,
- });
- }
+ if (!this.revisionActions.download) {
+ this.revisionActions = {
+ ...this.revisionActions,
+ download: DOWNLOAD_ACTION,
+ };
+ fire(this, 'revision-actions-changed', {
+ value: this.revisionActions,
+ });
}
if (
!this.actions.includedIn &&
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index ae60449..f083609 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -390,7 +390,7 @@
</gr-copy-clipboard>
</div>
<div class="commitActions">
- <gr-change-actions hidden="" id="actions"> </gr-change-actions>
+ <gr-change-actions id="actions"> </gr-change-actions>
</div>
</div>
<h2 class="assistive-tech-only">Change metadata</h2>
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 d053928..97b8de3 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
@@ -835,10 +835,14 @@
if (changedProperties.has('files')) {
this.filesChanged();
this.numFilesShown = Math.min(this.files.length, DEFAULT_NUM_FILES_SHOWN);
+ fire(this, 'files-shown-changed', {length: this.numFilesShown});
}
if (changedProperties.has('expandedFiles')) {
this.expandedFilesChanged(changedProperties.get('expandedFiles'));
}
+ if (changedProperties.has('numFilesShown')) {
+ fire(this, 'files-shown-changed', {length: this.numFilesShown});
+ }
}
override connectedCallback() {
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 bd81828..5fcafc3 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
@@ -395,7 +395,6 @@
this.serverConfig?.auth.use_contributor_agreements,
() => html`<li><a href="#Agreements">Agreements</a></li>`
)}
- <li><a href="#MailFilters">Mail Filters</a></li>
<gr-endpoint-decorator name="settings-menu-item">
</gr-endpoint-decorator>
</ul>
@@ -680,91 +679,6 @@
<gr-agreements-list id="agreementsList"></gr-agreements-list>
</fieldset>`
)}
- <h2 id="MailFilters">Mail Filters</h2>
- <fieldset class="filters">
- <p>
- Gerrit emails include metadata about the change to support writing
- mail filters.
- </p>
- <p>
- Here are some example Gmail queries that can be used for filters
- or for searching through archived messages. View the
- <a
- href=${this.getFilterDocsLink(this.docsBaseUrl)}
- target="_blank"
- rel="noopener noreferrer"
- >Gerrit documentation</a
- >
- for the complete set of footers.
- </p>
- <table>
- <tbody>
- <tr>
- <th>Name</th>
- <th>Query</th>
- </tr>
- <tr>
- <td>Changes requesting my review</td>
- <td>
- <code class="queryExample">
- "Gerrit-Reviewer: <em>Your Name</em>
- <<em>your.email@example.com</em>>"
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes requesting my attention</td>
- <td>
- <code class="queryExample">
- "Gerrit-Attention: <em>Your Name</em>
- <<em>your.email@example.com</em>>"
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes from a specific owner</td>
- <td>
- <code class="queryExample">
- "Gerrit-Owner: <em>Owner name</em>
- <<em>owner.email@example.com</em>>"
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes targeting a specific branch</td>
- <td>
- <code class="queryExample">
- "Gerrit-Branch: <em>branch-name</em>"
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes in a specific project</td>
- <td>
- <code class="queryExample">
- "Gerrit-Project: <em>project-name</em>"
- </code>
- </td>
- </tr>
- <tr>
- <td>Messages related to a specific Change ID</td>
- <td>
- <code class="queryExample">
- "Gerrit-Change-Id: <em>Change ID</em>"
- </code>
- </td>
- </tr>
- <tr>
- <td>Messages related to a specific change number</td>
- <td>
- <code class="queryExample">
- "Gerrit-Change-Number: <em>change number</em>"
- </code>
- </td>
- </tr>
- </tbody>
- </table>
- </fieldset>
<gr-endpoint-decorator name="settings-screen">
</gr-endpoint-decorator>
</div>
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
index d77d35c..30a2922 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
@@ -148,7 +148,6 @@
<li><a href="#EmailAddresses"> Email Addresses </a></li>
<li><a href="#Groups"> Groups </a></li>
<li><a href="#Identities"> Identities </a></li>
- <li><a href="#MailFilters"> Mail Filters </a></li>
<gr-endpoint-decorator name="settings-menu-item">
</gr-endpoint-decorator>
</ul>
@@ -454,92 +453,6 @@
<fieldset>
<gr-identities id="identities"> </gr-identities>
</fieldset>
- <h2 id="MailFilters">Mail Filters</h2>
- <fieldset class="filters">
- <p>
- Gerrit emails include metadata about the change to support writing
- mail filters.
- </p>
- <p>
- Here are some example Gmail queries that can be used for filters
- or for searching through archived messages. View the
- <a
- href="https://test.com/user-notify.html"
- rel="noopener noreferrer"
- target="_blank"
- >
- Gerrit documentation
- </a>
- for the complete set of footers.
- </p>
- <table>
- <tbody>
- <tr>
- <th>Name</th>
- <th>Query</th>
- </tr>
- <tr>
- <td>Changes requesting my review</td>
- <td>
- <code class="queryExample">
- "Gerrit-Reviewer: <em> Your Name </em> <
- <em> your.email@example.com </em> >"
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes requesting my attention</td>
- <td>
- <code class="queryExample">
- "Gerrit-Attention: <em> Your Name </em> <
- <em> your.email@example.com </em> >"
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes from a specific owner</td>
- <td>
- <code class="queryExample">
- "Gerrit-Owner: <em> Owner name </em> <
- <em> owner.email@example.com </em> >"
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes targeting a specific branch</td>
- <td>
- <code class="queryExample">
- "Gerrit-Branch: <em> branch-name </em> "
- </code>
- </td>
- </tr>
- <tr>
- <td>Changes in a specific project</td>
- <td>
- <code class="queryExample">
- "Gerrit-Project: <em> project-name </em> "
- </code>
- </td>
- </tr>
- <tr>
- <td>Messages related to a specific Change ID</td>
- <td>
- <code class="queryExample">
- "Gerrit-Change-Id: <em> Change ID </em> "
- </code>
- </td>
- </tr>
- <tr>
- <td>Messages related to a specific change number</td>
- <td>
- <code class="queryExample">
- "Gerrit-Change-Number: <em> change number </em> "
- </code>
- </td>
- </tr>
- </tbody>
- </table>
- </fieldset>
<gr-endpoint-decorator name="settings-screen">
</gr-endpoint-decorator>
</div>
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index d27be9f..e2229b5 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -1187,7 +1187,10 @@
async createSuggestEdit(e: MouseEvent) {
e.stopPropagation();
const line = await this.getCommentedCode();
- this.messageText += `${USER_SUGGESTION_START_PATTERN}${line}${'\n```'}`;
+ const addNewLine = this.messageText.length !== 0;
+ this.messageText += `${
+ addNewLine ? '\n' : ''
+ }${USER_SUGGESTION_START_PATTERN}${line}${'\n```'}`;
}
async getCommentedCode() {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
index 70e653a..e557ca8 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
@@ -147,20 +147,17 @@
test('move action button to overflow', async () => {
const key = changeActions.add(ActionType.REVISION, 'Bork!');
await element.updateComplete;
- assert.isTrue(queryAndAssert<GrDropdown>(element, '#moreActions').hidden);
- assert.isOk(
- queryAndAssert<GrButton>(element, `[data-action-key="${key}"]`)
- );
+
+ let items = queryAndAssert<GrDropdown>(element, '#moreActions').items;
+ assert.isFalse(items?.some(item => item.name === 'Bork!'));
+ assert.isOk(query<GrButton>(element, `[data-action-key="${key}"]`));
+
changeActions.setActionOverflow(ActionType.REVISION, key, true);
await element.updateComplete;
+
+ items = queryAndAssert<GrDropdown>(element, '#moreActions').items;
+ assert.isTrue(items?.some(item => item.name === 'Bork!'));
assert.isNotOk(query<GrButton>(element, `[data-action-key="${key}"]`));
- assert.isFalse(
- queryAndAssert<GrDropdown>(element, '#moreActions').hidden
- );
- assert.strictEqual(
- queryAndAssert<GrDropdown>(element, '#moreActions').items![0].name,
- 'Bork!'
- );
});
test('change actions priority', async () => {
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 c29417e..4afdf00 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -122,9 +122,11 @@
private changeNum?: NumericChangeId;
+ // Represents the current location of the ':' or '@' that triggered a drop-down.
// private but used in tests
specialCharIndex = -1;
+ // Represents the current search string being used to query either emoji or mention suggestions.
// private but used in tests
currentSearchString?: string;
@@ -280,8 +282,7 @@
this.fireChangedEvents();
// Add to updated because we want this.textarea.selectionStart and
// this.textarea is null in the willUpdate lifecycle
- this.computeSpecialCharIndex();
- this.computeCurrentSearchString();
+ this.computeIndexAndSearchString();
this.handleTextChanged();
}
}
@@ -373,6 +374,13 @@
return;
}
+ const selection = this.getVisibleDropdown().getCurrentText();
+ if (selection === '') {
+ // Nothing was selected, so treat this like a newline and reset the dropdown.
+ this.indent(e);
+ this.resetDropdown();
+ return;
+ }
e.preventDefault();
e.stopPropagation();
this.setValue(this.getVisibleDropdown().getCurrentText());
@@ -402,17 +410,17 @@
// below needs to happen after iron-autogrow-textarea has set the
// incorrect value.
await this.updateComplete;
- this.textarea!.selectionStart = specialCharIndex + 1;
- this.textarea!.selectionEnd = specialCharIndex + 1;
+ this.textarea!.selectionStart = specialCharIndex + text.length + 1;
+ this.textarea!.selectionEnd = specialCharIndex + text.length + 1;
this.resetDropdown();
}
private addValueToText(value: string) {
if (!this.text) return '';
return (
- this.text.substr(0, this.specialCharIndex ?? 0) +
+ this.text.substring(0, this.specialCharIndex ?? 0) +
value +
- this.text.substr(this.textarea!.selectionStart)
+ this.text.substring(this.textarea!.selectionStart)
);
}
@@ -425,7 +433,7 @@
*/
updateCaratPosition() {
if (typeof this.textarea!.value === 'string') {
- this.hiddenText!.textContent = this.textarea!.value.substr(
+ this.hiddenText!.textContent = this.textarea!.value.substring(
0,
this.textarea!.selectionStart
);
@@ -436,12 +444,7 @@
return caratSpan;
}
- private shouldResetDropdown(
- text: string,
- charIndex: number,
- suggestions?: Item[],
- char?: string
- ) {
+ private shouldResetDropdown(text: string, charIndex: number, char?: string) {
// Under any of the following conditions, close and reset the dropdown:
// - The cursor is no longer at the end of the current search string
// - The search string is an space or new line
@@ -452,32 +455,10 @@
(this.currentSearchString ?? '').length + charIndex + 1 ||
this.currentSearchString === ' ' ||
this.currentSearchString === '\n' ||
- !(text[charIndex] === char) ||
- !suggestions ||
- !suggestions.length
+ !(text[charIndex] === char)
);
}
- // When special char is detected, set index. We are interested only on
- // special char after space or in beginning of textarea
- // In case of mentions we are interested if previous char is '\n' as well
- private getSpecialCharIndex(text: string) {
- const charAtCursor = text[this.textarea!.selectionStart - 1];
- if (
- this.textarea!.selectionStart < 2 ||
- text[this.textarea!.selectionStart - 2] === ' '
- ) {
- return this.textarea!.selectionStart - 1;
- }
- if (
- charAtCursor === '@' &&
- text[this.textarea!.selectionStart - 2] === '\n'
- ) {
- return this.textarea!.selectionStart - 1;
- }
- return -1;
- }
-
private async computeSuggestions() {
this.suggestions = [];
if (this.currentSearchString === undefined) {
@@ -513,7 +494,6 @@
this.shouldResetDropdown(
this.text,
this.specialCharIndex,
- this.suggestions,
this.text[this.specialCharIndex]
)
) {
@@ -539,26 +519,20 @@
);
}
- private computeSpecialCharIndex() {
- const charAtCursor = this.text[this.textarea!.selectionStart - 1];
-
- if (charAtCursor === '@' && this.specialCharIndex === -1) {
- this.specialCharIndex = this.getSpecialCharIndex(this.text);
- }
- if (charAtCursor === ':' && this.specialCharIndex === -1) {
- this.specialCharIndex = this.getSpecialCharIndex(this.text);
- }
- }
-
- private computeCurrentSearchString() {
- if (this.specialCharIndex === -1) {
+ private computeIndexAndSearchString() {
+ const currentCarat = this.textarea?.selectionStart ?? this.text.length;
+ const m = this.text
+ .substring(0, currentCarat)
+ .match(/(?:^|\s)([:@][\S]*)$/);
+ if (!m) {
+ this.specialCharIndex = -1;
this.currentSearchString = undefined;
return;
}
- this.currentSearchString = this.text.substr(
- this.specialCharIndex + 1,
- this.textarea!.selectionStart - this.specialCharIndex - 1
- );
+ this.currentSearchString = m[1].substring(1);
+ if (this.specialCharIndex !== -1) return;
+
+ this.specialCharIndex = currentCarat - m[1].length;
}
// Private but used in tests.
@@ -645,7 +619,7 @@
// When nothing is selected, selectionStart is the caret position. We want
// the indentation level of the current line, not the end of the text which
// may be different.
- const currentLine = this.textarea!.textarea.value.substr(
+ const currentLine = this.textarea!.textarea.value.substring(
0,
this.textarea!.selectionStart
)
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
index 4dcaa80..aa32f7d 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
@@ -220,20 +220,6 @@
]);
});
- test('emoji selector does not open when previous char is \n', async () => {
- element.textarea!.focus();
- await waitUntil(() => element.textarea!.focused === true);
-
- element.textarea!.selectionStart = 1;
- element.textarea!.selectionEnd = 1;
- element.text = '\n:';
-
- await element.updateComplete;
-
- assert.isTrue(element.emojiSuggestions!.isHidden);
- assert.isTrue(element.mentionsSuggestions!.isHidden);
- });
-
test('selecting mentions from dropdown', async () => {
stubRestApi('getSuggestedAccounts').returns(
Promise.resolve([
@@ -300,19 +286,19 @@
assert.isTrue(element.emojiSuggestions!.isHidden);
assert.isFalse(element.mentionsSuggestions!.isHidden);
- element.text = '@h ';
+ element.text = '@h';
await waitUntil(() => element.suggestions.length > 0);
await element.updateComplete;
assert.isTrue(element.emojiSuggestions!.isHidden);
assert.isFalse(element.mentionsSuggestions!.isHidden);
- element.text = '@h :';
+ element.text = '@h:';
await waitUntil(() => element.suggestions.length > 0);
await element.updateComplete;
assert.isTrue(element.emojiSuggestions!.isHidden);
assert.isFalse(element.mentionsSuggestions!.isHidden);
- element.text = '@h :D';
+ element.text = '@h:D';
await waitUntil(() => element.suggestions.length > 0);
await element.updateComplete;
assert.isTrue(element.emojiSuggestions!.isHidden);
@@ -347,11 +333,16 @@
element.text = ':D@';
await element.updateComplete;
// emoji dropdown hidden since we have no more suggestions
- assert.isTrue(element.emojiSuggestions!.isHidden);
+ assert.isFalse(element.emojiSuggestions!.isHidden);
assert.isTrue(element.mentionsSuggestions!.isHidden);
element.text = ':D@b';
await element.updateComplete;
+ assert.isFalse(element.emojiSuggestions!.isHidden);
+ assert.isTrue(element.mentionsSuggestions!.isHidden);
+
+ element.text = ':D@b ';
+ await element.updateComplete;
assert.isTrue(element.emojiSuggestions!.isHidden);
assert.isTrue(element.mentionsSuggestions!.isHidden);
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 5d23da1..b3c7dad 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -319,9 +319,14 @@
if (this.diffElement) {
this.highlights.init(this.diffElement, this);
}
+ this.observeNodes();
}
override disconnectedCallback() {
+ if (this.nodeObserver) {
+ this.nodeObserver.disconnect();
+ this.nodeObserver = undefined;
+ }
this.removeSelectionListeners();
this.diffSelection.cleanup();
this.highlights.cleanup();
@@ -422,7 +427,6 @@
if (changedProperties.has('groups')) {
if (this.groups?.length > 0) {
this.loading = false;
- this.observeNodes();
}
}
}
diff --git a/tools/bzl/classpath.bzl b/tools/bzl/classpath.bzl
index 3be7a12..3c80fc3 100644
--- a/tools/bzl/classpath.bzl
+++ b/tools/bzl/classpath.bzl
@@ -2,7 +2,7 @@
all = []
for d in ctx.attr.deps:
if JavaInfo in d:
- all.append(d[JavaInfo].transitive_runtime_deps)
+ all.append(d[JavaInfo].transitive_runtime_jars)
if hasattr(d[JavaInfo].compilation_info, "runtime_classpath"):
all.append(d[JavaInfo].compilation_info.runtime_classpath)
elif hasattr(d, "files"):
diff --git a/tools/bzl/javadoc.bzl b/tools/bzl/javadoc.bzl
index 3add025..a181104 100644
--- a/tools/bzl/javadoc.bzl
+++ b/tools/bzl/javadoc.bzl
@@ -17,7 +17,7 @@
def _impl(ctx):
zip_output = ctx.outputs.zip
- transitive_jars = depset(transitive = [j[JavaInfo].transitive_deps for j in ctx.attr.libs])
+ transitive_jars = depset(transitive = [j[JavaInfo].transitive_compile_time_jars for j in ctx.attr.libs])
# TODO(davido): Remove list to depset conversion on source_jars, when this issue is fixed:
# https://github.com/bazelbuild/bazel/issues/4221
diff --git a/tools/bzl/pkg_war.bzl b/tools/bzl/pkg_war.bzl
index e2be145..4792de2 100644
--- a/tools/bzl/pkg_war.bzl
+++ b/tools/bzl/pkg_war.bzl
@@ -88,7 +88,7 @@
transitive_libs = []
for j in ctx.attr.libs:
if JavaInfo in j:
- transitive_libs.append(j[JavaInfo].transitive_runtime_deps)
+ transitive_libs.append(j[JavaInfo].transitive_runtime_jars)
elif hasattr(j, "files"):
transitive_libs.append(j.files)
@@ -102,7 +102,7 @@
# Add pgm lib
transitive_pgmlibs = []
for j in ctx.attr.pgmlibs:
- transitive_pgmlibs.append(j[JavaInfo].transitive_runtime_deps)
+ transitive_pgmlibs.append(j[JavaInfo].transitive_runtime_jars)
transitive_pgmlib_deps = depset(transitive = transitive_pgmlibs)
for dep in transitive_pgmlib_deps.to_list():
@@ -117,7 +117,7 @@
if ctx.attr.context:
for jar in ctx.attr.context:
if JavaInfo in jar:
- transitive_context_libs.append(jar[JavaInfo].transitive_runtime_deps)
+ transitive_context_libs.append(jar[JavaInfo].transitive_runtime_jars)
elif hasattr(jar, "files"):
transitive_context_libs.append(jar.files)
