Refactored star/unstar error handling

supysonic/api/annotation.py

@@ -18,6 +18,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import sys
 import time
 import uuid
 
@@ -32,110 +33,85 @@ from ..lastfm import LastFm
 from ..py23 import dict
 
 from . import api, get_entity
-from .exceptions import GenericError, MissingParameter, NotFound
+from .exceptions import AggregateException, GenericError, MissingParameter, NotFound
 
-def try_star(cls, starred_cls, eid):
+def star_single(cls, eid):
     """ Stars an entity
 
     :param cls: entity class, Folder, Artist, Album or Track
-    :param starred_cls: class used for the db representation of the starring of ent
     :param eid: id of the entity to star
-    :return error dict, if any. None otherwise
     """
 
-    try:
     uid = uuid.UUID(eid)
     e = cls[uid]
-    except ValueError:
-        return dict(code = 0, message = 'Invalid {} id {}'.format(cls.__name__, eid))
-    except ObjectNotFound:
-        return dict(code = 70, message = 'Unknown {} id {}'.format(cls.__name__, eid))
 
+    starred_cls = getattr(sys.modules[__name__], 'Starred' + cls.__name__)
     try:
         starred_cls[request.user, uid]
-        return dict(code = 0, message = '{} {} already starred'.format(cls.__name__, eid))
+        raise GenericError('{} {} already starred'.format(cls.__name__, eid))
     except ObjectNotFound:
         pass
 
     starred_cls(user = request.user, starred = e)
-    return None
 
-def try_unstar(starred_cls, eid):
+def unstar_single(cls, eid):
     """ Unstars an entity
 
-    :param starred_cls: class used for the db representation of the starring of the entity
+    :param cls: entity class, Folder, Artist, Album or Track
     :param eid: id of the entity to unstar
-    :return error dict, if any. None otherwise
     """
 
-    try:
     uid = uuid.UUID(eid)
-    except ValueError:
+    starred_cls = getattr(sys.modules[__name__], 'Starred' + cls.__name__)
-        return dict(code = 0, message = 'Invalid id {}'.format(eid))
-
     delete(s for s in starred_cls if s.user.id == request.user.id and s.starred.id == uid)
     return None
 
-def merge_errors(errors):
+def handle_star_request(func):
-    error = None
+    id, albumId, artistId = map(request.values.getlist, [ 'id', 'albumId', 'artistId' ])
-    errors = [ e for e in errors if e ]
-    if len(errors) == 1:
-        error = errors[0]
-    elif len(errors) > 1:
-        codes = set(map(lambda e: e['code'], errors))
-        error = dict(code = list(codes)[0] if len(codes) == 1 else 0, error = errors)
 
-    return error
+    if not id and not albumId and not artistId:
+        raise MissingParameter('id, albumId or artistId')
+
+    errors = []
+    for eid in id:
+        terr = None
+        ferr = None
+
+        try:
+            func(Track, eid)
+        except Exception as e:
+            terr = e
+        try:
+            func(Folder, eid)
+        except Exception as e:
+            ferr = e
+
+        if terr and ferr:
+            errors += [ terr, ferr ]
+
+    for alId in albumId:
+        try:
+            func(Album, alId)
+        except Exception as e:
+            errors.append(e)
+
+    for arId in artistId:
+        try:
+            func(Artist, arId)
+        except Exception as e:
+            errors.append(e)
+
+    if errors:
+        raise AggregateException(errors)
+    return request.formatter.empty
 
 @api.route('/star.view', methods = [ 'GET', 'POST' ])
 def star():
-    id, albumId, artistId = map(request.values.getlist, [ 'id', 'albumId', 'artistId' ])
+    return handle_star_request(star_single)
-
-    if not id and not albumId and not artistId:
-        raise MissingParameter('id, albumId or artistId')
-
-    errors = []
-    for eid in id:
-        terr = try_star(Track, StarredTrack, eid)
-        ferr = try_star(Folder, StarredFolder, eid)
-        if terr and ferr:
-            errors += [ terr, ferr ]
-
-    for alId in albumId:
-        errors.append(try_star(Album, StarredAlbum, alId))
-
-    for arId in artistId:
-        errors.append(try_star(Artist, StarredArtist, arId))
-
-    error = merge_errors(errors)
-    if error:
-        return request.formatter('error', error)
-    return request.formatter.empty
 
 @api.route('/unstar.view', methods = [ 'GET', 'POST' ])
 def unstar():
