Skip to content

Fix handling of unmapped properties in proxy objects#2698

Merged
alcaeus merged 1 commit intodoctrine:2.9.xfrom
alcaeus:fix-unmapped-properties-in-proxy
Nov 18, 2024
Merged

Fix handling of unmapped properties in proxy objects#2698
alcaeus merged 1 commit intodoctrine:2.9.xfrom
alcaeus:fix-unmapped-properties-in-proxy

Conversation

@alcaeus
Copy link
Member

@alcaeus alcaeus commented Nov 15, 2024

Q A
Type bug
BC Break no
Fixed issues

Summary

While working on a project that contained unmapped hooked properties, I realised that proxies try to unset these hooked properties, despite them not even being mapped properties. Note that this doesn't mean ODM supports hooked properties - it just removes an edge case where you can't use hooked properties at all, even when they aren't mapped and thus shouldn't be handled by ODM at all.

To work around this issue, when creating a proxy we now also skip all properties of the class that aren't mapped, leaving them set and not triggering initialisation when they are accessed.

@alcaeus alcaeus requested a review from GromNaN November 15, 2024 13:22
@alcaeus alcaeus force-pushed the fix-unmapped-properties-in-proxy branch from a8186c0 to a8500a7 Compare November 15, 2024 13:26
@melkamar
Copy link
Contributor

Sorry about necroposting, but after upgrading to 2.11 recently, I think we have stumbled onto this and it's causing a "bug" in our code. I used quotes because I am not sure if our observed behavior is a bug or works-as-intended.

Our simplified scenario:

  • Document has mappedProperty and unmappedProperty
  • When the document is fetched from the database in its lazy state, unmappedProperty is null
  • We have a service that inserts stuff into the unmappedProperty when the document is loaded: #[AsDocumentListener(event: Events::postLoad)] (...)
  • Our assumption (which seems to have broken with this PR) is that when we access any property, we can rely on the document to get fully loaded.
  • In order to work around this, we had to resort to make a "dummy fetch" to force the loading.
class Document
{
    private int $mappedProperty;
    private ServiceInterface $unmappedProperty;

    public function getUnmappedProperty()
    {
        $this->mappedProperty; // Dummy call to get proxy to initialize
        return $this->unmappedProperty;
    }

    /**
     * Called from a listener on Events::postLoad
     */
    public function initialize(ServiceInterface $service)
    {
        $this->unmappedProperty = $service;
    }
}

Is this expected? Is our (admittedly awkward) scenario unsupported? Or should I open an issue with this?

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.

4 participants