Skip to content

Conversation

@goneall
Copy link
Member

@goneall goneall commented Jan 25, 2025

Fixes #284

NOTE: This requires a core library update which includes this PR: spdx/spdx-java-core#23

@goneall
Copy link
Member Author

goneall commented Jan 25, 2025

@dwalluck - please review - note it depends on an update to the core library to compile.

@goneall
Copy link
Member Author

goneall commented Jan 26, 2025

Note that we can not make use of the SpdxModelFactory.init(IModelStore modelStore, String defaultDocumentUri ...) in the unit test cases since the teardown uses the DefaultModelStore.initialize(IModelStore modelStore, String defaultDocumentUri ...) directly.

I'm now wondering if we should remove the SpdxModelFactory.init(IModelStore modelStore, String defaultDocumentUri ...) method and just initialize the DefaultModelStore if it has not already been initialized.

You can always call the DefaultModelStore.initialize(IModelStore modelStore, String defaultDocumentUri ...) before or after the init() call to control the defaults.

@pmonks
Copy link
Collaborator

pmonks commented Jan 26, 2025

@goneall that would need careful documentation and perhaps also a thrown exception in the event that someone tries to configure a non-default model store after init has been called (assuming model stores can't be switched at any time ofc).

@goneall
Copy link
Member Author

goneall commented Jan 26, 2025

@goneall that would need careful documentation and perhaps also a thrown exception in the event that someone tries to configure a non-default model store after init has been called (assuming model stores can't be switched at any time ofc).

Good point - I'm just going to remove the call to explicitly set the defaults on init. I'll also add some documentation.

@goneall
Copy link
Member Author

goneall commented Jan 26, 2025

I removed the call to explicitly set defaults on init and added documentation.

@dwalluck
Copy link
Contributor

I've run my units tests both with and without initializing the DefaultModelStore and they pass.

Also adds documentation to the GETTING-STARTED.md file
@goneall goneall force-pushed the defaultmodelstore2 branch from 5182ced to 6d3bf15 Compare January 31, 2025 22:53
@goneall goneall marked this pull request as ready for review January 31, 2025 22:53
@goneall goneall merged commit 4a04e0a into master Feb 1, 2025
1 check passed
@goneall goneall deleted the defaultmodelstore2 branch February 1, 2025 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add back default values for the SPDX default model store

4 participants