From 254f59d1d852a832fe8facf6c305cf23850e9229 Mon Sep 17 00:00:00 2001 From: Brian Kaney Date: Thu, 20 Jul 2023 07:38:03 -0400 Subject: [PATCH 1/5] POC Fix for resolving tarball location of packages --- src/load.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/load.ts b/src/load.ts index ed53b22..f990656 100644 --- a/src/load.ts +++ b/src/load.ts @@ -11,6 +11,13 @@ import { getCustomRegistry } from './utils/customRegistry'; import { AxiosResponse } from 'axios'; import { LatestVersionUnavailableError } from './errors/LatestVersionUnavailableError'; +async function getDistUrl(registry: string, packageName: string, version: string): Promise { + // 1 get the manifest information about the package from the registry + const res = await axiosGet(`${registry}/${packageName}`); + // 2 return the tarball location + return res.data?.versions?.[version]?.dist?.tarball; +} + /** * Loads multiple dependencies from a directory (the user FHIR cache or a specified directory) or from online * @param {string[]} fhirPackages - An array of FHIR packages to download and load definitions from (format: packageId#version) @@ -220,9 +227,9 @@ export async function mergeDependency( } else if (!loadedPackage) { const customRegistry = getCustomRegistry(log); if (customRegistry) { - packageUrl = `${customRegistry.replace(/\/$/, '')}/${packageName}/${version}`; + packageUrl = await getDistUrl(customRegistry.replace(/\/$/, ''), packageName, version); //`${customRegistry.replace(/\/$/, '')}/${packageName}/${version}`; } else { - packageUrl = `https://packages.fhir.org/${packageName}/${version}`; + packageUrl = await getDistUrl('https://packages.fhir.org', packageName, version)// `https://packages.fhir.org/${packageName}/${version}`; } } From 567a9a2c7228a48f0baa0a94c5c2faa0d26ed27b Mon Sep 17 00:00:00 2001 From: Brian Kaney Date: Thu, 20 Jul 2023 09:06:09 -0400 Subject: [PATCH 2/5] Fall back to special FHIR location --- src/load.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/load.ts b/src/load.ts index f990656..a94aed6 100644 --- a/src/load.ts +++ b/src/load.ts @@ -14,8 +14,15 @@ import { LatestVersionUnavailableError } from './errors/LatestVersionUnavailable async function getDistUrl(registry: string, packageName: string, version: string): Promise { // 1 get the manifest information about the package from the registry const res = await axiosGet(`${registry}/${packageName}`); - // 2 return the tarball location - return res.data?.versions?.[version]?.dist?.tarball; + // 2 find the NPM tarball location + const npmLocation = res.data?.versions?.[version]?.dist?.tarball; + + // 3 if found, use it, otherwise fallback to the FHIR spec location + if (npmLocation) { + return npmLocation + } else { + return `${registry}/${packageName}/${version}` + } } /** From 34c038b8c9da5f34c49829839af9d50d2d34157e Mon Sep 17 00:00:00 2001 From: Brian Kaney Date: Fri, 21 Jul 2023 07:33:01 -0400 Subject: [PATCH 3/5] Only apply pattern to custom registry. Also linting --- src/load.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/load.ts b/src/load.ts index a94aed6..eb85402 100644 --- a/src/load.ts +++ b/src/load.ts @@ -13,15 +13,15 @@ import { LatestVersionUnavailableError } from './errors/LatestVersionUnavailable async function getDistUrl(registry: string, packageName: string, version: string): Promise { // 1 get the manifest information about the package from the registry - const res = await axiosGet(`${registry}/${packageName}`); + const res = await axiosGet(`${registry.replace(/\/$/, '')}/${packageName}`); // 2 find the NPM tarball location const npmLocation = res.data?.versions?.[version]?.dist?.tarball; // 3 if found, use it, otherwise fallback to the FHIR spec location if (npmLocation) { - return npmLocation + return npmLocation; } else { - return `${registry}/${packageName}/${version}` + return `${registry}/${packageName}/${version}`; } } @@ -234,9 +234,9 @@ export async function mergeDependency( } else if (!loadedPackage) { const customRegistry = getCustomRegistry(log); if (customRegistry) { - packageUrl = await getDistUrl(customRegistry.replace(/\/$/, ''), packageName, version); //`${customRegistry.replace(/\/$/, '')}/${packageName}/${version}`; + packageUrl = await getDistUrl(customRegistry, packageName, version); } else { - packageUrl = await getDistUrl('https://packages.fhir.org', packageName, version)// `https://packages.fhir.org/${packageName}/${version}`; + packageUrl = `https://packages.fhir.org/${packageName}/${version}`; } } From 1f7e7eae053e39c3cf2e6bf00282558a0ad78503 Mon Sep 17 00:00:00 2001 From: Brian Kaney Date: Fri, 21 Jul 2023 10:39:41 -0400 Subject: [PATCH 4/5] Add tests, lint, and fmt --- src/load.ts | 7 +++--- test/load.test.ts | 62 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/load.ts b/src/load.ts index eb85402..e514ebc 100644 --- a/src/load.ts +++ b/src/load.ts @@ -12,16 +12,17 @@ import { AxiosResponse } from 'axios'; import { LatestVersionUnavailableError } from './errors/LatestVersionUnavailableError'; async function getDistUrl(registry: string, packageName: string, version: string): Promise { + const cleanedRegistry = registry.replace(/\/$/, ''); // 1 get the manifest information about the package from the registry - const res = await axiosGet(`${registry.replace(/\/$/, '')}/${packageName}`); + const res = await axiosGet(`${cleanedRegistry}/${packageName}`); // 2 find the NPM tarball location const npmLocation = res.data?.versions?.[version]?.dist?.tarball; - + // 3 if found, use it, otherwise fallback to the FHIR spec location if (npmLocation) { return npmLocation; } else { - return `${registry}/${packageName}/${version}`; + return `${cleanedRegistry}/${packageName}/${version}`; } } diff --git a/test/load.test.ts b/test/load.test.ts index 1949d7c..251244d 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -474,7 +474,7 @@ describe('#mergeDependency()', () => { // This function encapsulates that testing logic. It's coupled more tightly to // the actual implementation than I'd prefer, but... at least it's in one place. const expectDownloadSequence = ( - sources: string | string[], + sources: string | string[] | { source: string; omitResponseType?: boolean }[], destination: string | null, isCurrent = false, isCurrentFound = true @@ -485,14 +485,33 @@ describe('#mergeDependency()', () => { if (isCurrent) { const mockCalls: any[] = [['https://build.fhir.org/ig/qas.json']]; if (isCurrentFound) { - mockCalls.push( - [sources[0].replace(/package\.tgz$/, 'package.manifest.json')], - [sources[0], { responseType: 'arraybuffer' }] - ); + if (typeof sources[0] === 'string') { + mockCalls.push( + [sources[0].replace(/package\.tgz$/, 'package.manifest.json')], + [sources[0], { responseType: 'arraybuffer' }] + ); + } else { + mockCalls.push([sources[0].source.replace(/package\.tgz$/, 'package.manifest.json')]); + if (sources[0].omitResponseType !== true) { + mockCalls.push([sources[0].source, { responseType: 'arraybuffer' }]); + } + } } expect(axiosSpy.mock.calls).toEqual(mockCalls); } else { - expect(axiosSpy.mock.calls).toEqual(sources.map(s => [s, { responseType: 'arraybuffer' }])); + expect(axiosSpy.mock.calls).toEqual( + sources.map(s => { + if (typeof s === 'string') { + return [s, { responseType: 'arraybuffer' }]; + } else { + if (s.omitResponseType === true) { + return [s.source]; + } else { + return [s.source, { responseType: 'arraybuffer' }]; + } + } + }) + ); } if (destination != null) { const tempTarFile = writeSpy.mock.calls[0][0]; @@ -591,6 +610,10 @@ describe('#mergeDependency()', () => { date: '20200413230227' } }; + } else if (uri === 'https://packages.fhir.org/hl7.fhir.r4.core') { + return { data: TERM_PKG_RESPONSE }; + } else if (uri === 'https://packages2.fhir.org/hl7.fhir.r4.core') { + return { data: EXT_PKG_RESPONSE }; } else if ( uri === 'https://packages.fhir.org/sushi-test/0.2.0' || uri === 'https://build.fhir.org/ig/sushi/sushi-test/branches/testbranch/package.tgz' || @@ -736,13 +759,31 @@ describe('#mergeDependency()', () => { ); }); - it('should try to load a package from a custom registry', async () => { + it('should try to load a package from a custom registry that is like NPM', async () => { + // packages.fhir.org supports NPM clients + process.env.FPL_REGISTRY = 'https://packages.fhir.org'; + await expect(mergeDependency('hl7.fhir.r4.core', '4.0.1', defs, 'foo', log)).rejects.toThrow( + 'The package hl7.fhir.r4.core#4.0.1 could not be loaded locally or from the custom FHIR package registry https://packages.fhir.org.' + ); // the package is never actually added to the cache, since tar is mocked + expectDownloadSequence( + [ + { source: 'https://packages.fhir.org/hl7.fhir.r4.core', omitResponseType: true }, + { source: 'https://packages.fhir.org/hl7.fhir.r4.core/4.0.1' } + ], + path.join('foo', 'hl7.fhir.r4.core#4.0.1') + ); + }); + + it('should try to load a package from a custom registry that is not like NPM', async () => { process.env.FPL_REGISTRY = 'https://custom-registry.example.org'; await expect(mergeDependency('good-thing', '0.3.6', defs, 'foo', log)).rejects.toThrow( 'The package good-thing#0.3.6 could not be loaded locally or from the custom FHIR package registry https://custom-registry.example.org' ); // the package is never actually added to the cache, since tar is mocked expectDownloadSequence( - 'https://custom-registry.example.org/good-thing/0.3.6', + [ + { source: 'https://custom-registry.example.org/good-thing', omitResponseType: true }, + { source: 'https://custom-registry.example.org/good-thing/0.3.6' } + ], path.join('foo', 'good-thing#0.3.6') ); }); @@ -753,7 +794,10 @@ describe('#mergeDependency()', () => { 'The package good-thing#0.3.6 could not be loaded locally or from the custom FHIR package registry https://custom-registry.example.org/' ); // the package is never actually added to the cache, since tar is mocked expectDownloadSequence( - 'https://custom-registry.example.org/good-thing/0.3.6', + [ + { source: 'https://custom-registry.example.org/good-thing', omitResponseType: true }, + { source: 'https://custom-registry.example.org/good-thing/0.3.6' } + ], path.join('foo', 'good-thing#0.3.6') ); }); From 9a84178ef60ab96251f21790eedcf58cafe54a8b Mon Sep 17 00:00:00 2001 From: Brian Kaney Date: Tue, 25 Jul 2023 10:34:58 -0400 Subject: [PATCH 5/5] Update test to more accurately represent response --- test/load.test.ts | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/test/load.test.ts b/test/load.test.ts index 251244d..7cdd227 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -611,9 +611,27 @@ describe('#mergeDependency()', () => { } }; } else if (uri === 'https://packages.fhir.org/hl7.fhir.r4.core') { - return { data: TERM_PKG_RESPONSE }; - } else if (uri === 'https://packages2.fhir.org/hl7.fhir.r4.core') { - return { data: EXT_PKG_RESPONSE }; + return { + data: { + _id: 'hl7.fhir.r4.core', + name: 'hl7.fhir.r4.core', + 'dist-tags': { latest: '4.0.1' }, + versions: { + '4.0.1': { + name: 'hl7.fhir.r4.core', + version: '4.0.1', + description: + 'Definitions (API, structures and terminologies) for the R4 version of the FHIR standard', + dist: { + shasum: '0e4b8d99f7918587557682c8b47df605424547a5', + tarball: 'https://packages.fhir.org/hl7.fhir.r4.core/4.0.1' + }, + fhirVersion: 'R4', + url: 'https://packages.fhir.org/hl7.fhir.r4.core/4.0.1' + } + } + } + }; } else if ( uri === 'https://packages.fhir.org/sushi-test/0.2.0' || uri === 'https://build.fhir.org/ig/sushi/sushi-test/branches/testbranch/package.tgz' ||