search

ampache / ampache

A web based audio/video streaming application and file manager allowing you to access your music & videos from anywhere, using almost any internet enabled device.
http://ampache.org
GNU Affero General Public License v3.0
3.55k stars 590 forks source link

Regression: Subsonic JSON API (see #1829) #2983

Closed darkphoenix closed 3 years ago

darkphoenix commented 3 years ago

It appears the issues I described in #1829 have resurfaced, presumably because my (admittedly not that pretty) fix from back then got edited out along the way - see https://develop.ampache.dev/rest/getSong.view?u=demo&p=demodemo&v=1.10.0&c=hi&id=1&f=json vs the correct http://demo.subsonic.org/rest/getSong.view?u=guest&p=guest&v=1.10.0&c=hi&id=216&f=json - there should not be an array there. I'm not sure if there is a more elegant way than explicitly passing whether or not to create single element arrays, as I did in PR #1830 at the time, but something definitely needs to be done as this appears to break at least some functionality in every client using the JSON version of the API, including at least the py-sonic library (and as a result, the ability to use Mopidy and a few other players that support Subsonic as a backend).

darkphoenix commented 3 years ago

After some further looking through the code, it appears the alwaysArray logic is still there - but the parameter pass for getSong, getArtist, and getAlbum specifically got stripped out in 7358c20, for some reason, so it breaks for those functions. I've only recently updated from my own branch to develop again, and I only now tried to use one of those clients again, hence why it presumably got through unnoticed at the time?

lachlan-00 commented 3 years ago

i see it in master too. the overrides are where it should stay so i'll revert that now

lachlan-00 commented 3 years ago

5e7cf1017 puts an empty array in as it doesn't need to check for any arrays. b2495848a fixes up getalbum and getartist as well

i'll do another check (i also didn't realise the demo for subsonic was available so that's going to help!)

lachlan-00 commented 3 years ago

demo and develop sites updated (added to master with a0e367f21)

darkphoenix commented 3 years ago

I see you reduced the arrays somewhat based on the actual response - that's a good bit neater than what it originally was, to be honest, I just took the global alwaysArray list and removed the respective items. Anyway, thanks for the quick response! And the demo site was really just a spontaneous idea, when I was testing I didn't remember the correct syntax and the official API spec page only had XML examples, so I picked a random song from the actual demo as an example :P

lachlan-00 commented 3 years ago

I figure if you aren't going to array anything we should reduce the foreach to the minimum.

i always use the subsonic api website where the examples are all xml so it'll be good to have that as a backup now.

Please try and break anything else you can in subsonic because that's a really important feature to maintain.

darkphoenix commented 3 years ago

I'll try my best :D

I'm back to another setup where I use Mopidy for music, which relies on py-sonic and therefore the JSON API - so I figure if more stuff breaks I'll stumble over it sooner or later. For now everything I've tried seems to work with this bit fixed, so I think this can be closed - if I find something else I'll come back with another issue.

lachlan-00 commented 3 years ago

that's good then, thanks for the detail on this and making it easy to fix.