search

Andrew-McGee / foam

A Fomantic UI styled web music player frontend for Ampache. Designed to be minimal, tidy and usable.
GNU General Public License v3.0
17 stars 2 forks source link

hardcoded version string (v3 doesn't have json) #14

Closed lachlan-00 closed 1 year ago

lachlan-00 commented 1 year ago

One thing i noticed. Calls using a v3 api string (350001) will force to v5. image

do you code against v5 of the api? might be better to set this to a 5.5.3 or 553000 to make sure you get the right responses

I found a few with a hardcoded v3 version

includes/playlistAPI.php

$url = $ampurl.'/server/json.server.php?action=handshake&auth='.$passphrase.'&timestamp='.$time.'&version=350001&user='.$ampusr;

includes/callAPI.php

$url = $ampurl.'/server/json.server.php?action=handshake&auth='.$passphrase.'&timestamp='.$time.'&version=350001&user='.$ampusr;

includes/favAPI.php

$url = $ampurl.'/server/json.server.php?action=handshake&auth='.$passphrase.'&timestamp='.$time.'&version=350001&user='.$ampusr;

includes/playlistfromqAPI.php

$url = $ampurl.'/server/json.server.php?action=handshake&auth='.$passphrase.'&timestamp='.$time.'&version=350001&user='.$ampusr;
Andrew-McGee commented 1 year ago

Good picklup - wow this is ancient and a bit of cut and pasting to blame here. I will look at moving this duplicated code into one shared function and switch to v5. It was probably written before 5 was released (or copied an example from docs). All the calls are in json so no sense even hard coding 3.

Andrew-McGee commented 1 year ago

Have now removed these hardcoded versions in the API calls. Unnecessary and straight up copied from the API docs here: https://ampache.org/api/api-5/#sending-handshake-request

lachlan-00 commented 1 year ago

Have now removed these hardcoded versions in the API calls. Unnecessary and straight up copied from the API docs here: https://ampache.org/api/api-5/#sending-handshake-request

awesome, i'll update the examples which are ALSO just copy pastes!

(website updated now too)

paulijar commented 1 year ago

So is the demo at http://foamdemo.mcgee.technology/ still using the unfixed version? Because I can see, that it makes handshake with the version 350001 but it works only if the server responses follow the APIv5 conventions. I'm developer of another Ampache-compatible server, and in my current development version, I was returning APIv5-compatible responses only if the version requested by the client is 5+.

lachlan-00 commented 1 year ago

that's the way it should be for 5.2+

when we supported one api version it didn't matter but after that release there is support for version based on handshake. ping can also change the version if needed. (no idea why you'd need to but it's possible.)

paulijar commented 1 year ago

Maybe I misunderstand your point @lachlan-00, but no, I don't think that this is the "way it should be". Your original point from the OP is valid and it's not good to ask for JSON API 3.x.x as that doesn't exist and then the client depends on unspecified operation. But http://foamdemo.mcgee.technology/ still does that, so presumably it's running the latest release v0.6.2-beta which doesn't contain the commit 8d931456da0128a850b3ef2563df23f884791ec0. On the other hand, https://demo.ampache.dev/foam/ apparently runs a newer development version as it doesn't specify any API version on handshake.

But I don't think that not specifying the API version in handshake is ideal, either. The foam client seems to work correctly only if the server responds using the API v6.x.x. It would be much more future proof to specify this required API version on handshake.