From ab5714721e94b313bfbc26f8afc2c45bcbd674c0 Mon Sep 17 00:00:00 2001 From: Arne Gudermann Date: Wed, 29 Nov 2023 17:37:52 +0100 Subject: [PATCH] Refactor setBoneValue --- src/viur/core/bones/base.py | 36 +++++++++++++-------- src/viur/core/bones/relational.py | 53 +++++++++++++++++-------------- src/viur/core/bones/spatial.py | 17 +++++++--- src/viur/core/skeleton.py | 23 ++++++++++---- 4 files changed, 83 insertions(+), 46 deletions(-) diff --git a/src/viur/core/bones/base.py b/src/viur/core/bones/base.py index 45a67df43..60ee2ddf7 100644 --- a/src/viur/core/bones/base.py +++ b/src/viur/core/bones/base.py @@ -8,6 +8,8 @@ import copy import hashlib import inspect +import warnings + import logging from dataclasses import dataclass, field from datetime import datetime, timedelta @@ -1075,16 +1077,17 @@ def mergeFrom(self, valuesCache: Dict, boneName: str, otherSkel: 'viur.core.skel def setBoneValue(self, skel: 'SkeletonInstance', - boneName: str, + bone_name: str, value: Any, append: bool, - language: Union[None, str] = None) -> bool: + language: Union[None, str] = None, + **kwargs) -> bool: """ Sets the value of a bone in a skeleton instance, with optional support for appending and language-specific values. Sanity checks are being performed. :param skel: The SkeletonInstance object representing the skeleton to which the bone belongs. - :param boneName: The property-name of the bone in the skeleton whose value should be set or modified. + :param bone_name: The property-name of the bone in the skeleton whose value should be set or modified. :param value: The value to be assigned. Its type depends on the type of the bone. :param append: If True, the given value is appended to the bone's values instead of replacing it. \ Only supported for bones with multiple=True. @@ -1097,6 +1100,13 @@ def setBoneValue(self, the value is valid. If the value is invalid, no modification occurs. The function supports appending values to bones with multiple=True and setting or appending language-specific values for bones that support languages. """ + # fixme: Remove in viur-core >= 4 + if "boneName" in kwargs: + msg = "boneName was replaced by bone_name in 'setBoneValue'" + warnings.warn(msg, DeprecationWarning, stacklevel=3) + logging.warning(msg, stacklevel=3) + bone_name = kwargs["boneName"] + assert not (bool(self.languages) ^ bool(language)), "Language is required or not supported" assert not append or self.multiple, "Can't append - bone is not multiple" @@ -1105,13 +1115,13 @@ def setBoneValue(self, val = [] errors = [] for singleValue in value: - singleValue, singleError = self.singleValueFromClient(singleValue, skel, boneName, {boneName: value}) + singleValue, singleError = self.singleValueFromClient(singleValue, skel, bone_name, {bone_name: value}) val.append(singleValue) if singleError: errors.extend(singleError) else: # set or append one value - val, errors = self.singleValueFromClient(value, skel, boneName, {boneName: value}) + val, errors = self.singleValueFromClient(value, skel, bone_name, {bone_name: value}) if errors: for e in errors: @@ -1119,17 +1129,17 @@ def setBoneValue(self, # If an invalid datatype (or a non-parseable structure) have been passed, abort the store return False if not append and not language: - skel[boneName] = val + skel[bone_name] = val elif append and language: - if not language in skel[boneName] or not isinstance(skel[boneName][language], list): - skel[boneName][language] = [] - skel[boneName][language].append(val) + if not language in skel[bone_name] or not isinstance(skel[bone_name][language], list): + skel[bone_name][language] = [] + skel[bone_name][language].append(val) elif append: - if not isinstance(skel[boneName], list): - skel[boneName] = [] - skel[boneName].append(val) + if not isinstance(skel[bone_name], list): + skel[bone_name] = [] + skel[bone_name].append(val) else: # Just language - skel[boneName][language] = val + skel[bone_name][language] = val return True def getSearchTags(self, skel: 'viur.core.skeleton.SkeletonInstance', name: str) -> Set[str]: diff --git a/src/viur/core/bones/relational.py b/src/viur/core/bones/relational.py index c5a192f7e..346a7882b 100644 --- a/src/viur/core/bones/relational.py +++ b/src/viur/core/bones/relational.py @@ -1190,17 +1190,18 @@ def createRelSkelFromKey(self, key: Union[str, db.Key], rel: Union[dict, None] = def setBoneValue( self, skel: 'SkeletonInstance', - boneName: str, + bone_name: str, value: Any, append: bool, - language: Union[None, str] = None + language: Union[None, str] = None, + **kwargs ) -> bool: """ Sets the value of the specified bone in the given skeleton. Sanity checks are performed to ensure the value is valid. If the value is invalid, no modifications are made. :param SkeletonInstance skel: Dictionary with the current values from the skeleton we belong to. - :param str boneName: The name of the bone to be modified. + :param str bone_name: The name of the bone to be modified. :param value: The value to be assigned. The type depends on the bone type. :param bool append: If true, the given value is appended to the values of the bone instead of replacing it. Only supported on bones with multiple=True. @@ -1210,24 +1211,30 @@ def setBoneValue( :return: True if the operation succeeded, False otherwise. :rtype: bool """ + # fixme: Remove in viur-core >= 4 + if "boneName" in kwargs: + msg = "boneName was replaced by bone_name in 'setBoneValue'" + warnings.warn(msg, DeprecationWarning, stacklevel=3) + logging.warning(msg, stacklevel=3) + bone_name = kwargs["boneName"] assert not (bool(self.languages) ^ bool(language)), "Language is required or not supported" assert not append or self.multiple, "Can't append - bone is not multiple" if not self.multiple and not self.using: if not (isinstance(value, str) or isinstance(value, db.Key)): logging.error(value) logging.error(type(value)) - raise ValueError("You must supply exactly one Database-Key to %s" % boneName) + raise ValueError(f"You must supply exactly one Database-Key to {bone_name}") realValue = (value, None) elif not self.multiple and self.using: if not isinstance(value, tuple) or len(value) != 2 or \ not (isinstance(value[0], str) or isinstance(value[0], db.Key)) or \ not isinstance(value[1], self._skeletonInstanceClassRef): - raise ValueError("You must supply a tuple of (Database-Key, relSkel) to %s" % boneName) + raise ValueError(f"You must supply a tuple of (Database-Key, relSkel) to {bone_name}") realValue = value elif self.multiple and not self.using: if not (isinstance(value, str) or isinstance(value, db.Key)) and not (isinstance(value, list)) \ and all([isinstance(x, str) or isinstance(x, db.Key) for x in value]): - raise ValueError("You must supply a Database-Key or a list hereof to %s" % boneName) + raise ValueError(f"You must supply a Database-Key or a list hereof to {bone_name}") if isinstance(value, list): realValue = [(x, None) for x in value] else: @@ -1240,7 +1247,7 @@ def setBoneValue( (isinstance(x, tuple) and len(x) == 2 and (isinstance(x[0], str) or isinstance(x[0], db.Key)) and isinstance(x[1], self._skeletonInstanceClassRef) for x in value))): - raise ValueError("You must supply (db.Key, RelSkel) or a list hereof to %s" % boneName) + raise ValueError(f"You must supply (db.Key, RelSkel) or a list hereof to {bone_name}") if not isinstance(value, list): realValue = [value] else: @@ -1250,11 +1257,11 @@ def setBoneValue( if not rel: return False if language: - if boneName not in skel or not isinstance(skel[boneName], dict): - skel[boneName] = {} - skel[boneName][language] = rel + if bone_name not in skel or not isinstance(skel[bone_name], dict): + skel[bone_name] = {} + skel[bone_name][language] = rel else: - skel[boneName] = rel + skel[bone_name] = rel else: tmpRes = [] for val in realValue: @@ -1264,22 +1271,22 @@ def setBoneValue( tmpRes.append(rel) if append: if language: - if boneName not in skel or not isinstance(skel[boneName], dict): - skel[boneName] = {} - if not isinstance(skel[boneName].get(language), list): - skel[boneName][language] = [] - skel[boneName][language].extend(tmpRes) + if bone_name not in skel or not isinstance(skel[bone_name], dict): + skel[bone_name] = {} + if not isinstance(skel[bone_name].get(language), list): + skel[bone_name][language] = [] + skel[bone_name][language].extend(tmpRes) else: - if boneName not in skel or not isinstance(skel[boneName], list): - skel[boneName] = [] - skel[boneName].extend(tmpRes) + if bone_name not in skel or not isinstance(skel[bone_name], list): + skel[bone_name] = [] + skel[bone_name].extend(tmpRes) else: if language: - if boneName not in skel or not isinstance(skel[boneName], dict): - skel[boneName] = {} - skel[boneName][language] = tmpRes + if bone_name not in skel or not isinstance(skel[bone_name], dict): + skel[bone_name] = {} + skel[bone_name][language] = tmpRes else: - skel[boneName] = tmpRes + skel[bone_name] = tmpRes return True def getReferencedBlobs(self, skel: 'viur.core.skeleton.SkeletonInstance', name: str) -> Set[str]: diff --git a/src/viur/core/bones/spatial.py b/src/viur/core/bones/spatial.py index 189ebd93c..f433d7f60 100644 --- a/src/viur/core/bones/spatial.py +++ b/src/viur/core/bones/spatial.py @@ -3,6 +3,7 @@ - The `SpatialBone` to handle coordinates - and `haversine` to calculate the distance between two points on earth using their latitude and longitude. """ +import warnings import logging from copy import deepcopy @@ -366,10 +367,11 @@ def customMultiQueryMerge(self, name, lat, lng, dbFilter: db.Query, def setBoneValue( self, skel: 'SkeletonInstance', - boneName: str, + bone_name: str, value: Any, append: bool, - language: Union[None, str] = None + language: Union[None, str] = None, + **kwargs ) -> bool: """ Sets the value of the bone to the provided 'value'. @@ -377,7 +379,7 @@ def setBoneValue( (default) value and the function will return False. :param skel: Dictionary with the current values from the skeleton the bone belongs to - :param boneName: The name of the bone that should be modified + :param bone_name: The name of the bone that should be modified :param value: The value that should be assigned. Its type depends on the type of the bone :param append: If True, the given value will be appended to the existing bone values instead of replacing them. Only supported on bones with multiple=True @@ -385,10 +387,17 @@ def setBoneValue( :return: A boolean indicating whether the operation succeeded or not :rtype: bool """ + # fixme: Remove in viur-core >= 4 + if "boneName" in kwargs: + msg = "boneName was replaced by bone_name in 'setBoneValue'" + warnings.warn(msg, DeprecationWarning, stacklevel=3) + logging.warning(msg, stacklevel=3) + bone_name = kwargs["boneName"] if append: raise ValueError("append is not possible on %s bones" % self.type) assert isinstance(value, tuple) and len(value) == 2, "Value must be a tuple of (lat, lng)" - skel[boneName] = value + skel[bone_name] = value + return True def structure(self) -> dict: return super().structure() | { diff --git a/src/viur/core/skeleton.py b/src/viur/core/skeleton.py index 9eb3c10de..038908e7f 100644 --- a/src/viur/core/skeleton.py +++ b/src/viur/core/skeleton.py @@ -216,6 +216,7 @@ def __getattr__(self, item): "delete", "postDeletedHandler", "refresh"}: return partial(getattr(self.skeletonCls, item), self) try: + logging.debug(f"try ? getattr {item}") return self.boneMap[item] except KeyError: raise AttributeError(f"{self.__class__.__name__!r} object has no attribute '{item}'") @@ -325,8 +326,12 @@ def setSystemInitialized(cls): bone.setSystemInitialized() @classmethod - def setBoneValue(cls, skelValues: Any, boneName: str, value: Any, - append: bool = False, language: Optional[str] = None) -> bool: + def setBoneValue(cls, skel: SkeletonInstance, + bone_name: str, + value: Any, + append: bool = False, + language: Optional[str] = None, + **kwargs) -> bool: """ Allow setting a bones value without calling fromClient or assigning to valuesCache directly. Santy-Checks are performed; if the value is invalid, that bone flips back to its original @@ -339,11 +344,17 @@ def setBoneValue(cls, skelValues: Any, boneName: str, value: Any, :param language: Set/append which language :return: Wherever that operation succeeded or not. """ - bone = getattr(skelValues, boneName, None) + # fixme: Remove in viur-core >= 4 + if "boneName" in kwargs: + msg = "boneName was replaced by bone_name in 'setBoneValue'" + warnings.warn(msg, DeprecationWarning, stacklevel=3) + logging.warning(msg, stacklevel=3) + bone_name = kwargs["boneName"] + bone = getattr(skel, bone_name, None) if not isinstance(bone, BaseBone): - raise ValueError("%s is no valid bone on this skeleton (%s)" % (boneName, str(skelValues))) - skelValues[boneName] # FIXME, ensure this bone is unserialized first - return bone.setBoneValue(skelValues, boneName, value, append, language) + raise ValueError(f"{bone_name} is no valid bone on this skeleton ({skel})") + bone.unserialize(skel, bone_name) + return bone.setBoneValue(skel, bone_name, value, append, language) @classmethod def fromClient(cls, skelValues: SkeletonInstance, data: Dict[str, Union[List[str], str]],