Fix tests and some small bugs

supysonic/api/__init__.py

@@ -17,7 +17,6 @@ from flask import Blueprint
 from pony.orm import ObjectNotFound
 from pony.orm import commit
 
-from ..db import Folder
 from ..managers.user import UserManager
 from ..py23 import dict
 
@@ -82,9 +81,11 @@ def get_client_prefs():
 
 
 def get_entity(cls, param="id"):
-    eid = get_entity_id(cls, request.values[param])
+    eid = request.values[param]
-    if eid is None:
+    if cls == Folder:
-        return
+        eid = int(eid)
+    else:
+        eid = uuid.UUID(eid)
     entity = cls[eid]
     return entity
 
@@ -95,11 +96,11 @@ def get_entity_id(cls, eid):
         try:
             return int(eid)
         except ValueError:
-            return None
+            raise GenericError("Invalid ID")
     try:
         return uuid.UUID(eid)
-    except ValueError:
+    except (AttributeError, ValueError):
-        return None
+        raise GenericError("Invalid ID")
 
 
 from .errors import *

supysonic/api/annotation.py

@@ -77,6 +77,11 @@ def handle_star_request(func):
             terr = e
         try:
             func(Folder, eid)
+        except GenericError as e:
+            if e.message == "Invalid ID" and isinstance(terr, ObjectNotFound):
+                ferr = NotFound("Folder not in database")
+            else:
+                ferr = e
         except Exception as e:
             ferr = e
 
@@ -115,35 +120,43 @@ def rate():
     id = request.values["id"]
     rating = request.values["rating"]
 
+    try:
         tid = get_entity_id(Track, id)
+    except GenericError:
+        tid = None
+    try:
         fid = get_entity_id(Folder, id)
+    except GenericError:
+        fid = None
     uid = None
     rating = int(rating)
 
+    if tid is None and fid is None:
+        raise GenericError("Invalid ID")
+
     if not 0 <= rating <= 5:
         raise GenericError("rating must be between 0 and 5 (inclusive)")
 
     if rating == 0:
+        if tid is not None:
             delete(
                 r for r in RatingTrack if r.user.id == request.user.id and r.rated.id == tid
             )
+        else:
             delete(
                 r
                 for r in RatingFolder
                 if r.user.id == request.user.id and r.rated.id == fid
             )
     else:
-        try:
+        if tid is not None:
             rated = Track[tid]
             rating_cls = RatingTrack
             uid = tid
-        except ObjectNotFound:
+        else:
-            try:
             rated = Folder[fid]
             rating_cls = RatingFolder
             uid = fid
-            except ObjectNotFound:
-                raise NotFound("Track or Folder")
 
         try:
             rating_info = rating_cls[request.user, uid]

supysonic/api/media.py

@@ -187,22 +187,33 @@ def stream_media():
 @api.route("/download.view", methods=["GET", "POST"])
 def download_media():
     id = request.values["id"]
-    uid = get_entity_id(Track, id)
-    fid = get_entity_id(Folder, id)
 
-    try:  # Track -> direct download
+    try:
+        uid = get_entity_id(Track, id)
+    except GenericError:
+        uid = None
+    try:
+        fid = get_entity_id(Folder, id)
+    except GenericError:
+        fid = None
+
+    if uid is None and fid is None:
+        raise GenericError("Invalid ID")
+
+    if uid is not None:
+        try:
             rv = Track[uid]
             return send_file(rv.path, mimetype=rv.mimetype, conditional=True)
-    except ObjectNotFound:
-        pass
-
-    try:  # Folder -> stream zipped tracks, non recursive
-        rv = Folder[fid]
         except ObjectNotFound:
             try:  # Album -> stream zipped tracks
                 rv = Album[uid]
             except ObjectNotFound:
-            raise NotFound("Track, Folder or Album")
+                raise NotFound("Track or Album")
+    else:
+        try:  # Folder -> stream zipped tracks, non recursive
+            rv = Folder[fid]
+        except ObjectNotFound:
+            raise NotFound("Folder")
 
     z = ZipFile(compression=ZIP_DEFLATED)
     for track in rv.tracks:
@@ -217,8 +228,17 @@ def cover_art():
     cache = current_app.cache
 
     eid = request.values["id"]
+    try:
         fid = get_entity_id(Folder, eid)
