refactor: fetch source_document alongside, and only what's required

Instead of fully fetching all information of an SBOM and then just using
the ID, this fetches the bare minimum. But then also fetches the
source_document in the same step. Passing on to other parts which need
it. This should prevent one N+1 select issues.

Author: ctron

modules/fundamental/src/purl/model/details/purl.rs

@@ -28,8 +28,8 @@ use trustify_cvss::cvss3::{score::Score as Cvss3Score, severity::Severity};
 use trustify_entity::{
     advisory, advisory_vulnerability_score, base_purl, cpe, license, organization, product,
     product_status, product_version, product_version_range, purl_status, qualified_purl, sbom,
-    sbom_license_expanded, sbom_package, sbom_package_license, sbom_package_purl_ref, status,
-    version_range, versioned_purl, vulnerability,
+    sbom_license_expanded, sbom_node, sbom_package, sbom_package_license, sbom_package_purl_ref,
+    status, version_range, versioned_purl, vulnerability,
 };
 use trustify_module_ingestor::common::{Deprecation, DeprecationForExt};
 use utoipa::ToSchema;
@@ -455,7 +455,7 @@ impl PurlLicenseSummary {
             let entry = summaries.entry(row.sbom.sbom_id);
             if let Entry::Vacant(entry) = entry {
                 let summary = PurlLicenseSummary {
-                    sbom: SbomHead::from_entity(&row.sbom, None, tx).await?,
+                    sbom: SbomHead::from_entity(&row.sbom, &row.sbom_node, tx).await?,
                     licenses: vec![],
                 };
 
@@ -476,13 +476,15 @@ impl PurlLicenseSummary {
 #[derive(Debug)]
 pub struct LicenseCatcher {
     sbom: sbom::Model,
+    sbom_node: sbom_node::Model,
     license: license::Model,
 }
 
 impl FromQueryResult for LicenseCatcher {
     fn from_query_result(res: &QueryResult, _pre: &str) -> Result<Self, DbErr> {
         Ok(Self {
             sbom: Self::from_query_result_multi_model(res, "", sbom::Entity)?,
+            sbom_node: Self::from_query_result_multi_model(res, "", sbom_node::Entity)?,
             license: Self::from_query_result_multi_model(res, "", license::Entity)?,
         })
     }
@@ -492,6 +494,7 @@ impl FromQueryResultMultiModel for LicenseCatcher {
     fn try_into_multi_model<E: EntityTrait>(select: Select<E>) -> Result<Select<E>, DbErr> {
         select
             .try_model_columns(sbom::Entity)?
+            .try_model_columns(sbom_node::Entity)?
             .try_model_columns(license::Entity)
     }
 }

modules/fundamental/src/sbom/endpoints/mod.rs

@@ -22,6 +22,7 @@ use crate::{
         service::{SbomService, sbom::FetchOptions},
     },
     sbom_group::service::SbomGroupService,
+    source_document::model::SourceDocument,
 };
 use actix_web::{HttpResponse, Responder, delete, get, http::header, post, web};
 use config::Config;
@@ -330,7 +331,7 @@ all!(GetSbomAdvisories -> ReadSbom, ReadAdvisory);
         ("id" = Id, Path),
     ),
     responses(
-        (status = 200, description = "Matching SBOM", body = SbomSummary),
+        (status = 204, description = "Matching SBOM as deleted"),
         (status = 404, description = "The SBOM could not be found"),
     ),
 )]
@@ -345,15 +346,17 @@ pub async fn delete(
     let tx = db.begin().await?;
 
     let id = Id::from_str(&id)?;
-    match service.fetch_sbom_summary(id, &tx).await? {
-        Some(v) => match service.delete_sbom(v.head.id, &tx).await? {
+    match service.fetch_sbom(id, &tx).await? {
+        Some((v, _, source_document)) => match service.delete_sbom(v.sbom_id, &tx).await? {
             false => Ok(HttpResponse::NotFound().finish()),
             true => {
                 tx.commit().await?;
-                if let Err(e) = delete_doc(&v.source_document, i.storage()).await {
+                if let Err(e) =
+                    delete_doc(&SourceDocument::from_entity(&source_document), i.storage()).await
+                {
                     log::warn!("Ignoring {e}");
                 }
-                Ok(HttpResponse::Ok().json(v))
+                Ok(HttpResponse::NoContent().finish())
             }
         },
         None => Ok(HttpResponse::NotFound().finish()),
@@ -386,12 +389,12 @@ pub async fn packages(
     let id = Id::from_str(&id).map_err(Error::IdKey)?;
     let tx = db.begin_read().await?;
 
-    let Some(sbom) = fetch.fetch_sbom_summary(id, &tx).await? else {
+    let Some((sbom, _, _)) = fetch.fetch_sbom(id, &tx).await? else {
         return Ok(HttpResponse::NotFound().finish());
     };
 
     let result = fetch
-        .fetch_sbom_packages(sbom.head.id, search, paginated, &tx)
+        .fetch_sbom_packages(sbom.sbom_id, search, paginated, &tx)
         .await?;
 
     Ok(HttpResponse::Ok().json(result))
@@ -438,13 +441,13 @@ pub async fn related(
     let id = Id::from_str(&id).map_err(Error::IdKey)?;
     let tx = db.begin_read().await?;
 
-    let Some(sbom) = fetch.fetch_sbom_summary(id, &tx).await? else {
+    let Some((sbom, _, _)) = fetch.fetch_sbom(id, &tx).await? else {
         return Ok(HttpResponse::NotFound().finish());
     };
 
     let result = fetch
         .fetch_related_packages(
-            sbom.head.id,
+            sbom.sbom_id,
             search,
             paginated,
             related.which,

modules/fundamental/src/sbom/model/details.rs

@@ -25,8 +25,8 @@ use trustify_common::{db::VersionMatches, memo::Memo};
 use trustify_cvss::cvss3::{score::Score, severity::Severity};
 use trustify_entity::{
     advisory, advisory_vulnerability, advisory_vulnerability_score, base_purl, cpe, organization,
-    purl_status, qualified_purl, sbom, sbom_node, sbom_package, sbom_package_purl_ref, status,
-    version_range, versioned_purl, vulnerability,
+    purl_status, qualified_purl, sbom, sbom_node, sbom_package, sbom_package_purl_ref,
+    source_document, status, version_range, versioned_purl, vulnerability,
 };
 use utoipa::ToSchema;
 use uuid::Uuid;
@@ -75,18 +75,16 @@ impl SbomDetails {
     /// turn an (sbom, sbom_node) row into an [`SbomDetails`], if possible
     #[instrument(skip(service, tx), err(level=tracing::Level::INFO))]
     pub async fn from_entity<C>(
-        (sbom, node): (sbom::Model, Option<sbom_node::Model>),
+        (sbom, node, source_document): (sbom::Model, sbom_node::Model, source_document::Model),
         service: &SbomService,
         tx: &C,
         statuses: Vec<String>,
     ) -> Result<Option<SbomDetails>, Error>
     where
         C: ConnectionTrait,
     {
-        let summary = match SbomSummary::from_entity((sbom.clone(), node), service, tx).await? {
-            Some(summary) => summary,
-            None => return Ok(None),
-        };
+        let summary =
+            SbomSummary::from_entity((sbom.clone(), node, source_document), service, tx).await?;
 
         let mut query = sbom
             .find_related(sbom_package::Entity)

modules/fundamental/src/sbom/model/mod.rs

@@ -53,7 +53,7 @@ impl SbomHead {
     )]
     pub async fn from_entity<C: ConnectionTrait>(
         sbom: &sbom::Model,
-        sbom_node: Option<sbom_node::Model>,
+        sbom_node: &sbom_node::Model,
         db: &C,
     ) -> Result<Self, Error> {
         let number_of_packages = sbom.find_related(sbom_package::Entity).count(db).await?;
@@ -64,7 +64,7 @@ impl SbomHead {
             published: sbom.published,
             authors: sbom.authors.clone(),
             suppliers: sbom.suppliers.clone(),
-            name: sbom_node.map(|node| node.name).unwrap_or("".to_string()),
+            name: sbom_node.name.clone(),
             data_licenses: sbom.data_licenses.clone(),
             number_of_packages,
         })
@@ -85,26 +85,17 @@ pub struct SbomSummary {
 impl SbomSummary {
     #[instrument(skip(service, db), err(level=tracing::Level::INFO))]
     pub async fn from_entity<C: ConnectionTrait>(
-        (sbom, node): (sbom::Model, Option<sbom_node::Model>),
+        (sbom, node, source_document): (sbom::Model, sbom_node::Model, source_document::Model),
         service: &SbomService,
         db: &C,
-    ) -> Result<Option<SbomSummary>, Error> {
+    ) -> Result<SbomSummary, Error> {
         // TODO: consider improving the n-select issues here
         let described_by = service.describes_packages(sbom.sbom_id, (), db).await?;
 
-        let source_document = sbom
-            .find_related(source_document::Entity)
-            .one(db)
-            .await?
-            .ok_or_else(|| Error::NotFound("Missing source document".to_string()))?;
-
-        Ok(match node {
-            Some(_) => Some(SbomSummary {
-                head: SbomHead::from_entity(&sbom, node, db).await?,
-                source_document: SourceDocument::from_entity(&source_document),
-                described_by,
-            }),
-            None => None,
+        Ok(SbomSummary {
+            head: SbomHead::from_entity(&sbom, &node, db).await?,
+            source_document: SourceDocument::from_entity(&source_document),
+            described_by,
         })
     }
 }

modules/fundamental/src/sbom/service/sbom.rs

@@ -37,7 +37,7 @@ use trustify_entity::{
     license, organization, package_relates_to_package,
     qualified_purl::{self, CanonicalPurl},
     relationship::Relationship,
-    sbom::{self, SbomNodeLink},
+    sbom::{self},
     sbom_group_assignment, sbom_license_expanded, sbom_node, sbom_package, sbom_package_cpe_ref,
     sbom_package_license, sbom_package_purl_ref, source_document, status, versioned_purl,
     vulnerability,
@@ -67,20 +67,21 @@ impl FetchOptions {
 }
 
 impl SbomService {
+    /// Fetch an SBOM, its node, and source document
     #[instrument(skip(self, connection), err(level=tracing::Level::INFO))]
-    async fn fetch_sbom<C: ConnectionTrait>(
+    pub async fn fetch_sbom<C: ConnectionTrait>(
         &self,
         id: Id,
         connection: &C,
-    ) -> Result<Option<(sbom::Model, Option<sbom_node::Model>)>, Error> {
+    ) -> Result<Option<(sbom::Model, sbom_node::Model, source_document::Model)>, Error> {
         let select = sbom::Entity::find()
-            .join(JoinType::LeftJoin, sbom::Relation::SourceDocument.def())
+            .find_also_linked(sbom::SbomNodeLink)
+            .find_also_related(source_document::Entity)
             .try_filter(id)?;
 
-        Ok(select
-            .find_also_linked(SbomNodeLink)
-            .one(connection)
-            .await?)
+        let map = |(sbom, node, source_document)| Some((sbom, node?, source_document?));
+
+        Ok(select.one(connection).await?.and_then(map))
     }
 
     /// fetch one sbom
@@ -108,7 +109,7 @@ impl SbomService {
         connection: &C,
     ) -> Result<Option<SbomSummary>, Error> {
         Ok(match self.fetch_sbom(id, connection).await? {
-            Some(row) => SbomSummary::from_entity(row, self, connection).await?,
+            Some(row) => Some(SbomSummary::from_entity(row, self, connection).await?),
             None => None,
         })
     }
@@ -269,8 +270,8 @@ impl SbomService {
         }
 
         let limiter = query
-            .join(JoinType::Join, sbom::Relation::SourceDocument.def())
-            .find_also_linked(SbomNodeLink)
+            .find_also_linked(sbom::SbomNodeLink)
+            .find_also_related(source_document::Entity)
             .filtering_with(
                 search,
                 Columns::from_entity::<sbom::Entity>()
@@ -292,11 +293,14 @@ impl SbomService {
         let total = limiter.total().await?;
         let sboms = limiter.fetch().await?;
 
-        let items = stream::iter(sboms.into_iter())
-            .then(|row| async { SbomSummary::from_entity(row, self, connection).await })
-            .try_filter_map(futures_util::future::ok)
-            .try_collect()
-            .await?;
+        let items = stream::iter(
+            sboms
+                .into_iter()
+                .filter_map(|(sbom, node, source_document)| Some((sbom, node?, source_document?))),
+        )
+        .then(|row| async { SbomSummary::from_entity(row, self, connection).await })
+        .try_collect()
+        .await?;
 
         Ok(PaginatedResults { total, items })
     }
@@ -559,12 +563,15 @@ impl SbomService {
                 .filter(sbom_package_cpe_ref::Column::CpeId.eq(cpe.uuid())),
         };
 
-        let query = select.find_also_linked(SbomNodeLink).filtering_with(
-            query,
-            Columns::from_entity::<sbom::Entity>()
-                .add_columns(sbom_node::Entity)
-                .alias("sbom_node", "r0"),
-        )?;
+        let query = select
+            .find_also_linked(sbom::SbomNodeLink)
+            .find_also_related(source_document::Entity)
+            .filtering_with(
+                query,
+                Columns::from_entity::<sbom::Entity>()
+                    .add_columns(sbom_node::Entity)
+                    .alias("sbom_node", "r0"),
+            )?;
 
         // limit and execute
 
@@ -575,11 +582,14 @@ impl SbomService {
 
         // collect results
 
-        let items = stream::iter(sboms.into_iter())
-            .then(|row| async { SbomSummary::from_entity(row, self, connection).await })
-            .try_filter_map(futures_util::future::ok)
-            .try_collect()
-            .await?;
+        let items = stream::iter(
+            sboms
+                .into_iter()
+                .filter_map(|(sbom, node, source_document)| Some((sbom, node?, source_document?))),
+        )
+        .then(|row| async { SbomSummary::from_entity(row, self, connection).await })
+        .try_collect()
+        .await?;
 
         Ok(PaginatedResults { items, total })
     }

modules/fundamental/src/vulnerability/model/details/vulnerability_advisory.rs

@@ -547,12 +547,7 @@ impl VulnerabilitySbomStatus {
                 Some(existing_entry) => existing_entry,
                 None => {
                     let new_entry = VulnerabilitySbomStatus {
-                        head: SbomHead::from_entity(
-                            &status.sbom,
-                            Some(status.sbom_node.clone()),
-                            tx,
-                        )
-                        .await?,
+                        head: SbomHead::from_entity(&status.sbom, &status.sbom_node, tx).await?,
                         version: status.sbom_package.version.clone(),
                         purl_statuses: Default::default(),
                     };