Skip to content

Commit

Permalink
fix: defer loading of head.html to allow inclusion of request cookie
Browse files Browse the repository at this point in the history 
fixes #2274
  • Loading branch information
@tripodsan
tripodsan committed Nov 15, 2023
1 parent 3f92501 commit 926aff5
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 61 deletions.
21 changes: 20 additions & 1 deletion src/server/HeadHtmlSupport.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export default class HeadHtmlSupport {
this.remoteStatus = 0;
this.localHtml = '';
this.localStatus = 0;
this.cookie = '';
this.url = new URL(proxyUrl);
this.url.pathname = '/head.html';
this.filePath = resolve(directory, 'head.html');
Expand All @@ -89,8 +90,13 @@ export default class HeadHtmlSupport {

async loadRemote() {
// load head from server
const headers = {};
if (this.cookie) {
headers.cookie = this.cookie;
}

Check warning on line 96 in src/server/HeadHtmlSupport.js

View check run for this annotation

Codecov / codecov/patch

src/server/HeadHtmlSupport.js#L95-L96 

Added lines #L95 - L96 were not covered by tests
const resp = await getFetch()(this.url, {
cache: 'no-store',
headers,
});
this.remoteStatus = resp.status;
if (resp.ok) {
Expand All @@ -103,6 +109,18 @@ export default class HeadHtmlSupport {
}
}

setCookie(cookie) {
if (this.cookie !== cookie) {
this.log.trace('registering head-html cookie to ', cookie);
this.cookie = cookie;
this.remoteStatus = 0;
}
}

invalidateLocal() {
this.localStatus = 0;
}

Check warning on line 122 in src/server/HeadHtmlSupport.js

View check run for this annotation

Codecov / codecov/patch

src/server/HeadHtmlSupport.js#L121-L122 

Added lines #L121 - L122 were not covered by tests

async loadLocal() {
try {
this.localHtml = (await fs.readFile(this.filePath, 'utf-8')).trim();
Expand All @@ -114,7 +132,7 @@ export default class HeadHtmlSupport {
}
}

async init() {
async update() {
if (!this.localStatus) {
await this.loadLocal();
}
Expand All @@ -127,6 +145,7 @@ export default class HeadHtmlSupport {
}

async replace(source) {
await this.update();
if (!this.isModified) {
this.log.trace('head.html ignored: not modified locally.');
return source;
Expand Down
4 changes: 1 addition & 3 deletions src/server/HelixProject.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,13 @@ export class HelixProject extends BaseProject {
log: this.log,
proxyUrl: this.proxyUrl,
});
await this._headHtml.init();

// register local head in live-reload
if (this.liveReload) {
this.liveReload.registerFiles([this._headHtml.filePath], '/');
this.liveReload.on('modified', async (modified) => {
if (modified.indexOf('/') >= 0) {
await this._headHtml.loadLocal();
await this._headHtml.init();
this._headHtml.invalidateLocal();

Check warning on line 87 in src/server/HelixProject.js

View check run for this annotation

Codecov / codecov/patch

src/server/HelixProject.js#L87 

Added line #L87 was not covered by tests
}
});
}
Expand Down
3 changes: 2 additions & 1 deletion src/server/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ window.LiveReloadOptions = {

const isHTML = ret.status === 200 && contentType.indexOf('text/html') === 0;
const livereload = isHTML && opts.injectLiveReload;
const replaceHead = isHTML && opts.headHtml && opts.headHtml.isModified;
const replaceHead = isHTML && opts.headHtml;
const doIndex = isHTML && opts.indexer && url.indexOf('.plain.html') < 0;

if (isHTML) {
Expand Down Expand Up @@ -322,6 +322,7 @@ window.LiveReloadOptions = {
ctx.log.trace(lines.join('\n'));
}
if (replaceHead) {
await opts.headHtml.setCookie(req.headers.cookie);
textBody = await opts.headHtml.replace(textBody);
}
if (livereload) {
Expand Down
4 changes: 3 additions & 1 deletion src/up.cmd.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ export default class UpCommand extends AbstractServerCommand {
const resp = await getFetch()(fstabUrl);
await resp.buffer();
if (!resp.ok) {
if (ref === 'main') {
if (resp.status === 401) {
this.log.warn(chalk`Unable to verify {yellow ${ref}} branch via {blue ${fstabUrl}} for authenticated sites.`);

Check warning on line 123 in src/up.cmd.js

View check run for this annotation

Codecov / codecov/patch

src/up.cmd.js#L123 

Added line #L123 was not covered by tests
} else if (ref === 'main') {
this.log.warn(chalk`Unable to verify {yellow main} branch via {blue ${fstabUrl}} (${resp.status}). Maybe not pushed yet?`);
} else {
this.log.warn(chalk`Unable to verify {yellow ${ref}} branch on {blue ${fstabUrl}} (${resp.status}). Fallback to {yellow main} branch.`);
Expand Down
33 changes: 17 additions & 16 deletions test/head-html.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ function removePosition(tree) {

async function init(hhs, localHtml, remoteHtml) {
hhs.localHtml = localHtml;
hhs.localStatus = 200;
hhs.remoteHtml = remoteHtml;
hhs.remoteStatus = 200;
if (remoteHtml) {
hhs.remoteDom = await HeadHtmlSupport.toDom(hhs.remoteHtml);
HeadHtmlSupport.hash(hhs.remoteDom);
Expand Down Expand Up @@ -149,7 +151,7 @@ describe('Head.html loading tests', () => {
it('loads remote head.html', async () => {
const directory = await setupProject(path.join(__rootdir, 'test', 'fixtures', 'project'), testRoot);

const scope = nock('https://main--blog--adobe.hlx.page')
nock('https://main--blog--adobe.hlx.page')
.get('/head.html')
.reply(200, '<!-- remote head html --> <script>a=1;</script>');

Expand Down Expand Up @@ -180,13 +182,12 @@ describe('Head.html loading tests', () => {
type: 'root',
});
assert.strictEqual(hhs.remoteStatus, 200);
scope.done();
});

it('loads missing remote head.html', async () => {
const directory = await setupProject(path.join(__rootdir, 'test', 'fixtures', 'project'), testRoot);

const scope = nock('https://main--blog--adobe.hlx.page')
nock('https://main--blog--adobe.hlx.page')
.get('/head.html')
.reply(404);

Expand All @@ -198,10 +199,9 @@ describe('Head.html loading tests', () => {
await hhs.loadRemote();
assert.strictEqual(hhs.remoteDom, null);
assert.strictEqual(hhs.remoteStatus, 404);
scope.done();
});

it('init loads local and remote head.html', async () => {
it('update loads local and remote head.html', async () => {
const directory = await setupProject(path.join(__rootdir, 'test', 'fixtures', 'project'), testRoot);

nock('https://main--blog--adobe.hlx.page')
Expand All @@ -213,20 +213,21 @@ describe('Head.html loading tests', () => {
proxyUrl: 'https://main--blog--adobe.hlx.page',
directory,
});
await hhs.init();
await hhs.update();
assert.strictEqual(hhs.remoteHtml, '<!-- remote head html --><a>fooo</a>');
assert.strictEqual(hhs.remoteStatus, 200);
assert.strictEqual(hhs.localHtml, '<!-- local head html -->\n<link rel="stylesheet" href="/styles.css"/>');
assert.strictEqual(hhs.localStatus, 200);
assert.strictEqual(hhs.isModified, true);

await hhs.init(); // only once
// only once
await hhs.update();
});

it('init loads local and remote head.html (not modified)', async () => {
it('update loads local and remote head.html (not modified)', async () => {
const directory = await setupProject(path.join(__rootdir, 'test', 'fixtures', 'project'), testRoot);

const scope = nock('https://main--blog--adobe.hlx.page')
nock('https://main--blog--adobe.hlx.page')
.get('/head.html')
.reply(200, '<!-- local head html -->\n<link rel="stylesheet" href="/styles.css"/>');

Expand All @@ -235,34 +236,34 @@ describe('Head.html loading tests', () => {
proxyUrl: 'https://main--blog--adobe.hlx.page',
directory,
});
await hhs.init();
await hhs.update();
assert.strictEqual(hhs.remoteHtml, '<!-- local head html -->\n<link rel="stylesheet" href="/styles.css"/>');
assert.strictEqual(hhs.remoteStatus, 200);
assert.strictEqual(hhs.localHtml, '<!-- local head html -->\n<link rel="stylesheet" href="/styles.css"/>');
assert.strictEqual(hhs.localStatus, 200);
assert.strictEqual(hhs.isModified, false);

await hhs.init(); // only once
scope.done();
// only once
await hhs.update();
});

it('init sets modified to false if local status failed', async () => {
it('update sets modified to false if local status failed', async () => {
const hhs = new HeadHtmlSupport(DEFAULT_OPTS());
hhs.localHtml = '';
hhs.remoteHtml = '';
hhs.localStatus = 404;
hhs.remoteStatus = 200;
await hhs.init();
await hhs.update();
assert.strictEqual(hhs.isModified, false);
});

it('init sets modified to false if remote status failed', async () => {
it('update sets modified to false if remote status failed', async () => {
const hhs = new HeadHtmlSupport(DEFAULT_OPTS());
hhs.localHtml = '';
hhs.remoteHtml = '';
hhs.localStatus = 200;
hhs.remoteStatus = 404;
await hhs.init();
await hhs.update();
assert.strictEqual(hhs.isModified, false);
});
});
24 changes: 3 additions & 21 deletions test/server.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ describe('Helix Server', () => {
.withProxyUrl('https://main--foo--bar.hlx.page/')
.withHttpPort(0);

nock('https://main--foo--bar.hlx.page')
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');

await project.init();
try {
await project.start();
Expand All @@ -124,10 +120,6 @@ describe('Helix Server', () => {
.withProxyUrl('https://main--foo--bar.hlx.page/')
.withHttpPort(0);

nock('https://main--foo--bar.hlx.page')
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');

await project.init();
try {
await project.start();
Expand Down Expand Up @@ -164,9 +156,7 @@ describe('Helix Server', () => {

nock('http://main--foo--bar.hlx.page')
.get('/notfound.css')
.reply(404)
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
.reply(404);

try {
await project.start();
Expand All @@ -188,9 +178,7 @@ describe('Helix Server', () => {
nock('http://main--foo--bar.hlx.page')
.get('/local.html')
.optionally(true)
.reply(200, 'foo')
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
.reply(200, 'foo');

try {
await project.start();
Expand Down Expand Up @@ -219,9 +207,7 @@ describe('Helix Server', () => {
.get('/missing')
.reply(404, 'server 404 html', {
'content-type': 'text/html',
})
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
});

try {
await project.start();
Expand Down Expand Up @@ -366,10 +352,6 @@ describe('Helix Server', () => {
// a request like: /subfolder/query-index.json?sheet=foo&limit=20?sheet=foo&limit=20
assert.strictEqual(uri, '/subfolder/query-index.json?sheet=foo&limit=20');
return [200, '{ "data": [] }', { 'content-type': 'application/json' }];
})
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>', {
'content-type': 'text/html',
});

try {
Expand Down
24 changes: 6 additions & 18 deletions test/up-cmd.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ describe('Integration test for up command with helix pages', function suite() {
.get('/index.html')
.reply(200, '## Welcome')
.get('/not-found.txt')
.reply(404)
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
.reply(404);

let port;
await new Promise((resolve, reject) => {
Expand Down Expand Up @@ -116,9 +114,7 @@ describe('Integration test for up command with helix pages', function suite() {
.get('/index.html')
.reply(200, '## Welcome')
.get('/not-found.txt')
.reply(404)
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
.reply(404);

cmd
.on('started', async () => {
Expand Down Expand Up @@ -158,9 +154,7 @@ describe('Integration test for up command with helix pages', function suite() {
.get('/index.html')
.reply(200, '## Welcome')
.get('/not-found.txt')
.reply(404)
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
.reply(404);

nock('https://master--dummy-foo--adobe.hlx.page')
.get('/fstab.yaml')
Expand Down Expand Up @@ -204,19 +198,15 @@ describe('Integration test for up command with helix pages', function suite() {
.get('/index.html')
.reply(200, '## Welcome')
.get('/not-found.txt')
.reply(404)
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
.reply(404);

nock('https://master--dummy-foo--adobe.hlx.page')
.get('/fstab.yaml')
.reply(404, 'dummy');

nock('https://new-branch--dummy-foo--adobe.hlx.page')
.get('/fstab.yaml')
.reply(200, 'yep!')
.get('/head.html')
.reply(200, '## Welcome');
.reply(200, 'yep!');

let timer;
cmd
Expand Down Expand Up @@ -261,9 +251,7 @@ describe('Integration test for up command with helix pages', function suite() {
.get('/index.html')
.reply(200, '## Welcome')
.get('/not-found.txt')
.reply(404)
.get('/head.html')
.reply(200, '<link rel="stylesheet" href="/styles.css"/>');
.reply(404);

nock('https://master--dummy-foo--adobe.hlx.page')
.get('/fstab.yaml')
Expand Down

0 comments on commit 926aff5

Please sign in to comment.