From 0849204280a7097d4c4040f03ae01674a6d81c38 Mon Sep 17 00:00:00 2001 From: Oleg Kopysov Date: Mon, 29 Apr 2024 11:59:17 +0300 Subject: [PATCH] fix: Fix license search by alternative names and remove code duplicates (#497) * fix: Fix incorrect work of license check by alternative name Signed-off-by: Oleg Kopysov * refactor: Remove code duplicates Signed-off-by: Oleg Kopysov * fix: Fix unit tests after the source code update Signed-off-by: Oleg Kopysov --------- Signed-off-by: Oleg Kopysov --- .../repository/LPVSLicenseRepository.java | 2 +- .../com/lpvs/service/LPVSLicenseService.java | 59 +++++++++++-------- .../java/com/lpvs/util/LPVSWebhookUtil.java | 45 ++++++-------- src/main/resources/database_dump.sql | 16 ++--- .../lpvs/service/LPVSLicenseServiceTest.java | 18 ++++++ .../com/lpvs/util/LPVSWebhookUtilTest.java | 8 +-- 6 files changed, 81 insertions(+), 67 deletions(-) diff --git a/src/main/java/com/lpvs/repository/LPVSLicenseRepository.java b/src/main/java/com/lpvs/repository/LPVSLicenseRepository.java index e15a1286..39394434 100644 --- a/src/main/java/com/lpvs/repository/LPVSLicenseRepository.java +++ b/src/main/java/com/lpvs/repository/LPVSLicenseRepository.java @@ -49,7 +49,7 @@ public interface LPVSLicenseRepository extends JpaRepository */ @Query( value = - "SELECT * FROM license_list WHERE license_list.license_alternative_names LIKE %:licenseName% ORDER BY id DESC LIMIT 1", + "SELECT * FROM license_list WHERE FIND_IN_SET(:licenseName, license_alternative_names) > 0 ORDER BY id DESC LIMIT 1", nativeQuery = true) LPVSLicense searchByAlternativeLicenseNames(@Param("licenseName") String licenseName); diff --git a/src/main/java/com/lpvs/service/LPVSLicenseService.java b/src/main/java/com/lpvs/service/LPVSLicenseService.java index 2a0703c1..995abf51 100644 --- a/src/main/java/com/lpvs/service/LPVSLicenseService.java +++ b/src/main/java/com/lpvs/service/LPVSLicenseService.java @@ -97,6 +97,23 @@ public LPVSLicenseService( this.exitHandler = exitHandler; } + /** + * Load all license conflicts from the database. + */ + private void loadLicenseConflicts() { + List conflicts = + lpvsLicenseConflictRepository.takeAllLicenseConflicts(); + for (LPVSLicenseConflict conflict : conflicts) { + Conflict conf = + new Conflict<>( + conflict.getConflictLicense().getSpdxId(), + conflict.getRepositoryLicense().getSpdxId()); + if (!licenseConflicts.contains(conf)) { + licenseConflicts.add(conf); + } + } + } + /** * Initializes the LPVSLicenseService by loading licenses and license conflicts from the database. */ @@ -129,17 +146,7 @@ private void init() { licenseConflicts = new ArrayList<>(); if (licenseConflictsSource.equalsIgnoreCase("db")) { - List conflicts = - lpvsLicenseConflictRepository.takeAllLicenseConflicts(); - for (LPVSLicenseConflict conflict : conflicts) { - Conflict conf = - new Conflict<>( - conflict.getConflictLicense().getSpdxId(), - conflict.getRepositoryLicense().getSpdxId()); - if (!licenseConflicts.contains(conf)) { - licenseConflicts.add(conf); - } - } + loadLicenseConflicts(); // print info log.info( "LICENSE CONFLICTS: loaded " @@ -163,20 +170,10 @@ private void init() { public void reloadFromTables() { if (licenses.isEmpty()) { licenses = lpvsLicenseRepository.takeAllLicenses(); - log.info("LOADED " + licenses.size() + " licenses from DB."); - - List conflicts = - lpvsLicenseConflictRepository.takeAllLicenseConflicts(); - for (LPVSLicenseConflict conflict : conflicts) { - Conflict conf = - new Conflict<>( - conflict.getConflictLicense().getSpdxId(), - conflict.getRepositoryLicense().getSpdxId()); - if (!licenseConflicts.contains(conf)) { - licenseConflicts.add(conf); - } - } - log.info("LOADED " + licenseConflicts.size() + " license conflicts from DB."); + log.info("RELOADED " + licenses.size() + " licenses from DB."); + + loadLicenseConflicts(); + log.info("RELOADED " + licenseConflicts.size() + " license conflicts from DB."); } } @@ -276,8 +273,18 @@ public List> findConflicts( // 1. Check conflict between repository license and detected licenses String repositoryLicense = webhookConfig.getRepositoryLicense(); - // ToDo: add check for license alternative names. Reason: GitHub can use not SPDX ID. + if (repositoryLicense != null) { + LPVSLicense repoLicense = lpvsLicenseRepository.searchBySpdxId(repositoryLicense); + if (repoLicense == null) { + repoLicense = + lpvsLicenseRepository.searchByAlternativeLicenseNames(repositoryLicense); + } + + if (repoLicense != null) { + repositoryLicense = repoLicense.getSpdxId(); + } + for (String detectedLicenseUnique : detectedLicensesUnique) { for (Conflict licenseConflict : licenseConflicts) { Conflict possibleConflict = diff --git a/src/main/java/com/lpvs/util/LPVSWebhookUtil.java b/src/main/java/com/lpvs/util/LPVSWebhookUtil.java index 5d71cdea..1e2d9e72 100644 --- a/src/main/java/com/lpvs/util/LPVSWebhookUtil.java +++ b/src/main/java/com/lpvs/util/LPVSWebhookUtil.java @@ -109,23 +109,31 @@ public static boolean checkPayload(String payload) { } /** - * Retrieves the organization name from the repository URL in the LPVSQueue object. + * Checks if the given LPVSQueue object is not null and has a non-null repository URL. * - * @param webhookConfig LPVSQueue object containing repository information. - * @return The organization name. - * @throws IllegalArgumentException If the provided LPVSQueue object is null or if the repository URL is absent. + * @param webhookConfig the LPVSQueue object to check + * @throws IllegalArgumentException If the webhook configuration is null or if the repository URL is absent. */ - public static String getRepositoryOrganization(LPVSQueue webhookConfig) { + private static void checkWebhookConfig(LPVSQueue webhookConfig) { if (null == webhookConfig) { log.error("Webhook Config is absent"); - throw new IllegalArgumentException("Webhook is absent"); + throw new IllegalArgumentException("Webhook Config is absent"); } if (null == webhookConfig.getRepositoryUrl()) { log.error("No repository URL info in webhook config"); throw new IllegalArgumentException("No repository URL info in webhook config"); } + } + /** + * Retrieves the organization name from the repository URL in the LPVSQueue object. + * + * @param webhookConfig LPVSQueue object containing repository information. + * @return The organization name. + */ + public static String getRepositoryOrganization(LPVSQueue webhookConfig) { + checkWebhookConfig(webhookConfig); List url = Arrays.asList(webhookConfig.getRepositoryUrl().split("/")); return url.get(url.size() - 2); } @@ -135,19 +143,9 @@ public static String getRepositoryOrganization(LPVSQueue webhookConfig) { * * @param webhookConfig LPVSQueue object containing repository information. * @return The repository name. - * @throws IllegalArgumentException If the provided LPVSQueue object is null or if the repository URL is absent. */ public static String getRepositoryName(LPVSQueue webhookConfig) { - if (null == webhookConfig) { - log.error("Webhook Config is absent"); - throw new IllegalArgumentException("Webhook is absent"); - } - - if (null == webhookConfig.getRepositoryUrl()) { - log.error("No repository URL info in webhook config"); - throw new IllegalArgumentException("No repository URL info in webhook config"); - } - + checkWebhookConfig(webhookConfig); List url = Arrays.asList(webhookConfig.getRepositoryUrl().split("/")); return url.get(url.size() - 1); } @@ -162,7 +160,7 @@ public static String getRepositoryName(LPVSQueue webhookConfig) { public static String getRepositoryUrl(LPVSQueue webhookConfig) { if (null == webhookConfig) { log.error("Webhook Config is absent"); - throw new IllegalArgumentException("Webhook is absent"); + throw new IllegalArgumentException("Webhook Config is absent"); } return webhookConfig.getRepositoryUrl(); } @@ -175,16 +173,7 @@ public static String getRepositoryUrl(LPVSQueue webhookConfig) { * @throws IllegalArgumentException If the provided LPVSQueue object is null or if the pull request URL is absent. */ public static String getPullRequestId(LPVSQueue webhookConfig) { - if (null == webhookConfig) { - log.error("Webhook Config is absent"); - throw new IllegalArgumentException("Webhook is absent"); - } - - if (null == webhookConfig.getRepositoryUrl()) { - log.error("No repository URL info in webhook config"); - throw new IllegalArgumentException("No repository URL info in webhook config"); - } - + checkWebhookConfig(webhookConfig); if (null == webhookConfig.getPullRequestUrl()) { log.error("Pull Request URL is absent in webhook config"); throw new IllegalArgumentException("Pull Request URL is absent in webhook config"); diff --git a/src/main/resources/database_dump.sql b/src/main/resources/database_dump.sql index f3f821a9..49f5ab1c 100644 --- a/src/main/resources/database_dump.sql +++ b/src/main/resources/database_dump.sql @@ -94,14 +94,14 @@ CREATE TABLE IF NOT EXISTS member ( UNIQUE (email,provider) ); -INSERT INTO license_list (id, license_name, license_spdx, license_usage) VALUES -(1, 'GNU General Public License v3.0 only','GPL-3.0-only','PROHIBITED'), -(2, 'OpenSSL License','OpenSSL','PERMITTED'), -(3, 'GNU Lesser General Public License v2.0 or later','LGPL-2.0-or-later','RESTRICTED'), -(4, 'MIT License', 'MIT', 'PERMITTED'), -(5, 'Apache License 2.0', 'Apache-2.0', 'PERMITTED'), -(6, 'GNU General Public License v2.0 only', 'GPL-2.0-only', 'RESTRICTED'), -(7, 'GNU Lesser General Public License v3.0 or later', 'LGPL-3.0-or-later', 'PROHIBITED'); +INSERT INTO license_list (id, license_name, license_spdx, license_alternative_names, license_usage) VALUES +(1, 'GNU General Public License v3.0 only','GPL-3.0-only','','PROHIBITED'), +(2, 'OpenSSL License','OpenSSL','OPENSSL_LICENSE,SSLeay license and OpenSSL License','PERMITTED'), +(3, 'GNU Lesser General Public License v2.0 or later','LGPL-2.0-or-later','','RESTRICTED'), +(4, 'MIT License','MIT','Bouncy Castle Licence,The MIT License,The MIT License (MIT)','PERMITTED'), +(5, 'Apache License 2.0','Apache-2.0','Android-Apache-2.0,Apache 2,Apache 2.0,Apache 2.0 license,Apache License (v2.0),Apache License v2,Apache License v2.0,Apache License Version 2.0,Apache License Version 2.0 January 2004,Apache Public License 2.0,Apache Software License (Apache 2.0),Apache Software License (Apache License 2.0),Apache Software License - Version 2.0,Apache v2,Apache v2.0,Apache Version 2.0,Apache-2.0 License,APACHE2,ASF 2.0,http://www.apache.org/licenses/LICENSE-2.0.txt,https://www.apache.org/licenses/LICENSE-2.0,https://www.apache.org/licenses/LICENSE-2.0.txt,the Apache License ASL Version 2.0,The Apache License Version 2.0,The Apache Software License Version 2.0','PERMITTED'), +(6, 'GNU General Public License v2.0 only','GPL-2.0-only','','RESTRICTED'), +(7, 'GNU Lesser General Public License v3.0 or later','LGPL-3.0-or-later','GNU Lesser General Public License v3 or later (LGPLv3+),Lesser General Public License version 3 or greater,LGPLv3+','PROHIBITED'); INSERT INTO license_conflicts (conflict_license_id, repository_license_id) VALUES (1, 3), (1, 6), (2, 6), (2, 3), (3, 5), (3, 7), (5, 6), (6, 7); diff --git a/src/test/java/com/lpvs/service/LPVSLicenseServiceTest.java b/src/test/java/com/lpvs/service/LPVSLicenseServiceTest.java index f2f83a12..6282fc7c 100644 --- a/src/test/java/com/lpvs/service/LPVSLicenseServiceTest.java +++ b/src/test/java/com/lpvs/service/LPVSLicenseServiceTest.java @@ -12,6 +12,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.when; import java.lang.reflect.Field; @@ -153,6 +154,13 @@ public void testFindConflicts() { add(new LPVSLicenseService.Conflict<>("", "")); } }); + + LPVSLicenseRepository lpvsLicenseRepository = Mockito.mock(LPVSLicenseRepository.class); + ReflectionTestUtils.setField( + licenseService, "lpvsLicenseRepository", lpvsLicenseRepository); + when(lpvsLicenseRepository.searchBySpdxId(anyString())).thenReturn(null); + when(lpvsLicenseRepository.searchByAlternativeLicenseNames(anyString())).thenReturn(null); + Assertions.assertNotNull(licenseService.findConflicts(webhookConfig, fileList)); } @@ -425,6 +433,12 @@ void setUp() throws NoSuchFieldException, IllegalAccessException { conflicts_field.setAccessible(true); conflicts_field.set(licenseService, List.of(conflict_1)); + LPVSLicenseRepository lpvsLicenseRepository = Mockito.mock(LPVSLicenseRepository.class); + Field license_repository = + licenseService.getClass().getDeclaredField("lpvsLicenseRepository"); + license_repository.setAccessible(true); + license_repository.set(licenseService, lpvsLicenseRepository); + lpvs_license_1 = new LPVSLicense(1L, null, spdx_id_1, null, null, null); lpvs_file_1 = new LPVSFile( @@ -461,6 +475,10 @@ void setUp() throws NoSuchFieldException, IllegalAccessException { null, null); + when(lpvsLicenseRepository.searchBySpdxId(anyString())).thenReturn(null); + when(lpvsLicenseRepository.searchByAlternativeLicenseNames(anyString())) + .thenReturn(lpvs_license_1); + webhookConfig = new LPVSQueue(); webhookConfig.setRepositoryLicense("MIT"); } diff --git a/src/test/java/com/lpvs/util/LPVSWebhookUtilTest.java b/src/test/java/com/lpvs/util/LPVSWebhookUtilTest.java index 4cef7aef..91294125 100644 --- a/src/test/java/com/lpvs/util/LPVSWebhookUtilTest.java +++ b/src/test/java/com/lpvs/util/LPVSWebhookUtilTest.java @@ -205,7 +205,7 @@ public void checkNull() { () -> { LPVSWebhookUtil.getRepositoryOrganization(null); }); - assertEquals("Webhook is absent", exception.getMessage()); + assertEquals("Webhook Config is absent", exception.getMessage()); exception = assertThrows( @@ -221,7 +221,7 @@ public void checkNull() { () -> { LPVSWebhookUtil.getRepositoryName(null); }); - assertEquals("Webhook is absent", exception.getMessage()); + assertEquals("Webhook Config is absent", exception.getMessage()); exception = assertThrows( @@ -237,7 +237,7 @@ public void checkNull() { () -> { LPVSWebhookUtil.getRepositoryUrl(null); }); - assertEquals("Webhook is absent", exception.getMessage()); + assertEquals("Webhook Config is absent", exception.getMessage()); exception = assertThrows( @@ -245,7 +245,7 @@ public void checkNull() { () -> { LPVSWebhookUtil.getPullRequestId(null); }); - assertEquals("Webhook is absent", exception.getMessage()); + assertEquals("Webhook Config is absent", exception.getMessage()); exception = assertThrows(