[core] set value of dict property not working #44435

iscai-msft · 2025-12-15T21:45:25Z

it works if you do m["my_dict"]["c"] = 3.

The reason is when accessing via attribute syntax, in our serialization step, we enter __get__, which then calls this serialization line. since we create a new dictionary type here, we aren't mutating the original type to also include c.

Copilot

Pull request overview

This PR fixes a bug where mutations to dictionary, list, and set properties accessed via attribute syntax were not persisting. The root cause was that each attribute access created a new deserialized object, so mutations to that object didn't affect the underlying data. The fix introduces a caching mechanism that stores deserialized mutable collections and syncs them back to storage when accessed via dictionary syntax.

Key Changes:

Implements caching of deserialized mutable collections (dict, list, set) to preserve mutations
Adds sync-back logic in __getitem__ to serialize cached values before returning via dictionary syntax
Adds cache invalidation in __set__ when a new value is assigned via attribute syntax
Comprehensive test coverage for dictionary, list, and set mutation scenarios including datetime serialization

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.

File	Description
sdk/core/azure-core/tests/test_serialization.py	Adds comprehensive tests for dictionary, list, and set mutations via both attribute and dictionary syntax, including edge cases with datetime serialization
sdk/core/azure-core/tests/specs_sdk/modeltypes/modeltypes/_utils/model_base.py	Implements caching mechanism in `_RestField.__get__` to store deserialized mutable collections, adds cache invalidation in `__set__`, and adds sync-back logic in `__getitem__` to serialize cached values to storage

Comments suppressed due to low confidence (1)

sdk/core/azure-core/tests/specs_sdk/modeltypes/modeltypes/_utils/model_base.py:375

The setitem method should invalidate the cached deserialized object when a value is set via dictionary syntax. Currently, when users do m["my_dict"] = new_value, the old cached deserialized object at _deserialized_my_dict is not cleared, which means subsequent attribute access (m.my_dict) would return the stale cached value instead of the newly set value.

    def __setitem__(self, key: str, value: typing.Any) -> None:
        self._data.__setitem__(key, value)

Copilot · 2025-12-16T19:10:13Z

sdk/core/azure-core/tests/specs_sdk/modeltypes/modeltypes/_utils/model_base.py

+        # Sync any cached deserialized value back to storage before returning
+        if hasattr(self, "_attr_to_rest_field"):
+            cache_attr = f"_deserialized_{key}"
+            if hasattr(self, cache_attr):
+                cached_value = object.__getattribute__(self, cache_attr)
+                rf = _get_rest_field(self._attr_to_rest_field, key)
+                if rf:
+                    # Serialize the cached value back to storage
+                    self._data[key] = _serialize(cached_value, rf._format)


The getitem method synchronizes cached values back to storage on every dictionary access. This could have performance implications if getitem is called frequently, as it performs serialization each time a cached value exists. Consider adding a "dirty" flag to track if the cached value has been modified, or document this performance characteristic clearly.

Copilot · 2025-12-16T19:10:14Z

sdk/core/azure-core/tests/test_serialization.py

