$ref attribute

[psihonavt]

Hello Julian,

Are you planning to implement support of $ref attribute in your library?
$ref attribute gives a lot of flexibility in building schemas for validating dynamic JSONs.

Thanks.

[Julian]

Hi there!

I am ultimately, but we haven't had a personal need for it yet, and, even though the entire JSON Schema draft spec is quite confusingly written, $ref is particularly confusing when it comes to what the proper behavior for conflicts and things are, so I haven't spent the time to decipher it yet. Patches welcome of course if you'd like to take a stab at it :), otherwise I'm sure eventually it'll happen.

[psihonavt]

Get it,
anyway, thanks for sharing your library!

[Julian]

Sure thing, hope it's otherwise useful to you. You can leave this open, maybe it will motivate this to happen :).

[gazpachoking]

Hmm, I started implementing $ref for local json-pointers (as that much support would allow at least the meta-schema to be fully validated,) but realized that validate_ref does not have access to the root schema to look up the reference. Any ideas on how that should be handled?

[gazpachoking]

Well, got it working at least. I'm not so wild about the root schema hack though, let me know if you think of something better, or of any other issues before I issue a pull request.

[gazpachoking]

Another thing I was wondering while writing this: is ignoring a non-applicable property the correct behavior? If so, I also fixed a bug with the dependencies validator I wrote in this branch. What I mean:
Should this schema:

{
    "dependencies": {
        "foo": "bar"
    }
}

validate an instance that is not of type "object"?
The schema gets a default type of "any", but I'm not sure if the dependencies property should make it fail when the instance is not an object, or just be ignored. My leaning after reading the spec is that it should be ignored.

[Julian]

Oy, yes that is hairy. I'll have to checkout and play with it a little bit to see if anything comes to me.

And yes, I agree that validate("foo", that) should not be a ValidationError as per the spec – if you could file that as a separate pul req I can merge it immediately (with a test please if possible :), otherwise I'll take a look when I review this.

[gazpachoking]

What if Validator objects got passed the schema on init and kept it? This would make it easy for validator methods to get at the root of the schema. This would also mean that metavalidation could be done once, and the Validator object would be safely reusable without repeating it.

EDIT: A set_schema(self, meta_validate=True) method could also be added to change the schema after creation.

EDIT2: This would also allow easy implementation of $ref support for inline addressing.

[Julian]

That'd work, not sure how much I like it, but no matter what it'd constitute an API change and would require a deprecation, where we'd anyways need to implement things as-is to satisfy that. So I'd like to avoid that if possible unless it's unavoidable or overwhelmingly convincing.

[gazpachoking]

My main case for this is that it seems there are a few actions that should only be done once, at the first load of the schema. Meta validation would be one of them, and scanning the schema for id members to create a lookup table of inline sub-schemas would be the other that I can think of right now. I made a branch that works this way, still didn't implement inline schema addressing yet, maybe we can come up with some better way.

[Julian]

That sounds reasonable (that there might be things that should be done when validation starts, and only once), but I take the first call to iter_errors to be the beginning of validation, not the construction of a Validator.

Right now, as you can tell, jsonschema avoids doing meta-validation more than once by manually passing flags, which is non-ideal.

[gazpachoking]

If we assume that the first call to iter_errors is the start of validation, how do we tell which is the first call to it? The hack I used certainly does this, but it does not give a way to do things only once per schema, like meta-validation. It doesn't seem right to have to remember this from your calling app, and change the meta-validate flag on your next call to iter_errros.

Do you have any alternate ideas on how we can accomplish this only-once stuff?

[gazpachoking]

Maybe if we have a separate method for iter_errors to be called externally than used for internal recursive stuff. When the external one was called, it would do the once-only work. The next time it was called, the new schema could be compared to the last one validated, and once-only work skipped if they are the same.

EDIT: like this

