Do not use default copyManager in LicenseInfoFactory in case none is provided

[martin-bpw]

There are two methods in LicenseInfoFactory, that take @nullable copyManager, and in javadoc one can read "@param copyManager if non-null, allows for copying of any properties set which use other model stores or document URI's". However, in method body, a default one is used (And we are warned about it on different place, in DefaultModelStore - "WARNING: The model store is in memory and will continue to grow as it is utilized. There is NO garbage collection.").

Either javadoc shall be modified, or (preferably) the default copyManager shall not be used as default.

[goneall]

Thanks @martin-bpw for pointing out the inconsistency. I'll need to do a bit of research to see if changing the default to null would break anything - but in either case, I agree we should fix the inconsistency.

[goneall]

It turns out that we do need a copy manager for the license parser to work since we would be copying between the store used for the listed licenses and the store used for the non-standard licenses.

It doesn't have to be the default copy manager, however.

I think we can just change the default (and the documentation) to create a new copy manager instance that would go away after the parsing is complete.

As a workaround, you can pass in a new ModelCopyManager() to the LicenseInfoFactory.

@martin-bpw - can you give the work-around a try? If it works, I can create a PR with the change.

[martin-bpw]

It turns out that we do need a copy manager for the license parser to work since we would be copying between the store used for the listed licenses and the store used for the non-standard licenses.

It doesn't have to be the default copy manager, however.

I think we can just change the default (and the documentation) to create a new copy manager instance that would go away after the parsing is complete.

As a workaround, you can pass in a new ModelCopyManager() to the LicenseInfoFactory.

@martin-bpw - can you give the work-around a try? If it works, I can create a PR with the change.

Yes this would actually be even easier - I also had this idea, to use directly new instance of copy manager instead of one coming from variable. What made me go to direction I did in PR was like a bit of concern - like that its switching from "always reusing" to "never reusing" - and I thought like one instance per Document parsing/etc was like good middle ground.

But I would fully agree also with just creating new as you suggest. That would do.

[martin-bpw]

This is the place - (debuging my code without changes from PR), where it would help:

But it is still spdx-java-jackson-store lib so client code cannot put it there, lib code change still needed (but its micro change).

[goneall]

Yes this would actually be even easier - I also had this idea, to use directly new instance of copy manager instead of one coming from variable. What made me go to direction I did in PR was like a bit of concern - like that its switching from "always reusing" to "never reusing" - and I thought like one instance per Document parsing/etc was like good middle ground.

@martin-bpw good point on going to "never reusing" is a bit extreme. There may be some code out that that depend on the "always reusing", so I think I'll just update the JavaDocs and see if there are other instances where we should pass in a non-default copy manager.