Merge branch 'stable-2.16' into stable-3.0

* stable-2.16: Fix gr-label-info test Set version to 2.16.21-SNAPSHOT Set version to 2.16.20 Trim parameterized strings evaluated from LdapRealm Update account full name when it changes in LDAP Set Api version for plugin jars Change-Id: I1fbcf22a821b86d9452947df19df155138877e3b
2020-05-31 10:34:00 +09:00
parent 7c6f3ab5ce f34f4b3b2d
commit c3cd09aa55
7 changed files with 104 additions and 13 deletions
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -17,6 +17,7 @@ package com.google.gerrit.server.account;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;

+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -77,8 +78,9 @@ public class AccountManager {
  private final boolean autoUpdateAccountActiveStatus;
  private final SetInactiveFlag setInactiveFlag;

+  @VisibleForTesting
  @Inject
-  AccountManager(
+  public AccountManager(
      Sequences sequences,
      @GerritServerConfig Config cfg,
      Accounts accounts,
@@ -238,13 +240,7 @@ public class AccountManager {

    if (!Strings.isNullOrEmpty(who.getDisplayName())
        && !Objects.equals(user.getAccount().getFullName(), who.getDisplayName())) {
-      if (realm.allowsEdit(AccountFieldName.FULL_NAME)) {
      accountUpdates.add(a -> a.setFullName(who.getDisplayName()));
-      } else {
-        logger.atWarning().log(
-            "Not changing already set display name '%s' to '%s'",
-            user.getAccount().getFullName(), who.getDisplayName());
-      }
    }

    if (!realm.allowsEdit(AccountFieldName.USER_NAME)
--- a/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
+++ b/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
@@ -217,7 +217,7 @@ class LdapRealm extends AbstractRealm {
      values.put(name, m.get(name));
    }

-    String r = p.replace(values);
+    String r = p.replace(values).trim();
    return r.isEmpty() ? null : r;
  }

--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
@@ -22,7 +22,9 @@ import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.AccountFieldName;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.ServerInitiated;
 import com.google.gerrit.server.account.AccountException;
 import com.google.gerrit.server.account.AccountManager;
@@ -30,12 +32,16 @@ import com.google.gerrit.server.account.AccountState;
 import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.AuthRequest;
 import com.google.gerrit.server.account.AuthResult;
+import com.google.gerrit.server.account.SetInactiveFlag;
 import com.google.gerrit.server.account.externalids.ExternalId;
 import com.google.gerrit.server.account.externalids.ExternalIdNotes;
 import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.group.db.GroupsUpdate;
 import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.server.ssh.SshKeyCache;
 import com.google.inject.Inject;
+import com.google.inject.util.Providers;
 import java.util.Optional;
 import java.util.Set;
 import org.eclipse.jgit.lib.Repository;
@@ -48,6 +54,12 @@ public class AccountManagerIT extends AbstractDaemonTest {
  @Inject @ServerInitiated private AccountsUpdate accountsUpdate;
  @Inject private ExternalIdNotes.Factory extIdNotesFactory;

+  @Inject private Sequences sequences;
+  @Inject private IdentifiedUser.GenericFactory userFactory;
+  @Inject private SshKeyCache sshKeyCache;
+  @Inject private GroupsUpdate.Factory groupsUpdateFactory;
+  @Inject private SetInactiveFlag setInactiveFlag;
+
  @Test
  public void authenticateNewAccountWithEmail() throws Exception {
    String email = "foo@example.com";
@@ -197,6 +209,31 @@ public class AccountManagerIT extends AbstractDaemonTest {

  @Test
  public void authenticateWithUsernameAndUpdateDisplayName() throws Exception {
+    authenticateWithUsernameAndUpdateDisplayName(accountManager);
+  }
+
+  @Test
+  public void readOnlyFullNameField_authenticateWithUsernameAndUpdateDisplayName()
+      throws Exception {
+    TestRealm realm = server.getTestInjector().getInstance(TestRealm.class);
+    realm.denyEdit(AccountFieldName.FULL_NAME);
+    authenticateWithUsernameAndUpdateDisplayName(
+        new AccountManager(
+            sequences,
+            cfg,
+            accounts,
+            Providers.of(accountsUpdate),
+            accountCache,
+            realm,
+            userFactory,
+            sshKeyCache,
+            projectCache,
+            externalIds,
+            groupsUpdateFactory,
+            setInactiveFlag));
+  }
+
+  private void authenticateWithUsernameAndUpdateDisplayName(AccountManager am) throws Exception {
    String username = "foo";
    String email = "foo@example.com";
    Account.Id accountId = new Account.Id(seq.nextAccountId());
@@ -212,7 +249,7 @@ public class AccountManagerIT extends AbstractDaemonTest {
    AuthRequest who = AuthRequest.forUser(username);
    String newName = "Updated Name";
    who.setDisplayName(newName);
-    AuthResult authResult = accountManager.authenticate(who);
+    AuthResult authResult = am.authenticate(who);
    assertAuthResultForExistingAccount(authResult, accountId, gerritExtIdKey);

    Optional<AccountState> accountState = accounts.get(accountId);
--- a/javatests/com/google/gerrit/acceptance/api/accounts/BUILD
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/BUILD
@@ -1,3 +1,4 @@
+load("@rules_java//java:defs.bzl", "java_library")
 load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")

 acceptance_tests(
@@ -9,8 +10,18 @@ acceptance_tests(
        "no_windows",
    ],
    deps = [
+        ":util",
        "//java/com/google/gerrit/git",
        "//java/com/google/gerrit/mail",
        "//java/com/google/gerrit/server/util/time",
    ],
 )
+
+java_library(
+    name = "util",
+    testonly = True,
+    srcs = glob(["TestRealm.java"]),
+    deps = [
+        "//java/com/google/gerrit/acceptance:lib",
+    ],
+)
--- a/javatests/com/google/gerrit/acceptance/api/accounts/TestRealm.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/TestRealm.java
@@ -0,0 +1,46 @@
+// Copyright (C) 2020 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.acceptance.api.accounts;
+
+import com.google.gerrit.extensions.client.AccountFieldName;
+import com.google.gerrit.server.account.DefaultRealm;
+import com.google.gerrit.server.account.EmailExpander;
+import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.config.AuthConfig;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.util.HashSet;
+import java.util.Set;
+
+@Singleton
+public class TestRealm extends DefaultRealm {
+
+  private final Set<AccountFieldName> readOnlyFields = new HashSet<>();
+
+  @Inject
+  public TestRealm(EmailExpander emailExpander, Provider<Emails> emails, AuthConfig authConfig) {
+    super(emailExpander, emails, authConfig);
+  }
+
+  public void denyEdit(AccountFieldName field) {
+    readOnlyFields.add(field);
+  }
+
+  @Override
+  public boolean allowsEdit(AccountFieldName field) {
+    return !readOnlyFields.contains(field);
+  }
+}
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.html
@@ -229,9 +229,9 @@ limitations under the License.
      assert.isTrue(isHidden(element.$$('.placeholder')));
      element.labelInfo = {values: [], rejected: [], all: [{value: 1}]};
      assert.isTrue(isHidden(element.$$('.placeholder')));
-      element.labelInfo = {accepted: []};
+      element.labelInfo = {approved: []};
      assert.isTrue(isHidden(element.$$('.placeholder')));
-      element.labelInfo = {values: [], accepted: [], all: [{value: 1}]};
+      element.labelInfo = {values: [], approved: [], all: [{value: 1}]};
      assert.isTrue(isHidden(element.$$('.placeholder')));
    });
  });
--- a/tools/bzl/plugin.bzl
+++ b/tools/bzl/plugin.bzl
@@ -1,5 +1,6 @@
 load("@rules_java//java:defs.bzl", "java_binary", "java_library")
 load("//tools/bzl:genrule2.bzl", "genrule2")
+load("//:version.bzl", "GERRIT_VERSION")

 PLUGIN_DEPS = ["//plugins:plugin-lib"]

@@ -61,7 +62,7 @@ def gerrit_plugin(
            "GEN_VERSION=$$(cat bazel-out/stable-status.txt | grep -w STABLE_BUILD_%s_LABEL | cut -d ' ' -f 2)" % dir_name.upper(),
            "cd $$TMP",
            "unzip -q $$ROOT/$<",
-            "echo \"Implementation-Version: $$GEN_VERSION\n$$(cat META-INF/MANIFEST.MF)\" > META-INF/MANIFEST.MF",
+            "echo \"Implementation-Version: $$GEN_VERSION\nGerrit-ApiVersion: " + GERRIT_VERSION + "\n$$(cat META-INF/MANIFEST.MF)\" > META-INF/MANIFEST.MF",
            "find . -exec touch '{}' ';'",
            "zip -Xqr $$ROOT/$@ .",
        ]),