Azuracast: The example response of some API endpoints don't match the response

[Vaalyn]

Installation method
Docker

Host OS (for Docker installations)
Debian 9

Describe the bug
On the Swagger page the API response examples for the /admin/users and /admin/user/{id} endpoint look like this:

[
  {
    "id": 1,
    "email": "[email protected]",
    "auth_password": "",
    "name": "Demo Account",
    "timezone": "America/Chicago",
    "locale": "en_US",
    "theme": "dark",
    "two_factor_secret": "A1B2C3D4",
    "created_at": 1555551974,
    "updated_at": 1555551974,
    "roles": [
      null
    ]
  }
]

The response of these API endpoints look like this:

[
    {
        "id": 1,
        "email": "[email protected]",
        "name": "admin",
        "timezone": "Europe/Berlin",
        "locale": "en_US.UTF-8",
        "theme": "dark",
        "created_at": 1540470707,
        "updated_at": 1540470707,
        "roles": [
            {
                "id": 1,
                "name": "Super Administrator",
                "permissions": {
                    "global": [
                        "administer all"
                    ],
                    "station": []
                }
            }
        ],
        "api_keys": [
            {
                "id": "a1234567f1cd29e6",
                "comment": "My API key"
            }
        ],
        "links": {
            "self": "https://demo.azuracast.com/api/admin/user/1"
        }
    }
]

The following fields are not part of the response: auth_password, two_factor_secret

The following fields are missing from in the example: api_keys, links

The locale field has a slightly different output

[Vaalyn]

This mismatch between the API response and the documented response seems to also be there for other endpoints. I'll probably use this issue to aggregate the endpoints where documentation and response are not matching so that we can fix this.

@SlvrEagle23 Can you give me a quick rundown on how the mapping between the entities that are retrieved by the API Crud controller and the response object is working? I'm not sure if I completely understood it. As I understood it we fetch the entity via Doctrine from the DB and then use the Symfony Serializer to transform that to the object in the response via the PropertyNormalizer. Is that correct? If so then I don't really understand why for example in the GET /station/{station_id}/files endpoints response the lyrics field is missing but the title is in there as there is no difference in the declaration of the properties in the StationMedia entity nor in the getter/setter for these fields.

This is an example of the response from the demo installation:

[
    {
        "id": 1,
        "song_id": "0063deb4c9224e4f4a18a67c4ff86bbb",
        "title": "Strolling Through",
        "artist": "Silent Partner",
        "album": "YouTube Audio Library",
        "length": 142,
        "length_text": "2:22",
        "path": "strolling_through.mp3",
        "mtime": 1556182835,
        "playlists": [
            {
                "id": 1,
                "name": "default",
                "weight": 4
            }
        ],
        "unique_id": "62b10ebcbe556daf089fee74",
        "custom_fields": [],
        "links": {
            "self": "https://demo.azuracast.com/api/station/1/file/1"
        }
    },
    ...
]

This is the example from the Swagger page:

[
  {
    "id": 1,
    "song_id": "098F6BCD4621D373CADE4E832627B4F6",
    "title": "Test Song",
    "artist": "Test Artist",
    "album": "Test Album",
    "lyrics": "...Never gonna give you up...",
    "isrc": "GBARL0600786",
    "length": 240,
    "length_text": "4:00",
    "path": "test.mp3",
    "mtime": 1556187131,
    "fade_overlap": 2,
    "fade_in": 3,
    "fade_out": 3,
    "cue_in": 30,
    "cue_out": 30,
    "playlists": [
      null
    ]
  }
]

[SlvrEagle23]

@Vaalyn It's stripping out null values. It probably shouldn't be doing this in the case of Doctrine API mappings.

[SlvrEagle23]

Also, one overarching explanation for why the API docs sometimes differ from what the API itself can do: on the same entity, there's often a set of data that you can _set_ and a set of data that you can _get_, and those collections are different. Usually the settable data is a subset of the gettable data. It would likely be a complete and total pain to separate these things out into groups, and I'm not even sure that the current annotation-driven method of generating the OpenAPI spec would play nicely with that.

[Vaalyn]

@SlvrEagle23 Oh that makes sense then. Do you know if we can override this behaviour without having to modify the normalizer?

I'm not familiar enough with the OpenAPI spec generation to really comment on that but as long as the example matches the response from the API it should be fine even if there is a difference in what you can set and what you can get. What I mean is if the example says it will return X but it returns nothing then this is a problem but if the get route returns a value that you can't set with the post/put route like for example the id and the api doc says you should set it in the json this is in my opinion a minor inconvenience that's not really a problem.

[SlvrEagle23]

@Vaalyn I already override the normalizer...I had to make my own completely from scratch, a DoctrineEntityNormalizer, that handles entity normalization for us (it's in AzuraCore).

Basically, the reason I was dropping nulls is because if no "getter" is available, I wanted to drop that value, so I just had it return null, which had the unfortunate side effect of removing null values altogether. This is clearly not desirable for API consistency. Working on fixing it right now.

[SlvrEagle23]

@Vaalyn There we go, much better!

[Vaalyn]

@SlvrEagle23 Thank you for the explanation and quick help with fixing this =)

[SlvrEagle23]

@Vaalyn I am noticing now that we're getting a bunch of null relations (i.e. station: null), let me get that taken care of too.

[SlvrEagle23]

@Vaalyn Fixed :D

[Vaalyn]

@SlvrEagle23 Thank you again for fixing this so fast =)

It's working correctly I think. This is what my local dev test installation now returns:

[
    {
        "id": 1,
        "song_id": "21c8d185274e109cf78656a95a0d99da",
        "title": "We Got This Together",
        "artist": "Daniel Ingram",
        "album": null,
        "lyrics": null,
        "isrc": null,
        "length": 213,
        "length_text": "3:33",
        "path": "01_-_we_got_this_together.mp3",
        "mtime": 1555812811,
        "fade_overlap": null,
        "fade_in": null,
        "fade_out": null,
        "cue_in": null,
        "cue_out": null,
        "playlists": [
            {
                "id": 1,
                "name": "default",
                "weight": 86
            }
        ],
        "unique_id": "31cb6f982ec1ccc4647da41e",
        "custom_fields": [],
        "links": {
            "self": "http://docker.local/api/station/1/file/1"
        }
    },
    ...
]

For now I'll close this issue as the main issue is resolved now but I'll keep an eye open for any further issues in this regard.

Something else that we might need to think about now is if / how we can indicate that some fields can return null as a value. In the entity description at the bottom of the Swagger page there is no mention of nullable or something like that.