Skip to content
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

Moves all state to dedicated context class #47

Merged
merged 7 commits into from
Feb 23, 2013
Merged

Moves all state to dedicated context class #47

merged 7 commits into from
Feb 23, 2013

Conversation

schmittjoh
Copy link
Owner

This commit moves all state that might change between serialization/deserialization runs into a dedicated class. This class is available to all classes which are concerned with the serialization and will make it super easy to propagate user specific settings to these classes.

In terms of BC, this affects mostly integration points like FOSRestBundle (cc @lsmith77) and will also affect some custom handlers of users. It will also require some of the pending PRs to be updated.

At this point, I'm interested in feedback whether I missed something that is broken by this change, or use-cases which this change makes impossible.

This commit moves all state that might change between serialization/deserialization
runs into a dedicated class. This class is available to all classes which are
concerned with the serialization and will it make super easy to propagate user
specific settings to these classes.

return null;
}

if ($isSerializing) {
foreach ($metadata->preSerializeMethods as $method) {
/** @var $method \ReflectionMethod */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be better to put the right phpdoc on $metadata->preSerializeMethods so that it is infered by the IDE (\ReflectionMethod[]) ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this later on, see below.

@lsmith77
Copy link
Contributor

i guess the Context is in many ways similar to what I have already done with the serializer specific methods in the View class in FOSRestBundle, so I guess it might actually simplify some things.

@schmittjoh
Copy link
Owner Author

@lsmith77, yeah I think you'll be able to simplify the API of the View class, and you would also not need to duplicate the API anymore.

I've pushed a small update which splits the context into two different sub-classes, one for serialization and another for deserialization. We probably also need to make a similar change in the GraphNavigator.

I also added a nice example how this new extension point can be used to invoke the Symfony Validator after the root object has been deserialized:

<?php

$serializer->deserialize($data, 'json', 'class', 
    DeserializationContext::create()
        ->setAttribute('validation_groups', array('Group1', 'Group2')));

@schmittjoh schmittjoh merged commit 934dd3c into master Feb 23, 2013
@lsmith77
Copy link
Contributor

anyone motivated to work on a refactoring of FOSRestBundle for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants