publish to mercure preserving the request's format and also respect t…

[toriqo]

publish to mercure preserving the request's format and also respect the entity's serializer context

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	no
Fixed tickets	-
License	MIT
Doc PR	-

[toriqo]

ping @dunglas @soyuka

[dunglas]

I thought about this during the initial implementation, unfortunately I don’t think that this design can work: we have no clue about what format is expected by the subscriber, and using the format used by the publisher may fail.

Example:

1st publisher sends a JSON-LD request
2nd subscribers uses JSON:API

Subscribers receive messages in mixed formats. It doesn’t work.

What we could do instead is:

• using the format configured as the prefered one (the first one in the list of the formats configuration) instead of hard coding JSON-LD (easy, and should already by implemented like that)
• adding a new option to configure the format to use for Mercure (a bit lire work than for the 1st proposal)
• create a topic by enabled format (like http://example.com/books.[jsonld|jsonapi|hal|...] and publish updates in all supported format at every change (more resource consuming, should be optin).

WDYT?

[toriqo]

Well, i thought about this in the beginning (about your 3rd option in fact) then i realized this is the api-platforms own “auto reply”. It’s not responding to publishers (those talk directly to mercure), it only reponds to api clients (therefore getCurrentRequest()).
So it doesn’t really matter what publishers send, isn’t it? So request format should be safe...

PS: i got it, client one using hal sends a request, client two who’s also subscribed uses jsonld. End of the internet :)

options #1 and #2 are the same as having it hardcoded, still forcing the subscriber to a certain format, rather than letting them choose the format they want.

so i'll start working on option #3, if it's ok.

[toriqo]

i realized the 3rd option involves mercure, as the topic is a request to that, not the api platform.
so i took the easy way out and used the preferred format in api_platform.yaml

[dunglas]

We shouldn’t use request stack here, because it makes the feature not fully compliant with GraphQL, and create a hard dependency to Symfony (we want to support Laravel and maybe PSR-7 at some point).

[toriqo]

The only solution i see is making use of $GLOBALS['request']... unless api-platform has some other way to get the current request

[toriqo]

btw, i think Laravel also uses Symfony's HttpFoundation...

[dunglas]

Yes we have our own abstraction layer: https://github.com/api-platform/core/blob/master/src/EventListener/ReadListener.php#L64-L69

However I'm not sure it's the good way to handle this problem. I would only make the format configurable for now.

Yes Laravel uses HttpFoundation, but it doesn't use the RequestStack class.

[toriqo]

Well, i’d say the context is way more important and required. Otherwise the mercure message will contain sensitive information about the object or object relations which someone might not include in a respose. Also, the message itself will grow a lot. I’ve had messages of 8MB and 12MB dispatched by mercure because it included information i didnt want, which i doubt it’s ok.
So i’d focus on the context rather than the format.

[dunglas]

Otherwise the mercure message will contain sensitive information about the object or object relations which someone might not include in a response.

Indeed it's a big issue. Maybe could we always use the default normalization context for now, so we don't have to depend on the request?

I’ve had messages of 8MB and 12MB dispatched by mercure because it included information i didnt want, which i doubt it’s ok.

I've another idea in mind for this specific problem: we should add the option to dispatch only the properties that changed. The client will have to patch the document it retrieved originally using the API.
To do so, we can:

1. use the Doctrine UnitOfWork to detect the changed property
2. Generate a JSON document containing following the JSON Merge Patch format (it will contain only the diff with the original document): https://tools.ietf.org/html/rfc7386
3. Set the appropriate relation in the Link header as described in the Mercure spec:

content-type: the content type of the updates that will pushed by the hub. If omitted, the subscriber MUST assume that the content type will be the same as that of the original resource. Setting the content-type attribute is especially useful to hint that partial updates will be pushed, using formats such as JSON Patch [@RFC6902] or JSON Merge Patch [@RFC7386].
https://github.com/dunglas/mercure/blob/master/spec/mercure.md#discovery

[toriqo]

But the issue will still exist on the PUT method and disabling it isn't an option.
Forcing api-platform to publish only on PATCH (and POST) again, not an option.

wdyt?

[dunglas]

Maybe was I unclear: the idea is to serialize the Mercure Payload using the JSON Merge PATCH format for all HTTP verbs (except POST of course because we need the full entity first). It's the serialization format of the update, and it's not related to the PATCH HTTP verb.

To summaries:

When the publisher send a PUT, PATCH or DELETE request, API Platform send the diff through Mercure, serialized using the JSON Merge Patch format.

[dunglas]

• we always apply default normalization groups (this should be done in this PR, JSON Merge Patch implem should be in another PR).

[toriqo]

Oh, now i get it.
Ok, i’ll change this PR to only take care of the normalization and another PR for the merge patch

[toriqo]

switching to 2.4 as bug fix