diff --git a/client-gui2 b/client-gui2 index 09dd5432..43a95c35 160000 --- a/client-gui2 +++ b/client-gui2 @@ -1 +1 @@ -Subproject commit 09dd543243fc720b1737cb4fa950586c80949726 +Subproject commit 43a95c353369d17b5fd235ad67cdedc76026ad71 diff --git a/common/build.rs b/common/build.rs index a8a51dd0..b4a1d4dc 100644 --- a/common/build.rs +++ b/common/build.rs @@ -9,6 +9,7 @@ fn main() -> Result<(), Box> { .type_attribute(".komp_ac.accounting", serde) .type_attribute(".komp_ac.analytics", serde) .field_attribute(".komp_ac.analytics.ReportDashboardFilter.primary", "#[serde(default)]") + .field_attribute(".komp_ac.analytics.ReportDashboardDefinition.queries", "#[serde(default)]") .type_attribute(".komp_ac.auth", serde) .type_attribute(".komp_ac.common", serde) .type_attribute(".komp_ac.document_data", serde) diff --git a/common/proto/analytics.proto b/common/proto/analytics.proto index abc1b054..6f7c66dd 100644 --- a/common/proto/analytics.proto +++ b/common/proto/analytics.proto @@ -99,6 +99,7 @@ service ReportingService { rpc ListAssets(ListReportAssetsRequest) returns (ListReportAssetsResponse); rpc GetAsset(GetReportAssetRequest) returns (ReportAsset); rpc SaveDraft(SaveReportDraftRequest) returns (ReportAsset); + rpc Validate(ValidateReportRequest) returns (ReportAsset); rpc Publish(PublishReportRequest) returns (ReportAsset); rpc ListVersions(ReportAssetRef) returns (ListReportVersionsResponse); rpc RestoreDraft(RestoreReportDraftRequest) returns (ReportAsset); @@ -376,6 +377,13 @@ message ReportDashboardDefinition { uint32 refresh_seconds = 6; repeated ReportDatasetVersionRef dataset_versions = 7; ReportPeriod period = 8; + // Queries owned by this report; keys are used by panels and filter targets. + repeated ReportQuery queries = 9; +} + +message ReportQuery { + string key = 1; + ReportDatasetDefinition definition = 2; } message ReportDatasetVersionRef { @@ -445,6 +453,11 @@ message SaveReportDraftRequest { ReportDefinition definition = 3; } +message ValidateReportRequest { + ReportAssetRef asset = 1; + uint64 expected_revision = 2; +} + message PublishReportRequest { ReportAssetRef asset = 1; uint64 expected_revision = 2; diff --git a/common/src/proto/descriptor.bin b/common/src/proto/descriptor.bin index a95be341..359038c8 100644 Binary files a/common/src/proto/descriptor.bin and b/common/src/proto/descriptor.bin differ diff --git a/common/src/proto/komp_ac.analytics.rs b/common/src/proto/komp_ac.analytics.rs index 482bc74b..19214f9e 100644 --- a/common/src/proto/komp_ac.analytics.rs +++ b/common/src/proto/komp_ac.analytics.rs @@ -426,6 +426,18 @@ pub struct ReportDashboardDefinition { pub dataset_versions: ::prost::alloc::vec::Vec, #[prost(message, optional, tag = "8")] pub period: ::core::option::Option, + /// Queries owned by this report; keys are used by panels and filter targets. + #[prost(message, repeated, tag = "9")] + #[serde(default)] + pub queries: ::prost::alloc::vec::Vec, +} +#[derive(serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct ReportQuery { + #[prost(string, tag = "1")] + pub key: ::prost::alloc::string::String, + #[prost(message, optional, tag = "2")] + pub definition: ::core::option::Option, } #[derive(serde::Serialize, serde::Deserialize)] #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] @@ -547,6 +559,14 @@ pub struct SaveReportDraftRequest { } #[derive(serde::Serialize, serde::Deserialize)] #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +pub struct ValidateReportRequest { + #[prost(message, optional, tag = "1")] + pub asset: ::core::option::Option, + #[prost(uint64, tag = "2")] + pub expected_revision: u64, +} +#[derive(serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] pub struct PublishReportRequest { #[prost(message, optional, tag = "1")] pub asset: ::core::option::Option, @@ -1725,6 +1745,29 @@ pub mod reporting_service_client { ); self.inner.unary(req, path, codec).await } + pub async fn validate( + &mut self, + request: impl tonic::IntoRequest, + ) -> std::result::Result, tonic::Status> { + self.inner + .ready() + .await + .map_err(|e| { + tonic::Status::unknown( + format!("Service was not ready: {}", e.into()), + ) + })?; + let codec = tonic_prost::ProstCodec::default(); + let path = http::uri::PathAndQuery::from_static( + "/komp_ac.analytics.ReportingService/Validate", + ); + let mut req = request.into_request(); + req.extensions_mut() + .insert( + GrpcMethod::new("komp_ac.analytics.ReportingService", "Validate"), + ); + self.inner.unary(req, path, codec).await + } pub async fn publish( &mut self, request: impl tonic::IntoRequest, @@ -1966,6 +2009,10 @@ pub mod reporting_service_server { &self, request: tonic::Request, ) -> std::result::Result, tonic::Status>; + async fn validate( + &self, + request: tonic::Request, + ) -> std::result::Result, tonic::Status>; async fn publish( &self, request: tonic::Request, @@ -2231,6 +2278,51 @@ pub mod reporting_service_server { }; Box::pin(fut) } + "/komp_ac.analytics.ReportingService/Validate" => { + #[allow(non_camel_case_types)] + struct ValidateSvc(pub Arc); + impl< + T: ReportingService, + > tonic::server::UnaryService + for ValidateSvc { + type Response = super::ReportAsset; + type Future = BoxFuture< + tonic::Response, + tonic::Status, + >; + fn call( + &mut self, + request: tonic::Request, + ) -> Self::Future { + let inner = Arc::clone(&self.0); + let fut = async move { + ::validate(&inner, request).await + }; + Box::pin(fut) + } + } + let accept_compression_encodings = self.accept_compression_encodings; + let send_compression_encodings = self.send_compression_encodings; + let max_decoding_message_size = self.max_decoding_message_size; + let max_encoding_message_size = self.max_encoding_message_size; + let inner = self.inner.clone(); + let fut = async move { + let method = ValidateSvc(inner); + let codec = tonic_prost::ProstCodec::default(); + let mut grpc = tonic::server::Grpc::new(codec) + .apply_compression_config( + accept_compression_encodings, + send_compression_encodings, + ) + .apply_max_message_size_config( + max_decoding_message_size, + max_encoding_message_size, + ); + let res = grpc.unary(method, req).await; + Ok(res) + }; + Box::pin(fut) + } "/komp_ac.analytics.ReportingService/Publish" => { #[allow(non_camel_case_types)] struct PublishSvc(pub Arc); diff --git a/common/src/reporting.rs b/common/src/reporting.rs index eca47544..0783e29d 100644 --- a/common/src/reporting.rs +++ b/common/src/reporting.rs @@ -133,6 +133,62 @@ pub fn validate_parameter(parameter: &ReportParameter, allow_lookup: bool) -> Re Ok(()) } +/// Drafts may have unfinished SQL and bindings. Their envelope and domain values remain typed. +pub fn validate_draft(definition: &ReportDefinition) -> Result<(), String> { + if definition.schema_version != REPORT_SCHEMA_VERSION { + return Err(format!("Unsupported report schema version {}", definition.schema_version)); + } + if serde_json::to_vec(definition).map_err(|error| error.to_string())?.len() > MAX_REPORT_BYTES { + return Err("Report definition exceeds 1 MiB".into()); + } + asset_kind(definition)?; + if title(definition).len() > 160 || description(definition).len() > 4000 { + return Err("Report title or description is too long".into()); + } + let parameter = |p: &ReportParameter| -> Result<(), String> { + enum_value::(p.data_type, "parameter type")?; + enum_value::(p.control, "filter control")?; + enum_value::(p.default_kind, "default kind")?; + for scalar in p.default_values.iter().chain(p.choices.iter().filter_map(|choice| choice.value.as_ref())) { + let value = scalar.value.as_ref().ok_or("A scalar value is required")?; + if matches!(value, report_scalar::Value::NullValue(code) if *code != 0) { return Err("Invalid null value".into()); } + } + for choice in &p.choices { choice.value.as_ref().ok_or("A choice value is required")?; } + Ok(()) + }; + let dataset = |d: &ReportDatasetDefinition| -> Result<(), String> { + for p in &d.parameters { parameter(p)?; } + for c in &d.columns { + enum_value::(c.data_type, "column type")?; + enum_value::(c.number_format, "number format")?; + } + Ok(()) + }; + match definition.content.as_ref().ok_or("A report definition is required")? { + report_definition::Content::Dataset(d) => dataset(d)?, + report_definition::Content::Dashboard(d) => { + for q in &d.queries { dataset(q.definition.as_ref().ok_or("A query definition is required")?)?; } + for f in &d.filters { parameter(f.parameter.as_ref().ok_or("A filter parameter is required")?)?; } + for p in &d.panels { + enum_value::(p.kind, "chart kind")?; + enum_value::(p.orientation, "orientation")?; + enum_value::(p.sort_order, "sort order")?; + enum_value::(p.null_policy, "null policy")?; + for action in &p.actions { action.target.as_ref().ok_or("An action target is required")?; } + } + for g in &d.grants { + g.subject.as_ref().ok_or("A grant subject is required")?; + for c in &g.capabilities { enum_value::(*c, "capability")?; } + } + if let Some(period) = &d.period { period.selection.as_ref().ok_or("A period selection is required")?; } + if let Some(report_period::Selection::DateRange(range)) = d.period.as_ref().and_then(|p| p.selection.as_ref()) { + enum_value::(range.end_boundary, "period end boundary")?; + } + } + } + Ok(()) +} + pub fn validate_definition(definition: &ReportDefinition) -> Result<(), String> { if definition.schema_version != REPORT_SCHEMA_VERSION { return Err(format!( @@ -225,10 +281,18 @@ fn validate_dataset(dataset: &ReportDatasetDefinition) -> Result<(), String> { } fn validate_dashboard(dashboard: &ReportDashboardDefinition) -> Result<(), String> { + let mut query_keys = HashSet::new(); + if dashboard.queries.len() > 64 { return Err("A report allows at most 64 queries".into()); } + for query in &dashboard.queries { + nonempty(&query.key, "Query key", 64)?; + if !query_keys.insert(query.key.as_str()) { return Err(format!("Duplicate query key '{}'", query.key)); } + let dataset = query.definition.as_ref().ok_or_else(|| format!("SQL · {}: query definition is missing", query.key))?; + validate_dataset(dataset).map_err(|error| format!("SQL · {}: {error}", query.key))?; + } let referenced = referenced_datasets(dashboard); let mut dataset_versions = HashSet::new(); for reference in &dashboard.dataset_versions { - if !referenced.contains(reference.dataset_id.as_str()) || !dataset_versions.insert(&reference.dataset_id) { + if query_keys.contains(reference.dataset_id.as_str()) || !referenced.contains(reference.dataset_id.as_str()) || !dataset_versions.insert(&reference.dataset_id) { return Err("Dataset version references must be unique and used by the dashboard".into()); } } @@ -303,16 +367,16 @@ fn validate_dashboard(dashboard: &ReportDashboardDefinition) -> Result<(), Strin let sort: ReportSortOrder = enum_value(panel.sort_order, "sort order")?; let _: ReportNullPolicy = enum_value(panel.null_policy, "null policy")?; if !(1..=12).contains(&panel.width) || !(160..=1200).contains(&panel.height) { - return Err("Panel width must be 1–12 and height 160–1200".into()); + return Err(format!("Charts · {} [{}]: Panel width must be 1–12 and height 160–1200", panel.title, panel.id)); } if sort != ReportSortOrder::Source && panel.sort_field.is_empty() { - return Err("A sorted panel needs a sort field".into()); + return Err(format!("Charts · {} [{}]: A sorted panel needs a sort field", panel.title, panel.id)); } if panel.axis_min.is_some_and(|number| !number.is_finite()) || panel.axis_max.is_some_and(|number| !number.is_finite()) || matches!((panel.axis_min, panel.axis_max), (Some(min), Some(max)) if min >= max) { - return Err("Invalid chart axis bounds".into()); + return Err(format!("Charts · {} [{}]: Invalid chart axis bounds", panel.title, panel.id)); } if panel.colors.len() > 32 || panel.colors.iter().any(|color| { @@ -321,10 +385,10 @@ fn validate_dashboard(dashboard: &ReportDashboardDefinition) -> Result<(), Strin || !color[1..].bytes().all(|byte| byte.is_ascii_hexdigit()) }) { - return Err("Chart colors must be hexadecimal CSS colors".into()); + return Err(format!("Charts · {} [{}]: Chart colors must be hexadecimal CSS colors", panel.title, panel.id)); } if panel.actions.len() > 8 { - return Err("A panel can have at most eight actions".into()); + return Err(format!("Charts · {} [{}]: A panel can have at most eight actions", panel.title, panel.id)); } for action in &panel.actions { nonempty(&action.label, "Action label", 160)?; @@ -483,7 +547,7 @@ pub fn validate_dashboard_datasets( if panel.y_fields.iter().collect::>().len() != panel.y_fields.len() || panel.table_fields.iter().collect::>().len() != panel.table_fields.len() { - return Err("Panel field selections must be unique".into()); + return Err(format!("Charts · {} [{}]: Panel field selections must be unique", panel.title, panel.id)); } let field = |key: &str| { columns @@ -506,12 +570,12 @@ pub fn validate_dashboard_datasets( } if kind != ReportPanelKind::Table { if panel.y_fields.is_empty() || panel.y_fields.len() > 16 { - return Err("Charts need between 1 and 16 measures".into()); + return Err(format!("Charts · {} [{}]: Charts need between 1 and 16 measures", panel.title, panel.id)); } for key in &panel.y_fields { let kind: ReportDataType = enum_value(field(key)?.data_type, "measure type")?; if !matches!(kind, ReportDataType::Integer | ReportDataType::Decimal) { - return Err("Chart measures must be numeric".into()); + return Err(format!("Charts · {} [{}]: Chart measures must be numeric", panel.title, panel.id)); } } if !matches!(kind, ReportPanelKind::Kpi | ReportPanelKind::Gauge) { @@ -529,7 +593,7 @@ pub fn validate_dashboard_datasets( | ReportPanelKind::Kpi ) && panel.y_fields.len() != 1 { - return Err("This panel kind requires exactly one measure".into()); + return Err(format!("Charts · {} [{}]: This panel kind requires exactly one measure", panel.title, panel.id)); } if kind == ReportPanelKind::Heatmap { field(&panel.series_field)?; @@ -540,7 +604,7 @@ pub fn validate_dashboard_datasets( ].into_iter().flatten() { let data_type: ReportDataType = enum_value(field(key)?.data_type, "numeric field type")?; if !matches!(data_type, ReportDataType::Integer | ReportDataType::Decimal) { - return Err("Scatter coordinates and size fields must be numeric".into()); + return Err(format!("Charts · {} [{}]: Scatter coordinates and size fields must be numeric", panel.title, panel.id)); } } } @@ -575,3 +639,48 @@ pub fn validate_dashboard_datasets( } Ok(()) } + +#[cfg(test)] +mod authoring_tests { + use super::*; + + fn query() -> ReportDatasetDefinition { + ReportDatasetDefinition { + title: "Sales".into(), sql: "SELECT 1 AS total".into(), max_rows: 100, + columns: vec![ReportDatasetColumn { key: "total".into(), label: "Total".into(), + data_type: ReportDataType::Integer.into(), number_format: ReportNumberFormat::Number.into(), ..Default::default() }], + ..Default::default() + } + } + + #[test] + fn unfinished_drafts_save_but_cannot_publish_and_unknown_types_are_rejected() { + let mut dataset = query(); + dataset.sql.clear(); + let definition = |dataset| ReportDefinition { schema_version: REPORT_SCHEMA_VERSION, + content: Some(report_definition::Content::Dataset(dataset)) }; + assert!(validate_draft(&definition(dataset.clone())).is_ok()); + assert!(validate_definition(&definition(dataset.clone())).is_err()); + dataset.columns[0].data_type = 999; + assert!(validate_draft(&definition(dataset)).is_err()); + } + + #[test] + fn embedded_queries_require_unique_keys_and_cannot_have_external_version_pins() { + let mut dashboard = ReportDashboardDefinition { + title: "Sales".into(), queries: vec![ReportQuery { key: "sales".into(), definition: Some(query()) }], + panels: vec![ReportPanel { id: "total".into(), title: "Sales".into(), dataset_id: "sales".into(), + kind: ReportPanelKind::Table.into(), orientation: ReportOrientation::Vertical.into(), + sort_order: ReportSortOrder::Source.into(), null_policy: ReportNullPolicy::Gap.into(), width: 6, height: 360, + ..Default::default() }], ..Default::default() + }; + let definition = |dashboard| ReportDefinition { schema_version: REPORT_SCHEMA_VERSION, + content: Some(report_definition::Content::Dashboard(dashboard)) }; + assert!(validate_definition(&definition(dashboard.clone())).is_ok()); + dashboard.queries.push(dashboard.queries[0].clone()); + assert!(validate_definition(&definition(dashboard.clone())).is_err()); + dashboard.queries.pop(); + dashboard.dataset_versions.push(ReportDatasetVersionRef { dataset_id: "sales".into(), version: 1 }); + assert!(validate_definition(&definition(dashboard)).is_err()); + } +} diff --git a/komp-app/src/grpc/reporting.rs b/komp-app/src/grpc/reporting.rs index bf030f81..d8969d82 100644 --- a/komp-app/src/grpc/reporting.rs +++ b/komp-app/src/grpc/reporting.rs @@ -39,6 +39,15 @@ impl GrpcClient { Ok(response.into_inner()) } + pub async fn validate_report(&mut self, request: ValidateReportRequest) -> Result { + let request = self.authenticated_request(request)?; + let response = ReportingServiceClient::new(self.channel.clone()) + .validate(request) + .await + .context("gRPC ReportingService validate call failed")?; + Ok(response.into_inner()) + } + pub async fn publish_report(&mut self, request: PublishReportRequest) -> Result { let request = self.authenticated_request(request)?; let response = ReportingServiceClient::new(self.channel.clone()) diff --git a/server b/server index 7efce3df..96339d88 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit 7efce3df1842958d09b4501ebb5199b6807f127d +Subproject commit 96339d88c4322cc8957d28524fca757f7e4ba8fa