-    id, albumId, artistId = map(request.values.getlist, [ 'id', 'albumId', 'artistId' ])
+    return handle_star_request(unstar_single)
-
-    if not id and not albumId and not artistId:
-        raise MissingParameter('id, albumId or artistId')
-
-    errors = []
-    for eid in id:
-        terr = try_unstar(StarredTrack, eid)
-        ferr = try_unstar(StarredFolder, eid)
-        if terr and ferr:
-            errors += [ terr, ferr ]
-
-    for alId in albumId:
-        errors.append(try_unstar(StarredAlbum, alId))
-
-    for arId in artistId:
-        errors.append(try_unstar(StarredArtist, arId))
-
-    error = merge_errors(errors)
-    if error:
-        return request.formatter('error', error)
-    return request.formatter.empty
 
 @api.route('/setRating.view', methods = [ 'GET', 'POST' ])
 def rate():

supysonic/api/exceptions.py

@@ -7,10 +7,10 @@
 #
 # Distributed under terms of the GNU AGPLv3 license.
 
-from flask import request
+from flask import current_app, request
 from werkzeug.exceptions import HTTPException
 
-class SubsonicAPIError(HTTPException):
+class SubsonicAPIException(HTTPException):
     code = 400
     api_code = None
     message = None
@@ -24,7 +24,7 @@ class SubsonicAPIError(HTTPException):
         code = self.api_code if self.api_code is not None else '??'
         return '{}: {}'.format(code, self.message)
 
-class GenericError(SubsonicAPIError):
+class GenericError(SubsonicAPIException):
     api_code = 0
 
     def __init__(self, message, *args, **kwargs):
@@ -34,40 +34,40 @@ class GenericError(SubsonicAPIError):
 class ServerError(GenericError):
     code = 500
 
-class MissingParameter(SubsonicAPIError):
+class MissingParameter(SubsonicAPIException):
     api_code = 10
 
     def __init__(self, param, *args, **kwargs):
         super(MissingParameter, self).__init__(*args, **kwargs)
         self.message = "Required parameter '{}' is missing.".format(param)
 
-class ClientMustUpgrade(SubsonicAPIError):
+class ClientMustUpgrade(SubsonicAPIException):
     api_code = 20
     message = 'Incompatible Subsonic REST protocol version. Client must upgrade.'
 
-class ServerMustUpgrade(SubsonicAPIError):
+class ServerMustUpgrade(SubsonicAPIException):
     code = 501
     api_code = 30
     message = 'Incompatible Subsonic REST protocol version. Server must upgrade.'
 
-class Unauthorized(SubsonicAPIError):
+class Unauthorized(SubsonicAPIException):
     code = 401
     api_code = 40
     message = 'Wrong username or password.'
 
-class Forbidden(SubsonicAPIError):
+class Forbidden(SubsonicAPIException):
     code = 403
     api_code = 50
     message = 'User is not authorized for the given operation.'
 
-class TrialExpired(SubsonicAPIError):
+class TrialExpired(SubsonicAPIException):
     code = 402
     api_code = 60
     message = ("The trial period for the Supysonic server is over."
         "But since it doesn't use any licensing you shouldn't be seeing this error ever."
         "So something went wrong or you got scammed.")
 
-class NotFound(SubsonicAPIError):
+class NotFound(SubsonicAPIException):
     code = 404
     api_code = 70
 
@@ -75,3 +75,30 @@ class NotFound(SubsonicAPIError):
         super(NotFound, self).__init__(*args, **kwargs)
         self.message = '{} not found'.format(entity)
 
+class AggregateException(SubsonicAPIException):
+    def __init__(self, exceptions, *args, **kwargs):
+        super(AggregateException, self).__init__(*args, **kwargs)
+
+        self.exceptions = []
+        for exc in exceptions:
+            if not isinstance(exc, SubsonicAPIException):
+                # Try to convert regular exceptions to SubsonicAPIExceptions
+                handler = current_app._find_error_handler(exc) # meh
+                if handler:
+                    exc = handler(exc)
+                    assert isinstance(exc, SubsonicAPIException)
+                else:
+                    exc = GenericError(str(exc))
+            self.exceptions.append(exc)
+
+    def get_response(self, environ = None):
+        if len(self.exceptions) == 1:
+            return self.exceptions[0].get_response()
+
+        codes = set(exc.api_code for exc in self.exceptions)
+        errors = [ dict(code = exc.api_code, message = exc.message) for exc in self.exceptions ]
+
+        rv = request.formatter('error', dict(code = list(codes)[0] if len(codes) == 1 else 0, error = errors))
+        rv.status_code = self.code
+        return rv
+