Add proper support for composite primary key, in MasterView

at least hopefully this is complete, and didn't break anything else...

tailbone/views/master.py

@@ -2106,15 +2106,24 @@ class MasterView(View):
         return cls.get_model_class().__name__.lower()
 
     @classmethod
-    def get_model_key(cls):
+    def get_model_key(cls, as_tuple=False):
         """
-        Return a string name for the primary key of the model class.
+        Returns the primary key(s) for the model class.  Note that this will
+        return a *string* value unless a tuple is requested.  If the model has
+        a composite key then the string result would be a comma-delimited list
+        of names, e.g. ``foo_id,bar_id``.
         """
         if hasattr(cls, 'model_key'):
-            return cls.model_key
+            keys = cls.model_key
+            if isinstance(keys, six.string_types):
+                keys = [keys]
+        else:
+            keys = get_primary_keys(cls.get_model_class())
 
-        pkeys = get_primary_keys(cls.get_model_class())
+        if as_tuple:
-        return ','.join(pkeys)
+            return tuple(keys)
+
+        return ','.join(keys)
 
     @classmethod
     def get_model_title(cls):
@@ -2778,12 +2787,29 @@ class MasterView(View):
         Fetch the current model instance by inspecting the route kwargs and
         doing a database lookup.  If the instance cannot be found, raises 404.
         """
-        # TODO: this can't handle composite model key..is that needed?
+        model_keys = self.get_model_key(as_tuple=True)
-        key = self.request.matchdict[self.get_model_key()]
+
-        instance = self.Session.query(self.get_model_class()).get(key)
+        # if just one primary key, simple get() will work
-        if not instance:
+        if len(model_keys) == 1:
-            raise httpexceptions.HTTPNotFound()
+            model_key = model_keys[0]
-        return instance
+            key = self.request.matchdict[model_key]
+
+            obj = self.Session.query(self.get_model_class()).get(key)
+            if not obj:
+                raise self.notfound()
+            return obj
+
+        else: # composite key; fetch accordingly
+            # TODO: should perhaps use filter() instead of get() here?
+            query = self.Session.query(self.get_model_class())
+            for i, model_key in enumerate(model_keys):
+                key = self.request.matchdict[model_key]
+                query = query.filter(getattr(self.model_class, model_key) == key)
+            try:
+                obj = query.one()
+            except NoResultFound:
+                raise self.notfound()
+            return obj
 
     def get_instance_title(self, instance):
         """
@@ -3318,7 +3344,7 @@ class MasterView(View):
         key = self.request.matchdict['row_uuid']
         instance = self.Session.query(self.model_row_class).get(key)
         if not instance:
-            raise httpexceptions.HTTPNotFound()
+            raise self.notfound()
         return instance
 
     def make_row_form(self, instance=None, factory=None, fields=None, schema=None, **kwargs):
@@ -3429,6 +3455,26 @@ class MasterView(View):
         """
         cls._defaults(config)
 
+    @classmethod
+    def get_instance_url_prefix(cls):
+        """
+        Generate the URL prefix specific to an instance for this model view.
+        Winds up looking something like:
+
+        * ``/products/{uuid}``
+        * ``/params/{foo}|{bar}|{baz}``
+        """
+        url_prefix = cls.get_url_prefix()
+        model_keys = cls.get_model_key(as_tuple=True)
+
+        prefix = '{}/'.format(url_prefix)
+        for i, key in enumerate(model_keys):
+            if i:
+                prefix += '|'
+            prefix += '{{{}}}'.format(key)
+
+        return prefix
+
     @classmethod
     def _defaults(cls, config):
         """
@@ -3437,6 +3483,7 @@ class MasterView(View):
         rattail_config = config.registry.settings.get('rattail_config')
         route_prefix = cls.get_route_prefix()
         url_prefix = cls.get_url_prefix()
+        instance_url_prefix = cls.get_instance_url_prefix()
         permission_prefix = cls.get_permission_prefix()
         model_key = cls.get_model_key()
         model_title = cls.get_model_title()
@@ -3553,11 +3600,11 @@ class MasterView(View):
                             permission='{}.view'.format(permission_prefix))
 
             # view by record key
-            config.add_route('{}.view'.format(route_prefix), '{}/{{{}}}'.format(url_prefix, model_key))
+            config.add_route('{}.view'.format(route_prefix), instance_url_prefix)
             config.add_view(cls, attr='view', route_name='{}.view'.format(route_prefix),
                             permission='{}.view'.format(permission_prefix))
             if cls.supports_mobile:
