Skip to content

fix: scoped keys for prefix content (DON'T BACKPORT TO V5) #703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 30, 2024
Merged

fix: scoped keys for prefix content (DON'T BACKPORT TO V5) #703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82966df
fix: scoped keys for prefix content
stefan-guggisberg Sep 30, 2024
41ec5ec
trivial: typos in comments
stefan-guggisberg Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/json-pipe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ async function fetchJsonContent(state, req, res) {
if (state.content.sourceBus === 'content') {
keys.push(await computeSurrogateKey(`${contentBusId}${info.path}`));
keys.push(contentBusId);
const contentKeyPrefix = partition === 'preview' ? 'p_' : '';
if (partition === 'preview') {
// temporarily provide additional preview content keys
// TODO: eventually provide either (prefixed) preview or (unprefixed) live content keys
keys.push(`${contentKeyPrefix}${await computeSurrogateKey(`${contentBusId}${info.path}`)}`);
keys.push(`${contentKeyPrefix}${contentBusId}`);
}
} else {
keys.push(`${ref}--${repo}--${owner}_code`);
keys.push(await computeSurrogateKey(`${ref}--${repo}--${owner}${info.path}`));
Expand Down Expand Up @@ -104,6 +111,13 @@ async function computeSurrogateKeys(state) {
if (state.content.sourceBus.includes('content')) {
keys.push(await computeSurrogateKey(`${state.contentBusId}${state.info.path}`));
keys.push(state.contentBusId);
const contentKeyPrefix = state.partition === 'preview' ? 'p_' : '';
if (state.partition === 'preview') {
// temporarily provide additional preview content keys
// TODO: eventually provide either (prefixed) preview or (unprefixed) live content keys
keys.push(`${contentKeyPrefix}${await computeSurrogateKey(`${state.contentBusId}${state.info.path}`)}`);
keys.push(`${contentKeyPrefix}${state.contentBusId}`);
}
}

return keys;
Expand Down
19 changes: 16 additions & 3 deletions src/steps/fetch-404.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { getPathKey } from './set-x-surrogate-key-header.js';
*/
export default async function fetch404(state, req, res) {
const {
owner, repo, ref, contentBusId,
owner, repo, ref, contentBusId, partition,
} = state;
const ret = await state.s3Loader.getObject('helix-code-bus', `${owner}/${repo}/${ref}/404.html`);
if (ret.status === 200) {
Expand All @@ -46,12 +46,25 @@ export default async function fetch404(state, req, res) {
`${ref}--${repo}--${owner}_404`,
`${ref}--${repo}--${owner}_code`,
];
const contentKeyPrefix = partition === 'preview' ? 'p_' : '';
if (partition === 'preview') {
// temporarily provide additional preview content keys
// TODO: eventually provide either (prefixed) preview or (unprefixed) live content keys
keys.push(`${contentKeyPrefix}${pathKey}`);
keys.push(`${contentKeyPrefix}${contentBusId}`);
}

if (state.info.unmappedPath) {
keys.push(await getPathKey({
const unmappedPathKey = await getPathKey({
contentBusId,
info: { path: state.info.unmappedPath },
}));
});
keys.push(unmappedPathKey);
if (partition === 'preview') {
// temporarily provide additional preview content key
// TODO: eventually provide either (prefixed) preview or (unprefixed) live content key
keys.push(`${contentKeyPrefix}${unmappedPathKey}`);
}
}

res.headers.set('x-surrogate-key', keys.join(' '));
Expand Down
7 changes: 7 additions & 0 deletions src/steps/fetch-content.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ export default async function fetchContent(state, req, res) {
} else {
keys.push(await computeSurrogateKey(`${contentBusId}${info.path}`));
keys.push(contentBusId);
const contentKeyPrefix = partition === 'preview' ? 'p_' : '';
if (partition === 'preview') {
// temporarily provide additional preview content keys
// TODO: eventually provide either (prefixed) preview or (unprefixed) live content keys
keys.push(`${contentKeyPrefix}${await computeSurrogateKey(`${contentBusId}${info.path}`)}`);
keys.push(`${contentKeyPrefix}${contentBusId}`);
}
}
res.headers.set('x-surrogate-key', keys.join(' '));
res.error = 'moved';
Expand Down
21 changes: 20 additions & 1 deletion src/steps/set-x-surrogate-key-header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ export async function getPathKey(state) {
*/
export default async function setXSurrogateKeyHeader(state, req, res) {
const {
contentBusId, owner, repo, ref,
contentBusId, owner, repo, ref, partition,
} = state;

const isCode = state.content.sourceBus === 'code';

const contentKeyPrefix = partition === 'preview' ? 'p_' : '';
const keys = [];
const hash = await getPathKey(state);
if (isCode) {
Expand All @@ -57,6 +58,13 @@ export default async function setXSurrogateKeyHeader(state, req, res) {
keys.push(`${contentBusId}_metadata`);
keys.push(`${ref}--${repo}--${owner}_head`);
keys.push(contentBusId);
if (partition === 'preview') {
// temporarily provide additional preview content keys
// TODO: eventually provide either (prefixed) preview or (unprefixed) live content keys
keys.push(`${contentKeyPrefix}${hash}`);
keys.push(`${contentKeyPrefix}${contentBusId}_metadata`);
keys.push(`${contentKeyPrefix}${contentBusId}`);
}
}
// for folder-mapped resources, we also need to include the surrogate key of the mapped metadata
if (state.mapped) {
Expand All @@ -67,6 +75,17 @@ export default async function setXSurrogateKeyHeader(state, req, res) {
info: { path: state.info.unmappedPath },
}));
}
if (partition === 'preview') {
// temporarily provide additional preview content keys
// TODO: eventually provide either (prefixed) preview or (unprefixed) live content keys
keys.push(`${contentKeyPrefix}${hash}_metadata`);
if (state.info.unmappedPath) {
keys.push(`${contentKeyPrefix}${await getPathKey({
contentBusId,
info: { path: state.info.unmappedPath },
})}`);
}
}
}
res.headers.set('x-surrogate-key', keys.join(' '));
}
65 changes: 55 additions & 10 deletions test/json-pipe.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ describe('JSON Pipe Test', () => {
'last-modified': 'Wed, 12 Oct 2009 17:50:00 GMT',
},
}),
)
.reply(
'helix-content-bus',
'foobar/live/en/index.json',
new PipelineResponse(TEST_SINGLE_SHEET, {
headers: {
'content-type': 'application/json',
'x-amz-meta-x-source-location': 'foo-bar',
'last-modified': 'Wed, 12 Oct 2009 17:50:00 GMT',
},
}),
),
timer: {
update: () => {},
Expand All @@ -126,7 +137,23 @@ describe('JSON Pipe Test', () => {

it('fetches correct content', async () => {
const state = createDefaultState();
const resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10&offset=5'));
let resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10&offset=5'));
assert.strictEqual(resp.status, 200);
assert.deepStrictEqual(await resp.json(), {
':type': 'sheet',
offset: 5,
limit: 10,
total: TEST_DATA.length,
data: TEST_DATA.slice(5, 15),
});
assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'content-type': 'application/json',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar p_Atrz_qDg26DmSe9a p_foobar',
'last-modified': 'Wed, 12 Oct 2009 17:50:00 GMT',
});
// live (code coverage)
state.partition = 'live';
resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10&offset=5'));
assert.strictEqual(resp.status, 200);
assert.deepStrictEqual(await resp.json(), {
':type': 'sheet',
Expand All @@ -148,7 +175,7 @@ describe('JSON Pipe Test', () => {
assert.strictEqual(resp.status, 404);
assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'x-error': 'failed to load /config.json: 404',
'x-surrogate-key': 'U_NW4adJU7Qazf-I pzrU-nNKQOUYNTEf ref--repo--owner_code kz8SoCaNqfp4ohQo foobar',
'x-surrogate-key': 'U_NW4adJU7Qazf-I pzrU-nNKQOUYNTEf ref--repo--owner_code kz8SoCaNqfp4ohQo foobar p_kz8SoCaNqfp4ohQo p_foobar',
});
});

Expand All @@ -165,7 +192,7 @@ describe('JSON Pipe Test', () => {
assert.strictEqual(resp.status, 200);
assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'content-type': 'application/json',
'x-surrogate-key': 'U_NW4adJU7Qazf-I pzrU-nNKQOUYNTEf ref--repo--owner_code kz8SoCaNqfp4ohQo foobar',
'x-surrogate-key': 'U_NW4adJU7Qazf-I pzrU-nNKQOUYNTEf ref--repo--owner_code kz8SoCaNqfp4ohQo foobar p_kz8SoCaNqfp4ohQo p_foobar',
});
assert.deepStrictEqual(await resp.json(), {
public: {
Expand Down Expand Up @@ -193,7 +220,7 @@ describe('JSON Pipe Test', () => {
});
assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'content-type': 'application/json',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar p_Atrz_qDg26DmSe9a p_foobar',
'last-modified': 'Wed, 12 Oct 2009 17:50:00 GMT',
});
});
Expand All @@ -214,7 +241,7 @@ describe('JSON Pipe Test', () => {
'access-control-allow-origin': '*',
'content-security-policy': 'default-src \'self\'',
'last-modified': 'Wed, 12 Oct 2009 17:50:00 GMT',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar p_Atrz_qDg26DmSe9a p_foobar',
'content-type': 'application/json',
});
});
Expand All @@ -229,7 +256,7 @@ describe('JSON Pipe Test', () => {
'access-control-allow-origin': '*',
'content-security-policy': 'default-src \'self\'',
'x-error': 'failed to load /en/index.json: 404',
'x-surrogate-key': 'SIMSxecp2CJXqGYs ref--repo--owner_code Atrz_qDg26DmSe9a foobar',
'x-surrogate-key': 'SIMSxecp2CJXqGYs ref--repo--owner_code Atrz_qDg26DmSe9a foobar p_Atrz_qDg26DmSe9a p_foobar',
});
});

Expand All @@ -244,8 +271,26 @@ describe('JSON Pipe Test', () => {
'x-amz-meta-redirect-location': '/de/index.json',
},
}),
);
const resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10&offset=5'));
)
.reply(
'helix-content-bus',
'foobar/live/en/index.json',
new PipelineResponse(TEST_SINGLE_SHEET, {
headers: {
'content-type': 'application/json',
'x-amz-meta-redirect-location': '/de/index.json',
},
}),
);
let resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10&offset=5'));
assert.strictEqual(resp.status, 301);
assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'location': '/de/index.json',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar p_Atrz_qDg26DmSe9a p_foobar',
});
// live (code coverage)
state.partition = 'live';
resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10&offset=5'));
assert.strictEqual(resp.status, 301);
assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'location': '/de/index.json',
Expand Down Expand Up @@ -301,7 +346,7 @@ describe('JSON Pipe Test', () => {
const headers = Object.fromEntries(resp.headers.entries());
assert.deepStrictEqual(headers, {
'last-modified': 'Wed, 12 Oct 2009 17:50:00 GMT',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar p_Atrz_qDg26DmSe9a p_foobar',
'content-type': 'application/json',
});
});
Expand Down Expand Up @@ -333,7 +378,7 @@ describe('JSON Pipe Test', () => {
const headers = Object.fromEntries(resp.headers.entries());
assert.deepStrictEqual(headers, {
'last-modified': 'Wed, 12 Oct 2009 15:50:00 GMT',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar',
'x-surrogate-key': 'Atrz_qDg26DmSe9a foobar p_Atrz_qDg26DmSe9a p_foobar',
'content-type': 'application/json',
});
});
Expand Down
35 changes: 29 additions & 6 deletions test/rendering.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
config = DEFAULT_CONFIG;
});

