Skip to content

Commit 25d4024

Browse files
lgarber-akamailgarber-akamai
committed
Add support for deeply nested oneOfs; group nested oneOf options on help pages
1 parent b697682 commit 25d4024

File tree

5 files changed

+195
-52
lines changed

5 files changed

+195
-52
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ test-unit:
6464
@mkdir -p /tmp/linode/.config
6565
@orig_xdg_config_home=$${XDG_CONFIG_HOME:-}; \
6666
export LINODE_CLI_TEST_MODE=1 XDG_CONFIG_HOME=/tmp/linode/.config; \
67-
pytest -v tests/unit; \
67+
pytest -vv tests/unit; \
6868
exit_code=$$?; \
6969
export XDG_CONFIG_HOME=$$orig_xdg_config_home; \
7070
exit $$exit_code

‎linodecli/baked/request.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ def __init__( # pylint: disable=too-many-arguments
4545
:type parent: Optional[str]
4646
:param depth: The depth of this argument, or how many parent arguments this argument has.
4747
:type depth: int
48+
:param option_variants: A mapping of options, defined using oneOf in the to spec,
49+
to a variant of this argument.
4850
"""
4951
#: The name of this argument, mostly used for display and docs
5052
self.name = name
@@ -144,6 +146,7 @@ def _parse_request_model(
144146
:returns: The flattened request model, as a list
145147
:rtype: list[OpenAPIRequestArg]
146148
"""
149+
147150
args = []
148151

149152
properties, required = _aggregate_schema_properties(schema)

‎linodecli/baked/util.py‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"""
44

55
from collections import defaultdict
6-
from typing import Any, Dict, Set, Tuple
6+
from typing import Any, Dict, List, Set, Tuple
77

88
from openapi3.schemas import Schema
99

@@ -23,28 +23,38 @@ def _aggregate_schema_properties(
2323
properties = {}
2424
required = defaultdict(lambda: 0)
2525

26-
def _handle_schema(_schema: Schema):
27-
if _schema.properties is None:
26+
def __inner(
27+
path: List[str],
28+
entry: Schema,
29+
):
30+
if isinstance(entry, dict):
31+
# TODO: Figure out why this happens (openapi3 package bug?)
32+
entry = Schema(path, entry, schema._root)
33+
34+
if entry.properties is None:
35+
# If there aren't any properties, this might be a
36+
# composite schema
37+
for composition_field in ["oneOf", "allOf", "anyOf"]:
38+
for i, nested_entry in enumerate(
39+
getattr(entry, composition_field) or []
40+
):
41+
__inner(
42+
schema.path + [composition_field, str(i)],
43+
nested_entry,
44+
)
45+
2846
return
2947

48+
# This is a valid option
49+
properties.update(entry.properties)
50+
3051
nonlocal schema_count
3152
schema_count += 1
3253

33-
properties.update(dict(_schema.properties))
34-
35-
# Aggregate required keys and their number of usages.
36-
if _schema.required is not None:
37-
for key in _schema.required:
38-
required[key] += 1
39-
40-
_handle_schema(schema)
41-
42-
one_of = schema.oneOf or []
43-
any_of = schema.anyOf or []
54+
for key in entry.required or []:
55+
required[key] += 1
4456

45-
for entry in one_of + any_of:
46-
# pylint: disable=protected-access
47-
_handle_schema(Schema(schema.path, entry, schema._root))
57+
__inner(schema.path, schema)
4858

4959
return (
5060
properties,

‎linodecli/help_pages.py‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -298,50 +298,60 @@ def _help_group_arguments(
298298
"""
299299
Returns help page groupings for a list of POST/PUT arguments.
300300
"""
301+
args = [arg for arg in args if not arg.read_only]
301302
args_sorted = sorted(args, key=lambda a: a.path)
302303

303-
groups_tmp = defaultdict(list)
304+
paths = {tuple(arg.path.split(".")) for arg in args_sorted}
305+
path_to_args = defaultdict(list)
304306

305-
# Initial grouping by root parent
306307
for arg in args_sorted:
307-
if arg.read_only:
308-
continue
308+
arg_path = tuple(arg.path.split("."))
309+
310+
if not arg.is_parent:
311+
# Parent arguments are grouped in with their children
312+
arg_path = arg_path[:-1]
309313

310-
groups_tmp[arg.path.split(".", 1)[0]].append(arg)
314+
# Find first common parent
315+
while len(arg_path) > 1 and arg_path not in paths:
316+
arg_path = arg_path[:-1]
317+
318+
path_to_args[arg_path].append(arg)
311319

312320
group_required = []
313321
groups = []
314322
ungrouped = []
315323

316-
for group in groups_tmp.values():
317-
# If the group has more than one element,
318-
# leave it as is in the result
319-
if len(group) > 1:
324+
# def __recurse_group(group_mapping: Dict[str, List[OpenAPIRequestArg]]) -> List[OpenAPIRequestArg]:
325+
326+
for k, group in sorted(
327+
path_to_args.items(), key=lambda a: (len(a[0]), a[0], len(a[1]))
328+
):
329+
if len(k) > 0 and len(group) > 1:
330+
# This is a named subgroup
320331
groups.append(
321332
# Args should be ordered by least depth -> required -> path
322333
sorted(group, key=lambda v: (v.depth, not v.required, v.path)),
323334
)
324335
continue
325336

326-
target_arg = group[0]
327-
328337
# If the group's argument is required,
329-
# add it to the required group
330-
if target_arg.required:
331-
group_required.append(target_arg)
332-
continue
338+
# add it to the top-level required group
339+
for arg in group:
340+
if arg.required:
341+
group_required.append(arg)
342+
continue
333343

334-
# Add ungrouped arguments (single value groups) to the
335-
# "ungrouped" group.
336-
ungrouped.append(target_arg)
344+
# Add ungrouped arguments (single value groups) to the
345+
# "ungrouped" group.
346+
ungrouped.append(arg)
337347

338348
result = []
339349

340350
if len(group_required) > 0:
341-
result.append(group_required)
351+
result.append(sorted(group_required, key=lambda v: v.path))
342352

343353
if len(ungrouped) > 0:
344-
result.append(ungrouped)
354+
result.append(sorted(ungrouped, key=lambda v: v.path))
345355

346356
result += groups
347357

‎tests/unit/test_help_pages.py‎

Lines changed: 134 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,54 +14,174 @@ def test_group_arguments(self, capsys):
1414
# NOTE: We use SimpleNamespace here so we can do deep comparisons using ==
1515
args = [
1616
SimpleNamespace(
17-
read_only=False, required=False, depth=0, path="foobaz"
17+
read_only=False,
18+
required=False,
19+
depth=0,
20+
path="foobaz",
21+
parent=None,
22+
is_parent=False,
23+
),
24+
SimpleNamespace(
25+
read_only=False,
26+
required=False,
27+
depth=0,
28+
path="foobar",
29+
parent=None,
30+
is_parent=False,
1831
),
1932
SimpleNamespace(
20-
read_only=False, required=False, depth=0, path="foobar"
33+
read_only=False,
34+
required=True,
35+
depth=0,
36+
path="barfoo",
37+
parent=None,
38+
is_parent=False,
39+
),
40+
SimpleNamespace(
41+
read_only=False,
42+
required=False,
43+
depth=0,
44+
path="foo",
45+
parent=None,
46+
is_parent=True,
47+
),
48+
SimpleNamespace(
49+
read_only=False,
50+
required=False,
51+
depth=1,
52+
path="foo.bar",
53+
parent="foo",
54+
is_parent=False,
2155
),
2256
SimpleNamespace(
23-
read_only=False, required=True, depth=0, path="barfoo"
57+
read_only=False,
58+
required=False,
59+
depth=1,
60+
path="foo.foo",
61+
parent="foo",
62+
is_parent=False,
2463
),
2564
SimpleNamespace(
26-
read_only=False, required=False, depth=0, path="foo"
65+
read_only=False,
66+
required=True,
67+
depth=1,
68+
path="foo.baz",
69+
parent="foo",
70+
is_parent=True,
2771
),
2872
SimpleNamespace(
29-
read_only=False, required=False, depth=1, path="foo.bar"
73+
read_only=False,
74+
required=True,
75+
depth=1,
76+
path="foo.foobar",
77+
parent="foo",
78+
is_parent=False,
3079
),
3180
SimpleNamespace(
32-
read_only=False, required=False, depth=1, path="foo.foo"
81+
read_only=False,
82+
required=True,
83+
depth=2,
84+
path="foo.baz.foo",
85+
parent="foo.baz",
86+
is_parent=False,
3387
),
3488
SimpleNamespace(
35-
read_only=False, required=True, depth=1, path="foo.baz"
89+
read_only=False,
90+
required=True,
91+
depth=2,
92+
path="foo.baz.bar",
93+
parent="foo.baz",
94+
is_parent=False,
3695
),
3796
]
3897

3998
expected = [
4099
[
41100
SimpleNamespace(
42-
read_only=False, required=True, path="barfoo", depth=0
101+
read_only=False,
102+
required=True,
103+
path="barfoo",
104+
depth=0,
105+
parent=None,
106+
is_parent=False,
43107
),
44108
],
45109
[
46110
SimpleNamespace(
47-
read_only=False, required=False, path="foobar", depth=0
111+
read_only=False,
112+
required=False,
113+
path="foobar",
114+
depth=0,
115+
parent=None,
116+
is_parent=False,
48117
),
49118
SimpleNamespace(
50-
read_only=False, required=False, path="foobaz", depth=0
119+
read_only=False,
120+
required=False,
121+
path="foobaz",
122+
depth=0,
123+
parent=None,
124+
is_parent=False,
51125
),
52126
],
53127
[
54128
SimpleNamespace(
55-
read_only=False, required=False, path="foo", depth=0
129+
read_only=False,
130+
required=False,
131+
path="foo",
132+
depth=0,
133+
parent=None,
134+
is_parent=True,
135+
),
136+
SimpleNamespace(
137+
read_only=False,
138+
required=True,
139+
depth=1,
140+
path="foo.foobar",
141+
parent="foo",
142+
is_parent=False,
143+
),
144+
SimpleNamespace(
145+
read_only=False,
146+
required=False,
147+
path="foo.bar",
148+
depth=1,
149+
parent="foo",
150+
is_parent=False,
56151
),
57152
SimpleNamespace(
58-
read_only=False, required=True, path="foo.baz", depth=1
153+
read_only=False,
154+
required=False,
155+
path="foo.foo",
156+
depth=1,
157+
parent="foo",
158+
is_parent=False,
159+
),
160+
],
161+
[
162+
SimpleNamespace(
163+
read_only=False,
164+
required=True,
165+
path="foo.baz",
166+
depth=1,
167+
parent="foo",
168+
is_parent=True,
59169
),
60170
SimpleNamespace(
61-
read_only=False, required=False, path="foo.bar", depth=1
171+
read_only=False,
172+
required=True,
173+
depth=2,
174+
path="foo.baz.bar",
175+
parent="foo.baz",
176+
is_parent=False,
62177
),
63178
SimpleNamespace(
64-
read_only=False, required=False, path="foo.foo", depth=1
179+
read_only=False,
180+
required=True,
181+
depth=2,
182+
path="foo.baz.foo",
183+
parent="foo.baz",
184+
is_parent=False,
65185
),
66186
],
67187
]

0 commit comments

Comments
 (0)