diff --git a/ic_python_db/db_engine.py b/ic_python_db/db_engine.py index 84da046..6c0ee8a 100644 --- a/ic_python_db/db_engine.py +++ b/ic_python_db/db_engine.py @@ -366,6 +366,66 @@ def check_upgrade_compatibility(self, raise_on_error: bool = True): return check_upgrade_compatibility(self, raise_on_error=raise_on_error) + # ── Reverse Index API ────────────────────────────────────────────────────── + # + # Reverse indexes allow OneToMany / ManyToMany / inverse-OneToOne relations + # to be resolved without scanning all child entities. + # + # Key format: _ri:{ParentType}:{parent_id}:{relation_name} + # Value: JSON array of child entity IDs, e.g. ["1", "2", "3"] + + def _ri_key(self, parent_type: str, parent_id: str, relation_name: str) -> str: + return f"_ri:{parent_type}:{parent_id}:{relation_name}" + + def reverse_index_get( + self, parent_type: str, parent_id: str, relation_name: str + ) -> List[str]: + """Read the reverse index for a parent entity's relation. + + Returns: + List of child entity IDs, or empty list if no index exists. + """ + key = self._ri_key(parent_type, parent_id, relation_name) + raw = self._db_storage.get(key) + if raw: + return json.loads(raw) + return [] + + def reverse_index_add( + self, parent_type: str, parent_id: str, relation_name: str, child_id: str + ) -> None: + """Add a child ID to a parent's reverse index.""" + key = self._ri_key(parent_type, parent_id, relation_name) + raw = self._db_storage.get(key) + ids = json.loads(raw) if raw else [] + if child_id not in ids: + ids.append(child_id) + self._db_storage.insert(key, json.dumps(ids)) + + def reverse_index_remove( + self, parent_type: str, parent_id: str, relation_name: str, child_id: str + ) -> None: + """Remove a child ID from a parent's reverse index.""" + key = self._ri_key(parent_type, parent_id, relation_name) + raw = self._db_storage.get(key) + if not raw: + return + ids = json.loads(raw) + if child_id in ids: + ids.remove(child_id) + if ids: + self._db_storage.insert(key, json.dumps(ids)) + else: + self._db_storage.remove(key) + + def reverse_index_delete( + self, parent_type: str, parent_id: str, relation_name: str + ) -> None: + """Delete an entire reverse index entry (used when parent is deleted).""" + key = self._ri_key(parent_type, parent_id, relation_name) + if self._db_storage.get(key) is not None: + self._db_storage.remove(key) + def get_audit( self, id_from: Optional[int] = None, id_to: Optional[int] = None ) -> Dict[str, str]: diff --git a/ic_python_db/entity.py b/ic_python_db/entity.py index 80a7ccf..c3128e0 100644 --- a/ic_python_db/entity.py +++ b/ic_python_db/entity.py @@ -71,7 +71,6 @@ class AdminUser(Entity): _id (str): Unique sequential identifier ("1", "2", "3", ...) _loaded (bool): True if loaded from DB, False if newly created _counted (bool): True if entity has been counted (prevents double-counting) - _relations (dict): Dictionary mapping relation names to related entities _do_not_save (bool): Temporary flag to prevent saving during initialization Class-level attributes: @@ -161,8 +160,6 @@ class User(Entity): self._loaded = False if kwargs.get("_loaded") is None else kwargs["_loaded"] self._counted = False # Track if this entity has been counted - self._relations = {} - # Add to context self.__class__._context.add(self) @@ -439,62 +436,62 @@ def load( if stored_version != current_version: entity._save() - # Restore legacy "relations" block if present in serialized data. - # Otherwise keep the _relations that __init__ already populated - # via relationship descriptors (OneToMany, ManyToOne, etc.). - if "relations" in data: - relations_data = data.pop("relations") - relations = {} - for rel_name, rel_refs in relations_data.items(): - relations[rel_name] = [] - for ref in rel_refs: - related = ( - Entity.db() - ._entity_types[ref["_type"]] - .load(ref["_id"], level=level - 1) - ) - if related: - relations[rel_name].append(related) - entity._relations = relations - return entity @classmethod def find(cls: Type[T], d) -> List[T]: - D = d - L = [_.serialize() for _ in cls.instances()] + """Find entities matching a dictionary filter. + + Uses load_some over the full ID range. Prefer indexed lookups where possible. + """ + max_id = cls.max_id() + if max_id == 0: + return [] + all_entities = cls.load_some(1, max_id) return [ - cls.load(d["_id"]) for d in L if all(d.get(k) == v for k, v in D.items()) + e + for e in all_entities + if all(getattr(e, k, None) == v for k, v in d.items()) ] @classmethod def instances(cls: Type[T]) -> List[T]: """Get all instances of this entity type, including subclass instances. - Uses load_some() for O(max_id) performance instead of scanning all keys. + .. deprecated:: + This method loads ALL entities and is O(max_id). Prefer load_some() + with pagination or use relationship accessors. Returns: List of entities """ + try: + import warnings + + warnings.warn( + "Entity.instances() is deprecated and will be removed. " + "Use load_some() with pagination instead.", + DeprecationWarning, + stacklevel=2, + ) + except (ImportError, AttributeError): + pass db = Database.get_instance() full_type_name = cls.get_full_type_name() db.register_entity_type(cls, full_type_name) - # Use load_some for O(max_id) instead of O(total_keys) max_id = cls.max_id() if max_id == 0: instances = [] else: instances = cls.load_some(1, max_id) - # Also check for subclass instances - # Track processed classes to avoid duplicates when types are registered under multiple names processed_classes = {cls} for type_name, type_cls in db._entity_types.items(): if type_cls in processed_classes: - continue # Skip already processed classes + continue if type_name == full_type_name or type_name == cls.__name__: - continue # Skip self + continue if db.is_subclass(type_name, cls): processed_classes.add(type_cls) subclass_max_id = type_cls.max_id() @@ -572,10 +569,11 @@ def load_some( return ret def delete(self) -> None: + """Delete this entity from the database, cleaning up all reverse indexes.""" logger.debug(f"Deleting entity {self._type}@{self._id}") - """Delete this entity from the database.""" from .constants import ACTION_DELETE from .hooks import call_entity_hook + from .properties import ManyToMany, ManyToOne, OneToMany, OneToOne # Call hook before deletion allow, _ = call_entity_hook(self, None, self, None, ACTION_DELETE) @@ -583,17 +581,74 @@ def delete(self) -> None: if not allow: raise PermissionError("Hook rejected entity deletion") - self.db().delete(self._type, self._id) + db = self.db() + + # Clean up reverse indexes for all relation properties + for cls in reversed(self.__class__.__mro__): + for attr_name, attr_value in cls.__dict__.items(): + if attr_name.startswith("_"): + continue + if isinstance(attr_value, ManyToOne): + # Remove self from parent's reverse index + parent_ref = self.__dict__.get(f"_rel_{attr_name}") + if parent_ref is not None: + for type_name in attr_value._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class and db.load(type_name, parent_ref): + db.reverse_index_remove( + type_name, + parent_ref, + attr_value.reverse_name, + self._id, + ) + break + elif isinstance(attr_value, OneToOne): + # Remove self from target's reverse index + target_ref = self.__dict__.get(f"_rel_{attr_name}") + if target_ref is not None: + for type_name in attr_value._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class and db.load(type_name, target_ref): + db.reverse_index_remove( + type_name, + target_ref, + attr_value.reverse_name, + self._id, + ) + break + # Also delete any reverse index where self is parent (inverse side) + db.reverse_index_delete(self._type, self._id, attr_name) + elif isinstance(attr_value, OneToMany): + # Delete the reverse index entry for this parent + db.reverse_index_delete(self._type, self._id, attr_name) + elif isinstance(attr_value, ManyToMany): + # Remove self from each related entity's reverse index + related_ids = db.reverse_index_get(self._type, self._id, attr_name) + for related_id in related_ids: + for type_name in attr_value._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class and db.load(type_name, related_id): + db.reverse_index_remove( + type_name, + related_id, + attr_value.reverse_name, + self._id, + ) + break + # Delete own index + db.reverse_index_delete(self._type, self._id, attr_name) + + db.delete(self._type, self._id) # Remove from entity registry - self.db().unregister_entity(self._type, self._id) + db.unregister_entity(self._type, self._id) # Decrement the count when an entity is deleted type_name = self.__class__.get_full_type_name() count_key = f"{type_name}_count" - current_count = int(self.db().load("_system", count_key) or 0) + current_count = int(db.load("_system", count_key) or 0) if current_count > 0: - self.db().save("_system", count_key, str(current_count - 1)) + db.save("_system", count_key, str(current_count - 1)) else: raise ValueError( f"Entity count for {type_name} is already zero; cannot decrement further." @@ -605,7 +660,7 @@ def delete(self) -> None: if hasattr(self, alias_field): alias_value = getattr(self, alias_field) if alias_value is not None: - self.db().delete(self._alias_key(), alias_value) + db.delete(self._alias_key(), alias_value) logger.debug(f"Deleted entity {self._type}@{self._id}") @@ -651,61 +706,73 @@ def _get_entity_reference(entity): return entity._id def _serialize_full(self) -> Dict[str, Any]: - """Full serialization including all relations. Used by _save() for persistence.""" + """Full serialization including relation FK references. Used by _save().""" data = self._serialize_base() - from ic_python_db.properties import ManyToMany, OneToMany - - for rel_name, rel_entities in self._relations.items(): - if rel_entities: - rel_prop = getattr(self.__class__, rel_name, None) - is_to_many = isinstance(rel_prop, (OneToMany, ManyToMany)) + from ic_python_db.properties import ManyToOne, OneToOne - if len(rel_entities) == 1 and not is_to_many: - data[rel_name] = self._get_entity_reference(rel_entities[0]) - else: - data[rel_name] = [ - self._get_entity_reference(e) for e in rel_entities - ] + for cls in reversed(self.__class__.__mro__): + for attr_name, attr_value in cls.__dict__.items(): + if attr_name.startswith("_"): + continue + if isinstance(attr_value, (ManyToOne, OneToOne)): + ref = self.__dict__.get(f"_rel_{attr_name}") + if ref is not None: + data[attr_name] = ref return data + def _resolve_ref_with_alias(self, entity_id: str, allowed_types: list) -> str: + """Resolve an entity ID to its best reference (alias value or ID).""" + db = self.db() + for type_name in allowed_types: + entity_class = db._entity_types.get(type_name) + if entity_class: + entity = entity_class.load(entity_id) + if entity: + return self._get_entity_reference(entity) + return entity_id + def serialize(self) -> Dict[str, Any]: """Convert the entity to a portable serializable dictionary. - OneToMany relations are skipped (reconstructed from reverse ManyToOne). - For bilateral OneToOne relations, only the alphabetically-earlier entity - type serializes the reference, avoiding circular dependencies. + Includes: ManyToOne FK, owning-side OneToOne FK, ManyToMany entries. + Excludes: OneToMany (reconstructed from ManyToOne during import). Returns: Dict containing the entity's serializable data """ data = self._serialize_base() - from ic_python_db.properties import ManyToMany, OneToMany, OneToOne + from ic_python_db.properties import ManyToMany, ManyToOne, OneToOne - for rel_name, rel_entities in self._relations.items(): - if rel_entities: - rel_prop = getattr(self.__class__, rel_name, None) + db = self.db() - # Skip OneToMany — always reconstructed from reverse ManyToOne - if isinstance(rel_prop, OneToMany): + for cls in reversed(self.__class__.__mro__): + for attr_name, attr_value in cls.__dict__.items(): + if attr_name.startswith("_"): continue - # For OneToOne bilateral, only serialize on one deterministic side: - # the entity whose type name is alphabetically <= the target type. - if isinstance(rel_prop, OneToOne): - target_type = rel_entities[0]._type if rel_entities else None - if target_type and self._type > target_type: - continue - - is_to_many = isinstance(rel_prop, (OneToMany, ManyToMany)) - - if len(rel_entities) == 1 and not is_to_many: - data[rel_name] = self._get_entity_reference(rel_entities[0]) - else: - data[rel_name] = [ - self._get_entity_reference(e) for e in rel_entities - ] + if isinstance(attr_value, ManyToOne): + ref = self.__dict__.get(f"_rel_{attr_name}") + if ref is not None: + data[attr_name] = self._resolve_ref_with_alias( + ref, attr_value._get_allowed_types() + ) + elif isinstance(attr_value, OneToOne): + ref = self.__dict__.get(f"_rel_{attr_name}") + if ref is not None: + data[attr_name] = self._resolve_ref_with_alias( + ref, attr_value._get_allowed_types() + ) + elif isinstance(attr_value, ManyToMany): + related_ids = db.reverse_index_get(self._type, self._id, attr_name) + if related_ids: + data[attr_name] = [ + self._resolve_ref_with_alias( + rid, attr_value._get_allowed_types() + ) + for rid in related_ids + ] return data @@ -970,70 +1037,3 @@ def __hash__(self) -> int: Hash value """ return hash((self._type, self._id)) - - def add_relation(self, from_rel: str, to_rel: str, other: "Entity") -> None: - """Add a bidirectional relationship with another entity. - - Args: - from_rel: Name of relation from this entity to other - to_rel: Name of relation from other entity to this - other: Entity to create relationship with - """ - # Add forward relation - if from_rel not in self._relations: - self._relations[from_rel] = [] - if other not in self._relations[from_rel]: - self._relations[from_rel].append(other) - - # Add reverse relation - if to_rel not in other._relations: - other._relations[to_rel] = [] - if self not in other._relations[to_rel]: - other._relations[to_rel].append(self) - - # Save both entities - self._save() - other._save() - - def get_relations( - self, relation_name: str, entity_type: str = None - ) -> List["Entity"]: - """Get all related entities for a relation, optionally filtered by type. - - Args: - relation_name: Name of the relation to follow - entity_type: Optional type name to filter entities by - - Returns: - List of related entities - """ - if relation_name not in self._relations: - return [] - - entities = self._relations[relation_name] - if entity_type: - entities = [e for e in entities if e._type == entity_type] - - return entities - - def remove_relation(self, from_rel: str, to_rel: str, other: "Entity") -> None: - """Remove a bidirectional relationship with another entity. - - Args: - from_rel: Name of relation from this entity to other - to_rel: Name of relation from other entity to this - other: Entity to remove relationship with - """ - # Remove forward relation - if from_rel in self._relations: - if other in self._relations[from_rel]: - self._relations[from_rel].remove(other) - - # Remove reverse relation - if to_rel in other._relations: - if self in other._relations[to_rel]: - other._relations[to_rel].remove(self) - - # Save both entities - self._save() - other._save() diff --git a/ic_python_db/properties.py b/ic_python_db/properties.py index c6290ee..381fba0 100644 --- a/ic_python_db/properties.py +++ b/ic_python_db/properties.py @@ -1,28 +1,28 @@ -"""Property definitions for Entity classes.""" +"""Property definitions for Entity classes. + +Relation properties (ManyToOne, OneToMany, OneToOne, ManyToMany) use persisted +reverse indexes in stable storage so that relationships can be resolved without +scanning all entities. +""" from typing import ( TYPE_CHECKING, Any, Callable, Generic, - Iterator, List, Optional, Type, TypeVar, Union, - overload, ) if TYPE_CHECKING: from .entity import Entity -# TypeVar for property value types T = TypeVar("T") -# TypeVar for entity types in relations E = TypeVar("E", bound="Entity") -# Prefix used for storing property values in entity __dict__ PROPERTY_STORAGE_PREFIX = "prop" @@ -30,13 +30,6 @@ class Property(Generic[T]): """Definition of an entity property. A generic descriptor class that provides type-safe property access. - The type parameter T indicates the type of value this property holds. - - Attributes: - name: Name of the property - type: Type of the property (e.g. str, int) - default: Default value if not set - validator: Optional function to validate values """ name: str @@ -57,29 +50,19 @@ def __init__( self.validator = validator def __set_name__(self, owner: type, name: str) -> None: - """Set the property name when class is created.""" self.name = name - @overload - def __get__(self, obj: None, objtype: Optional[type]) -> "Property[T]": ... - - @overload - def __get__(self, obj: object, objtype: Optional[type]) -> Optional[T]: ... - def __get__( self, obj: object, objtype: Optional[type] = None ) -> Union["Property[T]", Optional[T]]: - """Get the property value.""" if obj is None: - return self + return self # type: ignore[return-value] return obj.__dict__.get(f"_{PROPERTY_STORAGE_PREFIX}_{self.name}", self.default) def __set__(self, obj, value): - """Set the property value with type checking and validation.""" from .constants import ACTION_CREATE, ACTION_MODIFY from .hooks import call_entity_hook - # Get old value and determine action old_value = obj.__dict__.get( f"_{PROPERTY_STORAGE_PREFIX}_{self.name}", self.default ) @@ -89,7 +72,6 @@ def __set__(self, obj, value): else ACTION_MODIFY ) - # Call hook before setting allow, modified_value = call_entity_hook( obj, self.name, old_value, value, action ) @@ -185,113 +167,47 @@ def __init__(self, default: Optional[bool] = None): super().__init__(name="", type=bool, default=default) -class Relation(Generic[E]): - """Base property for defining and accessing relations. +# ── Relation Properties ─────────────────────────────────────────────────────── - This is the base class for all relation properties. It provides common functionality - for managing relationships between entities. - For one-to-one relationships (default), this property returns a single entity and - enforces single cardinality. For one-to-many or many-to-many relationships, it - returns a list of entities. +class Relation(Generic[E]): + """Base class for relation properties. - The type parameter E indicates the type of related entity. + Provides shared utilities for resolving and validating related entities. + Subclasses implement specific relationship semantics using persisted + reverse indexes. """ name: Optional[str] entity_types: Union[str, List[str]] reverse_name: Optional[str] - many: bool def __init__( self, entity_types: Union[str, List[str]], reverse_name: Optional[str] = None, - many: bool = False, ): - """Initialize relation property. - - Args: - entity_types: Type names of related entities - reverse_name: Optional name for reverse relation - many: Whether this property can hold multiple entities (default: False) - """ self.entity_types = entity_types self.name = None self.reverse_name = reverse_name - self.many = many def __set_name__(self, owner: type, name: str) -> None: - """Set the property name when class is created.""" self.name = name if self.reverse_name is None: self.reverse_name = name - @overload - def __get__(self, obj: None, objtype: Optional[type]) -> "Relation[E]": ... - - @overload - def __get__( - self, obj: object, objtype: Optional[type] - ) -> Union[Optional[E], List[E]]: ... - - def __get__( - self, obj: object, objtype: Optional[type] = None - ) -> Union["Relation[E]", Optional[E], List[E]]: - """Get related entities.""" - if obj is None: - return self - relations = obj.get_relations(self.name) - if not self.many: - # For single relationships, return the first entity or None - return relations[0] if relations else None - return relations + @property + def many(self) -> bool: + """Whether this relation holds multiple entities.""" + return isinstance(self, (OneToMany, ManyToMany)) - def __set__(self, obj, value): - """Set related entities. - - Args: - value: For many=False: a single Entity instance - For many=True: a list/tuple of Entity instances - """ - if self.many: - if not isinstance(value, (list, tuple)): - raise TypeError(f"{self.name} requires a list or tuple of entities") - values_list = value - else: - values_list = [value] - - # Get existing and new relations as sets - existing = set(obj.get_relations(self.name)) - new = set(values_list) - - # For one-to-many, check if entities have existing relations - if isinstance(self, OneToMany): - for entity in new: - existing_relations = entity.get_relations(self.reverse_name) - if existing_relations: - old_relation = existing_relations[0] - if old_relation != obj: - # Remove the entity from the old relation's list - old_relation._relations[self.name].remove(entity) - # Remove the old relation from the entity's list - entity._relations[self.reverse_name].remove(old_relation) - - # Remove relations that are not in new set - to_remove = existing - new - for entity in to_remove: - obj.remove_relation(self.name, self.reverse_name, entity) - - # Add relations that are not in existing set - to_add = new - existing - for entity in to_add: - obj.add_relation(self.name, self.reverse_name, entity) + def _get_allowed_types(self) -> List[str]: + if isinstance(self.entity_types, str): + return [self.entity_types] + return list(self.entity_types) def validate_entity(self, entity: Any) -> bool: - """Validate that an entity is of the correct type. - - Handles both namespaced (e.g., "app::User") and non-namespaced (e.g., "User") types. - """ + """Validate that an entity is of the correct type.""" from .entity import Entity if entity is None: @@ -299,17 +215,8 @@ def validate_entity(self, entity: Any) -> bool: if not isinstance(entity, Entity): raise TypeError(f"{self.name} must be set to Entity instances") - # Convert entity_types to list for uniform handling - allowed_types = ( - [self.entity_types] - if isinstance(self.entity_types, str) - else self.entity_types - ) - - # Check if entity type matches any allowed type - # This handles both exact matches and namespace variations + allowed_types = self._get_allowed_types() if entity._type not in allowed_types: - # Also check if the class name matches (for backward compatibility) entity_class_name = ( entity._type.split("::")[-1] if "::" in entity._type else entity._type ) @@ -319,22 +226,13 @@ def validate_entity(self, entity: Any) -> bool: ) if not type_matches: raise TypeError( - f"{self.name} must be set an Entity instance of any of the following types: {self.entity_types}, " - f"but got type '{entity._type}'" + f"{self.name} must be an Entity of type {self.entity_types}, " + f"but got '{entity._type}'" ) - return True def resolve_entity(self, obj: Any, value: Any) -> Optional[E]: - """Resolve a value to an Entity instance. - - Args: - obj: The entity object that owns this relation - value: Can be an Entity instance, string ID, or string name/alias - - Returns: - Entity instance or None - """ + """Resolve a value (Entity, ID string, or alias) to an Entity instance.""" from .entity import Entity if value is None: @@ -344,23 +242,14 @@ def resolve_entity(self, obj: Any, value: Any) -> Optional[E]: return value # type: ignore[return-value] if isinstance(value, (str, int)): - # Try to find entity by ID or name (alias) using each allowed entity type - entity_types = ( - [self.entity_types] - if isinstance(self.entity_types, str) - else self.entity_types - ) - for entity_type_name in entity_types: - # Get the entity class from the database registry + for entity_type_name in self._get_allowed_types(): entity_class = obj.db()._entity_types.get(entity_type_name) - if entity_class: found_entity = entity_class[value] if found_entity: return found_entity - raise ValueError( - f"No entity of types {self.entity_types} found with ID or name '{value}'" + f"No entity of type {self.entity_types} found with ID or name '{value}'" ) raise TypeError( @@ -368,114 +257,94 @@ def resolve_entity(self, obj: Any, value: Any) -> Optional[E]: ) -class RelationList(Generic[E]): - """Helper class for managing lists of related entities.""" - - def __init__(self, obj: Any, prop: "Relation[E]"): - self.obj = obj - self.prop = prop - - def add(self, entity: Union[E, str, int]) -> None: - """Add a new relation.""" - # Resolve entity (supports string ID/name) - resolved = self.prop.resolve_entity(self.obj, entity) - - # Validate entity type using the base validate_entity method - self.prop.validate_entity(resolved) - - # For one-to-many, check if entity already has a relation - if isinstance(self.prop, OneToMany): - existing_relations = resolved.get_relations(self.prop.reverse_name) - if existing_relations: - # Remove existing relation since it's one-to-many - old_relation = existing_relations[0] - # Remove the entity from the old relation's list - if old_relation != self.obj: - old_relation._relations[self.prop.name].remove(resolved) - # Remove the old relation from the entity's list - resolved._relations[self.prop.reverse_name].remove(old_relation) - - self.obj.add_relation(self.prop.name, self.prop.reverse_name, resolved) - - def remove(self, entity: Union[E, str, int]) -> None: - """Remove a relation.""" - self.obj.remove_relation(self.prop.name, self.prop.reverse_name, entity) - - def __iter__(self) -> "Iterator[E]": - return iter(self.obj.get_relations(self.prop.name)) - - def __len__(self) -> int: - return len(self.obj.get_relations(self.prop.name)) - - -class OneToOne(Relation[E]): - """Property for defining one-to-one relationships. +class ManyToOne(Relation[E]): + """Many-to-one relationship (child stores FK to parent). - This property type represents a one-to-one relationship where each entity - can be related to exactly one entity on the other side. + This is the "owning" side. Setting this property: + - Stores the parent's ID on the child entity + - Updates the parent's reverse index so it can find this child Example: - class Person(Entity): - profile = OneToOne('Profile', 'person') + class Employee(Entity): + department = ManyToOne('Department', 'employees') - class Profile(Entity): - person = OneToOne('Person', 'profile') + class Department(Entity): + employees = OneToMany('Employee', 'department') """ def __init__( - self, - entity_types: Union[str, List[str]], - reverse_name: Optional[str] = None, + self, entity_types: Union[str, List[str]], reverse_name: Optional[str] = None ): - super().__init__(entity_types, reverse_name, many=False) + super().__init__(entity_types, reverse_name) + + def __get__( + self, obj: object, objtype: Optional[type] = None + ) -> Union["ManyToOne[E]", Optional[E]]: + if obj is None: + return self # type: ignore[return-value] + parent_ref = obj.__dict__.get(f"_rel_{self.name}") + if parent_ref is None: + return None + for type_name in self._get_allowed_types(): + entity_class = obj.db()._entity_types.get(type_name) + if entity_class: + entity = entity_class.load(parent_ref) + if entity: + return entity # type: ignore[return-value] + return None def __set__(self, obj, value): - """Set the related entity with one-to-one constraints.""" + from .db_engine import Database + from .entity import Entity + + # Fast path: loading from DB — just store the raw reference, indexes are intact + if getattr(obj, "_do_not_save", False) and getattr(obj, "_loaded", False): + if value is None: + obj.__dict__[f"_rel_{self.name}"] = None + elif isinstance(value, Entity): + obj.__dict__[f"_rel_{self.name}"] = value._id + else: + obj.__dict__[f"_rel_{self.name}"] = str(value) + return + if value is not None: - # Check if trying to set multiple values if isinstance(value, (list, tuple)): raise ValueError( - f"{self.name} cannot be set to multiple values (one-to-one relationship)" + f"{self.name} cannot be set to multiple values (many-to-one)" ) - - # Validate entity type value = self.resolve_entity(obj, value) + self.validate_entity(value) - # Check that the reverse property is OneToOne - reverse_prop = value.__class__.__dict__.get(self.reverse_name) - if not isinstance(reverse_prop, OneToOne): - raise ValueError( - f"Reverse property '{self.reverse_name}' must be OneToOne" - ) + db = Database.get_instance() + old_ref = obj.__dict__.get(f"_rel_{self.name}") - # Get current value if any - current = self.__get__(obj) - if current is value: - return - if current is not None: - # Remove existing relation - obj.remove_relation(self.name, self.reverse_name, current) + # Remove from old parent's reverse index + if old_ref is not None and obj._id is not None: + for type_name in self._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class and db.load(type_name, old_ref): + db.reverse_index_remove( + type_name, old_ref, self.reverse_name, obj._id + ) + break - # Check if value is already related to another entity and remove that relation - existing = value.get_relations(self.reverse_name) - if existing: - existing_entity = existing[0] - if existing_entity is obj: - return - # Remove the existing relation from both sides - existing_entity.remove_relation(self.name, self.reverse_name, value) - value.remove_relation(self.reverse_name, self.name, existing_entity) + # Set new reference + if value is not None: + obj.__dict__[f"_rel_{self.name}"] = value._id + if obj._id is not None: + db.reverse_index_add(value._type, value._id, self.reverse_name, obj._id) + else: + obj.__dict__[f"_rel_{self.name}"] = None - # Set the new relation - super().__set__(obj, value) + if not obj._do_not_save: + obj._save() class OneToMany(Relation[E]): - """Property for defining one-to-many relationships. + """One-to-many relationship (parent side, resolved via reverse index). - This property type represents a one-to-many relationship where the 'one' side - owns multiple instances of the 'many' side, but each 'many' instance belongs to - only one owner. + Accessing this property reads the persisted reverse index to find child + IDs, then loads each child. No scanning required. Example: class Department(Entity): @@ -486,102 +355,181 @@ class Employee(Entity): """ def __init__(self, entity_types: Union[str, List[str]], reverse_name: str): - super().__init__(entity_types, reverse_name, many=True) + super().__init__(entity_types, reverse_name) - def __set__(self, obj, values): - """Set related entities with one-to-many constraints.""" - if not isinstance(values, (list, tuple)): - raise TypeError(f"{self.name} must be set to a list of entities") - - resolved_values = [] - for value in values: - # Resolve value to Entity instance - resolved_value = self.resolve_entity(obj, value) - - # Validate entity type using the base validate_entity method - self.validate_entity(resolved_value) - resolved_values.append(resolved_value) - - # Check that the reverse property is ManyToOne - reverse_prop = resolved_value.__class__.__dict__.get(self.reverse_name) - if not isinstance(reverse_prop, ManyToOne): - raise ValueError( - f"Reverse property '{self.reverse_name}' must be ManyToOne" - ) + def __get__( + self, obj: object, objtype: Optional[type] = None + ) -> Union["OneToMany[E]", List[E]]: + if obj is None: + return self # type: ignore[return-value] - # Replace original values with resolved entities - super().__set__(obj, resolved_values) + from .db_engine import Database - @overload - def __get__(self, obj: None, objtype: Optional[type]) -> "OneToMany[E]": ... + db = Database.get_instance() + child_ids = db.reverse_index_get(obj._type, obj._id, self.name) - @overload - def __get__(self, obj: object, objtype: Optional[type]) -> "RelationList[E]": ... + children = [] + for child_id in child_ids: + for type_name in self._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class: + child = entity_class.load(child_id) + if child: + children.append(child) + break + return children # type: ignore[return-value] - def __get__( - self, obj: object, objtype: Optional[type] = None - ) -> Union["OneToMany[E]", "RelationList[E]"]: - """Get related entities as a RelationList.""" - if obj is None: - return self - return RelationList(obj, self) + def __set__(self, obj, values): + raise AttributeError( + f"Cannot set OneToMany '{self.name}' directly. " + f"Set the ManyToOne '{self.reverse_name}' on each child instead." + ) -class ManyToOne(Relation[E]): - """Property for defining many-to-one relationships. +class OneToOne(Relation[E]): + """One-to-one relationship. - This property type represents a many-to-one relationship where multiple entities - can belong to a single owner entity. + The "owning" side stores the FK and updates the reverse index. + The "inverse" side resolves via the reverse index. Example: - class Employee(Entity): - department = ManyToOne('Department', 'employees') + class Person(Entity): + profile = OneToOne('Profile', 'person') - class Department(Entity): - employees = OneToMany('Employee', 'department') + class Profile(Entity): + person = OneToOne('Person', 'profile') """ def __init__( - self, entity_types: Union[str, List[str]], reverse_name: Optional[str] = None + self, + entity_types: Union[str, List[str]], + reverse_name: Optional[str] = None, ): - super().__init__(entity_types, reverse_name, many=False) + super().__init__(entity_types, reverse_name) + + def __get__( + self, obj: object, objtype: Optional[type] = None + ) -> Union["OneToOne[E]", Optional[E]]: + if obj is None: + return self # type: ignore[return-value] + + from .db_engine import Database + + # Check if this side stores the FK (owning side) + local_ref = obj.__dict__.get(f"_rel_{self.name}") + if local_ref is not None: + for type_name in self._get_allowed_types(): + entity_class = obj.db()._entity_types.get(type_name) + if entity_class: + entity = entity_class.load(local_ref) + if entity: + return entity # type: ignore[return-value] + return None + + # Inverse side: check reverse index + db = Database.get_instance() + child_ids = db.reverse_index_get(obj._type, obj._id, self.name) + if child_ids: + for type_name in self._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class: + entity = entity_class.load(child_ids[0]) + if entity: + return entity # type: ignore[return-value] + return None def __set__(self, obj, value): - """Set the related entity with many-to-one constraints.""" + from .db_engine import Database + from .entity import Entity + + # Fast path: loading from DB — just store the raw reference, indexes are intact + if getattr(obj, "_do_not_save", False) and getattr(obj, "_loaded", False): + if value is None: + obj.__dict__[f"_rel_{self.name}"] = None + elif isinstance(value, Entity): + obj.__dict__[f"_rel_{self.name}"] = value._id + else: + obj.__dict__[f"_rel_{self.name}"] = str(value) + return + if value is not None: - # Check if trying to set multiple values if isinstance(value, (list, tuple)): raise ValueError( - f"{self.name} cannot be set to multiple values (many-to-one relationship)" + f"{self.name} cannot be set to multiple values (one-to-one)" ) - - # Resolve value to Entity instance value = self.resolve_entity(obj, value) - - # Validate entity type using the base validate_entity method self.validate_entity(value) - # Check that the reverse property is OneToMany - # Use getattr to walk MRO so inherited relations are found - reverse_prop = getattr(value.__class__, self.reverse_name, None) - if not reverse_prop: - raise ValueError( - f"Reverse property '{self.reverse_name}' not found in {value.__class__.__name__} entity" - ) + db = Database.get_instance() + old_ref = obj.__dict__.get(f"_rel_{self.name}") - if not isinstance(reverse_prop, OneToMany): - raise ValueError( - f"Reverse property '{self.reverse_name}' must be OneToMany and it is '{reverse_prop.__class__.__name__}'" + # Remove from old target's reverse index + if old_ref is not None and obj._id is not None: + for type_name in self._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class and db.load(type_name, old_ref): + db.reverse_index_remove( + type_name, old_ref, self.reverse_name, obj._id + ) + break + + # Enforce OneToOne exclusivity: if the target entity already has a direct + # FK on the reverse relation pointing to a different entity, clear it. + if value is not None: + existing_on_target = value.__dict__.get(f"_rel_{self.reverse_name}") + if existing_on_target is not None and existing_on_target != obj._id: + # The target already links to someone else via reverse relation. + # Clear that link and its reverse index entry. + db.reverse_index_remove( + obj._type, existing_on_target, self.name, value._id ) + value.__dict__[f"_rel_{self.reverse_name}"] = None + value._save() + + # Also check if obj was previously linked via reverse index (inverse side) + # e.g., if someone else set "other.rel = obj" previously + existing_in_ri = db.reverse_index_get(obj._type, obj._id, self.name) + for old_other_id in existing_in_ri: + if old_other_id != value._id: + db.reverse_index_remove(obj._type, obj._id, self.name, old_other_id) + # Clear the other entity's direct FK + for tn in self._get_allowed_types(): + ec = db._entity_types.get(tn) + if ec: + other_entity = ec.load(old_other_id) + if ( + other_entity + and other_entity.__dict__.get( + f"_rel_{self.reverse_name}" + ) + == obj._id + ): + other_entity.__dict__[f"_rel_{self.reverse_name}"] = ( + None + ) + other_entity._save() + break + + # Set new reference + if value is not None: + obj.__dict__[f"_rel_{self.name}"] = value._id + if obj._id is not None: + db.reverse_index_add(value._type, value._id, self.reverse_name, obj._id) + else: + obj.__dict__[f"_rel_{self.name}"] = None + # Also clear any reverse index entries pointing at obj for this relation + existing_in_ri = db.reverse_index_get(obj._type, obj._id, self.name) + for old_other_id in existing_in_ri: + db.reverse_index_remove(obj._type, obj._id, self.name, old_other_id) - super().__set__(obj, value) + if not obj._do_not_save: + obj._save() class ManyToMany(Relation[E]): - """Property for defining many-to-many relationships. + """Many-to-many relationship with bidirectional reverse indexes. - This property type represents a many-to-many relationship where entities - on both sides can be related to multiple entities on the other side. + Both sides maintain a reverse index. Setting on either side updates both. Example: class Student(Entity): @@ -592,53 +540,67 @@ class Course(Entity): """ def __init__(self, entity_types: Union[str, List[str]], reverse_name: str): - super().__init__(entity_types, reverse_name, many=True) + super().__init__(entity_types, reverse_name) + + def __get__( + self, obj: object, objtype: Optional[type] = None + ) -> Union["ManyToMany[E]", List[E]]: + if obj is None: + return self # type: ignore[return-value] + + from .db_engine import Database + + db = Database.get_instance() + related_ids = db.reverse_index_get(obj._type, obj._id, self.name) + + entities = [] + for related_id in related_ids: + for type_name in self._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class: + entity = entity_class.load(related_id) + if entity: + entities.append(entity) + break + return entities # type: ignore[return-value] def __set__(self, obj, values): - """Set related entities with many-to-many constraints.""" + from .db_engine import Database from .entity import Entity - # Convert single entity to list for convenience + if values is None: + values = [] if isinstance(values, Entity): values = [values] elif isinstance(values, (str, int)): - # Handle single string ID/name values = [values] - elif values is not None and not isinstance(values, (list, tuple)): + elif not isinstance(values, (list, tuple)): raise TypeError(f"{self.name} must be set to an entity or list of entities") - if values is not None: - resolved_values = [] - for value in values: - # Resolve value to Entity instance - resolved_value = self.resolve_entity(obj, value) - - # Validate entity type using the base validate_entity method - self.validate_entity(resolved_value) - resolved_values.append(resolved_value) - - # Check that the reverse property is ManyToMany - reverse_prop = resolved_value.__class__.__dict__.get(self.reverse_name) - if not isinstance(reverse_prop, ManyToMany): - raise ValueError( - f"Reverse property '{self.reverse_name}' must be ManyToMany" + resolved = [] + for v in values: + entity = self.resolve_entity(obj, v) + self.validate_entity(entity) + resolved.append(entity) + + db = Database.get_instance() + + # Remove old relations from both sides' indexes + old_ids = db.reverse_index_get(obj._type, obj._id, self.name) + for old_id in old_ids: + db.reverse_index_remove(obj._type, obj._id, self.name, old_id) + for type_name in self._get_allowed_types(): + entity_class = db._entity_types.get(type_name) + if entity_class and db.load(type_name, old_id): + db.reverse_index_remove( + type_name, old_id, self.reverse_name, obj._id ) + break - # Replace original values with resolved entities - values = resolved_values - - super().__set__(obj, values) - - @overload - def __get__(self, obj: None, objtype: Optional[type]) -> "ManyToMany[E]": ... - - @overload - def __get__(self, obj: object, objtype: Optional[type]) -> "RelationList[E]": ... + # Add new relations to both sides' indexes + for entity in resolved: + db.reverse_index_add(obj._type, obj._id, self.name, entity._id) + db.reverse_index_add(entity._type, entity._id, self.reverse_name, obj._id) - def __get__( - self, obj: object, objtype: Optional[type] = None - ) -> Union["ManyToMany[E]", "RelationList[E]"]: - """Get related entities as a RelationList.""" - if obj is None: - return self - return RelationList(obj, self) + if not obj._do_not_save: + obj._save() diff --git a/tests/src/tests/test_enhanced_relations.py b/tests/src/tests/test_enhanced_relations.py index 78c3af8..a063166 100644 --- a/tests/src/tests/test_enhanced_relations.py +++ b/tests/src/tests/test_enhanced_relations.py @@ -117,7 +117,7 @@ def test_many_to_one_entity_resolution(self): assert len(dept.employees) == 3, "dept.employees should now have 3 members" def test_one_to_many_entity_resolution(self): - """Test OneToMany relations with mixed entity, ID, and name resolution.""" + """Test OneToMany relations using ManyToOne side with entity resolution.""" user1 = User(username="alice", email="alice@example.com") user2 = User(username="bob", email="bob@example.com") user3 = User(username="charlie", email="charlie@example.com") @@ -131,8 +131,11 @@ def test_one_to_many_entity_resolution(self): assert user3.department is None, "user3.department should initially be None" assert user4.department is None, "user4.department should initially be None" - # Set using mix of entity instances and string IDs/names - dept.employees = [user3, user4._id, "alice"] # Mix of entity, ID, and name + # Set using ManyToOne side with various resolution methods + user3.department = dept # entity instance + user4.department = dept._id # string ID + user1.department = "Engineering" # name/alias + expected_usernames = {"charlie", "diana", "alice"} actual_usernames = {emp.username for emp in dept.employees} assert ( @@ -199,8 +202,8 @@ def test_many_to_many_single_entity_assignment(self): len(user1.projects) == 1 ), "user1.projects should still have exactly 1 project" - def test_relation_list_add_with_entity_resolution(self): - """Test RelationList.add with entity, ID, and name resolution.""" + def test_incremental_many_to_many_assignment(self): + """Test ManyToMany incremental addition with entity resolution.""" user1 = User(username="alice", email="alice@example.com") user2 = User(username="bob", email="bob@example.com") project1 = Project(name="WebApp", description="Main web application") @@ -208,8 +211,8 @@ def test_relation_list_add_with_entity_resolution(self): # Verify initial state assert len(project1.members) == 0, "project1.members should initially be empty" - # Add using string ID - project1.members.add(user1._id) + # Set using string ID + project1.members = [user1._id] assert ( user1 in project1.members ), f"user1 should be in project1.members after adding by ID '{user1._id}'" @@ -218,8 +221,8 @@ def test_relation_list_add_with_entity_resolution(self): ), "project1 should be in user1.projects (reverse relation)" assert len(project1.members) == 1, "project1.members should have 1 member" - # Add using alias/name - project1.members.add("bob") + # Add another using alias/name (must re-set full list) + project1.members = [user1, "bob"] assert ( user2 in project1.members ), "user2 should be in project1.members after adding by alias 'bob'" @@ -242,9 +245,9 @@ def test_invalid_entity_resolution(self): user1.projects = "nonexistent_id" assert False, "Should have raised ValueError for nonexistent ID" except ValueError as e: - assert "No entity of types" in str( + assert "No entity of type" in str( e - ), f"Expected 'No entity of types' in error message, got: {str(e)}" + ), f"Expected 'No entity of type' in error message, got: {str(e)}" # Test invalid type try: diff --git a/tests/src/tests/test_entity.py b/tests/src/tests/test_entity.py index e64fb5a..1efacaa 100644 --- a/tests/src/tests/test_entity.py +++ b/tests/src/tests/test_entity.py @@ -8,19 +8,15 @@ class Person(Entity): """Test entity class.""" - # Example on how to override __init__ - # def __init__(self, name: str, age: int = 0, **kwargs): - # super().__init__(**kwargs) - # self.name = name - # self.age = age - __alias__ = "name" name = String(min_length=2, max_length=50) age = Integer() + department = ManyToOne("Department", "employees") class Department(Entity): name = String(min_length=2, max_length=50) + employees = OneToMany("Person", "department") class TestEntity: @@ -45,19 +41,19 @@ def test_entity_update(self): assert loaded.age == 31 def test_entity_relations(self): - """Test entity relations.""" - person = Person(name="John") + """Test entity relations using ManyToOne/OneToMany descriptors.""" dept = Department(name="IT") + person = Person(name="John") - # Test adding relation - person.add_relation("works_in", "has_employee", dept) + # Set relation via ManyToOne + person.department = dept - # Check person's relations + # Verify relationship from both sides loaded_person = Person[person._id] loaded_dept = Department[dept._id] - assert loaded_person.get_relations("works_in", "Department")[0] == dept - assert loaded_dept.get_relations("has_employee", "Person")[0] == person + assert loaded_person.department == dept + assert loaded_person in loaded_dept.employees def test_entity_duplicate_key(self): """Test that saving an entity with a duplicate ID raises an error.""" @@ -86,16 +82,16 @@ def test_getitem_by_id(self): assert Person["non_existent"] is None def test_entity_duplicate_relation(self): - """Test handling of duplicate relations.""" - person = Person(name="John") + """Test that setting same relation twice doesn't duplicate.""" dept = Department(name="IT") + person = Person(name="John") - # Adding same relation twice should only create one - person.add_relation("works_in", "has_employee", dept) - person.add_relation("works_in", "has_employee", dept) + # Setting same ManyToOne relation twice + person.department = dept + person.department = dept - loaded = Person[person._id] - assert len(loaded.get_relations("works_in", "Department")) == 1 + # Should still only have one employee + assert len(dept.employees) == 1 def test_entity_alias(self): """Test entity lookup by alias (__alias__ functionality).""" diff --git a/tests/src/tests/test_migrations.py b/tests/src/tests/test_migrations.py index 7212631..7eb4e2e 100644 --- a/tests/src/tests/test_migrations.py +++ b/tests/src/tests/test_migrations.py @@ -476,7 +476,6 @@ class User(Entity): profile = Profile(bio="Developer") user = User(name="Alice", profile=profile) user_id = user._id - profile_id = profile._id Database.get_instance().clear_registry() @@ -511,14 +510,12 @@ def migrate(cls, obj, from_version, to_version): assert loaded_user is not None assert loaded_user.name == "Alice" - # Verify migration renamed the relation field in stored data + # Verify migration renamed the relation field in stored data (owning side only) db = Database.get_instance() user_data = db.load("User", user_id) - profile_data = db.load("Profile", profile_id) assert "user_profile" in user_data - assert "user" in profile_data - # Note: Bidirectional relation consistency after field rename - # may require manual data fix or more complex migration logic + # Note: In the new architecture, only the owning side stores the FK. + # The inverse side (Profile) reads from the reverse index. def test_migration_persistence_verification(self): """Test that migration changes are persisted to database.""" diff --git a/tests/src/tests/test_namespaces.py b/tests/src/tests/test_namespaces.py index e0947cb..09367f6 100644 --- a/tests/src/tests/test_namespaces.py +++ b/tests/src/tests/test_namespaces.py @@ -172,9 +172,11 @@ class BlogUser(Entity): assert post1.author == user assert post1 in user.posts - # Test OneToMany assignment - user.posts = [post1, post2] - assert list(user.posts) == [post1, post2] + # Test setting via ManyToOne side + post2.author = user + assert post1 in user.posts + assert post2 in user.posts + assert len(user.posts) == 2 assert post1.author == user assert post2.author == user diff --git a/tests/src/tests/test_relationships.py b/tests/src/tests/test_relationships.py index 758cc6f..21bb498 100644 --- a/tests/src/tests/test_relationships.py +++ b/tests/src/tests/test_relationships.py @@ -105,8 +105,9 @@ def test_one_to_many(self): emp2 = Employee(name="Bob") emp3 = Employee(name="Charlie") - # Add employees to department - dept.employees = [emp1, emp2] + # Add employees to department via the ManyToOne side + emp1.department = dept + emp2.department = dept # Verify relationships assert len(dept.employees) == 2 @@ -120,14 +121,14 @@ def test_one_to_many(self): ) # Add another employee - dept.employees = [emp1, emp2, emp3] + emp3.department = dept # Verify relationships assert len(dept.employees) == 3 assert emp3.department == dept # Move employee to new department - dept2.employees = [emp1] + emp1.department = dept2 # Verify relationships assert len(dept.employees) == 2 @@ -139,6 +140,9 @@ def test_one_to_many(self): ValueError, lambda: setattr(emp1, "department", [dept, dept2]) ) + # Verify OneToMany cannot be set directly + Tester.assert_raises(AttributeError, lambda: setattr(dept, "employees", [emp1])) + def test_many_to_many(self): """Test many-to-many relationships.""" # Create students @@ -255,7 +259,8 @@ def test_one_to_many_survives_load(self): dept = Department(name="Engineering") emp1 = Employee(name="Alice") emp2 = Employee(name="Bob") - dept.employees = [emp1, emp2] + emp1.department = dept + emp2.department = dept # Sanity: relations work in memory assert len(dept.employees) == 2 @@ -305,6 +310,134 @@ def test_one_to_one_survives_load(self): assert loaded_person.profile.bio == "Engineer" +class TestReverseIndexResolution: + """Core regression test for GitHub issue #9. + + The fundamental problem: in the old architecture, OneToMany/ManyToMany + resolution depended on loading ALL child entities (via instances()) to + populate an in-memory _relations dict. This meant: + + 1. After a canister upgrade (or GC clearing weak refs), calling + parent.children returned [] unless you first called + list(Child.instances()) to repopulate the in-memory dict. + + 2. As entity count grew, this O(max_id) scan exceeded IC instruction + limits, making relationship access impossible at scale. + + The fix: persisted reverse indexes in stable storage. Relationships + are now resolved in O(k) by reading a small index entry, with no + dependence on loading unrelated entities. + """ + + def setUp(self): + Database.get_instance().clear() + + def test_one_to_many_without_loading_all_children(self): + """OneToMany resolves from reverse index — no need to load all children. + + This is the exact scenario that broke the demo simulator: + parent.children must work by loading ONLY the parent, without + scanning all Child entities first. + """ + dept = Department(name="Engineering") + emp1 = Employee(name="Alice") + emp2 = Employee(name="Bob") + emp1.department = dept + emp2.department = dept + + # Also create unrelated entities to prove we don't scan them + other_dept = Department(name="Sales") + emp3 = Employee(name="Carol") + emp3.department = other_dept + + # Simulate canister upgrade: clear ALL in-memory state + Database.get_instance()._entity_registry.clear() + Entity._context.clear() + + # Load ONLY the parent — do NOT load any children first + loaded_dept = Department.load(str(dept._id)) + + # OneToMany must resolve without having loaded children beforehand + employees = loaded_dept.employees + assert len(employees) == 2, ( + f"Expected 2 employees from reverse index, got {len(employees)}. " + "This fails in the old architecture unless instances() is called first." + ) + names = {e.name for e in employees} + assert names == {"Alice", "Bob"} + + def test_many_to_many_without_loading_all_related(self): + """ManyToMany resolves from reverse index without scanning.""" + s1 = Student(name="Alice") + s2 = Student(name="Bob") + c1 = Course(name="Math") + c2 = Course(name="Physics") + + s1.courses = [c1, c2] + s2.courses = [c1] + + # Simulate canister upgrade + Database.get_instance()._entity_registry.clear() + Entity._context.clear() + + # Load ONLY course1 — do NOT load students first + loaded_course = Course.load(str(c1._id)) + + # ManyToMany must resolve from reverse index + students = loaded_course.students + assert ( + len(students) == 2 + ), f"Expected 2 students from reverse index, got {len(students)}." + names = {s.name for s in students} + assert names == {"Alice", "Bob"} + + def test_one_to_one_inverse_without_loading_target(self): + """OneToOne inverse side resolves from reverse index.""" + person = Person(name="Eve") + profile = Profile(bio="Researcher") + person.profile = profile # person owns FK + + # Simulate canister upgrade + Database.get_instance()._entity_registry.clear() + Entity._context.clear() + + # Load ONLY the profile (inverse side) — do NOT load person first + loaded_profile = Profile.load(str(profile._id)) + + # Inverse OneToOne must resolve from reverse index + assert loaded_profile.person is not None, ( + "Inverse OneToOne must resolve from reverse index without " + "loading the owning entity first." + ) + assert loaded_profile.person.name == "Eve" + + def test_scalability_no_full_scan(self): + """Relationship access cost is O(k) not O(max_id). + + Create many unrelated entities and verify that accessing a + relationship on one parent doesn't require touching them. + """ + dept = Department(name="Target") + emp = Employee(name="OnlyEmployee") + emp.department = dept + + # Create 50 unrelated employees in a different department + other = Department(name="Other") + for i in range(50): + e = Employee(name=f"Other_{i}") + e.department = other + + # Simulate canister upgrade + Database.get_instance()._entity_registry.clear() + Entity._context.clear() + + # Access should only load the 1 child, not all 51 employees + loaded_dept = Department.load(str(dept._id)) + employees = loaded_dept.employees + assert len(employees) == 1 + assert employees[0].name == "OnlyEmployee" + + def run(test_name: str = None, test_var: str = None): tester = Tester(TestRelationships) results = tester.run_tests() @@ -312,7 +445,9 @@ def run(test_name: str = None, test_var: str = None): results2 = tester2.run_tests() tester3 = Tester(TestLoadPreservesRelations) results3 = tester3.run_tests() - return results or results2 or results3 + tester4 = Tester(TestReverseIndexResolution) + results4 = tester4.run_tests() + return results or results2 or results3 or results4 if __name__ == "__main__": diff --git a/tests/src/tests/test_serialization.py b/tests/src/tests/test_serialization.py index da1470e..ae6872c 100644 --- a/tests/src/tests/test_serialization.py +++ b/tests/src/tests/test_serialization.py @@ -34,9 +34,9 @@ def setUp(self): def test_serialization_format(self): """Test that relations are serialized in the correct format. - OneToMany is always skipped (reconstructed from reverse ManyToOne). - OneToOne is serialized only on the alphabetically-earlier entity type side. - "Child" < "Parent" → Child serializes favorite_parent; Parent skips favorite_child. + OneToMany is never serialized (reconstructed from ManyToOne during import). + ManyToOne and owning-side OneToOne are serialized as single FK references. + ManyToMany is serialized as a list. """ # Create entities @@ -44,10 +44,10 @@ def test_serialization_format(self): child1 = Child(name="Bob") child2 = Child(name="Charlie") - # Set up relations - parent.children = [child1] # OneToMany with single item - parent.favorite_child = child1 # OneToOne - child1.siblings = [child2] # ManyToMany with single item + # Set up relations via the owning side + child1.parent = parent # ManyToOne + parent.favorite_child = child1 # OneToOne (parent owns) + child1.siblings = [child2] # ManyToMany # Test serialization parent_data = parent.serialize() @@ -56,16 +56,11 @@ def test_serialization_format(self): # OneToMany is always skipped in serialize assert "children" not in parent_data, "OneToMany should be skipped" - # OneToOne: "Parent" > "Child" → Parent skips favorite_child + # OneToOne: Parent owns favorite_child → it IS in parent_data assert ( - "favorite_child" not in parent_data - ), "OneToOne on alphabetically-later side should be skipped" - - # OneToOne: "Child" < "Parent" → Child serializes favorite_parent - assert isinstance( - child1_data["favorite_parent"], str - ), "OneToOne on alphabetically-earlier side should serialize as single value" - assert child1_data["favorite_parent"] == parent._id + "favorite_child" in parent_data + ), "Owning-side OneToOne should be serialized" + assert parent_data["favorite_child"] == child1._id # ManyToOne should be a single value assert isinstance( @@ -75,6 +70,11 @@ def test_serialization_format(self): child1_data["parent"] == parent._id ), f"Expected '{parent._id}', got {child1_data['parent']}" + # Child1 does NOT own favorite_parent (it's the inverse side) + assert ( + "favorite_parent" not in child1_data + ), "Inverse OneToOne should not be serialized" + # ManyToMany should always be a list, even with single item assert isinstance( child1_data["siblings"], list @@ -96,12 +96,6 @@ def test_serialization_format(self): len(child1_data["siblings"]) == 2 ), "ManyToMany should contain both siblings" - assert str(parent_data) == "{'_type': 'Parent', '_id': '1', 'name': 'Alice'}" - assert ( - str(child1_data) - == "{'_type': 'Child', '_id': '1', 'name': 'Bob', 'parent': '1', 'favorite_parent': '1', 'siblings': ['2', '3']}" - ) - def test_deserialization(self): """Test that entities can be reconstructed from serialized data.""" Database.get_instance().clear() @@ -111,8 +105,8 @@ def test_deserialization(self): child1 = Child(name="Bob") child2 = Child(name="Charlie") - # Set up relations - parent.children = [child1] + # Set up relations via owning side + child1.parent = parent parent.favorite_child = child1 child1.siblings = [child2] @@ -120,20 +114,13 @@ def test_deserialization(self): parent_data = parent.serialize() child1_data = child1.serialize() - print("parent_data", parent_data) - print("child1_data", child1_data) - # Clear database to test deserialization Database.get_instance().clear() # Recreate entities from serialized data - # Note: We need to create all entities first before setting relations recreated_parent = Parent.deserialize(parent_data) recreated_child1 = Child.deserialize(child1_data) - print("recreated_parent", recreated_parent) - print("recreated_child1", recreated_child1) - # Verify basic properties assert recreated_parent.name == "Alice" assert recreated_parent._id == "1" @@ -163,9 +150,8 @@ def test_deserialization(self): def test_round_trip_serialization(self): """Test that serialize -> deserialize produces equivalent entities. - Import order: Parent first (no forward refs), then Children (ManyToOne - + OneToOne refs to Parent). OneToMany on Parent is reconstructed from - ManyToOne on Children. + Two-pass import: create all entities first (relations may fail to resolve), + then re-deserialize to link remaining relations. """ Database.get_instance().clear() @@ -175,8 +161,9 @@ def test_round_trip_serialization(self): child2 = Child(name="Charlie") child3 = Child(name="David") - # Set up complex relations - parent.children = [child1, child2] + # Set up complex relations via owning side + child1.parent = parent + child2.parent = parent parent.favorite_child = child1 child1.siblings = [child2, child3] child2.siblings = [child1, child3] @@ -187,52 +174,30 @@ def test_round_trip_serialization(self): child2_data = child2.serialize() child3_data = child3.serialize() - # Parent should have no forward refs (OneToMany + OneToOne skipped) + # Parent has OneToMany (skipped) + owning OneToOne (included) assert "children" not in parent_data - assert "favorite_child" not in parent_data + assert "favorite_child" in parent_data - # Children carry ManyToOne + OneToOne refs + # Children carry ManyToOne refs assert "parent" in child1_data - assert "favorite_parent" in child1_data # Clear and recreate from serialized data Database.get_instance().clear() - # Import order: Parent first, then Children + # Pass 1: Import parent first, then children (ManyToOne resolves) recreated_parent = Parent.deserialize(parent_data) Child.deserialize(child3_data) Child.deserialize(child2_data) recreated_child1 = Child.deserialize(child1_data) - # Verify the recreated entities have the same serialized output - recreated_parent_data = recreated_parent.serialize() - recreated_child1_data = recreated_child1.serialize() - - print(f"Original parent: {parent_data}") - print(f"Recreated parent: {recreated_parent_data}") - print(f"Original child1: {child1_data}") - print(f"Recreated child1: {recreated_child1_data}") - - assert ( - recreated_parent_data == parent_data - ), f"Parent data mismatch:\nOriginal: {parent_data}\nRecreated: {recreated_parent_data}" - - # For child1, check each field individually to handle list ordering - for key, value in child1_data.items(): - if key == "siblings": # ManyToMany relation - check set equality - assert set(recreated_child1_data[key]) == set( - value - ), f"Siblings mismatch: {recreated_child1_data[key]} != {value}" - else: - assert ( - recreated_child1_data[key] == value - ), f"Field {key} mismatch: {recreated_child1_data[key]} != {value}" + # Pass 2: Re-deserialize parent to resolve favorite_child now that children exist + recreated_parent = Parent.deserialize(parent_data) # Verify basic properties are preserved assert recreated_parent.name == "Alice" assert recreated_child1.name == "Bob" - # Verify relations are properly restored via reverse links + # Verify relations are properly restored assert len(recreated_parent.children) == 2, "Parent should have 2 children" assert ( recreated_parent.favorite_child is not None @@ -246,7 +211,7 @@ def test_generic_deserialization(self): # Create entities parent = Parent(name="Alice") child = Child(name="Bob") - parent.children = [child] + child.parent = parent # Serialize entities parent_data = parent.serialize() @@ -689,7 +654,6 @@ def test_serialize_skips_one_to_many(self): child1 = Child(name="Bob") child2 = Child(name="Charlie") - parent.children = [child1, child2] child1.parent = parent child2.parent = parent @@ -703,28 +667,29 @@ def test_serialize_skips_one_to_many(self): assert child_data["parent"] == parent._id def test_serialize_one_to_one_deterministic(self): - """Test that serialize() emits OneToOne on only one deterministic side. + """Test that serialize() emits OneToOne only on the owning side. - Rule: serialize only if self._type <= target._type (alphabetically). + The side that calls __set__ owns the FK and serializes it. """ Database.get_instance().clear() parent = Parent(name="Alice") child1 = Child(name="Bob") + # Parent owns favorite_child (it calls __set__) parent.favorite_child = child1 - # "Child" < "Parent" → Child serializes favorite_parent - child_data = child1.serialize() + # Parent serializes its own FK: favorite_child + parent_data = parent.serialize() assert ( - "favorite_parent" in child_data - ), "Child (alphabetically earlier) should serialize OneToOne to Parent" + "favorite_child" in parent_data + ), "Owning side should serialize OneToOne FK" - # "Parent" > "Child" → Parent does NOT serialize favorite_child - parent_data = parent.serialize() + # Child does NOT own favorite_parent (inverse side, read from reverse index) + child_data = child1.serialize() assert ( - "favorite_child" not in parent_data - ), "Parent (alphabetically later) should skip OneToOne to Child" + "favorite_parent" not in child_data + ), "Inverse side should not serialize OneToOne" def test_serialize_keeps_many_to_many(self): """Test that serialize keeps ManyToMany (self-referential).""" @@ -741,21 +706,15 @@ def test_serialize_keeps_many_to_many(self): # ── Issue #4 regression tests ────────────────────────────────────────── def test_issue4_bidirectional_one_to_one_import_order(self): - """Regression test for issue #4: bidirectional OneToOne import crash. - - Reproduces the exact scenario from the issue: User↔Member with - OneToOne on both sides. Without the fix, whichever entity type is - imported first will reference the other which doesn't exist yet, - crashing with: - ValueError: No entity of types Member found with ID or name 'mem_xxx' + """Regression test for issue #4: bidirectional OneToOne import. - The fix: serialize() skips the OneToOne on the alphabetically-later - side ("User" > "Member4" → User.member is skipped). - Import order: User first (no member ref), Member second (user ref resolves). + The owning side (the one that called __set__) serializes the FK. + The inverse side reads from the reverse index after import. + Import order: create the target first, then the entity with the FK. """ Database.get_instance().clear() - # Define User↔Member with bidirectional OneToOne (mirrors real realm entities) + # Define User↔Member with bidirectional OneToOne class User(Entity): __alias__ = "name" name = String() @@ -766,7 +725,7 @@ class Member4(Entity): id = String() user = OneToOne("User", "member") - # Create linked entities + # Create linked entities - Member4 owns "user" FK user = User(name="system") member = Member4(id="mem_9f03f2ee") member.user = user @@ -779,28 +738,26 @@ class Member4(Entity): user_data = user.serialize() member_data = member.serialize() - # User should NOT have 'member' (since "User" > "Member4") + # User does NOT have 'member' (inverse side, read from reverse index) assert ( "member" not in user_data - ), f"Should skip OneToOne on alphabetically-later side; got {user_data}" - # Member4 SHOULD have 'user' (since "Member4" < "User") + ), f"Inverse side should not serialize OneToOne; got {user_data}" + # Member4 SHOULD have 'user' (owning side) assert ( "user" in member_data - ), f"Should keep OneToOne on alphabetically-earlier side; got {member_data}" + ), f"Owning side should serialize OneToOne; got {member_data}" assert member_data["user"] == "system" # alias - # Clear and reimport in dependency order + # Clear and reimport: User first (target), then Member (has FK) Database.get_instance().clear() - recreated_user = User.deserialize(user_data) # No member ref → no crash - recreated_member = Member4.deserialize( - member_data - ) # user ref → resolves to existing User + recreated_user = User.deserialize(user_data) + recreated_member = Member4.deserialize(member_data) # Both sides should be reconstructed assert recreated_member.user == recreated_user, "OneToOne should resolve" assert ( recreated_user.member == recreated_member - ), "Reverse OneToOne should be set" + ), "Reverse OneToOne should be set via reverse index" def test_issue4_load_some_resilience(self): """Regression test for issue #4: load_some resilience with dangling refs.