Merge "Extract methods for rendering header and footer in gr-app-element"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 234389e..5c5c4c0 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -4255,6 +4255,14 @@
+
Default is 5 seconds. Negative values will be converted to 0.
+[[plugins.transitionalPushOptions]]plugins.transitionalPushOptions::
++
+Additional push options which should be accepted by gerrit as valid
+options even if they are not registered by any plugin(e.g. "myplugin~foo").
++
+This config can be used when gerrit migrates from a deprecated plugin to the new one. The new plugin
+can (temporary) accept push options of the old plugin without registering such options.
+
[[receive]]
=== Section receive
diff --git a/Documentation/config-submit-requirements.txt b/Documentation/config-submit-requirements.txt
index 8298be3..601f2bf 100644
--- a/Documentation/config-submit-requirements.txt
+++ b/Documentation/config-submit-requirements.txt
@@ -187,6 +187,20 @@
redefine a submit requirement in a child project and make the submit requirement
always non-applicable.
+[[operator_has_submodule_update]]
+has:submodule-update::
++
+An operator that returns true if the diff of the latest patchset against the
+default parent has a submodule modified file, that is, a ".gitmodules" or a
+git link file.
++
+The optional `base` parameter can also be supplied for merge commits like
+`has:submodule-update,base=1`, or `has:submodule-update,base=2`. In these cases,
+the operator returns true if the diff of the latest patchset against parent
+number identified by `base` has a submodule modified file. Note that the
+operator will return false if the base parameter is greater than the number of
+parents for the latest patchset for the change.
+
[[operator_file]]
file:"'<filePattern>',withDiffContaining='<contentPattern>'"::
+
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index b160f59..cceb2ab 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2124,6 +2124,66 @@
HTTP/1.1 204 No Content
----
+[[apply-patch]]
+=== Create patch-set from patch
+--
+'POST /changes/link:#change-id[\{change-id\}]/patch:apply'
+--
+
+Creates a new patch set on a destination change from the provided patch.
+
+The patch must be provided in the request body, inside a
+link:#applypatchpatchset-input[ApplyPatchPatchSetInput] entity.
+
+If a base commit is given, the patch is applied on top of it. Otherwise, the
+patch is applied on top of the target change's branch tip.
+
+Applying the patch will fail if the destination change is closed, or in case of any conflicts.
+
+.Request
+----
+ POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/patch:apply HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "patch": {
+ "patch": "new file mode 100644\n--- /dev/null\n+++ b/a_new_file.txt\n@@ -0,0 +1,2 @@ \
++Patch compatible `git diff` output \
++For example: `link:#get-patch[<gerrit patch>] | base64 -d | sed -z 's/\n/\\n/g'`"
+ }
+ }
+----
+
+As response a link:#change-info[ChangeInfo] entity is returned that
+describes the destination change after applying the patch.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9941",
+ "project": "myProject",
+ "branch": "release-branch",
+ "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9941",
+ "subject": "Original change subject",
+ "status": "NEW",
+ "created": "2013-02-01 09:59:32.126000000",
+ "updated": "2013-02-21 11:16:36.775000000",
+ "mergeable": true,
+ "insertions": 12,
+ "deletions": 11,
+ "_number": 3965,
+ "owner": {
+ "name": "John Doe"
+ },
+ "current_revision": "184ebe53805e102605d11f6b143486d15c23a09c"
+ }
+----
+
[[get-included-in]]
=== Get Included In
--
@@ -6506,6 +6566,46 @@
at the server or permissions are modified. Not present if false.
|====================================
+[[applypatch-input]]
+=== ApplyPatchInput
+The `ApplyPatchInput` entity contains information about a patch to apply.
+
+A new commit will be created from the patch, and saved as a new patch set.
+
+[options="header",cols="1,^1,5"]
+|=================================
+|Field Name ||Description
+|`patch` |required|
+The patch to be applied. Must be compatible with `git diff` output.
+For example, link:#get-patch[Get Patch] output.
+|=================================
+
+[[applypatchpatchset-input]]
+=== ApplyPatchPatchSetInput
+The `ApplyPatchPatchSetInput` entity contains information for creating a new patch set from a
+given patch.
+
+[options="header",cols="1,^1,5"]
+|=================================
+|Field Name ||Description
+|`patch` |required|
+The details of the patch to be applied as a link:#applypatch-input[ApplyPatchInput] entity.
+|`commit_message` |optional|
+The commit message for the new patch set. If not specified, a predefined message will be used.
+|`base` |optional|
+40-hex digit SHA-1 of the commit which will be the parent commit of the newly created patch set.
+If set, it must be a merged commit or a change revision on the destination branch.
+Otherwise, the target change's branch tip will be used.
+|`author` |optional|
+The author of the commit to create. Must be an
+link:rest-api-accounts.html#account-input[AccountInput] entity with at least
+the `name` and `email` fields set.
+The caller needs "Forge Author" permission when using this field, unless specifies their own details.
+This field does not affect the owner of the change, which will continue to use the identity of the
+caller.
+|=================================
+
+
[[approval-info]]
=== ApprovalInfo
The `ApprovalInfo` entity contains information about an approval from a
@@ -6878,10 +6978,12 @@
If set, the target branch (see `branch` field) must exist (it is not
possible to create it automatically by setting the `new_branch` field
to `true`.
+|`patch` |optional|
+The detail of a patch to be applied as an link:#applypatch-input[ApplyPatchInput] entity.
|`author` |optional|
-An link:rest-api-accounts.html#account-input[AccountInput] entity
-that will set the author of the commit to create. The author must be
-specified as name/email combination.
+The author of the commit to create. Must be an
+link:rest-api-accounts.html#account-input[AccountInput] entity with at least
+the `name` and `email` fields set.
The caller needs "Forge Author" permission when using this field.
This field does not affect the owner of the change, which will
continue to use the identity of the caller.
@@ -7719,11 +7821,11 @@
The detail of the source commit for merge as a link:#merge-input[MergeInput]
entity.
|`author` |optional|
-An link:rest-api-accounts.html#account-input[AccountInput] entity
-that will set the author of the commit to create. The author must be
-specified as name/email combination.
+The author of the commit to create. Must be an
+link:rest-api-accounts.html#account-input[AccountInput] entity with at least
+the `name` and `email` fields set.
The caller needs "Forge Author" permission when using this field.
-This field does not affect the owner of the change, which will
+This field does not affect the owner or the committer of the change, which will
continue to use the identity of the caller.
|==================================
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index c5d319f..3e48eec 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1253,34 +1253,61 @@
assertThat(refValues.keySet()).containsAnyIn(trees.keySet());
}
- protected void assertDiffForNewFile(
- DiffInfo diff, RevCommit commit, String path, String expectedContentSideB) throws Exception {
- List<String> expectedLines = ImmutableList.copyOf(expectedContentSideB.split("\n", -1));
+ protected void assertDiffForFullyModifiedFile(
+ DiffInfo diff,
+ String commitName,
+ String path,
+ String expectedContentSideA,
+ String expectedContentSideB)
+ throws Exception {
+ assertDiffForFile(diff, commitName, path);
- assertThat(diff.binary).isNull();
+ ImmutableList<String> expectedOldLines =
+ ImmutableList.copyOf(expectedContentSideA.split("\n", -1));
+ ImmutableList<String> expectedNewLines =
+ ImmutableList.copyOf(expectedContentSideB.split("\n", -1));
+
+ assertThat(diff.changeType).isEqualTo(ChangeType.MODIFIED);
+
+ assertThat(diff.metaA).isNotNull();
+ assertThat(diff.metaB).isNotNull();
+
+ assertThat(diff.metaA.name).isEqualTo(path);
+ assertThat(diff.metaA.lines).isEqualTo(expectedOldLines.size());
+ assertThat(diff.metaB.name).isEqualTo(path);
+ assertThat(diff.metaB.lines).isEqualTo(expectedNewLines.size());
+
+ DiffInfo.ContentEntry contentEntry = diff.content.get(0);
+ assertThat(contentEntry.a).containsExactlyElementsIn(expectedOldLines).inOrder();
+ assertThat(contentEntry.b).containsExactlyElementsIn(expectedNewLines).inOrder();
+ assertThat(contentEntry.ab).isNull();
+ assertThat(contentEntry.common).isNull();
+ assertThat(contentEntry.editA).isNull();
+ assertThat(contentEntry.editB).isNull();
+ assertThat(contentEntry.skip).isNull();
+ }
+
+ protected void assertDiffForNewFile(
+ DiffInfo diff, @Nullable RevCommit commit, String path, String expectedContentSideB)
+ throws Exception {
+ assertDiffForNewFile(diff, commit.name(), path, expectedContentSideB);
+ }
+
+ protected void assertDiffForNewFile(
+ DiffInfo diff, String commitName, String path, String expectedContentSideB) throws Exception {
+ assertDiffForFile(diff, commitName, path);
+
+ ImmutableList<String> expectedLines =
+ ImmutableList.copyOf(expectedContentSideB.split("\n", -1));
+
assertThat(diff.changeType).isEqualTo(ChangeType.ADDED);
- assertThat(diff.diffHeader).isNotNull();
- assertThat(diff.intralineStatus).isNull();
- assertThat(diff.webLinks).isNull();
- assertThat(diff.editWebLinks).isNull();
assertThat(diff.metaA).isNull();
assertThat(diff.metaB).isNotNull();
- assertThat(diff.metaB.commitId).isEqualTo(commit.name());
- String expectedContentType = "text/plain";
- if (COMMIT_MSG.equals(path)) {
- expectedContentType = FileContentUtil.TEXT_X_GERRIT_COMMIT_MESSAGE;
- } else if (MERGE_LIST.equals(path)) {
- expectedContentType = FileContentUtil.TEXT_X_GERRIT_MERGE_LIST;
- }
- assertThat(diff.metaB.contentType).isEqualTo(expectedContentType);
-
- assertThat(diff.metaB.lines).isEqualTo(expectedLines.size());
assertThat(diff.metaB.name).isEqualTo(path);
- assertThat(diff.metaB.webLinks).isNull();
+ assertThat(diff.metaB.lines).isEqualTo(expectedLines.size());
- assertThat(diff.content).hasSize(1);
DiffInfo.ContentEntry contentEntry = diff.content.get(0);
assertThat(contentEntry.b).containsExactlyElementsIn(expectedLines).inOrder();
assertThat(contentEntry.a).isNull();
@@ -1291,6 +1318,57 @@
assertThat(contentEntry.skip).isNull();
}
+ protected void assertDiffForDeletedFile(DiffInfo diff, String path, String expectedContentSideA)
+ throws Exception {
+ assertDiffHeaders(diff);
+
+ ImmutableList<String> expectedOriginalLines =
+ ImmutableList.copyOf(expectedContentSideA.split("\n", -1));
+
+ assertThat(diff.changeType).isEqualTo(ChangeType.DELETED);
+
+ assertThat(diff.metaA).isNotNull();
+ assertThat(diff.metaB).isNull();
+
+ assertThat(diff.metaA.name).isEqualTo(path);
+ assertThat(diff.metaA.lines).isEqualTo(expectedOriginalLines.size());
+
+ DiffInfo.ContentEntry contentEntry = diff.content.get(0);
+ assertThat(contentEntry.a).containsExactlyElementsIn(expectedOriginalLines).inOrder();
+ assertThat(contentEntry.b).isNull();
+ assertThat(contentEntry.ab).isNull();
+ assertThat(contentEntry.common).isNull();
+ assertThat(contentEntry.editA).isNull();
+ assertThat(contentEntry.editB).isNull();
+ assertThat(contentEntry.skip).isNull();
+ }
+
+ private void assertDiffForFile(DiffInfo diff, String commitName, String path) throws Exception {
+ assertDiffHeaders(diff);
+
+ assertThat(diff.metaB.commitId).isEqualTo(commitName);
+
+ String expectedContentType = "text/plain";
+ if (COMMIT_MSG.equals(path)) {
+ expectedContentType = FileContentUtil.TEXT_X_GERRIT_COMMIT_MESSAGE;
+ } else if (MERGE_LIST.equals(path)) {
+ expectedContentType = FileContentUtil.TEXT_X_GERRIT_MERGE_LIST;
+ }
+
+ assertThat(diff.metaB.contentType).isEqualTo(expectedContentType);
+
+ assertThat(diff.metaB.name).isEqualTo(path);
+ assertThat(diff.metaB.webLinks).isNull();
+ }
+
+ private void assertDiffHeaders(DiffInfo diff) throws Exception {
+ assertThat(diff.binary).isNull();
+ assertThat(diff.diffHeader).isNotNull();
+ assertThat(diff.intralineStatus).isNull();
+ assertThat(diff.webLinks).isNull();
+ assertThat(diff.editWebLinks).isNull();
+ }
+
protected void assertPermitted(ChangeInfo info, String label, Integer... expected) {
assertThat(info.permittedLabels).isNotNull();
Collection<String> strs = info.permittedLabels.get(label);
diff --git a/java/com/google/gerrit/acceptance/PushOneCommit.java b/java/com/google/gerrit/acceptance/PushOneCommit.java
index 8a53184..36f4ba1 100644
--- a/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -276,6 +276,11 @@
return this;
}
+ public PushOneCommit setTopLevelTreeId(ObjectId treeId) throws Exception {
+ commitBuilder.setTopLevelTree(treeId);
+ return this;
+ }
+
public PushOneCommit setParent(RevCommit parent) throws Exception {
commitBuilder.noParents();
commitBuilder.parent(parent);
diff --git a/java/com/google/gerrit/extensions/api/changes/ApplyPatchInput.java b/java/com/google/gerrit/extensions/api/changes/ApplyPatchInput.java
new file mode 100644
index 0000000..493329c
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/ApplyPatchInput.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2022 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;
+
+/** Information about a patch to apply. */
+public class ApplyPatchInput {
+ /**
+ * Required. The patch to be applied.
+ *
+ * <p>Must be compatible with `git diff` output. For example, Gerrit API `Get Patch` output.
+ */
+ public String patch;
+}
diff --git a/java/com/google/gerrit/extensions/api/changes/ApplyPatchPatchSetInput.java b/java/com/google/gerrit/extensions/api/changes/ApplyPatchPatchSetInput.java
new file mode 100644
index 0000000..872ea42
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/ApplyPatchPatchSetInput.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2022 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;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.api.accounts.AccountInput;
+
+/** Information for creating a new patch set from a given patch. */
+public class ApplyPatchPatchSetInput {
+
+ /** The patch to be applied. */
+ public ApplyPatchInput patch;
+
+ /**
+ * The commit message for the new patch set. If not specified, a predefined message will be used.
+ */
+ @Nullable public String commitMessage;
+
+ /**
+ * 40-hex digit SHA-1 of the commit which will be the parent commit of the newly created patch
+ * set. If set, it must be a merged commit or a change revision on the destination branch.
+ * Otherwise, the target change's branch tip will be used.
+ */
+ @Nullable public String base;
+
+ /**
+ * The author of the new patch set. Must include both {@link AccountInput#name} and {@link
+ * AccountInput#email} fields.
+ */
+ @Nullable public AccountInput author;
+}
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 018a6cf..cce28e9 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -152,6 +152,8 @@
/** Create a merge patch set for the change. */
ChangeInfo createMergePatchSet(MergePatchSetInput in) throws RestApiException;
+ ChangeInfo applyPatch(ApplyPatchPatchSetInput in) throws RestApiException;
+
default List<ChangeInfo> submittedTogether() throws RestApiException {
SubmittedTogetherInfo info =
submittedTogether(
@@ -824,6 +826,11 @@
}
@Override
+ public ChangeInfo applyPatch(ApplyPatchPatchSetInput in) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public PureRevertInfo pureRevert() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/common/ChangeInput.java b/java/com/google/gerrit/extensions/common/ChangeInput.java
index ea12ef1..55a5883 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInput.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInput.java
@@ -15,6 +15,7 @@
package com.google.gerrit.extensions.common;
import com.google.gerrit.extensions.api.accounts.AccountInput;
+import com.google.gerrit.extensions.api.changes.ApplyPatchInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.NotifyInfo;
import com.google.gerrit.extensions.api.changes.RecipientType;
@@ -35,6 +36,7 @@
public Boolean newBranch;
public Map<String, String> validationOptions;
public MergeInput merge;
+ public ApplyPatchInput patch;
public AccountInput author;
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 18919ed..9ccd00b 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -89,6 +89,7 @@
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
import com.google.gerrit.server.query.change.DistinctVotersPredicate;
+import com.google.gerrit.server.query.change.HasSubmoduleUpdatePredicate;
import com.google.gerrit.server.restapi.group.GroupModule;
import com.google.gerrit.server.rules.DefaultSubmitRule.DefaultSubmitRuleModule;
import com.google.gerrit.server.rules.IgnoreSelfApprovalRule.IgnoreSelfApprovalRuleModule;
@@ -197,6 +198,7 @@
factory(ChangeData.AssistedFactory.class);
factory(ChangeIsVisibleToPredicate.Factory.class);
factory(DistinctVotersPredicate.Factory.class);
+ factory(HasSubmoduleUpdatePredicate.Factory.class);
factory(ProjectState.Factory.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeOperatorFactory.class);
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index ac63135..e0569f4 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ListMultimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.AbandonInput;
+import com.google.gerrit.extensions.api.changes.ApplyPatchPatchSetInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput;
import com.google.gerrit.extensions.api.changes.AttentionSetApi;
import com.google.gerrit.extensions.api.changes.AttentionSetInput;
@@ -69,6 +70,7 @@
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.restapi.change.Abandon;
import com.google.gerrit.server.restapi.change.AddToAttentionSet;
+import com.google.gerrit.server.restapi.change.ApplyPatch;
import com.google.gerrit.server.restapi.change.AttentionSet;
import com.google.gerrit.server.restapi.change.ChangeIncludedIn;
import com.google.gerrit.server.restapi.change.ChangeMessages;
@@ -139,6 +141,7 @@
private final RevertSubmission revertSubmission;
private final Restore restore;
private final CreateMergePatchSet updateByMerge;
+ private final ApplyPatch applyPatch;
private final Provider<SubmittedTogether> submittedTogether;
private final Rebase.CurrentRevision rebase;
private final DeleteChange deleteChange;
@@ -191,6 +194,7 @@
RevertSubmission revertSubmission,
Restore restore,
CreateMergePatchSet updateByMerge,
+ ApplyPatch applyPatch,
Provider<SubmittedTogether> submittedTogether,
Rebase.CurrentRevision rebase,
DeleteChange deleteChange,
@@ -241,6 +245,7 @@
this.abandon = abandon;
this.restore = restore;
this.updateByMerge = updateByMerge;
+ this.applyPatch = applyPatch;
this.submittedTogether = submittedTogether;
this.rebase = rebase;
this.deleteChange = deleteChange;
@@ -389,6 +394,15 @@
}
@Override
+ public ChangeInfo applyPatch(ApplyPatchPatchSetInput in) throws RestApiException {
+ try {
+ return applyPatch.apply(change, in).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot apply patch", e);
+ }
+ }
+
+ @Override
public SubmittedTogetherInfo submittedTogether(
EnumSet<ListChangesOption> listOptions, EnumSet<SubmittedTogetherOption> submitOptions)
throws RestApiException {
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index e5b063b..f442500 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -201,6 +201,7 @@
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
import com.google.gerrit.server.query.change.DistinctVotersPredicate;
+import com.google.gerrit.server.query.change.HasSubmoduleUpdatePredicate;
import com.google.gerrit.server.quota.QuotaEnforcer;
import com.google.gerrit.server.restapi.change.OnPostReview;
import com.google.gerrit.server.restapi.change.SuggestReviewers;
@@ -299,6 +300,7 @@
factory(ChangeJson.AssistedFactory.class);
factory(ChangeIsVisibleToPredicate.Factory.class);
factory(DistinctVotersPredicate.Factory.class);
+ factory(HasSubmoduleUpdatePredicate.Factory.class);
factory(DeadlineChecker.Factory.class);
factory(EmailNewPatchSet.Factory.class);
factory(MultiProgressMonitor.Factory.class);
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index f90daff..9ba90da 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -26,10 +26,13 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.CommitInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
@@ -44,6 +47,8 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -58,15 +63,19 @@
import java.time.Instant;
import java.util.ArrayList;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.InvalidObjectIdException;
+import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -190,6 +199,41 @@
}
/**
+ * Creates a commit with the specified tree ID.
+ *
+ * @param oi ObjectInserter for inserting the newly created commit.
+ * @param authorIdent of the new commit
+ * @param committerIdent of the new commit
+ * @param parentCommit of the new commit. Can be null.
+ * @param commitMessage for the new commit.
+ * @param treeId of the content for the new commit.
+ * @return the newly created commit.
+ * @throws IOException if fails to insert the commit.
+ */
+ public static ObjectId createCommitWithTree(
+ ObjectInserter oi,
+ PersonIdent authorIdent,
+ PersonIdent committerIdent,
+ @Nullable RevCommit parentCommit,
+ String commitMessage,
+ ObjectId treeId)
+ throws IOException {
+ logger.atFine().log("Creating commit with tree: %s", treeId.getName());
+ CommitBuilder commit = new CommitBuilder();
+ commit.setTreeId(treeId);
+ if (parentCommit != null) {
+ commit.setParentId(parentCommit);
+ }
+ commit.setAuthor(authorIdent);
+ commit.setCommitter(committerIdent);
+ commit.setMessage(commitMessage);
+
+ ObjectId id = oi.insert(commit);
+ oi.flush();
+ return id;
+ }
+
+ /**
* Creates a revert commit.
*
* @param message Commit message for the revert commit.
@@ -227,12 +271,6 @@
RevCommit parentToCommitToRevert = commitToRevert.getParent(0);
revWalk.parseHeaders(parentToCommitToRevert);
- CommitBuilder revertCommitBuilder = new CommitBuilder();
- revertCommitBuilder.addParentId(commitToRevert);
- revertCommitBuilder.setTreeId(parentToCommitToRevert.getTree());
- revertCommitBuilder.setAuthor(authorIdent);
- revertCommitBuilder.setCommitter(authorIdent);
-
Change changeToRevert = notes.getChange();
String subject = changeToRevert.getSubject();
if (subject.length() > 63) {
@@ -244,11 +282,11 @@
ChangeMessages.get().revertChangeDefaultMessage, subject, patch.commitId().name());
}
if (generatedChangeId != null) {
- revertCommitBuilder.setMessage(ChangeIdUtil.insertId(message, generatedChangeId, true));
+ message = ChangeIdUtil.insertId(message, generatedChangeId, true);
}
- ObjectId id = oi.insert(revertCommitBuilder);
- oi.flush();
- return id;
+
+ return createCommitWithTree(
+ oi, authorIdent, committerIdent, commitToRevert, message, parentToCommitToRevert.getTree());
}
private Change.Id createRevertChangeFromCommit(
@@ -358,4 +396,75 @@
return true;
}
}
+
+ /**
+ * Returns the parent commit for a new commit.
+ *
+ * <p>If {@code baseSha1} is provided, the method verifies it can be used as a base. If {@code
+ * baseSha1} is not provided the tip of the {@code destRef} is returned.
+ *
+ * @param project The name of the project.
+ * @param changeQuery Used for looking up the base commit.
+ * @param revWalk Used for parsing the base commit.
+ * @param destRef The destination branch.
+ * @param baseSha1 The hash of the base commit. Nullable.
+ * @return the base commit. Either the commit matching the provided hash, or the direct parent if
+ * a hash was not provided.
+ * @throws IOException if the branch reference cannot be parsed.
+ * @throws RestApiException if the base commit cannot be fetched.
+ */
+ public static RevCommit getBaseCommit(
+ String project,
+ InternalChangeQuery changeQuery,
+ RevWalk revWalk,
+ Ref destRef,
+ @Nullable String baseSha1)
+ throws IOException, RestApiException {
+ RevCommit destRefTip = revWalk.parseCommit(destRef.getObjectId());
+ // The tip commit of the destination ref is the default base for the newly created change.
+ if (Strings.isNullOrEmpty(baseSha1)) {
+ return destRefTip;
+ }
+
+ ObjectId baseObjectId;
+ try {
+ baseObjectId = ObjectId.fromString(baseSha1);
+ } catch (InvalidObjectIdException e) {
+ throw new BadRequestException(
+ String.format("Base %s doesn't represent a valid SHA-1", baseSha1), e);
+ }
+
+ RevCommit baseCommit;
+ try {
+ baseCommit = revWalk.parseCommit(baseObjectId);
+ } catch (MissingObjectException e) {
+ throw new UnprocessableEntityException(
+ String.format("Base %s doesn't exist", baseObjectId.name()), e);
+ }
+
+ changeQuery.enforceVisibility(true);
+ List<ChangeData> changeDatas = changeQuery.byBranchCommit(project, destRef.getName(), baseSha1);
+
+ if (changeDatas.isEmpty()) {
+ if (revWalk.isMergedInto(baseCommit, destRefTip)) {
+ // The base commit is a merged commit with no change associated.
+ return baseCommit;
+ }
+ throw new UnprocessableEntityException(
+ String.format("Commit %s does not exist on branch %s", baseSha1, destRef.getName()));
+ } else if (changeDatas.size() != 1) {
+ throw new ResourceConflictException("Multiple changes found for commit " + baseSha1);
+ }
+
+ Change change = changeDatas.get(0).change();
+ if (!change.isAbandoned()) {
+ // The base commit is a valid change revision.
+ return baseCommit;
+ }
+
+ throw new ResourceConflictException(
+ String.format(
+ "Change %s with commit %s is %s",
+ change.getChangeId(), baseSha1, ChangeUtil.status(change)));
+ }
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 61a41cc..893d9c2 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -439,6 +439,7 @@
private MessageSender messageSender;
private ReceiveCommitsResult.Builder result;
private ImmutableMap<String, String> loggingTags;
+ private ImmutableList<String> transitionalPluginOptions;
/** This object is for single use only. */
private boolean used;
@@ -590,6 +591,8 @@
useRefCache
? ReceivePackRefCache.withAdvertisedRefs(() -> allRefsWatcher.getAllRefs())
: ReceivePackRefCache.noCache(receivePack.getRepository().getRefDatabase());
+ this.transitionalPluginOptions =
+ ImmutableList.copyOf(config.getStringList("plugins", null, "transitionalPushOptions"));
}
void init() {
@@ -2132,6 +2135,9 @@
}
private boolean isPluginPushOption(String pushOptionName) {
+ if (transitionalPluginOptions.contains(pushOptionName)) {
+ return true;
+ }
return StreamSupport.stream(pluginPushOptions.entries().spliterator(), /* parallel= */ false)
.anyMatch(e -> pushOptionName.equals(e.getPluginName() + "~" + e.get().getName()));
}
diff --git a/java/com/google/gerrit/server/query/change/HasSubmoduleUpdatePredicate.java b/java/com/google/gerrit/server/query/change/HasSubmoduleUpdatePredicate.java
new file mode 100644
index 0000000..4a21261
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/HasSubmoduleUpdatePredicate.java
@@ -0,0 +1,106 @@
+// Copyright (C) 2022 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.query.change;
+
+import static com.google.gerrit.server.query.change.SubmitRequirementChangeQueryBuilder.SUBMODULE_UPDATE_HAS_ARG;
+
+import com.google.gerrit.entities.Patch.FileMode;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.gerrit.server.patch.DiffOperations;
+import com.google.gerrit.server.patch.DiffOptions;
+import com.google.gerrit.server.patch.filediff.FileDiffOutput;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Optional;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/**
+ * Submit requirement predicate that returns true if the diff of the latest patchset against the
+ * parent number identified by {@link #base} has a submodule modified file, that is, a .gitmodules
+ * or a git link file.
+ */
+public class HasSubmoduleUpdatePredicate extends SubmitRequirementPredicate {
+ private static final String GIT_MODULES_FILE = ".gitmodules";
+
+ private final DiffOperations diffOperations;
+ private final GitRepositoryManager repoManager;
+ private final int base;
+
+ public interface Factory {
+ HasSubmoduleUpdatePredicate create(int base);
+ }
+
+ @Inject
+ public HasSubmoduleUpdatePredicate(
+ DiffOperations diffOperations, GitRepositoryManager repoManager, @Assisted int base) {
+ super("has", SUBMODULE_UPDATE_HAS_ARG);
+ this.diffOperations = diffOperations;
+ this.repoManager = repoManager;
+ this.base = base;
+ }
+
+ @Override
+ public boolean match(ChangeData cd) {
+ try {
+ try (Repository repo = repoManager.openRepository(cd.project());
+ RevWalk rw = new RevWalk(repo)) {
+ RevCommit revCommit = rw.parseCommit(cd.currentPatchSet().commitId());
+ if (base > revCommit.getParentCount()) {
+ return false;
+ }
+ }
+ Map<String, FileDiffOutput> diffList =
+ diffOperations.listModifiedFilesAgainstParent(
+ cd.project(), cd.currentPatchSet().commitId(), base, DiffOptions.DEFAULTS);
+ return diffList.values().stream().anyMatch(HasSubmoduleUpdatePredicate::isGitLink);
+ } catch (DiffNotAvailableException e) {
+ throw new StorageException(
+ String.format(
+ "Failed to evaluate the diff for commit %s against parent number %d",
+ cd.currentPatchSet().commitId(), base),
+ e);
+ } catch (IOException e) {
+ throw new StorageException(
+ String.format("Failed to open repo for project %s", cd.project()), e);
+ }
+ }
+
+ /**
+ * Return true if the modified file is a {@link #GIT_MODULES_FILE} or a git link regardless of if
+ * the modification type is add, remove or modify.
+ */
+ private static boolean isGitLink(FileDiffOutput fileDiffOutput) {
+ Optional<String> oldPath = fileDiffOutput.oldPath();
+ Optional<String> newPath = fileDiffOutput.newPath();
+ Optional<FileMode> oldMode = fileDiffOutput.oldMode();
+ Optional<FileMode> newMode = fileDiffOutput.newMode();
+
+ return (oldPath.isPresent() && oldPath.get().equals(GIT_MODULES_FILE))
+ || (newPath.isPresent() && newPath.get().equals(GIT_MODULES_FILE))
+ || (oldMode.isPresent() && oldMode.get().equals(FileMode.GITLINK))
+ || (newMode.isPresent() && newMode.get().equals(FileMode.GITLINK));
+ }
+
+ @Override
+ public int getCost() {
+ return 1;
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
index 1e7a408..3f4c158 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.common.base.Splitter;
import com.google.gerrit.index.SchemaFieldDefs.SchemaField;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryBuilder;
@@ -21,6 +22,8 @@
import com.google.gerrit.server.query.FileEditsPredicate;
import com.google.gerrit.server.query.FileEditsPredicate.FileEditsArgs;
import com.google.inject.Inject;
+import java.util.List;
+import java.util.Locale;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
@@ -37,6 +40,7 @@
new QueryBuilder.Definition<>(SubmitRequirementChangeQueryBuilder.class);
private final DistinctVotersPredicate.Factory distinctVotersPredicateFactory;
+ private final HasSubmoduleUpdatePredicate.Factory hasSubmoduleUpdateFactory;
/**
* Regular expression for the {@link #file(String)} operator. Field value is of the form:
@@ -48,16 +52,21 @@
private static final Pattern FILE_EDITS_PATTERN =
Pattern.compile("'((?:(?:\\\\')|(?:[^']))*)',withDiffContaining='((?:(?:\\\\')|(?:[^']))*)'");
+ public static final String SUBMODULE_UPDATE_HAS_ARG = "submodule-update";
+ private static final Splitter SUBMODULE_UPDATE_SPLITTER = Splitter.on(",");
+
private final FileEditsPredicate.Factory fileEditsPredicateFactory;
@Inject
SubmitRequirementChangeQueryBuilder(
Arguments args,
DistinctVotersPredicate.Factory distinctVotersPredicateFactory,
- FileEditsPredicate.Factory fileEditsPredicateFactory) {
+ FileEditsPredicate.Factory fileEditsPredicateFactory,
+ HasSubmoduleUpdatePredicate.Factory hasSubmoduleUpdateFactory) {
super(def, args);
this.distinctVotersPredicateFactory = distinctVotersPredicateFactory;
this.fileEditsPredicateFactory = fileEditsPredicateFactory;
+ this.hasSubmoduleUpdateFactory = hasSubmoduleUpdateFactory;
}
@Override
@@ -79,6 +88,37 @@
return super.is(value);
}
+ @Override
+ public Predicate<ChangeData> has(String value) throws QueryParseException {
+ if (value.toLowerCase(Locale.US).startsWith(SUBMODULE_UPDATE_HAS_ARG)) {
+ List<String> args = SUBMODULE_UPDATE_SPLITTER.splitToList(value);
+ if (args.size() > 2) {
+ throw error(
+ String.format(
+ "wrong number of arguments for the has:%s operator", SUBMODULE_UPDATE_HAS_ARG));
+ } else if (args.size() == 2) {
+ List<String> baseValue = Splitter.on("=").splitToList(args.get(1));
+ if (baseValue.size() != 2) {
+ throw error("unexpected base value format");
+ }
+ if (!baseValue.get(0).toLowerCase(Locale.US).equals("base")) {
+ throw error("unexpected base value format");
+ }
+ try {
+ int base = Integer.parseInt(baseValue.get(1));
+ return hasSubmoduleUpdateFactory.create(base);
+ } catch (NumberFormatException e) {
+ throw error(
+ String.format(
+ "failed to parse the parent number %s: %s", baseValue.get(1), e.getMessage()));
+ }
+ } else {
+ return hasSubmoduleUpdateFactory.create(0);
+ }
+ }
+ return super.has(value);
+ }
+
@Operator
public Predicate<ChangeData> authoremail(String who) throws QueryParseException {
return new RegexAuthorEmailPredicate(who);
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
new file mode 100644
index 0000000..044fa0d
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
@@ -0,0 +1,194 @@
+// Copyright (C) 2022 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.restapi.change;
+
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.extensions.api.changes.ApplyPatchPatchSetInput;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.PreconditionFailedException;
+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.ChangeUtil;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.git.CommitUtil;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ContributorAgreementsChecker;
+import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.util.CommitMessageUtil;
+import com.google.gerrit.server.util.time.TimeUtil;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+@Singleton
+public class ApplyPatch implements RestModifyView<ChangeResource, ApplyPatchPatchSetInput> {
+ private final ChangeJson.Factory jsonFactory;
+ private final ContributorAgreementsChecker contributorAgreements;
+ private final Provider<IdentifiedUser> user;
+ private final GitRepositoryManager gitManager;
+ private final BatchUpdate.Factory batchUpdateFactory;
+ private final PatchSetInserter.Factory patchSetInserterFactory;
+ private final Provider<InternalChangeQuery> queryProvider;
+ private final ZoneId serverZoneId;
+
+ @Inject
+ ApplyPatch(
+ ChangeJson.Factory jsonFactory,
+ ContributorAgreementsChecker contributorAgreements,
+ Provider<IdentifiedUser> user,
+ GitRepositoryManager gitManager,
+ BatchUpdate.Factory batchUpdateFactory,
+ PatchSetInserter.Factory patchSetInserterFactory,
+ Provider<InternalChangeQuery> queryProvider,
+ @GerritPersonIdent PersonIdent myIdent) {
+ this.jsonFactory = jsonFactory;
+ this.contributorAgreements = contributorAgreements;
+ this.user = user;
+ this.gitManager = gitManager;
+ this.batchUpdateFactory = batchUpdateFactory;
+ this.patchSetInserterFactory = patchSetInserterFactory;
+ this.queryProvider = queryProvider;
+ this.serverZoneId = myIdent.getZoneId();
+ }
+
+ @Override
+ public Response<ChangeInfo> apply(ChangeResource rsrc, ApplyPatchPatchSetInput input)
+ throws IOException, UpdateException, RestApiException, PermissionBackendException,
+ ConfigInvalidException, NoSuchProjectException, InvalidChangeOperationException {
+ NameKey project = rsrc.getProject();
+ contributorAgreements.check(project, rsrc.getUser());
+ BranchNameKey destBranch = rsrc.getChange().getDest();
+
+ try (Repository repo = gitManager.openRepository(project);
+ // This inserter and revwalk *must* be passed to any BatchUpdates
+ // created later on, to ensure the applied commit is flushed
+ // before patch sets are updated.
+ ObjectInserter oi = repo.newObjectInserter();
+ ObjectReader reader = oi.newReader();
+ CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) {
+ Ref destRef = repo.getRefDatabase().exactRef(destBranch.branch());
+ if (destRef == null) {
+ throw new ResourceNotFoundException(
+ String.format("Branch %s does not exist.", destBranch.branch()));
+ }
+ ChangeData destChange = rsrc.getChangeData();
+ if (destChange == null) {
+ throw new PreconditionFailedException(
+ "patch:apply cannot be called without a destination change.");
+ }
+
+ if (destChange.change().isClosed()) {
+ throw new PreconditionFailedException(
+ String.format(
+ "patch:apply with Change-Id %s could not update the existing change %d "
+ + "in destination branch %s of project %s, because the change was closed (%s)",
+ destChange.getId(),
+ destChange.getId().get(),
+ destBranch.branch(),
+ destBranch.project(),
+ destChange.change().getStatus().name()));
+ }
+
+ RevCommit baseCommit =
+ CommitUtil.getBaseCommit(
+ project.get(), queryProvider.get(), revWalk, destRef, input.base);
+ ObjectId treeId = ApplyPatchUtil.applyPatch(repo, oi, input.patch, baseCommit);
+
+ Instant now = TimeUtil.now();
+ PersonIdent committerIdent = user.get().newCommitterIdent(now, serverZoneId);
+ PersonIdent authorIdent =
+ input.author == null
+ ? committerIdent
+ : new PersonIdent(input.author.name, input.author.email, now, serverZoneId);
+ String commitMessage =
+ CommitMessageUtil.checkAndSanitizeCommitMessage(
+ input.commitMessage != null
+ ? input.commitMessage
+ : "The following patch was applied:\n>\t"
+ + input.patch.patch.replaceAll("\n", "\n>\t"));
+
+ ObjectId appliedCommit =
+ CommitUtil.createCommitWithTree(
+ oi, authorIdent, committerIdent, baseCommit, commitMessage, treeId);
+ CodeReviewCommit commit = revWalk.parseCommit(appliedCommit);
+ oi.flush();
+
+ Change resultChange;
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.now())) {
+ bu.setRepository(repo, revWalk, oi);
+ resultChange =
+ insertPatchSet(bu, repo, patchSetInserterFactory, destChange.notes(), commit);
+ } catch (NoSuchChangeException | RepositoryNotFoundException e) {
+ throw new ResourceConflictException(e.getMessage());
+ }
+ ChangeJson json = jsonFactory.create(ListChangesOption.CURRENT_REVISION);
+ ChangeInfo changeInfo = json.format(resultChange);
+ return Response.ok(changeInfo);
+ }
+ }
+
+ private static Change insertPatchSet(
+ BatchUpdate bu,
+ Repository git,
+ PatchSetInserter.Factory patchSetInserterFactory,
+ ChangeNotes destNotes,
+ CodeReviewCommit commit)
+ throws IOException, UpdateException, RestApiException {
+ Change destChange = destNotes.getChange();
+ PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId());
+ PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, commit);
+ inserter.setMessage(buildMessageForPatchSet(psId));
+ bu.addOp(destChange.getId(), inserter);
+ bu.execute();
+ return inserter.getChange();
+ }
+
+ private static String buildMessageForPatchSet(PatchSet.Id psId) {
+ return new StringBuilder(String.format("Uploaded patch set %s.", psId.get())).toString();
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
new file mode 100644
index 0000000..d4f549a
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
@@ -0,0 +1,68 @@
+// Copyright (C) 2022 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.restapi.change;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.gerrit.extensions.api.changes.ApplyPatchInput;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import org.eclipse.jgit.api.errors.PatchApplyException;
+import org.eclipse.jgit.api.errors.PatchFormatException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.patch.PatchApplier;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevTree;
+
+/** Utility for applying a patch. */
+public final class ApplyPatchUtil {
+
+ /**
+ * Applies the given patch on top of the merge tip, using the given object inserter.
+ *
+ * @param repo to apply the patch in
+ * @param oi to operate with
+ * @param input the patch for applying
+ * @param mergeTip the tip to apply the patch on
+ * @return the tree ID with the applied patch
+ * @throws IOException if unable to create the jgit PatchApplier object
+ * @throws RestApiException for any other failure
+ */
+ public static ObjectId applyPatch(
+ Repository repo, ObjectInserter oi, ApplyPatchInput input, RevCommit mergeTip)
+ throws IOException, RestApiException {
+ checkNotNull(mergeTip);
+ RevTree tip = mergeTip.getTree();
+ InputStream patchStream =
+ new ByteArrayInputStream(input.patch.getBytes(StandardCharsets.UTF_8));
+ try {
+ PatchApplier applier = new PatchApplier(repo, tip, oi);
+ PatchApplier.Result applyResult = applier.applyPatch(patchStream);
+ return applyResult.getTreeId();
+ } catch (PatchFormatException e) {
+ throw new BadRequestException("Invalid patch format: " + input.patch, e);
+ } catch (PatchApplyException e) {
+ throw RestApiException.wrap("Cannot apply patch: " + input.patch, e);
+ }
+ }
+
+ private ApplyPatchUtil() {}
+}
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java b/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java
index 718759a..87d8cd0 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java
@@ -122,6 +122,7 @@
post(CHANGE_KIND, "ready").to(SetReadyForReview.class);
put(CHANGE_KIND, "message").to(PutMessage.class);
post(CHANGE_KIND, "check.submit_requirement").to(CheckSubmitRequirement.class);
+ post(CHANGE_KIND, "patch:apply").to(ApplyPatch.class);
get(CHANGE_KIND, "suggest_reviewers").to(SuggestChangeReviewers.class);
child(CHANGE_KIND, "reviewers").to(Reviewers.class);
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index d0113e5..4619b86 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -31,9 +31,7 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -46,6 +44,7 @@
import com.google.gerrit.server.change.SetCherryPickOp;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.git.CommitUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.MergeUtil;
@@ -76,8 +75,6 @@
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.InvalidObjectIdException;
-import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
@@ -85,7 +82,6 @@
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.ChangeIdUtil;
@Singleton
@@ -267,7 +263,9 @@
String.format("Branch %s does not exist.", dest.branch()));
}
- RevCommit baseCommit = getBaseCommit(destRef, project.get(), revWalk, input.base);
+ RevCommit baseCommit =
+ CommitUtil.getBaseCommit(
+ project.get(), queryProvider.get(), revWalk, destRef, input.base);
CodeReviewCommit commitToCherryPick = revWalk.parseCommit(sourceCommit);
@@ -384,57 +382,6 @@
}
}
- private RevCommit getBaseCommit(Ref destRef, String project, RevWalk revWalk, String base)
- throws RestApiException, IOException {
- RevCommit destRefTip = revWalk.parseCommit(destRef.getObjectId());
- // The tip commit of the destination ref is the default base for the newly created change.
- if (Strings.isNullOrEmpty(base)) {
- return destRefTip;
- }
-
- ObjectId baseObjectId;
- try {
- baseObjectId = ObjectId.fromString(base);
- } catch (InvalidObjectIdException e) {
- throw new BadRequestException(
- String.format("Base %s doesn't represent a valid SHA-1", base), e);
- }
-
- RevCommit baseCommit;
- try {
- baseCommit = revWalk.parseCommit(baseObjectId);
- } catch (MissingObjectException e) {
- throw new UnprocessableEntityException(
- String.format("Base %s doesn't exist", baseObjectId.name()), e);
- }
-
- InternalChangeQuery changeQuery = queryProvider.get();
- changeQuery.enforceVisibility(true);
- List<ChangeData> changeDatas = changeQuery.byBranchCommit(project, destRef.getName(), base);
-
- if (changeDatas.isEmpty()) {
- if (revWalk.isMergedInto(baseCommit, destRefTip)) {
- // The base commit is a merged commit with no change associated.
- return baseCommit;
- }
- throw new UnprocessableEntityException(
- String.format("Commit %s does not exist on branch %s", base, destRef.getName()));
- } else if (changeDatas.size() != 1) {
- throw new ResourceConflictException("Multiple changes found for commit " + base);
- }
-
- Change change = changeDatas.get(0).change();
- if (!change.isAbandoned()) {
- // The base commit is a valid change revision.
- return baseCommit;
- }
-
- throw new ResourceConflictException(
- String.format(
- "Change %s with commit %s is %s",
- change.getChangeId(), base, ChangeUtil.status(change)));
- }
-
private Change.Id insertPatchSet(
BatchUpdate bu,
Repository git,
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index 760d99d..2cb427a 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -61,6 +61,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.git.CommitUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.MergeUtilFactory;
@@ -94,7 +95,6 @@
import org.eclipse.jgit.errors.InvalidObjectIdException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.NoMergeBaseException;
-import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@@ -293,6 +293,10 @@
}
}
+ if (input.merge != null && input.patch != null) {
+ throw new BadRequestException("Only one of `merge` and `patch` arguments can be set.");
+ }
+
if (input.author != null
&& (Strings.isNullOrEmpty(input.author.email)
|| Strings.isNullOrEmpty(input.author.name))) {
@@ -373,9 +377,19 @@
"merge commit has conflicts in the following files: %s",
c.getFilesWithGitConflicts());
}
+ } else if (input.patch != null) {
+ // create a commit with the given patch.
+ if (mergeTip == null) {
+ throw new BadRequestException("Cannot apply patch on top of an empty tree.");
+ }
+ ObjectId treeId = ApplyPatchUtil.applyPatch(git, oi, input.patch, mergeTip);
+ c =
+ rw.parseCommit(
+ CommitUtil.createCommitWithTree(
+ oi, author, committer, mergeTip, commitMessage, treeId));
} else {
- // create an empty commit
- c = newCommit(oi, rw, author, committer, mergeTip, commitMessage);
+ // create an empty commit.
+ c = createEmptyCommit(oi, rw, author, committer, mergeTip, commitMessage);
}
// Flush inserter so that commit becomes visible to validators
oi.flush();
@@ -526,7 +540,7 @@
return commitMessage;
}
- private static CodeReviewCommit newCommit(
+ private static CodeReviewCommit createEmptyCommit(
ObjectInserter oi,
CodeReviewRevWalk rw,
PersonIdent authorIdent,
@@ -535,17 +549,14 @@
String commitMessage)
throws IOException {
logger.atFine().log("Creating empty commit");
- CommitBuilder commit = new CommitBuilder();
- if (mergeTip == null) {
- commit.setTreeId(emptyTreeId(oi));
- } else {
- commit.setTreeId(mergeTip.getTree().getId());
- commit.setParentId(mergeTip);
- }
- commit.setAuthor(authorIdent);
- commit.setCommitter(committerIdent);
- commit.setMessage(commitMessage);
- return rw.parseCommit(insert(oi, commit));
+ ObjectId treeID = mergeTip == null ? emptyTreeId(oi) : mergeTip.getTree().getId();
+ return rw.parseCommit(
+ CommitUtil.createCommitWithTree(
+ oi, authorIdent, committerIdent, mergeTip, commitMessage, treeID));
+ }
+
+ private static ObjectId emptyTreeId(ObjectInserter inserter) throws IOException {
+ return inserter.insert(new TreeFormatter());
}
private CodeReviewCommit newMergeCommit(
@@ -615,14 +626,4 @@
return stringBuilder.toString();
}
-
- private static ObjectId insert(ObjectInserter inserter, CommitBuilder commit) throws IOException {
- ObjectId id = inserter.insert(commit);
- inserter.flush();
- return id;
- }
-
- private static ObjectId emptyTreeId(ObjectInserter inserter) throws IOException {
- return inserter.insert(new TreeFormatter());
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
new file mode 100644
index 0000000..bb00fd5
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
@@ -0,0 +1,409 @@
+// Copyright (C) 2022 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.acceptance.api.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.permissionKey;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.eclipse.jgit.lib.Constants.HEAD;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.extensions.api.accounts.AccountInput;
+import com.google.gerrit.extensions.api.changes.ApplyPatchInput;
+import com.google.gerrit.extensions.api.changes.ApplyPatchPatchSetInput;
+import com.google.gerrit.extensions.api.changes.RevisionApi;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.extensions.common.DiffInfo;
+import com.google.gerrit.extensions.common.GitPerson;
+import com.google.gerrit.extensions.common.testing.GitPersonSubject;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.inject.Inject;
+import java.io.IOException;
+import org.eclipse.jgit.api.errors.PatchApplyException;
+import org.eclipse.jgit.util.Base64;
+import org.junit.Test;
+
+public class ApplyPatchIT extends AbstractDaemonTest {
+
+ private static final String COMMIT_MESSAGE = "Applying patch";
+ private static final String DESTINATION_BRANCH = "destBranch";
+
+ private static final String ADDED_FILE_NAME = "a_new_file.txt";
+ private static final String ADDED_FILE_CONTENT = "First added line\nSecond added line\n";
+ private static final String ADDED_FILE_DIFF =
+ "diff --git a/a_new_file.txt b/a_new_file.txt\n"
+ + "new file mode 100644\n"
+ + "--- /dev/null\n"
+ + "+++ b/a_new_file.txt\n"
+ + "@@ -0,0 +1,2 @@\n"
+ + "+First added line\n"
+ + "+Second added line\n";
+
+ @Inject private ProjectOperations projectOperations;
+ @Inject private RequestScopeOperations requestScopeOperations;
+
+ @Test
+ public void applyAddedFilePatch_success() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+
+ ChangeInfo result = applyPatch(in);
+
+ DiffInfo diff = fetchDiffForFile(result, ADDED_FILE_NAME);
+ assertDiffForNewFile(diff, result.currentRevision, ADDED_FILE_NAME, ADDED_FILE_CONTENT);
+ }
+
+ private static final String MODIFIED_FILE_NAME = "modified_file.txt";
+ private static final String MODIFIED_FILE_ORIGINAL_CONTENT =
+ "First original line\nSecond original line";
+ private static final String MODIFIED_FILE_NEW_CONTENT = "Modified line\n";
+ private static final String MODIFIED_FILE_DIFF =
+ "diff --git a/modified_file.txt b/modified_file.txt\n"
+ + "new file mode 100644\n"
+ + "--- a/modified_file.txt\n"
+ + "+++ b/modified_file.txt\n"
+ + "@@ -1,2 +1 @@\n"
+ + "-First original line\n"
+ + "-Second original line\n"
+ + "+Modified line\n";
+
+ @Test
+ public void applyModifiedFilePatch_success() throws Exception {
+ initBaseWithFile(MODIFIED_FILE_NAME, MODIFIED_FILE_ORIGINAL_CONTENT);
+ ApplyPatchPatchSetInput in = buildInput(MODIFIED_FILE_DIFF);
+
+ ChangeInfo result = applyPatch(in);
+
+ DiffInfo diff = fetchDiffForFile(result, MODIFIED_FILE_NAME);
+ assertDiffForFullyModifiedFile(
+ diff,
+ result.currentRevision,
+ MODIFIED_FILE_NAME,
+ MODIFIED_FILE_ORIGINAL_CONTENT,
+ MODIFIED_FILE_NEW_CONTENT);
+ }
+
+ @Test
+ public void applyDeletedFilePatch_success() throws Exception {
+ final String deletedFileName = "deleted_file.txt";
+ final String deletedFileOriginalContent = "content to be deleted.\n";
+ final String deletedFileDiff =
+ "diff --git a/deleted_file.txt b/deleted_file.txt\n"
+ + "--- a/deleted_file.txt\n"
+ + "+++ /dev/null\n";
+ initBaseWithFile(deletedFileName, deletedFileOriginalContent);
+ ApplyPatchPatchSetInput in = buildInput(deletedFileDiff);
+
+ ChangeInfo result = applyPatch(in);
+
+ DiffInfo diff = fetchDiffForFile(result, deletedFileName);
+ assertDiffForDeletedFile(diff, deletedFileName, deletedFileOriginalContent);
+ }
+
+ @Test
+ public void applyRenamedFilePatch_success() throws Exception {
+ final String renamedFileOriginalName = "renamed_file_origin.txt";
+ final String renamedFileNewName = "renamed_file_new.txt";
+ final String renamedFileDiff =
+ "diff --git a/renamed_file_origin.txt b/renamed_file_new.txt\n"
+ + "rename from renamed_file_origin.txt\n"
+ + "rename to renamed_file_new.txt\n"
+ + "--- a/renamed_file_origin.txt\n"
+ + "+++ b/renamed_file_new.txt\n"
+ + "@@ -1,2 +1 @@\n"
+ + "-First original line\n"
+ + "-Second original line\n"
+ + "+Modified line\n";
+ initBaseWithFile(renamedFileOriginalName, MODIFIED_FILE_ORIGINAL_CONTENT);
+ ApplyPatchPatchSetInput in = buildInput(renamedFileDiff);
+
+ ChangeInfo result = applyPatch(in);
+
+ DiffInfo originalFileDiff = fetchDiffForFile(result, renamedFileOriginalName);
+ assertDiffForDeletedFile(
+ originalFileDiff, renamedFileOriginalName, MODIFIED_FILE_ORIGINAL_CONTENT);
+ DiffInfo newFileDiff = fetchDiffForFile(result, renamedFileNewName);
+ assertDiffForNewFile(
+ newFileDiff, result.currentRevision, renamedFileNewName, MODIFIED_FILE_NEW_CONTENT);
+ }
+
+ @Test
+ public void applyGerritBasedPatchWithSingleFile_success() throws Exception {
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
+ PushOneCommit.Result baseCommit = createChange("Add file", ADDED_FILE_NAME, ADDED_FILE_CONTENT);
+ baseCommit.assertOkStatus();
+ BinaryResult originalPatch = gApi.changes().id(baseCommit.getChangeId()).current().patch();
+ ApplyPatchPatchSetInput in = buildInput(originalPatch.asString());
+ createBranchWithRevision(BranchNameKey.create(project, DESTINATION_BRANCH), head);
+
+ ChangeInfo result = applyPatch(in);
+
+ BinaryResult resultPatch = gApi.changes().id(result.id).current().patch();
+ assertThat(removeHeader(resultPatch)).isEqualTo(removeHeader(originalPatch));
+ }
+
+ @Test
+ public void applyGerritBasedPatchWithMultipleFiles_success() throws Exception {
+ PushOneCommit.Result commonBaseCommit =
+ createChange("File for modification", MODIFIED_FILE_NAME, MODIFIED_FILE_ORIGINAL_CONTENT);
+ commonBaseCommit.assertOkStatus();
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
+ PushOneCommit.Result commitToPatch =
+ createChange("Add file", ADDED_FILE_NAME, ADDED_FILE_CONTENT);
+ amendChange(
+ commitToPatch.getChangeId(), "Modify file", MODIFIED_FILE_NAME, MODIFIED_FILE_NEW_CONTENT);
+ commitToPatch.assertOkStatus();
+ BinaryResult originalPatch = gApi.changes().id(commitToPatch.getChangeId()).current().patch();
+ ApplyPatchPatchSetInput in = buildInput(originalPatch.asString());
+ createBranchWithRevision(BranchNameKey.create(project, DESTINATION_BRANCH), head);
+
+ ChangeInfo result = applyPatch(in);
+
+ BinaryResult resultPatch = gApi.changes().id(result.id).current().patch();
+ assertThat(removeHeader(resultPatch)).isEqualTo(removeHeader(originalPatch));
+ }
+
+ @Test
+ public void applyGerritBasedPatchUsingRest_success() throws Exception {
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
+ PushOneCommit.Result baseCommit = createChange("Add file", ADDED_FILE_NAME, ADDED_FILE_CONTENT);
+ baseCommit.assertOkStatus();
+ createBranchWithRevision(BranchNameKey.create(project, DESTINATION_BRANCH), head);
+ RestResponse patchResp =
+ userRestSession.get("/changes/" + baseCommit.getChangeId() + "/revisions/current/patch");
+ patchResp.assertOK();
+ String originalPatch = new String(Base64.decode(patchResp.getEntityContent()), UTF_8);
+ ApplyPatchPatchSetInput in = buildInput(originalPatch);
+ PushOneCommit.Result destChange = createChange();
+
+ RestResponse resp =
+ adminRestSession.post("/changes/" + destChange.getChangeId() + "/patch:apply", in);
+
+ resp.assertOK();
+ BinaryResult resultPatch = gApi.changes().id(destChange.getChangeId()).current().patch();
+ assertThat(removeHeader(resultPatch)).isEqualTo(removeHeader(originalPatch));
+ }
+
+ @Test
+ public void applyPatchWithConflict_fails() throws Exception {
+ initBaseWithFile(MODIFIED_FILE_NAME, "Unexpected base content");
+ ApplyPatchPatchSetInput in = buildInput(MODIFIED_FILE_DIFF);
+
+ Throwable error = assertThrows(RestApiException.class, () -> applyPatch(in));
+
+ assertThat(error).hasMessageThat().contains("Cannot apply patch");
+ assertThat(error).hasCauseThat().isInstanceOf(PatchApplyException.class);
+ assertThat(error).hasCauseThat().hasMessageThat().contains("Cannot apply: HunkHeader");
+ }
+
+ @Test
+ public void applyPatchWithoutAddPatchSetPermissions_fails() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .remove(
+ permissionKey(Permission.ADD_PATCH_SET)
+ .ref("refs/for/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+ PushOneCommit.Result destChange = createChange("dest change", "a file", "with content");
+ // Add-patch is always allowed for the change owner, so we need to use another account.
+ requestScopeOperations.setApiUser(accountCreator.user2().id());
+
+ Throwable error =
+ assertThrows(
+ AuthException.class, () -> gApi.changes().id(destChange.getChangeId()).applyPatch(in));
+
+ assertThat(error).hasMessageThat().contains("patch set");
+ }
+
+ @Test
+ public void applyPatchWithCustomMessage_success() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ in.commitMessage = "custom commit message";
+
+ ChangeInfo result = applyPatch(in);
+
+ assertThat(gApi.changes().id(result.id).current().commit(false).message)
+ .contains(in.commitMessage);
+ }
+
+ @Test
+ public void applyPatchWithBaseCommit_success() throws Exception {
+ PushOneCommit.Result baseCommit =
+ createChange("base commit", MODIFIED_FILE_NAME, MODIFIED_FILE_ORIGINAL_CONTENT);
+ baseCommit.assertOkStatus();
+ PushOneCommit.Result ignoredCommit =
+ createChange("Ignored file modification", MODIFIED_FILE_NAME, "Ignored file modification");
+ ignoredCommit.assertOkStatus();
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(MODIFIED_FILE_DIFF);
+ in.base = baseCommit.getCommit().getName();
+
+ ChangeInfo result = applyPatch(in);
+
+ assertThat(gApi.changes().id(result.id).current().commit(false).parents.get(0).commit)
+ .isEqualTo(in.base);
+ }
+
+ @Test
+ public void applyPatchWithDefaultAuthor_success() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+
+ ChangeInfo result = applyPatch(in);
+
+ GitPerson person = gApi.changes().id(result.id).current().commit(false).author;
+ GitPersonSubject.assertThat(person).email().isEqualTo(admin.email());
+ }
+
+ @Test
+ public void applyPatchWithAuthorOverrideMissingEmail_throwsIllegalArgument() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ in.author = new AccountInput();
+ in.author.name = "name";
+
+ Throwable error = assertThrows(IllegalArgumentException.class, () -> applyPatch(in));
+
+ assertThat(error).hasMessageThat().contains("E-mail");
+ }
+
+ @Test
+ public void applyPatchWithAuthorOverrideMissingName_throwsIllegalArgument() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ in.author = new AccountInput();
+ in.author.name = null;
+ in.author.email = "gerritlessjane@invalid";
+
+ Throwable error = assertThrows(IllegalArgumentException.class, () -> applyPatch(in));
+
+ assertThat(error).hasMessageThat().contains("Name");
+ }
+
+ @Test
+ public void applyPatchWithAuthorOverride_success() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ in.author = new AccountInput();
+ in.author.email = "gerritlessjane@invalid";
+ // This is an email address that doesn't exist as account on the Gerrit server.
+ in.author.name = "Gerritless Jane";
+
+ ChangeInfo result = applyPatch(in);
+
+ RevisionApi rApi = gApi.changes().id(result.id).current();
+ GitPerson author = rApi.commit(false).author;
+ GitPersonSubject.assertThat(author).email().isEqualTo(in.author.email);
+ GitPersonSubject.assertThat(author).name().isEqualTo(in.author.name);
+ GitPerson committer = rApi.commit(false).committer;
+ GitPersonSubject.assertThat(committer).email().isEqualTo(admin.getNameEmail().email());
+ }
+
+ @Test
+ public void applyPatchWithAuthorWithoutPermissions_fails() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ in.author = new AccountInput();
+ in.author.name = "Jane";
+ in.author.email = "jane@invalid";
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.FORGE_AUTHOR).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ Throwable error = assertThrows(ResourceConflictException.class, () -> applyPatch(in));
+
+ assertThat(error).hasMessageThat().contains("forge author");
+ }
+
+ @Test
+ public void applyPatchWithSelfAsForgedAuthor_success() throws Exception {
+ initDestBranch();
+ ApplyPatchPatchSetInput in = buildInput(ADDED_FILE_DIFF);
+ in.author = new AccountInput();
+ in.author.name = admin.fullName();
+ in.author.email = admin.email();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.FORGE_AUTHOR).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ ChangeInfo result = applyPatch(in);
+
+ GitPerson person = gApi.changes().id(result.id).current().commit(false).author;
+ GitPersonSubject.assertThat(person).email().isEqualTo(admin.email());
+ }
+
+ private void initDestBranch() throws Exception {
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, ApplyPatchIT.DESTINATION_BRANCH), head);
+ }
+
+ private void initBaseWithFile(String fileName, String fileContent) throws Exception {
+ PushOneCommit.Result baseCommit =
+ createChange("Add original file: " + fileName, fileName, fileContent);
+ baseCommit.assertOkStatus();
+ initDestBranch();
+ }
+
+ private ApplyPatchPatchSetInput buildInput(String patch) {
+ ApplyPatchPatchSetInput in = new ApplyPatchPatchSetInput();
+ in.patch = new ApplyPatchInput();
+ in.patch.patch = patch;
+ return in;
+ }
+
+ private ChangeInfo applyPatch(ApplyPatchPatchSetInput input) throws RestApiException {
+ return gApi.changes()
+ .create(new ChangeInput(project.get(), DESTINATION_BRANCH, COMMIT_MESSAGE))
+ .applyPatch(input);
+ }
+
+ private DiffInfo fetchDiffForFile(ChangeInfo result, String fileName) throws RestApiException {
+ return gApi.changes().id(result.id).current().file(fileName).diff();
+ }
+
+ private String removeHeader(BinaryResult bin) throws IOException {
+ return removeHeader(bin.asString());
+ }
+
+ private String removeHeader(String s) throws IOException {
+ return s.substring(s.indexOf("\ndiff --git"), s.length() - 1);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
index a443739..bdae0c4 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
@@ -19,15 +19,20 @@
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.project.testing.TestLabels.label;
import static com.google.gerrit.server.project.testing.TestLabels.value;
+import static org.eclipse.jgit.lib.Constants.HEAD;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseTimezone;
import com.google.gerrit.acceptance.VerifyNoPiiInChangeNotes;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelType;
@@ -37,6 +42,10 @@
import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.merge.MergeStrategy;
+import org.eclipse.jgit.merge.ThreeWayMerger;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -198,6 +207,128 @@
"distinctvoters:\"[Code-Review,Custom-Label,Custom-Label2],value=1,count>2\"", c1);
}
+ @Test
+ public void hasSubmoduleUpdate_withSubmoduleChangeInParent1() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+ PushOneCommit.Result r1 = createGitSubmoduleCommit("refs/for/master");
+ testRepo.reset(initial);
+ PushOneCommit.Result r2 = createNormalCommit("refs/for/master", "file1");
+ PushOneCommit.Result merge =
+ createMergeCommitChange(
+ "refs/for/master",
+ r1.getCommit(),
+ r2.getCommit(),
+ mergeAndGetTreeId(r1.getCommit(), r2.getCommit()));
+
+ assertNotMatching("has:submodule-update,base=1", merge.getChange().getId());
+ assertMatching("has:submodule-update,base=2", merge.getChange().getId());
+ assertNotMatching("has:submodule-update", merge.getChange().getId());
+ }
+
+ @Test
+ public void hasSubmoduleUpdate_withSubmoduleChangeInParent2() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+ PushOneCommit.Result r1 = createNormalCommit("refs/for/master", "file1");
+ testRepo.reset(initial);
+ PushOneCommit.Result r2 = createGitSubmoduleCommit("refs/for/master");
+ PushOneCommit.Result merge =
+ createMergeCommitChange(
+ "refs/for/master",
+ r1.getCommit(),
+ r2.getCommit(),
+ mergeAndGetTreeId(r1.getCommit(), r2.getCommit()));
+
+ assertMatching("has:submodule-update,base=1", merge.getChange().getId());
+ assertNotMatching("has:submodule-update,base=2", merge.getChange().getId());
+ assertNotMatching("has:submodule-update", merge.getChange().getId());
+ }
+
+ @Test
+ public void hasSubmoduleUpdate_withoutSubmoduleChange_doesNotMatch() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+ PushOneCommit.Result r1 = createNormalCommit("refs/for/master", "file1");
+ testRepo.reset(initial);
+ PushOneCommit.Result r2 = createNormalCommit("refs/for/master", "file2");
+ PushOneCommit.Result merge =
+ createMergeCommitChange(
+ "refs/for/master",
+ r1.getCommit(),
+ r2.getCommit(),
+ mergeAndGetTreeId(r1.getCommit(), r2.getCommit()));
+
+ assertNotMatching("has:submodule-update,base=1", merge.getChange().getId());
+ assertNotMatching("has:submodule-update,base=2", merge.getChange().getId());
+ assertNotMatching("has:submodule-update", merge.getChange().getId());
+ }
+
+ @Test
+ public void hasSubmoduleUpdate_withBaseParamGreaterThanParentCount_doesNotMatch()
+ throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+ PushOneCommit.Result r1 = createNormalCommit("refs/for/master", "file1");
+ testRepo.reset(initial);
+ PushOneCommit.Result r2 = createGitSubmoduleCommit("refs/for/master");
+ PushOneCommit.Result merge =
+ createMergeCommitChange(
+ "refs/for/master",
+ r1.getCommit(),
+ r2.getCommit(),
+ mergeAndGetTreeId(r1.getCommit(), r2.getCommit()));
+
+ assertNotMatching("has:submodule-update,base=3", merge.getChange().getId());
+ }
+
+ @Test
+ public void hasSubmoduleUpdate_withWrongArgs_throws() {
+ assertError(
+ "has:submodule-update,base=xyz",
+ changeOperations.newChange().project(project).create(),
+ "failed to parse the parent number xyz: For input string: \"xyz\"");
+ assertError(
+ "has:submodule-update,base=1,arg=foo",
+ changeOperations.newChange().project(project).create(),
+ "wrong number of arguments for the has:submodule-update operator");
+ assertError(
+ "has:submodule-update,base",
+ changeOperations.newChange().project(project).create(),
+ "unexpected base value format");
+ }
+
+ private PushOneCommit.Result createGitSubmoduleCommit(String ref) throws Exception {
+ return pushFactory
+ .create(admin.newIdent(), testRepo, "subject", ImmutableMap.of())
+ .addGitSubmodule(
+ "modules/module-a", ObjectId.fromString("19f1787342cb15d7e82a762f6b494e91ccb4dd34"))
+ .to(ref);
+ }
+
+ private PushOneCommit.Result createNormalCommit(String ref, String fileName) throws Exception {
+ return pushFactory
+ .create(admin.newIdent(), testRepo, "subject", ImmutableMap.of(fileName, fileName))
+ .to(ref);
+ }
+
+ private PushOneCommit.Result createMergeCommitChange(
+ String ref, RevCommit parent1, RevCommit parent2, @Nullable ObjectId treeId)
+ throws Exception {
+ PushOneCommit m = pushFactory.create(admin.newIdent(), testRepo);
+ m.setParents(ImmutableList.of(parent1, parent2));
+ if (treeId != null) {
+ m.setTopLevelTreeId(treeId);
+ }
+ PushOneCommit.Result result = m.to(ref);
+ result.assertOkStatus();
+ return result;
+ }
+
+ private ObjectId mergeAndGetTreeId(RevCommit c1, RevCommit c2) throws Exception {
+ ThreeWayMerger threeWayMerger = MergeStrategy.RESOLVE.newMerger(repo(), true);
+ threeWayMerger.setBase(c1.getParent(0));
+ boolean mergeResult = threeWayMerger.merge(c1, c2);
+ assertThat(mergeResult).isTrue();
+ return threeWayMerger.getResultTreeId();
+ }
+
private void assertMatching(String requirement, Change.Id change) {
assertThat(evaluate(requirement, change).status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
@@ -208,6 +339,12 @@
.isEqualTo(SubmitRequirementExpressionResult.Status.FAIL);
}
+ private void assertError(String requirement, Change.Id change, String errorMessage) {
+ SubmitRequirementExpressionResult result = evaluate(requirement, change);
+ assertThat(result.status()).isEqualTo(SubmitRequirementExpressionResult.Status.ERROR);
+ assertThat(result.errorMessage().get()).isEqualTo(errorMessage);
+ }
+
private SubmitRequirementExpressionResult evaluate(String requirement, Change.Id change) {
ChangeData cd = changeDataFactory.create(project, change);
return submitRequirementsEvaluator.evaluateExpression(
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 3b94a50..08f65da 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -2678,6 +2678,23 @@
}
@Test
+ @GerritConfig(
+ name = "plugins.transitionalPushOptions",
+ values = {"gerrit~foo", "gerrit~bar"})
+ public void transitionalPushOptionsArePassedToCommitValidationListener() throws Exception {
+ TestValidator validator = new TestValidator();
+ try (Registration registration = extensionRegistry.newRegistration().add(validator)) {
+ PushOneCommit push =
+ pushFactory.create(admin.newIdent(), testRepo, "change2", "b.txt", "content");
+ push.setPushOptions(ImmutableList.of("trace=123", "gerrit~foo", "gerrit~bar=456"));
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertOkStatus();
+ assertThat(validator.pushOptions())
+ .containsExactly("trace", "123", "gerrit~foo", "", "gerrit~bar", "456");
+ }
+ }
+
+ @Test
public void pluginPushOptionsHelp() throws Exception {
PluginPushOption fooOption = new TestPluginPushOption("foo", "some description");
PluginPushOption barOption = new TestPluginPushOption("bar", "other description");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index dbebbf9..67c784b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -19,7 +19,10 @@
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.entities.Permission.CREATE;
import static com.google.gerrit.entities.Permission.READ;
+import static com.google.gerrit.entities.RefNames.HEAD;
import static com.google.gerrit.entities.RefNames.changeMetaRef;
+import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
+import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.common.testing.GitPersonSubject.assertThat;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -47,6 +50,7 @@
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.accounts.AccountInput;
+import com.google.gerrit.extensions.api.changes.ApplyPatchInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -59,6 +63,7 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
+import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.GitPerson;
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -74,6 +79,7 @@
import com.google.gerrit.server.git.validators.CommitValidationMessage;
import com.google.gerrit.server.submit.ChangeAlreadyMergedException;
import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gson.stream.JsonReader;
import com.google.inject.Inject;
import java.io.ByteArrayOutputStream;
import java.util.ArrayList;
@@ -85,6 +91,8 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.api.errors.PatchApplyException;
+import org.eclipse.jgit.api.errors.PatchFormatException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -94,6 +102,7 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.util.Base64;
import org.junit.Before;
import org.junit.Test;
@@ -923,6 +932,167 @@
}
@Test
+ public void createChangeWithBothMergeAndPatch_fails() throws Exception {
+ ChangeInput input = newMergeChangeInput("foo", "master", "");
+ input.patch = new ApplyPatchInput();
+ assertCreateFails(
+ input, BadRequestException.class, "Only one of `merge` and `patch` arguments can be set");
+ }
+
+ private static final String PATCH_FILE_NAME = "a_file.txt";
+ private static final String PATCH_NEW_FILE_CONTENT = "First added line\nSecond added line\n";
+ private static final String PATCH_INPUT =
+ "diff --git a/a_file.txt b/a_file.txt\n"
+ + "new file mode 100644\n"
+ + "index 0000000..f0eec86\n"
+ + "--- /dev/null\n"
+ + "+++ b/a_file.txt\n"
+ + "@@ -0,0 +1,2 @@\n"
+ + "+First added line\n"
+ + "+Second added line\n";
+ private static final String MODIFICATION_PATCH_INPUT =
+ "diff --git a/a_file.txt b/a_file.txt\n"
+ + "new file mode 100644\n"
+ + "--- a/a_file.txt\n"
+ + "+++ b/a_file.txt.txt\n"
+ + "@@ -1,2 +1 @@\n"
+ + "-First original line\n"
+ + "-Second original line\n"
+ + "+Modified line\n";
+
+ @Test
+ public void createPatchApplyingChange_success() throws Exception {
+ createBranch(BranchNameKey.create(project, "other"));
+ ChangeInput input = newPatchApplyingChangeInput("other", PATCH_INPUT);
+
+ ChangeInfo info = assertCreateSucceeds(input);
+
+ DiffInfo diff = gApi.changes().id(info.id).current().file(PATCH_FILE_NAME).diff();
+ assertDiffForNewFile(diff, info.currentRevision, PATCH_FILE_NAME, PATCH_NEW_FILE_CONTENT);
+ }
+
+ @Test
+ public void createPatchApplyingChange_fromGerritPatch_success() throws Exception {
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
+ PushOneCommit.Result baseCommit =
+ createChange("Add file", PATCH_FILE_NAME, PATCH_NEW_FILE_CONTENT);
+ baseCommit.assertOkStatus();
+ BinaryResult originalPatch = gApi.changes().id(baseCommit.getChangeId()).current().patch();
+ createBranchWithRevision(BranchNameKey.create(project, "other"), head);
+ ChangeInput input = newPatchApplyingChangeInput("other", originalPatch.asString());
+
+ ChangeInfo info = assertCreateSucceeds(input);
+
+ DiffInfo diff = gApi.changes().id(info.id).current().file(PATCH_FILE_NAME).diff();
+ assertDiffForNewFile(diff, info.currentRevision, PATCH_FILE_NAME, PATCH_NEW_FILE_CONTENT);
+ }
+
+ @Test
+ public void createPatchApplyingChange_fromGerritPatchUsingRest_success() throws Exception {
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
+ PushOneCommit.Result baseCommit =
+ createChange("Add file", PATCH_FILE_NAME, PATCH_NEW_FILE_CONTENT);
+ baseCommit.assertOkStatus();
+ createBranchWithRevision(BranchNameKey.create(project, "other"), head);
+ RestResponse patchResp =
+ userRestSession.get("/changes/" + baseCommit.getChangeId() + "/revisions/current/patch");
+ patchResp.assertOK();
+ String originalPatch = new String(Base64.decode(patchResp.getEntityContent()), UTF_8);
+ ChangeInput input = newPatchApplyingChangeInput("other", originalPatch);
+
+ ChangeInfo info = assertCreateSucceedsUsingRest(input);
+
+ DiffInfo diff = gApi.changes().id(info.id).current().file(PATCH_FILE_NAME).diff();
+ assertDiffForNewFile(diff, info.currentRevision, PATCH_FILE_NAME, PATCH_NEW_FILE_CONTENT);
+ }
+
+ @Test
+ public void createPatchApplyingChange_withParentChange_success() throws Exception {
+ Result change = createChange();
+ ChangeInput input = newPatchApplyingChangeInput("other", PATCH_INPUT);
+ input.baseChange = change.getChangeId();
+
+ ChangeInfo info = assertCreateSucceeds(input);
+
+ assertThat(gApi.changes().id(info.id).current().commit(false).parents.get(0).commit)
+ .isEqualTo(change.getCommit().getId().name());
+ DiffInfo diff = gApi.changes().id(info.id).current().file(PATCH_FILE_NAME).diff();
+ assertDiffForNewFile(diff, info.currentRevision, PATCH_FILE_NAME, PATCH_NEW_FILE_CONTENT);
+ }
+
+ @Test
+ public void createPatchApplyingChange_withParentCommit_success() throws Exception {
+ createBranch(BranchNameKey.create(project, "other"));
+ Result baseChange = createChange("refs/heads/other");
+ PushOneCommit.Result ignoredCommit = createChange();
+ ignoredCommit.assertOkStatus();
+ ChangeInput input = newPatchApplyingChangeInput("other", PATCH_INPUT);
+ input.baseCommit = baseChange.getCommit().getId().name();
+
+ ChangeInfo info = assertCreateSucceeds(input);
+
+ assertThat(gApi.changes().id(info.id).current().commit(false).parents.get(0).commit)
+ .isEqualTo(input.baseCommit);
+ DiffInfo diff = gApi.changes().id(info.id).current().file(PATCH_FILE_NAME).diff();
+ assertDiffForNewFile(diff, info.currentRevision, PATCH_FILE_NAME, PATCH_NEW_FILE_CONTENT);
+ }
+
+ @Test
+ public void createPatchApplyingChange_withEmptyTip_fails() throws Exception {
+ ChangeInput input = newPatchApplyingChangeInput("foo", "patch");
+ input.newBranch = true;
+ assertCreateFails(
+ input, BadRequestException.class, "Cannot apply patch on top of an empty tree");
+ }
+
+ @Test
+ public void createPatchApplyingChange_fromBadPatch_fails() throws Exception {
+ final String invalidPatch = "@@ -2,2 +2,3 @@ a\n" + " b\n" + "+c\n" + " d";
+ createBranch(BranchNameKey.create(project, "other"));
+ ChangeInput input = newPatchApplyingChangeInput("other", invalidPatch);
+ assertCreateFailsWithCause(
+ input, BadRequestException.class, PatchFormatException.class, "Format error");
+ }
+
+ @Test
+ public void createPatchApplyingChange_withAuthorOverride_success() throws Exception {
+ createBranch(BranchNameKey.create(project, "other"));
+ ChangeInput input = newPatchApplyingChangeInput("other", PATCH_INPUT);
+ input.author = new AccountInput();
+ input.author.email = "gerritlessjane@invalid";
+ // This is an email address that doesn't exist as account on the Gerrit server.
+ input.author.name = "Gerritless Jane";
+ ChangeInfo info = assertCreateSucceeds(input);
+
+ RevisionApi rApi = gApi.changes().id(info.id).current();
+ GitPerson author = rApi.commit(false).author;
+ assertThat(author).email().isEqualTo(input.author.email);
+ assertThat(author).name().isEqualTo(input.author.name);
+ GitPerson committer = rApi.commit(false).committer;
+ assertThat(committer).email().isEqualTo(admin.getNameEmail().email());
+ }
+
+ @Test
+ public void createPatchApplyingChange_withInfeasiblePatch_fails() throws Exception {
+ createBranch(BranchNameKey.create(project, "other"));
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ testRepo,
+ "Adding unexpected base content, which will cause the patch to fail",
+ PATCH_FILE_NAME,
+ "unexpected base content");
+ Result conflictingChange = push.to("refs/heads/other");
+ conflictingChange.assertOkStatus();
+ ChangeInput input = newPatchApplyingChangeInput("other", MODIFICATION_PATCH_INPUT);
+
+ assertCreateFailsWithCause(
+ input, RestApiException.class, PatchApplyException.class, "Cannot apply: HunkHeader");
+ }
+
+ @Test
@UseSystemTime
public void sha1sOfTwoNewChangesDiffer() throws Exception {
ChangeInput changeInput = newChangeInput(ChangeStatus.NEW);
@@ -1084,17 +1254,38 @@
private ChangeInfo assertCreateSucceeds(ChangeInput in) throws Exception {
ChangeInfo out = gApi.changes().create(in).get();
+ validateCreateSucceeds(in, out);
+ return out;
+ }
+
+ private ChangeInfo assertCreateSucceedsUsingRest(ChangeInput in) throws Exception {
+ RestResponse resp = adminRestSession.post("/changes/", in);
+ resp.assertCreated();
+ ChangeInfo res = readContentFromJson(resp, ChangeInfo.class);
+ // The original result doesn't contain any revision data.
+ ChangeInfo out = gApi.changes().id(res.changeId).get(ALL_REVISIONS, CURRENT_COMMIT);
+ validateCreateSucceeds(in, out);
+ return out;
+ }
+
+ private static <T> T readContentFromJson(RestResponse r, Class<T> clazz) throws Exception {
+ try (JsonReader jsonReader = new JsonReader(r.getReader())) {
+ return newGson().fromJson(jsonReader, clazz);
+ }
+ }
+
+ private void validateCreateSucceeds(ChangeInput in, ChangeInfo out) throws Exception {
assertThat(out.project).isEqualTo(in.project);
assertThat(RefNames.fullName(out.branch)).isEqualTo(RefNames.fullName(in.branch));
assertThat(out.subject).isEqualTo(Splitter.on("\n").splitToList(in.subject).get(0));
assertThat(out.topic).isEqualTo(in.topic);
assertThat(out.status).isEqualTo(in.status);
- if (in.isPrivate) {
+ if (Boolean.TRUE.equals(in.isPrivate)) {
assertThat(out.isPrivate).isTrue();
} else {
assertThat(out.isPrivate).isNull();
}
- if (in.workInProgress) {
+ if (Boolean.TRUE.equals(in.workInProgress)) {
assertThat(out.workInProgress).isTrue();
} else {
assertThat(out.workInProgress).isNull();
@@ -1103,7 +1294,6 @@
assertThat(out.submitted).isNull();
assertThat(out.containsGitConflicts).isNull();
assertThat(in.status).isEqualTo(ChangeStatus.NEW);
- return out;
}
private ChangeInfo assertCreateSucceedsWithConflicts(ChangeInput in) throws Exception {
@@ -1132,6 +1322,17 @@
assertThat(thrown).hasMessageThat().contains(errSubstring);
}
+ private void assertCreateFailsWithCause(
+ ChangeInput in,
+ Class<? extends RestApiException> errType,
+ Class<? extends Exception> causeType,
+ String causeSubstring)
+ throws Exception {
+ Throwable thrown = assertThrows(errType, () -> gApi.changes().create(in));
+ assertThat(thrown).hasCauseThat().isInstanceOf(causeType);
+ assertThat(thrown).hasCauseThat().hasMessageThat().contains(causeSubstring);
+ }
+
// TODO(davido): Expose setting of account preferences in the API
private void setSignedOffByFooter(boolean value) throws Exception {
RestResponse r = adminRestSession.get("/accounts/" + admin.email() + "/preferences");
@@ -1174,6 +1375,19 @@
return in;
}
+ private ChangeInput newPatchApplyingChangeInput(String targetBranch, String patch) {
+ // create a change applying the given patch on the target branch in gerrit
+ ChangeInput in = new ChangeInput();
+ in.project = project.get();
+ in.branch = targetBranch;
+ in.subject = "apply patch to " + targetBranch;
+ in.status = ChangeStatus.NEW;
+ ApplyPatchInput patchInput = new ApplyPatchInput();
+ patchInput.patch = patch;
+ in.patch = patchInput;
+ return in;
+ }
+
/**
* Create an empty commit in master, two new branches with one commit each.
*
diff --git a/polygerrit-ui/FE_Style_Guide.md b/polygerrit-ui/FE_Style_Guide.md
index 56b5aee..d2b865b 100644
--- a/polygerrit-ui/FE_Style_Guide.md
+++ b/polygerrit-ui/FE_Style_Guide.md
@@ -125,10 +125,6 @@
Do not use getAppContext() anywhere else in a class.
-**Note:** This rule doesn't apply for HTML/Polymer elements classes. A browser creates instances of such classes
-implicitly and calls the constructor without parameters. See
-[Assign required services in a HTML/Polymer element constructor](#assign-dependencies-in-html-element-constructor)
-
**Good:**
```Javascript
export class UserService {
@@ -160,10 +156,3 @@
}
```
-
-## <a name="assign-dependencies-in-html-element-constructor"></a>Assign required services in a HTML/Polymer element constructor
-If a class is a custom HTML/Polymer element, the class must assign all required services in the constructor.
-A browser creates instances of such classes implicitly, so it is impossible to pass anything as a parameter to
-the element's class constructor.
-
-Do not use appContext anywhere except the constructor of the class.
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
index 85be2d5..a8d2157 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
@@ -19,7 +19,6 @@
import '../gr-repo-list/gr-repo-list';
import {getBaseUrl} from '../../../utils/url-util';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {
AdminNavLinksOption,
getAdminLinks,
@@ -113,10 +112,12 @@
private reloading = false;
// private but used in the tests
- readonly jsAPI = getAppContext().jsApiService;
+ readonly jsAPI = getAppContext().pluginLoader.jsApiService;
private readonly restApiService = getAppContext().restApiService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
private readonly getAdminViewModel = resolve(this, adminViewModelToken);
private readonly getGroupViewModel = resolve(this, groupViewModelToken);
@@ -460,7 +461,7 @@
const promises: [Promise<AccountDetailInfo | undefined>, Promise<void>] =
[
this.restApiService.getAccount(),
- getPluginLoader().awaitPluginsLoaded(),
+ this.pluginLoader.awaitPluginsLoaded(),
];
const result = await Promise.all(promises);
this.account = result[0];
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
index d65d171..f83bbe2 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-admin-view';
import {AdminSubsectionLink, GrAdminView} from './gr-admin-view';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {stubBaseUrl, stubElement, stubRestApi} from '../../../test/test-utils';
import {GerritView} from '../../../services/router/router-model';
import {query, queryAll, queryAndAssert} from '../../../test/test-utils';
@@ -20,6 +19,7 @@
import {RepoDetailView} from '../../../models/views/repo';
import {testResolver} from '../../../test/common-test-setup';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
+import {getAppContext} from '../../../services/app-context';
function createAdminCapabilities() {
return {
@@ -36,7 +36,9 @@
element = await fixture(html`<gr-admin-view></gr-admin-view>`);
stubRestApi('getProjectConfig').returns(Promise.resolve(undefined));
const pluginsLoaded = Promise.resolve();
- sinon.stub(getPluginLoader(), 'awaitPluginsLoaded').returns(pluginsLoaded);
+ sinon
+ .stub(getAppContext().pluginLoader, 'awaitPluginsLoaded')
+ .returns(pluginsLoaded);
await pluginsLoaded;
await element.updateComplete;
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index cd7aa9d..bee1aaa 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -18,12 +18,10 @@
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getDisplayName} from '../../../utils/display-name-util';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAppContext} from '../../../services/app-context';
import {truncatePath} from '../../../utils/path-list-util';
import {changeStatuses} from '../../../utils/change-util';
import {isSelf, isServiceUser} from '../../../utils/account-util';
-import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
import {
ChangeInfo,
ServerInfo,
@@ -118,7 +116,9 @@
@state() private dynamicCellEndpoints?: string[];
- reporting: ReportingService = getAppContext().reportingService;
+ private readonly reporting = getAppContext().reportingService;
+
+ private readonly pluginLoader = getAppContext().pluginLoader;
private readonly getBulkActionsModel = resolve(this, bulkActionsModelToken);
@@ -138,13 +138,11 @@
override connectedCallback() {
super.connectedCallback();
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- this.dynamicCellEndpoints = getPluginEndpoints().getDynamicEndpoints(
- 'change-list-item-cell'
- );
- });
+ this.pluginLoader.awaitPluginsLoaded().then(() => {
+ this.dynamicCellEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ 'change-list-item-cell'
+ );
+ });
this.addEventListener('click', this.onItemClick);
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index 1eebf88..8d996a1 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -11,7 +11,6 @@
import {getAppContext} from '../../../services/app-context';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {GrCursorManager} from '../../shared/gr-cursor-manager/gr-cursor-manager';
import {
AccountInfo,
@@ -179,8 +178,8 @@
this.restApiService.getConfig().then(config => {
this.config = config;
});
- getPluginLoader()
- .awaitPluginsLoaded()
+ getAppContext()
+ .pluginLoader.awaitPluginsLoaded()
.then(() => {
this.dynamicHeaderEndpoints =
getPluginEndpoints().getDynamicEndpoints('change-list-header');
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 834f2b0..8c1f0b2 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -18,7 +18,6 @@
import '../gr-confirm-submit-dialog/gr-confirm-submit-dialog';
import '../../../styles/shared-styles';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAppContext} from '../../../services/app-context';
import {CURRENT} from '../../../utils/patch-set-util';
import {
@@ -33,7 +32,7 @@
HttpMethod,
NotifyType,
} from '../../../constants/constants';
-import {EventType as PluginEventType, TargetElement} from '../../../api/plugin';
+import {TargetElement} from '../../../api/plugin';
import {
AccountInfo,
ActionInfo,
@@ -54,7 +53,6 @@
ReviewInput,
} from '../../../types/common';
import {GrConfirmAbandonDialog} from '../gr-confirm-abandon-dialog/gr-confirm-abandon-dialog';
-import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
import {GrCreateChangeDialog} from '../../admin/gr-create-change-dialog/gr-create-change-dialog';
import {GrConfirmSubmitDialog} from '../gr-confirm-submit-dialog/gr-confirm-submit-dialog';
@@ -108,6 +106,8 @@
import {createSearchUrl} from '../../../models/views/search';
import {createChangeUrl} from '../../../models/views/change';
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
+import {ShowRevisionActionsDetail} from '../../shared/gr-js-api-interface/gr-js-api-types';
+import {whenVisible} from '../../../utils/dom-util';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -351,7 +351,7 @@
@query('#mainContent') mainContent?: Element;
- @query('#overlay') overlay?: GrOverlay;
+ @query('#actionsModal') actionsModal?: HTMLDialogElement;
@query('#confirmRebase') confirmRebase?: GrConfirmRebaseDialog;
@@ -391,8 +391,9 @@
private readonly reporting = getAppContext().reportingService;
- // Accessed in tests
- readonly jsAPI = getAppContext().jsApiService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
+ private readonly jsAPI = getAppContext().pluginLoader.jsApiService;
private readonly getChangeModel = resolve(this, changeModelToken);
@@ -547,12 +548,6 @@
constructor() {
super();
- this.addEventListener('fullscreen-overlay-opened', () =>
- this.handleHideBackgroundContent()
- );
- this.addEventListener('fullscreen-overlay-closed', () =>
- this.handleShowBackgroundContent()
- );
}
override connectedCallback() {
@@ -583,6 +578,11 @@
gr-button {
display: block;
}
+ dialog {
+ padding: 0;
+ border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
+ }
#actionLoadingMessage {
align-items: center;
color: var(--deemphasized-text-color);
@@ -670,7 +670,7 @@
<span id="moreMessage">More</span>
</gr-dropdown>
</div>
- <gr-overlay id="overlay" with-backdrop="">
+ <dialog id="actionsModal" tabindex="-1">
<gr-confirm-rebase-dialog
id="confirmRebase"
class="confirmDialog"
@@ -769,7 +769,7 @@
Do you really want to delete the edit?
</div>
</gr-dialog>
- </gr-overlay>
+ </dialog>
`;
}
@@ -884,17 +884,12 @@
}
private handleLoadingComplete() {
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => (this.loading = false));
+ this.pluginLoader.awaitPluginsLoaded().then(() => (this.loading = false));
}
// private but used in test
- sendShowRevisionActions(detail: {
- change: ChangeInfo;
- revisionActions: ActionNameToActionInfoMap;
- }) {
- this.jsAPI.handleEvent(PluginEventType.SHOW_REVISION_ACTIONS, detail);
+ sendShowRevisionActions(detail: ShowRevisionActionsDetail) {
+ this.jsAPI.handleShowRevisionActions(detail);
}
addActionButton(type: ActionType, label: string) {
@@ -1566,20 +1561,20 @@
for (const dialogEl of dialogEls) {
(dialogEl as HTMLElement).hidden = true;
}
- assertIsDefined(this.overlay, 'overlay');
- this.overlay.close();
+ assertIsDefined(this.actionsModal, 'actionsModal');
+ this.actionsModal.close();
}
// private but used in test
handleRebaseConfirm(e: CustomEvent<ConfirmRebaseEventDetail>) {
assertIsDefined(this.confirmRebase, 'confirmRebase');
- assertIsDefined(this.overlay, 'overlay');
+ assertIsDefined(this.actionsModal, 'actionsModal');
const el = this.confirmRebase;
const payload = {
base: e.detail.base,
allow_conflicts: e.detail.allowConflicts,
};
- this.overlay.close();
+ this.actionsModal.close();
el.hidden = true;
this.fireAction(
'/rebase',
@@ -1602,7 +1597,7 @@
private handleCherryPickRestApi(conflicts: boolean) {
assertIsDefined(this.confirmCherrypick, 'confirmCherrypick');
- assertIsDefined(this.overlay, 'overlay');
+ assertIsDefined(this.actionsModal, 'actionsModal');
const el = this.confirmCherrypick;
if (!el.branch) {
fireAlert(this, ERR_BRANCH_EMPTY);
@@ -1612,7 +1607,7 @@
fireAlert(this, ERR_COMMIT_EMPTY);
return;
}
- this.overlay.close();
+ this.actionsModal.close();
el.hidden = true;
this.fireAction(
'/cherrypick',
@@ -1630,13 +1625,13 @@
// private but used in test
handleMoveConfirm() {
assertIsDefined(this.confirmMove, 'confirmMove');
- assertIsDefined(this.overlay, 'overlay');
+ assertIsDefined(this.actionsModal, 'actionsModal');
const el = this.confirmMove;
if (!el.branch) {
fireAlert(this, ERR_BRANCH_EMPTY);
return;
}
- this.overlay.close();
+ this.actionsModal.close();
el.hidden = true;
this.fireAction('/move', assertUIActionInfo(this.actions.move), false, {
destination_branch: el.branch,
@@ -1646,11 +1641,11 @@
private handleRevertDialogConfirm(e: CustomEvent<ConfirmRevertEventDetail>) {
assertIsDefined(this.confirmRevertDialog, 'confirmRevertDialog');
- assertIsDefined(this.overlay, 'overlay');
+ assertIsDefined(this.actionsModal, 'actionsModal');
const revertType = e.detail.revertType;
const message = e.detail.message;
const el = this.confirmRevertDialog;
- this.overlay.close();
+ this.actionsModal.close();
el.hidden = true;
switch (revertType) {
case RevertType.REVERT_SINGLE_CHANGE:
@@ -1682,9 +1677,9 @@
// private but used in test
handleAbandonDialogConfirm() {
assertIsDefined(this.confirmAbandonDialog, 'confirmAbandonDialog');
- assertIsDefined(this.overlay, 'overlay');
+ assertIsDefined(this.actionsModal, 'actionsModal');
const el = this.confirmAbandonDialog;
- this.overlay.close();
+ this.actionsModal.close();
el.hidden = true;
this.fireAction(
'/abandon',
@@ -1703,8 +1698,8 @@
}
private handleCloseCreateFollowUpChange() {
- assertIsDefined(this.overlay, 'overlay');
- this.overlay.close();
+ assertIsDefined(this.actionsModal, 'actionsModal');
+ this.actionsModal.close();
}
private handleDeleteConfirm() {
@@ -1818,8 +1813,9 @@
this.hideAllDialogs();
if (dialog.init) dialog.init();
dialog.hidden = false;
- assertIsDefined(this.overlay, 'overlay');
- this.overlay.open().then(() => {
+ assertIsDefined(this.actionsModal, 'actionsModal');
+ this.actionsModal.showModal();
+ whenVisible(dialog, () => {
if (dialog.resetFocus) {
dialog.resetFocus();
}
@@ -2099,18 +2095,6 @@
);
}
- // private but used in test
- handleHideBackgroundContent() {
- assertIsDefined(this.mainContent, 'mainContent');
- this.mainContent.classList.add('overlayOpen');
- }
-
- // private but used in test
- handleShowBackgroundContent() {
- assertIsDefined(this.mainContent, 'mainContent');
- this.mainContent.classList.remove('overlayOpen');
- }
-
/**
* Merge sources of change actions into a single ordered array of action
* values.
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index a5431ed..d815207 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-change-actions';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {
createAccountWithId,
createApproval,
@@ -50,7 +49,6 @@
import {fixture, html, assert} from '@open-wc/testing';
import {GrConfirmCherrypickDialog} from '../gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog';
import {GrDropdown} from '../../shared/gr-dropdown/gr-dropdown';
-import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {GrConfirmSubmitDialog} from '../gr-confirm-submit-dialog/gr-confirm-submit-dialog';
import {GrConfirmRebaseDialog} from '../gr-confirm-rebase-dialog/gr-confirm-rebase-dialog';
import {GrConfirmMoveDialog} from '../gr-confirm-move-dialog/gr-confirm-move-dialog';
@@ -59,6 +57,7 @@
import {EventType} from '../../../types/events';
import {testResolver} from '../../../test/common-test-setup';
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
+import {getAppContext} from '../../../services/app-context';
// TODO(dhruvsri): remove use of _populateRevertMessage as it's private
suite('gr-change-actions tests', () => {
@@ -119,7 +118,7 @@
});
sinon
- .stub(getPluginLoader(), 'awaitPluginsLoaded')
+ .stub(getAppContext().pluginLoader, 'awaitPluginsLoaded')
.returns(Promise.resolve());
element = await fixture<GrChangeActions>(html`
@@ -207,13 +206,7 @@
<span id="moreMessage"> More </span>
</gr-dropdown>
</div>
- <gr-overlay
- aria-hidden="true"
- id="overlay"
- style="outline: none; display: none;"
- tabindex="-1"
- with-backdrop=""
- >
+ <dialog id="actionsModal" tabindex="-1">
<gr-confirm-rebase-dialog class="confirmDialog" id="confirmRebase">
</gr-confirm-rebase-dialog>
<gr-confirm-cherrypick-dialog
@@ -279,7 +272,7 @@
Do you really want to delete the edit?
</div>
</gr-dialog>
- </gr-overlay>
+ </dialog>
`
);
});
@@ -479,9 +472,6 @@
stubRestApi('getFromProjectLookup').returns(
Promise.resolve('test' as RepoName)
);
- sinon
- .stub(queryAndAssert<GrOverlay>(element, '#overlay'), 'open')
- .returns(Promise.resolve());
element.change = {
...createChangeViewChange(),
revisions: {
@@ -518,9 +508,6 @@
stubRestApi('getFromProjectLookup').returns(
Promise.resolve('test' as RepoName)
);
- sinon
- .stub(queryAndAssert<GrOverlay>(element, '#overlay'), 'open')
- .returns(Promise.resolve());
element.change = {
...createChangeViewChange(),
revisions: {
@@ -713,42 +700,12 @@
);
});
- test('fullscreen-overlay-opened hides content', () => {
- const spy = sinon.spy(element, 'handleHideBackgroundContent');
- queryAndAssert<GrOverlay>(element, '#overlay').dispatchEvent(
- new CustomEvent('fullscreen-overlay-opened', {
- composed: true,
- bubbles: true,
- })
- );
- assert.isTrue(spy.called);
- assert.isTrue(
- queryAndAssert<Element>(element, '#mainContent').classList.contains(
- 'overlayOpen'
- )
- );
- });
-
- test('fullscreen-overlay-closed shows content', () => {
- const spy = sinon.spy(element, 'handleShowBackgroundContent');
- queryAndAssert<GrOverlay>(element, '#overlay').dispatchEvent(
- new CustomEvent('fullscreen-overlay-closed', {
- composed: true,
- bubbles: true,
- })
- );
- assert.isTrue(spy.called);
- assert.isFalse(
- queryAndAssert<Element>(element, '#mainContent').classList.contains(
- 'overlayOpen'
- )
- );
- });
-
test('setReviewOnRevert', () => {
const review = {labels: {Foo: 1, 'Bar-Baz': -2}};
const changeId = 1234 as NumericChangeId;
- sinon.stub(element.jsAPI, 'getReviewPostRevert').returns(review);
+ sinon
+ .stub(getAppContext().pluginLoader.jsApiService, 'getReviewPostRevert')
+ .returns(review);
const saveStub = stubRestApi('saveChangeReview').returns(
Promise.resolve(new Response())
);
@@ -2685,7 +2642,7 @@
stubRestApi('send').returns(Promise.reject(new Error('error')));
sinon
- .stub(getPluginLoader(), 'awaitPluginsLoaded')
+ .stub(getAppContext().pluginLoader, 'awaitPluginsLoaded')
.returns(Promise.resolve());
element = await fixture<GrChangeActions>(html`
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
index 038c34a..ed4affd 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
@@ -5,7 +5,7 @@
*/
import '../../../test/common-test-setup';
import './gr-change-metadata';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+
import {ChangeRole, GrChangeMetadata} from './gr-change-metadata';
import {
createServerInfo,
@@ -56,6 +56,7 @@
import {nothing} from 'lit';
import {fixture, html, assert} from '@open-wc/testing';
import {EventType} from '../../../types/events';
+import {getAppContext} from '../../../services/app-context';
suite('gr-change-metadata tests', () => {
let element: GrChangeMetadata;
@@ -978,7 +979,7 @@
const hookEl = (await plugin!
.hook('change-metadata-item')
.getLastAttached()) as MetadataGrEndpointDecorator;
- getPluginLoader().loadPlugins([]);
+ getAppContext().pluginLoader.loadPlugins([]);
await element.updateComplete;
assert.strictEqual(hookEl.plugin, plugin!);
assert.strictEqual(hookEl.change, element.change);
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 6a7aeeb..007dd15 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -39,7 +39,6 @@
import {querySelectorAll, windowLocationReload} from '../../../utils/dom-util';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {RevisionInfo as RevisionInfoClass} from '../../shared/revision-info/revision-info';
import {
ChangeStatus,
@@ -65,7 +64,6 @@
isInvolved,
roleDetails,
} from '../../../utils/change-util';
-import {EventType as PluginEventType} from '../../../api/plugin';
import {customElement, property, query, state} from 'lit/decorators.js';
import {GrApplyFixDialog} from '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog';
import {GrFileListHeader} from '../gr-file-list-header/gr-file-list-header';
@@ -531,7 +529,7 @@
// Accessed in tests.
readonly reporting = getAppContext().reportingService;
- readonly jsAPI = getAppContext().jsApiService;
+ readonly jsAPI = getAppContext().pluginLoader.jsApiService;
private readonly getChecksModel = resolve(this, checksModelToken);
@@ -539,6 +537,8 @@
private readonly flagsService = getAppContext().flagsService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
private readonly getUserModel = resolve(this, userModelToken);
private readonly getChangeModel = resolve(this, changeModelToken);
@@ -817,7 +817,7 @@
if (!this.isFirstConnection) return;
this.isFirstConnection = false;
- getPluginLoader()
+ this.pluginLoader
.awaitPluginsLoaded()
.then(() => {
this.pluginTabsHeaderEndpoints =
@@ -1149,11 +1149,6 @@
flex: initial;
margin: 0;
}
- /* Change actions are the only thing thant need to remain visible due
- to the fact that they may have the currently visible overlay open. */
- #mainContent.overlayOpen .hideOnMobileOverlay {
- display: none;
- }
gr-reply-dialog {
height: 100vh;
min-width: initial;
@@ -1419,7 +1414,7 @@
this.getEditMode()
);
return html` <div class="changeInfo">
- <div class="changeInfo-column changeMetadata hideOnMobileOverlay">
+ <div class="changeInfo-column changeMetadata">
<gr-change-metadata
id="metadata"
.change=${this.change}
@@ -1435,7 +1430,7 @@
</gr-change-metadata>
</div>
<div id="mainChangeInfo" class="changeInfo-column mainChangeInfo">
- <div id="commitAndRelated" class="hideOnMobileOverlay">
+ <div id="commitAndRelated">
<div class="commitContainer">
<h3 class="assistive-tech-only">Commit Message</h3>
<div>
@@ -1589,7 +1584,6 @@
</gr-file-list-header>
<gr-file-list
id="fileList"
- class="hideOnMobileOverlay"
.change=${this.change}
.changeNum=${this.changeNum}
.patchRange=${this.patchRange}
@@ -1695,7 +1689,6 @@
<section class="changeLog">
<h2 class="assistive-tech-only">Change Log</h2>
<gr-messages-list
- class="hideOnMobileOverlay"
.labels=${this.change?.labels}
.messages=${this.change?.messages}
.reviewerUpdates=${this.change?.reviewer_updates}
@@ -2238,11 +2231,9 @@
this.performPostLoadTasks();
});
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- this.initActiveTab();
- });
+ this.pluginLoader.awaitPluginsLoaded().then(() => {
+ this.initActiveTab();
+ });
}
private initActiveTab() {
@@ -2258,7 +2249,7 @@
// Private but used in tests.
sendShowChangeEvent() {
assertIsDefined(this.patchRange, 'patchRange');
- this.jsAPI.handleEvent(PluginEventType.SHOW_CHANGE, {
+ this.jsAPI.handleShowChange({
change: this.change,
patchNum: this.patchRange.patchNum,
info: {mergeable: this.mergeable},
@@ -2319,23 +2310,21 @@
// Private but used in tests.
maybeShowRevertDialog() {
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- if (
- !this.loggedIn ||
- !this.change ||
- this.change.status !== ChangeStatus.MERGED
- ) {
- // Do not display dialog if not logged-in or the change is not
- // merged.
- return;
- }
- if (this._getUrlParameter('revert')) {
- assertIsDefined(this.actions);
- this.actions.showRevertDialog();
- }
- });
+ this.pluginLoader.awaitPluginsLoaded().then(() => {
+ if (
+ !this.loggedIn ||
+ !this.change ||
+ this.change.status !== ChangeStatus.MERGED
+ ) {
+ // Do not display dialog if not logged-in or the change is not
+ // merged.
+ return;
+ }
+ if (this._getUrlParameter('revert')) {
+ assertIsDefined(this.actions);
+ this.actions.showRevertDialog();
+ }
+ });
}
private maybeShowReplyDialog() {
@@ -2644,7 +2633,7 @@
return;
}
this.handleLabelRemoved(oldLabels, newLabels);
- this.jsAPI.handleEvent(PluginEventType.LABEL_CHANGE, {
+ this.jsAPI.handleLabelChange({
change: this.change,
});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 2491610..301e44b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -20,8 +20,7 @@
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {_testOnly_resetEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
-import {EventType, PluginApi} from '../../../api/plugin';
+import {PluginApi} from '../../../api/plugin';
import {
mockPromise,
pressKey,
@@ -76,6 +75,7 @@
DetailedLabelInfo,
RepoName,
QuickLabelInfo,
+ PatchSetNumber,
} from '../../../types/common';
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
@@ -109,6 +109,7 @@
CommentsModel,
commentsModelToken,
} from '../../../models/comments/comments-model';
+import {getAppContext} from '../../../services/app-context';
suite('gr-change-view tests', () => {
let element: GrChangeView;
@@ -358,7 +359,6 @@
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
- getPluginLoader().loadPlugins([]);
window.Gerrit.install(
plugin => {
plugin.registerDynamicCustomComponent(
@@ -422,11 +422,11 @@
</div>
<h2 class="assistive-tech-only">Change metadata</h2>
<div class="changeInfo">
- <div class="changeInfo-column changeMetadata hideOnMobileOverlay">
+ <div class="changeInfo-column changeMetadata">
<gr-change-metadata id="metadata"> </gr-change-metadata>
</div>
<div class="changeInfo-column mainChangeInfo" id="mainChangeInfo">
- <div class="hideOnMobileOverlay" id="commitAndRelated">
+ <div id="commitAndRelated">
<div class="commitContainer">
<h3 class="assistive-tech-only">Commit Message</h3>
<div>
@@ -508,8 +508,7 @@
<section class="tabContent">
<div>
<gr-file-list-header id="fileListHeader"> </gr-file-list-header>
- <gr-file-list class="hideOnMobileOverlay" id="fileList">
- </gr-file-list>
+ <gr-file-list id="fileList"> </gr-file-list>
</div>
</section>
<gr-endpoint-decorator name="change-view-integration">
@@ -530,7 +529,7 @@
</paper-tabs>
<section class="changeLog">
<h2 class="assistive-tech-only">Change Log</h2>
- <gr-messages-list class="hideOnMobileOverlay"> </gr-messages-list>
+ <gr-messages-list> </gr-messages-list>
</section>
</div>
<gr-apply-fix-dialog id="applyFixDialog"> </gr-apply-fix-dialog>
@@ -1935,7 +1934,7 @@
test('revert dialog opened with revert param', async () => {
const awaitPluginsLoadedStub = sinon
- .stub(getPluginLoader(), 'awaitPluginsLoaded')
+ .stub(getAppContext().pluginLoader, 'awaitPluginsLoaded')
.callsFake(() => Promise.resolve());
element.patchRange = {
@@ -2229,13 +2228,12 @@
element.change = {...change};
element.patchRange = {patchNum: 4 as RevisionPatchSetNum};
element.mergeable = true;
- const showStub = sinon.stub(element.jsAPI, 'handleEvent');
+ const showStub = sinon.stub(element.jsAPI, 'handleShowChange');
element.sendShowChangeEvent();
assert.isTrue(showStub.calledOnce);
- assert.equal(showStub.lastCall.args[0], EventType.SHOW_CHANGE);
- assert.deepEqual(showStub.lastCall.args[1], {
+ assert.deepEqual(showStub.lastCall.args[0], {
change,
- patchNum: 4,
+ patchNum: 4 as PatchSetNumber,
info: {mergeable: true},
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
index 5f4835a..830f27a 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -170,7 +170,7 @@
`;
}
- private readonly jsAPI = getAppContext().jsApiService;
+ private readonly jsAPI = getAppContext().pluginLoader.jsApiService;
private computeIfSingleRevert() {
return this.revertType === RevertType.REVERT_SINGLE_CHANGE;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index ccc885e..c7d552d 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -22,7 +22,6 @@
import {diffFilePaths, pluralize} from '../../../utils/string-util';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAppContext} from '../../../services/app-context';
import {
DiffViewMode,
@@ -286,6 +285,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
private readonly getUserModel = resolve(this, userModelToken);
private readonly getChangeModel = resolve(this, changeModelToken);
@@ -823,55 +824,53 @@
override connectedCallback() {
super.connectedCallback();
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- this.dynamicHeaderEndpoints = getPluginEndpoints().getDynamicEndpoints(
- 'change-view-file-list-header'
+ this.pluginLoader.awaitPluginsLoaded().then(() => {
+ this.dynamicHeaderEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ 'change-view-file-list-header'
+ );
+ this.dynamicContentEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ 'change-view-file-list-content'
+ );
+ this.dynamicPrependedHeaderEndpoints =
+ getPluginEndpoints().getDynamicEndpoints(
+ 'change-view-file-list-header-prepend'
);
- this.dynamicContentEndpoints = getPluginEndpoints().getDynamicEndpoints(
- 'change-view-file-list-content'
+ this.dynamicPrependedContentEndpoints =
+ getPluginEndpoints().getDynamicEndpoints(
+ 'change-view-file-list-content-prepend'
);
- this.dynamicPrependedHeaderEndpoints =
- getPluginEndpoints().getDynamicEndpoints(
- 'change-view-file-list-header-prepend'
- );
- this.dynamicPrependedContentEndpoints =
- getPluginEndpoints().getDynamicEndpoints(
- 'change-view-file-list-content-prepend'
- );
- this.dynamicSummaryEndpoints = getPluginEndpoints().getDynamicEndpoints(
- 'change-view-file-list-summary'
- );
+ this.dynamicSummaryEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ 'change-view-file-list-summary'
+ );
- if (
- this.dynamicHeaderEndpoints.length !==
- this.dynamicContentEndpoints.length
- ) {
- this.reporting.error(
- 'Plugin change-view-file-list',
- new Error('dynamic header/content mismatch')
- );
- }
- if (
- this.dynamicPrependedHeaderEndpoints.length !==
- this.dynamicPrependedContentEndpoints.length
- ) {
- this.reporting.error(
- 'Plugin change-view-file-list',
- new Error('dynamic prepend header/content mismatch')
- );
- }
- if (
- this.dynamicHeaderEndpoints.length !==
- this.dynamicSummaryEndpoints.length
- ) {
- this.reporting.error(
- 'Plugin change-view-file-list',
- new Error('dynamic header/summary mismatch')
- );
- }
- });
+ if (
+ this.dynamicHeaderEndpoints.length !==
+ this.dynamicContentEndpoints.length
+ ) {
+ this.reporting.error(
+ 'Plugin change-view-file-list',
+ new Error('dynamic header/content mismatch')
+ );
+ }
+ if (
+ this.dynamicPrependedHeaderEndpoints.length !==
+ this.dynamicPrependedContentEndpoints.length
+ ) {
+ this.reporting.error(
+ 'Plugin change-view-file-list',
+ new Error('dynamic prepend header/content mismatch')
+ );
+ }
+ if (
+ this.dynamicHeaderEndpoints.length !==
+ this.dynamicSummaryEndpoints.length
+ ) {
+ this.reporting.error(
+ 'Plugin change-view-file-list',
+ new Error('dynamic header/summary mismatch')
+ );
+ }
+ });
this.diffCursor = new GrDiffCursor();
this.diffCursor.replaceDiffs(this.diffs);
}
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index dcc6039..6213506 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -7,6 +7,7 @@
import {SinonStubbedMember} from 'sinon';
import {PluginApi} from '../../../api/plugin';
import {ChangeStatus} from '../../../constants/constants';
+import {getAppContext} from '../../../services/app-context';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import '../../../test/common-test-setup';
import {
@@ -38,7 +39,6 @@
import {ParsedChangeInfo} from '../../../types/types';
import {getChangeNumber} from '../../../utils/change-util';
import {GrEndpointDecorator} from '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import './gr-related-changes-list';
import {
ChangeMarkersInList,
@@ -676,7 +676,7 @@
'0.1',
'http://some/plugins/url1.js'
);
- getPluginLoader().loadPlugins([]);
+ getAppContext().pluginLoader.loadPlugins([]);
await waitEventLoop();
assert.strictEqual(hookEl!.plugin, plugin!);
assert.strictEqual(hookEl!.change, element.change);
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
index 6fba4e4..4b07aea 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
@@ -11,7 +11,7 @@
stubRestApi,
waitEventLoop,
} from '../../../test/test-utils';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+
import {GrReplyDialog} from './gr-reply-dialog';
import {fixture, html, assert} from '@open-wc/testing';
import {
@@ -22,6 +22,7 @@
} from '../../../types/common';
import {createChange} from '../../../test/test-data-generators';
import {GrButton} from '../../shared/gr-button/gr-button';
+import {getAppContext} from '../../../services/app-context';
suite('gr-reply-dialog-it tests', () => {
let element: GrReplyDialog;
@@ -116,8 +117,9 @@
);
element = await fixture(html`<gr-reply-dialog></gr-reply-dialog>`);
setupElement(element);
- getPluginLoader().loadPlugins([]);
- await getPluginLoader().awaitPluginsLoaded();
+ const pluginLoader = getAppContext().pluginLoader;
+ pluginLoader.loadPlugins([]);
+ await pluginLoader.awaitPluginsLoaded();
await waitEventLoop();
await waitEventLoop();
const labelScoreRows = queryAndAssert(
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index ec5760b..b9b3b71 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -390,7 +390,7 @@
private readonly restApiService: RestApiService =
getAppContext().restApiService;
- private readonly jsAPI = getAppContext().jsApiService;
+ private readonly jsAPI = getAppContext().pluginLoader.jsApiService;
private readonly flagsService = getAppContext().flagsService;
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
index e663ae1..ae96a09 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
@@ -3,8 +3,6 @@
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-/* Import to get Gerrit interface */
-/* TODO(taoalpha): decouple gr-gerrit from gr-js-api-interface */
import '../gr-error-dialog/gr-error-dialog';
import '../../shared/gr-alert/gr-alert';
import '../../shared/gr-overlay/gr-overlay';
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
index 6c6d6a0..6f65c33 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
@@ -11,7 +11,6 @@
import '../gr-account-dropdown/gr-account-dropdown';
import '../gr-smart-search/gr-smart-search';
import {getBaseUrl, getDocsBaseUrl} from '../../../utils/url-util';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAdminLinks, NavLink} from '../../../utils/admin-nav-util';
import {
AccountDetailInfo,
@@ -141,7 +140,9 @@
private readonly restApiService = getAppContext().restApiService;
- private readonly jsAPI = getAppContext().jsApiService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
+ private readonly jsAPI = getAppContext().pluginLoader.jsApiService;
private readonly getUserModel = resolve(this, userModelToken);
@@ -571,7 +572,7 @@
return Promise.all([
this.restApiService.getAccount(),
this.restApiService.getTopMenus(),
- getPluginLoader().awaitPluginsLoaded(),
+ this.pluginLoader.awaitPluginsLoaded(),
]).then(result => {
const account = result[0];
this.account = account;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index d3428de..d053092 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -67,7 +67,6 @@
waitForEventOnce,
fire,
} from '../../../utils/event-util';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {assertIsDefined} from '../../../utils/common-util';
import {DiffContextExpandedEventDetail} from '../../../embed/diff/gr-diff-builder/gr-diff-builder';
import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer';
@@ -329,6 +328,8 @@
// visible for testing
readonly reporting = getAppContext().reportingService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
private readonly flags = getAppContext().flagsService;
private readonly restApiService = getAppContext().restApiService;
@@ -337,7 +338,7 @@
readonly getUserModel = resolve(this, userModelToken);
// visible for testing
- readonly jsAPI = getAppContext().jsApiService;
+ readonly jsAPI = getAppContext().pluginLoader.jsApiService;
// visible for testing
readonly syntaxLayer: GrSyntaxLayerWorker;
@@ -555,7 +556,7 @@
async initLayers() {
const preferencesPromise = this.restApiService.getPreferences();
- await getPluginLoader().awaitPluginsLoaded();
+ await this.pluginLoader.awaitPluginsLoaded();
const prefs = await preferencesPromise;
const enableTokenHighlight = !prefs?.disable_token_highlighting;
diff --git a/polygerrit-ui/app/elements/gr-app-global-var-init.ts b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
index 4132c0e..3deeb96 100644
--- a/polygerrit-ui/app/elements/gr-app-global-var-init.ts
+++ b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
@@ -13,11 +13,12 @@
import {GrAnnotation} from '../embed/diff/gr-diff-highlight/gr-annotation';
import {GrPluginActionContext} from './shared/gr-js-api-interface/gr-plugin-action-context';
-import {initGerritPluginApi} from './shared/gr-js-api-interface/gr-gerrit';
-import {AppContext} from '../services/app-context';
+import {AppContext, injectAppContext} from '../services/app-context';
+import {Finalizable} from '../services/registry';
-export function initGlobalVariables(appContext: AppContext) {
+export function initGlobalVariables(appContext: AppContext & Finalizable) {
+ injectAppContext(appContext);
window.GrAnnotation = GrAnnotation;
window.GrPluginActionContext = GrPluginActionContext;
- initGerritPluginApi(appContext);
+ window.Gerrit = appContext.pluginLoader;
}
diff --git a/polygerrit-ui/app/elements/gr-app.ts b/polygerrit-ui/app/elements/gr-app.ts
index 721b46d..ad017bb 100644
--- a/polygerrit-ui/app/elements/gr-app.ts
+++ b/polygerrit-ui/app/elements/gr-app.ts
@@ -43,14 +43,13 @@
initErrorReporter,
initWebVitals,
} from '../services/gr-reporting/gr-reporting_impl';
-import {injectAppContext} from '../services/app-context';
import {html, LitElement} from 'lit';
import {customElement} from 'lit/decorators.js';
import {ServiceWorkerInstaller} from '../services/service-worker-installer';
import {userModelToken} from '../models/user/user-model';
const appContext = createAppContext();
-injectAppContext(appContext);
+initGlobalVariables(appContext);
const reportingService = appContext.reportingService;
initVisibilityReporter(reportingService);
initPerformanceReporter(reportingService);
@@ -131,5 +130,3 @@
'gr-app': GrApp;
}
}
-
-initGlobalVariables(appContext);
diff --git a/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api.ts b/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api.ts
index da0035e..033df49 100644
--- a/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api.ts
@@ -5,7 +5,7 @@
*/
import {EventType, PluginApi} from '../../../api/plugin';
import {AdminPluginApi, MenuLink} from '../../../api/admin';
-import {getAppContext} from '../../../services/app-context';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
/**
* GrAdminApi class.
@@ -16,9 +16,10 @@
// TODO(TS): maybe define as enum if its a limited set
private menuLinks: MenuLink[] = [];
- private readonly reporting = getAppContext().reportingService;
-
- constructor(private readonly plugin: PluginApi) {
+ constructor(
+ private readonly reporting: ReportingService,
+ private readonly plugin: PluginApi
+ ) {
this.reporting.trackApi(this.plugin, 'admin', 'constructor');
this.plugin.on(EventType.ADMIN_MENU_LINKS, this);
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.ts b/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.ts
index da7aa9e..81f6013 100644
--- a/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.ts
@@ -6,9 +6,9 @@
import {assert} from '@open-wc/testing';
import {AdminPluginApi} from '../../../api/admin';
import {PluginApi} from '../../../api/plugin';
+import {getAppContext} from '../../../services/app-context';
import '../../../test/common-test-setup';
import '../../shared/gr-js-api-interface/gr-js-api-interface';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
suite('gr-admin-api tests', () => {
let adminApi: AdminPluginApi;
@@ -22,7 +22,7 @@
'0.1',
'http://test.com/plugins/testplugin/static/test.js'
);
- getPluginLoader().loadPlugins([]);
+ getAppContext().pluginLoader.loadPlugins([]);
adminApi = plugin.admin();
});
diff --git a/polygerrit-ui/app/elements/plugins/gr-attribute-helper/gr-attribute-helper.ts b/polygerrit-ui/app/elements/plugins/gr-attribute-helper/gr-attribute-helper.ts
index ab71255..bc4e701 100644
--- a/polygerrit-ui/app/elements/plugins/gr-attribute-helper/gr-attribute-helper.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-attribute-helper/gr-attribute-helper.ts
@@ -5,17 +5,19 @@
*/
import {AttributeHelperPluginApi} from '../../../api/attribute-helper';
import {PluginApi} from '../../../api/plugin';
-import {getAppContext} from '../../../services/app-context';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
export class GrAttributeHelper implements AttributeHelperPluginApi {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private readonly _promises = new Map<string, Promise<any>>();
- private readonly reporting = getAppContext().reportingService;
-
// TODO(TS): Change any to something more like HTMLElement.
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- constructor(readonly plugin: PluginApi, public element: any) {
+ constructor(
+ private readonly reporting: ReportingService,
+ readonly plugin: PluginApi,
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ public element: any
+ ) {
this.reporting.trackApi(this.plugin, 'attribute', 'constructor');
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
index 4d59dc9..51cddbe 100644
--- a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api.ts
@@ -11,7 +11,8 @@
CheckResult,
CheckRun,
} from '../../../api/checks';
-import {getAppContext} from '../../../services/app-context';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
+import {PluginsModel} from '../../../models/plugins/plugins-model';
const DEFAULT_CONFIG: ChecksApiConfig = {
fetchPollingIntervalSeconds: 60,
@@ -32,11 +33,11 @@
export class GrChecksApi implements ChecksPluginApi {
private state = State.NOT_REGISTERED;
- private readonly reporting = getAppContext().reportingService;
-
- private readonly pluginsModel = getAppContext().pluginsModel;
-
- constructor(readonly plugin: PluginApi) {
+ constructor(
+ private readonly reporting: ReportingService,
+ private readonly pluginsModel: PluginsModel,
+ readonly plugin: PluginApi
+ ) {
this.reporting.trackApi(this.plugin, 'checks', 'constructor');
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api_test.ts b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api_test.ts
index 011fbbf..8fc1a30 100644
--- a/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api_test.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-checks-api/gr-checks-api_test.ts
@@ -4,10 +4,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../../../test/common-test-setup';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {PluginApi} from '../../../api/plugin';
import {ChecksPluginApi} from '../../../api/checks';
import {assert} from '@open-wc/testing';
+import {getAppContext} from '../../../services/app-context';
suite('gr-settings-api tests', () => {
let checksApi: ChecksPluginApi | undefined;
@@ -21,7 +21,7 @@
'0.1',
'http://test.com/plugins/testplugin/static/test.js'
);
- getPluginLoader().loadPlugins([]);
+ getAppContext().pluginLoader.loadPlugins([]);
assert.isOk(pluginApi);
checksApi = pluginApi!.checks();
});
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
index 61279cf..0aecedd 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
@@ -9,7 +9,6 @@
getPluginEndpoints,
ModuleInfo,
} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {PluginApi} from '../../../api/plugin';
import {HookApi, PluginElement} from '../../../api/hook';
import {getAppContext} from '../../../services/app-context';
@@ -38,6 +37,8 @@
private readonly reporting = getAppContext().reportingService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
override render() {
return html`<slot></slot>`;
}
@@ -46,15 +47,13 @@
super.connectedCallback();
assertIsDefined(this.name);
getPluginEndpoints().onNewEndpoint(this.name, this.initModule);
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- assertIsDefined(this.name);
- const modules = getPluginEndpoints().getDetails(this.name);
- for (const module of modules) {
- this.initModule(module);
- }
- });
+ this.pluginLoader.awaitPluginsLoaded().then(() => {
+ assertIsDefined(this.name);
+ const modules = getPluginEndpoints().getDetails(this.name);
+ for (const module of modules) {
+ this.initModule(module);
+ }
+ });
}
override disconnectedCallback() {
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.ts b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.ts
index c3e6911..687b4aa 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.ts
@@ -13,7 +13,6 @@
queryAndAssert,
resetPlugins,
} from '../../../test/test-utils';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {GrEndpointDecorator} from './gr-endpoint-decorator';
import {PluginApi} from '../../../api/plugin';
import {GrEndpointParam} from '../gr-endpoint-param/gr-endpoint-param';
@@ -100,9 +99,6 @@
const replacementHookPromise = mockPromise();
replacementHook.onAttached(() => replacementHookPromise.resolve());
- // Mimic all plugins loaded.
- getPluginLoader().loadPlugins([]);
-
await decorationHookPromise;
await decorationHookSlotPromise;
await replacementHookPromise;
diff --git a/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts b/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts
index 9915d4c..641d87b 100644
--- a/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts
@@ -8,12 +8,14 @@
UnsubscribeCallback,
} from '../../../api/event-helper';
import {PluginApi} from '../../../api/plugin';
-import {getAppContext} from '../../../services/app-context';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
export class GrEventHelper implements EventHelperPluginApi {
- private readonly reporting = getAppContext().reportingService;
-
- constructor(readonly plugin: PluginApi, readonly element: HTMLElement) {
+ constructor(
+ private readonly reporting: ReportingService,
+ readonly plugin: PluginApi,
+ readonly element: HTMLElement
+ ) {
this.reporting.trackApi(this.plugin, 'event', 'constructor');
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.ts b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.ts
index 43d9805..bf1ba73 100644
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.ts
@@ -5,9 +5,9 @@
*/
import {updateStyles} from '@polymer/polymer/lib/mixins/element-mixin';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {LitElement, html, PropertyValues} from 'lit';
import {customElement, property} from 'lit/decorators.js';
+import {getAppContext} from '../../../services/app-context';
@customElement('gr-external-style')
export class GrExternalStyle extends LitElement {
@@ -20,6 +20,8 @@
stylesElements: HTMLElement[] = [];
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
override render() {
return html`<slot></slot>`;
}
@@ -29,9 +31,7 @@
// We remove all styles defined for different name.
this.removeStyles();
this.importAndApply();
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => this.importAndApply());
+ this.pluginLoader.awaitPluginsLoaded().then(() => this.importAndApply());
}
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.ts b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.ts
index ce87acb..ce998de 100644
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.ts
@@ -7,9 +7,9 @@
import {mockPromise, MockPromise, resetPlugins} from '../../../test/test-utils';
import './gr-external-style';
import {GrExternalStyle} from './gr-external-style';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {PluginApi} from '../../../api/plugin';
import {fixture, html, assert} from '@open-wc/testing';
+import {getAppContext} from '../../../services/app-context';
suite('gr-external-style integration tests', () => {
const TEST_URL = 'http://some.com/plugins/url.js';
@@ -60,7 +60,9 @@
setup(() => {
pluginsLoaded = mockPromise();
- sinon.stub(getPluginLoader(), 'awaitPluginsLoaded').returns(pluginsLoaded);
+ sinon
+ .stub(getAppContext().pluginLoader, 'awaitPluginsLoaded')
+ .returns(pluginsLoaded);
});
teardown(() => {
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
index 2add2bb..d8fa7eb 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
@@ -5,11 +5,11 @@
*/
import {LitElement} from 'lit';
import {customElement, state} from 'lit/decorators.js';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {ServerInfo} from '../../../types/common';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {configModelToken} from '../../../models/config/config-model';
+import {getAppContext} from '../../../services/app-context';
@customElement('gr-plugin-host')
export class GrPluginHost extends LitElement {
@@ -18,6 +18,8 @@
private readonly getConfigModel = resolve(this, configModelToken);
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
constructor() {
super();
subscribe(
@@ -30,7 +32,7 @@
? [config.default_theme]
: [];
const instanceId = config?.gerrit?.instance_id;
- getPluginLoader().loadPlugins([...themes, ...jsPlugins], instanceId);
+ this.pluginLoader.loadPlugins([...themes, ...jsPlugins], instanceId);
}
);
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
index a58314e..f235582 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
@@ -5,7 +5,6 @@
*/
import '../../../test/common-test-setup';
import './gr-plugin-host';
-import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {GrPluginHost} from './gr-plugin-host';
import {fixture, html, assert} from '@open-wc/testing';
import {SinonStub} from 'sinon';
@@ -15,6 +14,7 @@
configModelToken,
} from '../../../models/config/config-model';
import {testResolver} from '../../../test/common-test-setup';
+import {getAppContext} from '../../../services/app-context';
suite('gr-plugin-host tests', () => {
let element: GrPluginHost;
@@ -22,7 +22,7 @@
let configModel: ConfigModel;
setup(async () => {
- loadPluginsStub = sinon.stub(getPluginLoader(), 'loadPlugins');
+ loadPluginsStub = sinon.stub(getAppContext().pluginLoader, 'loadPlugins');
element = await fixture<GrPluginHost>(html`
<gr-plugin-host></gr-plugin-host>
`);
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
index b34724c..086d2f0 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
@@ -4,7 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {getBaseUrl} from '../../../utils/url-util';
-import {getPluginLoader} from '../gr-js-api-interface/gr-plugin-loader';
import {AccountInfo} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {LitElement, css, html} from 'lit';
@@ -26,6 +25,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly pluginLoader = getAppContext().pluginLoader;
+
static override get styles() {
return [
css`
@@ -54,7 +55,7 @@
super.connectedCallback();
Promise.all([
this.restApiService.getConfig(),
- getPluginLoader().awaitPluginsLoaded(),
+ this.pluginLoader.awaitPluginsLoaded(),
]).then(([cfg]) => {
this.hasAvatars = Boolean(cfg?.plugin?.has_avatars);
this.updateHostVisibilityAndImage();
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 af7c64f..4a05739 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -513,6 +513,13 @@
</gr-endpoint-param>
<gr-endpoint-param name="editing" .value=${this.editing}>
</gr-endpoint-param>
+ <gr-endpoint-param name="message" .value=${this.messageText}>
+ </gr-endpoint-param>
+ <gr-endpoint-param
+ name="isDraft"
+ .value=${isDraftOrUnsaved(this.comment)}
+ >
+ </gr-endpoint-param>
<div id="container" class=${classMap(classes)}>
${this.renderHeader()}
<div class="body">
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 4e8c70f..7155b75 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
@@ -102,6 +102,8 @@
<gr-endpoint-decorator name="comment">
<gr-endpoint-param name="comment"></gr-endpoint-param>
<gr-endpoint-param name="editing"></gr-endpoint-param>
+ <gr-endpoint-param name="message"></gr-endpoint-param>
+ <gr-endpoint-param name="isDraft"></gr-endpoint-param>
<div class="container" id="container">
<div class="header" id="header">
<div class="headerLeft">
@@ -138,6 +140,8 @@
<gr-endpoint-decorator name="comment">
<gr-endpoint-param name="comment"></gr-endpoint-param>
<gr-endpoint-param name="editing"></gr-endpoint-param>
+ <gr-endpoint-param name="message"></gr-endpoint-param>
+ <gr-endpoint-param name="isDraft"></gr-endpoint-param>
<div class="container" id="container">
<div class="header" id="header">
<div class="headerLeft">
@@ -176,6 +180,8 @@
<gr-endpoint-decorator name="comment">
<gr-endpoint-param name="comment"></gr-endpoint-param>
<gr-endpoint-param name="editing"></gr-endpoint-param>
+ <gr-endpoint-param name="message"></gr-endpoint-param>
+ <gr-endpoint-param name="isDraft"></gr-endpoint-param>
<div class="container" id="container">
<div class="header" id="header">
<div class="headerLeft">
@@ -268,6 +274,8 @@
<gr-endpoint-decorator name="comment">
<gr-endpoint-param name="comment"></gr-endpoint-param>
<gr-endpoint-param name="editing"></gr-endpoint-param>
+ <gr-endpoint-param name="message"></gr-endpoint-param>
+ <gr-endpoint-param name="isDraft"></gr-endpoint-param>
<div class="container draft" id="container">
<div class="header" id="header">
<div class="headerLeft">
@@ -343,6 +351,8 @@
<gr-endpoint-decorator name="comment">
<gr-endpoint-param name="comment"></gr-endpoint-param>
<gr-endpoint-param name="editing"></gr-endpoint-param>
+ <gr-endpoint-param name="message"></gr-endpoint-param>
+ <gr-endpoint-param name="isDraft"></gr-endpoint-param>
<div class="container draft" id="container">
<div class="header" id="header">
<div class="headerLeft">
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
index b8bfd21..b70daa8 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
@@ -65,7 +65,7 @@
private readonly reporting = getAppContext().reportingService;
- private readonly jsApiService = getAppContext().jsApiService;
+ private readonly jsApiService = getAppContext().pluginLoader.jsApiService;
constructor(public plugin: PluginApi, el?: GrChangeActionsElement) {
this.reporting.trackApi(this.plugin, 'actions', 'constructor');
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
index df9adc9..87ef61f 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
@@ -11,7 +11,6 @@
queryAndAssert,
resetPlugins,
} from '../../../test/test-utils';
-import {getPluginLoader} from './gr-plugin-loader';
import {GrChangeActions} from '../../change/gr-change-actions/gr-change-actions';
import {fixture, html, assert} from '@open-wc/testing';
import {PluginApi} from '../../../api/plugin';
@@ -24,6 +23,7 @@
import {ChangeViewChangeInfo} from '../../../types/common';
import {GrDropdown} from '../gr-dropdown/gr-dropdown';
import {GrIcon} from '../gr-icon/gr-icon';
+import {getAppContext} from '../../../services/app-context';
suite('gr-change-actions-js-api-interface tests', () => {
let element: GrChangeActions;
@@ -41,7 +41,7 @@
'http://test.com/plugins/testplugin/static/test.js'
);
// Mimic all plugins loaded.
- getPluginLoader().loadPlugins([]);
+ getAppContext().pluginLoader.loadPlugins([]);
changeActions = plugin.changeActions();
element = await fixture<GrChangeActions>(html`
<gr-change-actions></gr-change-actions>
@@ -76,7 +76,7 @@
);
changeActions = plugin.changeActions();
// Mimic all plugins loaded.
- getPluginLoader().loadPlugins([]);
+ getAppContext().pluginLoader.loadPlugins([]);
});
teardown(() => {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
deleted file mode 100644
index 4900ed5..0000000
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
+++ /dev/null
@@ -1,306 +0,0 @@
-/**
- * @license
- * Copyright 2019 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-
-/**
- * This defines the Gerrit instance. All methods directly attached to Gerrit
- * should be defined or linked here.
- */
-import {getPluginLoader, PluginOptionMap} from './gr-plugin-loader';
-import {send} from './gr-api-utils';
-import {getAppContext, AppContext} from '../../../services/app-context';
-import {PluginApi} from '../../../api/plugin';
-import {AuthService} from '../../../services/gr-auth/gr-auth';
-import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
-import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
-import {HttpMethod} from '../../../constants/constants';
-import {RequestPayload} from '../../../types/common';
-import {
- EventCallback,
- EventEmitterService,
-} from '../../../services/gr-event-interface/gr-event-interface';
-import {Gerrit} from '../../../api/gerrit';
-import {fontStyles} from '../../../styles/gr-font-styles';
-import {formStyles} from '../../../styles/gr-form-styles';
-import {menuPageStyles} from '../../../styles/gr-menu-page-styles';
-import {spinnerStyles} from '../../../styles/gr-spinner-styles';
-import {subpageStyles} from '../../../styles/gr-subpage-styles';
-import {tableStyles} from '../../../styles/gr-table-styles';
-import {assertIsDefined} from '../../../utils/common-util';
-import {iconStyles} from '../../../styles/gr-icon-styles';
-
-/**
- * These are the methods and properties that are exposed explicitly in the
- * public global `Gerrit` interface. In reality JavaScript plugins do depend
- * on some of this "internal" stuff. But we want to convert plugins to
- * TypeScript one by one and while doing that remove those dependencies.
- */
-export interface GerritInternal extends EventEmitterService, Gerrit {
- css(rule: string): string;
- install(
- callback: (plugin: PluginApi) => void,
- opt_version?: string,
- src?: string
- ): void;
- getLoggedIn(): Promise<boolean>;
- get(url: string, callback?: (response: unknown) => void): void;
- post(
- url: string,
- payload?: RequestPayload,
- callback?: (response: unknown) => void
- ): void;
- put(
- url: string,
- payload?: RequestPayload,
- callback?: (response: unknown) => void
- ): void;
- delete(url: string, callback?: (response: unknown) => void): void;
- isPluginLoaded(pathOrUrl: string): boolean;
- awaitPluginsLoaded(): Promise<unknown>;
- _loadPlugins(plugins: string[], opts: PluginOptionMap): void;
- _arePluginsLoaded(): boolean;
- _isPluginEnabled(pathOrUrl: string): boolean;
- _isPluginLoaded(pathOrUrl: string): boolean;
- _customStyleSheet?: CSSStyleSheet;
-
- // exposed methods
- Auth: AuthService;
-}
-
-export function initGerritPluginApi(appContext: AppContext) {
- window.Gerrit = window.Gerrit ?? new GerritImpl(appContext);
-}
-
-export function _testOnly_getGerritInternalPluginApi(): GerritInternal {
- if (!window.Gerrit) throw new Error('initGerritPluginApi was not called');
- return window.Gerrit as GerritInternal;
-}
-
-export function deprecatedDelete(
- url: string,
- callback?: (response: Response) => void
-) {
- console.warn('.delete() is deprecated! Use plugin.restApi().delete()');
- return getAppContext()
- .restApiService.send(HttpMethod.DELETE, url)
- .then(response => {
- if (response.status !== 204) {
- return response.text().then(text => {
- if (text) {
- return Promise.reject(new Error(text));
- } else {
- return Promise.reject(new Error(`${response.status}`));
- }
- });
- }
- if (callback) callback(response);
- return response;
- });
-}
-
-const fakeApi = {
- getPluginName: () => 'global',
-};
-
-/**
- * TODO(brohlfs): Reduce this step by step until it only contains install().
- */
-class GerritImpl implements GerritInternal {
- _customStyleSheet?: CSSStyleSheet;
-
- public readonly Auth: AuthService;
-
- private readonly reportingService: ReportingService;
-
- private readonly eventEmitter: EventEmitterService;
-
- private readonly restApiService: RestApiService;
-
- public readonly styles = {
- font: fontStyles,
- form: formStyles,
- icon: iconStyles,
- menuPage: menuPageStyles,
- spinner: spinnerStyles,
- subPage: subpageStyles,
- table: tableStyles,
- };
-
- constructor(appContext: AppContext) {
- this.Auth = appContext.authService;
- this.reportingService = appContext.reportingService;
- this.eventEmitter = appContext.eventEmitter;
- this.restApiService = appContext.restApiService;
- assertIsDefined(this.reportingService, 'reportingService');
- assertIsDefined(this.eventEmitter, 'eventEmitter');
- assertIsDefined(this.restApiService, 'restApiService');
- }
-
- finalize() {}
-
- /**
- * @deprecated Use plugin.styles().css(rulesStr) instead. Please, consult
- * the documentation how to replace it accordingly.
- */
- css(rulesStr: string) {
- this.reportingService.trackApi(fakeApi, 'global', 'css');
- console.warn(
- 'Gerrit.css(rulesStr) is deprecated!',
- 'Use plugin.styles().css(rulesStr)'
- );
- if (!this._customStyleSheet) {
- const styleEl = document.createElement('style');
- document.head.appendChild(styleEl);
- this._customStyleSheet = styleEl.sheet!;
- }
-
- const name = `__pg_js_api_class_${this._customStyleSheet.cssRules.length}`;
- this._customStyleSheet.insertRule('.' + name + '{' + rulesStr + '}', 0);
- return name;
- }
-
- install(
- callback: (plugin: PluginApi) => void,
- version?: string,
- src?: string
- ) {
- getPluginLoader().install(callback, version, src);
- }
-
- getLoggedIn() {
- this.reportingService.trackApi(fakeApi, 'global', 'getLoggedIn');
- console.warn(
- 'Gerrit.getLoggedIn() is deprecated! ' +
- 'Use plugin.restApi().getLoggedIn()'
- );
- return this.restApiService.getLoggedIn();
- }
-
- get(url: string, callback?: (response: unknown) => void) {
- this.reportingService.trackApi(fakeApi, 'global', 'get');
- console.warn('.get() is deprecated! Use plugin.restApi().get()');
- send(this.restApiService, HttpMethod.GET, url, callback);
- }
-
- post(
- url: string,
- payload?: RequestPayload,
- callback?: (response: unknown) => void
- ) {
- this.reportingService.trackApi(fakeApi, 'global', 'post');
- console.warn('.post() is deprecated! Use plugin.restApi().post()');
- send(this.restApiService, HttpMethod.POST, url, callback, payload);
- }
-
- put(
- url: string,
- payload?: RequestPayload,
- callback?: (response: unknown) => void
- ) {
- this.reportingService.trackApi(fakeApi, 'global', 'put');
- console.warn('.put() is deprecated! Use plugin.restApi().put()');
- send(this.restApiService, HttpMethod.PUT, url, callback, payload);
- }
-
- delete(url: string, callback?: (response: Response) => void) {
- this.reportingService.trackApi(fakeApi, 'global', 'delete');
- deprecatedDelete(url, callback);
- }
-
- awaitPluginsLoaded() {
- this.reportingService.trackApi(fakeApi, 'global', 'awaitPluginsLoaded');
- return getPluginLoader().awaitPluginsLoaded();
- }
-
- // TODO(taoalpha): consider removing these proxy methods
- // and using getPluginLoader() directly
- _loadPlugins(plugins: string[] = []) {
- this.reportingService.trackApi(fakeApi, 'global', '_loadPlugins');
- getPluginLoader().loadPlugins(plugins);
- }
-
- _arePluginsLoaded() {
- this.reportingService.trackApi(fakeApi, 'global', '_arePluginsLoaded');
- return getPluginLoader().arePluginsLoaded();
- }
-
- _isPluginEnabled(pathOrUrl: string) {
- this.reportingService.trackApi(fakeApi, 'global', '_isPluginEnabled');
- return getPluginLoader().isPluginEnabled(pathOrUrl);
- }
-
- isPluginLoaded(pathOrUrl: string) {
- return this._isPluginLoaded(pathOrUrl);
- }
-
- _isPluginLoaded(pathOrUrl: string) {
- this.reportingService.trackApi(fakeApi, 'global', '_isPluginLoaded');
- return getPluginLoader().isPluginLoaded(pathOrUrl);
- }
-
- /**
- * Enabling EventEmitter interface on Gerrit.
- *
- * This will enable to signal across different parts of js code without relying on DOM,
- * including core to core, plugin to plugin and also core to plugin.
- *
- * @example
- *
- * // Emit this event from pluginA
- * Gerrit.install(pluginA => {
- * fetch("some-api").then(() => {
- * Gerrit.on("your-special-event", {plugin: pluginA});
- * });
- * });
- *
- * // Listen on your-special-event from pluginB
- * Gerrit.install(pluginB => {
- * Gerrit.on("your-special-event", ({plugin}) => {
- * // do something, plugin is pluginA
- * });
- * });
- */
- addListener(eventName: string, cb: EventCallback) {
- this.reportingService.trackApi(fakeApi, 'global', 'addListener');
- return this.eventEmitter.addListener(eventName, cb);
- }
-
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- dispatch(eventName: string, detail: any) {
- this.reportingService.trackApi(fakeApi, 'global', 'dispatch');
- return this.eventEmitter.dispatch(eventName, detail);
- }
-
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- emit(eventName: string, detail: any) {
- this.reportingService.trackApi(fakeApi, 'global', 'emit');
- return this.eventEmitter.emit(eventName, detail);
- }
-
- off(eventName: string, cb: EventCallback) {
- this.reportingService.trackApi(fakeApi, 'global', 'off');
- this.eventEmitter.off(eventName, cb);
- }
-
- on(eventName: string, cb: EventCallback) {
- this.reportingService.trackApi(fakeApi, 'global', 'on');
- return this.eventEmitter.on(eventName, cb);
- }
-
- once(eventName: string, cb: EventCallback) {
- this.reportingService.trackApi(fakeApi, 'global', 'once');
- return this.eventEmitter.once(eventName, cb);
- }
-
- removeAllListeners(eventName: string) {
- this.reportingService.trackApi(fakeApi, 'global', 'removeAllListeners');
- this.eventEmitter.removeAllListeners(eventName);
- }
-
- removeListener(eventName: string, cb: EventCallback) {
- this.reportingService.trackApi(fakeApi, 'global', 'removeListener');
- this.eventEmitter.removeListener(eventName, cb);
- }
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.ts
deleted file mode 100644
index b906891..0000000
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.ts
+++ /dev/null
@@ -1,63 +0,0 @@
-/**
- * @license
- * Copyright 2019 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../test/common-test-setup';
-import {getPluginLoader} from './gr-plugin-loader';
-import {resetPlugins} from '../../../test/test-utils';
-import {
- GerritInternal,
- _testOnly_getGerritInternalPluginApi,
-} from './gr-gerrit';
-import {stubRestApi} from '../../../test/test-utils';
-import {getAppContext} from '../../../services/app-context';
-import {GrJsApiInterface} from './gr-js-api-interface-element';
-import {SinonFakeTimers} from 'sinon';
-import {Timestamp} from '../../../api/rest-api';
-import {assert} from '@open-wc/testing';
-
-suite('gr-gerrit tests', () => {
- let element: GrJsApiInterface;
- let clock: SinonFakeTimers;
- let pluginApi: GerritInternal;
-
- setup(() => {
- clock = sinon.useFakeTimers();
-
- stubRestApi('getAccount').returns(
- Promise.resolve({name: 'Judy Hopps', registered_on: '' as Timestamp})
- );
- stubRestApi('send').returns(
- Promise.resolve({...new Response(), status: 200})
- );
- element = getAppContext().jsApiService as GrJsApiInterface;
- pluginApi = _testOnly_getGerritInternalPluginApi();
- });
-
- teardown(() => {
- clock.restore();
- element._removeEventCallbacks();
- resetPlugins();
- });
-
- suite('proxy methods', () => {
- test('Gerrit._isPluginEnabled proxy to getPluginLoader()', () => {
- const stubFn = sinon.stub();
- sinon
- .stub(getPluginLoader(), 'isPluginEnabled')
- .callsFake((...args) => stubFn(...args));
- pluginApi._isPluginEnabled('test_plugin');
- assert.isTrue(stubFn.calledWith('test_plugin'));
- });
-
- test('Gerrit._isPluginLoaded proxy to getPluginLoader()', () => {
- const stubFn = sinon.stub();
- sinon
- .stub(getPluginLoader(), 'isPluginLoaded')
- .callsFake((...args) => stubFn(...args));
- pluginApi._isPluginLoaded('test_plugin');
- assert.isTrue(stubFn.calledWith('test_plugin'));
- });
- });
-});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
index 6142aa2..5a42656 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
@@ -3,7 +3,6 @@
* Copyright 2020 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {getPluginLoader} from './gr-plugin-loader';
import {hasOwnProperty} from '../../../utils/common-util';
import {
ChangeInfo,
@@ -24,40 +23,20 @@
import {MenuLink} from '../../../api/admin';
import {Finalizable} from '../../../services/registry';
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
+import {getAppContext} from '../../../services/app-context';
+import {Provider} from '../../../models/dependency';
const elements: {[key: string]: HTMLElement} = {};
const eventCallbacks: {[key: string]: EventCallback[]} = {};
export class GrJsApiInterface implements JsApiService, Finalizable {
- constructor(readonly reporting: ReportingService) {}
+ constructor(
+ private waitForPluginsToLoad: Provider<Promise<void>>,
+ readonly reporting: ReportingService
+ ) {}
finalize() {}
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- handleEvent(type: EventType, detail: any) {
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- switch (type) {
- case EventType.SHOW_CHANGE:
- this._handleShowChange(detail);
- break;
- case EventType.LABEL_CHANGE:
- this._handleLabelChange(detail);
- break;
- case EventType.SHOW_REVISION_ACTIONS:
- this._handleShowRevisionActions(detail);
- break;
- default:
- console.warn(
- 'handleEvent called with unsupported event type:',
- type
- );
- break;
- }
- });
- }
-
addElement(key: TargetElement, el: HTMLElement) {
elements[key] = el;
}
@@ -98,7 +77,8 @@
}
}
- _handleShowChange(detail: ShowChangeDetail) {
+ async handleShowChange(detail: ShowChangeDetail) {
+ await this.waitForPluginsToLoad();
// Note (issue 8221) Shallow clone the change object and add a mergeable
// getter with deprecation warning. This makes the change detail appear as
// though SKIP_MERGEABLE was not set, so that plugins that expect it can
@@ -143,7 +123,8 @@
}
}
- _handleShowRevisionActions(detail: ShowRevisionActionsDetail) {
+ async handleShowRevisionActions(detail: ShowRevisionActionsDetail) {
+ await this.waitForPluginsToLoad();
const registeredCallbacks = this._getEventCallbacks(
EventType.SHOW_REVISION_ACTIONS
);
@@ -174,7 +155,8 @@
}
}
- _handleLabelChange(detail: {change: ChangeInfo}) {
+ async handleLabelChange(detail: {change?: ParsedChangeInfo}) {
+ await this.waitForPluginsToLoad();
for (const cb of this._getEventCallbacks(EventType.LABEL_CHANGE)) {
try {
cb(detail.change);
@@ -267,8 +249,8 @@
* will resolve to null.
*/
getCoverageAnnotationApis(): Promise<GrAnnotationActionsInterface[]> {
- return getPluginLoader()
- .awaitPluginsLoaded()
+ return getAppContext()
+ .pluginLoader.awaitPluginsLoaded()
.then(() => {
const providers: GrAnnotationActionsInterface[] = [];
this._getEventCallbacks(EventType.ANNOTATE_DIFF).forEach(cb => {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.ts
index 240bc0b..2ec4f27 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.ts
@@ -5,4 +5,3 @@
*/
import './gr-js-api-interface-element';
import './gr-public-js-api';
-import './gr-gerrit';
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
index 7c09bad..a6b9dee 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
@@ -8,7 +8,6 @@
import {GrPopupInterface} from '../../plugins/gr-popup-interface/gr-popup-interface';
import {EventType} from '../../../api/plugin';
import {PLUGIN_LOADING_TIMEOUT_MS} from './gr-api-utils';
-import {getPluginLoader} from './gr-plugin-loader';
import {
stubRestApi,
stubBaseUrl,
@@ -34,11 +33,11 @@
stubRestApi('getAccount').returns(Promise.resolve({name: 'Judy Hopps'}));
sendStub = stubRestApi('send').returns(Promise.resolve({status: 200}));
- element = getAppContext().jsApiService;
+ element = getAppContext().pluginLoader.jsApiService;
errorStub = sinon.stub(element.reporting, 'error');
window.Gerrit.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
- getPluginLoader().loadPlugins([]);
+ getAppContext().pluginLoader.loadPlugins([]);
});
teardown(() => {
@@ -81,8 +80,9 @@
plugin.on(EventType.SHOW_CHANGE, (change, revision, info) => {
resolve({change, revision, info});
});
- element.handleEvent(EventType.SHOW_CHANGE,
- {change: testChange, patchNum: 1, info: {mergeable: false}});
+ element.handleShowChange(
+ {change: testChange, patchNum: 1, info: {mergeable: false}}
+ );
const {change, revision, info} = await promise;
assert.deepEqual(change, expectedChange);
@@ -102,8 +102,9 @@
plugin.on(EventType.SHOW_REVISION_ACTIONS, (actions, change) => {
resolve({change, actions});
});
- element.handleEvent(EventType.SHOW_REVISION_ACTIONS,
- {change: testChange, revisionActions: {test: {}}});
+ element.handleShowRevisionActions(
+ {change: testChange, revisionActions: {test: {}}}
+ );
const {change, actions} = await promise;
assert.deepEqual(change, testChange);
@@ -111,16 +112,15 @@
assert.isTrue(errorStub.calledOnce);
});
- test('handleEvent awaits plugins load', async () => {
+ test('handleShowChange awaits plugins load', async () => {
const testChange = {
_number: 42,
revisions: {def: {_number: 2}, abc: {_number: 1}},
};
const spy = sinon.spy();
- getPluginLoader().loadPlugins(['plugins/test.js']);
+ getAppContext().pluginLoader.loadPlugins(['plugins/test.js']);
plugin.on(EventType.SHOW_CHANGE, spy);
- element.handleEvent(EventType.SHOW_CHANGE,
- {change: testChange, patchNum: 1});
+ element.handleShowChange({change: testChange, patchNum: 1});
assert.isFalse(spy.called);
// Timeout on loading plugins
@@ -201,7 +201,7 @@
const testChange = {_number: 42};
plugin.on(EventType.LABEL_CHANGE, throwErrFn);
plugin.on(EventType.LABEL_CHANGE, resolve);
- element.handleEvent(EventType.LABEL_CHANGE, {change: testChange});
+ element.handleLabelChange({change: testChange});
const change = await promise;
assert.deepEqual(change, testChange);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
index 7aad2f0..c7a5eae 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
@@ -17,14 +17,14 @@
import {MenuLink} from '../../../api/admin';
export interface ShowChangeDetail {
- change: ChangeInfo;
- patchNum: PatchSetNum;
- info: {mergeable: boolean};
+ change?: ParsedChangeInfo;
+ patchNum?: PatchSetNum;
+ info: {mergeable: boolean | null};
}
export interface ShowRevisionActionsDetail {
change: ChangeInfo;
- revisionActions: {[key: string]: ActionInfo};
+ revisionActions: {[key: string]: ActionInfo | undefined};
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -38,8 +38,9 @@
revertSubmissionMsg: string,
origMsg: string
): string;
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- handleEvent(eventName: EventType, detail: any): void;
+ handleShowChange(detail: ShowChangeDetail): void;
+ handleShowRevisionActions(detail: ShowRevisionActionsDetail): void;
+ handleLabelChange(detail: {change?: ParsedChangeInfo}): void;
modifyRevertMsg(
change: ChangeInfo,
revertMsg: string,
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
index 4a90314..521a0a0 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
@@ -3,7 +3,6 @@
* Copyright 2019 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {getAppContext} from '../../../services/app-context';
import {
PLUGIN_LOADING_TIMEOUT_MS,
getPluginNameFromUrl,
@@ -16,6 +15,19 @@
import {PluginApi} from '../../../api/plugin';
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
import {fireAlert} from '../../../utils/event-util';
+import {JsApiService} from './gr-js-api-types';
+import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
+import {Finalizable} from '../../../services/registry';
+import {PluginsModel} from '../../../models/plugins/plugins-model';
+import {Gerrit} from '../../../api/gerrit';
+import {fontStyles} from '../../../styles/gr-font-styles';
+import {formStyles} from '../../../styles/gr-form-styles';
+import {menuPageStyles} from '../../../styles/gr-menu-page-styles';
+import {spinnerStyles} from '../../../styles/gr-spinner-styles';
+import {subpageStyles} from '../../../styles/gr-subpage-styles';
+import {tableStyles} from '../../../styles/gr-table-styles';
+import {iconStyles} from '../../../styles/gr-icon-styles';
+import {GrJsApiInterface} from './gr-js-api-interface-element';
enum PluginState {
/** State that indicates the plugin is pending to be loaded. */
@@ -64,32 +76,58 @@
* Retrieve plugin.
* Check plugin status and if all plugins loaded.
*/
-export class PluginLoader {
- _pluginListLoaded = false;
+export class PluginLoader implements Gerrit, Finalizable {
+ public readonly styles = {
+ font: fontStyles,
+ form: formStyles,
+ icon: iconStyles,
+ menuPage: menuPageStyles,
+ spinner: spinnerStyles,
+ subPage: subpageStyles,
+ table: tableStyles,
+ };
- _plugins = new Map<string, PluginObject>();
+ private pluginListLoaded = false;
- _reporting: ReportingService | null = null;
+ private plugins = new Map<string, PluginObject>();
// Promise that resolves when all plugins loaded
- _loadingPromise: Promise<void> | null = null;
+ private loadingPromise: Promise<void> | null = null;
- // Resolver to resolve _loadingPromise once all plugins loaded
- _loadingResolver: (() => void) | null = null;
+ // Resolver to resolve loadingPromise once all plugins loaded
+ private loadingResolver: (() => void) | null = null;
private instanceId?: string;
- _getReporting() {
- if (!this._reporting) {
- this._reporting = getAppContext().reportingService;
- }
- return this._reporting;
+ public readonly jsApiService: JsApiService;
+
+ constructor(
+ private readonly reportingService: ReportingService,
+ private readonly restApiService: RestApiService,
+ private readonly pluginsModel: PluginsModel
+ ) {
+ this.jsApiService = new GrJsApiInterface(
+ () => this.awaitPluginsLoaded(),
+ this.reportingService
+ );
+ }
+
+ reset() {
+ this.pluginListLoaded = false;
+ this.plugins = new Map<string, PluginObject>();
+ this.loadingPromise = null;
+ this.loadingResolver = null;
+ this.instanceId = undefined;
+ }
+
+ finalize() {
+ this.reset();
}
/**
* Use the plugin name or use the full url if not recognized.
*/
- _getPluginKeyFromUrl(url: string) {
+ private getPluginKeyFromUrl(url: string) {
return getPluginNameFromUrl(url) || `${UNKNOWN_PLUGIN_PREFIX}${url}`;
}
@@ -98,41 +136,41 @@
*/
loadPlugins(plugins: string[] = [], instanceId?: string) {
this.instanceId = instanceId;
- this._pluginListLoaded = true;
+ this.pluginListLoaded = true;
plugins.forEach(path => {
- const url = this._urlFor(path, window.ASSETS_PATH);
- const pluginKey = this._getPluginKeyFromUrl(url);
+ const url = this.urlFor(path, window.ASSETS_PATH);
+ const pluginKey = this.getPluginKeyFromUrl(url);
// Skip if already installed.
- if (this._plugins.has(pluginKey)) return;
- this._plugins.set(pluginKey, {
+ if (this.plugins.has(pluginKey)) return;
+ this.plugins.set(pluginKey, {
name: pluginKey,
url,
state: PluginState.PENDING,
plugin: null,
});
- if (this._isPathEndsWith(url, '.js')) {
- this._loadJsPlugin(path);
+ if (this.isPathEndsWith(url, '.js')) {
+ this.loadJsPlugin(path);
} else {
- this._failToLoad(`Unrecognized plugin path ${path}`, path);
+ this.failToLoad(`Unrecognized plugin path ${path}`, path);
}
});
this.awaitPluginsLoaded().then(() => {
const loaded = this.getPluginsByState(PluginState.LOADED);
const failed = this.getPluginsByState(PluginState.LOAD_FAILED);
- this._getReporting().pluginsLoaded(loaded.map(p => p.name));
- this._getReporting().pluginsFailed(failed.map(p => p.name));
+ this.reportingService.pluginsLoaded(loaded.map(p => p.name));
+ this.reportingService.pluginsFailed(failed.map(p => p.name));
});
}
- _isPathEndsWith(url: string | URL, suffix: string) {
+ private isPathEndsWith(url: string | URL, suffix: string) {
if (!(url instanceof URL)) {
try {
url = new URL(url);
} catch (e: unknown) {
- this._getReporting().error(
+ this.reportingService.error(
'GrPluginLoader',
new Error('url parse error'),
e
@@ -145,7 +183,7 @@
}
private getPluginsByState(state: PluginState) {
- return [...this._plugins.values()].filter(p => p.state === state);
+ return [...this.plugins.values()].filter(p => p.state === state);
}
install(
@@ -163,31 +201,37 @@
src = script && script.baseURI;
}
if (!src) {
- this._failToLoad('Failed to determine src.');
+ this.failToLoad('Failed to determine src.');
return;
}
if (version && version !== API_VERSION) {
- this._failToLoad(
+ this.failToLoad(
`Plugin ${src} install error: only version ${API_VERSION} is supported in PolyGerrit. ${version} was given.`,
src
);
return;
}
- const url = this._urlFor(src);
+ const url = this.urlFor(src);
const pluginObject = this.getPlugin(url);
let plugin = pluginObject && pluginObject.plugin;
if (!plugin) {
- plugin = new Plugin(url);
+ plugin = new Plugin(
+ url,
+ this.jsApiService,
+ this.reportingService,
+ this.restApiService,
+ this.pluginsModel
+ );
}
try {
callback(plugin);
- this._pluginInstalled(url, plugin);
+ this.pluginInstalled(url, plugin);
} catch (e: unknown) {
if (e instanceof Error) {
- this._failToLoad(`${e.name}: ${e.message}`, src);
+ this.failToLoad(`${e.name}: ${e.message}`, src);
} else {
- this._getReporting().error(
+ this.reportingService.error(
'GrPluginLoader',
new Error('plugin callback error'),
e
@@ -197,27 +241,27 @@
}
arePluginsLoaded() {
- if (!this._pluginListLoaded) return false;
+ if (!this.pluginListLoaded) return false;
return this.getPluginsByState(PluginState.PENDING).length === 0;
}
- _checkIfCompleted() {
+ private checkIfCompleted() {
if (this.arePluginsLoaded()) {
getPluginEndpoints().setPluginsReady();
- if (this._loadingResolver) {
- this._loadingResolver();
- this._loadingResolver = null;
- this._loadingPromise = null;
+ if (this.loadingResolver) {
+ this.loadingResolver();
+ this.loadingResolver = null;
+ this.loadingPromise = null;
}
}
}
- _timeout() {
+ private timeout() {
const pending = this.getPluginsByState(PluginState.PENDING);
for (const plugin of pending) {
- this._updatePluginState(plugin.url, PluginState.LOAD_FAILED);
+ this.updatePluginState(plugin.url, PluginState.LOAD_FAILED);
}
- this._checkIfCompleted();
+ this.checkIfCompleted();
const errorMessage = `Timeout when loading plugins: ${pending
.map(p => p.name)
.join(',')}`;
@@ -225,21 +269,25 @@
return errorMessage;
}
- _failToLoad(message: string, pluginUrl?: string) {
+ // Private but mocked in tests.
+ failToLoad(message: string, pluginUrl?: string) {
// Show an alert with the error
fireAlert(document, `Plugin install error: ${message} from ${pluginUrl}`);
- if (pluginUrl) this._updatePluginState(pluginUrl, PluginState.LOAD_FAILED);
- this._checkIfCompleted();
+ if (pluginUrl) this.updatePluginState(pluginUrl, PluginState.LOAD_FAILED);
+ this.checkIfCompleted();
}
- _updatePluginState(pluginUrl: string, state: PluginState): PluginObject {
- const key = this._getPluginKeyFromUrl(pluginUrl);
- if (this._plugins.has(key)) {
- this._plugins.get(key)!.state = state;
+ private updatePluginState(
+ pluginUrl: string,
+ state: PluginState
+ ): PluginObject {
+ const key = this.getPluginKeyFromUrl(pluginUrl);
+ if (this.plugins.has(key)) {
+ this.plugins.get(key)!.state = state;
} else {
// Plugin is not recorded for some reason.
console.info(`Plugin loaded separately: ${pluginUrl}`);
- this._plugins.set(key, {
+ this.plugins.set(key, {
name: key,
url: pluginUrl,
state,
@@ -247,59 +295,61 @@
});
}
console.debug(`Plugin ${key} ${state}`);
- return this._plugins.get(key)!;
+ return this.plugins.get(key)!;
}
- _pluginInstalled(url: string, plugin: PluginApi) {
- const pluginObj = this._updatePluginState(url, PluginState.LOADED);
+ private pluginInstalled(url: string, plugin: PluginApi) {
+ const pluginObj = this.updatePluginState(url, PluginState.LOADED);
pluginObj.plugin = plugin;
- this._getReporting().pluginLoaded(plugin.getPluginName() || url);
- this._checkIfCompleted();
+ this.reportingService.pluginLoaded(plugin.getPluginName() || url);
+ this.checkIfCompleted();
}
/**
* Checks if given plugin path/url is enabled or not.
*/
isPluginEnabled(pathOrUrl: string) {
- const url = this._urlFor(pathOrUrl);
- const key = this._getPluginKeyFromUrl(url);
- return this._plugins.has(key);
+ const url = this.urlFor(pathOrUrl);
+ const key = this.getPluginKeyFromUrl(url);
+ return this.plugins.has(key);
}
/**
* Returns the plugin object with a given url.
*/
getPlugin(pathOrUrl: string) {
- const url = this._urlFor(pathOrUrl);
- const key = this._getPluginKeyFromUrl(url);
- return this._plugins.get(key);
+ const url = this.urlFor(pathOrUrl);
+ const key = this.getPluginKeyFromUrl(url);
+ return this.plugins.get(key);
}
/**
* Checks if given plugin path/url is loaded or not.
*/
isPluginLoaded(pathOrUrl: string): boolean {
- const url = this._urlFor(pathOrUrl);
- const key = this._getPluginKeyFromUrl(url);
- return this._plugins.has(key)
- ? this._plugins.get(key)!.state === PluginState.LOADED
+ const url = this.urlFor(pathOrUrl);
+ const key = this.getPluginKeyFromUrl(url);
+ return this.plugins.has(key)
+ ? this.plugins.get(key)!.state === PluginState.LOADED
: false;
}
- _loadJsPlugin(pluginUrl: string) {
- const urlWithAP = this._urlFor(pluginUrl, window.ASSETS_PATH);
- const urlWithoutAP = this._urlFor(pluginUrl);
+ // Private but mocked in tests.
+ loadJsPlugin(pluginUrl: string) {
+ const urlWithAP = this.urlFor(pluginUrl, window.ASSETS_PATH);
+ const urlWithoutAP = this.urlFor(pluginUrl);
let onerror = undefined;
if (urlWithAP !== urlWithoutAP) {
- onerror = () => this._createScriptTag(urlWithoutAP);
+ onerror = () => this.createScriptTag(urlWithoutAP);
}
- this._createScriptTag(urlWithAP, onerror);
+ this.createScriptTag(urlWithAP, onerror);
}
- _createScriptTag(url: string, onerror?: OnErrorEventHandler) {
+ // Private but mocked in tests.
+ createScriptTag(url: string, onerror?: OnErrorEventHandler) {
if (!onerror) {
- onerror = () => this._failToLoad(`${url} load error`, url);
+ onerror = () => this.failToLoad(`${url} load error`, url);
}
const el = document.createElement('script');
@@ -313,7 +363,7 @@
return document.body.appendChild(el);
}
- _urlFor(pathOrUrl: string, assetsPath?: string): string {
+ private urlFor(pathOrUrl: string, assetsPath?: string): string {
if (isThemeFile(pathOrUrl)) {
if (assetsPath && this.instanceId) {
return `${assetsPath}/hosts/${this.instanceId}${THEME_JS}`;
@@ -341,39 +391,28 @@
awaitPluginsLoaded() {
// Resolve if completed.
- this._checkIfCompleted();
+ this.checkIfCompleted();
if (this.arePluginsLoaded()) {
return Promise.resolve();
}
- if (!this._loadingPromise) {
+ if (!this.loadingPromise) {
// specify window here so that TS pulls the correct setTimeout method
// if window is not specified, then the function is pulled from node
// and the return type is NodeJS.Timeout object
let timerId: number;
- this._loadingPromise = Promise.race([
- new Promise<void>(resolve => (this._loadingResolver = resolve)),
+ this.loadingPromise = Promise.race([
+ new Promise<void>(resolve => (this.loadingResolver = resolve)),
new Promise(
(_, reject) =>
(timerId = window.setTimeout(() => {
- reject(new Error(this._timeout()));
+ reject(new Error(this.timeout()));
}, PLUGIN_LOADING_TIMEOUT_MS))
),
]).finally(() => {
if (timerId) clearTimeout(timerId);
}) as Promise<void>;
}
- return this._loadingPromise;
+ return this.loadingPromise;
}
}
-
-// TODO(dmfilippov): Convert to service and add to appContext
-let pluginLoader = new PluginLoader();
-export function _testOnly_resetPluginLoader() {
- pluginLoader = new PluginLoader();
- return pluginLoader;
-}
-
-export function getPluginLoader() {
- return pluginLoader;
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.ts
index 3005c37..ce1babc 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.ts
@@ -5,18 +5,16 @@
*/
import '../../../test/common-test-setup';
import {PLUGIN_LOADING_TIMEOUT_MS} from './gr-api-utils';
-import {PluginLoader, _testOnly_resetPluginLoader} from './gr-plugin-loader';
-import {
- resetPlugins,
- stubBaseUrl,
- waitEventLoop,
-} from '../../../test/test-utils';
+import {PluginLoader} from './gr-plugin-loader';
+import {stubBaseUrl, waitEventLoop} from '../../../test/test-utils';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils';
import {PluginApi} from '../../../api/plugin';
import {SinonFakeTimers} from 'sinon';
import {Timestamp} from '../../../api/rest-api';
import {EventType} from '../../../types/events';
import {assert} from '@open-wc/testing';
+import {getAppContext} from '../../../services/app-context';
+import {_testOnly_resetEndpoints} from './gr-plugin-endpoints';
suite('gr-plugin-loader tests', () => {
let plugin: PluginApi;
@@ -35,14 +33,19 @@
stubRestApi('send').returns(
Promise.resolve({...new Response(), status: 200})
);
- pluginLoader = _testOnly_resetPluginLoader();
+ pluginLoader = new PluginLoader(
+ getAppContext().reportingService,
+ getAppContext().restApiService,
+ getAppContext().pluginsModel
+ );
+ window.Gerrit = pluginLoader;
bodyStub = sinon.stub(document.body, 'appendChild');
url = window.location.origin;
});
teardown(() => {
clock.restore();
- resetPlugins();
+ _testOnly_resetEndpoints();
});
test('reuse plugin for install calls', () => {
@@ -73,11 +76,11 @@
test('report pluginsLoaded', async () => {
const pluginsLoadedStub = sinon.stub(
- pluginLoader._getReporting(),
+ getAppContext().reportingService,
'pluginsLoaded'
);
pluginsLoadedStub.reset();
- (window.Gerrit as any)._loadPlugins([]);
+ pluginLoader.loadPlugins([]);
await waitEventLoop();
assert.isTrue(pluginsLoadedStub.called);
});
@@ -99,11 +102,11 @@
});
test('plugins installed successfully', async () => {
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(() => void 0, undefined, url);
});
const pluginsLoadedStub = sinon.stub(
- pluginLoader._getReporting(),
+ getAppContext().reportingService,
'pluginsLoaded'
);
@@ -119,7 +122,7 @@
});
test('isPluginEnabled and isPluginLoaded', async () => {
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(() => void 0, undefined, url);
});
@@ -147,7 +150,7 @@
const alertStub = sinon.stub();
addListenerForTest(document, EventType.SHOW_ALERT, alertStub);
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(
() => {
if (url === plugins[0]) {
@@ -160,7 +163,7 @@
});
const pluginsLoadedStub = sinon.stub(
- pluginLoader._getReporting(),
+ getAppContext().reportingService,
'pluginsLoaded'
);
@@ -181,7 +184,7 @@
const alertStub = sinon.stub();
addListenerForTest(document, EventType.SHOW_ALERT, alertStub);
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(
() => {
if (url === plugins[0]) {
@@ -194,7 +197,7 @@
});
const pluginsLoadedStub = sinon.stub(
- pluginLoader._getReporting(),
+ getAppContext().reportingService,
'pluginsLoaded'
);
@@ -220,7 +223,7 @@
const alertStub = sinon.stub();
addListenerForTest(document, EventType.SHOW_ALERT, alertStub);
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(
() => {
throw new Error('failed');
@@ -231,7 +234,7 @@
});
const pluginsLoadedStub = sinon.stub(
- pluginLoader._getReporting(),
+ getAppContext().reportingService,
'pluginsLoaded'
);
@@ -252,12 +255,12 @@
const alertStub = sinon.stub();
addListenerForTest(document, EventType.SHOW_ALERT, alertStub);
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(() => {}, url === plugins[0] ? '' : 'alpha', url);
});
const pluginsLoadedStub = sinon.stub(
- pluginLoader._getReporting(),
+ getAppContext().reportingService,
'pluginsLoaded'
);
@@ -270,11 +273,11 @@
});
test('multiple assets for same plugin installed successfully', async () => {
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(() => void 0, undefined, url);
});
const pluginsLoadedStub = sinon.stub(
- pluginLoader._getReporting(),
+ getAppContext().reportingService,
'pluginsLoaded'
);
@@ -295,7 +298,7 @@
setup(() => {
loadJsPluginStub = sinon.stub();
sinon
- .stub(pluginLoader, '_createScriptTag')
+ .stub(pluginLoader, 'createScriptTag')
.callsFake((url: string, _onerror?: OnErrorEventHandler | undefined) =>
loadJsPluginStub(url)
);
@@ -303,7 +306,7 @@
test('invalid plugin path', () => {
const failToLoadStub = sinon.stub();
- sinon.stub(pluginLoader, '_failToLoad').callsFake((...args) => {
+ sinon.stub(pluginLoader, 'failToLoad').callsFake((...args) => {
failToLoadStub(...args);
});
@@ -353,7 +356,7 @@
window.ASSETS_PATH = 'https://cdn.com';
loadJsPluginStub = sinon.stub();
sinon
- .stub(pluginLoader, '_createScriptTag')
+ .stub(pluginLoader, 'createScriptTag')
.callsFake((url: string, _onerror?: OnErrorEventHandler | undefined) =>
loadJsPluginStub(url)
);
@@ -409,7 +412,7 @@
installed = true;
}
}
- sinon.stub(pluginLoader, '_loadJsPlugin').callsFake(url => {
+ sinon.stub(pluginLoader, 'loadJsPlugin').callsFake(url => {
window.Gerrit.install(() => pluginCallback(url), undefined, url);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
index b4f7324..65e4960 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
@@ -5,9 +5,10 @@
*/
import {HttpMethod} from '../../../constants/constants';
import {RequestPayload} from '../../../types/common';
-import {getAppContext} from '../../../services/app-context';
import {ErrorCallback, RestPluginApi} from '../../../api/rest';
import {PluginApi} from '../../../api/plugin';
+import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
async function getErrorMessage(response: Response): Promise<string> {
const text = await response.text();
@@ -24,11 +25,12 @@
}
export class GrPluginRestApi implements RestPluginApi {
- private readonly restApi = getAppContext().restApiService;
-
- private readonly reporting = getAppContext().reportingService;
-
- constructor(readonly plugin: PluginApi, private readonly prefix = '') {
+ constructor(
+ private readonly restApi: RestApiService,
+ private readonly reporting: ReportingService,
+ readonly plugin: PluginApi,
+ private readonly prefix = ''
+ ) {
this.reporting.trackApi(this.plugin, 'rest', 'constructor');
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api_test.ts
index d6d7fc2..c5bef85 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api_test.ts
@@ -14,6 +14,7 @@
createServerInfo,
} from '../../../test/test-data-generators';
import {HttpMethod} from '../../../api/rest-api';
+import {getAppContext} from '../../../services/app-context';
suite('gr-plugin-rest-api tests', () => {
let instance: GrPluginRestApi;
@@ -32,7 +33,11 @@
'0.1',
'http://test.com/plugins/testplugin/static/test.js'
);
- instance = new GrPluginRestApi(pluginApi!);
+ instance = new GrPluginRestApi(
+ getAppContext().restApiService,
+ getAppContext().reportingService,
+ pluginApi!
+ );
});
test('fetch', async () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
index 21ab10a..b6eb18c 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
@@ -21,7 +21,6 @@
import {HttpMethod} from '../../../constants/constants';
import {GrChangeActions} from '../../change/gr-change-actions/gr-change-actions';
import {GrChecksApi} from '../../plugins/gr-checks-api/gr-checks-api';
-import {getAppContext} from '../../../services/app-context';
import {AdminPluginApi} from '../../../api/admin';
import {AnnotationPluginApi} from '../../../api/annotation';
import {EventHelperPluginApi} from '../../../api/event-helper';
@@ -32,6 +31,10 @@
import {RestPluginApi} from '../../../api/rest';
import {HookApi, PluginElement, RegisterOptions} from '../../../api/hook';
import {AttributeHelperPluginApi} from '../../../api/attribute-helper';
+import {JsApiService} from './gr-js-api-types';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
+import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
+import {PluginsModel} from '../../../models/plugins/plugins-model';
/**
* Plugin-provided custom components can affect content in extension
@@ -60,13 +63,13 @@
private readonly _name: string = PLUGIN_NAME_NOT_SET;
- private readonly jsApi = getAppContext().jsApiService;
-
- private readonly report = getAppContext().reportingService;
-
- private readonly restApiService = getAppContext().restApiService;
-
- constructor(url?: string) {
+ constructor(
+ url: string,
+ private readonly jsApi: JsApiService,
+ private readonly report: ReportingService,
+ private readonly restApiService: RestApiService,
+ private readonly pluginsModel: PluginsModel
+ ) {
this.domHooks = new GrDomHooksManager(this);
if (!url) {
@@ -228,27 +231,27 @@
}
checks(): GrChecksApi {
- return new GrChecksApi(this);
+ return new GrChecksApi(this.report, this.pluginsModel, this);
}
reporting(): ReportingPluginApi {
- return new GrReportingJsApi(this);
+ return new GrReportingJsApi(this.report, this);
}
admin(): AdminPluginApi {
- return new GrAdminApi(this);
+ return new GrAdminApi(this.report, this);
}
restApi(prefix?: string): RestPluginApi {
- return new GrPluginRestApi(this, prefix);
+ return new GrPluginRestApi(this.restApiService, this.report, this, prefix);
}
attributeHelper(element: HTMLElement): AttributeHelperPluginApi {
- return new GrAttributeHelper(this, element);
+ return new GrAttributeHelper(this.report, this, element);
}
eventHelper(element: HTMLElement): EventHelperPluginApi {
- return new GrEventHelper(this, element);
+ return new GrEventHelper(this.report, this, element);
}
popup(): Promise<PopupPluginApi>;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
index fab0e6c..d82b68d 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
@@ -3,17 +3,18 @@
* Copyright 2020 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {getAppContext} from '../../../services/app-context';
import {PluginApi} from '../../../api/plugin';
import {EventDetails, ReportingPluginApi} from '../../../api/reporting';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
/**
* Defines all methods that will be exported to plugin from reporting service.
*/
export class GrReportingJsApi implements ReportingPluginApi {
- private readonly reporting = getAppContext().reportingService;
-
- constructor(private readonly plugin: PluginApi) {
+ constructor(
+ private readonly reporting: ReportingService,
+ private readonly plugin: PluginApi
+ ) {
this.reporting.trackApi(this.plugin, 'reporting', 'constructor');
}
diff --git a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
index aedf493..f1fecee 100644
--- a/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
+++ b/polygerrit-ui/app/embed/gr-diff-app-context-init.ts
@@ -63,12 +63,12 @@
restApiService: (_ctx: Partial<AppContext>) => {
throw new Error('restApiService is not implemented');
},
- jsApiService: (_ctx: Partial<AppContext>) => {
- throw new Error('jsApiService is not implemented');
- },
pluginsModel: (_ctx: Partial<AppContext>) => {
throw new Error('pluginsModel is not implemented');
},
+ pluginLoader: (_Ctx: Partial<AppContext>) => {
+ throw new Error('pluginLoader is not implemented');
+ },
};
return create<AppContext>(appRegistry);
}
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 2b54eb5..376e458 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -14,7 +14,6 @@
import {ChangeModel, changeModelToken} from '../models/change/change-model';
import {FilesModel, filesModelToken} from '../models/change/files-model';
import {ChecksModel, checksModelToken} from '../models/checks/checks-model';
-import {GrJsApiInterface} from '../elements/shared/gr-js-api-interface/gr-js-api-interface-element';
import {GrStorageService, storageServiceToken} from './storage/gr-storage_impl';
import {UserModel, userModelToken} from '../models/user/user-model';
import {
@@ -64,6 +63,7 @@
import {RepoViewModel, repoViewModelToken} from '../models/views/repo';
import {SearchViewModel, searchViewModelToken} from '../models/views/search';
import {navigationToken} from '../elements/core/gr-navigation/gr-navigation';
+import {PluginLoader} from '../elements/shared/gr-js-api-interface/gr-plugin-loader';
/**
* The AppContext lazy initializator for all services
@@ -85,12 +85,16 @@
assertIsDefined(ctx.authService, 'authService');
return new GrRestApiServiceImpl(ctx.authService);
},
- jsApiService: (ctx: Partial<AppContext>) => {
- const reportingService = ctx.reportingService;
- assertIsDefined(reportingService, 'reportingService');
- return new GrJsApiInterface(reportingService);
- },
pluginsModel: (_ctx: Partial<AppContext>) => new PluginsModel(),
+ pluginLoader: (ctx: Partial<AppContext>) => {
+ const reportingService = ctx.reportingService;
+ const restApiService = ctx.restApiService;
+ const pluginsModel = ctx.pluginsModel;
+ assertIsDefined(reportingService, 'reportingService');
+ assertIsDefined(restApiService, 'restApiService');
+ assertIsDefined(pluginsModel, 'pluginsModel');
+ return new PluginLoader(reportingService, restApiService, pluginsModel);
+ },
};
return create<AppContext>(appRegistry);
}
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index a8074b6..cc2962a 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -9,8 +9,8 @@
import {ReportingService} from './gr-reporting/gr-reporting';
import {AuthService} from './gr-auth/gr-auth';
import {RestApiService} from './gr-rest-api/gr-rest-api';
-import {JsApiService} from '../elements/shared/gr-js-api-interface/gr-js-api-types';
import {PluginsModel} from '../models/plugins/plugins-model';
+import {PluginLoader} from '../elements/shared/gr-js-api-interface/gr-plugin-loader';
export interface AppContext {
flagsService: FlagsService;
@@ -18,8 +18,8 @@
eventEmitter: EventEmitterService;
authService: AuthService;
restApiService: RestApiService;
- jsApiService: JsApiService;
pluginsModel: PluginsModel;
+ pluginLoader: PluginLoader;
}
/**
diff --git a/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_test.js b/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_test.js
index a63eda3..6ff2fd8e 100644
--- a/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_test.js
+++ b/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_test.js
@@ -4,69 +4,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../../test/common-test-setup';
-import {mockPromise} from '../../test/test-utils';
import {EventEmitter} from './gr-event-interface_impl';
import {assert} from '@open-wc/testing';
suite('gr-event-interface tests', () => {
- let gerrit;
- setup(() => {
- gerrit = window.Gerrit;
- });
-
- suite('test on Gerrit', () => {
- setup(() => {
- gerrit.removeAllListeners();
- });
-
- test('communicate between plugin and Gerrit', async () => {
- const eventName = 'test-plugin-event';
- let p;
- const promise = mockPromise();
- gerrit.on(eventName, e => {
- assert.equal(e.value, 'test');
- assert.equal(e.plugin, p);
- promise.resolve();
- });
- gerrit.install(plugin => {
- p = plugin;
- gerrit.emit(eventName, {value: 'test', plugin});
- }, '0.1',
- 'http://test.com/plugins/testplugin/static/test.js');
- await promise;
- });
-
- test('listen on events from core', async () => {
- const eventName = 'test-plugin-event';
- const promise = mockPromise();
- gerrit.on(eventName, e => {
- assert.equal(e.value, 'test');
- promise.resolve();
- });
-
- gerrit.emit(eventName, {value: 'test'});
- await promise;
- });
-
- test('communicate across plugins', async () => {
- const eventName = 'test-plugin-event';
- const promise = mockPromise();
- gerrit.install(plugin => {
- gerrit.on(eventName, e => {
- assert.equal(e.plugin.getPluginName(), 'testB');
- promise.resolve();
- });
- }, '0.1',
- 'http://test.com/plugins/testA/static/testA.js');
-
- gerrit.install(plugin => {
- gerrit.emit(eventName, {plugin});
- }, '0.1',
- 'http://test.com/plugins/testB/static/testB.js');
- await promise;
- });
- });
-
suite('test on interfaces', () => {
let testObj;
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 5ce4e39..a56c84f 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
@@ -2739,15 +2739,9 @@
_changeBaseURL(
changeNum: NumericChangeId,
- revisionId?: RevisionId,
- project?: RepoName
+ revisionId?: RevisionId
): Promise<string> {
- // TODO(kaspern): For full slicer migration, app should warn with a call
- // stack every time _changeBaseURL is called without a project.
- const projectPromise = project
- ? Promise.resolve(project)
- : this.getFromProjectLookup(changeNum);
- return projectPromise.then(project => {
+ return this.getFromProjectLookup(changeNum).then(project => {
// TODO(TS): unclear why project can't be null here. Fix it
let url = `/changes/${encodeURIComponent(
project as RepoName
@@ -3076,9 +3070,6 @@
_getChangeURLAndSend(req: SendJSONChangeRequest): Promise<ParsedJSON>;
- /**
- * Alias for _changeBaseURL.then(send).
- */
_getChangeURLAndSend(
req: SendChangeRequest
): Promise<ParsedJSON | Response | undefined> {
@@ -3106,9 +3097,6 @@
});
}
- /**
- * Alias for _changeBaseURL.then(fetchJSON).
- */
_getChangeURLAndFetch(
req: FetchChangeJSON,
noAcceptHeader?: boolean
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index 31462b5..5cec8cda 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -6,13 +6,12 @@
// TODO(dmfilippov): remove bundled-polymer.js imports when the following issue
// https://github.com/Polymer/polymer-resin/issues/9 is resolved.
import '../scripts/bundled-polymer';
-import {AppContext, injectAppContext} from '../services/app-context';
+import {AppContext} from '../services/app-context';
import {Finalizable} from '../services/registry';
import {
createTestAppContext,
createTestDependencies,
} from './test-app-context-init';
-import {_testOnly_resetPluginLoader} from '../elements/shared/gr-js-api-interface/gr-plugin-loader';
import {_testOnlyResetGrRestApiSharedObjects} from '../services/gr-rest-api/gr-rest-api-impl';
import {
cleanupTestUtils,
@@ -102,7 +101,8 @@
// overwritten by some other code.
assert.equal(getCleanupsCount(), 0);
appContext = createTestAppContext();
- injectAppContext(appContext);
+ initGlobalVariables(appContext);
+
finalizers.push(appContext);
const dependencies = createTestDependencies(appContext, testResolver);
for (const [token, provider] of dependencies) {
@@ -111,21 +111,17 @@
document.addEventListener('request-dependency', resolveDependency);
// The following calls is necessary to avoid influence of previously executed
// tests.
- initGlobalVariables(appContext);
const selection = document.getSelection();
if (selection) {
selection.removeAllRanges();
}
- const pl = _testOnly_resetPluginLoader();
// For testing, always init with empty plugin list
// Since when serve in gr-app, we always retrieve the list
// from project config and init loading after that, all
// `awaitPluginsLoaded` will rely on that to kick off,
// in testing, we want to kick start this earlier.
- // You still can manually call _testOnly_resetPluginLoader
- // to reset this behavior if you need to test something specific.
- pl.loadPlugins([]);
+ appContext.pluginLoader.loadPlugins([]);
_testOnlyResetGrRestApiSharedObjects();
});
@@ -155,7 +151,6 @@
'restore() method is called for this test-fixture. Usually the call' +
'happens automatically.'
);
- return;
}
if (
element.tagName === 'DIV' &&
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index a0b6130..2fd421e 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -14,7 +14,6 @@
import {GrAuthMock} from '../services/gr-auth/gr-auth_mock';
import {FlagsServiceImplementation} from '../services/flags/flags_impl';
import {EventEmitter} from '../services/gr-event-interface/gr-event-interface_impl';
-import {GrJsApiInterface} from '../elements/shared/gr-js-api-interface/gr-js-api-interface-element';
import {PluginsModel} from '../models/plugins/plugins-model';
import {MockHighlightService} from '../services/highlight/highlight-service-mock';
import {createAppDependencies, Creator} from '../services/app-context-init';
@@ -22,6 +21,7 @@
import {DependencyToken} from '../models/dependency';
import {storageServiceToken} from '../services/storage/gr-storage_impl';
import {highlightServiceToken} from '../services/highlight/highlight-service';
+import {PluginLoader} from '../elements/shared/gr-js-api-interface/gr-plugin-loader';
export function createTestAppContext(): AppContext & Finalizable {
const appRegistry: Registry<AppContext> = {
@@ -34,11 +34,16 @@
return new GrAuthMock(ctx.eventEmitter);
},
restApiService: (_ctx: Partial<AppContext>) => grRestApiMock,
- jsApiService: (ctx: Partial<AppContext>) => {
- assertIsDefined(ctx.reportingService, 'reportingService');
- return new GrJsApiInterface(ctx.reportingService);
- },
pluginsModel: (_ctx: Partial<AppContext>) => new PluginsModel(),
+ pluginLoader: (ctx: Partial<AppContext>) => {
+ const reportingService = ctx.reportingService;
+ const restApiService = ctx.restApiService;
+ const pluginsModel = ctx.pluginsModel;
+ assertIsDefined(reportingService, 'reportingService');
+ assertIsDefined(restApiService, 'restApiService');
+ assertIsDefined(pluginsModel, 'pluginsModel');
+ return new PluginLoader(reportingService, restApiService, pluginsModel);
+ },
};
return create<AppContext>(appRegistry);
}
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index d7b178a..d7983b0 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -4,7 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../types/globals';
-import {_testOnly_resetPluginLoader} from '../elements/shared/gr-js-api-interface/gr-plugin-loader';
import {_testOnly_resetEndpoints} from '../elements/shared/gr-js-api-interface/gr-plugin-endpoints';
import {getAppContext} from '../services/app-context';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
@@ -49,11 +48,10 @@
}
// Provide reset plugins function to clear installed plugins between tests.
-// No gr-app found (running tests)
export const resetPlugins = () => {
_testOnly_resetEndpoints();
- const pl = _testOnly_resetPluginLoader();
- pl.loadPlugins([]);
+ getAppContext().pluginLoader.reset();
+ getAppContext().pluginLoader.loadPlugins([]);
};
export type CleanupCallback = () => void;
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 4281f43..644fe93 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -245,3 +245,14 @@
)
);
}
+
+/**
+ * Noop function that can be used to suppress the tsetse must-use-promises rule.
+ *
+ * Example Usage:
+ * async function x() {
+ * await doA();
+ * noAwait(doB());
+ * }
+ */
+export function noAwait(_: {then: Function} | null | undefined) {}
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index 0b53f61..5f3a9c86 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -453,7 +453,7 @@
const path: EventTarget[] = e.composedPath() ?? [];
for (const el of path) {
if (!isElementTarget(el)) continue;
- if (el.tagName === 'GR-OVERLAY') return true;
+ if (el.tagName === 'GR-OVERLAY' || el.tagName === 'DIALOG') return true;
}
return false;
}
diff --git a/polygerrit-ui/app/workers/service-worker-class.ts b/polygerrit-ui/app/workers/service-worker-class.ts
index ee85c0e..f9cc591 100644
--- a/polygerrit-ui/app/workers/service-worker-class.ts
+++ b/polygerrit-ui/app/workers/service-worker-class.ts
@@ -18,6 +18,7 @@
} from './service-worker-indexdb';
import {createDashboardUrl} from '../models/views/dashboard';
import {createChangeUrl} from '../models/views/change';
+import {noAwait} from '../utils/async-util';
export class ServiceWorker {
constructor(
@@ -156,7 +157,7 @@
const prevLatestUpdateTimestampMs = this.latestUpdateTimestampMs;
this.latestUpdateTimestampMs = Date.now();
await this.saveState();
- this.sendReport('polling');
+ noAwait(this.sendReport('polling'));
const changes = await this.getLatestAttentionSetChanges();
const latestAttentionChanges = filterAttentionChangesAfter(
changes,
