opendev/gerrit
Code Issues Proposed changes

AccountIndex: Add fields that allow to detect an account document as stale

Browse Source 
To be able to tell whether an account is stale in the index we need to
record a state ID for each source that contributes to the account
document (user branch + external IDs). For the user branch we can record
its ref state (similar to how the change index records ref states / ref
state patterns for all refs that contribute to a change). For the
external IDs branch the ref state is not sufficient because it changes
whenever any external ID is updated and we don't want to detect all
accounts as stale after each external ID update. Hence for the external
IDs we record the IDs of the note blobs instead.

Change-Id: I7f0383cb46e461cc5158bf13f31d608a927bfd49
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-08-17 17:12:43 +02:00
parent 1b9a0cf038
commit 916112ed91
13 changed files with 272 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
View File

@@ -30,6 +30,7 @@ import com.google.gerrit.server.account.WatchConfig.NotifyType;
import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.group.Groups;
import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.query.account.InternalAccountQuery;
@@ -76,15 +77,18 @@ public class AccountCacheImpl implements AccountCache {
};
}
private final AllUsersName allUsersName;
private final LoadingCache<Account.Id, Optional<AccountState>> byId;
private final LoadingCache<String, Optional<Account.Id>> byName;
private final Provider<AccountIndexer> indexer;
@Inject
AccountCacheImpl(
AllUsersName allUsersName,
@Named(BYID_NAME) LoadingCache<Account.Id, Optional<AccountState>> byId,
@Named(BYUSER_NAME) LoadingCache<String, Optional<Account.Id>> byUsername,
Provider<AccountIndexer> indexer) {
this.allUsersName = allUsersName;
this.byId = byId;
this.byName = byUsername;
this.indexer = indexer;
@@ -142,16 +146,21 @@ public class AccountCacheImpl implements AccountCache {
}
}
private static AccountState missing(Account.Id accountId) {
private AccountState missing(Account.Id accountId) {
Account account = new Account(accountId, TimeUtil.nowTs());
account.setActive(false);
Set<AccountGroup.UUID> anon = ImmutableSet.of();
return new AccountState(
account, anon, Collections.emptySet(), new HashMap<ProjectWatchKey, Set<NotifyType>>());
allUsersName,
account,
anon,
Collections.emptySet(),
new HashMap<ProjectWatchKey, Set<NotifyType>>());
}
static class ByIdLoader extends CacheLoader<Account.Id, Optional<AccountState>> {
private final SchemaFactory<ReviewDb> schema;
private final AllUsersName allUsersName;
private final Accounts accounts;
private final GroupCache groupCache;
private final Groups groups;
@@ -163,6 +172,7 @@ public class AccountCacheImpl implements AccountCache {
@Inject
ByIdLoader(
SchemaFactory<ReviewDb> sf,
AllUsersName allUsersName,
Accounts accounts,
GroupCache groupCache,
Groups groups,
@@ -170,8 +180,9 @@ public class AccountCacheImpl implements AccountCache {
@Named(BYUSER_NAME) LoadingCache<String, Optional<Account.Id>> byUsername,
Provider<WatchConfig.Accessor> watchConfig,
ExternalIds externalIds) {
this.accounts = accounts;
this.schema = sf;
this.allUsersName = allUsersName;
this.accounts = accounts;
this.groupCache = groupCache;
this.groups = groups;
this.loader = loader;
@@ -219,6 +230,7 @@ public class AccountCacheImpl implements AccountCache {
return Optional.of(
new AccountState(
allUsersName,
account,
internalGroups,
externalIds.byAccount(who),

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

@@ -29,6 +29,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.WatchConfig.NotifyType;
import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.config.AllUsersName;
import java.util.Collection;
import java.util.HashSet;
import java.util.Map;
@@ -43,6 +44,7 @@ public class AccountState {
public static final Function<AccountState, Account.Id> ACCOUNT_ID_FUNCTION =
a -> a.getAccount().getId();
private final AllUsersName allUsersName;
private final Account account;
private final Set<AccountGroup.UUID> internalGroups;
private final Collection<ExternalId> externalIds;
@@ -50,10 +52,12 @@ public class AccountState {
private Cache<IdentifiedUser.PropertyKey<Object>, Object> properties;
public AccountState(
AllUsersName allUsersName,
Account account,
Set<AccountGroup.UUID> actualGroups,
Collection<ExternalId> externalIds,
Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
this.allUsersName = allUsersName;
this.account = account;
this.internalGroups = actualGroups;
this.externalIds = externalIds;
@@ -61,6 +65,10 @@ public class AccountState {
this.account.setUserName(getUserName(externalIds));
}
public AllUsersName getAllUsersNameForIndexing() {
return allUsersName;
}
/** Get the cached account metadata. */
public Account getAccount() {
return account;

14
gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalId.java
View File

@@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.hash.Hashing;
@@ -31,6 +32,7 @@ import java.util.Objects;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@AutoValue
@@ -176,7 +178,8 @@ public abstract class ExternalId implements Serializable {
extId.key(), extId.accountId(), extId.email(), extId.password(), blobId);
}
static ExternalId create(
@VisibleForTesting
public static ExternalId create(
Key key,
Account.Id accountId,
@Nullable String email,
@@ -305,6 +308,15 @@ public abstract class ExternalId implements Serializable {
return key().isScheme(scheme);
}
public byte[] toByteArray() {
checkState(blobId() != null, "Missing blobId in external ID %s", key().get());
byte[] b = new byte[2 * Constants.OBJECT_ID_STRING_LENGTH + 1];
key().sha1().copyTo(b, 0);
b[Constants.OBJECT_ID_STRING_LENGTH] = ':';
blobId().copyTo(b, Constants.OBJECT_ID_STRING_LENGTH + 1);
return b;
}
/**
* For checking if two external IDs are equals the blobId is excluded and external IDs that have
* different blob IDs but identical other fields are considered equal. This way an external ID

65
gerrit-server/src/main/java/com/google/gerrit/server/index/RefState.java Normal file
View File

@@ -0,0 +1,65 @@
// 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.index;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.auto.value.AutoValue;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Project;
import java.io.IOException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@AutoValue
public abstract class RefState {
public static RefState create(String ref, String sha) {
return new AutoValue_RefState(ref, ObjectId.fromString(sha));
}
public static RefState create(String ref, @Nullable ObjectId id) {
return new AutoValue_RefState(ref, firstNonNull(id, ObjectId.zeroId()));
}
public static RefState of(Ref ref) {
return new AutoValue_RefState(ref.getName(), ref.getObjectId());
}
public byte[] toByteArray(Project.NameKey project) {
byte[] a = (project.toString() + ':' + ref() + ':').getBytes(UTF_8);
byte[] b = new byte[a.length + Constants.OBJECT_ID_STRING_LENGTH];
System.arraycopy(a, 0, b, 0, a.length);
id().copyTo(b, a.length);
return b;
}
public static void check(boolean condition, String str) {
checkArgument(condition, "invalid RefState: %s", str);
}
public abstract String ref();
public abstract ObjectId id();
public boolean match(Repository repo) throws IOException {
Ref ref = repo.exactRef(ref());
ObjectId expected = ref != null ? ref.getObjectId() : ObjectId.zeroId();
return id().equals(expected);
}
}

44
gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountField.java
View File

@@ -17,20 +17,26 @@ package com.google.gerrit.server.index.account;
import static com.google.gerrit.index.FieldDef.exact;
import static com.google.gerrit.index.FieldDef.integer;
import static com.google.gerrit.index.FieldDef.prefix;
import static com.google.gerrit.index.FieldDef.storedOnly;
import static com.google.gerrit.index.FieldDef.timestamp;
import static java.util.stream.Collectors.toSet;
import com.google.common.base.Predicates;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.SchemaUtil;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.index.RefState;
import java.sql.Timestamp;
import java.util.Collections;
import java.util.Locale;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
/** Secondary index schemas for accounts. */
public class AccountField {
@@ -98,5 +104,43 @@ public class AccountField {
.transform(k -> k.project().get())
.toSet());
/**
* All values of all refs that were used in the course of indexing this document, except the
* refs/meta/external-ids notes branch which is handled specially (see {@link
* #EXTERNAL_ID_STATE}).
*
* <p>Emitted as UTF-8 encoded strings of the form {@code project:ref/name:[hex sha]}.
*/
public static final FieldDef<AccountState, Iterable<byte[]>> REF_STATE =
storedOnly("ref_state")
.buildRepeatable(
a -> {
if (a.getAccount().getMetaId() == null) {
return ImmutableList.of();
}
return ImmutableList.of(
RefState.create(
RefNames.refsUsers(a.getAccount().getId()),
ObjectId.fromString(a.getAccount().getMetaId()))
.toByteArray(a.getAllUsersNameForIndexing()));
});
/**
* All note values of all external IDs that were used in the course of indexing this document.
*
* <p>Emitted as UTF-8 encoded strings of the form {@code [hex sha of external ID]:[hex sha of
* note blob]}, or with other words {@code [note ID]:[note data ID]}.
*/
public static final FieldDef<AccountState, Iterable<byte[]>> EXTERNAL_ID_STATE =
storedOnly("external_id_state")
.buildRepeatable(
a ->
a.getExternalIds()
.stream()
.filter(e -> e.blobId() != null)
.map(e -> e.toByteArray())
.collect(toSet()));
private AccountField() {}
}

5
gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
View File

@@ -34,7 +34,10 @@ public class AccountSchemaDefinitions extends SchemaDefinitions<AccountState> {
AccountField.USERNAME,
AccountField.WATCHED_PROJECT);
static final Schema<AccountState> V5 = schema(V4, AccountField.PREFERRED_EMAIL);
@Deprecated static final Schema<AccountState> V5 = schema(V4, AccountField.PREFERRED_EMAIL);
static final Schema<AccountState> V6 =
schema(V5, AccountField.REF_STATE, AccountField.EXTERNAL_ID_STATE);
public static final String NAME = "accounts";
public static final AccountSchemaDefinitions INSTANCE = new AccountSchemaDefinitions();

2
gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
View File

@@ -50,7 +50,7 @@ import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.index.change.StalenessChecker.RefState;
import com.google.gerrit.server.index.RefState;
import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.notedb.ChangeNotes;

41
gerrit-server/src/main/java/com/google/gerrit/server/index/change/StalenessChecker.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.server.index.change;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -35,6 +34,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.RefState;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.query.change.ChangeData;
@@ -47,8 +47,6 @@ import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
@@ -221,43 +219,6 @@ public class StalenessChecker {
}
}
@AutoValue
public abstract static class RefState {
static RefState create(String ref, String sha) {
return new AutoValue_StalenessChecker_RefState(ref, ObjectId.fromString(sha));
}
static RefState create(String ref, @Nullable ObjectId id) {
return new AutoValue_StalenessChecker_RefState(ref, firstNonNull(id, ObjectId.zeroId()));
}
static RefState of(Ref ref) {
return new AutoValue_StalenessChecker_RefState(ref.getName(), ref.getObjectId());
}
byte[] toByteArray(Project.NameKey project) {
byte[] a = (project.toString() + ':' + ref() + ':').getBytes(UTF_8);
byte[] b = new byte[a.length + Constants.OBJECT_ID_STRING_LENGTH];
System.arraycopy(a, 0, b, 0, a.length);
id().copyTo(b, a.length);
return b;
}
private static void check(boolean condition, String str) {
checkArgument(condition, "invalid RefState: %s", str);
}
abstract String ref();
abstract ObjectId id();
private boolean match(Repository repo) throws IOException {
Ref ref = repo.exactRef(ref());
ObjectId expected = ref != null ? ref.getObjectId() : ObjectId.zeroId();
return id().equals(expected);
}
}
/**
* Pattern for matching refs.
*

94
gerrit-server/src/test/java/com/google/gerrit/server/index/account/AccountFieldTest.java Normal file
View File

@@ -0,0 +1,94 @@
// 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.index.account;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.testutil.GerritBaseTests;
import java.util.List;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
public class AccountFieldTest extends GerritBaseTests {
@Test
public void refStateFieldValues() throws Exception {
AllUsersName allUsersName = new AllUsersName(AllUsersNameProvider.DEFAULT);
Account account = new Account(new Account.Id(1), TimeUtil.nowTs());
String metaId = "0e39795bb25dc914118224995c53c5c36923a461";
account.setMetaId(metaId);
List<String> values =
toStrings(
AccountField.REF_STATE.get(
new AccountState(
allUsersName,
account,
ImmutableSet.of(),
ImmutableSet.of(),
ImmutableMap.of())));
assertThat(values).hasSize(1);
String expectedValue =
allUsersName.get() + ":" + RefNames.refsUsers(account.getId()) + ":" + metaId;
assertThat(Iterables.getOnlyElement(values)).isEqualTo(expectedValue);
}
@Test
public void externalIdStateFieldValues() throws Exception {
Account.Id id = new Account.Id(1);
Account account = new Account(id, TimeUtil.nowTs());
ExternalId extId1 =
ExternalId.create(
ExternalId.Key.create(ExternalId.SCHEME_MAILTO, "foo.bar@example.com"),
id,
"foo.bar@example.com",
null,
ObjectId.fromString("1b9a0cf038ea38a0ab08617c39aa8e28413a27ca"));
ExternalId extId2 =
ExternalId.create(
ExternalId.Key.create(ExternalId.SCHEME_USERNAME, "foo"),
id,
null,
"secret",
ObjectId.fromString("5b3a73dc9a668a5b89b5f049225261e3e3291d1a"));
List<String> values =
toStrings(
AccountField.EXTERNAL_ID_STATE.get(
new AccountState(
null,
account,
ImmutableSet.of(),
ImmutableSet.of(extId1, extId2),
ImmutableMap.of())));
String expectedValue1 = extId1.key().sha1().name() + ":" + extId1.blobId().name();
String expectedValue2 = extId2.key().sha1().name() + ":" + extId2.blobId().name();
assertThat(values).containsExactly(expectedValue1, expectedValue2);
}
private List<String> toStrings(Iterable<byte[]> values) {
return Streams.stream(values).map(v -> new String(v, UTF_8)).collect(toList());
}
}

2
gerrit-server/src/test/java/com/google/gerrit/server/index/change/StalenessCheckerTest.java
View File

@@ -28,7 +28,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.StalenessChecker.RefState;
import com.google.gerrit.server.index.RefState;
import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern;
import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.testutil.GerritBaseTests;

8
gerrit-server/src/test/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
View File

@@ -25,6 +25,8 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.mail.Address;
import java.util.Arrays;
import java.util.Collections;
@@ -383,6 +385,10 @@ public class FromAddressGeneratorProviderTest {
account.setFullName(name);
account.setPreferredEmail(email);
return new AccountState(
account, Collections.emptySet(), Collections.emptySet(), new HashMap<>());
new AllUsersName(AllUsersNameProvider.DEFAULT),
account,
Collections.emptySet(),
Collections.emptySet(),
new HashMap<>());
}
}

9
gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java
View File

@@ -20,6 +20,8 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
import java.util.HashMap;
import java.util.Map;
@@ -78,6 +80,11 @@ public class FakeAccountCache implements AccountCache {
}
private static AccountState newState(Account account) {
return new AccountState(account, ImmutableSet.of(), ImmutableSet.of(), new HashMap<>());
return new AccountState(
new AllUsersName(AllUsersNameProvider.DEFAULT),
account,
ImmutableSet.of(),
ImmutableSet.of(),
new HashMap<>());
}
}