Repository: Redocly/redocly-cli
Pull request #1527 (2024-04-15 13:07:58)

fix: resolve config extensions using only one resolver
## What/Why/How?

Currently, when resolving the config, the external refs' resolver gets re-declared inside the `resolveConfig` function. That makes caching some external resources (particularly those from the `extends` config section) impossible. Also, those disconnected resolvers make it impossible to substitute the fs provider (it's useful when working in a browser environment).

This PR eliminates the discrepancy and allows using one `externalRefResolver` throughout the entire `loadConfig` function.

## Reference

## Testing

## Screenshots (optional)

## Check yourself

- [x] Code changed? - Tested with redoc/reference-docs/workflows (internal)
- [x] All new/updated code is covered with tests
- [ ] New package installed? - Tested in different environments (browser/node)

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :--------- | :-------------: |
|                                                                           🟡                                                                           | Statements | 76.84%     |    4415/5746    |
|                                                                           🟡                                                                           | Branches   | 67.07%     |    2399/3577    |
|                                                                           🟡                                                                           | Functions  | 70.58%     |     734/1040    |
|                                                                           🟡                                                                           | Lines      | 77%        |    4145/5383    |


## Test suite run success
721 tests passing in 101 suites.

<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from 12f4337a7b657274287c92834cbc0d951378fbb3</p>


