Merge "Refactor AccountUpdate to be an interface"
diff --git a/java/com/google/gerrit/server/account/AccountConfig.java b/java/com/google/gerrit/server/account/AccountConfig.java
index f2331e3..e4fa718 100644
--- a/java/com/google/gerrit/server/account/AccountConfig.java
+++ b/java/com/google/gerrit/server/account/AccountConfig.java
@@ -95,7 +95,7 @@
}
@Override
- protected String getRefName() {
+ public String getRefName() {
return ref;
}
@@ -186,7 +186,7 @@
* @return the new account
* @throws DuplicateKeyException if the user branch already exists
*/
- Account getNewAccount(Instant registeredOn) throws DuplicateKeyException {
+ public Account getNewAccount(Instant registeredOn) throws DuplicateKeyException {
checkLoaded();
if (revision != null) {
throw new DuplicateKeyException(String.format("account %s already exists", accountId));
diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java
index e801b14..2080e93 100644
--- a/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -14,123 +14,39 @@
package com.google.gerrit.server.account;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.ACCOUNTS_UPDATE;
-import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.toList;
-import static java.util.stream.Collectors.toSet;
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Strings;
-import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.exceptions.DuplicateKeyException;
-import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.git.LockFailureException;
-import com.google.gerrit.git.RefUpdateUtil;
-import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIdNotes;
-import com.google.gerrit.server.account.externalids.ExternalIdNotes.ExternalIdNotesLoader;
-import com.google.gerrit.server.account.externalids.ExternalIds;
-import com.google.gerrit.server.account.storage.notedb.AccountsNoteDbImpl;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.config.CachedPreferences;
-import com.google.gerrit.server.config.VersionedDefaultPreferences;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.meta.MetaDataUpdate;
-import com.google.gerrit.server.index.change.ReindexAfterRefUpdate;
import com.google.gerrit.server.notedb.Sequences;
-import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryableAction.Action;
-import com.google.gerrit.server.update.context.RefUpdateContext;
-import com.google.inject.Provider;
-import com.google.inject.assistedinject.Assisted;
-import com.google.inject.assistedinject.AssistedInject;
+import com.google.inject.BindingAnnotation;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashSet;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
import java.util.List;
-import java.util.Objects;
import java.util.Optional;
-import java.util.Set;
import java.util.function.Consumer;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.BatchRefUpdate;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.transport.ReceiveCommand;
/**
* Creates and updates accounts.
*
- * <p>This class should be used for all account updates. See {@link AccountDelta} for what can be
- * updated.
- *
- * <p>Batch updates of multiple different accounts can be performed atomically, see {@link
- * #updateBatch(List)}. Batch creation is not supported.
- *
- * <p>For any account update the caller must provide a commit message, the account ID and an {@link
- * ConfigureDeltaFromState}. The account updater reads the current {@link AccountState} and prepares
- * updates to the account by calling setters on the provided {@link AccountDelta.Builder}. If the
- * current account state is of no interest the caller may also provide a {@link Consumer} for {@link
- * AccountDelta.Builder} instead of the account updater.
- *
- * <p>The provided commit message is used for the update of the user branch. Using a precise and
- * unique commit message allows to identify the code from which an update was made when looking at a
- * commit in the user branch, and thus help debugging.
+ * <p>This interface should be used for all account updates. See {@link AccountDelta} for what can
+ * be updated.
*
* <p>For creating a new account a new account ID can be retrieved from {@link
* Sequences#nextAccountId()}.
*
- * <p>The account updates are written to NoteDb. In NoteDb accounts are represented as user branches
- * in the {@code All-Users} repository. Optionally a user branch can contain a 'account.config' file
- * that stores account properties, such as full name, display name, preferred email, status and the
- * active flag. The timestamp of the first commit on a user branch denotes the registration date.
- * The initial commit on the user branch may be empty (since having an 'account.config' is
- * optional). See {@link AccountConfig} for details of the 'account.config' file format. In addition
- * the user branch can contain a 'preferences.config' config file to store preferences (see {@link
- * StoredPreferences}) and a 'watch.config' config file to store project watches (see {@link
- * ProjectWatches}). External IDs are stored separately in the {@code refs/meta/external-ids} notes
- * branch (see {@link ExternalIdNotes}).
- *
- * <p>On updating an account the account is evicted from the account cache and reindexed. The
- * eviction from the account cache and the reindexing is done by the {@link ReindexAfterRefUpdate}
- * class which receives the event about updating the user branch that is triggered by this class.
- *
- * <p>If external IDs are updated, the ExternalIdCache is automatically updated by {@link
- * ExternalIdNotes}. In addition {@link ExternalIdNotes} takes care about evicting and reindexing
- * corresponding accounts. This is needed because external ID updates don't touch the user branches.
- * Hence in this case the accounts are not evicted and reindexed via {@link ReindexAfterRefUpdate}.
- *
- * <p>Reindexing and flushing accounts from the account cache can be disabled by
- *
- * <ul>
- * <li>binding {@link GitReferenceUpdated#DISABLED} and
- * <li>passing an {@link
- * com.google.gerrit.server.account.externalids.ExternalIdNotes.FactoryNoReindex} factory as
- * parameter of {@link AccountsUpdate.Factory#create(IdentifiedUser,
- * ExternalIdNotes.ExternalIdNotesLoader)}
- * </ul>
- *
- * <p>If there are concurrent account updates updating the user branch in NoteDb may fail with
- * {@link LockFailureException}. In this case the account update is automatically retried and the
- * account updater is invoked once more with the updated account state. This means the whole
- * read-modify-write sequence is atomic. Retrying is limited by a timeout. If the timeout is
- * exceeded the account update can still fail with {@link LockFailureException}.
+ * <p>See the implementing classes for more information.
*/
-public class AccountsUpdate {
- public interface Factory {
+public interface AccountsUpdate {
+ abstract class AccountsUpdateLoader {
/**
* Creates an {@code AccountsUpdate} which uses the identity of the specified user as author for
* all commits related to accounts. The server identity will be used as committer.
@@ -140,9 +56,8 @@
* AccountsUpdate} instead.
*
* @param currentUser the user to which modifications should be attributed
- * @param externalIdNotesLoader the loader that should be used to load external ID notes
*/
- AccountsUpdate create(IdentifiedUser currentUser, ExternalIdNotesLoader externalIdNotesLoader);
+ public abstract AccountsUpdate create(IdentifiedUser currentUser);
/**
* Creates an {@code AccountsUpdate} which uses the server identity as author and committer for
@@ -151,10 +66,34 @@
* <p><strong>Note</strong>: Please use this method with care and consider using the {@link
* com.google.gerrit.server.ServerInitiated} annotation on the provider of an {@code
* AccountsUpdate} instead.
- *
- * @param externalIdNotesLoader the loader that should be used to load external ID notes
*/
- AccountsUpdate createWithServerIdent(ExternalIdNotesLoader externalIdNotesLoader);
+ public abstract AccountsUpdate createWithServerIdent();
+
+ @BindingAnnotation
+ @Target({FIELD, PARAMETER, METHOD})
+ @Retention(RUNTIME)
+ public @interface WithReindex {}
+
+ @BindingAnnotation
+ @Target({FIELD, PARAMETER, METHOD})
+ @Retention(RUNTIME)
+ public @interface NoReindex {}
+ }
+
+ /** Data holder for the set of arguments required to update an account. Used for batch updates. */
+ class UpdateArguments {
+ public final String message;
+ public final Account.Id accountId;
+ public final AccountsUpdate.ConfigureDeltaFromState configureDeltaFromState;
+
+ public UpdateArguments(
+ String message,
+ Account.Id accountId,
+ AccountsUpdate.ConfigureDeltaFromState configureDeltaFromState) {
+ this.message = message;
+ this.accountId = accountId;
+ this.configureDeltaFromState = configureDeltaFromState;
+ }
}
/**
@@ -162,13 +101,13 @@
* delta to be applied to it in a later step. This is done by implementing this interface.
*
* <p>If the current account state is not needed, use a {@link Consumer} of {@link
- * AccountDelta.Builder} instead.
+ * com.google.gerrit.server.account.AccountDelta.Builder} instead.
*/
@FunctionalInterface
- public interface ConfigureDeltaFromState {
+ interface ConfigureDeltaFromState {
/**
* Receives the current {@link AccountState} (which is immutable) and configures an {@link
- * AccountDelta.Builder} with changes to the account.
+ * com.google.gerrit.server.account.AccountDelta.Builder} with changes to the account.
*
* @param accountState the state of the account that is being updated
* @param delta the changes to be applied
@@ -176,145 +115,18 @@
void configure(AccountState accountState, AccountDelta.Builder delta) throws IOException;
}
- /** Data holder for the set of arguments required to update an account. Used for batch updates. */
- public static class UpdateArguments {
- private final String message;
- private final Account.Id accountId;
- private final ConfigureDeltaFromState configureDeltaFromState;
-
- public UpdateArguments(
- String message, Account.Id accountId, ConfigureDeltaFromState configureDeltaFromState) {
- this.message = message;
- this.accountId = accountId;
- this.configureDeltaFromState = configureDeltaFromState;
- }
- }
-
- private final GitRepositoryManager repoManager;
- private final GitReferenceUpdated gitRefUpdated;
- private final Optional<IdentifiedUser> currentUser;
- private final AllUsersName allUsersName;
- private final ExternalIds externalIds;
- private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
- private final RetryHelper retryHelper;
- private final ExternalIdNotesLoader extIdNotesLoader;
- private final PersonIdent committerIdent;
- private final PersonIdent authorIdent;
-
- /** Invoked after reading the account config. */
- private final Runnable afterReadRevision;
-
- /** Invoked after updating the account but before committing the changes. */
- private final Runnable beforeCommit;
-
- /** Single instance that accumulates updates from the batch. */
- @Nullable private ExternalIdNotes externalIdNotes;
-
- @AssistedInject
- AccountsUpdate(
- GitRepositoryManager repoManager,
- GitReferenceUpdated gitRefUpdated,
- AllUsersName allUsersName,
- ExternalIds externalIds,
- Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
- RetryHelper retryHelper,
- @GerritPersonIdent PersonIdent serverIdent,
- @Assisted ExternalIdNotesLoader extIdNotesLoader) {
- this(
- repoManager,
- gitRefUpdated,
- Optional.empty(),
- allUsersName,
- externalIds,
- metaDataUpdateInternalFactory,
- retryHelper,
- extIdNotesLoader,
- serverIdent,
- createPersonIdent(serverIdent, Optional.empty()),
- AccountsUpdate::doNothing,
- AccountsUpdate::doNothing);
- }
-
- @AssistedInject
- AccountsUpdate(
- GitRepositoryManager repoManager,
- GitReferenceUpdated gitRefUpdated,
- AllUsersName allUsersName,
- ExternalIds externalIds,
- Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
- RetryHelper retryHelper,
- @GerritPersonIdent PersonIdent serverIdent,
- @Assisted IdentifiedUser currentUser,
- @Assisted ExternalIdNotesLoader extIdNotesLoader) {
- this(
- repoManager,
- gitRefUpdated,
- Optional.of(currentUser),
- allUsersName,
- externalIds,
- metaDataUpdateInternalFactory,
- retryHelper,
- extIdNotesLoader,
- serverIdent,
- createPersonIdent(serverIdent, Optional.of(currentUser)),
- AccountsUpdate::doNothing,
- AccountsUpdate::doNothing);
- }
-
- @VisibleForTesting
- public AccountsUpdate(
- GitRepositoryManager repoManager,
- GitReferenceUpdated gitRefUpdated,
- Optional<IdentifiedUser> currentUser,
- AllUsersName allUsersName,
- ExternalIds externalIds,
- Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
- RetryHelper retryHelper,
- ExternalIdNotesLoader extIdNotesLoader,
- PersonIdent committerIdent,
- PersonIdent authorIdent,
- Runnable afterReadRevision,
- Runnable beforeCommit) {
- this.repoManager = requireNonNull(repoManager, "repoManager");
- this.gitRefUpdated = requireNonNull(gitRefUpdated, "gitRefUpdated");
- this.currentUser = currentUser;
- this.allUsersName = requireNonNull(allUsersName, "allUsersName");
- this.externalIds = requireNonNull(externalIds, "externalIds");
- this.metaDataUpdateInternalFactory =
- requireNonNull(metaDataUpdateInternalFactory, "metaDataUpdateInternalFactory");
- this.retryHelper = requireNonNull(retryHelper, "retryHelper");
- this.extIdNotesLoader = requireNonNull(extIdNotesLoader, "extIdNotesLoader");
- this.committerIdent = requireNonNull(committerIdent, "committerIdent");
- this.authorIdent = requireNonNull(authorIdent, "authorIdent");
- this.afterReadRevision = requireNonNull(afterReadRevision, "afterReadRevision");
- this.beforeCommit = requireNonNull(beforeCommit, "beforeCommit");
- }
-
/** Returns an instance that runs all specified consumers. */
- public static ConfigureDeltaFromState joinConsumers(
- List<Consumer<AccountDelta.Builder>> consumers) {
+ static ConfigureDeltaFromState joinConsumers(List<Consumer<AccountDelta.Builder>> consumers) {
return (accountStateIgnored, update) -> consumers.forEach(c -> c.accept(update));
}
- private static ConfigureDeltaFromState fromConsumer(Consumer<AccountDelta.Builder> consumer) {
- return (a, u) -> consumer.accept(u);
- }
-
- private static PersonIdent createPersonIdent(
- PersonIdent serverIdent, Optional<IdentifiedUser> user) {
- return user.isPresent() ? user.get().newCommitterIdent(serverIdent) : serverIdent;
- }
-
/**
* Like {@link #insert(String, Account.Id, ConfigureDeltaFromState)}, but using a {@link Consumer}
* instead, i.e. the update does not depend on the current account state (which, for insertion,
* would only contain the account ID).
*/
- public AccountState insert(
- String message, Account.Id accountId, Consumer<AccountDelta.Builder> init)
- throws IOException, ConfigInvalidException {
- return insert(message, accountId, fromConsumer(init));
- }
+ AccountState insert(String message, Account.Id accountId, Consumer<AccountDelta.Builder> init)
+ throws IOException, ConfigInvalidException;
/**
* Inserts a new account.
@@ -327,41 +139,16 @@
* @throws IOException if creating the user branch fails due to an IO error
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public AccountState insert(String message, Account.Id accountId, ConfigureDeltaFromState init)
- throws IOException, ConfigInvalidException {
- return execute(
- ImmutableList.of(
- repo -> {
- AccountConfig accountConfig = read(repo, accountId);
- Account account = accountConfig.getNewAccount(committerIdent.getWhenAsInstant());
- AccountState accountState = AccountState.forAccount(account);
- AccountDelta.Builder deltaBuilder = AccountDelta.builder();
- init.configure(accountState, deltaBuilder);
-
- AccountDelta accountDelta = deltaBuilder.build();
- accountConfig.setAccountDelta(accountDelta);
- externalIdNotes =
- createExternalIdNotes(
- repo, accountConfig.getExternalIdsRev(), accountId, accountDelta);
- CachedPreferences defaultPreferences =
- CachedPreferences.fromConfig(
- VersionedDefaultPreferences.get(repo, allUsersName));
-
- return new UpdatedAccount(message, accountConfig, defaultPreferences, true);
- }))
- .get(0)
- .get();
- }
+ AccountState insert(String message, Account.Id accountId, ConfigureDeltaFromState init)
+ throws IOException, ConfigInvalidException;
/**
* Like {@link #update(String, Account.Id, ConfigureDeltaFromState)}, but using a {@link Consumer}
* instead, i.e. the update does not depend on the current account state.
*/
- public Optional<AccountState> update(
+ Optional<AccountState> update(
String message, Account.Id accountId, Consumer<AccountDelta.Builder> update)
- throws IOException, ConfigInvalidException {
- return update(message, accountId, fromConsumer(update));
- }
+ throws IOException, ConfigInvalidException;
/**
* Gets the account and updates it atomically.
@@ -378,76 +165,9 @@
* after the retry timeout exceeded
* @throws ConfigInvalidException if any of the account fields has an invalid value
*/
- public Optional<AccountState> update(
+ Optional<AccountState> update(
String message, Account.Id accountId, ConfigureDeltaFromState configureDeltaFromState)
- throws LockFailureException, IOException, ConfigInvalidException {
- return updateBatch(
- ImmutableList.of(new UpdateArguments(message, accountId, configureDeltaFromState)))
- .get(0);
- }
-
- /**
- * Deletes all the account state data.
- *
- * @param message commit message for the account update, must not be {@code null or empty}
- * @param accountId ID of the account
- * @throws IOException if updating the user branch fails due to an IO error
- * @throws ConfigInvalidException if any of the account fields has an invalid value
- */
- public void delete(String message, Account.Id accountId)
- throws IOException, ConfigInvalidException {
- ImmutableSet<ExternalId> accountExternalIds = externalIds.byAccount(accountId);
- Consumer<AccountDelta.Builder> delta =
- deltaBuilder -> deltaBuilder.deleteAccount(accountExternalIds);
- update(message, accountId, delta);
- }
-
- private ExecutableUpdate createExecutableUpdate(UpdateArguments updateArguments) {
- return repo -> {
- AccountConfig accountConfig = read(repo, updateArguments.accountId);
- CachedPreferences defaultPreferences =
- CachedPreferences.fromConfig(VersionedDefaultPreferences.get(repo, allUsersName));
- Optional<AccountState> accountState =
- AccountsNoteDbImpl.getFromAccountConfig(externalIds, accountConfig, defaultPreferences);
- if (!accountState.isPresent()) {
- return null;
- }
-
- AccountDelta.Builder deltaBuilder = AccountDelta.builder();
- updateArguments.configureDeltaFromState.configure(accountState.get(), deltaBuilder);
-
- AccountDelta delta = deltaBuilder.build();
-
- ExternalIdNotes.checkSameAccount(
- Iterables.concat(
- delta.getCreatedExternalIds(),
- delta.getUpdatedExternalIds(),
- delta.getDeletedExternalIds()),
- updateArguments.accountId);
-
- if (delta.hasExternalIdUpdates()) {
- // Only load the externalIds if they are going to be updated
- // This makes e.g. preferences updates faster.
- if (externalIdNotes == null) {
- externalIdNotes =
- extIdNotesLoader.load(
- repo, accountConfig.getExternalIdsRev().orElse(ObjectId.zeroId()));
- }
- externalIdNotes.replace(delta.getDeletedExternalIds(), delta.getCreatedExternalIds());
- externalIdNotes.upsert(delta.getUpdatedExternalIds());
- }
-
- if (delta.getShouldDeleteAccount().orElse(false)) {
- return new DeletedAccount(updateArguments.message, accountConfig.getRefName());
- }
-
- accountConfig.setAccountDelta(delta);
- CachedPreferences cachedDefaultPreferences =
- CachedPreferences.fromConfig(VersionedDefaultPreferences.get(repo, allUsersName));
- return new UpdatedAccount(
- updateArguments.message, accountConfig, cachedDefaultPreferences, false);
- };
- }
+ throws LockFailureException, IOException, ConfigInvalidException;
/**
* Updates multiple different accounts atomically. This will only store a single new value (aka
@@ -459,238 +179,16 @@
* the error. Callers should be aware that a single "update of death" (or a set of updates that
* together have this property) will always prevent the entire batch from being executed.
*/
- public ImmutableList<Optional<AccountState>> updateBatch(List<UpdateArguments> updates)
- throws IOException, ConfigInvalidException {
- checkArgument(
- updates.stream().map(u -> u.accountId.get()).distinct().count() == updates.size(),
- "updates must all be for different accounts");
- return execute(updates.stream().map(this::createExecutableUpdate).collect(toList()));
- }
+ ImmutableList<Optional<AccountState>> updateBatch(List<UpdateArguments> updates)
+ throws IOException, ConfigInvalidException;
- private AccountConfig read(Repository allUsersRepo, Account.Id accountId)
- throws IOException, ConfigInvalidException {
- AccountConfig accountConfig = new AccountConfig(accountId, allUsersName, allUsersRepo).load();
- afterReadRevision.run();
- return accountConfig;
- }
-
- private ImmutableList<Optional<AccountState>> execute(List<ExecutableUpdate> executableUpdates)
- throws IOException, ConfigInvalidException {
- try (RefUpdateContext ctx = RefUpdateContext.open(ACCOUNTS_UPDATE)) {
- List<Optional<AccountState>> accountState = new ArrayList<>();
- List<UpdatedAccount> updatedAccounts = new ArrayList<>();
- executeWithRetry(
- () -> {
-
- // Reset state for retry.
- externalIdNotes = null;
- accountState.clear();
- updatedAccounts.clear();
- try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
- for (ExecutableUpdate executableUpdate : executableUpdates) {
- updatedAccounts.add(executableUpdate.execute(allUsersRepo));
- }
- commit(
- allUsersRepo,
- updatedAccounts.stream().filter(Objects::nonNull).collect(toList()));
- for (UpdatedAccount ua : updatedAccounts) {
- accountState.add(
- ua == null || ua.deleted ? Optional.empty() : ua.getAccountState());
- }
- }
- return null;
- });
-
- return ImmutableList.copyOf(accountState);
- }
- }
-
- private void executeWithRetry(Action<Void> action) throws IOException, ConfigInvalidException {
- try {
- retryHelper.accountUpdate("updateAccount", action).call();
- } catch (Exception e) {
- Throwables.throwIfUnchecked(e);
- Throwables.throwIfInstanceOf(e, IOException.class);
- Throwables.throwIfInstanceOf(e, ConfigInvalidException.class);
- throw new StorageException(e);
- }
- }
-
- private ExternalIdNotes createExternalIdNotes(
- Repository allUsersRepo, Optional<ObjectId> rev, Account.Id accountId, AccountDelta update)
- throws IOException, ConfigInvalidException, DuplicateKeyException {
- ExternalIdNotes.checkSameAccount(
- Iterables.concat(
- update.getCreatedExternalIds(),
- update.getUpdatedExternalIds(),
- update.getDeletedExternalIds()),
- accountId);
-
- ExternalIdNotes extIdNotes = extIdNotesLoader.load(allUsersRepo, rev.orElse(ObjectId.zeroId()));
- extIdNotes.replace(update.getDeletedExternalIds(), update.getCreatedExternalIds());
- extIdNotes.upsert(update.getUpdatedExternalIds());
- return extIdNotes;
- }
-
- private void commit(Repository allUsersRepo, List<UpdatedAccount> updatedAccounts)
- throws IOException {
- if (updatedAccounts.isEmpty()) {
- return;
- }
-
- beforeCommit.run();
-
- BatchRefUpdate batchRefUpdate = allUsersRepo.getRefDatabase().newBatchUpdate();
- Set<Account.Id> accountsToSkipForReindex = new HashSet<>();
- // External ids may be not updated if:
- // * externalIdNotes is not loaded (there were no externalId updates in the delta)
- // * new revCommit is identical to the previous externalId tip
- boolean externalIdsUpdated = false;
- if (externalIdNotes != null) {
- String externalIdUpdateMessage =
- updatedAccounts.size() == 1
- ? Iterables.getOnlyElement(updatedAccounts).message
- : "Batch update for " + updatedAccounts.size() + " accounts";
- ObjectId oldExternalIdsRevision = externalIdNotes.getRevision();
- // These update the same ref, so they need to be stacked on top of one another using the same
- // ExternalIdNotes instance.
- RevCommit revCommit =
- commitExternalIdUpdates(externalIdUpdateMessage, allUsersRepo, batchRefUpdate);
- externalIdsUpdated = !Objects.equals(revCommit.getId(), oldExternalIdsRevision);
- }
- for (UpdatedAccount updatedAccount : updatedAccounts) {
- if (updatedAccount.deleted) {
- RefUpdate ru = RefUpdateUtil.deleteChecked(allUsersRepo, updatedAccount.refName);
- gitRefUpdated.fire(allUsersName, ru, ReceiveCommand.Type.DELETE, null);
- accountsToSkipForReindex.add(Account.Id.fromRef(updatedAccount.refName));
- continue;
- }
- // These updates are all for different refs (because batches never update the same account
- // more than once), so there can be multiple commits in the same batch, all with the same base
- // revision in their AccountConfig.
- // We allow empty commits:
- // 1) When creating a new account, so that the user branch gets created with an empty commit
- // when no account properties are set and hence no
- // 'account.config' file will be created.
- // 2) When updating "refs/meta/external-ids", so that refs/users/* meta ref is updated too.
- // This allows to schedule reindexing of account transactionally on refs/users/* meta
- // updates.
- boolean allowEmptyCommit = externalIdsUpdated || updatedAccount.created;
- commitAccountConfig(
- updatedAccount.message,
- allUsersRepo,
- batchRefUpdate,
- updatedAccount.accountConfig,
- allowEmptyCommit);
- }
-
- RefUpdateUtil.executeChecked(batchRefUpdate, allUsersRepo);
-
- if (externalIdsUpdated) {
- accountsToSkipForReindex.addAll(getUpdatedAccountIds(batchRefUpdate));
- extIdNotesLoader.updateExternalIdCacheAndMaybeReindexAccounts(
- externalIdNotes, accountsToSkipForReindex);
- }
-
- gitRefUpdated.fire(
- allUsersName, batchRefUpdate, currentUser.map(IdentifiedUser::state).orElse(null));
- }
-
- private static Set<Account.Id> getUpdatedAccountIds(BatchRefUpdate batchRefUpdate) {
- return batchRefUpdate.getCommands().stream()
- .map(c -> Account.Id.fromRef(c.getRefName()))
- .filter(Objects::nonNull)
- .collect(toSet());
- }
-
- private void commitAccountConfig(
- String message,
- Repository allUsersRepo,
- BatchRefUpdate batchRefUpdate,
- AccountConfig accountConfig,
- boolean allowEmptyCommit)
- throws IOException {
- try (MetaDataUpdate md = createMetaDataUpdate(message, allUsersRepo, batchRefUpdate)) {
- md.setAllowEmpty(allowEmptyCommit);
- accountConfig.commit(md);
- }
- }
-
- private RevCommit commitExternalIdUpdates(
- String message, Repository allUsersRepo, BatchRefUpdate batchRefUpdate) throws IOException {
- try (MetaDataUpdate md = createMetaDataUpdate(message, allUsersRepo, batchRefUpdate)) {
- return externalIdNotes.commit(md);
- }
- }
-
- private MetaDataUpdate createMetaDataUpdate(
- String message, Repository allUsersRepo, BatchRefUpdate batchRefUpdate) {
- MetaDataUpdate metaDataUpdate =
- metaDataUpdateInternalFactory.get().create(allUsersName, allUsersRepo, batchRefUpdate);
- if (!message.endsWith("\n")) {
- message = message + "\n";
- }
-
- metaDataUpdate.getCommitBuilder().setMessage(message);
- metaDataUpdate.getCommitBuilder().setCommitter(committerIdent);
- metaDataUpdate.getCommitBuilder().setAuthor(authorIdent);
- return metaDataUpdate;
- }
-
- private static void doNothing() {}
-
- @FunctionalInterface
- private interface ExecutableUpdate {
- UpdatedAccount execute(Repository allUsersRepo) throws IOException, ConfigInvalidException;
- }
-
- private class UpdatedAccount {
- final String message;
- final AccountConfig accountConfig;
- final CachedPreferences defaultPreferences;
- final String refName;
- final boolean created;
- final boolean deleted;
-
- UpdatedAccount(
- String message,
- AccountConfig accountConfig,
- CachedPreferences defaultPreferences,
- boolean created) {
- this(
- message,
- requireNonNull(accountConfig),
- defaultPreferences,
- accountConfig.getRefName(),
- created,
- false);
- }
-
- protected UpdatedAccount(
- String message,
- AccountConfig accountConfig,
- CachedPreferences defaultPreferences,
- String refName,
- boolean created,
- boolean deleted) {
- checkState(!Strings.isNullOrEmpty(message), "message for account update must be set");
- this.message = requireNonNull(message);
- this.accountConfig = accountConfig;
- this.defaultPreferences = defaultPreferences;
- this.refName = refName;
- this.created = created;
- this.deleted = deleted;
- }
-
- Optional<AccountState> getAccountState() throws IOException {
- return AccountsNoteDbImpl.getFromAccountConfig(
- externalIds, accountConfig, externalIdNotes, defaultPreferences);
- }
- }
-
- private class DeletedAccount extends UpdatedAccount {
- DeletedAccount(String message, String refName) {
- super(message, null, null, refName, false, true);
- }
- }
+ /**
+ * Deletes all the account state data.
+ *
+ * @param message commit message for the account update, must not be {@code null or empty}
+ * @param accountId ID of the account
+ * @throws IOException if updating the user branch fails due to an IO error
+ * @throws ConfigInvalidException if any of the account fields has an invalid value
+ */
+ void delete(String message, Account.Id accountId) throws IOException, ConfigInvalidException;
}
diff --git a/java/com/google/gerrit/server/account/storage/notedb/AccountNoteDbStorageModule.java b/java/com/google/gerrit/server/account/storage/notedb/AccountNoteDbStorageModule.java
index 4cbe48f..51f6b86 100644
--- a/java/com/google/gerrit/server/account/storage/notedb/AccountNoteDbStorageModule.java
+++ b/java/com/google/gerrit/server/account/storage/notedb/AccountNoteDbStorageModule.java
@@ -14,13 +14,25 @@
package com.google.gerrit.server.account.storage.notedb;
+import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.account.AccountsUpdate;
+import com.google.gerrit.server.account.externalids.ExternalIdUpsertPreprocessor;
import com.google.inject.AbstractModule;
import com.google.inject.Singleton;
public class AccountNoteDbStorageModule extends AbstractModule {
@Override
protected void configure() {
- binder().bind(Accounts.class).to(AccountsNoteDbImpl.class).in(Singleton.class);
+ bind(Accounts.class).to(AccountsNoteDbImpl.class).in(Singleton.class);
+
+ DynamicMap.mapOf(binder(), ExternalIdUpsertPreprocessor.class);
+
+ bind(AccountsUpdate.AccountsUpdateLoader.class)
+ .annotatedWith(AccountsUpdate.AccountsUpdateLoader.WithReindex.class)
+ .to(AccountsUpdateNoteDbImpl.Factory.class);
+ bind(AccountsUpdate.AccountsUpdateLoader.class)
+ .annotatedWith(AccountsUpdate.AccountsUpdateLoader.NoReindex.class)
+ .to(AccountsUpdateNoteDbImpl.FactoryNoReindex.class);
}
}
diff --git a/java/com/google/gerrit/server/account/storage/notedb/AccountsNoteDbImpl.java b/java/com/google/gerrit/server/account/storage/notedb/AccountsNoteDbImpl.java
index ce8cd4d..9fb7917 100644
--- a/java/com/google/gerrit/server/account/storage/notedb/AccountsNoteDbImpl.java
+++ b/java/com/google/gerrit/server/account/storage/notedb/AccountsNoteDbImpl.java
@@ -141,7 +141,7 @@
* @return the account state, {@link Optional#empty()} if the account doesn't exist
* @throws IOException if accessing the external IDs fails
*/
- public static Optional<AccountState> getFromAccountConfig(
+ static Optional<AccountState> getFromAccountConfig(
ExternalIds externalIds, AccountConfig accountConfig, CachedPreferences defaultPreferences)
throws IOException {
return getFromAccountConfig(externalIds, accountConfig, null, defaultPreferences);
@@ -164,7 +164,7 @@
* @return the account state, {@link Optional#empty()} if the account doesn't exist
* @throws IOException if accessing the external IDs fails
*/
- public static Optional<AccountState> getFromAccountConfig(
+ static Optional<AccountState> getFromAccountConfig(
ExternalIds externalIds,
AccountConfig accountConfig,
@Nullable ExternalIdNotes extIdNotes,
diff --git a/java/com/google/gerrit/server/account/storage/notedb/AccountsUpdateNoteDbImpl.java b/java/com/google/gerrit/server/account/storage/notedb/AccountsUpdateNoteDbImpl.java
new file mode 100644
index 0000000..cc8ef14
--- /dev/null
+++ b/java/com/google/gerrit/server/account/storage/notedb/AccountsUpdateNoteDbImpl.java
@@ -0,0 +1,642 @@
+// Copyright (C) 2017 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.storage.notedb;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.ACCOUNTS_UPDATE;
+import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toSet;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
+import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.exceptions.DuplicateKeyException;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.git.LockFailureException;
+import com.google.gerrit.git.RefUpdateUtil;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountConfig;
+import com.google.gerrit.server.account.AccountDelta;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.AccountsUpdate;
+import com.google.gerrit.server.account.ProjectWatches;
+import com.google.gerrit.server.account.StoredPreferences;
+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.config.AllUsersName;
+import com.google.gerrit.server.config.CachedPreferences;
+import com.google.gerrit.server.config.VersionedDefaultPreferences;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.index.change.ReindexAfterRefUpdate;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gerrit.server.update.RetryableAction.Action;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Consumer;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.transport.ReceiveCommand;
+
+/**
+ * Creates and updates accounts which are stored in All-Users NoteDB repository.
+ *
+ * <p>Batch updates of multiple different accounts can be performed atomically, see {@link
+ * #updateBatch(List)}. Batch creation is not supported.
+ *
+ * <p>For any account update the caller must provide a commit message, the account ID and an {@link
+ * com.google.gerrit.server.account.AccountsUpdate.ConfigureDeltaFromState}. The account updater
+ * reads the current {@link AccountState} and prepares updates to the account by calling setters on
+ * the provided {@link com.google.gerrit.server.account.AccountDelta.Builder}. If the current
+ * account state is of no interest the caller may also provide a {@link Consumer} for {@link
+ * com.google.gerrit.server.account.AccountDelta.Builder} instead of the account updater.
+ *
+ * <p>The provided commit message is used for the update of the user branch. Using a precise and
+ * unique commit message allows to identify the code from which an update was made when looking at a
+ * commit in the user branch, and thus help debugging.
+ *
+ * <p>The account updates are written to NoteDb. In NoteDb accounts are represented as user branches
+ * in the {@code All-Users} repository. Optionally a user branch can contain a 'account.config' file
+ * that stores account properties, such as full name, display name, preferred email, status and the
+ * active flag. The timestamp of the first commit on a user branch denotes the registration date.
+ * The initial commit on the user branch may be empty (since having an 'account.config' is
+ * optional). See {@link AccountConfig} for details of the 'account.config' file format. In addition
+ * the user branch can contain a 'preferences.config' config file to store preferences (see {@link
+ * StoredPreferences}) and a 'watch.config' config file to store project watches (see {@link
+ * ProjectWatches}). External IDs are stored separately in the {@code refs/meta/external-ids} notes
+ * branch (see {@link ExternalIdNotes}).
+ *
+ * <p>On updating an account the account is evicted from the account cache and reindexed. The
+ * eviction from the account cache and the reindexing is done by the {@link ReindexAfterRefUpdate}
+ * class which receives the event about updating the user branch that is triggered by this class.
+ *
+ * <p>If external IDs are updated, the ExternalIdCache is automatically updated by {@link
+ * ExternalIdNotes}. In addition {@link ExternalIdNotes} takes care about evicting and reindexing
+ * corresponding accounts. This is needed because external ID updates don't touch the user branches.
+ * Hence in this case the accounts are not evicted and reindexed via {@link ReindexAfterRefUpdate}.
+ *
+ * <p>Reindexing and flushing accounts from the account cache can be disabled by
+ *
+ * <ul>
+ * <li>using {@link
+ * com.google.gerrit.server.account.storage.notedb.AccountsUpdateNoteDbImpl.FactoryNoReindex}
+ * and
+ * <li>binding {@link GitReferenceUpdated#DISABLED}
+ * </ul>
+ *
+ * <p>If there are concurrent account updates which updating the user branch in NoteDb may fail with
+ * {@link LockFailureException}. In this case the account update is automatically retried and the
+ * account updater is invoked once more with the updated account state. This means the whole
+ * read-modify-write sequence is atomic. Retrying is limited by a timeout. If the timeout is
+ * exceeded the account update can still fail with {@link LockFailureException}.
+ */
+public class AccountsUpdateNoteDbImpl implements AccountsUpdate {
+ private static class AbstractFactory extends AccountsUpdateLoader {
+ GitRepositoryManager repoManager;
+ GitReferenceUpdated gitRefUpdated;
+ AllUsersName allUsersName;
+ ExternalIds externalIds;
+ ExternalIdNotes.ExternalIdNotesLoader extIdNotesFactory;
+ Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
+ RetryHelper retryHelper;
+ PersonIdent serverIdent;
+
+ private AbstractFactory(
+ GitRepositoryManager repoManager,
+ GitReferenceUpdated gitRefUpdated,
+ AllUsersName allUsersName,
+ ExternalIds externalIds,
+ Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+ RetryHelper retryHelper,
+ @GerritPersonIdent PersonIdent serverIdent,
+ ExternalIdNotes.ExternalIdNotesLoader extIdNotesFactory) {
+ this.repoManager = repoManager;
+ this.gitRefUpdated = gitRefUpdated;
+ this.allUsersName = allUsersName;
+ this.externalIds = externalIds;
+ this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
+ this.retryHelper = retryHelper;
+ this.serverIdent = serverIdent;
+ this.extIdNotesFactory = extIdNotesFactory;
+ }
+
+ @Override
+ public AccountsUpdate create(IdentifiedUser currentUser) {
+ return new AccountsUpdateNoteDbImpl(
+ repoManager,
+ gitRefUpdated,
+ Optional.of(currentUser),
+ allUsersName,
+ externalIds,
+ extIdNotesFactory,
+ metaDataUpdateInternalFactory,
+ retryHelper,
+ serverIdent,
+ createPersonIdent(serverIdent, Optional.of(currentUser)),
+ AccountsUpdateNoteDbImpl::doNothing,
+ AccountsUpdateNoteDbImpl::doNothing);
+ }
+
+ @Override
+ public AccountsUpdate createWithServerIdent() {
+ return new AccountsUpdateNoteDbImpl(
+ repoManager,
+ gitRefUpdated,
+ Optional.empty(),
+ allUsersName,
+ externalIds,
+ extIdNotesFactory,
+ metaDataUpdateInternalFactory,
+ retryHelper,
+ serverIdent,
+ createPersonIdent(serverIdent, Optional.empty()),
+ AccountsUpdateNoteDbImpl::doNothing,
+ AccountsUpdateNoteDbImpl::doNothing);
+ }
+ }
+
+ @Singleton
+ public static class Factory extends AbstractFactory {
+ @Inject
+ Factory(
+ GitRepositoryManager repoManager,
+ GitReferenceUpdated gitRefUpdated,
+ AllUsersName allUsersName,
+ ExternalIds externalIds,
+ Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+ RetryHelper retryHelper,
+ @GerritPersonIdent PersonIdent serverIdent,
+ ExternalIdNotes.Factory extIdNotesFactory) {
+ super(
+ repoManager,
+ gitRefUpdated,
+ allUsersName,
+ externalIds,
+ metaDataUpdateInternalFactory,
+ retryHelper,
+ serverIdent,
+ extIdNotesFactory);
+ }
+ }
+
+ @Singleton
+ public static class FactoryNoReindex extends AbstractFactory {
+ @Inject
+ FactoryNoReindex(
+ GitRepositoryManager repoManager,
+ GitReferenceUpdated gitRefUpdated,
+ AllUsersName allUsersName,
+ ExternalIds externalIds,
+ Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+ RetryHelper retryHelper,
+ @GerritPersonIdent PersonIdent serverIdent,
+ ExternalIdNotes.FactoryNoReindex extIdNotesFactory) {
+ super(
+ repoManager,
+ gitRefUpdated,
+ allUsersName,
+ externalIds,
+ metaDataUpdateInternalFactory,
+ retryHelper,
+ serverIdent,
+ extIdNotesFactory);
+ }
+ }
+
+ private final GitRepositoryManager repoManager;
+ private final GitReferenceUpdated gitRefUpdated;
+ private final Optional<IdentifiedUser> currentUser;
+ private final AllUsersName allUsersName;
+ private final ExternalIds externalIds;
+
+ private final ExternalIdNotes.ExternalIdNotesLoader extIdNotesFactory;
+ private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
+ private final RetryHelper retryHelper;
+ private final PersonIdent committerIdent;
+ private final PersonIdent authorIdent;
+
+ /** Invoked after reading the account config. */
+ private final Runnable afterReadRevision;
+
+ /** Invoked after updating the account but before committing the changes. */
+ private final Runnable beforeCommit;
+
+ /** Single instance that accumulates updates from the batch. */
+ @Nullable private ExternalIdNotes externalIdNotes;
+
+ @VisibleForTesting
+ public AccountsUpdateNoteDbImpl(
+ GitRepositoryManager repoManager,
+ GitReferenceUpdated gitRefUpdated,
+ Optional<IdentifiedUser> currentUser,
+ AllUsersName allUsersName,
+ ExternalIds externalIds,
+ ExternalIdNotes.ExternalIdNotesLoader extIdNotesFactory,
+ Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+ RetryHelper retryHelper,
+ PersonIdent committerIdent,
+ PersonIdent authorIdent,
+ Runnable afterReadRevision,
+ Runnable beforeCommit) {
+ this.repoManager = requireNonNull(repoManager, "repoManager");
+ this.gitRefUpdated = requireNonNull(gitRefUpdated, "gitRefUpdated");
+ this.currentUser = currentUser;
+ this.allUsersName = requireNonNull(allUsersName, "allUsersName");
+ this.externalIds = requireNonNull(externalIds, "externalIds");
+ this.extIdNotesFactory = extIdNotesFactory;
+ this.metaDataUpdateInternalFactory =
+ requireNonNull(metaDataUpdateInternalFactory, "metaDataUpdateInternalFactory");
+ this.retryHelper = requireNonNull(retryHelper, "retryHelper");
+ this.committerIdent = requireNonNull(committerIdent, "committerIdent");
+ this.authorIdent = requireNonNull(authorIdent, "authorIdent");
+ this.afterReadRevision = requireNonNull(afterReadRevision, "afterReadRevision");
+ this.beforeCommit = requireNonNull(beforeCommit, "beforeCommit");
+ }
+
+ private static ConfigureDeltaFromState fromConsumer(Consumer<AccountDelta.Builder> consumer) {
+ return (a, u) -> consumer.accept(u);
+ }
+
+ private static PersonIdent createPersonIdent(
+ PersonIdent serverIdent, Optional<IdentifiedUser> user) {
+ return user.isPresent() ? user.get().newCommitterIdent(serverIdent) : serverIdent;
+ }
+
+ @Override
+ public AccountState insert(
+ String message, Account.Id accountId, Consumer<AccountDelta.Builder> init)
+ throws IOException, ConfigInvalidException {
+ return insert(message, accountId, fromConsumer(init));
+ }
+
+ @Override
+ public AccountState insert(String message, Account.Id accountId, ConfigureDeltaFromState init)
+ throws IOException, ConfigInvalidException {
+ return execute(
+ ImmutableList.of(
+ repo -> {
+ AccountConfig accountConfig = read(repo, accountId);
+ Account account = accountConfig.getNewAccount(committerIdent.getWhenAsInstant());
+ AccountState accountState = AccountState.forAccount(account);
+ AccountDelta.Builder deltaBuilder = AccountDelta.builder();
+ init.configure(accountState, deltaBuilder);
+
+ AccountDelta accountDelta = deltaBuilder.build();
+ accountConfig.setAccountDelta(accountDelta);
+ externalIdNotes =
+ createExternalIdNotes(
+ repo, accountConfig.getExternalIdsRev(), accountId, accountDelta);
+ CachedPreferences defaultPreferences =
+ CachedPreferences.fromConfig(
+ VersionedDefaultPreferences.get(repo, allUsersName));
+
+ return new UpdatedAccount(message, accountConfig, defaultPreferences, true);
+ }))
+ .get(0)
+ .get();
+ }
+
+ @Override
+ public Optional<AccountState> update(
+ String message, Account.Id accountId, Consumer<AccountDelta.Builder> update)
+ throws IOException, ConfigInvalidException {
+ return update(message, accountId, fromConsumer(update));
+ }
+
+ @Override
+ public Optional<AccountState> update(
+ String message, Account.Id accountId, ConfigureDeltaFromState configureDeltaFromState)
+ throws LockFailureException, IOException, ConfigInvalidException {
+ return updateBatch(
+ ImmutableList.of(new UpdateArguments(message, accountId, configureDeltaFromState)))
+ .get(0);
+ }
+
+ @Override
+ public void delete(String message, Account.Id accountId)
+ throws IOException, ConfigInvalidException {
+ ImmutableSet<ExternalId> accountExternalIds = externalIds.byAccount(accountId);
+ Consumer<AccountDelta.Builder> delta =
+ deltaBuilder -> deltaBuilder.deleteAccount(accountExternalIds);
+ update(message, accountId, delta);
+ }
+
+ private ExecutableUpdate createExecutableUpdate(UpdateArguments updateArguments) {
+ return repo -> {
+ AccountConfig accountConfig = read(repo, updateArguments.accountId);
+ CachedPreferences defaultPreferences =
+ CachedPreferences.fromConfig(VersionedDefaultPreferences.get(repo, allUsersName));
+ Optional<AccountState> accountState =
+ AccountsNoteDbImpl.getFromAccountConfig(externalIds, accountConfig, defaultPreferences);
+ if (!accountState.isPresent()) {
+ return null;
+ }
+
+ AccountDelta.Builder deltaBuilder = AccountDelta.builder();
+ updateArguments.configureDeltaFromState.configure(accountState.get(), deltaBuilder);
+
+ AccountDelta delta = deltaBuilder.build();
+ ExternalIdNotes.checkSameAccount(
+ Iterables.concat(
+ delta.getCreatedExternalIds(),
+ delta.getUpdatedExternalIds(),
+ delta.getDeletedExternalIds()),
+ updateArguments.accountId);
+
+ if (delta.hasExternalIdUpdates()) {
+ // Only load the externalIds if they are going to be updated
+ // This makes e.g. preferences updates faster.
+ if (externalIdNotes == null) {
+ externalIdNotes =
+ extIdNotesFactory.load(
+ repo, accountConfig.getExternalIdsRev().orElse(ObjectId.zeroId()));
+ }
+ externalIdNotes.replace(delta.getDeletedExternalIds(), delta.getCreatedExternalIds());
+ externalIdNotes.upsert(delta.getUpdatedExternalIds());
+ }
+
+ if (delta.getShouldDeleteAccount().orElse(false)) {
+ return new DeletedAccount(updateArguments.message, accountConfig.getRefName());
+ }
+
+ accountConfig.setAccountDelta(delta);
+ CachedPreferences cachedDefaultPreferences =
+ CachedPreferences.fromConfig(VersionedDefaultPreferences.get(repo, allUsersName));
+ return new UpdatedAccount(
+ updateArguments.message, accountConfig, cachedDefaultPreferences, false);
+ };
+ }
+
+ @Override
+ public ImmutableList<Optional<AccountState>> updateBatch(List<UpdateArguments> updates)
+ throws IOException, ConfigInvalidException {
+ checkArgument(
+ updates.stream().map(u -> u.accountId.get()).distinct().count() == updates.size(),
+ "updates must all be for different accounts");
+ return execute(updates.stream().map(this::createExecutableUpdate).collect(toList()));
+ }
+
+ private AccountConfig read(Repository allUsersRepo, Account.Id accountId)
+ throws IOException, ConfigInvalidException {
+ AccountConfig accountConfig = new AccountConfig(accountId, allUsersName, allUsersRepo).load();
+ afterReadRevision.run();
+ return accountConfig;
+ }
+
+ private ImmutableList<Optional<AccountState>> execute(List<ExecutableUpdate> executableUpdates)
+ throws IOException, ConfigInvalidException {
+ try (RefUpdateContext ctx = RefUpdateContext.open(ACCOUNTS_UPDATE)) {
+ List<Optional<AccountState>> accountState = new ArrayList<>();
+ List<UpdatedAccount> updatedAccounts = new ArrayList<>();
+ executeWithRetry(
+ () -> {
+
+ // Reset state for retry.
+ externalIdNotes = null;
+ accountState.clear();
+ updatedAccounts.clear();
+ try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
+ for (ExecutableUpdate executableUpdate : executableUpdates) {
+ updatedAccounts.add(executableUpdate.execute(allUsersRepo));
+ }
+ commit(
+ allUsersRepo,
+ updatedAccounts.stream().filter(Objects::nonNull).collect(toList()));
+ for (UpdatedAccount ua : updatedAccounts) {
+ accountState.add(
+ ua == null || ua.deleted ? Optional.empty() : ua.getAccountState());
+ }
+ }
+ return null;
+ });
+
+ return ImmutableList.copyOf(accountState);
+ }
+ }
+
+ private void executeWithRetry(Action<Void> action) throws IOException, ConfigInvalidException {
+ try {
+ retryHelper.accountUpdate("updateAccount", action).call();
+ } catch (Exception e) {
+ Throwables.throwIfUnchecked(e);
+ Throwables.throwIfInstanceOf(e, IOException.class);
+ Throwables.throwIfInstanceOf(e, ConfigInvalidException.class);
+ throw new StorageException(e);
+ }
+ }
+
+ private ExternalIdNotes createExternalIdNotes(
+ Repository allUsersRepo, Optional<ObjectId> rev, Account.Id accountId, AccountDelta update)
+ throws IOException, ConfigInvalidException, DuplicateKeyException {
+ ExternalIdNotes.checkSameAccount(
+ Iterables.concat(
+ update.getCreatedExternalIds(),
+ update.getUpdatedExternalIds(),
+ update.getDeletedExternalIds()),
+ accountId);
+
+ ExternalIdNotes extIdNotes =
+ extIdNotesFactory.load(allUsersRepo, rev.orElse(ObjectId.zeroId()));
+ extIdNotes.replace(update.getDeletedExternalIds(), update.getCreatedExternalIds());
+ extIdNotes.upsert(update.getUpdatedExternalIds());
+ return extIdNotes;
+ }
+
+ private void commit(Repository allUsersRepo, List<UpdatedAccount> updatedAccounts)
+ throws IOException {
+ if (updatedAccounts.isEmpty()) {
+ return;
+ }
+
+ beforeCommit.run();
+
+ BatchRefUpdate batchRefUpdate = allUsersRepo.getRefDatabase().newBatchUpdate();
+ Set<Account.Id> accountsToSkipForReindex = new HashSet<>();
+ // External ids may be not updated if:
+ // * externalIdNotes is not loaded (there were no externalId updates in the delta)
+ // * new revCommit is identical to the previous externalId tip
+ boolean externalIdsUpdated = false;
+ if (externalIdNotes != null) {
+ String externalIdUpdateMessage =
+ updatedAccounts.size() == 1
+ ? Iterables.getOnlyElement(updatedAccounts).message
+ : "Batch update for " + updatedAccounts.size() + " accounts";
+ ObjectId oldExternalIdsRevision = externalIdNotes.getRevision();
+ // These update the same ref, so they need to be stacked on top of one another using the same
+ // ExternalIdNotes instance.
+ RevCommit revCommit =
+ commitExternalIdUpdates(externalIdUpdateMessage, allUsersRepo, batchRefUpdate);
+ externalIdsUpdated = !Objects.equals(revCommit.getId(), oldExternalIdsRevision);
+ }
+ for (UpdatedAccount updatedAccount : updatedAccounts) {
+ if (updatedAccount.deleted) {
+ RefUpdate ru = RefUpdateUtil.deleteChecked(allUsersRepo, updatedAccount.refName);
+ gitRefUpdated.fire(allUsersName, ru, ReceiveCommand.Type.DELETE, null);
+ accountsToSkipForReindex.add(Account.Id.fromRef(updatedAccount.refName));
+ continue;
+ }
+ // These updates are all for different refs (because batches never update the same account
+ // more than once), so there can be multiple commits in the same batch, all with the same base
+ // revision in their AccountConfig.
+ // We allow empty commits:
+ // 1) When creating a new account, so that the user branch gets created with an empty commit
+ // when no account properties are set and hence no
+ // 'account.config' file will be created.
+ // 2) When updating "refs/meta/external-ids", so that refs/users/* meta ref is updated too.
+ // This allows to schedule reindexing of account transactionally on refs/users/* meta
+ // updates.
+ boolean allowEmptyCommit = externalIdsUpdated || updatedAccount.created;
+ commitAccountConfig(
+ updatedAccount.message,
+ allUsersRepo,
+ batchRefUpdate,
+ updatedAccount.accountConfig,
+ allowEmptyCommit);
+ }
+
+ RefUpdateUtil.executeChecked(batchRefUpdate, allUsersRepo);
+
+ if (externalIdsUpdated) {
+ accountsToSkipForReindex.addAll(getUpdatedAccountIds(batchRefUpdate));
+ extIdNotesFactory.updateExternalIdCacheAndMaybeReindexAccounts(
+ externalIdNotes, accountsToSkipForReindex);
+ }
+
+ gitRefUpdated.fire(
+ allUsersName, batchRefUpdate, currentUser.map(IdentifiedUser::state).orElse(null));
+ }
+
+ private static Set<Account.Id> getUpdatedAccountIds(BatchRefUpdate batchRefUpdate) {
+ return batchRefUpdate.getCommands().stream()
+ .map(c -> Account.Id.fromRef(c.getRefName()))
+ .filter(Objects::nonNull)
+ .collect(toSet());
+ }
+
+ private void commitAccountConfig(
+ String message,
+ Repository allUsersRepo,
+ BatchRefUpdate batchRefUpdate,
+ AccountConfig accountConfig,
+ boolean allowEmptyCommit)
+ throws IOException {
+ try (MetaDataUpdate md = createMetaDataUpdate(message, allUsersRepo, batchRefUpdate)) {
+ md.setAllowEmpty(allowEmptyCommit);
+ accountConfig.commit(md);
+ }
+ }
+
+ private RevCommit commitExternalIdUpdates(
+ String message, Repository allUsersRepo, BatchRefUpdate batchRefUpdate) throws IOException {
+ try (MetaDataUpdate md = createMetaDataUpdate(message, allUsersRepo, batchRefUpdate)) {
+ return externalIdNotes.commit(md);
+ }
+ }
+
+ private MetaDataUpdate createMetaDataUpdate(
+ String message, Repository allUsersRepo, BatchRefUpdate batchRefUpdate) {
+ MetaDataUpdate metaDataUpdate =
+ metaDataUpdateInternalFactory.get().create(allUsersName, allUsersRepo, batchRefUpdate);
+ if (!message.endsWith("\n")) {
+ message = message + "\n";
+ }
+
+ metaDataUpdate.getCommitBuilder().setMessage(message);
+ metaDataUpdate.getCommitBuilder().setCommitter(committerIdent);
+ metaDataUpdate.getCommitBuilder().setAuthor(authorIdent);
+ return metaDataUpdate;
+ }
+
+ private static void doNothing() {}
+
+ @FunctionalInterface
+ private interface ExecutableUpdate {
+ UpdatedAccount execute(Repository allUsersRepo) throws IOException, ConfigInvalidException;
+ }
+
+ private class UpdatedAccount {
+ final String message;
+ final AccountConfig accountConfig;
+ final CachedPreferences defaultPreferences;
+ final String refName;
+ final boolean created;
+ final boolean deleted;
+
+ UpdatedAccount(
+ String message,
+ AccountConfig accountConfig,
+ CachedPreferences defaultPreferences,
+ boolean created) {
+ this(
+ message,
+ requireNonNull(accountConfig),
+ defaultPreferences,
+ accountConfig.getRefName(),
+ created,
+ false);
+ }
+
+ protected UpdatedAccount(
+ String message,
+ AccountConfig accountConfig,
+ CachedPreferences defaultPreferences,
+ String refName,
+ boolean created,
+ boolean deleted) {
+ checkState(!Strings.isNullOrEmpty(message), "message for account update must be set");
+ this.message = requireNonNull(message);
+ this.accountConfig = accountConfig;
+ this.defaultPreferences = defaultPreferences;
+ this.refName = refName;
+ this.created = created;
+ this.deleted = deleted;
+ }
+
+ Optional<AccountState> getAccountState() throws IOException {
+ return AccountsNoteDbImpl.getFromAccountConfig(
+ externalIds, accountConfig, externalIdNotes, defaultPreferences);
+ }
+ }
+
+ private class DeletedAccount extends UpdatedAccount {
+ DeletedAccount(String message, String refName) {
+ super(message, null, null, refName, false, true);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index d54d0f2..cb321e1 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -108,7 +108,6 @@
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.ExternalIdCacheModule;
import com.google.gerrit.server.account.externalids.ExternalIdModule;
-import com.google.gerrit.server.account.externalids.ExternalIdUpsertPreprocessor;
import com.google.gerrit.server.account.storage.notedb.AccountNoteDbStorageModule;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.auth.AuthBackend;
@@ -511,7 +510,6 @@
bind(ReloadPluginListener.class)
.annotatedWith(UniqueAnnotations.create())
.to(PluginConfigFactory.class);
- DynamicMap.mapOf(binder(), ExternalIdUpsertPreprocessor.class);
bind(AttentionSetObserver.class);
}
diff --git a/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java b/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
index af93b8a..d0b4470 100644
--- a/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
@@ -29,7 +29,6 @@
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.account.AccountsUpdate;
-import com.google.gerrit.server.account.externalids.ExternalIdNotes;
import com.google.inject.Provides;
import com.google.inject.multibindings.OptionalBinder;
@@ -112,23 +111,22 @@
post(ACCOUNT_KIND, "drafts:delete").to(DeleteDraftComments.class);
// The gpgkeys REST endpoints are bound via GpgApiModule.
-
- factory(AccountsUpdate.Factory.class);
}
@Provides
@ServerInitiated
AccountsUpdate provideServerInitiatedAccountsUpdate(
- AccountsUpdate.Factory accountsUpdateFactory, ExternalIdNotes.Factory extIdNotesFactory) {
- return accountsUpdateFactory.createWithServerIdent(extIdNotesFactory);
+ @AccountsUpdate.AccountsUpdateLoader.WithReindex
+ AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory) {
+ return accountsUpdateFactory.createWithServerIdent();
}
@Provides
@UserInitiated
AccountsUpdate provideUserInitiatedAccountsUpdate(
- AccountsUpdate.Factory accountsUpdateFactory,
- IdentifiedUser currentUser,
- ExternalIdNotes.Factory extIdNotesFactory) {
- return accountsUpdateFactory.create(currentUser, extIdNotesFactory);
+ @AccountsUpdate.AccountsUpdateLoader.WithReindex
+ AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory,
+ IdentifiedUser currentUser) {
+ return accountsUpdateFactory.create(currentUser);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 2c65019..3fff964 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -146,6 +146,7 @@
import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory;
import com.google.gerrit.server.account.externalids.ExternalIdNotes;
import com.google.gerrit.server.account.externalids.ExternalIds;
+import com.google.gerrit.server.account.storage.notedb.AccountsUpdateNoteDbImpl;
import com.google.gerrit.server.change.AccountPatchReviewStore;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -2170,26 +2171,8 @@
String status = "happy";
String fullName = "Foo";
AtomicBoolean doneBgUpdate = new AtomicBoolean(false);
- PersonIdent ident = serverIdent.get();
AccountsUpdate update =
- new AccountsUpdate(
- repoManager,
- gitReferenceUpdated,
- Optional.empty(),
- allUsers,
- externalIds,
- metaDataUpdateInternalFactory,
- new RetryHelper(
- cfg,
- retryMetrics,
- null,
- null,
- null,
- exceptionHooks,
- r -> r.withBlockStrategy(noSleepBlockStrategy)),
- extIdNotesFactory,
- ident,
- ident,
+ getAccountsUpdate(
() -> {
if (!doneBgUpdate.getAndSet(true)) {
try {
@@ -2226,28 +2209,8 @@
List<String> status = ImmutableList.of("foo", "bar", "baz");
String fullName = "Foo";
AtomicInteger bgCounter = new AtomicInteger(0);
- PersonIdent ident = serverIdent.get();
AccountsUpdate update =
- new AccountsUpdate(
- repoManager,
- gitReferenceUpdated,
- Optional.empty(),
- allUsers,
- externalIds,
- metaDataUpdateInternalFactory,
- new RetryHelper(
- cfg,
- retryMetrics,
- null,
- null,
- null,
- exceptionHooks,
- r ->
- r.withStopStrategy(StopStrategies.stopAfterAttempt(status.size()))
- .withBlockStrategy(noSleepBlockStrategy)),
- extIdNotesFactory,
- ident,
- ident,
+ getAccountsUpdate(
() -> {
try {
accountsUpdateProvider
@@ -2260,7 +2223,17 @@
// Ignore, the expected exception is asserted later
}
},
- Runnables.doNothing());
+ Runnables.doNothing(),
+ new RetryHelper(
+ cfg,
+ retryMetrics,
+ null,
+ null,
+ null,
+ exceptionHooks,
+ r ->
+ r.withStopStrategy(StopStrategies.stopAfterAttempt(status.size()))
+ .withBlockStrategy(noSleepBlockStrategy)));
assertThat(bgCounter.get()).isEqualTo(0);
AccountInfo accountInfo = gApi.accounts().id(admin.id().get()).get();
assertThat(accountInfo.status).isNull();
@@ -2286,26 +2259,8 @@
AtomicInteger bgCounterA1 = new AtomicInteger(0);
AtomicInteger bgCounterA2 = new AtomicInteger(0);
- PersonIdent ident = serverIdent.get();
AccountsUpdate update =
- new AccountsUpdate(
- repoManager,
- gitReferenceUpdated,
- Optional.empty(),
- allUsers,
- externalIds,
- metaDataUpdateInternalFactory,
- new RetryHelper(
- cfg,
- retryMetrics,
- null,
- null,
- null,
- exceptionHooks,
- r -> r.withBlockStrategy(noSleepBlockStrategy)),
- extIdNotesFactory,
- ident,
- ident,
+ getAccountsUpdate(
Runnables.doNothing(),
() -> {
try {
@@ -2360,27 +2315,9 @@
AtomicInteger bgCounterA1 = new AtomicInteger(0);
AtomicInteger bgCounterA2 = new AtomicInteger(0);
- PersonIdent ident = serverIdent.get();
ExternalId extIdA2 = externalIdFactory.create("foo", "A-2", accountId);
AccountsUpdate update =
- new AccountsUpdate(
- repoManager,
- gitReferenceUpdated,
- Optional.empty(),
- allUsers,
- externalIds,
- metaDataUpdateInternalFactory,
- new RetryHelper(
- cfg,
- retryMetrics,
- null,
- null,
- null,
- exceptionHooks,
- r -> r.withBlockStrategy(noSleepBlockStrategy)),
- extIdNotesFactory,
- ident,
- ident,
+ getAccountsUpdate(
Runnables.doNothing(),
() -> {
try {
@@ -3681,6 +3618,37 @@
"login?account_id=" + accountId, HttpServletResponse.SC_MOVED_TEMPORARILY);
}
+ private AccountsUpdate getAccountsUpdate(Runnable afterReadRevision, Runnable beforeCommit) {
+ return getAccountsUpdate(
+ afterReadRevision,
+ beforeCommit,
+ new RetryHelper(
+ cfg,
+ retryMetrics,
+ null,
+ null,
+ null,
+ exceptionHooks,
+ r -> r.withBlockStrategy(noSleepBlockStrategy)));
+ }
+
+ private AccountsUpdate getAccountsUpdate(
+ Runnable afterReadRevision, Runnable beforeCommit, RetryHelper retryHelper) {
+ return new AccountsUpdateNoteDbImpl(
+ repoManager,
+ gitReferenceUpdated,
+ Optional.empty(),
+ allUsers,
+ externalIds,
+ extIdNotesFactory,
+ metaDataUpdateInternalFactory,
+ retryHelper,
+ serverIdent.get(),
+ serverIdent.get(),
+ afterReadRevision,
+ beforeCommit);
+ }
+
private void httpGetAndAssertStatus(String urlPath, int expectedHttpStatus)
throws ClientProtocolException, IOException {
HttpGet httpGet = new HttpGet(canonicalWebUrl.get() + urlPath);
