-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Initial version of HTML report generation for single scan #573
Conversation
Signed-off-by: Oleg Kopysov <o.kopysov@samsung.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #573 +/- ##
===========================================
+ Coverage 0 93.04% +93.04%
- Complexity 0 575 +575
===========================================
Files 0 49 +49
Lines 0 1999 +1999
Branches 0 231 +231
===========================================
+ Hits 0 1860 +1860
- Misses 0 66 +66
- Partials 0 73 +73 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Oleg Kopysov <o.kopysov@samsung.com>
@tiokim do you have any comments regarding PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
@@ -38,6 +38,11 @@ | |||
</properties> | |||
|
|||
<dependencies> | |||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's check solution with 'ConfigurableApplicationContext'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check comments and suggestion. Some part need to be simplified.
private String scannerType; | ||
|
||
private final SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); | ||
private Map<String, GroupInfo<Map<String, GroupInfo<Map<String, GroupInfo<List<LPVSFile>>>>>>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to create custom classes for this structure:
'ComponentInfo'
'AccessInfo'
'LicenseInfo'
if (detectedLicenseInfo == null && scanResults != null && !scanResults.isEmpty()) { | ||
List<LPVSFile> filesScanResults = getLpvsFilesFromScanResults(scanResults); | ||
|
||
detectedLicenseInfo = | ||
filesScanResults.stream() | ||
.collect( | ||
Collectors.groupingBy( | ||
LPVSFile -> | ||
LPVSFile.getLicenses().stream() | ||
.findFirst() | ||
.get() | ||
.getAccess(), | ||
Collectors.collectingAndThen( | ||
Collectors.groupingBy( | ||
LPVSFile -> | ||
LPVSFile.getLicenses().stream() | ||
.findFirst() | ||
.get() | ||
.getSpdxId(), | ||
Collectors.collectingAndThen( | ||
Collectors.groupingBy( | ||
LPVSFile -> | ||
LPVSFile | ||
.getComponentVendor() | ||
+ " / " | ||
+ LPVSFile | ||
.getComponentName() | ||
+ ":::" | ||
+ LPVSFile | ||
.getComponentUrl(), | ||
Collectors | ||
.collectingAndThen( | ||
Collectors | ||
.toList(), | ||
elements -> | ||
new GroupInfo<>( | ||
elements | ||
.size(), | ||
elements))), | ||
groupedByComponent -> | ||
new GroupInfo<>( | ||
groupedByComponent | ||
.values() | ||
.stream() | ||
.mapToLong( | ||
GroupInfo | ||
::getCount) | ||
.sum(), | ||
groupedByComponent))), | ||
groupedByLicense -> | ||
new GroupInfo<>( | ||
groupedByLicense | ||
.values() | ||
.stream() | ||
.mapToLong( | ||
GroupInfo | ||
::getCount) | ||
.sum(), | ||
groupedByLicense)))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to much for single instruction. Split required here.
How about:
Map<String, GroupInfo<?>> detectedLicenseInfo = filesScanResults.stream()
.collect(Collectors.groupingBy(
this::getFirstLicenseAccess,
Collectors.collectingAndThen(
Collectors.groupingBy(
this::getFirstLicenseSpdxId,
Collectors.collectingAndThen(
Collectors.groupingBy(
this::getComponentKey,
Collectors.collectingAndThen(
Collectors.toList(),
this::createGroupInfo
)
),
this::summarizeGroupInfo
)
),
this::summarizeGroupInfo
)
));
and after that implement corresponding methods?
together with created classes above it will be much easier to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review! Overall it looks great!
|
||
@BeforeEach | ||
public void setUp() throws Exception { | ||
lic1 = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if the variable name is better named around the functionality like licMIT instead of simple number.
Pull Request
Description
The current PR contains improvements in the HTML reports generated by a single scan (scan of PR or local file(s)).
Type of change
Please delete options that are not relevant.
Testing
Checklist: