fix: Enfore serialized values are always strings in the datastore

src/viur/core/bones/numeric.py

@@ -209,6 +209,7 @@ def refresh_single_value(value: t.Any) -> float | int:
                 return self._convert_to_numeric(value)
             return value
 
+        # TODO: duplicate code, this is the same iteration logic as in StringBone
         new_value = {}
         for _, lang, value in self.iter_bone_value(skel, boneName):
             new_value.setdefault(lang, []).append(refresh_single_value(value))

src/viur/core/bones/string.py

@@ -1,11 +1,17 @@
-import warnings
-
+import datetime
 import logging
 import typing as t
+import warnings
+from numbers import Number
 
 from viur.core import current, db, utils
 from viur.core.bones.base import BaseBone, ReadFromClientError, ReadFromClientErrorSeverity
 
+if t.TYPE_CHECKING:
+    from ..skeleton import SkeletonInstance
+
+DB_TYPE_INDEXED: t.TypeAlias = dict[t.Literal["val", "idx", "sort_idx"], str]
+
 
 class StringBone(BaseBone):
     """
@@ -60,41 +66,70 @@ def __init__(
             self.natural_sorting = None
         # else: keep self.natural_sorting as is
 
-    def singleValueSerialize(self, value, skel: 'SkeletonInstance', name: str, parentIndexed: bool):
+    def type_coerce_single_value(self, value: t.Any) -> str:
+        """Convert a value to a string (if not already)
+
+        Converts a value that is not a string into a string
+        if a meaningful conversion is possible (simple data types only).
+        """
+        if isinstance(value, str):
+            return value
+        elif isinstance(value, Number):
+            return str(value)
+        elif isinstance(value, (datetime.datetime, datetime.date, datetime.time)):
+            return value.isoformat()
+        elif isinstance(value, db.Key):
+            return value.to_legacy_urlsafe().decode("ASCII")
+        elif not value:  # None or any other falsy value
+            return self.getEmptyValue()
+        else:
+            raise ValueError(
+                f"Value {value} of type {type(value)} cannot be coerced for {type(self).__name__} {self.name}"
+            )
+
+    def singleValueSerialize(
+        self,
+        value: t.Any,
+        skel: "SkeletonInstance",
+        name: str,
+        parentIndexed: bool,
+    ) -> str | DB_TYPE_INDEXED:
         """
         Serializes a single value of this data field for storage in the database.
 
         :param value: The value to serialize.
+            It should be a str value, if not it is forced with :meth:`type_coerce_single_value`.
         :param skel: The skeleton instance that this data field belongs to.
         :param name: The name of this data field.
         :param parentIndexed: A boolean value indicating whether the parent object has an index on
             this data field or not.
         :return: The serialized value.
         """
+        value = self.type_coerce_single_value(value)
         if (not self.caseSensitive or self.natural_sorting) and parentIndexed:
-            serialized = {"val": value}
+            serialized: DB_TYPE_INDEXED = {"val": value}
             if not self.caseSensitive:
-                serialized["idx"] = value.lower() if isinstance(value, str) else None
+                serialized["idx"] = value.lower()
             if self.natural_sorting:
                 serialized["sort_idx"] = self.natural_sorting(value)
             return serialized
         return value
 
-    def singleValueUnserialize(self, value):
+    def singleValueUnserialize(self, value: str | DB_TYPE_INDEXED) -> str:
         """
         Unserializes a single value of this data field from the database.
 
         :param value: The serialized value to unserialize.
         :return: The unserialized value.
         """
         if isinstance(value, dict) and "val" in value:
-            return value["val"]
-        elif value:
+            value = value["val"]  # Process with the raw value
+        if value:
             return str(value)
         else:
-            return ""
+            return self.getEmptyValue()
 
-    def getEmptyValue(self):
+    def getEmptyValue(self) -> str:
         """
         Returns the empty value for this data field.
 
@@ -114,7 +149,7 @@ def isEmpty(self, value):
 
         return not bool(str(value).strip())
 
-    def isInvalid(self, value):
+    def isInvalid(self, value: t.Any) -> str | None:
         """
         Returns None if the value would be valid for
         this bone, an error-message otherwise.
@@ -139,7 +174,7 @@ def singleValueFromClient(self, value, skel, bone_name, client_data):
     def buildDBFilter(
         self,
         name: str,
-        skel: 'viur.core.skeleton.SkeletonInstance',
+        skel: "SkeletonInstance",
         dbFilter: db.Query,
         rawFilter: dict,
         prefix: t.Optional[str] = None
@@ -207,7 +242,7 @@ def buildDBFilter(
     def buildDBSort(
         self,
         name: str,
-        skel: 'viur.core.skeleton.SkeletonInstance',
+        skel: "SkeletonInstance",
         dbFilter: db.Query,
         rawFilter: dict
     ) -> t.Optional[db.Query]:
@@ -219,7 +254,6 @@ def buildDBSort(
         :param dbFilter: A Query object representing the current DB filter.
         :param rawFilter: A dictionary containing the raw filter.
         :return: The Query object with the specified sort applied.
-        :rtype: Optional[google.cloud.ndb.query.Query]
         """
         if ((orderby := rawFilter.get("orderby"))
             and (orderby == name
@@ -285,15 +319,14 @@ def natural_sorting(self, value: str | None) -> str | None:
             "ẞ": "SS",
         }))
 
