From 1e8c8eeaa4d736b1825cf2b8300c874ef39c3f0b Mon Sep 17 00:00:00 2001
From: Hanno Braun <hanno@braun-odw.eu>
Date: Tue, 8 Nov 2022 14:19:27 +0100
Subject: [PATCH] Move validation check to new infrastructure

---
 crates/fj-kernel/src/objects/cycle.rs  | 29 --------
 crates/fj-kernel/src/validate/cycle.rs | 97 +++++++++++++++++++++++++-
 2 files changed, 95 insertions(+), 31 deletions(-)

diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs
index 7a5e12769..412f07b50 100644
--- a/crates/fj-kernel/src/objects/cycle.rs
+++ b/crates/fj-kernel/src/objects/cycle.rs
@@ -2,7 +2,6 @@ use std::slice;
 
 use fj_interop::ext::SliceExt;
 use fj_math::{Scalar, Winding};
-use pretty_assertions::assert_eq;
 
 use crate::{path::SurfacePath, storage::Handle};
 
@@ -35,34 +34,6 @@ impl Cycle {
             "Cycle must contain at least one half-edge"
         );
 
-        if half_edges.len() != 1 {
-            // Verify that all edges connect.
-            for [a, b] in half_edges.as_slice().array_windows_ext() {
-                let [_, prev] = a.vertices();
-                let [next, _] = b.vertices();
-
-                assert_eq!(
-                    prev.surface_form().id(),
-                    next.surface_form().id(),
-                    "Edges in cycle do not connect"
-                );
-            }
-        }
-
-        // Verify that the edges form a cycle
-        if let Some(first) = half_edges.first() {
-            if let Some(last) = half_edges.last() {
-                let [first, _] = first.vertices();
-                let [_, last] = last.vertices();
-
-                assert_eq!(
-                    first.surface_form().id(),
-                    last.surface_form().id(),
-                    "Edges do not form a cycle"
-                );
-            }
-        }
-
         Self { half_edges }
     }
 
diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs
index 83ce06b68..3e528b68c 100644
--- a/crates/fj-kernel/src/validate/cycle.rs
+++ b/crates/fj-kernel/src/validate/cycle.rs
@@ -1,4 +1,9 @@
-use crate::objects::Cycle;
+use itertools::Itertools;
+
+use crate::{
+    objects::{Cycle, SurfaceVertex},
+    storage::Handle,
+};
 
 use super::{Validate2, ValidationConfig};
 
@@ -9,10 +14,98 @@ impl Validate2 for Cycle {
         &self,
         _: &ValidationConfig,
     ) -> Result<(), Self::Error> {
+        CycleValidationError::check_half_edge_connections(self)?;
         Ok(())
     }
 }
 
 /// [`Cycle`] validation error
 #[derive(Debug, thiserror::Error)]
-pub enum CycleValidationError {}
+pub enum CycleValidationError {
+    /// Half-edges are not connected
+    #[error(
+        "`HalfEdge`s of `Cycle` are not connected\n\
+        - Front vertex of previous `HalfEdge`: {prev:#?}\n\
+        - Back vertex of next `HalfEdge`: {next:#?}"
+    )]
+    HalfEdgeConnection {
+        /// The front vertex of the previous half-edge
+        prev: Handle<SurfaceVertex>,
+
+        /// The back vertex of the next half-edge
+        next: Handle<SurfaceVertex>,
+    },
+}
+
+impl CycleValidationError {
+    fn check_half_edge_connections(cycle: &Cycle) -> Result<(), Self> {
+        for (a, b) in cycle.half_edges().circular_tuple_windows() {
+            let [_, prev] = a.vertices();
+            let [next, _] = b.vertices();
+
+            let prev = prev.surface_form();
+            let next = next.surface_form();
+
+            if prev.id() != next.id() {
+                return Err(Self::HalfEdgeConnection {
+                    prev: prev.clone(),
+                    next: next.clone(),
+                });
+            }
+        }
+
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::{
+        builder::{CycleBuilder, HalfEdgeBuilder, VertexBuilder},
+        objects::{Cycle, Objects},
+        partial::HasPartial,
+        validate::Validate2,
+    };
+
+    #[test]
+    fn cycle_half_edge_connections() -> anyhow::Result<()> {
+        let objects = Objects::new();
+
+        let valid = Cycle::partial()
+            .with_poly_chain_from_points(
+                objects.surfaces.xy_plane(),
+                [[0., 0.], [1., 0.], [0., 1.]],
+            )
+            .close_with_line_segment()
+            .build(&objects)?;
+        let invalid = {
+            let mut half_edges = valid
+                .half_edges()
+                .map(|half_edge| half_edge.to_partial())
+                .collect::<Vec<_>>();
+
+            let first_half_edge = &mut half_edges[0];
+            let [first_vertex, _] = first_half_edge.vertices();
+
+            // Sever connection between the last and first half-edge in the
+            // cycle.
+            let first_vertex = first_vertex.into_partial().infer_surface_form();
+            *first_half_edge = first_half_edge
+                .clone()
+                .with_back_vertex(first_vertex)
+                .infer_global_form();
+
+            let half_edges = half_edges
+                .into_iter()
+                .map(|half_edge| half_edge.build(&objects))
+                .collect::<Result<Vec<_>, _>>()?;
+
+            Cycle::new(half_edges)
+        };
+
+        assert!(valid.validate().is_ok());
+        assert!(invalid.validate().is_err());
+
+        Ok(())
+    }
+}