+def test_list_set():
+    """Test that list mutations via attribute syntax persist and sync to dictionary syntax."""
+
+    class MyModel(HybridModel):
+        my_list: List[int] = rest_field(visibility=["read", "create", "update", "delete", "query"])
+
+    # Test 1: Basic mutation via attribute syntax
+    m = MyModel(my_list=[1, 2, 3])
+    assert m.my_list == [1, 2, 3]
+
+    # Test 2: Append via attribute syntax and verify it persists
+    m.my_list.append(4)
+    assert m.my_list == [1, 2, 3, 4]
+
+    # Test 3: Verify mutation is reflected in dictionary syntax
+    assert m["my_list"] == [1, 2, 3, 4]
+
+    # Test 4: Modify existing element via attribute syntax
+    m.my_list[0] = 100
+    assert m.my_list[0] == 100
+    assert m["my_list"][0] == 100
+
+    # Test 5: Extend list via attribute syntax
+    m.my_list.extend([5, 6])
+    assert m.my_list == [100, 2, 3, 4, 5, 6]
+    assert m["my_list"] == [100, 2, 3, 4, 5, 6]
+
+    # Test 6: Remove element via attribute syntax
+    m.my_list.remove(2)
+    assert 2 not in m.my_list
+    assert 2 not in m["my_list"]
+
+    # Test 7: Pop element
+    popped = m.my_list.pop()
+    assert popped == 6
+    assert 6 not in m.my_list
+    assert 6 not in m["my_list"]
+
+    # Test 8: Insert element
+    m.my_list.insert(0, 999)
+    assert m.my_list[0] == 999
+    assert m["my_list"][0] == 999
+
+    # Test 9: Clear via attribute syntax
+    m.my_list.clear()
+    assert len(m.my_list) == 0
+    assert len(m["my_list"]) == 0
+
+    # Test 10: Reassign entire list via attribute syntax
+    m.my_list = [10, 20, 30]
+    assert m.my_list == [10, 20, 30]
+    assert m["my_list"] == [10, 20, 30]
+
+    # Test 11: Mutation after reassignment
+    m.my_list.append(40)
+    assert m.my_list == [10, 20, 30, 40]
+    assert m["my_list"] == [10, 20, 30, 40]
+
+    # Test 12: Multiple accesses maintain same cached object
+    list_ref1 = m.my_list
+    list_ref2 = m.my_list
+    assert list_ref1 is list_ref2
+    list_ref1.append(50)
+    assert 50 in list_ref2
+    assert 50 in m.my_list
+


The test should verify the behavior when deleting a field via dictionary syntax (del m["my_list"]) and then recreating it. Since delitem doesn't clear the cache (currently not in the changed region but related to this feature), this could lead to stale cache issues. Add a test case: create list, mutate it, delete field via del m["my_list"], recreate with m.my_list = [new values], and verify the new list is returned.

Copilot · 2025-12-16T19:10:14Z

sdk/core/azure-core/tests/test_serialization.py

+def test_set_collection():
+    """Test that set mutations via attribute syntax persist and sync to dictionary syntax."""
+
+    class MyModel(HybridModel):
+        my_set: typing.Set[int] = rest_field(visibility=["read", "create", "update", "delete", "query"])
+
+    # Test 1: Basic mutation via attribute syntax
+    m = MyModel(my_set={1, 2, 3})
+    assert m.my_set == {1, 2, 3}
+
+    # Test 2: Add via attribute syntax and verify it persists
+    m.my_set.add(4)
+    assert 4 in m.my_set
+
+    # Test 3: Verify mutation is reflected in dictionary syntax
+    assert 4 in m["my_set"]
+
+    # Test 4: Remove element via attribute syntax
+    m.my_set.remove(2)
+    assert 2 not in m.my_set
+    assert 2 not in m["my_set"]
+
+    # Test 5: Update set via attribute syntax
+    m.my_set.update({5, 6, 7})
+    assert m.my_set == {1, 3, 4, 5, 6, 7}
+    assert m["my_set"] == {1, 3, 4, 5, 6, 7}
+
+    # Test 6: Discard element
+    m.my_set.discard(1)
+    assert 1 not in m.my_set
+    assert 1 not in m["my_set"]
+
+    # Test 7: Clear via attribute syntax
+    m.my_set.clear()
+    assert len(m.my_set) == 0
+    assert len(m["my_set"]) == 0
+
+    # Test 8: Reassign entire set via attribute syntax
+    m.my_set = {10, 20, 30}
+    assert m.my_set == {10, 20, 30}
+    assert m["my_set"] == {10, 20, 30}
+
+    # Test 9: Mutation after reassignment
+    m.my_set.add(40)
+    assert 40 in m.my_set
+    assert 40 in m["my_set"]
+
+    # Test 10: Multiple accesses maintain same cached object
+    set_ref1 = m.my_set
+    set_ref2 = m.my_set
+    assert set_ref1 is set_ref2
+    set_ref1.add(50)
+    assert 50 in set_ref2
+    assert 50 in m.my_set
+


