diff --git a/CONVENTIONS.md b/CONVENTIONS.md index 8420a65c3..9c5def84d 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -133,10 +133,11 @@ Any files modified by steps 1–2 (e.g., `openapi.yaml`, JSON schema files) must ## Endpoint Patterns -- Endpoints are registered in a `configure()` function that takes `ServiceConfig`, `Database`, and config params +- Endpoints are registered in a `configure()` function that takes `ServiceConfig`, the `ReadOnly` and/or `ReadWrite` connection types, and config params - Services are injected via `web::Data` (Actix application data) - Authorization uses `Require` extractor or `authorizer.require(&user, Permission::...)` call -- Read operations acquire a read transaction: `let tx = db.begin_read().await?;` +- Read operations use the `ReadOnly` connection: `let tx = db.begin().await?;` +- Write operations use the `ReadWrite` connection and its `transaction()` method - List endpoints accept `Query` (search/filter), `Paginated` (pagination), and return `PaginatedResults` - Every endpoint has a `#[utoipa::path(...)]` attribute for OpenAPI documentation with `tag`, `operation_id`, `params`, and `responses` - Route attributes use Actix macros: `#[get("/v3/...")]`, `#[post("/v3/...")]`, `#[delete("/v3/...")]` diff --git a/common/src/config.rs b/common/src/config.rs index 97d134f9c..48dd3c81d 100644 --- a/common/src/config.rs +++ b/common/src/config.rs @@ -31,6 +31,20 @@ const ENV_DB_MAX_LIFETIME: &str = "TRUSTD_DB_MAX_LIFETIME"; const ENV_DB_IDLE_TIMEOUT: &str = "TRUSTD_DB_IDLE_TIMEOUT"; const ENV_DB_SSLMODE: &str = "TRUSTD_DB_SSLMODE"; +const ENV_DB_RO_URL: &str = "TRUSTD_DB_RO_URL"; +const ENV_DB_RO_NAME: &str = "TRUSTD_DB_RO_NAME"; +const ENV_DB_RO_USER: &str = "TRUSTD_DB_RO_USER"; +const ENV_DB_RO_PASS: &str = "TRUSTD_DB_RO_PASSWORD"; +const ENV_DB_RO_HOST: &str = "TRUSTD_DB_RO_HOST"; +const ENV_DB_RO_PORT: &str = "TRUSTD_DB_RO_PORT"; +const ENV_DB_RO_MAX_CONN: &str = "TRUSTD_DB_RO_MAX_CONN"; +const ENV_DB_RO_MIN_CONN: &str = "TRUSTD_DB_RO_MIN_CONN"; +const ENV_DB_RO_CONNECT_TIMEOUT: &str = "TRUSTD_DB_RO_CONNECT_TIMEOUT"; +const ENV_DB_RO_ACQUIRE_TIMEOUT: &str = "TRUSTD_DB_RO_ACQUIRE_TIMEOUT"; +const ENV_DB_RO_MAX_LIFETIME: &str = "TRUSTD_DB_RO_MAX_LIFETIME"; +const ENV_DB_RO_IDLE_TIMEOUT: &str = "TRUSTD_DB_RO_IDLE_TIMEOUT"; +const ENV_DB_RO_SSLMODE: &str = "TRUSTD_DB_RO_SSLMODE"; + /// PostgreSQL SSL mode #[derive(Copy, Clone, Debug, Default, clap::ValueEnum, Eq, PartialEq, strum::Display)] #[strum(serialize_all = "kebab-case")] @@ -164,6 +178,70 @@ impl Database { } } +/// Read-only database options, mirroring `Database` with all fields optional. +/// +/// When a field is not set, the corresponding value from the R/W `Database` config is used. +/// If no R/O fields are set at all, the R/W connection is reused for reads. +#[derive(clap::Parser, Debug, Clone, Default, Eq, PartialEq)] +#[command(next_help_heading = "Read-Only Database")] +#[group(id = "database-ro")] +pub struct DatabaseReadOnly { + /// A complete URL for the read-only database. + #[arg(id = "db-ro-url", long, env = ENV_DB_RO_URL)] + pub url: Option, + #[arg(id = "db-ro-user", long, env = ENV_DB_RO_USER)] + pub username: Option, + #[arg(id = "db-ro-password", long, env = ENV_DB_RO_PASS)] + pub password: Option>, + #[arg(id = "db-ro-host", long, env = ENV_DB_RO_HOST)] + pub host: Option, + #[arg(id = "db-ro-port", long, env = ENV_DB_RO_PORT)] + pub port: Option, + #[arg(id = "db-ro-name", long, env = ENV_DB_RO_NAME)] + pub name: Option, + #[arg(id = "db-ro-max-conn", long, env = ENV_DB_RO_MAX_CONN)] + pub max_conn: Option, + #[arg(id = "db-ro-min-conn", long, env = ENV_DB_RO_MIN_CONN)] + pub min_conn: Option, + #[arg(id = "db-ro-sslmode", long, env = ENV_DB_RO_SSLMODE, value_enum)] + pub sslmode: Option, + #[arg(id = "db-ro-conn-timeout", long, env = ENV_DB_RO_CONNECT_TIMEOUT)] + pub connect_timeout: Option, + #[arg(id = "db-ro-acquire-timeout", long, env = ENV_DB_RO_ACQUIRE_TIMEOUT)] + pub acquire_timeout: Option, + #[arg(id = "db-ro-max-lifetime", long, env = ENV_DB_RO_MAX_LIFETIME)] + pub max_lifetime: Option, + #[arg(id = "db-ro-idle-timeout", long, env = ENV_DB_RO_IDLE_TIMEOUT)] + pub idle_timeout: Option, +} + +impl DatabaseReadOnly { + /// Builds a `Database` config by overlaying R/O values on top of the R/W fallback. + pub fn to_database_config(&self, fallback: &Database) -> Database { + Database { + url: self.url.clone().or_else(|| fallback.url.clone()), + username: self + .username + .clone() + .unwrap_or_else(|| fallback.username.clone()), + password: self + .password + .clone() + .unwrap_or_else(|| fallback.password.clone()), + host: self.host.clone().unwrap_or_else(|| fallback.host.clone()), + port: self.port.unwrap_or(fallback.port), + name: self.name.clone().unwrap_or_else(|| fallback.name.clone()), + max_conn: self.max_conn.unwrap_or(fallback.max_conn), + min_conn: self.min_conn.unwrap_or(fallback.min_conn), + sslmode: self.sslmode.unwrap_or(fallback.sslmode), + connect_timeout: self.connect_timeout.unwrap_or(fallback.connect_timeout), + acquire_timeout: self.acquire_timeout.unwrap_or(fallback.acquire_timeout), + max_lifetime: self.max_lifetime.unwrap_or(fallback.max_lifetime), + idle_timeout: self.idle_timeout.unwrap_or(fallback.idle_timeout), + } + } +} + #[cfg(test)] mod test { use super::*; @@ -223,4 +301,103 @@ mod test { "postgres://postgres:trustify@localhost:5432/trustify?sslmode=disable" ); } + + /// Helper to create a default R/W config for use in R/O fallback tests. + fn rw_default() -> Database { + Database { + url: None, + username: DB_USER.into(), + password: DB_PASS.into(), + host: DB_HOST.into(), + port: DB_PORT, + name: DB_NAME.into(), + max_conn: DB_MAX_CONN, + min_conn: DB_MIN_CONN, + connect_timeout: DB_CONNECT_TIMEOUT, + acquire_timeout: DB_ACQUIRE_TIMEOUT, + max_lifetime: DB_MAX_LIFETIME, + idle_timeout: DB_IDLE_TIMEOUT, + sslmode: SslMode::default(), + } + } + + /// Verify that an unconfigured R/O config falls back to the R/W config entirely. + #[test] + fn ro_fallback_uses_rw_config() { + // given: a default R/W config and an empty R/O config + let rw = rw_default(); + let ro = DatabaseReadOnly::default(); + + // when: building the R/O database config + let result = ro.to_database_config(&rw); + + // then: the result is identical to the R/W config + assert_eq!(result, rw); + } + + /// Verify that individual R/O fields override the R/W fallback while inheriting the rest. + #[test] + fn ro_overrides_host_and_port() { + // given: a default R/W config and an R/O config with host and port set + let rw = rw_default(); + let ro = DatabaseReadOnly { + host: Some("replica.example.com".into()), + port: Some(5433), + ..Default::default() + }; + + // when: building the R/O database config + let result = ro.to_database_config(&rw); + + // then: host and port are overridden, everything else falls back to R/W + assert_eq!(result.host, "replica.example.com"); + assert_eq!(result.port, 5433); + assert_eq!(result.username, rw.username); + assert_eq!(result.password, rw.password); + assert_eq!(result.name, rw.name); + } + + /// Verify that the R/O URL takes precedence over the R/W URL. + #[test] + fn ro_url_overrides_rw_url() { + // given: both R/W and R/O specify a URL + let rw = Database { + url: Some("postgres://primary:5432/trustify".into()), + ..rw_default() + }; + let ro = DatabaseReadOnly { + url: Some("postgres://replica:5433/trustify".into()), + ..Default::default() + }; + + // when: building the R/O database config + let result = ro.to_database_config(&rw); + + // then: the R/O URL wins + assert_eq!( + result.url.as_deref(), + Some("postgres://replica:5433/trustify") + ); + } + + /// Verify that R/O credentials override R/W credentials independently. + #[test] + fn ro_separate_credentials() { + // given: an R/O config that only overrides username and password + let rw = rw_default(); + let ro = DatabaseReadOnly { + username: Some("readonly_user".into()), + password: Some("readonly_pass".into()), + ..Default::default() + }; + + // when: building the R/O database config + let result = ro.to_database_config(&rw); + + // then: credentials come from R/O, connection target falls back to R/W + assert_eq!(result.username, "readonly_user"); + assert_eq!(result.password.0, "readonly_pass"); + assert_eq!(result.host, rw.host); + assert_eq!(result.port, rw.port); + } } diff --git a/common/src/db/mod.rs b/common/src/db/mod.rs index 8f8c6f557..42032a11a 100644 --- a/common/src/db/mod.rs +++ b/common/src/db/mod.rs @@ -10,6 +10,7 @@ mod func; pub use create::*; pub use func::*; +use actix_web::{HttpResponse, ResponseError}; use anyhow::Context; use reqwest::Url; use sea_orm::{ @@ -341,6 +342,295 @@ impl<'a> IntoSchemaManagerConnection<'a> for &'a Database { } } +/// Read-write database connection wrapper. +/// +/// Provides full database access including read-write transactions. +/// Used by endpoints and services that perform mutations (ingestion, imports, deletes). +#[derive(Clone, Debug)] +pub struct ReadWrite(Database); + +impl ReadWrite { + /// Wraps an existing database connection for read-write access. + pub fn new(db: Database) -> Self { + Self(db) + } + + /// Close the connection. + pub async fn close(self) -> anyhow::Result<()> { + self.0.close().await + } + + /// Runs a closure inside a transaction, committing on success and rolling back on error. + pub async fn transaction(&self, f: F) -> Result + where + F: AsyncFnOnce(&DatabaseTransaction) -> Result, + E: From + Display, + { + self.0.transaction(f).await + } + + /// Runs a closure inside a transaction with the given isolation level and access mode. + pub async fn transaction_with_config( + &self, + isolation_level: Option, + access_mode: Option, + f: F, + ) -> Result + where + F: AsyncFnOnce(&DatabaseTransaction) -> Result, + E: From + Display, + { + self.0 + .transaction_with_config(isolation_level, access_mode, f) + .await + } + + /// Extracts the inner Database, consuming this wrapper. + pub fn into_inner(self) -> Database { + self.0 + } +} + +impl Deref for ReadWrite { + type Target = Database; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +#[async_trait::async_trait] +impl ConnectionTrait for ReadWrite { + fn get_database_backend(&self) -> DbBackend { + self.0.get_database_backend() + } + + async fn execute(&self, stmt: Statement) -> Result { + self.0.execute(stmt).await + } + + async fn execute_unprepared(&self, sql: &str) -> Result { + self.0.execute_unprepared(sql).await + } + + async fn query_one(&self, stmt: Statement) -> Result, DbErr> { + self.0.query_one(stmt).await + } + + async fn query_all(&self, stmt: Statement) -> Result, DbErr> { + self.0.query_all(stmt).await + } + + fn support_returning(&self) -> bool { + self.0.support_returning() + } +} + +#[async_trait::async_trait] +impl ConnectionTrait for &ReadWrite { + fn get_database_backend(&self) -> DbBackend { + self.0.get_database_backend() + } + + async fn execute(&self, stmt: Statement) -> Result { + self.0.execute(stmt).await + } + + async fn execute_unprepared(&self, sql: &str) -> Result { + self.0.execute_unprepared(sql).await + } + + async fn query_one(&self, stmt: Statement) -> Result, DbErr> { + self.0.query_one(stmt).await + } + + async fn query_all(&self, stmt: Statement) -> Result, DbErr> { + self.0.query_all(stmt).await + } + + fn support_returning(&self) -> bool { + self.0.support_returning() + } +} + +#[async_trait::async_trait] +impl StreamTrait for ReadWrite { + type Stream<'a> = ::Stream<'a>; + + fn stream<'a>( + &'a self, + stmt: Statement, + ) -> Pin, DbErr>> + 'a + Send>> { + self.0.stream(stmt) + } +} + +#[async_trait::async_trait] +impl<'b> StreamTrait for &'b ReadWrite { + type Stream<'a> + = ::Stream<'a> + where + 'b: 'a; + + fn stream<'a>( + &'a self, + stmt: Statement, + ) -> Pin, DbErr>> + 'a + Send>> { + self.0.stream(stmt) + } +} + +impl<'a> IntoSchemaManagerConnection<'a> for &'a ReadWrite { + fn into_schema_manager_connection(self) -> SchemaManagerConnection<'a> { + (&self.0).into_schema_manager_connection() + } +} + +#[async_trait::async_trait] +impl TransactionTrait for ReadWrite { + async fn begin(&self) -> Result { + self.0.begin().await + } + + async fn begin_with_config( + &self, + isolation_level: Option, + access_mode: Option, + ) -> Result { + self.0.begin_with_config(isolation_level, access_mode).await + } + + async fn transaction(&self, callback: F) -> Result> + where + F: for<'c> FnOnce( + &'c DatabaseTransaction, + ) -> Pin> + Send + 'c>> + + Send, + T: Send, + E: std::fmt::Display + std::fmt::Debug + Send, + { + TransactionTrait::transaction(&self.0, callback).await + } + + async fn transaction_with_config( + &self, + callback: F, + isolation_level: Option, + access_mode: Option, + ) -> Result> + where + F: for<'c> FnOnce( + &'c DatabaseTransaction, + ) -> Pin> + Send + 'c>> + + Send, + T: Send, + E: std::fmt::Display + std::fmt::Debug + Send, + { + TransactionTrait::transaction_with_config(&self.0, callback, isolation_level, access_mode) + .await + } +} + +/// Error returned by `ReadOnly::begin()`, compatible with both actix handlers and module error types. +#[derive(Debug, thiserror::Error)] +pub enum DbError { + #[error(transparent)] + Database(DbErr), + #[error("unavailable")] + Unavailable, + #[error("cannot open a read-write transaction on a read-only connection")] + ReadOnly, +} + +impl From for DbError { + fn from(err: DbErr) -> Self { + if err.is_read_only() { + Self::Unavailable + } else { + Self::Database(err) + } + } +} + +impl ResponseError for DbError { + fn error_response(&self) -> HttpResponse { + match self { + Self::Unavailable => HttpResponse::ServiceUnavailable() + .json(crate::error::ErrorInformation::new("Unavailable", self)), + Self::ReadOnly => HttpResponse::Forbidden() + .json(crate::error::ErrorInformation::new("ReadOnly", self)), + Self::Database(err) => { + log::warn!("{err}"); + HttpResponse::InternalServerError() + .json(crate::error::ErrorInformation::new("Database", "")) + } + } + } +} + +/// Read-only database connection factory. +/// +/// Does not implement `ConnectionTrait` directly — callers must use `begin()` to obtain +/// a `DatabaseTransaction` opened with `AccessMode::ReadOnly`. All operations then go +/// through that transaction, which PostgreSQL enforces as read-only. +#[derive(Clone, Debug)] +pub struct ReadOnly(Database); + +impl ReadOnly { + /// Wraps an existing database connection for read-only access. + pub fn new(db: Database) -> Self { + Self(db) + } + + /// Get the name of the database. + pub fn name(&self) -> &str { + self.0.name() + } + + /// Ping the database for health checks. + pub async fn ping(&self) -> anyhow::Result<()> { + self.0.ping().await + } + + /// Close the connection. + pub async fn close(self) -> anyhow::Result<()> { + self.0.close().await + } + + /// Begins a read-only transaction. + pub async fn begin(&self) -> Result { + Ok(self + .0 + .begin_with_config(None, Some(AccessMode::ReadOnly)) + .await?) + } + + /// Begins a read-only transaction with the given isolation level. + /// + /// The access mode is always forced to `ReadOnly`; passing `ReadWrite` returns an error. + pub async fn begin_with_config( + &self, + isolation_level: Option, + access_mode: Option, + ) -> Result { + let mode = Self::validate_access_mode(access_mode)?; + Ok(self.0.begin_with_config(isolation_level, mode).await?) + } + + /// Validates the access mode, rejecting explicit read-write requests. + fn validate_access_mode(mode: Option) -> Result, DbError> { + match mode { + Some(AccessMode::ReadWrite) => Err(DbError::ReadOnly), + _ => Ok(Some(AccessMode::ReadOnly)), + } + } + + /// Extracts the inner Database, consuming this wrapper. + pub fn into_inner(self) -> Database { + self.0 + } +} + /// Remove the password from the URL and replace it with `***`, if present. /// /// If this is not a URL, or does not contain a password, this is a no-op. @@ -387,4 +677,22 @@ mod test { fn url_strip_password_not_a_url() { assert_eq!("foo-bar-baz", strip_password("foo-bar-baz".to_string())) } + + #[test] + fn read_only_rejects_explicit_read_write_mode() { + let result = ReadOnly::validate_access_mode(Some(AccessMode::ReadWrite)); + assert!( + matches!(result, Err(DbError::ReadOnly)), + "explicit ReadWrite must be rejected" + ); + } + + #[test] + fn read_only_allows_none_and_read_only_mode() { + let result = ReadOnly::validate_access_mode(None); + assert_eq!(result.unwrap(), Some(AccessMode::ReadOnly)); + + let result = ReadOnly::validate_access_mode(Some(AccessMode::ReadOnly)); + assert_eq!(result.unwrap(), Some(AccessMode::ReadOnly)); + } } diff --git a/docs/adrs/00018-dual-database-connections.md b/docs/adrs/00018-dual-database-connections.md new file mode 100644 index 000000000..2161bebca --- /dev/null +++ b/docs/adrs/00018-dual-database-connections.md @@ -0,0 +1,211 @@ +# 00018. Dual database connections (R/W + R/O) + +Date: 2026-05-08 + +## Status + +APPROVED + +## Context + +Trustify uses a single `db::Database` struct — a thin wrapper around SeaORM's `DatabaseConnection` — for +all database operations. Every module receives the same connection via `configure()` functions in the +server startup path. This means every query, whether it is an expensive full-text search or a small +metadata write, competes for connections from the same pool against the same PostgreSQL instance. + +As the number of Trustify pods scales, all instances share the same database primary. A single PostgreSQL +instance becomes the bottleneck — not because writes are heavy, but because read traffic (searches, SBOM +lookups, vulnerability queries, analysis) vastly outweighs write traffic (ingestion, imports) and +saturates the connection pool. Adding more pods only increases contention on the same database. + +PostgreSQL supports streaming replication with read replicas that can serve read-only queries +independently of the primary. Routing read traffic to replicas allows horizontal scaling of the database +layer alongside the application layer. However, PostgreSQL itself does not route queries — the +application must decide which connection to use. This requires application-level database routing: the +ability to maintain two separate connections and direct each query to the appropriate target. + +ADR 00016 introduced a read-only mode flag (`--read-only` / `TRUSTD_READ_ONLY`) that gates mutating HTTP +requests at the middleware level. That decision addressed the application-level request routing but did not +address the database connection topology. This ADR builds on that foundation. + +## Decision + +### Newtype wrappers with actix extraction + +Two newtype wrappers are introduced in `common::db`: + +```rust +/// Connection to the read-write primary. Full SeaORM capabilities. +pub struct ReadWrite(Database); + +/// Connection to a read-only replica (or the primary in fallback mode). +/// Implements ConnectionTrait by delegation. Its TransactionTrait +/// implementation always opens BEGIN TRANSACTION READ ONLY. +pub struct ReadOnly(Database); +``` + +Both types implement SeaORM's `ConnectionTrait` by delegating to the inner `Database`. The key difference +is the `TransactionTrait` implementation: + +* `ReadWrite` opens regular read-write transactions (current behavior). +* `ReadOnly` always opens transactions with `BEGIN TRANSACTION READ ONLY`. PostgreSQL rejects any + INSERT, UPDATE, or DELETE within a read-only transaction, and a read-only transaction cannot be + escalated to read-write once started. This provides hard enforcement at the database protocol level. + +Both types are registered as `web::Data` and `web::Data` in the actix application +state. Endpoint handlers declare which connection they need by extracting the appropriate type: + +```rust +async fn list_advisories( + db: web::Data, + // ... +) -> Result { + // db is guaranteed to be a read-only connection +} + +async fn ingest_sbom( + db: web::Data, + // ... +) -> Result { + // db is a full read-write connection +} +``` + +This makes the intent explicit in the handler signature and lets the Rust compiler and actix's extractor +system verify that the required connection type is available at startup. No runtime routing, middleware +introspection, or HTTP-method-based switching is needed. + +### Read-only transaction enforcement + +The `ReadOnly` wrapper's `TransactionTrait` implementation always creates read-only transactions. This +enforcement works regardless of the underlying connection target: + +* **Single database (fallback):** Both `ReadWrite` and `ReadOnly` point at the same PostgreSQL instance. + Writes through `ReadOnly` still fail because the read-only transaction rejects them. +* **Primary + replica:** `ReadOnly` points at a streaming replica. Writes fail both because of the + read-only transaction and because the replica itself rejects writes. Defense in depth. + +This means that even in development or simple deployments with a single database, code that should only +read cannot accidentally write — the read-only transaction prevents it. + +### Configuration + +A new set of CLI arguments and environment variables is added for the read-only connection, mirroring +the existing `--db-*` / `TRUSTD_DB_*` parameters: + +| Purpose | R/W (existing) | R/O (new) | +|---------|---------------|-----------| +| URL | `--db-url` / `TRUSTD_DB_URL` | `--db-ro-url` / `TRUSTD_DB_RO_URL` | +| Host | `--db-host` / `TRUSTD_DB_HOST` | `--db-ro-host` / `TRUSTD_DB_RO_HOST` | +| Port | `--db-port` / `TRUSTD_DB_PORT` | `--db-ro-port` / `TRUSTD_DB_RO_PORT` | +| User | `--db-user` / `TRUSTD_DB_USER` | `--db-ro-user` / `TRUSTD_DB_RO_USER` | +| Password | `--db-password` / `TRUSTD_DB_PASSWORD` | `--db-ro-password` / `TRUSTD_DB_RO_PASSWORD` | +| Name | `--db-name` / `TRUSTD_DB_NAME` | `--db-ro-name` / `TRUSTD_DB_RO_NAME` | +| Max connections | `--db-max-conn` / `TRUSTD_DB_MAX_CONN` | `--db-ro-max-conn` / `TRUSTD_DB_RO_MAX_CONN` | +| Min connections | `--db-min-conn` / `TRUSTD_DB_MIN_CONN` | `--db-ro-min-conn` / `TRUSTD_DB_RO_MIN_CONN` | +| SSL mode | `--db-sslmode` / `TRUSTD_DB_SSLMODE` | `--db-ro-sslmode` / `TRUSTD_DB_RO_SSLMODE` | + +The R/O connection pool has its own independent pool sizing. In a read-heavy deployment, operators may +want a larger R/O pool than the R/W pool. + +In `common::config`, this is represented as an optional second `Database` struct: + +```rust +/// Read-only database options. If not set, the R/W connection is used for reads. +#[command(flatten)] +pub database_ro: Option, +``` + +### Fallback behavior + +All R/O parameters are optional. When none are provided, the `ReadOnly` wrapper is constructed from a +clone of the R/W `Database` connection. This ensures full backward compatibility — existing deployments +with a single database work without any configuration changes. + +The read-only transaction enforcement still applies in fallback mode, so the safety guarantee is +maintained regardless of deployment topology. + +### Composition with read-only mode (ADR 00016) + +The two features compose naturally. The R/W connection (`--db-*`) is always required; the R/O +connection (`--db-ro-*`) can be omitted, in which case it falls back to the R/W connection: + +| Scenario | `--db-*` (R/W) | `--db-ro-*` (R/O) | `--read-only` | Effect | +|---|---|---|---|---| +| Single DB, full mode | Primary | *(omit)* | `false` | R/O falls back to primary | +| Single DB, read-only | Primary | *(omit)* | `true` | R/O falls back to primary; writes rejected by middleware | +| Primary + replica, full mode | Primary | Replica | `false` | Reads go to replica, writes to primary | +| Primary + replica, read-only | Primary | Replica | `true` | Reads from replica; writes rejected by middleware | + +When `--read-only` is active (ADR 00016), mutating HTTP requests are rejected by middleware before they +reach any handler. The R/W connection is still configured and available, but no write operations will be +performed because the middleware prevents mutating requests from reaching any handler. + +### Migrations + +Database migrations only run against the R/W connection. Read-only replicas receive schema changes +through PostgreSQL's streaming replication. The migration tooling (`trustd db migrate`) uses the +existing `--db-*` parameters and is unaffected by this change. + +### Service-level changes + +Each module's `configure()` function signature changes to accept the connection type it needs: + +* **Read-only modules** (fundamental queries, analysis, user preferences reads) receive `ReadOnly`. +* **Read-write modules** (ingestor, importer) receive `ReadWrite`. +* **Mixed modules** (e.g. fundamental, which has both query and mutation endpoints) receive both. + +The `server/src/profile/api.rs` `configure()` function constructs both wrapper types and passes them +to each module accordingly. + +## Alternatives considered + +### Middleware-based routing by HTTP method + +All GET/HEAD requests automatically use the R/O connection; POST/PUT/PATCH/DELETE use R/W. This avoids +changing handler signatures but has drawbacks: + +* Some GET endpoints may need read-after-write consistency (e.g. a GET immediately after an ingest). + These would silently read stale data from a replica. +* No compile-time visibility into which connection a handler uses. +* Couples connection routing to HTTP semantics rather than business logic. + +**Why not chosen:** The type-based approach is more explicit, catches misuse at compile time via actix's +extractor system, and allows handlers to opt into the correct connection based on their actual data access +pattern rather than HTTP method conventions. + +### Single connection with read-only transaction wrapper + +Instead of two separate connection pools, use a single pool and wrap read-only operations in read-only +transactions. This avoids the configuration complexity of a second connection. + +**Why not chosen:** The primary motivation is offloading read traffic to replicas, which requires a +separate connection pointing at a different host. A single pool cannot target multiple PostgreSQL +instances. The read-only transaction is used as an enforcement mechanism, not as a routing mechanism. + +### Connection routing inside the Database struct + +Add a `Database::read()` / `Database::write()` method that returns the appropriate inner connection. +Callers must remember to call the right method. + +**Why not chosen:** This pushes the routing decision into every call site as a runtime choice, with no +compile-time enforcement. Forgetting to call `.read()` would silently use the wrong connection. The +newtype approach makes the choice visible in the function signature and leverages actix's dependency +injection to verify availability at startup. + +## Consequences + +* Two new types `ReadWrite` and `ReadOnly` are added to `common::db`. Both implement `ConnectionTrait`. + `ReadOnly`'s `TransactionTrait` always opens read-only transactions, preventing accidental writes even + in single-database deployments. +* A new set of `--db-ro-*` / `TRUSTD_DB_RO_*` configuration parameters is added. All are optional — + existing deployments are unaffected. +* Module `configure()` function signatures change to accept `ReadOnly`, `ReadWrite`, or both instead of + the current `Database`. This is an internal API change; the public HTTP API is unaffected. +* Endpoint handlers are updated to extract `web::Data` or `web::Data`. This makes + the data access pattern of each handler self-documenting. +* When `--read-only` is active (ADR 00016), the R/W connection can be omitted. The two features compose + without conflict. +* Operators can independently size the R/W and R/O connection pools. Read-heavy deployments benefit from + a larger R/O pool backed by multiple replicas. +* Migrations remain on the R/W connection only. No changes to migration tooling. diff --git a/modules/analysis/src/endpoints/mod.rs b/modules/analysis/src/endpoints/mod.rs index 62dee068e..34596d222 100644 --- a/modules/analysis/src/endpoints/mod.rs +++ b/modules/analysis/src/endpoints/mod.rs @@ -19,12 +19,12 @@ use trustify_auth::{ utoipa::AuthResponse, }; use trustify_common::{ - db::{Database, query::Query}, + db::{self, query::Query}, model::{Paginated, PaginatedResults}, }; use utoipa_actix_web::service_config::ServiceConfig; -pub fn configure(config: &mut ServiceConfig, db: Database, analysis: AnalysisService) { +pub fn configure(config: &mut ServiceConfig, db: db::ReadOnly, analysis: AnalysisService) { config .app_data(web::Data::new(analysis)) .app_data(web::Data::new(db)) @@ -57,14 +57,15 @@ struct StatusQuery { /// Get the status of the analysis service. pub async fn analysis_status( service: web::Data, - db: web::Data, + db: web::Data, user: UserInformation, authorizer: web::Data, web::Query(StatusQuery { details }): web::Query, _: Require, ) -> actix_web::Result { authorizer.require(&user, Permission::ReadSystemInformation)?; - Ok(HttpResponse::Ok().json(service.status(db.as_ref(), details).await?)) + let tx = db.begin().await?; + Ok(HttpResponse::Ok().json(service.status(&tx, details).await?)) } #[utoipa::path( @@ -84,19 +85,16 @@ pub async fn analysis_status( /// Retrieve SBOM components (packages) by name, Package URL, or CPE. pub async fn get_component( service: web::Data, - db: web::Data, + db: web::Data, key: web::Path, web::Query(options): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { let query = OwnedComponentReference::try_from(key.as_str())?; + let tx = db.begin().await?; - Ok(HttpResponse::Ok().json( - service - .retrieve(&query, options, paginated, db.as_ref()) - .await?, - )) + Ok(HttpResponse::Ok().json(service.retrieve(&query, options, paginated, &tx).await?)) } #[utoipa::path( @@ -116,17 +114,14 @@ pub async fn get_component( /// Retrieve SBOM components (packages) by a complex search. pub async fn search_component( service: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(options): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { - Ok(HttpResponse::Ok().json( - service - .retrieve(&search, options, paginated, db.as_ref()) - .await?, - )) + let tx = db.begin().await?; + Ok(HttpResponse::Ok().json(service.retrieve(&search, options, paginated, &tx).await?)) } #[utoipa::path( @@ -147,7 +142,7 @@ pub async fn search_component( /// Render an SBOM graph pub async fn render_sbom_graph( service: web::Data, - db: web::Data, + db: web::Data, path: web::Path<(String, String)>, _: Require, ) -> actix_web::Result { @@ -158,8 +153,9 @@ pub async fn render_sbom_graph( }; let sbom = parse_sbom_id(&sbom)?; + let tx = db.begin().await?; - let graph = service.load_graph(db.as_ref(), sbom).await?; + let graph = service.load_graph(&tx, sbom).await?; if let Some((data, content_type)) = service.render(graph.as_ref(), ext) { Ok(HttpResponse::Ok().content_type(content_type).body(data)) @@ -185,15 +181,16 @@ pub async fn render_sbom_graph( /// Retrieve latest SBOM components (packages) by a complex search. pub async fn search_latest_component( service: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(options): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { + let tx = db.begin().await?; Ok(HttpResponse::Ok().json( service - .retrieve_latest(&search, options, paginated, db.as_ref()) + .retrieve_latest(&search, options, paginated, &tx) .await?, )) } @@ -215,17 +212,18 @@ pub async fn search_latest_component( /// Retrieve latest SBOM components (packages) by name, Package URL, or CPE. pub async fn get_latest_component( service: web::Data, - db: web::Data, + db: web::Data, key: web::Path, web::Query(options): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { let query = OwnedComponentReference::try_from(key.as_str())?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json( service - .retrieve_latest(&query, options, paginated, db.as_ref()) + .retrieve_latest(&query, options, paginated, &tx) .await?, )) } diff --git a/modules/analysis/src/error.rs b/modules/analysis/src/error.rs index ad238543b..28eea507b 100644 --- a/modules/analysis/src/error.rs +++ b/modules/analysis/src/error.rs @@ -4,7 +4,7 @@ use actix_web::{HttpResponse, ResponseError}; use cpe::uri::OwnedUri; use sea_orm::DbErr; use std::str::FromStr; -use trustify_common::db::DatabaseErrors; +use trustify_common::db::{DatabaseErrors, DbError}; use trustify_common::error::ErrorInformation; use trustify_common::id::IdError; use trustify_common::purl::PurlErr; @@ -51,6 +51,16 @@ impl From for Error { } } +impl From for Error { + fn from(value: DbError) -> Self { + match value { + DbError::Database(err) => Self::Database(err), + DbError::Unavailable => Self::Unavailable, + DbError::ReadOnly => Self::Internal(value.to_string()), + } + } +} + impl ResponseError for Error { fn error_response(&self) -> HttpResponse { match self { diff --git a/modules/analysis/src/service/mod.rs b/modules/analysis/src/service/mod.rs index 5fc876f7d..019f0316d 100644 --- a/modules/analysis/src/service/mod.rs +++ b/modules/analysis/src/service/mod.rs @@ -46,7 +46,10 @@ use tokio::{ }; use tracing::instrument; use trustify_common::{ - db::query::{Value, ValueContext}, + db::{ + ReadOnly, + query::{Value, ValueContext}, + }, model::{PaginatedResults, Pagination}, }; use trustify_entity::{ @@ -294,10 +297,7 @@ impl AnalysisService { /// /// Also, we do not implement default because of this. As a new instance has the implication /// of having its own cache. So creating a new instance should be a deliberate choice. - pub fn new(config: AnalysisConfig, connection: C) -> Self - where - C: ConnectionTrait + Send + 'static, - { + pub fn new(config: AnalysisConfig, connection: ReadOnly) -> Self { let meter = global::meter("AnalysisService"); let graph_cache = Arc::new(GraphMap::new( @@ -358,26 +358,35 @@ impl AnalysisService { } } - async fn loader( + /// Background task that loads graphs into cache using a read-only connection. + async fn loader( mut rx: mpsc::UnboundedReceiver, service: InnerService, - connection: C, - ) where - C: ConnectionTrait + Send + 'static, - { + connection: ReadOnly, + ) { let mut next = vec![]; while rx.recv_many(&mut next, 8).await != 0 { let (ids, txs): (Vec<_>, Vec<_>) = next.drain(..).map(|entry| (entry.id, entry.tx)).unzip(); - match service.load_graphs(&connection, ids).await { - Ok(r) => { - log::info!("Loaded {} graphs", r.len()); - } + let tx_result = match connection.begin().await { + Ok(tx) => match service.load_graphs(&tx, ids).await { + Ok(r) => { + log::info!("Loaded {} graphs", r.len()); + Ok(()) + } + Err(err) => { + log::warn!("Failed to load graphs into cache: {err}"); + Err(()) + } + }, Err(err) => { - log::warn!("Failed to load graphs into cache: {err}"); + log::warn!("Failed to begin read-only transaction: {err}"); + Err(()) } - } + }; + + let _ = tx_result; // notify listeners if they are interested for tx in txs { diff --git a/modules/analysis/src/service/test/mod.rs b/modules/analysis/src/service/test/mod.rs index 84f4ee6df..badf463f7 100644 --- a/modules/analysis/src/service/test/mod.rs +++ b/modules/analysis/src/service/test/mod.rs @@ -26,7 +26,7 @@ async fn test_simple_analysis_service(ctx: &TrustifyContext) -> Result<(), anyho ctx.ingest_documents(["spdx/simple.json", "spdx/simple.json"]) .await?; //double ingestion intended - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -86,7 +86,7 @@ async fn test_simple_analysis_cyclonedx_service( ctx.ingest_documents(["cyclonedx/simple.json", "cyclonedx/simple.json"]) .await?; //double ingestion intended - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -144,7 +144,7 @@ async fn test_simple_analysis_cyclonedx_service( async fn test_simple_by_name_analysis_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -193,7 +193,7 @@ async fn simple_by_name_analysis_service_filter_rel( ) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -234,7 +234,7 @@ async fn simple_by_name_analysis_service_filter_rel( async fn test_simple_by_purl_analysis_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let component_purl: Purl = Purl::from_str("pkg:rpm/redhat/B@0.0.0").map_err(Error::Purl)?; @@ -286,7 +286,7 @@ async fn test_quarkus_analysis_service(ctx: &TrustifyContext) -> Result<(), anyh ]) .await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -335,7 +335,7 @@ async fn test_quarkus_analysis_service(ctx: &TrustifyContext) -> Result<(), anyh async fn test_status_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let all_graphs = service.load_all_graphs(&ctx.db).await?; assert_eq!(all_graphs.len(), 1); @@ -368,7 +368,7 @@ async fn test_status_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> async fn test_cache_size_used(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); assert_eq!(service.cache_size_used(), 0u64); let all_graphs = service.load_all_graphs(&ctx.db).await?; @@ -396,7 +396,7 @@ async fn test_cache_size_used(ctx: &TrustifyContext) -> Result<(), anyhow::Error max_cache_size: BinaryByteSize::from(small_sbom_size * 2), ..Default::default() }, - ctx.db.clone(), + ReadOnly::new(ctx.db.clone()), ); let all_graphs = service.load_all_graphs(&ctx.db).await?; @@ -415,7 +415,7 @@ async fn test_cache_size_used(ctx: &TrustifyContext) -> Result<(), anyhow::Error async fn test_simple_deps_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -455,7 +455,7 @@ async fn test_simple_deps_service(ctx: &TrustifyContext) -> Result<(), anyhow::E async fn test_simple_deps_cyclonedx_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["cyclonedx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -491,7 +491,7 @@ async fn test_simple_deps_cyclonedx_service(ctx: &TrustifyContext) -> Result<(), async fn test_simple_by_name_deps_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -525,7 +525,7 @@ async fn test_simple_by_name_deps_service(ctx: &TrustifyContext) -> Result<(), a async fn test_simple_by_purl_deps_service(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ctx.ingest_documents(["spdx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let component_purl: Purl = Purl::from_str("pkg:rpm/redhat/AA@0.0.0?arch=src").map_err(Error::Purl)?; @@ -561,7 +561,7 @@ async fn test_quarkus_deps_service(ctx: &TrustifyContext) -> Result<(), anyhow:: ]) .await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -586,7 +586,7 @@ async fn test_retrieve_all_sbom_roots_by_name(ctx: &TrustifyContext) -> Result<( ctx.ingest_documents(["spdx/quarkus-bom-3.2.11.Final-redhat-00001.json"]) .await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let component_name = "quarkus-vertx-http".to_string(); let analysis_graph = service @@ -658,7 +658,7 @@ async fn load_performance(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { log::info!("Start populating graph"); let start = SystemTime::now(); - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); service.load_all_graphs(&ctx.db).await?; log::info!( @@ -687,7 +687,7 @@ async fn load_performance_parallel(ctx: &TrustifyContext) -> Result<(), anyhow:: log::info!("Start populating graph"); let start = SystemTime::now(); - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let mut tasks = vec![]; for _ in 0..10 { diff --git a/modules/analysis/src/service/test/query.rs b/modules/analysis/src/service/test/query.rs index 9bd5983c5..906e4efae 100644 --- a/modules/analysis/src/service/test/query.rs +++ b/modules/analysis/src/service/test/query.rs @@ -5,7 +5,7 @@ use crate::{ use rstest::rstest; use std::collections::HashSet; use test_context::test_context; -use trustify_common::db::query::Query; +use trustify_common::db::{ReadOnly, query::Query}; use trustify_test_context::{TrustifyContext, q::escape_q}; /// Ensure that the DB logic and the in-memory logic are aligned @@ -33,7 +33,7 @@ async fn alignment( ) -> Result<(), anyhow::Error> { ctx.ingest_documents(["cyclonedx/simple.json"]).await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let q = Query { q, diff --git a/modules/analysis/src/service/test/recursive.rs b/modules/analysis/src/service/test/recursive.rs index 88d6daf54..9548b76e3 100644 --- a/modules/analysis/src/service/test/recursive.rs +++ b/modules/analysis/src/service/test/recursive.rs @@ -7,7 +7,7 @@ use crate::{ }; use rstest::rstest; use test_context::test_context; -use trustify_common::model::Paginated; +use trustify_common::{db::ReadOnly, model::Paginated}; use trustify_test_context::{IngestionResult, TrustifyContext}; #[test_context(TrustifyContext)] @@ -23,7 +23,7 @@ async fn test_circular_deps_cyclonedx_service_count( ctx.ingest_documents(["cyclonedx/cyclonedx-circular.json"]) .await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -67,7 +67,7 @@ async fn test_circular_deps_cyclonedx_service( .await? .into_id(); - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( @@ -116,7 +116,7 @@ async fn test_circular_deps_spdx_service( ) -> Result<(), anyhow::Error> { let [sbom] = ctx.ingest_documents(["spdx/loop.json"]).await?.into_id(); - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let analysis_graph = service .retrieve( diff --git a/modules/analysis/src/test/mod.rs b/modules/analysis/src/test/mod.rs index 6ee5f5867..2632d2f37 100644 --- a/modules/analysis/src/test/mod.rs +++ b/modules/analysis/src/test/mod.rs @@ -6,6 +6,7 @@ use crate::{ model::{BaseSummary, Node as GraphNode}, service::AnalysisService, }; +use trustify_common::db; use trustify_entity::relationship::Relationship; use trustify_test_context::{ TrustifyContext, @@ -13,8 +14,9 @@ use trustify_test_context::{ }; pub async fn caller(ctx: &TrustifyContext) -> anyhow::Result { - let analysis = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); - call::caller(|svc| configure(svc, ctx.db.clone(), analysis)).await + let db_ro = db::ReadOnly::new(ctx.db.clone()); + let analysis = AnalysisService::new(AnalysisConfig::default(), db_ro.clone()); + call::caller(|svc| configure(svc, db_ro, analysis)).await } #[derive(PartialEq, Eq, Debug, Copy, Clone)] diff --git a/modules/fundamental/src/advisory/endpoints/label.rs b/modules/fundamental/src/advisory/endpoints/label.rs index cecf0ef42..5a3f93689 100644 --- a/modules/fundamental/src/advisory/endpoints/label.rs +++ b/modules/fundamental/src/advisory/endpoints/label.rs @@ -1,15 +1,16 @@ use crate::{ + Error, advisory::service::AdvisoryService, common::{service::DocumentType, service::fetch_labels}, - db::DatabaseExt, }; use actix_web::{HttpResponse, Responder, get, patch, put, web}; +use sea_orm::TransactionTrait; use trustify_auth::{ Permission, UpdateAdvisory, authenticator::user::UserInformation, authorizer::{Authorizer, Require}, }; -use trustify_common::{db::Database, id::Id}; +use trustify_common::{db, id::Id}; use trustify_entity::labels::{Labels, Update}; use utoipa::IntoParams; @@ -41,14 +42,14 @@ mod default { #[get("/v3/advisory-labels")] /// List all unique key/value labels from all Advisories pub async fn all( - db: web::Data, + db: web::Data, web::Query(query): web::Query, authorizer: web::Data, user: UserInformation, ) -> actix_web::Result { authorizer.require(&user, Permission::ReadAdvisory)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let result = fetch_labels(DocumentType::Advisory, query.filter_text, query.limit, &tx).await?; Ok(HttpResponse::Ok().json(result)) @@ -70,7 +71,7 @@ pub async fn all( #[put("/v3/advisory/{id}/label")] pub async fn set( advisory: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Json(labels): web::Json, _: Require, @@ -102,17 +103,19 @@ pub async fn set( #[patch("/v3/advisory/{id}/label")] pub async fn update( advisory: web::Data, + db: web::Data, id: web::Path, web::Json(update): web::Json, _: Require, -) -> actix_web::Result { - Ok( - match advisory - .update_labels(id.into_inner(), |labels| update.apply_to(labels)) - .await? - { - Some(()) => HttpResponse::NoContent(), - None => HttpResponse::NotFound(), - }, - ) +) -> Result { + let tx = db.begin().await?; + let result = advisory + .update_labels(id.into_inner(), |labels| update.apply_to(labels), &tx) + .await?; + tx.commit().await?; + + Ok(match result { + Some(()) => HttpResponse::NoContent(), + None => HttpResponse::NotFound(), + }) } diff --git a/modules/fundamental/src/advisory/endpoints/mod.rs b/modules/fundamental/src/advisory/endpoints/mod.rs index 656e369da..b4c7592eb 100644 --- a/modules/fundamental/src/advisory/endpoints/mod.rs +++ b/modules/fundamental/src/advisory/endpoints/mod.rs @@ -10,7 +10,6 @@ use crate::{ service::AdvisoryService, }, common::service::delete_doc, - db::DatabaseExt, endpoints::Deprecation, }; use actix_web::{HttpResponse, Responder, delete, get, http::header, post, web}; @@ -21,7 +20,7 @@ use std::str::FromStr; use time::OffsetDateTime; use trustify_auth::{CreateAdvisory, DeleteAdvisory, ReadAdvisory, authorizer::Require}; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, decompress::decompress_async, id::Id, model::{BinaryData, Paginated, PaginatedResults}, @@ -36,14 +35,16 @@ use uuid::Uuid; pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db_rw: db::ReadWrite, + db_ro: db::ReadOnly, upload_limit: usize, cache: PaginationCache, ) { - let advisory_service = AdvisoryService::new(db.clone(), cache); + let advisory_service = AdvisoryService::new(cache); config - .app_data(web::Data::new(db)) + .app_data(web::Data::new(db_rw)) + .app_data(web::Data::new(db_ro)) .app_data(web::Data::new(advisory_service)) .app_data(web::Data::new(Config { upload_limit })) .service(all) @@ -89,13 +90,13 @@ struct AdvisoryQuery { /// List advisories pub async fn all( state: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, web::Query(Deprecation { deprecated }): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json( state .fetch_advisories(search, paginated, deprecated, &tx) @@ -118,12 +119,12 @@ pub async fn all( /// Get an advisory pub async fn get( state: web::Data, - db: web::Data, + db: web::Data, key: web::Path, _: Require, ) -> actix_web::Result { let hash_key = Id::from_str(&key).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let fetched = state.fetch_advisory(hash_key, &tx).await?; if let Some(fetched) = fetched { @@ -149,7 +150,7 @@ pub async fn get( pub async fn delete( i: web::Data, service: web::Data, - db: web::Data, + db: web::Data, key: web::Path, _: Require, ) -> Result { @@ -213,7 +214,7 @@ pub async fn upload( }): web::Query, content_type: Option>, bytes: web::Bytes, - db: web::Data, + db: web::Data, _: Require, ) -> Result { let bytes = decompress_async(bytes, content_type.map(|ct| ct.0), config.upload_limit).await??; @@ -251,15 +252,14 @@ pub async fn upload( #[get("/v3/advisory/{key}/download")] /// Download an advisory document pub async fn download( - db: web::Data, + db: web::Data, ingestor: web::Data, advisory: web::Data, key: web::Path, _: Require, ) -> Result { - // the user requested id let id = Id::from_str(&key).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; // look up document by id let Some(advisory) = advisory.fetch_advisory(id, &tx).await? else { diff --git a/modules/fundamental/src/advisory/service/mod.rs b/modules/fundamental/src/advisory/service/mod.rs index 5aaa5d832..08a8213c4 100644 --- a/modules/fundamental/src/advisory/service/mod.rs +++ b/modules/fundamental/src/advisory/service/mod.rs @@ -5,13 +5,13 @@ use crate::{ use sea_orm::{ ActiveModelTrait, ActiveValue::Set, ConnectionTrait, DatabaseBackend, DbErr, EntityTrait, FromQueryResult, IntoActiveModel, QueryResult, QuerySelect, QueryTrait, RelationTrait, Select, - Statement, TransactionTrait, + Statement, }; use sea_query::{ColumnType, Expr, JoinType}; use tracing::instrument; use trustify_common::{ db::{ - Database, UpdateDeprecatedAdvisory, + UpdateDeprecatedAdvisory, limiter::{LimitedResult, LimiterAsModelTrait}, multi_model::{FromQueryResultMultiModel, SelectIntoMultiModel}, pagination_cache::PaginationCache, @@ -25,13 +25,13 @@ use trustify_module_ingestor::common::{Deprecation, DeprecationExt}; use uuid::Uuid; pub struct AdvisoryService { - db: Database, cache: PaginationCache, } impl AdvisoryService { - pub fn new(db: Database, cache: PaginationCache) -> Self { - Self { db, cache } + /// Creates a new advisory service. + pub fn new(cache: PaginationCache) -> Self { + Self { cache } } #[instrument(skip(self, connection))] @@ -143,18 +143,17 @@ impl AdvisoryService { /// Update the labels of an advisory /// - /// Returns `Ok(Some(()))` if a document was found and updated. If no document was found, it will - /// return `Ok(None)`. - /// - /// The function will handle its own transaction. - pub async fn update_labels(&self, id: Id, mutator: F) -> Result, Error> + /// Finds the advisory by id using `FOR UPDATE`, applies the mutator, and stores the result. + /// The caller must provide a transaction for `FOR UPDATE` semantics. + pub async fn update_labels( + &self, + id: Id, + mutator: F, + connection: &impl ConnectionTrait, + ) -> Result, Error> where F: FnOnce(Labels) -> Labels, { - let tx = self.db.begin().await.map_err(Error::from)?; - - // work around missing "FOR UPDATE" issue - let mut query = advisory::Entity::find() .try_filter(id) .map_err(Error::IdKey)? @@ -162,33 +161,20 @@ impl AdvisoryService { query.sql.push_str(" FOR UPDATE"); - // find the current entry - let Some(result) = advisory::Entity::find() .from_raw_sql(query) - .one(&tx) + .one(connection) .await .map_err(Error::from)? else { - // return early, nothing found return Ok(None); }; - // perform the mutation - let labels = result.labels.clone(); let mut result = result.into_active_model(); result.labels = Set(mutator(labels).validate()?); - // store - - result.update(&tx).await.map_err(Error::from)?; - - // commit - - tx.commit().await.map_err(Error::from)?; - - // return + result.update(connection).await.map_err(Error::from)?; Ok(Some(())) } diff --git a/modules/fundamental/src/advisory/service/test.rs b/modules/fundamental/src/advisory/service/test.rs index 560ffeaeb..2dd159ae4 100644 --- a/modules/fundamental/src/advisory/service/test.rs +++ b/modules/fundamental/src/advisory/service/test.rs @@ -1,5 +1,6 @@ use super::*; use crate::{advisory::model::AdvisoryHead, source_document::model::SourceDocument}; +use sea_orm::TransactionTrait; use std::{collections::HashMap, str::FromStr}; use test_context::test_context; use test_log::test; @@ -76,7 +77,7 @@ async fn all_advisories(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ingest_sample_advisory(ctx, "RHSA-2", "RHSA-2").await?; - let fetch = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let fetch = AdvisoryService::new(PaginationCache::for_test()); let fetched = fetch .fetch_advisories( q(""), @@ -141,7 +142,7 @@ async fn single_advisory(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ingest_sample_advisory(ctx, "RHSA-2", "RHSA-2").await?; - let fetch = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let fetch = AdvisoryService::new(PaginationCache::for_test()); let jenny256 = Id::sha256(&digests.sha256); let jenny384 = Id::sha384(&digests.sha384); let jenny512 = Id::sha512(&digests.sha512); @@ -226,7 +227,7 @@ async fn delete_advisory(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ) .await?; - let fetch = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let fetch = AdvisoryService::new(PaginationCache::for_test()); let jenny256 = Id::sha256(&digests.sha256); let fetched = fetch.fetch_advisory(jenny256.clone(), &ctx.db).await?; @@ -284,7 +285,7 @@ async fn set_advisory_label(ctx: &TrustifyContext) -> Result<(), anyhow::Error> ) .await?; - let advisory_service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let advisory_service = AdvisoryService::new(PaginationCache::for_test()); let jenny256 = Id::sha256(&digests.sha256); let fetched = advisory_service @@ -360,7 +361,7 @@ async fn update_advisory_label(ctx: &TrustifyContext) -> Result<(), anyhow::Erro ) .await?; - let advisory_service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let advisory_service = AdvisoryService::new(PaginationCache::for_test()); let jenny256 = Id::sha256(&digests.sha256); let fetched = advisory_service @@ -381,9 +382,11 @@ async fn update_advisory_label(ctx: &TrustifyContext) -> Result<(), anyhow::Erro update_map.insert("label_3".to_string(), "Third Label".to_string()); let update_labels = Labels(update_map); let update = trustify_entity::labels::Update::new(); + let tx = ctx.db.begin().await?; advisory_service - .update_labels(id.clone(), |_| update.apply_to(update_labels)) + .update_labels(id.clone(), |_| update.apply_to(update_labels), &tx) .await?; + tx.commit().await?; let fetched_again = advisory_service.fetch_advisory(id.clone(), &ctx.db).await?; //update only alters values of pre-existing keys - it won't add in an entirely new key/value pair diff --git a/modules/fundamental/src/db.rs b/modules/fundamental/src/db.rs deleted file mode 100644 index 4153e8cf1..000000000 --- a/modules/fundamental/src/db.rs +++ /dev/null @@ -1,29 +0,0 @@ -use crate::error::Error; -use sea_orm::{AccessMode, DatabaseTransaction, IsolationLevel, TransactionTrait}; - -#[async_trait::async_trait] -pub trait DatabaseExt { - /// Begin a REPEATABLE READ transaction for consistent read operations. - /// - /// This ensures that all queries within the transaction see a consistent snapshot - /// of the database, preventing race conditions from concurrent write operations, e.g. DELETE - /// - /// Uses REPEATABLE READ isolation level with READ ONLY access mode, which is - /// lightweight in PostgreSQL (no locks acquired, uses MVCC snapshots). - async fn begin_read(&self) -> Result; -} - -#[async_trait::async_trait] -impl DatabaseExt for T -where - T: TransactionTrait + Sync, -{ - async fn begin_read(&self) -> Result { - self.begin_with_config( - Some(IsolationLevel::RepeatableRead), - Some(AccessMode::ReadOnly), - ) - .await - .map_err(Error::from) - } -} diff --git a/modules/fundamental/src/endpoints.rs b/modules/fundamental/src/endpoints.rs index a67155d45..960436618 100644 --- a/modules/fundamental/src/endpoints.rs +++ b/modules/fundamental/src/endpoints.rs @@ -1,5 +1,5 @@ use actix_web::web; -use trustify_common::db::{Database, pagination_cache::PaginationCache}; +use trustify_common::db::{self, pagination_cache::PaginationCache}; use trustify_module_analysis::service::AnalysisService; use trustify_module_ingestor::graph::Graph; use trustify_module_ingestor::service::IngestorService; @@ -16,28 +16,36 @@ pub struct Config { pub fn configure( svc: &mut utoipa_actix_web::service_config::ServiceConfig, config: Config, - db: Database, + db_rw: db::ReadWrite, + db_ro: db::ReadOnly, storage: impl Into, analysis: AnalysisService, cache: PaginationCache, ) { - let ingestor_service = IngestorService::new(Graph::new(db.clone()), storage, Some(analysis)); + let ingestor_service = IngestorService::new(Graph::new(), storage, Some(analysis)); svc.app_data(web::Data::new(ingestor_service)); crate::advisory::endpoints::configure( svc, - db.clone(), + db_rw.clone(), + db_ro.clone(), config.advisory_upload_limit, cache.clone(), ); - crate::license::endpoints::configure(svc, db.clone()); - crate::organization::endpoints::configure(svc, db.clone(), cache.clone()); - crate::purl::endpoints::configure(svc, db.clone(), cache.clone()); - crate::product::endpoints::configure(svc, db.clone(), cache.clone()); - crate::sbom::endpoints::configure(svc, db.clone(), config.sbom_upload_limit, cache.clone()); - crate::vulnerability::endpoints::configure(svc, db.clone(), cache.clone()); - crate::weakness::endpoints::configure(svc, db.clone(), cache.clone()); - crate::sbom_group::endpoints::configure(svc, db, config.max_group_name_length, cache); + crate::license::endpoints::configure(svc, db_ro.clone()); + crate::organization::endpoints::configure(svc, db_ro.clone(), cache.clone()); + crate::purl::endpoints::configure(svc, db_ro.clone(), cache.clone()); + crate::product::endpoints::configure(svc, db_rw.clone(), db_ro.clone(), cache.clone()); + crate::sbom::endpoints::configure( + svc, + db_rw.clone(), + db_ro.clone(), + config.sbom_upload_limit, + cache.clone(), + ); + crate::vulnerability::endpoints::configure(svc, db_ro.clone(), cache.clone()); + crate::weakness::endpoints::configure(svc, db_ro.clone(), cache.clone()); + crate::sbom_group::endpoints::configure(svc, db_rw, db_ro, config.max_group_name_length, cache); } #[derive(Clone, Debug, PartialEq, Eq, Default, ToSchema, serde::Deserialize, IntoParams)] diff --git a/modules/fundamental/src/error.rs b/modules/fundamental/src/error.rs index 362016037..6cf5cad46 100644 --- a/modules/fundamental/src/error.rs +++ b/modules/fundamental/src/error.rs @@ -2,7 +2,7 @@ use actix_web::{HttpResponse, ResponseError, body::BoxBody}; use sea_orm::DbErr; use std::borrow::Cow; use trustify_common::{ - db::{DatabaseErrors, limiter::LimiterError, pagination_cache::LimitError}, + db::{DatabaseErrors, DbError, limiter::LimiterError, pagination_cache::LimitError}, decompress, error::ErrorInformation, id::IdError, @@ -89,6 +89,16 @@ impl From for Error { } } +impl From for Error { + fn from(value: DbError) -> Self { + match value { + DbError::Database(err) => Self::Database(err), + DbError::Unavailable => Self::Unavailable, + DbError::ReadOnly => Self::Internal(value.to_string()), + } + } +} + impl ResponseError for Error { fn error_response(&self) -> HttpResponse { match self { diff --git a/modules/fundamental/src/lib.rs b/modules/fundamental/src/lib.rs index d9deaf8c6..c8b3c183e 100644 --- a/modules/fundamental/src/lib.rs +++ b/modules/fundamental/src/lib.rs @@ -2,7 +2,6 @@ pub mod advisory; pub mod common; -pub mod db; pub mod endpoints; pub mod error; pub mod license; diff --git a/modules/fundamental/src/license/endpoints/mod.rs b/modules/fundamental/src/license/endpoints/mod.rs index 2aa8ec71c..449976070 100644 --- a/modules/fundamental/src/license/endpoints/mod.rs +++ b/modules/fundamental/src/license/endpoints/mod.rs @@ -1,14 +1,11 @@ -use crate::{ - db::DatabaseExt, - license::{ - endpoints::spdx::{get_spdx_license, list_spdx_licenses}, - service::{LicenseService, LicenseText}, - }, +use crate::license::{ + endpoints::spdx::{get_spdx_license, list_spdx_licenses}, + service::{LicenseService, LicenseText}, }; use actix_web::{HttpResponse, Responder, get, web}; use trustify_auth::{ReadSbom, authorizer::Require}; use trustify_common::{ - db::{Database, query::Query}, + db::{self, query::Query}, model::{Paginated, PaginatedResults}, }; use trustify_query::TrustifyQuery; @@ -16,7 +13,7 @@ use trustify_query_derive::Query; pub mod spdx; -pub fn configure(config: &mut utoipa_actix_web::service_config::ServiceConfig, db: Database) { +pub fn configure(config: &mut utoipa_actix_web::service_config::ServiceConfig, db: db::ReadOnly) { let license_service = LicenseService::new(); config @@ -47,12 +44,12 @@ struct LicenseQuery { #[get("/v3/license")] pub async fn list_licenses( service: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json(service.licenses(search, paginated, &tx).await?)) } diff --git a/modules/fundamental/src/organization/endpoints/mod.rs b/modules/fundamental/src/organization/endpoints/mod.rs index c7d61e63e..30ae31399 100644 --- a/modules/fundamental/src/organization/endpoints/mod.rs +++ b/modules/fundamental/src/organization/endpoints/mod.rs @@ -1,24 +1,21 @@ #[cfg(test)] mod test; -use crate::{ - db::DatabaseExt, - organization::{ - model::{OrganizationDetails, OrganizationSummary}, - service::OrganizationService, - }, +use crate::organization::{ + model::{OrganizationDetails, OrganizationSummary}, + service::OrganizationService, }; use actix_web::{HttpResponse, Responder, get, web}; use trustify_auth::{ReadMetadata, authorizer::Require}; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, model::Paginated, }; use uuid::Uuid; pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db: db::ReadOnly, cache: PaginationCache, ) { let service = OrganizationService::new(cache); @@ -44,12 +41,12 @@ pub fn configure( /// List organizations pub async fn all( state: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json(state.fetch_organizations(search, paginated, &tx).await?)) } @@ -68,11 +65,11 @@ pub async fn all( /// Retrieve organization details pub async fn get( state: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let fetched = state.fetch_organization(*id, &tx).await?; if let Some(fetched) = fetched { diff --git a/modules/fundamental/src/product/endpoints/mod.rs b/modules/fundamental/src/product/endpoints/mod.rs index 7bbf7ddfb..dca3f7688 100644 --- a/modules/fundamental/src/product/endpoints/mod.rs +++ b/modules/fundamental/src/product/endpoints/mod.rs @@ -3,7 +3,6 @@ mod test; use crate::{ Error, - db::DatabaseExt, product::{ model::{details::ProductDetails, summary::ProductSummary}, service::ProductService, @@ -13,19 +12,21 @@ use actix_web::{HttpResponse, Responder, delete, get, web}; use sea_orm::TransactionTrait; use trustify_auth::{DeleteMetadata, ReadMetadata, authorizer::Require}; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, model::{Paginated, PaginatedResults}, }; use uuid::Uuid; pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db_rw: db::ReadWrite, + db_ro: db::ReadOnly, cache: PaginationCache, ) { let service = ProductService::new(cache); config - .app_data(web::Data::new(db)) + .app_data(web::Data::new(db_rw)) + .app_data(web::Data::new(db_ro)) .app_data(web::Data::new(service)) .service(all) .service(delete) @@ -46,12 +47,12 @@ pub fn configure( #[get("/v3/product")] pub async fn all( state: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json(state.fetch_products(search, paginated, &tx).await?)) } @@ -69,11 +70,11 @@ pub async fn all( #[get("/v3/product/{id}")] pub async fn get( state: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let fetched = state.fetch_product(*id, &tx).await?; if let Some(fetched) = fetched { Ok(HttpResponse::Ok().json(fetched)) @@ -96,7 +97,7 @@ pub async fn get( #[delete("/v3/product/{id}")] pub async fn delete( state: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> Result { diff --git a/modules/fundamental/src/purl/endpoints/base.rs b/modules/fundamental/src/purl/endpoints/base.rs index e74405753..8afddbe1b 100644 --- a/modules/fundamental/src/purl/endpoints/base.rs +++ b/modules/fundamental/src/purl/endpoints/base.rs @@ -1,6 +1,5 @@ use crate::{ Error, - db::DatabaseExt, purl::{ model::{details::base_purl::BasePurlDetails, summary::base_purl::BasePurlSummary}, service::PurlService, @@ -11,7 +10,7 @@ use sea_orm::prelude::Uuid; use std::str::FromStr; use trustify_auth::{ReadSbom, authorizer::Require}; use trustify_common::{ - db::{Database, query::Query}, + db::{self, query::Query}, id::IdError, model::{Paginated, PaginatedResults}, purl::Purl, @@ -31,11 +30,11 @@ use trustify_common::{ /// Retrieve details about a base versionless pURL pub async fn get_base_purl( service: web::Data, - db: web::Data, + db: web::Data, key: web::Path, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; if key.starts_with("pkg:") { let purl = Purl::from_str(&key).map_err(|e| Error::IdKey(IdError::Purl(e)))?; Ok(HttpResponse::Ok().json(service.base_purl_by_purl(&purl, &tx).await?)) @@ -60,10 +59,10 @@ pub async fn get_base_purl( /// List base versionless pURLs pub async fn all_base_purls( service: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json(service.base_purls(search, paginated, &tx).await?)) } diff --git a/modules/fundamental/src/purl/endpoints/mod.rs b/modules/fundamental/src/purl/endpoints/mod.rs index d9803a553..0ac1fe541 100644 --- a/modules/fundamental/src/purl/endpoints/mod.rs +++ b/modules/fundamental/src/purl/endpoints/mod.rs @@ -1,6 +1,5 @@ use crate::{ Error, - db::DatabaseExt, endpoints::Deprecation, purl::{ model::{ @@ -15,7 +14,7 @@ use sea_orm::prelude::Uuid; use std::str::FromStr; use trustify_auth::{ReadAdvisory, ReadSbom, authorizer::Require}; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, id::IdError, model::{Paginated, PaginatedResults}, purl::Purl, @@ -28,7 +27,7 @@ mod test; pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db: db::ReadOnly, cache: PaginationCache, ) { let purl_service = PurlService::new(cache); @@ -59,12 +58,12 @@ pub fn configure( /// Retrieve details of a fully-qualified pURL pub async fn get( service: web::Data, - db: web::Data, + db: web::Data, key: web::Path, web::Query(Deprecation { deprecated }): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; if key.starts_with("pkg") { let purl = Purl::from_str(&key).map_err(Error::Purl)?; Ok(HttpResponse::Ok().json(service.purl_by_purl(&purl, deprecated, &tx).await?)) @@ -89,12 +88,12 @@ pub async fn get( /// List fully-qualified pURLs pub async fn all( service: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json(service.purls(search, paginated, &tx).await?)) } @@ -114,11 +113,11 @@ mod v2 { #[deprecated = "Use the v3 version of this API"] pub async fn recommend( purl_service: web::Data, - db: web::Data, + db: web::Data, request: web::Json, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let recommendations = purl_service.recommend_purls(&request.purls, &tx).await?; let response = RecommendResponse { recommendations }; @@ -141,11 +140,11 @@ mod v3 { #[post("/v3/purl/recommend")] pub async fn recommend( purl_service: web::Data, - db: web::Data, + db: web::Data, request: web::Json, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let recommendations = purl_service.recommend_purls(&request.purls, &tx).await?; let response = RecommendResponse { recommendations }; diff --git a/modules/fundamental/src/sbom/endpoints/label.rs b/modules/fundamental/src/sbom/endpoints/label.rs index 14e64980d..b43282a2f 100644 --- a/modules/fundamental/src/sbom/endpoints/label.rs +++ b/modules/fundamental/src/sbom/endpoints/label.rs @@ -1,16 +1,17 @@ use crate::{ + Error, common::service::{DocumentType, fetch_labels}, - db::DatabaseExt, sbom::service::SbomService, }; use actix_web::{HttpResponse, Responder, get, patch, put, web}; +use sea_orm::TransactionTrait; use serde::Deserialize; use trustify_auth::{ Permission, UpdateSbom, authenticator::user::UserInformation, authorizer::{Authorizer, Require}, }; -use trustify_common::{db::Database, id::Id}; +use trustify_common::{db, id::Id}; use trustify_entity::labels::{Labels, Update}; use utoipa::IntoParams; @@ -42,14 +43,14 @@ mod default { #[get("/v3/sbom-labels")] /// List all unique key/value labels from all SBOMs pub async fn all( - db: web::Data, + db: web::Data, web::Query(query): web::Query, authorizer: web::Data, user: UserInformation, ) -> actix_web::Result { authorizer.require(&user, Permission::ReadSbom)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let result = fetch_labels(DocumentType::Sbom, query.filter_text, query.limit, &tx).await?; Ok(HttpResponse::Ok().json(result)) @@ -71,19 +72,21 @@ pub async fn all( #[patch("/v3/sbom/{id}/label")] pub async fn update( sbom: web::Data, + db: web::Data, id: web::Path, web::Json(update): web::Json, _: Require, -) -> actix_web::Result { - Ok( - match sbom - .update_labels(id.into_inner(), |labels| update.apply_to(labels)) - .await? - { - Some(()) => HttpResponse::NoContent(), - None => HttpResponse::NotFound(), - }, - ) +) -> Result { + let tx = db.begin().await?; + let result = sbom + .update_labels(id.into_inner(), |labels| update.apply_to(labels), &tx) + .await?; + tx.commit().await?; + + Ok(match result { + Some(()) => HttpResponse::NoContent(), + None => HttpResponse::NotFound(), + }) } /// Replace the labels of an SBOM @@ -102,7 +105,7 @@ pub async fn update( #[put("/v3/sbom/{id}/label")] pub async fn set( sbom: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Json(labels): web::Json, _: Require, diff --git a/modules/fundamental/src/sbom/endpoints/mod.rs b/modules/fundamental/src/sbom/endpoints/mod.rs index 477ecd21a..ce3798a83 100644 --- a/modules/fundamental/src/sbom/endpoints/mod.rs +++ b/modules/fundamental/src/sbom/endpoints/mod.rs @@ -11,7 +11,6 @@ use uuid::Uuid; use crate::{ Error, common::{LicenseRefMapping, service::delete_doc}, - db::DatabaseExt, license::{ get_sanitize_filename, service::{LicenseService, license_export::LicenseExporter}, @@ -38,7 +37,7 @@ use trustify_auth::{ authorizer::{Authorizer, Require}, }; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, decompress::decompress_async, id::Id, model::{BinaryData, Paginated, PaginatedResults}, @@ -59,14 +58,16 @@ pub struct ModelGetParams { pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db_rw: db::ReadWrite, + db_ro: db::ReadOnly, upload_limit: usize, cache: PaginationCache, ) { - let sbom_service = SbomService::new(db.clone(), cache); + let sbom_service = SbomService::new(cache); config - .app_data(web::Data::new(db)) + .app_data(web::Data::new(db_rw)) + .app_data(web::Data::new(db_ro)) .app_data(web::Data::new(sbom_service)) .app_data(web::Data::new(Config { upload_limit })) .service(v2::all) @@ -105,12 +106,12 @@ const CONTENT_TYPE_GZIP: &str = "application/gzip"; #[get("/v3/sbom/{id}/all-license-ids")] pub async fn get_unique_licenses( fetcher: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { let parsed_id = Id::from_str(&id).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let all_licenses_info = fetcher.get_all_license_info(parsed_id, &tx).await?; match all_licenses_info { Some(all_licenses) => Ok(HttpResponse::Ok().json(all_licenses)), @@ -132,11 +133,11 @@ pub async fn get_unique_licenses( #[get("/v3/sbom/{id}/license-export")] pub async fn get_license_export( fetcher: web::Data, - db: web::Data, + db: web::Data, id: web::Path, ) -> actix_web::Result { let id = Id::from_str(&id).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let license_export_result = fetcher.license_export(id, &tx).await?; if let Some(name_group_version) = license_export_result.sbom_name_group_version.clone() { @@ -193,7 +194,7 @@ mod v2 { #[deprecated = "Use the v3 version of this API"] pub async fn all( fetch: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, QsQuery(group_filter): QsQuery, @@ -202,7 +203,7 @@ mod v2 { ) -> actix_web::Result { authorizer.require(&user, Permission::ReadSbom)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let mut options = FetchOptions::default(); if !group_filter.group.is_empty() { options = options.groups(group_filter.group); @@ -236,7 +237,7 @@ mod v3 { #[get("/v3/sbom")] pub async fn all( fetch: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, QsQuery(group_filter): QsQuery, @@ -245,7 +246,7 @@ mod v3 { ) -> actix_web::Result { authorizer.require(&user, Permission::ReadSbom)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let mut options = FetchOptions::default(); if !group_filter.group.is_empty() { options = options.groups(group_filter.group); @@ -278,7 +279,7 @@ mod v3 { #[get("/v3/sbom/by-package")] pub async fn all_related( sbom: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, web::Query(all_related): web::Query, @@ -288,7 +289,7 @@ pub async fn all_related( authorizer.require(&user, Permission::ReadSbom)?; let id = (&all_related).try_into()?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let result = sbom.find_related_sboms(id, paginated, search, &tx).await?; @@ -312,7 +313,7 @@ pub async fn all_related( #[get("/v3/sbom/count-by-package")] pub async fn count_related( sbom: web::Data, - db: web::Data, + db: web::Data, web::Json(ids): web::Json>, _: Require, ) -> actix_web::Result { @@ -321,7 +322,7 @@ pub async fn count_related( .map(SbomExternalPackageReference::try_from) .collect::, _>>()?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let result = sbom.count_related_sboms(ids, &tx).await?; Ok(HttpResponse::Ok().json(result)) @@ -342,13 +343,13 @@ pub async fn count_related( #[get("/v3/sbom/{id}")] pub async fn get( fetcher: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { let id = Id::from_str(&id).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; match fetcher.fetch_sbom_summary(id, &tx).await? { Some(v) => Ok(HttpResponse::Ok().json(v)), @@ -371,12 +372,12 @@ pub async fn get( #[get("/v3/sbom/{id}/advisory")] pub async fn get_sbom_advisories( fetcher: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { let id = Id::from_str(&id).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let statuses: Vec = vec!["affected".to_string()]; match fetcher.fetch_sbom_details(id, statuses, &tx).await? { @@ -403,7 +404,7 @@ all!(GetSbomAdvisories -> ReadSbom, ReadAdvisory); pub async fn delete( i: web::Data, service: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> Result { @@ -444,14 +445,14 @@ pub async fn delete( #[get("/v3/sbom/{id}/packages")] pub async fn packages( fetch: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Query(search): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { let id = Id::from_str(&id).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let Some((sbom, _, _)) = fetch.fetch_sbom(id, &tx).await? else { return Ok(HttpResponse::NotFound().finish()); @@ -481,14 +482,14 @@ pub async fn packages( #[get("/v3/sbom/{id}/models")] pub async fn models( fetch: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Query(search): web::Query, web::Query(paginated): web::Query, web::Query(ModelGetParams { counts }): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let result = fetch .fetch_sbom_models(Some(id.into_inner()), search, paginated, counts, &tx) .await?; @@ -511,13 +512,13 @@ pub async fn models( #[get("/v3/sbom/models")] pub async fn all_models( fetch: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, web::Query(ModelGetParams { counts }): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let result = fetch .fetch_sbom_models(None, search, paginated, counts, &tx) .await?; @@ -555,7 +556,7 @@ struct RelatedQuery { #[get("/v3/sbom/{id}/related")] pub async fn related( fetch: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Query(search): web::Query, web::Query(paginated): web::Query, @@ -563,7 +564,7 @@ pub async fn related( _: Require, ) -> actix_web::Result { let id = Id::from_str(&id).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let Some((sbom, _, _)) = fetch.fetch_sbom(id, &tx).await? else { return Ok(HttpResponse::NotFound().finish()); @@ -637,7 +638,7 @@ pub async fn upload( ingestor: web::Data, sbom_group: web::Data, config: web::Data, - db: web::Data, + db: web::Data, QsQuery(UploadQuery { labels, format, @@ -650,28 +651,26 @@ pub async fn upload( ) -> Result { let bytes = decompress_async(bytes, content_type.map(|ct| ct.0), config.upload_limit).await??; - let result = db - .transaction(async |tx| { - let mut result = ingestor - .ingest(&bytes, format, labels, None, cache, tx) - .await - .map_err(Error::Ingestor)?; - - if !group.is_empty() { - sbom_group - .update_assignments(&result.id, None, group, tx) - .await?; - } + let tx = db.begin().await?; - // Rewrite ID to have the prefix: Although the field is "id" it always carried the ID, - // but with the `urn:uuid:` prefix. Which was used for "key" fields. Which accepted - // for than the actual ID. The whole naming is flawed and confusing. But in order to - // keep the API stable, we need to return the ID with the prefix. - result.id = format!("urn:uuid:{}", result.id); + let mut result = ingestor + .ingest(&bytes, format, labels, None, cache, &tx) + .await + .map_err(Error::Ingestor)?; - Ok::<_, Error>(result) - }) - .await?; + if !group.is_empty() { + sbom_group + .update_assignments(&result.id, None, group, &tx) + .await?; + } + + // Rewrite ID to have the prefix: Although the field is "id" it always carried the ID, + // but with the `urn:uuid:` prefix. Which was used for "key" fields. Which accepted + // for than the actual ID. The whole naming is flawed and confusing. But in order to + // keep the API stable, we need to return the ID with the prefix. + result.id = format!("urn:uuid:{}", result.id); + + tx.commit().await?; log::info!("Uploaded SBOM: {}", result.id); Ok(HttpResponse::Created().json(result)) @@ -692,13 +691,13 @@ pub async fn upload( #[get("/v3/sbom/{key}/download")] pub async fn download( ingestor: web::Data, - db: web::Data, + db: web::Data, sbom: web::Data, key: web::Path, _: Require, ) -> Result { let id = Id::from_str(&key).map_err(Error::IdKey)?; - let tx = db.begin_read().await?; + let tx = db.begin().await?; let Some(sbom) = sbom.fetch_sbom_summary(id, &tx).await? else { return Ok(HttpResponse::NotFound().finish()); diff --git a/modules/fundamental/src/sbom/service/label.rs b/modules/fundamental/src/sbom/service/label.rs index ac1c3cd0a..741c1d623 100644 --- a/modules/fundamental/src/sbom/service/label.rs +++ b/modules/fundamental/src/sbom/service/label.rs @@ -1,7 +1,7 @@ use crate::{Error, sbom::service::SbomService}; use sea_orm::{ ActiveModelTrait, ActiveValue::Set, ConnectionTrait, DatabaseBackend, EntityTrait, - IntoActiveModel, QueryTrait, TransactionTrait, + IntoActiveModel, QueryTrait, }; use sea_query::Expr; use trustify_common::id::{Id, TrySelectForId}; @@ -30,47 +30,37 @@ impl SbomService { /// Update the labels of an SBOM /// - /// Returns `Ok(Some(()))` if a document was found and updated. If no document was found, it will - /// return `Ok(None)`. - /// - /// The function will handle its own transaction. - pub async fn update_labels(&self, id: Id, mutator: F) -> Result, Error> + /// Finds the SBOM by id using `FOR UPDATE`, applies the mutator, and stores the result. + /// The caller must provide a transaction for `FOR UPDATE` semantics. + pub async fn update_labels( + &self, + id: Id, + mutator: F, + connection: &impl ConnectionTrait, + ) -> Result, Error> where F: FnOnce(Labels) -> Labels, { - let tx = self.db.begin().await?; - - // work around missing "FOR UPDATE" issue - let mut query = sbom::Entity::find() .try_filter(id)? .build(DatabaseBackend::Postgres); query.sql.push_str(" FOR UPDATE"); - // find the current entry - - let Some(result) = sbom::Entity::find().from_raw_sql(query).one(&tx).await? else { - // return early, nothing found + let Some(result) = sbom::Entity::find() + .from_raw_sql(query) + .one(connection) + .await? + else { return Ok(None); }; - // perform the mutation - let labels = result.labels.clone(); let mut result = result.into_active_model(); result.labels = Set(mutator(labels).validate()?); result.revision = Set(Uuid::now_v7()); - // store - - result.update(&tx).await?; - - // commit - - tx.commit().await?; - - // return + result.update(connection).await?; Ok(Some(())) } diff --git a/modules/fundamental/src/sbom/service/mod.rs b/modules/fundamental/src/sbom/service/mod.rs index bee430f47..38d3c1646 100644 --- a/modules/fundamental/src/sbom/service/mod.rs +++ b/modules/fundamental/src/sbom/service/mod.rs @@ -5,15 +5,15 @@ pub mod sbom; #[cfg(test)] mod test; -use trustify_common::db::{Database, pagination_cache::PaginationCache}; +use trustify_common::db::pagination_cache::PaginationCache; pub struct SbomService { - db: Database, pub(crate) cache: PaginationCache, } impl SbomService { - pub fn new(db: Database, cache: PaginationCache) -> Self { - Self { db, cache } + /// Creates a new SBOM service. + pub fn new(cache: PaginationCache) -> Self { + Self { cache } } } diff --git a/modules/fundamental/src/sbom/service/sbom.rs b/modules/fundamental/src/sbom/service/sbom.rs index 3d85d47e7..a363e3218 100644 --- a/modules/fundamental/src/sbom/service/sbom.rs +++ b/modules/fundamental/src/sbom/service/sbom.rs @@ -1127,6 +1127,7 @@ mod test { use super::*; use test_context::test_context; use test_log::test; + use trustify_common::db::pagination_cache::PaginationCache; use trustify_common::db::query::q; use trustify_common::hashing::Digests; @@ -1182,7 +1183,7 @@ mod test { assert_eq!(sbom_v1.sbom.sbom_id, sbom_v1_again.sbom.sbom_id); assert_ne!(sbom_v1.sbom.sbom_id, sbom_v2.sbom.sbom_id); - let fetch = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let fetch = SbomService::new(PaginationCache::for_test()); let fetched = fetch .fetch_sboms::<_, SbomPackage>( @@ -1247,7 +1248,7 @@ mod test { ) .await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let paginated_with_total = Paginated { total: true, @@ -1331,7 +1332,7 @@ mod test { ) .await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); assert!(service.delete_sbom(sbom_v1.sbom.sbom_id, &ctx.db).await?); assert!(!service.delete_sbom(sbom_v1.sbom.sbom_id, &ctx.db).await?); diff --git a/modules/fundamental/src/sbom/service/test.rs b/modules/fundamental/src/sbom/service/test.rs index 9337bde1f..731464337 100644 --- a/modules/fundamental/src/sbom/service/test.rs +++ b/modules/fundamental/src/sbom/service/test.rs @@ -33,7 +33,7 @@ async fn sbom_details_status(ctx: &TrustifyContext) -> Result<(), anyhow::Error> ]) .await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let id_3_2_12 = results[3].id.clone(); @@ -70,7 +70,7 @@ async fn count_sboms(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ]) .await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let neither_purl = Purl::from_str( "pkg:maven/io.smallrye/smallrye-graphql@0.0.0.redhat-00000?repository_url=https://maven.repository.redhat.com/ga/&type=jar", @@ -117,7 +117,7 @@ async fn sbom_set_labels(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ]) .await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let id_3_2_12 = Id::parse_uuid(&results[3].id)?; @@ -153,7 +153,7 @@ async fn sbom_update_labels(ctx: &TrustifyContext) -> Result<(), anyhow::Error> ]) .await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let id_3_2_12 = Id::parse_uuid(&results[3].id)?; @@ -170,9 +170,11 @@ async fn sbom_update_labels(ctx: &TrustifyContext) -> Result<(), anyhow::Error> update_map.insert("label_3".to_string(), "Third Label".to_string()); let update_labels = Labels(update_map); let update = trustify_entity::labels::Update::new(); + let tx = ctx.db.begin().await?; service - .update_labels(id_3_2_12.clone(), |_| update.apply_to(update_labels)) + .update_labels(id_3_2_12.clone(), |_| update.apply_to(update_labels), &tx) .await?; + tx.commit().await?; let details = service .fetch_sbom_details(id_3_2_12, Default::default(), &ctx.db) @@ -191,7 +193,7 @@ async fn sbom_update_labels(ctx: &TrustifyContext) -> Result<(), anyhow::Error> #[test_context(TrustifyContext)] #[test(tokio::test)] async fn fetch_sboms_filter_by_license(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let paginated_with_total = Paginated { total: true, @@ -392,7 +394,7 @@ async fn fetch_sboms_filter_by_license(ctx: &TrustifyContext) -> Result<(), anyh #[test_context(TrustifyContext)] #[test(tokio::test)] async fn fetch_sbom_packages_filter_by_license(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let paginated_with_total = Paginated { total: true, @@ -543,7 +545,7 @@ async fn delete_sbom_orphaned_purl_test(ctx: &TrustifyContext) -> Result<(), any ); let tx = ctx.db.begin().await?; - let sbom_service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom_service = SbomService::new(PaginationCache::for_test()); // delete the UBI SBOM assert!(sbom_service.delete_sbom(ubi9_sbom.id.parse()?, &tx).await?); tx.commit().await?; @@ -614,7 +616,7 @@ async fn delete_sbom_preserves_advisory_referenced_packages( ); // Delete one of the SBOMs - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let sbom_uuid = results[1].id.parse().expect("SBOM should have a UUID"); let tx = ctx.db.begin().await?; assert!( @@ -707,7 +709,7 @@ async fn delete_sbom_preserves_advisory_referenced_packages( #[test_context(TrustifyContext)] #[test(tokio::test)] async fn test_sbom_package_licenses_coalesce(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); // RED: No packages before ingestion // GREEN: Ingest SPDX (uses expanded_license) and CycloneDX (uses raw license.text) @@ -788,7 +790,7 @@ async fn test_sbom_package_licenses_coalesce(ctx: &TrustifyContext) -> Result<() async fn test_sbom_package_license_filtering_with_coalesce( ctx: &TrustifyContext, ) -> Result<(), anyhow::Error> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); // RED: No packages before ingestion // GREEN: Ingest SPDX with Apache licenses @@ -836,7 +838,7 @@ async fn test_sbom_package_license_filtering_with_coalesce( async fn test_sbom_package_license_not_null_filter( ctx: &TrustifyContext, ) -> Result<(), anyhow::Error> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); // RED: No packages before ingestion // GREEN: Ingest SBOM with licenses diff --git a/modules/fundamental/src/sbom_group/endpoints/mod.rs b/modules/fundamental/src/sbom_group/endpoints/mod.rs index e13083ea1..cd7a76bf7 100644 --- a/modules/fundamental/src/sbom_group/endpoints/mod.rs +++ b/modules/fundamental/src/sbom_group/endpoints/mod.rs @@ -5,7 +5,7 @@ use super::{ model::*, service::{ListOptions, SbomGroupService}, }; -use crate::{Error, db::DatabaseExt}; +use crate::Error; use actix_web::{ HttpRequest, HttpResponse, Responder, delete, get, http::header::{self, ETag, EntityTag, IfMatch}, @@ -19,7 +19,7 @@ use trustify_auth::{ authorizer::Require, }; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, endpoints::extract_revision, model::{Paginated, Revisioned}, }; @@ -27,14 +27,16 @@ use utoipa::ToSchema; pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db_rw: db::ReadWrite, + db_ro: db::ReadOnly, max_group_name_length: usize, cache: PaginationCache, ) { let service = SbomGroupService::new(max_group_name_length, cache); config - .app_data(web::Data::new(db)) + .app_data(web::Data::new(db_rw)) + .app_data(web::Data::new(db_ro)) .app_data(web::Data::new(service)) .service(list) .service(create) @@ -68,13 +70,13 @@ pub fn configure( /// List SBOM groups async fn list( service: web::Data, - db: web::Data, + db: web::Data, web::Query(pagination): web::Query, web::Query(options): web::Query, web::Query(query): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let result = service.list(options, pagination, query, &tx).await?; Ok(HttpResponse::Ok().json(result)) @@ -109,16 +111,16 @@ struct CreateResponse { async fn create( req: HttpRequest, service: web::Data, - db: web::Data, + db: web::Data, web::Json(group): web::Json, _: Require, ) -> Result { + let tx = db.begin().await?; let Revisioned { revision, value: id, - } = db - .transaction(async |tx| service.create(group, tx).await) - .await?; + } = service.create(group, &tx).await?; + tx.commit().await?; Ok(HttpResponse::Created() .append_header((header::LOCATION, format!("{}/{}", req.path(), id))) @@ -147,7 +149,7 @@ async fn create( /// Delete an SBOM group async fn delete( service: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Header(if_match): web::Header, _: Require, @@ -183,7 +185,7 @@ async fn delete( /// Update an SBOM group async fn update( service: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Json(group): web::Json, web::Header(if_match): web::Header, @@ -221,11 +223,11 @@ async fn update( /// Read the SBOM group information async fn read( service: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let group = service.read(&id, &tx).await?; Ok(match group { @@ -254,11 +256,11 @@ async fn read( /// Get SBOM group assignments async fn read_assignments( service: web::Data, - db: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let assignments = service.read_assignments(&id, &tx).await?; Ok(match assignments { @@ -289,20 +291,19 @@ async fn read_assignments( /// Update SBOM group assignments async fn update_assignments( service: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Json(group_ids): web::Json>, web::Header(if_match): web::Header, _: Require, -) -> actix_web::Result { +) -> Result { let revision = extract_revision(&if_match); - db.transaction(async |tx| { - service - .update_assignments(&id, revision, group_ids, tx) - .await - }) - .await?; + let tx = db.begin().await?; + service + .update_assignments(&id, revision, group_ids, &tx) + .await?; + tx.commit().await?; Ok(HttpResponse::NoContent().finish()) } @@ -322,16 +323,15 @@ async fn update_assignments( /// Bulk update SBOM group assignments async fn bulk_update_assignments( service: web::Data, - db: web::Data, + db: web::Data, web::Json(request): web::Json, _: Require, -) -> actix_web::Result { - db.transaction(async |tx| { - service - .bulk_update_assignments(request.sbom_ids, request.group_ids, tx) - .await - }) - .await?; +) -> Result { + let tx = db.begin().await?; + service + .bulk_update_assignments(request.sbom_ids, request.group_ids, &tx) + .await?; + tx.commit().await?; Ok(HttpResponse::NoContent().finish()) } diff --git a/modules/fundamental/src/test/common.rs b/modules/fundamental/src/test/common.rs index c4678f8f2..9b56aae55 100644 --- a/modules/fundamental/src/test/common.rs +++ b/modules/fundamental/src/test/common.rs @@ -1,4 +1,4 @@ -use trustify_common::db::pagination_cache::PaginationCache; +use trustify_common::db::{self, pagination_cache::PaginationCache}; use trustify_module_analysis::config::AnalysisConfig; use trustify_module_analysis::service::AnalysisService; use trustify_test_context::{ @@ -15,17 +15,20 @@ pub async fn caller_with( config: Config, cache: PaginationCache, ) -> anyhow::Result { - let analysis = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let db_rw = db::ReadWrite::new(ctx.db.clone()); + let db_ro = db::ReadOnly::new(ctx.db.clone()); + let analysis = AnalysisService::new(AnalysisConfig::default(), db_ro.clone()); call::caller(|svc| { configure( svc, config, - ctx.db.clone(), + db_rw, + db_ro.clone(), ctx.storage.clone(), analysis.clone(), cache, ); - trustify_module_analysis::endpoints::configure(svc, ctx.db.clone(), analysis); + trustify_module_analysis::endpoints::configure(svc, db_ro, analysis); }) .await } diff --git a/modules/fundamental/src/vulnerability/endpoints/mod.rs b/modules/fundamental/src/vulnerability/endpoints/mod.rs index 22ee7f91e..396ba078c 100644 --- a/modules/fundamental/src/vulnerability/endpoints/mod.rs +++ b/modules/fundamental/src/vulnerability/endpoints/mod.rs @@ -3,7 +3,6 @@ mod test; use crate::common::model::Severity; use crate::{ - db::DatabaseExt, endpoints::Deprecation, vulnerability::{ model::{ @@ -17,7 +16,7 @@ use actix_web::{HttpResponse, Responder, get, post, web}; use time::OffsetDateTime; use trustify_auth::{ReadAdvisory, authorizer::Require}; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, model::{Paginated, PaginatedResults}, }; use trustify_query::TrustifyQuery; @@ -33,7 +32,7 @@ pub struct VulnerabilityGetParams { pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db: db::ReadOnly, cache: PaginationCache, ) { let service = VulnerabilityService::new(cache); @@ -75,13 +74,13 @@ struct VulnerabilityQuery { /// List vulnerabilities pub async fn all( state: web::Data, - db: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, web::Query(Deprecation { deprecated }): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; Ok(HttpResponse::Ok().json( state .fetch_vulnerabilities(search, paginated, deprecated, &tx) @@ -105,13 +104,13 @@ pub async fn all( /// Retrieve vulnerability details pub async fn get( state: web::Data, - db: web::Data, + db: web::Data, id: web::Path, web::Query(Deprecation { deprecated }): web::Query, web::Query(VulnerabilityGetParams { scores }): web::Query, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let vuln = state .fetch_vulnerability(&id, deprecated, scores, &tx) .await?; @@ -135,11 +134,11 @@ pub async fn get( /// Analyze the provided purls for the known vulnerabilities pub async fn analyze( service: web::Data, - db: web::Data, + db: web::Data, web::Json(AnalysisRequest { purls }): web::Json, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let details = service.analyze_purls_v2(purls, &tx).await?; Ok(HttpResponse::Ok().json(details)) @@ -156,11 +155,11 @@ pub async fn analyze( #[post("/v3/vulnerability/analyze")] pub async fn analyze_v3( service: web::Data, - db: web::Data, + db: web::Data, web::Json(AnalysisRequest { purls }): web::Json, _: Require, ) -> actix_web::Result { - let tx = db.begin_read().await?; + let tx = db.begin().await?; let details = service.analyze_purls_v3(purls, &tx).await?; Ok(HttpResponse::Ok().json(details)) diff --git a/modules/fundamental/src/vulnerability/service/test.rs b/modules/fundamental/src/vulnerability/service/test.rs index 72c5a7e9a..21957ee83 100644 --- a/modules/fundamental/src/vulnerability/service/test.rs +++ b/modules/fundamental/src/vulnerability/service/test.rs @@ -130,7 +130,7 @@ async fn statuses_too(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { #[test(tokio::test)] async fn commons_compress(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let vuln_service = VulnerabilityService::new(PaginationCache::for_test()); - let sbom_service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom_service = SbomService::new(PaginationCache::for_test()); // Ingest a CVE declaring the vulnerability present in versions // [1.21,1.26.0) of commons-compress, along with 2 sboms, each of @@ -228,7 +228,7 @@ async fn commons_compress(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn sbom_without_cpe_matching(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom_service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom_service = SbomService::new(PaginationCache::for_test()); let ingest_results = ctx .ingest_documents([ @@ -268,7 +268,7 @@ async fn sbom_without_cpe_matching(ctx: &TrustifyContext) -> Result<(), anyhow:: #[test_context(TrustifyContext)] #[test(tokio::test)] async fn sbom_with_multiple_cpes_not_breaking(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom_service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom_service = SbomService::new(PaginationCache::for_test()); let ingest_results = ctx .ingest_documents([ @@ -294,7 +294,7 @@ async fn sbom_with_multiple_cpes_not_breaking(ctx: &TrustifyContext) -> Result<( #[test(tokio::test)] async fn product_statuses(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let vuln_service = VulnerabilityService::new(PaginationCache::for_test()); - let sbom_service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom_service = SbomService::new(PaginationCache::for_test()); let purl_service = PurlService::new(PaginationCache::for_test()); let ingest_results = ctx diff --git a/modules/fundamental/src/weakness/endpoints/mod.rs b/modules/fundamental/src/weakness/endpoints/mod.rs index 015b9d414..4b083ccee 100644 --- a/modules/fundamental/src/weakness/endpoints/mod.rs +++ b/modules/fundamental/src/weakness/endpoints/mod.rs @@ -2,18 +2,19 @@ use crate::{license::model::LicenseSummary, weakness::service::WeaknessService}; use actix_web::{HttpResponse, Responder, get, web}; use trustify_auth::{ReadWeakness, authorizer::Require}; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, model::{Paginated, PaginatedResults}, }; pub fn configure( config: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db: db::ReadOnly, cache: PaginationCache, ) { - let weakness_service = WeaknessService::new(db, cache); + let weakness_service = WeaknessService::new(cache); config + .app_data(web::Data::new(db)) .app_data(web::Data::new(weakness_service)) .service(list_weaknesses) .service(get_weakness); @@ -34,11 +35,13 @@ pub fn configure( /// List weaknesses pub async fn list_weaknesses( state: web::Data, + db: web::Data, web::Query(search): web::Query, web::Query(paginated): web::Query, _: Require, ) -> actix_web::Result { - Ok(HttpResponse::Ok().json(state.list_weaknesses(search, paginated).await?)) + let tx = db.begin().await?; + Ok(HttpResponse::Ok().json(state.list_weaknesses(search, paginated, &tx).await?)) } #[utoipa::path( @@ -53,10 +56,12 @@ pub async fn list_weaknesses( /// Retrieve weakness details pub async fn get_weakness( state: web::Data, + db: web::Data, id: web::Path, _: Require, ) -> actix_web::Result { - if let Some(weakness_details) = state.get_weakness(&id).await? { + let tx = db.begin().await?; + if let Some(weakness_details) = state.get_weakness(&id, &tx).await? { Ok(HttpResponse::Ok().json(weakness_details)) } else { Ok(HttpResponse::NotFound().finish()) diff --git a/modules/fundamental/src/weakness/service/mod.rs b/modules/fundamental/src/weakness/service/mod.rs index 95a358bf5..2065bdad0 100644 --- a/modules/fundamental/src/weakness/service/mod.rs +++ b/modules/fundamental/src/weakness/service/mod.rs @@ -2,10 +2,9 @@ use crate::{ Error, weakness::model::{WeaknessDetails, WeaknessSummary}, }; -use sea_orm::EntityTrait; +use sea_orm::{ConnectionTrait, EntityTrait}; use trustify_common::{ db::{ - Database, limiter::{LimitedResult, LimiterTrait}, pagination_cache::PaginationCache, query::{Filtering, Query}, @@ -15,22 +14,24 @@ use trustify_common::{ use trustify_entity::weakness; pub struct WeaknessService { - db: Database, cache: PaginationCache, } impl WeaknessService { - pub fn new(db: Database, cache: PaginationCache) -> Self { - Self { db, cache } + /// Creates a new weakness service. + pub fn new(cache: PaginationCache) -> Self { + Self { cache } } - pub async fn list_weaknesses( + /// Lists weaknesses matching the given query. + pub async fn list_weaknesses( &self, query: Query, paginated: impl Pagination, + connection: &C, ) -> Result, Error> { let limiter = weakness::Entity::find().filtering(query)?.limiting( - &self.db, + connection, paginated, &self.cache, )?; @@ -44,8 +45,13 @@ impl WeaknessService { }) } - pub async fn get_weakness(&self, id: &str) -> Result, Error> { - if let Some(found) = weakness::Entity::find_by_id(id).one(&self.db).await? { + /// Gets a single weakness by ID. + pub async fn get_weakness( + &self, + id: &str, + connection: &impl ConnectionTrait, + ) -> Result, Error> { + if let Some(found) = weakness::Entity::find_by_id(id).one(connection).await? { Ok(Some(WeaknessDetails::from_entity(&found).await?)) } else { Ok(None) diff --git a/modules/fundamental/tests/advisory/csaf/delete.rs b/modules/fundamental/tests/advisory/csaf/delete.rs index c88fba024..3ba769037 100644 --- a/modules/fundamental/tests/advisory/csaf/delete.rs +++ b/modules/fundamental/tests/advisory/csaf/delete.rs @@ -4,9 +4,7 @@ use super::prepare_ps_state_change; use test_context::test_context; use test_log::test; use time::OffsetDateTime; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::model::Limit; -use trustify_common::purl::Purl; +use trustify_common::{db::pagination_cache::PaginationCache, model::Limit, purl::Purl}; use trustify_entity::labels::Labels; use trustify_module_fundamental::advisory::model::AdvisoryHead; use trustify_module_fundamental::common::model::{Score, ScoreType}; @@ -38,7 +36,7 @@ async fn simple(ctx: &TrustifyContext) -> anyhow::Result<()> { // now delete the newer one - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); service .delete_advisory(r2.id.parse().expect("must be a UUID variant"), &ctx.db) .await?; @@ -78,7 +76,7 @@ async fn delete_check_vulns(ctx: &TrustifyContext) -> anyhow::Result<()> { // now delete the newer one - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); service .delete_advisory(r2.id.parse().expect("must be a UUID variant"), &ctx.db) .await?; diff --git a/modules/fundamental/tests/advisory/csaf/parallel.rs b/modules/fundamental/tests/advisory/csaf/parallel.rs index e0912d4f9..93837143b 100644 --- a/modules/fundamental/tests/advisory/csaf/parallel.rs +++ b/modules/fundamental/tests/advisory/csaf/parallel.rs @@ -1,8 +1,10 @@ use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::query::Query, model::Paginated}; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + model::Paginated, +}; use trustify_module_fundamental::advisory::service::AdvisoryService; use trustify_module_ingestor::common::Deprecation; use trustify_test_context::TrustifyContext; @@ -17,7 +19,7 @@ async fn ingest_10(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { .ingest_parallel(["csaf/cve-2023-33201.json"; 10]) .await?; - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); let result = service .fetch_advisories( diff --git a/modules/fundamental/tests/advisory/csaf/timeout.rs b/modules/fundamental/tests/advisory/csaf/timeout.rs index 2f9149c08..fb05923b7 100644 --- a/modules/fundamental/tests/advisory/csaf/timeout.rs +++ b/modules/fundamental/tests/advisory/csaf/timeout.rs @@ -1,8 +1,10 @@ use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::query::Query, model::Paginated}; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + model::Paginated, +}; use trustify_module_fundamental::advisory::service::AdvisoryService; use trustify_module_ingestor::common::Deprecation; use trustify_test_context::TrustifyContext; @@ -26,7 +28,7 @@ async fn timeout(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { ]) .await?; - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); let result = service .fetch_advisories( diff --git a/modules/fundamental/tests/advisory/cve/delete.rs b/modules/fundamental/tests/advisory/cve/delete.rs index 9cb1a7188..37a816167 100644 --- a/modules/fundamental/tests/advisory/cve/delete.rs +++ b/modules/fundamental/tests/advisory/cve/delete.rs @@ -22,7 +22,7 @@ async fn withdrawn(ctx: &TrustifyContext) -> anyhow::Result<()> { // now delete the newer one - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); service .delete_advisory(r2.id.parse().expect("must be a UUID variant"), &ctx.db) .await?; diff --git a/modules/fundamental/tests/advisory/cve/parallel.rs b/modules/fundamental/tests/advisory/cve/parallel.rs index 4e613da24..6b26b3053 100644 --- a/modules/fundamental/tests/advisory/cve/parallel.rs +++ b/modules/fundamental/tests/advisory/cve/parallel.rs @@ -1,8 +1,10 @@ use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::query::Query, model::Paginated}; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + model::Paginated, +}; use trustify_module_fundamental::advisory::service::AdvisoryService; use trustify_module_ingestor::common::Deprecation; use trustify_test_context::TrustifyContext; @@ -20,7 +22,7 @@ async fn ingest_10(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { futures_util::future::try_join_all(f).await?; - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); let result = service .fetch_advisories( diff --git a/modules/fundamental/tests/advisory/osv/delete.rs b/modules/fundamental/tests/advisory/osv/delete.rs index f9ceacd56..5b602ddd2 100644 --- a/modules/fundamental/tests/advisory/osv/delete.rs +++ b/modules/fundamental/tests/advisory/osv/delete.rs @@ -22,7 +22,7 @@ async fn fixed(ctx: &TrustifyContext) -> anyhow::Result<()> { // now delete the newer one - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); service .delete_advisory(r2.id.parse().expect("must be a UUID variant"), &ctx.db) .await?; diff --git a/modules/fundamental/tests/advisory/osv/parallel.rs b/modules/fundamental/tests/advisory/osv/parallel.rs index 2620ccbcc..fdae1a9bc 100644 --- a/modules/fundamental/tests/advisory/osv/parallel.rs +++ b/modules/fundamental/tests/advisory/osv/parallel.rs @@ -1,8 +1,10 @@ use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::query::Query, model::Paginated}; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + model::Paginated, +}; use trustify_module_fundamental::advisory::service::AdvisoryService; use trustify_module_ingestor::common::Deprecation; use trustify_test_context::TrustifyContext; @@ -20,7 +22,7 @@ async fn ingest_10(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { futures_util::future::try_join_all(f).await?; - let service = AdvisoryService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = AdvisoryService::new(PaginationCache::for_test()); let result = service .fetch_advisories( diff --git a/modules/fundamental/tests/dataset.rs b/modules/fundamental/tests/dataset.rs index 70c8c338a..126b8d08f 100644 --- a/modules/fundamental/tests/dataset.rs +++ b/modules/fundamental/tests/dataset.rs @@ -7,8 +7,7 @@ use std::time::Instant; use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::id::Id; +use trustify_common::{db::pagination_cache::PaginationCache, id::Id}; use trustify_module_fundamental::sbom::service::SbomService; use trustify_module_storage::service::StorageBackend; use trustify_test_context::{Dataset, TrustifyContext}; @@ -18,7 +17,7 @@ use trustify_test_context::{Dataset, TrustifyContext}; #[test(tokio::test)] #[instrument] async fn ingest(ctx: TrustifyContext) -> anyhow::Result<()> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let storage = &ctx.storage; let start = Instant::now(); diff --git a/modules/fundamental/tests/db.rs b/modules/fundamental/tests/db.rs new file mode 100644 index 000000000..12f601b0e --- /dev/null +++ b/modules/fundamental/tests/db.rs @@ -0,0 +1,86 @@ +use sea_orm::{AccessMode, ConnectionTrait, DbBackend, Statement, TransactionTrait}; +use test_context::test_context; +use test_log::test; +use trustify_common::db::{DbError, ReadOnly, ReadWrite}; +use trustify_test_context::TrustifyContext; + +/// ReadOnly::begin() opens a transaction that PostgreSQL enforces as read-only. +#[test_context(TrustifyContext)] +#[test(tokio::test)] +async fn read_only_begin_rejects_writes(ctx: &TrustifyContext) -> anyhow::Result<()> { + let ro = ReadOnly::new(ctx.db.clone()); + let tx = ro.begin().await?; + + let result = tx + .execute(Statement::from_string( + DbBackend::Postgres, + "CREATE TEMP TABLE _ro_test (id int)".to_string(), + )) + .await; + + assert!( + result.is_err(), + "write must fail on a read-only transaction" + ); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("read-only"), + "error should mention read-only, got: {err}" + ); + + Ok(()) +} + +/// ReadOnly::begin_with_config rejects an explicit ReadWrite access mode +/// before even reaching the database. +#[test_context(TrustifyContext)] +#[test(tokio::test)] +async fn read_only_begin_with_config_rejects_read_write( + ctx: &TrustifyContext, +) -> anyhow::Result<()> { + let ro = ReadOnly::new(ctx.db.clone()); + let result = ro + .begin_with_config(None, Some(AccessMode::ReadWrite)) + .await; + + assert!( + matches!(result, Err(DbError::ReadOnly)), + "explicit ReadWrite must be rejected at the Rust level" + ); + + Ok(()) +} + +/// ReadWrite allows writes through its transaction. +#[test_context(TrustifyContext)] +#[test(tokio::test)] +async fn read_write_allows_writes(ctx: &TrustifyContext) -> anyhow::Result<()> { + let rw = ReadWrite::new(ctx.db.clone()); + let tx = rw.begin().await?; + + tx.execute(Statement::from_string( + DbBackend::Postgres, + "CREATE TEMP TABLE _rw_test (id int)".to_string(), + )) + .await?; + + tx.rollback().await?; + + Ok(()) +} + +/// ReadOnly allows read queries without error. +#[test_context(TrustifyContext)] +#[test(tokio::test)] +async fn read_only_allows_reads(ctx: &TrustifyContext) -> anyhow::Result<()> { + let ro = ReadOnly::new(ctx.db.clone()); + let tx = ro.begin().await?; + + tx.query_one(Statement::from_string( + DbBackend::Postgres, + "SELECT 1 AS n".to_string(), + )) + .await?; + + Ok(()) +} diff --git a/modules/fundamental/tests/issues/oom.rs b/modules/fundamental/tests/issues/oom.rs index 4b2ca29f4..d9e34b806 100644 --- a/modules/fundamental/tests/issues/oom.rs +++ b/modules/fundamental/tests/issues/oom.rs @@ -3,8 +3,7 @@ use std::str::FromStr; use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::id::Id; +use trustify_common::{db::pagination_cache::PaginationCache, id::Id}; use trustify_module_fundamental::sbom::service::SbomService; use trustify_test_context::TrustifyContext; @@ -16,7 +15,7 @@ use trustify_test_context::TrustifyContext; async fn fetch(ctx: &TrustifyContext) -> anyhow::Result<()> { // this requires an imported dataset - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); // update this digest to point to a "large SBOM" // the following statement can be used" /* diff --git a/modules/fundamental/tests/sbom/cyclonedx/cpe.rs b/modules/fundamental/tests/sbom/cyclonedx/cpe.rs index 84ae241c1..5a1151a6b 100644 --- a/modules/fundamental/tests/sbom/cyclonedx/cpe.rs +++ b/modules/fundamental/tests/sbom/cyclonedx/cpe.rs @@ -1,7 +1,6 @@ use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::model::Paginated; +use trustify_common::{db::pagination_cache::PaginationCache, model::Paginated}; use trustify_module_fundamental::sbom::model::SbomPackage; use trustify_module_fundamental::sbom::service::SbomService; use trustify_test_context::TrustifyContext; @@ -12,7 +11,7 @@ use trustify_test_context::TrustifyContext; async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let result = ctx.ingest_document("cyclonedx/simple_cpe.json").await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .describes_packages::<_, _, SbomPackage>( @@ -41,7 +40,7 @@ async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { async fn simple_ref(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let result = ctx.ingest_document("cyclonedx/simple_cpe_2.json").await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .describes_packages::<_, _, SbomPackage>( @@ -70,7 +69,7 @@ async fn simple_ref(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { async fn simple_comp(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let result = ctx.ingest_document("cyclonedx/simple_cpe_3.json").await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .describes_packages::<_, _, SbomPackage>( diff --git a/modules/fundamental/tests/sbom/cyclonedx/external/rh.rs b/modules/fundamental/tests/sbom/cyclonedx/external/rh.rs index 0e3bb11b5..ac64ebee9 100644 --- a/modules/fundamental/tests/sbom/cyclonedx/external/rh.rs +++ b/modules/fundamental/tests/sbom/cyclonedx/external/rh.rs @@ -21,7 +21,7 @@ async fn cdx_prod_comp(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let _prod: Uuid = result.pop().unwrap().id.parse().expect("must have a uid"); let _comp: Uuid = result.pop().unwrap().id.parse().expect("must have a uid"); - let _service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let _service = SbomService::new(PaginationCache::for_test()); // TODO: implement when we have the tools diff --git a/modules/fundamental/tests/sbom/cyclonedx/parallel.rs b/modules/fundamental/tests/sbom/cyclonedx/parallel.rs index 5be8f4ec5..a376c0a96 100644 --- a/modules/fundamental/tests/sbom/cyclonedx/parallel.rs +++ b/modules/fundamental/tests/sbom/cyclonedx/parallel.rs @@ -1,8 +1,10 @@ use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::query::Query, model::Paginated}; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + model::Paginated, +}; use trustify_module_fundamental::sbom::model::SbomPackage; use trustify_module_fundamental::sbom::service::SbomService; use trustify_test_context::TrustifyContext; @@ -22,7 +24,7 @@ async fn ingest_10(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { futures_util::future::try_join_all(f).await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let result = service .fetch_sboms::<_, SbomPackage>( diff --git a/modules/fundamental/tests/sbom/cyclonedx/purl.rs b/modules/fundamental/tests/sbom/cyclonedx/purl.rs index 20d6de127..2f4145352 100644 --- a/modules/fundamental/tests/sbom/cyclonedx/purl.rs +++ b/modules/fundamental/tests/sbom/cyclonedx/purl.rs @@ -1,8 +1,7 @@ use itertools::Itertools; use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::model::Paginated; +use trustify_common::{db::pagination_cache::PaginationCache, model::Paginated}; use trustify_entity::relationship::Relationship; use trustify_module_fundamental::sbom::model::{SbomNodeReference, SbomPackage, Which}; use trustify_module_fundamental::{ @@ -22,7 +21,7 @@ fn to_strings(purls: &[PurlSummary]) -> Vec { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn simple_ref(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let result = ctx .ingest_document("cyclonedx/openssl-3.0.7-18.el9_2.cdx_1.6_aliases.sbom.json") diff --git a/modules/fundamental/tests/sbom/cyclonedx/reingest.rs b/modules/fundamental/tests/sbom/cyclonedx/reingest.rs index 8150a560b..a3fc6501e 100644 --- a/modules/fundamental/tests/sbom/cyclonedx/reingest.rs +++ b/modules/fundamental/tests/sbom/cyclonedx/reingest.rs @@ -4,8 +4,7 @@ use anyhow::bail; use sea_orm::EntityTrait; use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::model::Paginated; +use trustify_common::{db::pagination_cache::PaginationCache, model::Paginated}; use trustify_entity::sbom; use trustify_module_fundamental::sbom::model::SbomPackage; use trustify_module_fundamental::sbom::service::SbomService; @@ -25,7 +24,7 @@ async fn reingest(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { .await? .expect("must be found"); - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .describes_packages::<_, _, SbomPackage>( diff --git a/modules/fundamental/tests/sbom/details.rs b/modules/fundamental/tests/sbom/details.rs index 71c809119..9d27210d8 100644 --- a/modules/fundamental/tests/sbom/details.rs +++ b/modules/fundamental/tests/sbom/details.rs @@ -1,8 +1,7 @@ use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::id::Id; +use trustify_common::{db::pagination_cache::PaginationCache, id::Id}; use trustify_module_fundamental::common::model::{Score, ScoreType, ScoredVector, Severity}; use trustify_module_fundamental::sbom::{model::details::SbomDetails, service::SbomService}; use trustify_test_context::TrustifyContext; @@ -11,7 +10,7 @@ use trustify_test_context::TrustifyContext; #[test(tokio::test)] #[instrument] async fn sbom_details_cyclonedx_osv(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom = SbomService::new(PaginationCache::for_test()); // ingest the SBOM let result1 = ctx.ingest_document("cyclonedx/ghsa_test.json").await?; diff --git a/modules/fundamental/tests/sbom/graph.rs b/modules/fundamental/tests/sbom/graph.rs index 0c1325eb7..c4dbb77f2 100644 --- a/modules/fundamental/tests/sbom/graph.rs +++ b/modules/fundamental/tests/sbom/graph.rs @@ -2,10 +2,9 @@ use std::convert::TryInto; use std::str::FromStr; use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::hashing::Digests; -use trustify_common::purl::Purl; -use trustify_common::sbom::SbomLocator; +use trustify_common::{ + db::pagination_cache::PaginationCache, hashing::Digests, purl::Purl, sbom::SbomLocator, +}; use trustify_entity::relationship::Relationship; use trustify_module_fundamental::purl::model::PurlHead; use trustify_module_fundamental::purl::model::summary::purl::PurlSummary; @@ -277,7 +276,7 @@ async fn ingest_package_relates_to_package_dependency_of( ctx: &TrustifyContext, ) -> Result<(), anyhow::Error> { let system = &ctx.graph; - let fetch = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let fetch = SbomService::new(PaginationCache::for_test()); let sbom1 = system .ingest_sbom( diff --git a/modules/fundamental/tests/sbom/mod.rs b/modules/fundamental/tests/sbom/mod.rs index 4d63cb2fe..806358d9c 100644 --- a/modules/fundamental/tests/sbom/mod.rs +++ b/modules/fundamental/tests/sbom/mod.rs @@ -11,8 +11,10 @@ mod spdx; use sea_orm::{DatabaseTransaction, TransactionTrait}; use std::time::Instant; use tracing::{Instrument, info_span, instrument}; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::Database, hashing::Digests}; +use trustify_common::{ + db::{Database, pagination_cache::PaginationCache}, + hashing::Digests, +}; use trustify_module_fundamental::sbom::service::SbomService; use trustify_module_ingestor::{ graph::{ @@ -51,8 +53,8 @@ where // instance until that test returns. let db = &ctx.db; - let graph = Graph::new(db.clone()); - let service = SbomService::new(db.clone(), PaginationCache::for_test()); + let graph = Graph::new(); + let service = SbomService::new(PaginationCache::for_test()); let start = Instant::now(); let sbom = info_span!("parse json") diff --git a/modules/fundamental/tests/sbom/reingest.rs b/modules/fundamental/tests/sbom/reingest.rs index faa744eed..f15cf792d 100644 --- a/modules/fundamental/tests/sbom/reingest.rs +++ b/modules/fundamental/tests/sbom/reingest.rs @@ -4,11 +4,12 @@ use std::str::FromStr; use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::db::query::Query; -use trustify_common::id::Id; -use trustify_common::model::Paginated; -use trustify_common::purl::Purl; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + id::Id, + model::Paginated, + purl::Purl, +}; use trustify_module_fundamental::sbom::model::SbomExternalPackageReference; use trustify_module_fundamental::sbom::{model::details::SbomDetails, service::SbomService}; use trustify_module_ingestor::service::{Cache, Format}; @@ -34,7 +35,7 @@ fn assert_by_cleaning_id(sbom1: &mut SbomDetails, sbom2: &mut SbomDetails) { #[instrument] #[test(tokio::test)] async fn quarkus(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom = SbomService::new(PaginationCache::for_test()); // ingest the first version let result1 = ctx @@ -113,7 +114,7 @@ async fn quarkus(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { #[instrument] #[test(tokio::test)] async fn nhc(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom = SbomService::new(PaginationCache::for_test()); // ingest the first version let result1 = ctx.ingest_document("nhc/v1/nhc-0.4.z.json.xz").await?; @@ -164,7 +165,7 @@ async fn nhc(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { #[instrument] #[test(tokio::test)] async fn nhc_same(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom = SbomService::new(PaginationCache::for_test()); // ingest the first version let result1 = ctx.ingest_document("nhc/v1/nhc-0.4.z.json.xz").await?; @@ -218,7 +219,7 @@ async fn nhc_same(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { #[instrument] #[test(tokio::test)] async fn nhc_same_content(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom = SbomService::new(PaginationCache::for_test()); // ingest the first version let result1 = ctx.ingest_document("nhc/v1/nhc-0.4.z.json.xz").await?; @@ -295,7 +296,7 @@ async fn nhc_same_content(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { #[instrument] #[test(tokio::test)] async fn syft_rerun(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom = SbomService::new(PaginationCache::for_test()); // ingest the first version let result1 = ctx.ingest_document("syft-ubi-example/v1.json.xz").await?; diff --git a/modules/fundamental/tests/sbom/spdx.rs b/modules/fundamental/tests/sbom/spdx.rs index cf051ac19..ab8d3aba8 100644 --- a/modules/fundamental/tests/sbom/spdx.rs +++ b/modules/fundamental/tests/sbom/spdx.rs @@ -14,9 +14,10 @@ use test_context::test_context; use test_log::test; use time::OffsetDateTime; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::model::Paginated; -use trustify_common::{id::Id, purl::Purl, sbom::spdx::parse_spdx}; +use trustify_common::{ + db::pagination_cache::PaginationCache, id::Id, model::Paginated, purl::Purl, + sbom::spdx::parse_spdx, +}; use trustify_entity::relationship::Relationship; use trustify_module_fundamental::sbom::model::SbomPackage; use trustify_module_fundamental::{ @@ -134,7 +135,7 @@ async fn test_parse_spdx(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_spdx_broken_refs(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let sbom = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let sbom = SbomService::new(PaginationCache::for_test()); let err = ctx .ingest_document("spdx/broken-refs.json") diff --git a/modules/fundamental/tests/sbom/spdx/aliases.rs b/modules/fundamental/tests/sbom/spdx/aliases.rs index 644be4842..0760d8c1d 100644 --- a/modules/fundamental/tests/sbom/spdx/aliases.rs +++ b/modules/fundamental/tests/sbom/spdx/aliases.rs @@ -1,6 +1,7 @@ use itertools::Itertools; use test_context::test_context; use test_log::test; +use trustify_common::db::ReadOnly; use trustify_common::model::Paginated; use trustify_module_analysis::{ config::AnalysisConfig, @@ -16,7 +17,7 @@ async fn cpe_purl(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { .ingest_document("spdx/openssl-3.0.7-18.el9_2.spdx.alias.json") .await?; - let service = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let service = AnalysisService::new(AnalysisConfig::default(), ReadOnly::new(ctx.db.clone())); let result = service .retrieve( diff --git a/modules/fundamental/tests/sbom/spdx/corner_cases.rs b/modules/fundamental/tests/sbom/spdx/corner_cases.rs index 92cb48364..eae189b15 100644 --- a/modules/fundamental/tests/sbom/spdx/corner_cases.rs +++ b/modules/fundamental/tests/sbom/spdx/corner_cases.rs @@ -9,9 +9,7 @@ use sea_orm::ConnectionTrait; use strum::VariantArray; use test_context::test_context; use test_log::test; -use trustify_common::id::Id; -use trustify_common::model::Paginated; -use trustify_common::purl::Purl; +use trustify_common::{id::Id, model::Paginated, purl::Purl}; use trustify_entity::relationship::Relationship; use trustify_module_fundamental::sbom::model::SbomPackage; use trustify_module_fundamental::{ @@ -42,7 +40,7 @@ async fn related_packages_transitively<'a, C: ConnectionTrait>( #[test_context(TrustifyContext)] #[test(tokio::test)] async fn infinite_loop(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let result = ctx.ingest_document("spdx/loop.json").await?; @@ -114,7 +112,7 @@ async fn double_ref(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { .await? .expect("must be found"); - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .fetch_sbom_packages( id, @@ -159,7 +157,7 @@ async fn self_ref(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { .await? .expect("must be found"); - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .fetch_sbom_packages( id, @@ -204,7 +202,7 @@ async fn self_ref_package(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { .await? .expect("must be found"); - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .fetch_sbom_packages( id, @@ -252,7 +250,7 @@ async fn special_char(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { bail!("must be an id") }; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .fetch_sbom_packages( id, diff --git a/modules/fundamental/tests/sbom/spdx/external/rh.rs b/modules/fundamental/tests/sbom/spdx/external/rh.rs index f3d73420d..8e89edd49 100644 --- a/modules/fundamental/tests/sbom/spdx/external/rh.rs +++ b/modules/fundamental/tests/sbom/spdx/external/rh.rs @@ -21,7 +21,7 @@ async fn spdx_prod_comp(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let _prod: Uuid = result.pop().unwrap().id.parse().expect("must have a uid"); let _comp: Uuid = result.pop().unwrap().id.parse().expect("must have a uid"); - let _service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let _service = SbomService::new(PaginationCache::for_test()); // TODO: implement when we have the tools diff --git a/modules/fundamental/tests/sbom/spdx/issue_1417.rs b/modules/fundamental/tests/sbom/spdx/issue_1417.rs index 62c601bcf..6f131e49a 100644 --- a/modules/fundamental/tests/sbom/spdx/issue_1417.rs +++ b/modules/fundamental/tests/sbom/spdx/issue_1417.rs @@ -1,8 +1,10 @@ use anyhow::bail; use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::query::Query, model::Limit}; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + model::Limit, +}; use trustify_module_fundamental::sbom::service::SbomService; use trustify_test_context::TrustifyContext; @@ -18,7 +20,7 @@ async fn multi_purls(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { bail!("must be an id") }; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let sbom = service .fetch_sbom_packages( diff --git a/modules/fundamental/tests/sbom/spdx/parallel.rs b/modules/fundamental/tests/sbom/spdx/parallel.rs index efbf31aeb..81d78fbba 100644 --- a/modules/fundamental/tests/sbom/spdx/parallel.rs +++ b/modules/fundamental/tests/sbom/spdx/parallel.rs @@ -1,8 +1,10 @@ use test_context::test_context; use test_log::test; use tracing::instrument; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::{db::query::Query, model::Paginated}; +use trustify_common::{ + db::{pagination_cache::PaginationCache, query::Query}, + model::Paginated, +}; use trustify_module_fundamental::sbom::model::SbomPackage; use trustify_module_fundamental::sbom::service::SbomService; use trustify_test_context::TrustifyContext; @@ -20,7 +22,7 @@ async fn ingest_10(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { futures_util::future::try_join_all(f).await?; - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let result = service .fetch_sboms::<_, SbomPackage>( diff --git a/modules/fundamental/tests/sbom/spdx/reingest.rs b/modules/fundamental/tests/sbom/spdx/reingest.rs index 3e9e4ffe6..63fcc8ef9 100644 --- a/modules/fundamental/tests/sbom/spdx/reingest.rs +++ b/modules/fundamental/tests/sbom/spdx/reingest.rs @@ -4,8 +4,7 @@ use anyhow::bail; use sea_orm::EntityTrait; use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; -use trustify_common::model::Paginated; +use trustify_common::{db::pagination_cache::PaginationCache, model::Paginated}; use trustify_entity::sbom; use trustify_module_fundamental::sbom::model::SbomPackage; use trustify_module_fundamental::sbom::service::SbomService; @@ -25,7 +24,7 @@ async fn reingest(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { .await? .expect("must be found"); - let service = SbomService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = SbomService::new(PaginationCache::for_test()); let packages = service .describes_packages::<_, _, SbomPackage>( diff --git a/modules/fundamental/tests/weakness/mod.rs b/modules/fundamental/tests/weakness/mod.rs index a688094c9..be6922684 100644 --- a/modules/fundamental/tests/weakness/mod.rs +++ b/modules/fundamental/tests/weakness/mod.rs @@ -4,6 +4,7 @@ use roxmltree::Document; use std::io::Read; use test_context::test_context; use test_log::test; +use trustify_common::db::ReadOnly; use trustify_common::db::pagination_cache::PaginationCache; use trustify_common::{hashing::HashingRead, model::Paginated}; use trustify_entity::labels::Labels; @@ -18,7 +19,8 @@ async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { const TOTAL_ITEMS_FOUND: u64 = 964; let loader = CweCatalogLoader::new(); - let service = WeaknessService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = WeaknessService::new(PaginationCache::for_test()); + let db_ro = ReadOnly::new(ctx.db.clone()); // extract document from zip file @@ -42,6 +44,7 @@ async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { // fetch data + let tx = db_ro.begin().await?; let all = service .list_weaknesses( Default::default(), @@ -50,13 +53,14 @@ async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { limit: 10, total: true, }, + &tx, ) .await?; assert_eq!(Some(TOTAL_ITEMS_FOUND), all.total); let w = service - .get_weakness("CWE-1004") + .get_weakness("CWE-1004", &tx) .await? .expect("must be found"); @@ -80,6 +84,7 @@ async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { // fetch data again + let tx = db_ro.begin().await?; let all = service .list_weaknesses( Default::default(), @@ -88,6 +93,7 @@ async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { limit: 10, total: true, }, + &tx, ) .await?; @@ -96,7 +102,7 @@ async fn simple(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { assert_eq!(Some(964), all.total); let w = service - .get_weakness("CWE-1004") + .get_weakness("CWE-1004", &tx) .await? .expect("must be found"); diff --git a/modules/importer/src/endpoints.rs b/modules/importer/src/endpoints.rs index 6df2388db..a693261b8 100644 --- a/modules/importer/src/endpoints.rs +++ b/modules/importer/src/endpoints.rs @@ -11,7 +11,7 @@ use trustify_auth::{ CreateImporter, DeleteImporter, ReadImporter, UpdateImporter, authorizer::Require, }; use trustify_common::{ - db::{Database, pagination_cache::PaginationCache, query::Query}, + db::{self, pagination_cache::PaginationCache, query::Query}, endpoints::extract_revision, model::{Paginated, PaginatedResults, Revisioned}, }; @@ -19,7 +19,7 @@ use trustify_common::{ /// Mount the "importer" module. pub fn configure( svc: &mut utoipa_actix_web::service_config::ServiceConfig, - db: Database, + db: db::ReadWrite, cache: PaginationCache, ) { svc.app_data(web::Data::new(ImporterService::new(db, cache))) diff --git a/modules/importer/src/runner/clearly_defined/mod.rs b/modules/importer/src/runner/clearly_defined/mod.rs index 472810c57..f2e4583e4 100644 --- a/modules/importer/src/runner/clearly_defined/mod.rs +++ b/modules/importer/src/runner/clearly_defined/mod.rs @@ -19,11 +19,8 @@ impl super::ImportRunner { clearly_defined: ClearlyDefinedImporter, continuation: serde_json::Value, ) -> Result { - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let report = Arc::new(Mutex::new(ReportBuilder::new())); let continuation = serde_json::from_value(continuation).unwrap_or_default(); diff --git a/modules/importer/src/runner/clearly_defined/walker.rs b/modules/importer/src/runner/clearly_defined/walker.rs index 87fbf4922..b7cf4196e 100644 --- a/modules/importer/src/runner/clearly_defined/walker.rs +++ b/modules/importer/src/runner/clearly_defined/walker.rs @@ -7,7 +7,7 @@ use std::io::{BufRead, Read}; use std::sync::Arc; use tokio::sync::Mutex; use tokio_util::bytes::Buf; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::{Cache, Format, IngestorService}; @@ -15,7 +15,7 @@ pub struct ClearlyDefinedWalker { continuation: ClearlyDefinedItemContinuation, source: String, ingestor: IngestorService, - db: Database, + db: ReadWrite, progress: P, report: Arc>, coordinates_seen_this_run: HashSet, @@ -26,7 +26,7 @@ impl ClearlyDefinedWalker

