From 18171d9d300c1ebfbc9a17ab98cf7b8e0312f5c6 Mon Sep 17 00:00:00 2001 From: Oleg Kopysov Date: Thu, 19 Sep 2024 22:09:22 +0300 Subject: [PATCH] refactor: Add queue ID to the PR diffs filepath Signed-off-by: Oleg Kopysov --- .../java/com/lpvs/entity/LPVSPullRequest.java | 6 +++ .../repository/LPVSPullRequestRepository.java | 35 +++--------------- .../webhook/LPVSWebhookServiceImpl.java | 13 +------ src/main/java/com/lpvs/util/LPVSFileUtil.java | 15 ++++++-- src/main/resources/database_dump.sql | 1 + .../com/lpvs/entity/LPVSPullRequestTest.java | 26 +++++++++---- .../entity/history/HistoryPageEntityTest.java | 15 +++++--- .../scanner/LPVSScanossDetectServiceTest.java | 4 +- .../java/com/lpvs/util/LPVSFileUtilTest.java | 37 ++++++++++--------- .../com/lpvs/util/LPVSPayloadUtilTest.java | 6 +-- src/test/resources/{A_B.json => 1-A_B.json} | 0 11 files changed, 81 insertions(+), 77 deletions(-) rename src/test/resources/{A_B.json => 1-A_B.json} (100%) diff --git a/src/main/java/com/lpvs/entity/LPVSPullRequest.java b/src/main/java/com/lpvs/entity/LPVSPullRequest.java index 7df86b14..4d0d3cc0 100644 --- a/src/main/java/com/lpvs/entity/LPVSPullRequest.java +++ b/src/main/java/com/lpvs/entity/LPVSPullRequest.java @@ -36,6 +36,12 @@ public class LPVSPullRequest implements Serializable { @Column(name = "id") private Long id; + /** + * ID of the connected queue element. + */ + @Column(name = "queue_id") + private Long queueId; + /** * The date of the pull request. */ diff --git a/src/main/java/com/lpvs/repository/LPVSPullRequestRepository.java b/src/main/java/com/lpvs/repository/LPVSPullRequestRepository.java index cfea80d7..2dc51004 100644 --- a/src/main/java/com/lpvs/repository/LPVSPullRequestRepository.java +++ b/src/main/java/com/lpvs/repository/LPVSPullRequestRepository.java @@ -20,38 +20,15 @@ * Extends {@link org.springframework.data.jpa.repository.JpaRepository} for basic CRUD operations. */ public interface LPVSPullRequestRepository extends JpaRepository { + /** - * Retrieves the latest LPVSPullRequest entity based on the provided criteria. + * Find pull request with the specified queue ID. * - * @param user The user associated with the pull request. If {@code null}, this parameter is ignored. - * @param repositoryName The name of the repository associated with the pull request. If {@code null}, this parameter is ignored. - * @param pullRequestFilesUrl The URL of the pull request files. If {@code null}, this parameter is ignored. - * @param pullRequestHead The head of the pull request. If {@code null}, this parameter is ignored. - * @param pullRequestBase The base of the pull request. If {@code null}, this parameter is ignored. - * @param sender The sender of the pull request. If {@code null}, this parameter is ignored. - * @param status The status of the pull request. If {@code null}, this parameter is ignored. - * @return The latest LPVSPullRequest entity matching the provided criteria, - * or {@code null} if no matching entity is found. + * @param queueId ID of the related element from the queue. + * @return {@link LPVSPullRequest} entity with the specified queue ID. */ - @Query( - value = - "SELECT pr FROM LPVSPullRequest pr " - + "WHERE (:user IS NULL OR pr.user = :user) " - + "AND (:repositoryName IS NULL OR pr.repositoryName = :repositoryName) " - + "AND (:pullRequestFilesUrl IS NULL OR pr.pullRequestFilesUrl = :pullRequestFilesUrl) " - + "AND (:pullRequestHead IS NULL OR pr.pullRequestHead = :pullRequestHead) " - + "AND (:pullRequestBase IS NULL OR pr.pullRequestBase = :pullRequestBase) " - + "AND (:sender IS NULL OR pr.sender = :sender) " - + "AND (:status IS NULL OR pr.status = :status) " - + "ORDER BY pr.date DESC LIMIT 1") - LPVSPullRequest findLatestByPullRequestInfo( - @Param("user") String user, - @Param("repositoryName") String repositoryName, - @Param("pullRequestFilesUrl") String pullRequestFilesUrl, - @Param("pullRequestHead") String pullRequestHead, - @Param("pullRequestBase") String pullRequestBase, - @Param("sender") String sender, - @Param("status") String status); + @Query(value = "SELECT pr FROM LPVSPullRequest pr WHERE pr.queueId = :queueId LIMIT 1") + LPVSPullRequest findByQueueId(@Param("queueId") Long queueId); /** * Find all pull requests with the specified base name, paginated. diff --git a/src/main/java/com/lpvs/service/webhook/LPVSWebhookServiceImpl.java b/src/main/java/com/lpvs/service/webhook/LPVSWebhookServiceImpl.java index e537ba86..060923c5 100644 --- a/src/main/java/com/lpvs/service/webhook/LPVSWebhookServiceImpl.java +++ b/src/main/java/com/lpvs/service/webhook/LPVSWebhookServiceImpl.java @@ -115,20 +115,11 @@ public void processWebHook(LPVSQueue webhookConfig) { + (webhookConfig.getAttempts() + 1) + " for PR: " + webhookConfig.getPullRequestUrl()); - LPVSPullRequest pullRequest = - lpvsPullRequestRepository.findLatestByPullRequestInfo( - webhookConfig.getUserId(), - LPVSPayloadUtil.getRepositoryOrganization(webhookConfig) - + "/" - + LPVSPayloadUtil.getRepositoryName(webhookConfig), - webhookConfig.getPullRequestFilesUrl(), - webhookConfig.getPullRequestHead(), - webhookConfig.getPullRequestBase(), - webhookConfig.getSender(), - LPVSPullRequestStatus.INTERNAL_ERROR.getPullRequestStatus()); + LPVSPullRequest pullRequest = lpvsPullRequestRepository.findByQueueId(id); if (pullRequest == null) { pullRequest = new LPVSPullRequest(); + pullRequest.setQueueId(id); pullRequest.setUser(webhookConfig.getUserId()); pullRequest.setRepositoryName( LPVSPayloadUtil.getRepositoryOrganization(webhookConfig) diff --git a/src/main/java/com/lpvs/util/LPVSFileUtil.java b/src/main/java/com/lpvs/util/LPVSFileUtil.java index 42585c01..0550aecd 100644 --- a/src/main/java/com/lpvs/util/LPVSFileUtil.java +++ b/src/main/java/com/lpvs/util/LPVSFileUtil.java @@ -189,8 +189,7 @@ public static void deleteIfExists(String path) { * @return The local directory path. */ public static String getLocalDirectoryPath(LPVSQueue webhookConfig) { - if (webhookConfig.getHeadCommitSHA() == null - || webhookConfig.getHeadCommitSHA().isEmpty()) { + if (StringUtils.isBlank(webhookConfig.getHeadCommitSHA())) { return System.getProperty("user.home") + File.separator + "LPVS" @@ -199,6 +198,8 @@ public static String getLocalDirectoryPath(LPVSQueue webhookConfig) { + File.separator + LPVSPayloadUtil.getRepositoryName(webhookConfig) + File.separator + + webhookConfig.getId().toString() + + "-" + LPVSPayloadUtil.getPullRequestId(webhookConfig); } else { return System.getProperty("user.home") @@ -209,6 +210,8 @@ public static String getLocalDirectoryPath(LPVSQueue webhookConfig) { + File.separator + LPVSPayloadUtil.getRepositoryName(webhookConfig) + File.separator + + webhookConfig.getId().toString() + + "-" + webhookConfig.getHeadCommitSHA(); } } @@ -222,9 +225,13 @@ public static String getLocalDirectoryPath(LPVSQueue webhookConfig) { public static String getScanResultsJsonFilePath(LPVSQueue webhookConfig) { String fileName = null; if (StringUtils.isBlank(webhookConfig.getHeadCommitSHA())) { - fileName = LPVSPayloadUtil.getPullRequestId(webhookConfig); + fileName = + webhookConfig.getId() + "-" + LPVSPayloadUtil.getPullRequestId(webhookConfig); } else { - fileName = HtmlUtils.htmlEscape(webhookConfig.getHeadCommitSHA()); + fileName = + webhookConfig.getId() + + "-" + + HtmlUtils.htmlEscape(webhookConfig.getHeadCommitSHA()); } return System.getProperty("user.home") diff --git a/src/main/resources/database_dump.sql b/src/main/resources/database_dump.sql index 7e66ea64..8a962504 100644 --- a/src/main/resources/database_dump.sql +++ b/src/main/resources/database_dump.sql @@ -24,6 +24,7 @@ CREATE TABLE IF NOT EXISTS lpvs_license_conflicts ( CREATE TABLE IF NOT EXISTS lpvs_pull_requests ( id bigint NOT NULL AUTO_INCREMENT, + queue_id bigint DEFAULT NULL, scan_date datetime NOT NULL, user varchar(255) DEFAULT NULL, repository_name varchar(255) NOT NULL, diff --git a/src/test/java/com/lpvs/entity/LPVSPullRequestTest.java b/src/test/java/com/lpvs/entity/LPVSPullRequestTest.java index 0e053f3b..8db3de91 100644 --- a/src/test/java/com/lpvs/entity/LPVSPullRequestTest.java +++ b/src/test/java/com/lpvs/entity/LPVSPullRequestTest.java @@ -18,6 +18,7 @@ public class LPVSPullRequestTest { LPVSPullRequest lpvsPullRequest; final long pullRequestId = 1234567890L; + final long queueId = 54321L; final Date date = new Date(); final String user = "user"; final String repositoryName = "repositoryName"; @@ -33,6 +34,7 @@ void setUp() { lpvsPullRequest = new LPVSPullRequest( pullRequestId, + queueId, date, user, repositoryName, @@ -47,6 +49,7 @@ void setUp() { @Test public void gettersTest() { assertEquals(lpvsPullRequest.getId(), pullRequestId); + assertEquals(lpvsPullRequest.getQueueId(), queueId); assertEquals(lpvsPullRequest.getDate(), date); assertEquals(lpvsPullRequest.getUser(), user); assertEquals(lpvsPullRequest.getRepositoryName(), repositoryName); @@ -66,6 +69,15 @@ public void setPullRequestIdTest() { assertEquals(lpvsPullRequest.getId(), newActualValue); } + @Test + public void setQueueIdIdTest() { + final long newActualValue = 0L; + assertEquals(lpvsPullRequest.getQueueId(), queueId); + lpvsPullRequest.setQueueId(newActualValue); + assertNotEquals(lpvsPullRequest.getQueueId(), queueId); + assertEquals(lpvsPullRequest.getQueueId(), newActualValue); + } + @Test public void setPullRequestDateTest() { final Date newActualValue = new Date(System.currentTimeMillis() - 3600 * 1000); @@ -179,13 +191,13 @@ public void testEquals() { pr6.setPullRequestFilesUrl( pullRequestFilesUrl + "1"); /* initialize with different pullRequestFilesUrl */ - assertTrue(pr1.equals(pr2)); // Objects are equal - assertFalse(pr1.equals(pr3)); // Date is different - assertFalse(pr1.equals(pr4)); // RepositoryName is different - assertFalse(pr1.equals(pr5)); // PullRequestUrl is different - assertFalse(pr1.equals(pr6)); // PullRequestFilesUrl is different + assertEquals(pr1, pr2); // Objects are equal + assertNotEquals(pr1, pr3); // Date is different + assertNotEquals(pr1, pr4); // RepositoryName is different + assertNotEquals(pr1, pr5); // PullRequestUrl is different + assertNotEquals(pr1, pr6); // PullRequestFilesUrl is different - assertFalse(pr1.equals(null)); // Null comparison - assertFalse(pr1.equals(new Object())); // Different class comparison + assertNotEquals(null, pr1); // Null comparison + assertNotEquals(pr1, new Object()); // Different class comparison } } diff --git a/src/test/java/com/lpvs/entity/history/HistoryPageEntityTest.java b/src/test/java/com/lpvs/entity/history/HistoryPageEntityTest.java index a4c061bb..b7bbb121 100644 --- a/src/test/java/com/lpvs/entity/history/HistoryPageEntityTest.java +++ b/src/test/java/com/lpvs/entity/history/HistoryPageEntityTest.java @@ -28,9 +28,11 @@ public class HistoryPageEntityTest { @BeforeEach public void setUp() { pullRequests.add( - new LPVSPullRequest(1L, null, "Title 1", null, null, null, null, null, null, null)); + new LPVSPullRequest( + 1L, null, null, "Title 1", null, null, null, null, null, null, null)); pullRequests.add( - new LPVSPullRequest(2L, null, "Title 2", null, null, null, null, null, null, null)); + new LPVSPullRequest( + 2L, null, null, "Title 2", null, null, null, null, null, null, null)); prPage = new PageImpl<>(pullRequests); historyPageEntity = new HistoryPageEntity(prPage, count); } @@ -47,15 +49,18 @@ public void testInequality() { // Test inequality when objects are not the same List pullRequests1 = new ArrayList<>(); pullRequests1.add( - new LPVSPullRequest(1L, null, "Title 1", null, null, null, null, null, null, null)); + new LPVSPullRequest( + 1L, null, null, "Title 1", null, null, null, null, null, null, null)); pullRequests1.add( - new LPVSPullRequest(2L, null, "Title 2", null, null, null, null, null, null, null)); + new LPVSPullRequest( + 2L, null, null, "Title 2", null, null, null, null, null, null, null)); Page prPage1 = new PageImpl<>(pullRequests1); Long count1 = 2L; HistoryPageEntity entity1 = new HistoryPageEntity(prPage1, count1); List pullRequests2 = new ArrayList<>(); pullRequests2.add( - new LPVSPullRequest(3L, null, "Title 3", null, null, null, null, null, null, null)); + new LPVSPullRequest( + 3L, null, null, "Title 3", null, null, null, null, null, null, null)); Page prPage2 = new PageImpl<>(pullRequests2); Long count2 = 1L; HistoryPageEntity entity2 = new HistoryPageEntity(prPage2, count2); diff --git a/src/test/java/com/lpvs/service/scan/scanner/LPVSScanossDetectServiceTest.java b/src/test/java/com/lpvs/service/scan/scanner/LPVSScanossDetectServiceTest.java index f16b6a16..3a297f81 100644 --- a/src/test/java/com/lpvs/service/scan/scanner/LPVSScanossDetectServiceTest.java +++ b/src/test/java/com/lpvs/service/scan/scanner/LPVSScanossDetectServiceTest.java @@ -50,7 +50,7 @@ public class LPVSScanossDetectServiceTest { @BeforeEach public void setUp() throws URISyntaxException, IOException { MockitoAnnotations.openMocks(this); - String resourcePath = "A_B.json"; + String resourcePath = "1-A_B.json"; String destinationPath = System.getProperty("user.home") + File.separator @@ -136,6 +136,7 @@ public void testWithNullHeadCommitSHA() { .thenReturn("https://github.com/Samsung/LPVS"); Mockito.when(LPVSPayloadUtil.getRepositoryName(webhookConfig)).thenReturn("C"); Mockito.when(webhookConfig.getPullRequestUrl()).thenReturn("A_B"); + Mockito.when(webhookConfig.getId()).thenReturn(1L); ReflectionTestUtils.setField( licenseService, "licenseConflictsSource", licenseConflictsSource); Mockito.when(licenseService.getLicenseBySpdxIdAndName(anyString(), any())) @@ -158,6 +159,7 @@ public void testWithNullHeadCommitSHADB() { Mockito.when(webhookConfig.getRepositoryUrl()) .thenReturn("https://github.com/Samsung/LPVS"); Mockito.when(webhookConfig.getPullRequestUrl()).thenReturn("A_B"); + Mockito.when(webhookConfig.getId()).thenReturn(1L); Mockito.when(LPVSPayloadUtil.getRepositoryName(webhookConfig)).thenReturn("C"); ReflectionTestUtils.setField( licenseService, "licenseConflictsSource", licenseConflictsSource); diff --git a/src/test/java/com/lpvs/util/LPVSFileUtilTest.java b/src/test/java/com/lpvs/util/LPVSFileUtilTest.java index 5b22b646..b4c2f745 100644 --- a/src/test/java/com/lpvs/util/LPVSFileUtilTest.java +++ b/src/test/java/com/lpvs/util/LPVSFileUtilTest.java @@ -31,6 +31,7 @@ public class LPVSFileUtilTest { @BeforeEach public void setUp() { webhookConfig = new LPVSQueue(); + webhookConfig.setId(1L); webhookConfig.setRepositoryUrl("http://test.com/test/test"); webhookConfig.setPullRequestUrl("http://test.com/test/test/pull/123"); } @@ -38,7 +39,7 @@ public void setUp() { @Test public void testSaveGithubDiffs() { webhookConfig.setHeadCommitSHA("aaaa"); - String expected = getExpectedProjectsPathWithCommitSHA(); + String expected = getExpectedProjectsPathWithCommitSHA(1); GHPullRequestFileDetail detail = new GHPullRequestFileDetail(); ReflectionTestUtils.setField(detail, "filename", "I_am_a_file"); @@ -57,7 +58,7 @@ public void testSaveGithubDiffs() { @Test public void testSaveGithubDiffsFileNameWithSlash() { webhookConfig.setHeadCommitSHA("aaaa"); - String expected = getExpectedProjectsPathWithCommitSHA(); + String expected = getExpectedProjectsPathWithCommitSHA(1); GHPullRequestFileDetail detail = new GHPullRequestFileDetail(); ReflectionTestUtils.setField(detail, "filename", "dir/I_am_a_file"); @@ -76,7 +77,7 @@ public void testSaveGithubDiffsFileNameWithSlash() { @Test public void testSaveGithubDiffsEmptyPatchLines() { webhookConfig.setHeadCommitSHA("aaaa"); - String expected = getExpectedProjectsPathWithCommitSHA(); + String expected = getExpectedProjectsPathWithCommitSHA(1); GHPullRequestFileDetail detail = new GHPullRequestFileDetail(); ReflectionTestUtils.setField(detail, "filename", "I_am_a_file"); @@ -153,7 +154,7 @@ public void testCopyFilesDirectoryWithNullFiles() throws IOException { @Test public void testGetLocalDirectoryPathWithHeadCommitSHA() { webhookConfig.setHeadCommitSHA("aaaa"); - String expected = getExpectedProjectsPathWithCommitSHA(); + String expected = getExpectedProjectsPathWithCommitSHA(1); assertEquals(expected, LPVSFileUtil.getLocalDirectoryPath(webhookConfig)); } @@ -161,7 +162,7 @@ public void testGetLocalDirectoryPathWithHeadCommitSHA() { @Test public void testGetLocalDirectoryPathWithHeadCommitSHAEmpty() { webhookConfig.setHeadCommitSHA(""); - String expected = getExpectedProjectsPathWithPullRequestId(); + String expected = getExpectedProjectsPathWithPullRequestId(1); assertEquals(expected, LPVSFileUtil.getLocalDirectoryPath(webhookConfig)); } @@ -169,7 +170,7 @@ public void testGetLocalDirectoryPathWithHeadCommitSHAEmpty() { @Test public void testGetLocalDirectoryPathWithoutHeadCommitSHA() { webhookConfig.setHeadCommitSHA(null); - String expected = getExpectedProjectsPathWithPullRequestId(); + String expected = getExpectedProjectsPathWithPullRequestId(1); assertEquals(expected, LPVSFileUtil.getLocalDirectoryPath(webhookConfig)); } @@ -177,7 +178,7 @@ public void testGetLocalDirectoryPathWithoutHeadCommitSHA() { @Test public void testGetScanResultsJsonFilePathWithHeadCommitSHA() { webhookConfig.setHeadCommitSHA("aaaa"); - String expected = getExpectedJsonFilePathWithCommitSHA(); + String expected = getExpectedJsonFilePathWithCommitSHA(1); assertEquals(expected, LPVSFileUtil.getScanResultsJsonFilePath(webhookConfig)); } @@ -185,7 +186,7 @@ public void testGetScanResultsJsonFilePathWithHeadCommitSHA() { @Test public void testGetScanResultsJsonFilePathWithHeadCommitSHAEmpty() { webhookConfig.setHeadCommitSHA(""); - String expected = getExpectedJsonFilePathWithPullRequestId(); + String expected = getExpectedJsonFilePathWithPullRequestId(1); assertEquals(expected, LPVSFileUtil.getScanResultsJsonFilePath(webhookConfig)); } @@ -193,7 +194,7 @@ public void testGetScanResultsJsonFilePathWithHeadCommitSHAEmpty() { @Test public void testGetScanResultsJsonFilePathWithoutHeadCommitSHA() { webhookConfig.setHeadCommitSHA(null); - String expected = getExpectedJsonFilePathWithPullRequestId(); + String expected = getExpectedJsonFilePathWithPullRequestId(1); assertEquals(expected, LPVSFileUtil.getScanResultsJsonFilePath(webhookConfig)); } @@ -221,7 +222,7 @@ public void testSaveFileWithEmptyPatchedLines() { assert (result2.equals(false)); } - private static String getExpectedProjectsPathWithCommitSHA() { + private static String getExpectedProjectsPathWithCommitSHA(long id) { return System.getProperty("user.home") + File.separator + "LPVS" @@ -230,10 +231,11 @@ private static String getExpectedProjectsPathWithCommitSHA() { + File.separator + "test" + File.separator - + "aaaa"; + + id + + "-aaaa"; } - private static String getExpectedProjectsPathWithPullRequestId() { + private static String getExpectedProjectsPathWithPullRequestId(long id) { return System.getProperty("user.home") + File.separator + "LPVS" @@ -242,7 +244,8 @@ private static String getExpectedProjectsPathWithPullRequestId() { + File.separator + "test" + File.separator - + "123"; + + id + + "-123"; } private static String getExpectedResultsPath() { @@ -255,12 +258,12 @@ private static String getExpectedResultsPath() { + "test"; } - private static String getExpectedJsonFilePathWithPullRequestId() { - return getExpectedResultsPath() + File.separator + "123.json"; + private static String getExpectedJsonFilePathWithPullRequestId(long id) { + return getExpectedResultsPath() + File.separator + id + "-123.json"; } - private static String getExpectedJsonFilePathWithCommitSHA() { - return getExpectedResultsPath() + File.separator + "aaaa.json"; + private static String getExpectedJsonFilePathWithCommitSHA(long id) { + return getExpectedResultsPath() + File.separator + id + "-aaaa.json"; } private void deleteDirectory(File directory) { diff --git a/src/test/java/com/lpvs/util/LPVSPayloadUtilTest.java b/src/test/java/com/lpvs/util/LPVSPayloadUtilTest.java index b1b72cbc..782b62bc 100644 --- a/src/test/java/com/lpvs/util/LPVSPayloadUtilTest.java +++ b/src/test/java/com/lpvs/util/LPVSPayloadUtilTest.java @@ -365,7 +365,7 @@ public void testCreateInputStreamReader() throws IOException, URISyntaxException Path path = Paths.get( Objects.requireNonNull( - getClass().getClassLoader().getResource("A_B.json")) + getClass().getClassLoader().getResource("1-A_B.json")) .toURI()); InputStream inputStream = Files.newInputStream(path); InputStreamReader inputStreamReader = @@ -392,7 +392,7 @@ public void testCreateBufferReader() throws URISyntaxException, IOException { Path path = Paths.get( Objects.requireNonNull( - getClass().getClassLoader().getResource("A_B.json")) + getClass().getClassLoader().getResource("1-A_B.json")) .toURI()); InputStream inputStream = Files.newInputStream(path); InputStreamReader inputStreamReader = @@ -421,7 +421,7 @@ public void testConvertInputStreamToString() throws IOException, URISyntaxExcept Path path = Paths.get( Objects.requireNonNull( - getClass().getClassLoader().getResource("A_B.json")) + getClass().getClassLoader().getResource("1-A_B.json")) .toURI()); InputStream inputStream = Files.newInputStream(path); assertNotNull(LPVSPayloadUtil.convertInputStreamToString(inputStream)); diff --git a/src/test/resources/A_B.json b/src/test/resources/1-A_B.json similarity index 100% rename from src/test/resources/A_B.json rename to src/test/resources/1-A_B.json