From 888c3b48cdcd181652d46dbcefc5012fee8fdf4b Mon Sep 17 00:00:00 2001 From: Ethan Dalool Date: Sat, 4 Mar 2017 21:56:23 -0800 Subject: [PATCH] Completely revise exception message formatting technique --- etiquette/exceptions.py | 138 ++++++++++++++++++------------------- etiquette/helpers.py | 4 +- etiquette/objects.py | 7 +- etiquette/photodb.py | 65 ++++++++--------- etiquette/searchhelpers.py | 10 +-- etiquette_site.py | 17 ++--- templates/tags.html | 4 +- 7 files changed, 122 insertions(+), 123 deletions(-) diff --git a/etiquette/exceptions.py b/etiquette/exceptions.py index e4a3cc5..d150cad 100644 --- a/etiquette/exceptions.py +++ b/etiquette/exceptions.py @@ -1,104 +1,100 @@ +import re + +def pascal_to_loudsnakes(text): + match = re.findall('[A-Z][a-z]*', text) + text = '_'.join(match) + text = text.upper() + return text + class EtiquetteException(Exception): - pass + error_message = '' + def __init__(self, *args): + self.error_type = pascal_to_loudsnakes(type(self).__name__) + Exception.__init__(self, *args) + +class WithFormat(EtiquetteException): + def __init__(self, *args, **kwargs): + self.error_message = self.error_message.format(*args, **kwargs) + EtiquetteException.__init__(self, self.error_message) # NO SUCH -class NoSuchAlbum(EtiquetteException): - error_type = 'NO_SUCH_ALBUM' - error_message = 'Album "{album}" does not exist.' +class NoSuch(WithFormat): pass -class NoSuchBookmark(EtiquetteException): - error_type = 'NO_SUCH_BOOKMARK' - error_message = 'Bookmark "{bookmark}" does not exist.' - pass +class NoSuchAlbum(NoSuch): + error_message = 'Album "{}" does not exist.' -class NoSuchGroup(EtiquetteException): - error_type = 'NO_SUCH_GROUP' - error_message = 'Group "{group}" does not exist.' - pass +class NoSuchBookmark(NoSuch): + error_message = 'Bookmark "{}" does not exist.' -class NoSuchPhoto(EtiquetteException): - error_type = 'NO_SUCH_PHOTO' - error_message = 'Photo "{photo}" does not exist.' - pass +class NoSuchGroup(NoSuch): + error_message = 'Group "{}" does not exist.' -class NoSuchSynonym(EtiquetteException): - error_type = 'NO_SUCH_SYNONYM' - error_message = 'Synonym "{synonym}" does not exist.' - pass +class NoSuchPhoto(NoSuch): + error_message = 'Photo "{}" does not exist.' -class NoSuchTag(EtiquetteException): - error_type = 'NO_SUCH_TAG' - error_message = 'Tag "{tag}" does not exist.' - pass +class NoSuchSynonym(NoSuch): + error_message = 'Synonym "{}" does not exist.' -class NoSuchUser(EtiquetteException): - error_type = 'NO_SUCH_User' - error_message = 'User "{user}" does not exist.' - pass +class NoSuchTag(NoSuch): + error_message = 'Tag "{}" does not exist.' + +class NoSuchUser(NoSuch): + error_message = 'User "{}" does not exist.' # EXISTS -class GroupExists(EtiquetteException): - pass +class GroupExists(WithFormat): + error_message = '{member} already in group {group}' -class PhotoExists(EtiquetteException): - pass +class PhotoExists(WithFormat): + error_message = 'Photo "{}" already exists.' + def __init__(self, photo): + self.photo = photo + WithFormat.__init__(self, photo.id) -class TagExists(EtiquetteException): - pass +class TagExists(WithFormat): + error_message = 'Tag "{}" already exists.' + def __init__(self, tag): + self.tag = tag + WithFormat.__init__(self, tag.name) -class UserExists(EtiquetteException): - error_type = 'USER_EXISTS' - error_message = 'Username "{username}" already exists.' - pass +class UserExists(WithFormat): + error_message = 'User "{}" already exists.' + def __init__(self, user): + self.user = user + WithFormat.__init__(self, user.username) # TAG ERRORS class CantSynonymSelf(EtiquetteException): - error_type = 'TAG_SYNONYM_ITSELF' error_message = 'Cannot apply synonym to self.' - pass class RecursiveGrouping(EtiquetteException): - error_type = 'RECURSIVE_GROUPING' - error_message = 'Cannot create a group within itself.' - pass + error_message = '{group} is an ancestor of {member}.' -class TagTooLong(EtiquetteException): - error_type = 'TAG_TOO_LONG' - error_message = 'Tag "{tag}" is too long.' - pass +class TagTooLong(WithFormat): + error_message = 'Tag "{}" is too long.' -class TagTooShort(EtiquetteException): - error_type = 'TAG_TOO_SHORT' - error_message = 'Tag "{tag}" has too few valid characters.' - pass +class TagTooShort(WithFormat): + error_message = 'Tag "{}" has too few valid characters.' # USER ERRORS -class InvalidUsernameChars(EtiquetteException): - error_type = 'INVALID_USERNAME_CHARACTERS' - error_message = 'Username "{username}" contains invalid characters: {badchars}' - pass +class InvalidUsernameChars(WithFormat): + error_message = 'Username "{username}" contains invalid characters: {badchars}.' -class PasswordTooShort(EtiquetteException): - error_type = 'PASSWORD_TOO_SHORT' - error_message = 'Password is shorter than the minimum of {min_length}' - pass +class PasswordTooShort(WithFormat): + error_message = 'Password is shorter than the minimum of {min_length}.' -class UsernameTooLong(EtiquetteException): - error_type = 'USERNAME_TOO_LONG' - error_message = 'Username "{username}" is longer than maximum of {max_length}' - pass +class UsernameTooLong(WithFormat): + error_message = 'Username "{username}" is longer than maximum of {max_length}.' -class UsernameTooShort(EtiquetteException): - error_type = 'USERNAME_TOO_SHORT' - error_message = 'Username "{username}" is shorter than minimum of {min_length}' - pass +class UsernameTooShort(WithFormat): + error_message = 'Username "{username}" is shorter than minimum of {min_length}.' class WrongLogin(EtiquetteException): - pass + error_message = 'Wrong username-password combination.' # GENERAL ERRORS @@ -108,10 +104,8 @@ class NotExclusive(EtiquetteException): ''' pass -class OutOfOrder(EtiquetteException): +class OutOfOrder(WithFormat): ''' For when a requested minmax range (a, b) has b > a ''' - error_type = 'OUT_OF_ORDER' - error_message = 'Field "{field}": minimum "{min}" and maximum "{max}" are out of order.' - pass + error_message = 'Range "{range}": minimum "{min}" and maximum "{max}" are out of order.' diff --git a/etiquette/helpers.py b/etiquette/helpers.py index d2273f8..5fe99b4 100644 --- a/etiquette/helpers.py +++ b/etiquette/helpers.py @@ -151,7 +151,7 @@ def get_mimetype(filepath): def hyphen_range(s): ''' - Given a string like '1-3', return ints (1, 3) representing lower + Given a string like '1-3', return numbers (1, 3) representing lower and upper bounds. Supports bytestring.parsebytes and hh:mm:ss format. @@ -173,7 +173,7 @@ def hyphen_range(s): low = _unitconvert(low) high = _unitconvert(high) if low is not None and high is not None and low > high: - raise exceptions.OutOfOrder(s, low, high) + raise exceptions.OutOfOrder(range=s, min=low, max=high) return low, high def hms_to_seconds(hms): diff --git a/etiquette/objects.py b/etiquette/objects.py index c26d70f..4689e43 100644 --- a/etiquette/objects.py +++ b/etiquette/objects.py @@ -61,11 +61,11 @@ class GroupableMixin: that_group = self else: that_group = self.group_getter(id=parent_id) - raise exceptions.GroupExists('%s already in group %s' % (member, that_group)) + raise exceptions.GroupExists(member=member, group=that_group) for parent in self.walk_parents(): if parent == member: - raise exceptions.RecursiveGrouping('%s is an ancestor of %s' % (member, self)) + raise exceptions.RecursiveGrouping(member=member, group=self) self.photodb._cached_frozen_children = None cur.execute( @@ -808,8 +808,9 @@ class Tag(ObjectBase, GroupableMixin): def add_synonym(self, synname, *, commit=True): synname = self.photodb.normalize_tagname(synname) + print(synname, self.name) if synname == self.name: - raise ValueError('Cannot assign synonym to itself.') + raise exceptions.CantSynonymSelf() try: self.photodb.get_tag_by_name(synname) diff --git a/etiquette/photodb.py b/etiquette/photodb.py index af2c284..3566868 100644 --- a/etiquette/photodb.py +++ b/etiquette/photodb.py @@ -163,14 +163,14 @@ def operate(operand_stack, operator_stack): operand_stack.append(value) #print('after:', operand_stack, operator_stack) -def raise_no_such_thing(exception_class, thing_id=None, thing_name=None, comment=''): - if thing_id is not None: - message = 'ID: %s. %s' % (thing_id, comment) - elif thing_name is not None: - message = 'Name: %s. %s' % (thing_name, comment) - else: - message = '' - raise exception_class(message) +# def raise_no_such_thing(exception_class, thing_id=None, thing_name=None, comment=''): +# if thing_id is not None: +# message = 'ID: %s. %s' % (thing_id, comment) +# elif thing_name is not None: +# message = 'Name: %s. %s' % (thing_name, comment) +# else: +# message = '' +# raise exception_class(message) def searchfilter_must_may_forbid(photo_tags, tag_musts, tag_mays, tag_forbids, frozen_children): if tag_musts and not all(any(option in photo_tags for option in frozen_children[must]) for must in tag_musts): @@ -382,7 +382,7 @@ class PDBPhotoMixin: cur.execute('SELECT * FROM photos WHERE filepath == ?', [filepath]) fetch = cur.fetchone() if fetch is None: - raise_no_such_thing(exceptions.NoSuchPhoto, thing_name=filepath) + raise exceptions.NoSuchPhoto(filepath) photo = objects.Photo(self, fetch) return photo @@ -440,9 +440,7 @@ class PDBPhotoMixin: except exceptions.NoSuchPhoto: pass else: - exc = exceptions.PhotoExists(filename, existing) - exc.photo = existing - raise exc + raise exceptions.PhotoExists(existing) self.log.debug('New Photo: %s' % filename) author_id = self.get_user_id_or_none(author) @@ -743,11 +741,14 @@ class PDBPhotoMixin: for node in expression_tree.walk_leaves(): if node.token in frozen_children: continue + + exc = exceptions.NoSuchTag(node.token) if warning_bag is not None: - warning_bag.add(exceptions.NoSuchTag.error_message.format(tag=node.token)) + warning_bag.add(exc.error_message) node.token = None else: - raise_no_such_thing(exceptions.NoSuchTag, thing_name=node.token) + raise exc + expression_tree.prune() if filename: @@ -873,10 +874,8 @@ class PDBTagMixin: if id is not None: return self.get_tag_by_id(id) - elif name is not None: - return self.get_tag_by_name(name) else: - raise_no_such_thing(exceptions.NoSuchTag, thing_id=id, thing_name=name) + return self.get_tag_by_name(name) def get_tag_by_id(self, id): return self.get_thing_by_id('tag', thing_id=id) @@ -890,17 +889,18 @@ class PDBTagMixin: cur = self.sql.cursor() while True: - # Return if it's a toplevel, or resolve the synonym and try that. + # Return if it's a toplevel... cur.execute('SELECT * FROM tags WHERE name == ?', [tagname]) fetch = cur.fetchone() if fetch is not None: return objects.Tag(self, fetch) + # ...or resolve the synonym and try again. cur.execute('SELECT * FROM tag_synonyms WHERE name == ?', [tagname]) fetch = cur.fetchone() if fetch is None: - # was not a top tag or synonym - raise_no_such_thing(exceptions.NoSuchTag, thing_name=tagname) + # was not a master tag or synonym + raise exceptions.NoSuchTag(tagname) tagname = fetch[constants.SQL_SYN['master']] def get_tags(self): @@ -935,6 +935,7 @@ class PDBTagMixin: replaced by underscores, and is stripped of any not-whitelisted characters. ''' + original_tagname = tagname tagname = tagname.lower() tagname = tagname.replace('-', '_') tagname = tagname.replace(' ', '_') @@ -942,18 +943,20 @@ class PDBTagMixin: tagname = ''.join(tagname) if len(tagname) < self.config['min_tag_name_length']: + exc = exceptions.TagTooShort(original_tagname) if warning_bag is not None: - warning_bag.add(exceptions.TagTooShort.error_message.format(tag=tagname)) + warning_bag.add(exc.error_message) return None else: - raise exceptions.TagTooShort(tagname) + raise exc elif len(tagname) > self.config['max_tag_name_length']: + exc = exceptions.TagTooLong(tagname) if warning_bag is not None: - warning_bag.add(exceptions.TagTooLong.format(tag=tagname)) + warning_bag.add(exc.error_message) return None else: - raise exceptions.TagTooLong(tagname) + raise exc else: return tagname @@ -1033,20 +1036,20 @@ class PDBUserMixin: def register_user(self, username, password, commit=True): if len(username) < self.config['min_username_length']: - raise exceptions.UsernameTooShort(username) + raise exceptions.UsernameTooShort(username=username, min_length=self.config['min_username_length']) if len(username) > self.config['max_username_length']: - raise exceptions.UsernameTooLong(username) + raise exceptions.UsernameTooLong(username=username, max_length=self.config['max_username_length']) badchars = [c for c in username if c not in self.config['valid_username_chars']] if badchars: - raise exceptions.InvalidUsernameChars(badchars) + raise exceptions.InvalidUsernameChars(username=username, badchars=badchars) if not isinstance(password, bytes): password = password.encode('utf-8') if len(password) < self.config['min_password_length']: - raise exceptions.PasswordTooShort + raise exceptions.PasswordTooShort(min_length=self.config['min_password_length']) cur = self.sql.cursor() cur.execute('SELECT * FROM users WHERE username == ?', [username]) @@ -1288,7 +1291,7 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs ebstring = ebstring.strip() ebstring = ebstring.strip('.+=') if ebstring == '': - return + return output_notes if '=' in ebstring and '+' in ebstring: raise ValueError('Cannot rename and assign snynonym at once') @@ -1312,7 +1315,7 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs raise ValueError('Too many plus signs') if not tag: - return None + return output_notes if rename_to: tag = self.get_tag(tag) @@ -1382,7 +1385,7 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs cur.execute(query, [thing_id]) thing = cur.fetchone() if thing is None: - return raise_no_such_thing(thing_map['exception'], thing_id=thing_id) + raise thing_map['exception'](thing_id) thing = thing_map['class'](self, thing) return thing diff --git a/etiquette/searchhelpers.py b/etiquette/searchhelpers.py index 69bc881..dae8d9f 100644 --- a/etiquette/searchhelpers.py +++ b/etiquette/searchhelpers.py @@ -77,7 +77,7 @@ def minmax(key, value, minimums, maximums, warning_bag=None): except exceptions.OutOfOrder as e: if warning_bag: - warning_bag.add(e.error_message.format(field=key, min=e.args[1], max=e.args[2])) + warning_bag.add(e.error_message) return else: raise @@ -113,9 +113,9 @@ def normalize_authors(authors, photodb, warning_bag=None): try: user = get_user(photodb, requested_author) - except exceptions.NoSuchUser: + except exceptions.NoSuchUser as e: if warning_bag: - warning_bag.add(exceptions.NoSuchUser.format(username=requested_author)) + warning_bag.add(e.error_message) else: raise else: @@ -309,9 +309,9 @@ def normalize_tag_mmf(tags, photodb, warning_bag=None): try: tag = photodb.get_tag(name=tag) - except exceptions.NoSuchTag: + except exceptions.NoSuchTag as e: if warning_bag: - warning_bag.add(exceptions.NoSuchTag.format(tag=tag)) + warning_bag.add(e.error_message) continue else: raise diff --git a/etiquette_site.py b/etiquette_site.py index 200d9ce..8882e8e 100644 --- a/etiquette_site.py +++ b/etiquette_site.py @@ -76,11 +76,16 @@ def P_wrapper(function): return function(thingid) except exceptions.EtiquetteException as e: + if isinstance(e, exceptions.NoSuch): + status = 404 + else: + status = 400 + if response_type == 'html': - flask.abort(404, e.error_message) + flask.abort(status, e.error_message) else: response = {'error_type': e.error_type, 'error_message': e.error_message} - response = jsonify.make_json_response(response, status=404) + response = jsonify.make_json_response(response, status=status) flask.abort(response) except Exception as e: @@ -390,21 +395,17 @@ def get_file(photoid): return send_file(photo.real_filepath, override_mimetype=photo.mimetype) -def get_photo_core(photoid): - photo = P_photo(photoid) - return photo - @site.route('/photo/', methods=['GET']) @session_manager.give_token def get_photo_html(photoid): - photo = get_photo_core(photoid) + photo = P_photo(photoid, response_type='html') session = session_manager.get(request) return flask.render_template('photo.html', photo=photo, session=session) @site.route('/photo/.json', methods=['GET']) @session_manager.give_token def get_photo_json(photoid): - photo = get_photo_core(photoid) + photo = P_photo(photoid, response_type='json') photo = jsonify.photo(photo) photo = jsonify.make_json_response(photo) return photo diff --git a/templates/tags.html b/templates/tags.html index 1516d03..0113edf 100644 --- a/templates/tags.html +++ b/templates/tags.html @@ -133,10 +133,10 @@ function receive_callback(responses) { var response = responses[index]; var tagname = response["tagname"]; - if ("error" in response) + if ("error_type" in response) { message_positivity = "message_negative"; - message_text = '"' + tagname + '" ' + response["error"]; + message_text = '"' + tagname + '" ' + response["error_message"]; } else if ("action" in response) {