refactor(server): pf-3836 processDocsFunction, metadata, errors (#209)

* getResources, refactor processDocsFunction for metadata, errors
* helpers, new createError, isUrlObject, parseUrl

Author: cdcabrera

src/__tests__/__snapshots__/server.getResources.test.ts.snap

@@ -12,7 +12,19 @@ exports[`processDocsFunction should handle errors gracefully: errors 1`] = `
     "content": "❌ Failed to load bad-file.md: File not found",
     "isSuccess": false,
     "path": "bad-file.md",
-    "resolvedPath": undefined,
+    "resolvedPath": "/bad-file.md",
+  },
+]
+`;
+
+exports[`processDocsFunction should process local and remote inputs, de-duplicate with first metadata 1`] = `
+[
+  {
+    "content": "local file content",
+    "isSuccess": true,
+    "lorem": "ipsum",
+    "path": "file.md",
+    "resolvedPath": "/file.md",
   },
 ]
 `;
@@ -51,7 +63,7 @@ exports[`processDocsFunction should process local and remote inputs, files and U
 ]
 `;
 
-exports[`processDocsFunction should process local and remote inputs, filter empty strings 1`] = `
+exports[`processDocsFunction should process local and remote inputs, filter empty strings with varied input 1`] = `
 [
   {
     "content": "local file content",
@@ -68,23 +80,38 @@ exports[`processDocsFunction should process local and remote inputs, filter empt
 ]
 `;
 
+exports[`processDocsFunction should process local and remote inputs, metadata passthrough 1`] = `
+[
+  {
+    "content": "local file content",
+    "dolor": "sit",
+    "isSuccess": true,
+    "lorem": "ispum",
+    "path": "file.md",
+    "resolvedPath": "/file.md",
+  },
+]
+`;
+
 exports[`promiseQueue should execute promises in order: allSettled 1`] = `
 [
   {
     "status": "fulfilled",
     "value": {
       "content": "/dolor-sit.md",
+      "path": "dolor-sit.md",
       "resolvedPath": "/dolor-sit.md",
     },
   },
   {
-    "reason": "https://example.com/remote.md",
+    "reason": [Error: https://example.com/remote.md],
     "status": "rejected",
   },
   {
     "status": "fulfilled",
     "value": {
       "content": "/lorem-ipsum.md",
+      "path": "lorem-ipsum.md",
       "resolvedPath": "/lorem-ipsum.md",
     },
   },

src/__tests__/server.getResources.test.ts

@@ -320,6 +320,7 @@ describe('loadFileFetch', () => {
     expect(mockReadCall).toHaveBeenCalledTimes(expectedIsFetch ? 0 : 1);
     expect(result).toEqual({
       content: 'content',
+      path: expect.any(String),
       resolvedPath: expect.any(String)
     });
   });
@@ -374,15 +375,33 @@ describe('processDocsFunction', () => {
       fetchMemoHits: 1
     },
     {
-      description: 'filter empty strings',
+      description: 'filter empty strings with varied input',
       inputs: [
+        { doc: 'file.md' },
         'file.md',
         '',
         '   ',
         'file2.md'
       ],
       options: {},
       fileMemoHits: 2
+    },
+    {
+      description: 'de-duplicate with first metadata',
+      inputs: [
+        { doc: 'file.md', lorem: 'ipsum' },
+        { doc: 'file.md', dolor: 'sit' }
+      ],
+      options: {},
+      fileMemoHits: 1
+    },
+    {
+      description: 'metadata passthrough',
+      inputs: [
+        { doc: 'file.md', lorem: 'ispum', dolor: 'sit' }
+      ],
+      options: {},
+      fileMemoHits: 1
     }
   ])('should process local and remote inputs, $description', async ({ inputs, options, fileMemoHits = 0, fetchMemoHits = 0 }) => {
     const result = await processDocsFunction(inputs, {

src/__tests__/server.helpers.test.ts

@@ -1,5 +1,6 @@
 import {
   buildSearchString,
+  createError,
   freezeObject,
   generateHash,
   hashCode,
@@ -8,11 +9,13 @@ import {
   isPromise,
   isReferenceLike,
   isUrl,
+  isUrlObject,
   isPath,
   isWhitelistedUrl,
   listAllCombinations,
   listIncrementalCombinations,
   mergeObjects,
+  parseUrl,
   portValid,
   splitUri,
   stringJoin,
@@ -57,12 +60,66 @@ describe('buildSearchString', () => {
         lorem: 'ipsum dolor'
       },
       expected: 'lorem=ipsum+dolor'
+    },
+    {
+      description: 'prefix with base that has query',
+      values: {
+        lorem: 'ipsum'
+      },
+      options: {
+        prefix: true,
+        base: 'patternfly://docs?version=1'
+      },
+      expected: '&lorem=ipsum'
     }
   ])('should build a search string, $description', ({ values, options, expected }) => {
     expect(buildSearchString(values, options || {})).toBe(expected);
   });
 });
 
+describe('createError', () => {
+  it.each([
+    {
+      description: 'use an explicit message and metadata',
+      message: 'Custom error message',
+      metadata: { code: 404, details: 'Not found' },
+      expectedMessage: 'Custom error message',
+      expectedMetadata: { code: 404, details: 'Not found' }
+    },
+    {
+      description: 'use an Error object as message',
+      message: new Error('Original error message'),
+      metadata: { path: '/tmp' },
+      expectedMessage: 'Original error message',
+      expectedMetadata: { path: '/tmp' }
+    },
+    {
+      description: 'use cause message when message is undefined',
+      message: undefined,
+      options: { cause: new Error('Cause error') },
+      metadata: { retry: true },
+      expectedMessage: 'Cause error',
+      expectedMetadata: { retry: true }
+    },
+    {
+      description: 'use fallback message when nothing else is provided',
+      message: undefined,
+      metadata: {},
+      expectedMessage: 'An error occurred',
+      expectedMetadata: {}
+    }
+  ])('should create an error, $description', ({ message, options, metadata, expectedMessage, expectedMetadata }: any) => {
+    const err = createError(message, options || {}, metadata);
+
+    expect(err).toBeInstanceOf(Error);
+    expect(err.message).toBe(expectedMessage);
+    Object.entries(expectedMetadata).forEach(([key, value]) => {
+      expect((err as any)[key]).toBe(value);
+    });
+    expect(err.cause).toBe(options?.cause);
+  });
+});
+
 describe('freezeObject', () => {
   it.each([
     {
@@ -463,6 +520,55 @@ describe('isPromise', () => {
   });
 });
 
+describe('isUrlObject', () => {
+  it.each([
+    {
+      description: 'valid URL object with no protocol restrictions',
+      obj: new URL('https://patternfly.org'),
+      options: {},
+      expected: true
+    },
+    {
+      description: 'valid URL object with matching allowed protocol',
+      obj: new URL('patternfly://docs'),
+      options: { allowedProtocols: ['patternfly'] },
+      expected: true
+    },
+    {
+      description: 'valid URL object with non-matching allowed protocol',
+      obj: new URL('http://patternfly.org'),
+      options: { allowedProtocols: ['https'] },
+      expected: false
+    },
+    {
+      description: 'invalid input (string instead of URL object)',
+      obj: 'https://patternfly.org',
+      options: {},
+      expected: false
+    },
+    {
+      description: 'invalid input, null',
+      obj: null,
+      options: {},
+      expected: false
+    },
+    {
+      description: 'invalid input, undefined',
+      obj: undefined,
+      options: {},
+      expected: false
+    },
+    {
+      description: 'invalid input, empty string',
+      obj: ' ',
+      options: {},
+      expected: false
+    }
+  ])('should return $expected for $description', ({ obj, options, expected }) => {
+    expect(isUrlObject(obj, options)).toBe(expected);
+  });
+});
+
 describe('isPlainObject', () => {
   it.each([
     {
@@ -729,6 +835,11 @@ describe('isWhitelistedUrl', () => {
       description: 'invalid URL string',
       url: 'not-a-url',
       expected: false
+    },
+    {
+      description: 'reject encoded slash in pathname',
+      url: 'https://github.com/patternfly%2Fother-repo',
+      expected: false
     }
   ])('should confirm if a URL is whitelisted, $description', ({ url, expected }) => {
     const whitelist = [
@@ -950,6 +1061,84 @@ describe('portValid', () => {
   });
 });
 
+describe('parseUrl', () => {
+  it.each([
+    {
+      description: 'absolute URI without prefix',
+      uri: 'patternfly://docs/button?v=1',
+      options: { isStrict: false },
+      expected: {
+        protocol: 'patternfly:',
+        hostname: 'docs',
+        path: 'button',
+        params: { v: '1' }
+      }
+    },
+    {
+      description: 'relative URI with prefix',
+      uri: 'docs/button',
+      options: { prefix: 'patternfly', isStrict: false },
+      expected: {
+        protocol: 'patternfly:',
+        hostname: 'docs',
+        path: 'button',
+        params: {}
+      }
+    },
+    {
+      description: 'absolute URI with ignored prefix',
+      uri: 'https://google.com/search?q=test',
+      options: { prefix: 'patternfly://' },
+      expected: {
+        protocol: 'https:',
+        hostname: 'google.com',
+        path: 'search',
+        params: { q: 'test' }
+      }
+    },
+    {
+      description: 'invalid URI',
+      uri: 'not a uri',
+      expected: undefined
+    },
+    {
+      description: 'relative URI without prefix',
+      uri: 'docs/button',
+      expected: undefined
+    },
+    {
+      description: 'absolute URI without hostname',
+      uri: 'patternfly:docs/button',
+      options: { isStrict: false },
+      expected: {
+        protocol: 'patternfly:',
+        hostname: '',
+        path: 'docs/button',
+        params: {}
+      }
+    },
+    {
+      description: 'pass through a malformed percent-encoding in absolute URI',
+      uri: 'https://patternfly.org/docs/bad%zzpath',
+      options: {},
+      expected: {
+        protocol: 'https:',
+        hostname: 'patternfly.org',
+        path: 'docs/bad%zzpath',
+        params: {}
+      }
+    },
+    {
+      description: 'return undefined for malformed percent-encoding in prefixed path',
+      uri: 'docs/bad%zzpath',
+      options: { prefix: 'patternfly' },
+      expected: undefined
+    }
+  ])('should parse a URI, $description', ({ uri, options, expected }) => {
+    expect(parseUrl(uri, options)).toEqual(expected);
+  });
+});
+
 describe('splitUri', () => {
   it.each([
     {