def iter_errors(self, instance, schema, meta_validate=True):
    if schema != self._schema:
        self._once_only(schema, meta_validate=meta_validate)
    self._iter_errors(instance, schema)

[Julian]

Well I'm not totally disagreeing :).

The problems are definitely related to iter_errors doing too much, so OK, let's change that then.

The way the objects are, I see Validator as a thing you use to get ready to validate with one or more schemas and iter_errors to be a thing you use with many schemas, but you're right that the middle case of one schema many times is something important, and that by better separating the components we can support it better.

So, I think we should go ahead with what you have in mind – deprecating iter_errors' current use and introducing that use_schema method. So Validator should take an optional schema arg, and use_schema should be used to set the schema in use, where it can do the one-time things. I haven't thought out yet what implications this has for how iter_errors should look but I'll give that some thought in a minute. I guess schema could / should be optional there, and not do any meta-validation at that point.

[gazpachoking]

I updated my branch a bit with these ideas. I separated iter_errors and is_valid to two different methods, one for internal sub-schema validation, and one for external calls to the Validator. The internal one takes schema as an argument, the external doesn't. These could be combined again to make schema an optional argument if you think that is more appropriate.

[Julian]

This looks OK more or less, but I think after a bit of thought now is as good a time as any to introduce a Draft3Validator class and deprecate Validator, so I'm going to do that rather than just merging on top of Validator and needing to worry about backwards compat.

Oh and the reason why I'd like it to be an optional arg is because the "internal" iter_errors is also public API for Validator subclass implementors, so I'd like it to look like that. I'll make that change when I move it to the new class.

[gazpachoking]

I already made a Draft3Validator class over in my draft 4 branch if you are interested in that. Haven't merged $ref support over there yet though.

[gazpachoking]

Ok, I updated the draft 4 branch with recent changes, and added in $ref support. Not sure if we need to change anything else with regards to how the Draft3Validator class is used.

[Julian]

I'd like to pull in just the addition of Draft3Validator without $ref or any Draft 4 stuff yet, so that we can first do a deprecation release and the introduction of the new interfaces. Don't want to have you doing too much double work, so if you'd like you can leave it and I'll selectively merge :).

[gazpachoking]

Yeah, the draft 4 stuff certainly isn't ready to be merged in yet, but it should be pretty clean to just cut that stuff out.

[gazpachoking]

Ok, I made another branch https://github.com/gazpachoking/jsonschema/tree/draft_3_noref without the ref or draft 4 changes (and your latest master changes). I also made https://github.com/gazpachoking/jsonschema/tree/draft_3 branch which includes ref, but not draft 4 stuff. Turns out I'm not super awesome at git branching yet, so I hope I did everything right. :P

EDIT: Also, I didn't update the readme yet, and some docstrings may still need tweaking.

[Julian]

Cool thanks :). As you might be able to tell, those small refactorings are gonna make the rename easier to test, so hopefully today or tomorrow I'll look at getting this merged.

[Julian]

OK, I did a bit more than just merging ;), but what I've done should make things a bit easier to work with. Take a look at the current HEAD and see if you've got more ideas.

[gazpachoking]

Looks good to me, I like all your changes. Only thing I was wondering about was why Validator.init takes _args and *_kwargs. Why not just accept types keyword and pass it in to super? A minor nitpick I know, people need to stop using that class anyway.

[Julian]

No particular reason. Mostly the args that you do see there are the ones I removed from Draft3Validator but are there for backwards compat (without doing anything) since I wasn't too happy with them anyhow, so *args and **kwargs were put there before I even decided what Draft3Validator would take. We can put types there explicitly. There's a few messy things, docstrings that need cleanup and things like that, so I'm sure a few cleanup commits besides this would also be nice but I've got some other work to do at the moment.

[Julian]

Local refs, non local (url refs) and pre-cached url refs. Think that covers everything right? If so, think this is done. Try out HEAD and feel free to re-open if I missed something.