From febe6ce8920a84ee0eae3b09cb9f67e57c4685c0 Mon Sep 17 00:00:00 2001 From: Qynn Swaan Date: Sun, 20 Aug 2023 17:19:23 -0400 Subject: [PATCH] provide more details with error responses --- bukuserver/api.py | 65 +++++++++++++++++----------------- bukuserver/forms.py | 43 +++++++++++++++-------- bukuserver/response.py | 23 +++++++----- tests/test_server.py | 79 +++++++++++++++++++++++++++++++++--------- 4 files changed, 139 insertions(+), 71 deletions(-) diff --git a/bukuserver/api.py b/bukuserver/api.py index 98bf1003..8b34412d 100644 --- a/bukuserver/api.py +++ b/bukuserver/api.py @@ -15,10 +15,10 @@ try: from response import Response - from forms import ApiBookmarkCreateForm, ApiBookmarkEditForm, ApiTagForm + from forms import ApiBookmarkCreateForm, ApiBookmarkEditForm, ApiBookmarkRangeEditForm, ApiTagForm except ImportError: from bukuserver.response import Response - from bukuserver.forms import ApiBookmarkCreateForm, ApiBookmarkEditForm, ApiTagForm + from bukuserver.forms import ApiBookmarkCreateForm, ApiBookmarkEditForm, ApiBookmarkRangeEditForm, ApiTagForm STATISTIC_DATA = None @@ -96,10 +96,9 @@ def get(self, tag: T.Optional[str]): def put(self, tag: str): form = ApiTagForm({}) - try: - form.process_data(request.get_json()) - except (ValueError, TypeError): - return Response.INPUT_NOT_VALID() + error_response, data = form.process_data(request.get_json()) + if error_response is not None: + return error_response(data=data) bukudb = get_bukudb() tags = search_tag(db=bukudb, stag=tag) if tag not in tags[1]: @@ -138,10 +137,9 @@ def get(self, rec_id: T.Union[int, None]): def post(self, rec_id: None = None): form = ApiBookmarkCreateForm({}) - try: - form.process_data(request.get_json()) - except (ValueError, TypeError): - return Response.INPUT_NOT_VALID() + error_response, error_data = form.process_data(request.get_json()) + if error_response is not None: + return error_response(data=error_data) bukudb = getattr(flask.g, 'bukudb', get_bukudb()) result_flag = bukudb.add_rec( form.url.data, @@ -152,10 +150,9 @@ def post(self, rec_id: None = None): def put(self, rec_id: int): form = ApiBookmarkEditForm({}) - try: - form.process_data(request.get_json()) - except (ValueError, TypeError): - return Response.INPUT_NOT_VALID() + error_response, error_data = form.process_data(request.get_json()) + if error_response is not None: + return error_response(data=error_data) bukudb = getattr(flask.g, 'bukudb', get_bukudb()) result_flag = bukudb.update_rec( rec_id, @@ -182,7 +179,7 @@ def get(self, starting_id: int, ending_id: int): bukudb = getattr(flask.g, 'bukudb', get_bukudb()) max_id = bukudb.get_max_id() or 0 if starting_id > ending_id or ending_id > max_id: - return Response.FAILURE() + return Response.RANGE_NOT_VALID() result = {'bookmarks': {i: entity(bukudb.get_rec_by_id(i)) for i in range(starting_id, ending_id + 1)}} return Response.SUCCESS(data=result) @@ -191,22 +188,28 @@ def put(self, starting_id: int, ending_id: int): bukudb = getattr(flask.g, 'bukudb', get_bukudb()) max_id = bukudb.get_max_id() or 0 if starting_id > ending_id or ending_id > max_id: - return Response.FAILURE() - - kwargs_list = [] + return Response.RANGE_NOT_VALID() + updates = [] + errors = {} for rec_id in range(starting_id, ending_id + 1): - form = ApiBookmarkEditForm({}) - try: - form.process_data(request.get_json().get(str(rec_id))) - except (ValueError, TypeError): - return Response.INPUT_NOT_VALID() - kwargs_list.append({'index': rec_id, - 'url': form.url.data, - 'title_in': form.title.data, - 'tags_in': form.tags_str, - 'desc': form.description.data}) - for kwargs in kwargs_list: - if not bukudb.update_rec(**kwargs): + json = request.get_json().get(str(rec_id)) + if json is None: + errors[rec_id] = 'Input required.' + continue + form = ApiBookmarkRangeEditForm({}) + error_response, error_data = form.process_data(json) + if error_response is not None: + errors[rec_id] = error_data.get('errors') + updates += [{'index': rec_id, + 'url': form.url.data, + 'title_in': form.title.data, + 'tags_in': form.tags_in, + 'desc': form.description.data}] + + if errors: + return Response.INPUT_NOT_VALID(data={'errors': errors}) + for update in updates: + if not bukudb.update_rec(**update): return Response.FAILURE() return Response.SUCCESS() @@ -214,7 +217,7 @@ def delete(self, starting_id: int, ending_id: int): bukudb = getattr(flask.g, 'bukudb', get_bukudb()) max_id = bukudb.get_max_id() or 0 if starting_id > ending_id or ending_id > max_id: - return Response.FAILURE() + return Response.RANGE_NOT_VALID() idx = min([starting_id, ending_id]) result_flag = bukudb.delete_rec(idx, starting_id, ending_id, is_range=True) return Response.from_flag(result_flag) diff --git a/bukuserver/forms.py b/bukuserver/forms.py index e2ad829b..6bab55bf 100644 --- a/bukuserver/forms.py +++ b/bukuserver/forms.py @@ -4,10 +4,13 @@ from wtforms.fields import BooleanField, FieldList, StringField, TextAreaField, HiddenField from wtforms.validators import DataRequired, InputRequired, ValidationError from buku import DELIM, parse_tags +from bukuserver.response import Response def validate_tag(form, field): + if not isinstance(field.data, str): + raise ValidationError('Tag must be a string.') if DELIM in field.data: - raise ValidationError("Tag must not contain delimiter ({})".format(DELIM)) + raise ValidationError('Tag must not contain delimiter \"{}\".'.format(DELIM)) class SearchBookmarksForm(FlaskForm): @@ -33,37 +36,49 @@ class ApiTagForm(FlaskForm): class Meta: csrf = False - tags = FieldList( - StringField(validators=[DataRequired(), validate_tag]), - min_entries=1, - ) + tags = FieldList(StringField(validators=[DataRequired(), validate_tag]), min_entries=1) tags_str = None - def process_data(self, data): + def process_data(self, data: dict) -> tuple[Response, dict]: + """Generate comma-separated string tags_str based on list of tags.""" tags = data.get('tags') if tags and not isinstance(tags, list): - raise TypeError("List of tags expected.") + return Response.INPUT_NOT_VALID, {'errors': {'tags': 'List of tags expected.'}} + super().process(data=data) if not self.validate(): - raise ValueError("Input data not valid.") + return Response.INPUT_NOT_VALID, {'errors': self.errors} + self.tags_str = None if tags is None else parse_tags([DELIM.join(tags)]) + return None, None class ApiBookmarkCreateForm(ApiTagForm): - class Meta: csrf = False url = StringField(validators=[DataRequired()]) title = StringField() description = StringField() - - tags = FieldList( - StringField(validators=[validate_tag]), - min_entries=0, - ) + tags = FieldList(StringField(validators=[validate_tag]), min_entries=0) class ApiBookmarkEditForm(ApiBookmarkCreateForm): url = StringField() + + +class ApiBookmarkRangeEditForm(ApiBookmarkEditForm): + + del_tags = BooleanField('Delete tags list from existing tags', default=False) + + tags_in = None + + def process_data(self, data: dict) -> tuple[Response, dict]: + """Generate comma-separated string tags_in based on list of tags.""" + error_response, data = super().process_data(data) + + if self.tags_str is not None: + self.tags_in = ("-" if self.del_tags.data else "+") + self.tags_str + + return error_response, data diff --git a/bukuserver/response.py b/bukuserver/response.py index fd55bba5..35e48f60 100644 --- a/bukuserver/response.py +++ b/bukuserver/response.py @@ -1,17 +1,23 @@ from enum import Enum from flask import jsonify -from flask_api import status +from flask_api.status import HTTP_200_OK, HTTP_400_BAD_REQUEST, HTTP_404_NOT_FOUND OK, FAIL = 0, 1 class Response(Enum): - SUCCESS = (status.HTTP_200_OK, "Success.") - FAILURE = (status.HTTP_400_BAD_REQUEST, "Failure.") - INPUT_NOT_VALID = (status.HTTP_400_BAD_REQUEST, "Input data not valid.") - BOOKMARK_NOT_FOUND = (status.HTTP_404_NOT_FOUND, "Bookmark not found.") - TAG_NOT_FOUND = (status.HTTP_404_NOT_FOUND, "Tag not found.") - TAG_NOT_VALID = (status.HTTP_400_BAD_REQUEST, "Invalid tag.") + SUCCESS = (HTTP_200_OK, "Success.") + FAILURE = (HTTP_400_BAD_REQUEST, "Failure.") + INPUT_NOT_VALID = (HTTP_400_BAD_REQUEST, "Input data not valid.") + BOOKMARK_NOT_FOUND = (HTTP_404_NOT_FOUND, "Bookmark not found.") + TAG_NOT_FOUND = (HTTP_404_NOT_FOUND, "Tag not found.") + RANGE_NOT_VALID = (HTTP_400_BAD_REQUEST, "Range not valid.") + TAG_NOT_VALID = (HTTP_400_BAD_REQUEST, "Invalid tag.") + + @staticmethod + def bad_request(message: str): + json = {'status': Response.FAILURE.status, 'message': message} + return (jsonify(json), Response.FAILURE.status_code, {'ContentType': 'application/json'}) @staticmethod def from_flag(flag: bool): @@ -27,7 +33,7 @@ def message(self) -> str: @property def status(self) -> int: - return OK if self.status_code == status.HTTP_200_OK else FAIL + return OK if self.status_code == HTTP_200_OK else FAIL def json(self, data :dict = None) -> dict: return dict(status=self.status, message=self.message, **data or {}) # pylint: disable=R1735 @@ -36,7 +42,6 @@ def __call__(self, *, data :dict = None): """Generates a tuple in the form (response, status, headers) If passed, data is added to the response's JSON. - """ return (jsonify(self.json(data)), self.status_code, {'ContentType': 'application/json'}) diff --git a/tests/test_server.py b/tests/test_server.py index 44bff496..06d8f358 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -92,9 +92,9 @@ def test_not_allowed(client, url, methods): ['delete', '/api/bookmarks/1', None, Response.FAILURE], ['post', '/api/bookmarks/1/refresh', None, Response.FAILURE], ['get', '/api/bookmarks/1/tiny', None, Response.FAILURE], - ['get', '/api/bookmarks/1/2', None, Response.FAILURE], - ['put', '/api/bookmarks/1/2', {1: {'title': 'one'}, 2: {'title': 'two'}}, Response.FAILURE], - ['delete', '/api/bookmarks/1/2', None, Response.FAILURE], + ['get', '/api/bookmarks/1/2', None, Response.RANGE_NOT_VALID], + ['put', '/api/bookmarks/1/2', {1: {'title': 'one'}, 2: {'title': 'two'}}, Response.RANGE_NOT_VALID], + ['delete', '/api/bookmarks/1/2', None, Response.RANGE_NOT_VALID], ] ) def test_invalid_id(client, method, url, json, exp_res): @@ -110,12 +110,17 @@ def test_tag_api(client): assert_response(rd, Response.SUCCESS, {'tags': ['tag1', 'tag2']}) rd = client.get('/api/tags/tag1') assert_response(rd, Response.SUCCESS, {'name': 'tag1', 'usage_count': 1}) - rd = client.put('/api/tags/tag1', json={}) - assert_response(rd, Response.INPUT_NOT_VALID) - rd = client.put('/api/tags/tag1', json={'tags': []}) - assert_response(rd, Response.INPUT_NOT_VALID) - rd = client.put('/api/tags/tag1', json={'tags': 'not a list'}) - assert_response(rd, Response.INPUT_NOT_VALID) + rd = client.put('/api/tags/tag1', json={'tags': 'string'}) + assert_response(rd, Response.INPUT_NOT_VALID, data={'errors': {'tags': 'List of tags expected.'}}) + for json in [{}, {'tags': None}, {'tags': ''}, {'tags':[]}]: + rd = client.put('/api/tags/tag1', json={'tags': []}) + assert_response(rd, Response.INPUT_NOT_VALID, data={'errors': {'tags': [['This field is required.']]}}) + rd = client.put('/api/tags/tag1', json={'tags': ['ok', '', None]}) + errors = {'tags': [[], ['This field is required.'], ['This field is required.']]} + assert_response(rd, Response.INPUT_NOT_VALID, data={'errors': errors}) + rd = client.put('/api/tags/tag1', json={'tags': ['one,two', 3,]}) + errors = {'tags': [['Tag must not contain delimiter \",\".'], ['Tag must be a string.']]} + assert_response(rd, Response.INPUT_NOT_VALID, data={'errors': errors}) rd = client.put('/api/tags/tag1', json={'tags': ['tag3', 'TAG 4']}) assert_response(rd, Response.SUCCESS) rd = client.get('/api/tags') @@ -137,7 +142,8 @@ def test_tag_api(client): def test_bookmark_api(client): url = 'http://google.com' rd = client.post('/api/bookmarks', json={}) - assert_response(rd, Response.INPUT_NOT_VALID) + errors = {'url': ['This field is required.']} + assert_response(rd, Response.INPUT_NOT_VALID, data={'errors': errors}) rd = client.post('/api/bookmarks', json={'url': url}) assert_response(rd, Response.SUCCESS) rd = client.post('/api/bookmarks', json={'url': url}) @@ -147,13 +153,17 @@ def test_bookmark_api(client): rd = client.get('/api/bookmarks/1') assert_response(rd, Response.SUCCESS, {'description': '', 'tags': [], 'title': 'Google', 'url': url}) rd = client.put('/api/bookmarks/1', json={'tags': 'not a list'}) - assert_response(rd, Response.INPUT_NOT_VALID) + assert_response(rd, Response.INPUT_NOT_VALID, data={'errors': {'tags': 'List of tags expected.'}}) rd = client.put('/api/bookmarks/1', json={'tags': ['tag1', 'tag2']}) assert_response(rd, Response.SUCCESS) rd = client.put('/api/bookmarks/1', json={}) assert_response(rd, Response.SUCCESS) rd = client.get('/api/bookmarks/1') assert_response(rd, Response.SUCCESS, {'description': '', 'tags': ['tag1', 'tag2'], 'title': 'Google', 'url': url}) + rd = client.put('/api/bookmarks/1', json={'tags': [], 'description': 'Description'}) + assert_response(rd, Response.SUCCESS) + rd = client.get('/api/bookmarks/1') + assert_response(rd, Response.SUCCESS, {'description': 'Description', 'tags': [], 'title': 'Google', 'url': url}) @pytest.mark.parametrize('d_url', ['/api/bookmarks', '/api/bookmarks/1']) @@ -216,15 +226,50 @@ def test_bookmark_range_api(client): for kwargs in kwargs_list: rd = client.post('/api/bookmarks', **kwargs) assert_response(rd, Response.SUCCESS) - rd = client.put('/api/bookmarks/1/2', json={1: {'tags': ['tag1 A', 'tag1 B']}, 2: {'tags': ['tag2']}}) + + rd = client.put('/api/bookmarks/1/2', json={ + 1: {'tags': ['tag1 A', 'tag1 B', 'tag1 C']}, + 2: {'tags': ['tag2']} + }) assert_response(rd, Response.SUCCESS) - rd = client.get('/api/bookmarks/2/1') - assert_response(rd, Response.FAILURE) rd = client.get('/api/bookmarks/1/2') assert_response(rd, Response.SUCCESS, {'bookmarks': { - '1': {'description': '', 'tags': ['tag1 a', 'tag1 b'], 'title': 'Google', 'url': 'http://google.com'}, - '2': {'description': '', 'tags': ['tag2'], 'title': 'Example Domain', 'url': 'http://example.com'}}}) - + '1': {'description': '', 'tags': ['tag1 a', 'tag1 b', 'tag1 c'], 'title': 'Google', 'url': 'http://google.com'}, + '2': {'description': '', 'tags': ['tag2',], 'title': 'Example Domain', 'url': 'http://example.com'}}}) + rd = client.put('/api/bookmarks/1/2', json={ + 1: {'title': 'Bookmark 1', 'tags': ['tag1 C', 'tag1 A'], 'del_tags': True}, + 2: {'title': 'Bookmark 2', 'tags': ['-', 'tag2'], 'del_tags': False} + }) + assert_response(rd, Response.SUCCESS) + rd = client.get('/api/bookmarks/1/2') + assert_response(rd, Response.SUCCESS, {'bookmarks': { + '1': {'description': '', 'tags': ['tag1 b'], 'title': 'Bookmark 1', 'url': 'http://google.com'}, + '2': {'description': '', 'tags': ['-', 'tag2',], 'title': 'Bookmark 2', 'url': 'http://example.com'}}}) + + rd = client.put('/api/bookmarks/2/1', json={}) + assert_response(rd, Response.RANGE_NOT_VALID) + + rd = client.put('/api/bookmarks/1/2', json={}) + assert_response(rd, Response.INPUT_NOT_VALID, data={ + 'errors': { + '1': 'Input required.', + '2': 'Input required.' + } + }) + rd = client.put('/api/bookmarks/1/2', json={1: {'tags': []}}) + assert_response(rd, Response.INPUT_NOT_VALID, data={'errors': {'2': 'Input required.'}}) + rd = client.put('/api/bookmarks/1/2', json={ + 1: {'tags': ['ok', 'with,delim']}, + 2: {'tags': 'string'}, + }) + assert_response(rd, Response.INPUT_NOT_VALID, data={ + 'errors': { + '1': {'tags': [[], ['Tag must not contain delimiter \",\".']]}, + '2': {'tags': 'List of tags expected.'} + } + }) + rd = client.get('/api/bookmarks/2/1') + assert_response(rd, Response.RANGE_NOT_VALID) rd = client.delete('/api/bookmarks/1/2') assert_response(rd, Response.SUCCESS) rd = client.get('/api/bookmarks')