opendev/gerrit
Code Issues Proposed changes

Add a method to query accounts only by their external ID's email

Browse Source 
And use this method from the Prolog predicates, in order to remove the
dependency on the index when indexing rules. This fixes the two
remaining tests in RulesIT.

The newly introduced method doesn't query the index to find accounts
by their preferred email. Instead, the external IDs cache is assumed to
be consistent with reality. This method is used in PatchSetInfoFactory
and EventFactory to allow usage of these two classes from an environment
with no index access.

The underlying issue (accounts with no external IDs) is already fixed,
and this change is a first step towards removing the patches required by
it.

Change-Id: I93a91c11f6473dfe9e4d9b56c27a351234b50e56
This commit is contained in:
Maxime Guerreiro 2018-06-27 13:13:19 +00:00
parent 20282d6879
commit 7deacf1f1f
3 changed files with 14 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
java/com/google/gerrit/server/account/Emails.java
View File

@ -95,6 +95,16 @@ public class Emails {
return builder.build();
}
/**
* Returns the accounts with the given email.
*
* <p>This method behaves just like {@link #getAccountFor(String)}, except that accounts are not
* looked up by their preferred email. Thus, this method does not rely on the accounts index.
*/
public ImmutableSet<Account.Id> getAccountForExternal(String email) throws IOException {
return externalIds.byEmail(email).stream().map(ExternalId::accountId).collect(toImmutableSet());
}
private <T> T executeIndexQuery(Action<T> action) throws OrmException {
try {
return retryHelper.execute(ActionType.INDEX_QUERY, action, OrmException.class::isInstance);

5
java/gerrit/AbstractCommitUserIdentityPredicate.java
View File

@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.rules.PrologEnvironment;
import com.google.gwtorm.server.OrmException;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.exceptions.SystemException;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
@ -56,11 +55,11 @@ abstract class AbstractCommitUserIdentityPredicate extends Predicate.P3 {
Emails emails = env.getArgs().getEmails();
Account.Id id = null;
try {
ImmutableSet<Account.Id> ids = emails.getAccountFor(userId.getEmailAddress());
ImmutableSet<Account.Id> ids = emails.getAccountForExternal(userId.getEmailAddress());
if (ids.size() == 1) {
id = ids.iterator().next();
}
} catch (IOException | OrmException e) {
} catch (IOException e) {
throw new SystemException(e.getMessage());
}

8
javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
View File

@ -71,21 +71,17 @@ public class RulesIT extends AbstractDaemonTest {
@Test
public void testUserPredicate() throws Exception {
// This test results in a RULE_ERROR as Prolog tries to find accounts by email, using the index.
// TODO(maximeg) get OK results
modifySubmitRules(
String.format(
"gerrit:commit_author(user(%d), '%s', '%s')",
user.getId().get(), user.fullName, user.email));
assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.RULE_ERROR);
assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}
@Test
public void testCommitAuthorPredicate() throws Exception {
// This test results in a RULE_ERROR as Prolog tries to find accounts by email, using the index.
// TODO(maximeg) get OK results
modifySubmitRules("gerrit:commit_author(Id)");
assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.RULE_ERROR);
assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}
private SubmitRecord.Status statusForRule() throws Exception {