wip: Entire rework of the skel()/*Skel() behavior

- faces some review issues of @sveneberth
- no satisfying integration with existing code

Author: phorward

src/viur/core/prototypes/list.py

@@ -20,26 +20,7 @@ class List(SkelModule):
     handler = "list"
     accessRights = ("add", "edit", "view", "delete", "manage")
 
-    def viewSkel(self, *args, **kwargs) -> SkeletonInstance:
-        """
-            Retrieve a new instance of a :class:`viur.core.skeleton.SkeletonInstance` that is used by the application
-            for viewing an existing entry from the list.
-
-            The default is a Skeleton instance returned by :func:`~baseSkel`.
-
-            This SkeletonInstance can be post-processed (just returning a subskel or manually removing single bones) - which
-            is the recommended way to ensure a given user cannot see certain fields. A Jinja-Template may choose not to
-            display certain bones, but if the json or xml render is attached (or the user can use the vi or admin render)
-            he could still see all values. This also prevents the user from filtering by these bones, so no binary search
-            is possible.
-
-            .. seealso:: :func:`addSkel`, :func:`editSkel`, :func:`~baseSkel`
-
-            :return: Returns a Skeleton instance for viewing an entry.
-        """
-        return self.skel(bones_from_request=True, **kwargs)
-
-    def addSkel(self, *args, **kwargs) -> SkeletonInstance:
+    def addSkel(self, **kwargs) -> SkeletonInstance:
         """
             Retrieve a new instance of a :class:`viur.core.skeleton.Skeleton` that is used by the application
             for adding an entry to the list.
@@ -55,26 +36,9 @@ def addSkel(self, *args, **kwargs) -> SkeletonInstance:
 
             :return: Returns a Skeleton instance for adding an entry.
         """
-        return self.baseSkel(*args, **kwargs)
-
-    def editSkel(self, *args, **kwargs) -> SkeletonInstance:
-        """
-            Retrieve a new instance of a :class:`viur.core.skeleton.Skeleton` that is used by the application
-            for editing an existing entry from the list.
-
-            The default is a Skeleton instance returned by :func:`~baseSkel`.
-
-            Like in :func:`viewSkel`, the skeleton can be post-processed. Bones that are being removed aren't visible
-            and cannot be set, but it's also possible to just set a bone to readOnly (revealing it's value to the user,
-            but preventing any modification.
-
-            .. seealso:: :func:`viewSkel`, :func:`editSkel`, :func:`~baseSkel`
-
-            :return: Returns a Skeleton instance for editing an entry.
-        """
-        return self.baseSkel(*args, **kwargs)
+        return self.skel(**kwargs)
 
-    def cloneSkel(self, *args, **kwargs) -> SkeletonInstance:
+    def cloneSkel(self, **kwargs) -> SkeletonInstance:
         """
         Retrieve a new instance of a :class:`viur.core.skeleton.Skeleton` that is used by the application
         for cloning an existing entry from the list.
@@ -89,7 +53,7 @@ def cloneSkel(self, *args, **kwargs) -> SkeletonInstance:
 
         :return: Returns a SkeletonInstance for editing an entry.
         """
-        return self.baseSkel(*args, **kwargs)
+        return self.skel(**kwargs)
 
     ## External exposed functions
 
@@ -110,7 +74,7 @@ def preview(self, *args, **kwargs) -> t.Any:
         if not self.canPreview():
             raise errors.Unauthorized()
 
-        skel = self.viewSkel()
+        skel = self.viewSkel(allow_client_defined=utils.string.is_prefix(self.render.kind, "json"))
         skel.fromClient(kwargs)
 
         return self.render.view(skel)
@@ -126,7 +90,7 @@ def structure(self, action: t.Optional[str] = "view") -> t.Any:
         # FIXME: In ViUR > 3.7 this could also become dynamic (ActionSkel paradigm).
         match action:
             case "view":
-                skel = self.viewSkel()
+                skel = self.viewSkel(allow_client_defined=utils.string.is_prefix(self.render.kind, "json"))
                 if not self.canView(skel):
                     raise errors.Unauthorized()
 
@@ -168,7 +132,7 @@ def view(self, key: db.Key | int | str, *args, **kwargs) -> t.Any:
             :raises: :exc:`viur.core.errors.NotFound`, when no entry with the given *key* was found.
             :raises: :exc:`viur.core.errors.Unauthorized`, if the current user does not have the required permissions.
         """
-        skel = self.viewSkel()
+        skel = self.viewSkel(allow_client_defined=utils.string.is_prefix(self.render.kind, "json"))
         if not skel.read(key):
             raise errors.NotFound()
 
@@ -195,8 +159,10 @@ def list(self, *args, **kwargs) -> t.Any:
 
             :raises: :exc:`viur.core.errors.Unauthorized`, if the current user does not have the required permissions.
         """
+        skel = self.viewSkel(allow_client_defined=utils.string.is_prefix(self.render.kind, "json"))
+
         # The general access control is made via self.listFilter()
-        if not (query := self.listFilter(self.viewSkel().all().mergeExternalFilter(kwargs))):
+        if not (query := self.listFilter(skel.all().mergeExternalFilter(kwargs))):
             raise errors.Unauthorized()
 
         self._apply_default_order(query)
@@ -323,10 +289,13 @@ def index(self, *args, **kwargs) -> t.Any:
             :return: The rendered entity or list.
         """
         if args and args[0]:
+            skel = self.viewSkel(
+                allow_client_defined=utils.string.is_prefix(self.render.kind, "json"),
+                _excludeFromAccessLog=True,
+            )
+
             # We probably have a Database or SEO-Key here
-            seoKey = str(args[0]).lower()
-            skel = self.viewSkel().all(_excludeFromAccessLog=True).filter("viur.viurActiveSeoKeys =", seoKey).getSkel()
-            if skel:
+            if skel := skel.all().filter("viur.viurActiveSeoKeys =", str(args[0]).lower()).getSkel():
                 db.currentDbAccessLog.get(set()).add(skel["key"])
                 if not self.canView(skel):
                     raise errors.Forbidden()

src/viur/core/prototypes/singleton.py

@@ -27,32 +27,6 @@ def getKey(self) -> str:
         """
         return f"{self.editSkel().kindName}-modulekey"
 
-    def viewSkel(self, *args, **kwargs) -> SkeletonInstance:
-        """
-        Retrieve a new instance of a :class:`viur.core.skeleton.Skeleton` that is used by the application
-        for viewing the existing entry.
-
-        The default is a Skeleton instance returned by :func:`~baseSkel`.
-
-        .. seealso:: :func:`addSkel`, :func:`editSkel`, :func:`~baseSkel`
-
-        :return: Returns a Skeleton instance for viewing the singleton entry.
-        """
-        return self.skel(bones_from_request=True, **kwargs)
-
-    def editSkel(self, *args, **kwargs) -> SkeletonInstance:
-        """
-        Retrieve a new instance of a :class:`viur.core.skeleton.Skeleton` that is used by the application
-        for editing the existing entry.
-
-        The default is a Skeleton instance returned by :func:`~baseSkel`.
-
-        .. seealso:: :func:`viewSkel`, :func:`editSkel`, :func:`~baseSkel`
-
-        :return: Returns a Skeleton instance for editing the entry.
-        """
-        return self.baseSkel(*args, **kwargs)
-
     ## External exposed functions
 
     @exposed
@@ -75,7 +49,7 @@ def preview(self, *args, **kwargs) -> t.Any:
         if not self.canPreview():
             raise errors.Unauthorized()
 
-        skel = self.viewSkel()
+        skel = self.viewSkel(allow_client_defined=utils.string.is_prefix(self.render.kind, "json"))
         skel.fromClient(kwargs)
 
         return self.render.view(skel)
@@ -91,7 +65,7 @@ def structure(self, action: t.Optional[str] = "view") -> t.Any:
         # FIXME: In ViUR > 3.7 this could also become dynamic (ActionSkel paradigm).
         match action:
             case "view":
-                skel = self.viewSkel()
+                skel = self.viewSkel(allow_client_defined=utils.string.is_prefix(self.render.kind, "json"))
                 if not self.canView():
                     raise errors.Unauthorized()
 
@@ -120,7 +94,7 @@ def view(self, *args, **kwargs) -> t.Any:
         :raises: :exc:`viur.core.errors.Unauthorized`, if the current user does not have the required permissions.
         """
 
-        skel = self.viewSkel()
+        skel = self.viewSkel(allow_client_defined=utils.string.is_prefix(self.render.kind, "json"))
         if not self.canView():
             raise errors.Unauthorized()
 

src/viur/core/prototypes/skelmodule.py

@@ -1,7 +1,7 @@
 import os
 import yaml
 import logging
-from viur.core import Module, db, current
+from viur.core import Module, db, current, errors
 from viur.core.decorators import *
 from viur.core.config import conf
 from viur.core.skeleton import skeletonByKind, Skeleton, SkeletonInstance
@@ -47,6 +47,8 @@ def __load_indexes_from_file() -> dict[str, list]:
 
 DATASTORE_INDEXES = __load_indexes_from_file()
 
+X_VIUR_BONELIST = "X-VIUR-BONELIST"
+"""Defines the header parameter that might contain a client-defined bone list."""
 
 class SkelModule(Module):
     """
@@ -99,7 +101,7 @@ def _resolveSkelCls(self, *args, **kwargs) -> t.Type[Skeleton]:
         """
         return skeletonByKind(self.kindName)
 
-    def baseSkel(self, *args, **kwargs) -> SkeletonInstance:
+    def baseSkel(self, **kwargs) -> SkeletonInstance:
         """
         Returns an instance of an unmodified base skeleton for this module.
 
@@ -110,17 +112,56 @@ def baseSkel(self, *args, **kwargs) -> SkeletonInstance:
         """
         return self._resolveSkelCls(**kwargs)()
 
+    def viewSkel(self, **kwargs) -> SkeletonInstance:
+        """
+        Retrieve a new instance of a :class:`viur.core.skeleton.SkeletonInstance` that is used by the application
+        for viewing an existing entry from the list.
+
+        The default is a Skeleton instance returned by :func:`~baseSkel`.
+
+        This SkeletonInstance can be post-processed (just returning a subskel or manually removing single bones) - which
+        is the recommended way to ensure a given user cannot see certain fields. A Jinja-Template may choose not to
+        display certain bones, but if the json or xml render is attached (or the user can use the vi or admin render)
+        he could still see all values. This also prevents the user from filtering by these bones, so no binary search
+        is possible.
+
+        :param client_derive: Allows to
+
+        .. seealso:: :func:`addSkel`, :func:`editSkel`, :func:`~baseSkel`
+
+        :return: Returns a Skeleton instance for viewing an entry.
+        """
+        return self.skel(**kwargs)
+
+    def editSkel(self, **kwargs) -> SkeletonInstance:
+        """
+        Retrieve a new instance of a :class:`viur.core.skeleton.Skeleton` that is used by the application
+        for editing an existing entry from the list.
+
+        The default is a Skeleton instance returned by :func:`~baseSkel`.
+
+        Like in :func:`viewSkel`, the skeleton can be post-processed. Bones that are being removed aren't visible
+        and cannot be set, but it's also possible to just set a bone to readOnly (revealing it's value to the user,
+        but preventing any modification.
+
+        .. seealso:: :func:`viewSkel`, :func:`editSkel`, :func:`~baseSkel`
+
+        :return: Returns a Skeleton instance for editing an entry.
+        """
+        return self.skel(**kwargs)
+
     def skel(
         self,
+        *,
         bones: t.Iterable[str] = (),
-        bones_from_request: bool = False,
+        allow_client_defined: bool = False,
         **kwargs,
     ) -> SkeletonInstance:
         """
         Retrieve module-specific skeleton, optionally as subskel.
 
-        :param bones: ALlows to specify a list of bones to form a subskel.
-        :param bones_from_request: Evaluates header X-VIUR-BONELIST to contain a comma-separated list of bones.
+        :param bones: Allows to specify a list of bones to form a subskel.
+        :param client_derive: Evaluates header X-VIUR-BONELIST to contain a comma-separated list of bones.
             Using this parameter enforces that the Skeleton class has a subskel named "*" for required bones that
             must exist.
 
@@ -129,24 +170,24 @@ def skel(
         skel_cls = self._resolveSkelCls(**kwargs)
         bones = set(bones) if bones else set()
 
-        if (
-            bones_from_request  # feature generally enabled?
-            and skel_cls.subSkels.get("*")  # a named subSkel "*"" must exist
-            # and (bonelist := current.request.get().kwargs.get("x-viur-bonelist"))  # param must be given (DEBUG!)
-            and (bonelist := current.request.get().request.headers.get("x-viur-bonelist"))  # header must be given
-        ):
-            bones |= {bone.strip() for bone in bonelist.split(",")}
+        if allow_client_defined:
+            if bonelist := current.request.get().kwargs.get(X_VIUR_BONELIST.lower()):  # DEBUG
+            # if bonelist := current.request.get().request.headers.get(X_VIUR_BONELIST)
+                if "*" not in skel_cls.subSkels:  # a named star-subskel "*"" must exist!
+                    raise errors.BadRequest(f"Use of {X_VIUR_BONELIST!r} requires for a star-subskel")
+
+                bones |= {bone.strip() for bone in bonelist.split(",")}
 
         # Return a subskel?
         if bones:
-            # When coming from outside of a request, "*" must always be contained.
-            if bones_from_request:
+            # When coming from outside of a request, "*" is always involved.
+            if allow_client_defined:
                 return skel_cls.subskel("*", bones=bones)
 
             return skel_cls.subskel(bones=bones)
 
         # Otherwise, return full skeleton
-        return skel_cls()
+        return skel_cls()  # FIXME: This is fishy, it should return a baseSkel(), but then some customer project break
 
     def _apply_default_order(self, query: db.Query):
         """