Skip to content
Permalink
Browse files

Fix failing tests in IsUnique rule.

A while back the default behavior was changed in 3.2.x to make IsUnique
strict about nulls. When that code was merged into `3.next` I deleted
the 'duplicate' code. Then that change was reverted in `master` leaving
these tests failing. This restores the behavior and fixes the failing
tests.
  • Loading branch information...
markstory committed May 27, 2016
1 parent a71cc74 commit 86e24efc7d60643509c0e285c16515e86362fbff
Showing with 11 additions and 9 deletions.
  1. +11 −9 src/ORM/Rule/IsUnique.php
@@ -52,17 +52,14 @@ public function __invoke(EntityInterface $entity, array $options)
if (!$entity->extract($this->_fields, true)) {
return true;
}
$allowMultipleNulls = true;
if (isset($options['allowMultipleNulls'])) {
$allowMultipleNulls = $options['allowMultipleNulls'] === true ? true : false;
}
$options += ['allowMultipleNulls' => true];
$allowMultipleNulls = $options['allowMultipleNulls'];
$alias = $options['repository']->alias();
$conditions = $this->_alias($alias, $entity->extract($this->_fields));
$conditions = $this->_alias($alias, $entity->extract($this->_fields), $allowMultipleNulls);
if ($entity->isNew() === false) {
$keys = (array)$options['repository']->primaryKey();
$keys = $this->_alias($alias, $entity->extract($keys));
$keys = $this->_alias($alias, $entity->extract($keys), $allowMultipleNulls);
if (array_filter($keys, 'strlen')) {
$conditions['NOT'] = $keys;
}
@@ -79,13 +76,18 @@ public function __invoke(EntityInterface $entity, array $options)
*
* @param string $alias The alias to add.
* @param array $conditions The conditions to alias.
* @param bool $multipleNulls Whether or not to allow multiple nulls.
* @return array
*/
protected function _alias($alias, $conditions)
protected function _alias($alias, $conditions, $multipleNulls)
{
$aliased = [];
foreach ($conditions as $key => $value) {
$aliased["$alias.$key"] = $value;
if ($multipleNulls) {
$aliased["$alias.$key"] = $value;
} else {
$aliased["$alias.$key IS"] = $value;
}
}
return $aliased;
}

5 comments on commit 86e24ef

@ionas

This comment has been minimized.

Copy link
Contributor

ionas replied May 27, 2016

Thank you.
So by default it will work like before (for BC sake) but you can set allowMultipleNulls to false?

@ionas

This comment has been minimized.

Copy link
Contributor

ionas replied May 27, 2016

I'd do it the other way around and in 4.x change the default behavior possibly:
e.g.
default in 3.x: treatNullsAsUnique => false
default in 4.x (or 5.x): treatNullsAsUnique => true

Just a different perspective.

@markstory

This comment has been minimized.

Copy link
Member Author

markstory replied May 27, 2016

Yeah that's an option. I think sticking with SQL equivalence as the default is probably going to surprise fewer people though.

@ionas

This comment has been minimized.

Copy link
Contributor

ionas replied May 27, 2016

I agree, however that's not BC, is it?

What I meant is mostly changing the names because the current option flag doesn't really "tell" me what to expect, e.g. treatNullsAsUnique or similar does what you expect... allowMultipleNulls I don't really understand by reading just it.

@markstory

This comment has been minimized.

Copy link
Member Author

markstory replied May 27, 2016

Right now the code enforces SQL UNIQUE behavior (which allows multi-column indices to have duplicates if one column contains a null). The new option allows you to enforce no duplicates including nulls.

Please sign in to comment.
You can’t perform that action at this time.