Skip to content

Commit

Permalink
refactor: improve MCD (Module Circular Dependency) metric of the code (
Browse files Browse the repository at this point in the history
…#615)

Signed-off-by: Oleg Kopysov <o.kopysov@samsung.com>
  • Loading branch information
o-kopysov authored Sep 18, 2024
1 parent 149c5a0 commit b63cb7a
Show file tree
Hide file tree
Showing 11 changed files with 230 additions and 246 deletions.
59 changes: 59 additions & 0 deletions src/main/java/com/lpvs/entity/LPVSConflict.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Copyright (c) 2024, Samsung Electronics Co., Ltd. All rights reserved.
*
* Use of this source code is governed by a MIT license that can be
* found in the LICENSE file.
*/
package com.lpvs.entity;

import lombok.AllArgsConstructor;
import lombok.Getter;

import java.util.Objects;

/**
* Represents a license conflict between two licenses.
*
* @param <License1> Type of the first license.
* @param <License2> Type of the second license.
*/
@Getter
@AllArgsConstructor
public class LPVSConflict<License1, License2> {

/**
* The first license in the conflict.
*/
private final License1 l1;

/**
* The second license in the conflict.
*/
private final License2 l2;

/**
* Compares this LPVSConflict object with another object for equality.
*
* @param o The object to compare with this LPVSConflict.
* @return {@code true} if the objects are equal, {@code false} otherwise.
*/
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
LPVSConflict<?, ?> conflict = (LPVSConflict<?, ?>) o;
return (l1.equals(conflict.l1) && l2.equals(conflict.l2))
|| (l1.equals(conflict.l2) && l2.equals(conflict.l1));
}

/**
* Generates a hash code value for this LPVSConflict object.
* The hash code is computed based on the hash codes of the two licenses.
*
* @return A hash code value for this LPVSConflict object.
*/
@Override
public int hashCode() {
return Objects.hash(l1, l2);
}
}
49 changes: 24 additions & 25 deletions src/main/java/com/lpvs/entity/report/LPVSReportBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
import com.lpvs.entity.LPVSLicense;
import com.lpvs.entity.LPVSQueue;
import com.lpvs.entity.enums.LPVSVcs;
import com.lpvs.service.LPVSLicenseService;
import com.lpvs.entity.LPVSConflict;
import com.lpvs.util.LPVSCommentUtil;
import io.micrometer.common.util.StringUtils;
import lombok.AllArgsConstructor;
import lombok.Getter;
Expand All @@ -30,8 +31,6 @@
import java.util.*;
import java.util.stream.Collectors;

import static com.lpvs.util.LPVSCommentUtil.getMatchedLinesAsLink;