-    def getSearchTags(self, skel: 'viur.core.skeleton.SkeletonInstance', name: str) -> set[str]:
+    def getSearchTags(self, skel: "SkeletonInstance", name: str) -> set[str]:
         """
         Returns a set of lowercased words that represent searchable tags for the given bone.
 
         :param skel: The skeleton instance being searched.
         :param name: The name of the bone to generate tags for.
 
         :return: A set of lowercased words representing searchable tags.
-        :rtype: set
         """
         result = set()
         for idx, lang, value in self.iter_bone_value(skel, name):
@@ -304,14 +337,13 @@ def getSearchTags(self, skel: 'viur.core.skeleton.SkeletonInstance', name: str)
                     result.add(word.lower())
         return result
 
-    def getUniquePropertyIndexValues(self, skel, name: str) -> list[str]:
+    def getUniquePropertyIndexValues(self, skel: "SkeletonInstance", name: str) -> list[str]:
         """
         Returns a list of unique index values for a given property name.
 
         :param skel: The skeleton instance.
         :param name: The name of the property.
         :return: A list of unique index values for the property.
-        :rtype: List[str]
         :raises NotImplementedError: If the StringBone has languages and the implementation
             for this case is not yet defined.
         """
@@ -321,6 +353,24 @@ def getUniquePropertyIndexValues(self, skel, name: str) -> list[str]:
 
         return super().getUniquePropertyIndexValues(skel, name)
 
+    def refresh(self, skel: "SkeletonInstance", bone_name: str) -> None:
+        super().refresh(skel, bone_name)
+
+        # TODO: duplicate code, this is the same iteration logic as in NumericBone
+        new_value = {}
+        for _, lang, value in self.iter_bone_value(skel, bone_name):
+            new_value.setdefault(lang, []).append(self.type_coerce_single_value(value))
+
+        if not self.multiple:
+            # take the first one
+            new_value = {lang: values[0] for lang, values in new_value.items() if values}
+
+        if self.languages:
+            skel[bone_name] = new_value
+        elif not self.languages:
+            # just the value(s) with None language
+            skel[bone_name] = new_value.get(None, [] if self.multiple else self.getEmptyValue())
+
     def structure(self) -> dict:
         ret = super().structure() | {
             "maxlength": self.max_length,

tests/bones/test_string_bone.py

@@ -1,4 +1,5 @@
 import unittest
+from unittest.mock import patch
 
 
 class TestStringBone(unittest.TestCase):
@@ -169,9 +170,9 @@ def test_singleValueSerialize_caseSensitive(self):
         res = bone.singleValueSerialize("Foo", skel, self.bone_name, False)
         self.assertEqual("Foo", res)
         res = bone.singleValueSerialize(None, skel, self.bone_name, True)
-        self.assertEqual(None, res)
+        self.assertEqual("", res)
         res = bone.singleValueSerialize(None, skel, self.bone_name, False)
-        self.assertEqual(None, res)
+        self.assertEqual("", res)
 
     def test_singleValueSerialize_caseInSensitive(self):
         from viur.core.bones import StringBone
@@ -182,9 +183,9 @@ def test_singleValueSerialize_caseInSensitive(self):
         res = bone.singleValueSerialize("Foo", skel, self.bone_name, False)
         self.assertEqual("Foo", res)
         res = bone.singleValueSerialize(None, skel, self.bone_name, True)
-        self.assertDictEqual({"val": None, "idx": None}, res)
+        self.assertDictEqual({"val": "", "idx": ""}, res)
         res = bone.singleValueSerialize(None, skel, self.bone_name, False)
-        self.assertEqual(None, res)
+        self.assertEqual("", res)
 
     def test_singleValueUnserialize(self):
         from viur.core.bones import StringBone

tests/main.py

@@ -1,12 +1,11 @@
 #!/usr/bin/env python3
-from unittest import mock
-
 import importlib.util
 import os
 import pathlib
 import sys
 import unittest
 from types import ModuleType
+from unittest import mock
 
 # top_level_dir is the parent-folder of "tests" and "core"
 tld = pathlib.Path(__file__).resolve().parent.parent
@@ -73,8 +72,8 @@ def __init__(self, *args, **kwargs):
         "GetOrInsert",
         "IsInTransaction",
         "KEY_SPECIAL_PROPERTY",
-        "Key",
-        "KeyClass",
+        # "Key",
+        # "KeyClass",
         "keyHelper",
         "Put",
         "Query",
@@ -90,6 +89,9 @@ def __init__(self, *args, **kwargs):
         setattr(viur_datastore, attr, mock.MagicMock())
 
     viur_datastore.config = {}
+    # classes must not be instances of MagicMock, otherwise isinstance checks does not work
+    viur_datastore.Key = mock.MagicMock
+    viur_datastore.KeyClass = mock.MagicMock
     sys.modules["viur.datastore"] = viur_datastore
 
     os.environ["GAE_VERSION"] = "v42"

tests/test_utils.py

@@ -78,4 +78,4 @@ def test_parse_timedelta(self):
         self.assertEqual(td(seconds=60), utils.parse.timedelta(60.0))
         self.assertEqual(td(seconds=60), utils.parse.timedelta("60"))
         self.assertEqual(td(seconds=60), utils.parse.timedelta("60.0"))
-        self.assertNotEquals(td(seconds=0), utils.parse.timedelta(60.0))
+        self.assertNotEqual(td(seconds=0), utils.parse.timedelta(60.0))