Merge pull request #92 from postatum/99832394_self_route_self_param

Rework _self response param generation

docs/source/views.rst

@@ -74,8 +74,16 @@ It is recommended that your views reside in a package:
 For singular resources:
     there is no need to define ``index()``
 
-When using SQLA, each view must define the following property:
-    *Model*: model being served by the current view
+Each view must define the following property:
+    *Model*: model being served by the current view. Must be set at class definition for features to work properly. E.g.:
+
+.. code-block:: python
+
+    from nefertari.view import BaseView
+    from example_api.models import Story
+
+    class StoriesView(BaseView):
+        Model = Story
 
 Optional properties:
     *_json_encoder*: encoder to encode objects to JSON. Database-specific encoders are available at ``nefertari.engine.JSONEncoder``

nefertari/__init__.py

@@ -25,6 +25,9 @@ def includeme(config):
         config.registry._root_resources = {}
     if not hasattr(config.registry, '_resources_map'):
         config.registry._resources_map = {}
+    # Map of {ModelName: model_collection_resource}
+    if not hasattr(config.registry, '_model_collections'):
+        config.registry._model_collections = {}
 
     config.add_request_method(get_resource_map, 'resource_map', reify=True)
 

nefertari/polymorphic.py

@@ -10,8 +10,6 @@
         ACLs of all the collections requested.
     * PolymorphicESView: View that handles polymorphic collection read
         requests.
-    * add_url_polymorphic: Wrapper class that is used instead of default
-        `nefertari.wrappers.add_object_url`.
 
 To use this module, simply include it in your `main()` after
 Pyramid ACLAuthorizationPolicy is set up and nefertari is included.
@@ -34,7 +32,6 @@
 from nefertari.view import BaseView
 from nefertari.acl import BaseACL
 from nefertari.utils import dictset
-from nefertari import wrappers
 
 
 def includeme(config):
@@ -51,7 +48,6 @@ class PolymorphicHelperMixin(object):
     """ Helper mixin class that contains methods used by:
         * PolymorphicACL
         * PolymorphicESView
-        * add_url_polymorphic wrapper
     """
     def get_collections(self):
         """ Get names of collections from request matchdict.
@@ -72,10 +68,9 @@ def get_resources(self, collections):
         :return: Gathered resources
         :rtype: list of Resource instances
         """
-        res_map = self.request.registry._resources_map
+        res_map = self.request.registry._model_collections
         resources = [res for res in res_map.values()
-                     if res.collection_name in collections
-                     and not res.is_singular]
+                     if res.collection_name in collections]
         resources = [res for res in resources if res]
         return set(resources)
 
@@ -156,18 +151,6 @@ def _run_init_actions(self):
         self.setup_default_wrappers()
         self.set_public_limits()
 
-    def setup_default_wrappers(self):
-        self._after_calls['index'] = [
-            wrappers.wrap_in_dict(self.request),
-            wrappers.add_meta(self.request),
-            add_url_polymorphic(self.request),
-            wrappers.add_etag(self.request),
-        ]
-        if self._auth_enabled:
-            self._after_calls['index'] += [
-                wrappers.apply_privacy(self.request),
-            ]
-
     def determine_types(self):
         """ Determine ES type names from request data.
 
@@ -191,35 +174,3 @@ def index(self, collections):
         """
         self._query_params.process_int_param('_limit', 20)
         return self.get_collection_es()
-
-
-class add_url_polymorphic(PolymorphicHelperMixin, wrappers.add_object_url):
-    """ Wrapper that adds '_self' to each object in results
-
-    For each object in `result['data']` adds a uri which points
-    to current object
-    """
-    def get_models_map(self):
-        """ Get map of {model_name: resource} for each collection
-        requested by request.
-        """
-        collections = self.get_collections()
-        resources = self.get_resources(collections)
-        return {res.view.Model.__name__: res for res in resources}
-
-    def _set_object_self(self, obj):
-        """ Override to generate urls instead of just concatenating.
-
-        '_self' key is not set for singular resources.
-        """
-        try:
-            type_, obj_pk = obj['_type'], obj['_pk']
-        except KeyError:
-            return
-        resource = self.model_resources[type_]
-        obj['_self'] = self.request.route_url(
-            resource.uid, **{resource.id_name: obj_pk})
-
-    def __call__(self, **kwargs):
-        self.model_resources = self.get_models_map()
-        return super(add_url_polymorphic, self).__call__(**kwargs)

