opendev/gerrit
Code Issues Proposed changes

AccountManager: Allow unlinking of several external IDs at once

Browse Source 
If external IDs are unlinked one by one the account is reindexed once
per external ID that is deleted. If AccountManager can unlink several
external IDs at once, the account needs to be reindexed only once.

Change-Id: Id6dfd87df330fcdd7ff2e2dae06acbfe7f910feb
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-06-20 10:01:15 +02:00
parent e3b4760747
commit 10d3d3bd03
4 changed files with 61 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.account;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.audit.AuditService;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
@@ -412,45 +413,67 @@ public class AccountManager {
}
/**
* Unlink an authentication identity from an existing account.
* Unlink an external identity from an existing account.
*
* @param from account to unlink the identity from.
* @param who the identity to delete
* @return the result of unlinking the identity from the user.
* @throws AccountException the identity belongs to a different account, or it cannot be unlinked
* at this time.
* @param from account to unlink the external identity from
* @param extIdKey the key of the external ID that should be deleted
* @throws AccountException the identity belongs to a different account, or the identity was not
* found
*/
public AuthResult unlink(Account.Id from, AuthRequest who)
public void unlink(Account.Id from, ExternalId.Key extIdKey)
throws AccountException, OrmException, IOException, ConfigInvalidException {
unlink(from, ImmutableList.of(extIdKey));
}
/**
* Unlink an external identities from an existing account.
*
* @param from account to unlink the external identity from
* @param extIdKeys the keys of the external IDs that should be deleted
* @throws AccountException any of the identity belongs to a different account, or any of the
* identity was not found
*/
public void unlink(Account.Id from, Collection<ExternalId.Key> extIdKeys)
throws AccountException, OrmException, IOException, ConfigInvalidException {
if (extIdKeys.isEmpty()) {
return;
}
try (ReviewDb db = schema.open()) {
ExternalId extId = externalIds.get(who.getExternalIdKey());
if (extId != null) {
if (!extId.accountId().equals(from)) {
throw new AccountException(
"Identity '" + who.getExternalIdKey().get() + "' in use by another account");
List<ExternalId> extIds = new ArrayList<>(extIdKeys.size());
for (ExternalId.Key extIdKey : extIdKeys) {
ExternalId extId = externalIds.get(extIdKey);
if (extId != null) {
if (!extId.accountId().equals(from)) {
throw new AccountException(
"Identity '" + extIdKey.get() + "' in use by another account");
}
extIds.add(extId);
} else {
throw new AccountException("Identity '" + extIdKey.get() + "' not found");
}
externalIdsUpdateFactory.create().delete(extId);
if (who.getEmailAddress() != null) {
accountsUpdateFactory
.create()
.update(
db,
from,
a -> {
if (a.getPreferredEmail() != null
&& a.getPreferredEmail().equals(who.getEmailAddress())) {
a.setPreferredEmail(null);
}
});
byEmailCache.evict(who.getEmailAddress());
}
} else {
throw new AccountException("Identity '" + who.getExternalIdKey().get() + "' not found");
}
return new AuthResult(from, who.getExternalIdKey(), false);
externalIdsUpdateFactory.create().delete(extIds);
if (extIds.stream().anyMatch(e -> e.email() != null)) {
accountsUpdateFactory
.create()
.update(
db,
from,
a -> {
if (a.getPreferredEmail() != null) {
for (ExternalId extId : extIds) {
if (a.getPreferredEmail().equals(extId.email())) {
a.setPreferredEmail(null);
break;
}
}
}
});
extIds.stream().forEach(e -> byEmailCache.evict(e.email()));
}
}
}
}

7
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteEmail.java
View File

@@ -92,11 +92,8 @@ public class DeleteEmail implements RestModifyView<AccountResource.Email, Input>
}
try {
for (ExternalId extId : extIds) {
AuthRequest authRequest = new AuthRequest(extId.key());
authRequest.setEmailAddress(email);
accountManager.unlink(user.getAccountId(), authRequest);
}
accountManager.unlink(
user.getAccountId(), extIds.stream().map(e -> e.key()).collect(toSet()));
} catch (AccountException e) {
throw new ResourceConflictException(e.getMessage());
}

8
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteExternalIds.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.account;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toSet;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -96,11 +97,8 @@ public class DeleteExternalIds implements RestModifyView<AccountResource, List<S
}
try {
for (ExternalId extId : toDelete) {
AuthRequest authRequest = new AuthRequest(extId.key());
authRequest.setEmailAddress(extId.email());
accountManager.unlink(extId.accountId(), authRequest);
}
accountManager.unlink(
resource.getUser().getAccountId(), toDelete.stream().map(e -> e.key()).collect(toSet()));
} catch (AccountException e) {
throw new ResourceConflictException(e.getMessage());
}