From 664c3e06140cf09f3949a77b4636dd55884f1805 Mon Sep 17 00:00:00 2001 From: Alexandre Gomes Gaigalas Date: Fri, 19 Dec 2025 11:07:24 -0300 Subject: [PATCH] Simplify circular dependency detection - Used topological sort for all entries, not just constants - Removed checks during instantiation, relying on instantiation-time check only. - Updated tests and documentation. --- AGENTS.md | 2 +- Makefile | 2 +- apywire/exceptions.py | 57 ++++++ apywire/generator.py | 109 ++--------- apywire/runtime.py | 108 ++++------- apywire/threads.py | 8 +- apywire/wiring.py | 237 +++++++++++++++-------- docs/api-reference.md | 5 +- docs/development.md | 2 +- docs/getting-started.md | 17 -- docs/user-guide/advanced.md | 25 +-- docs/user-guide/async-support.md | 2 +- docs/user-guide/basic-usage.md | 17 +- docs/user-guide/circular-dependencies.md | 116 +++++++++++ docs/user-guide/generator.md | 3 - docs/user-guide/index.md | 2 +- docs/user-guide/thread-safety.md | 13 +- mkdocs.yml | 1 + tests/test_constant_placeholders.py | 20 +- tests/test_edge_cases.py | 16 -- tests/test_exceptions.py | 59 ++++++ tests/test_generator.py | 33 ++-- tests/test_internals.py | 38 ---- tests/test_single.py | 9 +- tests/test_threading.py | 18 +- 25 files changed, 500 insertions(+), 419 deletions(-) create mode 100644 docs/user-guide/circular-dependencies.md create mode 100644 tests/test_exceptions.py diff --git a/AGENTS.md b/AGENTS.md index e3df46e..8acddd4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,7 +28,7 @@ Objects are instantiated only when accessed, using `__getattr__` to intercept at Strings like `{name}` in spec values are converted to `_WiredRef` markers during parsing, then resolved to actual objects at instantiation time. See `constants.py` for delimiter patterns. **Thread Safety Model:** -Uses optimistic per-attribute locking (`dict[str, threading.Lock]`) with a global fallback lock. Thread-local state tracks the resolving stack for circular dependency detection. See `threads.py` for `ThreadSafeMixin` implementation. +Uses optimistic per-attribute locking (`dict[str, threading.Lock]`) with a global fallback lock. Thread-local state stores lock-related per-thread state (mode and held locks) used by the `ThreadSafeMixin`; see `threads.py` for implementation details. **Compilation Equivalence:** Generated code must behave identically to runtime `Wiring`. The compiler generates accessor methods that replicate lazy loading, caching, and optional async/thread-safe behavior. diff --git a/Makefile b/Makefile index 795cc4e..2d0c20e 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ reuse: --license ISC \ --recursive . -format: reuse +format: uv run black apywire tests uv run isort apywire tests diff --git a/apywire/exceptions.py b/apywire/exceptions.py index ccc08c7..4f88b04 100644 --- a/apywire/exceptions.py +++ b/apywire/exceptions.py @@ -31,8 +31,65 @@ class CircularWiringError(WiringError): This class is a `WiringError` subtype for simpler programmatic handling of wiring-specific failures. + + Utility: construct a helpful message from a dependency mapping and + unprocessed nodes when a topological sort fails. """ + @classmethod + def from_unprocessed( + cls, dependencies: dict[str, set[str]], unprocessed: list[str] + ) -> "CircularWiringError": + """Create a CircularWiringError that includes a cycle when possible. + + Args: + dependencies: Full mapping of node -> dependencies + unprocessed: List of nodes left unprocessed by topological sort + + Returns: + CircularWiringError instance with a helpful message. + """ + all_nodes = set(dependencies.keys()) + + # Use DFS limited to unprocessed nodes to find any back-edge cycle path + visited: set[str] = set() + stack: list[str] = [] + on_stack: set[str] = set() + + def dfs(node: str) -> list[str] | None: + visited.add(node) + stack.append(node) + on_stack.add(node) + for nbr in dependencies.get(node, set()): + if nbr not in all_nodes: + continue + if nbr not in visited: + res = dfs(nbr) + if res: + return res + elif nbr in on_stack: + try: + idx = stack.index(nbr) + except ValueError: + idx = 0 + return stack[idx:] + [nbr] + stack.pop() + on_stack.remove(node) + return None + + for start in unprocessed: + if start not in visited: + cyc = dfs(start) + if cyc: + return cls( + f"Circular dependency detected: { + ', '.join(unprocessed) + }; " + f"cycle: {' -> '.join(cyc)}" + ) + + return cls(f"Circular dependency detected: {', '.join(unprocessed)}") + class LockUnavailableError(RuntimeError): """Exception raised when a per-attribute lock cannot be acquired in diff --git a/apywire/generator.py b/apywire/generator.py index d4385b2..372168f 100644 --- a/apywire/generator.py +++ b/apywire/generator.py @@ -23,11 +23,9 @@ from apywire.constants import ( PLACEHOLDER_END, PLACEHOLDER_START, - SPEC_KEY_DELIMITER, ) -from apywire.exceptions import CircularWiringError from apywire.runtime import _Constructor -from apywire.wiring import Spec, _ConstantValue, _SpecMapping +from apywire.wiring import Spec, SpecParser, _ConstantValue, _SpecMapping # Parameter sentinel values _PARAM_EMPTY: object = inspect.Parameter.empty # type: ignore[misc] @@ -37,7 +35,7 @@ _VAR_KEYWORD: object = inspect.Parameter.VAR_KEYWORD # type: ignore[misc] -class Generator: +class Generator(SpecParser): """Generates apywire specs from class introspection. This class inspects class constructors to automatically generate @@ -73,7 +71,6 @@ def generate(cls, *entries: str) -> Spec: Raises: ValueError: If entry format is invalid - CircularWiringError: If circular dependencies detected Example: >>> spec = Generator.generate("datetime.datetime now") @@ -96,9 +93,13 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None: spec: Spec dict to populate visited: Set of already-visited class keys to detect cycles """ - module_name, class_name, instance_name, factory_method = ( - cls._parse_entry(entry) - ) + parsed = cls._parse_request_string(entry) + if parsed is None: + raise ValueError( + f"Invalid entry format '{entry}': " + f"expected 'module.Class name'" + ) + module_name, class_name, instance_name, factory_method = parsed # Build the spec key type_path = f"{module_name}.{class_name}" @@ -108,9 +109,9 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None: # Check for circular dependency if spec_key in visited: - raise CircularWiringError( - f"Circular dependency detected: '{spec_key}'" - ) + # Cycle detected: stop recursion. The runtime container will + # handle the cycle (or raise error) when resolving. + return visited.add(spec_key) # Import the class @@ -136,8 +137,10 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None: else: spec[spec_key] = {} return - except (ValueError, TypeError): - # Some built-in classes don't have inspectable signatures + except (ValueError, TypeError, RecursionError): + # Some built-in classes (or pathological annotations) don't have + # inspectable signatures; fall back to an empty entry to avoid + # infinite recursion. spec[spec_key] = {} return @@ -174,60 +177,13 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None: if not is_dependency: # For non-dependency params, set a constant value - if has_default and cls._is_constant(param_default): + if has_default and cls._is_spec_constant(param_default): spec[const_name] = cast(_ConstantValue, param_default) else: spec[const_name] = default_value spec[spec_key] = cast(_SpecMapping, params) - @classmethod - def _parse_entry(cls, entry: str) -> tuple[str, str, str, str | None]: - """Parse an entry string into components. - - Args: - entry: Entry string like "module.Class name" or - "module.Class name.factoryMethod" - - Returns: - Tuple of (module_name, class_name, instance_name, factory_method) - - Raises: - ValueError: If entry format is invalid - """ - if SPEC_KEY_DELIMITER not in entry: - raise ValueError( - f"Invalid entry format '{entry}': " - f"expected 'module.Class name'" - ) - - type_str, name_part = entry.rsplit(SPEC_KEY_DELIMITER, 1) - parts = type_str.split(".") - - if len(parts) < 2: - raise ValueError( - f"Invalid entry format '{entry}': " - f"missing module qualification" - ) - - module_name = ".".join(parts[:-1]) - class_name = parts[-1] - - # Check for factory method - factory_method: str | None = None - if "." in name_part: - instance_name, factory_method = name_part.split(".", 1) - else: - instance_name = name_part - - if factory_method and "." in factory_method: - raise ValueError( - f"invalid generator '{name_part}': nested factory methods " - f"are not supported." - ) - - return module_name, class_name, instance_name, factory_method - @classmethod def _get_default_for_type( cls, @@ -338,33 +294,10 @@ def _generate_dependency( # Recursively process (will add to spec) try: - cls._process_entry(entry, spec, visited.copy()) - except (CircularWiringError, AttributeError, ModuleNotFoundError): - # If circular or can't find module, just reference it + # Propagate the same visited set so that cycles are detected + cls._process_entry(entry, spec, visited) + except (AttributeError, ModuleNotFoundError): + # If can't find module, just reference it pass return None, True - - @classmethod - def _is_constant(cls, value: object) -> bool: - """Check if a value is a valid constant for a spec. - - Args: - value: Value to check - - Returns: - True if value can be stored as a spec constant - """ - if value is None or value is ...: - return True - if isinstance(value, (str, bytes, bool, int, float, complex)): - return True - if isinstance(value, (list, tuple, dict)): - # Check contents recursively - if isinstance(value, dict): - return all( - cls._is_constant(k) and cls._is_constant(v) - for k, v in value.items() - ) - return all(cls._is_constant(v) for v in value) - return False diff --git a/apywire/runtime.py b/apywire/runtime.py index 6610a61..970c95d 100644 --- a/apywire/runtime.py +++ b/apywire/runtime.py @@ -16,7 +16,6 @@ from apywire.constants import PLACEHOLDER_REGEX, SYNTHETIC_CONST from apywire.exceptions import ( - CircularWiringError, UnknownPlaceholderError, WiringError, ) @@ -94,12 +93,6 @@ def _init_thread_safety( self, max_lock_attempts, lock_retry_sleep ) - def _get_resolving_stack(self) -> list[str]: - """Return the resolving stack for the current context.""" - if self._thread_safe: - return ThreadSafeMixin._get_resolving_stack(self) - return self._resolving_stack - def __getattr__(self, name: str) -> Accessor: """Return a callable accessor for the named wired object.""" # If the name is in our parsed spec or constants, return an accessor. @@ -185,71 +178,54 @@ def _instantiate_impl(self, name: str) -> _RuntimeValue: "module.Class instance.from_date"), the factory method is called instead of the class constructor. """ - # Check for circular dependencies - stack = self._get_resolving_stack() - if name in stack: - raise CircularWiringError( - f"Circular wiring dependency detected: " - f"{' -> '.join(stack)} -> {name}" + if name in self._values: + return self._values[name] + + if name not in self._parsed: + # Should have been caught by __getattr__ or _resolve_runtime, + # but just in case. + raise UnknownPlaceholderError( + f"Unknown placeholder '{name}' referenced." ) - stack.append(name) - try: - if name in self._values: - return self._values[name] + entry = self._parsed[name] - if name not in self._parsed: - # Should have been caught by __getattr__ or _resolve_runtime, - # but just in case. - raise UnknownPlaceholderError( - f"Unknown placeholder '{name}' referenced." - ) + # Check for synthetic auto-promoted constant + if entry.module_name == SYNTHETIC_CONST and entry.class_name == "str": + # This is an auto-promoted constant with string interpolation + value = self._format_string_constant(entry.data, context=name) + self._values[name] = value + return value - entry = self._parsed[name] - - # Check for synthetic auto-promoted constant - if ( - entry.module_name == SYNTHETIC_CONST - and entry.class_name == "str" - ): - # This is an auto-promoted constant with string interpolation - value = self._format_string_constant(entry.data, context=name) - self._values[name] = value - return value - - module = importlib.import_module(entry.module_name) - cls = cast(_Constructor, getattr(module, entry.class_name)) - - # If a factory method is specified, get it from the class - if entry.factory_method: - constructor = cast( - _Constructor, getattr(cls, entry.factory_method) - ) - else: - constructor = cls + module = importlib.import_module(entry.module_name) + cls = cast(_Constructor, getattr(module, entry.class_name)) - # Resolve arguments - kwargs = self._resolve_runtime(entry.data, context=name) + # If a factory method is specified, get it from the class + if entry.factory_method: + constructor = cast( + _Constructor, getattr(cls, entry.factory_method) + ) + else: + constructor = cls - try: - if isinstance(kwargs, dict): - # Separate positional args (int keys) from keyword args - # (str keys) - pos_args, kwargs_dict = self._separate_args_kwargs(kwargs) - instance = constructor(*pos_args, **kwargs_dict) - elif isinstance(kwargs, list): - # All positional arguments - instance = constructor(*kwargs) - else: - instance = constructor(kwargs) - except Exception as e: - raise WiringError( - f"failed to instantiate '{name}': {e}" - ) from e - - return instance - finally: - stack.pop() + # Resolve arguments + kwargs = self._resolve_runtime(entry.data, context=name) + + try: + if isinstance(kwargs, dict): + # Separate positional args (int keys) from keyword args + # (str keys) + pos_args, kwargs_dict = self._separate_args_kwargs(kwargs) + instance = constructor(*pos_args, **kwargs_dict) + elif isinstance(kwargs, list): + # All positional arguments + instance = constructor(*kwargs) + else: + instance = constructor(kwargs) + except Exception as e: + raise WiringError(f"failed to instantiate '{name}': {e}") from e + + return instance def _resolve_runtime( self, diff --git a/apywire/threads.py b/apywire/threads.py index 91ed277..d30adf8 100644 --- a/apywire/threads.py +++ b/apywire/threads.py @@ -23,14 +23,12 @@ class _ThreadLocalState(threading.local): """Thread-local storage for wiring resolution state. Provides typed attributes initialized per-thread: - - resolving_stack: Stack for circular dependency detection - mode: Current instantiation mode ('optimistic', 'global', or None) - held_locks: Locks held by this thread """ def __init__(self) -> None: super().__init__() - self.resolving_stack: list[str] = [] self.mode: Literal["optimistic", "global"] | None = None self.held_locks: list[threading.RLock] = [] @@ -71,9 +69,6 @@ def _get_attribute_lock(self, name: str) -> threading.RLock: self._attr_locks[name] = threading.RLock() return self._attr_locks[name] - def _get_resolving_stack(self) -> list[str]: - return self._local.resolving_stack - def _get_held_locks(self) -> list[threading.RLock]: return self._local.held_locks @@ -121,12 +116,11 @@ def _wrap_instantiation_error(self, e: Exception, name: str) -> NoReturn: This method always raises an exception and never returns. """ from apywire.exceptions import ( - CircularWiringError, UnknownPlaceholderError, WiringError, ) - if isinstance(e, (UnknownPlaceholderError, CircularWiringError)): + if isinstance(e, UnknownPlaceholderError): raise # All other exceptions (including WiringError) are wrapped raise WiringError(f"failed to instantiate '{name}'") from e diff --git a/apywire/wiring.py b/apywire/wiring.py index a17c2b5..69d4a24 100644 --- a/apywire/wiring.py +++ b/apywire/wiring.py @@ -104,7 +104,77 @@ class _UnresolvedParsedEntry(NamedTuple): data: _SpecMapping -class WiringBase: +class SpecParser: + """Base class for parsing wiring specs.""" + + @staticmethod + def _parse_request_string( + key: str, + ) -> tuple[str, str, str, str | None] | None: + """Parse a request string into components. + + Args: + key: Entry string like "module.Class name" or + "module.Class name.factoryMethod" + + Returns: + Tuple of (module_name, class_name, instance_name, factory_method) + or None if the key is not a wired object definition + (e.g. constant). + """ + if SPEC_KEY_DELIMITER not in key: + return None + + type_str, name_part = key.rsplit(SPEC_KEY_DELIMITER, 1) + parts = type_str.split(".") + module_name = ".".join(parts[:-1]) + class_name = parts[-1] + + if "." in name_part: + name, factory_method = name_part.split(".", 1) + if "." in factory_method: + raise ValueError( + f"invalid spec key '{key}': nested factory methods " + f"are not supported." + ) + else: + name = name_part + factory_method = None + + if not module_name: + raise ValueError( + f"invalid spec key '{key}': missing module qualification" + ) + + return module_name, class_name, name, factory_method + + @staticmethod + def _is_spec_constant(value: object) -> bool: + """Check if a value is a valid constant for a spec. + + Args: + value: Value to check + + Returns: + True if value can be stored as a spec constant + """ + if value is None or value is ...: + return True + if isinstance(value, (str, bytes, bool, int, float, complex)): + return True + if isinstance(value, (list, tuple, dict)): + # Check contents recursively + if isinstance(value, dict): + return all( + SpecParser._is_spec_constant(k) + and SpecParser._is_spec_constant(v) + for k, v in value.items() + ) + return all(SpecParser._is_spec_constant(v) for v in value) + return False + + +class WiringBase(SpecParser): """Base class for wiring containers.""" _parsed: dict[str, _ParsedEntry] @@ -132,19 +202,44 @@ def __init__( self._max_lock_attempts = max_lock_attempts self._lock_retry_sleep = lock_retry_sleep - parsed: list[_UnresolvedParsedEntry] = [] - consts: dict[str, _SpecMapping | _ConstantValue] = {} + # Pre-parse all keys to correctly identify dependencies + # map: exposed_name -> (is_const, raw_value, parsed_entry_or_none) + all_entries: dict[ + str, tuple[bool, _SpecValue, _UnresolvedParsedEntry | None] + ] = {} - # First pass: classify entries into wired classes or constants for key, value in spec.items(): - entry = self._parse_spec_entry(key, value) - if entry is not None: + parsed_entry = self._parse_spec_entry(key, value) + if parsed_entry: + all_entries[parsed_entry.name] = (False, value, parsed_entry) + else: + all_entries[key] = (True, value, None) + + # Build full dependency graph for early cycle detection + full_deps: dict[str, set[str]] = {} + for name, (_, entry_val, _) in all_entries.items(): + deps = self._find_placeholder_names(entry_val) + full_deps[name] = deps + + # Detect circular dependencies across ALL entries + # This will raise CircularWiringError if a cycle exists. + sorted_keys = self._topological_sort(full_deps) + + # Now proceed with separation and processing + parsed: list[_UnresolvedParsedEntry] = [] + consts: dict[str, _SpecValue] = {} + + # Separation based on the pre-parsed results + for name in sorted_keys: + # keys in sorted_keys are 'exposed names' + # we need to find the original entry to process + is_const, entry_val, entry = all_entries[name] + if not is_const and entry: parsed.append(entry) else: - # It's a constant - consts[key] = value + consts[name] = entry_val - # Parse wired objects first + # Parse wired objects self._parsed: dict[str, _ParsedEntry] = { entry.name: _ParsedEntry( entry.module_name, @@ -155,33 +250,33 @@ def __init__( for entry in parsed } - # Classify constants by placeholder type - raw_consts: dict[str, _SpecMapping | _ConstantValue] = {} - const_with_refs: dict[ - str, tuple[_SpecMapping | _ConstantValue, set[str]] - ] = {} + # Handle constants + # Classify constants by placeholder type using ALREADY calculated deps + raw_consts: dict[str, _SpecValue] = {} + const_with_refs: dict[str, tuple[_SpecValue, set[str]]] = {} - for key, value in consts.items(): - placeholder_names = self._find_placeholder_names(value) + for key, const_val in consts.items(): + # REUSE full_deps[key] which we already computed! + placeholder_names = full_deps[key] if not placeholder_names: - # No placeholders - store immediately - raw_consts[key] = value + raw_consts[key] = const_val else: - # Has placeholders - classify later - const_with_refs[key] = (value, placeholder_names) + const_with_refs[key] = (const_val, placeholder_names) # Promote constants to accessors if they reference wired objects: - # Transitive: mark direct refs, then propagate to dependents + # Transitive promotion logic remains... const_deps_graph: dict[str, set[str]] = { key: placeholder_names - for key, (value, placeholder_names) in const_with_refs.items() + for key, (val_entry, placeholder_names) in const_with_refs.items() } - # Initially, mark constants that directly reference a wired object + to_promote = set() - for key, (value, placeholder_names) in const_with_refs.items(): + # Initial set: constants ref wired objects directly + for key, (_, placeholder_names) in const_with_refs.items(): if any(name in self._parsed for name in placeholder_names): to_promote.add(key) - # Propagate promotion transitively + + # Propagate promotion changed = True while changed: changed = False @@ -191,80 +286,48 @@ def __init__( if any(dep in to_promote for dep in deps): to_promote.add(key) changed = True - # Now, split into promoted and pure-constant refs - const_with_const_refs: dict[str, _SpecMapping | _ConstantValue] = {} - const_with_wired_refs: dict[str, _SpecMapping | _ConstantValue] = {} - for key, (value, placeholder_names) in const_with_refs.items(): + + # Split into promoted and pure-constant refs + const_with_const_refs: dict[str, _SpecValue] = {} + const_with_wired_refs: dict[str, _SpecValue] = {} + + for key, (const_val, _) in const_with_refs.items(): if key in to_promote: - const_with_wired_refs[key] = value + const_with_wired_refs[key] = const_val else: - const_with_const_refs[key] = value + const_with_const_refs[key] = const_val - # Topologically sort and expand constant-only refs - if const_with_const_refs: - const_deps = { - k: self._find_placeholder_names(v) - for k, v in const_with_const_refs.items() - } - sorted_const_keys = self._topological_sort(const_deps) - else: - sorted_const_keys = [] - - # Resolve constants in dependency order + # Resolve pure constants in dependency order + # We can reuse sorted_keys order, filtering for pure constants resolved_consts: dict[str, _RuntimeValue] = dict(raw_consts) - for key in sorted_const_keys: - resolved = self._resolve_constant( - const_with_const_refs[key], resolved_consts - ) - resolved_consts[key] = resolved + + for key in sorted_keys: + if key in const_with_const_refs: + resolved = self._resolve_constant( + const_with_const_refs[key], resolved_consts + ) + resolved_consts[key] = resolved self._values: dict[str, _RuntimeValue] = resolved_consts # Create synthetic parsed entries for auto-promoted constants - for key, value in const_with_wired_refs.items(): - # Create a synthetic entry that will format the string at runtime + for key, wired_val in const_with_wired_refs.items(): self._parsed[key] = _ParsedEntry( - SYNTHETIC_CONST, # Synthetic module marker - "str", # Will use string formatting - None, # No factory method - cast(str | _WiredRef, self._resolve(value)), + SYNTHETIC_CONST, + "str", + None, + cast(str | _WiredRef, self._resolve(wired_val)), ) - if not self._thread_safe: - # Non-thread-safe mode: use simple list for resolving stack - self._resolving_stack: list[str] = [] def _parse_spec_entry( self, key: str, value: _SpecMapping | _ConstantValue ) -> _UnresolvedParsedEntry | None: """Parse a spec entry. Returns None for constants.""" - if SPEC_KEY_DELIMITER not in key: - return None # It's a constant + result = self._parse_request_string(key) + if result is None: + return None - # class wiring: "module.Class name" or - # "module.Class name.factoryMethod" - type_str, name_part = key.rsplit(SPEC_KEY_DELIMITER, 1) - parts = type_str.split(".") - module_name = ".".join(parts[:-1]) - class_name = parts[-1] - - # Check if name_part contains a factory method - # e.g., "myInstance.from_date" -> name="myInstance", - # factory_method="from_date" - if "." in name_part: - name, factory_method = name_part.split(".", 1) - if "." in factory_method: - raise ValueError( - f"invalid spec key '{key}': nested factory methods " - f"are not supported." - ) - else: - name = name_part - factory_method = None - - if not module_name: - raise ValueError( - f"invalid spec key '{key}': missing module qualification" - ) + module_name, class_name, name, factory_method = result return _UnresolvedParsedEntry( module_name, @@ -409,9 +472,11 @@ def _topological_sort( if len(result) != len(dependencies): # Find nodes in cycle for error message unprocessed = [n for n in dependencies if n not in result] - raise CircularWiringError( - f"Circular dependency detected in constants: " - f"{', '.join(unprocessed)}" + + # Delegate construction of a helpful exception to the + # CircularWiringError helper to keep concerns separated. + raise CircularWiringError.from_unprocessed( + dependencies, unprocessed ) return result diff --git a/docs/api-reference.md b/docs/api-reference.md index 3e3c550..e007cad 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -125,7 +125,7 @@ Base exception class for all apywire errors. options: show_source: false -Raised when a circular dependency is detected: +Raised when a circular dependency is detected; see [Circular dependencies](user-guide/circular-dependencies.md) for examples and mitigation strategies: ```python from apywire import CircularWiringError, Wiring @@ -135,9 +135,8 @@ spec = { "MyClass b": {"dep": "{a}"}, } -wired = Wiring(spec) try: - obj = wired.a() + wired = Wiring(spec) except CircularWiringError as e: print(f"Circular dependency: {e}") ``` diff --git a/docs/development.md b/docs/development.md index f04743c..526b575 100644 --- a/docs/development.md +++ b/docs/development.md @@ -223,7 +223,7 @@ At instantiation time, `_WiredRef` markers are resolved to actual objects. Thread-safe mode uses: 1. **Per-attribute locks**: `dict[str, threading.Lock]` 2. **Global fallback lock**: Single `threading.Lock` -3. **Thread-local state**: For circular dependency detection +3. **Thread-local state**: For lock-related per-thread state used by the thread-safety mixin (mode, held locks) See `apywire/threads.py` for details. diff --git a/docs/getting-started.md b/docs/getting-started.md index 0feb636..db573ab 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -150,23 +150,6 @@ When you access a wired object: 4. Instantiates the object with the resolved parameters 5. Caches it for future access -### 5. Circular Dependency Detection - -apywire automatically detects circular dependencies: - -```python -spec = { - "MyClass a": {"dependency": "{b}"}, - "MyClass b": {"dependency": "{a}"}, # Circular! -} - -wired = Wiring(spec) -try: - obj = wired.a() -except CircularWiringError as e: - print(f"Circular dependency detected: {e}") -``` - ## Next Steps Now that you understand the basics, explore more advanced features: diff --git a/docs/user-guide/advanced.md b/docs/user-guide/advanced.md index f6e3c0d..2cfffe6 100644 --- a/docs/user-guide/advanced.md +++ b/docs/user-guide/advanced.md @@ -229,30 +229,7 @@ assert coord.logger is coord.services[1].logger ### CircularWiringError -Raised when a circular dependency is detected: - -```python -from apywire import CircularWiringError - -spec = { - "MyClass a": {"dependency": "{b}"}, - "MyClass b": {"dependency": "{a}"}, # Circular! -} - -wired = Wiring(spec) -try: - obj = wired.a() -except CircularWiringError as e: - print(f"Circular dependency: {e}") - # Error message shows the dependency chain -``` - -The error message includes the full resolution chain: - -``` -Circular dependency detected while resolving 'a': - a -> b -> a -``` +A cycle in wiring will raise `CircularWiringError`; see the guide: [Circular dependencies](user-guide/circular-dependencies.md) for examples and mitigation strategies. ### UnknownPlaceholderError diff --git a/docs/user-guide/async-support.md b/docs/user-guide/async-support.md index fe109c5..5c8a9e6 100644 --- a/docs/user-guide/async-support.md +++ b/docs/user-guide/async-support.md @@ -256,7 +256,7 @@ async def main(): ## Error Handling -Async access raises the same exceptions as sync access: +Async access raises the same exceptions as sync access; see [Circular dependencies](user-guide/circular-dependencies.md) for details on `CircularWiringError`. ```python from apywire import UnknownPlaceholderError, CircularWiringError diff --git a/docs/user-guide/basic-usage.md b/docs/user-guide/basic-usage.md index 99d8009..00dc5ff 100644 --- a/docs/user-guide/basic-usage.md +++ b/docs/user-guide/basic-usage.md @@ -280,22 +280,9 @@ except UnknownPlaceholderError as e: print(f"Unknown placeholder: {e}") ``` -### Circular Dependencies +### Circular dependencies -```python -from apywire import Wiring, CircularWiringError - -spec = { - "MyClass a": {"dep": "{b}"}, - "MyClass b": {"dep": "{a}"}, -} - -wired = Wiring(spec) -try: - obj = wired.a() -except CircularWiringError as e: - print(f"Circular dependency: {e}") -``` +See [Circular dependencies](user-guide/circular-dependencies.md) for examples, expected exceptions, and advice to avoid cycles. ### Import Errors diff --git a/docs/user-guide/circular-dependencies.md b/docs/user-guide/circular-dependencies.md new file mode 100644 index 0000000..d69c427 --- /dev/null +++ b/docs/user-guide/circular-dependencies.md @@ -0,0 +1,116 @@ + + +# Circular dependencies + +Circular dependencies occur when two or more wired entries reference each other (directly or transitively), producing a resolution loop that cannot be satisfied. + +This guide explains how apywire detects cycles, what exception is raised, examples, and practical ways to avoid them. + +--- + +## Overview + +When a spec contains placeholders that form a cycle, wiring cannot determine a deterministic instantiation order. apywire treats cycles as errors and reports them so you can fix your design or introduce a controlled indirection. + +## How apywire detects cycles + +- During parsing and initialization, apywire builds a dependency graph that maps exposed wiring names to the placeholders they reference. +- The container attempts a topological sort (Kahn's algorithm) to determine an instantiation order. If the sort cannot process all nodes, the remaining nodes are part of a cycle. +- The `Generator` utility also tracks visited entries and stops recursion silently when it detects recursion. + +## Exception raised + +When a cycle is detected apywire raises `CircularWiringError`. The exception message lists the involved entries and, when possible, includes a readable cycle chain showing the resolution order. + +Example (wired objects): + +```python +from apywire import Wiring, CircularWiringError + +spec = { + "MyClass a": {"dep": "{b}"}, + "MyClass b": {"dep": "{a}"}, +} + +try: + Wiring(spec) +except CircularWiringError as e: + print(e) + # Example output: Circular dependency detected: a, b; cycle: a -> b -> a +``` + +Example (constants): + +```python +from apywire import Wiring, CircularWiringError + +spec = { + "a": "{b}", + "b": "{a}", +} + +try: + Wiring(spec) +except CircularWiringError as e: + print(e) + # Example output (actual): Circular dependency detected: a, b; cycle: a -> b -> a +``` + +## Examples + +Simple two-node cycle (wired objects): + +```python +from apywire import Wiring, CircularWiringError + +spec = { + "MyClass a": {"dependency": "{b}"}, + "MyClass b": {"dependency": "{a}"}, # Circular! +} + +try: + wired = Wiring(spec) +except CircularWiringError as e: + print(e) +``` + +Cycle in constant placeholders: + +```python +from apywire import Wiring, CircularWiringError + +spec = { + "a": "{b}", + "b": "{a}", +} + +with pytest.raises(CircularWiringError): + Wiring(spec) +``` + +For async accessors, cycles are detected during Wiring() initialization just like for synchronous usage, and a `CircularWiringError` is raised before any async accessors are created. + +## How to avoid circular dependencies + +- Split responsibilities into smaller, decoupled components so they don't require mutual placeholders. +- Use a factory or late-bound accessor instead of a placeholder for one side of the relationship. +- Inject interfaces or configuration rather than concrete implementations when suitable. +- Use compilation or explicit initialization ordering when you need concrete control over instantiation. + +## Thread-safety and cross-thread detection + +Cycle detection for placeholders is performed during spec parsing/initialization using a topological sort, so cycles are detected early and reported via `CircularWiringError`. When `thread_safe=True`, apywire additionally uses per-attribute locks to serialize instantiation; thread-local storage is used to keep lock-related per-thread state and avoid cross-thread interference (see `threads.py` for details). + +## Troubleshooting and tips + +- Reproduce the cycle with a small spec and a focused test. +- Inspect the error message chain to identify the involved entries. +- If a cycle is intentional, convert one dependency to a factory or resolver to avoid placeholder-based cycles. + +--- + +See also: `Wiring` (../user-guide/basic-usage.md) — if you have questions about a specific scenario, add a short reproducer test and we can help iterate on recommendations. diff --git a/docs/user-guide/generator.md b/docs/user-guide/generator.md index 8fb31c7..5cf5817 100644 --- a/docs/user-guide/generator.md +++ b/docs/user-guide/generator.md @@ -86,8 +86,6 @@ expected_spec = { - Non-constant parameter defaults (e.g. object instances) fall back to type defaults. - Dependencies (constructor parameters typed as another class) are generated recursively as separate spec entries, unless importing the dependency fails. -- Circular dependencies may raise `apywire.CircularWiringError` or generate partial - specs (tests consider both outcomes acceptable). - Built-in types or types whose signatures cannot be inspected may result in an empty `{}` entry for that wiring key. @@ -222,7 +220,6 @@ Returns: Raises: - `ValueError` for invalid entry strings (e.g., missing delimiter or module) -- `apywire.exceptions.CircularWiringError` when a cycle is detected ## Notes and Limitations diff --git a/docs/user-guide/index.md b/docs/user-guide/index.md index 2c6784b..181e5a0 100644 --- a/docs/user-guide/index.md +++ b/docs/user-guide/index.md @@ -73,7 +73,7 @@ Looking for something specific? 1. Define test-specific specs with mock objects 2. Use placeholder references to inject test doubles 3. Leverage lazy loading to avoid unnecessary instantiation -4. Test circular dependency error handling +4. Test circular dependency behavior — see [Circular dependencies](user-guide/circular-dependencies.md) ## Best Practices diff --git a/docs/user-guide/thread-safety.md b/docs/user-guide/thread-safety.md index ccc0e04..d59e57a 100644 --- a/docs/user-guide/thread-safety.md +++ b/docs/user-guide/thread-safety.md @@ -247,18 +247,7 @@ The generated code will include the same two-level locking mechanism. ## Thread-Local State -apywire uses thread-local storage to track the resolution stack for circular dependency detection: - -```python -# Under the hood -import threading - -class _ThreadLocalState(threading.local): - def __init__(self): - self.resolving: list[str] = [] -``` - -This ensures circular dependency detection works correctly across threads without interference. +apywire uses thread-local storage to keep lock-related per-thread state used by the thread-safety mixin (for example: the current locking `mode` and the list of `held_locks`). This keeps per-thread locking context isolated and prevents cross-thread interference. See `apywire/threads.py` for the concrete implementation. ## Testing Thread Safety diff --git a/mkdocs.yml b/mkdocs.yml index e2e57b6..4f1fc15 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -49,6 +49,7 @@ nav: - Thread Safety: user-guide/thread-safety.md - Compilation: user-guide/compilation.md - Generator: user-guide/generator.md + - Circular Dependencies: user-guide/circular-dependencies.md - Advanced Features: user-guide/advanced.md - API Reference: api-reference.md - Examples: examples.md diff --git a/tests/test_constant_placeholders.py b/tests/test_constant_placeholders.py index 8fdf35c..e498b6e 100644 --- a/tests/test_constant_placeholders.py +++ b/tests/test_constant_placeholders.py @@ -95,7 +95,12 @@ def test_circular_dependency_in_constants() -> None: with pytest.raises(apywire.CircularWiringError) as exc_info: apywire.Wiring(spec, thread_safe=False) - assert "Circular dependency detected in constants" in str(exc_info.value) + msg = str(exc_info.value) + assert "Circular dependency detected" in msg + # Ensure the message lists the involved entries + assert "a" in msg and "b" in msg + # Prefer a readable cycle chain if present + assert "->" in msg or "," in msg def test_circular_with_wired_objects() -> None: @@ -115,11 +120,14 @@ def __init__(self, dep: object) -> None: "test_module.MyClass a": {"dep": "{b}"}, "test_module.MyClass b": {"dep": "{a}"}, } - wired = apywire.Wiring(spec, thread_safe=False) - - # Should raise when accessed, not at init - with pytest.raises(apywire.CircularWiringError): - wired.a() + # Should raise at init time now + with pytest.raises(apywire.CircularWiringError) as exc_info: + apywire.Wiring(spec, thread_safe=False) + + msg = str(exc_info.value) + assert "Circular dependency detected" in msg + assert "a" in msg and "b" in msg + assert "->" in msg or "," in msg finally: del sys.modules["test_module"] diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index 1be3e3e..c68f177 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -30,10 +30,6 @@ def test_thread_safety_init_and_helpers() -> None: """Test initialization and basic helpers of ThreadSafeMixin.""" container = MockContainer() - # Test _get_resolving_stack initialization - assert container._get_resolving_stack() == [] - assert container._get_resolving_stack() is container._get_resolving_stack() - # Test _get_held_locks initialization assert container._get_held_locks() == [] assert container._get_held_locks() is container._get_held_locks() @@ -205,18 +201,6 @@ def test_wiring_aio_lazy_init() -> None: assert wired.aio is aio -def test_wiring_resolving_stack_non_thread_safe() -> None: - """Test resolving stack initialization in non-thread-safe mode.""" - from apywire import Wiring - - wired = Wiring({}, thread_safe=False) - # It IS initialized in __init__ for non-thread-safe mode - assert hasattr(wired, "_resolving_stack") - stack = wired._get_resolving_stack() - assert stack == [] - assert wired._get_resolving_stack() is stack - - def test_compile_constant_aio() -> None: """Test _compile_constant_property with aio=True.""" from apywire import Spec, WiringCompiler diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py new file mode 100644 index 0000000..155c575 --- /dev/null +++ b/tests/test_exceptions.py @@ -0,0 +1,59 @@ +# SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas +# +# SPDX-License-Identifier: ISC + +"""Tests for exception classes and helpers.""" + +from __future__ import annotations + +import re + +import apywire +from apywire import CircularWiringError +from apywire.exceptions import FormatError + + +def test_circular_wiring_error_from_unprocessed_detects_cycle() -> None: + """The helper should return an error message that includes a cycle path.""" + + deps = { + "a": {"b"}, + "b": {"a"}, + "c": set(), + } + + err = CircularWiringError.from_unprocessed(deps, ["a", "b"]) + + msg = str(err) + assert "Circular dependency detected" in msg + assert "a" in msg and "b" in msg + assert "cycle:" in msg + # cycle should be rendered as a -> b -> a or b -> a -> b + assert re.search(r"a\s*->\s*b\s*->\s*a|b\s*->\s*a\s*->\s*b", msg) + + +def test_circular_wiring_error_from_unprocessed_no_cycle_path() -> None: + """If no cycle path can be extracted, message should list the nodes.""" + + deps: dict[str, set[str]] = { + "a": set(), + "b": set(), + } + + err = CircularWiringError.from_unprocessed(deps, ["a", "b"]) + msg = str(err) + + assert msg == "Circular dependency detected: a, b" + + +def test_format_error_includes_format_and_message() -> None: + fe = FormatError("json", "invalid token") + assert fe.format == "json" + assert str(fe) == "JSON format error: invalid token" + + +def test_exception_hierarchy() -> None: + """Sanity checks for exception subclassing.""" + assert issubclass(apywire.UnknownPlaceholderError, apywire.WiringError) + assert issubclass(apywire.CircularWiringError, apywire.WiringError) + assert issubclass(apywire.LockUnavailableError, RuntimeError) diff --git a/tests/test_generator.py b/tests/test_generator.py index 27c87c9..84b59ed 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -570,26 +570,26 @@ def __init__(self) -> None: def test_is_constant_with_collections() -> None: - """Test _is_constant with various collection types.""" + """Test _is_spec_constant with various collection types.""" from apywire.generator import Generator # Test nested lists and dicts - assert Generator._is_constant([1, 2, 3]) - assert Generator._is_constant({"a": 1, "b": 2}) - assert Generator._is_constant([1, [2, 3]]) - assert Generator._is_constant({"a": {"b": 1}}) - assert Generator._is_constant((1, 2, 3)) + assert Generator._is_spec_constant([1, 2, 3]) + assert Generator._is_spec_constant({"a": 1, "b": 2}) + assert Generator._is_spec_constant([1, [2, 3]]) + assert Generator._is_spec_constant({"a": {"b": 1}}) + assert Generator._is_spec_constant((1, 2, 3)) # Test with non-constants class Obj: pass - assert not Generator._is_constant(Obj()) - assert not Generator._is_constant([Obj()]) - assert not Generator._is_constant({"key": Obj()}) + assert not Generator._is_spec_constant(Obj()) + assert not Generator._is_spec_constant([Obj()]) + assert not Generator._is_spec_constant({"key": Obj()}) # Test ellipsis - assert Generator._is_constant(...) + assert Generator._is_spec_constant(...) def test_generate_dependency_with_missing_module() -> None: @@ -627,9 +627,12 @@ def __init__(self) -> None: def test_generate_actual_circular_dependency() -> None: - """Test that actual circular dependency raises error.""" - from apywire.exceptions import CircularWiringError + """Test that circular dependency stops recursion silently. + The generator now returns immediately when a cycle is detected, + rather than raising an error. This allows partial specs to be + generated, with the runtime container handling cycle detection. + """ # Manually create a spec with visited tracking spec: Spec = {} visited: set[str] = {"mockmod_circ.Node root"} @@ -650,10 +653,10 @@ def __init__(self) -> None: # Try to process entry that's already in visited from apywire.generator import Generator + # Should return silently (no error) when cycle detected Generator._process_entry("mockmod_circ.Node root", spec, visited) - assert False, "Should have raised CircularWiringError" - except CircularWiringError as e: - assert "Circular dependency detected" in str(e) + # Spec should be empty since we returned early + assert spec == {} finally: if "mockmod_circ" in sys.modules: del sys.modules["mockmod_circ"] diff --git a/tests/test_internals.py b/tests/test_internals.py index a4931de..e60ca21 100644 --- a/tests/test_internals.py +++ b/tests/test_internals.py @@ -3,7 +3,6 @@ # SPDX-License-Identifier: ISC import ast -import threading import apywire from apywire.wiring import _ResolvedValue, _WiredRef @@ -61,40 +60,3 @@ class Dummy: # ast.Constant.value is typed as a union of constant types, but we're # testing the fallback behavior, so we cast to object for the check assert cast(object, const_node.value) is d - - -def test_get_resolving_stack_returns_per_thread_or_shared() -> None: - # Non-thread-safe: stacks should be the same across calls and threads - w1 = apywire.Wiring({}, thread_safe=False) - s1 = w1._get_resolving_stack() - s2 = w1._get_resolving_stack() - assert s1 is s2 - - stack_id_in_thread = None - - def capture_shared_stack_id() -> None: - nonlocal stack_id_in_thread - stack_id_in_thread = id(w1._get_resolving_stack()) - - t = threading.Thread(target=capture_shared_stack_id) - t.start() - t.join() - assert stack_id_in_thread == id(s1) - - # Thread-safe: each thread should get a distinct resolving stack - w2 = apywire.Wiring({}, thread_safe=True) - st_main = w2._get_resolving_stack() - st_main2 = w2._get_resolving_stack() - assert st_main is st_main2 - - main_stack_id = id(st_main) - stack_id_other_thread = None - - def capture_local_stack_id() -> None: - nonlocal stack_id_other_thread - stack_id_other_thread = id(w2._get_resolving_stack()) - - t2 = threading.Thread(target=capture_local_stack_id) - t2.start() - t2.join() - assert stack_id_other_thread != main_stack_id diff --git a/tests/test_single.py b/tests/test_single.py index a19cdc8..ae7bd20 100644 --- a/tests/test_single.py +++ b/tests/test_single.py @@ -649,14 +649,11 @@ def __init__(self) -> None: "mymod_circ.A a": {"b": "{b}"}, "mymod_circ.B b": {"a": "{a}"}, } - wired: apywire.Wiring = apywire.Wiring(spec, thread_safe=False) try: - _ = wired.a() - assert ( - False - ), "Should have raised CircularWiringError for circular dependency" + apywire.Wiring(spec, thread_safe=False) + assert False, "Should have raised CircularWiringError at init" except apywire.CircularWiringError as e: - assert "Circular wiring dependency detected" in str(e) + assert "Circular dependency detected" in str(e) finally: if "mymod_circ" in sys.modules: del sys.modules["mymod_circ"] diff --git a/tests/test_threading.py b/tests/test_threading.py index bf5afe4..0ea9033 100644 --- a/tests/test_threading.py +++ b/tests/test_threading.py @@ -385,12 +385,9 @@ def __init__(self) -> None: "mymod_circ_nonthreaded.A a": {"b": "{b}"}, "mymod_circ_nonthreaded.B b": {"a": "{a}"}, } - wired: apywire.Wiring = apywire.Wiring(spec, thread_safe=False) - try: - _ = wired.a() - assert False, "Should have raised CircularWiringError" - except apywire.CircularWiringError as e: - assert "Circular wiring dependency detected" in str(e) + with pytest.raises(apywire.CircularWiringError) as exc_info: + apywire.Wiring(spec, thread_safe=False) + assert "Circular dependency detected" in str(exc_info.value) finally: if "mymod_circ_nonthreaded" in sys.modules: del sys.modules["mymod_circ_nonthreaded"] @@ -420,12 +417,9 @@ def __init__(self) -> None: "mymod_circ_threaded.A a": {"b": "{b}"}, "mymod_circ_threaded.B b": {"a": "{a}"}, } - wired: apywire.Wiring = apywire.Wiring(spec, thread_safe=True) - try: - _ = wired.a() - assert False, "Should have raised CircularWiringError" - except apywire.CircularWiringError as e: - assert "Circular wiring dependency detected" in str(e) + with pytest.raises(apywire.CircularWiringError) as exc_info: + apywire.Wiring(spec, thread_safe=True) + assert "Circular dependency detected" in str(exc_info.value) finally: if "mymod_circ_threaded" in sys.modules: del sys.modules["mymod_circ_threaded"]