From 4a175c76f9ba5d8489da27a0f3ddc7caf10a322a Mon Sep 17 00:00:00 2001 From: Lance Edgar Date: Mon, 29 Jul 2019 19:27:23 -0500 Subject: [PATCH] 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 | 105 ++++++++++++++++++++++++++++----------- 1 file changed, 76 insertions(+), 29 deletions(-) diff --git a/tailbone/views/master.py b/tailbone/views/master.py index 8f8ff251..676e99fd 100644 --- a/tailbone/views/master.py +++ b/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()) - return ','.join(pkeys) + if as_tuple: + 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? - key = self.request.matchdict[self.get_model_key()] - instance = self.Session.query(self.get_model_class()).get(key) - if not instance: - raise httpexceptions.HTTPNotFound() - return instance + model_keys = self.get_model_key(as_tuple=True) + + # if just one primary key, simple get() will work + if len(model_keys) == 1: + model_key = model_keys[0] + 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))