Skip to content

Commit 272e73d

Browse files
mrrajanclaude
andcommitted
fix: add transaction safety and update comments from code review
This ensures both expanded_license dictionary and sbom_license_expanded junction table inserts are atomic - if either fails, both roll back. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 08b89e8 commit 272e73d

4 files changed

Lines changed: 25 additions & 12 deletions

File tree

modules/fundamental/src/license/service/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,9 @@ impl LicenseService {
285285
.distinct()
286286
.column_as(expanded_license::Column::ExpandedText, LICENSE_TEXT);
287287

288-
// Build query for non-expanded licenses: includes both
288+
// Build query for licenses not yet linked to any SBOM: includes both
289289
// (a) pre-loaded SPDX dictionary entries with no SBOM connection yet, AND
290-
// (b) CycloneDX licenses that exist in sbom_package_license but were never expanded.
290+
// (b) licenses from older SBOMs ingested before license expansion was implemented.
291291
// Use NOT EXISTS instead of LEFT JOIN + IS NULL to find licenses without SBOMs.
292292
// On large tables, LEFT JOIN scans all rows while NOT EXISTS
293293
// uses a Nested Loop Anti Join with index-only scan.

modules/ingestor/src/graph/sbom/common/expanded_license.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
use sea_orm::{ConnectionTrait, DbErr, Statement};
1+
use sea_orm::{ConnectionTrait, DbErr, Statement, TransactionTrait};
22
use uuid::Uuid;
33

44
/// Populates expanded_license and sbom_license_expanded tables during SBOM ingestion
55
///
6-
/// This function uses a single SQL statement with CTEs to:
6+
/// This function uses two SQL statements within a transaction to:
77
/// 1. Call expand_license_expression_with_mappings() once per license
88
/// 2. Insert distinct expanded texts into the expanded_license dictionary
99
/// 3. Populate the sbom_license_expanded junction table
@@ -16,13 +16,23 @@ use uuid::Uuid;
1616
///
1717
/// While SeaORM could express this via custom expressions, it would be significantly
1818
/// more verbose and harder to maintain than the raw SQL.
19+
///
20+
/// **Transaction safety**: Both dictionary and junction table inserts run in a single
21+
/// transaction to prevent partial state if the second insert fails.
22+
///
23+
/// **Note on SQL duplication**: Similar SQL appears in migration m0002120 for backfilling
24+
/// existing data. The migration processes ALL SBOMs at once, while this function runs
25+
/// per-SBOM during ingestion. Keep both in sync when updating license expansion logic.
1926
pub async fn populate_expanded_license(
2027
sbom_id: Uuid,
21-
db: &impl ConnectionTrait,
28+
db: &(impl ConnectionTrait + TransactionTrait),
2229
) -> Result<(), DbErr> {
30+
// Begin transaction to ensure atomicity between dictionary and junction table inserts
31+
let txn = db.begin().await?;
32+
2333
// Step 1: Insert into expanded_license dictionary
24-
db.execute(Statement::from_sql_and_values(
25-
db.get_database_backend(),
34+
txn.execute(Statement::from_sql_and_values(
35+
txn.get_database_backend(),
2636
r#"
2737
INSERT INTO expanded_license (expanded_text)
2838
SELECT DISTINCT expand_license_expression_with_mappings(
@@ -45,8 +55,8 @@ ON CONFLICT (text_hash) DO NOTHING
4555

4656
// Step 2: Insert into sbom_license_expanded junction table
4757
// Use CTE to call expand_license_expression_with_mappings() only once per (sbom_id, license_id)
48-
db.execute(Statement::from_sql_and_values(
49-
db.get_database_backend(),
58+
txn.execute(Statement::from_sql_and_values(
59+
txn.get_database_backend(),
5060
r#"
5161
WITH license_expansions AS (
5262
SELECT DISTINCT
@@ -76,5 +86,8 @@ SET expanded_license_id = EXCLUDED.expanded_license_id
7686
))
7787
.await?;
7888

89+
// Commit transaction - both inserts succeed or both roll back
90+
txn.commit().await?;
91+
7992
Ok(())
8093
}

modules/ingestor/src/graph/sbom/cyclonedx.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ impl<'a> From<Information<'a>> for SbomInformation {
134134

135135
impl SbomContext {
136136
#[instrument(skip(connection, sbom, warnings), err(level=tracing::Level::INFO))]
137-
pub async fn ingest_cyclonedx<C: ConnectionTrait>(
137+
pub async fn ingest_cyclonedx<C: ConnectionTrait + sea_orm::TransactionTrait>(
138138
&self,
139139
mut sbom: Box<CycloneDx>,
140140
warnings: &dyn ReportSink,
@@ -283,7 +283,7 @@ impl<'a> Creator<'a> {
283283
#[instrument(skip(self, db, processors), err(level=tracing::Level::INFO))]
284284
pub async fn create(
285285
self,
286-
db: &impl ConnectionTrait,
286+
db: &(impl ConnectionTrait + sea_orm::TransactionTrait),
287287
processors: &mut [Box<dyn Processor>],
288288
) -> Result<(), Error> {
289289
let mut purls = PurlCreator::new();

modules/ingestor/src/graph/sbom/spdx.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ impl<'a> From<Information<'a>> for SbomInformation {
102102

103103
impl SbomContext {
104104
#[instrument(skip(db, sbom_data, warnings), ret(level=tracing::Level::DEBUG))]
105-
pub async fn ingest_spdx<C: ConnectionTrait>(
105+
pub async fn ingest_spdx<C: ConnectionTrait + sea_orm::TransactionTrait>(
106106
&self,
107107
sbom_data: SPDX,
108108
warnings: &dyn ReportSink,

0 commit comments

Comments
 (0)