From 75d618b48365543cd6e872661f4f9492d7e1260f Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Tue, 21 Jan 2020 18:13:40 -0500 Subject: [PATCH 1/2] ElasticContainer: Upgrade V7_5 to elasticsearch 7.5.2 Upgrade elasticsearch-rest-client to 7.5.2, accordingly. Change-Id: I39059811c2ae2167e41f9f3851446dbbde3cf1e4 --- .../com/google/gerrit/elasticsearch/ElasticContainer.java | 2 +- tools/nongoogle.bzl | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index c67a842bea..53acdeb020 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -63,7 +63,7 @@ public class ElasticContainer extends ElasticsearchContainer { case V7_4: return "blacktop/elasticsearch:7.4.2"; case V7_5: - return "blacktop/elasticsearch:7.5.1"; + return "blacktop/elasticsearch:7.5.2"; } throw new IllegalStateException("No tests for version: " + version.name()); } diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl index 82022a73d5..4ffcba6891 100644 --- a/tools/nongoogle.bzl +++ b/tools/nongoogle.bzl @@ -94,8 +94,8 @@ def declare_nongoogle_deps(): # and httpasyncclient as necessary. maven_jar( name = "elasticsearch-rest-client", - artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.1", - sha1 = "094c155906dc94146fc5adc344ea2c676d487cf2", + artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.2", + sha1 = "e11393f600a425b7f62e6f653e19a9e53556fd79", ) maven_jar( From 4839f01dd37e305095c45ce35a59978aee729ffd Mon Sep 17 00:00:00 2001 From: Ben Rohlfs Date: Wed, 22 Jan 2020 08:39:22 +0100 Subject: [PATCH 2/2] Fix handling of LDAP groups that contain dots Polymer's property path system has no way to handle dots in property names, so using this.set() will result in splitting up the group name into multiple path parts. Tweaked the tests to expose this problem and added ample comments to the code. Bug: Issue 11980 Change-Id: I96d6dfa50908ddafb4d0fa12d247d33444c36b4e --- .../app/elements/admin/gr-permission/gr-permission.js | 10 ++++++++-- .../admin/gr-permission/gr-permission_test.html | 10 +++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js index 633a1db226..e33c1ec478 100644 --- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js +++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js @@ -252,7 +252,12 @@ // The group id is encoded, but have to decode in order for the access // API to work as expected. const groupId = decodeURIComponent(e.detail.value.id).replace(/\+/g, ' '); - this.set(['permission', 'value', 'rules', groupId], {}); + // We cannot use "this.set(...)" here, because groupId may contain dots, + // and dots in property path names are totally unsupported by Polymer. + // Apparently Polymer picks up this change anyway, otherwise we should + // have looked at using MutableData: + // https://polymer-library.polymer-project.org/2.0/docs/devguide/data-system#mutable-data + this.permission.value.rules[groupId] = {}; // Purposely don't recompute sorted array so that the newly added rule // is the last item of the array. @@ -273,7 +278,8 @@ Polymer.dom.flush(); const value = this._rules[this._rules.length - 1].value; value.added = true; - this.set(['permission', 'value', 'rules', groupId], value); + // See comment above for why we cannot use "this.set(...)" here. + this.permission.value.rules[groupId] = value; this.dispatchEvent(new CustomEvent('access-modified', {bubbles: true})); }, diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html index 5f1f9b5306..a432ab0b20 100644 --- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html +++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html @@ -310,11 +310,11 @@ limitations under the License. element.name = 'Priority'; element.section = 'refs/*'; element.groups = {}; - element.$.groupAutocomplete.text = 'ldap/tests tests'; + element.$.groupAutocomplete.text = 'ldap/tests te.st'; const e = { detail: { value: { - id: 'ldap:CN=test+test', + id: 'ldap:CN=test+te.st', }, }, }; @@ -323,11 +323,11 @@ limitations under the License. assert.equal(Object.keys(element._groupsWithRules).length, 2); element._handleAddRuleItem(e); flushAsynchronousOperations(); - assert.deepEqual(element.groups, {'ldap:CN=test test': { - name: 'ldap/tests tests'}}); + assert.deepEqual(element.groups, {'ldap:CN=test te.st': { + name: 'ldap/tests te.st'}}); assert.equal(element._rules.length, 3); assert.equal(Object.keys(element._groupsWithRules).length, 3); - assert.deepEqual(element.permission.value.rules['ldap:CN=test test'], + assert.deepEqual(element.permission.value.rules['ldap:CN=test te.st'], {action: 'ALLOW', min: -2, max: 2, added: true}); // New rule should be removed if cancel from editing. element.editing = false;