From f5e3ecd83c53977997bba87b30e3855d810e073e Mon Sep 17 00:00:00 2001 From: Mykola Rudyk Date: Mon, 12 Feb 2024 10:37:19 +0200 Subject: [PATCH 1/3] fix: remove PR if max attempt reached Signed-off-by: Mykola Rudyk --- .../com/lpvs/service/LPVSQueueService.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/lpvs/service/LPVSQueueService.java b/src/main/java/com/lpvs/service/LPVSQueueService.java index 977cf8a2..195718eb 100644 --- a/src/main/java/com/lpvs/service/LPVSQueueService.java +++ b/src/main/java/com/lpvs/service/LPVSQueueService.java @@ -271,7 +271,23 @@ public void processWebHook(LPVSQueue webhookConfig) { pullRequest = lpvsPullRequestRepository.saveAndFlush(pullRequest); log.error("Can't authorize commentResults() " + e); e.printStackTrace(); - delete(webhookConfig); + int currentAttempts = webhookConfig.getAttempts(); + if (currentAttempts < maxAttempts) { + webhookConfig.setAttempts(currentAttempts++); + 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()); + delete(webhookConfig); + } } } } From c9ce7861de3172f2d350605725c556f90f682c84 Mon Sep 17 00:00:00 2001 From: Mykola Rudyk Date: Wed, 14 Feb 2024 09:50:42 +0200 Subject: [PATCH 2/3] test: update tests to keep coverage Signed-off-by: Mykola Rudyk --- src/test/java/com/lpvs/service/LPVSQueueServiceTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java b/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java index bebd0c73..8882c8a5 100644 --- a/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java +++ b/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java @@ -156,6 +156,12 @@ public void testProcessWebHook__NoPRDownloaded() throws IOException { verifyNoMoreInteractions(mockGitHubService); verifyNoMoreInteractions(mockDetectService); verifyNoMoreInteractions(mockLicenseService); + + // test when max attempts reached + webhookConfig.setAttempts(maxAttempts); + queueService.processWebHook(webhookConfig); + + verify(mockGitHubService, times(2)).getPullRequestFiles(webhookConfig); } } From 97d15c0262c0189495ff5328f927777a6747df96 Mon Sep 17 00:00:00 2001 From: Mykola Rudyk Date: Wed, 14 Feb 2024 12:39:02 +0200 Subject: [PATCH 3/3] fix: add changes after review comment, remove unnecessary check Signed-off-by: Mykola Rudyk --- .../com/lpvs/service/LPVSQueueService.java | 57 ++++++------------- src/main/resources/application.properties | 3 + .../lpvs/service/LPVSQueueServiceTest.java | 19 +------ 3 files changed, 21 insertions(+), 58 deletions(-) diff --git a/src/main/java/com/lpvs/service/LPVSQueueService.java b/src/main/java/com/lpvs/service/LPVSQueueService.java index 195718eb..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,22 +231,19 @@ 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(); + log.error("Can't authorize commentResults() " + e.getMessage()); int currentAttempts = webhookConfig.getAttempts(); if (currentAttempts < maxAttempts) { - webhookConfig.setAttempts(currentAttempts++); + webhookConfig.setAttempts(currentAttempts + 1); try { addFirst(webhookConfig); } catch (InterruptedException e1) { @@ -286,6 +256,11 @@ public void processWebHook(LPVSQueue webhookConfig) { + 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 8882c8a5..a895cbbe 100644 --- a/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java +++ b/src/test/java/com/lpvs/service/LPVSQueueServiceTest.java @@ -150,8 +150,6 @@ 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); @@ -162,6 +160,8 @@ public void testProcessWebHook__NoPRDownloaded() throws IOException { queueService.processWebHook(webhookConfig); verify(mockGitHubService, times(2)).getPullRequestFiles(webhookConfig); + verify(mockGitHubService, times(1)) + .commentResults(eq(webhookConfig), any(), any(), eq(lpvsPullRequest)); } } @@ -841,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