fix(carto): Fix broken encoding in POST requests (#8865)

visgl · May 6, 2024 · 280a07d · 280a07d
1 parent 2604ca5
commit 280a07d
Show file tree

Hide file tree

Showing 10 changed files with 86 additions and 30 deletions.
diff --git a/modules/carto/src/api/request-with-parameters.ts b/modules/carto/src/api/request-with-parameters.ts
@@ -1,12 +1,16 @@
+import {isPureObject} from '@loaders.gl/core';
 import {CartoAPIError} from './carto-api-error';
 import {DEFAULT_HEADERS, DEFAULT_PARAMETERS, MAX_GET_LENGTH} from './common';
 import type {APIErrorContext} from './types';
 
 /**
  * Simple encode parameter
  */
-function encodeParameter(name: string, value: string | boolean | number): string {
-  return `${name}=${encodeURIComponent(value)}`;
+function encodeParameter(name: string, value: unknown): string {
+  if (isPureObject(value) || Array.isArray(value)) {
+    return `${name}=${encodeURIComponent(JSON.stringify(value))}`;
+  }
+  return `${name}=${encodeURIComponent(value as string | boolean | number)}`;
 }
 
 const REQUEST_CACHE = new Map<string, Promise<unknown>>();
@@ -17,10 +21,11 @@ export async function requestWithParameters<T = any>({
   errorContext
 }: {
   baseUrl: string;
-  parameters?: Record<string, string>;
+  parameters?: Record<string, unknown>;
   headers: Record<string, string>;
   errorContext: APIErrorContext;
 }): Promise<T> {
+  parameters = {...DEFAULT_PARAMETERS, ...parameters};
   const key = createCacheKey(baseUrl, parameters || {}, customHeaders || {});
   if (REQUEST_CACHE.has(key)) {
     return REQUEST_CACHE.get(key) as Promise<T>;
@@ -58,19 +63,17 @@ export async function requestWithParameters<T = any>({
 
 function createCacheKey(
   baseUrl: string,
-  parameters: Record<string, string>,
+  parameters: Record<string, unknown>,
   headers: Record<string, string>
 ): string {
   const parameterEntries = Object.entries(parameters).sort(([a], [b]) => (a > b ? 1 : -1));
   const headerEntries = Object.entries(headers).sort(([a], [b]) => (a > b ? 1 : -1));
   return JSON.stringify({baseUrl, parameters: parameterEntries, headers: headerEntries});
 }
 
-function createURLWithParameters(baseUrl: string, parameters: Record<string, string>): string {
-  const encodedParameters = Object.entries({...DEFAULT_PARAMETERS, ...parameters}).map(
-    ([key, value]) => {
-      return encodeParameter(key, value);
-    }
+function createURLWithParameters(baseUrl: string, parameters: Record<string, unknown>): string {
+  const encodedParameters = Object.entries(parameters).map(([key, value]) =>
+    encodeParameter(key, value)
   );
   return `${baseUrl}?${encodedParameters.join('&')}`;
 }
diff --git a/modules/carto/src/sources/base-source.ts b/modules/carto/src/sources/base-source.ts
@@ -19,7 +19,7 @@ export const SOURCE_DEFAULTS: SourceOptionalOptions = {
   headers: {}
 };
 
-export async function baseSource<UrlParameters extends Record<string, string>>(
+export async function baseSource<UrlParameters extends Record<string, unknown>>(
   endpoint: MapType,
   options: Partial<SourceOptionalOptions> & SourceRequiredOptions,
   urlParameters: UrlParameters

diff --git a/modules/carto/src/sources/boundary-query-source.ts b/modules/carto/src/sources/boundary-query-source.ts
@@ -12,11 +12,11 @@ export type BoundaryQuerySourceOptions = SourceOptions &
   };
 type UrlParameters = {
   columns?: string;
-  filters?: string;
+  filters?: Record<string, unknown>;
   tilesetTableName: string;
   matchingColumn: string;
   propertiesSqlQuery: string;
-  queryParameters?: string;
+  queryParameters?: Record<string, unknown> | unknown[];
 };
 
 export const boundaryQuerySource = async function (
@@ -40,10 +40,10 @@ export const boundaryQuerySource = async function (
     urlParameters.columns = columns.join(',');
   }
   if (filters) {
-    urlParameters.filters = JSON.stringify(filters);
+    urlParameters.filters = filters;
   }
   if (queryParameters) {
-    urlParameters.queryParameters = JSON.stringify(queryParameters);
+    urlParameters.queryParameters = queryParameters;
   }
   return baseSource<UrlParameters>('boundary', options, urlParameters) as Promise<TilejsonResult>;
 };
diff --git a/modules/carto/src/sources/boundary-table-source.ts b/modules/carto/src/sources/boundary-table-source.ts
@@ -9,7 +9,7 @@ export type BoundaryTableSourceOptions = SourceOptions &
     propertiesTableName: string;
   };
 type UrlParameters = {
-  filters?: string;
+  filters?: Record<string, unknown>;
   tilesetTableName: string;
   columns?: string;
   matchingColumn: string;
@@ -30,7 +30,7 @@ export const boundaryTableSource = async function (
     urlParameters.columns = columns.join(',');
   }
   if (filters) {
-    urlParameters.filters = JSON.stringify(filters);
+    urlParameters.filters = filters;
   }
   return baseSource<UrlParameters>('boundary', options, urlParameters) as Promise<TilejsonResult>;
 };
diff --git a/modules/carto/src/sources/h3-query-source.ts b/modules/carto/src/sources/h3-query-source.ts
@@ -16,7 +16,7 @@ type UrlParameters = {
   spatialDataType: SpatialDataType;
   spatialDataColumn?: string;
   q: string;
-  queryParameters?: string;
+  queryParameters?: Record<string, unknown> | unknown[];
 };
 
 export const h3QuerySource = async function (
@@ -40,7 +40,7 @@ export const h3QuerySource = async function (
     urlParameters.aggregationResLevel = String(aggregationResLevel);
   }
   if (queryParameters) {
-    urlParameters.queryParameters = JSON.stringify(queryParameters);
+    urlParameters.queryParameters = queryParameters;
   }
   return baseSource<UrlParameters>('query', options, urlParameters) as Promise<TilejsonResult>;
 };
diff --git a/modules/carto/src/sources/quadbin-query-source.ts b/modules/carto/src/sources/quadbin-query-source.ts
@@ -17,7 +17,7 @@ type UrlParameters = {
   spatialDataType: SpatialDataType;
   spatialDataColumn?: string;
   q: string;
-  queryParameters?: string;
+  queryParameters?: Record<string, unknown> | unknown[];
 };
 
 export const quadbinQuerySource = async function (
@@ -41,7 +41,7 @@ export const quadbinQuerySource = async function (
     urlParameters.aggregationResLevel = String(aggregationResLevel);
   }
   if (queryParameters) {
-    urlParameters.queryParameters = JSON.stringify(queryParameters);
+    urlParameters.queryParameters = queryParameters;
   }
   return baseSource<UrlParameters>('query', options, urlParameters) as Promise<TilejsonResult>;
 };
diff --git a/modules/carto/src/sources/vector-query-source.ts b/modules/carto/src/sources/vector-query-source.ts
@@ -17,12 +17,12 @@ export type VectorQuerySourceOptions = SourceOptions &
 
 type UrlParameters = {
   columns?: string;
-  filters?: string;
+  filters?: Record<string, unknown>;
   spatialDataType: SpatialDataType;
   spatialDataColumn?: string;
   tileResolution?: string;
   q: string;
-  queryParameters?: string;
+  queryParameters?: Record<string, unknown> | unknown[];
 };
 
 export const vectorQuerySource = async function (
@@ -48,10 +48,10 @@ export const vectorQuerySource = async function (
     urlParameters.columns = columns.join(',');
   }
   if (filters) {
-    urlParameters.filters = JSON.stringify(filters);
+    urlParameters.filters = filters;
   }
   if (queryParameters) {
-    urlParameters.queryParameters = JSON.stringify(queryParameters);
+    urlParameters.queryParameters = queryParameters;
   }
   return baseSource<UrlParameters>('query', options, urlParameters) as Promise<TilejsonResult>;
 };
diff --git a/modules/carto/src/sources/vector-table-source.ts b/modules/carto/src/sources/vector-table-source.ts
@@ -16,7 +16,7 @@ export type VectorTableSourceOptions = SourceOptions &
   ColumnsOption;
 type UrlParameters = {
   columns?: string;
-  filters?: string;
+  filters?: Record<string, unknown>;
   spatialDataType: SpatialDataType;
   spatialDataColumn?: string;
   tileResolution?: string;
@@ -45,7 +45,7 @@ export const vectorTableSource = async function (
     urlParameters.columns = columns.join(',');
   }
   if (filters) {
-    urlParameters.filters = JSON.stringify(filters);
+    urlParameters.filters = filters;
   }
   return baseSource<UrlParameters>('table', options, urlParameters) as Promise<TilejsonResult>;
 };
diff --git a/test/modules/carto/api/request-with-parameters.spec.ts b/test/modules/carto/api/request-with-parameters.spec.ts
@@ -101,3 +101,51 @@ test('requestWithParameters#nocacheErrorContext', async t => {
   );
   t.end();
 });
+
+test('requestWithParameters#method', async t => {
+  await withMockFetchMapsV3(async calls => {
+    t.equals(calls.length, 0, '0 initial calls');
+
+    await Promise.all([
+      requestWithParameters({
+        baseUrl: 'https://example.com/v1/params',
+        headers: {},
+        parameters: {object: {a: 1, b: 2}, array: [1, 2, 3], string: 'short'}
+      }),
+      requestWithParameters({
+        baseUrl: `https://example.com/v1/params`,
+        headers: {},
+        parameters: {object: {a: 1, b: 2}, array: [1, 2, 3], string: 'long'.padEnd(10_000, 'g')}
+      })
+    ]);
+
+    t.equals(calls.length, 2, '2 requests');
+
+    // GET
+    t.true(calls[0].url.startsWith('https://example.com/v1/params?'), 'get - url');
+    t.equals(calls[0].method, undefined, 'get - method');
+    t.equals(calls[0].body, undefined, 'get - body');
+    t.deepEquals(
+      Array.from(new URL(calls[0].url).searchParams.entries()),
+      [
+        ['v', '3.4'],
+        ['deckglVersion', 'untranspiled source'],
+        ['object', '{"a":1,"b":2}'],
+        ['array', '[1,2,3]'],
+        ['string', 'short']
+      ],
+      'get - params'
+    );
+
+    // POST
+    const postBody = JSON.parse(calls[1].body as string);
+    t.equals(calls[1].method, 'POST', 'post - method');
+    t.equals(postBody.v, '3.4', 'post - body.v');
+    t.equals(postBody.deckglVersion, 'untranspiled source', 'post - body.deckglVersion');
+    t.deepEquals(postBody.object, {a: 1, b: 2}, 'post - body.object');
+    t.deepEquals(postBody.array, [1, 2, 3], 'post - body.array');
+    t.true(postBody.string.startsWith('longgg'), 'post - body.string');
+    t.equals(calls[1].url, 'https://example.com/v1/params', 'post - url');
+  });
+  t.end();
+});
diff --git a/test/modules/carto/mock-fetch.ts b/test/modules/carto/mock-fetch.ts
@@ -5,7 +5,12 @@ import binaryTileData from './data/binaryTile.json';
 const BINARY_TILE = new Uint8Array(binaryTileData).buffer;
 
 const fetch = globalThis.fetch;
-type MockFetchCall = {url: string; headers: Record<string, unknown>};
+type MockFetchCall = {
+  url: string;
+  headers: Record<string, unknown>;
+  method?: 'GET' | 'POST';
+  body?: string;
+};
 
 export const TILEJSON_RESPONSE = {
   tilejson: '2.2.0',
@@ -96,8 +101,8 @@ async function setupMockFetchMapsV3(
 ): Promise<MockFetchCall[]> {
   const calls: MockFetchCall[] = [];
 
-  const mockFetch = (url: string, {headers}) => {
-    calls.push({url, headers});
+  const mockFetch = (url: string, {headers, method, body}) => {
+    calls.push({url, headers, method, body});
     if (url.indexOf('formatTiles=binary') !== -1) {
       headers = {...headers, 'Content-Type': 'application/vnd.carto-vector-tile'};
     }
@@ -114,7 +119,7 @@ function teardownMockFetchMaps() {
 }
 
 export async function withMockFetchMapsV3(
-  testFunc: (calls: {url: string; headers: Record<string, unknown>}[]) => Promise<void>,
+  testFunc: (calls: MockFetchCall[]) => Promise<void>,
   responseFunc: (
     url: string,
     headers: HeadersInit,