Handle datetime objects

[bobh66]

This patch adds support for nested date and datetime objects when the schema only defines type: object with no further specification. The lack of anyOf support in Swagger2.0 and the need to support generic object parameters make it impossible to always specify the exact schema of an object.

When a datetime object is embedded in an object that has no specified schema, the json.dumps() would fail without a handler for datetime types.

[coveralls]

Coverage increased (+0.006%) to 98.013% when pulling 0908ca51ab3fe4fac6e438d545013b1c04ddfc6d on bobh66:handle-datetime-objects into 2b9bb67961ca74ae94e54de33f2f4db8098f601b on Yelp:master.

[macisamuele]

@bobh66 could you give more context around the error that you're reporting and fixing? And potentially ways of reproducing it?

marshal_param performs an initial marshaling of the value (in your case the request body) according to what the endpoint specs define for the parameter. This means that if your body contains a datetime python object and the body specs expect a type: string / format: date-time it will be converted to it's ISO8601 representation.

I tried to reproduce it on master with the following code and I got no errors.

empty_swagger_spec = Spec(spec_dict={})

param_spec = {
    'in': 'body',
    'name': 'body',
    'schema': {
        'type': 'string',
        'format': 'date'
    },
}
param = Param(empty_swagger_spec, Mock(spec=Operation), param_spec)
request = {
    'headers': {
    }
}
marshal_param(param, datetime.date(2017, 10, 26), request) == datetime.date.today().isoformat()
assert request == {
    'headers': {'Content-Type': APP_JSON},
    'data': '"2017-10-26"',
}

[bobh66]

@macisamuele - thanks - you helped me realize that it's only a problem with nested datetime objects, which is what I'm working with. I have a top-level body parameter that is defined as type: object because it could be any of multiple different things depending on what's happening, and it's the embedded datetime objects that are seeing the problem.

I updated the tests to cause the problem and pushed a new patchset.

if you try to marshal {'updated_at": datetime(2017, 10, 26)} as type "object" it will fail on master.

Thanks

[macisamuele]

@bobh66 now I understand your issue. The JSON serialization problem is caused by marshal_schema_object not converting the datetime object to str. This happens because your specs do not specify any attribute and so marshal_schema_object is pretty much a NoOp. NOTE: this is an intended behavior.

Have you tried to specify the schema of the object?

{
    "type": "object",
    "properties": {
        "updated_at": {"type": "string", "format": "date-time"}
    }
}

[bobh66]

@macisamuele - I'm sure it would work if I could specify the schema, but the application requires me to accept a variety of different object schemas, and since Swagger2.0 doesn't support anyOf, there isn't much I can do beyond using object. Even if anyOf was supported there are scenarios where I simply don't know what the object will be until I receive it, so it's impossible to specify all of the possible schemas.

For example, in one case I need to supply JSON object inputs to a workflow engine, which vary depending on the workflow that is being executed, and the workflows can change over time. I don't want to have to specify the schema for every possible workflow input object, which is impossible to keep up with, so I have to call them objects and rely on the workflow engine to do the validation.

[coveralls]

Coverage increased (+0.006%) to 98.013% when pulling 782761c34bf92f648b6feeca0e36fc053cc53a2c on bobh66:handle-datetime-objects into 2b9bb67961ca74ae94e54de33f2f4db8098f601b on Yelp:master.

[macisamuele]

What you're describing is a problem with the specs and not an issue with the library. If the specs doesn't specify the format of a field is fair to assume that the input object should be JSON serializable and if a datetime instance is part of the object it is not true. This PR aims to workaround this issue by imposing something that is not compliant with the swagger specs.

For what's my understanding of your problem you have a python application (could be a script or could be a service) that communicates with a swagger service using bravado-core. I don't understand how you could have a datetime object if you haven't created it. The craziest condition that I could imagine is that you're running a swagger service that accepts a type: string; format: date-time and forward the input object to a swagger service that specifies the body simply as type: object. In this condition, you have a priori knowledge of the fields that are not serializable and you could convert them.

If you're not able to workaround your issue in any different way I would suggest you to open a PR to make dumps method configurable, so you can replace simplejson.dumps with your preferred one or a partial that injects kwargs (like default)

[bobh66]

The datetime object comes from bravado converting the response of a separate query based on the spec for that response. Then I have to take that response, modify it and pass it to another service nested inside another object. Yes I could convert the datetime fields at that point, but I thought it was reasonable to expect bravado to accept the datetime object that it generated in the first place. I understand that it does that today just fine at the top level, and it would work at lower levels if I could spec it that way, but I can't specify something that can be changed at will by the user at a higher level.

I considered making dumps configurable but I thought that was a rather complicated solution to a simple problem, and I avoid complexity whenever I can. If that's the preferred solution I'll go that way instead.

[macisamuele]

bravado converted the string representation of a datetime (ISO8001) to a datetime because that was made explicit from specs

it was reasonable to expect bravado to accept the datetime object that it generated in the first place

it did it correctly at top level because you specified type and format that instructed bravado-core to do that

it does that today just fine at the top level

having the default behavior that is not compliant with what the specs define is riskier.

I considered making dumps configurable ...

If you're using bravado, probably you can use also_return_response. In that way you'll have access to the raw response received by bravado (which basically correspond to the simplejson.load of the body -> JSON serializable)