In order parameter validation is broken

For example the following validation:

validatorHelper.param.inOrder() //
                .type(boolean.class.getName()).optional() //
                .type(long.class.getName()).optional() //
                .type(int.class.getName()).optional() //
                .validate(executableElement, valid);

allows this method:

void select(AdapterView<Adapter> a, long c, boolean b, Bundle bundle) { }

@yDelouis

@yDelouis can you look into this?

This is blocking #1076.

i think the issue is in this loop: https://github.com/excilys/androidannotations/blob/develop/AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/ValidatorParameterHelper.java#L210

it should check the first parameter for a match in parameterRequirements if the current parameter is optional, it should check the items in parameterRequirements until it found a match. the next parameter should start to check with the next item in parameterRequirements after the match. when there a no more parameters to check but there are required parameters invalidate. same when there are more parameters available to check but no more required parameters.

unfourtunately the possibility to flag a required parameter as multiple makes this even more complex...

we definitely need unit tests for the validator. :/

@dodgex

Thanks @dodgex for investigating. I agree, we should definitely create a test suite for the validation API. This could be either a unit test with mocking the javax.model classes, or an integration test which we already have for compile-time tests.

@WonderCsabo

i have played a bit with mockito and mocked the stuff to get tests ready for diffrent variations of validations with the ValidatorParameterHelper. i've created a gist with some very basic tests but it should be possible to test every current kind of validation that the helper offers.

i already talked with @WonderCsabo on hangouts about this solution, but he is not yet sure if that is good to do it that way as this code more or less replaces everything (that is used by the validation helper) with custom logic. if he gets some time he wants to create a testcase using compile-time tests.

@dodgex

@dodgex i definitely do not have time for experimenting with the compile time tests. If the other collaborators say your way is good enough, i will happy to accept a PR with your current test logic.

However, we should fix this bug ASAP, with or without tests.

@WonderCsabo

@WonderCsabo then i misunderstood you. but yeah, this issue should be fixed ASAP.

@yDelouis

@yDelouis please fix this ASAP, we can write the tests later (but we should really verify the correctness).

Finally fixed.

Navigation Menu

Repository navigation

In order parameter validation is broken #1443

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

In order parameter validation is broken #1443

Description

Activity

WonderCsabo commented on Jun 4, 2015

WonderCsabo commented on Jun 4, 2015

dodgex commented on Jun 4, 2015

WonderCsabo commented on Jun 4, 2015

dodgex commented on Jun 7, 2015

WonderCsabo commented on Jun 7, 2015

dodgex commented on Jun 7, 2015

WonderCsabo commented on Jun 24, 2015

WonderCsabo commented on Oct 29, 2015

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions