Skip to content

Commit

Permalink
Ensure only a single object instance exists per tree object (#415)
Browse files Browse the repository at this point in the history
Ensure only a single instance refers to the same tree object or container.

Add a mixin class `ObjectCacheMixin` which adds a class attribute 
`_OBJECT_CACHE` to each of its subclasses. The object cache is a 
`WeakValueDictionary`, meaning the instances will be properly deleted
once their refcount _outside_ of the object cache goes to zero.

The `ObjectCacheMixin` subclasses need to provide a method `_cache_key_valid`; 
this enables making certain cache key values disallowed. In particular, empty 
resource paths are not considered valid for tree objects.

A decorator `constructor_with_cache` is provided to decorate constructor
classmethods s.t. they use the cache.

The arguments to the `__init__` method in the container classes `Mapping`, 
`MutableMapping`, `LinkedObjectList` and `EdgePropertyList` are prefixed
with underscores, to prevent accidental use of the direct constructor.

Tree objects which are directly instantiated are added to the cache during 
`.store()`.
  • Loading branch information
greschd authored Feb 14, 2024
1 parent 77b25ba commit 86c3806
Show file tree
Hide file tree
Showing 8 changed files with 508 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from google.protobuf.message import Message
from typing_extensions import Self

from .._object_cache import ObjectCacheMixin, constructor_with_cache
from ..base import CreatableTreeObject
from .property_helper import _exposed_grpc_property, _wrap_doc, grpc_data_getter, grpc_data_setter

Expand Down Expand Up @@ -41,7 +42,7 @@ def _set_callback_apply_changes(self, callback_apply_changes: Callable[[], None]
ValueT = TypeVar("ValueT", bound=GenericEdgePropertyType)


class EdgePropertyList(MutableSequence[ValueT]):
class EdgePropertyList(ObjectCacheMixin, MutableSequence[ValueT]):
"""Wrap graph edges of a specific type.
Wraps links between objects of a specific type, for instance FabricWithAngle
Expand All @@ -61,31 +62,75 @@ class EdgePropertyList(MutableSequence[ValueT]):
For instance, element sets of an oriented element set.
"""

def __init__(
self,
@classmethod
@constructor_with_cache(
# NOTE greschd Feb'23:
# We use the parent object's id() as part of the cache key since
# the LinkedObjectList keeps its parent alive. This means the
# id() does not change (and cannot be reused) as long as the
# cache entry is alive.
# This is somewhat incidental, but checked (indirectly) by the
# object permanence tests. If we want to get rid of this, we
# would instead need to find a way to handle the case where the
# parent object is being stored.
key_getter=lambda *args, parent_object, attribute_name, **kwargs: (
id(parent_object),
attribute_name,
),
raise_on_invalid_key=True,
)
def _initialize_with_cache(
cls: type[Self],
*,
parent_object: CreatableTreeObject,
object_type: type[GenericEdgePropertyType],
attribute_name: str,
from_pb_constructor: Callable[[CreatableTreeObject, Message, Callable[[], None]], ValueT],
) -> Self:
return cls(
_parent_object=parent_object,
_object_type=object_type,
_attribute_name=attribute_name,
_from_pb_constructor=from_pb_constructor,
)

@staticmethod
def _cache_key_valid(key: Any) -> bool:
try:
(parent_object_id, attribute_name) = key
if not attribute_name:
return False
if not isinstance(parent_object_id, int):
return False
return True
except Exception:
return False

def __init__(
self,
*,
_parent_object: CreatableTreeObject,
_object_type: type[GenericEdgePropertyType],
_attribute_name: str,
_from_pb_constructor: Callable[[CreatableTreeObject, Message, Callable[[], None]], ValueT],
) -> None:
getter = grpc_data_getter(attribute_name, from_protobuf=list)
setter = grpc_data_setter(attribute_name, to_protobuf=lambda x: x)
getter = grpc_data_getter(_attribute_name, from_protobuf=list)
setter = grpc_data_setter(_attribute_name, to_protobuf=lambda x: x)

self._parent_object = parent_object
self._object_type = object_type
self._name = attribute_name.split(".")[-1]
self._parent_object = _parent_object
self._object_type = _object_type
self._name = _attribute_name.split(".")[-1]

self._object_constructor: Callable[
[Message], ValueT
] = lambda pb_object: from_pb_constructor(
] = lambda pb_object: _from_pb_constructor(
self._parent_object, pb_object, self._apply_changes
)

# get initial object list
def get_object_list_from_parent_object() -> list[ValueT]:
obj_list = []
for item in getter(parent_object):
for item in getter(_parent_object):
obj_list.append(self._object_constructor(item))
return obj_list

Expand All @@ -101,7 +146,7 @@ def set_object_list(items: list[ValueT]) -> None:
# update callback in case item was copied from another tree object
# or if it is a new object
item._set_callback_apply_changes(self._apply_changes)
setter(parent_object, pb_obj_list)
setter(_parent_object, pb_obj_list)
# keep object list in sync with the backend. This is needed for the in-place editing
self._object_list = items

Expand Down Expand Up @@ -291,7 +336,7 @@ def define_edge_property_list(
"""Define a list of linked tree objects with link properties."""

def getter(self: CreatableTreeObject) -> EdgePropertyList[GenericEdgePropertyType]:
return EdgePropertyList(
return EdgePropertyList._initialize_with_cache(
parent_object=self,
object_type=value_type,
attribute_name=attribute_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

from grpc import Channel
import numpy as np
from typing_extensions import Self

from ansys.api.acp.v0.base_pb2 import ResourcePath

from .._object_cache import ObjectCacheMixin, constructor_with_cache
from ..base import CreatableTreeObject, TreeObject
from .polymorphic_from_pb import tree_object_from_resource_path
from .property_helper import _exposed_grpc_property, _wrap_doc, grpc_data_getter, grpc_data_setter
Expand All @@ -20,33 +22,75 @@
__all__ = ["LinkedObjectList", "define_linked_object_list"]


class LinkedObjectList(MutableSequence[ValueT]):
class LinkedObjectList(ObjectCacheMixin, MutableSequence[ValueT]):
"""List of linked tree objects."""

def __init__(
self,
@classmethod
@constructor_with_cache(
# NOTE greschd Feb'23:
# We use the parent object's id() as part of the cache key since
# the LinkedObjectList keeps its parent alive. This means the
# id() does not change (and cannot be reused) as long as the
# cache entry is alive.
# This is somewhat incidental, but checked (indirectly) by the
# object permanence tests. If we want to get rid of this, we
# would instead need to find a way to handle the case where the
# parent object is being stored.
key_getter=lambda *args, parent_object, attribute_name, **kwargs: (
id(parent_object),
attribute_name,
),
raise_on_invalid_key=True,
)
def _initialize_with_cache(
cls: type[Self],
*,
parent_object: TreeObject,
attribute_name: str,
object_constructor: Callable[[ResourcePath, Channel], ValueT],
) -> Self:
return cls(
_parent_object=parent_object,
_attribute_name=attribute_name,
_object_constructor=object_constructor,
)

@staticmethod
def _cache_key_valid(key: Any) -> bool:
try:
(parent_object_id, attribute_name) = key
if not attribute_name:
return False
if not isinstance(parent_object_id, int):
return False
return True
except Exception:
return False

def __init__(
self,
*,
_parent_object: TreeObject,
_attribute_name: str,
_object_constructor: Callable[[ResourcePath, Channel], ValueT],
) -> None:
getter = grpc_data_getter(attribute_name, from_protobuf=list)
setter = grpc_data_setter(attribute_name, to_protobuf=lambda x: x)
getter = grpc_data_getter(_attribute_name, from_protobuf=list)
setter = grpc_data_setter(_attribute_name, to_protobuf=lambda x: x)

self._get_resourcepath_list = cast(
Callable[[], list[ResourcePath]], lambda: getter(parent_object)
Callable[[], list[ResourcePath]], lambda: getter(_parent_object)
)

def set_resourcepath_list(value: list[ResourcePath]) -> None:
if not all([rp.value for rp in value]):
# Check for empty resource paths
raise RuntimeError("Cannot link to unstored objects.")
setter(parent_object, value)
setter(_parent_object, value)

self._set_resourcepath_list = set_resourcepath_list
self._object_constructor: Callable[
[ResourcePath], ValueT
] = lambda resource_path: object_constructor(resource_path, parent_object._channel)
] = lambda resource_path: _object_constructor(resource_path, _parent_object._channel)

def __len__(self) -> int:
return len(self._get_resourcepath_list())
Expand Down Expand Up @@ -227,7 +271,7 @@ def define_linked_object_list(
"""Define a list of linked tree objects."""

def getter(self: ValueT) -> LinkedObjectList[ChildT]:
return LinkedObjectList(
return LinkedObjectList._initialize_with_cache(
parent_object=self,
attribute_name=attribute_name,
object_constructor=object_class._from_resource_path,
Expand All @@ -246,9 +290,11 @@ def define_polymorphic_linked_object_list(

def getter(self: ValueT) -> LinkedObjectList[Any]:
return LinkedObjectList(
parent_object=self,
attribute_name=attribute_name,
object_constructor=partial(tree_object_from_resource_path, allowed_types=allowed_types),
_parent_object=self,
_attribute_name=attribute_name,
_object_constructor=partial(
tree_object_from_resource_path, allowed_types=allowed_types
),
)

def setter(self: ValueT, value: list[Any]) -> None:
Expand Down
84 changes: 65 additions & 19 deletions src/ansys/acp/core/_tree_objects/_grpc_helpers/mapping.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
from __future__ import annotations

from collections.abc import Iterator
from typing import Callable, Generic, TypeVar
from typing import Any, Callable, Generic, TypeVar

from grpc import Channel
from typing_extensions import Concatenate, ParamSpec
from typing_extensions import Concatenate, ParamSpec, Self

from ansys.api.acp.v0.base_pb2 import CollectionPath, DeleteRequest, ListRequest

from ..._utils.property_protocols import ReadOnlyProperty
from ..._utils.resource_paths import join as _rp_join
from .._object_cache import ObjectCacheMixin, constructor_with_cache
from ..base import CreatableTreeObject, TreeObject, TreeObjectBase
from .exceptions import wrap_grpc_errors
from .property_helper import _exposed_grpc_property, _wrap_doc
Expand All @@ -21,26 +22,52 @@
__all__ = ["Mapping", "MutableMapping", "define_mutable_mapping", "define_create_method"]


class Mapping(Generic[ValueT]):
class Mapping(ObjectCacheMixin, Generic[ValueT]):
"""Mapping interface for collections of TreeObjects.
Note: We could derive from collections.abc.Mapping to make sure
this class conforms to the Mapping interface.
"""

def __init__(
self,
@classmethod
@constructor_with_cache(
key_getter=lambda *args, collection_path, **kwargs: collection_path.value,
raise_on_invalid_key=True,
)
def _initialize_with_cache(
cls,
*,
channel: Channel,
collection_path: CollectionPath,
stub: ReadableResourceStub,
object_constructor: Callable[[ObjectInfo, Channel | None], ValueT],
) -> Self:
return cls(
_channel=channel,
_collection_path=collection_path,
_stub=stub,
_object_constructor=object_constructor,
)

def __init__(
self,
*,
_channel: Channel,
_collection_path: CollectionPath,
_stub: ReadableResourceStub,
_object_constructor: Callable[[ObjectInfo, Channel | None], ValueT],
) -> None:
self._collection_path = collection_path
self._stub = stub
self._collection_path = _collection_path
self._stub = _stub

self._channel = _channel
self._object_constructor = _object_constructor

self._channel = channel
self._object_constructor = object_constructor
@staticmethod
def _cache_key_valid(key: Any) -> bool:
if isinstance(key, str):
return bool(key)
return False

def __iter__(self) -> Iterator[str]:
yield from (obj.info.id for obj in self._get_objectinfo_list())
Expand Down Expand Up @@ -121,19 +148,38 @@ def __repr__(self) -> str:
class MutableMapping(Mapping[CreatableValueT]):
"""Mutable mapping interface for collections of TreeObjects."""

def __init__(
self,
@classmethod
@constructor_with_cache(
key_getter=lambda *args, collection_path, **kwargs: collection_path.value,
raise_on_invalid_key=True,
)
def _initialize_with_cache(
cls,
*,
channel: Channel,
collection_path: CollectionPath,
stub: EditableAndReadableResourceStub,
stub: EditableAndReadableResourceStub, # type: ignore # violates Liskov substitution
object_constructor: Callable[[ObjectInfo, Channel | None], CreatableValueT],
) -> None:
self._collection_path = collection_path
self._stub: EditableAndReadableResourceStub = stub
) -> Self:
return cls(
_channel=channel,
_collection_path=collection_path,
_stub=stub,
_object_constructor=object_constructor,
)

self._channel = channel
self._object_constructor = object_constructor
def __init__(
self,
*,
_channel: Channel,
_collection_path: CollectionPath,
_stub: EditableAndReadableResourceStub,
_object_constructor: Callable[[ObjectInfo, Channel | None], CreatableValueT],
) -> None:
self._collection_path = _collection_path
self._stub: EditableAndReadableResourceStub = _stub
self._channel = _channel
self._object_constructor = _object_constructor

def __delitem__(self, key: str) -> None:
obj_info = self._get_objectinfo_by_id(key)
Expand Down Expand Up @@ -180,7 +226,7 @@ def get_read_only_collection_property(
"""Define a read-only mapping of child tree objects."""

def collection_property(self: ParentT) -> Mapping[ValueT]:
return Mapping(
return Mapping._initialize_with_cache(
channel=self._channel,
collection_path=CollectionPath(
value=_rp_join(self._resource_path.value, object_class._COLLECTION_LABEL)
Expand Down Expand Up @@ -226,7 +272,7 @@ def define_mutable_mapping(
"""Define a mutable mapping of child tree objects."""

def collection_property(self: ParentT) -> MutableMapping[CreatableValueT]:
return MutableMapping(
return MutableMapping._initialize_with_cache(
channel=self._channel,
collection_path=CollectionPath(
value=_rp_join(self._resource_path.value, object_class._COLLECTION_LABEL)
Expand Down
Loading

0 comments on commit 86c3806

Please sign in to comment.