Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Caching and UPDATED-sorting for the Github Data Sync Algorithm #94

Merged
merged 5 commits into from
Sep 18, 2024
Merged

Caching and UPDATED-sorting for the Github Data Sync Algorithm #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c5b1e00
Update Github storing algorithm
GODrums Sep 18, 2024
6da0d83
Improve loop performance
GODrums Sep 18, 2024
2707cc7
Add PR Number
GODrums Sep 18, 2024
f8d3792
Respond with reviews instead of counter
GODrums Sep 18, 2024
ee4d935
fix leaderboard story
FelixTJDietrich Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions server/application-server/openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ components:
updatedAt:
type: string
format: date-time
number:
type: integer
format: int32
title:
type: string
url:
Expand Down Expand Up @@ -450,11 +453,36 @@ components:
type: integer
format: int32
changesRequested:
type: integer
format: int32
type: array
items:
$ref: "#/components/schemas/PullRequestReviewDTO"
approvals:
type: integer
format: int32
type: array
items:
$ref: "#/components/schemas/PullRequestReviewDTO"
comments:
type: array
items:
$ref: "#/components/schemas/PullRequestReviewDTO"
PullRequestReviewDTO:
type: object
properties:
id:
type: integer
format: int32
format: int64
createdAt:
type: string
format: date-time
updatedAt:
type: string
format: date-time
submittedAt:
type: string
format: date-time
state:
type: string
enum:
- COMMENTED
- APPROVED
- CHANGES_REQUESTED
- DISMISSED
3 changes: 3 additions & 0 deletions ...on-server/src/main/java/de/tum/in/www1/hephaestus/codereview/pullrequest/PullRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
@NoArgsConstructor
@ToString(callSuper = true)
public class PullRequest extends BaseGitServiceEntity {

private int number;

@NonNull
private String title;

Expand Down
1 change: 1 addition & 0 deletions .../src/main/java/de/tum/in/www1/hephaestus/codereview/pullrequest/PullRequestConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public PullRequest convert(@NonNull GHPullRequest source) {
IssueState state = convertState(source.getState());
PullRequest pullRequest = new PullRequest();
convertBaseFields(source, pullRequest);
pullRequest.setNumber(source.getNumber());
pullRequest.setTitle(source.getTitle());
pullRequest.setUrl(source.getHtmlUrl().toString());
pullRequest.setState(state);
Expand Down
10 changes: 10 additions & 0 deletions ...in/java/de/tum/in/www1/hephaestus/codereview/pullrequest/review/PullRequestReviewDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package de.tum.in.www1.hephaestus.codereview.pullrequest.review;

import java.time.OffsetDateTime;

import com.fasterxml.jackson.annotation.JsonInclude;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record PullRequestReviewDTO(Long id, OffsetDateTime createdAt, OffsetDateTime updatedAt,
OffsetDateTime submittedAt, PullRequestReviewState state) {
}
8 changes: 5 additions & 3 deletions ...lication-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package de.tum.in.www1.hephaestus.leaderboard;

import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.in.www1.hephaestus.codereview.pullrequest.review.PullRequestReviewDTO;
import de.tum.in.www1.hephaestus.codereview.user.UserType;
import lombok.AllArgsConstructor;
import lombok.Getter;
Expand All @@ -19,7 +21,7 @@ public class LeaderboardEntry {
private UserType type;
private int score;
private int rank;
private int changesRequested;
private int approvals;
private int comments;
private PullRequestReviewDTO[] changesRequested;
private PullRequestReviewDTO[] approvals;
private PullRequestReviewDTO[] comments;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens with the dismissed reviews?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting point! They were counted as comments for now. This should be fixed in: #97.

In the future we might want to do some sort of state replacement similar to the original Python script.

}
61 changes: 43 additions & 18 deletions ...cation-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
package de.tum.in.www1.hephaestus.leaderboard;

import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;

import de.tum.in.www1.hephaestus.codereview.pullrequest.PullRequest;
import de.tum.in.www1.hephaestus.codereview.pullrequest.review.PullRequestReviewDTO;
import de.tum.in.www1.hephaestus.codereview.user.User;
import de.tum.in.www1.hephaestus.codereview.user.UserService;
import de.tum.in.www1.hephaestus.codereview.user.UserType;
Expand All @@ -22,6 +29,9 @@ public class LeaderboardService {

private final UserService userService;

@Value("${monitoring.timeframe}")
private int timeframe;

public LeaderboardService(UserService userService) {
this.userService = userService;
}
Expand All @@ -32,32 +42,47 @@ public List<LeaderboardEntry> createLeaderboard() {
List<User> users = userService.getAllUsers();
logger.info("Found " + users.size() + " users");

OffsetDateTime cutOffTime = new Date(System.currentTimeMillis() - 1000 * 60 * 60 * 24 * timeframe)
.toInstant().atOffset(ZoneOffset.UTC);

List<LeaderboardEntry> leaderboard = users.stream().map(user -> {
if (user.getType() != UserType.USER) {
return null;
}
int comments = user.getIssueComments().size();
AtomicInteger changesRequested = new AtomicInteger(0);
AtomicInteger changesApproved = new AtomicInteger(0);
logger.info("User: " + user.getLogin());
AtomicInteger score = new AtomicInteger(0);
user.getReviews().stream().forEach(review -> {
switch (review.getState()) {
case CHANGES_REQUESTED:
changesRequested.incrementAndGet();
break;
case APPROVED:
changesApproved.incrementAndGet();
break;
default:
break;
}
score.addAndGet(calculateScore(review.getPullRequest()));
});
Set<PullRequestReviewDTO> changesRequestedSet = new HashSet<>();
Set<PullRequestReviewDTO> approvedSet = new HashSet<>();
Set<PullRequestReviewDTO> commentSet = new HashSet<>();

user.getReviews().stream()
.filter(review -> (review.getCreatedAt() != null && review.getCreatedAt().isAfter(cutOffTime))
|| (review.getUpdatedAt() != null && review.getUpdatedAt().isAfter(cutOffTime)))
.forEach(review -> {
if (review.getPullRequest().getAuthor().getLogin().equals(user.getLogin())) {
return;
}
PullRequestReviewDTO reviewDTO = new PullRequestReviewDTO(review.getId(), review.getCreatedAt(),
review.getUpdatedAt(), review.getSubmittedAt(), review.getState());
switch (review.getState()) {
case CHANGES_REQUESTED:
changesRequestedSet.add(reviewDTO);
break;
case APPROVED:
approvedSet.add(reviewDTO);
break;
default:
commentSet.add(reviewDTO);
break;
}
score.addAndGet(calculateScore(review.getPullRequest()));
});
return new LeaderboardEntry(user.getLogin(), user.getAvatarUrl(), user.getName(), user.getType(),
score.get(),
0, // preliminary rank
changesRequested.get(),
changesApproved.get(), comments);
changesRequestedSet.toArray(new PullRequestReviewDTO[changesRequestedSet.size()]),
approvedSet.toArray(new PullRequestReviewDTO[approvedSet.size()]),
commentSet.toArray(new PullRequestReviewDTO[commentSet.size()]));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everything is fine for now, in the future we might want store the score for each activity also in the database and then we can just calculate rank and aggregated score more easily.

For example, a pull request review comes in via webhooks, we estimate the effort it took to do this review and store it in the database. I think as it is now the score is not stable since the PR might get more changes and reviews get dismissed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is very true.
We for sure have to improve a few elements for a correct and performant long-running execution.

}).filter(Objects::nonNull).collect(Collectors.toCollection(ArrayList::new));

// update ranks by score
Expand Down
Loading