The test should include edge cases for empty collections to ensure caching works correctly with empty dict, list, and set. Add test cases that start with empty collections (m = MyModel(my_set=set())) and verify mutations work correctly, as well as testing transitions between empty and non-empty states.

Copilot · 2025-12-16T19:10:14Z

sdk/core/azure-core/tests/specs_sdk/modeltypes/modeltypes/_utils/model_base.py

+        except AttributeError:
+            pass


The bare except for AttributeError when setting cache could mask other AttributeError exceptions that might occur during object.setattr. While this is defensive programming, it would be better to only catch the specific AttributeError that occurs when the object doesn't support attribute assignment, or to add logging to track when this exception occurs for debugging purposes.

Suggested change

except AttributeError:

pass

except AttributeError as exc:

_LOGGER.debug(

"Failed to cache deserialized attribute '%s' on object of type '%s': %s",

cache_attr, type(obj).__name__, exc

)

Copilot · 2025-12-16T19:10:14Z

sdk/core/azure-core/tests/specs_sdk/modeltypes/modeltypes/_utils/model_base.py

+        try:
+            object.__delattr__(obj, cache_attr)
+        except AttributeError:
+            pass


The cache invalidation logic in set uses a bare except for AttributeError when clearing the cache, but it should handle the case where the attribute doesn't exist more explicitly. While the current implementation works, using hasattr before delattr would be more explicit and maintainable.

Suggested change

try:

object.__delattr__(obj, cache_attr)

except AttributeError:

pass

if hasattr(obj, cache_attr):

object.__delattr__(obj, cache_attr)

Copilot · 2025-12-16T19:10:15Z

sdk/core/azure-core/tests/specs_sdk/modeltypes/modeltypes/_utils/model_base.py

+        cache_attr = f"_deserialized_{self._rest_name}"
+        if hasattr(obj, cache_attr):
+            return object.__getattribute__(obj, cache_attr)
+
+        deserialized = _deserialize(self._type, _serialize(item, self._format), rf=self)
+
+        # Cache mutable types so mutations persist
+        try:
+            if isinstance(deserialized, (dict, list, set)):
+                object.__setattr__(obj, cache_attr, deserialized)
+        except AttributeError:
+            pass
+
+        return deserialized

    def __set__(self, obj: Model, value) -> None:
+        # Clear the cached deserialized object when setting a new value
+        cache_attr = f"_deserialized_{self._rest_name}"
+        try:
+            object.__delattr__(obj, cache_attr)
+        except AttributeError:
+            pass


The caching mechanism uses object.setattr and object.getattribute to bypass the normal attribute access. However, there's a potential issue if the Model class or its subclasses override setattr or getattribute. This approach also doesn't account for potential conflicts with user-defined attributes that might have names starting with deserialized. Consider using a dedicated cache dictionary (like self._cache = {}) to isolate cached values more safely.

Suggested change

cache_attr = f"_deserialized_{self._rest_name}"

if hasattr(obj, cache_attr):

return object.__getattribute__(obj, cache_attr)

deserialized = _deserialize(self._type, _serialize(item, self._format), rf=self)

# Cache mutable types so mutations persist

try:

if isinstance(deserialized, (dict, list, set)):

object.__setattr__(obj, cache_attr, deserialized)

except AttributeError:

pass

return deserialized

def __set__(self, obj: Model, value) -> None:

# Clear the cached deserialized object when setting a new value

cache_attr = f"_deserialized_{self._rest_name}"

try:

object.__delattr__(obj, cache_attr)

except AttributeError:

pass

if not hasattr(obj, "_deserialized_cache"):

obj._deserialized_cache = {}

cache = obj._deserialized_cache

cache_key = self._rest_name

if cache_key in cache:

return cache[cache_key]

deserialized = _deserialize(self._type, _serialize(item, self._format), rf=self)

# Cache mutable types so mutations persist