+    except GenericError:
+        fid = None
+    try:
         tid = get_entity_id(Track, eid)
+    except GenericError:
+        tid = None
+
+    if not fid and not tid:
+        raise GenericError("Invalid ID")
 
     if fid and Folder.exists(id=eid):
         res = get_entity(Folder)

tests/api/test_album_songs.py

@@ -118,7 +118,7 @@ class AlbumSongsTestCase(ApiTestBase):
         self._make_request("getRandomSongs", {"fromYear": "year"}, error=0)
         self._make_request("getRandomSongs", {"toYear": "year"}, error=0)
         self._make_request("getRandomSongs", {"musicFolderId": "idid"}, error=0)
-        self._make_request("getRandomSongs", {"musicFolderId": uuid.uuid4()}, error=70)
+        self._make_request("getRandomSongs", {"musicFolderId": 1234567890}, error=70)
 
         rv, child = self._make_request(
             "getRandomSongs", tag="randomSongs", skip_post=True

tests/api/test_annotation.py

@@ -27,6 +27,10 @@ class AnnotationTestCase(ApiTestBase):
             artist = Artist(name="Artist")
             album = Album(name="Album", artist=artist)
 
+            # Populate folder ids
+            root = Folder.get(name="Root")
+            folder = Folder.get(name="Folder")
+
             track = Track(
                 title="Track",
                 album=album,
@@ -73,7 +77,7 @@ class AnnotationTestCase(ApiTestBase):
             self.assertIn("starred", Folder[self.folderid].as_subsonic_child(self.user))
         self._make_request("star", {"id": str(self.folderid)}, error=0, skip_xsd=True)
 
-        self._make_request("star", {"albumId": str(self.folderid)}, error=70)
+        self._make_request("star", {"albumId": str(self.folderid)}, error=0)
         self._make_request("star", {"albumId": str(self.artistid)}, error=70)
         self._make_request("star", {"albumId": str(self.trackid)}, error=70)
         self._make_request("star", {"albumId": str(self.albumid)}, skip_post=True)
@@ -81,7 +85,7 @@ class AnnotationTestCase(ApiTestBase):
             self.assertIn("starred", Album[self.albumid].as_subsonic_album(self.user))
         self._make_request("star", {"albumId": str(self.albumid)}, error=0)
 
-        self._make_request("star", {"artistId": str(self.folderid)}, error=70)
+        self._make_request("star", {"artistId": str(self.folderid)}, error=0)
         self._make_request("star", {"artistId": str(self.albumid)}, error=70)
         self._make_request("star", {"artistId": str(self.trackid)}, error=70)
         self._make_request("star", {"artistId": str(self.artistid)}, skip_post=True)
@@ -213,7 +217,7 @@ class AnnotationTestCase(ApiTestBase):
         self._make_request("scrobble", error=10)
         self._make_request("scrobble", {"id": "song"}, error=0)
         self._make_request("scrobble", {"id": str(uuid.uuid4())}, error=70)
-        self._make_request("scrobble", {"id": str(self.folderid)}, error=70)
+        self._make_request("scrobble", {"id": str(self.folderid)}, error=0)
 
         self._make_request("scrobble", {"id": str(self.trackid)})
         self._make_request("scrobble", {"id": str(self.trackid), "submission": True})

tests/api/test_browse.py

@@ -82,7 +82,7 @@ class BrowseTestCase(ApiTestBase):
 
     def test_get_indexes(self):
         self._make_request("getIndexes", {"musicFolderId": "abcdef"}, error=0)
-        self._make_request("getIndexes", {"musicFolderId": str(uuid.uuid4())}, error=70)
+        self._make_request("getIndexes", {"musicFolderId": 1234567890}, error=70)
         self._make_request("getIndexes", {"ifModifiedSince": "quoi"}, error=0)
 
         rv, child = self._make_request(
@@ -109,7 +109,7 @@ class BrowseTestCase(ApiTestBase):
     def test_get_music_directory(self):
         self._make_request("getMusicDirectory", error=10)
         self._make_request("getMusicDirectory", {"id": "id"}, error=0)
-        self._make_request("getMusicDirectory", {"id": str(uuid.uuid4())}, error=70)
+        self._make_request("getMusicDirectory", {"id": 1234567890}, error=70)
 
         # should test with folders with both children folders and tracks. this code would break in that case
         with db_session:

tests/api/test_media.py

@@ -31,6 +31,7 @@ class MediaTestCase(ApiTestBase):
                 root=True,
                 cover_art="cover.jpg",
             )
+            folder = Folder.get(name="Root")
             self.folderid = folder.id
 
             artist = Artist(name="Artist")
@@ -74,7 +75,7 @@ class MediaTestCase(ApiTestBase):
         self._make_request("stream", error=10)
         self._make_request("stream", {"id": "string"}, error=0)
         self._make_request("stream", {"id": str(uuid.uuid4())}, error=70)
-        self._make_request("stream", {"id": str(self.folderid)}, error=70)
+        self._make_request("stream", {"id": str(self.folderid)}, error=0)
         self._make_request(
             "stream", {"id": str(self.trackid), "maxBitRate": "string"}, error=0
         )

tests/base/test_db.py

@@ -50,7 +50,12 @@ class DbTestCase(unittest.TestCase):
             parent=root_folder,
         )
 
-        return root_folder, child_folder, child_2
+        # Folder IDs don't get populated until we query the db.
+        return (
+            db.Folder.get(name="Root folder"),
+            db.Folder.get(name="Child folder"),
+            db.Folder.get(name="Child Folder (No Art)")
+        )
 
     def create_some_tracks(self, artist=None, album=None):
         root, child, child_2 = self.create_some_folders()
@@ -209,7 +214,7 @@ class DbTestCase(unittest.TestCase):
 
         root_folder, folder_art, folder_noart = self.create_some_folders()
         track1 = self.create_track_in(
-            root_folder, folder_noart, artist=artist, album=album
+            folder_noart, root_folder, artist=artist, album=album
         )
 
         album_dict = album.as_subsonic_album(user)

tests/frontend/test_folder.py

@@ -72,8 +72,8 @@ class FolderTestCase(FrontendTestBase):
 
         self._login("alice", "Alic3")
         rv = self.client.get("/folder/del/string", follow_redirects=True)
-        self.assertIn("badly formed", rv.data)
+        self.assertIn("Invalid folder id", rv.data)
-        rv = self.client.get("/folder/del/" + str(uuid.uuid4()), follow_redirects=True)
+        rv = self.client.get("/folder/del/1234567890", follow_redirects=True)
         self.assertIn("No such folder", rv.data)
         rv = self.client.get("/folder/del/" + str(folder.id), follow_redirects=True)
         self.assertIn("Music folders", rv.data)
@@ -87,8 +87,8 @@ class FolderTestCase(FrontendTestBase):
         self._login("alice", "Alic3")
 
         rv = self.client.get("/folder/scan/string", follow_redirects=True)
-        self.assertIn("badly formed", rv.data)
+        self.assertIn("Invalid folder id", rv.data)
-        rv = self.client.get("/folder/scan/" + str(uuid.uuid4()), follow_redirects=True)
+        rv = self.client.get("/folder/scan/1234567890", follow_redirects=True)
         self.assertIn("No such folder", rv.data)
         rv = self.client.get("/folder/scan/" + str(folder.id), follow_redirects=True)
         self.assertIn("start", rv.data)

tests/managers/test_manager_folder.py

@@ -76,7 +76,7 @@ class FolderManagerTestCase(unittest.TestCase):
         self.assertRaises(ValueError, FolderManager.get, 0xDEADBEEF)
 
         # Non-existent folder
-        self.assertRaises(ObjectNotFound, FolderManager.get, uuid.uuid4())
+        self.assertRaises(ObjectNotFound, FolderManager.get, 1234567890)
 
     @db_session
     def test_add_folder(self):
@@ -114,12 +114,12 @@ class FolderManagerTestCase(unittest.TestCase):
             self.create_folders()
 
         with db_session:
-            # Delete invalid UUID
+            # Delete invalid Folder ID
             self.assertRaises(ValueError, FolderManager.delete, "invalid-uuid")
             self.assertEqual(db.Folder.select().count(), 3)
 
             # Delete non-existent folder
-            self.assertRaises(ObjectNotFound, FolderManager.delete, uuid.uuid4())
+            self.assertRaises(ObjectNotFound, FolderManager.delete, 1234567890)
             self.assertEqual(db.Folder.select().count(), 3)
 
             # Delete non-root folder