async function render(url, selector = '', expectedStatus = 200) {
async function render(url, selector = '', expectedStatus = 200, partition = 'live') {
const req = new PipelineRequest(url, {
headers: new Map([['host', url.hostname]]),
body: '',
Expand All @@ -160,7 +160,7 @@
org: 'adobe',
site: 'helix-pages',
ref: 'super-test',
partition: 'live',
partition,
config,
path: selector ? `${url.pathname}${selector}.html` : url.pathname,
timer: {
Expand All @@ -174,7 +174,7 @@
}

// eslint-disable-next-line default-param-last
async function testRender(url, domSelector = 'main', expStatus) {
async function testRender(url, domSelector = 'main', expStatus, partition = 'live') {
if (!(url instanceof URL)) {
// eslint-disable-next-line no-param-reassign
url = new URL(`https://helix-pages.com/${url}`);
Expand All @@ -191,9 +191,9 @@
// eslint-disable-next-line no-param-reassign
expStatus = expHtml === null ? 404 : 200;
}
const response = await render(url, '', expStatus);
const response = await render(url, '', expStatus, partition);
const actHtml = response.body;
console.log(actHtml);

Check warning on line 196 in test/rendering.test.js

View workflow job for this annotation

GitHub Actions / Test 

Unexpected console statement
if (expStatus === 200) {
const $actMain = new JSDOM(actHtml).window.document.querySelector(domSelector);
const $expMain = new JSDOM(expHtml).window.document.querySelector(domSelector);
Expand Down Expand Up @@ -516,6 +516,8 @@

it('renders 404 if content not found', async () => {
await testRender('not-found', 'html');
// preview (code coverage)
await testRender('not-found', 'html', 404, 'preview');
});

it('can render empty table row', async () => {
Expand Down Expand Up @@ -593,12 +595,27 @@
'x-surrogate-key': 'gPHXKWdMY_R8KV2Z foo-id super-test--helix-pages--adobe_404 super-test--helix-pages--adobe_code QJqsV4atnOA47sHc',
});
assert.strictEqual(body.trim(), '');
// preview (code coverage)
const resp = await testRender(new URL('https://helix-pipeline.com/broken/folder'), 'html', 404, 'preview');
assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'access-control-allow-origin': '*',
'content-type': 'text/html; charset=utf-8',
link: '</scripts/scripts.js>; rel=modulepreload; as=script; crossorigin=use-credentials',
'x-error': 'failed to load /not-a-page.md from content-bus: 404',
'x-surrogate-key': 'gPHXKWdMY_R8KV2Z foo-id super-test--helix-pages--adobe_404 super-test--helix-pages--adobe_code p_gPHXKWdMY_R8KV2Z p_foo-id QJqsV4atnOA47sHc p_QJqsV4atnOA47sHc',
});
assert.strictEqual(resp.body.trim(), '');
});

it('renders 301 for redirect file', async () => {
loader.headers('one-section.md', 'x-amz-meta-redirect-location', 'https://www.adobe.com');
const ret = await render(new URL('https://localhost/one-section'), '', 301);
assert.strictEqual(ret.headers.get('location'), 'https://www.adobe.com');
let resp = await render(new URL('https://localhost/one-section'), '', 301);
assert.strictEqual(resp.headers.get('location'), 'https://www.adobe.com');
assert.strictEqual(resp.headers.get('x-surrogate-key'), 'oHjg_WDu20CBS4rD foo-id');
// preview (code coverage)
resp = await render(new URL('https://localhost/one-section'), '', 301, 'preview');
assert.strictEqual(resp.headers.get('location'), 'https://www.adobe.com');
assert.strictEqual(resp.headers.get('x-surrogate-key'), 'oHjg_WDu20CBS4rD foo-id p_oHjg_WDu20CBS4rD p_foo-id');
});

it('appends .plain.html in redirects', async () => {
Expand Down Expand Up @@ -640,6 +657,12 @@
let resp = await render(new URL('https://helix-pipeline.com/products'), '', 200);
assert.match(resp.body, /<meta property="og:url" content="https:\/\/www.adobe.com\/products">/);

// coverage
loader.status('products.md', 404);
loader.status('generic-product.md', 200);
resp = await render(new URL('https://helix-pipeline.com/products'), '', 200, 'preview');
assert.match(resp.body, /<meta property="og:url" content="https:\/\/www.adobe.com\/products">/);

loader.status('product1.md', 404);
loader.rewrite('generic-product/metadata.json', 'metadata-product.json');
resp = await render(new URL('https://helix-pipeline.com/products/product1'), '', 200);
Expand Down
Loading