From fe7cdf23cad804e7266102c2db9a3fffea0de872 Mon Sep 17 00:00:00 2001
From: Andy Lu <andy@stitchdata.com>
Date: Fri, 18 Mar 2022 11:27:27 -0400
Subject: [PATCH] Conversion window validation (#33)

* Add conversion_window validation

* Consolidate conversion window tests

* Enable invalid conversion window integer test

Co-authored-by: Bryant Gray <bgray@talend.com>
---
 tap_google_ads/streams.py                     | 16 ++++++-
 ...st_google_ads_conversion_window_invalid.py |  2 -
 tests/unittests/test_conversion_window.py     | 44 +++++++++++++++++++
 tests/unittests/test_utils.py                 |  1 +
 4 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/tap_google_ads/streams.py b/tap_google_ads/streams.py
index 7cf2971..c7ab677 100644
--- a/tap_google_ads/streams.py
+++ b/tap_google_ads/streams.py
@@ -23,6 +23,20 @@
 DEFAULT_CONVERSION_WINDOW = 30
 
 
+def get_conversion_window(config):
+    """Fetch the conversion window from the config and error on invalid values"""
+    conversion_window = config.get("conversion_window") or DEFAULT_CONVERSION_WINDOW
+
+    try:
+        conversion_window = int(conversion_window)
+    except (ValueError, TypeError) as err:
+        raise RuntimeError("Conversion Window must be an int or string") from err
+
+    if conversion_window in set(range(1,31)) or conversion_window in {60, 90}:
+        return conversion_window
+
+    raise RuntimeError("Conversion Window must be between 1 - 30 inclusive, 60, or 90")
+
 def create_nested_resource_schema(resource_schema, fields):
     new_schema = {
         "type": ["null", "object"],
@@ -440,7 +454,7 @@ def sync(self, sdk_client, customer, stream, config, state):
         singer.write_state(state)
 
         conversion_window = timedelta(
-            days=int(config.get("conversion_window") or DEFAULT_CONVERSION_WINDOW)
+            days=get_conversion_window(config)
         )
         conversion_window_date = utils.now().replace(hour=0, minute=0, second=0, microsecond=0) - conversion_window
 
diff --git a/tests/test_google_ads_conversion_window_invalid.py b/tests/test_google_ads_conversion_window_invalid.py
index 32928e7..8d4975f 100644
--- a/tests/test_google_ads_conversion_window_invalid.py
+++ b/tests/test_google_ads_conversion_window_invalid.py
@@ -117,8 +117,6 @@ class ConversionWindowTestZeroInteger(ConversionWindowInvalidTest):
 
     conversion_window = 0
 
-    @unittest.skip("https://jira.talendforge.org/browse/TDL-18168"
-                   "[tap-google-ads] Invalid conversion_window values can be set when running tap directly")
     def test_run(self):
         self.run_test()
 
diff --git a/tests/unittests/test_conversion_window.py b/tests/unittests/test_conversion_window.py
index 4e5dac4..2e732af 100644
--- a/tests/unittests/test_conversion_window.py
+++ b/tests/unittests/test_conversion_window.py
@@ -4,6 +4,7 @@
 from unittest.mock import MagicMock
 from unittest.mock import Mock
 from unittest.mock import patch
+from tap_google_ads.streams import get_conversion_window
 from tap_google_ads.streams import ReportStream
 from tap_google_ads.streams import make_request
 
@@ -228,5 +229,48 @@ def execute(self, conversion_window, fake_make_request):
         self.assertEqual(len(all_queries_requested), 1)
 
 
+class TestGetConversionWindow(unittest.TestCase):
+    def test_int_conversion_date_in_allowable_range(self):
+        actual = get_conversion_window({"conversion_window": 12})
+        expected = 12
+        self.assertEqual(expected, actual)
+
+    def test_str_conversion_date_in_allowable_range(self):
+        actual = get_conversion_window({"conversion_window": "12"})
+        expected = 12
+        self.assertEqual(expected, actual)
+
+    def test_conversion_date_outside_allowable_range(self):
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": 42})
+
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": "42"})
+
+    def test_non_int_or_str_conversion_date(self):
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": {"12": 12}})
+
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": [12]})
+
+    def test_empty_data_types_conversion_date_returns_default(self):
+        expected = 30
+
+        actual = get_conversion_window({"conversion_window": ""})
+        self.assertEqual(expected, actual)
+
+        actual = get_conversion_window({"conversion_window": {}})
+        self.assertEqual(expected, actual)
+
+        actual = get_conversion_window({"conversion_window": []})
+        self.assertEqual(expected, actual)
+
+    def test_None_conversion_date_returns_default(self):
+        actual = get_conversion_window({"conversion_window": None})
+        expected = 30
+        self.assertEqual(expected, actual)
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/unittests/test_utils.py b/tests/unittests/test_utils.py
index 75e2842..6571e6d 100644
--- a/tests/unittests/test_utils.py
+++ b/tests/unittests/test_utils.py
@@ -354,5 +354,6 @@ def test_shuffle_last_customer(self):
         ]
         self.assertListEqual(expected, actual)
 
+
 if __name__ == '__main__':
     unittest.main()