Add support for passing json values to arguments as .txt files (micro…

…soft#84)

Add support for inputting JSON values by providing a path to a file rather than only by provided a JSON string. 

To do so, set argument value to the relative or absolute path of the text file prefixed by "@".

requirements.txt

@@ -3,4 +3,5 @@ python-coveralls
 nose2
 pylint
 vcrpy
+contextlib2
 mock

src/README.rst

@@ -26,7 +26,8 @@ Unreleased
 - Fix bug in displaying property help text (#71)
 - Add tests to verify correctness of help text (#71)
 - Add scaling policy parameter to the command for service create and the command for service update (#76)
-- Add container group with commands: invoke-api(invoke raw container REST API), logs(get container logs) (#82)
+- Add new group called container with commands: invoke-api (invoke raw container REST API) and logs (get container logs) (#82)
+- Add support for passing json values to arguments as .txt files (#84)
 
 4.0.0
 -----

src/setup.py

@@ -58,7 +58,8 @@ def read(fname):
             'nose2',
             'pylint',
             'vcrpy',
-            'mock'
+            'mock',
+            'contextlib2'
         ]
     },
     entry_points={

src/sfctl/params.py

@@ -5,15 +5,39 @@
 # -----------------------------------------------------------------------------
 
 """Custom parameter handling for commands"""
+from __future__ import print_function
 import json
 from knack.arguments import (ArgumentsContext, CLIArgumentType)
 
-def json_encoded(arg_str):
-    """Convert from argument JSON string to complex object"""
-
-    return json.loads(arg_str)
 
-def custom_arguments(self, _): #pylint: disable=too-many-statements
+def json_encoded(arg_str):
+    """Convert from argument JSON string to complex object.
+    This function also accepts a file path to a .txt file containing the JSON string.
+    File paths should be prefixed by '@'
+    Path can be relative path or absolute path."""
+
+    if (arg_str and arg_str[0] == '@'):
+        try:
+            with open(arg_str[1:], 'r') as json_file:
+                json_str = json_file.read()
+                return json.loads(json_str)
+        except IOError:
+            # This is the error that python 2.7 returns on no file found
+            pass
+        except ValueError as ex:
+            print('Decoding JSON value from file {0} failed: \n{1}'.format(arg_str[1:], ex))
+            raise
+
+    try:
+        return json.loads(arg_str)
+    except ValueError:
+        print('Hint: You can also pass the json argument in a .txt file. '
+              'To do so, set argument value to the relative or absolute path of the text file '
+              'prefixed by "@".')
+        raise
+
+
+def custom_arguments(self, _):  # pylint: disable=too-many-statements
     """Load specialized arguments for commands"""
 
     # Global argument
@@ -179,4 +203,4 @@ def custom_arguments(self, _): #pylint: disable=too-many-statements
     with ArgumentsContext(self, 'is') as arg_context:
         # expect the parameter command_input in the python method as --command in commandline.
         arg_context.argument('command_input',
-                             CLIArgumentType(options_list=('--command')))
+                             CLIArgumentType(options_list='--command'))

src/sfctl/tests/command_processing_test.py

@@ -0,0 +1,149 @@
+# -----------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for
+# license information.
+# -----------------------------------------------------------------------------
+
+"""Tests to ensure that commands are processed correctly"""
+
+from os import path
+import unittest
+from contextlib2 import redirect_stdout
+from sfctl.params import json_encoded
+
+try:
+    # Python 2
+    from cStringIO import StringIO
+except ImportError:
+    # Python 3
+    from io import StringIO
+
+
+class CommandsProcessTests(unittest.TestCase):
+    """Processing commands tests"""
+
+    def test_json_encoded_argument_processing_file_input(self):  # pylint: disable=invalid-name
+        """Make sure that method json_encoded in src/params.py correctly:
+            - Reads the .txt files
+            - If input is not a file, reads and serializes the input as json
+            - Returns correct error messages
+        """
+
+        # --------------------------------------
+        # Pass in json as a file
+        # --------------------------------------
+
+        # Create object that contains the correct object that should be loaded from reading the file
+        pets_dictionary = dict()
+        pets_dictionary['Coco'] = 'Golden Retriever'
+        pets_dictionary['Lily'] = 'Ragdoll Cat'
+        pets_dictionary['Poofy'] = 'Golden Doodle'
+
+        dictionary = dict()
+        dictionary['name'] = 'John'
+        dictionary['last_name'] = 'Smith'
+        dictionary['pets'] = pets_dictionary
+
+        # Test .txt files containing json live in the same folder as this file.
+        # Get their full paths.
+        file_path_correct_json = '@' + path.join(path.dirname(__file__), 'correct_json.txt')
+        file_path_incorrect_json = '@' + path.join(path.dirname(__file__), 'incorrect_json.txt')
+        file_path_empty_file = '@' + path.join(path.dirname(__file__), 'empty_file.txt')
+
+        # Use str_io capture here to avoid the printed clutter when running the tests.
+        # Using ValueError instead of json.decoder.JSONDecodeError because that is not
+        # supported in python 2.7.
+        # Test that incorrect or empty file paths return error.
+        str_io = StringIO()
+        with redirect_stdout(str_io):
+            with self.assertRaises(ValueError):
+                json_encoded(file_path_empty_file)
+            with self.assertRaises(ValueError):
+                json_encoded(file_path_incorrect_json)
+
+        # Test that correct file path returns correct serialized object
+        self.assertEqual(dictionary, json_encoded(file_path_correct_json))
+
+        # Test that appropriate error messages are printed out on error
+
+        str_io = StringIO()
+        with redirect_stdout(str_io):
+            try:
+                json_encoded(file_path_empty_file)
+            except Exception:  # pylint: disable=broad-except
+                pass
+
+        printed_output = str_io.getvalue()
+        self.assertIn('Decoding JSON value from file {0} failed'.format(file_path_empty_file.lstrip('@')),  # pylint: disable=line-too-long
+                      printed_output)
+        self.assertTrue('Expecting value: line 1 column 1 (char 0)' in printed_output
+                        or
+                        'No JSON object could be decoded' in printed_output)
+        self.assertNotIn('Hint: You can also pass the json argument in a .txt file', printed_output)
+
+        str_io = StringIO()
+        with redirect_stdout(str_io):
+            try:
+                json_encoded(file_path_incorrect_json)
+            except Exception:  # pylint: disable=broad-except
+                pass
+
+        printed_output = str_io.getvalue()
+        self.assertIn('Decoding JSON value from file {0} failed'.format(file_path_incorrect_json.lstrip('@')),  # pylint: disable=line-too-long
+                      printed_output)
+        self.assertTrue('Expecting property name enclosed in double quotes: line 1 column 2 (char 1)' in printed_output  # pylint: disable=line-too-long
+                        or
+                        'Expecting property name: line 1 column 2 (char 1)' in printed_output)
+        self.assertNotIn('Hint: You can also pass the json argument in a .txt file', printed_output)
+
+    def test_json_encoded_argument_processing_string_input(self):  # pylint: disable=invalid-name
+        """Make sure that method json_encoded in src/params.py correctly:
+            - Reads the .txt files
+            - If input is not a file, reads and serializes the input as json
+            - Returns correct error messages
+        """
+
+        # --------------------------------------
+        # Pass in json as a string
+        # --------------------------------------
+
+        str_io = StringIO()
+
+        # str_io captures the output of a wrongly formatted json
+        with redirect_stdout(str_io):
+            try:
+                json_encoded('')
+            except Exception:  # pylint: disable=broad-except
+                pass
+
+        printed_output = str_io.getvalue()
+        self.assertIn('Hint: You can also pass the json argument in a .txt file', printed_output)
+        self.assertIn('To do so, set argument value to the relative or '
+                      'absolute path of the text file prefixed by "@".', printed_output)
+
+        # str_io captures the output of a wrongly formatted json
+        str_io = StringIO()
+        with redirect_stdout(str_io):
+            try:
+                json_encoded('{3.14 : "pie"}')
+            except Exception:  # pylint: disable=broad-except
+                pass
+
+        printed_output = str_io.getvalue()
+        self.assertIn('Hint: You can also pass the json argument in a .txt file', printed_output)
+        self.assertIn('To do so, set argument value to the relative or '
+                      'absolute path of the text file prefixed by "@".', printed_output)
+
+        # Capture output with str_io even though it's not used in order to prevent test to writing
+        # to output, in order to keep tests looking clean.
+        # These tests ensure that incorrectly formatted json throws error.
+        str_io = StringIO()
+        with redirect_stdout(str_io):
+            with self.assertRaises(ValueError):
+                json_encoded('')
+            with self.assertRaises(ValueError):
+                json_encoded('{3.14 : "pie"}')
+
+        # Test to ensure that correct json is serialized correctly.
+        simple_dictionary = {'k': 23}
+        self.assertEqual(simple_dictionary, json_encoded('{"k": 23}'))

src/sfctl/tests/correct_json.txt

@@ -0,0 +1,9 @@
+{
+    "name":"John",
+    "last_name": "Smith",
+    "pets": {
+        "Coco":"Golden Retriever",
+        "Lily":"Ragdoll Cat",
+        "Poofy":"Golden Doodle"
+    }
+ }

src/sfctl/tests/help_text_test.py

@@ -9,7 +9,6 @@
 
 from __future__ import print_function
 import unittest
-from sys import stderr
 from subprocess import Popen, PIPE
 
 
@@ -188,7 +187,6 @@ def validate_output(self, command_input, subgroups=(), commands=()):  # pylint:
 
             if err:
                 err = err.decode('utf-8')
-                print(err, file=stderr)
                 self.assertEqual(b'', err, msg='ERROR: in command: ' + help_command)
 
             if not returned_string:
@@ -198,7 +196,6 @@ def validate_output(self, command_input, subgroups=(), commands=()):  # pylint:
             lines = returned_string.splitlines()
 
             for line in lines:
-                print(line, file=stderr)
 
                 if not line.strip():
                     continue
@@ -243,8 +240,6 @@ def validate_output(self, command_input, subgroups=(), commands=()):  # pylint:
                                   + help_command
                                   + '. This may be a problem due incorrect expected ordering.'))
 
-            print(file=stderr)
-
         except Exception as exception:  # pylint: disable=broad-except
             if not err:
                 self.fail(msg='ERROR: Command {0} returned error at execution. Output: {1} Error: {2}'.format(help_command, returned_string, str(exception)))  # pylint: disable=line-too-long

src/sfctl/tests/incorrect_json.txt

@@ -0,0 +1 @@
+{'this_should_be_double_quotes', 70}

src/sfctl/tests/request_generation_test.py

@@ -12,6 +12,7 @@
 from __future__ import print_function
 from os import (remove, environ)
 import json
+import logging
 import vcr
 from mock import patch
 from knack.testsdk import ScenarioTest
@@ -109,12 +110,18 @@ def validate_command(self, command, method, path, query, body=None,  # pylint: d
 
         # This calls the command and the HTTP request it recorded into
         # generated_file_path
+
+        # Reduce noise in test output for this test only
+        logging.disable(logging.INFO)
         with vcr.use_cassette('paths_generation_test.json', record_mode='all', serializer='json'):
             try:
                 self.cmd(command)
             except Exception as exception:  # pylint: disable=broad-except
                 self.fail('ERROR while running command "{0}". Error: "{1}"'.format(command, str(exception)))
 
+        # re-enable logging
+        logging.disable(logging.NOTSET)
+
         # Read recorded JSON file
         with open(generated_file_path, 'r') as http_recording_file:
             json_str = http_recording_file.read()
@@ -194,6 +201,7 @@ def paths_generation_helper(self):  # pylint: disable=too-many-statements
              '"Async": false, '
              '"ApplicationTypeBuildPath": "test_path"}'),
             validate_flat_dictionary)
+
         self.validate_command(  # provision-application-type external-store
             'application provision --external-provision '
             '--application-package-download-uri=test_path --application-type-name=name '
@@ -359,7 +367,7 @@ def paths_generation_helper(self):  # pylint: disable=too-many-statements
             validate_flat_dictionary)
 
         # container commands
-        self.validate_command( # get container logs
+        self.validate_command(  # get container logs
             'sfctl container invoke-api --node-name Node01 --application-id samples/winnodejs '
             '--service-manifest-name NodeServicePackage --code-package-name NodeService.Code '
             '--code-package-instance-id 131668159770315380 --container-api-uri-path "/containers/{id}/logs?stdout=true&stderr=true"',
@@ -368,7 +376,7 @@ def paths_generation_helper(self):  # pylint: disable=too-many-statements
             ['api-version=6.2', 'ServiceManifestName=NodeServicePackage', 'CodePackageName=NodeService.Code', 'CodePackageInstanceId=131668159770315380', 'timeout=60'],
             ('{"UriPath": "/containers/{id}/logs?stdout=true&stderr=true"}'),
             validate_flat_dictionary)
-        self.validate_command( # get container logs
+        self.validate_command(  # get container logs
             'sfctl container logs --node-name Node01 --application-id samples/winnodejs '
             '--service-manifest-name NodeServicePackage --code-package-name NodeService.Code '
             '--code-package-instance-id 131668159770315380',
@@ -377,10 +385,10 @@ def paths_generation_helper(self):  # pylint: disable=too-many-statements
             ['api-version=6.2', 'ServiceManifestName=NodeServicePackage', 'CodePackageName=NodeService.Code', 'CodePackageInstanceId=131668159770315380', 'timeout=60'],
             ('{"UriPath": "/containers/{id}/logs?stdout=true&stderr=true"}'),
             validate_flat_dictionary)
-        self.validate_command( # update container
+        self.validate_command(  # update container
             'sfctl container invoke-api --node-name N0020 --application-id nodejs1 --service-manifest-name NodeOnSF '
             '--code-package-name Code --code-package-instance-id 131673596679688285 --container-api-uri-path "/containers/{id}/update"'
-            ' --container-api-http-verb=POST --container-api-body "DummyRequestBody"', # Manual testing with a JSON string for "--container-api-body" works,
+            ' --container-api-http-verb=POST --container-api-body "DummyRequestBody"',  # Manual testing with a JSON string for "--container-api-body" works,
             # Have to pass "DummyRequestBody" here since a real JSON string confuses test validation code.
             'POST',
             '/Nodes/N0020/$/GetApplications/nodejs1/$/GetCodePackages/$/ContainerApi',
@@ -612,12 +620,12 @@ def paths_generation_helper(self):  # pylint: disable=too-many-statements
             'GET',
             '/Partitions/id/$/GetReplicas/replicaId/$/GetHealth',
             ['api-version=6.0', 'EventsHealthStateFilter=2'])
-        self.validate_command( # info
+        self.validate_command(  # info
             'replica info --partition-id=id --replica-id=replicaId',
             'GET',
             '/Partitions/id/$/GetReplicas/replicaId',
             ['api-version=6.0'])
-        self.validate_command( # list
+        self.validate_command(  # list
             'replica list --continuation-token=ct --partition-id=id',
             'GET',
             '/Partitions/id/$/GetReplicas',
@@ -740,7 +748,7 @@ def paths_generation_helper(self):  # pylint: disable=too-many-statements
             validate_flat_dictionary)
 
         # Chaos commands:
-        self.validate_command(#get chaos schedule
+        self.validate_command(  # get chaos schedule
             'chaos schedule set ' +
             '--version 0 --start-date-utc 2016-01-01T00:00:00.000Z ' +
             '--expiry-date-utc 2038-01-01T00:00:00.000Z ' +
@@ -765,25 +773,25 @@ def paths_generation_helper(self):  # pylint: disable=too-many-statements
             '/Tools/Chaos/Schedule',
             ['api-version=6.2'])
 
-        self.validate_command(#get chaos schedule
+        self.validate_command(  # get chaos schedule
             'chaos schedule get',
             'GET',
             '/Tools/Chaos/Schedule',
             ['api-version=6.2'])
 
-        self.validate_command(#stop chaos
+        self.validate_command(  # stop chaos
             'chaos stop',
             'POST',
             '/Tools/Chaos/$/Stop',
             ['api-version=6.0'])
 
-        self.validate_command(#get chaos events
+        self.validate_command(  # get chaos events
             'chaos events',
             'GET',
             '/Tools/Chaos/Events',
             ['api-version=6.2'])
 
-        self.validate_command(#get chaos
+        self.validate_command(  # get chaos
             'chaos get',
             'GET',
             '/Tools/Chaos',