Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: restore customFileHandlers provider #3624

Merged
merged 1 commit into from
Jan 20, 2021
Merged

fix: restore customFileHandlers provider #3624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class Server extends KarmaEventEmitter {
filesPromise: ['factory', createFilesPromise],
socketServer: ['factory', createSocketIoServer],
executor: ['factory', Executor.factory],
// TODO: Deprecated. Remove in the next major
customFileHandlers: ['value', []],
reporter: ['factory', reporter.createReporters],
capturedBrowsers: ['factory', BrowserCollection.factory],
args: ['value', {}],
Expand Down
21 changes: 21 additions & 0 deletions lib/web-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,25 @@ const proxyMiddleware = require('./middleware/proxy')

const log = require('./logger').create('web-server')

function createCustomHandler (customFileHandlers, config) {
let warningDone = false

return function (request, response, next) {
const handler = customFileHandlers.find((handler) => handler.urlRegex.test(request.url))

if (customFileHandlers.length > 0 && !warningDone) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The awkward logic is here because handlers are added dynamically (e.g. https://github.com/angular/angular-cli/pull/19784/files#diff-7e99b8c9ae4a43ae87d525fccfd19e3c7449e3f12b3d0e07aedf208a8a451f3cL223) and we can't rely on the presence of the provider either, because it is always there.

So I resorted to checking that there is at least one custom handler is present when handling a request and printing the warning only once to avoid spamming the logs.

The check is not perfect as the warning is printed only when this handler is reached but this should be good enough in practice.

warningDone = true
log.warn('The `customFileHandlers` is deprecated and will be removed in Karma 7. Please upgrade plugins relying on this provider.')
}

return handler
? handler.handler(request, response, 'fake/static', 'fake/adapter', config.basePath, 'fake/root')
: next()
}
}

createCustomHandler.$inject = ['customFileHandlers', 'config']

function createFilesPromise (emitter, fileList) {
// Set an empty list of files to avoid race issues with
// file_list_modified not having been emitted yet
Expand Down Expand Up @@ -58,6 +77,8 @@ function createWebServer (injector, config) {
handler.use(injector.invoke(sourceFilesMiddleware.create))
// TODO(vojta): extract the proxy into a plugin
handler.use(proxyMiddlewareInstance)
// TODO: Deprecated. Remove in the next major
handler.use(injector.invoke(createCustomHandler))

if (config.middleware) {
config.middleware.forEach((middleware) => handler.use(injector.get('middleware:' + middleware)))
Expand Down
24 changes: 23 additions & 1 deletion test/unit/web-server.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('web-server', () => {
// NOTE(vojta): only loading once, to speed things up
// this relies on the fact that none of these tests mutate fs
const m = mocks.loadFile(path.join(__dirname, '/../../lib/web-server.js'), _mocks, _globals)
server = emitter = null
let customFileHandlers = server = emitter = null
let beforeMiddlewareActive = false
let middlewareActive = false
const servedFiles = (files) => {
Expand All @@ -40,6 +40,7 @@ describe('web-server', () => {

describe('request', () => {
beforeEach(() => {
customFileHandlers = []
emitter = new EventEmitter()
const config = {
basePath: '/base/path',
Expand All @@ -56,6 +57,7 @@ describe('web-server', () => {

const injector = new di.Injector([{
config: ['value', config],
customFileHandlers: ['value', customFileHandlers],
emitter: ['value', emitter],
fileList: ['value', { files: { served: [], included: [] } }],
filesPromise: ['factory', m.createFilesPromise],
Expand Down Expand Up @@ -180,6 +182,22 @@ describe('web-server', () => {
})
})

it('should load custom handlers', () => {
servedFiles(new Set())

customFileHandlers.push({
urlRegex: /\/some\/weird/,
handler (request, response, staticFolder, adapterFolder, baseFolder, urlRoot) {
response.writeHead(222)
response.end('CONTENT')
}
})

return request(server)
.get('/some/weird/url')
.expect(222, 'CONTENT')
})

it('should serve 404 for non-existing files', () => {
servedFiles(new Set())

Expand All @@ -196,6 +214,7 @@ describe('web-server', () => {
cert: fs.readFileSync(path.join(__dirname, '/certificates/server.crt'))
}

customFileHandlers = []
emitter = new EventEmitter()

const injector = new di.Injector([{
Expand All @@ -206,6 +225,7 @@ describe('web-server', () => {
httpsServerOptions: credentials,
client: { useIframe: true, useSingleWindow: false }
}],
customFileHandlers: ['value', customFileHandlers],
emitter: ['value', emitter],
fileList: ['value', { files: { served: [], included: [] } }],
filesPromise: ['factory', m.createFilesPromise],
Expand Down Expand Up @@ -244,10 +264,12 @@ describe('web-server', () => {
cert: fs.readFileSync(path.join(__dirname, '/certificates/server.crt'))
}

customFileHandlers = []
emitter = new EventEmitter()

const injector = new di.Injector([{
config: ['value', { basePath: '/base/path', urlRoot: '/', httpModule: http2, protocol: 'https:', httpsServerOptions: credentials }],
customFileHandlers: ['value', customFileHandlers],
emitter: ['value', emitter],
fileList: ['value', { files: { served: [], included: [] } }],
filesPromise: ['factory', m.createFilesPromise],
Expand Down