diff --git a/src/main/java/org/eclipsefoundation/git/eca/service/impl/CachedUserService.java b/src/main/java/org/eclipsefoundation/git/eca/service/impl/CachedUserService.java
index 54e841859933d0621809aa88d4f2b3d9e5790266..45ad50178491186e32490dd7c31a49ed7ce24f21 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/service/impl/CachedUserService.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/service/impl/CachedUserService.java
@@ -81,8 +81,7 @@ public class CachedUserService implements UserService {
if (StringUtils.isBlank(mail)) {
return null;
}
- Optional<EclipseUser> u = cache.get(mail, new MultivaluedMapImpl<>(), EclipseUser.class,
- () -> retrieveUser(mail));
+ Optional<EclipseUser> u = cache.get(mail, new MultivaluedMapImpl<>(), EclipseUser.class, () -> retrieveUser(mail));
if (u.isPresent()) {
LOGGER.debug("Found user with email {}", mail);
return u.get();
@@ -96,8 +95,9 @@ public class CachedUserService implements UserService {
if (StringUtils.isBlank(username)) {
return null;
}
- Optional<EclipseUser> u = cache.get("gh:" + username, new MultivaluedMapImpl<>(), EclipseUser.class,
- () -> accounts.getUserByGithubUname(getBearerToken(), username));
+ Optional<EclipseUser> u = cache
+ .get("gh:" + username, new MultivaluedMapImpl<>(), EclipseUser.class,
+ () -> accounts.getUserByGithubUname(getBearerToken(), username));
if (u.isPresent()) {
LOGGER.debug("Found user with name {}", username);
return u.get();
@@ -111,30 +111,39 @@ public class CachedUserService implements UserService {
if (StringUtils.isBlank(mail)) {
return false;
}
+ // set the values used to validate whether user is a bot
+ final String checkedField;
+ final String checkedValue;
+ // check if email is a noreply address
+ if (patterns.stream().anyMatch(pattern -> pattern.matcher(mail.trim()).find())) {
+ checkedValue = extractNoReplyUserFromAddress(mail);
+ checkedField = "username";
+ LOGGER.trace("Checking {} for bot status using noreply logic", checkedValue);
+ } else {
+ checkedValue = mail;
+ checkedField = "email";
+ LOGGER.trace("Checking {} for bot status using standard logic", checkedValue);
+ }
+
+ // get the bots to be compared against
List<JsonNode> botObjs = getBots();
// if there are no matching projects, then check against all bots, not just
// project bots
if (filteredProjects == null || filteredProjects.isEmpty()) {
return botObjs.stream().anyMatch(bot -> checkFieldsForMatchingMail(bot, mail));
}
- // for each of the matched projects, check the bot for the matching project ID
- for (Project p : filteredProjects) {
+ // check if any of the projects match any of the bots
+ return filteredProjects.stream().anyMatch(p -> botObjs.stream().anyMatch(bot -> {
LOGGER.debug("Checking project {} for matching bots", p.getProjectId());
- for (JsonNode bot : botObjs) {
- // if the project ID match, and one of the email fields matches, then user is
- // bot
- if (p.getProjectId().equalsIgnoreCase(bot.get("projectId").asText())
- && checkFieldsForMatchingMail(bot, mail)) {
- return true;
- }
- }
- }
- return false;
+ return p.getProjectId().equalsIgnoreCase(bot.get("projectId").asText())
+ && checkFieldsForMatchingValue(bot, checkedValue, checkedField);
+ }));
+
}
/**
- * Checks for standard and noreply email address matches for a Git user and
- * converts to a Eclipse Foundation account object.
+ * Checks for standard and noreply email address matches for a Git user and converts to a Eclipse Foundation account
+ * object.
*
* @param user the user to attempt account retrieval for.
* @return the user account if found by mail, or null if none found.
@@ -164,13 +173,11 @@ public class CachedUserService implements UserService {
}
/**
- * Checks git user for no-reply address, and attempts to ratify user through
- * reverse lookup in API service. Currently, this service only recognizes Github
- * no-reply addresses as they have a route to be mapped.
+ * Checks git user for no-reply address, and attempts to ratify user through reverse lookup in API service. Currently,
+ * this service only recognizes Github no-reply addresses as they have a route to be mapped.
*
* @param user the Git user account to check for no-reply mail address
- * @return the Eclipse user if email address is detected no reply and one can be
- * mapped, otherwise null
+ * @return the Eclipse user if email address is detected no reply and one can be mapped, otherwise null
*/
private EclipseUser checkForNoReplyUser(String mail) {
if (StringUtils.isBlank(mail)) {
@@ -180,20 +187,9 @@ public class CachedUserService implements UserService {
LOGGER.debug("Checking user with mail {} for no-reply", mail);
boolean isNoReply = patterns.stream().anyMatch(pattern -> pattern.matcher(mail.trim()).find());
if (isNoReply) {
- // get the username/ID string before the first @ symbol.
- String noReplyUser = mail.substring(0, mail.indexOf("@", 0));
- // split based on +, if more than one part, use second (contains user),
- // otherwise, use whole string
- String[] nameParts = noReplyUser.split("[\\+]");
- String namePart;
- if (nameParts.length > 1 && nameParts[1] != null) {
- namePart = nameParts[1];
- } else {
- namePart = nameParts[0];
- }
- String uname = namePart.trim();
- LOGGER.debug("User with mail {} detected as noreply account, checking services for username match on '{}'",
- mail, uname);
+ // get the user from the noreply address
+ String uname = extractNoReplyUserFromAddress(mail);
+ LOGGER.debug("User with mail {} detected as noreply account, checking services for username match on '{}'", mail, uname);
// check github for no-reply (only allowed noreply currently)
if (mail.endsWith("noreply.github.com")) {
@@ -209,18 +205,21 @@ public class CachedUserService implements UserService {
}
/**
- * Checks JSON node to look for email fields, both at the root, and nested email
- * fields.
+ * Checks JSON node to look for email fields, both at the root, and nested email fields.
*
- * @param bot the bots JSON object representation
+ * @param bot the bots JSON object representation
* @param mail the email to match against
* @return true if the bot has a matching email value, otherwise false
*/
private boolean checkFieldsForMatchingMail(JsonNode bot, String mail) {
+ return checkFieldsForMatchingValue(bot, mail, "email");
+ }
+
+ private boolean checkFieldsForMatchingValue(JsonNode bot, String targetValue, String fieldName) {
// check the root email for the bot for match
- JsonNode botmail = bot.get("email");
- if (mail != null && botmail != null && mail.equalsIgnoreCase(botmail.asText(""))) {
- LOGGER.debug("Found matching bot at root level for '{}'", mail);
+ JsonNode botField = bot.get(fieldName);
+ if (targetValue != null && botField != null && targetValue.equalsIgnoreCase(botField.asText(""))) {
+ LOGGER.debug("Found matching bot at root level for '{}'", targetValue);
return true;
}
Iterator<Entry<String, JsonNode>> i = bot.fields();
@@ -229,11 +228,11 @@ public class CachedUserService implements UserService {
// check that our field is an object with fields
JsonNode node = e.getValue();
if (node.isObject()) {
- LOGGER.debug("Checking {} for bot email", e.getKey());
+ LOGGER.debug("Checking {} for bot value in field {}", e.getKey(), fieldName);
// if the mail matches (ignoring case) user is bot
- JsonNode botAliasMail = node.get("email");
- if (mail != null && botAliasMail != null && mail.equalsIgnoreCase(botAliasMail.asText(""))) {
- LOGGER.debug("Found match for bot email {}", mail);
+ JsonNode botAliasField = node.get(fieldName);
+ if (targetValue != null && botAliasField != null && targetValue.equalsIgnoreCase(botAliasField.asText(""))) {
+ LOGGER.debug("Found match for bot value {}", targetValue);
return true;
}
}
@@ -241,9 +240,23 @@ public class CachedUserService implements UserService {
return false;
}
+ private String extractNoReplyUserFromAddress(String mail) {
+ // get the username/ID string before the first @ symbol.
+ String noReplyUser = mail.substring(0, mail.indexOf("@", 0));
+ // split based on +, if more than one part, use second (contains user),
+ // otherwise, use whole string
+ String[] nameParts = noReplyUser.split("[\\+]");
+ String namePart;
+ if (nameParts.length > 1 && nameParts[1] != null) {
+ namePart = nameParts[1];
+ } else {
+ namePart = nameParts[0];
+ }
+ return namePart.trim();
+ }
+
private List<JsonNode> getBots() {
- Optional<List<JsonNode>> allBots = cache.get("allBots", new MultivaluedMapImpl<>(), JsonNode.class,
- () -> bots.getBots());
+ Optional<List<JsonNode>> allBots = cache.get("allBots", new MultivaluedMapImpl<>(), JsonNode.class, () -> bots.getBots());
if (!allBots.isPresent()) {
return Collections.emptyList();
}
diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties
index 41425558d831a452d5b74a06245f4e10f8132292..c2a762554ee0fc27d80703f7c9e1d4a847d526aa 100644
--- a/src/main/resources/application.properties
+++ b/src/main/resources/application.properties
@@ -4,7 +4,7 @@ org.eclipsefoundation.git.eca.api.ProjectsAPI/mp-rest/url=https://projects.eclip
org.eclipsefoundation.git.eca.api.BotsAPI/mp-rest/url=https://api.eclipse.org
quarkus.rest-client."org.eclipsefoundation.git.eca.api.GitlabAPI".url=https://gitlab.eclipse.org/api/v4/
-eclipse.noreply.email-patterns=@users.noreply.github.com\$
+eclipse.noreply.email-patterns=@users.noreply.github.com\$,@noreply.github.com\$
## Base HTTP settings
quarkus.http.enable-compression=true
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 434fcec2bd044dff0a3de619f2649df1b2628451..87af6dec504161f92ef113dda57b2136ee99db81 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
@@ -28,8 +28,8 @@ import org.junit.jupiter.api.Test;
import io.quarkus.test.junit.QuarkusTest;
/**
- * Tests user service impl using test stub data available from API stubs. While
- * not a perfect test as there is no auth, it is good enough for unit testing.
+ * Tests user service impl using test stub data available from API stubs. While not a perfect test as there is no auth,
+ * it is good enough for unit testing.
*
* @author Martin Lowe
*
@@ -98,6 +98,11 @@ public class CachedUserServiceTest {
Assertions.assertTrue(users.userIsABot("1.bot@eclipse.org", getTestProject()));
}
+ @Test
+ void isUserABot_success_noreply() {
+ Assertions.assertTrue(users.userIsABot("123456789+projbot@noreply.github.com", getTestProject()));
+ }
+
@Test
void isUserABot_nullOrEmptyAddress() {
Assertions.assertFalse(users.userIsABot(null, getTestProject()));
@@ -132,8 +137,7 @@ public class CachedUserServiceTest {
* @return the sample.proj project in a list for use in tests.
*/
private List<Project> getTestProject() {
- Optional<Project> proj = projects.getProjects().stream().filter(p -> p.getProjectId().equals("sample.proj"))
- .findFirst();
+ Optional<Project> proj = projects.getProjects().stream().filter(p -> p.getProjectId().equals("sample.proj")).findFirst();
if (proj.isEmpty()) {
Assertions.fail("Could not find one of the needed test projects for test, bad state.");
}
diff --git a/src/test/resources/application.properties b/src/test/resources/application.properties
index 98a247c88062082d6fb2bb77282ff0a327b0029c..4d28ba44717ead6563452e792698d6fb9dd5faa8 100644
--- a/src/test/resources/application.properties
+++ b/src/test/resources/application.properties
@@ -2,7 +2,7 @@
org.eclipsefoundation.git.eca.api.ProjectsAPI/mp-rest/url=https://projects.eclipse.org
org.eclipsefoundation.git.eca.api.BotsAPI/mp-rest/url=https://api.eclipse.org
-eclipse.noreply.email-patterns=@users.noreply.github.com\$
+eclipse.noreply.email-patterns=@users.noreply.github.com\$,@noreply.github.com\$
eclipse.mail.allowlist=noreply@github.com
eclipse.reports.access-key=samplekey
eclipse.gitlab.access-token=token_val