Skip to content

Commit 6b0cc35

Browse files
pmialansemenov
pmi
authored and
alansemenov
committed
Content path with special chars breaks preview #982 
1 parent 1b8e367 commit 6b0cc35

File tree

3 files changed

+133
-92
lines changed

3 files changed

+133
-92
lines changed

‎src/guillotine/fetchContentPathsForLocale.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@ export async function fetchContentPathsForLocale(
1212
query: string = GET_STATIC_PATHS_QUERY,
1313
count = 999
1414
): Promise<ContentPathItem[]> {
15+
const decodedPath = decodeURIComponent(path);
1516
const contentApiUrl = getContentApiUrl({
16-
contentPath: path,
17+
contentPath: decodedPath,
1718
locale: config.locale
1819
});
1920
const body: ContentApiBaseBody = {
2021
query,
2122
variables: {
22-
path,
23+
path: decodedPath,
2324
pathRegex: `/content${config.site}/*`,
2425
count
2526
}

‎src/guillotine/getCleanContentPathArrayOrThrow400.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ export const getCleanContentPathArrayOrThrow400 = (contentPath: string | string[
1414
}));
1515
}
1616

17-
return contentPath;
17+
return decodeURIComponent(contentPath);
1818

1919
} else {
20-
return (contentPath).join('/');
20+
return decodeURIComponent((contentPath).join('/'));
2121
}
2222
};

‎test/guillotine/fetchContent.test.ts‎

Lines changed: 128 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ globalThis.console = {
2828
error: jest.fn(),
2929
// warn: jest.fn(),
3030
log: jest.fn(),
31-
info: jest.fn(),
31+
info: jest.fn()
3232
// debug: jest.fn()
3333
} as Console;
3434

@@ -72,9 +72,9 @@ const PAGE_COMPONENT: RecursivePartial<Component> = {
7272
'com-example-myproject': {
7373
main: {}
7474
}
75-
},
75+
}
7676
// template: null
77-
},
77+
}
7878
// layout: null,
7979
// text: null,
8080
// part: null,
@@ -344,13 +344,13 @@ function mockHeadersImport(draft: boolean, mode: string) {
344344
}
345345

