From a6678bc1d3ff9e9bc0a564582f35b208310bb661 Mon Sep 17 00:00:00 2001 From: Florian Ehrenstorfer Date: Mon, 27 Jan 2025 16:03:40 +0100 Subject: [PATCH] create rule based detection of bad practices(#226) --- .../hephaestus/activity/ActivityService.java | 6 +- .../PullRequestBadPracticeRuleRepository.java | 19 ++++ .../PullRequestBadPracticeDetector.java | 95 ++++++++++++++++++- .../activity/model/BadPracticeType.java | 28 ++++++ .../model/PullRequestBadPracticeRule.java | 0 5 files changed, 141 insertions(+), 7 deletions(-) create mode 100644 server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/PullRequestBadPracticeRuleRepository.java create mode 100644 server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/BadPracticeType.java create mode 100644 server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/PullRequestBadPracticeRule.java diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/ActivityService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/ActivityService.java index dd7cf9f4..41ebe795 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/ActivityService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/ActivityService.java @@ -85,10 +85,10 @@ public List detectBadPractices(String login) { Set.of(Issue.State.OPEN) ); - return pullRequests + List badPractices = pullRequestBadPracticeDetector.detectAndSyncBadPractices(pullRequests); + + return badPractices .stream() - .map(pullRequestBadPracticeDetector::detectAndSyncBadPractices) - .flatMap(List::stream) .map(PullRequestBadPracticeDTO::fromPullRequestBadPractice) .collect(Collectors.toList()); } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/PullRequestBadPracticeRuleRepository.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/PullRequestBadPracticeRuleRepository.java new file mode 100644 index 00000000..c1bf4dd0 --- /dev/null +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/PullRequestBadPracticeRuleRepository.java @@ -0,0 +1,19 @@ +package de.tum.in.www1.hephaestus.activity; + +import de.tum.in.www1.hephaestus.activity.model.PullRequestBadPracticeRule; +import java.util.List; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.stereotype.Repository; + +@Repository +public interface PullRequestBadPracticeRuleRepository extends JpaRepository { + @Query( + """ + SELECT prbp + FROM PullRequestBadPracticeRule prbp + WHERE prbp.active = true + """ + ) + List findAllActive(); +} diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java index 385d9ecf..2d86ead6 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java @@ -1,9 +1,22 @@ package de.tum.in.www1.hephaestus.activity.badpracticedetector; import de.tum.in.www1.hephaestus.activity.PullRequestBadPracticeRepository; +import de.tum.in.www1.hephaestus.activity.PullRequestBadPracticeRuleRepository; +import de.tum.in.www1.hephaestus.activity.model.BadPracticeType; import de.tum.in.www1.hephaestus.activity.model.PullRequestBadPractice; +import de.tum.in.www1.hephaestus.activity.model.PullRequestBadPracticeRule; import de.tum.in.www1.hephaestus.gitprovider.pullrequest.PullRequest; + +import java.util.LinkedList; import java.util.List; +import java.util.stream.Collectors; + +import de.tum.in.www1.hephaestus.gitprovider.pullrequest.PullRequestRepository; +import de.tum.in.www1.hephaestus.intelligenceservice.api.DetectorApi; +import de.tum.in.www1.hephaestus.intelligenceservice.model.DetectorRequest; +import de.tum.in.www1.hephaestus.intelligenceservice.model.DetectorResponse; +import de.tum.in.www1.hephaestus.intelligenceservice.model.PullRequestWithBadPractices; +import de.tum.in.www1.hephaestus.intelligenceservice.model.Rule; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -14,16 +27,90 @@ public class PullRequestBadPracticeDetector { private static final Logger logger = LoggerFactory.getLogger(PullRequestBadPracticeDetector.class); + @Autowired + private PullRequestRepository pullRequestRepository; + @Autowired private PullRequestBadPracticeRepository pullRequestBadPracticeRepository; - public List detectAndSyncBadPractices(PullRequest pullRequest) { - logger.info("Detecting bad practices for pull request: {}.", pullRequest.getId()); + @Autowired + private PullRequestBadPracticeRuleRepository pullRequestBadPracticeRuleRepository; + + private final DetectorApi detectorApi = new DetectorApi(); + + public List detectAndSyncBadPractices(List pullRequests) { + logger.info("Detecting bad practices for pull requests."); + + List rules = pullRequestBadPracticeRuleRepository.findAllActive(); + + DetectorRequest detectorRequest = new DetectorRequest(); + detectorRequest.setPullRequests(mapToApiPullRequests(pullRequests)); + detectorRequest.setRules(mapToApiRules(rules)); + DetectorResponse detectorResponse = detectorApi.detectDetectorPost(detectorRequest); + + List detectedBadPractices = new LinkedList<>(); + + detectorResponse.getDetectBadPractices().forEach(prWithBadPractices -> + detectedBadPractices.addAll(handleDetectedBadPractices(prWithBadPractices)) + ); + + return detectedBadPractices; + } + + private List handleDetectedBadPractices(PullRequestWithBadPractices prWithBadPractices) { + + Long pullRequestId = Long.valueOf(prWithBadPractices.getPullRequestId()); + PullRequest pullRequest = pullRequestRepository.findById(pullRequestId).orElse(null); + + if (pullRequest == null) { + logger.error("Pull request with id {} not found.", prWithBadPractices.getPullRequestId()); + return List.of(); + } List existingBadPractices = pullRequestBadPracticeRepository.findByPullRequestId( - pullRequest.getId() + pullRequest.getId() ); - return existingBadPractices; + List newBadPractices = prWithBadPractices.getBadPracticeIds().stream() + .map(badPracticeRule -> { + PullRequestBadPractice pullRequestBadPractice = new PullRequestBadPractice(); + pullRequestBadPractice.setPullrequest(pullRequest); + pullRequestBadPractice.setType(findBadPracticeType(badPracticeRule)); + pullRequestBadPractice.setResolved(false); + return pullRequestBadPractice; + }) + .collect(Collectors.toList()); + + pullRequestBadPracticeRepository.saveAll(newBadPractices); + existingBadPractices.removeAll(newBadPractices); + existingBadPractices.forEach(badPractice -> badPractice.setResolved(true)); + pullRequestBadPracticeRepository.saveAll(existingBadPractices); + return newBadPractices; + } + + private BadPracticeType findBadPracticeType(String badPracticeRuleId) { + return pullRequestBadPracticeRuleRepository.findById(Long.valueOf(badPracticeRuleId)) + .map(PullRequestBadPracticeRule::getType) + .orElse(null); + } + + private List mapToApiPullRequests(List pullRequests) { + return pullRequests.stream().map(pullRequest -> { + de.tum.in.www1.hephaestus.intelligenceservice.model.PullRequest apiPullRequest = new de.tum.in.www1.hephaestus.intelligenceservice.model.PullRequest(); + apiPullRequest.setId(String.valueOf(pullRequest.getId())); + apiPullRequest.setTitle(pullRequest.getTitle()); + apiPullRequest.setDescription(pullRequest.getBody()); + return apiPullRequest; + }).collect(Collectors.toList()); + } + + private List mapToApiRules(List rules) { + return rules.stream().map(rule -> { + Rule apiRule = new Rule(); + apiRule.setBadPracticeId(String.valueOf(rule.getId())); + apiRule.setName(rule.getType().getTitle()); + apiRule.setDescription(rule.getType().getLlmPrompt()); + return apiRule; + }).collect(Collectors.toList()); } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/BadPracticeType.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/BadPracticeType.java new file mode 100644 index 00000000..45df85eb --- /dev/null +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/BadPracticeType.java @@ -0,0 +1,28 @@ +package de.tum.in.www1.hephaestus.activity.model; + +import lombok.Getter; + +@Getter +public enum BadPracticeType { + + UncheckedCheckbox( + "Unchecked Checkbox", + "This pull request contains an unchecked checkbox.", + "The pull request contains an unchecked checkbox. An unchecked checkbox looks like this: '- [ ]'." + ), + EmptySection( + "Empty Section", + "This pull request contains an empty section.", + "The pull request contains an empty section. An empty section is a section that does not contain any content. An empty section is directly followed by another section of the same or bigger title size." + ); + + private final String title; + private final String description; + private final String llmPrompt; + + BadPracticeType(String title, String description, String llmPrompt) { + this.title = title; + this.description = description; + this.llmPrompt = llmPrompt; + } +} diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/PullRequestBadPracticeRule.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/PullRequestBadPracticeRule.java new file mode 100644 index 00000000..e69de29b