Skip to content
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

API V2: The patch operation "remove" doesn't remove cross references #207

Closed
CamilleLetavernier opened this issue Apr 11, 2022 · 1 comment · Fixed by #208
Closed

API V2: The patch operation "remove" doesn't remove cross references #207

CamilleLetavernier opened this issue Apr 11, 2022 · 1 comment · Fixed by #208
Assignees
Labels
bug Something isn't working modelserver EMF.cloud Model Server sponsored

Comments

@CamilleLetavernier
Copy link
Member

When using the Patch operation "remove" to delete an object from the model, cross references are not unset. This is a bug in the JsonPatchHelper, that returns a "Remove" command instead of a "Delete" command.

@CamilleLetavernier CamilleLetavernier added bug Something isn't working modelserver EMF.cloud Model Server labels Apr 11, 2022
@CamilleLetavernier CamilleLetavernier self-assigned this Apr 11, 2022
@CamilleLetavernier
Copy link
Member Author

Note: this is only an issue for the "EMF-like" Json Patches, using this syntax to delete an object from the model:

[{
   "op": "remove",
   "path": "mymodel.xmi#objectId"
}]

Which corresponds to an EMF "Delete Command".

For standard Json Patches, the semantic is instead closer to the EMF "Remove Command":

[{
   "op": "remove",
   "path": "parentid/reference/childIndex"
}]

or an unset Command:

[{
   "op": "remove",
   "path": "parentid/reference"
}]

In these cases, we don't want to remove cross-references.

However, it is unclear to me what the expected behavior is, when unsetting/removing an object from a Containment reference (when reference is a Containment ref). The client might want to provide a patch with multiple operations to unset the cross-references, or to ignore the EMF semantics and leave the responsibility to the EMF server. We should define the expected behavior in this case.

IMO, it makes sense to treat removal from a Containment reference like a Deletion, and also clean-up all cross references. If the Client intends to reuse the Object, it would use a "Move" operation instead (Although this isn't supported yet: issue #179), or an "Add" to the new location (EMF is then responsible for removing the object from its former parent, and adding it to the new parent).

CamilleLetavernier added a commit that referenced this issue Apr 11, 2022
- Use DeleteCommand instead of RemoveCommand, when deleting an object
(Remove from list and Unset feature still rely on RemoveCommand or
SetCommand and won't modify cross-references)

fixes #207
CamilleLetavernier added a commit that referenced this issue Apr 11, 2022
- Use DeleteCommand instead of RemoveCommand, when deleting an object
(Remove from list and Unset feature still rely on RemoveCommand or
SetCommand and won't modify cross-references)

fixes #207

Contributed on behalf of STMicroelectronics.

Signed-off-by: Camille Letavernier <cletavernier@eclipsesource.com>
CamilleLetavernier added a commit that referenced this issue Apr 12, 2022
- Use DeleteCommand instead of RemoveCommand, when deleting an object
(Remove from list and Unset feature still rely on RemoveCommand or
SetCommand and won't modify cross-references)

fixes #207

Contributed on behalf of STMicroelectronics.

Signed-off-by: Camille Letavernier <cletavernier@eclipsesource.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working modelserver EMF.cloud Model Server sponsored
Projects
None yet
2 participants