-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ampache API improvements #1078
Merged
Merged
Ampache API improvements #1078
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
paulijar
force-pushed
the
feature/909_Ampache_API_improvements
branch
from
August 12, 2023 17:00
e165a8c
to
a8b3f62
Compare
paulijar
force-pushed
the
feature/909_Ampache_API_improvements
branch
2 times, most recently
from
September 3, 2023 10:10
5a3d3f5
to
673c0e5
Compare
paulijar
force-pushed
the
feature/909_Ampache_API_improvements
branch
from
September 3, 2023 14:58
a5e6a62
to
1b61a33
Compare
paulijar
force-pushed
the
feature/909_Ampache_API_improvements
branch
from
September 10, 2023 15:08
b752ccf
to
8b1a0c0
Compare
paulijar
force-pushed
the
feature/909_Ampache_API_improvements
branch
from
September 20, 2023 20:58
8b1a0c0
to
1efcaa5
Compare
The Last.fm connection must be set up on the server or the method will return only empty results.
- Most AmpacheAPI methods now return only an array which is then passed to the `ampacheResponse()` method commonly within the `dispatch()` method. This saves us a lot of redundant calls to `ampacheResponse()`. - Added type declarations to all method parameters and return values
…ndexes` The support is only partial as the optional arguments `filter`, `add`, and `update` may not be used together with this `type` value.
…bjects All the returned `artist` objects now contain the fields `time`, `albumcount`, and `songcount`. The returned `album` objects contain the new fields `time` and `songcount`. These had been added to the API spec in the version 4.4.0.
To match the API spec v4.4.0.
…sponse Also this has changed on the API spec v4.4.0. Obviously, the `handshake` fields are included on the `ping` response only in case the client has passed a valid session `auth` token as an argument.
For now, the API version information is not actually used for anything.
Got rid of the AmpacheUser utility class. This was used to share the information about the authorized user between AmpacheMiddleware and AmpacheController. Now we use the AmpacheSession for the same purpose. This has the added benefit of sharing the current token, too, and we no longer need to pass it around as an argument of dozens of functions. The AmpacheSession also carries the information about the requested API version which will come in handy very soon.
Move all the logic related to session management from AmpacheController to AmpacheMiddleware. This achieves better separation of concerns. The middleware was already previously handling part of the session management logic.
…s >4 In the APIv4, most JSON responses had an anonymous array as the root element. This has changed in APIv5 so that the root is now an object with a single named property and array as its value. This is more straight forward for us, and similar to how the Subsonic API has always worked.
Added actions `genres`, `genre`, `genre_artists`, `genre_albums`, `genre_songs`. These are APIv5 replacements for the former "tag" functions. We now provide then regardless of the API version. They are otherwise identical to the "tag" functions but the the result elements have the name "genre" instead of "tag". Also, the `handshake` result as well as any results containing songs, albums, or artists now use the key "genre" instead of "tag" when the active API version is 5 or greater.
The new error format is used when the requested API version in the session is greater than 4. The new format includes also a new property "errorType" which is now set as "system" on all errors.
For some reason, the Kodi Ampache plugin didn't work at all with the format "5.0.0" although that is the format used in the API spec for versions 5 and 6 and the plugin claims to use APIv6.
This is a difference between the APIv5 `genre_*` functions and the APIv4 `tag_*` functions. IMHO this is quite pointless, but that's how the API has been specified.
Added new API methods `bookmarks`, `get_bookmark`, `bookmark_create`, `bookmark_edit`, `bookmark_delete`. The API spec is currently missing any examples on these methods so the format of the results was reverse engineered from the Ampache source code. As a noteworthy exception, we don't support the optional argument `date` on the methods `bookmark_create` and `bookmark_edit`. Our Entity/Mapper/BusinessLayer architecture has been built upon the principle that the "create" and "update" timestamps get set automatically on the base class level, and overriding this would require some design changes which were not seen worthwhile.
The episode already contained both of these information in human-readable format with keys `filelenght` and `filesize` but the new fields contain the raw numeric value. This was assumed to be the purpose of these fields although that is not clearly stated in the API spec.
Added methods `live_streams` and `live_stream`.
…tream` The support for the type `playlist` had been added in the API spec v5.1.0, along with `podcast` and `podcast_episode`. The types `genre` and `live_stream` should not be supported according to any existing API spec version but this proprietary extension costs us nothing.
The APIv5 added a new group "searches" which refers to automatically generated playlists. The real Ampache server may have multiple such lists. As per APIv5, the `handshake` response now gives separately the counts for normal playlists, searches, and for these two combined. The Kodi Ampache plugin couldn't show the playlists on APIv5 without this `handshake` change. Also, the `playlists` method now has the optional argument `hide_search` which can be used to opt out of seeing the "All tracks" list. This change came already in the APIv4.3.0.
…array While using the JSON APIv5+, the responses of "singular" actions (like `song`, `artist`) are different from "plural" actions (like `songs`, `artists`). For singular actions, the responses are like: { id: "1", name: "Something", ... } For plural actions (even with just one result), the responses are like: { song: [ { id: "1", name: "Something", ... }, ... ] } With JSON APIv4, the responses of both singular and plural actions are like: [ { id: "1", name: "Something", ... }, ... ] In the XML API, there are no such differences.
…sts` This has the same limitations as the type `album_artist` in the method `get_indexes`: When the parameter `album_artist=1` is given, the parameters `add` and `update` are not supported.
Originally, this function returned results using the `get_indexes` format but this has been changed in Ampache v6.0.1, see ampache/ampache#3580.
These actions are now available and support a very limited set of hard-coded preference values which is just enough for the Ample client.
This is a new action introduced in APIv6, designed as a replacement for `get_indexes` which is now deprecated.
To get rid of the deprecation warnings from Scrutinizer. The class was deprecated in NC14 and removed completely in NC26 and that's the whole point of having this own copy. Our copy is alive and well and not going anywhere in foreseeable future.
The script hadn't been tested in its latest versions, as most testing has happened on an old NC version which still uses the obsolete database.xml.
This is a new action introduced by the APIv6.
It makes sense to utilize the previously introduces BusinessLayer function `findAllIdsAndNames` also here. This way, we don't need to construct complete Entity objects just to throw away most of their data. This makes a clearly measurable difference on the execution time. For example, the time to list ~2000 songs reduced from around two seconds to around or under one second.
The change concerns the actions `artists` and `get_indexes` when they are used to fetch album artists. Now both of these actions support all the possible arguments also in this case.
The syntax for regular expressions is different on PostgreSQL compared to MySQL/MariaDB. We now check the DB type from the config (i.e. from the contents of config.php) and variate the SQL queries accordingly. Note: Newer versions of SQLite should understand the same regex syntax as MySQL. But the gotcha is that SQLite is not shipped with a built-in regex engine and that should be somehow injected as an external function. That might be impossible for us as the SQLite driver is buried so deep under the NC/OC abstractions. Note2: The regex patterns supported on PostgreSQL and MySQL are not exactly the same although some basic things work the same.
These had got broken when the support for the arg `include` was added to the actions `artists` and `albums`. Those commits broke the compatibility of the function interfaces and it hadn't been taken into account that these functions are called explicitly from `genre_artists` and `genre_albums`.
The schema of the sessions table has changed.
paulijar
force-pushed
the
feature/909_Ampache_API_improvements
branch
from
October 1, 2023 19:34
1efcaa5
to
dc7c238
Compare
The SQLite DBMS has such a design flaw that notnull columns without default value cannot be added to existing tables even when the table in question is empty. To work around this, the NC table migration logic now drops the table music_ampache_sessions and recreates it from scratch.
The placeholder SQL rule `true` used when "exclude childless" not applicable for the table in question did not work on SQLite. Using `1=1` instead seems to work.
The method `findAllRated` had no overridden implementation in AlbumBusinessLayer; hence the "extra fields" were not injected to the results. Consequently, genre and year were missing from the Ampache results.
The inspection completed: 143 updated code elements |
This is now released as part of Music v1.9.1. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The purpose of this PR is to:
The scope of this PR ended up being larger than initially expected. Here's the final change log:
Added
rate
get_similar
genres
,genre
,genre_artists
,genre_albums
,genre_songs
bookmarks
,get_bookmark
,bookmark_create
,bookmark_edit
,bookmark_delete
live_streams
,live_stream
,live_stream_create
,live_stream_edit
,live_stream_delete
list
browse
user_preference
anduser_preferences
with mock-up contentadvanced_search
with partial support, not all search rules supported and some operators work only with MySQL/MariaDBalbum_artist
in the methodget_indexes
album_artist
in the methodartists
playlist
in the methodstats
playlist
in the methodsdownlaod
andstream
playlist
in the methodflag
top50
in the methodartist_songs
highest
in the methodstats
include
in the methodsalbum
,albums
,artist
, andartists
time
,albumcount
,songcount
,prefix
, andbasename
to theartist
type resultstime
,diskcount
,songcount
,prefix
, andbasename
to thealbum
type resultsdisk
,format
,stream_format
,stream_bitrate
,stream_mime
, andplaylisttrack
tosong
type resultstime
,size
,bitrate
,stream_bitrate
,rating
, andpreciserating
topodcast_episode
type resultsrating
andpreciserating
topodcast
type resultsflag
,rating
andpreciserating
toplaylist
type resultslanguage
,lyrics
,mode
,rate
,replaygain_album_gain
,replaygain_album_peak
,replaygain_track_gain
,replaygain_track_peak
,r128_album_gain
, andr128_track_gain
tosong
type resultsartists
tosong
andalbum
type resultshandshake
response on the response ofping
within a valid sessionChanged
handshake
handshake
music.ampache_api_default_ver
art
tag of the entities are now cache-friendly, i.e. don't depend on the sessionrating
andpreciserating
now show the user-given rating instead of constant 0 on all applicabale result objects