-                config.add_route('mobile.{}.view'.format(route_prefix), '/mobile{}/{{{}}}'.format(url_prefix, model_key))
+                config.add_route('mobile.{}.view'.format(route_prefix), '/mobile{}'.format(instance_url_prefix))
                 config.add_view(cls, attr='mobile_view', route_name='mobile.{}.view'.format(route_prefix),
                                 permission='{}.view'.format(permission_prefix))
 
@@ -3565,22 +3612,22 @@ class MasterView(View):
             if cls.has_versions and rattail_config and rattail_config.versioning_enabled():
                 config.add_tailbone_permission(permission_prefix, '{}.versions'.format(permission_prefix),
                                                "View version history for {}".format(model_title))
-                config.add_route('{}.versions'.format(route_prefix), '{}/{{{}}}/versions/'.format(url_prefix, model_key))
+                config.add_route('{}.versions'.format(route_prefix), '{}/versions/'.format(instance_url_prefix))
                 config.add_view(cls, attr='versions', route_name='{}.versions'.format(route_prefix),
                                 permission='{}.versions'.format(permission_prefix))
-                config.add_route('{}.version'.format(route_prefix), '{}/{{{}}}/versions/{{txnid}}'.format(url_prefix, model_key))
+                config.add_route('{}.version'.format(route_prefix), '{}/versions/{{txnid}}'.format(instance_url_prefix))
                 config.add_view(cls, attr='view_version', route_name='{}.version'.format(route_prefix),
                                 permission='{}.versions'.format(permission_prefix))
 
         # image
         if cls.has_image:
-            config.add_route('{}.image'.format(route_prefix), '{}/{{{}}}/image'.format(url_prefix, model_key))
+            config.add_route('{}.image'.format(route_prefix), '{}/image'.format(instance_url_prefix))
             config.add_view(cls, attr='image', route_name='{}.image'.format(route_prefix),
                             permission='{}.view'.format(permission_prefix))
 
         # thumbnail
         if cls.has_thumbnail:
-            config.add_route('{}.thumbnail'.format(route_prefix), '{}/{{{}}}/thumbnail'.format(url_prefix, model_key))
+            config.add_route('{}.thumbnail'.format(route_prefix), '{}/thumbnail'.format(instance_url_prefix))
             config.add_view(cls, attr='thumbnail', route_name='{}.thumbnail'.format(route_prefix),
                             permission='{}.view'.format(permission_prefix))
 
@@ -3588,7 +3635,7 @@ class MasterView(View):
         if cls.cloneable:
             config.add_tailbone_permission(permission_prefix, '{}.clone'.format(permission_prefix),
                                            "Clone an existing {0} as a new {0}".format(model_title))
-            config.add_route('{}.clone'.format(route_prefix), '{}/{{{}}}/clone'.format(url_prefix, model_key))
+            config.add_route('{}.clone'.format(route_prefix), '{}/clone'.format(instance_url_prefix))
             config.add_view(cls, attr='clone', route_name='{}.clone'.format(route_prefix),
                             permission='{}.clone'.format(permission_prefix))
 
@@ -3596,13 +3643,13 @@ class MasterView(View):
         if cls.touchable:
             config.add_tailbone_permission(permission_prefix, '{}.touch'.format(permission_prefix),
                                            "\"Touch\" a {} to trigger datasync for it".format(model_title))
-            config.add_route('{}.touch'.format(route_prefix), '{}/{{{}}}/touch'.format(url_prefix, model_key))
+            config.add_route('{}.touch'.format(route_prefix), '{}/touch'.format(instance_url_prefix))
             config.add_view(cls, attr='touch', route_name='{}.touch'.format(route_prefix),
                             permission='{}.touch'.format(permission_prefix))
 
         # download
         if cls.downloadable:
