Scan components to find asset folders to serve as static resources

Dockerfile

@@ -1,4 +1,4 @@
-FROM node:10.11-slim
+FROM node:10.15-slim
 
 # derived from https://github.com/alekzonder/docker-puppeteer/blob/master/Dockerfile
 RUN apt-get update && \

README.md

@@ -1,6 +1,6 @@
 # fb-runner-node
 
-Form Builder Runner
+This Form Builder Runner repository is the backend which powers the forms which are deployed.
 
 ## Pre-requisites
 
@@ -16,8 +16,10 @@ npm install
 
 ## Usage
 
-```
-npm start
+Set the `SERVICE_PATH` environment variable to point the the path of a form on your filesystem.
+
+```sh
+SERVICE_PATH=~/Documents/formbuilder/forms/my-test-form-1 npm start
 ```
 
 ## Service data

lib/middleware/routes-static/routes-static.js

@@ -1,28 +1,60 @@
 const path = require('path')
 const express = require('express')
+const { stat } = require('fs').promises
 
 const staticOptions = {
   index: false,
   immutable: true,
   maxAge: '28d'
 }
 
+async function locateAssetFolders(componentDirs) {
+  const possibleAssetFolders = componentDirs.map(({sourcePath}) => {
+    return path.resolve(sourcePath, 'assets')
+  })
+
+  const assetFolders = possibleAssetFolders.filter(async (folder) => {
+    let isDirectory;
+
+    try {
+      isDirectory = (await stat(folder)).isDirectory()
+    } catch (err) {
+      // Don't serve asset folders which do not exist
+    }
+
+    return isDirectory;
+  })
+
+  return Promise.all(assetFolders)
+}
+
 const router = express.Router()
 const serveStatic = (assetsUrlPrefix, staticPath) => {
   router.use(assetsUrlPrefix, express.static(staticPath, staticOptions))
 }
 
-const routesStatic = (assetsUrlPrefix, staticPaths = [], serviceDir) => {
+async function routesStatic(options = {}) {
+  const {
+    assetsUrlPrefix,
+    staticPaths = [],
+    serviceDir,
+    componentDirs = []
+  } = options
+
   staticPaths.forEach(staticPath => {
     // TODO: add ability to pass an explicit url prefix { urlPrefix: '/foo', dir}
     serveStatic(assetsUrlPrefix, staticPath)
   })
 
-  let nodeModules = path.join(serviceDir, 'node_modules')
+  if (componentDirs.length) {
+    for (let directory of await locateAssetFolders(componentDirs)) {
+      serveStatic(assetsUrlPrefix, directory)
+    }
+  }
 
-  serveStatic(assetsUrlPrefix, path.resolve(nodeModules, '@ministryofjustice', 'fb-components-core', 'assets'))
-  serveStatic(assetsUrlPrefix, path.resolve(nodeModules, 'govuk-frontend', 'assets'))
-  serveStatic(assetsUrlPrefix, path.resolve(nodeModules, 'govuk-frontend'))
+  // TODO: Why are we serving this folder?
+  let nodeModulesPath = path.join(serviceDir, 'node_modules')
+  serveStatic(assetsUrlPrefix, path.resolve(nodeModulesPath, 'govuk-frontend'))
 
   return router
 }

lib/middleware/routes-static/routes-static.test.multiple-paths.unit.spec.js

@@ -1,27 +1,30 @@
-const test = require('tape')
+const test = require('tap').test
 const express = require('express')
 const request = require('supertest')
 const path = require('path')
 
 const routesStatic = require('./routes-static')
 
-const callRoutesStatic = (assetsUrlPrefix = '', paths = []) => {
+const callRoutesStatic = async ({assetsUrlPrefix = '', paths = [], serviceDir = __dirname}) => {
   const app = express()
-  app.use(routesStatic.init(assetsUrlPrefix, paths))
+  app.use(await routesStatic.init({
+    assetsUrlPrefix,
+    staticPaths: paths,
+    serviceDir
+  }))
   return app
 }
 
 const staticPath = path.resolve(__dirname, '..', 'spec', 'static', 'static-a')
 const staticPathB = path.resolve(__dirname, '..', 'spec', 'static', 'static-b')
 
-test('When a file that exists within more than one static route is requested', t => {
-  const app = callRoutesStatic('', [staticPathB, staticPath])
+test('When a file that exists within more than one static route is requested', async t => {
+  const app = await callRoutesStatic({
+    assetsUrlPrefix: '',
+    paths: [staticPathB, staticPath]
+  })
 
-  request(app)
-    .get('/static.txt')
-    .end((err, res) => {
-      t.equals(err, null, 'it should not invoke an error')
-      t.equals(res.text, 'static-b static.txt', 'it should serve the correct file')
-      t.end()
-    })
+  const {error, text} = await request(app).get('/static.txt')
+  t.equals(error, false, 'it should not invoke an error')
+  t.equals(text, 'static-b static.txt', 'it should serve the correct file')
 })

lib/middleware/routes-static/routes-static.test.no-paths.unit.spec.js

@@ -1,36 +1,18 @@
-const test = require('tape')
+const test = require('tap').test
 const express = require('express')
 const request = require('supertest')
 
 const routesStatic = require('./routes-static')
 
-const callRoutesStatic = (assetsUrlPrefix = '', paths = []) => {
+const callRoutesStatic = async ({assetsUrlPrefix = '', paths = [], serviceDir = __dirname} = {}) => {
   const app = express()
-  app.use(routesStatic.init(assetsUrlPrefix, paths))
+  app.use(await routesStatic.init({assetsUrlPrefix, staticPaths: paths, serviceDir}))
   return app
 }
 
-test('When a file that does not exist within a static route is requested', t => {
-  const app = callRoutesStatic()
-
-  request(app)
-    .get('/static.txt')
-    .end((err, res) => {
-      t.equals(err, null, 'it should not invoke an error')
-      t.equals(res.status, 404, 'it should return 404')
-      t.end()
-    })
-})
-
-test('When a file from govukfrontend is requested', t => {
-  const app = callRoutesStatic()
-
-  request(app)
-    .get('/images/govuk-crest-2x.png')
-    .end((err, res) => {
-      t.equals(err, null, 'it should not invoke an error')
-      t.equals(res.status, 200, 'it should return 200')
-      t.equals(res.headers['content-type'], 'image/png', 'it should return the correct mime type')
-      t.end()
-    })
+test('When a file that does not exist within a static route is requested', async t => {
+  const app = await callRoutesStatic()
+  const {error, status} = await request(app).get('/static.txt')
+  t.ok(error, null, 'it should invoke an error')
+  t.equals(status, 404, 'it should return 404')
 })

lib/middleware/routes-static/routes-static.test.node-module-asset-serving.unit.spec.js

@@ -0,0 +1,55 @@
+const test = require('tap').test
+const path = require('path')
+const express = require('express')
+const request = require('supertest')
+
+const routesStatic = require('./routes-static')
+
+const callRoutesStatic = async ({assetsUrlPrefix = '', paths = [], serviceDir = __dirname, componentDirs}) => {
+  const app = express()
+  app.use(await routesStatic.init({assetsUrlPrefix, staticPaths: paths, serviceDir, componentDirs}))
+  return app
+}
+
+const staticPath = path.resolve(__dirname, '..', 'spec')
+
+test('When a file from an assets folder is requested', async t => {
+  const componentDirs = [{
+    sourcePath: path.resolve(staticPath, 'static', 'static-b')
+  }]
+
+  const app = await callRoutesStatic({serviceDir: staticPath, componentDirs})
+
+  const {error, status, headers} = await request(app).get('/image-resource.png')
+  t.equals(error, false, 'it should not invoke an error')
+  t.equals(status, 200, 'it should return 200')
+  t.equals(headers['content-type'], 'image/png', 'it should return the correct mime type')
+})
+
+test('When a specified node module does not have an assets folder', async t => {
+  const componentDirs = [{
+    sourcePath: path.resolve(staticPath, 'static', 'static-a')
+  }]
+
+  const app = await callRoutesStatic({serviceDir: staticPath, componentDirs})
+
+  const {status: status1} = await request(app).get('/static.txt')
+  t.equals(status1, 404, 'it should not serve assets at the root level')
+
+  const {status: status2} = await request(app).get('/assets/static.txt')
+  t.equals(status2, 404, 'it should not serve assets at any URL')
+})
+
+test('When an asset folder is present within another asset folder', async t => {
+  const componentDirs = [{
+    sourcePath: path.resolve(staticPath, 'static', 'static-b')
+  }]
+
+  const app = await callRoutesStatic({serviceDir: staticPath, componentDirs})
+
+  const {status: statusNotFound} = await request(app).get('/image-resource-2.png')
+  t.equals(statusNotFound, 404, 'it should not be found at the top level')
+
+  const {status: statusFound} = await request(app).get('/assets/image-resource-2.png')
+  t.equals(statusFound, 200, 'it should be found by specifying the nested asset folder')
+})

lib/middleware/routes-static/routes-static.unit.spec.js

@@ -1,44 +1,34 @@
-const test = require('tape')
+const test = require('tap').test
 const express = require('express')
 const request = require('supertest')
 const path = require('path')
 
 const routesStatic = require('./routes-static')
 
-const callRoutesStatic = (assetsUrlPrefix = '', paths = []) => {
+const callRoutesStatic = async ({assetsUrlPrefix = '', paths = [], serviceDir = __dirname}) => {
   const app = express()
-  app.use(routesStatic.init(assetsUrlPrefix, paths))
+  app.use(await routesStatic.init({assetsUrlPrefix, staticPaths: paths, serviceDir}))
   return app
 }
 
 const staticPath = path.resolve(__dirname, '..', 'spec', 'static', 'static-a')
 
-test('When routesStatic is required ', t => {
-  t.equal(typeof routesStatic.init, 'function', 'it should export the init method')
-
-  t.end()
+test('When a file that exists within a static route is requested', async t => {
+  const app = await callRoutesStatic({
+    assetsUrlPrefix: '',
+    paths: [staticPath]
+  })
+  const {error, text, status} = await request(app).get('/static.txt')
+  t.equals(error, false, 'it should not invoke an error')
+  t.equals(text, 'static-a static.txt', 'it should serve the file')
 })
 
-test('When a file that exists within a static route is requested', t => {
-  const app = callRoutesStatic('', [staticPath])
-
-  request(app)
-    .get('/static.txt')
-    .end((err, res) => {
-      t.equals(err, null, 'it should not invoke an error')
-      t.equals(res.text, 'static-a static.txt', 'it should serve the file')
-      t.end()
-    })
-})
-
-test('When a url prefix is passed to routes-static', t => {
-  const app = callRoutesStatic('/url/prefix', [staticPath])
-
-  request(app)
-    .get('/url/prefix/static.txt')
-    .end((err, res) => {
-      t.equals(err, null, 'it should not invoke an error')
-      t.equals(res.text, 'static-a static.txt', 'it should use that prefix and serve the correct file')
-      t.end()
-    })
+test('When a url prefix is passed to routes-static', async t => {
+  const app = await callRoutesStatic({
+    assetsUrlPrefix: '/url/prefix',
+    paths: [staticPath]
+  })
+  const {error, text} = await request(app).get('/url/prefix/static.txt')
+  t.equals(error, false, 'it should not invoke an error')
+  t.equals(text, 'static-a static.txt', 'it should use that prefix and serve the correct file')
 })

lib/server/server.js

@@ -226,7 +226,15 @@ const configureMiddleware = async (options = {}) => {
     path.join(appDir, 'app', 'assets'),
     path.join(kitDir, 'app', 'assets')
   ]
-  app.use(routesStatic.init(ASSET_SRC_PATH, staticPaths, servicePath))
+
+  const routesStaticConfig = {
+    assetsUrlPrefix: ASSET_SRC_PATH,
+    serviceDir: servicePath,
+    staticPaths,
+    componentDirs
+  }
+
+  app.use(await routesStatic.init(routesStaticConfig))
 
   // Run arbitrary routes after static routes run
   if (options.postStaticRoutes) {