jwaliszko / ExpressiveAnnotations

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

No more than 27 unique attributes of the same type can be applied for a single field or property. #96

Closed senzacionale closed 8 years ago

senzacionale commented 8 years ago
<td class="center">
            @Html.ValidationMessageFor(modelItem => Model.StudentsWorksReportsFormModel.StudentWorkReportViewModelList[i].StudentWorkReportFormModel.StartDate)
            <div class="input-append date" id="@Html.RenderNumbers("StartDatePickerComponent", i)">
                @Html.TextBoxFor(modelItem => Model.StudentsWorksReportsFormModel.StudentWorkReportViewModelList[i].StudentWorkReportFormModel.StartDate,
                    new
                    {
                        @Value = Model.StudentsWorksReportsFormModel.StudentWorkReportViewModelList[i].StudentWorkReportFormModel.StartDate != DateTime.MinValue ? Model.StudentsWorksReportsFormModel.StudentWorkReportViewModelList[i].StudentWorkReportFormModel.StartDate.ToString("d") : "",
                        id = @Html.RenderNumbers("StartDate", i),
                        @readonly = "readonly",
                        @class = "input-small focused"
                    })
                <span class="add-on"><i class="icon-th"></i></span>
            </div>
        </td>

it is looped 100 times, so 100 customers:

and there is a AssertThat in model:

[AssertThat("StartDate <= EndDate", 1, ErrorMessage = Translations.Global.DATE_START_NOT_AFTER_DATE_END)]

which throws and exception: No more than 27 unique attributes of the same type can be applied for a single field or property.

why is this not allowed?

Can you make this method public?

private void AssertAttribsQuantityAllowed(int count)
        {
            const int max = 27;
            if (count > max)
                throw new InvalidOperationException(
                    $"No more than {max} unique attributes of the same type can be applied for a single field or property.");
        }
jwaliszko commented 8 years ago

When a user annotates a single property more than 27 times (using unique attributes of the same type):

[AssertThat("expression 0")]
[AssertThat("expression 1")]
...
[AssertThat("expression N")]
public string Field { get; set; }

the exception is thrown - to protect him from some client-side issue (explained later).

The particular problem you've reported here (unveiled during rendering a view based on large enough list of model items) is a side effect of this protective assertion, I haven't been aware of, up to now.

Mentioned quantity assertion has been introduced after I've faced a problem, of how to register correct number of adapters for client-side component of EA. Correct means as many as many there are unique attributes.

To be more precise, when attribute is applied to some model field, corresponding validation adapter is created and related with such a field at client-side, i.e. $.validator.unobtrusive.adapters.add(name, ... It would be ideal to have exactly as many adapters as there are unique annotations. Unfortunately I haven't found a way to do it - how to communicate that to client-side, and then register them before the DOM is ready?

Due to that fact, I've been searching for an alternative, yet simple workaround. As a result, each field has been restricted to have fixed number of adapters (with following unique names, e.g. assertthat, assertthata, asserthatb, ..., assertthatz). 27 is a number which seemed to be not too high, to have redundant handlers (unfortunately most of the time 27 is more than enough, so memory is not managed effectively), and not too low (not to reduce usability by introducing some artificial boundaries from the user perspective).

For more details please see expressive.annotations.validate.js sources and logic related with annotations = ' abcdefghijklmnopqrstuvwxyz'; statement. More in-depth explanation of the functionality related to multiple attributes handling in EA, can be also found here: http://stackoverflow.com/q/25414296/270315.

Any ideas solving this problem correctly are very welcome.

rwil02 commented 8 years ago

Html.TextBoxFor and the like generate an ID / name based on a prefix built from a 'stack' of container names in order to be unique by 'instance' right?

Is there any way for you to access that unique prefix in the right place to pass it to the adapters.add?

senzacionale commented 8 years ago
annotations = ' abcdefghijklmnopqrstuvwxyz'; // suffixes for attributes annotating single field multiple times

Why not to use combinations of them?

Like: assertthatab, assertthatudm...

jwaliszko commented 8 years ago

@senzacionale: it would be a huge waste of JavaScript memory. Having e.g. 100 adapters created always for each field, while only one or to are used most of the time, is not an option. And then, what if instead of 100, 200 adapters are required for some specific case? The only solution is to provide exact numbers of them, i.e. as many as there are corresponding annotations.

senzacionale commented 8 years ago

@jwaliszko I know that my Model "SatrtDate" list count is 100, because I have a loop and I know exact number.

can I do something with this?

jwaliszko commented 8 years ago

@rwil02: I don't understand how this could be helpful in the context of appropriate adapters quantity registration.

jwaliszko commented 8 years ago

@senzacionale: Unfortunately currently I don't know any other than fork and custom code change.

senzacionale commented 8 years ago

Can you give me some hints how to check the numbers and loop?

jwaliszko commented 8 years ago

@senzacionale: I refer you to this source: http://stackoverflow.com/q/25414296/270315, where I've described my thinking of how current implementation has been initially done. There has been some refactoring since that time, but the basic conception hasn't changed. It will give you more complete outlook of the design.

In short, you should modify AllocateSuffix() method to allocate as many unique suffixes as you need - each suffix guarantees unique validation type name (adapter name). The same suffixes need to be provided at client-side - again all logic related to this line: annotations = ' abcdefghijklmnopqrstuvwxyz'; should be adjusted (including this line itself). Take under consideration that the validation types names are enforced by MVC framework to consist only of lowercase letters, for some reason.

Keep in mind that it is extremely rough workaround which has nothing in common with a desired solution.

jwaliszko commented 8 years ago

If anyone would be interested of how to simply reproduce this particular issue using MvcWebSample project, go to Contact.cs model class and replace this line:

Addresses = new List<Address> { new Address { Type = Resources.HomeAddress }, new Address { Type = Resources.OfficeAddress } };

with this one:

Addresses = Enumerable.Range(0, 28).Select(x => new Address { Type = Resources.HomeAddress } ).ToList();
rwil02 commented 8 years ago

Well, at the moment, you basically have (e.g.) 2 fields, a,& b repeated multiple times Let's say A has 'Assert that a > b' and b has 'Assert that b != null' If you have 100 items, you get 100 of each But those 100 items are rendered as X1_A, X1_B X2_A, X2_B etc

so really you have something like List['X1'] = {a, b, AssertThatA, AssertThatB} where a = X1_A and b = X1_b List['X2'] = {a, b, AssertThatA, AssertThatB} where a = X2_A and b = X2_b

and not a, b, AssertThatA, AssertThatB where a = X1_A and b = X1_b a, b, AssertThatC, AssertThatD where a = X2_A and b = X2_b

And that X1, X2 etc are related to calls to something like PrefixScope or StartItem?

jwaliszko commented 8 years ago

@senzacionale, @rwil02: I still do not know how to register the exact numbers of adapters, but at least the side effect related to this particular issue is solved here: https://github.com/jwaliszko/ExpressiveAnnotations/commit/ad078cccc63ca1d83464d55fd913a9dd44999907 (to be shipped within next release - v2.6.6).

senzacionale commented 8 years ago

I just don't understand how for example jquery required method works for all properties?

jwaliszko commented 8 years ago

v2.6.6 released

senzacionale commented 8 years ago

Thank you for fast fix.

jwaliszko commented 8 years ago

@senzacionale: you're welcome, thanks for reporting this problem.

izakbar commented 12 months ago

If I have array of data fields and each is same type and name - just array index is different, then EA treats them all as same for purpose of the count > 27 check. It can very quickly get to >27 assertions. If I have 8 instances of the data item on the form and there are 4 x asserts on the field.. I get count of 32. boom EA says no because it sees each array index as same field. I can understand not wanting 100s of adaptors on each field, but these are seperate instances of the field - but ea sees them as the same (I think). Either having the array index as part of the AttributeWeakId would fix or allowing >27 at users risk.

Simple workaround to allow more than 27 that I have currently implemented locally is to use count.ToString("X2").ToLowerVariant(); instead of char.ConvertFromUtf32(95+ count); - and changed const int max = 27 to const int max = 64 -- this is rubbish - doesnt work in js side :(

Okay, what I have found is that if I pass the context.Attributes["id"] as extra parameter to RequireIfValidator and AssertValidator and then in ExpressiveValidator instead of using metadata.PropertyName - use the passed id (if not null and length>0) otherwise use metadata.PropertyName. This seems to work nicely as now each instance of the data only has the 4 asserts :) and they are assert asserta b c - and more importantly they fire ! - and ofc this I did in the .net core fork version.. which doesn't translate super well to this version. Oh joy.