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

ci: add sf 7.1 #622

Merged
merged 1 commit into from
Jun 14, 2024
Merged

Conversation

nikophil
Copy link
Member

@nikophil nikophil commented Jun 10, 2024

our proxy system does not work with symfony/var-exporter 7.1:

TypeError: Value of type null returned from ZenstruckFoundryTestsFixtureDocumentGenericDocumentProxy::__get() must be compatible with unset property ZenstruckFoundryTestsFixtureDocumentGenericDocumentProxy::$_autoRefresh of type bool

/home/runner/work/foundry/foundry/src/Persistence/IsProxy.php:118

or

TypeError: Cannot assign null to property AppDomainProgrammingProxy::$_autoRefresh of type bool

/app/opa-v4/vendor/zenstruck/foundry/src/Persistence/IsProxy.php:48

I don't understand why it fails now, because everything is fine in 7.0.8 and nothing big was release since then https://github.com/symfony/var-exporter/releases

Here is how a proxy class generated with ProxyGenerator looks like:

<?php

class ZenstruckFoundryTestsFixtureEntityGenericEntityProxy extends \Zenstruck\Foundry\Tests\Fixture\Entity\GenericEntity implements \Zenstruck\Foundry\Persistence\Proxy, \Symfony\Component\VarExporter\LazyObjectInterface
{
    use \Zenstruck\Foundry\Persistence\IsProxy, \Symfony\Component\VarExporter\LazyProxyTrait;

    private const LAZY_OBJECT_PROPERTY_SCOPES = [
        "\0".'Zenstruck\\Foundry\\Tests\\Fixture\\Model\\GenericModel'."\0".'date' => ['Zenstruck\\Foundry\\Tests\\Fixture\\Model\\GenericModel', 'date', null],
        "\0".'Zenstruck\\Foundry\\Tests\\Fixture\\Model\\GenericModel'."\0".'prop1' => ['Zenstruck\\Foundry\\Tests\\Fixture\\Model\\GenericModel', 'prop1', null],
        'date' => ['Zenstruck\\Foundry\\Tests\\Fixture\\Model\\GenericModel', 'date', null],
        'id' => [parent::class, 'id', null],
        'prop1' => ['Zenstruck\\Foundry\\Tests\\Fixture\\Model\\GenericModel', 'prop1', null],
    ];

    public function getProp1(): string
    {
        $this->_autoRefresh();

        if (isset($this->lazyObjectReal)) {
            return ($this->lazyObjectState->realInstance ??= ($this->lazyObjectState->initializer)())->getProp1(...\func_get_args());
        }

        return parent::getProp1(...\func_get_args());
    }

    public function setProp1(string $prop1): void
    {
        $this->_autoRefresh();

        if (isset($this->lazyObjectReal)) {
            ($this->lazyObjectState->realInstance ??= ($this->lazyObjectState->initializer)())->setProp1(...\func_get_args());
        } else {
            parent::setProp1(...\func_get_args());
        }
    }

    public function getDate(): ?\DateTimeImmutable
    {
        $this->_autoRefresh();

        if (isset($this->lazyObjectReal)) {
            return ($this->lazyObjectState->realInstance ??= ($this->lazyObjectState->initializer)())->getDate(...\func_get_args());
        }

        return parent::getDate(...\func_get_args());
    }

    public function setDate(?\DateTimeImmutable $date): void
    {
        $this->_autoRefresh();

        if (isset($this->lazyObjectReal)) {
            ($this->lazyObjectState->realInstance ??= ($this->lazyObjectState->initializer)())->setDate(...\func_get_args());
        } else {
            parent::setDate(...\func_get_args());
        }
    }
}

// Help opcache.preload discover always-needed symbols
class_exists(\Symfony\Component\VarExporter\Internal\Hydrator::class);
class_exists(\Symfony\Component\VarExporter\Internal\LazyObjectRegistry::class);
class_exists(\Symfony\Component\VarExporter\Internal\LazyObjectState::class);

@nikophil nikophil force-pushed the depdencies/compatibility-with-7.1 branch 4 times, most recently from 5c3fd75 to 3333722 Compare June 10, 2024 11:20
@nicolas-grekas
Copy link

I see that autorefresh is called while the entity might not be initialized yet?
Did you declare the _autoRefresh property as being "skipped" when constructing the lazy object?

@nikophil
Copy link
Member Author

nikophil commented Jun 11, 2024

Hi @nicolas-grekas

I see that autorefresh is called while the entity might not be initialized yet?

hmmm indeed, maybe it would make sense to only call it if the object is initialized

Did you declare the _autoRefresh property as being "skipped" when constructing the lazy object?

I don't think 🤔 Not sure to see how to do that... Would that help in our problem? The property $_autoRefresh is coming from IsProxy trait which is used directly in the proxy class, I would have expected it to be initialized with its default property.

You can see here how the proxy is created.

@nikophil
Copy link
Member Author

Something which is super strange (for me, at least 😅) is that I have plenty of deprecations which say for instance:

Creation of dynamic property Zenstruck\Foundry\Tests\Fixture\Entity\Address\StandardAddress::$_autoRefresh is deprecated

meaning $this in the IsProxy trait context refers to StandardAddress and not to ZenstruckFoundryTestsFixtureEntityAddressStandardAddressProxy class

@nicolas-grekas
Copy link

Just follow my advice, the current use is not compliant with the lazy objects. Chose one way or the other but the current one is indeed broken ;)

@nikophil
Copy link
Member Author

nikophil commented Jun 12, 2024

Hi @nicolas-grekas

I understand our implementation is broken, but I don't see how to fix it 😅

how could I declare the property to be skipped, please? this sounds like the simplest solution

thanks!

@nicolas-grekas
Copy link

Check the signature of the factory method you're calling.

@nikophil
Copy link
Member Author

You mean createLazyProxy()?

LazyProxyTrait::createLazyProxy() does not allow to do this 🤔

public static function createLazyProxy(\Closure $initializer, ?object $instance = null): static

only LazyProxyTrait::createLazyGhost() does:

public static function createLazyGhost(\Closure $initializer, ?array $skippedProperties = null, ?object $instance = null): static

@nikophil nikophil force-pushed the depdencies/compatibility-with-7.1 branch from 3333722 to bf11ca7 Compare June 12, 2024 16:38
@kbond kbond merged commit b76c294 into zenstruck:2.x Jun 14, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants