From aec9d6a59da7acf11a3df11d743496c61d628e05 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 24 May 2013 16:59:20 -0400 Subject: [PATCH] Fixes issue #51. All view methods return dictionaries representing the JSON response, and are rendered into real HTTP response objects via the jsonpify() function as applied by the mimerender decorator. --- CHANGES | 2 + docs/installation.rst | 1 + flask_restless/views.py | 122 +++++++++++++++++++++++++++------------- requirements.txt | 1 + setup.py | 3 +- tests/test_views.py | 19 ++++--- 6 files changed, 101 insertions(+), 47 deletions(-) diff --git a/CHANGES b/CHANGES index ee7c8887..0d06c337 100644 --- a/CHANGES +++ b/CHANGES @@ -27,6 +27,8 @@ Not yet released. - #29: replace custom ``jsonify_status_code`` function with built-in support for ``return jsonify(), status_code`` style return statements (new in Flask 0.9). +- #51: Use `mimerender `_ to render + dictionaries to JSON format. - #256: makes search parameters available to postprocessors for :http:method:`get` and :http:method:`patch` requests that access multiple resources. diff --git a/docs/installation.rst b/docs/installation.rst index 1eb437e0..5cad9eae 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -17,6 +17,7 @@ installed if you use ``pip``): * `Flask `_ version 0.10 or greater * `SQLAlchemy `_ version 0.8 or greater +* `mimerender `_ version 0.5.2 or greater * `python-dateutil `_ version strictly greater than 2.0 * `Flask-SQLAlchemy `_, *only if* diff --git a/flask_restless/views.py b/flask_restless/views.py index 5b8ff492..cf434f24 100644 --- a/flask_restless/views.py +++ b/flask_restless/views.py @@ -34,6 +34,7 @@ from flask import jsonify as _jsonify from flask import request from flask.views import MethodView +from mimerender import FlaskMimeRender from sqlalchemy.exc import DataError from sqlalchemy.exc import IntegrityError from sqlalchemy.exc import OperationalError @@ -68,6 +69,14 @@ #: Format string for creating Link headers in paginated responses. LINKTEMPLATE = '<{0}?page={1}&results_per_page={2}>; rel="{3}"' +#: String used internally as a dictionary key for passing header information +#: from view functions to the :func:`jsonpify` function. +_HEADERS = '__restless_headers' + +#: String used internally as a dictionary key for passing status code +#: information from view functions to the :func:`jsonpify` function. +_STATUS = '__restless_status_code' + class ProcessingException(HTTPException): """Raised when a preprocessor or postprocessor encounters a problem. @@ -223,24 +232,39 @@ def jsonpify(*args, **kw): function specified as a query parameter called ``'callback'`` (or does nothing if no such callback function is specified in the request). - If `headers` is specified, it must be a dictionary specifying headers to - set before sending the JSONified response to the client. Headers on the - response will be overwritten by headers specified in the `headers` - dictionary. + If the keyword arguments include the string specified by :data:`_HEADERS`, + its value must be a dictionary specifying headers to set before sending the + JSONified response to the client. Headers on the response will be + overwritten by headers specified in this dictionary. + + If the keyword arguments include the string specified by :data:`_STATUS`, + its value must be an integer representing the status code of the response. + Otherwise, the status code of the response will be :http:status:`200`. """ - headers = kw.pop('headers', None) - # FIXME jsonify already sets headers, so we are doing it twice... + # HACK In order to make the headers and status code available in the + # content of the response, we need to send it from the view function to + # this jsonpify function via its keyword arguments. This is a limitation of + # the mimerender library: it has no way of making the headers and status + # code known to the rendering functions. + headers = kw.pop(_HEADERS, {}) + status_code = kw.pop(_STATUS, 200) response = jsonify(*args, **kw) callback = request.args.get('callback', False) if callback: # Reload the data from the constructed JSON string so we can wrap it in # a JSONP function. data = json.loads(response.data) - # Add the headers as metadata to the JSONP response. + # Force the 'Content-Type' header to be 'application/javascript'. + # + # Note that this is different from the mimetype used in Flask for JSON + # responses; Flask uses 'application/json'. We use + # 'application/javascript' because a JSONP response is valid + # Javascript, but not valid JSON. + headers['Content-Type'] = 'application/javascript' + # Add the headers and status code as metadata to the JSONP response. meta = _headers_to_json(headers) if headers is not None else {} - # TODO add a jsonpify_status_code function? - meta['status'] = 200 + meta['status'] = status_code inner = json.dumps(dict(meta=meta, data=data)) content = '{0}({1})'.format(callback, inner) # Note that this is different from the mimetype used in Flask for JSON @@ -251,6 +275,7 @@ def jsonpify(*args, **kw): # Set the headers on the HTTP response as well. if headers: set_headers(response, headers) + response.status_code = status_code return response @@ -315,6 +340,17 @@ def _parse_excludes(column_names): del relations[column] return columns, relations +#: Creates the mimerender object necessary for decorating responses with a +#: function that automatically formats the dictionary in the appropriate format +#: based on the ``Accept`` header. +#: +#: Technical details: the first pair of parantheses instantiates the +#: :class:`mimerender.FlaskMimeRender` class. The second pair of parentheses +#: creates the decorator, so that we can simply use the variable ``mimerender`` +#: as a decorator. +mimerender = FlaskMimeRender()(default='json', json=jsonpify, + xml=lambda: None) # TODO fill in xml renderer + class ModelView(MethodView): """Base class for :class:`flask.MethodView` classes which represent a view @@ -332,6 +368,9 @@ class ModelView(MethodView): """ + #: List of decorators applied to every method of this class. + decorators = [mimerender] + def __init__(self, session, model, *args, **kw): """Calls the constructor of the superclass and specifies the model for which this class provides a ReSTful API. @@ -374,27 +413,27 @@ def get(self): """ if 'q' not in request.args or not request.args.get('q'): - return jsonify(message='Empty query parameter'), 400 + return dict(message='Empty query parameter'), 400 # if parsing JSON fails, return a 400 error in JSON format try: data = json.loads(str(request.args.get('q'))) or {} except (TypeError, ValueError, OverflowError) as exception: current_app.logger.exception(str(exception)) - return jsonify(message='Unable to decode data'), 400 + return dict(message='Unable to decode data'), 400 try: result = evaluate_functions(self.session, self.model, data.get('functions', [])) if not result: - return jsonpify(), 204 - return jsonpify(result) + return {}, 204 + return result except AttributeError as exception: current_app.logger.exception(str(exception)) message = 'No such field "{0}"'.format(exception.field) - return jsonify(message=message), 400 + return dict(message=message), 400 except OperationalError as exception: current_app.logger.exception(str(exception)) message = 'No such function "{0}"'.format(exception.function) - return jsonify(message=message), 400 + return dict(message=message), 400 class API(ModelView): @@ -406,7 +445,7 @@ class API(ModelView): """ #: List of decorators applied to every method of this class. - decorators = [catch_processing_exceptions] + decorators = ModelView.decorators + [catch_processing_exceptions] def __init__(self, session, model, exclude_columns=None, include_columns=None, include_methods=None, @@ -734,7 +773,7 @@ def _handle_validation_exception(self, exception): self.session.rollback() errors = self._extract_error_messages(exception) or \ 'Could not determine specific validation errors' - return jsonify(validation_errors=errors), 400 + return dict(validation_errors=errors), 400 def _extract_error_messages(self, exception): """Tries to extract a dictionary mapping field name to validation error @@ -865,7 +904,7 @@ def _instid_to_dict(self, instid): """ inst = get_by(self.session, self.model, instid, self.primary_key) if inst is None: - abort(404) + return {_STATUS: 404}, 404 return self._inst_to_dict(inst) def _search(self): @@ -940,7 +979,7 @@ def _search(self): search_params = json.loads(request.args.get('q', '{}')) except (TypeError, ValueError, OverflowError) as exception: current_app.logger.exception(str(exception)) - return jsonify(message='Unable to decode data'), 400 + return dict(message='Unable to decode data'), 400 for preprocessor in self.preprocessors['GET_MANY']: preprocessor(search_params=search_params) @@ -949,12 +988,12 @@ def _search(self): try: result = search(self.session, self.model, search_params) except NoResultFound: - return jsonify(message='No result found'), 400 + return dict(message='No result found'), 400 except MultipleResultsFound: - return jsonify(message='Multiple results found'), 400 + return dict(message='Multiple results found'), 400 except Exception as exception: current_app.logger.exception(str(exception)) - return jsonify(message='Unable to construct query'), 400 + return dict(message='Unable to construct query'), 400 # create a placeholder for the relations of the returned models relations = frozenset(get_relations(self.model)) @@ -993,7 +1032,11 @@ def _search(self): for postprocessor in self.postprocessors['GET_MANY']: postprocessor(result=result, search_params=search_params) - return jsonpify(result, headers=headers) + # HACK Provide the headers directly in the result dictionary, so that + # the :func:`jsonpify` function has access to them. See the note there + # for more information. + result[_HEADERS] = headers + return result, 200, headers def get(self, instid, relationname, relationinstid): """Returns a JSON representation of an instance of model with the @@ -1016,7 +1059,7 @@ def get(self, instid, relationname, relationinstid): # get the instance of the "main" model whose ID is instid instance = get_by(self.session, self.model, instid, self.primary_key) if instance is None: - abort(404) + return {_STATUS: 404}, 404 # If no relation is requested, just return the instance. Otherwise, # get the value of the relation specified by `relationname`. if relationname is None: @@ -1041,7 +1084,7 @@ def get(self, instid, relationname, relationinstid): result = to_dict(related_value, deep) for postprocessor in self.postprocessors['GET_SINGLE']: postprocessor(result=result) - return jsonpify(result) + return result def delete(self, instid, relationname, relationinstid): """Removes the specified instance of the model with the specified name @@ -1070,7 +1113,7 @@ def delete(self, instid, relationname, relationinstid): if not relationinstid: msg = ('Cannot DELETE entire "{0}"' ' relation').format(relationname) - return jsonify(message=msg), 400 + return dict(message=msg), 400 # Otherwise, get the related instance to delete. relation = getattr(inst, relationname) related_model = get_related_model(self.model, relationname) @@ -1084,7 +1127,7 @@ def delete(self, instid, relationname, relationinstid): is_deleted = True for postprocessor in self.postprocessors['DELETE']: postprocessor(is_deleted=is_deleted) - return jsonify(), 204 + return {}, 204 def post(self): """Creates a new instance of a given model based on request data. @@ -1115,7 +1158,7 @@ def post(self): # issue #267). if not is_msie and not content_is_json: msg = 'Request must have "Content-Type: application/json" header' - return jsonify(message=msg), 415 + return dict(message=msg), 415 # try to read the parameters for the model from the body of the request try: @@ -1127,7 +1170,7 @@ def post(self): params = request.get_json() or {} except (BadRequest, TypeError, ValueError, OverflowError) as exception: current_app.logger.exception(str(exception)) - return jsonify(message='Unable to decode data'), 400 + return dict(message='Unable to decode data'), 400 # apply any preprocessors to the POST arguments for preprocessor in self.preprocessors['POST']: @@ -1138,7 +1181,7 @@ def post(self): for field in params: if not has_field(self.model, field): msg = "Model does not have field '{0}'".format(field) - return jsonify(message=msg), 400 + return dict(message=msg), 400 # Getting the list of relations that will be added later cols = get_columns(self.model) @@ -1191,12 +1234,12 @@ def post(self): for postprocessor in self.postprocessors['POST']: postprocessor(result=result) - return jsonify(headers=headers, **result), 201 + return result, 201, headers except self.validation_exceptions as exception: return self._handle_validation_exception(exception) except (DataError, IntegrityError, ProgrammingError) as exception: current_app.logger.exception(str(exception)) - return jsonify(message=str(exception)), 400 + return dict(message=str(exception)), 400 def patch(self, instid, relationname, relationinstid): """Updates the instance specified by ``instid`` of the named model, or @@ -1232,7 +1275,7 @@ def patch(self, instid, relationname, relationinstid): # issue #267). if not is_msie and not content_is_json: msg = 'Request must have "Content-Type: application/json" header' - return jsonify(message=msg), 415 + return dict(message=msg), 415 # try to load the fields/values to update from the body of the request try: @@ -1245,7 +1288,7 @@ def patch(self, instid, relationname, relationinstid): except (BadRequest, TypeError, ValueError, OverflowError) as exception: # this also happens when request.data is empty current_app.logger.exception(str(exception)) - return jsonify(message='Unable to decode data'), 400 + return dict(message='Unable to decode data'), 400 # Check if the request is to patch many instances of the current model. patchmany = instid is None @@ -1265,19 +1308,20 @@ def patch(self, instid, relationname, relationinstid): for field in data: if not has_field(self.model, field): msg = "Model does not have field '{0}'".format(field) - return jsonify(message=msg), 400 + return dict(message=msg), 400 + if patchmany: try: # create a SQLALchemy Query from the query parameter `q` query = create_query(self.session, self.model, search_params) except Exception as exception: current_app.logger.exception(str(exception)) - return jsonify(message='Unable to construct query'), 400 + return dict(message='Unable to construct query'), 400 else: # create a SQLAlchemy Query which has exactly the specified row query = query_by_primary_key(self.session, self.model, instid) if query.count() == 0: - abort(404) + return {_STATUS: 404}, 404 assert query.count() == 1, 'Multiple rows with same ID' relations = self._update_relations(query, data) @@ -1302,7 +1346,7 @@ def patch(self, instid, relationname, relationinstid): return self._handle_validation_exception(exception) except (DataError, IntegrityError, ProgrammingError) as exception: current_app.logger.exception(str(exception)) - return jsonify(message=str(exception)), 400 + return dict(message=str(exception)), 400 # Perform any necessary postprocessing. if patchmany: @@ -1315,7 +1359,7 @@ def patch(self, instid, relationname, relationinstid): for postprocessor in self.postprocessors['PATCH_SINGLE']: postprocessor(result=result) - return jsonify(result) + return result def put(self, *args, **kw): """Alias for :meth:`patch`.""" diff --git a/requirements.txt b/requirements.txt index c2785d9d..60fc8ff5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,3 +2,4 @@ flask>=0.10 flask-sqlalchemy sqlalchemy>=0.8 python-dateutil>2.0 +mimerender>=0.5.2 diff --git a/setup.py b/setup.py index 666d5fff..ee95d3e4 100644 --- a/setup.py +++ b/setup.py @@ -19,7 +19,8 @@ #: The installation requirements for Flask-Restless. Flask-SQLAlchemy is not #: required, so the user must install it explicitly. -requirements = ['flask>=0.10', 'sqlalchemy>=0.8', 'python-dateutil>2.0'] +requirements = ['flask>=0.10', 'sqlalchemy>=0.8', 'python-dateutil>2.0', + 'mimerender>=0.5.2'] setup( diff --git a/tests/test_views.py b/tests/test_views.py index 1abd07c7..ecffef9b 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -241,6 +241,7 @@ def test_jsonp(self): response = self.app.get('/api/eval/person?' 'q={0}&callback=baz'.format(query)) assert response.status_code == 200 + assert response.mimetype == 'application/javascript' assert response.data.startswith(b'baz(') assert response.data.endswith(b')') @@ -1428,21 +1429,25 @@ def test_accept(self): """ # A request without an Accept header should return JSON. - response = self.app.get('/api/person/1') + headers = None + response = self.app.get('/api/person/1', headers=headers) assert 200 == response.status_code assert 'Content-Type' in response.headers assert 'application/json' == response.headers['Content-Type'] assert 1 == loads(response.data)['id'] - response = self.app.get('/api/person/1', - headers=dict(Accept='application/json')) + headers = dict(Accept='application/json') + response = self.app.get('/api/person/1', headers=headers) assert 200 == response.status_code assert 'Content-Type' in response.headers assert 'application/json' == response.headers['Content-Type'] assert 1 == loads(response.data)['id'] - #headers = dict(Accept='application/xml') - #assert 'Content-Type' in response.headers - #assert 'application/xml' == response.headers['Content-Type'] - #assert '1' in response.data + # Check for accepting XML. + # headers = dict(Accept='application/xml') + # response = self.app.get('/api/person/1', headers=headers) + # assert 200 == response.status_code + # assert 'Content-Type' in response.headers + # assert 'application/xml' == response.headers['Content-Type'] + # assert '1' in response.data class TestSearch(TestSupportPrefilled):