Add API to return licenses as a map in addition to list

[dwalluck]

I see that the license list is actually using a map internally, but the API returns a list Map::values(). Is it possible to add an API which also returns a Map with key String licenseId and value SpdxListedLicense? This would be more performant rather than having to recreate a map from the list.

[goneall]

Sounds like a useful API - if you'd like to create a PR, I can review merge. Otherwise, I'll take a look at this after implementing the SPDX 3.0 support.

[goneall]

@dwalluck - Finished with the SPDX 3 upgrade - can you point me to the API that returns the list?

[dwalluck]

@goneall Thanks for following up. As an aside, I quickly tried to test 2.0.0-RC1 and I got a fatal error loading the cached SPDX licenses:

[main] WARN  org.spdx.library.ListedLicenses - Unable to access the most current listed licenses from https://spdx.org/licenses - using locally cached licenses: No implementation found for SPDX spec version 3.0.1 Note: you can set the org.spdx.useJARLicenseInfoOnly property to true to avoid this warning.
[main] ERROR org.spdx.library.ListedLicenses - Error loading cached SPDX licenses

I no longer see an API which returns a list. But, there's also currently no API which returns a map.

I guess it comes down to a design choice here since SpdxListedLicenseModelStore already has a couple maps present, Map<String, LicenseJson> listedLicenseCache and Map<String, ExceptionJson> listedExceptionCache.

However, the ListedLicense* objects themselves are not cached and can only be created on the fly via ListedLicense::getListedLicenseById* or getListedExceptionById* which returns a new object every time these methods are called. Maybe you could also add a Map<String, ListedLicense>/Map<String, ListedLicenseException> in ListedLicense since this data is static.

[goneall]

I quickly tried to test 2.0.0-RC1 and I got a fatal error loading the cached SPDX licenses...

I wasn't able to duplicate the problem.

The warning is expected if you're not connected to the internet. The hard error, on the other hand, is completely unexpected.

I'll add some additional logging to see if we can find out what is wrong.

The code just reads the license information from resource files included in the Jar file.

[goneall]

@pmonks - any thoughts on the design approach?

[pmonks]

I'm very much in favour of a "map-based" representation of the license and exception lists held by the library and accessible (perhaps only as an unmodifiableMap, so callers can't modify that core data and mess things up?) via a public API; FWIW my code constructs exactly these maps itself so this would be a (welcome!) simplification of my code.

One thought: there may be runtime performance benefits for the library to make more use of such maps itself e.g. in various methods in the org.spdx.library.model.license.ListedLicenses class (note: I'm looking at the 1.1.12 version of the code right now - I believe that class has changed packages and/or names in v2.0). For example determining whether an id is valid, or retrieving an org.spdx.library.model.license.SpdxListedLicense instance for a given id. Of course these optimisations could come later - there's no hard requirement to gate this specific issue on making use of the new maps everywhere that makes sense internally.

[dwalluck]

I quickly tried to test 2.0.0-RC1 and I got a fatal error loading the cached SPDX licenses...

I wasn't able to duplicate the problem.

The warning is expected if you're not connected to the internet. The hard error, on the other hand, is completely unexpected.

I'll add some additional logging to see if we can find out what is wrong.

The code just reads the license information from resource files included in the Jar file.

I was definitely connected to the internet. Making the error message show the exception stack trace would be helpful.

[dwalluck]

I also changed to use all 1.0.0-RC2-SNAPSHOT versions, but it didn't make a difference. Maybe the project can benefit from a BOM since the two models, the core, and the library all seem to be on different versions of the same dependencies.

java.lang.RuntimeException: Unexpected error loading SPDX Listed Licenses
        at org.spdx.library.ListedLicenses.initializeLicenseModelStore(ListedLicenses.java:130)
        at org.spdx.library.ListedLicenses.<init>(ListedLicenses.java:77)
        at org.spdx.library.ListedLicenses.getListedLicenses(ListedLicenses.java:155)
Caused by: org.spdx.core.ModelRegistryException: No implementation found for SPDX spec version 3.0.1
        at org.spdx.core.ModelRegistry.typeToClass(ModelRegistry.java:194)
        at org.spdx.core.TypedValue.<init>(TypedValue.java:31)
        at org.spdx.storage.listedlicense.LicenseCreatorAgent.<init>(LicenseCreatorAgent.java:55)
        at org.spdx.storage.listedlicense.SpdxListedLicenseModelStore.<init>(SpdxListedLicenseModelStore.java:118)
        at org.spdx.storage.listedlicense.SpdxListedLicenseLocalStore.<init>(SpdxListedLicenseLocalStore.java:37)
        at org.spdx.library.ListedLicenses.initializeLicenseModelStore(ListedLicenses.java:127)
        ... 6 more

[dwalluck]

@goneall I tracked it down to missing calls to SpdxModelFactory.init() and DefaultModelStore.initialize() in my application (it apparently needs both). I am not sure about the proper arguments to DefaultModelStore.initialize() as neither of these methods used to be necessary. I believe that a call to ListedLicenses.getListedLicenses() took care of the initialization before so it looks like some sort of regression.