346346
function mockFetch({
347-
contentJson,
348-
contentOk = true,
349-
contentStatus = 200,
350-
metaJson,
351-
metaOk = true,
347+
contentJson,
348+
contentOk = true,
349+
contentStatus = 200,
350+
metaJson,
351+
metaOk = true,
352352
metaStatus = 200
353-
}: {
353+
}: {
354354
contentJson: GuillotineResponseJson
355355
contentOk?: boolean
356356
contentStatus?: number
@@ -423,6 +423,7 @@ describe('guillotine', () => {
423423

424424
await import('../../src/server').then(async (moduleName) => {
425425
const context: Context = {
426+
contentPath: 'some/path',
426427
headers: {
427428
get(name: string) {
428429
if (name === 'content-studio-mode') {
@@ -462,7 +463,7 @@ describe('guillotine', () => {
462463
catchAll: false,
463464
defaultLocale: 'en',
464465
locale: 'no',
465-
path: '',
466+
path: 'some/path',
466467
renderMode: mode,
467468
requestType: 'type',
468469
type: 'type'
@@ -486,7 +487,7 @@ describe('guillotine', () => {
486487
metaJson: GUILLOTINE_RESULT_META_MINIMAL
487488
});
488489
const context: Context = {
489-
contentPath: '/content/path',
490+
contentPath: '/content/path'
490491
// headers
491492
};
492493
import('../../src/guillotine/fetchContent').then(({fetchContent}) => {
@@ -716,14 +717,15 @@ describe('guillotine', () => {
716717
}); // import
717718
}); // it
718719

719-
it('changes requestType to page for fetchContentData when requestType is not component, but the meta response has components', () => {
720-
mockFetch({
721-
contentJson: GUILLOTINE_RESULT_CONTENT_WITH_ARTICLES,
722-
metaJson: GUILLOTINE_RESULT_META_FOR_ARTICLES
723-
});
724-
const CONTENT_PATH = 'articles'; // Must be present in contentJson
725-
const BASE_URL = '/base/url';
726-
const QUERY_COMMON = `query($path:ID!){
720+
it('changes requestType to page for fetchContentData when requestType is not component, but the meta response has components',
721+
() => {
722+
mockFetch({
723+
contentJson: GUILLOTINE_RESULT_CONTENT_WITH_ARTICLES,
724+
metaJson: GUILLOTINE_RESULT_META_FOR_ARTICLES
725+
});
726+
const CONTENT_PATH = 'articles'; // Must be present in contentJson
727+
const BASE_URL = '/base/url';
728+
const QUERY_COMMON = `query($path:ID!){
727729
guillotine {
728730
get(key:$path) {
729731
displayName
@@ -738,89 +740,127 @@ describe('guillotine', () => {
738740
}
739741
}
740742
}`;
741-
Promise.all([import('../../src'), import('../../src/server')]).then(async ([{ComponentRegistry}, {fetchContent}]) => {
742-
ComponentRegistry.setCommonQuery(QUERY_COMMON);
743-
ComponentRegistry.addContentType(CATCH_ALL, {
744-
configQuery: '{catchAll contentType configQuery}',
745-
query: 'query { guillotine { catchAll } }',
746-
view: () => {
747-
return 'catchAll contentType';
748-
}
749-
});
750-
ComponentRegistry.addLayout(CATCH_ALL, {
751-
configQuery: '{catchAll layout configQuery}',
752-
query: '{catchAll layout query}',
753-
view: () => {
754-
return 'catchAll layout';
755-
}
756-
});
757-
ComponentRegistry.addMacro(CATCH_ALL, {
758-
configQuery: '{catchAll macro configQuery}',
759-
query: '{catchAll macro query}',
760-
view: () => {
761-
return 'catchAll macro';
762-
}
763-
});
764-
ComponentRegistry.addPage(CATCH_ALL, {
765-
configQuery: '{catchAll page configQuery}',
766-
query: '{catchAll page query}',
767-
view: () => {
768-
return 'catchAll page';
769-
}
770-
});
771-
ComponentRegistry.addPart(CATCH_ALL, {
772-
configQuery: '{catchAll part configQuery}',
773-
query: '{catchAll part query}',
774-
view: () => {
775-
return 'catchAll part';
776-
}
777-
});
743+
Promise.all([import('../../src'), import('../../src/server')]).then(async ([{ComponentRegistry}, {fetchContent}]) => {
744+
ComponentRegistry.setCommonQuery(QUERY_COMMON);
745+
ComponentRegistry.addContentType(CATCH_ALL, {
746+
configQuery: '{catchAll contentType configQuery}',
747+
query: 'query { guillotine { catchAll } }',
748+
view: () => {
749+
return 'catchAll contentType';
750+
}
751+
});
752+
ComponentRegistry.addLayout(CATCH_ALL, {
753+
configQuery: '{catchAll layout configQuery}',
754+
query: '{catchAll layout query}',
755+
view: () => {
756+
return 'catchAll layout';
757+
}
758+
});
759+
ComponentRegistry.addMacro(CATCH_ALL, {
760+
configQuery: '{catchAll macro configQuery}',
761+
query: '{catchAll macro query}',
762+
view: () => {
763+
return 'catchAll macro';
764+
}
765+
});
766+
ComponentRegistry.addPage(CATCH_ALL, {
767+
configQuery: '{catchAll page configQuery}',
768+
query: '{catchAll page query}',
769+
view: () => {
770+
return 'catchAll page';
771+
}
772+
});
773+
ComponentRegistry.addPart(CATCH_ALL, {
774+
configQuery: '{catchAll part configQuery}',
775+
query: '{catchAll part query}',
776+
view: () => {
777+
return 'catchAll part';
778+
}
779+
});
780+
const context: Context = {
781+
contentPath: CONTENT_PATH, // Anything but _/component
782+
headers: {
783+
get(name: string) {
784+
if (name === RENDER_MODE_HEADER) {
785+
return RENDER_MODE.NEXT
786+
}
787+
if (name === PROJECT_ID_HEADER) {
788+
return 'project';
789+
}
790+
if (name === JSESSIONID_HEADER) {
791+
return '1234';
792+
}
793+
if (name === XP_BASE_URL_HEADER) {
794+
return BASE_URL;
795+
}
796+
console.debug('headers get name', name);
797+
return '';
798+
}
799+
} as Context['headers']
800+
};
801+
const promise = fetchContent(context);
802+
expect(promise).resolves.toStrictEqual({
803+
common: {},
804+
data: {
805+
get: {
806+
children: [{
807+
_path: 'articles'
808+
}]
809+
}
810+
},
811+
meta: {
812+
id: 'f5815ec2-26e0-4595-b37b-c2ba0d3c1e1c',
813+
apiUrl: 'http://localhost:8080/site/project/master',
814+
baseUrl: BASE_URL,
815+
canRender: true,
816+
catchAll: false,
817+
defaultLocale: 'en',
818+
locale: 'en',
819+
path: CONTENT_PATH,
820+
requestType: XP_REQUEST_TYPE.PAGE, // Changed internally from type to page
821+
renderMode: 'next',
822+
type: 'portal:site'
823+
},
824+
page: PAGE_COMPONENT
825+
}); // expect
826+
}); // import
827+
}); // it
828+
829+
it('handles fetchContent with contentPath containing encoded chars', () => {
830+
831+
mockFetch({
832+
contentJson: GUILLOTINE_RESULT_CONTENT,
833+
metaJson: GUILLOTINE_RESULT_META
834+
})
835+
836+
const BASE_URL = '/base/url';
837+
import('../../src/guillotine/fetchContent').then(async (moduleName) => {
838+
839+
const unescapedPath = 'path/with speciål+chars_and%percent!';
778840
const context: Context = {
779-
contentPath: CONTENT_PATH, // Anything but _/component
841+
contentPath: encodeURIComponent(unescapedPath),
780842
headers: {
781843
get(name: string) {
782844
if (name === RENDER_MODE_HEADER) {
783-
return RENDER_MODE.NEXT
845+
return 'next';
784846
}
785847
if (name === PROJECT_ID_HEADER) {
786-
return 'project';
848+
return 'prosjekt'; // Affects locale
787849
}
788850
if (name === JSESSIONID_HEADER) {
789851
return '1234';
790852
}
791853
if (name === XP_BASE_URL_HEADER) {
792854
return BASE_URL;
793855
}
794-
console.debug('headers get name', name);
795-
return '';
796-
}
797-
} as Context['headers']
798-
};
799-
const promise = fetchContent(context);
800-
expect(promise).resolves.toStrictEqual({
801-
common: {},
802-
data: {
803-
get: {
804-
children: [{
805-
_path: 'articles'
806-
}]
856+
console.error('headers get name', name);
807857
}
808-
},
809-
meta: {
810-
id: 'f5815ec2-26e0-4595-b37b-c2ba0d3c1e1c',
811-
apiUrl: 'http://localhost:8080/site/project/master',
812-
baseUrl: BASE_URL,
813-
canRender: true,
814-
catchAll: false,
815-
defaultLocale: 'en',
816-
locale: 'en',
817-
path: CONTENT_PATH,
818-
requestType: XP_REQUEST_TYPE.PAGE, // Changed internally from type to page
819-
renderMode: 'next',
820-
type: 'portal:site'
821-
},
822-
page: PAGE_COMPONENT
823-
}); // expect
858+
}
859+
} as Context;
860+
861+
const result = await moduleName.fetchContent(context);
862+
expect(result.meta.path).toEqual(unescapedPath);
863+
824864
}); // import
825865
}); // it
826866

0 commit comments

Comments
 (0)