Repository: FHIR/sushi
Pull request #685 (2020-12-02 19:16:04)

Support a sushi-config.yaml-only option (no FSH files in a current tank configuration)
This PR addresses #672. This supports a sushi-config.yaml-only option and have SUSHI run on a current tank configuration without any FSH files.

On master, if we run SUSHI on a current tank configuration but do not have a `fsh` subdirectory with FSH files, SUSHI will incorrectly think we are in a legacy tank configuration. In order to appropriately detect that we are in a current tank configuration but without the `input/fsh` path, we need to check a few different conditions:
- there must be no `.fsh` files anywhere in the tank (if they are in any arbitrary folder, we are in a legacy "flat" tank)
- there must be no `fsh/` directory (if there is, we are in a legacy IG Publisher tank)
- there must be no `ig-data` directory (if there is, we are in a legacy "flat" tank)

Once we detect that we are indeed in the current tank configuration without FSH files (the sushi-config.yaml-only option), we need SUSHI to properly look in the correct places for BYO-resources, configuration, etc. Since we have been changing the "input" path based on the tank configuration we are in, we do a lot of path manipulation (going up a directory, up two directories, etc) in a bunch of different places. The easiest way I could figure out to have this sushi-config.yaml-only option be supported without changing all those places we manipulate paths was to set the input path as if there was an `input/fsh` folder. This lets the rest of the code operate as it would if we were in a typical current tank configuration. The one place that needs to be handled separately was when we process the FSH files. If we are in this new case where our path is to an `input/fsh` directory that does not exist, we can safely say there are no FSH files.

I had also attempted to change how we change and use the "input" path, but it proved to be fairly convoluted and didn't feel like a huge improvement, so I decided to go with this approach due to the relative simplicity of it, especially given that we might be able to simplify a lot of this when we stop supporting legacy tank configurations.

jafeltra: @cmoesel your comments should be addressed. I also realized I didn't add any tests for `hasFshFiles` so I added two simple tests for that function.

And thank you for testing this out on so many combinations of tanks! I'm always doubtful that I checked every case, so I appreciate it! The only additional (I think) case that I had tested that you didn't explicitly list was SUSHI 1.0.0 IG w/ fsh/ with no files. That should work the same on this and on master.

Patch:
diff --git a/src/app.ts b/src/app.ts
index 6a973fa..ee58f92 100644
--- a/src/app.ts
+++ b/src/app.ts
@@ -5,7 +5,7 @@ import fs from 'fs-extra';
 import program from 'commander';
 import chalk from 'chalk';
 import { pad, padStart, sample, padEnd } from 'lodash';
-import { FSHTank } from './import';
+import { FSHTank, RawFSH } from './import';
 import { exportFHIR, Package } from './export';
 import { IGExporter } from './ig';
 import { logger, stats, Type } from './utils';
