From 26e39aeb1b7e2cb776a9d510b5ccf82a0b607dbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Manero?= Date: Tue, 13 Dec 2022 18:28:13 +0100 Subject: [PATCH 1/4] feat (WIP): fetchCommitsFromPullRequest Use Case --- .../FetchCommitsFromPullRequest.java | 39 + .../mdas/githubstats/domain/Commit.java | 27 +- .../mdas/githubstats/domain/PullRequest.java | 69 +- .../repository/CommitExternalRepository.java | 26 + .../repository/CommitGitHubRepository.java | 40 + .../controller/UserOptionController.java | 20 +- .../changelog/changes/000_initial_schema.yaml | 740 +++++++++--------- .../FetchCommitsFromPullRequestTest.java | 84 ++ 8 files changed, 618 insertions(+), 427 deletions(-) create mode 100644 src/main/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequest.java create mode 100644 src/main/java/io/pakland/mdas/githubstats/domain/repository/CommitExternalRepository.java create mode 100644 src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/CommitGitHubRepository.java create mode 100644 src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java diff --git a/src/main/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequest.java b/src/main/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequest.java new file mode 100644 index 00000000..041dfb85 --- /dev/null +++ b/src/main/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequest.java @@ -0,0 +1,39 @@ +package io.pakland.mdas.githubstats.application; + +import io.pakland.mdas.githubstats.application.exceptions.HttpException; +import io.pakland.mdas.githubstats.domain.Commit; +import io.pakland.mdas.githubstats.domain.repository.CommitExternalRepository; + +import java.util.ArrayList; +import java.util.List; + +public class FetchCommitsFromPullRequest { + private final CommitExternalRepository commitExternalRepository; + + public FetchCommitsFromPullRequest(CommitExternalRepository commitExternalRepository) { + this.commitExternalRepository = commitExternalRepository; + } + + public List execute(String repositoryOwner, String repositoryName, Integer pullRequestNumber) throws HttpException { + int page = 1; + List commitList = new ArrayList<>(); + int responseResults; + do { + CommitExternalRepository.FetchCommitsFromPullRequestRequest request = CommitExternalRepository.FetchCommitsFromPullRequestRequest.builder() + .repositoryOwner(repositoryOwner) + .repositoryName(repositoryName) + .pullRequestNumber(pullRequestNumber) + .page(page) + .perPage(100) + .build(); + List apiResults = this.commitExternalRepository.fetchCommitsFromPullRequest( + request); + + commitList.addAll(apiResults); + responseResults = apiResults.size(); + page++; + } while (responseResults > 0); + + return commitList; + } +} diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java b/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java index 90c331ee..36a095c7 100644 --- a/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java +++ b/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java @@ -1,11 +1,14 @@ package io.pakland.mdas.githubstats.domain; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Data; import lombok.NoArgsConstructor; import lombok.ToString; import javax.persistence.*; import java.time.Instant; +import java.util.Date; +import java.util.Map; @Data @NoArgsConstructor @@ -16,7 +19,8 @@ public class Commit { @Id @Column(updatable = false, nullable = false) - private Integer id; + @JsonProperty("sha") + private String sha; @ManyToOne(fetch = FetchType.LAZY) private User user; @@ -24,22 +28,11 @@ public class Commit { @ManyToOne(fetch = FetchType.LAZY) private PullRequest pullRequest; - private int additions; + @Column(name = "date") + private Date date; - private int deletions; - - private Instant date; - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof Commit)) return false; - return id != null && id.equals(((Commit) o).getId()); + @JsonProperty("commit") + private void unpackNameFromNestedObject(Map> owner) { + this.date = Date.from(Instant.parse(owner.get("commiter").get("date"))); } - - @Override - public int hashCode() { - return getClass().hashCode(); - } - } diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/PullRequest.java b/src/main/java/io/pakland/mdas/githubstats/domain/PullRequest.java index 308ba56b..d657570a 100644 --- a/src/main/java/io/pakland/mdas/githubstats/domain/PullRequest.java +++ b/src/main/java/io/pakland/mdas/githubstats/domain/PullRequest.java @@ -1,12 +1,11 @@ package io.pakland.mdas.githubstats.domain; -import javax.persistence.*; - import com.fasterxml.jackson.annotation.JsonProperty; import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import javax.persistence.*; import java.util.ArrayList; import java.util.List; @@ -16,33 +15,41 @@ @Entity @Table(name = "pull_request") public class PullRequest { - @Id - @Column(updatable = false, nullable = false) - @JsonProperty("id") - private Integer id; - - @Column(name="number") - @JsonProperty("number") - private Integer number; - - @Column - @JsonProperty("state") - private PullRequestState state; - - @OneToMany( - mappedBy = "pullRequest", - cascade = CascadeType.ALL, - orphanRemoval = true - ) - private List userReviews = new ArrayList<>(); - - @OneToMany( - mappedBy = "pullRequest", - cascade = CascadeType.ALL, - orphanRemoval = true - ) - private List commits = new ArrayList<>(); - - @ManyToOne(fetch = FetchType.LAZY) - private Repository repository; + @Id + @Column(updatable = false, nullable = false) + @JsonProperty("id") + private Integer id; + + @Column(name = "number") + @JsonProperty("number") + private Integer number; + + @Column(name = "state") + @JsonProperty("state") + private PullRequestState state; + + @Column(name = "additions") + @JsonProperty("additions") + private Integer additions; + + @Column(name = "deletions") + @JsonProperty("deletions") + private Integer deletions; + + @OneToMany( + mappedBy = "pullRequest", + cascade = CascadeType.ALL, + orphanRemoval = true + ) + private List userReviews = new ArrayList<>(); + + @OneToMany( + mappedBy = "pullRequest", + cascade = CascadeType.ALL, + orphanRemoval = true + ) + private List commits = new ArrayList<>(); + + @ManyToOne(fetch = FetchType.LAZY) + private Repository repository; } diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/repository/CommitExternalRepository.java b/src/main/java/io/pakland/mdas/githubstats/domain/repository/CommitExternalRepository.java new file mode 100644 index 00000000..93ef4284 --- /dev/null +++ b/src/main/java/io/pakland/mdas/githubstats/domain/repository/CommitExternalRepository.java @@ -0,0 +1,26 @@ +package io.pakland.mdas.githubstats.domain.repository; + +import io.pakland.mdas.githubstats.application.exceptions.HttpException; +import io.pakland.mdas.githubstats.domain.Commit; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.NoArgsConstructor; + +import java.util.List; + +public interface CommitExternalRepository { + List fetchCommitsFromPullRequest(FetchCommitsFromPullRequestRequest request) throws HttpException; + + @NoArgsConstructor + @AllArgsConstructor + @Builder + @Getter + public static class FetchCommitsFromPullRequestRequest { + private String repositoryOwner; + private String repositoryName; + private Integer pullRequestNumber; + private Integer page; + private Integer perPage; + } +} 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 new file mode 100644 index 00000000..abc1f885 --- /dev/null +++ b/src/main/java/io/pakland/mdas/githubstats/infrastructure/github/repository/CommitGitHubRepository.java @@ -0,0 +1,40 @@ +package io.pakland.mdas.githubstats.infrastructure.github.repository; + +import io.pakland.mdas.githubstats.application.exceptions.HttpException; +import io.pakland.mdas.githubstats.domain.Commit; +import io.pakland.mdas.githubstats.domain.repository.CommitExternalRepository; +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); + + public CommitGitHubRepository(WebClientConfiguration webClientConfiguration) { + this.webClientConfiguration = webClientConfiguration; + } + + @Override + public List fetchCommitsFromPullRequest(FetchCommitsFromPullRequestRequest request) throws HttpException { + + 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(Commit.class) + .collectList() + .block(); + } 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()); + } +} diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java index 04e0ddbf..6ff71e35 100644 --- a/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java +++ b/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java @@ -1,10 +1,7 @@ package io.pakland.mdas.githubstats.infrastructure.shell.controller; import io.pakland.mdas.githubstats.FetchUsersFromTeam; -import io.pakland.mdas.githubstats.application.FetchAvailableOrganizations; -import io.pakland.mdas.githubstats.application.FetchPullRequestsFromRepository; -import io.pakland.mdas.githubstats.application.FetchRepositoriesFromTeam; -import io.pakland.mdas.githubstats.application.FetchTeamsFromOrganization; +import io.pakland.mdas.githubstats.application.*; import io.pakland.mdas.githubstats.application.exceptions.HttpException; import io.pakland.mdas.githubstats.domain.*; import io.pakland.mdas.githubstats.domain.repository.*; @@ -28,16 +25,18 @@ public class UserOptionController { private UserExternalRepository userExternalRepository; private RepositoryExternalRepository repositoryExternalRepository; private PullRequestExternalRepository pullRequestExternalRepository; + private CommitExternalRepository commitExternalRepository; public UserOptionController(UserOptionRequest userOptionRequest) { this.userOptionRequest = userOptionRequest; WebClientConfiguration webClientConfiguration = new WebClientConfiguration( - "https://api.github.com", userOptionRequest.getApiKey()); + "https://api.github.com", userOptionRequest.getApiKey()); this.organizationExternalRepository = new OrganizationGitHubRepository(webClientConfiguration); this.teamExternalRepository = new TeamGitHubRepository(webClientConfiguration); this.userExternalRepository = new UserGitHubRepository(webClientConfiguration); this.repositoryExternalRepository = new RepositoryGitHubRepository(webClientConfiguration); this.pullRequestExternalRepository = new PullRequestGitHubRepository(webClientConfiguration); + this.commitExternalRepository = new CommitGitHubRepository(webClientConfiguration); } public void execute() { @@ -54,7 +53,6 @@ public void execute() { for (Team team : teamList) { // Fetch the members of each team. - logger.info(organization.getLogin()); List userList = new FetchUsersFromTeam(userExternalRepository) .execute(organization.getLogin(), team.getSlug()); // Fetch the repositories for each team. @@ -70,7 +68,15 @@ public void execute() { TODO: if the user of the PR belongs to the team, increment the prs executed inside the team, TODO: else increment the prs executed outside the team. */ - // TODO: fetch commits from each PR. + // TODO: Save for later calculate the Additions from PR aggregation. + // TODO: Save for later calculate the Deletions from PR aggregation. + // TODO: Save for later calculate the commit number from PR aggregation. + List commitList = new FetchCommitsFromPullRequest(commitExternalRepository) + .execute(repository.getOwnerLogin(), repository.getName(), pullRequest.getNumber()); + logger.info(String.valueOf(commitList.size())); + for (Commit commit : commitList) { + // TODO: Fetch PR reviews. + } } repository.setPullRequests(pullRequestList); diff --git a/src/main/resources/db/changelog/changes/000_initial_schema.yaml b/src/main/resources/db/changelog/changes/000_initial_schema.yaml index 8b6143b6..2de880b2 100644 --- a/src/main/resources/db/changelog/changes/000_initial_schema.yaml +++ b/src/main/resources/db/changelog/changes/000_initial_schema.yaml @@ -1,374 +1,370 @@ databaseChangeLog: -- changeSet: - id: 1669767244074-1 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: comment_pkey - name: id - type: INT - - column: - constraints: - nullable: false - name: length - type: INTEGER - - column: - name: user_review_id - type: INT - tableName: comment -- changeSet: - id: 1669767244074-2 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: commit_pkey - name: id - type: INT - - column: - constraints: - nullable: false - name: additions - type: INTEGER - - column: - name: date - type: TIMESTAMP WITHOUT TIME ZONE - - column: - constraints: - nullable: false - name: deletions - type: INTEGER - - column: - name: pull_request_id - type: INT - - column: - name: user_id - type: INT - tableName: commit -- changeSet: - id: 1669767244074-3 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: historic_queries_pkey - name: id - type: INT - - column: - constraints: - nullable: false - name: from - type: VARCHAR(255) - - column: - constraints: - nullable: false - name: name - type: VARCHAR(255) - - column: - constraints: - nullable: false - name: to - type: VARCHAR(255) - - column: - name: team_id - type: INT - tableName: historic_queries -- changeSet: - id: 1669767244074-4 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: organization_pkey - name: id - type: INT - - column: - name: login - type: VARCHAR(255) - - column: - name: organization_url - type: VARCHAR(255) - tableName: organization -- changeSet: - id: 1669767244074-5 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: pull_request_pkey - name: id - type: INT - - column: - name: number - type: INT - - column: - name: state - type: INT - - column: - name: repository_id - type: INT - tableName: pull_request -- changeSet: - id: 1669767244074-6 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: repository_pkey - name: id - type: INT - - column: - name: name - type: VARCHAR(255) - - column: - name: owner_login - type: VARCHAR(255) - - column: - name: team_id - type: INT - tableName: repository -- changeSet: - id: 1669767244074-7 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: team_pkey - name: id - type: INT - - column: - name: slug - type: VARCHAR(255) - - column: - name: organization_id - type: INT - tableName: team -- changeSet: - id: 1669767244074-8 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: user_pkey - name: id - type: INT - - column: - name: login - type: VARCHAR(255) - - column: - name: team_id - type: INT - tableName: user -- changeSet: - id: 1669767244074-9 - author: Paco Lozano - changes: - - createTable: - columns: - - column: - autoIncrement: false - constraints: - nullable: false - primaryKey: true - primaryKeyName: user_review_pkey - name: id - type: INT - - column: - name: pull_request_id - type: INT - - column: - name: user_id - type: INT - tableName: user_review -- changeSet: - id: 1669767244074-10 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: user_review_id - baseTableName: comment - constraintName: fk8s2cpckjmj44ngrn9md8sxwyp - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: user_review - validate: true -- changeSet: - id: 1669767244074-11 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: pull_request_id - baseTableName: user_review - constraintName: fkeqpkkltl0ltj61js2acoaitpi - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: pull_request - validate: true -- changeSet: - id: 1669767244074-12 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: user_id - baseTableName: commit - constraintName: fkf2u03vla3r16vjhvuk7i089wc - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: user - validate: true -- changeSet: - id: 1669767244074-13 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: pull_request_id - baseTableName: commit - constraintName: fkiip0bvo92fj8nylct2gggo0js - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: pull_request - validate: true -- changeSet: - id: 1669767244074-14 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: user_id - baseTableName: user_review - constraintName: fknd27adjba2cprm0uxexficol7 - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: user - validate: true -- changeSet: - id: 1669767244074-15 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: repository_id - baseTableName: pull_request - constraintName: fko0xkv3or7y7dpnr62ytul9kca - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: repository - validate: true -- changeSet: - id: 1669767244074-16 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: team_id - baseTableName: user - constraintName: fkoog08jgyx3j3mxu9dxgohg39w - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: team - validate: true -- changeSet: - id: 1669767244074-17 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: team_id - baseTableName: repository - constraintName: fkpa55jqergf2krgr03kyv6g55m - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: team - validate: true -- changeSet: - id: 1669767244074-18 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: team_id - baseTableName: historic_queries - constraintName: fkt1xkfefkor7uasbn5abhhmj3v - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: team - validate: true -- changeSet: - id: 1669767244074-19 - author: Paco Lozano - changes: - - addForeignKeyConstraint: - baseColumnNames: organization_id - baseTableName: team - constraintName: fkt2rwhhxcjdmje0gqqybiyjdpn - deferrable: false - initiallyDeferred: false - onDelete: NO ACTION - onUpdate: NO ACTION - referencedColumnNames: id - referencedTableName: organization - validate: true + - changeSet: + id: 1669767244074-1 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: comment_pkey + name: id + type: INT + - column: + constraints: + nullable: false + name: length + type: INTEGER + - column: + name: user_review_id + type: INT + tableName: comment + - changeSet: + id: 1669767244074-2 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: commit_pkey + name: sha + type: VARCHAR(255) + - column: + name: date + type: TIMESTAMP WITHOUT TIME ZONE + - column: + name: pull_request_id + type: INT + - column: + name: user_id + type: INT + tableName: commit + - changeSet: + id: 1669767244074-3 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: historic_queries_pkey + name: id + type: INT + - column: + constraints: + nullable: false + name: from + type: VARCHAR(255) + - column: + constraints: + nullable: false + name: name + type: VARCHAR(255) + - column: + constraints: + nullable: false + name: to + type: VARCHAR(255) + - column: + name: team_id + type: INT + tableName: historic_queries + - changeSet: + id: 1669767244074-4 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: organization_pkey + name: id + type: INT + - column: + name: login + type: VARCHAR(255) + - column: + name: organization_url + type: VARCHAR(255) + tableName: organization + - changeSet: + id: 1669767244074-5 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: pull_request_pkey + name: id + type: INT + - column: + name: number + type: INT + - column: + name: state + type: INT + - column: + name: additions + type: INT + - column: + name: deletions + type: INT + - column: + name: repository_id + type: INT + tableName: pull_request + - changeSet: + id: 1669767244074-6 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: repository_pkey + name: id + type: INT + - column: + name: name + type: VARCHAR(255) + - column: + name: owner_login + type: VARCHAR(255) + - column: + name: team_id + type: INT + tableName: repository + - changeSet: + id: 1669767244074-7 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: team_pkey + name: id + type: INT + - column: + name: slug + type: VARCHAR(255) + - column: + name: organization_id + type: INT + tableName: team + - changeSet: + id: 1669767244074-8 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: user_pkey + name: id + type: INT + - column: + name: login + type: VARCHAR(255) + - column: + name: team_id + type: INT + tableName: user + - changeSet: + id: 1669767244074-9 + author: Paco Lozano + changes: + - createTable: + columns: + - column: + autoIncrement: false + constraints: + nullable: false + primaryKey: true + primaryKeyName: user_review_pkey + name: id + type: INT + - column: + name: pull_request_id + type: INT + - column: + name: user_id + type: INT + tableName: user_review + - changeSet: + id: 1669767244074-10 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: user_review_id + baseTableName: comment + constraintName: fk8s2cpckjmj44ngrn9md8sxwyp + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: user_review + validate: true + - changeSet: + id: 1669767244074-11 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: pull_request_id + baseTableName: user_review + constraintName: fkeqpkkltl0ltj61js2acoaitpi + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: pull_request + validate: true + - changeSet: + id: 1669767244074-12 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: user_id + baseTableName: commit + constraintName: fkf2u03vla3r16vjhvuk7i089wc + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: user + validate: true + - changeSet: + id: 1669767244074-13 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: pull_request_id + baseTableName: commit + constraintName: fkiip0bvo92fj8nylct2gggo0js + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: pull_request + validate: true + - changeSet: + id: 1669767244074-14 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: user_id + baseTableName: user_review + constraintName: fknd27adjba2cprm0uxexficol7 + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: user + validate: true + - changeSet: + id: 1669767244074-15 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: repository_id + baseTableName: pull_request + constraintName: fko0xkv3or7y7dpnr62ytul9kca + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: repository + validate: true + - changeSet: + id: 1669767244074-16 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: team_id + baseTableName: user + constraintName: fkoog08jgyx3j3mxu9dxgohg39w + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: team + validate: true + - changeSet: + id: 1669767244074-17 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: team_id + baseTableName: repository + constraintName: fkpa55jqergf2krgr03kyv6g55m + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: team + validate: true + - changeSet: + id: 1669767244074-18 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: team_id + baseTableName: historic_queries + constraintName: fkt1xkfefkor7uasbn5abhhmj3v + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: team + validate: true + - changeSet: + id: 1669767244074-19 + author: Paco Lozano + changes: + - addForeignKeyConstraint: + baseColumnNames: organization_id + baseTableName: team + constraintName: fkt2rwhhxcjdmje0gqqybiyjdpn + deferrable: false + initiallyDeferred: false + onDelete: NO ACTION + onUpdate: NO ACTION + referencedColumnNames: id + referencedTableName: organization + validate: true diff --git a/src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java b/src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java new file mode 100644 index 00000000..56965a6b --- /dev/null +++ b/src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java @@ -0,0 +1,84 @@ +package io.pakland.mdas.githubstats.application; + +import io.pakland.mdas.githubstats.application.exceptions.HttpException; +import io.pakland.mdas.githubstats.domain.Commit; +import io.pakland.mdas.githubstats.domain.PullRequestState; +import io.pakland.mdas.githubstats.domain.repository.PullRequestExternalRepository.FetchPullRequestFromRepositoryRequest; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class FetchCommitsFromPullRequestTest { + + private void validateRequestCaptor( + ArgumentCaptor captor) { + assertEquals("github-stats-22", captor.getValue().getRepositoryOwner()); + assertEquals("github-stats", captor.getValue().getRepository()); + assertEquals(1, captor.getValue().getPage()); + assertEquals(100, captor.getValue().getPerPage()); + assertEquals(PullRequestState.ALL, captor.getValue().getState()); + } + + @Test + public void whenValidOrganizationAndRepository_shouldReturnTheListOfPullRequests() + throws HttpException { + Commit commit = new Commit(); + commit.setId(1157910685); + prOne.setNumber(71); + PullRequest prTwo = new PullRequest(); + prTwo.setId(1157867973); + prTwo.setNumber(69); + + PullRequestExternalRepository repository = Mockito.mock( + PullRequestExternalRepository.class); + Mockito.when(repository.fetchPullRequestsFromRepository(Mockito.any( + FetchPullRequestFromRepositoryRequest.class))).thenReturn(List.of(prOne, prTwo)); + + List response = new FetchPullRequestsFromRepository(repository).execute( + "github-stats-22", "github-stats" + ); + + ArgumentCaptor captor = ArgumentCaptor.forClass( + FetchPullRequestFromRepositoryRequest.class); + Mockito.verify(repository).fetchPullRequestsFromRepository(captor.capture()); + + validateRequestCaptor(captor); + assertEquals(2, response.size()); + assertEquals(prOne.getId(), response.get(0).getId()); + assertEquals(prTwo.getId(), response.get(1).getId()); + } + + @Test + public void whenRepositoryReturnsEmptyResponse_shouldReturnEmptyList() throws HttpException { +// PullRequestExternalRepository repository = Mockito.mock( +// PullRequestExternalRepository.class); +// Mockito.when(repository.fetchPullRequestsFromRepository(Mockito.any( +// FetchPullRequestFromRepositoryRequest.class))).thenReturn(new ArrayList<>()); +// +// List response = new FetchPullRequestsFromRepository(repository).execute( +// "github-stats-22", "github-stats" +// ); +// +// ArgumentCaptor captor = ArgumentCaptor.forClass( +// FetchPullRequestFromRepositoryRequest.class); +// Mockito.verify(repository).fetchPullRequestsFromRepository(captor.capture()); +// +// validateRequestCaptor(captor); +// assertEquals(0, response.size()); + } + + @Test + public void whenRepositoryThrowsException_shouldThrowHttpException() throws HttpException { +// PullRequestExternalRepository repository = Mockito.mock( +// PullRequestExternalRepository.class); +// Mockito.when(repository.fetchPullRequestsFromRepository(Mockito.any( +// FetchPullRequestFromRepositoryRequest.class))) +// .thenThrow(new HttpException(404, "Page not found.")); +// +// assertThrows(HttpException.class, () -> { +// new FetchPullRequestsFromRepository(repository).execute("github-stats-22", +// "github-stats"); +// }); + } +} From 4d4d19830cdda05ccae14221e771d74d9ecb3c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Manero?= Date: Tue, 13 Dec 2022 20:32:50 +0100 Subject: [PATCH 2/4] feat: tests passing --- .../FetchAvailableOrganizations.java | 2 - .../FetchPullRequestsFromRepository.java | 20 ++- .../FetchRepositoriesFromTeam.java | 2 - .../FetchTeamsFromOrganization.java | 2 - .../mdas/githubstats/domain/Commit.java | 11 +- .../githubstats/domain/CommitAggregation.java | 5 +- .../controller/UserOptionController.java | 24 ++-- .../changelog/changes/000_initial_schema.yaml | 2 +- .../application/AggregateCommitsTest.java | 59 +++++---- .../FetchCommitsFromPullRequestTest.java | 125 ++++++++++-------- 10 files changed, 131 insertions(+), 121 deletions(-) diff --git a/src/main/java/io/pakland/mdas/githubstats/application/FetchAvailableOrganizations.java b/src/main/java/io/pakland/mdas/githubstats/application/FetchAvailableOrganizations.java index d97d7763..30541026 100644 --- a/src/main/java/io/pakland/mdas/githubstats/application/FetchAvailableOrganizations.java +++ b/src/main/java/io/pakland/mdas/githubstats/application/FetchAvailableOrganizations.java @@ -3,11 +3,9 @@ import io.pakland.mdas.githubstats.application.exceptions.HttpException; import io.pakland.mdas.githubstats.domain.Organization; import io.pakland.mdas.githubstats.domain.repository.OrganizationExternalRepository; -import org.springframework.stereotype.Service; import java.util.List; -@Service public class FetchAvailableOrganizations { private final OrganizationExternalRepository organizationExternalRepository; diff --git a/src/main/java/io/pakland/mdas/githubstats/application/FetchPullRequestsFromRepository.java b/src/main/java/io/pakland/mdas/githubstats/application/FetchPullRequestsFromRepository.java index ac364dbd..eb732988 100644 --- a/src/main/java/io/pakland/mdas/githubstats/application/FetchPullRequestsFromRepository.java +++ b/src/main/java/io/pakland/mdas/githubstats/application/FetchPullRequestsFromRepository.java @@ -5,36 +5,34 @@ import io.pakland.mdas.githubstats.domain.PullRequestState; import io.pakland.mdas.githubstats.domain.repository.PullRequestExternalRepository; import io.pakland.mdas.githubstats.domain.repository.PullRequestExternalRepository.FetchPullRequestFromRepositoryRequest; -import org.springframework.stereotype.Service; import java.util.ArrayList; import java.util.List; -@Service public class FetchPullRequestsFromRepository { private final PullRequestExternalRepository pullRequestExternalRepository; public FetchPullRequestsFromRepository( - PullRequestExternalRepository pullRequestExternalRepository) { + PullRequestExternalRepository pullRequestExternalRepository) { this.pullRequestExternalRepository = pullRequestExternalRepository; } public List execute(String repositoryOwnerLogin, String repositoryName) - throws HttpException { + throws HttpException { int page = 1; List pullRequestList = new ArrayList<>(); int responseResults; do { FetchPullRequestFromRepositoryRequest request = FetchPullRequestFromRepositoryRequest.builder() - .repositoryOwner(repositoryOwnerLogin) - .repository(repositoryName) - .page(page) - .perPage(100) - .state(PullRequestState.ALL) - .build(); + .repositoryOwner(repositoryOwnerLogin) + .repository(repositoryName) + .page(page) + .perPage(100) + .state(PullRequestState.ALL) + .build(); List apiResults = this.pullRequestExternalRepository.fetchPullRequestsFromRepository( - request); + request); pullRequestList.addAll(apiResults); responseResults = apiResults.size(); diff --git a/src/main/java/io/pakland/mdas/githubstats/application/FetchRepositoriesFromTeam.java b/src/main/java/io/pakland/mdas/githubstats/application/FetchRepositoriesFromTeam.java index a770bdb4..1f27a09e 100644 --- a/src/main/java/io/pakland/mdas/githubstats/application/FetchRepositoriesFromTeam.java +++ b/src/main/java/io/pakland/mdas/githubstats/application/FetchRepositoriesFromTeam.java @@ -3,11 +3,9 @@ import io.pakland.mdas.githubstats.application.exceptions.HttpException; import io.pakland.mdas.githubstats.domain.Repository; import io.pakland.mdas.githubstats.domain.repository.RepositoryExternalRepository; -import org.springframework.stereotype.Service; import java.util.List; -@Service public class FetchRepositoriesFromTeam { private final RepositoryExternalRepository repositoryExternalRepository; diff --git a/src/main/java/io/pakland/mdas/githubstats/application/FetchTeamsFromOrganization.java b/src/main/java/io/pakland/mdas/githubstats/application/FetchTeamsFromOrganization.java index 1a31dcac..023e6e22 100644 --- a/src/main/java/io/pakland/mdas/githubstats/application/FetchTeamsFromOrganization.java +++ b/src/main/java/io/pakland/mdas/githubstats/application/FetchTeamsFromOrganization.java @@ -3,11 +3,9 @@ import io.pakland.mdas.githubstats.application.exceptions.HttpException; import io.pakland.mdas.githubstats.domain.Team; import io.pakland.mdas.githubstats.domain.repository.TeamExternalRepository; -import org.springframework.stereotype.Service; import java.util.List; -@Service public class FetchTeamsFromOrganization { private final TeamExternalRepository teamExternalRepository; diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java b/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java index 36a095c7..52f9d684 100644 --- a/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java +++ b/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java @@ -6,7 +6,6 @@ import lombok.ToString; import javax.persistence.*; -import java.time.Instant; import java.util.Date; import java.util.Map; @@ -22,17 +21,17 @@ public class Commit { @JsonProperty("sha") private String sha; + @Column(name = "date") + private Date date; + @ManyToOne(fetch = FetchType.LAZY) private User user; @ManyToOne(fetch = FetchType.LAZY) private PullRequest pullRequest; - @Column(name = "date") - private Date date; - @JsonProperty("commit") - private void unpackNameFromNestedObject(Map> owner) { - this.date = Date.from(Instant.parse(owner.get("commiter").get("date"))); + private void unpackDateFromNestedObject(Map commit) { + // this.date = Date.from(Instant.parse(commiter.get("date").toString())); } } diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java b/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java index 94bd2d9e..e0a0b3dc 100644 --- a/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java +++ b/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java @@ -11,8 +11,9 @@ public class CommitAggregation { public static CommitAggregation aggregate(List commits) { CommitAggregation commitAggregation = new CommitAggregation(); commitAggregation.numCommits = (int) commits.stream().distinct().count(); - commitAggregation.linesAdded = commits.stream().mapToInt(Commit::getAdditions).sum(); - commitAggregation.linesRemoved = commits.stream().mapToInt(Commit::getDeletions).sum(); + //TODO: This should come from the PR, not the commit. +// commitAggregation.linesAdded = commits.stream().mapToInt(Commit::getAdditions).sum(); +// commitAggregation.linesRemoved = commits.stream().mapToInt(Commit::getDeletions).sum(); return commitAggregation; } diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java index 8f671894..a79a3f20 100644 --- a/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java +++ b/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java @@ -7,12 +7,13 @@ import io.pakland.mdas.githubstats.domain.repository.*; import io.pakland.mdas.githubstats.infrastructure.github.repository.*; import io.pakland.mdas.githubstats.infrastructure.shell.model.UserOptionRequest; -import java.util.List; import lombok.NoArgsConstructor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Component; +import java.util.List; + @Component @NoArgsConstructor public class UserOptionController { @@ -43,30 +44,30 @@ public void execute() { // TODO: If the execution succeeds, we should make an entry to the historic_queries table. // Fetch the API key's available organizations. List organizationList = new FetchAvailableOrganizations( - this.organizationExternalRepository) - .execute(); + this.organizationExternalRepository) + .execute(); // Start building the github-stats relational schema. for (Organization organization : organizationList) { // Fetch the teams belonging to the available organization. List teamList = new FetchTeamsFromOrganization(teamExternalRepository) - .execute(organization.getLogin()); + .execute(organization.getLogin()); for (Team team : teamList) { // Fetch the members of each team. List userList = new FetchUsersFromTeam(userExternalRepository) - .execute(organization.getLogin(), team.getSlug()); + .execute(organization.getLogin(), team.getSlug()); // Fetch the repositories for each team. List repositoryList = new FetchRepositoriesFromTeam( - repositoryExternalRepository) - .execute(organization.getLogin(), team.getSlug()); + repositoryExternalRepository) + .execute(organization.getLogin(), team.getSlug()); // Add the team to the repository repositoryList.forEach(r -> r.setTeam(team)); for (Repository repository : repositoryList) { // Fetch pull requests from each team. List pullRequestList = new FetchPullRequestsFromRepository( - pullRequestExternalRepository) - .execute(repository.getOwnerLogin(), repository.getName()); + pullRequestExternalRepository) + .execute(repository.getOwnerLogin(), repository.getName()); for (PullRequest pullRequest : pullRequestList) { // Add the repository to the pull request @@ -75,14 +76,13 @@ public void execute() { TODO: if the user of the PR belongs to the team, increment the prs executed inside the team, TODO: else increment the prs executed outside the team. */ - // TODO: Save for later calculate the Additions from PR aggregation. - // TODO: Save for later calculate the Deletions from PR aggregation. - // TODO: Save for later calculate the commit number from PR aggregation. + // TODO: Save for later calculate the Additions, Deletionjs and commit num. from PR aggregation. List commitList = new FetchCommitsFromPullRequest(commitExternalRepository) .execute(repository.getOwnerLogin(), repository.getName(), pullRequest.getNumber()); logger.info(String.valueOf(commitList.size())); for (Commit commit : commitList) { // TODO: Fetch PR reviews. + } } diff --git a/src/main/resources/db/changelog/changes/000_initial_schema.yaml b/src/main/resources/db/changelog/changes/000_initial_schema.yaml index 2de880b2..21dae8f8 100644 --- a/src/main/resources/db/changelog/changes/000_initial_schema.yaml +++ b/src/main/resources/db/changelog/changes/000_initial_schema.yaml @@ -38,7 +38,7 @@ databaseChangeLog: type: VARCHAR(255) - column: name: date - type: TIMESTAMP WITHOUT TIME ZONE + type: TIMESTAMP - column: name: pull_request_id type: INT diff --git a/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java b/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java index f5dd228c..d72472d8 100644 --- a/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java +++ b/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java @@ -10,7 +10,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class AggregateCommitsTest { @@ -30,38 +29,40 @@ public void aggregatingCommits_shouldGiveValidNumCommits() { @Test public void aggregatingCommits_shouldGiveValidLinesAdded() { - List commits = new ArrayList<>(); - - int totalLines = 0; - - for (int i = 0; i < 10; i++) { - int numLines = new Random().nextInt(1000); - Commit commit = mock(Commit.class); - when(commit.getAdditions()).thenReturn(numLines); - commits.add(commit); - totalLines += numLines; - } - - CommitAggregation commitAggregation = CommitAggregation.aggregate(commits); - assertEquals(commitAggregation.getLinesAdded(), totalLines); + //TODO: Additions should come form PullRequests. +// List commits = new ArrayList<>(); +// +// int totalLines = 0; +// +// for (int i = 0; i < 10; i++) { +// int numLines = new Random().nextInt(1000); +// Commit commit = mock(Commit.class); +// when(commit.getAdditions()).thenReturn(numLines); +// commits.add(commit); +// totalLines += numLines; +// } +// +// CommitAggregation commitAggregation = CommitAggregation.aggregate(commits); +// assertEquals(commitAggregation.getLinesAdded(), totalLines); } @Test public void aggregatingCommits_shouldGiveValidLinesRemoved() { - List commits = new ArrayList<>(); - - int totalLines = 0; - - for (int i = 0; i < 10; i++) { - int numLines = new Random().nextInt(1000); - Commit commit = mock(Commit.class); - when(commit.getDeletions()).thenReturn(numLines); - commits.add(commit); - totalLines += numLines; - } - - CommitAggregation commitAggregation = CommitAggregation.aggregate(commits); - assertEquals(commitAggregation.getLinesRemoved(), totalLines); + //TODO: Additions should come form PullRequests. +// List commits = new ArrayList<>(); +// +// int totalLines = 0; +// +// for (int i = 0; i < 10; i++) { +// int numLines = new Random().nextInt(1000); +// Commit commit = mock(Commit.class); +// when(commit.getDeletions()).thenReturn(numLines); +// commits.add(commit); +// totalLines += numLines; +// } +// +// CommitAggregation commitAggregation = CommitAggregation.aggregate(commits); +// assertEquals(commitAggregation.getLinesRemoved(), totalLines); } } diff --git a/src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java b/src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java index 56965a6b..831da5c4 100644 --- a/src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java +++ b/src/test/java/io/pakland/mdas/githubstats/application/FetchCommitsFromPullRequestTest.java @@ -2,83 +2,100 @@ import io.pakland.mdas.githubstats.application.exceptions.HttpException; import io.pakland.mdas.githubstats.domain.Commit; -import io.pakland.mdas.githubstats.domain.PullRequestState; -import io.pakland.mdas.githubstats.domain.repository.PullRequestExternalRepository.FetchPullRequestFromRepositoryRequest; +import io.pakland.mdas.githubstats.domain.repository.CommitExternalRepository; +import io.pakland.mdas.githubstats.domain.repository.CommitExternalRepository.FetchCommitsFromPullRequestRequest; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; +import java.util.List; + +import static org.junit.Assert.assertThrows; import static org.junit.jupiter.api.Assertions.assertEquals; public class FetchCommitsFromPullRequestTest { - private void validateRequestCaptor( - ArgumentCaptor captor) { - assertEquals("github-stats-22", captor.getValue().getRepositoryOwner()); - assertEquals("github-stats", captor.getValue().getRepository()); - assertEquals(1, captor.getValue().getPage()); - assertEquals(100, captor.getValue().getPerPage()); - assertEquals(PullRequestState.ALL, captor.getValue().getState()); - } - @Test public void whenValidOrganizationAndRepository_shouldReturnTheListOfPullRequests() throws HttpException { Commit commit = new Commit(); - commit.setId(1157910685); - prOne.setNumber(71); - PullRequest prTwo = new PullRequest(); - prTwo.setId(1157867973); - prTwo.setNumber(69); + commit.setSha("a0b3ed9d5f1356575f2b16ab8ef5d93c5ce77575"); + commit.setDate(new Date()); + Commit commit1 = new Commit(); + commit1.setSha("f16b593d35d6e66dc7e1c8727d4eaa829d3973ed"); + commit1.setDate(new Date()); - PullRequestExternalRepository repository = Mockito.mock( - PullRequestExternalRepository.class); - Mockito.when(repository.fetchPullRequestsFromRepository(Mockito.any( - FetchPullRequestFromRepositoryRequest.class))).thenReturn(List.of(prOne, prTwo)); + CommitExternalRepository repository = Mockito.mock( + CommitExternalRepository.class); + FetchCommitsFromPullRequestRequest fetchCommitsFromPullRequestRequest = FetchCommitsFromPullRequestRequest.builder() + .pullRequestNumber(1) + .repositoryName("github-stats") + .repositoryOwner("github-stats-22") + .page(1) + .perPage(100) + .build(); + FetchCommitsFromPullRequestRequest fetchCommitsFromPullRequestRequest1 = FetchCommitsFromPullRequestRequest.builder() + .pullRequestNumber(1) + .repositoryName("github-stats") + .repositoryOwner("github-stats-22") + .page(2) + .perPage(100) + .build(); - List response = new FetchPullRequestsFromRepository(repository).execute( - "github-stats-22", "github-stats" - ); + Mockito.when(repository.fetchCommitsFromPullRequest(Mockito.any( + FetchCommitsFromPullRequestRequest.class))).thenReturn(new ArrayList<>(Arrays.asList(commit, commit1))).thenReturn(new ArrayList<>()); - ArgumentCaptor captor = ArgumentCaptor.forClass( - FetchPullRequestFromRepositoryRequest.class); - Mockito.verify(repository).fetchPullRequestsFromRepository(captor.capture()); + List response = new FetchCommitsFromPullRequest(repository).execute( + "github-stats-22", "github-stats", 1 + ); - validateRequestCaptor(captor); + ArgumentCaptor captor = ArgumentCaptor.forClass( + FetchCommitsFromPullRequestRequest.class); + Mockito.verify(repository, Mockito.times(2)).fetchCommitsFromPullRequest(captor.capture()); + assertEquals("github-stats-22", captor.getValue().getRepositoryOwner()); + assertEquals("github-stats", captor.getValue().getRepositoryName()); + assertEquals(2, captor.getValue().getPage()); + assertEquals(100, captor.getValue().getPerPage()); assertEquals(2, response.size()); - assertEquals(prOne.getId(), response.get(0).getId()); - assertEquals(prTwo.getId(), response.get(1).getId()); + assertEquals(commit.getSha(), response.get(0).getSha()); + assertEquals(commit1.getSha(), response.get(1).getSha()); } @Test public void whenRepositoryReturnsEmptyResponse_shouldReturnEmptyList() throws HttpException { -// PullRequestExternalRepository repository = Mockito.mock( -// PullRequestExternalRepository.class); -// Mockito.when(repository.fetchPullRequestsFromRepository(Mockito.any( -// FetchPullRequestFromRepositoryRequest.class))).thenReturn(new ArrayList<>()); -// -// List response = new FetchPullRequestsFromRepository(repository).execute( -// "github-stats-22", "github-stats" -// ); -// -// ArgumentCaptor captor = ArgumentCaptor.forClass( -// FetchPullRequestFromRepositoryRequest.class); -// Mockito.verify(repository).fetchPullRequestsFromRepository(captor.capture()); -// -// validateRequestCaptor(captor); -// assertEquals(0, response.size()); + CommitExternalRepository repository = Mockito.mock( + CommitExternalRepository.class); + Mockito.when(repository.fetchCommitsFromPullRequest(Mockito.any( + FetchCommitsFromPullRequestRequest.class))).thenReturn(new ArrayList<>()); + + List response = new FetchCommitsFromPullRequest(repository).execute( + "github-stats-22", "github-stats", 1 + ); + + ArgumentCaptor captor = ArgumentCaptor.forClass( + FetchCommitsFromPullRequestRequest.class); + Mockito.verify(repository).fetchCommitsFromPullRequest(captor.capture()); + + assertEquals("github-stats-22", captor.getValue().getRepositoryOwner()); + assertEquals("github-stats", captor.getValue().getRepositoryName()); + assertEquals(1, captor.getValue().getPage()); + assertEquals(100, captor.getValue().getPerPage()); + assertEquals(0, response.size()); } @Test public void whenRepositoryThrowsException_shouldThrowHttpException() throws HttpException { -// PullRequestExternalRepository repository = Mockito.mock( -// PullRequestExternalRepository.class); -// Mockito.when(repository.fetchPullRequestsFromRepository(Mockito.any( -// FetchPullRequestFromRepositoryRequest.class))) -// .thenThrow(new HttpException(404, "Page not found.")); -// -// assertThrows(HttpException.class, () -> { -// new FetchPullRequestsFromRepository(repository).execute("github-stats-22", -// "github-stats"); -// }); + CommitExternalRepository repository = Mockito.mock( + CommitExternalRepository.class); + Mockito.when(repository.fetchCommitsFromPullRequest(Mockito.any( + FetchCommitsFromPullRequestRequest.class))).thenThrow(new HttpException(404, "Page not found.")); + + assertThrows(HttpException.class, () -> { + new FetchCommitsFromPullRequest(repository).execute("github-stats-22", + "github-stats", 1); + }); } } From bec32f5134dfc278d4d1ab86e695a0315f24e2d9 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Wed, 14 Dec 2022 18:19:37 +0100 Subject: [PATCH 3/4] fix: remove unnecessary comments Removed the comments that were made regarding the `CommitAggregation` entity as it is something that should be done in a different way and it was just keeping outdated code commented. --- .../githubstats/domain/CommitAggregation.java | 14 +------ .../application/AggregateCommitsTest.java | 38 ------------------- 2 files changed, 1 insertion(+), 51 deletions(-) diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java b/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java index e0a0b3dc..dcb5c038 100644 --- a/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java +++ b/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java @@ -5,27 +5,15 @@ public class CommitAggregation { private int numCommits; - private int linesAdded; - private int linesRemoved; public static CommitAggregation aggregate(List commits) { CommitAggregation commitAggregation = new CommitAggregation(); commitAggregation.numCommits = (int) commits.stream().distinct().count(); - //TODO: This should come from the PR, not the commit. -// commitAggregation.linesAdded = commits.stream().mapToInt(Commit::getAdditions).sum(); -// commitAggregation.linesRemoved = commits.stream().mapToInt(Commit::getDeletions).sum(); return commitAggregation; } public int getNumCommits() { return numCommits; } - - public int getLinesAdded() { - return linesAdded; - } - - public int getLinesRemoved() { - return linesRemoved; - } + } diff --git a/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java b/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java index d72472d8..5580c85c 100644 --- a/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java +++ b/src/test/java/io/pakland/mdas/githubstats/application/AggregateCommitsTest.java @@ -27,42 +27,4 @@ public void aggregatingCommits_shouldGiveValidNumCommits() { assertEquals(commitAggregation.getNumCommits(), numCommits); } - @Test - public void aggregatingCommits_shouldGiveValidLinesAdded() { - //TODO: Additions should come form PullRequests. -// List commits = new ArrayList<>(); -// -// int totalLines = 0; -// -// for (int i = 0; i < 10; i++) { -// int numLines = new Random().nextInt(1000); -// Commit commit = mock(Commit.class); -// when(commit.getAdditions()).thenReturn(numLines); -// commits.add(commit); -// totalLines += numLines; -// } -// -// CommitAggregation commitAggregation = CommitAggregation.aggregate(commits); -// assertEquals(commitAggregation.getLinesAdded(), totalLines); - } - - @Test - public void aggregatingCommits_shouldGiveValidLinesRemoved() { - //TODO: Additions should come form PullRequests. -// List commits = new ArrayList<>(); -// -// int totalLines = 0; -// -// for (int i = 0; i < 10; i++) { -// int numLines = new Random().nextInt(1000); -// Commit commit = mock(Commit.class); -// when(commit.getDeletions()).thenReturn(numLines); -// commits.add(commit); -// totalLines += numLines; -// } -// -// CommitAggregation commitAggregation = CommitAggregation.aggregate(commits); -// assertEquals(commitAggregation.getLinesRemoved(), totalLines); - } - } From 1d1f8cb51d7648d9935446c6972dff444425c6a7 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Wed, 14 Dec 2022 18:29:37 +0100 Subject: [PATCH 4/4] fix: parsing nested date from commit response --- .../java/io/pakland/mdas/githubstats/domain/Commit.java | 6 ++++-- .../pakland/mdas/githubstats/domain/CommitAggregation.java | 2 +- .../shell/controller/UserOptionController.java | 3 +-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java b/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java index 52f9d684..14cc1298 100644 --- a/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java +++ b/src/main/java/io/pakland/mdas/githubstats/domain/Commit.java @@ -1,6 +1,7 @@ package io.pakland.mdas.githubstats.domain; import com.fasterxml.jackson.annotation.JsonProperty; +import java.time.Instant; import lombok.Data; import lombok.NoArgsConstructor; import lombok.ToString; @@ -31,7 +32,8 @@ public class Commit { private PullRequest pullRequest; @JsonProperty("commit") - private void unpackDateFromNestedObject(Map commit) { - // this.date = Date.from(Instant.parse(commiter.get("date").toString())); + private void unpackDateFromNestedObject(Map commitJson) { + Map committer = (Map)commitJson.get("committer"); + this.date = Date.from(Instant.parse(committer.get("date").toString())); } } diff --git a/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java b/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java index dcb5c038..54c6cc97 100644 --- a/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java +++ b/src/main/java/io/pakland/mdas/githubstats/domain/CommitAggregation.java @@ -15,5 +15,5 @@ public static CommitAggregation aggregate(List commits) { public int getNumCommits() { return numCommits; } - + } diff --git a/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java b/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java index a79a3f20..e6c2b447 100644 --- a/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java +++ b/src/main/java/io/pakland/mdas/githubstats/infrastructure/shell/controller/UserOptionController.java @@ -76,10 +76,9 @@ public void execute() { TODO: if the user of the PR belongs to the team, increment the prs executed inside the team, TODO: else increment the prs executed outside the team. */ - // TODO: Save for later calculate the Additions, Deletionjs and commit num. from PR aggregation. + // TODO: Save for later calculate the additions, deletions and commit num. from PR aggregation. List commitList = new FetchCommitsFromPullRequest(commitExternalRepository) .execute(repository.getOwnerLogin(), repository.getName(), pullRequest.getNumber()); - logger.info(String.valueOf(commitList.size())); for (Commit commit : commitList) { // TODO: Fetch PR reviews.