Skip to content

Commit

Permalink
DRY up validation logic.
Browse files Browse the repository at this point in the history
Previously the `is_valid` vs `explain_invalidity` methods were mostly a
cut/paste job.  `explain_invalidity` gathered all the errors, while
`is_valid` short-circuited to return early as soon as it new a geometry
was invalid.

Now both methods share a code path, in the new `visit_validation` method.

And there's another new method: `check_validation` which, because it
returns a result can be used nicely in error handling:

```
polygon.check_validation()?;
```

This also leverages generics (associated types) to return specific error
cases per geometry, which replaces some runtime `unreachable` checks
with compile time type checks.
  • Loading branch information
michaelkirk committed Dec 10, 2024
1 parent e028661 commit b89e715
Show file tree
Hide file tree
Showing 15 changed files with 937 additions and 1,251 deletions.
2 changes: 1 addition & 1 deletion geo/src/algorithm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,5 +304,5 @@ pub mod rhumb;
#[allow(deprecated)]
pub use rhumb::{RhumbBearing, RhumbDestination, RhumbDistance, RhumbIntermediate, RhumbLength};

mod validation;
pub mod validation;
pub use validation::Validation;
42 changes: 23 additions & 19 deletions geo/src/algorithm/validation/coord.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,32 @@
use super::{utils, Problem, ProblemAtPosition, ProblemPosition, ProblemReport, Validation};
use super::{utils, Validation};
use crate::{Coord, GeoFloat};

