Clarification about AlbumID3.artists

[deluan]

I just found what seems to be a conflict in OS spec: I'm adding artists to AlbumID3 in Navidrome, and the spec says:

Only the required ArtistID3 fields should be returned by default

And show this example:

    "artists": [
        {
            "id": "ar-1",
            "name": "Artist 1"
        },
        {
            "id": "ar-2",
            "name": "Artist 2"
        }
    ],

But at the same time, ArtistID3 has some OS fields, musicBrainzId, sortName, roles. And their specification says:

All OpenSubsonic added fields are optionals. But if a server support a field it must return it with an empty / default value when not present in it’s database so that clients knows what the server supports.

This causes the AlbumID3.artists to be marshaled as:

"artists": [
    {
        "id": "4Y4arYckrgDabKQyjrFYxh",
        "name": "TKKG",
        "musicBrainzId": "",
        "sortName": "",
        "roles": []
    }
]

with empty OS fields, conflicting with the AlbumID3 spec.

Am I missing something? Maybe the text is not clear (at least for me)

[kgarner7]

I think that's just a problem with the example. The spec should be have mbid, sort name, and roles.

[epoupon]

Hmm for me this is just a more compact artist ref (id/name) to list the album artists. Otherwise we have to include/repeat a lot of info.
That is how it is currently implemented in lms (you can try the demo instance)

[Tolriq]

Well we can drop XML for the new endpoints and have the OpenAPI only cover JSON.

That would not impact legacy clients at all since they would not use the OpenAPI, and if they want to migrate to OS they can switch to JSON.
Specially if the new endpoint only support JSON POST for complex endpoints parameters.

WDYT?

[deluan]

Not sure the value of having a spec that does not cover everything. Covering only JSON would not be useful for new servers wanting to use it to generate code.

Well we can drop XML for the new endpoints and have the OpenAPI only cover JSON.

If that means Using OpenAPI only for new endpoints could be confusing: It would force clients (and servers) to have multiple ways to handle the same API, if. they want to leverage code generation based on OpenAPI.

@jeffvli's client will probably generate a JSON only spec, it may be useful for clients, but servers will not be able to fully leverage it, if they want to serve a broader set of clients

I don't think there is a simple solution for this, that's why I stop pursuing it.

[Tolriq]

IMO I do not think an existing server would rewrite all it's code to migrate to OpenAPI auto generated code, but maybe I'm wrong on that one.

Any new server written from scratch, would probably not bother supporting older clients, when Navidrome will embrace multiple artists, people will want that even more.
There's more and more new clients on all platforms, I'm not sure it's worth to block new cool things for a couple of legacy clients that will disappear with time.

On Android, Google remove apps that do not update the target SDK, same on iOS (but later) and change rules.
This will leave very few users installing APK (not possible on iOS).

TL;DR; let's not block the future if we can simplify every one life for a wrong reason.

[deluan]

would probably not bother supporting older clients,

Yes, for DSub, but:
Supersonic is not an old client. c/c @dweymouth
play:Sub is actively maintained. c/c @MichaelBechHansen

I do not think an existing server would rewrite all it's code to migrate to OpenAPI auto generated code, but maybe I'm wrong on that one.

Rewriting Subsonic support was the reason I started working on the OpenAPI spec... :)

Anyway, this is my 2c. Would love to hear others opinions on this.

[Tolriq]

Supersonic support Jellyfin so probably can migrate to Json in a couple of hours.

No idea about the iOS one :(

Rewriting Subsonic support was the reason I started working on the OpenAPI spec... :)

And it's also the reason you could not finish ;)

Anyway to resume we both agree that a spec that does not cover everything is not really useful, so the compromise to have a full OpenAPI spec would be to only be JSON it seems.

If that's not a valid compromise, then I guess we keep JsonSchema for the new api endpoints at least for the documentation purpose if not the full auto client generation.

[deluan]

While we are at this, another thing that may need clarification. Child can be used for songs AND albums (and even artists!). So this section needs some better wording:

Field	Type	Req.	OpenS.	Details
artists	Array of ArtistID3	No	Yes	The list of all song artists of the song. (Note: Only the required ArtistID3 fields should be returned by default)
displayArtist	string	No	Yes	The single value display artist.
albumArtists	Array of ArtistID3	No	Yes	The list of all album artists of the song. (Note: Only the required ArtistID3 fields should be returned by default)
displayAlbumArtist	string	No	Yes	The single value display album artist.
contributors	Array of Contributor	No	Yes	The list of all contributor artists of the song.

And when the Child is an album, artists would be a collection of track artists found in the album or should it be album artists and we remove the albumArtists field?

displayArtist does not make sense for album in this context (as it is a track artist), so are we going to add it as an empty field in this case? Or it would contain the value of DisplayAlbumArtist, in which case we don't need to duplicate it in displayAlbumArtist, right?

[kgarner7]

But LMS already does this. Is @epoupon willing to change? I'm ok with both approaches.

No problem to change!

@kgarner7 can you revise this in your "errata" PR, please?

If displayAlbumArtist / albumArtists are only sometimes populated (depending on the use), then it seems more correct to split up Child (at least for OS) into proper types (e.g. AlbumChild, PodcastChild, SongChild), rather than say "these fields are only required for one variant of the use".

I'm fine noting that these fields are not in fact required (and depend on use), but that seems more like a kludge.

[kgarner7]

Actually, on a related note, since PodcastEpisode extends child, which of the OS attributes are actually relevant? artists? albumArtists? I could see an argument for the other OS fields (with displayComposer maybe having a different meaning)

[Tolriq]

If displayAlbumArtist / albumArtists are only sometimes populated (depending on the use), then it seems more correct to split up Child (at least for OS) into proper types (e.g. AlbumChild, PodcastChild, SongChild), rather than say "these fields are only required for one variant of the use".

This is not possible for now as it would be polymorphism again, you can't really know what will come back before it comes back.

But things like Child will disappear in the new endpoints.

The fields are already not required, and it's actually the other ones that are sometimes not filled. Doc should just mention that displayArtist is for songs as is artists

[epoupon]

We also have mediaType to know if it is meant to be a song or an album. And we already have fields like musicBrainzId that have different meaning depending on the type.

[kgarner7]

1. mediaType doesn't necessarily help with podcasts (you would also presumably want type)
2. Is song/album/artist the only possible value for mediaType (no podcast/similar)? If so, then this should probably be represented as an enum, rather than a string.
3. If podcast is not a mediaType, then I would argue that type should also be required for OS servers.
4. None of the above really answers which of artist/album artist fields are relevant for podcasts.