Missing protection against duplicate id's about spdx-java-library HOT 5 CLOSED

Yikes. A bit of a design flaw. Thanks @nicoweidner for pointing this out. This is probably a root cause of a few issues.

The change you proposed is very logical, but as you point out it is potentially breaking since the semantics.

I wonder if there may be another interface option that would preserve the existing semantics.

I was thinking of using the strict flag to force errors is the ID already exists, however, currently it can only be set after the ModelObject is created.

from spdx-java-library.

One thought is to add a new constructor with different semantics and deprecate the old one. We could add a strict parameter, for example. It would require, however, updating a lot of code.

The idea behind the strict parameter is, if it is set to true then the call will fail if the ID already exists. Otherwise, it would behave the way it does today. This is similar to how the library treats other checks on parameter validity.

I have mixed feeling about whether the strict approach is a good one. @nicoweidner - let me know what you think.

from spdx-java-library.

@nicoweidner I came up with another (partial) solution we could implement with low risk.

If create is true and there is another ModelObject in the store with the same ID, check to see if the type of the ModelObject in the store is the same as type type of the newly created ModelObject. We could fail if they are different types.

This would preserve the semantics of re-using an ModelObject even if create is true. It would prevent the non-symentric equals behavior mentioned in PR 112 comments.

I'm pretty comfortable no one would intentionally create a ModelObject with a different type expecting it to reuse the properties of a previous type.

The only case this doesn't cover is if someone accidentally creates 2 ModelObjects of the same type with the same ID.

from spdx-java-library.

@goneall Given the current situation, I prefer your latest suggestion of failing when someone tries to create an object with the same id, but of different type. That should prevent all the bad scenarios, and is little effort.

from spdx-java-library.

Fixed (mostly) with PR #121

from spdx-java-library.