Skip to content

Commit

Permalink
refactor: Add queue ID to the PR diffs filepath (#617)
Browse files Browse the repository at this point in the history
Signed-off-by: Oleg Kopysov <[email protected]>
  • Loading branch information
o-kopysov authored Sep 20, 2024
1 parent 4fff427 commit aa59ab7
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 77 deletions.
6 changes: 6 additions & 0 deletions src/main/java/com/lpvs/entity/LPVSPullRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
35 changes: 6 additions & 29 deletions src/main/java/com/lpvs/repository/LPVSPullRequestRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,15 @@
* Extends {@link org.springframework.data.jpa.repository.JpaRepository} for basic CRUD operations.
*/
public interface LPVSPullRequestRepository extends JpaRepository<LPVSPullRequest, Long> {

/**
* 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.
Expand Down
13 changes: 2 additions & 11 deletions src/main/java/com/lpvs/service/webhook/LPVSWebhookServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 11 additions & 4 deletions src/main/java/com/lpvs/util/LPVSFileUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
Expand All @@ -209,6 +210,8 @@ public static String getLocalDirectoryPath(LPVSQueue webhookConfig) {
+ File.separator
+ LPVSPayloadUtil.getRepositoryName(webhookConfig)
+ File.separator
+ webhookConfig.getId().toString()
+ "-"
+ webhookConfig.getHeadCommitSHA();
}
}
Expand All @@ -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")
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/database_dump.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 19 additions & 7 deletions src/test/java/com/lpvs/entity/LPVSPullRequestTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -33,6 +34,7 @@ void setUp() {
lpvsPullRequest =
new LPVSPullRequest(
pullRequestId,
queueId,
date,
user,
repositoryName,
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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
}
}
15 changes: 10 additions & 5 deletions src/test/java/com/lpvs/entity/history/HistoryPageEntityTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -47,15 +49,18 @@ public void testInequality() {
// Test inequality when objects are not the same
List<LPVSPullRequest> 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<LPVSPullRequest> prPage1 = new PageImpl<>(pullRequests1);
Long count1 = 2L;
HistoryPageEntity entity1 = new HistoryPageEntity(prPage1, count1);
List<LPVSPullRequest> 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<LPVSPullRequest> prPage2 = new PageImpl<>(pullRequests2);
Long count2 = 1L;
HistoryPageEntity entity2 = new HistoryPageEntity(prPage2, count2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()))
Expand All @@ -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);
Expand Down
37 changes: 20 additions & 17 deletions src/test/java/com/lpvs/util/LPVSFileUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ 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");
}

@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");
Expand All @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -153,47 +154,47 @@ public void testCopyFilesDirectoryWithNullFiles() throws IOException {
@Test
public void testGetLocalDirectoryPathWithHeadCommitSHA() {
webhookConfig.setHeadCommitSHA("aaaa");
String expected = getExpectedProjectsPathWithCommitSHA();
String expected = getExpectedProjectsPathWithCommitSHA(1);

assertEquals(expected, LPVSFileUtil.getLocalDirectoryPath(webhookConfig));
}

@Test
public void testGetLocalDirectoryPathWithHeadCommitSHAEmpty() {
webhookConfig.setHeadCommitSHA("");
String expected = getExpectedProjectsPathWithPullRequestId();
String expected = getExpectedProjectsPathWithPullRequestId(1);

assertEquals(expected, LPVSFileUtil.getLocalDirectoryPath(webhookConfig));
}

@Test
public void testGetLocalDirectoryPathWithoutHeadCommitSHA() {
webhookConfig.setHeadCommitSHA(null);
String expected = getExpectedProjectsPathWithPullRequestId();
String expected = getExpectedProjectsPathWithPullRequestId(1);

assertEquals(expected, LPVSFileUtil.getLocalDirectoryPath(webhookConfig));
}

@Test
public void testGetScanResultsJsonFilePathWithHeadCommitSHA() {
webhookConfig.setHeadCommitSHA("aaaa");
String expected = getExpectedJsonFilePathWithCommitSHA();
String expected = getExpectedJsonFilePathWithCommitSHA(1);

assertEquals(expected, LPVSFileUtil.getScanResultsJsonFilePath(webhookConfig));
}

@Test
public void testGetScanResultsJsonFilePathWithHeadCommitSHAEmpty() {
webhookConfig.setHeadCommitSHA("");
String expected = getExpectedJsonFilePathWithPullRequestId();
String expected = getExpectedJsonFilePathWithPullRequestId(1);

assertEquals(expected, LPVSFileUtil.getScanResultsJsonFilePath(webhookConfig));
}

@Test
public void testGetScanResultsJsonFilePathWithoutHeadCommitSHA() {
webhookConfig.setHeadCommitSHA(null);
String expected = getExpectedJsonFilePathWithPullRequestId();
String expected = getExpectedJsonFilePathWithPullRequestId(1);

assertEquals(expected, LPVSFileUtil.getScanResultsJsonFilePath(webhookConfig));
}
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -242,7 +244,8 @@ private static String getExpectedProjectsPathWithPullRequestId() {
+ File.separator
+ "test"
+ File.separator
+ "123";
+ id
+ "-123";
}

private static String getExpectedResultsPath() {
Expand All @@ -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) {
Expand Down
Loading

0 comments on commit aa59ab7

Please sign in to comment.