[Doctrine Bridge] Added a parameter ignoreNull on Unique entity to allow a nullable value on field. Added Test
merged branch Garfield-fr/master (PR #5015)

Commits
-------

f1c4b8b [Doctrine Bridge] Added a parameter ignoreNull on Unique entity to allow a nullable value on field. Added Test

Discussion
----------

[Doctrine Bridge] Added parameter ignoreNull to accept a nullable value on field

In my last project, i use this syntax to test unicity on 2 fields, but it fail because the validator stop if value is null. I dropped the test on validator and my unicity work fine.

```
@UniqueEntity(fields={"username", "deletedAt"})
```

It's possible to add this PR on Bridge.

Thanks

Bertrand

---------------------------------------------------------------------------

by stof at 2012-07-23T08:14:19Z

This is wrong. RDBMS allow several null values in a unique column and this change will break it.

---------------------------------------------------------------------------

by henrikbjorn at 2012-07-23T08:17:08Z

@stof seems weird indeed it would return if any of the values are null. Makes sense to do a query where the field `IS NULL` or whatever the find method does.

---------------------------------------------------------------------------

by stof at 2012-07-23T08:18:50Z

@henrikbjorn if you do a query with IS NULL, the validator would force to have only 1 entity with a null field whereas it is not the behavior of the DB-level constraint.

---------------------------------------------------------------------------

by henrikbjorn at 2012-07-23T08:20:41Z

In this case i suspect that he wants to achieve a `WHERE username = "henrikbjorn" AND deletedAt IS NULL` which would be valid right? Currently it just returns if any of the fields are null and the validation is never done.

---------------------------------------------------------------------------

by bschussek at 2012-07-23T08:27:24Z

I suggest to make this configurable as the handling of NULL values in UNIQUE columns [differs between SQL implementations](http://forums.mysql.com/read.php?22,53591,53591#msg-53591).

---------------------------------------------------------------------------

by Garfield-fr at 2012-07-23T08:52:53Z

@stof What the correct solution to test my unicity with deletedAt == null ?

I use this definition: @ORM\Column(name="deleted_at", type="datetime", nullable=true)

---------------------------------------------------------------------------

by Garfield-fr at 2012-07-23T20:28:44Z

In my local repository, i added a new parameter "$authorizedNullField" on UniqueEntity.php and tested this on UniqueEntityValidator.php:

Code: https://gist.github.com/4122efbe569e3c2c95c0

What about that ?

Thanks for your help

---------------------------------------------------------------------------

by stof at 2012-07-23T20:45:30Z

yep, this would be good (except for the naming which seems weird)

---------------------------------------------------------------------------

by Garfield-fr at 2012-07-23T20:46:44Z

No problem to change this. I don't find a good name for this ?

---------------------------------------------------------------------------

by stof at 2012-07-23T20:47:57Z

what about ``allowMultipleNull`` (defaulting to ``true`` for BC) ?

---------------------------------------------------------------------------

by Garfield-fr at 2012-07-23T20:51:30Z

Why multiple ? This option is for one or many. what about `allowNullable` ?

---------------------------------------------------------------------------

by stof at 2012-07-23T20:52:44Z

@Garfield-fr the current behavior allows having multiple null values without failing to the unique constraint

---------------------------------------------------------------------------

by Garfield-fr at 2012-07-23T20:56:07Z

ok. I make `allowMultipleNull`.

It's ok with that: https://gist.github.com/cae8d43780c45a5011ed

Thanks

---------------------------------------------------------------------------

by bschussek at 2012-07-23T20:58:12Z

What about `uniqueNull` (`false` by default)? `ignoreNull` (`true` by default)? I find `allowMultipleNull` a bit cumbersome.

---------------------------------------------------------------------------

by stof at 2012-07-23T20:58:26Z

no it is not. You have an issue in the validator. You have an extra negation.

Please update your PR

---------------------------------------------------------------------------

by Garfield-fr at 2012-07-23T21:01:59Z

@stof `ignoreNull` is ok for you ?

---------------------------------------------------------------------------

by stof at 2012-07-23T21:10:24Z

yes

---------------------------------------------------------------------------

by fabpot at 2012-08-05T07:48:03Z

Is it mergeable now? Is yes, @Garfield-fr Can you squash your commits?

---------------------------------------------------------------------------

by travisbot at 2012-08-05T08:43:23Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2039523) (merged 19ae3cf9 into c20c1d18).

---------------------------------------------------------------------------

by stof at 2012-08-05T12:09:02Z

@Garfield-fr when squashing the commits, you need to force the push as you are rewriting the history. You should not have merged with your remote branch

---------------------------------------------------------------------------

by Garfield-fr at 2012-08-05T12:10:15Z

What's the right solution for resolve this ?

---------------------------------------------------------------------------

by stof at 2012-08-05T12:11:09Z

@Garfield-fr reset your local branch to the squashed commit and force the push

---------------------------------------------------------------------------

by Garfield-fr at 2012-08-05T12:14:09Z

@stof Thanks for your help

---------------------------------------------------------------------------

by travisbot at 2012-08-05T12:19:06Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2040210) (merged f1c4b8b4 into 20d2e5a1).