1. put the geo validation in GeoSet class constructor, and show which geo_value is invalid in the message

2. move the valid FIPS and MSA codes to test_utils.py, and import them when needed.
3. fix other failed places related to this change

Author: LeonLu2

integrations/client/test_delphi_epidata.py

@@ -12,19 +12,10 @@
 # third party
 import delphi.operations.secrets as secrets
 from delphi.epidata.acquisition.covidcast.covidcast_meta_cache_updater import main as update_covidcast_meta_cache
-from delphi.epidata.acquisition.covidcast.test_utils import CovidcastBase, CovidcastTestRow
+from delphi.epidata.acquisition.covidcast.test_utils import CovidcastBase, CovidcastTestRow, FIPS, MSA
 from delphi.epidata.client.delphi_epidata import Epidata
 from delphi_utils import Nans
 
-
-# TODO replace these real geo_values with fake values, and use patch and mock to mock the return values of 
-# delphi_utils.geomap.GeoMapper().get_geo_values(geo_type) in parse_geo_sets() of _params.py
-
-global fips, msa # add two global lists of valid geo_values of these types to pass the validation in parse_geo_sets() of _params.py
-
-fips = ['04019', '19143', '29063'] # Example list of valid FIPS codes as strings
-msa = ['40660', '44180', '48620'] # Example list of valid MSAs as strings
-
 # py3tester coverage target
 __test_target__ = 'delphi.epidata.client.delphi_epidata'
 # all the Nans we use here are just one value, so this is a shortcut to it:
@@ -59,11 +50,11 @@ def test_covidcast(self):
 
     # insert placeholder data: three issues of one signal, one issue of another
     rows = [
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], issue=2020_02_02 + i, value=i, lag=i)
+      CovidcastTestRow.make_default_row(issue=2020_02_02 + i, value=i, lag=i)
       for i in range(3)
     ]
     row_latest_issue = rows[-1]
-    rows.append(CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], signal="sig2"))
+    rows.append(CovidcastTestRow.make_default_row(signal="sig2"))
     self._insert_rows(rows)
 
     with self.subTest(name='request two signals'):
@@ -227,10 +218,10 @@ def test_geo_value(self):
     # insert placeholder data: three counties, three MSAs
     N = 3
     rows = [
-      CovidcastTestRow.make_default_row(geo_type="fips", geo_value=fips[i], value=i)
+      CovidcastTestRow.make_default_row(geo_type="fips", geo_value=FIPS[i], value=i)
       for i in range(N)
     ] + [
-      CovidcastTestRow.make_default_row(geo_type="msa", geo_value=msa[i], value=i*10)
+      CovidcastTestRow.make_default_row(geo_type="msa", geo_value=MSA[i], value=i*10)
       for i in range(N)
     ]
     self._insert_rows(rows)
@@ -250,26 +241,29 @@ def fetch(geo):
     self.assertEqual(request['message'], 'success')
     self.assertEqual(request['epidata'], counties)
     # test fetch a specific region
-    request = fetch('{}'.format(fips[0]))
+    request = fetch([FIPS[0]])
     self.assertEqual(request['message'], 'success')
     self.assertEqual(request['epidata'], [counties[0]])
     # test fetch a specific yet not existing region
     request = fetch('55555')
-    self.assertEqual(request['message'], 'invalid geo_value for the requested geo_type')
+    self.assertEqual(request['message'], 'Invalid geo_value(s) 55555 for the requested geo_type fips')
     # test fetch a multiple regions
-    request = fetch(['{}'.format(fips[0]), '{}'.format(fips[1])])
+    request = fetch([FIPS[0], FIPS[1]])
     self.assertEqual(request['message'], 'success')
     self.assertEqual(request['epidata'], [counties[0], counties[1]])
     # test fetch a multiple regions in another variant
-    request = fetch(['{}'.format(fips[0]), '{}'.format(fips[2])])
+    request = fetch([FIPS[0], FIPS[2]])
     self.assertEqual(request['message'], 'success')
     self.assertEqual(request['epidata'], [counties[0], counties[2]])
     # test fetch a multiple regions but one is not existing
-    request = fetch(['{}'.format(fips[0]), '55555'])
-    self.assertEqual(request['message'], 'invalid geo_value for the requested geo_type')
+    request = fetch([FIPS[0], '55555'])
+    self.assertEqual(request['message'], 'Invalid geo_value(s) 55555 for the requested geo_type fips')
     # test fetch a multiple regions but specify no region
     request = fetch([])
-    self.assertEqual(request['message'], 'invalid geo_value for the requested geo_type')
+    self.assertEqual(request['message'], 'geo_value is empty for the requested geo_type fips!')
+    # test fetch a region with no results
+    request = fetch([FIPS[3]])
+    self.assertEqual(request['message'], 'no results')
 
   def test_covidcast_meta(self):
     """Test that the covidcast_meta endpoint returns expected data."""
@@ -333,10 +327,10 @@ def test_async_epidata(self):
     # insert placeholder data: three counties, three MSAs
     N = 3
     rows = [
-      CovidcastTestRow.make_default_row(geo_type="fips", geo_value=fips[i-1], value=i)
+      CovidcastTestRow.make_default_row(geo_type="fips", geo_value=FIPS[i-1], value=i)
       for i in range(N)
     ] + [
-      CovidcastTestRow.make_default_row(geo_type="msa", geo_value=msa[i-1], value=i*10)
+      CovidcastTestRow.make_default_row(geo_type="msa", geo_value=MSA[i-1], value=i*10)
       for i in range(N)
     ]
     self._insert_rows(rows)

integrations/server/test_covidcast.py

@@ -3,30 +3,19 @@
 # standard library
 from typing import Callable
 import unittest
-# from unittest.mock import MagicMock
-# from unittest.mock import patch
 
 # third party
 import mysql.connector
 import requests
 
 # first party
 from delphi_utils import Nans
-from delphi.epidata.acquisition.covidcast.test_utils import CovidcastBase, CovidcastTestRow
+from delphi.epidata.acquisition.covidcast.test_utils import CovidcastBase, CovidcastTestRow, FIPS, MSA
 from delphi.epidata.client.delphi_epidata import Epidata
 
 # use the local instance of the Epidata API
 BASE_URL = 'http://delphi_web_epidata/epidata/api.php'
 
-# TODO replace these real geo_values with fake values, and use patch and mock to mock the return values of 
-# delphi_utils.geomap.GeoMapper().get_geo_values(geo_type) in parse_geo_sets() of _params.py
-
-global fips, msa # add two global lists of valid geo_values of these types to pass the validation in parse_geo_sets() of _params.py
-
-fips = ['04019', '19143', '29063'] # Example list of valid FIPS codes as strings
-msa = ['40660', '44180', '48620'] # Example list of valid MSAs as strings
-
-
 class CovidcastTests(CovidcastBase):
   """Tests the `covidcast` endpoint."""
 
@@ -42,52 +31,52 @@ def request_based_on_row(self, row: CovidcastTestRow, **kwargs):
     return response
 
   def _insert_placeholder_set_one(self):
-    row = CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0])
+    row = CovidcastTestRow.make_default_row()
     self._insert_rows([row])
     return row
 
   def _insert_placeholder_set_two(self):
     rows = [
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[i-1], value=i*1., stderr=i*10., sample_size=i*100.)
+      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=MSA[i-1], value=i*1., stderr=i*10., sample_size=i*100.)
       for i in [1, 2, 3]
     ] + [
-      CovidcastTestRow.make_default_row(geo_type='fips', geo_value=fips[i-4], value=i*1., stderr=i*10., sample_size=i*100.)
+      CovidcastTestRow.make_default_row(geo_type='fips', geo_value=FIPS[i-4], value=i*1., stderr=i*10., sample_size=i*100.)
       for i in [4, 5, 6]
     ]
     self._insert_rows(rows)
     return rows
 
   def _insert_placeholder_set_three(self):
     rows = [
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], time_value=2000_01_01+i, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03, lag=2-i)
+      CovidcastTestRow.make_default_row(geo_value=MSA[0], time_value=2000_01_01+i, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03, lag=2-i)
       for i in [1, 2, 3]
     ] + [
       # time value intended to overlap with the time values above, with disjoint geo values
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=str(i)*5, time_value=2000_01_01+i-3, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03, lag=5-i)
+      CovidcastTestRow.make_default_row(geo_value=str(i)*5, time_value=2000_01_01+i-3, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03, lag=5-i)
       for i in [4, 5, 6]
     ]
     self._insert_rows(rows)
     return rows
 
   def _insert_placeholder_set_four(self):
     rows = [
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], source='src1', signal=str(i)*5, value=i*1., stderr=i*10., sample_size=i*100.)
+      CovidcastTestRow.make_default_row(source='src1', signal=str(i)*5, value=i*1., stderr=i*10., sample_size=i*100.)
       for i in [1, 2, 3]
     ] + [
       # signal intended to overlap with the signal above
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], source='src2', signal=str(i-3)*5, value=i*1., stderr=i*10., sample_size=i*100.)
+      CovidcastTestRow.make_default_row(source='src2', signal=str(i-3)*5, value=i*1., stderr=i*10., sample_size=i*100.)
       for i in [4, 5, 6]
     ]
     self._insert_rows(rows)
     return rows
 
   def _insert_placeholder_set_five(self):
     rows = [
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], time_value=2000_01_01, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03+i)
+      CovidcastTestRow.make_default_row(time_value=2000_01_01, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03+i)
       for i in [1, 2, 3]
     ] + [
       # different time_values, same issues
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], time_value=2000_01_01+i-3, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03+i-3)
+      CovidcastTestRow.make_default_row(time_value=2000_01_01+i-3, value=i*1., stderr=i*10., sample_size=i*100., issue=2000_01_03+i-3)
       for i in [4, 5, 6]
     ]
     self._insert_rows(rows)
@@ -317,30 +306,30 @@ def fetch(geo_value):
 
       return response
 
-    self.maxDiff = None
     # test fetch a specific region
-    r = fetch(msa[0])
+    r = fetch(MSA[0])
     self.assertEqual(r['message'], 'success')
     self.assertEqual(r['epidata'], expected[0:1])
     # test fetch a specific yet not existing region
     r = fetch('11111')
-    self.assertEqual(r['message'], 'invalid geo_value for the requested geo_type')
-
+    self.assertEqual(r['message'], 'Invalid geo_value(s) 11111 for the requested geo_type msa')
     # test fetch multiple regions
-    r = fetch('{},{}'.format(msa[0], msa[1]))
+    r = fetch('{},{}'.format(MSA[0], MSA[1]))
     self.assertEqual(r['message'], 'success')
     self.assertEqual(r['epidata'], expected[0:2])
     # test fetch multiple noncontiguous regions
-    r = fetch('{},{}'.format(msa[0], msa[2]))
+    r = fetch('{},{}'.format(MSA[0], MSA[2]))
     self.assertEqual(r['message'], 'success')
     self.assertEqual(r['epidata'], [expected[0], expected[2]])
     # test fetch multiple regions but one is not existing
-    r = fetch('{},11111'.format(msa[0]))
-    self.assertEqual(r['message'], 'invalid geo_value for the requested geo_type')
-
+    r = fetch('{},11111'.format(MSA[0]))
+    self.assertEqual(r['message'], 'Invalid geo_value(s) 11111 for the requested geo_type msa')
     # test fetch empty region
     r = fetch('')
-    self.assertEqual(r['message'], 'invalid geo_value for the requested geo_type')
+    self.assertEqual(r['message'], 'geo_value is empty for the requested geo_type msa!')
+    # test a region that has no results
+    r = fetch(MSA[3])
+    self.assertEqual(r['message'], 'no results')
 
   def test_location_timeline(self):
     """Select a timeline for a particular location."""
@@ -377,7 +366,7 @@ def test_nullable_columns(self):
     """Missing values should be surfaced as null."""
 
     row = CovidcastTestRow.make_default_row(
-      geo_type='msa', geo_value=msa[0], stderr=None, sample_size=None,
+      stderr=None, sample_size=None,
       missing_stderr=Nans.OTHER.value, missing_sample_size=Nans.OTHER.value
     )
     self._insert_rows([row])
@@ -398,7 +387,7 @@ def test_temporal_partitioning(self):
 
     # insert placeholder data
     rows = [
-      CovidcastTestRow.make_default_row(geo_type='msa', geo_value=msa[0], time_type=tt)
+      CovidcastTestRow.make_default_row(time_type=tt)
       for tt in "hour day week month year".split()
     ]
     self._insert_rows(rows)

src/acquisition/covidcast/test_utils.py

@@ -14,6 +14,11 @@
 # all the Nans we use here are just one value, so this is a shortcut to it:
 nmv = Nans.NOT_MISSING.value
 
+# TODO replace these real geo_values with fake values, and use patch and mock to mock the return values of 
+# delphi_utils.geomap.GeoMapper().get_geo_values(geo_type) in parse_geo_sets() of _params.py
+
+FIPS = ['04019', '19143', '29063', '36083'] # Example list of valid FIPS codes as strings
+MSA = ['40660', '44180', '48620', '49420'] # Example list of valid MSA codes as strings
 
 class CovidcastTestRow(CovidcastRow):
     @staticmethod
@@ -22,9 +27,9 @@ def make_default_row(**kwargs) -> "CovidcastTestRow":
             "source": "src",
             "signal": "sig",
             "time_type": "day",
-            "geo_type": "county",
+            "geo_type": "msa",
             "time_value": 2020_02_02,
-            "geo_value": "01234",
+            "geo_value": "40660",       # a valid geo_value for msa
             "value": 10.0,
             "stderr": 10.0,
             "sample_size": 10.0,

src/server/_params.py

@@ -54,6 +54,17 @@ class GeoSet:
     geo_type: str
     geo_values: Union[bool, Sequence[str]]
 
+    def __init__(self, geo_type: str, geo_values: Union[bool, Sequence[str]]):
+        if not isinstance(geo_values, bool):
+            if geo_values == ['']:
+                raise ValidationFailedException(f"geo_value is empty for the requested geo_type {geo_type}!")
+            allowed_values = delphi_utils.geomap.GeoMapper().get_geo_values(geo_type)
+            invalid_values = set(geo_values) - set(allowed_values)
+            if invalid_values:
+                raise ValidationFailedException(f"Invalid geo_value(s) {', '.join(invalid_values)} for the requested geo_type {geo_type}")
+        self.geo_type = geo_type
+        self.geo_values = geo_values
+
     def matches(self, geo_type: str, geo_value: str) -> bool:
         return self.geo_type == geo_type and (self.geo_values is True or (not isinstance(self.geo_values, bool) and geo_value in self.geo_values))
 
@@ -469,9 +480,9 @@ def parse_geo_sets() -> List[GeoSet]:
         if len(geo_values) == 1 and geo_values[0] == "*":
             return [GeoSet(geo_type, True)]
 
-        for geo_value in geo_values:
-            if geo_value not in delphi_utils.geomap.GeoMapper().get_geo_values(geo_type):
-                raise ValidationFailedException("invalid geo_value for the requested geo_type")
+        # for geo_value in geo_values:
+        #     if geo_value not in delphi_utils.geomap.GeoMapper().get_geo_values(geo_type):
+        #         raise ValidationFailedException("invalid geo_value for the requested geo_type")
 
         return [GeoSet(geo_type, geo_values)]
 

tests/acquisition/covidcast/test_covidcast_row.py

@@ -22,9 +22,9 @@ class TestCovidcastRows(unittest.TestCase):
         "source": ["src"] * 10,
         "signal": ["sig_base"] * 5 + ["sig_other"] * 5,
         "time_type": ["day"] * 10,
-        "geo_type": ["county"] * 10,
+        "geo_type": ["msa"] * 10,
         "time_value": [2021_05_01 + i for i in range(5)] * 2,
-        "geo_value": ["01234"] * 10,
+        "geo_value": ["40660"] * 10,
         "value": range(10),
         "stderr": [10.0] * 10,
         "sample_size": [10.0] * 10,