Skip to content

Commit

Permalink
wip: Entire rework of the skel()/*Skel() behavior
Browse files Browse the repository at this point in the history
- faces some review issues of @sveneberth
- no satisfying integration with existing code
  • Loading branch information
phorward committed Feb 5, 2025
1 parent 32399e2 commit b9dcb36
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 137 deletions.
63 changes: 16 additions & 47 deletions src/viur/core/prototypes/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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

Expand All @@ -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)
Expand All @@ -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()

Expand Down Expand Up @@ -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()

Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
32 changes: 3 additions & 29 deletions src/viur/core/prototypes/singleton.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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()

Expand Down Expand Up @@ -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()

Expand Down
71 changes: 56 additions & 15 deletions src/viur/core/prototypes/skelmodule.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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)

Check failure on line 175 in src/viur/core/prototypes/skelmodule.py

View workflow job for this annotation

GitHub Actions / linter (3.12)

E115: expected an indented block (comment)
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):
"""
Expand Down
Loading

0 comments on commit b9dcb36

Please sign in to comment.