Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace SerdeDeDataProvider with BufferProvider #1369

Merged
merged 51 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
8c7c922
Add serde_format to DataResponseMetadata
sffc Nov 25, 2021
7a0ccfe
Review feedback from previous PR
sffc Nov 25, 2021
84e151e
serde_format --> buffer_format; add attributes
sffc Nov 25, 2021
c5930db
rm attributes and make it non_exhaustive instead
sffc Nov 30, 2021
78497e6
fmt
sffc Nov 30, 2021
9464db3
Checkpoint
sffc Nov 30, 2021
d3c7596
Checkpoint
sffc Nov 30, 2021
eda0521
Checkpoint
sffc Nov 30, 2021
db58d37
Checkpoint: migrate fs provider to BufferFormat enum
sffc Nov 30, 2021
22b981b
Checkpoint: all of ICU4X building again
sffc Nov 30, 2021
ed238c3
fmt
sffc Dec 1, 2021
21b58ea
Checkpoint
sffc Dec 1, 2021
f3a4bdb
impl BufferProvider for FsDataProvider
sffc Dec 1, 2021
c59b20b
Migrate FsDataProvider to use SerdeBufferProvider for deserialization
sffc Dec 1, 2021
1697c85
Remove obsolete comment
sffc Dec 1, 2021
c4a79de
Small fix
sffc Dec 8, 2021
bc57f68
Merge remote-tracking branch 'upstream/main' into dp-work
sffc Dec 8, 2021
2bea550
More small fixes
sffc Dec 8, 2021
9767307
Checkpoint
sffc Dec 8, 2021
3e0a9a8
rm SerdeDeDataProvider
sffc Dec 8, 2021
a170c86
Rename method
sffc Dec 8, 2021
a868486
Small doc
sffc Dec 8, 2021
409c7ba
Checkpoint
sffc Dec 8, 2021
f061785
Checkpoint
sffc Dec 8, 2021
3f7a832
Checkpoint
sffc Dec 8, 2021
c0d355f
Checkpoint
sffc Dec 8, 2021
e740fae
Checkpoint
sffc Dec 8, 2021
a26a57b
fmt
sffc Dec 8, 2021
eb9f504
Update provider/core/src/buffer_provider.rs
sffc Dec 8, 2021
5563762
Resolve TODO in AbstractSerializer
sffc Dec 8, 2021
ad6e666
Comment on catchall match
sffc Dec 9, 2021
79a9abc
Fix test
sffc Dec 9, 2021
782c458
Checkpoint: add SerializingDataProvider
sffc Dec 9, 2021
a6af6bb
Checkpoint
sffc Dec 9, 2021
ffc21ae
Checkpoint
sffc Dec 9, 2021
d9796d4
Undo SerializingDataProvider
sffc Dec 9, 2021
18219d4
Checkpoint: serializable helper methods
sffc Dec 9, 2021
b9ecbe5
Rename serialization primitives
sffc Dec 9, 2021
67cb8cb
Update docs
sffc Dec 9, 2021
2a6f37b
Error refactoring
sffc Dec 9, 2021
613c76b
Rename features
sffc Dec 9, 2021
6a3a244
Everything building again
sffc Dec 9, 2021
9b062fb
Fix features build
sffc Dec 9, 2021
db1ad70
fmt
sffc Dec 9, 2021
cec8aed
Fix features in blob provider
sffc Dec 9, 2021
0d6be7e
Re-gen readme
sffc Dec 9, 2021
9d9af9e
Test fixes
sffc Dec 9, 2021
cb8b13c
A few more feature fixes
sffc Dec 9, 2021
bd5b889
Error message feedback
sffc Dec 9, 2021
2b83989
Merge remote-tracking branch 'upstream/main' into dp-work
sffc Dec 9, 2021
5619631
Merge remote-tracking branch 'sffc/dp-work' into dp-work
sffc Dec 9, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion experimental/segmenter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ criterion = "0.3"
serde-json-core = { version = "0.4", features = ["std"] }

[build-dependencies]
icu_provider_fs = { version = "0.4", path = "../../provider/fs", features = ["provider_json"] }
icu_provider_fs = { version = "0.4", path = "../../provider/fs", features = ["deserialize_json"] }
icu = { version = "0.4", path = "../../components/icu" }
icu_testdata = { version = "0.4", path = "../../provider/testdata" }
serde = { version = "1.0", features = ["derive"] }
Expand Down
4 changes: 2 additions & 2 deletions ffi/diplomat/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ path = "src/lib.rs"
default = ["provider_fs", "provider_static"]
wearos = ["smaller_static", "freertos"]

