Merge "Dashboard: strip the Invalid-Prolog-Rules-Label-Name prefix from labels"
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index 252eb61..c9ca972 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -214,14 +214,13 @@
return false;
}
- /** Returns the full commit message for the given project at the given patchset revision */
- public String getFullCommitMessage(Project.NameKey project, PatchSet patchSet)
- throws IOException {
+ /** Returns the commit for the given project at the given patchset revision */
+ public RevCommit getRevCommit(Project.NameKey project, PatchSet patchSet) throws IOException {
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {
RevCommit src = rw.parseCommit(ObjectId.fromString(patchSet.getRevision().get()));
rw.parseBody(src);
- return src.getFullMessage();
+ return src;
}
}
}
diff --git a/java/com/google/gerrit/server/account/Emails.java b/java/com/google/gerrit/server/account/Emails.java
index 8a48167..e91ce49 100644
--- a/java/com/google/gerrit/server/account/Emails.java
+++ b/java/com/google/gerrit/server/account/Emails.java
@@ -95,6 +95,16 @@
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);
diff --git a/java/com/google/gerrit/server/rules/PrologEnvironment.java b/java/com/google/gerrit/server/rules/PrologEnvironment.java
index 083898b..412e0f9 100644
--- a/java/com/google/gerrit/server/rules/PrologEnvironment.java
+++ b/java/com/google/gerrit/server/rules/PrologEnvironment.java
@@ -18,6 +18,7 @@
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchListCache;
@@ -176,6 +177,7 @@
private final int reductionLimit;
private final int compileLimit;
private final PatchSetUtil patchsetUtil;
+ private Emails emails;
@Inject
Args(
@@ -187,7 +189,8 @@
IdentifiedUser.GenericFactory userFactory,
Provider<AnonymousUser> anonymousUser,
@GerritServerConfig Config config,
- PatchSetUtil patchsetUtil) {
+ PatchSetUtil patchsetUtil,
+ Emails emails) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.repositoryManager = repositoryManager;
@@ -196,6 +199,7 @@
this.userFactory = userFactory;
this.anonymousUser = anonymousUser;
this.patchsetUtil = patchsetUtil;
+ this.emails = emails;
int limit = config.getInt("rules", null, "reductionLimit", 100000);
reductionLimit = limit <= 0 ? Integer.MAX_VALUE : limit;
@@ -247,5 +251,9 @@
public PatchSetUtil getPatchsetUtil() {
return patchsetUtil;
}
+
+ public Emails getEmails() {
+ return emails;
+ }
}
}
diff --git a/java/com/google/gerrit/server/rules/StoredValues.java b/java/com/google/gerrit/server/rules/StoredValues.java
index 6770732..8b9cfe3 100644
--- a/java/com/google/gerrit/server/rules/StoredValues.java
+++ b/java/com/google/gerrit/server/rules/StoredValues.java
@@ -20,7 +20,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
@@ -34,8 +33,6 @@
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -47,6 +44,7 @@
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
public final class StoredValues {
public static final StoredValue<Accounts> ACCOUNTS = create(Accounts.class);
@@ -74,32 +72,16 @@
}
}
- public static final StoredValue<PatchSetInfo> PATCH_SET_INFO =
- new StoredValue<PatchSetInfo>() {
+ public static final StoredValue<RevCommit> COMMIT =
+ new StoredValue<RevCommit>() {
@Override
- public PatchSetInfo createValue(Prolog engine) {
- Change change = getChange(engine);
- PatchSet ps = getPatchSet(engine);
- PrologEnvironment env = (PrologEnvironment) engine.control;
- PatchSetInfoFactory patchInfoFactory = env.getArgs().getPatchSetInfoFactory();
- try {
- return patchInfoFactory.get(change.getProject(), ps);
- } catch (PatchSetInfoNotAvailableException e) {
- throw new SystemException(e.getMessage());
- }
- }
- };
-
- public static final StoredValue<String> COMMIT_MESSAGE =
- new StoredValue<String>() {
- @Override
- public String createValue(Prolog engine) {
+ public RevCommit createValue(Prolog engine) {
Change change = getChange(engine);
PatchSet ps = getPatchSet(engine);
PrologEnvironment env = (PrologEnvironment) engine.control;
PatchSetUtil patchSetUtil = env.getArgs().getPatchsetUtil();
try {
- return patchSetUtil.getFullCommitMessage(change.getProject(), ps);
+ return patchSetUtil.getRevCommit(change.getProject(), ps);
} catch (IOException e) {
throw new SystemException(e.getMessage());
}
diff --git a/java/gerrit/AbstractCommitUserIdentityPredicate.java b/java/gerrit/AbstractCommitUserIdentityPredicate.java
index c2aaa76..1bfc95c 100644
--- a/java/gerrit/AbstractCommitUserIdentityPredicate.java
+++ b/java/gerrit/AbstractCommitUserIdentityPredicate.java
@@ -14,9 +14,12 @@
package gerrit;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.UserIdentity;
+import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.rules.PrologEnvironment;
import com.googlecode.prolog_cafe.exceptions.PrologException;
+import com.googlecode.prolog_cafe.exceptions.SystemException;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Predicate;
@@ -24,6 +27,8 @@
import com.googlecode.prolog_cafe.lang.StructureTerm;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+import java.io.IOException;
+import org.eclipse.jgit.lib.PersonIdent;
abstract class AbstractCommitUserIdentityPredicate extends Predicate.P3 {
private static final SymbolTerm user = SymbolTerm.intern("user", 1);
@@ -36,7 +41,7 @@
cont = n;
}
- protected Operation exec(Prolog engine, UserIdentity userId) throws PrologException {
+ protected Operation exec(Prolog engine, PersonIdent userId) throws PrologException {
engine.setB0();
Term a1 = arg1.dereference();
Term a2 = arg2.dereference();
@@ -46,7 +51,18 @@
Term nameTerm = Prolog.Nil;
Term emailTerm = Prolog.Nil;
- Account.Id id = userId.getAccount();
+ PrologEnvironment env = (PrologEnvironment) engine.control;
+ Emails emails = env.getArgs().getEmails();
+ Account.Id id = null;
+ try {
+ ImmutableSet<Account.Id> ids = emails.getAccountForExternal(userId.getEmailAddress());
+ if (ids.size() == 1) {
+ id = ids.iterator().next();
+ }
+ } catch (IOException e) {
+ throw new SystemException(e.getMessage());
+ }
+
if (id == null) {
idTerm = anonymous;
} else {
@@ -58,7 +74,7 @@
nameTerm = SymbolTerm.create(name);
}
- String email = userId.getEmail();
+ String email = userId.getEmailAddress();
if (email != null && !email.equals("")) {
emailTerm = SymbolTerm.create(email);
}
diff --git a/java/gerrit/BUILD b/java/gerrit/BUILD
index 4644af87..8281d8e 100644
--- a/java/gerrit/BUILD
+++ b/java/gerrit/BUILD
@@ -11,5 +11,6 @@
"//lib/flogger:api",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/prolog:runtime",
+ "@guava//jar",
],
)
diff --git a/java/gerrit/PRED_commit_author_3.java b/java/gerrit/PRED_commit_author_3.java
index a876b5e..998b30e 100644
--- a/java/gerrit/PRED_commit_author_3.java
+++ b/java/gerrit/PRED_commit_author_3.java
@@ -14,13 +14,12 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
public class PRED_commit_author_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_author_3(Term a1, Term a2, Term a3, Operation n) {
@@ -29,8 +28,7 @@
@Override
public Operation exec(Prolog engine) throws PrologException {
- PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
- UserIdentity author = psInfo.getAuthor();
- return exec(engine, author);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ return exec(engine, revCommit.getAuthorIdent());
}
}
diff --git a/java/gerrit/PRED_commit_committer_3.java b/java/gerrit/PRED_commit_committer_3.java
index b24b004..293d8ce 100644
--- a/java/gerrit/PRED_commit_committer_3.java
+++ b/java/gerrit/PRED_commit_committer_3.java
@@ -14,13 +14,12 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
public class PRED_commit_committer_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_committer_3(Term a1, Term a2, Term a3, Operation n) {
@@ -29,8 +28,7 @@
@Override
public Operation exec(Prolog engine) throws PrologException {
- PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
- UserIdentity committer = psInfo.getCommitter();
- return exec(engine, committer);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ return exec(engine, revCommit.getCommitterIdent());
}
}
diff --git a/java/gerrit/PRED_commit_message_1.java b/java/gerrit/PRED_commit_message_1.java
index 05bb4bb..eb996d6 100644
--- a/java/gerrit/PRED_commit_message_1.java
+++ b/java/gerrit/PRED_commit_message_1.java
@@ -21,6 +21,7 @@
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
/**
* Returns the commit message as a symbol
@@ -40,7 +41,8 @@
engine.setB0();
Term a1 = arg1.dereference();
- String commitMessage = StoredValues.COMMIT_MESSAGE.get(engine);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ String commitMessage = revCommit.getFullMessage();
SymbolTerm msg = SymbolTerm.create(commitMessage);
if (!a1.unify(msg, engine.trail)) {
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
index bf01a21..98dca3e 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
@@ -71,21 +71,17 @@
@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 {
diff --git a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
index 314941e..086dd65 100644
--- a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
+++ b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
@@ -49,7 +49,7 @@
bind(PrologEnvironment.Args.class)
.toInstance(
new PrologEnvironment.Args(
- null, null, null, null, null, null, null, cfg, null));
+ null, null, null, null, null, null, null, cfg, null, null));
}
});
}
