apiaryio / mson

Markdown Syntax for Object Notation
MIT License
903 stars 180 forks source link

MSON: nullability of data structures handled differently from built-in types? #96

Open stkent opened 8 years ago

stkent commented 8 years ago

If I define the following spec on Apiary.io:

FORMAT: 1A
HOST: http://polls.apiblueprint.org/

# Test

## Root [/]

### Get Everything [GET]

+ Response 200 (application/json)
    + Attributes
        + example1 (string, optional, nullable) - An optional nullable string
        + example2: value (string, optional, nullable) - An optional nullable string with a sample value

+ Response 201 (application/json)
    + Attributes
        + example1 (Example, optional, nullable) - An optional nullable Example
        + example2: value (string, optional, nullable) - An optional nullable string with a sample value

# Data Structures

## Example
+ title (optional, nullable)
+ description (optional, nullable)

then I see the generated response structures below:

Response 200

{
  "example1": null,
  "example2": "value"
}

Response 201

{
  "example1": {
    "title": null,
    "description": null
  },
  "example2": "value"
}

I expected the second response structure to match that of the first response.

If I update my Data Structure section to:

# Data Structures

## Example
+ title (optional)
+ description (optional)

then the generated response for status code 201 becomes

{
  "example1": {},
  "example2": "value"
}

Again, I would have expected this to match the structure of the status code 200 response. What am I not understanding here?

pksunkara commented 8 years ago

When you have (Example, optional, nullable), we give preference to the named type. But you do raise a good use case for nullable.

@kylef @zdne What do you guys think?

stkent commented 8 years ago

Thanks for the response! Just to check - there isn't a keyword to force an object to be represented as null in the generated response, is there? I wasn't sure if that already existed and I was just failing to find it.

zdne commented 8 years ago

Just to check - there isn't a keyword to force an object to be represented as null in the generated response

no there isn't. Here are the rendering recommendations for the nullable type modifier: https://github.com/apiaryio/mson/blob/master/rendering/nullable.md

The dilemma in this case in the fact that if a nullable specified but no value is provided the value should be rendered as null (in the case of JSON).

Since, in your case, Example does not have any sample value provided you are right the result should be

{
  "example1": null,
  "example2": "value"
}

So this is a bug in the parser / renderer

However. Interesting thing happens when the type has a sample value provided. What should happen then? Consider following example:


# My Object (object)
- value (My Number, optional, nullable)

# My Number (number)
## Sample
42

Should this render { "value": 42 } or {"value": null} ? Did the author wanted to say

  1. value is optional, of type My Number and since I haven't provided a sample value here render it as null (in JSON)
  2. value is optional, of type My Number and since My Number has a sample value provided use it and render it instead of null

I would say the former (ignore sample value of the type). But I would like to hear your opinion @stkent

stkent commented 8 years ago

Thanks for confirming the existing behavior as a bug.

Regarding your follow-up question: both options leave a tricky edge case to deal with:

  1. in this case, if I did want to use the sample My Number in my sample My Object, how would I do so without repeating the entire sample definition?
  2. in this case, if I did want to have a sample My Object with value set to null (which is valid, since it's nullable), how would I override the sample My Number?

It seems to me that option two is actually preferable here: if a sample exists, use it everywhere applicable unless that sample is explicitly overridden. That would involve the introduction of a null keyword, but I think that's cleaner than introducing some way to refer to named type samples for reuse.

There's a similar case to be made for handling an optional and nullable entry - if nothing is specified as a sample, should this field be omitted or set to null? Again, maybe a pair of keywords (null, xor omitted) are needed to allow authors to fully express their intent in this case.

pksunkara commented 8 years ago

The intention of the nullable keyword in the first place was to be the null you are talking about.

stkent commented 8 years ago

Yes, that was my understanding after reading through https://github.com/apiaryio/mson/issues/26. However, there is a difference between nullable as a keyword used to express allowed structure, and the ability to express whether a given element in a sample should be null. The latter cannot be inferred consistently from the former alone, and relying on omission to indicate that a sample element should be null limits the reusability of said samples.

zdne commented 8 years ago

@stkent

That would involve the introduction of a null keyword, but I think that's cleaner than introducing some way to refer to named type samples for reuse.

Re-reading this issue, I think we really should introduce the null as a possible sample value. (Precisely for the case no.2.)

I wanted to avoid so the MSON (parser) doesn't have to introspect the sample values but I think this is a valid need...