jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
54 stars 44 forks source link

Type specified as number but converted to string in JSON output by openapi render #111

Closed srchulo closed 5 years ago

srchulo commented 5 years ago

Here is my definition for an object that's returned by an API call:

  Chart:
    type: object
    required:
      - labels
      - last_period
      - this_period
    properties:
      labels:
        type: array
        items:
          type: string
          example: 'Mar 2017'
      last_period:
        type: array
        items:
          type: number
          format: float
          example: 1234.56
      this_period:
        type: array
        items:
          type: number
          format: float

Here is what the data structure looks like before passing it to openapi to render (printed using [p](https://metacpan.org/pod/Data::Printer#THE-p()-FUNCTION) from Data::Printer):

\ {
    labels        [
        [0]  "1/23",
        [1]  "1/24",
        [2]  "1/25",
        [3]  "1/26",
        [4]  "1/27",
        [5]  "1/28",
        [6]  "1/29",
        [7]  "1/30",
        [8]  "1/31",
        [9]  "2/1",
        [10] "2/2",
        [11] "2/3",
        [12] "2/4",
        [13] "2/5",
        [14] "2/6",
        [15] "2/7",
        [16] "2/8",
        [17] "2/9",
        [18] "2/10",
        [19] "2/11",
        [20] "2/12"
    ],
    last_period   [
        [0]  1456,
        [1]  1563,
        [2]  2194,
        [3]  578,
        [4]  0,
        [5]  0,
        [6]  207,
        [7]  1531,
        [8]  2921,
        [9]  400,
        [10] 1977,
        [11] 2246,
        [12] 2353,
        [13] 3146,
        [14] 2348,
        [15] 2967,
        [16] 2965,
        [17] 1587,
        [18] 1716,
        [19] 1402,
        [20] 996
    ],
    this_period   [
        [0]  860,
        [1]  1929,
        [2]  1832,
        [3]  1772,
        [4]  2254,
        [5]  2131,
        [6]  1241,
        [7]  1378,
        [8]  1171,
        [9]  3260,
        [10] 1974,
        [11] 1519,
        [12] 3055,
        [13] 805,
        [14] 2724,
        [15] 1585,
        [16] 4992,
        [17] 3073,
        [18] 2037,
        [19] 2445,
        [20] 1979
    ]
}

And here's what the JSON output is returned by openapi:

{
    "labels": [
        "1/23",
        "1/24",
        "1/25",
        "1/26",
        "1/27",
        "1/28",
        "1/29",
        "1/30",
        "1/31",
        "2/1",
        "2/2",
        "2/3",
        "2/4",
        "2/5",
        "2/6",
        "2/7",
        "2/8",
        "2/9",
        "2/10",
        "2/11",
        "2/12"
    ],
    "last_period": [
        1456,
        1563,
        2194,
        578,
        0,
        "0",
        207,
        1531,
        2921,
        400,
        1977,
        2246,
        2353,
        3146,
        2348,
        2967,
        2965,
        1587,
        1716,
        1402,
        996
    ],
    "this_period": [
        860,
        1929,
        1832,
        1772,
        2254,
        2131,
        1241,
        1378,
        1171,
        3260,
        1974,
        1519,
        3055,
        805,
        2724,
        1585,
        4992,
        3073,
        2037,
        2445,
        1979
    ]
}

As you can see, the one 0 in the last_period array is a string for some reason, when everything else is a number. This isn't actually causing any issues for me client side, but I wanted to let you know since this seems like a bug. Please let me know if any other information from me would be helpful!

jhthorsen commented 5 years ago

I think this is caused by the “format” key in your specification. I’m not sure when I can find time to fix that, so I would suggest trying to remove the “format” for now.

How did you pretty print the JSON? I’m curious, in case the pretty printer might have changed something...

You can also look at the tests in https://github.com/jhthorsen/mojolicious-plugin-openapi/blob/master/t/v2-formats.t and maybe improve them.

srchulo commented 5 years ago

So I removed "format", and still see the issue. I pretty printed it with Postman, but when I switch it to raw format, it still shows the quotes around the zero:

{"labels":["1\/23","1\/24","1\/25","1\/26","1\/27","1\/28","1\/29","1\/30","1\/31","2\/1","2\/2","2\/3","2\/4","2\/5","2\/6","2\/7","2\/8","2\/9","2\/10","2\/11","2\/12"],"last_period":[1456,1563,2194,578,0,"0",207,1531,2921,400,1977,2246,2353,3146,2348,2967,2965,1587,1716,1402,996],"this_period":[860,1929,1832,1772,2254,2131,1241,1378,1171,3260,1974,1519,3055,805,2724,1585,4992,3073,2037,2445,1979]}

And I actually see the same thing in the Chrome Network/Response tab:

{"labels":["1\/24","1\/25","1\/26","1\/27","1\/28","1\/29","1\/30","1\/31","2\/1","2\/2","2\/3","2\/4","2\/5","2\/6","2\/7","2\/8","2\/9","2\/10","2\/11","2\/12","2\/13"],"last_period":[1563,2194,578,0,"0",207,1531,2921,400,1977,2246,2353,3146,2348,2967,2965,1587,1716,1402,996,1718],"this_period":[1929,1832,1772,2254,2131,1241,1378,1171,3260,1974,1519,3055,805,2724,1585,4992,3073,2037,2445,2040,189]}

I'll try to take a look at the file and see if I can improve the tests, but that would still require updating the actual code, right?

jhthorsen commented 5 years ago

I think so, but this is very surprising to me, since there’s a lot of tests for making sure a number is indeed a number.

Which versions of JSON::Validator and M::P::OpenAPI do you have? Are you sure you have the latest?

jhthorsen commented 5 years ago

Related issue is https://github.com/mojolicious/json-validator/issues/147

I’m going to close this, since it’s a JV bug, and a new issue is opened.

srchulo commented 5 years ago

Ah, thanks for figuring out what this was! I appreciate it :)