Skip to content
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.

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

fix: apply folder-mapped metadata also for existing resources #765

Merged
merged 1 commit into from
Dec 4, 2024
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
19 changes: 18 additions & 1 deletion src/PipelineState.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/
import {PathInfo, S3Loader, PipelineTimer } from "./index";
import {PipelineContent} from "./PipelineContent";
import {PipelineSiteConfig} from "./site-config";
import {ModifiersSheet, PipelineSiteConfig} from "./site-config";

declare enum PipelineType {
html = 'html',
Expand Down Expand Up @@ -117,5 +117,22 @@ declare class PipelineState {
*/
liveHost: string;

/**
* specifies if the content as folder mapped (note that it remains false for content that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is folder mapped

* exists below a folder mapped path. in this case, the `mappedPath` would still be different
* from the `info.path`
*/
mapped: boolean;

/**
* the mapped path (target) of a folder mapping. this is set irrespective of the existence of the
* resource, when the path is below a folder mapped path
*/
mappedPath: string;

/**
* metadata from folder mapping
*/
mappedMetadata: Modifiers
}

6 changes: 3 additions & 3 deletions src/html-pipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import fetchContent from './steps/fetch-content.js';
import fetch404 from './steps/fetch-404.js';
import initConfig from './steps/init-config.js';
import fixSections from './steps/fix-sections.js';
import folderMapping from './steps/folder-mapping.js';
import { calculateFolderMapping, applyFolderMapping } from './steps/folder-mapping.js';
import getMetadata from './steps/get-metadata.js';
import html from './steps/make-html.js';
import parseMarkdown from './steps/parse-markdown.js';
Expand Down Expand Up @@ -110,7 +110,7 @@ export async function htmlPipe(state, req) {
state.content.sourceBus = 'code';
}

// ...and apply the folder mapping
calculateFolderMapping(state);
state.timer?.update('content-fetch');
let contentPromise = await fetchContent(state, req, res);
if (res.status === 404) {
Expand All @@ -119,7 +119,7 @@ export async function htmlPipe(state, req) {
contentPromise = fetchContentRedirectWith404Fallback(state, req, res);
} else {
// ...apply folder mapping if the current resource doesn't exist
await folderMapping(state);
applyFolderMapping(state);
if (state.info.unmappedPath) {
contentPromise = fetchContentWith404Fallback(state, req, res);
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/steps/extract-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export default function extractMetaData(state, req) {
// with local metadata from document
const metaConfig = Object.assign(
state.metadata.getModifiers(state.info.unmappedPath || state.info.path),
state.mappedMetadata.getModifiers(state.info.unmappedPath),
state.mappedMetadata.getModifiers(state.info.unmappedPath || state.info.path),
);

// prune empty values and explicit "" strings from sheet based metadata
Expand Down
5 changes: 3 additions & 2 deletions src/steps/fetch-mapped-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ import { Modifiers } from '../utils/modifiers.js';
// eslint-disable-next-line no-unused-vars
export default async function fetchMappedMetadata(state, res) {
state.mappedMetadata = Modifiers.EMPTY;
if (!state.mapped) {
const { mappedPath } = state;
if (!mappedPath) {
return;
}
const { contentBusId, partition } = state;
const metadataPath = `${state.info.path}/metadata.json`;
const metadataPath = `${mappedPath}/metadata.json`;
const key = `${contentBusId}/${partition}${metadataPath}`;
const ret = await state.s3Loader.getObject('helix-content-bus', key);
if (ret.status === 200) {
Expand Down
29 changes: 19 additions & 10 deletions src/steps/folder-mapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,35 @@ export function mapPath(folders, path) {
}

/**
* Checks the fstab for folder mapping entries and then re-adjusts the path infos if needed.
* if the remapped resource is *not* extensionless, it will be declared as code-bus resource.
* Checks if the resource path is below a folder-mapped configuration and updates `state.mappedPath`
* accordingly.
*
* @type PipelineStep
* @param {PipelineState} state
* @param state
*/
export default function folderMapping(state) {
export function calculateFolderMapping(state) {
const { folders } = state.config;
if (!folders) {
return;
}
const { path } = state.info;
const mapped = mapPath(folders, path);
if (mapped) {
state.info = getPathInfo(mapped);
state.mappedPath = mapPath(folders, path);
}

/**
* Applies folder mapping if the resource is mapped (i.e. if `state.mappedPath` is {@code true}.
*
* @type PipelineStep
* @param {PipelineState} state
*/
export function applyFolderMapping(state) {
const { info: { path }, mappedPath } = state;
if (mappedPath) {
state.info = getPathInfo(mappedPath);
state.info.unmappedPath = path;
if (getExtension(mapped)) {
if (getExtension(mappedPath)) {
// special case: use code-bus
state.content.sourceBus = 'code';
state.info.resourcePath = mapped;
state.info.resourcePath = mappedPath;
state.log.info(`mapped ${path} to ${state.info.resourcePath} (code-bus)`);
} else {
state.mapped = true;
Expand Down
10 changes: 10 additions & 0 deletions test/rendering.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@
}
const response = await render(url, '', expStatus, partition);
const actHtml = response.body;
console.log(actHtml);

Check warning on line 195 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 @@ -643,6 +643,16 @@
loader.status('product1.md', 200);
resp = await render(new URL('https://helix-pipeline.com/products/product1'), '', 200);
assert.match(resp.body, /<meta property="og:url" content="https:\/\/www.adobe.com\/products\/product1">/);
// folder mapped metadata is also applied.
assert.match(resp.body, /<meta name="keywords" content="Exactomento Mapped Folder">/);

assert.deepStrictEqual(Object.fromEntries(resp.headers.entries()), {
'access-control-allow-origin': '*',
'content-type': 'text/html; charset=utf-8',
'last-modified': 'Fri, 30 Apr 2021 03:47:18 GMT',
'x-surrogate-key': 'AkcHu8fRFT7HarTR foo-id_metadata super-test--helix-pages--adobe_head foo-id AkcHu8fRFT7HarTR_metadata z8NGXvKB0X5Fzcnd',
link: '</scripts/scripts.js>; rel=modulepreload; as=script; crossorigin=use-credentials',
});
});

it('renders 404', async () => {
Expand Down
5 changes: 5 additions & 0 deletions test/steps/fetch-mapped-metadata.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('Fetch Mapped Metadata', () => {
contentBusId: 'foo-id',
partition: 'live',
mapped: true,
mappedPath: '/mapped',
info: {
path: '/mapped',
},
Expand All @@ -44,6 +45,7 @@ describe('Fetch Mapped Metadata', () => {
contentBusId: 'foo-id',
partition: 'live',
mapped: true,
mappedPath: '/mapped',
info: {
path: '/mapped',
},
Expand All @@ -63,6 +65,7 @@ describe('Fetch Mapped Metadata', () => {
contentBusId: 'foo-id',
partition: 'live',
mapped: true,
mappedPath: '/mapped',
info: {
path: '/mapped',
},
Expand All @@ -81,6 +84,7 @@ describe('Fetch Mapped Metadata', () => {
log: console,
contentBusId: 'foo-id',
partition: 'live',
mappedPath: '/mapped',
mapped: true,
info: {
path: '/mapped',
Expand All @@ -102,6 +106,7 @@ describe('Fetch Mapped Metadata', () => {
contentBusId: 'foo-id',
partition: 'live',
mapped: true,
mappedPath: '/mapped',
info: {
path: '/mapped',
},
Expand Down
Loading