if isinstance(deserialized, (dict, list, set)):

cache[cache_key] = deserialized

return deserialized

def __set__(self, obj: Model, value) -> None:

# Clear the cached deserialized object when setting a new value

if hasattr(obj, "_deserialized_cache"):

cache = obj._deserialized_cache

cache_key = self._rest_name

cache.pop(cache_key, None)

Copilot · 2025-12-16T19:10:15Z

sdk/core/azure-core/tests/specs_sdk/modeltypes/modeltypes/_utils/model_base.py

+        # For mutable types (Dict, List, Set), cache the deserialized object to allow mutations to persist
+        cache_attr = f"_deserialized_{self._rest_name}"
+        if hasattr(obj, cache_attr):
+            return object.__getattribute__(obj, cache_attr)
+
+        deserialized = _deserialize(self._type, _serialize(item, self._format), rf=self)
+
+        # Cache mutable types so mutations persist
+        try:
+            if isinstance(deserialized, (dict, list, set)):
+                object.__setattr__(obj, cache_attr, deserialized)
+        except AttributeError:
+            pass


The caching mechanism is not thread-safe. If multiple threads access the same Model instance and mutate cached collections (dict, list, set) via attribute syntax, race conditions could occur when checking hasattr(obj, cache_attr) and calling object.setattr or object.getattribute. Consider documenting thread-safety expectations or adding synchronization if Model instances are intended to be shared across threads.

Copilot · 2025-12-16T19:10:15Z

sdk/core/azure-core/tests/test_serialization.py

+def test_dictionary_set():
+    """Test that dictionary mutations via attribute syntax persist and sync to dictionary syntax."""
+
+    class MyModel(HybridModel):
+        my_dict: Dict[str, int] = rest_field(visibility=["read", "create", "update", "delete", "query"])
+
+    # Test 1: Basic mutation via attribute syntax
+    m = MyModel(my_dict={"a": 1, "b": 2})
+    assert m.my_dict == {"a": 1, "b": 2}
+
+    # Test 2: Add new key via attribute syntax and verify it persists
+    m.my_dict["c"] = 3
+    assert m.my_dict["c"] == 3
+    assert m.my_dict == {"a": 1, "b": 2, "c": 3}
+
+    # Test 3: Verify mutation is reflected in dictionary syntax
+    assert m["my_dict"] == {"a": 1, "b": 2, "c": 3}
+
+    # Test 4: Modify existing key via attribute syntax
+    m.my_dict["a"] = 100
+    assert m.my_dict["a"] == 100
+    assert m["my_dict"]["a"] == 100
+
+    # Test 5: Delete key via attribute syntax
+    del m.my_dict["b"]
+    assert "b" not in m.my_dict
+    assert "b" not in m["my_dict"]
+
+    # Test 6: Update via dict methods
+    m.my_dict.update({"d": 4, "e": 5})
+    assert m.my_dict["d"] == 4
+    assert m.my_dict["e"] == 5
+    assert m["my_dict"]["d"] == 4
+
+    # Test 7: Clear via attribute syntax and verify via dictionary syntax
+    m.my_dict.clear()
+    assert len(m.my_dict) == 0
+    assert len(m["my_dict"]) == 0
+
+    # Test 8: Reassign entire dictionary via attribute syntax
+    m.my_dict = {"x": 10, "y": 20}
+    assert m.my_dict == {"x": 10, "y": 20}
+    assert m["my_dict"] == {"x": 10, "y": 20}
+
+    # Test 9: Mutation after reassignment
+    m.my_dict["z"] = 30
+    assert m.my_dict["z"] == 30
+    assert m["my_dict"]["z"] == 30
+
+    # Test 10: Access via dictionary syntax first, then mutate via attribute syntax
+    m.my_dict["w"] = 40
+    assert m["my_dict"]["w"] == 40
+
+    # Test 11: Multiple accesses maintain same cached object
+    dict_ref1 = m.my_dict
+    dict_ref2 = m.my_dict
+    assert dict_ref1 is dict_ref2
+    dict_ref1["new_key"] = 999
+    assert dict_ref2["new_key"] == 999
+    assert m.my_dict["new_key"] == 999
+


