From cc838f14d3376506d054623b260d3693300ecc00 Mon Sep 17 00:00:00 2001 From: mvn23 Date: Tue, 1 Oct 2019 16:55:23 +0200 Subject: [PATCH] Fix tests and some small bugs --- supysonic/api/__init__.py | 15 ++++---- supysonic/api/annotation.py | 49 ++++++++++++++++---------- supysonic/api/media.py | 50 +++++++++++++++++++-------- tests/api/test_album_songs.py | 2 +- tests/api/test_annotation.py | 10 ++++-- tests/api/test_browse.py | 4 +-- tests/api/test_media.py | 3 +- tests/base/test_db.py | 9 +++-- tests/frontend/test_folder.py | 8 ++--- tests/managers/test_manager_folder.py | 6 ++-- 10 files changed, 100 insertions(+), 56 deletions(-) diff --git a/supysonic/api/__init__.py b/supysonic/api/__init__.py index ecc419c..0b35035 100644 --- a/supysonic/api/__init__.py +++ b/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]) - if eid is None: - return + eid = request.values[param] + if cls == Folder: + 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: - return None + except (AttributeError, ValueError): + raise GenericError("Invalid ID") from .errors import * diff --git a/supysonic/api/annotation.py b/supysonic/api/annotation.py index 27d777f..457383a 100644 --- a/supysonic/api/annotation.py +++ b/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"] - tid = get_entity_id(Track, id) - fid = get_entity_id(Folder, id) + 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: - delete( - r for r in RatingTrack if r.user.id == request.user.id and r.rated.id == tid - ) - delete( - r - for r in RatingFolder - if r.user.id == request.user.id and r.rated.id == fid - ) + 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: - try: - rated = Folder[fid] - rating_cls = RatingFolder - uid = fid - except ObjectNotFound: - raise NotFound("Track or Folder") + else: + rated = Folder[fid] + rating_cls = RatingFolder + uid = fid try: rating_info = rating_cls[request.user, uid] diff --git a/supysonic/api/media.py b/supysonic/api/media.py index 45a1639..26792ab 100644 --- a/supysonic/api/media.py +++ b/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 - rv = Track[uid] - return send_file(rv.path, mimetype=rv.mimetype, conditional=True) - except ObjectNotFound: - pass + try: + uid = get_entity_id(Track, id) + except GenericError: + uid = None + try: + fid = get_entity_id(Folder, id) + except GenericError: + fid = None - try: # Folder -> stream zipped tracks, non recursive - rv = Folder[fid] - except ObjectNotFound: - try: # Album -> stream zipped tracks - rv = Album[uid] + 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: - raise NotFound("Track, Folder or Album") + try: # Album -> stream zipped tracks + rv = Album[uid] + except ObjectNotFound: + 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"] - fid = get_entity_id(Folder, eid) - tid = get_entity_id(Track, eid) + 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) diff --git a/tests/api/test_album_songs.py b/tests/api/test_album_songs.py index 10fefbc..3d86c81 100644 --- a/tests/api/test_album_songs.py +++ b/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 diff --git a/tests/api/test_annotation.py b/tests/api/test_annotation.py index ef2b732..da4bc74 100644 --- a/tests/api/test_annotation.py +++ b/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}) diff --git a/tests/api/test_browse.py b/tests/api/test_browse.py index 6d40bd8..395ead1 100644 --- a/tests/api/test_browse.py +++ b/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: diff --git a/tests/api/test_media.py b/tests/api/test_media.py index 42c14e3..b24f6ad 100644 --- a/tests/api/test_media.py +++ b/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 ) diff --git a/tests/base/test_db.py b/tests/base/test_db.py index 5251081..54768da 100644 --- a/tests/base/test_db.py +++ b/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) diff --git a/tests/frontend/test_folder.py b/tests/frontend/test_folder.py index c9f3221..c0ca1ea 100644 --- a/tests/frontend/test_folder.py +++ b/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) - rv = self.client.get("/folder/del/" + str(uuid.uuid4()), follow_redirects=True) + self.assertIn("Invalid folder id", rv.data) + 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) - rv = self.client.get("/folder/scan/" + str(uuid.uuid4()), follow_redirects=True) + self.assertIn("Invalid folder id", rv.data) + 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) diff --git a/tests/managers/test_manager_folder.py b/tests/managers/test_manager_folder.py index 84e9dd1..27a3b2e 100644 --- a/tests/managers/test_manager_folder.py +++ b/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