Skip to content

Commit

Permalink
fix: Numbers were serialized as strings in REST API (#369)
Browse files Browse the repository at this point in the history
  • Loading branch information
chubei authored Dec 8, 2022
1 parent 7742217 commit 114cad2
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 70 deletions.
15 changes: 8 additions & 7 deletions dozer-api/src/api_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ use dozer_cache::cache::{expression::QueryExpression, index};
use dozer_cache::errors::CacheError;
use dozer_cache::{AccessFilter, CacheReader};
use dozer_types::indexmap::IndexMap;
use dozer_types::json_value_to_field;
use dozer_types::record_to_json;
use dozer_types::json_str_to_field;
use dozer_types::record_to_map;
use dozer_types::serde_json::Value;
use dozer_types::types::{FieldType, Record, Schema};
use openapiv3::OpenAPI;

Expand Down Expand Up @@ -70,7 +71,7 @@ impl ApiHelper {
}

/// Get a single record by json string as primary key
pub fn get_record(&self, key: String) -> Result<IndexMap<String, String>, CacheError> {
pub fn get_record(&self, key: &str) -> Result<IndexMap<String, Value>, CacheError> {
let schema = self
.reader
.get_schema_and_indexes_by_name(&self.details.schema_name)?
Expand All @@ -81,23 +82,23 @@ impl ApiHelper {
.iter()
.map(|idx| schema.fields[*idx].typ)
.collect();
let key = json_value_to_field(&key, &field_types[0]).map_err(CacheError::TypeError)?;
let key = json_str_to_field(key, field_types[0]).map_err(CacheError::TypeError)?;

let key = index::get_primary_key(&[0], &[key]);
let rec = self.reader.get(&key)?;

record_to_json(&rec, &schema).map_err(CacheError::TypeError)
record_to_map(&rec, &schema).map_err(CacheError::TypeError)
}

/// Get multiple records
pub fn get_records_map(
&self,
exp: QueryExpression,
) -> Result<Vec<IndexMap<String, String>>, CacheError> {
) -> Result<Vec<IndexMap<String, Value>>, CacheError> {
let mut maps = vec![];
let (schema, records) = self.get_records(exp)?;
for rec in records.iter() {
let map = record_to_json(rec, &schema)?;
let map = record_to_map(rec, &schema)?;
maps.push(map);
}
Ok(maps)
Expand Down
2 changes: 1 addition & 1 deletion dozer-api/src/grpc/shared_impl/filter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ fn record_satisfies_filter(record: &Record, filter: &FilterExpression, schema: &
return false;
};

let Ok(value) = json_value_to_field(value.as_str().unwrap_or(&value.to_string()), &field_definition.typ) else {
let Ok(value) = json_value_to_field(value.clone(), field_definition.typ) else {
return false;
};

Expand Down
2 changes: 1 addition & 1 deletion dozer-api/src/rest/api_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub async fn get(
pipeline_details.into_inner(),
access.map(|a| a.into_inner()),
)?;
let key = path.into_inner();
let key = path.as_str();
helper
.get_record(key)
.map(|map| HttpResponse::Ok().json(map))
Expand Down
2 changes: 1 addition & 1 deletion dozer-api/src/rest/tests/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ async fn get_route() {
let val = body.as_object().unwrap();
assert_eq!(
val.get("film_id").unwrap().to_string(),
"\"268\"".to_string(),
"268".to_string(),
"Must be equal"
);
}
3 changes: 1 addition & 2 deletions dozer-cache/src/cache/plan/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ fn collect_filters(
FilterExpression::Simple(field_name, operator, value) => {
let (field_index, field_type) = get_field_index_and_type(field_name, &schema.fields)
.ok_or(PlanError::FieldNotFound)?;
let field =
json_value_to_field(value.as_str().unwrap_or(&value.to_string()), &field_type)?;
let field = json_value_to_field(value.clone(), field_type)?;
filters.push((IndexFilter::new(field_index, *operator, field), None));
}
FilterExpression::And(expressions) => {
Expand Down
124 changes: 67 additions & 57 deletions dozer-types/src/helper.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,21 @@
use crate::errors::types::{DeserializationError, SerializationError, TypeError};
use crate::errors::types::{DeserializationError, TypeError};
use crate::types::DATE_FORMAT;
use crate::{
errors::types,
types::{Field, FieldType, Record, Schema},
};
use crate::types::{Field, FieldType, Record, Schema};
use chrono::{DateTime, NaiveDate, SecondsFormat};
use indexmap::IndexMap;
use rust_decimal::Decimal;
use serde_json::Value;
use std::str::FromStr;
use std::string::FromUtf8Error;
/// Used in REST APIs for converting to JSON
pub fn record_to_json(
rec: &Record,
schema: &Schema,
) -> Result<IndexMap<String, String>, types::TypeError> {
let mut map: IndexMap<String, String> = IndexMap::new();
pub fn record_to_map(rec: &Record, schema: &Schema) -> Result<IndexMap<String, Value>, TypeError> {
let mut map = IndexMap::new();

for (idx, field_def) in schema.fields.iter().enumerate() {
if rec.values.len() > idx {
let field = rec.values[idx].clone();
let val = field_to_json_value(&field)?;
let val = field_to_json_value(field)
.map_err(|_| TypeError::InvalidFieldValue("Bson field is not valid utf8".into()))?;
map.insert(field_def.name.clone(), val);
}
}
Expand All @@ -27,64 +24,77 @@ pub fn record_to_json(
}

/// Used in REST APIs for converting raw value back and forth
pub fn field_to_json_value(field: &Field) -> Result<String, TypeError> {
fn field_to_json_value(field: Field) -> Result<Value, FromUtf8Error> {
match field {
Field::Int(n) => Ok(serde_json::to_string(n).map_err(SerializationError::Json))?,
Field::Float(n) => Ok(serde_json::to_string(n).map_err(SerializationError::Json))?,
Field::Boolean(b) => Ok(serde_json::to_string(b).map_err(SerializationError::Json))?,
Field::String(s) => Ok(s.to_owned()),
Field::Binary(b) => Ok(serde_json::to_string(b).map_err(SerializationError::Json))?,
Field::Null => Ok("null".to_owned()),
Field::Decimal(n) => Ok(serde_json::to_string(n).map_err(SerializationError::Json))?,
Field::Timestamp(ts) => Ok(ts.to_rfc3339_opts(SecondsFormat::Millis, true)),
Field::Bson(b) => Ok(serde_json::to_string(b).map_err(SerializationError::Json))?,
Field::UInt(n) => Ok(serde_json::to_string(n).map_err(SerializationError::Json))?,
Field::Text(n) => Ok(serde_json::to_string(n).map_err(SerializationError::Json))?,
Field::Date(n) => Ok(n.format(DATE_FORMAT).to_string()),
Field::UInt(n) => Ok(Value::from(n)),
Field::Int(n) => Ok(Value::from(n)),
Field::Float(n) => Ok(Value::from(n.0)),
Field::Boolean(b) => Ok(Value::from(b)),
Field::String(s) => Ok(Value::from(s)),
Field::Text(n) => Ok(Value::from(n)),
Field::Binary(b) => Ok(Value::from(b)),
Field::Decimal(n) => Ok(Value::String(n.to_string())),
Field::Timestamp(ts) => Ok(Value::String(
ts.to_rfc3339_opts(SecondsFormat::Millis, true),
)),
Field::Date(n) => Ok(Value::String(n.format(DATE_FORMAT).to_string())),
Field::Bson(b) => Ok(Value::from(b)),
Field::Null => Ok(Value::Null),
}
.map_err(TypeError::SerializationError)
}

/// Used in REST APIs for converting raw value back and forth
pub fn json_value_to_field(val: &str, typ: &FieldType) -> Result<Field, TypeError> {
match typ {
FieldType::Int => serde_json::from_str(val)
pub fn json_value_to_field(value: Value, typ: FieldType) -> Result<Field, TypeError> {
match (typ, &value) {
(FieldType::UInt, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::UInt),
(FieldType::Int, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::Int),
FieldType::Float => serde_json::from_str(val)
(FieldType::Float, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::Float),
FieldType::Boolean => serde_json::from_str(val)
(FieldType::Boolean, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::Boolean),
FieldType::String => Ok(Field::String(val.to_string())),
FieldType::Binary => serde_json::from_str(val)
(FieldType::String, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::String),
(FieldType::Text, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::Text),
(FieldType::Binary, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::Binary),

FieldType::Decimal => Decimal::from_str(val)
(FieldType::Decimal, Value::String(str)) => Decimal::from_str(str)
.map_err(|e| DeserializationError::Custom(Box::new(e)))
.map(Field::Decimal),
FieldType::Timestamp => DateTime::parse_from_rfc3339(val)
(FieldType::Timestamp, Value::String(str)) => DateTime::parse_from_rfc3339(str)
.map_err(|e| DeserializationError::Custom(Box::new(e)))
.map(Field::Timestamp),
FieldType::Bson => serde_json::from_str(val)
.map_err(DeserializationError::Json)
.map(Field::Bson),
FieldType::Null => Ok(Field::Null),
FieldType::UInt => serde_json::from_str(val)
.map_err(DeserializationError::Json)
.map(Field::UInt),
FieldType::Text => serde_json::from_str(val)
.map_err(DeserializationError::Json)
.map(Field::Text),
FieldType::Date => NaiveDate::parse_from_str(val, DATE_FORMAT)
(FieldType::Date, Value::String(str)) => NaiveDate::parse_from_str(str, DATE_FORMAT)
.map_err(|e| DeserializationError::Custom(Box::new(e)))
.map(Field::Date),
(FieldType::Bson, _) => serde_json::from_value(value)
.map_err(DeserializationError::Json)
.map(Field::Bson),
(FieldType::Null, Value::Null) => Ok(Field::Null),
_ => Err(DeserializationError::Custom(
"Json value type does not match field type"
.to_string()
.into(),
)),
}
.map_err(TypeError::DeserializationError)
}

pub fn json_str_to_field(value: &str, typ: FieldType) -> Result<Field, TypeError> {
let value = serde_json::from_str(value)
.map_err(|e| TypeError::DeserializationError(DeserializationError::Json(e)))?;
json_value_to_field(value, typ)
}

#[cfg(test)]
mod tests {
use crate::{
Expand All @@ -94,20 +104,18 @@ mod tests {
use chrono::{NaiveDate, Offset, TimeZone, Utc};
use ordered_float::OrderedFloat;
use rust_decimal::Decimal;
use serde_json::json;
fn test_field_conversion(field_type: FieldType, field: Field) -> anyhow::Result<()> {
// Convert the field to a JSON string.
let serialized = field_to_json_value(&field).unwrap();
fn test_field_conversion(field_type: FieldType, field: Field) {
// Convert the field to a JSON value.
let value = field_to_json_value(field.clone()).unwrap();

// Convert the JSON string back to a Field.
let deserialized = json_value_to_field(&serialized, &field_type)?;
// Convert the JSON value back to a Field.
let deserialized = json_value_to_field(value, field_type).unwrap();

assert_eq!(deserialized, field, "must be equal");
Ok(())
}

#[test]
fn test_field_types_str_conversion() -> anyhow::Result<()> {
fn test_field_types_str_conversion() {
let fields = vec![
(FieldType::Int, Field::Int(-1)),
(FieldType::UInt, Field::UInt(1)),
Expand All @@ -126,13 +134,15 @@ mod tests {
),
(
FieldType::Bson,
Field::Bson(bincode::serialize(&json!({"a": 1}))?),
Field::Bson(vec![
// BSON representation of `{"abc":"foo"}`
123, 34, 97, 98, 99, 34, 58, 34, 102, 111, 111, 34, 125,
]),
),
(FieldType::Text, Field::Text("lorem ipsum".to_string())),
];
for (field_type, field) in fields {
test_field_conversion(field_type, field)?;
test_field_conversion(field_type, field);
}
Ok(())
}
}
2 changes: 1 addition & 1 deletion dozer-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub mod models;
mod tests;
pub mod types;

pub use helper::{field_to_json_value, json_value_to_field, record_to_json};
pub use helper::{json_str_to_field, json_value_to_field, record_to_map};

// Re-exports
pub use bincode;
Expand Down

0 comments on commit 114cad2

Please sign in to comment.