jwaliszko / ExpressiveAnnotations

Annotation-based conditional validation library.
MIT License
351 stars 124 forks source link

AssertThat Addition Nullable Value #172

Closed gdycus closed 7 years ago

gdycus commented 7 years ago

The Children property can be null. If it is null this doesn't evaluate properly. I'm not sure but I assume if Adults is 1 and Children is null then Adults + Children = null. Is there a way to handle this scenario?

[AssertThat("(Adults + Children) <= MaxPeople", ErrorMessage = "Cannot exceed {MaxPeople} people")]

gdycus commented 7 years ago

Does anyone have any suggestions? Thanks!

jwaliszko commented 7 years ago

It is one of the null-related traps.

Take a look at the following expression: 1 + NInt where NInt is of Nullable<int> type.

When evaluated by EA parser, it results in null (EA server-side parser follows C# rules in that matter).

image

image

Such an expression is then passed to browser. At client-side, there is no separate parser built by EA. Instead, eval() method is used (see implementation outline). The problem is JavaScript follows different rules when computing such an expression. Evaluation results in 1 this time.

image

As a workaround I propose to be explicit of how you treats nulls. The alternative expression below is supposed to work coherently at both sides:

((Adults == null ? 0 : Adults) + (Children == null ? 0 : Children)) <= MaxPeople
gdycus commented 7 years ago

I'm getting a parser error

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ComponentModel.DataAnnotations.ValidationException: AssertThatValidator: validation applied to Adults field failed. ---> ExpressiveAnnotations.Analysis.ParseErrorException: Parse error on line 1, column 29: ... ? 0 : Children)) <= MaxPeople ... ^--- Argument types must match.

[AssertThat("(Adults + (Children == null ? 0 : Children)) <= MaxPeople", ErrorMessage = "Cannot exceed {MaxPeople} people")] public short Adults { get; set; } [AssertThat("(Adults + (Children == null ? 0 : Children)) <= MaxPeople", ErrorMessage = "Cannot exceed {MaxPeople} people")] public short? Children { get; set; } public int MaxPeople { get; set; }

jwaliszko commented 7 years ago

Indeed, implicit types conversion has been missing for operands of conditional operator... The fixed implementation is provided with the changeset https://github.com/jwaliszko/ExpressiveAnnotations/commit/fa765b320de5aa2638d75f6198a4b3d99e220b95. Will be shipped within next version.

gdycus commented 7 years ago

How long before the next release? Thanks

On Sep 16, 2017, at 6:08 PM, Jarosław Waliszko notifications@github.com<mailto:notifications@github.com> wrote:

Indeed, implicit types conversion has been missing for operands of conditional operator. The implementation is provided with the changeset fa765b3https://github.com/jwaliszko/ExpressiveAnnotations/commit/fa765b320de5aa2638d75f6198a4b3d99e220b95. Will be shipped within next version.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/jwaliszko/ExpressiveAnnotations/issues/172#issuecomment-330000273, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIyqOzHmHnUv28Hr-12tpOc-sLe2xS8Uks5sjFT3gaJpZM4PO9Kx.

jwaliszko commented 7 years ago

Most likely it should be released somewhere this week.

jwaliszko commented 7 years ago

Released with v2.9.6.