diff --git a/pom.xml b/pom.xml
index 98fd510e66d8e10a7b38c888f945bde3bc68d792..2757a2b002999e0f2f10f4d8958bac92cfece7ce 100644
--- a/pom.xml
+++ b/pom.xml
@@ -5,7 +5,7 @@
<artifactId>git-eca</artifactId>
<version>1.1.0</version>
<properties>
- <eclipse-api-version>0.7.3</eclipse-api-version>
+ <eclipse-api-version>0.7.4</eclipse-api-version>
<compiler-plugin.version>3.8.1</compiler-plugin.version>
<maven.compiler.parameters>true</maven.compiler.parameters>
<maven.compiler.source>11</maven.compiler.source>
diff --git a/src/main/java/org/eclipsefoundation/git/eca/api/models/EclipseUser.java b/src/main/java/org/eclipsefoundation/git/eca/api/models/EclipseUser.java
index d0d0e8c171f439134c39158062748e2605a1949c..443bb8cf78aad9e8b684bd0e7d7e5dbad825abe4 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/api/models/EclipseUser.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/api/models/EclipseUser.java
@@ -14,6 +14,7 @@ package org.eclipsefoundation.git.eca.api.models;
import javax.annotation.Nullable;
import org.eclipsefoundation.git.eca.model.GitUser;
+
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
@@ -37,6 +38,9 @@ public abstract class EclipseUser {
public abstract boolean getIsCommitter();
+ @Nullable
+ public abstract String getGithubHandle();
+
@Nullable
@JsonIgnore
public abstract Boolean getIsBot();
@@ -75,6 +79,8 @@ public abstract class EclipseUser {
public abstract Builder setIsCommitter(boolean isCommitter);
+ public abstract Builder setGithubHandle(@Nullable String githubHandle);
+
@JsonIgnore
public abstract Builder setIsBot(@Nullable Boolean isBot);
diff --git a/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java b/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java
index 811c9d14da69f7d9cdbaba36e45fa16218d964d5..3cb2ea3e279af8110a9dce9a63115d6b32440a08 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java
@@ -11,6 +11,8 @@
*/
package org.eclipsefoundation.git.eca.api.models;
+import java.util.List;
+
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
import com.google.auto.value.AutoValue;
@@ -23,8 +25,15 @@ import com.google.auto.value.AutoValue;
@JsonDeserialize(builder = AutoValue_GithubCommit.Builder.class)
public abstract class GithubCommit {
public abstract String getSha();
+
public abstract CommitData getCommit();
+ public abstract CommitUser getCommitter();
+
+ public abstract CommitUser getAuthor();
+
+ public abstract List<ParentCommit> getParents();
+
public static Builder builder() {
return new AutoValue_GithubCommit.Builder();
}
@@ -33,16 +42,24 @@ public abstract class GithubCommit {
@JsonPOJOBuilder(withPrefix = "set")
public abstract static class Builder {
public abstract Builder setSha(String sha);
+
public abstract Builder setCommit(CommitData commit);
+ public abstract Builder setCommitter(CommitUser committer);
+
+ public abstract Builder setAuthor(CommitUser author);
+
+ public abstract Builder setParents(List<ParentCommit> parents);
+
public abstract GithubCommit build();
}
@AutoValue
@JsonDeserialize(builder = AutoValue_GithubCommit_CommitData.Builder.class)
public abstract static class CommitData {
- public abstract CommitUser getAuthor();
- public abstract CommitUser getCommitter();
+ public abstract BriefCommitUser getAuthor();
+
+ public abstract BriefCommitUser getCommitter();
public static Builder builder() {
return new AutoValue_GithubCommit_CommitData.Builder();
@@ -51,29 +68,69 @@ public abstract class GithubCommit {
@AutoValue.Builder
@JsonPOJOBuilder(withPrefix = "set")
public abstract static class Builder {
- public abstract Builder setAuthor(CommitUser author);
- public abstract Builder setCommitter(CommitUser committer);
+ public abstract Builder setAuthor(BriefCommitUser author);
+
+ public abstract Builder setCommitter(BriefCommitUser committer);
public abstract CommitData build();
}
}
+
@AutoValue
- @JsonDeserialize(builder = AutoValue_GithubCommit_CommitUser.Builder.class)
- public abstract static class CommitUser {
+ @JsonDeserialize(builder = AutoValue_GithubCommit_BriefCommitUser.Builder.class)
+ public abstract static class BriefCommitUser {
public abstract String getName();
+
public abstract String getEmail();
public static Builder builder() {
- return new AutoValue_GithubCommit_CommitUser.Builder();
+ return new AutoValue_GithubCommit_BriefCommitUser.Builder();
}
@AutoValue.Builder
@JsonPOJOBuilder(withPrefix = "set")
public abstract static class Builder {
public abstract Builder setName(String name);
+
public abstract Builder setEmail(String email);
+ public abstract BriefCommitUser build();
+ }
+ }
+
+ @AutoValue
+ @JsonDeserialize(builder = AutoValue_GithubCommit_CommitUser.Builder.class)
+ public abstract static class CommitUser {
+ public abstract String getLogin();
+
+ public static Builder builder() {
+ return new AutoValue_GithubCommit_CommitUser.Builder();
+ }
+
+ @AutoValue.Builder
+ @JsonPOJOBuilder(withPrefix = "set")
+ public abstract static class Builder {
+ public abstract Builder setLogin(String login);
+
public abstract CommitUser build();
}
}
+
+ @AutoValue
+ @JsonDeserialize(builder = AutoValue_GithubCommit_ParentCommit.Builder.class)
+ public abstract static class ParentCommit {
+ public abstract String getSha();
+
+ public static Builder builder() {
+ return new AutoValue_GithubCommit_ParentCommit.Builder();
+ }
+
+ @AutoValue.Builder
+ @JsonPOJOBuilder(withPrefix = "set")
+ public abstract static class Builder {
+ public abstract Builder setSha(String sha);
+
+ public abstract ParentCommit build();
+ }
+ }
}
diff --git a/src/main/java/org/eclipsefoundation/git/eca/model/GitUser.java b/src/main/java/org/eclipsefoundation/git/eca/model/GitUser.java
index 0ba6d5e5a7d39858c212f171c9b22b896a1f9268..03ac8457d78f20d0cc3cb41bae59deaaf822efa2 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/model/GitUser.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/model/GitUser.java
@@ -32,6 +32,9 @@ public abstract class GitUser {
@Nullable
public abstract String getMail();
+ @Nullable
+ public abstract String getExternalId();
+
public static Builder builder() {
return new AutoValue_GitUser.Builder();
}
@@ -43,6 +46,8 @@ public abstract class GitUser {
public abstract Builder setMail(@Nullable String mail);
+ public abstract Builder setExternalId(@Nullable String externalId);
+
public abstract GitUser build();
}
}
\ No newline at end of file
diff --git a/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java b/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java
index e048f31a53332ef30483e3c6d864c18f1a1fcd33..5fc19e286cbe1327e1278844f9df4c8ed075ce68 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java
@@ -30,6 +30,7 @@ import org.eclipsefoundation.core.model.RequestWrapper;
import org.eclipsefoundation.core.service.APIMiddleware;
import org.eclipsefoundation.git.eca.api.GithubAPI;
import org.eclipsefoundation.git.eca.api.models.GithubCommit;
+import org.eclipsefoundation.git.eca.api.models.GithubCommit.ParentCommit;
import org.eclipsefoundation.git.eca.api.models.GithubCommitStatusRequest;
import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest;
import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
@@ -122,12 +123,15 @@ public abstract class GithubAdjacentResource {
.builder()
.setMail(c.getCommit().getAuthor().getEmail())
.setName(c.getCommit().getAuthor().getName())
+ .setExternalId(c.getAuthor().getLogin())
.build())
.setCommitter(GitUser
.builder()
.setMail(c.getCommit().getCommitter().getEmail())
.setName(c.getCommit().getCommitter().getName())
+ .setExternalId(c.getCommitter().getLogin())
.build())
+ .setParents(c.getParents().stream().map(ParentCommit::getSha).collect(Collectors.toList()))
.build())
.collect(Collectors.toList()))
.build();
diff --git a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultValidationService.java b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultValidationService.java
index 9d98fc5050187bf65c45bdc7017b8aeaeedef57b..647252e019e68fa4badc619a08dab7bc97e487d4 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultValidationService.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultValidationService.java
@@ -428,6 +428,20 @@ public class DefaultValidationService implements ValidationService {
* @return the Eclipse Account user information if found, or null if there was an error or no user exists.
*/
private EclipseUser getIdentifiedUser(GitUser user) {
+ // check if the external ID is set, and if so, attempt to look the user up.
+ if (StringUtils.isNotBlank(user.getExternalId())) {
+ // right now this is only supported for Github account lookups, so that will be used
+ EclipseUser actualUser = users.getUserByGithubUsername(user.getExternalId());
+ // if present, return the user. Otherwise, log and continue processing
+ if (actualUser != null) {
+ return actualUser;
+ } else {
+ LOGGER
+ .debug("An external ID of {} was passed, but no matching Eclipse users found. Falling back on default email lookup.",
+ user.getExternalId());
+ }
+ }
+
// don't process an empty email as it will never have a match
if (StringUtils.isBlank(user.getMail())) {
LOGGER.debug("Cannot get identified user if user is empty, returning null");
diff --git a/src/test/java/org/eclipsefoundation/git/eca/resource/ValidationResourceTest.java b/src/test/java/org/eclipsefoundation/git/eca/resource/ValidationResourceTest.java
index 144bec7874df81c929a7924a31f226f0c4878c50..1c2a56a055a9acecaadc3c79491193ac2346fd8c 100644
--- a/src/test/java/org/eclipsefoundation/git/eca/resource/ValidationResourceTest.java
+++ b/src/test/java/org/eclipsefoundation/git/eca/resource/ValidationResourceTest.java
@@ -552,13 +552,33 @@ class ValidationResourceTest {
.testPost(VALIDATE_FORBIDDEN_CASE, createGerritRequest(true, "/gitroot/sample/gerrit.other-project", Arrays.asList(c1)));
}
+ @Test
+ void validate_githubExternalId_success() {
+ GitUser g1 = GitUser.builder().setName("grunter").setMail("other-grunt@test.co").setExternalId("grunter2").build();
+ Commit c1 = createNoBodyCommit(g1, g1);
+
+ RestAssuredTemplates
+ .testPost(VALIDATE_SUCCESS_CASE,
+ createGitHubRequest(false, "http://www.github.com/eclipsefdn/prototype", Arrays.asList(c1)));
+ }
+
+ @Test
+ void validate_githubExternalId_success_fallback() {
+ GitUser g1 = GitUser.builder().setName("grunter").setMail("grunt@important.co").setExternalId("grunter").build();
+ Commit c1 = createNoBodyCommit(g1, g1);
+
+ RestAssuredTemplates
+ .testPost(VALIDATE_SUCCESS_CASE,
+ createGitHubRequest(false, "http://www.github.com/eclipsefdn/prototype", Arrays.asList(c1)));
+ }
+
/*
* NO REPLY TESTS
*/
@Test
void validateGithubNoReply_legacy() {
- GitUser g1 = GitUser.builder().setName("grunter").setMail("grunter@users.noreply.github.com").build();
+ GitUser g1 = GitUser.builder().setName("grunter").setMail("grunter2@users.noreply.github.com").build();
Commit c1 = createNoBodyCommit(g1, g1);
@@ -570,7 +590,7 @@ class ValidationResourceTest {
@Test
void validateGithubNoReply_success() {
// sometimes the user ID and user name are reversed
- GitUser g1 = GitUser.builder().setName("grunter").setMail("123456789+grunter@users.noreply.github.com").build();
+ GitUser g1 = GitUser.builder().setName("grunter").setMail("123456789+grunter2@users.noreply.github.com").build();
Commit c1 = createNoBodyCommit(g1, g1);
@@ -607,7 +627,7 @@ class ValidationResourceTest {
@Test
void validateAllowListAuthor_success() {
- GitUser g1 = GitUser.builder().setName("grunter").setMail("grunter@users.noreply.github.com").build();
+ GitUser g1 = GitUser.builder().setName("grunter").setMail("grunter2@users.noreply.github.com").build();
GitUser g2 = GitUser.builder().setName("grunter").setMail("noreply@github.com").build();
Commit c1 = createNoBodyCommit(g2, g1);
@@ -619,7 +639,7 @@ class ValidationResourceTest {
@Test
void validateAllowListCommitter_success() {
- GitUser g1 = GitUser.builder().setName("grunter").setMail("grunter@users.noreply.github.com").build();
+ GitUser g1 = GitUser.builder().setName("grunter").setMail("grunter2@users.noreply.github.com").build();
GitUser g2 = GitUser.builder().setName("grunter").setMail("noreply@github.com").build();
Commit c1 = createNoBodyCommit(g1, g2);
diff --git a/src/test/java/org/eclipsefoundation/git/eca/service/impl/CachedUserServiceTest.java b/src/test/java/org/eclipsefoundation/git/eca/service/impl/CachedUserServiceTest.java
index cd2da33dc199fa3122ce321025550d53fb39331b..231b401c7987c5d425c752cd7eac239b9e356996 100644
--- a/src/test/java/org/eclipsefoundation/git/eca/service/impl/CachedUserServiceTest.java
+++ b/src/test/java/org/eclipsefoundation/git/eca/service/impl/CachedUserServiceTest.java
@@ -52,7 +52,7 @@ class CachedUserServiceTest {
@Test
void getUser_noReplyGH_success() {
- EclipseUser u = users.getUser("123456789+grunter@users.noreply.github.com");
+ EclipseUser u = users.getUser("123456789+grunter2@users.noreply.github.com");
// assert that this is the user we expect and that it exists
Assertions.assertNotNull(u);
Assertions.assertEquals("grunter", u.getName());
@@ -76,7 +76,7 @@ class CachedUserServiceTest {
@Test
void getUserByGithubUsername_success() {
- EclipseUser u = users.getUserByGithubUsername("grunter");
+ EclipseUser u = users.getUserByGithubUsername("grunter2");
// assert that this is the user we expect and that it exists
Assertions.assertNotNull(u);
Assertions.assertEquals("grunt@important.co", u.getMail());
diff --git a/src/test/java/org/eclipsefoundation/git/eca/test/api/MockAccountsAPI.java b/src/test/java/org/eclipsefoundation/git/eca/test/api/MockAccountsAPI.java
index c6584a1eab86ae33bc001bafeeaca041e92549f1..445add9b61ccbf5821ff1b0198b263e1021a269a 100644
--- a/src/test/java/org/eclipsefoundation/git/eca/test/api/MockAccountsAPI.java
+++ b/src/test/java/org/eclipsefoundation/git/eca/test/api/MockAccountsAPI.java
@@ -26,8 +26,8 @@ import org.eclipsefoundation.git.eca.api.models.EclipseUser.ECA;
import io.quarkus.test.Mock;
/**
- * Simple stub for accounts API. Allows for easy testing of users that don't
- * really exist upstream, and so that we don't need a real auth token for data.
+ * Simple stub for accounts API. Allows for easy testing of users that don't really exist upstream, and so that we don't
+ * need a real auth token for data.
*
* @author Martin Lowe
*
@@ -42,42 +42,68 @@ public class MockAccountsAPI implements AccountsAPI {
public MockAccountsAPI() {
int id = 0;
this.users = new HashMap<>();
- users.put("newbie@important.co", EclipseUser.builder().setIsCommitter(false).setUid(id++)
- .setMail("newbie@important.co").setName("newbieAnon").setECA(ECA.builder().build())
- .build());
- users.put("slom@eclipse-foundation.org",
- EclipseUser.builder().setIsCommitter(false).setUid(id++)
- .setMail("slom@eclipse-foundation.org")
- .setName("barshall_blathers")
- .setECA(ECA.builder().setCanContributeSpecProject(true).setSigned(true)
- .build())
- .build());
- users.put("tester@eclipse-foundation.org",
- EclipseUser.builder().setIsCommitter(false).setUid(id++)
- .setMail("tester@eclipse-foundation.org")
- .setName("mctesterson")
- .setECA(ECA.builder().setCanContributeSpecProject(false).setSigned(true)
- .build())
- .build());
- users.put("code.wiz@important.co",
- EclipseUser.builder().setIsCommitter(true).setUid(id++).setMail("code.wiz@important.co")
- .setName("da_wizz")
- .setECA(ECA.builder().setCanContributeSpecProject(true).setSigned(true)
- .build())
- .build());
- users.put("grunt@important.co",
- EclipseUser.builder().setIsCommitter(true).setUid(id++).setMail("grunt@important.co")
- .setName("grunter")
- .setECA(ECA.builder().setCanContributeSpecProject(false).setSigned(true)
- .build())
- .build());
- users.put("paper.pusher@important.co",
- EclipseUser.builder().setIsCommitter(false).setUid(id++)
- .setMail("paper.pusher@important.co")
- .setName("sumAnalyst")
- .setECA(ECA.builder().setCanContributeSpecProject(false).setSigned(true)
- .build())
- .build());
+ users
+ .put("newbie@important.co",
+ EclipseUser
+ .builder()
+ .setIsCommitter(false)
+ .setUid(id++)
+ .setMail("newbie@important.co")
+ .setName("newbieAnon")
+ .setECA(ECA.builder().build())
+ .build());
+ users
+ .put("slom@eclipse-foundation.org",
+ EclipseUser
+ .builder()
+ .setIsCommitter(false)
+ .setUid(id++)
+ .setMail("slom@eclipse-foundation.org")
+ .setName("barshall_blathers")
+ .setECA(ECA.builder().setCanContributeSpecProject(true).setSigned(true).build())
+ .build());
+ users
+ .put("tester@eclipse-foundation.org",
+ EclipseUser
+ .builder()
+ .setIsCommitter(false)
+ .setUid(id++)
+ .setMail("tester@eclipse-foundation.org")
+ .setName("mctesterson")
+ .setECA(ECA.builder().setCanContributeSpecProject(false).setSigned(true).build())
+ .build());
+ users
+ .put("code.wiz@important.co",
+ EclipseUser
+ .builder()
+ .setIsCommitter(true)
+ .setUid(id++)
+ .setMail("code.wiz@important.co")
+ .setName("da_wizz")
+ .setECA(ECA.builder().setCanContributeSpecProject(true).setSigned(true).build())
+ .setGithubHandle("wiz_in_da_hub")
+ .build());
+ users
+ .put("grunt@important.co",
+ EclipseUser
+ .builder()
+ .setIsCommitter(true)
+ .setUid(id++)
+ .setMail("grunt@important.co")
+ .setName("grunter")
+ .setECA(ECA.builder().setCanContributeSpecProject(false).setSigned(true).build())
+ .setGithubHandle("grunter2")
+ .build());
+ users
+ .put("paper.pusher@important.co",
+ EclipseUser
+ .builder()
+ .setIsCommitter(false)
+ .setUid(id++)
+ .setMail("paper.pusher@important.co")
+ .setName("sumAnalyst")
+ .setECA(ECA.builder().setCanContributeSpecProject(false).setSigned(true).build())
+ .build());
}
@@ -89,7 +115,12 @@ public class MockAccountsAPI implements AccountsAPI {
@Override
public EclipseUser getUserByGithubUname(String token, String uname) {
// assumes github id is same as uname for purposes of lookup (simplifies fetch logic)
- return users.values().stream().filter(u -> u.getName().equals(uname)).findFirst().orElseGet(() -> null);
+ return users
+ .values()
+ .stream()
+ .filter(u -> u.getGithubHandle() != null && u.getGithubHandle().equals(uname))
+ .findFirst()
+ .orElseGet(() -> null);
}
}