Skip to content

Commit ce27599

Browse files
lxsaahclaude
andauthored
refactor(core)!: split AnyRecord into capability traits (036 W2) (#142)
* refactor(core)!: split AnyRecord into capability traits (036 W2) AnyRecord carried ~22 methods spanning four concerns; every remote-access or profiling change churned the core storage contract. Split by consumer: - AnyRecord (6 methods): storage/lifecycle only — validate, as_any(_mut), drain_config_errors, set_writable_erased, plus the cfg-gated json_access() accessor so the remote-access gate lives in one place. - RecordIntrospect (supertrait): graph/metadata introspection consumed by the builder's dependency-graph construction, route collection, and list_records. - JsonRecordAccess (cfg remote-access): latest_json / subscribe_json / set_from_json, reached via json_access(); the runtime .with_remote_access() checks stay inside the methods, so behavior is unchanged. Gate is `remote-access` (the 036 sketch said json-serialize). - RecordMetricsReset (supertrait): the cfg-gated no-op-default resets. Supertrait wiring keeps every dyn AnyRecord call site compiling unchanged; only the four JSON call sites switch to the accessor. Registry storage stays Vec<Box<dyn AnyRecord>>. Drops the dead outbound_connector_urls (cfg std) — zero callers in-tree and in aimdb-pro. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(design): mark 036 W2 implemented in PR #142 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent f275f88 commit ce27599

6 files changed

Lines changed: 145 additions & 73 deletions

File tree

aimdb-core/src/builder.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ impl AimDbInner {
232232
#[cfg(feature = "remote-access")]
233233
pub fn try_latest_as_json(&self, record_key: &str) -> Option<serde_json::Value> {
234234
let id = self.resolve_str(record_key)?;
235-
self.storages.get(id.index())?.latest_json()
235+
self.storages.get(id.index())?.json_access()?.latest_json()
236236
}
237237

238238
/// Sets a record value from JSON (remote access API)
@@ -260,7 +260,14 @@ impl AimDbInner {
260260
.resolve_str(record_key)
261261
.ok_or_else(|| DbError::record_key_not_found(record_key.to_string()))?;
262262

263-
self.storages[id.index()].set_from_json(json_value)
263+
self.storages[id.index()]
264+
.json_access()
265+
.ok_or_else(|| {
266+
DbError::runtime_error(alloc::format!(
267+
"Record '{record_key}' does not support JSON remote access"
268+
))
269+
})?
270+
.set_from_json(json_value)
264271
}
265272
}
266273

aimdb-core/src/lib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,11 @@ pub use typed_api::{
6666
Consumer, InboundConnectorBuilder, OutboundConnectorBuilder, Producer, RecordRegistrar,
6767
RecordT, StageKind,
6868
};
69-
pub use typed_record::{AnyRecord, AnyRecordExt, TypedRecord};
69+
#[cfg(feature = "remote-access")]
70+
pub use typed_record::JsonRecordAccess;
71+
pub use typed_record::{
72+
AnyRecord, AnyRecordExt, RecordIntrospect, RecordMetricsReset, TypedRecord,
73+
};
7074

7175
// JSON codec (feature `json-serialize`, no_std + alloc compatible)
7276
#[cfg(feature = "json-serialize")]

aimdb-core/src/remote/stream.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,14 @@ pub(crate) fn stream_record_updates(
3737
let record = inner
3838
.storage(id)
3939
.ok_or(DbError::InvalidRecordId { id: id.raw() })?;
40-
let reader = record.subscribe_json()?;
40+
let reader = record
41+
.json_access()
42+
.ok_or_else(|| {
43+
DbError::runtime_error(alloc::format!(
44+
"Record '{record_key}' does not support JSON remote access"
45+
))
46+
})?
47+
.subscribe_json()?;
4148

4249
// Pair the reader with an owned copy of the record key so lag/error
4350
// logs identify which record fell behind — the previous mpsc-based

aimdb-core/src/session/aimx/dispatch.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,15 @@ impl AimxSession {
229229
let record = self.db.inner().storage(id).ok_or(RpcError::NotFound)?;
230230
// `subscribe_json` fails if the record was not configured with
231231
// `.with_remote_access()`.
232-
let reader = record.subscribe_json().map_err(map_db_err)?;
232+
let reader = record
233+
.json_access()
234+
.ok_or_else(|| {
235+
map_db_err(DbError::runtime_error(alloc::format!(
236+
"Record '{name}' does not support JSON remote access"
237+
)))
238+
})?
239+
.subscribe_json()
240+
.map_err(map_db_err)?;
233241
self.drain_readers.insert(name.to_string(), reader);
234242
}
235243

aimdb-core/src/typed_record.rs

Lines changed: 112 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,22 @@ type ConsumerServiceFn<T> =
184184
type ProducerServiceFn<T> =
185185
Box<dyn FnOnce(crate::RuntimeContext, crate::Producer<T>) -> BoxFuture<'static, ()> + Send>;
186186

187-
/// Type-erased trait for records
187+
/// Type-erased trait for records — the storage and lifecycle contract.
188188
///
189189
/// Allows storage of heterogeneous record types in a single collection
190190
/// while maintaining type safety through downcast operations.
191191
///
192+
/// Since the 036 W2 split this trait carries only the storage/lifecycle
193+
/// surface. The other capabilities live in dedicated traits, reachable from
194+
/// any `dyn AnyRecord`:
195+
/// - [`RecordIntrospect`] (supertrait) — graph/metadata introspection
196+
/// - [`RecordMetricsReset`] (supertrait) — profiling/metrics counter resets
197+
/// - [`JsonRecordAccess`] — JSON remote access, via [`AnyRecord::json_access`]
198+
///
199+
/// Consumers: `AimDbBuilder::build()` (validation, config-error draining,
200+
/// typed downcasts via [`AnyRecordExt`]) and connectors applying the
201+
/// remote-access security policy (`set_writable_erased`).
202+
///
192203
/// # Thread Safety Requirements
193204
///
194205
/// This trait requires both `Send` and `Sync` because:
@@ -207,7 +218,7 @@ type ProducerServiceFn<T> =
207218
/// **Migration:** If your record type `T` is not `Sync`, wrap non-`Sync` fields
208219
/// in `Arc<Mutex<_>>` or `Arc<RwLock<_>>` to achieve interior mutability with
209220
/// thread-safe sharing.
210-
pub trait AnyRecord: Send + Sync {
221+
pub trait AnyRecord: RecordIntrospect + RecordMetricsReset + Send + Sync {
211222
/// Validates that the record has correct producer/consumer setup
212223
///
213224
/// Rules: Must have exactly one producer and at least one consumer.
@@ -219,18 +230,53 @@ pub trait AnyRecord: Send + Sync {
219230
/// Returns self as mutable Any for downcasting
220231
fn as_any_mut(&mut self) -> &mut dyn Any;
221232

233+
/// Drains the configuration mistakes recorded during registration.
234+
///
235+
/// Called by `AimDbBuilder::build()`, which fills in the record key and
236+
/// reports every finding via
237+
/// [`DbError::InvalidConfiguration`](crate::DbError::InvalidConfiguration).
238+
fn drain_config_errors(&mut self) -> Vec<crate::error::ConfigError>;
239+
240+
/// Sets the writable flag for this record (type-erased)
241+
///
242+
/// Used internally by the builder to apply security policy to records.
243+
fn set_writable_erased(&self, writable: bool);
244+
245+
/// Returns the record's JSON remote-access surface, if it has one.
246+
///
247+
/// This accessor is the single place the `remote-access` cfg-gate lives
248+
/// for consumers: they query the capability here instead of cfg-gating
249+
/// every call site. `TypedRecord` always returns `Some`; the runtime
250+
/// "configured with `.with_remote_access()`" checks stay inside the
251+
/// [`JsonRecordAccess`] methods.
252+
#[cfg(feature = "remote-access")]
253+
fn json_access(&self) -> Option<&dyn JsonRecordAccess> {
254+
None
255+
}
256+
}
257+
258+
/// Graph and metadata introspection for type-erased records.
259+
///
260+
/// Supertrait of [`AnyRecord`], so every stored record exposes it and a
261+
/// `dyn AnyRecord` can be upcast to `&dyn RecordIntrospect` where only
262+
/// introspection is needed.
263+
///
264+
/// Consumers: `AimDbBuilder::build()` (link validation and the dependency
265+
/// graph fed to [`crate::graph`]), the builder's inbound/outbound route
266+
/// collection, and `AimDbInner::list_records` (remote introspection
267+
/// metadata).
268+
pub trait RecordIntrospect {
222269
/// Returns the number of registered outbound connectors
223270
fn outbound_connector_count(&self) -> usize;
224271

225-
/// Returns the outbound connector URLs as strings
226-
#[cfg(feature = "std")]
227-
fn outbound_connector_urls(&self) -> Vec<String>;
228-
229272
/// Gets the outbound connector links
230273
///
231274
/// Returns outbound connector configuration list for spawning logic.
232275
fn outbound_connectors(&self) -> &[crate::connector::ConnectorLink];
233276

277+
/// Get the inbound connector links for this record
278+
fn inbound_connectors(&self) -> &[crate::connector::InboundConnectorLink];
279+
234280
/// Returns the number of registered consumers (tap observers)
235281
fn consumer_count(&self) -> usize;
236282

@@ -240,13 +286,6 @@ pub trait AnyRecord: Send + Sync {
240286
/// Returns whether a buffer is configured
241287
fn has_buffer(&self) -> bool;
242288

243-
/// Drains the configuration mistakes recorded during registration.
244-
///
245-
/// Called by `AimDbBuilder::build()`, which fills in the record key and
246-
/// reports every finding via
247-
/// [`DbError::InvalidConfiguration`](crate::DbError::InvalidConfiguration).
248-
fn drain_config_errors(&mut self) -> Vec<crate::error::ConfigError>;
249-
250289
/// Returns whether a transform is registered for this record
251290
fn has_transform(&self) -> bool;
252291

@@ -261,11 +300,6 @@ pub trait AnyRecord: Send + Sync {
261300
/// Returns the transform input keys (if a transform is registered)
262301
fn transform_input_keys(&self) -> Option<Vec<String>>;
263302

264-
/// Sets the writable flag for this record (type-erased)
265-
///
266-
/// Used internally by the builder to apply security policy to records.
267-
fn set_writable_erased(&self, writable: bool);
268-
269303
/// Collects metadata for this record
270304
#[cfg(feature = "remote-access")]
271305
fn collect_metadata(
@@ -274,12 +308,23 @@ pub trait AnyRecord: Send + Sync {
274308
key: crate::record_id::StringKey,
275309
id: crate::record_id::RecordId,
276310
) -> crate::remote::RecordMetadata;
311+
}
277312

278-
/// Internal: Returns JSON for type-erased remote access
313+
/// Type-erased JSON read/subscribe/write for remote access.
314+
///
315+
/// Internal to the remote-access protocol — application code reads values
316+
/// via `record.latest()?.as_json()` instead. Obtained from a record through
317+
/// [`AnyRecord::json_access`], which is where the `remote-access` cfg-gate
318+
/// lives for consumers.
319+
///
320+
/// Consumers: `AimDbInner::try_latest_as_json` / `set_record_from_json`
321+
/// (`record.get` / `record.set`), the AimX session dispatch (`record.subscribe`
322+
/// value drain), and `remote::stream::stream_record_updates`.
323+
#[cfg(feature = "remote-access")]
324+
pub trait JsonRecordAccess {
325+
/// Returns JSON for type-erased remote access
279326
///
280327
/// Used internally by remote access protocol. **Users should use `record.latest()?.as_json()`.**
281-
#[doc(hidden)]
282-
#[cfg(feature = "remote-access")]
283328
fn latest_json(&self) -> Option<serde_json::Value>;
284329

285330
/// Subscribe to record updates as JSON stream
@@ -301,16 +346,13 @@ pub trait AnyRecord: Send + Sync {
301346
///
302347
/// # Example (internal use)
303348
/// ```rust,ignore
304-
/// let type_id = TypeId::of::<Temperature>();
305-
/// let record: &Box<dyn AnyRecord> = db.records.get(&type_id)?;
306-
/// let mut json_reader = record.subscribe_json()?;
349+
/// let record: &Box<dyn AnyRecord> = db.storage(id)?;
350+
/// let mut json_reader = record.json_access().unwrap().subscribe_json()?;
307351
///
308352
/// while let Ok(json_val) = json_reader.recv_json().await {
309353
/// // Forward to remote client...
310354
/// }
311355
/// ```
312-
#[doc(hidden)]
313-
#[cfg(feature = "remote-access")]
314356
fn subscribe_json(&self) -> crate::DbResult<Box<dyn crate::buffer::JsonBufferReader + Send>>;
315357

316358
/// Sets a record value from JSON
@@ -339,18 +381,24 @@ pub trait AnyRecord: Send + Sync {
339381
///
340382
/// # Example (internal use)
341383
/// ```rust,ignore
342-
/// let type_id = TypeId::of::<AppConfig>();
343-
/// let record: &Box<dyn AnyRecord> = db.records.get(&type_id)?;
384+
/// let record: &Box<dyn AnyRecord> = db.storage(id)?;
344385
/// let json_val = serde_json::json!({"log_level": "debug"});
345-
/// record.set_from_json(json_val)?; // Only works if producer_count == 0
386+
/// // Only works if producer_count == 0
387+
/// record.json_access().unwrap().set_from_json(json_val)?;
346388
/// ```
347-
#[doc(hidden)]
348-
#[cfg(feature = "remote-access")]
349389
fn set_from_json(&self, json_value: serde_json::Value) -> crate::DbResult<()>;
390+
}
350391

351-
/// Get the inbound connector links for this record
352-
fn inbound_connectors(&self) -> &[crate::connector::InboundConnectorLink];
353-
392+
/// Observability counter resets (features `profiling` / `metrics`).
393+
///
394+
/// Supertrait of [`AnyRecord`] with no-op defaults, so the cfg-gated reset
395+
/// methods stay off the core storage contract while remaining callable on
396+
/// every stored record.
397+
///
398+
/// Consumers: `AimDb::reset_profiling` / `AimDb::reset_buffer_metrics`,
399+
/// driven by the AimX `control.reset_buffer_metrics` RPC and the MCP
400+
/// buffer-metrics tool.
401+
pub trait RecordMetricsReset {
354402
/// Resets this record's stage profiling counters (feature `profiling`).
355403
///
356404
/// Default implementation is a no-op; `TypedRecord` overrides it.
@@ -1134,16 +1182,31 @@ impl<T: Send + Sync + 'static + Debug + Clone> AnyRecord for TypedRecord<T> {
11341182
self
11351183
}
11361184

1137-
fn outbound_connector_count(&self) -> usize {
1138-
self.outbound_connectors.len()
1185+
fn drain_config_errors(&mut self) -> Vec<crate::error::ConfigError> {
1186+
core::mem::take(&mut self.config_errors)
11391187
}
11401188

1141-
#[cfg(feature = "std")]
1142-
fn outbound_connector_urls(&self) -> Vec<String> {
1143-
self.outbound_connectors
1144-
.iter()
1145-
.map(|link| format!("{}", link.url))
1146-
.collect()
1189+
fn set_writable_erased(&self, writable: bool) {
1190+
#[cfg(feature = "remote-access")]
1191+
{
1192+
self.writable
1193+
.store(writable, portable_atomic::Ordering::SeqCst);
1194+
}
1195+
#[cfg(not(feature = "remote-access"))]
1196+
{
1197+
let _ = writable; // Suppress unused warning
1198+
}
1199+
}
1200+
1201+
#[cfg(feature = "remote-access")]
1202+
fn json_access(&self) -> Option<&dyn JsonRecordAccess> {
1203+
Some(self)
1204+
}
1205+
}
1206+
1207+
impl<T: Send + Sync + 'static + Debug + Clone> RecordIntrospect for TypedRecord<T> {
1208+
fn outbound_connector_count(&self) -> usize {
1209+
self.outbound_connectors.len()
11471210
}
11481211

11491212
fn outbound_connectors(&self) -> &[crate::connector::ConnectorLink] {
@@ -1162,10 +1225,6 @@ impl<T: Send + Sync + 'static + Debug + Clone> AnyRecord for TypedRecord<T> {
11621225
TypedRecord::has_buffer(self)
11631226
}
11641227

1165-
fn drain_config_errors(&mut self) -> Vec<crate::error::ConfigError> {
1166-
core::mem::take(&mut self.config_errors)
1167-
}
1168-
11691228
fn has_transform(&self) -> bool {
11701229
TypedRecord::has_transform(self)
11711230
}
@@ -1182,16 +1241,8 @@ impl<T: Send + Sync + 'static + Debug + Clone> AnyRecord for TypedRecord<T> {
11821241
TypedRecord::transform_input_keys(self)
11831242
}
11841243

1185-
fn set_writable_erased(&self, writable: bool) {
1186-
#[cfg(feature = "remote-access")]
1187-
{
1188-
self.writable
1189-
.store(writable, portable_atomic::Ordering::SeqCst);
1190-
}
1191-
#[cfg(not(feature = "remote-access"))]
1192-
{
1193-
let _ = writable; // Suppress unused warning
1194-
}
1244+
fn inbound_connectors(&self) -> &[crate::connector::InboundConnectorLink] {
1245+
&self.inbound_connectors
11951246
}
11961247

11971248
#[cfg(feature = "remote-access")]
@@ -1247,9 +1298,10 @@ impl<T: Send + Sync + 'static + Debug + Clone> AnyRecord for TypedRecord<T> {
12471298

12481299
metadata
12491300
}
1301+
}
12501302

1251-
#[doc(hidden)]
1252-
#[cfg(feature = "remote-access")]
1303+
#[cfg(feature = "remote-access")]
1304+
impl<T: Send + Sync + 'static + Debug + Clone> JsonRecordAccess for TypedRecord<T> {
12531305
fn latest_json(&self) -> Option<serde_json::Value> {
12541306
log_debug!(
12551307
"latest_json called for type: {}",
@@ -1266,8 +1318,6 @@ impl<T: Send + Sync + 'static + Debug + Clone> AnyRecord for TypedRecord<T> {
12661318
result
12671319
}
12681320

1269-
#[doc(hidden)]
1270-
#[cfg(feature = "remote-access")]
12711321
fn subscribe_json(&self) -> crate::DbResult<Box<dyn crate::buffer::JsonBufferReader + Send>> {
12721322
use crate::DbError;
12731323

@@ -1302,8 +1352,6 @@ impl<T: Send + Sync + 'static + Debug + Clone> AnyRecord for TypedRecord<T> {
13021352
Ok(Box::new(json_reader))
13031353
}
13041354

1305-
#[doc(hidden)]
1306-
#[cfg(feature = "remote-access")]
13071355
fn set_from_json(&self, json_value: serde_json::Value) -> crate::DbResult<()> {
13081356
use crate::DbError;
13091357

@@ -1370,11 +1418,9 @@ impl<T: Send + Sync + 'static + Debug + Clone> AnyRecord for TypedRecord<T> {
13701418

13711419
Ok(())
13721420
}
1421+
}
13731422

1374-
fn inbound_connectors(&self) -> &[crate::connector::InboundConnectorLink] {
1375-
&self.inbound_connectors
1376-
}
1377-
1423+
impl<T: Send + Sync + 'static + Debug + Clone> RecordMetricsReset for TypedRecord<T> {
13781424
#[cfg(feature = "profiling")]
13791425
fn reset_profiling(&self) {
13801426
self.profiling.reset_all();

docs/design/036-followup-refactoring.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ The registry keeps storing `Box<dyn AnyRecord>`; consumers upcast to the capabil
6262

6363
**Payoff:** the core storage contract stops churning every time remote access or profiling evolves, and each consumer's dependency is visible in its signature. **Acceptance:** `AnyRecord`~8 methods; no behavior change; rustdoc for each trait states its consumer.
6464

65-
**Size:** M. Mechanical but wide. **File together with W1** (it touches the same files; do W2 first or in the same series — W2 shrinks the surface W1 has to move).
65+
**Size:** M. Mechanical but wide. **File together with W1** (it touches the same files; do W2 first or in the same series — W2 shrinks the surface W1 has to move). **Status:** implemented in PR [#142](https://github.com/aimdb-dev/aimdb/pull/142), stacked on #141 (W1 had already landed, so the "W2 first" ordering note was moot). Deviations from the sketch: the JSON trait's gate is `remote-access` — the actual gate on those methods — not `json-serialize`; the resets live on a `RecordMetricsReset` supertrait (default no-ops, so the supertrait list needs no cfg); the dead `outbound_connector_urls` (cfg `std`, zero callers in-tree and in aimdb-pro) was dropped rather than moved. Result: `AnyRecord` has 6 methods.
6666

6767
### W3 — Execute the KNX hardware validation matrix (035 §3)
6868

@@ -141,7 +141,7 @@ Both protocols now ride the session engine (the hard part), but two subscribe/wr
141141
| Item | Issue | When to file |
142142
|---|---|---|
143143
| W1 data-plane de-`Any` | PR [#141](https://github.com/aimdb-dev/aimdb/pull/141) | done — no separate issue, direct PR |
144-
| W2 `AnyRecord` split | | on #140 merge (same series as W1) |
144+
| W2 `AnyRecord` split | PR [#142](https://github.com/aimdb-dev/aimdb/pull/142) | done — stacked on #141, no separate issue |
145145
| W3 hardware matrix || none if run with #140; else a validation task |
146146
| W4 ACK-retransmit knob || on #140 merge |
147147
| W5 `StringKey` interner || opportunistic; file if not done by next release |

0 commit comments

Comments
 (0)