From 0fd0c6c729261cd224f80eaab037a174d85e2a24 Mon Sep 17 00:00:00 2001 From: sprooty Date: Mon, 3 Aug 2026 07:35:10 +0000 Subject: [PATCH] T35: stackarr-compat-core skeleton MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Establish the façade rule: compat crates contain DTOs, route wiring and translation **only**. Any logic belongs in core. **Acceptance:** crate exists; rule documented and enforceable at review. Reviewed: not yet — this is a checkpoint taken after the cheap gates passed and before review. harness-item: T35 --- .github/PULL_REQUEST_TEMPLATE.md | 2 + AGENTS.md | 8 +- CONTRIBUTING.md | 4 +- Cargo.lock | 7 + Cargo.toml | 2 + crates/stackarr-compat-core/Cargo.toml | 11 ++ .../stackarr-compat-core/src/facade_rule.rs | 187 ++++++++++++++++++ crates/stackarr-compat-core/src/lib.rs | 35 ++++ .../stackarr-compat-core/tests/facade_rule.rs | 95 +++++++++ docs/API-COMPATIBILITY.md | 3 +- docs/COMPAT-FACADE-RULE.md | 83 ++++++++ docs/CRATE-GUIDE.md | 14 ++ 12 files changed, 446 insertions(+), 5 deletions(-) create mode 100644 crates/stackarr-compat-core/Cargo.toml create mode 100644 crates/stackarr-compat-core/src/facade_rule.rs create mode 100644 crates/stackarr-compat-core/src/lib.rs create mode 100644 crates/stackarr-compat-core/tests/facade_rule.rs create mode 100644 docs/COMPAT-FACADE-RULE.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 8653adff..27cf4820 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -17,6 +17,8 @@ Closes # - [ ] `cargo clippy --workspace --all-features -- -D warnings` passes. - [ ] `cargo test --workspace --all-features` passes. - [ ] Relevant compatibility/conformance fixtures pass. +- [ ] Any `stackarr-compat-*` change is DTOs, route wiring, or translation only; + logic went to the core (docs/COMPAT-FACADE-RULE.md). - [ ] Documentation and checked-in contracts are updated. - [ ] No published crate was vendored and no private dependency source was added. - [ ] No frozen or deferred subsystem was widened. diff --git a/AGENTS.md b/AGENTS.md index 51045073..c9e2de24 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,10 +37,11 @@ The following list is checked against `Cargo.toml` in CI. - `crates/stackarr-cardigann-parity` - `crates/stackarr-stream` - `crates/stackarr-postgres` +- `crates/stackarr-compat-core` -The final entry is renamed to `stackarr-mariadb` during P1. Update this list in -the same commit as any workspace-member change. +`crates/stackarr-postgres` is renamed to `stackarr-mariadb` during P1. Update +this list in the same commit as any workspace-member change. The torrent engine is consumed from crates.io through the `swarmforge` package family and historical `librtbit` dependency aliases. The Usenet engine is the @@ -56,7 +57,8 @@ Until P5 is complete, `stackarr-stream`, Stremio routes, and trending, requests, and watchlist features are deferred. Compatibility crates contain DTOs, route wiring, and translation only. Put -business rules in the core. +business rules in the core. The rule, what it rejects, and how it is checked are +in [docs/COMPAT-FACADE-RULE.md](docs/COMPAT-FACADE-RULE.md). ## Required workflow diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a4bd6922..4fce405d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,7 +29,9 @@ only with executable evidence against the governing specification. - Use property tests for hostile input spaces such as release parsing and custom-format scoring. - Façade crates contain DTOs, route wiring, and translation only. Domain logic - belongs in the core. + belongs in the core. Read + [docs/COMPAT-FACADE-RULE.md](docs/COMPAT-FACADE-RULE.md) before adding to a + `stackarr-compat-*` crate; breaking that rule is grounds for rejection. - A new `#[allow(...)]` must have an adjacent comment that names the concrete reason it is necessary. diff --git a/Cargo.lock b/Cargo.lock index 61433b54..42da8089 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4792,6 +4792,13 @@ dependencies = [ "urlencoding", ] +[[package]] +name = "stackarr-compat-core" +version = "0.1.0" +dependencies = [ + "toml 1.1.4+spec-1.1.0", +] + [[package]] name = "stackarr-core" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 85067f7f..0e75d910 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ members = [ "crates/stackarr-cardigann-parity", "crates/stackarr-stream", "crates/stackarr-postgres", + "crates/stackarr-compat-core", ] [workspace.package] @@ -194,6 +195,7 @@ stackarr-plex = { path = "crates/stackarr-plex" } stackarr-cardigann = { path = "crates/stackarr-cardigann" } stackarr-stream = { path = "crates/stackarr-stream" } stackarr-postgres = { path = "crates/stackarr-postgres" } +stackarr-compat-core = { path = "crates/stackarr-compat-core" } # SwarmForge 0.1.0, published by rustTorrent. Cargo aliases preserve the # existing librtbit API names while the canonical crates.io package names make diff --git a/crates/stackarr-compat-core/Cargo.toml b/crates/stackarr-compat-core/Cargo.toml new file mode 100644 index 00000000..c06bc628 --- /dev/null +++ b/crates/stackarr-compat-core/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "stackarr-compat-core" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dev-dependencies] +toml = { workspace = true } diff --git a/crates/stackarr-compat-core/src/facade_rule.rs b/crates/stackarr-compat-core/src/facade_rule.rs new file mode 100644 index 00000000..cf5191ee --- /dev/null +++ b/crates/stackarr-compat-core/src/facade_rule.rs @@ -0,0 +1,187 @@ +//! The façade rule, expressed as data a test can check. +//! +//! A façade crate — any `stackarr-compat-*` member other than +//! `stackarr-compat-core` — contains DTOs, route wiring and translation only. +//! The full statement of the rule, including the parts a machine cannot judge, +//! is in `docs/COMPAT-FACADE-RULE.md`. +//! +//! What is checked here is the part a machine *can* judge: what a façade is +//! allowed to depend on. A façade that reaches past +//! [`FACADE_WORKSPACE_DEPENDENCIES`] is holding domain knowledge, and a façade +//! that opens a database connection or drives a download engine is holding +//! behavior. Both are the rule being broken in the one place it is visible +//! before the code is written. +//! +//! The lists are deliberately short and deliberately editable. Widening +//! [`FACADE_WORKSPACE_DEPENDENCIES`] is a legitimate move when a shared concern +//! genuinely grows a new home in the core — but it is a one-line diff in this +//! file, which is exactly the point: it surfaces in review instead of arriving +//! buried in a manifest. + +use std::fmt; + +/// Workspace crates a façade may depend on. +/// +/// `stackarr-compat-core` carries the shared arr concerns; `stackarr-core` +/// carries the domain types a DTO translates to and from. A façade that needs +/// anything else needs it through one of these two. +pub const FACADE_WORKSPACE_DEPENDENCIES: &[&str] = &["stackarr-compat-core", "stackarr-core"]; + +/// Dependencies that place storage or an embedded engine inside a façade. +/// +/// These are entry points, not implementation details: a façade holding one of +/// them is talking to a database or an engine directly rather than translating +/// a request for the core to answer. +pub const FACADE_FORBIDDEN_DEPENDENCIES: &[&str] = + &["librtbit", "nzb-web", "nzbdav-core", "rusqlite", "sqlx"]; + +/// Why a dependency breaks the façade rule. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Reason { + /// The façade reaches into a workspace crate it is not allowed to know + /// about, which means it is carrying domain knowledge of its own. + UnlistedWorkspaceCrate, + /// The façade talks to storage or an embedded engine directly, which means + /// it is carrying behavior that belongs in the core. + StorageOrEngine, +} + +impl fmt::Display for Reason { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::UnlistedWorkspaceCrate => formatter + .write_str("a façade may only depend on stackarr-compat-core and stackarr-core"), + Self::StorageOrEngine => { + formatter.write_str("a façade may not reach storage or an embedded engine directly") + } + } + } +} + +/// A single breach of the façade rule. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Violation { + /// The façade crate that declares the dependency. + pub crate_name: String, + /// The dependency that breaks the rule. + pub dependency: String, + /// Why it breaks the rule. + pub reason: Reason, +} + +impl fmt::Display for Violation { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + formatter, + "{} depends on {}: {}", + self.crate_name, self.dependency, self.reason + ) + } +} + +/// Whether `crate_name` names a compatibility façade. +/// +/// `stackarr-compat-core` is not a façade — it is the shared core the façades +/// sit on, and it is the one compat crate that is allowed to hold arr-generic +/// behavior. +#[must_use] +pub fn is_facade_crate(crate_name: &str) -> bool { + crate_name.starts_with("stackarr-compat-") && crate_name != "stackarr-compat-core" +} + +/// Every way `dependencies` breaks the façade rule for `crate_name`. +/// +/// Returns an empty vector for any crate that is not a façade, so this can be +/// applied to a whole workspace without filtering first. +#[must_use] +pub fn violations(crate_name: &str, dependencies: &[&str]) -> Vec { + if !is_facade_crate(crate_name) { + return Vec::new(); + } + + dependencies + .iter() + .filter_map(|dependency| { + let reason = if FACADE_FORBIDDEN_DEPENDENCIES.contains(dependency) { + Reason::StorageOrEngine + } else if dependency.starts_with("stackarr-") + && !FACADE_WORKSPACE_DEPENDENCIES.contains(dependency) + { + Reason::UnlistedWorkspaceCrate + } else { + return None; + }; + + Some(Violation { + crate_name: crate_name.to_owned(), + dependency: (*dependency).to_owned(), + reason, + }) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn facades_are_the_compat_crates_other_than_this_one() { + assert!(is_facade_crate("stackarr-compat-sonarr-v3")); + assert!(is_facade_crate("stackarr-compat-prowlarr-v1")); + assert!(!is_facade_crate("stackarr-compat-core")); + assert!(!is_facade_crate("stackarr-web")); + } + + #[test] + fn dtos_route_wiring_and_translation_are_allowed() { + let dependencies = ["stackarr-compat-core", "stackarr-core", "axum", "serde"]; + assert_eq!( + violations("stackarr-compat-sonarr-v3", &dependencies), + Vec::new() + ); + } + + #[test] + fn a_facade_may_not_reach_another_workspace_crate() { + let violations = violations("stackarr-compat-radarr-v3", &["stackarr-quality"]); + assert_eq!( + violations, + vec![Violation { + crate_name: "stackarr-compat-radarr-v3".to_owned(), + dependency: "stackarr-quality".to_owned(), + reason: Reason::UnlistedWorkspaceCrate, + }] + ); + } + + #[test] + fn a_facade_may_not_reach_storage_or_an_engine() { + let violations = violations("stackarr-compat-sonarr-v3", &["sqlx", "librtbit"]); + let reasons: Vec = violations.iter().map(|entry| entry.reason).collect(); + assert_eq!( + reasons, + vec![Reason::StorageOrEngine, Reason::StorageOrEngine] + ); + } + + #[test] + fn the_rule_does_not_apply_to_non_facade_crates() { + assert_eq!(violations("stackarr-web", &["sqlx"]), Vec::new()); + assert_eq!(violations("stackarr-compat-core", &["sqlx"]), Vec::new()); + } + + #[test] + fn a_violation_reads_as_a_review_comment() { + let violation = Violation { + crate_name: "stackarr-compat-sonarr-v3".to_owned(), + dependency: "sqlx".to_owned(), + reason: Reason::StorageOrEngine, + }; + assert_eq!( + violation.to_string(), + "stackarr-compat-sonarr-v3 depends on sqlx: \ + a façade may not reach storage or an embedded engine directly" + ); + } +} diff --git a/crates/stackarr-compat-core/src/lib.rs b/crates/stackarr-compat-core/src/lib.rs new file mode 100644 index 00000000..57b2c42a --- /dev/null +++ b/crates/stackarr-compat-core/src/lib.rs @@ -0,0 +1,35 @@ +//! Shared foundation for the arr compatibility façades. +//! +//! The Sonarr v3, Radarr v3 and Prowlarr v1 façades are three views of one +//! product. Everything they have in common — authentication, error shapes, +//! `ProviderResource` field reflection and the SignalR hub — belongs here, and +//! everything that decides anything belongs in the core crates behind them. +//! +//! # The façade rule +//! +//! **A compat crate contains DTOs, route wiring and translation only. Any logic +//! that appears in a façade belongs in the core.** +//! +//! That rule is what keeps StackArr one product instead of three. It is +//! documented in full in `docs/COMPAT-FACADE-RULE.md` and expressed as +//! checkable data in [`facade_rule`], which the `facade_rule` integration test +//! applies to every workspace member. +//! +//! # Scope of this crate +//! +//! This crate is a skeleton. It currently carries the façade rule itself; the +//! shared arr concerns land on top of it: +//! +//! - authentication — `X-Api-Key` header, `?apikey=` querystring, forms-auth +//! cookie; +//! - arr error response shapes and status codes; +//! - `ProviderResource` field reflection, preserving option shape, privacy, +//! visibility and ordering; and +//! - the SignalR negotiate handshake and JSON hub protocol. +//! +//! Each arrives with the golden files and generated conformance tests that +//! govern it, never ahead of them. + +#![warn(missing_docs)] + +pub mod facade_rule; diff --git a/crates/stackarr-compat-core/tests/facade_rule.rs b/crates/stackarr-compat-core/tests/facade_rule.rs new file mode 100644 index 00000000..81c529a9 --- /dev/null +++ b/crates/stackarr-compat-core/tests/facade_rule.rs @@ -0,0 +1,95 @@ +//! Applies the façade rule to the workspace as it actually stands. +//! +//! The rule is enforceable at review because review has something to point at: +//! this test fails on the pull request that puts domain knowledge or storage +//! access inside a compatibility façade. + +use std::path::{Path, PathBuf}; + +use stackarr_compat_core::facade_rule::{is_facade_crate, violations}; + +fn workspace_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .ancestors() + .nth(2) + .expect("the crate lives two directories below the workspace root") + .to_path_buf() +} + +fn read_manifest(path: &Path) -> toml::Value { + let contents = std::fs::read_to_string(path) + .unwrap_or_else(|error| panic!("cannot read {}: {error}", path.display())); + toml::from_str(&contents) + .unwrap_or_else(|error| panic!("cannot parse {}: {error}", path.display())) +} + +fn workspace_members(root: &Path) -> Vec { + read_manifest(&root.join("Cargo.toml"))["workspace"]["members"] + .as_array() + .expect("workspace.members is an array") + .iter() + .map(|member| { + member + .as_str() + .expect("a workspace member is a path string") + .to_owned() + }) + .collect() +} + +/// The dependency names a manifest declares, resolving `package = "..."` +/// renames back to the crate actually being pulled in. +fn declared_dependencies(manifest: &toml::Value) -> Vec { + let mut names = Vec::new(); + for table in ["dependencies", "build-dependencies"] { + let Some(entries) = manifest.get(table).and_then(toml::Value::as_table) else { + continue; + }; + for (name, specification) in entries { + let renamed = specification.get("package").and_then(toml::Value::as_str); + names.push(renamed.unwrap_or(name).to_owned()); + } + } + names +} + +#[test] +fn every_facade_crate_obeys_the_facade_rule() { + let root = workspace_root(); + let mut breaches = Vec::new(); + + for member in workspace_members(&root) { + let manifest = read_manifest(&root.join(&member).join("Cargo.toml")); + let name = manifest["package"]["name"] + .as_str() + .expect("a member declares package.name") + .to_owned(); + if !is_facade_crate(&name) { + continue; + } + + let dependencies = declared_dependencies(&manifest); + let borrowed: Vec<&str> = dependencies.iter().map(String::as_str).collect(); + breaches.extend(violations(&name, &borrowed)); + } + + assert!( + breaches.is_empty(), + "the façade rule is broken; move the logic into the core:\n{}", + breaches + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n") + ); +} + +#[test] +fn this_crate_is_a_workspace_member() { + assert!( + workspace_members(&workspace_root()) + .iter() + .any(|member| member == "crates/stackarr-compat-core"), + "the façades have no shared core to sit on" + ); +} diff --git a/docs/API-COMPATIBILITY.md b/docs/API-COMPATIBILITY.md index 4cd4876b..92935c10 100644 --- a/docs/API-COMPATIBILITY.md +++ b/docs/API-COMPATIBILITY.md @@ -2,7 +2,8 @@ StackArr's native `/api/v1` API is independent of the compatibility work and is not a claim of arr compatibility. Compatibility is implemented as additive, -thin façades over the shared core. +thin façades over the shared core. A façade holds DTOs, route wiring, and +translation only; see [COMPAT-FACADE-RULE.md](COMPAT-FACADE-RULE.md). ## Pinned targets diff --git a/docs/COMPAT-FACADE-RULE.md b/docs/COMPAT-FACADE-RULE.md new file mode 100644 index 00000000..97273604 --- /dev/null +++ b/docs/COMPAT-FACADE-RULE.md @@ -0,0 +1,83 @@ +# The façade rule + +> A compat crate contains DTOs, route wiring and translation **only**. Any logic +> that appears in a façade belongs in the core. + +This is the difference between one product and three. Sonarr v3, Radarr v3 and +Prowlarr v1 compatibility are three views of the same StackArr domain. The +moment a façade decides something for itself, that decision exists in one view +and not the others, and the three views start drifting apart — silently, one +endpoint at a time. + +## The crates + +| Crate | Role | +| --- | --- | +| `stackarr-compat-core` | Shared arr concerns: authentication, error shapes, `ProviderResource` field reflection, the SignalR hub. | +| `stackarr-compat-sonarr-v3` | Thin façade. Not yet created. | +| `stackarr-compat-radarr-v3` | Thin façade. Not yet created. | +| `stackarr-compat-prowlarr-v1` | Thin façade. Not yet created. | + +A *façade* is any `stackarr-compat-*` crate other than `stackarr-compat-core`. +`stackarr-compat-core` is the one compat crate allowed to hold behavior, and +only behavior that is arr-generic rather than domain-specific: how a request is +authenticated, how an error is shaped, how a hub message is framed. + +## What belongs in a façade + +- Serde types matching the arr wire contract, field for field, including + casing, nullability and ordering. +- Route registration, path prefixes, per-façade API keys and instance identity. +- Translation between those wire types and StackArr domain types. +- Tests that pin the wire contract: golden-file comparisons and generated + conformance tests. + +## What does not + +- Filtering, sorting, ranking, scoring, matching or eligibility decisions. +- Quality, custom-format, profile, naming or release-decision behavior. +- Database access, migrations, transactions or caching policy. +- Talking to an indexer, download client or metadata provider. +- Defaults that a client will read as data. If a value has to be *chosen* + rather than *converted*, the core chooses it and the façade reports it. + +A translation may be tedious — an enum with twenty arms, a shape that needs +flattening — without being logic. The test is whether the same request against a +different façade would need the same decision made again. If it would, it +belongs in the core. + +## How it is enforced + +Three layers, deliberately overlapping: + +1. **Review.** The pull request template carries the rule as a checkbox, and it + is a legitimate, expected reason to reject a change. +2. **A test.** `stackarr_compat_core::facade_rule` holds the mechanical part of + the rule as data, and the crate's `facade_rule` integration test applies it to + every workspace member on every `cargo test` run. A façade may depend on + `stackarr-compat-core` and `stackarr-core`; anything else in the + `stackarr-*` family fails the build, as does a direct dependency on storage + (`sqlx`, `rusqlite`) or an embedded engine (`librtbit`, `nzb-web`, + `nzbdav-core`). +3. **Conformance.** Façade behavior is pinned to recorded arr responses, so + logic invented inside a façade shows up as a wire-shape diff. + +The test catches the boundary being crossed in a manifest, which is where it is +cheapest to catch. It cannot catch a `match` arm that quietly invents a rule — +that is what layers 1 and 3 are for. + +## Changing the allowlist + +`FACADE_WORKSPACE_DEPENDENCIES` in +`crates/stackarr-compat-core/src/facade_rule.rs` is short and editable on +purpose. Widening it is sometimes correct: a shared concern may genuinely grow a +new home in the core. It is a one-line diff in a file whose only job is this +rule, so it arrives in review as a visible decision rather than buried in a +manifest. + +Before widening it, check the alternative: the thing the façade wants usually +belongs behind `stackarr-core`, or is a shared arr concern that belongs in +`stackarr-compat-core` where all three façades get it at once. + +See [UNIFIED-ARR-PLAN.md](UNIFIED-ARR-PLAN.md) §5 for the target architecture +and [API-COMPATIBILITY.md](API-COMPATIBILITY.md) for the pinned wire contracts. diff --git a/docs/CRATE-GUIDE.md b/docs/CRATE-GUIDE.md index eaafaef9..b2db19fd 100644 --- a/docs/CRATE-GUIDE.md +++ b/docs/CRATE-GUIDE.md @@ -360,6 +360,20 @@ Every crate in the workspace, what it does, and how to use it. --- +### stackarr-compat-core + +**Purpose**: Shared foundation for the arr compatibility façades (Sonarr v3, Radarr v3, Prowlarr v1). Skeleton — it currently carries the façade rule; auth, error shapes, `ProviderResource` field reflection, and the SignalR hub land here as their conformance tests do. + +**Key exports**: +- `facade_rule::violations()` / `is_facade_crate()` — the façade rule as checkable data +- `facade_rule::FACADE_WORKSPACE_DEPENDENCIES` / `FACADE_FORBIDDEN_DEPENDENCIES` — what a façade may and may not depend on + +**Rule**: a compat crate contains DTOs, route wiring, and translation only; logic belongs in the core. The `facade_rule` integration test applies the dependency half of that to every workspace member. See [COMPAT-FACADE-RULE.md](COMPAT-FACADE-RULE.md). + +**Dependencies**: none (toml as a dev-dependency for the workspace check) + +--- + ### stackarr-bootstrap **Purpose**: Standalone discovery node for remote server-client pairing, server name resolution, and unified invite/claim code management. Runs as a separate binary.