diff --git a/osc_lib/cli/parseractions.py b/osc_lib/cli/parseractions.py index 85d7310..88cd7bf 100644 --- a/osc_lib/cli/parseractions.py +++ b/osc_lib/cli/parseractions.py @@ -137,6 +137,77 @@ class MultiKeyValueAction(argparse.Action): getattr(namespace, self.dest, []).append(params) +class MultiKeyValueCommaAction(MultiKeyValueAction): + """Custom action to parse arguments from a set of key=value pair + + Ensures that ``dest`` is a dict. + Parses dict by separating comma separated string into individual values + Ex. key1=val1,val2,key2=val3 => {"key1": "val1,val2", "key2": "val3"} + """ + + def __call__(self, parser, namespace, values, option_string=None): + """Overwrite the __call__ function of MultiKeyValueAction + + This is done to handle scenarios where we may have comma seperated + data as a single value. + """ + # Make sure we have an empty list rather than None + if getattr(namespace, self.dest, None) is None: + setattr(namespace, self.dest, []) + + params = {} + key = '' + for kv in values.split(','): + # Add value if an assignment else raise ArgumentTypeError + if '=' in kv: + kv_list = kv.split('=', 1) + # NOTE(qtang): Prevent null key setting in property + if '' == kv_list[0]: + msg = _("A key must be specified before '=': %s") + raise argparse.ArgumentTypeError(msg % str(kv)) + else: + params.update([kv_list]) + key = kv_list[0] + else: + # If the ',' split does not have key=value pair, then it + # means the current value is a part of the previous + # key=value pair, so append it. + try: + params[key] = "%s,%s" % (params[key], kv) + except KeyError: + msg = _("A key=value pair is required: %s") + raise argparse.ArgumentTypeError(msg % str(kv)) + + # Check key validation + valid_keys = self.required_keys | self.optional_keys + if valid_keys: + invalid_keys = [k for k in params if k not in valid_keys] + if invalid_keys: + msg = _( + "Invalid keys %(invalid_keys)s specified.\n" + "Valid keys are: %(valid_keys)s" + ) + raise argparse.ArgumentTypeError(msg % { + 'invalid_keys': ', '.join(invalid_keys), + 'valid_keys': ', '.join(valid_keys), + }) + + if self.required_keys: + missing_keys = [k for k in self.required_keys if k not in params] + if missing_keys: + msg = _( + "Missing required keys %(missing_keys)s.\n" + "Required keys are: %(required_keys)s" + ) + raise argparse.ArgumentTypeError(msg % { + 'missing_keys': ', '.join(missing_keys), + 'required_keys': ', '.join(self.required_keys), + }) + + # Update the dest dict + getattr(namespace, self.dest, []).append(params) + + class RangeAction(argparse.Action): """A custom action to parse a single value or a range of values diff --git a/osc_lib/tests/cli/test_parseractions.py b/osc_lib/tests/cli/test_parseractions.py index 451f44e..5c62910 100644 --- a/osc_lib/tests/cli/test_parseractions.py +++ b/osc_lib/tests/cli/test_parseractions.py @@ -181,6 +181,214 @@ class TestMultiKeyValueAction(utils.TestCase): ) +class TestMultiKeyValueCommaAction(utils.TestCase): + + def setUp(self): + super(TestMultiKeyValueCommaAction, self).setUp() + self.parser = argparse.ArgumentParser() + + # Typical usage + self.parser.add_argument( + '--test', + metavar='req1=xxx,yyy', + action=parseractions.MultiKeyValueCommaAction, + dest='test', + default=None, + required_keys=['req1'], + optional_keys=['opt2'], + help='Test', + ) + + def test_mkvca_required(self): + results = self.parser.parse_args([ + '--test', 'req1=aaa,bbb', + ]) + actual = getattr(results, 'test', []) + expect = [ + {'req1': 'aaa,bbb'}, + ] + self.assertItemsEqual(expect, actual) + + results = self.parser.parse_args([ + '--test', 'req1=', + ]) + actual = getattr(results, 'test', []) + expect = [ + {'req1': ''}, + ] + self.assertItemsEqual(expect, actual) + + results = self.parser.parse_args([ + '--test', 'req1=aaa,bbb', + '--test', 'req1=', + ]) + actual = getattr(results, 'test', []) + expect = [ + {'req1': 'aaa,bbb'}, + {'req1': ''}, + ] + self.assertItemsEqual(expect, actual) + + def test_mkvca_optional(self): + results = self.parser.parse_args([ + '--test', 'req1=aaa,bbb', + ]) + actual = getattr(results, 'test', []) + expect = [ + {'req1': 'aaa,bbb'}, + ] + self.assertItemsEqual(expect, actual) + + results = self.parser.parse_args([ + '--test', 'req1=aaa,bbb', + '--test', 'req1=,opt2=ccc', + ]) + actual = getattr(results, 'test', []) + expect = [ + {'req1': 'aaa,bbb'}, + {'req1': '', 'opt2': 'ccc'}, + ] + self.assertItemsEqual(expect, actual) + + try: + results = self.parser.parse_args([ + '--test', 'req1=aaa,bbb', + '--test', 'opt2=ccc', + ]) + self.fail('ArgumentTypeError should be raised') + except argparse.ArgumentTypeError as e: + self.assertEqual( + 'Missing required keys req1.\nRequired keys are: req1', + str(e), + ) + + def test_mkvca_multiples(self): + results = self.parser.parse_args([ + '--test', 'req1=aaa,bbb,opt2=ccc', + ]) + actual = getattr(results, 'test', []) + expect = [{ + 'req1': 'aaa,bbb', + 'opt2': 'ccc', + }] + self.assertItemsEqual(expect, actual) + + def test_mkvca_no_required_optional(self): + self.parser.add_argument( + '--test-empty', + metavar='req1=xxx,yyy', + action=parseractions.MultiKeyValueCommaAction, + dest='test_empty', + default=None, + required_keys=[], + optional_keys=[], + help='Test', + ) + + results = self.parser.parse_args([ + '--test-empty', 'req1=aaa,bbb', + ]) + actual = getattr(results, 'test_empty', []) + expect = [ + {'req1': 'aaa,bbb'}, + ] + self.assertItemsEqual(expect, actual) + + results = self.parser.parse_args([ + '--test-empty', 'xyz=aaa,bbb', + ]) + + actual = getattr(results, 'test_empty', []) + expect = [ + {'xyz': 'aaa,bbb'}, + ] + self.assertItemsEqual(expect, actual) + + def test_mkvca_invalid_key(self): + try: + self.parser.parse_args([ + '--test', 'req1=aaa,bbb=', + ]) + self.fail('ArgumentTypeError should be raised') + except argparse.ArgumentTypeError as e: + self.assertIn( + 'Invalid keys bbb specified.\nValid keys are:', + str(e), + ) + + try: + self.parser.parse_args([ + '--test', 'nnn=aaa', + ]) + self.fail('ArgumentTypeError should be raised') + except argparse.ArgumentTypeError as e: + self.assertIn( + 'Invalid keys nnn specified.\nValid keys are:', + str(e), + ) + + def test_mkvca_value_no_key(self): + try: + self.parser.parse_args([ + '--test', 'req1=aaa,=bbb', + ]) + self.fail('ArgumentTypeError should be raised') + except argparse.ArgumentTypeError as e: + self.assertEqual( + "A key must be specified before '=': =bbb", + str(e), + ) + try: + self.parser.parse_args([ + '--test', '=nnn', + ]) + self.fail('ArgumentTypeError should be raised') + except argparse.ArgumentTypeError as e: + self.assertEqual( + "A key must be specified before '=': =nnn", + str(e), + ) + + try: + self.parser.parse_args([ + '--test', 'nnn', + ]) + self.fail('ArgumentTypeError should be raised') + except argparse.ArgumentTypeError as e: + self.assertIn( + 'A key=value pair is required:', + str(e), + ) + + def test_mkvca_required_keys_not_list(self): + self.assertRaises( + TypeError, + self.parser.add_argument, + '--test-required-dict', + metavar='req1=xxx', + action=parseractions.MultiKeyValueCommaAction, + dest='test_required_dict', + default=None, + required_keys={'aaa': 'bbb'}, + optional_keys=['opt1', 'opt2'], + help='Test', + ) + + def test_mkvca_optional_keys_not_list(self): + self.assertRaises( + TypeError, + self.parser.add_argument, + '--test-optional-dict', + metavar='req1=xxx', + action=parseractions.MultiKeyValueCommaAction, + dest='test_optional_dict', + default=None, + required_keys=['req1', 'req2'], + optional_keys={'aaa': 'bbb'}, + help='Test', + ) + + class TestNonNegativeAction(utils.TestCase): def setUp(self): diff --git a/releasenotes/notes/add-MultiKeyValueCommaAction-class-01dd254a287d70d2.yaml b/releasenotes/notes/add-MultiKeyValueCommaAction-class-01dd254a287d70d2.yaml new file mode 100644 index 0000000..d8f1d6f --- /dev/null +++ b/releasenotes/notes/add-MultiKeyValueCommaAction-class-01dd254a287d70d2.yaml @@ -0,0 +1,6 @@ +--- +feature: + - | + Add ``MultiKeyValueCommaAction`` as a ``MultiKeyValueAction`` sublass + that allows values to include a comma. For example: + ``--property key1=value1,value2,key2=value3,value4,value5``.