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

Allow Constructed Object to be Passed to Deserialize #79

Closed
cmorelli opened this Issue Apr 14, 2013 · 25 comments

Comments

Projects
None yet
@cmorelli

cmorelli commented Apr 14, 2013

It doesn't appear that there's a way to give an already constructed object to the deserialize method (in which case the serializer would just skip the object construction phase and begin mapping properties).

My use case is simple:
In a REST API, I want to allow a user to create or update objects. Each type of operation (create, update, delete, read) has a different set of security rules. However, with the current implementation of the deserializer (in which we are using the Doctrine object constructor), the user can issue a create request, but specify an "id" as part of the payload. The Doctrine object constructor will then return a mapped entity before deserialization begins, allowing the end user to perform an update (while the server still thinks it's doing a create).

Also, on this point:
I was considering making a separate object constructor - but I need to give some attributes to the object constructor on each use (such as the ID which it should construct an object for). Right now, the only way to get those attributes to the deserializer is to include them in the payload string that gets passed to it - which is precisely what I'd like to avoid.

There are ways to handle this in the actual REST endpoint, but none of them are really ideal.What I'd like to be able to do is pass an already-constructed object (which my REST endpoint will handle fetching) to the deserializer and just have it do it's property mapping onto the object. This way, I can always pass a blank object in create calls, while I can pass a constructed object in update calls.

Would this be possible? I could submit a PR if this is something you'd be willing to implement.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Apr 14, 2013

Contributor

I would suggest you to use the Symfony Form component for this instead. Binding values in an existing object correspond to 80% of its codebase (the remaining part being the rendering of the HTML).
See https://github.com/simplethings/SimpleThingsFormSerializerBundle for an implementation of the Form used in this way.

Binding data in an existing object requires totally different work than deserializing.

Contributor

stof commented Apr 14, 2013

I would suggest you to use the Symfony Form component for this instead. Binding values in an existing object correspond to 80% of its codebase (the remaining part being the rendering of the HTML).
See https://github.com/simplethings/SimpleThingsFormSerializerBundle for an implementation of the Form used in this way.

Binding data in an existing object requires totally different work than deserializing.

@cmorelli

This comment has been minimized.

Show comment
Hide comment
@cmorelli

cmorelli Apr 14, 2013

Forms could be used - but I lose a lot of control over the serialization/deserialization process that I currently have (and love) with JMS.

Second, I can't say I agree that it's totally different work. Isn't the whole purpose of the object constructor to allow one to return already existing objects rather than creating new ones? For example, the Doctrine object constructor works by accepting identifiers in the payload and returning managed entities from that data. I don't see how this request falls outside the realm of that?

The object constructor paradigm could also work for me, if I could easily pass arbitrary attributes to it. The only catch here is that I need to be able to pass my identifiers to construct the object from separate from the actual payload. I could hack it to work, I'd just prefer to not hack if there could be an API-supported way of doing it.

To comment again on your final sentence: binding data to an existing object should really be the exact same as binding data to a new object. The only difference being that the "existing object" presumably has some attributes already set on it. If, instead of constructing a new object, the current serializer took an already existing object - the rest of the process would be the exact same.

cmorelli commented Apr 14, 2013

Forms could be used - but I lose a lot of control over the serialization/deserialization process that I currently have (and love) with JMS.

Second, I can't say I agree that it's totally different work. Isn't the whole purpose of the object constructor to allow one to return already existing objects rather than creating new ones? For example, the Doctrine object constructor works by accepting identifiers in the payload and returning managed entities from that data. I don't see how this request falls outside the realm of that?

The object constructor paradigm could also work for me, if I could easily pass arbitrary attributes to it. The only catch here is that I need to be able to pass my identifiers to construct the object from separate from the actual payload. I could hack it to work, I'd just prefer to not hack if there could be an API-supported way of doing it.

To comment again on your final sentence: binding data to an existing object should really be the exact same as binding data to a new object. The only difference being that the "existing object" presumably has some attributes already set on it. If, instead of constructing a new object, the current serializer took an already existing object - the rest of the process would be the exact same.

@schmittjoh

This comment has been minimized.

Show comment
Hide comment
@schmittjoh

schmittjoh Apr 14, 2013

Owner

You can set the attributes on the DeserializationContext which you can access from your object constructor. Did you consider that?

Owner

schmittjoh commented Apr 14, 2013

You can set the attributes on the DeserializationContext which you can access from your object constructor. Did you consider that?

@cmorelli

This comment has been minimized.

Show comment
Hide comment
@cmorelli

cmorelli Apr 14, 2013

Am I missing how to get the context from the object constructor? I did consider that approach - and it would work fine - but it doesn't appear to get passed down into the constructor.

cmorelli commented Apr 14, 2013

Am I missing how to get the context from the object constructor? I did consider that approach - and it would work fine - but it doesn't appear to get passed down into the constructor.

@schmittjoh

This comment has been minimized.

Show comment
Hide comment
@schmittjoh

schmittjoh Apr 14, 2013

Owner

You're right. I think we should add it there.

Owner

schmittjoh commented Apr 14, 2013

You're right. I think we should add it there.

@cmorelli

This comment has been minimized.

Show comment
Hide comment
@cmorelli

cmorelli Apr 14, 2013

I definitely support that approach. Except injecting the context would be a BC break.

Thoughts?

cmorelli commented Apr 14, 2013

I definitely support that approach. Except injecting the context would be a BC break.

Thoughts?

@schmittjoh

This comment has been minimized.

Show comment
Hide comment
@schmittjoh

schmittjoh Apr 14, 2013

Owner

The ObjectConstructorInterface is not exactly user-facing. I think we can slightly change the arguments and add the context.

Owner

schmittjoh commented Apr 14, 2013

The ObjectConstructorInterface is not exactly user-facing. I think we can slightly change the arguments and add the context.

@cmorelli

This comment has been minimized.

Show comment
Hide comment
@cmorelli

cmorelli Apr 14, 2013

+1 from me. I should have some time today to get this working and submit a PR if you're busy.

cmorelli commented Apr 14, 2013

+1 from me. I should have some time today to get this working and submit a PR if you're busy.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Apr 14, 2013

Contributor

@cmorelli The issue is that you would need to handle exisitng objects in many places of the graph, not only at the root.
And did you look at the link I gave ? It supports different serialization formats

Contributor

stof commented Apr 14, 2013

@cmorelli The issue is that you would need to handle exisitng objects in many places of the graph, not only at the root.
And did you look at the link I gave ? It supports different serialization formats

@cmorelli

This comment has been minimized.

Show comment
Hide comment
@cmorelli

cmorelli Apr 14, 2013

@stof That is true - I was only considering the passing of existing objects at the root (not considering the rest of the graph). That being said, I think the solution that @schmittjoh posted is the best way to go. The object constructor can choose if it wants to use any attributes from the DeserializationContext to create its objects. This would work at any level of the graph.

cmorelli commented Apr 14, 2013

@stof That is true - I was only considering the passing of existing objects at the root (not considering the rest of the graph). That being said, I think the solution that @schmittjoh posted is the best way to go. The object constructor can choose if it wants to use any attributes from the DeserializationContext to create its objects. This would work at any level of the graph.

@evillemez

This comment has been minimized.

Show comment
Hide comment
@evillemez

evillemez Jun 11, 2013

+1 Any news on this? Same use case here, and I'm currently using a hacky solution where I parse the JMS metadata myself to manually recurse and set properties. It makes much more sense to be able to pass in an already constructed object to handle the same way as creation.

Using the form component would work, sure, but... it's a lot of extra code to load for doing something that's almost already doable.

I'm not super familiar with the internals, but if someone points me in the right direction I'd be happy to work on this if no one else currently is. I'm about to go on vacation for two weeks, but as soon as I get back I would have time to start on this around June 26th.

evillemez commented Jun 11, 2013

+1 Any news on this? Same use case here, and I'm currently using a hacky solution where I parse the JMS metadata myself to manually recurse and set properties. It makes much more sense to be able to pass in an already constructed object to handle the same way as creation.

Using the form component would work, sure, but... it's a lot of extra code to load for doing something that's almost already doable.

I'm not super familiar with the internals, but if someone points me in the right direction I'd be happy to work on this if no one else currently is. I'm about to go on vacation for two weeks, but as soon as I get back I would have time to start on this around June 26th.

@eugene-dounar

This comment has been minimized.

Show comment
Hide comment
@eugene-dounar

eugene-dounar Sep 17, 2013

Hi there!
What about adding a "target" property to DeserializationContext which contains a constructed object to serialize data into? As far as I can see it would not break BC and is super simple to implement.

@schmittjoh would you accept a PR with that?

eugene-dounar commented Sep 17, 2013

Hi there!
What about adding a "target" property to DeserializationContext which contains a constructed object to serialize data into? As far as I can see it would not break BC and is super simple to implement.

@schmittjoh would you accept a PR with that?

@schmittjoh

This comment has been minimized.

Show comment
Hide comment
@schmittjoh

schmittjoh Sep 17, 2013

Owner

What we can do easily is to add the context as an argument for the object constructor. Then, you can easily implement your own constructor as you need it.

Owner

schmittjoh commented Sep 17, 2013

What we can do easily is to add the context as an argument for the object constructor. Then, you can easily implement your own constructor as you need it.

@evillemez

This comment has been minimized.

Show comment
Hide comment
@evillemez

evillemez Nov 27, 2013

It looks like this issue was closed from #160, but are there any docs for it anywhere?

What I'm curious about is, it looks like in order to deserialize into a preexisting object, I need to change the object constructor. In the SerializerBundle I can do this in the config - but I don't necessarily want to always use that object constructor - I only want to use it in the contexts where I'm receiving data via an API. Should I configure a new serializer service for this case?

Edit: Also, does this only handle objects at the root?

evillemez commented Nov 27, 2013

It looks like this issue was closed from #160, but are there any docs for it anywhere?

What I'm curious about is, it looks like in order to deserialize into a preexisting object, I need to change the object constructor. In the SerializerBundle I can do this in the config - but I don't necessarily want to always use that object constructor - I only want to use it in the contexts where I'm receiving data via an API. Should I configure a new serializer service for this case?

Edit: Also, does this only handle objects at the root?

@eugene-dounar

This comment has been minimized.

Show comment
Hide comment
@eugene-dounar

eugene-dounar Nov 27, 2013

@evillemez you can pass a fallback constructor to your own constructor and call it when no object is passed with serialization context. See DoctrineObjectConstructor - it does the same thing. I think it would be much cleaner than having 2 serializer services .

eugene-dounar commented Nov 27, 2013

@evillemez you can pass a fallback constructor to your own constructor and call it when no object is passed with serialization context. See DoctrineObjectConstructor - it does the same thing. I think it would be much cleaner than having 2 serializer services .

@evillemez

This comment has been minimized.

Show comment
Hide comment
@evillemez

evillemez Nov 27, 2013

@eugene-dounar Ah, ok... I see. From the code, this looks like it only handles serializing into a pre-existing object at the root level of the graph, is that actually the case or am I misunderstanding it?

evillemez commented Nov 27, 2013

@eugene-dounar Ah, ok... I see. From the code, this looks like it only handles serializing into a pre-existing object at the root level of the graph, is that actually the case or am I misunderstanding it?

@eschwartz

This comment has been minimized.

Show comment
Hide comment
@eschwartz

eschwartz Sep 26, 2014

Thanks @eugene-dounar -- this is so great! We were going crazy in my office trying to figure out how to do partial updates to Doctrine-ORM-managed entities.

Using the deserializer for partial updates (eg PUT requests) seems like a common enough use case that the InitializedObjectConstructor (test fixture) should be made a usable part of the serializer service. At the very least, could we add some documentation on how to accomplish this?

We ended up copy/pasting the InitializedObjectConstructor into our code base. For anyone else banging their heads against their desks, here's what we ended up doing (using ZF2):

// module.config.php
'service_manager' => array(
  'factories' => array(
    // Create a jms serializer as a ZF2 service,
    // configured to use the InitializedObjectCtor
    'serializer' => function() {
      // ...
      $defaultObjCtor = new \JMS\Serializer\Construction\UnserializeObjectConstructor();
      $initializedObjCtor = new \MyApp\path\to\copy\and\pasted\InitializedObjectCtor($defaultObjCtor);

      return \JMS\Serializer\SerializerBuilder::create()
        ->setObjectConstructor($initializedObjCtor)
        ->build();
    }
  )
)

// MyApp\Controller\UserRestController
// ...
public function update($id, array $data) {
  // Grab the existing user from the ORM
  $user = $this->doctrineEntityManager->findById($id);

  // Create a deserialization context, targeting the existing user
  $context = new \JMS\Serializer\DeserializationContext();
  $context->attributes->set('target', $user);

  // Deserialize the data "on to" to the existing user
  $this->serializer->deserialize(json_encode($data), 'MyApp\Model\User', 'json', $context); 

  // Save the updated user
  $this->doctrineEntityManager->persist($user);
  $this->doctrineEntityManager->flush();

  // return ... 
}

The whole context injection thing is a little rough on the eyes, so we'll probably end up wrapping the JMS Serializer to accept an object as a second param (I'm a JS dev, so I'm totally cool with mixing up my parameter types :p )

eschwartz commented Sep 26, 2014

Thanks @eugene-dounar -- this is so great! We were going crazy in my office trying to figure out how to do partial updates to Doctrine-ORM-managed entities.

Using the deserializer for partial updates (eg PUT requests) seems like a common enough use case that the InitializedObjectConstructor (test fixture) should be made a usable part of the serializer service. At the very least, could we add some documentation on how to accomplish this?

We ended up copy/pasting the InitializedObjectConstructor into our code base. For anyone else banging their heads against their desks, here's what we ended up doing (using ZF2):

// module.config.php
'service_manager' => array(
  'factories' => array(
    // Create a jms serializer as a ZF2 service,
    // configured to use the InitializedObjectCtor
    'serializer' => function() {
      // ...
      $defaultObjCtor = new \JMS\Serializer\Construction\UnserializeObjectConstructor();
      $initializedObjCtor = new \MyApp\path\to\copy\and\pasted\InitializedObjectCtor($defaultObjCtor);

      return \JMS\Serializer\SerializerBuilder::create()
        ->setObjectConstructor($initializedObjCtor)
        ->build();
    }
  )
)

// MyApp\Controller\UserRestController
// ...
public function update($id, array $data) {
  // Grab the existing user from the ORM
  $user = $this->doctrineEntityManager->findById($id);

  // Create a deserialization context, targeting the existing user
  $context = new \JMS\Serializer\DeserializationContext();
  $context->attributes->set('target', $user);

  // Deserialize the data "on to" to the existing user
  $this->serializer->deserialize(json_encode($data), 'MyApp\Model\User', 'json', $context); 

  // Save the updated user
  $this->doctrineEntityManager->persist($user);
  $this->doctrineEntityManager->flush();

  // return ... 
}

The whole context injection thing is a little rough on the eyes, so we'll probably end up wrapping the JMS Serializer to accept an object as a second param (I'm a JS dev, so I'm totally cool with mixing up my parameter types :p )

@terwey

This comment has been minimized.

Show comment
Hide comment
@terwey

terwey Jan 31, 2015

I followed @eschwartz recommendation and just did a

cp vendor/jms/serializer/tests/JMS/Serializer/Tests/Fixtures/InitializedObjectConstructor.php src/myApp/

Modified the namespace in the InitializedObjectConstructor.php and it all works like a charm after this.

Can this class be moved into a more proper and permanent location so I don't have to maintain it myself?

terwey commented Jan 31, 2015

I followed @eschwartz recommendation and just did a

cp vendor/jms/serializer/tests/JMS/Serializer/Tests/Fixtures/InitializedObjectConstructor.php src/myApp/

Modified the namespace in the InitializedObjectConstructor.php and it all works like a charm after this.

Can this class be moved into a more proper and permanent location so I don't have to maintain it myself?

@RedEdgeKatelyn

This comment has been minimized.

Show comment
Hide comment
@RedEdgeKatelyn

RedEdgeKatelyn Mar 3, 2015

For anyone trying to do this in Symfony, I copied vendor/jms/serializer/tests/JMS/Serializer/Tests/Fixtures/InitializedObjectConstructor.php to a Services folder in my Bundle and added this to config.yml:

services:
    jms_serializer.object_constructor:
        alias: jms_serializer.initialized_object_constructor
        public: false
    jms_serializer.initialized_object_constructor:
         class:        VendorName\Bundle\ApiBundle\Services\InitializedObjectConstructor
         arguments:    ["@jms_serializer.doctrine_object_constructor"]

RedEdgeKatelyn commented Mar 3, 2015

For anyone trying to do this in Symfony, I copied vendor/jms/serializer/tests/JMS/Serializer/Tests/Fixtures/InitializedObjectConstructor.php to a Services folder in my Bundle and added this to config.yml:

services:
    jms_serializer.object_constructor:
        alias: jms_serializer.initialized_object_constructor
        public: false
    jms_serializer.initialized_object_constructor:
         class:        VendorName\Bundle\ApiBundle\Services\InitializedObjectConstructor
         arguments:    ["@jms_serializer.doctrine_object_constructor"]
@faheemhameed

This comment has been minimized.

Show comment
Hide comment
@faheemhameed

faheemhameed Jan 30, 2016

Note about the Symfony helped a lot. Thanks!

faheemhameed commented Jan 30, 2016

Note about the Symfony helped a lot. Thanks!

@tom10271

This comment has been minimized.

Show comment
Hide comment
@tom10271

tom10271 Mar 4, 2016

@RedEdgeKatelyn I am not sure code changed or not but your code snippet is not work.

Here is the workable version:

    jms_serializer.object_constructor:
        alias: jms_serializer.initialized_object_constructor
        public: false

    jms_serializer.initialized_object_constructor:
         class:        Acme\YourBundle\Serializer\InitializedObjectConstructor
         arguments:    ["@jms_serializer.unserialize_object_constructor"] 

tom10271 commented Mar 4, 2016

@RedEdgeKatelyn I am not sure code changed or not but your code snippet is not work.

Here is the workable version:

    jms_serializer.object_constructor:
        alias: jms_serializer.initialized_object_constructor
        public: false

    jms_serializer.initialized_object_constructor:
         class:        Acme\YourBundle\Serializer\InitializedObjectConstructor
         arguments:    ["@jms_serializer.unserialize_object_constructor"] 
@bendbennett

This comment has been minimized.

Show comment
Hide comment
@bendbennett

bendbennett May 30, 2016

Thanks @RedEdgeKatelyn and @tom10271 as a Symfony user your solution worked for me. But I couldn't help but feel that this should be soluble through configuration.

Stumbled across a SO post and followed the suggested configuration change (in app/config/services.yml):

services:
    jms_serializer.object_constructor:
        alias: jms_serializer.doctrine_object_constructor

This worked for me when I then used JMS serializer:

$this->serializer->deserialize($request->getContent(), 'MyApp\Entity\User', 'json');

Only thing to watch out for is that the id of the object must appear in the body of the request, otherwise instead of updating a pre-hydrated instance of User, a new instance of User will be created.

Can also confirm that the additional changes suggested in the SO post suggested for Mongo also work:

services:
    jms_serializer.doctrine_object_constructor:
        class:        %jms_serializer.doctrine_object_constructor.class%
        public:       false
        arguments:    ["@doctrine_mongodb", "@jms_serializer.unserialize_object_constructor"]

    jms_serializer.object_constructor:
        alias: jms_serializer.doctrine_object_constructor

Kudos to Heyflynn and con

Works with PHP 7 too :)

