Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: perf improvements #1139

Merged
merged 3 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions migration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ mod m0000790_alter_sbom_alter_document_id;
mod m0000800_alter_product_version_range_scheme;
mod m0000810_fix_get_purl;
mod m0000820_create_conversation;
mod m0000830_perf_indexes;

pub struct Migrator;

Expand Down Expand Up @@ -205,6 +206,7 @@ impl MigratorTrait for Migrator {
Box::new(m0000800_alter_product_version_range_scheme::Migration),
Box::new(m0000810_fix_get_purl::Migration),
Box::new(m0000820_create_conversation::Migration),
Box::new(m0000830_perf_indexes::Migration),
]
}
}
Expand Down
116 changes: 116 additions & 0 deletions migration/src/m0000830_perf_indexes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
use sea_orm_migration::prelude::*;

#[derive(DeriveMigrationName)]
pub struct Migration;

#[async_trait::async_trait]
#[allow(deprecated)]
impl MigrationTrait for Migration {
async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> {
manager
.create_index(
Index::create()
.table(VersionedPurl::Table)
.name(Indexes::VersionedPurlBasePurlIdIDX.to_string())
.col(VersionedPurl::BasePurlId)
.to_owned(),
)
.await?;
manager
.create_index(
Index::create()
.table(PurlStatus::Table)
.name(Indexes::PurlStatusBasePurlIdIDX.to_string())
.col(PurlStatus::BasePurlId)
.to_owned(),
)
.await?;
manager
.create_index(
Index::create()
.table(SbomPackagePurlRef::Table)
.name(Indexes::SbomPackagePurlRefSbomIdIDX.to_string())
.col(SbomPackagePurlRef::SbomId)
.to_owned(),
)
.await?;
manager
.create_index(
Index::create()
.table(SbomPackagePurlRef::Table)
.name(Indexes::SbomPackagePurlRefNodeIdIDX.to_string())
.col(SbomPackagePurlRef::NodeId)
.to_owned(),
)
.await?;
Ok(())
}

async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> {
manager
.drop_index(
Index::drop()
.if_exists()
.table(SbomPackagePurlRef::Table)
.name(Indexes::SbomPackagePurlRefNodeIdIDX.to_string())
.to_owned(),
)
.await?;
manager
.drop_index(
Index::drop()
.if_exists()
.table(SbomPackagePurlRef::Table)
.name(Indexes::SbomPackagePurlRefSbomIdIDX.to_string())
.to_owned(),
)
.await?;
manager
.drop_index(
Index::drop()
.if_exists()
.table(PurlStatus::Table)
.name(Indexes::PurlStatusBasePurlIdIDX.to_string())
.to_owned(),
)
.await?;
manager
.drop_index(
Index::drop()
.if_exists()
.table(VersionedPurl::Table)
.name(Indexes::VersionedPurlBasePurlIdIDX.to_string())
.to_owned(),
)
.await?;
Ok(())
}
}

#[allow(clippy::enum_variant_names)]
#[derive(DeriveIden)]
enum Indexes {
VersionedPurlBasePurlIdIDX,
PurlStatusBasePurlIdIDX,
SbomPackagePurlRefSbomIdIDX,
SbomPackagePurlRefNodeIdIDX,
}

#[derive(DeriveIden)]
enum VersionedPurl {
Table,
BasePurlId,
}

#[derive(DeriveIden)]
enum PurlStatus {
Table,
BasePurlId,
}

#[derive(DeriveIden)]
enum SbomPackagePurlRef {
Table,
SbomId,
NodeId,
}
8 changes: 5 additions & 3 deletions modules/fundamental/src/ai/service/tools/sbom_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ For example, input "quarkus" instead of "quarkus 3.2.11".
Err(_) => None,
Ok(id) => {
log::info!("Fetching SBOM details by Id: {}", id);
service.fetch_sbom_details(id, &self.db).await?
service.fetch_sbom_details(id, vec![], &self.db).await?
}
};

Expand All @@ -76,7 +76,9 @@ For example, input "quarkus" instead of "quarkus 3.2.11".
Err(_) => None,
Ok(id) => {
log::info!("Fetching SBOM details by UUID: {}", id);
service.fetch_sbom_details(Id::Uuid(id), &self.db).await?
service
.fetch_sbom_details(Id::Uuid(id), vec![], &self.db)
.await?
}
};
}
Expand All @@ -100,7 +102,7 @@ For example, input "quarkus" instead of "quarkus 3.2.11".
0 => None,
1 => {
service
.fetch_sbom_details(Id::Uuid(results.items[0].head.id), &self.db)
.fetch_sbom_details(Id::Uuid(results.items[0].head.id), vec![], &self.db)
.await?
}
_ => {
Expand Down
2 changes: 1 addition & 1 deletion modules/fundamental/src/purl/service/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ async fn gc_purls(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {
) -> Result<(), anyhow::Error> {
let sbom_service = SbomService::new(ctx.db.clone());
let sbom = sbom_service
.fetch_sbom_details(id, &ctx.db)
.fetch_sbom_details(id, vec![], &ctx.db)
.await?
.expect("fetch_sbom");
assert_eq!(
Expand Down
6 changes: 5 additions & 1 deletion modules/fundamental/src/sbom/endpoints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,11 @@ pub async fn get_sbom_advisories(
_: Require<GetSbomAdvisories>,
) -> actix_web::Result<impl Responder> {
let id = Id::from_str(&id).map_err(Error::IdKey)?;
match fetcher.fetch_sbom_details(id, db.as_ref()).await? {
let statuses: Vec<String> = vec!["affected".to_string()];
match fetcher
.fetch_sbom_details(id, statuses, db.as_ref())
.await?
{
Some(v) => Ok(HttpResponse::Ok().json(v.advisories)),
None => Ok(HttpResponse::NotFound().finish()),
}
Expand Down
29 changes: 19 additions & 10 deletions modules/fundamental/src/sbom/model/details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ impl SbomDetails {
(sbom, node): (sbom::Model, Option<sbom_node::Model>),
service: &SbomService,
tx: &C,
statuses: Vec<String>,
) -> Result<Option<SbomDetails>, Error> {
let mut relevant_advisory_info = sbom
let mut query = sbom
.find_related(sbom_package::Entity)
.join(JoinType::Join, sbom_package::Relation::Node.def())
.join(JoinType::LeftJoin, sbom_package::Relation::Purl.def())
Expand All @@ -60,6 +61,20 @@ impl SbomDetails {
JoinType::LeftJoin,
qualified_purl::Relation::VersionedPurl.def(),
)
.join(JoinType::LeftJoin, versioned_purl::Relation::BasePurl.def())
.join(JoinType::Join, base_purl::Relation::PurlStatus.def())
.join(JoinType::Join, purl_status::Relation::Status.def());

if !statuses.is_empty() {
query = query
.filter(Expr::col((status::Entity, status::Column::Slug)).is_in(statuses.clone()));
}

let mut relevant_advisory_info = query
.join(
JoinType::LeftJoin,
purl_status::Relation::VersionRange.def(),
)
.filter(SimpleExpr::FunctionCall(
Func::cust(VersionMatches)
.arg(Expr::col((
Expand All @@ -68,13 +83,6 @@ impl SbomDetails {
)))
.arg(Expr::col((version_range::Entity, Asterisk))),
))
.join(JoinType::LeftJoin, versioned_purl::Relation::BasePurl.def())
.join(JoinType::Join, base_purl::Relation::PurlStatus.def())
.join(JoinType::Join, purl_status::Relation::Status.def())
.join(
JoinType::LeftJoin,
purl_status::Relation::VersionRange.def(),
)
.join(JoinType::LeftJoin, purl_status::Relation::ContextCpe.def())
.join(JoinType::Join, purl_status::Relation::Advisory.def())
.join(JoinType::Join, purl_status::Relation::Vulnerability.def())
Expand Down Expand Up @@ -149,7 +157,7 @@ impl SbomDetails {
JOIN "version_range" ON "product_version_range"."version_range_id" = "version_range"."id" AND version_matches("product_version"."version", "version_range".*)

-- now find matching purls in these statuses
JOIN base_purl ON "product_status"."package" LIKE CONCAT("base_purl"."namespace", '/', "base_purl"."name") OR "product_status"."package" = "base_purl"."name"
JOIN base_purl ON product_status.package = base_purl.name OR product_status.package LIKE CONCAT(base_purl.namespace, '/', base_purl.name)
JOIN "versioned_purl" ON "versioned_purl"."base_purl_id" = "base_purl"."id"
JOIN "qualified_purl" ON "qualified_purl"."versioned_purl_id" = "versioned_purl"."id"
join sbom_package_purl_ref ON sbom_package_purl_ref.qualified_purl_id = qualified_purl.id AND sbom_package_purl_ref.sbom_id = sbom.sbom_id
Expand All @@ -162,13 +170,14 @@ impl SbomDetails {
JOIN "vulnerability" ON "product_status"."vulnerability_id" = "vulnerability"."id"
WHERE
"sbom"."sbom_id" = $1
AND ($2::text[] = ARRAY[]::text[] OR "status"."slug" = ANY($2::text[]))
"#;

let result: Vec<QueryResult> = tx
.query_all(Statement::from_sql_and_values(
DbBackend::Postgres,
product_advisory_info,
[sbom.sbom_id.into()],
[sbom.sbom_id.into(), statuses.into()],
))
.await?;

Expand Down
3 changes: 2 additions & 1 deletion modules/fundamental/src/sbom/service/sbom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,12 @@ impl SbomService {
pub async fn fetch_sbom_details<C: ConnectionTrait>(
&self,
id: Id,
statuses: Vec<String>,

connection: &C,
) -> Result<Option<SbomDetails>, Error> {
Ok(match self.fetch_sbom(id, connection).await? {
Some(row) => SbomDetails::from_entity(row, self, connection).await?,
Some(row) => SbomDetails::from_entity(row, self, connection, statuses).await?,
None => None,
})
}
Expand Down
6 changes: 4 additions & 2 deletions modules/fundamental/src/sbom/service/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ async fn sbom_details_status(ctx: &TrustifyContext) -> Result<(), anyhow::Error>

let id_3_2_12 = results[3].id.clone();

let details = service.fetch_sbom_details(id_3_2_12, &ctx.db).await?;
let details = service
.fetch_sbom_details(id_3_2_12, vec![], &ctx.db)
.await?;

assert!(details.is_some());

Expand All @@ -31,7 +33,7 @@ async fn sbom_details_status(ctx: &TrustifyContext) -> Result<(), anyhow::Error>
log::debug!("{details:#?}");

let details = service
.fetch_sbom_details(Id::Uuid(details.summary.head.id), &ctx.db)
.fetch_sbom_details(Id::Uuid(details.summary.head.id), vec![], &ctx.db)
.await?;

assert!(details.is_some());
Expand Down
12 changes: 9 additions & 3 deletions modules/fundamental/src/vulnerability/service/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ async fn commons_compress(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {

let sat_id = ingest_results[1].id.clone();

let sat_sbom = sbom_service.fetch_sbom_details(sat_id, &ctx.db).await?;
let sat_sbom = sbom_service
.fetch_sbom_details(sat_id, vec![], &ctx.db)
.await?;
assert!(sat_sbom.is_some());

let sat_sbom = sat_sbom.unwrap();
Expand All @@ -155,7 +157,9 @@ async fn commons_compress(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {

let quarkus_id = ingest_results[3].id.clone();

let quarkus_sbom = sbom_service.fetch_sbom_details(quarkus_id, &ctx.db).await?;
let quarkus_sbom = sbom_service
.fetch_sbom_details(quarkus_id, vec![], &ctx.db)
.await?;

assert!(quarkus_sbom.is_some());

Expand Down Expand Up @@ -224,7 +228,9 @@ async fn product_statuses(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {

let quarkus_id = ingest_results[1].id.clone();

let quarkus_sbom = sbom_service.fetch_sbom_details(quarkus_id, &ctx.db).await?;
let quarkus_sbom = sbom_service
.fetch_sbom_details(quarkus_id, vec![], &ctx.db)
.await?;

assert!(quarkus_sbom.is_some());

Expand Down
Loading
Loading