From 26fc59887e8c596dad4e67d350b26c5e908be259 Mon Sep 17 00:00:00 2001 From: Daniil Baturin Date: Thu, 12 Jan 2017 20:23:52 +0700 Subject: T245: improve handling of nodes with duplicate names. Two tag nodes with the same name ("ethernet eth0 {...} ethernet eth0 {...}") is an error. Two leaf nodes with the same name, however, are not an error. Values of the next nodes are merged into the first one, while all other data (comment and inactive and ephemeral properties are inherited from the first node. This mimics the old syntax of multi nodes, so a person who uses the old syntax out of habit in a handwritten config will get the result they expect. --- src/curly_parser.mly | 51 +++++++++++++++++++++++++++++++++++++++++------ src/vytree.ml | 7 ++++--- src/vytree.mli | 2 +- test/curly_parser_test.ml | 23 +++++++++++++++++++++ test/vytree_test.ml | 4 ++-- 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/curly_parser.mly b/src/curly_parser.mly index 58c6067..7daa082 100644 --- a/src/curly_parser.mly +++ b/src/curly_parser.mly @@ -1,5 +1,25 @@ %{ open Config_tree + + exception Duplicate_child of (string * string) + + (* Used for checking if after merging immediate children, + any of them have duplicate children inside, + e.g. "interfaces { ethernet eth0 {...} ethernet eth0 {...} }" *) + let find_duplicate_children n = + let rec aux xs = + let xs = List.sort compare xs in + match xs with + | [] | [_] -> () + | x :: x' :: xs -> + if x = x' then raise (Duplicate_child (Vytree.name_of_node n, x)) + else aux (x' :: xs) + in + aux @@ Vytree.list_children n + + (* When merging nodes with values, append values of subsequent nodes to the + first one *) + let merge_data l r = {l with values=(List.append l.values r.values)} %} %token IDENTIFIER @@ -43,7 +63,13 @@ node: | comment = opt_comment; name = IDENTIFIER; LEFT_BRACE; children = list(node_content); RIGHT_BRACE { let node = Vytree.make_full {default_data with comment=comment} name [] in - List.fold_left Vytree.adopt node (List.rev children) |> Vytree.merge_children + let node = List.fold_left Vytree.adopt node (List.rev children) |> Vytree.merge_children merge_data in + try + List.iter find_duplicate_children (Vytree.children_of_node node); + node + with + | Duplicate_child (child, dup) -> + failwith (Printf.sprintf "Node \"%s %s\" has two children named \"%s\"" name child dup) } ; @@ -52,15 +78,28 @@ tag_node: { let outer_node = Vytree.make_full default_data name [] in let inner_node = Vytree.make_full {default_data with comment=comment} tag [] in - let inner_node = List.fold_left Vytree.adopt inner_node (List.rev children) |> Vytree.merge_children - in Vytree.adopt outer_node inner_node + let inner_node = List.fold_left Vytree.adopt inner_node (List.rev children) |> Vytree.merge_children merge_data in + let node = Vytree.adopt outer_node inner_node in + try + List.iter find_duplicate_children (Vytree.children_of_node inner_node); + node + with + | Duplicate_child (child, dup) -> + failwith (Printf.sprintf "Node \"%s %s %s\" has two children named \"%s\"" name tag child dup) } node_content: n = node { n } | n = leaf_node { n } | n = tag_node { n }; %public config: - ns = list(node); EOF - { - let root = make "root" in List.fold_left Vytree.adopt root (List.rev ns) |> Vytree.merge_children + | ns = list(node); EOF + { + let root = make "root" in + let root = List.fold_left Vytree.adopt root (List.rev ns) |> Vytree.merge_children merge_data in + try + List.iter find_duplicate_children (Vytree.children_of_node root); + root + with + | Duplicate_child (child, dup) -> + failwith (Printf.sprintf "Node \"%s\" has two children named \"%s\"" child dup) } ; diff --git a/src/vytree.ml b/src/vytree.ml index 74dbe45..156b326 100644 --- a/src/vytree.ml +++ b/src/vytree.ml @@ -92,16 +92,17 @@ let rec insert ?(position=Default) node path data = may be normal and even expected, such as "ethernet eth0" and "ethernet eth1" in the "curly" format. *) -let merge_children node = +let merge_children merge_data node = (* Given a node N and a list of nodes NS, find all nodes in NS that have the same name as N and merge their children into N *) - let rec merge_into n ns = + let rec merge_into n ns = match ns with | [] -> n | n' :: ns' -> if n.name = n'.name then let children = List.append n.children n'.children in - let n = {n with children=children} in + let data = merge_data n.data n'.data in + let n = {n with children=children; data=data} in merge_into n ns' else merge_into n ns' in diff --git a/src/vytree.mli b/src/vytree.mli index 02d8edf..9bc8844 100644 --- a/src/vytree.mli +++ b/src/vytree.mli @@ -23,7 +23,7 @@ val insert : ?position:position -> 'a t -> string list -> 'a -> 'a t val insert_multi_level : 'a -> 'a t -> string list -> string list -> 'a -> 'a t -val merge_children : 'a t -> 'a t +val merge_children : ('a -> 'a -> 'a) -> 'a t -> 'a t val delete : 'a t -> string list -> 'a t diff --git a/test/curly_parser_test.ml b/test/curly_parser_test.ml index 7e53065..8e3d3fd 100644 --- a/test/curly_parser_test.ml +++ b/test/curly_parser_test.ml @@ -27,6 +27,10 @@ let config_with_comment = "foo { /* comment */ bar { } }" let config_with_leaf_node_comment = "foo { /* comment */ bar baz; }" let config_with_tag_node_comment = "foo { /* comment */ bar baz { } }" +let config_with_duplicate_node = "foo { bar { baz {} } bar { baz {} } }" +let config_with_duplicate_tag_node = "foo { bar baz0 { } bar baz0 { } }" +let config_with_duplicate_leaf_node = "foo { bar baz; bar quux; }" + let parse s = Curly_parser.config Curly_lexer.token (Lexing.from_string s) (* Empty config is considered valid, creates just the root node *) @@ -99,6 +103,22 @@ let test_parse_with_tag test_ctxt = assert_equal (CT.get_value config ["foo"; "bar"; "baz"; "quux"]) "xyzzy"; assert_equal (CT.get_value config ["foo"; "bar"; "qwerty"; "quux"]) "foobar" +(* Normal nodes with duplicate children are detected *) +let test_parse_node_duplicate_child test_ctxt = + try ignore @@ parse config_with_duplicate_node; assert_failure "Duplicated node child didn't cause errors" + with (Failure _) -> () + +(* Tag nodes with duplicate children are detected *) +let test_parse_tag_node_duplicate_child test_ctxt = + try ignore @@ parse config_with_duplicate_tag_node; assert_failure "Duplicated tag node child didn't cause errors" + with (Failure _) -> () + +(* If there are duplicate leaf nodes, values of the next ones are merged into the first one, + the rest of the data is lost *) +let test_parse_duplicate_leaf_node test_ctxt = + let config = parse config_with_duplicate_leaf_node in + assert_equal (CT.get_values config ["foo"; "bar"]) ["baz"; "quux"] + let suite = "VyConf curly config parser tests" >::: [ @@ -116,6 +136,9 @@ let suite = "test_parse_with_comment" >:: test_parse_with_comment; "test_parse_with_leaf_node_comment" >:: test_parse_with_leaf_node_comment; "test_parse_with_tag_node_comment" >:: test_parse_with_tag_node_comment; + "test_parse_node_duplicate_child" >:: test_parse_node_duplicate_child; + "test_parse_tag_node_duplicate_child" >:: test_parse_tag_node_duplicate_child; + "test_parse_duplicate_leaf_node" >:: test_parse_duplicate_leaf_node; ] let () = diff --git a/test/vytree_test.ml b/test/vytree_test.ml index 3cf3ae6..4da2335 100644 --- a/test/vytree_test.ml +++ b/test/vytree_test.ml @@ -139,7 +139,7 @@ let test_merge_children_no_duplicates test_ctxt = [make_full () "foo" [make () "bar"]; make () "bar"; make_full () "baz" [make () "quuz"]] in - let node' = merge_children node in + let node' = merge_children (fun x y -> x) node in assert_equal (list_children node') ["foo"; "bar"; "baz"] @@ -151,7 +151,7 @@ let test_merge_children_has_duplicates test_ctxt = [make_full () "foo" [make () "bar"]; make () "quux"; make_full () "foo" [make () "baz"]] in - let node' = merge_children node in + let node' = merge_children (fun x y -> x) node in assert_equal (list_children node') ["foo"; "quux"]; assert_equal (get node' ["foo"] |> list_children) ["bar"; "baz"] -- cgit v1.2.3