fix: fix insert error when insert multi times with the same spec by O…

…verrideFile api (#1314) * fix: fix insert error when insert multi times with the same spec by OverrideFile api Signed-off-by: zongz <zongzhe1024@163.com> * fix: fix test case Signed-off-by: zongz <zongzhe1024@163.com> --------- Signed-off-by: zongz <zongzhe1024@163.com>
kcl-lang · May 15, 2024 · b377d05 · b377d05
1 parent b3bbfb9
commit b377d05
Show file tree

Hide file tree

Showing 8 changed files with 149 additions and 28 deletions.
diff --git a/kclvm/query/src/override.rs b/kclvm/query/src/override.rs
@@ -255,6 +255,9 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
     // Because the delete action may delete the global variable.
     // TODO: combine the code of walk_module, walk_assign_stmt and walk_unification_stmt
     fn walk_module(&mut self, module: &'ctx mut ast::Module) {
+        if self.has_override {
+            return;
+        }
         match self.action {
             // Walk the module body to find the target and override it.
             ast::OverrideAction::CreateOrUpdate => {
@@ -390,6 +393,9 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
     }
 
     fn walk_unification_stmt(&mut self, unification_stmt: &'ctx mut ast::UnificationStmt) {
+        if self.has_override {
+            return;
+        }
         let name = match unification_stmt.target.node.names.get(0) {
             Some(name) => name,
             None => bug!(
@@ -401,11 +407,13 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
             return;
         }
         self.override_target_count = 1;
-        self.has_override = true;
         self.walk_schema_expr(&mut unification_stmt.value.node);
     }
 
     fn walk_assign_stmt(&mut self, assign_stmt: &'ctx mut ast::AssignStmt) {
+        if self.has_override {
+            return;
+        }
         if let ast::Expr::Schema(_) | ast::Expr::Config(_) = &assign_stmt.value.node {
             self.override_target_count = 0;
             for target in &assign_stmt.targets {
@@ -420,12 +428,14 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
             if self.override_target_count == 0 {
                 return;
             }
-            self.has_override = true;
             self.walk_expr(&mut assign_stmt.value.node);
         }
     }
 
     fn walk_schema_expr(&mut self, schema_expr: &'ctx mut ast::SchemaExpr) {
+        if self.has_override {
+            return;
+        }
         if self.override_target_count == 0 {
             return;
         }
@@ -444,18 +454,25 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
                                 operation: ast::ConfigEntryOperation::Override,
                                 insert_index: -1,
                             })));
+                        self.has_override = true;
                     }
                 }
+            } else {
+                self.has_override = true;
             }
         }
         self.override_target_count = 0;
     }
 
     fn walk_config_expr(&mut self, config_expr: &'ctx mut ast::ConfigExpr) {
+        if self.has_override {
+            return;
+        }
         // Lookup config all fields and replace if it is matched with the override spec.
         if !self.lookup_config_and_replace(config_expr) {
             return;
         }
+        self.has_override = true;
         self.override_target_count = 0;
     }
 

diff --git a/kclvm/query/src/test_data/except.k b/kclvm/query/src/test_data/except.k
@@ -13,14 +13,9 @@ if True:
 config = Config {
     image = "image/image:v1"
     data = {id = 1, value = "override_value"}
-    data = {id = 1, value = "override_value"}
-}
-
-config: Config {
-    image = "image/image:v1"
-    data = {id = 1, value = "override_value"}
 }
 
+config: Config {image = "image/image:v3"}
 dict_config = {"image": "image/image:v2", "data": {"id": 2, "value2": "override_value2"}}
 envs = [{key = "key1", value = "value1"}, {key = "key2", value = "value2"}]
 isfilter = False
@@ -36,6 +31,4 @@ insert_config = {key = 1}
 uni_config = {
     labels: {key1: 1}
 }
-
 config_unification: Config {"image": "image/image:v4"}
