From d7a10367304cff6bf9c04b1ef51650884da7f8be Mon Sep 17 00:00:00 2001
From: Thomas NOEL <tnoel@entrouvert.com>
Date: Tue, 31 Mar 2020 14:45:41 +0200
Subject: [PATCH] data_source: handle err in JSON outputs (#41195)

---
 tests/test_datasource.py | 66 +++++++++++++++++++++++++++++++
 tests/utilities.py       |  1 +
 wcs/data_sources.py      | 84 ++++++++++++++++++++++------------------
 3 files changed, 113 insertions(+), 38 deletions(-)

diff --git a/tests/test_datasource.py b/tests/test_datasource.py
index 91fdb1d8..c1efa1aa 100644
--- a/tests/test_datasource.py
+++ b/tests/test_datasource.py
@@ -296,6 +296,10 @@ def test_json_datasource_bad_url(http_requests, caplog):
     assert 'Error loading JSON data source' in caplog.records[-1].message
     assert 'error' in caplog.records[-1].message
 
+    datasource = {'type': 'json', 'value': 'http://remote.example.net/json-list-err1'}
+    assert data_sources.get_items(datasource) == []
+    assert 'Error reading JSON data source output (err 1)' in caplog.records[-1].message
+
 
 def test_json_datasource_bad_url_scheme(caplog):
     datasource = {'type': 'json', 'value': ''}
@@ -496,6 +500,68 @@ def test_named_datasource_json_cache(requests_pub):
         assert urlopen.call_count == 3
 
 
+def test_named_datasource_id_parameter(requests_pub):
+    NamedDataSource.wipe()
+    datasource = NamedDataSource(name='foobar')
+    datasource.data_source = {'type': 'json', 'value': 'http://whatever/'}
+    datasource.id_parameter = 'id'
+    datasource.store()
+
+    with mock.patch('wcs.qommon.misc.urlopen') as urlopen:
+        value = [{'id': '1', 'text': 'foo'}]
+        urlopen.side_effect = lambda *args: StringIO(json.dumps({'data': value}))
+        assert datasource.get_structured_value('1') == value[0]
+        assert urlopen.call_count == 1
+        assert urlopen.call_args[0][0] == 'http://whatever/?id=1'
+
+        # try again, get from request.datasources_cache
+        assert datasource.get_structured_value('1') == value[0]
+        assert urlopen.call_count == 1  # no new call
+
+    get_request().datasources_cache = {}
+    with mock.patch('wcs.qommon.misc.urlopen') as urlopen:
+        value = [{'id': '1', 'text': 'bar'}, {'id': '2', 'text': 'foo'}]
+        urlopen.side_effect = lambda *args: StringIO(json.dumps({'data': value}))
+        assert datasource.get_structured_value('1') == value[0]
+        assert urlopen.call_count == 1
+
+    get_request().datasources_cache = {}
+    with mock.patch('wcs.qommon.misc.urlopen') as urlopen:
+        urlopen.side_effect = lambda *args: StringIO(json.dumps({'data': []}))  # empty list
+        assert datasource.get_structured_value('1') is None
+        assert urlopen.call_count == 1
+
+    get_request().datasources_cache = {}
+    with mock.patch('wcs.qommon.misc.urlopen') as urlopen:
+        value = [{'id': '1', 'text': 'foo'}]
+        urlopen.side_effect = lambda *args: StringIO(json.dumps({'data': value, 'err': 0}))
+        assert datasource.get_structured_value('1') == value[0]
+        assert urlopen.call_count == 1
+
+    get_request().datasources_cache = {}
+    with mock.patch('wcs.qommon.misc.urlopen') as urlopen:
+        value = [{'id': '1', 'text': 'foo'}]
+        urlopen.side_effect = lambda *args: StringIO(json.dumps({'data': value, 'err': 1}))
+        assert datasource.get_structured_value('1') is None
+        assert urlopen.call_count == 1
+        # no cache for errors
+        assert datasource.get_structured_value('1') is None
+        assert urlopen.call_count == 2  # called again
+
+    get_request().datasources_cache = {}
+    with mock.patch('wcs.qommon.misc.urlopen') as urlopen:
+        value = {'id': '1', 'text': 'foo'}  # not a list
+        urlopen.side_effect = lambda *args: StringIO(json.dumps({'data': value}))
+        assert datasource.get_structured_value('1') is None
+        assert urlopen.call_count == 1
+
+    get_request().datasources_cache = {}
+    with mock.patch('wcs.qommon.misc.urlopen') as urlopen:
+        urlopen.side_effect = lambda *args: StringIO('not json')
+        assert datasource.get_structured_value('1') is None
+        assert urlopen.call_count == 1
+
+
 def test_named_datasource_in_formdef():
     from wcs.formdef import FormDef
     datasource = NamedDataSource(name='foobar')
diff --git a/tests/utilities.py b/tests/utilities.py
index 87a3ee53..1616dfdc 100644
--- a/tests/utilities.py
+++ b/tests/utilities.py
@@ -329,6 +329,7 @@ class HttpRequestsMocking(object):
             'http://remote.example.net/json-list': (200, '{"data": [{"id": "a", "text": "b"}]}', None),
             'http://remote.example.net/json-err0': (200, '{"data": "foo", "err": 0}', None),
             'http://remote.example.net/json-err1': (200, '{"data": "", "err": 1}', None),
+            'http://remote.example.net/json-list-err1': (200, '{"data": [{"id": "a", "text": "b"}], "err": 1}', None),
             'http://remote.example.net/json-errstr': (200, '{"data": "", "err": "bug"}', None),
             'http://remote.example.net/json-errheader0': (200, '{"foo": "bar"}',
                                               {'x-error-code': '0'}),
diff --git a/wcs/data_sources.py b/wcs/data_sources.py
index bcf75432..1aca589e 100644
--- a/wcs/data_sources.py
+++ b/wcs/data_sources.py
@@ -116,6 +116,33 @@ def get_items(data_source, include_disabled=False, mode=None):
     return tupled_items
 
 
+def request_json_items(url):
+    url = sign_url_auto_orig(url)
+    try:
+        entries = misc.json_loads(misc.urlopen(url).read())
+        if not isinstance(entries, dict):
+            raise ValueError('not a json dict')
+        if entries.get('err') not in (None, 0, "0"):
+            raise ValueError('err %s' % entries['err'])
+        if not isinstance(entries.get('data'), list):
+            raise ValueError('not a json dict with a data list attribute')
+    except misc.ConnectionError as e:
+        get_logger().warn('Error loading JSON data source (%s)' % str(e))
+        return None
+    except (ValueError, TypeError) as e:
+        get_logger().warn('Error reading JSON data source output (%s)' % str(e))
+        return None
+    items = []
+    for item in entries.get('data'):
+        # skip malformed items
+        if item.get('id') is None or item.get('id') == '':
+            continue
+        if 'text' not in item:
+            item['text'] = item['id']
+        items.append(item)
+    return items
+
+
 def get_structured_items(data_source, mode=None):
     cache_duration = 0
 
@@ -195,33 +222,14 @@ def get_structured_items(data_source, mode=None):
             if items is not None:
                 return items
 
-        unsigned_url = url
-        url = sign_url_auto_orig(url)
-        try:
-            entries = misc.json_loads(misc.urlopen(url).read())
-            if type(entries) is not dict:
-                raise ValueError('not a json dict')
-            if type(entries.get('data')) is not list:
-                raise ValueError('not a json dict with a data list attribute')
-            items = []
-            for item in entries.get('data'):
-                # skip malformed items
-                if item.get('id') is None or item.get('id') == '':
-                    continue
-                if 'text' not in item:
-                    item['text'] = item['id']
-                items.append(item)
-            if hasattr(request, 'datasources_cache'):
-                request.datasources_cache[unsigned_url] = items
-
-            if cache_duration:
-                cache.set(cache_key, items, cache_duration)
-
-            return items
-        except misc.ConnectionError as e:
-            get_logger().warn('Error loading JSON data source (%s)' % str(e))
-        except ValueError as e:
-            get_logger().warn('Error reading JSON data source output (%s)' % str(e))
+        items = request_json_items(url)
+        if items is None:
+            return []
+        if hasattr(request, 'datasources_cache'):
+            request.datasources_cache[url] = items
+        if cache_duration:
+            cache.set(cache_key, items, cache_duration)
+        return items
     return []
 
 
@@ -356,7 +364,7 @@ class NamedDataSource(XmlStorableObject):
                     get_session().get_data_source_query_url_token(self.get_json_query_url()))
         return None
 
-    def load_json(self, param_name, param_value):
+    def get_value_by_id(self, param_name, param_value):
         url = self.data_source.get('value').strip()
         if Template.is_template_string(url):
             vars = get_publisher().substitutions.get_context_variables(mode='lazy')
@@ -370,14 +378,17 @@ class NamedDataSource(XmlStorableObject):
 
         request = get_request()
         if hasattr(request, 'datasources_cache') and url in request.datasources_cache:
-            return request.datasources_cache[url]
+            items = request.datasources_cache[url]
+            if not items:  # cache may contains empty list from get_structured_items
+                return None
+            return items[0]
 
-        unsigned_url = url
-        url = sign_url_auto_orig(url)
-        resp = misc.urlopen(url).read()
+        items = request_json_items(url)
+        if not items:  # None or empty list are not valid
+            return None
         if hasattr(request, 'datasources_cache'):
-            request.datasources_cache[unsigned_url] = resp
-        return resp
+            request.datasources_cache[url] = items
+        return items[0]
 
     def get_display_value(self, option_id):
         value = self.get_structured_value(option_id)
@@ -388,10 +399,7 @@ class NamedDataSource(XmlStorableObject):
     def get_structured_value(self, option_id):
         value = None
         if self.type == 'json' and self.id_parameter:
-            resp = self.load_json(self.id_parameter, option_id)
-            response = misc.json_loads(resp)
-            if response['data']:
-                value = response['data'][0]
+            value = self.get_value_by_id(self.id_parameter, option_id)
         else:
             structured_items = get_structured_items(self.data_source, mode='lazy')
             for item in structured_items:
-- 
2.25.1

