Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  Expose jetty threadpool metrics via dropwizard
  Fix editing name & email for service user
  Fix accountBelongsToRealm implementation
  Fix support for python3 in download_bower.py

Change-Id: I4e2d984af74eac279ad22d11f3e606c3ef361c94

java/com/google/gerrit/metrics/dropwizard/BUILD

@@ -8,6 +8,7 @@ java_library(
         "//java/com/google/gerrit/common:annotations",
         "//java/com/google/gerrit/extensions:api",
         "//java/com/google/gerrit/metrics",
+        "//java/com/google/gerrit/pgm/http/jetty",
         "//java/com/google/gerrit/server",
         "//lib:args4j",
         "//lib:guava",

java/com/google/gerrit/pgm/http/jetty/BUILD

@@ -10,6 +10,7 @@ java_library(
         "//java/com/google/gerrit/httpd",
         "//java/com/google/gerrit/launcher",
         "//java/com/google/gerrit/lifecycle",
+        "//java/com/google/gerrit/metrics",
         "//java/com/google/gerrit/server",
         "//java/com/google/gerrit/server/util/time",
         "//java/com/google/gerrit/sshd",

java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java

@@ -0,0 +1,91 @@
+// 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.pgm.http.jetty;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.metrics.CallbackMetric;
+import com.google.gerrit.metrics.CallbackMetric0;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+@Singleton
+public class JettyMetrics {
+
+  @Inject
+  JettyMetrics(JettyServer jetty, MetricMaker metrics) {
+    CallbackMetric0<Integer> minPoolSize =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/min_pool_size",
+            Integer.class,
+            new Description("Minimum thread pool size").setGauge());
+    CallbackMetric0<Integer> maxPoolSize =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/max_pool_size",
+            Integer.class,
+            new Description("Maximum thread pool size").setGauge());
+    CallbackMetric0<Integer> poolSize =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/pool_size",
+            Integer.class,
+            new Description("Current thread pool size").setGauge());
+    CallbackMetric0<Integer> idleThreads =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/idle_threads",
+            Integer.class,
+            new Description("Idle httpd threads").setGauge().setUnit("threads"));
+    CallbackMetric0<Integer> busyThreads =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/active_threads",
+            Integer.class,
+            new Description("Active httpd threads").setGauge().setUnit("threads"));
+    CallbackMetric0<Integer> reservedThreads =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/reserved_threads",
+            Integer.class,
+            new Description("Reserved httpd threads").setGauge().setUnit("threads"));
+    CallbackMetric0<Integer> queueSize =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/queue_size",
+            Integer.class,
+            new Description("Thread pool queue size").setGauge().setUnit("requests"));
+    CallbackMetric0<Boolean> lowOnThreads =
+        metrics.newCallbackMetric(
+            "httpd/jetty/threadpool/is_low_on_threads",
+            Boolean.class,
+            new Description("Whether thread pool is low on threads").setGauge());
+    JettyServer.Metrics jettyMetrics = jetty.getMetrics();
+    metrics.newTrigger(
+        ImmutableSet.<CallbackMetric<?>>of(
+            idleThreads,
+            busyThreads,
+            reservedThreads,
+            minPoolSize,
+            maxPoolSize,
+            poolSize,
+            queueSize,
+            lowOnThreads),
+        () -> {
+          minPoolSize.set(jettyMetrics.getMinThreads());
+          maxPoolSize.set(jettyMetrics.getMaxThreads());
+          poolSize.set(jettyMetrics.getThreads());
+          idleThreads.set(jettyMetrics.getIdleThreads());
+          busyThreads.set(jettyMetrics.getBusyThreads());
+          reservedThreads.set(jettyMetrics.getReservedThreads());
+          queueSize.set(jettyMetrics.getQueueSize());
+          lowOnThreads.set(jettyMetrics.isLowOnThreads());
+        });
+  }
+}

java/com/google/gerrit/pgm/http/jetty/JettyModule.java

@@ -31,5 +31,6 @@ public class JettyModule extends LifecycleModule {
     bind(JettyServer.class);
     listener().to(JettyServer.Lifecycle.class);
     install(new FactoryModuleBuilder().build(HttpLogFactory.class));
+    bind(JettyMetrics.class);
   }
 }

java/com/google/gerrit/pgm/http/jetty/JettyServer.java

@@ -66,7 +66,6 @@ import org.eclipse.jetty.util.BlockingArrayQueue;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
-import org.eclipse.jetty.util.thread.ThreadPool;
 import org.eclipse.jgit.lib.Config;
 
 @Singleton
