From 5b39ff3e0baabfb4333e39cd4aa0606abc23411c Mon Sep 17 00:00:00 2001
From: reinterpretcat <ilya.builuk@gmail.com>
Date: Tue, 4 Jun 2024 23:42:34 +0200
Subject: [PATCH] Add check for state keys duplication

---
 vrp-core/src/models/goal.rs             | 19 +++++++++++-
 vrp-core/tests/unit/models/goal_test.rs | 40 +++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/vrp-core/src/models/goal.rs b/vrp-core/src/models/goal.rs
index 4486c52b5..0c0c098f1 100644
--- a/vrp-core/src/models/goal.rs
+++ b/vrp-core/src/models/goal.rs
@@ -79,7 +79,7 @@ pub struct GoalContextBuilder {
 impl GoalContextBuilder {
     /// Creates a `GoalBuilder` with the given list of features.
     pub fn with_features(features: Vec<Feature>) -> GenericResult<Self> {
-        let ids_all = features.iter().map(|feature| feature.name.clone()).collect::<Vec<_>>();
+        let ids_all = features.iter().map(|feature| feature.name.as_str()).collect::<Vec<_>>();
         let ids_unique = ids_all.iter().collect::<HashSet<_>>();
 
         if ids_unique.len() != ids_all.len() {
@@ -89,6 +89,23 @@ impl GoalContextBuilder {
             )
             .into());
         }
+        drop(ids_unique);
+
+        // check state keys duplication
+        let state_keys = features
+            .iter()
+            .filter_map(|f| f.state.as_ref().map(|state| (f.name.clone(), state.state_keys().collect::<HashSet<_>>())))
+            .collect::<Vec<_>>();
+        state_keys.iter().try_for_each(|(outer_name, outer_keys)| {
+            state_keys.iter().filter(|(inner_name, _)| inner_name != outer_name).try_for_each(
+                |(inner_name, inner_keys)| {
+                    inner_keys.intersection(outer_keys).next().map_or(Ok(()), |_| {
+                        Err(format!("feature {outer_name} and {inner_name} shares common state keys for caching"))
+                    })
+                },
+            )
+        })?;
+        drop(state_keys);
 
         Ok(Self { main_goal: None, alternative_goals: Vec::default(), features })
     }
diff --git a/vrp-core/tests/unit/models/goal_test.rs b/vrp-core/tests/unit/models/goal_test.rs
index 7cee5c6bc..0b69edd12 100644
--- a/vrp-core/tests/unit/models/goal_test.rs
+++ b/vrp-core/tests/unit/models/goal_test.rs
@@ -199,3 +199,43 @@ fn can_use_objective_total_order_impl(left_fitness: Vec<f64>, right_fitness: Vec
 
     assert_eq!(goal_ctx.total_order(&left, &right), expected);
 }
+
+#[test]
+fn can_detect_same_key_usage() {
+    let mut registry = StateKeyRegistry::default();
+    let same_key = registry.next_key();
+
+    let goal_ctx = GoalContextBuilder::with_features(vec![
+        create_distance_balanced_feature("name_1", None, registry.next_key(), same_key).unwrap(),
+        create_distance_balanced_feature("name_2", None, registry.next_key(), registry.next_key()).unwrap(),
+        create_distance_balanced_feature("name_3", None, registry.next_key(), same_key).unwrap(),
+    ]);
+
+    match goal_ctx {
+        Ok(_) => unreachable!(),
+        Err(message) => {
+            assert_eq!(message, GenericError::from("feature name_1 and name_3 shares common state keys for caching"))
+        }
+    }
+}
+
+#[test]
+fn can_detect_same_name_usage() {
+    let goal_ctx = GoalContextBuilder::with_features(vec![
+        create_objective_feature_with_dynamic_cost("name_1", Arc::new(|_, _| 1.)),
+        create_objective_feature_with_dynamic_cost("name_2", Arc::new(|_, _| 1.)),
+        create_objective_feature_with_dynamic_cost("name_1", Arc::new(|_, _| 1.)),
+    ]);
+
+    match goal_ctx {
+        Ok(_) => unreachable!(),
+        Err(message) => {
+            assert_eq!(
+                message,
+                GenericError::from(
+                    "some of the features are defined more than once, check ids list: name_1,name_2,name_1"
+                )
+            )
+        }
+    }
+}