From f566be04ff439dcab85ae0db03bd2bc4d1bba3e3 Mon Sep 17 00:00:00 2001
From: Martin Lowe <martin.lowe@eclipse-foundation.org>
Date: Thu, 24 Aug 2023 11:34:15 -0400
Subject: [PATCH] Iss #137 - Remove magic numbers, split out large lambdas for
code smell
Resolves #137
---
.../config/EclipseQuteTemplateExtensions.java | 27 ++--
.../git/eca/dto/PrivateProjectEvent.java | 10 +-
.../git/eca/helper/ProjectHelper.java | 101 ++++++------
.../git/eca/resource/ReportsResource.java | 8 +-
.../git/eca/resource/ValidationResource.java | 7 +-
.../eca/service/impl/CachedUserService.java | 2 +-
.../impl/DefaultGithubApplicationService.java | 13 +-
.../impl/DefaultSystemHookService.java | 69 ++++-----
.../impl/DefaultValidationService.java | 146 ++++++++++--------
.../eca/tasks/GithubRevalidationQueue.java | 5 +-
.../ScheduledPrivateProjectScanTask.java | 71 ++++-----
11 files changed, 248 insertions(+), 211 deletions(-)
diff --git a/src/main/java/org/eclipsefoundation/git/eca/config/EclipseQuteTemplateExtensions.java b/src/main/java/org/eclipsefoundation/git/eca/config/EclipseQuteTemplateExtensions.java
index b5b7bdac..c37a561a 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/config/EclipseQuteTemplateExtensions.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/config/EclipseQuteTemplateExtensions.java
@@ -17,12 +17,19 @@ import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.eclipsefoundation.git.eca.dto.CommitValidationMessage;
import org.eclipsefoundation.git.eca.dto.CommitValidationStatus;
+import org.eclipsefoundation.git.eca.namespace.APIStatusCode;
import io.quarkus.qute.TemplateExtension;
@TemplateExtension
public class EclipseQuteTemplateExtensions {
+ private static final int EMAIL_ADDRESS_PARTS = 2;
+ private static final int DOMAIN_PARTS_MINIMUM = 2;
+
+ // division point where a character gets obfuscated in the email domain
+ private static final int OBFUSCATION_DIVISION_POINT = 2;
+
/**
* Made to count nested errors in a list of commit status objects.
*
@@ -64,18 +71,18 @@ public class EclipseQuteTemplateExtensions {
}
// split on the @ symbol
String[] emailParts = email.split("\\@");
- if (emailParts.length != 2) {
+ if (emailParts.length != EMAIL_ADDRESS_PARTS) {
// valid emails will have 2 sections when split this way
return "";
}
String[] domainParts = emailParts[1].split("\\.");
- if (domainParts.length < 2) {
+ if (domainParts.length < DOMAIN_PARTS_MINIMUM) {
// emails should have at least 2 parts here
return "";
}
// the index in the middle of the first domain part of the email address
- int middleIndexDomain = Math.floorDiv(domainParts[0].length(), 2);
+ int middleIndexDomain = Math.floorDiv(domainParts[0].length(), OBFUSCATION_DIVISION_POINT);
// build the obfuscated email address in the pattern defined in the block comment
StringBuilder sb = new StringBuilder();
@@ -99,25 +106,25 @@ public class EclipseQuteTemplateExtensions {
*/
static String getMessageForError(CommitValidationMessage message) {
String out = "";
- if (message.getStatusCode() == -406) {
+ if (message.getStatusCode() == APIStatusCode.ERROR_PROXY_PUSH.getValue()) {
out = String
.format("Committer does not have permission to push on behalf of another user (Legacy). (%s)",
EclipseQuteTemplateExtensions.obfuscateEmail(message.getCommitterEmail()));
- } else if (message.getStatusCode() == -405) {
+ } else if (message.getStatusCode() == APIStatusCode.ERROR_COMMITTER.getValue()) {
out = String
.format("Committer did not have a signed ECA on file. (%s)",
EclipseQuteTemplateExtensions.obfuscateEmail(message.getCommitterEmail()));
- } else if (message.getStatusCode() == -404) {
+ } else if (message.getStatusCode() == APIStatusCode.ERROR_AUTHOR.getValue()) {
out = String
.format("Author did not have a signed ECA on file. (%s)",
EclipseQuteTemplateExtensions.obfuscateEmail(message.getAuthorEmail()));
- } else if (message.getStatusCode() == -403) {
+ } else if (message.getStatusCode() == APIStatusCode.ERROR_SPEC_PROJECT.getValue()) {
out = String
.format("Committer does not have permission to commit on specification projects. (%s)",
EclipseQuteTemplateExtensions.obfuscateEmail(message.getCommitterEmail()));
- } else if (message.getStatusCode() == -402) {
+ } else if (message.getStatusCode() == APIStatusCode.ERROR_SIGN_OFF.getValue()) {
out = "Sign-off not detected in the commit message (Legacy).";
- } else if (message.getStatusCode() == -401) {
+ } else if (message.getStatusCode() == APIStatusCode.ERROR_DEFAULT.getValue()) {
out = "Request format/state error detected.";
} else {
out = "Unaccounted for error detected, please contact administrators.";
@@ -126,6 +133,6 @@ public class EclipseQuteTemplateExtensions {
}
private EclipseQuteTemplateExtensions() {
-
+
}
}
diff --git a/src/main/java/org/eclipsefoundation/git/eca/dto/PrivateProjectEvent.java b/src/main/java/org/eclipsefoundation/git/eca/dto/PrivateProjectEvent.java
index b8f0a620..69284757 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/dto/PrivateProjectEvent.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/dto/PrivateProjectEvent.java
@@ -17,6 +17,7 @@ import java.time.LocalDateTime;
import java.time.LocalTime;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import javax.inject.Inject;
@@ -187,6 +188,9 @@ public class PrivateProjectEvent extends BareNode {
@Singleton
public static class PrivateProjectEventFilter implements DtoFilter<PrivateProjectEvent> {
+ private static final int LAST_HOUR_IN_DAY = (int) TimeUnit.HOURS.convert(1, TimeUnit.DAYS) - 1;
+ private static final int LAST_MINUTE_IN_HOUR = (int) TimeUnit.MINUTES.convert(1, TimeUnit.HOURS) - 1;
+
@Inject
ParameterizedSQLStatementBuilder builder;
@@ -256,15 +260,15 @@ public class PrivateProjectEvent extends BareNode {
statement
.addClause(new ParameterizedSQLStatement.Clause(TABLE.getAlias() + ".creationDate BETWEEN ? AND ?",
new Object[] { LocalDateTime.of(LocalDate.parse(since), LocalTime.of(0, 0)),
- LocalDateTime.of(LocalDate.parse(until), LocalTime.of(23, 59)) }));
+ LocalDateTime.of(LocalDate.parse(until), LocalTime.of(LAST_HOUR_IN_DAY, LAST_MINUTE_IN_HOUR)) }));
} else if (StringUtils.isNotBlank(since)) {
statement
.addClause(new ParameterizedSQLStatement.Clause(TABLE.getAlias() + ".creationDate > ?",
new Object[] { LocalDateTime.of(LocalDate.parse(since), LocalTime.of(0, 0)) }));
} else if (StringUtils.isNotBlank(until)) {
statement
- .addClause(new ParameterizedSQLStatement.Clause(TABLE.getAlias() + ".creationDate < ?",
- new Object[] { LocalDateTime.of(LocalDate.parse(until), LocalTime.of(23, 59)) }));
+ .addClause(new ParameterizedSQLStatement.Clause(TABLE.getAlias() + ".creationDate < ?", new Object[] {
+ LocalDateTime.of(LocalDate.parse(until), LocalTime.of(LAST_HOUR_IN_DAY, LAST_MINUTE_IN_HOUR)) }));
}
return statement;
diff --git a/src/main/java/org/eclipsefoundation/git/eca/helper/ProjectHelper.java b/src/main/java/org/eclipsefoundation/git/eca/helper/ProjectHelper.java
index c0e343fb..a44e4317 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/helper/ProjectHelper.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/helper/ProjectHelper.java
@@ -111,52 +111,59 @@ public final class ProjectHelper {
}
private List<Project> adaptInterestGroups(List<InterestGroup> igs) {
- return igs
- .stream()
- .map(ig -> Project
- .builder()
- .setProjectId(ig.getProjectId())
- .setGerritRepos(Collections.emptyList())
- .setGithubRepos(Collections.emptyList())
- .setGitlab(ig.getGitlab())
- .setCommitters(ig
- .getParticipants()
- .stream()
- .map(p -> ProjectParticipant
- .builder()
- .setFullName(p.getFullName())
- .setUrl(p.getUrl())
- .setUsername(p.getUsername())
- .build())
- .collect(Collectors.toList()))
- .setProjectLeads(ig
- .getLeads()
- .stream()
- .map(p -> ProjectParticipant
- .builder()
- .setFullName(p.getFullName())
- .setUrl(p.getUrl())
- .setUsername(p.getUsername())
- .build())
- .collect(Collectors.toList()))
- .setContributors(Collections.emptyList())
- .setShortProjectId(ig.getShortProjectId())
- .setSlsaLevel("")
- .setSummary("")
- .setWebsiteUrl("")
- .setWebsiteRepo(Collections.emptyList())
- .setGitlab(ig.getGitlab())
- .setGithub(GithubProject.builder().setOrg("").setIgnoredRepos(Collections.emptyList()).build())
- .setWorkingGroups(Collections.emptyList())
- .setIndustryCollaborations(Collections.emptyList())
- .setReleases(Collections.emptyList())
- .setTopLevelProject("")
- .setUrl("")
- .setLogo(ig.getLogo())
- .setTags(Collections.emptyList())
- .setName(ig.getTitle())
- .setSpecProjectWorkingGroup(Collections.emptyMap())
- .build())
- .collect(Collectors.toList());
+ return igs.stream().map(this::convertToProject).collect(Collectors.toList());
+ }
+
+ /**
+ * Convert an interest group into a project, as they are essentially the same structure behind the API.
+ *
+ * @param ig the interest group to convert
+ * @return the project built from the interest group
+ */
+ private Project convertToProject(InterestGroup ig) {
+ return Project
+ .builder()
+ .setProjectId(ig.getProjectId())
+ .setGerritRepos(Collections.emptyList())
+ .setGithubRepos(Collections.emptyList())
+ .setGitlab(ig.getGitlab())
+ .setCommitters(ig
+ .getParticipants()
+ .stream()
+ .map(p -> ProjectParticipant
+ .builder()
+ .setFullName(p.getFullName())
+ .setUrl(p.getUrl())
+ .setUsername(p.getUsername())
+ .build())
+ .collect(Collectors.toList()))
+ .setProjectLeads(ig
+ .getLeads()
+ .stream()
+ .map(p -> ProjectParticipant
+ .builder()
+ .setFullName(p.getFullName())
+ .setUrl(p.getUrl())
+ .setUsername(p.getUsername())
+ .build())
+ .collect(Collectors.toList()))
+ .setContributors(Collections.emptyList())
+ .setShortProjectId(ig.getShortProjectId())
+ .setSlsaLevel("")
+ .setSummary("")
+ .setWebsiteUrl("")
+ .setWebsiteRepo(Collections.emptyList())
+ .setGitlab(ig.getGitlab())
+ .setGithub(GithubProject.builder().setOrg("").setIgnoredRepos(Collections.emptyList()).build())
+ .setWorkingGroups(Collections.emptyList())
+ .setIndustryCollaborations(Collections.emptyList())
+ .setReleases(Collections.emptyList())
+ .setTopLevelProject("")
+ .setUrl("")
+ .setLogo(ig.getLogo())
+ .setTags(Collections.emptyList())
+ .setName(ig.getTitle())
+ .setSpecProjectWorkingGroup(Collections.emptyMap())
+ .build();
}
}
diff --git a/src/main/java/org/eclipsefoundation/git/eca/resource/ReportsResource.java b/src/main/java/org/eclipsefoundation/git/eca/resource/ReportsResource.java
index 3ff3389f..14f72042 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/resource/ReportsResource.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/resource/ReportsResource.java
@@ -21,17 +21,14 @@ import javax.ws.rs.QueryParam;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
+import org.eclipsefoundation.core.exception.UnauthorizedException;
import org.eclipsefoundation.core.model.RequestWrapper;
import org.eclipsefoundation.git.eca.config.EcaReportsConfig;
import org.eclipsefoundation.git.eca.namespace.GitEcaParameterNames;
import org.eclipsefoundation.git.eca.service.ReportsService;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Path("/reports")
public class ReportsResource {
- private static final Logger LOGGER = LoggerFactory.getLogger(ReportsResource.class);
-
@Inject
EcaReportsConfig config;
@@ -45,8 +42,7 @@ public class ReportsResource {
public Response getPrivateProjectEvents(@QueryParam("key") String passedKey, @QueryParam("status") String status,
@QueryParam("since") LocalDate since, @QueryParam("until") LocalDate until) {
if (!config.accessKey().equals(passedKey)) {
- LOGGER.debug("Bad key passed for access, access blocked");
- return Response.status(401).build();
+ throw new UnauthorizedException("Bad key passed for report access, access blocked");
}
if (StringUtils.isNotBlank(status) && !isValidStatus(status)) {
throw new BadRequestException(String.format("Invalid 'status' parameter: %s", status));
diff --git a/src/main/java/org/eclipsefoundation/git/eca/resource/ValidationResource.java b/src/main/java/org/eclipsefoundation/git/eca/resource/ValidationResource.java
index 9d4f0c5f..9af79f78 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/resource/ValidationResource.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/resource/ValidationResource.java
@@ -19,6 +19,7 @@ import java.util.Objects;
import javax.inject.Inject;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
+import javax.ws.rs.NotFoundException;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
@@ -26,6 +27,8 @@ import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
+import org.eclipsefoundation.core.exception.FinalForbiddenException;
+import org.eclipsefoundation.core.helper.LoggingHelper;
import org.eclipsefoundation.core.model.RequestWrapper;
import org.eclipsefoundation.core.service.CachingService;
import org.eclipsefoundation.git.eca.api.models.EclipseUser;
@@ -95,11 +98,11 @@ public class ValidationResource {
public Response getUserStatus(@QueryParam("email") String email) {
EclipseUser user = users.getUser(email);
if (Objects.isNull(user)) {
- return Response.status(404).build();
+ throw new NotFoundException(String.format("No user found with mail '%s'", LoggingHelper.format(email)));
}
if (!user.getECA().getSigned()) {
- return Response.status(403).build();
+ throw new FinalForbiddenException("");
}
return Response.ok().build();
}
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 df38a198..2d847b7e 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
@@ -244,7 +244,7 @@ public class CachedUserService implements UserService {
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[] nameParts = noReplyUser.split("\\+");
String namePart;
if (nameParts.length > 1 && nameParts[1] != null) {
namePart = nameParts[1];
diff --git a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java
index 0875d375..1b374c05 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java
@@ -51,6 +51,10 @@ import com.github.benmanes.caffeine.cache.Caffeine;
public class DefaultGithubApplicationService implements GithubApplicationService {
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultGithubApplicationService.class);
+ // cache kept small as there should only ever really be 1 entry, but room added for safety
+ private static final Integer MAX_CACHE_SIZE = 10;
+ private static final Long INSTALL_REPO_FETCH_MAX_TIMEOUT = 10l;
+
@ConfigProperty(name = "eclipse.github.default-api-version", defaultValue = "2022-11-28")
String apiVersion;
@@ -74,8 +78,8 @@ public class DefaultGithubApplicationService implements GithubApplicationService
this.installationRepositoriesCache = Caffeine
.newBuilder()
.executor(exec)
- .maximumSize(10)
- .refreshAfterWrite(Duration.ofMinutes(60))
+ .maximumSize(MAX_CACHE_SIZE)
+ .refreshAfterWrite(Duration.ofMinutes(TimeUnit.MINUTES.convert(1, TimeUnit.HOURS)))
.buildAsync(k -> loadInstallationRepositories());
// do initial map population
getAllInstallRepos();
@@ -89,13 +93,14 @@ public class DefaultGithubApplicationService implements GithubApplicationService
private MultivaluedMap<String, String> getAllInstallRepos() {
try {
- return this.installationRepositoriesCache.get("all").get(10L, TimeUnit.SECONDS);
+ return this.installationRepositoriesCache.get("all").get(INSTALL_REPO_FETCH_MAX_TIMEOUT, TimeUnit.SECONDS);
} catch (InterruptedException e) {
LOGGER.error("Thread interrupted while building repository cache, no entries will be available for current call");
Thread.currentThread().interrupt();
} catch (Exception e) {
// rewrap exception and throw
- throw new ApplicationException("Thread interrupted while building repository cache, no entries will be available for current call", e);
+ throw new ApplicationException(
+ "Thread interrupted while building repository cache, no entries will be available for current call", e);
}
return new MultivaluedMapImpl<>();
}
diff --git a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultSystemHookService.java b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultSystemHookService.java
index 394f717a..a47a725e 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultSystemHookService.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultSystemHookService.java
@@ -55,6 +55,8 @@ public class DefaultSystemHookService implements SystemHookService {
@ConfigProperty(name = "eclipse.system-hook.pool-size")
Integer poolSize;
+ @ConfigProperty(name = "eclipse.system-hook.delay-in-ms", defaultValue = "300")
+ Integer schedulingDelay;
@Inject
@RestClient
@@ -84,7 +86,7 @@ public class DefaultSystemHookService implements SystemHookService {
@Override
public void processProjectCreateHook(RequestWrapper wrapper, SystemHook hook) {
if (hook != null) {
- ses.schedule(() -> trackPrivateProjectCreation(wrapper, hook), 300, TimeUnit.MILLISECONDS);
+ ses.schedule(() -> trackPrivateProjectCreation(wrapper, hook), schedulingDelay, TimeUnit.MILLISECONDS);
}
}
@@ -98,33 +100,30 @@ public class DefaultSystemHookService implements SystemHookService {
@Override
public void processProjectRenameHook(RequestWrapper wrapper, SystemHook hook) {
if (hook != null) {
- ses.schedule(() -> trackPrivateProjectRenaming(wrapper, hook), 300, TimeUnit.MILLISECONDS);
+ ses.schedule(() -> trackPrivateProjectRenaming(wrapper, hook), schedulingDelay, TimeUnit.MILLISECONDS);
}
}
/**
- * Gathers all relevant data related to the created project and persists the
- * information into a database
+ * Gathers all relevant data related to the created project and persists the information into a database
*
* @param wrapper the request wrapper containing all uri, ip, and query params
- * @param hook the incoming system hook
+ * @param hook the incoming system hook
*/
@Transactional
void trackPrivateProjectCreation(RequestWrapper wrapper, SystemHook hook) {
try {
- LOGGER.debug("Tracking creation of project: [id: {}, path: {}]", hook.getProjectId(),
- hook.getPathWithNamespace());
+ LOGGER.debug("Tracking creation of project: [id: {}, path: {}]", hook.getProjectId(), hook.getPathWithNamespace());
- Optional<GitlabProjectResponse> response = cache.get(Integer.toString(hook.getProjectId()),
- new MultivaluedMapImpl<>(), GitlabProjectResponse.class,
- () -> api.getProjectInfo(apiToken, hook.getProjectId()));
+ Optional<GitlabProjectResponse> response = cache
+ .get(Integer.toString(hook.getProjectId()), new MultivaluedMapImpl<>(), GitlabProjectResponse.class,
+ () -> api.getProjectInfo(apiToken, hook.getProjectId()));
if (response.isPresent()) {
PrivateProjectEvent dto = mapToDto(hook, response.get());
dao.add(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class)), Arrays.asList(dto));
} else {
- LOGGER.error("No info for project: [id: {}, path: {}]", hook.getProjectId(),
- hook.getPathWithNamespace());
+ LOGGER.error("No info for project: [id: {}, path: {}]", hook.getProjectId(), hook.getPathWithNamespace());
}
} catch (Exception e) {
@@ -133,63 +132,54 @@ public class DefaultSystemHookService implements SystemHookService {
}
/**
- * Retrieves the event record for the project that has been deleted. Adds the
- * deletionDate field and updates the DB
+ * Retrieves the event record for the project that has been deleted. Adds the deletionDate field and updates the DB
* record.
*
* @param wrapper the request wrapper containing all uri, ip, and query params
- * @param hook the incoming system hook
+ * @param hook the incoming system hook
*/
@Transactional
void trackPrivateProjectDeletion(RequestWrapper wrapper, SystemHook hook) {
try {
- LOGGER.debug("Tracking deletion of project: [id: {}, path: {}]", hook.getProjectId(),
- hook.getPathWithNamespace());
+ LOGGER.debug("Tracking deletion of project: [id: {}, path: {}]", hook.getProjectId(), hook.getPathWithNamespace());
MultivaluedMap<String, String> params = new MultivaluedMapImpl<>();
params.add(GitEcaParameterNames.PROJECT_ID.getName(), Integer.toString(hook.getProjectId()));
params.add(GitEcaParameterNames.PROJECT_PATH.getName(), hook.getPathWithNamespace());
- List<PrivateProjectEvent> results = dao
- .get(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class), params));
+ List<PrivateProjectEvent> results = dao.get(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class), params));
if (results.isEmpty()) {
- LOGGER.error("No results for project event: [id: {}, path: {}]", hook.getProjectId(),
- hook.getPathWithNamespace());
+ LOGGER.error("No results for project event: [id: {}, path: {}]", hook.getProjectId(), hook.getPathWithNamespace());
} else {
results.get(0).setDeletionDate(hook.getUpdatedAt().toLocalDateTime());
dao.add(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class)), results);
}
} catch (Exception e) {
- LOGGER.error(String.format("Error updating project: [id: %s, path: %s]", hook.getProjectId(),
- hook.getPathWithNamespace()), e);
+ LOGGER.error(String.format("Error updating project: [id: %s, path: %s]", hook.getProjectId(), hook.getPathWithNamespace()), e);
}
}
/**
- * Retrieves the event record for the project that has been renamed. Updates the
- * projectPath field and creates a new DB
+ * Retrieves the event record for the project that has been renamed. Updates the projectPath field and creates a new DB
* record.
*
* @param wrapper the request wrapper containing all uri, ip, and query params
- * @param hook the incoming system hook
+ * @param hook the incoming system hook
*/
@Transactional
void trackPrivateProjectRenaming(RequestWrapper wrapper, SystemHook hook) {
try {
- LOGGER.debug("Tracking renaming of project: [id: {}, path: {}]", hook.getProjectId(),
- hook.getOldPathWithNamespace());
+ LOGGER.debug("Tracking renaming of project: [id: {}, path: {}]", hook.getProjectId(), hook.getOldPathWithNamespace());
MultivaluedMap<String, String> params = new MultivaluedMapImpl<>();
params.add(GitEcaParameterNames.PROJECT_ID.getName(), Integer.toString(hook.getProjectId()));
params.add(GitEcaParameterNames.PROJECT_PATH.getName(), hook.getOldPathWithNamespace());
- List<PrivateProjectEvent> results = dao
- .get(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class), params));
+ List<PrivateProjectEvent> results = dao.get(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class), params));
if (results.isEmpty()) {
- LOGGER.error("No results for project event: [id: {}, path: {}]", hook.getProjectId(),
- hook.getOldPathWithNamespace());
+ LOGGER.error("No results for project event: [id: {}, path: {}]", hook.getProjectId(), hook.getOldPathWithNamespace());
} else {
// Create a new event and track in the DB
PrivateProjectEvent event = new PrivateProjectEvent(results.get(0).getCompositeId().getUserId(),
@@ -202,16 +192,16 @@ public class DefaultSystemHookService implements SystemHookService {
dao.add(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class)), Arrays.asList(event));
}
} catch (Exception e) {
- LOGGER.error(String.format("Error updating project: [id: %s, path: %s]", hook.getProjectId(),
- hook.getOldPathWithNamespace()), e);
+ LOGGER
+ .error(String.format("Error updating project: [id: %s, path: %s]", hook.getProjectId(), hook.getOldPathWithNamespace()),
+ e);
}
}
/**
- * Takes the received hook and Gitlab api response body and maps the values to a
- * PrivateProjectEvent dto.
+ * Takes the received hook and Gitlab api response body and maps the values to a PrivateProjectEvent dto.
*
- * @param hookthe received system hook
+ * @param hookthe received system hook
* @param projectInfo The gitlab project data
* @return A PrivateProjectEvent object
*/
@@ -237,8 +227,9 @@ public class DefaultSystemHookService implements SystemHookService {
* @return the username or null
*/
private String fetchUserName(Integer userId) {
- Optional<GitlabUserResponse> response = cache.get(Integer.toString(userId), new MultivaluedMapImpl<>(),
- GitlabUserResponse.class, () -> api.getUserInfo(apiToken, userId));
+ Optional<GitlabUserResponse> response = cache
+ .get(Integer.toString(userId), new MultivaluedMapImpl<>(), GitlabUserResponse.class,
+ () -> api.getUserInfo(apiToken, userId));
return response.isPresent() ? response.get().getUsername() : null;
}
}
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 5491d3b3..6e548ae0 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
@@ -14,6 +14,7 @@ package org.eclipsefoundation.git.eca.service.impl;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Map.Entry;
import java.util.Optional;
import java.util.stream.Collectors;
@@ -22,6 +23,7 @@ import javax.inject.Inject;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.Status;
import org.apache.commons.lang3.StringUtils;
import org.eclipsefoundation.core.helper.DateTimeHelper;
@@ -36,6 +38,7 @@ import org.eclipsefoundation.git.eca.dto.CommitValidationStatusGrouping;
import org.eclipsefoundation.git.eca.helper.CommitHelper;
import org.eclipsefoundation.git.eca.helper.ProjectHelper;
import org.eclipsefoundation.git.eca.model.Commit;
+import org.eclipsefoundation.git.eca.model.CommitStatus;
import org.eclipsefoundation.git.eca.model.GitUser;
import org.eclipsefoundation.git.eca.model.ValidationRequest;
import org.eclipsefoundation.git.eca.model.ValidationResponse;
@@ -61,7 +64,7 @@ import org.slf4j.LoggerFactory;
public class DefaultValidationService implements ValidationService {
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultValidationService.class);
- @Inject
+ @Inject
MailValidationConfig config;
@Inject
@@ -159,66 +162,13 @@ public class DefaultValidationService implements ValidationService {
public void updateCommitValidationStatus(RequestWrapper wrapper, ValidationResponse r, ValidationRequest req,
List<CommitValidationStatus> statuses, Project p) {
// iterate over commit responses, and update statuses in DB
- List<CommitValidationStatus> updatedStatuses = new ArrayList<>();
- r.getCommits().entrySet().stream().filter(e -> !ValidationResponse.NIL_HASH_PLACEHOLDER.equalsIgnoreCase(e.getKey())).forEach(e -> {
- // get the commit for current status
- Optional<Commit> commit = req.getCommits().stream().filter(c -> e.getKey().equals(c.getHash())).findFirst();
- if (commit.isEmpty()) {
- // this should always have a match (response commits are built from request commits)
- LOGGER.error("Could not find request commit associated with commit messages for commit hash '{}'", e.getKey());
- return;
- }
- Commit c = commit.get();
-
- // update the status if present, otherwise make new one.
- Optional<CommitValidationStatus> status = statuses.stream().filter(s -> e.getKey().equals(s.getCommitHash())).findFirst();
- CommitValidationStatus base;
- if (status.isPresent()) {
- base = status.get();
- } else {
- base = new CommitValidationStatus();
- base.setProject(CommitHelper.getProjectId(p));
- base.setCommitHash(e.getKey());
- base.setUserMail(c.getAuthor().getMail());
- base.setProvider(req.getProvider());
- base.setRepoUrl(req.getRepoUrl().toString());
- base.setCreationDate(DateTimeHelper.now());
- base.setEstimatedLoc(req.getEstimatedLoc());
- }
- base.setLastModified(DateTimeHelper.now());
- updatedStatuses.add(base);
-
- // if there are errors, update validation messages
- if (!e.getValue().getErrors().isEmpty() || (base.getErrors() != null && !base.getErrors().isEmpty())) {
- // generate new errors, looking for errors not found in current list
- List<CommitValidationMessage> currentErrors = base.getErrors() != null ? base.getErrors() : new ArrayList<>();
- List<CommitValidationMessage> newErrors = e
- .getValue()
- .getErrors()
- .stream()
- .filter(err -> currentErrors.stream().noneMatch(ce -> ce.getStatusCode() == err.getCode().getValue()))
- .map(err -> {
- CommitValidationMessage m = new CommitValidationMessage();
- m.setAuthorEmail(c.getAuthor().getMail());
- m.setCommitterEmail(c.getCommitter().getMail());
- m.setStatusCode(err.getCode().getValue());
- // TODO add a checked way to set this
- m.setEclipseId(null);
- m.setProviderId(null);
- m.setCommit(base);
- return m;
- })
- .collect(Collectors.toList());
- LOGGER.debug("Encountered {} new errors for commit with hash '{}'", newErrors.size(), e.getKey());
- currentErrors.addAll(newErrors);
- // remove errors that weren't encountered on this run
- currentErrors
- .removeIf(err -> e.getValue().getErrors().isEmpty()
- || e.getValue().getErrors().stream().noneMatch(msg -> msg.getCode().getValue() == err.getStatusCode()));
- LOGGER.trace("Encountered {} errors: {}", currentErrors.size(), currentErrors);
- base.setErrors(currentErrors);
- }
- });
+ List<CommitValidationStatus> updatedStatuses = r
+ .getCommits()
+ .entrySet()
+ .stream()
+ .filter(e -> !ValidationResponse.NIL_HASH_PLACEHOLDER.equalsIgnoreCase(e.getKey()))
+ .map(e -> recordUpdatedValidationStatus(req, statuses, p, e))
+ .collect(Collectors.toList());
String fingerprint = generateRequestHash(req);
// update the base commit status and messages
dao.add(new RDBMSQuery<>(wrapper, filters.get(CommitValidationStatus.class)), updatedStatuses);
@@ -227,6 +177,76 @@ public class DefaultValidationService implements ValidationService {
updatedStatuses.stream().map(s -> new CommitValidationStatusGrouping(fingerprint, s)).collect(Collectors.toList()));
}
+ /**
+ * Records new or updated validation status for passed commit entry. Checks existing records and updates where
+ * appropriate, otherwise creating new entires.
+ *
+ * @param req the complete validation request
+ * @param existingStatuses the statuses that may exist for the current request
+ * @param p the Eclipse project for the current request if it exists.
+ * @param e the current commit that is being updated
+ * @return the new or updated commit status
+ */
+ private CommitValidationStatus recordUpdatedValidationStatus(ValidationRequest req, List<CommitValidationStatus> existingStatuses,
+ Project p, Entry<String, CommitStatus> e) {
+ // get the commit for current status
+ Optional<Commit> commit = req.getCommits().stream().filter(c -> e.getKey().equals(c.getHash())).findFirst();
+ if (commit.isEmpty()) {
+ // this should always have a match (response commits are built from request commits)
+ LOGGER.error("Could not find request commit associated with commit messages for commit hash '{}'", e.getKey());
+ return null;
+ }
+ Commit c = commit.get();
+ // update the status if present, otherwise make new one.
+ Optional<CommitValidationStatus> status = existingStatuses.stream().filter(s -> e.getKey().equals(s.getCommitHash())).findFirst();
+ CommitValidationStatus base;
+ if (status.isPresent()) {
+ base = status.get();
+ } else {
+ base = new CommitValidationStatus();
+ base.setProject(CommitHelper.getProjectId(p));
+ base.setCommitHash(e.getKey());
+ base.setUserMail(c.getAuthor().getMail());
+ base.setProvider(req.getProvider());
+ base.setRepoUrl(req.getRepoUrl().toString());
+ base.setCreationDate(DateTimeHelper.now());
+ base.setEstimatedLoc(req.getEstimatedLoc());
+ }
+ base.setLastModified(DateTimeHelper.now());
+
+ // if there are errors, update validation messages
+ if (!e.getValue().getErrors().isEmpty() || (base.getErrors() != null && !base.getErrors().isEmpty())) {
+ // generate new errors, looking for errors not found in current list
+ List<CommitValidationMessage> currentErrors = base.getErrors() != null ? base.getErrors() : new ArrayList<>();
+ List<CommitValidationMessage> newErrors = e
+ .getValue()
+ .getErrors()
+ .stream()
+ .filter(err -> currentErrors.stream().noneMatch(ce -> ce.getStatusCode() == err.getCode().getValue()))
+ .map(err -> {
+ CommitValidationMessage m = new CommitValidationMessage();
+ m.setAuthorEmail(c.getAuthor().getMail());
+ m.setCommitterEmail(c.getCommitter().getMail());
+ m.setStatusCode(err.getCode().getValue());
+ // TODO add a checked way to set this
+ m.setEclipseId(null);
+ m.setProviderId(null);
+ m.setCommit(base);
+ return m;
+ })
+ .collect(Collectors.toList());
+ LOGGER.debug("Encountered {} new errors for commit with hash '{}'", newErrors.size(), e.getKey());
+ currentErrors.addAll(newErrors);
+ // remove errors that weren't encountered on this run
+ currentErrors
+ .removeIf(err -> e.getValue().getErrors().isEmpty()
+ || e.getValue().getErrors().stream().noneMatch(msg -> msg.getCode().getValue() == err.getStatusCode()));
+ LOGGER.trace("Encountered {} errors: {}", currentErrors.size(), currentErrors);
+ base.setErrors(currentErrors);
+ }
+ return base;
+ }
+
/**
* REQUEST VALIDATION LOGIC
*/
@@ -242,7 +262,6 @@ public class DefaultValidationService implements ValidationService {
private void processCommit(Commit c, ValidationResponse response, List<Project> filteredProjects) {
// retrieve the author + committer for the current request
GitUser author = c.getAuthor();
- GitUser committer = c.getCommitter();
response.addMessage(c.getHash(), String.format("Reviewing commit: %1$s", c.getHash()));
response.addMessage(c.getHash(), String.format("Authored by: %1$s <%2$s>", author.getName(), author.getMail()));
@@ -273,6 +292,7 @@ public class DefaultValidationService implements ValidationService {
return;
}
+ GitUser committer = c.getCommitter();
// retrieve the eclipse account for the committer
EclipseUser eclipseCommitter = getIdentifiedUser(committer);
// check if whitelisted or bot
@@ -454,7 +474,7 @@ public class DefaultValidationService implements ValidationService {
return foundUser;
} catch (WebApplicationException e) {
Response r = e.getResponse();
- if (r != null && r.getStatus() == 404) {
+ if (r != null && r.getStatus() == Status.NOT_FOUND.getStatusCode()) {
LOGGER.error("No users found for mail '{}'", user.getMail());
} else {
LOGGER.error("Error while checking for user", e);
diff --git a/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java b/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java
index cfaedf15..dd52e71e 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java
@@ -51,6 +51,9 @@ import io.quarkus.scheduler.Scheduled;
public class GithubRevalidationQueue {
private static final Logger LOGGER = LoggerFactory.getLogger(GithubRevalidationQueue.class);
+ // full repo name should be 2 parts, org and actual repo name
+ private static final int FULL_REPO_NAME_PARTS = 2;
+
@ConfigProperty(name = "eclipse.git-eca.tasks.gh-revalidation.enabled", defaultValue = "true")
Instance<Boolean> isEnabled;
@@ -115,7 +118,7 @@ public class GithubRevalidationQueue {
try {
// split the full repo name into the org and repo name
String[] repoFullNameParts = requestToRevalidate.getRepositoryFullName().split("/");
- if (repoFullNameParts.length != 2) {
+ if (repoFullNameParts.length != FULL_REPO_NAME_PARTS) {
throw new IllegalStateException("Record with ID '" + Long.toString(requestToRevalidate.getId())
+ "' is in an invalid state (repository full name is not valid)");
}
diff --git a/src/main/java/org/eclipsefoundation/git/eca/tasks/ScheduledPrivateProjectScanTask.java b/src/main/java/org/eclipsefoundation/git/eca/tasks/ScheduledPrivateProjectScanTask.java
index d5c7ed04..275cb1da 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/tasks/ScheduledPrivateProjectScanTask.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/tasks/ScheduledPrivateProjectScanTask.java
@@ -48,6 +48,8 @@ import io.quarkus.scheduler.Scheduled;
public class ScheduledPrivateProjectScanTask {
public static final Logger LOGGER = LoggerFactory.getLogger(ScheduledPrivateProjectScanTask.class);
+ private static final int PRIVATE_PROJECT_PAGE_SIZE = 100;
+
@ConfigProperty(name = "eclipse.scheduled.private-project.enabled", defaultValue = "true")
Instance<Boolean> enabled;
@@ -70,18 +72,19 @@ public class ScheduledPrivateProjectScanTask {
APIMiddleware middleware = middlewareHandle.get();
// Fetch all private projects from GL, while paginating results
- Optional<List<GitlabProjectResponse>> response = cache.get("all", new MultivaluedMapImpl<>(),
- GitlabProjectResponse.class,
- () -> middleware.getAll(p -> api.getPrivateProjects(p, apiToken, "private", 100),
- GitlabProjectResponse.class));
+ Optional<List<GitlabProjectResponse>> response = cache
+ .get("all", new MultivaluedMapImpl<>(), GitlabProjectResponse.class,
+ () -> middleware
+ .getAll(p -> api.getPrivateProjects(p, apiToken, "private", PRIVATE_PROJECT_PAGE_SIZE),
+ GitlabProjectResponse.class));
response.ifPresent(this::processGitlabResponseList);
}
}
/**
- * Processes the list of gitlab project response objects and updates the missed
- * deletion, creation, and rename events in the DB
+ * Processes the list of gitlab project response objects and updates the missed deletion, creation, and rename events in
+ * the DB
*
* @param projectList the list of private projects to process
*/
@@ -101,55 +104,55 @@ public class ScheduledPrivateProjectScanTask {
* Updates the DB records that aren't included in the Gitlab project list.
*
* @param projectList The list of private projects to process
- * @param dao The dao service
- * @param filters The filters service
+ * @param dao The dao service
+ * @param filters The filters service
*/
- private void updateMissedDeletionEvents(List<GitlabProjectResponse> projectList, DefaultHibernateDao dao,
- FilterService filters) {
+ private void updateMissedDeletionEvents(List<GitlabProjectResponse> projectList, DefaultHibernateDao dao, FilterService filters) {
RequestWrapper wrapper = new FlatRequestWrapper(URI.create("https://api.eclipse.org"));
// Ad ids to be reverse searched against
MultivaluedMap<String, String> excludingParams = new MultivaluedMapImpl<>();
- excludingParams.put(GitEcaParameterNames.NOT_IN_PROJECT_IDS.getName(),
- projectList.stream().map(p -> Integer.toString(p.getId())).collect(Collectors.toList()));
+ excludingParams
+ .put(GitEcaParameterNames.NOT_IN_PROJECT_IDS.getName(),
+ projectList.stream().map(p -> Integer.toString(p.getId())).collect(Collectors.toList()));
// Get all excluding the ids found on GL
List<PrivateProjectEvent> deletedResults = dao
.get(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class), excludingParams));
if (deletedResults != null) {
- List<PrivateProjectEvent> dtos = deletedResults.stream()
- .map(this::createDtoWithDeletionDate).collect(Collectors.toList());
+ List<PrivateProjectEvent> dtos = deletedResults.stream().map(this::createDtoWithDeletionDate).collect(Collectors.toList());
dao.add(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class)), dtos);
}
}
/**
- * Creates new DB records for each private project that was missed when created
- * or renamed.
+ * Creates new DB records for each private project that was missed when created or renamed.
*
* @param projectList The list of private projects to process
- * @param dao The dao service
- * @param filters The filters service
+ * @param dao The dao service
+ * @param filters The filters service
*/
private void updateMissedCreationAndRenameEvents(List<GitlabProjectResponse> projectList, DefaultHibernateDao dao,
FilterService filters) {
RequestWrapper wrapper = new FlatRequestWrapper(URI.create("https://api.eclipse.org"));
MultivaluedMap<String, String> includingParams = new MultivaluedMapImpl<>();
- includingParams.put(GitEcaParameterNames.PROJECT_IDS.getName(),
- projectList.stream().map(p -> Integer.toString(p.getId())).collect(Collectors.toList()));
+ includingParams
+ .put(GitEcaParameterNames.PROJECT_IDS.getName(),
+ projectList.stream().map(p -> Integer.toString(p.getId())).collect(Collectors.toList()));
// Get by id since project ids never change
- List<PrivateProjectEvent> results = dao
- .get(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class), includingParams));
+ List<PrivateProjectEvent> results = dao.get(new RDBMSQuery<>(wrapper, filters.get(PrivateProjectEvent.class), includingParams));
if (results != null) {
// Includes only projects with namespaces that don't exist or events not in DB
- List<PrivateProjectEvent> dtos = projectList.stream()
- .filter(p -> results.stream().noneMatch(r -> r.getCompositeId().getProjectPath()
- .equalsIgnoreCase(p.getPathWithNamespace())))
+ List<PrivateProjectEvent> dtos = projectList
+ .stream()
+ .filter(p -> results
+ .stream()
+ .noneMatch(r -> r.getCompositeId().getProjectPath().equalsIgnoreCase(p.getPathWithNamespace())))
.map(this::createDto)
.collect(Collectors.toList());
@@ -158,15 +161,14 @@ public class ScheduledPrivateProjectScanTask {
}
/**
- * Takes a PrivateProjectEvent, copies it, updates the deletion date, and
- * returns the copy.
+ * Takes a PrivateProjectEvent, copies it, updates the deletion date, and returns the copy.
*
* @param event The event to update
* @return a PrivateProjectEvent with an updated deletion date
*/
private PrivateProjectEvent createDtoWithDeletionDate(PrivateProjectEvent event) {
- PrivateProjectEvent out = new PrivateProjectEvent(event.getCompositeId().getUserId(),
- event.getCompositeId().getProjectId(), event.getCompositeId().getProjectPath());
+ PrivateProjectEvent out = new PrivateProjectEvent(event.getCompositeId().getUserId(), event.getCompositeId().getProjectId(),
+ event.getCompositeId().getProjectPath());
out.setEFUsername(fetchUserName(event.getCompositeId().getUserId()));
out.setCreationDate(event.getCreationDate());
@@ -176,15 +178,13 @@ public class ScheduledPrivateProjectScanTask {
}
/**
- * Takes the received GitlabProjectResponse object and maps it to a
- * PrivateProjectEvent DTO.
+ * Takes the received GitlabProjectResponse object and maps it to a PrivateProjectEvent DTO.
*
* @param project received project data
* @return A PrivateProjectEvent object
*/
private PrivateProjectEvent createDto(GitlabProjectResponse project) {
- PrivateProjectEvent dto = new PrivateProjectEvent(project.getCreatorId(), project.getId(),
- project.getPathWithNamespace());
+ PrivateProjectEvent dto = new PrivateProjectEvent(project.getCreatorId(), project.getId(), project.getPathWithNamespace());
dto.setEFUsername(fetchUserName(project.getCreatorId()));
dto.setCreationDate(project.getCreatedAt().toLocalDateTime());
@@ -203,8 +203,9 @@ public class ScheduledPrivateProjectScanTask {
* @return the username or null
*/
private String fetchUserName(Integer userId) {
- Optional<GitlabUserResponse> response = cache.get(Integer.toString(userId), new MultivaluedMapImpl<>(),
- GitlabUserResponse.class, () -> api.getUserInfo(apiToken, userId));
+ Optional<GitlabUserResponse> response = cache
+ .get(Integer.toString(userId), new MultivaluedMapImpl<>(), GitlabUserResponse.class,
+ () -> api.getUserInfo(apiToken, userId));
return response.isPresent() ? response.get().getUsername() : null;
}
}
--
GitLab