Feature/output formats (#128)

kiran-chaudhary · web-flow · commit 6b1a7daaa29d · 2020-07-31T19:45:49.000+05:30
* Added support for multiple output formats, ascii-table, csv, formatted-json, json

* Added tests

* Reverts dependency of texttable module, uses existing table logic instead

   Refactored find_format_method to return tabular format output instead
   of printing it.

* Added format options to command secuirty-data list and legal-hold list and show subcommands

* Set newline to default value

* Filter json results

* refactor- proper naming convention

* Added changelog

* Fix failing test

* Added test for commands with -f option

* imporvise docs

* Fix 3.5 test failure
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,11 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 - `--password` option added to `profile create` and `profile update` commands, enabling creating profiles while bypassing the interactive password prompt.
 - Profiles can now save multiple alert and file event checkpoints. The name of the checkpoint to be used for a given query should be passed to `-c` (`--use-checkpoint`).
 - `-y/--assume-yes` option added to `profile delete` and `profile delete-all` commands to not require interactive prompt.
+- Below subcommands accept argument `--format/-f` to display result in formats `csv`, `table`, `json`, `formatted-json`:
+    - `code42 alert-rules list`
+    - `code42 legal-hold list`
+    - `code42 legal-hold show`
+    - `code42 security-data saved-search list`
 
 ### Removed
 
diff --git a/src/code42cli/cmds/alert_rules.py b/src/code42cli/cmds/alert_rules.py
@@ -14,8 +14,7 @@
 from code42cli.file_readers import read_csv_arg
 from code42cli.options import OrderedGroup
 from code42cli.options import sdk_options
-from code42cli.util import find_format_width
-from code42cli.util import format_to_table
+from code42cli.output_formats import format_option
 
 
 class AlertRuleTypes:
@@ -75,13 +74,14 @@ def remove_user(state, rule_id, username):
 
 
 @alert_rules.command("list")
+@format_option
 @sdk_options()
-def list_alert_rules(state):
+def list_alert_rules(state, format=None):
     """Fetch existing alert rules."""
     selected_rules = _get_all_rules_metadata(state.sdk)
     if selected_rules:
-        rows, column_size = find_format_width(selected_rules, _HEADER_KEYS_MAP)
-        format_to_table(rows, column_size)
+        formatted_output = format(selected_rules, _HEADER_KEYS_MAP)
+        echo(formatted_output)
 
 
 @alert_rules.command()
diff --git a/src/code42cli/cmds/enums.py b/src/code42cli/cmds/enums.py
@@ -0,0 +1,8 @@
+class OutputFormat:
+    TABLE = "TABLE"
+    CSV = "CSV"
+    JSON = "JSON"
+    RAW = "RAW-JSON"
+
+    def __iter__(self):
+        return iter([self.TABLE, self.CSV, self.JSON, self.RAW])
diff --git a/src/code42cli/cmds/legal_hold.py b/src/code42cli/cmds/legal_hold.py
@@ -17,9 +17,9 @@
 from code42cli.file_readers import read_csv_arg
 from code42cli.options import OrderedGroup
 from code42cli.options import sdk_options
-from code42cli.util import find_format_width
+from code42cli.output_formats import format_option
 from code42cli.util import format_string_list_to_columns
-from code42cli.util import format_to_table
+
 
 _MATTER_KEYS_MAP = OrderedDict()
 _MATTER_KEYS_MAP["legalHoldUid"] = "Matter ID"
@@ -71,21 +71,23 @@ def remove_user(state, matter_id, username):
 
 
 @legal_hold.command("list")
+@format_option
 @sdk_options()
-def _list(state):
+def _list(state, format=None):
     """Fetch existing legal hold matters."""
     matters = _get_all_active_matters(state.sdk)
     if matters:
-        rows, column_size = find_format_width(matters, _MATTER_KEYS_MAP)
-        format_to_table(rows, column_size)
+        output = format(matters, _MATTER_KEYS_MAP)
+        echo(output)
 
 
 @legal_hold.command()
 @click.argument("matter-id")
 @click.option("--include-inactive", is_flag=True)
 @click.option("--include-policy", is_flag=True)
+@format_option
 @sdk_options()
-def show(state, matter_id, include_inactive=False, include_policy=False):
+def show(state, matter_id, include_inactive=False, include_policy=False, format=None):
     """Display details of a given legal hold matter."""
     matter = _check_matter_is_accessible(state.sdk, matter_id)
     matter["creator_username"] = matter["creator"]["username"]
@@ -103,10 +105,9 @@ def show(state, matter_id, include_inactive=False, include_policy=False):
         member["user"]["username"] for member in memberships if not member["active"]
     ]
 
-    rows, column_size = find_format_width([matter], _MATTER_KEYS_MAP)
+    output = format([matter], _MATTER_KEYS_MAP)
+    echo(output)
 
-    echo("")
-    format_to_table(rows, column_size)
     _print_matter_members(active_usernames, member_type="active")
 
     if include_inactive:
diff --git a/src/code42cli/cmds/securitydata.py b/src/code42cli/cmds/securitydata.py
@@ -1,3 +1,4 @@
+from _collections import OrderedDict
 from pprint import pformat
 
 import click