@@ -106,7 +106,19 @@ async function app() {
   let config: Configuration;
 
   try {
-    const rawFSH = getRawFSHes(input);
+    let rawFSH: RawFSH[];
+    if (
+      path.basename(path.dirname(input)) === 'input' &&
+      path.basename(input) === 'fsh' &&
+      !fs.existsSync(input)
+    ) {
+      // If we have a path that ends with input/fsh but that folder does not exist,
+      // we are in a sushi-config.yaml-only case (new tank configuration with no FSH files)
+      // so we can safely say there are no FSH files and therefore rawFSH is empty.
+      rawFSH = [];
+    } else {
+      rawFSH = getRawFSHes(input);
+    }
     if (
       rawFSH.length === 0 &&
       !fs.existsSync(path.join(originalInput, 'config.yaml')) &&
diff --git a/src/utils/Processing.ts b/src/utils/Processing.ts
index 75244c2..4835d35 100644
--- a/src/utils/Processing.ts
+++ b/src/utils/Processing.ts
@@ -31,19 +31,36 @@ export function ensureInputDir(input: string): string {
   return input;
 }
 
+export function hasFshFiles(path: string): boolean {
+  try {
+    const files = getFilesRecursive(path).filter(file => file.endsWith('.fsh'));
+    return files.length > 0;
+  } catch (error) {
+    return false;
+  }
+}
+
 export function findInputDir(input: string): string {
   const originalInput = input;
 
-  // TODO: Legacy support. Remove when no longer supported.
-  // Use input/fsh/ subdirectory if not already specified and present
   const inputFshSubdirectoryPath = path.join(originalInput, 'input', 'fsh');
-  if (fs.existsSync(inputFshSubdirectoryPath)) {
+  const fshSubdirectoryPath = path.join(originalInput, 'fsh');
+  const rootIgDataPath = path.join(originalInput, 'ig-data');
+  const currentTankWithNoFsh =
+    !fs.existsSync(inputFshSubdirectoryPath) &&
+    !fs.existsSync(fshSubdirectoryPath) &&
+    !fs.existsSync(rootIgDataPath) &&
+    !hasFshFiles(originalInput);
+
+  // Use input/fsh/ subdirectory if not already specified and present
+  // or when in the current tank configuration without FSH files
+  if (fs.existsSync(inputFshSubdirectoryPath) || currentTankWithNoFsh) {
     input = path.join(originalInput, 'input', 'fsh');
   }
 
+  // TODO: Legacy support. Remove when no longer supported.
   // Use fsh/ subdirectory if not already specified and present
-  const fshSubdirectoryPath = path.join(originalInput, 'fsh');
-  if (!fs.existsSync(inputFshSubdirectoryPath)) {
+  if (!fs.existsSync(inputFshSubdirectoryPath) && !currentTankWithNoFsh) {
     let msg =
       '\n\n!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! IMPORTANT !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n';
     if (fs.existsSync(fshSubdirectoryPath)) {


Test patch:
diff --git a/test/utils/Processing.test.ts b/test/utils/Processing.test.ts
index fd16b2b..a34cb6b 100644
--- a/test/utils/Processing.test.ts
+++ b/test/utils/Processing.test.ts
@@ -12,6 +12,7 @@ import {
   readConfig,
   loadExternalDependencies,
   getRawFSHes,
+  hasFshFiles,
   writeFHIRResources,
   init
 } from '../../src/utils/Processing';
@@ -31,9 +32,16 @@ describe('Processing', () => {
       tempRoot = temp.mkdirSync('sushi-test');
       fs.mkdirpSync(path.join(tempRoot, 'has-fsh', 'fsh')); // TODO: Tests legacy support. Remove when no longer supported.
       fs.mkdirpSync(path.join(tempRoot, 'has-input-fsh', 'input', 'fsh'));
+      fs.mkdirpSync(path.join(tempRoot, 'has-input', 'input'));
       fs.mkdirpSync(path.join(tempRoot, 'has-fsh-and-input-fsh', 'fsh')); // TODO: Tests legacy support. Remove when no longer supported.
       fs.mkdirpSync(path.join(tempRoot, 'has-fsh-and-input-fsh', 'input', 'fsh'));
+      fs.mkdirpSync(path.join(tempRoot, 'flat-tank', 'ig-data'));
       fs.mkdirSync(path.join(tempRoot, 'no-fsh'));
+      fs.ensureFileSync(path.join(tempRoot, 'no-fsh', 'notfsh.txt'));
+    });
+
+    beforeEach(() => {
+      loggerSpy.reset();
     });
 
     afterAll(() => {
@@ -61,6 +69,13 @@ describe('Processing', () => {
       expect(foundInput).toBe(path.join(tempRoot, 'has-input-fsh', 'input', 'fsh'));
     });
 
+    it('should use path to input/fsh as input if input/ subdirectory present but no input/fsh present', () => {
+      const input = path.join(tempRoot, 'has-input');
+      const foundInput = findInputDir(input);
+      expect(foundInput).toBe(path.join(tempRoot, 'has-input', 'input', 'fsh'));
+      expect(loggerSpy.getAllMessages()).toHaveLength(0);
+    });
+
     // TODO: Tests legacy support. Remove when no longer supported.
     it('should prefer path to input/fsh over fsh/ if both present', () => {
       const input = path.join(tempRoot, 'has-fsh-and-input-fsh');
@@ -68,11 +83,17 @@ describe('Processing', () => {
       expect(foundInput).toBe(path.join(tempRoot, 'has-fsh-and-input-fsh', 'input', 'fsh'));
     });
 
-    it('should find a path to the provided directory if the fsh subdirectory is not present', () => {
-      const input = path.join(tempRoot, 'no-fsh');
+    it('should find a path to the provided directory if the fsh subdirectory is not present and a root ig-data is present (legacy flat tank)', () => {
+      const input = path.join(tempRoot, 'flat-tank');
       const foundInput = findInputDir(input);
       expect(foundInput).toBe(input);
     });
+
+    it('should find a path to input/fsh if no fsh files are present, no root level ig-data folder, and no fsh subdirectory (current tank with no fsh files)', () => {
+      const input = path.join(tempRoot, 'no-fsh');
+      const foundInput = findInputDir(input);
+      expect(foundInput).toBe(path.join(tempRoot, 'no-fsh', 'input', 'fsh'));
+    });
   });
 
   describe('#ensureOutputDir()', () => {
@@ -335,6 +356,36 @@ describe('Processing', () => {
     });
   });
 
+  describe('#hasFshFiles()', () => {
+    let tempRoot: string;
+
+    beforeAll(() => {
+      tempRoot = temp.mkdirSync('sushi-test');
+      fs.mkdirSync(path.join(tempRoot, 'some-fsh'));
+      fs.mkdirSync(path.join(tempRoot, 'some-fsh', 'nested'));
+      fs.ensureFileSync(path.join(tempRoot, 'some-fsh', 'notfsh.txt'));
+      fs.ensureFileSync(path.join(tempRoot, 'some-fsh', 'nested', 'myfsh.fsh'));
+      fs.mkdirSync(path.join(tempRoot, 'no-fsh'));
+      fs.mkdirSync(path.join(tempRoot, 'no-fsh', 'nested'));
+      fs.ensureFileSync(path.join(tempRoot, 'no-fsh', 'notfsh.txt'));
+      fs.ensureFileSync(path.join(tempRoot, 'no-fsh', 'nested', 'notfsh.txt'));
+    });
+
+    afterAll(() => {
+      temp.cleanupSync();
+    });
+
+    it('should return true if FSH files exist in any subdirectory', () => {
+      const result = hasFshFiles(path.join(tempRoot, 'some-fsh'));
+      expect(result).toBe(true);
+    });
+
+    it('should return false if there are no FSH files in any subdirectory', () => {
+      const result = hasFshFiles(path.join(tempRoot, 'no-fsh'));
+      expect(result).toBe(false);
+    });
+  });
+
   describe('#writeFHIRResources()', () => {
     let tempRoot: string;
     let tempIGPubRoot: string;