provider_fs = ["icu_provider_fs", "icu_provider_fs/provider_json"]
provider_fs = ["icu_provider_fs", "icu_provider_fs/deserialize_json"]
provider_static = ["icu_testdata"]
smaller_static = ["provider_static"]

Expand All @@ -65,7 +65,7 @@ icu_locid = { path = "../../components/locid" }
icu_locid_macros = { path = "../../components/locid/macros" }
icu_plurals = { path = "../../components/plurals/" }
icu_properties = { path = "../../components/properties/" }
icu_provider = { path = "../../provider/core", features = ["provider_serde"] }
icu_provider = { path = "../../provider/core", features = ["serde"] }
icu_provider_blob = { path = "../../provider/blob" }
tinystr = { version = "0.4.10", features = ["alloc"], default-features = false }
writeable = { path = "../../utils/writeable/" }
Expand Down
5 changes: 3 additions & 2 deletions ffi/diplomat/src/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ pub mod ffi {
provider: &ICU4XDataProvider,
options: ICU4XFixedDecimalFormatOptions,
) -> ICU4XFixedDecimalFormatResult {
let provider = provider.0.as_ref();
Self::try_new_impl(locale, provider, options)
use icu_provider::serde::AsDeserializingBufferProvider;
let provider = provider.0.as_deserializing();
Self::try_new_impl(locale, &provider, options)
}