impl<F: GeoFloat> Validation for Coord<F> {
fn is_valid(&self) -> bool {
if utils::check_coord_is_not_finite(self) {
return false;
use std::fmt;

#[derive(Debug, Clone, PartialEq)]
pub enum InvalidCoord {
/// A valid [`Coord`] must be finite.
NonFinite,
}

impl fmt::Display for InvalidCoord {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
InvalidCoord::NonFinite => write!(f, "coordinite was non-finite"),
}
true
}
fn explain_invalidity(&self) -> Option<ProblemReport> {
let mut reason = Vec::new();
}

if utils::check_coord_is_not_finite(self) {
reason.push(ProblemAtPosition(
Problem::NotFinite,
ProblemPosition::Point,
));
}
impl<F: GeoFloat> Validation for Coord<F> {
type Error = InvalidCoord;

// Return the reason(s) of invalidity, or None if valid
if reason.is_empty() {
None
} else {
Some(ProblemReport(reason))
fn visit_validation<T>(
&self,
mut handle_validation_error: Box<dyn FnMut(Self::Error) -> Result<(), T> + '_>,
) -> Result<(), T> {
if utils::check_coord_is_not_finite(self) {
handle_validation_error(InvalidCoord::NonFinite)?;
}
Ok(())
}
}
100 changes: 76 additions & 24 deletions geo/src/algorithm/validation/geometry.rs
Original file line number Diff line number Diff line change
@@ -1,33 +1,85 @@
use super::{ProblemReport, Validation};
use super::Validation;
use super::{
InvalidGeometryCollection, InvalidLine, InvalidLineString, InvalidMultiLineString,
InvalidMultiPoint, InvalidMultiPolygon, InvalidPoint, InvalidPolygon, InvalidRect,
InvalidTriangle,
};
use crate::{GeoFloat, Geometry};

impl<F: GeoFloat> Validation for Geometry<F> {
fn is_valid(&self) -> bool {
use std::fmt;

/// A [`Geometry`] is valid if it's inner variant is valid.
/// e.g. `Geometry::Polygon(polygon)` is valid if and only if `polygon` is valid.
#[derive(Debug, Clone, PartialEq)]
pub enum InvalidGeometry {
InvalidPoint(InvalidPoint),
InvalidLine(InvalidLine),
InvalidLineString(InvalidLineString),
InvalidPolygon(InvalidPolygon),
InvalidMultiPoint(InvalidMultiPoint),
InvalidMultiLineString(InvalidMultiLineString),
InvalidMultiPolygon(InvalidMultiPolygon),
InvalidGeometryCollection(InvalidGeometryCollection),
InvalidRect(InvalidRect),
InvalidTriangle(InvalidTriangle),
}

impl fmt::Display for InvalidGeometry {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Geometry::Point(e) => e.is_valid(),
Geometry::Line(e) => e.is_valid(),
Geometry::Rect(e) => e.is_valid(),
Geometry::Triangle(e) => e.is_valid(),
Geometry::LineString(e) => e.is_valid(),
Geometry::Polygon(e) => e.is_valid(),
Geometry::MultiPoint(e) => e.is_valid(),
Geometry::MultiLineString(e) => e.is_valid(),
Geometry::MultiPolygon(e) => e.is_valid(),
Geometry::GeometryCollection(e) => e.is_valid(),
InvalidGeometry::InvalidPoint(err) => write!(f, "{}", err),
InvalidGeometry::InvalidLine(err) => write!(f, "{}", err),
InvalidGeometry::InvalidLineString(err) => write!(f, "{}", err),
InvalidGeometry::InvalidPolygon(err) => write!(f, "{}", err),
InvalidGeometry::InvalidMultiPoint(err) => write!(f, "{}", err),
InvalidGeometry::InvalidMultiLineString(err) => write!(f, "{}", err),
InvalidGeometry::InvalidMultiPolygon(err) => write!(f, "{}", err),
InvalidGeometry::InvalidGeometryCollection(err) => write!(f, "{}", err),
InvalidGeometry::InvalidRect(err) => write!(f, "{}", err),
InvalidGeometry::InvalidTriangle(err) => write!(f, "{}", err),
}
}
fn explain_invalidity(&self) -> Option<ProblemReport> {
}

impl<F: GeoFloat> Validation for Geometry<F> {
type Error = InvalidGeometry;

fn visit_validation<T>(
&self,
mut handle_validation_error: Box<dyn FnMut(Self::Error) -> Result<(), T> + '_>,
) -> Result<(), T> {
match self {
Geometry::Point(e) => e.explain_invalidity(),
Geometry::Line(e) => e.explain_invalidity(),
Geometry::Rect(e) => e.explain_invalidity(),
Geometry::Triangle(e) => e.explain_invalidity(),
Geometry::LineString(e) => e.explain_invalidity(),
Geometry::Polygon(e) => e.explain_invalidity(),
Geometry::MultiPoint(e) => e.explain_invalidity(),
Geometry::MultiLineString(e) => e.explain_invalidity(),
Geometry::MultiPolygon(e) => e.explain_invalidity(),
Geometry::GeometryCollection(e) => e.explain_invalidity(),
Geometry::Point(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidPoint(err))
}))?,
Geometry::Line(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidLine(err))
}))?,
Geometry::LineString(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidLineString(err))
}))?,
Geometry::Polygon(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidPolygon(err))
}))?,
Geometry::MultiPoint(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidMultiPoint(err))
}))?,
Geometry::MultiLineString(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidMultiLineString(err))
}))?,
Geometry::MultiPolygon(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidMultiPolygon(err))
}))?,
Geometry::GeometryCollection(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidGeometryCollection(err))
}))?,
Geometry::Rect(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidRect(err))
}))?,
Geometry::Triangle(g) => g.visit_validation(Box::new(|err| {
handle_validation_error(InvalidGeometry::InvalidTriangle(err))
}))?,
}
Ok(())
}
}
148 changes: 84 additions & 64 deletions geo/src/algorithm/validation/geometry_collection.rs
Original file line number Diff line number Diff line change
@@ -1,94 +1,114 @@
use super::{GeometryPosition, ProblemAtPosition, ProblemPosition, ProblemReport, Validation};
use super::{GeometryIndex, InvalidGeometry, Validation};
use crate::{GeoFloat, GeometryCollection};

