Merge "MergeUtil#newMerger: Throw a specific exception for invalid strategy"
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 0535397..9c6637b 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1670,18 +1670,37 @@
if (!res.isCommitted()) {
res.reset();
traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
- StringBuilder msg = new StringBuilder("Internal server error");
ImmutableList<String> userMessages =
globals.exceptionHooks.stream()
.map(h -> h.getUserMessage(err))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(toImmutableList());
+
+ Optional<Integer> statusCode =
+ globals.exceptionHooks.stream()
+ .map(h -> h.getStatusCode(err))
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .findFirst();
+ if (statusCode.isPresent() && statusCode.get() < 400) {
+ StringBuilder msg = new StringBuilder();
+ if (userMessages.size() == 1) {
+ msg.append(userMessages.get(0));
+ } else {
+ userMessages.forEach(m -> msg.append("\n* ").append(m));
+ }
+
+ res.setStatus(statusCode.get());
+ logger.atFinest().withCause(err).log("REST call finished: %d", statusCode);
+ return replyText(req, res, true, msg.toString());
+ }
+
+ StringBuilder msg = new StringBuilder("Internal server error");
if (!userMessages.isEmpty()) {
- msg.append("\n");
userMessages.forEach(m -> msg.append("\n* ").append(m));
}
- return replyError(req, res, SC_INTERNAL_SERVER_ERROR, msg.toString(), err);
+ return replyError(req, res, statusCode.orElse(SC_INTERNAL_SERVER_ERROR), msg.toString(), err);
}
return 0;
}
diff --git a/java/com/google/gerrit/server/ExceptionHook.java b/java/com/google/gerrit/server/ExceptionHook.java
index db44b4b..a2bade0 100644
--- a/java/com/google/gerrit/server/ExceptionHook.java
+++ b/java/com/google/gerrit/server/ExceptionHook.java
@@ -55,7 +55,9 @@
}
/**
- * Returns an error message that should be returned to the user.
+ * Returns a message that should be returned to the user.
+ *
+ * <p>This message is included into the HTTP response that is sent to the user.
*
* @param throwable throwable that was thrown while executing an operation
* @return error message that should be returned to the user, {@link Optional#empty()} if no
@@ -64,4 +66,25 @@
default Optional<String> getUserMessage(Throwable throwable) {
return Optional.empty();
}
+
+ /**
+ * Returns the HTTP status code that should be returned to the user.
+ *
+ * <p>If no value is returned ({@link Optional#empty()}) the HTTP status code defaults to {@code
+ * 500 Internal Server Error}.
+ *
+ * <p>{@link #getUserMessage(Throwable)} allows to define which message should be included into
+ * the body of the HTTP response.
+ *
+ * <p>Implementors may use this method to change the status code for certain exceptions (e.g.
+ * using this method it would be possible to return {@code 409 Conflict} for {@link
+ * com.google.gerrit.git.LockFailureException}s instead of {@code 500 Internal Server Error}).
+ *
+ * @param throwable throwable that was thrown while executing an operation
+ * @return HTTP status code that should be returned to the user, {@link Optional#empty()} if the
+ * exception should result in {@code 500 Internal Server Error}
+ */
+ default Optional<Integer> getStatusCode(Throwable throwable) {
+ return Optional.empty();
+ }
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 6703651..0817762 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -269,25 +269,6 @@
Do you really want to delete the edit?
</div>
</gr-dialog>
- <gr-dialog
- id="showRevertSubmissionChangesDialog"
- class="confirmDialog"
- confirm-label="Close"
- cancel-label=''
- on-confirm="_handleShowRevertSubmissionChangesConfirm">
- <div class="header" slot="header">
- Reverted Changes
- </div>
- <div class="main" slot="main">
- <template is="dom-repeat" items="[[_revertChanges]]">
- <div>
- <a href$="[[item.link]]" target="_blank">
- Change [[item._number]]
- </a>
- </div>
- </template>
- </div>
- </gr-dialog>
</gr-overlay>
<gr-js-api-interface id="jsAPI"></gr-js-api-interface>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 8ad2a06..9d2854a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -420,10 +420,6 @@
type: Boolean,
value: true,
},
- _revertChanges: {
- type: Array,
- value: [],
- },
};
}
@@ -1262,7 +1258,6 @@
_handleResponse(action, response) {
if (!response) { return; }
return this.$.restAPI.getResponseObject(response).then(obj => {
- let revertChanges = [];
switch (action.__key) {
case ChangeActions.REVERT:
this._waitForChangeReachable(obj._number)
@@ -1288,27 +1283,11 @@
Gerrit.Nav.navigateToChange(this.change);
break;
case ChangeActions.REVERT_SUBMISSION:
- revertChanges = obj.revert_changes || [];
- revertChanges = revertChanges.map(change => {
- change.link = '/q/' + encodeURIComponent(change.change_id);
- return change;
- });
- // list of reverted changes can never be 0
- if (revertChanges.length === 1) {
- // redirect to the change if only 1 change is reverted
- const change = revertChanges[0];
- this._waitForChangeReachable(change._number).then(success => {
- if (success) {
- Gerrit.Nav.navigateToChange(change);
- } else {
- console.error('Change ' + change._number + ' not reachable');
- }
- });
- } else {
- // show multiple reverted changes in a dialog
- this._revertChanges = revertChanges;
- this._showActionDialog(this.$.showRevertSubmissionChangesDialog);
- }
+ if (!obj.revert_changes || !obj.revert_changes.length) return;
+ /* If there is only 1 change then gerrit will automatically
+ redirect to that change */
+ Gerrit.Nav.navigateToSearchQuery('topic: ' +
+ obj.revert_changes[0].topic);
break;
default:
this.dispatchEvent(new CustomEvent('reload-change',
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index d9b7986..50f6fb0 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -1446,7 +1446,6 @@
suite('happy path', () => {
let sendStub;
- let waitForChangeReachableStub;
setup(() => {
sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true}));
@@ -1454,8 +1453,6 @@
.returns(Promise.resolve({}));
getResponseObjectStub = sandbox.stub(element.$.restAPI,
'getResponseObject');
- waitForChangeReachableStub = sandbox.stub(element,
- '_waitForChangeReachable').returns(Promise.resolve(true));
sandbox.stub(Gerrit.Nav,
'navigateToChange').returns(Promise.resolve(true));
});
@@ -1471,12 +1468,15 @@
});
suite('single changes revert', () => {
+ let navigateToSearchQueryStub;
setup(() => {
getResponseObjectStub
.returns(Promise.resolve({revert_changes: [
{change_id: 12345},
]}));
showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ navigateToSearchQueryStub = sandbox.stub(Gerrit.Nav,
+ 'navigateToSearchQuery');
});
test('revert submission single change', done => {
@@ -1484,7 +1484,7 @@
'/revert_submission', false, cleanup).then(res => {
element._handleResponse({__key: 'revert_submission'}, {}).
then(() => {
- assert.isTrue(waitForChangeReachableStub.called);
+ assert.isTrue(navigateToSearchQueryStub.called);
done();
});
});
@@ -1493,12 +1493,16 @@
suite('multiple changes revert', () => {
let showActionDialogStub;
+ let navigateToSearchQueryStub;
setup(() => {
getResponseObjectStub
.returns(Promise.resolve({revert_changes: [
- {change_id: 12345}, {change_id: 23456},
+ {change_id: 12345, topic: 'T'},
+ {change_id: 23456, topic: 'T'},
]}));
showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ navigateToSearchQueryStub = sandbox.stub(Gerrit.Nav,
+ 'navigateToSearchQuery');
});
test('revert submission multiple change', done => {
@@ -1506,7 +1510,9 @@
'/revert_submission', false, cleanup).then(res => {
element._handleResponse({__key: 'revert_submission'}, {}).then(
() => {
- assert.isTrue(showActionDialogStub.called);
+ assert.isFalse(showActionDialogStub.called);
+ assert.isTrue(navigateToSearchQueryStub.calledWith(
+ 'topic: T'));
done();
});
});
