Merge "ChangeQueryBuilder#label: Reject using 'user' and 'group' args together"
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index dab63f6..66c6531 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -2397,7 +2397,6 @@
|Field Name|Description
|`url` |The URL to the avatar image.
|`height` |The height of the avatar image in pixels.
-|`width` |The width of the avatar image in pixels.
|======================
[[capability-info]]
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 7b6e5c5..6b36749 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -3270,7 +3270,8 @@
PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit/foo HTTP/1.0
----
-To upload a file as binary data in the request body:
+To upload a file as binary data a link:#file-content-input[FileContentInput]
+entity can be specified in the request body:
.Request
----
@@ -3283,17 +3284,10 @@
}
----
-Note that it must be base-64 encoded data uri.
+When a change edit doesn't exist for this change yet it is created
+automatically.
-The "file_mode" field is optional, and if provided must be in octal format. The field
-indicates whether the file is executable or not and has a value of either 100755
-(executable) or 100644 (not executable). If it's unset, this indicates no change
-has been made. New files default to not being executable if this parameter is not
-provided
-
-When change edit doesn't exist for this change yet it is created. When file
-content isn't provided, it is wiped out for that file. As response
-"`204 No Content`" is returned.
+As response "`204 No Content`" is returned:
.Response
----
@@ -8260,6 +8254,24 @@
Only set if link:#download-commands[download commands] are requested.
|==========================
+[[file-content-input]]
+=== FileContentInput
+The `FileContentInput` entity contains information for creating/updating a file
+in a change edit.
+
+[options="header",cols="1,^1,5"]
+|==============================
+|Field Name ||Description
+|`binary_content` |optional|The file content as a base-64 encoded data URI. If
+no content is provided, an empty is created or if an existing file is updated
+the file content is removed so that the file becomes empty. The content must be
+a SHA1 if the file mode is `160000` (gitlink).
+|`file_mode` |optional|The file mode in octal format. Supported values are
+`100644` (regular file), `100755` (executable file), `120000` (symlink) and
+`160000` (gitlink). If unset, new files are created with file mode `100644`
+(regular file) and for existing files the existing file mode is kept.
+|==============================
+
[[file-info]]
=== FileInfo
The `FileInfo` entity contains information about a file in a patch set.
diff --git a/Documentation/rest-api-groups.txt b/Documentation/rest-api-groups.txt
index c43ebf2..5e9136b 100644
--- a/Documentation/rest-api-groups.txt
+++ b/Documentation/rest-api-groups.txt
@@ -507,6 +507,47 @@
UUID was specified and the UUID is already in use, the response is
"`409 Conflict`".
+[[delete-group]]
+=== Delete Group
+--
+'DELETE /groups/link:#group-id[\{group-id\}]'
+--
+
+Delete group.
+The group to delete must be internal group.
+
+This endpoint is only allowed for Gerrit's internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
+.Request
+----
+ DELETE /groups/MyProject-Committers HTTP/1.0
+----
+
+.Response
+----
+ HTTP/1.1 204 No Content
+----
+
+[[delete-internal-group]]
+=== Delete Internal Group
+--
+'POST /groups/link:#group-id[\{group-id\}].delete'
+--
+
+Delete group.
+The deleted group must be internal group.
+
+.Request
+----
+ POST /groups/MyProject-Committers.delete HTTP/1.0
+----
+
+.Response
+----
+ HTTP/1.1 204 No Content
+----
+
[[get-group-detail]]
=== Get Group Detail
--
@@ -1479,47 +1520,6 @@
HTTP/1.1 204 No Content
----
-[[delete-group]]
-=== Delete Group
---
-'DELETE /groups/link:#group-id[\{group-id\}]'
---
-
-Delete group.
-The group to delete must be internal group.
-
-This endpoint is only allowed for Gerrit's internal groups; attempting to call on a
-non-internal group will return 405 Method Not Allowed.
-
-.Request
-----
- DELETE /groups/MyProject-Committers HTTP/1.0
-----
-
-.Response
-----
- HTTP/1.1 204 No Content
-----
-
-[[delete-internal-group]]
-=== Delete Internal Group
---
-'POST /groups/link:#group-id[\{group-id\}].delete'
---
-
-Delete group.
-The deleted group must be internal group.
-
-.Request
-----
- POST /groups/MyProject-Committers.delete HTTP/1.0
-----
-
-.Response
-----
- HTTP/1.1 204 No Content
-----
-
[[ids]]
== IDs
diff --git a/SUBMITTING_PATCHES b/SUBMITTING_PATCHES
index 14437da..8a5b785 100644
--- a/SUBMITTING_PATCHES
+++ b/SUBMITTING_PATCHES
@@ -49,6 +49,16 @@
your patch. It is virtually impossible to remove a patch once it
has been applied and pushed out.
+In order to contribute to Gerrit a Contributor License Agreement (CLA)
+must be completed before contributions are accepted, see
+https://gerrit-review.googlesource.com/Documentation/dev-cla.html.
+
+All submissions to Google Open Source projects need to follow Google’s
+Contributor License Agreement (CLA), in which contributors agree that
+their contribution is an original work of authorship. This doesn’t
+prohibit the use of coding assistance tools, but what’s submitted does
+need to be a contributor’s original creation.
+
(3) Sending your patches.
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 975567f..6f6b7c2 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -409,6 +409,7 @@
RefNames.REFS_GROUPS + "*",
RefNames.REFS_STARRED_CHANGES + "*",
RefNames.REFS_DRAFT_COMMENTS + "*")
+ .deleteNewProjects()
.build();
}
diff --git a/java/com/google/gerrit/acceptance/ProjectResetter.java b/java/com/google/gerrit/acceptance/ProjectResetter.java
index c71641b..dcbcad4 100644
--- a/java/com/google/gerrit/acceptance/ProjectResetter.java
+++ b/java/com/google/gerrit/acceptance/ProjectResetter.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.index.group.GroupIndexer;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RefPatternMatcher;
+import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.Arrays;
@@ -49,6 +50,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -134,28 +136,35 @@
groupIncludeCache,
groupIndexer,
projectCache,
- input.refsByProject);
+ input.refsByProject,
+ input.deleteNewProjects);
}
}
public static class Config {
private final ImmutableMultimap<Project.NameKey, String> refsByProject;
+ private final boolean deleteNewProjects;
- private Config(ImmutableMultimap<Project.NameKey, String> refsByProject) {
+ private Config(
+ ImmutableMultimap<Project.NameKey, String> refsByProject, boolean deleteNewProjects) {
this.refsByProject = refsByProject;
+ this.deleteNewProjects = deleteNewProjects;
}
public Builder toBuilder() {
Builder builder = new Builder();
builder.refsByProject.putAll(refsByProject);
+ builder.deleteNewProjects.set(deleteNewProjects);
return builder;
}
public static class Builder {
private final ListMultimap<Project.NameKey, String> refsByProject;
+ private final AtomicBoolean deleteNewProjects;
public Builder() {
this.refsByProject = MultimapBuilder.hashKeys().arrayListValues().build();
+ this.deleteNewProjects = new AtomicBoolean(false);
}
public Builder reset(Project.NameKey project, String... refPatterns) {
@@ -167,8 +176,13 @@
return this;
}
+ public Builder deleteNewProjects() {
+ deleteNewProjects.set(true);
+ return this;
+ }
+
public Config build() {
- return new Config(ImmutableMultimap.copyOf(refsByProject));
+ return new Config(ImmutableMultimap.copyOf(refsByProject), deleteNewProjects.get());
}
}
}
@@ -184,9 +198,11 @@
@Inject @Nullable private ProjectCache projectCache;
private final Multimap<Project.NameKey, String> refsPatternByProject;
+ private final boolean deleteNewProjects;
// State to which to reset to.
private final ListMultimap<Project.NameKey, RefState> savedRefStatesByProject;
+ private final Set<Project.NameKey> savedProjectSet;
// Results of the resetting
private ListMultimap<Project.NameKey, String> keptRefsByProject;
@@ -203,7 +219,8 @@
@Nullable GroupIncludeCache groupIncludeCache,
@Nullable GroupIndexer groupIndexer,
@Nullable ProjectCache projectCache,
- Multimap<Project.NameKey, String> refPatternByProject)
+ Multimap<Project.NameKey, String> refPatternByProject,
+ boolean deleteNewProjects)
throws IOException {
this.repoManager = repoManager;
this.allUsersName = allUsersName;
@@ -215,7 +232,10 @@
this.groupIncludeCache = groupIncludeCache;
this.projectCache = projectCache;
this.refsPatternByProject = refPatternByProject;
+ this.deleteNewProjects = deleteNewProjects;
this.savedRefStatesByProject = readRefStates();
+ this.savedProjectSet =
+ repoManager instanceof InMemoryRepositoryManager ? repoManager.list() : projectCache.all();
}
@Override
@@ -227,6 +247,7 @@
() -> {
restoreRefs();
deleteNewlyCreatedRefs();
+ deleteNewlyCreatedProjects();
evictCachesAndReindex();
});
}
@@ -312,6 +333,18 @@
}
}
+ private void deleteNewlyCreatedProjects() {
+ if (deleteNewProjects && repoManager instanceof InMemoryRepositoryManager) {
+ InMemoryRepositoryManager inMemoryRepoManager = (InMemoryRepositoryManager) repoManager;
+ Sets.difference(inMemoryRepoManager.list(), savedProjectSet)
+ .forEach(
+ project -> {
+ inMemoryRepoManager.deleteRepository(project);
+ projectCache.evictAndReindex(project);
+ });
+ }
+ }
+
private void evictCachesAndReindex() throws IOException {
evictAndReindexProjects();
evictAndReindexAccounts();
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
index 3d74477..1b80d9e 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
@@ -62,7 +62,9 @@
* .createV2();
* </pre>
*
- * <p><strong>Note:</strong> There must be at least one existing user and repository.
+ * <p><strong>Note:</strong> There must be at least one existing user.
+ *
+ * <p><strong>Note:</strong> If a project is not specified the change is created in a new project.
*
* @return a builder to create the new change
*/
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index 87e76d5..dc3d947 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
@@ -42,7 +43,6 @@
import com.google.gerrit.server.edit.tree.TreeModification;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.update.context.RefUpdateContext;
@@ -52,7 +52,6 @@
import java.io.IOException;
import java.time.Instant;
import java.util.Arrays;
-import java.util.Objects;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.AnyObjectId;
@@ -84,11 +83,11 @@
private final IdentifiedUser.GenericFactory userFactory;
private final PersonIdent serverIdent;
private final BatchUpdate.Factory batchUpdateFactory;
- private final ProjectCache projectCache;
private final ChangeFinder changeFinder;
private final PerPatchsetOperationsImpl.Factory perPatchsetOperationsFactory;
private final PerCommentOperationsImpl.Factory perCommentOperationsFactory;
private final PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory;
+ private final ProjectOperations projectOperations;
@Inject
public ChangeOperationsImpl(
@@ -100,11 +99,11 @@
IdentifiedUser.GenericFactory userFactory,
@GerritPersonIdent PersonIdent serverIdent,
BatchUpdate.Factory batchUpdateFactory,
- ProjectCache projectCache,
ChangeFinder changeFinder,
PerPatchsetOperationsImpl.Factory perPatchsetOperationsFactory,
PerCommentOperationsImpl.Factory perCommentOperationsFactory,
- PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory) {
+ PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory,
+ ProjectOperations projectOperations) {
this.seq = seq;
this.changeInserterFactory = changeInserterFactory;
this.patchsetInserterFactory = patchsetInserterFactory;
@@ -113,11 +112,11 @@
this.userFactory = userFactory;
this.serverIdent = serverIdent;
this.batchUpdateFactory = batchUpdateFactory;
- this.projectCache = projectCache;
this.changeFinder = changeFinder;
this.perPatchsetOperationsFactory = perPatchsetOperationsFactory;
this.perCommentOperationsFactory = perCommentOperationsFactory;
this.perDraftCommentOperationsFactory = perDraftCommentOperationsFactory;
+ this.projectOperations = projectOperations;
}
@Override
@@ -173,22 +172,7 @@
return changeCreation.project().get();
}
- return getArbitraryProject();
- }
-
- private Project.NameKey getArbitraryProject() {
- Project.NameKey allProjectsName = projectCache.getAllProjects().getNameKey();
- Project.NameKey allUsersName = projectCache.getAllUsers().getNameKey();
- Optional<Project.NameKey> arbitraryProject =
- projectCache.all().stream()
- .filter(
- name ->
- !Objects.equals(name, allProjectsName) && !Objects.equals(name, allUsersName))
- .findFirst();
- checkState(
- arbitraryProject.isPresent(),
- "At least one repository must be available on the Gerrit server");
- return arbitraryProject.get();
+ return projectOperations.newProject().create();
}
private IdentifiedUser getChangeOwner(TestChangeCreation changeCreation)
diff --git a/java/com/google/gerrit/entities/Patch.java b/java/com/google/gerrit/entities/Patch.java
index bef6580..1dc7149 100644
--- a/java/com/google/gerrit/entities/Patch.java
+++ b/java/com/google/gerrit/entities/Patch.java
@@ -21,6 +21,7 @@
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.entities.Patch.FileMode;
import java.util.List;
/**
@@ -204,6 +205,10 @@
public int getMode() {
return mode;
}
+
+ public int getModeAsOctal() {
+ return Integer.parseInt(Integer.toOctalString(getMode()));
+ }
}
private Patch() {}
diff --git a/java/com/google/gerrit/extensions/api/changes/FileContentInput.java b/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
index 6349595..81131c8 100644
--- a/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
@@ -20,6 +20,25 @@
/** Content to be added to a file (new or existing) via change edit. */
public class FileContentInput {
@DefaultInput public RawInput content;
- public String binary_content;
+
+ /**
+ * The file content as a base-64 encoded data URI.
+ *
+ * <p>If no content is provided, an empty is created or if an existing file is updated the file
+ * content is removed so that the file becomes empty.
+ *
+ * <p>The content must be a SHA1 if the file mode is {@code 160000} (gitlink).
+ */
+ public String binaryContent;
+
+ /**
+ * The file mode in octal format.
+ *
+ * <p>Supported values are {@code 100644} (regular file), {@code 100755} (executable file), {@code
+ * 120000} (symlink) and {@code 160000} (gitlink).
+ *
+ * <p>If unset, new files are created with file mode {@code 100644} (regular file) and for
+ * existing files the existing file mode is kept.
+ */
public Integer fileMode;
}
diff --git a/java/com/google/gerrit/extensions/common/AvatarInfo.java b/java/com/google/gerrit/extensions/common/AvatarInfo.java
index b620ac2..55ae08f 100644
--- a/java/com/google/gerrit/extensions/common/AvatarInfo.java
+++ b/java/com/google/gerrit/extensions/common/AvatarInfo.java
@@ -38,22 +38,17 @@
/** The height of the avatar image in pixels. */
public Integer height;
- /** The width of the avatar image in pixels. */
- public Integer width;
-
@Override
public boolean equals(Object o) {
if (o instanceof AvatarInfo) {
AvatarInfo avatarInfo = (AvatarInfo) o;
- return Objects.equals(url, avatarInfo.url)
- && Objects.equals(height, avatarInfo.height)
- && Objects.equals(width, avatarInfo.width);
+ return Objects.equals(url, avatarInfo.url) && Objects.equals(height, avatarInfo.height);
}
return false;
}
@Override
public int hashCode() {
- return Objects.hash(url, height, width);
+ return Objects.hash(url, height);
}
}
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index be55ec1..3ac5989 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -44,6 +44,8 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.edit.tree.ChangeFileContentModification;
import com.google.gerrit.server.edit.tree.DeleteFileModification;
+import com.google.gerrit.server.edit.tree.InvalidFileModeException;
+import com.google.gerrit.server.edit.tree.InvalidGitLinkException;
import com.google.gerrit.server.edit.tree.RenameFileModification;
import com.google.gerrit.server.edit.tree.RestoreFileModification;
import com.google.gerrit.server.edit.tree.TreeCreator;
@@ -598,6 +600,14 @@
newTreeId = treeCreator.createNewTreeAndGetId(repository);
} catch (InvalidPathException e) {
throw new BadRequestException(e.getMessage());
+ } catch (InvalidFileModeException e) {
+ throw new BadRequestException(
+ String.format(
+ "file mode %s is invalid: supported values are 100644 (regular file), 100755"
+ + " (executable file), 120000 (symlink) and 160000 (gitlink)",
+ Integer.toOctalString(e.getInvalidFileMode())));
+ } catch (InvalidGitLinkException e) {
+ throw new BadRequestException(e.getMessage());
}
if (ObjectId.isEqual(newTreeId, baseCommit.getTree())) {
diff --git a/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java b/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java
index af0496a..6fe9206 100644
--- a/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java
+++ b/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java
@@ -15,7 +15,9 @@
package com.google.gerrit.server.edit.tree;
import static com.google.gerrit.entities.Patch.FileMode.EXECUTABLE_FILE;
+import static com.google.gerrit.entities.Patch.FileMode.GITLINK;
import static com.google.gerrit.entities.Patch.FileMode.REGULAR_FILE;
+import static com.google.gerrit.entities.Patch.FileMode.SYMLINK;
import static java.util.Objects.requireNonNull;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
@@ -31,6 +33,7 @@
import java.util.List;
import org.eclipse.jgit.dircache.DirCacheEditor;
import org.eclipse.jgit.dircache.DirCacheEntry;
+import org.eclipse.jgit.errors.InvalidObjectIdException;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -74,7 +77,7 @@
/** A {@code PathEdit} which changes the contents of a file. */
private static class ChangeContent extends DirCacheEditor.PathEdit {
-
+ private final String filePath;
private final RawInput newContent;
private final Repository repository;
private final Integer newGitFileMode;
@@ -85,13 +88,17 @@
Repository repository,
@Nullable Integer newGitFileMode) {
super(filePath);
+ this.filePath = filePath;
this.newContent = newContent;
this.repository = repository;
this.newGitFileMode = newGitFileMode;
}
private boolean isValidGitFileMode(int gitFileMode) {
- return (gitFileMode == EXECUTABLE_FILE.getMode()) || (gitFileMode == REGULAR_FILE.getMode());
+ return gitFileMode == EXECUTABLE_FILE.getMode()
+ || gitFileMode == REGULAR_FILE.getMode()
+ || gitFileMode == GITLINK.getMode()
+ || gitFileMode == SYMLINK.getMode();
}
@Override
@@ -99,15 +106,24 @@
try {
if (newGitFileMode != null && newGitFileMode != 0) {
if (!isValidGitFileMode(newGitFileMode)) {
- throw new IllegalStateException("GitFileMode " + newGitFileMode + " is invalid");
+ throw new InvalidFileModeException(
+ String.format("GitFileMode %s is invalid", newGitFileMode), newGitFileMode);
}
- dirCacheEntry.setFileMode(FileMode.fromBits(newGitFileMode));
+
+ FileMode fileMode = FileMode.fromBits(newGitFileMode);
+ dirCacheEntry.setFileMode(fileMode);
}
if (dirCacheEntry.getFileMode() == FileMode.GITLINK) {
dirCacheEntry.setLength(0);
dirCacheEntry.setLastModified(Instant.EPOCH);
- ObjectId newObjectId = ObjectId.fromString(getNewContentBytes(), 0);
- dirCacheEntry.setObjectId(newObjectId);
+
+ try {
+ ObjectId newObjectId = ObjectId.fromString(getNewContentBytes(), 0);
+ dirCacheEntry.setObjectId(newObjectId);
+ } catch (InvalidObjectIdException e) {
+ throw new InvalidGitLinkException(
+ String.format("The content for gitlink '%s' must be a valid SHA1.", filePath), e);
+ }
} else {
if (dirCacheEntry.getRawMode() == 0) {
dirCacheEntry.setFileMode(FileMode.REGULAR_FILE);
diff --git a/java/com/google/gerrit/server/edit/tree/InvalidFileModeException.java b/java/com/google/gerrit/server/edit/tree/InvalidFileModeException.java
new file mode 100644
index 0000000..3578dfc
--- /dev/null
+++ b/java/com/google/gerrit/server/edit/tree/InvalidFileModeException.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.edit.tree;
+
+/** Exception to be throw if an invalid or unsupported file mode is being used. */
+public class InvalidFileModeException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ private final int invalidFileMode;
+
+ InvalidFileModeException(String message, int invalidFileMode) {
+ super(message);
+ this.invalidFileMode = invalidFileMode;
+ }
+
+ public int getInvalidFileMode() {
+ return invalidFileMode;
+ }
+}
diff --git a/java/com/google/gerrit/server/edit/tree/InvalidGitLinkException.java b/java/com/google/gerrit/server/edit/tree/InvalidGitLinkException.java
new file mode 100644
index 0000000..ae42805
--- /dev/null
+++ b/java/com/google/gerrit/server/edit/tree/InvalidGitLinkException.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.edit.tree;
+
+/** Exception to be throw if the content of a gitlink is invalid (i.e. if it's not a SHA1). */
+public class InvalidGitLinkException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ InvalidGitLinkException(String message, Throwable cause) {
+ super(message, cause);
+ }
+}
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 4c1f7e3..edc0846 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
-import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.naturalOrder;
import static java.util.stream.Collectors.joining;
@@ -121,13 +120,6 @@
public class MergeUtil {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- /**
- * Length of abbreviated hex SHA-1s in merged filenames.
- *
- * <p>This is a constant so output is stable over time even if the SHA-1 prefix becomes ambiguous.
- */
- private static final int NAME_ABBREV_LEN = 6;
-
private static final String R_HEADS_MASTER = Constants.R_HEADS + Constants.MASTER;
public static boolean useRecursiveMerge(Config cfg) {
@@ -360,13 +352,13 @@
String.format(
"%-" + nameLength + "s (%s %s)",
oursName,
- abbreviateName(ours, NAME_ABBREV_LEN),
+ ours.getName(),
oursMsg.substring(0, Math.min(oursMsg.length(), 60)));
String theirsNameFormatted =
String.format(
"%-" + nameLength + "s (%s %s)",
theirsName,
- abbreviateName(theirs, NAME_ABBREV_LEN),
+ theirs.getName(),
theirsMsg.substring(0, Math.min(theirsMsg.length(), 60)));
MergeFormatter fmt = new MergeFormatter();
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index cc4626e..e254727 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.gerrit.entities.Patch.FileMode.EXECUTABLE_FILE;
-import static com.google.gerrit.entities.Patch.FileMode.REGULAR_FILE;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -337,22 +335,12 @@
}
@Nullable
- private Integer decimalAsOctal(Integer inputMode) throws BadRequestException {
+ private Integer octalAsDecimal(Integer inputMode) {
if (inputMode == null) {
return null;
}
- switch (inputMode) {
- case 100755 -> {
- return EXECUTABLE_FILE.getMode();
- }
- case 100644 -> {
- return REGULAR_FILE.getMode();
- }
- }
-
- throw new BadRequestException(
- "file_mode (" + inputMode + ") was invalid: supported values are 100644 or 100755.");
+ return Integer.parseInt(Integer.toString(inputMode), 8);
}
public Response<Object> apply(
@@ -363,13 +351,13 @@
IOException,
PermissionBackendException {
- if (fileContentInput.content == null && fileContentInput.binary_content == null) {
+ if (fileContentInput.content == null && fileContentInput.binaryContent == null) {
throw new BadRequestException("either content or binary_content is required");
}
RawInput newContent;
- if (fileContentInput.binary_content != null) {
- Matcher m = BINARY_DATA_PATTERN.matcher(fileContentInput.binary_content);
+ if (fileContentInput.binaryContent != null) {
+ Matcher m = BINARY_DATA_PATTERN.matcher(fileContentInput.binaryContent);
if (m.matches() && BASE64.equals(m.group(2))) {
newContent = RawInputUtil.create(Base64.decode(m.group(3)));
} else {
@@ -379,7 +367,7 @@
newContent = fileContentInput.content;
}
- if (Patch.COMMIT_MSG.equals(path) && fileContentInput.binary_content == null) {
+ if (Patch.COMMIT_MSG.equals(path) && fileContentInput.binaryContent == null) {
EditMessage.Input editMessageInput = new EditMessage.Input();
editMessageInput.message =
new String(ByteStreams.toByteArray(newContent.getInputStream()), UTF_8);
@@ -396,7 +384,7 @@
rsrc.getNotes(),
path,
newContent,
- decimalAsOctal(fileContentInput.fileMode));
+ octalAsDecimal(fileContentInput.fileMode));
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 34220b0..c1d31ca 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -152,6 +152,11 @@
inserter.setMessage(
String.format("Patch Set %s: Commit message was updated.", psId.getId()));
inserter.setDescription("Edit commit message");
+
+ // Copy the conflicts information if present. Amending the commit to set a new commit
+ // message doesn't change anything about the conflicts.
+ ps.conflicts().ifPresent(conflicts -> inserter.setConflicts(conflicts));
+
bu.setNotify(resolveNotify(input, resource));
bu.addOp(resource.getChange().getId(), inserter);
bu.execute();
diff --git a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java
index e85a032..e11d856 100644
--- a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java
+++ b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java
@@ -290,6 +290,24 @@
verifyNoMoreInteractions(cache, indexer, includeCache);
}
+ @Test
+ public void newCreateProjectsAreDeleted() throws Exception {
+ Project.NameKey project2 = Project.nameKey("bar");
+
+ ProjectCache projectCache = mock(ProjectCache.class);
+
+ try (ProjectResetter resetProject =
+ builder(null, null, null, null, null, null, projectCache)
+ .build(new ProjectResetter.Config.Builder().deleteNewProjects().build())) {
+ Repository repo2 = repoManager.createRepository(project2);
+ createRef("refs/heads/master");
+ createRef(repo2, RefNames.REFS_CONFIG);
+ }
+
+ assertThat(repoManager.list()).doesNotContain(project2);
+ verify(projectCache, only()).evictAndReindex(project2);
+ }
+
@CanIgnoreReturnValue
private Ref createRef(String ref) throws IOException {
return createRef(repo, ref);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 22b8180..685e4df 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -115,6 +115,7 @@
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.accounts.DeleteDraftCommentsInput;
import com.google.gerrit.extensions.api.changes.AttentionSetInput;
+import com.google.gerrit.extensions.api.changes.ChangeIdentifier;
import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
import com.google.gerrit.extensions.api.changes.DraftApi;
@@ -238,7 +239,6 @@
@UseTimezone(timezone = "US/Eastern")
@VerifyNoPiiInChangeNotes(true)
public class ChangeIT extends AbstractDaemonTest {
-
@Inject private AccountOperations accountOperations;
@Inject private ChangeIndexCollection changeIndexCollection;
@Inject private GroupOperations groupOperations;
@@ -264,105 +264,114 @@
public void get() throws Exception {
TestChange change = changeOperations.newChange().createAndGet();
- ChangeInfo c = gApi.changes().id(change.id()).info();
+ ChangeInfo changeInfo = gApi.changes().id(change.id()).info();
- assertThat(c.id).isEqualTo(change.project() + "~" + change.numericChangeId());
- assertThat(c.project).isEqualTo(change.project().get());
- assertThat(c.branch).isEqualTo(change.dest().shortName());
- assertThat(c.status).isEqualTo(ChangeStatus.NEW);
- assertThat(c.subject).isEqualTo(change.subject());
- assertThat(c.submitType).isEqualTo(SubmitType.MERGE_IF_NECESSARY);
- assertThat(c.mergeable).isNull();
- assertThat(c.changeId).isEqualTo(change.changeId());
- assertThat(c._number).isEqualTo(change.numericChangeId().get());
- assertThat(c.currentRevisionNumber)
+ assertThat(changeInfo.id).isEqualTo(change.project() + "~" + change.numericChangeId());
+ assertThat(changeInfo.project).isEqualTo(change.project().get());
+ assertThat(changeInfo.branch).isEqualTo(change.dest().shortName());
+ assertThat(changeInfo.status).isEqualTo(ChangeStatus.NEW);
+ assertThat(changeInfo.subject).isEqualTo(change.subject());
+ assertThat(changeInfo.submitType).isEqualTo(SubmitType.MERGE_IF_NECESSARY);
+ assertThat(changeInfo.mergeable).isNull();
+ assertThat(changeInfo.changeId).isEqualTo(change.changeId());
+ assertThat(changeInfo._number).isEqualTo(change.numericChangeId().get());
+ assertThat(changeInfo.currentRevisionNumber)
.isEqualTo(changeOperations.change(change.id()).currentPatchset().get().patchsetId().get());
// With NoteDb timestamps are rounded to seconds.
- assertThat(c.created)
+ assertThat(changeInfo.created)
.isEqualTo(Timestamp.from(change.createdOn().truncatedTo(ChronoUnit.SECONDS)));
- assertThat(c.created).isEqualTo(c.updated);
+ assertThat(changeInfo.created).isEqualTo(changeInfo.updated);
- assertThat(c.owner._accountId).isEqualTo(change.owner().get());
- assertThat(c.owner.name).isNull();
- assertThat(c.owner.email).isNull();
- assertThat(c.owner.username).isNull();
- assertThat(c.owner.avatars).isNull();
- assertThat(c.submissionId).isNull();
+ assertThat(changeInfo.owner._accountId).isEqualTo(change.owner().get());
+ assertThat(changeInfo.owner.name).isNull();
+ assertThat(changeInfo.owner.email).isNull();
+ assertThat(changeInfo.owner.username).isNull();
+ assertThat(changeInfo.owner.avatars).isNull();
+ assertThat(changeInfo.submissionId).isNull();
}
@Test
public void cannotGetInvisibleChange() throws Exception {
- PushOneCommit.Result r = createChange();
+ TestChange change = changeOperations.newChange().createAndGet();
// Remove read access
projectOperations
- .project(project)
+ .project(change.project())
.forUpdate()
.add(block(Permission.READ).ref("refs/heads/*").group(REGISTERED_USERS))
.update();
requestScopeOperations.setApiUser(user.id());
ResourceNotFoundException thrown =
- assertThrows(
- ResourceNotFoundException.class,
- () -> gApi.changes().id(project.get(), r.getChange().getId().get()).get());
+ assertThrows(ResourceNotFoundException.class, () -> gApi.changes().id(change.id()).get());
assertThat(thrown)
.hasMessageThat()
- .isEqualTo(String.format("Not found: %s~%d", project.get(), r.getChange().getId().get()));
+ .isEqualTo(
+ String.format(
+ "Not found: %s~%d", change.project().get(), change.numericChangeId().get()));
}
@Test
public void adminCanGetChangeWithoutExplicitReadPermission() throws Exception {
- PushOneCommit.Result r = createChange();
+ TestChange change = changeOperations.newChange().createAndGet();
// Remove read access
projectOperations
- .project(project)
+ .project(change.project())
.forUpdate()
.add(block(Permission.READ).ref("refs/heads/*").group(REGISTERED_USERS))
.update();
requestScopeOperations.setApiUser(admin.id());
- ChangeInfo changeInfo = gApi.changes().id(project.get(), r.getChange().getId().get()).get();
+ ChangeInfo changeInfo = gApi.changes().id(change.id()).get();
assertThat(changeInfo.id)
- .isEqualTo(String.format("%s~%d", project.get(), r.getChange().getId().get()));
+ .isEqualTo(String.format("%s~%d", change.project().get(), change.numericChangeId().get()));
}
@Test
public void diffStatShouldComputeInsertionsAndDeletions() throws Exception {
- String fileName = "a_new_file.txt";
- String fileContent = "First line\nSecond line\n";
- PushOneCommit.Result result = createChange("Add a file", fileName, fileContent);
- String triplet = project.get() + "~master~" + result.getChangeId();
- ChangeInfo change = gApi.changes().id(triplet).get();
- assertThat(change.insertions).isNotNull();
- assertThat(change.deletions).isNotNull();
+ ChangeIdentifier changeIdentifier =
+ changeOperations
+ .newChange()
+ .file("a_new_file.txt")
+ .content("First line\nSecond line\n")
+ .createV2();
+
+ ChangeInfo changeInfo = gApi.changes().id(changeIdentifier).get();
+ assertThat(changeInfo.insertions).isEqualTo(2);
+ assertThat(changeInfo.deletions).isEqualTo(0);
}
@Test
public void diffStatShouldSkipInsertionsAndDeletions() throws Exception {
- String fileName = "a_new_file.txt";
- String fileContent = "First line\nSecond line\n";
- PushOneCommit.Result result = createChange("Add a file", fileName, fileContent);
- String triplet = project.get() + "~master~" + result.getChangeId();
- ChangeInfo change =
- gApi.changes().id(triplet).get(ImmutableList.of(ListChangesOption.SKIP_DIFFSTAT));
- assertThat(change.insertions).isNull();
- assertThat(change.deletions).isNull();
+ ChangeIdentifier changeIdentifier =
+ changeOperations
+ .newChange()
+ .file("a_new_file.txt")
+ .content("First line\nSecond line\n")
+ .createV2();
+ ChangeInfo changeInfo =
+ gApi.changes().id(changeIdentifier).get(ImmutableList.of(ListChangesOption.SKIP_DIFFSTAT));
+ assertThat(changeInfo.insertions).isNull();
+ assertThat(changeInfo.deletions).isNull();
}
@Test
public void skipDiffstatOptionAvoidsAllDiffComputations() throws Exception {
- String fileName = "a_new_file.txt";
- String fileContent = "First line\nSecond line\n";
- PushOneCommit.Result result = createChange("Add a file", fileName, fileContent);
- String triplet = project.get() + "~master~" + result.getChangeId();
+ ChangeIdentifier changeIdentifier =
+ changeOperations
+ .newChange()
+ .file("a_new_file.txt")
+ .content("First line\nSecond line\n")
+ .createV2();
+
CacheStats startIntra = cloneStats(intraCache.stats());
CacheStats startSummary = cloneStats(diffSummaryCache.stats());
@SuppressWarnings("unused")
- var unused = gApi.changes().id(triplet).get(ImmutableList.of(ListChangesOption.SKIP_DIFFSTAT));
+ var unused =
+ gApi.changes().id(changeIdentifier).get(ImmutableList.of(ListChangesOption.SKIP_DIFFSTAT));
assertThat(intraCache.stats()).since(startIntra).hasMissCount(0);
assertThat(intraCache.stats()).since(startIntra).hasHitCount(0);
@@ -373,57 +382,57 @@
@Test
@GerritConfig(name = "change.mergeabilityComputationBehavior", value = "NEVER")
public void excludeMergeableInChangeInfo() throws Exception {
- PushOneCommit.Result r = createChange();
- ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
+
+ ChangeInfo c = gApi.changes().id(changeIdentifier).get();
assertThat(c.mergeable).isNull();
}
@Test
public void getSubmissionId() throws Exception {
- PushOneCommit.Result changeResult = createChange();
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
- merge(changeResult);
- assertThat(gApi.changes().id(changeId).get().submissionId).isNotNull();
+ assertThat(gApi.changes().id(changeIdentifier).get().submissionId).isNull();
+
+ gApi.changes().id(changeIdentifier).current().review(ReviewInput.approve());
+ gApi.changes().id(changeIdentifier).current().submit();
+
+ assertThat(gApi.changes().id(changeIdentifier).get().submissionId).isNotNull();
}
@Test
public void setWorkInProgressNotAllowedWithoutPermission() throws Exception {
- PushOneCommit.Result rwip = createChange();
- String changeId = rwip.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
- assertThrows(AuthException.class, () -> gApi.changes().id(changeId).setWorkInProgress());
+ assertThrows(
+ AuthException.class, () -> gApi.changes().id(changeIdentifier).setWorkInProgress());
assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted");
}
@Test
public void setWorkInProgressAllowedAsAdmin() throws Exception {
- requestScopeOperations.setApiUser(user.id());
- String changeId =
- gApi.changes().create(new ChangeInput(project.get(), "master", "Test Change")).get().id;
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(user.id()).createV2();
requestScopeOperations.setApiUser(admin.id());
- gApi.changes().id(changeId).setWorkInProgress();
- assertThat(gApi.changes().id(changeId).get().workInProgress).isTrue();
+ gApi.changes().id(changeIdentifier).setWorkInProgress();
+ assertThat(gApi.changes().id(changeIdentifier).get().workInProgress).isTrue();
}
@Test
public void setWorkInProgressAllowedAsProjectOwner() throws Exception {
- requestScopeOperations.setApiUser(user.id());
- String changeId =
- gApi.changes().create(new ChangeInput(project.get(), "master", "Test Change")).get().id;
+ TestChange change = changeOperations.newChange().owner(user.id()).createAndGet();
com.google.gerrit.acceptance.TestAccount user2 = accountCreator.user2();
projectOperations
- .project(project)
+ .project(change.project())
.forUpdate()
.add(allow(Permission.OWNER).ref("refs/*").group(REGISTERED_USERS))
.update();
requestScopeOperations.setApiUser(user2.id());
- gApi.changes().id(changeId).setWorkInProgress();
- assertThat(gApi.changes().id(changeId).get().workInProgress).isTrue();
+ gApi.changes().id(change.id()).setWorkInProgress();
+ assertThat(gApi.changes().id(change.id()).get().workInProgress).isTrue();
}
@Test
@@ -438,54 +447,52 @@
@Test
public void setReadyForReviewNotAllowedWithoutPermission() throws Exception {
- PushOneCommit.Result rready = createChange();
- String changeId = rready.getChangeId();
- gApi.changes().id(changeId).setWorkInProgress();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
+ gApi.changes().id(changeIdentifier).setWorkInProgress();
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
- assertThrows(AuthException.class, () -> gApi.changes().id(changeId).setReadyForReview());
+ assertThrows(
+ AuthException.class, () -> gApi.changes().id(changeIdentifier).setReadyForReview());
assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted");
}
@Test
public void setReadyForReviewAllowedAsAdmin() throws Exception {
- requestScopeOperations.setApiUser(user.id());
- String changeId =
- gApi.changes().create(new ChangeInput(project.get(), "master", "Test Change")).get().id;
- gApi.changes().id(changeId).setWorkInProgress();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(user.id()).createV2();
+ gApi.changes().id(changeIdentifier).setWorkInProgress();
requestScopeOperations.setApiUser(admin.id());
- gApi.changes().id(changeId).setReadyForReview();
- assertThat(gApi.changes().id(changeId).get().workInProgress).isNull();
+ gApi.changes().id(changeIdentifier).setReadyForReview();
+ assertThat(gApi.changes().id(changeIdentifier).get().workInProgress).isNull();
}
@Test
public void setReadyForReviewAllowedAsProjectOwner() throws Exception {
- requestScopeOperations.setApiUser(user.id());
- String changeId =
- gApi.changes().create(new ChangeInput(project.get(), "master", "Test Change")).get().id;
- gApi.changes().id(changeId).setWorkInProgress();
+ TestChange change = changeOperations.newChange().owner(user.id()).createAndGet();
- com.google.gerrit.acceptance.TestAccount user2 = accountCreator.user2();
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(change.id()).setWorkInProgress();
+
+ TestAccount user2 = accountCreator.user2();
projectOperations
- .project(project)
+ .project(change.project())
.forUpdate()
.add(allow(Permission.OWNER).ref("refs/*").group(REGISTERED_USERS))
.update();
requestScopeOperations.setApiUser(user2.id());
- gApi.changes().id(changeId).setReadyForReview();
- assertThat(gApi.changes().id(changeId).get().workInProgress).isNull();
+ gApi.changes().id(change.id()).setReadyForReview();
+ assertThat(gApi.changes().id(change.id()).get().workInProgress).isNull();
}
@Test
public void setReadyForReviewSendsNotificationsForRevertChange() throws Exception {
- PushOneCommit.Result r = createChange();
- gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
- gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
+ gApi.changes().id(changeIdentifier).current().review(ReviewInput.approve());
+ gApi.changes().id(changeIdentifier).current().submit();
RevertInput in = new RevertInput();
in.workInProgress = true;
- String changeId = gApi.changes().id(r.getChangeId()).revert(in).get().changeId;
+ String changeId = gApi.changes().id(changeIdentifier).revert(in).get().changeId;
requestScopeOperations.setApiUser(admin.id());
gApi.changes().id(changeId).setReadyForReview();
@@ -497,7 +504,7 @@
// 3. Change has been successfully merged by Administrator
// 4. Patch Set 1: Reverted
List<ChangeMessageInfo> sourceMessages =
- new ArrayList<>(gApi.changes().id(r.getChangeId()).get().messages);
+ new ArrayList<>(gApi.changes().id(changeIdentifier).get().messages);
assertThat(sourceMessages).hasSize(4);
String expectedMessage = String.format("Created a revert of this change as %s", changeId);
assertThat(sourceMessages.get(3).message).isEqualTo(expectedMessage);
@@ -521,9 +528,10 @@
conf.enableReviewerByEmail = InheritableBoolean.TRUE;
gApi.projects().name(project.get()).config(conf);
- PushOneCommit.Result r = createWorkInProgressChange();
- String changeId = r.getChangeId();
- assertThat(gApi.changes().id(changeId).get().pendingReviewers).isEmpty();
+ ChangeIdentifier changeIdentifier =
+ changeOperations.newChange().project(project).owner(admin.id()).createV2();
+ gApi.changes().id(changeIdentifier).setWorkInProgress();
+ assertThat(gApi.changes().id(changeIdentifier).get().pendingReviewers).isEmpty();
// Add some pending reviewers.
String email1 = name("user1") + "@example.com";
@@ -564,10 +572,10 @@
.reviewer("byemail2@example.com")
.reviewer("byemail3@example.com", CC, false)
.reviewer("byemail4@example.com", CC, false);
- ReviewResult result = gApi.changes().id(changeId).current().review(in);
+ ReviewResult result = gApi.changes().id(changeIdentifier).current().review(in);
assertThat(result.changeInfo).isNotNull();
assertThat(result.reviewers).isNotEmpty();
- ChangeInfo info = gApi.changes().id(changeId).get();
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
Function<Collection<AccountInfo>, Collection<String>> toEmails =
ais -> ais.stream().map(ai -> ai.email).collect(toSet());
assertThat(toEmails.apply(info.pendingReviewers.get(REVIEWER)))
@@ -577,11 +585,11 @@
assertThat(info.pendingReviewers.get(REMOVED)).isNull();
// Stage some pending reviewer removals.
- gApi.changes().id(changeId).reviewer(email1).remove();
- gApi.changes().id(changeId).reviewer(email3).remove();
- gApi.changes().id(changeId).reviewer("byemail1@example.com").remove();
- gApi.changes().id(changeId).reviewer("byemail3@example.com").remove();
- info = gApi.changes().id(changeId).get();
+ gApi.changes().id(changeIdentifier).reviewer(email1).remove();
+ gApi.changes().id(changeIdentifier).reviewer(email3).remove();
+ gApi.changes().id(changeIdentifier).reviewer("byemail1@example.com").remove();
+ gApi.changes().id(changeIdentifier).reviewer("byemail3@example.com").remove();
+ info = gApi.changes().id(changeIdentifier).get();
assertThat(toEmails.apply(info.pendingReviewers.get(REVIEWER)))
.containsExactly(email2, "byemail2@example.com");
assertThat(toEmails.apply(info.pendingReviewers.get(CC)))
@@ -591,8 +599,8 @@
// "Undo" a removal.
in = ReviewInput.noScore().reviewer(email1);
- gApi.changes().id(changeId).current().review(in);
- info = gApi.changes().id(changeId).get();
+ gApi.changes().id(changeIdentifier).current().review(in);
+ info = gApi.changes().id(changeIdentifier).get();
assertThat(toEmails.apply(info.pendingReviewers.get(REVIEWER)))
.containsExactly(email1, email2, "byemail2@example.com");
assertThat(toEmails.apply(info.pendingReviewers.get(CC)))
@@ -601,8 +609,8 @@
.containsExactly(email3, "byemail1@example.com", "byemail3@example.com");
// "Commit" by moving out of WIP.
- gApi.changes().id(changeId).setReadyForReview();
- info = gApi.changes().id(changeId).get();
+ gApi.changes().id(changeIdentifier).setReadyForReview();
+ info = gApi.changes().id(changeIdentifier).get();
assertThat(info.pendingReviewers).isEmpty();
assertThat(toEmails.apply(info.reviewers.get(REVIEWER)))
.containsExactly(email1, email2, "byemail2@example.com");
@@ -613,37 +621,36 @@
@Test
public void toggleWorkInProgressState() throws Exception {
- PushOneCommit.Result r = createChange();
- String changeId = r.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
// With message
- gApi.changes().id(changeId).setWorkInProgress("Needs some refactoring");
+ gApi.changes().id(changeIdentifier).setWorkInProgress("Needs some refactoring");
- ChangeInfo info = gApi.changes().id(changeId).get();
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isTrue();
assertThat(Iterables.getLast(info.messages).message).contains("Needs some refactoring");
assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_WIP);
- gApi.changes().id(changeId).setReadyForReview("PTAL");
+ gApi.changes().id(changeIdentifier).setReadyForReview("PTAL");
- info = gApi.changes().id(changeId).get();
+ info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isNull();
assertThat(Iterables.getLast(info.messages).message).contains("PTAL");
assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_READY);
// No message
- gApi.changes().id(changeId).setWorkInProgress();
+ gApi.changes().id(changeIdentifier).setWorkInProgress();
- info = gApi.changes().id(changeId).get();
+ info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isTrue();
assertThat(Iterables.getLast(info.messages).message).isEqualTo("Set Work In Progress");
assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_WIP);
- gApi.changes().id(changeId).setReadyForReview();
+ gApi.changes().id(changeIdentifier).setReadyForReview();
- info = gApi.changes().id(changeId).get();
+ info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isNull();
assertThat(Iterables.getLast(info.messages).message).isEqualTo("Set Ready For Review");
assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_READY);
@@ -651,13 +658,12 @@
@Test
public void toggleWorkInProgressStateByNonOwnerWithPermission() throws Exception {
- PushOneCommit.Result r = createChange();
- String changeId = r.getChangeId();
+ TestChange change = changeOperations.newChange().createAndGet();
String refactor = "Needs some refactoring";
String ptal = "PTAL";
projectOperations
- .project(project)
+ .project(change.project())
.forUpdate()
.add(
allow(Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
@@ -666,17 +672,17 @@
.update();
requestScopeOperations.setApiUser(user.id());
- gApi.changes().id(changeId).setWorkInProgress(refactor);
+ gApi.changes().id(change.id()).setWorkInProgress(refactor);
- ChangeInfo info = gApi.changes().id(changeId).get();
+ ChangeInfo info = gApi.changes().id(change.id()).get();
assertThat(info.workInProgress).isTrue();
assertThat(Iterables.getLast(info.messages).message).contains(refactor);
assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_WIP);
- gApi.changes().id(changeId).setReadyForReview(ptal);
+ gApi.changes().id(change.id()).setReadyForReview(ptal);
- info = gApi.changes().id(changeId).get();
+ info = gApi.changes().id(change.id()).get();
assertThat(info.workInProgress).isNull();
assertThat(Iterables.getLast(info.messages).message).contains(ptal);
assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_READY);
@@ -684,44 +690,41 @@
@Test
public void reviewAndStartReview() throws Exception {
- PushOneCommit.Result r = createWorkInProgressChange();
- r.assertOkStatus();
- assertThat(r.getChange().change().isWorkInProgress()).isTrue();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
+ gApi.changes().id(changeIdentifier).setWorkInProgress();
ReviewInput in = ReviewInput.noScore().setWorkInProgress(false);
- ReviewResult result = gApi.changes().id(r.getChangeId()).current().review(in);
+ ReviewResult result = gApi.changes().id(changeIdentifier).current().review(in);
assertThat(result.ready).isTrue();
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isNull();
}
@Test
public void reviewAndMoveToWorkInProgress() throws Exception {
- PushOneCommit.Result r = createChange();
- assertThat(r.getChange().change().isWorkInProgress()).isFalse();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
- ReviewResult result = gApi.changes().id(r.getChangeId()).current().review(in);
+ ReviewResult result = gApi.changes().id(changeIdentifier).current().review(in);
assertThat(result.ready).isNull();
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isTrue();
}
@Test
public void reviewAndSetWorkInProgressAndAddReviewerAndVote() throws Exception {
- PushOneCommit.Result r = createChange();
- assertThat(r.getChange().change().isWorkInProgress()).isFalse();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
ReviewInput in =
ReviewInput.approve()
.reviewer(user.email())
.label(LabelId.CODE_REVIEW, 1)
.setWorkInProgress(true);
- gApi.changes().id(r.getChangeId()).current().review(in);
+ gApi.changes().id(changeIdentifier).current().review(in);
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isTrue();
assertThat(info.reviewers.get(REVIEWER).stream().map(ai -> ai._accountId).collect(toList()))
.containsExactly(admin.id().get(), user.id().get());
@@ -731,24 +734,24 @@
@Test
public void reviewRemoveInactiveReviewer() throws Exception {
- PushOneCommit.Result r = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
ReviewInput in = ReviewInput.approve().reviewer(user.email());
- gApi.changes().id(r.getChangeId()).current().review(in);
+ gApi.changes().id(changeIdentifier).current().review(in);
accountOperations.account(user.id()).forUpdate().inactive().update();
in = ReviewInput.noScore().reviewer(Integer.toString(user.id().get()), REMOVED, false);
- gApi.changes().id(r.getChangeId()).current().review(in);
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ gApi.changes().id(changeIdentifier).current().review(in);
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.reviewers.get(REVIEWER).stream().map(ai -> ai._accountId).collect(toList()))
.containsExactly(admin.id().get());
}
@Test
public void removeReviewerWithoutPermissionsOnChangePostReview_allowed() throws Exception {
- PushOneCommit.Result r = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
ReviewInput in = ReviewInput.approve().reviewer(user.email());
- gApi.changes().id(r.getChangeId()).current().review(in);
+ gApi.changes().id(changeIdentifier).current().review(in);
AccountGroup.UUID restrictedGroup =
groupOperations.newGroup().name("restricted-group").addMember(user.id()).create();
@@ -762,18 +765,17 @@
in = ReviewInput.noScore().reviewer(Integer.toString(user.id().get()), REMOVED, false);
requestScopeOperations.setApiUser(admin.id());
- gApi.changes().id(r.getChangeId()).current().review(in);
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ gApi.changes().id(changeIdentifier).current().review(in);
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.reviewers.get(REVIEWER).stream().map(ai -> ai._accountId).collect(toList()))
.containsExactly(admin.id().get());
}
@Test
public void removeReviewerWithoutPermissionsOnChange_allowed() throws Exception {
-
- PushOneCommit.Result r = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
ReviewInput in = ReviewInput.approve().reviewer(user.email());
- gApi.changes().id(r.getChangeId()).current().review(in);
+ gApi.changes().id(changeIdentifier).current().review(in);
AccountGroup.UUID restrictedGroup =
groupOperations.newGroup().name("restricted-group").addMember(user.id()).create();
@@ -785,69 +787,61 @@
.update();
requestScopeOperations.setApiUser(admin.id());
- gApi.changes().id(r.getChangeId()).reviewer(user.id().toString()).remove();
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ gApi.changes().id(changeIdentifier).reviewer(user.id().toString()).remove();
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.reviewers.get(REVIEWER).stream().map(ai -> ai._accountId).collect(toList()))
.containsExactly(admin.id().get());
}
@Test
public void reviewWithWorkInProgressAndReadyReturnsError() throws Exception {
- PushOneCommit.Result r = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
ReviewInput in = ReviewInput.noScore();
in.ready = true;
in.workInProgress = true;
- ReviewResult result = gApi.changes().id(r.getChangeId()).current().review(in);
+ ReviewResult result = gApi.changes().id(changeIdentifier).current().review(in);
assertThat(result.error).isEqualTo(PostReview.ERROR_WIP_READY_MUTUALLY_EXCLUSIVE);
}
@Test
- @TestProjectInput(cloneAs = "user1")
public void reviewWithWorkInProgressChangeOwner() throws Exception {
- PushOneCommit push = pushFactory.create(user.newIdent(), testRepo);
- PushOneCommit.Result r = push.to("refs/for/master");
- r.assertOkStatus();
- assertThat(r.getChange().change().getOwner()).isEqualTo(user.id());
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(user.id()).createV2();
requestScopeOperations.setApiUser(user.id());
ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
- gApi.changes().id(r.getChangeId()).current().review(in);
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ gApi.changes().id(changeIdentifier).current().review(in);
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isTrue();
}
@Test
- @TestProjectInput(cloneAs = "user1")
public void reviewWithWithWorkInProgressAdmin() throws Exception {
- PushOneCommit push = pushFactory.create(user.newIdent(), testRepo);
- PushOneCommit.Result r = push.to("refs/for/master");
- r.assertOkStatus();
- assertThat(r.getChange().change().getOwner()).isEqualTo(user.id());
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(user.id()).createV2();
requestScopeOperations.setApiUser(admin.id());
ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
- gApi.changes().id(r.getChangeId()).current().review(in);
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ gApi.changes().id(changeIdentifier).current().review(in);
+ ChangeInfo info = gApi.changes().id(changeIdentifier).get();
assertThat(info.workInProgress).isTrue();
}
@Test
public void reviewWithWorkInProgressByNonOwnerReturnsError() throws Exception {
- PushOneCommit.Result r = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(admin.id()).createV2();
ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
assertThrows(
- AuthException.class, () -> gApi.changes().id(r.getChangeId()).current().review(in));
+ AuthException.class, () -> gApi.changes().id(changeIdentifier).current().review(in));
assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted");
}
@Test
public void reviewWithWorkInProgressByNonOwnerWithPermission() throws Exception {
- PushOneCommit.Result r = createChange();
+ TestChange change = changeOperations.newChange().owner(admin.id()).createAndGet();
ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
projectOperations
- .project(project)
+ .project(change.project())
.forUpdate()
.add(
allow(Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
@@ -855,50 +849,49 @@
.group(REGISTERED_USERS))
.update();
requestScopeOperations.setApiUser(user.id());
- gApi.changes().id(r.getChangeId()).current().review(in);
- ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ gApi.changes().id(change.id()).current().review(in);
+ ChangeInfo info = gApi.changes().id(change.id()).get();
assertThat(info.workInProgress).isTrue();
}
@Test
public void reviewWithReadyByNonOwnerReturnsError() throws Exception {
- PushOneCommit.Result r = createChange();
- change(r).setWorkInProgress();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(admin.id()).createV2();
+ gApi.changes().id(changeIdentifier).setWorkInProgress();
ReviewInput in = ReviewInput.noScore().setReady(true);
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
assertThrows(
- AuthException.class, () -> gApi.changes().id(r.getChangeId()).current().review(in));
+ AuthException.class, () -> gApi.changes().id(changeIdentifier).current().review(in));
assertThat(thrown).hasMessageThat().contains("toggle work in progress state not permitted");
}
@Test
public void getAmbiguous() throws Exception {
- PushOneCommit.Result r1 = createChange();
- String changeId = r1.getChangeId();
+ TestChange change =
+ changeOperations.newChange().project(project).branch("master").createAndGet();
@SuppressWarnings("unused")
- var unused = gApi.changes().id(changeId).get();
+ var unused = gApi.changes().id(change.id()).get();
BranchInput b = new BranchInput();
b.revision = repo().exactRef("HEAD").getObjectId().name();
gApi.projects().name(project.get()).branch("other").create(b);
- PushOneCommit push2 =
- pushFactory.create(
- admin.newIdent(),
- testRepo,
- PushOneCommit.SUBJECT,
- PushOneCommit.FILE_NAME,
- PushOneCommit.FILE_CONTENT,
- changeId);
- PushOneCommit.Result r2 = push2.to("refs/for/other");
- assertThat(r2.getChangeId()).isEqualTo(changeId);
+ TestChange otherChange =
+ changeOperations
+ .newChange()
+ .project(project)
+ .branch("other")
+ .commitMessage("Summary line\n\nChange-Id: " + change.changeId())
+ .createAndGet();
+ assertThat(otherChange.changeId()).isEqualTo(change.changeId());
ResourceNotFoundException thrown =
- assertThrows(ResourceNotFoundException.class, () -> gApi.changes().id(changeId).get());
- assertThat(thrown).hasMessageThat().contains("Multiple changes found for " + changeId);
+ assertThrows(
+ ResourceNotFoundException.class, () -> gApi.changes().id(change.changeId()).get());
+ assertThat(thrown).hasMessageThat().contains("Multiple changes found for " + change.changeId());
}
@Test
@@ -907,15 +900,12 @@
}
@Test
- @TestProjectInput(cloneAs = "user1")
public void deleteNewChangeAsNormalUser() throws Exception {
- PushOneCommit.Result changeResult =
- pushFactory.create(user.newIdent(), testRepo).to("refs/for/master");
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(user.id()).createV2();
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
- assertThrows(AuthException.class, () -> gApi.changes().id(changeId).delete());
+ assertThrows(AuthException.class, () -> gApi.changes().id(changeIdentifier).delete());
assertThat(thrown).hasMessageThat().contains("delete not permitted");
}
@@ -926,7 +916,7 @@
.forUpdate()
.add(allow(Permission.DELETE_CHANGES).ref("refs/*").group(REGISTERED_USERS))
.update();
- deleteChangeAsUser(admin, user);
+ deleteChangeAsUser(project, admin, user);
}
@Test
@@ -958,7 +948,7 @@
.forUpdate()
.add(allow(Permission.DELETE_OWN_CHANGES).ref("refs/*").group(REGISTERED_USERS))
.update();
- deleteChangeAsUser(user, user);
+ deleteChangeAsUser(project, user, user);
}
@Test
@@ -968,7 +958,7 @@
.forUpdate()
.add(allow(Permission.DELETE_OWN_CHANGES).ref("refs/*").group(CHANGE_OWNER))
.update();
- deleteChangeAsUser(user, user);
+ deleteChangeAsUser(project, user, user);
}
private void deleteChangeAsUser(
@@ -989,26 +979,24 @@
.forUpdate()
.add(allow(Permission.VIEW_PRIVATE_CHANGES).ref("refs/*").group(ANONYMOUS_USERS))
.update();
+ TestChange change =
+ changeOperations.newChange().project(projectName).owner(owner.id()).createAndGet();
requestScopeOperations.setApiUser(owner.id());
- ChangeInput in = new ChangeInput();
- in.project = projectName.get();
- in.branch = "refs/heads/master";
- in.subject = "test";
- ChangeInfo changeInfo = gApi.changes().create(in).get();
- String changeId = changeInfo.changeId;
- int id = changeInfo._number;
- String commit = changeInfo.currentRevision;
- assertThat(gApi.changes().id(changeId).info().owner._accountId).isEqualTo(owner.id().get());
+ String commit =
+ changeOperations.change(change.id()).currentPatchset().get().commitId().getName();
+
+ assertThat(gApi.changes().id(change.id()).info().owner._accountId)
+ .isEqualTo(owner.id().get());
requestScopeOperations.setApiUser(deleteAs.id());
- gApi.changes().id(changeId).delete();
+ gApi.changes().id(change.id()).delete();
- assertThat(query(changeId)).isEmpty();
+ assertThat(query(change.id().id())).isEmpty();
- String ref = Change.id(id).toRefPrefix() + "1";
+ String ref = change.numericChangeId().toRefPrefix() + "1";
eventRecorder.assertRefUpdatedEvents(projectName.get(), ref, null, commit, commit, null);
- eventRecorder.assertChangeDeletedEvents(changeId, deleteAs.email());
+ eventRecorder.assertChangeDeletedEvents(change.changeId(), deleteAs.email());
} finally {
projectOperations
.project(project)
@@ -1033,12 +1021,11 @@
.update();
try {
- PushOneCommit.Result changeResult = createChange();
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().project(project).createV2();
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
- assertThrows(AuthException.class, () -> gApi.changes().id(changeId).delete());
+ assertThrows(AuthException.class, () -> gApi.changes().id(changeIdentifier).delete());
assertThat(thrown).hasMessageThat().contains("delete not permitted");
} finally {
projectOperations
@@ -1052,57 +1039,50 @@
@Test
@TestProjectInput(createEmptyCommit = false)
public void deleteNewChangeForBranchWithoutCommits() throws Exception {
- PushOneCommit.Result changeResult = createChange();
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().project(project).createV2();
- gApi.changes().id(changeId).delete();
+ gApi.changes().id(changeIdentifier).delete();
- assertThat(query(changeId)).isEmpty();
+ assertThat(query(changeIdentifier.id())).isEmpty();
}
@Test
- @TestProjectInput(cloneAs = "user1")
public void deleteAbandonedChangeAsNormalUser() throws Exception {
- PushOneCommit.Result changeResult =
- pushFactory.create(user.newIdent(), testRepo).to("refs/for/master");
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(user.id()).createV2();
requestScopeOperations.setApiUser(user.id());
- gApi.changes().id(changeId).abandon();
+ gApi.changes().id(changeIdentifier).abandon();
AuthException thrown =
- assertThrows(AuthException.class, () -> gApi.changes().id(changeId).delete());
+ assertThrows(AuthException.class, () -> gApi.changes().id(changeIdentifier).delete());
assertThat(thrown).hasMessageThat().contains("delete not permitted");
}
@Test
- @TestProjectInput(cloneAs = "user1")
public void deleteAbandonedChangeOfAnotherUserAsAdmin() throws Exception {
- PushOneCommit.Result changeResult =
- pushFactory.create(user.newIdent(), testRepo).to("refs/for/master");
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(user.id()).createV2();
- gApi.changes().id(changeId).abandon();
+ gApi.changes().id(changeIdentifier).abandon();
- gApi.changes().id(changeId).delete();
+ gApi.changes().id(changeIdentifier).delete();
- assertThat(query(changeId)).isEmpty();
+ assertThat(query(changeIdentifier.id())).isEmpty();
}
@Test
public void deleteMergedChange() throws Exception {
- PushOneCommit.Result changeResult = createChange();
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
- merge(changeResult);
+ gApi.changes().id(changeIdentifier).current().review(ReviewInput.approve());
+ gApi.changes().id(changeIdentifier).current().submit();
MethodNotAllowedException thrown =
- assertThrows(MethodNotAllowedException.class, () -> gApi.changes().id(changeId).delete());
+ assertThrows(
+ MethodNotAllowedException.class, () -> gApi.changes().id(changeIdentifier).delete());
assertThat(thrown).hasMessageThat().contains("delete not permitted");
}
@Test
- @TestProjectInput(cloneAs = "user1")
public void deleteMergedChangeWithDeleteOwnChangesPermission() throws Exception {
projectOperations
.project(project)
@@ -1111,15 +1091,15 @@
.update();
try {
- PushOneCommit.Result changeResult =
- pushFactory.create(user.newIdent(), testRepo).to("refs/for/master");
- String changeId = changeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().project(project).createV2();
- merge(changeResult);
+ gApi.changes().id(changeIdentifier).current().review(ReviewInput.approve());
+ gApi.changes().id(changeIdentifier).current().submit();
requestScopeOperations.setApiUser(user.id());
MethodNotAllowedException thrown =
- assertThrows(MethodNotAllowedException.class, () -> gApi.changes().id(changeId).delete());
+ assertThrows(
+ MethodNotAllowedException.class, () -> gApi.changes().id(changeIdentifier).delete());
assertThat(thrown).hasMessageThat().contains("delete not permitted");
} finally {
projectOperations
@@ -1132,40 +1112,47 @@
@Test
public void deleteNewChangeWithMergedPatchSet() throws Exception {
- PushOneCommit.Result changeResult = createChange();
- String changeId = changeResult.getChangeId();
- Change.Id id = changeResult.getChange().getId();
+ TestChange change = changeOperations.newChange().project(project).createAndGet();
- merge(changeResult);
- setChangeStatus(id, Change.Status.NEW);
+ gApi.changes().id(change.id()).current().review(ReviewInput.approve());
+ gApi.changes().id(change.id()).current().submit();
+
+ setChangeStatus(change.numericChangeId(), Change.Status.NEW);
ResourceConflictException thrown =
- assertThrows(ResourceConflictException.class, () -> gApi.changes().id(changeId).delete());
+ assertThrows(
+ ResourceConflictException.class, () -> gApi.changes().id(change.id()).delete());
assertThat(thrown)
.hasMessageThat()
- .contains(String.format("Cannot delete change %s: patch set 1 is already merged", id));
+ .contains(
+ String.format(
+ "Cannot delete change %s: patch set 1 is already merged",
+ change.numericChangeId()));
}
@Test
public void deleteChangeUpdatesIndex() throws Exception {
- PushOneCommit.Result changeResult = createChange();
- String changeId = changeResult.getChangeId();
- Change.Id id = changeResult.getChange().getId();
+ TestChange change = changeOperations.newChange().project(project).createAndGet();
ChangeIndex idx = changeIndexCollection.getSearchIndex();
Optional<ChangeData> result =
- idx.get(id, IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
+ idx.get(
+ change.numericChangeId(),
+ IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
assertThat(result).isPresent();
- gApi.changes().id(changeId).delete();
- result = idx.get(id, IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
+ gApi.changes().id(change.id()).delete();
+ result =
+ idx.get(
+ change.numericChangeId(),
+ IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
assertThat(result).isEmpty();
}
@Test
public void deleteChangeRemovesDraftComment() throws Exception {
- PushOneCommit.Result r = createChange();
+ TestChange change = changeOperations.newChange().createAndGet();
requestScopeOperations.setApiUser(user.id());
@@ -1174,15 +1161,14 @@
dri.path = "a.txt";
dri.line = 1;
- gApi.changes().id(r.getChangeId()).current().createDraft(dri);
- Change.Id num = r.getChange().getId();
+ gApi.changes().id(change.id()).current().createDraft(dri);
- assertThat(getDraftsCountForChange(num, user.id())).isGreaterThan(0);
+ assertThat(getDraftsCountForChange(change.numericChangeId(), user.id())).isGreaterThan(0);
requestScopeOperations.setApiUser(admin.id());
- gApi.changes().id(r.getChangeId()).delete();
- assertThat(getDraftsCountForChange(num, user.id())).isEqualTo(0);
+ gApi.changes().id(change.id()).delete();
+ assertThat(getDraftsCountForChange(change.numericChangeId(), user.id())).isEqualTo(0);
}
@UsedAt(UsedAt.Project.GOOGLE)
@@ -1196,56 +1182,54 @@
@Test
public void deleteChangeRemovesItsChangeEdit() throws Exception {
- PushOneCommit.Result result = createChange();
+ TestChange change = changeOperations.newChange().createAndGet();
requestScopeOperations.setApiUser(user.id());
- String changeId = result.getChangeId();
- gApi.changes().id(changeId).edit().create();
+ gApi.changes().id(change.id()).edit().create();
gApi.changes()
- .id(changeId)
+ .id(change.id())
.edit()
.modifyFile(FILE_NAME, RawInputUtil.create("foo".getBytes(UTF_8)));
requestScopeOperations.setApiUser(admin.id());
- String expected = RefNames.refsUsers(user.id()) + "/edit-" + result.getChange().getId() + "/1";
- try (Repository repo = repoManager.openRepository(project)) {
+ String expected = RefNames.refsUsers(user.id()) + "/edit-" + change.numericChangeId() + "/1";
+ try (Repository repo = repoManager.openRepository(change.project())) {
assertThat(repo.getRefDatabase().getRefsByPrefix(expected)).isNotEmpty();
- gApi.changes().id(changeId).delete();
+ gApi.changes().id(change.id()).delete();
}
// On google infra, repo should be reopened for getting updated refs.
- try (Repository repo = repoManager.openRepository(project)) {
+ try (Repository repo = repoManager.openRepository(change.project())) {
assertThat(repo.getRefDatabase().getRefsByPrefix(expected)).isEmpty();
}
}
@Test
public void deleteChangeDoesntRemoveOtherChangeEdits() throws Exception {
- PushOneCommit.Result result = createChange();
- PushOneCommit.Result irrelevantChangeResult = createChange();
- requestScopeOperations.setApiUser(admin.id());
- String changeId = result.getChangeId();
- String irrelevantChangeId = irrelevantChangeResult.getChangeId();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
+ ChangeIdentifier irrelevanetChangeIdentifier = changeOperations.newChange().createV2();
- gApi.changes().id(irrelevantChangeId).edit().create();
+ requestScopeOperations.setApiUser(admin.id());
+
+ gApi.changes().id(irrelevanetChangeIdentifier).edit().create();
gApi.changes()
- .id(irrelevantChangeId)
+ .id(irrelevanetChangeIdentifier)
.edit()
.modifyFile(FILE_NAME, RawInputUtil.create("foo".getBytes(UTF_8)));
- gApi.changes().id(changeId).delete();
+ gApi.changes().id(changeIdentifier).delete();
- assertThat(gApi.changes().id(irrelevantChangeId).edit().get()).isPresent();
+ assertThat(gApi.changes().id(irrelevanetChangeIdentifier).edit().get()).isPresent();
}
@Test
public void attentionSetListener_firesOnChange() throws Exception {
- PushOneCommit.Result r1 = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().createV2();
TestAttentionSetListener attentionSetListener = new TestAttentionSetListener();
try (Registration registration =
extensionRegistry.newRegistration().add(attentionSetListener)) {
- gApi.changes().id(r1.getChangeId()).addReviewer(user.email());
+ gApi.changes().id(changeIdentifier).addReviewer(user.email());
assertThat(attentionSetListener.firedCount).isEqualTo(1);
assertThat(attentionSetListener.lastEvent.usersAdded().size()).isEqualTo(1);
attentionSetListener
@@ -1255,12 +1239,12 @@
// Adding the user with the same reason doesn't fire an event.
AttentionSetInput addUser = new AttentionSetInput(user.email(), "Reviewer was added");
- gApi.changes().id(r1.getChangeId()).addToAttentionSet(addUser);
+ gApi.changes().id(changeIdentifier).addToAttentionSet(addUser);
assertThat(attentionSetListener.firedCount).isEqualTo(1);
// Adding the user with a different reason fires an event.
addUser = new AttentionSetInput(user.email(), "some reason");
- gApi.changes().id(r1.getChangeId()).addToAttentionSet(addUser);
+ gApi.changes().id(changeIdentifier).addToAttentionSet(addUser);
assertThat(attentionSetListener.firedCount).isEqualTo(2);
assertThat(attentionSetListener.lastEvent.usersAdded().size()).isEqualTo(1);
attentionSetListener
@@ -1269,7 +1253,7 @@
.forEach(u -> assertThat(u).isEqualTo(user.id().get()));
// Removing the user fires an event.
- gApi.changes().id(r1.getChangeId()).attention(user.username()).remove(addUser);
+ gApi.changes().id(changeIdentifier).attention(user.username()).remove(addUser);
assertThat(attentionSetListener.firedCount).isEqualTo(3);
assertThat(attentionSetListener.lastEvent.usersAdded()).isEmpty();
assertThat(attentionSetListener.lastEvent.usersRemoved().size()).isEqualTo(1);
@@ -1402,21 +1386,18 @@
.update();
// create change
- TestRepository<InMemoryRepository> repo = cloneProject(p, admin);
- PushOneCommit push = pushFactory.create(admin.newIdent(), repo);
- PushOneCommit.Result result = push.to("refs/for/master");
- result.assertOkStatus();
+ ChangeIdentifier changeIdentifier =
+ changeOperations.newChange().project(p).owner(admin.id()).createV2();
// check the user cannot see the change
requestScopeOperations.setApiUser(user.id());
- assertThrows(
- ResourceNotFoundException.class, () -> gApi.changes().id(result.getChangeId()).get());
+ assertThrows(ResourceNotFoundException.class, () -> gApi.changes().id(changeIdentifier).get());
// try to add user as reviewer
requestScopeOperations.setApiUser(admin.id());
ReviewerInput in = new ReviewerInput();
in.reviewer = user.email();
- ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
+ ReviewerResult r = gApi.changes().id(changeIdentifier).addReviewer(in);
assertThat(r.input).isEqualTo(user.email());
assertThat(r.error).contains("does not have permission to see this change");
@@ -1425,14 +1406,14 @@
@Test
public void addReviewerThatIsInactiveByUsername() throws Exception {
- PushOneCommit.Result result = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(admin.id()).createV2();
String username = name("new-user");
Account.Id id = accountOperations.newAccount().username(username).inactive().create();
ReviewerInput in = new ReviewerInput();
in.reviewer = username;
- ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
+ ReviewerResult r = gApi.changes().id(changeIdentifier).addReviewer(in);
assertThat(r.input).isEqualTo(in.reviewer);
assertThat(r.error).isNull();
@@ -1446,14 +1427,14 @@
@Test
public void addReviewerThatIsInactiveById() throws Exception {
- PushOneCommit.Result result = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(admin.id()).createV2();
String username = name("new-user");
Account.Id id = accountOperations.newAccount().username(username).inactive().create();
ReviewerInput in = new ReviewerInput();
in.reviewer = Integer.toString(id.get());
- ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
+ ReviewerResult r = gApi.changes().id(changeIdentifier).addReviewer(in);
assertThat(r.input).isEqualTo(in.reviewer);
assertThat(r.error).isNull();
@@ -1470,14 +1451,14 @@
ConfigInput conf = new ConfigInput();
conf.enableReviewerByEmail = InheritableBoolean.TRUE;
gApi.projects().name(project.get()).config(conf);
- PushOneCommit.Result result = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(admin.id()).createV2();
String email = "user@domain.com";
Account.Id id = accountOperations.newAccount().preferredEmail(email).inactive().create();
ReviewerInput in = new ReviewerInput();
in.reviewer = email;
in.state = ReviewerState.CC;
- ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
+ ReviewerResult r = gApi.changes().id(changeIdentifier).addReviewer(in);
assertThat(r.input).isEqualTo(email);
assertThat(r.error).isNull();
@@ -1491,10 +1472,10 @@
@UseClockStep
public void addReviewer() throws Exception {
testAddReviewerViaPostReview(
- (changeId, reviewer) -> {
+ (changeIdentifier, reviewer) -> {
ReviewerInput in = new ReviewerInput();
in.reviewer = reviewer;
- gApi.changes().id(changeId).addReviewer(in);
+ gApi.changes().id(changeIdentifier).addReviewer(in);
});
}
@@ -1502,21 +1483,25 @@
@UseClockStep
public void addReviewerViaPostReview() throws Exception {
testAddReviewerViaPostReview(
- (changeId, reviewer) -> {
+ (changeIdentifier, reviewer) -> {
ReviewerInput reviewerInput = new ReviewerInput();
reviewerInput.reviewer = reviewer;
ReviewInput reviewInput = new ReviewInput();
reviewInput.reviewers = ImmutableList.of(reviewerInput);
- gApi.changes().id(changeId).current().review(reviewInput);
+ gApi.changes().id(changeIdentifier).current().review(reviewInput);
});
}
private void testAddReviewerViaPostReview(AddReviewerCaller addReviewer) throws Exception {
- PushOneCommit.Result r = createChange();
- ChangeResource rsrc = parseResource(r);
- Instant oldTs = rsrc.getChange().getLastUpdatedOn();
+ TestChange change =
+ changeOperations
+ .newChange()
+ .owner(admin.id())
+ .commitMessage(PushOneCommit.SUBJECT)
+ .createAndGet();
+ Instant oldTs = change.lastUpdatedOn();
- addReviewer.call(r.getChangeId(), user.email());
+ addReviewer.call(change.id(), user.email());
ImmutableList<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
@@ -1526,7 +1511,7 @@
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertMailReplyTo(m, admin.email());
- ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
+ ChangeInfo c = gApi.changes().id(change.id()).get();
// Adding a reviewer records that user as reviewer.
Collection<AccountInfo> reviewers = c.reviewers.get(REVIEWER);
@@ -1538,30 +1523,31 @@
assertThat(c.reviewers.get(CC)).isNull();
// Ensure lastUpdatedOn is updated.
- rsrc = parseResource(r);
- assertThat(rsrc.getChange().getLastUpdatedOn()).isNotEqualTo(oldTs);
+ assertThat(changeOperations.change(change.id()).get().lastUpdatedOn()).isNotEqualTo(oldTs);
}
@Test
public void postingMessageOnOwnChangeDoesntAddCallerAsReviewer() throws Exception {
- PushOneCommit.Result r = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(admin.id()).createV2();
+ requestScopeOperations.setApiUser(admin.id());
ReviewInput reviewInput = new ReviewInput();
reviewInput.message = "Foo Bar";
- gApi.changes().id(r.getChangeId()).current().review(reviewInput);
+ gApi.changes().id(changeIdentifier).current().review(reviewInput);
- ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
+ ChangeInfo c = gApi.changes().id(changeIdentifier).get();
assertThat(c.reviewers.get(REVIEWER)).isNull();
assertThat(c.reviewers.get(CC)).isNull();
}
@Test
public void listReviewers() throws Exception {
- PushOneCommit.Result r = createChange();
+ ChangeIdentifier changeIdentifier = changeOperations.newChange().owner(admin.id()).createV2();
+
ReviewerInput in = new ReviewerInput();
in.reviewer = user.email();
- gApi.changes().id(r.getChangeId()).addReviewer(in);
- assertThat(gApi.changes().id(r.getChangeId()).reviewers()).hasSize(1);
+ gApi.changes().id(changeIdentifier).addReviewer(in);
+ assertThat(gApi.changes().id(changeIdentifier).reviewers()).hasSize(1);
String username1 = name("user1");
String email1 = username1 + "@example.com";
@@ -1574,12 +1560,12 @@
.create();
in.reviewer = email1;
in.state = ReviewerState.CC;
- gApi.changes().id(r.getChangeId()).addReviewer(in);
+ gApi.changes().id(changeIdentifier).addReviewer(in);
if (server.isUsernameSupported()) {
- assertThat(gApi.changes().id(r.getChangeId()).reviewers().stream().map(a -> a.username))
+ assertThat(gApi.changes().id(changeIdentifier).reviewers().stream().map(a -> a.username))
.containsExactly(user.username(), username1);
}
- assertThat(gApi.changes().id(r.getChangeId()).reviewers().stream().map(a -> a._accountId))
+ assertThat(gApi.changes().id(changeIdentifier).reviewers().stream().map(a -> a._accountId))
.containsExactly(user.id().get(), user1Id.get());
}
@@ -5010,7 +4996,7 @@
@FunctionalInterface
private interface AddReviewerCaller {
- void call(String changeId, String reviewer) throws RestApiException;
+ void call(ChangeIdentifier changeIdentifier, String reviewer) throws RestApiException;
}
public static class TestAttentionSetListenerModule extends AbstractModule {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java b/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java
index 710ae54..bde14a2 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java
@@ -21,7 +21,6 @@
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
-import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -305,12 +304,10 @@
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String fileContent = new String(os.toByteArray(), UTF_8);
- String sourceSha1 = abbreviateName(changeA.getCommit(), 6);
- String targetSha1 = abbreviateName(currentMaster.getCommit(), 6);
assertThat(fileContent)
.isEqualTo(
"<<<<<<< TARGET BRANCH ("
- + targetSha1
+ + currentMaster.getCommit().getName()
+ " "
+ targetSubject
+ ")\n"
@@ -320,7 +317,7 @@
+ sourceContent
+ "\n"
+ ">>>>>>> SOURCE BRANCH ("
- + sourceSha1
+ + changeA.getCommit().getName()
+ " "
+ sourceSubject
+ ")\n");
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
index 30f500e..b05e7be 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
@@ -449,7 +449,8 @@
accountOperations.newAccount().preferredEmail(reviewerEmail).create();
// Create the change.
- Change.Id changeId = changeOperations.newChange().owner(nonVisibleOwner).create();
+ Change.Id changeId =
+ changeOperations.newChange().project(project).owner(nonVisibleOwner).create();
// Add a review.
requestScopeOperations.setApiUser(nonVisibleReviewer);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
index e63e8d7..f2433c2 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
@@ -23,7 +23,6 @@
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
-import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
import static com.google.gerrit.server.project.testing.TestLabels.value;
@@ -506,7 +505,6 @@
.content(mergeContent)
.create();
String commitThatIsBeingRebased = getCurrentRevision(mergeChangeId);
- String mergeSha1 = abbreviateName(ObjectId.fromString(commitThatIsBeingRebased), 6);
// Create a change in master onto which the merge change can be rebased. This change touches
// the file again so that there is a conflict on rebase.
@@ -549,11 +547,10 @@
/* expectedNumRevisions= */ 2);
// Verify the file contents.
- String baseSha1 = abbreviateName(ObjectId.fromString(baseCommit), 6);
assertThat(getFileContent(mergeChangeId, file))
.isEqualTo(
"<<<<<<< PATCH SET ("
- + mergeSha1
+ + commitThatIsBeingRebased
+ " "
+ mergeCommitMessage
+ ")\n"
@@ -563,7 +560,7 @@
+ newBaseContent
+ "\n"
+ ">>>>>>> BASE ("
- + baseSha1
+ + baseCommit
+ " "
+ newBaseCommitMessage
+ ")\n");
@@ -1584,12 +1581,10 @@
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String fileContent = new String(os.toByteArray(), UTF_8);
- String patchSetSha1 = abbreviateName(patchSet, 6);
- String baseSha1 = abbreviateName(base, 6);
assertThat(fileContent)
.isEqualTo(
"<<<<<<< PATCH SET ("
- + patchSetSha1
+ + patchSet.getName()
+ " "
+ patchSetSubject
+ ")\n"
@@ -1599,7 +1594,7 @@
+ baseContent
+ "\n"
+ ">>>>>>> BASE ("
- + baseSha1
+ + base.getName()
+ " "
+ baseSubject
+ ")\n");
@@ -2411,12 +2406,10 @@
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String fileContent = new String(os.toByteArray(), UTF_8);
- String patchSetSha1 = abbreviateName(parentPatchSet, 6);
- String baseSha1 = abbreviateName(base, 6);
assertThat(fileContent)
.isEqualTo(
"<<<<<<< PATCH SET ("
- + patchSetSha1
+ + parentPatchSet.getName()
+ " "
+ patchSetSubject
+ ")\n"
@@ -2426,7 +2419,7 @@
+ baseContent
+ "\n"
+ ">>>>>>> BASE ("
- + baseSha1
+ + base.getName()
+ " "
+ baseSubject
+ ")\n");
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
index a29244f..e5604b6 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
@@ -413,7 +413,12 @@
.create();
Change.Id changeId =
- changeOperations.newChange().author(authorId).committer(committerId).create();
+ changeOperations
+ .newChange()
+ .project(project)
+ .author(authorId)
+ .committer(committerId)
+ .create();
ChangeInfo changeInfo = gApi.changes().id(changeId.get()).get();
assertAuthor(changeInfo, "authoremail@example.com");
@@ -442,7 +447,12 @@
.create();
Change.Id changeId =
- changeOperations.newChange().author(authorId).committer(committerId).create();
+ changeOperations
+ .newChange()
+ .project(project)
+ .author(authorId)
+ .committer(committerId)
+ .create();
ChangeInfo changeInfo = gApi.changes().id(changeId.get()).get();
assertCommitter(changeInfo, "committeremail@example.com");
diff --git a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java
index fb6259b..0dc0604 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java
@@ -22,7 +22,6 @@
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
-import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -397,12 +396,10 @@
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String fileContent = new String(os.toByteArray(), UTF_8);
- String destSha1 = abbreviateName(projectOperations.project(project).getHead(destBranch), 6);
- String changeSha1 = abbreviateName(r.getCommit(), 6);
assertThat(fileContent)
.isEqualTo(
"<<<<<<< HEAD ("
- + destSha1
+ + projectOperations.project(project).getHead(destBranch).getName()
+ " test commit)\n"
+ destContent
+ "\n"
@@ -410,7 +407,7 @@
+ changeContent
+ "\n"
+ ">>>>>>> CHANGE ("
- + changeSha1
+ + r.getCommit().getName()
+ " test commit)\n");
}
@@ -488,12 +485,10 @@
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String fileContent = new String(os.toByteArray(), UTF_8);
- String destSha1 = abbreviateName(existingChange.getCommit(), 6);
- String changeSha1 = abbreviateName(srcChange.getCommit(), 6);
assertThat(fileContent)
.isEqualTo(
"<<<<<<< HEAD ("
- + destSha1
+ + existingChange.getCommit().getName()
+ " test commit)\n"
+ destContent
+ "\n"
@@ -501,7 +496,7 @@
+ changeContent
+ "\n"
+ ">>>>>>> CHANGE ("
- + changeSha1
+ + srcChange.getCommit().getName()
+ " test commit)\n");
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java
index 53d9bda..f4a97bd 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java
@@ -1560,10 +1560,12 @@
@Test
public void commentOnParentIsPortedToNewPosition() throws Exception {
// Set up change and patchsets.
- Change.Id parentChangeId = changeOps.newChange().file("myFile").content("Line 1\n").create();
+ Change.Id parentChangeId =
+ changeOps.newChange().project(project).file("myFile").content("Line 1\n").create();
Change.Id childChangeId =
changeOps
.newChange()
+ .project(project)
.childOf()
.change(parentChangeId)
.file("myFile")
@@ -1592,11 +1594,14 @@
@Test
public void commentOnFirstParentIsPortedToNewPosition() throws Exception {
// Set up change and patchsets.
- Change.Id parent1ChangeId = changeOps.newChange().file("file1").content("Line 1\n").create();
- Change.Id parent2ChangeId = changeOps.newChange().file("file2").content("Line 1\n").create();
+ Change.Id parent1ChangeId =
+ changeOps.newChange().project(project).file("file1").content("Line 1\n").create();
+ Change.Id parent2ChangeId =
+ changeOps.newChange().project(project).file("file2").content("Line 1\n").create();
Change.Id childChangeId =
changeOps
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -1634,11 +1639,14 @@
@Test
public void commentOnSecondParentIsPortedToNewPosition() throws Exception {
// Set up change and patchsets.
- Change.Id parent1ChangeId = changeOps.newChange().file("file1").content("Line 1\n").create();
- Change.Id parent2ChangeId = changeOps.newChange().file("file2").content("Line 1\n").create();
+ Change.Id parent1ChangeId =
+ changeOps.newChange().project(project).file("file1").content("Line 1\n").create();
+ Change.Id parent2ChangeId =
+ changeOps.newChange().project(project).file("file2").content("Line 1\n").create();
Change.Id childChangeId =
changeOps
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -1677,11 +1685,14 @@
public void commentOnAutoMergeCommitIsPortedToNewPosition() throws Exception {
// Set up change and patchsets. Use the same file so that there's a meaningful auto-merge
// commit/diff.
- Change.Id parent1ChangeId = changeOps.newChange().file("file1").content("Line 1\n").create();
- Change.Id parent2ChangeId = changeOps.newChange().file("file1").content("Line 1\n").create();
+ Change.Id parent1ChangeId =
+ changeOps.newChange().project(project).file("file1").content("Line 1\n").create();
+ Change.Id parent2ChangeId =
+ changeOps.newChange().project(project).file("file1").content("Line 1\n").create();
Change.Id childChangeId =
changeOps
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -1729,11 +1740,14 @@
public void commentOnFirstParentIsPortedToSingleParentWhenPatchsetChangedToNonMergeCommit()
throws Exception {
// Set up change and patchsets.
- Change.Id parent1ChangeId = changeOps.newChange().file("file1").content("Line 1\n").create();
- Change.Id parent2ChangeId = changeOps.newChange().file("file2").content("Line 1\n").create();
+ Change.Id parent1ChangeId =
+ changeOps.newChange().project(project).file("file1").content("Line 1\n").create();
+ Change.Id parent2ChangeId =
+ changeOps.newChange().project(project).file("file2").content("Line 1\n").create();
Change.Id childChangeId =
changeOps
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -1770,11 +1784,14 @@
public void commentOnSecondParentBecomesPatchsetLevelCommentWhenPatchsetChangedToNonMergeCommit()
throws Exception {
// Set up change and patchsets.
- Change.Id parent1ChangeId = changeOps.newChange().file("file1").content("Line 1\n").create();
- Change.Id parent2ChangeId = changeOps.newChange().file("file2").content("Line 1\n").create();
+ Change.Id parent1ChangeId =
+ changeOps.newChange().project(project).file("file1").content("Line 1\n").create();
+ Change.Id parent2ChangeId =
+ changeOps.newChange().project(project).file("file2").content("Line 1\n").create();
Change.Id childChangeId =
changeOps
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 7eb33b6..7d82d99 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -3068,6 +3068,7 @@
Change.Id changeId1 =
changeOperations
.newChange()
+ .project(project)
.file(imageFileName)
.content(new String(imageBytes, UTF_8))
.create();
@@ -3076,6 +3077,7 @@
Change.Id changeId2 =
changeOperations
.newChange()
+ .project(project)
.childOf()
.change(changeId1)
.file(imageFileName)
@@ -3106,6 +3108,7 @@
Change.Id changeId1 =
changeOperations
.newChange()
+ .project(project)
.file(imageFileName)
.content(new String(imageBytes, UTF_8))
.create();
@@ -3113,6 +3116,7 @@
Change.Id changeId2 =
changeOperations
.newChange()
+ .project(project)
.childOf()
.change(changeId1)
.file(imageFileName)
@@ -3142,13 +3146,19 @@
byte[] imageBytes1 = createRgbImage(255, 0, 0);
String imageContent1 = new String(imageBytes1, UTF_8);
Change.Id changeId1 =
- changeOperations.newChange().file(imageFileName1).content(imageContent1).create();
+ changeOperations
+ .newChange()
+ .project(project)
+ .file(imageFileName1)
+ .content(imageContent1)
+ .create();
String imageFileName2 = "another_image.png";
byte[] imageBytes2 = createRgbImage(0, 255, 0);
Change.Id changeId2 =
changeOperations
.newChange()
+ .project(project)
.childOf()
.change(changeId1)
.file(imageFileName2)
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index de9a040..37e5511 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -31,7 +31,6 @@
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
-import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -825,12 +824,10 @@
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String fileContent = new String(os.toByteArray(), UTF_8);
- String destSha1 = abbreviateName(projectOperations.project(project).getHead(destBranch), 6);
- String changeSha1 = abbreviateName(r.getCommit(), 6);
assertThat(fileContent)
.isEqualTo(
"<<<<<<< HEAD ("
- + destSha1
+ + projectOperations.project(project).getHead(destBranch).getName()
+ " test commit)\n"
+ destContent
+ "\n"
@@ -838,7 +835,7 @@
+ changeContent
+ "\n"
+ ">>>>>>> CHANGE ("
- + changeSha1
+ + r.getCommit().getName()
+ " test commit)\n");
// Get details of cherry-pick change.
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 918916b..fe7857f 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -48,6 +48,7 @@
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.Patch.FileMode;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
@@ -77,7 +78,6 @@
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.restapi.change.ChangeEdits;
@@ -89,6 +89,7 @@
import com.google.inject.Inject;
import java.io.IOException;
import java.sql.Timestamp;
+import java.util.Base64;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -109,7 +110,6 @@
private static final String FILE_NAME2 = "foo2";
private static final String FILE_NAME3 = "foo3";
private static final String FILE_NAME4 = "foo4";
- private static final int FILE_MODE = 100644;
private static final String CONTENT_OLD_STR = "bar";
private static final byte[] CONTENT_OLD = CONTENT_OLD_STR.getBytes(UTF_8);
private static final String CONTENT_NEW_STR = "baz";
@@ -338,11 +338,11 @@
+ "=======\n"
+ "%s\n"
+ ">>>>>>> EDIT (%s %s)\n",
- ObjectIds.abbreviateName(currentPatchSet.commitId(), 6),
+ currentPatchSet.commitId().getName(),
gApi.changes().id(changeId).get().subject,
CONTENT_NEW2_STR,
CONTENT_NEW_STR,
- ObjectIds.abbreviateName(ObjectId.fromString(originalEdit.get().commit.commit), 6),
+ originalEdit.get().commit.commit,
originalEdit.get().commit.subject)
.getBytes(UTF_8));
assertThat(rebasedEdit).baseRevision().isEqualTo(currentPatchSet.commitId().name());
@@ -383,12 +383,12 @@
+ "=======\n"
+ "%s\n"
+ ">>>>>>> EDIT (%s %s)\n",
- ObjectIds.abbreviateName(currentPatchSet.commitId(), 6),
+ currentPatchSet.commitId().getName(),
gApi.changes().id(changeId).get().subject,
CONTENT_NEW2_STR,
CONTENT_OLD_STR,
CONTENT_NEW_STR,
- ObjectIds.abbreviateName(ObjectId.fromString(originalEdit.get().commit.commit), 6),
+ originalEdit.get().commit.commit,
originalEdit.get().commit.subject)
.getBytes(UTF_8));
assertThat(rebasedEdit).baseRevision().isEqualTo(currentPatchSet.commitId().name());
@@ -1217,21 +1217,148 @@
}
@Test
- public void changeEditModifyFileModeRest() throws Exception {
- createEmptyEditFor(changeId);
+ public void changeEditNewFileIsCreatedAsRegularFileIfFileModeIsNotSet() throws Exception {
+ String fileName = "new.txt";
+
FileContentInput in = new FileContentInput();
- in.binary_content = CONTENT_BINARY_ENCODED_NEW;
- in.fileMode = FILE_MODE;
- adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertNoContent();
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW;
+ gApi.changes().id(changeId).edit().modifyFile(fileName, in);
+
+ ensureSameBytes(getFileContentOfEdit(changeId, fileName), CONTENT_BINARY_DECODED_NEW);
+
+ // Publish the change edit so that we can assert the file mode.
+ gApi.changes().id(changeId).edit().publish(new PublishChangeEditInput());
+
+ int mode = gApi.changes().id(changeId).get().getCurrentRevision().files.get(fileName).newMode;
+ assertThat(mode).isEqualTo(FileMode.REGULAR_FILE.getMode());
+ }
+
+ @Test
+ public void changeEditModifyingFileDoesNotUpdateFileModeIfFileModeIsNotSet() throws Exception {
+ FileContentInput in = new FileContentInput();
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW;
+ in.fileMode = FileMode.EXECUTABLE_FILE.getModeAsOctal();
+ gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, in);
+
ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_BINARY_DECODED_NEW);
+
+ // Update the file content without setting the file mode:
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW2;
+ in.fileMode = null;
+ gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, in);
+
+ ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_BINARY_DECODED_NEW2);
+
+ // Publish the change edit so that we can assert the file mode.
+ gApi.changes().id(changeId).edit().publish(new PublishChangeEditInput());
+
+ int mode = gApi.changes().id(changeId).get().getCurrentRevision().files.get(FILE_NAME).newMode;
+ assertThat(mode).isEqualTo(FileMode.EXECUTABLE_FILE.getMode());
+ }
+
+ @Test
+ public void changeEditModifyFileMode() throws Exception {
+ int mode = gApi.changes().id(changeId).get().getCurrentRevision().files.get(FILE_NAME).newMode;
+ assertThat(mode).isEqualTo(FileMode.REGULAR_FILE.getMode());
+
+ FileContentInput in = new FileContentInput();
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW;
+ in.fileMode = FileMode.EXECUTABLE_FILE.getModeAsOctal();
+ gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, in);
+
+ ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_BINARY_DECODED_NEW);
+
+ // Publish the change edit so that we can assert the new file mode.
+ gApi.changes().id(changeId).edit().publish(new PublishChangeEditInput());
+
+ mode = gApi.changes().id(changeId).get().getCurrentRevision().files.get(FILE_NAME).newMode;
+ assertThat(mode).isEqualTo(FileMode.EXECUTABLE_FILE.getMode());
+ }
+
+ @Test
+ public void changeEditCreateGitLink() throws Exception {
+ String gitlinkFileName = "foo/bar";
+ byte[] gitlinkContent = "7bcbf0ae382f9a20f96563b6af2c0e6fff20a22f".getBytes(UTF_8);
+
+ FileContentInput in = new FileContentInput();
+ in.binaryContent =
+ "data:text/plain;base64," + Base64.getEncoder().encodeToString(gitlinkContent);
+ in.fileMode = FileMode.GITLINK.getModeAsOctal();
+ gApi.changes().id(changeId).edit().modifyFile(gitlinkFileName, in);
+
+ ensureSameBytes(getFileContentOfEdit(changeId, gitlinkFileName), gitlinkContent);
+
+ // Publish the change edit so that we can assert the file mode.
+ gApi.changes().id(changeId).edit().publish(new PublishChangeEditInput());
+
+ int mode =
+ gApi.changes().id(changeId).get().getCurrentRevision().files.get(gitlinkFileName).newMode;
+ assertThat(mode).isEqualTo(FileMode.GITLINK.getMode());
+ }
+
+ @Test
+ public void changeEditCreateSymLink() throws Exception {
+ String symlinkFileName = "foo/bar";
+ byte[] symlinkContent = "path/to/target".getBytes(UTF_8);
+
+ FileContentInput in = new FileContentInput();
+ in.binaryContent =
+ "data:text/plain;base64," + Base64.getEncoder().encodeToString(symlinkContent);
+ in.fileMode = FileMode.SYMLINK.getModeAsOctal();
+ gApi.changes().id(changeId).edit().modifyFile(symlinkFileName, in);
+
+ ensureSameBytes(getFileContentOfEdit(changeId, symlinkFileName), symlinkContent);
+
+ // Publish the change edit so that we can assert the file mode.
+ gApi.changes().id(changeId).edit().publish(new PublishChangeEditInput());
+
+ int mode =
+ gApi.changes().id(changeId).get().getCurrentRevision().files.get(symlinkFileName).newMode;
+ assertThat(mode).isEqualTo(FileMode.SYMLINK.getMode());
+ }
+
+ @Test
+ public void changeEditCreateGitLink_invalidContentIsRejected() throws Exception {
+ String gitlinkFileName = "foo/bar";
+ byte[] invalidGitlinkContent = "not a SHA1".getBytes(UTF_8);
+
+ FileContentInput in = new FileContentInput();
+ in.binaryContent =
+ "data:text/plain;base64," + Base64.getEncoder().encodeToString(invalidGitlinkContent);
+ in.fileMode = FileMode.GITLINK.getModeAsOctal();
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(changeId).edit().modifyFile(gitlinkFileName, in));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format("The content for gitlink '%s' must be a valid SHA1.", gitlinkFileName));
+ }
+
+ @Test
+ public void changeEditModifyFileMode_invalidFileModeIsRejected() throws Exception {
+ FileContentInput in = new FileContentInput();
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW;
+ in.fileMode = 16;
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, in));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "file mode %s is invalid: supported values are 100644 (regular file), 100755"
+ + " (executable file), 120000 (symlink) and 160000 (gitlink)",
+ in.fileMode));
}
@Test
public void changeEditModifyFileSetEmptyContentModeRest() throws Exception {
createEmptyEditFor(changeId);
FileContentInput in = new FileContentInput();
- in.binary_content = CONTENT_BINARY_ENCODED_EMPTY;
- in.fileMode = FILE_MODE;
+ in.binaryContent = CONTENT_BINARY_ENCODED_EMPTY;
adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertNoContent();
ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), "".getBytes(UTF_8));
}
@@ -1239,10 +1366,10 @@
@Test
public void createAndUploadBinaryInChangeEditOneRequestRest() throws Exception {
FileContentInput in = new FileContentInput();
- in.binary_content = CONTENT_BINARY_ENCODED_NEW;
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW;
adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertNoContent();
ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_BINARY_DECODED_NEW);
- in.binary_content = CONTENT_BINARY_ENCODED_NEW2;
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW2;
adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertNoContent();
ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_BINARY_DECODED_NEW2);
}
@@ -1250,7 +1377,7 @@
@Test
public void invalidBase64UploadBinaryInChangeEditOneRequestRest() throws Exception {
FileContentInput in = new FileContentInput();
- in.binary_content = CONTENT_BINARY_ENCODED_NEW3;
+ in.binaryContent = CONTENT_BINARY_ENCODED_NEW3;
adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertBadRequest();
}
@@ -1259,7 +1386,7 @@
createEmptyEditFor(changeId);
FileContentInput in = new FileContentInput();
- in.binary_content = null;
+ in.binaryContent = null;
adminRestSession.put(urlEditFile(changeId, FILE_NAME), in).assertBadRequest();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index ef6e59d..bc357a8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -28,7 +28,6 @@
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.common.testing.GitPersonSubject.assertThat;
-import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.testing.TestActionRefUpdateContext.testRefAction;
@@ -806,12 +805,10 @@
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String fileContent = new String(os.toByteArray(), UTF_8);
- String sourceSha1 = abbreviateName(projectOperations.project(project).getHead(sourceBranch), 6);
- String targetSha1 = abbreviateName(projectOperations.project(project).getHead(targetBranch), 6);
assertThat(fileContent)
.isEqualTo(
"<<<<<<< TARGET BRANCH ("
- + targetSha1
+ + projectOperations.project(project).getHead(targetBranch).getName()
+ " "
+ targetSubject
+ ")\n"
@@ -821,7 +818,7 @@
+ sourceContent
+ "\n"
+ ">>>>>>> SOURCE BRANCH ("
- + sourceSha1
+ + projectOperations.project(project).getHead(sourceBranch).getName()
+ " "
+ sourceSubject
+ ")\n");
@@ -918,6 +915,64 @@
}
@Test
+ public void updateCommitMessageOfMergeChangeWithConflicts() throws Exception {
+ // Create a change with a merge commit that contains conflicts
+ String sourceBranch = "source";
+ String targetBranch = "target";
+ ImmutableMap<String, Result> results =
+ changeInTwoBranches(
+ sourceBranch,
+ "source change",
+ "shared.txt",
+ "source content",
+ targetBranch,
+ "target change",
+ "shared.txt",
+ "target content");
+ ChangeInput in =
+ newMergeChangeInput(targetBranch, sourceBranch, "", /* allowConflicts= */ true);
+ in.subject = "Merge " + sourceBranch + " to " + targetBranch;
+ ChangeInfo change = assertCreateSucceedsWithConflicts(in);
+
+ // Verify the conflicts information
+ RevisionInfo currentRevision =
+ gApi.changes().id(change.id).get(CURRENT_REVISION, CURRENT_COMMIT).getCurrentRevision();
+ assertThat(currentRevision._number).isEqualTo(1);
+ assertThat(currentRevision.commit.parents.get(0).commit)
+ .isEqualTo(results.get(targetBranch).getCommit().name());
+ assertThat(currentRevision.conflicts).isNotNull();
+ assertThat(currentRevision.conflicts.ours)
+ .isEqualTo(results.get(targetBranch).getCommit().name());
+ assertThat(currentRevision.conflicts.theirs)
+ .isEqualTo(results.get(sourceBranch).getCommit().name());
+ assertThat(currentRevision.conflicts.containsConflicts).isTrue();
+
+ // Update the commit message
+ gApi.changes()
+ .id(change.id)
+ .setMessage(
+ "Merge "
+ + sourceBranch
+ + " branch into "
+ + targetBranch
+ + " branch\n\nChange-Id: "
+ + change.changeId);
+
+ // Verify that the conflicts information has been copied
+ currentRevision =
+ gApi.changes().id(change.id).get(CURRENT_REVISION, CURRENT_COMMIT).getCurrentRevision();
+ assertThat(currentRevision._number).isEqualTo(2);
+ assertThat(currentRevision.commit.parents.get(0).commit)
+ .isEqualTo(results.get(targetBranch).getCommit().name());
+ assertThat(currentRevision.conflicts).isNotNull();
+ assertThat(currentRevision.conflicts.ours)
+ .isEqualTo(results.get(targetBranch).getCommit().name());
+ assertThat(currentRevision.conflicts.theirs)
+ .isEqualTo(results.get(sourceBranch).getCommit().name());
+ assertThat(currentRevision.conflicts.containsConflicts).isTrue();
+ }
+
+ @Test
public void invalidSource() throws Exception {
changeInTwoBranches("branchA", "a.txt", "branchB", "b.txt");
ChangeInput in = newMergeChangeInput("branchA", "invalid", "");
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
index 8f0624c..4dc64b9 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -576,10 +576,12 @@
@Test
public void byFileEdits_deletedContent_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 2\n", ""))
.childOf()
@@ -596,10 +598,12 @@
@Test
public void byFileEdits_deletedContent_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 1\n", ""))
.childOf()
@@ -616,10 +620,12 @@
@Test
public void byFileEdits_addedContent_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT + "line 4\n")
.childOf()
@@ -636,10 +642,12 @@
@Test
public void byFileEdits_addedContent_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT + "line 4\n")
.childOf()
@@ -656,10 +664,12 @@
@Test
public void byFileEdits_addedFile_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file("new_file.txt")
.content("content of the new file")
.childOf()
@@ -677,10 +687,12 @@
@Test
public void byFileEdits_addedFile_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file("new_file.txt")
.content("content of the new file")
.childOf()
@@ -698,10 +710,12 @@
@Test
public void byFileEdits_modifiedContent_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line three\n"))
.childOf()
@@ -718,10 +732,12 @@
@Test
public void byFileEdits_modifiedContent_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line three\n"))
.childOf()
@@ -738,10 +754,12 @@
@Test
public void byFileEdits_modifiedContentPattern_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line three\n"))
.childOf()
@@ -759,10 +777,12 @@
@Test
public void byFileEdits_exactMatchingWithFilePath_matching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line three\n"))
.childOf()
@@ -780,10 +800,12 @@
@Test
public void byFileEdits_exactMatchingWithFilePath_nonMatching() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line three\n"))
.childOf()
@@ -801,10 +823,12 @@
@Test
public void byFileEdits_notMatchingWithFilePath() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line three\n"))
.childOf()
@@ -823,10 +847,12 @@
@Test
public void byFileEdits_escapeSingleQuotes() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line 'three' is modified\n"))
.childOf()
@@ -844,10 +870,12 @@
@Test
public void byFileEdits_doubleEscapeSingleQuote() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
// This will be written to the file as: line \'three\' is modified.
.content(CONTENT.replace("line 3\n", "line \\'three\\' is modified\n"))
@@ -867,10 +895,12 @@
@Test
public void byFileEdits_escapeDoubleQuotes() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line \"three\" is modified\n"))
.childOf()
@@ -888,10 +918,12 @@
@Test
public void byFileEdits_invalidSyntax() throws Exception {
- Change.Id parent = changeOperations.newChange().file(FILE_NAME).content(CONTENT).create();
+ Change.Id parent =
+ changeOperations.newChange().project(project).file(FILE_NAME).content(CONTENT).create();
Change.Id childId =
changeOperations
.newChange()
+ .project(project)
.file(FILE_NAME)
.content(CONTENT.replace("line 3\n", "line three\n"))
.childOf()
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index b240ee2..87fe3db 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -351,9 +351,10 @@
@Test
public void createdChangeUsesSpecifiedChangeAsParent() throws Exception {
- Change.Id parentChangeId = changeOperations.newChange().create();
+ Change.Id parentChangeId = changeOperations.newChange().project(project).create();
- Change.Id changeId = changeOperations.newChange().childOf().change(parentChangeId).create();
+ Change.Id changeId =
+ changeOperations.newChange().project(project).childOf().change(parentChangeId).create();
ChangeInfo change = getChangeFromServer(changeId);
CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
@@ -377,11 +378,16 @@
@Test
public void createdChangeUsesSpecifiedPatchsetAsParent() throws Exception {
- Change.Id parentChangeId = changeOperations.newChange().create();
+ Change.Id parentChangeId = changeOperations.newChange().project(project).create();
TestPatchset parentPatchset = changeOperations.change(parentChangeId).currentPatchset().get();
Change.Id changeId =
- changeOperations.newChange().childOf().patchset(parentPatchset.patchsetId()).create();
+ changeOperations
+ .newChange()
+ .project(project)
+ .childOf()
+ .patchset(parentPatchset.patchsetId())
+ .create();
ChangeInfo change = getChangeFromServer(changeId);
CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
@@ -425,11 +431,12 @@
@Test
public void createdChangeUsesSpecifiedCommitAsParent() throws Exception {
// Currently, the easiest way to create a commit is by creating another change.
- Change.Id anotherChangeId = changeOperations.newChange().create();
+ Change.Id anotherChangeId = changeOperations.newChange().project(project).create();
ObjectId parentCommitId =
changeOperations.change(anotherChangeId).currentPatchset().get().commitId();
- Change.Id changeId = changeOperations.newChange().childOf().commit(parentCommitId).create();
+ Change.Id changeId =
+ changeOperations.newChange().project(project).childOf().commit(parentCommitId).create();
ChangeInfo change = getChangeFromServer(changeId);
CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
@@ -456,12 +463,13 @@
@Test
public void createdChangeUsesSpecifiedChangesInGivenOrderAsParents() throws Exception {
- Change.Id parent1ChangeId = changeOperations.newChange().create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id parent1ChangeId = changeOperations.newChange().project(project).create();
+ Change.Id parent2ChangeId = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -484,13 +492,19 @@
@Test
public void createdChangeUsesMergedParentsAsBaseCommit() throws Exception {
Change.Id parent1ChangeId =
- changeOperations.newChange().file("file1").content("Line 1").create();
+ changeOperations.newChange().project(project).file("file1").content("Line 1").create();
Change.Id parent2ChangeId =
- changeOperations.newChange().file("file2").content("Some other content").create();
+ changeOperations
+ .newChange()
+ .project(project)
+ .file("file2")
+ .content("Some other content")
+ .create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -507,9 +521,9 @@
@Test
public void mergeConflictsOfParentsAreReported() {
Change.Id parent1ChangeId =
- changeOperations.newChange().file("file1").content("Content 1").create();
+ changeOperations.newChange().project(project).file("file1").content("Content 1").create();
Change.Id parent2ChangeId =
- changeOperations.newChange().file("file1").content("Content 2").create();
+ changeOperations.newChange().project(project).file("file1").content("Content 2").create();
IllegalStateException exception =
assertThrows(
@@ -517,6 +531,7 @@
() ->
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -529,13 +544,14 @@
@Test
public void mergeConflictsCanBeAvoidedByUsingTheFirstParentAsBase() throws Exception {
Change.Id parent1ChangeId =
- changeOperations.newChange().file("file1").content("Content 1").create();
+ changeOperations.newChange().project(project).file("file1").content("Content 1").create();
Change.Id parent2ChangeId =
- changeOperations.newChange().file("file1").content("Content 2").create();
+ changeOperations.newChange().project(project).file("file1").content("Content 2").create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOfButBaseOnFirst()
.change(parent1ChangeId)
.and()
@@ -549,12 +565,13 @@
@Test
public void createdChangeHasAllParentsEvenWhenBasedOnFirst() throws Exception {
- Change.Id parent1ChangeId = changeOperations.newChange().create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id parent1ChangeId = changeOperations.newChange().project(project).create();
+ Change.Id parent2ChangeId = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOfButBaseOnFirst()
.change(parent1ChangeId)
.and()
@@ -603,15 +620,16 @@
@Test
public void createdChangeCanHaveMoreThanTwoParentsWhenBasedOnFirst() throws Exception {
Change.Id parent1ChangeId =
- changeOperations.newChange().file("file1").content("Content 1").create();
+ changeOperations.newChange().project(project).file("file1").content("Content 1").create();
Change.Id parent2ChangeId =
- changeOperations.newChange().file("file2").content("Content 2").create();
+ changeOperations.newChange().project(project).file("file2").content("Content 2").create();
Change.Id parent3ChangeId =
- changeOperations.newChange().file("file3").content("Content 3").create();
+ changeOperations.newChange().project(project).file("file3").content("Content 3").create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOfButBaseOnFirst()
.change(parent1ChangeId)
.followedBy()
@@ -640,6 +658,7 @@
Change.Id parentChangeId =
changeOperations
.newChange()
+ .project(project)
.file("file1")
.content("Content 1")
.file("file2")
@@ -649,6 +668,7 @@
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.childOf()
.change(parentChangeId)
.file("file1")
@@ -665,13 +685,14 @@
@Test
public void changeFromMergedParentsMayHaveAdditionalFileModifications() throws Exception {
Change.Id parent1ChangeId =
- changeOperations.newChange().file("file1").content("Content 1").create();
+ changeOperations.newChange().project(project).file("file1").content("Content 1").create();
Change.Id parent2ChangeId =
- changeOperations.newChange().file("file2").content("Content 2").create();
+ changeOperations.newChange().project(project).file("file2").content("Content 2").create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -691,12 +712,13 @@
public void changeBasedOnFirstOfMultipleParentsMayHaveAdditionalFileModifications()
throws Exception {
Change.Id parent1ChangeId =
- changeOperations.newChange().file("file1").content("Content 1").create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
+ changeOperations.newChange().project(project).file("file1").content("Content 1").create();
+ Change.Id parent2ChangeId = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOfButBaseOnFirst()
.change(parent1ChangeId)
.and()
@@ -1605,14 +1627,24 @@
@Test
public void newPatchsetCanHaveADifferentParent() throws Exception {
- Change.Id originalParentChange = changeOperations.newChange().create();
- Change.Id changeId =
- changeOperations.newChange().childOf().change(originalParentChange).create();
- Change.Id newParentChange = changeOperations.newChange().create();
+ Change.Id originalParentChange = changeOperations.newChange().project(project).create();
+ Change.Id changeIdentifier =
+ changeOperations
+ .newChange()
+ .project(project)
+ .childOf()
+ .change(originalParentChange)
+ .create();
+ Change.Id newParentChange = changeOperations.newChange().project(project).create();
- changeOperations.change(changeId).newPatchset().parent().change(newParentChange).create();
+ changeOperations
+ .change(changeIdentifier)
+ .newPatchset()
+ .parent()
+ .change(newParentChange)
+ .create();
- ChangeInfo change = getChangeFromServer(changeId);
+ ChangeInfo change = getChangeFromServer(changeIdentifier);
CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
ObjectId newParentCommitId =
changeOperations.change(newParentChange).currentPatchset().get().commitId();
@@ -1625,18 +1657,19 @@
@Test
public void newPatchsetCanHaveDifferentParents() throws Exception {
- Change.Id originalParent1Change = changeOperations.newChange().create();
- Change.Id originalParent2Change = changeOperations.newChange().create();
+ Change.Id originalParent1Change = changeOperations.newChange().project(project).create();
+ Change.Id originalParent2Change = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(originalParent1Change)
.and()
.change(originalParent2Change)
.create();
- Change.Id newParent1Change = changeOperations.newChange().create();
- Change.Id newParent2Change = changeOperations.newChange().create();
+ Change.Id newParent1Change = changeOperations.newChange().project(project).create();
+ Change.Id newParent2Change = changeOperations.newChange().project(project).create();
changeOperations
.change(changeId)
@@ -1661,11 +1694,16 @@
@Test
public void newPatchsetCanHaveADifferentNumberOfParents() throws Exception {
- Change.Id originalParentChange = changeOperations.newChange().create();
+ Change.Id originalParentChange = changeOperations.newChange().project(project).create();
Change.Id changeId =
- changeOperations.newChange().childOf().change(originalParentChange).create();
- Change.Id newParent1Change = changeOperations.newChange().create();
- Change.Id newParent2Change = changeOperations.newChange().create();
+ changeOperations
+ .newChange()
+ .project(project)
+ .childOf()
+ .change(originalParentChange)
+ .create();
+ Change.Id newParent1Change = changeOperations.newChange().project(project).create();
+ Change.Id newParent2Change = changeOperations.newChange().project(project).create();
changeOperations
.change(changeId)
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
index be71e6e..2cc91dd 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
@@ -210,11 +210,12 @@
@Test
public void commentCanBeCreatedOnSecondParentCommit() throws Exception {
- Change.Id parent1ChangeId = changeOperations.newChange().create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id parent1ChangeId = changeOperations.newChange().project(project).create();
+ Change.Id parent2ChangeId = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -236,8 +237,9 @@
@Test
public void commentCanBeCreatedOnNonExistingSecondParentCommit() throws Exception {
- Change.Id parentChangeId = changeOperations.newChange().create();
- Change.Id changeId = changeOperations.newChange().childOf().change(parentChangeId).create();
+ Change.Id parentChangeId = changeOperations.newChange().project(project).create();
+ Change.Id changeId =
+ changeOperations.newChange().project(project).childOf().change(parentChangeId).create();
String commentUuid =
changeOperations
@@ -257,11 +259,12 @@
@Test
public void commentCanBeCreatedOnAutoMergeCommit() throws Exception {
- Change.Id parent1ChangeId = changeOperations.newChange().create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id parent1ChangeId = changeOperations.newChange().project(project).create();
+ Change.Id parent2ChangeId = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -578,11 +581,12 @@
@Test
public void draftCommentCanBeCreatedOnSecondParentCommit() throws Exception {
- Change.Id parent1ChangeId = changeOperations.newChange().create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id parent1ChangeId = changeOperations.newChange().project(project).create();
+ Change.Id parent2ChangeId = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
@@ -604,8 +608,9 @@
@Test
public void draftCommentCanBeCreatedOnNonExistingSecondParentCommit() throws Exception {
- Change.Id parentChangeId = changeOperations.newChange().create();
- Change.Id changeId = changeOperations.newChange().childOf().change(parentChangeId).create();
+ Change.Id parentChangeId = changeOperations.newChange().project(project).create();
+ Change.Id changeId =
+ changeOperations.newChange().project(project).childOf().change(parentChangeId).create();
String commentUuid =
changeOperations
@@ -625,11 +630,12 @@
@Test
public void draftCommentCanBeCreatedOnAutoMergeCommit() throws Exception {
- Change.Id parent1ChangeId = changeOperations.newChange().create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id parent1ChangeId = changeOperations.newChange().project(project).create();
+ Change.Id parent2ChangeId = changeOperations.newChange().project(project).create();
Change.Id changeId =
changeOperations
.newChange()
+ .project(project)
.mergeOf()
.change(parent1ChangeId)
.and()
diff --git a/plugins/replication b/plugins/replication
index 38a79f7..721b87a 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 38a79f76b4432679b69736dd7cea4ed254a6f09d
+Subproject commit 721b87a89e3a1bd2c368e051dec12445697680c5
diff --git a/polygerrit-ui/app/api/change-updates.ts b/polygerrit-ui/app/api/change-updates.ts
new file mode 100644
index 0000000..6ed9a8d
--- /dev/null
+++ b/polygerrit-ui/app/api/change-updates.ts
@@ -0,0 +1,35 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+export declare interface ChangeUpdatesPluginApi {
+ /**
+ * Must only be called once. You cannot register twice. You cannot unregister.
+ */
+ register(publisher: ChangeUpdatesPublisher): void;
+}
+
+/**
+ * Publisher that notifies subscribers whenever the change updates.
+ *
+ * This update could be change being submitted, new patchset being uploaded etc.
+ * Plugins are expected to use the ChangeUpdatesPluginApi to register a publisher.
+ * Gerrit will use the "subscribe" method to register a callback to this publisher.
+ * One use case is updating the ChangeModel state whenever the change is updated.
+ */
+export declare interface ChangeUpdatesPublisher {
+ /**
+ * Subcribers can use this method to add a callback that will be triggered when updates to the change happen.
+ *
+ * @param repo Repository containing the change.
+ * @param change The change number of the change for which update events are published.
+ * @param callback The callback to be called when the change is updated.
+ */
+ subscribe(repo: string, change: number, callback: () => void): void;
+ /**
+ * Remove existing callbacks. Does nothing if no subscriber was added.
+ */
+ unsubscribe(): void;
+}
diff --git a/polygerrit-ui/app/api/plugin.ts b/polygerrit-ui/app/api/plugin.ts
index 8110d71..ed85fe2 100644
--- a/polygerrit-ui/app/api/plugin.ts
+++ b/polygerrit-ui/app/api/plugin.ts
@@ -17,6 +17,7 @@
import {HookApi, RegisterOptions} from './hook';
import {StylePluginApi} from './styles';
import {SuggestionsPluginApi} from './suggestions';
+import {ChangeUpdatesPluginApi} from './change-updates';
export enum TargetElement {
CHANGE_ACTIONS = 'changeactions',
@@ -66,6 +67,7 @@
attributeHelper(element: Element): AttributeHelperPluginApi;
changeActions(): ChangeActionsPluginApi;
changeReply(): ChangeReplyPluginApi;
+ changeUpdates(): ChangeUpdatesPluginApi;
checks(): ChecksPluginApi;
suggestions(): SuggestionsPluginApi;
eventHelper(element: Node): EventHelperPluginApi;
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index 962e765..d7a132f 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -327,14 +327,13 @@
}
/**
- * The AvartarInfo entity contains information about an avatar image ofan
+ * The AvatarInfo entity contains information about an avatar image of an
* account.
* https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#avatar-info
*/
export declare interface AvatarInfo {
url: string;
height: number;
- width: number;
}
// The refs/heads/ prefix is omitted in Branch name
diff --git a/polygerrit-ui/app/constants/reporting.ts b/polygerrit-ui/app/constants/reporting.ts
index 589340f..9db6ab6 100644
--- a/polygerrit-ui/app/constants/reporting.ts
+++ b/polygerrit-ui/app/constants/reporting.ts
@@ -61,7 +61,7 @@
STARTUP_DIFF_VIEW_DISPLAYED = 'StartupDiffViewDisplayed',
// Time from startup to showing initial content of the file list.
STARTUP_FILE_LIST_DISPLAYED = 'StartupFileListDisplayed',
- // Time from startup to when the webcomponentsready event is fired. If the event is fired from the webcomponents-lite polyfill, this may be arbitrarily long after the app has started.
+ // Time from startup to when the webcomponentsready event is fired. If the event is fired from the webcomponents-bundle polyfill, this may be arbitrarily long after the app has started.
WEB_COMPONENTS_READY = 'WebComponentsReady',
// Time to received all data for change view
CHANGE_DATA = 'ChangeDataLoaded',
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 41172ca..b32a6f5 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
@@ -159,15 +159,6 @@
const TRAILING_WHITESPACE_REGEX = /[ \t]+$/gm;
const PREFIX = '#message-';
-
-const ReloadToastMessage = {
- NEWER_REVISION: 'A newer patch set has been uploaded',
- RESTORED: 'This change has been restored',
- ABANDONED: 'This change has been abandoned',
- MERGED: 'This change has been merged',
- NEW_MESSAGE: 'There are new messages on this change',
-};
-
@customElement('gr-change-view')
export class GrChangeView extends LitElement {
/**
@@ -2191,51 +2182,7 @@
this.startUpdateCheckTimer();
return;
}
- const change = this.change;
- this.getChangeModel()
- .fetchChangeUpdates(change, /* includeExtraOptions = */ true)
- .then(result => {
- let toastMessage = null;
- if (!result.isLatest) {
- toastMessage = ReloadToastMessage.NEWER_REVISION;
- } else if (result.newStatus === ChangeStatus.MERGED) {
- toastMessage = ReloadToastMessage.MERGED;
- } else if (result.newStatus === ChangeStatus.ABANDONED) {
- toastMessage = ReloadToastMessage.ABANDONED;
- } else if (result.newStatus === ChangeStatus.NEW) {
- toastMessage = ReloadToastMessage.RESTORED;
- } else if (result.newMessages) {
- toastMessage = ReloadToastMessage.NEW_MESSAGE;
- if (result.newMessages.author?.name) {
- toastMessage += ` from ${result.newMessages.author.name}`;
- }
- }
-
- // We have to make sure that the update is still relevant for the user.
- // Since starting to fetch the change update the user may have sent a
- // reply, or the change might have been reloaded, or it could be in the
- // process of being reloaded.
- const changeWasReloaded = change !== this.change;
- if (
- !toastMessage ||
- this.loading ||
- changeWasReloaded ||
- !this.isViewCurrent
- ) {
- this.startUpdateCheckTimer();
- return;
- }
-
- this.cancelUpdateCheckTimer();
- fire(this, 'show-alert', {
- message: toastMessage,
- // Persist this alert.
- dismissOnNavigation: true,
- showDismiss: true,
- action: 'Reload',
- callback: () => this.getChangeModel().navigateToChangeResetReload(),
- });
- });
+ this.getChangeModel().throttledShowUpdateChangeNotification();
}, delay * 1000);
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-change-updates-api/gr-change-updates-api.ts b/polygerrit-ui/app/elements/plugins/gr-change-updates-api/gr-change-updates-api.ts
new file mode 100644
index 0000000..ebd2f25
--- /dev/null
+++ b/polygerrit-ui/app/elements/plugins/gr-change-updates-api/gr-change-updates-api.ts
@@ -0,0 +1,42 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {PluginApi} from '../../../api/plugin';
+import {PluginsModel} from '../../../models/plugins/plugins-model';
+import {
+ ChangeUpdatesPluginApi,
+ ChangeUpdatesPublisher,
+} from '../../../api/change-updates';
+
+enum State {
+ NOT_REGISTERED,
+ REGISTERED,
+}
+
+/**
+ * Plugin API for change updates.
+ *
+ * Instance of this type created and returned to plugins that want to provide change updates data.
+ * Plugins normally just call register() once at startup.
+ */
+export class GrChangeUpdatesApi implements ChangeUpdatesPluginApi {
+ private state = State.NOT_REGISTERED;
+
+ constructor(
+ private readonly pluginsModel: PluginsModel,
+ readonly plugin: PluginApi
+ ) {}
+
+ register(publisher: ChangeUpdatesPublisher): void {
+ if (this.state === State.REGISTERED) {
+ throw new Error('Only one publisher can be registered per plugin.');
+ }
+ this.state = State.REGISTERED;
+ this.pluginsModel.changeUpdatesRegister({
+ pluginName: this.plugin.getPluginName(),
+ publisher,
+ });
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts
index 47026b4..dc162cf 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts
@@ -33,7 +33,6 @@
{
url: `https://a.b.c/photo${i}.jpg`,
height: 32,
- width: 32,
},
],
});
@@ -45,7 +44,6 @@
// Account with duplicate avatar will be skipped.
url: 'https://a.b.c/photo1.jpg',
height: 32,
- width: 32,
},
],
});
@@ -92,7 +90,6 @@
{
url: `https://a.b.c/photo${i}.jpg`,
height: 32,
- width: 32,
},
],
});
@@ -104,7 +101,6 @@
// Account with duplicate avatar will be skipped.
url: 'https://a.b.c/photo1.jpg',
height: 32,
- width: 32,
},
],
});
@@ -152,7 +148,6 @@
{
url: 'https://a.b.c/photo0.jpg',
height: 32,
- width: 32,
},
],
registered_on: '1234' as Timestamp,
@@ -217,7 +212,6 @@
{
url: `https://a.b.c/photo${i}.jpg`,
height: 32,
- width: 32,
},
],
});
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
index 2b9cacd..4a2175d 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
@@ -21,7 +21,6 @@
{
url: 'https://cdn.example.com/s12-p/photo.jpg',
height: 12,
- width: 0,
},
];
@@ -149,17 +148,14 @@
{
url: 'https://cdn.example.com/s12-p/photo.jpg',
height: 12,
- width: 0,
},
{
url: 'https://cdn.example.com/s16-p/photo.jpg',
height: 16,
- width: 0,
},
{
url: 'https://cdn.example.com/s100-p/photo.jpg',
height: 100,
- width: 0,
},
],
};
@@ -181,7 +177,6 @@
{
url: 'https://cdn.example.com/s95-p/photo.jpg',
height: 95,
- width: 0,
},
],
};
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
index b076373..35c93e67 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
@@ -36,6 +36,7 @@
import {GrPluginStyleApi} from './gr-plugin-style-api';
import {StylePluginApi} from '../../../api/styles';
import {GrSuggestionsApi} from '../../plugins/gr-suggestions-api/gr-suggestions-api';
+import {GrChangeUpdatesApi} from '../../plugins/gr-change-updates-api/gr-change-updates-api';
const PLUGIN_NAME_NOT_SET = 'NULL';
@@ -204,6 +205,10 @@
return new GrChecksApi(this.report, this.pluginsModel, this);
}
+ changeUpdates(): GrChangeUpdatesApi {
+ return new GrChangeUpdatesApi(this.pluginsModel, this);
+ }
+
suggestions(): GrSuggestionsApi {
return new GrSuggestionsApi(this.report, this.pluginsModel, this);
}
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index 113465b..df09209 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -42,7 +42,7 @@
LoadingStatus,
ParsedChangeInfo,
} from '../../types/types';
-import {fireAlert, fireTitleChange} from '../../utils/event-util';
+import {fire, fireAlert, fireTitleChange} from '../../utils/event-util';
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
import {select} from '../../utils/observable-util';
import {assertIsDefined} from '../../utils/common-util';
@@ -66,9 +66,18 @@
import {ReportingService} from '../../services/gr-reporting/gr-reporting';
import {Timing} from '../../constants/reporting';
import {GrReviewerUpdatesParser} from '../../elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser';
+import {throttleWrap} from '../../utils/async-util';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
+const ReloadToastMessage = {
+ NEWER_REVISION: 'A newer patch set has been uploaded',
+ RESTORED: 'This change has been restored',
+ ABANDONED: 'This change has been abandoned',
+ MERGED: 'This change has been merged',
+ NEW_MESSAGE: 'There are new messages on this change',
+};
+
export interface ChangeState {
/**
* If `change` is undefined, this must be either NOT_LOADED or LOADING.
@@ -449,6 +458,10 @@
public readonly revertingChangeIds$;
+ public throttledShowUpdateChangeNotification: () => void;
+
+ private isViewCurrent: boolean = false;
+
constructor(
private readonly navigation: NavigationService,
private readonly viewModel: ChangeViewModel,
@@ -511,6 +524,19 @@
this.latestRevisionWithEdit$ = this.selectRevision(
this.latestPatchNumWithEdit$
);
+ this.change$.subscribe(change => {
+ const changeUpdatesPlugins =
+ this.pluginLoader.pluginsModel.getChangeUpdatesPlugins();
+ for (const plugin of changeUpdatesPlugins) {
+ plugin.publisher.unsubscribe();
+ }
+ if (!change) return;
+ for (const plugin of changeUpdatesPlugins) {
+ plugin.publisher.subscribe(change.project, change._number, () =>
+ this.throttledShowUpdateChangeNotification()
+ );
+ }
+ });
this.isOwner$ = select(
combineLatest([this.change$, this.userModel.account$]),
([change, account]) => isOwner(change, account)
@@ -542,7 +568,14 @@
this.latestPatchNum$.subscribe(
latestPatchNum => (this.latestPatchNum = latestPatchNum)
),
+ this.viewModel.childView$.subscribe(childView => {
+ this.isViewCurrent = childView === ChangeChildView.OVERVIEW;
+ }),
];
+ this.throttledShowUpdateChangeNotification = throttleWrap(
+ () => this.showRefreshChangeNotification(),
+ 60 * 1000
+ );
}
private reportChangeReload() {
@@ -856,6 +889,49 @@
this.navigation.setUrl(url);
}
+ // private but used in tests
+ async showRefreshChangeNotification() {
+ const change = this.change;
+ if (!change) return;
+ const toastMessage = await this.getChangeUpdateToastMessage(change);
+ if (!toastMessage) return;
+ // We have to make sure that the update is still relevant for the user.
+ // Since starting to fetch the change update the user may have sent a
+ // reply, or the change might have been reloaded, or it could be in the
+ // process of being reloaded.
+ if (change !== this.change || !this.isViewCurrent) {
+ return;
+ }
+ fire(document, 'show-alert', {
+ message: toastMessage,
+ // Persist this alert.
+ dismissOnNavigation: true,
+ showDismiss: true,
+ action: 'Reload',
+ callback: () => this.navigateToChangeResetReload(),
+ });
+ }
+
+ async getChangeUpdateToastMessage(change: ParsedChangeInfo) {
+ const result = await this.fetchChangeUpdates(change);
+ let toastMessage = null;
+ if (!result.isLatest) {
+ toastMessage = ReloadToastMessage.NEWER_REVISION;
+ } else if (result.newStatus === ChangeStatus.MERGED) {
+ toastMessage = ReloadToastMessage.MERGED;
+ } else if (result.newStatus === ChangeStatus.ABANDONED) {
+ toastMessage = ReloadToastMessage.ABANDONED;
+ } else if (result.newStatus === ChangeStatus.NEW) {
+ toastMessage = ReloadToastMessage.RESTORED;
+ } else if (result.newMessages) {
+ toastMessage = ReloadToastMessage.NEW_MESSAGE;
+ if (result.newMessages.author?.name) {
+ toastMessage += ` from ${result.newMessages.author.name}`;
+ }
+ }
+ return toastMessage;
+ }
+
/**
* Check whether there are new updates on the change.
*
diff --git a/polygerrit-ui/app/models/change/change-model_test.ts b/polygerrit-ui/app/models/change/change-model_test.ts
index d3de4695..2fc8f8a 100644
--- a/polygerrit-ui/app/models/change/change-model_test.ts
+++ b/polygerrit-ui/app/models/change/change-model_test.ts
@@ -21,6 +21,7 @@
import {
mockPromise,
stubRestApi,
+ waitUntil,
waitUntilObserved,
} from '../../test/test-utils';
import {
@@ -353,6 +354,48 @@
});
});
+ test('subscribes to change updates provider and shows notification to refetch the change', async () => {
+ await waitForLoadingStatus(LoadingStatus.NOT_LOADED);
+ const pluginLoader = testResolver(pluginLoaderToken);
+ const unsubscribeStub = sinon.stub();
+ const callbacks: (() => void)[] = [];
+ let subscribeCallCount = 0;
+ const subscribeStub = (
+ _repo: string,
+ _changeNum: number,
+ callback: () => void
+ ) => {
+ subscribeCallCount++;
+ callbacks.push(callback);
+ };
+ stubRestApi('getChangeDetail').returns(
+ Promise.resolve(createParsedChange())
+ );
+ sinon.stub(pluginLoader.pluginsModel, 'getChangeUpdatesPlugins').returns([
+ {
+ pluginName: 'plugin',
+ publisher: {
+ unsubscribe: unsubscribeStub,
+ subscribe: subscribeStub,
+ },
+ },
+ ]);
+
+ changeModel.updateStateChange(createParsedChange());
+ assert.isTrue(unsubscribeStub.calledOnce);
+ assert.equal(subscribeCallCount, 1);
+
+ const showRefreshChangeNotificationStub = sinon.stub(
+ changeModel,
+ 'showRefreshChangeNotification'
+ );
+
+ for (const callback of callbacks) {
+ callback();
+ }
+ await waitUntil(() => showRefreshChangeNotificationStub.callCount === 1);
+ });
+
test('fireShowChange from overview', async () => {
await waitForLoadingStatus(LoadingStatus.NOT_LOADED);
const pluginLoader = testResolver(pluginLoaderToken);
diff --git a/polygerrit-ui/app/models/plugins/plugins-model.ts b/polygerrit-ui/app/models/plugins/plugins-model.ts
index 880bcd0..9df61bc 100644
--- a/polygerrit-ui/app/models/plugins/plugins-model.ts
+++ b/polygerrit-ui/app/models/plugins/plugins-model.ts
@@ -14,6 +14,7 @@
import {select} from '../../utils/observable-util';
import {CoverageProvider, TokenHoverListener} from '../../api/annotation';
import {SuggestionsProvider} from '../../api/suggestions';
+import {ChangeUpdatesPublisher} from '../../api/change-updates';
export interface CoveragePlugin {
pluginName: string;
@@ -26,6 +27,11 @@
config: ChecksApiConfig;
}
+export interface ChangeUpdatesPlugin {
+ pluginName: string;
+ publisher: ChangeUpdatesPublisher;
+}
+
export interface SuggestionPlugin {
pluginName: string;
provider: SuggestionsProvider;
@@ -54,6 +60,11 @@
*/
coveragePlugins: CoveragePlugin[];
/**
+ * List of plugins that have registered a publisher for change updated events.
+ */
+ changeUpdatesPlugins: ChangeUpdatesPlugin[];
+
+ /**
* List of plugins that have called checks().register().
*/
checksPlugins: ChecksPlugin[];
@@ -89,6 +100,11 @@
public coveragePlugins$ = select(this.state$, state => state.coveragePlugins);
+ public changeUpdatesPlugins$ = select(
+ this.state$,
+ state => state.changeUpdatesPlugins
+ );
+
public suggestionsPlugins$ = select(
this.state$,
state => state.suggestionsPlugins
@@ -100,6 +116,7 @@
super({
pluginsLoaded: false,
coveragePlugins: [],
+ changeUpdatesPlugins: [],
checksPlugins: [],
suggestionsPlugins: [],
tokenHighlightPlugins: [],
@@ -122,6 +139,26 @@
this.setState(nextState);
}
+ getChangeUpdatesPlugins() {
+ return this.getState().changeUpdatesPlugins;
+ }
+
+ changeUpdatesRegister(plugin: ChangeUpdatesPlugin) {
+ const nextState = {...this.getState()};
+ nextState.changeUpdatesPlugins = [...nextState.changeUpdatesPlugins];
+ const alreadyRegistered = nextState.changeUpdatesPlugins.some(
+ p => p.pluginName === plugin.pluginName
+ );
+ if (alreadyRegistered) {
+ console.warn(
+ `${plugin.pluginName} tried to register twice as a change updates provider. Ignored.`
+ );
+ return;
+ }
+ nextState.changeUpdatesPlugins.push(plugin);
+ this.setState(nextState);
+ }
+
checksRegister(plugin: ChecksPlugin) {
const nextState = {...this.getState()};
nextState.checksPlugins = [...nextState.checksPlugins];
diff --git a/polygerrit-ui/app/package.json b/polygerrit-ui/app/package.json
index 397aa9a1..b40a8b1 100644
--- a/polygerrit-ui/app/package.json
+++ b/polygerrit-ui/app/package.json
@@ -22,7 +22,7 @@
"@types/resemblejs": "^4.1.3",
"@types/resize-observer-browser": "^0.1.11",
"@webcomponents/shadycss": "^1.11.2",
- "@webcomponents/webcomponentsjs": "^1.3.3",
+ "@webcomponents/webcomponentsjs": "^2.8.0",
"highlight.js": "^11.11.1",
"highlightjs-closure-templates": "https://github.com/highlightjs/highlightjs-closure-templates#02fb0646e0499084f96a99b8c6f4a0d7bd1d33ba",
"highlightjs-epp": "https://github.com/highlightjs/highlightjs-epp#9f9e1a92f37c217c68899c7d3bdccb4d134681b9",
diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index 2c29cc7..593c898 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -111,8 +111,8 @@
"//lib/fonts:robotofonts",
"//lib/js:highlightjs__files",
"//lib/js:emojis__files",
- "@ui_npm//:node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js",
- "@ui_npm//:node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js.map",
+ "@ui_npm//@webcomponents/webcomponentsjs",
+ "@ui_npm//:node_modules/@webcomponents/webcomponentsjs/package.json",
"@ui_npm//:node_modules/resemblejs/resemble.js",
"@ui_npm//@polymer/font-roboto-local",
"@ui_npm//:node_modules/@polymer/font-roboto-local/package.json",
@@ -120,7 +120,8 @@
outs = outs,
cmd = " && ".join([
"FONT_DIR=$$(dirname $(location @ui_npm//:node_modules/@polymer/font-roboto-local/package.json))/fonts",
- "mkdir -p $$TMP/polygerrit_ui/{workers,styles/themes,fonts/{roboto,robotomono},bower_components/{emojis,highlightjs,webcomponentsjs,resemblejs},elements}",
+ "WEBCOMPONENTJS_DIR=$$(dirname $(location @ui_npm//:node_modules/@webcomponents/webcomponentsjs/package.json))",
+ "mkdir -p $$TMP/polygerrit_ui/{workers,styles/themes,fonts/{roboto,robotomono},bower_components/{emojis,highlightjs,webcomponentsjs,webcomponentsjs/bundles,resemblejs},elements}",
"for f in $(locations " + name + "_app_sources); do ext=$${f##*.}; cp -p $$f $$TMP/polygerrit_ui/elements/" + app_name + ".$$ext; done",
"cp $(locations //lib/fonts:robotofonts) $$TMP/polygerrit_ui/fonts/",
"cp $(locations //lib/fonts:material-icons) $$TMP/polygerrit_ui/fonts/",
@@ -129,8 +130,8 @@
"for f in $(locations " + name + "_worker_sources); do cp $$f $$TMP/polygerrit_ui/workers; done",
"for f in $(locations //lib/js:highlightjs__files); do cp $$f $$TMP/polygerrit_ui/bower_components/highlightjs/ ; done",
"for f in $(locations //lib/js:emojis__files); do cp $$f $$TMP/polygerrit_ui/bower_components/emojis/ ; done",
- "cp $(location @ui_npm//:node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js) $$TMP/polygerrit_ui/bower_components/webcomponentsjs/webcomponents-lite.js",
- "cp $(location @ui_npm//:node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js.map) $$TMP/polygerrit_ui/bower_components/webcomponentsjs/webcomponents-lite.js.map",
+ "cp $$WEBCOMPONENTJS_DIR/webcomponents-loader.js $$TMP/polygerrit_ui/bower_components/webcomponentsjs/webcomponents-loader.js",
+ "cp $$WEBCOMPONENTJS_DIR/bundles/* $$TMP/polygerrit_ui/bower_components/webcomponentsjs/bundles/",
"cp $(location @ui_npm//:node_modules/resemblejs/resemble.js) $$TMP/polygerrit_ui/bower_components/resemblejs/resemble.js",
"cp $$FONT_DIR/roboto/*.ttf $$TMP/polygerrit_ui/fonts/roboto/",
"cp $$FONT_DIR/robotomono/*.ttf $$TMP/polygerrit_ui/fonts/robotomono/",
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 32a6867..599ecc1 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -229,19 +229,15 @@
if (existingPromise) existingPromise.delegate(promise);
return promise;
}
-const THROTTLE_INTERVAL_MS = 500;
/**
* Ensure only one call is made within THROTTLE_INTERVAL_MS and any call within
* this interval is ignored
*/
-export function throttleWrap<T>(fn: (e: T) => void) {
+export function throttleWrap<T>(fn: (e?: T) => void, throttleInterval = 500) {
let lastCall: number | undefined;
- return (e: T) => {
- if (
- lastCall !== undefined &&
- Date.now() - lastCall < THROTTLE_INTERVAL_MS
- ) {
+ return (e?: T) => {
+ if (lastCall !== undefined && Date.now() - lastCall < throttleInterval) {
return;
}
lastCall = Date.now();
diff --git a/polygerrit-ui/app/yarn.lock b/polygerrit-ui/app/yarn.lock
index 418e873..2dab265 100644
--- a/polygerrit-ui/app/yarn.lock
+++ b/polygerrit-ui/app/yarn.lock
@@ -212,8 +212,6 @@
"@polymer/paper-behaviors" "^3.0.0-pre.27"
"@polymer/paper-styles" "^3.0.0-pre.26"
"@polymer/polymer" "^3.0.0"
- "@polymer/paper-styles" "^3.0.0-pre.26"
- "@polymer/polymer" "^3.0.0"
"@polymer/paper-input@^3.2.1":
version "3.2.1"
@@ -300,12 +298,7 @@
resolved "https://registry.yarnpkg.com/@webcomponents/shadycss/-/shadycss-1.11.2.tgz#7539b0ad29598aa2eafee8b341059e20ac9e1006"
integrity sha512-vRq+GniJAYSBmTRnhCYPAPq6THYqovJ/gzGThWbgEZUQaBccndGTi1hdiUP15HzEco0I6t4RCtXyX0rsSmwgPw==
-"@webcomponents/webcomponentsjs@^1.3.3":
- version "1.3.3"
- resolved "https://registry.yarnpkg.com/@webcomponents/webcomponentsjs/-/webcomponentsjs-1.3.3.tgz#5bb82a0d3210c836bd4623e13a4a93145cb9dc27"
- integrity sha512-eLH04VBMpuZGzBIhOnUjECcQPEPcmfhWEijW9u1B5I+2PPYdWf3vWUExdDxu4Y3GljRSTCOlWnGtS9tpzmXMyQ==
-
-"@webcomponents/webcomponentsjs@^2.0.3":
+"@webcomponents/webcomponentsjs@^2.0.3", "@webcomponents/webcomponentsjs@^2.8.0":
version "2.8.0"
resolved "https://registry.yarnpkg.com/@webcomponents/webcomponentsjs/-/webcomponentsjs-2.8.0.tgz#ab21f027594fa827c1889e8b646da7be27c7908a"
integrity sha512-loGD63sacRzOzSJgQnB9ZAhaQGkN7wl2Zuw7tsphI5Isa0irijrRo6EnJii/GgjGefIFO8AIO7UivzRhFaEk9w==
diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
index 86970f8..b9b60c2 100644
--- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
@@ -141,9 +141,9 @@
{/if}
<link rel="preload" as="style" href="{$staticResourcePath}/styles/main.css">{\n}
- <script src="{$staticResourcePath}/bower_components/webcomponentsjs/webcomponents-lite.js"></script>{\n}
+ <script src="{$staticResourcePath}/bower_components/webcomponentsjs/webcomponents-loader.js"></script>{\n}
- // Content between webcomponents-lite and the load of the main app element
+ // Content between webcomponents-loader and the load of the main app element
// run before polymer-resin is installed so may have security consequences.
// Contact your local security engineer if you have any questions, and
// CC them on any changes that load content before gr-app.js.
diff --git a/web-dev-server.config.mjs b/web-dev-server.config.mjs
index 53a240e..8820ecd 100644
--- a/web-dev-server.config.mjs
+++ b/web-dev-server.config.mjs
@@ -25,9 +25,14 @@
// polygerrit-ui/app/rules.bzl
async (context, next) => {
- if ( context.url.includes("/bower_components/webcomponentsjs/webcomponents-lite.js") ) {
- context.response.redirect("/node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js");
-
+ if ( context.url.includes("/bower_components/webcomponentsjs/webcomponents-loader.js") ) {
+ context.response.redirect("/node_modules/@webcomponents/webcomponentsjs/webcomponents-loader.js");
+ } else if (context.url.includes("/bower_components/webcomponentsjs/bundles/")) {
+ const bundlePath = context.url.replace(
+ "/bower_components/webcomponentsjs/bundles/",
+ "/node_modules/@webcomponents/webcomponentsjs/bundles/"
+ );
+ context.response.redirect(bundlePath);
} else if ( context.url.startsWith( "/fonts/" ) ) {
const fontFile = path.join( "lib/fonts", path.basename(context.url) );
context.body = fs.createReadStream( fontFile );
