Repository: FHIR/sushi
Pull request #1213 (2023-02-16 16:00:59)

Load dependencies in deterministic order
This PR changes dependency loading logic so that automatic dependencies are loaded first (in the order specified in our code), then configured dependencies (in the order specified in the config), then FHIR core. Dependencies that are loaded later get higher priority in name and id resolution.

This makes builds more repeatable but also solves the issue that the automatic `hl7.terminology` dependency was replacing FHIR Core canonicals w/ `hl7.terminology` canonicals.

To test this, run a regression against master and include this repo:
```
saulakravitz/vrdr-fsh#master
```

The master branch of SUSHI will generate value set bindings for `http://terminology.hl7.org/ValueSet/location-physical-type` and `http://terminology.hl7.org/ValueSet/organization-type`, but this branch will generate bindings for `http://hl7.org/fhir/ValueSet/location-physical-type` and `http://hl7.org/fhir/ValueSet/organization-type`. The original R4 bindings are preferred.



Patch:
diff --git a/src/utils/Processing.ts b/src/utils/Processing.ts
index 499c457..4237fe0 100644
--- a/src/utils/Processing.ts
+++ b/src/utils/Processing.ts
@@ -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.
@@ -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 ` +
@@ -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".
@@ -301,9 +296,9 @@ 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;
@@ -311,7 +306,7 @@ function loadConfiguredDependencies(
         logger.error(message);
       });
     }
-  });
+  }
 }
 
 export function getRawFSHes(input: string): RawFSH[] {


Test patch:
diff --git a/test/utils/Processing.test.ts b/test/utils/Processing.test.ts
index 3591ecc..d2914e4 100644
--- a/test/utils/Processing.test.ts
+++ b/test/utils/Processing.test.ts
@@ -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);
@@ -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'];
@@ -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');
@@ -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);
@@ -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);
@@ -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);
@@ -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');
@@ -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');
@@ -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);
@@ -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);
@@ -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);
@@ -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);