opendev
/
gerrit
Code Issues Proposed changes

ExternalIds NoteDb migration: Avoid intermediate migration state 

The problem we are facing on the stable-2.14 branch is: we have
intermediate NoteDb migration state for accounts entity due to
merge of: Ic9bd5791e84. That why we are writing to both backends:
ReviewDb and NoteDb. It creates potential risk to be out of sync
between ReviewDb and NoteDb (and secondary index). In addition it
is always bad from performance point of view to unnecessary write
to 2 different backends. The real migration to NoteDb for accounts
entities (phase 2) happens in: Ia1dae9306b7 and Schema_144, that is
migrating the external IDs from ReviewDb to NoteDb, and that change
is not a part of stable-2.14.

In retrospective, we shouldn't include partially migrated code paths
for the production releases. It's error prone and bad for the
performance. Originally, multi-phase upgrade procedure was done on
master only to support multi master and zero downtime upgrades. These
feature is not related to open source gerrit version.

Moreover, now, that we are facing intermittent account corruption
problems: Issue 7652 that is hard to track down, understand and fix,
we are seeing automatic recovery attempt: [1], that is trying to
detect database corruption and synchronize both backends. This change
takes a different approach and avoids two backends where only ReviewDb
is actually used on production release line 2.14.

To avoid fixing too many caller sites the interfaces of ExternalIds,
ExternalIdsOnInit, ExternalIdsBatchUpdate and ExternalIdsUpdate are
mostly preserved, but the code paths for NoteDb mutations is dropped.

This partially reverts commit 744d2b8967.

[1] https://gerrit-review.googlesource.com/162450

Change-Id: Iec8d0c5639e462d88a7c5d0906febfd6f3337277
This commit is contained in:
David Ostrovsky 2018-03-07 20:59:25 +01:00
parent eb2f987ddb
commit 1eb5429227
14 changed files with 39 additions and 699 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

147
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
View File

@ -15,51 +15,22 @@
package com.google.gerrit.acceptance.rest.account;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.server.account.ExternalId.SCHEME_USERNAME;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.junit.Assert.fail;
import com.github.rholder.retry.BlockStrategy;
import com.github.rholder.retry.Retryer;
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.common.AccountExternalIdInfo;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.ExternalId;
import com.google.gerrit.server.account.ExternalIds;
import com.google.gerrit.server.account.ExternalIdsUpdate;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.LockFailureException;
import com.google.gson.reflect.TypeToken;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.api.errors.TransportException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.junit.Test;
@Sandboxed
public class ExternalIdIT extends AbstractDaemonTest {
@Inject private AllUsersName allUsers;
@Inject private ExternalIdsUpdate.Server extIdsUpdate;
@Inject private ExternalIds externalIds;
@Test
public void getExternalIDs() throws Exception {
@ -135,122 +106,4 @@ public class ExternalIdIT extends AbstractDaemonTest {
assertThat(response.getEntityContent())
.isEqualTo(String.format("External id %s does not exist", externalIdStr));
}
@Test
public void fetchExternalIdsBranch() throws Exception {
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, user);
// refs/meta/external-ids is only visible to users with the 'Access Database' capability
try {
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS);
fail("expected TransportException");
} catch (TransportException e) {
assertThat(e.getMessage())
.isEqualTo(
"Remote does not have " + RefNames.REFS_EXTERNAL_IDS + " available for fetch.");
}
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
// re-clone to get new request context, otherwise the old global capabilities are still cached
// in the IdentifiedUser object
allUsersRepo = cloneProject(allUsers, user);
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS);
}
@Test
public void pushToExternalIdsBranch() throws Exception {
grant(Permission.READ, allUsers, RefNames.REFS_EXTERNAL_IDS);
grant(Permission.PUSH, allUsers, RefNames.REFS_EXTERNAL_IDS);
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS + ":externalIds");
allUsersRepo.reset("externalIds");
PushOneCommit push = pushFactory.create(db, admin.getIdent(), allUsersRepo);
push.to(RefNames.REFS_EXTERNAL_IDS)
.assertErrorStatus("not allowed to update " + RefNames.REFS_EXTERNAL_IDS);
}
@Test
public void retryOnLockFailure() throws Exception {
Retryer<Void> retryer =
ExternalIdsUpdate.retryerBuilder()
.withBlockStrategy(
new BlockStrategy() {
@Override
public void block(long sleepTime) {
// Don't sleep in tests.
}
})
.build();
ExternalId.Key fooId = ExternalId.Key.create("foo", "foo");
ExternalId.Key barId = ExternalId.Key.create("bar", "bar");
final AtomicBoolean doneBgUpdate = new AtomicBoolean(false);
ExternalIdsUpdate update =
new ExternalIdsUpdate(
repoManager,
accountCache,
allUsers,
serverIdent.get(),
serverIdent.get(),
() -> {
if (!doneBgUpdate.getAndSet(true)) {
try {
extIdsUpdate.create().insert(db, ExternalId.create(barId, admin.id));
} catch (IOException | ConfigInvalidException | OrmException e) {
// Ignore, the successful insertion of the external ID is asserted later
}
}
},
retryer);
assertThat(doneBgUpdate.get()).isFalse();
update.insert(db, ExternalId.create(fooId, admin.id));
assertThat(doneBgUpdate.get()).isTrue();
assertThat(externalIds.get(fooId)).isNotNull();
assertThat(externalIds.get(barId)).isNotNull();
}
@Test
public void failAfterRetryerGivesUp() throws Exception {
ExternalId.Key[] extIdsKeys = {
ExternalId.Key.create("foo", "foo"),
ExternalId.Key.create("bar", "bar"),
ExternalId.Key.create("baz", "baz")
};
final AtomicInteger bgCounter = new AtomicInteger(0);
ExternalIdsUpdate update =
new ExternalIdsUpdate(
repoManager,
accountCache,
allUsers,
serverIdent.get(),
serverIdent.get(),
() -> {
try {
extIdsUpdate
.create()
.insert(db, ExternalId.create(extIdsKeys[bgCounter.getAndAdd(1)], admin.id));
} catch (IOException | ConfigInvalidException | OrmException e) {
// Ignore, the successful insertion of the external ID is asserted later
}
},
RetryerBuilder.<Void>newBuilder()
.retryIfException(e -> e instanceof LockFailureException)
.withStopStrategy(StopStrategies.stopAfterAttempt(extIdsKeys.length))
.build());
assertThat(bgCounter.get()).isEqualTo(0);
try {
update.insert(db, ExternalId.create(ExternalId.Key.create("abc", "abc"), admin.id));
fail("expected LockFailureException");
} catch (LockFailureException e) {
// Ignore, expected
}
assertThat(bgCounter.get()).isEqualTo(extIdsKeys.length);
for (ExternalId.Key extIdKey : extIdsKeys) {
assertThat(externalIds.get(extIdKey)).isNotNull();
}
}
}

