diff --git a/problemreductions-cli/tests/cli_tests.rs b/problemreductions-cli/tests/cli_tests.rs index 90224cc29..9a60a7572 100644 --- a/problemreductions-cli/tests/cli_tests.rs +++ b/problemreductions-cli/tests/cli_tests.rs @@ -7428,7 +7428,7 @@ fn test_create_bcnf_rejects_out_of_range_attribute_indices() { "CLI should return a user-facing error, got: {stderr}" ); assert!( - stderr.contains("outside universe of size 3"), + stderr.contains("out of range (num_attributes = 3)"), "expected out-of-range error, got: {stderr}" ); } @@ -7454,7 +7454,7 @@ fn test_create_bcnf_rejects_out_of_range_lhs_attribute_indices() { ); let stderr = String::from_utf8_lossy(&output.stderr); assert!( - stderr.contains("subsets[0] contains attribute 4 outside universe of size 3"), + stderr.contains("Functional dependency 0 contains attribute 4 which is out of range"), "expected lhs-specific out-of-range error, got: {stderr}" ); } @@ -7480,7 +7480,7 @@ fn test_create_bcnf_rejects_out_of_range_target_attribute_indices() { ); let stderr = String::from_utf8_lossy(&output.stderr); assert!( - stderr.contains("target contains attribute 4 outside universe of size 3"), + stderr.contains("target_subset contains attribute 4 which is out of range"), "expected target-specific out-of-range error, got: {stderr}" ); } diff --git a/src/models/misc/additional_key.rs b/src/models/misc/additional_key.rs index d15016c1b..99739ab6c 100644 --- a/src/models/misc/additional_key.rs +++ b/src/models/misc/additional_key.rs @@ -61,6 +61,7 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "AdditionalKeyData")] pub struct AdditionalKey { num_attributes: usize, dependencies: Vec<(Vec, Vec)>, @@ -68,6 +69,26 @@ pub struct AdditionalKey { known_keys: Vec>, } +#[derive(Deserialize)] +struct AdditionalKeyData { + num_attributes: usize, + dependencies: Vec<(Vec, Vec)>, + relation_attrs: Vec, + known_keys: Vec>, +} + +impl TryFrom for AdditionalKey { + type Error = crate::registry::ConstructionError; + fn try_from(data: AdditionalKeyData) -> Result { + Self::try_new( + data.num_attributes, + data.dependencies, + data.relation_attrs, + data.known_keys, + ) + } +} + impl AdditionalKey { /// Create a new AdditionalKey instance. /// @@ -81,42 +102,58 @@ impl AdditionalKey { relation_attrs: Vec, known_keys: Vec>, ) -> Self { + Self::try_new(num_attributes, dependencies, relation_attrs, known_keys) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_attributes: usize, + dependencies: Vec<(Vec, Vec)>, + relation_attrs: Vec, + known_keys: Vec>, + ) -> Result { // Validate all attribute indices for &a in &relation_attrs { - assert!( - a < num_attributes, - "relation_attrs element {a} >= num_attributes {num_attributes}" - ); + if a >= num_attributes { + return Err(format!( + "relation_attrs element {a} >= num_attributes {num_attributes}" + ) + .into()); + } } // Validate relation_attrs uniqueness let mut sorted_ra = relation_attrs.clone(); sorted_ra.sort_unstable(); sorted_ra.dedup(); - assert_eq!( - sorted_ra.len(), - relation_attrs.len(), - "relation_attrs contains duplicates" - ); + if sorted_ra.len() != relation_attrs.len() { + return Err("relation_attrs contains duplicates".into()); + } for (lhs, rhs) in &dependencies { for &a in lhs { - assert!( - a < num_attributes, - "dependency lhs attribute {a} >= num_attributes {num_attributes}" - ); + if a >= num_attributes { + return Err(format!( + "dependency lhs attribute {a} >= num_attributes {num_attributes}" + ) + .into()); + } } for &a in rhs { - assert!( - a < num_attributes, - "dependency rhs attribute {a} >= num_attributes {num_attributes}" - ); + if a >= num_attributes { + return Err(format!( + "dependency rhs attribute {a} >= num_attributes {num_attributes}" + ) + .into()); + } } } for key in &known_keys { for &a in key { - assert!( - a < num_attributes, - "known_keys attribute {a} >= num_attributes {num_attributes}" - ); + if a >= num_attributes { + return Err(format!( + "known_keys attribute {a} >= num_attributes {num_attributes}" + ) + .into()); + } } } // Sort known_keys entries internally for consistent comparison @@ -127,12 +164,12 @@ impl AdditionalKey { k }) .collect(); - Self { + Ok(Self { num_attributes, dependencies, relation_attrs, known_keys, - } + }) } /// Returns the number of attributes in the universal set A. diff --git a/src/models/misc/boyce_codd_normal_form_violation.rs b/src/models/misc/boyce_codd_normal_form_violation.rs index a09631428..ec40d58df 100644 --- a/src/models/misc/boyce_codd_normal_form_violation.rs +++ b/src/models/misc/boyce_codd_normal_form_violation.rs @@ -58,6 +58,7 @@ inventory::submit! { /// .unwrap()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "BoyceCoddNormalFormViolationData")] pub struct BoyceCoddNormalFormViolation { /// Total number of attributes (elements are `0..num_attributes`). num_attributes: usize, @@ -67,6 +68,24 @@ pub struct BoyceCoddNormalFormViolation { target_subset: Vec, } +#[derive(Deserialize)] +struct BoyceCoddNormalFormViolationData { + num_attributes: usize, + functional_deps: Vec<(Vec, Vec)>, + target_subset: Vec, +} + +impl TryFrom for BoyceCoddNormalFormViolation { + type Error = crate::registry::ConstructionError; + fn try_from(data: BoyceCoddNormalFormViolationData) -> Result { + Self::try_new( + data.num_attributes, + data.functional_deps, + data.target_subset, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct BoyceCoddNormalFormViolationCreateSpec { /// Total number of attributes in A. @@ -82,32 +101,7 @@ impl TryFrom for BoyceCoddNormalFormViol type Error = crate::registry::ConstructionError; fn try_from(spec: BoyceCoddNormalFormViolationCreateSpec) -> Result { - if spec.target.is_empty() { - return Err("target must be non-empty".to_string().into()); - } - for (dependency_index, (lhs, rhs)) in spec.subsets.iter().enumerate() { - if lhs.is_empty() { - return Err(format!("subsets[{dependency_index}] has an empty left side").into()); - } - if let Some(&attribute) = lhs - .iter() - .chain(rhs) - .find(|&&attribute| attribute >= spec.n) - { - return Err(format!( - "subsets[{dependency_index}] contains attribute {attribute} outside universe of size {}", - spec.n - ).into()); - } - } - if let Some(&attribute) = spec.target.iter().find(|&&attribute| attribute >= spec.n) { - return Err(format!( - "target contains attribute {attribute} outside universe of size {}", - spec.n - ) - .into()); - } - Ok(Self::new(spec.n, spec.subsets, spec.target)) + Self::try_new(spec.n, spec.subsets, spec.target) } } @@ -129,27 +123,32 @@ impl BoyceCoddNormalFormViolation { functional_deps: Vec<(Vec, Vec)>, target_subset: Vec, ) -> Self { - assert!(!target_subset.is_empty(), "target_subset must be non-empty"); + Self::try_new(num_attributes, functional_deps, target_subset) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_attributes: usize, + functional_deps: Vec<(Vec, Vec)>, + target_subset: Vec, + ) -> Result { + if target_subset.is_empty() { + return Err("target_subset must be non-empty".into()); + } let mut functional_deps = functional_deps; for (fd_index, (lhs, rhs)) in functional_deps.iter_mut().enumerate() { - assert!( - !lhs.is_empty(), - "Functional dependency {} has an empty LHS", - fd_index - ); + if lhs.is_empty() { + return Err(format!("Functional dependency {} has an empty LHS", fd_index).into()); + } lhs.sort_unstable(); lhs.dedup(); rhs.sort_unstable(); rhs.dedup(); for &attr in lhs.iter().chain(rhs.iter()) { - assert!( - attr < num_attributes, - "Functional dependency {} contains attribute {} which is out of range (num_attributes = {})", - fd_index, - attr, - num_attributes - ); + if attr >= num_attributes { + return Err(format!("Functional dependency {} contains attribute {} which is out of range (num_attributes = {})", fd_index, attr, num_attributes).into()); + } } } @@ -157,19 +156,16 @@ impl BoyceCoddNormalFormViolation { target_subset.sort_unstable(); target_subset.dedup(); for &attr in &target_subset { - assert!( - attr < num_attributes, - "target_subset contains attribute {} which is out of range (num_attributes = {})", - attr, - num_attributes - ); + if attr >= num_attributes { + return Err(format!("target_subset contains attribute {} which is out of range (num_attributes = {})", attr, num_attributes).into()); + } } - Self { + Ok(Self { num_attributes, functional_deps, target_subset, - } + }) } /// Return the total number of attributes. diff --git a/src/models/misc/conjunctive_boolean_query.rs b/src/models/misc/conjunctive_boolean_query.rs index a6ce31f50..5853e5285 100644 --- a/src/models/misc/conjunctive_boolean_query.rs +++ b/src/models/misc/conjunctive_boolean_query.rs @@ -76,6 +76,7 @@ pub enum QueryArg { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(try_from = "ConjunctiveBooleanQueryData")] pub struct ConjunctiveBooleanQuery { domain_size: usize, relations: Vec, @@ -83,6 +84,26 @@ pub struct ConjunctiveBooleanQuery { conjuncts: Vec<(usize, Vec)>, } +#[derive(Deserialize)] +struct ConjunctiveBooleanQueryData { + domain_size: usize, + relations: Vec, + num_variables: usize, + conjuncts: Vec<(usize, Vec)>, +} + +impl TryFrom for ConjunctiveBooleanQuery { + type Error = crate::registry::ConstructionError; + fn try_from(data: ConjunctiveBooleanQueryData) -> Result { + Self::try_new( + data.domain_size, + data.relations, + data.num_variables, + data.conjuncts, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct ConjunctiveBooleanQueryCreateSpec { /// Size of the finite domain. @@ -111,59 +132,12 @@ impl TryFrom for ConjunctiveBooleanQuery { } } - for (relation_index, relation) in spec.relations.iter().enumerate() { - for (tuple_index, tuple) in relation.tuples.iter().enumerate() { - if tuple.len() != relation.arity { - return Err(format!( - "relation {relation_index} tuple {tuple_index} has length {}, expected arity {}", - tuple.len(), - relation.arity - ).into()); - } - for (entry_index, &value) in tuple.iter().enumerate() { - if value >= spec.domain_size { - return Err(format!( - "relation {relation_index} tuple {tuple_index} entry {entry_index} is {value}, must be less than domain size {}", - spec.domain_size - ).into()); - } - } - } - } - - for (conjunct_index, (relation_index, args)) in spec.conjuncts.iter().enumerate() { - let relation = spec.relations.get(*relation_index).ok_or_else(|| { - format!( - "conjunct {conjunct_index} relation index {relation_index} is out of range for {} relations", - spec.relations.len() - ) - })?; - if args.len() != relation.arity { - return Err(format!( - "conjunct {conjunct_index} has {} arguments, expected arity {}", - args.len(), - relation.arity - ) - .into()); - } - for (argument_index, arg) in args.iter().enumerate() { - if let QueryArg::Constant(value) = arg { - if *value >= spec.domain_size { - return Err(format!( - "conjunct {conjunct_index} argument {argument_index} constant {value} must be less than domain size {}", - spec.domain_size - ).into()); - } - } - } - } - - Ok(Self { - domain_size: spec.domain_size, - relations: spec.relations, + Self::try_new( + spec.domain_size, + spec.relations, num_variables, - conjuncts: spec.conjuncts, - }) + spec.conjuncts, + ) } } @@ -185,57 +159,73 @@ impl ConjunctiveBooleanQuery { num_variables: usize, conjuncts: Vec<(usize, Vec)>, ) -> Self { + Self::try_new(domain_size, relations, num_variables, conjuncts) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + domain_size: usize, + relations: Vec, + num_variables: usize, + conjuncts: Vec<(usize, Vec)>, + ) -> Result { for (i, rel) in relations.iter().enumerate() { for (j, tuple) in rel.tuples.iter().enumerate() { - assert!( - tuple.len() == rel.arity, - "Relation {i}: tuple {j} has length {}, expected arity {}", - tuple.len(), - rel.arity - ); + if tuple.len() != rel.arity { + return Err(format!( + "Relation {i}: tuple {j} has length {}, expected arity {}", + tuple.len(), + rel.arity + ) + .into()); + } for (k, &val) in tuple.iter().enumerate() { - assert!( - val < domain_size, - "Relation {i}: tuple {j}, entry {k} is {val}, must be < {domain_size}" - ); + if val >= domain_size { + return Err(format!( + "Relation {i}: tuple {j}, entry {k} is {val}, must be < {domain_size}" + ) + .into()); + } } } } for (i, (rel_idx, args)) in conjuncts.iter().enumerate() { - assert!( - *rel_idx < relations.len(), - "Conjunct {i}: relation index {rel_idx} out of range (have {} relations)", - relations.len() - ); - assert!( - args.len() == relations[*rel_idx].arity, - "Conjunct {i}: has {} args, expected arity {}", - args.len(), - relations[*rel_idx].arity - ); + if *rel_idx >= relations.len() { + return Err(format!( + "Conjunct {i}: relation index {rel_idx} out of range (have {} relations)", + relations.len() + ) + .into()); + } + if args.len() != relations[*rel_idx].arity { + return Err(format!( + "Conjunct {i}: has {} args, expected arity {}", + args.len(), + relations[*rel_idx].arity + ) + .into()); + } for (k, arg) in args.iter().enumerate() { match arg { QueryArg::Variable(v) => { - assert!( - *v < num_variables, - "Conjunct {i}, arg {k}: Variable({v}) >= num_variables ({num_variables})" - ); + if *v >= num_variables { + return Err(format!("Conjunct {i}, arg {k}: Variable({v}) >= num_variables ({num_variables})").into()); + } } QueryArg::Constant(c) => { - assert!( - *c < domain_size, - "Conjunct {i}, arg {k}: Constant({c}) >= domain_size ({domain_size})" - ); + if *c >= domain_size { + return Err(format!("Conjunct {i}, arg {k}: Constant({c}) >= domain_size ({domain_size})").into()); + } } } } } - Self { + Ok(Self { domain_size, relations, num_variables, conjuncts, - } + }) } /// Returns the size of the finite domain. diff --git a/src/models/misc/conjunctive_query_foldability.rs b/src/models/misc/conjunctive_query_foldability.rs index d8f6ed074..dc61d1e2b 100644 --- a/src/models/misc/conjunctive_query_foldability.rs +++ b/src/models/misc/conjunctive_query_foldability.rs @@ -86,6 +86,7 @@ pub enum Term { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ConjunctiveQueryFoldabilityData")] pub struct ConjunctiveQueryFoldability { /// Size of the finite domain D. domain_size: usize, @@ -101,6 +102,30 @@ pub struct ConjunctiveQueryFoldability { query2_conjuncts: Vec<(usize, Vec)>, } +#[derive(Deserialize)] +struct ConjunctiveQueryFoldabilityData { + domain_size: usize, + num_distinguished: usize, + num_undistinguished: usize, + relation_arities: Vec, + query1_conjuncts: Vec<(usize, Vec)>, + query2_conjuncts: Vec<(usize, Vec)>, +} + +impl TryFrom for ConjunctiveQueryFoldability { + type Error = crate::registry::ConstructionError; + fn try_from(data: ConjunctiveQueryFoldabilityData) -> Result { + Self::try_new( + data.domain_size, + data.num_distinguished, + data.num_undistinguished, + data.relation_arities, + data.query1_conjuncts, + data.query2_conjuncts, + ) + } +} + impl ConjunctiveQueryFoldability { /// Create a new `ConjunctiveQueryFoldability` instance. /// @@ -129,6 +154,25 @@ impl ConjunctiveQueryFoldability { query1_conjuncts: Vec<(usize, Vec)>, query2_conjuncts: Vec<(usize, Vec)>, ) -> Self { + Self::try_new( + domain_size, + num_distinguished, + num_undistinguished, + relation_arities, + query1_conjuncts, + query2_conjuncts, + ) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + domain_size: usize, + num_distinguished: usize, + num_undistinguished: usize, + relation_arities: Vec, + query1_conjuncts: Vec<(usize, Vec)>, + query2_conjuncts: Vec<(usize, Vec)>, + ) -> Result { let instance = Self { domain_size, num_distinguished, @@ -137,55 +181,63 @@ impl ConjunctiveQueryFoldability { query1_conjuncts, query2_conjuncts, }; - instance.validate(); - instance + instance.validate()?; + Ok(instance) } - /// Validate the instance, panicking on any inconsistency. - fn validate(&self) { + /// Check relation arities and argument indices. + fn validate(&self) -> Result<(), crate::registry::ConstructionError> { for (query_name, conjuncts) in [ ("Q1", &self.query1_conjuncts), ("Q2", &self.query2_conjuncts), ] { for (atom_idx, (rel_idx, args)) in conjuncts.iter().enumerate() { - assert!( - *rel_idx < self.relation_arities.len(), - "Atom {atom_idx} of {query_name}: relation index {rel_idx} out of range \ + if *rel_idx >= self.relation_arities.len() { + return Err(format!( + "Atom {atom_idx} of {query_name}: relation index {rel_idx} out of range \ (num_relations = {})", - self.relation_arities.len() - ); + self.relation_arities.len() + ) + .into()); + }; let arity = self.relation_arities[*rel_idx]; - assert_eq!( - args.len(), - arity, - "Atom {atom_idx} of {query_name}: relation {rel_idx} has arity {arity} \ + if args.len() != arity { + return Err(format!( + "Atom {atom_idx} of {query_name}: relation {rel_idx} has arity {arity} \ but got {} arguments", - args.len() - ); + args.len() + ) + .into()); + }; for term in args { match term { - Term::Constant(i) => assert!( - *i < self.domain_size, - "Atom {atom_idx} of {query_name}: Constant({i}) out of range \ + Term::Constant(i) => { + if *i >= self.domain_size { + return Err(format!( + "Atom {atom_idx} of {query_name}: Constant({i}) out of range \ (domain_size = {})", - self.domain_size - ), - Term::Distinguished(i) => assert!( - *i < self.num_distinguished, - "Atom {atom_idx} of {query_name}: Distinguished({i}) out of range \ - (num_distinguished = {})", - self.num_distinguished - ), - Term::Undistinguished(i) => assert!( - *i < self.num_undistinguished, - "Atom {atom_idx} of {query_name}: Undistinguished({i}) out of range \ - (num_undistinguished = {})", - self.num_undistinguished - ), + self.domain_size + ) + .into()); + } + } + Term::Distinguished(i) => { + if *i >= self.num_distinguished { + return Err(format!("Atom {atom_idx} of {query_name}: Distinguished({i}) out of range \ + (num_distinguished = {})", self.num_distinguished).into()); + } + } + Term::Undistinguished(i) => { + if *i >= self.num_undistinguished { + return Err(format!("Atom {atom_idx} of {query_name}: Undistinguished({i}) out of range \ + (num_undistinguished = {})", self.num_undistinguished).into()); + } + } } } } } + Ok(()) } /// Returns the size of the finite domain D. diff --git a/src/models/misc/consistency_of_database_frequency_tables.rs b/src/models/misc/consistency_of_database_frequency_tables.rs index b177ed320..82eff4f6b 100644 --- a/src/models/misc/consistency_of_database_frequency_tables.rs +++ b/src/models/misc/consistency_of_database_frequency_tables.rs @@ -97,6 +97,7 @@ inventory::submit! { /// The Consistency of Database Frequency Tables decision problem. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(try_from = "ConsistencyOfDatabaseFrequencyTablesData")] pub struct ConsistencyOfDatabaseFrequencyTables { num_objects: usize, attribute_domains: Vec, @@ -104,6 +105,26 @@ pub struct ConsistencyOfDatabaseFrequencyTables { known_values: Vec, } +#[derive(Deserialize)] +struct ConsistencyOfDatabaseFrequencyTablesData { + num_objects: usize, + attribute_domains: Vec, + frequency_tables: Vec, + known_values: Vec, +} + +impl TryFrom for ConsistencyOfDatabaseFrequencyTables { + type Error = crate::registry::ConstructionError; + fn try_from(data: ConsistencyOfDatabaseFrequencyTablesData) -> Result { + Self::try_new( + data.num_objects, + data.attribute_domains, + data.frequency_tables, + data.known_values, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct ConsistencyOfDatabaseFrequencyTablesCreateSpec { /// Number of database objects. @@ -124,19 +145,12 @@ impl TryFrom { type Error = crate::registry::ConstructionError; fn try_from(spec: ConsistencyOfDatabaseFrequencyTablesCreateSpec) -> Result { - let known_values = spec.known_values.unwrap_or_default(); - validate_cdft_create( + Self::try_new( spec.num_objects, - &spec.attribute_domains, - &spec.frequency_tables, - &known_values, - )?; - Ok(Self { - num_objects: spec.num_objects, - attribute_domains: spec.attribute_domains, - frequency_tables: spec.frequency_tables, - known_values, - }) + spec.attribute_domains, + spec.frequency_tables, + spec.known_values.unwrap_or_default(), + ) } } @@ -220,20 +234,34 @@ impl ConsistencyOfDatabaseFrequencyTables { frequency_tables: Vec, known_values: Vec, ) -> Self { + Self::try_new( + num_objects, + attribute_domains, + frequency_tables, + known_values, + ) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_objects: usize, + attribute_domains: Vec, + frequency_tables: Vec, + known_values: Vec, + ) -> Result { validate_cdft_create( num_objects, &attribute_domains, &frequency_tables, &known_values, - ) - .unwrap_or_else(|error| panic!("{error}")); + )?; - Self { + Ok(Self { num_objects, attribute_domains, frequency_tables, known_values, - } + }) } /// Returns the number of objects. diff --git a/src/models/set/consecutive_sets.rs b/src/models/set/consecutive_sets.rs index 9fc0aa171..64218b4a6 100644 --- a/src/models/set/consecutive_sets.rs +++ b/src/models/set/consecutive_sets.rs @@ -71,6 +71,7 @@ inventory::submit! { /// .unwrap()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ConsecutiveSetsData")] pub struct ConsecutiveSets { /// Size of the alphabet (elements are 0..alphabet_size-1). alphabet_size: usize, @@ -80,6 +81,21 @@ pub struct ConsecutiveSets { bound_k: usize, } +#[derive(Deserialize)] +struct ConsecutiveSetsData { + alphabet_size: usize, + subsets: Vec>, + bound_k: usize, +} + +impl TryFrom for ConsecutiveSets { + type Error = crate::registry::ConstructionError; + + fn try_from(data: ConsecutiveSetsData) -> Result { + Self::try_new(data.alphabet_size, data.subsets, data.bound_k) + } +} + impl ConsecutiveSets { /// Create a new Consecutive Sets problem. /// @@ -88,31 +104,34 @@ impl ConsecutiveSets { /// Panics if `bound_k` is zero, if any subset contains duplicate elements, /// or if any element is outside the alphabet. pub fn new(alphabet_size: usize, subsets: Vec>, bound_k: usize) -> Self { - assert!(bound_k > 0, "bound_k must be positive, got 0"); - let mut subsets = subsets; - for (i, subset) in subsets.iter_mut().enumerate() { + Self::try_new(alphabet_size, subsets, bound_k).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + alphabet_size: usize, + mut subsets: Vec>, + bound_k: usize, + ) -> Result { + if bound_k == 0 { + return Err("bound_k must be positive, got 0".into()); + } + for (index, subset) in subsets.iter_mut().enumerate() { let mut seen = HashSet::with_capacity(subset.len()); - for &elem in subset.iter() { - assert!( - elem < alphabet_size, - "Subset {} contains element {} which is outside alphabet of size {}", - i, - elem, - alphabet_size - ); - assert!( - seen.insert(elem), - "Subset {} contains duplicate elements", - i - ); + for &element in subset.iter() { + if element >= alphabet_size { + return Err(format!("subset {index} contains element {element} outside alphabet of size {alphabet_size}").into()); + } + if !seen.insert(element) { + return Err(format!("subset {index} contains duplicate elements").into()); + } } subset.sort(); } - Self { + Ok(Self { alphabet_size, subsets, bound_k, - } + }) } /// Get the alphabet size. diff --git a/src/models/set/exact_cover_by_3_sets.rs b/src/models/set/exact_cover_by_3_sets.rs index ac6f64852..43cce38b0 100644 --- a/src/models/set/exact_cover_by_3_sets.rs +++ b/src/models/set/exact_cover_by_3_sets.rs @@ -52,6 +52,7 @@ inventory::submit! { /// assert!(problem.evaluate(&solutions[0]).unwrap()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ExactCoverBy3SetsCreateSpec")] pub struct ExactCoverBy3Sets { /// Size of the universe (elements are 0..universe_size, must be divisible by 3). universe_size: usize, @@ -68,28 +69,9 @@ struct ExactCoverBy3SetsCreateSpec { impl TryFrom for ExactCoverBy3Sets { type Error = crate::registry::ConstructionError; - fn try_from(mut spec: ExactCoverBy3SetsCreateSpec) -> Result { - if !spec.universe_size.is_multiple_of(3) { - return Err("universe_size must be divisible by 3".into()); - } - for (index, subset) in spec.subsets.iter_mut().enumerate() { - if subset[0] == subset[1] || subset[0] == subset[2] || subset[1] == subset[2] { - return Err(format!("subset {index} contains duplicate elements").into()); - } - if let Some(&element) = subset - .iter() - .find(|&&element| element >= spec.universe_size) - { - return Err( - format!("subset {index} contains out-of-range element {element}").into(), - ); - } - subset.sort(); - } - Ok(Self { - universe_size: spec.universe_size, - subsets: spec.subsets, - }) + + fn try_from(spec: ExactCoverBy3SetsCreateSpec) -> Result { + Self::try_new(spec.universe_size, spec.subsets) } } @@ -101,34 +83,33 @@ impl ExactCoverBy3Sets { /// Panics if `universe_size` is not divisible by 3, or if any subset /// contains duplicate elements or elements outside the universe. pub fn new(universe_size: usize, subsets: Vec<[usize; 3]>) -> Self { - assert!( - universe_size.is_multiple_of(3), - "Universe size must be divisible by 3, got {}", - universe_size - ); - let mut subsets = subsets; - for (i, subset) in subsets.iter_mut().enumerate() { - assert!( - subset[0] != subset[1] && subset[0] != subset[2] && subset[1] != subset[2], - "Subset {} contains duplicate elements: {:?}", - i, - subset + Self::try_new(universe_size, subsets).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + universe_size: usize, + mut subsets: Vec<[usize; 3]>, + ) -> Result { + if !universe_size.is_multiple_of(3) { + return Err( + format!("Universe size must be divisible by 3, got {universe_size}").into(), ); - for &elem in subset.iter() { - assert!( - elem < universe_size, - "Subset {} contains element {} which is outside universe of size {}", - i, - elem, - universe_size + } + for (index, subset) in subsets.iter_mut().enumerate() { + if subset[0] == subset[1] || subset[0] == subset[2] || subset[1] == subset[2] { + return Err(format!("subset {index} contains duplicate elements").into()); + } + if let Some(&element) = subset.iter().find(|&&element| element >= universe_size) { + return Err( + format!("Subset {index} contains element {element} which is outside universe of size {universe_size}").into(), ); } subset.sort(); } - Self { + Ok(Self { universe_size, subsets, - } + }) } /// Get the universe size. diff --git a/src/models/set/minimum_cardinality_key.rs b/src/models/set/minimum_cardinality_key.rs index 89b3a21ac..35425c051 100644 --- a/src/models/set/minimum_cardinality_key.rs +++ b/src/models/set/minimum_cardinality_key.rs @@ -31,6 +31,7 @@ inventory::submit! { /// find a subset `K ⊆ A` of minimum cardinality such that the closure of `K` /// under `F` equals `A` (i.e., `K` is a key). #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MinimumCardinalityKeyData")] pub struct MinimumCardinalityKey { /// Number of attributes (elements are `0..num_attributes`). num_attributes: usize, @@ -38,6 +39,20 @@ pub struct MinimumCardinalityKey { dependencies: Vec<(Vec, Vec)>, } +#[derive(Deserialize)] +struct MinimumCardinalityKeyData { + num_attributes: usize, + dependencies: Vec<(Vec, Vec)>, +} + +impl TryFrom for MinimumCardinalityKey { + type Error = crate::registry::ConstructionError; + + fn try_from(data: MinimumCardinalityKeyData) -> Result { + Self::try_new(data.num_attributes, data.dependencies) + } +} + impl MinimumCardinalityKey { /// Create a new Minimum Cardinality Key instance. /// @@ -45,27 +60,30 @@ impl MinimumCardinalityKey { /// /// Panics if any attribute index in a dependency lies outside the attribute set. pub fn new(num_attributes: usize, dependencies: Vec<(Vec, Vec)>) -> Self { - let mut dependencies = dependencies; - for (dep_index, (lhs, rhs)) in dependencies.iter_mut().enumerate() { + Self::try_new(num_attributes, dependencies).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_attributes: usize, + mut dependencies: Vec<(Vec, Vec)>, + ) -> Result { + for (index, (lhs, rhs)) in dependencies.iter_mut().enumerate() { lhs.sort_unstable(); lhs.dedup(); rhs.sort_unstable(); rhs.dedup(); - for &attr in lhs.iter().chain(rhs.iter()) { - assert!( - attr < num_attributes, - "Dependency {} contains attribute {} which is outside attribute set of size {}", - dep_index, - attr, - num_attributes - ); + if let Some(attribute) = lhs + .iter() + .chain(rhs.iter()) + .find(|&&attribute| attribute >= num_attributes) + { + return Err(format!("dependency {index} contains attribute {attribute} outside attribute set of size {num_attributes}").into()); } } - - Self { + Ok(Self { num_attributes, dependencies, - } + }) } /// Return the number of attributes. diff --git a/src/models/set/minimum_hitting_set.rs b/src/models/set/minimum_hitting_set.rs index ca2aa38d2..7ad1038b6 100644 --- a/src/models/set/minimum_hitting_set.rs +++ b/src/models/set/minimum_hitting_set.rs @@ -26,11 +26,26 @@ inventory::submit! { /// Given a universe `U` and a collection of subsets of `U`, find a minimum-size /// subset `H ⊆ U` such that `H` intersects every set in the collection. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MinimumHittingSetData")] pub struct MinimumHittingSet { universe_size: usize, sets: Vec>, } +#[derive(Deserialize)] +struct MinimumHittingSetData { + universe_size: usize, + sets: Vec>, +} + +impl TryFrom for MinimumHittingSet { + type Error = crate::registry::ConstructionError; + + fn try_from(data: MinimumHittingSetData) -> Result { + Self::try_new(data.universe_size, data.sets) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct MinimumHittingSetCreateSpec { /// Size of the universe U. @@ -43,16 +58,7 @@ impl TryFrom for MinimumHittingSet { type Error = crate::registry::ConstructionError; fn try_from(spec: MinimumHittingSetCreateSpec) -> Result { - for (set_index, set) in spec.subsets.iter().enumerate() { - if let Some(&element) = set.iter().find(|&&element| element >= spec.universe_size) { - return Err(format!( - "subsets[{set_index}] contains element {element} outside universe of size {}", - spec.universe_size - ) - .into()); - } - } - Ok(Self::new(spec.universe_size, spec.subsets)) + Self::try_new(spec.universe_size, spec.subsets) } } @@ -63,22 +69,24 @@ impl MinimumHittingSet { /// /// Panics if any set contains an element outside `0..universe_size`. pub fn new(universe_size: usize, sets: Vec>) -> Self { - let mut sets = sets; - for (set_index, set) in sets.iter_mut().enumerate() { + Self::try_new(universe_size, sets).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + universe_size: usize, + mut sets: Vec>, + ) -> Result { + for (index, set) in sets.iter_mut().enumerate() { set.sort_unstable(); set.dedup(); - for &element in set.iter() { - assert!( - element < universe_size, - "Set {set_index} contains element {element} which is outside universe of size {universe_size}" - ); + if let Some(element) = set.iter().find(|&&element| element >= universe_size) { + return Err(format!("set {index} contains element {element} outside universe of size {universe_size}").into()); } } - - Self { + Ok(Self { universe_size, sets, - } + }) } /// Get the universe size. diff --git a/src/models/set/prime_attribute_name.rs b/src/models/set/prime_attribute_name.rs index 42a3c1839..59e620c13 100644 --- a/src/models/set/prime_attribute_name.rs +++ b/src/models/set/prime_attribute_name.rs @@ -60,6 +60,7 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "PrimeAttributeNameData")] pub struct PrimeAttributeName { /// Number of attributes (elements are 0..num_attributes). num_attributes: usize, @@ -69,6 +70,21 @@ pub struct PrimeAttributeName { query_attribute: usize, } +#[derive(Deserialize)] +struct PrimeAttributeNameData { + num_attributes: usize, + dependencies: Vec<(Vec, Vec)>, + query_attribute: usize, +} + +impl TryFrom for PrimeAttributeName { + type Error = crate::registry::ConstructionError; + + fn try_from(data: PrimeAttributeNameData) -> Result { + Self::try_new(data.num_attributes, data.dependencies, data.query_attribute) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct PrimeAttributeNameCreateSpec { /// Number of attributes. @@ -83,35 +99,7 @@ impl TryFrom for PrimeAttributeName { type Error = crate::registry::ConstructionError; fn try_from(spec: PrimeAttributeNameCreateSpec) -> Result { - if spec.query_attribute >= spec.universe_size { - return Err(format!( - "query_attribute {} is outside universe of size {}", - spec.query_attribute, spec.universe_size - ) - .into()); - } - for (dependency_index, (lhs, rhs)) in spec.dependencies.iter().enumerate() { - if lhs.is_empty() { - return Err( - format!("dependencies[{dependency_index}] has an empty left side").into(), - ); - } - if let Some(&attribute) = lhs - .iter() - .chain(rhs) - .find(|&&attribute| attribute >= spec.universe_size) - { - return Err(format!( - "dependencies[{dependency_index}] contains attribute {attribute} outside universe of size {}", - spec.universe_size - ).into()); - } - } - Ok(Self::new( - spec.universe_size, - spec.dependencies, - spec.query_attribute, - )) + Self::try_new(spec.universe_size, spec.dependencies, spec.query_attribute) } } @@ -127,29 +115,35 @@ impl PrimeAttributeName { dependencies: Vec<(Vec, Vec)>, query_attribute: usize, ) -> Self { - assert!( - query_attribute < num_attributes, - "Query attribute {} is outside attribute set of size {}", - query_attribute, - num_attributes - ); - for (i, (lhs, rhs)) in dependencies.iter().enumerate() { - assert!(!lhs.is_empty(), "Dependency {} has empty LHS", i); - for &attr in lhs.iter().chain(rhs.iter()) { - assert!( - attr < num_attributes, - "Dependency {} references attribute {} which is outside attribute set of size {}", - i, - attr, - num_attributes - ); + Self::try_new(num_attributes, dependencies, query_attribute) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_attributes: usize, + dependencies: Vec<(Vec, Vec)>, + query_attribute: usize, + ) -> Result { + if query_attribute >= num_attributes { + return Err(format!("Query attribute {query_attribute} is outside attribute set of size {num_attributes}").into()); + } + for (index, (lhs, rhs)) in dependencies.iter().enumerate() { + if lhs.is_empty() { + return Err(format!("Dependency {index} has empty LHS").into()); + } + if let Some(attribute) = lhs + .iter() + .chain(rhs.iter()) + .find(|&&attribute| attribute >= num_attributes) + { + return Err(format!("dependency {index} contains attribute {attribute} outside attribute set of size {num_attributes}").into()); } } - Self { + Ok(Self { num_attributes, dependencies, query_attribute, - } + }) } /// Get the number of attributes. diff --git a/src/models/set/set_basis.rs b/src/models/set/set_basis.rs index be76ee298..278b24b0a 100644 --- a/src/models/set/set_basis.rs +++ b/src/models/set/set_basis.rs @@ -28,6 +28,7 @@ inventory::submit! { /// `S` such that every set in `C` can be expressed as the union of some /// subcollection of `B`. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "SetBasisData")] pub struct SetBasis { /// Size of the universe (elements are `0..universe_size`). universe_size: usize, @@ -37,6 +38,21 @@ pub struct SetBasis { k: usize, } +#[derive(Deserialize)] +struct SetBasisData { + universe_size: usize, + collection: Vec>, + k: usize, +} + +impl TryFrom for SetBasis { + type Error = crate::registry::ConstructionError; + + fn try_from(data: SetBasisData) -> Result { + Self::try_new(data.universe_size, data.collection, data.k) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct SetBasisCreateSpec { /// Size of the ground set S. @@ -51,16 +67,7 @@ impl TryFrom for SetBasis { type Error = crate::registry::ConstructionError; fn try_from(spec: SetBasisCreateSpec) -> Result { - for (set_index, set) in spec.subsets.iter().enumerate() { - if let Some(&element) = set.iter().find(|&&element| element >= spec.universe_size) { - return Err(format!( - "subsets[{set_index}] contains element {element} outside universe of size {}", - spec.universe_size - ) - .into()); - } - } - Ok(Self::new(spec.universe_size, spec.subsets, spec.k)) + Self::try_new(spec.universe_size, spec.subsets, spec.k) } } @@ -71,26 +78,26 @@ impl SetBasis { /// /// Panics if any element in `collection` lies outside the universe. pub fn new(universe_size: usize, collection: Vec>, k: usize) -> Self { - let mut collection = collection; - for (set_index, set) in collection.iter_mut().enumerate() { + Self::try_new(universe_size, collection, k).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + universe_size: usize, + mut collection: Vec>, + k: usize, + ) -> Result { + for (index, set) in collection.iter_mut().enumerate() { set.sort_unstable(); set.dedup(); - for &element in set.iter() { - assert!( - element < universe_size, - "Set {} contains element {} which is outside universe of size {}", - set_index, - element, - universe_size - ); + if let Some(element) = set.iter().find(|&&element| element >= universe_size) { + return Err(format!("set {index} contains element {element} outside universe of size {universe_size}").into()); } } - - Self { + Ok(Self { universe_size, collection, k, - } + }) } /// Return the universe size. @@ -158,9 +165,6 @@ impl SetBasis { fn can_represent_target(basis: &[Vec], target: &[usize], universe_size: usize) -> bool { let mut target_membership = vec![false; universe_size]; for &element in target { - if element >= universe_size { - return false; - } target_membership[element] = true; } diff --git a/src/models/set/three_dimensional_matching.rs b/src/models/set/three_dimensional_matching.rs index 6b7fb2641..829caa6fc 100644 --- a/src/models/set/three_dimensional_matching.rs +++ b/src/models/set/three_dimensional_matching.rs @@ -56,6 +56,7 @@ inventory::submit! { /// assert!(problem.evaluate(&solutions[0]).unwrap()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ThreeDimensionalMatchingData")] pub struct ThreeDimensionalMatching { /// Size of each set W, X, Y (elements are 0..universe_size). universe_size: usize, @@ -63,6 +64,20 @@ pub struct ThreeDimensionalMatching { triples: Vec<(usize, usize, usize)>, } +#[derive(Deserialize)] +struct ThreeDimensionalMatchingData { + universe_size: usize, + triples: Vec<(usize, usize, usize)>, +} + +impl TryFrom for ThreeDimensionalMatching { + type Error = crate::registry::ConstructionError; + + fn try_from(data: ThreeDimensionalMatchingData) -> Result { + Self::try_new(data.universe_size, data.triples) + } +} + impl ThreeDimensionalMatching { /// Create a new 3DM problem. /// @@ -70,33 +85,25 @@ impl ThreeDimensionalMatching { /// /// Panics if any triple contains an element outside 0..universe_size. pub fn new(universe_size: usize, triples: Vec<(usize, usize, usize)>) -> Self { - for (i, &(w, x, y)) in triples.iter().enumerate() { - assert!( - w < universe_size, - "Triple {} has w-coordinate {} which is outside 0..{}", - i, - w, - universe_size - ); - assert!( - x < universe_size, - "Triple {} has x-coordinate {} which is outside 0..{}", - i, - x, - universe_size - ); - assert!( - y < universe_size, - "Triple {} has y-coordinate {} which is outside 0..{}", - i, - y, - universe_size - ); + Self::try_new(universe_size, triples).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + universe_size: usize, + triples: Vec<(usize, usize, usize)>, + ) -> Result { + for (index, &(w, x, y)) in triples.iter().enumerate() { + if w >= universe_size || x >= universe_size || y >= universe_size { + return Err(format!( + "triple {index} contains a coordinate outside 0..{universe_size}" + ) + .into()); + } } - Self { + Ok(Self { universe_size, triples, - } + }) } /// Get the universe size (q). diff --git a/src/topology/bipartite_graph.rs b/src/topology/bipartite_graph.rs index a99d5dbdf..28f02e7e0 100644 --- a/src/topology/bipartite_graph.rs +++ b/src/topology/bipartite_graph.rs @@ -22,6 +22,7 @@ use serde::{Deserialize, Serialize}; /// assert!(g.has_edge(0, 2)); // left 0 -> right 0 (unified index 2) /// ``` #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(try_from = "BipartiteGraphData")] pub struct BipartiteGraph { left_size: usize, right_size: usize, @@ -29,6 +30,20 @@ pub struct BipartiteGraph { edges: Vec<(usize, usize)>, } +#[derive(Deserialize)] +struct BipartiteGraphData { + left_size: usize, + right_size: usize, + edges: Vec<(usize, usize)>, +} + +impl TryFrom for BipartiteGraph { + type Error = crate::registry::ConstructionError; + fn try_from(data: BipartiteGraphData) -> Result { + Self::try_new(data.left_size, data.right_size, data.edges) + } +} + impl BipartiteGraph { /// Create a new bipartite graph. /// @@ -40,27 +55,39 @@ impl BipartiteGraph { /// /// # Panics /// - /// Panics if any edge references an out-of-bounds left or right vertex index. + /// Panics if any edge references an out-of-bounds left or right vertex index, + /// or if the combined vertex count overflows `usize`. pub fn new(left_size: usize, right_size: usize, edges: Vec<(usize, usize)>) -> Self { + Self::try_new(left_size, right_size, edges).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + left_size: usize, + right_size: usize, + edges: Vec<(usize, usize)>, + ) -> Result { + left_size + .checked_add(right_size) + .ok_or("bipartite vertex count overflows usize")?; for &(u, v) in &edges { - assert!( - u < left_size, - "left vertex {} out of bounds (left_size={})", - u, - left_size - ); - assert!( - v < right_size, - "right vertex {} out of bounds (right_size={})", - v, - right_size - ); + if u >= left_size { + return Err( + format!("left vertex {} out of bounds (left_size={})", u, left_size).into(), + ); + } + if v >= right_size { + return Err(format!( + "right vertex {} out of bounds (right_size={})", + v, right_size + ) + .into()); + } } - Self { + Ok(Self { left_size, right_size, edges, - } + }) } /// Returns the number of vertices in the left partition. diff --git a/src/topology/directed_graph.rs b/src/topology/directed_graph.rs index 84fe30027..1b30bb469 100644 --- a/src/topology/directed_graph.rs +++ b/src/topology/directed_graph.rs @@ -53,21 +53,28 @@ impl DirectedGraph { /// /// Panics if any arc references a vertex index >= `num_vertices`. pub fn new(num_vertices: usize, arcs: Vec<(usize, usize)>) -> Self { + Self::try_new(num_vertices, arcs).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + arcs: Vec<(usize, usize)>, + ) -> Result { let mut inner = DiGraph::new(); for _ in 0..num_vertices { inner.add_node(()); } for (u, v) in arcs { - assert!( - u < num_vertices && v < num_vertices, - "arc ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "arc ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } inner.add_edge(NodeIndex::new(u), NodeIndex::new(v), ()); } - Self { inner } + Ok(Self { inner }) } /// Creates an empty directed graph with the given number of vertices and no arcs. @@ -263,7 +270,7 @@ impl<'de> Deserialize<'de> for DirectedGraph { arcs: Vec<(usize, usize)>, } let data = GraphData::deserialize(deserializer)?; - Ok(DirectedGraph::new(data.num_vertices, data.arcs)) + DirectedGraph::try_new(data.num_vertices, data.arcs).map_err(serde::de::Error::custom) } } diff --git a/src/topology/graph.rs b/src/topology/graph.rs index 263b64aeb..b21d5d223 100644 --- a/src/topology/graph.rs +++ b/src/topology/graph.rs @@ -116,21 +116,28 @@ impl SimpleGraph { /// /// Panics if any edge references a vertex index >= num_vertices. pub fn new(num_vertices: usize, edges: Vec<(usize, usize)>) -> Self { + Self::try_new(num_vertices, edges).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + edges: Vec<(usize, usize)>, + ) -> Result { let mut inner = UnGraph::new_undirected(); for _ in 0..num_vertices { inner.add_node(()); } for (u, v) in edges { - assert!( - u < num_vertices && v < num_vertices, - "edge ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "edge ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } inner.add_edge(NodeIndex::new(u), NodeIndex::new(v), ()); } - Self { inner } + Ok(Self { inner }) } /// Creates an empty graph with the given number of vertices. @@ -279,7 +286,7 @@ impl<'de> Deserialize<'de> for SimpleGraph { edges: Vec<(usize, usize)>, } let data = GraphData::deserialize(deserializer)?; - Ok(SimpleGraph::new(data.num_vertices, data.edges)) + SimpleGraph::try_new(data.num_vertices, data.edges).map_err(serde::de::Error::custom) } } diff --git a/src/topology/mixed_graph.rs b/src/topology/mixed_graph.rs index 9d95fbac9..af69ea157 100644 --- a/src/topology/mixed_graph.rs +++ b/src/topology/mixed_graph.rs @@ -12,12 +12,27 @@ use serde::{Deserialize, Serialize}; /// so higher-level models can use that order as part of their configuration /// semantics, but edge-membership queries treat them as unordered pairs. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MixedGraphData")] pub struct MixedGraph { num_vertices: usize, arcs: Vec<(usize, usize)>, edges: Vec<(usize, usize)>, } +#[derive(Deserialize)] +struct MixedGraphData { + num_vertices: usize, + arcs: Vec<(usize, usize)>, + edges: Vec<(usize, usize)>, +} + +impl TryFrom for MixedGraph { + type Error = crate::registry::ConstructionError; + fn try_from(data: MixedGraphData) -> Result { + Self::try_new(data.num_vertices, data.arcs, data.edges) + } +} + impl MixedGraph { /// Create a new mixed graph. /// @@ -25,31 +40,39 @@ impl MixedGraph { /// /// Panics if any endpoint references a vertex outside `0..num_vertices`. pub fn new(num_vertices: usize, arcs: Vec<(usize, usize)>, edges: Vec<(usize, usize)>) -> Self { + Self::try_new(num_vertices, arcs, edges).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + arcs: Vec<(usize, usize)>, + edges: Vec<(usize, usize)>, + ) -> Result { for &(u, v) in &arcs { - assert!( - u < num_vertices && v < num_vertices, - "arc ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "arc ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } } for &(u, v) in &edges { - assert!( - u < num_vertices && v < num_vertices, - "edge ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "edge ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } } - Self { + Ok(Self { num_vertices, arcs, edges, - } + }) } /// Create an empty mixed graph with no arcs or undirected edges. diff --git a/src/topology/planar_graph.rs b/src/topology/planar_graph.rs index a29a2b042..868eb7484 100644 --- a/src/topology/planar_graph.rs +++ b/src/topology/planar_graph.rs @@ -19,7 +19,7 @@ use serde::{Deserialize, Serialize}; /// assert_eq!(g.num_vertices(), 4); /// assert_eq!(g.num_edges(), 6); /// ``` -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize)] pub struct PlanarGraph { inner: SimpleGraph, } @@ -30,24 +30,35 @@ impl PlanarGraph { /// # Panics /// Panics if the graph violates the necessary planarity condition |E| <= 3|V| - 6. pub fn new(num_vertices: usize, edges: Vec<(usize, usize)>) -> Self { - let inner = SimpleGraph::new(num_vertices, edges); - if num_vertices >= 3 { - let max_edges = 3 * num_vertices - 6; - assert!( - inner.num_edges() <= max_edges, - "graph has {} edges but a planar graph on {} vertices can have at most {} edges", - inner.num_edges(), - num_vertices, - max_edges - ); - } - Self { inner } + Self::from_inner(SimpleGraph::new(num_vertices, edges)) + .unwrap_or_else(|error| panic!("{error}")) } /// Get a reference to the underlying SimpleGraph. pub fn inner(&self) -> &SimpleGraph { &self.inner } + fn from_inner(inner: SimpleGraph) -> Result { + let num_vertices = inner.num_vertices(); + if num_vertices >= 3 { + let max_edges = 3 * (num_vertices as u128) - 6; + if inner.num_edges() as u128 > max_edges { + return Err(format!("graph has {} edges but a planar graph on {num_vertices} vertices can have at most {max_edges} edges", inner.num_edges()).into()); + } + } + Ok(Self { inner }) + } +} + +impl<'de> Deserialize<'de> for PlanarGraph { + fn deserialize>(deserializer: D) -> Result { + #[derive(Deserialize)] + struct PlanarGraphData { + inner: SimpleGraph, + } + let data = PlanarGraphData::deserialize(deserializer)?; + Self::from_inner(data.inner).map_err(serde::de::Error::custom) + } } impl Graph for PlanarGraph { diff --git a/src/unit_tests/models/misc/additional_key.rs b/src/unit_tests/models/misc/additional_key.rs index 8df337615..58c70a901 100644 --- a/src/unit_tests/models/misc/additional_key.rs +++ b/src/unit_tests/models/misc/additional_key.rs @@ -1,4 +1,31 @@ use super::*; + +#[test] +fn test_additional_key_validates_persisted_input() { + let valid = serde_json::to_value(AdditionalKey::new( + 2, + vec![(vec![0], vec![1])], + vec![0, 1], + vec![vec![0]], + )) + .unwrap(); + let restored: AdditionalKey = serde_json::from_value(valid.clone()).unwrap(); + assert_eq!(serde_json::to_value(restored).unwrap(), valid); + for (field, value) in [ + ("relation_attrs", serde_json::json!([2])), + ("relation_attrs", serde_json::json!([0, 0])), + ("dependencies", serde_json::json!([[[2], [0]]])), + ("dependencies", serde_json::json!([[[0], [2]]])), + ("known_keys", serde_json::json!([[2]])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::traits::Problem; diff --git a/src/unit_tests/models/misc/boyce_codd_normal_form_violation.rs b/src/unit_tests/models/misc/boyce_codd_normal_form_violation.rs index a15ea8bd2..4b7c53120 100644 --- a/src/unit_tests/models/misc/boyce_codd_normal_form_violation.rs +++ b/src/unit_tests/models/misc/boyce_codd_normal_form_violation.rs @@ -1,4 +1,29 @@ use super::*; + +#[test] +fn test_boyce_codd_normal_form_violation_validates_persisted_input() { + let valid = serde_json::to_value(BoyceCoddNormalFormViolation::new( + 2, + vec![(vec![0], vec![1])], + vec![0, 1], + )) + .unwrap(); + let restored: BoyceCoddNormalFormViolation = serde_json::from_value(valid.clone()).unwrap(); + assert_eq!(serde_json::to_value(restored).unwrap(), valid); + for (field, value) in [ + ("target_subset", serde_json::json!([])), + ("target_subset", serde_json::json!([2])), + ("functional_deps", serde_json::json!([[[], [1]]])), + ("functional_deps", serde_json::json!([[[0], [2]]])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::traits::Problem; diff --git a/src/unit_tests/models/misc/conjunctive_boolean_query.rs b/src/unit_tests/models/misc/conjunctive_boolean_query.rs index 165eb95a0..3645dda91 100644 --- a/src/unit_tests/models/misc/conjunctive_boolean_query.rs +++ b/src/unit_tests/models/misc/conjunctive_boolean_query.rs @@ -1,4 +1,35 @@ use super::*; + +#[test] +fn test_conjunctive_boolean_query_validates_persisted_input() { + let valid = serde_json::to_value(ConjunctiveBooleanQuery::new( + 2, + vec![Relation { + arity: 1, + tuples: vec![vec![0]], + }], + 1, + vec![(0, vec![QueryArg::Variable(0)])], + )) + .unwrap(); + let restored: ConjunctiveBooleanQuery = serde_json::from_value(valid.clone()).unwrap(); + assert_eq!(serde_json::to_value(restored).unwrap(), valid); + for (field, value) in [ + ("relations", serde_json::json!([{"arity":1,"tuples":[[]]}])), + ("relations", serde_json::json!([{"arity":1,"tuples":[[2]]}])), + ("conjuncts", serde_json::json!([[1,[{"Variable":0}]]])), + ("conjuncts", serde_json::json!([[0, []]])), + ("conjuncts", serde_json::json!([[0,[{"Variable":1}]]])), + ("conjuncts", serde_json::json!([[0,[{"Constant":2}]]])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::traits::Problem; diff --git a/src/unit_tests/models/misc/conjunctive_query_foldability.rs b/src/unit_tests/models/misc/conjunctive_query_foldability.rs index 29fc69f39..57010afbf 100644 --- a/src/unit_tests/models/misc/conjunctive_query_foldability.rs +++ b/src/unit_tests/models/misc/conjunctive_query_foldability.rs @@ -1,4 +1,42 @@ use super::*; + +#[test] +fn test_conjunctive_query_foldability_validates_persisted_input() { + let valid = serde_json::to_value(ConjunctiveQueryFoldability::new( + 1, + 1, + 1, + vec![1], + vec![(0, vec![Term::Distinguished(0)])], + vec![(0, vec![Term::Undistinguished(0)])], + )) + .unwrap(); + let restored: ConjunctiveQueryFoldability = serde_json::from_value(valid.clone()).unwrap(); + assert_eq!(serde_json::to_value(restored).unwrap(), valid); + for (field, value) in [ + ("query1_conjuncts", serde_json::json!([[1, []]])), + ("query1_conjuncts", serde_json::json!([[0, []]])), + ( + "query1_conjuncts", + serde_json::json!([[0,[{"Constant":1}]]]), + ), + ( + "query1_conjuncts", + serde_json::json!([[0,[{"Distinguished":1}]]]), + ), + ( + "query2_conjuncts", + serde_json::json!([[0,[{"Undistinguished":1}]]]), + ), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::traits::Problem; diff --git a/src/unit_tests/models/misc/consistency_of_database_frequency_tables.rs b/src/unit_tests/models/misc/consistency_of_database_frequency_tables.rs index 1aadc7b88..eabdcf4a9 100644 --- a/src/unit_tests/models/misc/consistency_of_database_frequency_tables.rs +++ b/src/unit_tests/models/misc/consistency_of_database_frequency_tables.rs @@ -1,4 +1,23 @@ use super::*; + +#[test] +fn test_consistency_of_database_frequency_tables_validates_persisted_input() { + let valid = serde_json::to_value(issue_yes_instance()).unwrap(); + let restored: ConsistencyOfDatabaseFrequencyTables = + serde_json::from_value(valid.clone()).unwrap(); + assert_eq!(serde_json::to_value(restored).unwrap(), valid); + for (field, value) in [ + ("attribute_domains", serde_json::json!([0, 3, 2])), + ("num_objects", serde_json::json!(0)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForceProblem as _; #[test] diff --git a/src/unit_tests/models/set/consecutive_sets.rs b/src/unit_tests/models/set/consecutive_sets.rs index 4c94c0afa..0bb7fdf3c 100644 --- a/src/unit_tests/models/set/consecutive_sets.rs +++ b/src/unit_tests/models/set/consecutive_sets.rs @@ -139,3 +139,10 @@ fn test_consecutive_sets_duplicate_elements() { fn test_consecutive_sets_zero_bound() { ConsecutiveSets::new(3, vec![vec![0, 1]], 0); } + +#[test] +fn json_rejects_invalid_instance() { + let json = serde_json::json!({"alphabet_size":3,"subsets":[[0,0]],"bound_k":3}); + assert!(serde_json::from_value::(json.clone()).is_err()); + assert!(crate::registry::load_dyn("ConsecutiveSets", &Default::default(), json).is_err()); +} diff --git a/src/unit_tests/models/set/exact_cover_by_3_sets.rs b/src/unit_tests/models/set/exact_cover_by_3_sets.rs index a7cdf05cc..a4d877b66 100644 --- a/src/unit_tests/models/set/exact_cover_by_3_sets.rs +++ b/src/unit_tests/models/set/exact_cover_by_3_sets.rs @@ -169,3 +169,27 @@ fn test_exact_cover_by_3_sets_element_out_of_range() { fn test_exact_cover_by_3_sets_duplicate_elements() { ExactCoverBy3Sets::new(6, vec![[0, 0, 1]]); } + +#[test] +fn construction_and_json_reject_invalid_triples() { + for (universe_size, subsets) in [ + (5, vec![[0, 1, 2]]), + (6, vec![[0, 1, 7]]), + (6, vec![[0, 0, 1]]), + ] { + let json = serde_json::json!({"universe_size": universe_size, "subsets": subsets}); + assert!(serde_json::from_value::(json.clone()).is_err()); + assert!(crate::registry::load_dyn("ExactCoverBy3Sets", &Default::default(), json).is_err()); + } +} + +#[test] +fn construction_and_json_sort_triples() { + let expected = ExactCoverBy3Sets::new(3, vec![[2, 0, 1]]); + let loaded: ExactCoverBy3Sets = serde_json::from_value(serde_json::json!({ + "universe_size": 3, "subsets": [[2, 0, 1]] + })) + .unwrap(); + assert_eq!(expected.subsets(), &[[0, 1, 2]]); + assert_eq!(loaded.subsets(), expected.subsets()); +} diff --git a/src/unit_tests/models/set/minimum_cardinality_key.rs b/src/unit_tests/models/set/minimum_cardinality_key.rs index d73369de6..3a941556d 100644 --- a/src/unit_tests/models/set/minimum_cardinality_key.rs +++ b/src/unit_tests/models/set/minimum_cardinality_key.rs @@ -177,3 +177,10 @@ fn test_minimum_cardinality_key_paper_example() { let witness = solver.solve(&problem).unwrap().unwrap(); assert_eq!(witness, solution); } + +#[test] +fn json_rejects_invalid_instance() { + let json = serde_json::json!({"num_attributes":3,"dependencies":[[[0,3],[1]]]}); + assert!(serde_json::from_value::(json.clone()).is_err()); + assert!(crate::registry::load_dyn("MinimumCardinalityKey", &Default::default(), json).is_err()); +} diff --git a/src/unit_tests/models/set/minimum_hitting_set.rs b/src/unit_tests/models/set/minimum_hitting_set.rs index 2b7a59b5f..566abf56b 100644 --- a/src/unit_tests/models/set/minimum_hitting_set.rs +++ b/src/unit_tests/models/set/minimum_hitting_set.rs @@ -176,3 +176,10 @@ fn test_minimum_hitting_set_canonical_example_spec() { let best = solver.solve(&problem).unwrap().unwrap(); assert_eq!(problem.evaluate(&best).unwrap(), Min(Some(3))); } + +#[test] +fn json_rejects_invalid_instance() { + let json = serde_json::json!({"universe_size":3,"sets":[[0,3]]}); + assert!(serde_json::from_value::(json.clone()).is_err()); + assert!(crate::registry::load_dyn("MinimumHittingSet", &Default::default(), json).is_err()); +} diff --git a/src/unit_tests/models/set/prime_attribute_name.rs b/src/unit_tests/models/set/prime_attribute_name.rs index e9550959a..11abd0bd4 100644 --- a/src/unit_tests/models/set/prime_attribute_name.rs +++ b/src/unit_tests/models/set/prime_attribute_name.rs @@ -210,3 +210,11 @@ fn test_prime_attribute_name_empty_lhs() { fn test_prime_attribute_name_dep_out_of_range() { PrimeAttributeName::new(3, vec![(vec![0], vec![5])], 0); } + +#[test] +fn json_rejects_invalid_instance() { + let json = + serde_json::json!({"num_attributes":3,"dependencies":[[[],[1]]],"query_attribute":0}); + assert!(serde_json::from_value::(json.clone()).is_err()); + assert!(crate::registry::load_dyn("PrimeAttributeName", &Default::default(), json).is_err()); +} diff --git a/src/unit_tests/models/set/set_basis.rs b/src/unit_tests/models/set/set_basis.rs index a1c0aabdd..40706c8c0 100644 --- a/src/unit_tests/models/set/set_basis.rs +++ b/src/unit_tests/models/set/set_basis.rs @@ -130,20 +130,6 @@ fn test_set_basis_rejects_wrong_config_length() { assert!(problem.evaluate(&solution).is_err()); } -#[test] -fn test_set_basis_deserialized_invalid_target_returns_false() { - let problem: SetBasis = serde_json::from_value(serde_json::json!({ - "universe_size": 4, - "collection": [[0, 4]], - "k": 1 - })) - .unwrap(); - - assert!(!problem - .evaluate(&vec![vec![true, false, false, false]]) - .unwrap()); -} - #[test] fn test_set_basis_deserialized_unsorted_target_still_evaluates_correctly() { let problem: SetBasis = serde_json::from_value(serde_json::json!({ @@ -205,3 +191,10 @@ fn test_set_basis_empty_collection_with_k_positive() { assert!(problem.evaluate(&vec![vec![false; 2]; 2]).unwrap()); assert!(problem.evaluate(&vec![vec![true; 2]; 2]).unwrap()); } + +#[test] +fn json_rejects_invalid_instance() { + let json = serde_json::json!({"universe_size":3,"collection":[[0,3]],"k":1}); + assert!(serde_json::from_value::(json.clone()).is_err()); + assert!(crate::registry::load_dyn("SetBasis", &Default::default(), json).is_err()); +} diff --git a/src/unit_tests/models/set/three_dimensional_matching.rs b/src/unit_tests/models/set/three_dimensional_matching.rs index 3ef8b0df4..1a1c9a022 100644 --- a/src/unit_tests/models/set/three_dimensional_matching.rs +++ b/src/unit_tests/models/set/three_dimensional_matching.rs @@ -152,3 +152,12 @@ fn test_three_dimensional_matching_duplicate_coordinates() { assert!(!problem.evaluate(&vec![true, false, true]).unwrap()); // T0+T2: w={0,0} not distinct assert!(!problem.evaluate(&vec![false, true, true]).unwrap()); // T1+T2: x={1,1} not distinct } + +#[test] +fn json_rejects_invalid_instance() { + let json = serde_json::json!({"universe_size":2,"triples":[[0,2,0]]}); + assert!(serde_json::from_value::(json.clone()).is_err()); + assert!( + crate::registry::load_dyn("ThreeDimensionalMatching", &Default::default(), json).is_err() + ); +} diff --git a/src/unit_tests/topology/bipartite_graph.rs b/src/unit_tests/topology/bipartite_graph.rs index 46e3b381a..0aa6dd7e8 100644 --- a/src/unit_tests/topology/bipartite_graph.rs +++ b/src/unit_tests/topology/bipartite_graph.rs @@ -58,3 +58,22 @@ fn test_bipartite_graph_invalid_left_index() { fn test_bipartite_graph_invalid_right_index() { BipartiteGraph::new(2, 2, vec![(0, 2)]); } + +#[test] +fn deserialize_checks_partition_endpoints_and_total_size() { + for edges in [vec![(1, 0)], vec![(0, 1)]] { + assert!(serde_json::from_value::(serde_json::json!({ + "left_size": 1, "right_size": 1, "edges": edges + })) + .is_err()); + } + assert!(serde_json::from_value::(serde_json::json!({ + "left_size": usize::MAX, "right_size": 1, "edges": [] + })) + .is_err()); + let graph: BipartiteGraph = serde_json::from_value(serde_json::json!({ + "left_size": 1, "right_size": 1, "edges": [[0, 0]] + })) + .unwrap(); + assert_eq!(graph.edges(), vec![(0, 1)]); +} diff --git a/src/unit_tests/topology/directed_graph.rs b/src/unit_tests/topology/directed_graph.rs index 086728ae0..98b8e420b 100644 --- a/src/unit_tests/topology/directed_graph.rs +++ b/src/unit_tests/topology/directed_graph.rs @@ -234,3 +234,11 @@ fn test_directed_graph_json_format() { fn test_directed_graph_invalid_arc() { DirectedGraph::new(3, vec![(0, 5)]); } + +#[test] +fn deserialize_rejects_out_of_range_arcs() { + assert!(serde_json::from_value::(serde_json::json!({ + "num_vertices": 2, "arcs": [[2, 0]] + })) + .is_err()); +} diff --git a/src/unit_tests/topology/graph.rs b/src/unit_tests/topology/graph.rs index da89fb5ba..f98e2e4d2 100644 --- a/src/unit_tests/topology/graph.rs +++ b/src/unit_tests/topology/graph.rs @@ -153,3 +153,11 @@ fn test_simplegraph_json_format() { assert!(!json_str.contains("node_holes")); assert!(json_str.contains("num_vertices")); } + +#[test] +fn deserialize_rejects_out_of_range_edges() { + assert!(serde_json::from_value::(serde_json::json!({ + "num_vertices": 2, "edges": [[0, 2]] + })) + .is_err()); +} diff --git a/src/unit_tests/topology/mixed_graph.rs b/src/unit_tests/topology/mixed_graph.rs index 316e4ac02..8dcea1800 100644 --- a/src/unit_tests/topology/mixed_graph.rs +++ b/src/unit_tests/topology/mixed_graph.rs @@ -56,3 +56,13 @@ fn test_mixed_graph_serialization_roundtrip() { fn test_mixed_graph_panics_on_out_of_bounds_arc() { MixedGraph::new(3, vec![(0, 3)], vec![]); } + +#[test] +fn deserialize_checks_both_arc_and_edge_endpoints() { + for (arcs, edges) in [(vec![(0, 2)], vec![]), (vec![], vec![(2, 0)])] { + assert!(serde_json::from_value::(serde_json::json!({ + "num_vertices": 2, "arcs": arcs, "edges": edges + })) + .is_err()); + } +} diff --git a/src/unit_tests/topology/planar_graph.rs b/src/unit_tests/topology/planar_graph.rs index 7fae09cb6..463985412 100644 --- a/src/unit_tests/topology/planar_graph.rs +++ b/src/unit_tests/topology/planar_graph.rs @@ -44,3 +44,19 @@ fn test_planar_graph_tree() { let g = PlanarGraph::new(4, vec![(0, 1), (1, 2), (2, 3)]); assert_eq!(g.num_edges(), 3); } + +#[test] +fn deserialize_checks_the_edge_bound() { + let edges: Vec<_> = (0..5) + .flat_map(|u| ((u + 1)..5).map(move |v| (u, v))) + .collect(); + assert!(serde_json::from_value::(serde_json::json!({ + "inner": {"num_vertices": 5, "edges": edges} + })) + .is_err()); + let graph: PlanarGraph = serde_json::from_value(serde_json::json!({ + "inner": {"num_vertices": 2, "edges": [[0, 1]]} + })) + .unwrap(); + assert_eq!(graph.num_edges(), 1); +}