partial objects (missing attributes)

[dpopowich]

If I have a definition like this:

   "definitions": {
      "Foo": {
         "properties": {
            "id": {
               "type": "integer"
            },
            "name": {
               "type": "string"
            },
            "job": {
               "type": "string"
            }
         },
         "type": "object"
      },
     ...

And I reference this in a PUT spec:

"paths": {
      "/foo/{id}": {
         "put": {
            "parameters": [
               ...
               {
                  "required": true,
                  "in": "body",
                  "name": "body",
                  "schema": {
                     "$ref": "#/definitions/Foo"
                  }

               }
            ], ...

When I call PUT /foo/14 with a JSON body of:

{
   "id": 14,
   "name": "joeschmoe"
}

The unmarshalled python output from bravado-core is:

{
   "id": 14,
   "name": "joeschmoe",
   "job": None
}

That is, job was missing, but bravado-core explicitly looks for missing properties and sets them on the object with a None value.

I cannot find this behavior specified in the swagger spec anywhere. If it exists, can someone point me to this requirement (forcing missing properties to null) in the spec?

If this behavior is not required by the swagger spec and bravado-core is overreaching here, how about an option to allow partial objects?

Otherwise, how is someone supposed to provide PATCH or similar functionality?

[sjaensch]

None means "unknown" / "not set". The current behavior is to make sure all attributes defined in the swagger spec are present in the unmarshalled object. You may want to discard the attributes that have the value None.

[dpopowich]

How do I determine the difference between "value not sent" and "set this value to None"?

Or, another way to ask the question, how do I implement PATCH, if bravado-core is going to set every value not delivered to None? Imagine an object with 100 properties and I want to send a PATCH to update 5 properties. One can't simply filter out all properties with value None because a legal value for a property may be None (e.g, one object references another and you want to disassociate the foreign reference so you pass null as the value).

Or yet another way to ask the question, where in the swagger spec does it say optional properties are to be treated as null?

I think bravado-core is overreaching with this behavior. It should at least be configurable.

[sjaensch]

Unless you use the optional x-nullable support, the value should never be None. It's either what the caller passed, converted to the right type according to the spec (which might be an empty string, the number 0, the boolean value False and so on), or it will be None in the case the caller never passed an argument with that name. So providing a query parameter in the form of intvar=&... (i.e. an empty value for an integer argument) will fail request validation.

[dpopowich]

I don't seem to be getting my point across. If a property is missing that is specified in a swagger JSON body parameter, bravado-core forces the value to None, setting it on the unmarshalled object even if it wasn't passed in the request.

My reading of the spec says If a property is not required it is optional. Placing missing properties into the unmarshalled object (even with value None) renders them indistinguishable from sent properties.

I've asked repeatedly, with this behavior how would I implement PATCH? If you think that through I hope you can see the problem: it is impossible to determine which properties were sent in the partial object since you cannot simply filter out properties with value None, as None may be a valid value sent from a client.

I cannot find this behavior specified in the swagger spec and I believe bravado-core is overreaching with an assumption that a missing property should be added with value None. To be clear:

I want any missing property from the request to be not present in the unmarshalled object.

If not globally, there should be minimally a parameter or configuration option that turns off this behavior.

[sjaensch]

Hey @dpopowich, we seem to be having a misunderstanding here. You need to distinguish between the spec (i.e. how data is being sent over the wire) and what you get if you use bravado-core to unmarshal objects for you.

The design decision was made quite some time ago to make sure all attributes should be present, whether they were sent or not. If they were not sent then the value would be None. This was fine since you weren't allowed to send attributes with the value of None anyway - that's how you distinguish them.

I've now repeatedly told you how to implement this. Have you ever actually tried it out? If an attribute has the value None then it was not sent by the other side. There is no other way in which a property can be None (unless you use the nonstandard x-nullable extension). Please try it out - I don't see any blockers to implementing the behavior you're looking for.

[dpopowich]

So, if a property has x-nullable: true how do you tell the difference between it being sent as null and it not being sent at all? And if an object has just one single property with x-nullable: true how do you discover which properties were sent and which were not without having to keep some extra catalog in your Python code of the x-nullable properties?

I'm using pyramid-swagger, which calls bravado-core. I want to be able to reference request.swagger_data and KNOW without further processing that the keys in the dictionary were passed in the request. You're suggesting I could do this:

    sent = dict([(k,v) for k,v in request.swagger_data.items() if v is not None])

But if just one property is marked with x-nullable: true this does not work and my pyramid view implementation code would need to know which keys are nullable, and even then, I'd not know if they were sent or not.

[sjaensch]

@dpopowich is this still an issue you're facing or are we having more of a theoretical discussion?
x-nullable could make this a bit more complicated, if your PATCH endpoint would behave differently for attributes that were passed in with the value None vs. them not being passed in at all. From a Swagger spec standpoint both of these cases are equivalent, it means the attribute's value is unknown. x-nullable was added to support existing server implementations that don't filter out attributes with None values.

I'd much rather tackle this as part of a larger Swagger 3.0 work unless you're running into real issues.

[dpopowich]

First, my apologies for leaving off the detail of x-nullable in my initial post...that created a lot of unnecessary confusion.

So, here's my situation: I am building a REST api with pyramid (and cornice) serving records from an ancient legacy RDB (wrapped by sqlalchemy). Nearly every table has nullable columns and these are not filtered out on request. The current consumer of the REST api is a single-page web app with the client data models managed with Backbone.

Because nearly every model has nullable columns, my swagger schema objects have a lot of x-nullable properties, making it impossible to distinguish unsent properties from those sent with value null. Imagine a PATCH that can send any subset of an object with 120 properties (many of which are marked x-nullable)...I cannot use None as a sentinel.

This issue, plus bravado-core not handling readOnly (or so it would appear - is this scheduled for inclusion?) and anyOf/oneOf not available until swagger 3 - I'm having a lot of difficulty expressing my API needs.

[dpopowich]

Related to this issue (and let me know if I should create a new issue) is the non-use of default in missing properties: if a property is missing, the value out of bravado-core is None regardless of whether or not default is set in the schema.

[sjaensch]

I'm not yet completely sure I understand your needs fully, but at this point I think merging #152 makes sense. In regards to your last comment, default not being honored sounds like a bug, please file an issue. Please make sure to point out if that property has x-nullable set, as the bug might be related to that interaction.

[dpopowich]

Great. It would be wonderful to see #152 included.

Regarding default, I created issue #153.