Merge "Add a sample ref-updated hook to repack geometrically"
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 857725d..b7493a3 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -133,7 +133,7 @@
----
[label "Verified"]
- function = MaxWithBlock
+ function = NoBlock
value = -1 Fails
value = 0 No score
value = +1 Verified
@@ -159,6 +159,23 @@
+
*Any +1 enables submit.*
+Set the function to "NoBlock" to enable configuring submit-requirements.
+All other possible label function values are deprecated. The default is still
+"MaxWithBlock" which doesn't allow using the more flexible submit-requirements.
+
+Add a submit-requirement for the "Verified" label to define which
+conditions are required to make the change submittable:
+
+----
+ [submit-requirement "Verified"]
+ submittableIf = label:Verified=MAX AND -label:Verified=MIN
+ applicableIf = -branch:refs/meta/config
+----
+
+See the
+link:config-submit-requirements.html#examples[submit-requirements
+documentation] for more details.
+
For a change to be submittable, the change must have a `+1 Verified`
in this label, and no `-1 Fails`. Thus, `-1 Fails` can block a submit,
while `+1 Verified` enables a submit.
@@ -568,12 +585,25 @@
----
[label "Copyright-Check"]
- function = MaxWithBlock
+ function = NoBlock
value = -1 Do not have copyright
value = 0 No score
value = +1 Copyright clear
----
+Add a submit-requirement for the "Copyright-Check" label to define which
+score is required to make the change submittable:
+
+----
+ [submit-requirement "Copyright-Check"]
+ submittableIf = label:Copyright-Check=MAX AND -label:Copyright-Check=MIN
+ applicableIf = -branch:refs/meta/config
+----
+
+See the
+link:config-submit-requirements.html#examples[submit-requirements
+documentation] for more details.
+
The new column will appear at the end of the table, and `-1 Do not have
copyright` will block submit, while `+1 Copyright clear` is required to
enable submit.
diff --git a/Documentation/pgm-reindex.txt b/Documentation/pgm-reindex.txt
index 183c132..2946d44a 100644
--- a/Documentation/pgm-reindex.txt
+++ b/Documentation/pgm-reindex.txt
@@ -44,6 +44,16 @@
populated disk caches on large Gerrit sites, it is recommended that
bloom filters are disabled to improve performance.
+--reuse::
+ Reuse the change documents that already exist instead of
+ recreating the whole index from scratch. Each existing document in
+ the index will be checked and reindexed if found to be stale.
+
+ NOTE: Only supported when reindexing changes.
+
+ Use this option if offline reindexing is restarted or crashed.
+ Without this option a restart recreates the complete index
+ from scratch without reusing existing index documents.
== CONTEXT
The secondary index must be enabled. See
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 0305aaa..c199d82 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2751,7 +2751,7 @@
}
----
-[[set-work-in-pogress]]
+[[set-work-in-progress]]
=== Set Work-In-Progress
--
'POST /changes/link:#change-id[\{change-id\}]/wip'
@@ -3634,6 +3634,9 @@
Rebases change edit on top of latest patch set.
+Optionally, input parameters may be specified in the request body as a
+link:#rebase-change-edit-input[RebaseChangeEditInput] entity.
+
If one of the secondary emails associated with the user performing the operation was used as the
committer email in the latest patch set, the same email will be used as the committer email in the
new change edit commit; otherwise, the user's preferred email will be used.
@@ -3643,16 +3646,48 @@
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit:rebase HTTP/1.0
----
-When change was rebased on top of latest patch set, response
-"`204 No Content`" is returned. When change edit is already
-based on top of the latest patch set, the response
-"`409 Conflict`" is returned.
+When the change was rebased on top of latest patch set, the response code is
+"`200 OK`" and the rebased change edit is returned as an
+link:#edit-info[EditInfo] entity.
.Response
----
- HTTP/1.1 204 No Content
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "commit": {
+ "parents": [
+ {
+ "commit": "1eee2c9d8f352483781e772f35dc586a69ff5646",
+ }
+ ],
+ "author": {
+ "name": "Shawn O. Pearce",
+ "email": "sop@google.com",
+ "date": "2012-04-24 18:08:08.000000000",
+ "tz": -420
+ },
+ "committer": {
+ "name": "Shawn O. Pearce",
+ "email": "sop@google.com",
+ "date": "2012-04-24 18:08:08.000000000",
+ "tz": -420
+ },
+ "subject": "Use an EventBus to manage star icons",
+ "message": "Use an EventBus to manage star icons\n\nImage widgets that need to ..."
+ },
+ "base_patch_set_number": s,
+ "base_revision": "c35558e0925e6985c91f3a16921537d5e572b7a3",
+ "ref": "refs/users/01/1000001/edit-76482/2"
+ }
----
+When the change edit is already based on top of the latest patch set, the
+response code is "`409 Conflict`".
+
[[delete-edit]]
=== Delete Change Edit
--
@@ -5816,7 +5851,12 @@
Applies a list of <<fix-replacement-info,FixReplacementInfo>> loaded from the
<<apply-provided-fix-input,ApplyProvidedFixInput>> entity. The fixes are passed as part of the request body. The
application of the fixes creates a new change edit. `Apply Provided Fix` can only be applied on the current
-patchset.
+patchset. To apply a fix that was suggested to a non-current patchset, set `originalPatchsetForFix` to a patchset
+number where the fix was suggested. When `originalPatchsetForFix` is set, gerrit generates a patch (using
+`fix-replacement-info` and `originalPatchsetForFix`) and then tries to apply it to the current patchset.
+If it is not possible to apply a patch, an error is returned (see below for the list of errors).
+If the provided fix modifies the commit message and the message was changed between `originalPatchsetForFix`
+and the current patchset, the request is rejected..
If one of the secondary emails associated with the user performing the operation was used as the
committer email in the current patch set, the same email will be used as the committer email in the
@@ -8019,6 +8059,12 @@
|`files` |optional|
The files of the change edit as a map that maps the file names to
link:#file-info[FileInfo] entities.
+|`contains_git_conflicts` |optional, not set if `false`|
+Whether the change edit contains conflicts. +
+If `true`, some of the file contents of the change edit contain git conflict
+markers to indicate the conflicts. +
+Only set if this edit info is returned in response to a request that
+link:#rebase-edit[rebases the change edit] and conflicts are allowed.
|===========================
[[fetch-info]]
@@ -8477,6 +8523,21 @@
|`end` | Last index.
|===========================
+[[rebase-change-edit-input]]
+=== RebaseChangeEditInput
+The `RebaseChangeEditInput` entity contains information for rebasing a change edit.
+
+[options="header",cols="1,^1,5"]
+|====================================
+|Field Name ||Description
+|`allow_conflicts` |optional, defaults to false|
+If `true`, the rebase also succeeds if there are conflicts. +
+If there are conflicts the file contents of the rebased patch set contain
+git conflict markers to indicate the conflicts. +
+Callers can find out whether there were conflicts by checking the
+`contains_git_conflicts` field in the returned link:#edit-info[EditInfo].
+|====================================
+
[[rebase-input]]
=== RebaseInput
The `RebaseInput` entity contains information for changing parent when rebasing.
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 0cb407e..444cc23 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1514,8 +1514,9 @@
"name": "accounts",
"versions": {
"13": {
- "write": true,
- "search": true
+ "is_write": true,
+ "is_search": true,
+ "num_docs": 3250
}
}
},
@@ -1523,12 +1524,14 @@
"name": "changes",
"versions": {
"83": {
- "write": true,
- "search": true
+ "is_write": true,
+ "is_search": true,
+ "num_docs": 250000
},
"84": {
- "write": true,
- "search": false
+ "is_write": true,
+ "is_search": false,
+ "num_docs": 150000
}
}
},
@@ -1536,8 +1539,9 @@
"name": "groups",
"versions": {
"10": {
- "write": true,
- "search": true
+ "is_write": true,
+ "is_search": true,
+ "num_docs": 500
}
}
},
@@ -1545,8 +1549,9 @@
"name": "projects",
"versions": {
"8": {
- "write": true,
- "search": true
+ "is_write": true,
+ "is_search": true,
+ "num_docs": 90
}
}
}
@@ -1576,12 +1581,14 @@
"name": "changes",
"versions": {
"83": {
- "write": true,
- "search": true
+ "is_write": true,
+ "is_search": true,
+ "num_docs": 250000
},
"84": {
- "write": true,
- "search": false
+ "is_write": true,
+ "is_search": false,
+ "num_docs": 150000
}
}
}
@@ -1607,12 +1614,14 @@
)]}'
{
"83": {
- "write": true,
- "search": true
+ "is_write": true,
+ "is_search": true,
+ "num_docs": 250000
},
"84": {
- "write": true,
- "search": false
+ "is_write": true,
+ "is_search": false,
+ "num_docs": 150000
}
}
----
@@ -1636,8 +1645,9 @@
)]}'
{
- "write": true,
- "search": false
+ "is_write": true,
+ "is_search": false,
+ "num_docs": 150000
}
----
diff --git a/java/com/google/gerrit/acceptance/DisabledAccountIndex.java b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
index 7660948..4af9a31 100644
--- a/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
+++ b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
@@ -70,6 +70,11 @@
}
@Override
+ public int numDocs() {
+ throw new UnsupportedOperationException("AccountIndex is disabled");
+ }
+
+ @Override
public DataSource<AccountState> getSource(Predicate<AccountState> p, QueryOptions opts) {
throw new UnsupportedOperationException("AccountIndex is disabled");
}
diff --git a/java/com/google/gerrit/acceptance/DisabledChangeIndex.java b/java/com/google/gerrit/acceptance/DisabledChangeIndex.java
index c028a8e..4748e31 100644
--- a/java/com/google/gerrit/acceptance/DisabledChangeIndex.java
+++ b/java/com/google/gerrit/acceptance/DisabledChangeIndex.java
@@ -77,6 +77,11 @@
}
@Override
+ public int numDocs() {
+ throw new UnsupportedOperationException("ChangeIndex is disabled");
+ }
+
+ @Override
public DataSource<ChangeData> getSource(Predicate<ChangeData> p, QueryOptions opts)
throws QueryParseException {
throw new UnsupportedOperationException("ChangeIndex is disabled");
diff --git a/java/com/google/gerrit/acceptance/DisabledProjectIndex.java b/java/com/google/gerrit/acceptance/DisabledProjectIndex.java
index f2aad4a..ec1bcd4 100644
--- a/java/com/google/gerrit/acceptance/DisabledProjectIndex.java
+++ b/java/com/google/gerrit/acceptance/DisabledProjectIndex.java
@@ -75,6 +75,11 @@
}
@Override
+ public int numDocs() {
+ throw new UnsupportedOperationException("ProjectIndex is disabled");
+ }
+
+ @Override
public DataSource<ProjectData> getSource(Predicate<ProjectData> p, QueryOptions opts) {
throw new UnsupportedOperationException("ProjectIndex is disabled");
}
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
index 2fd8a07..6d99ded 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
@@ -91,6 +91,14 @@
void rebase() throws RestApiException;
/**
+ * Rebases the change edit on top of the latest patch set of this change.
+ *
+ * @param input params for rebasing the change edit
+ * @throws RestApiException if the change edit couldn't be rebased or a change edit wasn't present
+ */
+ EditInfo rebase(RebaseChangeEditInput input) throws RestApiException;
+
+ /**
* Publishes the change edit using default settings. See {@link #publish(PublishChangeEditInput)}
* for more details.
*
@@ -238,6 +246,11 @@
}
@Override
+ public EditInfo rebase(RebaseChangeEditInput input) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void publish() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/changes/RebaseChangeEditInput.java b/java/com/google/gerrit/extensions/api/changes/RebaseChangeEditInput.java
new file mode 100644
index 0000000..4eb0ebb
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/RebaseChangeEditInput.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2024 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.api.changes;
+
+public class RebaseChangeEditInput {
+ /**
+ * Whether the rebase should succeed if there are conflicts.
+ *
+ * <p>If there are conflicts the file contents of the rebased change contain git conflict markers
+ * to indicate the conflicts.
+ */
+ public boolean allowConflicts;
+}
diff --git a/java/com/google/gerrit/extensions/common/ApplyProvidedFixInput.java b/java/com/google/gerrit/extensions/common/ApplyProvidedFixInput.java
index cd28d83..7d48fce 100644
--- a/java/com/google/gerrit/extensions/common/ApplyProvidedFixInput.java
+++ b/java/com/google/gerrit/extensions/common/ApplyProvidedFixInput.java
@@ -24,4 +24,6 @@
public ApplyProvidedFixInput() {}
public List<FixReplacementInfo> fixReplacementInfos;
+
+ public Integer originalPatchsetForFix;
}
diff --git a/java/com/google/gerrit/extensions/common/EditInfo.java b/java/com/google/gerrit/extensions/common/EditInfo.java
index 0cd5af3..9d170de 100644
--- a/java/com/google/gerrit/extensions/common/EditInfo.java
+++ b/java/com/google/gerrit/extensions/common/EditInfo.java
@@ -23,4 +23,16 @@
public String ref;
public Map<String, FetchInfo> fetch;
public Map<String, FileInfo> files;
+
+ /**
+ * Whether the change edit contains conflicts.
+ *
+ * <p>If {@code true}, some of the file contents of the change contain git conflict markers to
+ * indicate the conflicts.
+ *
+ * <p>Only set if this edit info is returned in response to a request that rebases the change edit
+ * (see {@link com.google.gerrit.server.restapi.change.RebaseChangeEdit}) and conflicts are
+ * allowed.
+ */
+ public Boolean containsGitConflicts;
}
diff --git a/java/com/google/gerrit/extensions/common/testing/EditInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/EditInfoSubject.java
index 0d1e82a..3cdbe44 100644
--- a/java/com/google/gerrit/extensions/common/testing/EditInfoSubject.java
+++ b/java/com/google/gerrit/extensions/common/testing/EditInfoSubject.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.extensions.common.testing.CommitInfoSubject.commits;
import static com.google.gerrit.truth.MapSubject.mapEntries;
+import com.google.common.truth.BooleanSubject;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
@@ -62,4 +63,10 @@
isNotNull();
return check("files").about(mapEntries()).that(editInfo.files);
}
+
+ public BooleanSubject containsGitConflicts() {
+ isNotNull();
+ return check("containsGitConflicts")
+ .that(editInfo.containsGitConflicts != null ? editInfo.containsGitConflicts : false);
+ }
}
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index fa67034..0e37684 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -96,7 +96,7 @@
import com.google.gerrit.server.permissions.DefaultPermissionBackendModule;
import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
import com.google.gerrit.server.plugins.PluginModule;
-import com.google.gerrit.server.project.DefaultProjectNameLockManager.DefaultProjectNameLockManagerModule;
+import com.google.gerrit.server.project.DefaultLockManager.DefaultLockManagerModule;
import com.google.gerrit.server.restapi.RestApiModule;
import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore.JdbcAccountPatchReviewStoreModule;
import com.google.gerrit.server.schema.NoteDbSchemaVersionCheck;
@@ -370,7 +370,7 @@
modules.add(new AttentionSetOwnerAdderModule());
modules.add(new ChangeCleanupRunnerModule());
modules.add(new AccountDeactivatorModule());
- modules.add(new DefaultProjectNameLockManagerModule());
+ modules.add(new DefaultLockManagerModule());
modules.add(new ExternalIdCaseSensitivityMigrator.ExternalIdCaseSensitivityMigratorModule());
return dbInjector.createChildInjector(
ModuleOverloader.override(
diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
index a92dd18..bb21662 100644
--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
@@ -21,6 +21,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.FluentLogger;
+import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.UsedAt.Project;
@@ -30,6 +31,7 @@
import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.httpd.raw.IndexPreloadingUtil.RequestedPage;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gson.Gson;
@@ -43,6 +45,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
+import java.util.regex.Matcher;
/** Helper for generating parts of {@code index.html}. */
@UsedAt(Project.GOOGLE)
@@ -80,6 +83,29 @@
return data.build();
}
+ /**
+ * Returns the basePatchNum that was specified in the URL when present. If no basePatchNum is
+ * specified then it points to PARENT which is represented by 0
+ */
+ public static Integer computeBasePatchNum(@Nullable String requestedPath) {
+ if (requestedPath == null) {
+ return 0;
+ }
+ Matcher matcher = IndexPreloadingUtil.CHANGE_URL_PATTERN.matcher(requestedPath);
+ String basePatchNum = null;
+ if (matcher.matches()) {
+ basePatchNum = matcher.group("basePatchNum");
+ }
+ if (basePatchNum == null) {
+ return 0; // No match is found
+ }
+ Integer basePatchNumInt = Ints.tryParse(basePatchNum);
+ if (basePatchNumInt == null) {
+ return 0; // tryParse was unable to parse
+ }
+ return basePatchNumInt;
+ }
+
/** Returns dynamic parameters of {@code index.html}. */
public static ImmutableMap<String, Object> dynamicTemplateData(
GerritApi gerritApi, String requestedURL, String canonicalURL)
@@ -99,11 +125,19 @@
String requestedPath = IndexPreloadingUtil.getPath(requestedURL);
IndexPreloadingUtil.RequestedPage page = IndexPreloadingUtil.parseRequestedPage(requestedPath);
+ Integer basePatchNum = computeBasePatchNum(requestedPath);
switch (page) {
case CHANGE:
case DIFF:
- data.put(
- "defaultChangeDetailHex", ListOption.toHex(IndexPreloadingUtil.CHANGE_DETAIL_OPTIONS));
+ if (basePatchNum.equals(0)) {
+ data.put(
+ "defaultChangeDetailHex",
+ ListOption.toHex(IndexPreloadingUtil.CHANGE_DETAIL_OPTIONS_WITHOUT_PARENTS));
+ } else {
+ data.put(
+ "defaultChangeDetailHex",
+ ListOption.toHex(IndexPreloadingUtil.CHANGE_DETAIL_OPTIONS_WITH_PARENTS));
+ }
data.put(
"changeRequestsPath",
IndexPreloadingUtil.computeChangeRequestsPath(requestedPath, page).get());
@@ -111,7 +145,8 @@
break;
case PROFILE:
case DASHBOARD:
- // Dashboard is preloaded queries are added later when we check user is authenticated.
+ // Dashboard is preloaded queries are added later when we check user is
+ // authenticated.
case PAGE_WITHOUT_PRELOADING:
break;
}
diff --git a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
index cc11638..43d0172 100644
--- a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
@@ -45,7 +45,8 @@
}
public static final String CHANGE_CANONICAL_PATH = "/c/(?<project>.+)/\\+/(?<changeNum>\\d+)";
- public static final String BASE_PATCH_NUM_PATH_PART = "(/(-?\\d+|edit)(\\.\\.(\\d+|edit))?)";
+ public static final String BASE_PATCH_NUM_PATH_PART =
+ "(/(?<basePatchNum>-?\\d+|edit)(\\.\\.(\\d+|edit))?)";
public static final Pattern CHANGE_URL_PATTERN =
Pattern.compile(CHANGE_CANONICAL_PATH + BASE_PATCH_NUM_PATH_PART + "?" + "/?$");
public static final Pattern DIFF_URL_PATTERN =
@@ -90,7 +91,22 @@
ListChangesOption.SUBMIT_REQUIREMENTS,
ListChangesOption.STAR);
- public static final ImmutableSet<ListChangesOption> CHANGE_DETAIL_OPTIONS =
+ public static final ImmutableSet<ListChangesOption> CHANGE_DETAIL_OPTIONS_WITHOUT_PARENTS =
+ ImmutableSet.of(
+ ListChangesOption.ALL_COMMITS,
+ ListChangesOption.ALL_REVISIONS,
+ ListChangesOption.CHANGE_ACTIONS,
+ ListChangesOption.DETAILED_ACCOUNTS,
+ ListChangesOption.DETAILED_LABELS,
+ ListChangesOption.DOWNLOAD_COMMANDS,
+ ListChangesOption.MESSAGES,
+ ListChangesOption.REVIEWER_UPDATES,
+ ListChangesOption.SUBMITTABLE,
+ ListChangesOption.WEB_LINKS,
+ ListChangesOption.SKIP_DIFFSTAT,
+ ListChangesOption.SUBMIT_REQUIREMENTS);
+
+ public static final ImmutableSet<ListChangesOption> CHANGE_DETAIL_OPTIONS_WITH_PARENTS =
ImmutableSet.of(
ListChangesOption.ALL_COMMITS,
ListChangesOption.ALL_REVISIONS,
diff --git a/java/com/google/gerrit/index/Index.java b/java/com/google/gerrit/index/Index.java
index ec530c1..fdf806c 100644
--- a/java/com/google/gerrit/index/Index.java
+++ b/java/com/google/gerrit/index/Index.java
@@ -75,6 +75,9 @@
/** Delete all documents from the index. */
void deleteAll();
+ /** Return the number of documents in this index */
+ int numDocs();
+
/**
* Convert the given operator predicate into a source searching the index and returning only the
* documents matching that predicate.
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index 570290e..b06143e 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -119,6 +119,13 @@
}
}
+ @Override
+ public int numDocs() {
+ synchronized (indexedDocuments) {
+ return indexedDocuments.size();
+ }
+ }
+
public int getQueryCount() {
return queryCount;
}
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index e00c394..85ffd93 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -326,6 +326,21 @@
}
}
+ @Override
+ public int numDocs() {
+ try {
+ IndexSearcher searcher = acquire();
+ try {
+ return searcher.getIndexReader().numDocs();
+ } finally {
+ release(searcher);
+ }
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log(e.getMessage());
+ throw new StorageException(e);
+ }
+ }
+
public IndexWriter getWriter() {
return writer;
}
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index ffd25ba..e5f7787 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -265,6 +265,11 @@
}
@Override
+ public int numDocs() {
+ return openIndex.numDocs() + closedIndex.numDocs();
+ }
+
+ @Override
public ChangeDataSource getSource(Predicate<ChangeData> p, QueryOptions opts)
throws QueryParseException {
Set<Change.Status> statuses = ChangeIndexRewriter.getPossibleStatus(p);
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 198eeaa..6fc39e5 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -63,6 +63,7 @@
import com.google.gerrit.server.account.AccountCacheImpl;
import com.google.gerrit.server.account.AccountDeactivator.AccountDeactivatorModule;
import com.google.gerrit.server.account.InternalAccountDirectory.InternalAccountDirectoryModule;
+import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCacheImpl;
import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCaseSensitivityMigrator;
import com.google.gerrit.server.account.storage.notedb.AccountNoteDbReadStorageModule;
import com.google.gerrit.server.account.storage.notedb.AccountNoteDbWriteStorageModule;
@@ -101,6 +102,7 @@
import com.google.gerrit.server.mail.EmailModule;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier.SignedTokenEmailTokenVerifierModule;
import com.google.gerrit.server.mail.receive.MailReceiver.MailReceiverModule;
+import com.google.gerrit.server.mail.send.FromAddressGeneratorProvider;
import com.google.gerrit.server.mail.send.SmtpEmailSender.SmtpEmailSenderModule;
import com.google.gerrit.server.mime.MimeUtil2Module;
import com.google.gerrit.server.notedb.NoteDbDraftCommentsModule;
@@ -110,7 +112,7 @@
import com.google.gerrit.server.permissions.DefaultPermissionBackendModule;
import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
import com.google.gerrit.server.plugins.PluginModule;
-import com.google.gerrit.server.project.DefaultProjectNameLockManager.DefaultProjectNameLockManagerModule;
+import com.google.gerrit.server.project.DefaultLockManager.DefaultLockManagerModule;
import com.google.gerrit.server.restapi.RestApiModule;
import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore.JdbcAccountPatchReviewStoreModule;
import com.google.gerrit.server.schema.NoteDbSchemaVersionCheck;
@@ -475,7 +477,10 @@
modules.add(new AccountNoteDbWriteStorageModule());
modules.add(new AccountNoteDbReadStorageModule());
+ modules.add(new ExternalIdCacheImpl.ExternalIdCacheModule());
+ modules.add(new ExternalIdCacheImpl.ExternalIdCacheBindingModule());
modules.add(new RepoSequenceModule());
+ modules.add(new FromAddressGeneratorProvider.UserAddressGenModule());
modules.add(new NoteDbDraftCommentsModule());
modules.add(new NoteDbStarredChangesModule());
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
@@ -561,7 +566,7 @@
modules.add(new ChangeCleanupRunnerModule());
}
modules.add(new LocalMergeSuperSetComputationModule());
- modules.add(new DefaultProjectNameLockManagerModule());
+ modules.add(new DefaultLockManagerModule());
List<Module> libModules =
LibModuleLoader.loadModules(cfgInjector, LibModuleType.SYS_MODULE_TYPE);
diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java
index d393a89..7424407 100644
--- a/java/com/google/gerrit/pgm/Reindex.java
+++ b/java/com/google/gerrit/pgm/Reindex.java
@@ -33,6 +33,7 @@
import com.google.gerrit.pgm.util.SiteProgram;
import com.google.gerrit.server.LibModuleLoader;
import com.google.gerrit.server.ModuleOverloader;
+import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCacheImpl;
import com.google.gerrit.server.account.storage.notedb.AccountNoteDbReadStorageModule;
import com.google.gerrit.server.account.storage.notedb.AccountNoteDbWriteStorageModule;
import com.google.gerrit.server.cache.CacheDisplay;
@@ -243,6 +244,8 @@
});
modules.add(new AccountNoteDbWriteStorageModule());
modules.add(new AccountNoteDbReadStorageModule());
+ modules.add(new ExternalIdCacheImpl.ExternalIdCacheModule());
+ modules.add(new ExternalIdCacheImpl.ExternalIdCacheBindingModule());
modules.add(new RepoSequenceModule());
modules.add(new NoteDbDraftCommentsModule());
modules.add(new NoteDbStarredChangesModule());
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index f45f1be..f75984d 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -42,7 +42,6 @@
import com.google.gerrit.server.account.GroupIncludeCacheImpl;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.account.ServiceUserClassifierImpl;
-import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCacheModule;
import com.google.gerrit.server.cache.CacheRemovalListener;
import com.google.gerrit.server.cache.h2.H2CacheModule;
import com.google.gerrit.server.cache.mem.DefaultMemoryCacheModule;
@@ -181,7 +180,6 @@
modules.add(new DefaultPermissionBackendModule());
modules.add(new DefaultMemoryCacheModule());
modules.add(new H2CacheModule());
- modules.add(new ExternalIdCacheModule());
modules.add(new GroupModule());
modules.add(new NoteDbModule());
modules.add(AccountCacheImpl.module());
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
index a23e7bc..886fe70 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
@@ -28,6 +28,12 @@
* cache is up to date.
*
* <p>All returned collections are unmodifiable.
+ *
+ * <p>NOTE: Modules which bind {@link ExternalIdCache} by using modules other than {@link
+ * com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCacheImpl.ExternalIdCacheBindingModule},
+ * should also provide an {@code Optional<}{@link
+ * com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCacheImpl}{@code >}
+ * binding.
*/
public interface ExternalIdCache {
Optional<ExternalId> byKey(ExternalId.Key key) throws IOException;
diff --git a/java/com/google/gerrit/server/account/externalids/storage/notedb/DisabledExternalIdCache.java b/java/com/google/gerrit/server/account/externalids/storage/notedb/DisabledExternalIdCache.java
index 8e53277..fd19fcc 100644
--- a/java/com/google/gerrit/server/account/externalids/storage/notedb/DisabledExternalIdCache.java
+++ b/java/com/google/gerrit/server/account/externalids/storage/notedb/DisabledExternalIdCache.java
@@ -21,6 +21,8 @@
import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.inject.AbstractModule;
import com.google.inject.Module;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Optional;
@@ -32,6 +34,12 @@
protected void configure() {
bind(ExternalIdCache.class).to(DisabledExternalIdCache.class);
}
+
+ @Provides
+ @Singleton
+ Optional<ExternalIdCacheImpl> provideNoteDbExternalIdCacheImpl() {
+ return Optional.empty();
+ }
};
}
diff --git a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdCacheImpl.java b/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdCacheImpl.java
index dbfe205..20c94eb 100644
--- a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdCacheImpl.java
+++ b/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdCacheImpl.java
@@ -14,27 +14,86 @@
package com.google.gerrit.server.account.externalids.storage.notedb;
+import static com.google.inject.Scopes.SINGLETON;
+
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdCache;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer;
+import com.google.inject.AbstractModule;
import com.google.inject.Inject;
+import com.google.inject.Provides;
import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import java.io.IOException;
+import java.time.Duration;
import java.util.Optional;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
-/** Caches external IDs of all accounts. The external IDs are always loaded from NoteDb. */
-@Singleton
-class ExternalIdCacheImpl implements ExternalIdCache {
+/**
+ * Caches external IDs of all accounts. The external IDs are always loaded from NoteDb. *
+ *
+ * <p>This class should be bounded as a Singleton. However, due to internal limitations in Google,
+ * it cannot be marked as a singleton. The common installation pattern should therefore be:
+ *
+ * <pre>{@code
+ * * install(new ExternalIdCacheModule());
+ * * install(new ExternalIdCacheBindingModule());
+ * *
+ * }</pre>
+ */
+public class ExternalIdCacheImpl implements ExternalIdCache {
public static final String CACHE_NAME = "external_ids_map";
+ public static class ExternalIdCacheModule extends CacheModule {
+ @Override
+ protected void configure() {
+ persist(CACHE_NAME, ObjectId.class, new TypeLiteral<AllExternalIds>() {})
+ // The cached data is potentially pretty large and we are always only interested
+ // in the latest value. However, due to a race condition, it is possible for different
+ // threads to observe different values of the meta ref, and hence request different keys
+ // from the cache. Extend the cache size by 1 to cover this case, but expire the extra
+ // object after a short period of time, since it may be a potentially large amount of
+ // memory.
+ // When loading a new value because the primary data advanced, we want to leverage the old
+ // cache state to recompute only what changed. This doesn't affect cache size though as
+ // Guava calls the loader first and evicts later on.
+ .maximumWeight(2)
+ .expireFromMemoryAfterAccess(Duration.ofMinutes(1))
+ .diskLimit(-1)
+ .version(1)
+ .keySerializer(ObjectIdCacheSerializer.INSTANCE)
+ .valueSerializer(AllExternalIds.Serializer.INSTANCE);
+ }
+ }
+
+ public static class ExternalIdCacheBindingModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(ExternalIdCache.class).to(ExternalIdCacheImpl.class).in(SINGLETON);
+ }
+
+ /**
+ * Used by {@link ExternalIdsNoteDbImpl}. Modules which bind {@link ExternalIdCache} by using
+ * modules other than {@link ExternalIdCacheBindingModule}, should also provide an {@code
+ * Optional<ExternalIdCacheImpl>} binding.
+ */
+ @Provides
+ @Singleton
+ Optional<ExternalIdCacheImpl> provideNoteDbExternalIdCacheImpl(
+ ExternalIdCacheImpl externalIdCache) {
+ return Optional.of(externalIdCache);
+ }
+ }
+
private final Cache<ObjectId, AllExternalIds> extIdsByAccount;
private final ExternalIdReader externalIdReader;
private final ExternalIdCacheLoader externalIdCacheLoader;
diff --git a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdCacheModule.java b/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdCacheModule.java
deleted file mode 100644
index aca0e1a..0000000
--- a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdCacheModule.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright (C) 2017 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.account.externalids.storage.notedb;
-
-import com.google.gerrit.server.account.externalids.ExternalIdCache;
-import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer;
-import com.google.inject.TypeLiteral;
-import java.time.Duration;
-import org.eclipse.jgit.lib.ObjectId;
-
-public class ExternalIdCacheModule extends CacheModule {
- @Override
- protected void configure() {
- persist(ExternalIdCacheImpl.CACHE_NAME, ObjectId.class, new TypeLiteral<AllExternalIds>() {})
- // The cached data is potentially pretty large and we are always only interested
- // in the latest value. However, due to a race condition, it is possible for different
- // threads to observe different values of the meta ref, and hence request different keys
- // from the cache. Extend the cache size by 1 to cover this case, but expire the extra
- // object after a short period of time, since it may be a potentially large amount of
- // memory.
- // When loading a new value because the primary data advanced, we want to leverage the old
- // cache state to recompute only what changed. This doesn't affect cache size though as
- // Guava calls the loader first and evicts later on.
- .maximumWeight(2)
- .expireFromMemoryAfterAccess(Duration.ofMinutes(1))
- .diskLimit(-1)
- .version(1)
- .keySerializer(ObjectIdCacheSerializer.INSTANCE)
- .valueSerializer(AllExternalIds.Serializer.INSTANCE);
-
- bind(ExternalIdCacheImpl.class);
- bind(ExternalIdCache.class).to(ExternalIdCacheImpl.class);
- }
-}
diff --git a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdsNoteDbImpl.java b/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdsNoteDbImpl.java
index 7a2945c..4c26442 100644
--- a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdsNoteDbImpl.java
+++ b/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdsNoteDbImpl.java
@@ -21,7 +21,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.config.AuthConfig;
@@ -47,21 +46,15 @@
@Inject
ExternalIdsNoteDbImpl(
ExternalIdReader externalIdReader,
- ExternalIdCache externalIdCache,
+ Optional<ExternalIdCacheImpl> externalIdCacheImpl,
ExternalIdKeyFactory externalIdKeyFactory,
AuthConfig authConfig) {
this.externalIdReader = externalIdReader;
- if (externalIdCache instanceof ExternalIdCacheImpl) {
- this.externalIdCache = (ExternalIdCacheImpl) externalIdCache;
- } else if (externalIdCache instanceof DisabledExternalIdCache) {
- // Supported case for testing only. Non of the disabled cache methods should be called, so
- // it's safe to not assign the var.
- this.externalIdCache = null;
- } else {
- throw new IllegalStateException(
- "The cache provided in ExternalIdsNoteDbImpl should be either ExternalIdCacheImpl or"
- + " DisabledExternalIdCache");
- }
+ this.externalIdCache =
+ externalIdCacheImpl.orElse(
+ // Supported case for tests or Google implementation. None of the disabled cache methods
+ // should be called from these flows, so it's safe to not assign the var.
+ null);
this.externalIdKeyFactory = externalIdKeyFactory;
this.authConfig = authConfig;
}
diff --git a/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
index c76eeeb..bdabcbd 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
@@ -16,10 +16,12 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
import com.google.gerrit.extensions.api.changes.ChangeEditIdentityType;
import com.google.gerrit.extensions.api.changes.FileContentInput;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
import com.google.gerrit.extensions.client.ChangeEditDetailOption;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.Input;
@@ -150,9 +152,14 @@
@Override
public void rebase() throws RestApiException {
+ rebase(new RebaseChangeEditInput());
+ }
+
+ @Override
+ @CanIgnoreReturnValue
+ public EditInfo rebase(RebaseChangeEditInput input) throws RestApiException {
try {
- @SuppressWarnings("unused")
- var unused = rebaseChangeEdit.apply(changeResource, null);
+ return rebaseChangeEdit.apply(changeResource, input).value();
} catch (Exception e) {
throw asRestApiException("Cannot rebase change edit", e);
}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 0f6ae5f..4ad8fa7 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -109,7 +109,6 @@
import com.google.gerrit.server.account.ServiceUserClassifierImpl;
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.ExternalIdModule;
-import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCacheModule;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.auth.AuthBackend;
import com.google.gerrit.server.auth.UniversalAuthBackend;
@@ -189,8 +188,8 @@
import com.google.gerrit.server.plugins.ReloadPluginListener;
import com.google.gerrit.server.project.AccessControlModule;
import com.google.gerrit.server.project.CommentLinkProvider;
+import com.google.gerrit.server.project.LockManager;
import com.google.gerrit.server.project.ProjectCacheImpl;
-import com.google.gerrit.server.project.ProjectNameLockManager;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.PrologRulesWarningValidator;
import com.google.gerrit.server.project.SubmitRequirementConfigValidator;
@@ -279,7 +278,6 @@
install(new AccessControlModule());
install(new AccountModule());
install(new CmdLineParserModule());
- install(new ExternalIdCacheModule());
install(new ExternalIdModule());
install(new GitModule());
install(new GroupDbModule());
@@ -453,7 +451,7 @@
DynamicItem.itemOf(binder(), AccountPatchReviewStore.class);
DynamicSet.setOf(binder(), ActionVisitor.class);
DynamicItem.itemOf(binder(), MergeSuperSetComputation.class);
- DynamicItem.itemOf(binder(), ProjectNameLockManager.class);
+ DynamicItem.itemOf(binder(), LockManager.class);
DynamicSet.setOf(binder(), SubmitRule.class);
DynamicSet.setOf(binder(), SubmitRequirement.class);
DynamicSet.setOf(binder(), QuotaEnforcer.class);
diff --git a/java/com/google/gerrit/server/config/GerritIsReplicaProvider.java b/java/com/google/gerrit/server/config/GerritIsReplicaProvider.java
index 213cd1c..f242a50 100644
--- a/java/com/google/gerrit/server/config/GerritIsReplicaProvider.java
+++ b/java/com/google/gerrit/server/config/GerritIsReplicaProvider.java
@@ -17,7 +17,6 @@
import com.google.gerrit.server.util.ReplicaUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
/**
@@ -25,7 +24,6 @@
*
* <p>The returned boolean indicates whether Gerrit is run as a read-only replica.
*/
-@Singleton
public final class GerritIsReplicaProvider implements Provider<Boolean> {
private final Config config;
diff --git a/java/com/google/gerrit/server/config/GerritServerConfigModule.java b/java/com/google/gerrit/server/config/GerritServerConfigModule.java
index 8ddcdac..b811834 100644
--- a/java/com/google/gerrit/server/config/GerritServerConfigModule.java
+++ b/java/com/google/gerrit/server/config/GerritServerConfigModule.java
@@ -71,6 +71,7 @@
bind(SecureStore.class).toProvider(SecureStoreProvider.class).in(SINGLETON);
bind(Boolean.class)
.annotatedWith(GerritIsReplica.class)
- .toProvider(GerritIsReplicaProvider.class);
+ .toProvider(GerritIsReplicaProvider.class)
+ .in(SINGLETON);
}
}
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 7a89fb3..1030baa 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -14,9 +14,12 @@
package com.google.gerrit.server.edit;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BooleanProjectConfig;
@@ -25,6 +28,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.ChangeEditIdentityType;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
@@ -37,6 +41,7 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.edit.tree.ChangeFileContentModification;
import com.google.gerrit.server.edit.tree.DeleteFileModification;
import com.google.gerrit.server.edit.tree.RenameFileModification;
@@ -44,6 +49,9 @@
import com.google.gerrit.server.edit.tree.TreeCreator;
import com.google.gerrit.server.edit.tree.TreeModification;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -62,15 +70,19 @@
import java.time.Instant;
import java.time.ZoneId;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import org.eclipse.jgit.diff.DiffAlgorithm;
import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
import org.eclipse.jgit.diff.RawText;
import org.eclipse.jgit.diff.RawTextComparator;
+import org.eclipse.jgit.diff.Sequence;
+import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.dircache.InvalidPathException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -81,9 +93,9 @@
import org.eclipse.jgit.merge.MergeChunk;
import org.eclipse.jgit.merge.MergeResult;
import org.eclipse.jgit.merge.MergeStrategy;
+import org.eclipse.jgit.merge.ResolveMerger;
import org.eclipse.jgit.merge.ThreeWayMerger;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
@@ -106,9 +118,11 @@
private final ProjectCache projectCache;
private final NoteDbEdits noteDbEdits;
private final ChangeUtil changeUtil;
+ private final boolean useDiff3;
@Inject
ChangeEditModifier(
+ @GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent gerritIdent,
ChangeIndexer indexer,
Provider<CurrentUser> currentUser,
@@ -126,6 +140,9 @@
this.projectCache = projectCache;
noteDbEdits = new NoteDbEdits(gitReferenceUpdated, zoneId, indexer, currentUser);
this.changeUtil = changeUtil;
+ this.useDiff3 =
+ cfg.getBoolean(
+ "change", /* subsection= */ null, "diff3ConflictView", /* defaultValue= */ false);
}
/**
@@ -157,12 +174,15 @@
*
* @param repository the affected Git repository
* @param notes the {@link ChangeNotes} of the change whose change edit should be rebased
+ * @param input the request input
+ * @return the rebased change edit commit
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
* @throws InvalidChangeOperationException if a change edit doesn't exist for the specified
* change, the change edit is already based on the latest patch set, or the change represents
* the root commit
*/
- public void rebaseEdit(Repository repository, ChangeNotes notes)
+ public CodeReviewCommit rebaseEdit(
+ Repository repository, ChangeNotes notes, RebaseChangeEditInput input)
throws AuthException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
assertCanEdit(notes);
@@ -182,14 +202,16 @@
notes.getChangeId(), currentPatchSet.id()));
}
- rebase(notes.getProjectName(), repository, changeEdit, currentPatchSet);
+ return rebase(
+ notes.getProjectName(), repository, changeEdit, currentPatchSet, input.allowConflicts);
}
- private void rebase(
+ private CodeReviewCommit rebase(
Project.NameKey project,
Repository repository,
ChangeEdit changeEdit,
- PatchSet currentPatchSet)
+ PatchSet currentPatchSet,
+ boolean allowConflicts)
throws IOException, MergeConflictException, InvalidChangeOperationException {
RevCommit currentEditCommit = changeEdit.getEditCommit();
if (currentEditCommit.getParentCount() == 0) {
@@ -197,20 +219,10 @@
"Rebase change edit against root commit not supported");
}
- RevCommit basePatchSetCommit = NoteDbEdits.lookupCommit(repository, currentPatchSet.commitId());
- RevTree basePatchSetTree = basePatchSetCommit.getTree();
-
- ObjectId newTreeId = merge(repository, changeEdit, basePatchSetTree);
Instant nowTimestamp = TimeUtil.now();
- String commitMessage = currentEditCommit.getFullMessage();
- ObjectId newEditCommitId =
- createCommit(
- repository,
- basePatchSetCommit,
- newTreeId,
- commitMessage,
- currentEditCommit.getAuthorIdent(),
- new PersonIdent(currentEditCommit.getCommitterIdent(), nowTimestamp));
+ RevCommit basePatchSetCommit = NoteDbEdits.lookupCommit(repository, currentPatchSet.commitId());
+ CodeReviewCommit newEditCommit =
+ merge(repository, changeEdit, basePatchSetCommit, nowTimestamp, allowConflicts);
noteDbEdits.baseEditOnDifferentPatchset(
project,
@@ -218,8 +230,10 @@
changeEdit,
currentPatchSet,
currentEditCommit,
- newEditCommitId,
+ newEditCommit,
nowTimestamp);
+
+ return newEditCommit;
}
/**
@@ -532,7 +546,7 @@
return editBasePatchSet.id().equals(patchSet.id());
}
- private static ObjectId createNewTree(
+ public static ObjectId createNewTree(
Repository repository, RevCommit baseCommit, List<TreeModification> treeModifications)
throws BadRequestException, IOException, InvalidChangeOperationException {
if (treeModifications.isEmpty()) {
@@ -554,22 +568,106 @@
return newTreeId;
}
- private static ObjectId merge(Repository repository, ChangeEdit changeEdit, ObjectId newTreeId)
+ private CodeReviewCommit merge(
+ Repository repository,
+ ChangeEdit changeEdit,
+ RevCommit basePatchSetCommit,
+ Instant timestamp,
+ boolean allowConflicts)
throws IOException, MergeConflictException {
PatchSet basePatchSet = changeEdit.getBasePatchSet();
ObjectId basePatchSetCommitId = basePatchSet.commitId();
ObjectId editCommitId = changeEdit.getEditCommit();
- ThreeWayMerger threeWayMerger = MergeStrategy.RESOLVE.newMerger(repository, true);
- threeWayMerger.setBase(basePatchSetCommitId);
- boolean successful = threeWayMerger.merge(newTreeId, editCommitId);
+ try (CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(repository);
+ ObjectInserter objectInserter = repository.newObjectInserter()) {
+ ThreeWayMerger merger = MergeStrategy.RESOLVE.newMerger(repository, true);
+ merger.setBase(basePatchSetCommitId);
+
+ DirCache dc = DirCache.newInCore();
+ if (allowConflicts && merger instanceof ResolveMerger) {
+ // The DirCache must be set on ResolveMerger before calling
+ // ResolveMerger#merge(AnyObjectId...) otherwise the entries in DirCache don't get
+ // populated.
+ ((ResolveMerger) merger).setDirCache(dc);
+ }
+
+ boolean successful = merger.merge(basePatchSetCommit, editCommitId);
+
+ ObjectId newTreeId;
+ ImmutableSet<String> filesWithGitConflicts;
+ if (successful) {
+ newTreeId = merger.getResultTreeId();
+ filesWithGitConflicts = null;
+ } else {
+ List<String> conflicts = ImmutableList.of();
+ if (merger instanceof ResolveMerger) {
+ conflicts = ((ResolveMerger) merger).getUnmergedPaths();
+ }
+
+ if (!allowConflicts || !(merger instanceof ResolveMerger)) {
+ throw new MergeConflictException(
+ String.format(
+ "Rebasing change edit onto another patchset results in merge conflicts.\n\n"
+ + "%s\n\n"
+ + "Download the edit patchset and rebase manually to preserve changes.",
+ MergeUtil.createConflictMessage(conflicts)));
+ }
+
+ Map<String, MergeResult<? extends Sequence>> mergeResults =
+ ((ResolveMerger) merger).getMergeResults();
+ filesWithGitConflicts =
+ mergeResults.entrySet().stream()
+ .filter(e -> e.getValue().containsConflicts())
+ .map(Map.Entry::getKey)
+ .collect(toImmutableSet());
+
+ newTreeId =
+ MergeUtil.mergeWithConflicts(
+ revWalk,
+ objectInserter,
+ dc,
+ "PATCH SET",
+ basePatchSetCommit,
+ "EDIT",
+ revWalk.parseCommit(editCommitId),
+ mergeResults,
+ useDiff3);
+ objectInserter.flush();
+ }
+
+ RevCommit currentEditCommit = changeEdit.getEditCommit();
+ ObjectId newEditCommitId =
+ createCommit(
+ objectInserter,
+ basePatchSetCommit,
+ newTreeId,
+ currentEditCommit.getFullMessage(),
+ currentEditCommit.getAuthorIdent(),
+ new PersonIdent(currentEditCommit.getCommitterIdent(), timestamp));
+
+ CodeReviewCommit newEditCommit = revWalk.parseCommit(newEditCommitId);
+ newEditCommit.setFilesWithGitConflicts(filesWithGitConflicts);
+ return newEditCommit;
+ }
+ }
+
+ private static ObjectId mergeTrees(Repository repository, ChangeEdit changeEdit, ObjectId treeId)
+ throws IOException, MergeConflictException {
+ PatchSet basePatchSet = changeEdit.getBasePatchSet();
+ ObjectId basePatchSetCommitId = basePatchSet.commitId();
+ ObjectId editCommitId = changeEdit.getEditCommit();
+
+ ThreeWayMerger merger = MergeStrategy.RESOLVE.newMerger(repository, true);
+ merger.setBase(basePatchSetCommitId);
+ boolean successful = merger.merge(treeId, editCommitId);
if (!successful) {
throw new MergeConflictException(
"Rebasing change edit onto another patchset results in merge conflicts. Download the edit"
+ " patchset and rebase manually to preserve changes.");
}
- return threeWayMerger.getResultTreeId();
+ return merger.getResultTreeId();
}
private String createNewCommitMessage(
@@ -606,18 +704,30 @@
PersonIdent committer)
throws IOException {
try (ObjectInserter objectInserter = repository.newObjectInserter()) {
- CommitBuilder builder = new CommitBuilder();
- builder.setTreeId(tree);
- builder.setParentIds(basePatchsetCommit.getParents());
- builder.setAuthor(author);
- builder.setCommitter(committer);
- builder.setMessage(commitMessage);
- ObjectId newCommitId = objectInserter.insert(builder);
- objectInserter.flush();
- return newCommitId;
+ return createCommit(
+ objectInserter, basePatchsetCommit, tree, commitMessage, author, committer);
}
}
+ private ObjectId createCommit(
+ ObjectInserter objectInserter,
+ RevCommit basePatchsetCommit,
+ ObjectId tree,
+ String commitMessage,
+ PersonIdent author,
+ PersonIdent committer)
+ throws IOException {
+ CommitBuilder builder = new CommitBuilder();
+ builder.setTreeId(tree);
+ builder.setParentIds(basePatchsetCommit.getParents());
+ builder.setAuthor(author);
+ builder.setCommitter(committer);
+ builder.setMessage(commitMessage);
+ ObjectId newCommitId = objectInserter.insert(builder);
+ objectInserter.flush();
+ return newCommitId;
+ }
+
private PersonIdent getCommitterIdent(RevCommit basePatchsetCommit, Instant commitTimestamp) {
IdentifiedUser user = currentUser.get().asIdentifiedUser();
return Optional.ofNullable(basePatchsetCommit.getCommitterIdent())
@@ -690,7 +800,7 @@
if (ObjectId.isEqual(changeEdit.getEditCommit(), commitToModify)) {
return newTreeId;
}
- return merge(repository, changeEdit, newTreeId);
+ return mergeTrees(repository, changeEdit, newTreeId);
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java b/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
index ecf808d..4c63453 100644
--- a/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
+++ b/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.mail.MailUtil;
+import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -41,12 +42,43 @@
public class FromAddressGeneratorProvider implements Provider<FromAddressGenerator> {
private final FromAddressGenerator generator;
+ public static class UserAddressGenModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(UserAddressGenFactory.class).to(DefaultUserAddressGenFactory.class);
+ }
+ }
+
+ /** A generic interface for creating user address generators. */
+ public interface UserAddressGenFactory {
+ FromAddressGenerator create(
+ AccountCache accountCache,
+ Pattern domainPattern,
+ String anonymousCowardName,
+ ParameterizedString nameRewriteTmpl,
+ Address serverAddress);
+ }
+
+ public static class DefaultUserAddressGenFactory implements UserAddressGenFactory {
+ @Override
+ public FromAddressGenerator create(
+ AccountCache accountCache,
+ Pattern domainPattern,
+ String anonymousCowardName,
+ ParameterizedString nameRewriteTmpl,
+ Address serverAddress) {
+ return new UserGen(
+ accountCache, domainPattern, anonymousCowardName, nameRewriteTmpl, serverAddress);
+ }
+ }
+
@Inject
FromAddressGeneratorProvider(
@GerritServerConfig Config cfg,
@AnonymousCowardName String anonymousCowardName,
@GerritPersonIdent PersonIdent myIdent,
- AccountCache accountCache) {
+ AccountCache accountCache,
+ UserAddressGenFactory userAddressGenFactory) {
final String from = cfg.getString("sendemail", null, "from");
final Address srvAddr = toAddress(myIdent);
@@ -58,7 +90,8 @@
Pattern domainPattern = MailUtil.glob(domains);
ParameterizedString namePattern = new ParameterizedString("${user} (Code Review)");
generator =
- new UserGen(accountCache, domainPattern, anonymousCowardName, namePattern, srvAddr);
+ userAddressGenFactory.create(
+ accountCache, domainPattern, anonymousCowardName, namePattern, srvAddr);
} else if ("SERVER".equalsIgnoreCase(from)) {
generator = new ServerGen(srvAddr);
} else {
diff --git a/java/com/google/gerrit/server/patch/ApplyPatchUtil.java b/java/com/google/gerrit/server/patch/ApplyPatchUtil.java
index a319a11..11471c5 100644
--- a/java/com/google/gerrit/server/patch/ApplyPatchUtil.java
+++ b/java/com/google/gerrit/server/patch/ApplyPatchUtil.java
@@ -23,7 +23,6 @@
import com.google.gerrit.extensions.api.changes.ApplyPatchInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.patch.DiffUtil;
import com.google.gerrit.server.util.CommitMessageUtil;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
diff --git a/java/com/google/gerrit/server/project/DefaultProjectNameLockManager.java b/java/com/google/gerrit/server/project/DefaultLockManager.java
similarity index 71%
rename from java/com/google/gerrit/server/project/DefaultProjectNameLockManager.java
rename to java/com/google/gerrit/server/project/DefaultLockManager.java
index 762e244..ab1148e 100644
--- a/java/com/google/gerrit/server/project/DefaultProjectNameLockManager.java
+++ b/java/com/google/gerrit/server/project/DefaultLockManager.java
@@ -15,28 +15,26 @@
package com.google.gerrit.server.project;
import com.google.common.util.concurrent.Striped;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.inject.AbstractModule;
import com.google.inject.Singleton;
import java.util.concurrent.locks.Lock;
-/** In-memory lock for project names. */
+/** In-memory lock manager */
@Singleton
-public class DefaultProjectNameLockManager implements ProjectNameLockManager {
+public class DefaultLockManager implements LockManager {
- public static class DefaultProjectNameLockManagerModule extends AbstractModule {
+ public static class DefaultLockManagerModule extends AbstractModule {
@Override
protected void configure() {
- DynamicItem.bind(binder(), ProjectNameLockManager.class)
- .to(DefaultProjectNameLockManager.class);
+ DynamicItem.bind(binder(), LockManager.class).to(DefaultLockManager.class);
}
}
Striped<Lock> locks = Striped.lock(10);
@Override
- public Lock getLock(Project.NameKey name) {
+ public Lock getLock(String name) {
return locks.get(name);
}
}
diff --git a/java/com/google/gerrit/server/project/ProjectNameLockManager.java b/java/com/google/gerrit/server/project/LockManager.java
similarity index 67%
rename from java/com/google/gerrit/server/project/ProjectNameLockManager.java
rename to java/com/google/gerrit/server/project/LockManager.java
index f67dd04..8a85b32 100644
--- a/java/com/google/gerrit/server/project/ProjectNameLockManager.java
+++ b/java/com/google/gerrit/server/project/LockManager.java
@@ -14,17 +14,19 @@
package com.google.gerrit.server.project;
-import com.google.gerrit.entities.Project;
import java.util.concurrent.locks.Lock;
/**
- * A per-repo lock mechanism.
- *
- * <p>This ensures that project creation (repo creation, config creation, first commit) is atomic,
- * and can be used to separate creation and deletion in the delete-project plugin.
+ * A global locking mechanism.
*
* <p>This is an interface because distributed setup may need something beyond an in-memory lock.
+ *
+ * <p>A Gerrit system consisting of a single Gerrit server only needs an in-memory lock manager
+ * which is implemented by the DefaultLockManager.
+ *
+ * <p>A distributed setup, consisting of more than one Gerrit server, can implement a distributed
+ * lock manager that provides global locks.
*/
-public interface ProjectNameLockManager {
- public Lock getLock(Project.NameKey name);
+public interface LockManager {
+ public Lock getLock(String name);
}
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java b/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
index a55ef84b..4e05420 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
@@ -16,10 +16,12 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
+import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.entities.Comment.Range;
import com.google.gerrit.entities.FixReplacement;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.changes.ApplyPatchInput;
import com.google.gerrit.extensions.common.ApplyProvidedFixInput;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -27,15 +29,19 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditJson;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.edit.CommitModification;
+import com.google.gerrit.server.edit.tree.ChangeFileContentModification;
import com.google.gerrit.server.fixes.FixReplacementInterpreter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.patch.ApplyPatchUtil;
+import com.google.gerrit.server.patch.MagicFile;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.ProjectCache;
@@ -45,7 +51,14 @@
import java.io.IOException;
import java.util.List;
import java.util.stream.Collectors;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.patch.PatchApplier;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.TreeWalk;
/** Applies a fix that is provided as part of the request body. */
@Singleton
@@ -74,7 +87,7 @@
public Response<EditInfo> apply(
RevisionResource revisionResource, ApplyProvidedFixInput applyProvidedFixInput)
throws AuthException, BadRequestException, ResourceConflictException, IOException,
- ResourceNotFoundException, PermissionBackendException {
+ ResourceNotFoundException, PermissionBackendException, RestApiException {
if (applyProvidedFixInput == null) {
throw new BadRequestException("applyProvidedFixInput is required");
}
@@ -83,9 +96,19 @@
}
Project.NameKey project = revisionResource.getProject();
ProjectState projectState = projectCache.get(project).orElseThrow(illegalState(project));
- PatchSet patchSet = revisionResource.getPatchSet();
+ PatchSet targetPatchSet = revisionResource.getPatchSet();
ChangeNotes changeNotes = revisionResource.getNotes();
+ PatchSet originPatchSetForFix =
+ applyProvidedFixInput.originalPatchsetForFix != null
+ && applyProvidedFixInput.originalPatchsetForFix > 0
+ ? changeNotes
+ .getPatchSets()
+ .get(
+ PatchSet.id(
+ revisionResource.getChange().getId(),
+ applyProvidedFixInput.originalPatchsetForFix))
+ : targetPatchSet;
List<FixReplacement> fixReplacements =
applyProvidedFixInput.fixReplacementInfos.stream()
@@ -94,15 +117,87 @@
try (Repository repository = gitRepositoryManager.openRepository(project)) {
CommitModification commitModification =
- fixReplacementInterpreter.toCommitModification(
- repository, projectState, patchSet.commitId(), fixReplacements);
+ getCommitModification(
+ repository, projectState, originPatchSetForFix, targetPatchSet, fixReplacements);
ChangeEdit changeEdit =
changeEditModifier.combineWithModifiedPatchSetTree(
- repository, changeNotes, patchSet, commitModification);
+ repository, changeNotes, targetPatchSet, commitModification);
return Response.ok(changeEditJson.toEditInfo(changeEdit, false));
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
}
+
+ /**
+ * Returns CommitModification for fixes and rebase it if the fix is for an older patchset.
+ *
+ * <p>The method creates CommitModification by applying {@code fixReplacements} to the {@code
+ * basePatchSetForFix}. If the {@code targetPatchSetForFix} is different from the {@code
+ * basePatchSetForFix}, CommitModification is created from the {@link PatchApplier.Result}, after
+ * applying the patch generated from {@code basePatchSetForFix} to the {@code
+ * targetPatchSetForFix}.
+ *
+ * <p>Note: if there is a fix for a commit message and commit messages are different in {@code
+ * basePatchSetForFix} and {@code targetPatchSetForFix}, the method can't move the fix to the
+ * {@code targetPatchSetForFix} and throws {@link ResourceConflictException}. This limitations
+ * exists because the method uses ApplyPatchUtil which operates only on files.
+ */
+ private CommitModification getCommitModification(
+ Repository repository,
+ ProjectState projectState,
+ PatchSet basePatchSetForFix,
+ PatchSet targetPatchSetForFix,
+ List<FixReplacement> fixReplacements)
+ throws IOException, InvalidChangeOperationException, RestApiException {
+ CommitModification originCommitModification =
+ fixReplacementInterpreter.toCommitModification(
+ repository, projectState, basePatchSetForFix.commitId(), fixReplacements);
+ if (basePatchSetForFix.id().equals(targetPatchSetForFix.id())) {
+ return originCommitModification;
+ }
+ RevCommit originCommit = repository.parseCommit(basePatchSetForFix.commitId());
+ ObjectId newTreeId =
+ ChangeEditModifier.createNewTree(
+ repository, originCommit, originCommitModification.treeModifications());
+ CommitModification.Builder resultBuilder = CommitModification.builder();
+ String patch;
+ try (RevWalk rw = new RevWalk(repository)) {
+ ObjectId targetCommit = targetPatchSetForFix.commitId();
+ if (originCommitModification.newCommitMessage().isPresent()) {
+ MagicFile originCommitMessageFile =
+ MagicFile.forCommitMessage(rw.getObjectReader(), originCommit);
+ String originCommitMessage = originCommitMessageFile.modifiableContent();
+ MagicFile targetCommitMessageFile =
+ MagicFile.forCommitMessage(rw.getObjectReader(), targetCommit);
+ String targetCommitMessage = targetCommitMessageFile.modifiableContent();
+ if (!originCommitMessage.equals(targetCommitMessage)) {
+ throw new ResourceConflictException(
+ "The fix attempts to modify commit message of an older patchset, but commit message has been updated in a newer patchset. The fix can't be applied.");
+ }
+ resultBuilder.newCommitMessage(originCommitModification.newCommitMessage().get());
+ }
+
+ patch =
+ ApplyPatchUtil.getResultPatch(
+ repository, repository.newObjectReader(), originCommit, rw.lookupTree(newTreeId));
+ PatchApplier.Result result;
+ try (ObjectInserter oi = repository.newObjectInserter()) {
+ ApplyPatchInput inp = new ApplyPatchInput();
+ inp.patch = patch;
+ inp.allowConflicts = false;
+ result =
+ ApplyPatchUtil.applyPatch(repository, oi, inp, repository.parseCommit(targetCommit));
+ oi.flush();
+ for (String path : result.getPaths()) {
+ try (TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), path, result.getTreeId())) {
+ ObjectLoader loader = rw.getObjectReader().open(tw.getObjectId(0));
+ resultBuilder.addTreeModification(
+ new ChangeFileContentModification(path, RawInputUtil.create(loader.getBytes())));
+ }
+ }
+ }
+ }
+ return resultBuilder.build();
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/change/PreviewFix.java b/java/com/google/gerrit/server/restapi/change/PreviewFix.java
index e771898..042f73d 100644
--- a/java/com/google/gerrit/server/restapi/change/PreviewFix.java
+++ b/java/com/google/gerrit/server/restapi/change/PreviewFix.java
@@ -128,6 +128,11 @@
if (applyProvidedFixInput.fixReplacementInfos == null) {
throw new BadRequestException("applyProvidedFixInput.fixReplacementInfos is required");
}
+ if (applyProvidedFixInput.originalPatchsetForFix != null
+ && applyProvidedFixInput.originalPatchsetForFix > 0) {
+ throw new BadRequestException(
+ "applyProvidedFixInput.originalPatchsetForFix is not supported on preview.");
+ }
PreviewFix previewFix = previewFixFactory.create(revisionResource);
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
index 9fb8de8..ad35b37 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
@@ -14,42 +14,70 @@
package com.google.gerrit.server.restapi.change;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.common.Input;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
+import com.google.gerrit.extensions.common.EditInfo;
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.RestModifyView;
import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.edit.ChangeEdit;
+import com.google.gerrit.server.edit.ChangeEditJson;
import com.google.gerrit.server.edit.ChangeEditModifier;
+import com.google.gerrit.server.edit.ChangeEditUtil;
+import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
import org.eclipse.jgit.lib.Repository;
@Singleton
-public class RebaseChangeEdit implements RestModifyView<ChangeResource, Input> {
+public class RebaseChangeEdit implements RestModifyView<ChangeResource, RebaseChangeEditInput> {
private final GitRepositoryManager repositoryManager;
private final ChangeEditModifier editModifier;
+ private final ChangeEditUtil editUtil;
+ private final ChangeEditJson editJson;
@Inject
- RebaseChangeEdit(GitRepositoryManager repositoryManager, ChangeEditModifier editModifier) {
+ RebaseChangeEdit(
+ GitRepositoryManager repositoryManager,
+ ChangeEditModifier editModifier,
+ ChangeEditUtil editUtil,
+ ChangeEditJson editJson) {
this.repositoryManager = repositoryManager;
this.editModifier = editModifier;
+ this.editUtil = editUtil;
+ this.editJson = editJson;
}
@Override
- public Response<Object> apply(ChangeResource rsrc, Input in)
+ public Response<EditInfo> apply(ChangeResource rsrc, RebaseChangeEditInput input)
throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
+ if (input == null) {
+ input = new RebaseChangeEditInput();
+ }
+
Project.NameKey project = rsrc.getProject();
try (Repository repository = repositoryManager.openRepository(project)) {
- editModifier.rebaseEdit(repository, rsrc.getNotes());
+ CodeReviewCommit rebasedChangeEditCommit =
+ editModifier.rebaseEdit(repository, rsrc.getNotes(), input);
+
+ Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getNotes(), rsrc.getUser());
+ checkState(edit.isPresent(), "change edit missing after rebase");
+ EditInfo editInfo = editJson.toEditInfo(edit.get(), /* downloadCommands= */ false);
+ if (!rebasedChangeEditCommit.getFilesWithGitConflicts().isEmpty()) {
+ editInfo.containsGitConflicts = true;
+ }
+ return Response.ok(editInfo);
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
- return Response.none();
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetIndexVersion.java b/java/com/google/gerrit/server/restapi/config/GetIndexVersion.java
index 1955511..98cf7e3 100644
--- a/java/com/google/gerrit/server/restapi/config/GetIndexVersion.java
+++ b/java/com/google/gerrit/server/restapi/config/GetIndexVersion.java
@@ -17,6 +17,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.index.Index;
import com.google.gerrit.index.IndexCollection;
import com.google.gerrit.server.config.IndexVersionResource;
import com.google.gerrit.server.restapi.config.IndexInfo.IndexVersionInfo;
@@ -27,9 +28,10 @@
public Response<IndexVersionInfo> apply(IndexVersionResource rsrc)
throws ResourceNotFoundException {
IndexCollection<?, ?, ?> indexCollection = rsrc.getIndexDefinition().getIndexCollection();
- int version = rsrc.getIndex().getSchema().getVersion();
+ Index<?, ?> i = rsrc.getIndex();
+ int version = i.getSchema().getVersion();
boolean isSearch = indexCollection.getSearchIndex().getSchema().getVersion() == version;
boolean isWrite = indexCollection.getWriteIndex(version) != null;
- return Response.ok(IndexInfo.IndexVersionInfo.create(isWrite, isSearch));
+ return Response.ok(IndexInfo.IndexVersionInfo.create(isWrite, isSearch, i.numDocs()));
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/IndexInfo.java b/java/com/google/gerrit/server/restapi/config/IndexInfo.java
index 5d52fe1..9fba0034 100644
--- a/java/com/google/gerrit/server/restapi/config/IndexInfo.java
+++ b/java/com/google/gerrit/server/restapi/config/IndexInfo.java
@@ -28,15 +28,18 @@
String name, IndexCollection<?, ?, ?> indexCollection) {
ImmutableSortedMap.Builder<Integer, IndexVersionInfo> versions =
ImmutableSortedMap.naturalOrder();
- int searchIndexVersion = indexCollection.getSearchIndex().getSchema().getVersion();
+ Index<?, ?> searchIndex = indexCollection.getSearchIndex();
+ int searchIndexVersion = searchIndex.getSchema().getVersion();
boolean searchIndexAdded = false;
for (Index<?, ?> index : indexCollection.getWriteIndexes()) {
boolean isSearchIndex = index.getSchema().getVersion() == searchIndexVersion;
- versions.put(index.getSchema().getVersion(), IndexVersionInfo.create(true, isSearchIndex));
+ versions.put(
+ index.getSchema().getVersion(),
+ IndexVersionInfo.create(true, isSearchIndex, index.numDocs()));
searchIndexAdded = searchIndexAdded || isSearchIndex;
}
if (!searchIndexAdded) {
- versions.put(searchIndexVersion, IndexVersionInfo.create(false, true));
+ versions.put(searchIndexVersion, IndexVersionInfo.create(false, true, searchIndex.numDocs()));
}
return new AutoValue_IndexInfo(name, versions.build());
@@ -52,12 +55,14 @@
@AutoValue
public abstract static class IndexVersionInfo {
- static IndexVersionInfo create(boolean write, boolean search) {
- return new AutoValue_IndexInfo_IndexVersionInfo(write, search);
+ static IndexVersionInfo create(boolean write, boolean search, int numDocs) {
+ return new AutoValue_IndexInfo_IndexVersionInfo(write, search, numDocs);
}
abstract boolean isWrite();
abstract boolean isSearch();
+
+ abstract int numDocs();
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 8be96b5..6d6d34e 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -45,10 +45,10 @@
import com.google.gerrit.server.plugincontext.PluginItemContext;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.CreateProjectArgs;
+import com.google.gerrit.server.project.LockManager;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectCreator;
import com.google.gerrit.server.project.ProjectJson;
-import com.google.gerrit.server.project.ProjectNameLockManager;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
@@ -78,7 +78,7 @@
private final Provider<PutConfig> putConfig;
private final AllProjectsName allProjects;
private final AllUsersName allUsers;
- private final PluginItemContext<ProjectNameLockManager> lockManager;
+ private final PluginItemContext<LockManager> lockManager;
private final ProjectCreator projectCreator;
private final Config gerritConfig;
@@ -94,7 +94,7 @@
Provider<PutConfig> putConfig,
AllProjectsName allProjects,
AllUsersName allUsers,
- PluginItemContext<ProjectNameLockManager> lockManager,
+ PluginItemContext<LockManager> lockManager,
@GerritServerConfig Config gerritConfig) {
this.projectsCollection = projectsCollection;
this.projectCreator = projectCreator;
@@ -167,7 +167,7 @@
throw new BadRequestException(e.getMessage());
}
- Lock nameLock = lockManager.call(lockManager -> lockManager.getLock(args.getProject()));
+ Lock nameLock = lockManager.call(lockManager -> lockManager.getLock(args.getProject().get()));
nameLock.lock();
try {
try {
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java b/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java
index 33f0b58..b942ec8 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjectsImpl.java
@@ -21,6 +21,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Joiner;
+import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Iterables;
@@ -562,7 +563,18 @@
private Stream<ProjectState> filter(PermissionBackend.WithUser perm) throws BadRequestException {
return StreamSupport.stream(scan().spliterator(), false)
- .map(projectCache::get)
+ .map(
+ key -> {
+ try {
+ return projectCache.get(key);
+ } catch (StorageException e) {
+ if (Throwables.getCausalChain(e).stream().anyMatch(IOException.class::isInstance)) {
+ logger.atSevere().log(
+ "Unable to load project %s : %s", key.get(), e.getCause().getMessage());
+ }
+ return Optional.<ProjectState>empty();
+ }
+ })
.filter(Optional::isPresent)
.map(Optional::get)
.filter(p -> permissionCheck(p, perm));
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index 8e065b9..789655f 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -49,6 +49,7 @@
import com.google.gerrit.server.Sequence.LightweightGroups;
import com.google.gerrit.server.account.AccountCacheImpl;
import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCacheImpl;
import com.google.gerrit.server.account.storage.notedb.AccountNoteDbReadStorageModule;
import com.google.gerrit.server.account.storage.notedb.AccountNoteDbWriteStorageModule;
import com.google.gerrit.server.api.GerritApiModule;
@@ -103,6 +104,7 @@
import com.google.gerrit.server.index.group.GroupSchemaDefinitions;
import com.google.gerrit.server.mail.EmailModule;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier.SignedTokenEmailTokenVerifierModule;
+import com.google.gerrit.server.mail.send.FromAddressGeneratorProvider;
import com.google.gerrit.server.notedb.NoteDbDraftCommentsModule;
import com.google.gerrit.server.notedb.NoteDbStarredChangesModule;
import com.google.gerrit.server.notedb.RepoSequence.DisabledGitRefUpdatedRepoGroupsSequenceProvider;
@@ -110,7 +112,7 @@
import com.google.gerrit.server.patch.DiffExecutor;
import com.google.gerrit.server.permissions.DefaultPermissionBackendModule;
import com.google.gerrit.server.plugins.ServerInformationImpl;
-import com.google.gerrit.server.project.DefaultProjectNameLockManager.DefaultProjectNameLockManagerModule;
+import com.google.gerrit.server.project.DefaultLockManager.DefaultLockManagerModule;
import com.google.gerrit.server.restapi.RestApiModule;
import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore;
import com.google.gerrit.server.schema.SchemaCreator;
@@ -207,7 +209,10 @@
install(cfgInjector.getInstance(GerritGlobalModule.class));
install(new AccountNoteDbWriteStorageModule());
install(new AccountNoteDbReadStorageModule());
+ install(new ExternalIdCacheImpl.ExternalIdCacheModule());
+ install(new ExternalIdCacheImpl.ExternalIdCacheBindingModule());
install(new RepoSequenceModule());
+ install(new FromAddressGeneratorProvider.UserAddressGenModule());
install(new NoteDbDraftCommentsModule());
install(new NoteDbStarredChangesModule());
install(new ChangeCleanupRunnerModule());
@@ -297,7 +302,7 @@
bind(ServerInformation.class).to(ServerInformationImpl.class);
install(new RestApiModule());
install(new OAuthRestModule());
- install(new DefaultProjectNameLockManagerModule());
+ install(new DefaultLockManagerModule());
install(new FileInfoJsonModule());
install(new ConfigExperimentFeaturesModule());
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
index b9ef0bf..d66a0a5 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
@@ -357,6 +357,142 @@
}
@Test
+ public void applyProvidedFixesOnNewerPatchsetWithModifiedFile() throws Exception {
+ // Remember patch set and add another one.
+ int previousRevision = gApi.changes().id(changeId).get().currentRevisionNumber;
+ amendChange(
+ changeId,
+ "refs/for/master",
+ admin,
+ testRepo,
+ PushOneCommit.SUBJECT,
+ FILE_NAME,
+ "New line at the start\n" + FILE_CONTENT);
+ ApplyProvidedFixInput applyProvidedFixInput =
+ createApplyProvidedFixInput(FILE_NAME, "Modified content", 3, 1, 3, 3);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+ gApi.changes().id(changeId).current().applyFix(applyProvidedFixInput);
+
+ Optional<BinaryResult> file = gApi.changes().id(changeId).edit().getFile(FILE_NAME);
+ BinaryResultSubject.assertThat(file)
+ .value()
+ .asString()
+ .isEqualTo(
+ "New line at the start\nFirst line\nSecond line\nTModified contentrd line\nFourth line\nFifth line\n"
+ + "Sixth line\nSeventh line\nEighth line\nNinth line\nTenth line\n");
+
+ applyProvidedFixInput = createApplyProvidedFixInput(FILE_NAME, "(1st)", 1, 5, 1, 5);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+ gApi.changes().id(changeId).current().applyFix(applyProvidedFixInput);
+
+ file = gApi.changes().id(changeId).edit().getFile(FILE_NAME);
+ BinaryResultSubject.assertThat(file)
+ .value()
+ .asString()
+ .isEqualTo(
+ "New line at the start\nFirst(1st) line\nSecond line\nTModified contentrd line\nFourth line\nFifth line\n"
+ + "Sixth line\nSeventh line\nEighth line\nNinth line\nTenth line\n");
+ }
+
+ @Test
+ public void applyProvidedFixWithTwoFilesOnNewerPatchsetWithModifiedFile() throws Exception {
+ // Remember patch set and add another one.
+ int previousRevision = gApi.changes().id(changeId).get().currentRevisionNumber;
+ // Remove first 2 lines;
+ String modifiedContent =
+ FILE_CONTENT.substring(FILE_CONTENT.indexOf("\n", FILE_CONTENT.indexOf("\n") + 1) + 1);
+ amendChange(
+ changeId,
+ "refs/for/master",
+ admin,
+ testRepo,
+ PushOneCommit.SUBJECT,
+ FILE_NAME,
+ modifiedContent);
+ amendChange(
+ changeId,
+ "refs/for/master",
+ admin,
+ testRepo,
+ PushOneCommit.SUBJECT,
+ FILE_NAME2,
+ "New line at the start\n" + FILE_CONTENT2);
+ FixReplacementInfo fixReplacementInfo1 = new FixReplacementInfo();
+ fixReplacementInfo1.path = FILE_NAME;
+ fixReplacementInfo1.range = createRange(10, 0, 10, 0);
+ fixReplacementInfo1.replacement = "First modification\n";
+
+ FixReplacementInfo fixReplacementInfo2 = new FixReplacementInfo();
+ fixReplacementInfo2.path = FILE_NAME2;
+ fixReplacementInfo2.range = createRange(1, 0, 2, 0);
+ fixReplacementInfo2.replacement = "Different file modification\n";
+
+ ApplyProvidedFixInput applyProvidedFixInput = new ApplyProvidedFixInput();
+
+ applyProvidedFixInput.fixReplacementInfos =
+ Arrays.asList(fixReplacementInfo1, fixReplacementInfo2);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+
+ gApi.changes().id(changeId).current().applyFix(applyProvidedFixInput);
+
+ Optional<BinaryResult> file = gApi.changes().id(changeId).edit().getFile(FILE_NAME);
+ BinaryResultSubject.assertThat(file)
+ .value()
+ .asString()
+ .isEqualTo(
+ "Third line\nFourth line\nFifth line\n"
+ + "Sixth line\nSeventh line\nEighth line\nNinth line\nFirst modification\nTenth line\n");
+ Optional<BinaryResult> file2 = gApi.changes().id(changeId).edit().getFile(FILE_NAME2);
+ BinaryResultSubject.assertThat(file2)
+ .value()
+ .asString()
+ .isEqualTo("New line at the start\nDifferent file modification\n2nd line\n3rd line\n");
+ }
+
+ @Test
+ public void applyProvidedFixOnCommitMessageCanBeAppliedToNewerPatchset() throws Exception {
+ // Set a dedicated commit message.
+ String footer = "\nChange-Id: " + changeId + "\n";
+ String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n" + footer;
+ gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
+ gApi.changes().id(changeId).edit().publish();
+ int previousRevision = gApi.changes().id(changeId).get().currentRevisionNumber;
+ // Upload a new patchset with the same commit message.
+ amendChange(changeId, originalCommitMessage, FILE_NAME, "a" + FILE_CONTENT);
+ ApplyProvidedFixInput applyProvidedFixInput =
+ createApplyProvidedFixInput(Patch.COMMIT_MSG, "Modified line\n", 7, 0, 8, 0);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+
+ gApi.changes().id(changeId).current().applyFix(applyProvidedFixInput);
+ String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+ assertThat(commitMessage).isEqualTo("Modified line\nLine 2 of commit message\n" + footer);
+ }
+
+ @Test
+ public void applyProvidedFixOnCommitMessageRejectedIfNewerPatchsetHasDifferentCommitMessage()
+ throws Exception {
+ // Set a dedicated commit message.
+ String footer = "\nChange-Id: " + changeId + "\n";
+ String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n" + footer;
+ gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
+ gApi.changes().id(changeId).edit().publish();
+ int previousRevision = gApi.changes().id(changeId).get().currentRevisionNumber;
+ // Upload a new patchset with the same commit message.
+ amendChange(changeId, "a" + originalCommitMessage, FILE_NAME, "a" + FILE_CONTENT);
+ ApplyProvidedFixInput applyProvidedFixInput =
+ createApplyProvidedFixInput(Patch.COMMIT_MSG, "Modified line\n", 7, 0, 8, 0);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(changeId).current().applyFix(applyProvidedFixInput));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("commit message has been updated in a newer patchset");
+ }
+
+ @Test
public void applyProvidedFixOnCurrentPatchSetWithChangeEditOnPreviousPatchSetCannotBeApplied()
throws Exception {
// Create an empty change edit.
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/PreviewProvidedFixIT.java b/javatests/com/google/gerrit/acceptance/api/revision/PreviewProvidedFixIT.java
index c257e703..c5dba34 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/PreviewProvidedFixIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/PreviewProvidedFixIT.java
@@ -29,6 +29,7 @@
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.DiffInfo.IntraLineStatus;
import com.google.gerrit.extensions.common.FixReplacementInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import java.util.Arrays;
import java.util.List;
@@ -169,6 +170,19 @@
}
@Test
+ public void previewFixForDifferentPatchset() throws Exception {
+ int previousRevision = gApi.changes().id(changeId).get().currentRevisionNumber;
+ amendChange(changeId);
+ ApplyProvidedFixInput applyProvidedFixInput =
+ createApplyProvidedFixInput(FILE_NAME, "Modified content\n", 1, 0, 2, 0);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(changeId).current().getFixPreview(applyProvidedFixInput));
+ }
+
+ @Test
public void previewFixForCommitMsg() throws Exception {
String footer = "Change-Id: " + changeId;
updateCommitMessage(
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 9f673c8..8d86b1e 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -37,6 +37,7 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -54,6 +55,7 @@
import com.google.gerrit.extensions.api.changes.FileContentInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
+import com.google.gerrit.extensions.api.changes.RebaseChangeEditInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewerInput;
import com.google.gerrit.extensions.client.ChangeEditDetailOption;
@@ -72,8 +74,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.restapi.change.ChangeEdits;
@@ -105,8 +109,10 @@
private static final String FILE_NAME3 = "foo3";
private static final String FILE_NAME4 = "foo4";
private static final int FILE_MODE = 100644;
- private static final byte[] CONTENT_OLD = "bar".getBytes(UTF_8);
- private static final byte[] CONTENT_NEW = "baz".getBytes(UTF_8);
+ private static final String CONTENT_OLD_STR = "bar";
+ private static final byte[] CONTENT_OLD = CONTENT_OLD_STR.getBytes(UTF_8);
+ private static final String CONTENT_NEW_STR = "baz";
+ private static final byte[] CONTENT_NEW = CONTENT_NEW_STR.getBytes(UTF_8);
private static final String CONTENT_NEW2_STR = "quxÄÜÖßµ";
private static final byte[] CONTENT_NEW2 = CONTENT_NEW2_STR.getBytes(UTF_8);
private static final String CONTENT_BINARY_ENCODED_NEW =
@@ -269,12 +275,145 @@
Optional<EditInfo> originalEdit = getEdit(changeId2);
assertThat(originalEdit).value().baseRevision().isEqualTo(previousPatchSet.commitId().name());
Timestamp beforeRebase = originalEdit.get().commit.committer.date;
- gApi.changes().id(changeId2).edit().rebase();
+ EditInfo rebasedEdit = gApi.changes().id(changeId2).edit().rebase(new RebaseChangeEditInput());
ensureSameBytes(getFileContentOfEdit(changeId2, FILE_NAME), CONTENT_NEW);
ensureSameBytes(getFileContentOfEdit(changeId2, FILE_NAME2), CONTENT_NEW2);
- Optional<EditInfo> rebasedEdit = getEdit(changeId2);
- assertThat(rebasedEdit).value().baseRevision().isEqualTo(currentPatchSet.commitId().name());
- assertThat(rebasedEdit).value().commit().committer().date().isNotEqualTo(beforeRebase);
+ assertThat(rebasedEdit).baseRevision().isEqualTo(currentPatchSet.commitId().name());
+ assertThat(rebasedEdit).commit().committer().date().isNotEqualTo(beforeRebase);
+ }
+
+ @Test
+ public void rebaseEditWithConflictsFails() throws Exception {
+ PatchSet previousPatchSet = getCurrentPatchSet(changeId2);
+ createEmptyEditFor(changeId2);
+ gApi.changes().id(changeId2).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW));
+
+ // add new patch set that touches the same file as the edit
+ addNewPatchSetWithModifiedFile(changeId2, FILE_NAME, new String(CONTENT_NEW2, UTF_8));
+
+ Optional<EditInfo> originalEdit = getEdit(changeId2);
+ assertThat(originalEdit).value().baseRevision().isEqualTo(previousPatchSet.commitId().name());
+
+ MergeConflictException exception =
+ assertThrows(
+ MergeConflictException.class, () -> gApi.changes().id(changeId2).edit().rebase());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Rebasing change edit onto another patchset results in merge conflicts.\n\n"
+ + "merge conflict(s):\n%s\n\n"
+ + "Download the edit patchset and rebase manually to preserve changes.",
+ FILE_NAME));
+ }
+
+ @Test
+ public void rebaseEditWithConflictsAllowed() throws Exception {
+ // Create change where FILE_NAME has OLD_CONTENT
+ String changeId = newChange(admin.newIdent());
+
+ PatchSet previousPatchSet = getCurrentPatchSet(changeId);
+ createEmptyEditFor(changeId);
+ gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW));
+
+ // add new patch set that touches the same file as the edit
+ addNewPatchSetWithModifiedFile(changeId, FILE_NAME, new String(CONTENT_NEW2, UTF_8));
+ PatchSet currentPatchSet = getCurrentPatchSet(changeId);
+
+ Optional<EditInfo> originalEdit = getEdit(changeId);
+ assertThat(originalEdit).value().baseRevision().isEqualTo(previousPatchSet.commitId().name());
+
+ Timestamp beforeRebase = originalEdit.get().commit.committer.date;
+
+ RebaseChangeEditInput input = new RebaseChangeEditInput();
+ input.allowConflicts = true;
+ EditInfo rebasedEdit = gApi.changes().id(changeId).edit().rebase(input);
+
+ ensureSameBytes(
+ getFileContentOfEdit(changeId, FILE_NAME),
+ String.format(
+ "<<<<<<< PATCH SET (%s %s)\n"
+ + "%s\n"
+ + "=======\n"
+ + "%s\n"
+ + ">>>>>>> EDIT (%s %s)\n",
+ ObjectIds.abbreviateName(currentPatchSet.commitId(), 6),
+ gApi.changes().id(changeId).get().subject,
+ CONTENT_NEW2_STR,
+ CONTENT_NEW_STR,
+ ObjectIds.abbreviateName(ObjectId.fromString(originalEdit.get().commit.commit), 6),
+ originalEdit.get().commit.subject)
+ .getBytes(UTF_8));
+ assertThat(rebasedEdit).baseRevision().isEqualTo(currentPatchSet.commitId().name());
+ assertThat(rebasedEdit).commit().committer().date().isNotEqualTo(beforeRebase);
+ assertThat(rebasedEdit).containsGitConflicts().isTrue();
+ }
+
+ @Test
+ @GerritConfig(name = "change.diff3ConflictView", value = "true")
+ public void rebaseEditWithConflictsAllowedUsingDiff3() throws Exception {
+ // Create change where FILE_NAME has OLD_CONTENT
+ String changeId = newChange(admin.newIdent());
+
+ PatchSet previousPatchSet = getCurrentPatchSet(changeId);
+ createEmptyEditFor(changeId);
+ gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW));
+
+ // add new patch set that touches the same file as the edit
+ addNewPatchSetWithModifiedFile(changeId, FILE_NAME, new String(CONTENT_NEW2, UTF_8));
+ PatchSet currentPatchSet = getCurrentPatchSet(changeId);
+
+ Optional<EditInfo> originalEdit = getEdit(changeId);
+ assertThat(originalEdit).value().baseRevision().isEqualTo(previousPatchSet.commitId().name());
+
+ Timestamp beforeRebase = originalEdit.get().commit.committer.date;
+
+ RebaseChangeEditInput input = new RebaseChangeEditInput();
+ input.allowConflicts = true;
+ EditInfo rebasedEdit = gApi.changes().id(changeId).edit().rebase(input);
+
+ ensureSameBytes(
+ getFileContentOfEdit(changeId, FILE_NAME),
+ String.format(
+ "<<<<<<< PATCH SET (%s %s)\n"
+ + "%s\n"
+ + "||||||| BASE\n"
+ + "%s\n"
+ + "=======\n"
+ + "%s\n"
+ + ">>>>>>> EDIT (%s %s)\n",
+ ObjectIds.abbreviateName(currentPatchSet.commitId(), 6),
+ gApi.changes().id(changeId).get().subject,
+ CONTENT_NEW2_STR,
+ CONTENT_OLD_STR,
+ CONTENT_NEW_STR,
+ ObjectIds.abbreviateName(ObjectId.fromString(originalEdit.get().commit.commit), 6),
+ originalEdit.get().commit.subject)
+ .getBytes(UTF_8));
+ assertThat(rebasedEdit).baseRevision().isEqualTo(currentPatchSet.commitId().name());
+ assertThat(rebasedEdit).commit().committer().date().isNotEqualTo(beforeRebase);
+ assertThat(rebasedEdit).containsGitConflicts().isTrue();
+ }
+
+ @Test
+ public void rebaseEditWithConflictsAllowedNoConflicts() throws Exception {
+ PatchSet previousPatchSet = getCurrentPatchSet(changeId2);
+ createEmptyEditFor(changeId2);
+ gApi.changes().id(changeId2).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW));
+ addNewPatchSet(changeId2);
+ PatchSet currentPatchSet = getCurrentPatchSet(changeId2);
+
+ Optional<EditInfo> originalEdit = getEdit(changeId2);
+ assertThat(originalEdit).value().baseRevision().isEqualTo(previousPatchSet.commitId().name());
+ Timestamp beforeRebase = originalEdit.get().commit.committer.date;
+ RebaseChangeEditInput input = new RebaseChangeEditInput();
+ input.allowConflicts = true;
+ EditInfo rebasedEdit = gApi.changes().id(changeId2).edit().rebase(input);
+ ensureSameBytes(getFileContentOfEdit(changeId2, FILE_NAME), CONTENT_NEW);
+ ensureSameBytes(getFileContentOfEdit(changeId2, FILE_NAME2), CONTENT_NEW2);
+ assertThat(rebasedEdit).baseRevision().isEqualTo(currentPatchSet.commitId().name());
+ assertThat(rebasedEdit).commit().committer().date().isNotEqualTo(beforeRebase);
+ assertThat(rebasedEdit).containsGitConflicts().isFalse();
}
@Test
@@ -312,7 +451,7 @@
Optional<EditInfo> originalEdit = getEdit(changeId2);
assertThat(originalEdit).value().baseRevision().isEqualTo(previousPatchSet.commitId().name());
Timestamp beforeRebase = originalEdit.get().commit.committer.date;
- adminRestSession.post(urlRebase(changeId2)).assertNoContent();
+ adminRestSession.post(urlRebase(changeId2)).assertOK();
ensureSameBytes(getFileContentOfEdit(changeId2, FILE_NAME), CONTENT_NEW);
ensureSameBytes(getFileContentOfEdit(changeId2, FILE_NAME2), CONTENT_NEW2);
Optional<EditInfo> rebasedEdit = getEdit(changeId2);
diff --git a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
index 2a06900..c24c1f2 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
@@ -28,6 +28,8 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.change.IndexOperations;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.server.index.change.ChangeIndex;
+import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import java.util.List;
@@ -39,6 +41,8 @@
@Inject private ExtensionRegistry extensionRegistry;
@Inject private IndexOperations.Change changeIndexOperations;
+ @Inject private ChangeIndexCollection changeIndexes;
+
@Test
@GerritConfig(name = "index.autoReindexIfStale", value = "false")
public void indexChange() throws Exception {
@@ -109,4 +113,19 @@
assertThat(ids).doesNotContain(change.getId().get());
}
}
+
+ @Test
+ public void testNumDocs() throws Exception {
+ testNumDocs(changeIndexes.getSearchIndex());
+ for (ChangeIndex i : changeIndexes.getWriteIndexes()) {
+ testNumDocs(i);
+ }
+ }
+
+ private void testNumDocs(ChangeIndex index) throws Exception {
+ int before = index.numDocs();
+ createChange("a change", "a.txt", "test");
+ int after = index.numDocs();
+ assertThat(after).isEqualTo(before + 1);
+ }
}
diff --git a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
index b5c8a50..2f6d565 100644
--- a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
+++ b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
@@ -96,7 +96,7 @@
}
@Test
- public void usePreloadRest() throws Exception {
+ public void usePreloadRestWithBasePatchNum() throws Exception {
Accounts accountsApi = mock(Accounts.class);
when(accountsApi.self()).thenThrow(new AuthException("user needs to be authenticated"));
@@ -114,12 +114,43 @@
when(gerritApi.accounts()).thenReturn(accountsApi);
when(gerritApi.config()).thenReturn(configApi);
- assertThat(dynamicTemplateData(gerritApi, "/c/project/+/123", ""))
+ String requestedPath = "/c/project/+/123/4..6";
+ assertThat(IndexHtmlUtil.computeBasePatchNum(requestedPath)).isEqualTo(4);
+
+ assertThat(dynamicTemplateData(gerritApi, requestedPath, ""))
.containsAtLeast(
"defaultChangeDetailHex", "9996394",
"changeRequestsPath", "changes/project~123");
}
+ @Test
+ public void usePreloadRestWithNoBasePatchNum() throws Exception {
+ Accounts accountsApi = mock(Accounts.class);
+ when(accountsApi.self()).thenThrow(new AuthException("user needs to be authenticated"));
+
+ Server serverApi = mock(Server.class);
+ when(serverApi.getVersion()).thenReturn("123");
+ when(serverApi.topMenus()).thenReturn(ImmutableList.of());
+ ServerInfo serverInfo = new ServerInfo();
+ serverInfo.defaultTheme = "my-default-theme";
+ when(serverApi.getInfo()).thenReturn(serverInfo);
+
+ Config configApi = mock(Config.class);
+ when(configApi.server()).thenReturn(serverApi);
+
+ GerritApi gerritApi = mock(GerritApi.class);
+ when(gerritApi.accounts()).thenReturn(accountsApi);
+ when(gerritApi.config()).thenReturn(configApi);
+
+ String requestedPath = "/c/project/+/123";
+ assertThat(IndexHtmlUtil.computeBasePatchNum(requestedPath)).isEqualTo(0);
+
+ assertThat(dynamicTemplateData(gerritApi, requestedPath, ""))
+ .containsAtLeast(
+ "defaultChangeDetailHex", "1996394",
+ "changeRequestsPath", "changes/project~123");
+ }
+
private static SanitizedContent ordain(String s) {
return UnsafeSanitizedContentOrdainer.ordainAsSafe(
s, SanitizedContent.ContentKind.TRUSTED_RESOURCE_URI);
diff --git a/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java
index 05965fb..14554c4 100644
--- a/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java
+++ b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java
@@ -59,12 +59,12 @@
Ref ref1 = createRefWithNonEmptyTreeCommit(usersRepo, 1, 1000001);
Ref ref2 = createRefWithEmptyTreeCommit(usersRepo, 1, 1000002);
- DeleteZombieCommentsRefs clean =
+ try (DeleteZombieCommentsRefs clean =
new DeleteZombieCommentsRefs(
- new AllUsersName("All-Users"), repoManager, null, (msg) -> {});
- int deletedDrafts = clean.execute();
- assertThat(deletedDrafts).isEqualTo(1);
-
+ new AllUsersName("All-Users"), repoManager, null, (msg) -> {})) {
+ int deletedDrafts = clean.execute();
+ assertThat(deletedDrafts).isEqualTo(1);
+ }
/* Check that ref1 still exists, and ref2 is deleted */
assertThat(usersRepo.exactRef(ref1.getName())).isNotNull();
assertThat(usersRepo.exactRef(ref2.getName())).isNull();
@@ -81,21 +81,21 @@
assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(3);
int cleanupPercentage = 50;
- DeleteZombieCommentsRefs clean =
+ try (DeleteZombieCommentsRefs clean =
new DeleteZombieCommentsRefs(
- new AllUsersName("All-Users"), repoManager, cleanupPercentage, (msg) -> {});
- int deletedDrafts = clean.execute();
- assertThat(deletedDrafts).isEqualTo(1);
+ new AllUsersName("All-Users"), repoManager, cleanupPercentage, (msg) -> {})) {
+ int deletedDrafts = clean.execute();
+ assertThat(deletedDrafts).isEqualTo(1);
+ /* ref1 not deleted, ref2 deleted, ref3 not deleted because of the clean percentage */
+ assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(2);
+ assertThat(usersRepo.exactRef(ref1.getName())).isNotNull();
+ assertThat(usersRepo.exactRef(ref2.getName())).isNull();
+ assertThat(usersRepo.exactRef(ref3.getName())).isNotNull();
- /* ref1 not deleted, ref2 deleted, ref3 not deleted because of the clean percentage */
- assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(2);
- assertThat(usersRepo.exactRef(ref1.getName())).isNotNull();
- assertThat(usersRepo.exactRef(ref2.getName())).isNull();
- assertThat(usersRepo.exactRef(ref3.getName())).isNotNull();
-
- /* Re-execute the cleanup and make sure nothing's changed */
- deletedDrafts = clean.execute();
- assertThat(deletedDrafts).isEqualTo(0);
+ /* Re-execute the cleanup and make sure nothing's changed */
+ deletedDrafts = clean.execute();
+ assertThat(deletedDrafts).isEqualTo(0);
+ }
assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(2);
assertThat(usersRepo.exactRef(ref1.getName())).isNotNull();
assertThat(usersRepo.exactRef(ref2.getName())).isNull();
@@ -103,13 +103,13 @@
/* Increase the cleanup percentage */
cleanupPercentage = 70;
- clean =
+ try (DeleteZombieCommentsRefs clean =
new DeleteZombieCommentsRefs(
- new AllUsersName("All-Users"), repoManager, cleanupPercentage, (msg) -> {});
+ new AllUsersName("All-Users"), repoManager, cleanupPercentage, (msg) -> {})) {
- deletedDrafts = clean.execute();
- assertThat(deletedDrafts).isEqualTo(1);
-
+ int deletedDrafts = clean.execute();
+ assertThat(deletedDrafts).isEqualTo(1);
+ }
/* Now ref3 is deleted */
assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(1);
assertThat(usersRepo.exactRef(ref1.getName())).isNotNull();
@@ -141,11 +141,12 @@
assertThat(usersRepo.getRefDatabase().getRefs().size())
.isEqualTo(goodRefs.size() + badRefs.size());
- DeleteZombieCommentsRefs clean =
+ try (DeleteZombieCommentsRefs clean =
new DeleteZombieCommentsRefs(
- new AllUsersName("All-Users"), repoManager, null, (msg) -> {});
- int deletedDrafts = clean.execute();
- assertThat(deletedDrafts).isEqualTo(5001);
+ new AllUsersName("All-Users"), repoManager, null, (msg) -> {})) {
+ int deletedDrafts = clean.execute();
+ assertThat(deletedDrafts).isEqualTo(5001);
+ }
assertThat(
usersRepo.getRefDatabase().getRefs().stream()
diff --git a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
index 6e3514e..1a51c00 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
@@ -110,6 +110,11 @@
}
@Override
+ public int numDocs() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public ChangeDataSource getSource(Predicate<ChangeData> p, QueryOptions opts)
throws QueryParseException {
return new FakeChangeIndex.Source(p);
diff --git a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
index 629b0cc..8a8db44 100644
--- a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
+++ b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.Address;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.mail.send.FromAddressGeneratorProvider.DefaultUserAddressGenFactory;
import com.google.gerrit.server.util.time.TimeUtil;
import java.util.Arrays;
import java.util.List;
@@ -47,7 +48,9 @@
}
private FromAddressGenerator create() {
- return new FromAddressGeneratorProvider(config, "Anonymous Coward", ident, accountCache).get();
+ return new FromAddressGeneratorProvider(
+ config, "Anonymous Coward", ident, accountCache, new DefaultUserAddressGenFactory())
+ .get();
}
private void setFrom(String newFrom) {
diff --git a/plugins/plugin-manager b/plugins/plugin-manager
index 86f7ec6..ed7870e 160000
--- a/plugins/plugin-manager
+++ b/plugins/plugin-manager
@@ -1 +1 @@
-Subproject commit 86f7ec61a9785df246f653a1336520b9607399b1
+Subproject commit ed7870eb3c8b6e48511d0eb3bd54606927b46019
diff --git a/plugins/replication b/plugins/replication
index 60693ef..90c6420 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 60693efae661105fb6278c16cae9502d29bfa7f3
+Subproject commit 90c64204e1e27ec245f41b3d08b10f431dd72faf
diff --git a/polygerrit-ui/app/elements/admin/gr-server-info/gr-server-info.ts b/polygerrit-ui/app/elements/admin/gr-server-info/gr-server-info.ts
index b67239e..836a5eb2 100644
--- a/polygerrit-ui/app/elements/admin/gr-server-info/gr-server-info.ts
+++ b/polygerrit-ui/app/elements/admin/gr-server-info/gr-server-info.ts
@@ -55,12 +55,6 @@
.genericList tr th:last-of-type {
text-align: left;
}
- .metadataDescription,
- .metadataName,
- .metadataValue,
- .metadataWebLinks {
- white-space: nowrap;
- }
.placeholder {
color: var(--deemphasized-text-color);
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-fix-preview.ts b/polygerrit-ui/app/elements/checks/gr-checks-fix-preview.ts
index e91d9c9..1780839 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-fix-preview.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-fix-preview.ts
@@ -272,7 +272,8 @@
const res = await this.restApiService.applyFixSuggestion(
changeNum,
basePatchNum,
- this.fixSuggestionInfo.replacements
+ this.fixSuggestionInfo.replacements,
+ this.latestPatchNum
);
this.applyingFix = false;
this.reporting.timeEnd(Timing.APPLY_FIX_LOAD, {
@@ -315,16 +316,14 @@
}
private isApplyEditDisabled() {
- if (!this.diff || this.patchSet === undefined) return true;
- return this.patchSet !== this.latestPatchNum;
+ if (this.patchSet === undefined) return true;
+ return !this.diff;
}
private computeApplyFixTooltip() {
if (this.patchSet === undefined) return '';
if (!this.diff) return 'Fix is still loading ...';
- return this.patchSet !== this.latestPatchNum
- ? 'You cannot apply this fix because it is from a previous patchset'
- : '';
+ return '';
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index 7c224a9..5319c90 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -56,12 +56,6 @@
@query('#applyFixDialog')
applyFixDialog?: GrDialog;
- /** The currently observed dialog by `dialogOberserver`. */
- observedDialog?: GrDialog;
-
- /** The current observer observing the `observedDialog`. */
- dialogObserver?: ResizeObserver;
-
@query('#nextFix')
nextFix?: GrButton;
@@ -211,10 +205,6 @@
`;
}
- override disconnectedCallback() {
- super.disconnectedCallback();
- }
-
private renderHeader() {
return html`
<div slot="header">${this.currentFix?.description ?? ''}</div>
@@ -249,13 +239,21 @@
private renderFooter() {
const fixCount = this.fixSuggestions?.length ?? 0;
const reasonForDisabledApplyButton = this.computeTooltip();
- if (fixCount < 2 && !reasonForDisabledApplyButton) return nothing;
- return html`<div slot="footer" class="fix-picker">
- ${when(fixCount >= 2, () =>
- this.renderNavForMultipleSuggestedFixes(fixCount)
- )}
- ${this.renderWarning(reasonForDisabledApplyButton)}
- </div>`;
+ const shouldRenderNav = fixCount >= 2;
+ const shouldRenderWarning = !!reasonForDisabledApplyButton;
+
+ if (!shouldRenderNav && !shouldRenderWarning) return nothing;
+
+ return html`
+ <div slot="footer" class="fix-picker">
+ ${when(shouldRenderNav, () =>
+ this.renderNavForMultipleSuggestedFixes(fixCount)
+ )}
+ ${when(shouldRenderWarning, () =>
+ this.renderWarning(reasonForDisabledApplyButton)
+ )}
+ </div>
+ `;
}
private renderNavForMultipleSuggestedFixes(fixCount: number) {
@@ -392,23 +390,20 @@
private computeTooltip() {
if (!this.change || !this.patchNum) return '';
- return this.latestPatchNum !== this.patchNum
- ? 'You cannot apply this fix because it is from a previous patchset'
- : '';
+ if (this.isApplyFixLoading) return 'Fix is still loading ...';
+ return '';
}
private computeDisableApplyFixButton() {
if (!this.change || !this.patchNum) return true;
- return this.patchNum !== this.latestPatchNum || this.isApplyFixLoading;
+ return this.isApplyFixLoading;
}
// visible for testing
async handleApplyFix(e: Event) {
if (e) e.stopPropagation();
- const changeNum = this.changeNum;
- const patchNum = this.patchNum;
- const change = this.change;
+ const {changeNum, patchNum, change} = this;
if (!changeNum || !patchNum || !change || !this.currentFix) {
throw new Error('Not all required properties are set.');
}
@@ -419,7 +414,8 @@
res = await this.restApiService.applyFixSuggestion(
changeNum,
patchNum,
- this.fixSuggestions[0].replacements
+ this.fixSuggestions[0].replacements,
+ this.latestPatchNum
);
} else {
res = await this.restApiService.applyRobotFixSuggestion(
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
index dc7ab98..4331cf4 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
@@ -162,25 +162,7 @@
await open(TWO_FIXES);
const button = getConfirmButton();
assert.isTrue(button.hasAttribute('disabled'));
- assert.equal(button.getAttribute('title'), '');
- });
-
- test('apply fix button is disabled on older patchset', async () => {
- element.change = element.change = {
- ...createParsedChange(),
- revisions: createRevisions(2),
- current_revision: getCurrentRevision(0),
- };
- element.latestPatchNum = element.change.revisions[
- element.change.current_revision
- ]._number as PatchSetNumber;
- await open(TWO_FIXES);
- const button = getConfirmButton();
- assert.isTrue(button.hasAttribute('disabled'));
- assert.equal(
- button.getAttribute('title'),
- 'You cannot apply this fix because it is from a previous patchset'
- );
+ assert.equal(button.getAttribute('title'), 'Fix is still loading ...');
});
});
diff --git a/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences.ts b/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences.ts
index c97a036..a65c8a4 100644
--- a/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences.ts
+++ b/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences.ts
@@ -124,15 +124,13 @@
() => this.getConfigModel().docsBaseUrl$,
docsBaseUrl => (this.docsBaseUrl = docsBaseUrl)
);
- if (this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT_V2)) {
- subscribe(
- this,
- () => this.getPluginLoader().pluginsModel.suggestionsPlugins$,
- // We currently support results from only 1 provider.
- suggestionsPlugins =>
- (this.suggestionsProvider = suggestionsPlugins?.[0]?.provider)
- );
- }
+ subscribe(
+ this,
+ () => this.getPluginLoader().pluginsModel.suggestionsPlugins$,
+ // We currently support results from only 1 provider.
+ suggestionsPlugins =>
+ (this.suggestionsProvider = suggestionsPlugins?.[0]?.provider)
+ );
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index b55c9fd..7a3b7ec 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -436,32 +436,29 @@
this.autocompleteComment();
}
);
- if (this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT_V2)) {
- subscribe(
- this,
- () =>
- this.generateSuggestionTrigger$.pipe(
- debounceTime(GENERATE_SUGGESTION_DEBOUNCE_DELAY_MS)
- ),
- () => {
- this.generateSuggestEdit();
+ subscribe(
+ this,
+ () =>
+ this.generateSuggestionTrigger$.pipe(
+ debounceTime(GENERATE_SUGGESTION_DEBOUNCE_DELAY_MS)
+ ),
+ () => {
+ this.generateSuggestEdit();
+ }
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().preferences$,
+ prefs => {
+ this.autocompleteEnabled = !!prefs.allow_autocompleting_comments;
+ if (
+ this.generateSuggestion !==
+ !!prefs.allow_suggest_code_while_commenting
+ ) {
+ this.generateSuggestion = !!prefs.allow_suggest_code_while_commenting;
}
- );
- subscribe(
- this,
- () => this.getUserModel().preferences$,
- prefs => {
- this.autocompleteEnabled = !!prefs.allow_autocompleting_comments;
- if (
- this.generateSuggestion !==
- !!prefs.allow_suggest_code_while_commenting
- ) {
- this.generateSuggestion =
- !!prefs.allow_suggest_code_while_commenting;
- }
- }
- );
- }
+ }
+ );
}
override connectedCallback() {
@@ -1150,7 +1147,6 @@
// private but used in test
showGeneratedSuggestion() {
return (
- this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT_V2) &&
this.suggestionsProvider &&
this.editing &&
!this.permanentEditingMode &&
@@ -1224,14 +1220,8 @@
if (this.generateSuggestion) {
this.generateSuggestionTrigger$.next();
} else {
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.ML_SUGGESTED_EDIT_V2
- )
- ) {
- this.generatedFixSuggestion = undefined;
- this.autoSaveTrigger$.next();
- }
+ this.generatedFixSuggestion = undefined;
+ this.autoSaveTrigger$.next();
}
this.reporting.reportInteraction(
this.generateSuggestion
@@ -1240,9 +1230,7 @@
);
}}
/>
- ${this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT_V2)
- ? 'Attach AI-suggested fix'
- : 'Generate Suggestion'}
+ Attach AI-suggested fix
${when(
this.suggestionLoading,
() => html`<span class="loadingSpin"></span>`,
@@ -1278,13 +1266,7 @@
}
}
- private generateSuggestEdit() {
- if (this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT_V2)) {
- this.generateSuggestEdit_v2();
- }
- }
-
- private async generateSuggestEdit_v2() {
+ private async generateSuggestEdit() {
const suggestionsProvider = this.suggestionsProvider;
const changeInfo = this.getChangeModel().getChange();
if (
@@ -1854,8 +1836,6 @@
}
getFixSuggestions(): FixSuggestionInfo[] | undefined {
- if (!this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT_V2))
- return undefined;
if (!this.generateSuggestion) return undefined;
if (!this.generatedFixSuggestion) return undefined;
// Disable fix suggestions when the comment already has a user suggestion
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 7292c64..fe19d9b 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -48,7 +48,6 @@
CommentsModel,
commentsModelToken,
} from '../../../models/comments/comments-model';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {GrSuggestionDiffPreview} from '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
suite('gr-comment tests', () => {
@@ -1005,11 +1004,6 @@
},
],
};
- setup(async () => {
- stubFlags('isEnabled')
- .withArgs(KnownExperimentId.ML_SUGGESTED_EDIT_V2)
- .returns(true);
- });
test('renders suggestions in comment', async () => {
const comment = {
diff --git a/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts b/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
index 36266a0..d03bd54 100644
--- a/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
+++ b/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
@@ -16,7 +16,7 @@
import {configModelToken} from '../../../models/config/config-model';
import {GrSuggestionDiffPreview} from '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
import {changeModelToken} from '../../../models/change/change-model';
-import {Comment, NumericChangeId, PatchSetNumber} from '../../../types/common';
+import {Comment, PatchSetNumber} from '../../../types/common';
import {OpenFixPreviewEventDetail} from '../../../types/events';
import {pluginLoaderToken} from '../gr-js-api-interface/gr-plugin-loader';
import {SuggestionsProvider} from '../../../api/suggestions';
@@ -25,7 +25,7 @@
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
import {getAppContext} from '../../../services/app-context';
import {Interaction} from '../../../constants/reporting';
-import {isFileUnchanged} from '../../../utils/diff-util';
+import {waitUntil} from '../../../utils/async-util';
export const COLLAPSE_SUGGESTION_STORAGE_KEY = 'collapseSuggestionStorageKey';
@@ -48,15 +48,11 @@
@state() latestPatchNum?: PatchSetNumber;
- @state() changeNum?: NumericChangeId;
-
@state()
suggestionsProvider?: SuggestionsProvider;
@state() private isOwner = false;
- @state() private enableApplyOnUnModifiedFile = false;
-
/**
* This is just a reflected property such that css rules can be based on it.
*/
@@ -73,7 +69,7 @@
private readonly reporting = getAppContext().reportingService;
- private readonly restApiService = getAppContext().restApiService;
+ @state() private previewLoaded = false;
constructor() {
super();
@@ -92,17 +88,6 @@
() => this.getChangeModel().isOwner$,
x => (this.isOwner = x)
);
- subscribe(
- this,
- () => this.getChangeModel().changeNum$,
- x => (this.changeNum = x)
- );
- }
-
- override updated(changed: PropertyValues) {
- if (changed.has('changeNum') || changed.has('latestPatchNum')) {
- this.checkIfcanEnableApplyOnUnModifiedFile();
- }
}
override connectedCallback() {
@@ -295,9 +280,7 @@
if (!this.comment?.fix_suggestions) return;
this.applyingFix = true;
try {
- await this.suggestionDiffPreview?.applyFixSuggestion(
- this.enableApplyOnUnModifiedFile
- );
+ await this.suggestionDiffPreview?.applyFixSuggestion();
} finally {
this.applyingFix = false;
}
@@ -305,37 +288,29 @@
private isApplyEditDisabled() {
if (this.comment?.patch_set === undefined) return true;
- if (this.enableApplyOnUnModifiedFile) return false;
- return this.comment.patch_set !== this.latestPatchNum;
+ return !this.previewLoaded;
}
private computeApplyEditTooltip() {
if (this.comment?.patch_set === undefined) return '';
- return this.comment.patch_set !== this.latestPatchNum
- ? 'You cannot apply this fix because it is from a previous patchset'
- : '';
+ if (!this.previewLoaded) return 'Fix is still loading ...';
+ return '';
}
- private async checkIfcanEnableApplyOnUnModifiedFile() {
- // if enabled we don't need to enable
- if (!this.isApplyEditDisabled()) return;
-
- const basePatchNum = this.comment?.patch_set;
- const path = this.comment?.path;
-
- if (!basePatchNum || !this.latestPatchNum || !path || !this.changeNum) {
- return;
+ override updated(changedProperties: PropertyValues) {
+ super.updated(changedProperties);
+ if (changedProperties.has('comment') && this.comment?.fix_suggestions) {
+ this.waitForPreviewToLoad();
}
+ }
- const diff = await this.restApiService.getDiff(
- this.changeNum,
- basePatchNum,
- this.latestPatchNum,
- path
- );
-
- if (diff && isFileUnchanged(diff)) {
- this.enableApplyOnUnModifiedFile = true;
+ private async waitForPreviewToLoad() {
+ this.previewLoaded = false;
+ try {
+ await waitUntil(() => !!this.suggestionDiffPreview?.preview);
+ this.previewLoaded = true;
+ } catch (error) {
+ console.error('Error waiting for preview to load:', error);
}
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
index d059c7c..6fb7d533 100644
--- a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
+++ b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
@@ -313,9 +313,9 @@
* Similar code flow is in gr-apply-fix-dialog.handleApplyFix
* Used in gr-fix-suggestions
*/
- public applyFixSuggestion(onLatestPatchset = false) {
+ public applyFixSuggestion() {
if (this.suggestion || !this.fixSuggestionInfo) return;
- return this.applyFix(this.fixSuggestionInfo, onLatestPatchset);
+ return this.applyFix(this.fixSuggestionInfo);
}
/**
@@ -337,10 +337,7 @@
this.applyFix(fixSuggestions[0]);
}
- private async applyFix(
- fixSuggestion: FixSuggestionInfo,
- onLatestPatchset = false
- ) {
+ private async applyFix(fixSuggestion: FixSuggestionInfo) {
const changeNum = this.changeNum;
const basePatchNum = this.comment?.patch_set as BasePatchSetNum;
if (!changeNum || !basePatchNum || !fixSuggestion) return;
@@ -348,8 +345,9 @@
this.reporting.time(Timing.APPLY_FIX_LOAD);
const res = await this.restApiService.applyFixSuggestion(
changeNum,
- onLatestPatchset ? this.latestPatchNum ?? basePatchNum : basePatchNum,
- fixSuggestion.replacements
+ basePatchNum,
+ fixSuggestion.replacements,
+ this.latestPatchNum
);
this.reporting.timeEnd(Timing.APPLY_FIX_LOAD, {
method: '1-click',
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
index 3f4f99e..14e7134 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
@@ -7,7 +7,7 @@
import '../../shared/gr-icon/gr-icon';
import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
import '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
-import {css, html, LitElement, nothing} from 'lit';
+import {css, html, LitElement, nothing, PropertyValues} from 'lit';
import {customElement, state, query} from 'lit/decorators.js';
import {fire} from '../../../utils/event-util';
import {getDocUrl} from '../../../utils/url-util';
@@ -18,6 +18,7 @@
import {changeModelToken} from '../../../models/change/change-model';
import {Comment, PatchSetNumber} from '../../../types/common';
import {commentModelToken} from '../gr-comment-model/gr-comment-model';
+import {waitUntil} from '../../../utils/async-util';
declare global {
interface HTMLElementEventMap {
@@ -44,6 +45,8 @@
@state() comment?: Comment;
+ @state() private previewLoaded = false;
+
private readonly getConfigModel = resolve(this, configModelToken);
private readonly getChangeModel = resolve(this, changeModelToken);
@@ -154,14 +157,30 @@
private isApplyEditDisabled() {
if (this.comment?.patch_set === undefined) return true;
- return this.comment.patch_set !== this.latestPatchNum;
+ return !this.previewLoaded;
}
private computeApplyEditTooltip() {
if (this.comment?.patch_set === undefined) return '';
- return this.comment.patch_set !== this.latestPatchNum
- ? 'You cannot apply this fix because it is from a previous patchset'
- : '';
+ if (!this.previewLoaded) return 'Fix is still loading ...';
+ return '';
+ }
+
+ override updated(changedProperties: PropertyValues) {
+ super.updated(changedProperties);
+ if (changedProperties.has('textContent') && this.textContent) {
+ this.waitForPreviewToLoad();
+ }
+ }
+
+ private async waitForPreviewToLoad() {
+ this.previewLoaded = false;
+ try {
+ await waitUntil(() => !!this.suggestionDiffPreview?.preview);
+ this.previewLoaded = true;
+ } catch (error) {
+ console.error('Error waiting for preview to load:', error);
+ }
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
index c97d23d..8baee43 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
@@ -76,7 +76,7 @@
role="button"
tabindex="-1"
flatten=""
- title="You cannot apply this fix because it is from a previous patchset"
+ title="Fix is still loading ..."
>Apply edit</gr-button
>
</div>
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 0bf3c97..750b421 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -19,7 +19,6 @@
CHECKS_DEVELOPER = 'UiFeature__checks_developer',
PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
- ML_SUGGESTED_EDIT_V2 = 'UiFeature__ml_suggested_edit_v2',
REVISION_PARENTS_DATA = 'UiFeature__revision_parents_data',
COMMENT_AUTOCOMPLETION = 'UiFeature__comment_autocompletion_enabled',
SAVE_PROJECT_CONFIG_FOR_REVIEW = 'UiFeature__save_project_config_for_review',
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 0765485..a65eddc 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -2368,15 +2368,28 @@
async applyFixSuggestion(
changeNum: NumericChangeId,
- patchNum: PatchSetNum,
- fixReplacementInfos: FixReplacementInfo[]
+ fixPatchNum: PatchSetNum,
+ fixReplacementInfos: FixReplacementInfo[],
+ targetPatchNum?: PatchSetNum
): Promise<Response> {
- const url = await this._changeBaseURL(changeNum, patchNum);
+ const url = await this._changeBaseURL(
+ changeNum,
+ targetPatchNum ?? fixPatchNum
+ );
+ const body: {
+ fix_replacement_infos: FixReplacementInfo[];
+ original_patchset_for_fix?: PatchSetNum;
+ } = {
+ fix_replacement_infos: fixReplacementInfos,
+ };
+ if (targetPatchNum !== undefined && targetPatchNum !== fixPatchNum) {
+ body.original_patchset_for_fix = fixPatchNum;
+ }
return this._restApiHelper.fetch({
fetchOptions: getFetchOptions({
method: HttpMethod.POST,
headers: {Accept: 'application/json'},
- body: {fix_replacement_infos: fixReplacementInfos},
+ body,
}),
url: `${url}/fix:apply`,
anonymizedUrl: `${ANONYMIZED_REVISION_BASE_URL}/fix:apply`,
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
index af12a9c..cb4a43d 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
@@ -20,6 +20,7 @@
createChange,
createComment,
createEditInfo,
+ createFixReplacementInfo,
createParsedChange,
createServerInfo,
TEST_PROJECT_NAME,
@@ -1898,4 +1899,69 @@
anonymizedUrl: '/accounts/self/starred.changes/*',
});
});
+ suite('applyFixSuggestion', () => {
+ const fixReplacementInfo = createFixReplacementInfo();
+ let fetchStub: sinon.SinonStub;
+ setup(() => {
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
+ fetchStub = sinon
+ .stub(element._restApiHelper, 'fetch')
+ .resolves(new Response(makePrefixedJSON({})));
+ });
+ test('applyFixSuggestion without targetPatchNum', async () => {
+ await element.applyFixSuggestion(
+ 123 as NumericChangeId,
+ 1 as PatchSetNum,
+ [fixReplacementInfo]
+ );
+ assert.isTrue(fetchStub.calledOnce);
+ assert.equal(
+ fetchStub.lastCall.args[0].url,
+ '/changes/test-project~123/revisions/1/fix:apply'
+ );
+ const body = JSON.parse(fetchStub.lastCall.args[0].fetchOptions.body);
+ assert.isTrue(
+ Object.keys(body).length === 1 &&
+ body.fix_replacement_infos.length === 1
+ );
+ assert.deepEqual(body.fix_replacement_infos[0], fixReplacementInfo);
+ });
+
+ test('applyFixSuggestion with same patchNum and targetPatchNum', async () => {
+ const fixReplacementInfo = createFixReplacementInfo();
+ await element.applyFixSuggestion(
+ 123 as NumericChangeId,
+ 1 as PatchSetNum,
+ [fixReplacementInfo],
+ 1 as PatchSetNum
+ );
+ assert.isTrue(fetchStub.calledOnce);
+ assert.equal(
+ fetchStub.lastCall.args[0].url,
+ '/changes/test-project~123/revisions/1/fix:apply'
+ );
+ const body = JSON.parse(fetchStub.lastCall.args[0].fetchOptions.body);
+ assert.isTrue(Object.keys(body).length === 1);
+ assert.deepEqual(body.fix_replacement_infos[0], fixReplacementInfo);
+ });
+
+ test('applyFixSuggestion with targetPatchNum', async () => {
+ const fixReplacementInfo = createFixReplacementInfo();
+ await element.applyFixSuggestion(
+ 123 as NumericChangeId,
+ 1 as PatchSetNum,
+ [fixReplacementInfo],
+ 2 as PatchSetNum
+ );
+ assert.isTrue(fetchStub.calledOnce);
+ assert.equal(
+ fetchStub.lastCall.args[0].url,
+ '/changes/test-project~123/revisions/2/fix:apply'
+ );
+ const body = JSON.parse(fetchStub.lastCall.args[0].fetchOptions.body);
+ assert.isTrue(Object.keys(body).length === 2);
+ assert.deepEqual(body.fix_replacement_infos[0], fixReplacementInfo);
+ assert.deepEqual(body.original_patchset_for_fix, 1);
+ });
+ });
});
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index 3f79115..799cf43 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -738,8 +738,9 @@
*/
applyFixSuggestion(
changeNum: NumericChangeId,
- patchNum: PatchSetNum,
- fixReplacementInfos: FixReplacementInfo[]
+ fixPatchNum: PatchSetNum,
+ fixReplacementInfos: FixReplacementInfo[],
+ targetPatchNum?: PatchSetNum
): Promise<Response>;
/**
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index 2309977..5f40cad 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -98,6 +98,7 @@
import {EditRevisionInfo, ParsedChangeInfo} from '../types/types';
import {
DetailedLabelInfo,
+ FixReplacementInfo,
PatchSetNumber,
QuickLabelInfo,
SubmitRequirementExpressionInfo,
@@ -1231,3 +1232,11 @@
export function createQuickLabelInfo(): QuickLabelInfo {
return {};
}
+
+export function createFixReplacementInfo(): FixReplacementInfo {
+ return {
+ path: 'test/path',
+ range: createRange(),
+ replacement: 'replacement',
+ };
+}
diff --git a/tools/bzl/classpath.bzl b/tools/bzl/classpath.bzl
index 3c80fc3..e196c10 100644
--- a/tools/bzl/classpath.bzl
+++ b/tools/bzl/classpath.bzl
@@ -1,3 +1,5 @@
+load("@rules_java//java:defs.bzl", "JavaInfo")
+
def _classpath_collector(ctx):
all = []
for d in ctx.attr.deps:
diff --git a/tools/bzl/javadoc.bzl b/tools/bzl/javadoc.bzl
index 5aba90e..131254e 100644
--- a/tools/bzl/javadoc.bzl
+++ b/tools/bzl/javadoc.bzl
@@ -14,6 +14,8 @@
# Javadoc rule.
+load("@rules_java//java:defs.bzl", "JavaInfo", "java_common")
+
def _impl(ctx):
zip_output = ctx.outputs.zip
diff --git a/tools/bzl/pkg_war.bzl b/tools/bzl/pkg_war.bzl
index 52fa1dd..1c3444e 100644
--- a/tools/bzl/pkg_war.bzl
+++ b/tools/bzl/pkg_war.bzl
@@ -14,6 +14,7 @@
# War packaging.
+load("@rules_java//java:defs.bzl", "JavaInfo")
load("//tools:deps.bzl", "AUTO_VALUE_GSON_VERSION")
load("//tools:nongoogle.bzl", "AUTO_FACTORY_VERSION", "AUTO_VALUE_VERSION")
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index 603e6c8..366f22c 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-acceptance-framework</artifactId>
- <version>3.11.0-SNAPSHOT</version>
+ <version>3.12.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Acceptance Test Framework</name>
<description>Framework for Gerrit's acceptance tests</description>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index 241d59b..59f9016 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-extension-api</artifactId>
- <version>3.11.0-SNAPSHOT</version>
+ <version>3.12.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Extension API</name>
<description>API for Gerrit Extensions</description>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index 00132bd..c549677 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-api</artifactId>
- <version>3.11.0-SNAPSHOT</version>
+ <version>3.12.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin API</name>
<description>API for Gerrit Plugins</description>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index 5a21ab7..9b26260 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-war</artifactId>
- <version>3.11.0-SNAPSHOT</version>
+ <version>3.12.0-SNAPSHOT</version>
<packaging>war</packaging>
<name>Gerrit Code Review - WAR</name>
<description>Gerrit WAR</description>
diff --git a/version.bzl b/version.bzl
index 181159e..8d942e01 100644
--- a/version.bzl
+++ b/version.bzl
@@ -2,4 +2,4 @@
# Used by :api_install and :api_deploy targets
# when talking to the destination repository.
#
-GERRIT_VERSION = "3.11.0-SNAPSHOT"
+GERRIT_VERSION = "3.12.0-SNAPSHOT"
