Use decorated serializer to resolve url of uploaded files #916
Conversation
This matches the recommended approach of using our extension points, and also works for GraphQL
| if (!$mediaObject instanceof MediaObject) { | ||
| continue; | ||
| } | ||
| public function denormalize($data, $class, $format = null, array $context = []) |
There was a problem hiding this comment.
I don't think we need to denormalize a MediaObject. You can only use the NormalizerInterface IMO.
There was a problem hiding this comment.
You're probably right - I just took the example from the Serialization docs and modified it accordingly. Will update this 👍
There was a problem hiding this comment.
The problem is, if we're declaring this as a decorating service in Symfony DI, then it should implement the same interfaces. I'd suggest that we do NOT declare this as a decorating service (i.e. no decorates).
This way we can also remove SerializerAwareInterface.
There was a problem hiding this comment.
@teohhanhui Sounds good - does that change the behavior in any way? Is there any additional configuration required if you do not decorate the existing service?
| $mediaObject->contentUrl = $this->storage->resolveUri($mediaObject, 'file'); | ||
| public function setSerializer(SerializerInterface $serializer) | ||
| { | ||
| if($this->decorated instanceof SerializerAwareInterface) { |
There was a problem hiding this comment.
| if($this->decorated instanceof SerializerAwareInterface) { | |
| if ($this->decorated instanceof SerializerAwareInterface) { |
I know this CS is in the other part as well.
| @@ -164,64 +164,65 @@ final class CreateMediaObjectAction | |||
| Returning the plain file path on the filesystem where the file is stored is not useful for the client, which needs a | |||
| URL to work with. | |||
|
|
|||
| An [event subscriber](events.md#custom-event-listeners) could be used to set the `contentUrl` property: | |||
| We will be [decorating a serializer](serialization.md#decorating-a-serializer-and-adding-extra-data) to resolve the `contentUrl` property: | |||
There was a problem hiding this comment.
That section you reference is wrongly named. It should be "decorating a normalizer".
There was a problem hiding this comment.
@teohhanhui What do you mean? I took both the title and the link from the existing documentation.
(If we move away from the decoration, then this section has to be updated anyway.)
There was a problem hiding this comment.
It could be renamed, but in another PR as here it's just a link.
| use Vich\UploaderBundle\Storage\StorageInterface; | ||
|
|
||
| final class ResolveMediaObjectContentUrlSubscriber implements EventSubscriberInterface | ||
| final class MediaObjectContentUrlNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface |
There was a problem hiding this comment.
Just name it MediaObjectNormalizer 😄
There was a problem hiding this comment.
I tried to match the previous name somewhat, but shortening it is fine with me.
| if (!$mediaObject instanceof MediaObject) { | ||
| continue; | ||
| } | ||
| public function denormalize($data, $class, $format = null, array $context = []) |
There was a problem hiding this comment.
The problem is, if we're declaring this as a decorating service in Symfony DI, then it should implement the same interfaces. I'd suggest that we do NOT declare this as a decorating service (i.e. no decorates).
This way we can also remove SerializerAwareInterface.
ghost
mentioned this pull request
|
Superseded by #1339. Thank you @lukasluecke. |
This matches the recommended approach of using our extension points, and also works for GraphQL