Open cxreg opened 8 years ago
Thanks. Will investigate as soon as I can.
I've dived into this a little bit. I'm not 100% sure why the scoping changes around uri exposed this, but what is happening is that the empty schema fails is_schema because none of the properties are present. That causes it never to call compile() on this schema, and thus the _test method is never assigned.
It seems like is_schema should not be duck typing, since contextually we should know whether or not a schema is expected in a given spot, is that not so?
If it can't be fixed that way, it seems like maybe just conditionally not executing _test if it's not defined is a safe fix
It seems like is_schema should not be duck typing, since contextually we should know whether or not a schema is expected in a given spot, is that not so?
I think I added is_schema
so JSCK could handle structural nesting. E.g.
definitions:
user:
create:
type: 'object'
required: ['email', 'password']
update:
type: 'object'
If it can't be fixed that way, it seems like maybe just conditionally not executing _test if it's not defined is a safe fix
Maybe. That would produce correct behavior for this case. I think the underlying problem is in the compile_definitions
logic. It is plausible that an object in the schema document might be both a valid schema and a container of other schemas. Bad form to do so, in my opinion, but possibly something JSCK should support.
Here's my first thought on how to fix it:
compile_definitions2: (context, object) ->
if @is_schema(object)
@compile(context, object)
if @test_type "object", object
for name, definition of object when not_schema_keyword(name)
@compile_definitions context.child(name), definition
not_schema_keyword
Clever.
@cxreg, this branch appears to fix the original problem, while also somewhat rigorizing the approach.
Interesting. I think you still need the @test_type "object" conditionalizing @is_schema so as not to throw TypeError on numbers, strings, null, etc? Would that only happen on a malformed schema? Still probably bad.
This does look like a better approach if you are happy with the results.
I moved the type test to the top of compile_definitions
, to simplify the logic there, leaving is_schema
as a function with implied type of object. Perhaps I should inline it.
Starting in 0.2.8 this throws a fatal error. It only appears to happen if you have a reference to an empty schema. An empty schema by itself is fine as are refs in other situations.
I get this error
count@bumba:~/jsck-test$ ./crash-0.3.0.js
/home/count/jsck-test/node_modules/jsck/lib/validator.js:316 return (_ref1 = _this.uris[uri])._test.apply(_ref1, args); ^ TypeError: Cannot call method 'apply' of undefined at /home/count/jsck-test/node_modules/jsck/lib/validator.js:316:54 at /home/count/jsck-test/node_modules/jsck/lib/draft4/objects.js:52:15 at Object.test_function as _test at Object.validate (/home/count/jsck-test/node_modules/jsck/lib/validator.js:109:22) at Validator.validate (/home/count/jsck-test/node_modules/jsck/lib/validator.js:94:34) at Object. (/home/count/jsck-test/crash-0.3.0.js:11:21)
at Module._compile (module.js:456:26)
at Object.Module._extensions..js (module.js:474:10)
at Module.load (module.js:356:32)
at Function.Module._load (module.js:312:12)