Skip to content

Commit

Permalink
Load dependencies in deterministic order (#1213)
Browse files Browse the repository at this point in the history 
First load automatic dependencies (in order), then configured dependencies (in order), then FHIR core.

This makes builds more repeatable but also solves the issue that Core should have highest priority when resolving resources.
  • Loading branch information
@cmoesel
cmoesel authored Feb 20, 2023
1 parent 6d5233f commit 3b7e4fa
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 54 deletions.
81 changes: 38 additions & 43 deletions src/utils/Processing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,22 +212,20 @@ export async function loadExternalDependencies(
);
}
dependencies.push({ packageId: fhirPackageId, version: fhirVersion });
const automaticPromises = loadAutomaticDependencies(dependencies, defs);
const configuredPromises = loadConfiguredDependencies(
dependencies,
fhirVersion,
config.filePath,
defs
);

return Promise.all(configuredPromises.concat(automaticPromises)).then(() => {});
// Load automatic dependencies first so they have lowest priority in resolution
await loadAutomaticDependencies(dependencies, defs);

// Then load configured dependencies, with FHIR core last so it has highest priority in resolution
await loadConfiguredDependencies(dependencies, fhirVersion, config.filePath, defs);
}

export function loadAutomaticDependencies(
export async function loadAutomaticDependencies(
configuredDependencies: ImplementationGuideDependsOn[],
defs: FHIRDefinitions
): Promise<void | FHIRDefinitions>[] {
return AUTOMATIC_DEPENDENCIES.map(dep => {
): Promise<void> {
// Load dependencies serially so dependency loading order is predictable and repeatable
for (let dep of AUTOMATIC_DEPENDENCIES) {
const alreadyConfigured = configuredDependencies.some(cd => {
// hl7.some.package, hl7.some.package.r4, and hl7.some.package.r5 all represent the same content,
// so they are essentially interchangeable and we should allow for any of them in the config.
Expand All @@ -237,43 +235,40 @@ export function loadAutomaticDependencies(
);
return configRootId === packageRootId;
});
if (alreadyConfigured) {
return Promise.resolve();
} else {
let p = Promise.resolve();
if (dep.version === 'latest') {
// clone it before we modify it so we don't overwrite the global (mostly helpful for testing)
dep = cloneDeep(dep);
p = axiosGet(`https://packages.fhir.org/${dep.packageId}`, { responseType: 'json' }).then(
res => {
if (res?.data?.['dist-tags']?.latest?.length) {
dep.version = res.data['dist-tags'].latest;
} else {
throw new Error(`Could not determine latest released version of ${dep.packageId}.`);
}
if (!alreadyConfigured) {
try {
if (dep.version === 'latest') {
// clone it before we modify it so we don't overwrite the global (mostly helpful for testing)
dep = cloneDeep(dep);
const res = await axiosGet(`https://packages.fhir.org/${dep.packageId}`, {
responseType: 'json'
});
if (res?.data?.['dist-tags']?.latest?.length) {
dep.version = res.data['dist-tags'].latest;
} else {
throw new Error(`Could not determine latest released version of ${dep.packageId}.`);
}
);
}
await loadDependency(dep.packageId, dep.version, defs);
} catch (e) {
let message = `Failed to load automatically-provided ${dep.packageId}#${dep.version}: ${e.message}`;
if (/certificate/.test(e.message)) {
message += CERTIFICATE_MESSAGE;
}
logger.warn(message);
}
return p
.then(() => loadDependency(dep.packageId, dep.version, defs))
.catch(e => {
let message = `Failed to load automatically-provided ${dep.packageId}#${dep.version}: ${e.message}`;
if (/certificate/.test(e.message)) {
message += CERTIFICATE_MESSAGE;
}
logger.warn(message);
});
}
});
}
}

function loadConfiguredDependencies(
async function loadConfiguredDependencies(
dependencies: ImplementationGuideDependsOn[],
fhirVersion: string,
configPath: string,
defs: FHIRDefinitions
): Promise<void | FHIRDefinitions>[] {
return dependencies.map(dep => {
): Promise<void> {
// Load dependencies serially so dependency loading order is predictable and repeatable
for (const dep of dependencies) {
if (dep.version == null) {
logger.error(
`Failed to load ${dep.packageId}: No version specified. To specify the version in your ` +
Expand All @@ -286,7 +281,7 @@ function loadConfiguredDependencies(
` uri: ${dep.uri ?? 'http://my-fhir-ig.org/ImplementationGuide/123'}\n` +
' version: current'
);
return Promise.resolve();
continue;
} else if (EXT_PKG_TO_FHIR_PKG_MAP[dep.packageId]) {
// It is a special "virtual" FHIR extensions package indicating we need to load supplemental
// FHIR versions to support "implied extensions".
Expand All @@ -301,17 +296,17 @@ function loadConfiguredDependencies(
logger.info(
`Loading supplemental version of FHIR to support extensions from ${dep.packageId}`
);
return loadSupplementalFHIRPackage(EXT_PKG_TO_FHIR_PKG_MAP[dep.packageId], defs);
await loadSupplementalFHIRPackage(EXT_PKG_TO_FHIR_PKG_MAP[dep.packageId], defs);
} else {
return loadDependency(dep.packageId, dep.version, defs).catch(e => {
await loadDependency(dep.packageId, dep.version, defs).catch(e => {
let message = `Failed to load ${dep.packageId}#${dep.version}: ${e.message}`;
if (/certificate/.test(e.message)) {
message += CERTIFICATE_MESSAGE;
}
logger.error(message);
});
}
});
}
}

export function getRawFSHes(input: string): RawFSH[] {
Expand Down
63 changes: 52 additions & 11 deletions test/utils/Processing.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,12 @@ describe('Processing', () => {
// it can also find automatic dependencies
if (
/^hl7.fhir.(r2|r3|r4|r4b|r5|us).core$/.test(packageName) ||
AUTOMATIC_DEPENDENCIES.some(dep => dep.packageId === packageName)
AUTOMATIC_DEPENDENCIES.some(dep => {
const [depRootId, packageRootId] = [dep.packageId, packageName].map(id =>
/\.r[4-9]$/.test(id ?? '') ? (id ?? '').slice(0, -3) : id
);
return depRootId === packageRootId;
})
) {
FHIRDefs.packages.push(`${packageName}#${version}`);
return Promise.resolve(FHIRDefs);
Expand Down Expand Up @@ -399,6 +404,42 @@ describe('Processing', () => {
});
});

it('should load automatic dependencies first so they have lowest priority', () => {
const usCoreDependencyConfig = cloneDeep(minimalConfig);
usCoreDependencyConfig.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
const defs = new FHIRDefinitions();
return loadExternalDependencies(defs, usCoreDependencyConfig).then(() => {
expect(defs.packages.length).toBe(2 + AUTOMATIC_DEPENDENCIES.length);
expect(defs.packages).toEqual([
'hl7.fhir.uv.tools#current',
'hl7.terminology.r4#1.2.3-test',
'hl7.fhir.us.core#3.1.0',
'hl7.fhir.r4.core#4.0.1'
]);
expect(loggerSpy.getAllLogs('warn')).toHaveLength(0);
});
});

it('should honor user-specified order when user puts automatic dependencies in the config', () => {
const usCoreDependencyConfig = cloneDeep(minimalConfig);
usCoreDependencyConfig.dependencies = [
{ packageId: 'hl7.fhir.us.core', version: '3.1.0' },
{ packageId: 'hl7.terminology.r4', version: '8.8.8' },
{ packageId: 'hl7.fhir.uv.tools', version: '9.9.9' }
];
const defs = new FHIRDefinitions();
return loadExternalDependencies(defs, usCoreDependencyConfig).then(() => {
expect(defs.packages.length).toBe(2 + AUTOMATIC_DEPENDENCIES.length);
expect(defs.packages).toEqual([
'hl7.fhir.us.core#3.1.0',
'hl7.terminology.r4#8.8.8',
'hl7.fhir.uv.tools#9.9.9',
'hl7.fhir.r4.core#4.0.1'
]);
expect(loggerSpy.getAllLogs('warn')).toHaveLength(0);
});
});

it('should support prerelease FHIR R4B dependencies', () => {
const config = cloneDeep(minimalConfig);
config.fhirVersion = ['4.1.0'];
Expand Down Expand Up @@ -637,7 +678,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(2);
expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
expect(defs.packages).toContain('hl7.terminology.r4#1.2.3-test');
Expand All @@ -662,7 +703,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(2);
expect(defs.packages).toContain('hl7.terminology.r4#2.4.6-test');
expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
Expand All @@ -680,7 +721,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.uv.tools', version: '2.2.0-test' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.fhir.uv.tools#current');
expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
Expand All @@ -698,7 +739,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.uv.tools.r4', version: '4.0.0-test' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.fhir.uv.tools#current');
expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
Expand All @@ -716,7 +757,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.terminology', version: '4.0.0-test' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.terminology.r4#1.2.3-test');
expect(defs.packages).not.toContain('hl7.terminology.r4#latest');
Expand All @@ -735,7 +776,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.terminology.r5', version: '4.0.0-test' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.terminology.r4#1.2.3-test');
expect(defs.packages).not.toContain('hl7.terminology.r4#latest');
Expand All @@ -758,7 +799,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.fhir.uv.tools#current');
expect(loggerSpy.getAllMessages('warn')).toHaveLength(1);
Expand All @@ -785,7 +826,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.terminology.r4#4.0.0');
expect(loggerSpy.getAllMessages('warn')).toHaveLength(1);
Expand All @@ -812,7 +853,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.terminology.r4#4.0.0');
expect(loggerSpy.getAllMessages('warn')).toHaveLength(1);
Expand All @@ -839,7 +880,7 @@ describe('Processing', () => {
const config = cloneDeep(minimalConfig);
config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
const defs = new FHIRDefinitions();
return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
return loadAutomaticDependencies(config.dependencies, defs).then(() => {
expect(defs.packages).toHaveLength(AUTOMATIC_DEPENDENCIES.length - 1);
expect(defs.packages).not.toContain('hl7.fhir.uv.tools#current');
expect(loggerSpy.getAllMessages('warn')).toHaveLength(1);
Expand Down

0 comments on commit 3b7e4fa

Please sign in to comment.