Refactor MVT and GDAL errors (#148)

Relates to #138 --------- Co-authored-by: Yuri Astrakhan <[email protected]>
georust · Jun 22, 2023 · 8f3d8e4 · 8f3d8e4
1 parent 102d999
commit 8f3d8e4
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 36 deletions.
diff --git a/geozero/src/error.rs b/geozero/src/error.rs
@@ -37,6 +37,12 @@ pub enum GeozeroError {
     // General
     #[error("I/O error")]
     IoError(#[from] std::io::Error),
+    #[cfg(feature = "with-mvt")]
+    #[error("MVT error")]
+    MvtError(#[from] crate::mvt::MvtError),
+    #[cfg(feature = "with-gdal")]
+    #[error("GDAL error")]
+    GdalError(#[from] crate::gdal::GdalError),
 }
 
 pub type Result<T> = std::result::Result<T, GeozeroError>;
diff --git a/geozero/src/gdal/gdal_error.rs b/geozero/src/gdal/gdal_error.rs
@@ -0,0 +1,9 @@
+//! GDAL error type.
+use gdal_sys::OGRwkbGeometryType;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum GdalError {
+    #[error("Unsupported geometry type: {0}")]
+    UnsupportedGeometryType(OGRwkbGeometryType::Type),
+}
diff --git a/geozero/src/gdal/gdal_writer.rs b/geozero/src/gdal/gdal_writer.rs
@@ -1,5 +1,7 @@
 use crate::error::{GeozeroError, Result};
+use crate::gdal::gdal_error::GdalError;
 use crate::{CoordDimensions, FeatureProcessor, GeomProcessor, PropertyProcessor};
+
 use gdal::vector::Geometry;
 use gdal_sys::OGRwkbGeometryType;
 
@@ -73,10 +75,8 @@ impl GeomProcessor for GdalWriter {
                 self.line.set_point_2d(idx, (x, y));
             }
             _ => {
-                return Err(GeozeroError::Geometry(format!(
-                    "Unsupported geometry type {}",
-                    self.geom.geometry_type()
-                )))
+                let unsupported_type = self.geom.geometry_type();
+                return Err(GdalError::UnsupportedGeometryType(unsupported_type))?;
             }
         }
         Ok(())
@@ -107,10 +107,8 @@ impl GeomProcessor for GdalWriter {
                 self.line.set_point(idx, (x, y, z));
             }
             _ => {
-                return Err(GeozeroError::Geometry(format!(
-                    "Unsupported geometry type {}",
-                    self.geom.geometry_type()
-                )))
+                let unsupported_type = self.geom.geometry_type();
+                return Err(GdalError::UnsupportedGeometryType(unsupported_type))?;
             }
         }
         Ok(())
@@ -151,11 +149,8 @@ impl GeomProcessor for GdalWriter {
                     let n = poly.geometry_count();
                     self.line = unsafe { poly.get_unowned_geometry(n - 1) };
                 }
-                _ => {
-                    return Err(GeozeroError::Geometry(format!(
-                        "Unsupported geometry type {}",
-                        self.geom.geometry_type()
-                    )))
+                unsupported_type => {
+                    return Err(GdalError::UnsupportedGeometryType(unsupported_type))?;
                 }
             };
         }

diff --git a/geozero/src/gdal/mod.rs b/geozero/src/gdal/mod.rs
@@ -1,7 +1,9 @@
 //! GDAL conversions.
+mod gdal_error;
 pub(crate) mod gdal_reader;
 pub(crate) mod gdal_writer;
 
+pub use gdal_error::GdalError;
 pub use gdal_reader::*;
 pub use gdal_writer::*;
 

diff --git a/geozero/src/mvt/mod.rs b/geozero/src/mvt/mod.rs
@@ -38,6 +38,9 @@ pub(crate) mod conversion {
     }
 }
 
+mod mvt_error;
+pub use mvt_error::MvtError;
+
 #[cfg(feature = "with-wkb")]
 mod wkb {
     use crate::error::Result;

diff --git a/geozero/src/mvt/mvt_error.rs b/geozero/src/mvt/mvt_error.rs
@@ -0,0 +1,18 @@
+//! MVT error type.
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum MvtError {
+    #[error("invalid feature.tags length: {0}")]
+    InvalidFeatureTagsLength(usize),
+    #[error("invalid key index {0}")]
+    InvalidKeyIndex(u32),
+    #[error("invalid value index {0}")]
+    InvalidValueIndex(u32),
+    #[error("unsupported value type for key {0}")]
+    UnsupportedKeyValueType(String),
+    #[error("geometry format")]
+    GeometryFormat,
+    #[error("too few coordinates in line or ring")]
+    TooFewCoordinates,
+}
diff --git a/geozero/src/mvt/mvt_reader.rs b/geozero/src/mvt/mvt_reader.rs
@@ -1,8 +1,11 @@
-use crate::error::{GeozeroError, Result};
+use crate::error::Result;
 use crate::mvt::vector_tile::{tile, tile::GeomType};
 use crate::{ColumnValue, FeatureProcessor, GeomProcessor, GeozeroDatasource, GeozeroGeometry};
 
-use super::mvt_commands::{Command, CommandInteger, ParameterInteger};
+use super::{
+    mvt_commands::{Command, CommandInteger, ParameterInteger},
+    mvt_error::MvtError,
+};
 
 impl GeozeroDatasource for tile::Layer {
     fn process<P: FeatureProcessor>(&mut self, processor: &mut P) -> Result<()> {
@@ -35,19 +38,17 @@ fn process_properties(
     processor.properties_begin()?;
     for (i, pair) in feature.tags.chunks(2).enumerate() {
         let [key_idx, value_idx] = pair else {
-            return Err(GeozeroError::Feature(format!(
-                "invalid feature.tags length: {:?}",
-                feature.tags.len()
-            )));
+            return Err(MvtError::InvalidFeatureTagsLength(
+                feature.tags.len()))?
         };
         let key = layer
             .keys
             .get(*key_idx as usize)
-            .ok_or_else(|| GeozeroError::Feature(format!("invalid key index {key_idx}")))?;
+            .ok_or(MvtError::InvalidKeyIndex(*key_idx))?;
         let value = layer
             .values
             .get(*value_idx as usize)
-            .ok_or_else(|| GeozeroError::Feature(format!("invalid value index {value_idx}")))?;
+            .ok_or(MvtError::InvalidValueIndex(*value_idx))?;
 
         if let Some(ref v) = value.string_value {
             processor.property(i, key, &ColumnValue::String(v))?;
@@ -64,9 +65,7 @@ fn process_properties(
         } else if let Some(v) = value.bool_value {
             processor.property(i, key, &ColumnValue::Bool(v))?;
         } else {
-            return Err(GeozeroError::Property(format!(
-                "unsupported value type for key {key}",
-            )));
+            return Err(MvtError::UnsupportedKeyValueType(key.to_string()))?;
         }
     }
     processor.properties_end()
@@ -155,11 +154,11 @@ fn process_linestring<P: GeomProcessor>(
     processor: &mut P,
 ) -> Result<()> {
     if geom[0] != CommandInteger::from(Command::MoveTo, 1) {
-        return Err(GeozeroError::GeometryFormat);
+        return Err(MvtError::GeometryFormat)?;
     }
     let lineto = CommandInteger(geom[3]);
     if lineto.id() != Command::LineTo as u32 {
-        return Err(GeozeroError::GeometryFormat);
+        return Err(MvtError::GeometryFormat)?;
     }
     processor.linestring_begin(tagged, 1 + lineto.count() as usize, idx)?;
     process_coord(cursor, &geom[1..3], 0, processor)?;
@@ -208,14 +207,14 @@ fn process_polygon<P: GeomProcessor>(
 
     for (i, ring) in rings.iter().enumerate() {
         if ring[0] != CommandInteger::from(Command::MoveTo, 1) {
-            return Err(GeozeroError::GeometryFormat);
+            return Err(MvtError::GeometryFormat)?;
         }
         if *ring.last().unwrap() != CommandInteger::from(Command::ClosePath, 1) {
-            return Err(GeozeroError::GeometryFormat);
+            return Err(MvtError::GeometryFormat)?;
         }
         let lineto = CommandInteger(ring[3]);
         if lineto.id() != Command::LineTo as u32 {
-            return Err(GeozeroError::GeometryFormat);
+            return Err(MvtError::GeometryFormat)?;
         }
         processor.linestring_begin(false, 1 + lineto.count() as usize, i)?;
         let mut start_cursor = *cursor;
@@ -260,7 +259,7 @@ fn process_polygons<P: GeomProcessor>(
             // add interior ring to previous polygon
             last_slice.push(slice);
         } else {
-            return Err(GeozeroError::GeometryFormat);
+            return Err(MvtError::GeometryFormat)?;
         }
         geom = rest;
     }

diff --git a/geozero/src/mvt/mvt_writer.rs b/geozero/src/mvt/mvt_writer.rs
@@ -1,11 +1,13 @@
 //! Encode geometries according to MVT spec
 //! <https://github.com/mapbox/vector-tile-spec/tree/master/2.1>
 
-use crate::error::{GeozeroError, Result};
+use crate::error::Result;
 use crate::mvt::mvt_commands::{Command, CommandInteger, ParameterInteger};
 use crate::mvt::vector_tile::{tile, tile::GeomType};
 use crate::GeomProcessor;
 
+use super::mvt_error::MvtError;
+
 /// Generator for MVT geometry type.
 pub struct MvtWriter {
     pub(crate) feature: tile::Feature,
@@ -81,11 +83,7 @@ impl GeomProcessor for MvtWriter {
             let num_coords = match self.line_state {
                 LineState::Line(size) if size > 1 => size - 1,
                 LineState::Ring(size) if size > 2 => size - 2,
-                _ => {
-                    return Err(GeozeroError::Geometry(
-                        "Too few coordinates in line or ring".to_string(),
-                    ))
-                }
+                _ => return Err(MvtError::TooFewCoordinates)?,
             };
             self.feature
                 .geometry