Skip to content

Commit aaedffb

Browse files
committed
config: replace list append with smart merging dict-like lists
Previously, a CLI options used allow_append=True to allow setting +components etc options without removing all existing entries. This works when adding a single option, but breaks when adding multiple - as later option will override earlier options (even with different name). Replace this special CLI handling, with a more generic merging of dict-like lists. Specifically, when merging two lists and both consists of only one-key dicts (or just strings), merge them similar to merging dicts, but preserving the structure.
1 parent 8f8464b commit aaedffb

File tree

3 files changed

+84
-8
lines changed

3 files changed

+84
-8
lines changed

qubesbuilder/cli/cli_main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ def parse_config_from_cli(array):
146146
parsed_dict = {"+" + key: value}
147147
else:
148148
parsed_dict = parse_dict_from_cli(s)
149-
result = deep_merge(result, parsed_dict, allow_append=True)
149+
result = deep_merge(result, parsed_dict)
150150
return result
151151

152152

qubesbuilder/config.py

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,67 @@ def extract_key_from_list(input_list: list):
6060
return result
6161

6262

63-
def deep_merge(a: dict, b: dict, allow_append: bool = False) -> dict:
63+
def is_list_dict_like(value: Union[str, dict, list]):
64+
"""
65+
Checks if a list has only either strings, or one-key dicts, like it's
66+
used in "components" or "stages" or "templates"
67+
"""
68+
if not isinstance(value, list):
69+
return False
70+
for el in value:
71+
if not el:
72+
return False
73+
if isinstance(el, str):
74+
continue
75+
if not isinstance(el, dict):
76+
return False
77+
if len(el) != 1:
78+
return False
79+
if not isinstance(next(iter(el.keys())), str):
80+
return False
81+
return True
82+
83+
84+
def merge_dict_likes(result: list, b_list: list):
85+
"""
86+
Merge dict-like lists (lists with single-key dicts or strings only)
87+
This modifies *result* list in-place.
88+
"""
89+
# this is not very efficient but fortunately data size is small
90+
for b_value in b_list:
91+
if isinstance(b_value, str):
92+
# append if not already present
93+
keys_in_result = map(
94+
lambda x: x if isinstance(x, str) else next(iter(x.keys())),
95+
result,
96+
)
97+
if b_value not in keys_in_result:
98+
result.append(b_value)
99+
continue
100+
b_key = next(iter(b_value.keys()))
101+
for index, a_value in enumerate(result):
102+
if isinstance(a_value, str):
103+
# was plain string, replace with single-key dict
104+
result[index] = deepcopy(b_value)
105+
break
106+
# both are single-key dicts, compare the key
107+
a_key = next(iter(a_value.keys()))
108+
if a_key == b_key:
109+
result[index] = deep_merge(a_value, b_value)
110+
111+
112+
def deep_merge(a: dict, b: dict) -> dict:
64113
result = deepcopy(a)
65114
for b_key, b_value in b.items():
66115
a_value = result.get(b_key, None)
67116
if isinstance(a_value, dict) and isinstance(b_value, dict):
68-
result[b_key] = deep_merge(a_value, b_value, allow_append)
69-
else:
70-
if isinstance(result.get(b_key, None), list) and allow_append:
71-
result[b_key] += deepcopy(b_value)
72-
else:
73-
result[b_key] = deepcopy(b_value)
117+
result[b_key] = deep_merge(a_value, b_value)
118+
continue
119+
if isinstance(a_value, list) and isinstance(b_value, list):
120+
if is_list_dict_like(a_value) and is_list_dict_like(b_value):
121+
merge_dict_likes(result[b_key], b_value)
122+
continue
123+
result[b_key] = deepcopy(b_value)
74124
return result
75125

76126

tests/test_functions.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,32 @@ def test_parse_config_entry_from_array_10():
193193
assert parsed_dict == expected_dict
194194

195195

196+
def test_parse_config_entry_from_array_11_merge_multiple():
197+
array = ["+tata:titi+toto", "+tata:titi+toto:foo=bar"]
198+
parsed_dict = parse_config_from_cli(array)
199+
expected_dict = {
200+
"+tata": {"titi": [{"toto": {"foo": "bar"}}]},
201+
}
202+
assert parsed_dict == expected_dict
203+
204+
205+
def test_parse_config_entry_from_array_12_merge_multiple():
206+
array = ["+tata:titi+toto:foo=bar", "+tata:titi+toto"]
207+
parsed_dict = parse_config_from_cli(array)
208+
expected_dict = {
209+
"+tata": {"titi": [{"toto": {"foo": "bar"}}]},
210+
}
211+
assert parsed_dict == expected_dict
212+
213+
def test_parse_config_entry_from_array_13_merge_multiple():
214+
array = ["+tata:titi+toto:foo=bar", "+tata:titi+toto:foo=baz"]
215+
parsed_dict = parse_config_from_cli(array)
216+
expected_dict = {
217+
"+tata": {"titi": [{"toto": {"foo": "baz"}}]},
218+
}
219+
assert parsed_dict == expected_dict
220+
221+
196222
def test_deep_check_dict():
197223
data = {
198224
"key1": "value1",

0 commit comments

Comments
 (0)