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

Fixes problem with passthru setter #117

Merged
merged 1 commit into from Apr 13, 2018

Conversation

Projects
None yet
3 participants
@dakujem
Contributor

dakujem commented Jan 24, 2018

So in 3.1 you changed passthru getter so that you can specify the type in the callable, but you forgot that the passthru setter must do the same...

Let me demonstrate on a very common use case where one uses JSON as string in the DB and wants an array in the Entity.

Simplest Entity:

/**
 * @property int $id
 * @property array|NULL $attrs m:passThru(jsonDecodeData|jsonEncodeData)
 */
class Foo extends LeanMapper\Entity
{

	protected function jsonEncodeData($data)
	{
		return !empty($data) ? Json::encode($data) : NULL;
	}

	protected function jsonDecodeData($data)
	{
		return !empty($data) ? Json::decode($data, Json::FORCE_ARRAY) : [];
	}

}

You set array, and you expect to get arrays as well:

$entity->attrs = ['foo' => 'bar',];
var_dump($entity->attrs);

Whoa! Warning: json_decode() expects parameter 1 to be string, array given.
Why? because after the setting calls to Entity::getModifiedRowData return:

attrs => array (1)
0 => "{"foo":"bar"}" (13)

instead of expected:

attrs => "{"foo":"bar"}" (13)

It is caused by calling settype with the returned string and "array" as the type.

This also obviously leads to invalid SQL like

UPDATE `products` 
SET `attrs`={'foo':'bar'}
WHERE ...

while it should be

UPDATE `products` 
SET `attrs`="{'foo':'bar'}"
WHERE ...

(it is not escaped properly).

@castamir castamir self-assigned this Jan 25, 2018

@castamir

This comment has been minimized.

Show comment
Hide comment
@castamir

castamir Jan 25, 2018

Collaborator

thanks, give me some time to check it out please

Collaborator

castamir commented Jan 25, 2018

thanks, give me some time to check it out please

janpecha added a commit to inlm/LeanMapper that referenced this pull request Jan 25, 2018

@janpecha janpecha added this to the Stable 3.2.0 milestone Apr 12, 2018

@janpecha janpecha merged commit a7a7dee into Tharos:develop Apr 13, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

janpecha added a commit to inlm/LeanMapper that referenced this pull request Apr 13, 2018

@janpecha

This comment has been minimized.

Show comment
Hide comment
@janpecha

janpecha Apr 13, 2018

Collaborator

Thanks for PR!

Collaborator

janpecha commented Apr 13, 2018

Thanks for PR!

janpecha added a commit to inlm/LeanMapper that referenced this pull request Apr 13, 2018

janpecha added a commit to inlm/LeanMapper that referenced this pull request Apr 13, 2018

@dakujem

This comment has been minimized.

Show comment
Hide comment
@dakujem

dakujem Apr 16, 2018

Contributor

You're welcome.

Contributor

dakujem commented Apr 16, 2018

You're welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment