Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model definitions, array of primitive types (i.e. type : [ "string", "null" ] ) #459

Closed
chimmelb opened this issue Feb 12, 2014 · 12 comments
Milestone

Comments

@chimmelb
Copy link

I'm using swagger on a project and validating the JSON HTTP requests and responses with the same swagger models. Quite handy since both are JSON v4 compliant . . . almost.

It is valid JSON for the "type" of a property to be an array of primitives. See section 5.5.2 of the JSON schema validation document.

My use case has a model object definition like this:

var contact = {
  "id" : "Contact",
  "description" : "Contact information.",
  "additionalProperties" : false,
  "properties" : {
    //...
    "middleName" : {
      "type" : "any",//["string","null"],
      "description" : "middle name, is a string, may be null"
    },
  }
}

I use a JSON v4 compliant validation module to ensure input and outputs to our system are valid. Like when an object is returned from our database(-layer) I validate against the same Model that is defined in swagger. Sweet! However, some database fields can be null.

I use "type" : "any" to make swagger and my validation happy, except it's not true that any type will work, and I'm stuck with un-enforceable comments to let the user of the API know how to pass data.

Supporting the JSON v4 spec with an array of types would be great when using the JSON v4 model in both swagger and validation, especially with a pass-through database that allows NULLs.

@rage-shadowman
Copy link

That is what the required property is for. If a property is not required, then it may be left out (unset, null, undefined).

@chimmelb
Copy link
Author

I think that is a slightly different meaning. required means the property must be present, while "type" : [ "string", "null" ] means it is either a string or null, and may or may not be required. Take for example this model definition:

var contact = {
  "id" : "Person",
  "description" : "an example person schema",
  "additionalProperties" : false,
  "properties" : {
    "firstName" : {
      "type" : ["string","null"],
    },
    "lastName" : {
      "type" : "string",
      "required" : true
    },
  }
}

An object like { firstName : "Bob" } would fail validation because "lastName" is required. Also { firstName : 7, lastName "Boberson" } will fail, as a number is a bad type for "firstName" An object like { lastName : "Boberson" } would pass validation because "firstName" is not required. { firstName : null, lastName : "Boberson" } will also pass validation because null is a valid type for "firstName" (like the case where it is NULL in my DB).

@fehguy
Copy link
Contributor

fehguy commented Feb 13, 2014

Hi, what your saying makes sense, but isn't supported in swagger. Attributes are either required or not, and each attribute needs to have a fixed dataType. In your case, it is true that it is a valid JSON Schema representation, however with swagger you'd want to solve it like such:

{
  "Contact" : {
     "id": "Contact",
     "required": [
        "lastName"
     ],
     "properties": {
        "firstName": {
           "type": "string"
        },
        "lastName": {
           "type": "string"
        }
    }
}

the missing / null work would need to happen before it goes in your db.

@chimmelb
Copy link
Author

I'm inheriting a DB schema, so some of the data we have is already null. When it comes back from the DB, it's not that a property (required or not) is missing, its that the value is null, like { "prop" : null }. In that case "prop" would fail for type "string", and would also be present to pass any "required" checks.

It still is a convenience thing for me to have a single Model that I use for swagger and JSON validation (rather than two different ones). My "type" : "any" work around is sufficient for now. I'll keep following this issue and hope to see it in a future swagger update to match the JSON spec : )

You're putting together some great stuff here, by the way, this has been an excellent scheme to follow for our REST API definitions!

@rage-shadowman
Copy link

Weird... I would've assumed that if a property is required, it must be present and must not be null or undefined. That certainly seems a more useful definition of "required" than the one mentioned by @chimmelb above.

@rage-shadowman
Copy link

You could always send your JSON through a filter that removes properties with null values...

We do this in Java via Jackson's:
ObjectMapper.setSerializationInclusion( JsonSerialize.Inclusion.NON_NULL );

@quasipedia
Copy link

@rage-shadowman - I'm not sure I understand why you don't see the usefulness for a required property to be set to null. Having a property unset and having it set to null are not the same thing.

An example of why this could be useful: if you are loading a JSON object into a python dictionary (the standard conversion for the json module in the core library) you will obtain very different behaviours for the two cases:

>>> o = '{"foo": 42, "bar": null}'
>>> print(loads(o)['bar'])
None
>>> o = '{"foo": 42}'
>>> print(loads(o)['bar'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'bar'

...or did I misunderstood the point you were trying to make?

@rage-shadowman
Copy link

I just think that a missing property is essentially the same as a null value. Even in javascript if you use if( !object.prop ){ doSomething(); } then you will run doSomething(); whether prop was missing or null.

You have found a reason why it makes a difference (essentially the same is not exactly the same). You could probably work around this by using a template that you extend with the json object if you wanted to. I don't know how that syntax would look in python though.

@quasipedia
Copy link

@rage-shadowman

essentially the same is not exactly the same

Well, there you have it above. Having a property set to null is not the same as not having a property altogether. It seems to me that you are implicitly using the mental model of a relational DB (in which you cannot have records in a table that simply "miss a column").

However this is an artificial constraint that should not be imposed onto an API specification.

The most obvious user case for which assuming a non-existent property is equal to null is plain wrong is when you are using duck typing or any other introspective technique relying on the behaviour/structure of data to infer business logic.

A silly example: you might have a class Worker that subclasses to Translator adding Translator.books. Now, if you are transferring data without metadata associated to it, you need the books attribute to distinguish between the two models: a translator must have a books property even if she/he has only translated business documents (in which case it will be set to null.

As I said, the example above it a little silly, but especially when you have no control over the source of the data (as in the OP case, where (s)he uses a legacy DB), duck typing is a very useful technique.

The definitive reason not to go with the assumption you suggested is however not the user caes, but the fact the two are simply not just the same thing in terms of formal logic (and thus implementation in many languages).

@webron
Copy link
Contributor

webron commented Mar 2, 2014

Using my phone, unfortunately, so I'll keep it brief.

Mac is absolutely right regarding the null issue - a null field is not
equivalent to a non-existing field. MongoDB is an example where they have
two distinct different meanings.

Whether it's good to rely on it or not is besides the point.
On Mar 1, 2014 12:28 PM, "Mac Ryan" notifications@github.com wrote:

@rage-shadowman https://github.com/rage-shadowman

essentially the same is not exactly the same

Well, there you have it above. Having a property set to null is not the
same as not having a property altogether
. It seems to me that you are
implicitly using the mental model of a relational DB (in which you cannot
have records in a table that simply "miss a column").

However this is an artificial constraint that should not be imposed onto
an API specification.

The most obvious user case for which assuming a non-existent property is
equal to null is plain wrong is when you are using duck typing or any
other introspective technique relying on the behaviour/structure of data to
infer business logic.

A silly example: you might have a class Worker that subclasses to
Translator adding Translator.books. Now, if you are transferring data
without metadata associated to it, you need the books attribute to
distinguish between the two models: a translator must have a booksproperty even if she/he has only translated business documents (in which
case it will be set to null.

As I said, the example above it a little silly, but especially when you
have no control over the source of the data (as in the OP case, where (s)he
uses a legacy DB), duck typing is a very useful technique.

The definitive reason not to go with the assumption you suggested is
however not the user caes, but the fact the two are simply not just the
same thing in terms of formal logic (and thus implementation in many
languages).


Reply to this email directly or view it on GitHubhttps://github.com//issues/459#issuecomment-36421400
.

@dinoboff
Copy link

Support for null value is also import to support array items type. You need to be able to define the type of an array that may contain null values.

chimmelb added a commit to chimmelb/swagger-node-restify that referenced this issue Mar 27, 2014
@fehguy fehguy modified the milestone: v1.5.0 Sep 19, 2014
@fehguy
Copy link
Contributor

fehguy commented Dec 20, 2014

This issue has drifted some since opened. I'm opening a new one here:

OAI/OpenAPI-Specification#229

@fehguy fehguy closed this as completed Dec 20, 2014
pkarman pushed a commit to 18F/e-manifest that referenced this issue Feb 17, 2016
… anyOf construct, so just remove the offending attributes from the model definition (swagger-api/swagger-core#459)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants