From 2f75abae5f96e3cb402bff9d863b4b1f0bae42b1 Mon Sep 17 00:00:00 2001 From: Don McCurdy Date: Tue, 17 Sep 2024 14:19:09 -0400 Subject: [PATCH 1/4] feat(carto): Allow configuring MAX_GET_LENGTH --- modules/carto/src/api/common.ts | 20 +++++++++++++++++++- modules/carto/src/api/index.ts | 1 + modules/carto/src/index.ts | 2 +- test/modules/carto/api/common.spec.ts | 12 ++++++++++++ test/modules/carto/index.ts | 1 + 5 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 test/modules/carto/api/common.spec.ts diff --git a/modules/carto/src/api/common.ts b/modules/carto/src/api/common.ts index 147c0183f5f..52b88902e16 100644 --- a/modules/carto/src/api/common.ts +++ b/modules/carto/src/api/common.ts @@ -1,4 +1,22 @@ export const DEFAULT_API_BASE_URL = 'https://gcp-us-east1.api.carto.com'; export const DEFAULT_CLIENT = 'deck-gl-carto'; export const V3_MINOR_VERSION = '3.4'; -export const MAX_GET_LENGTH = 8192; + +// Fastly default limit is 8192; leave some padding. +export let MAX_GET_LENGTH = 7000; + +/** + * Returns maximum URL character length. Above this limit, requests use POST. + * Used to avoid browser and CDN limits. + */ +export function getRequestURLLimit(): number { + return MAX_GET_LENGTH; +} + +/** + * Assigns maximum URL character length. Above this limit, requests use POST. + * Used to avoid browser and CDN limits. + */ +export function setRequestURLLimit(limit: number): void { + MAX_GET_LENGTH = limit; +} diff --git a/modules/carto/src/api/index.ts b/modules/carto/src/api/index.ts index f60309335ed..71a92b4ed33 100644 --- a/modules/carto/src/api/index.ts +++ b/modules/carto/src/api/index.ts @@ -1,4 +1,5 @@ export {CartoAPIError} from './carto-api-error'; +export {getRequestURLLimit, setRequestURLLimit} from './common'; export {fetchMap} from './fetch-map'; export type {FetchMapOptions, FetchMapResult} from './fetch-map'; export type { diff --git a/modules/carto/src/index.ts b/modules/carto/src/index.ts index 4da1dead449..41b38085eb3 100644 --- a/modules/carto/src/index.ts +++ b/modules/carto/src/index.ts @@ -36,7 +36,7 @@ export { export {default as colorBins} from './style/color-bins-style'; export {default as colorCategories} from './style/color-categories-style'; export {default as colorContinuous} from './style/color-continuous-style'; -export {CartoAPIError, fetchMap, query} from './api/index'; +export {CartoAPIError, fetchMap, query, getRequestURLLimit, setRequestURLLimit} from './api/index'; export {fetchBasemapProps} from './api/basemap'; export type { APIErrorContext, diff --git a/test/modules/carto/api/common.spec.ts b/test/modules/carto/api/common.spec.ts new file mode 100644 index 00000000000..89127a91982 --- /dev/null +++ b/test/modules/carto/api/common.spec.ts @@ -0,0 +1,12 @@ +import {setRequestURLLimit, getRequestURLLimit} from '@deck.gl/carto'; +import test from 'tape-catch'; + +test('request URL limit', async t => { + const defaultLimit = getRequestURLLimit(); + setRequestURLLimit(9999); + + t.true(Number.isInteger(defaultLimit), 'integer default url limit'); + t.true(defaultLimit > 0, 'positive default url limit'); + t.equal(getRequestURLLimit(), 9999, 'custom url limit'); + t.end(); +}); diff --git a/test/modules/carto/index.ts b/test/modules/carto/index.ts index dbcd51b0cb1..e83a94f4695 100644 --- a/test/modules/carto/index.ts +++ b/test/modules/carto/index.ts @@ -1,5 +1,6 @@ import './api/basemap.spec'; import './api/carto-api-error.spec'; +import './api/common.spec'; import './api/fetch-map.spec'; import './api/layer-map.spec'; import './api/parse-map.spec'; From b27e73572dcb9c89fba53388cee12655fced9726 Mon Sep 17 00:00:00 2001 From: Don McCurdy Date: Wed, 18 Sep 2024 14:00:47 -0400 Subject: [PATCH 2/4] Refactor to 'maxLengthURL' parameter on sources --- modules/carto/src/api/common.ts | 18 +--------- modules/carto/src/api/index.ts | 1 - .../carto/src/api/request-with-parameters.ts | 8 +++-- modules/carto/src/index.ts | 2 +- modules/carto/src/sources/base-source.ts | 11 ++++--- modules/carto/src/sources/types.ts | 7 ++++ test/modules/carto/api/common.spec.ts | 12 ------- .../carto/api/request-with-parameters.spec.ts | 33 +++++++++++++++++++ test/modules/carto/index.ts | 1 - 9 files changed, 54 insertions(+), 39 deletions(-) delete mode 100644 test/modules/carto/api/common.spec.ts diff --git a/modules/carto/src/api/common.ts b/modules/carto/src/api/common.ts index 52b88902e16..120ffa321e0 100644 --- a/modules/carto/src/api/common.ts +++ b/modules/carto/src/api/common.ts @@ -3,20 +3,4 @@ export const DEFAULT_CLIENT = 'deck-gl-carto'; export const V3_MINOR_VERSION = '3.4'; // Fastly default limit is 8192; leave some padding. -export let MAX_GET_LENGTH = 7000; - -/** - * Returns maximum URL character length. Above this limit, requests use POST. - * Used to avoid browser and CDN limits. - */ -export function getRequestURLLimit(): number { - return MAX_GET_LENGTH; -} - -/** - * Assigns maximum URL character length. Above this limit, requests use POST. - * Used to avoid browser and CDN limits. - */ -export function setRequestURLLimit(limit: number): void { - MAX_GET_LENGTH = limit; -} +export const DEFAULT_MAX_LENGTH_URL = 7000; diff --git a/modules/carto/src/api/index.ts b/modules/carto/src/api/index.ts index 71a92b4ed33..f60309335ed 100644 --- a/modules/carto/src/api/index.ts +++ b/modules/carto/src/api/index.ts @@ -1,5 +1,4 @@ export {CartoAPIError} from './carto-api-error'; -export {getRequestURLLimit, setRequestURLLimit} from './common'; export {fetchMap} from './fetch-map'; export type {FetchMapOptions, FetchMapResult} from './fetch-map'; export type { diff --git a/modules/carto/src/api/request-with-parameters.ts b/modules/carto/src/api/request-with-parameters.ts index a38724b178c..c7641e591b7 100644 --- a/modules/carto/src/api/request-with-parameters.ts +++ b/modules/carto/src/api/request-with-parameters.ts @@ -1,7 +1,7 @@ import {VERSION} from '@deck.gl/core'; import {isPureObject} from '../utils'; import {CartoAPIError} from './carto-api-error'; -import {MAX_GET_LENGTH, V3_MINOR_VERSION} from './common'; +import {DEFAULT_MAX_LENGTH_URL, V3_MINOR_VERSION} from './common'; import type {APIErrorContext} from './types'; /** @@ -25,12 +25,14 @@ export async function requestWithParameters({ baseUrl, parameters = {}, headers: customHeaders = {}, - errorContext + errorContext, + maxLengthURL = DEFAULT_MAX_LENGTH_URL }: { baseUrl: string; parameters?: Record; headers?: Record; errorContext: APIErrorContext; + maxLengthURL?: number; }): Promise { parameters = {...DEFAULT_PARAMETERS, ...parameters}; baseUrl = excludeURLParameters(baseUrl, Object.keys(parameters)); @@ -44,7 +46,7 @@ export async function requestWithParameters({ /* global fetch */ const fetchPromise = - url.length > MAX_GET_LENGTH + url.length > maxLengthURL ? fetch(baseUrl, {method: 'POST', body: JSON.stringify(parameters), headers}) : fetch(url, {headers}); diff --git a/modules/carto/src/index.ts b/modules/carto/src/index.ts index 41b38085eb3..4da1dead449 100644 --- a/modules/carto/src/index.ts +++ b/modules/carto/src/index.ts @@ -36,7 +36,7 @@ export { export {default as colorBins} from './style/color-bins-style'; export {default as colorCategories} from './style/color-categories-style'; export {default as colorContinuous} from './style/color-continuous-style'; -export {CartoAPIError, fetchMap, query, getRequestURLLimit, setRequestURLLimit} from './api/index'; +export {CartoAPIError, fetchMap, query} from './api/index'; export {fetchBasemapProps} from './api/basemap'; export type { APIErrorContext, diff --git a/modules/carto/src/sources/base-source.ts b/modules/carto/src/sources/base-source.ts index 45b31fbc0ce..503f9d1524e 100644 --- a/modules/carto/src/sources/base-source.ts +++ b/modules/carto/src/sources/base-source.ts @@ -32,7 +32,7 @@ export async function baseSource>( } } const baseUrl = buildSourceUrl(mergedOptions); - const {clientId, format} = mergedOptions; + const {clientId, maxLengthURL, format} = mergedOptions; const headers = {Authorization: `Bearer ${options.accessToken}`, ...options.headers}; const parameters = {client: clientId, ...urlParameters}; @@ -46,7 +46,8 @@ export async function baseSource>( baseUrl, parameters, headers, - errorContext + errorContext, + maxLengthURL }); const dataUrl = mapInstantiation[format].url[0]; @@ -59,7 +60,8 @@ export async function baseSource>( const json = await requestWithParameters({ baseUrl: dataUrl, headers, - errorContext + errorContext, + maxLengthURL }); if (accessToken) { json.accessToken = accessToken; @@ -70,6 +72,7 @@ export async function baseSource>( return await requestWithParameters({ baseUrl: dataUrl, headers, - errorContext + errorContext, + maxLengthURL }); } diff --git a/modules/carto/src/sources/types.ts b/modules/carto/src/sources/types.ts index 11786e1c2dd..a43b390298d 100644 --- a/modules/carto/src/sources/types.ts +++ b/modules/carto/src/sources/types.ts @@ -35,6 +35,13 @@ export type SourceOptionalOptions = { clientId: string; /** @deprecated use `query` instead **/ format: Format; + + /** + * Maximum URL character length. Above this limit, requests use POST. + * Used to avoid browser and CDN limits. + * @default {@link DEFAULT_MAX_LENGTH_URL} + */ + maxLengthURL?: number; }; export type SourceOptions = SourceRequiredOptions & Partial; diff --git a/test/modules/carto/api/common.spec.ts b/test/modules/carto/api/common.spec.ts deleted file mode 100644 index 89127a91982..00000000000 --- a/test/modules/carto/api/common.spec.ts +++ /dev/null @@ -1,12 +0,0 @@ -import {setRequestURLLimit, getRequestURLLimit} from '@deck.gl/carto'; -import test from 'tape-catch'; - -test('request URL limit', async t => { - const defaultLimit = getRequestURLLimit(); - setRequestURLLimit(9999); - - t.true(Number.isInteger(defaultLimit), 'integer default url limit'); - t.true(defaultLimit > 0, 'positive default url limit'); - t.equal(getRequestURLLimit(), 9999, 'custom url limit'); - t.end(); -}); diff --git a/test/modules/carto/api/request-with-parameters.spec.ts b/test/modules/carto/api/request-with-parameters.spec.ts index f17c7303d6a..0624b2a35b3 100644 --- a/test/modules/carto/api/request-with-parameters.spec.ts +++ b/test/modules/carto/api/request-with-parameters.spec.ts @@ -192,3 +192,36 @@ test('requestWithParameters#precedence', async t => { }); t.end(); }); + +test('requestWithParameters#maxLengthURL', async t => { + await withMockFetchMapsV3(async calls => { + t.equals(calls.length, 0, '0 initial calls'); + + await Promise.all([ + requestWithParameters({ + baseUrl: 'https://example.com/v1/item/1' + }), + requestWithParameters({ + baseUrl: 'https://example.com/v1/item/2', + maxLengthURL: 10 + }), + requestWithParameters({ + baseUrl: `https://example.com/v1/item/3`, + parameters: {content: 'long'.padEnd(10_000, 'g')} // > default limit + }), + requestWithParameters({ + baseUrl: `https://example.com/v1/item/4`, + parameters: {content: 'long'.padEnd(10_000, 'g')}, + maxLengthURL: 15_000 + }) + ]); + + t.equals(calls.length, 4, '4 requests'); + t.deepEquals( + calls.map(({method}) => method ?? 'GET'), + ['GET', 'POST', 'POST', 'GET'], + 'request method' + ); + }); + t.end(); +}); diff --git a/test/modules/carto/index.ts b/test/modules/carto/index.ts index e83a94f4695..dbcd51b0cb1 100644 --- a/test/modules/carto/index.ts +++ b/test/modules/carto/index.ts @@ -1,6 +1,5 @@ import './api/basemap.spec'; import './api/carto-api-error.spec'; -import './api/common.spec'; import './api/fetch-map.spec'; import './api/layer-map.spec'; import './api/parse-map.spec'; From 0c89a922d1a23132184efe23ceb8ea9065367de6 Mon Sep 17 00:00:00 2001 From: Don McCurdy Date: Wed, 18 Sep 2024 14:22:11 -0400 Subject: [PATCH 3/4] Include query() and fetchMap() --- modules/carto/src/api/fetch-map.ts | 56 ++++++++++++++++-------- modules/carto/src/api/query.ts | 4 +- modules/carto/src/sources/base-source.ts | 5 ++- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/modules/carto/src/api/fetch-map.ts b/modules/carto/src/api/fetch-map.ts index 757a2532866..a4950674a57 100644 --- a/modules/carto/src/api/fetch-map.ts +++ b/modules/carto/src/api/fetch-map.ts @@ -1,6 +1,6 @@ /* eslint-disable camelcase */ import {CartoAPIError} from './carto-api-error'; -import {DEFAULT_API_BASE_URL, DEFAULT_CLIENT} from './common'; +import {DEFAULT_API_BASE_URL, DEFAULT_CLIENT, DEFAULT_MAX_LENGTH_URL} from './common'; import {buildPublicMapUrl, buildStatsUrl} from './endpoints'; import { GeojsonResult, @@ -36,13 +36,14 @@ type Dataset = { }; /* global clearInterval, setInterval, URL */ -/* eslint-disable complexity, max-statements */ +/* eslint-disable complexity, max-statements, max-params */ async function _fetchMapDataset( dataset: Dataset, accessToken: string, apiBaseUrl: string, clientId?: string, - headers?: Record + headers?: Record, + maxLengthURL = DEFAULT_MAX_LENGTH_URL ) { const { aggregationExp, @@ -64,7 +65,8 @@ async function _fetchMapDataset( clientId, connectionName, format, - headers + headers, + maxLengthURL }; if (type === 'tileset') { @@ -114,7 +116,8 @@ async function _fetchTilestats( attribute: string, dataset: Dataset, accessToken: string, - apiBaseUrl: string + apiBaseUrl: string, + maxLengthURL = DEFAULT_MAX_LENGTH_URL ) { const {connectionName, data, id, source, type, queryParameters} = dataset; const errorContext: APIErrorContext = { @@ -144,7 +147,8 @@ async function _fetchTilestats( baseUrl, headers, parameters, - errorContext + errorContext, + maxLengthURL }); // Replace tilestats for attribute with value from API @@ -158,17 +162,19 @@ async function fillInMapDatasets( {datasets, token}: {datasets: Dataset[]; token: string}, clientId: string, apiBaseUrl: string, - headers?: Record + headers?: Record, + maxLengthURL = DEFAULT_MAX_LENGTH_URL ) { const promises = datasets.map(dataset => - _fetchMapDataset(dataset, token, apiBaseUrl, clientId, headers) + _fetchMapDataset(dataset, token, apiBaseUrl, clientId, headers, maxLengthURL) ); return await Promise.all(promises); } async function fillInTileStats( {datasets, keplerMapConfig, token}: {datasets: Dataset[]; keplerMapConfig: any; token: string}, - apiBaseUrl: string + apiBaseUrl: string, + maxLengthURL = DEFAULT_MAX_LENGTH_URL ) { const attributes: {attribute: string; dataset: any}[] = []; const {layers} = keplerMapConfig.config.visState; @@ -197,7 +203,7 @@ async function fillInTileStats( } const promises = filteredAttributes.map(({attribute, dataset}) => - _fetchTilestats(attribute, dataset, token, apiBaseUrl) + _fetchTilestats(attribute, dataset, token, apiBaseUrl, maxLengthURL) ); return await Promise.all(promises); } @@ -237,6 +243,13 @@ export type FetchMapOptions = { * Callback function that will be invoked whenever data in layers is changed. If provided, `autoRefresh` must also be provided. */ onNewData?: (map: any) => void; + + /** + * Maximum URL character length. Above this limit, requests use POST. + * Used to avoid browser and CDN limits. + * @default {@link DEFAULT_MAX_LENGTH_URL} + */ + maxLengthURL?: number; }; export type FetchMapResult = ParseMapResult & { @@ -255,7 +268,8 @@ export async function fetchMap({ clientId = DEFAULT_CLIENT, headers = {}, autoRefresh, - onNewData + onNewData, + maxLengthURL = DEFAULT_MAX_LENGTH_URL }: FetchMapOptions): Promise { assert(cartoMapId, 'Must define CARTO map id: fetchMap({cartoMapId: "XXXX-XXXX-XXXX"})'); assert(apiBaseUrl, 'Must define apiBaseUrl'); @@ -275,7 +289,7 @@ export async function fetchMap({ const baseUrl = buildPublicMapUrl({apiBaseUrl, cartoMapId}); const errorContext: APIErrorContext = {requestType: 'Public map', mapId: cartoMapId}; - const map = await requestWithParameters({baseUrl, headers, errorContext}); + const map = await requestWithParameters({baseUrl, headers, errorContext, maxLengthURL}); // Periodically check if the data has changed. Note that this // will not update when a map is published. @@ -283,10 +297,16 @@ export async function fetchMap({ if (autoRefresh) { // eslint-disable-next-line @typescript-eslint/no-misused-promises const intervalId = setInterval(async () => { - const changed = await fillInMapDatasets(map, clientId, apiBaseUrl, { - ...headers, - 'If-Modified-Since': new Date().toUTCString() - }); + const changed = await fillInMapDatasets( + map, + clientId, + apiBaseUrl, + { + ...headers, + 'If-Modified-Since': new Date().toUTCString() + }, + maxLengthURL + ); if (onNewData && changed.some(v => v === true)) { onNewData(parseMap(map)); } @@ -315,11 +335,11 @@ export async function fetchMap({ fetchBasemapProps({config: map.keplerMapConfig.config, errorContext}), // Mutates map.datasets so that dataset.data contains data - fillInMapDatasets(map, clientId, apiBaseUrl, headers) + fillInMapDatasets(map, clientId, apiBaseUrl, headers, maxLengthURL) ]); // Mutates attributes in visualChannels to contain tile stats - await fillInTileStats(map, apiBaseUrl); + await fillInTileStats(map, apiBaseUrl, maxLengthURL); const out = {...parseMap(map), basemap, ...{stopAutoRefresh}}; diff --git a/modules/carto/src/api/query.ts b/modules/carto/src/api/query.ts index 060c23039ae..db91dc0a431 100644 --- a/modules/carto/src/api/query.ts +++ b/modules/carto/src/api/query.ts @@ -11,6 +11,7 @@ export const query = async function (options: QueryOptions): Promise>( From 0e55cdb6801238ee9ee9a68f9e3b8cd21e5ab02a Mon Sep 17 00:00:00 2001 From: Don McCurdy Date: Thu, 19 Sep 2024 10:57:30 -0400 Subject: [PATCH 4/4] docs(carto): add maxLengthURL, remove mapsUrl --- docs/api-reference/carto/data-sources.md | 36 +++++++++++++----------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/docs/api-reference/carto/data-sources.md b/docs/api-reference/carto/data-sources.md index fb2ccf85f82..2751c6f4a15 100644 --- a/docs/api-reference/carto/data-sources.md +++ b/docs/api-reference/carto/data-sources.md @@ -11,8 +11,8 @@ import {vectorTableSource} from '@deck.gl/carto'; const data = vectorTableSource({ accessToken: 'XXX', connectionName: 'carto_dw', - tableName: 'carto-demo-data.demo_tables.chicago_crime_sample', -}) + tableName: 'carto-demo-data.demo_tables.chicago_crime_sample' +}); ``` ### Promise API @@ -51,7 +51,7 @@ type SourceOptions = { apiBaseUrl?: string; clientId?: string; headers?: Record; - mapsUrl?: string; + maxLengthURL?: number; }; ``` @@ -64,7 +64,7 @@ type VectorTableSourceOptions = { columns?: string[]; spatialDataColumn?: string; tableName: string; -} +}; ``` #### vectorQuerySource @@ -74,7 +74,7 @@ type VectorQuerySourceOptions = { spatialDataColumn?: string; sqlQuery: string; queryParameters: QueryParameters; -} +}; ``` #### vectorTilesetSource @@ -82,7 +82,7 @@ type VectorQuerySourceOptions = { ```ts type VectorTilesetSourceOptions = { tableName: string; -} +}; ``` #### h3TableSource @@ -94,7 +94,7 @@ type H3TableSourceOptions = { columns?: string[]; spatialDataColumn?: string; tableName: string; -} +}; ``` #### h3QuerySource @@ -106,7 +106,7 @@ type H3QuerySourceOptions = { spatialDataColumn?: string; sqlQuery: string; queryParameters: QueryParameters; -} +}; ``` #### h3TilesetSource @@ -114,7 +114,7 @@ type H3QuerySourceOptions = { ```ts type H3TilesetSourceOptions = { tableName: string; -} +}; ``` #### quadbinTableSource @@ -126,7 +126,7 @@ type QuadbinTableSourceOptions = { columns?: string[]; spatialDataColumn?: string; tableName: string; -} +}; ``` #### quadbinQuerySource @@ -138,7 +138,7 @@ type QuadbinQuerySourceOptions = { spatialDataColumn?: string; sqlQuery: string; queryParameters: QueryParameters; -} +}; ``` #### quadbinTilesetSource @@ -146,7 +146,7 @@ type QuadbinQuerySourceOptions = { ```ts type QuadbinTilesetSourceOptions = { tableName: string; -} +}; ``` #### rasterTilesetSource (Experimental) @@ -154,7 +154,7 @@ type QuadbinTilesetSourceOptions = { ```ts type RasterTilesetSourceOptions = { tableName: string; -} +}; ``` Boundary sources are experimental sources where both the tileset and the properties props need a specific schema to work. [Read more about Boundaries in the CARTO documentation](https://docs.carto.com/carto-for-developers/guides/use-boundaries-in-your-application). @@ -166,7 +166,7 @@ type BoundaryTableSourceOptions = { tilesetTableName: string; columns?: string[]; propertiesTableName: string; -} +}; ``` #### boundaryQuerySource (Experimental) @@ -176,7 +176,7 @@ type BoundaryQuerySourceOptions = { tilesetTableName: string; propertiesSqlQuery: string; queryParameters?: QueryParameters; -} +}; ``` ### QueryParameters @@ -184,6 +184,7 @@ type BoundaryQuerySourceOptions = { QueryParameters are used to parametrize SQL queries. The format depends on the source's provider, some examples: [PostgreSQL and Redshift](https://node-postgres.com/features/queries): + ```ts vectorQuerySource({ ..., @@ -193,6 +194,7 @@ vectorQuerySource({ ``` [BigQuery positional](https://cloud.google.com/bigquery/docs/parameterized-queries#node.js): + ```ts vectorQuerySource({ ..., @@ -201,8 +203,8 @@ vectorQuerySource({ }) ``` - [BigQuery named parameters](https://cloud.google.com/bigquery/docs/parameterized-queries#node.js): + ```ts vectorQuerySource({ ..., @@ -212,6 +214,7 @@ vectorQuerySource({ ``` [Snowflake positional](https://docs.snowflake.com/en/user-guide/nodejs-driver-use.html#binding-statement-parameters) : + ```ts vectorQuerySource({ ..., @@ -230,6 +233,7 @@ vectorQuerySource({ ``` [Databricks ODBC](https://github.com/markdirish/node-odbc#bindparameters-callback) + ```ts vectorQuerySource({ ...