Merge "Document AbstractDaemonTest.commonServer"
diff --git a/java/com/google/gerrit/extensions/validators/CommentValidationContext.java b/java/com/google/gerrit/extensions/validators/CommentValidationContext.java
new file mode 100644
index 0000000..1cb00e3
--- /dev/null
+++ b/java/com/google/gerrit/extensions/validators/CommentValidationContext.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.validators;
+
+import com.google.auto.value.AutoValue;
+
+/**
+ * Holds a comment validators context in order to pass it to a validation plugin.
+ *
+ * <p>This is used to provided additional context around that comment that can be used by the
+ * validator to determine what validations should be run. For example, a comment validator may only
+ * want to validate a comment if it's on a change in the project foo.
+ *
+ * @see CommentValidator
+ */
+@AutoValue
+public abstract class CommentValidationContext {
+
+ /** Returns the change id the comment is being added to. */
+ public abstract int getChangeId();
+
+ /** Returns the project the comment is being added to. */
+ public abstract String getProject();
+
+ public static Builder builder() {
+ return new AutoValue_CommentValidationContext.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder changeId(int value);
+
+ public abstract Builder project(String value);
+
+ public abstract CommentValidationContext build();
+ }
+}
diff --git a/java/com/google/gerrit/extensions/validators/CommentValidator.java b/java/com/google/gerrit/extensions/validators/CommentValidator.java
index cfefdef..ba73e46 100644
--- a/java/com/google/gerrit/extensions/validators/CommentValidator.java
+++ b/java/com/google/gerrit/extensions/validators/CommentValidator.java
@@ -30,5 +30,5 @@
* @return An empty list if all comments are valid, or else a list of validation failures.
*/
ImmutableList<CommentValidationFailure> validateComments(
- ImmutableList<CommentForValidation> comments);
+ CommentValidationContext ctx, ImmutableList<CommentForValidation> comments);
}
diff --git a/java/com/google/gerrit/git/RefUpdateUtil.java b/java/com/google/gerrit/git/RefUpdateUtil.java
index fa7b98f..bd88962 100644
--- a/java/com/google/gerrit/git/RefUpdateUtil.java
+++ b/java/com/google/gerrit/git/RefUpdateUtil.java
@@ -150,6 +150,7 @@
public static void deleteChecked(Repository repo, String refName) throws IOException {
RefUpdate ru = repo.updateRef(refName);
ru.setForceUpdate(true);
+ ru.setCheckConflicting(false);
switch (ru.delete()) {
case FORCED:
// Ref was deleted.
diff --git a/java/com/google/gerrit/pgm/init/InitModule.java b/java/com/google/gerrit/pgm/init/InitModule.java
index f2fc001..b658675 100644
--- a/java/com/google/gerrit/pgm/init/InitModule.java
+++ b/java/com/google/gerrit/pgm/init/InitModule.java
@@ -34,8 +34,6 @@
@Override
protected void configure() {
bind(SitePaths.class);
- bind(Libraries.class);
- bind(LibraryDownloader.class);
factory(Section.Factory.class);
factory(VersionedAuthorizedKeysOnInit.Factory.class);
diff --git a/java/com/google/gerrit/pgm/init/Libraries.java b/java/com/google/gerrit/pgm/init/Libraries.java
deleted file mode 100644
index c599e99..0000000
--- a/java/com/google/gerrit/pgm/init/Libraries.java
+++ /dev/null
@@ -1,141 +0,0 @@
-// Copyright (C) 2009 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.pgm.init;
-
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.gerrit.pgm.init.api.LibraryDownload;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.io.Reader;
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
-import java.util.List;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
-
-/** Standard {@link LibraryDownloader} instances derived from configuration. */
-@Singleton
-class Libraries {
- private static final String RESOURCE_FILE = "com/google/gerrit/pgm/init/libraries.config";
-
- private final Provider<LibraryDownloader> downloadProvider;
- private final List<String> skippedDownloads;
- private final boolean skipAllDownloads;
-
- /* final */ LibraryDownloader db2Driver;
- /* final */ LibraryDownloader db2DriverLicense;
- /* final */ LibraryDownloader hanaDriver;
- /* final */ LibraryDownloader mariadbDriver;
- /* final */ LibraryDownloader mysqlDriver;
- /* final */ LibraryDownloader oracleDriver;
-
- @Inject
- Libraries(
- final Provider<LibraryDownloader> downloadProvider,
- @LibraryDownload List<String> skippedDownloads,
- @LibraryDownload Boolean skipAllDownloads) {
- this.downloadProvider = downloadProvider;
- this.skippedDownloads = skippedDownloads;
- this.skipAllDownloads = skipAllDownloads;
- init();
- }
-
- private void init() {
- final Config cfg = new Config();
- try {
- cfg.fromText(read(RESOURCE_FILE));
- } catch (IOException | ConfigInvalidException e) {
- throw new RuntimeException(e.getMessage(), e);
- }
-
- for (Field f : Libraries.class.getDeclaredFields()) {
- if ((f.getModifiers() & Modifier.STATIC) == 0 && f.getType() == LibraryDownloader.class) {
- try {
- f.set(this, downloadProvider.get());
- } catch (IllegalArgumentException | IllegalAccessException e) {
- throw new IllegalStateException("Cannot initialize " + f.getName());
- }
- }
- }
-
- for (Field f : Libraries.class.getDeclaredFields()) {
- if ((f.getModifiers() & Modifier.STATIC) == 0 && f.getType() == LibraryDownloader.class) {
- try {
- init(f, cfg);
- } catch (IllegalArgumentException
- | IllegalAccessException
- | NoSuchFieldException
- | SecurityException e) {
- throw new IllegalStateException("Cannot configure " + f.getName());
- }
- }
- }
- }
-
- private void init(Field field, Config cfg)
- throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException,
- SecurityException {
- String n = field.getName();
- LibraryDownloader dl = (LibraryDownloader) field.get(this);
- dl.setName(get(cfg, n, "name"));
- dl.setJarUrl(get(cfg, n, "url"));
- dl.setSHA1(getOptional(cfg, n, "sha1"));
- dl.setRemove(get(cfg, n, "remove"));
- for (String d : cfg.getStringList("library", n, "needs")) {
- dl.addNeeds((LibraryDownloader) getClass().getDeclaredField(d).get(this));
- }
- dl.setSkipDownload(skipAllDownloads || skippedDownloads.contains(n));
- }
-
- private static String getOptional(Config cfg, String name, String key) {
- return doGet(cfg, name, key, false);
- }
-
- private static String get(Config cfg, String name, String key) {
- return doGet(cfg, name, key, true);
- }
-
- private static String doGet(Config cfg, String name, String key, boolean required) {
- String val = cfg.getString("library", name, key);
- if ((val == null || val.isEmpty()) && required) {
- throw new IllegalStateException(
- "Variable library." + name + "." + key + " is required within " + RESOURCE_FILE);
- }
- return val;
- }
-
- private static String read(String p) throws IOException {
- try (InputStream in = Libraries.class.getClassLoader().getResourceAsStream(p)) {
- if (in == null) {
- throw new FileNotFoundException("Cannot load resource " + p);
- }
- try (Reader r = new InputStreamReader(in, UTF_8)) {
- final StringBuilder buf = new StringBuilder();
- final char[] tmp = new char[512];
- int n;
- while (0 < (n = r.read(tmp))) {
- buf.append(tmp, 0, n);
- }
- return buf.toString();
- }
- }
- }
-}
diff --git a/java/com/google/gerrit/pgm/init/LibraryDownloader.java b/java/com/google/gerrit/pgm/init/LibraryDownloader.java
deleted file mode 100644
index 0b31ee2..0000000
--- a/java/com/google/gerrit/pgm/init/LibraryDownloader.java
+++ /dev/null
@@ -1,316 +0,0 @@
-// Copyright (C) 2009 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.pgm.init;
-
-import com.google.common.hash.Funnels;
-import com.google.common.hash.Hasher;
-import com.google.common.hash.Hashing;
-import com.google.common.io.ByteStreams;
-import com.google.gerrit.common.Die;
-import com.google.gerrit.common.IoUtil;
-import com.google.gerrit.pgm.init.api.ConsoleUI;
-import com.google.gerrit.server.config.SitePaths;
-import com.google.inject.Inject;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.net.HttpURLConnection;
-import java.net.Proxy;
-import java.net.ProxySelector;
-import java.net.URISyntaxException;
-import java.net.URL;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.List;
-import org.eclipse.jgit.util.HttpSupport;
-
-/** Get optional or required 3rd party library files into $site_path/lib. */
-class LibraryDownloader {
- private final ConsoleUI ui;
- private final Path lib_dir;
- private final StaleLibraryRemover remover;
-
- private boolean required;
- private String name;
- private String jarUrl;
- private String sha1;
- private String remove;
- private List<LibraryDownloader> needs;
- private LibraryDownloader neededBy;
- private Path dst;
- private boolean download; // download or copy
- private boolean exists;
- private boolean skipDownload;
-
- @Inject
- LibraryDownloader(ConsoleUI ui, SitePaths site, StaleLibraryRemover remover) {
- this.ui = ui;
- this.lib_dir = site.lib_dir;
- this.remover = remover;
- this.needs = new ArrayList<>(2);
- }
-
- void setName(String name) {
- this.name = name;
- }
-
- void setJarUrl(String url) {
- this.jarUrl = url;
- download = jarUrl.startsWith("http");
- }
-
- void setSHA1(String sha1) {
- this.sha1 = sha1;
- }
-
- void setRemove(String remove) {
- this.remove = remove;
- }
-
- void addNeeds(LibraryDownloader lib) {
- needs.add(lib);
- }
-
- void setSkipDownload(boolean skipDownload) {
- this.skipDownload = skipDownload;
- }
-
- void downloadRequired() {
- setRequired(true);
- download();
- }
-
- void downloadOptional() {
- required = false;
- download();
- }
-
- private void setRequired(boolean r) {
- required = r;
- for (LibraryDownloader d : needs) {
- d.setRequired(r);
- }
- }
-
- private void download() {
- if (skipDownload) {
- return;
- }
-
- if (jarUrl == null || !jarUrl.contains("/")) {
- throw new IllegalStateException("Invalid JarUrl for " + name);
- }
-
- final String jarName = jarUrl.substring(jarUrl.lastIndexOf('/') + 1);
- if (jarName.contains("/") || jarName.contains("\\")) {
- throw new IllegalStateException("Invalid JarUrl: " + jarUrl);
- }
-
- if (name == null) {
- name = jarName;
- }
-
- dst = lib_dir.resolve(jarName);
- if (Files.exists(dst)) {
- exists = true;
- } else if (shouldGet()) {
- doGet();
- }
-
- if (exists) {
- for (LibraryDownloader d : needs) {
- d.neededBy = this;
- d.downloadRequired();
- }
- }
- }
-
- private boolean shouldGet() {
- if (ui.isBatch()) {
- return required;
- }
- final StringBuilder msg = new StringBuilder();
- msg.append("\n");
- msg.append("Gerrit Code Review is not shipped with %s\n");
- if (neededBy != null) {
- msg.append(String.format("** This library is required by %s. **\n", neededBy.name));
- } else if (required) {
- msg.append("** This library is required for your configuration. **\n");
- } else {
- msg.append(" If available, Gerrit can take advantage of features\n");
- msg.append(" in the library, but will also function without it.\n");
- }
- msg.append(String.format("%s and install it now", download ? "Download" : "Copy"));
- return ui.yesno(true, msg.toString(), name);
- }
-
- private void doGet() {
- if (!Files.exists(lib_dir)) {
- try {
- Files.createDirectories(lib_dir);
- } catch (IOException e) {
- throw new Die("Cannot create " + lib_dir, e);
- }
- }
-
- try {
- remover.remove(remove);
- if (download) {
- doGetByHttp();
- } else {
- doGetByLocalCopy();
- }
- verifyFileChecksum();
- } catch (IOException err) {
- try {
- Files.delete(dst);
- } catch (IOException e) {
- // Delete failed; leave alone.
- }
-
- if (ui.isBatch()) {
- throw new Die("error: Cannot get " + jarUrl, err);
- }
-
- System.err.println();
- System.err.println();
- System.err.println("error: " + err.getMessage());
- System.err.println("Please download:");
- System.err.println();
- System.err.println(" " + jarUrl);
- System.err.println();
- System.err.println("and save as:");
- System.err.println();
- System.err.println(" " + dst.toAbsolutePath());
- System.err.println();
- System.err.flush();
-
- ui.waitForUser();
-
- if (Files.exists(dst)) {
- verifyFileChecksum();
-
- } else if (!ui.yesno(!required, "Continue without this library")) {
- throw new Die("aborted by user");
- }
- }
-
- if (Files.exists(dst)) {
- exists = true;
- IoUtil.loadJARs(dst);
- }
- }
-
- private void doGetByLocalCopy() throws IOException {
- System.err.print("Copying " + jarUrl + " ...");
- Path p = url2file(jarUrl);
- if (!Files.exists(p)) {
- StringBuilder msg =
- new StringBuilder()
- .append("\n")
- .append("Can not find the %s at this location: %s\n")
- .append("Please provide alternative URL");
- p = url2file(ui.readString(null, msg.toString(), name, jarUrl));
- }
- Files.copy(p, dst);
- }
-
- private static Path url2file(String urlString) throws IOException {
- final URL url = new URL(urlString);
- try {
- return Paths.get(url.toURI());
- } catch (URISyntaxException e) {
- return Paths.get(url.getPath());
- }
- }
-
- private void doGetByHttp() throws IOException {
- System.err.print("Downloading " + jarUrl + " ...");
- System.err.flush();
- try (InputStream in = openHttpStream(jarUrl);
- OutputStream out = Files.newOutputStream(dst)) {
- ByteStreams.copy(in, out);
- System.err.println(" OK");
- System.err.flush();
- } catch (IOException err) {
- deleteDst();
- System.err.println(" !! FAIL !!");
- System.err.println(err);
- System.err.flush();
- throw err;
- }
- }
-
- private static InputStream openHttpStream(String urlStr) throws IOException {
- ProxySelector proxySelector = ProxySelector.getDefault();
- URL url = new URL(urlStr);
- Proxy proxy = HttpSupport.proxyFor(proxySelector, url);
- HttpURLConnection c = (HttpURLConnection) url.openConnection(proxy);
-
- switch (HttpSupport.response(c)) {
- case HttpURLConnection.HTTP_OK:
- return c.getInputStream();
-
- case HttpURLConnection.HTTP_NOT_FOUND:
- throw new FileNotFoundException(url.toString());
-
- default:
- throw new IOException(
- url.toString() + ": " + HttpSupport.response(c) + " " + c.getResponseMessage());
- }
- }
-
- @SuppressWarnings("deprecation") // Use Hashing.sha1 for compatibility.
- private void verifyFileChecksum() {
- if (sha1 == null) {
- System.err.println();
- System.err.flush();
- return;
- }
- Hasher h = Hashing.sha1().newHasher();
- try (InputStream in = Files.newInputStream(dst);
- OutputStream out = Funnels.asOutputStream(h)) {
- ByteStreams.copy(in, out);
- } catch (IOException e) {
- deleteDst();
- throw new Die("cannot checksum " + dst, e);
- }
- if (sha1.equals(h.hash().toString())) {
- System.err.println("Checksum " + dst.getFileName() + " OK");
- System.err.flush();
- } else if (ui.isBatch()) {
- deleteDst();
- throw new Die(dst + " SHA-1 checksum does not match");
-
- } else if (!ui.yesno(
- null /* force an answer */,
- "error: SHA-1 checksum does not match\nUse %s anyway", //
- dst.getFileName())) {
- deleteDst();
- throw new Die("aborted by user");
- }
- }
-
- private void deleteDst() {
- try {
- Files.delete(dst);
- } catch (IOException e) {
- System.err.println(" Failed to clean up lib: " + dst);
- }
- }
-}
diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java
index 09042ab..3d34d6b 100644
--- a/java/com/google/gerrit/server/PublishCommentUtil.java
+++ b/java/com/google/gerrit/server/PublishCommentUtil.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -118,16 +119,18 @@
/**
* Helper to run the specified set of {@link CommentValidator}-s on the specified comments.
*
- * @return See {@link CommentValidator#validateComments(ImmutableList)}.
+ * @return See {@link CommentValidator#validateComments(CommentValidationContext,ImmutableList)}.
*/
public static ImmutableList<CommentValidationFailure> findInvalidComments(
+ CommentValidationContext ctx,
PluginSetContext<CommentValidator> commentValidators,
ImmutableList<CommentForValidation> commentsForValidation) {
ImmutableList.Builder<CommentValidationFailure> commentValidationFailures =
new ImmutableList.Builder<>();
commentValidators.runEach(
validator ->
- commentValidationFailures.addAll(validator.validateComments(commentsForValidation)));
+ commentValidationFailures.addAll(
+ validator.validateComments(ctx, commentsForValidation)));
return commentValidationFailures.build();
}
}
diff --git a/java/com/google/gerrit/server/change/FileInfoJson.java b/java/com/google/gerrit/server/change/FileInfoJson.java
index a823975..aca4fb0 100644
--- a/java/com/google/gerrit/server/change/FileInfoJson.java
+++ b/java/com/google/gerrit/server/change/FileInfoJson.java
@@ -21,6 +21,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListEntry;
@@ -30,6 +31,8 @@
import com.google.inject.Singleton;
import java.util.Map;
import java.util.TreeMap;
+import java.util.concurrent.ExecutionException;
+import org.eclipse.jgit.errors.NoMergeBaseException;
import org.eclipse.jgit.lib.ObjectId;
@Singleton
@@ -42,31 +45,44 @@
}
public Map<String, FileInfo> toFileInfoMap(Change change, PatchSet patchSet)
- throws PatchListNotAvailableException {
+ throws ResourceConflictException, PatchListNotAvailableException {
return toFileInfoMap(change, patchSet.commitId(), null);
}
public Map<String, FileInfo> toFileInfoMap(
Change change, ObjectId objectId, @Nullable PatchSet base)
- throws PatchListNotAvailableException {
+ throws ResourceConflictException, PatchListNotAvailableException {
ObjectId a = base != null ? base.commitId() : null;
return toFileInfoMap(change, PatchListKey.againstCommit(a, objectId, Whitespace.IGNORE_NONE));
}
public Map<String, FileInfo> toFileInfoMap(Change change, ObjectId objectId, int parent)
- throws PatchListNotAvailableException {
+ throws ResourceConflictException, PatchListNotAvailableException {
return toFileInfoMap(
change, PatchListKey.againstParentNum(parent + 1, objectId, Whitespace.IGNORE_NONE));
}
private Map<String, FileInfo> toFileInfoMap(Change change, PatchListKey key)
- throws PatchListNotAvailableException {
+ throws ResourceConflictException, PatchListNotAvailableException {
return toFileInfoMap(change.getProject(), key);
}
public Map<String, FileInfo> toFileInfoMap(Project.NameKey project, PatchListKey key)
- throws PatchListNotAvailableException {
- PatchList list = patchListCache.get(key, project);
+ throws ResourceConflictException, PatchListNotAvailableException {
+ PatchList list;
+ try {
+ list = patchListCache.get(key, project);
+ } catch (PatchListNotAvailableException e) {
+ Throwable cause = e.getCause();
+ if (cause instanceof ExecutionException) {
+ cause = cause.getCause();
+ }
+ if (cause instanceof NoMergeBaseException) {
+ throw new ResourceConflictException(
+ String.format("Cannot create auto merge commit: %s", e.getMessage()), e);
+ }
+ throw e;
+ }
Map<String, FileInfo> files = new TreeMap<>();
for (PatchListEntry e : list.getPatches()) {
diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
index fbd14c4..a4994fd 100644
--- a/java/com/google/gerrit/server/change/RevisionJson.java
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -49,6 +49,7 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GpgException;
@@ -311,9 +312,13 @@
}
if (has(ALL_FILES) || (out.isCurrent && has(CURRENT_FILES))) {
- out.files = fileInfoJson.toFileInfoMap(c, in);
- out.files.remove(Patch.COMMIT_MSG);
- out.files.remove(Patch.MERGE_LIST);
+ try {
+ out.files = fileInfoJson.toFileInfoMap(c, in);
+ out.files.remove(Patch.COMMIT_MSG);
+ out.files.remove(Patch.MERGE_LIST);
+ } catch (ResourceConflictException e) {
+ logger.atWarning().withCause(e).log("creating file list failed");
+ }
}
if (out.isCurrent && has(CURRENT_ACTIONS) && userProvider.get().isIdentifiedUser()) {
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index c05a47d..128388d 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -53,6 +53,7 @@
import java.util.List;
import java.util.Optional;
import java.util.TimeZone;
+import org.eclipse.jgit.dircache.InvalidPathException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -250,13 +251,14 @@
* @param filePath the path of the file whose contents should be modified
* @param newContent the new file content
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
+ * @throws BadRequestException if the user provided bad input (e.g. invalid file paths)
* @throws InvalidChangeOperationException if the file already had the specified content
* @throws PermissionBackendException
* @throws ResourceConflictException if the project state does not permit the operation
*/
public void modifyFile(
Repository repository, ChangeNotes notes, String filePath, RawInput newContent)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new ChangeFileContentModification(filePath, newContent));
}
@@ -269,12 +271,13 @@
* @param notes the {@link ChangeNotes} of the change whose change edit should be modified
* @param file path of the file which should be deleted
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
+ * @throws BadRequestException if the user provided bad input (e.g. invalid file paths)
* @throws InvalidChangeOperationException if the file does not exist
* @throws PermissionBackendException
* @throws ResourceConflictException if the project state does not permit the operation
*/
public void deleteFile(Repository repository, ChangeNotes notes, String file)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new DeleteFileModification(file));
}
@@ -288,6 +291,7 @@
* @param currentFilePath the current path/name of the file
* @param newFilePath the desired path/name of the file
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
+ * @throws BadRequestException if the user provided bad input (e.g. invalid file paths)
* @throws InvalidChangeOperationException if the file was already renamed to the specified new
* name
* @throws PermissionBackendException
@@ -295,7 +299,7 @@
*/
public void renameFile(
Repository repository, ChangeNotes notes, String currentFilePath, String newFilePath)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new RenameFileModification(currentFilePath, newFilePath));
}
@@ -313,14 +317,14 @@
* @throws PermissionBackendException
*/
public void restoreFile(Repository repository, ChangeNotes notes, String file)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new RestoreFileModification(file));
}
private void modifyTree(
Repository repository, ChangeNotes notes, TreeModification treeModification)
- throws AuthException, IOException, InvalidChangeOperationException,
+ throws AuthException, BadRequestException, IOException, InvalidChangeOperationException,
PermissionBackendException, ResourceConflictException {
assertCanEdit(notes);
@@ -370,8 +374,8 @@
ChangeNotes notes,
PatchSet patchSet,
List<TreeModification> treeModifications)
- throws AuthException, IOException, InvalidChangeOperationException, MergeConflictException,
- PermissionBackendException, ResourceConflictException {
+ throws AuthException, BadRequestException, IOException, InvalidChangeOperationException,
+ MergeConflictException, PermissionBackendException, ResourceConflictException {
assertCanEdit(notes);
Optional<ChangeEdit> optionalChangeEdit = lookupChangeEdit(notes);
@@ -486,10 +490,15 @@
private static ObjectId createNewTree(
Repository repository, RevCommit baseCommit, List<TreeModification> treeModifications)
- throws IOException, InvalidChangeOperationException {
- TreeCreator treeCreator = new TreeCreator(baseCommit);
- treeCreator.addTreeModifications(treeModifications);
- ObjectId newTreeId = treeCreator.createNewTreeAndGetId(repository);
+ throws BadRequestException, IOException, InvalidChangeOperationException {
+ ObjectId newTreeId;
+ try {
+ TreeCreator treeCreator = new TreeCreator(baseCommit);
+ treeCreator.addTreeModifications(treeModifications);
+ newTreeId = treeCreator.createNewTreeAndGetId(repository);
+ } catch (InvalidPathException e) {
+ throw new BadRequestException(e.getMessage());
+ }
if (ObjectId.isEqual(newTreeId, baseCommit.getTree())) {
throw new InvalidChangeOperationException("no changes were made");
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 3d531b2..d8aa054 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -93,6 +93,7 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.validators.CommentForValidation;
import com.google.gerrit.extensions.validators.CommentForValidation.CommentType;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.ApprovalsUtil;
@@ -2014,8 +2015,13 @@
: CommentType.FILE_COMMENT,
comment.message))
.collect(toImmutableList());
+ CommentValidationContext ctx =
+ CommentValidationContext.builder()
+ .changeId(change.getChangeId())
+ .project(change.getProject().get())
+ .build();
ImmutableList<CommentValidationFailure> commentValidationFailures =
- PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
+ PublishCommentUtil.findInvalidComments(ctx, commentValidators, draftsForValidation);
magicBranch.setCommentsValid(commentValidationFailures.isEmpty());
commentValidationFailures.forEach(
failure ->
@@ -3337,7 +3343,8 @@
}
logger.atFine().log(
- "Auto-closing %d changes with existing patch sets and %d with new patch sets",
+ "Auto-closing %d changes with existing patch sets and %d with new patch"
+ + " sets",
existingPatchSets, newPatchSets);
bu.execute();
} catch (IOException | StorageException | PermissionBackendException e) {
diff --git a/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java b/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
index 8237e69..3a8bcac 100644
--- a/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
+++ b/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -33,7 +34,7 @@
@Override
public ImmutableList<CommentValidationFailure> validateComments(
- ImmutableList<CommentForValidation> comments) {
+ CommentValidationContext ctx, ImmutableList<CommentForValidation> comments) {
return comments.stream()
.filter(c -> c.getText().length() > maxCommentLength)
.map(
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 71d8c15..e79696a 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -34,6 +34,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.mail.HtmlParser;
@@ -287,8 +288,14 @@
MAIL_COMMENT_TYPE_TO_VALIDATION_TYPE.get(comment.getType()),
comment.getMessage()))
.collect(ImmutableList.toImmutableList());
+ CommentValidationContext commentValidationCtx =
+ CommentValidationContext.builder()
+ .changeId(cd.change().getChangeId())
+ .project(cd.change().getProject().get())
+ .build();
ImmutableList<CommentValidationFailure> commentValidationFailures =
- PublishCommentUtil.findInvalidComments(commentValidators, parsedCommentsForValidation);
+ PublishCommentUtil.findInvalidComments(
+ commentValidationCtx, commentValidators, parsedCommentsForValidation);
if (!commentValidationFailures.isEmpty()) {
sendRejectionEmail(message, InboundEmailRejectionSender.Error.COMMENT_REJECTED);
return;
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index aa36b73..cbc1b79 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -138,7 +138,8 @@
@Override
public Response<?> apply(ChangeResource rsrc, IdString id, Input in)
- throws IOException, AuthException, ResourceConflictException, PermissionBackendException {
+ throws IOException, AuthException, BadRequestException, ResourceConflictException,
+ PermissionBackendException {
return deleteContent.apply(rsrc, id.get());
}
}
@@ -184,7 +185,8 @@
@Override
public Response<EditInfo> apply(ChangeResource rsrc)
- throws AuthException, IOException, ResourceNotFoundException, PermissionBackendException {
+ throws AuthException, IOException, ResourceNotFoundException, ResourceConflictException,
+ PermissionBackendException {
Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getNotes(), rsrc.getUser());
if (!edit.isPresent()) {
return Response.none();
@@ -239,7 +241,8 @@
@Override
public Response<?> apply(ChangeResource resource, Post.Input input)
- throws AuthException, IOException, ResourceConflictException, PermissionBackendException {
+ throws AuthException, BadRequestException, IOException, ResourceConflictException,
+ PermissionBackendException {
Project.NameKey project = resource.getProject();
try (Repository repository = repositoryManager.openRepository(project)) {
if (isRestoreFile(input)) {
@@ -325,12 +328,14 @@
@Override
public Response<?> apply(ChangeEditResource rsrc, Input input)
- throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
+ throws AuthException, BadRequestException, ResourceConflictException, IOException,
+ PermissionBackendException {
return apply(rsrc.getChangeResource(), rsrc.getPath());
}
public Response<?> apply(ChangeResource rsrc, String filePath)
- throws AuthException, IOException, ResourceConflictException, PermissionBackendException {
+ throws AuthException, BadRequestException, IOException, ResourceConflictException,
+ PermissionBackendException {
try (Repository repository = repositoryManager.openRepository(rsrc.getProject())) {
editModifier.deleteFile(repository, rsrc.getNotes(), filePath);
} catch (InvalidChangeOperationException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 03c2fc4..324069d 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -77,6 +77,7 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.json.OutputFormat;
@@ -999,16 +1000,22 @@
}
}
+ CommentValidationContext commentValidationCtx =
+ CommentValidationContext.builder()
+ .changeId(ctx.getChange().getChangeId())
+ .project(ctx.getChange().getProject().get())
+ .build();
switch (in.drafts) {
case PUBLISH:
case PUBLISH_ALL_REVISIONS:
- validateComments(Streams.concat(drafts.values().stream(), toPublish.stream()));
+ validateComments(
+ commentValidationCtx, Streams.concat(drafts.values().stream(), toPublish.stream()));
publishCommentUtil.publish(ctx, ctx.getUpdate(psId), drafts.values(), in.tag);
comments.addAll(drafts.values());
break;
case KEEP:
default:
- validateComments(toPublish.stream());
+ validateComments(commentValidationCtx, toPublish.stream());
break;
}
ChangeUpdate changeUpdate = ctx.getUpdate(psId);
@@ -1017,7 +1024,8 @@
return !toPublish.isEmpty();
}
- private void validateComments(Stream<Comment> comments) throws CommentsRejectedException {
+ private void validateComments(CommentValidationContext ctx, Stream<Comment> comments)
+ throws CommentsRejectedException {
ImmutableList<CommentForValidation> draftsForValidation =
comments
.map(
@@ -1029,7 +1037,7 @@
comment.message))
.collect(toImmutableList());
ImmutableList<CommentValidationFailure> draftValidationFailures =
- PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
+ PublishCommentUtil.findInvalidComments(ctx, commentValidators, draftsForValidation);
if (!draftValidationFailures.isEmpty()) {
throw new CommentsRejectedException(draftValidationFailures);
}
@@ -1415,8 +1423,14 @@
buf.append(String.format("\n\n(%d comments)", comments.size()));
}
if (!msg.isEmpty()) {
+ CommentValidationContext commentValidationCtx =
+ CommentValidationContext.builder()
+ .changeId(ctx.getChange().getChangeId())
+ .project(ctx.getChange().getProject().get())
+ .build();
ImmutableList<CommentValidationFailure> messageValidationFailure =
PublishCommentUtil.findInvalidComments(
+ commentValidationCtx,
commentValidators,
ImmutableList.of(
CommentForValidation.create(
diff --git a/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java b/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java
index 0ee8279..0d5ab88 100644
--- a/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -86,7 +87,7 @@
@Override
public Response<Map<String, FileInfo>> apply(CommitResource resource)
- throws PatchListNotAvailableException {
+ throws ResourceConflictException, PatchListNotAvailableException {
RevCommit commit = resource.getCommit();
PatchListKey key;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
index 7156c8d..524a05e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -37,6 +38,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.validators.CommentForValidation;
import com.google.gerrit.extensions.validators.CommentForValidation.CommentType;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.restapi.change.PostReview;
import com.google.gerrit.server.update.CommentsRejectedException;
@@ -80,14 +82,17 @@
@Test
public void validateCommentsInInput_commentOK() throws Exception {
+ PushOneCommit.Result r = createChange();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- PushOneCommit.Result r = createChange();
-
ReviewInput input = new ReviewInput();
CommentInput comment = newComment(r.getChange().currentFilePaths().get(0));
comment.updated = new Timestamp(0);
@@ -101,14 +106,17 @@
@Test
public void validateCommentsInInput_commentRejected() throws Exception {
+ PushOneCommit.Result r = createChange();
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- PushOneCommit.Result r = createChange();
-
ReviewInput input = new ReviewInput();
CommentInput comment = newComment(r.getChange().currentFilePaths().get(0));
comment.updated = new Timestamp(0);
@@ -151,14 +159,17 @@
@Test
public void validateDrafts_draftOK() throws Exception {
+ PushOneCommit.Result r = createChange();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- PushOneCommit.Result r = createChange();
-
DraftInput draft =
testCommentHelper.newDraft(
r.getChange().currentFilePaths().get(0), Side.REVISION, 1, COMMENT_TEXT);
@@ -174,14 +185,18 @@
@Test
public void validateDrafts_draftRejected() throws Exception {
+ PushOneCommit.Result r = createChange();
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.INLINE_COMMENT, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- PushOneCommit.Result r = createChange();
DraftInput draft =
testCommentHelper.newDraft(
@@ -218,7 +233,8 @@
testCommentHelper.addDraft(r.getChangeId(), r.getCommit().getName(), draftFile);
assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
- when(mockCommentValidator.validateComments(capture.capture())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(any(), capture.capture()))
+ .thenReturn(ImmutableList.of());
ReviewInput input = new ReviewInput();
input.drafts = DraftHandling.PUBLISH;
@@ -236,11 +252,15 @@
@Test
public void validateCommentsInChangeMessage_messageOK() throws Exception {
+ PushOneCommit.Result r = createChange();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- PushOneCommit.Result r = createChange();
ReviewInput input = new ReviewInput().message(COMMENT_TEXT);
int numMessages = gApi.changes().id(r.getChangeId()).get().messages.size();
@@ -253,13 +273,17 @@
@Test
public void validateCommentsInChangeMessage_messageRejected() throws Exception {
+ PushOneCommit.Result r = createChange();
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- PushOneCommit.Result r = createChange();
ReviewInput input = new ReviewInput().message(COMMENT_TEXT);
assertThat(gApi.changes().id(r.getChangeId()).get().messages)
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index b0f183e..2883d8c 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -59,6 +59,7 @@
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -436,6 +437,16 @@
}
@Test
+ public void renameExistingFileToInvalidPath() throws Exception {
+ createEmptyEditFor(changeId);
+ BadRequestException badRequest =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(changeId).edit().renameFile(FILE_NAME, "invalid/path/"));
+ assertThat(badRequest.getMessage()).isEqualTo("Invalid path: invalid/path/");
+ }
+
+ @Test
public void createEditByDeletingExistingFileRest() throws Exception {
adminRestSession.delete(urlEditFile(changeId, FILE_NAME)).assertNoContent();
assertThat(getFileContentOfEdit(changeId, FILE_NAME)).isAbsent();
diff --git a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
index d8b65b7..ccfe783 100644
--- a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
@@ -32,6 +32,7 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.validators.CommentForValidation;
import com.google.gerrit.extensions.validators.CommentForValidation.CommentType;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.testing.TestCommentHelper;
import com.google.inject.Inject;
@@ -53,6 +54,7 @@
private static final String COMMENT_TEXT = "The comment text";
@Captor private ArgumentCaptor<ImmutableList<CommentForValidation>> capture;
+ @Captor private ArgumentCaptor<CommentValidationContext> captureCtx;
@Override
public Module createModule() {
@@ -76,14 +78,18 @@
@Test
public void validateComments_commentOK() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String revId = result.getCommit().getName();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(result.getChange().getId().get())
+ .project(result.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- PushOneCommit.Result result = createChange();
- String changeId = result.getChangeId();
- String revId = result.getCommit().getName();
DraftInput comment = testCommentHelper.newDraft(COMMENT_TEXT);
testCommentHelper.addDraft(changeId, revId, comment);
assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
@@ -97,14 +103,18 @@
public void validateComments_commentRejected() throws Exception {
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT);
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String revId = result.getCommit().getName();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(result.getChange().getId().get())
+ .project(result.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- PushOneCommit.Result result = createChange();
- String changeId = result.getChangeId();
- String revId = result.getCommit().getName();
DraftInput comment = testCommentHelper.newDraft(COMMENT_TEXT);
testCommentHelper.addDraft(changeId, revId, comment);
assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
@@ -116,7 +126,8 @@
@Test
public void validateComments_inlineVsFileComments_allOK() throws Exception {
- when(mockCommentValidator.validateComments(capture.capture())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(captureCtx.capture(), capture.capture()))
+ .thenReturn(ImmutableList.of());
PushOneCommit.Result result = createChange();
String changeId = result.getChangeId();
String revId = result.getCommit().getName();
@@ -132,6 +143,9 @@
assertThat(capture.getAllValues()).hasSize(1);
+ assertThat(captureCtx.getValue().getProject()).isEqualTo(result.getChange().project().get());
+ assertThat(captureCtx.getValue().getChangeId()).isEqualTo(result.getChange().getId().get());
+
assertThat(capture.getAllValues().get(0))
.containsExactly(
CommentForValidation.create(
@@ -143,7 +157,7 @@
@Test
@GerritConfig(name = "change.maxCommentLength", value = "" + MAX_COMMENT_LENGTH)
public void validateComments_enforceLimits_commentTooLarge() throws Exception {
- when(mockCommentValidator.validateComments(any())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(any(), any())).thenReturn(ImmutableList.of());
PushOneCommit.Result result = createChange();
String changeId = result.getChangeId();
int commentLength = MAX_COMMENT_LENGTH + 1;
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index 5531709..2409f52 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -29,6 +29,7 @@
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.mail.MailMessage;
import com.google.gerrit.mail.MailProcessingUtil;
@@ -70,7 +71,7 @@
@BeforeClass
public static void setUpMock() {
// Let the mock comment validator accept all comments during test setup.
- when(mockCommentValidator.validateComments(any())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(any(), any())).thenReturn(ImmutableList.of());
}
@Before
@@ -274,7 +275,8 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- setupFailValidation(CommentForValidation.CommentType.CHANGE_MESSAGE);
+ setupFailValidation(
+ CommentForValidation.CommentType.CHANGE_MESSAGE, changeInfo.project, changeInfo._number);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", COMMENT_TEXT, null, null, null);
@@ -298,7 +300,8 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- setupFailValidation(CommentForValidation.CommentType.INLINE_COMMENT);
+ setupFailValidation(
+ CommentForValidation.CommentType.INLINE_COMMENT, changeInfo.project, changeInfo._number);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, COMMENT_TEXT, null, null);
@@ -322,7 +325,8 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- setupFailValidation(CommentForValidation.CommentType.FILE_COMMENT);
+ setupFailValidation(
+ CommentForValidation.CommentType.FILE_COMMENT, changeInfo.project, changeInfo._number);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, null, COMMENT_TEXT, null);
@@ -341,10 +345,12 @@
return canonicalWebUrl.get() + "c/" + changeInfo.project + "/+/" + changeInfo._number;
}
- private void setupFailValidation(CommentForValidation.CommentType type) {
+ private void setupFailValidation(
+ CommentForValidation.CommentType type, String failProject, int failChange) {
CommentForValidation commentForValidation = CommentForValidation.create(type, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder().changeId(failChange).project(failProject).build(),
ImmutableList.of(CommentForValidation.create(type, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
}
diff --git a/javatests/com/google/gerrit/pgm/init/LibrariesTest.java b/javatests/com/google/gerrit/pgm/init/LibrariesTest.java
deleted file mode 100644
index 5aa4718..0000000
--- a/javatests/com/google/gerrit/pgm/init/LibrariesTest.java
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright (C) 2009 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.pgm.init;
-
-import static org.junit.Assert.assertNotNull;
-import static org.mockito.Mockito.verifyZeroInteractions;
-
-import com.google.gerrit.pgm.init.api.ConsoleUI;
-import com.google.gerrit.server.config.SitePaths;
-import java.nio.file.Paths;
-import java.util.Collections;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class LibrariesTest {
- @Mock ConsoleUI ui;
- @Mock StaleLibraryRemover remover;
-
- @Test
- public void create() throws Exception {
- final SitePaths site = new SitePaths(Paths.get("."));
-
- Libraries lib =
- new Libraries(
- () -> new LibraryDownloader(ui, site, remover), Collections.emptyList(), false);
-
- assertNotNull(lib.mysqlDriver);
- verifyZeroInteractions(ui);
- verifyZeroInteractions(remover);
- }
-}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index f4bd6a6..283bd74 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -205,6 +205,9 @@
<gr-confirm-revert-dialog id="confirmRevertDialog"
class="confirmDialog"
on-confirm="_handleRevertDialogConfirm"
+ commit-message="[[commitMessage]]"
+ change="[[change]]"
+ changes="[[_revertChanges]]"
on-cancel="_handleConfirmDialogCancel"
hidden></gr-confirm-revert-dialog>
<gr-confirm-revert-submission-dialog id="confirmRevertSubmissionDialog"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 2c4ca82..a4ed899 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -192,6 +192,11 @@
const AWAIT_CHANGE_ATTEMPTS = 5;
const AWAIT_CHANGE_TIMEOUT_MS = 1000;
+ const REVERT_TYPES = {
+ REVERT_SINGLE_CHANGE: 1,
+ REVERT_SUBMISSION: 2,
+ };
+
/**
* @appliesMixin Gerrit.FireMixin
* @appliesMixin Gerrit.PatchSetMixin
@@ -421,6 +426,7 @@
type: Boolean,
value: true,
},
+ _revertChanges: Array,
};
}
@@ -915,16 +921,13 @@
return null;
}
- _modifyRevertMsg() {
- return this.$.jsAPI.modifyRevertMsg(this.change,
- this.$.confirmRevertDialog.message, this.commitMessage);
- }
-
showRevertDialog() {
- this.$.confirmRevertDialog.populateRevertMessage(
- this.commitMessage, this.change.current_revision);
- this.$.confirmRevertDialog.message = this._modifyRevertMsg();
- this._showActionDialog(this.$.confirmRevertDialog);
+ const query = 'submissionid:' + this.change.submission_id;
+ this.$.restAPI.getChanges('', query)
+ .then(changes => {
+ this._revertChanges = changes;
+ this._showActionDialog(this.$.confirmRevertDialog);
+ });
}
showRevertSubmissionDialog() {
@@ -932,7 +935,7 @@
this.$.restAPI.getChanges('', query)
.then(changes => {
this.$.confirmRevertSubmissionDialog.
- populateRevertSubmissionMessage(
+ _populateRevertSubmissionMessage(
this.commitMessage, this.change, changes);
this._showActionDialog(this.$.confirmRevertSubmissionDialog);
});
@@ -1143,20 +1146,24 @@
);
}
- _handleRevertDialogConfirm() {
+ _handleRevertDialogConfirm(e) {
+ const revertType = e.detail.revertType;
+ const message = e.detail.message;
const el = this.$.confirmRevertDialog;
this.$.overlay.close();
el.hidden = true;
- this._fireAction('/revert', this.actions.revert, false,
- {message: el.message});
- }
-
- _handleRevertSubmissionDialogConfirm() {
- const el = this.$.confirmRevertSubmissionDialog;
- this.$.overlay.close();
- el.hidden = true;
- this._fireAction('/revert_submission', this.actions.revert_submission,
- false, {message: el.message});
+ switch (revertType) {
+ case REVERT_TYPES.REVERT_SINGLE_CHANGE:
+ this._fireAction('/revert', this.actions.revert, false,
+ {message});
+ break;
+ case REVERT_TYPES.REVERT_SUBMISSION:
+ this._fireAction('/revert_submission', this.actions.revert_submission,
+ false, {message});
+ break;
+ default:
+ console.error('invalid revert type');
+ }
}
_handleAbandonDialogConfirm() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 1c894ee..9c1a527 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -37,6 +37,7 @@
</test-fixture>
<script>
+ // TODO(dhruvsri): remove use of _populateRevertMessage as it's private
suite('gr-change-actions tests', () => {
let element;
let sandbox;
@@ -795,12 +796,12 @@
});
suite('revert change', () => {
- let alertStub;
let fireActionStub;
setup(() => {
fireActionStub = sandbox.stub(element, '_fireAction');
- alertStub = sandbox.stub(window, 'alert');
+ element.commitMessage = 'random commit message';
+ element.change.current_revision = 'abcdef';
element.actions = {
revert: {
method: 'POST',
@@ -813,50 +814,149 @@
});
test('revert change with plugin hook', done => {
+ const newRevertMsg = 'Modified revert msg';
+ sandbox.stub(element.$.confirmRevertDialog, '_modifyRevertMsg',
+ () => newRevertMsg);
element.change = {
current_revision: 'abc1234',
};
- const newRevertMsg = 'Modified revert msg';
- sandbox.stub(element, '_modifyRevertMsg',
- () => newRevertMsg);
- sandbox.stub(element.$.confirmRevertDialog, 'populateRevertMessage',
- () => 'original msg');
+ sandbox.stub(element.$.confirmRevertDialog,
+ '_populateRevertSubmissionMessage', () => 'original msg');
flush(() => {
- const revertButton =
- element.$$('gr-button[data-action-key="revert"]');
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
MockInteractions.tap(revertButton);
-
- assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
- done();
+ flush(() => {
+ assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
+ done();
+ });
});
});
- test('works', () => {
- element.change = {
- current_revision: 'abc1234',
- };
- sandbox.stub(element.$.confirmRevertDialog, 'populateRevertMessage',
- () => 'original msg');
- const revertButton = element.$$('gr-button[data-action-key="revert"]');
- MockInteractions.tap(revertButton);
+ suite('revert change submitted together', () => {
+ setup(() => {
+ element.change = {
+ submission_id: '199',
+ current_revision: '2000',
+ };
+ sandbox.stub(element.$.restAPI, 'getChanges')
+ .returns(Promise.resolve([
+ {change_id: '12345678901234', topic: 'T', subject: 'random'},
+ {change_id: '23456', topic: 'T', subject: 'a'.repeat(100)},
+ ]));
+ });
- element.$.confirmRevertDialog.message = 'foo message';
- element._handleRevertDialogConfirm();
- assert.notOk(alertStub.called);
+ test('confirm revert dialog shows both options', done => {
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
+ MockInteractions.tap(revertButton);
+ flush(() => {
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ const revertSingleChangeLabel = confirmRevertDialog
+ .shadowRoot.querySelector('.revertSingleChange');
+ const revertSubmissionLabel = confirmRevertDialog.
+ shadowRoot.querySelector('.revertSubmission');
+ assert(revertSingleChangeLabel.innerText.trim() ===
+ 'Revert single change');
+ assert(revertSubmissionLabel.innerText.trim() ===
+ 'Revert entire submission (2 Changes)');
+ let expectedMsg = 'Revert submission 199' + '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>' + '\n' +
+ 'Reverted Changes:' + '\n' +
+ '1234567890:random' + '\n' +
+ '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+ '\n';
+ assert.equal(confirmRevertDialog.message, expectedMsg);
+ const radioInputs = confirmRevertDialog.shadowRoot
+ .querySelectorAll('input[name="revertOptions"]');
+ MockInteractions.tap(radioInputs[0]);
+ flush(() => {
+ expectedMsg = 'Revert "random commit message"\n\nThis reverts '
+ + 'commit 2000.\n\nReason'
+ + ' for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog.message, expectedMsg);
+ done();
+ });
+ });
+ });
- const action = {
- __key: 'revert',
- __type: 'change',
- __primary: false,
- enabled: true,
- label: 'Revert',
- method: 'POST',
- title: 'Revert the change',
- };
- assert.deepEqual(fireActionStub.lastCall.args, [
- '/revert', action, false, {
- message: 'foo message',
- }]);
+ test('message modification is retained on switching', done => {
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ MockInteractions.tap(revertButton);
+ flush(() => {
+ const radioInputs = confirmRevertDialog.shadowRoot
+ .querySelectorAll('input[name="revertOptions"]');
+ const revertSubmissionMsg = 'Revert submission 199' + '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>' + '\n' +
+ 'Reverted Changes:' + '\n' +
+ '1234567890:random' + '\n' +
+ '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+ '\n';
+ const singleChangeMsg =
+ 'Revert "random commit message"\n\nThis reverts '
+ + 'commit 2000.\n\nReason'
+ + ' for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog.message, revertSubmissionMsg);
+ const newRevertMsg = revertSubmissionMsg + 'random';
+ const newSingleChangeMsg = singleChangeMsg + 'random';
+ confirmRevertDialog.message = newRevertMsg;
+ MockInteractions.tap(radioInputs[0]);
+ flush(() => {
+ assert.equal(confirmRevertDialog.message, singleChangeMsg);
+ confirmRevertDialog.message = newSingleChangeMsg;
+ MockInteractions.tap(radioInputs[1]);
+ flush(() => {
+ assert.equal(confirmRevertDialog.message, newRevertMsg);
+ MockInteractions.tap(radioInputs[0]);
+ flush(() => {
+ assert.equal(confirmRevertDialog.message, newSingleChangeMsg);
+ done();
+ });
+ });
+ });
+ });
+ });
+ });
+
+ suite('revert single change', () => {
+ setup(() => {
+ element.change = {
+ submission_id: '199',
+ current_revision: '2000',
+ };
+ sandbox.stub(element.$.restAPI, 'getChanges')
+ .returns(Promise.resolve([
+ {change_id: '12345678901234', topic: 'T', subject: 'random'},
+ ]));
+ });
+
+ test('confirm revert dialog shows one radio button', done => {
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
+ MockInteractions.tap(revertButton);
+ flush(() => {
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ const radioInputs = confirmRevertDialog.shadowRoot
+ .querySelectorAll('input[name="revertOptions"]');
+ assert.equal(radioInputs.length, 1);
+ const msg = 'Revert "random commit message"\n\n'
+ + 'This reverts commit 2000.\n\nReason '
+ + 'for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog.message, msg);
+ const confirmButton = element.$.confirmRevertDialog.shadowRoot
+ .querySelector('gr-dialog')
+ .shadowRoot.querySelector('#confirm');
+ MockInteractions.tap(confirmButton);
+ flush(() => {
+ assert.equal(fireActionStub.getCall(0).args[0], '/revert');
+ assert.equal(fireActionStub.getCall(0).args[1].__key, 'revert');
+ assert.equal(fireActionStub.getCall(0).args[3].message, msg);
+ done();
+ });
+ });
+ });
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
index fc3a8c1..7bffe8a 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
@@ -21,6 +21,7 @@
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.html">
+<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<dom-module id="gr-confirm-revert-dialog">
<template>
@@ -37,6 +38,13 @@
display: block;
width: 100%;
}
+ .revertSubmissionLayout {
+ display: flex;
+ }
+ .label {
+ margin-left: var(--spacing-m);
+ margin-bottom: var(--spacing-m);
+ }
iron-autogrow-textarea {
font-family: var(--monospace-font-family);
font-size: var(--font-size-mono);
@@ -50,7 +58,45 @@
on-cancel="_handleCancelTap">
<div class="header" slot="header">Revert Merged Change</div>
<div class="main" slot="main">
+ <div class="revertSubmissionLayout">
+ <input
+ name="revertOptions"
+ type="radio"
+ id="revertSingleChange"
+ on-change="_handleRevertSingleChangeClicked"
+ checked="[[_computeIfSingleRevert(_revertType)]]">
+ <label for="revertSingleChange" class="label revertSingleChange">
+ Revert single change
+ </label>
+ </div>
+ <template is="dom-if" if="[[_showRevertSubmission]]">
+ <div on-click="_handleRevertSubmissionClicked" class="revertSubmissionLayout">
+ <input
+ name="revertOptions"
+ type="radio"
+ id="revertSubmission"
+ checked="[[_computeIfRevertSubmission(_revertType)]]">
+ <label for="revertSubmission" class="label revertSubmission">
+ Revert entire submission ([[changes.length]] Changes)
+ </label>
+ </template>
<gr-endpoint-decorator name="confirm-revert-change">
+ <!-- Duplicating the text-area as a plugin in the case of a single
+ revert will override the entire textarea which should not happen
+ for multiple revert -->
+ <template is="dom-if" if="[[_computeIfSingleRevert(_revertType)]]">
+ <label for="messageInput">
+ Revert Commit Message
+ </label>
+ <iron-autogrow-textarea
+ id="messageInput"
+ class="message"
+ autocomplete="on"
+ max-rows="15"
+ bind-value="{{message}}"></iron-autogrow-textarea>
+ </template>
+ </gr-endpoint-decorator>
+ <template is="dom-if" if="[[_computeIfRevertSubmission(_revertType)]]">
<label for="messageInput">
Revert Commit Message
</label>
@@ -60,9 +106,10 @@
autocomplete="on"
max-rows="15"
bind-value="{{message}}"></iron-autogrow-textarea>
- </gr-endpoint-decorator>
+ </template>
</div>
</gr-dialog>
+ <gr-js-api-interface id="jsAPI"></gr-js-api-interface>
</template>
<script src="gr-confirm-revert-dialog.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
index bf727ec..438440e 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
@@ -19,6 +19,13 @@
const ERR_COMMIT_NOT_FOUND =
'Unable to find the commit hash of this change.';
+ const CHANGE_SUBJECT_LIMIT = 50;
+
+ // TODO(dhruvsri): clean up repeated definitions after moving to js modules
+ const REVERT_TYPES = {
+ REVERT_SINGLE_CHANGE: 1,
+ REVERT_SUBMISSION: 2,
+ };
/**
* @appliesMixin Gerrit.FireMixin
@@ -45,12 +52,55 @@
static get properties() {
return {
message: String,
+ _revertType: {
+ type: Number,
+ value: REVERT_TYPES.REVERT_SINGLE_CHANGE,
+ },
+ _showRevertSubmission: {
+ type: Boolean,
+ value: false,
+ },
+ changes: {
+ type: Array,
+ value() { return []; },
+ },
+ change: Object,
+ commitMessage: String,
};
}
- populateRevertMessage(message, commitHash) {
+ static get observers() {
+ return [
+ 'onInputUpdate(change, commitMessage, changes)',
+ ];
+ }
+
+ _computeIfSingleRevert(revertType) {
+ return revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE;
+ }
+
+ _computeIfRevertSubmission(revertType) {
+ return revertType === REVERT_TYPES.REVERT_SUBMISSION;
+ }
+
+ _modifyRevertMsg(change, commitMessage, message) {
+ return this.$.jsAPI.modifyRevertMsg(change,
+ message, commitMessage);
+ }
+
+ onInputUpdate(change, commitMessage, changes) {
+ if (!change || !changes) return;
+ this._populateRevertSingleChangeMessage(
+ change, commitMessage, change.current_revision);
+ if (changes.length > 1) {
+ this._populateRevertSubmissionMessage(
+ change, changes);
+ }
+ }
+
+ _populateRevertSingleChangeMessage(change, commitMessage, commitHash) {
// Figure out what the revert title should be.
- const originalTitle = message.split('\n')[0];
+ const originalTitle = (commitMessage || '').split('\n')[0];
const revertTitle = `Revert "${originalTitle}"`;
if (!commitHash) {
this.fire('show-alert', {message: ERR_COMMIT_NOT_FOUND});
@@ -58,20 +108,77 @@
}
const revertCommitText = `This reverts commit ${commitHash}.`;
- this.message = `${revertTitle}\n\n${revertCommitText}\n\n` +
+ this.revertSingleChangeMessage =
+ `${revertTitle}\n\n${revertCommitText}\n\n` +
`Reason for revert: <INSERT REASONING HERE>\n`;
+ // This is to give plugins a chance to update message
+ this.revertSingleChangeMessage =
+ this._modifyRevertMsg(change, commitMessage,
+ this.revertSingleChangeMessage);
+ this.message = this.revertSingleChangeMessage;
+ }
+
+ _getTrimmedChangeSubject(subject) {
+ if (!subject) return '';
+ if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
+ return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
+ }
+
+ _modifyRevertSubmissionMsg(change) {
+ return this.$.jsAPI.modifyRevertSubmissionMsg(change,
+ this.revertSubmissionMessage, this.commitMessage);
+ }
+
+ _populateRevertSubmissionMessage(change, changes) {
+ // Follow the same convention of the revert
+ const commitHash = change.current_revision;
+ if (!commitHash) {
+ this.fire('show-alert', {message: ERR_COMMIT_NOT_FOUND});
+ return;
+ }
+ if (!changes || changes.length <= 1) return;
+ const submissionId = change.submission_id;
+ const revertTitle = 'Revert submission ' + submissionId;
+ this.changes = changes;
+ this.revertSubmissionMessage = revertTitle + '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>\n';
+ this.revertSubmissionMessage += 'Reverted Changes:\n';
+ changes.forEach(change => {
+ this.revertSubmissionMessage += change.change_id.substring(0, 10) + ':'
+ + this._getTrimmedChangeSubject(change.subject) + '\n';
+ });
+ this.revertSubmissionMessage = this._modifyRevertSubmissionMsg(change);
+ this.message = this.revertSubmissionMessage;
+ this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+ this._showRevertSubmission = true;
+ }
+
+ _handleRevertSingleChangeClicked() {
+ if (this._revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE) return;
+ this.revertSubmissionMessage = this.message;
+ this.message = this.revertSingleChangeMessage;
+ this._revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
+ }
+
+ _handleRevertSubmissionClicked() {
+ if (this._revertType === REVERT_TYPES.REVERT_SUBMISSION) return;
+ this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+ this.revertSingleChangeMessage = this.message;
+ this.message = this.revertSubmissionMessage;
}
_handleConfirmTap(e) {
e.preventDefault();
e.stopPropagation();
- this.fire('confirm', null, {bubbles: false});
+ this.fire('confirm', {revertType: this._revertType,
+ message: this.message}, {bubbles: false});
}
_handleCancelTap(e) {
e.preventDefault();
e.stopPropagation();
- this.fire('cancel', null, {bubbles: false});
+ this.fire('cancel', {revertType: this._revertType},
+ {bubbles: false});
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
index dbdfba2..1d28d32 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
@@ -50,13 +50,14 @@
assert.isNotOk(element.message);
const alertStub = sandbox.stub();
element.addEventListener('show-alert', alertStub);
- element.populateRevertMessage('not a commitHash in sight', undefined);
+ element._populateRevertSingleChangeMessage({},
+ 'not a commitHash in sight', undefined);
assert.isTrue(alertStub.calledOnce);
});
test('single line', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'one line commit\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "one line commit"\n\n' +
@@ -67,7 +68,7 @@
test('multi line', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "many lines"\n\n' +
@@ -78,7 +79,7 @@
test('issue above change id', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'much lines\nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "much lines"\n\n' +
@@ -89,7 +90,7 @@
test('revert a revert', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'Revert "one line commit"\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "Revert "one line commit""\n\n' +
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
index d59f5cd..ae8dfa5 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
@@ -50,7 +50,7 @@
};
}
- getTrimmedChangeSubject(subject) {
+ _getTrimmedChangeSubject(subject) {
if (!subject) return '';
if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
@@ -61,7 +61,7 @@
this.message, this.commitMessage);
}
- populateRevertSubmissionMessage(message, change, changes) {
+ _populateRevertSubmissionMessage(message, change, changes) {
// Follow the same convention of the revert
const commitHash = change.current_revision;
if (!commitHash) {
@@ -77,7 +77,7 @@
changes = changes || [];
changes.forEach(change => {
this.message += change.change_id.substring(0, 10) + ': ' +
- this.getTrimmedChangeSubject(change.subject) + '\n';
+ this._getTrimmedChangeSubject(change.subject) + '\n';
});
this.message = this._modifyRevertSubmissionMsg(change);
}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
index cc4bd54..af99c7e 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
@@ -51,7 +51,7 @@
assert.isNotOk(element.message);
const alertStub = sandbox.stub();
element.addEventListener('show-alert', alertStub);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'not a commitHash in sight'
);
assert.isTrue(alertStub.calledOnce);
@@ -59,7 +59,7 @@
test('single line', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'one line commit\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
@@ -69,7 +69,7 @@
test('multi line', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
@@ -79,7 +79,7 @@
test('issue above change id', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'test \nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
@@ -89,7 +89,7 @@
test('revert a revert', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'Revert "one line commit"\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
index 359a039..e46f959 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
@@ -192,15 +192,20 @@
}
}
- /**
- * For Polymer 2, use shadowRoot.getSelection instead.
- */
_getSelection() {
- const diffHost = util.querySelector(document.body, 'gr-diff');
- const selection = diffHost &&
- diffHost.shadowRoot &&
- diffHost.shadowRoot.getSelection();
- return selection ? selection: window.getSelection();
+ const diffHosts = util.querySelectorAll(document.body, 'gr-diff');
+ if (!diffHosts.length) return window.getSelection();
+
+ const curDiffHost = diffHosts.find(diffHost => {
+ if (!diffHost || !diffHost.shadowRoot) return false;
+ const selection = diffHost.shadowRoot.getSelection();
+ // Pick the one with valid selection:
+ // https://developer.mozilla.org/en-US/docs/Web/API/Selection/type
+ return selection && selection.type !== 'None';
+ });
+
+ return curDiffHost ?
+ curDiffHost.shadowRoot.getSelection(): window.getSelection();
}
/**
diff --git a/polygerrit-ui/app/scripts/util.js b/polygerrit-ui/app/scripts/util.js
index 672c43f..565b9b3 100644
--- a/polygerrit-ui/app/scripts/util.js
+++ b/polygerrit-ui/app/scripts/util.js
@@ -105,7 +105,7 @@
*/
util.querySelector = (el, selector) => {
let nodes = [el];
- let element = null;
+ let result = null;
while (nodes.length) {
const node = nodes.pop();
@@ -113,19 +113,49 @@
if (!node || !node.querySelector) continue;
// Try find it with native querySelector directly
- element = node.querySelector(selector);
+ result = node.querySelector(selector);
- if (element) {
+ if (result) {
break;
- } else if (node.shadowRoot) {
- // If shadowHost detected, add the host and its children
- nodes = nodes.concat(Array.from(node.children));
- nodes.push(node.shadowRoot);
- } else {
- nodes = nodes.concat(Array.from(node.children));
}
+
+ // Add all nodes with shadowRoot and loop through
+ const allShadowNodes = [...node.querySelectorAll('*')]
+ .filter(child => !!child.shadowRoot)
+ .map(child => child.shadowRoot);
+ nodes = nodes.concat(allShadowNodes);
}
- return element;
+ return result;
+ };
+
+ /**
+ * Query selector all dom elements matching with certain selector.
+ *
+ * This is shadow DOM compatible, but only works when selector is within
+ * one shadow host, won't work if your selector is crossing
+ * multiple shadow hosts.
+ *
+ * Note: this can be very expensive, only use when have to.
+ */
+ util.querySelectorAll = (el, selector) => {
+ let nodes = [el];
+ const results = new Set();
+ while (nodes.length) {
+ const node = nodes.pop();
+
+ if (!node || !node.querySelectorAll) continue;
+
+ // Try find all from regular children
+ [...node.querySelectorAll(selector)]
+ .forEach(el => results.add(el));
+
+ // Add all nodes with shadowRoot and loop through
+ const allShadowNodes = [...node.querySelectorAll('*')]
+ .filter(child => !!child.shadowRoot)
+ .map(child => child.shadowRoot);
+ nodes = nodes.concat(allShadowNodes);
+ }
+ return [...results];
};
window.util = util;
diff --git a/resources/com/google/gerrit/pgm/init/libraries.config b/resources/com/google/gerrit/pgm/init/libraries.config
deleted file mode 100644
index 3d3545b..0000000
--- a/resources/com/google/gerrit/pgm/init/libraries.config
+++ /dev/null
@@ -1,50 +0,0 @@
-# Copyright (C) 2009 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.
-
-[library "mysqlDriver"]
- name = MySQL Connector/J 5.1.43
- url = https://repo1.maven.org/maven2/mysql/mysql-connector-java/5.1.43/mysql-connector-java-5.1.43.jar
- sha1 = dee9103eec0d877f3a21c82d4d9e9f4fbd2d6e0a
- remove = mysql-connector-java-.*[.]jar
-
-[library "mariadbDriver"]
- name = MariaDB Connector/J 2.3.0
- url = https://repo1.maven.org/maven2/org/mariadb/jdbc/mariadb-java-client/2.3.0/mariadb-java-client-2.3.0.jar
- sha1 = c2b1a6002a169757d0649449288e9b3b776af76b
- remove = mariadb-java-client-.*[.]jar
-
-[library "oracleDriver"]
- name = Oracle JDBC driver 11g Release 2 (11.2.0)
- url = file:///u01/app/oracle/product/11.2.0/xe/jdbc/lib/ojdbc6.jar
- sha1 = 2f89cd9176772c3a6c261ce6a8e3d0d4425f5679
- remove = ojdbc6.jar
-
-[library "db2Driver"]
- name = DB2 Type 4 JDBC driver (10.5)
- url = file:///opt/ibm/db2/V10.5/java/db2jcc4.jar
- sha1 = 9344d4fd41d6511f2d1d1deb7759056495b3a39b
- needs = db2DriverLicense
- remove = db2jcc4.jar
-
-# Omit SHA-1 for license JAR as it's not stable and depends on the product
-# the customer has purchased.
-[library "db2DriverLicense"]
- name = DB2 Type 4 JDBC driver license (10.5)
- url = file:///opt/ibm/db2/V10.5/java/db2jcc_license_cu.jar
- remove = db2jcc_license_cu.jar
-
-[library "hanaDriver"]
- name = HANA JDBC driver
- url = file:///usr/sap/hdbclient/ngdbc.jar
- remove = ngdbc.jar
