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

[2.3.2] consecutive _real() calls return different objects #808

Open
KDederichs opened this issue Feb 7, 2025 · 13 comments
Open

[2.3.2] consecutive _real() calls return different objects #808

KDederichs opened this issue Feb 7, 2025 · 13 comments
Labels
bug Something isn't working

Comments

@KDederichs
Copy link
Contributor

Like the title says, since 2.3.2 calling _real() multiple times returns a different object every time.

This used to work in 2.3.1 but starting from 2.3.2 only the second element is contained in the collection, prompting me to believe I get a new 'real' object every time I call it.

        $boxSubscription->_real()->addApplicationPdf(base64_decode(SignatureFixture::BASE64));
        $boxSubscription->_real()->addKvForm(base64_decode(SignatureFixture::BASE64));
        $boxSubscription->_save();

Is that intended? I'd assume it should always return the same real instance of the entity

@nikophil
Copy link
Member

nikophil commented Feb 9, 2025

Hi @KDederichs

Is that intended?

absolutely not

I'm trying to reproduce your problem, but I don't find how
see #809

Could you elaborate, please?

@KDederichs
Copy link
Contributor Author

@nikophil I PR'd you a failing test case

@nikophil
Copy link
Member

nikophil commented Feb 9, 2025

perfect!

@KDederichs
Copy link
Contributor Author

Oh btw, it's the same code snipped that gave you this gem:
#710

Dunno if it's in any way related, just in case you're wondering why I use _real() there....
So basically atm I can not use anything higher than 2.3.1 ^^

@nikophil
Copy link
Member

nikophil commented Feb 10, 2025

#710 is a bug that we cannot fix for now, and I think we will never be able to fix. One of the stuff that makes me says that we should drop this whole proxy system that creates more problems than it fixes 🤷

I PR'd you a failing test case

I'm looking forward to it, I'm still not capable to reproduce your problem

@KDederichs
Copy link
Contributor Author

Oh I think there's a miscommunication here, when I said that I actually DID PR you the test case (to your test repo)
nikophil#7

@nikophil
Copy link
Member

haha ok, that's how I did understand your sentence, but this never come up in my notifications 🤷

I'll check this really soon

@KDederichs
Copy link
Contributor Author

yeah Github has been a bit inconsistent with notifications the last week or so I feel...
Thanks for taking a look though :)

@nikophil
Copy link
Member

@KDederichs hum ok, it is a OneToMany! based on your example here, I was thinking it was an array of strings

Still, I cannot reproduce: https://github.com/zenstruck/foundry/pull/809/files

could you make my new test fail? (please do it in this repo, no problem to add another PR)

@KDederichs
Copy link
Contributor Author

Sure, I'll probably have to bother github workers to see if fails though, locally I'm getting this very strange error, no clue why:

An error occurred inside PHPUnit.

Message:  Method Zenstruck\Foundry\Tests\Integration\ORM\EntityRelationship\ProxyEntityFactoryRelationshipTest::() does not exist
Location: /Users/kaidederichs/Source_Code/foundry/tests/Fixture/DoctrineCascadeRelationship/ChangesEntityRelationshipCascadePersist.php:88

#0 /Users/kaidederichs/Source_Code/foundry/tests/Fixture/DoctrineCascadeRelationship/ChangesEntityRelationshipCascadePersist.php(88): ReflectionMethod->__construct('Zenstruck\\Found...', '')
#1 /Users/kaidederichs/Source_Code/foundry/vendor/phpunit/phpunit/src/Metadata/Api/DataProvider.php(169): Zenstruck\Foundry\Tests\Integration\ORM\EntityRelationship\EntityFactoryRelationshipTestCase::provideCascadeRelationshipsCombinations()
#2 /Users/kaidederichs/Source_Code/foundry/vendor/phpunit/phpunit/src/Metadata/Api/DataProvider.php(67): PHPUnit\Metadata\Api\DataProvider->dataProvidedByMethods('Zenstruck\\Found...', 'provideCascadeR...', Object

@nikophil
Copy link
Member

hey @KDederichs

about the problem with phpunit: how do you run your tests? We have a phpunit command wrapper to handle various tests configuration we want to test, you should run ./phpunit --filter real_method_always_return_same_instance and not vendor/bin/phpunit --filter real_method_always_return_same_instance

about your initial problem: I think it comes from autorefresh behavior of the proxies: $object->_real() internally autorefreshes the object. Sorry for this question, but are you sure that the exact same code works on 2.3.1? I took your test on this version, and several previous versions, and it did fail as well for the same reason 🤔

We usually have a check for this: if there is a modification in you object, it should not autorefresh, but here the modification is related to a relationship, and it appears that the unit of work does not sees it as a modification, so to allows the autorefresh.

I'm wondering if we should not disable autorefresh when calling _real() . This does not break our test suite, and it would fix your problem.

For now, I think you can do something like:

$boxSubscription = $boxSubscription->_real()
$boxSubscription->addApplicationPdf(base64_decode(SignatureFixture::BASE64));
$boxSubscription->addKvForm(base64_decode(SignatureFixture::BASE64));
save($boxSubscription)

and keep on using the unproxied object.

@KDederichs
Copy link
Contributor Author

Yeah I'm running the exact same code with 2.3.1 and version 2.2.2 with no issues for month now so I'm not sure why that's failing in your test suit :D

Your workaround works fine for 2.3.3 though so I'll keep using that in the meantime, thanks!

@nikophil
Copy link
Member

Your workaround works fine for 2.3.3 though so I'll keep using that in the meantime, thanks!

ok that's cool

I'm running the exact same code with 2.3.1 and version 2.2.2

ok, I'll have a look at some point 😅

@nikophil nikophil added the bug Something isn't working label Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants