diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py index 6694559c..4e1489cf 100644 --- a/oslo_policy/generator.py +++ b/oslo_policy/generator.py @@ -13,6 +13,7 @@ import logging import sys import textwrap +import warnings from oslo_config import cfg import stevedore @@ -89,19 +90,45 @@ def _format_help_text(description): if not description: return '#' - lines = description.splitlines() formatted_lines = [] - # wrap each line to support multi line descriptions - for line in lines: - if not line: + paragraph = [] + + def _wrap_paragraph(lines): + return textwrap.wrap(' '.join(lines), 70, initial_indent='# ', + subsequent_indent='# ') + + for line in description.strip().splitlines(): + if not line.strip(): + # empty line -> line break, so dump anything we have + formatted_lines.extend(_wrap_paragraph(paragraph)) formatted_lines.append('#') + paragraph = [] + elif len(line) == len(line.lstrip()): + # no leading whitespace = paragraph, which should be wrapped + paragraph.append(line.rstrip()) else: - formatted_lines.append(textwrap.fill(line, 70, - initial_indent='# ', - subsequent_indent='# ', - break_long_words=False, - replace_whitespace=False)) - return "\n".join(formatted_lines) + # leading whitespace - literal block, which should not be wrapping + if paragraph: + # ...however, literal blocks need a new line before them to + # delineate things + # TODO(stephenfin): Raise an exception here and stop doing + # anything else in oslo.policy 2.0 + warnings.warn( + 'Invalid policy description: literal blocks must be ' + 'preceded by a new line. This will raise an exception in ' + 'a future version of oslo.policy:\n%s' % description, + FutureWarning) + formatted_lines.extend(_wrap_paragraph(paragraph)) + formatted_lines.append('#') + paragraph = [] + + formatted_lines.append('# %s' % line.rstrip()) + + if paragraph: + # dump anything we might still have in the buffer + formatted_lines.extend(_wrap_paragraph(paragraph)) + + return '\n'.join(formatted_lines) def _format_rule_default_yaml(default, include_help=True): diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py index 2052572b..74f75014 100644 --- a/oslo_policy/tests/test_generator.py +++ b/oslo_policy/tests/test_generator.py @@ -11,6 +11,7 @@ import operator import sys +import warnings import fixtures import mock @@ -227,10 +228,9 @@ class GenerateSampleYAMLTestCase(base.PolicyBaseTestCase): expected = '''# Create a bar. #"foo:create_bar": "role:fizz" -# DEPRECATED -# "foo:post_bar":"role:fizz" has been deprecated since N in favor of -# "foo:create_bar":"role:fizz". -# foo:post_bar is being removed in favor of foo:create_bar +# DEPRECATED "foo:post_bar":"role:fizz" has been deprecated since N in +# favor of "foo:create_bar":"role:fizz". foo:post_bar is being removed +# in favor of foo:create_bar "foo:post_bar": "rule:foo:create_bar" ''' stdout = self._capture_stdout() @@ -244,26 +244,15 @@ class GenerateSampleYAMLTestCase(base.PolicyBaseTestCase): ) self.assertEqual(expected, stdout.getvalue()) - def test_empty_line_formatting(self): + def _test_formatting(self, description, expected): rule = [policy.RuleDefault('admin', 'is_admin:True', - description='Check Summary \n' - '\n' - 'This is a description to ' - 'check that empty line has ' - 'no white spaces.')] + description=description)] ext = stevedore.extension.Extension(name='check_rule', entry_point=None, plugin=None, obj=rule) test_mgr = stevedore.named.NamedExtensionManager.make_test_instance( extensions=[ext], namespace=['check_rule']) - # no whitespace on empty line - expected = '''# Check Summary -# -# This is a description to check that empty line has no white spaces. -#"admin": "is_admin:True" - -''' output_file = self.get_config_file_fullname('policy.yaml') with mock.patch('stevedore.named.NamedExtensionManager', return_value=test_mgr) as mock_ext_mgr: @@ -278,6 +267,76 @@ class GenerateSampleYAMLTestCase(base.PolicyBaseTestCase): self.assertEqual(expected, written_policy) + def test_empty_line_formatting(self): + description = ('Check Summary \n' + '\n' + 'This is a description to ' + 'check that empty line has ' + 'no white spaces.') + expected = """# Check Summary +# +# This is a description to check that empty line has no white spaces. +#"admin": "is_admin:True" + +""" + + self._test_formatting(description, expected) + + def test_paragraph_formatting(self): + description = """ +Here's a neat description with a paragraph. We want to make sure that it wraps +properly. +""" + expected = """# Here's a neat description with a paragraph. We want \ +to make sure +# that it wraps properly. +#"admin": "is_admin:True" + +""" + + self._test_formatting(description, expected) + + def test_literal_block_formatting(self): + description = """Here's another description. + + This one has a literal block. + These lines should be kept apart. + They should not be wrapped, even though they may be longer than 70 chars +""" + expected = """# Here's another description. +# +# This one has a literal block. +# These lines should be kept apart. +# They should not be wrapped, even though they may be longer than 70 chars +#"admin": "is_admin:True" + +""" + + self._test_formatting(description, expected) + + def test_invalid_formatting(self): + description = """Here's a broken description. + +We have some text... + Followed by a literal block without any spaces. + We don't support definition lists, so this is just wrong! +""" + expected = """# Here's a broken description. +# +# We have some text... +# +# Followed by a literal block without any spaces. +# We don't support definition lists, so this is just wrong! +#"admin": "is_admin:True" + +""" + + with warnings.catch_warnings(record=True) as warns: + self._test_formatting(description, expected) + self.assertEqual(1, len(warns)) + self.assertTrue(issubclass(warns[-1].category, FutureWarning)) + self.assertIn('Invalid policy description', str(warns[-1].message)) + class GenerateSampleJSONTestCase(base.PolicyBaseTestCase): def setUp(self):