Merge "Add 'CherryPickOf' field for a change"
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index c97e4a4..036dfbf 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -342,8 +342,14 @@
=== Elasticsearch
Successfully running the Elasticsearch tests requires Docker, and
-may require setting the local
-link:https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html[virtual memory,role=external,window=_blank].
+may require setting the local virtual memory on
+link:https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html[linux,role=external,window=_blank] and
+link:https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html#_set_vm_max_map_count_to_at_least_262144[macOS,role=external,window=_blank].
+
+On macOS, if using link:https://docs.docker.com/docker-for-mac/[Docker Desktop,role=external,window=_blank],
+the effective memory value can be set in the Preferences, under the Advanced tab.
+The default value usually does not suffice and is causing premature container exits.
+That default is currently 2 GB and should be set to at least 5 (GB).
If Docker is not available, the Elasticsearch tests will be skipped.
Note that Bazel currently does not show
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index b373129..236792b 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -141,10 +141,10 @@
=== NoteDb
-* `notedb/update_latency`: NoteDb update latency by table.
-* `notedb/stage_update_latency`: Latency for staging updates to NoteDb by table.
-* `notedb/read_latency`: NoteDb read latency by table.
-* `notedb/parse_latency`: NoteDb parse latency by table.
+* `notedb/update_latency`: NoteDb update latency for changes.
+* `notedb/stage_update_latency`: Latency for staging change updates to NoteDb.
+* `notedb/read_latency`: NoteDb read latency for changes.
+* `notedb/parse_latency`: NoteDb parse latency for changes.
* `notedb/external_id_cache_load_count`: Total number of times the external ID
cache loader was called.
* `notedb/external_id_partial_read_latency`: Latency for generating a new external ID
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 27a31b1..e3e9009 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1251,7 +1251,6 @@
)]}'
{
"changes_per_page": 25,
- "download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"diff_view": "SIDE_BY_SIDE",
@@ -1305,7 +1304,6 @@
{
"changes_per_page": 50,
"expand_inline_diffs": true,
- "download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"size_bar_in_change_table": true,
@@ -1353,7 +1351,6 @@
{
"changes_per_page": 50,
"expand_inline_diffs": true,
- "download_command": "CHECKOUT",
"date_format": "STD",
"time_format": "HHMM_12",
"size_bar_in_change_table": true,
@@ -1415,7 +1412,6 @@
)]}'
{
"context": 10,
- "theme": "DEFAULT",
"ignore_whitespace": "IGNORE_ALL",
"intraline_difference": true,
"line_length": 100,
@@ -1446,7 +1442,6 @@
{
"context": 10,
- "theme": "ECLIPSE",
"ignore_whitespace": "IGNORE_ALL",
"intraline_difference": true,
"line_length": 100,
@@ -1472,7 +1467,6 @@
)]}'
{
"context": 10,
- "theme": "ECLIPSE",
"ignore_whitespace": "IGNORE_ALL",
"intraline_difference": true,
"line_length": 100,
@@ -1509,8 +1503,6 @@
)]}'
{
- "theme": "ECLIPSE",
- "key_map_type": "VIM",
"tab_size": 4,
"line_length": 80,
"indent_unit": 2,
@@ -1542,8 +1534,6 @@
Content-Type: application/json;charset=UTF-8
{
- "theme": "ECLIPSE",
- "key_map_type": "VIM",
"tab_size": 4,
"line_length": 80,
"indent_unit": 2,
@@ -1570,8 +1560,6 @@
)]}'
{
- "theme": "ECLIPSE",
- "key_map_type": "VIM",
"tab_size": 4,
"line_length": 80,
"cursor_blink_rate": 530,
@@ -2392,13 +2380,18 @@
The `ContributorAgreementInfo` entity contains information about a
contributor agreement.
-[options="header",cols="1,6"]
-|=================================
-|Field Name |Description
-|`name` |The name of the agreement.
-|`description` |The description of the agreement.
-|`url` |The URL of the agreement.
-|=================================
+[options="header",cols="1,^1,5"]
+|================================
+|Field Name ||Description
+|`name` ||The unique name of the contributor agreement.
+|`description` ||The description of the contributor agreement.
+|`url` ||The URL of the contributor agreement.
+|`auto_verify_group`|optional|
+The group to which a user that signs the contributor agreement online
+is added automatically as a link:rest-api-groups.html#group-info[
+GroupInfo] entity. If not set, users cannot sign the contributor
+agreement online.
+|================================
[[contributor-agreement-input]]
=== ContributorAgreementInput
@@ -2452,9 +2445,6 @@
|Field Name ||Description
|`context` ||
The number of lines of context when viewing a patch.
-|`theme` ||
-The CodeMirror theme name in upper case, for example `DEFAULT`. All the themes
-from the CodeMirror release that Gerrit is using are available.
|`expand_all_comments` |not set if `false`|
Whether all inline comments should be automatically expanded.
|`ignore_whitespace` ||
@@ -2576,12 +2566,6 @@
[options="header",cols="1,^1,5"]
|===========================================
|Field Name ||Description
-|`theme` ||
-The CodeMirror theme name in upper case, for example `DEFAULT`. All the themes
-from the CodeMirror release that Gerrit is using are available.
-|`key_map_type` ||
-The CodeMirror key map. Currently only a subset of key maps are
-supported: `DEFAULT`, `EMACS`, `SUBLIME`, `VIM`.
|`tab_size` ||
Number of spaces that should be used to display one tab.
|`line_length` ||
@@ -2737,8 +2721,6 @@
The type of download URL the user prefers to use. May be any key from
the `schemes` map in
link:rest-api-config.html#download-info[DownloadInfo].
-|`download_command` ||
-The type of download command the user prefers to use.
|`date_format` ||
The format to display the date in.
Allowed values are `STD`, `US`, `ISO`, `EURO`, `UK`.
@@ -2799,8 +2781,6 @@
(PolyGerrit only).
|`download_scheme` |optional|
The type of download URL the user prefers to use.
-|`download_command` |optional|
-The type of download command the user prefers to use.
|`date_format` |optional|
The format to display the date in.
Allowed values are `STD`, `US`, `ISO`, `EURO`, `UK`.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index d5abbe3..73ff085 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -73,7 +73,9 @@
Queries changes visible to the caller. The
link:user-search.html#_search_operators[query string] must be provided
by the `q` parameter. The `n` parameter can be used to limit the
-returned results.
+returned results. The `no-limit` parameter can be used remove the default
+limit on queries and return all results. This might not be supported by
+all index backends.
As result a list of link:#change-info[ChangeInfo] entries is returned.
The change output is sorted by the last update time, most recently
@@ -369,11 +371,6 @@
as link:#tracking-id-info[TrackingIdInfo].
--
-[[no-limit]]
---
-* `NO-LIMIT`: Return all results
---
-
.Request
----
GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES&o=DOWNLOAD_COMMANDS HTTP/1.0
@@ -1520,8 +1517,14 @@
Creates open revert changes for all of the changes of a certain submission.
The subject of each revert change will be 'Revert "<subject-of-reverted-change"'.
-If the subject is above 63 characters, the subject will be cut to 59 characters
-with "..." in the end.
+If the subject is above 60 characters, the subject will be cut to 56 characters
+with "..." in the end. However, whenever reverting the submission of a revert
+submission, the subject will be shortened from
+'Revert "Revert "<subject-of-reverted-change""' to
+'Revert^2 "<subject-of-reverted-change"'. Also, for every future revert submission,
+the only difference in the subject will be the number of the revert (instead of
+Revert^2 the subject will change to Revert^3 and so on).
+There are no guarantees about the subjects if the users change the default subjects.
Details for the revert can be specified in the request body inside a link:#revert-input[
RevertInput] The topic of all created revert changes will be
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index affe1ea..e9e3b94 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1477,8 +1477,8 @@
|`git_basic_auth_policy` |optional|
The link:config-gerrit.html#auth.gitBasicAuthPolicy[policy] to authenticate
Git over HTTP and REST API requests when
-link:config-gerrit.html#auth.type[authentication type] is `LDAP`.
-Can be `HTTP`, `LDAP` or `HTTP_LDAP`.
+link:config-gerrit.html#auth.type[authentication type] is `LDAP`,
+`LDAP_BIND` or `OAUTH`. Can be `HTTP`, `LDAP`, `HTTP_LDAP` or `OAUTH`.
|==========================================
[[cache-info]]
diff --git a/e2e-tests/load-tests/Dockerfile b/e2e-tests/load-tests/Dockerfile
new file mode 100644
index 0000000..ceae672
--- /dev/null
+++ b/e2e-tests/load-tests/Dockerfile
@@ -0,0 +1,32 @@
+FROM denvazh/gatling:3.2.1
+
+ARG gatling_git_version=1.0.9
+RUN apk add --no-cache maven
+RUN mvn dependency:get \
+ -DgroupId=com.gerritforge \
+ -DartifactId=gatling-git_2.12 \
+ -Dversion=$gatling_git_version \
+ -Dtype=pom
+RUN mvn dependency:copy-dependencies \
+ -f /root/.m2/repository/com/gerritforge/gatling-git_2.12/$gatling_git_version/gatling-git_2.12-$gatling_git_version.pom \
+ -DoutputDirectory=/opt/gatling/lib/
+RUN mvn dependency:get \
+ -Dartifact=com.gerritforge:gatling-git_2.12:$gatling_git_version:jar \
+ -Ddest=/opt/gatling/lib/gatling-git.jar
+
+ARG gatling_home=/home/gatling
+RUN addgroup -g 1000 -S appgroup && \
+ adduser -u 1000 -S gatling -G appgroup -h $gatling_home
+RUN cp -R /opt/gatling/* $gatling_home && \
+ chown -R gatling:appgroup $gatling_home
+
+WORKDIR $gatling_home
+USER gatling
+
+COPY ./src/test/scala/com/google/gerrit/scenarios $gatling_home/user-files/simulations
+COPY ./src/test/resources/application.conf $gatling_home/conf
+COPY ./src/test/resources/data $gatling_home/user-files/resources/data
+
+ENV GATLING_HOME=$gatling_home
+
+ENTRYPOINT ["/home/gatling/bin/gatling.sh"]
diff --git a/e2e-tests/load-tests/README.md b/e2e-tests/load-tests/README.md
new file mode 100644
index 0000000..534fde5
--- /dev/null
+++ b/e2e-tests/load-tests/README.md
@@ -0,0 +1,11 @@
+# How to build the Docker image
+
+```$shell
+docker build . -t e2e-tests
+```
+
+# How to run a test
+
+```$shell
+docker run -it e2e-tests -s com.google.gerrit.scenarios.ReplayRecordsFromFeederScenario
+```
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index fd61aa5..a933dac 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -124,6 +124,7 @@
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.restapi.change.Revisions;
import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.util.git.DelegateSystemReader;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.FakeEmailSender;
import com.google.gerrit.testing.FakeEmailSender.Message;
@@ -135,6 +136,7 @@
import com.google.inject.Provider;
import com.jcraft.jsch.KeyPair;
import java.io.ByteArrayOutputStream;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -170,11 +172,14 @@
import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.transport.FetchResult;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.Transport;
import org.eclipse.jgit.transport.TransportBundleStream;
import org.eclipse.jgit.transport.URIish;
+import org.eclipse.jgit.util.FS;
+import org.eclipse.jgit.util.SystemReader;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
@@ -289,6 +294,7 @@
private ProjectResetter resetter;
private List<Repository> toClose;
private String systemTimeZone;
+ private SystemReader oldSystemReader;
@Before
public void clearSender() {
@@ -385,6 +391,10 @@
}
protected void beforeTest(Description description) throws Exception {
+ // SystemReader must be overridden before creating any repos, since they read the user/system
+ // configs at initialization time, and are then stored in the RepositoryCache forever.
+ oldSystemReader = setFakeSystemReader(temporaryFolder.getRoot());
+
this.description = description;
GerritServer.Description classDesc =
GerritServer.Description.forTestClass(description, configName);
@@ -447,6 +457,28 @@
methodDesc.useSystemTime(), methodDesc.useClockStep(), methodDesc.useTimezone());
}
+ private static SystemReader setFakeSystemReader(File tempDir) {
+ SystemReader oldSystemReader = SystemReader.getInstance();
+ SystemReader.setInstance(
+ new DelegateSystemReader(oldSystemReader) {
+ @Override
+ public FileBasedConfig openJGitConfig(Config parent, FS fs) {
+ return new FileBasedConfig(parent, new File(tempDir, "jgit.config"), FS.detect());
+ }
+
+ @Override
+ public FileBasedConfig openUserConfig(Config parent, FS fs) {
+ return new FileBasedConfig(parent, new File(tempDir, "user.config"), FS.detect());
+ }
+
+ @Override
+ public FileBasedConfig openSystemConfig(Config parent, FS fs) {
+ return new FileBasedConfig(parent, new File(tempDir, "system.config"), FS.detect());
+ }
+ });
+ return oldSystemReader;
+ }
+
private void setTimeSettings(
boolean useSystemTime,
@Nullable UseClockStep useClockStep,
@@ -602,6 +634,8 @@
server.close();
server = null;
}
+ SystemReader.setInstance(oldSystemReader);
+ oldSystemReader = null;
}
protected void closeSsh() {
diff --git a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
index a095daa..1d6f51f 100644
--- a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
+++ b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
@@ -144,6 +144,11 @@
SystemReader.setInstance(
new DelegateSystemReader(oldSystemReader) {
@Override
+ public FileBasedConfig openJGitConfig(Config parent, FS fs) {
+ return new FileBasedConfig(parent, new File(tempDir, "jgit.config"), FS.detect());
+ }
+
+ @Override
public FileBasedConfig openUserConfig(Config parent, FS fs) {
return new FileBasedConfig(parent, new File(tempDir, "user.config"), FS.detect());
}
diff --git a/java/com/google/gerrit/acceptance/UseLocalDisk.java b/java/com/google/gerrit/acceptance/UseLocalDisk.java
index e177bb4..192caa0 100644
--- a/java/com/google/gerrit/acceptance/UseLocalDisk.java
+++ b/java/com/google/gerrit/acceptance/UseLocalDisk.java
@@ -21,6 +21,15 @@
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
+/**
+ * Annotation to mark tests that require a local disk for the execution.
+ *
+ * <p>Tests that do not have this annotation are executed in memory.
+ *
+ * <p>Using this annotation makes the execution of the test more expensive/slower. This is why it
+ * should only be used if the test requires a local disk (e.g. if the test triggers the Git garbage
+ * collection functionality which only works with a local disk).
+ */
@Target({TYPE, METHOD})
@Retention(RUNTIME)
public @interface UseLocalDisk {}
diff --git a/java/com/google/gerrit/acceptance/UseSsh.java b/java/com/google/gerrit/acceptance/UseSsh.java
index 5509140..12a9977 100644
--- a/java/com/google/gerrit/acceptance/UseSsh.java
+++ b/java/com/google/gerrit/acceptance/UseSsh.java
@@ -21,6 +21,14 @@
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
+/**
+ * Annotation to mark SSH tests.
+ *
+ * <p>When running tests the SSH functionality is disabled unless the {@link UseSsh} annotation is
+ * used.
+ *
+ * <p>SSH tests can be skipped when executing tests (see {@link com.google.gerrit.testing.SshMode}).
+ */
@Target({TYPE, METHOD})
@Retention(RUNTIME)
public @interface UseSsh {}
diff --git a/java/com/google/gerrit/acceptance/rest/PluginCollection.java b/java/com/google/gerrit/acceptance/rest/PluginCollection.java
index c0daf93..dc40cde 100644
--- a/java/com/google/gerrit/acceptance/rest/PluginCollection.java
+++ b/java/com/google/gerrit/acceptance/rest/PluginCollection.java
@@ -45,7 +45,7 @@
@Override
public PluginResource parse(ConfigResource parent, IdString id)
throws ResourceNotFoundException, Exception {
- throw new ResourceNotFoundException();
+ throw new ResourceNotFoundException(id);
}
@Override
diff --git a/java/com/google/gerrit/common/data/GroupDetail.java b/java/com/google/gerrit/common/data/GroupDetail.java
deleted file mode 100644
index 991d5df..0000000
--- a/java/com/google/gerrit/common/data/GroupDetail.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright (C) 2008 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.common.data;
-
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.AccountGroup;
-import java.util.Set;
-
-public class GroupDetail {
- private Set<Account.Id> members;
- private Set<AccountGroup.UUID> includes;
-
- public GroupDetail(Set<Account.Id> members, Set<AccountGroup.UUID> includes) {
- this.members = members;
- this.includes = includes;
- }
-
- public Set<Account.Id> getMembers() {
- return members;
- }
-
- public Set<AccountGroup.UUID> getIncludes() {
- return includes;
- }
-}
diff --git a/java/com/google/gerrit/common/data/GroupInfo.java b/java/com/google/gerrit/common/data/GroupInfo.java
deleted file mode 100644
index 5c36104..0000000
--- a/java/com/google/gerrit/common/data/GroupInfo.java
+++ /dev/null
@@ -1,71 +0,0 @@
-// Copyright (C) 2011 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.common.data;
-
-import com.google.gerrit.entities.AccountGroup;
-
-/** Summary information about an {@link AccountGroup}, for simple tabular displays. */
-public class GroupInfo {
- protected AccountGroup.UUID uuid;
- protected String name;
- protected String description;
- protected String url;
-
- protected GroupInfo() {}
-
- /**
- * Create an anonymous group info, when only the id is known.
- *
- * <p>This constructor should only be a last-ditch effort, when the usual group lookup has failed
- * and a stale group id has been discovered in the data store.
- */
- public GroupInfo(AccountGroup.UUID uuid) {
- this.uuid = uuid;
- }
-
- /**
- * Create a group description from a real data store record.
- *
- * @param a the data store record holding the specific group details.
- */
- public GroupInfo(GroupDescription.Basic a) {
- uuid = a.getGroupUUID();
- name = a.getName();
- url = a.getUrl();
-
- if (a instanceof GroupDescription.Internal) {
- description = ((GroupDescription.Internal) a).getDescription();
- }
- }
-
- /** @return the unique local id of the group */
- public AccountGroup.UUID getId() {
- return uuid;
- }
-
- /** @return the name of the group; null if not supplied */
- public String getName() {
- return name;
- }
-
- /** @return the description of the group; null if not supplied */
- public String getDescription() {
- return description;
- }
-
- public String getUrl() {
- return url;
- }
-}
diff --git a/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java b/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java
index 522ec88..ed01a4d 100644
--- a/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java
@@ -60,7 +60,6 @@
public Boolean hideEmptyPane;
public Boolean matchBrackets;
public Boolean lineWrapping;
- public Theme theme;
public Whitespace ignoreWhitespace;
public Boolean retainHeader;
public Boolean skipDeleted;
@@ -88,7 +87,6 @@
i.hideEmptyPane = false;
i.matchBrackets = false;
i.lineWrapping = false;
- i.theme = Theme.DEFAULT;
i.ignoreWhitespace = Whitespace.IGNORE_NONE;
i.retainHeader = false;
i.skipDeleted = false;
diff --git a/java/com/google/gerrit/extensions/client/EditPreferencesInfo.java b/java/com/google/gerrit/extensions/client/EditPreferencesInfo.java
index 7ab22e1..6672cb1 100644
--- a/java/com/google/gerrit/extensions/client/EditPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/EditPreferencesInfo.java
@@ -30,8 +30,6 @@
public Boolean indentWithTabs;
public Boolean autoCloseBrackets;
public Boolean showBase;
- public Theme theme;
- public KeyMapType keyMapType;
public static EditPreferencesInfo defaults() {
EditPreferencesInfo i = new EditPreferencesInfo();
@@ -49,8 +47,6 @@
i.indentWithTabs = false;
i.autoCloseBrackets = false;
i.showBase = false;
- i.theme = Theme.DEFAULT;
- i.keyMapType = KeyMapType.DEFAULT;
return i;
}
}
diff --git a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
index 458bcf5..212f6da 100644
--- a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
@@ -124,8 +124,6 @@
public Integer changesPerPage;
/** Type of download URL the user prefers to use. */
public String downloadScheme;
- /** Type of download command the user prefers to use. */
- public DownloadCommand downloadCommand;
public DateFormat dateFormat;
public TimeFormat timeFormat;
@@ -184,7 +182,6 @@
GeneralPreferencesInfo p = new GeneralPreferencesInfo();
p.changesPerPage = DEFAULT_PAGESIZE;
p.downloadScheme = null;
- p.downloadCommand = DownloadCommand.CHECKOUT;
p.dateFormat = DateFormat.STD;
p.timeFormat = TimeFormat.HHMM_12;
p.expandInlineDiffs = false;
diff --git a/java/com/google/gerrit/extensions/client/KeyMapType.java b/java/com/google/gerrit/extensions/client/KeyMapType.java
deleted file mode 100644
index 9c85ac7..0000000
--- a/java/com/google/gerrit/extensions/client/KeyMapType.java
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright (C) 2015 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.client;
-
-public enum KeyMapType {
- DEFAULT,
- EMACS,
- SUBLIME,
- VIM
-}
diff --git a/java/com/google/gerrit/extensions/client/Theme.java b/java/com/google/gerrit/extensions/client/Theme.java
deleted file mode 100644
index d7a5b80..0000000
--- a/java/com/google/gerrit/extensions/client/Theme.java
+++ /dev/null
@@ -1,123 +0,0 @@
-// Copyright (C) 2014 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.client;
-
-public enum Theme {
- // Light themes
- DEFAULT,
- DAY_3024,
- DUOTONE_LIGHT,
- BASE16_LIGHT,
- ECLIPSE,
- ELEGANT,
- MDN_LIKE,
- NEAT,
- NEO,
- PARAISO_LIGHT,
- SOLARIZED,
- TTCN,
- XQ_LIGHT,
- YETI,
-
- // Dark themes
- NIGHT_3024,
- ABCDEF,
- AMBIANCE,
- BASE16_DARK,
- BESPIN,
- BLACKBOARD,
- COBALT,
- COLORFORTH,
- DRACULA,
- DUOTONE_DARK,
- ERLANG_DARK,
- HOPSCOTCH,
- ICECODER,
- ISOTOPE,
- LESSER_DARK,
- LIQUIBYTE,
- MATERIAL,
- MBO,
- MIDNIGHT,
- MONOKAI,
- NIGHT,
- PARAISO_DARK,
- PASTEL_ON_DARK,
- RAILSCASTS,
- RUBYBLUE,
- SETI,
- THE_MATRIX,
- TOMORROW_NIGHT_BRIGHT,
- TOMORROW_NIGHT_EIGHTIES,
- TWILIGHT,
- VIBRANT_INK,
- XQ_DARK,
- ZENBURN;
-
- public boolean isDark() {
- switch (this) {
- case ABCDEF:
- case AMBIANCE:
- case BASE16_DARK:
- case BESPIN:
- case BLACKBOARD:
- case COBALT:
- case COLORFORTH:
- case DRACULA:
- case DUOTONE_DARK:
- case ERLANG_DARK:
- case HOPSCOTCH:
- case ICECODER:
- case ISOTOPE:
- case LESSER_DARK:
- case LIQUIBYTE:
- case MATERIAL:
- case MBO:
- case MIDNIGHT:
- case MONOKAI:
- case NIGHT:
- case NIGHT_3024:
- case PARAISO_DARK:
- case PASTEL_ON_DARK:
- case RAILSCASTS:
- case RUBYBLUE:
- case SETI:
- case THE_MATRIX:
- case TOMORROW_NIGHT_BRIGHT:
- case TOMORROW_NIGHT_EIGHTIES:
- case TWILIGHT:
- case VIBRANT_INK:
- case XQ_DARK:
- case ZENBURN:
- return true;
- case BASE16_LIGHT:
- case DEFAULT:
- case DAY_3024:
- case DUOTONE_LIGHT:
- case ECLIPSE:
- case ELEGANT:
- case MDN_LIKE:
- case NEAT:
- case NEO:
- case PARAISO_LIGHT:
- case SOLARIZED:
- case TTCN:
- case XQ_LIGHT:
- case YETI:
- default:
- return false;
- }
- }
-}
diff --git a/java/com/google/gerrit/extensions/common/AccountDetailInfo.java b/java/com/google/gerrit/extensions/common/AccountDetailInfo.java
index 3ffa97a..14027ac 100644
--- a/java/com/google/gerrit/extensions/common/AccountDetailInfo.java
+++ b/java/com/google/gerrit/extensions/common/AccountDetailInfo.java
@@ -16,6 +16,15 @@
import java.sql.Timestamp;
+/**
+ * Representation of a (detailed) account in the REST API.
+ *
+ * <p>This class determines the JSON format of (detailed) accounts in the REST API.
+ *
+ * <p>This class extends {@link AccountInfo} (which defines fields for account properties that are
+ * frequently used) and provides additional fields for account details which are needed only in some
+ * cases.
+ */
public class AccountDetailInfo extends AccountInfo {
public Timestamp registeredOn;
diff --git a/java/com/google/gerrit/extensions/common/AccountInfo.java b/java/com/google/gerrit/extensions/common/AccountInfo.java
index d1bbe88..3dcee71 100644
--- a/java/com/google/gerrit/extensions/common/AccountInfo.java
+++ b/java/com/google/gerrit/extensions/common/AccountInfo.java
@@ -18,6 +18,14 @@
import java.util.List;
import java.util.Objects;
+/**
+ * Representation of an account in the REST API.
+ *
+ * <p>This class determines the JSON format of accounts in the REST API.
+ *
+ * <p>This class defines fields for account properties that are frequently used. Additional fields
+ * are defined in {@link AccountDetailInfo}.
+ */
public class AccountInfo {
public Integer _accountId;
public String name;
diff --git a/java/com/google/gerrit/extensions/common/AccountsInfo.java b/java/com/google/gerrit/extensions/common/AccountsInfo.java
index e1c2825..a2a4826 100644
--- a/java/com/google/gerrit/extensions/common/AccountsInfo.java
+++ b/java/com/google/gerrit/extensions/common/AccountsInfo.java
@@ -14,6 +14,12 @@
package com.google.gerrit.extensions.common;
+/**
+ * Representation of account-related server configuration in the REST API.
+ *
+ * <p>This class determines the JSON format of account-related server configuration in the REST API.
+ */
public class AccountsInfo {
+ /** The value of the {@code accounts.visibility} parameter in {@code gerrit.config}. */
public AccountVisibility visibility;
}
diff --git a/java/com/google/gerrit/extensions/common/AgreementInfo.java b/java/com/google/gerrit/extensions/common/AgreementInfo.java
index 4242fcd..c092463 100644
--- a/java/com/google/gerrit/extensions/common/AgreementInfo.java
+++ b/java/com/google/gerrit/extensions/common/AgreementInfo.java
@@ -14,9 +14,25 @@
package com.google.gerrit.extensions.common;
+/**
+ * Representation of a contributor agreement in the REST API.
+ *
+ * <p>This class determines the JSON format of a contributor agreement in the REST API.
+ */
public class AgreementInfo {
+ /** The unique name of the contributor agreement. */
public String name;
+
+ /** The description of the contributor agreement. */
public String description;
+
+ /** The URL of the contributor agreement. */
public String url;
+
+ /**
+ * Group to which a user that signs the contributor agreement online is added automatically.
+ *
+ * <p>May be {@code null}. In this case users cannot sign the contributor agreement online.
+ */
public GroupInfo autoVerifyGroup;
}
diff --git a/java/com/google/gerrit/extensions/restapi/MethodNotAllowedException.java b/java/com/google/gerrit/extensions/restapi/MethodNotAllowedException.java
index 61c6345..2d59f27 100644
--- a/java/com/google/gerrit/extensions/restapi/MethodNotAllowedException.java
+++ b/java/com/google/gerrit/extensions/restapi/MethodNotAllowedException.java
@@ -18,10 +18,6 @@
public class MethodNotAllowedException extends RestApiException {
private static final long serialVersionUID = 1L;
- public MethodNotAllowedException() {
- super();
- }
-
/** @param msg error text for client describing why the method is not allowed. */
public MethodNotAllowedException(String msg) {
super(msg);
diff --git a/java/com/google/gerrit/extensions/restapi/PreconditionFailedException.java b/java/com/google/gerrit/extensions/restapi/PreconditionFailedException.java
index 1fa6cd0..b8c9d38 100644
--- a/java/com/google/gerrit/extensions/restapi/PreconditionFailedException.java
+++ b/java/com/google/gerrit/extensions/restapi/PreconditionFailedException.java
@@ -18,8 +18,6 @@
public class PreconditionFailedException extends RestApiException {
private static final long serialVersionUID = 1L;
- public PreconditionFailedException() {}
-
/** @param msg message to return to the client describing the error. */
public PreconditionFailedException(String msg) {
super(msg);
diff --git a/java/com/google/gerrit/extensions/restapi/RestApiException.java b/java/com/google/gerrit/extensions/restapi/RestApiException.java
index f3d7dec..de141be 100644
--- a/java/com/google/gerrit/extensions/restapi/RestApiException.java
+++ b/java/com/google/gerrit/extensions/restapi/RestApiException.java
@@ -21,13 +21,17 @@
private static final long serialVersionUID = 1L;
private CacheControl caching = CacheControl.NONE;
- public RestApiException() {}
+ public static RestApiException wrap(String msg, Exception e) {
+ return new RestApiException(msg, e);
+ }
- public RestApiException(String msg) {
+ protected RestApiException() {}
+
+ protected RestApiException(String msg) {
super(msg);
}
- public RestApiException(String msg, Throwable cause) {
+ protected RestApiException(String msg, Throwable cause) {
super(msg, cause);
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index f390ebc..de39472 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1237,7 +1237,7 @@
return obj;
}
}
- throw new MethodNotAllowedException();
+ throw new MethodNotAllowedException("raw input not supported");
}
private Object parseString(String value, Type type)
diff --git a/java/com/google/gerrit/index/project/IndexedProjectQuery.java b/java/com/google/gerrit/index/project/IndexedProjectQuery.java
index cdfeabd..5fc74ca 100644
--- a/java/com/google/gerrit/index/project/IndexedProjectQuery.java
+++ b/java/com/google/gerrit/index/project/IndexedProjectQuery.java
@@ -22,6 +22,10 @@
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
+/**
+ * Wrapper around {@link Predicate}s that are returned by the {@link
+ * com.google.gerrit.index.IndexRewriter}. See {@link IndexedQuery}.
+ */
public class IndexedProjectQuery extends IndexedQuery<Project.NameKey, ProjectData>
implements DataSource<ProjectData> {
diff --git a/java/com/google/gerrit/index/project/ProjectData.java b/java/com/google/gerrit/index/project/ProjectData.java
index 2bf9a4b5..c70b823 100644
--- a/java/com/google/gerrit/index/project/ProjectData.java
+++ b/java/com/google/gerrit/index/project/ProjectData.java
@@ -23,6 +23,11 @@
import java.util.List;
import java.util.Optional;
+/**
+ * Representation of a Gerrit project in the project index.
+ *
+ * <p>Includes information about all parent projects.
+ */
public class ProjectData {
private final Project project;
private final Optional<ProjectData> parent;
diff --git a/java/com/google/gerrit/index/project/ProjectIndex.java b/java/com/google/gerrit/index/project/ProjectIndex.java
index 3e99d55..b2ddaff 100644
--- a/java/com/google/gerrit/index/project/ProjectIndex.java
+++ b/java/com/google/gerrit/index/project/ProjectIndex.java
@@ -19,6 +19,10 @@
import com.google.gerrit.index.IndexDefinition;
import com.google.gerrit.index.query.Predicate;
+/**
+ * Index for Gerrit projects (repositories). This class is mainly used for typing the generic parent
+ * class that contains actual implementations.
+ */
public interface ProjectIndex extends Index<Project.NameKey, ProjectData> {
public interface Factory
diff --git a/java/com/google/gerrit/index/project/ProjectIndexCollection.java b/java/com/google/gerrit/index/project/ProjectIndexCollection.java
index 30227a3..7ff23c5 100644
--- a/java/com/google/gerrit/index/project/ProjectIndexCollection.java
+++ b/java/com/google/gerrit/index/project/ProjectIndexCollection.java
@@ -19,6 +19,7 @@
import com.google.gerrit.index.IndexCollection;
import com.google.inject.Singleton;
+/** Collection of active project indices. See {@link IndexCollection} for details on collections. */
@Singleton
public class ProjectIndexCollection
extends IndexCollection<Project.NameKey, ProjectData, ProjectIndex> {
diff --git a/java/com/google/gerrit/index/project/ProjectIndexRewriter.java b/java/com/google/gerrit/index/project/ProjectIndexRewriter.java
index 9e2bbdc..230161b 100644
--- a/java/com/google/gerrit/index/project/ProjectIndexRewriter.java
+++ b/java/com/google/gerrit/index/project/ProjectIndexRewriter.java
@@ -23,6 +23,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
+/** Rewriter for the project index. See {@link IndexRewriter} for details. */
@Singleton
public class ProjectIndexRewriter implements IndexRewriter<ProjectData> {
private final ProjectIndexCollection indexes;
diff --git a/java/com/google/gerrit/index/project/ProjectIndexer.java b/java/com/google/gerrit/index/project/ProjectIndexer.java
index bd5efeb2..34e8075 100644
--- a/java/com/google/gerrit/index/project/ProjectIndexer.java
+++ b/java/com/google/gerrit/index/project/ProjectIndexer.java
@@ -16,6 +16,7 @@
import com.google.gerrit.entities.Project;
+/** Interface for indexing a Gerrit project. */
public interface ProjectIndexer {
/**
diff --git a/java/com/google/gerrit/index/project/ProjectPredicate.java b/java/com/google/gerrit/index/project/ProjectPredicate.java
index 4926eef..11875ef 100644
--- a/java/com/google/gerrit/index/project/ProjectPredicate.java
+++ b/java/com/google/gerrit/index/project/ProjectPredicate.java
@@ -17,6 +17,7 @@
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.query.IndexPredicate;
+/** Predicate that is mapped to a field in the project index. */
public class ProjectPredicate extends IndexPredicate<ProjectData> {
public ProjectPredicate(FieldDef<ProjectData, ?> def, String value) {
super(def, value);
diff --git a/java/com/google/gerrit/index/query/IndexPredicate.java b/java/com/google/gerrit/index/query/IndexPredicate.java
index 7811a32..aac6682 100644
--- a/java/com/google/gerrit/index/query/IndexPredicate.java
+++ b/java/com/google/gerrit/index/query/IndexPredicate.java
@@ -17,7 +17,7 @@
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.FieldType;
-/** Index-aware predicate that includes a field type annotation. */
+/** Predicate that is mapped to a field in the index. */
public abstract class IndexPredicate<I> extends OperatorPredicate<I> {
private final FieldDef<I, ?> def;
diff --git a/java/com/google/gerrit/server/DynamicOptions.java b/java/com/google/gerrit/server/DynamicOptions.java
index 44d3493..41dc082 100644
--- a/java/com/google/gerrit/server/DynamicOptions.java
+++ b/java/com/google/gerrit/server/DynamicOptions.java
@@ -80,7 +80,7 @@
* .to(MyOptionsClassNameProvider.class);
*
* static class MyOptionsClassNameProvider implements DynamicOptions.ClassNameProvider {
- * @Override
+ * {@literal @}Override
* public String getClassName() {
* return "com.googlesource.gerrit.plugins.myplugin.CommandOptions";
* }
@@ -107,11 +107,11 @@
* .to(MyOptionsModulesClassNamesProvider.class);
*
* static class MyOptionsModulesClassNamesProvider implements DynamicOptions.ClassNameProvider {
- * @Override
+ * {@literal @}Override
* public String getClassName() {
* return "com.googlesource.gerrit.plugins.myplugin.CommandOptions";
* }
- * @Override
+ * {@literal @}Override
* public Iterable<String> getModulesClassNames()() {
* return "com.googlesource.gerrit.plugins.myplugin.MyOptionsModule";
* }
diff --git a/java/com/google/gerrit/server/account/GroupControl.java b/java/com/google/gerrit/server/account/GroupControl.java
index 2228525..64fd7c6 100644
--- a/java/com/google/gerrit/server/account/GroupControl.java
+++ b/java/com/google/gerrit/server/account/GroupControl.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.account;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
@@ -29,6 +30,7 @@
/** Access control management for a group of accounts managed in Gerrit. */
public class GroupControl {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Singleton
public static class GenericFactory {
@@ -111,15 +113,46 @@
/** Can this user see this group exists? */
public boolean isVisible() {
- /* Check for canAdministrateServer may seem redundant, but allows
- * for visibility of all groups that are not an internal group to
- * server administrators.
- */
- return user.isInternalUser()
- || groupBackend.isVisibleToAll(group.getGroupUUID())
- || user.getEffectiveGroups().contains(group.getGroupUUID())
- || isOwner()
- || canAdministrateServer();
+ if (user.isInternalUser()) {
+ logger.atFine().log(
+ "group %s is visible to internal user %s",
+ group.getGroupUUID().get(), user.getLoggableName());
+ return true;
+ }
+
+ if (groupBackend.isVisibleToAll(group.getGroupUUID())) {
+ logger.atFine().log(
+ "group %s is visible to user %s (group is visible to all users)",
+ group.getGroupUUID().get(), user.getLoggableName());
+ return true;
+ }
+
+ if (user.getEffectiveGroups().contains(group.getGroupUUID())) {
+ logger.atFine().log(
+ "group %s is visible to user %s (user is member of the group)",
+ group.getGroupUUID().get(), user.getLoggableName());
+ return true;
+ }
+
+ if (isOwner()) {
+ logger.atFine().log(
+ "group %s is visible to user %s (user is owner of the group)",
+ group.getGroupUUID().get(), user.getLoggableName());
+ return true;
+ }
+
+ // The check for canAdministrateServer may seem redundant, but it's needed to make external
+ // groups visible to server administrators.
+ if (canAdministrateServer()) {
+ logger.atFine().log(
+ "group %s is visible to user %s (user is admin)",
+ group.getGroupUUID().get(), user.getLoggableName());
+ return true;
+ }
+
+ logger.atFine().log(
+ "group %s is not visible to user %s", group.getGroupUUID().get(), user.getLoggableName());
+ return false;
}
public boolean isOwner() {
@@ -127,11 +160,28 @@
return isOwner;
}
- // Keep this logic in sync with VisibleRefFilter#isOwner(...).
+ // Keep this logic in sync with DefaultRefFilter#isGroupOwner(...).
if (group instanceof GroupDescription.Internal) {
AccountGroup.UUID ownerUUID = ((GroupDescription.Internal) group).getOwnerGroupUUID();
- isOwner = getUser().getEffectiveGroups().contains(ownerUUID) || canAdministrateServer();
+ if (getUser().getEffectiveGroups().contains(ownerUUID)) {
+ logger.atFine().log(
+ "user %s is owner of group %s", user.getLoggableName(), group.getGroupUUID().get());
+ isOwner = true;
+ } else if (canAdministrateServer()) {
+ logger.atFine().log(
+ "user %s is owner of group %s (user is admin)",
+ user.getLoggableName(), group.getGroupUUID().get());
+ isOwner = true;
+ } else {
+ logger.atFine().log(
+ "user %s is not an owner of group %s",
+ user.getLoggableName(), group.getGroupUUID().get());
+ isOwner = false;
+ }
} else {
+ logger.atFine().log(
+ "user %s is not an owner of external group %s",
+ user.getLoggableName(), group.getGroupUUID().get());
isOwner = false;
}
return isOwner;
@@ -141,7 +191,12 @@
try {
perm.check(GlobalPermission.ADMINISTRATE_SERVER);
return true;
- } catch (AuthException | PermissionBackendException denied) {
+ } catch (AuthException e) {
+ return false;
+ } catch (PermissionBackendException e) {
+ logger.atFine().log(
+ "Failed to check %s global capability for user %s",
+ GlobalPermission.ADMINISTRATE_SERVER, user.getLoggableName());
return false;
}
}
diff --git a/java/com/google/gerrit/server/account/Preferences.java b/java/com/google/gerrit/server/account/Preferences.java
index c15e6b0..ece610b 100644
--- a/java/com/google/gerrit/server/account/Preferences.java
+++ b/java/com/google/gerrit/server/account/Preferences.java
@@ -23,13 +23,10 @@
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat;
-import com.google.gerrit.extensions.client.KeyMapType;
import com.google.gerrit.extensions.client.MenuItem;
-import com.google.gerrit.extensions.client.Theme;
import java.util.Optional;
@AutoValue
@@ -40,8 +37,6 @@
public abstract Optional<String> downloadScheme();
- public abstract Optional<DownloadCommand> downloadCommand();
-
public abstract Optional<DateFormat> dateFormat();
public abstract Optional<TimeFormat> timeFormat();
@@ -82,8 +77,6 @@
abstract Builder downloadScheme(@Nullable String val);
- abstract Builder downloadCommand(@Nullable DownloadCommand val);
-
abstract Builder dateFormat(@Nullable DateFormat val);
abstract Builder timeFormat(@Nullable TimeFormat val);
@@ -125,7 +118,6 @@
return (new AutoValue_Preferences_General.Builder())
.changesPerPage(info.changesPerPage)
.downloadScheme(info.downloadScheme)
- .downloadCommand(info.downloadCommand)
.dateFormat(info.dateFormat)
.timeFormat(info.timeFormat)
.expandInlineDiffs(info.expandInlineDiffs)
@@ -150,7 +142,6 @@
GeneralPreferencesInfo info = new GeneralPreferencesInfo();
info.changesPerPage = changesPerPage().orElse(null);
info.downloadScheme = downloadScheme().orElse(null);
- info.downloadCommand = downloadCommand().orElse(null);
info.dateFormat = dateFormat().orElse(null);
info.timeFormat = timeFormat().orElse(null);
info.expandInlineDiffs = expandInlineDiffs().orElse(null);
@@ -202,10 +193,6 @@
public abstract Optional<Boolean> showBase();
- public abstract Optional<Theme> theme();
-
- public abstract Optional<KeyMapType> keyMapType();
-
@AutoValue.Builder
public abstract static class Builder {
abstract Builder tabSize(@Nullable Integer val);
@@ -236,10 +223,6 @@
abstract Builder showBase(@Nullable Boolean val);
- abstract Builder theme(@Nullable Theme val);
-
- abstract Builder keyMapType(@Nullable KeyMapType val);
-
abstract Edit build();
}
@@ -259,8 +242,6 @@
.indentWithTabs(info.indentWithTabs)
.autoCloseBrackets(info.autoCloseBrackets)
.showBase(info.showBase)
- .theme(info.theme)
- .keyMapType(info.keyMapType)
.build();
}
@@ -280,8 +261,6 @@
info.indentWithTabs = indentWithTabs().orElse(null);
info.autoCloseBrackets = autoCloseBrackets().orElse(null);
info.showBase = showBase().orElse(null);
- info.theme = theme().orElse(null);
- info.keyMapType = keyMapType().orElse(null);
return info;
}
}
@@ -326,8 +305,6 @@
public abstract Optional<Boolean> lineWrapping();
- public abstract Optional<Theme> theme();
-
public abstract Optional<Whitespace> ignoreWhitespace();
public abstract Optional<Boolean> retainHeader();
@@ -378,8 +355,6 @@
abstract Builder lineWrapping(@Nullable Boolean val);
- abstract Builder theme(@Nullable Theme val);
-
abstract Builder ignoreWhitespace(@Nullable Whitespace val);
abstract Builder retainHeader(@Nullable Boolean val);
@@ -414,7 +389,6 @@
.hideEmptyPane(info.hideEmptyPane)
.matchBrackets(info.matchBrackets)
.lineWrapping(info.lineWrapping)
- .theme(info.theme)
.ignoreWhitespace(info.ignoreWhitespace)
.retainHeader(info.retainHeader)
.skipDeleted(info.skipDeleted)
@@ -444,7 +418,6 @@
info.hideEmptyPane = hideEmptyPane().orElse(null);
info.matchBrackets = matchBrackets().orElse(null);
info.lineWrapping = lineWrapping().orElse(null);
- info.theme = theme().orElse(null);
info.ignoreWhitespace = ignoreWhitespace().orElse(null);
info.retainHeader = retainHeader().orElse(null);
info.skipDeleted = skipDeleted().orElse(null);
diff --git a/java/com/google/gerrit/server/api/ApiUtil.java b/java/com/google/gerrit/server/api/ApiUtil.java
index c5b8b12..8a8e37c 100644
--- a/java/com/google/gerrit/server/api/ApiUtil.java
+++ b/java/com/google/gerrit/server/api/ApiUtil.java
@@ -32,7 +32,7 @@
public static RestApiException asRestApiException(String msg, Exception e)
throws RuntimeException {
Throwables.throwIfUnchecked(e);
- return e instanceof RestApiException ? (RestApiException) e : new RestApiException(msg, e);
+ return e instanceof RestApiException ? (RestApiException) e : RestApiException.wrap(msg, e);
}
private ApiUtil() {}
diff --git a/java/com/google/gerrit/server/api/plugins/PluginApiImpl.java b/java/com/google/gerrit/server/api/plugins/PluginApiImpl.java
index 3932177..59c396a 100644
--- a/java/com/google/gerrit/server/api/plugins/PluginApiImpl.java
+++ b/java/com/google/gerrit/server/api/plugins/PluginApiImpl.java
@@ -69,7 +69,11 @@
@Override
public void disable() throws RestApiException {
- disable.apply(resource, new Input());
+ try {
+ disable.apply(resource, new Input());
+ } catch (Exception e) {
+ throw asRestApiException("Cannot disable plugin", e);
+ }
}
@Override
diff --git a/java/com/google/gerrit/server/api/projects/ProjectsImpl.java b/java/com/google/gerrit/server/api/projects/ProjectsImpl.java
index 5d25d1a..f311b35 100644
--- a/java/com/google/gerrit/server/api/projects/ProjectsImpl.java
+++ b/java/com/google/gerrit/server/api/projects/ProjectsImpl.java
@@ -156,7 +156,7 @@
.withStart(r.getStart())
.apply();
} catch (StorageException e) {
- throw new RestApiException("Cannot query projects", e);
+ throw asRestApiException("Cannot query projects", e);
}
}
}
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 4980c4a..770f36c 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -566,10 +566,16 @@
reviewerInputs.stream(),
Streams.stream(
newAddReviewerInputFromCommitIdentity(
- change, patchSetInfo.getAuthor().getAccount(), NotifyHandling.NONE)),
+ change,
+ patchSetInfo.getCommitId(),
+ patchSetInfo.getAuthor().getAccount(),
+ NotifyHandling.NONE)),
Streams.stream(
newAddReviewerInputFromCommitIdentity(
- change, patchSetInfo.getCommitter().getAccount(), NotifyHandling.NONE)))
+ change,
+ patchSetInfo.getCommitId(),
+ patchSetInfo.getCommitter().getAccount(),
+ NotifyHandling.NONE)))
.collect(toImmutableList());
}
}
diff --git a/java/com/google/gerrit/server/change/ChangeMessages.java b/java/com/google/gerrit/server/change/ChangeMessages.java
index 6cd3726..6f2e1ef 100644
--- a/java/com/google/gerrit/server/change/ChangeMessages.java
+++ b/java/com/google/gerrit/server/change/ChangeMessages.java
@@ -23,6 +23,9 @@
}
public String revertChangeDefaultMessage;
+ public String revertSubmissionDefaultMessage;
+ public String revertSubmissionUserMessage;
+ public String revertSubmissionOfRevertSubmissionUserMessage;
public String reviewerCantSeeChange;
public String reviewerInvalid;
diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java
index 0f03c88..61616c0 100644
--- a/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -225,7 +225,9 @@
problem("Missing change owner: " + change().getOwner());
}
} catch (IOException | ConfigInvalidException e) {
- error("Failed to look up owner", e);
+ ProblemInfo problem = problem("Failed to look up owner");
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s: %s", notes.getChangeId(), problem);
}
}
@@ -237,7 +239,9 @@
String.format("Current patch set %d not found", change().currentPatchSetId().get()));
}
} catch (StorageException e) {
- error("Failed to look up current patch set", e);
+ ProblemInfo problem = problem("Failed to look up current patch set");
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s: %s", notes.getChangeId(), problem);
}
}
@@ -249,9 +253,15 @@
rw = new RevWalk(oi.newReader());
return true;
} catch (RepositoryNotFoundException e) {
- return error("Destination repository not found: " + project, e);
+ ProblemInfo problem = problem("Destination repository not found: " + project);
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s: %s", notes.getChangeId(), problem);
+ return false;
} catch (IOException e) {
- return error("Failed to open repository: " + project, e);
+ ProblemInfo problem = problem("Failed to open repository: " + project);
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s: %s", notes.getChangeId(), problem);
+ return false;
}
}
@@ -261,7 +271,10 @@
// Iterate in descending order.
all = PS_ID_ORDER.sortedCopy(psUtil.byChange(notes));
} catch (StorageException e) {
- return error("Failed to look up patch sets", e);
+ ProblemInfo problem = problem("Failed to look up patch sets");
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s: %s", notes.getChangeId(), problem);
+ return false;
}
patchSetsBySha = MultimapBuilder.hashKeys(all.size()).treeSetValues(PS_ID_ORDER).build();
@@ -271,7 +284,9 @@
repo.getRefDatabase()
.exactRef(all.stream().map(ps -> ps.id().toRefName()).toArray(String[]::new));
} catch (IOException e) {
- error("error reading refs", e);
+ ProblemInfo problem = problem("Error reading refs");
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s: %s", notes.getChangeId(), problem);
refs = Collections.emptyMap();
}
@@ -430,7 +445,8 @@
continue;
}
} catch (StorageException e) {
- warn(e);
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s", notes.getChangeId());
// Include this patch set; should cause an error below, which is good.
}
thisCommitPsIds.add(psId);
@@ -480,7 +496,10 @@
break;
}
} catch (IOException e) {
- error("Error looking up expected merged commit " + fix.expectMergedAs, e);
+ ProblemInfo problem =
+ problem("Error looking up expected merged commit " + fix.expectMergedAs);
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s: %s", notes.getChangeId(), problem);
}
}
@@ -561,7 +580,8 @@
insertPatchSetProblem.status = Status.FIXED;
insertPatchSetProblem.outcome = "Inserted as patch set " + psId.get();
} catch (StorageException | IOException | UpdateException | RestApiException e) {
- warn(e);
+ logger.atWarning().withCause(e).log(
+ "Error in consistency check of change %s", notes.getChangeId());
for (ProblemInfo pi : currProblems) {
pi.status = Status.FIX_FAILED;
pi.outcome = "Error inserting merged patch set";
@@ -763,18 +783,6 @@
return problems.get(problems.size() - 1);
}
- private boolean error(String msg, Throwable t) {
- problem(msg);
- // TODO(dborowitz): Expose stack trace to administrators.
- warn(t);
- return false;
- }
-
- private void warn(Throwable t) {
- logger.atWarning().withCause(t).log(
- "Error in consistency check of change %s", notes.getChangeId());
- }
-
private Result result() {
return Result.create(notes, problems);
}
diff --git a/java/com/google/gerrit/server/change/ReviewerAdder.java b/java/com/google/gerrit/server/change/ReviewerAdder.java
index ba6ba21..4f66492 100644
--- a/java/com/google/gerrit/server/change/ReviewerAdder.java
+++ b/java/com/google/gerrit/server/change/ReviewerAdder.java
@@ -81,6 +81,7 @@
import java.util.function.Function;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
public class ReviewerAdder {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -125,12 +126,16 @@
}
public static Optional<InternalAddReviewerInput> newAddReviewerInputFromCommitIdentity(
- Change change, @Nullable Account.Id accountId, NotifyHandling notify) {
+ Change change, ObjectId commitId, @Nullable Account.Id accountId, NotifyHandling notify) {
if (accountId == null || accountId.equals(change.getOwner())) {
// If git ident couldn't be resolved to a user, or if it's not forged, do nothing.
return Optional.empty();
}
+ logger.atFine().log(
+ "Adding account %d from author/committer identity of commit %s as reviewer to change %d",
+ accountId.get(), commitId.name(), change.getChangeId());
+
InternalAddReviewerInput in = new InternalAddReviewerInput();
in.reviewer = accountId.toString();
in.state = REVIEWER;
diff --git a/java/com/google/gerrit/server/extensions/events/AbstractChangeEvent.java b/java/com/google/gerrit/server/extensions/events/AbstractChangeEvent.java
index a5800fb2..b7ee043 100644
--- a/java/com/google/gerrit/server/extensions/events/AbstractChangeEvent.java
+++ b/java/com/google/gerrit/server/extensions/events/AbstractChangeEvent.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.events.ChangeEvent;
import java.sql.Timestamp;
+/** Base class for all change events. */
public abstract class AbstractChangeEvent implements ChangeEvent {
private final ChangeInfo changeInfo;
private final AccountInfo who;
diff --git a/java/com/google/gerrit/server/extensions/events/AbstractRevisionEvent.java b/java/com/google/gerrit/server/extensions/events/AbstractRevisionEvent.java
index 6b72b5e..9d4d299 100644
--- a/java/com/google/gerrit/server/extensions/events/AbstractRevisionEvent.java
+++ b/java/com/google/gerrit/server/extensions/events/AbstractRevisionEvent.java
@@ -21,6 +21,7 @@
import com.google.gerrit.extensions.events.RevisionEvent;
import java.sql.Timestamp;
+/** Base class for all revision events. */
public abstract class AbstractRevisionEvent extends AbstractChangeEvent implements RevisionEvent {
private final RevisionInfo revisionInfo;
diff --git a/java/com/google/gerrit/server/extensions/events/AgreementSignup.java b/java/com/google/gerrit/server/extensions/events/AgreementSignup.java
index b692cf5..47fdd0e 100644
--- a/java/com/google/gerrit/server/extensions/events/AgreementSignup.java
+++ b/java/com/google/gerrit/server/extensions/events/AgreementSignup.java
@@ -21,6 +21,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
+/** Helper class to fire an event when a user has signed a contributor agreement. */
@Singleton
public class AgreementSignup {
private final PluginSetContext<AgreementSignupListener> listeners;
@@ -40,6 +41,7 @@
listeners.runEach(l -> l.onAgreementSignup(event));
}
+ /** Event to be fired when a user has signed a contributor agreement. */
private static class Event extends AbstractNoNotifyEvent
implements AgreementSignupListener.Event {
private final AccountInfo account;
diff --git a/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java b/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java
index a76b69b..2189690 100644
--- a/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java
+++ b/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java
@@ -27,6 +27,7 @@
import com.google.inject.Singleton;
import java.sql.Timestamp;
+/** Helper class to fire an event when a user has been set as assignee on a change. */
@Singleton
public class AssigneeChanged {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -58,6 +59,7 @@
}
}
+ /** Event to be fired when a user has been set as assignee on a change. */
private static class Event extends AbstractChangeEvent implements AssigneeChangedListener.Event {
private final AccountInfo oldAssignee;
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java b/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
index b27ffb9..c7a9283 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.sql.Timestamp;
+/** Helper class to fire an event when a change has been abandoned. */
@Singleton
public class ChangeAbandoned {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -78,6 +79,7 @@
}
}
+ /** Event to be fired when a change has been abandoned. */
private static class Event extends AbstractRevisionEvent
implements ChangeAbandonedListener.Event {
private final String reason;
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java b/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java
index df71f27..1ed6209 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java
@@ -27,6 +27,7 @@
import com.google.inject.Singleton;
import java.sql.Timestamp;
+/** Helper class to fire an event when a change has been deleted. */
@Singleton
public class ChangeDeleted {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -52,6 +53,7 @@
}
}
+ /** Event to be fired when a change has been deleted. */
private static class Event extends AbstractChangeEvent implements ChangeDeletedListener.Event {
Event(ChangeInfo change, AccountInfo deleter, Timestamp when) {
super(change, deleter, when, NotifyHandling.ALL);
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeMerged.java b/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
index add1c51..06d0008 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.sql.Timestamp;
+/** Helper class to fire an event when a change has been merged. */
@Singleton
public class ChangeMerged {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -72,6 +73,7 @@
}
}
+ /** Event to be fired when a change has been merged. */
private static class Event extends AbstractRevisionEvent implements ChangeMergedListener.Event {
private final String newRevisionId;
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeRestored.java b/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
index fa91dbd..1af56d0 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.sql.Timestamp;
+/** Helper class to fire an event when a change has been restored. */
@Singleton
public class ChangeRestored {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -72,6 +73,7 @@
}
}
+ /** Event to be fired when a change has been restored. */
private static class Event extends AbstractRevisionEvent implements ChangeRestoredListener.Event {
private String reason;
diff --git a/java/com/google/gerrit/server/extensions/events/ChangeReverted.java b/java/com/google/gerrit/server/extensions/events/ChangeReverted.java
index 5fb004f..d608c52 100644
--- a/java/com/google/gerrit/server/extensions/events/ChangeReverted.java
+++ b/java/com/google/gerrit/server/extensions/events/ChangeReverted.java
@@ -25,6 +25,7 @@
import com.google.inject.Singleton;
import java.sql.Timestamp;
+/** Helper class to fire an event when a change has been reverted. */
@Singleton
public class ChangeReverted {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -50,6 +51,7 @@
}
}
+ /** Event to be fired when a change has been reverted. */
private static class Event extends AbstractChangeEvent implements ChangeRevertedListener.Event {
private final ChangeInfo revertChange;
diff --git a/java/com/google/gerrit/server/extensions/events/CommentAdded.java b/java/com/google/gerrit/server/extensions/events/CommentAdded.java
index e45b206..151298c 100644
--- a/java/com/google/gerrit/server/extensions/events/CommentAdded.java
+++ b/java/com/google/gerrit/server/extensions/events/CommentAdded.java
@@ -36,6 +36,7 @@
import java.sql.Timestamp;
import java.util.Map;
+/** Helper class to fire an event when a comment or vote has been added to a change. */
@Singleton
public class CommentAdded {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -82,6 +83,7 @@
}
}
+ /** Event to be fired when a comment or vote has been added to a change. */
private static class Event extends AbstractRevisionEvent implements CommentAddedListener.Event {
private final String comment;
diff --git a/java/com/google/gerrit/server/extensions/events/EventUtil.java b/java/com/google/gerrit/server/extensions/events/EventUtil.java
index 3dcf3b8..a35140a 100644
--- a/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -46,8 +46,8 @@
/**
* Formats change and revision info objects to serve as payload for Gerrit events.
*
- * <p>Uses configurable options ({@code event.payload.listChangeOptions}) to decide which fields to
- * populate.
+ * <p>Uses configurable options ({@code event.payload.listChangeOptions}) to decide which change
+ * fields to populate.
*/
@Singleton
public class EventUtil {
diff --git a/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java b/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
index 99f105e..6ed0a08 100644
--- a/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
+++ b/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
@@ -27,6 +27,7 @@
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.transport.ReceiveCommand;
+/** Helper class to fire an event when a Git reference has been updated. */
@Singleton
public class GitReferenceUpdated {
public static final GitReferenceUpdated DISABLED =
@@ -153,6 +154,7 @@
listeners.runEach(l -> l.onGitReferenceUpdated(event));
}
+ /** Event to be fired when a Git reference has been updated. */
public static class Event implements GitReferenceUpdatedListener.Event {
private final String projectName;
private final String ref;
diff --git a/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java b/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java
index df60ec0..5d9c5c2 100644
--- a/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java
+++ b/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java
@@ -30,6 +30,7 @@
import java.util.Collection;
import java.util.Set;
+/** Helper class to fire an event when the hashtags of a change has been edited. */
@Singleton
public class HashtagsEdited {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -63,6 +64,7 @@
}
}
+ /** Event to be fired when the hashtags of a change has been edited. */
private static class Event extends AbstractChangeEvent implements HashtagsEditedListener.Event {
private Collection<String> updatedHashtags;
diff --git a/java/com/google/gerrit/server/extensions/events/PluginEvent.java b/java/com/google/gerrit/server/extensions/events/PluginEvent.java
index 60d27c9..6723588 100644
--- a/java/com/google/gerrit/server/extensions/events/PluginEvent.java
+++ b/java/com/google/gerrit/server/extensions/events/PluginEvent.java
@@ -14,12 +14,15 @@
package com.google.gerrit.server.extensions.events;
+import com.google.gerrit.common.UsedAt;
import com.google.gerrit.extensions.events.PluginEventListener;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+/** Helper class to let plugins fire a plugin-specific event. */
@Singleton
+@UsedAt(UsedAt.Project.PLUGINS_ALL)
public class PluginEvent {
private final PluginSetContext<PluginEventListener> listeners;
@@ -36,6 +39,7 @@
listeners.runEach(l -> l.onPluginEvent(e));
}
+ /** Event to be fired by plugins. */
private static class Event extends AbstractNoNotifyEvent implements PluginEventListener.Event {
private final String pluginName;
private final String type;
diff --git a/java/com/google/gerrit/server/extensions/events/PrivateStateChanged.java b/java/com/google/gerrit/server/extensions/events/PrivateStateChanged.java
index 72adff7..bcc6b8e 100644
--- a/java/com/google/gerrit/server/extensions/events/PrivateStateChanged.java
+++ b/java/com/google/gerrit/server/extensions/events/PrivateStateChanged.java
@@ -33,6 +33,7 @@
import java.io.IOException;
import java.sql.Timestamp;
+/** Helper class to fire an event when the private flag of a change has been toggled. */
@Singleton
public class PrivateStateChanged {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -67,6 +68,7 @@
}
}
+ /** Event to be fired when the private flag of a change has been toggled. */
private static class Event extends AbstractRevisionEvent
implements PrivateStateChangedListener.Event {
diff --git a/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java b/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
index 1af428cb..35e7828 100644
--- a/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
+++ b/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
@@ -36,6 +36,7 @@
import java.sql.Timestamp;
import java.util.List;
+/** Helper class to fire an event when reviewers have been added to a change. */
@Singleton
public class ReviewerAdded {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -79,6 +80,7 @@
}
}
+ /** Event to be fired when reviewers have been added to a change. */
private static class Event extends AbstractRevisionEvent implements ReviewerAddedListener.Event {
private final List<AccountInfo> reviewers;
diff --git a/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java b/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
index 61632f2..147f980 100644
--- a/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
+++ b/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
@@ -36,6 +36,7 @@
import java.sql.Timestamp;
import java.util.Map;
+/** Helper class to fire an event when a reviewer has been deleted from a change. */
@Singleton
public class ReviewerDeleted {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -86,6 +87,7 @@
}
}
+ /** Event to be fired when a reviewer has been deleted from a change. */
private static class Event extends AbstractRevisionEvent
implements ReviewerDeletedListener.Event {
private final AccountInfo reviewer;
diff --git a/java/com/google/gerrit/server/extensions/events/RevisionCreated.java b/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
index bdfa8c1..8179e9a 100644
--- a/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
+++ b/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
@@ -35,6 +35,7 @@
import java.io.IOException;
import java.sql.Timestamp;
+/** Helper class to fire an event when a revision has been created for a change. */
@Singleton
public class RevisionCreated {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -93,6 +94,7 @@
}
}
+ /** Event to be fired when a revision has been created for a change. */
private static class Event extends AbstractRevisionEvent
implements RevisionCreatedListener.Event {
diff --git a/java/com/google/gerrit/server/extensions/events/TopicEdited.java b/java/com/google/gerrit/server/extensions/events/TopicEdited.java
index cb982a1..e4089b1 100644
--- a/java/com/google/gerrit/server/extensions/events/TopicEdited.java
+++ b/java/com/google/gerrit/server/extensions/events/TopicEdited.java
@@ -27,6 +27,7 @@
import com.google.inject.Singleton;
import java.sql.Timestamp;
+/** Helper class to fire an event when the topic of a change has been edited. */
@Singleton
public class TopicEdited {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -53,6 +54,7 @@
}
}
+ /** Event to be fired when the topic of a change has been edited. */
private static class Event extends AbstractChangeEvent implements TopicEditedListener.Event {
private final String oldTopic;
diff --git a/java/com/google/gerrit/server/extensions/events/VoteDeleted.java b/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
index 8533a65..ef4e461 100644
--- a/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
+++ b/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
@@ -36,6 +36,7 @@
import java.sql.Timestamp;
import java.util.Map;
+/** Helper class to fire an event when a vote has been deleted from a change. */
@Singleton
public class VoteDeleted {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -86,6 +87,7 @@
}
}
+ /** Event to be fired when a vote has been deleted from a change. */
private static class Event extends AbstractRevisionEvent implements VoteDeletedListener.Event {
private final AccountInfo reviewer;
private final Map<String, ApprovalInfo> approvals;
diff --git a/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java b/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java
index 16c5e25..359a3a8 100644
--- a/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java
+++ b/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java
@@ -33,6 +33,7 @@
import java.io.IOException;
import java.sql.Timestamp;
+/** Helper class to fire an event when the work-in-progress state of a change has been toggled. */
@Singleton
public class WorkInProgressStateChanged {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -68,6 +69,7 @@
}
}
+ /** Event to be fired when the work-in-progress state of a change has been toggled. */
private static class Event extends AbstractRevisionEvent
implements WorkInProgressStateChangedListener.Event {
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 393ccb7..818212c 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -14,19 +14,41 @@
package com.google.gerrit.server.git;
+import static com.google.common.base.MoreObjects.firstNonNull;
+
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Account.Id;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeMessages;
+import com.google.gerrit.server.change.NotifyResolver;
+import com.google.gerrit.server.extensions.events.ChangeReverted;
+import com.google.gerrit.server.mail.send.RevertedSender;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.util.time.TimeUtil;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
+import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.BatchUpdateOp;
+import com.google.gerrit.server.update.ChangeContext;
+import com.google.gerrit.server.update.Context;
+import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -34,6 +56,10 @@
import java.sql.Timestamp;
import java.text.MessageFormat;
import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Set;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -47,14 +73,41 @@
/** Static utilities for working with {@link RevCommit}s. */
@Singleton
public class CommitUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final GitRepositoryManager repoManager;
private final Provider<PersonIdent> serverIdent;
+ private final Sequences seq;
+ private final ApprovalsUtil approvalsUtil;
+ private final ChangeInserter.Factory changeInserterFactory;
+ private final NotifyResolver notifyResolver;
+ private final RevertedSender.Factory revertedSenderFactory;
+ private final ChangeMessagesUtil cmUtil;
+ private final ChangeReverted changeReverted;
+ private final BatchUpdate.Factory updateFactory;
@Inject
CommitUtil(
- GitRepositoryManager repoManager, @GerritPersonIdent Provider<PersonIdent> serverIdent) {
+ GitRepositoryManager repoManager,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent,
+ Sequences seq,
+ ApprovalsUtil approvalsUtil,
+ ChangeInserter.Factory changeInserterFactory,
+ NotifyResolver notifyResolver,
+ RevertedSender.Factory revertedSenderFactory,
+ ChangeMessagesUtil cmUtil,
+ ChangeReverted changeReverted,
+ BatchUpdate.Factory updateFactory) {
this.repoManager = repoManager;
this.serverIdent = serverIdent;
+ this.seq = seq;
+ this.approvalsUtil = approvalsUtil;
+ this.changeInserterFactory = changeInserterFactory;
+ this.notifyResolver = notifyResolver;
+ this.revertedSenderFactory = revertedSenderFactory;
+ this.cmUtil = cmUtil;
+ this.changeReverted = changeReverted;
+ this.updateFactory = updateFactory;
}
public static CommitInfo toCommitInfo(RevCommit commit) throws IOException {
@@ -81,49 +134,79 @@
}
/**
- * Allows creating a revert commit.
+ * Allows creating a revert change.
*
- * @param message Commit message for the revert commit.
* @param notes ChangeNotes of the change being reverted.
* @param user Current User performing the revert.
+ * @param input the RevertInput entity for conducting the revert.
+ * @param timestamp timestamp for the created change.
* @return ObjectId that represents the newly created commit.
- * @throws ResourceConflictException Can't revert the initial commit.
- * @throws IOException Thrown in case of I/O errors.
*/
- public ObjectId createRevertCommit(String message, ChangeNotes notes, CurrentUser user)
- throws ResourceConflictException, IOException {
- message = Strings.emptyToNull(message);
+ public Change.Id createRevertChange(
+ ChangeNotes notes, CurrentUser user, RevertInput input, Timestamp timestamp)
+ throws RestApiException, UpdateException, ConfigInvalidException, IOException {
+ String message = Strings.emptyToNull(input.message);
- Project.NameKey project = notes.getProjectName();
- try (Repository git = repoManager.openRepository(project);
+ try (Repository git = repoManager.openRepository(notes.getProjectName());
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
RevWalk revWalk = new RevWalk(reader)) {
- return createRevertCommit(message, notes, user, null, TimeUtil.nowTs(), oi, revWalk);
+ ObjectId generatedChangeId = Change.generateChangeId();
+ ObjectId revCommit =
+ createRevertCommit(message, notes, user, timestamp, oi, revWalk, generatedChangeId);
+ return createRevertChangeFromCommit(
+ revCommit, input, notes, user, generatedChangeId, timestamp, oi, revWalk, git);
+ } catch (RepositoryNotFoundException e) {
+ throw new ResourceNotFoundException(notes.getChangeId().toString(), e);
}
}
/**
+ * Wrapper function for creating a revert Commit.
+ *
* @param message Commit message for the revert commit.
* @param notes ChangeNotes of the change being reverted.
* @param user Current User performing the revert.
- * @param generatedChangeId The changeId for the commit message, can be null since it is not
- * needed for commits, only for changes.
+ * @param ts Timestamp of creation for the commit.
+ * @return ObjectId that represents the newly created commit.
+ */
+ public ObjectId createRevertCommit(
+ String message, ChangeNotes notes, CurrentUser user, Timestamp ts)
+ throws RestApiException, IOException {
+
+ try (Repository git = repoManager.openRepository(notes.getProjectName());
+ ObjectInserter oi = git.newObjectInserter();
+ ObjectReader reader = oi.newReader();
+ RevWalk revWalk = new RevWalk(reader)) {
+ return createRevertCommit(message, notes, user, ts, oi, revWalk, null);
+ } catch (RepositoryNotFoundException e) {
+ throw new ResourceNotFoundException(notes.getProjectName().toString(), e);
+ }
+ }
+
+ /**
+ * Creates a revert commit.
+ *
+ * @param message Commit message for the revert commit.
+ * @param notes ChangeNotes of the change being reverted.
+ * @param user Current User performing the revert.
* @param ts Timestamp of creation for the commit.
* @param oi ObjectInserter for inserting the newly created commit.
* @param revWalk Used for parsing the original commit.
+ * @param generatedChangeId The changeId for the commit message, can be null since it is not
+ * needed for commits, only for changes.
* @return ObjectId that represents the newly created commit.
* @throws ResourceConflictException Can't revert the initial commit.
* @throws IOException Thrown in case of I/O errors.
*/
- public ObjectId createRevertCommit(
+ private ObjectId createRevertCommit(
String message,
ChangeNotes notes,
CurrentUser user,
- @Nullable ObjectId generatedChangeId,
Timestamp ts,
ObjectInserter oi,
- RevWalk revWalk)
+ RevWalk revWalk,
+ @Nullable ObjectId generatedChangeId)
throws ResourceConflictException, IOException {
PatchSet patch = notes.getCurrentPatchSet();
@@ -162,4 +245,94 @@
oi.flush();
return id;
}
+
+ private Change.Id createRevertChangeFromCommit(
+ ObjectId revertCommitId,
+ RevertInput input,
+ ChangeNotes notes,
+ CurrentUser user,
+ @Nullable ObjectId generatedChangeId,
+ Timestamp ts,
+ ObjectInserter oi,
+ RevWalk revWalk,
+ Repository git)
+ throws IOException, RestApiException, UpdateException, ConfigInvalidException {
+ RevCommit revertCommit = revWalk.parseCommit(revertCommitId);
+ Change changeToRevert = notes.getChange();
+ Change.Id changeId = Change.id(seq.nextChangeId());
+ NotifyResolver.Result notify =
+ notifyResolver.resolve(firstNonNull(input.notify, NotifyHandling.ALL), input.notifyDetails);
+
+ ChangeInserter ins =
+ changeInserterFactory
+ .create(changeId, revertCommit, notes.getChange().getDest().branch())
+ .setTopic(input.topic == null ? changeToRevert.getTopic() : input.topic.trim());
+ ins.setMessage("Uploaded patch set 1.");
+
+ ReviewerSet reviewerSet = approvalsUtil.getReviewers(notes);
+
+ Set<Id> reviewers = new HashSet<>();
+ reviewers.add(changeToRevert.getOwner());
+ reviewers.addAll(reviewerSet.byState(ReviewerStateInternal.REVIEWER));
+ reviewers.remove(user.getAccountId());
+ Set<Account.Id> ccs = new HashSet<>(reviewerSet.byState(ReviewerStateInternal.CC));
+ ccs.remove(user.getAccountId());
+ ins.setReviewersAndCcs(reviewers, ccs);
+ ins.setRevertOf(notes.getChangeId());
+
+ try (BatchUpdate bu = updateFactory.create(notes.getProjectName(), user, ts)) {
+ bu.setRepository(git, revWalk, oi);
+ bu.setNotify(notify);
+ bu.insertChange(ins);
+ bu.addOp(changeId, new NotifyOp(changeToRevert, ins));
+ bu.addOp(changeToRevert.getId(), new PostRevertedMessageOp(generatedChangeId));
+ bu.execute();
+ }
+ return changeId;
+ }
+
+ private class NotifyOp implements BatchUpdateOp {
+ private final Change change;
+ private final ChangeInserter ins;
+
+ NotifyOp(Change change, ChangeInserter ins) {
+ this.change = change;
+ this.ins = ins;
+ }
+
+ @Override
+ public void postUpdate(Context ctx) throws Exception {
+ changeReverted.fire(change, ins.getChange(), ctx.getWhen());
+ try {
+ RevertedSender cm = revertedSenderFactory.create(ctx.getProject(), change.getId());
+ cm.setFrom(ctx.getAccountId());
+ cm.setNotify(ctx.getNotify(change.getId()));
+ cm.send();
+ } catch (Exception err) {
+ logger.atSevere().withCause(err).log(
+ "Cannot send email for revert change %s", change.getId());
+ }
+ }
+ }
+
+ private class PostRevertedMessageOp implements BatchUpdateOp {
+ private final ObjectId computedChangeId;
+
+ PostRevertedMessageOp(ObjectId computedChangeId) {
+ this.computedChangeId = computedChangeId;
+ }
+
+ @Override
+ public boolean updateChange(ChangeContext ctx) {
+ Change change = ctx.getChange();
+ PatchSet.Id patchSetId = change.currentPatchSetId();
+ ChangeMessage changeMessage =
+ ChangeMessagesUtil.newMessage(
+ ctx,
+ "Created a revert of this change as I" + computedChangeId.name(),
+ ChangeMessagesUtil.TAG_REVERT);
+ cmUtil.addChangeMessage(ctx.getUpdate(patchSetId), changeMessage);
+ return true;
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index e03e0fc..b188224 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -293,7 +293,7 @@
} else if ((e instanceof ExecutionException) && (e.getCause() instanceof RestApiException)) {
return (RestApiException) e.getCause();
}
- return new RestApiException("Error inserting change/patchset", e);
+ return RestApiException.wrap("Error inserting change/patchset", e);
}
// ReceiveCommits has a lot of fields, sorry. Here and in the constructor they are split up
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 24154d60..62114b0 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -368,10 +368,16 @@
Streams.concat(
Streams.stream(
newAddReviewerInputFromCommitIdentity(
- change, psInfo.getAuthor().getAccount(), NotifyHandling.NONE)),
+ change,
+ psInfo.getCommitId(),
+ psInfo.getAuthor().getAccount(),
+ NotifyHandling.NONE)),
Streams.stream(
newAddReviewerInputFromCommitIdentity(
- change, psInfo.getCommitter().getAccount(), NotifyHandling.NONE)));
+ change,
+ psInfo.getCommitId(),
+ psInfo.getCommitter().getAccount(),
+ NotifyHandling.NONE)));
if (magicBranch != null) {
inputs =
Streams.concat(
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index e8f6096..90d6b66 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -44,6 +44,11 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.validators.ValidationMessage.Type;
+import com.google.gerrit.server.patch.DiffSummary;
+import com.google.gerrit.server.patch.DiffSummaryKey;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListKey;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
@@ -76,7 +81,8 @@
import org.eclipse.jgit.util.SystemReader;
/**
- * Represents a list of CommitValidationListeners to run for a push to one branch of one project.
+ * Represents a list of {@link CommitValidationListener}s to run for a push to one branch of one
+ * project.
*/
public class CommitValidators {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -94,15 +100,16 @@
private final AllProjectsName allProjects;
private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
private final AccountValidator accountValidator;
- private final String installCommitMsgHookCommand;
private final ProjectCache projectCache;
private final ProjectConfig.Factory projectConfigFactory;
+ private final PatchListCache patchListCache;
+ private final Config config;
@Inject
Factory(
@GerritPersonIdent PersonIdent gerritIdent,
DynamicItem<UrlFormatter> urlFormatter,
- @GerritServerConfig Config cfg,
+ @GerritServerConfig Config config,
PluginSetContext<CommitValidationListener> pluginValidators,
GitRepositoryManager repoManager,
AllUsersName allUsers,
@@ -110,19 +117,20 @@
ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
AccountValidator accountValidator,
ProjectCache projectCache,
- ProjectConfig.Factory projectConfigFactory) {
+ ProjectConfig.Factory projectConfigFactory,
+ PatchListCache patchListCache) {
this.gerritIdent = gerritIdent;
this.urlFormatter = urlFormatter;
+ this.config = config;
this.pluginValidators = pluginValidators;
this.repoManager = repoManager;
this.allUsers = allUsers;
this.allProjects = allProjects;
this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
this.accountValidator = accountValidator;
- this.installCommitMsgHookCommand =
- cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null;
this.projectCache = projectCache;
this.projectConfigFactory = projectConfigFactory;
+ this.patchListCache = patchListCache;
}
public CommitValidators forReceiveCommits(
@@ -146,12 +154,7 @@
new CommitterUploaderValidator(user, perm, urlFormatter.get()),
new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(
- projectState,
- user,
- urlFormatter.get(),
- installCommitMsgHookCommand,
- sshInfo,
- change),
+ projectState, user, urlFormatter.get(), config, sshInfo, change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators, skipValidation),
@@ -176,14 +179,10 @@
new ProjectStateValidationListener(projectState),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, urlFormatter.get()),
+ new FileCountValidator(patchListCache, config),
new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.project())),
new ChangeIdValidator(
- projectState,
- user,
- urlFormatter.get(),
- installCommitMsgHookCommand,
- sshInfo,
- change),
+ projectState, user, urlFormatter.get(), config, sshInfo, change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@@ -268,14 +267,14 @@
ProjectState projectState,
IdentifiedUser user,
UrlFormatter urlFormatter,
- String installCommitMsgHookCommand,
+ Config config,
SshInfo sshInfo,
Change change) {
this.projectState = projectState;
- this.urlFormatter = urlFormatter;
- this.installCommitMsgHookCommand = installCommitMsgHookCommand;
- this.sshInfo = sshInfo;
this.user = user;
+ this.urlFormatter = urlFormatter;
+ installCommitMsgHookCommand = config.getString("gerrit", null, "installCommitMsgHookCommand");
+ this.sshInfo = sshInfo;
this.change = change;
}
@@ -387,6 +386,40 @@
}
}
+ /** Limits the number of files per change. */
+ private static class FileCountValidator implements CommitValidationListener {
+
+ private final PatchListCache patchListCache;
+ private final int maxFileCount;
+
+ FileCountValidator(PatchListCache patchListCache, Config config) {
+ this.patchListCache = patchListCache;
+ maxFileCount = config.getInt("change", null, "maxFiles", 50_000);
+ }
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
+ throws CommitValidationException {
+ PatchListKey patchListKey =
+ PatchListKey.againstBase(
+ receiveEvent.commit.getId(), receiveEvent.commit.getParentCount());
+ DiffSummaryKey diffSummaryKey = DiffSummaryKey.fromPatchListKey(patchListKey);
+ try {
+ DiffSummary diffSummary =
+ patchListCache.getDiffSummary(diffSummaryKey, receiveEvent.project.getNameKey());
+ if (diffSummary.getPaths().size() > maxFileCount) {
+ throw new CommitValidationException(
+ String.format(
+ "Exceeding maximum number of files per change (%d > %d)",
+ diffSummary.getPaths().size(), maxFileCount));
+ }
+ } catch (PatchListNotAvailableException e) {
+ logger.atWarning().withCause(e).log("Failed to validate file count");
+ }
+ return Collections.emptyList();
+ }
+ }
+
/** If this is the special project configuration branch, validate the config. */
public static class ConfigValidator implements CommitValidationListener {
private final ProjectConfig.Factory projectConfigFactory;
diff --git a/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java b/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java
index 9e6db44..d8ecbd9 100644
--- a/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java
+++ b/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java
@@ -27,7 +27,7 @@
import com.google.gerrit.server.account.AccountState;
/**
- * Wrapper around {@link Predicate}s that is returned by the {@link
+ * Wrapper around {@link Predicate}s that are returned by the {@link
* com.google.gerrit.index.IndexRewriter}. See {@link IndexedQuery}.
*/
public class IndexedAccountQuery extends IndexedQuery<Account.Id, AccountState>
diff --git a/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java b/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java
index 680db12..319b834 100644
--- a/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java
+++ b/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java
@@ -27,7 +27,7 @@
import java.util.Set;
/**
- * Wrapper around {@link Predicate}s that is returned by the {@link
+ * Wrapper around {@link Predicate}s that are returned by the {@link
* com.google.gerrit.index.IndexRewriter}. See {@link IndexedQuery}.
*/
public class IndexedGroupQuery extends IndexedQuery<AccountGroup.UUID, InternalGroup>
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index d312530..3bb4770 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -116,9 +116,6 @@
// Type of a sequence in NoteDb (ACCOUNTS, CHANGES, GROUPS).
public abstract Optional<String> noteDbSequenceType();
- // Name of a "table" in NoteDb (if set, always CHANGES).
- public abstract Optional<String> noteDbTable();
-
// The ID of a patch set.
public abstract Optional<Integer> patchSetId();
@@ -164,10 +161,10 @@
* httpStatus=Optional.empty, indexName=Optional.empty, indexVersion=Optional[0],
* methodName=Optional.empty, multiple=Optional.empty, operationName=Optional.empty,
* partial=Optional.empty, noteDbFilePath=Optional.empty, noteDbRefName=Optional.empty,
- * noteDbSequenceType=Optional.empty, noteDbTable=Optional.empty, patchSetId=Optional.empty,
- * pluginMetadata=[], pluginName=Optional.empty, projectName=Optional.empty,
- * pushType=Optional.empty, resourceCount=Optional.empty, restViewName=Optional.empty,
- * revision=Optional.empty, username=Optional.empty}
+ * noteDbSequenceType=Optional.empty, patchSetId=Optional.empty, pluginMetadata=[],
+ * pluginName=Optional.empty, projectName=Optional.empty, pushType=Optional.empty,
+ * resourceCount=Optional.empty, restViewName=Optional.empty, revision=Optional.empty,
+ * username=Optional.empty}
* </pre>
*
* <p>That's hard to read in logs. This is why this method
@@ -314,8 +311,6 @@
public abstract Builder noteDbSequenceType(@Nullable String noteDbSequenceType);
- public abstract Builder noteDbTable(@Nullable String noteDbTable);
-
public abstract Builder patchSetId(int patchSetId);
abstract ImmutableList.Builder<PluginMetadata> pluginMetadataBuilder();
diff --git a/java/com/google/gerrit/server/mail/MailUtil.java b/java/com/google/gerrit/server/mail/MailUtil.java
index 1040a55..ff22d23 100644
--- a/java/com/google/gerrit/server/mail/MailUtil.java
+++ b/java/com/google/gerrit/server/mail/MailUtil.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -33,6 +34,7 @@
import org.eclipse.jgit.revwalk.FooterLine;
public class MailUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public static MailRecipients getRecipientsFromFooters(
AccountResolver accountResolver, List<FooterLine> footerLines)
@@ -41,11 +43,19 @@
for (FooterLine footerLine : footerLines) {
try {
if (isReviewer(footerLine)) {
- recipients.reviewers.add(toAccountId(accountResolver, footerLine.getValue().trim()));
+ Account.Id accountId = toAccountId(accountResolver, footerLine.getValue().trim());
+ recipients.reviewers.add(accountId);
+ logger.atFine().log(
+ "Added account %d from footer line \"%s\" as reviewer", accountId.get(), footerLine);
} else if (footerLine.matches(FooterKey.CC)) {
- recipients.cc.add(toAccountId(accountResolver, footerLine.getValue().trim()));
+ Account.Id accountId = toAccountId(accountResolver, footerLine.getValue().trim());
+ recipients.cc.add(accountId);
+ logger.atFine().log(
+ "Added account %d from footer line \"%s\" as cc", accountId.get(), footerLine);
}
} catch (UnprocessableEntityException e) {
+ logger.atFine().log(
+ "Skip adding reviewer/cc from footer line \"%s\": %s", footerLine, e.getMessage());
continue;
}
}
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index 9a1ba35..9b5b4d4 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.notedb;
-import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
@@ -23,7 +22,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.metrics.Timer1;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -141,7 +140,7 @@
if (args.failOnLoadForTest.get()) {
throw new StorageException("Reading from NoteDb is disabled");
}
- try (Timer1.Context<NoteDbTable> timer = args.metrics.readLatency.start(CHANGES);
+ try (Timer0.Context timer = args.metrics.readLatency.start();
Repository repo = args.repoManager.openRepository(getProjectName());
// Call openHandle even if reading is disabled, to trigger
// auto-rebuilding before this object may get passed to a ChangeUpdate.
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 369d1f2..5468e23 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -37,7 +37,6 @@
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_WORK_IN_PROGRESS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.parseCommitMessageRange;
-import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.joining;
@@ -66,7 +65,7 @@
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.mail.Address;
-import com.google.gerrit.metrics.Timer1;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.server.AssigneeStatusUpdate;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
@@ -187,7 +186,7 @@
walk.reset();
walk.markStart(walk.parseCommit(tip));
- try (Timer1.Context<NoteDbTable> timer = metrics.parseLatency.start(CHANGES)) {
+ try (Timer0.Context timer = metrics.parseLatency.start()) {
ChangeNotesCommit commit;
while ((commit = walk.next()) != null) {
parse(commit);
diff --git a/java/com/google/gerrit/server/notedb/IntBlob.java b/java/com/google/gerrit/server/notedb/IntBlob.java
index c0af37f..61b9ae0 100644
--- a/java/com/google/gerrit/server/notedb/IntBlob.java
+++ b/java/com/google/gerrit/server/notedb/IntBlob.java
@@ -39,6 +39,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
+/** An object blob in a Git repository that stores a single integer value. */
@AutoValue
public abstract class IntBlob {
public static Optional<IntBlob> parse(Repository repo, String refName) throws IOException {
diff --git a/java/com/google/gerrit/server/notedb/NoteDbMetrics.java b/java/com/google/gerrit/server/notedb/NoteDbMetrics.java
index 18ffd17..246e7e1 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbMetrics.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbMetrics.java
@@ -16,69 +16,61 @@
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
-import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
-import com.google.gerrit.metrics.Timer1;
-import com.google.gerrit.server.logging.Metadata;
+import com.google.gerrit.metrics.Timer0;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+/** Metrics for accessing and updating changes in NoteDb. */
@Singleton
class NoteDbMetrics {
/** End-to-end latency for writing a collection of updates. */
- final Timer1<NoteDbTable> updateLatency;
+ final Timer0 updateLatency;
/**
* The portion of {@link #updateLatency} due to preparing the sequence of updates.
*
* <p>May include some I/O (e.g. reading old refs), but excludes writes.
*/
- final Timer1<NoteDbTable> stageUpdateLatency;
+ final Timer0 stageUpdateLatency;
/** End-to-end latency for reading changes from NoteDb, including reading ref(s) and parsing. */
- final Timer1<NoteDbTable> readLatency;
+ final Timer0 readLatency;
/**
* The portion of {@link #readLatency} due to parsing commits, but excluding I/O (to a best
* effort).
*/
- final Timer1<NoteDbTable> parseLatency;
+ final Timer0 parseLatency;
@Inject
NoteDbMetrics(MetricMaker metrics) {
- Field<NoteDbTable> tableField =
- Field.ofEnum(NoteDbTable.class, "table", Metadata.Builder::noteDbTable).build();
-
updateLatency =
metrics.newTimer(
"notedb/update_latency",
- new Description("NoteDb update latency by table")
+ new Description("NoteDb update latency for changes")
.setCumulative()
- .setUnit(Units.MILLISECONDS),
- tableField);
+ .setUnit(Units.MILLISECONDS));
stageUpdateLatency =
metrics.newTimer(
"notedb/stage_update_latency",
- new Description("Latency for staging updates to NoteDb by table")
+ new Description("Latency for staging change updates to NoteDb")
.setCumulative()
- .setUnit(Units.MICROSECONDS),
- tableField);
+ .setUnit(Units.MICROSECONDS));
readLatency =
metrics.newTimer(
"notedb/read_latency",
- new Description("NoteDb read latency by table")
+ new Description("NoteDb read latency for changes")
.setCumulative()
- .setUnit(Units.MILLISECONDS),
- tableField);
+ .setUnit(Units.MILLISECONDS));
parseLatency =
metrics.newTimer(
"notedb/parse_latency",
- new Description("NoteDb parse latency by table")
+ new Description("NoteDb parse latency for changes")
.setCumulative()
- .setUnit(Units.MICROSECONDS),
- tableField);
+ .setUnit(Units.MICROSECONDS));
}
}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbTable.java b/java/com/google/gerrit/server/notedb/NoteDbTable.java
deleted file mode 100644
index e299fdf..0000000
--- a/java/com/google/gerrit/server/notedb/NoteDbTable.java
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (C) 2016 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.notedb;
-
-public enum NoteDbTable {
- ACCOUNTS,
- GROUPS,
- CHANGES;
-
- public String key() {
- return name().toLowerCase();
- }
-
- @Override
- public String toString() {
- return key();
- }
-}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 1b92c0e..2d1a04a 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -17,7 +17,6 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
@@ -27,7 +26,7 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.git.RefUpdateUtil;
-import com.google.gerrit.metrics.Timer1;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -273,7 +272,7 @@
* @throws IOException if a storage layer error occurs.
*/
private void stage() throws IOException {
- try (Timer1.Context<NoteDbTable> timer = metrics.stageUpdateLatency.start(CHANGES)) {
+ try (Timer0.Context timer = metrics.stageUpdateLatency.start()) {
if (isEmpty()) {
return;
}
@@ -298,7 +297,7 @@
executed = true;
return null;
}
- try (Timer1.Context<NoteDbTable> timer = metrics.updateLatency.start(CHANGES)) {
+ try (Timer0.Context timer = metrics.updateLatency.start()) {
stage();
// ChangeUpdates must execute before ChangeDraftUpdates.
//
diff --git a/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index 15fa0f4..b663b9d 100644
--- a/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -138,11 +138,10 @@
throws PatchListNotAvailableException {
Project.NameKey project = change.getProject();
ObjectId b = patchSet.commitId();
- Whitespace ws = Whitespace.IGNORE_NONE;
if (parentNum != null) {
- return get(PatchListKey.againstParentNum(parentNum, b, ws), project);
+ return get(PatchListKey.againstParentNum(parentNum, b, Whitespace.IGNORE_NONE), project);
}
- return get(PatchListKey.againstDefaultBase(b, ws), project);
+ return get(PatchListKey.againstDefaultBase(b, Whitespace.IGNORE_NONE), project);
}
@Override
diff --git a/java/com/google/gerrit/server/patch/PatchListKey.java b/java/com/google/gerrit/server/patch/PatchListKey.java
index bf38029..386c50d 100644
--- a/java/com/google/gerrit/server/patch/PatchListKey.java
+++ b/java/com/google/gerrit/server/patch/PatchListKey.java
@@ -59,6 +59,12 @@
return new PatchListKey(otherCommitId, newId, whitespace);
}
+ public static PatchListKey againstBase(ObjectId id, int parentCount) {
+ return parentCount > 1
+ ? PatchListKey.againstParentNum(1, id, Whitespace.IGNORE_NONE)
+ : PatchListKey.againstDefaultBase(id, Whitespace.IGNORE_NONE);
+ }
+
/**
* Old patch-set ID
*
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index d831ab6..8ee0fee 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -70,7 +70,7 @@
* private final PermissionBackend permissions;
* private final Provider<CurrentUser> user;
*
- * @Inject
+ * {@literal @}Inject
* Foo(PermissionBackend permissions, Provider<CurrentUser> user) {
* this.permissions = permissions;
* this.user = user;
diff --git a/java/com/google/gerrit/server/plugins/DisablePlugin.java b/java/com/google/gerrit/server/plugins/DisablePlugin.java
index 8adae52..9e238f8 100644
--- a/java/com/google/gerrit/server/plugins/DisablePlugin.java
+++ b/java/com/google/gerrit/server/plugins/DisablePlugin.java
@@ -45,12 +45,9 @@
}
@Override
- public Response<PluginInfo> apply(PluginResource resource, Input input) throws RestApiException {
- try {
- permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
- } catch (PermissionBackendException e) {
- throw new RestApiException("Could not check permission", e);
- }
+ public Response<PluginInfo> apply(PluginResource resource, Input input)
+ throws RestApiException, PermissionBackendException {
+ permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
loader.checkRemoteAdminEnabled();
String name = resource.getName();
if (mandatoryPluginsCollection.contains(name)) {
diff --git a/java/com/google/gerrit/server/query/account/AccountPredicates.java b/java/com/google/gerrit/server/query/account/AccountPredicates.java
index 1eed7ea..e4da946 100644
--- a/java/com/google/gerrit/server/query/account/AccountPredicates.java
+++ b/java/com/google/gerrit/server/query/account/AccountPredicates.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import java.util.List;
+/** Utility class to create predicates for account index queries. */
public class AccountPredicates {
public static boolean hasActive(Predicate<AccountState> p) {
return QueryBuilder.find(p, AccountPredicate.class, AccountField.ACTIVE.getName()) != null;
@@ -130,6 +131,7 @@
return new CanSeeChangePredicate(args.permissionBackend, changeNotes);
}
+ /** Predicate that is mapped to a field in the account index. */
static class AccountPredicate extends IndexPredicate<AccountState>
implements Matchable<AccountState> {
AccountPredicate(FieldDef<AccountState, ?> def, String value) {
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 381dc6e..8879388 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -45,7 +45,6 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.ApprovalsUtil;
@@ -397,12 +396,7 @@
return Optional.empty();
}
- ObjectId id = ps.commitId();
- Whitespace ws = Whitespace.IGNORE_NONE;
- PatchListKey pk =
- parentCount > 1
- ? PatchListKey.againstParentNum(1, id, ws)
- : PatchListKey.againstDefaultBase(id, ws);
+ PatchListKey pk = PatchListKey.againstBase(ps.commitId(), parentCount);
DiffSummaryKey key = DiffSummaryKey.fromPatchListKey(pk);
try {
diffSummary = Optional.of(patchListCache.getDiffSummary(key, c.getProject()));
diff --git a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
index 7428e3a..a176a58 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
@@ -19,6 +19,7 @@
import com.google.gerrit.index.query.Matchable;
import com.google.gerrit.index.query.Predicate;
+/** Predicate that is mapped to a field in the change index. */
public abstract class ChangeIndexPredicate extends IndexPredicate<ChangeData>
implements Matchable<ChangeData> {
/**
diff --git a/java/com/google/gerrit/server/query/group/GroupPredicates.java b/java/com/google/gerrit/server/query/group/GroupPredicates.java
index 17a7000..5231c5a 100644
--- a/java/com/google/gerrit/server/query/group/GroupPredicates.java
+++ b/java/com/google/gerrit/server/query/group/GroupPredicates.java
@@ -23,6 +23,7 @@
import com.google.gerrit.server.index.group.GroupField;
import java.util.Locale;
+/** Utility class to create predicates for group index queries. */
public class GroupPredicates {
public static Predicate<InternalGroup> id(AccountGroup.Id groupId) {
return new GroupPredicate(GroupField.ID, groupId.toString());
@@ -63,6 +64,7 @@
return new GroupPredicate(GroupField.SUBGROUP, subgroupUuid.get());
}
+ /** Predicate that is mapped to a field in the group index. */
static class GroupPredicate extends IndexPredicate<InternalGroup> {
GroupPredicate(FieldDef<InternalGroup, ?> def, String value) {
super(def, value);
diff --git a/java/com/google/gerrit/server/query/project/ProjectPredicates.java b/java/com/google/gerrit/server/query/project/ProjectPredicates.java
index 4e56fac..8b4048f 100644
--- a/java/com/google/gerrit/server/query/project/ProjectPredicates.java
+++ b/java/com/google/gerrit/server/query/project/ProjectPredicates.java
@@ -22,6 +22,7 @@
import com.google.gerrit.index.query.Predicate;
import java.util.Locale;
+/** Utility class to create predicates for project index queries. */
public class ProjectPredicates {
public static Predicate<ProjectData> name(Project.NameKey nameKey) {
return new ProjectPredicate(ProjectField.NAME, nameKey.get());
diff --git a/java/com/google/gerrit/server/restapi/access/AccessCollection.java b/java/com/google/gerrit/server/restapi/access/AccessCollection.java
index 8ae2ce7..d8832a2 100644
--- a/java/com/google/gerrit/server/restapi/access/AccessCollection.java
+++ b/java/com/google/gerrit/server/restapi/access/AccessCollection.java
@@ -24,6 +24,7 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
+/** REST collection that serves requests to {@code /access/}. */
@Singleton
public class AccessCollection implements RestCollection<TopLevelResource, AccessResource> {
private final Provider<ListAccess> list;
diff --git a/java/com/google/gerrit/server/restapi/access/AccessResource.java b/java/com/google/gerrit/server/restapi/access/AccessResource.java
index 915165b..4847da4 100644
--- a/java/com/google/gerrit/server/restapi/access/AccessResource.java
+++ b/java/com/google/gerrit/server/restapi/access/AccessResource.java
@@ -18,6 +18,13 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.inject.TypeLiteral;
+/**
+ * REST resource that represents members in {@link AccessCollection}.
+ *
+ * <p>{@link AccessCollection} doesn't support accessing single members, hence this class only
+ * defines the {@link TypeLiteral} for the resource kind that is needed for binding the REST
+ * collection.
+ */
public class AccessResource implements RestResource {
public static final TypeLiteral<RestView<AccessResource>> ACCESS_KIND =
new TypeLiteral<RestView<AccessResource>>() {};
diff --git a/java/com/google/gerrit/server/restapi/access/ListAccess.java b/java/com/google/gerrit/server/restapi/access/ListAccess.java
index 2520821..1e1bade 100644
--- a/java/com/google/gerrit/server/restapi/access/ListAccess.java
+++ b/java/com/google/gerrit/server/restapi/access/ListAccess.java
@@ -27,6 +27,11 @@
import java.util.TreeMap;
import org.kohsuke.args4j.Option;
+/**
+ * REST endpoint to list members of the {@link AccessCollection}.
+ *
+ * <p>This REST endpoint handles {@code GET /access/} requests.
+ */
public class ListAccess implements RestReadView<TopLevelResource> {
@Option(
diff --git a/java/com/google/gerrit/server/restapi/access/Module.java b/java/com/google/gerrit/server/restapi/access/Module.java
index 7da2e26b..3a4955d 100644
--- a/java/com/google/gerrit/server/restapi/access/Module.java
+++ b/java/com/google/gerrit/server/restapi/access/Module.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
+/** Guice module that binds all REST endpoints for {@code /access/}. */
public class Module extends RestApiModule {
@Override
protected void configure() {
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index e220f35..aa36b73 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -30,7 +30,6 @@
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
@@ -120,7 +119,8 @@
@Override
public Response<?> apply(ChangeResource resource, IdString id, FileContentInput input)
- throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
+ throws AuthException, BadRequestException, ResourceConflictException, IOException,
+ PermissionBackendException {
putEdit.apply(resource, id.get(), input);
return Response.none();
}
@@ -280,19 +280,24 @@
@Override
public Response<?> apply(ChangeEditResource rsrc, FileContentInput input)
- throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
+ throws AuthException, BadRequestException, ResourceConflictException, IOException,
+ PermissionBackendException {
return apply(rsrc.getChangeResource(), rsrc.getPath(), input);
}
public Response<?> apply(ChangeResource rsrc, String path, FileContentInput input)
- throws ResourceConflictException, AuthException, IOException, PermissionBackendException {
+ throws AuthException, BadRequestException, ResourceConflictException, IOException,
+ PermissionBackendException {
if (Strings.isNullOrEmpty(path) || path.charAt(0) == '/') {
throw new ResourceConflictException("Invalid path: " + path);
}
- RawInput newContent = input.content;
+ if (input.content == null) {
+ throw new BadRequestException("new content required");
+ }
+
try (Repository repository = repositoryManager.openRepository(rsrc.getProject())) {
- editModifier.modifyFile(repository, rsrc.getNotes(), path, newContent);
+ editModifier.modifyFile(repository, rsrc.getNotes(), path, input.content);
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPick.java b/java/com/google/gerrit/server/restapi/change/CherryPick.java
index 725defc..2902eca 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPick.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPick.java
@@ -39,7 +39,6 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.submit.IntegrationException;
-import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -52,7 +51,6 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final PermissionBackend permissionBackend;
- private final BatchUpdate.Factory updateFactory;
private final CherryPickChange cherryPickChange;
private final ChangeJson.Factory json;
private final ContributorAgreementsChecker contributorAgreements;
@@ -61,13 +59,11 @@
@Inject
CherryPick(
PermissionBackend permissionBackend,
- BatchUpdate.Factory updateFactory,
CherryPickChange cherryPickChange,
ChangeJson.Factory json,
ContributorAgreementsChecker contributorAgreements,
ProjectCache projectCache) {
this.permissionBackend = permissionBackend;
- this.updateFactory = updateFactory;
this.cherryPickChange = cherryPickChange;
this.json = json;
this.contributorAgreements = contributorAgreements;
@@ -96,7 +92,6 @@
try {
CherryPickChange.Result cherryPickResult =
cherryPickChange.cherryPick(
- updateFactory,
rsrc.getChange(),
rsrc.getPatchSet(),
input,
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index 255c15c..3496491 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -109,6 +109,7 @@
private final ProjectCache projectCache;
private final ApprovalsUtil approvalsUtil;
private final NotifyResolver notifyResolver;
+ private final BatchUpdate.Factory batchUpdateFactory;
@Inject
CherryPickChange(
@@ -124,7 +125,8 @@
ChangeNotes.Factory changeNotesFactory,
ProjectCache projectCache,
ApprovalsUtil approvalsUtil,
- NotifyResolver notifyResolver) {
+ NotifyResolver notifyResolver,
+ BatchUpdate.Factory batchUpdateFactory) {
this.seq = seq;
this.queryProvider = queryProvider;
this.gitManager = gitManager;
@@ -138,12 +140,12 @@
this.projectCache = projectCache;
this.approvalsUtil = approvalsUtil;
this.notifyResolver = notifyResolver;
+ this.batchUpdateFactory = batchUpdateFactory;
}
/**
* This function is used for cherry picking a change.
*
- * @param batchUpdateFactory Used for applying changes to the database.
* @param change Change to cherry pick.
* @param patch The patch of that change.
* @param input Input object for different configurations of cherry pick.
@@ -158,22 +160,17 @@
* @throws ConfigInvalidException Can't find account to notify.
* @throws NoSuchProjectException Can't find project state.
*/
- public Result cherryPick(
- BatchUpdate.Factory batchUpdateFactory,
- Change change,
- PatchSet patch,
- CherryPickInput input,
- BranchNameKey dest)
+ public Result cherryPick(Change change, PatchSet patch, CherryPickInput input, BranchNameKey dest)
throws IOException, InvalidChangeOperationException, IntegrationException, UpdateException,
RestApiException, ConfigInvalidException, NoSuchProjectException {
return cherryPick(
- batchUpdateFactory,
change,
change.getProject(),
patch.commitId(),
input,
dest,
false,
+ TimeUtil.nowTs(),
null,
null,
null,
@@ -185,7 +182,6 @@
* This function is called directly to cherry pick a commit. Also, it is used to cherry pick a
* change as well as long as sourceChange is not null.
*
- * @param batchUpdateFactory Used for applying changes to the database.
* @param sourceChange Change to cherry pick. Can be null, and then the function will only cherry
* pick a commit.
* @param project Project name
@@ -203,7 +199,6 @@
* @throws NoSuchProjectException Can't find project state.
*/
public Result cherryPick(
- BatchUpdate.Factory batchUpdateFactory,
@Nullable Change sourceChange,
Project.NameKey project,
ObjectId sourceCommit,
@@ -212,13 +207,13 @@
throws IOException, InvalidChangeOperationException, IntegrationException, UpdateException,
RestApiException, ConfigInvalidException, NoSuchProjectException {
return cherryPick(
- batchUpdateFactory,
sourceChange,
project,
sourceCommit,
input,
dest,
false,
+ TimeUtil.nowTs(),
null,
null,
null,
@@ -231,7 +226,6 @@
* null) with a few other parameters that are especially useful for cherry-picking a commit that
* is the revert-of another change.
*
- * @param batchUpdateFactory Used for applying changes to the database.
* @param sourceChange Change to cherry pick. Can be null, and then the function will only cherry
* pick a commit.
* @param project Project name
@@ -240,6 +234,7 @@
* @param dest Destination branch for the cherry pick.
* @param ignoreIdenticalTree When false, we throw an error when trying to cherry-pick creates an
* empty commit. When true, we allow creation of an empty commit.
+ * @param timestamp the current timestamp.
* @param topic Topic name for the change created.
* @param revertedChange The id of the change that is reverted. This is used for the "revertOf"
* field to mark the created cherry pick change as "revertOf" the original change that was
@@ -262,13 +257,13 @@
* @throws NoSuchProjectException Can't find project state.
*/
public Result cherryPick(
- BatchUpdate.Factory batchUpdateFactory,
@Nullable Change sourceChange,
Project.NameKey project,
ObjectId sourceCommit,
CherryPickInput input,
BranchNameKey dest,
boolean ignoreIdenticalTree,
+ Timestamp timestamp,
@Nullable String topic,
@Nullable Change.Id revertedChange,
@Nullable ObjectId changeIdForNewChange,
@@ -306,8 +301,7 @@
String message = Strings.nullToEmpty(input.message).trim();
message = message.isEmpty() ? commitToCherryPick.getFullMessage() : message;
- Timestamp now = TimeUtil.nowTs();
- PersonIdent committerIdent = identifiedUser.newCommitterIdent(now, serverTimeZone);
+ PersonIdent committerIdent = identifiedUser.newCommitterIdent(timestamp, serverTimeZone);
final ObjectId generatedChangeId =
changeIdForNewChange != null ? changeIdForNewChange : Change.generateChangeId();
@@ -359,7 +353,7 @@
+ " reside on the same branch. "
+ "Cannot create a new patch set.");
}
- try (BatchUpdate bu = batchUpdateFactory.create(project, identifiedUser, now)) {
+ try (BatchUpdate bu = batchUpdateFactory.create(project, identifiedUser, timestamp)) {
bu.setRepository(git, revWalk, oi);
bu.setNotify(resolveNotify(input));
Change.Id changeId;
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java b/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
index 729e32d..8b5b07c 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.submit.IntegrationException;
-import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -46,7 +45,6 @@
@Singleton
public class CherryPickCommit implements RestModifyView<CommitResource, CherryPickInput> {
private final PermissionBackend permissionBackend;
- private final BatchUpdate.Factory updateFactory;
private final Provider<CurrentUser> user;
private final CherryPickChange cherryPickChange;
private final ChangeJson.Factory json;
@@ -55,13 +53,11 @@
@Inject
CherryPickCommit(
PermissionBackend permissionBackend,
- BatchUpdate.Factory updateFactory,
Provider<CurrentUser> user,
CherryPickChange cherryPickChange,
ChangeJson.Factory json,
ContributorAgreementsChecker contributorAgreements) {
this.permissionBackend = permissionBackend;
- this.updateFactory = updateFactory;
this.user = user;
this.cherryPickChange = cherryPickChange;
this.json = json;
@@ -92,7 +88,6 @@
try {
CherryPickChange.Result cherryPickResult =
cherryPickChange.cherryPick(
- updateFactory,
null,
projectName,
rsrc.getCommit(),
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index 310f71b..c0b28d2 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -20,6 +20,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
@@ -86,7 +87,6 @@
import org.eclipse.jgit.errors.InvalidObjectIdException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.NoMergeBaseException;
-import org.eclipse.jgit.errors.NoMergeBaseException.MergeBaseFailureReason;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -104,6 +104,8 @@
@Singleton
public class CreateChange
implements RestCollectionModifyView<TopLevelResource, ChangeResource, ChangeInput> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final BatchUpdate.Factory updateFactory;
private final String anonymousCowardName;
private final GitRepositoryManager gitManager;
@@ -294,20 +296,30 @@
BatchUpdate.Factory updateFactory)
throws RestApiException, PermissionBackendException, IOException, ConfigInvalidException,
UpdateException {
+ logger.atFine().log(
+ "Creating new change for target branch %s in project %s"
+ + " (new branch = %s, base change = %s, base commit = %s)",
+ input.branch, projectState.getName(), input.newBranch, input.baseChange, input.baseCommit);
+
try (Repository git = gitManager.openRepository(projectState.getNameKey());
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
RevWalk rw = new RevWalk(reader)) {
PatchSet basePatchSet = null;
List<String> groups = Collections.emptyList();
+
if (input.baseChange != null) {
ChangeNotes baseChange = getBaseChange(input.baseChange);
basePatchSet = psUtil.current(baseChange);
groups = basePatchSet.groups();
+ logger.atFine().log("base patch set = %s (groups = %s)", basePatchSet.id(), groups);
}
+
ObjectId parentCommit =
getParentCommit(
git, rw, input.branch, input.newBranch, basePatchSet, input.baseCommit, input.merge);
+ logger.atFine().log(
+ "parent commit = %s", parentCommit != null ? parentCommit.name() : "NULL");
RevCommit mergeTip = parentCommit == null ? null : rw.parseCommit(parentCommit);
@@ -464,6 +476,7 @@
RevCommit mergeTip,
String commitMessage)
throws IOException {
+ logger.atFine().log("Creating empty commit");
CommitBuilder commit = new CommitBuilder();
if (mergeTip == null) {
commit.setTreeId(emptyTreeId(oi));
@@ -487,6 +500,9 @@
PersonIdent authorIdent,
String commitMessage)
throws RestApiException, IOException {
+ logger.atFine().log(
+ "Creating merge commit: source = %s, strategy = %s", merge.source, merge.strategy);
+
if (Strings.isNullOrEmpty(merge.source)) {
throw new BadRequestException("merge.source must be non-empty");
}
@@ -500,6 +516,7 @@
// default merge strategy from project settings
String mergeStrategy =
firstNonNull(Strings.emptyToNull(merge.strategy), mergeUtil.mergeStrategyName());
+ logger.atFine().log("merge strategy = %s", mergeStrategy);
try {
return MergeUtil.createMergeCommit(
@@ -512,12 +529,8 @@
commitMessage,
rw);
} catch (NoMergeBaseException e) {
- if (MergeBaseFailureReason.TOO_MANY_MERGE_BASES == e.getReason()
- || MergeBaseFailureReason.CONFLICTS_DURING_MERGE_BASE_CALCULATION == e.getReason()) {
- throw new ResourceConflictException(
- String.format("Cannot create merge commit: %s", e.getMessage()), e);
- }
- throw e;
+ throw new ResourceConflictException(
+ String.format("Cannot create merge commit: %s", e.getMessage()), e);
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetDiff.java b/java/com/google/gerrit/server/restapi/change/GetDiff.java
index 82bdc34..c1d286a 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDiff.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDiff.java
@@ -29,6 +29,7 @@
import com.google.gerrit.extensions.common.DiffWebLinkInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.CacheControl;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -102,8 +103,8 @@
@Override
public Response<DiffInfo> apply(FileResource resource)
- throws ResourceConflictException, ResourceNotFoundException, AuthException,
- InvalidChangeOperationException, IOException, PermissionBackendException {
+ throws BadRequestException, ResourceConflictException, ResourceNotFoundException,
+ AuthException, InvalidChangeOperationException, IOException, PermissionBackendException {
DiffPreferencesInfo prefs = new DiffPreferencesInfo();
if (whitespace != null) {
prefs.ignoreWhitespace = whitespace;
@@ -130,6 +131,9 @@
RevisionResource baseResource =
revisions.parse(resource.getRevision().getChangeResource(), IdString.fromDecoded(base));
basePatchSet = baseResource.getPatchSet();
+ if (basePatchSet.id().get() == 0) {
+ throw new BadRequestException("edit not allowed as base");
+ }
psf = patchScriptFactoryFactory.create(notes, fileName, basePatchSet.id(), pId, prefs);
} else if (parentNum > 0) {
psf = patchScriptFactoryFactory.create(notes, fileName, parentNum - 1, pId, prefs);
diff --git a/java/com/google/gerrit/server/restapi/change/GetRelated.java b/java/com/google/gerrit/server/restapi/change/GetRelated.java
index 5f075f6..6eff9b0 100644
--- a/java/com/google/gerrit/server/restapi/change/GetRelated.java
+++ b/java/com/google/gerrit/server/restapi/change/GetRelated.java
@@ -19,6 +19,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -52,6 +53,8 @@
@Singleton
public class GetRelated implements RestReadView<RevisionResource> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final Provider<InternalChangeQuery> queryProvider;
private final PatchSetUtil psUtil;
private final RelatedChangesSorter sorter;
@@ -84,6 +87,7 @@
private List<RelatedChangeAndCommitInfo> getRelated(RevisionResource rsrc)
throws IOException, PermissionBackendException {
Set<String> groups = getAllGroups(rsrc.getNotes(), psUtil);
+ logger.atFine().log("groups = %s", groups);
if (groups.isEmpty()) {
return Collections.emptyList();
}
@@ -101,6 +105,7 @@
boolean isEdit = rsrc.getEdit().isPresent();
PatchSet basePs = isEdit ? rsrc.getEdit().get().getBasePatchSet() : rsrc.getPatchSet();
+ logger.atFine().log("isEdit = %s, basePs = %s", isEdit, basePs);
cds = reloadChangeIfStale(cds, rsrc.getChange(), basePs);
@@ -111,6 +116,9 @@
// Replace base of an edit with the edit itself.
ps = rsrc.getPatchSet();
commit = rsrc.getEdit().get().getEditCommit();
+ logger.atFine().log(
+ "Replaced base of edit (patch set %s, commit %s) with edit (patch set %s, commit %s)",
+ d.patchSet().id(), d.commit(), ps.id(), commit);
} else {
commit = d.commit();
}
diff --git a/java/com/google/gerrit/server/restapi/change/Revert.java b/java/com/google/gerrit/server/restapi/change/Revert.java
index d0b2562..7ebb954 100644
--- a/java/com/google/gerrit/server/restapi/change/Revert.java
+++ b/java/com/google/gerrit/server/restapi/change/Revert.java
@@ -14,17 +14,12 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -33,49 +28,25 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
-import com.google.gerrit.server.ApprovalsUtil;
-import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.ReviewerSet;
-import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.NotifyResolver;
-import com.google.gerrit.server.extensions.events.ChangeReverted;
import com.google.gerrit.server.git.CommitUtil;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.mail.send.RevertedSender;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.notedb.ReviewerStateInternal;
-import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ContributorAgreementsChecker;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.update.BatchUpdate;
-import com.google.gerrit.server.update.BatchUpdateOp;
-import com.google.gerrit.server.update.ChangeContext;
-import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
-import java.util.HashSet;
-import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
@Singleton
public class Revert
@@ -83,52 +54,25 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final PermissionBackend permissionBackend;
- private final BatchUpdate.Factory updateFactory;
- private final GitRepositoryManager repoManager;
- private final ChangeInserter.Factory changeInserterFactory;
- private final ChangeMessagesUtil cmUtil;
- private final Sequences seq;
private final PatchSetUtil psUtil;
- private final RevertedSender.Factory revertedSenderFactory;
private final ChangeJson.Factory json;
- private final ApprovalsUtil approvalsUtil;
- private final ChangeReverted changeReverted;
private final ContributorAgreementsChecker contributorAgreements;
private final ProjectCache projectCache;
- private final NotifyResolver notifyResolver;
private final CommitUtil commitUtil;
@Inject
Revert(
PermissionBackend permissionBackend,
- BatchUpdate.Factory updateFactory,
- GitRepositoryManager repoManager,
- ChangeInserter.Factory changeInserterFactory,
- ChangeMessagesUtil cmUtil,
- Sequences seq,
PatchSetUtil psUtil,
- RevertedSender.Factory revertedSenderFactory,
ChangeJson.Factory json,
- ApprovalsUtil approvalsUtil,
- ChangeReverted changeReverted,
ContributorAgreementsChecker contributorAgreements,
ProjectCache projectCache,
- NotifyResolver notifyResolver,
CommitUtil commitUtil) {
this.permissionBackend = permissionBackend;
- this.updateFactory = updateFactory;
- this.repoManager = repoManager;
- this.changeInserterFactory = changeInserterFactory;
- this.cmUtil = cmUtil;
- this.seq = seq;
this.psUtil = psUtil;
- this.revertedSenderFactory = revertedSenderFactory;
this.json = json;
- this.approvalsUtil = approvalsUtil;
- this.changeReverted = changeReverted;
this.contributorAgreements = contributorAgreements;
this.projectCache = projectCache;
- this.notifyResolver = notifyResolver;
this.commitUtil = commitUtil;
}
@@ -144,70 +88,19 @@
contributorAgreements.check(rsrc.getProject(), rsrc.getUser());
permissionBackend.user(rsrc.getUser()).ref(change.getDest()).check(CREATE_CHANGE);
projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
-
- Change.Id revertId = revert(updateFactory, rsrc.getNotes(), rsrc.getUser(), input);
- return Response.ok(json.noOptions().format(rsrc.getProject(), revertId));
- }
-
- private Change.Id revert(
- BatchUpdate.Factory updateFactory, ChangeNotes notes, CurrentUser user, RevertInput input)
- throws IOException, RestApiException, UpdateException, ConfigInvalidException {
+ ChangeNotes notes = rsrc.getNotes();
Change.Id changeIdToRevert = notes.getChangeId();
PatchSet.Id patchSetId = notes.getChange().currentPatchSetId();
PatchSet patch = psUtil.get(notes, patchSetId);
if (patch == null) {
throw new ResourceNotFoundException(changeIdToRevert.toString());
}
-
- Project.NameKey project = notes.getProjectName();
- try (Repository git = repoManager.openRepository(project);
- ObjectInserter oi = git.newObjectInserter();
- ObjectReader reader = oi.newReader();
- RevWalk revWalk = new RevWalk(reader)) {
-
- Timestamp now = TimeUtil.nowTs();
- ObjectId generatedChangeId = Change.generateChangeId();
- Change changeToRevert = notes.getChange();
- ObjectId revertCommitId =
- commitUtil.createRevertCommit(
- input.message, notes, user, generatedChangeId, now, oi, revWalk);
-
- RevCommit revertCommit = revWalk.parseCommit(revertCommitId);
-
- Change.Id changeId = Change.id(seq.nextChangeId());
- NotifyResolver.Result notify =
- notifyResolver.resolve(
- firstNonNull(input.notify, NotifyHandling.ALL), input.notifyDetails);
-
- ChangeInserter ins =
- changeInserterFactory
- .create(changeId, revertCommit, notes.getChange().getDest().branch())
- .setTopic(input.topic == null ? changeToRevert.getTopic() : input.topic.trim());
- ins.setMessage("Uploaded patch set 1.");
-
- ReviewerSet reviewerSet = approvalsUtil.getReviewers(notes);
-
- Set<Account.Id> reviewers = new HashSet<>();
- reviewers.add(changeToRevert.getOwner());
- reviewers.addAll(reviewerSet.byState(ReviewerStateInternal.REVIEWER));
- reviewers.remove(user.getAccountId());
- Set<Account.Id> ccs = new HashSet<>(reviewerSet.byState(ReviewerStateInternal.CC));
- ccs.remove(user.getAccountId());
- ins.setReviewersAndCcs(reviewers, ccs);
- ins.setRevertOf(changeIdToRevert);
-
- try (BatchUpdate bu = updateFactory.create(project, user, now)) {
- bu.setRepository(git, revWalk, oi);
- bu.setNotify(notify);
- bu.insertChange(ins);
- bu.addOp(changeId, new NotifyOp(changeToRevert, ins));
- bu.addOp(changeToRevert.getId(), new PostRevertedMessageOp(generatedChangeId));
- bu.execute();
- }
- return changeId;
- } catch (RepositoryNotFoundException e) {
- throw new ResourceNotFoundException(changeIdToRevert.toString(), e);
- }
+ Timestamp timestamp = TimeUtil.nowTs();
+ return Response.ok(
+ json.noOptions()
+ .format(
+ rsrc.getProject(),
+ commitUtil.createRevertChange(notes, rsrc.getUser(), input, timestamp)));
}
@Override
@@ -231,49 +124,4 @@
.ref(change.getDest())
.testCond(CREATE_CHANGE)));
}
-
- private class NotifyOp implements BatchUpdateOp {
- private final Change change;
- private final ChangeInserter ins;
-
- NotifyOp(Change change, ChangeInserter ins) {
- this.change = change;
- this.ins = ins;
- }
-
- @Override
- public void postUpdate(Context ctx) throws Exception {
- changeReverted.fire(change, ins.getChange(), ctx.getWhen());
- try {
- RevertedSender cm = revertedSenderFactory.create(ctx.getProject(), change.getId());
- cm.setFrom(ctx.getAccountId());
- cm.setNotify(ctx.getNotify(change.getId()));
- cm.send();
- } catch (Exception err) {
- logger.atSevere().withCause(err).log(
- "Cannot send email for revert change %s", change.getId());
- }
- }
- }
-
- private class PostRevertedMessageOp implements BatchUpdateOp {
- private final ObjectId computedChangeId;
-
- PostRevertedMessageOp(ObjectId computedChangeId) {
- this.computedChangeId = computedChangeId;
- }
-
- @Override
- public boolean updateChange(ChangeContext ctx) {
- Change change = ctx.getChange();
- PatchSet.Id patchSetId = change.currentPatchSetId();
- ChangeMessage changeMessage =
- ChangeMessagesUtil.newMessage(
- ctx,
- "Created a revert of this change as I" + computedChangeId.name(),
- ChangeMessagesUtil.TAG_REVERT);
- cmUtil.addChangeMessage(ctx.getUpdate(patchSetId), changeMessage);
- return true;
- }
- }
}
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index a7d19d1..8fc59a2 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -34,6 +34,7 @@
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.RevertSubmissionInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -73,6 +74,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
+import java.sql.Timestamp;
import java.text.MessageFormat;
import java.util.ArrayDeque;
import java.util.ArrayList;
@@ -83,6 +85,8 @@
import java.util.List;
import java.util.Queue;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.apache.commons.lang.RandomStringUtils;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -110,16 +114,17 @@
private final ChangeMessagesUtil cmUtil;
private final CommitUtil commitUtil;
private final ChangeNotes.Factory changeNotesFactory;
- private final ChangeResource.Factory changeResourceFactory;
private final ChangeReverted changeReverted;
private final RevertedSender.Factory revertedSenderFactory;
private final Sequences seq;
private final NotifyResolver notifyResolver;
- private final Revert revert;
private final BatchUpdate.Factory updateFactory;
private CherryPickInput cherryPickInput;
private List<ChangeInfo> results;
+ private static final Pattern patternRevertSubject = Pattern.compile("Revert \"(.+)\"");
+ private static final Pattern patternRevertSubjectWithNum =
+ Pattern.compile("Revert\\^(\\d+) \"(.+)\"");
@Inject
RevertSubmission(
@@ -136,12 +141,10 @@
ChangeMessagesUtil cmUtil,
CommitUtil commitUtil,
ChangeNotes.Factory changeNotesFactory,
- ChangeResource.Factory changeResourceFactory,
ChangeReverted changeReverted,
RevertedSender.Factory revertedSenderFactory,
Sequences seq,
NotifyResolver notifyResolver,
- Revert revert,
BatchUpdate.Factory updateFactory) {
this.queryProvider = queryProvider;
this.user = user;
@@ -156,12 +159,10 @@
this.cmUtil = cmUtil;
this.commitUtil = commitUtil;
this.changeNotesFactory = changeNotesFactory;
- this.changeResourceFactory = changeResourceFactory;
this.changeReverted = changeReverted;
this.revertedSenderFactory = revertedSenderFactory;
this.seq = seq;
this.notifyResolver = notifyResolver;
- this.revert = revert;
this.updateFactory = updateFactory;
results = new ArrayList<>();
cherryPickInput = null;
@@ -185,6 +186,22 @@
}
List<ChangeData> changeDatas = queryProvider.get().bySubmissionId(submissionId);
+ checkPermissionsForAllChanges(changeResource, changeDatas);
+ input.topic = createTopic(input.topic, submissionId);
+ return Response.ok(revertSubmission(changeDatas, input));
+ }
+
+ private String createTopic(String topic, String submissionId) {
+ if (topic == null) {
+ return String.format(
+ "revert-%s-%s", submissionId, RandomStringUtils.randomAlphabetic(10).toUpperCase());
+ }
+ return topic;
+ }
+
+ private void checkPermissionsForAllChanges(
+ ChangeResource changeResource, List<ChangeData> changeDatas)
+ throws IOException, AuthException, PermissionBackendException, ResourceConflictException {
for (ChangeData changeData : changeDatas) {
Change change = changeData.change();
@@ -203,23 +220,130 @@
"current patch set %s of change %s not found",
change.currentPatchSetId(), change.currentPatchSetId()));
}
-
- if (input.topic == null) {
- input.topic =
- String.format(
- "revert-%s-%s", submissionId, RandomStringUtils.randomAlphabetic(10).toUpperCase());
- }
-
- return Response.ok(revertSubmission(changeDatas, input));
}
private RevertSubmissionInfo revertSubmission(
List<ChangeData> changeData, RevertInput revertInput)
- throws RestApiException, IOException, UpdateException, PermissionBackendException,
- NoSuchProjectException, ConfigInvalidException, StorageException {
+ throws RestApiException, IOException, UpdateException, ConfigInvalidException,
+ StorageException {
+
Multimap<BranchNameKey, ChangeData> changesPerProjectAndBranch = ArrayListMultimap.create();
changeData.stream().forEach(c -> changesPerProjectAndBranch.put(c.change().getDest(), c));
+ cherryPickInput = createCherryPickInput(revertInput);
+ Timestamp timestamp = TimeUtil.nowTs();
+ for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
+ cherryPickInput.base = null;
+ Project.NameKey project = projectAndBranch.project();
+ cherryPickInput.destination = projectAndBranch.branch();
+ Collection<ChangeData> changesInProjectAndBranch =
+ changesPerProjectAndBranch.get(projectAndBranch);
+
+ // Sort the changes topologically.
+ Iterator<PatchSetData> sortedChangesInProjectAndBranch =
+ sorter.sort(changesInProjectAndBranch).iterator();
+
+ Set<ObjectId> commitIdsInProjectAndBranch =
+ changesInProjectAndBranch.stream()
+ .map(c -> c.currentPatchSet().commitId())
+ .collect(Collectors.toSet());
+
+ revertAllChangesInProjectAndBranch(
+ revertInput,
+ project,
+ sortedChangesInProjectAndBranch,
+ commitIdsInProjectAndBranch,
+ timestamp);
+ }
+ results.sort(Comparator.comparing(c -> c.revertOf));
+ RevertSubmissionInfo revertSubmissionInfo = new RevertSubmissionInfo();
+ revertSubmissionInfo.revertChanges = results;
+ return revertSubmissionInfo;
+ }
+
+ private void revertAllChangesInProjectAndBranch(
+ RevertInput revertInput,
+ Project.NameKey project,
+ Iterator<PatchSetData> sortedChangesInProjectAndBranch,
+ Set<ObjectId> commitIdsInProjectAndBranch,
+ Timestamp timestamp)
+ throws IOException, RestApiException, UpdateException, ConfigInvalidException {
+
+ String groupName = null;
+ String initialMessage = revertInput.message;
+ while (sortedChangesInProjectAndBranch.hasNext()) {
+ ChangeNotes changeNotes = sortedChangesInProjectAndBranch.next().data().notes();
+ if (cherryPickInput.base == null) {
+ cherryPickInput.base = getBase(changeNotes, commitIdsInProjectAndBranch).name();
+ }
+
+ revertInput.message = getMessage(initialMessage, changeNotes);
+ if (cherryPickInput.base.equals(changeNotes.getCurrentPatchSet().commitId().getName())) {
+ // This is the code in case this is the first revert of this project + branch, and the
+ // revert would be on top of the change being reverted.
+ craeteNormalRevert(revertInput, changeNotes, timestamp);
+ groupName = cherryPickInput.base;
+ } else {
+ // This is the code in case this is the second revert (or more) of this project + branch.
+ if (groupName == null) {
+ groupName = cherryPickInput.base;
+ }
+ createCherryPickedRevert(revertInput, project, groupName, changeNotes, timestamp);
+ }
+ }
+ }
+
+ private void createCherryPickedRevert(
+ RevertInput revertInput,
+ Project.NameKey project,
+ String groupName,
+ ChangeNotes changeNotes,
+ Timestamp timestamp)
+ throws IOException, ConfigInvalidException, UpdateException, RestApiException {
+ ObjectId revCommitId =
+ commitUtil.createRevertCommit(revertInput.message, changeNotes, user.get(), timestamp);
+ // TODO (paiking): As a future change, the revert should just be done directly on the
+ // target rather than just creating a commit and then cherry-picking it.
+ cherryPickInput.message = revertInput.message;
+ ObjectId generatedChangeId = Change.generateChangeId();
+ Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
+ // TODO (paiking): In the the future, the timestamp should be the same for all the revert
+ // changes.
+ try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ bu.setNotify(
+ notifyResolver.resolve(
+ firstNonNull(cherryPickInput.notify, NotifyHandling.ALL),
+ cherryPickInput.notifyDetails));
+ bu.addOp(
+ changeNotes.getChange().getId(),
+ new CreateCherryPickOp(
+ revCommitId,
+ revertInput.topic,
+ generatedChangeId,
+ cherryPickRevertChangeId,
+ groupName,
+ timestamp));
+ bu.addOp(changeNotes.getChange().getId(), new PostRevertedMessageOp(generatedChangeId));
+ bu.addOp(
+ cherryPickRevertChangeId,
+ new NotifyOp(changeNotes.getChange(), cherryPickRevertChangeId));
+
+ bu.execute();
+ }
+ }
+
+ private void craeteNormalRevert(
+ RevertInput revertInput, ChangeNotes changeNotes, Timestamp timestamp)
+ throws IOException, RestApiException, UpdateException, ConfigInvalidException {
+
+ Change.Id revertId =
+ commitUtil.createRevertChange(changeNotes, user.get(), revertInput, timestamp);
+ results.add(json.noOptions().format(changeNotes.getProjectName(), revertId));
+ cherryPickInput.base =
+ changeNotesFactory.createChecked(revertId).getCurrentPatchSet().commitId().getName();
+ }
+
+ private CherryPickInput createCherryPickInput(RevertInput revertInput) {
cherryPickInput = new CherryPickInput();
// To create a revert change, we create a revert commit that is then cherry-picked. The revert
// change is created for the cherry-picked commit. Notifications are sent only for this change,
@@ -228,102 +352,48 @@
cherryPickInput.notifyDetails = revertInput.notifyDetails;
cherryPickInput.parent = 1;
cherryPickInput.keepReviewers = true;
-
- for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
- String groupName = null;
- Project.NameKey project = projectAndBranch.project();
- cherryPickInput.destination = projectAndBranch.branch();
- Collection<ChangeData> changesInProjectAndBranch =
- changesPerProjectAndBranch.get(projectAndBranch);
-
- // Sort the changes topologically.
- Iterator<PatchSetData> sortedChangesInProject =
- sorter.sort(changesInProjectAndBranch).iterator();
-
- Set<ObjectId> commitIdsInProjectAndBranch =
- changesInProjectAndBranch.stream()
- .map(c -> c.currentPatchSet().commitId())
- .collect(Collectors.toSet());
-
- while (sortedChangesInProject.hasNext()) {
- ChangeNotes changeNotes = sortedChangesInProject.next().data().notes();
- if (cherryPickInput.base == null) {
- cherryPickInput.base = getBase(changeNotes, commitIdsInProjectAndBranch).name();
- }
-
- revertInput.message = getMessage(revertInput, changeNotes);
- // This is the code in case this is the first revert of this project + branch, and the
- // revert would be on top of the change being reverted.
- if (cherryPickInput.base.equals(changeNotes.getCurrentPatchSet().commitId().getName())) {
- ChangeInfo revertChangeInfo =
- revert
- .apply(changeResourceFactory.create(changeNotes, user.get()), revertInput)
- .value();
- results.add(revertChangeInfo);
- cherryPickInput.base =
- changeNotesFactory
- .createChecked(Change.id(revertChangeInfo._number))
- .getCurrentPatchSet()
- .commitId()
- .getName();
- groupName = cherryPickInput.base;
- } else {
- // This is the code in case this is the second revert (or more) of this project + branch.
- ObjectId revCommitId =
- commitUtil.createRevertCommit(revertInput.message, changeNotes, user.get());
- // TODO (paiking): As a future change, the revert should just be done directly on the
- // target rather than just creating a commit and then cherry-picking it.
- cherryPickInput.message = revertInput.message;
- ObjectId generatedChangeId = Change.generateChangeId();
- Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
- if (groupName == null) {
- groupName = cherryPickInput.base;
- }
- // TODO (paiking): In the the future, the timestamp should be the same for all the revert
- // changes.
- try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.nowTs())) {
- bu.setNotify(
- notifyResolver.resolve(
- firstNonNull(cherryPickInput.notify, NotifyHandling.ALL),
- cherryPickInput.notifyDetails));
- bu.addOp(
- changeNotes.getChange().getId(),
- new CreateCherryPickOp(
- revCommitId,
- revertInput.topic,
- generatedChangeId,
- cherryPickRevertChangeId,
- groupName));
- bu.addOp(changeNotes.getChange().getId(), new PostRevertedMessageOp(generatedChangeId));
- bu.addOp(
- cherryPickRevertChangeId,
- new NotifyOp(changeNotes.getChange(), cherryPickRevertChangeId));
-
- bu.execute();
- }
- }
- }
- cherryPickInput.base = null;
- }
- results.sort(Comparator.comparing(c -> c.revertOf));
- RevertSubmissionInfo revertSubmissionInfo = new RevertSubmissionInfo();
- revertSubmissionInfo.revertChanges = results;
- return revertSubmissionInfo;
+ return cherryPickInput;
}
- private String getMessage(RevertInput revertInput, ChangeNotes changeNotes) {
+ private String getMessage(String initialMessage, ChangeNotes changeNotes) {
String subject = changeNotes.getChange().getSubject();
- if (subject.length() > 63) {
- subject = subject.substring(0, 59) + "...";
+ if (subject.length() > 60) {
+ subject = subject.substring(0, 56) + "...";
}
- if (revertInput.message == null) {
+ if (initialMessage == null) {
+ initialMessage =
+ MessageFormat.format(
+ ChangeMessages.get().revertSubmissionDefaultMessage,
+ changeNotes.getCurrentPatchSet().commitId().name());
+ }
+
+ // For performance purposes: Almost all cases will end here.
+ if (!subject.startsWith("Revert")) {
return MessageFormat.format(
- ChangeMessages.get().revertChangeDefaultMessage,
- subject,
+ ChangeMessages.get().revertSubmissionUserMessage, subject, initialMessage);
+ }
+
+ Matcher matcher = patternRevertSubjectWithNum.matcher(subject);
+
+ if (matcher.matches()) {
+ return MessageFormat.format(
+ ChangeMessages.get().revertSubmissionOfRevertSubmissionUserMessage,
+ Integer.valueOf(matcher.group(1)) + 1,
+ matcher.group(2),
changeNotes.getCurrentPatchSet().commitId().name());
}
- return String.format("Revert \"%s\"\n\n%s", subject, revertInput.message);
+ matcher = patternRevertSubject.matcher(subject);
+ if (matcher.matches()) {
+ return MessageFormat.format(
+ ChangeMessages.get().revertSubmissionOfRevertSubmissionUserMessage,
+ 2,
+ matcher.group(1),
+ changeNotes.getCurrentPatchSet().commitId().name());
+ }
+
+ return MessageFormat.format(
+ ChangeMessages.get().revertSubmissionUserMessage, subject, initialMessage);
}
/**
@@ -499,18 +569,21 @@
private final ObjectId computedChangeId;
private final Change.Id cherryPickRevertChangeId;
private final String groupName;
+ private final Timestamp timestamp;
CreateCherryPickOp(
ObjectId revCommitId,
String topic,
ObjectId computedChangeId,
Change.Id cherryPickRevertChangeId,
- String groupName) {
+ String groupName,
+ Timestamp timestamp) {
this.revCommitId = revCommitId;
this.topic = topic;
this.computedChangeId = computedChangeId;
this.cherryPickRevertChangeId = cherryPickRevertChangeId;
this.groupName = groupName;
+ this.timestamp = timestamp;
}
@Override
@@ -518,7 +591,6 @@
Change change = ctx.getChange();
Result cherryPickResult =
cherryPickChange.cherryPick(
- updateFactory,
change,
change.getProject(),
revCommitId,
@@ -526,6 +598,7 @@
BranchNameKey.create(
change.getProject(), RefNames.fullName(cherryPickInput.destination)),
true,
+ timestamp,
topic,
change.getId(),
computedChangeId,
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 287c2bc..53e96f7 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -212,12 +212,10 @@
return Response.ok(new Output(change));
}
- String msg =
+ throw new IllegalStateException(
String.format(
"change %s of project %s unexpectedly had status %s after submit attempt",
- updatedChange.getId(), updatedChange.getProject(), updatedChange.getStatus());
- logger.atWarning().log(msg);
- throw new RestApiException(msg);
+ updatedChange.getId(), updatedChange.getProject(), updatedChange.getStatus()));
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateDashboard.java b/java/com/google/gerrit/server/restapi/project/CreateDashboard.java
index 9904b1f..e9a0d7f 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateDashboard.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateDashboard.java
@@ -17,7 +17,7 @@
import com.google.gerrit.extensions.api.projects.DashboardInfo;
import com.google.gerrit.extensions.api.projects.SetDashboardInput;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
@@ -48,7 +48,7 @@
throws RestApiException, IOException, PermissionBackendException {
parent.getProjectState().checkStatePermitsWrite();
if (!DashboardsCollection.isDefaultDashboard(id)) {
- throw new ResourceNotFoundException(id);
+ throw new MethodNotAllowedException("cannot create non-default dashboard");
}
SetDefaultDashboard set = setDefault.get();
set.inherited = inherited;
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteDashboard.java b/java/com/google/gerrit/server/restapi/project/DeleteDashboard.java
index 9d9e5f5..e507f05 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteDashboard.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteDashboard.java
@@ -43,6 +43,6 @@
}
// TODO: Implement delete of dashboards by API.
- throw new MethodNotAllowedException();
+ throw new MethodNotAllowedException("cannot delete non-default dashboard");
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetDashboard.java b/java/com/google/gerrit/server/restapi/project/SetDashboard.java
index e8e0c0d..56c62dc 100644
--- a/java/com/google/gerrit/server/restapi/project/SetDashboard.java
+++ b/java/com/google/gerrit/server/restapi/project/SetDashboard.java
@@ -40,7 +40,7 @@
return defaultSetter.get().apply(resource, input);
}
- // TODO: Implement creation/update of dashboards by API.
- throw new MethodNotAllowedException();
+ // TODO: Implement update of dashboards by API.
+ throw new MethodNotAllowedException("cannot update non-default dashboard");
}
}
diff --git a/java/com/google/gerrit/server/schema/JdbcUtil.java b/java/com/google/gerrit/server/schema/JdbcUtil.java
deleted file mode 100644
index 9f6822c..0000000
--- a/java/com/google/gerrit/server/schema/JdbcUtil.java
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright (C) 2012 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.schema;
-
-import com.google.gerrit.common.UsedAt;
-
-public class JdbcUtil {
-
- public static String hostname(String hostname) {
- if (hostname == null || hostname.isEmpty()) {
- hostname = "localhost";
-
- } else if (hostname.contains(":") && !hostname.startsWith("[")) {
- hostname = "[" + hostname + "]";
- }
- return hostname;
- }
-
- // TODO(dborowitz): Still used by plugins post-ReviewDb?
- @UsedAt(UsedAt.Project.PLUGINS_ALL)
- public static String port(String port) {
- if (port != null && !port.isEmpty()) {
- return ":" + port;
- }
- return "";
- }
-}
diff --git a/java/com/google/gerrit/server/schema/Schema_180.java b/java/com/google/gerrit/server/schema/Schema_180.java
index 6912b3e..7e13d0d 100644
--- a/java/com/google/gerrit/server/schema/Schema_180.java
+++ b/java/com/google/gerrit/server/schema/Schema_180.java
@@ -14,6 +14,15 @@
package com.google.gerrit.server.schema;
+/**
+ * Schema 180 for Gerrit metadata.
+ *
+ * <p>180 is the first schema version that is supported by NoteDb. All previous schema versions were
+ * for ReviewDb. Since ReviewDb no longer exists those schema versions have been deleted.
+ *
+ * <p>Upgrading to this schema version creates the {@code refs/meta/version} ref in NoteDb that
+ * stores the number of the current schema version.
+ */
public class Schema_180 implements NoteDbSchemaVersion {
@Override
public void upgrade(Arguments args, UpdateUI ui) {
diff --git a/java/com/google/gerrit/server/schema/Schema_181.java b/java/com/google/gerrit/server/schema/Schema_181.java
index 3054ad3..d357ae7 100644
--- a/java/com/google/gerrit/server/schema/Schema_181.java
+++ b/java/com/google/gerrit/server/schema/Schema_181.java
@@ -17,10 +17,16 @@
import com.google.gerrit.gpg.PublicKeyStore;
import org.eclipse.jgit.lib.Repository;
+/**
+ * Schema 181 for Gerrit metadata.
+ *
+ * <p>Upgrading to this schema version populates the GPG subkey to master key map (see {@link
+ * PublicKeyStore}.
+ */
public class Schema_181 implements NoteDbSchemaVersion {
@Override
public void upgrade(Arguments args, UpdateUI ui) throws Exception {
- ui.message("Rebuild GPGP note map to build subkey to master key map");
+ ui.message("Rebuild GPG note map to build subkey to master key map");
try (Repository repo = args.repoManager.openRepository(args.allUsers);
PublicKeyStore store = new PublicKeyStore(repo)) {
store.rebuildSubkeyMasterKeyMap();
diff --git a/java/com/google/gerrit/server/submit/MergeSuperSet.java b/java/com/google/gerrit/server/submit/MergeSuperSet.java
index b36eb7d..f8540fb 100644
--- a/java/com/google/gerrit/server/submit/MergeSuperSet.java
+++ b/java/com/google/gerrit/server/submit/MergeSuperSet.java
@@ -18,7 +18,6 @@
import static java.util.Objects.requireNonNull;
import com.google.common.base.Strings;
-import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -58,6 +57,7 @@
public class MergeSuperSet {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final ChangeData.Factory changeDataFactory;
private final Provider<InternalChangeQuery> queryProvider;
private final Provider<MergeOpRepoManager> repoManagerProvider;
private final DynamicItem<MergeSuperSetComputation> mergeSuperSetComputation;
@@ -72,6 +72,7 @@
@Inject
MergeSuperSet(
@GerritServerConfig Config cfg,
+ ChangeData.Factory changeDataFactory,
Provider<InternalChangeQuery> queryProvider,
Provider<MergeOpRepoManager> repoManagerProvider,
DynamicItem<MergeSuperSetComputation> mergeSuperSetComputation,
@@ -79,6 +80,7 @@
ProjectCache projectCache,
ChangeNotes.Factory notesFactory) {
this.cfg = cfg;
+ this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider;
this.mergeSuperSetComputation = mergeSuperSetComputation;
@@ -105,14 +107,7 @@
orm = repoManagerProvider.get();
closeOrm = true;
}
- List<ChangeData> cds = queryProvider.get().byLegacyChangeId(change.getId());
- checkState(
- cds.size() == 1,
- "Expected exactly one ChangeData for change ID %s, got %s",
- change.getId(),
- cds.size());
- ChangeData cd = Iterables.getFirst(cds, null);
-
+ ChangeData cd = changeDataFactory.create(change.getProject(), change.getId());
boolean visible = false;
if (cd != null) {
ProjectState projectState = projectCache.checkedGet(cd.project());
diff --git a/java/gerrit/AbstractCommitUserIdentityPredicate.java b/java/gerrit/AbstractCommitUserIdentityPredicate.java
index bd8cf1a..51c4a3b 100644
--- a/java/gerrit/AbstractCommitUserIdentityPredicate.java
+++ b/java/gerrit/AbstractCommitUserIdentityPredicate.java
@@ -30,6 +30,20 @@
import java.io.IOException;
import org.eclipse.jgit.lib.PersonIdent;
+/**
+ * Abstract Prolog predicate for a Git person identity of a change.
+ *
+ * <p>Checks that the terms that are provided as input to this Prolog predicate match a Git person
+ * identity of the change (either author or committer).
+ *
+ * <p>The terms that are provided as input to this Prolog predicate are:
+ *
+ * <ul>
+ * <li>a user ID term that matches the account ID of the Git person identity
+ * <li>a string atom that matches the full name of the Git person identity
+ * <li>a string atom that matches the email of the Git person identity
+ * </ul>
+ */
abstract class AbstractCommitUserIdentityPredicate extends Predicate.P3 {
private static final SymbolTerm user = SymbolTerm.intern("user", 1);
private static final SymbolTerm anonymous = SymbolTerm.intern("anonymous");
diff --git a/java/gerrit/PRED_change_branch_1.java b/java/gerrit/PRED_change_branch_1.java
index 4501169..62744f7 100644
--- a/java/gerrit/PRED_change_branch_1.java
+++ b/java/gerrit/PRED_change_branch_1.java
@@ -23,6 +23,16 @@
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+/**
+ * Prolog predicate for the destination branch of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is a string atom that
+ * matches the destination branch of the change.
+ *
+ * <pre>
+ * 'change_branch'(-Branch)
+ * </pre>
+ */
public class PRED_change_branch_1 extends Predicate.P1 {
public PRED_change_branch_1(Term a1, Operation n) {
arg1 = a1;
diff --git a/java/gerrit/PRED_change_owner_1.java b/java/gerrit/PRED_change_owner_1.java
index d42c0e1..f6fbb80 100644
--- a/java/gerrit/PRED_change_owner_1.java
+++ b/java/gerrit/PRED_change_owner_1.java
@@ -25,6 +25,16 @@
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+/**
+ * Prolog predicate for the owner of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is a user ID term that
+ * matches the account ID of the change owner.
+ *
+ * <pre>
+ * 'change_owner'(user(-ID))
+ * </pre>
+ */
public class PRED_change_owner_1 extends Predicate.P1 {
private static final SymbolTerm user = SymbolTerm.intern("user", 1);
diff --git a/java/gerrit/PRED_change_project_1.java b/java/gerrit/PRED_change_project_1.java
index a973e1c..b2ef109 100644
--- a/java/gerrit/PRED_change_project_1.java
+++ b/java/gerrit/PRED_change_project_1.java
@@ -23,6 +23,16 @@
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+/**
+ * Prolog predicate for the project of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is a string atom that
+ * matches the project of the change.
+ *
+ * <pre>
+ * 'change_project'(-Project)
+ * </pre>
+ */
public class PRED_change_project_1 extends Predicate.P1 {
public PRED_change_project_1(Term a1, Operation n) {
arg1 = a1;
diff --git a/java/gerrit/PRED_change_topic_1.java b/java/gerrit/PRED_change_topic_1.java
index 11d737a..f0175ef 100644
--- a/java/gerrit/PRED_change_topic_1.java
+++ b/java/gerrit/PRED_change_topic_1.java
@@ -23,6 +23,16 @@
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+/**
+ * Prolog predicate for the topic of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is a string atom that
+ * matches the topic of the change.
+ *
+ * <pre>
+ * 'change_topic'(-Topic)
+ * </pre>
+ */
public class PRED_change_topic_1 extends Predicate.P1 {
public PRED_change_topic_1(Term a1, Operation n) {
arg1 = a1;
diff --git a/java/gerrit/PRED_commit_author_3.java b/java/gerrit/PRED_commit_author_3.java
index 998b30e..3381344 100644
--- a/java/gerrit/PRED_commit_author_3.java
+++ b/java/gerrit/PRED_commit_author_3.java
@@ -21,6 +21,27 @@
import com.googlecode.prolog_cafe.lang.Term;
import org.eclipse.jgit.revwalk.RevCommit;
+/**
+ * Prolog predicate for the Git author of the current patch set of a change.
+ *
+ * <p>Checks that the terms that are provided as input to this Prolog predicate match the Git author
+ * of the current patch set of the change.
+ *
+ * <p>The terms that are provided as input to this Prolog predicate are:
+ *
+ * <ul>
+ * <li>a user ID term that matches the account ID of the Git author of the current patch set of
+ * the change
+ * <li>a string atom that matches the full name of the Git author of the current patch set of the
+ * change
+ * <li>a string atom that matches the email of the Git author of the current patch set of the
+ * change
+ * </ul>
+ *
+ * <pre>
+ * 'commit_author'(user(-ID), -FullName, -Email)
+ * </pre>
+ */
public class PRED_commit_author_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_author_3(Term a1, Term a2, Term a3, Operation n) {
super(a1, a2, a3, n);
diff --git a/java/gerrit/PRED_commit_committer_3.java b/java/gerrit/PRED_commit_committer_3.java
index 293d8ce..1757336 100644
--- a/java/gerrit/PRED_commit_committer_3.java
+++ b/java/gerrit/PRED_commit_committer_3.java
@@ -21,6 +21,27 @@
import com.googlecode.prolog_cafe.lang.Term;
import org.eclipse.jgit.revwalk.RevCommit;
+/**
+ * Prolog predicate for the Git committer of the current patch set of a change.
+ *
+ * <p>Checks that the terms that are provided as input to this Prolog predicate match the Git
+ * committer of the current patch set of the change.
+ *
+ * <p>The terms that are provided as input to this Prolog predicate are:
+ *
+ * <ul>
+ * <li>a user ID term that matches the account ID of the Git committer of the current patch set of
+ * the change
+ * <li>a string atom that matches the full name of the Git committer of the current patch set of
+ * the change
+ * <li>a string atom that matches the email of the Git committer of the current patch set of the
+ * change
+ * </ul>
+ *
+ * <pre>
+ * 'commit_committer'(user(-ID), -FullName, -Email)
+ * </pre>
+ */
public class PRED_commit_committer_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_committer_3(Term a1, Term a2, Term a3, Operation n) {
super(a1, a2, a3, n);
diff --git a/java/gerrit/PRED_commit_message_1.java b/java/gerrit/PRED_commit_message_1.java
index eb996d6..3485af6 100644
--- a/java/gerrit/PRED_commit_message_1.java
+++ b/java/gerrit/PRED_commit_message_1.java
@@ -24,7 +24,10 @@
import org.eclipse.jgit.revwalk.RevCommit;
/**
- * Returns the commit message as a symbol
+ * Prolog predicate for the commit message of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is a string atom that
+ * matches the commit message of the change.
*
* <pre>
* 'commit_message'(-Msg)
diff --git a/java/gerrit/PRED_project_default_submit_type_1.java b/java/gerrit/PRED_project_default_submit_type_1.java
index d70a9e4..77a0261 100644
--- a/java/gerrit/PRED_project_default_submit_type_1.java
+++ b/java/gerrit/PRED_project_default_submit_type_1.java
@@ -24,6 +24,16 @@
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+/**
+ * Prolog predicate for the default submit type of the project of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is a string atom that
+ * matches the default submit type of the change's project.
+ *
+ * <pre>
+ * 'project_default_submit_type'(-SubmitType)
+ * </pre>
+ */
public class PRED_project_default_submit_type_1 extends Predicate.P1 {
private static final SymbolTerm[] term;
diff --git a/java/gerrit/PRED_pure_revert_1.java b/java/gerrit/PRED_pure_revert_1.java
index 6300a668..19e7b68 100644
--- a/java/gerrit/PRED_pure_revert_1.java
+++ b/java/gerrit/PRED_pure_revert_1.java
@@ -22,7 +22,17 @@
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
-/** Checks if change is a pure revert of the change it references in 'revertOf'. */
+/**
+ * Prolog Predicate that checks if the change is a pure revert of the change it references in
+ * 'revertOf'.
+ *
+ * <p>The input is an integer atom where '1' represents a pure revert and '0' represents a non-pure
+ * revert.
+ *
+ * <pre>
+ * 'pure_revert'(-PureRevert)
+ * </pre>
+ */
public class PRED_pure_revert_1 extends Predicate.P1 {
public PRED_pure_revert_1(Term a1, Operation n) {
arg1 = a1;
diff --git a/java/gerrit/PRED_unresolved_comments_count_1.java b/java/gerrit/PRED_unresolved_comments_count_1.java
index d4abcc54..9a1fcca 100644
--- a/java/gerrit/PRED_unresolved_comments_count_1.java
+++ b/java/gerrit/PRED_unresolved_comments_count_1.java
@@ -22,6 +22,16 @@
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
+/**
+ * Prolog predicate for the number of unresolved comments of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is an integer atom
+ * that matches the number of unresolved comments of the change.
+ *
+ * <pre>
+ * 'unresolved_comments_count'(-NumberOfUnresolvedComments)
+ * </pre>
+ */
public class PRED_unresolved_comments_count_1 extends Predicate.P1 {
public PRED_unresolved_comments_count_1(Term a1, Operation n) {
arg1 = a1;
diff --git a/java/gerrit/PRED_uploader_1.java b/java/gerrit/PRED_uploader_1.java
index 681d86c..89e367e 100644
--- a/java/gerrit/PRED_uploader_1.java
+++ b/java/gerrit/PRED_uploader_1.java
@@ -27,6 +27,16 @@
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+/**
+ * Prolog predicate for the uploader of the current patch set of a change.
+ *
+ * <p>Checks that the term that is provided as input to this Prolog predicate is a user ID term that
+ * matches the account ID of the uploader of the current patch set.
+ *
+ * <pre>
+ * 'uploader'(user(-ID))
+ * </pre>
+ */
public class PRED_uploader_1 extends Predicate.P1 {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
index d7e765b..a442ddd 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
@@ -21,7 +21,6 @@
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
-import com.google.gerrit.extensions.client.Theme;
import org.junit.Test;
@NoHttpd
@@ -43,7 +42,6 @@
i.fontSize *= -1;
i.lineLength *= -1;
i.cursorBlinkRate = 500;
- i.theme = Theme.MIDNIGHT;
i.ignoreWhitespace = Whitespace.IGNORE_ALL;
i.expandAllComments ^= true;
i.intralineDifference ^= true;
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
index 00d1733..f142c08 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
@@ -19,8 +19,6 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.extensions.client.EditPreferencesInfo;
-import com.google.gerrit.extensions.client.KeyMapType;
-import com.google.gerrit.extensions.client.Theme;
import org.junit.Test;
@NoHttpd
@@ -43,8 +41,6 @@
assertThat(out.indentWithTabs).isNull();
assertThat(out.autoCloseBrackets).isNull();
assertThat(out.showBase).isNull();
- assertThat(out.theme).isEqualTo(Theme.DEFAULT);
- assertThat(out.keyMapType).isEqualTo(KeyMapType.DEFAULT);
// change some default values
out.lineLength = 80;
@@ -61,8 +57,6 @@
out.indentWithTabs = true;
out.autoCloseBrackets = true;
out.showBase = true;
- out.theme = Theme.TWILIGHT;
- out.keyMapType = KeyMapType.EMACS;
EditPreferencesInfo info = gApi.accounts().id(admin.id().toString()).setEditPreferences(out);
@@ -94,7 +88,5 @@
assertThat(out.indentWithTabs).isEqualTo(in.indentWithTabs);
assertThat(out.autoCloseBrackets).isEqualTo(in.autoCloseBrackets);
assertThat(out.showBase).isEqualTo(in.showBase);
- assertThat(out.theme).isEqualTo(in.theme);
- assertThat(out.keyMapType).isEqualTo(in.keyMapType);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
index 3b106a4..6717fb7 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
@@ -27,7 +27,6 @@
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat;
@@ -69,7 +68,6 @@
// change all default values
i.changesPerPage *= -1;
- i.downloadCommand = DownloadCommand.REPO_DOWNLOAD;
i.dateFormat = DateFormat.US;
i.timeFormat = TimeFormat.HHMM_24;
i.emailStrategy = EmailStrategy.DISABLED;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 7703b1c..fdf3758 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -693,42 +693,60 @@
@Test
public void revertSubmissionWithSetMessage() throws Exception {
- String result = createChange("change", "a.txt", "message").getChangeId();
- gApi.changes().id(result).current().review(ReviewInput.approve());
- gApi.changes().id(result).current().submit();
+ String firstResult = createChange("first change", "a.txt", "message").getChangeId();
+ String secondResult = createChange("second change", "b.txt", "message").getChangeId();
+ approve(firstResult);
+ approve(secondResult);
+ gApi.changes().id(secondResult).current().submit();
RevertInput revertInput = new RevertInput();
String commitMessage = "Message from input";
revertInput.message = commitMessage;
- ChangeInfo revertChange =
- gApi.changes().id(result).revertSubmission(revertInput).revertChanges.get(0);
- assertThat(revertChange.subject).isEqualTo("Revert \"change\"");
- assertThat(gApi.changes().id(revertChange.id).current().commit(false).message)
+ List<ChangeInfo> revertChanges =
+ gApi.changes().id(firstResult).revertSubmission(revertInput).revertChanges;
+ assertThat(revertChanges.get(0).subject).isEqualTo("Revert \"first change\"");
+ assertThat(gApi.changes().id(revertChanges.get(0).id).current().commit(false).message)
.isEqualTo(
String.format(
- "Revert \"change\"\n\n%s\n\nChange-Id: %s\n",
- commitMessage, revertChange.changeId));
+ "Revert \"first change\"\n\n%s\n\nChange-Id: %s\n",
+ commitMessage, revertChanges.get(0).changeId));
+ assertThat(revertChanges.get(1).subject).isEqualTo("Revert \"second change\"");
+ assertThat(gApi.changes().id(revertChanges.get(1).id).current().commit(false).message)
+ .isEqualTo(
+ String.format(
+ "Revert \"second change\"\n\n%s\n\nChange-Id: %s\n",
+ commitMessage, revertChanges.get(1).changeId));
}
@Test
public void revertSubmissionWithoutMessage() throws Exception {
- String result = createChange("change", "a.txt", "message").getChangeId();
- gApi.changes().id(result).current().review(ReviewInput.approve());
- gApi.changes().id(result).current().submit();
+ String firstResult = createChange("first change", "a.txt", "message").getChangeId();
+ String secondResult = createChange("second change", "b.txt", "message").getChangeId();
+ approve(firstResult);
+ approve(secondResult);
+ gApi.changes().id(secondResult).current().submit();
RevertInput revertInput = new RevertInput();
- ChangeInfo revertChange =
- gApi.changes().id(result).revertSubmission(revertInput).revertChanges.get(0);
- assertThat(revertChange.subject).isEqualTo("Revert \"change\"");
- assertThat(gApi.changes().id(revertChange.id).current().commit(false).message)
+ List<ChangeInfo> revertChanges =
+ gApi.changes().id(firstResult).revertSubmission(revertInput).revertChanges;
+ assertThat(revertChanges.get(0).subject).isEqualTo("Revert \"first change\"");
+ assertThat(gApi.changes().id(revertChanges.get(0).id).current().commit(false).message)
.isEqualTo(
String.format(
- "Revert \"change\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n",
- gApi.changes().id(result).get().currentRevision, revertChange.changeId));
+ "Revert \"first change\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n",
+ gApi.changes().id(firstResult).get().currentRevision,
+ revertChanges.get(0).changeId));
+ assertThat(revertChanges.get(1).subject).isEqualTo("Revert \"second change\"");
+ assertThat(gApi.changes().id(revertChanges.get(1).id).current().commit(false).message)
+ .isEqualTo(
+ String.format(
+ "Revert \"second change\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n",
+ gApi.changes().id(secondResult).get().currentRevision,
+ revertChanges.get(1).changeId));
}
@Test
public void revertSubmissionRevertsChangeWithLongSubject() throws Exception {
String changeTitle =
- "This change has a very long title and therefore it will be cut to 50 characters when the"
+ "This change has a very long title and therefore it will be cut to 56 characters when the"
+ " revert change will revert this change";
String result = createChange(changeTitle, "a.txt", "message").getChangeId();
gApi.changes().id(result).current().review(ReviewInput.approve());
@@ -737,12 +755,12 @@
ChangeInfo revertChange =
gApi.changes().id(result).revertSubmission(revertInput).revertChanges.get(0);
assertThat(revertChange.subject)
- .isEqualTo(String.format("Revert \"%s...\"", changeTitle.substring(0, 59)));
+ .isEqualTo(String.format("Revert \"%s...\"", changeTitle.substring(0, 56)));
assertThat(gApi.changes().id(revertChange.id).current().commit(false).message)
.isEqualTo(
String.format(
"Revert \"%s...\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n",
- changeTitle.substring(0, 59),
+ changeTitle.substring(0, 56),
gApi.changes().id(result).get().currentRevision,
revertChange.changeId));
}
@@ -766,8 +784,16 @@
}
// submit all changes
gApi.changes().id(resultCommits.get(1).getChangeId()).current().submit();
- List<ChangeApi> revertChanges =
- getChangeApis(gApi.changes().id(resultCommits.get(1).getChangeId()).revertSubmission());
+ RevertSubmissionInfo revertSubmissionInfo =
+ gApi.changes().id(resultCommits.get(1).getChangeId()).revertSubmission();
+ assertThat(
+ revertSubmissionInfo.revertChanges.stream()
+ .map(change -> change.created)
+ .distinct()
+ .count())
+ .isEqualTo(1);
+
+ List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
assertThat(revertChanges).hasSize(3);
@@ -835,8 +861,16 @@
approve(secondResult.getChangeId());
approve(firstResult.getChangeId());
gApi.changes().id(secondResult.getChangeId()).current().submit();
- List<ChangeApi> revertChanges =
- getChangeApis(gApi.changes().id(firstResult.getChangeId()).revertSubmission());
+ RevertSubmissionInfo revertSubmissionInfo =
+ gApi.changes().id(firstResult.getChangeId()).revertSubmission();
+ assertThat(
+ revertSubmissionInfo.revertChanges.stream()
+ .map(change -> change.created)
+ .distinct()
+ .count())
+ .isEqualTo(1);
+
+ List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
Collections.reverse(revertChanges);
String sha1SecondChange = secondResult.getCommit().getName();
String sha1FirstRevert = revertChanges.get(0).current().commit(false).commit;
@@ -900,8 +934,16 @@
approve(firstResult.getChangeId());
// submit both changes
gApi.changes().id(secondResult.getChangeId()).current().submit();
- List<ChangeApi> revertChanges =
- getChangeApis(gApi.changes().id(secondResult.getChangeId()).revertSubmission());
+ RevertSubmissionInfo revertSubmissionInfo =
+ gApi.changes().id(secondResult.getChangeId()).revertSubmission();
+ assertThat(
+ revertSubmissionInfo.revertChanges.stream()
+ .map(change -> change.created)
+ .distinct()
+ .count())
+ .isEqualTo(1);
+
+ List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
// has size 2 because of the same topic, and submitWholeTopic is true.
assertThat(gApi.changes().id(revertChanges.get(0).get()._number).submittedTogether())
.hasSize(2);
@@ -937,8 +979,15 @@
}
// submit all changes
gApi.changes().id(resultCommits.get(1).getChangeId()).current().submit();
- List<ChangeApi> revertChanges =
- getChangeApis(gApi.changes().id(resultCommits.get(1).getChangeId()).revertSubmission());
+ RevertSubmissionInfo revertSubmissionInfo =
+ gApi.changes().id(resultCommits.get(1).getChangeId()).revertSubmission();
+ assertThat(
+ revertSubmissionInfo.revertChanges.stream()
+ .map(change -> change.created)
+ .distinct()
+ .count())
+ .isEqualTo(1);
+ List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
assertThat(revertChanges.get(0).current().files().get("c.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(1).current().files().get("a.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(2).current().files().get("b.txt").linesDeleted).isEqualTo(1);
@@ -984,8 +1033,15 @@
approve(fourthResult.getChangeId());
gApi.changes().id(fourthResult.getChangeId()).current().submit();
- List<ChangeApi> revertChanges =
- getChangeApis(gApi.changes().id(secondResult.getChangeId()).revertSubmission());
+ RevertSubmissionInfo revertSubmissionInfo =
+ gApi.changes().id(secondResult.getChangeId()).revertSubmission();
+ assertThat(
+ revertSubmissionInfo.revertChanges.stream()
+ .map(change -> change.created)
+ .distinct()
+ .count())
+ .isEqualTo(1);
+ List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
Collections.reverse(revertChanges);
assertThat(revertChanges.get(0).current().files().get("c.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(1).current().files().get("b.txt").linesDeleted).isEqualTo(1);
@@ -1034,8 +1090,15 @@
approve(fourthResult.getChangeId());
gApi.changes().id(fourthResult.getChangeId()).current().submit();
- List<ChangeApi> revertChanges =
- getChangeApis(gApi.changes().id(secondResult.getChangeId()).revertSubmission());
+ RevertSubmissionInfo revertSubmissionInfo =
+ gApi.changes().id(secondResult.getChangeId()).revertSubmission();
+ assertThat(
+ revertSubmissionInfo.revertChanges.stream()
+ .map(change -> change.created)
+ .distinct()
+ .count())
+ .isEqualTo(1);
+ List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
Collections.reverse(revertChanges);
assertThat(revertChanges.get(0).current().files().get("c.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(1).current().files().get("b.txt").linesDeleted).isEqualTo(1);
@@ -1055,6 +1118,56 @@
assertThat(gApi.changes().id(revertChanges.get(1).id()).current().related().changes).hasSize(3);
}
+ @Test
+ public void revertSubmissionSubjects() throws Exception {
+ String firstResult = createChange("first change", "a.txt", "message").getChangeId();
+ String secondResult = createChange("second change", "b.txt", "other").getChangeId();
+ approve(firstResult);
+ approve(secondResult);
+ gApi.changes().id(secondResult).current().submit();
+
+ List<ChangeApi> firstRevertChanges =
+ getChangeApis(gApi.changes().id(firstResult).revertSubmission());
+ assertThat(firstRevertChanges.get(0).get().subject).isEqualTo("Revert \"first change\"");
+ assertThat(firstRevertChanges.get(1).get().subject).isEqualTo("Revert \"second change\"");
+ approve(firstRevertChanges.get(0).id());
+ approve(firstRevertChanges.get(1).id());
+ gApi.changes().id(firstRevertChanges.get(0).id()).current().submit();
+
+ List<ChangeApi> secondRevertChanges =
+ getChangeApis(gApi.changes().id(firstRevertChanges.get(0).id()).revertSubmission());
+ assertThat(secondRevertChanges.get(0).get().subject).isEqualTo("Revert^2 \"second change\"");
+ assertThat(secondRevertChanges.get(1).get().subject).isEqualTo("Revert^2 \"first change\"");
+ approve(secondRevertChanges.get(0).id());
+ approve(secondRevertChanges.get(1).id());
+ gApi.changes().id(secondRevertChanges.get(0).id()).current().submit();
+
+ List<ChangeApi> thirdRevertChanges =
+ getChangeApis(gApi.changes().id(secondRevertChanges.get(0).id()).revertSubmission());
+ assertThat(thirdRevertChanges.get(0).get().subject).isEqualTo("Revert^3 \"first change\"");
+ assertThat(thirdRevertChanges.get(1).get().subject).isEqualTo("Revert^3 \"second change\"");
+ }
+
+ @Test
+ public void revertSubmissionWithUserChangedSubjects() throws Exception {
+ String firstResult = createChange("Revert^aa", "a.txt", "message").getChangeId();
+ String secondResult = createChange("Revert", "b.txt", "other").getChangeId();
+ String thirdResult = createChange("Revert^934 \"change x\"", "c.txt", "another").getChangeId();
+ String fourthResult = createChange("Revert^934", "d.txt", "last").getChangeId();
+ approve(firstResult);
+ approve(secondResult);
+ approve(thirdResult);
+ approve(fourthResult);
+ gApi.changes().id(fourthResult).current().submit();
+
+ List<ChangeApi> firstRevertChanges =
+ getChangeApis(gApi.changes().id(firstResult).revertSubmission());
+ assertThat(firstRevertChanges.get(0).get().subject).isEqualTo("Revert \"Revert^aa\"");
+ assertThat(firstRevertChanges.get(1).get().subject).isEqualTo("Revert \"Revert\"");
+ assertThat(firstRevertChanges.get(2).get().subject).isEqualTo("Revert^935 \"change x\"");
+ assertThat(firstRevertChanges.get(3).get().subject).isEqualTo("Revert \"Revert^934\"");
+ }
+
@Override
protected PushOneCommit.Result createChange() throws Exception {
return createChange("refs/for/master");
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index d4a4c45..717d3cc 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -21,6 +21,7 @@
import static com.google.gerrit.extensions.common.testing.DiffInfoSubject.assertThat;
import static com.google.gerrit.extensions.common.testing.FileInfoSubject.assertThat;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toMap;
@@ -41,6 +42,7 @@
import com.google.gerrit.extensions.common.ChangeType;
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testing.ConfigSuite;
@@ -2718,6 +2720,23 @@
assertThat(diffInfo).content().element(0).numberOfSkippedLines().isGreaterThan(0);
}
+ @Test
+ public void editNotAllowedAsBase() throws Exception {
+ gApi.changes().id(changeId).edit().create();
+
+ BadRequestException e =
+ assertThrows(
+ BadRequestException.class,
+ () -> getDiffRequest(changeId, CURRENT, FILE_NAME).withBase("edit").get());
+ assertThat(e).hasMessageThat().isEqualTo("edit not allowed as base");
+
+ e =
+ assertThrows(
+ BadRequestException.class,
+ () -> getDiffRequest(changeId, CURRENT, FILE_NAME).withBase("0").get());
+ assertThat(e).hasMessageThat().isEqualTo("edit not allowed as base");
+ }
+
private static CommentInput createCommentInput(
int startLine, int startCharacter, int endLine, int endCharacter, String message) {
CommentInput comment = new CommentInput();
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index bf93d58..b0f183e 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -520,6 +520,14 @@
}
@Test
+ public void changeEditNoContentProvidedRest() throws Exception {
+ createEmptyEditFor(changeId);
+ adminRestSession
+ .put(urlEditFile(changeId, FILE_NAME), new FileContentInput())
+ .assertBadRequest();
+ }
+
+ @Test
public void emptyPutRequest() throws Exception {
createEmptyEditFor(changeId);
adminRestSession.put(urlEditFile(changeId, FILE_NAME)).assertNoContent();
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index c692a3b..c67a842 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -51,7 +51,7 @@
case V6_7:
return "blacktop/elasticsearch:6.7.2";
case V6_8:
- return "blacktop/elasticsearch:6.8.5";
+ return "blacktop/elasticsearch:6.8.6";
case V7_0:
return "blacktop/elasticsearch:7.0.1";
case V7_1:
@@ -63,7 +63,7 @@
case V7_4:
return "blacktop/elasticsearch:7.4.2";
case V7_5:
- return "blacktop/elasticsearch:7.5.0";
+ return "blacktop/elasticsearch:7.5.1";
}
throw new IllegalStateException("No tests for version: " + version.name());
}
diff --git a/javatests/com/google/gerrit/server/config/ConfigUtilTest.java b/javatests/com/google/gerrit/server/config/ConfigUtilTest.java
index 865bda6..035878c 100644
--- a/javatests/com/google/gerrit/server/config/ConfigUtilTest.java
+++ b/javatests/com/google/gerrit/server/config/ConfigUtilTest.java
@@ -22,7 +22,6 @@
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
-import com.google.gerrit.extensions.client.Theme;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -48,8 +47,6 @@
public String s;
public String sd;
public String nd;
- public Theme t;
- public Theme td;
public List<String> list;
public Map<String, String> map;
@@ -67,8 +64,6 @@
i.s = "foo";
i.sd = "bar";
// i.nd = null; // Don't need to explicitly set it; it's null by default
- i.t = Theme.DEFAULT;
- i.td = Theme.DEFAULT;
return i;
}
}
@@ -86,7 +81,6 @@
in.bb = true;
in.bd = false;
in.s = "baz";
- in.t = Theme.MIDNIGHT;
Config cfg = new Config();
ConfigUtil.storeSection(cfg, SECT, SUB, in, d);
@@ -117,8 +111,6 @@
assertThat(out.s).isEqualTo(in.s);
assertThat(out.sd).isEqualTo(d.sd);
assertThat(out.nd).isNull();
- assertThat(out.t).isEqualTo(in.t);
- assertThat(out.td).isEqualTo(d.td);
}
@Test
diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
index c7ca887..083493d 100644
--- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
+++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
@@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
@@ -35,10 +36,13 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.server.patch.DiffSummary;
+import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.InMemoryTestEnvironment;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.name.Named;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@@ -57,8 +61,9 @@
new InMemoryTestEnvironment(
() -> {
Config cfg = new Config();
- cfg.setInt("change", null, "maxUpdates", MAX_UPDATES);
+ cfg.setInt("change", null, "maxFiles", 2);
cfg.setInt("change", null, "maxPatchSets", MAX_PATCH_SETS);
+ cfg.setInt("change", null, "maxUpdates", MAX_UPDATES);
return cfg;
});
@@ -70,6 +75,9 @@
@Inject private Provider<CurrentUser> user;
@Inject private Sequences sequences;
+ @Inject
+ private @Named("diff_summary") Cache<DiffSummaryKey, DiffSummary> diffSummaryCache;
+
private Project.NameKey project;
private TestRepository<Repository> repo;
@@ -243,6 +251,59 @@
assertThat(getMetaId(changeId)).isEqualTo(oldMetaId);
}
+ @Test
+ public void limitFileCount_exceed() throws Exception {
+ Change.Id changeId = createChangeWithUpdates(1);
+ ChangeNotes notes = changeNotesFactory.create(project, changeId);
+
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId())
+ .add("bar.txt", "bar")
+ .add("baz.txt", "baz")
+ .add("boom.txt", "boom")
+ .message("blah")
+ .create();
+ bu.addOp(
+ changeId,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(changeId, 2), commitId)
+ .setMessage("blah"));
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Exceeding maximum number of files per change (3 > 2)");
+ }
+ }
+
+ @Test
+ public void limitFileCount_cacheKeyMatches() throws Exception {
+ Change.Id changeId = createChangeWithUpdates(1);
+ ChangeNotes notes = changeNotesFactory.create(project, changeId);
+
+ int cacheSizeBefore = diffSummaryCache.asMap().size();
+
+ // We don't want to depend on the test helper used above so we perform an explicit commit here.
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId())
+ .add("bar.txt", "bar")
+ .add("baz.txt", "baz")
+ .message("blah")
+ .create();
+ bu.addOp(
+ changeId,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(changeId, 3), commitId)
+ .setMessage("blah"));
+ bu.execute();
+ }
+
+ // Assert that we only performed the diff computation once. This would e.g. catch
+ // bugs/deviations in the computation of the cache key.
+ assertThat(diffSummaryCache.asMap()).hasSize(cacheSizeBefore + 1);
+ }
+
private Change.Id createChangeWithUpdates(int totalUpdates) throws Exception {
checkArgument(totalUpdates > 0);
checkArgument(totalUpdates <= MAX_UPDATES);
diff --git a/plugins/plugin-manager b/plugins/plugin-manager
index 828d666..2933add 160000
--- a/plugins/plugin-manager
+++ b/plugins/plugin-manager
@@ -1 +1 @@
-Subproject commit 828d666bbb4aae1a2c348a12d7855ec5db3be46f
+Subproject commit 2933add62ecf2cbfc28cfe2cff81ff0e0eecc913
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index b596de8..e21dbe2 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -61,17 +61,6 @@
),
)
-filegroup(
- name = "bower_components",
- srcs = glob(
- [
- "bower_components/**/*.html",
- "bower_components/**/*.js",
- "bower_components/**/*.js.map",
- ],
- ),
-)
-
genrule2(
name = "pg_code_zip",
srcs = [":pg_code"],
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.js b/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.js
index f11e5ed..dfcacb4 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.js
@@ -76,9 +76,9 @@
}
/**
- * @param {?boolean} value - fallback to false if undefined
+ * @param {string} value - fallback to 'false' if undefined
*/
- _computeChecked(value = false) {
+ _computeChecked(value = 'false') {
return JSON.parse(value);
}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-abandon-dialog/gr-confirm-abandon-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-abandon-dialog/gr-confirm-abandon-dialog.html
index 05a2bb2..9e7857c4 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-abandon-dialog/gr-confirm-abandon-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-abandon-dialog/gr-confirm-abandon-dialog.html
@@ -45,14 +45,7 @@
font-family: var(--monospace-font-family);
font-size: var(--font-size-mono);
line-height: var(--line-height-mono);
- padding: 0;
width: 73ch; /* Add a char to account for the border. */
-
- --iron-autogrow-textarea {
- border: 1px solid var(--border-color);
- box-sizing: border-box;
- font-family: var(--monospace-font-family);
- }
}
</style>
<gr-dialog
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
index 8ddcd83..cab9fd6 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
@@ -47,18 +47,11 @@
display: block;
width: 100%;
}
- .main .message {
- width: 100%;
- }
iron-autogrow-textarea {
font-family: var(--monospace-font-family);
font-size: var(--font-size-mono);
line-height: var(--line-height-mono);
- padding: 0;
- --iron-autogrow-textarea: {
- font-family: var(--monospace-font-family);
- width: 72ch;
- };
+ width: 73ch; /* Add a char to account for the border. */
}
</style>
<gr-dialog
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.html
index 24c1132..f65ec03 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-move-dialog/gr-confirm-move-dialog.html
@@ -37,9 +37,6 @@
label {
cursor: pointer;
}
- iron-autogrow-textarea {
- padding: 0;
- }
.main {
display: flex;
flex-direction: column;
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 2e1e6ae..fc3a8c1 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
@@ -41,14 +41,7 @@
font-family: var(--monospace-font-family);
font-size: var(--font-size-mono);
line-height: var(--line-height-mono);
- padding: 0;
width: 73ch; /* Add a char to account for the border. */
-
- --iron-autogrow-textarea {
- border: 1px solid var(--border-color);
- box-sizing: border-box;
- font-family: var(--monospace-font-family);
- }
}
</style>
<gr-dialog
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.html
index 0107c33..f2cfef8 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.html
@@ -40,14 +40,9 @@
}
iron-autogrow-textarea {
font-family: var(--monospace-font-family);
- padding: 0;
+ font-size: var(--font-size-mono);
+ line-height: var(--line-height-mono);
width: 73ch; /* Add a char to account for the border. */
-
- --iron-autogrow-textarea {
- border: 1px solid var(--border-color);
- box-sizing: border-box;
- font-family: var(--monospace-font-family);
- }
}
</style>
<gr-dialog
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
index 46e7d32..403d9d5 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
@@ -398,4 +398,4 @@
assert.equal(element._registerText, 'Sign up');
});
});
- </script>
\ No newline at end of file
+ </script>
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index 5c98281..889f1f3 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -436,8 +436,13 @@
// Finalize the interval. Either from a registered start mark or
// the navigation start time (if baseTime is 0).
- const startMark = baseTime === 0 ? undefined : `${name}-start`;
- window.performance.measure(name, startMark);
+ if (baseTime !== 0) {
+ window.performance.measure(name, `${name}-start`);
+ } else {
+ // Microsft Edge does not handle the 2nd param correctly
+ // (if undefined).
+ window.performance.measure(name);
+ }
},
/**
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
index a96cf44..afd0e59 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
@@ -43,10 +43,6 @@
display: block;
font-family: var(--font-family);
padding: var(--spacing-m);
- --iron-autogrow-textarea: {
- box-sizing: border-box;
- padding: 2px;
- };
}
:host([disabled]) {
pointer-events: none;
diff --git a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.html b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.html
index 62ab307..e986ad0 100644
--- a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.html
+++ b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.html
@@ -45,14 +45,7 @@
font-family: var(--monospace-font-family);
font-size: var(--font-size-mono);
line-height: var(--line-height-mono);
- padding: 0;
width: 73ch; /* Add a char to account for the border. */
-
- --iron-autogrow-textarea {
- border: 1px solid var(--border-color);
- box-sizing: border-box;
- font-family: var(--monospace-font-family);
- }
}
</style>
<gr-dialog
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html
index 45ddfc8..950be2a 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html
@@ -41,9 +41,11 @@
background-color: var(--view-background-color);
width: 100%;
+ /* You have to also repeat everything from shared-styles here, because
+ you can only *replace* --iron-autogrow-textarea vars as a whole. */
--iron-autogrow-textarea: {
- padding: var(--spacing-m);
box-sizing: border-box;
+ padding: var(--spacing-m);
overflow-y: hidden;
white-space: pre;
};
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.html b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.html
index 6803eb9..42a4f3b 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.html
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.html
@@ -59,11 +59,7 @@
visibility: visible;
white-space: normal;
}
- /*This is needed to not add a scroll bar on the side of gr-textarea
- since there is 2px of padding in iron-autogrow-textarea for the
- native textarea*/
iron-autogrow-textarea {
- padding: 2px;
position: relative;
/** This is needed for firefox */
diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index e1304e6..93c662a 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -19,10 +19,9 @@
],
language = "ECMASCRIPT_2017",
deps = [name + "_closure_lib"],
+ dependency_mode = "PRUNE_LEGACY",
)
- # TODO(davido): Remove JSC_REFERENCE_BEFORE_DECLARE when this is fixed upstream:
- # https://github.com/Polymer/polymer-resin/issues/7
closure_js_library(
name = name + "_closure_lib",
srcs = [appName + ".js"],
@@ -31,9 +30,7 @@
# and remove this supression
suppress = [
"JSC_JSDOC_MISSING_TYPE_WARNING",
- "JSC_REFERENCE_BEFORE_DECLARE",
"JSC_UNNECESSARY_ESCAPE",
- "JSC_UNUSED_LOCAL_ASSIGNMENT",
],
deps = [
"//lib/polymer_externs:polymer_closure",
diff --git a/polygerrit-ui/app/styles/gr-form-styles.html b/polygerrit-ui/app/styles/gr-form-styles.html
index 7c9ae0d..5133051 100644
--- a/polygerrit-ui/app/styles/gr-form-styles.html
+++ b/polygerrit-ui/app/styles/gr-form-styles.html
@@ -93,15 +93,8 @@
width: 100%;
}
.gr-form-styles iron-autogrow-textarea {
- border: none;
height: auto;
min-height: 4em;
- --iron-autogrow-textarea: {
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- box-sizing: border-box;
- padding: var(--spacing-s);
- }
}
.gr-form-styles gr-autocomplete {
width: 14em;
diff --git a/polygerrit-ui/app/styles/shared-styles.html b/polygerrit-ui/app/styles/shared-styles.html
index 51b92e1..ce6bead 100644
--- a/polygerrit-ui/app/styles/shared-styles.html
+++ b/polygerrit-ui/app/styles/shared-styles.html
@@ -53,13 +53,15 @@
color: var(--primary-text-color);
border: 1px solid var(--border-color);
border-radius: var(--border-radius);
+ padding: 0;
box-sizing: border-box;
/* iron-autogrow-textarea has a "-webkit-appearance: textarea" :host
css rule, which prevents overriding the border color. Clear that. */
-webkit-appearance: none;
--iron-autogrow-textarea: {
- padding: 4px;
+ box-sizing: border-box;
+ padding: var(--spacing-s);
};
}
a {
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.html b/polygerrit-ui/app/styles/themes/dark-theme.html
index cfc1f62..80243dc 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.html
+++ b/polygerrit-ui/app/styles/themes/dark-theme.html
@@ -98,8 +98,8 @@
--light-rebased-add-highlight-color: #487165;
--light-remove-add-highlight-color: #2f3f2f;
--light-remove-highlight-color: #320404;
- --coverage-covered: #e0f2f1;
- --coverage-not-covered: #ffd1a4;
+ --coverage-covered: #112826;
+ --coverage-not-covered: #6b3600;
/* syntax colors */
--syntax-attr-color: #80cbbf;
diff --git a/resources/com/google/gerrit/server/change/ChangeMessages.properties b/resources/com/google/gerrit/server/change/ChangeMessages.properties
index ec20677..3763d8e 100644
--- a/resources/com/google/gerrit/server/change/ChangeMessages.properties
+++ b/resources/com/google/gerrit/server/change/ChangeMessages.properties
@@ -1,4 +1,7 @@
revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}.
+revertSubmissionDefaultMessage = This reverts commit {0}.
+revertSubmissionUserMessage = Revert \"{0}\"\n\n{1}
+revertSubmissionOfRevertSubmissionUserMessage = Revert^{0} \"{1}\"\n\n{2}
reviewerCantSeeChange = {0} does not have permission to see this change
reviewerInvalid = {0} is not a valid user identifier
diff --git a/tools/bzl/js.bzl b/tools/bzl/js.bzl
index 8805614e..b14a761 100644
--- a/tools/bzl/js.bzl
+++ b/tools/bzl/js.bzl
@@ -494,6 +494,7 @@
deps = [
name + "_closure_lib",
],
+ dependency_mode = "PRUNE_LEGACY",
)
if html_plugin:
diff --git a/tools/bzl/maven_jar.bzl b/tools/bzl/maven_jar.bzl
index 0b4a52a..2f5447b 100644
--- a/tools/bzl/maven_jar.bzl
+++ b/tools/bzl/maven_jar.bzl
@@ -94,6 +94,7 @@
srcjar_attr = 'srcjar = "%s",' % srcjar
contents = """
{header}
+load("@rules_java//java:defs.bzl", "java_import")
package(default_visibility = ['//visibility:public'])
java_import(
name = 'jar',
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index e16c0a0..fa8008c 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -102,8 +102,8 @@
# and httpasyncclient as necessary.
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.0",
- sha1 = "62535b6fc3a4e943e88e7640eac22e29f03a696d",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.1",
+ sha1 = "094c155906dc94146fc5adc344ea2c676d487cf2",
)
maven_jar(
