Merge pull request #22 from umaar/scan-node-modules-for-asset-folders

Scan components to find asset folders to serve as static resources

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,6 +1,10 @@
 const path = require('path')
+const util = require('util')
+const fs = require('fs')
 const express = require('express')
 
+const stat = util.promisify(fs.stat)
+
 const staticOptions = {
   index: false,
   immutable: true,
@@ -12,21 +16,40 @@ const serveStatic = (assetsUrlPrefix, staticPath) => {
   router.use(assetsUrlPrefix, express.static(staticPath, staticOptions))
 }
 
-const routesStatic = (assetsUrlPrefix, staticPaths = [], serviceDir) => {
+async function init (options = {}) {
+  const {
+    assetsUrlPrefix,
+    staticPaths = [],
+    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')
+  const possibleAssetFolders = componentDirs.map(({sourcePath}) => {
+    return path.resolve(sourcePath, 'assets')
+  })
+
+  for (let resource of possibleAssetFolders) {
+    let isDirectory
+
+    try {
+      const resourceDetails = await stat(resource)
+      isDirectory = resourceDetails.isDirectory()
+    } catch (err) {
+      // Don't serve this folder as it doesn't exist
+    }
 
-  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'))
+    if (isDirectory) {
+      serveStatic(assetsUrlPrefix, resource)
+    }
+  }
 
   return router
 }
 
 module.exports = {
-  init: routesStatic
+  init
 }

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

@@ -1,27 +1,29 @@
-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 = [], servicePath) => {
+const callRoutesStatic = async ({assetsUrlPrefix = '', paths = []}) => {
   const app = express()
-  app.use(routesStatic.init(assetsUrlPrefix, paths, servicePath))
+  app.use(await routesStatic.init({
+    assetsUrlPrefix,
+    staticPaths: paths
+  }))
   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], __dirname)
+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,23 +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 = [], servicePath = __dirname) => {
+const callRoutesStatic = async ({assetsUrlPrefix = '', paths = []} = {}) => {
   const app = express()
-  app.use(routesStatic.init(assetsUrlPrefix, paths, servicePath))
+  app.use(await routesStatic.init({assetsUrlPrefix, staticPaths: paths}))
   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 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 = [], componentDirs}) => {
+  const app = express()
+  app.use(await routesStatic.init({assetsUrlPrefix, staticPaths: paths, 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({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({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({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,35 @@
-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 = [], servicePath) => {
+const callRoutesStatic = async ({assetsUrlPrefix = '', paths = []}) => {
   const app = express()
-  app.use(routesStatic.init(assetsUrlPrefix, paths, servicePath))
+  app.use(await routesStatic.init({assetsUrlPrefix, staticPaths: paths}))
   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')
+  t.equals(status, 200, 'it should return a successful status')
 })
 
-test('When a file that exists within a static route is requested', t => {
-  const app = callRoutesStatic('', [staticPath], __dirname)
-
-  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], __dirname)
-
-  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

@@ -235,7 +235,14 @@ 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,
+    staticPaths,
+    componentDirs
+  }
+
+  app.use(await routesStatic.init(routesStaticConfig))
 
   // Run arbitrary routes after static routes run
   if (options.postStaticRoutes) {