-            config.add_route('{}.download'.format(route_prefix), '{}/{{{}}}/download'.format(url_prefix, model_key))
+            config.add_route('{}.download'.format(route_prefix), '{}/download'.format(instance_url_prefix))
             config.add_view(cls, attr='download', route_name='{}.download'.format(route_prefix),
                             permission='{}.download'.format(permission_prefix))
             config.add_tailbone_permission(permission_prefix, '{}.download'.format(permission_prefix),
@@ -3613,11 +3660,11 @@ class MasterView(View):
             config.add_tailbone_permission(permission_prefix, '{}.edit'.format(permission_prefix),
                                            "Edit {}".format(model_title))
         if cls.editable:
-            config.add_route('{}.edit'.format(route_prefix), '{}/{{{}}}/edit'.format(url_prefix, model_key))
+            config.add_route('{}.edit'.format(route_prefix), '{}/edit'.format(instance_url_prefix))
             config.add_view(cls, attr='edit', route_name='{}.edit'.format(route_prefix),
                             permission='{}.edit'.format(permission_prefix))
         if cls.mobile_editable:
-            config.add_route('mobile.{}.edit'.format(route_prefix), '/mobile{}/{{{}}}/edit'.format(url_prefix, model_key))
+            config.add_route('mobile.{}.edit'.format(route_prefix), '/mobile{}/edit'.format(instance_url_prefix))
             config.add_view(cls, attr='mobile_edit', route_name='mobile.{}.edit'.format(route_prefix),
                             permission='{}.edit'.format(permission_prefix))
 
@@ -3626,17 +3673,17 @@ class MasterView(View):
             config.add_tailbone_permission(permission_prefix, '{}.execute'.format(permission_prefix),
                                            "Execute {}".format(model_title))
         if cls.executable:
-            config.add_route('{}.execute'.format(route_prefix), '{}/{{{}}}/execute'.format(url_prefix, model_key))
+            config.add_route('{}.execute'.format(route_prefix), '{}/execute'.format(instance_url_prefix))
             config.add_view(cls, attr='execute', route_name='{}.execute'.format(route_prefix),
                             permission='{}.execute'.format(permission_prefix))
         if cls.mobile_executable:
-            config.add_route('mobile.{}.execute'.format(route_prefix), '/mobile{}/{{{}}}/execute'.format(url_prefix, model_key))
+            config.add_route('mobile.{}.execute'.format(route_prefix), '/mobile{}/execute'.format(instance_url_prefix))
             config.add_view(cls, attr='mobile_execute', route_name='mobile.{}.execute'.format(route_prefix),
                             permission='{}.execute'.format(permission_prefix))
 
         # delete
         if cls.deletable:
-            config.add_route('{0}.delete'.format(route_prefix), '{0}/{{{1}}}/delete'.format(url_prefix, model_key))
+            config.add_route('{0}.delete'.format(route_prefix), '{}/delete'.format(instance_url_prefix))
             config.add_view(cls, attr='delete', route_name='{0}.delete'.format(route_prefix),
                             permission='{0}.delete'.format(permission_prefix))
             config.add_tailbone_permission(permission_prefix, '{0}.delete'.format(permission_prefix),
@@ -3671,15 +3718,15 @@ class MasterView(View):
                 config.add_tailbone_permission(permission_prefix, '{}.create_row'.format(permission_prefix),
                                                "Create new {} rows".format(model_title))
             if cls.rows_creatable:
-                config.add_route('{}.create_row'.format(route_prefix), '{}/{{{}}}/new-row'.format(url_prefix, model_key))
+                config.add_route('{}.create_row'.format(route_prefix), '{}/new-row'.format(instance_url_prefix))
                 config.add_view(cls, attr='create_row', route_name='{}.create_row'.format(route_prefix),
                                 permission='{}.create_row'.format(permission_prefix))
             if cls.mobile_rows_creatable:
-                config.add_route('mobile.{}.create_row'.format(route_prefix), '/mobile{}/{{{}}}/new-row'.format(url_prefix, model_key))
+                config.add_route('mobile.{}.create_row'.format(route_prefix), '/mobile{}/new-row'.format(instance_url_prefix))
                 config.add_view(cls, attr='mobile_create_row', route_name='mobile.{}.create_row'.format(route_prefix),
                                 permission='{}.create_row'.format(permission_prefix))
                 if cls.mobile_rows_quickable:
-                    config.add_route('mobile.{}.quick_row'.format(route_prefix), '/mobile{}/{{{}}}/quick-row'.format(url_prefix, model_key))
+                    config.add_route('mobile.{}.quick_row'.format(route_prefix), '/mobile{}/quick-row'.format(instance_url_prefix))
                     config.add_view(cls, attr='mobile_quick_row', route_name='mobile.{}.quick_row'.format(route_prefix),
                                     permission='{}.create_row'.format(permission_prefix))