From 6fec366452da30fa6d149aa7c5f4984495a359c1 Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 01:28:03 +0800 Subject: [PATCH 1/5] Validate string model loading and rebuild derived length bounds --- src/models/misc/closest_string.rs | 61 ++++++++++------ src/models/misc/grouping_by_swapping.rs | 71 +++++++++--------- src/models/misc/longest_common_subsequence.rs | 66 +++++++++-------- .../misc/shortest_common_supersequence.rs | 59 +++++++++------ .../misc/shortest_common_superstring.rs | 43 ++++++++--- .../misc/string_to_string_correction.rs | 72 +++++++++++-------- src/unit_tests/models/misc/closest_string.rs | 11 +++ .../models/misc/grouping_by_swapping.rs | 10 +++ .../models/misc/longest_common_subsequence.rs | 20 ++++++ .../misc/shortest_common_supersequence.rs | 20 ++++++ .../misc/shortest_common_superstring.rs | 20 ++++++ .../misc/string_to_string_correction.rs | 10 +++ 12 files changed, 317 insertions(+), 146 deletions(-) diff --git a/src/models/misc/closest_string.rs b/src/models/misc/closest_string.rs index 722d23dfd..6910a10bd 100644 --- a/src/models/misc/closest_string.rs +++ b/src/models/misc/closest_string.rs @@ -46,11 +46,26 @@ inventory::submit! { /// syntactically feasible; the objective is its worst-case Hamming distance /// to the input strings. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ClosestStringData")] pub struct ClosestString { alphabet_size: usize, strings: Vec>, } +#[derive(Deserialize)] +struct ClosestStringData { + alphabet_size: usize, + strings: Vec>, +} + +impl TryFrom for ClosestString { + type Error = crate::registry::ConstructionError; + + fn try_from(data: ClosestStringData) -> Result { + Self::try_new(data.alphabet_size, data.strings) + } +} + impl ClosestString { /// Create a new `ClosestString` instance. /// @@ -62,30 +77,34 @@ impl ClosestString { /// - `alphabet_size == 0` while any input string is non-empty, /// - any symbol in any input string is `>= alphabet_size`. pub fn new(alphabet_size: usize, strings: Vec>) -> Self { - assert!( - !strings.is_empty(), - "ClosestString requires at least one input string" - ); + Self::try_new(alphabet_size, strings).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + alphabet_size: usize, + strings: Vec>, + ) -> Result { + if strings.is_empty() { + return Err("ClosestString requires at least one input string".into()); + } let string_length = strings[0].len(); - assert!( - strings.iter().all(|s| s.len() == string_length), - "all input strings must have the same length" - ); - assert!( - alphabet_size > 0 || string_length == 0, - "alphabet_size must be > 0 when input strings are non-empty" - ); - assert!( - strings - .iter() - .flat_map(|s| s.iter()) - .all(|&symbol| symbol < alphabet_size), - "input symbols must be less than alphabet_size" - ); - Self { + if !(strings.iter().all(|s| s.len() == string_length)) { + return Err("all input strings must have the same length".into()); + } + if !(alphabet_size > 0 || string_length == 0) { + return Err("alphabet_size must be > 0 when input strings are non-empty".into()); + } + if !(strings + .iter() + .flat_map(|s| s.iter()) + .all(|&symbol| symbol < alphabet_size)) + { + return Err("input symbols must be less than alphabet_size".into()); + } + Ok(Self { alphabet_size, strings, - } + }) } /// Returns the alphabet size `q`. diff --git a/src/models/misc/grouping_by_swapping.rs b/src/models/misc/grouping_by_swapping.rs index 863c0fec6..8b22940c2 100644 --- a/src/models/misc/grouping_by_swapping.rs +++ b/src/models/misc/grouping_by_swapping.rs @@ -27,12 +27,28 @@ inventory::submit! { /// adjacent swap position `i` (swap positions `i` and `i + 1`) or the special /// no-op value `string_len - 1`. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "GroupingBySwappingData")] pub struct GroupingBySwapping { alphabet_size: usize, string: Vec, budget: usize, } +#[derive(Deserialize)] +struct GroupingBySwappingData { + alphabet_size: usize, + string: Vec, + budget: usize, +} + +impl TryFrom for GroupingBySwapping { + type Error = crate::registry::ConstructionError; + + fn try_from(data: GroupingBySwappingData) -> Result { + Self::try_new(data.alphabet_size, data.string, data.budget) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct GroupingBySwappingCreateSpec { /// Optional alphabet size; omitted values are inferred from the string. @@ -61,27 +77,7 @@ impl TryFrom for GroupingBySwapping { .transpose()? .unwrap_or(0); let alphabet_size = spec.alphabet_size.unwrap_or(inferred_alphabet_size); - if alphabet_size < inferred_alphabet_size { - return Err(format!( - "alphabet size {alphabet_size} is smaller than inferred alphabet size {inferred_alphabet_size}" - ).into()); - } - if alphabet_size == 0 && !spec.string.is_empty() { - return Err("alphabet size must be positive for a non-empty string" - .to_string() - .into()); - } - if spec.string.is_empty() && spec.bound != 0 { - return Err("bound must be zero when the string is empty" - .to_string() - .into()); - } - - Ok(Self { - alphabet_size, - string: spec.string, - budget: spec.bound, - }) + Self::try_new(alphabet_size, spec.string, spec.bound) } } @@ -93,23 +89,28 @@ impl GroupingBySwapping { /// Panics if the string contains a symbol outside the declared alphabet, /// or if the string is empty while the budget is positive. pub fn new(alphabet_size: usize, string: Vec, budget: usize) -> Self { - assert!( - alphabet_size > 0 || string.is_empty(), - "alphabet_size must be > 0 when string is non-empty" - ); - assert!( - string.iter().all(|&symbol| symbol < alphabet_size), - "input symbols must be less than alphabet_size" - ); - assert!( - !string.is_empty() || budget == 0, - "budget must be 0 when string is empty" - ); - Self { + Self::try_new(alphabet_size, string, budget).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + alphabet_size: usize, + string: Vec, + budget: usize, + ) -> Result { + if !(alphabet_size > 0 || string.is_empty()) { + return Err("alphabet_size must be > 0 when string is non-empty".into()); + } + if !(string.iter().all(|&symbol| symbol < alphabet_size)) { + return Err("input symbols must be less than alphabet_size".into()); + } + if !(!string.is_empty() || budget == 0) { + return Err("budget must be 0 when string is empty".into()); + } + Ok(Self { alphabet_size, string, budget, - } + }) } /// Returns the alphabet size. diff --git a/src/models/misc/longest_common_subsequence.rs b/src/models/misc/longest_common_subsequence.rs index 15bb0c0d2..1f28fda78 100644 --- a/src/models/misc/longest_common_subsequence.rs +++ b/src/models/misc/longest_common_subsequence.rs @@ -36,12 +36,27 @@ inventory::submit! { /// subsequence consists of the symbols before padding starts. The objective is /// to maximize the effective length. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "LongestCommonSubsequenceData")] pub struct LongestCommonSubsequence { alphabet_size: usize, strings: Vec>, max_length: usize, } +#[derive(Deserialize)] +struct LongestCommonSubsequenceData { + alphabet_size: usize, + strings: Vec>, +} + +impl TryFrom for LongestCommonSubsequence { + type Error = crate::registry::ConstructionError; + + fn try_from(data: LongestCommonSubsequenceData) -> Result { + Self::try_new(data.alphabet_size, data.strings) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct LongestCommonSubsequenceCreateSpec { /// Optional alphabet size; omitted values are inferred from the strings. @@ -74,21 +89,7 @@ impl TryFrom for LongestCommonSubsequence { .transpose()? .unwrap_or(0); let alphabet_size = spec.alphabet_size.unwrap_or(inferred_alphabet_size); - if alphabet_size < inferred_alphabet_size { - return Err(format!( - "alphabet size {alphabet_size} is smaller than inferred alphabet size {inferred_alphabet_size}" - ).into()); - } - if alphabet_size == 0 { - return Err("alphabet size must be positive".to_string().into()); - } - let max_length = spec.strings.iter().map(Vec::len).min().unwrap_or(0); - - Ok(Self { - alphabet_size, - strings: spec.strings, - max_length, - }) + Self::try_new(alphabet_size, spec.strings) } } @@ -101,26 +102,31 @@ impl LongestCommonSubsequence { /// # Panics /// /// Panics if `alphabet_size == 0` and any input string is non-empty, or if - /// an input symbol is outside the declared alphabet, or if all strings are - /// empty (max_length would be 0, requiring at least one non-empty string). + /// an input symbol is outside the declared alphabet. pub fn new(alphabet_size: usize, strings: Vec>) -> Self { + Self::try_new(alphabet_size, strings).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + alphabet_size: usize, + strings: Vec>, + ) -> Result { let max_length = strings.iter().map(|s| s.len()).min().unwrap_or(0); - assert!( - alphabet_size > 0 || strings.iter().all(|s| s.is_empty()), - "alphabet_size must be > 0 when any input string is non-empty" - ); - assert!( - strings - .iter() - .flat_map(|s| s.iter()) - .all(|&symbol| symbol < alphabet_size), - "input symbols must be less than alphabet_size" - ); - Self { + if !(alphabet_size > 0 || strings.iter().all(|s| s.is_empty())) { + return Err("alphabet_size must be > 0 when any input string is non-empty".into()); + } + if !(strings + .iter() + .flat_map(|s| s.iter()) + .all(|&symbol| symbol < alphabet_size)) + { + return Err("input symbols must be less than alphabet_size".into()); + } + Ok(Self { alphabet_size, strings, max_length, - } + }) } /// Returns the alphabet size. diff --git a/src/models/misc/shortest_common_supersequence.rs b/src/models/misc/shortest_common_supersequence.rs index b158a5347..5ea1fdea0 100644 --- a/src/models/misc/shortest_common_supersequence.rs +++ b/src/models/misc/shortest_common_supersequence.rs @@ -55,12 +55,27 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ShortestCommonSupersequenceData")] pub struct ShortestCommonSupersequence { alphabet_size: usize, strings: Vec>, max_length: usize, } +#[derive(Deserialize)] +struct ShortestCommonSupersequenceData { + alphabet_size: usize, + strings: Vec>, +} + +impl TryFrom for ShortestCommonSupersequence { + type Error = crate::registry::ConstructionError; + + fn try_from(data: ShortestCommonSupersequenceData) -> Result { + Self::try_new(data.alphabet_size, data.strings) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct ShortestCommonSupersequenceCreateSpec { /// Input strings; the alphabet and maximum length are inferred from them. @@ -72,10 +87,6 @@ impl TryFrom for ShortestCommonSuperseque type Error = crate::registry::ConstructionError; fn try_from(spec: ShortestCommonSupersequenceCreateSpec) -> Result { - if spec.strings.is_empty() { - return Err("must have at least one string".to_string().into()); - } - let alphabet_size = spec .strings .iter() @@ -89,17 +100,7 @@ impl TryFrom for ShortestCommonSuperseque }) .transpose()? .unwrap_or(0); - let max_length = spec.strings.iter().try_fold(0_usize, |total, string| { - total - .checked_add(string.len()) - .ok_or_else(|| "maximum supersequence length overflows usize".to_string()) - })?; - - Ok(Self { - alphabet_size, - strings: spec.strings, - max_length, - }) + Self::try_new(alphabet_size, spec.strings) } } @@ -114,17 +115,29 @@ impl ShortestCommonSupersequence { /// Panics if `strings` is empty, or if `alphabet_size` is 0 and any input /// string is non-empty. pub fn new(alphabet_size: usize, strings: Vec>) -> Self { - assert!(!strings.is_empty(), "must have at least one string"); - let max_length: usize = strings.iter().map(|s| s.len()).sum(); - assert!( - alphabet_size > 0 || strings.iter().all(|s| s.is_empty()), - "alphabet_size must be > 0 when any input string is non-empty" - ); - Self { + Self::try_new(alphabet_size, strings).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + alphabet_size: usize, + strings: Vec>, + ) -> Result { + if strings.is_empty() { + return Err("must have at least one string".into()); + } + let max_length = strings.iter().try_fold(0usize, |total, string| { + total + .checked_add(string.len()) + .ok_or("maximum string length overflows usize") + })?; + if !(alphabet_size > 0 || strings.iter().all(|s| s.is_empty())) { + return Err("alphabet_size must be > 0 when any input string is non-empty".into()); + } + Ok(Self { alphabet_size, strings, max_length, - } + }) } /// Returns the alphabet size. diff --git a/src/models/misc/shortest_common_superstring.rs b/src/models/misc/shortest_common_superstring.rs index 81fee2a1c..ed634b785 100644 --- a/src/models/misc/shortest_common_superstring.rs +++ b/src/models/misc/shortest_common_superstring.rs @@ -63,12 +63,27 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "ShortestCommonSuperstringData")] pub struct ShortestCommonSuperstring { alphabet_size: usize, strings: Vec>, max_length: usize, } +#[derive(Deserialize)] +struct ShortestCommonSuperstringData { + alphabet_size: usize, + strings: Vec>, +} + +impl TryFrom for ShortestCommonSuperstring { + type Error = crate::registry::ConstructionError; + + fn try_from(data: ShortestCommonSuperstringData) -> Result { + Self::try_new(data.alphabet_size, data.strings) + } +} + impl ShortestCommonSuperstring { /// Create a new ShortestCommonSuperstring instance. /// @@ -80,17 +95,29 @@ impl ShortestCommonSuperstring { /// Panics if `strings` is empty, or if `alphabet_size` is 0 and any input /// string is non-empty. pub fn new(alphabet_size: usize, strings: Vec>) -> Self { - assert!(!strings.is_empty(), "must have at least one string"); - let max_length: usize = strings.iter().map(|s| s.len()).sum(); - assert!( - alphabet_size > 0 || strings.iter().all(|s| s.is_empty()), - "alphabet_size must be > 0 when any input string is non-empty" - ); - Self { + Self::try_new(alphabet_size, strings).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + alphabet_size: usize, + strings: Vec>, + ) -> Result { + if strings.is_empty() { + return Err("must have at least one string".into()); + } + let max_length = strings.iter().try_fold(0usize, |total, string| { + total + .checked_add(string.len()) + .ok_or("maximum string length overflows usize") + })?; + if !(alphabet_size > 0 || strings.iter().all(|s| s.is_empty())) { + return Err("alphabet_size must be > 0 when any input string is non-empty".into()); + } + Ok(Self { alphabet_size, strings, max_length, - } + }) } /// Returns the alphabet size. diff --git a/src/models/misc/string_to_string_correction.rs b/src/models/misc/string_to_string_correction.rs index 0343806e8..d5912afb0 100644 --- a/src/models/misc/string_to_string_correction.rs +++ b/src/models/misc/string_to_string_correction.rs @@ -66,6 +66,7 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "StringToStringCorrectionData")] pub struct StringToStringCorrection { alphabet_size: usize, source: Vec, @@ -73,6 +74,22 @@ pub struct StringToStringCorrection { bound: usize, } +#[derive(Deserialize)] +struct StringToStringCorrectionData { + alphabet_size: usize, + source: Vec, + target: Vec, + bound: usize, +} + +impl TryFrom for StringToStringCorrection { + type Error = crate::registry::ConstructionError; + + fn try_from(data: StringToStringCorrectionData) -> Result { + Self::try_new(data.alphabet_size, data.source, data.target, data.bound) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct StringToStringCorrectionCreateSpec { /// Optional alphabet size; omitted values are inferred from both strings. @@ -105,22 +122,12 @@ impl TryFrom for StringToStringCorrection { .transpose()? .unwrap_or(0); let alphabet_size = spec.alphabet_size.unwrap_or(inferred_alphabet_size); - if alphabet_size < inferred_alphabet_size { - return Err(format!( - "alphabet size {alphabet_size} is smaller than inferred alphabet size {inferred_alphabet_size}" - ).into()); - } - if alphabet_size == 0 && (!spec.source_string.is_empty() || !spec.target_string.is_empty()) - { - return Err("alphabet size must be positive when either string is non-empty".into()); - } - - Ok(Self { + Self::try_new( alphabet_size, - source: spec.source_string, - target: spec.target_string, - bound: spec.bound, - }) + spec.source_string, + spec.target_string, + spec.bound, + ) } } @@ -133,24 +140,31 @@ impl StringToStringCorrection { /// non-empty, or if any symbol in `source` or `target` is /// `>= alphabet_size`. pub fn new(alphabet_size: usize, source: Vec, target: Vec, bound: usize) -> Self { - assert!( - alphabet_size > 0 || (source.is_empty() && target.is_empty()), - "alphabet_size must be > 0 when source or target is non-empty" - ); - assert!( - source.iter().all(|&s| s < alphabet_size), - "all source symbols must be < alphabet_size" - ); - assert!( - target.iter().all(|&s| s < alphabet_size), - "all target symbols must be < alphabet_size" - ); - Self { + Self::try_new(alphabet_size, source, target, bound) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + alphabet_size: usize, + source: Vec, + target: Vec, + bound: usize, + ) -> Result { + if !(alphabet_size > 0 || (source.is_empty() && target.is_empty())) { + return Err("alphabet_size must be > 0 when source or target is non-empty".into()); + } + if !(source.iter().all(|&s| s < alphabet_size)) { + return Err("all source symbols must be < alphabet_size".into()); + } + if !(target.iter().all(|&s| s < alphabet_size)) { + return Err("all target symbols must be < alphabet_size".into()); + } + Ok(Self { alphabet_size, source, target, bound, - } + }) } /// Returns the alphabet size. diff --git a/src/unit_tests/models/misc/closest_string.rs b/src/unit_tests/models/misc/closest_string.rs index fba543793..df523fb4f 100644 --- a/src/unit_tests/models/misc/closest_string.rs +++ b/src/unit_tests/models/misc/closest_string.rs @@ -126,3 +126,14 @@ fn test_closest_string_serialization() { problem.evaluate(&vec![0, 0, 0]).unwrap() ); } + +#[test] +fn deserialize_rejects_invalid_input() { + for json in [ + serde_json::json!({"alphabet_size": 2, "strings": []}), + serde_json::json!({"alphabet_size": 2, "strings": [[0], [0, 1]]}), + serde_json::json!({"alphabet_size": 2, "strings": [[2]]}), + ] { + assert!(serde_json::from_value::(json).is_err()); + } +} diff --git a/src/unit_tests/models/misc/grouping_by_swapping.rs b/src/unit_tests/models/misc/grouping_by_swapping.rs index 7b56a31d8..e8911fffc 100644 --- a/src/unit_tests/models/misc/grouping_by_swapping.rs +++ b/src/unit_tests/models/misc/grouping_by_swapping.rs @@ -147,3 +147,13 @@ fn test_grouping_by_swapping_create_spec_rejects_nonzero_bound_for_empty_string( assert!(result.is_err()); } + +#[test] +fn deserialize_rejects_invalid_input() { + for json in [ + serde_json::json!({"alphabet_size": 1, "string": [1], "budget": 0}), + serde_json::json!({"alphabet_size": 0, "string": [], "budget": 1}), + ] { + assert!(serde_json::from_value::(json).is_err()); + } +} diff --git a/src/unit_tests/models/misc/longest_common_subsequence.rs b/src/unit_tests/models/misc/longest_common_subsequence.rs index 913a300db..ac7665979 100644 --- a/src/unit_tests/models/misc/longest_common_subsequence.rs +++ b/src/unit_tests/models/misc/longest_common_subsequence.rs @@ -220,3 +220,23 @@ fn test_lcs_create_spec_rejects_all_empty_strings() { assert!(result.is_err()); } + +#[test] +fn deserialize_rejects_invalid_input() { + for json in [ + serde_json::json!({"alphabet_size": 1, "strings": [[1]], "max_length": 1}), + serde_json::json!({"alphabet_size": 0, "strings": [[0]], "max_length": 1}), + ] { + assert!(serde_json::from_value::(json).is_err()); + } +} + +#[test] +fn deserialize_rebuilds_length_bound() { + let problem: LongestCommonSubsequence = serde_json::from_value(serde_json::json!({ + "alphabet_size": 2, "strings": [[0, 1], [1]], "max_length": 99 + })) + .unwrap(); + assert_eq!(problem.max_length(), 1); + assert_eq!(problem.dimensions().len(), 1); +} diff --git a/src/unit_tests/models/misc/shortest_common_supersequence.rs b/src/unit_tests/models/misc/shortest_common_supersequence.rs index 65dfccfcb..c4a233371 100644 --- a/src/unit_tests/models/misc/shortest_common_supersequence.rs +++ b/src/unit_tests/models/misc/shortest_common_supersequence.rs @@ -249,3 +249,23 @@ fn test_shortestcommonsupersequence_paper_example() { // Optimal SCS for "abc" and "bac" is length 4 assert_eq!(val.0.unwrap(), 4); } + +#[test] +fn deserialize_rejects_invalid_input() { + for json in [ + serde_json::json!({"alphabet_size": 2, "strings": [], "max_length": 0}), + serde_json::json!({"alphabet_size": 0, "strings": [[0]], "max_length": 1}), + ] { + assert!(serde_json::from_value::(json).is_err()); + } +} + +#[test] +fn deserialize_rebuilds_length_bound() { + let problem: ShortestCommonSupersequence = serde_json::from_value(serde_json::json!({ + "alphabet_size": 2, "strings": [[0, 1], [1]], "max_length": 99 + })) + .unwrap(); + assert_eq!(problem.max_length(), 3); + assert_eq!(problem.dimensions().len(), 3); +} diff --git a/src/unit_tests/models/misc/shortest_common_superstring.rs b/src/unit_tests/models/misc/shortest_common_superstring.rs index c2462e6c3..71386218f 100644 --- a/src/unit_tests/models/misc/shortest_common_superstring.rs +++ b/src/unit_tests/models/misc/shortest_common_superstring.rs @@ -262,3 +262,23 @@ fn test_shortestcommonsuperstring_paper_example() { Min(Some(3)) ); } + +#[test] +fn deserialize_rejects_invalid_input() { + for json in [ + serde_json::json!({"alphabet_size": 2, "strings": [], "max_length": 0}), + serde_json::json!({"alphabet_size": 0, "strings": [[0]], "max_length": 1}), + ] { + assert!(serde_json::from_value::(json).is_err()); + } +} + +#[test] +fn deserialize_rebuilds_length_bound() { + let problem: ShortestCommonSuperstring = serde_json::from_value(serde_json::json!({ + "alphabet_size": 2, "strings": [[0, 1], [1]], "max_length": 99 + })) + .unwrap(); + assert_eq!(problem.max_length(), 3); + assert_eq!(problem.dimensions().len(), 3); +} diff --git a/src/unit_tests/models/misc/string_to_string_correction.rs b/src/unit_tests/models/misc/string_to_string_correction.rs index 4530eab36..e5c099713 100644 --- a/src/unit_tests/models/misc/string_to_string_correction.rs +++ b/src/unit_tests/models/misc/string_to_string_correction.rs @@ -198,3 +198,13 @@ fn test_string_to_string_correction_create_spec_rejects_small_alphabet() { assert!(result.is_err()); } + +#[test] +fn deserialize_rejects_invalid_input() { + for json in [ + serde_json::json!({"alphabet_size": 1, "source": [1], "target": [], "bound": 1}), + serde_json::json!({"alphabet_size": 1, "source": [], "target": [1], "bound": 1}), + ] { + assert!(serde_json::from_value::(json).is_err()); + } +} From 92677c0f35695dcbcb459d26fccd94ceda838b58 Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 02:07:52 +0800 Subject: [PATCH 2/5] Validate scheduling model inputs through shared constructors --- src/models/misc/flow_shop_scheduling.rs | 57 ++++++--- src/models/misc/job_shop_scheduling.rs | 85 +++++++------ ...encing_with_release_times_and_deadlines.rs | 58 ++++++--- src/models/misc/staff_scheduling.rs | 113 +++++++++--------- .../models/misc/flow_shop_scheduling.rs | 17 +++ .../models/misc/job_shop_scheduling.rs | 25 ++++ ...encing_with_release_times_and_deadlines.rs | 24 ++++ .../models/misc/staff_scheduling.rs | 23 ++++ 8 files changed, 271 insertions(+), 131 deletions(-) diff --git a/src/models/misc/flow_shop_scheduling.rs b/src/models/misc/flow_shop_scheduling.rs index bd9ee27eb..1ef72562f 100644 --- a/src/models/misc/flow_shop_scheduling.rs +++ b/src/models/misc/flow_shop_scheduling.rs @@ -58,6 +58,7 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "FlowShopSchedulingData")] pub struct FlowShopScheduling { /// Number of processors (machines). num_processors: usize, @@ -67,6 +68,21 @@ pub struct FlowShopScheduling { deadline: i64, } +#[derive(Deserialize)] +struct FlowShopSchedulingData { + num_processors: usize, + task_lengths: Vec>, + deadline: i64, +} + +impl TryFrom for FlowShopScheduling { + type Error = crate::registry::ConstructionError; + + fn try_from(data: FlowShopSchedulingData) -> Result { + Self::try_new(data.num_processors, data.task_lengths, data.deadline) + } +} + impl FlowShopScheduling { /// Create a new Flow Shop Scheduling instance. /// @@ -79,26 +95,37 @@ impl FlowShopScheduling { /// # Panics /// Panics if any job does not have exactly `num_processors` tasks. pub fn new(num_processors: usize, task_lengths: Vec>, deadline: i64) -> Self { + Self::try_new(num_processors, task_lengths, deadline) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_processors: usize, + task_lengths: Vec>, + deadline: i64, + ) -> Result { for (j, tasks) in task_lengths.iter().enumerate() { - assert_eq!( - tasks.len(), - num_processors, - "Job {} has {} tasks, expected {}", - j, - tasks.len(), - num_processors - ); + if tasks.len() != num_processors { + return Err(format!( + "Job {} has {} tasks, expected {}", + j, + tasks.len(), + num_processors + ) + .into()); + } } - assert!( - task_lengths.iter().flatten().all(|&length| length >= 0), - "task lengths must be nonnegative" - ); - assert!(deadline >= 0, "deadline must be nonnegative"); - Self { + if task_lengths.iter().flatten().any(|&length| length < 0) { + return Err("task lengths must be nonnegative".into()); + } + if deadline < 0 { + return Err("deadline must be nonnegative".into()); + } + Ok(Self { num_processors, task_lengths, deadline, - } + }) } /// Get the number of processors. diff --git a/src/models/misc/job_shop_scheduling.rs b/src/models/misc/job_shop_scheduling.rs index 2be0b6f7e..b0125dace 100644 --- a/src/models/misc/job_shop_scheduling.rs +++ b/src/models/misc/job_shop_scheduling.rs @@ -25,11 +25,26 @@ inventory::submit! { } #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "JobShopSchedulingData")] pub struct JobShopScheduling { num_processors: usize, jobs: Vec>, } +#[derive(Deserialize)] +struct JobShopSchedulingData { + num_processors: usize, + jobs: Vec>, +} + +impl TryFrom for JobShopScheduling { + type Error = crate::registry::ConstructionError; + + fn try_from(data: JobShopSchedulingData) -> Result { + Self::try_new(data.num_processors, data.jobs) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct JobShopSchedulingCreateSpec { /// Jobs expressed as ordered processor-duration operations. @@ -63,29 +78,7 @@ impl TryFrom for JobShopScheduling { return Err("num_processors must be positive".to_string().into()); } - for (job_index, job) in spec.jobs.iter().enumerate() { - for (task_index, &(processor, _)) in job.iter().enumerate() { - if processor >= num_processors { - return Err(format!( - "job {job_index} task {task_index} uses processor {processor}, but num_processors is {num_processors}" - ).into()); - } - } - for (task_index, pair) in job.windows(2).enumerate() { - if pair[0].0 == pair[1].0 { - return Err(format!( - "job {job_index} tasks {task_index} and {} must use different processors", - task_index + 1 - ) - .into()); - } - } - } - - Ok(Self { - num_processors, - jobs: spec.jobs, - }) + Self::try_new(num_processors, spec.jobs) } } @@ -97,40 +90,42 @@ struct FlattenedTasks { impl JobShopScheduling { pub fn new(num_processors: usize, jobs: Vec>) -> Self { - let num_tasks: usize = jobs.iter().map(Vec::len).sum(); - if num_tasks > 0 { - assert!( - num_processors > 0, - "num_processors must be positive when tasks are present" - ); + Self::try_new(num_processors, jobs).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_processors: usize, + jobs: Vec>, + ) -> Result { + if jobs.iter().any(|job| !job.is_empty()) && num_processors == 0 { + return Err("num_processors must be positive when tasks are present".into()); + } + if jobs.iter().flatten().any(|&(_, length)| length < 0) { + return Err("operation lengths must be nonnegative".into()); } - assert!( - jobs.iter().flatten().all(|&(_, length)| length >= 0), - "operation lengths must be nonnegative" - ); for (job_index, job) in jobs.iter().enumerate() { for (task_index, &(processor, _length)) in job.iter().enumerate() { - assert!( - processor < num_processors, - "job {job_index} task {task_index} uses processor {processor}, but num_processors = {num_processors}" - ); + if processor >= num_processors { + return Err(format!("job {job_index} task {task_index} uses processor {processor}, but num_processors = {num_processors}").into()); + } } for (task_index, pair) in job.windows(2).enumerate() { - assert_ne!( - pair[0].0, - pair[1].0, - "job {job_index} tasks {task_index} and {} must use different processors", - task_index + 1 - ); + if pair[0].0 == pair[1].0 { + return Err(format!( + "job {job_index} tasks {task_index} and {} must use different processors", + task_index + 1 + ) + .into()); + } } } - Self { + Ok(Self { num_processors, jobs, - } + }) } pub fn num_processors(&self) -> usize { diff --git a/src/models/misc/sequencing_with_release_times_and_deadlines.rs b/src/models/misc/sequencing_with_release_times_and_deadlines.rs index 3bc6449ea..c07c67451 100644 --- a/src/models/misc/sequencing_with_release_times_and_deadlines.rs +++ b/src/models/misc/sequencing_with_release_times_and_deadlines.rs @@ -57,12 +57,29 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "SequencingWithReleaseTimesAndDeadlinesData")] pub struct SequencingWithReleaseTimesAndDeadlines { lengths: Vec, release_times: Vec, deadlines: Vec, } +#[derive(Deserialize)] +struct SequencingWithReleaseTimesAndDeadlinesData { + lengths: Vec, + release_times: Vec, + deadlines: Vec, +} + +impl TryFrom + for SequencingWithReleaseTimesAndDeadlines +{ + type Error = crate::registry::ConstructionError; + fn try_from(data: SequencingWithReleaseTimesAndDeadlinesData) -> Result { + Self::try_new(data.lengths, data.release_times, data.deadlines) + } +} + impl SequencingWithReleaseTimesAndDeadlines { /// Create a new instance. /// @@ -70,25 +87,34 @@ impl SequencingWithReleaseTimesAndDeadlines { /// /// Panics if the three vectors have different lengths. pub fn new(lengths: Vec, release_times: Vec, deadlines: Vec) -> Self { - assert_eq!(lengths.len(), release_times.len()); - assert_eq!(lengths.len(), deadlines.len()); - assert!( - lengths.iter().all(|&length| length >= 0), - "task lengths must be nonnegative" - ); - assert!( - release_times.iter().all(|&release| release >= 0), - "release times must be nonnegative" - ); - assert!( - deadlines.iter().all(|&deadline| deadline >= 0), - "deadlines must be nonnegative" - ); - Self { + Self::try_new(lengths, release_times, deadlines).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + lengths: Vec, + release_times: Vec, + deadlines: Vec, + ) -> Result { + if lengths.len() != release_times.len() { + return Err("lengths and release_times must have the same length".into()); + } + if lengths.len() != deadlines.len() { + return Err("lengths and deadlines must have the same length".into()); + } + if lengths.iter().any(|&length| length < 0) { + return Err("task lengths must be nonnegative".into()); + } + if release_times.iter().any(|&release| release < 0) { + return Err("release times must be nonnegative".into()); + } + if deadlines.iter().any(|&deadline| deadline < 0) { + return Err("deadlines must be nonnegative".into()); + } + Ok(Self { lengths, release_times, deadlines, - } + }) } /// Returns the processing times. diff --git a/src/models/misc/staff_scheduling.rs b/src/models/misc/staff_scheduling.rs index 1d5d60a0d..8d448074a 100644 --- a/src/models/misc/staff_scheduling.rs +++ b/src/models/misc/staff_scheduling.rs @@ -27,6 +27,7 @@ inventory::submit! { /// pattern. A configuration is satisfying iff the total assigned workers does /// not exceed `num_workers` and every period's staffing requirement is met. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "StaffSchedulingData")] pub struct StaffScheduling { shifts_per_schedule: usize, schedules: Vec>, @@ -34,6 +35,27 @@ pub struct StaffScheduling { num_workers: i64, } +#[derive(Deserialize)] +struct StaffSchedulingData { + shifts_per_schedule: usize, + schedules: Vec>, + requirements: Vec, + num_workers: i64, +} + +impl TryFrom for StaffScheduling { + type Error = crate::registry::ConstructionError; + + fn try_from(data: StaffSchedulingData) -> Result { + Self::try_new( + data.shifts_per_schedule, + data.schedules, + data.requirements, + data.num_workers, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct StaffSchedulingCreateSpec { /// Required number of active periods in each schedule pattern. @@ -50,39 +72,7 @@ impl TryFrom for StaffScheduling { type Error = crate::registry::ConstructionError; fn try_from(spec: StaffSchedulingCreateSpec) -> Result { - if usize::try_from(spec.num_workers) - .ok() - .and_then(|workers| workers.checked_add(1)) - .is_none() - { - return Err("num_workers must be nonnegative and encodable by dims()" - .to_string() - .into()); - } - for (schedule_index, schedule) in spec.schedules.iter().enumerate() { - if schedule.len() != spec.requirements.len() { - return Err(format!( - "schedules[{schedule_index}] has {} periods, expected {}", - schedule.len(), - spec.requirements.len() - ) - .into()); - } - let active_periods = schedule.iter().filter(|&&active| active).count(); - if active_periods != spec.k { - return Err(format!( - "schedules[{schedule_index}] has {active_periods} active periods, expected {}", - spec.k - ) - .into()); - } - } - Ok(Self::new( - spec.k, - spec.schedules, - spec.requirements, - spec.num_workers, - )) + Self::try_new(spec.k, spec.schedules, spec.requirements, spec.num_workers) } } @@ -101,38 +91,51 @@ impl StaffScheduling { requirements: Vec, num_workers: i64, ) -> Self { - assert!( - usize::try_from(num_workers) - .ok() - .and_then(|workers| workers.checked_add(1)) - .is_some(), - "num_workers must be nonnegative and encodable by dims()" - ); + Self::try_new(shifts_per_schedule, schedules, requirements, num_workers) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + shifts_per_schedule: usize, + schedules: Vec>, + requirements: Vec, + num_workers: i64, + ) -> Result { + if usize::try_from(num_workers) + .ok() + .and_then(|workers| workers.checked_add(1)) + .is_none() + { + return Err("num_workers must be nonnegative and encodable by dims()".into()); + } let num_periods = requirements.len(); for (index, schedule) in schedules.iter().enumerate() { - assert_eq!( - schedule.len(), - num_periods, - "schedule {} has {} periods, expected {}", - index, - schedule.len(), - num_periods - ); + if schedule.len() != num_periods { + return Err(format!( + "schedule {} has {} periods, expected {}", + index, + schedule.len(), + num_periods + ) + .into()); + } let ones = schedule.iter().filter(|&&active| active).count(); - assert_eq!( - ones, shifts_per_schedule, - "schedule {} has {} active periods, expected {}", - index, ones, shifts_per_schedule - ); + if ones != shifts_per_schedule { + return Err(format!( + "schedule {} has {} active periods, expected {}", + index, ones, shifts_per_schedule + ) + .into()); + } } - Self { + Ok(Self { shifts_per_schedule, schedules, requirements, num_workers, - } + }) } /// Get the number of periods. diff --git a/src/unit_tests/models/misc/flow_shop_scheduling.rs b/src/unit_tests/models/misc/flow_shop_scheduling.rs index a3b327622..d0ebd9267 100644 --- a/src/unit_tests/models/misc/flow_shop_scheduling.rs +++ b/src/unit_tests/models/misc/flow_shop_scheduling.rs @@ -1,4 +1,21 @@ use super::*; + +#[test] +fn test_flow_shop_scheduling_rejects_invalid_json() { + let valid = serde_json::to_value(FlowShopScheduling::new(2, vec![vec![1, 2]], 3)).unwrap(); + for (field, value) in [ + ("task_lengths", serde_json::json!([[1]])), + ("task_lengths", serde_json::json!([[-1, 2]])), + ("deadline", serde_json::json!(-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/job_shop_scheduling.rs b/src/unit_tests/models/misc/job_shop_scheduling.rs index e190677ca..80d775adb 100644 --- a/src/unit_tests/models/misc/job_shop_scheduling.rs +++ b/src/unit_tests/models/misc/job_shop_scheduling.rs @@ -1,4 +1,23 @@ use super::*; + +#[test] +fn test_job_shop_scheduling_rejects_invalid_json() { + let valid = + serde_json::to_value(JobShopScheduling::new(2, vec![vec![(0, 1), (1, 2)]])).unwrap(); + for (field, value) in [ + ("num_processors", serde_json::json!(0)), + ("jobs", serde_json::json!([[[2, 1]]])), + ("jobs", serde_json::json!([[[0, -1]]])), + ("jobs", serde_json::json!([[[0, 1], [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; @@ -123,6 +142,12 @@ fn test_job_shop_scheduling_create_spec_derives_processor_count() { #[test] fn test_job_shop_scheduling_create_spec_rejects_invalid_jobs() { + assert!(JobShopScheduling::try_from(JobShopSchedulingCreateSpec { + jobs: vec![vec![(0, -1)]], + num_processors: Some(1), + }) + .is_err()); + let empty = JobShopScheduling::try_from(JobShopSchedulingCreateSpec { jobs: vec![], num_processors: None, diff --git a/src/unit_tests/models/misc/sequencing_with_release_times_and_deadlines.rs b/src/unit_tests/models/misc/sequencing_with_release_times_and_deadlines.rs index 1c86ed2d8..91b02b864 100644 --- a/src/unit_tests/models/misc/sequencing_with_release_times_and_deadlines.rs +++ b/src/unit_tests/models/misc/sequencing_with_release_times_and_deadlines.rs @@ -1,4 +1,28 @@ use super::*; + +#[test] +fn test_sequencing_with_release_times_and_deadlines_rejects_invalid_json() { + let valid = serde_json::to_value(SequencingWithReleaseTimesAndDeadlines::new( + vec![1], + vec![0], + vec![2], + )) + .unwrap(); + for (field, value) in [ + ("release_times", serde_json::json!([])), + ("deadlines", serde_json::json!([])), + ("lengths", serde_json::json!([-1])), + ("release_times", serde_json::json!([-1])), + ("deadlines", serde_json::json!([-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/staff_scheduling.rs b/src/unit_tests/models/misc/staff_scheduling.rs index ac58c7aba..efdc2018c 100644 --- a/src/unit_tests/models/misc/staff_scheduling.rs +++ b/src/unit_tests/models/misc/staff_scheduling.rs @@ -1,4 +1,27 @@ use super::*; + +#[test] +fn test_staff_scheduling_rejects_invalid_json() { + let valid = serde_json::to_value(StaffScheduling::new( + 1, + vec![vec![true, false]], + vec![1, 0], + 1, + )) + .unwrap(); + for (field, value) in [ + ("num_workers", serde_json::json!(-1)), + ("schedules", serde_json::json!([[true]])), + ("schedules", serde_json::json!([[true, true]])), + ] { + 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; From 5e62728a7541b26bcd01ab74749a28062d74472a Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 02:15:07 +0800 Subject: [PATCH 3/5] Share multiprocessor and weighted tardiness input validation --- problemreductions-cli/src/dispatch.rs | 2 +- problemreductions-cli/tests/cli_tests.rs | 2 +- src/models/misc/multiprocessor_scheduling.rs | 50 ++++++------- ...quencing_to_minimize_weighted_tardiness.rs | 72 ++++++++----------- .../models/misc/multiprocessor_scheduling.rs | 21 +++++- ...quencing_to_minimize_weighted_tardiness.rs | 34 +++++++++ 6 files changed, 108 insertions(+), 73 deletions(-) diff --git a/problemreductions-cli/src/dispatch.rs b/problemreductions-cli/src/dispatch.rs index 341125bb4..c48f99f52 100644 --- a/problemreductions-cli/src/dispatch.rs +++ b/problemreductions-cli/src/dispatch.rs @@ -596,7 +596,7 @@ mod tests { ); let err = loaded.err().unwrap(); assert!( - err.to_string().contains("expected positive integer, got 0"), + err.to_string().contains("num_processors must be positive"), "unexpected error: {err}" ); } diff --git a/problemreductions-cli/tests/cli_tests.rs b/problemreductions-cli/tests/cli_tests.rs index 90224cc29..6ea1c5290 100644 --- a/problemreductions-cli/tests/cli_tests.rs +++ b/problemreductions-cli/tests/cli_tests.rs @@ -7845,7 +7845,7 @@ fn test_evaluate_multiprocessor_scheduling_rejects_zero_processors_json() { ); let stderr = String::from_utf8_lossy(&output.stderr); assert!( - stderr.contains("expected positive integer, got 0"), + stderr.contains("num_processors must be positive"), "stderr: {stderr}" ); diff --git a/src/models/misc/multiprocessor_scheduling.rs b/src/models/misc/multiprocessor_scheduling.rs index 0cfb11a7c..8eb7c7f0c 100644 --- a/src/models/misc/multiprocessor_scheduling.rs +++ b/src/models/misc/multiprocessor_scheduling.rs @@ -50,11 +50,11 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MultiprocessorSchedulingCreateSpec")] pub struct MultiprocessorScheduling { /// Processing time for each task. lengths: Vec, /// Number of identical processors. - #[serde(deserialize_with = "positive_usize::deserialize")] num_processors: usize, /// Global deadline. deadline: i64, @@ -72,10 +72,7 @@ struct MultiprocessorSchedulingCreateSpec { impl TryFrom for MultiprocessorScheduling { type Error = crate::registry::ConstructionError; fn try_from(spec: MultiprocessorSchedulingCreateSpec) -> Result { - if spec.num_processors == 0 { - return Err("num_processors must be positive".to_string().into()); - } - Ok(Self::new(spec.lengths, spec.num_processors, spec.deadline)) + Self::try_new(spec.lengths, spec.num_processors, spec.deadline) } } @@ -85,17 +82,28 @@ impl MultiprocessorScheduling { /// # Panics /// Panics if `num_processors` is zero. pub fn new(lengths: Vec, num_processors: usize, deadline: i64) -> Self { - assert!(num_processors > 0, "num_processors must be positive"); - assert!( - lengths.iter().all(|&length| length >= 0), - "task lengths must be nonnegative" - ); - assert!(deadline >= 0, "deadline must be nonnegative"); - Self { + Self::try_new(lengths, num_processors, deadline).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + lengths: Vec, + num_processors: usize, + deadline: i64, + ) -> Result { + if num_processors == 0 { + return Err("num_processors must be positive".into()); + } + if lengths.iter().any(|&length| length < 0) { + return Err("task lengths must be nonnegative".into()); + } + if deadline < 0 { + return Err("deadline must be nonnegative".into()); + } + Ok(Self { lengths, num_processors, deadline, - } + }) } /// Returns the processing times for each task. @@ -193,22 +201,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/multiprocessor_scheduling.rs"] mod tests; diff --git a/src/models/misc/sequencing_to_minimize_weighted_tardiness.rs b/src/models/misc/sequencing_to_minimize_weighted_tardiness.rs index 08b6d8419..ebcba6231 100644 --- a/src/models/misc/sequencing_to_minimize_weighted_tardiness.rs +++ b/src/models/misc/sequencing_to_minimize_weighted_tardiness.rs @@ -52,6 +52,7 @@ inventory::submit! { /// assert!(solver.solve(&problem).unwrap().is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "SequencingToMinimizeWeightedTardinessCreateSpec")] pub struct SequencingToMinimizeWeightedTardiness { lengths: Vec, weights: Vec, @@ -77,22 +78,7 @@ impl TryFrom fn try_from( spec: SequencingToMinimizeWeightedTardinessCreateSpec, ) -> Result { - if spec.lengths.len() != spec.weights.len() { - return Err("weights length must equal lengths length" - .to_string() - .into()); - } - if spec.lengths.len() != spec.deadlines.len() { - return Err("deadlines length must equal lengths length" - .to_string() - .into()); - } - Ok(Self::new( - spec.lengths, - spec.weights, - spec.deadlines, - spec.bound, - )) + Self::try_new(spec.lengths, spec.weights, spec.deadlines, spec.bound) } } @@ -103,35 +89,39 @@ impl SequencingToMinimizeWeightedTardiness { /// /// Panics if the input vectors do not have the same length. pub fn new(lengths: Vec, weights: Vec, deadlines: Vec, bound: i64) -> Self { - assert_eq!( - lengths.len(), - weights.len(), - "weights length must equal lengths length" - ); - assert_eq!( - lengths.len(), - deadlines.len(), - "deadlines length must equal lengths length" - ); - assert!( - lengths.iter().all(|&length| length >= 0), - "task lengths must be nonnegative" - ); - assert!( - weights.iter().all(|&weight| weight >= 0), - "task weights must be nonnegative" - ); - assert!( - deadlines.iter().all(|&deadline| deadline >= 0), - "deadlines must be nonnegative" - ); - assert!(bound >= 0, "bound must be nonnegative"); - Self { + Self::try_new(lengths, weights, deadlines, bound).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + lengths: Vec, + weights: Vec, + deadlines: Vec, + bound: i64, + ) -> Result { + if lengths.len() != weights.len() { + return Err("weights length must equal lengths length".into()); + } + if lengths.len() != deadlines.len() { + return Err("deadlines length must equal lengths length".into()); + } + if lengths.iter().any(|&length| length < 0) { + return Err("task lengths must be nonnegative".into()); + } + if weights.iter().any(|&weight| weight < 0) { + return Err("task weights must be nonnegative".into()); + } + if deadlines.iter().any(|&deadline| deadline < 0) { + return Err("deadlines must be nonnegative".into()); + } + if bound < 0 { + return Err("bound must be nonnegative".into()); + } + Ok(Self { lengths, weights, deadlines, bound, - } + }) } /// Returns the job lengths. diff --git a/src/unit_tests/models/misc/multiprocessor_scheduling.rs b/src/unit_tests/models/misc/multiprocessor_scheduling.rs index a8581588c..de3b8277c 100644 --- a/src/unit_tests/models/misc/multiprocessor_scheduling.rs +++ b/src/unit_tests/models/misc/multiprocessor_scheduling.rs @@ -1,4 +1,23 @@ use super::*; + +#[test] +fn test_multiprocessor_scheduling_rejects_invalid_inputs() { + let valid = serde_json::to_value(MultiprocessorScheduling::new(vec![1], 1, 2)).unwrap(); + for (field, value) in [ + ("lengths", serde_json::json!([-1])), + ("deadline", serde_json::json!(-1)), + ("num_processors", serde_json::json!(0)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid.clone()).is_err(), + "{field}" + ); + let spec = serde_json::from_value::(invalid).unwrap(); + assert!(MultiprocessorScheduling::try_from(spec).is_err(), "{field}"); + } +} use crate::solvers::BruteForceProblem as _; #[test] @@ -176,7 +195,7 @@ fn test_multiprocessor_scheduling_deserialization_rejects_zero_processors() { })) .unwrap_err(); assert!( - err.to_string().contains("expected positive integer, got 0"), + err.to_string().contains("num_processors must be positive"), "unexpected error: {err}" ); } diff --git a/src/unit_tests/models/misc/sequencing_to_minimize_weighted_tardiness.rs b/src/unit_tests/models/misc/sequencing_to_minimize_weighted_tardiness.rs index 3214d9c90..acfa4f316 100644 --- a/src/unit_tests/models/misc/sequencing_to_minimize_weighted_tardiness.rs +++ b/src/unit_tests/models/misc/sequencing_to_minimize_weighted_tardiness.rs @@ -1,4 +1,38 @@ use super::*; + +#[test] +fn test_sequencing_to_minimize_weighted_tardiness_rejects_invalid_inputs() { + let valid = serde_json::to_value(SequencingToMinimizeWeightedTardiness::new( + vec![1], + vec![1], + vec![2], + 0, + )) + .unwrap(); + for (field, value) in [ + ("weights", serde_json::json!([])), + ("deadlines", serde_json::json!([])), + ("lengths", serde_json::json!([-1])), + ("weights", serde_json::json!([-1])), + ("deadlines", serde_json::json!([-1])), + ("bound", serde_json::json!(-1)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid.clone()) + .is_err(), + "{field}" + ); + let spec = + serde_json::from_value::(invalid) + .unwrap(); + assert!( + SequencingToMinimizeWeightedTardiness::try_from(spec).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForceProblem as _; #[test] From 0d7d55c66ae74a37ff951968e2ced2d8e3de045f Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 02:56:31 +0800 Subject: [PATCH 4/5] Validate persisted precedence and deadline schedules --- .../misc/precedence_constrained_scheduling.rs | 99 +++++++------- .../scheduling_with_individual_deadlines.rs | 125 +++++++++--------- .../misc/precedence_constrained_scheduling.rs | 25 ++++ .../scheduling_with_individual_deadlines.rs | 24 ++++ 4 files changed, 163 insertions(+), 110 deletions(-) diff --git a/src/models/misc/precedence_constrained_scheduling.rs b/src/models/misc/precedence_constrained_scheduling.rs index b062b26a2..e351d5d4a 100644 --- a/src/models/misc/precedence_constrained_scheduling.rs +++ b/src/models/misc/precedence_constrained_scheduling.rs @@ -47,6 +47,7 @@ inventory::submit! { /// assert!(solution.is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "PrecedenceConstrainedSchedulingData")] pub struct PrecedenceConstrainedScheduling { num_tasks: usize, num_processors: usize, @@ -54,6 +55,26 @@ pub struct PrecedenceConstrainedScheduling { precedences: Vec<(usize, usize)>, } +#[derive(Deserialize)] +struct PrecedenceConstrainedSchedulingData { + num_tasks: usize, + num_processors: usize, + deadline: i64, + precedences: Vec<(usize, usize)>, +} + +impl TryFrom for PrecedenceConstrainedScheduling { + type Error = crate::registry::ConstructionError; + fn try_from(data: PrecedenceConstrainedSchedulingData) -> Result { + Self::try_new( + data.num_tasks, + data.num_processors, + data.deadline, + data.precedences, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct PrecedenceConstrainedSchedulingCreateSpec { num_tasks: usize, @@ -66,38 +87,12 @@ impl TryFrom for PrecedenceConstraine type Error = crate::registry::ConstructionError; fn try_from(spec: PrecedenceConstrainedSchedulingCreateSpec) -> Result { - if spec.num_tasks > 0 && spec.num_processors == 0 { - return Err("num_processors must be positive when there are tasks" - .to_string() - .into()); - } - if spec.num_tasks > 0 && spec.deadline == 0 { - return Err("deadline must be positive when there are tasks" - .to_string() - .into()); - } - if spec.deadline < 0 || usize::try_from(spec.deadline).is_err() { - return Err("deadline must be nonnegative and fit usize" - .to_string() - .into()); - } - let precedences = spec.precedences.unwrap_or_default(); - if let Some(&(pred, succ)) = precedences - .iter() - .find(|&&(pred, succ)| pred >= spec.num_tasks || succ >= spec.num_tasks) - { - return Err(format!( - "precedence ({pred}, {succ}) is out of range for {} tasks", - spec.num_tasks - ) - .into()); - } - Ok(Self::new( + Self::try_new( spec.num_tasks, spec.num_processors, spec.deadline, - precedences, - )) + spec.precedences.unwrap_or_default(), + ) } } @@ -114,32 +109,42 @@ impl PrecedenceConstrainedScheduling { deadline: i64, precedences: Vec<(usize, usize)>, ) -> Self { + Self::try_new(num_tasks, num_processors, deadline, precedences) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_tasks: usize, + num_processors: usize, + deadline: i64, + precedences: Vec<(usize, usize)>, + ) -> Result { if num_tasks > 0 { - assert!( - num_processors > 0, - "num_processors must be > 0 when there are tasks" - ); - assert!(deadline > 0, "deadline must be > 0 when there are tasks"); + if num_processors == 0 { + return Err("num_processors must be > 0 when there are tasks".into()); + } + if deadline <= 0 { + return Err("deadline must be > 0 when there are tasks".into()); + } + } + if deadline < 0 || usize::try_from(deadline).is_err() { + return Err("deadline must be nonnegative and fit usize".into()); } - assert!( - deadline >= 0 && usize::try_from(deadline).is_ok(), - "deadline must be nonnegative and fit usize" - ); for &(i, j) in &precedences { - assert!( - i < num_tasks && j < num_tasks, - "Precedence ({}, {}) out of bounds for {} tasks", - i, - j, - num_tasks - ); + if i >= num_tasks || j >= num_tasks { + return Err(format!( + "Precedence ({}, {}) out of bounds for {} tasks", + i, j, num_tasks + ) + .into()); + } } - Self { + Ok(Self { num_tasks, num_processors, deadline, precedences, - } + }) } /// Get the number of tasks. diff --git a/src/models/misc/scheduling_with_individual_deadlines.rs b/src/models/misc/scheduling_with_individual_deadlines.rs index 6f8b21658..83810aedc 100644 --- a/src/models/misc/scheduling_with_individual_deadlines.rs +++ b/src/models/misc/scheduling_with_individual_deadlines.rs @@ -29,6 +29,7 @@ inventory::submit! { /// satisfies `sigma(u) + 1 <= sigma(v)` and no time slot hosts more than /// `num_processors` tasks. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "SchedulingWithIndividualDeadlinesData")] pub struct SchedulingWithIndividualDeadlines { num_tasks: usize, num_processors: usize, @@ -36,6 +37,26 @@ pub struct SchedulingWithIndividualDeadlines { precedences: Vec<(usize, usize)>, } +#[derive(Deserialize)] +struct SchedulingWithIndividualDeadlinesData { + num_tasks: usize, + num_processors: usize, + deadlines: Vec, + precedences: Vec<(usize, usize)>, +} + +impl TryFrom for SchedulingWithIndividualDeadlines { + type Error = crate::registry::ConstructionError; + fn try_from(data: SchedulingWithIndividualDeadlinesData) -> Result { + Self::try_new( + data.num_tasks, + data.num_processors, + data.deadlines, + data.precedences, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct SchedulingWithIndividualDeadlinesCreateSpec { /// Number of tasks. @@ -50,43 +71,12 @@ struct SchedulingWithIndividualDeadlinesCreateSpec { impl TryFrom for SchedulingWithIndividualDeadlines { type Error = crate::registry::ConstructionError; fn try_from(spec: SchedulingWithIndividualDeadlinesCreateSpec) -> Result { - if spec.deadlines.len() != spec.num_tasks { - return Err(format!( - "deadlines has {} entries, expected {}", - spec.deadlines.len(), - spec.num_tasks - ) - .into()); - } - if spec.deadlines.iter().any(|&deadline| deadline < 0) { - return Err("deadlines must be nonnegative".to_string().into()); - } - if spec - .deadlines - .iter() - .any(|&deadline| usize::try_from(deadline).is_err()) - { - return Err("deadlines must fit usize to define schedule slots" - .to_string() - .into()); - } - let precedences = spec.precedences.unwrap_or_default(); - if let Some(&(pred, succ)) = precedences - .iter() - .find(|&&(p, s)| p >= spec.num_tasks || s >= spec.num_tasks) - { - return Err(format!( - "precedence ({pred}, {succ}) is out of range for {} tasks", - spec.num_tasks - ) - .into()); - } - Ok(Self::new( + Self::try_new( spec.num_tasks, spec.num_processors, spec.deadlines, - precedences, - )) + spec.precedences.unwrap_or_default(), + ) } } @@ -97,42 +87,51 @@ impl SchedulingWithIndividualDeadlines { deadlines: Vec, precedences: Vec<(usize, usize)>, ) -> Self { - assert_eq!( - deadlines.len(), - num_tasks, - "deadlines length must equal num_tasks" - ); - assert!( - deadlines.iter().all(|&deadline| deadline >= 0), - "deadlines must be nonnegative" - ); - assert!( - deadlines - .iter() - .all(|&deadline| usize::try_from(deadline).is_ok()), - "deadlines must fit usize to define schedule slots" - ); + Self::try_new(num_tasks, num_processors, deadlines, precedences) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_tasks: usize, + num_processors: usize, + deadlines: Vec, + precedences: Vec<(usize, usize)>, + ) -> Result { + if deadlines.len() != num_tasks { + return Err("deadlines length must equal num_tasks".into()); + } + if deadlines.iter().any(|&deadline| deadline < 0) { + return Err("deadlines must be nonnegative".into()); + } + if deadlines + .iter() + .any(|&deadline| usize::try_from(deadline).is_err()) + { + return Err("deadlines must fit usize to define schedule slots".into()); + } for &(pred, succ) in &precedences { - assert!( - pred < num_tasks, - "predecessor index {} out of range (num_tasks = {})", - pred, - num_tasks - ); - assert!( - succ < num_tasks, - "successor index {} out of range (num_tasks = {})", - succ, - num_tasks - ); + if pred >= num_tasks { + return Err(format!( + "predecessor index {} out of range (num_tasks = {})", + pred, num_tasks + ) + .into()); + } + if succ >= num_tasks { + return Err(format!( + "successor index {} out of range (num_tasks = {})", + succ, num_tasks + ) + .into()); + } } - Self { + Ok(Self { num_tasks, num_processors, deadlines, precedences, - } + }) } pub fn num_tasks(&self) -> usize { diff --git a/src/unit_tests/models/misc/precedence_constrained_scheduling.rs b/src/unit_tests/models/misc/precedence_constrained_scheduling.rs index 47cf34313..582527c2f 100644 --- a/src/unit_tests/models/misc/precedence_constrained_scheduling.rs +++ b/src/unit_tests/models/misc/precedence_constrained_scheduling.rs @@ -1,4 +1,29 @@ use super::*; + +#[test] +fn test_precedence_constrained_scheduling_validates_persisted_input() { + let valid = + serde_json::to_value(PrecedenceConstrainedScheduling::new(2, 1, 2, vec![(0, 1)])).unwrap(); + for (field, value) in [ + ("num_processors", serde_json::json!(0)), + ("deadline", serde_json::json!(0)), + ("deadline", serde_json::json!(-1)), + ("precedences", serde_json::json!([[2, 0]])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } + assert!( + serde_json::from_value::(serde_json::json!({ + "num_tasks": 0, "num_processors": 0, "deadline": -1, "precedences": [] + })) + .is_err() + ); +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::traits::Problem; diff --git a/src/unit_tests/models/misc/scheduling_with_individual_deadlines.rs b/src/unit_tests/models/misc/scheduling_with_individual_deadlines.rs index 3a017d7a4..0b8aaa0b1 100644 --- a/src/unit_tests/models/misc/scheduling_with_individual_deadlines.rs +++ b/src/unit_tests/models/misc/scheduling_with_individual_deadlines.rs @@ -1,4 +1,28 @@ use super::*; + +#[test] +fn test_scheduling_with_individual_deadlines_validates_persisted_input() { + let valid = serde_json::to_value(SchedulingWithIndividualDeadlines::new( + 2, + 1, + vec![1, 2], + vec![(0, 1)], + )) + .unwrap(); + for (field, value) in [ + ("deadlines", serde_json::json!([1])), + ("deadlines", serde_json::json!([-1, 2])), + ("precedences", serde_json::json!([[2, 0]])), + ("precedences", 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::BruteForceProblem as _; #[test] From ab760eafe4c3e9d5474d2ffd793f7ac1f201cee8 Mon Sep 17 00:00:00 2001 From: GiggleLiu Date: Fri, 18 Sep 2026 11:46:49 +0800 Subject: [PATCH 5/5] Simplify negated condition flagged by clippy nonminimal_bool Co-Authored-By: Claude Fable 5.1 --- src/models/misc/grouping_by_swapping.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/misc/grouping_by_swapping.rs b/src/models/misc/grouping_by_swapping.rs index 8b22940c2..8f806cab5 100644 --- a/src/models/misc/grouping_by_swapping.rs +++ b/src/models/misc/grouping_by_swapping.rs @@ -103,7 +103,7 @@ impl GroupingBySwapping { if !(string.iter().all(|&symbol| symbol < alphabet_size)) { return Err("input symbols must be less than alphabet_size".into()); } - if !(!string.is_empty() || budget == 0) { + if string.is_empty() && budget != 0 { return Err("budget must be 0 when string is empty".into()); } Ok(Self {