Skip to content

Commit be60120

Browse files
hannessoloCopilot
hannessolo
and
Copilot
authored
fix: read JSON sheets from the DA source (#328)
* fix: read JSON sheets from the DA source Route JSON GET and HEAD requests through the DA source reader instead of AEM preview so sheets do not need to be previewed first. Add regression tests for JSON content, headers, authorization, and case-insensitive extensions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: fall back to AEM preview for code-stored JSON Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: handle JSON with parallel asset reads Reuse the GET and HEAD asset routing for JSON so DA and AEM are read in parallel with the same response precedence. Update regression tests for concurrency, fallback, and upstream failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7536b34 commit be60120

4 files changed

Lines changed: 356 additions & 22 deletions

File tree

‎src/handlers/get.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export default async function getHandler({ req, env, daCtx }) {
2424

2525
if (path.startsWith('/gimme_cookie')) return getCookie({ req, env, daCtx });
2626

27-
const resourceRegex = /\.(css|js|js\.map|json|xml|woff|woff2|otf|ttf|plain\.html|html)$/i;
27+
const resourceRegex = /\.(css|js|js\.map|xml|woff|woff2|otf|ttf|plain\.html|html)$/i;
2828
if (resourceRegex.test(path)) {
2929
try {
3030
return await handleAEMProxyRequest({ req, env, daCtx });
@@ -35,7 +35,7 @@ export default async function getHandler({ req, env, daCtx }) {
3535
}
3636
}
3737

38-
const assetRegex = /\.(png|jpg|jpeg|webp|gif|svg|ico|avif)$/i;
38+
const assetRegex = /\.(png|jpg|jpeg|webp|gif|svg|ico|avif|json)$/i;
3939
if (assetRegex.test(path)) {
4040
const [daSourceGetRes, aemProxyRes] = await Promise.allSettled([
4141
daSourceGet({ req, env, daCtx }),

‎src/handlers/head.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export default async function headHandler({ req, env, daCtx }) {
3030
if (path.startsWith('/favicon.ico')) return head404();
3131
if (path.startsWith('/robots.txt')) return getRobots();
3232

33-
const resourceRegex = /\.(css|js|js\.map|json|xml|woff|woff2|otf|ttf|plain\.html|html)$/i;
33+
const resourceRegex = /\.(css|js|js\.map|xml|woff|woff2|otf|ttf|plain\.html|html)$/i;
3434
if (resourceRegex.test(path)) {
3535
try {
3636
return await aemHead({ req, env, daCtx });
@@ -41,7 +41,7 @@ export default async function headHandler({ req, env, daCtx }) {
4141
}
4242
}
4343

44-
const assetRegex = /\.(png|jpg|jpeg|webp|gif|svg|ico|avif)$/i;
44+
const assetRegex = /\.(png|jpg|jpeg|webp|gif|svg|ico|avif|json)$/i;
4545
if (assetRegex.test(path)) {
4646
const [daSourceHeadRes, aemHeadRes] = await Promise.allSettled([
4747
daSourceHead({ env, daCtx }),

‎test/handlers/get.test.js‎

Lines changed: 168 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,6 @@ describe('GET handler', () => {
129129
assert.strictEqual(await res.text(), 'proxied-resource');
130130
});
131131

132-
it('proxies to AEM for .json', async () => {
133-
const daCtx = getDaCtx(reqs.nonHtmlFile);
134-
const env = {};
135-
136-
const res = await getHandler({ req: reqs.nonHtmlFile, env, daCtx });
137-
138-
assert.strictEqual(res.status, 200);
139-
assert.strictEqual(await res.text(), 'proxied-resource');
140-
});
141-
142132
it('proxies to AEM for explicit .html', async () => {
143133
const req = new Request('https://main--site--org.preview.da.live/folder/content.html');
144134
const daCtx = getDaCtx(req);
@@ -151,6 +141,174 @@ describe('GET handler', () => {
151141
});
152142
});
153143

144+
describe('JSON sheets', () => {
145+
it('starts the AEM read before the DA JSON read settles', async () => {
146+
const req = new Request('https://main--site--org.preview.da.live/config.json');
147+
let resolveSource;
148+
const source = new Promise((resolve) => {
149+
resolveSource = resolve;
150+
});
151+
let aemStarted = false;
152+
const handler = (await esmock('../../src/handlers/get.js', {
153+
'../../src/routes/da-admin.js': { daSourceGet: async () => source },
154+
'../../src/routes/aem-proxy.js': {
155+
handleAEMProxyRequest: async () => {
156+
aemStarted = true;
157+
return new Response('{"code":true}');
158+
},
159+
},
160+
})).default;
161+
162+
const pending = handler({ req, env: {}, daCtx: getDaCtx(req) });
163+
const startedBeforeSourceSettled = aemStarted;
164+
resolveSource(new Response(null, { status: 404 }));
165+
const res = await pending;
166+
167+
assert.ok(startedBeforeSourceSettled);
168+
assert.strictEqual(await res.text(), '{"code":true}');
169+
});
170+
171+
['json', 'JSON'].forEach((ext) => {
172+
it(`falls back to AEM for code-stored .${ext} when DA returns 404`, async () => {
173+
const req = new Request(`https://main--site--org.preview.da.live/config.${ext}?limit=1`);
174+
const daCtx = getDaCtx(req);
175+
const body = '{"code":true}';
176+
let sourceRead = false;
177+
const handler = (await esmock('../../src/handlers/get.js', {
178+
'../../src/routes/da-admin.js': {
179+
daSourceGet: async () => {
180+
sourceRead = true;
181+
return new Response(null, { status: 404 });
182+
},
183+
},
184+
'../../src/routes/aem-proxy.js': {
185+
handleAEMProxyRequest: async (args) => {
186+
assert.ok(sourceRead);
187+
assert.strictEqual(args.req, req);
188+
assert.strictEqual(args.daCtx, daCtx);
189+
return new Response(body, { headers: { 'Content-Type': 'application/json' } });
190+
},
191+
},
192+
})).default;
193+
194+
const res = await handler({ req, env: {}, daCtx });
195+
196+
assert.strictEqual(res.status, 200);
197+
assert.strictEqual(res.headers.get('Content-Type'), 'application/json');
198+
assert.strictEqual(await res.text(), body);
199+
});
200+
});
201+
202+
[401, 403, 500, 503].forEach((status) => {
203+
it(`prefers AEM 200 for JSON when DA returns ${status}`, async () => {
204+
const req = new Request('https://main--site--org.preview.da.live/config.json');
205+
const response = new Response('{"code":true}', {
206+
headers: { 'Content-Type': 'application/json' },
207+
});
208+
const handler = (await esmock('../../src/handlers/get.js', {
209+
'../../src/routes/da-admin.js': {
210+
daSourceGet: async () => new Response(null, { status }),
211+
},
212+
'../../src/routes/aem-proxy.js': {
213+
handleAEMProxyRequest: async () => response,
214+
},
215+
})).default;
216+
217+
const res = await handler({ req, env: {}, daCtx: getDaCtx(req) });
218+
219+
assert.strictEqual(res, response);
220+
});
221+
});
222+
223+
[500, 503].forEach((status) => {
224+
[404, 'failure'].forEach((result) => {
225+
it(`preserves DA ${status} for JSON when AEM returns ${result}`, async () => {
226+
const req = new Request('https://main--site--org.preview.da.live/config.json');
227+
const response = new Response('DA error', { status });
228+
const handler = (await esmock('../../src/handlers/get.js', {
229+
'../../src/routes/da-admin.js': { daSourceGet: async () => response },
230+
'../../src/routes/aem-proxy.js': {
231+
handleAEMProxyRequest: async () => {
232+
if (result === 'failure') {
233+
throw new UpstreamError(PREVIEW_HOST, new TypeError('fetch failed'));
234+
}
235+
return new Response(null, { status: result });
236+
},
237+
},
238+
})).default;
239+
240+
const res = await handler({ req, env: {}, daCtx: getDaCtx(req) });
241+
242+
assert.strictEqual(res, response);
243+
});
244+
});
245+
});
246+
247+
[404, 'failure'].forEach((result) => {
248+
it(`handles AEM ${result} after DA returns 404 for JSON`, async () => {
249+
const req = new Request('https://main--site--org.preview.da.live/config.json');
250+
const handler = (await esmock('../../src/handlers/get.js', {
251+
'../../src/routes/da-admin.js': {
252+
daSourceGet: async () => new Response(null, { status: 404 }),
253+
},
254+
'../../src/routes/aem-proxy.js': {
255+
handleAEMProxyRequest: async () => {
256+
if (result === 'failure') {
257+
throw new UpstreamError(PREVIEW_HOST, new TypeError('fetch failed'));
258+
}
259+
return new Response(null, { status: result });
260+
},
261+
},
262+
})).default;
263+
264+
const res = await handler({ req, env: {}, daCtx: getDaCtx(req) });
265+
266+
assert.strictEqual(res.status, 404);
267+
assert.strictEqual(await res.text(), '');
268+
});
269+
});
270+
271+
['json', 'JSON'].forEach((ext) => {
272+
it(`reads an unpreviewed .${ext} sheet from DA without changing the JSON`, async () => {
273+
const body = '{"total":1,"data":[{"title":"Unpreviewed"}]}';
274+
const req = new Request(`https://main--site--org.preview.da.live/folder/content.${ext}?limit=1`, {
275+
headers: { Authorization: 'Bearer test-token' },
276+
});
277+
const daCtx = getDaCtx(req);
278+
const { daSourceGet } = await esmock('../../src/routes/da-admin.js', {
279+
'../../src/storage/site.js': {
280+
default: async () => ({ exists: true, onSourceBus: false }),
281+
},
282+
});
283+
const handler = (await esmock('../../src/handlers/get.js', {
284+
'../../src/routes/da-admin.js': { daSourceGet },
285+
'../../src/routes/aem-proxy.js': {
286+
handleAEMProxyRequest: async () => new Response('{"code":true}', {
287+
headers: { 'Content-Type': 'application/json' },
288+
}),
289+
},
290+
})).default;
291+
const env = {
292+
DA_ADMIN: 'https://admin.da.live',
293+
daadmin: {
294+
fetch: async (url, init) => {
295+
assert.strictEqual(url.toString(), 'https://admin.da.live/source/org/site/folder/content.json');
296+
assert.strictEqual(init.method, 'GET');
297+
assert.strictEqual(init.headers.get('Authorization'), 'Bearer test-token');
298+
return new Response(body, { headers: { 'Content-Type': 'application/json' } });
299+
},
300+
},
301+
};
302+
303+
const res = await handler({ req, env, daCtx });
304+
305+
assert.strictEqual(res.status, 200);
306+
assert.strictEqual(res.headers.get('Content-Type'), 'application/json');
307+
assert.strictEqual(await res.text(), body);
308+
});
309+
});
310+
});
311+
154312
describe('assets', () => {
155313
describe('when daSourceGet returns 200', () => {
156314
let getHandler;

0 commit comments

Comments
 (0)