/// GeometryCollection is valid if all its elements are valid
impl<F: GeoFloat> Validation for GeometryCollection<F> {
fn is_valid(&self) -> bool {
for geometry in self.0.iter() {
if !geometry.is_valid() {
return false;
use std::fmt;

/// A [`GeometryCollection`] is valid if all its elements are valid.
#[derive(Debug, Clone, PartialEq)]
pub enum InvalidGeometryCollection {
/// Which element is invalid, and what was invalid about it.
InvalidGeometry(GeometryIndex, Box<InvalidGeometry>),
}

impl fmt::Display for InvalidGeometryCollection {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
InvalidGeometryCollection::InvalidGeometry(idx, err) => {
write!(f, "geometry at index {} is invalid: {}", idx.0, err)
}
}
true
}
fn explain_invalidity(&self) -> Option<ProblemReport> {
let mut reason = Vec::new();
}

impl<F: GeoFloat> Validation for GeometryCollection<F> {
type Error = InvalidGeometryCollection;

fn visit_validation<T>(
&self,
mut handle_validation_error: Box<dyn FnMut(Self::Error) -> Result<(), T> + '_>,
) -> Result<(), T> {
// Loop over all the geometries, collect the reasons of invalidity
// and change the ProblemPosition to reflect the GeometryCollection
for (i, geometry) in self.0.iter().enumerate() {
let temp_reason = geometry.explain_invalidity();
if let Some(temp_reason) = temp_reason {
for ProblemAtPosition(problem, position) in temp_reason.0 {
reason.push(ProblemAtPosition(
problem,
ProblemPosition::GeometryCollection(
GeometryPosition(i),
Box::new(position),
),
));
}
}
}
// Return the reason(s) of invalidity, or None if valid
if reason.is_empty() {
None
} else {
Some(ProblemReport(reason))
geometry.visit_validation(Box::new(&mut |geometry_err| {
let err = InvalidGeometryCollection::InvalidGeometry(
GeometryIndex(i),
Box::new(geometry_err),
);
handle_validation_error(err)
}))?;
}
Ok(())
}
}

#[cfg(test)]
mod tests {
use super::super::{
CoordinatePosition, GeometryPosition, Problem, ProblemAtPosition, ProblemPosition,
ProblemReport, Validation,
};
use crate::{Coord, Geometry, GeometryCollection, LineString, Point};
use super::*;
use crate::algorithm::validation::{assert_validation_errors, InvalidLineString};
use crate::{geometry::*, wkt};

use geos::Geom;

#[test]
fn test_geometrycollection_contain_invalid_element() {
let gc = GeometryCollection(vec![
Geometry::Point(Point::new(0., 0.)),
Geometry::LineString(LineString(vec![
Coord { x: 0., y: 0. },
Coord { x: 1., y: 1. },
])),
Geometry::LineString(LineString(vec![
Coord { x: 0., y: 0. },
Coord { x: 0., y: 0. },
])),
]);
assert!(!gc.is_valid());
assert_eq!(
gc.explain_invalidity(),
Some(ProblemReport(vec![ProblemAtPosition(
Problem::TooFewPoints,
ProblemPosition::GeometryCollection(
GeometryPosition(2),
Box::new(ProblemPosition::LineString(CoordinatePosition(0)))
)
)]))
let gc = wkt!(
GEOMETRYCOLLECTION(
POINT(0. 0.),
LINESTRING(0. 0.,1. 1.),
LINESTRING(0. 0.,0. 0.)
)
);
assert_validation_errors!(
gc,
vec![InvalidGeometryCollection::InvalidGeometry(
GeometryIndex(2),
Box::new(InvalidGeometry::InvalidLineString(
InvalidLineString::TooFewPoints
)),
)]
);

let geoms =
gc.0.iter()
.map(|geometry| match geometry {
Geometry::Point(pt) => {
let geos_point: geos::Geometry = pt.try_into().unwrap();
geos_point
}
Geometry::LineString(ls) => {
let geos_linestring: geos::Geometry = ls.try_into().unwrap();
geos_linestring
}
Geometry::Point(pt) => pt.try_into().unwrap(),
Geometry::LineString(ls) => ls.try_into().unwrap(),
_ => unreachable!(),
})
.collect::<Vec<_>>();
.collect::<Vec<geos::Geometry>>();

let geometrycollection_geos: geos::Geometry =
geos::Geometry::create_geometry_collection(geoms).unwrap();
assert_eq!(gc.is_valid(), geometrycollection_geos.is_valid());
}

#[test]
fn test_display() {
let gc = wkt!(
GEOMETRYCOLLECTION(
POINT(0. 0.),
LINESTRING(0. 0.,1. 1.),
LINESTRING(0. 0.,0. 0.),
POLYGON(
(0. 0., 1. 1., 1. 0., 0. 0.),
(0. 0., 1. 1., 1. 0., 0. 0.)
)
)
);
let errors = gc.validation_errors();
assert_eq!(
errors[0].to_string(),
"geometry at index 2 is invalid: line string must have at least 2 distinct points"
);

assert_eq!(
errors[1].to_string(),
"geometry at index 3 is invalid: interior ring at index 0 is not contained within the polygon's exterior"
);

assert_eq!(
errors[2].to_string(),
"geometry at index 3 is invalid: exterior ring and interior ring at index 0 intersect on a line"
);
}
}
Loading

0 comments on commit b89e715

Please sign in to comment.