Merge "New documentation page that explains Gerrit for GitHub users"
diff --git a/Documentation/backup.txt b/Documentation/backup.txt
index ed044ba..cd247af 100644
--- a/Documentation/backup.txt
+++ b/Documentation/backup.txt
@@ -162,9 +162,9 @@
Best you use a filesystem supporting snapshots to create a backup archive
of such a replica.
-For 2.x Gerrit versions also set up a database slave for the data stored in the
+For 2.x Gerrit versions also set up a database replica for the data stored in the
SQL database. If you are using 2.16 and migrated to _NoteDb_ you may consider to
-skip setting up a database slave, instead take a backup of the database which only
+skip setting up a database replica, instead take a backup of the database which only
contains the current schema version in this case.
In addition you need to ensure that no write operations are in flight before you
take the replica offline. Otherwise the database backup might be inconsistent
@@ -176,8 +176,8 @@
link:https://gerrit.googlesource.com/plugins/replication/+/refs/heads/master/src/main/resources/Documentation/config.md[server option]
`remote.NAME.replicateProjectDeletions`.
-If you are using Gerrit slaves to offload read traffic you can use one of these
-slaves for creating backups.
+If you are using Gerrit replica to offload read traffic you can use one of these
+replica for creating backups.
[#cons-backup-offline]
=== Take master offline for backup
diff --git a/java/com/google/gerrit/exceptions/InvalidMergeStrategyException.java b/java/com/google/gerrit/exceptions/InvalidMergeStrategyException.java
new file mode 100644
index 0000000..d9c5776
--- /dev/null
+++ b/java/com/google/gerrit/exceptions/InvalidMergeStrategyException.java
@@ -0,0 +1,23 @@
+// Copyright (C) 2019 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.exceptions;
+
+public class InvalidMergeStrategyException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ public InvalidMergeStrategyException(String strategy) {
+ super("invalid merge strategy: " + strategy);
+ }
+}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 0535397..bd32c0b 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.get().intValue());
+ 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/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 8475d03..4495317 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -40,6 +40,7 @@
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -265,7 +266,8 @@
boolean ignoreIdenticalTree,
boolean allowConflicts)
throws MissingObjectException, IncorrectObjectTypeException, IOException,
- MergeIdenticalTreeException, MergeConflictException, MethodNotAllowedException {
+ MergeIdenticalTreeException, MergeConflictException, MethodNotAllowedException,
+ InvalidMergeStrategyException {
ThreeWayMerger m = newThreeWayMerger(inserter, repoConfig);
m.setBase(originalCommit.getParent(parentIndex));
@@ -431,7 +433,8 @@
PersonIdent committerIndent,
String commitMsg,
RevWalk rw)
- throws IOException, MergeIdenticalTreeException, MergeConflictException {
+ throws IOException, MergeIdenticalTreeException, MergeConflictException,
+ InvalidMergeStrategyException {
if (!MergeStrategy.THEIRS.getName().equals(mergeStrategy)
&& rw.isMergedInto(originalCommit, mergeTip)) {
@@ -745,7 +748,7 @@
BranchNameKey destBranch,
CodeReviewCommit mergeTip,
CodeReviewCommit n)
- throws IntegrationException {
+ throws IntegrationException, InvalidMergeStrategyException {
ThreeWayMerger m = newThreeWayMerger(inserter, repoConfig);
try {
if (m.merge(mergeTip, n)) {
@@ -866,7 +869,8 @@
.collect(joining(",", "Merge changes ", merged.size() > 5 ? ", ..." : ""));
}
- public ThreeWayMerger newThreeWayMerger(ObjectInserter inserter, Config repoConfig) {
+ public ThreeWayMerger newThreeWayMerger(ObjectInserter inserter, Config repoConfig)
+ throws InvalidMergeStrategyException {
return newThreeWayMerger(inserter, repoConfig, mergeStrategyName());
}
@@ -890,7 +894,8 @@
}
public static ThreeWayMerger newThreeWayMerger(
- ObjectInserter inserter, Config repoConfig, String strategyName) {
+ ObjectInserter inserter, Config repoConfig, String strategyName)
+ throws InvalidMergeStrategyException {
Merger m = newMerger(inserter, repoConfig, strategyName);
checkArgument(
m instanceof ThreeWayMerger,
@@ -899,9 +904,12 @@
return (ThreeWayMerger) m;
}
- public static Merger newMerger(ObjectInserter inserter, Config repoConfig, String strategyName) {
+ public static Merger newMerger(ObjectInserter inserter, Config repoConfig, String strategyName)
+ throws InvalidMergeStrategyException {
MergeStrategy strategy = MergeStrategy.get(strategyName);
- checkArgument(strategy != null, "invalid merge strategy: %s", strategyName);
+ if (strategy == null) {
+ throw new InvalidMergeStrategyException(strategyName);
+ }
return strategy.newMerger(
new ObjectInserter.Filter() {
@Override
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index f4a8b75..310f71b 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -27,6 +27,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.SubmitType;
@@ -339,8 +340,8 @@
bu.execute();
}
return ins.getChange();
- } catch (IllegalArgumentException e) {
- throw new BadRequestException(e.getMessage(), e);
+ } catch (InvalidMergeStrategyException e) {
+ throw new BadRequestException(e.getMessage());
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/CheckMergeability.java b/java/com/google/gerrit/server/restapi/project/CheckMergeability.java
index 69a6da8..4864fde 100644
--- a/java/com/google/gerrit/server/restapi/project/CheckMergeability.java
+++ b/java/com/google/gerrit/server/restapi/project/CheckMergeability.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.project;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.MergeableInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -120,7 +121,7 @@
result.conflicts = ((ResolveMerger) m).getUnmergedPaths();
}
}
- } catch (IllegalArgumentException e) {
+ } catch (InvalidMergeStrategyException e) {
throw new BadRequestException(e.getMessage());
}
return Response.ok(result);
diff --git a/plugins/replication b/plugins/replication
index 38a027f..b3ed3c8 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 38a027fb3013c9ed28c5b2a1703e32975785f8b7
+Subproject commit b3ed3c8f9a8bab51beed794de5be8fd6da44231a
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();
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index 8687e77..779e247 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -373,7 +373,7 @@
const comment = e.detail.comment;
const msg = comment.message;
const quoteStr = '> ' + msg.replace(NEWLINE_PATTERN, '\n> ') + '\n\n';
- const response = quoteStr + 'Please Fix';
+ const response = quoteStr + 'Please fix.';
this._createReplyComment(comment, response, false, true);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
index 17e0b1f..ceadae1 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
@@ -420,7 +420,7 @@
});
assert.equal(drafts.length, 1);
assert.equal(
- drafts[0].message, '> is this a crossover episode!?\n\nPlease Fix');
+ drafts[0].message, '> is this a crossover episode!?\n\nPlease fix.');
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
assert.isTrue(drafts[0].unresolved);
done();
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
index 3111475..81dfaeb 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
@@ -938,7 +938,7 @@
__commentSide: 'right',
line: 10,
in_reply_to: 'eb0d03fd_5e95904f',
- message: '> This is a robot comment with a fix.\n\nPlease Fix',
+ message: '> This is a robot comment with a fix.\n\nPlease fix.',
unresolved: true,
},
];
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 6a29da6..e16c0a0 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -23,8 +23,8 @@
maven_jar(
name = "dropwizard-core",
- artifact = "io.dropwizard.metrics:metrics-core:4.1.1",
- sha1 = "ebfafc716d9c3b6151dc7c2c09ce925a163a4f21",
+ artifact = "io.dropwizard.metrics:metrics-core:4.1.2",
+ sha1 = "bba231bbf3024c19e75622ec168821cbbd4261a4",
)
SSHD_VERS = "2.3.0"
