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

Dispatch an event when proxy initialization fails #1336

Merged
merged 1 commit into from
Jan 24, 2016

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Jan 8, 2016

Supersedes #532, Closes #1258.

This builds on top of the code added in #532. The new DocumentNotFoundEventArgs class contains the proxy object and its identifier. Event subscribers can choose to handle the missing document and call disableException() on the $eventArgs object to prevent the DocumentNotFound exception from being thrown.

As mentioned in the ticket, this event is not available as a lifecycle callback because we don't have a valid object on which to call a lifecycle callback.

* Disable the throwing of an exception
*
* This method indicates to the proxy initializer that the missing document
* has been handled and no exception should be thrown. This can't be reset.
Copy link
Member

Choose a reason for hiding this comment

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

Why this can't be reset?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea was that once a missing proxy has been handled (e.g. the object has been properly initialized) there shouldn't be the need for a different event subscriber to re-enable that exception.

Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be better to have stopPropagation for that instead of combining two things (and our events are not supporting it anyway as of now :D )

@alcaeus alcaeus force-pushed the proxy-initialization-failed-event branch from eaa7fe5 to a241bbd Compare January 8, 2016 10:23
* @param object $proxy
* @param string $identifier
*/
public function __construct($proxy, $identifier)
Copy link
Member

Choose a reason for hiding this comment

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

How about exposing corresponding DocumentManager? It'd make event more powerful

@alcaeus
Copy link
Member Author

alcaeus commented Jan 8, 2016

To be honest, I'm not sure about the whole "prevent this exception from being thrown" thing. It was an idea that popped in my head last night and I thought I'd go with it. In theory, it makes sense but I think in practice there will be issues with that:

  • The proxy object is empty, e.g. no initialization has been taken care of
  • The proxy object has a different constructor, so an event subscriber can't call the original constructor of the class to prepare it as an empty object
  • There may not be null-protection checks in place so instead of exceptions users might be left with fatal errors

@alcaeus alcaeus force-pushed the proxy-initialization-failed-event branch from a241bbd to 404e655 Compare January 8, 2016 10:57
@alcaeus
Copy link
Member Author

alcaeus commented Jan 8, 2016

After additional discussion we decided to completely open up the disableException behavior for subscribers to disable and re-enable the exception as they wish. When disabling the exception, a subscriber must ensure the proxy object is properly initialized.

@alcaeus alcaeus force-pushed the proxy-initialization-failed-event branch from 404e655 to 04f4555 Compare January 24, 2016 18:20
alcaeus added a commit that referenced this pull request Jan 24, 2016
Dispatch an event when proxy initialization fails
@alcaeus alcaeus merged commit 453ef86 into doctrine:master Jan 24, 2016
@alcaeus alcaeus deleted the proxy-initialization-failed-event branch January 24, 2016 18:35
@szappacosta
Copy link

@alcaeus Do you have an example of implementation ?
I would like to disable this exception everywhere, and retrieve a null value when i call the object.

@alcaeus
Copy link
Member Author

alcaeus commented Nov 7, 2017

The documentation specifies how you can disable the exception: http://docs.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/reference/events.html#documentnotfound. Replacing the proxy object with null is not possible: proxy initialization is called from within a proxy object and we can't replace an object itself with null. This is not possible with the current proxy system.

@szappacosta
Copy link

With the exception disabled, what happens if i call for example $this->getImage().
I will have a proxy object of Image class ?
Thanks for your answer =)

@alcaeus
Copy link
Member Author

alcaeus commented Nov 7, 2017

Let's say you loaded a document User that has a (non-existing) reference to an Image. Calling $user->getImage() will always yield an instance of an Image: if the Image instance is already managed by the document manager, it will return that instance, regardless of whether it's a proxy or not. If it's not, the document manager will create an instance of a proxy class that extends the original Image. It's important to note that just calling $user->getImage() will not trigger proxy initialization, and neither will calling $user->getImage()->getId().

Only when calling any other method or accessing a public property of $user->getImage() does proxy initialization kick in: the initializer tasks the DocumentPersister with loading data for the ID that is already known. If the initializer doesn't find a document, the documentNotFound event is thrown. If you catch that event, you get the uninitialized proxy instance and can do whatever you want with it. If you call disableException, the initializer will not throw a DocumentNotFoundException, but the proxy object will always be in the state you left it in.

It's important to note that at the time of initialization, we can't reset the invalid proxy instance. The proxy object might be used by multiple documents (think of multiple users holding a reference to the image), and since the proxy object itself is responsible for initializing itself, we can't change anything about the proxy object because you can't overwrite the value of $this.

If you want to control what happens with an invalid reference, initialize it in a controlled way while you still know the owning object and catch the DocumentNotFoundException that is thrown. I hope this helps clear up some confusion.

@szappacosta
Copy link

It is, very detailed answer. Thanks a lot

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

Successfully merging this pull request may close these issues.

3 participants