This repository was archived by the owner on Feb 26, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Feb 26, 2023. It is now read-only.
In order parameter validation is broken #1443
Description
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) { }
Activity
WonderCsabo commentedon Jun 4, 2015
@yDelouis can you look into this?
WonderCsabo commentedon Jun 4, 2015
This is blocking #1076.
dodgex commentedon Jun 4, 2015
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
parameterRequirementsif the current parameter is optional, it should check the items inparameterRequirementsuntil it found a match. the next parameter should start to check with the next item inparameterRequirementsafter 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
multiplemakes this even more complex...we definitely need unit tests for the validator. :/
WonderCsabo commentedon Jun 4, 2015
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.modelclasses, or an integration test which we already have for compile-time tests.dodgex commentedon Jun 7, 2015
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.
WonderCsabo commentedon Jun 7, 2015
@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.
dodgex commentedon Jun 7, 2015
@WonderCsabo then i misunderstood you. but yeah, this issue should be fixed ASAP.
WonderCsabo commentedon Jun 24, 2015
@yDelouis please fix this ASAP, we can write the tests later (but we should really verify the correctness).
WonderCsabo commentedon Oct 29, 2015
Finally fixed.