nefertari/resource.py

@@ -222,6 +222,8 @@ def get_ancestors(self):
 
     ancestors = property(get_ancestors)
     resource_map = property(lambda self: self.config.registry._resources_map)
+    model_collections = property(
+        lambda self: self.config.registry._model_collections)
     is_root = property(lambda self: not self.member_name)
     is_singular = property(
         lambda self: not self.is_root and not self.collection_name)
@@ -332,6 +334,18 @@ def add(self, member_name, collection_name='', parent=None, uid='',
             list(new_resource.action_route_map.values()),
             new_resource))
 
+        # Store resources in {modelName: resource} map if:
+        #   * Its view has Model defined
+        #   * It's not singular
+        #   * Its parent is root or it's not already stored
+        model = new_resource.view.Model
+        is_collection = model is not None and not new_resource.is_singular
+        if is_collection:
+            is_needed = (model.__name__ not in self.model_collections or
+                         new_resource.parent is root_resource)
+            if is_needed:
+                self.model_collections[model.__name__] = new_resource
+
         parent.children.append(new_resource)
         view._resource = new_resource
         view._factory = _factory

nefertari/wrappers.py

@@ -2,7 +2,6 @@
 from hashlib import md5
 
 import six
-from six.moves import urllib
 from nefertari import engine
 
 
