From 48cc7c5245eb5ceed33456d730622a58c5a87127 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Fri, 29 Nov 2024 16:17:22 +0000 Subject: [PATCH] yarn-plugin: use yarn's httpUtils to fetch release manifests This allows us to leverage yarn's built in proxy configuration when making the request. The yarn HTTP utilities handle caching of GET requests, so we can remove our memoization of the request, too. Signed-off-by: MT Lewis --- .changeset/beige-chicken-type.md | 5 ++ packages/yarn-plugin/package.json | 1 - .../handlers/beforeWorkspacePacking.test.ts | 49 ++++++----- .../src/handlers/beforeWorkspacePacking.ts | 1 + .../src/resolver/BackstageResolver.test.ts | 82 +++++++++---------- .../src/resolver/BackstageResolver.ts | 18 +++- packages/yarn-plugin/src/util.ts | 51 ++++++++++-- yarn.lock | 1 - 8 files changed, 130 insertions(+), 78 deletions(-) create mode 100644 .changeset/beige-chicken-type.md diff --git a/.changeset/beige-chicken-type.md b/.changeset/beige-chicken-type.md new file mode 100644 index 0000000000..d73500a7f9 --- /dev/null +++ b/.changeset/beige-chicken-type.md @@ -0,0 +1,5 @@ +--- +'yarn-plugin-backstage': patch +--- + +Use yarn's built-in http utilities for fetching release manifests diff --git a/packages/yarn-plugin/package.json b/packages/yarn-plugin/package.json index 0cc923a993..48af10b9da 100644 --- a/packages/yarn-plugin/package.json +++ b/packages/yarn-plugin/package.json @@ -35,7 +35,6 @@ "@yarnpkg/core": "^4.0.3", "@yarnpkg/fslib": "^3.0.2", "@yarnpkg/plugin-pack": "^4.0.0", - "lodash": "^4.17.21", "semver": "^7.6.0" }, "devDependencies": { diff --git a/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.test.ts b/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.test.ts index d156182923..ee4af2c4f6 100644 --- a/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.test.ts +++ b/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.test.ts @@ -14,35 +14,24 @@ * limitations under the License. */ -import { Manifest, Workspace } from '@yarnpkg/core'; +import { + Configuration, + Manifest, + Project, + Workspace, + httpUtils, +} from '@yarnpkg/core'; import { npath, ppath } from '@yarnpkg/fslib'; import { createMockDirectory } from '@backstage/backend-test-utils'; import { beforeWorkspacePacking } from './beforeWorkspacePacking'; -jest.mock('@backstage/release-manifests', () => ({ - getManifestByVersion: jest.fn().mockResolvedValue({ - releaseVersion: '1.23.45', - packages: [ - { - name: '@backstage/core', - version: '3.2.1', - }, - { - name: '@backstage/plugin-1', - version: '6.5.4', - }, - { - name: '@backstage/plugin-2', - version: '9.8.7', - }, - ], - }), -})); - const makeWorkspace = (manifest: object) => { return { manifest: Manifest.fromText(JSON.stringify(manifest)), + project: new Project(ppath.cwd(), { + configuration: Configuration.create(ppath.cwd()), + }), } as Workspace; }; @@ -58,6 +47,24 @@ describe('beforeWorkspacePacking', () => { .spyOn(process, 'cwd') .mockReturnValue(npath.toPortablePath(mockDir.path)); + jest.spyOn(httpUtils, 'get').mockResolvedValue({ + releaseVersion: '1.23.45', + packages: [ + { + name: '@backstage/core', + version: '3.2.1', + }, + { + name: '@backstage/plugin-1', + version: '6.5.4', + }, + { + name: '@backstage/plugin-2', + version: '9.8.7', + }, + ], + }); + mockDir.setContent({ 'backstage.json': JSON.stringify({ version: '1.23.45', diff --git a/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts b/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts index fbdea3fa34..e5823607e3 100644 --- a/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts +++ b/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts @@ -70,6 +70,7 @@ export const beforeWorkspacePacking = async ( rawManifest[finalDependencyType][ident] = `^${await getPackageVersion( bindBackstageVersion(descriptor, backstageVersion), + workspace.project.configuration, )}`; } } diff --git a/packages/yarn-plugin/src/resolver/BackstageResolver.test.ts b/packages/yarn-plugin/src/resolver/BackstageResolver.test.ts index f6369b6cdf..8cba189cb8 100644 --- a/packages/yarn-plugin/src/resolver/BackstageResolver.test.ts +++ b/packages/yarn-plugin/src/resolver/BackstageResolver.test.ts @@ -14,31 +14,22 @@ * limitations under the License. */ -import { structUtils } from '@yarnpkg/core'; +import { + Configuration, + Project, + ResolveOptions, + structUtils, + ThrowReport, + httpUtils, +} from '@yarnpkg/core'; import { npath, ppath } from '@yarnpkg/fslib'; -import { getManifestByVersion } from '@backstage/release-manifests'; import { BackstageResolver } from './BackstageResolver'; import { createMockDirectory } from '@backstage/backend-test-utils'; -jest.mock('@backstage/release-manifests', () => ({ - getManifestByVersion: jest.fn().mockResolvedValue({ - releaseVersion: '1.23.45', - packages: [ - { - name: '@backstage/core', - version: '6.7.8', - }, - ], - }), -})); - -const getManifestByVersionMock = getManifestByVersion as jest.MockedFunction< - typeof getManifestByVersion ->; - describe('BackstageResolver', () => { const mockDir = createMockDirectory(); let backstageResolver: BackstageResolver; + let resolveOptions: ResolveOptions; beforeEach(() => { jest @@ -49,6 +40,16 @@ describe('BackstageResolver', () => { .spyOn(process, 'cwd') .mockReturnValue(npath.toPortablePath(mockDir.path)); + jest.spyOn(httpUtils, 'get').mockResolvedValue({ + releaseVersion: '1.23.45', + packages: [ + { + name: '@backstage/core', + version: '6.7.8', + }, + ], + }); + mockDir.setContent({ 'backstage.json': JSON.stringify({ version: '1.23.45', @@ -71,6 +72,14 @@ describe('BackstageResolver', () => { }); backstageResolver = new BackstageResolver(); + + resolveOptions = { + resolver: backstageResolver, + project: new Project(ppath.cwd(), { + configuration: Configuration.create(ppath.cwd()), + }), + report: new ThrowReport(), + }; }); afterEach(() => { @@ -140,7 +149,7 @@ describe('BackstageResolver', () => { ); await expect( - backstageResolver.getCandidates(descriptor), + backstageResolver.getCandidates(descriptor, {}, resolveOptions), ).resolves.toEqual([structUtils.makeLocator(descriptor, 'npm:6.7.8')]); }); @@ -151,6 +160,8 @@ describe('BackstageResolver', () => { structUtils.makeIdent('backstage', 'core'), 'npm:1.2.3', ), + {}, + resolveOptions, ), ).rejects.toThrow(/unsupported version protocol/i); }); @@ -162,6 +173,8 @@ describe('BackstageResolver', () => { structUtils.makeIdent('backstage', 'core'), 'backstage:^', ), + {}, + resolveOptions, ), ).rejects.toThrow(/missing Backstage version/i); }); @@ -173,6 +186,8 @@ describe('BackstageResolver', () => { structUtils.makeIdent('backstage', 'core'), 'backstage:^::v=1&v=2', ), + {}, + resolveOptions, ), ).rejects.toThrow(/multiple Backstage versions/i); }); @@ -190,34 +205,12 @@ describe('BackstageResolver', () => { structUtils.makeIdent('backstage', 'core'), `backstage:${selector}`, ), + {}, + resolveOptions, ), ).rejects.toThrow(/unexpected version selector/i); }, ); - - it('memoizes manifest retrieval', async () => { - const descriptor1 = structUtils.makeDescriptor( - structUtils.makeIdent('backstage', 'core'), - 'backstage:^::v=1.23.45', - ); - - for (let i = 0; i < 5; i++) { - await backstageResolver.getCandidates(descriptor1); - } - - expect(getManifestByVersionMock).toHaveBeenCalledTimes(1); - - const descriptor2 = structUtils.makeDescriptor( - structUtils.makeIdent('backstage', 'core'), - 'backstage:^::v=6.78.90', - ); - - for (let i = 0; i < 5; i++) { - await backstageResolver.getCandidates(descriptor2); - } - - expect(getManifestByVersionMock).toHaveBeenCalledTimes(2); - }); }); describe('getSatisfying', () => { @@ -243,6 +236,7 @@ describe('BackstageResolver', () => { 'npm:1.2.3', ), ], + resolveOptions, ), ).resolves.toEqual({ locators: [ @@ -277,6 +271,7 @@ describe('BackstageResolver', () => { 'npm:7.8.9', ), ], + resolveOptions, ), ).resolves.toEqual({ locators: [ @@ -298,6 +293,7 @@ describe('BackstageResolver', () => { ), {}, [], + resolveOptions, ), ).rejects.toThrow(/unsupported version protocol/i); }); diff --git a/packages/yarn-plugin/src/resolver/BackstageResolver.ts b/packages/yarn-plugin/src/resolver/BackstageResolver.ts index 0e575cc687..d2d0c98d1e 100644 --- a/packages/yarn-plugin/src/resolver/BackstageResolver.ts +++ b/packages/yarn-plugin/src/resolver/BackstageResolver.ts @@ -20,6 +20,7 @@ import { Locator, Package, Resolver, + ResolveOptions, } from '@yarnpkg/core'; import { PROTOCOL } from '../constants'; import { @@ -60,11 +61,18 @@ export class BackstageResolver implements Resolver { * concrete version into the appropriate concrete npm version for that * backstage release. */ - async getCandidates(descriptor: Descriptor): Promise { + async getCandidates( + descriptor: Descriptor, + _dependencies: Record, + opts: ResolveOptions, + ): Promise { return [ structUtils.makeLocator( descriptor, - `npm:${await getPackageVersion(descriptor)}`, + `npm:${await getPackageVersion( + descriptor, + opts.project.configuration, + )}`, ), ]; } @@ -79,8 +87,12 @@ export class BackstageResolver implements Resolver { descriptor: Descriptor, _dependencies: Record, locators: Array, + opts: ResolveOptions, ): Promise<{ locators: Locator[]; sorted: boolean }> { - const packageVersion = await getPackageVersion(descriptor); + const packageVersion = await getPackageVersion( + descriptor, + opts.project.configuration, + ); return { locators: locators.filter( diff --git a/packages/yarn-plugin/src/util.ts b/packages/yarn-plugin/src/util.ts index 5fd0091b2b..ee89a647e3 100644 --- a/packages/yarn-plugin/src/util.ts +++ b/packages/yarn-plugin/src/util.ts @@ -14,18 +14,18 @@ * limitations under the License. */ +import { + Configuration, + Descriptor, + httpUtils, + structUtils, +} from '@yarnpkg/core'; import { ppath, xfs } from '@yarnpkg/fslib'; import { valid as semverValid } from 'semver'; -import memoize from 'lodash/memoize'; -import { getManifestByVersion as getManifestByVersionBase } from '@backstage/release-manifests'; import { BACKSTAGE_JSON, findPaths } from '@backstage/cli-common'; -import { Descriptor, structUtils } from '@yarnpkg/core'; -import { PROTOCOL } from './constants'; +import { getManifestByVersion } from '@backstage/release-manifests'; -const getManifestByVersion = memoize( - getManifestByVersionBase, - ({ version }) => version, -); +import { PROTOCOL } from './constants'; export const getCurrentBackstageVersion = () => { const workspaceRoot = ppath.resolve(findPaths(ppath.cwd()).targetRoot); @@ -50,7 +50,10 @@ export const bindBackstageVersion = ( return structUtils.bindDescriptor(descriptor, { v: backstageVersion }); }; -export const getPackageVersion = async (descriptor: Descriptor) => { +export const getPackageVersion = async ( + descriptor: Descriptor, + configuration: Configuration, +) => { const ident = structUtils.stringifyIdent(descriptor); const range = structUtils.parseRange(descriptor.range); @@ -80,6 +83,36 @@ export const getPackageVersion = async (descriptor: Descriptor) => { const manifest = await getManifestByVersion({ version: range.params.v, + // We override the fetch function used inside getManifestByVersion with a + // custom implementation that calls yarn's built-in `httpUtils` method + // instead. This has a couple of benefits: + // + // 1. This means that the fetch should leverage yarn's built-in cache, so we + // don't need to explicitly memoize the fetch. + // 2. The request should automatically take account of any proxy settings + // configured in yarn. + fetch: async (url: string) => { + const response = await httpUtils.get(url, { + configuration, + jsonResponse: true, + }); + + // The release-manifests package expects fetchFn to resolve with a subset + // of the native HTTP Response object, but yarn's httpUtils implementation + // keeps most of the details hidden. This means we need to construct an + // object which quacks like a Response in the appropriate ways. + return { + // The function has some custom handling for non-200 errors. Yarn + // doesn't provide the status code, but if we've got to this point + // without throwing, we can assume the request has been successful. + status: 200, + // The requested URL, used to correctly report errors + url, + // Yarn automatically parses the response as JSON, so our implementation + // can simply return it. + json: () => response, + }; + }, }); const manifestEntry = manifest.packages.find( diff --git a/yarn.lock b/yarn.lock index 11c894ed4c..88f9b8f322 100644 --- a/yarn.lock +++ b/yarn.lock @@ -47612,7 +47612,6 @@ __metadata: "@yarnpkg/core": ^4.0.3 "@yarnpkg/fslib": ^3.0.2 "@yarnpkg/plugin-pack": ^4.0.0 - lodash: ^4.17.21 nodemon: ^3.0.1 semver: ^7.6.0 languageName: unknown