@@ -15,11 +16,15 @@
 from code42cli.options import incompatible_with
 from code42cli.options import OrderedGroup
 from code42cli.options import sdk_options
-from code42cli.util import find_format_width
-from code42cli.util import format_to_table
+from code42cli.output_formats import format_option as format_output
+
 
 logger = get_main_cli_logger()
 
+_HEADER_KEYS_MAP = OrderedDict()
+_HEADER_KEYS_MAP["name"] = "Name"
+_HEADER_KEYS_MAP["id"] = "Id"
+
 search_options = searchopt.create_search_options("file events")
 
 format_option = click.option(
@@ -210,12 +215,15 @@ def saved_search(state):
 
 
 @saved_search.command("list")
+@format_output
 @sdk_options()
-def _list(state):
+def _list(state, format=None):
     """List available saved searches."""
     response = state.sdk.securitydata.savedsearches.get()
-    header = {"name": "Name", "id": "Id"}
-    format_to_table(*find_format_width(response["searches"], header))
+    result = response["searches"]
+    if result:
+        output = format(result, _HEADER_KEYS_MAP)
+        echo(output)
 
 
 @saved_search.command()
diff --git a/src/code42cli/output_formats.py b/src/code42cli/output_formats.py
@@ -0,0 +1,59 @@
+import json
+
+import click
+
+from code42cli.cmds.enums import OutputFormat
+from code42cli.util import find_format_width
+from code42cli.util import format_to_table
+
+
+def output_format(_, __, value):
+    if value is not None:
+        if value == OutputFormat.CSV:
+            return to_csv
+        if value == OutputFormat.RAW:
+            return to_json
+        if value == OutputFormat.TABLE:
+            return to_table
+        if value == OutputFormat.JSON:
+            return to_formatted_json
+    # default option
+    return to_table
+
+
+format_option = click.option(
+    "-f",
+    "--format",
+    type=click.Choice(OutputFormat(), case_sensitive=False),
+    help="The output format of the result. Defaults to table format.",
+    callback=output_format,
+)
+
+
+def to_csv(output, header):
+    columns = ",".join(header.values())
+
+    lines = []
+    lines.append(columns)
+    for row in output:
+        items = [str(row[key]) for key in header.keys()]
+        line = ",".join(items)
+        lines.append(line)
+    return "\n".join(lines)
+
+
+def to_table(output, header):
+    rows, column_size = find_format_width(output, header)
+    return format_to_table(rows, column_size)
+
+
+def _filter(output, header):
+    return [{header[key]: row[key] for key in header.keys()} for row in output]
+
+
+def to_json(output, header=None):
+    return json.dumps(_filter(output, header))
+
+
+def to_formatted_json(output, header=None):
+    return json.dumps(_filter(output, header), indent=4)
diff --git a/src/code42cli/util.py b/src/code42cli/util.py
@@ -68,11 +68,14 @@ def find_format_width(record, header):
 
 
 def format_to_table(rows, column_size):
-    """Prints result in left justified format in a tabular form."""
+    """Formats given rows into a string of left justified table."""
+    lines = []
     for row in rows:
+        line = ""
         for key in row.keys():
-            echo(str(row[key]).ljust(column_size[key] + _PADDING_SIZE), nl=False)
-        echo("")
+            line += str(row[key]).ljust(column_size[key] + _PADDING_SIZE)
+        lines.append(line)
+    return "\n".join(lines)
 
 
 def format_string_list_to_columns(string_list, max_width=None):
diff --git a/tests/cmds/test_alert_rules.py b/tests/cmds/test_alert_rules.py
@@ -25,6 +25,19 @@
     ]
 }
 
