Skip to content

Commit

Permalink
refactor(server): Provide file-service handlers in the root injector. (
Browse files Browse the repository at this point in the history
…#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.
  • Loading branch information
johnjbarton authored Jun 15, 2018
1 parent c1eb236 commit a32ba27
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 15 deletions.
6 changes: 6 additions & 0 deletions lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
39 changes: 25 additions & 14 deletions lib/web-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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
}
12 changes: 11 additions & 1 deletion test/unit/web-server.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -83,7 +86,6 @@ describe('web-server', () => {
}
}]
}])

server = injector.invoke(m.createWebServer)
})

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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],
Expand Down

0 comments on commit a32ba27

Please sign in to comment.