Skip to content

Fix NamespaceSet.pop() removes item from all backends#514

Merged
s-heppner merged 1 commit into
eclipse-basyx:developfrom
rwth-iat:fix/b3-namespace-set-pop
May 13, 2026
Merged

Fix NamespaceSet.pop() removes item from all backends#514
s-heppner merged 1 commit into
eclipse-basyx:developfrom
rwth-iat:fix/b3-namespace-set-pop

Conversation

@zrgt
Copy link
Copy Markdown
Contributor

@zrgt zrgt commented May 5, 2026

Summary

NamespaceSet.pop() used popitem() on the first backend only, leaving stale entries in all remaining backends. Any subsequent add() of an item with the same semantic_id raised a false AASConstraintViolation.

Fix iterates all backends and removes the popped item's key from each.

Fixes #496

Test plan

  • test_namespaceset_pop_removes_from_all_backends — verifies pop then re-add with same semantic_id succeeds
  • Full test.model.test_base suite passes (53 tests)

pop() only removed item from first backend via popitem(), leaving stale
entries in remaining backends and causing false AASConstraintViolation
on subsequent add() for same semantic_id.

Fixes eclipse-basyx#496
@s-heppner
Copy link
Copy Markdown
Member

Wait, this is nonsensical though, isn't it? Namespaces only ever identify via one attribute? I think here the class may have been misunderstood, which might mean, we did not implement them cleanly enough, but this is not a bug, as far as I can see.

@zrgt
Copy link
Copy Markdown
Contributor Author

zrgt commented May 7, 2026

Thanks for the review, @s-heppner!

You're right that current production code uses only one attribute per NamespaceSet. However, NamespaceSet._backend is typed as Dict[str, Tuple[Dict, bool]] — the class is explicitly designed to support multiple uniqueness attributes. The existing test helpers ExampleNamespaceReferable.set1 and ExampleOrderedNamespace.set1 (with both id_short + semantic_id) were deliberately created to exercise this general case, showing multi-attribute support was always intentional.

A pop() that silently leaves stale entries in registered backends is a real correctness bug for the class — the fix is a no-op for single-attribute callers.

@s-heppner s-heppner merged commit cd39f2e into eclipse-basyx:develop May 13, 2026
15 checks passed
@s-heppner s-heppner deleted the fix/b3-namespace-set-pop branch May 13, 2026 13:15
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.

2 participants