Skip to content
This repository has been archived by the owner on Mar 31, 2020. It is now read-only.

fix validating array with multiple types #30

Merged
merged 2 commits into from
Nov 14, 2014

Conversation

sstrigler
Copy link
Contributor

When validating a schema like

{
    "title" : "AgendaModify",
    "type" : "array",
    "required" : true,
    "items" :{
        "type": [
            {
                "type": "object",
                "properties" : {
                    "phone" : {
                        "type": "string",
                        "pattern" : "^\+\d{10,15}$",
                        "required": true
                    }
                },
                "additionalProperties": false
            },
            {
                "type": "object",
                "properties" : {
                    "email" : {
                        "type": "string",
                        "pattern" : "^\S+@\S+\.\S+$",
                        "required": true
                    }
                },
                "additionalProperties": false
            }
        ]
    },
    "additionalItems": false
}

jesse would just accept any types for items in array. This fixes that situation as results from validation are actually evaluated.

@Cy6erBr4in
Copy link
Contributor

can you show an example of data that is validated incorrectly by jesse?

@sstrigler
Copy link
Contributor Author

Got failing tests now, so don't merge. About to fix that.

@Cy6erBr4in
Copy link
Contributor

I'm asking since I have doubts about the fix :)

@sstrigler
Copy link
Contributor Author

Data that would pass but shouldn't:

[{"phone": "+46732048290"}, {"email":"some@email.com"}, {"bogus": "shouldn't be here"}]

@sstrigler
Copy link
Contributor Author

Forget about the fix, it's broken :)

@Cy6erBr4in
Copy link
Contributor

My guess is that either your schema is invalid (didn't check yet), or your expectations are wrong :)

@sstrigler
Copy link
Contributor Author

I'm pretty sure there is a bug in jesse. jesse_schema_validator:validate_with_state/3 doesn't throw exceptions but the code in check_union_type expects catching exceptions. Only jesse_schema_validator:validate/3 would throw exceptions. validate_with_state returns a state (with the error set).

@Cy6erBr4in
Copy link
Contributor

what? how is it possible that validate/3 throws exceptions and validate_with_state/3 doesn't even when validate/3 calls validate_with_state/3 internally? :) validate_with_state/3 will call jesse_validator_draft3:check_value/3 which can throw an exception.

btw, have you set allowed_errors to something but 0? :) I'm asking because I think I found the problem

@sstrigler
Copy link
Contributor Author

Nah, you're right, I found out too in the meantime, validate_with_state can throw errors, but just look at what validate does: it takes the result from validate_with_state and passes it to result/1 which throws an error if any. About allowed_errors, I need to check. I've got to catch a plane now, so won't get back before Monday probably. And yeah, things are more complicated than I thought initially. ;)

@sstrigler
Copy link
Contributor Author

Oh, you're right, allowed_errors is set to 5

@Cy6erBr4in
Copy link
Contributor

OK, then the problem is actually here: https://github.com/klarna/jesse/blob/master/src/jesse_validator_draft3.erl#L284

the original state will be passed here (it has allowed_errors == 5), and validate_with_state/3 won't throw any exceptions because of that, so what we really want to do here is to call validate_with_state/3 either with a new default state (with allowed_errors == 0), or with the original state, but with changed allowed_errors to 0. Do you follow? :)

@sstrigler
Copy link
Contributor Author

Yes, it starts to make sense now :D

@Cy6erBr4in
Copy link
Contributor

I'll have a look later today.

@sstrigler
Copy link
Contributor Author

I've changed the pull request by adding a set_allowed_errors to jesse_state, but haven't tested yet with my specific case. And now I really have to get on that damn plane 👯

@Cy6erBr4in
Copy link
Contributor

have a nice flight! :)

@Cy6erBr4in
Copy link
Contributor

it does fix the issue! I'll merge it soon, just want to do some more checks.

@Cy6erBr4in Cy6erBr4in merged commit b48ba5f into klarna:master Nov 14, 2014
@sstrigler
Copy link
Contributor Author

Unfortunately this doesn't fix the issue completely. Now I get validation errors like

[{"validationErrors",
  [[{"type","wrong_type"},
    {"path",[3,"add","contacts"]},
    {"value",[{"email","peter@email.com"}]}],
...

My actual schema is a bit different then the one from above, ie. data to validate looks like

  "add": {
           "contacts": [
                         { "phone": "+46737341553"},
                         { "email": "alice@web.de" }
                       ]
         }
}

so please don't be confused about 'paths' used.

Maybe this is because 'additionalItems: false' doesn't get the correct 'state' with the items already validated?

@Cy6erBr4in
Copy link
Contributor

could you please create a new issue and include the following:

  1. full schema
  2. an example (with data as close to the original as possible)

thanks in advance!

@sstrigler
Copy link
Contributor Author

will do

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants