Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor: LPVSScanossDetectService refactoring and unit test fix #524

Merged
merged 4 commits into from
Jun 3, 2024
+167 −97
Merged

refactor: LPVSScanossDetectService refactoring and unit test fix #524

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82523c6
fix: correct the path to read the real file in the unit tests.
tiokim May 12, 2024
cb3f15f
test: Add an unit test to check the exception
tiokim May 30, 2024
b148997
refactor: Refactor checkLicenses by extract method
tiokim May 31, 2024
3df2eab
chore: Add javadoc comments and small fix for Gson
tiokim Jun 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 98 additions & 88 deletions src/main/java/com/lpvs/service/scan/scanner/LPVSScanossDetectService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,107 +147,27 @@ public List<LPVSFile> checkLicenses(LPVSQueue webhookConfig) {
List<LPVSFile> detectedFiles = new ArrayList<>();
try {
Gson gson = new Gson();
Reader reader;
if (webhookConfig.getHeadCommitSHA() == null
|| webhookConfig.getHeadCommitSHA().equals("")) {
reader =
Files.newBufferedReader(
Paths.get(
System.getProperty("user.home")
+ File.separator
+ "Results"
+ File.separator
+ LPVSPayloadUtil.getRepositoryName(webhookConfig)
+ File.separator
+ LPVSPayloadUtil.getPullRequestId(webhookConfig)
+ ".json"));
} else {
reader =
Files.newBufferedReader(
Paths.get(
System.getProperty("user.home")
+ File.separator
+ "Results"
+ File.separator
+ LPVSPayloadUtil.getRepositoryName(webhookConfig)
+ File.separator
+ webhookConfig.getHeadCommitSHA()
+ ".json"));
}
Reader reader = getReader(webhookConfig);

// convert JSON file to map
Map<String, ArrayList<Object>> map =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to avoid usage of gson object here:
Map<String, ArrayList<Object>> map = new Gson().fromJson(reader, new TypeToken<Map<String, ArrayList<Object>>>() {}.getType());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks for your precise review!

gson.fromJson(
reader, new TypeToken<Map<String, ArrayList<Object>>>() {}.getType());

// parse map entries
long ind = 0L;
if (null == map) {
log.error("Error parsing Json File");
return detectedFiles;
}

// parse map entries
long ind = 0L;
for (Map.Entry<String, ArrayList<Object>> entry : map.entrySet()) {
LPVSFile file = new LPVSFile();
file.setId(ind++);
file.setFilePath(entry.getKey().toString());

String content =
entry.getValue()
.toString()
.replaceAll("=\\[", "\" : [")
.replaceAll("=", "\" : \"")
.replaceAll("\\}, \\{", "\"},{")
.replaceAll("\\}\\],", "\"}],")
.replaceAll("\\{", "{\"")
.replaceAll(", ", "\", \"")
.replaceAll("\\]\"", "]")
.replaceAll("}\",", "\"},")
.replaceAll(": \\[", ": [\"")
.replaceAll("],", "\"],")
.replaceAll("\"\\{\"", "{\"")
.replaceAll("\"}\"]", "\"}]")
.replaceAll("\\[\"\"\\]", "[]")
.replaceAll(
"incompatible_with\" : (\".*?\"), \"name",
"incompatible_with\" : \\[$1\\], \"name");
content = content.substring(1, content.length() - 1);
if (content.endsWith("}")) {
content = content.substring(0, content.length() - 1) + "\"}";
}
content = content.replaceAll("}\"}", "\"}}");
ScanossJsonStructure object = gson.fromJson(content, ScanossJsonStructure.class);
if (object.id != null) file.setSnippetType(object.id);
if (object.matched != null) file.setSnippetMatch(object.matched);
if (object.lines != null) file.setMatchedLines(object.lines);
if (object.file != null) file.setComponentFilePath(object.file);
if (object.file_url != null) file.setComponentFileUrl(object.file_url);
if (object.component != null) file.setComponentName(object.component);
if (object.oss_lines != null) file.setComponentLines(object.oss_lines);
if (object.url != null) file.setComponentUrl(object.url);
if (object.version != null) file.setComponentVersion(object.version);
if (object.vendor != null) file.setComponentVendor(object.vendor);

Set<LPVSLicense> licenses = new HashSet<>();
if (object.licenses != null) {
for (ScanossJsonStructure.ScanossLicense license : object.licenses) {
// Check detected licenses
LPVSLicense lic =
licenseService.getLicenseBySpdxIdAndName(
license.name, Optional.empty());
lic.setChecklistUrl(license.checklist_url);
licenses.add(lic);

// Check for the license conflicts if the property
// "license_conflict=scanner"
if (licenseService.licenseConflictsSource.equalsIgnoreCase("scanner")) {
if (license.incompatible_with != null) {
for (String incompatibleLicense : license.incompatible_with) {
licenseService.addLicenseConflict(
incompatibleLicense, license.name);
}
}
}
}
}
String content = buildContent(entry);
ScanossJsonStructure object = getScanossJsonStructure(gson, content, file);
Set<LPVSLicense> licenses = buildLicenseSet(object);
file.setLicenses(new HashSet<>(licenses));
if (!file.getLicenses().isEmpty()) detectedFiles.add(file);
}
Expand All @@ -261,6 +181,96 @@ public List<LPVSFile> checkLicenses(LPVSQueue webhookConfig) {
return detectedFiles;
}

private Set<LPVSLicense> buildLicenseSet(ScanossJsonStructure object) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Javadoc comments for all functions (including private).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc comments generated by GenAI are added. Please let me know if I have anything to revise!

if (object.licenses == null) {
return new HashSet<>();
}

Set<LPVSLicense> licenses = new HashSet<>();
for (ScanossJsonStructure.ScanossLicense license : object.licenses) {
// Check detected licenses
LPVSLicense lic =
licenseService.getLicenseBySpdxIdAndName(license.name, Optional.empty());
lic.setChecklistUrl(license.checklist_url);
licenses.add(lic);

// Check for the license conflicts if the property
// "license_conflict=scanner"
if (licenseService.licenseConflictsSource.equalsIgnoreCase("scanner")) {
if (license.incompatible_with != null) {
for (String incompatibleLicense : license.incompatible_with) {
licenseService.addLicenseConflict(incompatibleLicense, license.name);
}
}
}
}
return licenses;
}

private ScanossJsonStructure getScanossJsonStructure(Gson gson, String content, LPVSFile file) {
ScanossJsonStructure object = gson.fromJson(content, ScanossJsonStructure.class);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to replace input parameter Gson gson and avoid usage of gson object like this:
ScanossJsonStructure object = new Gson().fromJson(content, ScanossJsonStructure.class);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks for your precise review!

if (object.id != null) file.setSnippetType(object.id);
if (object.matched != null) file.setSnippetMatch(object.matched);
if (object.lines != null) file.setMatchedLines(object.lines);
if (object.file != null) file.setComponentFilePath(object.file);
if (object.file_url != null) file.setComponentFileUrl(object.file_url);
if (object.component != null) file.setComponentName(object.component);
if (object.oss_lines != null) file.setComponentLines(object.oss_lines);
if (object.url != null) file.setComponentUrl(object.url);
if (object.version != null) file.setComponentVersion(object.version);
if (object.vendor != null) file.setComponentVendor(object.vendor);
return object;
}

private static Reader getReader(LPVSQueue webhookConfig) throws IOException {
String fileName = null;
if (webhookConfig.getHeadCommitSHA() == null
|| webhookConfig.getHeadCommitSHA().isBlank()) {
fileName = LPVSPayloadUtil.getPullRequestId(webhookConfig);
} else {
fileName = webhookConfig.getHeadCommitSHA();
}

return Files.newBufferedReader(
Paths.get(
System.getProperty("user.home")
+ File.separator
+ "Results"
+ File.separator
+ LPVSPayloadUtil.getRepositoryName(webhookConfig)
+ File.separator
+ fileName
+ ".json"));
}

private static String buildContent(Map.Entry<String, ArrayList<Object>> entry) {
String content =
entry.getValue()
.toString()
.replaceAll("=\\[", "\" : [")
.replaceAll("=", "\" : \"")
.replaceAll("\\}, \\{", "\"},{")
.replaceAll("\\}\\],", "\"}],")
.replaceAll("\\{", "{\"")
.replaceAll(", ", "\", \"")
.replaceAll("\\]\"", "]")
.replaceAll("}\",", "\"},")
.replaceAll(": \\[", ": [\"")
.replaceAll("],", "\"],")
.replaceAll("\"\\{\"", "{\"")
.replaceAll("\"}\"]", "\"}]")
.replaceAll("\\[\"\"\\]", "[]")
.replaceAll(
"incompatible_with\" : (\".*?\"), \"name",
"incompatible_with\" : \\[$1\\], \"name");
content = content.substring(1, content.length() - 1);
if (content.endsWith("}")) {
content = content.substring(0, content.length() - 1) + "\"}";
}
content = content.replaceAll("}\"}", "\"}}");
return content;
}

/**
* Represents the JSON structure of Scanoss scan results.
*/
Expand Down
46 changes: 40 additions & 6 deletions src/test/java/com/lpvs/service/scan/scanner/LPVSScanossDetectServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.MockedConstruction;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.springframework.boot.test.system.CapturedOutput;
import org.springframework.boot.test.system.OutputCaptureExtension;
import org.springframework.test.util.ReflectionTestUtils;

import java.io.*;
Expand All @@ -28,6 +31,8 @@
import java.nio.file.StandardCopyOption;
import java.util.Objects;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -123,11 +128,20 @@ public void testWithNullHeadCommitSHA() {
Mockito.when(webhookConfig.getHeadCommitSHA()).thenReturn(null);
Mockito.when(webhookConfig.getRepositoryUrl())
.thenReturn("https://github.com/Samsung/LPVS");
Mockito.when(LPVSPayloadUtil.getRepositoryName(webhookConfig)).thenReturn("A");
Mockito.when(webhookConfig.getPullRequestUrl()).thenReturn("A/B");
Mockito.when(LPVSPayloadUtil.getRepositoryName(webhookConfig)).thenReturn("C");
Mockito.when(webhookConfig.getPullRequestUrl()).thenReturn("A_B");
ReflectionTestUtils.setField(
licenseService, "licenseConflictsSource", licenseConflictsSource);
Assertions.assertNotNull(scanossDetectService.checkLicenses(webhookConfig));
Mockito.when(licenseService.getLicenseBySpdxIdAndName(anyString(), any()))
.thenReturn(
new LPVSLicense() {
{
setLicenseName("MIT");
setLicenseId(1L);
setSpdxId("MIT");
}
});
Assertions.assertEquals(scanossDetectService.checkLicenses(webhookConfig).size(), 3);
}

@Test
Expand All @@ -137,11 +151,31 @@ public void testWithNullHeadCommitSHADB() {
Mockito.when(webhookConfig.getHeadCommitSHA()).thenReturn(null);
Mockito.when(webhookConfig.getRepositoryUrl())
.thenReturn("https://github.com/Samsung/LPVS");
Mockito.when(webhookConfig.getPullRequestUrl()).thenReturn("A/B");
Mockito.when(LPVSPayloadUtil.getRepositoryName(webhookConfig)).thenReturn("A");
Mockito.when(webhookConfig.getPullRequestUrl()).thenReturn("A_B");
Mockito.when(LPVSPayloadUtil.getRepositoryName(webhookConfig)).thenReturn("C");
ReflectionTestUtils.setField(
licenseService, "licenseConflictsSource", licenseConflictsSource);
Assertions.assertNotNull(scanossDetectService.checkLicenses(webhookConfig));

Mockito.when(licenseService.getLicenseBySpdxIdAndName(anyString(), any()))
.thenReturn(
new LPVSLicense() {
{
setLicenseName("MIT");
setLicenseId(1L);
setSpdxId("MIT");
}
});
Assertions.assertEquals(scanossDetectService.checkLicenses(webhookConfig).size(), 3);
}

@Test
@ExtendWith(OutputCaptureExtension.class)
public void testCheckLicenseException(CapturedOutput capturedOutput) {
Mockito.when(lpvsQueue.getHeadCommitSHA()).thenReturn("AB");
Mockito.when(lpvsQueue.getRepositoryUrl()).thenReturn("https://github.com/Samsung/LPVS");
scanossDetectService.checkLicenses(lpvsQueue);
assertThat(
capturedOutput.toString(), containsString("Error while processing Webhook ID = "));
}

@Test
Expand Down
Loading