Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions ilc/server/registry/Registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ module.exports = class Registry {
return filteredData;
};

this.getTemplate = async (templateName, { locale, forDomain } = {}) => {
this.getTemplate = async (templateName, { locale, forDomain, routeKey } = {}) => {
if (templateName === '500' && forDomain) {
const routerDomains = await this.getRouterDomains();
const redefined500 = routerDomains.data.find((item) => item.domainName === forDomain)?.template500;
templateName = redefined500 || templateName;
}

return await getTemplateMemoized(templateName, { locale, domain: forDomain });
return await getTemplateMemoized(templateName, { locale, domain: forDomain, routeKey });
};
}

Expand Down Expand Up @@ -113,7 +113,9 @@ module.exports = class Registry {
}
}

#getTemplate = async (templateName, { locale, domain }) => {
// Note: `routeKey` is intentionally unused here — it serves as a cache key differentiator
// in the memoization layer (via JSON.stringify of args) to prevent cache collisions across routes.
#getTemplate = async (templateName, { locale, domain, routeKey: _routeKey }) => {
if (!VALID_TEMPLATE_NAME.test(templateName)) {
throw new ValidationRegistryError({
message: `Invalid template name ${templateName}`,
Expand Down
27 changes: 21 additions & 6 deletions ilc/server/routes/wildcardRequestHandlerFactory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,23 +280,30 @@ describe('wildcardRequestHandlerFactory', () => {
});

describe('Request headers', () => {
it('should set x-request-host and x-request-uri headers', async () => {
const mockRequest = {
function createMockRequest(url: string) {
return {
host: 'test.com',
headers: {} as any,
log: mockLogger,
raw: {
url: '/test-path',
url,
ilcState: {},
},
};
}

const mockReply = {
function createMockReply() {
return {
redirect: sinon.stub(),
header: sinon.stub(),
status: sinon.stub().returns({ send: sinon.stub() }),
res: {},
};
}

it('should set x-request-host and x-request-uri headers', async () => {
const mockRequest = createMockRequest('/test-path');
const mockReply = createMockReply();

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

Expand Down Expand Up @@ -519,7 +526,11 @@ describe('wildcardRequestHandlerFactory', () => {
sinon.assert.calledWith(mockReply.status, 200);
sinon.assert.calledOnce(sendStub);
sinon.assert.calledWith(sendStub, templateContent);
sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', { locale: 'en-US' });
sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', {
locale: 'en-US',
forDomain: 'test.com',
routeKey: '/test',
});
});

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

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

sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', { locale: 'fr-FR' });
sinon.assert.calledWith(mockRegistryService.getTemplate, 'test-template', {
locale: 'fr-FR',
forDomain: 'test.com',
routeKey: '/test',
});
});
});

Expand Down
7 changes: 6 additions & 1 deletion ilc/server/routes/wildcardRequestHandlerFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,12 @@ export function wildcardRequestHandlerFactory(
const isRouteWithoutSlots = !Object.keys(route.slots).length;
if (isRouteWithoutSlots) {
const locale = req.raw.ilcState?.locale;
const { data } = await registryService.getTemplate(route.template, { locale });
const routeKey = route.route || `special:${route.specialRole}`;
const { data } = await registryService.getTemplate(route.template, {
locale,
forDomain: currentDomain,
routeKey,
});

reply.header('Content-Type', 'text/html');
reply.status(200).send(data.content);
Expand Down
6 changes: 4 additions & 2 deletions ilc/server/tailor/fetch-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ module.exports = (configsInjector, newrelic, registryService) => async (request,
const childTemplate = router.getFragmentsTpl(request.ilcState);
const currRoute = router.getRoute();

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

const routeName = currRoute.route?.replace(/^\/(.+)/, '$1') || `special:${currRoute.specialRole}`;
const routeName = routeKey.replace(/^\//, '');
if (routeName) {
newrelic.setTransactionName(routeName);
}
Expand Down
25 changes: 20 additions & 5 deletions ilc/server/tailor/fetch-template.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,21 @@ describe('fetch templates', () => {
setTransactionName: sinon.spy(),
};

const registryService = {
getTemplate: async (arg) => {
const result = await arg;
const defaultGetTemplate = async (arg) => {
const result = await arg;
return result;
};

return result;
},
const registryService = {
getTemplate: defaultGetTemplate,
};

let currentRoute = {};

const request = {
headers: {
'x-request-host': 'test.com',
},
router: {
getRoute: () => currentRoute,
},
Expand All @@ -43,6 +47,7 @@ describe('fetch templates', () => {
newrelic.setTransactionName.resetHistory();
parseTemplate.reset();
currentRoute = {};
registryService.getTemplate = defaultGetTemplate;
});

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

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

sinon.assert.calledOnceWithExactly(registryService.getTemplate, 'exist', {
forDomain: 'test.com',
routeKey: '/exist',
});
sinon.assert.calledOnceWithExactly(newrelic.setTransactionName, 'exist');
});

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

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

sinon.assert.calledOnceWithExactly(registryService.getTemplate, 'exist', {
forDomain: 'test.com',
routeKey: 'special:exist',
});
sinon.assert.calledOnceWithExactly(newrelic.setTransactionName, 'special:exist');
});

Expand Down
1 change: 1 addition & 0 deletions ilc/server/types/Registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ interface RegistryOptions {
interface TemplateOptions {
locale?: string;
forDomain?: string;
routeKey?: string;
}

interface Template {
Expand Down
35 changes: 22 additions & 13 deletions registry/server/templates/routes/getRenderedTemplate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { templateNameSchema } from './validation';
import RouterDomains from '../../routerDomains/interfaces';
import { getLogger } from '../../util/logger';
import { appendDigest } from '../../util/hmac';
import { JSONValue, parseJSON } from '../../common/services/json';

type GetTemplateRenderedRequestParams = {
name: string;
Expand All @@ -27,21 +28,21 @@ const validateRequestBeforeGetTemplateRendered = validateRequestFactory([
},
]);

async function getTemplateByDomain(domain: string, templateName: string): Promise<Template | null> {
async function getDomainByName(domain: string): Promise<RouterDomains | undefined> {
const [domainItem] = await db
.select('id')
.select('id', 'props')
.from<RouterDomains>('router_domains')
.where('domainName', String(domain));

if (!domainItem) {
return null;
}
return domainItem;
}

async function getTemplateByDomainId(domainId: number, templateName: string): Promise<Template | undefined> {
const [template] = await db
.selectVersionedRowsFrom<Template>(Tables.Templates, 'name', EntityTypes.templates, [`${Tables.Templates}.*`])
.join('routes', 'templates.name', 'routes.templateName')
.where({
domainId: domainItem.id,
domainId,
name: templateName,
});

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

async function getRenderedTemplate(req: Request<GetTemplateRenderedRequestParams>, res: Response): Promise<void> {
let template;
let template: Template | undefined;
let brandId: string | undefined;

const { name: templateName } = req.params;

const { locale, domain } = req.query;
const locale = req.query.locale as string;
const domain = req.query.domain as string;

if (domain) {
template = await getTemplateByDomain(String(domain), templateName);
const domainItem = await getDomainByName(domain);
if (domainItem) {
const domainProps = domainItem.props ? parseJSON<Record<string, JSONValue>>(domainItem.props) : null;
brandId = typeof domainProps?.brandId === 'string' ? domainProps.brandId : undefined;
template = await getTemplateByDomainId(domainItem.id, templateName);
}
}

if (!template) {
template = await getTemplateByName(templateName);
template && getLogger().info(`Template ${templateName} is not attached to the domain, found by template name.`);
if (template) {
getLogger().info(`Template ${templateName} is not attached to the domain, found by template name.`);
}
}

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

if (localizedTemplate) {
content = localizedTemplate.content;
}
}

try {
const renderedTemplate = await renderTemplate(content);
const renderedTemplate = await renderTemplate(content, brandId ? { brandId } : undefined);
res.status(200).send({ ...template, ...renderedTemplate });
} catch (e) {
if (e instanceof errors.FetchIncludeError) {
Expand Down
62 changes: 61 additions & 1 deletion registry/server/templates/services/renderTemplate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('renderTemplate', () => {
attributes: {
id: 'include-id-2',
src: `${includesHost}/get/include/2`,
timeout: 100,
timeout: 300,
},
},
{
Expand Down Expand Up @@ -318,6 +318,66 @@ describe('renderTemplate', () => {
);
});

it('should send x-ilc-request-brand header in include fetch when brandId context is provided', async () => {
const include = {
api: {
route: '/get/include/brand',
delay: 0,
response: {
status: 200,
data: '<div>Brand include</div>',
headers: {},
},
},
attributes: {
id: 'brand-include',
src: `${includesHost}/get/include/brand`,
timeout: 100,
},
};

scope
.get(include.api.route)
.matchHeader('x-ilc-request-brand', 'spaceship')
.reply(include.api.response.status, include.api.response.data, include.api.response.headers);

const template = `<html><head></head><body><include id="${include.attributes.id}" src="${include.attributes.src}" timeout="${include.attributes.timeout}" /></body></html>`;

const renderedTemplate = await renderTemplate(template, { brandId: 'spaceship' });

chai.expect(renderedTemplate.content).to.contain('Brand include');
});

it('should not send x-ilc-request-brand header when no brandId context is provided', async () => {
const include = {
api: {
route: '/get/include/nobrand',
delay: 0,
response: {
status: 200,
data: '<div>No brand include</div>',
headers: {},
},
},
attributes: {
id: 'nobrand-include',
src: `${includesHost}/get/include/nobrand`,
timeout: 100,
},
};

scope.get(include.api.route).reply(function (_uri, _body) {
chai.expect(this.req.headers['x-ilc-request-brand']).to.be.undefined;
return [include.api.response.status, include.api.response.data, include.api.response.headers];
});

const template = `<html><head></head><body><include id="${include.attributes.id}" src="${include.attributes.src}" timeout="${include.attributes.timeout}" /></body></html>`;

const renderedTemplate = await renderTemplate(template);

chai.expect(renderedTemplate.content).to.contain('No brand include');
});

it('should throw an error when a template has duplicate includes sources or ids', async () => {
let catchedError;

Expand Down
Loading
Loading