Fix missing Submit ACL on dependant changes
The MergeOp logic doesn't check Submit permissions for dependant
changes. This can cause an issue with topics, since changes in the same
topic are not necessarily in the same branch (or even project).
This issue was caused by inconsistancy in checks between Submit UiAction
and MergeOp. To avoid similar issues in the future, instead of simply
fixing the check in MergeOp, we update both Submit.java and MergeOp to
use a shared implementation.
As the result of this, READ permission is potentially checked multiple
times. However there are potential reasons why it can be useful
(staleness) and trying to avoid it would add more complexity to the
change.
The check in Submit.java used to have special logic that uses current
ChangeData, instead of potentially stale information in ChangeSet
(because it's generated from index). To maintain the behaviour we
replace the data for the triggering change in the ChangeSet before
calling checkCommonSubmitProblems.
countChangesThatWereSubmittedWithRebaserApproval was moved out of
checking logic and now only counts the changes, if all changes passed
the checks. Which seems more inline with the intention based on the
name.
The Submit permission error for dependent changes is surfaced as
ResourceConflictException (as opposed to AuthException), since the error
message can contain multiple problems from commitStatus, and it's not
worth having a custom logic to "sometimes" have better error code.
Google-Bug-Id: b/347935402
Release-Notes: skip
Change-Id: Ic6dbda6dec1deebb6c14bb9056e0c20db768a9e1
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 36b859c..0a47d62 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.change;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static java.util.stream.Collectors.joining;
@@ -71,10 +72,10 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
-import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -99,8 +100,6 @@
"Submit all ${topicSize} changes of the same topic "
+ "(${submitSize} changes including ancestors and other "
+ "changes related by topic)";
- private static final String BLOCKED_HIDDEN_SUBMIT_TOOLTIP =
- "This change depends on other hidden changes which are not ready";
private static final String CLICK_FAILURE_TOOLTIP = "Clicking the button would fail";
private static final String CHANGE_UNMERGEABLE = "Problems with integrating this change";
@@ -240,48 +239,15 @@
*/
@Nullable
private String problemsForSubmittingChangeset(ChangeData cd, ChangeSet cs, CurrentUser user) {
- try {
- if (cs.furtherHiddenChanges()) {
- logger.atFine().log(
- "Change %d cannot be submitted by user %s because it depends on hidden changes: %s",
- cd.getId().get(), user.getLoggableName(), cs.nonVisibleChanges());
- return BLOCKED_HIDDEN_SUBMIT_TOOLTIP;
- }
- for (ChangeData c : cs.changes()) {
- Set<ChangePermission> can =
- permissionBackend
- .user(user)
- .change(c)
- .test(EnumSet.of(ChangePermission.READ, ChangePermission.SUBMIT));
- if (!can.contains(ChangePermission.READ)) {
- logger.atFine().log(
- "Change %d cannot be submitted by user %s because it depends on change %d which the user cannot read",
- cd.getId().get(), user.getLoggableName(), c.getId().get());
- return BLOCKED_HIDDEN_SUBMIT_TOOLTIP;
- }
- if (!can.contains(ChangePermission.SUBMIT)) {
- return "You don't have permission to submit change " + c.getId();
- }
- if (c.change().isWorkInProgress()) {
- return "Change " + c.getId() + " is marked work in progress";
- }
- try {
- // The data in the change index may be stale (e.g. if submit requirements have been
- // changed). For that one change for which the submit action is computed, use the
- // freshly loaded ChangeData instance 'cd' instead of the potentially stale ChangeData
- // instance 'c' that was loaded from the index. This makes a difference if the ChangeSet
- // 'cs' only contains this one single change. If the ChangeSet contains further changes
- // those may still be stale.
- MergeOp.checkSubmitRequirements(cd.getId().equals(c.getId()) ? cd : c);
- } catch (ResourceConflictException e) {
- return (c.getId() == cd.getId())
- ? String.format("Change %s is not ready: %s", cd.getId(), e.getMessage())
- : String.format(
- "Change %s must be submitted with change %s but %s is not ready: %s",
- cd.getId(), c.getId(), c.getId(), e.getMessage());
- }
- }
+ Optional<String> reason =
+ MergeOp.checkCommonSubmitProblems(cd.change(), cs, false, permissionBackend, user).stream()
+ .findFirst()
+ .map(MergeOp.ChangeProblem::getProblem);
+ if (reason.isPresent()) {
+ return reason.get();
+ }
+ try {
if (!useMergeabilityCheck) {
return null;
}
@@ -298,7 +264,7 @@
return "Problems with change(s): "
+ unmergeable.stream().map(c -> c.getId().toString()).collect(joining(", "));
}
- } catch (PermissionBackendException | IOException e) {
+ } catch (IOException e) {
logger.atSevere().withCause(e).log("Error checking if change is submittable");
throw new StorageException("Could not determine problems for the change", e);
}
@@ -331,6 +297,15 @@
mergeSuperSet
.get()
.completeChangeSet(cd.change(), resource.getUser(), /*includingTopicClosure= */ false);
+ // Replace potentially stale ChangeData for the current change with the fresher one.
+ cs =
+ new ChangeSet(
+ cs.changes().stream()
+ .map(csChange -> csChange.getId().equals(cd.getId()) ? cd : csChange)
+ .collect(toImmutableList()),
+ cs.nonVisibleChanges());
+ String submitProblems = problemsForSubmittingChangeset(cd, cs, resource.getUser());
+
String topic = change.getTopic();
int topicSize = 0;
if (!Strings.isNullOrEmpty(topic)) {
@@ -338,8 +313,6 @@
}
boolean treatWithTopic = submitWholeTopic && !Strings.isNullOrEmpty(topic) && topicSize > 1;
- String submitProblems = problemsForSubmittingChangeset(cd, cs, resource.getUser());
-
if (submitProblems != null) {
return new UiAction.Description()
.setLabel(treatWithTopic ? submitTopicLabel : (cs.size() > 1) ? labelWithParents : label)
diff --git a/java/com/google/gerrit/server/submit/MergeMetrics.java b/java/com/google/gerrit/server/submit/MergeMetrics.java
index b56d9ef..ed07f2f 100644
--- a/java/com/google/gerrit/server/submit/MergeMetrics.java
+++ b/java/com/google/gerrit/server/submit/MergeMetrics.java
@@ -54,32 +54,36 @@
.setRate());
}
- public void countChangesThatWereSubmittedWithRebaserApproval(ChangeData cd) {
- if (isRebaseOnBehalfOfUploader(cd)
- && hasCodeReviewApprovalOfRealUploader(cd)
- && !hasCodeReviewApprovalOfUserThatIsNotTheRealUploader(cd)
- && ignoresCodeReviewApprovalsOfUploader(cd)) {
- // 1. The patch set that is being submitted was created by rebasing on behalf of the uploader.
- // The uploader of the patch set is the original uploader on whom's behalf the rebase was
- // done. The real uploader is the user that did the rebase on behalf of the uploader (e.g. by
- // clicking on the rebase button).
- //
- // 2. The change has a Code-Review approval of the real uploader (aka the rebaser).
- //
- // 3. The change doesn't have a Code-Review approval of any other user (a user that is not the
- // real uploader).
- //
- // 4. Code-Review approvals of the uploader are ignored.
- //
- // If instead of a rebase on behalf of the uploader a normal rebase would have been done the
- // rebaser would have been the uploader of the patch set. In this case the Code-Review
- // approval of the rebaser would not have counted since Code-Review approvals of the uploader
- // are ignored.
- //
- // In this case we assume that the change would not be submittable if a normal rebase had been
- // done. This is not always correct (e.g. if there are approvals of multiple reviewers) but
- // it's good enough for the metric.
- countChangesThatWereSubmittedWithRebaserApproval.increment();
+ public void countChangesThatWereSubmittedWithRebaserApproval(ChangeSet cs) {
+ for (ChangeData cd : cs.changes()) {
+ if (isRebaseOnBehalfOfUploader(cd)
+ && hasCodeReviewApprovalOfRealUploader(cd)
+ && !hasCodeReviewApprovalOfUserThatIsNotTheRealUploader(cd)
+ && ignoresCodeReviewApprovalsOfUploader(cd)) {
+ // 1. The patch set that is being submitted was created by rebasing on behalf of the
+ // uploader.
+ //
+ // The uploader of the patch set is the original uploader on whose behalf the rebase was
+ // done. The real uploader is the user that did the rebase on behalf of the uploader (e.g.
+ // by clicking on the rebase button).
+ //
+ // 2. The change has a Code-Review approval of the real uploader (aka the rebaser).
+ //
+ // 3. The change doesn't have a Code-Review approval of any other user (a user that is not
+ // the real uploader).
+ //
+ // 4. Code-Review approvals of the uploader are ignored.
+ //
+ // If instead of a rebase on behalf of the uploader a normal rebase would have been done the
+ // rebaser would have been the uploader of the patch set. In this case the Code-Review
+ // approval of the rebaser would not have counted since Code-Review approvals of the
+ // uploader are ignored.
+ //
+ // In this case we assume that the change would not be submittable if a normal rebase had
+ // been done. This is not always correct (e.g. if there are approvals of multiple reviewers)
+ // but it's good enough for the metric.
+ countChangesThatWereSubmittedWithRebaserApproval.increment();
+ }
}
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index eb41690..233f00e 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -69,6 +69,7 @@
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.change.NotifyResolver;
@@ -83,6 +84,8 @@
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
@@ -112,6 +115,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Deque;
+import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
@@ -292,6 +296,7 @@
private final ChangeData.Factory changeDataFactory;
private final StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory;
private final MergeMetrics mergeMetrics;
+ private final PermissionBackend permissionBackend;
// Changes that were updated by this MergeOp.
private final Map<Change.Id, Change> updatedChanges;
@@ -336,7 +341,8 @@
MergeMetrics mergeMetrics,
ProjectCache projectCache,
ExperimentFeatures experimentFeatures,
- @GerritServerConfig Config config) {
+ @GerritServerConfig Config config,
+ PermissionBackend permissionBackend) {
this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory;
this.batchUpdates = batchUpdates;
@@ -362,6 +368,7 @@
hasImplicitMergeTimeoutSeconds =
ConfigUtil.getTimeUnit(
config, "change", null, "implicitMergeCalculationTimeout", 60, TimeUnit.SECONDS);
+ this.permissionBackend = permissionBackend;
}
@Override
@@ -371,6 +378,12 @@
}
}
+ /**
+ * Check that SRs are fulfilled or throw otherwise
+ *
+ * @param cd change that is being checked
+ * @throws ResourceConflictException the exception that is thrown if the SR is not fulfilled
+ */
public static void checkSubmitRequirements(ChangeData cd) throws ResourceConflictException {
PatchSet patchSet = cd.currentPatchSet();
if (patchSet == null) {
@@ -425,32 +438,119 @@
return cd.submitRecords(submitRuleOptions(/* allowClosed= */ false));
}
- private void checkSubmitRulesAndState(ChangeSet cs, boolean allowMerged)
- throws ResourceConflictException {
- checkArgument(
- !cs.furtherHiddenChanges(), "checkSubmitRulesAndState called for topic with hidden change");
+ @AutoValue
+ public abstract static class ChangeProblem {
+ public abstract Change.Id getChangeId();
+
+ public abstract String getProblem();
+
+ public static ChangeProblem create(Change.Id changeId, String problem) {
+ return new AutoValue_MergeOp_ChangeProblem(changeId, problem);
+ }
+ }
+
+ /**
+ * Returns a list of messages describing what prevents the current change from being submitted.
+ *
+ * <p>The method checks all changes in the {@code cs} for their current status, submitability and
+ * permissions.
+ *
+ * @param triggeringChange Change for which merge/submit action was initiated
+ * @param cs Set of changes that the current change depends on
+ * @param allowMerged True if change being already merged is not a problem to be reported
+ * @param permissionBackend Interface for checking user ACLs
+ * @param caller The user who is triggering a merge
+ * @return List of problems preventing merge
+ */
+ public static ImmutableList<ChangeProblem> checkCommonSubmitProblems(
+ Change triggeringChange,
+ ChangeSet cs,
+ boolean allowMerged,
+ PermissionBackend permissionBackend,
+ CurrentUser caller) {
+ ImmutableList.Builder<ChangeProblem> problems = ImmutableList.builder();
+ if (cs.furtherHiddenChanges()) {
+ logger.atFine().log(
+ "Change %d cannot be submitted by user %s because it depends on hidden changes: %s",
+ triggeringChange.getId().get(), caller.getLoggableName(), cs.nonVisibleChanges());
+ problems.add(
+ ChangeProblem.create(
+ triggeringChange.getId(),
+ String.format(
+ "Change %d depends on other hidden changes", triggeringChange.getId().get())));
+ }
for (ChangeData cd : cs.changes()) {
try {
- if (!cd.change().isNew()) {
+ Set<ChangePermission> can =
+ permissionBackend
+ .user(caller)
+ .change(cd)
+ .test(EnumSet.of(ChangePermission.READ, ChangePermission.SUBMIT));
+ if (!can.contains(ChangePermission.READ)) {
+ // The READ permission should already be handled during generation of ChangeSet, however
+ // MergeSuperSetComputation is an interface and on API level doesn't guarantee that this
+ // have been verified for all changes. Additionally, this protects against potential
+ // issues due to staleness.
+ logger.atFine().log(
+ "Change %d cannot be submitted by user %s because it depends on change %d which the"
+ + "user cannot read",
+ triggeringChange.getId().get(), caller.getLoggableName(), cd.getId().get());
+ problems.add(
+ ChangeProblem.create(
+ cd.getId(),
+ String.format(
+ "Change %d depends on other hidden changes",
+ triggeringChange.getId().get())));
+ } else if (!can.contains(ChangePermission.SUBMIT)) {
+ logger.atFine().log(
+ "Change %d cannot be submitted by user %s because it depends on change %d which the"
+ + "user cannot submit",
+ triggeringChange.getId().get(), caller.getLoggableName(), cd.getId().get());
+ problems.add(
+ ChangeProblem.create(
+ cd.getId(),
+ String.format("Insufficient permission to submit change %d", cd.getId().get())));
+ } else if (!cd.change().isNew()) {
if (!(cd.change().isMerged() && allowMerged)) {
- commitStatus.problem(
- cd.getId(), "Change " + cd.getId() + " is " + ChangeUtil.status(cd.change()));
+ problems.add(
+ ChangeProblem.create(
+ cd.getId(),
+ String.format(
+ "Change %d is %s", cd.getId().get(), ChangeUtil.status(cd.change()))));
}
} else if (cd.change().isWorkInProgress()) {
- commitStatus.problem(cd.getId(), "Change " + cd.getId() + " is work in progress");
+ problems.add(
+ ChangeProblem.create(
+ cd.getId(),
+ String.format("Change %d is marked work in progress", cd.getId().get())));
} else {
checkSubmitRequirements(cd);
- mergeMetrics.countChangesThatWereSubmittedWithRebaserApproval(cd);
}
} catch (ResourceConflictException e) {
- commitStatus.problem(cd.getId(), e.getMessage());
- } catch (StorageException e) {
+ // Exception is thrown means submit requirement is not fulfilled.
+ problems.add(
+ ChangeProblem.create(
+ cd.getId(),
+ triggeringChange.getId().equals(cd.getId())
+ ? String.format("Change %s is not ready: %s", cd.getId(), e.getMessage())
+ : String.format(
+ "Change %s must be submitted with change %s but %s is not ready: %s",
+ triggeringChange.getId(), cd.getId(), cd.getId(), e.getMessage())));
+ } catch (StorageException | PermissionBackendException e) {
String msg = "Error checking submit rules for change";
- logger.atWarning().withCause(e).log("%s %s", msg, cd.getId());
- commitStatus.problem(cd.getId(), msg);
+ logger.atWarning().withCause(e).log("%s %s", msg, triggeringChange.getId());
+ problems.add(ChangeProblem.create(cd.getId(), msg));
}
}
+ return problems.build();
+ }
+
+ private void checkSubmitRulesAndState(Change triggeringChange, ChangeSet cs, boolean allowMerged)
+ throws ResourceConflictException {
+ checkCommonSubmitProblems(triggeringChange, cs, allowMerged, permissionBackend, caller).stream()
+ .forEach(cp -> commitStatus.problem(cp.getChangeId(), cp.getProblem()));
commitStatus.maybeFailVerbose();
+ mergeMetrics.countChangesThatWereSubmittedWithRebaserApproval(cs);
}
private void bypassSubmitRulesAndRequirements(ChangeSet cs) {
@@ -580,7 +680,7 @@
this.commitStatus = new CommitStatus(filteredNoteDbChangeSet, isRetry);
if (checkSubmitRules) {
logger.atFine().log("Checking submit rules and state");
- checkSubmitRulesAndState(filteredNoteDbChangeSet, isRetry);
+ checkSubmitRulesAndState(change, filteredNoteDbChangeSet, isRetry);
} else {
logger.atFine().log("Bypassing submit rules");
bypassSubmitRulesAndRequirements(filteredNoteDbChangeSet);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/DefaultSubmitRequirementsIT.java b/javatests/com/google/gerrit/acceptance/api/change/DefaultSubmitRequirementsIT.java
index fc8eaed..9af7a2d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/DefaultSubmitRequirementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/DefaultSubmitRequirementsIT.java
@@ -41,7 +41,8 @@
.isEqualTo(
String.format(
"Failed to submit 1 change due to the following problems:\n"
- + "Change %s: submit requirement 'No-Unresolved-Comments' is unsatisfied.",
+ + "Change %1$s: Change %1$s is not ready: "
+ + "submit requirement 'No-Unresolved-Comments' is unsatisfied.",
r.getChange().getId().get()));
// Resolve the comment and check that the change can be submitted now.
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 25af040..0b86406 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -589,7 +589,7 @@
+ num
+ ": Change "
+ num
- + " is work in progress");
+ + " is marked work in progress");
}
@Test
@@ -607,7 +607,7 @@
+ num
+ ": Change "
+ num
- + " is work in progress");
+ + " is marked work in progress");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
index 1d8e0b8..7b3d1bc 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
@@ -100,7 +100,8 @@
"Failed to submit 2 changes due to the following problems:\n"
+ "Change "
+ id1
- + ": submit requirement 'Code-Review' is unsatisfied.");
+ + ": Change 8 must be submitted with change 7 but 7 is not ready: "
+ + "submit requirement 'Code-Review' is unsatisfied.");
RevCommit updatedHead = projectOperations.project(project).getHead("master");
assertThat(updatedHead.getId()).isEqualTo(initialHead.getId());
diff --git a/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java b/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
index 5a4f073..4ee5967 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
@@ -16,7 +16,10 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.NON_VISIBLE_CHANGES;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
@@ -25,6 +28,8 @@
import com.google.gerrit.acceptance.config.GerritConfig;
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.entities.Project;
import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo;
import com.google.gerrit.extensions.client.ChangeStatus;
@@ -32,6 +37,7 @@
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
import java.util.EnumSet;
@@ -336,6 +342,41 @@
assertSubmittedTogether(id2, id2, id1);
}
+ @Test
+ public void permissionToSubmitForSomeChangesInTopic() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.SUBMIT).ref("refs/heads/testbranch").group(REGISTERED_USERS))
+ .update();
+
+ createBranch(BranchNameKey.create(getProject(), "testbranch"));
+ RevCommit initialHead = projectOperations.project(project).getHead("master");
+ // Create two independent commits and push.
+ RevCommit c1_1 = commitBuilder().add("a.txt", "1").message("subject: 1").create();
+ String id1 = getChangeId(c1_1);
+ pushHead(testRepo, "refs/for/master%topic=" + name("connectingTopic"), false);
+
+ testRepo.reset(initialHead);
+ RevCommit c2_1 = commitBuilder().add("b.txt", "2").message("subject: 2").create();
+ String id2 = getChangeId(c2_1);
+ pushHead(testRepo, "refs/for/testbranch%topic=" + name("connectingTopic"), false);
+
+ approve(id1);
+ approve(id2);
+ if (isSubmitWholeTopicEnabled()) {
+ ResourceConflictException e =
+ assertThrows(ResourceConflictException.class, () -> submit(id1));
+ assertThat(e.getMessage())
+ .contains(
+ String.format(
+ "Insufficient permission to submit change %d",
+ gApi.changes().id(id2).get()._number));
+ } else {
+ submit(id1);
+ }
+ }
+
private String getChangeId(RevCommit c) throws Exception {
return GitUtil.getChangeId(testRepo, c).get();
}
