thephpleague / json-guard

Validation of json-schema.org compliant schemas.
http://json-guard.thephpleague.com/
MIT License
175 stars 26 forks source link

ValidationError uses different name styles for keyword and context #97

Closed northern closed 7 years ago

northern commented 7 years ago

Hi,

My application is multilingual and therefore the messages in the ValidationError aren't very useful to my use-case.

All error information is contained within the ValidationError object for me to make my own messages, however, I noticed a slight issue in that the keyword attribute is camel case style whereas the corresponding keywords in the context attribute are snake case style:

League\JsonGuard\ValidationError Object
(
    [message:League\JsonGuard\ValidationError:private] => String is not at least {min_length} characters long
    [keyword:League\JsonGuard\ValidationError:private] => minLength
    [pointer:League\JsonGuard\ValidationError:private] => /title
    [value:League\JsonGuard\ValidationError:private] => ab
    [context:League\JsonGuard\ValidationError:private] => Array
        (
            [min_length] => 3
       )
)

I can work around this by converting one into the other but in light of performance it might be good to pick one style and stick with it? That way they keyword attribute could be used directly as an index into the context attribute.

Thanks.

matt-allan commented 7 years ago

Hello,

That way they keyword attribute could be used directly as an index into the context attribute.

The keyword wasn't really meant to be used to index into the context. There isn't a guarantee that the context will have a key corresponding to the keyword, it just happens to match for that specific constraint. For instance, required doesn't have a required key in the context.

For most of the errors we probably could standardize on returning the parameter from the schema (i.e. minLength) and the value returned by the user. The only exceptions I can find are AdditionalProperties where we return the diff and Required where we return the missing properties.

It seems like that would make internationalization a lot simpler? You would need to do something like String {value} is not at least {parameter} characters long for every error instead of having to know what {parameter} is. For the few cases where we need a custom context variable we can specify a constant for it.

northern commented 7 years ago

Thanks for that. I much appreciate the feedback.

The keyword wasn't really meant to be used to index into the context.

I'm basically trying to find a way to interpret the ValidationError's. From an internationalisation point of view I have to dismiss the message attribute of the ValidationError, since I have to built my own message. Based on what you're describing it would be quite difficult to interpret the ValidationError attributes, if not impossible.

To to ask for your opinion, if the ValidationError didn't have the message in it. How would you generate the message based on the given attributes?

matt-allan commented 7 years ago

At the moment I'm using the keyword to look up the relevant message (since it's unique) and the message has placeholders that correspond to the context keys. Here is an example. I just manually mapped the context keys since they won't change in a minor release.

northern commented 7 years ago

Ah. OK. Thanks. That's seems like one way of doing it.

I think the fundamental issue I'm dealing with is that the ValidationError object seems to be about the "message" rather than the "cause" of the error.

I just happen to have a system where the UI is responsible for the message, not the validation process. Somehow I need to be able to communicate to the UI what the error and with only a message as the only real practical attribute this becomes difficult.

But thanks for your help. I'll try to somehow manage to work around this (hopefully :).

matt-allan commented 7 years ago

Hello 👋

I just tagged 1.0.0-alpha, which changed how errors are handled. All constraints now return the same context keys, so you can interpolate the messages without having to check if a key exists. The keys also map to constants on the error class so you can use those if you like. You can see the updated docs here.

I'm going to close this issue but feel free to re-open it if you have any questions.