Standardize error responses according to json-api

Fixes #91

api/__init__.py

@@ -13,7 +13,7 @@
 from flask_login import LoginManager
 from flask_cors import CORS
 
-from api.invalid_usage import InvalidUsage
+from api.error import ApiException
 from api.jwt_user import JwtUser
 from api.user import User
 
@@ -44,8 +44,8 @@ def after_request(response):
     response.headers['Access-Control-Allow-Methods'] = 'GET,PUT,POST,DELETE'
     return response
 
-@app.errorhandler(InvalidUsage)
-def handle_invalid_usage(error):
+@app.errorhandler(ApiException)
+def handle_api_exception(error):
     response = jsonify(error.to_dict())
     response.status_code = error.status_code
     response.headers['content-type'] = 'application/vnd.api+json'

api/achievements.py

@@ -3,6 +3,8 @@
 from flask_jwt import jwt_required, current_identity
 from api import *
 import faf.db as db
+
+from api.error import ApiException, Error, ErrorCode
 from api.query_commons import fetch_data
 
 MAX_PAGE_SIZE = 1000
@@ -448,8 +450,7 @@ def update_steps(achievement_id, player_id, steps, steps_function):
     """
     achievement = achievements_get(achievement_id)['data']['attributes']
     if achievement['type'] != 'INCREMENTAL':
-        raise InvalidUsage('Only incremental achievements can be incremented ({})'.format(achievement_id),
-                           status_code=400)
+        raise ApiException([Error(ErrorCode.ACHIEVEMENT_CANT_INCREMENT_STANDARD, achievement_id)])
 
     with db.connection:
         cursor = db.connection.cursor(db.pymysql.cursors.DictCursor)
@@ -510,8 +511,7 @@ def unlock_achievement(achievement_id, player_id):
         cursor.execute('SELECT type FROM achievement_definitions WHERE id = %s', achievement_id)
         achievement = cursor.fetchone()
         if achievement['type'] != 'STANDARD':
-            raise InvalidUsage('Only standard achievements can be unlocked directly ({})'.format(achievement_id),
-                               status_code=400)
+            raise ApiException([Error(ErrorCode.ACHIEVEMENT_CANT_UNLOCK_INCREMENTAL, achievement_id)])
 
         cursor.execute("""SELECT
                             state

api/error.py

