Merge "Evict initial members of group created by SSH command from AccountCache"

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java

@@ -18,72 +18,34 @@ import com.google.gerrit.common.errors.NameAlreadyUsedException;
 import com.google.gerrit.httpd.rpc.Handler;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.AccountGroup;
-import com.google.gerrit.reviewdb.AccountGroupMember;
-import com.google.gerrit.reviewdb.AccountGroupMemberAudit;
-import com.google.gerrit.reviewdb.AccountGroupName;
-import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.PerformCreateGroup;
-import com.google.gwtorm.client.OrmDuplicateKeyException;
+import com.google.gerrit.server.account.PerformCreateGroup;
 import com.google.gwtorm.client.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
-import java.util.Collections;
-
 class CreateGroup extends Handler<AccountGroup.Id> {
   interface Factory {
-    CreateGroup create(String newName);
+    CreateGroup create(String groupName);
   }
 
-  private final ReviewDb db;
+  private final PerformCreateGroup.Factory performCreateGroupFactory;
   private final IdentifiedUser user;
-  private final AccountCache accountCache;
+  private final String groupName;
-
-  private final String name;
 
   @Inject
-  CreateGroup(final ReviewDb db, final IdentifiedUser user,
+  CreateGroup(final PerformCreateGroup.Factory performCreateGroupFactory,
-      final AccountCache accountCache,
+      final IdentifiedUser user, @Assisted final String groupName) {
-
+    this.performCreateGroupFactory = performCreateGroupFactory;
-      @Assisted final String newName) {
-    this.db = db;
     this.user = user;
-    this.accountCache = accountCache;
+    this.groupName = groupName;
-
-    this.name = newName;
   }
 
   @Override
   public AccountGroup.Id call() throws OrmException, NameAlreadyUsedException {
-    final AccountGroup.NameKey key = new AccountGroup.NameKey(name);
+    final PerformCreateGroup performCreateGroup = performCreateGroupFactory.create();
-
-    if (db.accountGroupNames().get(key) != null) {
-      throw new NameAlreadyUsedException();
-    }
-
-    final AccountGroup.Id id = new AccountGroup.Id(db.nextAccountGroupId());
     final Account.Id me = user.getAccountId();
-
+    return performCreateGroup.createGroup(groupName, null, null, me);
-    final AccountGroup group = new AccountGroup(key, id);
-    db.accountGroups().insert(Collections.singleton(group));
-
-    try {
-      final AccountGroupName n = new AccountGroupName(group);
-      db.accountGroupNames().insert(Collections.singleton(n));
-    } catch (OrmDuplicateKeyException dupeErr) {
-      db.accountGroups().delete(Collections.singleton(group));
-      throw new NameAlreadyUsedException();
-    }
-
-    AccountGroupMember member = new AccountGroupMember(//
-        new AccountGroupMember.Key(me, id));
-
-    db.accountGroupMembersAudit().insert(
-        Collections.singleton(new AccountGroupMemberAudit(member, me)));
-    db.accountGroupMembers().insert(Collections.singleton(member));
-
-    accountCache.evict(me);
-    return id;
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java

@@ -0,0 +1,118 @@
+// Copyright (C) 2011 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.account;
+
+import com.google.gerrit.common.errors.NameAlreadyUsedException;
+import com.google.gerrit.reviewdb.Account;
+import com.google.gerrit.reviewdb.AccountGroup;
+import com.google.gerrit.reviewdb.AccountGroupMember;
+import com.google.gerrit.reviewdb.AccountGroupMemberAudit;
+import com.google.gerrit.reviewdb.AccountGroupName;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gwtorm.client.OrmDuplicateKeyException;
+import com.google.gwtorm.client.OrmException;
+import com.google.inject.Inject;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class PerformCreateGroup {
+
+  public interface Factory {
+    PerformCreateGroup create();
+  }
+
+  private final ReviewDb db;
+  private final AccountCache accountCache;
+  private final IdentifiedUser currentUser;
+
+  @Inject
+  PerformCreateGroup(final ReviewDb db, final AccountCache accountCache,
+      final IdentifiedUser currentUser) {
+    this.db = db;
+    this.accountCache = accountCache;
+    this.currentUser = currentUser;
+  }
+
+  /**
+   * Creates a new group.
+   *
+   * @param groupName the name for the new group
+   * @param groupDescription the description of the new group, <code>null</code>
+   *        if no description
+   * @param ownerGroupId the group that should own the new group, if
+   *        <code>null</code> the new group will own itself
+   * @param initialMembers initial members to be added to the new group
+   * @return the id of the new group
+   * @throws OrmException is thrown in case of any data store read or write
+   *         error
+   * @throws NameAlreadyUsedException is thrown in case a group with the given
+   *         name already exists
+   */
+  public AccountGroup.Id createGroup(final String groupName,
+      final String groupDescription, final AccountGroup.Id ownerGroupId,
+      final Account.Id... initialMembers) throws OrmException,
+      NameAlreadyUsedException {
+    final AccountGroup.Id groupId =
+        new AccountGroup.Id(db.nextAccountGroupId());
+    final AccountGroup.NameKey nameKey = new AccountGroup.NameKey(groupName);
+    final AccountGroup group = new AccountGroup(nameKey, groupId);
+    if (ownerGroupId != null) {
+      group.setOwnerGroupId(ownerGroupId);
+    }
+    if (groupDescription != null) {
+      group.setDescription(groupDescription);
+    }
+    final AccountGroupName gn = new AccountGroupName(group);
+    // first insert the group name to validate that the group name hasn't
+    // already been used to create another group
+    try {
+      db.accountGroupNames().insert(Collections.singleton(gn));
+    } catch (OrmDuplicateKeyException e) {
+      throw new NameAlreadyUsedException();
+    }
+    db.accountGroups().insert(Collections.singleton(group));
+
+    addMembers(groupId, initialMembers);
+
+    return groupId;
+  }
+
+  private void addMembers(final AccountGroup.Id groupId,
+      final Account.Id... members) throws OrmException {
+    final List<AccountGroupMember> memberships =
+        new ArrayList<AccountGroupMember>();
+    final List<AccountGroupMemberAudit> membershipsAudit =
+        new ArrayList<AccountGroupMemberAudit>();
+    for (Account.Id accountId : members) {
+      final AccountGroupMember membership =
+          new AccountGroupMember(new AccountGroupMember.Key(accountId, groupId));
+      memberships.add(membership);
+
+      final AccountGroupMemberAudit audit =
+          new AccountGroupMemberAudit(membership, currentUser.getAccountId());
+      membershipsAudit.add(audit);
+    }
+    db.accountGroupMembers().insert(memberships);
+    db.accountGroupMembersAudit().insert(membershipsAudit);
+
+    for (Account.Id accountId : members) {
+      accountCache.evict(accountId);
+    }
+  }
+
+}

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java

@@ -21,6 +21,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.RequestCleanup;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.account.GroupControl;
+import com.google.gerrit.server.account.PerformCreateGroup;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.ReceiveCommits;
 import com.google.gerrit.server.mail.AbandonedSender;
@@ -69,5 +70,6 @@ public class GerritRequestModule extends FactoryModule {
     factory(MergedSender.Factory.class);
     factory(MergeFailSender.Factory.class);
     factory(RegisterNewEmailSender.Factory.class);
+    factory(PerformCreateGroup.Factory.class);
   }
 }

gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java

@@ -347,6 +347,10 @@ public abstract class BaseCommand implements Command {
     return new UnloggedFailure(1, "fatal: " + msg);
   }
 
+  protected UnloggedFailure die(Throwable why) {
+    return new UnloggedFailure(1, "fatal: " + why.getMessage(), why);
+  }
+
   private final class TaskThunk implements CancelableRunnable, ProjectRunnable {
     private final CommandRunnable thunk;
     private final Context context;

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java

@@ -14,16 +14,13 @@
 
 package com.google.gerrit.sshd.commands;
 
+import com.google.gerrit.common.errors.NameAlreadyUsedException;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.AccountGroup;
-import com.google.gerrit.reviewdb.AccountGroupMember;
+import com.google.gerrit.server.account.PerformCreateGroup;
-import com.google.gerrit.reviewdb.AccountGroupMemberAudit;
+import com.google.gerrit.server.account.PerformCreateGroup;
-import com.google.gerrit.reviewdb.AccountGroupName;
-import com.google.gerrit.reviewdb.ReviewDb;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.sshd.AdminCommand;
 import com.google.gerrit.sshd.BaseCommand;
-import com.google.gwtorm.client.OrmDuplicateKeyException;
 import com.google.gwtorm.client.OrmException;
 import com.google.inject.Inject;
 
@@ -32,10 +29,7 @@ import org.kohsuke.args4j.Argument;
 import org.kohsuke.args4j.Option;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 
 /**
@@ -54,12 +48,6 @@ public class AdminCreateGroup extends BaseCommand {
   @Argument(index = 0, required = true, metaVar = "GROUP", usage = "name of group to be created")
   private String groupName;
 
-  @Inject
-  private IdentifiedUser currentUser;
-
-  @Inject
-  private ReviewDb db;
-
   private final Set<Account.Id> initialMembers = new HashSet<Account.Id>();
 
   @Option(name = "--member", aliases = {"-m"}, metaVar = "USERNAME", usage = "initial set of users to become members of the group")
@@ -67,6 +55,9 @@ public class AdminCreateGroup extends BaseCommand {
     initialMembers.add(id);
   }
 
+  @Inject
+  private PerformCreateGroup.Factory performCreateGroupFactory;
+
   @Override
   public void start(Environment env) throws IOException {
     startThread(new CommandRunnable() {
@@ -79,37 +70,12 @@ public class AdminCreateGroup extends BaseCommand {
   }
 
   private void createGroup() throws OrmException, UnloggedFailure {
-    AccountGroup.Id groupId = new AccountGroup.Id(db.nextAccountGroupId());
+    final PerformCreateGroup performCreateGroup =
-    AccountGroup.NameKey nameKey = new AccountGroup.NameKey(groupName);
+        performCreateGroupFactory.create();
-    AccountGroup group = new AccountGroup(nameKey, groupId);
-    if (ownerGroupId != null) {
-      group.setOwnerGroupId(ownerGroupId);
-    }
-    if (groupDescription != null) {
-      group.setDescription(groupDescription);
-    }
-    AccountGroupName gn = new AccountGroupName(group);
-    // first insert the group name to validate that the group name hasn't already been
-    // used to create another group
     try {
-      db.accountGroupNames().insert(Collections.singleton(gn));
+      performCreateGroup.createGroup(groupName, groupDescription, ownerGroupId, initialMembers.toArray(new Account.Id[initialMembers.size()]));
-    } catch (OrmDuplicateKeyException e) {
+    } catch (NameAlreadyUsedException e) {
-      throw die("group '" + groupName + "' already exists");
+      throw die(e);
     }
-    db.accountGroups().insert(Collections.singleton(group));
-
-    List<AccountGroupMember> memberships = new ArrayList<AccountGroupMember>();
-    List<AccountGroupMemberAudit> membershipsAudit = new ArrayList<AccountGroupMemberAudit>();
-    for (Account.Id accountId : initialMembers) {
-      AccountGroupMember membership =
-          new AccountGroupMember(new AccountGroupMember.Key(accountId, groupId));
-      memberships.add(membership);
-
-      AccountGroupMemberAudit audit =
-          new AccountGroupMemberAudit(membership, currentUser.getAccountId());
-      membershipsAudit.add(audit);
-    }
-    db.accountGroupMembers().insert(memberships);
-    db.accountGroupMembersAudit().insert(membershipsAudit);
   }
 }