8000 add 'guid' to list of exception to filter out by garak · Pull Request #17668 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

add 'guid' to list of exception to filter out #17668

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

add 'guid' to list of exception to filter out #17668

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b8edcb
add 'guid' to list of exception to filter out
garak Feb 3, 2016
91f9444
fix guid case
garak Feb 3, 2016
141eb23
Fix parameter type
garak Mar 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix guid case
  • Loading branch information
@garak
garak committed Feb 3, 2016
commit 91f9444e7683ab30f40e03f0831a4293ab878eeb
9 changes: 8 additions & 1 deletion src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,21 @@ public function getEntitiesByIds($identifier, array $values)
// Guess type
$entity = current($qb->getRootEntities());
$metadata = $qb->getEntityManager()->getClassMetadata($entity);
if (in_array($metadata->getTypeOfField($identifier), array('integer', 'bigint', 'smallint', 'guid'))) {
if (in_array($metadata->getTypeOfField($identifier), array('integer', 'bigint', 'smallint'))) {
$parameterType = Connection::PARAM_INT_ARRAY;

// Filter out non-integer values (e.g. ""). If we don't, some
// databases such as PostgreSQL fail.
$values = array_values(array_filter($values, function ($v) {
return (string) $v === (string) (int) $v;
}));
} elseif ('guid' === $metadata->getTypeOfField($identifier)) {
$parameterType = Connection::PARAM_INT_ARRAY;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is wrong. You should not change this to INT_ARRAY. Uuids are not bindable as integers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to suggestions. What should I do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the binding type should be a string array, not an int array


// Like above, but we just filter out empty strings.
$values = array_values(array_filter($values, function ($v) {
return (string) $v !== '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this actually need to validate the pattern of uuids, otherwise postgresql will still break for invalid uuids.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But validation can already be done via constraint
The purpose here is allowing empty strings for non-required uuid fields

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it cannot be done via constraint, as transforming the value is done before validating the model data (as it is necessary to build the model data).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, as I already told in PR comment, the purpose here is not validating the uuid. The purpose is just allowing empty values, otherwise an optional uuid is not working.

}));
} else {
$parameterType = Connection::PARAM_STR_ARRAY;
}
Expand Down
0