{ pub fn new( source: impl Into, ingestor: IngestorService, - db: Database, + db: ReadWrite, report: Arc>, progress: P, ) -> Self { @@ -199,6 +199,7 @@ mod test { use test_context::test_context; use test_log::test; use tokio::sync::Mutex; + use trustify_common::db::ReadWrite; use trustify_test_context::TrustifyContext; #[test_context(TrustifyContext)] @@ -207,7 +208,7 @@ mod test { let mut walker = ClearlyDefinedWalker::new( "https://clearlydefinedprod.blob.core.windows.net/changes-notifications/", ctx.ingestor.clone(), - ctx.db.clone(), + ReadWrite::new(ctx.db.clone()), Arc::new(Mutex::new(ReportBuilder::new())), (), ); diff --git a/modules/importer/src/runner/clearly_defined_curation/mod.rs b/modules/importer/src/runner/clearly_defined_curation/mod.rs index 0e36c5d8b..25425523a 100644 --- a/modules/importer/src/runner/clearly_defined_curation/mod.rs +++ b/modules/importer/src/runner/clearly_defined_curation/mod.rs @@ -14,7 +14,7 @@ use parking_lot::Mutex; use std::{path::Path, path::PathBuf, sync::Arc}; use tokio::runtime::Handle; use tracing::instrument; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::Cache; use trustify_module_ingestor::{ @@ -28,7 +28,7 @@ struct Context { labels: Labels, report: Arc>, ingestor: IngestorService, - db: Database, + db: ReadWrite, } impl Context { @@ -90,11 +90,8 @@ impl super::ImportRunner { clearly_defined: ClearlyDefinedCurationImporter, continuation: serde_json::Value, ) -> Result { - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let report = Arc::new(Mutex::new(ReportBuilder::new())); let continuation = serde_json::from_value(continuation).unwrap_or_default(); diff --git a/modules/importer/src/runner/common/heartbeat.rs b/modules/importer/src/runner/common/heartbeat.rs index ebf6ff256..132f4b4bd 100644 --- a/modules/importer/src/runner/common/heartbeat.rs +++ b/modules/importer/src/runner/common/heartbeat.rs @@ -9,7 +9,7 @@ use tokio::{ time::{Duration, interval}, }; use tokio_util::sync::CancellationToken; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::importer; pub struct Heart { @@ -20,7 +20,7 @@ pub struct Heart { impl Heart { pub const RATE: Duration = Duration::from_secs(10); - pub fn new(importer: Importer, db: Database, future: F, token: CancellationToken) -> Self + pub fn new(importer: Importer, db: ReadWrite, future: F, token: CancellationToken) -> Self where F: Future + 'static, { @@ -35,7 +35,7 @@ impl Heart { // Attempts to acquire exclusive optimistic lock. Upon success, // spawns the future, and regularly updates the lock until the // future completes. - async fn pump(importer: Importer, db: Database, future: F, token: CancellationToken) + async fn pump(importer: Importer, db: ReadWrite, future: F, token: CancellationToken) where F: Future + 'static, { @@ -73,7 +73,7 @@ impl Heart { // record with the current time, but only if the db matches the // state of the &Importer parameter, otherwise an error is // returned. Upon success, the updated Importer is returned. - async fn beat(importer: &Importer, db: &Database) -> Result { + async fn beat(importer: &Importer, db: &ReadWrite) -> Result { let t = OffsetDateTime::now_utc().unix_timestamp_nanos(); let model = importer::ActiveModel { name: Set(importer.name.to_owned()), diff --git a/modules/importer/src/runner/csaf/mod.rs b/modules/importer/src/runner/csaf/mod.rs index 619f810a2..4951e25ab 100644 --- a/modules/importer/src/runner/csaf/mod.rs +++ b/modules/importer/src/runner/csaf/mod.rs @@ -63,11 +63,8 @@ impl super::ImportRunner { }; // storage (called by validator) - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let storage = storage::StorageVisitor { context, diff --git a/modules/importer/src/runner/csaf/storage.rs b/modules/importer/src/runner/csaf/storage.rs index eb86bf9fa..5d54c71d0 100644 --- a/modules/importer/src/runner/csaf/storage.rs +++ b/modules/importer/src/runner/csaf/storage.rs @@ -5,7 +5,7 @@ use csaf_walker::{ }; use parking_lot::Mutex; use std::sync::Arc; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::{Cache, Format, IngestorService}; use walker_common::utils::url::Urlify; @@ -13,7 +13,7 @@ use walker_common::utils::url::Urlify; pub struct StorageVisitor { pub context: C, pub ingestor: IngestorService, - pub db: Database, + pub db: ReadWrite, /// the report to report our messages to pub report: Arc>, pub labels: Labels, diff --git a/modules/importer/src/runner/cve/mod.rs b/modules/importer/src/runner/cve/mod.rs index 4e02041f8..1dfb601f3 100644 --- a/modules/importer/src/runner/cve/mod.rs +++ b/modules/importer/src/runner/cve/mod.rs @@ -14,7 +14,7 @@ use parking_lot::Mutex; use std::{path::Path, path::PathBuf, sync::Arc}; use tokio::runtime::Handle; use tracing::instrument; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::Cache; use trustify_module_ingestor::{ @@ -28,7 +28,7 @@ struct Context { labels: Labels, report: Arc>, ingestor: IngestorService, - db: Database, + db: ReadWrite, } impl Context { @@ -90,11 +90,8 @@ impl super::ImportRunner { cve: CveImporter, continuation: serde_json::Value, ) -> Result { - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let report = Arc::new(Mutex::new(ReportBuilder::new())); let continuation = serde_json::from_value(continuation).unwrap_or_default(); diff --git a/modules/importer/src/runner/cwe/mod.rs b/modules/importer/src/runner/cwe/mod.rs index 54f0a8014..58896eae3 100644 --- a/modules/importer/src/runner/cwe/mod.rs +++ b/modules/importer/src/runner/cwe/mod.rs @@ -20,11 +20,8 @@ impl super::ImportRunner { cwe_catalog: CweImporter, continuation: serde_json::Value, ) -> Result { - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let report = Arc::new(Mutex::new(ReportBuilder::new())); let continuation = serde_json::from_value(continuation).unwrap_or_default(); diff --git a/modules/importer/src/runner/cwe/walker.rs b/modules/importer/src/runner/cwe/walker.rs index 9e42b7a79..5c09cbe2a 100644 --- a/modules/importer/src/runner/cwe/walker.rs +++ b/modules/importer/src/runner/cwe/walker.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use tokio::sync::Mutex; use tokio_util::bytes::Buf; use tracing::instrument; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::{Cache, Format, IngestorService}; use zip::ZipArchive; @@ -17,7 +17,7 @@ pub struct CweWalker { continuation: LastModified, source: String, ingestor: IngestorService, - db: Database, + db: ReadWrite, report: Arc>, } @@ -25,7 +25,7 @@ impl CweWalker { pub fn new( source: impl Into, ingestor: IngestorService, - db: Database, + db: ReadWrite, report: Arc>, ) -> Self { Self { diff --git a/modules/importer/src/runner/mod.rs b/modules/importer/src/runner/mod.rs index 0b6b97835..60f1eb4cb 100644 --- a/modules/importer/src/runner/mod.rs +++ b/modules/importer/src/runner/mod.rs @@ -20,13 +20,13 @@ use crate::{ use std::path::PathBuf; use time::OffsetDateTime; use tracing::instrument; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_module_analysis::service::AnalysisService; use trustify_module_storage::service::dispatch::DispatchBackend; #[derive(Clone)] pub struct ImportRunner { - pub db: Database, + pub db: ReadWrite, pub storage: DispatchBackend, pub working_dir: Option, pub analysis: Option, diff --git a/modules/importer/src/runner/osv/mod.rs b/modules/importer/src/runner/osv/mod.rs index 773ed9d27..54d97525c 100644 --- a/modules/importer/src/runner/osv/mod.rs +++ b/modules/importer/src/runner/osv/mod.rs @@ -16,7 +16,7 @@ use std::collections::HashSet; use std::{path::Path, path::PathBuf, sync::Arc}; use tokio::runtime::Handle; use tracing::instrument; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::Cache; use trustify_module_ingestor::{ @@ -32,7 +32,7 @@ struct Context { start_year: Option, report: Arc>, ingestor: IngestorService, - db: Database, + db: ReadWrite, } impl Context { @@ -117,11 +117,8 @@ impl super::ImportRunner { osv: OsvImporter, continuation: serde_json::Value, ) -> Result { - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let report = Arc::new(Mutex::new(ReportBuilder::new())); let continuation = serde_json::from_value(continuation).unwrap_or_default(); diff --git a/modules/importer/src/runner/quay/mod.rs b/modules/importer/src/runner/quay/mod.rs index 41c6edf8e..e255a58b3 100644 --- a/modules/importer/src/runner/quay/mod.rs +++ b/modules/importer/src/runner/quay/mod.rs @@ -21,11 +21,8 @@ impl super::ImportRunner { quay: QuayImporter, continuation: serde_json::Value, ) -> Result { - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let report = Arc::new(Mutex::new(ReportBuilder::new())); let continuation = serde_json::from_value(continuation).unwrap_or_default(); diff --git a/modules/importer/src/runner/quay/walker.rs b/modules/importer/src/runner/quay/walker.rs index 013e03519..f78fcbee9 100644 --- a/modules/importer/src/runner/quay/walker.rs +++ b/modules/importer/src/runner/quay/walker.rs @@ -17,7 +17,7 @@ use std::{collections::HashMap, future, sync::Arc}; use time::OffsetDateTime; use tokio::sync::Mutex; use tracing::instrument; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::{Cache, Format, IngestorService}; @@ -31,7 +31,7 @@ pub struct QuayWalker { continuation: LastModified, importer: QuayImporter, ingestor: IngestorService, - db: Database, + db: ReadWrite, report: Arc>, client: reqwest::Client, oci: oci::Client, @@ -42,7 +42,7 @@ impl QuayWalker { pub fn new( importer: QuayImporter, ingestor: IngestorService, - db: Database, + db: ReadWrite, report: Arc>, context: C, ) -> Result { @@ -317,6 +317,7 @@ mod test { use super::*; use test_context::test_context; use test_log::test; + use trustify_common::db::ReadWrite; use trustify_test_context::TrustifyContext; use wiremock::{ Mock, MockServer, ResponseTemplate, @@ -334,7 +335,7 @@ mod test { ..Default::default() }, ctx.ingestor.clone(), - ctx.db.clone(), + ReadWrite::new(ctx.db.clone()), Arc::new(Mutex::new(ReportBuilder::new())), (), )? @@ -394,7 +395,7 @@ mod test { ..Default::default() }, ctx.ingestor.clone(), - ctx.db.clone(), + ReadWrite::new(ctx.db.clone()), report.clone(), (), )?; @@ -438,7 +439,7 @@ mod test { ..Default::default() }, ctx.ingestor.clone(), - ctx.db.clone(), + ReadWrite::new(ctx.db.clone()), report.clone(), (), )?; @@ -461,7 +462,7 @@ mod test { ..Default::default() }, ctx.ingestor.clone(), - ctx.db.clone(), + ReadWrite::new(ctx.db.clone()), Arc::new(Mutex::new(ReportBuilder::new())), (), )?; diff --git a/modules/importer/src/runner/sbom/mod.rs b/modules/importer/src/runner/sbom/mod.rs index cf7f9972f..b5f6fc580 100644 --- a/modules/importer/src/runner/sbom/mod.rs +++ b/modules/importer/src/runner/sbom/mod.rs @@ -65,11 +65,8 @@ impl super::ImportRunner { // storage (called by validator) - let ingestor = IngestorService::new( - Graph::new(self.db.clone()), - self.storage.clone(), - self.analysis.clone(), - ); + let ingestor = + IngestorService::new(Graph::new(), self.storage.clone(), self.analysis.clone()); let storage = storage::StorageVisitor { context, source, diff --git a/modules/importer/src/runner/sbom/storage.rs b/modules/importer/src/runner/sbom/storage.rs index c5c6aea12..b756b7dac 100644 --- a/modules/importer/src/runner/sbom/storage.rs +++ b/modules/importer/src/runner/sbom/storage.rs @@ -10,7 +10,7 @@ use sbom_walker::{ validation::{ValidatedSbom, ValidatedVisitor, ValidationContext}, }; use std::sync::Arc; -use trustify_common::db::Database; +use trustify_common::db::ReadWrite; use trustify_entity::labels::Labels; use trustify_module_ingestor::service::{Cache, Format, IngestorService}; use walker_common::utils::url::Urlify; @@ -22,7 +22,7 @@ pub struct StorageVisitor { pub max_size: Option, pub labels: Labels, pub ingestor: IngestorService, - pub db: Database, + pub db: ReadWrite, /// the report to report our messages to pub report: Arc>, } diff --git a/modules/importer/src/server/mod.rs b/modules/importer/src/server/mod.rs index c6e19047b..910898392 100644 --- a/modules/importer/src/server/mod.rs +++ b/modules/importer/src/server/mod.rs @@ -17,7 +17,7 @@ use time::OffsetDateTime; use tokio::{task::LocalSet, time::MissedTickBehavior}; use tokio_util::sync::CancellationToken; use tracing::instrument; -use trustify_common::db::{Database, pagination_cache::PaginationCache}; +use trustify_common::db::{ReadWrite, pagination_cache::PaginationCache}; use trustify_module_analysis::service::AnalysisService; use trustify_module_storage::service::dispatch::DispatchBackend; @@ -25,7 +25,7 @@ use trustify_module_storage::service::dispatch::DispatchBackend; /// /// When `read_only` is true, the loop stays alive but no imports are started. pub async fn importer( - db: Database, + db: ReadWrite, cache: PaginationCache, storage: DispatchBackend, working_dir: Option, @@ -63,7 +63,7 @@ impl From for RunOutput { /// Single node, single process importer processor. struct Server { - db: Database, + db: ReadWrite, cache: PaginationCache, storage: DispatchBackend, working_dir: Option, diff --git a/modules/importer/src/service.rs b/modules/importer/src/service.rs index 215fbce4e..fece8f357 100644 --- a/modules/importer/src/service.rs +++ b/modules/importer/src/service.rs @@ -10,7 +10,7 @@ use time::OffsetDateTime; use tracing::instrument; use trustify_common::{ db::{ - Database, DatabaseErrors, + DatabaseErrors, ReadWrite, limiter::{LimitedResult, LimiterTrait}, pagination_cache::PaginationCache, query::{Filtering, Query}, @@ -126,12 +126,13 @@ where #[derive(Clone, Debug)] pub struct ImporterService { - db: Database, + db: ReadWrite, cache: PaginationCache, } impl ImporterService { - pub fn new(db: Database, cache: PaginationCache) -> Self { + /// Creates a new importer service backed by the given read-write connection. + pub fn new(db: ReadWrite, cache: PaginationCache) -> Self { Self { db, cache } } diff --git a/modules/importer/src/test.rs b/modules/importer/src/test.rs index a960b2967..b168a5e38 100644 --- a/modules/importer/src/test.rs +++ b/modules/importer/src/test.rs @@ -14,7 +14,7 @@ use serde_json::json; use std::time::Duration; use test_context::test_context; use test_log::test; -use trustify_common::db::pagination_cache::PaginationCache; +use trustify_common::db::{self, pagination_cache::PaginationCache}; use trustify_test_context::{ReadOnly, TrustifyContext, app::TestApp}; use utoipa_actix_web::AppExt; @@ -58,7 +58,7 @@ fn mock_importer(result: &Importer, source: impl Into) -> Importer { async fn app( ctx: &TrustifyContext, ) -> impl Service, Error = actix_web::Error> { - let db = ctx.db.clone(); + let db = db::ReadWrite::new(ctx.db.clone()); actix::init_service( App::new() .into_utoipa_app() diff --git a/modules/ingestor/src/endpoints.rs b/modules/ingestor/src/endpoints.rs index 69cf600ca..64f26c429 100644 --- a/modules/ingestor/src/endpoints.rs +++ b/modules/ingestor/src/endpoints.rs @@ -3,8 +3,9 @@ use crate::{ service::{Error, IngestorService}, }; use actix_web::{HttpResponse, Responder, post, web}; +use sea_orm::TransactionTrait; use trustify_auth::{UploadDataset, authorizer::Require}; -use trustify_common::{db::Database, model::BinaryData}; +use trustify_common::{db, model::BinaryData}; use trustify_entity::labels::Labels; use trustify_module_analysis::service::AnalysisService; use trustify_module_storage::service::dispatch::DispatchBackend; @@ -14,11 +15,11 @@ use utoipa::IntoParams; pub fn configure( svc: &mut utoipa_actix_web::service_config::ServiceConfig, config: Config, - db: Database, + db: db::ReadWrite, storage: impl Into, analysis: Option, ) { - let ingestor_service = IngestorService::new(Graph::new(db.clone()), storage, analysis); + let ingestor_service = IngestorService::new(Graph::new(), storage, analysis); svc.app_data(web::Data::new(ingestor_service)) .app_data(web::Data::new(config)) @@ -58,18 +59,16 @@ struct UploadParams { pub async fn upload_dataset( service: web::Data, config: web::Data, - db: web::Data, + db: web::Data, web::Query(UploadParams { labels }): web::Query, bytes: web::Bytes, _: Require, ) -> Result { - let result = db - .transaction(async |tx| { - service - .ingest_dataset(&bytes, labels, config.dataset_entry_limit, tx) - .await - }) + let tx = db.begin().await?; + let result = service + .ingest_dataset(&bytes, labels, config.dataset_entry_limit, &tx) .await?; + tx.commit().await?; Ok(HttpResponse::Created().json(result)) } diff --git a/modules/ingestor/src/graph/advisory/advisory_vulnerability.rs b/modules/ingestor/src/graph/advisory/advisory_vulnerability.rs index 095f01449..ffde71a94 100644 --- a/modules/ingestor/src/graph/advisory/advisory_vulnerability.rs +++ b/modules/ingestor/src/graph/advisory/advisory_vulnerability.rs @@ -110,7 +110,7 @@ mod test { async fn advisory_affected_vulnerability_assertions( ctx: TrustifyContext, ) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let advisory = system .ingest_advisory( @@ -172,7 +172,7 @@ mod test { async fn advisory_not_affected_vulnerability_assertions( ctx: TrustifyContext, ) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let advisory = system .ingest_advisory( diff --git a/modules/ingestor/src/graph/advisory/mod.rs b/modules/ingestor/src/graph/advisory/mod.rs index 0e20227c5..6abd9edaf 100644 --- a/modules/ingestor/src/graph/advisory/mod.rs +++ b/modules/ingestor/src/graph/advisory/mod.rs @@ -344,7 +344,7 @@ mod test { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn ingest_advisories(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let advisory1 = system .ingest_advisory( @@ -385,7 +385,7 @@ mod test { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn ingest_advisory_cve(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let advisory = system .ingest_advisory( @@ -433,7 +433,7 @@ mod test { } } - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let a1 = system .ingest_advisory( diff --git a/modules/ingestor/src/graph/cpe.rs b/modules/ingestor/src/graph/cpe.rs index afe74f82c..018a8822d 100644 --- a/modules/ingestor/src/graph/cpe.rs +++ b/modules/ingestor/src/graph/cpe.rs @@ -131,7 +131,7 @@ mod test { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn ingest_cpe(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let cpe = Cpe::from_str("cpe:/a:redhat:enterprise_linux:9::crb")?; diff --git a/modules/ingestor/src/graph/mod.rs b/modules/ingestor/src/graph/mod.rs index 2a14f1606..6c28c5345 100644 --- a/modules/ingestor/src/graph/mod.rs +++ b/modules/ingestor/src/graph/mod.rs @@ -26,7 +26,7 @@ use trustify_common::hashing::Digests; use trustify_entity::source_document; use uuid::Uuid; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct Graph { pub(crate) db_context: Arc>, } @@ -40,7 +40,7 @@ pub enum Error { } impl Graph { - pub fn new(_db: trustify_common::db::Database) -> Self { + pub fn new() -> Self { Self { db_context: Arc::new(Mutex::new(DbContext::new())), } diff --git a/modules/ingestor/src/graph/purl/mod.rs b/modules/ingestor/src/graph/purl/mod.rs index e4cdb9a89..4d1422053 100644 --- a/modules/ingestor/src/graph/purl/mod.rs +++ b/modules/ingestor/src/graph/purl/mod.rs @@ -398,7 +398,7 @@ mod tests { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn ingest_packages(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let pkg1 = system .ingest_package(&"pkg:maven/io.quarkus/quarkus-core".try_into()?, &ctx.db) @@ -424,7 +424,7 @@ mod tests { async fn ingest_package_versions_missing_version( ctx: TrustifyContext, ) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let result = system .ingest_package_version(&"pkg:maven/io.quarkus/quarkus-addons".try_into()?, &ctx.db) @@ -438,7 +438,7 @@ mod tests { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn ingest_package_versions(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let pkg1 = system .ingest_package_version( @@ -487,7 +487,7 @@ mod tests { #[case] expected_total: Option, #[case] expected_len: usize, ) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); for v in 0..200u64 { let version = format!("pkg:maven/io.quarkus/quarkus-core@{v}").try_into()?; @@ -524,7 +524,7 @@ mod tests { ) -> Result<(), anyhow::Error> { use std::time::Duration; - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); const TOTAL_ITEMS: u64 = 50; @@ -566,7 +566,7 @@ mod tests { use crate::graph::error::Error; use std::time::Duration; - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); for v in 0..10u64 { let version = format!("pkg:maven/io.quarkus/quarkus-core@{v}").try_into()?; @@ -606,7 +606,7 @@ mod tests { async fn ingest_qualified_packages_transactionally( ctx: TrustifyContext, ) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let tx_system = system.clone(); @@ -636,7 +636,7 @@ mod tests { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn ingest_qualified_packages(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let pkg1 = system .ingest_qualified_package( @@ -686,7 +686,7 @@ mod tests { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn query_qualified_packages(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); for i in [ "pkg:maven/io.quarkus/quarkus-core@1.2.3", diff --git a/modules/ingestor/src/graph/vulnerability/mod.rs b/modules/ingestor/src/graph/vulnerability/mod.rs index 016942a13..e4e1e7829 100644 --- a/modules/ingestor/src/graph/vulnerability/mod.rs +++ b/modules/ingestor/src/graph/vulnerability/mod.rs @@ -321,7 +321,7 @@ mod tests { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn ingest_cves(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let system = Graph::new(ctx.db.clone()); + let system = Graph::new(); let cve1 = system.ingest_vulnerability("CVE-123", (), &ctx.db).await?; let cve2 = system.ingest_vulnerability("CVE-123", (), &ctx.db).await?; diff --git a/modules/ingestor/src/service/advisory/csaf/loader.rs b/modules/ingestor/src/service/advisory/csaf/loader.rs index 1d3479d27..0bb9b1b80 100644 --- a/modules/ingestor/src/service/advisory/csaf/loader.rs +++ b/modules/ingestor/src/service/advisory/csaf/loader.rs @@ -249,7 +249,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn loader(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let tx = ctx.db.begin().await?; @@ -326,7 +326,7 @@ mod test { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn multiple_vulnerabilities(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let loader = CsafLoader::new(&graph); let (csaf, digests): (Csaf, _) = document("csaf/rhsa-2024_3666.json").await?; @@ -376,7 +376,7 @@ mod test { #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn product_status(ctx: TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let loader = CsafLoader::new(&graph); let (csaf, digests): (Csaf, _) = document("csaf/cve-2023-0044.json").await?; @@ -421,7 +421,7 @@ mod test { use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; use trustify_entity::{remediation, remediation_product_status}; - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let loader = CsafLoader::new(&graph); let (csaf, digests): (Csaf, _) = document("csaf/cve-2023-0044.json").await?; @@ -510,7 +510,7 @@ mod test { use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; use trustify_entity::{remediation, remediation_purl_status}; - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let loader = CsafLoader::new(&graph); let (csaf, digests): (Csaf, _) = document("csaf/rhsa-2024_3666.json").await?; diff --git a/modules/ingestor/src/service/advisory/cve/loader.rs b/modules/ingestor/src/service/advisory/cve/loader.rs index 0ec716072..f3b3e1431 100644 --- a/modules/ingestor/src/service/advisory/cve/loader.rs +++ b/modules/ingestor/src/service/advisory/cve/loader.rs @@ -582,7 +582,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn cve_loader(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let (cve, digests): (Cve, _) = document("mitre/CVE-2024-28111.json").await?; @@ -644,7 +644,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn divine_purls(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let (cve, digests): (Cve, _) = document("cve/CVE-2024-26308.json").await?; diff --git a/modules/ingestor/src/service/advisory/osv/loader.rs b/modules/ingestor/src/service/advisory/osv/loader.rs index 5a2e8420c..b4c2c951b 100644 --- a/modules/ingestor/src/service/advisory/osv/loader.rs +++ b/modules/ingestor/src/service/advisory/osv/loader.rs @@ -586,8 +586,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn loader(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let (osv, digests): (Vulnerability, _) = document("osv/RUSTSEC-2021-0079.json").await?; @@ -650,8 +649,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn loader_pypi(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let (osv, digests): (Vulnerability, _) = document("osv/GHSA-45c4-8wx5-qw6w.json").await?; @@ -715,8 +713,7 @@ mod test { let osv: Vulnerability = serde_json::from_str(osv_content)?; let digests = Digests::digest(osv_content.as_bytes()); - let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); // Load the OSV let loader = OsvLoader::new(&graph); @@ -755,7 +752,7 @@ mod test { #[test(tokio::test)] async fn loader_crates_io(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let (osv, digests): (Vulnerability, _) = document("osv/GHSA-434x-w66g-qw3r.json").await?; diff --git a/modules/ingestor/src/service/sbom/clearly_defined.rs b/modules/ingestor/src/service/sbom/clearly_defined.rs index 2f62e4e82..54f55737f 100644 --- a/modules/ingestor/src/service/sbom/clearly_defined.rs +++ b/modules/ingestor/src/service/sbom/clearly_defined.rs @@ -146,7 +146,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_clearly_defined(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let ingestor = IngestorService::new(graph, ctx.storage.clone(), Default::default()); let data = document_bytes("clearly-defined/aspnet.mvc-4.0.40804.json").await?; diff --git a/modules/ingestor/src/service/sbom/clearly_defined_curation.rs b/modules/ingestor/src/service/sbom/clearly_defined_curation.rs index daae2a78b..6fde3eb35 100644 --- a/modules/ingestor/src/service/sbom/clearly_defined_curation.rs +++ b/modules/ingestor/src/service/sbom/clearly_defined_curation.rs @@ -60,7 +60,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_clearly_defined(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let ingestor = IngestorService::new(graph, ctx.storage.clone(), Default::default()); let data = document_bytes("clearly-defined/chrono.yaml").await?; diff --git a/modules/ingestor/src/service/sbom/cyclonedx.rs b/modules/ingestor/src/service/sbom/cyclonedx.rs index 4a87400b8..c824f96f9 100644 --- a/modules/ingestor/src/service/sbom/cyclonedx.rs +++ b/modules/ingestor/src/service/sbom/cyclonedx.rs @@ -133,8 +133,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_cyclonedx(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let data = document_bytes("zookeeper-3.9.2-cyclonedx.json").await?; let ingestor = IngestorService::new(graph, ctx.storage.clone(), Default::default()); @@ -160,8 +159,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_ai_cyclonedx_nvidia(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let data = document_bytes("cyclonedx/ai/nvidia_canary-1b-v2_aibom.json").await?; let ingestor = IngestorService::new(graph, ctx.storage.clone(), Default::default()); @@ -209,8 +207,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_ai_cyclonedx_ibm(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let data = document_bytes("cyclonedx/ai/ibm-granite_granite-docling-258M_aibom.json").await?; @@ -237,8 +234,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_cryptographic_cyclonedx(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let db = &ctx.db; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let data = document_bytes("cyclonedx/cryptographic/cbom.json").await?; let ingestor = IngestorService::new(graph, ctx.storage.clone(), Default::default()); diff --git a/modules/ingestor/src/service/sbom/spdx.rs b/modules/ingestor/src/service/sbom/spdx.rs index 27d8200e4..f1e1a7c2f 100644 --- a/modules/ingestor/src/service/sbom/spdx.rs +++ b/modules/ingestor/src/service/sbom/spdx.rs @@ -82,7 +82,7 @@ mod test { #[test_context(TrustifyContext)] #[test(tokio::test)] async fn ingest_spdx(ctx: &TrustifyContext) -> Result<(), anyhow::Error> { - let graph = Graph::new(ctx.db.clone()); + let graph = Graph::new(); let data = document_bytes("ubi9-9.2-755.1697625012.json").await?; let ingestor = IngestorService::new(graph, ctx.storage.clone(), Default::default()); diff --git a/modules/ingestor/tests/common.rs b/modules/ingestor/tests/common.rs index 49acaaee3..9e26e6211 100644 --- a/modules/ingestor/tests/common.rs +++ b/modules/ingestor/tests/common.rs @@ -1,3 +1,4 @@ +use trustify_common::db; use trustify_module_analysis::{config::AnalysisConfig, service::AnalysisService}; use trustify_module_ingestor::endpoints::{Config, configure}; use trustify_test_context::{ @@ -9,12 +10,13 @@ pub async fn caller_with( ctx: &TrustifyContext, config: Config, ) -> anyhow::Result { - let analysis = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let analysis = + AnalysisService::new(AnalysisConfig::default(), db::ReadOnly::new(ctx.db.clone())); call::caller(|svc| { configure( svc, config, - ctx.db.clone(), + db::ReadWrite::new(ctx.db.clone()), ctx.storage.clone(), Some(analysis), ) diff --git a/modules/user/src/endpoints.rs b/modules/user/src/endpoints.rs index 8a4d0415f..0e7f7f1e5 100644 --- a/modules/user/src/endpoints.rs +++ b/modules/user/src/endpoints.rs @@ -4,12 +4,13 @@ use actix_web::{ http::header::{self, ETag, EntityTag, IfMatch}, put, web, }; +use sea_orm::TransactionTrait; use trustify_auth::authenticator::user::UserDetails; -use trustify_common::{db::Database, model::Revisioned}; +use trustify_common::{db, model::Revisioned}; /// mount the "user" module -pub fn configure(svc: &mut utoipa_actix_web::service_config::ServiceConfig, db: Database) { - svc.app_data(web::Data::new(UserPreferenceService::new(db))) +pub fn configure(svc: &mut utoipa_actix_web::service_config::ServiceConfig) { + svc.app_data(web::Data::new(UserPreferenceService::new())) .service(set) .service(get) .service(delete); @@ -37,10 +38,12 @@ pub fn configure(svc: &mut utoipa_actix_web::service_config::ServiceConfig, db: /// Get user preferences async fn get( service: web::Data, + db: web::Data, key: web::Path, user: UserDetails, -) -> Result { - Ok(match service.get(user.id, key.into_inner()).await? { +) -> actix_web::Result { + let tx = db.begin().await?; + Ok(match service.get(user.id, key.into_inner(), &tx).await? { Some(Revisioned { value, revision }) => HttpResponse::Ok() .append_header((header::ETAG, ETag(EntityTag::new_strong(revision)))) .json(value), @@ -71,6 +74,7 @@ async fn get( /// Set user preferences async fn set( service: web::Data, + db: web::Data, key: web::Path, user: UserDetails, web::Header(if_match): web::Header, @@ -85,7 +89,7 @@ async fn set( value: (), revision, } = service - .set(user.id, key.into_inner(), revision, data) + .set(user.id, key.into_inner(), revision, data, db.as_ref()) .await?; Ok(HttpResponse::NoContent() @@ -110,6 +114,7 @@ async fn set( /// Delete user preferences async fn delete( service: web::Data, + db: web::Data, key: web::Path, user: UserDetails, web::Header(if_match): web::Header, @@ -119,6 +124,10 @@ async fn delete( IfMatch::Items(items) => items.first().map(|etag| etag.tag()), }; - service.delete(user.id, key.into_inner(), revision).await?; + let tx = db.begin().await?; + service + .delete(user.id, key.into_inner(), revision, &tx) + .await?; + tx.commit().await?; Ok(HttpResponse::NoContent().finish()) } diff --git a/modules/user/src/service.rs b/modules/user/src/service.rs index 9eca825d9..c60698e44 100644 --- a/modules/user/src/service.rs +++ b/modules/user/src/service.rs @@ -1,12 +1,10 @@ use actix_web::{HttpResponse, ResponseError, body::BoxBody}; use sea_orm::{ - ActiveValue::Set, ColumnTrait, EntityTrait, PaginatorTrait, QueryFilter, TransactionTrait, + ActiveValue::Set, ColumnTrait, ConnectionTrait, EntityTrait, PaginatorTrait, QueryFilter, prelude::Uuid, }; use sea_query::{Alias, Expr, OnConflict}; -use trustify_common::{ - db::Database, db::DatabaseErrors, error::ErrorInformation, model::Revisioned, -}; +use trustify_common::{db::DatabaseErrors, error::ErrorInformation, model::Revisioned}; use trustify_entity::user_preferences; #[derive(Debug, thiserror::Error)] @@ -53,22 +51,23 @@ impl ResponseError for Error { } } -#[derive(Clone, Debug)] -pub struct UserPreferenceService { - db: Database, -} +#[derive(Clone, Debug, Default)] +pub struct UserPreferenceService; impl UserPreferenceService { - pub fn new(db: Database) -> Self { - Self { db } + /// Creates a new user preference service. + pub fn new() -> Self { + Self } + /// Sets a user preference, optionally checking the expected revision for optimistic concurrency. pub async fn set( &self, user_id: String, key: String, expected_revision: Option<&str>, data: serde_json::Value, + connection: &impl ConnectionTrait, ) -> Result, Error> { let next = Uuid::new_v4(); @@ -86,7 +85,7 @@ impl UserPreferenceService { .cast_as(Alias::new("text")) .eq(expected_revision), ) - .exec(&self.db) + .exec(connection) .await?; if result.rows_affected == 0 { @@ -117,7 +116,7 @@ impl UserPreferenceService { data: Set(data), }) .on_conflict(on_conflict) - .exec_without_returning(&self.db) + .exec_without_returning(connection) .await?; Ok(Revisioned { @@ -128,13 +127,15 @@ impl UserPreferenceService { } } + /// Retrieves a user preference by user ID and key. pub async fn get( &self, user_id: String, key: String, + connection: &impl ConnectionTrait, ) -> Result>, Error> { let result = user_preferences::Entity::find_by_id((user_id, key)) - .one(&self.db) + .one(connection) .await?; Ok(result.map(|result| Revisioned { @@ -143,11 +144,15 @@ impl UserPreferenceService { })) } + /// Deletes a user preference, optionally checking the expected revision. + /// + /// The caller must provide a transaction for atomicity when using revision checks. pub async fn delete( &self, user_id: String, key: String, expected_revision: Option<&str>, + connection: &impl ConnectionTrait, ) -> Result { let mut delete = user_preferences::Entity::delete_many() .filter(user_preferences::Column::UserId.eq(&user_id)) @@ -162,14 +167,12 @@ impl UserPreferenceService { ); } - let tx = self.db.begin().await?; - - let result = delete.exec(&tx).await?; + let result = delete.exec(connection).await?; - let result = if expected_revision.is_some() && result.rows_affected == 0 { + if expected_revision.is_some() && result.rows_affected == 0 { // now we need to figure out if the item wasn't there or if it was modified if user_preferences::Entity::find_by_id((user_id, key)) - .count(&tx) + .count(connection) .await? == 0 { @@ -179,10 +182,6 @@ impl UserPreferenceService { } } else { Ok(result.rows_affected > 0) - }; - - tx.commit().await?; - - result + } } } diff --git a/modules/user/src/test.rs b/modules/user/src/test.rs index d29b47197..d8ea05b5c 100644 --- a/modules/user/src/test.rs +++ b/modules/user/src/test.rs @@ -2,11 +2,12 @@ use crate::service::{Error, UserPreferenceService}; use actix_http::header; -use actix_web::{App, http::StatusCode, test as actix}; +use actix_web::{App, http::StatusCode, test as actix, web}; +use sea_orm::TransactionTrait; use serde_json::json; use test_context::test_context; use test_log::test; -use trustify_common::model::Revisioned; +use trustify_common::{db, model::Revisioned}; use trustify_test_context::TrustifyContext; use trustify_test_context::auth::TestAuthentication; use utoipa_actix_web::AppExt; @@ -14,29 +15,45 @@ use utoipa_actix_web::AppExt; #[test_context(TrustifyContext, skip_teardown)] #[test(tokio::test)] async fn collision(ctx: TrustifyContext) -> anyhow::Result<()> { - let service = UserPreferenceService::new(ctx.db.clone()); + let service = UserPreferenceService::new(); // initially it must be gone - let result = service.get("user-a".into(), "key-a".into()).await?; + let result = service + .get("user-a".into(), "key-a".into(), &ctx.db) + .await?; assert!(result.is_none()); - // setting one with an invalid revision should rais a mid air collision + // setting one with an invalid revision should raise a mid air collision let result = service - .set("user-a".into(), "key-a".into(), Some("a"), json!({"a": 1})) + .set( + "user-a".into(), + "key-a".into(), + Some("a"), + json!({"a": 1}), + &ctx.db, + ) .await; assert!(matches!(result, Result::Err(Error::MidAirCollision))); // now set a proper one service - .set("user-a".into(), "key-a".into(), None, json!({"a": 1})) + .set( + "user-a".into(), + "key-a".into(), + None, + json!({"a": 1}), + &ctx.db, + ) .await?; // we should be able to get it - let result = service.get("user-a".into(), "key-a".into()).await?; + let result = service + .get("user-a".into(), "key-a".into(), &ctx.db) + .await?; assert!(matches!( result, Some(Revisioned { @@ -48,13 +65,21 @@ async fn collision(ctx: TrustifyContext) -> anyhow::Result<()> { // try setting one again with an invalid revision let result = service - .set("user-a".into(), "key-a".into(), Some("a"), json!({"a": 1})) + .set( + "user-a".into(), + "key-a".into(), + Some("a"), + json!({"a": 1}), + &ctx.db, + ) .await; assert!(matches!(result, Result::Err(Error::MidAirCollision))); // must not change the data - let result = service.get("user-a".into(), "key-a".into()).await?; + let result = service + .get("user-a".into(), "key-a".into(), &ctx.db) + .await?; assert!(matches!( result, Some(Revisioned { @@ -66,12 +91,21 @@ async fn collision(ctx: TrustifyContext) -> anyhow::Result<()> { // now let's update the data service - .set("user-a".into(), "key-a".into(), None, json!({"a": 2})) + .set( + "user-a".into(), + "key-a".into(), + None, + json!({"a": 2}), + &ctx.db, + ) .await?; // it should change - let result = service.get("user-a".into(), "key-a".into()).await?.unwrap(); + let result = service + .get("user-a".into(), "key-a".into(), &ctx.db) + .await? + .unwrap(); assert!(matches!( result, Revisioned { @@ -88,12 +122,16 @@ async fn collision(ctx: TrustifyContext) -> anyhow::Result<()> { "key-a".into(), Some(&result.revision), json!({"a": 3}), + &ctx.db, ) .await?; // check result, must change - let result = service.get("user-a".into(), "key-a".into()).await?.unwrap(); + let result = service + .get("user-a".into(), "key-a".into(), &ctx.db) + .await? + .unwrap(); let Revisioned { value, revision } = result; assert!(matches!( value, @@ -102,29 +140,39 @@ async fn collision(ctx: TrustifyContext) -> anyhow::Result<()> { // try deleting wrong revision, must fail + let tx = ctx.db.begin().await?; let result = service - .delete("user-a".into(), "key-a".into(), Some("a")) + .delete("user-a".into(), "key-a".into(), Some("a"), &tx) .await; assert!(matches!(result, Result::Err(Error::MidAirCollision))); + tx.commit().await?; // try deleting correct revision, must succeed + let tx = ctx.db.begin().await?; let result = service - .delete("user-a".into(), "key-a".into(), Some(&revision)) + .delete("user-a".into(), "key-a".into(), Some(&revision), &tx) .await; assert!(matches!(result, Result::Ok(true))); + tx.commit().await?; // try deleting correct revision again, must succeed, but return false + let tx = ctx.db.begin().await?; let result = service - .delete("user-a".into(), "key-a".into(), Some(&revision)) + .delete("user-a".into(), "key-a".into(), Some(&revision), &tx) .await; assert!(matches!(result, Result::Ok(false))); + tx.commit().await?; // try deleting any revision, must succeed, but return false - let result = service.delete("user-a".into(), "key-a".into(), None).await; + let tx = ctx.db.begin().await?; + let result = service + .delete("user-a".into(), "key-a".into(), None, &tx) + .await; assert!(matches!(result, Result::Ok(false))); + tx.commit().await?; Ok(()) } @@ -132,14 +180,14 @@ async fn collision(ctx: TrustifyContext) -> anyhow::Result<()> { #[test_context(TrustifyContext, skip_teardown)] #[test(actix_web::test)] async fn wrong_rev(ctx: TrustifyContext) { - let db = &ctx.db; + let db_rw = db::ReadWrite::new(ctx.db.clone()); + let db_ro = db::ReadOnly::new(ctx.db.clone()); let app = actix::init_service( App::new() .into_utoipa_app() - .service( - utoipa_actix_web::scope("/api") - .configure(|svc| super::endpoints::configure(svc, db.clone())), - ) + .app_data(web::Data::new(db_rw)) + .app_data(web::Data::new(db_ro)) + .service(utoipa_actix_web::scope("/api").configure(super::endpoints::configure)) .into_app(), ) .await; diff --git a/server/src/openapi.rs b/server/src/openapi.rs index fc1586929..4e8bf4348 100644 --- a/server/src/openapi.rs +++ b/server/src/openapi.rs @@ -1,6 +1,6 @@ use crate::profile::api::{Config, ModuleConfig, configure, default_openapi_info}; use actix_web::App; -use trustify_common::db::pagination_cache::PaginationCache; +use trustify_common::db::{self, pagination_cache::PaginationCache}; use trustify_module_analysis::{config::AnalysisConfig, service::AnalysisService}; use trustify_module_storage::service::fs::FileSystemBackend; use utoipa_actix_web::AppExt; @@ -8,7 +8,9 @@ use utoipa_actix_web::AppExt; pub async fn create_openapi() -> anyhow::Result { let (db, _) = trustify_db::embedded::create().await?; let (storage, _temp) = FileSystemBackend::for_test().await?; - let analysis = AnalysisService::new(AnalysisConfig::default(), db.clone()); + let db_rw = db::ReadWrite::new(db.clone()); + let db_ro = db::ReadOnly::new(db.clone()); + let analysis = AnalysisService::new(AnalysisConfig::default(), db_ro.clone()); let (_, mut openapi) = App::new() .into_utoipa_app() @@ -17,7 +19,8 @@ pub async fn create_openapi() -> anyhow::Result { svc, Config { config: ModuleConfig::default(), - db, + db_rw, + db_ro, cache: PaginationCache::for_test(), storage: storage.into(), auth: None, diff --git a/server/src/profile/api.rs b/server/src/profile/api.rs index f0b947bf2..c872de397 100644 --- a/server/src/profile/api.rs +++ b/server/src/profile/api.rs @@ -14,7 +14,7 @@ use trustify_auth::{ swagger_ui::{SwaggerUiOidc, SwaggerUiOidcConfig}, }; use trustify_common::{ - config::Database, + config::{Database, DatabaseReadOnly}, db::{ self, pagination_cache::{PaginationCache, PaginationConfig}, @@ -102,6 +102,10 @@ pub struct Run { #[command(flatten)] pub database: Database, + /// Read-only database configuration (optional, falls back to primary database) + #[command(flatten)] + pub database_ro: DatabaseReadOnly, + /// Location of the storage #[command(flatten)] pub storage: StorageConfig, @@ -176,7 +180,8 @@ const SERVICE_ID: &str = "trustify"; struct InitData { authenticator: Option>, authorizer: Authorizer, - db: db::Database, + db_rw: db::ReadWrite, + db_ro: db::ReadOnly, cache: PaginationCache, storage: DispatchBackend, http: HttpServerConfig, @@ -252,6 +257,10 @@ impl InitData { trustify_db::Database(&db).migrate().await?; } + let ro_config = run.database_ro.to_database_config(&run.database); + let db_ro = db::ReadOnly::new(db::Database::new(&ro_config).await?); + let db_rw = db::ReadWrite::new(db.clone()); + let cache = run.pagination.into_cache(); if run.devmode || run.sample_data { @@ -290,10 +299,11 @@ impl InitData { }; Ok(InitData { - analysis: AnalysisService::new(run.analysis, db.clone()), + analysis: AnalysisService::new(run.analysis, db_ro.clone()), authenticator, authorizer, - db, + db_rw, + db_ro, cache, config, http: run.http, @@ -324,7 +334,8 @@ impl InitData { svc, Config { config: self.config.clone(), - db: self.db.clone(), + db_rw: self.db_rw.clone(), + db_ro: self.db_ro.clone(), cache: self.cache.clone(), storage: self.storage.clone(), auth: self.authenticator.clone(), @@ -373,7 +384,8 @@ pub fn default_openapi_info() -> Info { pub(crate) struct Config { pub(crate) config: ModuleConfig, - pub(crate) db: db::Database, + pub(crate) db_rw: db::ReadWrite, + pub(crate) db_ro: db::ReadOnly, pub(crate) cache: PaginationCache, pub(crate) storage: DispatchBackend, pub(crate) analysis: AnalysisService, @@ -389,7 +401,8 @@ pub(crate) fn configure(svc: &mut utoipa_actix_web::service_config::ServiceConfi fundamental, ui, }, - db, + db_rw, + db_ro, cache, storage, auth, @@ -397,7 +410,7 @@ pub(crate) fn configure(svc: &mut utoipa_actix_web::service_config::ServiceConfi read_only, } = config; - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let limit = ByteSize::gb(1).as_u64() as usize; svc.app_data(web::Data::new(ReadOnlyState(read_only))); @@ -412,24 +425,25 @@ pub(crate) fn configure(svc: &mut utoipa_actix_web::service_config::ServiceConfi utoipa_actix_web::scope("/api") .map(|scope| scope.wrap(new_auth(auth))) .configure(|svc| { - trustify_module_importer::endpoints::configure(svc, db.clone(), cache.clone()); + trustify_module_importer::endpoints::configure(svc, db_rw.clone(), cache.clone()); trustify_module_ingestor::endpoints::configure( svc, ingestor, - db.clone(), + db_rw.clone(), storage.clone(), Some(analysis.clone()), ); trustify_module_fundamental::endpoints::configure( svc, fundamental, - db.clone(), + db_rw.clone(), + db_ro.clone(), storage, analysis.clone(), cache, ); - trustify_module_analysis::endpoints::configure(svc, db.clone(), analysis); - trustify_module_user::endpoints::configure(svc, db.clone()); + trustify_module_analysis::endpoints::configure(svc, db_ro.clone(), analysis); + trustify_module_user::endpoints::configure(svc); trustify_module_ui::endpoints::configure(svc, ui) }), ); @@ -489,7 +503,8 @@ mod test { #[test(actix_web::test)] async fn routing(ctx: TrustifyContext) -> Result<(), anyhow::Error> { let ui = Arc::new(UiResources::new(&UI::default())?); - let analysis = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let analysis = + AnalysisService::new(AnalysisConfig::default(), db::ReadOnly::new(ctx.db.clone())); let app = actix_web::test::init_service( App::new() .into_utoipa_app() @@ -499,7 +514,8 @@ mod test { svc, Config { config: ModuleConfig::default(), - db: ctx.db.clone(), + db_rw: db::ReadWrite::new(ctx.db.clone()), + db_ro: db::ReadOnly::new(ctx.db.clone()), cache: PaginationCache::for_test(), storage: ctx.storage.clone().into(), auth: None, @@ -563,13 +579,15 @@ mod test { /// Creates a fully configured test app with all server endpoints and standard middleware. async fn caller(ctx: &TrustifyContext, read_only: bool) -> impl CallService { - let analysis = AnalysisService::new(AnalysisConfig::default(), ctx.db.clone()); + let analysis = + AnalysisService::new(AnalysisConfig::default(), db::ReadOnly::new(ctx.db.clone())); call::caller_app(move |svc| { configure( svc, Config { config: ModuleConfig::default(), - db: ctx.db.clone(), + db_rw: db::ReadWrite::new(ctx.db.clone()), + db_ro: db::ReadOnly::new(ctx.db.clone()), storage: ctx.storage.clone().into(), cache: PaginationCache::for_test(), auth: None, diff --git a/server/src/profile/importer.rs b/server/src/profile/importer.rs index 92e7b7951..b3184a0d1 100644 --- a/server/src/profile/importer.rs +++ b/server/src/profile/importer.rs @@ -99,7 +99,7 @@ impl InitData { } async fn run(self) -> anyhow::Result<()> { - let db = self.db; + let db = db::ReadWrite::new(self.db); let storage = self.storage; let importer = async { diff --git a/server/src/sample_data.rs b/server/src/sample_data.rs index f14d80513..eae50500a 100644 --- a/server/src/sample_data.rs +++ b/server/src/sample_data.rs @@ -1,6 +1,6 @@ use bytesize::ByteSize; use std::{collections::HashSet, time::Duration}; -use trustify_common::db::pagination_cache::PaginationCache; +use trustify_common::db::{ReadWrite, pagination_cache::PaginationCache}; use trustify_module_importer::model::{ ClearlyDefinedImporter, ClearlyDefinedPackageType, CveImporter, CweImporter, DEFAULT_SOURCE_CLEARLY_DEFINED_CURATION, DEFAULT_SOURCE_CVEPROJECT, DEFAULT_SOURCE_CWE_CATALOG, @@ -176,7 +176,7 @@ pub async fn sample_data( db: trustify_common::db::Database, cache: PaginationCache, ) -> anyhow::Result<()> { - let importer = ImporterService::new(db, cache); + let importer = ImporterService::new(ReadWrite::new(db), cache); add(&importer, "redhat-sbom", ImporterConfiguration::Sbom(SbomImporter { common: CommonImporter { @@ -355,7 +355,8 @@ mod test { async fn add_samples(ctx: TrustifyContext) -> anyhow::Result<()> { sample_data(ctx.db.clone(), PaginationCache::for_test()).await?; - let service = ImporterService::new(ctx.db.clone(), PaginationCache::for_test()); + let service = + ImporterService::new(ReadWrite::new(ctx.db.clone()), PaginationCache::for_test()); let result = service.list().await?; assert_eq!(result.len(), 16); diff --git a/test-context/src/lib.rs b/test-context/src/lib.rs index 71380120e..d54f6a415 100644 --- a/test-context/src/lib.rs +++ b/test-context/src/lib.rs @@ -79,7 +79,7 @@ impl TrustifyTestContext { storage: FileSystemBackend, resources: impl Into, ) -> Self { - let graph = Graph::new(db.clone()); + let graph = Graph::new(); let ingestor = IngestorService::new(graph.clone(), storage.clone(), Default::default()); let mem_limit_mb = env::var("MEM_LIMIT_MB") .unwrap_or("500".into()) diff --git a/xtask/src/dataset.rs b/xtask/src/dataset.rs index e5313ea1a..5112adec0 100644 --- a/xtask/src/dataset.rs +++ b/xtask/src/dataset.rs @@ -6,7 +6,7 @@ use serde_json::Value; use std::{env, io::BufReader, path::PathBuf, time::Duration}; use tar::Builder; use tokio::fs; -use trustify_common::model::BinaryByteSize; +use trustify_common::{db::ReadWrite, model::BinaryByteSize}; use trustify_module_importer::{ model::{CommonImporter, CsafImporter, CveImporter, ImporterConfiguration, SbomImporter}, runner::{ @@ -131,7 +131,7 @@ impl GenerateDump { }; let importer = ImportRunner { - db: db.clone(), + db: ReadWrite::new(db.clone()), storage: storage.into(), working_dir: self.working_dir.as_ref().map(|wd| wd.join("wd")), // The xtask doesn't need the analysis graph @@ -187,8 +187,7 @@ impl GenerateDump { // ingest files - let service = - IngestorService::new(Graph::new(runner.db.clone()), runner.storage.clone(), None); + let service = IngestorService::new(Graph::new(), runner.storage.clone(), None); for path in config.paths { log::info!("Ingesting: {}", path.display()); let path = wd.join(path);