Skip to content

Commit

Permalink
Implement Serialize without going through JsonObject
Browse files Browse the repository at this point in the history 
As described in <georust#152>,
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.
  • Loading branch information
@zyla
zyla committed Sep 20, 2023
1 parent 4cfbf3a commit 9d4b8c5
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 78 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

* Added conversion from `Vec<Feature>` to `GeoJson`.
* Changed `Serialize` impls to avoid creating intermediate `JsonObject`s.

## 0.24.1

Expand Down
71 changes: 33 additions & 38 deletions src/feature.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Geometry> for Feature {
fn from(geom: Geometry) -> Feature {
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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<JsonObject> {
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]),
Expand All @@ -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]),
Expand Down Expand Up @@ -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"),
Expand Down
43 changes: 23 additions & 20 deletions src/feature_collection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
///
Expand All @@ -44,7 +44,7 @@ use serde_json::json;
///
/// assert_eq!(
/// serialized,
/// "{\"features\":[],\"type\":\"FeatureCollection\"}"
/// "{\"type\":\"FeatureCollection\",\"features\":[]}"
/// );
/// ```
///
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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()
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/geojson.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}

Expand Down
71 changes: 52 additions & 19 deletions src/geometry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
///
Expand Down Expand Up @@ -123,6 +123,21 @@ impl Value {
pub fn from_json_value(value: JsonValue) -> Result<Self> {
Self::try_from(value)
}

fn serialize_to_map<SM: SerializeMap>(
&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<JsonObject> for Value {
Expand Down Expand Up @@ -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()
}
}

Expand All @@ -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),
}
}
}

Expand Down Expand Up @@ -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,
/// );
/// ```
Expand Down Expand Up @@ -291,6 +305,23 @@ impl Geometry {
pub fn from_json_value(value: JsonValue) -> Result<Self> {
Self::try_from(value)
}

fn serialize_to_map<SM: SerializeMap>(
&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<JsonObject> for Geometry {
Expand Down Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]]}"
);
}

Expand All @@ -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"),
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 9d4b8c5

Please sign in to comment.