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

SymfonyValidator errors with typed properties #205

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

martenb
Copy link
Contributor

@martenb martenb commented Dec 11, 2023

Ok, i switched to typed properties and found some bugs in SymfonyValidator,, i am not sure, how to fix it, maybe you will know?

	#[Assert\NotNull]
	#[Assert\Type(type: 'integer')]
	public $typedId;

when you send ['typedId' => 'sss'] the validator throws typeId is not integer, but

	#[Assert\NotNull]
	#[Assert\Type(type: 'integer')]
	public int $typedId;

throws typedId cannot be null...

This is real problem in nullable properties

	#[Assert\Type(type: 'integer')]
	public ?int $typedId = null;

when you send ['typedId' => 'sss'], it will pass without any errors...

@martenb martenb changed the title test: add failing tests :-( SymfonyValidator errors with typed properties Dec 11, 2023
@f3l1x
Copy link
Member

f3l1x commented Dec 15, 2023

Will take a 👀

@f3l1x
Copy link
Member

f3l1x commented Feb 23, 2024

At this time, implementation looks like this:

public function factory(array $data): self
{
$inst = new static();
// Fill properties with real data
$properties = $inst->getRequestProperties();
foreach ($properties as $property) {
if (!array_key_exists($property['name'], $data)) {
continue;
}
$value = $data[$property['name']];
// Normalize & convert value (only not null values)
if ($value !== null) {
$value = $this->normalize($property['name'], $value);
}
// Fill single property
try {
$inst->{$property['name']} = $value;
} catch (TypeError) {
// do nothing, entity will be invalid if something is missing and ValidationException will be thrown
}
}
return $inst;
}

It means, if you have typed property int $typedId and try to assign string to it, PHP will throws error, but there is a try/catch.

try {
$inst->{$property['name']} = $value;
} catch (TypeError) {
// do nothing, entity will be invalid if something is missing and ValidationException will be thrown
}

It's not the best implementation I know. Better implementation would be read the validations (NotNull, TypedInt, etc.) and validate input as array, but it's huge BC at this time.

@f3l1x
Copy link
Member

f3l1x commented Feb 23, 2024

I've added some more tests for the future 33ec0d4

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.

2 participants