diff --git a/CHANGELOG.md b/CHANGELOG.md index 615450ee26..37beca050e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,10 +14,11 @@ - [Fix][pull3485]: Typo in `sethome` message (chiizujin) - [Fix][pull3487]: Fix traceback when using `get`,`drop` and `give` with no arguments (InspectorCaracal) -- [Fix][issue3476]: Don't ignore EvEditor commands with wrong capitalization - (Griatch) +- [Fix][issue3476]: Don't ignore EvEditor commands with wrong capitalization (Griatch) - [Fix][issue3477]: The `at_server_reload_start()` hook was not firing on a reload (regression). +- [Fix][issue3488]: `AttributeProperty(, autocreate=False)`, where + `` was mutable would not update/save properly in-place (Griatch) - [Docs] Added new [Server-Lifecycle][doc-server-lifecycle] page to describe the hooks called on server start/stop/reload (Griatch) - [Docs] Doc typo fixes (Griatch, chiizujin) @@ -27,6 +28,7 @@ [pull3487]: https://github.com/evennia/evennia/pull/3487 [issue3476]: https://github.com/evennia/evennia/issues/3476 [issue3477]: https://github.com/evennia/evennia/issues/3477 +[issue3488]: https://github.com/evennia/evennia/issues/3488 [doc-server-lifecycle]: https://www.evennia.com/docs/latest/Concepts/Server-Lifecycle.html diff --git a/evennia/objects/tests.py b/evennia/objects/tests.py index de23269cdf..2d54f51012 100644 --- a/evennia/objects/tests.py +++ b/evennia/objects/tests.py @@ -3,12 +3,8 @@ from unittest import skip from evennia import DefaultCharacter, DefaultExit, DefaultObject, DefaultRoom from evennia.objects.models import ObjectDB from evennia.typeclasses.attributes import AttributeProperty -from evennia.typeclasses.tags import ( - AliasProperty, - PermissionProperty, - TagCategoryProperty, - TagProperty, -) +from evennia.typeclasses.tags import (AliasProperty, PermissionProperty, + TagCategoryProperty, TagProperty) from evennia.utils import create, search from evennia.utils.test_resources import BaseEvenniaTest, EvenniaTestCase @@ -356,6 +352,10 @@ class TestObjectPropertiesClass(DefaultObject): attr2 = AttributeProperty(default="attr2", category="attrcategory") attr3 = AttributeProperty(default="attr3", autocreate=False) attr4 = SubAttributeProperty(default="attr4") + attr5 = AttributeProperty(default=list, autocreate=False) + attr6 = AttributeProperty(default=[None], autocreate=False) + attr7 = AttributeProperty(default=list) + attr8 = AttributeProperty(default=[None]) cusattr = CustomizedProperty(default=5) tag1 = TagProperty() tag2 = TagProperty(category="tagcategory") @@ -541,3 +541,99 @@ class TestProperties(EvenniaTestCase): obj1.delete() obj2.delete() + + def test_not_create_attribute_with_autocreate_false(self): + """ + Test that AttributeProperty with autocreate=False does not create an attribute in the database. + + """ + obj = create.create_object(TestObjectPropertiesClass, key="obj1") + + self.assertEqual(obj.attr3, "attr3") + self.assertEqual(obj.attributes.get("attr3"), None) + + self.assertEqual(obj.attr5, []) + self.assertEqual(obj.attributes.get("attr5"), None) + + obj.delete() + + def test_callable_defaults__autocreate_false(self): + """ + Test https://github.com/evennia/evennia/issues/3488, where a callable default value like `list` + would produce an infinitely empty result even when appended to. + + """ + obj1 = create.create_object(TestObjectPropertiesClass, key="obj1") + obj2 = create.create_object(TestObjectPropertiesClass, key="obj2") + + self.assertEqual(obj1.attr5, []) + obj1.attr5.append(1) + self.assertEqual(obj1.attr5, [1]) + + # check cross-instance sharing + self.assertEqual(obj2.attr5, [], "cross-instance sharing detected") + + + def test_mutable_defaults__autocreate_false(self): + """ + Test https://github.com/evennia/evennia/issues/3488, where a mutable default value (like a + list `[]` or `[None]`) would not be updated in the database when appended to. + + Note that using a mutable default value is not recommended, as the mutable will share the + same memory space across all instances of the class. This means that if one instance modifiesA + the mutable, all instances will be affected. + + """ + obj1 = create.create_object(TestObjectPropertiesClass, key="obj1") + obj2 = create.create_object(TestObjectPropertiesClass, key="obj2") + + self.assertEqual(obj1.attr6, [None]) + obj1.attr6.append(1) + self.assertEqual(obj1.attr6, [None, 1]) + + obj1.attr6[1] = 2 + self.assertEqual(obj1.attr6, [None, 2]) + + # check cross-instance sharing + self.assertEqual(obj2.attr6, [None], "cross-instance sharing detected") + + obj1.delete() + obj2.delete() + + def test_callable_defaults__autocreate_true(self): + """ + Test callables with autocreate=True. + + """ + obj1 = create.create_object(TestObjectPropertiesClass, key="obj1") + obj2 = create.create_object(TestObjectPropertiesClass, key="obj1") + + self.assertEqual(obj1.attr7, []) + obj1.attr7.append(1) + self.assertEqual(obj1.attr7, [1]) + + # check cross-instance sharing + self.assertEqual(obj2.attr7, []) + + + def test_mutable_defaults__autocreate_true(self): + """ + Test mutable defaults with autocreate=True. + + """ + obj1 = create.create_object(TestObjectPropertiesClass, key="obj1") + obj2 = create.create_object(TestObjectPropertiesClass, key="obj2") + + self.assertEqual(obj1.attr8, [None]) + obj1.attr8.append(1) + self.assertEqual(obj1.attr8, [None, 1]) + + obj1.attr8[1] = 2 + self.assertEqual(obj1.attr8, [None, 2]) + + # check cross-instance sharing + self.assertEqual(obj2.attr8, [None]) + + obj1.delete() + obj2.delete() + diff --git a/evennia/typeclasses/attributes.py b/evennia/typeclasses/attributes.py index 5aedc03262..bdef3bdc33 100644 --- a/evennia/typeclasses/attributes.py +++ b/evennia/typeclasses/attributes.py @@ -12,11 +12,11 @@ which is a non-db version of Attributes. import fnmatch import re from collections import defaultdict +from copy import copy from django.conf import settings from django.db import models from django.utils.encoding import smart_str - from evennia.locks.lockhandler import LockHandler from evennia.utils.dbserialize import from_pickle, to_pickle from evennia.utils.idmapper.models import SharedMemoryModel @@ -166,6 +166,7 @@ class AttributeProperty: """ attrhandler_name = "attributes" + cached_default_name_template = "_property_attribute_default_{key}" def __init__(self, default=None, category=None, strattr=False, lockstring="", autocreate=True): """ @@ -207,21 +208,6 @@ class AttributeProperty: self._autocreate = autocreate self._key = "" - @property - def _default(self): - """ - Tries returning a new instance of default if callable. - - """ - if callable(self.__default): - return self.__default() - - return self.__default - - @_default.setter - def _default(self, value): - self.__default = value - def __set_name__(self, cls, name): """ Called when descriptor is first assigned to the class. It is called with @@ -230,17 +216,35 @@ class AttributeProperty: """ self._key = name + def _get_and_cache_default(self, instance): + """ + Get and cache the default value for this attribute. We make sure to convert any mutables + into _Saver* equivalent classes here and cache the result on the instance's AttributeHandler. + + """ + attrhandler = getattr(instance, self.attrhandler_name) + value = getattr(attrhandler, self.cached_default_name_template.format(key=self._key), None) + if not value: + if callable(self._default): + value = self._default() + else: + value = copy(self._default) + value = from_pickle(value, db_obj=instance) + setattr(attrhandler, self.cached_default_name_template.format(key=self._key), value) + return value + def __get__(self, instance, owner): """ Called when the attrkey is retrieved from the instance. """ - value = self._default + value = self._get_and_cache_default(instance) + try: value = self.at_get( getattr(instance, self.attrhandler_name).get( key=self._key, - default=self._default, + default=value, category=self._category, strattr=self._strattr, raise_exception=self._autocreate, @@ -250,7 +254,7 @@ class AttributeProperty: except AttributeError: if self._autocreate: # attribute didn't exist and autocreate is set - self.__set__(instance, self._default) + self.__set__(instance, value) else: raise return value