
There are 2 REST endpoints that allow callers to create merge commits in Gerrit: 1. Create Change REST endpoint: Creates a new change. If ChangeInput.merge is specified a merge commit for the new change is created. 2. Create Merge Patch Set REST endpoint: Creates a merge commit and adds it as a new patch set to an existing change. In both cases the merge is performed by Gerrit and if the merge fails due to conflicts in the files, so far the request is always rejected with '409 Conflict'. This change adds a new option 'allowConflicts' in MergeInput that allows the merge to succeed even if there are conflicts. If this option is set and there are conflicts: * the request still succeeds and the new change / patch set gets created * the conflicting files in the merge commit contain Git conflict markers * callers can know that there were conflicts by checking the 'containsGitConflicts' field in the returned ChangeInfo * the change is set to work-in-progress so that it's not accidentally submitted with conflicts * a change message is posted on the change that lists the files that have conflicts This functionality is consistent with the existing 'allowConflicts' option in CherryPickInput which allows to let a cherry-pick succeed even if there are conflicts. Also here the request succeeds, callers can check the 'containsGitConflicts' field in the returned ChangeInfo, the change is set to work-in-progress and a change message lists the files that have conflicts. Being able to create merge commits even if there are conflicts is useful because it: * allows robots to create merge commits, and let human users resolve conflicts if needed * allows to resolve conflicts on merge without having a local git client, e.g. by using online edit Implementation-wise some aspects should be pointed out: * To let callers know whether the request resulted in a merge with conflicts, ChangeInfo contains a new 'containsGitConflicts' field now. This field is only populated if the change info is returned in response to a request that creates a new change or patch set and conflicts are allowed. Doing this was already considered when the allow conflicts option was added for cherry-picks (see alternative 1. in the commit message of change Iae9eef38a). At that time we didn't take this approach because it might confuse users if this field is not populated for other requests. Instead we decided to add CherryPickChangeInfo that extends ChangeInfo. However now this approach doesn't scale well as we would need to add further classes that extend ChangeInfo (e.g. NewChangeInfo for the Create Change REST endpoint). To mitigate the concern that the 'containsGitConflicts' field might be confusing for users, it states very explicitly in the documentation when it is populated. * CherryPickChangeInfo is obsolete now, but we cannot remove it without breaking the Java API, hence we keep it. * To be able to test the ChangeInfo that is returned by the Create Change REST endpoint we need a new createAsInfo(ChangeInput) method in the Changes API that returns the ChangeInfo (instead of a ChangeApi). This follows the example of the cherryPickAsInfo(CherryPickInput) method that was added by change Iae9eef38a. * PatchSetInserter is enhanced with a method that allows to set the change to work-in-progress so that this flag can be set in the same BatchUpdateOp which also creates the patch set. It's not possible to set this flag from a separate BatchUpdateOp in the same BatchUpdate because the second BatchUpdateOp cannot observe the patch set that is created by the first BatchUpdateOp (and the patch set data is needed to send out the WorkInProgressStateChanged event). * PatchSetInserter is also bound in BatchProgramModule, but EventUtil is not available in this Guice stack which is why injecting WorkInProgressStateChanged into PatchSetInserter fails in this setup. To solve this, WorkInProgressStateChanged defines a disabled implementation that is bound in BatchProgramModule (this follows the example of RevisionCreated which is also needed by PatchSetInserter). Change-Id: Ib6bc8eedfd8a98bf1088660e27610e1eafb095fc Signed-off-by: Edwin Kempin <ekempin@google.com>
92 lines
3.4 KiB
Java
92 lines
3.4 KiB
Java
// Copyright (C) 2017 The Android Open Source Project
|
|
//
|
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
|
// you may not use this file except in compliance with the License.
|
|
// You may obtain a copy of the License at
|
|
//
|
|
// http://www.apache.org/licenses/LICENSE-2.0
|
|
//
|
|
// Unless required by applicable law or agreed to in writing, software
|
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
// See the License for the specific language governing permissions and
|
|
// limitations under the License.
|
|
|
|
package com.google.gerrit.server.extensions.events;
|
|
|
|
import com.google.common.flogger.FluentLogger;
|
|
import com.google.gerrit.entities.Change;
|
|
import com.google.gerrit.entities.PatchSet;
|
|
import com.google.gerrit.exceptions.StorageException;
|
|
import com.google.gerrit.extensions.api.changes.NotifyHandling;
|
|
import com.google.gerrit.extensions.common.AccountInfo;
|
|
import com.google.gerrit.extensions.common.ChangeInfo;
|
|
import com.google.gerrit.extensions.common.RevisionInfo;
|
|
import com.google.gerrit.extensions.events.WorkInProgressStateChangedListener;
|
|
import com.google.gerrit.server.GpgException;
|
|
import com.google.gerrit.server.account.AccountState;
|
|
import com.google.gerrit.server.patch.PatchListNotAvailableException;
|
|
import com.google.gerrit.server.permissions.PermissionBackendException;
|
|
import com.google.gerrit.server.plugincontext.PluginSetContext;
|
|
import com.google.inject.Inject;
|
|
import com.google.inject.Singleton;
|
|
import java.io.IOException;
|
|
import java.sql.Timestamp;
|
|
|
|
/** Helper class to fire an event when the work-in-progress state of a change has been toggled. */
|
|
@Singleton
|
|
public class WorkInProgressStateChanged {
|
|
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
|
|
|
|
public static final WorkInProgressStateChanged DISABLED =
|
|
new WorkInProgressStateChanged() {
|
|
@Override
|
|
public void fire(Change change, PatchSet patchSet, AccountState account, Timestamp when) {}
|
|
};
|
|
|
|
private final PluginSetContext<WorkInProgressStateChangedListener> listeners;
|
|
private final EventUtil util;
|
|
|
|
@Inject
|
|
WorkInProgressStateChanged(
|
|
PluginSetContext<WorkInProgressStateChangedListener> listeners, EventUtil util) {
|
|
this.listeners = listeners;
|
|
this.util = util;
|
|
}
|
|
|
|
private WorkInProgressStateChanged() {
|
|
this.listeners = null;
|
|
this.util = null;
|
|
}
|
|
|
|
public void fire(Change change, PatchSet patchSet, AccountState account, Timestamp when) {
|
|
if (listeners.isEmpty()) {
|
|
return;
|
|
}
|
|
try {
|
|
Event event =
|
|
new Event(
|
|
util.changeInfo(change),
|
|
util.revisionInfo(change.getProject(), patchSet),
|
|
util.accountInfo(account),
|
|
when);
|
|
listeners.runEach(l -> l.onWorkInProgressStateChanged(event));
|
|
} catch (StorageException
|
|
| PatchListNotAvailableException
|
|
| GpgException
|
|
| IOException
|
|
| PermissionBackendException e) {
|
|
logger.atSevere().withCause(e).log("Couldn't fire event");
|
|
}
|
|
}
|
|
|
|
/** Event to be fired when the work-in-progress state of a change has been toggled. */
|
|
private static class Event extends AbstractRevisionEvent
|
|
implements WorkInProgressStateChangedListener.Event {
|
|
|
|
protected Event(ChangeInfo change, RevisionInfo revision, AccountInfo who, Timestamp when) {
|
|
super(change, revision, who, when, NotifyHandling.ALL);
|
|
}
|
|
}
|
|
}
|