Skip to content

Initialize default model store in init method #286

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.

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

Merged
merged 3 commits into from
Feb 1, 2025
Merged

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
3 participants