From c44618e1c3fa67ff184cd9d6d84e7fee0e75ac98 Mon Sep 17 00:00:00 2001 From: m-rudyk <121865672+m-rudyk@users.noreply.github.com> Date: Thu, 15 Feb 2024 20:40:54 +0200 Subject: [PATCH] fix: remove PR if max attempt reached (#430) * fix: remove PR if max attempt reached Signed-off-by: Mykola Rudyk * test: update tests to keep coverage Signed-off-by: Mykola Rudyk * fix: add changes after review comment, remove unnecessary check Signed-off-by: Mykola Rudyk --------- Signed-off-by: Mykola Rudyk --- .../com/lpvs/service/LPVSQueueService.java | 73 ++++++++----------- src/main/resources/application.properties | 3 + .../lpvs/service/LPVSQueueServiceTest.java | 25 ++----- 3 files changed, 43 insertions(+), 58 deletions(-) diff --git a/src/main/java/com/lpvs/service/LPVSQueueService.java b/src/main/java/com/lpvs/service/LPVSQueueService.java index 977cf8a2..1a834b54 100644 --- a/src/main/java/com/lpvs/service/LPVSQueueService.java +++ b/src/main/java/com/lpvs/service/LPVSQueueService.java @@ -18,6 +18,7 @@ import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; +import java.io.IOException; import java.util.*; import java.util.concurrent.BlockingDeque; import java.util.concurrent.LinkedBlockingDeque; @@ -146,40 +147,7 @@ public BlockingDeque getQueue() { public void checkForQueue() throws InterruptedException { log.debug("Checking for previous queue"); List webhookConfigList = queueRepository.getQueueList(); - if (webhookConfigList.size() > 0) { - LPVSQueue webhookConfig = getLatestScan(webhookConfigList); - webhookConfig.setAttempts(webhookConfig.getAttempts() + 1); - queueRepository.save(webhookConfig); - } for (LPVSQueue webhook : webhookConfigList) { - log.info("PROCESSING WebHook id = " + webhook.getId()); - if (webhook.getAttempts() > maxAttempts) { - LPVSPullRequest pullRequest = new LPVSPullRequest(); - pullRequest.setPullRequestUrl(webhook.getPullRequestUrl()); - pullRequest.setUser(webhook.getUserId()); - pullRequest.setPullRequestFilesUrl(webhook.getPullRequestFilesUrl()); - pullRequest.setRepositoryName( - LPVSWebhookUtil.getRepositoryOrganization(webhook) - + "/" - + LPVSWebhookUtil.getRepositoryName(webhook)); - pullRequest.setDate(webhook.getDate()); - pullRequest.setStatus(LPVSPullRequestStatus.NO_ACCESS.toString()); - pullRequest.setPullRequestHead(webhook.getPullRequestHead()); - pullRequest.setPullRequestBase(webhook.getPullRequestBase()); - pullRequest.setSender(webhook.getSender()); - pullRequest = lpvsPullRequestRepository.saveAndFlush(pullRequest); - - if (webhook.getUserId().equals("GitHub hook")) { - gitHubService.setErrorCheck(webhook); - } - queueRepository.deleteById(webhook.getId()); - log.info( - "Webhook id = " - + webhook.getId() - + " is deleted. Details on PR #" - + pullRequest.getId()); - continue; - } log.info("Add WebHook id = " + webhook.getId() + " to the queue."); QUEUE.putFirst(webhook); } @@ -210,8 +178,13 @@ public LPVSQueue getLatestScan(List webhookConfigList) { public void processWebHook(LPVSQueue webhookConfig) { LPVSPullRequest pullRequest = new LPVSPullRequest(); try { - log.info("GitHub queue processing..."); - log.debug(webhookConfig.toString()); + log.info( + "Processing webhook ID: " + + webhookConfig.getId() + + ", attempt: " + + webhookConfig.getAttempts() + + " for PR: " + + webhookConfig.getPullRequestUrl()); String filePath = gitHubService.getPullRequestFiles(webhookConfig); @@ -258,20 +231,38 @@ public void processWebHook(LPVSQueue webhookConfig) { log.debug("Creating comment"); gitHubService.commentResults(webhookConfig, files, detectedConflicts, pullRequest); log.debug("Results posted on GitHub"); + delete(webhookConfig); } else { log.warn("Files are not found. Probably pull request is not exists."); - gitHubService.commentResults(webhookConfig, null, null, pullRequest); - delete(webhookConfig); throw new Exception( "Files are not found. Probably pull request does not exist. Terminating."); } - delete(webhookConfig); } catch (Exception | Error e) { pullRequest.setStatus(LPVSPullRequestStatus.INTERNAL_ERROR.toString()); pullRequest = lpvsPullRequestRepository.saveAndFlush(pullRequest); - log.error("Can't authorize commentResults() " + e); - e.printStackTrace(); - delete(webhookConfig); + log.error("Can't authorize commentResults() " + e.getMessage()); + int currentAttempts = webhookConfig.getAttempts(); + if (currentAttempts < maxAttempts) { + webhookConfig.setAttempts(currentAttempts + 1); + try { + addFirst(webhookConfig); + } catch (InterruptedException e1) { + log.warn("Failed to update Queue element"); + } + queueRepository.save(webhookConfig); + } else { + log.warn( + "Maximum amount of processing webhook reached for pull request: " + + pullRequest.getId() + + " " + + pullRequest.getPullRequestUrl()); + try { + gitHubService.commentResults(webhookConfig, null, null, pullRequest); + } catch (IOException e1) { + log.warn("Failed to post FAIL result " + e.getMessage()); + } + delete(webhookConfig); + } } } } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 0716fc69..d369f44d 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -12,6 +12,9 @@ scanner=scanoss # Corresponding env. variable LPVS_LICENSE_CONFLICT license_conflict=db +# Logger configuration +logging.pattern.console=%d{dd-MM-yyyy HH:mm:ss.SSS} %magenta([%thread]) %highlight(%-5level) %logger.%M - %msg%n + # GitHub settings # Corresponding env. variable LPVS_GITHUB_LOGIN github.login= diff --git a/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java b/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java index bebd0c73..a895cbbe 100644 --- a/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java +++ b/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java @@ -150,12 +150,18 @@ public void testProcessWebHook__NoPRDownloaded() throws IOException { queueService.processWebHook(webhookConfig); verify(mockGitHubService, times(1)).getPullRequestFiles(webhookConfig); - verify(mockGitHubService, times(1)) - .commentResults(eq(webhookConfig), any(), any(), eq(lpvsPullRequest)); verifyNoMoreInteractions(mockGitHubService); verifyNoMoreInteractions(mockDetectService); verifyNoMoreInteractions(mockLicenseService); + + // test when max attempts reached + webhookConfig.setAttempts(maxAttempts); + queueService.processWebHook(webhookConfig); + + verify(mockGitHubService, times(2)).getPullRequestFiles(webhookConfig); + verify(mockGitHubService, times(1)) + .commentResults(eq(webhookConfig), any(), any(), eq(lpvsPullRequest)); } } @@ -835,26 +841,11 @@ void setUp() { @Test public void testCheckForQueue() { - LPVSQueue webhookConfig = new LPVSQueue(); - webhookConfig.setAttempts(0); - webhookConfig.setDate(new Date()); - when(mocked_queueRepository.getQueueList()).thenReturn(List.of(webhookConfig)); - assertDoesNotThrow(() -> queueService.checkForQueue()); - verify(mocked_queueRepository).save(webhookConfig); - } - - @Test - public void testCheckForQueue__Alternative() { LPVSQueue webhookConfig = new LPVSQueue(); webhookConfig.setAttempts(100); webhookConfig.setDate(new Date()); - webhookConfig.setUserId("id"); - webhookConfig.setRepositoryUrl("https://github.com/Samsung/LPVS"); when(mocked_queueRepository.getQueueList()).thenReturn(List.of(webhookConfig)); - when(mocked_lpvsPullRequestRepository.saveAndFlush(Mockito.any(LPVSPullRequest.class))) - .thenAnswer(i -> i.getArguments()[0]); assertDoesNotThrow(() -> queueService.checkForQueue()); - verify(mocked_queueRepository).save(webhookConfig); } @Test