Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
164 changes: 133 additions & 31 deletions src/main/java/org/spdx/library/ListedLicenses.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@

import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.*;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

Expand All @@ -33,6 +30,7 @@
import org.spdx.library.model.v2.SpdxConstantsCompatV2;
import org.spdx.library.model.v2.SpdxModelFactoryCompatV2;
import org.spdx.library.model.v2.license.SpdxListedLicense;
import org.spdx.library.model.v2.license.SpdxListedLicenseException;
import org.spdx.library.model.v3_0_1.core.CreationInfo;
import org.spdx.library.model.v3_0_1.expandedlicensing.ListedLicense;
import org.spdx.library.model.v3_0_1.expandedlicensing.ListedLicenseException;
Expand Down Expand Up @@ -62,6 +60,11 @@ public class ListedLicenses {
private SpdxV2ListedLicenseModelStore licenseStoreV2;
private SpdxV3ListedLicenseModelStore licenseStoreV3;
private static ListedLicenses listedLicenses = null;
private Map<String, SpdxListedLicense> spdxListedLicenseMapCompatV2;
Copy link
Collaborator

@pmonks pmonks Jan 30, 2025

Choose a reason for hiding this comment

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

I'm still not entirely sure why we need version-specific storage of the license and exception lists, especially as this establishes a precedent that won't scale well over time (i.e. when SPDX model v4, v5, ..., are eventually released). The duplication in API surface, processing, and in-memory storage of these lists is concerning to me in the long term (and very difficult to fix, once introduced).

AFIACT the license and exception lists (deliberately?) use their own unique data structures unrelated to the SPDX model, and are versioned independently of the SPDX model too (so for example SPDX license list v3.26.0 has no special relationship with SPDX model v3.0.1, despite coincidentally sharing the same major version number); SPDX license list v3.26.0 is equally compatible with SPDX model v2.x or v3.x (and presumably in future, v4.x etc.).

It seems to me that there would be substantial benefits in having Spdx-Java-Library directly implement that existing separation in the source data, and simply store the license and exception lists separately, outside the SPDX model hierarchy, in their own dedicated class structure(s) that are independent of the generated model classes.

And yes, there may be value in having method(s) that can translate a license list object into an equivalent version-specific SPDX model object, but I think it would be better for that to be stateless (i.e. not cached in Spdx-Java-Library), and opt-in (i.e. such translations are only performed if/when a user calls those translation methods). This ensures that folx who use the license list, but either don't use the SPDX model objects at all, or only use a single version of them, aren't forced to incur runtime costs that never benefit them.

This happens to be my use case, fwiw - my code makes extensive use of the license and exception lists, but zero use of the SPDX model objects, and in that context shoehorning the license and exception lists into multiple version-specific SPDX model objects is going to be counterproductive (my code will be harder to understand, and there will be negative runtime impacts that my code doesn't benefit from).

Copy link
Member Author

Choose a reason for hiding this comment

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

@pmonks - I appreciate the concern for maintainability (as one of the maintainers ;) and I agree with many aspects of your comments, with a few differences outlined below.

I'm still not entirely sure why we need version-specific storage of the license and exception lists

We're actually not storing the data in the format - it is more of a mapping to a common data format (the license JSON files). Since there is a lot of existing code that expects the SPDX 2.X model representation, I believe there is a lot of value in having the library support that representation. Similarly, anyone implementing the SPDX 3.X model would likely require a matching Java object. I have code that expects both of these model representations, for example. That being said, I won't be making use of the map myself.

especially as this establishes a precedent that won't scale well over time (i.e. when SPDX model v4, v5, ..., are eventually released).

For the library, I've only supported that last 2 major releases. For example, SPDX 1.0 is no longer supported in this version of the library. Since we only do major releases of the spec every 3 to 5 years, this seems to be OK.

AFIACT the license and exception lists (deliberately?) use their own unique data structures unrelated to the SPDX model, and are versioned independently of the SPDX model too (so for example SPDX license list v3.26.0 has no special relationship with SPDX model v3.0.1, despite coincidentally sharing the same major version number); SPDX license list v3.26.0 is equally compatible with SPDX model v2.x or v3.x (and presumably in future, v4.x etc.).

The license list data published in the JSON files are actually based on the SPDX 2.X model. There are a few differences due to the license list JSON file format predating the SPDX 2.X JSON format. The data itself is versioned separately, the the model is consistent. We just started publishing data in SPDX 3 format in parallel to the legacy SPDX 2 data format. BTW - In the SPDX Java library implementation, I just use the SPDX 2.X data and map that to the SPDX 3 format since they are pretty compatible.

In a way, current SPDX java library implementation isn't that far from what you are proposing:

  • Nothing is created and there is no overhead until someone makes a call to a method that explicitly returns the Java model version (e.g. getSpdxLicenseById), that way if no one is using a particular model version there is no penalty or overhead.
  • Both the 2.X and 3.X model representations use the same underlying JSON data structures to store the actual values. This is in the License List format - which is based on the SPDX 2.X license model.
  • Access to the (same) license data is completely stateless for both model objects.
  • Access to the license data is lazy loaded at the license or exception level. Even though we have a class representing a license or exception, the data is only fetched when the data is actually referenced. At that point, we'll fetch the license data either from the cache, website, or resource file depending on the configuration.

In terms of the maintenance issues - it is a real pain to maintain both SPDX 3 and SPDX 2 versions of the model, but this is necessary to support high demand use cases like translating an SPDX document from SPDX 2 to SPDX 3. The incremental overhead for maintaining the license structures is very small compared to the effort of maintaining the general models.

private Map<String, ListedLicense> spdxListedLicenseMap;
private Map<String, org.spdx.library.model.v2.license.ListedLicenseException> spdxListedExceptionMapCompatV2;
private Map<String, ListedLicenseException> spdxListedExceptionMap;

/**
* Lock for any modifications to the underlying licenseModelStore
*/
Expand Down Expand Up @@ -201,13 +204,7 @@ public boolean isSpdxListedExceptionId(String exceptionId) {
* @throws InvalidSPDXAnalysisException on SPDX parsing error
*/
public SpdxListedLicense getListedLicenseByIdCompatV2(String licenseId) throws InvalidSPDXAnalysisException {
try {
return (SpdxListedLicense)SpdxModelFactoryCompatV2.getModelObjectV2(this.licenseStoreV2,
SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX, licenseId,
SpdxConstantsCompatV2.CLASS_SPDX_LISTED_LICENSE, null, false);
} catch (SpdxIdNotFoundException ex) {
return null;
}
return getSpdxListedLicensesCompatV2().get(licenseId);
}

/**
Expand All @@ -216,13 +213,7 @@ public SpdxListedLicense getListedLicenseByIdCompatV2(String licenseId) throws I
* @throws InvalidSPDXAnalysisException on SPDX parsing error
*/
public org.spdx.library.model.v2.license.ListedLicenseException getListedExceptionByIdCompatV2(String exceptionId) throws InvalidSPDXAnalysisException {
try {
return (org.spdx.library.model.v2.license.ListedLicenseException)SpdxModelFactoryCompatV2.getModelObjectV2(
this.licenseStoreV2, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX,
exceptionId, SpdxConstantsCompatV2.CLASS_SPDX_LISTED_LICENSE_EXCEPTION, null, false);
} catch (SpdxIdNotFoundException ex) {
return null;
}
return getSpdxListedLicenseExceptionsCompatV2().get(exceptionId);
}

/**
Expand All @@ -231,22 +222,11 @@ public org.spdx.library.model.v2.license.ListedLicenseException getListedExcepti
* @throws InvalidSPDXAnalysisException on SPDX parsing error
*/
public ListedLicense getListedLicenseById(String licenseId) throws InvalidSPDXAnalysisException {
try {
return new ListedLicense(this.licenseStoreV3, SpdxListedLicenseModelStore.licenseOrExceptionIdToObjectUri(licenseId), null,
false, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX);
} catch (SpdxIdNotFoundException ex) {
return null;
}

return getSpdxListedLicenses().get(licenseId);
}

public ListedLicenseException getListedExceptionById(String exceptionId) throws InvalidSPDXAnalysisException {
try {
return new ListedLicenseException(this.licenseStoreV3, SpdxListedLicenseModelStore.licenseOrExceptionIdToObjectUri(exceptionId), null,
false, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX);
} catch (SpdxIdNotFoundException ex) {
return null;
}
return getSpdxListedLicenseExceptions().get(exceptionId);

}

Expand All @@ -261,6 +241,128 @@ public List<String> getSpdxListedLicenseIds() {
listedLicenseModificationLock.readLock().unlock();
}
}

/**
* @return a map of SPDX listed license IDs to the SPDX listed license
* @throws InvalidSPDXAnalysisException on errors fetching the licenses
*/
public Map<String, ListedLicense> getSpdxListedLicenses() throws InvalidSPDXAnalysisException {
listedLicenseModificationLock.readLock().lock();
try {
if (Objects.nonNull(this.spdxListedLicenseMap)) {
return this.spdxListedLicenseMap;
}
} finally {
listedLicenseModificationLock.readLock().unlock();
}
listedLicenseModificationLock.writeLock().lock();
try {
if (Objects.nonNull(this.spdxListedLicenseMap)) {
return this.spdxListedLicenseMap;
}
Map<String, ListedLicense> allListedLicenses = new HashMap<>();
for (String licenseId : this.baseModelStore.getSpdxListedLicenseIds()) {
allListedLicenses.put(licenseId, new ListedLicense(this.licenseStoreV3, SpdxListedLicenseModelStore.licenseOrExceptionIdToObjectUri(licenseId), null,
false, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX));
}
this.spdxListedLicenseMap = Collections.unmodifiableMap(allListedLicenses);
return this.spdxListedLicenseMap;
} finally {
listedLicenseModificationLock.writeLock().unlock();
}
}

/**
* @return a map of SPDX listed license exception IDs to the SPDX listed license exception
* @throws InvalidSPDXAnalysisException on errors fetching the license exceptions
*/
public Map<String, ListedLicenseException> getSpdxListedLicenseExceptions() throws InvalidSPDXAnalysisException {
listedLicenseModificationLock.readLock().lock();
try {
if (Objects.nonNull(this.spdxListedExceptionMap)) {
return this.spdxListedExceptionMap;
}
} finally {
listedLicenseModificationLock.readLock().unlock();
}
listedLicenseModificationLock.writeLock().lock();
try {
if (Objects.nonNull(this.spdxListedExceptionMap)) {
return this.spdxListedExceptionMap;
}
Map<String, ListedLicenseException> allListedExceptions = new HashMap<>();
for (String exceptionId : this.baseModelStore.getSpdxListedExceptionIds()) {
allListedExceptions.put(exceptionId, new ListedLicenseException(this.licenseStoreV3, SpdxListedLicenseModelStore.licenseOrExceptionIdToObjectUri(exceptionId), null,
false, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX));
}
this.spdxListedExceptionMap = Collections.unmodifiableMap(allListedExceptions);
return this.spdxListedExceptionMap;
} finally {
listedLicenseModificationLock.writeLock().unlock();
}
}

/**
* @return a map of SPDX listed license IDs to the SPDX Spec version 2 listed license
* @throws InvalidSPDXAnalysisException on errors fetching the licenses
*/
protected Map<String, SpdxListedLicense> getSpdxListedLicensesCompatV2() throws InvalidSPDXAnalysisException {
listedLicenseModificationLock.readLock().lock();
try {
if (Objects.nonNull(this.spdxListedLicenseMapCompatV2)) {
return this.spdxListedLicenseMapCompatV2;
}
} finally {
listedLicenseModificationLock.readLock().unlock();
}
listedLicenseModificationLock.writeLock().lock();
try {
if (Objects.nonNull(this.spdxListedLicenseMapCompatV2)) {
return this.spdxListedLicenseMapCompatV2;
}
Map<String, SpdxListedLicense> allListedLicenses = new HashMap<>();
for (String licenseId : this.baseModelStore.getSpdxListedLicenseIds()) {
allListedLicenses.put(licenseId, (SpdxListedLicense)SpdxModelFactoryCompatV2.getModelObjectV2(this.licenseStoreV2,
SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX, licenseId,
SpdxConstantsCompatV2.CLASS_SPDX_LISTED_LICENSE, null, false));
}
this.spdxListedLicenseMapCompatV2 = Collections.unmodifiableMap(allListedLicenses);
return this.spdxListedLicenseMapCompatV2;
} finally {
listedLicenseModificationLock.writeLock().unlock();
}
}

/**
* @return a map of SPDX listed license exception IDs to the SPDX listed license exception
* @throws InvalidSPDXAnalysisException on errors fetching the license exceptions
*/
protected Map<String, org.spdx.library.model.v2.license.ListedLicenseException> getSpdxListedLicenseExceptionsCompatV2() throws InvalidSPDXAnalysisException {
listedLicenseModificationLock.readLock().lock();
try {
if (Objects.nonNull(this.spdxListedExceptionMapCompatV2)) {
return this.spdxListedExceptionMapCompatV2;
}
} finally {
listedLicenseModificationLock.readLock().unlock();
}
listedLicenseModificationLock.writeLock().lock();
try {
if (Objects.nonNull(this.spdxListedExceptionMapCompatV2)) {
return this.spdxListedExceptionMapCompatV2;
}
Map<String, org.spdx.library.model.v2.license.ListedLicenseException> allListedExceptions = new HashMap<>();
for (String exceptionId : this.baseModelStore.getSpdxListedExceptionIds()) {
allListedExceptions.put(exceptionId, (org.spdx.library.model.v2.license.ListedLicenseException)SpdxModelFactoryCompatV2.getModelObjectV2(
this.licenseStoreV2, SpdxConstantsCompatV2.LISTED_LICENSE_NAMESPACE_PREFIX,
exceptionId, SpdxConstantsCompatV2.CLASS_SPDX_LISTED_LICENSE_EXCEPTION, null, false));
}
this.spdxListedExceptionMapCompatV2 = Collections.unmodifiableMap(allListedExceptions);
return this.spdxListedExceptionMapCompatV2;
} finally {
listedLicenseModificationLock.writeLock().unlock();
}
}

/**
* @return The version of the loaded license list in the form M.N, where M is the major release and N is the minor release.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ public boolean isPropertyValueAssignableTo(PropertyDescriptor propertyDescriptor
* @param clazz class to test assignability
* @return true if the list associated with the propertyDescriptor have a value added of type clazz
*/
public boolean isCollectionMembersAssignableTo(PropertyDescriptor propertyDescriptor, Class<?> clazz) {
public static boolean isCollectionMembersAssignableTo(PropertyDescriptor propertyDescriptor, Class<?> clazz) {
if (SpdxConstantsCompatV2.RDFS_PROP_SEE_ALSO.equals(propertyDescriptor) ||
SpdxConstantsV3.PROP_SEE_ALSO.equals(propertyDescriptor)) {
return String.class.isAssignableFrom(clazz);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ public boolean isPropertyValueAssignableTo(PropertyDescriptor propertyDescriptor

}

public boolean isCollectionMembersAssignableTo(PropertyDescriptor propertyDescriptor, Class<?> clazz) throws InvalidSpdxPropertyException {
public static boolean isCollectionMembersAssignableTo(PropertyDescriptor propertyDescriptor, Class<?> clazz) throws InvalidSpdxPropertyException {
String propertyName = PROPERTY_DESCRIPTOR_TO_VALUE_NAME.get(propertyDescriptor);
if (Objects.isNull(propertyName)) {
throw new InvalidSpdxPropertyException("Invalid property for SPDX listed license:"+propertyDescriptor.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1098,11 +1098,9 @@ public boolean isCollectionMembersAssignableTo(String objectUri, PropertyDescrip
listedLicenseModificationLock.writeLock().unlock();
}
if (isLicenseId) {
LicenseJson license = fetchLicenseJson(id);
return license.isCollectionMembersAssignableTo(propertyDescriptor, clazz);
return LicenseJson.isCollectionMembersAssignableTo(propertyDescriptor, clazz);
} else if (isExceptionId) {
ExceptionJson exc = fetchExceptionJson(id);
return exc.isCollectionMembersAssignableTo(propertyDescriptor, clazz);
return ExceptionJson.isCollectionMembersAssignableTo(propertyDescriptor, clazz);
} else if (Objects.nonNull(crossRef)) {
return crossRef.isCollectionMembersAssignableTo(propertyDescriptor, clazz);
} else {
Expand Down
71 changes: 55 additions & 16 deletions src/test/java/org/spdx/library/ListedLicensesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.spdx.library;

import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.spdx.core.InvalidSPDXAnalysisException;
Expand All @@ -30,6 +31,7 @@
import org.spdx.storage.compatv2.CompatibleModelStoreWrapper;

import junit.framework.TestCase;
import org.spdx.utility.compare.UnitTestHelper;


/**
Expand Down Expand Up @@ -60,15 +62,15 @@ protected void tearDown() throws Exception {
}

/**
* Test method for {@link org.spdx.library.model.compat.v2.compat.v2.license.ListedLicenses#isSpdxListedLicenseId(java.lang.String)}.
* Test method for {@link org.spdx.library.ListedLicenses#isSpdxListedLicenseId(java.lang.String)}.
*/
public void testIsSpdxListedLicenseID() {
assertTrue(ListedLicenses.getListedLicenses().isSpdxListedLicenseId("Apache-2.0"));
}

/**
* Test method for {@link org.spdx.library.model.compat.v2.compat.v2.license.ListedLicenses#getListedLicenseById(java.lang.String)}.
* @throws InvalidSPDXAnalysisException
* Test method for {@link org.spdx.library.ListedLicenses#getListedLicenseById(java.lang.String)}.
* @throws InvalidSPDXAnalysisException on error getting license IDs
*/
public void testGetListedLicenseById() throws InvalidSPDXAnalysisException {
String id = "Apache-2.0";
Expand All @@ -95,7 +97,7 @@ public void testGetLicenseIbyIdLocal() throws InvalidSPDXAnalysisException {
}
}
/**
* Test method for {@link org.spdx.library.model.compat.v2.compat.v2.license.ListedLicenses#getSpdxListedLicenseIds()}.
* Test method for {@link org.spdx.library.ListedLicenses#getSpdxListedLicenseIds()}.
*/
public void testGetSpdxListedLicenseIds() {
List<String> result = ListedLicenses.getListedLicenses().getSpdxListedLicenseIds();
Expand Down Expand Up @@ -133,21 +135,21 @@ public void testGetExceptionByIdLocal() throws InvalidSPDXAnalysisException {
}
}

public void testGetExceptionIds() throws InvalidSPDXAnalysisException {
public void testGetExceptionIds() {
assertTrue(ListedLicenses.getListedLicenses().getSpdxListedExceptionIds().size() >= NUM_3_7_EXCEPTION);
}

public void testListedLicenseIdCaseSensitive() {
String expected = "Apache-2.0";
String lower = expected.toLowerCase();
assertEquals(expected, ListedLicenses.getListedLicenses().listedLicenseIdCaseSensitive(lower).get());
assertEquals(expected, ListedLicenses.getListedLicenses().listedLicenseIdCaseSensitive(lower).orElse(null));
assertFalse(ListedLicenses.getListedLicenses().listedLicenseIdCaseSensitive("NotaLicenseId").isPresent());
}

public void testListedExceptionIdCaseSensitive() {
String expected = "Classpath-exception-2.0";
String lower = expected.toLowerCase();
assertEquals(expected, ListedLicenses.getListedLicenses().listedExceptionIdCaseSensitive(lower).get());
assertEquals(expected, ListedLicenses.getListedLicenses().listedExceptionIdCaseSensitive(lower).orElse(null));
assertFalse(ListedLicenses.getListedLicenses().listedExceptionIdCaseSensitive("NotAnExceptionId").isPresent());
}

Expand All @@ -161,13 +163,50 @@ public void testGetLicenseIdProperty() throws InvalidSPDXAnalysisException {
assertEquals(id, idProp.get());
}

public void testGetExceptionIdProperty() throws InvalidSPDXAnalysisException {
String id = "Classpath-exception-2.0";
org.spdx.library.model.v2.license.ListedLicenseException ex = ListedLicenses.getListedLicenses().getListedExceptionByIdCompatV2(id);
Optional<Object> idProp = ex.getModelStore().getValue(
CompatibleModelStoreWrapper.documentUriIdToUri(ex.getDocumentUri(), id, false), SpdxConstantsCompatV2.PROP_LICENSE_EXCEPTION_ID);
assertTrue(idProp.isPresent());
assertTrue(idProp.get() instanceof String);
assertEquals(id, idProp.get());
}
public void testGetExceptionIdProperty() throws InvalidSPDXAnalysisException {
String id = "Classpath-exception-2.0";
org.spdx.library.model.v2.license.ListedLicenseException ex = ListedLicenses.getListedLicenses().getListedExceptionByIdCompatV2(id);
Optional<Object> idProp = ex.getModelStore().getValue(
CompatibleModelStoreWrapper.documentUriIdToUri(ex.getDocumentUri(), id, false), SpdxConstantsCompatV2.PROP_LICENSE_EXCEPTION_ID);
assertTrue(idProp.isPresent());
assertTrue(idProp.get() instanceof String);
assertEquals(id, idProp.get());
}

public void testGetListedLicenses() throws InvalidSPDXAnalysisException {
Map<String, ListedLicense> retval =ListedLicenses.getListedLicenses().getSpdxListedLicenses();
List<String> ids = ListedLicenses.getListedLicenses().getSpdxListedLicenseIds();
assertEquals(ids.size(), retval.size());
for (String id:ids) {
assertTrue(retval.get(id).getObjectUri().endsWith(id));
}
}

public void testGetListedLicensesCompatV2() throws InvalidSPDXAnalysisException {
Map<String, SpdxListedLicense> retval =ListedLicenses.getListedLicenses().getSpdxListedLicensesCompatV2();
List<String> ids = ListedLicenses.getListedLicenses().getSpdxListedLicenseIds();
assertEquals(ids.size(), retval.size());
for (String id:ids) {
assertEquals(id, retval.get(id).getId());
}
}

public void testGetListedLicenseExceptions() throws InvalidSPDXAnalysisException {
Map<String, ListedLicenseException> retval =ListedLicenses.getListedLicenses().getSpdxListedLicenseExceptions();
List<String> ids = ListedLicenses.getListedLicenses().getSpdxListedExceptionIds();
assertEquals(ids.size(), retval.size());
for (String id:ids) {
assertTrue(retval.get(id).getObjectUri().endsWith(id));
}
}

public void testGetListedLicenseExceptionsCompatV2() throws InvalidSPDXAnalysisException {
Map<String, org.spdx.library.model.v2.license.ListedLicenseException> retval =
ListedLicenses.getListedLicenses().getSpdxListedLicenseExceptionsCompatV2();
List<String> ids = ListedLicenses.getListedLicenses().getSpdxListedExceptionIds();
assertEquals(ids.size(), retval.size());
for (String id:ids) {
assertEquals(id, retval.get(id).getId());
}
}
}