From 3dd5d4341bbed32e53f4c043885bcb4e6de62a4a Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Tue, 28 Apr 2020 00:07:21 -0500 Subject: [PATCH 01/33] Add icu-util project --- Cargo.toml | 1 + components/util/Cargo.toml | 15 +++++++++++++++ components/util/README.md | 11 +++++++++++ components/util/src/lib.rs | 0 4 files changed, 27 insertions(+) create mode 100644 components/util/Cargo.toml create mode 100644 components/util/README.md create mode 100644 components/util/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 17ed9200055..2dea5e0e764 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,4 +4,5 @@ members = [ "components/icu", "components/icu4x", "components/locale", + "components/util", ] diff --git a/components/util/Cargo.toml b/components/util/Cargo.toml new file mode 100644 index 00000000000..55227907245 --- /dev/null +++ b/components/util/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "icu-util" +description = "Common trait definitions and utilities for ICU4X" +version = "0.0.1" +authors = ["The ICU4X Project Developers"] +edition = "2018" +readme = "README.md" +repository = "https://github.com/unicode-org/icu4x" +license = "MIT/Apache-2.0" +categories = ["internationalization"] +include = [ + "src/**/*", + "Cargo.toml", + "README.md" +] diff --git a/components/util/README.md b/components/util/README.md new file mode 100644 index 00000000000..fed98801c04 --- /dev/null +++ b/components/util/README.md @@ -0,0 +1,11 @@ +# ICU4X + +ICU4X is a set of internationalization components for Unicode. + +# Status [![crates.io](http://meritbadge.herokuapp.com/icu-util)](https://crates.io/crates/icu-util) + +The project is in an incubation period. + +# Authors + +The project is managed by a subcommittee of ICU-TC in the Unicode Consortium focused on providing solutions for client-side internationalization. diff --git a/components/util/src/lib.rs b/components/util/src/lib.rs new file mode 100644 index 00000000000..e69de29bb2d From 7cef64a20028acfb5dcdaffbb3bcc155a4a949f9 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Tue, 28 Apr 2020 01:37:16 -0500 Subject: [PATCH 02/33] Add icu-datap-json project --- Cargo.toml | 1 + components/datap_json/Cargo.toml | 18 ++++++++++++++++++ components/datap_json/README.md | 11 +++++++++++ components/datap_json/src/lib.rs | 3 +++ 4 files changed, 33 insertions(+) create mode 100644 components/datap_json/Cargo.toml create mode 100644 components/datap_json/README.md create mode 100644 components/datap_json/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 2dea5e0e764..678b18f032f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,7 @@ [workspace] members = [ + "components/datap_json", "components/icu", "components/icu4x", "components/locale", diff --git a/components/datap_json/Cargo.toml b/components/datap_json/Cargo.toml new file mode 100644 index 00000000000..20112fda40e --- /dev/null +++ b/components/datap_json/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "icu-datap-json" +description = "ICU4X data provider that reads from JSON files" +version = "0.0.1" +authors = ["The ICU4X Project Developers"] +edition = "2018" +readme = "README.md" +repository = "https://github.com/unicode-org/icu4x" +license = "MIT/Apache-2.0" +categories = ["internationalization"] +include = [ + "src/**/*", + "Cargo.toml", + "README.md" +] + +[dependencies] +icu-util = { path = "../util" } diff --git a/components/datap_json/README.md b/components/datap_json/README.md new file mode 100644 index 00000000000..fed98801c04 --- /dev/null +++ b/components/datap_json/README.md @@ -0,0 +1,11 @@ +# ICU4X + +ICU4X is a set of internationalization components for Unicode. + +# Status [![crates.io](http://meritbadge.herokuapp.com/icu-util)](https://crates.io/crates/icu-util) + +The project is in an incubation period. + +# Authors + +The project is managed by a subcommittee of ICU-TC in the Unicode Consortium focused on providing solutions for client-side internationalization. diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs new file mode 100644 index 00000000000..081aa4313b2 --- /dev/null +++ b/components/datap_json/src/lib.rs @@ -0,0 +1,3 @@ +#![no_std] + + From bb1e1b515aac3accfe412cd42f8e2a892c39adf6 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Tue, 28 Apr 2020 01:37:50 -0500 Subject: [PATCH 03/33] Add an initial data provider trait definition --- components/util/Cargo.toml | 3 ++ components/util/src/datap/decimal.rs | 18 +++++++++ components/util/src/datap/mod.rs | 56 ++++++++++++++++++++++++++++ components/util/src/lib.rs | 6 +++ components/util/src/std/mod.rs | 7 ++++ 5 files changed, 90 insertions(+) create mode 100644 components/util/src/datap/decimal.rs create mode 100644 components/util/src/datap/mod.rs create mode 100644 components/util/src/std/mod.rs diff --git a/components/util/Cargo.toml b/components/util/Cargo.toml index 55227907245..55da9bce043 100644 --- a/components/util/Cargo.toml +++ b/components/util/Cargo.toml @@ -13,3 +13,6 @@ include = [ "Cargo.toml", "README.md" ] + +[dependencies] +async-trait = "0.1.30" diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs new file mode 100644 index 00000000000..62b0c5fa141 --- /dev/null +++ b/components/util/src/datap/decimal.rs @@ -0,0 +1,18 @@ +// Decimal types + +use crate::Str; + +#[derive(PartialEq, Copy, Clone)] +pub enum Key { + SymbolsV1 = 1, +} + +#[derive(PartialEq, Clone)] +pub enum Payload { + // TODO: de-duplicate the name "SymbolsV1" between Key and Payload + SymbolsV1 { + zero_digit: char, + decimal_separator: Str, + grouping_separator: Str, + } +} diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs new file mode 100644 index 00000000000..095781c8b31 --- /dev/null +++ b/components/util/src/datap/mod.rs @@ -0,0 +1,56 @@ +// Data provider trait definitions + +pub mod decimal; + +use crate::std::Box; +use crate::std::String; +use crate::std::Cow; +use crate::Str; +use async_trait::async_trait; + +#[derive(PartialEq, Copy, Clone)] +pub enum Category { + Undefined = 0, + Decimal = 1, + PrivateUse = 4096, +} + +#[derive(PartialEq, Copy, Clone)] +pub enum Key { + Undefined, + Decimal(decimal::Key), + PrivateUse(u32) +} + +#[derive(PartialEq, Clone)] +pub struct Request { + // TODO: Make this a Locale instead of a String + pub locale: String, + pub category: Category, + pub key: Key, + // TODO: Make payload a dynamic type instead of a string + pub payload: Option, +} + +// TODO: Make ResponsePayload be a dynamic type specified by each caller +#[derive(PartialEq, Clone)] +pub enum ResponsePayload { + Undefined, + Decimal(decimal::Payload), + // TODO: Enable a dynamic type here + // PrivateUse(Any), +} + +#[derive(PartialEq, Clone)] +pub struct Response { + // TODO: Make this a Locale instead of a String + pub locale: String, + pub category: Category, + pub key: u32, + pub payload: Cow<'static, ResponsePayload>, +} + +#[async_trait] +pub trait DataProvider { + async fn load(&self, req: Request) -> Response; +} diff --git a/components/util/src/lib.rs b/components/util/src/lib.rs index e69de29bb2d..2f592cb318c 100644 --- a/components/util/src/lib.rs +++ b/components/util/src/lib.rs @@ -0,0 +1,6 @@ +#![no_std] + +pub mod datap; +pub mod std; + +pub type Str = std::Cow<'static, str>; diff --git a/components/util/src/std/mod.rs b/components/util/src/std/mod.rs new file mode 100644 index 00000000000..e2a59f8e61c --- /dev/null +++ b/components/util/src/std/mod.rs @@ -0,0 +1,7 @@ +extern crate alloc; + +pub use alloc::boxed::Box; +pub use alloc::string::String; +pub use alloc::borrow::Cow; + +pub use core::any::Any; From 52845b0613f8b0f6f76c02695312f32084374861 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Tue, 28 Apr 2020 03:38:47 -0500 Subject: [PATCH 04/33] Improving no_std support; adding serde; adding datap_json --- components/datap_json/Cargo.toml | 18 +++++++++++ components/datap_json/src/lib.rs | 31 +++++++++++++++++++ components/datap_json/src/schema.rs | 15 +++++++++ components/datap_json/tests/test_file_io.rs | 8 +++++ components/datap_json/tests/test_no_std.rs | 0 components/datap_json/tests/testdata/all.json | 9 ++++++ components/util/Cargo.toml | 17 ++++++++-- components/util/src/datap/decimal.rs | 10 +++--- components/util/src/datap/mod.rs | 8 ++--- components/util/src/lib.rs | 5 ++- 10 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 components/datap_json/src/schema.rs create mode 100644 components/datap_json/tests/test_file_io.rs create mode 100644 components/datap_json/tests/test_no_std.rs create mode 100644 components/datap_json/tests/testdata/all.json diff --git a/components/datap_json/Cargo.toml b/components/datap_json/Cargo.toml index 20112fda40e..c01fc8a50b1 100644 --- a/components/datap_json/Cargo.toml +++ b/components/datap_json/Cargo.toml @@ -14,5 +14,23 @@ include = [ "README.md" ] +[features] +default = ["std"] +std = ["no-std-compat/std", "serde/std", "serde_json/std"] + [dependencies] icu-util = { path = "../util" } + +[dependencies.serde] +version = "1.0" +default-features = false +features = ["derive", "alloc"] + +[dependencies.serde_json] +version = "1.0" +default-features = false +features = ["alloc"] + +[dependencies.no-std-compat] +version = "0.4.0" +features = ["alloc"] diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 081aa4313b2..850be3eaa56 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -1,3 +1,34 @@ #![no_std] +extern crate no_std_compat as std; +use std::prelude::v1::*; + + +mod schema; + +#[cfg(feature = "std")] +use std::io::Read; + +pub enum Error { + JsonError(serde_json::error::Error) +} + +impl From for Error { + fn from(err: serde_json::error::Error) -> Error { + Error::JsonError(err) + } +} + +pub type Result = std::result::Result; + +pub struct JsonDataProvider { + data: schema::JsonSchema, +} + +impl JsonDataProvider { + pub fn from_reader(reader: R) -> Result { + let result: schema::JsonSchema = serde_json::from_reader(reader)?; + Ok(JsonDataProvider{ data: result }) + } +} diff --git a/components/datap_json/src/schema.rs b/components/datap_json/src/schema.rs new file mode 100644 index 00000000000..b107c53b35d --- /dev/null +++ b/components/datap_json/src/schema.rs @@ -0,0 +1,15 @@ +use icu_util::datap; +use serde::{Deserialize, Serialize}; + +#[allow(unused_imports)] +use std::prelude::v1::*; + +#[derive(Serialize, Deserialize)] +struct DecimalJsonSchema { + symbols_v1: datap::decimal::Payload, +} + +#[derive(Serialize, Deserialize)] +pub(crate) struct JsonSchema { + decimal: DecimalJsonSchema, +} diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs new file mode 100644 index 00000000000..ef299f68d7a --- /dev/null +++ b/components/datap_json/tests/test_file_io.rs @@ -0,0 +1,8 @@ +#![feature(std)] + +extern crate icu_datap_json; + +#[test] +fn test_add() { + assert_eq!(adder::add(3, 2), 5); +} diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/components/datap_json/tests/testdata/all.json b/components/datap_json/tests/testdata/all.json new file mode 100644 index 00000000000..05ba64d8eaa --- /dev/null +++ b/components/datap_json/tests/testdata/all.json @@ -0,0 +1,9 @@ +{ + "decimal": { + "SymbolsV1": { + "zero_digit": "0", + "decimal_separator": ".", + "grouping_separator": "," + } + } +} diff --git a/components/util/Cargo.toml b/components/util/Cargo.toml index 55da9bce043..0f0c687da0d 100644 --- a/components/util/Cargo.toml +++ b/components/util/Cargo.toml @@ -14,5 +14,18 @@ include = [ "README.md" ] -[dependencies] -async-trait = "0.1.30" +[features] +default = ["std"] +std = ["serde/std", "no-std-compat/std"] + +[dependencies.async-trait] +version = "0.1.30" + +[dependencies.serde] +version = "1.0" +default-features = false +features = ["derive", "alloc"] + +[dependencies.no-std-compat] +version = "0.4.0" +features = ["alloc"] diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index 62b0c5fa141..3d760d69a1b 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -1,18 +1,20 @@ // Decimal types -use crate::Str; +use std::prelude::v1::*; + +use serde::{Deserialize, Serialize}; #[derive(PartialEq, Copy, Clone)] pub enum Key { SymbolsV1 = 1, } -#[derive(PartialEq, Clone)] +#[derive(PartialEq, Clone, Deserialize, Serialize)] pub enum Payload { // TODO: de-duplicate the name "SymbolsV1" between Key and Payload SymbolsV1 { zero_digit: char, - decimal_separator: Str, - grouping_separator: Str, + decimal_separator: String, + grouping_separator: String, } } diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 095781c8b31..092ec44300d 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -2,11 +2,11 @@ pub mod decimal; -use crate::std::Box; -use crate::std::String; -use crate::std::Cow; -use crate::Str; +use std::prelude::v1::*; use async_trait::async_trait; +use std::borrow::Cow; + +pub type Str = Cow<'static, str>; #[derive(PartialEq, Copy, Clone)] pub enum Category { diff --git a/components/util/src/lib.rs b/components/util/src/lib.rs index 2f592cb318c..f0e69ee2aab 100644 --- a/components/util/src/lib.rs +++ b/components/util/src/lib.rs @@ -1,6 +1,5 @@ #![no_std] -pub mod datap; -pub mod std; +extern crate no_std_compat as std; -pub type Str = std::Cow<'static, str>; +pub mod datap; From 996b397b24348a72c3fce187f9bb88b3be3d14d0 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Tue, 28 Apr 2020 04:21:47 -0500 Subject: [PATCH 05/33] Unit test for JSON-based data provider --- components/datap_json/Cargo.toml | 3 ++ components/datap_json/src/lib.rs | 18 ++++++++-- components/datap_json/src/schema.rs | 6 ++-- components/datap_json/tests/test_file_io.rs | 33 +++++++++++++++++-- components/datap_json/tests/testdata/all.json | 10 +++--- components/util/src/datap/decimal.rs | 2 +- components/util/src/datap/mod.rs | 10 +++--- 7 files changed, 63 insertions(+), 19 deletions(-) diff --git a/components/datap_json/Cargo.toml b/components/datap_json/Cargo.toml index c01fc8a50b1..0080935105d 100644 --- a/components/datap_json/Cargo.toml +++ b/components/datap_json/Cargo.toml @@ -21,6 +21,9 @@ std = ["no-std-compat/std", "serde/std", "serde_json/std"] [dependencies] icu-util = { path = "../util" } +[dependencies.async-trait] +version = "0.1.30" + [dependencies.serde] version = "1.0" default-features = false diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 850be3eaa56..74a50854fff 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -3,13 +3,17 @@ extern crate no_std_compat as std; use std::prelude::v1::*; +// use async_trait::async_trait; +use std::borrow::Cow; +use icu_util::datap; mod schema; #[cfg(feature = "std")] use std::io::Read; +#[derive(Debug)] pub enum Error { JsonError(serde_json::error::Error) } @@ -20,15 +24,23 @@ impl From for Error { } } -pub type Result = std::result::Result; - pub struct JsonDataProvider { data: schema::JsonSchema, } impl JsonDataProvider { - pub fn from_reader(reader: R) -> Result { + pub fn from_reader(reader: R) -> Result { let result: schema::JsonSchema = serde_json::from_reader(reader)?; Ok(JsonDataProvider{ data: result }) } } + +impl datap::DataProvider for JsonDataProvider { + fn load(&self, _request: datap::Request) -> Result> { + // TODO: Use the request variable + Ok(datap::Response { + locale: "root".to_string(), + payload: Cow::Owned(datap::ResponsePayload::Decimal(self.data.decimal.symbols_v1.clone())) + }) + } +} diff --git a/components/datap_json/src/schema.rs b/components/datap_json/src/schema.rs index b107c53b35d..6399da649da 100644 --- a/components/datap_json/src/schema.rs +++ b/components/datap_json/src/schema.rs @@ -5,11 +5,11 @@ use serde::{Deserialize, Serialize}; use std::prelude::v1::*; #[derive(Serialize, Deserialize)] -struct DecimalJsonSchema { - symbols_v1: datap::decimal::Payload, +pub(crate) struct DecimalJsonSchema { + pub(crate) symbols_v1: datap::decimal::Payload, } #[derive(Serialize, Deserialize)] pub(crate) struct JsonSchema { - decimal: DecimalJsonSchema, + pub(crate) decimal: DecimalJsonSchema, } diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index ef299f68d7a..83d8a1dd602 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -1,8 +1,35 @@ -#![feature(std)] +// error[E0554]: `#![feature]` may not be used on the stable release channel +// #![feature(std)] extern crate icu_datap_json; +use std::fs::File; +use std::io::BufReader; +use std::borrow::Borrow; + +use icu_datap_json::JsonDataProvider; +use icu_util::datap; +use icu_util::datap::DataProvider; + #[test] -fn test_add() { - assert_eq!(adder::add(3, 2), 5); +fn test_read_json() { + // TODO: Make this path relative to this file instead of package root + let file = File::open("./tests/testdata/all.json").unwrap(); + let reader = BufReader::new(file); + let json_data_provider = JsonDataProvider::from_reader(reader).unwrap(); + let data = json_data_provider.load(datap::Request { + locale: "root".to_string(), + category: datap::Category::Decimal, + key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), + payload: None + }).unwrap(); + let decimal_data = match data.payload.borrow() { + datap::ResponsePayload::Decimal(decimal_data) => decimal_data, + _ => unreachable!() + }; + assert_eq!(decimal_data, &datap::decimal::Payload::SymbolsV1 { + zero_digit: '0', + decimal_separator: ".".to_string(), + grouping_separator: ",".to_string(), + }); } diff --git a/components/datap_json/tests/testdata/all.json b/components/datap_json/tests/testdata/all.json index 05ba64d8eaa..b9aad92ae5e 100644 --- a/components/datap_json/tests/testdata/all.json +++ b/components/datap_json/tests/testdata/all.json @@ -1,9 +1,11 @@ { "decimal": { - "SymbolsV1": { - "zero_digit": "0", - "decimal_separator": ".", - "grouping_separator": "," + "symbols_v1": { + "SymbolsV1": { + "zero_digit": "0", + "decimal_separator": ".", + "grouping_separator": "," + } } } } diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index 3d760d69a1b..59f34d10b1c 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -9,7 +9,7 @@ pub enum Key { SymbolsV1 = 1, } -#[derive(PartialEq, Clone, Deserialize, Serialize)] +#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] pub enum Payload { // TODO: de-duplicate the name "SymbolsV1" between Key and Payload SymbolsV1 { diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 092ec44300d..c363c9d0f5a 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -3,8 +3,9 @@ pub mod decimal; use std::prelude::v1::*; -use async_trait::async_trait; +// use async_trait::async_trait; use std::borrow::Cow; +use std::error::Error; pub type Str = Cow<'static, str>; @@ -45,12 +46,11 @@ pub enum ResponsePayload { pub struct Response { // TODO: Make this a Locale instead of a String pub locale: String, - pub category: Category, - pub key: u32, pub payload: Cow<'static, ResponsePayload>, } -#[async_trait] +// TODO: Make this async +// #[async_trait] pub trait DataProvider { - async fn load(&self, req: Request) -> Response; + fn load(&self, req: Request) -> Result>; } From 0147b2439651b2cba8dba7ad0d353b3b756f180c Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 29 Apr 2020 21:08:39 -0500 Subject: [PATCH 06/33] Use dynamic type for response payload --- components/datap_json/src/lib.rs | 4 ++- components/datap_json/src/schema.rs | 1 + components/datap_json/tests/test_file_io.rs | 13 ++++----- components/datap_json/tests/testdata/all.json | 5 ++++ components/util/src/datap/decimal.rs | 27 +++++++++++++++++++ components/util/src/datap/mod.rs | 27 +++++++++++++++++-- 6 files changed, 68 insertions(+), 9 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 74a50854fff..25a5a16420c 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -7,6 +7,7 @@ use std::prelude::v1::*; use std::borrow::Cow; use icu_util::datap; +use icu_util::datap::Bovine; mod schema; @@ -40,7 +41,8 @@ impl datap::DataProvider for JsonDataProvider { // TODO: Use the request variable Ok(datap::Response { locale: "root".to_string(), - payload: Cow::Owned(datap::ResponsePayload::Decimal(self.data.decimal.symbols_v1.clone())) + // payload: Cow::Owned(datap::ResponsePayload::Decimal(self.data.decimal.symbols_v1.clone())) + payload2: Cow::Owned(self.data.decimal.symbols_v1_a.clone_into_box()) }) } } diff --git a/components/datap_json/src/schema.rs b/components/datap_json/src/schema.rs index 6399da649da..8324ee9ae6c 100644 --- a/components/datap_json/src/schema.rs +++ b/components/datap_json/src/schema.rs @@ -7,6 +7,7 @@ use std::prelude::v1::*; #[derive(Serialize, Deserialize)] pub(crate) struct DecimalJsonSchema { pub(crate) symbols_v1: datap::decimal::Payload, + pub(crate) symbols_v1_a: datap::decimal::SymbolsV1, } #[derive(Serialize, Deserialize)] diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index 83d8a1dd602..bf6f405101c 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -5,7 +5,7 @@ extern crate icu_datap_json; use std::fs::File; use std::io::BufReader; -use std::borrow::Borrow; +// use std::borrow::Borrow; use icu_datap_json::JsonDataProvider; use icu_util::datap; @@ -23,11 +23,12 @@ fn test_read_json() { key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), payload: None }).unwrap(); - let decimal_data = match data.payload.borrow() { - datap::ResponsePayload::Decimal(decimal_data) => decimal_data, - _ => unreachable!() - }; - assert_eq!(decimal_data, &datap::decimal::Payload::SymbolsV1 { + // let decimal_data = match data.payload.borrow() { + // datap::ResponsePayload::Decimal(decimal_data) => decimal_data, + // _ => unreachable!() + // }; + let decimal_data: &datap::decimal::SymbolsV1 = data.unwrap_payload(); + assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', decimal_separator: ".".to_string(), grouping_separator: ",".to_string(), diff --git a/components/datap_json/tests/testdata/all.json b/components/datap_json/tests/testdata/all.json index b9aad92ae5e..f566de168ff 100644 --- a/components/datap_json/tests/testdata/all.json +++ b/components/datap_json/tests/testdata/all.json @@ -6,6 +6,11 @@ "decimal_separator": ".", "grouping_separator": "," } + }, + "symbols_v1_a": { + "zero_digit": "0", + "decimal_separator": ".", + "grouping_separator": "," } } } diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index 59f34d10b1c..647f14e7e24 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -1,9 +1,12 @@ // Decimal types use std::prelude::v1::*; +use std::any::Any; use serde::{Deserialize, Serialize}; +use super::Bovine; + #[derive(PartialEq, Copy, Clone)] pub enum Key { SymbolsV1 = 1, @@ -18,3 +21,27 @@ pub enum Payload { grouping_separator: String, } } + +#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] +pub struct SymbolsV1 { + pub zero_digit: char, + pub decimal_separator: String, + pub grouping_separator: String, +} + +impl Bovine for SymbolsV1 { + // TODO: How to make this line return Box? Is that necessary? + fn clone_into_box(&self) -> Box { + Box::new(self.clone()) + } + + fn as_any(&self) -> &dyn Any { + self + } +} + +impl SymbolsV1 { + pub fn as_bovine(&self) -> &dyn Bovine { + self + } +} diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index c363c9d0f5a..32cc22c149b 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -6,6 +6,7 @@ use std::prelude::v1::*; // use async_trait::async_trait; use std::borrow::Cow; use std::error::Error; +use std::any::Any; pub type Str = Cow<'static, str>; @@ -42,11 +43,33 @@ pub enum ResponsePayload { // PrivateUse(Any), } -#[derive(PartialEq, Clone)] +pub trait Bovine: Any { + fn clone_into_box(&self) -> Box; + fn as_any(&self) -> &dyn Any; +} + +impl ToOwned for dyn Bovine { + type Owned = Box; + + fn to_owned(&self) -> Self::Owned { + Bovine::clone_into_box(self) + } +} + +// TODO: Enable PartialEq on Response (is that necessary?) +#[derive(Clone)] pub struct Response { // TODO: Make this a Locale instead of a String pub locale: String, - pub payload: Cow<'static, ResponsePayload>, + // pub payload: Cow<'static, ResponsePayload>, + pub payload2: Cow<'static, dyn Bovine>, + // pub payload3: Box>, +} + +impl Response { + pub fn unwrap_payload(&self) -> &T { + Any::downcast_ref::(self.payload2.as_any()).unwrap() + } } // TODO: Make this async From b495fcbbc0c21166dbbf5b6dad3fbde9b6cb5790 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 29 Apr 2020 21:09:08 -0500 Subject: [PATCH 07/33] Remove icu_util::std --- components/util/src/std/mod.rs | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 components/util/src/std/mod.rs diff --git a/components/util/src/std/mod.rs b/components/util/src/std/mod.rs deleted file mode 100644 index e2a59f8e61c..00000000000 --- a/components/util/src/std/mod.rs +++ /dev/null @@ -1,7 +0,0 @@ -extern crate alloc; - -pub use alloc::boxed::Box; -pub use alloc::string::String; -pub use alloc::borrow::Cow; - -pub use core::any::Any; From 7cff876b0edcac354c866dbee86ad713fb598ef4 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 29 Apr 2020 21:14:53 -0500 Subject: [PATCH 08/33] Remove obsolete code --- components/datap_json/src/lib.rs | 1 - components/datap_json/src/schema.rs | 1 - components/datap_json/tests/test_file_io.rs | 5 ----- components/util/src/datap/decimal.rs | 12 +----------- components/util/src/datap/mod.rs | 11 ----------- 5 files changed, 1 insertion(+), 29 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 25a5a16420c..65318c65ce2 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -41,7 +41,6 @@ impl datap::DataProvider for JsonDataProvider { // TODO: Use the request variable Ok(datap::Response { locale: "root".to_string(), - // payload: Cow::Owned(datap::ResponsePayload::Decimal(self.data.decimal.symbols_v1.clone())) payload2: Cow::Owned(self.data.decimal.symbols_v1_a.clone_into_box()) }) } diff --git a/components/datap_json/src/schema.rs b/components/datap_json/src/schema.rs index 8324ee9ae6c..3a1d6851706 100644 --- a/components/datap_json/src/schema.rs +++ b/components/datap_json/src/schema.rs @@ -6,7 +6,6 @@ use std::prelude::v1::*; #[derive(Serialize, Deserialize)] pub(crate) struct DecimalJsonSchema { - pub(crate) symbols_v1: datap::decimal::Payload, pub(crate) symbols_v1_a: datap::decimal::SymbolsV1, } diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index bf6f405101c..401da47a80e 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -5,7 +5,6 @@ extern crate icu_datap_json; use std::fs::File; use std::io::BufReader; -// use std::borrow::Borrow; use icu_datap_json::JsonDataProvider; use icu_util::datap; @@ -23,10 +22,6 @@ fn test_read_json() { key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), payload: None }).unwrap(); - // let decimal_data = match data.payload.borrow() { - // datap::ResponsePayload::Decimal(decimal_data) => decimal_data, - // _ => unreachable!() - // }; let decimal_data: &datap::decimal::SymbolsV1 = data.unwrap_payload(); assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index 647f14e7e24..1ff8db4fc8b 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -12,16 +12,7 @@ pub enum Key { SymbolsV1 = 1, } -#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] -pub enum Payload { - // TODO: de-duplicate the name "SymbolsV1" between Key and Payload - SymbolsV1 { - zero_digit: char, - decimal_separator: String, - grouping_separator: String, - } -} - +// TODO: de-duplicate the name "SymbolsV1" between Key and the struct #[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] pub struct SymbolsV1 { pub zero_digit: char, @@ -30,7 +21,6 @@ pub struct SymbolsV1 { } impl Bovine for SymbolsV1 { - // TODO: How to make this line return Box? Is that necessary? fn clone_into_box(&self) -> Box { Box::new(self.clone()) } diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 32cc22c149b..676401a2b83 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -34,15 +34,6 @@ pub struct Request { pub payload: Option, } -// TODO: Make ResponsePayload be a dynamic type specified by each caller -#[derive(PartialEq, Clone)] -pub enum ResponsePayload { - Undefined, - Decimal(decimal::Payload), - // TODO: Enable a dynamic type here - // PrivateUse(Any), -} - pub trait Bovine: Any { fn clone_into_box(&self) -> Box; fn as_any(&self) -> &dyn Any; @@ -61,9 +52,7 @@ impl ToOwned for dyn Bovine { pub struct Response { // TODO: Make this a Locale instead of a String pub locale: String, - // pub payload: Cow<'static, ResponsePayload>, pub payload2: Cow<'static, dyn Bovine>, - // pub payload3: Box>, } impl Response { From 9d0298d12ff0f1d792ac31937dc01bb6607228c6 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 29 Apr 2020 21:16:24 -0500 Subject: [PATCH 09/33] Rename payload2 to payload --- components/datap_json/src/lib.rs | 2 +- components/util/src/datap/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 65318c65ce2..a6e27e5c7d1 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -41,7 +41,7 @@ impl datap::DataProvider for JsonDataProvider { // TODO: Use the request variable Ok(datap::Response { locale: "root".to_string(), - payload2: Cow::Owned(self.data.decimal.symbols_v1_a.clone_into_box()) + payload: Cow::Owned(self.data.decimal.symbols_v1_a.clone_into_box()) }) } } diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 676401a2b83..ba79b73d262 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -52,12 +52,12 @@ impl ToOwned for dyn Bovine { pub struct Response { // TODO: Make this a Locale instead of a String pub locale: String, - pub payload2: Cow<'static, dyn Bovine>, + pub payload: Cow<'static, dyn Bovine>, } impl Response { pub fn unwrap_payload(&self) -> &T { - Any::downcast_ref::(self.payload2.as_any()).unwrap() + Any::downcast_ref::(self.payload.as_any()).unwrap() } } From 5950fdb4b228b2d345313d6ec9727c86ec0bef66 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 1 May 2020 01:28:10 -0500 Subject: [PATCH 10/33] Auto-implement Bovine for all data traits --- components/datap_json/tests/test_file_io.rs | 4 +-- components/util/src/datap/decimal.rs | 19 -------------- components/util/src/datap/mod.rs | 28 +++++++++++++++++++-- components/util/src/lib.rs | 1 + 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index 401da47a80e..bfad6ff7e31 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -16,13 +16,13 @@ fn test_read_json() { let file = File::open("./tests/testdata/all.json").unwrap(); let reader = BufReader::new(file); let json_data_provider = JsonDataProvider::from_reader(reader).unwrap(); - let data = json_data_provider.load(datap::Request { + let response = json_data_provider.load(datap::Request { locale: "root".to_string(), category: datap::Category::Decimal, key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), payload: None }).unwrap(); - let decimal_data: &datap::decimal::SymbolsV1 = data.unwrap_payload(); + let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', decimal_separator: ".".to_string(), diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index 1ff8db4fc8b..6babdc143fb 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -1,12 +1,9 @@ // Decimal types use std::prelude::v1::*; -use std::any::Any; use serde::{Deserialize, Serialize}; -use super::Bovine; - #[derive(PartialEq, Copy, Clone)] pub enum Key { SymbolsV1 = 1, @@ -19,19 +16,3 @@ pub struct SymbolsV1 { pub decimal_separator: String, pub grouping_separator: String, } - -impl Bovine for SymbolsV1 { - fn clone_into_box(&self) -> Box { - Box::new(self.clone()) - } - - fn as_any(&self) -> &dyn Any { - self - } -} - -impl SymbolsV1 { - pub fn as_bovine(&self) -> &dyn Bovine { - self - } -} diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index ba79b73d262..f664858998d 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -4,6 +4,8 @@ pub mod decimal; use std::prelude::v1::*; // use async_trait::async_trait; +use std::borrow::Borrow; +use std::borrow::BorrowMut; use std::borrow::Cow; use std::error::Error; use std::any::Any; @@ -37,6 +39,7 @@ pub struct Request { pub trait Bovine: Any { fn clone_into_box(&self) -> Box; fn as_any(&self) -> &dyn Any; + fn as_any_mut(&mut self) -> &mut dyn Any; } impl ToOwned for dyn Bovine { @@ -47,6 +50,19 @@ impl ToOwned for dyn Bovine { } } +// Implement Bovine for all 'static types implementing Clone. +impl Bovine for S { + fn clone_into_box(&self) -> Box { + Box::new(self.clone()) + } + fn as_any(&self) -> &dyn Any { + self + } + fn as_any_mut(&mut self) -> &mut dyn Any { + self + } +} + // TODO: Enable PartialEq on Response (is that necessary?) #[derive(Clone)] pub struct Response { @@ -55,9 +71,17 @@ pub struct Response { pub payload: Cow<'static, dyn Bovine>, } +// TODO: Should this be an implemention of std::borrow::Borrow? impl Response { - pub fn unwrap_payload(&self) -> &T { - Any::downcast_ref::(self.payload.as_any()).unwrap() + pub fn borrow_payload(&self) -> Option<&T> { + let borrowed: &dyn Bovine = self.payload.borrow(); + borrowed.as_any().downcast_ref::() + } + + pub fn borrow_payload_mut(&mut self) -> Option<&mut T> { + let boxed: &mut Box = self.payload.to_mut(); + let borrowed: &mut dyn Bovine = boxed.borrow_mut(); + borrowed.as_any_mut().downcast_mut::() } } diff --git a/components/util/src/lib.rs b/components/util/src/lib.rs index f0e69ee2aab..48609d6bb68 100644 --- a/components/util/src/lib.rs +++ b/components/util/src/lib.rs @@ -1,4 +1,5 @@ #![no_std] +#![feature(type_name_of_val)] extern crate no_std_compat as std; From 457d8ad110a303a9418916fbce0a0dfb4ddc51bc Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 1 May 2020 01:30:35 -0500 Subject: [PATCH 11/33] Rename Bovine to ClonableAny --- components/datap_json/src/lib.rs | 2 +- components/util/src/datap/mod.rs | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index a6e27e5c7d1..0e1b9618560 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -7,7 +7,7 @@ use std::prelude::v1::*; use std::borrow::Cow; use icu_util::datap; -use icu_util::datap::Bovine; +use icu_util::datap::ClonableAny; mod schema; diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index f664858998d..9de045194ac 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -36,23 +36,23 @@ pub struct Request { pub payload: Option, } -pub trait Bovine: Any { - fn clone_into_box(&self) -> Box; +pub trait ClonableAny: Any { + fn clone_into_box(&self) -> Box; fn as_any(&self) -> &dyn Any; fn as_any_mut(&mut self) -> &mut dyn Any; } -impl ToOwned for dyn Bovine { - type Owned = Box; +impl ToOwned for dyn ClonableAny { + type Owned = Box; fn to_owned(&self) -> Self::Owned { - Bovine::clone_into_box(self) + ClonableAny::clone_into_box(self) } } -// Implement Bovine for all 'static types implementing Clone. -impl Bovine for S { - fn clone_into_box(&self) -> Box { +// Implement ClonableAny for all 'static types implementing Clone. +impl ClonableAny for S { + fn clone_into_box(&self) -> Box { Box::new(self.clone()) } fn as_any(&self) -> &dyn Any { @@ -68,19 +68,19 @@ impl Bovine for S { pub struct Response { // TODO: Make this a Locale instead of a String pub locale: String, - pub payload: Cow<'static, dyn Bovine>, + pub payload: Cow<'static, dyn ClonableAny>, } // TODO: Should this be an implemention of std::borrow::Borrow? impl Response { pub fn borrow_payload(&self) -> Option<&T> { - let borrowed: &dyn Bovine = self.payload.borrow(); + let borrowed: &dyn ClonableAny = self.payload.borrow(); borrowed.as_any().downcast_ref::() } pub fn borrow_payload_mut(&mut self) -> Option<&mut T> { - let boxed: &mut Box = self.payload.to_mut(); - let borrowed: &mut dyn Bovine = boxed.borrow_mut(); + let boxed: &mut Box = self.payload.to_mut(); + let borrowed: &mut dyn ClonableAny = boxed.borrow_mut(); borrowed.as_any_mut().downcast_mut::() } } From 1791debd14aec527cd078bd288ba31020f937c56 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 1 May 2020 02:24:40 -0500 Subject: [PATCH 12/33] Additional polishing of Response struct --- components/datap_json/src/lib.rs | 11 ++-- components/datap_json/tests/test_file_io.rs | 1 + components/util/src/datap/decimal.rs | 2 +- components/util/src/datap/mod.rs | 65 ++++++++++++++++++--- 4 files changed, 64 insertions(+), 15 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 0e1b9618560..0f1142d0211 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -4,10 +4,8 @@ extern crate no_std_compat as std; use std::prelude::v1::*; // use async_trait::async_trait; -use std::borrow::Cow; use icu_util::datap; -use icu_util::datap::ClonableAny; mod schema; @@ -37,11 +35,10 @@ impl JsonDataProvider { } impl datap::DataProvider for JsonDataProvider { - fn load(&self, _request: datap::Request) -> Result> { + fn load(&self, _request: datap::Request) -> Result { // TODO: Use the request variable - Ok(datap::Response { - locale: "root".to_string(), - payload: Cow::Owned(self.data.decimal.symbols_v1_a.clone_into_box()) - }) + let mut response = datap::Response::with_owned_payload(self.data.decimal.symbols_v1_a.clone()); + response.locale = "en".to_string(); + Ok(response) } } diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index bfad6ff7e31..4c68a516659 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -22,6 +22,7 @@ fn test_read_json() { key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), payload: None }).unwrap(); + println!("{:?}", response); let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index 6babdc143fb..55737c1e781 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -4,7 +4,7 @@ use std::prelude::v1::*; use serde::{Deserialize, Serialize}; -#[derive(PartialEq, Copy, Clone)] +#[derive(PartialEq, Copy, Clone, Debug)] pub enum Key { SymbolsV1 = 1, } diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 9de045194ac..919f3c8c02e 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -9,23 +9,37 @@ use std::borrow::BorrowMut; use std::borrow::Cow; use std::error::Error; use std::any::Any; +use std::fmt::{Debug, Display, self}; +use core::ops::Deref; pub type Str = Cow<'static, str>; -#[derive(PartialEq, Copy, Clone)] +#[derive(PartialEq, Copy, Clone, Debug)] pub enum Category { Undefined = 0, Decimal = 1, PrivateUse = 4096, } -#[derive(PartialEq, Copy, Clone)] +impl Display for Category { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Debug::fmt(self, f) + } +} + +#[derive(PartialEq, Copy, Clone, Debug)] pub enum Key { Undefined, Decimal(decimal::Key), PrivateUse(u32) } +impl Display for Key { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + Debug::fmt(self, f) + } +} + #[derive(PartialEq, Clone)] pub struct Request { // TODO: Make this a Locale instead of a String @@ -36,7 +50,7 @@ pub struct Request { pub payload: Option, } -pub trait ClonableAny: Any { +trait ClonableAny: Debug + Any { fn clone_into_box(&self) -> Box; fn as_any(&self) -> &dyn Any; fn as_any_mut(&mut self) -> &mut dyn Any; @@ -51,7 +65,7 @@ impl ToOwned for dyn ClonableAny { } // Implement ClonableAny for all 'static types implementing Clone. -impl ClonableAny for S { +impl ClonableAny for S { fn clone_into_box(&self) -> Box { Box::new(self.clone()) } @@ -64,11 +78,11 @@ impl ClonableAny for S { } // TODO: Enable PartialEq on Response (is that necessary?) -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct Response { // TODO: Make this a Locale instead of a String pub locale: String, - pub payload: Cow<'static, dyn ClonableAny>, + payload: Cow<'static, dyn ClonableAny>, } // TODO: Should this be an implemention of std::borrow::Borrow? @@ -83,10 +97,47 @@ impl Response { let borrowed: &mut dyn ClonableAny = boxed.borrow_mut(); borrowed.as_any_mut().downcast_mut::() } + + pub fn with_owned_payload(t: T) -> Self { + Response { + locale: "und".to_string(), + payload: Cow::Owned(Box::new(t) as Box), + } + } + + pub fn with_borrowed_payload(t: &'static T) -> Self { + Response { + locale: "und".to_string(), + payload: Cow::Borrowed(t), + } + } +} + +#[derive(Debug)] +pub enum ResponseError { + UnsupportedCategoryError(Category), + UnsupportedKeyError(Category, Key), + ResourceError(Box), +} + +impl Display for ResponseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO: should the Error Display be different from Debug? + Debug::fmt(self, f) + } +} + +impl Error for ResponseError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + ResponseError::ResourceError(error) => Some(error.deref()), + _ => None + } + } } // TODO: Make this async // #[async_trait] pub trait DataProvider { - fn load(&self, req: Request) -> Result>; + fn load(&self, req: Request) -> Result; } From dbb767d7a3415298c3b6eb22abbae51a022f7d88 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 1 May 2020 02:26:18 -0500 Subject: [PATCH 13/33] Fix spelling of Cloneable --- components/util/src/datap/mod.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 919f3c8c02e..b96964121a4 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -50,23 +50,23 @@ pub struct Request { pub payload: Option, } -trait ClonableAny: Debug + Any { - fn clone_into_box(&self) -> Box; +trait CloneableAny: Debug + Any { + fn clone_into_box(&self) -> Box; fn as_any(&self) -> &dyn Any; fn as_any_mut(&mut self) -> &mut dyn Any; } -impl ToOwned for dyn ClonableAny { - type Owned = Box; +impl ToOwned for dyn CloneableAny { + type Owned = Box; fn to_owned(&self) -> Self::Owned { - ClonableAny::clone_into_box(self) + CloneableAny::clone_into_box(self) } } -// Implement ClonableAny for all 'static types implementing Clone. -impl ClonableAny for S { - fn clone_into_box(&self) -> Box { +// Implement CloneableAny for all 'static types implementing Clone. +impl CloneableAny for S { + fn clone_into_box(&self) -> Box { Box::new(self.clone()) } fn as_any(&self) -> &dyn Any { @@ -82,26 +82,26 @@ impl ClonableAny for S { pub struct Response { // TODO: Make this a Locale instead of a String pub locale: String, - payload: Cow<'static, dyn ClonableAny>, + payload: Cow<'static, dyn CloneableAny>, } // TODO: Should this be an implemention of std::borrow::Borrow? impl Response { pub fn borrow_payload(&self) -> Option<&T> { - let borrowed: &dyn ClonableAny = self.payload.borrow(); + let borrowed: &dyn CloneableAny = self.payload.borrow(); borrowed.as_any().downcast_ref::() } pub fn borrow_payload_mut(&mut self) -> Option<&mut T> { - let boxed: &mut Box = self.payload.to_mut(); - let borrowed: &mut dyn ClonableAny = boxed.borrow_mut(); + let boxed: &mut Box = self.payload.to_mut(); + let borrowed: &mut dyn CloneableAny = boxed.borrow_mut(); borrowed.as_any_mut().downcast_mut::() } pub fn with_owned_payload(t: T) -> Self { Response { locale: "und".to_string(), - payload: Cow::Owned(Box::new(t) as Box), + payload: Cow::Owned(Box::new(t) as Box), } } From 070cc8bba52f97a35408e5a1f301008396e31ae7 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 6 May 2020 00:04:24 -0500 Subject: [PATCH 14/33] Add no_std tests for JSON Data Provider --- components/datap_json/Cargo.toml | 5 +++ components/datap_json/src/lib.rs | 11 +++++++ components/datap_json/tests/test_file_io.rs | 7 +--- components/datap_json/tests/test_no_std.rs | 32 +++++++++++++++++++ components/datap_json/tests/testdata/all.json | 7 ---- components/util/src/datap/mod.rs | 1 + 6 files changed, 50 insertions(+), 13 deletions(-) diff --git a/components/datap_json/Cargo.toml b/components/datap_json/Cargo.toml index 0080935105d..01c5a568958 100644 --- a/components/datap_json/Cargo.toml +++ b/components/datap_json/Cargo.toml @@ -37,3 +37,8 @@ features = ["alloc"] [dependencies.no-std-compat] version = "0.4.0" features = ["alloc"] + +[[test]] +name = "file_io" +path = "tests/test_file_io.rs" +required-features = ["std"] diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 0f1142d0211..66d4038b614 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -28,10 +28,21 @@ pub struct JsonDataProvider { } impl JsonDataProvider { + #[cfg(feature = "std")] pub fn from_reader(reader: R) -> Result { let result: schema::JsonSchema = serde_json::from_reader(reader)?; Ok(JsonDataProvider{ data: result }) } + + pub fn from_str(data: &str) -> Result { + let result: schema::JsonSchema = serde_json::from_str(data)?; + Ok(JsonDataProvider{ data: result }) + } + + pub fn from_slice(data: &[u8]) -> Result { + let result: schema::JsonSchema = serde_json::from_slice(data)?; + Ok(JsonDataProvider{ data: result }) + } } impl datap::DataProvider for JsonDataProvider { diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index 4c68a516659..0ff6fa7ccf8 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -1,6 +1,3 @@ -// error[E0554]: `#![feature]` may not be used on the stable release channel -// #![feature(std)] - extern crate icu_datap_json; use std::fs::File; @@ -12,8 +9,7 @@ use icu_util::datap::DataProvider; #[test] fn test_read_json() { - // TODO: Make this path relative to this file instead of package root - let file = File::open("./tests/testdata/all.json").unwrap(); + let file = File::open("tests/testdata/all.json").unwrap(); let reader = BufReader::new(file); let json_data_provider = JsonDataProvider::from_reader(reader).unwrap(); let response = json_data_provider.load(datap::Request { @@ -22,7 +18,6 @@ fn test_read_json() { key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), payload: None }).unwrap(); - println!("{:?}", response); let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs index e69de29bb2d..4a3a2a333a0 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/datap_json/tests/test_no_std.rs @@ -0,0 +1,32 @@ +extern crate icu_datap_json; + +use icu_datap_json::JsonDataProvider; +use icu_util::datap; +use icu_util::datap::DataProvider; + +#[test] +fn test_read_string() { + let data = r#" + { + "decimal": { + "symbols_v1_a": { + "zero_digit": "0", + "decimal_separator": ".", + "grouping_separator": "," + } + } + }"#; + let json_data_provider = JsonDataProvider::from_str(data).unwrap(); + let response = json_data_provider.load(datap::Request { + locale: "root".to_string(), + category: datap::Category::Decimal, + key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), + payload: None + }).unwrap(); + let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); + assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { + zero_digit: '0', + decimal_separator: ".".to_string(), + grouping_separator: ",".to_string(), + }); +} diff --git a/components/datap_json/tests/testdata/all.json b/components/datap_json/tests/testdata/all.json index f566de168ff..7b79ec08437 100644 --- a/components/datap_json/tests/testdata/all.json +++ b/components/datap_json/tests/testdata/all.json @@ -1,12 +1,5 @@ { "decimal": { - "symbols_v1": { - "SymbolsV1": { - "zero_digit": "0", - "decimal_separator": ".", - "grouping_separator": "," - } - }, "symbols_v1_a": { "zero_digit": "0", "decimal_separator": ".", diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index b96964121a4..f9724ae8d77 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -50,6 +50,7 @@ pub struct Request { pub payload: Option, } +// Please try not to make this trait public, because it is easy to use incorrectly. It is fine as an internal auto-implemented trait. trait CloneableAny: Debug + Any { fn clone_into_box(&self) -> Box; fn as_any(&self) -> &dyn Any; From 1ed302a2c597edfececcfc514b4de0d848c8f852 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 6 May 2020 02:26:33 -0500 Subject: [PATCH 15/33] Adding take_payload, plus other cleanup --- components/datap_json/src/lib.rs | 11 ++-- components/datap_json/tests/test_no_std.rs | 56 +++++++++++++----- components/util/Cargo.toml | 3 + components/util/src/datap/mod.rs | 68 ++++++++++++++++------ 4 files changed, 100 insertions(+), 38 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 66d4038b614..1caf82cd1b0 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -46,10 +46,13 @@ impl JsonDataProvider { } impl datap::DataProvider for JsonDataProvider { - fn load(&self, _request: datap::Request) -> Result { - // TODO: Use the request variable - let mut response = datap::Response::with_owned_payload(self.data.decimal.symbols_v1_a.clone()); - response.locale = "en".to_string(); + fn load(&self, request: datap::Request) -> Result { + // Clone the object, since the response could outlive the provider. + let payload = self.data.decimal.symbols_v1_a.clone(); + let response = datap::ResponseBuilder { + data_locale: "und".to_string(), + request: request + }.with_owned_payload(payload); Ok(response) } } diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs index 4a3a2a333a0..f16c4655428 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/datap_json/tests/test_no_std.rs @@ -1,32 +1,58 @@ extern crate icu_datap_json; +use std::prelude::v1::*; +use std::borrow::Cow; + use icu_datap_json::JsonDataProvider; use icu_util::datap; use icu_util::datap::DataProvider; -#[test] -fn test_read_string() { - let data = r#" - { - "decimal": { - "symbols_v1_a": { - "zero_digit": "0", - "decimal_separator": ".", - "grouping_separator": "," - } - } - }"#; - let json_data_provider = JsonDataProvider::from_str(data).unwrap(); - let response = json_data_provider.load(datap::Request { +const DATA: &'static str = r#"{ + "decimal": { + "symbols_v1_a": { + "zero_digit": "0", + "decimal_separator": ".", + "grouping_separator": "," + } + } +}"#; + +fn get_response() -> datap::Response { + let json_data_provider = JsonDataProvider::from_str(DATA).unwrap(); + return json_data_provider.load(datap::Request { locale: "root".to_string(), category: datap::Category::Decimal, key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), payload: None }).unwrap(); - let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); +} + +fn check_data(decimal_data: &datap::decimal::SymbolsV1) { assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', decimal_separator: ".".to_string(), grouping_separator: ",".to_string(), }); } + +#[test] +fn test_read_string() { + let response = get_response(); + let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); + check_data(decimal_data); +} + +#[test] +fn test_borrow_payload_mut() { + let mut response = get_response(); + let decimal_data: &mut datap::decimal::SymbolsV1 = response.borrow_payload_mut().unwrap(); + check_data(decimal_data); +} + +#[test] +fn test_take_payload() { + let response = get_response(); + let decimal_data: Cow<'static, datap::decimal::SymbolsV1> = + response.take_payload().unwrap(); + check_data(&decimal_data); +} diff --git a/components/util/Cargo.toml b/components/util/Cargo.toml index 0f0c687da0d..b9816842c5c 100644 --- a/components/util/Cargo.toml +++ b/components/util/Cargo.toml @@ -18,6 +18,9 @@ include = [ default = ["std"] std = ["serde/std", "no-std-compat/std"] +[dependencies.downcast-rs] +version = "1.1.1" + [dependencies.async-trait] version = "0.1.30" diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index f9724ae8d77..03b2f29c5a0 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -8,9 +8,10 @@ use std::borrow::Borrow; use std::borrow::BorrowMut; use std::borrow::Cow; use std::error::Error; -use std::any::Any; use std::fmt::{Debug, Display, self}; use core::ops::Deref; +use downcast_rs::Downcast; +use downcast_rs::impl_downcast; pub type Str = Cow<'static, str>; @@ -40,21 +41,23 @@ impl Display for Key { } } -#[derive(PartialEq, Clone)] +#[derive(Debug, PartialEq, Clone)] pub struct Request { // TODO: Make this a Locale instead of a String pub locale: String, pub category: Category, pub key: Key, - // TODO: Make payload a dynamic type instead of a string + + // For now, the request payload is a string. If a more expressive type is needed in the + // future, it should implement PartialEq and Clone. Consider using a concrete type, rather + // than a detour through Any (like in Response), which complicates the code. pub payload: Option, } -// Please try not to make this trait public, because it is easy to use incorrectly. It is fine as an internal auto-implemented trait. -trait CloneableAny: Debug + Any { +// Please do not to make this trait public, because it is easy to use incorrectly. It is fine as +// an internal auto-implemented trait. +trait CloneableAny: Debug + Downcast { fn clone_into_box(&self) -> Box; - fn as_any(&self) -> &dyn Any; - fn as_any_mut(&mut self) -> &mut dyn Any; } impl ToOwned for dyn CloneableAny { @@ -70,22 +73,26 @@ impl CloneableAny for S { fn clone_into_box(&self) -> Box { Box::new(self.clone()) } - fn as_any(&self) -> &dyn Any { - self - } - fn as_any_mut(&mut self) -> &mut dyn Any { - self - } } -// TODO: Enable PartialEq on Response (is that necessary?) +// Adds the Downcast methods to all 'static types implementing CloneableAny. +impl_downcast!(CloneableAny); + #[derive(Debug, Clone)] pub struct Response { // TODO: Make this a Locale instead of a String - pub locale: String, + pub data_locale: String, + // TODO: Is it useful to have the Request saved in the Response? + pub request: Request, payload: Cow<'static, dyn CloneableAny>, } +// Used to build a response without a payload. +pub struct ResponseBuilder { + pub data_locale: String, + pub request: Request, +} + // TODO: Should this be an implemention of std::borrow::Borrow? impl Response { pub fn borrow_payload(&self) -> Option<&T> { @@ -99,16 +106,39 @@ impl Response { borrowed.as_any_mut().downcast_mut::() } - pub fn with_owned_payload(t: T) -> Self { + pub fn take_payload(self) -> Option> { + match self.payload { + Cow::Borrowed(borrowed) => { + match borrowed.as_any().downcast_ref::() { + Some(v) => Some(Cow::Borrowed(v)), + None => None + } + } + Cow::Owned(boxed) => { + match boxed.into_any().downcast::() { + Ok(boxed_t) => Some(Cow::Owned(*boxed_t)), + Err(_) => None + } + } + } + } +} + +impl ResponseBuilder { + // Move from self: the Response replaces the ResponseBuilder + pub fn with_owned_payload(self, t: T) -> Response { Response { - locale: "und".to_string(), + data_locale: self.data_locale, + request: self.request, payload: Cow::Owned(Box::new(t) as Box), } } - pub fn with_borrowed_payload(t: &'static T) -> Self { + // Move from self: the Response replaces the ResponseBuilder + pub fn with_borrowed_payload(self, t: &'static T) -> Response { Response { - locale: "und".to_string(), + data_locale: self.data_locale, + request: self.request, payload: Cow::Borrowed(t), } } From 4cd595a85fe947258f6c9f201c240d2d78d89347 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 6 May 2020 02:27:38 -0500 Subject: [PATCH 16/33] Running cargo fmt --- components/datap_json/src/lib.rs | 13 ++++---- components/datap_json/tests/test_file_io.rs | 27 +++++++++------- components/util/src/datap/mod.rs | 34 +++++++++------------ 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 1caf82cd1b0..01c2ec93329 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -14,7 +14,7 @@ use std::io::Read; #[derive(Debug)] pub enum Error { - JsonError(serde_json::error::Error) + JsonError(serde_json::error::Error), } impl From for Error { @@ -31,17 +31,17 @@ impl JsonDataProvider { #[cfg(feature = "std")] pub fn from_reader(reader: R) -> Result { let result: schema::JsonSchema = serde_json::from_reader(reader)?; - Ok(JsonDataProvider{ data: result }) + Ok(JsonDataProvider { data: result }) } pub fn from_str(data: &str) -> Result { let result: schema::JsonSchema = serde_json::from_str(data)?; - Ok(JsonDataProvider{ data: result }) + Ok(JsonDataProvider { data: result }) } pub fn from_slice(data: &[u8]) -> Result { let result: schema::JsonSchema = serde_json::from_slice(data)?; - Ok(JsonDataProvider{ data: result }) + Ok(JsonDataProvider { data: result }) } } @@ -51,8 +51,9 @@ impl datap::DataProvider for JsonDataProvider { let payload = self.data.decimal.symbols_v1_a.clone(); let response = datap::ResponseBuilder { data_locale: "und".to_string(), - request: request - }.with_owned_payload(payload); + request: request, + } + .with_owned_payload(payload); Ok(response) } } diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index 0ff6fa7ccf8..f12ef2230de 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -12,16 +12,21 @@ fn test_read_json() { let file = File::open("tests/testdata/all.json").unwrap(); let reader = BufReader::new(file); let json_data_provider = JsonDataProvider::from_reader(reader).unwrap(); - let response = json_data_provider.load(datap::Request { - locale: "root".to_string(), - category: datap::Category::Decimal, - key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), - payload: None - }).unwrap(); + let response = json_data_provider + .load(datap::Request { + locale: "root".to_string(), + category: datap::Category::Decimal, + key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), + payload: None, + }) + .unwrap(); let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); - assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { - zero_digit: '0', - decimal_separator: ".".to_string(), - grouping_separator: ",".to_string(), - }); + assert_eq!( + decimal_data, + &datap::decimal::SymbolsV1 { + zero_digit: '0', + decimal_separator: ".".to_string(), + grouping_separator: ",".to_string(), + } + ); } diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 03b2f29c5a0..55f61086905 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -4,14 +4,14 @@ pub mod decimal; use std::prelude::v1::*; // use async_trait::async_trait; +use core::ops::Deref; +use downcast_rs::impl_downcast; +use downcast_rs::Downcast; use std::borrow::Borrow; use std::borrow::BorrowMut; use std::borrow::Cow; use std::error::Error; -use std::fmt::{Debug, Display, self}; -use core::ops::Deref; -use downcast_rs::Downcast; -use downcast_rs::impl_downcast; +use std::fmt::{self, Debug, Display}; pub type Str = Cow<'static, str>; @@ -32,7 +32,7 @@ impl Display for Category { pub enum Key { Undefined, Decimal(decimal::Key), - PrivateUse(u32) + PrivateUse(u32), } impl Display for Key { @@ -62,7 +62,7 @@ trait CloneableAny: Debug + Downcast { impl ToOwned for dyn CloneableAny { type Owned = Box; - + fn to_owned(&self) -> Self::Owned { CloneableAny::clone_into_box(self) } @@ -108,18 +108,14 @@ impl Response { pub fn take_payload(self) -> Option> { match self.payload { - Cow::Borrowed(borrowed) => { - match borrowed.as_any().downcast_ref::() { - Some(v) => Some(Cow::Borrowed(v)), - None => None - } - } - Cow::Owned(boxed) => { - match boxed.into_any().downcast::() { - Ok(boxed_t) => Some(Cow::Owned(*boxed_t)), - Err(_) => None - } - } + Cow::Borrowed(borrowed) => match borrowed.as_any().downcast_ref::() { + Some(v) => Some(Cow::Borrowed(v)), + None => None, + }, + Cow::Owned(boxed) => match boxed.into_any().downcast::() { + Ok(boxed_t) => Some(Cow::Owned(*boxed_t)), + Err(_) => None, + }, } } } @@ -162,7 +158,7 @@ impl Error for ResponseError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { ResponseError::ResourceError(error) => Some(error.deref()), - _ => None + _ => None, } } } From 727f40b886226c552bd70bfeb02c215a096e682f Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Thu, 7 May 2020 01:20:08 -0500 Subject: [PATCH 17/33] Running cargo fmt --- components/datap_json/tests/test_no_std.rs | 32 ++++++++++++---------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs index f16c4655428..cbb04fd6366 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/datap_json/tests/test_no_std.rs @@ -1,7 +1,7 @@ extern crate icu_datap_json; -use std::prelude::v1::*; use std::borrow::Cow; +use std::prelude::v1::*; use icu_datap_json::JsonDataProvider; use icu_util::datap; @@ -19,20 +19,25 @@ const DATA: &'static str = r#"{ fn get_response() -> datap::Response { let json_data_provider = JsonDataProvider::from_str(DATA).unwrap(); - return json_data_provider.load(datap::Request { - locale: "root".to_string(), - category: datap::Category::Decimal, - key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), - payload: None - }).unwrap(); + return json_data_provider + .load(datap::Request { + locale: "root".to_string(), + category: datap::Category::Decimal, + key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), + payload: None, + }) + .unwrap(); } fn check_data(decimal_data: &datap::decimal::SymbolsV1) { - assert_eq!(decimal_data, &datap::decimal::SymbolsV1 { - zero_digit: '0', - decimal_separator: ".".to_string(), - grouping_separator: ",".to_string(), - }); + assert_eq!( + decimal_data, + &datap::decimal::SymbolsV1 { + zero_digit: '0', + decimal_separator: ".".to_string(), + grouping_separator: ",".to_string(), + } + ); } #[test] @@ -52,7 +57,6 @@ fn test_borrow_payload_mut() { #[test] fn test_take_payload() { let response = get_response(); - let decimal_data: Cow<'static, datap::decimal::SymbolsV1> = - response.take_payload().unwrap(); + let decimal_data: Cow<'static, datap::decimal::SymbolsV1> = response.take_payload().unwrap(); check_data(&decimal_data); } From d55f1fd939f45aa48a97bed7c354d9fd925ae997 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 15 May 2020 19:20:08 -0500 Subject: [PATCH 18/33] Remove unused unstable feature --- components/util/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/util/src/lib.rs b/components/util/src/lib.rs index 48609d6bb68..f0e69ee2aab 100644 --- a/components/util/src/lib.rs +++ b/components/util/src/lib.rs @@ -1,5 +1,4 @@ #![no_std] -#![feature(type_name_of_val)] extern crate no_std_compat as std; From 488fe228d7bd4ffa3b2f5632f87875ba7614ab3f Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 15 May 2020 19:33:24 -0500 Subject: [PATCH 19/33] Comments and refactoring. --- components/util/src/datap/helpers.rs | 29 +++++++++++++ components/util/src/datap/mod.rs | 62 +++++++++++----------------- 2 files changed, 52 insertions(+), 39 deletions(-) create mode 100644 components/util/src/datap/helpers.rs diff --git a/components/util/src/datap/helpers.rs b/components/util/src/datap/helpers.rs new file mode 100644 index 00000000000..530cafe0413 --- /dev/null +++ b/components/util/src/datap/helpers.rs @@ -0,0 +1,29 @@ +use std::prelude::v1::*; + +use downcast_rs::Downcast; +use downcast_rs::impl_downcast; +use std::fmt::{Debug}; + +// Please do not to make this trait public, because it is easy to use incorrectly. It is fine as +// an internal auto-implemented trait. +pub(super) trait CloneableAny: Debug + Downcast { + fn clone_into_box(&self) -> Box; +} + +impl ToOwned for dyn CloneableAny { + type Owned = Box; + + fn to_owned(&self) -> Self::Owned { + CloneableAny::clone_into_box(self) + } +} + +// Implement CloneableAny for all 'static types implementing Clone. +impl CloneableAny for S { + fn clone_into_box(&self) -> Box { + Box::new(self.clone()) + } +} + +// Adds the Downcast methods to all 'static types implementing CloneableAny. +impl_downcast!(CloneableAny); diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 55f61086905..f6622684fc6 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -2,19 +2,18 @@ pub mod decimal; -use std::prelude::v1::*; -// use async_trait::async_trait; +mod helpers; + use core::ops::Deref; -use downcast_rs::impl_downcast; -use downcast_rs::Downcast; +use helpers::CloneableAny; use std::borrow::Borrow; use std::borrow::BorrowMut; use std::borrow::Cow; use std::error::Error; use std::fmt::{self, Debug, Display}; +use std::prelude::v1::*; -pub type Str = Cow<'static, str>; - +/// A top-level collection of related data keys. #[derive(PartialEq, Copy, Clone, Debug)] pub enum Category { Undefined = 0, @@ -28,6 +27,7 @@ impl Display for Category { } } +/// A specific key within a category. #[derive(PartialEq, Copy, Clone, Debug)] pub enum Key { Undefined, @@ -41,6 +41,7 @@ impl Display for Key { } } +/// A struct to request a certain hunk of data from a data provider. #[derive(Debug, PartialEq, Clone)] pub struct Request { // TODO: Make this a Locale instead of a String @@ -51,33 +52,10 @@ pub struct Request { // For now, the request payload is a string. If a more expressive type is needed in the // future, it should implement PartialEq and Clone. Consider using a concrete type, rather // than a detour through Any (like in Response), which complicates the code. - pub payload: Option, -} - -// Please do not to make this trait public, because it is easy to use incorrectly. It is fine as -// an internal auto-implemented trait. -trait CloneableAny: Debug + Downcast { - fn clone_into_box(&self) -> Box; -} - -impl ToOwned for dyn CloneableAny { - type Owned = Box; - - fn to_owned(&self) -> Self::Owned { - CloneableAny::clone_into_box(self) - } -} - -// Implement CloneableAny for all 'static types implementing Clone. -impl CloneableAny for S { - fn clone_into_box(&self) -> Box { - Box::new(self.clone()) - } + pub payload: Option>, } -// Adds the Downcast methods to all 'static types implementing CloneableAny. -impl_downcast!(CloneableAny); - +/// A response object containing a data hunk ("payload"). #[derive(Debug, Clone)] pub struct Response { // TODO: Make this a Locale instead of a String @@ -87,25 +65,22 @@ pub struct Response { payload: Cow<'static, dyn CloneableAny>, } -// Used to build a response without a payload. -pub struct ResponseBuilder { - pub data_locale: String, - pub request: Request, -} - // TODO: Should this be an implemention of std::borrow::Borrow? impl Response { + /// Get an immutable reference to the payload in a response object. pub fn borrow_payload(&self) -> Option<&T> { let borrowed: &dyn CloneableAny = self.payload.borrow(); borrowed.as_any().downcast_ref::() } + /// Get a mutable reference to the payload in a response object. pub fn borrow_payload_mut(&mut self) -> Option<&mut T> { let boxed: &mut Box = self.payload.to_mut(); let borrowed: &mut dyn CloneableAny = boxed.borrow_mut(); borrowed.as_any_mut().downcast_mut::() } + /// Take ownership of the payload from a response object. Consumes the response object. pub fn take_payload(self) -> Option> { match self.payload { Cow::Borrowed(borrowed) => match borrowed.as_any().downcast_ref::() { @@ -120,8 +95,15 @@ impl Response { } } +/// Builder class used to construct a Response. +pub struct ResponseBuilder { + pub data_locale: String, + pub request: Request, +} + impl ResponseBuilder { - // Move from self: the Response replaces the ResponseBuilder + /// Construct a Response from the builder, with owned data. + /// Consumes both the builder and the data. pub fn with_owned_payload(self, t: T) -> Response { Response { data_locale: self.data_locale, @@ -130,7 +112,8 @@ impl ResponseBuilder { } } - // Move from self: the Response replaces the ResponseBuilder + /// Construct a Response from the builder, with borrowed data. + /// Consumes the builder, but not the data. pub fn with_borrowed_payload(self, t: &'static T) -> Response { Response { data_locale: self.data_locale, @@ -163,6 +146,7 @@ impl Error for ResponseError { } } +/// An abstract data providewr that takes a request object and returns a response with a payload. // TODO: Make this async // #[async_trait] pub trait DataProvider { From 427c76daa997967c2a59c88d2fa406f2078c4e09 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Sat, 16 May 2020 01:33:08 -0500 Subject: [PATCH 20/33] Change payload functions to return Error instead of Option --- components/datap_json/src/lib.rs | 5 ++- components/util/src/datap/helpers.rs | 4 +-- components/util/src/datap/mod.rs | 52 ++++++++++++++++++++++------ 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 01c2ec93329..f426ea78810 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -3,7 +3,6 @@ extern crate no_std_compat as std; use std::prelude::v1::*; -// use async_trait::async_trait; use icu_util::datap; @@ -23,22 +22,26 @@ impl From for Error { } } +/// A data provider reading from a JSON file. pub struct JsonDataProvider { data: schema::JsonSchema, } impl JsonDataProvider { + /// Create a JsonDataProvider from a file reader. #[cfg(feature = "std")] pub fn from_reader(reader: R) -> Result { let result: schema::JsonSchema = serde_json::from_reader(reader)?; Ok(JsonDataProvider { data: result }) } + /// Create a JsonDataProvider from a JSON string slice. pub fn from_str(data: &str) -> Result { let result: schema::JsonSchema = serde_json::from_str(data)?; Ok(JsonDataProvider { data: result }) } + /// Create a JsonDataProvider from a &[u8] slice. pub fn from_slice(data: &[u8]) -> Result { let result: schema::JsonSchema = serde_json::from_slice(data)?; Ok(JsonDataProvider { data: result }) diff --git a/components/util/src/datap/helpers.rs b/components/util/src/datap/helpers.rs index 530cafe0413..b2182de0ca2 100644 --- a/components/util/src/datap/helpers.rs +++ b/components/util/src/datap/helpers.rs @@ -1,8 +1,8 @@ use std::prelude::v1::*; -use downcast_rs::Downcast; use downcast_rs::impl_downcast; -use std::fmt::{Debug}; +use downcast_rs::Downcast; +use std::fmt::Debug; // Please do not to make this trait public, because it is easy to use incorrectly. It is fine as // an internal auto-implemented trait. diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index f6622684fc6..6b7cc231316 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -6,6 +6,8 @@ mod helpers; use core::ops::Deref; use helpers::CloneableAny; +use std::any::Any; +use std::any::TypeId; use std::borrow::Borrow; use std::borrow::BorrowMut; use std::borrow::Cow; @@ -65,31 +67,61 @@ pub struct Response { payload: Cow<'static, dyn CloneableAny>, } +#[derive(Debug)] +pub enum PayloadError { + /// The type argument does not match the payload. The actual TypeId is provided. + TypeMismatchError(TypeId), +} + +impl Display for PayloadError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO: should the Error Display be different from Debug? + Debug::fmt(self, f) + } +} + +impl Error for PayloadError {} + +impl From for PayloadError { + fn from(type_id: TypeId) -> Self { + PayloadError::TypeMismatchError(type_id) + } +} + // TODO: Should this be an implemention of std::borrow::Borrow? +// TODO: Should the error types be &dyn Any, like for Box::downcast? impl Response { /// Get an immutable reference to the payload in a response object. - pub fn borrow_payload(&self) -> Option<&T> { + pub fn borrow_payload(&self) -> Result<&T, PayloadError> { let borrowed: &dyn CloneableAny = self.payload.borrow(); - borrowed.as_any().downcast_ref::() + borrowed + .as_any() + .downcast_ref::() + .ok_or_else(|| PayloadError::from(borrowed.as_any().type_id())) } /// Get a mutable reference to the payload in a response object. - pub fn borrow_payload_mut(&mut self) -> Option<&mut T> { + pub fn borrow_payload_mut(&mut self) -> Result<&mut T, PayloadError> { let boxed: &mut Box = self.payload.to_mut(); - let borrowed: &mut dyn CloneableAny = boxed.borrow_mut(); - borrowed.as_any_mut().downcast_mut::() + let borrowed_mut: &mut dyn CloneableAny = boxed.borrow_mut(); + // TODO: If I move this into the lambda, I get E0502. Why? + let type_id = borrowed_mut.as_any().type_id(); + borrowed_mut + .as_any_mut() + .downcast_mut::() + .ok_or_else(|| PayloadError::from(type_id)) } /// Take ownership of the payload from a response object. Consumes the response object. - pub fn take_payload(self) -> Option> { + pub fn take_payload(self) -> Result, PayloadError> { match self.payload { Cow::Borrowed(borrowed) => match borrowed.as_any().downcast_ref::() { - Some(v) => Some(Cow::Borrowed(v)), - None => None, + Some(v) => Ok(Cow::Borrowed(v)), + None => Err(PayloadError::from(borrowed.as_any().type_id())), }, Cow::Owned(boxed) => match boxed.into_any().downcast::() { - Ok(boxed_t) => Some(Cow::Owned(*boxed_t)), - Err(_) => None, + Ok(boxed_t) => Ok(Cow::Owned(*boxed_t)), + Err(boxed_any) => Err(PayloadError::from(boxed_any.type_id())), }, } } From 6dd1f3430ae7c39ce43d4138deb5017cbca7d769 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Tue, 19 May 2020 19:35:58 -0500 Subject: [PATCH 21/33] Initial code review feedback --- components/datap_json/tests/test_file_io.rs | 2 +- components/util/src/datap/mod.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index f12ef2230de..baa626607da 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -1,4 +1,4 @@ -extern crate icu_datap_json; +use icu_datap_json; use std::fs::File; use std::io::BufReader; diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 6b7cc231316..44c28e602bd 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -91,7 +91,7 @@ impl From for PayloadError { // TODO: Should this be an implemention of std::borrow::Borrow? // TODO: Should the error types be &dyn Any, like for Box::downcast? impl Response { - /// Get an immutable reference to the payload in a response object. + /// Get an immutable reference to the payload in a Response object. pub fn borrow_payload(&self) -> Result<&T, PayloadError> { let borrowed: &dyn CloneableAny = self.payload.borrow(); borrowed @@ -100,7 +100,7 @@ impl Response { .ok_or_else(|| PayloadError::from(borrowed.as_any().type_id())) } - /// Get a mutable reference to the payload in a response object. + /// Get a mutable reference to the payload in a Response object. pub fn borrow_payload_mut(&mut self) -> Result<&mut T, PayloadError> { let boxed: &mut Box = self.payload.to_mut(); let borrowed_mut: &mut dyn CloneableAny = boxed.borrow_mut(); @@ -112,7 +112,7 @@ impl Response { .ok_or_else(|| PayloadError::from(type_id)) } - /// Take ownership of the payload from a response object. Consumes the response object. + /// Take ownership of the payload from a Response object. Consumes the Response object. pub fn take_payload(self) -> Result, PayloadError> { match self.payload { Cow::Borrowed(borrowed) => match borrowed.as_any().downcast_ref::() { From d8b7b12e35231a1a30553024ef08c52d9556232d Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 27 May 2020 18:53:54 -0500 Subject: [PATCH 22/33] Add lifetime parameter to DataProvider. --- components/datap_json/src/lib.rs | 7 ++++--- components/datap_json/tests/test_file_io.rs | 2 +- components/datap_json/tests/test_no_std.rs | 4 ++-- components/util/src/datap/mod.rs | 19 +++++++++++-------- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index f426ea78810..5f13150e7bf 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -48,13 +48,14 @@ impl JsonDataProvider { } } -impl datap::DataProvider for JsonDataProvider { - fn load(&self, request: datap::Request) -> Result { +impl datap::DataProvider<'static> for JsonDataProvider { + /// Loads JSON data. Always returns owned data, so the 'static lifetime is used. + fn load(&self, request: &datap::Request) -> Result, datap::ResponseError> { // Clone the object, since the response could outlive the provider. let payload = self.data.decimal.symbols_v1_a.clone(); let response = datap::ResponseBuilder { data_locale: "und".to_string(), - request: request, + request: request.clone(), // TODO: don't clone if the request is removed } .with_owned_payload(payload); Ok(response) diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index baa626607da..b4e6a7298d0 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -13,7 +13,7 @@ fn test_read_json() { let reader = BufReader::new(file); let json_data_provider = JsonDataProvider::from_reader(reader).unwrap(); let response = json_data_provider - .load(datap::Request { + .load(&datap::Request { locale: "root".to_string(), category: datap::Category::Decimal, key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs index cbb04fd6366..fc65332ae54 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/datap_json/tests/test_no_std.rs @@ -17,10 +17,10 @@ const DATA: &'static str = r#"{ } }"#; -fn get_response() -> datap::Response { +fn get_response() -> datap::Response<'static> { let json_data_provider = JsonDataProvider::from_str(DATA).unwrap(); return json_data_provider - .load(datap::Request { + .load(&datap::Request { locale: "root".to_string(), category: datap::Category::Decimal, key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 44c28e602bd..fe430e85747 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -59,12 +59,12 @@ pub struct Request { /// A response object containing a data hunk ("payload"). #[derive(Debug, Clone)] -pub struct Response { +pub struct Response<'d> { // TODO: Make this a Locale instead of a String pub data_locale: String, // TODO: Is it useful to have the Request saved in the Response? pub request: Request, - payload: Cow<'static, dyn CloneableAny>, + payload: Cow<'d, dyn CloneableAny>, } #[derive(Debug)] @@ -90,7 +90,8 @@ impl From for PayloadError { // TODO: Should this be an implemention of std::borrow::Borrow? // TODO: Should the error types be &dyn Any, like for Box::downcast? -impl Response { +impl<'d> Response<'d> { + // TODO: Return &'d T? /// Get an immutable reference to the payload in a Response object. pub fn borrow_payload(&self) -> Result<&T, PayloadError> { let borrowed: &dyn CloneableAny = self.payload.borrow(); @@ -100,6 +101,7 @@ impl Response { .ok_or_else(|| PayloadError::from(borrowed.as_any().type_id())) } + // TODO: Return &'d mut T? /// Get a mutable reference to the payload in a Response object. pub fn borrow_payload_mut(&mut self) -> Result<&mut T, PayloadError> { let boxed: &mut Box = self.payload.to_mut(); @@ -113,7 +115,7 @@ impl Response { } /// Take ownership of the payload from a Response object. Consumes the Response object. - pub fn take_payload(self) -> Result, PayloadError> { + pub fn take_payload(self) -> Result, PayloadError> { match self.payload { Cow::Borrowed(borrowed) => match borrowed.as_any().downcast_ref::() { Some(v) => Ok(Cow::Borrowed(v)), @@ -136,7 +138,8 @@ pub struct ResponseBuilder { impl ResponseBuilder { /// Construct a Response from the builder, with owned data. /// Consumes both the builder and the data. - pub fn with_owned_payload(self, t: T) -> Response { + /// Returns the 'static lifetime since there is no borrowed data. + pub fn with_owned_payload(self, t: T) -> Response<'static> { Response { data_locale: self.data_locale, request: self.request, @@ -146,7 +149,7 @@ impl ResponseBuilder { /// Construct a Response from the builder, with borrowed data. /// Consumes the builder, but not the data. - pub fn with_borrowed_payload(self, t: &'static T) -> Response { + pub fn with_borrowed_payload<'d, T: 'static + Clone + Debug>(self, t: &'d T) -> Response<'d> { Response { data_locale: self.data_locale, request: self.request, @@ -181,6 +184,6 @@ impl Error for ResponseError { /// An abstract data providewr that takes a request object and returns a response with a payload. // TODO: Make this async // #[async_trait] -pub trait DataProvider { - fn load(&self, req: Request) -> Result; +pub trait DataProvider<'d> { + fn load(&self, req: &Request) -> Result, ResponseError>; } From d95b1fc037ff8e5052bcec834da3f98b3b67adb3 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 27 May 2020 19:02:55 -0500 Subject: [PATCH 23/33] Removing Request from Response object. --- components/datap_json/src/lib.rs | 3 +-- components/util/src/datap/mod.rs | 9 ++------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 5f13150e7bf..0c7554a73ff 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -50,12 +50,11 @@ impl JsonDataProvider { impl datap::DataProvider<'static> for JsonDataProvider { /// Loads JSON data. Always returns owned data, so the 'static lifetime is used. - fn load(&self, request: &datap::Request) -> Result, datap::ResponseError> { + fn load(&self, _request: &datap::Request) -> Result, datap::ResponseError> { // Clone the object, since the response could outlive the provider. let payload = self.data.decimal.symbols_v1_a.clone(); let response = datap::ResponseBuilder { data_locale: "und".to_string(), - request: request.clone(), // TODO: don't clone if the request is removed } .with_owned_payload(payload); Ok(response) diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index fe430e85747..724b9da6b23 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -62,8 +62,6 @@ pub struct Request { pub struct Response<'d> { // TODO: Make this a Locale instead of a String pub data_locale: String, - // TODO: Is it useful to have the Request saved in the Response? - pub request: Request, payload: Cow<'d, dyn CloneableAny>, } @@ -91,8 +89,8 @@ impl From for PayloadError { // TODO: Should this be an implemention of std::borrow::Borrow? // TODO: Should the error types be &dyn Any, like for Box::downcast? impl<'d> Response<'d> { - // TODO: Return &'d T? /// Get an immutable reference to the payload in a Response object. + /// The payload may or may not be owned by the Response. pub fn borrow_payload(&self) -> Result<&T, PayloadError> { let borrowed: &dyn CloneableAny = self.payload.borrow(); borrowed @@ -101,8 +99,8 @@ impl<'d> Response<'d> { .ok_or_else(|| PayloadError::from(borrowed.as_any().type_id())) } - // TODO: Return &'d mut T? /// Get a mutable reference to the payload in a Response object. + /// The payload may or may not be owned by the Response. pub fn borrow_payload_mut(&mut self) -> Result<&mut T, PayloadError> { let boxed: &mut Box = self.payload.to_mut(); let borrowed_mut: &mut dyn CloneableAny = boxed.borrow_mut(); @@ -132,7 +130,6 @@ impl<'d> Response<'d> { /// Builder class used to construct a Response. pub struct ResponseBuilder { pub data_locale: String, - pub request: Request, } impl ResponseBuilder { @@ -142,7 +139,6 @@ impl ResponseBuilder { pub fn with_owned_payload(self, t: T) -> Response<'static> { Response { data_locale: self.data_locale, - request: self.request, payload: Cow::Owned(Box::new(t) as Box), } } @@ -152,7 +148,6 @@ impl ResponseBuilder { pub fn with_borrowed_payload<'d, T: 'static + Clone + Debug>(self, t: &'d T) -> Response<'d> { Response { data_locale: self.data_locale, - request: self.request, payload: Cow::Borrowed(t), } } From 40205d935c9632ad11a7faa5af36d1eb4fe119fd Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 27 May 2020 20:54:35 -0500 Subject: [PATCH 24/33] Upgrade JsonDataProvider to returned borrowed data --- components/datap_json/src/lib.rs | 10 +++---- components/datap_json/tests/test_no_std.rs | 31 +++++++++++++++++----- components/util/src/datap/mod.rs | 10 +++++-- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index 0c7554a73ff..b404249bcbf 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -48,15 +48,13 @@ impl JsonDataProvider { } } -impl datap::DataProvider<'static> for JsonDataProvider { - /// Loads JSON data. Always returns owned data, so the 'static lifetime is used. - fn load(&self, _request: &datap::Request) -> Result, datap::ResponseError> { - // Clone the object, since the response could outlive the provider. - let payload = self.data.decimal.symbols_v1_a.clone(); +impl<'a> datap::DataProvider<'a, 'a> for JsonDataProvider { + /// Loads JSON data. Returns borrowed data. + fn load(&'a self, _request: &datap::Request) -> Result, datap::ResponseError> { let response = datap::ResponseBuilder { data_locale: "und".to_string(), } - .with_owned_payload(payload); + .with_borrowed_payload(&self.data.decimal.symbols_v1_a); Ok(response) } } diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs index fc65332ae54..cef6f82f407 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/datap_json/tests/test_no_std.rs @@ -17,9 +17,12 @@ const DATA: &'static str = r#"{ } }"#; -fn get_response() -> datap::Response<'static> { - let json_data_provider = JsonDataProvider::from_str(DATA).unwrap(); - return json_data_provider +fn get_provider() -> JsonDataProvider { + JsonDataProvider::from_str(DATA).unwrap() +} + +fn get_response(provider: &JsonDataProvider) -> datap::Response { + return provider .load(&datap::Request { locale: "root".to_string(), category: datap::Category::Decimal, @@ -42,21 +45,35 @@ fn check_data(decimal_data: &datap::decimal::SymbolsV1) { #[test] fn test_read_string() { - let response = get_response(); + let provider = get_provider(); + let response = get_response(&provider); let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); check_data(decimal_data); } #[test] fn test_borrow_payload_mut() { - let mut response = get_response(); + let provider = get_provider(); + let mut response = get_response(&provider); let decimal_data: &mut datap::decimal::SymbolsV1 = response.borrow_payload_mut().unwrap(); check_data(decimal_data); } #[test] fn test_take_payload() { - let response = get_response(); - let decimal_data: Cow<'static, datap::decimal::SymbolsV1> = response.take_payload().unwrap(); + let provider = get_provider(); + let response = get_response(&provider); + let decimal_data: Cow = response.take_payload().unwrap(); check_data(&decimal_data); } + +#[test] +fn test_clone_payload() { + let final_data = { + let provider = get_provider(); + let response = get_response(&provider); + let decimal_data: Cow = response.take_payload().unwrap(); + decimal_data.into_owned() + }; + check_data(&final_data); +} diff --git a/components/util/src/datap/mod.rs b/components/util/src/datap/mod.rs index 724b9da6b23..8a625caaecc 100644 --- a/components/util/src/datap/mod.rs +++ b/components/util/src/datap/mod.rs @@ -177,8 +177,14 @@ impl Error for ResponseError { } /// An abstract data providewr that takes a request object and returns a response with a payload. +/// Lifetimes: +/// - 'a = lifetime of the DataProvider object +/// - 'd = lifetime of the borrowed payload +/// Note: 'd and 'a can be the same, but they do not need to be. For example, 'd = 'static if: +/// 1. The provider always returns data that lives in static memory +/// 2. The provider always returns owned data, not borrowed data // TODO: Make this async // #[async_trait] -pub trait DataProvider<'d> { - fn load(&self, req: &Request) -> Result, ResponseError>; +pub trait DataProvider<'a, 'd> { + fn load(&'a self, req: &Request) -> Result, ResponseError>; } From 08ac7f122e8ad1155ca59d93a4ec12843e6797a3 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 29 May 2020 16:00:15 -0500 Subject: [PATCH 25/33] Allow strings in SymbolsV1 to be static --- components/datap_json/tests/test_file_io.rs | 6 ++++-- components/datap_json/tests/test_no_std.rs | 4 ++-- components/util/src/datap/decimal.rs | 6 ++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index b4e6a7298d0..7f6867eca59 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -1,5 +1,7 @@ use icu_datap_json; +use std::borrow::Cow; + use std::fs::File; use std::io::BufReader; @@ -25,8 +27,8 @@ fn test_read_json() { decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', - decimal_separator: ".".to_string(), - grouping_separator: ",".to_string(), + decimal_separator: Cow::Borrowed("."), + grouping_separator: Cow::Borrowed(","), } ); } diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs index cef6f82f407..592a6435ad8 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/datap_json/tests/test_no_std.rs @@ -37,8 +37,8 @@ fn check_data(decimal_data: &datap::decimal::SymbolsV1) { decimal_data, &datap::decimal::SymbolsV1 { zero_digit: '0', - decimal_separator: ".".to_string(), - grouping_separator: ",".to_string(), + decimal_separator: Cow::Borrowed("."), + grouping_separator: Cow::Borrowed(","), } ); } diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index 55737c1e781..f1452287760 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -2,6 +2,8 @@ use std::prelude::v1::*; +use std::borrow::Cow; + use serde::{Deserialize, Serialize}; #[derive(PartialEq, Copy, Clone, Debug)] @@ -13,6 +15,6 @@ pub enum Key { #[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] pub struct SymbolsV1 { pub zero_digit: char, - pub decimal_separator: String, - pub grouping_separator: String, + pub decimal_separator: Cow<'static, str>, + pub grouping_separator: Cow<'static, str>, } From 2194dbf4051ad530ed2789d3afe58ad8658b67f2 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Fri, 29 May 2020 16:02:15 -0500 Subject: [PATCH 26/33] Add backreference --- components/util/src/datap/decimal.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/util/src/datap/decimal.rs b/components/util/src/datap/decimal.rs index f1452287760..591d1cd7b4a 100644 --- a/components/util/src/datap/decimal.rs +++ b/components/util/src/datap/decimal.rs @@ -15,6 +15,8 @@ pub enum Key { #[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] pub struct SymbolsV1 { pub zero_digit: char, + // String ownership discussion: + // https://github.com/unicode-org/icu4x/pull/61#discussion_r429051895 pub decimal_separator: Cow<'static, str>, pub grouping_separator: Cow<'static, str>, } From 0d6828aea40625cb904a752b6553bb8a832b5454 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Sat, 6 Jun 2020 01:42:05 -0500 Subject: [PATCH 27/33] Coverage --- components/datap_json/src/lib.rs | 13 +++++++++++++ components/datap_json/src/schema.rs | 4 ++-- components/datap_json/tests/test_file_io.rs | 1 + components/datap_json/tests/test_no_std.rs | 8 ++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/components/datap_json/src/lib.rs b/components/datap_json/src/lib.rs index b404249bcbf..94bf8ef11f7 100644 --- a/components/datap_json/src/lib.rs +++ b/components/datap_json/src/lib.rs @@ -23,6 +23,7 @@ impl From for Error { } /// A data provider reading from a JSON file. +#[derive(Debug)] pub struct JsonDataProvider { data: schema::JsonSchema, } @@ -58,3 +59,15 @@ impl<'a> datap::DataProvider<'a, 'a> for JsonDataProvider { Ok(response) } } + +#[test] +fn test_empty_str() { + let result = JsonDataProvider::from_str(""); + assert!(result.is_err()); + let err = result.unwrap_err(); + println!("{:?}", err); // Coverage for Debug trait + // An unconditional let is possible here because it is a one-element enum. + // If more cases are needed, see https://github.com/rust-lang/rfcs/pull/1303 + let Error::JsonError(json_err) = err; + assert_eq!(json_err.classify(), serde_json::error::Category::Eof); +} diff --git a/components/datap_json/src/schema.rs b/components/datap_json/src/schema.rs index 3a1d6851706..0057ca58240 100644 --- a/components/datap_json/src/schema.rs +++ b/components/datap_json/src/schema.rs @@ -4,12 +4,12 @@ use serde::{Deserialize, Serialize}; #[allow(unused_imports)] use std::prelude::v1::*; -#[derive(Serialize, Deserialize)] +#[derive(Serialize, Deserialize, Debug)] pub(crate) struct DecimalJsonSchema { pub(crate) symbols_v1_a: datap::decimal::SymbolsV1, } -#[derive(Serialize, Deserialize)] +#[derive(Serialize, Deserialize, Debug)] pub(crate) struct JsonSchema { pub(crate) decimal: DecimalJsonSchema, } diff --git a/components/datap_json/tests/test_file_io.rs b/components/datap_json/tests/test_file_io.rs index 7f6867eca59..253a4c7c3a3 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/datap_json/tests/test_file_io.rs @@ -14,6 +14,7 @@ fn test_read_json() { let file = File::open("tests/testdata/all.json").unwrap(); let reader = BufReader::new(file); let json_data_provider = JsonDataProvider::from_reader(reader).unwrap(); + println!("{:?}", json_data_provider); // Coverage for Debug trait let response = json_data_provider .load(&datap::Request { locale: "root".to_string(), diff --git a/components/datap_json/tests/test_no_std.rs b/components/datap_json/tests/test_no_std.rs index 592a6435ad8..2aceaca7721 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/datap_json/tests/test_no_std.rs @@ -51,6 +51,14 @@ fn test_read_string() { check_data(decimal_data); } +#[test] +fn test_read_utf8() { + let provider = JsonDataProvider::from_slice(DATA.as_bytes()).unwrap(); + let response = get_response(&provider); + let decimal_data: &datap::decimal::SymbolsV1 = response.borrow_payload().unwrap(); + check_data(decimal_data); +} + #[test] fn test_borrow_payload_mut() { let provider = get_provider(); From 25a806e62a31199f154ee293f815708856657464 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Sat, 6 Jun 2020 01:57:51 -0500 Subject: [PATCH 28/33] New crate names. --- Cargo.toml | 4 ++-- components/{util => data_provider}/Cargo.toml | 4 ++-- components/{datap_json => data_provider}/README.md | 0 .../{util/src/datap => data_provider/src}/decimal.rs | 0 .../{util/src/datap => data_provider/src}/helpers.rs | 0 .../src/datap/mod.rs => data_provider/src/lib.rs} | 4 +++- .../{datap_json => data_provider_json}/Cargo.toml | 4 ++-- components/{util => data_provider_json}/README.md | 0 .../{datap_json => data_provider_json}/src/lib.rs | 12 ++++++++---- .../{datap_json => data_provider_json}/src/schema.rs | 2 +- .../tests/test_file_io.rs | 8 ++++---- .../tests/test_no_std.rs | 8 ++++---- .../tests/testdata/all.json | 0 components/util/src/lib.rs | 5 ----- 14 files changed, 26 insertions(+), 25 deletions(-) rename components/{util => data_provider}/Cargo.toml (86%) rename components/{datap_json => data_provider}/README.md (100%) rename components/{util/src/datap => data_provider/src}/decimal.rs (100%) rename components/{util/src/datap => data_provider/src}/helpers.rs (100%) rename components/{util/src/datap/mod.rs => data_provider/src/lib.rs} (99%) rename components/{datap_json => data_provider_json}/Cargo.toml (91%) rename components/{util => data_provider_json}/README.md (100%) rename components/{datap_json => data_provider_json}/src/lib.rs (84%) rename components/{datap_json => data_provider_json}/src/schema.rs (91%) rename components/{datap_json => data_provider_json}/tests/test_file_io.rs (86%) rename components/{datap_json => data_provider_json}/tests/test_no_std.rs (93%) rename components/{datap_json => data_provider_json}/tests/testdata/all.json (100%) delete mode 100644 components/util/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 678b18f032f..671fac5ffdf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,9 +1,9 @@ [workspace] members = [ - "components/datap_json", + "components/data_provider", + "components/data_provider_json", "components/icu", "components/icu4x", "components/locale", - "components/util", ] diff --git a/components/util/Cargo.toml b/components/data_provider/Cargo.toml similarity index 86% rename from components/util/Cargo.toml rename to components/data_provider/Cargo.toml index b9816842c5c..fa9a42c95c2 100644 --- a/components/util/Cargo.toml +++ b/components/data_provider/Cargo.toml @@ -1,6 +1,6 @@ [package] -name = "icu-util" -description = "Common trait definitions and utilities for ICU4X" +name = "icu-data-provider" +description = "Trait and struct definitions for the ICU data provider" version = "0.0.1" authors = ["The ICU4X Project Developers"] edition = "2018" diff --git a/components/datap_json/README.md b/components/data_provider/README.md similarity index 100% rename from components/datap_json/README.md rename to components/data_provider/README.md diff --git a/components/util/src/datap/decimal.rs b/components/data_provider/src/decimal.rs similarity index 100% rename from components/util/src/datap/decimal.rs rename to components/data_provider/src/decimal.rs diff --git a/components/util/src/datap/helpers.rs b/components/data_provider/src/helpers.rs similarity index 100% rename from components/util/src/datap/helpers.rs rename to components/data_provider/src/helpers.rs diff --git a/components/util/src/datap/mod.rs b/components/data_provider/src/lib.rs similarity index 99% rename from components/util/src/datap/mod.rs rename to components/data_provider/src/lib.rs index 8a625caaecc..53f3a62d977 100644 --- a/components/util/src/datap/mod.rs +++ b/components/data_provider/src/lib.rs @@ -1,4 +1,6 @@ -// Data provider trait definitions +#![no_std] + +extern crate no_std_compat as std; pub mod decimal; diff --git a/components/datap_json/Cargo.toml b/components/data_provider_json/Cargo.toml similarity index 91% rename from components/datap_json/Cargo.toml rename to components/data_provider_json/Cargo.toml index 01c5a568958..db5cd447693 100644 --- a/components/datap_json/Cargo.toml +++ b/components/data_provider_json/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "icu-datap-json" +name = "icu-data-provider-json" description = "ICU4X data provider that reads from JSON files" version = "0.0.1" authors = ["The ICU4X Project Developers"] @@ -19,7 +19,7 @@ default = ["std"] std = ["no-std-compat/std", "serde/std", "serde_json/std"] [dependencies] -icu-util = { path = "../util" } +icu-data-provider = { path = "../data_provider" } [dependencies.async-trait] version = "0.1.30" diff --git a/components/util/README.md b/components/data_provider_json/README.md similarity index 100% rename from components/util/README.md rename to components/data_provider_json/README.md diff --git a/components/datap_json/src/lib.rs b/components/data_provider_json/src/lib.rs similarity index 84% rename from components/datap_json/src/lib.rs rename to components/data_provider_json/src/lib.rs index 94bf8ef11f7..acb8e8cae53 100644 --- a/components/datap_json/src/lib.rs +++ b/components/data_provider_json/src/lib.rs @@ -4,7 +4,11 @@ extern crate no_std_compat as std; use std::prelude::v1::*; -use icu_util::datap; +use icu_data_provider::DataProvider; +use icu_data_provider::Request; +use icu_data_provider::Response; +use icu_data_provider::ResponseBuilder; +use icu_data_provider::ResponseError; mod schema; @@ -49,10 +53,10 @@ impl JsonDataProvider { } } -impl<'a> datap::DataProvider<'a, 'a> for JsonDataProvider { +impl<'a> DataProvider<'a, 'a> for JsonDataProvider { /// Loads JSON data. Returns borrowed data. - fn load(&'a self, _request: &datap::Request) -> Result, datap::ResponseError> { - let response = datap::ResponseBuilder { + fn load(&'a self, _request: &Request) -> Result, ResponseError> { + let response = ResponseBuilder { data_locale: "und".to_string(), } .with_borrowed_payload(&self.data.decimal.symbols_v1_a); diff --git a/components/datap_json/src/schema.rs b/components/data_provider_json/src/schema.rs similarity index 91% rename from components/datap_json/src/schema.rs rename to components/data_provider_json/src/schema.rs index 0057ca58240..b8f59c3413f 100644 --- a/components/datap_json/src/schema.rs +++ b/components/data_provider_json/src/schema.rs @@ -1,4 +1,4 @@ -use icu_util::datap; +use icu_data_provider as datap; use serde::{Deserialize, Serialize}; #[allow(unused_imports)] diff --git a/components/datap_json/tests/test_file_io.rs b/components/data_provider_json/tests/test_file_io.rs similarity index 86% rename from components/datap_json/tests/test_file_io.rs rename to components/data_provider_json/tests/test_file_io.rs index 253a4c7c3a3..ec904a0f931 100644 --- a/components/datap_json/tests/test_file_io.rs +++ b/components/data_provider_json/tests/test_file_io.rs @@ -1,13 +1,13 @@ -use icu_datap_json; +use icu_data_provider_json; use std::borrow::Cow; use std::fs::File; use std::io::BufReader; -use icu_datap_json::JsonDataProvider; -use icu_util::datap; -use icu_util::datap::DataProvider; +use icu_data_provider_json::JsonDataProvider; +use icu_data_provider as datap; +use icu_data_provider::DataProvider; #[test] fn test_read_json() { diff --git a/components/datap_json/tests/test_no_std.rs b/components/data_provider_json/tests/test_no_std.rs similarity index 93% rename from components/datap_json/tests/test_no_std.rs rename to components/data_provider_json/tests/test_no_std.rs index 2aceaca7721..7925937dc99 100644 --- a/components/datap_json/tests/test_no_std.rs +++ b/components/data_provider_json/tests/test_no_std.rs @@ -1,11 +1,11 @@ -extern crate icu_datap_json; +extern crate icu_data_provider_json; use std::borrow::Cow; use std::prelude::v1::*; -use icu_datap_json::JsonDataProvider; -use icu_util::datap; -use icu_util::datap::DataProvider; +use icu_data_provider_json::JsonDataProvider; +use icu_data_provider as datap; +use icu_data_provider::DataProvider; const DATA: &'static str = r#"{ "decimal": { diff --git a/components/datap_json/tests/testdata/all.json b/components/data_provider_json/tests/testdata/all.json similarity index 100% rename from components/datap_json/tests/testdata/all.json rename to components/data_provider_json/tests/testdata/all.json diff --git a/components/util/src/lib.rs b/components/util/src/lib.rs deleted file mode 100644 index f0e69ee2aab..00000000000 --- a/components/util/src/lib.rs +++ /dev/null @@ -1,5 +0,0 @@ -#![no_std] - -extern crate no_std_compat as std; - -pub mod datap; From e738a9f4ef65a5ec1dbcbd8d23af8249cc2266a2 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Sat, 6 Jun 2020 02:15:24 -0500 Subject: [PATCH 29/33] Add key From impl --- components/data_provider/src/decimal.rs | 6 ++++++ components/data_provider_json/tests/test_file_io.rs | 2 +- components/data_provider_json/tests/test_no_std.rs | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/components/data_provider/src/decimal.rs b/components/data_provider/src/decimal.rs index 591d1cd7b4a..55dd6a211c1 100644 --- a/components/data_provider/src/decimal.rs +++ b/components/data_provider/src/decimal.rs @@ -11,6 +11,12 @@ pub enum Key { SymbolsV1 = 1, } +impl From for crate::Key { + fn from(value: Key) -> Self { + crate::Key::Decimal(value) + } +} + // TODO: de-duplicate the name "SymbolsV1" between Key and the struct #[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] pub struct SymbolsV1 { diff --git a/components/data_provider_json/tests/test_file_io.rs b/components/data_provider_json/tests/test_file_io.rs index ec904a0f931..2ea60295481 100644 --- a/components/data_provider_json/tests/test_file_io.rs +++ b/components/data_provider_json/tests/test_file_io.rs @@ -19,7 +19,7 @@ fn test_read_json() { .load(&datap::Request { locale: "root".to_string(), category: datap::Category::Decimal, - key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), + key: datap::decimal::Key::SymbolsV1.into(), payload: None, }) .unwrap(); diff --git a/components/data_provider_json/tests/test_no_std.rs b/components/data_provider_json/tests/test_no_std.rs index 7925937dc99..948bf8c465c 100644 --- a/components/data_provider_json/tests/test_no_std.rs +++ b/components/data_provider_json/tests/test_no_std.rs @@ -26,7 +26,7 @@ fn get_response(provider: &JsonDataProvider) -> datap::Response { .load(&datap::Request { locale: "root".to_string(), category: datap::Category::Decimal, - key: datap::Key::Decimal(datap::decimal::Key::SymbolsV1), + key: datap::decimal::Key::SymbolsV1.into(), payload: None, }) .unwrap(); From 82af10e6413c3a891173a4b8444b069c2c817a66 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Sat, 6 Jun 2020 02:25:32 -0500 Subject: [PATCH 30/33] Running cargo fmt --- components/data_provider_json/src/lib.rs | 3 ++- components/data_provider_json/tests/test_file_io.rs | 4 ++-- components/data_provider_json/tests/test_no_std.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/components/data_provider_json/src/lib.rs b/components/data_provider_json/src/lib.rs index acb8e8cae53..2e6b07e4c63 100644 --- a/components/data_provider_json/src/lib.rs +++ b/components/data_provider_json/src/lib.rs @@ -69,7 +69,8 @@ fn test_empty_str() { let result = JsonDataProvider::from_str(""); assert!(result.is_err()); let err = result.unwrap_err(); - println!("{:?}", err); // Coverage for Debug trait + // Coverage for Debug trait: + println!("{:?}", err); // An unconditional let is possible here because it is a one-element enum. // If more cases are needed, see https://github.com/rust-lang/rfcs/pull/1303 let Error::JsonError(json_err) = err; diff --git a/components/data_provider_json/tests/test_file_io.rs b/components/data_provider_json/tests/test_file_io.rs index 2ea60295481..ecaa71a7bb8 100644 --- a/components/data_provider_json/tests/test_file_io.rs +++ b/components/data_provider_json/tests/test_file_io.rs @@ -5,16 +5,16 @@ use std::borrow::Cow; use std::fs::File; use std::io::BufReader; -use icu_data_provider_json::JsonDataProvider; use icu_data_provider as datap; use icu_data_provider::DataProvider; +use icu_data_provider_json::JsonDataProvider; #[test] fn test_read_json() { let file = File::open("tests/testdata/all.json").unwrap(); let reader = BufReader::new(file); let json_data_provider = JsonDataProvider::from_reader(reader).unwrap(); - println!("{:?}", json_data_provider); // Coverage for Debug trait + println!("{:?}", json_data_provider); // Coverage for Debug trait let response = json_data_provider .load(&datap::Request { locale: "root".to_string(), diff --git a/components/data_provider_json/tests/test_no_std.rs b/components/data_provider_json/tests/test_no_std.rs index 948bf8c465c..8f848e07c91 100644 --- a/components/data_provider_json/tests/test_no_std.rs +++ b/components/data_provider_json/tests/test_no_std.rs @@ -3,9 +3,9 @@ extern crate icu_data_provider_json; use std::borrow::Cow; use std::prelude::v1::*; -use icu_data_provider_json::JsonDataProvider; use icu_data_provider as datap; use icu_data_provider::DataProvider; +use icu_data_provider_json::JsonDataProvider; const DATA: &'static str = r#"{ "decimal": { From 56d678e8f302e2e104fdd451e4fa428e0fdbe4db Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Sat, 6 Jun 2020 02:44:20 -0500 Subject: [PATCH 31/33] Use LanguageIdentifier instead of String --- components/data_provider/Cargo.toml | 3 +++ components/data_provider/src/lib.rs | 14 +++++++------- components/data_provider_json/src/lib.rs | 2 +- .../data_provider_json/tests/test_file_io.rs | 2 +- components/data_provider_json/tests/test_no_std.rs | 2 +- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/components/data_provider/Cargo.toml b/components/data_provider/Cargo.toml index fa9a42c95c2..7584a07edd4 100644 --- a/components/data_provider/Cargo.toml +++ b/components/data_provider/Cargo.toml @@ -18,6 +18,9 @@ include = [ default = ["std"] std = ["serde/std", "no-std-compat/std"] +[dependencies] +icu-locale = { path = "../locale" } + [dependencies.downcast-rs] version = "1.1.1" diff --git a/components/data_provider/src/lib.rs b/components/data_provider/src/lib.rs index 53f3a62d977..32c5b59b295 100644 --- a/components/data_provider/src/lib.rs +++ b/components/data_provider/src/lib.rs @@ -17,6 +17,8 @@ use std::error::Error; use std::fmt::{self, Debug, Display}; use std::prelude::v1::*; +use icu_locale::LanguageIdentifier; + /// A top-level collection of related data keys. #[derive(PartialEq, Copy, Clone, Debug)] pub enum Category { @@ -48,8 +50,7 @@ impl Display for Key { /// A struct to request a certain hunk of data from a data provider. #[derive(Debug, PartialEq, Clone)] pub struct Request { - // TODO: Make this a Locale instead of a String - pub locale: String, + pub langid: LanguageIdentifier, pub category: Category, pub key: Key, @@ -62,8 +63,7 @@ pub struct Request { /// A response object containing a data hunk ("payload"). #[derive(Debug, Clone)] pub struct Response<'d> { - // TODO: Make this a Locale instead of a String - pub data_locale: String, + pub data_langid: LanguageIdentifier, payload: Cow<'d, dyn CloneableAny>, } @@ -131,7 +131,7 @@ impl<'d> Response<'d> { /// Builder class used to construct a Response. pub struct ResponseBuilder { - pub data_locale: String, + pub data_langid: LanguageIdentifier, } impl ResponseBuilder { @@ -140,7 +140,7 @@ impl ResponseBuilder { /// Returns the 'static lifetime since there is no borrowed data. pub fn with_owned_payload(self, t: T) -> Response<'static> { Response { - data_locale: self.data_locale, + data_langid: self.data_langid, payload: Cow::Owned(Box::new(t) as Box), } } @@ -149,7 +149,7 @@ impl ResponseBuilder { /// Consumes the builder, but not the data. pub fn with_borrowed_payload<'d, T: 'static + Clone + Debug>(self, t: &'d T) -> Response<'d> { Response { - data_locale: self.data_locale, + data_langid: self.data_langid, payload: Cow::Borrowed(t), } } diff --git a/components/data_provider_json/src/lib.rs b/components/data_provider_json/src/lib.rs index 2e6b07e4c63..5bd004e3a5f 100644 --- a/components/data_provider_json/src/lib.rs +++ b/components/data_provider_json/src/lib.rs @@ -57,7 +57,7 @@ impl<'a> DataProvider<'a, 'a> for JsonDataProvider { /// Loads JSON data. Returns borrowed data. fn load(&'a self, _request: &Request) -> Result, ResponseError> { let response = ResponseBuilder { - data_locale: "und".to_string(), + data_langid: "und".parse().unwrap(), } .with_borrowed_payload(&self.data.decimal.symbols_v1_a); Ok(response) diff --git a/components/data_provider_json/tests/test_file_io.rs b/components/data_provider_json/tests/test_file_io.rs index ecaa71a7bb8..38a85fdcba1 100644 --- a/components/data_provider_json/tests/test_file_io.rs +++ b/components/data_provider_json/tests/test_file_io.rs @@ -17,7 +17,7 @@ fn test_read_json() { println!("{:?}", json_data_provider); // Coverage for Debug trait let response = json_data_provider .load(&datap::Request { - locale: "root".to_string(), + langid: "en-US".parse().unwrap(), category: datap::Category::Decimal, key: datap::decimal::Key::SymbolsV1.into(), payload: None, diff --git a/components/data_provider_json/tests/test_no_std.rs b/components/data_provider_json/tests/test_no_std.rs index 8f848e07c91..ae5e22a6cd7 100644 --- a/components/data_provider_json/tests/test_no_std.rs +++ b/components/data_provider_json/tests/test_no_std.rs @@ -24,7 +24,7 @@ fn get_provider() -> JsonDataProvider { fn get_response(provider: &JsonDataProvider) -> datap::Response { return provider .load(&datap::Request { - locale: "root".to_string(), + langid: "en-US".parse().unwrap(), category: datap::Category::Decimal, key: datap::decimal::Key::SymbolsV1.into(), payload: None, From d3a7d36db7c97cce992a6dab8a4a5c5fd125647f Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Mon, 8 Jun 2020 20:35:05 -0500 Subject: [PATCH 32/33] Rename directories to kebab case --- Cargo.toml | 4 ++-- .../{data_provider_json => data-provider-json}/Cargo.toml | 2 +- components/{data_provider => data-provider-json}/README.md | 0 .../{data_provider_json => data-provider-json}/src/lib.rs | 0 .../{data_provider_json => data-provider-json}/src/schema.rs | 0 .../tests/test_file_io.rs | 0 .../tests/test_no_std.rs | 0 .../tests/testdata/all.json | 0 components/{data_provider => data-provider}/Cargo.toml | 0 components/{data_provider_json => data-provider}/README.md | 0 components/{data_provider => data-provider}/src/decimal.rs | 0 components/{data_provider => data-provider}/src/helpers.rs | 0 components/{data_provider => data-provider}/src/lib.rs | 0 13 files changed, 3 insertions(+), 3 deletions(-) rename components/{data_provider_json => data-provider-json}/Cargo.toml (94%) rename components/{data_provider => data-provider-json}/README.md (100%) rename components/{data_provider_json => data-provider-json}/src/lib.rs (100%) rename components/{data_provider_json => data-provider-json}/src/schema.rs (100%) rename components/{data_provider_json => data-provider-json}/tests/test_file_io.rs (100%) rename components/{data_provider_json => data-provider-json}/tests/test_no_std.rs (100%) rename components/{data_provider_json => data-provider-json}/tests/testdata/all.json (100%) rename components/{data_provider => data-provider}/Cargo.toml (100%) rename components/{data_provider_json => data-provider}/README.md (100%) rename components/{data_provider => data-provider}/src/decimal.rs (100%) rename components/{data_provider => data-provider}/src/helpers.rs (100%) rename components/{data_provider => data-provider}/src/lib.rs (100%) diff --git a/Cargo.toml b/Cargo.toml index 671fac5ffdf..d8e0f7812fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,8 +1,8 @@ [workspace] members = [ - "components/data_provider", - "components/data_provider_json", + "components/data-provider", + "components/data-provider-json", "components/icu", "components/icu4x", "components/locale", diff --git a/components/data_provider_json/Cargo.toml b/components/data-provider-json/Cargo.toml similarity index 94% rename from components/data_provider_json/Cargo.toml rename to components/data-provider-json/Cargo.toml index db5cd447693..a7cf64ad09f 100644 --- a/components/data_provider_json/Cargo.toml +++ b/components/data-provider-json/Cargo.toml @@ -19,7 +19,7 @@ default = ["std"] std = ["no-std-compat/std", "serde/std", "serde_json/std"] [dependencies] -icu-data-provider = { path = "../data_provider" } +icu-data-provider = { path = "../data-provider" } [dependencies.async-trait] version = "0.1.30" diff --git a/components/data_provider/README.md b/components/data-provider-json/README.md similarity index 100% rename from components/data_provider/README.md rename to components/data-provider-json/README.md diff --git a/components/data_provider_json/src/lib.rs b/components/data-provider-json/src/lib.rs similarity index 100% rename from components/data_provider_json/src/lib.rs rename to components/data-provider-json/src/lib.rs diff --git a/components/data_provider_json/src/schema.rs b/components/data-provider-json/src/schema.rs similarity index 100% rename from components/data_provider_json/src/schema.rs rename to components/data-provider-json/src/schema.rs diff --git a/components/data_provider_json/tests/test_file_io.rs b/components/data-provider-json/tests/test_file_io.rs similarity index 100% rename from components/data_provider_json/tests/test_file_io.rs rename to components/data-provider-json/tests/test_file_io.rs diff --git a/components/data_provider_json/tests/test_no_std.rs b/components/data-provider-json/tests/test_no_std.rs similarity index 100% rename from components/data_provider_json/tests/test_no_std.rs rename to components/data-provider-json/tests/test_no_std.rs diff --git a/components/data_provider_json/tests/testdata/all.json b/components/data-provider-json/tests/testdata/all.json similarity index 100% rename from components/data_provider_json/tests/testdata/all.json rename to components/data-provider-json/tests/testdata/all.json diff --git a/components/data_provider/Cargo.toml b/components/data-provider/Cargo.toml similarity index 100% rename from components/data_provider/Cargo.toml rename to components/data-provider/Cargo.toml diff --git a/components/data_provider_json/README.md b/components/data-provider/README.md similarity index 100% rename from components/data_provider_json/README.md rename to components/data-provider/README.md diff --git a/components/data_provider/src/decimal.rs b/components/data-provider/src/decimal.rs similarity index 100% rename from components/data_provider/src/decimal.rs rename to components/data-provider/src/decimal.rs diff --git a/components/data_provider/src/helpers.rs b/components/data-provider/src/helpers.rs similarity index 100% rename from components/data_provider/src/helpers.rs rename to components/data-provider/src/helpers.rs diff --git a/components/data_provider/src/lib.rs b/components/data-provider/src/lib.rs similarity index 100% rename from components/data_provider/src/lib.rs rename to components/data-provider/src/lib.rs From c663a313ee815d9e546fb778057263d33182156e Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Mon, 8 Jun 2020 20:40:07 -0500 Subject: [PATCH 33/33] Use LanguageIdentifier::default() --- components/data-provider-json/Cargo.toml | 1 + components/data-provider-json/src/lib.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/components/data-provider-json/Cargo.toml b/components/data-provider-json/Cargo.toml index a7cf64ad09f..3c52a1e268c 100644 --- a/components/data-provider-json/Cargo.toml +++ b/components/data-provider-json/Cargo.toml @@ -20,6 +20,7 @@ std = ["no-std-compat/std", "serde/std", "serde_json/std"] [dependencies] icu-data-provider = { path = "../data-provider" } +icu-locale = { path = "../locale" } [dependencies.async-trait] version = "0.1.30" diff --git a/components/data-provider-json/src/lib.rs b/components/data-provider-json/src/lib.rs index 5bd004e3a5f..8f61dfb9747 100644 --- a/components/data-provider-json/src/lib.rs +++ b/components/data-provider-json/src/lib.rs @@ -9,6 +9,7 @@ use icu_data_provider::Request; use icu_data_provider::Response; use icu_data_provider::ResponseBuilder; use icu_data_provider::ResponseError; +use icu_locale::LanguageIdentifier; mod schema; @@ -57,7 +58,7 @@ impl<'a> DataProvider<'a, 'a> for JsonDataProvider { /// Loads JSON data. Returns borrowed data. fn load(&'a self, _request: &Request) -> Result, ResponseError> { let response = ResponseBuilder { - data_langid: "und".parse().unwrap(), + data_langid: LanguageIdentifier::default(), } .with_borrowed_payload(&self.data.decimal.symbols_v1_a); Ok(response)