Repository: FHIR/sushi
Pull request #226 (2020-02-17 15:15:58)

Sort pages in IG content
Numeric prefixes in IG content files enable sorting. The prefixes get removed during processing, unless doing so would cause problems. Because this code involves dealing with IG content files, there are some small content files added in to support the tests.

The case where two files have the same name other than their extension, for example `Something.xml` and `Something.md`, is not accounted for here.

This pull request is for task https://standardhealthrecord.atlassian.net/browse/CIMPL-268.

mint-thompson: Local type defined! I also fixed up a few of my jsdoc comments that were in the wrong order.

Patch:
diff --git a/src/ig/IGExporter.ts b/src/ig/IGExporter.ts
index 59b7e74..8c8838f 100644
--- a/src/ig/IGExporter.ts
+++ b/src/ig/IGExporter.ts
@@ -216,34 +216,32 @@ export class IGExporter {
    * Only add formats that are supported by the IG template
    * Intro and notes file contents are injected into relevant pages and should not be treated as their own page
    *
-   * @param igPath {string} - the path where the IG is exported to
+   * @param {string} igPath - the path where the IG is exported to
    */
   private addOtherPageContent(igPath: string): void {
     const inputPageContentPath = path.join(this.igDataPath, 'input', 'pagecontent');
     if (fs.existsSync(inputPageContentPath)) {
-      const pages = fs
-        .readdirSync(inputPageContentPath)
-        .filter(page => page !== 'index.md' && page !== 'index.xml')
-        .sort(); // Sorts alphabetically
+      const organizedPages = this.organizePageContent(fs.readdirSync(inputPageContentPath));
 
       let invalidFileTypeIncluded = false;
-      pages.forEach(page => {
+      organizedPages.forEach(page => {
         // All user defined pages are included in input/pagecontent
-        const pagePath = path.join(this.igDataPath, 'input', 'pagecontent', page);
-        fs.copySync(pagePath, path.join(igPath, 'input', 'pagecontent', page));
+        const pagePath = path.join(this.igDataPath, 'input', 'pagecontent', page.originalName);
 
-        const fileName = page.slice(0, page.lastIndexOf('.'));
-        const fileType = page.slice(page.lastIndexOf('.') + 1);
-        const isSupportedFileType = fileType === 'md' || fileType === 'xml';
-        const isIntroOrNotesFile = fileName.endsWith('-intro') || fileName.endsWith('-notes');
+        fs.copySync(
+          pagePath,
+          path.join(igPath, 'input', 'pagecontent', `${page.name}.${page.fileType}`)
+        );
+        const isSupportedFileType = page.fileType === 'md' || page.fileType === 'xml';
+        const isIntroOrNotesFile = page.name.endsWith('-intro') || page.name.endsWith('-notes');
         if (isSupportedFileType) {
           // Intro and notes files will be in supported formats but are not separate pages, so they should not be added to IG definition
           if (!isIntroOrNotesFile) {
             // Valid page files will be added to the IG definition
             this.ig.definition.page.page.push({
-              nameUrl: `${fileName}.html`,
-              title: `${titleCase(words(fileName).join(' '))}`,
-              generation: fileType === 'md' ? 'markdown' : 'html'
+              nameUrl: `${page.name}.html`,
+              title: page.title,
+              generation: page.fileType === 'md' ? 'markdown' : 'html'
             });
           }
         } else {
@@ -260,6 +258,80 @@ export class IGExporter {
     }
   }
 
+  /**
+   * Sorts and renames pages based on numeric prefixes.
+   * Numeric prefixes are used for applying a sort order, but should be removed
+   * from the page's name and title unless doing so would cause a name collision.
+   *
+   * @param {string[]} pages - list of file names with extensions
+   * @returns {PageMetadata []} - sorted list of file information objects
+   */
+  private organizePageContent(pages: string[]): PageMetadata[] {
+    const pageData = pages.map(page => {
+      const nameParts = page.match(/^(\d+)_(.*)/);
+      let prefix: number = null;
+      let name: string;
+      if (nameParts == null) {
+        name = page.slice(0, page.lastIndexOf('.'));
+      } else {
+        prefix = parseInt(nameParts[1]);
+        name = nameParts[2].slice(0, nameParts[2].lastIndexOf('.'));
+      }
+      return {
+        originalName: page,
+        prefix: prefix,
+        name: name,
+        title: titleCase(words(name).join(' ')),
+        fileType: page.slice(page.lastIndexOf('.') + 1)
+      };
+    });
+    let mightHaveDuplicates = true;
+    while (mightHaveDuplicates) {
+      mightHaveDuplicates = false;
+      pageData.forEach(page => {
+        const sameName = pageData.filter(otherPage => otherPage.name == page.name);
+        if (sameName.length > 1) {
+          mightHaveDuplicates = true;
+          sameName.forEach(matchingPage => {
+            matchingPage.name = matchingPage.originalName.slice(
+              0,
+              matchingPage.originalName.lastIndexOf('.')
+            );
+          });
+        }
+      });
+    }
+    return pageData.filter(page => page.name !== 'index').sort(this.compareIgFilenames);
+  }
+
+  /**
+   * Compares two file names, each of which may be prefixed with a number.
+   * If neither file has a prefix, compares the file names alphabetically.
+   * If one file has a prefix, that file is before the other.
+   * If both have a prefix, compares the prefixes numerically.
+   * If the prefixes are equal, resolves the tie by comparing the file names alphabetically.
+   *
+   * @param {PageMetadata} pageA - metadata for first file
+   * @param {PageMetadata} pageB - metadata for second file
+   * @returns {number} - positive when file b comes first, negative when file a comes first, zero when the file names are equal.
+   */
+  private compareIgFilenames(pageA: PageMetadata, pageB: PageMetadata): number {
+    if (pageA.prefix == null && pageB.prefix == null) {
+      return pageA.name.localeCompare(pageB.name);
+    } else if (pageA.prefix == null) {
+      return 1;
+    } else if (pageB.prefix == null) {
+      return -1;
+    } else {
+      const prefixComparison = pageA.prefix - pageB.prefix;
+      if (prefixComparison == 0) {
+        return pageA.name.localeCompare(pageB.name);
+      } else {
+        return prefixComparison;
+      }
+    }
+  }
+
   /**
    * Adds any user provided images that can be referenced directly in other pages
    *
@@ -465,3 +537,11 @@ export class IGExporter {
     logger.info('Generated default package-list.json');
   }
 }
+
+interface PageMetadata {
+  originalName: string;
+  prefix: number;
+  name: string;
+  title: string;
+  fileType: string;
+}


Test patch:
diff --git a/test/ig/IGExporter.test.ts b/test/ig/IGExporter.test.ts
index caab270..a86d4bd 100644
--- a/test/ig/IGExporter.test.ts
+++ b/test/ig/IGExporter.test.ts
@@ -571,4 +571,164 @@ describe('IGExporter', () => {
       });
     });
   });
+
+  describe('#sorted-pages-ig', () => {
+    let pkg: Package;
+    let exporter: IGExporter;
+    let tempOut: string;
+
+    beforeAll(() => {
+      const fixtures = path.join(__dirname, 'fixtures', 'sorted-pages-ig');
+      const config: Config = fs.readJSONSync(path.join(fixtures, 'package.json'));
+      pkg = new Package(config);
+      exporter = new IGExporter(pkg, new FHIRDefinitions(), path.resolve(fixtures, 'ig-data'));
+      tempOut = temp.mkdirSync('sushi-test');
+      // No need to regenerate the IG on every test -- generate it once and inspect what you
+      // need to in the tests
+      exporter.export(tempOut);
+    });
+
+    afterAll(() => {
+      temp.cleanupSync();
+    });
+
+    it('should add user-provided pages in the user-specified order', () => {
+      const pageContentPath = path.join(tempOut, 'input', 'pagecontent');
+      expect(fs.existsSync(pageContentPath)).toBeTruthy();
+
+      const igPath = path.join(tempOut, 'input', 'ImplementationGuide-sushi-test.json');
+      expect(fs.existsSync(igPath)).toBeTruthy();
+      const igContent = fs.readJSONSync(igPath);
+      expect(igContent.definition.page.page).toHaveLength(9);
+      expect(igContent.definition.page.page).toEqual([
+        {
+          nameUrl: 'index.html',
+          title: 'FSH Test IG',
+          generation: 'html'
+        },
+        {
+          nameUrl: 'oranges.html',
+          title: 'Oranges',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: 'apples.html',
+          title: 'Apples',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: 'bananas.html',
+          title: 'Bananas',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: 'pears.html',
+          title: 'Pears',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: 'left.html',
+          title: 'Left',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: 'right.html',
+          title: 'Right',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: 'big.html',
+          title: 'Big',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: 'pasta.html',
+          title: 'Pasta',
+          generation: 'markdown'
+        }
+      ]);
+    });
+
+    it('should remove numeric prefixes from copied files', () => {
+      const pageContentPath = path.join(tempOut, 'input', 'pagecontent');
+      expect(fs.existsSync(pageContentPath)).toBeTruthy();
+      const pageContentFiles = fs.readdirSync(pageContentPath);
+      expect(pageContentFiles).toHaveLength(9);
+      expect(pageContentFiles).toContain('index.xml');
+      expect(pageContentFiles).toContain('oranges.md');
+      expect(pageContentFiles).toContain('apples.md');
+      expect(pageContentFiles).toContain('bananas.md');
+      expect(pageContentFiles).toContain('pears.md');
+      expect(pageContentFiles).toContain('left.md');
+      expect(pageContentFiles).toContain('right.md');
+      expect(pageContentFiles).toContain('big.md');
+      expect(pageContentFiles).toContain('pasta.md');
+    });
+  });
+  describe('#name-collision-ig', () => {
+    let pkg: Package;
+    let exporter: IGExporter;
+    let tempOut: string;
+
+    beforeAll(() => {
+      const fixtures = path.join(__dirname, 'fixtures', 'name-collision-ig');
+      const config: Config = fs.readJSONSync(path.join(fixtures, 'package.json'));
+      pkg = new Package(config);
+      exporter = new IGExporter(pkg, new FHIRDefinitions(), path.resolve(fixtures, 'ig-data'));
+      tempOut = temp.mkdirSync('sushi-test');
+      // No need to regenerate the IG on every test -- generate it once and inspect what you
+      // need to in the tests
+      exporter.export(tempOut);
+    });
+
+    afterAll(() => {
+      temp.cleanupSync();
+    });
+
+    it('should not remove numeric prefixes from page names when doing so would cause name collisions', () => {
+      const igPath = path.join(tempOut, 'input', 'ImplementationGuide-sushi-test.json');
+      expect(fs.existsSync(igPath)).toBeTruthy();
+      const igContent = fs.readJSONSync(igPath);
+      expect(igContent.definition.page.page).toHaveLength(5);
+      expect(igContent.definition.page.page).toEqual([
+        {
+          nameUrl: 'index.html',
+          title: 'FSH Test IG',
+          generation: 'html'
+        },
+        {
+          nameUrl: '1_rocks.html',
+          title: 'Rocks',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: '2_rocks.html',
+          title: 'Rocks',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: '3_index.html',
+          title: 'Index',
+          generation: 'markdown'
+        },
+        {
+          nameUrl: '4_2_rocks.html',
+          title: '2 Rocks',
+          generation: 'markdown'
+        }
+      ]);
+    });
+
+    it('should not remove numeric prefixes from files when doing so would cause name collisions', () => {
+      const pageContentPath = path.join(tempOut, 'input', 'pagecontent');
+      expect(fs.existsSync(pageContentPath)).toBeTruthy();
+      const pageContentFiles = fs.readdirSync(pageContentPath);
+      expect(pageContentFiles).toHaveLength(5);
+      expect(pageContentFiles).toContain('index.xml');
+      expect(pageContentFiles).toContain('1_rocks.md');
+      expect(pageContentFiles).toContain('2_rocks.md');
+      expect(pageContentFiles).toContain('3_index.md');
+      expect(pageContentFiles).toContain('4_2_rocks.md');
+    });
+  });
 });
diff --git a/test/ig/fixtures/name-collision-ig/ig-data/ig.ini b/test/ig/fixtures/name-collision-ig/ig-data/ig.ini
new file mode 100644
index 0000000..00c4ddf
--- /dev/null
+++ b/test/ig/fixtures/name-collision-ig/ig-data/ig.ini
@@ -0,0 +1,13 @@
+[IG]
+ig = input/ImplementationGuide-sushi-test.json
+template = hl7.fhir.template
+usage-stats-opt-out = true
+copyrightyear = 2018+
+license = CC0-1.0
+version = 0.1.0
+ballotstatus = STU1
+fhirspec = http://hl7.org/fhir/R4/
+excludexml = Yes
+excludejson = Yes
+excludettl = Yes
+excludeMaps = Yes
diff --git a/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/1_rocks.md b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/1_rocks.md
new file mode 100644
index 0000000..8b52004
--- /dev/null
+++ b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/1_rocks.md
@@ -0,0 +1,1 @@
+This is the first file with rocks.
diff --git a/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/2_rocks.md b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/2_rocks.md
new file mode 100644
index 0000000..4382b95
--- /dev/null
+++ b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/2_rocks.md
@@ -0,0 +1,1 @@
+This is the second file with rocks
diff --git a/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/3_index.md b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/3_index.md
new file mode 100644
index 0000000..c8a7ee8
--- /dev/null
+++ b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/3_index.md
@@ -0,0 +1,1 @@
+This is another index file, which is probably a bad idea.
diff --git a/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/4_2_rocks.md b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/4_2_rocks.md
new file mode 100644
index 0000000..864df1c
--- /dev/null
+++ b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/4_2_rocks.md
@@ -0,0 +1,1 @@
+This is yet another file with even more rocks.
diff --git a/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/index.xml b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/index.xml
new file mode 100644
index 0000000..1632bba
--- /dev/null
+++ b/test/ig/fixtures/name-collision-ig/ig-data/input/pagecontent/index.xml
@@ -0,0 +1,6 @@
+<div xmlns="http://www.w3.org/1999/xhtml" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://hl7.org/fhir ../../input-cache/schemas/R5/fhir-single.xsd">
+  <h3>Index File</h3>
+	<p>
+    An index file in XML
+  </p>
+</div>
\ No newline at end of file
diff --git a/test/ig/fixtures/name-collision-ig/package.json b/test/ig/fixtures/name-collision-ig/package.json
new file mode 100644
index 0000000..50ae5cf
--- /dev/null
+++ b/test/ig/fixtures/name-collision-ig/package.json
@@ -0,0 +1,20 @@
+{
+  "name": "sushi-test",
+  "version": "0.1.0",
+  "canonical": "http://hl7.org/fhir/sushi-test",
+  "url": "http://hl7.org/fhir/sushi-test",
+  "title": "FSH Test IG",
+  "description": "Provides a simple example of how FSH can be used to create an IG",
+  "dependencies": {
+    "hl7.fhir.r4.core": "4.0.1"
+  },
+  "language": "en",
+  "author": "James Tuna",
+  "maintainers": [
+    {
+      "name": "Bill Cod",
+      "email": "cod@reef.gov"
+    }
+  ],
+  "license": "CC0-1.0"
+}
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/ig.ini b/test/ig/fixtures/sorted-pages-ig/ig-data/ig.ini
new file mode 100644
index 0000000..00c4ddf
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/ig.ini
@@ -0,0 +1,13 @@
+[IG]
+ig = input/ImplementationGuide-sushi-test.json
+template = hl7.fhir.template
+usage-stats-opt-out = true
+copyrightyear = 2018+
+license = CC0-1.0
+version = 0.1.0
+ballotstatus = STU1
+fhirspec = http://hl7.org/fhir/R4/
+excludexml = Yes
+excludejson = Yes
+excludettl = Yes
+excludeMaps = Yes
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/100_big.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/100_big.md
new file mode 100644
index 0000000..d927052
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/100_big.md
@@ -0,0 +1,1 @@
+One hundred is bigger than the other numbers.
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/1_oranges.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/1_oranges.md
new file mode 100644
index 0000000..d99bb30
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/1_oranges.md
@@ -0,0 +1,1 @@
+This file is all about oranges.
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/2_apples.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/2_apples.md
new file mode 100644
index 0000000..71dd47b
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/2_apples.md
@@ -0,0 +1,1 @@
+This file is all about apples.
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/3_bananas.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/3_bananas.md
new file mode 100644
index 0000000..6a7c517
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/3_bananas.md
@@ -0,0 +1,1 @@
+This file is all about bananas.
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/4_pears.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/4_pears.md
new file mode 100644
index 0000000..a6dab3a
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/4_pears.md
@@ -0,0 +1,1 @@
+This file is all about pears.
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/7_left.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/7_left.md
new file mode 100644
index 0000000..ad4ae42
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/7_left.md
@@ -0,0 +1,1 @@
+This file points to the left.
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/7_right.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/7_right.md
new file mode 100644
index 0000000..7e3bd19
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/7_right.md
@@ -0,0 +1,1 @@
+This file points to the right.
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/index.xml b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/index.xml
new file mode 100644
index 0000000..1632bba
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/index.xml
@@ -0,0 +1,6 @@
+<div xmlns="http://www.w3.org/1999/xhtml" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://hl7.org/fhir ../../input-cache/schemas/R5/fhir-single.xsd">
+  <h3>Index File</h3>
+	<p>
+    An index file in XML
+  </p>
+</div>
\ No newline at end of file
diff --git a/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/pasta.md b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/pasta.md
new file mode 100644
index 0000000..3b73fc3
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/ig-data/input/pagecontent/pasta.md
@@ -0,0 +1,1 @@
+Pasta is not a type of fruit.
diff --git a/test/ig/fixtures/sorted-pages-ig/package.json b/test/ig/fixtures/sorted-pages-ig/package.json
new file mode 100644
index 0000000..50ae5cf
--- /dev/null
+++ b/test/ig/fixtures/sorted-pages-ig/package.json
@@ -0,0 +1,20 @@
+{
+  "name": "sushi-test",
+  "version": "0.1.0",
+  "canonical": "http://hl7.org/fhir/sushi-test",
+  "url": "http://hl7.org/fhir/sushi-test",
+  "title": "FSH Test IG",
+  "description": "Provides a simple example of how FSH can be used to create an IG",
+  "dependencies": {
+    "hl7.fhir.r4.core": "4.0.1"
+  },
+  "language": "en",
+  "author": "James Tuna",
+  "maintainers": [
+    {
+      "name": "Bill Cod",
+      "email": "cod@reef.gov"
+    }
+  ],
+  "license": "CC0-1.0"
+}