Core: API Platform does not play well with immutable resource

[gorghoa]

Hi :wave:

As far as I understand, DataPersisterInterface::persist: void expect the resource to be mutated (at least with autocomputed identifier, the created identifier is expected).

It works great with system like doctrine, but if one is dealing with a persistence layer with some kind of immutable entities, he’s screwed (sorry, my vocabulary lacks of a less familiar term :'( ).

I don’t see the point to tight an object reference to the resource’s data.

I would rather have DataPersisterInterface::persist returning an object of the same resource class. The returned object would then be normalized, serialized, etc as usual.

The conceptual signature then should be something like DataPersistierInterface::persist<T>(T $resourceClasse): T

I don’t think this will be a huge impact on performance.

The trick may be to prevent a BC break. While don’t BC breaking the doctrine bridge will be easy, implementation in the wild could be impacted.

What do you think?

https://github.com/api-platform/core/blob/d2b657e2798c724ade0bd1bcd3bd089f7f143922/src/DataPersister/DataPersisterInterface.php#L37

[gorghoa]

Maybe the BC break could be avoided by using some kind of a flag (DI tag, or check if the persister implements an interface)?

Conceptually in ChainDataPersister.php#L50 :

    public function persist($data)
    {
        foreach ($this->persisters as $persister) {
            if ($persister->supports($data)) {
                if ($persister instanceof SideEffectFreeDatePersisterInterface::class) {
                   return $persister->persist($data);
               } else {
                   $persister->persist($data);
                   return $data;
               }
            }
        }
    }

(I know this foreach if if is really ugly, please focus on the goal ;) )

I have no idea of the impact in term of performance regarding api platform standards. Although I think using DI flags might be suppler and more efficient than using instanceof.

[dunglas]

👍for the idea.
We can just update the interface to indicate that DataPersisterInterface::persist returns an object, and all existing provider to return the object. This is not a BC break because it's will still be possible to not use the returned data, and this method as no typehint defined (because we don't support PHP 7.1 yet), so it's now or never!