From ca6299255830ad0f86ec71d74547671a179fbb73 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Wed, 4 Sep 2024 12:45:41 -0400 Subject: [PATCH 1/6] add `CHECK_NAME_DUPLICATES` feature --- env.example.yml | 1 + handler.ts | 9 +++++++-- serverless.yml | 1 + utils.ts | 12 +++++++----- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/env.example.yml b/env.example.yml index 39e5b48..064e0c8 100644 --- a/env.example.yml +++ b/env.example.yml @@ -5,3 +5,4 @@ GEOCODERS: BACKUP_GEOCODERS: COORDINATE_COMPARISON_PRECISION_DIGITS: defaults to 4 (~10m). What precision to use when comparing if two locations are the same +CHECK_NAME_DUPLICATES: defaults to true. If disabled, name-based duplicate checking will be disabled. Useful if your GTFS has common words in its stop names \ No newline at end of file diff --git a/handler.ts b/handler.ts index 100d1b3..464611b 100644 --- a/handler.ts +++ b/handler.ts @@ -24,7 +24,7 @@ import { // This plugin must be imported via cjs to ensure its existence (typescript recommendation) const BugsnagPluginAwsLambda = require('@bugsnag/plugin-aws-lambda') -const { BACKUP_GEOCODERS, BUGSNAG_NOTIFIER_KEY, GEOCODERS } = process.env +const { BACKUP_GEOCODERS, BUGSNAG_NOTIFIER_KEY, GEOCODERS, CHECK_NAME_DUPLICATES } = process.env const POIS = require('./pois.json') if (!GEOCODERS) { @@ -128,7 +128,12 @@ export const makeGeocoderRequests = async ( >( (prev, cur, idx) => { if (idx === 0) return cur - return mergeResponses({ customResponse: cur, primaryResponse: prev }) + return mergeResponses( + { customResponse: cur, primaryResponse: prev }, + // Default to false + CHECK_NAME_DUPLICATES === "false" ? false : true, + convertQSPToGeocoderArgs(event.queryStringParameters)?.focusPoint + ); }, // TODO: clean this reducer up. See https://github.com/ibi-group/pelias-stitch/pull/28#discussion_r1547582739 { features: [], type: 'FeatureCollection' } diff --git a/serverless.yml b/serverless.yml index 08350b4..da11c26 100644 --- a/serverless.yml +++ b/serverless.yml @@ -13,6 +13,7 @@ provider: BACKUP_GEOCODERS: ${self:custom.secrets.BACKUP_GEOCODERS} BUGSNAG_NOTIFIER_KEY: ${self:custom.secrets.BUGSNAG_NOTIFIER_KEY} COORDINATE_COMPARISON_PRECISION_DIGITS: ${self:custom.secrets.COORDINATE_COMPARISON_PRECISION_DIGITS, 4} + CHECK_NAME_DUPLICATES: ${self:custom.secrets.CHECK_NAME_DUPLICATES, true} package: patterns: - pois.json diff --git a/utils.ts b/utils.ts index d0692bf..99257b2 100644 --- a/utils.ts +++ b/utils.ts @@ -131,15 +131,16 @@ export const arePointsRoughlyEqual = ( */ const filterOutDuplicateStops = ( feature: Feature, - customFeatures: Feature[] + customFeatures: Feature[], + checkNameDuplicates: boolean ): boolean => { // If the names are the same, or if the feature is too far away, we can't consider the feature if ( customFeatures.find( (otherFeature: Feature) => - (feature?.properties?.name || '') + (checkNameDuplicates && (feature?.properties?.name || '') .toLowerCase() - .includes((otherFeature?.properties?.name || '').toLowerCase()) || + .includes((otherFeature?.properties?.name || '').toLowerCase())) || // Any feature this far away is likely not worth being considered feature?.properties?.distance > 7500 ) @@ -199,14 +200,15 @@ export const mergeResponses = ( customResponse: FeatureCollection primaryResponse: FeatureCollection }, - focusPoint?: LonLatOutput + checkNameDuplicates: boolean, + focusPoint?: LonLatOutput, ): FeatureCollection => { // Openstreetmap can sometimes include bus stop info with less // correct information than the GTFS feed. // Remove anything from the geocode.earth response that's within 10 meters of a custom result responses.primaryResponse.features = responses?.primaryResponse?.features?.filter((feature: Feature) => - filterOutDuplicateStops(feature, responses.customResponse.features) + filterOutDuplicateStops(feature, responses.customResponse.features, checkNameDuplicates) ) || [] // If a focus point is specified, sort custom features by distance to the focus point From e1979f83e6eb46f878ee339c8bae44a735799cf8 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Wed, 4 Sep 2024 12:50:02 -0400 Subject: [PATCH 2/6] chore: lint --- handler.ts | 11 ++++++++--- utils.ts | 15 ++++++++++----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/handler.ts b/handler.ts index 464611b..1b8b956 100644 --- a/handler.ts +++ b/handler.ts @@ -24,7 +24,12 @@ import { // This plugin must be imported via cjs to ensure its existence (typescript recommendation) const BugsnagPluginAwsLambda = require('@bugsnag/plugin-aws-lambda') -const { BACKUP_GEOCODERS, BUGSNAG_NOTIFIER_KEY, GEOCODERS, CHECK_NAME_DUPLICATES } = process.env +const { + BACKUP_GEOCODERS, + BUGSNAG_NOTIFIER_KEY, + CHECK_NAME_DUPLICATES, + GEOCODERS +} = process.env const POIS = require('./pois.json') if (!GEOCODERS) { @@ -131,9 +136,9 @@ export const makeGeocoderRequests = async ( return mergeResponses( { customResponse: cur, primaryResponse: prev }, // Default to false - CHECK_NAME_DUPLICATES === "false" ? false : true, + CHECK_NAME_DUPLICATES !== 'false', convertQSPToGeocoderArgs(event.queryStringParameters)?.focusPoint - ); + ) }, // TODO: clean this reducer up. See https://github.com/ibi-group/pelias-stitch/pull/28#discussion_r1547582739 { features: [], type: 'FeatureCollection' } diff --git a/utils.ts b/utils.ts index 99257b2..7569140 100644 --- a/utils.ts +++ b/utils.ts @@ -138,9 +138,10 @@ const filterOutDuplicateStops = ( if ( customFeatures.find( (otherFeature: Feature) => - (checkNameDuplicates && (feature?.properties?.name || '') - .toLowerCase() - .includes((otherFeature?.properties?.name || '').toLowerCase())) || + (checkNameDuplicates && + (feature?.properties?.name || '') + .toLowerCase() + .includes((otherFeature?.properties?.name || '').toLowerCase())) || // Any feature this far away is likely not worth being considered feature?.properties?.distance > 7500 ) @@ -201,14 +202,18 @@ export const mergeResponses = ( primaryResponse: FeatureCollection }, checkNameDuplicates: boolean, - focusPoint?: LonLatOutput, + focusPoint?: LonLatOutput ): FeatureCollection => { // Openstreetmap can sometimes include bus stop info with less // correct information than the GTFS feed. // Remove anything from the geocode.earth response that's within 10 meters of a custom result responses.primaryResponse.features = responses?.primaryResponse?.features?.filter((feature: Feature) => - filterOutDuplicateStops(feature, responses.customResponse.features, checkNameDuplicates) + filterOutDuplicateStops( + feature, + responses.customResponse.features, + checkNameDuplicates + ) ) || [] // If a focus point is specified, sort custom features by distance to the focus point From 97ddf3a0743c2c824eab077b7b67046939710514 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Wed, 4 Sep 2024 12:55:04 -0400 Subject: [PATCH 3/6] test: correct tests --- __tests__/util.test.ts | 2 ++ utils.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/__tests__/util.test.ts b/__tests__/util.test.ts index 9ccb68d..fff0d25 100644 --- a/__tests__/util.test.ts +++ b/__tests__/util.test.ts @@ -112,6 +112,7 @@ describe('response merging', () => { customResponse: CUSTOM_RESPONSE, primaryResponse: GEOCODE_EARTH_RESPONSE }, + true, { lat: 47.880281, lon: -122.238459 } ) const mergedFocusedOnSteinerStreet = mergeResponses( @@ -119,6 +120,7 @@ describe('response merging', () => { customResponse: CUSTOM_RESPONSE, primaryResponse: GEOCODE_EARTH_RESPONSE }, + true, { lat: 37.793899, lon: -122.43634 } ) expect(mergedFocusedOnBusStop).not.toEqual(mergedFocusedOnSteinerStreet) diff --git a/utils.ts b/utils.ts index 7569140..f3cbfa2 100644 --- a/utils.ts +++ b/utils.ts @@ -201,7 +201,7 @@ export const mergeResponses = ( customResponse: FeatureCollection primaryResponse: FeatureCollection }, - checkNameDuplicates: boolean, + checkNameDuplicates: boolean = true, focusPoint?: LonLatOutput ): FeatureCollection => { // Openstreetmap can sometimes include bus stop info with less From 9fe57a017101d7dd7fa70b1bba9a3c580bb88199 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Wed, 4 Sep 2024 12:58:07 -0400 Subject: [PATCH 4/6] chore: lint --- utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils.ts b/utils.ts index f3cbfa2..f6db7d3 100644 --- a/utils.ts +++ b/utils.ts @@ -201,7 +201,7 @@ export const mergeResponses = ( customResponse: FeatureCollection primaryResponse: FeatureCollection }, - checkNameDuplicates: boolean = true, + checkNameDuplicates = true, focusPoint?: LonLatOutput ): FeatureCollection => { // Openstreetmap can sometimes include bus stop info with less From 8274d31f913fb340678122f13a6ae7c5f7f9e0e9 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Wed, 4 Sep 2024 13:08:43 -0400 Subject: [PATCH 5/6] temporarily disable focus point sorting --- handler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/handler.ts b/handler.ts index 1b8b956..3b3e56d 100644 --- a/handler.ts +++ b/handler.ts @@ -136,8 +136,8 @@ export const makeGeocoderRequests = async ( return mergeResponses( { customResponse: cur, primaryResponse: prev }, // Default to false - CHECK_NAME_DUPLICATES !== 'false', - convertQSPToGeocoderArgs(event.queryStringParameters)?.focusPoint + CHECK_NAME_DUPLICATES !== 'false' + // convertQSPToGeocoderArgs(event.queryStringParameters)?.focusPoint ) }, // TODO: clean this reducer up. See https://github.com/ibi-group/pelias-stitch/pull/28#discussion_r1547582739 From aa972b2767620f3e5009e488764bf818d20bcabf Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Tue, 10 Sep 2024 11:21:54 -0400 Subject: [PATCH 6/6] refactor: address pr feedback --- handler.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/handler.ts b/handler.ts index 3b3e56d..02571f7 100644 --- a/handler.ts +++ b/handler.ts @@ -135,9 +135,10 @@ export const makeGeocoderRequests = async ( if (idx === 0) return cur return mergeResponses( { customResponse: cur, primaryResponse: prev }, - // Default to false + // Default to true CHECK_NAME_DUPLICATES !== 'false' - // convertQSPToGeocoderArgs(event.queryStringParameters)?.focusPoint + // TODO: use focus point here to pre-sort results? It's possible to grab + // the focus point by calling convertQSPToGeocoderArgs on event.queryStringParameters ) }, // TODO: clean this reducer up. See https://github.com/ibi-group/pelias-stitch/pull/28#discussion_r1547582739