pandastrike / jsck

JSON Schema Compiled checK
MIT License
159 stars 14 forks source link

Error thrown during validation if schema contains an `id` #103

Open jasonkarns opened 7 years ago

jasonkarns commented 7 years ago

Sample taken directly from the Readme, with an id added to the schema:

jsck = new JSCK.draft4
  id: "http://www.google.com"
  type: "object"
  properties:
    user:
      type: "object"
      required: ["login"]
      properties:
        login:
          type: "string"
          pattern: "^[\\w\\d_]{3,32}$"
        email:
          type: "string"
          format: "email"

jsck.validate
  user:
    login: "matthew"
    email: "matthew@pandastrike.com"

This throws the error:

Error: No schema found for '"#"'
    at Validator.validator (/Users/jasonkarns/Projects/shutterstock/analytics-schema/node_modules/jsck/lib/validator.js:142:15)
    at Validator.validate (/Users/jasonkarns/Projects/shutterstock/analytics-schema/node_modules/jsck/lib/validator.js:99:19)
    at repl:1:6
    at REPLServer.defaultEval (repl.js:262:27)
    at bound (domain.js:287:14)
    at REPLServer.runBound [as eval] (domain.js:300:12)
    at REPLServer.<anonymous> (repl.js:431:12)
    at emitOne (events.js:82:20)
    at REPLServer.emit (events.js:169:7)
    at REPLServer.Interface._onLine (readline.js:211:10)

Based on my debugging, this occurs because a schema's id is used as the key for that schema. If no id is provided, then the schema is indexed with # as the key. https://github.com/pandastrike/jsck/blob/fdc93ac2bcecc8bf10463b5b51569fa7219620aa/src/validator.coffee#L78-L80

Then when validation is attempted, the validate call just blindly uses '#' as the key to lookup the schema: https://github.com/pandastrike/jsck/blob/fdc93ac2bcecc8bf10463b5b51569fa7219620aa/src/validator.coffee#L86

If the schema in question has an id, then the schema lookup fails to find one indexed under '#', and throws the error: https://github.com/pandastrike/jsck/blob/fdc93ac2bcecc8bf10463b5b51569fa7219620aa/src/validator.coffee#L107

automatthew commented 7 years ago

Thanks for the report. I'll see what I can do.

automatthew commented 7 years ago

@jasonkarns, this is a usage issue (which may also indicate a design problem). I've modified your example to make it work:

jsck = new JSCK.draft4
  id: "urn:some.id#"
  type: "object"
  properties:
    user:
      type: "object"
      required: ["login"]
      properties:
        login:
          type: "string"
          pattern: "^[\\w\\d_]{3,32}$"
        email:
          type: "string"
          format: "email"

validator = jsck.validator(uri: "urn:some.id#")

validator.validate
  user:
    login: "matthew"
    email: "matthew@pandastrike.com"

An instance of JSCK may contain numerous schemas, all with different ids. The README example you worked with demonstrates the simple, single-schema usage. When there may be more than one schema involved, the current design requires that you specify which schema you want to validate a document with.

dyoder commented 7 years ago

@automatthew can we improve the docs on that?

automatthew commented 7 years ago

We can and should.

dyoder commented 7 years ago

@automatthew or if we don't see a URI, can we simply assume there's only one id and use that?

automatthew commented 7 years ago

Yeah, maybe with this logic:

dyoder commented 7 years ago

https://github.com/pandastrike/jsck/issues/104

jasonkarns commented 7 years ago

Part of this is still throwing me for a loop. I understand current behavior, but note in my example I'm only constructing the validator with a single schema. Presence of an ID in the schema does not change the fact that the validator still only has a single schema and should operate as such.

Basically, current behavior is saying that the content of a schema will alter the acceptable usage of the validator. And that's a flaw, IMO.

automatthew commented 7 years ago

I agree. I'll be altering it per my comment just above.

Another option I considered is having two different validation classes: one for the case where you are using a single schema, and another for when you want the validator to have access to several. That is arguably more semantically accurate, but probably even more confusing.

For most of our use cases (and it seems also for yours) we don't need to have multiple schema documents within a single validator. We use #/definitions/ and JSON pointers as needed.