From ef9e7af026ba5da863a5e36b8ab1707586fb6cee Mon Sep 17 00:00:00 2001 From: spl0k Date: Fri, 2 Mar 2018 22:51:49 +0100 Subject: [PATCH] Refactored UserManager to raise exceptions rather than returning status codes --- supysonic/api/__init__.py | 8 +-- supysonic/api/errors.py | 7 +-- supysonic/api/user.py | 19 ++----- supysonic/cli.py | 53 +++++++++--------- supysonic/frontend/__init__.py | 9 ++-- supysonic/frontend/folder.py | 1 - supysonic/frontend/playlist.py | 1 - supysonic/frontend/user.py | 74 +++++++++++++++---------- supysonic/managers/user.py | 83 +++++++---------------------- tests/frontend/test_login.py | 4 +- tests/frontend/test_user.py | 16 +++--- tests/managers/test_manager_user.py | 69 +++++++++++------------- tests/testbase.py | 7 ++- 13 files changed, 152 insertions(+), 199 deletions(-) diff --git a/supysonic/api/__init__.py b/supysonic/api/__init__.py index 8b78ac4..9bb822e 100644 --- a/supysonic/api/__init__.py +++ b/supysonic/api/__init__.py @@ -58,8 +58,8 @@ def decode_password(password): @api.before_request def authorize(): if request.authorization: - status, user = UserManager.try_auth(request.authorization.username, request.authorization.password) - if status == UserManager.SUCCESS: + user = UserManager.try_auth(request.authorization.username, request.authorization.password) + if user is not None: request.user = user return raise Unauthorized() @@ -68,8 +68,8 @@ def authorize(): password = request.values['p'] password = decode_password(password) - status, user = UserManager.try_auth(username, password) - if status != UserManager.SUCCESS: + user = UserManager.try_auth(username, password) + if user is None: raise Unauthorized() request.user = user diff --git a/supysonic/api/errors.py b/supysonic/api/errors.py index e60236f..7eef4f2 100644 --- a/supysonic/api/errors.py +++ b/supysonic/api/errors.py @@ -30,13 +30,10 @@ def not_found(e): rollback() return NotFound(e.entity.__name__) -@api.errorhandler(Exception) +@api.errorhandler(500) def generic_error(e): # pragma: nocover rollback() - if not current_app.testing: - return ServerError(e) - else: - raise e + return ServerError(e) #@api.errorhandler(404) @api.route('/', methods = [ 'GET', 'POST' ]) # blueprint 404 workaround diff --git a/supysonic/api/user.py b/supysonic/api/user.py index 6fff192..8b5837a 100644 --- a/supysonic/api/user.py +++ b/supysonic/api/user.py @@ -64,9 +64,7 @@ def user_add(): admin = True if admin in (True, 'True', 'true', 1, '1') else False password = decode_password(password) - status = UserManager.add(username, password, email, admin) - if status == UserManager.NAME_EXISTS: - raise GenericError('There is already a user with that username') + UserManager.add(username, password, email, admin) return request.formatter.empty @@ -74,14 +72,7 @@ def user_add(): @admin_only def user_del(): username = request.values['username'] - - user = User.get(name = username) - if user is None: - raise NotFound('User') - - status = UserManager.delete(user.id) - if status != UserManager.SUCCESS: - raise GenericError(UserManager.error_str(status)) + UserManager.delete_by_name(username) return request.formatter.empty @@ -94,11 +85,7 @@ def user_changepass(): raise Forbidden() password = decode_password(password) - status = UserManager.change_password2(username, password) - if status == UserManager.NO_SUCH_USER: - raise NotFound('User') - elif status != UserManager.SUCCESS: - raise GenericError(UserManager.error_str(status)) + UserManager.change_password2(username, password) return request.formatter.empty diff --git a/supysonic/cli.py b/supysonic/cli.py index a204f4e..05f3282 100755 --- a/supysonic/cli.py +++ b/supysonic/cli.py @@ -26,6 +26,7 @@ import sys import time from pony.orm import db_session +from pony.orm import ObjectNotFound from .db import Folder, User from .managers.folder import FolderManager @@ -122,6 +123,8 @@ class SupysonicCLI(cmd.Cmd): self.write_line('Unknown command %s' % line.split()[0]) self.do_help(None) + onecmd = db_session(cmd.Cmd.onecmd) + def postloop(self): self.write_line() @@ -148,7 +151,6 @@ class SupysonicCLI(cmd.Cmd): folder_scan_parser.add_argument('folders', metavar = 'folder', nargs = '*', help = 'Folder(s) to be scanned. If ommitted, all folders are scanned') folder_scan_parser.add_argument('-f', '--force', action = 'store_true', help = "Force scan of already know files even if they haven't changed") - @db_session def folder_list(self): self.write_line('Name\t\tPath\n----\t\t----') self.write_line('\n'.join('{0: <16}{1}'.format(f.name, f.path) for f in Folder.select(lambda f: f.root))) @@ -167,7 +169,6 @@ class SupysonicCLI(cmd.Cmd): else: self.write_line("Deleted folder '{}'".format(name)) - @db_session def folder_scan(self, folders, force): extensions = self.__config.BASE['scanner_extensions'] if extensions: @@ -217,30 +218,32 @@ class SupysonicCLI(cmd.Cmd): user_pass_parser.add_argument('name', help = 'Name/login of the user to which change the password') user_pass_parser.add_argument('password', nargs = '?', help = 'New password') - @db_session def user_list(self): self.write_line('Name\t\tAdmin\tEmail\n----\t\t-----\t-----') self.write_line('\n'.join('{0: <16}{1}\t{2}'.format(u.name, '*' if u.admin else '', u.mail) for u in User.select())) + def _ask_password(self): # pragma: nocover + password = getpass.getpass() + confirm = getpass.getpass('Confirm password: ') + if password != confirm: + raise ValueError("Passwords don't match") + return password + def user_add(self, name, admin, password, email): - if not password: - password = getpass.getpass() - confirm = getpass.getpass('Confirm password: ') - if password != confirm: - self.write_error_line("Passwords don't match") - return - status = UserManager.add(name, password, email, admin) - if status != UserManager.SUCCESS: - self.write_error_line(UserManager.error_str(status)) + try: + if not password: + password = self._ask_password() # pragma: nocover + UserManager.add(name, password, email, admin) + except ValueError as e: + self.write_error_line(str(e)) def user_delete(self, name): - ret = UserManager.delete_by_name(name) - if ret != UserManager.SUCCESS: - self.write_error_line(UserManager.error_str(ret)) - else: + try: + UserManager.delete_by_name(name) self.write_line("Deleted user '{}'".format(name)) + except ObjectNotFound as e: + self.write_error_line(str(e)) - @db_session def user_setadmin(self, name, off): user = User.get(name = name) if user is None: @@ -250,15 +253,11 @@ class SupysonicCLI(cmd.Cmd): self.write_line("{0} '{1}' admin rights".format('Revoked' if off else 'Granted', name)) def user_changepass(self, name, password): - if not password: - password = getpass.getpass() - confirm = getpass.getpass('Confirm password: ') - if password != confirm: - self.write_error_line("Passwords don't match") - return - status = UserManager.change_password2(name, password) - if status != UserManager.SUCCESS: - self.write_error_line(UserManager.error_str(status)) - else: + try: + if not password: + password = self._ask_password() # pragma: nocover + UserManager.change_password2(name, password) self.write_line("Successfully changed '{}' password".format(name)) + except ObjectNotFound as e: + self.write_error_line(str(e)) diff --git a/supysonic/frontend/__init__.py b/supysonic/frontend/__init__.py index 813b929..8d80890 100644 --- a/supysonic/frontend/__init__.py +++ b/supysonic/frontend/__init__.py @@ -12,6 +12,7 @@ from flask import redirect, request, session, url_for from flask import Blueprint from functools import wraps +from pony.orm import ObjectNotFound from ..db import Artist, Album, Track from ..managers.user import UserManager @@ -23,12 +24,12 @@ def login_check(): request.user = None should_login = True if session.get('userid'): - code, user = UserManager.get(session.get('userid')) - if code != UserManager.SUCCESS: - session.clear() - else: + try: + user = UserManager.get(session.get('userid')) request.user = user should_login = False + except (ValueError, ObjectNotFound): + session.clear() if should_login and request.endpoint != 'frontend.login': flash('Please login') diff --git a/supysonic/frontend/folder.py b/supysonic/frontend/folder.py index 3c3287c..a953440 100644 --- a/supysonic/frontend/folder.py +++ b/supysonic/frontend/folder.py @@ -24,7 +24,6 @@ import uuid from flask import current_app, flash, redirect, render_template, request, url_for from ..db import Folder -from ..managers.user import UserManager from ..managers.folder import FolderManager from ..scanner import Scanner diff --git a/supysonic/frontend/playlist.py b/supysonic/frontend/playlist.py index b6da015..1c45b95 100644 --- a/supysonic/frontend/playlist.py +++ b/supysonic/frontend/playlist.py @@ -24,7 +24,6 @@ from flask import flash, redirect, render_template, request, url_for from pony.orm import ObjectNotFound from ..db import Playlist -from ..managers.user import UserManager from . import frontend diff --git a/supysonic/frontend/user.py b/supysonic/frontend/user.py index e60df38..3aae037 100644 --- a/supysonic/frontend/user.py +++ b/supysonic/frontend/user.py @@ -21,6 +21,7 @@ from flask import flash, redirect, render_template, request, session, url_for from flask import current_app from functools import wraps +from pony.orm import ObjectNotFound from ..db import User, ClientPrefs from ..lastfm import LastFm @@ -42,9 +43,13 @@ def me_or_uuid(f, arg = 'uid'): elif not request.user.admin: return redirect(url_for('frontend.index')) else: - code, user = UserManager.get(uid) - if code != UserManager.SUCCESS: - flash(UserManager.error_str(code)) + try: + user = UserManager.get(uid) + except ValueError as e: + flash(str(e), 'error') + return redirect(url_for('frontend.index')) + except ObjectNotFound: + flash('No such user', 'error') return redirect(url_for('frontend.index')) if kwargs: @@ -104,9 +109,13 @@ def update_clients(uid, user): @frontend.route('/user//changeusername') @admin_only def change_username_form(uid): - code, user = UserManager.get(uid) - if code != UserManager.SUCCESS: - flash(UserManager.error_str(code)) + try: + user = UserManager.get(uid) + except ValueError as e: + flash(str(e), 'error') + return redirect(url_for('frontend.index')) + except ObjectNotFound: + flash('No such user', 'error') return redirect(url_for('frontend.index')) return render_template('change_username.html', user = user) @@ -114,8 +123,13 @@ def change_username_form(uid): @frontend.route('/user//changeusername', methods = [ 'POST' ]) @admin_only def change_username_post(uid): - code, user = UserManager.get(uid) - if code != UserManager.SUCCESS: + try: + user = UserManager.get(uid) + except ValueError as e: + flash(str(e), 'error') + return redirect(url_for('frontend.index')) + except ObjectNotFound: + flash('No such user', 'error') return redirect(url_for('frontend.index')) username = request.form.get('user') @@ -178,16 +192,16 @@ def change_password_post(uid, user): error = True if not error: - if user.id == request.user.id: - status = UserManager.change_password(user.id, current, new) - else: - status = UserManager.change_password2(user.name, new) + try: + if user.id == request.user.id: + UserManager.change_password(user.id, current, new) + else: + UserManager.change_password2(user.name, new) - if status != UserManager.SUCCESS: - flash(UserManager.error_str(status)) - else: flash('Password changed') return redirect(url_for('frontend.user_profile', uid = uid)) + except ValueError as e: + flash(str(e), 'error') return change_password_form(uid, user) @@ -216,23 +230,25 @@ def add_user_post(): mail = '' if not error: - status = UserManager.add(name, passwd, mail, admin) - if status == UserManager.SUCCESS: + try: + UserManager.add(name, passwd, mail, admin) flash("User '%s' successfully added" % name) return redirect(url_for('frontend.user_index')) - else: - flash(UserManager.error_str(status)) + except ValueError as e: + flash(str(e), 'error') return add_user_form() @frontend.route('/user/del/') @admin_only def del_user(uid): - status = UserManager.delete(uid) - if status == UserManager.SUCCESS: + try: + UserManager.delete(uid) flash('Deleted user') - else: - flash(UserManager.error_str(status)) + except ValueError as e: + flash(str(e), 'error') + except ObjectNotFound: + flash('No such user', 'error') return redirect(url_for('frontend.user_index')) @@ -240,7 +256,7 @@ def del_user(uid): @me_or_uuid def lastfm_reg(uid, user): token = request.args.get('token') - if token in ('', None): + if not token: flash('Missing LastFM auth token') return redirect(url_for('frontend.user_profile', uid = uid)) @@ -270,21 +286,21 @@ def login(): name, password = map(request.form.get, [ 'user', 'password' ]) error = False - if name in ('', None): + if not name: flash('Missing user name') error = True - if password in ('', None): + if not password: flash('Missing password') error = True if not error: - status, user = UserManager.try_auth(name, password) - if status == UserManager.SUCCESS: + user = UserManager.try_auth(name, password) + if user: session['userid'] = str(user.id) flash('Logged in!') return redirect(return_url) else: - flash(UserManager.error_str(status)) + flash('Wrong username or password') return render_template('login.html') diff --git a/supysonic/managers/user.py b/supysonic/managers/user.py index 2931eef..c8ebf99 100644 --- a/supysonic/managers/user.py +++ b/supysonic/managers/user.py @@ -14,45 +14,27 @@ import random import string import uuid -from pony.orm import db_session from pony.orm import ObjectNotFound -from ..db import User, ChatMessage, Playlist -from ..db import StarredFolder, StarredArtist, StarredAlbum, StarredTrack -from ..db import RatingFolder, RatingTrack +from ..db import User from ..py23 import strtype class UserManager: - SUCCESS = 0 - INVALID_ID = 1 - NO_SUCH_USER = 2 - NAME_EXISTS = 3 - WRONG_PASS = 4 - @staticmethod - @db_session def get(uid): - if isinstance(uid, strtype): - try: - uid = uuid.UUID(uid) - except ValueError: - return UserManager.INVALID_ID, None - elif isinstance(uid, uuid.UUID): + if isinstance(uid, uuid.UUID): pass + elif isinstance(uid, strtype): + uid = uuid.UUID(uid) else: - return UserManager.INVALID_ID, None + raise ValueError('Invalid user id') - try: - user = User[uid] - return UserManager.SUCCESS, user - except ObjectNotFound: - return UserManager.NO_SUCH_USER, None + return User[uid] @staticmethod - @db_session def add(name, password, mail, admin): - if User.get(name = name) is not None: - return UserManager.NAME_EXISTS + if User.exists(name = name): + raise ValueError("User '{}' exists".format(name)) crypt, salt = UserManager.__encrypt_password(password) @@ -64,74 +46,45 @@ class UserManager: admin = admin ) - return UserManager.SUCCESS + return user @staticmethod - @db_session def delete(uid): - status, user = UserManager.get(uid) - if status != UserManager.SUCCESS: - return status - + user = UserManager.get(uid) user.delete() - return UserManager.SUCCESS @staticmethod - @db_session def delete_by_name(name): user = User.get(name = name) if user is None: - return UserManager.NO_SUCH_USER - return UserManager.delete(user.id) + raise ObjectNotFound(User) + user.delete() @staticmethod - @db_session def try_auth(name, password): user = User.get(name = name) if user is None: - return UserManager.NO_SUCH_USER, None + return None elif UserManager.__encrypt_password(password, user.salt)[0] != user.password: - return UserManager.WRONG_PASS, None + return None else: - return UserManager.SUCCESS, user + return user @staticmethod - @db_session def change_password(uid, old_pass, new_pass): - status, user = UserManager.get(uid) - if status != UserManager.SUCCESS: - return status - + user = UserManager.get(uid) if UserManager.__encrypt_password(old_pass, user.salt)[0] != user.password: - return UserManager.WRONG_PASS + raise ValueError('Wrong password') user.password = UserManager.__encrypt_password(new_pass, user.salt)[0] - return UserManager.SUCCESS @staticmethod - @db_session def change_password2(name, new_pass): user = User.get(name = name) if user is None: - return UserManager.NO_SUCH_USER + raise ObjectNotFound(User) user.password = UserManager.__encrypt_password(new_pass, user.salt)[0] - return UserManager.SUCCESS - - @staticmethod - def error_str(err): - if err == UserManager.SUCCESS: - return 'No error' - elif err == UserManager.INVALID_ID: - return 'Invalid user id' - elif err == UserManager.NO_SUCH_USER: - return 'No such user' - elif err == UserManager.NAME_EXISTS: - return 'There is already a user with that name' - elif err == UserManager.WRONG_PASS: - return 'Wrong password' - else: - return 'Unkown error' @staticmethod def __encrypt_password(password, salt = None): diff --git a/tests/frontend/test_login.py b/tests/frontend/test_login.py index ccf7904..9e4b009 100644 --- a/tests/frontend/test_login.py +++ b/tests/frontend/test_login.py @@ -31,9 +31,9 @@ class LoginTestCase(FrontendTestBase): self.assertIn('Missing password', rv.data) # Login with not valid user or password rv = self._login('nonexistent', 'nonexistent') - self.assertIn('No such user', rv.data) + self.assertIn('Wrong username or password', rv.data) rv = self._login('alice', 'badpassword') - self.assertIn('Wrong password', rv.data) + self.assertIn('Wrong username or password', rv.data) def test_login_admin(self): # Login with a valid admin user diff --git a/tests/frontend/test_user.py b/tests/frontend/test_user.py index 850853a..3994159 100644 --- a/tests/frontend/test_user.py +++ b/tests/frontend/test_user.py @@ -11,6 +11,7 @@ import uuid +from flask import escape from pony.orm import db_session from supysonic.db import User, ClientPrefs @@ -38,7 +39,7 @@ class UserTestCase(FrontendTestBase): def test_details(self): self._login('alice', 'Alic3') rv = self.client.get('/user/string', follow_redirects = True) - self.assertIn('Invalid', rv.data) + self.assertIn('badly formed', rv.data) rv = self.client.get('/user/' + str(uuid.uuid4()), follow_redirects = True) self.assertIn('No such user', rv.data) rv = self.client.get('/user/' + str(self.users['bob'])) @@ -89,14 +90,17 @@ class UserTestCase(FrontendTestBase): self._login('alice', 'Alic3') rv = self.client.get('/user/whatever/changeusername', follow_redirects = True) - self.assertIn('Invalid', rv.data) + self.assertIn('badly formed', rv.data) rv = self.client.get('/user/{}/changeusername'.format(uuid.uuid4()), follow_redirects = True) self.assertIn('No such user', rv.data) self.client.get('/user/{}/changeusername'.format(self.users['bob'])) def test_change_username_post(self): self._login('alice', 'Alic3') - self.client.post('/user/whatever/changeusername') + rv = self.client.post('/user/whatever/changeusername', follow_redirects = True) + self.assertIn('badly formed', rv.data) + rv = self.client.post('/user/{}/changeusername'.format(uuid.uuid4()), follow_redirects = True) + self.assertIn('No such user', rv.data) path = '/user/{}/changeusername'.format(self.users['bob']) rv = self.client.post(path, follow_redirects = True) @@ -186,7 +190,7 @@ class UserTestCase(FrontendTestBase): rv = self.client.post('/user/add', data = { 'user': 'name', 'passwd': 'passwd' }) self.assertIn('passwords don', rv.data) rv = self.client.post('/user/add', data = { 'user': 'alice', 'passwd': 'passwd', 'passwd_confirm': 'passwd' }) - self.assertIn('already a user with that name', rv.data) + self.assertIn(escape("User 'alice' exists"), rv.data) with db_session: self.assertEqual(User.select().count(), 2) @@ -210,7 +214,7 @@ class UserTestCase(FrontendTestBase): self._login('alice', 'Alic3') rv = self.client.get('/user/del/string', follow_redirects = True) - self.assertIn('Invalid', rv.data) + self.assertIn('badly formed', rv.data) rv = self.client.get('/user/del/' + str(uuid.uuid4()), follow_redirects = True) self.assertIn('No such user', rv.data) rv = self.client.get(path, follow_redirects = True) @@ -219,7 +223,7 @@ class UserTestCase(FrontendTestBase): self.assertEqual(User.select().count(), 1) self._logout() rv = self._login('bob', 'B0b') - self.assertIn('No such user', rv.data) + self.assertIn('Wrong username or password', rv.data) def test_lastfm_link(self): self._login('alice', 'Alic3') diff --git a/tests/managers/test_manager_user.py b/tests/managers/test_manager_user.py index 1aa56ea..a0c9850 100644 --- a/tests/managers/test_manager_user.py +++ b/tests/managers/test_manager_user.py @@ -32,9 +32,9 @@ class UserManagerTestCase(unittest.TestCase): @db_session def create_data(self): # Create some users - self.assertEqual(UserManager.add('alice', 'ALICE', 'test@example.com', True), UserManager.SUCCESS) - self.assertEqual(UserManager.add('bob', 'BOB', 'bob@example.com', False), UserManager.SUCCESS) - self.assertEqual(UserManager.add('charlie', 'CHARLIE', 'charlie@example.com', False), UserManager.SUCCESS) + self.assertIsInstance(UserManager.add('alice', 'ALICE', 'test@example.com', True), db.User) + self.assertIsInstance(UserManager.add('bob', 'BOB', 'bob@example.com', False), db.User) + self.assertIsInstance(UserManager.add('charlie', 'CHARLIE', 'charlie@example.com', False), db.User) folder = db.Folder(name = 'Root', path = 'tests/assets', root = True) artist = db.Artist(name = 'Artist') @@ -62,9 +62,9 @@ class UserManagerTestCase(unittest.TestCase): def test_encrypt_password(self): func = UserManager._UserManager__encrypt_password - self.assertEqual(func(u'password',u'salt'), (u'59b3e8d637cf97edbe2384cf59cb7453dfe30789', u'salt')) - self.assertEqual(func(u'pass-word',u'pepper'), (u'd68c95a91ed7773aa57c7c044d2309a5bf1da2e7', u'pepper')) - self.assertEqual(func(u'éèàïô', u'ABC+'), (u'b639ba5217b89c906019d89d5816b407d8730898', u'ABC+')) + self.assertEqual(func('password','salt'), ('59b3e8d637cf97edbe2384cf59cb7453dfe30789', 'salt')) + self.assertEqual(func('pass-word','pepper'), ('d68c95a91ed7773aa57c7c044d2309a5bf1da2e7', 'pepper')) + self.assertEqual(func(u'éèàïô', 'ABC+'), ('b639ba5217b89c906019d89d5816b407d8730898', 'ABC+')) @db_session def test_get_user(self): @@ -73,14 +73,14 @@ class UserManagerTestCase(unittest.TestCase): # Get existing users for name in ['alice', 'bob', 'charlie']: user = db.User.get(name = name) - self.assertEqual(UserManager.get(user.id), (UserManager.SUCCESS, user)) + self.assertEqual(UserManager.get(user.id), user) # Get with invalid UUID - self.assertEqual(UserManager.get('invalid-uuid'), (UserManager.INVALID_ID, None)) - self.assertEqual(UserManager.get(0xfee1bad), (UserManager.INVALID_ID, None)) + self.assertRaises(ValueError, UserManager.get, 'invalid-uuid') + self.assertRaises(ValueError, UserManager.get, 0xfee1bad) # Non-existent user - self.assertEqual(UserManager.get(uuid.uuid4()), (UserManager.NO_SUCH_USER, None)) + self.assertRaises(ObjectNotFound, UserManager.get, uuid.uuid4()) @db_session def test_add_user(self): @@ -88,25 +88,25 @@ class UserManagerTestCase(unittest.TestCase): self.assertEqual(db.User.select().count(), 3) # Create duplicate - self.assertEqual(UserManager.add('alice', 'Alic3', 'alice@example.com', True), UserManager.NAME_EXISTS) + self.assertRaises(ValueError, UserManager.add, 'alice', 'Alic3', 'alice@example.com', True) @db_session def test_delete_user(self): self.create_data() # Delete invalid UUID - self.assertEqual(UserManager.delete('invalid-uuid'), UserManager.INVALID_ID) - self.assertEqual(UserManager.delete(0xfee1b4d), UserManager.INVALID_ID) + self.assertRaises(ValueError, UserManager.delete, 'invalid-uuid') + self.assertRaises(ValueError, UserManager.delete, 0xfee1b4d) self.assertEqual(db.User.select().count(), 3) # Delete non-existent user - self.assertEqual(UserManager.delete(uuid.uuid4()), UserManager.NO_SUCH_USER) + self.assertRaises(ObjectNotFound, UserManager.delete, uuid.uuid4()) self.assertEqual(db.User.select().count(), 3) # Delete existing users for name in ['alice', 'bob', 'charlie']: user = db.User.get(name = name) - self.assertEqual(UserManager.delete(user.id), UserManager.SUCCESS) + UserManager.delete(user.id) self.assertRaises(ObjectNotFound, db.User.__getitem__, user.id) commit() self.assertEqual(db.User.select().count(), 0) @@ -117,11 +117,11 @@ class UserManagerTestCase(unittest.TestCase): # Delete existing users for name in ['alice', 'bob', 'charlie']: - self.assertEqual(UserManager.delete_by_name(name), UserManager.SUCCESS) + UserManager.delete_by_name(name) self.assertFalse(db.User.exists(name = name)) # Delete non-existent user - self.assertEqual(UserManager.delete_by_name('null'), UserManager.NO_SUCH_USER) + self.assertRaises(ObjectNotFound, UserManager.delete_by_name, 'null') @db_session def test_try_auth(self): @@ -130,14 +130,15 @@ class UserManagerTestCase(unittest.TestCase): # Test authentication for name in ['alice', 'bob', 'charlie']: user = db.User.get(name = name) - self.assertEqual(UserManager.try_auth(name, name.upper()), (UserManager.SUCCESS, user)) + authed = UserManager.try_auth(name, name.upper()) + self.assertEqual(authed, user) # Wrong password - self.assertEqual(UserManager.try_auth('alice', 'bad'), (UserManager.WRONG_PASS, None)) - self.assertEqual(UserManager.try_auth('alice', 'alice'), (UserManager.WRONG_PASS, None)) + self.assertIsNone(UserManager.try_auth('alice', 'bad')) + self.assertIsNone(UserManager.try_auth('alice', 'alice')) # Non-existent user - self.assertEqual(UserManager.try_auth('null', 'null'), (UserManager.NO_SUCH_USER, None)) + self.assertIsNone(UserManager.try_auth('null', 'null')) @db_session def test_change_password(self): @@ -147,21 +148,21 @@ class UserManagerTestCase(unittest.TestCase): for name in ['alice', 'bob', 'charlie']: user = db.User.get(name = name) # Good password - self.assertEqual(UserManager.change_password(user.id, name.upper(), 'newpass'), UserManager.SUCCESS) - self.assertEqual(UserManager.try_auth(name, 'newpass'), (UserManager.SUCCESS, user)) + UserManager.change_password(user.id, name.upper(), 'newpass') + self.assertEqual(UserManager.try_auth(name, 'newpass'), user) # Old password - self.assertEqual(UserManager.try_auth(name, name.upper()), (UserManager.WRONG_PASS, None)) + self.assertEqual(UserManager.try_auth(name, name.upper()), None) # Wrong password - self.assertEqual(UserManager.change_password(user.id, 'badpass', 'newpass'), UserManager.WRONG_PASS) + self.assertRaises(ValueError, UserManager.change_password, user.id, 'badpass', 'newpass') # Ensure we still got the same number of users self.assertEqual(db.User.select().count(), 3) # With invalid UUID - self.assertEqual(UserManager.change_password('invalid-uuid', 'oldpass', 'newpass'), UserManager.INVALID_ID) + self.assertRaises(ValueError, UserManager.change_password, 'invalid-uuid', 'oldpass', 'newpass') # Non-existent user - self.assertEqual(UserManager.change_password(uuid.uuid4(), 'oldpass', 'newpass'), UserManager.NO_SUCH_USER) + self.assertRaises(ObjectNotFound, UserManager.change_password, uuid.uuid4(), 'oldpass', 'newpass') @db_session def test_change_password2(self): @@ -169,19 +170,13 @@ class UserManagerTestCase(unittest.TestCase): # With existing users for name in ['alice', 'bob', 'charlie']: - self.assertEqual(UserManager.change_password2(name, 'newpass'), UserManager.SUCCESS) + UserManager.change_password2(name, 'newpass') user = db.User.get(name = name) - self.assertEqual(UserManager.try_auth(name, 'newpass'), (UserManager.SUCCESS, user)) - self.assertEqual(UserManager.try_auth(name, name.upper()), (UserManager.WRONG_PASS, None)) + self.assertEqual(UserManager.try_auth(name, 'newpass'), user) + self.assertEqual(UserManager.try_auth(name, name.upper()), None) # Non-existent user - self.assertEqual(UserManager.change_password2('null', 'newpass'), UserManager.NO_SUCH_USER) - - def test_human_readable_error(self): - values = [ UserManager.SUCCESS, UserManager.INVALID_ID, UserManager.NO_SUCH_USER, UserManager.NAME_EXISTS, - UserManager.WRONG_PASS, 1594826, 'string', uuid.uuid4() ] - for value in values: - self.assertIsInstance(UserManager.error_str(value), strtype) + self.assertRaises(ObjectNotFound, UserManager.change_password2, 'null', 'newpass') if __name__ == '__main__': unittest.main() diff --git a/tests/testbase.py b/tests/testbase.py index 300d78f..1e570fc 100644 --- a/tests/testbase.py +++ b/tests/testbase.py @@ -15,6 +15,8 @@ import shutil import unittest import tempfile +from pony.orm import db_session + from supysonic.db import init_database, release_database from supysonic.config import DefaultConfig from supysonic.managers.user import UserManager @@ -95,8 +97,9 @@ class TestBase(unittest.TestCase): self.__app = create_application(config) self.client = self.__app.test_client() - UserManager.add('alice', 'Alic3', 'test@example.com', True) - UserManager.add('bob', 'B0b', 'bob@example.com', False) + with db_session: + UserManager.add('alice', 'Alic3', 'test@example.com', True) + UserManager.add('bob', 'B0b', 'bob@example.com', False) def _patch_client(self): self.client.get = patch_method(self.client.get)