Skip to content

Commit ee5d73e

Browse files
authored
Merge pull request #689 from namecheap/feat/add_multibrand
feat: add brandId to router domains
2 parents b3ef827 + d0ebfff commit ee5d73e

File tree

10 files changed

+339
-34
lines changed

10 files changed

+339
-34
lines changed

ilc/server/registry/Registry.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,14 @@ module.exports = class Registry {
4747
return filteredData;
4848
};
4949

50-
this.getTemplate = async (templateName, { locale, forDomain } = {}) => {
50+
this.getTemplate = async (templateName, { locale, forDomain, routeKey } = {}) => {
5151
if (templateName === '500' && forDomain) {
5252
const routerDomains = await this.getRouterDomains();
5353
const redefined500 = routerDomains.data.find((item) => item.domainName === forDomain)?.template500;
5454
templateName = redefined500 || templateName;
5555
}
5656

57-
return await getTemplateMemoized(templateName, { locale, domain: forDomain });
57+
return await getTemplateMemoized(templateName, { locale, domain: forDomain, routeKey });
5858
};
5959
}
6060

@@ -113,7 +113,9 @@ module.exports = class Registry {
113113
}
114114
}
115115

116-
#getTemplate = async (templateName, { locale, domain }) => {
116+
// Note: `routeKey` is intentionally unused here — it serves as a cache key differentiator
117+
// in the memoization layer (via JSON.stringify of args) to prevent cache collisions across routes.
118+
#getTemplate = async (templateName, { locale, domain, routeKey: _routeKey }) => {
117119
if (!VALID_TEMPLATE_NAME.test(templateName)) {
118120
throw new ValidationRegistryError({
119121
message: `Invalid template name ${templateName}`,

ilc/server/routes/wildcardRequestHandlerFactory.spec.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,23 +280,30 @@ describe('wildcardRequestHandlerFactory', () => {
280280
});
281281

282282
describe('Request headers', () => {
283-
it('should set x-request-host and x-request-uri headers', async () => {
284-
const mockRequest = {
283+
function createMockRequest(url: string) {
284+
return {
285285
host: 'test.com',
286286
headers: {} as any,
287287
log: mockLogger,
288288
raw: {
289-
url: '/test-path',
289+
url,
290290
ilcState: {},
291291
},
292292
};
293+
}
293294

294-
const mockReply = {
295+
function createMockReply() {
296+
return {
295297
redirect: sinon.stub(),
296298
header: sinon.stub(),
297299
status: sinon.stub().returns({ send: sinon.stub() }),
298300
res: {},
299301
};
302+
}
303+
304+
it('should set x-request-host and x-request-uri headers', async () => {
305+
const mockRequest = createMockRequest('/test-path');
306+
const mockReply = createMockReply();
300307

301308
await wildcardRequestHandler.call({} as any, mockRequest as any, mockReply as any);
302309

@@ -519,7 +526,11 @@ describe('wildcardRequestHandlerFactory', () => {
519526
sinon.assert.calledWith(mockReply.status, 200);
520527
sinon.assert.calledOnce(sendStub);
521528
sinon.assert.calledWith(sendStub, templateContent);
522-
sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', { locale: 'en-US' });
529+
sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', {
530+
locale: 'en-US',
531+
forDomain: 'test.com',
532+
routeKey: '/test',
533+
});
523534
});
524535

525536
it('should pass locale to getTemplate when available', async () => {
@@ -542,7 +553,11 @@ describe('wildcardRequestHandlerFactory', () => {
542553

543554
await wildcardRequestHandler.call({} as any, mockRequest as any, mockReply as any);
544555

545-
sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', { locale: 'fr-FR' });
556+
sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', {
557+
locale: 'fr-FR',
558+
forDomain: 'test.com',
559+
routeKey: '/test',
560+
});
546561
});
547562
});
548563

ilc/server/routes/wildcardRequestHandlerFactory.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,12 @@ export function wildcardRequestHandlerFactory(
9797
const isRouteWithoutSlots = !Object.keys(route.slots).length;
9898
if (isRouteWithoutSlots) {
9999
const locale = req.raw.ilcState?.locale;
100-
const { data } = await registryService.getTemplate(route.template, { locale });
100+
const routeKey = route.route || `special:${route.specialRole}`;
101+
const { data } = await registryService.getTemplate(route.template, {
102+
locale,
103+
forDomain: currentDomain,
104+
routeKey,
105+
});
101106

102107
reply.header('Content-Type', 'text/html');
103108
reply.status(200).send(data.content);

ilc/server/tailor/fetch-template.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ module.exports = (configsInjector, newrelic, registryService) => async (request,
1616
const childTemplate = router.getFragmentsTpl(request.ilcState);
1717
const currRoute = router.getRoute();
1818

19-
const template = await registryService.getTemplate(currRoute.template);
19+
const forDomain = request.headers['x-request-host'];
20+
const routeKey = currRoute.route || `special:${currRoute.specialRole}`;
21+
const template = await registryService.getTemplate(currRoute.template, { forDomain, routeKey });
2022
if (template === undefined) {
2123
throw new Error("Can't match route base template to config map");
2224
}
2325

24-
const routeName = currRoute.route?.replace(/^\/(.+)/, '$1') || `special:${currRoute.specialRole}`;
26+
const routeName = routeKey.replace(/^\//, '');
2527
if (routeName) {
2628
newrelic.setTransactionName(routeName);
2729
}

ilc/server/tailor/fetch-template.spec.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,21 @@ describe('fetch templates', () => {
1212
setTransactionName: sinon.spy(),
1313
};
1414

15-
const registryService = {
16-
getTemplate: async (arg) => {
17-
const result = await arg;
15+
const defaultGetTemplate = async (arg) => {
16+
const result = await arg;
17+
return result;
18+
};
1819

19-
return result;
20-
},
20+
const registryService = {
21+
getTemplate: defaultGetTemplate,
2122
};
2223

2324
let currentRoute = {};
2425

2526
const request = {
27+
headers: {
28+
'x-request-host': 'test.com',
29+
},
2630
router: {
2731
getRoute: () => currentRoute,
2832
},
@@ -43,6 +47,7 @@ describe('fetch templates', () => {
4347
newrelic.setTransactionName.resetHistory();
4448
parseTemplate.reset();
4549
currentRoute = {};
50+
registryService.getTemplate = defaultGetTemplate;
4651
});
4752

4853
it('should throw Error if template is undefined', async () => {
@@ -57,17 +62,27 @@ describe('fetch templates', () => {
5762
currentRoute.template = 'exist';
5863
currentRoute.route = '/exist';
5964

65+
registryService.getTemplate = sinon.stub().resolves({ data: 'exist' });
6066
await fetchTemplateSetup(request, parseTemplate);
6167

68+
sinon.assert.calledOnceWithExactly(registryService.getTemplate, 'exist', {
69+
forDomain: 'test.com',
70+
routeKey: '/exist',
71+
});
6272
sinon.assert.calledOnceWithExactly(newrelic.setTransactionName, 'exist');
6373
});
6474

6575
it('should set transaction name in newrelic if there is no route', async () => {
6676
currentRoute.template = 'exist';
6777
currentRoute.specialRole = 'exist';
6878

79+
registryService.getTemplate = sinon.stub().resolves({ data: 'exist' });
6980
await fetchTemplateSetup(request, parseTemplate);
7081

82+
sinon.assert.calledOnceWithExactly(registryService.getTemplate, 'exist', {
83+
forDomain: 'test.com',
84+
routeKey: 'special:exist',
85+
});
7186
sinon.assert.calledOnceWithExactly(newrelic.setTransactionName, 'special:exist');
7287
});
7388

ilc/server/types/Registry.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ interface RegistryOptions {
2222
interface TemplateOptions {
2323
locale?: string;
2424
forDomain?: string;
25+
routeKey?: string;
2526
}
2627

2728
interface Template {

registry/server/templates/routes/getRenderedTemplate.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { templateNameSchema } from './validation';
1313
import RouterDomains from '../../routerDomains/interfaces';
1414
import { getLogger } from '../../util/logger';
1515
import { appendDigest } from '../../util/hmac';
16+
import { JSONValue, parseJSON } from '../../common/services/json';
1617

1718
type GetTemplateRenderedRequestParams = {
1819
name: string;
@@ -27,21 +28,21 @@ const validateRequestBeforeGetTemplateRendered = validateRequestFactory([
2728
},
2829
]);
2930

30-
async function getTemplateByDomain(domain: string, templateName: string): Promise<Template | null> {
31+
async function getDomainByName(domain: string): Promise<RouterDomains | undefined> {
3132
const [domainItem] = await db
32-
.select('id')
33+
.select('id', 'props')
3334
.from<RouterDomains>('router_domains')
3435
.where('domainName', String(domain));
3536

36-
if (!domainItem) {
37-
return null;
38-
}
37+
return domainItem;
38+
}
3939

40+
async function getTemplateByDomainId(domainId: number, templateName: string): Promise<Template | undefined> {
4041
const [template] = await db
4142
.selectVersionedRowsFrom<Template>(Tables.Templates, 'name', EntityTypes.templates, [`${Tables.Templates}.*`])
4243
.join('routes', 'templates.name', 'routes.templateName')
4344
.where({
44-
domainId: domainItem.id,
45+
domainId,
4546
name: templateName,
4647
});
4748

@@ -65,19 +66,27 @@ async function getTemplateByName(templateName: string): Promise<Template | undef
6566
}
6667

6768
async function getRenderedTemplate(req: Request<GetTemplateRenderedRequestParams>, res: Response): Promise<void> {
68-
let template;
69+
let template: Template | undefined;
70+
let brandId: string | undefined;
6971

7072
const { name: templateName } = req.params;
71-
72-
const { locale, domain } = req.query;
73+
const locale = req.query.locale as string;
74+
const domain = req.query.domain as string;
7375

7476
if (domain) {
75-
template = await getTemplateByDomain(String(domain), templateName);
77+
const domainItem = await getDomainByName(domain);
78+
if (domainItem) {
79+
const domainProps = domainItem.props ? parseJSON<Record<string, JSONValue>>(domainItem.props) : null;
80+
brandId = typeof domainProps?.brandId === 'string' ? domainProps.brandId : undefined;
81+
template = await getTemplateByDomainId(domainItem.id, templateName);
82+
}
7683
}
7784

7885
if (!template) {
7986
template = await getTemplateByName(templateName);
80-
template && getLogger().info(`Template ${templateName} is not attached to the domain, found by template name.`);
87+
if (template) {
88+
getLogger().info(`Template ${templateName} is not attached to the domain, found by template name.`);
89+
}
8190
}
8291

8392
if (!template) {
@@ -91,15 +100,15 @@ async function getRenderedTemplate(req: Request<GetTemplateRenderedRequestParams
91100
.select()
92101
.from<LocalizedTemplateRow>(Tables.TemplatesLocalized)
93102
.where('templateName', templateName)
94-
.andWhere('locale', locale as string);
103+
.andWhere('locale', locale);
95104

96105
if (localizedTemplate) {
97106
content = localizedTemplate.content;
98107
}
99108
}
100109

101110
try {
102-
const renderedTemplate = await renderTemplate(content);
111+
const renderedTemplate = await renderTemplate(content, brandId ? { brandId } : undefined);
103112
res.status(200).send({ ...template, ...renderedTemplate });
104113
} catch (e) {
105114
if (e instanceof errors.FetchIncludeError) {

registry/server/templates/services/renderTemplate.spec.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('renderTemplate', () => {
6767
attributes: {
6868
id: 'include-id-2',
6969
src: `${includesHost}/get/include/2`,
70-
timeout: 100,
70+
timeout: 300,
7171
},
7272
},
7373
{
@@ -318,6 +318,66 @@ describe('renderTemplate', () => {
318318
);
319319
});
320320

321+
it('should send x-ilc-request-brand header in include fetch when brandId context is provided', async () => {
322+
const include = {
323+
api: {
324+
route: '/get/include/brand',
325+
delay: 0,
326+
response: {
327+
status: 200,
328+
data: '<div>Brand include</div>',
329+
headers: {},
330+
},
331+
},
332+
attributes: {
333+
id: 'brand-include',
334+
src: `${includesHost}/get/include/brand`,
335+
timeout: 100,
336+
},
337+
};
338+
339+
scope
340+
.get(include.api.route)
341+
.matchHeader('x-ilc-request-brand', 'spaceship')
342+
.reply(include.api.response.status, include.api.response.data, include.api.response.headers);
343+
344+
const template = `<html><head></head><body><include id="${include.attributes.id}" src="${include.attributes.src}" timeout="${include.attributes.timeout}" /></body></html>`;
345+
346+
const renderedTemplate = await renderTemplate(template, { brandId: 'spaceship' });
347+
348+
chai.expect(renderedTemplate.content).to.contain('Brand include');
349+
});
350+
351+
it('should not send x-ilc-request-brand header when no brandId context is provided', async () => {
352+
const include = {
353+
api: {
354+
route: '/get/include/nobrand',
355+
delay: 0,
356+
response: {
357+
status: 200,
358+
data: '<div>No brand include</div>',
359+
headers: {},
360+
},
361+
},
362+
attributes: {
363+
id: 'nobrand-include',
364+
src: `${includesHost}/get/include/nobrand`,
365+
timeout: 100,
366+
},
367+
};
368+
369+
scope.get(include.api.route).reply(function (_uri, _body) {
370+
chai.expect(this.req.headers['x-ilc-request-brand']).to.be.undefined;
371+
return [include.api.response.status, include.api.response.data, include.api.response.headers];
372+
});
373+
374+
const template = `<html><head></head><body><include id="${include.attributes.id}" src="${include.attributes.src}" timeout="${include.attributes.timeout}" /></body></html>`;
375+
376+
const renderedTemplate = await renderTemplate(template);
377+
378+
chai.expect(renderedTemplate.content).to.contain('No brand include');
379+
});
380+
321381
it('should throw an error when a template has duplicate includes sources or ids', async () => {
322382
let catchedError;
323383

0 commit comments

Comments
 (0)