Merge branch 'stable-2.11' into stable-2.12

* stable-2.11:
  Fix query for changes using a label with a group operator.
  Add !important back to .cm-searching and .cm-trailingspace
  HttpPluginServlet: Fix "short read of block" IO error on plugin docs
  HttpPluginServlet: Use try-with-resource in readWholeEntry
  Make InternalChangeQuery.query public so it can be used by plugins
  InitPlugins: Suggest to upgrade installed plugins per default

Change-Id: I95d6f0d34bd72f809467fb7a74c93130e94b0de2

gerrit-gwtui/src/main/java/net/codemirror/lib/style.css

@@ -62,11 +62,11 @@
 }
 
 .cm-searching {
-  background-color: #ffa;
+  background-color: #ffa !important;
 }
 
 .cm-trailingspace {
-  background-color: red;
+  background-color: red !important;
 }
 
 /* Line length margin displayed at NN columns to provide

gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java

@@ -641,11 +641,9 @@ class HttpPluginServlet extends HttpServlet
 
   private static byte[] readWholeEntry(PluginContentScanner scanner, PluginEntry entry)
       throws IOException {
-    byte[] data = new byte[entry.getSize().get().intValue()];
     try (InputStream in = scanner.getInputStream(entry)) {
-      IO.readFully(in, data, 0, data.length);
+      return IO.readWholeStream(in, entry.getSize().get().intValue()).array();
     }
-    return data;
   }
 
   private static class PluginHolder {

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitPlugins.java

@@ -112,17 +112,18 @@ public class InitPlugins implements InitStep {
       String pluginName = plugin.name;
       try {
         final Path tmpPlugin = plugin.pluginPath;
+        Path p = site.plugins_dir.resolve(plugin.name + ".jar");
+        boolean upgrade = Files.exists(p);
 
-        if (!(initFlags.installPlugins.contains(pluginName) || ui.yesno(false,
+        if (!(initFlags.installPlugins.contains(pluginName) || ui.yesno(upgrade,
             "Install plugin %s version %s", pluginName, plugin.version))) {
           Files.deleteIfExists(tmpPlugin);
           continue;
         }
 
-        final Path p = site.plugins_dir.resolve(plugin.name + ".jar");
+        if (upgrade) {
-        if (Files.exists(p)) {
           final String installedPluginVersion = getVersion(p);
-          if (!ui.yesno(false,
+          if (!ui.yesno(upgrade,
               "version %s is already installed, overwrite it",
               installedPluginVersion)) {
             Files.deleteIfExists(tmpPlugin);

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -18,11 +18,17 @@ import static com.google.gerrit.server.query.change.ChangeData.asChanges;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.gerrit.common.data.GroupDetail;
 import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.common.errors.NotSignedInException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.AccountGroupById;
+import com.google.gerrit.reviewdb.client.AccountGroupMember;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.RefNames;
@@ -36,6 +42,8 @@ import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.account.GroupBackends;
 import com.google.gerrit.server.account.VersionedAccountDestinations;
 import com.google.gerrit.server.account.VersionedAccountQueries;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.GroupDetailFactory;
 import com.google.gerrit.server.change.ChangeTriplet;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AllUsersName;
@@ -47,6 +55,7 @@ import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
 import com.google.gerrit.server.index.ChangeIndex;
 import com.google.gerrit.server.index.FieldDef;
 import com.google.gerrit.server.index.IndexCollection;
+import com.google.gerrit.server.index.IndexConfig;
 import com.google.gerrit.server.index.IndexRewriter;
 import com.google.gerrit.server.index.Schema;
 import com.google.gerrit.server.patch.PatchListCache;
@@ -146,6 +155,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     final Provider<InternalChangeQuery> queryProvider;
     final IndexRewriter rewriter;
     final IdentifiedUser.GenericFactory userFactory;
+    final GroupDetailFactory.Factory groupDetailFactory;
     final CapabilityControl.Factory capabilityControlFactory;
     final ChangeControl.GenericFactory changeControlGenericFactory;
     final ChangeData.Factory changeDataFactory;
@@ -158,12 +168,14 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     final PatchListCache patchListCache;
     final GitRepositoryManager repoManager;
     final ProjectCache projectCache;
+    final GroupCache groupCache;
     final Provider<ListChildProjects> listChildProjects;
     final SubmitStrategyFactory submitStrategyFactory;
     final ConflictsCache conflictsCache;
     final TrackingFooters trackingFooters;
     final boolean allowsDrafts;
     final ChangeIndex index;
+    final IndexConfig indexConfig;
 
     private final Provider<CurrentUser> self;
 
@@ -175,6 +187,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         IdentifiedUser.GenericFactory userFactory,
         Provider<CurrentUser> self,
         CapabilityControl.Factory capabilityControlFactory,
+        GroupDetailFactory.Factory groupDetailFactory,
         ChangeControl.GenericFactory changeControlGenericFactory,
         ChangeData.Factory changeDataFactory,
         FieldDef.FillArgs fillArgs,
@@ -186,20 +199,23 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         PatchListCache patchListCache,
         GitRepositoryManager repoManager,
         ProjectCache projectCache,
+        GroupCache groupCache,
         Provider<ListChildProjects> listChildProjects,
         IndexCollection indexes,
         SubmitStrategyFactory submitStrategyFactory,
         ConflictsCache conflictsCache,
         TrackingFooters trackingFooters,
+        IndexConfig indexConfig,
         @GerritServerConfig Config cfg) {
       this(db, queryProvider, rewriter, userFactory, self,
-          capabilityControlFactory, changeControlGenericFactory,
+          capabilityControlFactory, groupDetailFactory, changeControlGenericFactory,
           changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend,
           allProjectsName, allUsersName, patchListCache, repoManager,
-          projectCache, listChildProjects, submitStrategyFactory,
+          projectCache, groupCache, listChildProjects, submitStrategyFactory,
           conflictsCache, trackingFooters,
-          cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true),
+          indexes != null ? indexes.getSearchIndex() : null,
-          indexes != null ? indexes.getSearchIndex() : null);
+          indexConfig,
+          cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true));
     }
 
     private Arguments(
@@ -209,6 +225,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         IdentifiedUser.GenericFactory userFactory,
         Provider<CurrentUser> self,
         CapabilityControl.Factory capabilityControlFactory,
+        GroupDetailFactory.Factory groupDetailFactory,
         ChangeControl.GenericFactory changeControlGenericFactory,
         ChangeData.Factory changeDataFactory,
         FieldDef.FillArgs fillArgs,
@@ -220,18 +237,21 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         PatchListCache patchListCache,
         GitRepositoryManager repoManager,
         ProjectCache projectCache,
+        GroupCache groupCache,
         Provider<ListChildProjects> listChildProjects,
         SubmitStrategyFactory submitStrategyFactory,
         ConflictsCache conflictsCache,
         TrackingFooters trackingFooters,
-        boolean allowsDrafts,
+        ChangeIndex index,
-        ChangeIndex index) {
+        IndexConfig indexConfig,
+        boolean allowsDrafts) {
      this.db = db;
      this.queryProvider = queryProvider;
      this.rewriter = rewriter;
      this.userFactory = userFactory;
      this.self = self;
      this.capabilityControlFactory = capabilityControlFactory;
+     this.groupDetailFactory = groupDetailFactory;
      this.changeControlGenericFactory = changeControlGenericFactory;
      this.changeDataFactory = changeDataFactory;
      this.fillArgs = fillArgs;
@@ -243,22 +263,24 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
      this.patchListCache = patchListCache;
      this.repoManager = repoManager;
      this.projectCache = projectCache;
+     this.groupCache = groupCache;
      this.listChildProjects = listChildProjects;
      this.submitStrategyFactory = submitStrategyFactory;
      this.conflictsCache = conflictsCache;
      this.trackingFooters = trackingFooters;
      this.allowsDrafts = allowsDrafts;
      this.index = index;
+     this.indexConfig = indexConfig;
     }
 
     Arguments asUser(CurrentUser otherUser) {
       return new Arguments(db, queryProvider, rewriter, userFactory,
           Providers.of(otherUser),
-          capabilityControlFactory, changeControlGenericFactory,
+          capabilityControlFactory, groupDetailFactory, changeControlGenericFactory,
           changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend,
           allProjectsName, allUsersName, patchListCache, repoManager,
-          projectCache, listChildProjects, submitStrategyFactory,
+          projectCache, groupCache, listChildProjects, submitStrategyFactory,
-          conflictsCache, trackingFooters, allowsDrafts, index);
+          conflictsCache, trackingFooters, index, indexConfig, allowsDrafts);
     }
 
     Arguments asUser(Account.Id otherId) {
@@ -579,6 +601,19 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
       }
     }
 
+    // expand a group predicate into multiple user predicates
+    if (group != null) {
+      Set<Account.Id> allMembers = getMemberIds(
+          group, new HashSet<AccountGroup.UUID>());
+      int maxTerms = args.indexConfig.maxLimit();
+      if (allMembers.size() > maxTerms) {
+        // limit the number of query terms otherwise Gerrit will barf
+        accounts = ImmutableSet.copyOf(Iterables.limit(allMembers, maxTerms));
+      } else {
+        accounts = allMembers;
+      }
+    }
+
     return new LabelPredicate(args.projectCache,
         args.changeControlGenericFactory, args.userFactory, args.db,
         name, accounts, group);
@@ -916,6 +951,39 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     return g;
   }
 
+  private Set<Account.Id> getMemberIds(AccountGroup.UUID groupUUID,
+      Set<AccountGroup.UUID> seenGroups) throws OrmException {
+    seenGroups.add(groupUUID);
+
+    Set<Account.Id> members = new HashSet<>();
+    AccountGroup group = args.groupCache.get(groupUUID);
+    if (group != null) {
+      try {
+        GroupDetail groupDetail =
+            args.groupDetailFactory.create(group.getId()).call();
+        if (groupDetail.members != null) {
+          for (AccountGroupMember m : groupDetail.members) {
+            if (!members.contains(m.getAccountId())) {
+              members.add(m.getAccountId());
+            }
+          }
+        }
+        // Get members of subgroups
+        if (groupDetail.includes != null) {
+          for (AccountGroupById includedGroup : groupDetail.includes) {
+            if (!seenGroups.contains(includedGroup.getIncludeUUID())) {
+              members.addAll(
+                  getMemberIds(includedGroup.getIncludeUUID(), seenGroups));
+            }
+          }
+        }
+      } catch (NoSuchGroupException e) {
+        // The included group is not visible
+      }
+    }
+    return members;
+  }
+
   private List<Change> parseChange(String value) throws OrmException,
       QueryParseException {
     if (PAT_LEGACY_ID.matcher(value).matches()) {

gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java

@@ -240,7 +240,7 @@ public class InternalChangeQuery {
     return query(and(project(project), or(groupPredicates)));
   }
 
-  private List<ChangeData> query(Predicate<ChangeData> p) throws OrmException {
+  public List<ChangeData> query(Predicate<ChangeData> p) throws OrmException {
     try {
       return qp.queryChanges(p).changes();
     } catch (QueryParseException e) {

gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java

@@ -25,9 +25,10 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
     super(
         new FakeQueryBuilder.Definition<>(
           FakeQueryBuilder.class),
-        new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
+        new ChangeQueryBuilder.Arguments(null, null, null, null,
-          null, null, null, null, null, null, null, null, null, null, null,
+            null, null, null, null, null, null, null, null, null,
-          null, indexes, null, null, null, null));
+            null, null, null, null, null, null, null, indexes, null,
+            null, null, null, null));
   }
 
   @Operator

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -36,6 +36,7 @@ import com.google.gerrit.extensions.api.GerritApi;
 import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
 import com.google.gerrit.extensions.api.changes.HashtagsInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.groups.GroupInput;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.lifecycle.LifecycleManager;
@@ -50,11 +51,13 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountManager;
 import com.google.gerrit.server.account.AuthRequest;
+import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.change.ChangeInserter;
 import com.google.gerrit.server.change.ChangeTriplet;
 import com.google.gerrit.server.change.PatchSetInserter;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.group.AddMembers;
 import com.google.gerrit.server.index.IndexCollection;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ChangeControl;
@@ -132,6 +135,8 @@ public abstract class AbstractQueryChangesTest {
   @Inject protected QueryProcessor queryProcessor;
   @Inject protected SchemaCreator schemaCreator;
   @Inject protected ThreadLocalRequestContext requestContext;
+  @Inject protected GroupCache groupCache;
+  @Inject protected AddMembers addMembers;
 
   protected LifecycleManager lifecycle;
   protected ReviewDb db;
@@ -589,6 +594,49 @@ public abstract class AbstractQueryChangesTest {
     assertQuery("label:Code-Review=+1,group=Administrators", change);
   }
 
+  private String createGroup(String name, String owner) throws Exception {
+    GroupInput in = new GroupInput();
+    in.name = name;
+    in.ownerId = owner;
+    gApi.groups().create(in);
+    return name;
+  }
+
+  private Account.Id createAccount(String name) throws Exception {
+    return accountManager.authenticate(
+        AuthRequest.forUser(name)).getAccountId();
+  }
+
+  @Test
+  public void byLabelGroup() throws Exception {
+    Account.Id user1 = createAccount("user1");
+    createAccount("user2");
+    TestRepository<Repo> repo = createProject("repo");
+
+    // create group and add users
+    String g1 = createGroup("group1", "Administrators");
+    String g2 = createGroup("group2", "Administrators");
+    gApi.groups().id(g1).addMembers("user1");
+    gApi.groups().id(g2).addMembers("user2");
+
+    // create a change
+    ChangeInserter ins = newChange(repo, null, null, user1.get(), null);
+    Change change1 = insert(ins);
+
+    // post a review with user1
+    requestContext.setContext(newRequestContext(user1));
+    gApi.changes().id(change1.getId().get()).current()
+      .review(new ReviewInput().label("Code-Review", 1));
+
+    // verify that query with user1 will return results.
+    requestContext.setContext(newRequestContext(userId));
+    assertQuery("label:Code-Review=+1,group1", change1);
+    assertQuery("label:Code-Review=+1,group=group1", change1);
+    assertQuery("label:Code-Review=+1,user=user1", change1);
+    assertQuery("label:Code-Review=+1,user=user2");
+    assertQuery("label:Code-Review=+1,group=group2");
+  }
+
   @Test
   public void limit() throws Exception {
     TestRepository<Repo> repo = createProject("repo");