From a32ba27eeefcfe1e3ecd1ec7529a269ac13c0027 Mon Sep 17 00:00:00 2001
From: johnjbarton <johnjbarton@johnjbarton.com>
Date: Fri, 15 Jun 2018 11:13:25 -0700
Subject: [PATCH] refactor(server): Provide file-service handlers in the root
 injector. (#3042)

By removing the creation of file-service handlers from the webserver, we create functions with better focus
and we allow the file-service handlers to be used in beforeMiddlewaare. The only cost is that these objects are
now visible to all modules rather than being private to webserver. The potential for collision seems small.
---
 lib/server.js                |  6 ++++++
 lib/web-server.js            | 39 +++++++++++++++++++++++-------------
 test/unit/web-server.spec.js | 12 ++++++++++-
 3 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/lib/server.js b/lib/server.js
index f504f05f8..05af65c0c 100644
--- a/lib/server.js
+++ b/lib/server.js
@@ -18,6 +18,9 @@ const constant = require('./constants')
 const watcher = require('./watcher')
 const plugin = require('./plugin')
 
+const createServeFile = require('./web-server').createServeFile
+const createServeStaticFile = require('./web-server').createServeStaticFile
+const createFilesPromise = require('./web-server').createFilesPromise
 const createWebServer = require('./web-server').createWebServer
 const preprocessor = require('./preprocessor')
 const Launcher = require('./launcher').Launcher
@@ -72,6 +75,9 @@ class Server extends KarmaEventEmitter {
       preprocess: ['factory', preprocessor.createPreprocessor],
       fileList: ['factory', FileList.factory],
       webServer: ['factory', createWebServer],
+      serveFile: ['factory', createServeFile],
+      serveStaticFile: ['factory', createServeStaticFile],
+      filesPromise: ['factory', createFilesPromise],
       socketServer: ['factory', createSocketIoServer],
       executor: ['factory', Executor.factory],
       // TODO(vojta): remove
diff --git a/lib/web-server.js b/lib/web-server.js
index 9534ff9a9..bd63481d7 100644
--- a/lib/web-server.js
+++ b/lib/web-server.js
@@ -28,11 +28,7 @@ function createCustomHandler (customFileHandlers, config) {
 
 createCustomHandler.$inject = ['customFileHandlers', 'config']
 
-function createWebServer (injector, emitter, fileList) {
-  const config = injector.get('config')
-  common.initializeMimeTypes(config)
-  const serveStaticFile = common.createServeFile(fs, path.normalize(path.join(__dirname, '/../static')), config)
-  const serveFile = common.createServeFile(fs, null, config)
+function createFilesPromise (emitter, fileList) {
   const filesPromise = new common.PromiseContainer()
 
   // Set an empty list of files to avoid race issues with
@@ -41,13 +37,22 @@ function createWebServer (injector, emitter, fileList) {
 
   emitter.on('file_list_modified', (files) => filesPromise.set(Promise.resolve(files)))
 
-  // locals for webserver module
-  // NOTE(vojta): figure out how to do this with DI
-  injector = injector.createChild([{
-    serveFile: ['value', serveFile],
-    serveStaticFile: ['value', serveStaticFile],
-    filesPromise: ['value', filesPromise]
-  }])
+  return filesPromise
+}
+createFilesPromise.$inject = ['emitter', 'fileList']
+
+function createServeStaticFile (config) {
+  return common.createServeFile(fs, path.normalize(path.join(__dirname, '/../static')), config)
+}
+createServeStaticFile.$inject = ['config']
+
+function createServeFile (config) {
+  return common.createServeFile(fs, null, config)
+}
+createServeFile.$inject = ['config']
+
+function createWebServer (injector, config) {
+  common.initializeMimeTypes(config)
 
   const proxyMiddlewareInstance = injector.invoke(proxyMiddleware.create)
 
@@ -97,5 +102,11 @@ function createWebServer (injector, emitter, fileList) {
   return server
 }
 
-createWebServer.$inject = ['injector', 'emitter', 'fileList']
-exports.createWebServer = createWebServer
+createWebServer.$inject = ['injector', 'config']
+
+module.exports = {
+  createWebServer,
+  createServeFile,
+  createServeStaticFile,
+  createFilesPromise
+}
diff --git a/test/unit/web-server.spec.js b/test/unit/web-server.spec.js
index d429e443c..6d56cb04c 100644
--- a/test/unit/web-server.spec.js
+++ b/test/unit/web-server.spec.js
@@ -60,6 +60,9 @@ describe('web-server', () => {
         customFileHandlers: ['value', customFileHandlers],
         emitter: ['value', emitter],
         fileList: ['value', {files: {served: [], included: []}}],
+        filesPromise: ['factory', m.createFilesPromise],
+        serveStaticFile: ['factory', m.createServeStaticFile],
+        serveFile: ['factory', m.createServeFile],
         capturedBrowsers: ['value', null],
         reporter: ['value', null],
         executor: ['value', null],
@@ -83,7 +86,6 @@ describe('web-server', () => {
           }
         }]
       }])
-
       server = injector.invoke(m.createWebServer)
     })
 
@@ -167,6 +169,7 @@ describe('web-server', () => {
     })
 
     it('should serve no files when they are not available yet', () => {
+      servedFiles(new Set())
       return request(server)
         .get('/base/new.js')
         .expect(404)
@@ -226,6 +229,10 @@ describe('web-server', () => {
         customFileHandlers: ['value', customFileHandlers],
         emitter: ['value', emitter],
         fileList: ['value', {files: {served: [], included: []}}],
+        filesPromise: ['factory', m.createFilesPromise],
+        serveStaticFile: ['factory', m.createServeStaticFile],
+        serveFile: ['factory', m.createServeFile],
+
         capturedBrowsers: ['value', null],
         reporter: ['value', null],
         executor: ['value', null],
@@ -267,6 +274,9 @@ describe('web-server', () => {
         customFileHandlers: ['value', customFileHandlers],
         emitter: ['value', emitter],
         fileList: ['value', {files: {served: [], included: []}}],
+        filesPromise: ['factory', m.createFilesPromise],
+        serveStaticFile: ['factory', m.createServeStaticFile],
+        serveFile: ['factory', m.createServeFile],
         capturedBrowsers: ['value', null],
         reporter: ['value', null],
         executor: ['value', null],