Skip to content

Commit 85f80b6

Browse files
NSHkrNSHkr
NSHkr
authored and
NSHkr
committed
extract plan
1 parent ce7db8f commit 85f80b6

File tree

2 files changed

+324
-0
lines changed

2 files changed

+324
-0
lines changed

‎docs/extract/01_foundation_enhance.md‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
Okay, let's review the `ElixirScope.Foundation` layer as described in `API_FULL.md` and the provided `repomix-output.xml` to identify components that would benefit from being defined by behaviours (contracts) and other similar architectural modifications to improve modularity, testability, and potential for future extraction or alternative implementations.
2+
3+
**Key Principle for Using Behaviours:**
4+
5+
A module or a set of related modules should implement a behaviour if:
6+
7+
1. **Multiple Implementations Possible/Desirable:** You foresee different ways to achieve the same functionality (e.g., in-memory config vs. Etcd/Consul, ETS event store vs. PostgreSQL event store).
8+
2. **Clear Contract Definition:** The public API for a specific concern can be clearly defined and separated from its implementation details.
9+
3. **Testability:** Allows for easy mocking of dependencies during testing.
10+
4. **Extensibility:** Enables users or other parts of the system to provide their own implementations.
11+
5. **Decoupling:** Reduces direct dependencies on concrete implementations.
12+
13+
**Review of `ElixirScope.Foundation` Components:**
14+
15+
Based on `API_FULL.md` and `repomix-output.xml`:
16+
17+
---
18+
19+
1. **`ElixirScope.Foundation.Config` (`ConfigServer`, `ConfigLogic`, `ConfigValidator`)**
20+
* **Current State:** Already uses `@behaviour ElixirScope.Foundation.Contracts.Configurable`. This is excellent!
21+
* **Analysis:**
22+
* The `Configurable` contract clearly defines the API for getting, updating, validating, and subscribing to configuration.
23+
* `ConfigServer` is the GenServer implementation.
24+
* `ConfigLogic` contains pure business logic.
25+
* `ConfigValidator` contains validation logic.
26+
* **Recommendations:**
27+
* **Retain Behaviour:** The current use of `Configurable` is appropriate.
28+
* **Consider Data Source Abstraction (Optional Future):** If configuration could come from diverse sources (files, env vars, remote service like Etcd/Consul, database), the *loading* mechanism within `ConfigServer` or `ConfigLogic.build_config` could itself be abstracted further, perhaps with a `ConfigSource` behaviour. However, for now, the current `Configurable` contract is sufficient for the *runtime interaction* with the config service.
29+
30+
---
31+
32+
2. **`ElixirScope.Foundation.Events` (`EventStore`, `EventLogic`, `EventValidator`)**
33+
* **Current State:** Already uses `@behaviour ElixirScope.Foundation.Contracts.EventStore`. Also excellent!
34+
* **Analysis:**
35+
* The `EventStore` contract defines the API for storing, retrieving, and querying events.
36+
* `EventStore` (service) is the GenServer implementation (presumably in-memory ETS based on description).
37+
* `EventLogic` contains pure business logic for event creation/manipulation.
38+
* `EventValidator` contains validation.
39+
* **Recommendations:**
40+
* **Retain Behaviour:** The `EventStore` contract is well-placed. This allows for different backing stores (e.g., a persistent database-backed store, a distributed event store, or even a no-op store for certain environments) without changing the `ElixirScope.Foundation.Events` public API.
41+
* **Serialization Behaviour (Minor Consideration):** `EventLogic` has `serialize_event` and `deserialize_event` using `:erlang.term_to_binary`. If alternative serialization formats (JSON, Protobuf) were ever needed, a small `EventSerializer` behaviour could be introduced, though `:erlang.term_to_binary` is highly efficient for Elixir terms. For now, this is likely fine.
42+
43+
---
44+
45+
3. **`ElixirScope.Foundation.Telemetry` (`TelemetryService`)**
46+
* **Current State:** Already uses `@behaviour ElixirScope.Foundation.Contracts.Telemetry`. Good.
47+
* **Analysis:**
48+
* The `Telemetry` contract defines how metrics are emitted and managed.
49+
* `TelemetryService` is the GenServer implementation.
50+
* **Recommendations:**
51+
* **Retain Behaviour:** This allows for different telemetry backends or handlers in the future (e.g., forwarding to Prometheus, Datadog, or a different internal aggregation system).
52+
* **Consider Handler Abstraction (If Needed):** The `attach_handlers/1` suggests custom handlers. If the *types* of handlers become diverse and need to conform to a specific interface for how they process telemetry events, a `TelemetryEventHandler` behaviour could be defined. Standard Elixir `:telemetry` handlers already follow a contract, so this might be redundant unless ElixirScope imposes additional structure.
53+
54+
---
55+
56+
4. **`ElixirScope.Foundation.Infrastructure` (Facade for `CircuitBreaker`, `RateLimiter`, `ConnectionManager`)**
57+
* **`CircuitBreaker` (wraps `:fuse`)**
58+
* **Current State:** Direct wrapper around `:fuse`.
59+
* **Analysis:** The `CircuitBreaker` module itself presents a clean API. If you ever wanted to switch from `:fuse` to another circuit breaker library (e.g., `CircuitBreaker.ex`) or implement a custom one, `ElixirScope.Foundation.Infrastructure.CircuitBreaker` would need to change.
60+
* **Recommendations:**
61+
* **Introduce `CircuitBreakerProvider` Behaviour:**
62+
* Define a `CircuitBreakerProvider` behaviour with callbacks like `start_instance/2`, `execute/3`, `status/1`, `reset/1`.
63+
* `ElixirScope.Foundation.Infrastructure.CircuitBreaker` would then become a module that *uses* a configured implementation of this behaviour.
64+
* A `FuseAdapter` module would implement this behaviour using `:fuse`.
65+
* This makes the `Infrastructure` module independent of the specific circuit breaker library.
66+
* **`RateLimiter` (wraps `Hammer`)**
67+
* **Current State:** Direct wrapper around `Hammer`.
68+
* **Analysis:** Similar to `CircuitBreaker`. If you wanted to use a different rate-limiting backend (e.g., Redis-based for distributed rate limiting, or a different algorithm).
69+
* **Recommendations:**
70+
* **Introduce `RateLimiterProvider` Behaviour:**
71+
* Define a `RateLimiterProvider` behaviour with callbacks like `check_rate/4` (or 5 if metadata is always passed), `status/2`, `reset/2`.
72+
* `ElixirScope.Foundation.Infrastructure.RateLimiter` would use a configured implementation.
73+
* A `HammerAdapter` module would implement this behaviour.
74+
* **`ConnectionManager` (wraps `Poolboy`)**
75+
* **Current State:** Facade for `Poolboy`. The worker modules (like `HttpWorker`) are pluggable.
76+
* **Analysis:** `Poolboy` is a specific pooling library. While common, other pooling libraries exist (e.g., `sbroker`, `pooler`), or one might want a custom pooling strategy. The `worker_module` configuration makes the *type* of resource pooled flexible, but not the pooling *mechanism* itself.
77+
* **Recommendations:**
78+
* **Introduce `ConnectionPoolProvider` Behaviour:**
79+
* Define a behaviour with callbacks like `start_pool/2`, `stop_pool/1`, `with_connection/3` (or `checkout/checkin`), `status/1`.
80+
* `ElixirScope.Foundation.Infrastructure.ConnectionManager` would use a configured implementation.
81+
* A `PoolboyAdapter` module would implement this.
82+
* This decouples the `Infrastructure` facade from `Poolboy`.
83+
* **`ElixirScope.Foundation.Infrastructure` (Facade)**
84+
* **Current State:** Provides `execute_protected/3`.
85+
* **Analysis:** This facade is good. Its reliance on concrete implementations of circuit breaker, rate limiter, and pool manager would be shifted to rely on the proposed provider behaviours.
86+
* **Recommendations:** No new behaviour needed here, but it would consume the new provider behaviours.
87+
88+
---
89+
90+
5. **`ElixirScope.Foundation.ServiceRegistry` & `ElixirScope.Foundation.ProcessRegistry`**
91+
* **Current State:** `ProcessRegistry` directly uses `Registry`. `ServiceRegistry` is a higher-level API over `ProcessRegistry`.
92+
* **Analysis:**
93+
* Elixir's `Registry` is already a well-defined OTP component. Abstracting `Registry` itself is usually not necessary unless you need a fundamentally different discovery mechanism (e.g., Consul, Etcd for distributed scenarios *beyond* what `Registry` with distributed Erlang offers).
94+
* `ProcessRegistry` provides namespacing, which is a valuable addition.
95+
* `ServiceRegistry` adds health checks and lifecycle management on top.
96+
* **Recommendations:**
97+
* **Likely Fine As Is (for internal use):** For internal service discovery within an ElixirScope application, the current setup is probably sufficient.
98+
* **If for a Standalone Library (Future):** If `Foundation` becomes a standalone library intended for *very* broad use, one *could* define a `RegistryProvider` behaviour that `ProcessRegistry` consumes, allowing users to plug in different distributed registry backends. However, this adds significant complexity and might be overkill for most Elixir applications that can leverage `Registry` and Distributed Erlang.
99+
* The key abstraction is the *concept* of service registration and lookup. The current API of `ServiceRegistry` effectively provides this.
100+
101+
---
102+
103+
6. **`ElixirScope.Foundation.ErrorContext`**
104+
* **Current State:** Defines a concrete `ErrorContext.t` struct and functions to manage it.
105+
* **Analysis:** This is a specific implementation for contextual error information.
106+
* **Recommendations:**
107+
* **Likely Fine As Is:** This module provides a specific way of handling error context. It's more of a utility/data structure with associated functions. It doesn't scream for a behaviour unless you envision radically different ways of *structuring* or *propagating* operational context that cannot be achieved by just populating the `metadata` field differently.
108+
* If the *storage* or *retrieval* of the current context (e.g., from `Process.put/get`) became pluggable (e.g., async storage, distributed context), then that specific part could be behavioral.
109+
110+
---
111+
112+
7. **`ElixirScope.Foundation.Error` & `ElixirScope.Foundation.Types.Error`**
113+
* **Current State:** Defines a specific `Error.t` struct and a module for creating/handling these.
114+
* **Analysis:** This is ElixirScope's chosen structured error format.
115+
* **Recommendations:**
116+
* **Likely Fine As Is (for ElixirScope):** This is a core typing decision.
117+
* **If for a Standalone Library (Future):** A generic library might:
118+
* Define a `FoundationError` behaviour that applications can implement for their own error structs.
119+
* Or, provide this `Error.t` as *its* standard, and other applications can choose to adopt or wrap it.
120+
* The current `Error.new/3`, `wrap_error/4` are utility functions for this specific error type.
121+
122+
---
123+
124+
8. **`ElixirScope.Foundation.Utils`**
125+
* **Current State:** A collection of utility functions.
126+
* **Analysis:** These are generally standalone helper functions.
127+
* **Recommendations:**
128+
* **No Behaviour Needed:** Utility modules don't typically need to implement behaviours.
129+
* **Consider ID Generation Abstraction (Minor):** If `Utils.generate_id()` or `Utils.generate_correlation_id()` ever needed different strategies (e.g., ksuid, ULID, centrally coordinated IDs), a small `IdGenerator` behaviour could be introduced. For now, it's likely fine.
130+
131+
---
132+
133+
**Summary of Proposed Behavioural Changes & Other Modifications:**
134+
135+
* **New Behaviours to Introduce:**
136+
1. **`CircuitBreakerProvider`**:
137+
* `@callback execute(name :: atom(), operation :: (-> any()), metadata :: map()) :: {:ok, any()} | {:error, term()}`
138+
* `@callback start_instance(name :: atom(), options :: keyword()) :: :ok | {:error, term()}`
139+
* `@callback status(name :: atom()) :: :ok | :blown | {:error, term()}`
140+
* `@callback reset(name :: atom()) :: :ok | {:error, term()}`
141+
2. **`RateLimiterProvider`**:
142+
* `@callback check_rate(entity_id :: any(), rule_name :: atom(), limit :: pos_integer(), window_ms :: pos_integer(), metadata :: map()) :: :ok | {:error, term()}`
143+
* `@callback status(entity_id :: any(), rule_name :: atom()) :: {:ok, map()}` (or similar)
144+
3. **`ConnectionPoolProvider`**:
145+
* `@callback start_pool(pool_name :: atom(), config :: keyword()) :: {:ok, pid()} | {:error, term()}`
146+
* `@callback stop_pool(pool_name :: atom()) :: :ok | {:error, :not_found}`
147+
* `@callback with_connection(pool_name :: atom(), fun :: (pid() -> term()), timeout :: timeout()) :: {:ok, term()} | {:error, term()}`
148+
* `@callback status(pool_name :: atom()) :: {:ok, map()} | {:error, :not_found}`
149+
150+
* **Modules to Retain Existing Behaviours:**
151+
* `ElixirScope.Foundation.Config` (via `Configurable`)
152+
* `ElixirScope.Foundation.Events` (via `EventStore`)
153+
* `ElixirScope.Foundation.Telemetry` (via `Telemetry`)
154+
155+
* **Modules Likely Fine Without New Behaviours (for now):**
156+
* `ElixirScope.Foundation.ProcessRegistry` (as it wraps OTP's `Registry`)
157+
* `ElixirScope.Foundation.ServiceRegistry` (provides a higher-level API)
158+
* `ElixirScope.Foundation.ErrorContext`
159+
* `ElixirScope.Foundation.Error`
160+
* `ElixirScope.Foundation.Utils`
161+
162+
**Other Similar Modifications (Beyond Behaviours):**
163+
164+
1. **Configuration for Provider Implementations:**
165+
* The Foundation Config system (`ElixirScope.Foundation.Config`) would need to hold configuration specifying which adapter/implementation to use for `CircuitBreakerProvider`, `RateLimiterProvider`, and `ConnectionPoolProvider`.
166+
* Example:
167+
```elixir
168+
# In config.exs
169+
config :elixir_scope, ElixirScope.Foundation.Infrastructure,
170+
circuit_breaker_provider: ElixirScope.Foundation.Infrastructure.FuseAdapter,
171+
rate_limiter_provider: ElixirScope.Foundation.Infrastructure.HammerAdapter,
172+
connection_pool_provider: ElixirScope.Foundation.Infrastructure.PoolboyAdapter
173+
```
174+
* The `Infrastructure` module would then dynamically call the configured provider.
175+
176+
2. **Adapter Modules:**
177+
* `FuseAdapter` implementing `CircuitBreakerProvider`.
178+
* `HammerAdapter` implementing `RateLimiterProvider`.
179+
* `PoolboyAdapter` implementing `ConnectionPoolProvider`.
180+
* These adapters would contain the current logic from `CircuitBreaker.ex`, `RateLimiter.ex`, and `ConnectionManager.ex` that interacts directly with the third-party libraries.
181+
182+
3. **Clearer Directory Structure for `Infrastructure`:**
183+
* If these provider behaviours and adapters are introduced, the `elixir_scope/foundation/infrastructure/` directory could be structured like:
184+
```
185+
infrastructure/
186+
infrastructure.ex # The main facade (ElixirScope.Foundation.Infrastructure)
187+
circuit_breaker_provider.ex # Behaviour
188+
rate_limiter_provider.ex # Behaviour
189+
connection_pool_provider.ex # Behaviour
190+
providers/ # Directory for adapters
191+
fuse_adapter.ex
192+
hammer_adapter.ex
193+
poolboy_adapter.ex
194+
# Current circuit_breaker.ex, rate_limiter.ex, connection_manager.ex
195+
# would be refactored into the facade and adapters, or the facade
196+
# might just directly call the providers.
197+
```
198+
199+
**Benefits of These Changes:**
200+
201+
* **Increased Modularity & Testability:** Each component (facade, provider behaviour, specific adapter) can be tested in isolation. Adapters can be easily mocked.
202+
* **True Pluggability:** Allows swapping out underlying libraries (`:fuse`, `Hammer`, `Poolboy`) without changing the `ElixirScope.Foundation.Infrastructure` facade or its consumers.
203+
* **Preparation for Standalone Library:** This structure makes it much cleaner to extract the `Infrastructure` part (or the whole Foundation) into a standalone library in the future, as the contracts are already defined.
204+
205+
This refactoring primarily impacts the `Infrastructure` section, making it more robust and flexible. The other core parts of Foundation (`Config`, `Events`, `Telemetry`) are already well-designed with behaviours.

0 commit comments

Comments
 (0)