diff --git a/__tests__/test-utils/fixtures/geocoding/pelias/autocomplete-response.json b/__tests__/test-utils/fixtures/geocoding/pelias/autocomplete-response.json index 05a5ef965..ea9758c84 100644 --- a/__tests__/test-utils/fixtures/geocoding/pelias/autocomplete-response.json +++ b/__tests__/test-utils/fixtures/geocoding/pelias/autocomplete-response.json @@ -11,12 +11,6 @@ "Ends" ], "size": 10, - "sources": [ - "geonames", - "openaddresses", - "openstreetmap", - "whosonfirst" - ], "private": false, "focus.point.lat": 45.52, "focus.point.lon": -122.67, diff --git a/__tests__/util/__snapshots__/geocoder.js.snap b/__tests__/util/__snapshots__/geocoder.js.snap index c0fd6bcae..0ec26af09 100644 --- a/__tests__/util/__snapshots__/geocoder.js.snap +++ b/__tests__/util/__snapshots__/geocoder.js.snap @@ -195,12 +195,6 @@ Object { "parser": "addressit", "private": false, "size": 10, - "sources": Array [ - "geonames", - "openaddresses", - "openstreetmap", - "whosonfirst", - ], "text": "Mill Ends", "tokens": Array [ "Mill", @@ -215,7 +209,6 @@ Object { }, "isomorphicMapzenSearchQuery": Object { "api_key": "dummy-mapzen-key", - "sources": "gn,oa,osm,wof", "text": "Mill Ends", }, "type": "FeatureCollection", diff --git a/__tests__/util/geocoder.js b/__tests__/util/geocoder.js index c93f5938d..ada6e8601 100644 --- a/__tests__/util/geocoder.js +++ b/__tests__/util/geocoder.js @@ -1,6 +1,6 @@ import nock from 'nock' -import getGeocoder from '../../lib/util/geocoder' +import getGeocoder, { PeliasGeocoder } from '../../lib/util/geocoder' function mockResponsePath (geocoder, file) { return `__tests__/test-utils/fixtures/geocoding/${geocoder}/${file}` @@ -118,6 +118,71 @@ describe('geocoder', () => { const result = await getGeocoder(geocoder).getLocationFromGeocodedFeature(mockFeature) expect(result).toMatchSnapshot() }) + + // geocoder-specific tests + if (geocoderType === 'PELIAS') { + const mockSources = 'gn,oa,osm,wof' + + // sources should not be sent unless they are explicitly defined in the + // query. See https://github.com/ibi-group/trimet-mod-otp/issues/239 + it('should not send sources in autocomplete by default', () => { + // create mock API to check query + const mockPeliasAPI = { + autocomplete: query => { + expect(query.sources).not.toBe(expect.anything()) + return Promise.resolve() + } + } + const pelias = new PeliasGeocoder(mockPeliasAPI, geocoder) + pelias.autocomplete({ text: 'Mill Ends' }) + }) + + // should send sources if they're defined in the config + it('should send sources in autocomplete if defined in config', () => { + // create mock API to check query + const mockPeliasAPI = { + autocomplete: query => { + expect(query.sources).toBe(mockSources) + return Promise.resolve() + } + } + const pelias = new PeliasGeocoder( + mockPeliasAPI, + { ...geocoder, sources: mockSources } + ) + pelias.autocomplete({ text: 'Mill Ends' }) + }) + + // sources should not be sent unless they are explicitly defined in the + // query. See https://github.com/ibi-group/trimet-mod-otp/issues/239 + it('should not send sources in search by default', () => { + // create mock API to check query + const mockPeliasAPI = { + search: query => { + expect(query.sources).not.toBe(expect.anything()) + return Promise.resolve() + } + } + const pelias = new PeliasGeocoder(mockPeliasAPI, geocoder) + pelias.search({ text: 'Mill Ends' }) + }) + + // should send sources if they're defined in the config + it('should send sources in search if defined in config', () => { + // create mock API to check query + const mockPeliasAPI = { + search: query => { + expect(query.sources).toBe(mockSources) + return Promise.resolve() + } + } + const pelias = new PeliasGeocoder( + mockPeliasAPI, + { ...geocoder, sources: mockSources } + ) + pelias.search({ text: 'Mill Ends' }) + }) + } }) }) }) diff --git a/lib/util/geocoder.js b/lib/util/geocoder.js index c3aae4001..8ce68c9dc 100644 --- a/lib/util/geocoder.js +++ b/lib/util/geocoder.js @@ -20,14 +20,8 @@ class Geocoder { * address or POI, attempt to find possible matches. */ autocomplete (query) { - const {apiKey, baseUrl, boundary, focusPoint} = this.geocoderConfig - return this.api.autocomplete({ - apiKey, - boundary, - focusPoint, - url: baseUrl ? `${baseUrl}/autocomplete` : undefined, - ...query - }).then(this._rewriteAutocompleteResponse) + return this.api.autocomplete(this.getAutocompleteQuery(query)) + .then(this.rewriteAutocompleteResponse) } /** @@ -50,46 +44,76 @@ class Geocoder { * GPS coordiante. */ reverse (query) { + return this.api.reverse(this.getReverseQuery(query)) + .then(this.rewriteReverseResponse) + } + + /** + * Perform a search query. A search query is different from autocomplete in + * that it is assumed that the text provided is more or less a complete + * well-fromatted address. + */ + search (query) { + return this.api.search(this.getSearchQuery(query)) + .then(this.rewriteSearchResponse) + } + + /** + * Default autocomplete query generator + */ + getAutocompleteQuery (query) { + const {apiKey, baseUrl, boundary, focusPoint} = this.geocoderConfig + return { + apiKey, + boundary, + focusPoint, + url: baseUrl ? `${baseUrl}/autocomplete` : undefined, + ...query + } + } + + /** + * Default reverse query generator + */ + getReverseQuery (query) { const {apiKey, baseUrl} = this.geocoderConfig - return this.api.reverse({ + return { apiKey, format: true, url: baseUrl ? `${baseUrl}/reverse` : undefined, ...query - }).then(this._rewriteReverseResponse) + } } /** - * Perform a search query. This query assumes that the text being searched - * is more-or-less an exact address or POI. + * Default search query generator. */ - search (query) { + getSearchQuery (query) { const {apiKey, baseUrl, boundary, focusPoint} = this.geocoderConfig - return this.api.search({ + return { apiKey, boundary, focusPoint, - sources: null, url: baseUrl ? `${baseUrl}/search` : undefined, format: false, // keep as returned GeoJSON, ...query - }).then(this._rewriteSearchResponse) + } } /** * Default rewriter for autocomplete responses */ - _rewriteAutocompleteResponse (response) { return response } + rewriteAutocompleteResponse (response) { return response } /** * Default rewriter for reverse responses */ - _rewriteReverseResponse (response) { return response } + rewriteReverseResponse (response) { return response } /** * Default rewriter for search responses */ - _rewriteSearchResponse (response) { return response } + rewriteSearchResponse (response) { return response } } /** @@ -118,7 +142,7 @@ class ArcGISGeocoder extends Geocoder { * Rewrite an autocomplete response into an application specific data format. * Also, filter out any results that are collections. */ - _rewriteAutocompleteResponse (response) { + rewriteAutocompleteResponse (response) { return { // remove any autocomplete results that are collections // (eg multiple Starbucks) @@ -137,7 +161,7 @@ class ArcGISGeocoder extends Geocoder { * Rewrite the response into an application-specific data format using the * first feature returned from the geocoder. */ - _rewriteReverseResponse (response) { + rewriteReverseResponse (response) { const { features, query } = response const { lat, lon } = query return { @@ -159,7 +183,7 @@ class NoApiGeocoder extends Geocoder { * Use coordinate string parser. */ autocomplete (query) { - return this._parseCoordinateString(query.text) + return this.parseCoordinateString(query.text) } /** @@ -167,8 +191,8 @@ class NoApiGeocoder extends Geocoder { */ reverse (query) { let { lat, lon } = query.point - lat = this._roundGPSDecimal(lat) - lon = this._roundGPSDecimal(lon) + lat = this.roundGPSDecimal(lat) + lon = this.roundGPSDecimal(lon) return Promise.resolve({ lat, lon, name: `${lat}, ${lon}` }) } @@ -176,14 +200,14 @@ class NoApiGeocoder extends Geocoder { * Use coordinate string parser. */ search (query) { - return this._parseCoordinateString(query.text) + return this.parseCoordinateString(query.text) } /** * Attempt to parse the input as a GPS coordinate. If parseable, return a * feature. */ - _parseCoordinateString (string) { + parseCoordinateString (string) { let feature try { feature = { @@ -201,7 +225,7 @@ class NoApiGeocoder extends Geocoder { return Promise.resolve({ features: [feature] }) } - _roundGPSDecimal (number) { + roundGPSDecimal (number) { const roundFactor = 100000 return Math.round(number * roundFactor) / roundFactor } @@ -211,14 +235,55 @@ class NoApiGeocoder extends Geocoder { * Geocoder implementation for the Pelias geocoder. * See https://pelias.io * + * This is exported for testing purposes only. + * * @extends Geocoder */ -class PeliasGeocoder extends Geocoder { +export class PeliasGeocoder extends Geocoder { + /** + * Generate an autocomplete query specifically for the Pelias API. The + * `sources` parameter is a Pelias-specific option. + */ + getAutocompleteQuery (query) { + const {apiKey, baseUrl, boundary, focusPoint, sources} = this.geocoderConfig + return { + apiKey, + boundary, + focusPoint, + // explicitly send over null for sources if provided sources is not truthy + // in order to avoid default isomorphic-mapzen-search sources form being + // applied + sources: sources || null, + url: baseUrl ? `${baseUrl}/autocomplete` : undefined, + ...query + } + } + + /** + * Generate a search query specifically for the Pelias API. The + * `sources` parameter is a Pelias-specific option. + */ + getSearchQuery (query) { + const {apiKey, baseUrl, boundary, focusPoint, sources} = this.geocoderConfig + return { + apiKey, + boundary, + focusPoint, + // explicitly send over null for sources if provided sources is not truthy + // in order to avoid default isomorphic-mapzen-search sources form being + // applied + sources: sources || null, + url: baseUrl ? `${baseUrl}/search` : undefined, + format: false, // keep as returned GeoJSON, + ...query + } + } + /** * Rewrite the response into an application-specific data format using the * first feature returned from the geocoder. */ - _rewriteReverseResponse (response) { + rewriteReverseResponse (response) { const { 'point.lat': lat, 'point.lon': lon } = response.isomorphicMapzenSearchQuery return { lat,