@@ -256,28 +255,23 @@ class add_object_url(object):
     For each object in `result['data']` adds a uri which points
     to current object
     """
-    _is_singular = None
-
     def __init__(self, request):
         self.request = request
-
-    @property
-    def is_singular(self):
-        if self._is_singular is None:
-            route_name = self.request.matched_route.name
-            resource = self.request.registry._resources_map[route_name]
-            self._is_singular = resource.is_singular
-        return self._is_singular
+        self.model_collections = self.request.registry._model_collections
 
     def _set_object_self(self, obj):
         """ Add '_self' key value to :obj: dict. """
+        from nefertari.elasticsearch import ES
         location = self.request.path_url
         try:
-            obj_pk = urllib.parse.quote(str(obj['_pk']))
+            type_, obj_pk = obj['_type'], obj['_pk']
         except KeyError:
             return
-        if not self.is_singular and not location.endswith(obj_pk):
-            location += '/{}'.format(obj_pk)
+        resource = (self.model_collections.get(type_) or
+                    self.model_collections.get(ES.src2type(type_)))
+        if resource is not None:
+            location = self.request.route_url(
+                resource.uid, **{resource.id_name: obj_pk})
         obj.setdefault('_self', location)
 
     def __call__(self, **kwargs):

tests/test_polymorphic.py

@@ -14,25 +14,15 @@ def test_get_collections(self):
     def test_get_resources(self):
         mixin = polymorphic.PolymorphicHelperMixin()
         mixin.request = Mock()
-        resource1 = Mock(collection_name='stories', is_singular=False)
-        mixin.request.registry._resources_map = {
+        resource1 = Mock(collection_name='stories')
+        resource2 = Mock(collection_name='foo')
+        mixin.request.registry._model_collections = {
             'bar': resource1,
+            'baz': resource2,
         }
         resources = mixin.get_resources(['stories'])
         assert resources == set([resource1])
 
-    def test_get_resources_singular_dropped(self):
-        mixin = polymorphic.PolymorphicHelperMixin()
-        mixin.request = Mock()
-        resource1 = Mock(collection_name='stories', is_singular=False)
-        resource2 = Mock(collection_name='users', is_singular=True)
-        mixin.request.registry._resources_map = {
-            'bar': resource1,
-            'foo': resource2,
-        }
-        resources = mixin.get_resources(['stories', 'users'])
-        assert resources == set([resource1])
-
 
 class TestPolymorphicACL(object):
 
@@ -132,18 +122,6 @@ def test_run_init_actions(self, mock_wraps, mock_lims, mock_det):
         mock_wraps.assert_called_once_with()
         mock_lims.assert_called_once_with()
 
-    @patch.object(polymorphic.PolymorphicESView, 'determine_types')
-    def test_setup_default_wrappers(self, mock_det):
-        view = self._dummy_view()
-        view._auth_enabled = False
-        view.setup_default_wrappers()
-        assert list(view._after_calls.keys()) == ['index']
-        assert len(view._after_calls['index']) == 4
-        view._auth_enabled = True
-        view.setup_default_wrappers()
-        assert list(view._after_calls.keys()) == ['index']
-        assert len(view._after_calls['index']) == 5
-
     @patch.object(polymorphic.PolymorphicESView, 'get_resources')
     @patch.object(polymorphic.PolymorphicESView, 'get_collections')
     def test_determine_types(self, mock_coll, mock_res):
@@ -169,46 +147,3 @@ def test_index(self, mock_det, mock_get):
         mock_get.assert_called_once_with()
         assert response == mock_get()
         assert view._query_params['_limit'] == 20
-
-
-class TestAddUrlPolymorphicWrapper(object):
-
-    @patch.object(polymorphic.add_url_polymorphic, 'get_resources')
-    @patch.object(polymorphic.add_url_polymorphic, 'get_collections')
-    def test_get_models_map(self, mock_coll, mock_res):
-        mock_coll.return_value = ['stories', 'users']
-        resource1 = Mock()
-        resource1.view.Model = Mock(__name__='Story')
-        mock_res.return_value = [resource1]
-        wrapper = polymorphic.add_url_polymorphic(None)
-        model_resources = wrapper.get_models_map()
-        assert model_resources == {'Story': resource1}
-        mock_coll.assert_called_once_with()
-        mock_res.assert_called_once_with(['stories', 'users'])
-
-    def test_set_object_self(self):
-        wrapper = polymorphic.add_url_polymorphic(None)
-        wrapper.request = Mock()
-        wrapper.request.route_url.return_value = 'foobar'
-        resource1 = Mock(uid='mystories', id_name='story_id')
-        wrapper.model_resources = {'Story': resource1}
-        obj = {'_type': 'Story', '_pk': 4}
-        wrapper._set_object_self(obj)
-        assert obj == {'_type': 'Story', '_pk': 4, '_self': 'foobar'}
-        wrapper.request.route_url.assert_called_once_with(
-            'mystories', story_id=4)
-
-    @patch.object(polymorphic.add_url_polymorphic, 'get_models_map')
-    def test_call(self, mock_map):
-        resource1 = Mock(uid='mystories', id_name='story_id')
-        mock_map.return_value = {'Story': resource1}
-        wrapper = polymorphic.add_url_polymorphic(None)
-        wrapper.request = Mock()
-        wrapper.request.route_url.return_value = 'foobar'
-        obj = {'_type': 'Story', '_pk': 4}
-        assert wrapper(result=obj) == {
-            '_type': 'Story', '_pk': 4, '_self': 'foobar'}
-
-        obj = {'data': [{'_type': 'Story', '_pk': 4}]}
-        assert wrapper(result=obj) == {
-            'data': [{'_type': 'Story', '_pk': 4, '_self': 'foobar'}]}

tests/test_wrappers.py

@@ -100,77 +100,53 @@ def test_add_meta(self):
         result = wrappers.add_meta(request=request)(result=result)
         assert result['count'] == 1
 
-    @patch('nefertari.wrappers.urllib')
-    def test_add_meta_type_error(self, mock_lib):
-        mock_lib.parse.quote.side_effect = TypeError
+    def test_add_meta_type_error(self):
         result = {'data': [{'id': 4}]}
         request = DummyRequest(path='http://example.com', environ={})
         result = wrappers.add_meta(request=request)(result=result)
         assert result['count'] == 1
         assert result['data'][0] == {'id': 4}
 
-    def test_add_object_url_collection(self):
-        result = {'data': [{'_pk': 4}]}
-        request = DummyRequest(path='http://example.com', environ={})
-        wrapper = wrappers.add_object_url(request=request)
-        wrapper._is_singular = False
-        result = wrapper(result=result)
-        assert result['data'][0]['_self'] == 'http://example.com/4'
-
+    def test_add_object_url_collection_not_found_resource(self):
+        result = {'data': [{'_pk': 4, '_type': 'User'}]}
         environ = {'QUERY_STRING': '_limit=100'}
-        request = DummyRequest(path='http://example.com?_limit=100',
-                               environ=environ)
-        assert request.path == 'http://example.com?_limit=100'
-        result = wrappers.add_object_url(request=request)(result=result)
-        assert result['data'][0]['_self'] == 'http://example.com/4'
-
-    def test_add_object_url_item(self):
-        result = {'_pk': 4}
-        request = DummyRequest(path='http://example.com', environ={})
+        request = DummyRequest(path='http://example.com', environ=environ)
         wrapper = wrappers.add_object_url(request=request)
-        wrapper._is_singular = False
+        wrapper.model_collections = {'Story': 123}
         result = wrapper(result=result)
-        assert result == {'_pk': 4, '_self': 'http://example.com/4'}
+        assert result['data'][0]['_self'] == 'http://example.com'
 
-    def test_add_object_url_contains_id(self):
-        result = {'_pk': 4}
-        request = DummyRequest(path='http://example.com/4', environ={})
-        wrapper = wrappers.add_object_url(request=request)
-        wrapper._is_singular = False
-        result = wrapper(result=result)
-        assert result == {'_pk': 4, '_self': 'http://example.com/4'}
-
-    @patch('nefertari.wrappers.urllib')
-    def test_add_object_url_type_error(self, mock_lib):
-        mock_lib.parse.quote.side_effect = TypeError
+    def test_add_object_url_collection_no_type(self):
         result = {'data': [{'_pk': 4}]}
         request = DummyRequest(path='http://example.com', environ={})
         wrapper = wrappers.add_object_url(request=request)
-        wrapper._is_singular = False
+        wrapper.model_collections = {'Story': 123}
         result = wrapper(result=result)
-        assert result['data'][0] == {'_pk': 4}
+        assert '_self' not in result['data'][0]
 
-    def test_add_object_url_singular_resource(self):
-        result = {'_pk': 4}
-        request = DummyRequest(path='http://example.com', environ={})
+    def test_add_object_url_collection(self):
+        result = {'data': [{'_pk': 4, '_type': 'Story'}]}
+        request = Mock()
         wrapper = wrappers.add_object_url(request=request)
-        wrapper._is_singular = True
+        wrapper.model_collections = {
+            'Story': Mock(uid='stories_resource', id_name='story_id'),
+        }
         result = wrapper(result=result)
-        assert result == {'_pk': 4, '_self': 'http://example.com'}
+        request.route_url.assert_called_once_with(
+            'stories_resource', story_id=4)
+        assert result['data'][0]['_self'] == request.route_url()
 
-    def test_add_object_url_is_singular_property(self):
-        route = Mock()
-        route.name = 'foo'
-        registry = Mock(_resources_map={'foo': Mock(is_singular=False)})
-        request = Mock(matched_route=route, registry=registry)
+    def test_add_object_url_item(self):
+        result = {'_pk': 4, '_type': 'Story'}
+        request = Mock()
         wrapper = wrappers.add_object_url(request=request)
-        wrapper._is_singular = None
-        assert not wrapper.is_singular
-        assert not wrapper._is_singular
-        wrapper.request.registry._resources_map['foo'].is_singular = True
-        wrapper._is_singular = None
-        assert wrapper.is_singular
-        assert wrapper._is_singular
+        wrapper.model_collections = {
+            'Story': Mock(uid='stories_resource', id_name='story_id'),
+        }
+        result = wrapper(result=result)
+        request.route_url.assert_called_once_with(
+            'stories_resource', story_id=4)
+        assert result['_self'] == request.route_url()
 
     @patch('nefertari.utils.validate_data_privacy')
     def test_apply_request_privacy_valid(self, mock_validate):