diff --git a/provider/adapters/src/fork/by_error.rs b/provider/adapters/src/fork/by_error.rs index aae9967619b..2cde1aaaf74 100644 --- a/provider/adapters/src/fork/by_error.rs +++ b/provider/adapters/src/fork/by_error.rs @@ -234,7 +234,7 @@ where F: ForkByErrorPredicate, { fn supported_locales_for_key(&self, key: DataKey) -> Result, DataError> { - let mut last_error = DataErrorKind::MissingDataKey.with_key(key); + let mut last_error = F::ERROR.with_key(key); for provider in self.providers.iter() { let result = provider.supported_locales_for_key(key); match result { @@ -260,7 +260,7 @@ where key: DataKey, mut from: DataPayload, ) -> Result, (DataPayload, DataError)> { - let mut last_error = DataErrorKind::MissingDataKey.with_key(key); + let mut last_error = F::ERROR.with_key(key); for provider in self.providers.iter() { let result = provider.convert(key, from); match result { diff --git a/provider/adapters/src/fork/predicates.rs b/provider/adapters/src/fork/predicates.rs index 0fefe57043e..2905d63bc77 100644 --- a/provider/adapters/src/fork/predicates.rs +++ b/provider/adapters/src/fork/predicates.rs @@ -10,6 +10,9 @@ use icu_provider::prelude::*; /// /// [`ForkByErrorProvider`]: super::ForkByErrorProvider pub trait ForkByErrorPredicate { + /// The error to return if there are zero providers. + const ERROR: DataErrorKind = DataErrorKind::MissingDataKey; + /// This function is called when a data request fails and there are additional providers /// that could possibly fulfill the request. /// @@ -43,6 +46,8 @@ pub trait ForkByErrorPredicate { pub struct MissingDataKeyPredicate; impl ForkByErrorPredicate for MissingDataKeyPredicate { + const ERROR: DataErrorKind = DataErrorKind::MissingDataKey; + #[inline] fn test(&self, _: DataKey, _: Option, err: DataError) -> bool { matches!( @@ -125,6 +130,8 @@ impl ForkByErrorPredicate for MissingDataKeyPredicate { pub struct MissingLocalePredicate; impl ForkByErrorPredicate for MissingLocalePredicate { + const ERROR: DataErrorKind = DataErrorKind::MissingLocale; + #[inline] fn test(&self, _: DataKey, _: Option, err: DataError) -> bool { matches!( diff --git a/provider/fs/Cargo.toml b/provider/fs/Cargo.toml index 77ee6605ada..c01d4532982 100644 --- a/provider/fs/Cargo.toml +++ b/provider/fs/Cargo.toml @@ -33,7 +33,6 @@ displaydoc = { version = "0.2.3", default-features = false } icu_provider = { version = "1.2.0", path = "../core", features = ["serde", "std"] } serde = { version = "1.0", default-features = false, features = ["derive", "alloc"] } serde-json-core = { version = "0.4", default-features = false, features = ["std"] } -writeable = { version = "0.5.1", path = "../../utils/writeable" } # Dependencies for the export feature bincode = { version = "1.3", optional = true } @@ -48,6 +47,7 @@ icu_benchmark_macros = { path = "../../tools/benchmark/macros" } icu_locid = { path = "../../components/locid", features = ["serde"] } icu_provider = { path = "../core", features = ["deserialize_json", "deserialize_bincode_1", "deserialize_postcard_1", "datagen"] } icu_datagen = { path = "../datagen" } +writeable = { path = "../../utils/writeable" } [features] # Enables the "export" module and FilesystemExporter diff --git a/provider/fs/src/export/fs_exporter.rs b/provider/fs/src/export/fs_exporter.rs index 3b6eaef71e9..6037805fe17 100644 --- a/provider/fs/src/export/fs_exporter.rs +++ b/provider/fs/src/export/fs_exporter.rs @@ -7,14 +7,14 @@ use crate::manifest::Manifest; use icu_provider::datagen::*; use icu_provider::prelude::*; use serde::{Deserialize, Serialize}; +use std::fmt::Write; use std::fs; #[allow(deprecated)] // We're using SipHash, which is deprecated, but we want a stable hasher // (we're fine with it not being cryptographically secure since we're just using it to track diffs) use std::hash::{Hasher, SipHasher}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Mutex; -use writeable::Writeable; /// Choices of what to do if [`FilesystemExporter`] tries to write to a pre-existing directory. #[non_exhaustive] @@ -119,15 +119,19 @@ impl DataExporter for FilesystemExporter { locale: &DataLocale, obj: &DataPayload, ) -> Result<(), DataError> { - let mut path_buf = self.root.clone(); - path_buf.push(&*key.write_to_string()); - path_buf.push(&*locale.write_to_string()); - path_buf.set_extension(self.manifest.file_extension); - - if let Some(parent_dir) = path_buf.parent() { - fs::create_dir_all(parent_dir) - .map_err(|e| DataError::from(e).with_path_context(&parent_dir))?; - } + let mut path_buf = self.root.clone().into_os_string(); + write!( + &mut path_buf, + "/{key}/{locale}.{}", + self.manifest.file_extension + ) + .expect("infallible"); + + #[allow(clippy::unwrap_used)] // has parent by construction + let parent_dir = Path::new(&path_buf).parent().unwrap(); + + fs::create_dir_all(parent_dir) + .map_err(|e| DataError::from(e).with_path_context(&parent_dir))?; let mut file = HashingFile { file: if self.serializer.is_text_format() { @@ -163,6 +167,16 @@ impl DataExporter for FilesystemExporter { Ok(()) } + fn flush_with_fallback(&self, key: DataKey, _: FallbackMode) -> Result<(), DataError> { + let mut path_buf = self.root.clone().into_os_string(); + write!(&mut path_buf, "/{key}").expect("infallible"); + + fs::create_dir_all(&path_buf) + .map_err(|e| DataError::from(e).with_path_context(&path_buf))?; + + Ok(()) + } + fn close(&mut self) -> Result<(), DataError> { if let Some(fingerprints) = self.fingerprints.as_mut() { let fingerprints = fingerprints.get_mut().expect("poison"); diff --git a/provider/fs/src/fs_data_provider.rs b/provider/fs/src/fs_data_provider.rs index 5d6b279bb1e..b300fde7f83 100644 --- a/provider/fs/src/fs_data_provider.rs +++ b/provider/fs/src/fs_data_provider.rs @@ -5,9 +5,10 @@ use crate::manifest::Manifest; use icu_provider::prelude::*; use std::fmt::Debug; +use std::fmt::Write; use std::fs; +use std::path::Path; use std::path::PathBuf; -use writeable::Writeable; /// A data provider that reads ICU4X data from a filesystem directory. /// @@ -70,17 +71,24 @@ impl BufferProvider for FsDataProvider { key: DataKey, req: DataRequest, ) -> Result, DataError> { - let mut path_buf = self.root.join(&*key.write_to_string()); - if !path_buf.exists() { + if key.metadata().singleton && !req.locale.is_empty() { + return Err(DataErrorKind::ExtraneousLocale.with_req(key, req)); + } + let mut path = self.root.clone().into_os_string(); + write!(&mut path, "/{key}").expect("infallible"); + if !Path::new(&path).exists() { return Err(DataErrorKind::MissingDataKey.with_req(key, req)); } - path_buf.push(&*req.locale.write_to_string()); - path_buf.set_extension(self.manifest.file_extension); - if !path_buf.exists() { + write!( + &mut path, + "/{}.{}", + req.locale, self.manifest.file_extension + ) + .expect("infallible"); + if !Path::new(&path).exists() { return Err(DataErrorKind::MissingLocale.with_req(key, req)); } - let buffer = - fs::read(&path_buf).map_err(|e| DataError::from(e).with_path_context(&path_buf))?; + let buffer = fs::read(&path).map_err(|e| DataError::from(e).with_path_context(&path))?; let mut metadata = DataResponseMetadata::default(); metadata.buffer_format = Some(self.manifest.buffer_format); Ok(DataResponse {