eclipse-emfcloud / emfcloud-modelserver

Modelserver component
Other
40 stars 21 forks source link

API V2: Return patches for each affected resource #209

Closed CamilleLetavernier closed 2 years ago

CamilleLetavernier commented 2 years ago

This is related to Issue #159, although it can be done independently.

At the moment, when executing a Command or a Patch, we return a Json Patch describing the actual changes applied to to the model. However, this Patch only describes changes affecting the main resource (As defined by the ?modeluri query). So if we execute:

PATCH models?modeluri=main.xmi
patch: [{
  "op": "replace",
  "path": "secondary.xmi#objectId/feature"
  ...
}]

We will receive an empty patch, because the main resource wasn't modified. Instead of returning a single patch for the main resource, we should return a patch for each affected resource (which may or may not include the main resource). We can use a Json Map instead of a single patch:

patches: {
   "main.xmi": [ { "op": ...}],
   "secondary.xmi": [ { "op": ...}]
}

or an Array:

patches: [{
   modelUri: "main.xmi",
   patch: [ { "op": ...}]
}, {
   modelUri: "secondary.xmi",
   patch: [ { "op": ...}]
}]

The scope of this ticket is to define multiple patches in the reply of edit() operations, as well as Transactions. It is not about the ability to specify multiple patches during an edit operation (This will be covered by Issue #159)

Regarding Subscription listeners: each listener is registered for a single resource, so we don't need to change them. However, we need to make sure that listeners of all affected resources are notified; and not just the main resource listeners.

cdamus commented 2 years ago

Additionally/alternatively, we may want to consider supporting a paths=urifragments query parameter in the PATCH request to encode the Operation::path property in the reply's patch as EMF-URI-based "custom paths" as per #205 in the incremental update subscriptions.

For EMF-resource-savvy clients this may be enough to distinguish operations on multiple resources? Or, it could work together with a map of patches to provide the object IDs that are convenient for various client-side indexing.

CamilleLetavernier commented 2 years ago

Additionally/alternatively, we may want to consider supporting a paths=urifragments query parameter in the PATCH request to encode the Operation::path property in the reply's patch as EMF-URI-based "custom paths" as per #205 in the incremental update subscriptions.

I like that :+1:

For EMF-resource-savvy clients this may be enough to distinguish operations on multiple resources?

IDs/URI Fragments are not necessarily unique across resources (They probably are when using UUIDs, and probably aren't when using path-based EMF Fragments or name-based IDs)

Also, when using EMF-like paths for edition, we need to include the modelUri, so having access to the full URI (Instead of just the ID/Fragment), in a way or another, makes sense.

cdamus commented 2 years ago

For EMF-resource-savvy clients this may be enough to distinguish operations on multiple resources?

IDs/URI Fragments are not necessarily unique across resources (They probably are when using UUIDs, and probably aren't when using path-based EMF Fragments or name-based IDs)

Yeah, it's not strictly accurate that the parameter is paths=urifragments because the resulting paths do actually include the resource URI (relative to the workspace) with the fragment. So, they are "fragment URIs", not "URI fragments". More generally, they are just like what the server accepts in PATCH requests on models 😀

Also, when using EMF-like paths for edition, we need to include the modelUri, so having access to the full URI (Instead of just the ID/Fragment), in a way or another, makes sense.

Yes, we always still need the modelUri query parameter to identify the resource set. Must always keep in mind that multiple resource sets can conceivably have copies of the same resource loaded. Although how applications can be expected to deal with that, I do not know ...