From 0bdd999cdd48508a380b4e2ae2281c245005263c Mon Sep 17 00:00:00 2001
From: Fabio Caccamo <fabio.caccamo@gmail.com>
Date: Thu, 29 Dec 2022 15:38:25 +0100
Subject: [PATCH] Fix `s3_options` option forwarded to `json` decoder. (#204)

---
 benedict/dicts/io/io_util.py    | 24 +++++-----
 tests/github/test_issue_0198.py | 79 +++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 11 deletions(-)
 create mode 100644 tests/github/test_issue_0198.py

diff --git a/benedict/dicts/io/io_util.py b/benedict/dicts/io/io_util.py
index d9c7c44..1ae321f 100644
--- a/benedict/dicts/io/io_util.py
+++ b/benedict/dicts/io/io_util.py
@@ -22,7 +22,7 @@ def decode(s, format, **kwargs):
     options = kwargs.copy()
     if format in ["b64", "base64"]:
         options.setdefault("subformat", "json")
-    content = read_content(s, format, **options)
+    content = read_content(s, format, options)
     data = serializer.decode(content, **options)
     return data
 
@@ -76,30 +76,33 @@ def parse_s3_url(url):
     }
 
 
-def read_content(s, format=None, **options):
+def read_content(s, format=None, options=None):
     # s -> filepath or url or data
-    options.setdefault("format", format)
+    # options.setdefault("format", format)
+    options = options or {}
     s = s.strip()
     if is_data(s):
         return s
     elif is_url(s):
-        return read_content_from_url(s, **options)
+        requests_options = options.pop("requests_options", None) or {}
+        return read_content_from_url(s, requests_options, format)
     elif is_s3(s):
-        return read_content_from_s3(s, **options)
+        s3_options = options.pop("s3_options", None) or {}
+        return read_content_from_s3(s, s3_options, format)
     elif is_filepath(s):
-        return read_content_from_file(s, **options)
+        return read_content_from_file(s, format)
     # one-line data?!
     return s
 
 
-def read_content_from_file(filepath, format=None, **options):
+def read_content_from_file(filepath, format=None):
     binary_format = is_binary_format(format)
     if binary_format:
         return filepath
     return fsutil.read_file(filepath)
 
 
-def read_content_from_s3(url, s3_options, format=None, **options):
+def read_content_from_s3(url, s3_options, format=None):
     s3_url = parse_s3_url(url)
     dirpath = tempfile.gettempdir()
     filename = fsutil.get_filename(s3_url["key"])
@@ -107,12 +110,11 @@ def read_content_from_s3(url, s3_options, format=None, **options):
     s3 = boto3.client("s3", **s3_options)
     s3.download_file(s3_url["bucket"], s3_url["key"], filepath)
     s3.close()
-    content = read_content_from_file(filepath, format, **options)
+    content = read_content_from_file(filepath, format)
     return content
 
 
-def read_content_from_url(url, requests_options=None, format=None, **options):
-    requests_options = requests_options or {}
+def read_content_from_url(url, requests_options, format=None):
     binary_format = is_binary_format(format)
     if binary_format:
         dirpath = tempfile.gettempdir()
diff --git a/tests/github/test_issue_0198.py b/tests/github/test_issue_0198.py
new file mode 100644
index 0000000..04a4a57
--- /dev/null
+++ b/tests/github/test_issue_0198.py
@@ -0,0 +1,79 @@
+import unittest
+
+from decouple import config
+
+from benedict import benedict
+
+
+class github_issue_0198_test_case(unittest.TestCase):
+    """
+    This class describes a github issue 0198 test case.
+    https://github.com/fabiocaccamo/python-benedict/issues/198
+
+    To run this specific test:
+    - Run python -m unittest tests.github.test_issue_0198
+    """
+
+    def test_constructor_with_s3_url_and_s3_options_with_file_json(self):
+        aws_access_key_id = config("AWS_ACCESS_KEY_ID", default=None)
+        aws_secret_access_key = config("AWS_SECRET_ACCESS_KEY", default=None)
+        if not all([aws_access_key_id, aws_secret_access_key]):
+            # don't use s3 on GH CI
+            return
+        s3_options = {
+            "aws_access_key_id": aws_access_key_id,
+            "aws_secret_access_key": aws_secret_access_key,
+        }
+        d = benedict(
+            "s3://python-benedict/valid-content.json",
+            s3_options=s3_options,
+        )
+        expected_dict = {"a": 1, "b": 2, "c": 3, "x": 7, "y": 8, "z": 9}
+        self.assertEqual(d, expected_dict)
+
+    def test_constructor_with_s3_url_and_s3_options_with_file_xlsx(self):
+        aws_access_key_id = config("AWS_ACCESS_KEY_ID", default=None)
+        aws_secret_access_key = config("AWS_SECRET_ACCESS_KEY", default=None)
+        if not all([aws_access_key_id, aws_secret_access_key]):
+            # don't use s3 on GH CI
+            return
+        s3_options = {
+            "aws_access_key_id": aws_access_key_id,
+            "aws_secret_access_key": aws_secret_access_key,
+        }
+        d = benedict(
+            "s3://python-benedict/valid-content.xlsx",
+            s3_options=s3_options,
+        )
+        expected_dict = {
+            "values": [
+                {
+                    "mon": 10,
+                    "tue": 11,
+                    "wed": 12,
+                    "thu": 13,
+                    "fri": 14,
+                    "sat": 15,
+                    "sun": 16,
+                },
+                {
+                    "mon": 20,
+                    "tue": 21,
+                    "wed": 22,
+                    "thu": 23,
+                    "fri": 24,
+                    "sat": 25,
+                    "sun": 26,
+                },
+                {
+                    "mon": 30,
+                    "tue": 31,
+                    "wed": 32,
+                    "thu": 33,
+                    "fri": 34,
+                    "sat": 35,
+                    "sun": 36,
+                },
+            ]
+        }
+        self.assertEqual(d, expected_dict)