feat: major improvements on efficiency

mikededo · mikededo · commit f97e11c678ba · 2022-12-25T08:18:10.000+01:00
- Added an internal cache to avoid executing the same query when there
  are many teams in the same repository. This requests the same pull
  requests for each team, which translates on fetching the same comments
  and commits which is a really demanding task.
- Added logged so that we are aware of what is being done in the
  background. We can add an option to make it silent by default.
- Other minor improvements.
diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/entity/Commit.java b/src/main/java/io/pakland/mdas/githubstats/domain/entity/Commit.java
@@ -26,10 +26,6 @@ public class Commit {
     // when adding a commit to a set like structure
     @Override
     public int hashCode() {
-        int result = sha.hashCode();
-        result = 31 * result + (date != null ? date.hashCode() : 0);
-        result = 31 * result + additions;
-        result = 31 * result + deletions;
-        return result;
+        return sha.hashCode();
     }
 }
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/model/GitHubPullRequestDTO.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/model/GitHubPullRequestDTO.java
@@ -16,7 +16,7 @@ public class GitHubPullRequestDTO implements PullRequestDTO {
     private Integer number;
 
     @JsonProperty("state")
-    private PullRequestStateDTO state;
+    private GitHubPullRequestStateDTO state;
 
     @JsonProperty("closed_at")
     private Instant closedAt;
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/model/GitHubPullRequestStateDTO.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/model/GitHubPullRequestStateDTO.java
@@ -3,7 +3,9 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import io.pakland.mdas.githubstats.application.dto.PullRequestStateDTO;
 import io.pakland.mdas.githubstats.domain.enums.PullRequestState;
+import lombok.AllArgsConstructor;
 
+@AllArgsConstructor
 public class GitHubPullRequestStateDTO implements PullRequestStateDTO {
 
     @JsonProperty("state")
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/CommentGitHubRepository.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/CommentGitHubRepository.java
@@ -5,35 +5,48 @@
 import io.pakland.mdas.githubstats.domain.entity.Comment;
 import io.pakland.mdas.githubstats.domain.repository.CommentExternalRepository;
 import io.pakland.mdas.githubstats.infrastructure.github.model.GitHubCommentDTO;
+import java.util.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.reactive.function.client.WebClientResponseException;
 
-import java.util.List;
-
 public class CommentGitHubRepository implements CommentExternalRepository {
+
     private final WebClientConfiguration webClientConfiguration;
     private final Logger logger = LoggerFactory.getLogger(CommentGitHubRepository.class);
 
+    private final Map<Integer, Integer> internalQueryCache = new HashMap<>();
+    private final List<List<Comment>> commentStore = new ArrayList<>();
+
     public CommentGitHubRepository(WebClientConfiguration webClientConfiguration) {
         this.webClientConfiguration = webClientConfiguration;
     }
 
     @Override
     public List<Comment> fetchCommentsFromPullRequest(
-            String repositoryOwner, String repositoryName, Integer pullRequestNumber, Integer page, Integer perPage)
-            throws HttpException {
+        String repositoryOwner, String repositoryName, Integer pullRequestNumber, Integer page,
+        Integer perPage)
+        throws HttpException {
+        String query = String.format("/repos/%s/%s/pulls/%s/comments?%s", repositoryOwner,
+            repositoryName, pullRequestNumber, getRequestParams(page, perPage));
+        Integer dataPosition = internalQueryCache.get(query.hashCode());
+        if (dataPosition != null) {
+            return commentStore.get(dataPosition);
+        }
 
         try {
-
-            return this.webClientConfiguration.getWebClient().get()
-                    .uri(String.format("/repos/%s/%s/pulls/%s/comments?%s", repositoryOwner,
-                            repositoryName, pullRequestNumber, getRequestParams(page, perPage)))
-                    .retrieve()
-                    .bodyToFlux(GitHubCommentDTO.class)
-                    .map(CommentMapper::dtoToEntity)
-                    .collectList()
-                    .block();
+            logger.info(" - Fetching comments from pull request: " + pullRequestNumber);
+            List<Comment> result = this.webClientConfiguration.getWebClient().get()
+                .uri(query)
+                .retrieve()
+                .bodyToFlux(GitHubCommentDTO.class)
+                .map(CommentMapper::dtoToEntity)
+                .collectList()
+                .block();
+            commentStore.add(result);
+            internalQueryCache.put(query.hashCode(), commentStore.size() - 1);
+
+            return result;
 
         } catch (WebClientResponseException ex) {
             logger.error(ex.toString());
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/CommitGitHubRepository.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/CommitGitHubRepository.java
@@ -5,41 +5,57 @@
 import io.pakland.mdas.githubstats.domain.entity.Commit;
 import io.pakland.mdas.githubstats.domain.repository.CommitExternalRepository;
 import io.pakland.mdas.githubstats.infrastructure.github.model.GitHubCommitDTO;
+import java.util.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.reactive.function.client.WebClientResponseException;
 
-import java.util.List;
-
 public class CommitGitHubRepository implements CommitExternalRepository {
+
     private final WebClientConfiguration webClientConfiguration;
     private final Logger logger = LoggerFactory.getLogger(CommitGitHubRepository.class);
 
+    private final Map<Integer, Integer> internalQueryCache = new HashMap<>();
+    private final List<List<Commit>> commitStore = new ArrayList<>();
+
     public CommitGitHubRepository(WebClientConfiguration webClientConfiguration) {
         this.webClientConfiguration = webClientConfiguration;
     }
 
     @Override
-    public List<Commit> fetchCommitsFromPullRequest(FetchCommitsFromPullRequestRequest request) throws HttpException {
+    public List<Commit> fetchCommitsFromPullRequest(FetchCommitsFromPullRequestRequest request)
+        throws HttpException {
+        String query = String.format("/repos/%s/%s/pulls/%s/commits?%s",
+            request.getRepositoryOwner(),
+            request.getRepositoryName(), request.getPullRequestNumber(),
+            getRequestParams(request));
+        Integer resultPosition = internalQueryCache.get(query.hashCode());
+        if (resultPosition != null) {
+            return commitStore.get(resultPosition);
+        }
 
         try {
-
-            return this.webClientConfiguration.getWebClient().get()
-                    .uri(String.format("/repos/%s/%s/pulls/%s/commits?%s", request.getRepositoryOwner(),
-                            request.getRepositoryName(), request.getPullRequestNumber(), getRequestParams(request)))
-                    .retrieve()
-                    .bodyToFlux(GitHubCommitDTO.class)
-                    .map(CommitMapper::dtoToEntity)
-                    .collectList()
-                    .block();
-
+            logger.info(" - Fetching commits for pull request: " + request.getPullRequestNumber()
+                .toString());
+            List<Commit> result = this.webClientConfiguration.getWebClient().get()
+                .uri(query)
+                .retrieve()
+                .bodyToFlux(GitHubCommitDTO.class)
+                .map(CommitMapper::dtoToEntity)
+                .collectList()
+                .block();
+            commitStore.add(result);
+            internalQueryCache.put(query.hashCode(), commitStore.size() - 1);
+
+            return result;
         } catch (WebClientResponseException ex) {
             logger.error(ex.toString());
             throw new HttpException(ex.getRawStatusCode(), ex.getMessage());
         }
     }
 
     private String getRequestParams(FetchCommitsFromPullRequestRequest request) {
-        return String.format("per_page=%d&page=%d", request.getPerPage(), request.getPage() < 0 ? 1 : request.getPage());
+        return String.format("per_page=%d&page=%d", request.getPerPage(),
+            request.getPage() < 0 ? 1 : request.getPage());
     }
 }
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/OrganizationGitHubRepository.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/OrganizationGitHubRepository.java
@@ -22,6 +22,7 @@ public OrganizationGitHubRepository(WebClientConfiguration webClientConfiguratio
     @Override
     public List<Organization> fetchAvailableOrganizations() throws HttpException {
         try {
+            logger.info(" - Fetching organizations");
             return this.webClientConfiguration.getWebClient().get()
                 .uri("/user/orgs")
                 .retrieve()
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/PullRequestGitHubRepository.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/PullRequestGitHubRepository.java
@@ -23,6 +23,9 @@ public PullRequestGitHubRepository(WebClientConfiguration webClientConfiguration
     public List<PullRequest> fetchPullRequestsFromRepository(
         FetchPullRequestFromRepositoryRequest request) throws HttpException {
         try {
+            logger.info(
+                " - Fetching pull requests from repository: " + request.getRepositoryOwner() + "/"
+                    + request.getRepository());
             return this.webClientConfiguration.getWebClient().get()
                 .uri(String.format("/repos/%s/%s/pulls?%s", request.getRepositoryOwner(),
                     request.getRepository(), getRequestParams(request)))
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/RepositoryGitHubRepository.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/RepositoryGitHubRepository.java
@@ -23,6 +23,7 @@ public RepositoryGitHubRepository(WebClientConfiguration webClientConfiguration)
     public List<Repository> fetchTeamRepositories(String organizationLogin, String teamSlug)
         throws HttpException {
         try {
+            logger.info(" - Fetching repositories for team: " + organizationLogin + "/" + teamSlug);
             return this.webClientConfiguration.getWebClient().get()
                 .uri(String.format("/orgs/%s/teams/%s/repos", organizationLogin, teamSlug))
                 .retrieve()
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/TeamGitHubRepository.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/TeamGitHubRepository.java
@@ -22,6 +22,7 @@ public TeamGitHubRepository(WebClientConfiguration webClientConfiguration) {
     @Override
     public List<Team> fetchTeamsFromOrganization(String organizationName) throws HttpException {
         try {
+            logger.info(" - Fetching teams from organization: " + organizationName);
             return this.webClientConfiguration.getWebClient().get()
                 .uri(String.format("/orgs/%s/teams", organizationName))
                 .retrieve()
diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/UserGitHubRepository.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/UserGitHubRepository.java
@@ -25,6 +25,7 @@ public UserGitHubRepository(WebClientConfiguration webClientConfiguration) {
     public List<User> fetchUsersFromTeam(String organizationName, String teamName)
         throws HttpException {
         try {
+            logger.info(" - Fetching users from team: " + organizationName + "/" + teamName);
             return this.webClientConfiguration.getWebClient().get()
                 .uri(String.format("/orgs/%s/teams/%s/members", organizationName, teamName))
                 .retrieve()

Original file line number	Diff line number	Diff line change
`@@ -26,10 +26,6 @@ public class Commit {`
`26`	`26`	`// when adding a commit to a set like structure`
`27`	`27`	`@Override`
`28`	`28`	`public int hashCode() {`
`29`		`- int result = sha.hashCode();`
`30`		`- result = 31 * result + (date != null ? date.hashCode() : 0);`
`31`		`- result = 31 * result + additions;`
`32`		`- result = 31 * result + deletions;`
`33`		`- return result;`
	`29`	`+ return sha.hashCode();`
`34`	`30`	`}`
`35`	`31`	`}`