bendbennett commented May 30, 2016

Thanks @RedEdgeKatelyn and @tom10271 as a Symfony user your solution worked for me. But I couldn't help but feel that this should be soluble through configuration.

Stumbled across a SO post and followed the suggested configuration change (in app/config/services.yml):

services:
    jms_serializer.object_constructor:
        alias: jms_serializer.doctrine_object_constructor

This worked for me when I then used JMS serializer:

$this->serializer->deserialize($request->getContent(), 'MyApp\Entity\User', 'json');

Only thing to watch out for is that the id of the object must appear in the body of the request, otherwise instead of updating a pre-hydrated instance of User, a new instance of User will be created.

Can also confirm that the additional changes suggested in the SO post suggested for Mongo also work:

services:
    jms_serializer.doctrine_object_constructor:
        class:        %jms_serializer.doctrine_object_constructor.class%
        public:       false
        arguments:    ["@doctrine_mongodb", "@jms_serializer.unserialize_object_constructor"]

    jms_serializer.object_constructor:
        alias: jms_serializer.doctrine_object_constructor

Kudos to Heyflynn and con

Works with PHP 7 too :)

@goetas

This comment has been minimized.

Show comment
Hide comment
@goetas

goetas Apr 24, 2017

Collaborator

ObjectConstructorInterface has been updated and is possible to get objects from the context attributes

Collaborator

goetas commented Apr 24, 2017

ObjectConstructorInterface has been updated and is possible to get objects from the context attributes

@karousn

This comment has been minimized.

Show comment
Hide comment
@karousn

karousn Sep 17, 2018

@goetas : if possible to have an example of implementation for that, i looking to use context attributes but until now $context->attributes->all() have an empty array, i don't know how can i set my object there.

karousn commented Sep 17, 2018

@goetas : if possible to have an example of implementation for that, i looking to use context attributes but until now $context->attributes->all() have an empty array, i don't know how can i set my object there.

@goetas

This comment has been minimized.

Show comment
Hide comment
@goetas

goetas Sep 17, 2018

Collaborator

@karousn you can have a look at it on

$objectConstructor = new InitializedBlogPostConstructor();

Collaborator

goetas commented Sep 17, 2018

@karousn you can have a look at it on

$objectConstructor = new InitializedBlogPostConstructor();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment