From 69f38adb830e29d77eefee9fd71643d7a3ffd411 Mon Sep 17 00:00:00 2001 From: Aengus Walton Date: Fri, 4 Mar 2016 10:06:57 +0100 Subject: [PATCH] Fixes #476: Use user-defined primary key to generate URLs Instead of automatically taking the model's database primary key, flask-restless should use the primary key specified in the call to `create_api`, if one was specified, when generating URLs. This commit ensures this behaviour. --- CHANGES | 2 + docs/api.rst | 2 + flask_restless/__init__.py | 1 + flask_restless/helpers.py | 92 ++++++++++++++++++++++++++------- flask_restless/manager.py | 22 ++++++-- flask_restless/serialization.py | 4 +- flask_restless/views/base.py | 4 +- tests/test_manager.py | 19 +++++++ 8 files changed, 121 insertions(+), 25 deletions(-) diff --git a/CHANGES b/CHANGES index b4e76841..d271d592 100644 --- a/CHANGES +++ b/CHANGES @@ -38,6 +38,8 @@ Not yet released. - #474: include license files in built wheel for distribution. - #501: allows empty string for `url_prefix` keyword argument to :meth:`APIManager.create_api`. +- #476: use the primary key provided at the time of invoking + :meth:`APIManager.create_api` to build resource urls in responses. Older versions -------------- diff --git a/docs/api.rst b/docs/api.rst index aa7ffd3f..554ccdab 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -26,6 +26,8 @@ Global helper functions .. autofunction:: serializer_for(model, _apimanager=None) +.. autofunction:: primary_key_for(model, _apimanager=None) + .. autofunction:: url_for(model, instid=None, relationname=None, relationinstid=None, _apimanager=None, **kw) Serialization helpers diff --git a/flask_restless/__init__.py b/flask_restless/__init__.py index e4202bfc..d5b62cd8 100644 --- a/flask_restless/__init__.py +++ b/flask_restless/__init__.py @@ -26,6 +26,7 @@ from .helpers import model_for from .helpers import serializer_for from .helpers import url_for +from .helpers import primary_key_for from .manager import APIManager from .manager import IllegalArgumentError from .serialization import SerializationException diff --git a/flask_restless/helpers.py b/flask_restless/helpers.py index 2d44911d..69960ffd 100644 --- a/flask_restless/helpers.py +++ b/flask_restless/helpers.py @@ -184,22 +184,6 @@ def primary_key_names(model): and field.property.columns[0].primary_key] -def primary_key_name(model_or_instance): - """Returns the name of the primary key of the specified model or instance - of a model, as a string. - - If `model_or_instance` specifies multiple primary keys and ``'id'`` is one - of them, ``'id'`` is returned. If `model_or_instance` specifies multiple - primary keys and ``'id'`` is not one of them, only the name of the first - one in the list of primary keys is returned. - - """ - its_a_model = isinstance(model_or_instance, type) - model = model_or_instance if its_a_model else model_or_instance.__class__ - pk_names = primary_key_names(model) - return 'id' if 'id' in pk_names else pk_names[0] - - def primary_key_value(instance, as_string=False): """Returns the value of the primary key field of the specified `instance` of a SQLAlchemy model. @@ -211,7 +195,7 @@ def primary_key_value(instance, as_string=False): If `as_string` is ``True``, try to coerce the return value to a string. """ - result = getattr(instance, primary_key_name(instance)) + result = getattr(instance, primary_key_for(instance)) if not as_string: return result try: @@ -265,7 +249,7 @@ def query_by_primary_key(session, model, pk_value, primary_key=None): Presumably, the returned query should have at most one element. """ - pk_name = primary_key or primary_key_name(model) + pk_name = primary_key or primary_key_for(model) query = session_query(session, model) return query.filter(getattr(model, pk_name) == pk_value) @@ -488,6 +472,42 @@ def __call__(self, model, _apimanager=None, **kw): raise ValueError(message) +class PrimaryKeyFinder(KnowsAPIManagers, Singleton): + """The singleton class that backs the :func:`primary_key_for` function.""" + + def __call__(self, instance_or_model, _apimanager=None, **kw): + if isinstance(instance_or_model, type): + model = instance_or_model + else: + model = instance_or_model.__class__ + + if _apimanager is not None: + managers_to_search = [_apimanager] + else: + managers_to_search = self.created_managers + for manager in managers_to_search: + if model in manager.created_apis_for: + primary_key = manager.primary_key_for(model, **kw) + break + else: + message = ('Model "{0}" is not known to {1}; maybe you have not' + ' called APIManager.create_api() for this model?') + if _apimanager is not None: + manager_string = 'APIManager "{0}"'.format(_apimanager) + else: + manager_string = 'any APIManager objects' + message = message.format(model, manager_string) + raise ValueError(message) + + # If `APIManager.create_api(model)` was called without providing + # a value for the `primary_key` keyword argument, then we must + # compute the primary key name from the model directly. + if primary_key is None: + pk_names = primary_key_names(model) + primary_key = 'id' if 'id' in pk_names else pk_names[0] + return primary_key + + #: Returns the URL for the specified model, similar to :func:`flask.url_for`. #: #: `model` is a SQLAlchemy model class. This should be a model on which @@ -622,3 +642,39 @@ def __call__(self, model, _apimanager=None, **kw): #: #: model_for = ModelFinder() + +#: Returns the primary key to be used for the given model or model instance, +#: as specified by the ``primary_key`` keyword argument to +#: :meth:`APIManager.create_api` when it was previously invoked on the model. +#: +#: `primary_key` is a string corresponding to the primary key identifier +#: to be used by flask-restless for a model. If no primary key has been set +#: at the flask-restless level (by using the ``primary_key`` keyword argument +#: when calling :meth:`APIManager.create_api_blueprint`, the model's primary +#: key will be returned. If no API has been created for the model, this +#: function raises a `ValueError`. +#: +#: If `_apimanager` is not ``None``, it must be an instance of +#: :class:`APIManager`. Restrict our search for endpoints exposing `model` to +#: only endpoints created by the specified :class:`APIManager` instance. +#: +#: For example, suppose you have a model class ``Person`` and have created the +#: appropriate Flask application and SQLAlchemy session:: +#: +#: >>> from mymodels import Person +#: >>> manager = APIManager(app, session=session) +#: >>> manager.create_api(Person, primary_key='name') +#: >>> primary_key_for(Person) +#: 'name' +#: >>> my_person = Person(name="Bob") +#: >>> primary_key_for(my_person) +#: 'name' +#: +#: This is in contrast to the typical default: +#: +#: >>> manager = APIManager(app, session=session) +#: >>> manager.create_api(Person) +#: >>> primary_key_for(Person) +#: 'id' +#: +primary_key_for = PrimaryKeyFinder() diff --git a/flask_restless/manager.py b/flask_restless/manager.py index 00a3b469..f0ecd1d1 100644 --- a/flask_restless/manager.py +++ b/flask_restless/manager.py @@ -24,8 +24,9 @@ from flask import url_for as flask_url_for from .helpers import collection_name -from .helpers import serializer_for from .helpers import model_for +from .helpers import primary_key_for +from .helpers import serializer_for from .helpers import url_for from .serialization import DefaultSerializer from .serialization import DefaultDeserializer @@ -70,8 +71,10 @@ #: - `blueprint_name`, the name of the blueprint that contains this API, #: - `serializer`, the subclass of :class:`Serializer` provided for the #: model exposed by this API. +#: - `primary_key`, the primary key used by the model #: -APIInfo = namedtuple('APIInfo', 'collection_name blueprint_name serializer') +APIInfo = namedtuple('APIInfo', ['collection_name', 'blueprint_name', 'serializer', + 'primary_key']) class IllegalArgumentError(Exception): @@ -169,6 +172,7 @@ def __init__(self, app=None, session=None, flask_sqlalchemy_db=None, model_for.register(self) collection_name.register(self) serializer_for.register(self) + primary_key_for.register(self) #: A mapping whose keys are models for which this object has #: created an API via the :meth:`create_api_blueprint` method @@ -305,6 +309,18 @@ def serializer_for(self, model): """ return self.created_apis_for[model].serializer + def primary_key_for(self, model): + """Returns the primary key for the specified model, as specified + by the `primary_key` keyword argument to + :meth:`create_api_blueprint`. + + `model` is a SQLAlchemy model class. This must be a model on + which :meth:`create_api_blueprint` has been invoked previously, + otherwise a :exc:`KeyError` is raised. + + """ + return self.created_apis_for[model].primary_key + def init_app(self, app): """Registers any created APIs on the given Flask application. @@ -754,7 +770,7 @@ def create_api_blueprint(self, name, model, methods=READONLY_METHODS, # Finally, record that this APIManager instance has created an API for # the specified model. self.created_apis_for[model] = APIInfo(collection_name, blueprint.name, - serializer) + serializer, primary_key) return blueprint def create_api(self, *args, **kw): diff --git a/flask_restless/serialization.py b/flask_restless/serialization.py index 4f91405b..0b01d776 100644 --- a/flask_restless/serialization.py +++ b/flask_restless/serialization.py @@ -47,7 +47,7 @@ from .helpers import get_relations from .helpers import has_field from .helpers import is_like_list -from .helpers import primary_key_name +from .helpers import primary_key_for from .helpers import primary_key_value from .helpers import serializer_for from .helpers import strings_to_datetimes @@ -614,7 +614,7 @@ def __call__(self, instance, only=None): # If the primary key is not named "id", we'll duplicate the # primary key under the "id" key. - pk_name = primary_key_name(model) + pk_name = primary_key_for(model) if pk_name != 'id': result['id'] = result['attributes'][pk_name] # TODO Same problem as above. diff --git a/flask_restless/views/base.py b/flask_restless/views/base.py index 092fa289..f5732004 100644 --- a/flask_restless/views/base.py +++ b/flask_restless/views/base.py @@ -51,7 +51,7 @@ from ..helpers import collection_name from ..helpers import get_model from ..helpers import is_like_list -from ..helpers import primary_key_name +from ..helpers import primary_key_for from ..helpers import primary_key_value from ..helpers import serializer_for from ..helpers import url_for @@ -1623,7 +1623,7 @@ def _get_collection_helper(self, resource=None, relation_name=None, result['data'] = serialize(data, only=only) except SerializationException as exception: return errors_from_serialization_exceptions([exception]) - primary_key = self.primary_key or primary_key_name(data) + primary_key = primary_key_for(data) pk_value = result['data'][primary_key] # The URL at which a client can access the instance matching this # search query. diff --git a/tests/test_manager.py b/tests/test_manager.py index 2aa070bf..3cf8add5 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -36,6 +36,7 @@ from .helpers import ManagerTestBase from .helpers import FlaskTestBase from .helpers import force_content_type_jsonapi +from .helpers import loads from .helpers import skip from .helpers import skip_unless from .helpers import unregister_fsa_session_signals @@ -327,6 +328,24 @@ def test_url_for(self): assert url3.endswith('/api/people/1/articles') assert url4.endswith('/api/people/1/articles/2') + def test_url_for_explicitly_sets_primary_key_in_links(self): + """Should use the primary_key explicitly set when generating links""" + article = self.Article(id=1, title=u'my_article') + self.session.add(article) + self.session.commit() + self.manager.create_api(self.Article, primary_key='title') + + response = self.app.get('/api/article') + document = loads(response.data) + articles = document['data'] + article = articles[0] + + assert 'my_article' in article['links']['self'] + assert '/1' not in article['links']['self'] + author_links = article['relationships']['author']['links'] + assert author_links['self'] == ( + '/api/article/my_article/relationships/author') + @raises(ValueError) def test_url_for_nonexistent(self): """Tests that attempting to get the URL for an unknown model yields an