Merge "Add undefined check to gr-access-behavior"
diff --git a/Documentation/intro-gerrit-walkthrough.txt b/Documentation/intro-gerrit-walkthrough.txt
index 1fba1dc..b4f799c2 100644
--- a/Documentation/intro-gerrit-walkthrough.txt
+++ b/Documentation/intro-gerrit-walkthrough.txt
@@ -28,7 +28,7 @@
modify. To get this code, he runs the following `git clone` command:
----
-clone ssh://gerrithost:29418/RecipeBook.git RecipeBook
+git clone ssh://gerrithost:29418/RecipeBook.git RecipeBook
----
After he clones the repository, he runs a couple of commands to add a
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index f69c4ae..76151b4 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3058,12 +3058,12 @@
|Field Name | |Description
|`match` | |A JavaScript regular expression to match
positions to be replaced with a hyperlink, as documented in
-link#config-gerrit.html#commentlink.name.match[commentlink.name.match].
+link:config-gerrit.html#commentlink.name.match[commentlink.name.match].
|`link` | |The URL to direct the user to whenever the
regular expression is matched, as documented in
-link#config-gerrit.html#commentlink.name.link[commentlink.name.link].
+link:config-gerrit.html#commentlink.name.link[commentlink.name.link].
|`enabled` |optional|Whether the commentlink is enabled, as documented
-in link#config-gerrit.html#commentlink.name.enabled[
+in link:config-gerrit.html#commentlink.name.enabled[
commentlink.name.enabled]. If not set the commentlink is enabled.
|==================================================
diff --git a/java/com/google/gerrit/json/BUILD b/java/com/google/gerrit/json/BUILD
index 7282dc4..030dddc 100644
--- a/java/com/google/gerrit/json/BUILD
+++ b/java/com/google/gerrit/json/BUILD
@@ -4,5 +4,6 @@
visibility = ["//visibility:public"],
deps = [
"//lib:gson",
+ "//lib/flogger:api",
],
)
diff --git a/java/com/google/gerrit/json/EnumTypeAdapterFactory.java b/java/com/google/gerrit/json/EnumTypeAdapterFactory.java
new file mode 100644
index 0000000..dc74f67
--- /dev/null
+++ b/java/com/google/gerrit/json/EnumTypeAdapterFactory.java
@@ -0,0 +1,78 @@
+// 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.json;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gson.Gson;
+import com.google.gson.TypeAdapter;
+import com.google.gson.TypeAdapterFactory;
+import com.google.gson.internal.bind.TypeAdapters;
+import com.google.gson.reflect.TypeToken;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonToken;
+import com.google.gson.stream.JsonWriter;
+import java.io.IOException;
+
+/**
+ * A {@code TypeAdapterFactory} for enums.
+ *
+ * <p>This factory introduces a wrapper around Gson's own default enum handler to add the following
+ * special behavior: log when input which doesn't match any existing enum value is encountered.
+ */
+public class EnumTypeAdapterFactory implements TypeAdapterFactory {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ @SuppressWarnings({"unchecked"})
+ @Override
+ public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
+ TypeAdapter<T> defaultEnumAdapter = TypeAdapters.ENUM_FACTORY.create(gson, typeToken);
+ if (defaultEnumAdapter == null) {
+ // Not an enum. -> Enum type adapter doesn't apply.
+ return null;
+ }
+
+ return (TypeAdapter<T>) new EnumTypeAdapter(defaultEnumAdapter, typeToken);
+ }
+
+ private static class EnumTypeAdapter<T extends Enum<T>> extends TypeAdapter<T> {
+
+ private final TypeAdapter<T> defaultEnumAdapter;
+ private final TypeToken<T> typeToken;
+
+ public EnumTypeAdapter(TypeAdapter<T> defaultEnumAdapter, TypeToken<T> typeToken) {
+ this.defaultEnumAdapter = defaultEnumAdapter;
+ this.typeToken = typeToken;
+ }
+
+ @Override
+ public T read(JsonReader in) throws IOException {
+ // Still handle null values. -> Check them first.
+ if (in.peek() == JsonToken.NULL) {
+ in.nextNull();
+ return null;
+ }
+ T enumValue = defaultEnumAdapter.read(in);
+ if (enumValue == null) {
+ logger.atWarning().log("Expected an existing value for enum %s.", typeToken);
+ }
+ return enumValue;
+ }
+
+ @Override
+ public void write(JsonWriter out, T value) throws IOException {
+ defaultEnumAdapter.write(out, value);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/json/OutputFormat.java b/java/com/google/gerrit/json/OutputFormat.java
index a2d174f..3e7c319 100644
--- a/java/com/google/gerrit/json/OutputFormat.java
+++ b/java/com/google/gerrit/json/OutputFormat.java
@@ -55,7 +55,8 @@
GsonBuilder gb =
new GsonBuilder()
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
- .registerTypeAdapter(Timestamp.class, new SqlTimestampDeserializer());
+ .registerTypeAdapter(Timestamp.class, new SqlTimestampDeserializer())
+ .registerTypeAdapterFactory(new EnumTypeAdapterFactory());
if (this == OutputFormat.JSON) {
gb.setPrettyPrinting();
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
index 21bbbd2..68841da 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
@@ -63,9 +63,6 @@
// Maximum number of prior states we inspect to find a base for differential. If no cached state
// is found within this number of parents, we fall back to reading everything from scratch.
private static final int MAX_HISTORY_LOOKBACK = 10;
- // Maximum number of changes we perform using the differential approach. If more updates need to
- // be applied, we fall back to reading everything from scratch.
- private static final int MAX_DIFF_UPDATES = 50;
private final ExternalIdReader externalIdReader;
private final Provider<Cache<ObjectId, AllExternalIds>> externalIdCache;
diff --git a/java/com/google/gerrit/server/plugincontext/PluginContext.java b/java/com/google/gerrit/server/plugincontext/PluginContext.java
index 266eb92..90d56c8 100644
--- a/java/com/google/gerrit/server/plugincontext/PluginContext.java
+++ b/java/com/google/gerrit/server/plugincontext/PluginContext.java
@@ -272,7 +272,7 @@
Throwables.throwIfUnchecked(e);
pluginMetrics.incrementErrorCount(extension);
logger.atWarning().withCause(e).log(
- "Failure in %s of plugin invoke%s", extensionImpl.getClass(), extension.getPluginName());
+ "Failure in %s of plugin %s", extensionImpl.getClass(), extension.getPluginName());
}
}
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index 695feba..f50f857 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -291,7 +291,7 @@
.addTag(RequestId.Type.TRACE_ID, "retry-on-failure-" + new RequestId())
.forceLogging();
logger.atFine().withCause(t).log(
- "%s failed, retry with tracing eanbled",
+ "%s failed, retry with tracing enabled",
opts.caller().map(Class::getSimpleName).orElse("N/A"));
return true;
}
diff --git a/javatests/com/google/gerrit/json/JsonEnumMappingTest.java b/javatests/com/google/gerrit/json/JsonEnumMappingTest.java
new file mode 100644
index 0000000..6e57b01
--- /dev/null
+++ b/javatests/com/google/gerrit/json/JsonEnumMappingTest.java
@@ -0,0 +1,84 @@
+// 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.json;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gson.Gson;
+import org.junit.Test;
+
+public class JsonEnumMappingTest {
+
+ // Use the regular, pre-configured Gson object we use throughout the Gerrit server to ensure that
+ // the EnumTypeAdapterFactory is properly set up.
+ private final Gson gson = OutputFormat.JSON.newGson();
+
+ @Test
+ public void nullCanBeWrittenAndParsedBack() {
+ String resultingJson = gson.toJson(null, TestEnum.class);
+ TestEnum value = gson.fromJson(resultingJson, TestEnum.class);
+ assertThat(value).isNull();
+ }
+
+ @Test
+ public void enumValueCanBeWrittenAndParsedBack() {
+ String resultingJson = gson.toJson(TestEnum.ONE, TestEnum.class);
+ TestEnum value = gson.fromJson(resultingJson, TestEnum.class);
+ assertThat(value).isEqualTo(TestEnum.ONE);
+ }
+
+ @Test
+ public void enumValueCanBeParsed() {
+ TestData data = gson.fromJson("{\"value\":\"ONE\"}", TestData.class);
+ assertThat(data.value).isEqualTo(TestEnum.ONE);
+ }
+
+ @Test
+ public void mixedCaseEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"oNe\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ @Test
+ public void lowerCaseEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"one\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ @Test
+ public void notExistingEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"FOUR\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ @Test
+ public void emptyEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ private static class TestData {
+ TestEnum value;
+
+ public TestData(TestEnum value) {
+ this.value = value;
+ }
+ }
+
+ private enum TestEnum {
+ ONE,
+ TWO
+ }
+}
diff --git a/plugins/plugin-manager b/plugins/plugin-manager
index f681e7e..833889d 160000
--- a/plugins/plugin-manager
+++ b/plugins/plugin-manager
@@ -1 +1 @@
-Subproject commit f681e7ecb6ddbc52fe9e07cf3672ccdcad7d7d0b
+Subproject commit 833889d327a159b5ccea7064f4fcff3f94d4b26e
diff --git a/plugins/webhooks b/plugins/webhooks
index 8078749..f860a0c 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit 8078749bfd64d9dcd3372bc3f8965d192747c5b4
+Subproject commit f860a0cf6931164a6e5c2b333eaa0004ea14acec
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
index 4d7fd0c..5b728eb 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
@@ -198,10 +198,6 @@
this.reload();
},
- _computeSelectedTitle(params) {
- return this.getSelectedTitle(params.view);
- },
-
// TODO (beckysiegel): Update these functions after router abstraction is
// updated. They are currently copied from gr-dropdown (and should be
// updated there as well once complete).
@@ -228,6 +224,7 @@
* @param {string=} opt_detailType
*/
_computeSelectedClass(itemView, params, opt_detailType) {
+ if (!params) return '';
// Group params are structured differently from admin params. Compute
// selected differently for groups.
// TODO(wyatta): Simplify this when all routes work like group params.
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 e69d48f..1e50e4b 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
@@ -176,7 +176,7 @@
on-cancel="_handleConfirmDialogCancel"
branch="[[change.branch]]"
has-parent="[[hasParent]]"
- rebase-on-current="[[revisionActions.rebase.rebaseOnCurrent]]"
+ rebase-on-current="[[_revisionRebaseAction.rebaseOnCurrent]]"
hidden></gr-confirm-rebase-dialog>
<gr-confirm-cherrypick-dialog id="confirmCherrypick"
class="confirmDialog"
@@ -216,7 +216,7 @@
id="confirmSubmitDialog"
class="confirmDialog"
change="[[change]]"
- action="[[revisionActions.submit]]"
+ action="[[_revisionSubmitAction]]"
on-cancel="_handleConfirmDialogCancel"
on-confirm="_handleSubmitConfirm" hidden></gr-confirm-submit-dialog>
<gr-dialog id="createFollowUpDialog"
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 83a454b..a448377 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
@@ -271,6 +271,20 @@
type: Object,
value() { return {}; },
},
+ // If property binds directly to [[revisionActions.submit]] it is not
+ // updated when revisionActions doesn't contain submit action.
+ /** @type {?} */
+ _revisionSubmitAction: {
+ type: Object,
+ computed: '_getSubmitAction(revisionActions)',
+ },
+ // If property binds directly to [[revisionActions.rebase]] it is not
+ // updated when revisionActions doesn't contain rebase action.
+ /** @type {?} */
+ _revisionRebaseAction: {
+ type: Object,
+ computed: '_getRebaseAction(revisionActions)',
+ },
privateByDefault: String,
_loading: {
@@ -415,6 +429,28 @@
this._handleLoadingComplete();
},
+ _getSubmitAction(revisionActions) {
+ return this._getRevisionAction(revisionActions, 'submit', null);
+ },
+
+ _getRebaseAction(revisionActions) {
+ return this._getRevisionAction(revisionActions, 'rebase',
+ {rebaseOnCurrent: null}
+ );
+ },
+
+ _getRevisionAction(revisionActions, actionName, emptyActionValue) {
+ if (!revisionActions) {
+ return undefined;
+ }
+ if (revisionActions[actionName] === undefined) {
+ // Return null to fire an event when reveisionActions was loaded
+ // but doesn't contain actionName. undefined doesn't fire an event
+ return emptyActionValue;
+ }
+ return revisionActions[actionName];
+ },
+
reload() {
if (!this.changeNum || !this.latestPatchNum) {
return Promise.resolve();
@@ -575,7 +611,9 @@
_deleteAndNotify(actionName) {
if (this.actions[actionName]) {
delete this.actions[actionName];
- this.notifyPath('actions.' + actionName);
+ // We assign a fake value of 'false' to support Polymer 2
+ // see https://github.com/Polymer/polymer/issues/2631
+ this.notifyPath('actions.' + actionName, false);
}
},
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 184e175..b88e06b 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
@@ -1513,4 +1513,50 @@
assert.equal(reportStub.lastCall.args[0], 'type-key');
});
});
+
+ suite('getChangeRevisionActions returns only some actions', () => {
+ let element;
+ let sandbox;
+ let changeRevisionActions;
+
+ setup(() => {
+ stub('gr-rest-api-interface', {
+ getChangeRevisionActions() {
+ return Promise.resolve(changeRevisionActions);
+ },
+ send(method, url, payload) {
+ return Promise.reject(new Error('error'));
+ },
+ getProjectConfig() { return Promise.resolve({}); },
+ });
+
+ sandbox = sinon.sandbox.create();
+ sandbox.stub(Gerrit, 'awaitPluginsLoaded').returns(Promise.resolve());
+
+ element = fixture('basic');
+ // getChangeRevisionActions is not called without
+ // set the following properies
+ element.change = {};
+ element.changeNum = '42';
+ element.latestPatchNum = '2';
+
+
+ sandbox.stub(element.$.confirmCherrypick.$.restAPI,
+ 'getRepoBranches').returns(Promise.resolve([]));
+ sandbox.stub(element.$.confirmMove.$.restAPI,
+ 'getRepoBranches').returns(Promise.resolve([]));
+ return element.reload();
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('confirmSubmitDialog and confirmRebase properties are changed', () => {
+ changeRevisionActions = {};
+ element.reload();
+ assert.strictEqual(element.$.confirmSubmitDialog.action, null);
+ assert.strictEqual(element.$.confirmRebase.rebaseOnCurrent, null);
+ });
+ });
</script>
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index 97e9bdc..ca494d2 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -206,7 +206,7 @@
document.dispatchEvent(new CustomEvent(type, {detail}));
if (opt_noLog) { return; }
if (type === ERROR.TYPE && category === ERROR.CATEGORY) {
- console.error(eventValue.error || eventName);
+ console.error(eventValue && eventValue.error || eventName);
} else {
if (eventValue !== undefined) {
console.log(`Reporting: ${eventName}: ${eventValue}`);
@@ -228,7 +228,7 @@
*/
cachingReporter(type, category, eventName, eventValue, opt_noLog) {
if (type === ERROR.TYPE && category === ERROR.CATEGORY) {
- console.error(eventValue.error || eventName);
+ console.error(eventValue && eventValue.error || eventName);
}
if (this._isMetricsPluginLoaded()) {
if (pending.length) {
diff --git a/polygerrit-ui/app/elements/shared/revision-info/revision-info.html b/polygerrit-ui/app/elements/shared/revision-info/revision-info.html
index d337153..fca8ae1 100644
--- a/polygerrit-ui/app/elements/shared/revision-info/revision-info.html
+++ b/polygerrit-ui/app/elements/shared/revision-info/revision-info.html
@@ -35,6 +35,9 @@
* @return {Number}
*/
RevisionInfo.prototype.getMaxParents = function() {
+ if (!this._change || !this._change.revisions) {
+ return 0;
+ }
return Object.values(this._change.revisions)
.reduce((acc, rev) => Math.max(rev.commit.parents.length, acc), 0);
};
@@ -46,6 +49,9 @@
*/
RevisionInfo.prototype.getParentCountMap = function() {
const result = {};
+ if (!this._change || !this._change.revisions) {
+ return {};
+ }
Object.values(this._change.revisions)
.forEach(rev => { result[rev._number] = rev.commit.parents.length; });
return result;
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 9448662..2b052a0 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -197,7 +197,6 @@
for (let file of elements) {
file = elementsPath + file;
testFiles.push(file);
- testFiles.push(file + '?dom=shadow');
}
// Behaviors tests.