-
diff --git a/kclvm/query/src/test_data/simple.bk.k b/kclvm/query/src/test_data/simple.bk.k
@@ -12,8 +12,6 @@ if True:
 
 config = Config {
     image = "image/image:v1"
-    data = {id = 1, value = "override_value"}
-    data = {id = 1, value = "override_value"}
 }
 
 config : Config {

diff --git a/kclvm/query/src/test_data/simple.k b/kclvm/query/src/test_data/simple.k
@@ -0,0 +1,77 @@
+schema Data:
+    id?: int = 0
+    value?: str = "value"
+
+schema Config:
+    image: str
+    data?: Data
+
+if True:
+    configOther = Config {image = "image/other:v1"}
+
+
+config = Config {
+    image = "image/image:v1"
+}
+
+config : Config {
+    image = "image/image:v3"
+}
+
+dict_config = {
+    "image": "image/image:v1"
+    "data": {
+        "id": 1
+        "value": "override_value"
+    }
+}
+envs = [
+    {
+        "name": "ENV1"
+        "value": "value1"
+    }
+    {
+        "name": "ENV2"
+        "value": "value2"
+    }
+]
+
+isfilter = True
+
+count = 1
+
+msg = "Hello World"
+
+delete = "Delete"
+
+dict_delete = {
+    "image": "image/image:v1"
+    "data": {
+        "id": 1
+        "value": "override_value"
+    }
+}
+
+dict_delete_whole = {
+    "image": "image/image:v1"
+    "data": {
+        "id": 1
+        "value": "override_value"
+    }
+}
+
+insert_config = {} 
+
+uni_config = {
+    labels: {key1: "value1"}
+}
+
+config_unification: Config {
+    image = "image/image:v1"
+    data = {id = 1, value = "override_value"}
+}
+
+config_unification_delete: Config {
+    image = "image/image:v1"
+    data = {id = 1, value = "override_value"}
+}
diff --git a/kclvm/query/src/test_data/test_override_file/expect.k b/kclvm/query/src/test_data/test_override_file/expect.k
@@ -25,13 +25,21 @@ _access = test.ServiceAccess {
     a: {b: {c = 2}, c: {b = 3}}
 }
 
+_access3 = test.ServiceAccess {
+    iType = "kkkkkkk"
+    sType = dsType
+    TestStr = ["${test_str}"]
+    ports = [80, 443]
+    booltest = True
+}
+
 b = {"c": 2}
 d = {
     e: {
         f: {g = 3}
     }
 }
-_access3 = test.ServiceAccess {
+_access4 = test.ServiceAccess {
     iType = "kkkkkkk"
     sType = dsType
     TestStr = ["${test_str}"]
@@ -40,7 +48,5 @@ _access3 = test.ServiceAccess {
 }
 
 _access5 = {iType = "dddddd"}
-
 a = b
-
 _access6 = "a6"
diff --git a/kclvm/query/src/test_data/test_override_file/main.bk.k b/kclvm/query/src/test_data/test_override_file/main.bk.k
@@ -17,4 +17,12 @@ _access = test.ServiceAccess {
     TestStr = ["${test_str}"]
     ports = [80, 443]
     booltest = True
+}
+
+_access3 = test.ServiceAccess {
+    iType = "kkkkkkk"
+    sType = dsType
+    TestStr = ["${test_str}"]
+    ports = [80, 443]
+    booltest = True
 }
diff --git a/kclvm/query/src/test_data/test_override_file/main.k b/kclvm/query/src/test_data/test_override_file/main.k
@@ -17,4 +17,12 @@ _access = test.ServiceAccess {
     TestStr = ["${test_str}"]
     ports = [80, 443]
     booltest = True
+}
+
+_access3 = test.ServiceAccess {
+    iType = "kkkkkkk"
+    sType = dsType
+    TestStr = ["${test_str}"]
+    ports = [80, 443]
+    booltest = True
 }
diff --git a/kclvm/query/src/tests.rs b/kclvm/query/src/tests.rs
@@ -39,25 +39,28 @@ fn test_override_file_simple() {
     let simple_path = get_test_dir("simple.k".to_string());
     let simple_bk_path = get_test_dir("simple.bk.k".to_string());
     let except_path = get_test_dir("except.k".to_string());
+    fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();
     if simple_path.exists() {
         fs::remove_file(simple_path.clone()).unwrap();
     }
 
-    fs::copy(simple_bk_path, simple_path.clone()).unwrap();
+    fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();
 
     let import_paths = vec![];
     assert_eq!(
-        override_file(simple_path.to_str().unwrap(), &specs, &import_paths).unwrap(),
+        override_file(simple_path.clone().to_str().unwrap(), &specs, &import_paths).unwrap(),
         true
     );
 
-    let simple_content = fs::read_to_string(simple_path).unwrap();
+    let simple_content = fs::read_to_string(simple_path.clone()).unwrap();
     let expect_content = fs::read_to_string(except_path).unwrap();
 
     let simple_content = simple_content.replace("\r\n", "").replace("\n", "");
     let expect_content = expect_content.replace("\r\n", "").replace("\n", "");
 
     assert_eq!(simple_content, expect_content);
+
+    fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();
 }
 /// Test override_file result.
 #[test]
@@ -496,6 +499,14 @@ fn test_overridefile_insert() {
     TestStr = ["${test_str}"]
     ports = [80, 443]
     booltest = True
+}"#
+        .to_string(),
+        r#"_access4=test.ServiceAccess {
+    iType = "kkkkkkk"
+    sType = dsType
+    TestStr = ["${test_str}"]
+    ports = [80, 443]
+    booltest = True
 }"#
         .to_string(),
         r#"_access.iType="kkkkkkk""#.to_string(),
@@ -511,7 +522,6 @@ fn test_overridefile_insert() {
     let simple_bk_path = get_test_dir("test_override_file/main.bk.k".to_string());
     let except_path = get_test_dir("test_override_file/expect.k".to_string());
     fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();
-
     let import_paths = vec![
         "base.pkg.kusion_models.app".to_string(),
         "base.pkg.kusion_models.app.vip as vip".to_string(),
@@ -521,17 +531,21 @@ fn test_overridefile_insert() {
         ".values".to_string(),
     ];
 
-    assert_eq!(
-        override_file(&simple_path.display().to_string(), &specs, &import_paths).unwrap(),
-        true
-    );
+    // test insert multiple times
+    for _ in 1..=5 {
+        assert_eq!(
+            override_file(&simple_path.display().to_string(), &specs, &import_paths).unwrap(),
+            true
+        );
 
-    let simple_content = fs::read_to_string(simple_path.clone()).unwrap();
-    let expect_content = fs::read_to_string(except_path.clone()).unwrap();
+        let simple_content = fs::read_to_string(simple_path.clone()).unwrap();
+        let expect_content = fs::read_to_string(except_path.clone()).unwrap();
 
-    let simple_content = simple_content.replace("\r\n", "").replace("\n", "");
-    let expect_content = expect_content.replace("\r\n", "").replace("\n", "");
+        let simple_content = simple_content.replace("\r\n", "").replace("\n", "");
+        let expect_content = expect_content.replace("\r\n", "").replace("\n", "");
+
+        assert_eq!(simple_content, expect_content);
+    }
 
-    assert_eq!(simple_content, expect_content);
     fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();
 }