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

Issue 197. Fix handling of optional package fields #198

Merged
merged 3 commits into from
Sep 7, 2023
Merged

Issue 197. Fix handling of optional package fields #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6e9c90
Issue 197. Fix hanlding of optional package fields
Sep 6, 2023
20b0b19
Restore original formatting
Sep 6, 2023
095e801
Additional formatting
Sep 6, 2023
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
79 changes: 34 additions & 45 deletions src/main/java/org/spdx/library/model/SpdxPackage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,6 @@ public Optional<SpdxPackageVerificationCode> getPackageVerificationCode() throws
* @throws InvalidSPDXAnalysisException
*/
public SpdxPackage setPackageVerificationCode(SpdxPackageVerificationCode verificationCode) throws InvalidSPDXAnalysisException {
if (strict && Objects.isNull(verificationCode) && isFilesAnalyzed()) {
throw new InvalidSPDXAnalysisException("Can not set required verificationCode to null");
}
setPropertyValue(SpdxConstants.PROP_PACKAGE_VERIFICATION_CODE, verificationCode);
return this;
}
Expand Down Expand Up @@ -606,19 +603,13 @@ protected List<String> _verify(Set<String> verifiedIds, String specVersion) {

// files depends on if the filesAnalyzed flag
try {
if (getFiles().size() == 0) {
if (filesAnalyzed) {
Copy link
Member

Choose a reason for hiding this comment

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

I noticed we removed the check for files being present when files analyzed is true.

I went back and read the spec, and indeed there is no stated requirement that files be present - so this change is correct.

retval.add("Missing required package files for "+pkgName);
}
} else {
if (!filesAnalyzed) {
retval.add("Warning: Found analyzed files for package "+pkgName+" when analyzedFiles is set to false.");
}
for (SpdxFile file:getFiles()) {
List<String> verify = file.verify(verifiedIds, specVersion);
addNameToWarnings(verify);
retval.addAll(verify);
}
if (getFiles().size() != 0 && !filesAnalyzed) {
retval.add("Warning: Found analyzed files for package " + pkgName + " when analyzedFiles is set to false.");
}
for (SpdxFile file:getFiles()) {
List<String> verify = file.verify(verifiedIds, specVersion);
addNameToWarnings(verify);
retval.addAll(verify);
}
} catch (InvalidSPDXAnalysisException e) {
retval.add("Invalid package files: "+e.getMessage());
Expand All @@ -627,11 +618,11 @@ protected List<String> _verify(Set<String> verifiedIds, String specVersion) {
// verification code
try {
Optional<SpdxPackageVerificationCode> verificationCode = this.getPackageVerificationCode();
if (!verificationCode.isPresent() && filesAnalyzed) {
retval.add("Missing required package verification code for package " + pkgName);
} else if (verificationCode.isPresent() && !verificationCode.get().getValue().isEmpty() && !filesAnalyzed) {
if (verificationCode.isPresent()
&& !verificationCode.get().getValue().isEmpty()
&& !filesAnalyzed) {
retval.add("Verification code must not be included when files not analyzed.");
} else if (filesAnalyzed) {
} else if (filesAnalyzed && verificationCode.isPresent()) {
List<String> verify = verificationCode.get().verify(verifiedIds, specVersion);
addNameToWarnings(verify);
retval.addAll(verify);
Expand Down Expand Up @@ -730,32 +721,30 @@ protected List<String> _verify(Set<String> verifiedIds, String specVersion) {
}

private void verifyLicenseInfosInFiles(Collection<AnyLicenseInfo> licenseInfoFromFiles,
boolean filesAnalyzed, String pkgName, Set<String> verifiedIds, List<String> retval, String specVersion) {
if (licenseInfoFromFiles.size() == 0 && filesAnalyzed) {
if (Version.versionLessThan(specVersion, Version.TWO_POINT_THREE_VERSION)) {
retval.add("Missing required license information from files for "+pkgName);
}
} else {
boolean foundNonSimpleLic = false;
for (AnyLicenseInfo lic:licenseInfoFromFiles) {
List<String> verify = lic.verify(verifiedIds, specVersion);
addNameToWarnings(verify);
retval.addAll(verify);
if (!(lic instanceof SimpleLicensingInfo ||
lic instanceof SpdxNoAssertionLicense ||
lic instanceof SpdxNoneLicense ||
lic instanceof OrLaterOperator ||
lic instanceof WithExceptionOperator)) {
foundNonSimpleLic = true;
}
}
if (foundNonSimpleLic) {
retval.add("license info from files contains complex licenses for "+pkgName);
}
}
}
boolean filesAnalyzed, String pkgName, Set<String> verifiedIds, List<String> retval, String specVersion) {
if (licenseInfoFromFiles.size() != 0 && !filesAnalyzed) {
retval.add("License information from files must not be included when files not analyzed. Package " + pkgName);
} else {
boolean foundNonSimpleLic = false;
for (AnyLicenseInfo lic:licenseInfoFromFiles) {
List<String> verify = lic.verify(verifiedIds, specVersion);
addNameToWarnings(verify);
retval.addAll(verify);
if (!(lic instanceof SimpleLicensingInfo ||
lic instanceof SpdxNoAssertionLicense ||
lic instanceof SpdxNoneLicense ||
lic instanceof OrLaterOperator ||
lic instanceof WithExceptionOperator)) {
foundNonSimpleLic = true;
}
}
if (foundNonSimpleLic) {
retval.add("license info from files contains complex licenses for "+pkgName);
}
}
}

@Override
@Override
public int compareTo(SpdxPackage pkg) {
// sort order is determined by the name and the version

Expand Down
16 changes: 10 additions & 6 deletions src/test/java/org/spdx/library/model/SpdxPackageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,16 +221,20 @@ public void testVerify() throws InvalidSPDXAnalysisException {
pkg.setStrict(false);
List<String> result = pkg.verify();
assertEquals(0, result.size());
// verification code
// verification code is optional
pkg.setPackageVerificationCode(null);
assertEquals(1, pkg.verify().size());
assertEquals(0, pkg.verify().size());

// Make sure no files are allowed when filesAnalyzed is false
// Make sure no files and no licenses from files are allowed when filesAnalyzed is false
pkg.setFilesAnalyzed(false);
assertEquals(1, pkg.verify().size());
//Make sure we're valid with no files and no verification code when filesAnalyzed = false.
assertEquals(2, pkg.verify().size());

// Make sure no licenses are allowed when filesAnalyzed = false
pkg.getFiles().clear();
assertEquals(1, pkg.verify().size());

// Make sure we're valid with no files and no licenses and no verification code when filesAnalyzed = false
pkg.getLicenseInfoFromFiles().clear();
assertEquals(0, pkg.verify().size());
}

Expand Down