The test doesn't verify the behavior when using dictionary syntax to set a new value (m["my_dict"] = new_value) and then accessing via attribute syntax. This is a critical edge case to test, as the cache invalidation in setitem is currently missing (see related bug comment). Add a test case that sets via m["my_dict"] = {...} and verifies m.my_dict returns the newly set value, not a stale cached value.

I think a test case outlined by copilot here would be good.

I found that the following case:

def test_both_item_attr_access(): class MyModel(HybridModel): my_dict: dict[str, int] = rest_field() m = MyModel(my_dict={"a": 1, "b": 2}) m.my_dict["c"] = 3 assert m.my_dict["c"] == 3 m["my_dict"]["d"] = 4 assert m.my_dict["d"] == 4 assert m.my_dict == {"a": 1, "b": 2, "c": 3, "d": 4}

yields a:

> assert m.my_dict["d"] == 4 E KeyError: 'd'

Copilot · 2025-12-16T19:10:16Z

sdk/core/azure-core/tests/test_serialization.py

+def test_dictionary_set():
+    """Test that dictionary mutations via attribute syntax persist and sync to dictionary syntax."""
+
+    class MyModel(HybridModel):
+        my_dict: Dict[str, int] = rest_field(visibility=["read", "create", "update", "delete", "query"])
+
+    # Test 1: Basic mutation via attribute syntax
+    m = MyModel(my_dict={"a": 1, "b": 2})
+    assert m.my_dict == {"a": 1, "b": 2}
+
+    # Test 2: Add new key via attribute syntax and verify it persists
+    m.my_dict["c"] = 3
+    assert m.my_dict["c"] == 3
+    assert m.my_dict == {"a": 1, "b": 2, "c": 3}
+
+    # Test 3: Verify mutation is reflected in dictionary syntax
+    assert m["my_dict"] == {"a": 1, "b": 2, "c": 3}
+
+    # Test 4: Modify existing key via attribute syntax
+    m.my_dict["a"] = 100
+    assert m.my_dict["a"] == 100
+    assert m["my_dict"]["a"] == 100
+
+    # Test 5: Delete key via attribute syntax
+    del m.my_dict["b"]
+    assert "b" not in m.my_dict
+    assert "b" not in m["my_dict"]
+
+    # Test 6: Update via dict methods
+    m.my_dict.update({"d": 4, "e": 5})
+    assert m.my_dict["d"] == 4
+    assert m.my_dict["e"] == 5
+    assert m["my_dict"]["d"] == 4
+
+    # Test 7: Clear via attribute syntax and verify via dictionary syntax
+    m.my_dict.clear()
+    assert len(m.my_dict) == 0
+    assert len(m["my_dict"]) == 0
+
+    # Test 8: Reassign entire dictionary via attribute syntax
+    m.my_dict = {"x": 10, "y": 20}
+    assert m.my_dict == {"x": 10, "y": 20}
+    assert m["my_dict"] == {"x": 10, "y": 20}
+
+    # Test 9: Mutation after reassignment
+    m.my_dict["z"] = 30
+    assert m.my_dict["z"] == 30
+    assert m["my_dict"]["z"] == 30
+
+    # Test 10: Access via dictionary syntax first, then mutate via attribute syntax
+    m.my_dict["w"] = 40
+    assert m["my_dict"]["w"] == 40
+
+    # Test 11: Multiple accesses maintain same cached object
+    dict_ref1 = m.my_dict
+    dict_ref2 = m.my_dict
+    assert dict_ref1 is dict_ref2
+    dict_ref1["new_key"] = 999
+    assert dict_ref2["new_key"] == 999
+    assert m.my_dict["new_key"] == 999
+


