SimpleObjectHydrator produces wrong values with inheritance table and simple array type #5989

Closed
w3sami opened this Issue Aug 24, 2016 · 5 comments

Projects

None yet

2 participants

@w3sami
w3sami commented Aug 24, 2016

I am getting empty array instead of array with some values, since SimpleObjectHydrator converts the value with convertToPHPValue

line 129 $value = $type->convertToPHPValue($value, $this->_platform);

this doesent play well with the null check below

line 134:
// Prevent overwrite in case of inherit classes using same property name (See AbstractHydrator)
if ( ! isset($data[$fieldName]) || $value !== null) {
$data[$fieldName] = $value;
}

since arrayType->convertToPHPValue returns [] when null, resulting in a leak from the wrong left joined table.

solution is easy, dont use the converted value in the if, but instead a raw value.

eg line 125 + $rawValue = $value;
and
136: if ( ! isset($data[$fieldName]) || $rawValue !== null) {

sorry I don't have time to write a pr with tests

@Ocramius
Member

since arrayType->convertToPHPValue returns [] when null, resulting in a leak from the wrong left joined table.

Seems like an issue with the array type, rather than with the ORM itself?

@w3sami
w3sami commented Aug 24, 2016

SimpleObjectHydrator is the file with the problem, type is ok imo. Same
issue is with eg json array type. Hydrator compares with type converted
version not raw value.
For me it makes sense that null is empty array for array type.

On Aug 24, 2016 8:23 PM, "Marco Pivetta" notifications@github.com wrote:

since arrayType->convertToPHPValue returns [] when null, resulting in a
leak from the wrong left joined table.

Seems like an issue with the array type, rather than with the ORM itself?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#5989 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADiuoZhEIsq1rNG-ucjKzXaAhE9JTgyQks5qjH31gaJpZM4Jr3Ns
.

@w3sami
w3sami commented Aug 29, 2016

bumb! this is actually a quite serious bug, as it renders all our entities with joined inheritance + (json/)array type useless!!

@cvuorinen cvuorinen added a commit to cvuorinen/doctrine2 that referenced this issue Sep 2, 2016
@cvuorinen cvuorinen Create a failing test for issue #5989
Field with type=array in a joined inheritance gets overridden by empty array in the hydrator
c61be0f
@cvuorinen cvuorinen added a commit to cvuorinen/doctrine2 that referenced this issue Sep 2, 2016
@cvuorinen cvuorinen Create a failing test for issue #5989
Field with type=array in a joined inheritance gets overridden by empty array in the hydrator
611b735
@cvuorinen cvuorinen added a commit to cvuorinen/doctrine2 that referenced this issue Sep 3, 2016
@cvuorinen cvuorinen Create a failing test for issue #5989
Field with type=simple_array in a joined inheritance gets overridden by empty array in the hydrator
36f4a68
@cvuorinen cvuorinen added a commit to cvuorinen/doctrine2 that referenced this issue Sep 3, 2016
@cvuorinen cvuorinen Fix hydration in a joined inheritance with simple array or json array
SimpleArrayType and JsonArrayType convert NULL value to an empty array, which fails the null check that is used to prevent overwrite
Fixes issue #5989
8fb6895
@Ocramius
Member
Ocramius commented Sep 7, 2016

The fix in #6004 seems good, besides minor cleanup needed :-)

@Ocramius Ocramius added the Bug label Sep 7, 2016
@Ocramius Ocramius added this to the 2.5.5 milestone Sep 7, 2016
@Ocramius Ocramius self-assigned this Sep 7, 2016
@Ocramius Ocramius added a commit that referenced this issue Sep 8, 2016
@cvuorinen @Ocramius cvuorinen + Ocramius Create a failing test for issue #5989
Field with type=simple_array in a joined inheritance gets overridden by empty array in the hydrator
dce0aea
@Ocramius Ocramius added a commit that referenced this issue Sep 8, 2016
@cvuorinen @Ocramius cvuorinen + Ocramius Fix hydration in a joined inheritance with simple array or json array
SimpleArrayType and JsonArrayType convert NULL value to an empty array, which fails the null check that is used to prevent overwrite
Fixes issue #5989
7352b97
@Ocramius Ocramius added a commit that referenced this issue Sep 8, 2016
@Ocramius Ocramius Merge branch 'fix/#6004-#5989-fix-hydration-in-a-joined-inheritance-w…
…ith-simple-array-or-json-array-2.5' into 2.5

Close #6004
Close #5989
d592c14
@Ocramius Ocramius added a commit that referenced this issue Sep 8, 2016
@cvuorinen @Ocramius cvuorinen + Ocramius Create a failing test for issue #5989
Field with type=simple_array in a joined inheritance gets overridden by empty array in the hydrator
12b5e79
@Ocramius Ocramius added a commit that referenced this issue Sep 8, 2016
@cvuorinen @Ocramius cvuorinen + Ocramius Fix hydration in a joined inheritance with simple array or json array
SimpleArrayType and JsonArrayType convert NULL value to an empty array, which fails the null check that is used to prevent overwrite
Fixes issue #5989
95546d6
@Ocramius Ocramius added a commit that referenced this issue Sep 8, 2016
@Ocramius Ocramius Merge branch 'fix/#6004-#5989-fix-hydration-in-a-joined-inheritance-w…
…ith-simple-array-or-json-array'

Close #6004
Close #5989
5eebdcf
@Ocramius Ocramius added a commit that closed this issue Sep 8, 2016
@Ocramius Ocramius Merge branch 'fix/#6004-#5989-fix-hydration-in-a-joined-inheritance-w…
…ith-simple-array-or-json-array'

Close #6004
Close #5989
5eebdcf
@Ocramius Ocramius closed this in 5eebdcf Sep 8, 2016
@Ocramius
Member
Ocramius commented Sep 8, 2016

Fixed in #6004, thanks @w3sami and @cvuorinen!

@yvoyer yvoyer added a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016
@cvuorinen @yvoyer cvuorinen + yvoyer Create a failing test for issue #5989
Field with type=simple_array in a joined inheritance gets overridden by empty array in the hydrator
e77e994
@yvoyer yvoyer added a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016
@cvuorinen @yvoyer cvuorinen + yvoyer Fix hydration in a joined inheritance with simple array or json array
SimpleArrayType and JsonArrayType convert NULL value to an empty array, which fails the null check that is used to prevent overwrite
Fixes issue #5989
a4a7a36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment