feat: Add scanning of local files and folders (single scan) #558

o-kopysov · 2024-07-31T18:43:16Z

Pull Request

Description

Current PR contains:

implementation of the new feature - scanning of local files and folders with the possibility to generate reports in the console or an HTML file
update and extension of unit tests
fixes for several bugs that were found during the creation of new unit tests and local tests of the new feature
update of the documentation

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Code cleanup/refactoring
Documentation update
This change requires a documentation update
CI system update
Test Coverage update

Testing

Unit tests and local functional tests.

Checklist:

My code follows the style guidelines of this project
My code meets the required code coverage for lines (90% and above)
My code meets the required code coverage for branches (80% and above)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Signed-off-by: Oleg Kopysov <[email protected]>

codecov-commenter · 2024-07-31T18:49:18Z

Codecov Report

Attention: Patch coverage is 93.80531% with 7 lines in your changes missing coverage. Please review.

Project coverage is 93.08%. Comparing base (420cc39) to head (dc1381e).

Files	Patch %	Lines
src/main/java/com/lpvs/util/LPVSFileUtil.java	82.60%	0 Missing and 4 partials ⚠️
...va/com/lpvs/service/LPVSQueueProcessorService.java	0.00%	0 Missing and 1 partial ⚠️
.../java/com/lpvs/service/scan/LPVSDetectService.java	98.43%	0 Missing and 1 partial ⚠️
src/main/java/com/lpvs/util/LPVSPayloadUtil.java	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #558      +/-   ##
============================================
+ Coverage     92.64%   93.08%   +0.44%     
- Complexity      530      545      +15     
============================================
  Files            48       48              
  Lines          1753     1822      +69     
  Branches        208      221      +13     
============================================
+ Hits           1624     1696      +72     
+ Misses           64       59       -5     
- Partials         65       67       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Oleg Kopysov <[email protected]>

t-naumenko

LGTM

m-rudyk

Approved

o-kopysov · 2024-08-05T07:28:28Z

@tiokim Please take a look at this PR. I would be grateful for your review and feedback.

tiokim

LGTM!

tiokim · 2024-08-05T07:45:44Z

src/main/java/com/lpvs/service/LPVSQueueProcessorService.java

        // Check for any pending elements in the LPVSQueue.
        queueService.checkForQueue();

        // Process LPVSQueue elements until the trigger is set.