/// Creates a new [`ICU4XFixedDecimalFormat`] from a [`ICU4XStaticDataProvider`].
Expand Down
5 changes: 3 additions & 2 deletions ffi/diplomat/src/locale_canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ pub mod ffi {
/// Create a new [`ICU4XLocaleCanonicalizer`].
/// See [the Rust docs](https://unicode-org.github.io/icu4x-docs/doc/icu/locale_canonicalizer/struct.LocaleCanonicalizer.html#method.new) for more details.
pub fn create(provider: &ICU4XDataProvider) -> Option<Box<ICU4XLocaleCanonicalizer>> {
let provider = provider.0.as_ref();
LocaleCanonicalizer::new(provider)
use icu_provider::serde::AsDeserializingBufferProvider;
let provider = provider.0.as_deserializing();
LocaleCanonicalizer::new(&provider)
.ok()
.map(|lc| Box::new(ICU4XLocaleCanonicalizer(lc)))
}
Expand Down
5 changes: 3 additions & 2 deletions ffi/diplomat/src/pluralrules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ pub mod ffi {
provider: &ICU4XDataProvider,
ty: ICU4XPluralRuleType,
) -> ICU4XCreatePluralRulesResult {
let provider = provider.0.as_ref();
Self::try_new_impl(locale, provider, ty)
use icu_provider::serde::AsDeserializingBufferProvider;
let provider = provider.0.as_deserializing();
Self::try_new_impl(locale, &provider, ty)
}

/// Creates a new [`ICU4XPluralRules`] from a [`ICU4XStaticDataProvider`].
Expand Down
5 changes: 3 additions & 2 deletions ffi/diplomat/src/properties_maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ pub mod ffi {
/// Gets a map for Unicode property Script from a [`ICU4XDataProvider`].
/// See [the Rust docs](https://unicode-org.github.io/icu4x-docs/doc/icu_properties/maps/fn.get_script.html) for more information.
pub fn try_get_script(provider: &ICU4XDataProvider) -> ICU4XCodePointMapData16Response {
let provider = provider.0.as_ref();
Self::prepare_result_from_script(maps::get_script(provider))
use icu_provider::serde::AsDeserializingBufferProvider;
let provider = provider.0.as_deserializing();
Self::prepare_result_from_script(maps::get_script(&provider))
}

/// Gets a map for Unicode property Script from a [`ICU4XStaticDataProvider`].
Expand Down
5 changes: 3 additions & 2 deletions ffi/diplomat/src/properties_sets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ pub mod ffi {
pub fn try_get_ascii_hex_digit(
provider: &ICU4XDataProvider,
) -> ICU4XCodePointSetDataResult {
let provider = provider.0.as_ref();
Self::prepare_result(sets::get_ascii_hex_digit(provider))
use icu_provider::serde::AsDeserializingBufferProvider;
let provider = provider.0.as_deserializing();
Self::prepare_result(sets::get_ascii_hex_digit(&provider))
}

/// Gets a set for Unicode property ascii_hex_digit from a [`ICU4XStaticDataProvider`].
Expand Down
18 changes: 7 additions & 11 deletions ffi/diplomat/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub mod ffi {
))]
use alloc::string::ToString;

use icu_provider::serde::SerdeDeDataProvider;
use icu_provider::prelude::BufferProvider;
use icu_provider_blob::BlobDataProvider;
use icu_provider_blob::StaticDataProvider;
#[cfg(all(
Expand All @@ -23,7 +23,7 @@ pub mod ffi {
#[diplomat::opaque]
/// An ICU4X data provider, capable of loading ICU4X data keys from some source.
/// See [the Rust docs](https://unicode-org.github.io/icu4x-docs/doc/icu_provider/prelude/trait.DataProvider.html) for more information.
pub struct ICU4XDataProvider(pub Box<dyn SerdeDeDataProvider + 'static>);
pub struct ICU4XDataProvider(pub Box<dyn BufferProvider + 'static>);

/// A result type for `ICU4XDataProvider::create`.
pub struct ICU4XCreateDataProviderResult {
Expand All @@ -43,13 +43,10 @@ pub mod ffi {
not(any(target_arch = "wasm32", target_os = "none"))
))]
match FsDataProvider::try_new(path.to_string()) {
Ok(fs) => {
let erased = Box::new(fs);
ICU4XCreateDataProviderResult {
provider: Some(Box::new(ICU4XDataProvider(erased))),
success: true,
}
}
Ok(fs) => ICU4XCreateDataProviderResult {
provider: Some(Box::new(ICU4XDataProvider(Box::new(fs)))),
success: true,
},
Err(_) => ICU4XCreateDataProviderResult {
provider: None,
success: false,
Expand All @@ -75,9 +72,8 @@ pub mod ffi {
let provider = icu_testdata::get_smaller_static_provider();
#[cfg(not(feature = "smaller_static"))]
let provider = icu_testdata::get_static_provider();
let erased = Box::new(provider);
ICU4XCreateDataProviderResult {
provider: Some(Box::new(ICU4XDataProvider(erased))),
provider: Some(Box::new(ICU4XDataProvider(Box::new(provider)))),
success: true,
}
}
Expand Down
6 changes: 4 additions & 2 deletions ffi/diplomat/wasm/test/data-providers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ test("use create_from_byte_slice to format a simple decimal", async t => {
const bytes = new Uint8Array(nodeBuffer.buffer, nodeBuffer.byteOffset, nodeBuffer.length);
const result = ICU4XDataProvider.create_from_byte_slice(bytes);
t.assert(result.success);
const format = ICU4XFixedDecimalFormat.try_new(locale, result.provider, ICU4XFixedDecimalFormatOptions.default()).fdf;

const format = ICU4XFixedDecimalFormat.try_new(locale, result.provider, ICU4XFixedDecimalFormatOptions.default());
t.assert(format.success);

const decimal = ICU4XFixedDecimal.create(1234);
decimal.multiply_pow10(-2);

t.is(format.format(decimal), "১২.৩৪");
t.is(format.fdf.format(decimal), "১২.৩৪");
});

test("fail to create from invalid buffer", t => {
Expand Down
4 changes: 2 additions & 2 deletions provider/blob/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ include = [
all-features = true

[dependencies]
icu_provider = { version = "0.4", path = "../../provider/core", features = ["provider_serde"] }
icu_provider = { version = "0.4", path = "../../provider/core", features = ["deserialize_postcard_07"] }
icu_locid = { version = "0.4", path = "../../components/locid", features = ["serde"] }
serde = { version = "1.0", default-features = false, features = ["alloc"] }
postcard = { version = "0.7.0", default-features = false }
Expand All @@ -49,5 +49,5 @@ icu_locid_macros = { version = "0.4", path = "../../components/locid/macros" }
path = "src/lib.rs"

[features]
export = ["log", "postcard/alloc", "std", "litemap"]
export = ["log", "postcard/alloc", "std", "litemap", "icu_provider/serialize"]
std = ["icu_locid/std", "icu_provider/std"]
31 changes: 14 additions & 17 deletions provider/blob/src/blob_data_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::blob_schema::BlobSchema;
use crate::path_util;
use alloc::rc::Rc;
use alloc::string::String;
use icu_provider::buffer_provider::BufferFormat;
use icu_provider::prelude::*;
use icu_provider::serde::{SerdeDeDataProvider, SerdeDeDataReceiver};
use serde::de::Deserialize;
use yoke::trait_hack::YokeTraitHack;
use yoke::*;
Expand Down Expand Up @@ -111,28 +111,25 @@ where
.map_err(DataError::new_resc_error)?;
Ok(data.0)
})?;
let mut metadata = DataResponseMetadata::default();
// TODO(#1109): Set metadata.data_langid correctly.
metadata.buffer_format = Some(BufferFormat::Postcard07);
Ok(DataResponse {
metadata: DataResponseMetadata {
data_langid: req.resource_path.options.langid.clone(),
},
metadata,
payload: Some(payload),
})
}
}

impl SerdeDeDataProvider for BlobDataProvider {
fn load_to_receiver(
&self,
req: &DataRequest,
receiver: &mut dyn SerdeDeDataReceiver,
) -> Result<DataResponseMetadata, DataError> {
let file = self.get_file(req)?;
receiver.receive_yoked_buffer(file, |bytes, f2| {
let mut d = postcard::Deserializer::from_bytes(bytes);
f2(&mut <dyn erased_serde::Deserializer>::erase(&mut d))
})?;
Ok(DataResponseMetadata {
data_langid: req.resource_path.options.langid.clone(),
impl BufferProvider for BlobDataProvider {
fn load_buffer(&self, req: DataRequest) -> Result<DataResponse<BufferMarker>, DataError> {
let yoked_buffer = self.get_file(&req)?;
let mut metadata = DataResponseMetadata::default();
// TODO(#1109): Set metadata.data_langid correctly.
metadata.buffer_format = Some(BufferFormat::Postcard07);
Ok(DataResponse {
metadata,
payload: Some(DataPayload::from_yoked_buffer(yoked_buffer)),
})
}
}
31 changes: 15 additions & 16 deletions provider/blob/src/export/blob_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::blob_schema::*;
use crate::path_util;
use icu_provider::export::DataExporter;
use icu_provider::prelude::*;
use icu_provider::serde::SerdeSeDataStructMarker;
use icu_provider::serde::SerializeMarker;
use litemap::LiteMap;
use zerovec::ZeroMap;

Expand Down Expand Up @@ -35,25 +35,19 @@ impl Drop for BlobExporter<'_> {
}
}

/// TODO(#837): De-duplicate this code from icu_provider_fs.
fn serialize(obj: &dyn erased_serde::Serialize) -> Result<Vec<u8>, DataError> {
let mut serializer = postcard::Serializer {
output: postcard::flavors::AllocVec(Vec::new()),
};
obj.erased_serialize(&mut <dyn erased_serde::Serializer>::erase(&mut serializer))?;
Ok(serializer.output.0)
}

impl DataExporter<SerdeSeDataStructMarker> for BlobExporter<'_> {
impl DataExporter<SerializeMarker> for BlobExporter<'_> {
fn put_payload(
&mut self,
req: DataRequest,
obj: DataPayload<SerdeSeDataStructMarker>,
payload: DataPayload<SerializeMarker>,
) -> Result<(), DataError> {
let path = path_util::resource_path_to_string(&req.resource_path);
log::trace!("Adding: {}", path);
let buffer = serialize(obj.get().as_serialize())?;
self.resources.insert(path, buffer);
let mut serializer = postcard::Serializer {
output: postcard::flavors::AllocVec(Vec::new()),
};
payload.serialize(&mut <dyn erased_serde::Serializer>::erase(&mut serializer))?;
self.resources.insert(path, serializer.output.0);
Ok(())
}

Expand All @@ -67,8 +61,13 @@ impl DataExporter<SerdeSeDataStructMarker> for BlobExporter<'_> {
resources: zm.as_borrowed(),
});
log::info!("Serializing blob to output stream...");
let vec = serialize(&blob)?;
self.sink.write(&vec).map_err(|e| e.to_string())?;
let mut serializer = postcard::Serializer {
output: postcard::flavors::AllocVec(Vec::new()),
};
serde::Serialize::serialize(&blob, &mut serializer).map_err(DataError::new_resc_error)?;
self.sink
.write(&serializer.output.0)
.map_err(|e| e.to_string())?;
self.resources.clear();
Ok(())
}
Expand Down
31 changes: 14 additions & 17 deletions provider/blob/src/static_data_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

use crate::blob_schema::*;
use crate::path_util;
use icu_provider::buffer_provider::BufferFormat;
use icu_provider::prelude::*;
use icu_provider::serde::{SerdeDeDataProvider, SerdeDeDataReceiver};
use serde::de::Deserialize;
use zerovec::map::ZeroMapBorrowed;

Expand Down Expand Up @@ -117,28 +117,25 @@ where
let file = self.get_file(req)?;
let data = M::Yokeable::deserialize(&mut postcard::Deserializer::from_bytes(file))
.map_err(DataError::new_resc_error)?;
let mut metadata = DataResponseMetadata::default();
// TODO(#1109): Set metadata.data_langid correctly.
metadata.buffer_format = Some(BufferFormat::Postcard07);
Ok(DataResponse {
metadata: DataResponseMetadata {
data_langid: req.resource_path.options.langid.clone(),
},
metadata,
payload: Some(DataPayload::from_owned(data)),
})
}
}

impl SerdeDeDataProvider for StaticDataProvider {
fn load_to_receiver(
&self,
req: &DataRequest,
receiver: &mut dyn SerdeDeDataReceiver,
) -> Result<DataResponseMetadata, DataError> {
let file = self.get_file(req)?;
receiver.receive_static(&mut <dyn erased_serde::Deserializer>::erase(
&mut postcard::Deserializer::from_bytes(file),
))?;

Ok(DataResponseMetadata {
data_langid: req.resource_path.options.langid.clone(),
impl BufferProvider for StaticDataProvider {
fn load_buffer(&self, req: DataRequest) -> Result<DataResponse<BufferMarker>, DataError> {
let static_buffer = self.get_file(&req)?;
let mut metadata = DataResponseMetadata::default();
// TODO(#1109): Set metadata.data_langid correctly.
metadata.buffer_format = Some(BufferFormat::Postcard07);
Ok(DataResponse {
metadata,
payload: Some(DataPayload::from_static_buffer(static_buffer)),
})
}
}
2 changes: 1 addition & 1 deletion provider/cldr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ extra_features = [
all-features = true

[dependencies]
icu_provider = { version = "0.4", path = "../../provider/core", features = ["provider_serde"] }
icu_provider = { version = "0.4", path = "../../provider/core", features = ["serialize"] }
icu_locid = { version = "0.4", path = "../../components/locid" }
icu_plurals = { version = "0.4", path = "../../components/plurals" }
icu_datetime = { version = "0.4", path = "../../components/datetime", features = ["provider_transform_internals"] }
Expand Down
6 changes: 3 additions & 3 deletions provider/cldr/src/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use crate::CldrPaths;
use icu_provider::iter::{IterableDataProviderCore, KeyedDataProvider};
use icu_provider::prelude::*;
use icu_provider::serde::SerdeSeDataStructMarker;
use icu_provider::serde::SerializeMarker;
use std::convert::TryFrom;
use std::sync::RwLock;

Expand Down Expand Up @@ -34,7 +34,7 @@ fn map_poison<E>(_err: E) -> DataError {
/// A lazy-initialized CLDR JSON data provider.
impl<'b, T> LazyCldrProvider<T>
where
T: DataProvider<SerdeSeDataStructMarker>
T: DataProvider<SerializeMarker>
+ IterableDataProviderCore
+ KeyedDataProvider
+ TryFrom<&'b dyn CldrPaths>,
Expand All @@ -45,7 +45,7 @@ where
&self,
req: &DataRequest,
cldr_paths: &'b dyn CldrPaths,
) -> Result<Option<DataResponse<SerdeSeDataStructMarker>>, DataError> {
) -> Result<Option<DataResponse<SerializeMarker>>, DataError> {
if T::supports_key(&req.resource_path.key).is_err() {
return Ok(None);
}
Expand Down
6 changes: 3 additions & 3 deletions provider/cldr/src/transform/datetime/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ impl DataProvider<calendar::DatePatternsV1Marker> for DatePatternsProvider {
) -> Result<DataResponse<calendar::DatePatternsV1Marker>, DataError> {
DatePatternsProvider::supports_key(&req.resource_path.key)?;
let dates = self.0.dates_for(req)?;
let metadata = DataResponseMetadata::default();
// TODO(#1109): Set metadata.data_langid correctly.
Ok(DataResponse {
metadata: DataResponseMetadata {
data_langid: req.resource_path.options.langid.clone(),
},
metadata,
payload: Some(DataPayload::from_owned(calendar::DatePatternsV1::from(
dates,
))),
Expand Down
Loading