From 7b78cf0c00f3dd268b3175be4c9bdc727a406175 Mon Sep 17 00:00:00 2001 From: mimir-d Date: Tue, 8 Oct 2024 17:07:20 +0100 Subject: [PATCH] formalize the timestamp_provider mechanism - initially I thought that the Config object should be passed around (which I'm not yet convinced that this shouldnt be the case), but making an arbitrary choice here to limit the api surface - only expose the timestamp_provider methods from emitters - open to refactoring later, this is not public crate api Signed-off-by: mimir-d --- src/output/config.rs | 16 ++-------------- src/output/emitter.rs | 42 ++++++++++++++++++++++++++++++------------ src/output/step.rs | 17 ++++++++--------- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/output/config.rs b/src/output/config.rs index c765fea..35c0278 100644 --- a/src/output/config.rs +++ b/src/output/config.rs @@ -14,6 +14,7 @@ use crate::output::writer::{self, BufferWriter, FileWriter, StdoutWriter, Writer /// The configuration repository for the TestRun. pub struct Config { + // All fields are readable for any impl inside the crate. pub(crate) timestamp_provider: Box, pub(crate) writer: WriterType, } @@ -46,6 +47,7 @@ impl ConfigBuilder { } } + // TODO: docs for all these pub fn timezone(mut self, timezone: chrono_tz::Tz) -> Self { self.timestamp_provider = Box::new(ConfiguredTzProvider { tz: timezone }); self @@ -103,17 +105,3 @@ impl TimestampProvider for ConfiguredTzProvider { chrono::Local::now().with_timezone(&self.tz) } } - -pub struct NullTimestampProvider {} - -impl NullTimestampProvider { - // warn: linter is wrong here, this is used in a serde_json::json! block - #[allow(dead_code)] - pub const FORMATTED: &str = "1970-01-01T00:00:00.000Z"; -} - -impl TimestampProvider for NullTimestampProvider { - fn now(&self) -> chrono::DateTime { - chrono::DateTime::from_timestamp_nanos(0).with_timezone(&chrono_tz::UTC) - } -} diff --git a/src/output/emitter.rs b/src/output/emitter.rs index 21a2fa7..0d42aba 100644 --- a/src/output/emitter.rs +++ b/src/output/emitter.rs @@ -17,8 +17,7 @@ use crate::output::{ use crate::spec; pub struct JsonEmitter { - // HACK: public for tests, but this should come from config directly to where needed - pub(crate) timestamp_provider: Box, + timestamp_provider: Box, writer: writer::WriterType, seqno: Arc, } @@ -35,21 +34,26 @@ impl JsonEmitter { } } - fn serialize_artifact(&self, object: &spec::RootImpl) -> serde_json::Value { + fn incr_seqno(&self) -> u64 { + self.seqno.fetch_add(1, Ordering::AcqRel) + } + + fn serialize_artifact(&self, object: &spec::RootImpl) -> String { let root = spec::Root { artifact: object.clone(), timestamp: self.timestamp_provider.now(), seqno: self.incr_seqno(), }; - serde_json::json!(root) + + serde_json::json!(root).to_string() } - fn incr_seqno(&self) -> u64 { - self.seqno.fetch_add(1, Ordering::AcqRel) + pub fn timestamp_provider(&self) -> &(dyn config::TimestampProvider + Send + Sync + 'static) { + &*self.timestamp_provider } pub async fn emit(&self, object: &spec::RootImpl) -> Result<(), io::Error> { - let s = self.serialize_artifact(object).to_string(); + let s = self.serialize_artifact(object); match &self.writer { WriterType::File(file) => file.write(&s).await?, @@ -72,6 +76,20 @@ mod tests { use super::*; + pub struct NullTimestampProvider {} + + impl NullTimestampProvider { + // warn: linter is wrong here, this is used in a serde_json::json! block + #[allow(dead_code)] + pub const FORMATTED: &str = "1970-01-01T00:00:00.000Z"; + } + + impl config::TimestampProvider for NullTimestampProvider { + fn now(&self) -> chrono::DateTime { + chrono::DateTime::from_timestamp_nanos(0).with_timezone(&chrono_tz::UTC) + } + } + #[tokio::test] async fn test_emit_using_buffer_writer() -> Result<()> { let expected = json!({ @@ -80,13 +98,13 @@ mod tests { "minor": spec::SPEC_VERSION.1, }, "sequenceNumber": 0, - "timestamp": config::NullTimestampProvider::FORMATTED, + "timestamp": NullTimestampProvider::FORMATTED, }); let buffer = Arc::new(Mutex::new(vec![])); let writer = writer::BufferWriter::new(buffer.clone()); let emitter = JsonEmitter::new( - Box::new(config::NullTimestampProvider {}), + Box::new(NullTimestampProvider {}), writer::WriterType::Buffer(writer), ); @@ -112,7 +130,7 @@ mod tests { "minor": spec::SPEC_VERSION.1, }, "sequenceNumber": 0, - "timestamp": config::NullTimestampProvider::FORMATTED, + "timestamp": NullTimestampProvider::FORMATTED, }); let expected_2 = json!({ "schemaVersion": { @@ -120,13 +138,13 @@ mod tests { "minor": spec::SPEC_VERSION.1, }, "sequenceNumber": 1, - "timestamp": config::NullTimestampProvider::FORMATTED, + "timestamp": NullTimestampProvider::FORMATTED, }); let buffer = Arc::new(Mutex::new(vec![])); let writer = writer::BufferWriter::new(buffer.clone()); let emitter = JsonEmitter::new( - Box::new(config::NullTimestampProvider {}), + Box::new(NullTimestampProvider {}), writer::WriterType::Buffer(writer), ); diff --git a/src/output/step.rs b/src/output/step.rs index c02577e..9455077 100644 --- a/src/output/step.rs +++ b/src/output/step.rs @@ -9,10 +9,9 @@ use std::sync::atomic::{self, Ordering}; use std::sync::Arc; use crate::output as tv; -use crate::spec::TestStepStart; -use crate::spec::{self, TestStepArtifactImpl}; +use crate::spec::{self, TestStepArtifactImpl, TestStepStart}; use tv::measure::MeasurementSeries; -use tv::{emitter, error, log, measure}; +use tv::{config, emitter, error, log, measure}; use super::OcptvError; @@ -31,7 +30,7 @@ impl TestStep { name: name.to_owned(), emitter: Arc::new(StepEmitter { step_id: id.to_owned(), - run_emitter, + emitter: run_emitter, }), } } @@ -528,7 +527,8 @@ impl StartedTestStep { pub struct StepEmitter { step_id: String, - run_emitter: Arc, + // root emitter + emitter: Arc, } impl StepEmitter { @@ -538,13 +538,12 @@ impl StepEmitter { // TODO: can these copies be avoided? artifact: object.clone(), }); - self.run_emitter.emit(&root).await?; + self.emitter.emit(&root).await?; Ok(()) } - // HACK: - pub fn timestamp_provider(&self) -> &dyn tv::config::TimestampProvider { - &*self.run_emitter.timestamp_provider + pub fn timestamp_provider(&self) -> &(dyn config::TimestampProvider + Send + Sync + 'static) { + self.emitter.timestamp_provider() } }