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

Implement the handler of serialization to process the proxy of Doctrine ORM #46

Merged
merged 3 commits into from
Nov 27, 2011

Conversation

yethee
Copy link
Contributor

@yethee yethee commented Nov 22, 2011

I tried to implement a processing the proxy classes through the custom handler.

Note that not processing serialization callbacks if they defined on level of parents class of the proxy class. Since the handler invokes to accept() method of the navigator and passes type of parent class to second argument instead of null.

{
public function serialize(VisitorInterface $visitor, $data, $type, &$handled)
{
if ($data instanceof Proxy && get_class($data) === $type) {
Copy link
Owner

Choose a reason for hiding this comment

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

Is the get_class() check necessary? It should always be true, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, here need determine already handled the proxy or no.
In the beginning I used the test state of __isInitialized__ property...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below the accept() method is invoked, passing the type of parent class, so this handler called twice. We need to determine whether the entity is already initialized or no.

There is one issue. When accept() method is called from this handler (the $type argument is not null) not handled the callbacks of serialization, because the $isSerialization is false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we pass the type of parent class instead of null to accept() method, then skip loading of metadata for the proxy class.

@yethee
Copy link
Contributor Author

yethee commented Nov 23, 2011

I reviewed implementation of this handler and guess that not good... Invoke accept() method in this handler looks like a hack.

What do you think about the refactoring of the graph navigator, to extract logic of handling object from accept() method to separate methods?

I've done base implementation of refactoring in this commit 03aec39. Added a two new methods, acceptObjectSerialize() and acceptObjectDeserialize().

This solution allows us to make a second pass of the object through the graph navigator in the custom handlers.

@schmittjoh
Copy link
Owner

Yes, I agree with your assessment.

What I'd like to do instead is to pass to the GraphNavigator whether it is serializing, or deserializing upon instantiation (i.e. in Serializer::serialize() and Serializer::deserialize()). Then, we do not need to split the GraphNavigator, and consequentially also do not need to duplicate code.

@schmittjoh
Copy link
Owner

I've pushed the change:
f06cc98

This should make the implementation easier.

@yethee
Copy link
Contributor Author

yethee commented Nov 27, 2011

Added an implementation of detachObject() method.
I've rebased this branch on your master branch.

@schmittjoh schmittjoh merged commit 65ede16 into schmittjoh:master Nov 27, 2011
@schmittjoh
Copy link
Owner

Thanks, merged it.

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.

None yet

3 participants