The test should verify that after modifying a collection via attribute syntax (e.g., m.my_dict["c"] = 3), accessing the raw _data storage reflects the changes. This would ensure the sync-back mechanism in getitem is working correctly. Add assertions like assert m._data["my_dict"]["c"] == 3 after mutations to verify the underlying data integrity.

Copilot · 2025-12-16T19:10:16Z

sdk/core/azure-core/tests/test_serialization.py

+def test_dictionary_set_datetime():
+    """Test that dictionary with datetime values properly serializes/deserializes."""
+    from datetime import datetime, timezone
+
+    class MyModel(HybridModel):
+        my_dict: Dict[str, datetime] = rest_field(visibility=["read", "create", "update", "delete", "query"])
+
+    # Test 1: Initialize with datetime values
+    dt1 = datetime(2023, 1, 15, 10, 30, 45, tzinfo=timezone.utc)
+    dt2 = datetime(2023, 6, 20, 14, 15, 30, tzinfo=timezone.utc)
+    m = MyModel(my_dict={"created": dt1, "updated": dt2})
+
+    # Test 2: Access via attribute syntax returns datetime objects
+    assert isinstance(m.my_dict["created"], datetime)
+    assert isinstance(m.my_dict["updated"], datetime)
+    assert m.my_dict["created"] == dt1
+    assert m.my_dict["updated"] == dt2
+
+    # Test 3: Access via dictionary syntax returns serialized strings (ISO format)
+    dict_access = m["my_dict"]
+    assert isinstance(dict_access["created"], str)
+    assert isinstance(dict_access["updated"], str)
+    assert dict_access["created"] == "2023-01-15T10:30:45Z"
+    assert dict_access["updated"] == "2023-06-20T14:15:30Z"
+
+    # Test 4: Mutate via attribute syntax with new datetime
+    dt3 = datetime(2023, 12, 25, 18, 0, 0, tzinfo=timezone.utc)
+    m.my_dict["holiday"] = dt3
+    assert m.my_dict["holiday"] == dt3
+
+    # Test 5: Verify mutation is serialized in dictionary syntax
+    assert m["my_dict"]["holiday"] == "2023-12-25T18:00:00Z"
+
+    # Test 6: Update existing datetime via attribute syntax
+    dt4 = datetime(2024, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
+    m.my_dict["created"] = dt4
+    assert m.my_dict["created"] == dt4
+    assert m["my_dict"]["created"] == "2024-01-01T00:00:00Z"
+
+    # Test 7: Verify all datetimes are deserialized correctly after mutation
+    assert isinstance(m.my_dict["created"], datetime)
+    assert isinstance(m.my_dict["updated"], datetime)
+    assert isinstance(m.my_dict["holiday"], datetime)
+
+    # Test 8: Use dict update method with datetimes
+    dt5 = datetime(2024, 6, 15, 12, 30, 0, tzinfo=timezone.utc)
+    dt6 = datetime(2024, 7, 4, 16, 45, 0, tzinfo=timezone.utc)
+    m.my_dict.update({"event1": dt5, "event2": dt6})
+    assert m.my_dict["event1"] == dt5
+    assert m["my_dict"]["event1"] == "2024-06-15T12:30:00Z"
+
+    # Test 9: Reassign entire dictionary with new datetimes
+    dt7 = datetime(2025, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
+    dt8 = datetime(2025, 12, 31, 23, 59, 59, tzinfo=timezone.utc)
+    m.my_dict = {"start": dt7, "end": dt8}
+    assert m.my_dict["start"] == dt7
+    assert m.my_dict["end"] == dt8
+    assert m["my_dict"]["start"] == "2025-01-01T00:00:00Z"
+    assert m["my_dict"]["end"] == "2025-12-31T23:59:59Z"
+
+    # Test 10: Cached object maintains datetime type
+    dict_ref1 = m.my_dict
+    dict_ref2 = m.my_dict
+    assert dict_ref1 is dict_ref2
+    assert isinstance(dict_ref1["start"], datetime)
+    assert isinstance(dict_ref2["start"], datetime)
+


The test assumes that datetime values in the cache remain as datetime objects while values in _data are serialized strings. However, the test doesn't verify what happens when you access m._data["my_dict"] directly after mutations via m.my_dict. This distinction between cached deserialized values and serialized storage values should be explicitly tested to ensure the sync-back mechanism works correctly with datetime serialization.

pvaneck · 2025-12-16T19:58:34Z

sdk/core/azure-core/tests/test_serialization.py

+def test_dictionary_set():
+    """Test that dictionary mutations via attribute syntax persist and sync to dictionary syntax."""
+
+    class MyModel(HybridModel):
+        my_dict: Dict[str, int] = rest_field(visibility=["read", "create", "update", "delete", "query"])
+
+    # Test 1: Basic mutation via attribute syntax
+    m = MyModel(my_dict={"a": 1, "b": 2})
+    assert m.my_dict == {"a": 1, "b": 2}
+
+    # Test 2: Add new key via attribute syntax and verify it persists
+    m.my_dict["c"] = 3
+    assert m.my_dict["c"] == 3
+    assert m.my_dict == {"a": 1, "b": 2, "c": 3}
+
+    # Test 3: Verify mutation is reflected in dictionary syntax
+    assert m["my_dict"] == {"a": 1, "b": 2, "c": 3}
+
+    # Test 4: Modify existing key via attribute syntax
+    m.my_dict["a"] = 100
+    assert m.my_dict["a"] == 100
+    assert m["my_dict"]["a"] == 100
+
+    # Test 5: Delete key via attribute syntax
+    del m.my_dict["b"]
+    assert "b" not in m.my_dict
+    assert "b" not in m["my_dict"]
+
+    # Test 6: Update via dict methods
+    m.my_dict.update({"d": 4, "e": 5})
+    assert m.my_dict["d"] == 4
+    assert m.my_dict["e"] == 5
+    assert m["my_dict"]["d"] == 4
+
+    # Test 7: Clear via attribute syntax and verify via dictionary syntax
+    m.my_dict.clear()
+    assert len(m.my_dict) == 0
+    assert len(m["my_dict"]) == 0
+
+    # Test 8: Reassign entire dictionary via attribute syntax
+    m.my_dict = {"x": 10, "y": 20}
+    assert m.my_dict == {"x": 10, "y": 20}
+    assert m["my_dict"] == {"x": 10, "y": 20}
+
+    # Test 9: Mutation after reassignment
+    m.my_dict["z"] = 30
+    assert m.my_dict["z"] == 30
+    assert m["my_dict"]["z"] == 30
+
+    # Test 10: Access via dictionary syntax first, then mutate via attribute syntax
+    m.my_dict["w"] = 40
+    assert m["my_dict"]["w"] == 40
+
+    # Test 11: Multiple accesses maintain same cached object
+    dict_ref1 = m.my_dict
+    dict_ref2 = m.my_dict
+    assert dict_ref1 is dict_ref2
+    dict_ref1["new_key"] = 999
+    assert dict_ref2["new_key"] == 999
+    assert m.my_dict["new_key"] == 999
+


I think a test case outlined by copilot here would be good.

pvaneck · 2025-12-16T20:01:40Z

sdk/core/azure-core/tests/test_serialization.py

+    assert m.my_dict["new_key"] == 999
+
+
+def test_list_set():


Can we also add a test like:

def test_repr(): class MyModel(HybridModel): my_dict: dict[str, int] = rest_field() m = MyModel(my_dict={"a": 1, "b": 2}) m.my_dict["c"] = 3 assert repr(m) == "{'my_dict': {'a': 1, 'b': 2, 'c': 3}}"

pvaneck · 2025-12-16T20:03:44Z

sdk/core/azure-core/tests/test_serialization.py

+def test_dictionary_set():
+    """Test that dictionary mutations via attribute syntax persist and sync to dictionary syntax."""
+
+    class MyModel(HybridModel):
+        my_dict: Dict[str, int] = rest_field(visibility=["read", "create", "update", "delete", "query"])
+
+    # Test 1: Basic mutation via attribute syntax
+    m = MyModel(my_dict={"a": 1, "b": 2})
+    assert m.my_dict == {"a": 1, "b": 2}
+
+    # Test 2: Add new key via attribute syntax and verify it persists
+    m.my_dict["c"] = 3
+    assert m.my_dict["c"] == 3
+    assert m.my_dict == {"a": 1, "b": 2, "c": 3}
+
+    # Test 3: Verify mutation is reflected in dictionary syntax
+    assert m["my_dict"] == {"a": 1, "b": 2, "c": 3}
+
+    # Test 4: Modify existing key via attribute syntax
+    m.my_dict["a"] = 100
+    assert m.my_dict["a"] == 100
+    assert m["my_dict"]["a"] == 100
+
+    # Test 5: Delete key via attribute syntax
+    del m.my_dict["b"]
+    assert "b" not in m.my_dict
+    assert "b" not in m["my_dict"]
+
+    # Test 6: Update via dict methods
+    m.my_dict.update({"d": 4, "e": 5})
+    assert m.my_dict["d"] == 4
+    assert m.my_dict["e"] == 5
+    assert m["my_dict"]["d"] == 4
+
+    # Test 7: Clear via attribute syntax and verify via dictionary syntax
+    m.my_dict.clear()
+    assert len(m.my_dict) == 0
+    assert len(m["my_dict"]) == 0
+
+    # Test 8: Reassign entire dictionary via attribute syntax
+    m.my_dict = {"x": 10, "y": 20}
+    assert m.my_dict == {"x": 10, "y": 20}
+    assert m["my_dict"] == {"x": 10, "y": 20}
+
+    # Test 9: Mutation after reassignment
+    m.my_dict["z"] = 30
+    assert m.my_dict["z"] == 30
+    assert m["my_dict"]["z"] == 30
+
+    # Test 10: Access via dictionary syntax first, then mutate via attribute syntax
+    m.my_dict["w"] = 40
+    assert m["my_dict"]["w"] == 40
+
+    # Test 11: Multiple accesses maintain same cached object
+    dict_ref1 = m.my_dict
+    dict_ref2 = m.my_dict
+    assert dict_ref1 is dict_ref2
+    dict_ref1["new_key"] = 999
+    assert dict_ref2["new_key"] == 999
+    assert m.my_dict["new_key"] == 999
+


I found that the following case:

def test_both_item_attr_access(): class MyModel(HybridModel): my_dict: dict[str, int] = rest_field() m = MyModel(my_dict={"a": 1, "b": 2}) m.my_dict["c"] = 3 assert m.my_dict["c"] == 3 m["my_dict"]["d"] = 4 assert m.my_dict["d"] == 4 assert m.my_dict == {"a": 1, "b": 2, "c": 3, "d": 4}

yields a:

> assert m.my_dict["d"] == 4 E KeyError: 'd'

iscai-msft added 2 commits December 15, 2025 16:41

repro

8062b97

persist settings to attribute syntax version of dict properties

b7e9efa

iscai-msft marked this pull request as ready for review December 16, 2025 18:59

Copilot AI review requested due to automatic review settings December 16, 2025 18:59

iscai-msft requested review from annatisch, johanste, kashifkhan, lmazuel, pvaneck and xiangyan99 as code owners December 16, 2025 18:59

Copilot started reviewing on behalf of iscai-msft December 16, 2025 19:00 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

pvaneck reviewed Dec 16, 2025

View reviewed changes

fix setting with dict syntax

3de611b

-        except AttributeError:
-            pass
+        except AttributeError as exc:
+            _LOGGER.debug(
+                "Failed to cache deserialized attribute '%s' on object of type '%s': %s",
+                cache_attr, type(obj).__name__, exc
+            )

[core] set value of dict property not working #44435

Are you sure you want to change the base?

[core] set value of dict property not working #44435

Conversation

iscai-msft commented Dec 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

pvaneck Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

pvaneck Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

pvaneck Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

pvaneck Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

pvaneck Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants