From 2a134fb5f29807f1eb54ae698f19ba98b417e953 Mon Sep 17 00:00:00 2001 From: Terence Tuhinanshu Date: Fri, 19 Jan 2024 15:34:05 -0500 Subject: [PATCH 1/6] Add test for magic key case This is currently failing, to replicate the bug reported in #67. Will add a fix for this next. --- omgeo/tests/tests.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/omgeo/tests/tests.py b/omgeo/tests/tests.py index bd133a0..ffa4dae 100755 --- a/omgeo/tests/tests.py +++ b/omgeo/tests/tests.py @@ -208,6 +208,16 @@ def test_geocode_esri_wgs_340_12th_bounded(self): self.assertEqual('340 N 12th' in candidates[0].match_addr, True, '"340 N 12th" not found in match_addr. Got "%s"' % candidates[0].match_addr) + def test_geocode_esri_wgs_magicKey(self): + """Check that geocoding New York, USA with a magicKey returns one result.""" + esri = self.g_esri_wgs._sources[0] + suggestions = esri._get_json_obj( + f'{esri._endpoint}/suggest', + {'f': 'json', 'text': 'New York, USA'})['suggestions'] + pq = PlaceQuery(suggestions[0]['text'], key=suggestions[0]['magicKey']) + candidates = self.g_esri_wgs.get_candidates(pq) + self.assertOneCandidate(candidates) + def test_geocode_esri_wgs_zip_plus_4(self): """Check that geocoding 19127-1112 returns one result.""" candidates = self.g_esri_wgs_postal_ok.get_candidates(self.pq['zip_plus_4_in_postal_plus_country']) From ecf614a54be26990b34c1bf8b5ad0c1a6497f46e Mon Sep 17 00:00:00 2001 From: Terence Tuhinanshu Date: Fri, 19 Jan 2024 15:35:46 -0500 Subject: [PATCH 2/6] Tolerate missing Loc_name, warn if on missing keys Previously we assumed that Loc_name would always be provided, given that we ask for it in the requested field list. However, when using the magicKey, the Loc_name is not provided, causing geocoding to fail. By making it optional, we allow those geocoding requests to succeed. Also, add a WARNING to the log when there's a missing key, instead of just silently passing, for better observability. --- omgeo/services/esri.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/omgeo/services/esri.py b/omgeo/services/esri.py index 437bd9d..f6d516c 100644 --- a/omgeo/services/esri.py +++ b/omgeo/services/esri.py @@ -195,7 +195,7 @@ def _geocode(self, pq): c = Candidate() attributes = location['attributes'] c.match_addr = attributes['Match_addr'] - c.locator = attributes['Loc_name'] + c.locator = attributes.get('Loc_name', '') c.locator_type = attributes['Addr_type'] c.score = attributes['Score'] c.x = attributes['DisplayX'] # represents the actual location of the address. @@ -210,7 +210,8 @@ def _geocode(self, pq): setattr(c, out_key, attributes.get(in_key, '')) setattr(c, 'match_streetaddr', self._street_addr_from_response(attributes)) returned_candidates.append(c) - except KeyError: + except KeyError as e: + logger.warning('Missing key: ' + e) pass return returned_candidates From 09506e26ad668856670a53955e31349ec88cb07e Mon Sep 17 00:00:00 2001 From: Terence Tuhinanshu Date: Fri, 19 Jan 2024 15:37:29 -0500 Subject: [PATCH 3/6] Add Locality to default filter This new value is used for many responses now, need to include it for the tests to pass. --- omgeo/services/esri.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/omgeo/services/esri.py b/omgeo/services/esri.py index f6d516c..99f9e99 100644 --- a/omgeo/services/esri.py +++ b/omgeo/services/esri.py @@ -55,6 +55,7 @@ class EsriWGS(GeocodeService): DEFAULT_POSTPROCESSORS = [ AttrFilter(['PointAddress', 'StreetAddress', + 'Locality', # 'PostalExt', # 'Postal' ], @@ -62,6 +63,7 @@ class EsriWGS(GeocodeService): # AttrExclude(['USA_Postal'], 'locator'), #accept postal from everywhere but US (need PostalExt) AttrSorter(['PointAddress', 'StreetAddress', + 'Locality', # 'PostalExt', # 'Postal' ], From 99f696341fd8023214b6ce617bdedcaab2201600 Mon Sep 17 00:00:00 2001 From: Terence Tuhinanshu Date: Mon, 22 Jan 2024 15:26:26 -0500 Subject: [PATCH 4/6] Disable tests that depend on expired tokens --- .github/workflows/continuous_integration.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index ceadde8..5cebaa9 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -42,11 +42,11 @@ jobs: - name: Run tests run: python setup.py test - env: - BING_MAPS_API_KEY: ${{ secrets.BING_MAPS_API_KEY }} - ESRI_CLIENT_ID: ${{ secrets.ESRI_CLIENT_ID }} - ESRI_CLIENT_SECRET: ${{ secrets.ESRI_CLIENT_SECRET }} - GOOGLE_API_KEY: ${{ secrets.GOOGLE_API_KEY }} - MAPQUEST_API_KEY: ${{ secrets.MAPQUEST_API_KEY }} - PELIAS_API_KEY: ${{ secrets.PELIAS_API_KEY }} + # env: + # BING_MAPS_API_KEY: ${{ secrets.BING_MAPS_API_KEY }} + # ESRI_CLIENT_ID: ${{ secrets.ESRI_CLIENT_ID }} + # ESRI_CLIENT_SECRET: ${{ secrets.ESRI_CLIENT_SECRET }} + # GOOGLE_API_KEY: ${{ secrets.GOOGLE_API_KEY }} + # MAPQUEST_API_KEY: ${{ secrets.MAPQUEST_API_KEY }} + # PELIAS_API_KEY: ${{ secrets.PELIAS_API_KEY }} From de75706562c1c63082a31119a45f7a742c29ce7f Mon Sep 17 00:00:00 2001 From: Terence Tuhinanshu Date: Mon, 22 Jan 2024 15:41:09 -0500 Subject: [PATCH 5/6] Address failing tests Disable Google test if credentials are missing. Not sure what was wrong with the Philadelphia address for Census purposes, but switching to an Alexandria address fixes that. --- omgeo/tests/tests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/omgeo/tests/tests.py b/omgeo/tests/tests.py index ffa4dae..5c94861 100755 --- a/omgeo/tests/tests.py +++ b/omgeo/tests/tests.py @@ -239,6 +239,7 @@ def test_esri_short_region(self): candidate = self.g_esri_wgs.get_candidates(self.pq["azavea"])[0] self.assertEqual(candidate.match_region, "PA") + @unittest.skipIf(GOOGLE_API_KEY is None, GOOGLE_KEY_REQUIRED_MSG) def test_google_short_region(self): """Ensure that Google uses region abbreviations""" candidate = self.g_google.get_candidates(self.pq["azavea"])[0] @@ -282,8 +283,8 @@ def test_geocode_nom(self): self.assertEqual(len(candidates) > 0, True, 'No candidates returned.') def test_geocode_census(self): - """Test Azavea's address using US Census geocoder.""" - candidates = self.g_census.get_candidates(PlaceQuery('1200 Callowhill St, Philadelphia, PA')) + """Test Element 84's address using US Census geocoder.""" + candidates = self.g_census.get_candidates(PlaceQuery('210 N. Lee Street, Alexandria, VA')) self.assertEqual(len(candidates) > 0, True, 'No candidates returned.') def test_EsriWGS_address_components(self): From 10865a5dff6359a64ec0a8471130ff43faa137a5 Mon Sep 17 00:00:00 2001 From: Terence Tuhinanshu Date: Mon, 22 Jan 2024 16:57:44 -0500 Subject: [PATCH 6/6] Update CHANGES.rst for 6.2.0 --- CHANGES.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 71f67db..9b01fe6 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -228,3 +228,10 @@ v6.1.0, 2021-07-20 * Populate match_region using RegionAbbr rather than Region from EsriWGS. For example, when using the EsriWGS geocoder, expect 'PA' rather than 'Pennsylvania' in match_region. + +v6.2.0, 2024-01-22 +------------------ + * Fix /findAddressCandidates calls for EsriWGS when using magicKey. They + previously would provide an internal Loc_name field in the response which + is no longer supplied, which broke the API. Making that field optional in + our code fixes this.