From 63f1fde30f746995bb46dc6afa930ae87a8c8aa3 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 14 Jan 2025 15:57:04 +0000 Subject: [PATCH 1/5] Add failing test to demonstrate required behaviour --- src/graphql.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/graphql.rs b/src/graphql.rs index 44fe000d..05d6ef92 100644 --- a/src/graphql.rs +++ b/src/graphql.rs @@ -773,6 +773,37 @@ mod tests { ); } + #[rstest] + #[tokio::test] + async fn configuration_with_mismatched_numbers( + #[future(awt)] env: TestEnv, + ) -> Result<(), Box> { + tokio::fs::File::create_new(env.dir.as_ref().join("i22").join("5678.i22")) + .await + .unwrap(); + let query = r#"{ + configuration(beamline: "i22") { + latestScanNumber + } + }"#; + let result = env.schema.execute(query).await; + let exp = value!({ + "configuration": { + "latestScanNumber": 5678 + } + }); + assert!(result.errors.is_empty()); + assert_eq!(result.data, exp); + + let db_num = env.db.current_configuration("i22").await?.scan_number(); + let file_num = env.dir.as_ref().join("i22").join("5678.i22"); + let next_file_num = env.dir.as_ref().join("i22").join("5679.i22"); + assert_eq!(db_num, 122); + assert!(file_num.exists()); + assert!(!next_file_num.exists()); + Ok(()) + } + #[rstest] #[tokio::test] async fn empty_configure_for_existing(#[future(awt)] env: TestEnv) { From 99546590ca1992f612d4c93f9b530b492e1d17f0 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 14 Jan 2025 16:11:50 +0000 Subject: [PATCH 2/5] Check directory for highest number when returning configuration --- src/db_service.rs | 7 +++++++ src/graphql.rs | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/db_service.rs b/src/db_service.rs index 14740192..66191a46 100644 --- a/src/db_service.rs +++ b/src/db_service.rs @@ -89,6 +89,13 @@ impl BeamlineConfiguration { pub fn detector(&self) -> SqliteTemplateResult { self.detector.as_template() } + + pub fn with_scan_number(self, new_number: u32) -> Self { + Self { + scan_number: new_number, + ..self + } + } } impl<'r> FromRow<'r, SqliteRow> for BeamlineConfiguration { diff --git a/src/graphql.rs b/src/graphql.rs index 05d6ef92..0da1c99e 100644 --- a/src/graphql.rs +++ b/src/graphql.rs @@ -302,8 +302,16 @@ impl Query { ) -> async_graphql::Result { check_auth(ctx, |policy, token| policy.check_admin(token, &beamline)).await?; let db = ctx.data::()?; + let nt = ctx.data::()?; trace!("Getting config for {beamline:?}"); - Ok(db.current_configuration(&beamline).await?) + let conf = db.current_configuration(&beamline).await?; + let dir = nt.for_beamline(&beamline, conf.extension()).await?; + if let Some(high) = dir.prev().await?.filter(|n| n > &conf.scan_number()) { + // high file number in directory is higher than the configured value in the DB + Ok(conf.with_scan_number(high)) + } else { + Ok(conf) + } } } From 43a4c1732431c6c74bbeb272271752e8a0c95365 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 15 Jan 2025 11:24:09 +0000 Subject: [PATCH 3/5] Split configuration file number into db and file versions If the two numbers do not match, it is useful to see both in the configuration to help debugging issues, especially around interactions with the GDA files. --- src/db_service.rs | 7 ---- src/graphql.rs | 87 +++++++++++++++++++++++++++---------------- static/service_schema | 21 ++++++----- 3 files changed, 65 insertions(+), 50 deletions(-) diff --git a/src/db_service.rs b/src/db_service.rs index 66191a46..14740192 100644 --- a/src/db_service.rs +++ b/src/db_service.rs @@ -89,13 +89,6 @@ impl BeamlineConfiguration { pub fn detector(&self) -> SqliteTemplateResult { self.detector.as_template() } - - pub fn with_scan_number(self, new_number: u32) -> Self { - Self { - scan_number: new_number, - ..self - } - } } impl<'r> FromRow<'r, SqliteRow> for BeamlineConfiguration { diff --git a/src/graphql.rs b/src/graphql.rs index 0da1c99e..1a0dc376 100644 --- a/src/graphql.rs +++ b/src/graphql.rs @@ -147,6 +147,12 @@ struct ScanPaths { subdirectory: Subdirectory, } +/// GraphQL type to provide current configuration for a beamline +struct CurrentConfiguration { + db_config: BeamlineConfiguration, + high_file: Option, +} + /// Error to be returned when a path contains non-unicode characters #[derive(Debug)] struct NonUnicodePath; @@ -243,21 +249,24 @@ impl ScanPaths { } #[Object] -impl BeamlineConfiguration { +impl CurrentConfiguration { pub async fn visit_template(&self) -> async_graphql::Result { - Ok(self.visit()?.to_string()) + Ok(self.db_config.visit()?.to_string()) } pub async fn scan_template(&self) -> async_graphql::Result { - Ok(self.scan()?.to_string()) + Ok(self.db_config.scan()?.to_string()) } pub async fn detector_template(&self) -> async_graphql::Result { - Ok(self.detector()?.to_string()) + Ok(self.db_config.detector()?.to_string()) + } + pub async fn db_scan_number(&self) -> async_graphql::Result { + Ok(self.db_config.scan_number()) } - pub async fn latest_scan_number(&self) -> async_graphql::Result { - Ok(self.scan_number()) + pub async fn file_scan_number(&self) -> async_graphql::Result> { + Ok(self.high_file) } pub async fn tracker_file_extension(&self) -> async_graphql::Result> { - Ok(self.tracker_file_extension.as_deref()) + Ok(self.db_config.tracker_file_extension.as_deref()) } } @@ -299,19 +308,20 @@ impl Query { &self, ctx: &Context<'_>, beamline: String, - ) -> async_graphql::Result { + ) -> async_graphql::Result { check_auth(ctx, |policy, token| policy.check_admin(token, &beamline)).await?; let db = ctx.data::()?; let nt = ctx.data::()?; trace!("Getting config for {beamline:?}"); let conf = db.current_configuration(&beamline).await?; - let dir = nt.for_beamline(&beamline, conf.extension()).await?; - if let Some(high) = dir.prev().await?.filter(|n| n > &conf.scan_number()) { - // high file number in directory is higher than the configured value in the DB - Ok(conf.with_scan_number(high)) - } else { - Ok(conf) - } + let dir = nt + .for_beamline(&beamline, conf.tracker_file_extension.as_deref()) + .await?; + let high_file = dir.prev().await?; + Ok(CurrentConfiguration { + db_config: conf, + high_file, + }) } } @@ -363,15 +373,24 @@ impl Mutation { ctx: &Context<'ctx>, beamline: String, config: ConfigurationUpdates, - ) -> async_graphql::Result { + ) -> async_graphql::Result { check_auth(ctx, |pc, token| pc.check_admin(token, &beamline)).await?; let db = ctx.data::()?; trace!("Configuring: {beamline}: {config:?}"); - let upd = config.into_update(beamline); - match upd.update_beamline(db).await? { - Some(bc) => Ok(bc), - None => Ok(upd.insert_new(db).await?), - } + let upd = config.into_update(&beamline); + let db_config = match upd.update_beamline(db).await? { + Some(bc) => bc, + None => upd.insert_new(db).await?, + }; + let dir = ctx + .data::()? + .for_beamline(&beamline, db_config.tracker_file_extension.as_deref()) + .await?; + let high_file = dir.prev().await?; + Ok(CurrentConfiguration { + db_config, + high_file, + }) } } @@ -403,9 +422,9 @@ struct ConfigurationUpdates { } impl ConfigurationUpdates { - fn into_update(self, name: String) -> BeamlineConfigurationUpdate { + fn into_update>(self, name: S) -> BeamlineConfigurationUpdate { BeamlineConfigurationUpdate { - name, + name: name.into(), scan_number: self.scan_number, visit: self.visit.map(|t| t.0), scan: self.scan.map(|t| t.0), @@ -631,7 +650,7 @@ mod tests { Some(122), None, ); - cfg.into_update("i22".into()).insert_new(&db).await.unwrap(); + cfg.into_update("i22").insert_new(&db).await.unwrap(); let cfg = updates( Some("/tmp/{instrument}/data/{visit}/"), Some("{subdirectory}/{instrument}-{scan_number}"), @@ -639,7 +658,7 @@ mod tests { Some(621), Some("b21_ext"), ); - cfg.into_update("b21".into()).insert_new(&db).await.unwrap(); + cfg.into_update("b21").insert_new(&db).await.unwrap(); db } @@ -752,7 +771,7 @@ mod tests { async fn configuration(#[future(awt)] env: TestEnv) { let query = r#"{ configuration(beamline: "i22") { - visitTemplate scanTemplate detectorTemplate latestScanNumber trackerFileExtension + visitTemplate scanTemplate detectorTemplate dbScanNumber trackerFileExtension }}"#; let result = env.schema.execute(query).await; let exp = value!({ @@ -760,7 +779,7 @@ mod tests { "visitTemplate": "/tmp/{instrument}/data/{visit}", "scanTemplate": "{subdirectory}/{instrument}-{scan_number}", "detectorTemplate": "{subdirectory}/{instrument}-{scan_number}-{detector}", - "latestScanNumber": 122, + "dbScanNumber": 122, "trackerFileExtension": Value::Null }}); assert!(result.errors.is_empty()); @@ -791,13 +810,15 @@ mod tests { .unwrap(); let query = r#"{ configuration(beamline: "i22") { - latestScanNumber + dbScanNumber + fileScanNumber } }"#; let result = env.schema.execute(query).await; let exp = value!({ "configuration": { - "latestScanNumber": 5678 + "dbScanNumber": 122, + "fileScanNumber": 5678 } }); assert!(result.errors.is_empty()); @@ -817,7 +838,7 @@ mod tests { async fn empty_configure_for_existing(#[future(awt)] env: TestEnv) { let query = r#"mutation { configure(beamline: "i22", config: {}) { - visitTemplate scanTemplate detectorTemplate latestScanNumber + visitTemplate scanTemplate detectorTemplate dbScanNumber } }"#; let result = env.schema.execute(query).await; @@ -826,7 +847,7 @@ mod tests { "visitTemplate": "/tmp/{instrument}/data/{visit}", "scanTemplate": "{subdirectory}/{instrument}-{scan_number}", "detectorTemplate": "{subdirectory}/{instrument}-{scan_number}-{detector}", - "latestScanNumber": 122 + "dbScanNumber": 122 } }); println!("{result:#?}"); @@ -875,7 +896,7 @@ mod tests { scan: "{instrument}-{scan_number}" detector: "{scan_number}-{detector}" }) { - scanTemplate visitTemplate detectorTemplate latestScanNumber + scanTemplate visitTemplate detectorTemplate dbScanNumber } }"#, ) @@ -884,7 +905,7 @@ mod tests { "visitTemplate": "/tmp/{instrument}/{year}/{visit}", "scanTemplate": "{instrument}-{scan_number}", "detectorTemplate": "{scan_number}-{detector}", - "latestScanNumber": 0 + "dbScanNumber": 0 } }); assert!(result.errors.is_empty()); assert_eq!(result.data, exp); diff --git a/static/service_schema b/static/service_schema index 0254b3ff..c103d7ac 100644 --- a/static/service_schema +++ b/static/service_schema @@ -1,11 +1,3 @@ -type BeamlineConfiguration { - visitTemplate: String! - scanTemplate: String! - detectorTemplate: String! - latestScanNumber: Int! - trackerFileExtension: String -} - input ConfigurationUpdates { visit: VisitTemplate @@ -15,6 +7,15 @@ input ConfigurationUpdates { trackerFileExtension: String } +type CurrentConfiguration { + visitTemplate: String! + scanTemplate: String! + detectorTemplate: String! + dbScanNumber: Int! + fileScanNumber: Int + trackerFileExtension: String +} + scalar Detector """ @@ -40,12 +41,12 @@ type Mutation { Access scan file locations for the next scan """ scan(beamline: String!, visit: String!, sub: Subdirectory): ScanPaths! - configure(beamline: String!, config: ConfigurationUpdates!): BeamlineConfiguration! + configure(beamline: String!, config: ConfigurationUpdates!): CurrentConfiguration! } type Query { paths(beamline: String!, visit: String!): VisitPath! - configuration(beamline: String!): BeamlineConfiguration! + configuration(beamline: String!): CurrentConfiguration! } type ScanPaths { From 6d8846b1e6e590376f86d77461d5c5acc96e6dcb Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 21 Jan 2025 11:58:44 +0000 Subject: [PATCH 4/5] Revert tracker_file_extension being a method Now that it is wrapped in a CurrentConfiguration there is no conflict with the graphql version of the function. --- src/db_service.rs | 6 +++++- src/graphql.rs | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/db_service.rs b/src/db_service.rs index 14740192..2c36c68f 100644 --- a/src/db_service.rs +++ b/src/db_service.rs @@ -66,7 +66,7 @@ pub struct BeamlineConfiguration { visit: RawPathTemplate, scan: RawPathTemplate, detector: RawPathTemplate, - pub(crate) tracker_file_extension: Option, + tracker_file_extension: Option, } impl BeamlineConfiguration { @@ -89,6 +89,10 @@ impl BeamlineConfiguration { pub fn detector(&self) -> SqliteTemplateResult { self.detector.as_template() } + + pub fn tracker_file_extension(&self) -> Option<&str> { + self.tracker_file_extension.as_deref() + } } impl<'r> FromRow<'r, SqliteRow> for BeamlineConfiguration { diff --git a/src/graphql.rs b/src/graphql.rs index 1a0dc376..43898a5a 100644 --- a/src/graphql.rs +++ b/src/graphql.rs @@ -266,7 +266,7 @@ impl CurrentConfiguration { Ok(self.high_file) } pub async fn tracker_file_extension(&self) -> async_graphql::Result> { - Ok(self.db_config.tracker_file_extension.as_deref()) + Ok(self.db_config.tracker_file_extension()) } } @@ -315,7 +315,7 @@ impl Query { trace!("Getting config for {beamline:?}"); let conf = db.current_configuration(&beamline).await?; let dir = nt - .for_beamline(&beamline, conf.tracker_file_extension.as_deref()) + .for_beamline(&beamline, conf.tracker_file_extension()) .await?; let high_file = dir.prev().await?; Ok(CurrentConfiguration { @@ -347,7 +347,7 @@ impl Mutation { // isn't much we can do from here. let current = db.current_configuration(&beamline).await?; let dir = nt - .for_beamline(&beamline, current.tracker_file_extension.as_deref()) + .for_beamline(&beamline, current.tracker_file_extension()) .await?; let next_scan = db @@ -384,7 +384,7 @@ impl Mutation { }; let dir = ctx .data::()? - .for_beamline(&beamline, db_config.tracker_file_extension.as_deref()) + .for_beamline(&beamline, db_config.tracker_file_extension()) .await?; let high_file = dir.prev().await?; Ok(CurrentConfiguration { From 58437c3bb5b67b5def190825a96bfcad8226c13d Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 21 Jan 2025 12:18:47 +0000 Subject: [PATCH 5/5] Refactor configure nt access --- src/graphql.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/graphql.rs b/src/graphql.rs index 43898a5a..9a621a68 100644 --- a/src/graphql.rs +++ b/src/graphql.rs @@ -376,14 +376,14 @@ impl Mutation { ) -> async_graphql::Result { check_auth(ctx, |pc, token| pc.check_admin(token, &beamline)).await?; let db = ctx.data::()?; + let nt = ctx.data::()?; trace!("Configuring: {beamline}: {config:?}"); let upd = config.into_update(&beamline); let db_config = match upd.update_beamline(db).await? { Some(bc) => bc, None => upd.insert_new(db).await?, }; - let dir = ctx - .data::()? + let dir = nt .for_beamline(&beamline, db_config.tracker_file_extension()) .await?; let high_file = dir.prev().await?;