Skip to content
This repository has been archived by the owner on Oct 3, 2024. It is now read-only.

Commit

Permalink
Fix filter bug
Browse files Browse the repository at this point in the history
  • Loading branch information
@pohlm01
pohlm01 committed Sep 4, 2024
1 parent d5359ab commit 8f114a1
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 42 deletions.
6 changes: 6 additions & 0 deletions fixtures/events.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ VALUES ('event-1',
"values": [
"group-1"
]
},
{
"type": "PRIVATE_LABEL",
"values": [
"private value"
]
}
]'::jsonb,
null,
Expand Down
6 changes: 6 additions & 0 deletions fixtures/programs.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ VALUES ('program-1',
"values": [
"group-1"
]
},
{
"type": "PRIVATE_LABEL",
"values": [
"private value"
]
}
]'),
('program-2',
Expand Down
69 changes: 49 additions & 20 deletions openadr-vtn/src/data_source/postgres/event.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::api::event::QueryParams;
use crate::data_source::postgres::to_json_value;
use crate::data_source::postgres::{to_json_value, PgTargetsFilter};
use crate::data_source::{Crud, EventCrud};
use crate::error::AppError;
use crate::jwt::Claims;
Expand Down Expand Up @@ -118,8 +118,7 @@ struct PostgresFilter<'a> {
program_names: Option<&'a [String]>,
// TODO check whether we also need to extract `PowerServiceLocation`, `ServiceArea`,
// `ResourceNames`, and `Group`, i.e., only leave the `Private`
target_type: Option<&'a str>,
target_values: Option<&'a [String]>,
targets: Vec<PgTargetsFilter<'a>>,

skip: i64,
limit: i64,
Expand All @@ -137,9 +136,16 @@ impl<'a> From<&'a QueryParams> for PostgresFilter<'a> {
Some(TargetLabel::VENName) => filter.ven_names = query.target_values.as_deref(),
Some(TargetLabel::EventName) => filter.event_names = query.target_values.as_deref(),
Some(TargetLabel::ProgramName) => filter.program_names = query.target_values.as_deref(),
Some(_) => {
filter.target_type = query.target_type.as_ref().map(|t| t.as_str());
filter.target_values = query.target_values.as_deref()
Some(ref label) => {
if let Some(values) = query.target_values.as_ref() {
filter.targets = values
.iter()
.map(|value| PgTargetsFilter {
label: label.as_str(),
value: [value.as_str()],
})
.collect()
}
}
None => {}
};
Expand Down Expand Up @@ -218,24 +224,23 @@ impl Crud for PgEventStorage {
AND ($2::text[] IS NULL OR TRUE) -- TODO filter for ven_names
AND ($3::text[] IS NULL OR event_name = ANY($3))
AND ($4::text[] IS NULL OR p.program_name = ANY($4))
AND ($5::text IS NULL OR jsonb_path_query_array(e.targets, '$[*].type') ? $5)
AND ($6::text[] IS NULL OR jsonb_path_query_array(e.targets, '$[*].values[*]') ?| ($6))
OFFSET $7 LIMIT $8
AND ($5::jsonb = '[]'::jsonb OR $5::jsonb <@ e.targets)
OFFSET $6 LIMIT $7
"#,
pg_filter.program_id,
pg_filter.ven_names,
pg_filter.event_names,
pg_filter.program_names,
pg_filter.target_type,
pg_filter.target_values,
serde_json::to_value(pg_filter.targets)
.map_err(AppError::SerdeJsonInternalServerError)?,
pg_filter.skip,
pg_filter.limit
)
.fetch_all(&self.db)
.await?
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?)
.fetch_all(&self.db)
.await?
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?)
}

