diff options
13 files changed, 262 insertions, 13 deletions
diff --git a/Documentation/intro-gerrit-walkthrough.txt b/Documentation/intro-gerrit-walkthrough.txt index 1fba1dc04b..b4f799c220 100644 --- a/Documentation/intro-gerrit-walkthrough.txt +++ b/Documentation/intro-gerrit-walkthrough.txt @@ -28,7 +28,7 @@ project he works on. His first step is to get the source code that he wants to 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 f69c4ae52f..76151b40e0 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -3058,12 +3058,12 @@ link:config-gerrit.html#commentlink[commentlink]. |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 7282dc46ed..030dddcb63 100644 --- a/java/com/google/gerrit/json/BUILD +++ b/java/com/google/gerrit/json/BUILD @@ -4,5 +4,6 @@ java_library( 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 0000000000..dc74f67dbf --- /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 a2d174f4cc..3e7c319aa7 100644 --- a/java/com/google/gerrit/json/OutputFormat.java +++ b/java/com/google/gerrit/json/OutputFormat.java @@ -55,7 +55,8 @@ public enum OutputFormat { 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/plugincontext/PluginContext.java b/java/com/google/gerrit/server/plugincontext/PluginContext.java index 266eb9228a..90d56c8903 100644 --- a/java/com/google/gerrit/server/plugincontext/PluginContext.java +++ b/java/com/google/gerrit/server/plugincontext/PluginContext.java @@ -272,7 +272,7 @@ public class PluginContext<T> { 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 695febab11..f50f857239 100644 --- a/java/com/google/gerrit/server/update/RetryHelper.java +++ b/java/com/google/gerrit/server/update/RetryHelper.java @@ -291,7 +291,7 @@ public class RetryHelper { .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 0000000000..6e57b011ef --- /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/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js index 08b59c2c2b..fded936fb0 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js +++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js @@ -309,6 +309,9 @@ }, _computeCommands(repo, schemesObj, _selectedScheme) { + if (!schemesObj || !repo || !_selectedScheme) { + return []; + } const commands = []; let commandObj; if (schemesObj.hasOwnProperty(_selectedScheme)) { 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 e69d48f13f..1e50e4bfba 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 @@ limitations under the License. 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 @@ limitations under the License. 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 5a35c435ff..a448377028 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(); 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 184e175419..b88e06b3d8 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 @@ limitations under the License. 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/shared/gr-list-view/gr-list-view.js b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js index 53d05e1044..2508196650 100644 --- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js +++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js @@ -90,11 +90,11 @@ }, _hideNextArrow(loading, items) { - let lastPage = false; - if (items.length < this.itemsPerPage + 1) { - lastPage = true; + if (loading || !items || !items.length) { + return true; } - return loading || lastPage || !items || !items.length; + const lastPage = items.length < this.itemsPerPage + 1; + return lastPage; }, }); })(); |