Skip to content

Add required field validation to nested structures #9463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions awscli/clidocs.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,21 +248,23 @@ def _document_nested_structure(self, model, doc):
"""Recursively documents parameters in nested structures"""
member_type_name = getattr(model, 'type_name', None)
if member_type_name == 'structure':
required_members = model.metadata.get('required', [])
for member_name, member_shape in model.members.items():
is_required = member_name in required_members
self._doc_member(
doc, member_name, member_shape, stack=[model.name]
doc, member_name, member_shape, stack=[model.name], required=is_required
)
elif member_type_name == 'list':
self._doc_member(doc, '', model.member, stack=[model.name])
self._doc_member(doc, '', model.member, stack=[model.name], required=False)
elif member_type_name == 'map':
key_shape = model.key
key_name = key_shape.serialization.get('name', 'key')
self._doc_member(doc, key_name, key_shape, stack=[model.name])
self._doc_member(doc, key_name, key_shape, stack=[model.name], required=False)
value_shape = model.value
value_name = value_shape.serialization.get('name', 'value')
self._doc_member(doc, value_name, value_shape, stack=[model.name])
self._doc_member(doc, value_name, value_shape, stack=[model.name], required=False)

def _doc_member(self, doc, member_name, member_shape, stack):
def _doc_member(self, doc, member_name, member_shape, stack, required=False):
if member_shape.name in stack:
# Document the recursion once, otherwise just
# note the fact that it's recursive and return.
Expand All @@ -272,11 +274,11 @@ def _doc_member(self, doc, member_name, member_shape, stack):
return
stack.append(member_shape.name)
try:
self._do_doc_member(doc, member_name, member_shape, stack)
self._do_doc_member(doc, member_name, member_shape, stack, required)
finally:
stack.pop()

def _do_doc_member(self, doc, member_name, member_shape, stack):
def _do_doc_member(self, doc, member_name, member_shape, stack, required=False):
docs = member_shape.documentation
type_name = self._get_argument_type_name(
member_shape, member_shape.type_name
Expand All @@ -290,22 +292,29 @@ def _do_doc_member(self, doc, member_name, member_shape, stack):
doc.include_doc_string(docs)
if is_tagged_union_type(member_shape):
self._add_tagged_union_note(member_shape, doc)

if required:
doc.style.new_paragraph()
doc.write('This parameter is required.')

self._document_enums(member_shape, doc)
self._document_constraints(member_shape, doc)
doc.style.new_paragraph()
member_type_name = member_shape.type_name
if member_type_name == 'structure':
required_members = member_shape.metadata.get('required', [])
for sub_name, sub_shape in member_shape.members.items():
self._doc_member(doc, sub_name, sub_shape, stack)
sub_required = sub_name in required_members
self._doc_member(doc, sub_name, sub_shape, stack, required=sub_required)
elif member_type_name == 'map':
key_shape = member_shape.key
key_name = key_shape.serialization.get('name', 'key')
self._doc_member(doc, key_name, key_shape, stack)
self._doc_member(doc, key_name, key_shape, stack, required=False)
value_shape = member_shape.value
value_name = value_shape.serialization.get('name', 'value')
self._doc_member(doc, value_name, value_shape, stack)
self._doc_member(doc, value_name, value_shape, stack, required=False)
elif member_type_name == 'list':
self._doc_member(doc, '', member_shape.member, stack)
self._doc_member(doc, '', member_shape.member, stack, required=False)
doc.style.dedent()
doc.style.new_paragraph()

Expand Down
30 changes: 30 additions & 0 deletions tests/unit/test_clidocs.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,36 @@ def test_tagged_union_comes_after_docstring_output(self):
rendered = help_command.doc.getvalue().decode('utf-8')
self.assertRegex(rendered, r'FooBar[\s\S]*Tagged Union')

def test_documents_required_parameters(self):
"""Tests that required parameters are correctly documented."""
shape_map = {
'ParentStructure': {
'type': 'structure',
'required': ['RequiredParameter'],
'members': {
'RequiredParameter': {'shape': 'StringMember'},
'OptionalParameter': {'shape': 'StringMember'},
}
},
'StringMember': {'type': 'string'},
}

resolver = ShapeResolver(shape_map)
parent_shape = StructureShape(
'ParentStructure',
shape_map['ParentStructure'],
resolver
)

rendered = self.get_help_docs_for_argument(parent_shape)

required_index = rendered.find('RequiredParameter -> (string)')
optional_index = rendered.find('OptionalParameter -> (string)')

self.assertIn('This parameter is required', rendered[required_index:optional_index])
optional_text = rendered[optional_index:]
self.assertNotIn('This parameter is required', optional_text)

def test_documents_constraints(self):
shape = {'type': 'string', 'min': 0, 'max': 10, 'pattern': '.*'}
shape = StringShape('ConstrainedArg', shape)
Expand Down
Loading