{Core} Provide error recommendation for validate_file_or_dict

src/azure-cli-core/azure/cli/core/commands/tests/test_validators.py

@@ -0,0 +1,55 @@
+# --------------------------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for license information.
+# --------------------------------------------------------------------------------------------
+
+import unittest
+import os
+
+from azure.cli.core.commands.validators import validate_file_or_dict, JSON_RECOMMENDATION_MESSAGE
+from azure.cli.core.azclierror import InvalidArgumentValueError
+
+
+class TestValidators(unittest.TestCase):
+
+    def test_validate_file_or_dict(self):
+        json_str = '{"name": "my-resource"}'
+        curr_dir = os.path.dirname(os.path.realpath(__file__))
+        temp_file = os.path.join(curr_dir, 'temp.json')
+
+        # Parse JSON file
+        with open(temp_file, 'w') as f:
+            f.write(json_str)
+        json_read = validate_file_or_dict(temp_file)
+        assert json_read['name'] == 'my-resource'
+        os.remove(temp_file)
+
+        # Parse in-line JSON string
+        json_read = validate_file_or_dict(json_str)
+        assert json_read['name'] == 'my-resource'
+
+        # Test error 1: Parse JSON file with error
+        with open(temp_file, 'w') as f:
+            f.write(json_str)
+            f.write('error!')
+        with self.assertRaisesRegex(InvalidArgumentValueError, 'Failed to parse file') as ex:
+            validate_file_or_dict(temp_file)
+        assert ex.exception.recommendations[0] == JSON_RECOMMENDATION_MESSAGE
+        assert len(ex.exception.recommendations) == 1
+        os.remove(temp_file)
+
+        # Test error 2: A non-existing file, but it looks like a JSON file
+        with self.assertRaisesRegex(InvalidArgumentValueError, 'JSON file does not exist') as ex:
+            validate_file_or_dict("not-exist.json")
+        assert ex.exception.recommendations[0] == JSON_RECOMMENDATION_MESSAGE
+        assert len(ex.exception.recommendations) == 1
+
+        # Test error 3: A non-existing file, or invalid JSON string
+        with self.assertRaisesRegex(InvalidArgumentValueError, 'Failed to parse string as JSON') as ex:
+            validate_file_or_dict("invalid-string")
+        assert ex.exception.recommendations[0] == JSON_RECOMMENDATION_MESSAGE
+        assert 'The provided JSON string may have been parsed by the shell.' in ex.exception.recommendations[1]
+
+
+if __name__ == '__main__':
+    unittest.main()

src/azure-cli-core/azure/cli/core/commands/validators.py

@@ -10,8 +10,11 @@
 from azure.cli.core.profiles import ResourceType
 
 from knack.log import get_logger
+from knack.util import CLIError
 from knack.validators import DefaultStr, DefaultInt  # pylint: disable=unused-import
 
+JSON_RECOMMENDATION_MESSAGE = "Please provide a valid JSON file path or JSON string."
+
 logger = get_logger(__name__)
 
 
@@ -82,14 +85,32 @@ def get_default_location_from_resource_group(cmd, namespace):
 
 
 def validate_file_or_dict(string):
+    """Parse string as a JSON file or in-line JSON string."""
     import os
     string = os.path.expanduser(string)
-    if os.path.exists(string):
-        from azure.cli.core.util import get_file_json
-        return get_file_json(string)
-
-    from azure.cli.core.util import shell_safe_json_parse
-    return shell_safe_json_parse(string)
+    try:
+        if os.path.exists(string):
+            from azure.cli.core.util import get_file_json
+            # Error 1: 'string' is an existing file path, but the file contains invalid JSON string
+            # ex has no recommendation
+            return get_file_json(string)
+
+        # Error 2: If string ends with '.json', it can't be a JSON string, since a JSON string must ends with
+        # ], }, or ", so it must be JSON file, and we don't allow parsing it as in-line string
+        if string.endswith('.json'):
+            raise CLIError("JSON file does not exist: '{}'".format(string))
+
+        from azure.cli.core.util import shell_safe_json_parse
+        # Error 3: string is a non-existing file path or invalid JSON string
+        # ex has recommendations for shell interpretation
+        return shell_safe_json_parse(string)
+    except CLIError as ex:
+        from azure.cli.core.azclierror import InvalidArgumentValueError
+        new_ex = InvalidArgumentValueError(ex, recommendation=JSON_RECOMMENDATION_MESSAGE)
+        # Preserve the recommendation
+        if hasattr(ex, "recommendations"):
+            new_ex.set_recommendation(ex.recommendations)
+        raise new_ex from ex
 
 
 def validate_parameter_set(namespace, required, forbidden, dest_to_options=None, description=None):
@@ -100,8 +121,6 @@ def validate_parameter_set(namespace, required, forbidden, dest_to_options=None,
     included_forbidden = [x for x in forbidden if getattr(namespace, x) and
                           not hasattr(getattr(namespace, x), 'is_default')]
     if missing_required or included_forbidden:
-        from knack.util import CLIError
-
         def _dest_to_option(dest):
             try:
                 return dest_to_options[dest]

src/azure-cli-core/azure/cli/core/util.py

@@ -523,7 +523,8 @@ def get_file_json(file_path, throw_on_empty=True, preserve_order=False):
     try:
         return shell_safe_json_parse(content, preserve_order)
     except CLIError as ex:
-        raise CLIError("Failed to parse {} with exception:\n    {}".format(file_path, ex))
+        # Reading file bypasses shell interpretation, so we discard the recommendation for shell quoting.
+        raise CLIError("Failed to parse file '{}' with exception:\n{}".format(file_path, ex))
 
 
 def read_file_content(file_path, allow_binary=False):
@@ -563,12 +564,13 @@ def shell_safe_json_parse(json_or_dict_string, preserve_order=False, strict=True
         except Exception as ex:
             logger.debug(ex)  # log the exception which could be a python dict parsing error.
 
-            # Echo the JSON received by CLI
-            msg = "Failed to parse JSON: {}\nError detail: {}".format(json_or_dict_string, json_ex)
+            # Echo the string received by CLI. Because the user may intend to provide a file path, we don't decisively
+            # say it is a JSON string.
+            msg = "Failed to parse string as JSON:\n{}\nError detail: {}".format(json_or_dict_string, json_ex)
 
             # Recommendation for all shells
             from azure.cli.core.azclierror import InvalidArgumentValueError
-            recommendation = "The JSON may have been parsed by the shell. See " \
+            recommendation = "The provided JSON string may have been parsed by the shell. See " \
                              "https://docs.microsoft.com/cli/azure/use-cli-effectively#use-quotation-marks-in-arguments"
 
             # Recommendation especially for PowerShell