diff --git a/CHANGES.md b/CHANGES.md index 57b53d5..cb6feb0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ * * Add `TryFrom<&geometry::Value>` for geo_type variants. * +* Changed the format of the error produced when converting a geometry to an incompatible type - e.g. a LineString into a Point. + * ## 0.23.0 diff --git a/src/conversion/mod.rs b/src/conversion/mod.rs index cf853d3..3cd583d 100644 --- a/src/conversion/mod.rs +++ b/src/conversion/mod.rs @@ -63,12 +63,10 @@ macro_rules! assert_almost_eq { }}; } - macro_rules! try_from_owned_value { ($to:ty) => { #[cfg_attr(docsrs, doc(cfg(feature = "geo-types")))] - impl TryFrom for $to - { + impl TryFrom for $to { type Error = Error; fn try_from(value: geometry::Value) -> Result { @@ -78,7 +76,6 @@ macro_rules! try_from_owned_value { }; } - pub(crate) mod from_geo_types; pub(crate) mod to_geo_types; diff --git a/src/conversion/to_geo_types.rs b/src/conversion/to_geo_types.rs index 35d8727..414a74d 100644 --- a/src/conversion/to_geo_types.rs +++ b/src/conversion/to_geo_types.rs @@ -19,7 +19,7 @@ where fn try_from(value: &geometry::Value) -> Result { match value { geometry::Value::Point(point_type) => Ok(create_geo_point(&point_type)), - _ => Err(Error::InvalidGeometryConversion(value.clone())), + other => Err(mismatch_geom_err("Point", &other)), } } } @@ -40,7 +40,7 @@ where .map(|point_type| create_geo_point(&point_type)) .collect(), )), - _ => Err(Error::InvalidGeometryConversion(value.clone())), + other => Err(mismatch_geom_err("MultiPoint", &other)), } } } @@ -58,7 +58,7 @@ where geometry::Value::LineString(multi_point_type) => { Ok(create_geo_line_string(&multi_point_type)) } - _ => Err(Error::InvalidGeometryConversion(value.clone())), + other => Err(mismatch_geom_err("LineString", &other)), } } } @@ -76,7 +76,7 @@ where geometry::Value::MultiLineString(multi_line_string_type) => { Ok(create_geo_multi_line_string(&multi_line_string_type)) } - _ => Err(Error::InvalidGeometryConversion(value.clone())), + other => Err(mismatch_geom_err("MultiLineString", &other)), } } } @@ -92,7 +92,7 @@ where fn try_from(value: &geometry::Value) -> Result { match value { geometry::Value::Polygon(polygon_type) => Ok(create_geo_polygon(&polygon_type)), - _ => Err(Error::InvalidGeometryConversion(value.clone())), + other => Err(mismatch_geom_err("Polygon", &other)), } } } @@ -110,13 +110,12 @@ where geometry::Value::MultiPolygon(multi_polygon_type) => { Ok(create_geo_multi_polygon(&multi_polygon_type)) } - _ => Err(Error::InvalidGeometryConversion(value.clone())), + other => Err(mismatch_geom_err("MultiPolygon", other)), } } } try_from_owned_value!(geo_types::MultiPolygon); - #[cfg_attr(docsrs, doc(cfg(feature = "geo-types")))] impl TryFrom<&geometry::Value> for geo_types::GeometryCollection where @@ -134,14 +133,13 @@ where Ok(geo_types::GeometryCollection(geojson_geometries)) } - _ => Err(Error::InvalidGeometryConversion(value.clone())), + other => Err(mismatch_geom_err("GeometryCollection", other)), } } } try_from_owned_value!(geo_types::GeometryCollection); - #[cfg_attr(docsrs, doc(cfg(feature = "geo-types")))] impl TryFrom<&geometry::Value> for geo_types::Geometry where @@ -339,6 +337,13 @@ where ) } +fn mismatch_geom_err(expected_type: &'static str, found: &geometry::Value) -> Error { + Error::InvalidGeometryConversion { + expected_type, + found_type: found.type_name(), + } +} + #[cfg(test)] mod tests { use crate::{Geometry, Value}; @@ -594,6 +599,19 @@ mod tests { assert_almost_eq!(geo_point.y(), coords[1], 1e-6); } + #[test] + fn geojson_mismatch_geometry_conversion_test() { + let coord1 = vec![100.0, 0.2]; + let coord2 = vec![101.0, 1.0]; + let geojson_line_string = Value::LineString(vec![coord1.clone(), coord2.clone()]); + use std::convert::TryFrom; + let error = geo_types::Point::::try_from(geojson_line_string).unwrap_err(); + assert_eq!( + "Expected type: `Point`, but found `LineString`", + format!("{}", error) + ) + } + #[test] fn feature_collection_with_geom_collection() { let geojson_str = json!({ @@ -683,7 +701,7 @@ mod tests { let geojson_polygon = Value::Polygon(geojson_multi_line_string_type1); let _: geo_types::Polygon = (&geojson_polygon).try_into()?; - let geojson_line_string_type1 = vec![ + let geojson_line_string_type1 = vec![ coord1.clone(), coord2.clone(), coord3.clone(), @@ -700,7 +718,7 @@ mod tests { vec![geojson_line_string_type1], vec![geojson_line_string_type2], ]); - let _: geo_types::MultiPolygon = (&geojson_multi_polygon).try_into()?; + let _: geo_types::MultiPolygon = (&geojson_multi_polygon).try_into()?; let geojson_geometry_collection = Value::GeometryCollection(vec![ Geometry::new(geojson_multi_point), diff --git a/src/errors.rs b/src/errors.rs index 52fc50e..9c3bbb3 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,5 +1,4 @@ //! Module for all GeoJSON-related errors -use crate::geometry::Value as GValue; use crate::Feature; use serde_json::value::Value; use thiserror::Error; @@ -21,8 +20,11 @@ pub enum Error { /// This was previously `GeoJsonUnknownType`, but has been split for clarity #[error("Expected a Feature mapping, but got a `{0}`")] NotAFeature(String), - #[error("Encountered a mismatch when converting to a Geo type: `{0}`")] - InvalidGeometryConversion(GValue), + #[error("Expected type: `{expected_type}`, but found `{found_type}`")] + InvalidGeometryConversion { + expected_type: &'static str, + found_type: &'static str, + }, #[error( "Attempted to a convert a feature without a geometry into a geo_types::Geometry: `{0}`" )] diff --git a/src/geometry.rs b/src/geometry.rs index c53740e..6d134ed 100644 --- a/src/geometry.rs +++ b/src/geometry.rs @@ -83,10 +83,9 @@ pub enum Value { GeometryCollection(Vec), } -impl<'a> From<&'a Value> for JsonObject { - fn from(value: &'a Value) -> JsonObject { - let mut map = JsonObject::new(); - let ty = String::from(match value { +impl Value { + pub fn type_name(&self) -> &'static str { + match self { Value::Point(..) => "Point", Value::MultiPoint(..) => "MultiPoint", Value::LineString(..) => "LineString", @@ -94,10 +93,17 @@ impl<'a> From<&'a Value> for JsonObject { Value::Polygon(..) => "Polygon", Value::MultiPolygon(..) => "MultiPolygon", Value::GeometryCollection(..) => "GeometryCollection", - }); - - map.insert(String::from("type"), ::serde_json::to_value(&ty).unwrap()); + } + } +} +impl<'a> From<&'a Value> for JsonObject { + fn from(value: &'a Value) -> JsonObject { + let mut map = JsonObject::new(); + map.insert( + String::from("type"), + ::serde_json::to_value(value.type_name()).unwrap(), + ); map.insert( String::from(match value { Value::GeometryCollection(..) => "geometries",