Patch:
diff --git a/.changeset/many-steaks-hope.md b/.changeset/many-steaks-hope.md
new file mode 100644
index 00000000..6ef595cb
--- /dev/null
+++ b/.changeset/many-steaks-hope.md
@@ -0,0 +1,5 @@
+---
+"@redocly/openapi-core": patch
+---
+
+Improved caching for external configuration resources.
diff --git a/packages/core/src/bundle.ts b/packages/core/src/bundle.ts
index 0c80f6e7..78bd1471 100755
--- a/packages/core/src/bundle.ts
+++ b/packages/core/src/bundle.ts
@@ -232,7 +232,7 @@ export async function bundleDocument(opts: {
 
   walkDocument({
     document,
-    rootType: types.Root as NormalizedNodeType,
+    rootType: types.Root,
     normalizedVisitors: bundleVisitor,
     resolvedRefMap,
     ctx,
diff --git a/packages/core/src/config/config-resolvers.ts b/packages/core/src/config/config-resolvers.ts
index 9a512d81..257ac08f 100644
--- a/packages/core/src/config/config-resolvers.ts
+++ b/packages/core/src/config/config-resolvers.ts
@@ -12,7 +12,7 @@ import {
   transformConfig,
 } from './utils';
 import { isBrowser } from '../env';
-import { isNotString, isString, isDefined, parseYaml, keysOf } from '../utils';
+import { isNotString, isString, isDefined, keysOf } from '../utils';
 import { Config } from './config';
 import { colorize, logger } from '../logger';
 import { asserts, buildAssertCustomFunction } from '../rules/common/assertions/asserts';
@@ -63,14 +63,22 @@ export async function resolveConfigFileAndRefs({
   return { document, resolvedRefMap };
 }
 
-export async function resolveConfig(rawConfig: RawConfig, configPath?: string): Promise<Config> {
+export async function resolveConfig({
+  rawConfig,
+  configPath,
+  externalRefResolver,
+}: {
+  rawConfig: RawConfig;
+  configPath?: string;
+  externalRefResolver?: BaseResolver;
+}): Promise<Config> {
   if (rawConfig.styleguide?.extends?.some(isNotString)) {
     throw new Error(
       `Error configuration format not detected in extends value must contain strings`
     );
   }
 
-  const resolver = new BaseResolver(getResolveConfig(rawConfig.resolve));
+  const resolver = externalRefResolver ?? new BaseResolver(getResolveConfig(rawConfig.resolve));
 
   const apis = await resolveApis({
     rawConfig,
@@ -388,10 +396,8 @@ async function loadExtendStyleguideConfig(
   resolver: BaseResolver
 ): Promise<StyleguideRawConfig> {
   try {
-    const fileSource = await resolver.loadExternalRef(filePath);
-    const rawConfig = transformConfig(
-      parseYaml(fileSource.body) as RawConfig & DeprecatedInRawConfig
-    );
+    const { parsed } = (await resolver.resolveDocument(null, filePath)) as Document;
+    const rawConfig = transformConfig(parsed as RawConfig & DeprecatedInRawConfig);
     if (!rawConfig.styleguide) {
       throw new Error(`Styleguide configuration format not detected: "${filePath}"`);
     }
diff --git a/packages/core/src/config/load.ts b/packages/core/src/config/load.ts
index c7ba384f..2223ab40 100644
--- a/packages/core/src/config/load.ts
+++ b/packages/core/src/config/load.ts
@@ -20,6 +20,7 @@ async function addConfigMetadata({
   tokens,
   files,
   region,
+  externalRefResolver,
 }: {
   rawConfig: RawConfig;
   customExtends?: string[];
@@ -27,6 +28,7 @@ async function addConfigMetadata({
   tokens?: RegionalTokenWithValidity[];
   files?: string[];
   region?: Region;
+  externalRefResolver?: BaseResolver;
 }): Promise<Config> {
   if (customExtends !== undefined) {
     rawConfig.styleguide = rawConfig.styleguide || {};
@@ -64,10 +66,15 @@ async function addConfigMetadata({
     }
   }
 
-  return resolveConfig(
-    { ...rawConfig, files: files ?? rawConfig.files, region: region ?? rawConfig.region },
-    configPath
-  );
+  return resolveConfig({
+    rawConfig: {
+      ...rawConfig,
+      files: files ?? rawConfig.files,
+      region: region ?? rawConfig.region,
+    },
+    configPath,
+    externalRefResolver,
+  });
 }
 
 export type RawConfigProcessor = (
@@ -105,6 +112,7 @@ export async function loadConfig(
     tokens,
     files,
     region,
+    externalRefResolver,
   });
 }
 
@@ -156,6 +164,7 @@ type CreateConfigOptions = {
   extends?: string[];
   tokens?: RegionalTokenWithValidity[];
   configPath?: string;
+  externalRefResolver?: BaseResolver;
 };
 
 export async function createConfig(


Test patch:
diff --git a/packages/core/src/config/__tests__/config-resolvers.test.ts b/packages/core/src/config/__tests__/config-resolvers.test.ts
index 75680e6b..6ece8a00 100644
--- a/packages/core/src/config/__tests__/config-resolvers.test.ts
+++ b/packages/core/src/config/__tests__/config-resolvers.test.ts
@@ -390,7 +390,7 @@ describe('resolveConfig', () => {
       },
     };
 
-    const { apis } = await resolveConfig(rawConfig, configPath);
+    const { apis } = await resolveConfig({ rawConfig, configPath });
     //@ts-ignore
     expect(apis['petstore'].styleguide.plugins.length).toEqual(1);
     //@ts-ignore
@@ -427,7 +427,7 @@ describe('resolveConfig', () => {
       },
     };
 
-    const { apis } = await resolveConfig(rawConfig, configPath);
+    const { apis } = await resolveConfig({ rawConfig, configPath });
     expect(apis['petstore'].styleguide.rules).toBeDefined();
     expect(Object.keys(apis['petstore'].styleguide.rules || {}).length).toEqual(7);
     expect(apis['petstore'].styleguide.rules?.['operation-2xx-response']).toEqual('warn');
@@ -469,7 +469,7 @@ describe('resolveConfig', () => {
       },
     };
 
-    const { apis } = await resolveConfig(rawConfig, configPath);
+    const { apis } = await resolveConfig({ rawConfig, configPath });
     expect(apis['petstore'].styleguide.rules).toBeDefined();
     expect(apis['petstore'].styleguide.rules?.['operation-2xx-response']).toEqual('warn');
     expect(apis['petstore'].styleguide.rules?.['operation-4xx-response']).toEqual('error');
@@ -513,7 +513,7 @@ describe('resolveConfig', () => {
       },
     };
 
-    const { apis } = await resolveConfig(rawConfig, configPath);
+    const { apis } = await resolveConfig({ rawConfig, configPath });
     expect(apis['petstore'].styleguide.rules).toBeDefined();
     expect(apis['petstore'].styleguide.rules?.['operation-2xx-response']).toEqual('warn'); // from minimal ruleset
   });
diff --git a/packages/core/src/config/__tests__/fixtures/load-external.yaml b/packages/core/src/config/__tests__/fixtures/load-external.yaml
new file mode 100644
index 00000000..b51c1920
--- /dev/null
+++ b/packages/core/src/config/__tests__/fixtures/load-external.yaml
@@ -0,0 +1,2 @@
+extends:
+  - https://raw.githubusercontent.com/Redocly/redocly-cli-cookbook/main/rulesets/spec-compliant/redocly.yaml
diff --git a/packages/core/src/config/__tests__/load.test.ts b/packages/core/src/config/__tests__/load.test.ts
index b79bbd56..e1a456a8 100644
--- a/packages/core/src/config/__tests__/load.test.ts
+++ b/packages/core/src/config/__tests__/load.test.ts
@@ -5,6 +5,7 @@ import { lintConfig } from '../../lint';
 import { replaceSourceWithRef } from '../../../__tests__/utils';
 import type { RuleConfig, FlatRawConfig } from './../types';
 import type { NormalizedProblem } from '../../walk';
+import { BaseResolver } from '../../resolve';
 
 const fs = require('fs');
 const path = require('path');
@@ -58,6 +59,19 @@ describe('loadConfig', () => {
     });
     expect(mockFn).toHaveBeenCalled();
   });
+
+  it('should call externalRefResolver if such passed', async () => {
+    const externalRefResolver = new BaseResolver();
+    const resolverSpy = jest.spyOn(externalRefResolver, 'resolveDocument');
+    await loadConfig({
+      configPath: path.join(__dirname, './fixtures/load-external.yaml'),
+      externalRefResolver: externalRefResolver as any,
+    });
+    expect(resolverSpy).toHaveBeenCalledWith(
+      null,
+      'https://raw.githubusercontent.com/Redocly/redocly-cli-cookbook/main/rulesets/spec-compliant/redocly.yaml'
+    );
+  });
 });
 
 describe('findConfig', () => {