@@ -115,9 +114,49 @@ public class JettyServer {
     }
   }
 
+  static class Metrics {
+    private final QueuedThreadPool threadPool;
+
+    Metrics(QueuedThreadPool threadPool) {
+      this.threadPool = threadPool;
+    }
+
+    public int getIdleThreads() {
+      return threadPool.getIdleThreads();
+    }
+
+    public int getBusyThreads() {
+      return threadPool.getBusyThreads();
+    }
+
+    public int getReservedThreads() {
+      return threadPool.getReservedThreads();
+    }
+
+    public int getMinThreads() {
+      return threadPool.getMinThreads();
+    }
+
+    public int getMaxThreads() {
+      return threadPool.getMaxThreads();
+    }
+
+    public int getThreads() {
+      return threadPool.getThreads();
+    }
+
+    public int getQueueSize() {
+      return threadPool.getQueueSize();
+    }
+
+    public boolean isLowOnThreads() {
+      return threadPool.isLowOnThreads();
+    }
+  }
+
   private final SitePaths site;
   private final Server httpd;
-
+  private final Metrics metrics;
   private boolean reverseProxy;
 
   @Inject
@@ -129,8 +168,10 @@ public class JettyServer {
       HttpLogFactory httpLogFactory) {
     this.site = site;
 
-    httpd = new Server(threadPool(cfg, threadSettingsConfig));
+    QueuedThreadPool pool = threadPool(cfg, threadSettingsConfig);
+    httpd = new Server(pool);
     httpd.setConnectors(listen(httpd, cfg));
+    metrics = new Metrics(pool);
 
     Handler app = makeContext(env, cfg);
     if (cfg.getBoolean("httpd", "requestLog", !reverseProxy)) {
@@ -159,6 +200,10 @@ public class JettyServer {
     httpd.setStopAtShutdown(false);
   }
 
+  Metrics getMetrics() {
+    return metrics;
+  }
+
   private Connector[] listen(Server server, Config cfg) {
     // OpenID and certain web-based single-sign-on products can cause
     // some very long headers, especially in the Referer header. We
@@ -334,7 +379,7 @@ public class JettyServer {
     return site.resolve(path);
   }
 
-  private ThreadPool threadPool(Config cfg, ThreadSettingsConfig threadSettingsConfig) {
+  private QueuedThreadPool threadPool(Config cfg, ThreadSettingsConfig threadSettingsConfig) {
     int maxThreads = threadSettingsConfig.getHttpdMaxThreads();
     int minThreads = cfg.getInt("httpd", null, "minthreads", 5);
     int maxQueued = cfg.getInt("httpd", null, "maxqueued", 200);

java/com/google/gerrit/server/auth/ldap/LdapRealm.java

@@ -336,7 +336,7 @@ class LdapRealm extends AbstractRealm {
   @Override
   public boolean accountBelongsToRealm(Collection<ExternalId> externalIds) {
     for (ExternalId id : externalIds) {
-      if (id.toString().contains(SCHEME_GERRIT)) {
+      if (id.isScheme(SCHEME_GERRIT)) {
         return true;
       }
     }

java/com/google/gerrit/server/auth/oauth/OAuthRealm.java

@@ -122,7 +122,7 @@ public class OAuthRealm extends AbstractRealm {
   @Override
   public boolean accountBelongsToRealm(Collection<ExternalId> externalIds) {
     for (ExternalId id : externalIds) {
-      if (id.toString().contains(SCHEME_EXTERNAL)) {
+      if (id.isScheme(SCHEME_EXTERNAL)) {
         return true;
       }
     }

java/com/google/gerrit/server/restapi/account/DeleteEmail.java

@@ -24,6 +24,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountException;
@@ -79,12 +80,14 @@ public class DeleteEmail implements RestModifyView<AccountResource.Email, Input>
   public Response<?> apply(IdentifiedUser user, String email)
       throws ResourceNotFoundException, ResourceConflictException, MethodNotAllowedException,
           IOException, ConfigInvalidException {
-    if (!realm.allowsEdit(AccountFieldName.REGISTER_NEW_EMAIL)) {
+    Account.Id accountId = user.getAccountId();
+    if (realm.accountBelongsToRealm(externalIds.byAccount(accountId))
+        && !realm.allowsEdit(AccountFieldName.REGISTER_NEW_EMAIL)) {
       throw new MethodNotAllowedException("realm does not allow deleting emails");
     }
 
     Set<ExternalId> extIds =
-        externalIds.byAccount(user.getAccountId()).stream()
+        externalIds.byAccount(accountId).stream()
             .filter(e -> email.equals(e.email()))
             .collect(toSet());
     if (extIds.isEmpty()) {

java/com/google/gerrit/server/restapi/account/PutName.java

@@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.ServerInitiated;
@@ -29,6 +30,7 @@ import com.google.gerrit.server.account.AccountResource;
 import com.google.gerrit.server.account.AccountState;
 import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -43,6 +45,7 @@ public class PutName implements RestModifyView<AccountResource, NameInput> {
   private final Provider<CurrentUser> self;
   private final Realm realm;
   private final PermissionBackend permissionBackend;
+  private final ExternalIds externalIds;
   private final Provider<AccountsUpdate> accountsUpdateProvider;
 
   @Inject
@@ -50,10 +53,12 @@ public class PutName implements RestModifyView<AccountResource, NameInput> {
       Provider<CurrentUser> self,
       Realm realm,
       PermissionBackend permissionBackend,
+      ExternalIds externalIds,
       @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider) {
     this.self = self;
     this.realm = realm;
     this.permissionBackend = permissionBackend;
+    this.externalIds = externalIds;
     this.accountsUpdateProvider = accountsUpdateProvider;
   }
 
@@ -74,7 +79,9 @@ public class PutName implements RestModifyView<AccountResource, NameInput> {
       input = new NameInput();
     }
 
-    if (!realm.allowsEdit(AccountFieldName.FULL_NAME)) {
+    Account.Id accountId = user.getAccountId();
+    if (realm.accountBelongsToRealm(externalIds.byAccount(accountId))
+        && !realm.allowsEdit(AccountFieldName.FULL_NAME)) {
       throw new MethodNotAllowedException("realm does not allow editing name");
     }
 
@@ -82,7 +89,7 @@ public class PutName implements RestModifyView<AccountResource, NameInput> {
     AccountState accountState =
         accountsUpdateProvider
             .get()
-            .update("Set Full Name via API", user.getAccountId(), u -> u.setFullName(newName))
+            .update("Set Full Name via API", accountId, u -> u.setFullName(newName))
             .orElseThrow(() -> new ResourceNotFoundException("account not found"));
     return Strings.isNullOrEmpty(accountState.getAccount().getFullName())
         ? Response.none()

java/com/google/gerrit/server/restapi/account/PutUsername.java

@@ -78,10 +78,6 @@ public class PutUsername implements RestModifyView<AccountResource, UsernameInpu
       permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
     }
 
-    if (!realm.allowsEdit(AccountFieldName.USER_NAME)) {
-      throw new MethodNotAllowedException("realm does not allow editing username");
-    }
-
     if (input == null) {
       input = new UsernameInput();
     }
@@ -91,6 +87,11 @@ public class PutUsername implements RestModifyView<AccountResource, UsernameInpu
       throw new MethodNotAllowedException("Username cannot be changed.");
     }
 
+    if (realm.accountBelongsToRealm(externalIds.byAccount(accountId))
+        && !realm.allowsEdit(AccountFieldName.USER_NAME)) {
+      throw new MethodNotAllowedException("realm does not allow editing username");
+    }
+
     if (Strings.isNullOrEmpty(input.username)) {
       return input.username;
     }

javatests/com/google/gerrit/server/auth/ldap/LdapRealmTest.java

@@ -0,0 +1,94 @@
+// 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.server.auth.ldap;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GERRIT;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+import static com.google.gerrit.server.auth.ldap.LdapModule.GROUP_CACHE;
+import static com.google.gerrit.server.auth.ldap.LdapModule.GROUP_EXIST_CACHE;
+import static com.google.gerrit.server.auth.ldap.LdapModule.PARENT_GROUPS_CACHE;
+import static com.google.gerrit.server.auth.ldap.LdapModule.USERNAME_CACHE;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.testing.InMemoryModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.TypeLiteral;
+import java.util.Arrays;
+import java.util.Optional;
+import java.util.Set;
+import org.junit.Before;
+import org.junit.Test;
+
+public final class LdapRealmTest {
+  @Inject private LdapRealm ldapRealm = null;
+
+  @Before
+  public void setUpInjector() throws Exception {
+    Injector injector =
+        Guice.createInjector(
+            new InMemoryModule(),
+            new CacheModule() {
+              @Override
+              protected void configure() {
+                cache(GROUP_CACHE, String.class, new TypeLiteral<Set<AccountGroup.UUID>>() {})
+                    .loader(LdapRealm.MemberLoader.class);
+                cache(USERNAME_CACHE, String.class, new TypeLiteral<Optional<Account.Id>>() {})
+                    .loader(LdapRealm.UserLoader.class);
+                cache(GROUP_EXIST_CACHE, String.class, new TypeLiteral<Boolean>() {})
+                    .loader(LdapRealm.ExistenceLoader.class);
+                cache(
+                    PARENT_GROUPS_CACHE, String.class, new TypeLiteral<ImmutableSet<String>>() {});
+              }
+            });
+    injector.injectMembers(this);
+  }
+
+  private ExternalId id(String scheme, String id) {
+    return ExternalId.create(scheme, id, new Account.Id(1000));
+  }
+
+  private boolean accountBelongsToRealm(ExternalId... ids) {
+    return ldapRealm.accountBelongsToRealm(Arrays.asList(ids));
+  }
+
+  private boolean accountBelongsToRealm(String scheme, String id) {
+    return accountBelongsToRealm(id(scheme, id));
+  }
+
+  @Test
+  public void accountBelongsToRealm() throws Exception {
+    assertThat(accountBelongsToRealm(SCHEME_GERRIT, "test")).isTrue();
+    assertThat(accountBelongsToRealm(id(SCHEME_USERNAME, "test"), id(SCHEME_GERRIT, "test")))
+        .isTrue();
+    assertThat(accountBelongsToRealm(id(SCHEME_GERRIT, "test"), id(SCHEME_USERNAME, "test")))
+        .isTrue();
+
+    assertThat(accountBelongsToRealm(SCHEME_USERNAME, "test")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_MAILTO, "foo@bar.com")).isFalse();
+
+    assertThat(accountBelongsToRealm(SCHEME_USERNAME, "gerrit")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_USERNAME, "xxgerritxx")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_MAILTO, "gerrit.foo@bar.com")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_MAILTO, "bar.gerrit@bar.com")).isFalse();
+  }
+}

javatests/com/google/gerrit/server/auth/oauth/OAuthRealmTest.java

@@ -0,0 +1,69 @@
+// 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.server.auth.oauth;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_EXTERNAL;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.testing.InMemoryModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import java.util.Arrays;
+import org.junit.Before;
+import org.junit.Test;
+
+public final class OAuthRealmTest {
+  @Inject private OAuthRealm oauthRealm = null;
+
+  @Before
+  public void setUpInjector() throws Exception {
+    Injector injector = Guice.createInjector(new InMemoryModule());
+    injector.injectMembers(this);
+  }
+
+  private ExternalId id(String scheme, String id) {
+    return ExternalId.create(scheme, id, new Account.Id(1000));
+  }
+
+  private boolean accountBelongsToRealm(ExternalId... ids) {
+    return oauthRealm.accountBelongsToRealm(Arrays.asList(ids));
+  }
+
+  private boolean accountBelongsToRealm(String scheme, String id) {
+    return accountBelongsToRealm(id(scheme, id));
+  }
+
+  @Test
+  public void accountBelongsToRealm() throws Exception {
+    assertThat(accountBelongsToRealm(SCHEME_EXTERNAL, "test")).isTrue();
+    assertThat(accountBelongsToRealm(id(SCHEME_USERNAME, "test"), id(SCHEME_EXTERNAL, "test")))
+        .isTrue();
+    assertThat(accountBelongsToRealm(id(SCHEME_EXTERNAL, "test"), id(SCHEME_USERNAME, "test")))
+        .isTrue();
+
+    assertThat(accountBelongsToRealm(SCHEME_USERNAME, "test")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_MAILTO, "foo@bar.com")).isFalse();
+
+    assertThat(accountBelongsToRealm(SCHEME_USERNAME, "external")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_USERNAME, "xxexternalxx")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_MAILTO, "external.foo@bar.com")).isFalse();
+    assertThat(accountBelongsToRealm(SCHEME_MAILTO, "bar.external@bar.com")).isFalse();
+  }
+}

tools/js/download_bower.py

@@ -46,7 +46,8 @@ def bower_info(bower, name, package, version):
         raise
     out, err = p.communicate()
     if p.returncode:
-        sys.stderr.write(err)
+        # For python3 support we wrap str around err.
+        sys.stderr.write(str(err))
         raise OSError('Command failed: %s' % ' '.join(cmd))
 
     try: