diff --git a/problemreductions-cli/src/commands/create/tests.rs b/problemreductions-cli/src/commands/create/tests.rs index 79294e200..e6babc485 100644 --- a/problemreductions-cli/src/commands/create/tests.rs +++ b/problemreductions-cli/src/commands/create/tests.rs @@ -1251,7 +1251,7 @@ fn test_create_production_planning_rejects_mismatched_period_lengths() { let err = create(&args, &out).unwrap_err(); assert!(err .to_string() - .contains("demands has 5 entries, expected 6")); + .contains("all per-period vectors must have length num_periods")); } #[test] diff --git a/src/models/graph/acyclic_partition.rs b/src/models/graph/acyclic_partition.rs index 2ed924353..05ed0ff6c 100644 --- a/src/models/graph/acyclic_partition.rs +++ b/src/models/graph/acyclic_partition.rs @@ -29,7 +29,7 @@ inventory::submit! { } /// Acyclic Partition (Garey & Johnson ND15). -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize)] pub struct AcyclicPartition { graph: DirectedGraph, vertex_weights: Vec, @@ -38,6 +38,34 @@ pub struct AcyclicPartition { cost_bound: W::Sum, } +#[derive(Deserialize)] +#[serde(bound(deserialize = "W: WeightElement + Deserialize<'de>, W::Sum: Deserialize<'de>"))] +struct AcyclicPartitionData { + graph: DirectedGraph, + vertex_weights: Vec, + arc_costs: Vec, + weight_bound: W::Sum, + cost_bound: W::Sum, +} + +impl<'de, W> Deserialize<'de> for AcyclicPartition +where + W: WeightElement + Deserialize<'de>, + W::Sum: Deserialize<'de>, +{ + fn deserialize>(deserializer: D) -> Result { + let data = AcyclicPartitionData::::deserialize(deserializer)?; + Self::try_new( + data.graph, + data.vertex_weights, + data.arc_costs, + data.weight_bound, + data.cost_bound, + ) + .map_err(serde::de::Error::custom) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct AcyclicPartitionCreateSpec { #[create(codec = "arc-list")] @@ -76,31 +104,16 @@ impl TryFrom for AcyclicPartition { } let graph = DirectedGraph::new(num_vertices, spec.arcs); let vertex_weights = spec.weights.unwrap_or_else(|| vec![1; num_vertices]); - if vertex_weights.len() != num_vertices { - return Err(format!( - "weights has length {}, expected {num_vertices}", - vertex_weights.len() - ) - .into()); - } let arc_costs = spec .arc_weights .unwrap_or_else(|| vec![1; graph.num_arcs()]); - if arc_costs.len() != graph.num_arcs() { - return Err(format!( - "arc_weights has length {}, expected {}", - arc_costs.len(), - graph.num_arcs() - ) - .into()); - } - Ok(Self::new( + Self::try_new( graph, vertex_weights, arc_costs, spec.weight_bound, spec.cost_bound, - )) + ) } } @@ -113,23 +126,26 @@ impl AcyclicPartition { weight_bound: W::Sum, cost_bound: W::Sum, ) -> Self { - assert_eq!( - vertex_weights.len(), - graph.num_vertices(), - "vertex_weights length must match graph num_vertices" - ); - assert_eq!( - arc_costs.len(), - graph.num_arcs(), - "arc_costs length must match graph num_arcs" - ); - Self { + Self::try_new(graph, vertex_weights, arc_costs, weight_bound, cost_bound) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: DirectedGraph, + vertex_weights: Vec, + arc_costs: Vec, + weight_bound: W::Sum, + cost_bound: W::Sum, + ) -> Result { + Self::check_vertex_weights(&graph, &vertex_weights)?; + Self::check_arc_costs(&graph, &arc_costs)?; + Ok(Self { graph, vertex_weights, arc_costs, weight_bound, cost_bound, - } + }) } /// Get the underlying graph. @@ -149,24 +165,37 @@ impl AcyclicPartition { /// Replace the vertex weights. pub fn set_vertex_weights(&mut self, vertex_weights: Vec) { - assert_eq!( - vertex_weights.len(), - self.graph.num_vertices(), - "vertex_weights length must match graph num_vertices" - ); + Self::check_vertex_weights(&self.graph, &vertex_weights) + .unwrap_or_else(|error| panic!("{error}")); self.vertex_weights = vertex_weights; } + fn check_vertex_weights( + graph: &DirectedGraph, + vertex_weights: &[W], + ) -> Result<(), crate::registry::ConstructionError> { + if vertex_weights.len() != graph.num_vertices() { + return Err("vertex_weights length must match graph num_vertices".into()); + } + Ok(()) + } + /// Replace the arc costs. pub fn set_arc_costs(&mut self, arc_costs: Vec) { - assert_eq!( - arc_costs.len(), - self.graph.num_arcs(), - "arc_costs length must match graph num_arcs" - ); + Self::check_arc_costs(&self.graph, &arc_costs).unwrap_or_else(|error| panic!("{error}")); self.arc_costs = arc_costs; } + fn check_arc_costs( + graph: &DirectedGraph, + arc_costs: &[W], + ) -> Result<(), crate::registry::ConstructionError> { + if arc_costs.len() != graph.num_arcs() { + return Err("arc_costs length must match graph num_arcs".into()); + } + Ok(()) + } + /// Get the per-part weight bound. pub fn weight_bound(&self) -> &W::Sum { &self.weight_bound diff --git a/src/models/graph/bottleneck_traveling_salesman.rs b/src/models/graph/bottleneck_traveling_salesman.rs index fd5aaac2c..3f3ff671d 100644 --- a/src/models/graph/bottleneck_traveling_salesman.rs +++ b/src/models/graph/bottleneck_traveling_salesman.rs @@ -24,11 +24,25 @@ inventory::submit! { /// The Bottleneck Traveling Salesman problem on a simple weighted graph. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "BottleneckTravelingSalesmanData")] pub struct BottleneckTravelingSalesman { graph: SimpleGraph, edge_weights: Vec, } +#[derive(Deserialize)] +struct BottleneckTravelingSalesmanData { + graph: SimpleGraph, + edge_weights: Vec, +} + +impl TryFrom for BottleneckTravelingSalesman { + type Error = crate::registry::ConstructionError; + fn try_from(data: BottleneckTravelingSalesmanData) -> Result { + Self::try_new(data.graph, data.edge_weights) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct BottleneckTravelingSalesmanCreateSpec { #[create(codec = "edge-list")] @@ -46,15 +60,7 @@ impl TryFrom for BottleneckTravelingSales let edge_weights = spec .edge_weights .unwrap_or_else(|| vec![1; graph.num_edges()]); - if edge_weights.len() != graph.num_edges() { - return Err(format!( - "edge_weights has length {}, expected {}", - edge_weights.len(), - graph.num_edges() - ) - .into()); - } - Ok(Self::new(graph, edge_weights)) + Self::try_new(graph, edge_weights) } } @@ -92,15 +98,18 @@ fn simple_graph_from_create( impl BottleneckTravelingSalesman { /// Create a BottleneckTravelingSalesman problem from a graph with edge weights. pub fn new(graph: SimpleGraph, edge_weights: Vec) -> Self { - assert_eq!( - edge_weights.len(), - graph.num_edges(), - "edge_weights length must match num_edges" - ); - Self { + Self::try_new(graph, edge_weights).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: SimpleGraph, + edge_weights: Vec, + ) -> Result { + Self::check_weights(&graph, &edge_weights)?; + Ok(Self { graph, edge_weights, - } + }) } /// Get a reference to the underlying graph. @@ -115,9 +124,18 @@ impl BottleneckTravelingSalesman { /// Set new weights for the problem. pub fn set_weights(&mut self, weights: Vec) { - assert_eq!(weights.len(), self.graph.num_edges()); + Self::check_weights(&self.graph, &weights).unwrap_or_else(|error| panic!("{error}")); self.edge_weights = weights; } + fn check_weights( + graph: &SimpleGraph, + weights: &[i64], + ) -> Result<(), crate::registry::ConstructionError> { + if weights.len() != graph.num_edges() { + return Err("edge_weights length must match num_edges".into()); + } + Ok(()) + } /// Get all edges with their weights. pub fn edges(&self) -> Vec<(usize, usize, i64)> { diff --git a/src/models/graph/kth_best_spanning_tree.rs b/src/models/graph/kth_best_spanning_tree.rs index e841da212..1fb3b795d 100644 --- a/src/models/graph/kth_best_spanning_tree.rs +++ b/src/models/graph/kth_best_spanning_tree.rs @@ -34,7 +34,7 @@ inventory::submit! { /// /// A configuration is `k` consecutive binary blocks of length `|E|`. /// Each block selects the edges of one candidate spanning tree. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize)] pub struct KthBestSpanningTree { graph: SimpleGraph, weights: Vec, @@ -42,6 +42,27 @@ pub struct KthBestSpanningTree { bound: W::Sum, } +#[derive(Deserialize)] +#[serde(bound(deserialize = "W: WeightElement + Deserialize<'de>, W::Sum: Deserialize<'de>"))] +struct KthBestSpanningTreeData { + graph: SimpleGraph, + weights: Vec, + k: usize, + bound: W::Sum, +} + +impl<'de, W> Deserialize<'de> for KthBestSpanningTree +where + W: WeightElement + Deserialize<'de>, + W::Sum: Deserialize<'de>, +{ + fn deserialize>(deserializer: D) -> Result { + let data = KthBestSpanningTreeData::::deserialize(deserializer)?; + Self::try_new(data.graph, data.weights, data.k, data.bound) + .map_err(serde::de::Error::custom) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct KthBestSpanningTreeCreateSpec { #[create(codec = "edge-list")] @@ -61,18 +82,7 @@ impl TryFrom for KthBestSpanningTree { let weights = spec .edge_weights .unwrap_or_else(|| vec![1; graph.num_edges()]); - if weights.len() != graph.num_edges() { - return Err(format!( - "edge_weights has length {}, expected {}", - weights.len(), - graph.num_edges() - ) - .into()); - } - if spec.k == 0 { - return Err("k must be positive".to_string().into()); - } - Ok(Self::new(graph, weights, spec.k, spec.bound)) + Self::try_new(graph, weights, spec.k, spec.bound) } } @@ -112,19 +122,28 @@ impl KthBestSpanningTree { /// Panics if the number of weights does not match the number of edges, or /// if `k` is zero. pub fn new(graph: SimpleGraph, weights: Vec, k: usize, bound: W::Sum) -> Self { - assert_eq!( - weights.len(), - graph.num_edges(), - "weights length must match graph num_edges" - ); - assert!(k > 0, "k must be positive"); - - Self { + Self::try_new(graph, weights, k, bound).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: SimpleGraph, + weights: Vec, + k: usize, + bound: W::Sum, + ) -> Result { + if weights.len() != graph.num_edges() { + return Err("weights length must match graph num_edges".into()); + } + if k == 0 { + return Err("k must be positive".into()); + } + + Ok(Self { graph, weights, k, bound, - } + }) } /// Get the underlying graph. diff --git a/src/models/graph/maximum_common_edge_subgraph.rs b/src/models/graph/maximum_common_edge_subgraph.rs index 179151b64..39a70bafa 100644 --- a/src/models/graph/maximum_common_edge_subgraph.rs +++ b/src/models/graph/maximum_common_edge_subgraph.rs @@ -69,6 +69,7 @@ impl LabelledArc { /// vector and treated as a set (duplicates are deduplicated by the /// constructor). #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(try_from = "LabelledDigraphData")] pub struct LabelledDigraph { /// Number of vertices `|V|`. pub num_vertices: usize, @@ -76,25 +77,47 @@ pub struct LabelledDigraph { pub arcs: Vec, } +#[derive(Deserialize)] +struct LabelledDigraphData { + num_vertices: usize, + arcs: Vec, +} + +impl TryFrom for LabelledDigraph { + type Error = crate::registry::ConstructionError; + fn try_from(data: LabelledDigraphData) -> Result { + Self::try_new(data.num_vertices, data.arcs) + } +} + impl LabelledDigraph { /// Construct a new labelled digraph. /// /// # Panics /// Panics if any arc references a vertex index outside `0..num_vertices`. pub fn new(num_vertices: usize, arcs: Vec) -> Self { + Self::try_new(num_vertices, arcs).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + arcs: Vec, + ) -> Result { for arc in &arcs { - assert!( - arc.src < num_vertices, - "labelled arc source {} out of range for num_vertices = {}", - arc.src, - num_vertices - ); - assert!( - arc.dst < num_vertices, - "labelled arc destination {} out of range for num_vertices = {}", - arc.dst, - num_vertices - ); + if arc.src >= num_vertices { + return Err(format!( + "labelled arc source {} out of range for num_vertices = {}", + arc.src, num_vertices + ) + .into()); + } + if arc.dst >= num_vertices { + return Err(format!( + "labelled arc destination {} out of range for num_vertices = {}", + arc.dst, num_vertices + ) + .into()); + } } // Deduplicate while preserving order so set semantics hold. let mut seen = std::collections::HashSet::new(); @@ -104,10 +127,10 @@ impl LabelledDigraph { deduped.push(arc); } } - Self { + Ok(Self { num_vertices, arcs: deduped, - } + }) } /// Number of vertices `|V|`. diff --git a/src/models/graph/maximum_contact_map_overlap.rs b/src/models/graph/maximum_contact_map_overlap.rs index c7f340220..ae22629b7 100644 --- a/src/models/graph/maximum_contact_map_overlap.rs +++ b/src/models/graph/maximum_contact_map_overlap.rs @@ -70,6 +70,7 @@ inventory::submit! { /// entries are pairwise distinct (injectivity) and strictly increasing along /// the index order of `V_1` (order-preserving). #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(try_from = "MaximumContactMapOverlapData")] pub struct MaximumContactMapOverlap { num_vertices_1: usize, contacts_1: Vec<(usize, usize)>, @@ -77,51 +78,91 @@ pub struct MaximumContactMapOverlap { contacts_2: Vec<(usize, usize)>, } -/// Canonicalize a contact set: each pair is normalized to `(min, max)`, no -/// self-loops are allowed, all endpoints must be in range, and duplicates -/// (after normalization) cause a panic. +#[derive(Deserialize)] +struct MaximumContactMapOverlapData { + num_vertices_1: usize, + contacts_1: Vec<(usize, usize)>, + num_vertices_2: usize, + contacts_2: Vec<(usize, usize)>, +} + +impl TryFrom for MaximumContactMapOverlap { + type Error = crate::registry::ConstructionError; + fn try_from(data: MaximumContactMapOverlapData) -> Result { + Self::try_new( + data.num_vertices_1, + data.contacts_1, + data.num_vertices_2, + data.contacts_2, + ) + } +} + +/// Canonicalize a contact set: each pair is normalized to `(min, max)`. +/// Self-loops, out-of-range endpoints, and duplicates (after normalization) +/// are rejected. fn canonicalize_contacts( raw: Vec<(usize, usize)>, num_vertices: usize, side: &str, -) -> Vec<(usize, usize)> { +) -> Result, crate::registry::ConstructionError> { let mut seen: HashSet<(usize, usize)> = HashSet::new(); let mut out = Vec::with_capacity(raw.len()); for (u, v) in raw { - assert!( - u < num_vertices && v < num_vertices, - "{side} contact endpoint out of range for num_vertices = {num_vertices}: ({u}, {v})" - ); - assert!(u != v, "{side} contact has self-loop: ({u}, {v})"); + if u >= num_vertices || v >= num_vertices { + return Err(format!( + "{side} contact endpoint out of range for num_vertices = {num_vertices}: ({u}, {v})" + ) + .into()); + } + if u == v { + return Err(format!("{side} contact has self-loop: ({u}, {v})").into()); + } let (a, b) = if u < v { (u, v) } else { (v, u) }; - assert!( - seen.insert((a, b)), - "{side} has duplicate contact after normalization: ({a}, {b})" - ); + if !seen.insert((a, b)) { + return Err( + format!("{side} has duplicate contact after normalization: ({a}, {b})").into(), + ); + } out.push((a, b)); } - out + Ok(out) } impl MaximumContactMapOverlap { /// Construct a new instance from two ordered contact maps. /// - /// Contacts are canonicalized to `(min, max)` pairs. Self-loops, duplicate - /// contacts (after normalization), and out-of-range endpoints panic. + /// Contacts are canonicalized to `(min, max)` pairs. + /// + /// # Panics + /// + /// Panics on self-loops, duplicate contacts (after normalization), and + /// out-of-range endpoints. pub fn new( num_vertices_1: usize, contacts_1: Vec<(usize, usize)>, num_vertices_2: usize, contacts_2: Vec<(usize, usize)>, ) -> Self { - let contacts_1 = canonicalize_contacts(contacts_1, num_vertices_1, "G_1"); - let contacts_2 = canonicalize_contacts(contacts_2, num_vertices_2, "G_2"); - Self { + Self::try_new(num_vertices_1, contacts_1, num_vertices_2, contacts_2) + .unwrap_or_else(|error| panic!("{error}")) + } + + /// Create an instance, returning validation errors instead of panicking. + fn try_new( + num_vertices_1: usize, + contacts_1: Vec<(usize, usize)>, + num_vertices_2: usize, + contacts_2: Vec<(usize, usize)>, + ) -> Result { + let contacts_1 = canonicalize_contacts(contacts_1, num_vertices_1, "G_1")?; + let contacts_2 = canonicalize_contacts(contacts_2, num_vertices_2, "G_2")?; + Ok(Self { num_vertices_1, contacts_1, num_vertices_2, contacts_2, - } + }) } /// Number of ordered residues/vertices in `G_1`. diff --git a/src/models/graph/multiple_copy_file_allocation.rs b/src/models/graph/multiple_copy_file_allocation.rs index 0813b0113..91b3dd44d 100644 --- a/src/models/graph/multiple_copy_file_allocation.rs +++ b/src/models/graph/multiple_copy_file_allocation.rs @@ -33,12 +33,27 @@ inventory::submit! { /// /// where d(v, V') is the shortest-path distance from v to the nearest copy in V'. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MultipleCopyFileAllocationData")] pub struct MultipleCopyFileAllocation { graph: SimpleGraph, usage: Vec, storage: Vec, } +#[derive(Deserialize)] +struct MultipleCopyFileAllocationData { + graph: SimpleGraph, + usage: Vec, + storage: Vec, +} + +impl TryFrom for MultipleCopyFileAllocation { + type Error = crate::registry::ConstructionError; + fn try_from(data: MultipleCopyFileAllocationData) -> Result { + Self::try_new(data.graph, data.usage, data.storage) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct MultipleCopyFileAllocationCreateSpec { /// Network graph edges. @@ -77,38 +92,36 @@ impl TryFrom for MultipleCopyFileAllocatio if count < inferred { return Err("num_vertices is too small for graph endpoints".into()); } - if spec.usage.len() != count { - return Err("usage length must match num_vertices".into()); - } - if spec.storage.len() != count { - return Err("storage length must match num_vertices".into()); - } - Ok(Self { - graph: SimpleGraph::new(count, spec.graph), - usage: spec.usage, - storage: spec.storage, - }) + Self::try_new( + SimpleGraph::new(count, spec.graph), + spec.usage, + spec.storage, + ) } } impl MultipleCopyFileAllocation { /// Create a new Multiple Copy File Allocation instance. pub fn new(graph: SimpleGraph, usage: Vec, storage: Vec) -> Self { - assert_eq!( - usage.len(), - graph.num_vertices(), - "usage length must match graph num_vertices" - ); - assert_eq!( - storage.len(), - graph.num_vertices(), - "storage length must match graph num_vertices" - ); - Self { + Self::try_new(graph, usage, storage).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: SimpleGraph, + usage: Vec, + storage: Vec, + ) -> Result { + if usage.len() != graph.num_vertices() { + return Err("usage length must match graph num_vertices".into()); + } + if storage.len() != graph.num_vertices() { + return Err("storage length must match graph num_vertices".into()); + } + Ok(Self { graph, usage, storage, - } + }) } /// Get a reference to the underlying graph. diff --git a/src/models/misc/minimum_weight_and_or_graph.rs b/src/models/misc/minimum_weight_and_or_graph.rs index 879879967..703b8f614 100644 --- a/src/models/misc/minimum_weight_and_or_graph.rs +++ b/src/models/misc/minimum_weight_and_or_graph.rs @@ -88,40 +88,14 @@ struct MinimumWeightAndOrGraphCreateSpec { impl TryFrom for MinimumWeightAndOrGraph { type Error = crate::registry::ConstructionError; fn try_from(spec: MinimumWeightAndOrGraphCreateSpec) -> Result { - if spec.source >= spec.num_vertices { - return Err("source is outside the graph".to_string().into()); - } - if spec.gate_types.len() != spec.num_vertices { - return Err("gate_types length must equal num_vertices" - .to_string() - .into()); - } - if spec.gate_types[spec.source].is_none() { - return Err("source must be an AND or OR gate".to_string().into()); - } - if let Some(&(u, v)) = spec - .arcs - .iter() - .find(|&&(u, v)| u >= spec.num_vertices || v >= spec.num_vertices) - { - return Err(format!("arc ({u}, {v}) is out of bounds").into()); - } - let count = spec.arcs.len(); - let arc_weights = spec.arc_weights.unwrap_or_else(|| vec![1; count]); - if arc_weights.len() != count { - return Err(format!( - "arc_weights has {} entries, expected {count}", - arc_weights.len() - ) - .into()); - } - Ok(Self::new( + let arc_weights = spec.arc_weights.unwrap_or_else(|| vec![1; spec.arcs.len()]); + Self::try_new( spec.num_vertices, spec.arcs, spec.source, spec.gate_types, arc_weights, - )) + ) } } @@ -140,15 +114,14 @@ impl<'de> Deserialize<'de> for MinimumWeightAndOrGraph { D: Deserializer<'de>, { let data = MinimumWeightAndOrGraphData::deserialize(deserializer)?; - let outgoing = Self::build_outgoing(data.num_vertices, &data.arcs); - Ok(Self { - num_vertices: data.num_vertices, - arcs: data.arcs, - source: data.source, - gate_types: data.gate_types, - arc_weights: data.arc_weights, - outgoing, - }) + Self::try_new( + data.num_vertices, + data.arcs, + data.source, + data.gate_types, + data.arc_weights, + ) + .map_err(serde::de::Error::custom) } } @@ -167,49 +140,61 @@ impl MinimumWeightAndOrGraph { gate_types: Vec>, arc_weights: Vec, ) -> Self { - assert!( - source < num_vertices, - "Source vertex {} out of bounds for {} vertices", - source, - num_vertices - ); - assert_eq!( - gate_types.len(), - num_vertices, - "gate_types length {} does not match num_vertices {}", - gate_types.len(), - num_vertices - ); - assert_eq!( - arc_weights.len(), - arcs.len(), - "arc_weights length {} does not match number of arcs {}", - arc_weights.len(), - arcs.len() - ); - for (i, &(u, v)) in arcs.iter().enumerate() { - assert!( - u < num_vertices && v < num_vertices, - "Arc {} ({}, {}) out of bounds for {} vertices", - i, - u, - v, + Self::try_new(num_vertices, arcs, source, gate_types, arc_weights) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + arcs: Vec<(usize, usize)>, + source: usize, + gate_types: Vec>, + arc_weights: Vec, + ) -> Result { + if source >= num_vertices { + return Err(format!( + "Source vertex {} out of bounds for {} vertices", + source, num_vertices + ) + .into()); + } + if gate_types.len() != num_vertices { + return Err(format!( + "gate_types length {} does not match num_vertices {}", + gate_types.len(), num_vertices - ); + ) + .into()); + } + if arc_weights.len() != arcs.len() { + return Err(format!( + "arc_weights length {} does not match number of arcs {}", + arc_weights.len(), + arcs.len() + ) + .into()); + } + for (i, &(u, v)) in arcs.iter().enumerate() { + if u >= num_vertices || v >= num_vertices { + return Err(format!( + "Arc {} ({}, {}) out of bounds for {} vertices", + i, u, v, num_vertices + ) + .into()); + } + } + if gate_types[source].is_none() { + return Err("Source vertex must be an AND or OR gate, not a leaf".into()); } - assert!( - gate_types[source].is_some(), - "Source vertex must be an AND or OR gate, not a leaf" - ); let outgoing = Self::build_outgoing(num_vertices, &arcs); - Self { + Ok(Self { num_vertices, arcs, source, gate_types, arc_weights, outgoing, - } + }) } /// Build outgoing arc index lists for each vertex. diff --git a/src/models/misc/paintshop.rs b/src/models/misc/paintshop.rs index e758c9faf..dd78006a5 100644 --- a/src/models/misc/paintshop.rs +++ b/src/models/misc/paintshop.rs @@ -50,6 +50,7 @@ inventory::submit! { /// } /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "PaintShopData")] pub struct PaintShop { /// The sequence of car labels (as indices into unique cars). sequence_indices: Vec, @@ -61,42 +62,72 @@ pub struct PaintShop { num_cars: usize, } +#[derive(Deserialize)] +struct PaintShopData { + sequence_indices: Vec, + car_labels: Vec, +} + +impl TryFrom for PaintShop { + type Error = crate::registry::ConstructionError; + + fn try_from(data: PaintShopData) -> Result { + let sequence = data + .sequence_indices + .into_iter() + .map(|index| { + data.car_labels.get(index).ok_or_else(|| { + crate::registry::ConstructionError::from(format!( + "car index {index} is outside car_labels" + )) + }) + }) + .collect::, _>>()?; + Self::try_new(sequence) + } +} + impl PaintShop { /// Create a new Paint Shop problem from string labels. /// /// Each element in the sequence must appear exactly twice. pub fn new>(sequence: Vec) -> Self { - let sequence: Vec = sequence.iter().map(|s| s.as_ref().to_string()).collect(); - Self::from_strings(sequence) + Self::try_new(sequence).unwrap_or_else(|error| panic!("{error}")) } - /// Create from a vector of strings. - pub fn from_strings(sequence: Vec) -> Self { + fn try_new>( + sequence: Vec, + ) -> Result { // Build car-to-index mapping and count occurrences - let mut car_count: HashMap = HashMap::new(); - let mut car_to_index: HashMap = HashMap::new(); + let mut car_count: HashMap<&str, usize> = HashMap::new(); + let mut car_to_index: HashMap<&str, usize> = HashMap::new(); let mut car_labels: Vec = Vec::new(); for item in &sequence { - let count = car_count.entry(item.clone()).or_insert(0); + let item = item.as_ref(); + let count = car_count.entry(item).or_insert(0); if *count == 0 { - car_to_index.insert(item.clone(), car_labels.len()); - car_labels.push(item.clone()); + car_to_index.insert(item, car_labels.len()); + car_labels.push(item.to_owned()); } *count += 1; } // Verify each car appears exactly twice for (car, count) in &car_count { - assert_eq!( - *count, 2, - "Each car must appear exactly twice, but '{}' appears {} times", - car, count - ); + if *count != 2 { + return Err(format!( + "each car must appear exactly twice, but '{car}' appears {count} times" + ) + .into()); + } } // Convert sequence to indices - let sequence_indices: Vec = sequence.iter().map(|item| car_to_index[item]).collect(); + let sequence_indices: Vec = sequence + .iter() + .map(|item| car_to_index[item.as_ref()]) + .collect(); // Determine which positions are first occurrences let mut seen: HashSet = HashSet::new(); @@ -107,12 +138,12 @@ impl PaintShop { let num_cars = car_labels.len(); - Self { + Ok(Self { sequence_indices, car_labels, is_first, num_cars, - } + }) } /// Get the sequence length. @@ -183,6 +214,11 @@ impl PaintShop { ) }) } + + /// Create from a vector of strings. + pub fn from_strings(sequence: Vec) -> Self { + Self::new(sequence) + } } /// Count color switches in a painted sequence. diff --git a/src/models/misc/production_planning.rs b/src/models/misc/production_planning.rs index 58e0449cc..4d779bb26 100644 --- a/src/models/misc/production_planning.rs +++ b/src/models/misc/production_planning.rs @@ -24,8 +24,8 @@ inventory::submit! { } #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ProductionPlanningCreateSpec")] pub struct ProductionPlanning { - #[serde(deserialize_with = "positive_usize::deserialize")] num_periods: usize, demands: Vec, capacities: Vec, @@ -55,31 +55,7 @@ struct ProductionPlanningCreateSpec { impl TryFrom for ProductionPlanning { type Error = crate::registry::ConstructionError; fn try_from(spec: ProductionPlanningCreateSpec) -> Result { - if spec.num_periods == 0 { - return Err("num_periods must be positive".to_string().into()); - } - for (name, len) in [ - ("demands", spec.demands.len()), - ("capacities", spec.capacities.len()), - ("setup_costs", spec.setup_costs.len()), - ("production_costs", spec.production_costs.len()), - ("inventory_costs", spec.inventory_costs.len()), - ] { - if len != spec.num_periods { - return Err( - format!("{name} has {len} entries, expected {}", spec.num_periods).into(), - ); - } - } - if spec.capacities.iter().any(|&capacity| { - usize::try_from(capacity) - .ok() - .and_then(|v| v.checked_add(1)) - .is_none() - }) { - return Err("capacities must fit in usize for dims()".to_string().into()); - } - Ok(Self::new( + Self::try_new( spec.num_periods, spec.demands, spec.capacities, @@ -87,7 +63,7 @@ impl TryFrom for ProductionPlanning { spec.production_costs, spec.inventory_costs, spec.cost_bound, - )) + ) } } @@ -101,7 +77,30 @@ impl ProductionPlanning { inventory_costs: Vec, cost_bound: i64, ) -> Self { - assert!(num_periods > 0, "num_periods must be positive"); + Self::try_new( + num_periods, + demands, + capacities, + setup_costs, + production_costs, + inventory_costs, + cost_bound, + ) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_periods: usize, + demands: Vec, + capacities: Vec, + setup_costs: Vec, + production_costs: Vec, + inventory_costs: Vec, + cost_bound: i64, + ) -> Result { + if num_periods == 0 { + return Err("num_periods must be positive".into()); + } for len in [ demands.len(), capacities.len(), @@ -109,33 +108,33 @@ impl ProductionPlanning { production_costs.len(), inventory_costs.len(), ] { - assert_eq!( - len, num_periods, - "all per-period vectors must have length num_periods" - ); + if len != num_periods { + return Err("all per-period vectors must have length num_periods".into()); + } + } + if capacities.iter().any(|&capacity| { + usize::try_from(capacity) + .ok() + .and_then(|value| value.checked_add(1)) + .is_none() + }) { + return Err("capacities must fit in usize for dims()".into()); + } + if !(demands + .iter() + .chain(&capacities) + .chain(&setup_costs) + .chain(&production_costs) + .chain(&inventory_costs) + .all(|&value| value >= 0)) + { + return Err("demands, capacities, and costs must be nonnegative".into()); + } + if cost_bound < 0 { + return Err("cost bound must be nonnegative".into()); } - assert!( - capacities.iter().all(|&capacity| { - usize::try_from(capacity) - .ok() - .and_then(|value| value.checked_add(1)) - .is_some() - }), - "capacities must fit in usize for dims()" - ); - assert!( - demands - .iter() - .chain(&capacities) - .chain(&setup_costs) - .chain(&production_costs) - .chain(&inventory_costs) - .all(|&value| value >= 0), - "demands, capacities, and costs must be nonnegative" - ); - assert!(cost_bound >= 0, "cost bound must be nonnegative"); - Self { + Ok(Self { num_periods, demands, capacities, @@ -143,7 +142,7 @@ impl ProductionPlanning { production_costs, inventory_costs, cost_bound, - } + }) } pub fn num_periods(&self) -> usize { @@ -328,22 +327,6 @@ pub(crate) fn canonical_model_example_specs() -> Vec(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let value = usize::deserialize(deserializer)?; - if value == 0 { - return Err(D::Error::custom("expected positive integer, got 0")); - } - Ok(value) - } -} - #[cfg(test)] #[path = "../../unit_tests/models/misc/production_planning.rs"] mod tests; diff --git a/src/models/misc/timetable_design.rs b/src/models/misc/timetable_design.rs index b96bbe294..d04d72892 100644 --- a/src/models/misc/timetable_design.rs +++ b/src/models/misc/timetable_design.rs @@ -27,6 +27,7 @@ inventory::submit! { /// task-next, period-last order: /// `idx = ((c * num_tasks) + t) * num_periods + h`. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "TimetableDesignCreateSpec")] pub struct TimetableDesign { num_periods: usize, num_craftsmen: usize, @@ -54,77 +55,14 @@ struct TimetableDesignCreateSpec { impl TryFrom for TimetableDesign { type Error = crate::registry::ConstructionError; fn try_from(spec: TimetableDesignCreateSpec) -> Result { - if spec.craftsman_avail.len() != spec.num_craftsmen { - return Err(format!( - "craftsman_avail has {} rows, expected {}", - spec.craftsman_avail.len(), - spec.num_craftsmen - ) - .into()); - } - if let Some((index, row)) = spec - .craftsman_avail - .iter() - .enumerate() - .find(|(_, row)| row.len() != spec.num_periods) - { - return Err(format!( - "craftsman_avail row {index} has {} periods, expected {}", - row.len(), - spec.num_periods - ) - .into()); - } - if spec.task_avail.len() != spec.num_tasks { - return Err(format!( - "task_avail has {} rows, expected {}", - spec.task_avail.len(), - spec.num_tasks - ) - .into()); - } - if let Some((index, row)) = spec - .task_avail - .iter() - .enumerate() - .find(|(_, row)| row.len() != spec.num_periods) - { - return Err(format!( - "task_avail row {index} has {} periods, expected {}", - row.len(), - spec.num_periods - ) - .into()); - } - if spec.requirements.len() != spec.num_craftsmen { - return Err(format!( - "requirements has {} rows, expected {}", - spec.requirements.len(), - spec.num_craftsmen - ) - .into()); - } - if let Some((index, row)) = spec - .requirements - .iter() - .enumerate() - .find(|(_, row)| row.len() != spec.num_tasks) - { - return Err(format!( - "requirements row {index} has {} tasks, expected {}", - row.len(), - spec.num_tasks - ) - .into()); - } - Ok(Self::new( + Self::try_new( spec.num_periods, spec.num_craftsmen, spec.num_tasks, spec.craftsman_avail, spec.task_avail, spec.requirements, - )) + ) } } @@ -142,68 +80,93 @@ impl TimetableDesign { task_avail: Vec>, requirements: Vec>, ) -> Self { - assert_eq!( - craftsman_avail.len(), + Self::try_new( + num_periods, num_craftsmen, - "craftsman_avail has {} rows, expected {}", - craftsman_avail.len(), - num_craftsmen - ); + num_tasks, + craftsman_avail, + task_avail, + requirements, + ) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_periods: usize, + num_craftsmen: usize, + num_tasks: usize, + craftsman_avail: Vec>, + task_avail: Vec>, + requirements: Vec>, + ) -> Result { + if craftsman_avail.len() != num_craftsmen { + return Err(format!( + "craftsman_avail has {} rows, expected {}", + craftsman_avail.len(), + num_craftsmen + ) + .into()); + } for (craftsman, row) in craftsman_avail.iter().enumerate() { - assert_eq!( - row.len(), - num_periods, - "craftsman {} availability has {} periods, expected {}", - craftsman, - row.len(), - num_periods - ); + if row.len() != num_periods { + return Err(format!( + "craftsman {} availability has {} periods, expected {}", + craftsman, + row.len(), + num_periods + ) + .into()); + } } - assert_eq!( - task_avail.len(), - num_tasks, - "task_avail has {} rows, expected {}", - task_avail.len(), - num_tasks - ); + if task_avail.len() != num_tasks { + return Err(format!( + "task_avail has {} rows, expected {}", + task_avail.len(), + num_tasks + ) + .into()); + } for (task, row) in task_avail.iter().enumerate() { - assert_eq!( - row.len(), - num_periods, - "task {} availability has {} periods, expected {}", - task, - row.len(), - num_periods - ); + if row.len() != num_periods { + return Err(format!( + "task {} availability has {} periods, expected {}", + task, + row.len(), + num_periods + ) + .into()); + } } - assert_eq!( - requirements.len(), - num_craftsmen, - "requirements has {} rows, expected {}", - requirements.len(), - num_craftsmen - ); + if requirements.len() != num_craftsmen { + return Err(format!( + "requirements has {} rows, expected {}", + requirements.len(), + num_craftsmen + ) + .into()); + } for (craftsman, row) in requirements.iter().enumerate() { - assert_eq!( - row.len(), - num_tasks, - "requirements row {} has {} tasks, expected {}", - craftsman, - row.len(), - num_tasks - ); + if row.len() != num_tasks { + return Err(format!( + "requirements row {} has {} tasks, expected {}", + craftsman, + row.len(), + num_tasks + ) + .into()); + } } - Self { + Ok(Self { num_periods, num_craftsmen, num_tasks, craftsman_avail, task_avail, requirements, - } + }) } /// Get the number of periods. diff --git a/src/rules/bottlenecktravelingsalesman_ilp.rs b/src/rules/bottlenecktravelingsalesman_ilp.rs index 6951b619a..04b268a97 100644 --- a/src/rules/bottlenecktravelingsalesman_ilp.rs +++ b/src/rules/bottlenecktravelingsalesman_ilp.rs @@ -97,13 +97,6 @@ impl ReduceTo> for BottleneckTravelingSalesman { let edges = self.graph().edges(); let m = edges.len(); let weights = self.weights(); - if weights.len() != m { - return Err( - crate::rules::ReductionError::invalid_target::>( - "edge weights must match the source edges", - ), - ); - } let (num_x, num_z, num_vars, num_constraints) = ReductionBTSPToILP::dimensions(n, m)?; let x = |vertex: usize, position: usize| vertex * n + position; let z = |edge: usize, position: usize, direction: usize| { diff --git a/src/unit_tests/models/graph/acyclic_partition.rs b/src/unit_tests/models/graph/acyclic_partition.rs index 75ab99132..85dd33d8b 100644 --- a/src/unit_tests/models/graph/acyclic_partition.rs +++ b/src/unit_tests/models/graph/acyclic_partition.rs @@ -1,6 +1,29 @@ use super::*; use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; + +#[test] +fn test_acyclic_partition_validates_persisted_input() { + let valid = serde_json::to_value(AcyclicPartition::new( + DirectedGraph::new(2, vec![(0, 1)]), + vec![1i64, 1], + vec![1i64], + 2, + 2, + )) + .unwrap(); + for (field, value) in [ + ("vertex_weights", serde_json::json!([])), + ("arc_costs", serde_json::json!([])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::>(invalid).is_err(), + "{field}" + ); + } +} use crate::topology::DirectedGraph; use crate::traits::Problem; use serde_json; diff --git a/src/unit_tests/models/graph/bottleneck_traveling_salesman.rs b/src/unit_tests/models/graph/bottleneck_traveling_salesman.rs index 9aa70a24d..7730ffd19 100644 --- a/src/unit_tests/models/graph/bottleneck_traveling_salesman.rs +++ b/src/unit_tests/models/graph/bottleneck_traveling_salesman.rs @@ -1,6 +1,17 @@ use super::*; use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; + +#[test] +fn test_bottleneck_traveling_salesman_validates_persisted_input() { + let mut invalid = serde_json::to_value(BottleneckTravelingSalesman::new( + SimpleGraph::new(2, vec![(0, 1)]), + vec![1], + )) + .unwrap(); + invalid["edge_weights"] = serde_json::json!([]); + assert!(serde_json::from_value::(invalid).is_err()); +} use crate::topology::SimpleGraph; use crate::traits::Problem; use crate::types::Min; diff --git a/src/unit_tests/models/graph/kth_best_spanning_tree.rs b/src/unit_tests/models/graph/kth_best_spanning_tree.rs index bb266893d..ad5a3a13c 100644 --- a/src/unit_tests/models/graph/kth_best_spanning_tree.rs +++ b/src/unit_tests/models/graph/kth_best_spanning_tree.rs @@ -1,6 +1,28 @@ use super::*; use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; + +#[test] +fn test_kth_best_spanning_tree_validates_persisted_input() { + let valid = serde_json::to_value(KthBestSpanningTree::new( + SimpleGraph::new(2, vec![(0, 1)]), + vec![1i64], + 1, + 2, + )) + .unwrap(); + for (field, value) in [ + ("weights", serde_json::json!([])), + ("k", serde_json::json!(0)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::>(invalid).is_err(), + "{field}" + ); + } +} use crate::topology::SimpleGraph; use crate::traits::Problem; diff --git a/src/unit_tests/models/graph/maximum_common_edge_subgraph.rs b/src/unit_tests/models/graph/maximum_common_edge_subgraph.rs index 00118979c..c3e7f7d7f 100644 --- a/src/unit_tests/models/graph/maximum_common_edge_subgraph.rs +++ b/src/unit_tests/models/graph/maximum_common_edge_subgraph.rs @@ -1,6 +1,23 @@ use super::*; use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; + +#[test] +fn test_maximum_common_edge_subgraph_validates_persisted_input() { + let valid = + serde_json::to_value(LabelledDigraph::new(2, vec![LabelledArc::new(0, 0, 1)])).unwrap(); + for (field, value) in [ + ("arcs", serde_json::json!([{"src":2,"label":0,"dst":1}])), + ("arcs", serde_json::json!([{"src":0,"label":0,"dst":2}])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::traits::Problem; use crate::types::Max; diff --git a/src/unit_tests/models/graph/maximum_contact_map_overlap.rs b/src/unit_tests/models/graph/maximum_contact_map_overlap.rs index 03cb0e25d..bab3dc7be 100644 --- a/src/unit_tests/models/graph/maximum_contact_map_overlap.rs +++ b/src/unit_tests/models/graph/maximum_contact_map_overlap.rs @@ -2,6 +2,31 @@ use super::*; use crate::registry::find_problem_type_by_alias; use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; + +#[test] +fn test_maximum_contact_map_overlap_validates_persisted_input() { + let valid = serde_json::to_value(MaximumContactMapOverlap::new( + 2, + vec![(0, 1)], + 2, + vec![(0, 1)], + )) + .unwrap(); + for (field, value) in [ + ("contacts_1", serde_json::json!([[0, 2]])), + ("contacts_1", serde_json::json!([[0, 0]])), + ("contacts_1", serde_json::json!([[0, 1], [1, 0]])), + ("contacts_2", serde_json::json!([[0, 2]])), + ("contacts_2", serde_json::json!([[1, 1]])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::traits::Problem; use crate::types::Max; diff --git a/src/unit_tests/models/graph/multiple_copy_file_allocation.rs b/src/unit_tests/models/graph/multiple_copy_file_allocation.rs index f8b910cf7..0260274be 100644 --- a/src/unit_tests/models/graph/multiple_copy_file_allocation.rs +++ b/src/unit_tests/models/graph/multiple_copy_file_allocation.rs @@ -1,6 +1,27 @@ use super::*; use crate::solvers::BruteForceProblem as _; +#[test] +fn test_multiple_copy_file_allocation_validates_persisted_input() { + let valid = serde_json::to_value(MultipleCopyFileAllocation::new( + SimpleGraph::new(2, vec![(0, 1)]), + vec![1, 1], + vec![1, 1], + )) + .unwrap(); + for (field, value) in [ + ("usage", serde_json::json!([])), + ("storage", serde_json::json!([])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} + #[test] fn create_spec_preserves_isolated_vertices() { let problem = MultipleCopyFileAllocation::try_from(MultipleCopyFileAllocationCreateSpec { diff --git a/src/unit_tests/models/misc/minimum_weight_and_or_graph.rs b/src/unit_tests/models/misc/minimum_weight_and_or_graph.rs index 5cbe85f53..89b5ec61b 100644 --- a/src/unit_tests/models/misc/minimum_weight_and_or_graph.rs +++ b/src/unit_tests/models/misc/minimum_weight_and_or_graph.rs @@ -1,6 +1,32 @@ use super::*; use crate::solvers::BruteForceProblem as _; +#[test] +fn test_minimum_weight_and_or_graph_validates_persisted_input() { + let valid = serde_json::to_value(MinimumWeightAndOrGraph::new( + 2, + vec![(0, 1)], + 0, + vec![Some(true), None], + vec![1], + )) + .unwrap(); + for (field, value) in [ + ("source", serde_json::json!(2)), + ("source", serde_json::json!(1)), + ("gate_types", serde_json::json!([])), + ("arc_weights", serde_json::json!([])), + ("arcs", serde_json::json!([[0, 2]])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} + #[test] fn create_spec_defaults_arc_weights() { let p = MinimumWeightAndOrGraph::try_from(MinimumWeightAndOrGraphCreateSpec { diff --git a/src/unit_tests/models/misc/paintshop.rs b/src/unit_tests/models/misc/paintshop.rs index 93fce4912..f0d1c8824 100644 --- a/src/unit_tests/models/misc/paintshop.rs +++ b/src/unit_tests/models/misc/paintshop.rs @@ -1,6 +1,22 @@ use super::*; use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; + +#[test] +fn test_paintshop_validates_persisted_input() { + let valid = serde_json::to_value(PaintShop::new(vec!["a", "b", "a", "b"])).unwrap(); + for (field, value) in [ + ("sequence_indices", serde_json::json!([2, 1, 0, 1])), + ("sequence_indices", serde_json::json!([0, 1, 0])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::traits::Problem; include!("../../jl_helpers.rs"); diff --git a/src/unit_tests/models/misc/production_planning.rs b/src/unit_tests/models/misc/production_planning.rs index 1141a3d07..49058f798 100644 --- a/src/unit_tests/models/misc/production_planning.rs +++ b/src/unit_tests/models/misc/production_planning.rs @@ -1,6 +1,37 @@ use super::*; use crate::solvers::BruteForceProblem as _; +#[test] +fn test_production_planning_validates_persisted_input() { + let valid = serde_json::to_value(ProductionPlanning::new( + 1, + vec![1], + vec![1], + vec![1], + vec![1], + vec![1], + 3, + )) + .unwrap(); + for (field, value) in [ + ("num_periods", serde_json::json!(0)), + ("demands", serde_json::json!([])), + ("capacities", serde_json::json!([-1])), + ("demands", serde_json::json!([-1])), + ("setup_costs", serde_json::json!([-1])), + ("production_costs", serde_json::json!([-1])), + ("inventory_costs", serde_json::json!([-1])), + ("cost_bound", serde_json::json!(-1)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} + #[test] fn create_spec_rejects_period_vector_mismatch() { assert_eq!(ProductionPlanningCreateSpec::FIELDS[0].name, "num_periods"); diff --git a/src/unit_tests/models/misc/timetable_design.rs b/src/unit_tests/models/misc/timetable_design.rs index ee8aaad02..915a3eea9 100644 --- a/src/unit_tests/models/misc/timetable_design.rs +++ b/src/unit_tests/models/misc/timetable_design.rs @@ -1,6 +1,34 @@ use super::*; use crate::solvers::BruteForceProblem as _; +#[test] +fn test_timetable_design_validates_persisted_input() { + let valid = serde_json::to_value(TimetableDesign::new( + 1, + 1, + 1, + vec![vec![true]], + vec![vec![true]], + vec![vec![1]], + )) + .unwrap(); + for (field, value) in [ + ("craftsman_avail", serde_json::json!([])), + ("craftsman_avail", serde_json::json!([[]])), + ("task_avail", serde_json::json!([])), + ("task_avail", serde_json::json!([[]])), + ("requirements", serde_json::json!([])), + ("requirements", serde_json::json!([[]])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} + #[test] fn create_spec_rejects_matrix_shape_mismatch() { assert_eq!(TimetableDesignCreateSpec::FIELDS[3].name, "craftsman_avail"); diff --git a/src/unit_tests/rules/bottlenecktravelingsalesman_ilp.rs b/src/unit_tests/rules/bottlenecktravelingsalesman_ilp.rs index 5d92bc4bb..45383e2f4 100644 --- a/src/unit_tests/rules/bottlenecktravelingsalesman_ilp.rs +++ b/src/unit_tests/rules/bottlenecktravelingsalesman_ilp.rs @@ -217,7 +217,7 @@ fn test_bottleneck_ilp_empty_and_single_edge_are_infeasible() { } #[test] -fn test_bottleneck_ilp_dimensions_and_malformed_weights() { +fn test_bottleneck_ilp_dimensions_and_overflow() { assert_eq!( ReductionBTSPToILP::dimensions(4, 6).unwrap(), (16, 48, 70, 197) @@ -226,9 +226,4 @@ fn test_bottleneck_ilp_dimensions_and_malformed_weights() { for (n, m) in [(usize::MAX, 0), (1, usize::MAX), (0, usize::MAX)] { assert!(ReductionBTSPToILP::dimensions(n, m).is_err()); } - let source: BottleneckTravelingSalesman = serde_json::from_value(serde_json::json!({ - "graph": {"num_vertices": 0, "edges": []}, "edge_weights": [1] - })) - .unwrap(); - assert!(ReduceTo::>::reduce_to(&source).is_err()); }