From 92f458e11d32562701b52613daadf1c257075d8f Mon Sep 17 00:00:00 2001 From: Hendrik Ebbers Date: Mon, 14 Oct 2024 10:42:46 +0200 Subject: [PATCH] minor improvements --- .../com/openelements/issues/ApiEndpoint.java | 14 ++++++++ .../issues/config/RepositoryProperty.java | 7 ++++ .../com/openelements/issues/data/Issue.java | 34 +++++++++++++++---- .../openelements/issues/data/Repository.java | 18 ++++++++++ .../issues/services/GitHubCache.java | 32 +++++++++++++---- .../issues/services/GitHubClient.java | 14 +++++--- 6 files changed, 100 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/openelements/issues/ApiEndpoint.java b/src/main/java/com/openelements/issues/ApiEndpoint.java index 5c821e1..f493ca4 100644 --- a/src/main/java/com/openelements/issues/ApiEndpoint.java +++ b/src/main/java/com/openelements/issues/ApiEndpoint.java @@ -5,10 +5,12 @@ import com.openelements.issues.services.GitHubCache; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import org.jspecify.annotations.NonNull; import org.slf4j.Logger; import org.springframework.web.bind.annotation.CrossOrigin; import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RestController; @RestController @@ -53,4 +55,16 @@ public Set getContributors() { return issueCache.getContributors(); } + @GetMapping("/api/issues") + public Set getGoodFirstIssues(@PathVariable(required = false) Boolean isAssigned, @PathVariable(required = false) Boolean isClosed, @PathVariable(required = false) Set filteredLabels, @PathVariable(required = false) Set excludedLabels, @PathVariable(required = false) Set filteredLanguages) { + log.info("Getting good first issues"); + return issueCache.getAllIssues().stream() + .filter(issue -> isAssigned == null || issue.isAssigned() == isAssigned) + .filter(issue -> isClosed == null || issue.isClosed() == isClosed) + .filter(issue -> filteredLabels == null || issue.labels().containsAll(filteredLabels)) + .filter(issue -> excludedLabels == null || issue.labels().stream().noneMatch(excludedLabels::contains)) + .filter(issue -> filteredLanguages == null || issue.languageTags().containsAll(filteredLanguages)) + .collect(Collectors.toUnmodifiableSet()); + } + } diff --git a/src/main/java/com/openelements/issues/config/RepositoryProperty.java b/src/main/java/com/openelements/issues/config/RepositoryProperty.java index cf8c70d..f36de24 100644 --- a/src/main/java/com/openelements/issues/config/RepositoryProperty.java +++ b/src/main/java/com/openelements/issues/config/RepositoryProperty.java @@ -8,5 +8,12 @@ public record RepositoryProperty(@NonNull String org, @NonNull String repo) { public RepositoryProperty { Objects.requireNonNull(org, "org must not be null"); Objects.requireNonNull(repo, "repo must not be null"); + + if (org.isBlank()) { + throw new IllegalArgumentException("org is required"); + } + if (repo.isBlank()) { + throw new IllegalArgumentException("repo is required"); + } } } diff --git a/src/main/java/com/openelements/issues/data/Issue.java b/src/main/java/com/openelements/issues/data/Issue.java index 44261ce..0211082 100644 --- a/src/main/java/com/openelements/issues/data/Issue.java +++ b/src/main/java/com/openelements/issues/data/Issue.java @@ -6,14 +6,34 @@ public record Issue(@NonNull String title, @NonNull String link, @NonNull String org, @NonNull String repo, @NonNull String imageUrl, @NonNull String identifier, boolean isAssigned, boolean isClosed, @NonNull List labels, @NonNull List languageTags) { - public Issue { - Objects.requireNonNull(title, "title must not be null"); - Objects.requireNonNull(link, "link must not be null"); - Objects.requireNonNull(org, "org must not be null"); - Objects.requireNonNull(repo, "repo must not be null"); - Objects.requireNonNull(imageUrl, "imageUrl must not be null"); - Objects.requireNonNull(identifier, "identifier must not be null"); + public Issue(@NonNull String title, @NonNull String link, @NonNull String org, @NonNull String repo, @NonNull String imageUrl, @NonNull String identifier, boolean isAssigned, boolean isClosed, @NonNull List labels, @NonNull List languageTags) { + this.title = Objects.requireNonNull(title, "title must not be null"); + this.link = Objects.requireNonNull(link, "link must not be null"); + this.org = Objects.requireNonNull(org, "org must not be null"); + this.repo = Objects.requireNonNull(repo, "repo must not be null"); + this.imageUrl = Objects.requireNonNull(imageUrl, "imageUrl must not be null"); + this.identifier = Objects.requireNonNull(identifier, "identifier must not be null"); + this.isAssigned = isAssigned; + this.isClosed = isClosed; Objects.requireNonNull(labels, "labels must not be null"); + this.labels = List.copyOf(labels); Objects.requireNonNull(languageTags, "languageTags must not be null"); + this.languageTags = List.copyOf(languageTags); + + if (title.isBlank()) { + throw new IllegalArgumentException("title is required"); + } + if (link.isBlank()) { + throw new IllegalArgumentException("link is required"); + } + if (org.isBlank()) { + throw new IllegalArgumentException("org is required"); + } + if (repo.isBlank()) { + throw new IllegalArgumentException("repo is required"); + } + if (identifier.isBlank()) { + throw new IllegalArgumentException("identifier is required"); + } } } diff --git a/src/main/java/com/openelements/issues/data/Repository.java b/src/main/java/com/openelements/issues/data/Repository.java index e0c455a..6ac2ac9 100644 --- a/src/main/java/com/openelements/issues/data/Repository.java +++ b/src/main/java/com/openelements/issues/data/Repository.java @@ -1,7 +1,25 @@ package com.openelements.issues.data; import java.util.List; +import java.util.Objects; import org.jspecify.annotations.NonNull; public record Repository(@NonNull String org, @NonNull String name, @NonNull String imageUrl, @NonNull List languages) { + + + + public Repository(@NonNull String org, @NonNull String name, @NonNull String imageUrl, @NonNull List languages) { + this.org = Objects.requireNonNull(org, "org must not be null"); + this.name = Objects.requireNonNull(name, "name must not be null"); + this.imageUrl = Objects.requireNonNull(imageUrl, "imageUrl must not be null"); + Objects.requireNonNull(languages, "languages must not be null"); + this.languages = List.copyOf(languages); + + if (org.isBlank()) { + throw new IllegalArgumentException("org is required"); + } + if (name.isBlank()) { + throw new IllegalArgumentException("name is required"); + } + } } diff --git a/src/main/java/com/openelements/issues/services/GitHubCache.java b/src/main/java/com/openelements/issues/services/GitHubCache.java index 607513b..a5abab9 100644 --- a/src/main/java/com/openelements/issues/services/GitHubCache.java +++ b/src/main/java/com/openelements/issues/services/GitHubCache.java @@ -1,7 +1,12 @@ package com.openelements.issues.services; -import com.openelements.issues.LabelConstants; +import static com.openelements.issues.LabelConstants.GOOD_FIRST_ISSUE_CANDIDATE_LABEL; +import static com.openelements.issues.LabelConstants.GOOD_FIRST_ISSUE_LABEL; +import static com.openelements.issues.LabelConstants.HACKTOBERFEST_LABEL; +import static com.openelements.issues.LabelConstants.HELP_WANTED_LABEL; + import com.openelements.issues.config.IssueServiceProperties; +import com.openelements.issues.config.RepositoryProperty; import com.openelements.issues.data.Contributor; import com.openelements.issues.data.Issue; import com.openelements.issues.data.Repository; @@ -36,13 +41,20 @@ public GitHubCache(@NonNull final IssueServiceProperties properties, final GitHu this.gitHubClient = Objects.requireNonNull(gitHubClient, "gitHubClient must not be null"); this.issuesCache = new ConcurrentHashMap<>(); this.contributorsCache = new ConcurrentHashMap<>(); + log.info("Cache will be updated all {} seconds", CACHE_DURATION.getSeconds()); Executors.newSingleThreadScheduledExecutor().scheduleAtFixedRate(() -> { - properties.getRepositories().forEach(repository -> updateContributors(repository.org(), repository.repo())); - - properties.getRepositories().forEach(repository -> updateIssues(repository.org(), repository.repo(), LabelConstants.GOOD_FIRST_ISSUE_LABEL)); - properties.getRepositories().forEach(repository -> updateIssues(repository.org(), repository.repo(), LabelConstants.GOOD_FIRST_ISSUE_CANDIDATE_LABEL)); - properties.getRepositories().forEach(repository -> updateIssues(repository.org(), repository.repo(), LabelConstants.HACKTOBERFEST_LABEL)); - properties.getRepositories().forEach(repository -> updateIssues(repository.org(), repository.repo(), LabelConstants.HELP_WANTED_LABEL)); + try { + log.info("Updating cache"); + final List repos = properties.getRepositories(); + repos.forEach(repo -> updateContributors(repo.org(), repo.repo())); + repos.forEach(repo -> updateIssues(repo.org(), repo.repo(), GOOD_FIRST_ISSUE_LABEL)); + repos.forEach(repo -> updateIssues(repo.org(), repo.repo(), GOOD_FIRST_ISSUE_CANDIDATE_LABEL)); + repos.forEach(repo -> updateIssues(repo.org(), repo.repo(), HACKTOBERFEST_LABEL)); + repos.forEach(repo -> updateIssues(repo.org(), repo.repo(), HELP_WANTED_LABEL)); + log.info("Cache updated. Found {} contributors and {} issues", getContributors().size(), getAllIssues().size()); + } catch (final Exception e) { + log.error("Failed to update cache", e); + } }, 0, CACHE_DURATION.getSeconds(), TimeUnit.SECONDS); } @@ -70,6 +82,12 @@ private void updateIssues(@NonNull final String org, @NonNull final String repo, } } + public Set getAllIssues() { + return issuesCache.keySet().stream() + .flatMap(key -> issuesCache.get(key).stream()) + .collect(Collectors.toUnmodifiableSet()); + } + public Set getIssues(@NonNull final String label) { Objects.requireNonNull(label, "label must not be null"); return issuesCache.keySet().stream() diff --git a/src/main/java/com/openelements/issues/services/GitHubClient.java b/src/main/java/com/openelements/issues/services/GitHubClient.java index b4bc84e..8167b81 100644 --- a/src/main/java/com/openelements/issues/services/GitHubClient.java +++ b/src/main/java/com/openelements/issues/services/GitHubClient.java @@ -21,6 +21,10 @@ public class GitHubClient { private final static Logger log = LoggerFactory.getLogger(GitHubClient.class); + public static final String GITHUB_TOKEN = "GITHUB_TOKEN"; + public static final String GITHUB_API_URL = "https://api.github.com"; + public static final String HTTP_ACCEPT = "Accept"; + public static final String GITHUB_V_3_JSON = "application/vnd.github.v3+json"; private final RestClient restClient; @@ -28,14 +32,14 @@ public class GitHubClient { public GitHubClient() { Builder builder = RestClient.builder() - .baseUrl("https://api.github.com") - .defaultHeader("Accept", "application/vnd.github.v3+json"); - final String githubToken = System.getenv("GITHUB_TOKEN"); + .baseUrl(GITHUB_API_URL) + .defaultHeader(HTTP_ACCEPT, GITHUB_V_3_JSON); + final String githubToken = System.getenv(GITHUB_TOKEN); if(githubToken != null) { - log.info("Using GITHUB_TOKEN environment variable for GitHub API authentication"); + log.info("Using {} environment variable for GitHub API authentication", GITHUB_TOKEN); builder = builder.defaultHeader("Authorization", "token " + githubToken); } else { - log.warn("No GITHUB_TOKEN environment variable found, GitHub API rate limits will apply"); + log.warn("{} environment variable found, GitHub API rate limits will apply", GITHUB_TOKEN); } restClient = builder.build(); objectMapper = new ObjectMapper();