5
gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java
View File

@ -39,7 +39,6 @@ import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
@ -128,7 +127,7 @@ class HttpLoginServlet extends HttpServlet {
try {
log.debug("Associating external identity \"{}\" to user \"{}\"", remoteExternalId, user);
updateRemoteExternalId(arsp, remoteExternalId);
} catch (AccountException | OrmException | ConfigInvalidException e) {
} catch (AccountException | OrmException e) {
log.error(
"Unable to associate external identity \""
+ remoteExternalId
@ -157,7 +156,7 @@ class HttpLoginServlet extends HttpServlet {
}
private void updateRemoteExternalId(AuthResult arsp, String remoteAuthToken)
throws AccountException, OrmException, IOException, ConfigInvalidException {
throws AccountException, OrmException, IOException {
accountManager.updateLink(
arsp.getAccountId(),
new AuthRequest(ExternalId.Key.create(SCHEME_EXTERNAL, remoteAuthToken)));

5
gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
View File

@ -46,7 +46,6 @@ import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -194,7 +193,7 @@ class OAuthSession {
log.info("OAuth2: linking claimed identity to {}", claimedId.get().toString());
try {
accountManager.link(claimedId.get(), req);
} catch (OrmException | ConfigInvalidException e) {
} catch (OrmException e) {
log.error(
"Cannot link: "
+ user.getExternalId()
@ -214,7 +213,7 @@ class OAuthSession {
throws AccountException, IOException {
try {
accountManager.link(identifiedUser.get().getAccountId(), areq);
} catch (OrmException | ConfigInvalidException e) {
} catch (OrmException e) {
log.error(
"Cannot link: "
+ user.getExternalId()

5
gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
View File

@ -44,7 +44,6 @@ import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -170,7 +169,7 @@ class OAuthSessionOverOpenID {
log.debug("Claimed account already exists: link to it.");
try {
accountManager.link(claimedId.get(), areq);
} catch (OrmException | ConfigInvalidException e) {
} catch (OrmException e) {
log.error(
"Cannot link: "
+ user.getExternalId()
@ -189,7 +188,7 @@ class OAuthSessionOverOpenID {
try {
log.debug("Linking \"{}\" to \"{}\"", user.getExternalId(), accountId);
accountManager.link(accountId, areq);
} catch (OrmException | ConfigInvalidException e) {
} catch (OrmException e) {
log.error("Cannot link: " + user.getExternalId() + " to user identity: " + accountId);
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return;

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/LocalUsernamesToLowerCase.java
View File

@ -57,7 +57,7 @@ public class LocalUsernamesToLowerCase extends SiteProgram {
monitor.update(1);
}
externalIdsBatchUpdate.commit(db, "Convert local usernames to lower case");
externalIdsBatchUpdate.commit(db);
}
monitor.endTask();

59
gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/ExternalIdsOnInit.java
View File

@ -16,70 +16,13 @@ package com.google.gerrit.pgm.init;
import static com.google.gerrit.server.account.ExternalId.toAccountExternalIds;
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdentProvider;
import com.google.gerrit.server.account.ExternalId;
import com.google.gerrit.server.account.ExternalIds;
import com.google.gerrit.server.account.ExternalIdsUpdate;
import com.google.gerrit.server.config.SitePaths;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Collection;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.RepositoryCache.FileKey;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.FS;
public class ExternalIdsOnInit {
private final InitFlags flags;
private final SitePaths site;
private final String allUsers;
@Inject
public ExternalIdsOnInit(InitFlags flags, SitePaths site, AllUsersNameOnInitProvider allUsers) {
this.flags = flags;
this.site = site;
this.allUsers = allUsers.get();
}
public synchronized void insert(ReviewDb db, String commitMessage, Collection<ExternalId> extIds)
throws OrmException, IOException, ConfigInvalidException {
public synchronized void insert(ReviewDb db, Collection<ExternalId> extIds) throws OrmException {
db.accountExternalIds().insert(toAccountExternalIds(extIds));
File path = getPath();
if (path != null) {
try (Repository repo = new FileRepository(path);
RevWalk rw = new RevWalk(repo);
ObjectInserter ins = repo.newObjectInserter()) {
ObjectId rev = ExternalIds.readRevision(repo);
NoteMap noteMap = ExternalIds.readNoteMap(rw, rev);
for (ExternalId extId : extIds) {
ExternalIdsUpdate.insert(rw, ins, noteMap, extId);
}
PersonIdent serverIdent = new GerritPersonIdentProvider(flags.cfg).get();
ExternalIdsUpdate.commit(
repo, rw, ins, rev, noteMap, commitMessage, serverIdent, serverIdent);
}
}
}
private File getPath() {
Path basePath = site.resolve(flags.cfg.getString("gerrit", null, "basePath"));
if (basePath == null) {
throw new IllegalStateException("gerrit.basePath must be configured");
}
return FileKey.resolve(basePath.resolve(allUsers).toFile(), FS.DETECTED);
}
}

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
View File

@ -101,7 +101,7 @@ public class InitAdminUser implements InitStep {
if (email != null) {
extIds.add(ExternalId.createEmail(id, email));
}
externalIds.insert(db, "Add external IDs for initial admin user", extIds);
externalIds.insert(db, extIds);
Account a = new Account(id, TimeUtil.nowTs());
a.setFullName(name);

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

@ -132,7 +132,7 @@ public class AccountManager {
}
private void update(ReviewDb db, AuthRequest who, ExternalId extId)
throws OrmException, IOException, ConfigInvalidException {
throws OrmException, IOException {
IdentifiedUser user = userFactory.create(extId.accountId());
Account toUpdate = null;
@ -315,7 +315,7 @@ public class AccountManager {
String errorMessage,
Exception e,
boolean logException)
throws AccountUserNameException, OrmException, IOException, ConfigInvalidException {
throws AccountUserNameException, OrmException, IOException {
if (logException) {
log.error(errorMessage, e);
} else {
@ -346,7 +346,7 @@ public class AccountManager {
* this time.
*/
public AuthResult link(Account.Id to, AuthRequest who)
throws AccountException, OrmException, IOException, ConfigInvalidException {
throws AccountException, OrmException, IOException {
try (ReviewDb db = schema.open()) {
ExternalId extId = findExternalId(db, who.getExternalIdKey());
if (extId != null) {
@ -389,7 +389,7 @@ public class AccountManager {
* this time.
*/
public AuthResult updateLink(Account.Id to, AuthRequest who)
throws OrmException, AccountException, IOException, ConfigInvalidException {
throws OrmException, AccountException, IOException {
try (ReviewDb db = schema.open()) {
Collection<ExternalId> filteredExtIdsByScheme =
ExternalId.from(db.accountExternalIds().byAccount(to).toList())
@ -421,7 +421,7 @@ public class AccountManager {
* at this time.
*/
public AuthResult unlink(Account.Id from, AuthRequest who)
throws AccountException, OrmException, IOException, ConfigInvalidException {
throws AccountException, OrmException, IOException {
try (ReviewDb db = schema.open()) {
ExternalId extId = findExternalId(db, who.getExternalIdKey());
if (extId != null) {

2
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
View File

@ -154,7 +154,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
} catch (OrmDuplicateKeyException duplicateKey) {
try {
externalIdsUpdate.delete(db, extUser);
} catch (IOException | ConfigInvalidException | OrmException cleanupError) {
} catch (IOException | OrmException cleanupError) {
// Ignored
}
throw new UnprocessableEntityException("email '" + input.email + "' already exists");

2
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java
View File

@ -102,7 +102,7 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
public Response<EmailInfo> apply(IdentifiedUser user, EmailInput input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, OrmException, EmailException, MethodNotAllowedException,
IOException, ConfigInvalidException {
IOException {
if (input == null) {
input = new EmailInput();
}

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

@ -68,7 +68,7 @@ public class DeleteEmail implements RestModifyView<AccountResource.Email, Input>
public Response<?> apply(IdentifiedUser user, String email)
throws ResourceNotFoundException, ResourceConflictException, MethodNotAllowedException,
OrmException, IOException, ConfigInvalidException {
OrmException, IOException {
if (!realm.allowsEdit(AccountFieldName.REGISTER_NEW_EMAIL)) {
throw new MethodNotAllowedException("realm does not allow deleting emails");
}

65
gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalIdsBatchUpdate.java
View File

@ -18,52 +18,16 @@ import static com.google.gerrit.server.account.ExternalId.toAccountExternalIds;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevWalk;
/**
* This class allows to do batch updates to external IDs.
*
* <p>For NoteDb all updates will result in a single commit to the refs/meta/external-ids branch.
* This means callers can prepare many updates by invoking {@link #replace(ExternalId, ExternalId)}
* multiple times and when {@link ExternalIdsBatchUpdate#commit(ReviewDb, String)} is invoked a
* single NoteDb commit is created that contains all the prepared updates.
*/
/** This class allows to do batch updates to external IDs. */
public class ExternalIdsBatchUpdate {
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
private final PersonIdent serverIdent;
private final Set<ExternalId> toAdd = new HashSet<>();
private final Set<ExternalId> toDelete = new HashSet<>();
@Inject
public ExternalIdsBatchUpdate(
GitRepositoryManager repoManager,
AllUsersName allUsersName,
@GerritPersonIdent PersonIdent serverIdent) {
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.serverIdent = serverIdent;
}
/**
* Adds an external ID replacement to the batch.
*
* <p>The actual replacement is only done when {@link #commit(ReviewDb, String)} is invoked.
*/
/** Adds an external ID replacement to the batch. */
public void replace(ExternalId extIdToDelete, ExternalId extIdToAdd) {
ExternalIdsUpdate.checkSameAccount(ImmutableSet.of(extIdToDelete, extIdToAdd));
toAdd.add(extIdToAdd);
@ -79,37 +43,14 @@ public class ExternalIdsBatchUpdate {
* external ID with the same key is specified to be added, the old external ID with that key is
* deleted first and then the new external ID is added (so the external ID for that key is
* replaced).
*
* <p>For NoteDb a single commit is created that contains all the external ID updates.
*/
public void commit(ReviewDb db, String commitMessage)
throws IOException, OrmException, ConfigInvalidException {
public void commit(ReviewDb db) throws OrmException {
if (toDelete.isEmpty() && toAdd.isEmpty()) {
return;
}
db.accountExternalIds().delete(toAccountExternalIds(toDelete));
db.accountExternalIds().insert(toAccountExternalIds(toAdd));
try (Repository repo = repoManager.openRepository(allUsersName);
RevWalk rw = new RevWalk(repo);
ObjectInserter ins = repo.newObjectInserter()) {
ObjectId rev = ExternalIds.readRevision(repo);
NoteMap noteMap = ExternalIds.readNoteMap(rw, rev);
for (ExternalId extId : toDelete) {
ExternalIdsUpdate.remove(rw, noteMap, extId);
}
for (ExternalId extId : toAdd) {
ExternalIdsUpdate.insert(rw, ins, noteMap, extId);
}
ExternalIdsUpdate.commit(
repo, rw, ins, rev, noteMap, commitMessage, serverIdent, serverIdent);
}
toAdd.clear();
toDelete.clear();
}

429
gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalIdsUpdate.java
View File

@ -14,150 +14,59 @@
package com.google.gerrit.server.account;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.account.ExternalId.Key.toAccountExternalIdKeys;
import static com.google.gerrit.server.account.ExternalId.toAccountExternalIds;
import static com.google.gerrit.server.account.ExternalIds.MAX_NOTE_SZ;
import static com.google.gerrit.server.account.ExternalIds.readNoteMap;
import static com.google.gerrit.server.account.ExternalIds.readRevision;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toSet;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
import com.github.rholder.retry.RetryException;
import com.github.rholder.retry.Retryer;
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.github.rholder.retry.WaitStrategies;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Throwables;
import com.google.common.collect.Iterables;
import com.google.common.util.concurrent.Runnables;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LockFailureException;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
/**
* Updates externalIds in ReviewDb and NoteDb.
*
* <p>In NoteDb external IDs are stored in the All-Users repository in a Git Notes branch called
* refs/meta/external-ids where the sha1 of the external ID is used as note name. Each note content
* is a git config file that contains an external ID. It has exactly one externalId subsection with
* an accountId and optionally email and password:
*
* <pre>
* [externalId "username:jdoe"]
* accountId = 1003407
* email = jdoe@example.com
* password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7
* </pre>
*
* For NoteDb each method call results in one commit on refs/meta/external-ids branch.
*
* <p>On updating external IDs this class takes care to evict affected accounts from the account
* cache and thus triggers reindex for them.
*/
// Updates externalIds in ReviewDb.
public class ExternalIdsUpdate {
private static final String COMMIT_MSG = "Update external IDs";
/**
* Factory to create an ExternalIdsUpdate instance for updating external IDs by the Gerrit server.
*
* <p>The Gerrit server identity will be used as author and committer for all commits that update
* the external IDs.
*/
@Singleton
public static class Server {
private final GitRepositoryManager repoManager;
private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final Provider<PersonIdent> serverIdent;
@Inject
public Server(
GitRepositoryManager repoManager,
AccountCache accountCache,
AllUsersName allUsersName,
@GerritPersonIdent Provider<PersonIdent> serverIdent) {
this.repoManager = repoManager;
public Server(AccountCache accountCache) {
this.accountCache = accountCache;
this.allUsersName = allUsersName;
this.serverIdent = serverIdent;
}
public ExternalIdsUpdate create() {
PersonIdent i = serverIdent.get();
return new ExternalIdsUpdate(repoManager, accountCache, allUsersName, i, i);
return new ExternalIdsUpdate(accountCache);
}
}
/**
* Factory to create an ExternalIdsUpdate instance for updating external IDs by the current user.
*
* <p>The identity of the current user will be used as author for all commits that update the
* external IDs. The Gerrit server identity will be used as committer.
*/
@Singleton
public static class User {
private final GitRepositoryManager repoManager;
private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final Provider<PersonIdent> serverIdent;
private final Provider<IdentifiedUser> identifiedUser;
@Inject
public User(
GitRepositoryManager repoManager,
AccountCache accountCache,
AllUsersName allUsersName,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
Provider<IdentifiedUser> identifiedUser) {
this.repoManager = repoManager;
public User(AccountCache accountCache) {
this.accountCache = accountCache;
this.allUsersName = allUsersName;
this.serverIdent = serverIdent;
this.identifiedUser = identifiedUser;
}
public ExternalIdsUpdate create() {
PersonIdent i = serverIdent.get();
return new ExternalIdsUpdate(
repoManager, accountCache, allUsersName, createPersonIdent(i, identifiedUser.get()), i);
}
private PersonIdent createPersonIdent(PersonIdent ident, IdentifiedUser user) {
return user.newCommitterIdent(ident.getWhen(), ident.getTimeZone());
return new ExternalIdsUpdate(accountCache);
}
}
@ -172,48 +81,11 @@ public class ExternalIdsUpdate {
.withStopStrategy(StopStrategies.stopAfterDelay(10, TimeUnit.SECONDS));
}
private static final Retryer<Void> RETRYER = retryerBuilder().build();
private final GitRepositoryManager repoManager;
private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final PersonIdent committerIdent;
private final PersonIdent authorIdent;
private final Runnable afterReadRevision;
private final Retryer<Void> retryer;
private ExternalIdsUpdate(
GitRepositoryManager repoManager,
AccountCache accountCache,
AllUsersName allUsersName,
PersonIdent committerIdent,
PersonIdent authorIdent) {
this(
repoManager,
accountCache,
allUsersName,
committerIdent,
authorIdent,
Runnables.doNothing(),
RETRYER);
}
@VisibleForTesting
public ExternalIdsUpdate(
GitRepositoryManager repoManager,
AccountCache accountCache,
AllUsersName allUsersName,
PersonIdent committerIdent,
PersonIdent authorIdent,
Runnable afterReadRevision,
Retryer<Void> retryer) {
this.repoManager = checkNotNull(repoManager, "repoManager");
public ExternalIdsUpdate(AccountCache accountCache) {
this.accountCache = accountCache;
this.allUsersName = checkNotNull(allUsersName, "allUsersName");
this.committerIdent = checkNotNull(committerIdent, "committerIdent");
this.authorIdent = checkNotNull(authorIdent, "authorIdent");
this.afterReadRevision = checkNotNull(afterReadRevision, "afterReadRevision");
this.retryer = checkNotNull(retryer, "retryer");
}
/**
@ -221,8 +93,7 @@ public class ExternalIdsUpdate {
*
* <p>If the external ID already exists, the insert fails with {@link OrmDuplicateKeyException}.
*/
public void insert(ReviewDb db, ExternalId extId)
throws IOException, ConfigInvalidException, OrmException {
public void insert(ReviewDb db, ExternalId extId) throws IOException, OrmException {
insert(db, Collections.singleton(extId));
}
@ -232,16 +103,8 @@ public class ExternalIdsUpdate {
* <p>If any of the external ID already exists, the insert fails with {@link
* OrmDuplicateKeyException}.
*/
public void insert(ReviewDb db, Collection<ExternalId> extIds)
throws IOException, ConfigInvalidException, OrmException {
public void insert(ReviewDb db, Collection<ExternalId> extIds) throws IOException, OrmException {
db.accountExternalIds().insert(toAccountExternalIds(extIds));
updateNoteMap(
o -> {
for (ExternalId extId : extIds) {
insert(o.rw(), o.ins(), o.noteMap(), extId);
}
});
evictAccounts(extIds);
}
@ -250,8 +113,7 @@ public class ExternalIdsUpdate {
*
* <p>If the external ID already exists, it is overwritten, otherwise it is inserted.
*/
public void upsert(ReviewDb db, ExternalId extId)
throws IOException, ConfigInvalidException, OrmException {
public void upsert(ReviewDb db, ExternalId extId) throws IOException, OrmException {
upsert(db, Collections.singleton(extId));
}
@ -260,16 +122,8 @@ public class ExternalIdsUpdate {
*
* <p>If any of the external IDs already exists, it is overwritten. New external IDs are inserted.
*/
public void upsert(ReviewDb db, Collection<ExternalId> extIds)
throws IOException, ConfigInvalidException, OrmException {
public void upsert(ReviewDb db, Collection<ExternalId> extIds) throws IOException, OrmException {
db.accountExternalIds().upsert(toAccountExternalIds(extIds));
updateNoteMap(
o -> {
for (ExternalId extId : extIds) {
upsert(o.rw(), o.ins(), o.noteMap(), extId);
}
});
evictAccounts(extIds);
}
@ -279,8 +133,7 @@ public class ExternalIdsUpdate {
* <p>The deletion fails with {@link IllegalStateException} if there is an existing external ID
* that has the same key, but otherwise doesn't match the specified external ID.
*/
public void delete(ReviewDb db, ExternalId extId)
throws IOException, ConfigInvalidException, OrmException {
public void delete(ReviewDb db, ExternalId extId) throws IOException, OrmException {
delete(db, Collections.singleton(extId));
}
@ -291,16 +144,8 @@ public class ExternalIdsUpdate {
* that has the same key as any of the external IDs that should be deleted, but otherwise doesn't
* match the that external ID.
*/
public void delete(ReviewDb db, Collection<ExternalId> extIds)
throws IOException, ConfigInvalidException, OrmException {
public void delete(ReviewDb db, Collection<ExternalId> extIds) throws IOException, OrmException {
db.accountExternalIds().delete(toAccountExternalIds(extIds));
updateNoteMap(
o -> {
for (ExternalId extId : extIds) {
remove(o.rw(), o.noteMap(), extId);
}
});
evictAccounts(extIds);
}
@ -311,7 +156,7 @@ public class ExternalIdsUpdate {
* another account the deletion fails with {@link IllegalStateException}.
*/
public void delete(ReviewDb db, Account.Id accountId, ExternalId.Key extIdKey)
throws IOException, ConfigInvalidException, OrmException {
throws IOException, OrmException {
delete(db, accountId, Collections.singleton(extIdKey));
}
@ -322,21 +167,13 @@ public class ExternalIdsUpdate {
* external IDs belongs to another account the deletion fails with {@link IllegalStateException}.
*/
public void delete(ReviewDb db, Account.Id accountId, Collection<ExternalId.Key> extIdKeys)
throws IOException, ConfigInvalidException, OrmException {
throws IOException, OrmException {
db.accountExternalIds().deleteKeys(toAccountExternalIdKeys(extIdKeys));
updateNoteMap(
o -> {
for (ExternalId.Key extIdKey : extIdKeys) {
remove(o.rw(), o.noteMap(), accountId, extIdKey);
}
});
accountCache.evict(accountId);
}
/** Deletes all external IDs of the specified account. */
public void deleteAll(ReviewDb db, Account.Id accountId)
throws IOException, ConfigInvalidException, OrmException {
public void deleteAll(ReviewDb db, Account.Id accountId) throws IOException, OrmException {
delete(db, ExternalId.from(db.accountExternalIds().byAccount(accountId).toList()));
}
@ -356,22 +193,11 @@ public class ExternalIdsUpdate {
Account.Id accountId,
Collection<ExternalId.Key> toDelete,
Collection<ExternalId> toAdd)
throws IOException, ConfigInvalidException, OrmException {
throws IOException, OrmException {
checkSameAccount(toAdd, accountId);
db.accountExternalIds().deleteKeys(toAccountExternalIdKeys(toDelete));
db.accountExternalIds().insert(toAccountExternalIds(toAdd));
updateNoteMap(
o -> {
for (ExternalId.Key extIdKey : toDelete) {
remove(o.rw(), o.noteMap(), accountId, extIdKey);
}
for (ExternalId extId : toAdd) {
insert(o.rw(), o.ins(), o.noteMap(), extId);
}
});
accountCache.evict(accountId);
}
@ -382,7 +208,7 @@ public class ExternalIdsUpdate {
* {@link IllegalStateException}.
*/
public void replace(ReviewDb db, ExternalId toDelete, ExternalId toAdd)
throws IOException, ConfigInvalidException, OrmException {
throws IOException, OrmException {
replace(db, Collections.singleton(toDelete), Collections.singleton(toAdd));
}
@ -398,7 +224,7 @@ public class ExternalIdsUpdate {
* IllegalStateException}.
*/
public void replace(ReviewDb db, Collection<ExternalId> toDelete, Collection<ExternalId> toAdd)
throws IOException, ConfigInvalidException, OrmException {
throws IOException, OrmException {
Account.Id accountId = checkSameAccount(Iterables.concat(toDelete, toAdd));
if (accountId == null) {
// toDelete and toAdd are empty -> nothing to do
@ -440,228 +266,9 @@ public class ExternalIdsUpdate {
return accountId;
}
/**
* Inserts a new external ID and sets it in the note map.
*
* <p>If the external ID already exists, the insert fails with {@link OrmDuplicateKeyException}.
*/
public static void insert(RevWalk rw, ObjectInserter ins, NoteMap noteMap, ExternalId extId)
throws OrmDuplicateKeyException, ConfigInvalidException, IOException {
if (noteMap.contains(extId.key().sha1())) {
throw new OrmDuplicateKeyException(
String.format("external id %s already exists", extId.key().get()));
}
upsert(rw, ins, noteMap, extId);
}
/**
* Insert or updates an new external ID and sets it in the note map.
*
* <p>If the external ID already exists it is overwritten.
*/
public static void upsert(RevWalk rw, ObjectInserter ins, NoteMap noteMap, ExternalId extId)
throws IOException, ConfigInvalidException {
ObjectId noteId = extId.key().sha1();
Config c = new Config();
if (noteMap.contains(extId.key().sha1())) {
byte[] raw =
rw.getObjectReader().open(noteMap.get(noteId), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
try {
c.fromText(new String(raw, UTF_8));
} catch (ConfigInvalidException e) {
throw new ConfigInvalidException(
String.format("Invalid external id config for note %s: %s", noteId, e.getMessage()));
}
}
extId.writeToConfig(c);
byte[] raw = c.toText().getBytes(UTF_8);
ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
noteMap.set(noteId, dataBlob);
}
/**
* Removes an external ID from the note map.
*
* <p>The removal fails with {@link IllegalStateException} if there is an existing external ID
* that has the same key, but otherwise doesn't match the specified external ID.
*/
public static void remove(RevWalk rw, NoteMap noteMap, ExternalId extId)
throws IOException, ConfigInvalidException {
ObjectId noteId = extId.key().sha1();
if (!noteMap.contains(noteId)) {
return;
}
byte[] raw =
rw.getObjectReader().open(noteMap.get(noteId), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
ExternalId actualExtId = ExternalId.parse(noteId.name(), raw);
checkState(
extId.equals(actualExtId),
"external id %s should be removed, but it's not matching the actual external id %s",
extId.toString(),
actualExtId.toString());
noteMap.remove(noteId);
}
/**
* Removes an external ID from the note map by external ID key.
*
* <p>The external ID is only deleted if it belongs to the specified account. If the external IDs
* belongs to another account the deletion fails with {@link IllegalStateException}.
*/
private static void remove(
RevWalk rw, NoteMap noteMap, Account.Id accountId, ExternalId.Key extIdKey)
throws IOException, ConfigInvalidException {
ObjectId noteId = extIdKey.sha1();
if (!noteMap.contains(noteId)) {
return;
}
byte[] raw =
rw.getObjectReader().open(noteMap.get(noteId), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
ExternalId extId = ExternalId.parse(noteId.name(), raw);
checkState(
accountId.equals(extId.accountId()),
"external id %s should be removed for account %s,"
+ " but external id belongs to account %s",
extIdKey.get(),
accountId.get(),
extId.accountId().get());
noteMap.remove(noteId);
}
private void updateNoteMap(MyConsumer<OpenRepo> update)
throws IOException, ConfigInvalidException, OrmException {
try (Repository repo = repoManager.openRepository(allUsersName);
RevWalk rw = new RevWalk(repo);
ObjectInserter ins = repo.newObjectInserter()) {
retryer.call(new TryNoteMapUpdate(repo, rw, ins, update));
} catch (ExecutionException | RetryException e) {
if (e.getCause() != null) {
Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
Throwables.throwIfInstanceOf(e.getCause(), ConfigInvalidException.class);
Throwables.throwIfInstanceOf(e.getCause(), OrmException.class);
}
throw new OrmException(e);
}
}
private void commit(
Repository repo, RevWalk rw, ObjectInserter ins, ObjectId rev, NoteMap noteMap)
throws IOException {
commit(repo, rw, ins, rev, noteMap, COMMIT_MSG, committerIdent, authorIdent);
}
/** Commits updates to the external IDs. */
public static void commit(
Repository repo,
RevWalk rw,
ObjectInserter ins,
ObjectId rev,
NoteMap noteMap,
String commitMessage,
PersonIdent committerIdent,
PersonIdent authorIdent)
throws IOException {
CommitBuilder cb = new CommitBuilder();
cb.setMessage(commitMessage);
cb.setTreeId(noteMap.writeTree(ins));
cb.setAuthor(authorIdent);
cb.setCommitter(committerIdent);
if (!rev.equals(ObjectId.zeroId())) {
cb.setParentId(rev);
} else {
cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
}
if (cb.getTreeId() == null) {
if (rev.equals(ObjectId.zeroId())) {
cb.setTreeId(emptyTree(ins)); // No parent, assume empty tree.
} else {
RevCommit p = rw.parseCommit(rev);
cb.setTreeId(p.getTree()); // Copy tree from parent.
}
}
ObjectId commitId = ins.insert(cb);
ins.flush();
RefUpdate u = repo.updateRef(RefNames.REFS_EXTERNAL_IDS);
u.setRefLogIdent(committerIdent);
u.setRefLogMessage("Update external IDs", false);
u.setExpectedOldObjectId(rev);
u.setNewObjectId(commitId);
RefUpdate.Result res = u.update();
switch (res) {
case NEW:
case FAST_FORWARD:
case NO_CHANGE:
case RENAMED:
case FORCED:
break;
case LOCK_FAILURE:
throw new LockFailureException("Updating external IDs failed with " + res);
case IO_FAILURE:
case NOT_ATTEMPTED:
case REJECTED:
case REJECTED_CURRENT_BRANCH:
default:
throw new IOException("Updating external IDs failed with " + res);
}
}
private static ObjectId emptyTree(ObjectInserter ins) throws IOException {
return ins.insert(OBJ_TREE, new byte[] {});
}
private void evictAccounts(Collection<ExternalId> extIds) throws IOException {
for (Account.Id id : extIds.stream().map(ExternalId::accountId).collect(toSet())) {
accountCache.evict(id);
}
}
private static interface MyConsumer<T> {
void accept(T t) throws IOException, ConfigInvalidException, OrmException;
}
@AutoValue
abstract static class OpenRepo {
static OpenRepo create(Repository repo, RevWalk rw, ObjectInserter ins, NoteMap noteMap) {
return new AutoValue_ExternalIdsUpdate_OpenRepo(repo, rw, ins, noteMap);
}
abstract Repository repo();
abstract RevWalk rw();
abstract ObjectInserter ins();
abstract NoteMap noteMap();
}
private class TryNoteMapUpdate implements Callable<Void> {
private final Repository repo;
private final RevWalk rw;
private final ObjectInserter ins;
private final MyConsumer<OpenRepo> update;
private TryNoteMapUpdate(
Repository repo, RevWalk rw, ObjectInserter ins, MyConsumer<OpenRepo> update) {
this.repo = repo;
this.rw = rw;
this.ins = ins;
this.update = update;
}
@Override
public Void call() throws Exception {
ObjectId rev = readRevision(repo);
afterReadRevision.run();
NoteMap noteMap = readNoteMap(rw, rev);
update.accept(OpenRepo.create(repo, rw, ins, noteMap));
commit(repo, rw, ins, rev, noteMap);
return null;
}
}
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java
View File

@ -99,8 +99,7 @@ public class PutHttpPassword implements RestModifyView<AccountResource, Input> {
}
public Response<String> apply(IdentifiedUser user, String newPassword)
throws ResourceNotFoundException, ResourceConflictException, OrmException, IOException,
ConfigInvalidException {
throws ResourceNotFoundException, ResourceConflictException, OrmException, IOException {
if (user.getUserName() == null) {
throw new ResourceConflictException("username must be set");
}