fixes

Aerozol · May 28, 2024 · d7ff5ae · d7ff5ae
1 parent 0a26854
commit d7ff5ae
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 30 deletions.
diff --git a/frontend/js/src/user/year-in-music/2021/YearInMusic2021.tsx b/frontend/js/src/user/year-in-music/2021/YearInMusic2021.tsx
@@ -144,7 +144,13 @@ export default class YearInMusic extends React.Component<
       playlist.jspf.playlist.track = playlist.jspf.playlist.track.map(
         (track: JSPFTrack) => {
           const newTrack = { ...track };
-          const track_id = track.identifier;
+          let track_id;
+          if (Array.isArray(track.identifier)) {
+            // eslint-disable-next-line prefer-destructuring
+            track_id = track.identifier[0];
+          } else {
+            track_id = track.identifier;
+          }
           const found = track_id.match(uuidMatch);
           if (found) {
             const recording_mbid = found[0];

diff --git a/frontend/js/src/user/year-in-music/2022/YearInMusic2022.tsx b/frontend/js/src/user/year-in-music/2022/YearInMusic2022.tsx
@@ -151,7 +151,13 @@ export default class YearInMusic extends React.Component<
       /* Add a track image if it exists in the `{playlistName}-coverart` key */
       playlist.track = playlist.track.map((track: JSPFTrack) => {
         const newTrack = { ...track };
-        const track_id = track.identifier;
+        let track_id;
+        if (Array.isArray(track.identifier)) {
+          // eslint-disable-next-line prefer-destructuring
+          track_id = track.identifier[0];
+        } else {
+          track_id = track.identifier;
+        }
         const found = track_id.match(uuidMatch);
         if (found) {
           const recording_mbid = found[0];

diff --git a/listenbrainz/tests/integration/test_playlist_api.py b/listenbrainz/tests/integration/test_playlist_api.py
@@ -31,7 +31,7 @@ def get_test_data():
             },
             "track": [
                 {
-                    "identifier": "https://musicbrainz.org/recording/e8f9b188-f819-4e43-ab0f-4bd26ce9ff56"
+                    "identifier": ["https://musicbrainz.org/recording/e8f9b188-f819-4e43-ab0f-4bd26ce9ff56"]
                 }
             ],
         }
@@ -362,8 +362,11 @@ def test_playlist_json_with_missing_keys(self):
             headers={"Authorization": "Token {}".format(self.user["auth_token"])}
         )
         self.assert400(response)
-        self.assertEqual(response.json["error"],
-                         "JSPF playlist track 0 identifier must have the namespace 'https://musicbrainz.org/recording/' prepended to it.")
+        self.assertEqual(
+            response.json["error"],
+            "JSPF playlist track 0 must contain a identifier field having a fully qualified URI to a"
+            " recording_mbid. (e.g. https://musicbrainz.org/recording/8f3471b5-7e6a-48da-86a9-c1c07a0f47ae)"
+        )
 
     def test_playlist_create_with_collaborators(self):
         """ Test to ensure creating a playlist with collaborators works """
@@ -487,7 +490,7 @@ def test_playlist_recording_add(self):
             "playlist": {
                 "track": [
                     {
-                        "identifier": PLAYLIST_TRACK_URI_PREFIX + "4a77a078-e91a-4522-a409-3b58aa7de3ae"
+                        "identifier": [PLAYLIST_TRACK_URI_PREFIX + "4a77a078-e91a-4522-a409-3b58aa7de3ae"]
                     }
                 ],
                 "extension": {
@@ -539,10 +542,10 @@ def test_playlist_recording_move(self):
                 "title": "1980s flashback jams",
                 "track": [
                     {
-                        "identifier": PLAYLIST_TRACK_URI_PREFIX + "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56"
+                        "identifier": [PLAYLIST_TRACK_URI_PREFIX + "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56"]
                     },
                     {
-                        "identifier": PLAYLIST_TRACK_URI_PREFIX + "57ef4803-5181-4b3d-8dd6-8b9d9ca83e2a"
+                        "identifier": [PLAYLIST_TRACK_URI_PREFIX + "57ef4803-5181-4b3d-8dd6-8b9d9ca83e2a"]
                     }
                 ],
                 "extension": {
@@ -586,10 +589,10 @@ def test_playlist_recording_delete(self):
                 "title": "1980s flashback jams",
                 "track": [
                     {
-                        "identifier": PLAYLIST_TRACK_URI_PREFIX + "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56"
+                        "identifier": [PLAYLIST_TRACK_URI_PREFIX + "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56"]
                     },
                     {
-                        "identifier": PLAYLIST_TRACK_URI_PREFIX + "57ef4803-5181-4b3d-8dd6-8b9d9ca83e2a"
+                        "identifier": [PLAYLIST_TRACK_URI_PREFIX + "57ef4803-5181-4b3d-8dd6-8b9d9ca83e2a"]
                     }
                 ],
                 "extension": {

diff --git a/listenbrainz/webserver/views/playlist_api.py b/listenbrainz/webserver/views/playlist_api.py
@@ -44,6 +44,22 @@ def validate_create_playlist_required_items(jspf):
         log_raise_400("JSPF playlist.extension.https://musicbrainz.org/doc/jspf#playlist.public field must be given.")
 
 
+def get_track_recording_mbid(track):
+    recording_uris = track.get("identifier", [])
+    # This allows identifier to be a list, tuple or string. The string support is a leftover and should
+    # be removed after 2025-06, which marks a year or backward compatibility.
+    if isinstance(recording_uris, str):
+        recording_uris = [recording_uris]
+
+    for recording_uri in recording_uris:
+        if recording_uri.startswith(PLAYLIST_TRACK_URI_PREFIX):
+            recording_mbid = recording_uri[len(PLAYLIST_TRACK_URI_PREFIX):]
+            if is_valid_uuid(recording_mbid):
+                return recording_mbid
+
+    return None
+
+
 def validate_playlist(jspf):
     """
         Given a JSPF dict, ensure that title is present and that if tracks are present
@@ -80,22 +96,8 @@ def validate_playlist(jspf):
     )
 
     for i, track in enumerate(jspf["playlist"].get("track", [])):
-        recording_uris = track.get("identifier")
-
-        if not recording_uris:
-            log_raise_400(recording_uri_error % i)
-
-        # This allows identifier to be a list, tuple or string. The string support is a leftover and should
-        # be removed after 2025-06, which marks a year or backward compatibility.
-        if isinstance(recording_uris, str):
-            recording_uris = [recording_uris]
-
-        recording_mbid = None
-        for recording_uri in recording_uris:
-            if recording_uri.startswith(PLAYLIST_TRACK_URI_PREFIX):
-                recording_mbid = recording_uri[len(PLAYLIST_TRACK_URI_PREFIX):]
-
-        if recording_mbid is None or not is_valid_uuid(recording_mbid):
+        recording_mbid = get_track_recording_mbid(track)
+        if not recording_mbid:
             log_raise_400(recording_uri_error % i)
 
 
@@ -615,11 +617,10 @@ def add_playlist_item(playlist_mbid, offset):
     precordings = []
     if "track" in data["playlist"]:
         for track in data["playlist"]["track"]:
-            try:
-                mbid = UUID(track['identifier'][len(PLAYLIST_TRACK_URI_PREFIX):])
-            except (KeyError, ValueError):
+            recording_mbid = get_track_recording_mbid(track)
+            if not recording_mbid:
                 log_raise_400("Track %d has an invalid identifier field, it must be a complete URI.")
-            precordings.append(WritablePlaylistRecording(mbid=mbid, added_by_id=user["id"]))
+            precordings.append(WritablePlaylistRecording(mbid=UUID(recording_mbid), added_by_id=user["id"]))
 
     try:
         db_playlist.add_recordings_to_playlist(db_conn, ts_conn, playlist, precordings, offset)