Bugfix/attempt to autodetect file encodings when reading files (#265)

* Auto-detect file encoding on read_csv_arg

* add test

* use on flat file reader

* test ascii

* log when no encoding is detected

* init logger before logging

* move return out of finally to appease flake8

* changelog

* make FileOrString auto decoded, add test for FileOrString and file not exist case

Author: timabrmsn

CHANGELOG.md

@@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 The intended audience of this file is for py42 consumers -- as such, changes that don't affect
 how a consumer would use the library (e.g. adding unit tests, updating documentation, etc) are not captured here.
 
+## Unreleased
+
+### Fixed
+
+- Arguments/options that read data from files now attempt to autodetect file encodings. Resolving a bug where CSVs written
+  on Windows with Powershell would fail to be read properly.
+
 ## 1.4.0 - 2021-03-09
 
 ### Added

src/code42cli/click_ext/types.py

@@ -3,11 +3,29 @@
 from datetime import timedelta
 from datetime import timezone
 
+import chardet
 import click
 from click.exceptions import BadParameter
 
+from code42cli.logger import CliLogger
 
-class FileOrString(click.File):
+
+class AutoDecodedFile(click.File):
+    """Attempts to autodetect file's encoding prior to normal click.File processing."""
+
+    def convert(self, value, param, ctx):
+        try:
+            with open(value, "rb") as file:
+                self.encoding = chardet.detect(file.read())["encoding"]
+            if self.encoding is None:
+                CliLogger().log_error(f"Failed to detect encoding of file: {value}")
+        except Exception:
+            pass  # we'll let click.File do it's own exception handling for the filepath
+
+        return super().convert(value, param, ctx)
+
+
+class FileOrString(AutoDecodedFile):
     """Declares a parameter to be a file (if the argument begins with `@`), otherwise accepts it as
     a string.
     """

src/code42cli/file_readers.py

@@ -2,6 +2,7 @@
 
 import click
 
+from code42cli.click_ext.types import AutoDecodedFile
 from code42cli.errors import Code42CLIError
 
 
@@ -13,7 +14,7 @@ def read_csv_arg(headers):
     return click.argument(
         "csv_rows",
         metavar="CSV_FILE",
-        type=click.File("r"),
+        type=AutoDecodedFile("r"),
         callback=lambda ctx, param, arg: read_csv(arg, headers=headers),
     )
 
@@ -65,7 +66,7 @@ def read_flat_file(file):
 
 read_flat_file_arg = click.argument(
     "file_rows",
-    type=click.File("r"),
+    type=AutoDecodedFile("r"),
     metavar="FILE",
     callback=lambda ctx, param, arg: read_flat_file(arg),
 )

tests/test_file_readers.py

@@ -1,5 +1,8 @@
+import click.exceptions
 import pytest
 
+from code42cli.click_ext.types import AutoDecodedFile
+from code42cli.click_ext.types import FileOrString
 from code42cli.errors import Code42CLIError
 from code42cli.file_readers import read_csv
 
@@ -62,3 +65,38 @@ def test_read_csv_when_some_but_not_all_required_headers_present_raises(runner):
         with open("test_csv.csv") as csv:
             with pytest.raises(Code42CLIError):
                 read_csv(file=csv, headers=HEADERS + ["extra_header"])
+
+
+@pytest.mark.parametrize(
+    "encoding", ["utf8", "utf16", "latin_1"],
+)
+def test_read_csv_reads_various_encodings_automatically(runner, encoding):
+    with runner.isolated_filesystem():
+        with open("test.csv", "w", encoding=encoding) as file:
+            file.write("".join(HEADERED_CSV))
+
+        csv = AutoDecodedFile("r").convert("test.csv", None, None)
+        result_list = read_csv(csv, headers=HEADERS)
+
+        assert result_list == [
+            {"header1": "col1_val1", "header2": "col2_val1", "header3": "col3_val1"},
+            {"header1": "col1_val2", "header2": "col2_val2", "header3": "col3_val2"},
+        ]
+
+
+def test_AutoDecodedFile_raises_expected_exception_when_file_not_exists(runner):
+    with pytest.raises(click.exceptions.BadParameter):
+        AutoDecodedFile("r").convert("not_a_file", None, None)
+
+
+@pytest.mark.parametrize(
+    "encoding", ["utf8", "utf16", "latin_1"],
+)
+def test_FileOrString_arg_handles_various_encodings_automatically(runner, encoding):
+    test_data = '{"tést": "dåta"}'
+    with runner.isolated_filesystem():
+        with open("test1.json", "w", encoding=encoding) as file:
+            file.write(test_data)
+
+        result_data = FileOrString().convert("@test1.json", None, None)
+        assert result_data == test_data