Skip to content

Conversation

@EtienneLt
Copy link
Collaborator

@EtienneLt EtienneLt commented Oct 13, 2025

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • A PR or issue has been opened in all impacted repositories (if any)

Does this PR already have an issue describing the problem?
no

What kind of change does this PR introduce?
feature

What is the current behavior?
extension are delete one by one

What is the new behavior (if this is a feature change)?
add a buffer to delete all extension in one flush

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)
the method removeExtension now works with a map of identifiable ids by extension instead of one identifiable name and one extension name

Other informations
linked with powsybl/powsybl-network-store-server#146
#572 is needed before merging these one

Signed-off-by: Etienne LESOT <[email protected]>
Comment on lines 649 to 655
private static <D> void cloneExternalBuffer(NetworkCollectionIndex<ExternalAttributesCollectionBuffer<D>> buffer, UUID networkUuid,
int sourceVariantNum, int targetVariantNum) {
// clone resources from source variant collection
ExternalAttributesCollectionBuffer<D> clonedCollection =
new ExternalAttributesCollectionBuffer<>(buffer.getCollection(networkUuid, sourceVariantNum));
buffer.addCollection(networkUuid, targetVariantNum, clonedCollection);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it a deep clone, use the same mechanism as usual buffer (a clone method in the buffer class)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sonar prefers not

private final QuadriConsumer<UUID, Integer, ResourceType, Map<String, T>> removeFct;

private final Map<ResourceType, Map<String, T>> removeResourcesIds = new HashMap<>();
private final BiConsumer<Map<String, T>, Map<String, T>> addFct;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it more of a mergeFct than a addFct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is more a putAll function isnt it ?

@antoinebhs
Copy link
Collaborator

I'm dubious on whether or not it works with the existing buffer implementation for identifiables as they are closely related.
In a case where:

  • existing line1 with olg1, olg2
  • remove olg2 from line1
  • create olg2 on line 1
    I think that this implementation would remove olg2 on the server (similar for extensions) on flush

Another case:

  • existing line1 with olg1, olg2
  • remove olg2
  • remove line1
    We should not call the server to remove olg2 as the resource is removed and it does not exist anymore

Another case:

  • existing line1 with olg1, olg2
  • remove olg2
  • remove line1
  • create line1 with olg2
    It should not remove line2 on the server for the new resource

Can you verify that all these cases work fine with your implementation? And think about other similar cases that could be tricky as the new buffer is very linked to CollectionBuffer

Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants