Propagate failure if reading external IDs from NoteDb fails
When reading external IDs from NoteDb was enabled
ExternalIds#byAccount(...) and ExternalIds#byEmail(...) returned an
empty set if reading from NoteDb failed.
Change-Id: I453beea3827d65a64d80eb2d9d9842602c15b27e
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index b203b83..42709af 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -25,6 +25,7 @@
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.Sandboxed;
@@ -35,6 +36,7 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.externalids.DisabledExternalIdCache;
import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdReader;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
import com.google.gerrit.server.config.AllUsersName;
@@ -54,15 +56,18 @@
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.Test;
@Sandboxed
public class ExternalIdIT extends AbstractDaemonTest {
@Inject private AllUsersName allUsers;
-
@Inject private ExternalIdsUpdate.Server extIdsUpdate;
-
@Inject private ExternalIds externalIds;
+ @Inject private ExternalIdReader externalIdReader;
@Test
public void getExternalIDs() throws Exception {
@@ -267,4 +272,40 @@
ExternalId extId = externalIds.get(db, extIdKey);
assertThat(extId.accountId()).isEqualTo(accountId);
}
+
+ @Test
+ @GerritConfig(name = "user.readExternalIdsFromGit", value = "true")
+ public void byAccountFailIfReadingExternalIdsFails() throws Exception {
+ externalIdReader.setFailOnLoad(true);
+
+ // update external ID branch so that external IDs need to be reloaded
+ insertExtIdBehindGerritsBack(ExternalId.create("foo", "bar", admin.id));
+
+ exception.expect(IOException.class);
+ externalIds.byAccount(db, admin.id);
+ }
+
+ @Test
+ @GerritConfig(name = "user.readExternalIdsFromGit", value = "true")
+ public void byEmailFailIfReadingExternalIdsFails() throws Exception {
+ externalIdReader.setFailOnLoad(true);
+
+ // update external ID branch so that external IDs need to be reloaded
+ insertExtIdBehindGerritsBack(ExternalId.create("foo", "bar", admin.id));
+
+ exception.expect(IOException.class);
+ externalIds.byEmail(db, admin.email);
+ }
+
+ private void insertExtIdBehindGerritsBack(ExternalId extId) throws Exception {
+ try (Repository repo = repoManager.openRepository(allUsers);
+ RevWalk rw = new RevWalk(repo);
+ ObjectInserter ins = repo.newObjectInserter()) {
+ ObjectId rev = ExternalIdReader.readRevision(repo);
+ NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+ ExternalIdsUpdate.insert(rw, ins, noteMap, extId);
+ ExternalIdsUpdate.commit(
+ repo, rw, ins, rev, noteMap, "insert new ID", serverIdent.get(), serverIdent.get());
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java
index 7fb61fc..6713021 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java
@@ -28,7 +28,6 @@
import com.google.inject.Singleton;
import com.google.inject.name.Named;
import java.io.IOException;
-import java.util.Collections;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.locks.Lock;
@@ -222,8 +221,7 @@
try {
return extIdsByAccount.get(externalIdReader.readRevision()).get(accountId);
} catch (ExecutionException e) {
- log.warn("Cannot list external ids by account", e);
- return Collections.emptySet();
+ throw new IOException("Cannot list external ids by account", e);
}
}
@@ -237,8 +235,7 @@
.filter(e -> email.equals(e.email()))
.collect(toSet());
} catch (ExecutionException e) {
- log.warn("Cannot list external ids by email", e);
- return Collections.emptySet();
+ throw new IOException("Cannot list external ids by email", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java
index 7b607dc..d9f0c00 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdReader.java
@@ -16,6 +16,7 @@
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -76,6 +77,7 @@
private final boolean readFromGit;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
+ private boolean failOnLoad = false;
@Inject
ExternalIdReader(
@@ -85,6 +87,11 @@
this.allUsersName = allUsersName;
}
+ @VisibleForTesting
+ public void setFailOnLoad(boolean failOnLoad) {
+ this.failOnLoad = failOnLoad;
+ }
+
boolean readFromGit() {
return readFromGit;
}
@@ -97,6 +104,8 @@
/** Reads and returns all external IDs. */
Set<ExternalId> all(ReviewDb db) throws IOException, OrmException {
+ checkReadEnabled();
+
if (readFromGit) {
try (Repository repo = repoManager.openRepository(allUsersName)) {
return all(repo, readRevision(repo));
@@ -111,6 +120,8 @@
* branch.
*/
Set<ExternalId> all(ObjectId rev) throws IOException {
+ checkReadEnabled();
+
try (Repository repo = repoManager.openRepository(allUsersName)) {
return all(repo, rev);
}
@@ -142,6 +153,8 @@
@Nullable
ExternalId get(ReviewDb db, ExternalId.Key key)
throws IOException, ConfigInvalidException, OrmException {
+ checkReadEnabled();
+
if (readFromGit) {
try (Repository repo = repoManager.openRepository(allUsersName);
RevWalk rw = new RevWalk(repo)) {
@@ -159,6 +172,8 @@
/** Reads and returns the specified external ID from the given revision. */
@Nullable
ExternalId get(ExternalId.Key key, ObjectId rev) throws IOException, ConfigInvalidException {
+ checkReadEnabled();
+
if (rev.equals(ObjectId.zeroId())) {
return null;
}
@@ -181,4 +196,10 @@
rw.getObjectReader().open(noteMap.get(noteId), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
return ExternalId.parse(noteId.name(), raw);
}
+
+ private void checkReadEnabled() throws IOException {
+ if (failOnLoad) {
+ throw new IOException("Reading from external IDs is disabled");
+ }
+ }
}
