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

UnitOfWork calculates change set for enum value when the value stays the same #10124

Closed
akalineskou opened this issue Oct 12, 2022 · 8 comments
Closed

Comments

@akalineskou
Copy link

Bug Report

Q A
BC Break yes
Version 2.13.2

Summary

Ever since the enum bug fixes introduced in 2.13.2 https://github.com/doctrine/orm/pull/10041/files and subsequently 2.13.3 https://github.com/doctrine/orm/pull/10058/files the UnitOfWork will always calculate a change set for the entity because it compares the BackedEnum vs a string.
I've managed to narrow the issue to

if ($orgValue === $actualValue) {

Let's assume we have a backed enum Test with the value 'test'
$orgValue is a BackedEnum Test
$actualValue is a string test

This results in the entity being force updated even though no real changes were present.

This has major implications in our codeception test suite, since, for example, we call the endpoint to create an entity, get that entity in the codeception test (which uses doctrine orm) and it will have the enum value of foo, call another endpoint which will change the enum to bar (but in our codeception test the entity is still foo) so when we call flush to update another entity (unrelated to that one) it will override bar with foo in the database resulting in the test breaking since the enum changed value unexpectedly.

But this is a general thing, lets say you grab an entity X (with enum value foo) (without updating anything on it) somehow the enum changes from another call (bar) (but remember we havent updated it in this call) and we call flush to update/persist an unrelated entity Y, X would get updated in the database with the enum value foo. (when it was bar) thus resulting in bugs

Current behavior

We grab an entity that has an enum type value
A flush happens without changing anything on it, and it gets updated.

How to reproduce

To test might be a bit hard...
Lets assume the Entity has the enum value of Foo ('foo') and is flushed to the database (first time, stored).

Need to clear the entity manager and re-load it from the database (tried to replicate it when first persisted, but it wont replicate then)

One way might be to manually update the value in the database (query UPDATE table SET enum_type = 'bar')
Do a flush on the entity without changing anything, and check that the database should still be bar (if it is foo the test should fail since nothing should have been changed so no flush for that entity)

Another way could be using a post update event which would update the column every time the entity gets updated, ie: updatedAt. After the first flush where it's inserted, the updatedAt would be created, and after the second flush it should not call the post update, and not update the updatedAt and we could check this value that it didnt change.

Expected behavior

The expected behaviour is that if an enum does not change value, it should not trigger an update on the entity in the database.

@mbabker
Copy link
Contributor

mbabker commented Oct 12, 2022

x-ref #10125 as I spent this morning digging through the exact same issue 😅

@W0rma
Copy link
Contributor

W0rma commented Oct 18, 2022

Does #10074 fix your issue?

@akalineskou
Copy link
Author

Checked the latest release, 2.13.3, with that patch and it does indeed fix it

@michnovka
Copy link
Contributor

@derrabus this issue is fixed by #10074 , you can close this

@ThomasLandauer
Copy link
Contributor

@akalineskou Could you please double-check if this issue was really fixed by 2.13.4? I just opened a separate issue and I'd like to know if it's really unrelated.

@michnovka
Copy link
Contributor

@ThomasLandauer as explained in your issue #10251 your problem was caused because of using arrays of enums. This issue was indeed fixed by #10074 . @derrabus you can close this issue, @akalineskou confirmed his issue is fixed - #10124 (comment)

@mpdude
Copy link
Contributor

mpdude commented Feb 15, 2023

@akalineskou could you close this when it’s fixed?

@akalineskou
Copy link
Author

Yes sorry, it has been fixed.

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 a pull request may close this issue.

6 participants