+TEST_RULE_RESPONSE = {
+    "ruleMetadata": [
+        {
+            "observerRuleId": TEST_RULE_ID,
+            "type": "FED_FILE_TYPE_MISMATCH",
+            "isEnabled": True,
+            "ruleSource": "NOTVALID",
+            "name": "Test",
+            "severity": "high",
+        }
+    ]
+}
+
 TEST_USER_RULE_RESPONSE = {
     "ruleMetadata": [
         {
@@ -275,3 +288,19 @@ def test_remove_bulk_users_uses_expected_arguments(runner, mocker, cli_state):
             cli, ["alert-rules", "bulk", "add", "test_remove.csv"], obj=cli_state
         )
     assert bulk_processor.call_args[0][1] == [{"rule_id": "test", "username": "value"}]
+
+
+def test_list_cmd_prints_no_rules_found_when_f_is_passed_and_response_is_empty(
+    runner, cli_state
+):
+    cli_state.sdk.alerts.rules.get_all.return_value = [TEST_EMPTY_RULE_RESPONSE]
+    result = runner.invoke(cli, ["alert-rules", "list", "-f", "csv"], obj=cli_state)
+    assert cli_state.sdk.alerts.rules.get_all.call_count == 1
+    assert "No alert rules found" in result.output
+
+
+def test_list_cmd_formats_to_csv_when_format_is_passed(runner, cli_state):
+    cli_state.sdk.alerts.rules.get_all.return_value = [TEST_RULE_RESPONSE]
+    result = runner.invoke(cli, ["alert-rules", "list", "-f", "csv"], obj=cli_state)
+    assert cli_state.sdk.alerts.rules.get_all.call_count == 1
+    assert "RuleId,Name,Severity,Type,Source,Enabled" in result.output
diff --git a/tests/cmds/test_legal_hold.py b/tests/cmds/test_legal_hold.py
@@ -63,6 +63,30 @@
     TEST_PRESERVATION_POLICY_UID
 )
 
+TEST_LEGAL_HOLD_LIST = [
+    {
+        "legalHolds": [
+            {
+                "legalHoldUid": "932880202064992021",
+                "name": "test",
+                "description": "",
+                "active": True,
+                "creationDate": "2019-12-19T20:32:10.763Z",
+                "lastModified": "2019-12-19T20:32:10.781Z",
+                "creator": {
+                    "userUid": "921286907298179098",
+                    "username": "test@test.test",
+                    "email": "test@test.test",
+                },
+                "holdPolicyUid": "901109555892625150",
+                "creator_username": "test@test.test",
+            },
+        ],
+    }
+]
+
+TEST_LEGAL_HOLD_EMPTY_LIST = [{"legalHolds": []}]
+
 
 @pytest.fixture
 def preservation_policy_response(mocker):
@@ -371,3 +395,36 @@ def test_remove_bulk_users_uses_expected_arguments(runner, mocker, cli_state):
         assert bulk_processor.call_args[0][1] == [
             {"matter_id": "test", "username": "value"}
         ]
+
+
+def test_list_with_format_option_returns_expected_format(runner, cli_state):
+    cli_state.sdk.legalhold.get_all_matters.return_value = TEST_LEGAL_HOLD_LIST
+
+    result = runner.invoke(cli, ["legal-hold", "list", "-f", "csv"], obj=cli_state)
+    assert "Matter ID,Name,Description,Creator,Creation Date" in result.output
+    assert "932880202064992021" in result.output
+
+
+def test_list_with_format_option_returns_no_response_when_response_is_empty(
+    runner, cli_state
+):
+    cli_state.sdk.legalhold.get_all_matters.return_value = TEST_LEGAL_HOLD_EMPTY_LIST
+    result = runner.invoke(cli, ["legal-hold", "list", "-f", "csv"], obj=cli_state)
+    assert "Matter ID,Name,Description,Creator,Creation Date" not in result.output
+
+
+def test_show_with_format_option_returns_expected_format(
+    runner, cli_state, check_matter_accessible_success, get_user_id_success
+):
+    cli_state.sdk.legalhold.get_all_matter_custodians.return_value = (
+        ACTIVE_AND_INACTIVE_LEGAL_HOLD_MEMBERSHIPS_RESULT
+    )
+    result = runner.invoke(
+        cli, ["legal-hold", "show", TEST_MATTER_ID, "-f", "csv"], obj=cli_state
+    )
+
+    assert "Matter ID,Name,Description,Creator,Creation Date" in result.output
+    assert (
+        "88888,Test_Matter,,legal_admin@example.com,2020-01-01T00:00:00.000-06:00"
+        in result.output
+    )
diff --git a/tests/cmds/test_securitydata.py b/tests/cmds/test_securitydata.py
@@ -19,6 +19,19 @@
 CURSOR_TIMESTAMP = 1579500000.0
 
 
+TEST_LIST_RESPONSE = {
+    "searches": [
+        {
+            "id": "a083f08d-8f33-4cbd-81c4-8d1820b61185",
+            "name": "test-events",
+            "notes": "py42 is here",
+        },
+    ]
+}
+
+TEST_EMPTY_LIST_RESPONSE = {"searches": []}
+
+
 @pytest.fixture
 def file_event_extractor(mocker):
     mock = mocker.patch(
@@ -691,3 +704,24 @@ def test_search_with_or_query_flag_produces_expected_query(runner, cli_state):
         str(cli_state.sdk.securitydata.search_file_events.call_args[0][0])
     )
     assert actual_query == expected_query
+
+
+def test_saved_search_list_with_format_option_returns_csv_formatted_response(
+    runner, cli_state
+):
+    cli_state.sdk.securitydata.savedsearches.get.return_value = TEST_LIST_RESPONSE
+    result = runner.invoke(
+        cli, ["security-data", "saved-search", "list", "-f", "csv"], obj=cli_state
+    )
+    assert "Name,Id" in result.output
+    assert "test-events,a083f08d-8f33-4cbd-81c4-8d1820b61185" in result.output
+
+
+def test_saved_search_list_with_format_option_does_not_return_when_response_is_empty(
+    runner, cli_state
+):
+    cli_state.sdk.securitydata.savedsearches.get.return_value = TEST_EMPTY_LIST_RESPONSE
+    result = runner.invoke(
+        cli, ["security-data", "saved-search", "list", "-f", "csv"], obj=cli_state
+    )
+    assert "Name,Id" not in result.output
diff --git a/tests/test_output_formats.py b/tests/test_output_formats.py
