Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types - invalid value tracking #10

Closed
mabar opened this issue Jul 28, 2021 · 2 comments · Fixed by #19
Closed

Types - invalid value tracking #10

mabar opened this issue Jul 28, 2021 · 2 comments · Fixed by #19
Assignees

Comments

@mabar
Copy link
Member

mabar commented Jul 28, 2021

Types are representation of whole VO structure and are also able to track exact keys which are invalid. In order to check which keys were invalid, user has to compare invalid keys with sent data and that's impractical.

All the Type methods which mark value invalid should be required to also add that value to Type so it could be rendered together with invalid key by Formatter.

In this stage we should resolve the problem to satisfaction just in Type. Optimal rendering in ErrorFormatter could be quite complex and for now only minimal implementation in VisualErrorFormatter should be enough to verify solution - something like key: string (received int(123) instead)

@mabar mabar self-assigned this Jul 28, 2021
@mabar
Copy link
Member Author

mabar commented Jul 28, 2021

Most complicated part of issue is having an easy to understand and hard to misuse API.

e.g. StringRule uses SimpleValueType which can mark multiple parameters (minLength, notEmpty, pattern, ...) invalid at the same time. In case any of the parameters is invalid (markParameterInvalid()) then invalid value should be tracked, but it does not make sense to add invalid value with each markParameterInvalid() call.
Some higher level errors api (object wrapping invalid value and parameters) is needed in order to not be able mark parameter invalid without adding invalid value and vice-versa.

In case StringRule validation fails completely (value is not a string), rule throws an exception and invalidation is handled by rule above - one of ArrayOfRule, ListOfRule, AnyOfRule, AllOfRule or StructureRule.

AnyOfRule and AllOfRule should be handled carefully. Both of these rules use CompoundType but have one important difference:

  • AnyOfRule succeeds with first not throwing sub rule - that means if all subrules throw, all invalid subtypes will have the same value which shouldn't be duplicated.
  • AllOfRule mutates the value - output of every sub rule is input for following one. It makes possible cases like the following one - accept string AND int which accepts numeric string, effectively creating rule for accepting only numeric strings. In this case, value for each invalid subtype is potentially different. To understand conversion is happening behind the scene, tracking all mutated values would be helpful, BUT it also means potential exposure in case of rules initializing class instances (structure rule, enum rules, ...). Imho only original value should be tracked so API could be same as for AnyOfRule and castNumericString should be change to smth like numericString=require/optionally/never
     AllOf(
     	StringValue(),
     	IntValue(castNumericString=true),
     )
    

@mrceperka Hope it is understandable, let me know in case anything is unclear.

@github-actions
Copy link

github-actions bot commented Dec 4, 2021

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants