Skip to content

Commit bd15b76

Browse files
donmccurdydonmccurdy
authored
fix(carto): Clean up and add unit tests for requestWithParameters cache (visgl#8707)
1 parent db26ab6 commit bd15b76

File tree

5 files changed

+162
-48
lines changed

5 files changed

+162
-48
lines changed

‎modules/carto/src/api/request-with-parameters.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@ function encodeParameter(name: string, value: string | boolean | number): string
1111

1212
const REQUEST_CACHE = new Map<string, Promise<unknown>>();
1313
export async function requestWithParameters<T = any>({
14-
accessToken,
1514
baseUrl,
1615
parameters,
1716
headers: customHeaders,
1817
errorContext
1918
}: {
20-
accessToken?: string;
2119
baseUrl: string;
2220
parameters?: Record<string, string>;
2321
headers: Record<string, string>;
@@ -47,9 +45,6 @@ export async function requestWithParameters<T = any>({
4745
if (!response || !response.ok) {
4846
throw new Error(json.error);
4947
}
50-
if (accessToken) {
51-
json.accessToken = accessToken;
52-
}
5348
return json;
5449
})
5550
.catch((error: Error) => {

‎modules/carto/src/sources/base-source.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,15 @@ export async function baseSource<UrlParameters extends Record<string, string>>(
5656
errorContext.requestType = 'Map data';
5757

5858
if (format === 'tilejson') {
59-
return await requestWithParameters<TilejsonResult>({
60-
accessToken,
59+
const json = await requestWithParameters<TilejsonResult>({
6160
baseUrl: dataUrl,
6261
headers,
6362
errorContext
6463
});
64+
if (accessToken) {
65+
json.accessToken = accessToken;
66+
}
67+
return json;
6568
}
6669

6770
return await requestWithParameters<GeojsonResult | JsonResult>({

‎test/modules/carto/api/request-with-parameters.spec.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import test from 'tape-catch';
2+
import {requestWithParameters} from '@deck.gl/carto/api/request-with-parameters';
3+
import {withMockFetchMapsV3} from '../mock-fetch';
4+
import {CartoAPIError} from '@deck.gl/carto';
5+
6+
test('requestWithParameters#cacheBaseURL', async t => {
7+
await withMockFetchMapsV3(async calls => {
8+
t.equals(calls.length, 0, '0 initial calls');
9+
10+
await Promise.all([
11+
requestWithParameters({baseUrl: 'https://example.com/v1/baseURL', headers: {}}),
12+
requestWithParameters({baseUrl: 'https://example.com/v2/baseURL', headers: {}}),
13+
requestWithParameters({baseUrl: 'https://example.com/v2/baseURL', headers: {}})
14+
]);
15+
16+
t.equals(calls.length, 2, '2 unique requests');
17+
});
18+
t.end();
19+
});
20+
21+
test('requestWithParameters#cacheHeaders', async t => {
22+
await withMockFetchMapsV3(async calls => {
23+
t.equals(calls.length, 0, '0 initial calls');
24+
25+
await Promise.all([
26+
requestWithParameters({baseUrl: 'https://example.com/v1/headers', headers: {a: 1}}),
27+
requestWithParameters({baseUrl: 'https://example.com/v1/headers', headers: {a: 1}}),
28+
requestWithParameters({baseUrl: 'https://example.com/v1/headers', headers: {b: 1}})
29+
]);
30+
31+
t.equals(calls.length, 2, '2 unique requests');
32+
});
33+
t.end();
34+
});
35+
36+
test('requestWithParameters#cacheParameters', async t => {
37+
await withMockFetchMapsV3(async calls => {
38+
t.equals(calls.length, 0, '0 initial calls');
39+
40+
await Promise.all([
41+
requestWithParameters({
42+
baseUrl: 'https://example.com/v1/params',
43+
headers: {},
44+
parameters: {}
45+
}),
46+
requestWithParameters({
47+
baseUrl: 'https://example.com/v1/params',
48+
headers: {},
49+
parameters: {}
50+
}),
51+
requestWithParameters({
52+
baseUrl: 'https://example.com/v1/params',
53+
headers: {},
54+
parameters: {a: 1}
55+
})
56+
]);
57+
58+
t.equals(calls.length, 2, '2 unique requests');
59+
});
60+
t.end();
61+
});
62+
63+
test('requestWithParameters#nocacheErrorContext', async t => {
64+
await withMockFetchMapsV3(
65+
async calls => {
66+
t.equals(calls.length, 0, '0 initial calls');
67+
68+
let error1: Error | undefined;
69+
let error2: Error | undefined;
70+
71+
try {
72+
await requestWithParameters({
73+
baseUrl: 'https://example.com/v1/errorContext',
74+
errorContext: {requestType: 'Map data'}
75+
});
76+
t.fail('request #1 should fail, but did not');
77+
} catch (error) {
78+
error1 = error as Error;
79+
}
80+
81+
try {
82+
await requestWithParameters({
83+
baseUrl: 'https://example.com/v1/errorContext',
84+
errorContext: {requestType: 'SQL'}
85+
});
86+
t.fail('request #2 should fail, but did not');
87+
} catch (error) {
88+
error2 = error as Error;
89+
}
90+
91+
t.equals(calls.length, 2, '2 unique requests, failures not cached');
92+
t.true(error1 instanceof CartoAPIError, 'error #1 type');
93+
t.is((error1 as CartoAPIError).errorContext.requestType, 'Map data', 'error #1 context');
94+
t.true(error2 instanceof CartoAPIError, 'error #2 type');
95+
t.is((error2 as CartoAPIError).errorContext.requestType, 'SQL', 'error #2 context');
96+
},
97+
// @ts-ignore
98+
(url: string, headers: HeadersInit) => {
99+
return Promise.reject(new Error('404 Not Found'));
100+
}
101+
);
102+
t.end();
103+
});

‎test/modules/carto/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import './api/carto-api-error.spec';
22
import './api/fetch-map.spec';
33
import './api/layer-map.spec';
44
import './api/parse-map.spec';
5+
import './api/request-with-parameters.spec';
56
import './utils.spec';
67
import './layers/carto-vector-tile.spec';
78
import './layers/h3-tile-layer.spec';

‎test/modules/carto/mock-fetch.ts

Lines changed: 53 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -46,55 +46,62 @@ export const TILESTATS_RESPONSE = {
4646
type: 'Number'
4747
};
4848

49+
const createDefaultResponse = (
50+
url: string,
51+
headers: HeadersInit,
52+
cacheKey?: string
53+
): Promise<unknown> => {
54+
return Promise.resolve({
55+
json: () => {
56+
if (url.indexOf('format=tilejson') !== -1) {
57+
return TILEJSON_RESPONSE;
58+
}
59+
if (url.indexOf('format=geojson') !== -1) {
60+
return GEOJSON_RESPONSE;
61+
}
62+
63+
if (url.indexOf('tileset') !== -1) {
64+
return {
65+
tilejson: {
66+
url: [`https://xyz.com?format=tilejson&cache=${cacheKey}`]
67+
}
68+
};
69+
}
70+
if (url.indexOf('stats') !== -1) {
71+
return TILESTATS_RESPONSE;
72+
}
73+
if (url.indexOf('query') !== -1 || url.indexOf('table')) {
74+
return {
75+
tilejson: {
76+
url: [`https://xyz.com?format=tilejson&cache=${cacheKey}`]
77+
},
78+
geojson: {
79+
url: [`https://xyz.com?format=geojson&cache=${cacheKey}`]
80+
}
81+
};
82+
}
83+
return null;
84+
},
85+
arrayBuffer: () => BINARY_TILE,
86+
text: () => null, // Required to get loaders.gl to use arrayBuffer()
87+
ok: true,
88+
url,
89+
headers: new Headers(headers)
90+
});
91+
};
92+
4993
async function setupMockFetchMapsV3(
94+
responseFunc = createDefaultResponse,
5095
cacheKey = btoa(Math.random().toFixed(4))
5196
): Promise<MockFetchCall[]> {
5297
const calls: MockFetchCall[] = [];
5398

5499
const mockFetch = (url: string, {headers}) => {
55100
calls.push({url, headers});
56-
57101
if (url.indexOf('formatTiles=binary') !== -1) {
58102
headers = {...headers, 'Content-Type': 'application/vnd.carto-vector-tile'};
59103
}
60-
61-
return Promise.resolve({
62-
json: () => {
63-
if (url.indexOf('format=tilejson') !== -1) {
64-
return TILEJSON_RESPONSE;
65-
}
66-
if (url.indexOf('format=geojson') !== -1) {
67-
return GEOJSON_RESPONSE;
68-
}
69-
70-
if (url.indexOf('tileset') !== -1) {
71-
return {
72-
tilejson: {
73-
url: [`https://xyz.com?format=tilejson&cache=${cacheKey}`]
74-
}
75-
};
76-
}
77-
if (url.indexOf('stats') !== -1) {
78-
return TILESTATS_RESPONSE;
79-
}
80-
if (url.indexOf('query') !== -1 || url.indexOf('table')) {
81-
return {
82-
tilejson: {
83-
url: [`https://xyz.com?format=tilejson&cache=${cacheKey}`]
84-
},
85-
geojson: {
86-
url: [`https://xyz.com?format=geojson&cache=${cacheKey}`]
87-
}
88-
};
89-
}
90-
return null;
91-
},
92-
arrayBuffer: () => BINARY_TILE,
93-
text: () => null, // Required to get loaders.gl to use arrayBuffer()
94-
ok: true,
95-
url,
96-
headers: new Headers(headers)
97-
});
104+
return responseFunc(url, headers, cacheKey);
98105
};
99106

100107
globalThis.fetch = mockFetch as unknown as typeof fetch;
@@ -107,10 +114,15 @@ function teardownMockFetchMaps() {
107114
}
108115

109116
export async function withMockFetchMapsV3(
110-
testFunc: (calls: {url: string; headers: Record<string, unknown>}[]) => Promise<void>
117+
testFunc: (calls: {url: string; headers: Record<string, unknown>}[]) => Promise<void>,
118+
responseFunc: (
119+
url: string,
120+
headers: HeadersInit,
121+
cacheKey?: string
122+
) => Promise<unknown> = createDefaultResponse
111123
): Promise<void> {
112124
try {
113-
const calls = await setupMockFetchMapsV3();
125+
const calls = await setupMockFetchMapsV3(responseFunc);
114126
await testFunc(calls);
115127
} finally {
116128
teardownMockFetchMaps();

0 commit comments

Comments
 (0)