viur-framework / viur-core

The core component of ViUR, the Python framework for modern web development.
https://www.viur.dev
MIT License
13 stars 14 forks source link

`Skeleton.interBoneValidations` should be replaced by something useful #1207

Open phorward opened 2 months ago

phorward commented 2 months ago

interBoneValidations is one of the ugliest pieces in ViUR. It was intended to be used in the Skeleton to run validations on skeleton-level.

Levels of ugliness:

  1. The name interBoneValidations is ugly and misleading
  2. Why is it a list of validators, when all tests can be done in one function?
  3. Why does each function either return None or a list of ReadFromClientError?

Everywhere I've being used it, it looked liked this:

def _my_ugly_validation(skel):
    if skel["x"] in ("a", "c") and skel["y"] in ("a", "b"):
        return [
            skeleton.ReadFromClientError(
                skeleton.ReadFromClientErrorSeverity.Invalid,
                "No, but yes",
                ["x"]
            )
        ]

class MyUglySkeleton(Skeleton):
    interBoneValidations = [
        _my_ugly_validation
    ]

So in the end, it can be just a subclass-able hook function, let's call it just validate, which does the same, and belongs to the Skeleton class.

Proposal:

class MyLesserUglySkeleton(Skeleton):
    def validate(skel: SkeletonInstance) -> None | ReadFromClientError | t.Iterable[ReadFromClientError]:
        if skel["x"] in ("a", "c") and skel["y"] in ("a", "b"):
            return [
                skeleton.ReadFromClientError(
                    skeleton.ReadFromClientErrorSeverity.Invalid,
                    "No, but yes",
                    ["x"]
                )
            ]

This relates to #630 as well, which defines a well named and clear CRUD naming convention.