From 9d4b8c566f9600e2553cf161a61b051ed73d429d Mon Sep 17 00:00:00 2001 From: Maciej Bielecki Date: Tue, 19 Sep 2023 20:52:15 +0000 Subject: [PATCH] Implement `Serialize` without going through JsonObject As described in , serialization by constructing a JsonObject has to essentially clone all data, which requires extra allocations. After this change, we pass data directly to the `Serializer`. `JsonObject` and `JsonValue` conversion impls were rewritten to delegate to `Serialize`. This requires some unfortunate `panic`s where we expect `Serialize` to produce a JSON object, but I think it's better than duplicating the serialization logic. --- CHANGES.md | 1 + src/feature.rs | 71 ++++++++++++++++++--------------------- src/feature_collection.rs | 43 +++++++++++++----------- src/geojson.rs | 6 +++- src/geometry.rs | 71 ++++++++++++++++++++++++++++----------- 5 files changed, 114 insertions(+), 78 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d1e906d..cc779c8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,7 @@ ## Unreleased * Added conversion from `Vec` to `GeoJson`. +* Changed `Serialize` impls to avoid creating intermediate `JsonObject`s. ## 0.24.1 diff --git a/src/feature.rs b/src/feature.rs index 9e82e32..8fb437a 100644 --- a/src/feature.rs +++ b/src/feature.rs @@ -18,8 +18,7 @@ use std::str::FromStr; use crate::errors::{Error, Result}; use crate::{util, Feature, Geometry, Value}; use crate::{JsonObject, JsonValue}; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use serde_json::json; +use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; impl From for Feature { fn from(geom: Geometry) -> Feature { @@ -55,35 +54,13 @@ impl FromStr for Feature { impl<'a> From<&'a Feature> for JsonObject { fn from(feature: &'a Feature) -> JsonObject { - let mut map = JsonObject::new(); - map.insert(String::from("type"), json!("Feature")); - map.insert( - String::from("geometry"), - serde_json::to_value(&feature.geometry).unwrap(), - ); - if let Some(ref properties) = feature.properties { - map.insert( - String::from("properties"), - serde_json::to_value(properties).unwrap(), - ); - } else { - map.insert( - String::from("properties"), - serde_json::to_value(Some(serde_json::Map::new())).unwrap(), - ); - } - if let Some(ref bbox) = feature.bbox { - map.insert(String::from("bbox"), serde_json::to_value(bbox).unwrap()); - } - if let Some(ref id) = feature.id { - map.insert(String::from("id"), serde_json::to_value(id).unwrap()); + match serde_json::to_value(feature).unwrap() { + serde_json::Value::Object(obj) => obj, + value => panic!( + "serializing Feature should result in an Object, but got something {:?}", + value + ), } - if let Some(ref foreign_members) = feature.foreign_members { - for (key, value) in foreign_members { - map.insert(key.to_owned(), value.to_owned()); - } - } - map } } @@ -182,7 +159,26 @@ impl Serialize for Feature { where S: Serializer, { - JsonObject::from(self).serialize(serializer) + let mut map = serializer.serialize_map(None)?; + map.serialize_entry("type", "Feature")?; + map.serialize_entry("geometry", &self.geometry)?; + if let Some(ref properties) = self.properties { + map.serialize_entry("properties", properties)?; + } else { + map.serialize_entry("properties", &JsonObject::new())?; + } + if let Some(ref bbox) = self.bbox { + map.serialize_entry("bbox", bbox)?; + } + if let Some(ref id) = self.id { + map.serialize_entry("id", id)?; + } + if let Some(ref foreign_members) = self.foreign_members { + for (key, value) in foreign_members { + map.serialize_entry(key, value)?; + } + } + map.end() } } @@ -229,8 +225,7 @@ mod tests { use std::str::FromStr; fn feature_json_str() -> &'static str { - "{\"geometry\":{\"coordinates\":[1.1,2.1],\"type\":\"Point\"},\"properties\":{},\"type\":\ - \"Feature\"}" + "{\"type\":\"Feature\",\"geometry\":{\"type\":\"Point\",\"coordinates\":[1.1,2.1]},\"properties\":{}}" } fn properties() -> Option { @@ -314,8 +309,7 @@ mod tests { #[test] fn test_display_feature() { let f = feature().to_string(); - assert_eq!(f, "{\"geometry\":{\"coordinates\":[1.1,2.1],\"type\":\"Point\"},\"properties\":{},\"type\":\ - \"Feature\"}"); + assert_eq!(f, "{\"type\":\"Feature\",\"geometry\":{\"type\":\"Point\",\"coordinates\":[1.1,2.1]},\"properties\":{}}"); } #[test] @@ -344,7 +338,7 @@ mod tests { #[test] fn encode_decode_feature_with_id_number() { - let feature_json_str = "{\"geometry\":{\"coordinates\":[1.1,2.1],\"type\":\"Point\"},\"id\":0,\"properties\":{},\"type\":\"Feature\"}"; + let feature_json_str = "{\"type\":\"Feature\",\"geometry\":{\"type\":\"Point\",\"coordinates\":[1.1,2.1]},\"properties\":{},\"id\":0}"; let feature = crate::Feature { geometry: Some(Geometry { value: Value::Point(vec![1.1, 2.1]), @@ -370,7 +364,7 @@ mod tests { #[test] fn encode_decode_feature_with_id_string() { - let feature_json_str = "{\"geometry\":{\"coordinates\":[1.1,2.1],\"type\":\"Point\"},\"id\":\"foo\",\"properties\":{},\"type\":\"Feature\"}"; + let feature_json_str = "{\"type\":\"Feature\",\"geometry\":{\"type\":\"Point\",\"coordinates\":[1.1,2.1]},\"properties\":{},\"id\":\"foo\"}"; let feature = crate::Feature { geometry: Some(Geometry { value: Value::Point(vec![1.1, 2.1]), @@ -416,7 +410,8 @@ mod tests { fn encode_decode_feature_with_foreign_member() { use crate::JsonObject; use serde_json; - let feature_json_str = "{\"geometry\":{\"coordinates\":[1.1,2.1],\"type\":\"Point\"},\"other_member\":\"some_value\",\"properties\":{},\"type\":\"Feature\"}"; + let feature_json_str = "{\"type\":\"Feature\",\"geometry\":{\"type\":\"Point\",\"coordinates\":[1.1,2.1]},\"properties\":{},\"other_member\":\"some_value\"}"; + let mut foreign_members = JsonObject::new(); foreign_members.insert( String::from("other_member"), diff --git a/src/feature_collection.rs b/src/feature_collection.rs index ba2c83f..3964321 100644 --- a/src/feature_collection.rs +++ b/src/feature_collection.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use serde::ser::SerializeMap; use std::convert::TryFrom; use std::iter::FromIterator; use std::str::FromStr; @@ -20,7 +21,6 @@ use crate::errors::{Error, Result}; use crate::{util, Bbox, Feature}; use crate::{JsonObject, JsonValue}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use serde_json::json; /// Feature Collection Objects /// @@ -44,7 +44,7 @@ use serde_json::json; /// /// assert_eq!( /// serialized, -/// "{\"features\":[],\"type\":\"FeatureCollection\"}" +/// "{\"type\":\"FeatureCollection\",\"features\":[]}" /// ); /// ``` /// @@ -94,24 +94,13 @@ impl<'a> IntoIterator for &'a FeatureCollection { impl<'a> From<&'a FeatureCollection> for JsonObject { fn from(fc: &'a FeatureCollection) -> JsonObject { - let mut map = JsonObject::new(); - map.insert(String::from("type"), json!("FeatureCollection")); - map.insert( - String::from("features"), - serde_json::to_value(&fc.features).unwrap(), - ); - - if let Some(ref bbox) = fc.bbox { - map.insert(String::from("bbox"), serde_json::to_value(bbox).unwrap()); + match serde_json::to_value(fc).unwrap() { + serde_json::Value::Object(obj) => obj, + value => panic!( + "serializing FeatureCollection should result in an Object, but got something {:?}", + value + ), } - - if let Some(ref foreign_members) = fc.foreign_members { - for (key, value) in foreign_members { - map.insert(key.to_owned(), value.to_owned()); - } - } - - map } } @@ -168,7 +157,21 @@ impl Serialize for FeatureCollection { where S: Serializer, { - JsonObject::from(self).serialize(serializer) + let mut map = serializer.serialize_map(None)?; + map.serialize_entry("type", "FeatureCollection")?; + map.serialize_entry("features", &self.features)?; + + if let Some(ref bbox) = self.bbox { + map.serialize_entry("bbox", bbox)?; + } + + if let Some(ref foreign_members) = self.foreign_members { + for (key, value) in foreign_members { + map.serialize_entry(key, value)?; + } + } + + map.end() } } diff --git a/src/geojson.rs b/src/geojson.rs index 862e571..8a3656e 100644 --- a/src/geojson.rs +++ b/src/geojson.rs @@ -301,7 +301,11 @@ impl Serialize for GeoJson { where S: Serializer, { - JsonObject::from(self).serialize(serializer) + match self { + GeoJson::Geometry(ref geometry) => geometry.serialize(serializer), + GeoJson::Feature(ref feature) => feature.serialize(serializer), + GeoJson::FeatureCollection(ref fc) => fc.serialize(serializer), + } } } diff --git a/src/geometry.rs b/src/geometry.rs index a150654..f8fa06a 100644 --- a/src/geometry.rs +++ b/src/geometry.rs @@ -18,7 +18,7 @@ use std::{convert::TryFrom, fmt}; use crate::errors::{Error, Result}; use crate::{util, Bbox, LineStringType, PointType, PolygonType}; use crate::{JsonObject, JsonValue}; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; /// The underlying value for a `Geometry`. /// @@ -123,6 +123,21 @@ impl Value { pub fn from_json_value(value: JsonValue) -> Result { Self::try_from(value) } + + fn serialize_to_map( + &self, + map: &mut SM, + ) -> std::result::Result<(), SM::Error> { + map.serialize_entry("type", self.type_name())?; + map.serialize_entry( + match self { + Value::GeometryCollection(..) => "geometries", + _ => "coordinates", + }, + self, + )?; + Ok(()) + } } impl TryFrom for Value { @@ -155,16 +170,7 @@ impl fmt::Display for Value { impl<'a> From<&'a Value> for JsonValue { fn from(value: &'a Value) -> JsonValue { - match *value { - Value::Point(ref x) => ::serde_json::to_value(x), - Value::MultiPoint(ref x) => ::serde_json::to_value(x), - Value::LineString(ref x) => ::serde_json::to_value(x), - Value::MultiLineString(ref x) => ::serde_json::to_value(x), - Value::Polygon(ref x) => ::serde_json::to_value(x), - Value::MultiPolygon(ref x) => ::serde_json::to_value(x), - Value::GeometryCollection(ref x) => ::serde_json::to_value(x), - } - .unwrap() + ::serde_json::to_value(value).unwrap() } } @@ -173,7 +179,15 @@ impl Serialize for Value { where S: Serializer, { - JsonValue::from(self).serialize(serializer) + match self { + Value::Point(x) => x.serialize(serializer), + Value::MultiPoint(x) => x.serialize(serializer), + Value::LineString(x) => x.serialize(serializer), + Value::MultiLineString(x) => x.serialize(serializer), + Value::Polygon(x) => x.serialize(serializer), + Value::MultiPolygon(x) => x.serialize(serializer), + Value::GeometryCollection(x) => x.serialize(serializer), + } } } @@ -208,7 +222,7 @@ impl Serialize for Value { /// let geojson_string = geometry.to_string(); /// /// assert_eq!( -/// "{\"coordinates\":[7.428959,1.513394],\"type\":\"Point\"}", +/// "{\"type\":\"Point\",\"coordinates\":[7.428959,1.513394]}", /// geojson_string, /// ); /// ``` @@ -291,6 +305,23 @@ impl Geometry { pub fn from_json_value(value: JsonValue) -> Result { Self::try_from(value) } + + fn serialize_to_map( + &self, + map: &mut SM, + ) -> std::result::Result<(), SM::Error> { + self.value.serialize_to_map(map)?; + if let Some(ref bbox) = self.bbox { + map.serialize_entry("bbox", bbox)?; + } + + if let Some(ref foreign_members) = self.foreign_members { + for (key, value) in foreign_members { + map.serialize_entry(key, value)? + } + } + Ok(()) + } } impl TryFrom for Geometry { @@ -333,7 +364,9 @@ impl Serialize for Geometry { where S: Serializer, { - JsonObject::from(self).serialize(serializer) + let mut map = serializer.serialize_map(None)?; + self.serialize_to_map(&mut map)?; + map.end() } } @@ -374,7 +407,7 @@ mod tests { #[test] fn encode_decode_geometry() { - let geometry_json_str = "{\"coordinates\":[1.1,2.1],\"type\":\"Point\"}"; + let geometry_json_str = "{\"type\":\"Point\",\"coordinates\":[1.1,2.1]}"; let geometry = Geometry { value: Value::Point(vec![1.1, 2.1]), bbox: None, @@ -422,8 +455,8 @@ mod tests { let v = Value::LineString(vec![vec![0.0, 0.1], vec![0.1, 0.2], vec![0.2, 0.3]]); let geometry = Geometry::new(v); assert_eq!( - "{\"coordinates\":[[0.0,0.1],[0.1,0.2],[0.2,0.3]],\"type\":\"LineString\"}", - geometry.to_string() + geometry.to_string(), + "{\"type\":\"LineString\",\"coordinates\":[[0.0,0.1],[0.1,0.2],[0.2,0.3]]}" ); } @@ -439,7 +472,7 @@ mod tests { #[test] fn encode_decode_geometry_with_foreign_member() { let geometry_json_str = - "{\"coordinates\":[1.1,2.1],\"other_member\":true,\"type\":\"Point\"}"; + "{\"type\":\"Point\",\"coordinates\":[1.1,2.1],\"other_member\":true}"; let mut foreign_members = JsonObject::new(); foreign_members.insert( String::from("other_member"), @@ -482,7 +515,7 @@ mod tests { foreign_members: None, }; - let geometry_collection_string = "{\"geometries\":[{\"coordinates\":[100.0,0.0],\"type\":\"Point\"},{\"coordinates\":[[101.0,0.0],[102.0,1.0]],\"type\":\"LineString\"}],\"type\":\"GeometryCollection\"}"; + let geometry_collection_string = "{\"type\":\"GeometryCollection\",\"geometries\":[{\"type\":\"Point\",\"coordinates\":[100.0,0.0]},{\"type\":\"LineString\",\"coordinates\":[[101.0,0.0],[102.0,1.0]]}]}"; // Test encode let json_string = encode(&geometry_collection); assert_eq!(json_string, geometry_collection_string);