@@ -0,0 +1,103 @@
+from enum import Enum
+
+
+class ErrorCode(Enum):
+    ACHIEVEMENT_CANT_INCREMENT_STANDARD = dict(
+        code=100,
+        title='Invalid operation',
+        detail='Only incremental achievements can be incremented. Achievement ID: {0}.')
+    ACHIEVEMENT_CANT_UNLOCK_INCREMENTAL = dict(
+        code=101,
+        title='Invalid operation',
+        detail='Only standard achievements can be unlocked directly. Achievement ID: {0}.')
+    UPLOAD_FILE_MISSING = dict(
+        code=102,
+        title='Missing file',
+        detail='A file has to be provided as parameter "file".')
+    UPLOAD_METADATA_MISSING = dict(
+        code=103,
+        title='Missing metadata',
+        detail='A parameter "metadata" has to be provided.')
+    UPLOAD_INVALID_FILE_EXTENSION = dict(
+        code=104,
+        title='Invalid file extension',
+        detail='File must have the following extension: {0}.')
+    MAP_NAME_TOO_LONG = dict(
+        code=105,
+        title='Invalid map name',
+        detail='The map name must not exceed {0} characters, was: {1}')
+    MAP_NOT_ORIGINAL_AUTHOR = dict(
+        code=106,
+        title='Permission denied',
+        detail='Only the original author is allowed to upload new versions of map: {0}.')
+    MAP_VERSION_EXISTS = dict(
+        code=107,
+        title='Duplicate map version',
+        detail='Map "{0}" with version "{1}" already exists.')
+    MAP_NAME_CONFLICT = dict(
+        code=108,
+        title='Name clash',
+        detail='Another map with file name "{0}" already exists.')
+    MAP_NAME_MISSING = dict(
+        code=109,
+        title='Missing map name',
+        detail='The scenario file must specify a map name.')
+    MAP_DESCRIPTION_MISSING = dict(
+        code=110,
+        title='Missing description',
+        detail='The scenario file must specify a map description.')
+    MAP_FIRST_TEAM_FFA = dict(
+        code=111,
+        title='Invalid team name',
+        detail='The name of the first team has to be "FFA".')
+    MAP_TYPE_MISSING = dict(
+        code=112,
+        title='Missing map type',
+        detail='The scenario file must specify a map type.')
+    MAP_SIZE_MISSING = dict(
+        code=113,
+        title='Missing map size',
+        detail='The scenario file must specify a map size.')
+    MAP_VERSION_MISSING = dict(
+        code=114,
+        title='Missing map version',
+        detail='The scenario file must specify a map version.')
+    QUERY_INVALID_SORT_FIELD = dict(
+        code=115,
+        title='Invalid sort field',
+        detail='Sorting by "{0}" is not supported')
+    QUERY_INVALID_PAGE_SIZE = dict(
+        code=116,
+        title='Invalid page size',
+        detail='Page size is not valid: {0}')
+    QUERY_INVALID_PAGE_NUMBER = dict(
+        code=117,
+        title='Invalid page number',
+        detail='Page number is not valid: {0}')
+
+
+class Error:
+    def __init__(self, code: ErrorCode, *args):
+        self.code = code
+        self.args = args
+
+    def to_dict(self):
+        return {
+            'code': self.code.value['code'],
+            'title': self.code.value['title'],
+            'detail': self.code.value['detail'].format(*self.args),
+            'meta': {
+                'args': self.args
+            }
+        }
+
+
+class ApiException(Exception):
+    def __init__(self, errors, status_code=400):
+        self.errors = errors
+        self.status_code = status_code
+
+    def to_dict(self):
+        return {
+            'errors': [error.to_dict() for error in self.errors]
+        }

api/maps.py

@@ -10,13 +10,14 @@
 from flask import request
 from werkzeug.utils import secure_filename
 
-from api import app, InvalidUsage, oauth
+from api import app, oauth
+from api.error import ApiException, Error, ErrorCode
 from api.query_commons import fetch_data
 from faf import db
 
 logger = logging.getLogger(__name__)
 
-ALLOWED_EXTENSIONS = {'zip'}
+ALLOWED_EXTENSIONS = ['zip']
 MAX_PAGE_SIZE = 1000
 
 SELECT_EXPRESSIONS = {
@@ -81,13 +82,13 @@ def maps_upload():
     metadata_string = request.form.get('metadata')
 
     if not file:
-        raise InvalidUsage("No file has been provided")
+        raise ApiException([Error(ErrorCode.UPLOAD_FILE_MISSING)])
 
     if not metadata_string:
-        raise InvalidUsage("Value 'metadata' is missing")
+        raise ApiException([Error(ErrorCode.UPLOAD_METADATA_MISSING)])
 
     if not file_allowed(file.filename):
-        raise InvalidUsage("Invalid file extension")
+        raise ApiException([Error(ErrorCode.UPLOAD_INVALID_FILE_EXTENSION, *ALLOWED_EXTENSIONS)])
 
     metadata = json.loads(metadata_string)
 
@@ -243,7 +244,8 @@ def file_allowed(filename):
 
 
 def process_uploaded_map(temp_map_path, is_ranked):
-    map_info = parse_map_info(temp_map_path)
+    map_info = parse_map_info(temp_map_path, validate=False)
+    validate_map_info(map_info)
 
     display_name = map_info['display_name']
     name = map_info['name']
@@ -258,19 +260,19 @@ def process_uploaded_map(temp_map_path, is_ranked):
     height = int(size[1])
 
     if len(display_name) > 100:
-        raise InvalidUsage('Maximum map name length is 100, was: {}'.format(len(display_name)))
+        raise ApiException([Error(ErrorCode.MAP_NAME_TOO_LONG, 100, len(display_name))])
 
     user_id = request.oauth.user.id
     if not can_upload_map(display_name, user_id):
-        raise InvalidUsage('Only the original uploader is allowed to upload this map')
+        raise ApiException([Error(ErrorCode.MAP_NOT_ORIGINAL_AUTHOR, display_name)])
 
     if map_exists(display_name, version):
-        raise InvalidUsage('Map "{}" with version "{}" already exists'.format(display_name, version))
+        raise ApiException([Error(ErrorCode.MAP_VERSION_EXISTS, display_name, version)])
 
     zip_file_name = generate_zip_file_name(name, version)
     target_map_path = os.path.join(app.config['MAP_UPLOAD_PATH'], zip_file_name)
     if os.path.isfile(target_map_path):
-        raise InvalidUsage('Map with file name "{}" already exists'.format(zip_file_name))
+        raise ApiException([Error(ErrorCode.MAP_NAME_CONFLICT, zip_file_name)])
 
     shutil.move(temp_map_path, target_map_path)
 
@@ -314,21 +316,24 @@ def process_uploaded_map(temp_map_path, is_ranked):
                        })
 
 
-def validate_scenario_info(scenario_info):
-    if 'name' not in scenario_info:
-        raise InvalidUsage('Map name has to be specified')
-    if 'description' not in scenario_info:
-        raise InvalidUsage('Map description has to be specified')
-    if 'max_players' not in scenario_info \
-            or 'battle_type' not in scenario_info \
-            or scenario_info['battle_type'] != 'FFA':
-        raise InvalidUsage('Name of first team has to be "FFA"')
-    if 'map_type' not in scenario_info:
-        raise InvalidUsage('Map type has to be specified')
-    if 'map_size' not in scenario_info:
-        raise InvalidUsage('Map size has to be specified')
-    if 'version' not in scenario_info:
-        raise InvalidUsage('Map version has to be specified')
+def validate_map_info(map_info):
+    errors = []
+    if not map_info.get('display_name'):
+        errors.append(Error(ErrorCode.MAP_NAME_MISSING))
+    if not map_info.get('description'):
+        errors.append(Error(ErrorCode.MAP_DESCRIPTION_MISSING))
+    if not map_info.get('max_players') \
+            or map_info.get('battle_type') != 'FFA':
+        errors.append(Error(ErrorCode.MAP_FIRST_TEAM_FFA))
+    if not map_info.get('type'):
+        errors.append(Error(ErrorCode.MAP_TYPE_MISSING))
+    if not map_info.get('size'):
+        errors.append(Error(ErrorCode.MAP_SIZE_MISSING))
+    if not map_info.get('version'):
+        errors.append(Error(ErrorCode.MAP_VERSION_MISSING))
+
+    if errors:
+        raise ApiException(errors)
 
 
 def extract_preview(zip, member, target_folder, target_name):

api/mods.py

@@ -5,10 +5,12 @@
 from flask import request
 from werkzeug.utils import secure_filename
 
-from api import app, InvalidUsage
+from api import app
+from api.error import ApiException, ErrorCode
+from api.error import Error
 from api.query_commons import fetch_data
 
-ALLOWED_EXTENSIONS = {'zip'}
+ALLOWED_EXTENSIONS = ['zip']
 MAX_PAGE_SIZE = 1000
 
 SELECT_EXPRESSIONS = {
@@ -57,10 +59,10 @@ def mods_upload():
     """
     file = request.files.get('file')
     if not file:
-        raise InvalidUsage("No file has been provided")
+        raise ApiException([Error(ErrorCode.UPLOAD_FILE_MISSING)])
 
     if not file_allowed(file.filename):
-        raise InvalidUsage("Invalid file extension")
+        raise ApiException([Error(ErrorCode.UPLOAD_INVALID_FILE_EXTENSION, *ALLOWED_EXTENSIONS)])
 
     filename = secure_filename(file.filename)
     file.save(os.path.join(app.config['MOD_UPLOAD_PATH'], filename))

api/query_commons.py

@@ -1,6 +1,6 @@
 from pymysql.cursors import DictCursor
 
-from api import InvalidUsage
+from api.error import ApiException, Error, ErrorCode
 from faf import db
 
 
@@ -81,7 +81,7 @@ def get_order_by(sort_expression, valid_fields):
             column = expression
 
         if column not in valid_fields:
-            raise InvalidUsage("Invalid sort field")
+            raise ApiException([Error(ErrorCode.QUERY_INVALID_SORT_FIELD, column)])
 
         order_bys.append('`{}` {}'.format(column, order))
 
@@ -187,16 +187,20 @@ def fetch_data(schema, table, root_select_expression_dict, max_page_size, reques
 
 
 def get_page_attributes(max_page_size, request):
+    raw_page_size = request.values.get('page[size]', max_page_size)
     try:
-        page_size = int(request.values.get('page[size]', max_page_size))
+        page_size = int(raw_page_size)
         if page_size > max_page_size:
-            raise InvalidUsage("Invalid page size")
+            raise ApiException([Error(ErrorCode.QUERY_INVALID_PAGE_SIZE, page_size)])
     except ValueError:
-        raise InvalidUsage("Invalid page size")
+        raise ApiException([Error(ErrorCode.QUERY_INVALID_PAGE_SIZE, raw_page_size)])
+
+    raw_page = request.values.get('page[number]', 1)
     try:
-        page = int(request.values.get('page[number]', 1))
+        page = int(raw_page)
         if page < 1:
-            raise InvalidUsage("Invalid page number")
+            raise ApiException([Error(ErrorCode.QUERY_INVALID_PAGE_NUMBER, page)])
     except ValueError:
-        raise InvalidUsage("Invalid page number")
+        raise ApiException([Error(ErrorCode.QUERY_INVALID_PAGE_NUMBER, raw_page)])
+
     return page, page_size

api/ranked1v1.py

@@ -3,11 +3,12 @@
 from flask import request
 from pymysql.cursors import DictCursor
 
-from api import app, InvalidUsage
+from api import app
+from api.error import ApiException, ErrorCode
+from api.error import Error
 from api.query_commons import fetch_data
 from faf import db
 
-ALLOWED_EXTENSIONS = {'zip'}
 MAX_PAGE_SIZE = 5000
 
 SELECT_EXPRESSIONS = {
@@ -82,8 +83,9 @@ def ranked1v1():
     :status 200: No error
 
     """
-    if request.values.get('sort'):
-        raise InvalidUsage('Sorting is not supported for ranked1v1')
+    sort_field = request.values.get('sort')
+    if sort_field:
+        raise ApiException([Error(ErrorCode.QUERY_INVALID_SORT_FIELD, sort_field)])
 
     page = int(request.values.get('page[number]', 1))
     page_size = int(request.values.get('page[size]', MAX_PAGE_SIZE))

api/templates/authorize.html

@@ -134,7 +134,7 @@ <h2>{{ client.name }}</h2>
                     {% elif scope == "read_events" %}
                         Read events
                     {% elif scope == "upload_map" %}
-                    Upload maps
+                        Upload maps
                     {% endif %}
                 </li>
                 {% endfor %}

requirements.txt

@@ -11,7 +11,7 @@ pillow
 coveralls
 cryptography
 geoip2
-git+https://github.com/FAForever/faftools.git@777cf36c815b5aaf83faede713334f37196be106#egg=faftools
+git+https://github.com/FAForever/faftools.git@ad446d8bd6228a605de288c91e3fba01bb07e01d#egg=faftools
 marshmallow
 marshmallow_jsonapi
 peewee >= 2.4.2