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.

Sign up for GitHub

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

Jump to bottom

Enhance query filters to handle Uuid and enable analysis graph to use filters. #920

Merged
merged 2 commits into from
Oct 14, 2024
Merged

Enhance query filters to handle Uuid and enable analysis graph to use filters. #920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c3063c
chore: implement Uuid comparison in query.rs
JimFuller-RedHat Oct 14, 2024
ab2456e
fix typo
JimFuller-RedHat Oct 14, 2024
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
24 changes: 17 additions & 7 deletions common/src/db/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use std::sync::OnceLock;
use time::format_description::well_known::Rfc3339;
use time::macros::format_description;
use time::{Date, OffsetDateTime};
use uuid::Uuid;

/////////////////////////////////////////////////////////////////////////
// Public interface
Expand Down Expand Up @@ -473,6 +474,7 @@ impl Arg {
return Ok(Arg::Null);
}
Ok(match ct {
ColumnType::Uuid => Arg::Value(Value::from(s.parse::<Uuid>().map_err(err)?)),
JimFuller-RedHat marked this conversation as resolved.
Show resolved Hide resolved
ColumnType::Integer => Arg::Value(Value::from(s.parse::<i32>().map_err(err)?)),
ColumnType::Decimal(_) | ColumnType::Float | ColumnType::Double => {
Arg::Value(Value::from(s.parse::<f64>().map_err(err)?))
Expand Down Expand Up @@ -808,16 +810,16 @@ mod tests {
r#"(("advisory"."location" ILIKE '%foo%') OR ("advisory"."title" ILIKE '%foo%')) AND "advisory"."location" = 'bar'"#
);
assert_eq!(
where_clause(r"m\&m's&location=f\&oo&id=13")?,
r#"(("advisory"."location" ILIKE E'%m&m\'s%') OR ("advisory"."title" ILIKE E'%m&m\'s%')) AND "advisory"."location" = 'f&oo' AND "advisory"."id" = 13"#
where_clause(r"m\&m's&location=f\&oo&id=0e840505-e29b-41d4-a716-665544004400")?,
r#"(("advisory"."location" ILIKE E'%m&m\'s%') OR ("advisory"."title" ILIKE E'%m&m\'s%')) AND "advisory"."location" = 'f&oo' AND "advisory"."id" = '0e840505-e29b-41d4-a716-665544004400'"#
);
assert_eq!(
where_clause("a|b|c")?,
r#"("advisory"."location" ILIKE '%a%') OR ("advisory"."title" ILIKE '%a%') OR ("advisory"."location" ILIKE '%b%') OR ("advisory"."title" ILIKE '%b%') OR ("advisory"."location" ILIKE '%c%') OR ("advisory"."title" ILIKE '%c%')"#
);
assert_eq!(
where_clause("a|b&id=1")?,
r#"(("advisory"."location" ILIKE '%a%') OR ("advisory"."title" ILIKE '%a%') OR ("advisory"."location" ILIKE '%b%') OR ("advisory"."title" ILIKE '%b%')) AND "advisory"."id" = 1"#
where_clause("a|b&id=0e840505-e29b-41d4-a716-665544004400")?,
r#"(("advisory"."location" ILIKE '%a%') OR ("advisory"."title" ILIKE '%a%') OR ("advisory"."location" ILIKE '%b%') OR ("advisory"."title" ILIKE '%b%')) AND "advisory"."id" = '0e840505-e29b-41d4-a716-665544004400'"#
JimFuller-RedHat marked this conversation as resolved.
Show resolved Hide resolved
);
assert_eq!(
where_clause("a&b")?,
Expand Down Expand Up @@ -922,7 +924,11 @@ mod tests {
)
.unwrap()
.build(sea_orm::DatabaseBackend::Postgres)
.to_string()[45..]
.to_string()
.split("WHERE")
.last()
.expect("problem splitting string")
.trim()
.to_string();
assert_eq!(stmt, expected);
};
Expand Down Expand Up @@ -953,7 +959,11 @@ mod tests {
.column(advisory::Column::Id)
.filtering(q(query))?
.build(sea_orm::DatabaseBackend::Postgres)
.to_string()[45..]
.to_string()
.split("WHERE")
.last()
.expect("problem splitting string")
.trim()
.to_string())
}

Expand All @@ -965,7 +975,7 @@ mod tests {
#[sea_orm(table_name = "advisory")]
pub struct Model {
#[sea_orm(primary_key)]
pub id: i32,
pub id: Uuid,
pub location: String,
pub title: String,
pub published: Option<OffsetDateTime>,
Expand Down
53 changes: 53 additions & 0 deletions modules/analysis/src/endpoints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,4 +407,57 @@ mod test {
);
Ok(assert_eq!(&response["total"], 2))
}

#[test_context(TrustifyContext)]
#[test(actix_web::test)]
async fn test_retrieve_query_params_endpoint(
ctx: &TrustifyContext,
) -> Result<(), anyhow::Error> {
let app = caller(ctx).await?;
ctx.ingest_documents(["spdx/simple.json"]).await?;

// filter on node_id
let uri = "/api/v1/analysis/dep?q=node_id%3DSPDXRef-A";
let request: Request = TestRequest::get().uri(uri).to_request();
let response: Value = app.call_and_read_body_json(request).await;
assert_eq!(response["items"][0]["name"], "A");
assert_eq!(&response["total"], 1);

// filter on node_id
let uri = "/api/v1/analysis/root-component?q=node_id%3DSPDXRef-B";
let request: Request = TestRequest::get().uri(uri).to_request();
let response: Value = app.call_and_read_body_json(request).await;
assert_eq!(response["items"][0]["name"], "B");
assert_eq!(&response["total"], 1);

// filter on node_id & name
let uri = "/api/v1/analysis/root-component?q=node_id%3DSPDXRef-B%26name%3DB";
let request: Request = TestRequest::get().uri(uri).to_request();
let response: Value = app.call_and_read_body_json(request).await;
assert_eq!(response["items"][0]["name"], "B");
assert_eq!(&response["total"], 1);

// filter on sbom_id (which has urn:uuid: prefix)
let sbom_id = response["items"][0]["sbom_id"].as_str().unwrap();
let uri = format!(
"/api/v1/analysis/root-component?q=sbom_id=urn:uuid:{}",
sbom_id
);
let request: Request = TestRequest::get().uri(uri.clone().as_str()).to_request();
let response: Value = app.call_and_read_body_json(request).await;
assert_eq!(&response["total"], 7);

// negative test
let uri = "/api/v1/analysis/root-component?q=sbom_id=urn:uuid:99999999-9999-9999-9999-999999999999";
let request: Request = TestRequest::get().uri(uri).to_request();
let response: Value = app.call_and_read_body_json(request).await;
assert_eq!(&response["total"], 0);

// negative test
let uri = "/api/v1/analysis/root-component?q=node_id%3DSPDXRef-B%26name%3DA";
let request: Request = TestRequest::get().uri(uri).to_request();
let response: Value = app.call_and_read_body_json(request).await;

Ok(assert_eq!(&response["total"], 0))
}
}
84 changes: 68 additions & 16 deletions modules/analysis/src/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use petgraph::visit::{NodeIndexable, VisitMap, Visitable};
use petgraph::Direction;
use sea_query::Order;
use std::str::FromStr;
use trustify_common::db::query::Filtering;
use trustify_common::db::ConnectionOrTransaction;
use trustify_common::purl::Purl;
use trustify_entity::relationship::Relationship;
Expand Down Expand Up @@ -273,6 +274,23 @@ pub async fn load_graphs(
}
}

fn convert_query_to_hashmap(query: &Query) -> HashMap<String, String> {
if query.q.contains('=') {
query
.q
.split('&')
.filter_map(|pair| {
pair.split_once('=').map(|(key, value)| {
let value = value.strip_prefix("urn:uuid:").unwrap_or(value);
(key.to_owned(), value.to_owned())
})
})
.collect()
} else {
HashMap::from([("re_name".to_owned(), query.q.clone())])
}
}
JimFuller-RedHat marked this conversation as resolved.
Show resolved Hide resolved

impl AnalysisService {
pub fn new(db: Database) -> Self {
GraphMap::get_instance();
Expand Down Expand Up @@ -340,8 +358,9 @@ impl AnalysisService {
) -> Result<PaginatedResults<AncestorSummary>, Error> {
let connection = self.db.connection(&tx);

let graph_query_map = convert_query_to_hashmap(&query);
let search_sbom_node_name_subquery = sbom_node::Entity::find()
.filter(sbom_node::Column::Name.like(format!("%{}%", query.q.as_str())))
.filtering(query)?
.select_only()
.column(sbom_node::Column::SbomId)
.distinct()
Expand All @@ -363,15 +382,31 @@ impl AnalysisService {
let graph_manager = GraphMap::get_instance();
{
// RwLock for reading hashmap<graph>
let graph_read_gaurd = graph_manager.read();
let graph_read_guard = graph_manager.read();
for distinct_sbom_id in &distinct_sbom_ids {
if let Some(graph) = graph_read_gaurd.get(distinct_sbom_id.to_string().as_str()) {
if let Some(graph) = graph_read_guard.get(distinct_sbom_id.to_string().as_str()) {
// Iterate over matching node indices and process them directly
graph
.node_indices()
.filter(|&i| {
if let Some(node) = graph.node_weight(i) {
node.name.contains(&query.q.to_string())
if let Some(re_name) = graph_query_map.get("re_name") {
// if no specific url params supplied then use contains search
node.name.contains(re_name)
} else {
// if any specific url params supplied then match equals
let matches_sbom_id = graph_query_map
.get("sbom_id")
.map_or(true, |sbom_id| node.sbom_id.eq(sbom_id));
let matches_node_id = graph_query_map
.get("node_id")
.map_or(true, |node_id| node.node_id.eq(node_id));
let matches_name =
graph_query_map.get("name").map_or(true, |name| {
!name.is_empty() && node.name.eq(name)
});
matches_sbom_id && matches_node_id && matches_name
}
JimFuller-RedHat marked this conversation as resolved.
Show resolved Hide resolved
} else {
false // Return false if the node does not exist
}
Expand Down Expand Up @@ -433,9 +468,9 @@ impl AnalysisService {
let graph_manager = GraphMap::get_instance();
{
// RwLock for reading hashmap<graph>
let graph_read_gaurd = graph_manager.read();
let graph_read_guard = graph_manager.read();
for distinct_sbom_id in &distinct_sbom_ids {
if let Some(graph) = graph_read_gaurd.get(distinct_sbom_id.to_string().as_str()) {
if let Some(graph) = graph_read_guard.get(distinct_sbom_id.to_string().as_str()) {
// Iterate over matching node indices and process them directly
graph
.node_indices()
Expand Down Expand Up @@ -503,9 +538,9 @@ impl AnalysisService {
let graph_manager = GraphMap::get_instance();
{
// RwLock for reading hashmap<graph>
let graph_read_gaurd = graph_manager.read();
let graph_read_guard = graph_manager.read();
for distinct_sbom_id in &distinct_sbom_ids {
if let Some(graph) = graph_read_gaurd.get(distinct_sbom_id.to_string().as_str()) {
if let Some(graph) = graph_read_guard.get(distinct_sbom_id.to_string().as_str()) {
// Iterate over matching node indices and process them directly
graph
.node_indices()
Expand Down Expand Up @@ -564,8 +599,9 @@ impl AnalysisService {
) -> Result<PaginatedResults<DepSummary>, Error> {
let connection = self.db.connection(&tx);

let graph_query_map = convert_query_to_hashmap(&query);
let search_sbom_node_name_subquery = sbom_node::Entity::find()
.filter(sbom_node::Column::Name.like(format!("%{}%", query.q.as_str())))
.filtering(query)?
.select_only()
.column(sbom_node::Column::SbomId)
.distinct()
Expand All @@ -587,15 +623,31 @@ impl AnalysisService {
let graph_manager = GraphMap::get_instance();
{
// RwLock for reading hashmap<graph>
let graph_read_gaurd = graph_manager.read();
let graph_read_guard = graph_manager.read();
for distinct_sbom_id in &distinct_sbom_ids {
if let Some(graph) = graph_read_gaurd.get(distinct_sbom_id.to_string().as_str()) {
if let Some(graph) = graph_read_guard.get(distinct_sbom_id.to_string().as_str()) {
// Iterate over matching node indices and process them directly
graph
.node_indices()
.filter(|&i| {
if let Some(node) = graph.node_weight(i) {
node.name.contains(&query.q.to_string())
if let Some(re_name) = graph_query_map.get("re_name") {
// if no specific url params supplied then use contains search
node.name.contains(re_name)
} else {
// if any specific url params supplied then match equals
let matches_sbom_id = graph_query_map
.get("sbom_id")
.map_or(true, |sbom_id| node.sbom_id.eq(sbom_id));
let matches_node_id = graph_query_map
.get("node_id")
.map_or(true, |node_id| node.node_id.eq(node_id));
let matches_name =
graph_query_map.get("name").map_or(true, |name| {
!name.is_empty() && node.name.eq(name)
});
matches_sbom_id && matches_node_id && matches_name
}
} else {
false // Return false if the node does not exist
}
Expand Down Expand Up @@ -656,9 +708,9 @@ impl AnalysisService {
let graph_manager = GraphMap::get_instance();
{
// RwLock for reading hashmap<graph>
let graph_read_gaurd = graph_manager.read();
let graph_read_guard = graph_manager.read();
for distinct_sbom_id in &distinct_sbom_ids {
if let Some(graph) = graph_read_gaurd.get(distinct_sbom_id.to_string().as_str()) {
if let Some(graph) = graph_read_guard.get(distinct_sbom_id.to_string().as_str()) {
// Iterate over matching node indices and process them directly
graph
.node_indices()
Expand Down Expand Up @@ -726,9 +778,9 @@ impl AnalysisService {
let graph_manager = GraphMap::get_instance();
{
// RwLock for reading hashmap<graph>
let graph_read_gaurd = graph_manager.read();
let graph_read_guard = graph_manager.read();
for distinct_sbom_id in &distinct_sbom_ids {
if let Some(graph) = graph_read_gaurd.get(distinct_sbom_id.to_string().as_str()) {
if let Some(graph) = graph_read_guard.get(distinct_sbom_id.to_string().as_str()) {
// Iterate over matching node indices and process them directly
graph
.node_indices()
Expand Down