jpmckinney / validictory

🎓 deprecated general purpose python data validator
Other
240 stars 57 forks source link

TypeError during null date #5

Closed rennat closed 13 years ago

rennat commented 13 years ago

A type error is being raised when validating this property:

...
"shipDate": null, 
...

against this schema:

...
"shipDate": {
    "required": true, 
    "type": [
        "string", 
        null
    ], 
    "format": "date"
}, 
...

stack trace:

...
  File "/Library/Python/2.6/site-packages/validictory/__init__.py", line 26, in validate
    return v.validate(data,schema)
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 454, in validate
    self._validate(data, schema)
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 457, in _validate
    self.__validate("_data", {"_data": data}, schema)
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 486, in __validate
    newschema.get(schemaprop))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 151, in validate_properties
    properties.get(eachProp))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 486, in __validate
    newschema.get(schemaprop))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 151, in validate_properties
    properties.get(eachProp))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 486, in __validate
    newschema.get(schemaprop))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 257, in validate_additionalProperties
    additionalProperties)
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 486, in __validate
    newschema.get(schemaprop))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 151, in validate_properties
    properties.get(eachProp))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 486, in __validate
    newschema.get(schemaprop))
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 354, in validate_format
    format_validator(self, fieldname, value, format_option)
  File "/Library/Python/2.6/site-packages/validictory/validator.py", line 20, in validate_format_datetime
    datetime.strptime(value, dateformat_string)
TypeError: strptime() argument 1 must be string, not None
jamesturk commented 13 years ago

hm.. i guess i'm not entirely sure what the intended behavior is (though a TypeError is definitely a bug)

it seems to me like specifying a format but passing in a None value should result in a failure, thoughts?

rennat commented 13 years ago

I expected that a schema that can be a string or null would only use the format attribute if it was not null. It was definitely an assumption on my part and may be wrong but It makes sense to me that there will be cases (I have one) when you have an optional date but the property should always be present.

The JSON Schema specification draft's section on the format attribute doesn't address this specifically.

...A format SHOULD only be used to give meaning to primitive types (string, integer, number, or boolean)...

What about when an object can be one of those primitive types or null?

ericmoritz commented 13 years ago

This occurs when required=False on the field as well

rennat commented 13 years ago

The null could be subjective but required=false should definitely ignore format (logically if not explicitly within the spec)

jamesturk commented 13 years ago

the format part of the spec is definitely lacking in detail, I found a few discussions elsewhere about this exact same ambiguity

I think that the solution is to validate that the input value is a string if a format that expects a string is applied, otherwise it is too ambiguous if format was checked or not

as far as required=False, that stems from an issue with validators not being checked in any particular order.. I think I can solve that one, if either of you have a decent solution I'd be happy to accept a patch though

jamesturk commented 13 years ago

assuming the solution in #6 is agreeable I'll be merging it in soon

jamesturk commented 13 years ago

fixed in latest commit