[FOLIO-481] Design APIs for field-level validation errors Created: 22/Feb/17 Updated: 12/Nov/18 Resolved: 16/Mar/17 |
|
| Status: | Closed |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | Umbrella | Priority: | P3 |
| Reporter: | Jakub Skoczen | Assignee: | shale99 |
| Resolution: | Done | Votes: | 0 |
| Labels: | sprint10, sprint9 | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | 1 day, 5 hours | ||
| Original estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||
| Issue links: |
|
||||||||||||||||||||||||
| Sprint: | |||||||||||||||||||||||||
| Description |
|
In anticipation of
I will follow up with a proposal for structuring the API/error responses, first I am trying to confirm if redux-form imposes any constraints on the API. |
| Comments |
| Comment by shale99 [ 26/Feb/17 ] |
|
there is some support for validation in the RMB. 1. object validation (body content) / query param / header validation - the raml for an endpoint allows indicating if specific parameters / fields are required, default values, (if numeric) min / max values, regex patterns, etc... of the expected input. The RMB uses aspects to detect input that does not conform to the raml spec of an endpoint and returns the field, value and a message (for example lang param was eng but should be [a-zA-Z] {2}. 2. drools support - please see documentation - https://github.com/folio-org/raml-module-builder#drools-integration currently v8 of the RMB returns the validation information as a string - there is a current local implementation that returns validation as an errors json reply - see attached json schema files |
| Comment by shale99 [ 26/Feb/17 ] |
|
some examples (traits (headers, query params) and body fields (json schemas) have the same validation options): a field in a json schema containing 6 letters in english would look like this: "module":{ ",* a query param trait raml limit: |
| Comment by Jakub Skoczen [ 28/Feb/17 ] |
|
shale99 Looking at the Error schema, I think it would be good to have an explicit error type like e.g "validation" so that the UI can easily pick up those errors. Also, each validation error may refer to a nested field, how would we represent that in the error? Using a "path" for the field? e.g "user.address.street"? |
| Comment by shale99 [ 28/Feb/17 ] |
|
1. ok, what about adding a type to the error entry with a code indicating what type of error it is. |
| Comment by shale99 [ 02/Mar/17 ] |
|
i have this set up locally, a current rundown of the implementation (mostly existing, only the response has changed) - comments welcomed before commit. 1. RMB has multiple validation mechanisms - the two main ones are RAML and Drools validations (see examples above) comments? |
| Comment by Marc Johnson [ 02/Mar/17 ] |
|
Can you provide an example of what an error for a specific field could look like? What motivated choosing the 422 as the status code, rather than 400 or 409? Hugs, Marc |
| Comment by shale99 [ 05/Mar/17 ] |
|
summary of implementation: 1. Schemas attached to the issue for example, if a body is passed in the POST tenant request, but the body is missing the module_to field, the following will be returned automatically by the RMB - note this requires no manual coding on anyones part and is automatically handled by the RMB's understanding of the raml spec of the endpoints. Status Code: 422 Unprocessable Entity
Transfer-Encoding: chunked
{
"errors": [
{
"message": "may not be null",
"type": "1",
"code": "-1",
"parameters": [
{
"key": "moduleTo",
"value": "null"
}
]
}
]
}
5. for drools violations - the content returned in the error json is up to the person writing the rule, for example:
tenantDrools.drl
package rules
import org.folio.rest.jaxrs.model.TenantAttributes;
import org.folio.rest.jaxrs.model.Error;
import org.folio.rest.jaxrs.model.Parameter;
dialect "java"
rule "Validate tenant request for json in body with module_to field"
when
e: Error()
ta : TenantAttributes( ta.getModuleTo() == null)
then
Parameter p = new Parameter();
p.setKey("moduleTo");
p.setValue(""+ta.getModuleTo());
e.getParameters().add(p);
e.setMessage("module_to may not be null");
e.setCode("-1");
e.setType("1");
throw new java.lang.Exception("module_to may not be null");
end
|
| Comment by Marc Johnson [ 05/Mar/17 ] |
|
Thank you for the summary. Will the parameters collection contain anything else other than an record with the key as the name of the field and the (invalid) value as the value? Will nested fields be represented with dots, e.g. if I have a status property with a nested property of name, would the parameter key be status.name ? Hugs, Marc |
| Comment by shale99 [ 06/Mar/17 ] |
|
right now that is the idea - key = field , value = invalid value, with a nested field containing "." as separators. if there are other preferences by the UI that can be accommodated |
| Comment by Jason Skomorowski [ 10/Mar/17 ] |
|
I imagine that most/all validation will be server-side initially, but are we considering that in future we might pull down these validation rules and run them client-side to cut down on traffic? As for API preferences, I'll ping John Coburn as he's been doing the most with validation. |
| Comment by Jason Skomorowski [ 10/Mar/17 ] |
|
Hm, is there a way to extend this to offer endpoints which can somehow provide single-field validation for cases it where we want to offer it while the user is typing? (or immediately after they leave that field) |
| Comment by shale99 [ 12/Mar/17 ] |
|
you mean basically have a validate api where a single field / value are passed in to validate? |
| Comment by shale99 [ 12/Mar/17 ] |
|
so the easiest way for me to implement this if we decide on this is to have the same request endpoint serve as both the actual endpoint and as a validation endpoint. meaning, lets say we have a POST request to /users with a json user in the body. to turn this into a validation only check - add a flag on the url, lets say /users?validate_field=username you can pass multiple validate_fields on the url. and the response will either by a 2xx if the username field passes validation, or a 422 with the error schema indicated above |
| Comment by John Coburn [ 13/Mar/17 ] |
|
shale99 I like it. Since this can happen mid-form for the validate calls, the json user may only be partially filled - will that create an issue? Of course, the field to be validated will have to be there - but that may be all that's filled out at that point. Also does this behave with nested fields/dot syntax as well? /users?validate_field=personal.email will that work? |
| Comment by shale99 [ 14/Mar/17 ] |
|
that's exactly how it works. You can pass in a partial json object, that will be 100% ok. the way it works is that the partial json will be mapped into the appropriate object by the rmb (in the example's case, into a user's object) and then only the requested field will be checked against the raml spec. passing nested fields is exactly as in your example above (with a '.' delimiter). |
| Comment by shale99 [ 14/Mar/17 ] |
|
committed, currently per field validation is working on a raml spec basis. Looking to see how possible it is to activate this for drools based validations |
| Comment by John Coburn [ 14/Mar/17 ] |
|
Thanks shale99! To answer the concerns from redux-form in the description, I don't see any limitation/ constraint from redux-form for async validation at this point. There's some minor code to reshape the response to a flat object {fieldname: errorMessage}that redux-form wants to update the UI with, but nothing too crazy. How the front end works in short: |
| Comment by shale99 [ 14/Mar/17 ] |
|
thanks, understood. |
| Comment by shale99 [ 15/Mar/17 ] |
|
not sure if this should be closed - i have completely my part in this i believe |
| Comment by Marc Johnson [ 15/Mar/17 ] |
|
For a POST for a specific property (with a partial resource representation in the body), for example, to: /users?validate_field=personal.email Does it provide a 422 like when a POST / PUT request to create or update a resource fails for validation reasons? Does it only validate the specific property (and other data is only relevant for conditional validation) or can it return validation errors for other properties too? Does the collection resource RAML include this new behaviour (both this and the general 422 for JSON validation errors)? Hugs, Marc |
| Comment by shale99 [ 15/Mar/17 ] |
|
if you pass in the validate_field query param it will only validate that specific field/s in the body - others will be ignored from a validation standpoint - 422 is the validation status code whether the validate_field is passed in or not. |
| Comment by Marc Johnson [ 15/Mar/17 ] |
|
Thank you. So for a property level validation request, a 200 means the property is currently valid (including in the context of other properties provided) and a 422 for invalid with errors. Thank you for defining the trait. Adding the trait to the default set of traits that a collection resource type provides means that any modules using that resource type would need to provide this capability, so we may need to defer that. Hugs, Marc |
| Comment by shale99 [ 15/Mar/17 ] |
|
i actually think we should add those traits comments? |
| Comment by Marc Johnson [ 16/Mar/17 ] |
|
I agree that a uniform interface is useful to the UI, for that to work, all modules have to both declare and support that interface. This feels like a project level decision to me, as we would need to plan in the work to upgrade all of the existing storage modules and implement this functionality in the business logic modules. To me, it would be best to do that once we have an example of this mechanism used in the UI that we are happy with, as reacting to feedback on it will be more effort once it is standardised. Hugs, Marc |
| Comment by Mike Taylor [ 16/Mar/17 ] |
|
(Note to self: review this issue.) |
| Comment by Jakub Skoczen [ 21/Mar/17 ] |
|
Users module is already on v10, inventory will follow once mattype is completed. Loan storage needs update so do "auth" modules. |
| Comment by Marc Johnson [ 21/Mar/17 ] |
|
I believe shale99 was referring to the inventory storage module (being upgraded to RAML Module Builder V10) so as to support JSON based validation errors. I believe both the inventory and circulation (and most likely users, maybe Kurt Nordstrom can elaborate) business logic modules will need development in order to behave in this manner. Hugs, Marc |
| Comment by shale99 [ 22/Mar/17 ] |
|
i do believe we should open up to an audio discussion and discuss moving biz modules to RMB in order to avoid re-coding this and other functionality. the RMB is object oriented, meaning it converts the jsons in the body to objects adding jsr311 annotations and then validates based on those annotations - so modules that want to incorporate this will need to work with objects and not parse jsons directly - otherwise we need to re-code this and support two implementations. the aspects-rmb sub module exposes validation of traits and this may be able to be re-used. however, i see this as a bigger project type issues of whether we want to create a specific way for us to develop modules. as i mentioned on slack the rmb enforces a methodology as follows: 1. declare the schemas (your objects) developers do not worry about i am aware of the verbose-ness of the rmb (can be discussed how to improve) - but it is done due to validation purposes as the RMB strongly types everything , including response codes so that there can be nothing that does not conform to the raml (i believe this will save us headaches of raml docs and code getting out of sync in even the slightest way) |
| Comment by Jason Skomorowski [ 18/Apr/17 ] |
|
Do we have a plan for client-side validation? It seems like we might be able to reuse the same rules via projects that implement JSON schema / RAML in javascript: Of course, not all validations are valid client side eg. checking username uniqueness, foreign key existence, etc. |
| Comment by David Crossley [ 19/Apr/17 ] |
|
That sounds wonderful Jason. However the way that our RAML files refer to a second JSON schema from a parent JSON schema, does not for work for tools (such as that one) that depend on "raml-js-parser-2". See the workaround that is needed in
I am currently preparing a new issue report to better explain this. (Edit: 20170911: That is better handled now. See
|
| Comment by Jason Skomorowski [ 08/Sep/17 ] |
|
I started looking into supporting this in stripes-connect (
Is there a plan to expose complete validation to the validate_field parameter? Is there a way to validate a whole record rather than specific fields without actually creating the record when it succeeds? Does that method invoke the implementation-specific validation? |
| Comment by shale99 [ 09/Sep/17 ] |
|
i'll explain the reasoning although i agree the behavior is problematic |
| Comment by shale99 [ 09/Sep/17 ] |
|
i'll explain the reasoning although i agree the behavior is problematic |
| Comment by shale99 [ 09/Sep/17 ] |
|
a simple fix would be to have rmb do the initial validation - if it fails to return the 422, if it succeeds, to pass the request on to the module - a module can indicate that it accepts the validate_field trait and then it will get the pojo (json) alonf with the field to validate - the module will then need to validate (from a biz logic standpoint) the field. |