async fn update(
Expand Down Expand Up @@ -332,10 +337,16 @@ mod tests {
program_id: "program-1".parse().unwrap(),
event_name: Some("event-1-name".to_string()),
priority: Some(4).into(),
targets: Some(TargetMap(vec![TargetEntry {
label: TargetLabel::Group,
values: ["group-1".to_string()],
}])),
targets: Some(TargetMap(vec![
TargetEntry {
label: TargetLabel::Group,
values: ["group-1".to_string()],
},
TargetEntry {
label: TargetLabel::Private("PRIVATE_LABEL".to_string()),
values: ["private value".to_string()],
},
])),
report_descriptors: None,
payload_descriptors: None,
interval_period: Some(IntervalPeriod {
Expand Down Expand Up @@ -520,6 +531,24 @@ mod tests {
assert_eq!(events.len(), 0);
}

#[sqlx::test(fixtures("programs"))]
async fn filter_multiple_targets(db: PgPool) {
let repo: PgEventStorage = db.into();

let events = repo
.retrieve_all(
&QueryParams {
target_type: Some(TargetLabel::Group),
target_values: Some(vec!["private value".to_string()]),
..Default::default()
},
&Claims::any_business_user(),
)
.await
.unwrap();
assert_eq!(events.len(), 0);
}

#[sqlx::test(fixtures("programs", "events"))]
async fn filter_program_id_get_all(db: PgPool) {
let repo: PgEventStorage = db.into();
Expand Down
8 changes: 8 additions & 0 deletions openadr-vtn/src/data_source/postgres/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,11 @@ fn to_json_value<T: Serialize>(v: Option<T>) -> Result<Option<serde_json::Value>
v.map(|v| serde_json::to_value(v).map_err(AppError::SerdeJsonBadRequest))
.transpose()
}

#[derive(Serialize, Debug)]
struct PgTargetsFilter<'a> {
#[serde(rename = "type")]
label: &'a str,
#[serde(rename = "values")]
value: [&'a str; 1],
}
87 changes: 65 additions & 22 deletions openadr-vtn/src/data_source/postgres/program.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::api::program::QueryParams;
use crate::data_source::postgres::to_json_value;
use crate::data_source::postgres::{to_json_value, PgTargetsFilter};
use crate::data_source::{Crud, ProgramCrud};
use crate::error::AppError;
use crate::jwt::{BusinessIds, Claims, User};
use crate::jwt::{BusinessIds, Claims};
use axum::async_trait;
use chrono::{DateTime, Utc};
use openadr_wire::program::{ProgramContent, ProgramId};
Expand Down Expand Up @@ -123,8 +123,7 @@ struct PostgresFilter<'a> {
program_names: Option<&'a [String]>,
// TODO check whether we also need to extract `PowerServiceLocation`, `ServiceArea`,
// `ResourceNames`, and `Group`, i.e., only leave the `Private`
target_type: Option<&'a str>,
target_values: Option<&'a [String]>,
targets: Vec<PgTargetsFilter<'a>>,

skip: i64,
limit: i64,
Expand All @@ -133,16 +132,20 @@ struct PostgresFilter<'a> {
#[derive(Debug)]
struct PgPermissionFilter {
business_ids: Option<Vec<String>>,
ven_ids: Vec<String>,
}

impl From<&User> for PgPermissionFilter {
fn from(User(ref claims): &User) -> Self {
impl From<&Claims> for PgPermissionFilter {
fn from(claims: &Claims) -> Self {
let business_ids = match claims.business_ids() {
BusinessIds::Specific(v) => Some(v),
BusinessIds::Any => None,
};

Self { business_ids }
Self {
business_ids,
ven_ids: claims.ven_ids(),
}
}
}

Expand All @@ -157,9 +160,16 @@ impl<'a> From<&'a QueryParams> for PostgresFilter<'a> {
Some(TargetLabel::VENName) => filter.ven_names = query.target_values.as_deref(),
Some(TargetLabel::EventName) => filter.event_names = query.target_values.as_deref(),
Some(TargetLabel::ProgramName) => filter.program_names = query.target_values.as_deref(),
Some(_) => {
filter.target_type = query.target_type.as_ref().map(|t| t.as_str());
filter.target_values = query.target_values.as_deref()
Some(ref label) => {
if let Some(values) = query.target_values.as_ref() {
filter.targets = values
.iter()
.map(|value| PgTargetsFilter {
label: label.as_str(),
value: [value.as_str()],
})
.collect()
}
}
None => {}
};
Expand Down Expand Up @@ -211,14 +221,24 @@ impl Crud for PgProgramStorage {
async fn retrieve(
&self,
id: &Self::Id,
_user: &Self::PermissionFilter,
user: &Self::PermissionFilter,
) -> Result<Self::Type, Self::Error> {
let permission_filter: PgPermissionFilter = user.into();
trace!(?permission_filter);

Ok(sqlx::query_as!(
PostgresProgram,
r#"
SELECT * FROM program WHERE id = $1
SELECT *
FROM program p
WHERE id = $1
AND (
((jsonb_path_query_array(p.targets, '$[*].type') ? 'VEN_NAME')
AND ($2::text[] IS NULL OR jsonb_path_query_array(p.targets, '$[*].values[*]') ?| ($2)))
OR jsonb_path_query_array(p.targets, '$[*].type') ? 'VEN_NAME') IS NOT TRUE
"#,
id.as_str()
id.as_str(),
permission_filter.ven_ids.as_slice()
)
.fetch_one(&self.db)
.await?
Expand Down Expand Up @@ -257,15 +277,14 @@ impl Crud for PgProgramStorage {
WHERE ($1::text[] IS NULL OR TRUE) -- TODO implement filtering based on VEN names
AND ($2::text[] IS NULL OR e.event_name = ANY($2))
AND ($3::text[] IS NULL OR p.program_name = ANY($3))
AND ($4::text IS NULL OR jsonb_path_query_array(p.targets, '$[*].type') ? $4)
AND ($5::text[] IS NULL OR jsonb_path_query_array(p.targets, '$[*].values[*]') ?| ($5))
OFFSET $6 LIMIT $7
AND ($4::jsonb = '[]'::jsonb OR $4::jsonb <@ p.targets)
OFFSET $5 LIMIT $6
"#,
pg_filter.ven_names,
pg_filter.event_names,
pg_filter.program_names,
pg_filter.target_type,
pg_filter.target_values,
serde_json::to_value(pg_filter.targets)
.map_err(AppError::SerdeJsonInternalServerError)?,
pg_filter.skip,
pg_filter.limit,
)
Expand Down Expand Up @@ -393,10 +412,16 @@ mod tests {
payload_descriptors: Some(vec![PayloadDescriptor::EventPayloadDescriptor(
EventPayloadDescriptor::new(EventType::ExportPrice),
)]),
targets: Some(TargetMap(vec![TargetEntry {
label: TargetLabel::Group,
values: ["group-1".to_string()],
}])),
targets: Some(TargetMap(vec![
TargetEntry {
label: TargetLabel::Group,
values: ["group-1".to_string()],
},
TargetEntry {
label: TargetLabel::Private("PRIVATE_LABEL".to_string()),
values: ["private value".to_string()],
},
])),
},
}
}
Expand Down Expand Up @@ -542,6 +567,24 @@ mod tests {
.unwrap();
assert_eq!(programs.len(), 0);
}

#[sqlx::test(fixtures("programs"))]
async fn filter_multiple_targets(db: PgPool) {
let repo: PgProgramStorage = db.into();

let programs = repo
.retrieve_all(
&QueryParams {
target_type: Some(TargetLabel::Group),
target_values: Some(vec!["private value".to_string()]),
..Default::default()
},
&Claims::any_business_user(),
)
.await
.unwrap();
assert_eq!(programs.len(), 0);
}
}

mod get {
Expand Down
20 changes: 20 additions & 0 deletions openadr-vtn/src/data_source/postgres/report.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ impl TryFrom<PostgresReport> for Report {
}
}

struct PgId {
id: String,
}

#[async_trait]
impl Crud for PgReportStorage {
type Type = Report;
Expand All @@ -86,6 +90,22 @@ impl Crud for PgReportStorage {
new: Self::NewType,
_user: &Self::PermissionFilter,
) -> Result<Self::Type, Self::Error> {
let PgId { id } = sqlx::query_as!(
PgId,
r#"
SELECT program_id AS id FROM event WHERE id = $1
"#,
new.event_id.as_str(),
)
.fetch_one(&self.db)
.await?;

if id != new.program_id.as_str() {
return Err(AppError::BadRequest(
"event_id and program_id have to point to the same program",
));
}

Ok(sqlx::query_as!(
PostgresReport,
r#"
Expand Down

0 comments on commit 8f114a1

Please sign in to comment.