/**
* A class responsible for building reports based on the results of license scanning.
*/
Expand Down Expand Up @@ -106,7 +105,7 @@ private static class GroupInfo<T> {
public String generateHtmlReportSingleScan(
String path,
List<LPVSFile> scanResults,
List<LPVSLicenseService.Conflict<String, String>> conflicts,
List<LPVSConflict<String, String>> conflicts,
LPVSQueue webhookConfig,
LPVSVcs vcs) {
Context context = new Context();
Expand Down Expand Up @@ -159,9 +158,7 @@ public String generateHtmlReportSingleScan(
* @return A string containing scan results in command line friendly format.
*/
public String generateCommandLineComment(
String path,
List<LPVSFile> scanResults,
List<LPVSLicenseService.Conflict<String, String>> conflicts) {
String path, List<LPVSFile> scanResults, List<LPVSConflict<String, String>> conflicts) {
StringBuilder commentBuilder = new StringBuilder();
String date = sdf.format(new Date());
commentBuilder.append("\n");
Expand Down Expand Up @@ -221,8 +218,8 @@ public String generateCommandLineComment(
if (conflicts != null && !conflicts.isEmpty()) {
commentBuilder.append(
"Potential license conflict(s) detected: " + conflicts.size() + "\n");
for (LPVSLicenseService.Conflict<String, String> conflict : conflicts) {
commentBuilder.append(" - " + conflict.l1 + " and " + conflict.l2 + "\n");
for (LPVSConflict<String, String> conflict : conflicts) {
commentBuilder.append(" - " + conflict.getL1() + " and " + conflict.getL2() + "\n");
}
} else {
commentBuilder.append("No license conflicts detected.\n");
Expand Down Expand Up @@ -259,7 +256,7 @@ public static void saveHTMLToFile(String htmlContent, String filePath) {
*/
public String generatePullRequestComment(
List<LPVSFile> scanResults,
List<LPVSLicenseService.Conflict<String, String>> conflicts,
List<LPVSConflict<String, String>> conflicts,
LPVSQueue webhookConfig,
LPVSVcs vcs) {

Expand Down Expand Up @@ -346,7 +343,7 @@ private String generateLicenseTable(
* @return the code of the generated license conflicts table
*/
private String generateLicenseConflictsTable(
List<LPVSLicenseService.Conflict<String, String>> conflicts, LPVSVcs vcs) {
List<LPVSConflict<String, String>> conflicts, LPVSVcs vcs) {
if (vcs != null && vcs.equals(LPVSVcs.GITHUB)) {
return "<details>"
+ "\n"
Expand Down Expand Up @@ -407,8 +404,7 @@ private String generateLicenseTableMD(
* @param conflicts a list of license conflicts
* @return the MD content for the license conflicts table
*/
private String generateLicenseConflictsTableMD(
List<LPVSLicenseService.Conflict<String, String>> conflicts) {
private String generateLicenseConflictsTableMD(List<LPVSConflict<String, String>> conflicts) {
StringBuilder mdBuilder = new StringBuilder();
mdBuilder
.append("|Conflict>")
Expand All @@ -417,14 +413,14 @@ private String generateLicenseConflictsTableMD(
.append("|-|-|")
.append("\n");

for (LPVSLicenseService.Conflict<String, String> conflict : conflicts) {
for (LPVSConflict<String, String> conflict : conflicts) {
mdBuilder
.append("|")
.append(conflict.l1)
.append(conflict.getL1())
.append(" and ")
.append(conflict.l2)
.append(conflict.getL2())
.append("|")
.append(getExplanationForLicenseConflict(conflict.l1, conflict.l2))
.append(getExplanationForLicenseConflict(conflict.getL1(), conflict.getL2()))
.append("|")
.append("\n");
}
Expand Down Expand Up @@ -478,7 +474,9 @@ private void addBlockOfTableForLicenseTypeMD(
.append("|")
.append(fileInfo.getComponentFilePath())
.append("|")
.append(getMatchedLinesAsLink(webhookConfig, fileInfo, vcs))
.append(
LPVSCommentUtil.getMatchedLinesAsLink(
webhookConfig, fileInfo, vcs))
.append("|")
.append(fileInfo.getSnippetMatch())
.append("|")
Expand All @@ -495,8 +493,7 @@ private void addBlockOfTableForLicenseTypeMD(
* @param conflicts a list of license conflicts
* @return the HTML content for the license conflicts table
*/
private String generateLicenseConflictsTableHTML(
List<LPVSLicenseService.Conflict<String, String>> conflicts) {
private String generateLicenseConflictsTableHTML(List<LPVSConflict<String, String>> conflicts) {
StringBuilder htmlBuilder = new StringBuilder();
htmlBuilder.append("<table>");
htmlBuilder
Expand All @@ -505,16 +502,16 @@ private String generateLicenseConflictsTableHTML(
.append("<th>Explanation</th>")
.append("<tr>");

for (LPVSLicenseService.Conflict<String, String> conflict : conflicts) {
for (LPVSConflict<String, String> conflict : conflicts) {
htmlBuilder
.append("<tr>")
.append("<td>")
.append(conflict.l1)
.append(conflict.getL1())
.append(" and ")
.append(conflict.l2)
.append(conflict.getL2())
.append("</td>")
.append("<td>")
.append(getExplanationForLicenseConflict(conflict.l1, conflict.l2))
.append(getExplanationForLicenseConflict(conflict.getL1(), conflict.getL2()))
.append("</td>")
.append("</tr>");
}
Expand Down Expand Up @@ -691,7 +688,9 @@ private void addBlockOfTableForLicenseTypeHTML(

htmlBuilder
.append("</td><td>")
.append(getMatchedLinesAsLink(webhookConfig, fileInfo, vcs))
.append(
LPVSCommentUtil.getMatchedLinesAsLink(
webhookConfig, fileInfo, vcs))
.append("</td><td>")
.append(fileInfo.getSnippetMatch())
.append("</td>");
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/com/lpvs/service/LPVSGitHubService.java
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public void setErrorCheck(LPVSQueue webhookConfig) {
public void commentResults(
LPVSQueue webhookConfig,
List<LPVSFile> scanResults,
List<LPVSLicenseService.Conflict<String, String>> conflicts,
List<LPVSConflict<String, String>> conflicts,
LPVSPullRequest lpvsPullRequest)
throws Exception {

Expand Down Expand Up @@ -287,16 +287,16 @@ public void commentResults(

if (conflicts != null && !conflicts.isEmpty()) {
hasConflicts = true;
for (LPVSLicenseService.Conflict<String, String> conflict : conflicts) {
for (LPVSConflict<String, String> conflict : conflicts) {
LPVSDetectedLicense detectedIssue = new LPVSDetectedLicense();
detectedIssue.setPullRequest(lpvsPullRequest);
Long l1 =
lpvsLicenseRepository
.findFirstBySpdxIdOrderByLicenseIdDesc(conflict.l1)
.findFirstBySpdxIdOrderByLicenseIdDesc(conflict.getL1())
.getLicenseId();
Long l2 =
lpvsLicenseRepository
.findFirstBySpdxIdOrderByLicenseIdDesc(conflict.l2)
.findFirstBySpdxIdOrderByLicenseIdDesc(conflict.getL2())
.getLicenseId();
detectedIssue.setLicenseConflict(
lpvsLicenseConflictRepository.findLicenseConflict(l1, l2));
Expand Down
85 changes: 13 additions & 72 deletions src/main/java/com/lpvs/service/LPVSLicenseService.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
*/
package com.lpvs.service;

import com.lpvs.entity.LPVSFile;
import com.lpvs.entity.LPVSLicense;
import com.lpvs.entity.LPVSLicenseConflict;
import com.lpvs.entity.LPVSQueue;
import com.lpvs.entity.*;
import com.lpvs.repository.LPVSLicenseConflictRepository;
import com.lpvs.repository.LPVSLicenseRepository;
import com.lpvs.util.LPVSExitHandler;
Expand Down Expand Up @@ -67,7 +64,7 @@ public class LPVSLicenseService {
/**
* List of license conflicts loaded from the database.
*/
private List<Conflict<String, String>> licenseConflicts = new ArrayList<>();
private List<LPVSConflict<String, String>> licenseConflicts = new ArrayList<>();

/**
* Handler for exiting the application.
Expand Down Expand Up @@ -121,8 +118,8 @@ public LPVSLicenseService(
private void loadLicenseConflicts() {
List<LPVSLicenseConflict> conflicts = lpvsLicenseConflictRepository.findAll();
for (LPVSLicenseConflict conflict : conflicts) {
Conflict<String, String> conf =
new Conflict<>(
LPVSConflict<String, String> conf =
new LPVSConflict<>(
conflict.getConflictLicense().getSpdxId(),
conflict.getRepositoryLicense().getSpdxId());
if (!licenseConflicts.contains(conf)) {
Expand Down Expand Up @@ -336,7 +333,7 @@ public LPVSLicense findLicense(String licenseSpdxId, String licenseName) {
* @param license2 The second license in conflict.
*/
public void addLicenseConflict(String license1, String license2) {
Conflict<String, String> conf = new Conflict<>(license1, license2);
LPVSConflict<String, String> conf = new LPVSConflict<>(license1, license2);
if (!licenseConflicts.contains(conf)) {
licenseConflicts.add(conf);
}
Expand All @@ -349,9 +346,9 @@ public void addLicenseConflict(String license1, String license2) {
* @param scanResults List of scanned files.
* @return List of license conflicts found.
*/
public List<Conflict<String, String>> findConflicts(
public List<LPVSConflict<String, String>> findConflicts(
LPVSQueue webhookConfig, List<LPVSFile> scanResults) {
List<Conflict<String, String>> foundConflicts = new ArrayList<>();
List<LPVSConflict<String, String>> foundConflicts = new ArrayList<>();

if (scanResults.isEmpty() || licenseConflicts.isEmpty()) {
return foundConflicts;
Expand Down Expand Up @@ -383,9 +380,9 @@ public List<Conflict<String, String>> findConflicts(
}

for (String detectedLicenseUnique : detectedLicensesUnique) {
for (Conflict<String, String> licenseConflict : licenseConflicts) {
Conflict<String, String> possibleConflict =
new Conflict<>(detectedLicenseUnique, repositoryLicense);
for (LPVSConflict<String, String> licenseConflict : licenseConflicts) {
LPVSConflict<String, String> possibleConflict =
new LPVSConflict<>(detectedLicenseUnique, repositoryLicense);
if (licenseConflict.equals(possibleConflict)) {
foundConflicts.add(possibleConflict);
}
Expand All @@ -396,9 +393,9 @@ public List<Conflict<String, String>> findConflicts(
// 2. Check conflict between detected licenses
for (int i = 0; i < detectedLicensesUnique.size(); i++) {
for (int j = i + 1; j < detectedLicensesUnique.size(); j++) {
for (Conflict<String, String> licenseConflict : licenseConflicts) {
Conflict<String, String> possibleConflict =
new Conflict<>(
for (LPVSConflict<String, String> licenseConflict : licenseConflicts) {
LPVSConflict<String, String> possibleConflict =
new LPVSConflict<>(
(String) detectedLicensesUnique.toArray()[i],
(String) detectedLicensesUnique.toArray()[j]);
if (licenseConflict.equals(possibleConflict)) {
Expand All @@ -411,62 +408,6 @@ public List<Conflict<String, String>> findConflicts(
return foundConflicts;
}

/**
* Represents a license conflict between two licenses.
*
* @param <License1> Type of the first license.
* @param <License2> Type of the second license.
*/
public static class Conflict<License1, License2> {

/**
* The first license in the conflict.
*/
public License1 l1;

/**
* The second license in the conflict.
*/
public License2 l2;

/**
* Constructs a Conflict object with the specified licenses.
*
* @param l1 The first license.
* @param l2 The second license.
*/
public Conflict(License1 l1, License2 l2) {
this.l1 = l1;
this.l2 = l2;
}

/**
* Compares this Conflict object with another object for equality.
*
* @param o The object to compare with this Conflict.
* @return {@code true} if the objects are equal, {@code false} otherwise.
*/
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Conflict<?, ?> conflict = (Conflict<?, ?>) o;
return (l1.equals(conflict.l1) && l2.equals(conflict.l2))
|| (l1.equals(conflict.l2) && l2.equals(conflict.l1));
}

/**
* Generates a hash code value for this Conflict object.
* The hash code is computed based on the hash codes of the two licenses.
*
* @return A hash code value for this Conflict object.
*/
@Override
public int hashCode() {
return Objects.hash(l1, l2);
}
}

/**
* The OsoriConnection class provides methods for creating a connection to the OSORI database.
*/
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/lpvs/service/scan/LPVSDetectService.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.List;

import com.lpvs.entity.report.LPVSReportBuilder;
import com.lpvs.entity.LPVSConflict;
import com.lpvs.service.LPVSGitHubConnectionService;
import com.lpvs.service.LPVSGitHubService;
import com.lpvs.service.LPVSLicenseService;
Expand Down Expand Up @@ -126,7 +127,7 @@ public void runSingleScan() {
boolean generateReport = false;
LPVSQueue webhookConfig = null;
List<LPVSFile> scanResult = null;
List<LPVSLicenseService.Conflict<String, String>> detectedConflicts = null;
List<LPVSConflict<String, String>> detectedConflicts = null;
String path = null;

// Error case when both pull request scan and local files scan are set to true
Expand Down
Loading

0 comments on commit b63cb7a

Please sign in to comment.