Andrew-McGee/foam

hardcoded version string (v3 doesn't have json)

lachlan-00 opened this issue · 6 comments

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;

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.

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

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)

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+.

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.)

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 8d93145. 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.