-        while (trigger == null || trigger.isEmpty()) {
+        while ((trigger == null || trigger.isEmpty())


What about using isBlank instead of isEmpty?

tiokim · 2024-08-05T07:46:26Z

src/main/java/com/lpvs/service/scan/LPVSDetectService.java

+        List<LPVSLicenseService.Conflict<String, String>> detectedConflicts = null;
+
+        // Error case when both pull request scan and local files scan are set to true
+        if (trigger != null && !trigger.isEmpty() && localPath != null && !localPath.isEmpty()) {


What about using isBlank instead of isEmpty?

Signed-off-by: Oleg Kopysov <[email protected]>

t-naumenko · 2024-08-06T06:14:21Z

src/main/java/com/lpvs/service/scan/LPVSDetectService.java

+                log.info("Single scan of pull request completed.");
+            } catch (Exception ex) {
+                log.error("Single scan of pull request finished with errors.");
+                log.error("Can't trigger single scan: " + ex.getMessage());


"Single scan finished" and at the same time "Can't trigger single scan"
What about use something general: ""
log.error("Single scan of pull request failed with error: " + ex.getMessage());

t-naumenko · 2024-08-06T06:15:49Z

src/main/java/com/lpvs/service/scan/LPVSDetectService.java

            } catch (Exception ex) {
-                log.error("Single scan finished with errors.");
+                log.error("Single scan of local file(s) finished with errors.");
                log.error("Can't trigger single scan: " + ex.getMessage());


What about:
log.error("Single scan of local file(s) failed with error: " + ex.getMessage());

t-naumenko

There are 2 small suggestions about error logs, but in general LGTM

t-naumenko · 2024-08-06T06:30:03Z

src/main/java/com/lpvs/service/LPVSQueueProcessorService.java

        // Check for any pending elements in the LPVSQueue.
        queueService.checkForQueue();

        // Process LPVSQueue elements until the trigger is set.
-        while (trigger == null || trigger.isEmpty()) {
+        while ((trigger == null || trigger.isBlank())
+                && (localPath == null || localPath.isBlank())) {


localPath.isBlank() includes a case with null, right?
If so, there is no reason to check it for localPath and trigger:

while (trigger.isBlank()) && (localPath.isBlank()) ...

t-naumenko · 2024-08-06T07:12:51Z

src/main/java/com/lpvs/service/scan/LPVSDetectService.java

+        List<LPVSLicenseService.Conflict<String, String>> detectedConflicts = null;
+
+        // Error case when both pull request scan and local files scan are set to true
+        if (trigger != null && !trigger.isBlank() && localPath != null && !localPath.isBlank()) {


As isBlank include check for a null, so it could be simplified to:
if (!trigger.isBlank() && !localPath.isBlank())

Signed-off-by: Oleg Kopysov <[email protected]>

…to local-files-scan

Signed-off-by: Oleg Kopysov <[email protected]>

t-naumenko

Good improvement! Thank you!

feat: Add scanning of local files and folders (single scan)

60ec69c

Signed-off-by: Oleg Kopysov <[email protected]>

o-kopysov added documentation Improvements or additions to documentation new feature labels Jul 31, 2024

o-kopysov added this to the v1.5.3 milestone Jul 31, 2024

o-kopysov requested review from o-konoval, m-rudyk and t-naumenko July 31, 2024 18:43

o-kopysov self-assigned this Jul 31, 2024

o-kopysov requested review from tiokim, MoonkiHong and tdrozdovsky as code owners July 31, 2024 18:43

tdrozdovsky previously approved these changes Jul 31, 2024

View reviewed changes

o-kopysov dismissed tdrozdovsky’s stale review via 16b8214 August 1, 2024 10:17

fix: Improve code coverage

01e536f

Signed-off-by: Oleg Kopysov <[email protected]>

o-kopysov force-pushed the local-files-scan branch from 16b8214 to 01e536f Compare August 1, 2024 11:16

o-kopysov requested a review from tdrozdovsky August 1, 2024 11:17

t-naumenko previously approved these changes Aug 1, 2024

View reviewed changes

m-rudyk previously approved these changes Aug 2, 2024

View reviewed changes

tiokim previously approved these changes Aug 5, 2024

View reviewed changes

fix: Apply fixes after code review

3221d85

Signed-off-by: Oleg Kopysov <[email protected]>

o-kopysov dismissed stale reviews from tiokim, m-rudyk, and t-naumenko via 3221d85 August 5, 2024 08:08

o-kopysov added 3 commits August 5, 2024 11:15

feat: Add scanning of local files and folders (single scan)

74eea15

Signed-off-by: Oleg Kopysov <[email protected]>

fix: Improve code coverage

7675d6f

Signed-off-by: Oleg Kopysov <[email protected]>

fix: Apply fixes after code review

bf6065e

Signed-off-by: Oleg Kopysov <[email protected]>

o-kopysov force-pushed the local-files-scan branch from 3221d85 to bf6065e Compare August 5, 2024 08:15

t-naumenko reviewed Aug 6, 2024

View reviewed changes

t-naumenko previously approved these changes Aug 6, 2024

View reviewed changes

t-naumenko reviewed Aug 6, 2024

View reviewed changes

o-kopysov added 2 commits August 6, 2024 11:37

fix: Apply fixes after code review

8ff3e2a

Signed-off-by: Oleg Kopysov <[email protected]>

Merge branch 'local-files-scan' of https://github.com/Samsung/LPVS in…

a0cada0

…to local-files-scan

o-kopysov dismissed t-naumenko’s stale review via a0cada0 August 6, 2024 08:45

fix: Update unit tests and fix the build process

dc1381e

Signed-off-by: Oleg Kopysov <[email protected]>

o-kopysov requested review from t-naumenko, tiokim and m-rudyk August 6, 2024 11:35

t-naumenko approved these changes Aug 6, 2024

View reviewed changes

tdrozdovsky approved these changes Aug 7, 2024

View reviewed changes

o-kopysov merged commit 57c5e2b into main Aug 7, 2024
10 checks passed

o-kopysov deleted the local-files-scan branch August 7, 2024 07:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add scanning of local files and folders (single scan) #558

feat: Add scanning of local files and folders (single scan) #558

o-kopysov commented Jul 31, 2024

codecov-commenter commented Jul 31, 2024 •

edited

Loading

t-naumenko left a comment

m-rudyk left a comment

o-kopysov commented Aug 5, 2024

tiokim left a comment

tiokim Aug 5, 2024

tiokim Aug 5, 2024

t-naumenko Aug 6, 2024

t-naumenko Aug 6, 2024

t-naumenko left a comment

t-naumenko Aug 6, 2024 •

edited

Loading

t-naumenko Aug 6, 2024

t-naumenko left a comment

feat: Add scanning of local files and folders (single scan) #558

feat: Add scanning of local files and folders (single scan) #558

Conversation

o-kopysov commented Jul 31, 2024

Pull Request

Description

Type of change

Testing

Checklist:

codecov-commenter commented Jul 31, 2024 • edited Loading

Codecov Report

t-naumenko left a comment

Choose a reason for hiding this comment

m-rudyk left a comment

Choose a reason for hiding this comment

o-kopysov commented Aug 5, 2024

tiokim left a comment

Choose a reason for hiding this comment

tiokim Aug 5, 2024

Choose a reason for hiding this comment

tiokim Aug 5, 2024

Choose a reason for hiding this comment

t-naumenko Aug 6, 2024

Choose a reason for hiding this comment

t-naumenko Aug 6, 2024

Choose a reason for hiding this comment

t-naumenko left a comment

Choose a reason for hiding this comment

t-naumenko Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

t-naumenko Aug 6, 2024

Choose a reason for hiding this comment

t-naumenko left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 31, 2024 •

edited

Loading

t-naumenko Aug 6, 2024 •

edited

Loading