From d2bc995272ed596b2bd6acd0d4cab50fcb7859f0 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Thu, 21 Apr 2022 11:26:47 +0200 Subject: [PATCH] refactor: Replace RedirectAllHttpHandler usage with RedirectingHttpHandler --- RELEASE_NOTES.md | 1 + config/app/setup/handlers/redirect.json | 18 ++++--- src/index.ts | 3 +- src/server/util/RedirectAllHttpHandler.ts | 47 ---------------- .../{ => util}/RedirectingHttpHandler.ts | 30 +++++------ .../util/RedirectAllHttpHandler.test.ts | 53 ------------------- .../{ => util}/RedirectingHttpHandler.test.ts | 16 +++--- 7 files changed, 37 insertions(+), 131 deletions(-) delete mode 100644 src/server/util/RedirectAllHttpHandler.ts rename src/server/{ => util}/RedirectingHttpHandler.ts (76%) delete mode 100644 test/unit/server/util/RedirectAllHttpHandler.test.ts rename test/unit/server/{ => util}/RedirectingHttpHandler.test.ts (84%) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 62d79a47c..798a8f6d8 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -26,6 +26,7 @@ The following changes are relevant for v3 custom configs that replaced certain f ### Interface changes These changes are relevant if you wrote custom modules for the server that depend on existing interfaces. - `YargsCliExtractor` was changed to now take as input an array of parameter objects. +- `RedirectAllHttpHandler` was removed and fully replaced by `RedirectingHttpHandler`. ## v4.0.0 ### New features diff --git a/config/app/setup/handlers/redirect.json b/config/app/setup/handlers/redirect.json index 7f4a1af5d..e8727d2bf 100644 --- a/config/app/setup/handlers/redirect.json +++ b/config/app/setup/handlers/redirect.json @@ -4,11 +4,17 @@ { "comment": "Redirects all request to the setup.", "@id": "urn:solid-server:default:SetupRedirectHandler", - "@type": "RedirectAllHttpHandler", - "args_baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, - "args_target": "/setup", - "args_targetExtractor": { "@id": "urn:solid-server:default:TargetExtractor" }, - "args_responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" } - }, + "@type": "RedirectingHttpHandler", + "redirects": [ + { + "RedirectingHttpHandler:_redirects_key": ".*", + "RedirectingHttpHandler:_redirects_value": "/setup" + } + ], + "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, + "targetExtractor": { "@id": "urn:solid-server:default:TargetExtractor" }, + "responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" }, + "statusCode": 302 + } ] } diff --git a/src/index.ts b/src/index.ts index c2a39090a..d17e1d3f4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -269,7 +269,6 @@ export * from './server/HttpResponse'; export * from './server/HttpServerFactory'; export * from './server/OperationHttpHandler'; export * from './server/ParsingHttpHandler'; -export * from './server/RedirectingHttpHandler'; export * from './server/WebSocketHandler'; export * from './server/WebSocketServerFactory'; @@ -280,7 +279,7 @@ export * from './server/middleware/StaticAssetHandler'; export * from './server/middleware/WebSocketAdvertiser'; // Server/Util -export * from './server/util/RedirectAllHttpHandler'; +export * from './server/util/RedirectingHttpHandler'; export * from './server/util/RouterHandler'; // Storage/Accessors diff --git a/src/server/util/RedirectAllHttpHandler.ts b/src/server/util/RedirectAllHttpHandler.ts deleted file mode 100644 index 5a5f5a1a2..000000000 --- a/src/server/util/RedirectAllHttpHandler.ts +++ /dev/null @@ -1,47 +0,0 @@ -import type { TargetExtractor } from '../../http/input/identifier/TargetExtractor'; -import { RedirectResponseDescription } from '../../http/output/response/RedirectResponseDescription'; -import type { ResponseWriter } from '../../http/output/ResponseWriter'; -import { FoundHttpError } from '../../util/errors/FoundHttpError'; -import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; -import { getRelativeUrl, joinUrl } from '../../util/PathUtil'; -import type { HttpHandlerInput } from '../HttpHandler'; -import { HttpHandler } from '../HttpHandler'; - -export interface RedirectAllHttpHandlerArgs { - baseUrl: string; - target: string; - targetExtractor: TargetExtractor; - responseWriter: ResponseWriter; -} - -/** - * Will redirect all incoming requests to the given target. - * In case the incoming request already has the correct target, - * the `canHandle` call will reject the input. - */ -export class RedirectAllHttpHandler extends HttpHandler { - private readonly baseUrl: string; - private readonly target: string; - private readonly targetExtractor: TargetExtractor; - private readonly responseWriter: ResponseWriter; - - public constructor(args: RedirectAllHttpHandlerArgs) { - super(); - this.baseUrl = args.baseUrl; - this.target = args.target; - this.targetExtractor = args.targetExtractor; - this.responseWriter = args.responseWriter; - } - - public async canHandle({ request }: HttpHandlerInput): Promise { - const target = await getRelativeUrl(this.baseUrl, request, this.targetExtractor); - if (target === this.target) { - throw new NotImplementedHttpError('Target is already correct.'); - } - } - - public async handle({ response }: HttpHandlerInput): Promise { - const result = new RedirectResponseDescription(new FoundHttpError(joinUrl(this.baseUrl, this.target))); - await this.responseWriter.handleSafe({ response, result }); - } -} diff --git a/src/server/RedirectingHttpHandler.ts b/src/server/util/RedirectingHttpHandler.ts similarity index 76% rename from src/server/RedirectingHttpHandler.ts rename to src/server/util/RedirectingHttpHandler.ts index 059157f8a..129084c1a 100644 --- a/src/server/RedirectingHttpHandler.ts +++ b/src/server/util/RedirectingHttpHandler.ts @@ -1,18 +1,18 @@ -import type { TargetExtractor } from '../http/input/identifier/TargetExtractor'; -import { RedirectResponseDescription } from '../http/output/response/RedirectResponseDescription'; -import type { ResponseWriter } from '../http/output/ResponseWriter'; -import { getLoggerFor } from '../logging/LogUtil'; -import { FoundHttpError } from '../util/errors/FoundHttpError'; -import { MovedPermanentlyHttpError } from '../util/errors/MovedPermanentlyHttpError'; -import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError'; -import { PermanentRedirectHttpError } from '../util/errors/PermanentRedirectHttpError'; -import type { RedirectHttpError } from '../util/errors/RedirectHttpError'; -import { SeeOtherHttpError } from '../util/errors/SeeOtherHttpError'; -import { TemporaryRedirectHttpError } from '../util/errors/TemporaryRedirectHttpError'; -import { getRelativeUrl, joinUrl } from '../util/PathUtil'; -import type { HttpHandlerInput } from './HttpHandler'; -import { HttpHandler } from './HttpHandler'; -import type { HttpRequest } from './HttpRequest'; +import type { TargetExtractor } from '../../http/input/identifier/TargetExtractor'; +import { RedirectResponseDescription } from '../../http/output/response/RedirectResponseDescription'; +import type { ResponseWriter } from '../../http/output/ResponseWriter'; +import { getLoggerFor } from '../../logging/LogUtil'; +import { FoundHttpError } from '../../util/errors/FoundHttpError'; +import { MovedPermanentlyHttpError } from '../../util/errors/MovedPermanentlyHttpError'; +import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; +import { PermanentRedirectHttpError } from '../../util/errors/PermanentRedirectHttpError'; +import type { RedirectHttpError } from '../../util/errors/RedirectHttpError'; +import { SeeOtherHttpError } from '../../util/errors/SeeOtherHttpError'; +import { TemporaryRedirectHttpError } from '../../util/errors/TemporaryRedirectHttpError'; +import { getRelativeUrl, joinUrl } from '../../util/PathUtil'; +import type { HttpHandlerInput } from '../HttpHandler'; +import { HttpHandler } from '../HttpHandler'; +import type { HttpRequest } from '../HttpRequest'; const redirectErrorFactories: Record<301 | 302 | 303 | 307 | 308, (location: string) => RedirectHttpError> = { 301: (location: string): RedirectHttpError => new MovedPermanentlyHttpError(location), diff --git a/test/unit/server/util/RedirectAllHttpHandler.test.ts b/test/unit/server/util/RedirectAllHttpHandler.test.ts deleted file mode 100644 index d908c3a63..000000000 --- a/test/unit/server/util/RedirectAllHttpHandler.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { TargetExtractor } from '../../../../src/http/input/identifier/TargetExtractor'; -import type { ResponseWriter } from '../../../../src/http/output/ResponseWriter'; -import type { ResourceIdentifier } from '../../../../src/http/representation/ResourceIdentifier'; -import type { HttpRequest } from '../../../../src/server/HttpRequest'; -import type { HttpResponse } from '../../../../src/server/HttpResponse'; -import { RedirectAllHttpHandler } from '../../../../src/server/util/RedirectAllHttpHandler'; -import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; -import { joinUrl } from '../../../../src/util/PathUtil'; -import { SOLID_HTTP } from '../../../../src/util/Vocabularies'; - -describe('A RedirectAllHttpHandler', (): void => { - const baseUrl = 'http://test.com/'; - const target = '/foo'; - const absoluteTarget = 'http://test.com/foo'; - let request: HttpRequest; - const response: HttpResponse = {} as any; - let targetExtractor: jest.Mocked; - let responseWriter: jest.Mocked; - let handler: RedirectAllHttpHandler; - - beforeEach(async(): Promise => { - request = { url: '/foo' } as any; - - targetExtractor = { - handleSafe: jest.fn(({ request: req }): ResourceIdentifier => ({ path: joinUrl(baseUrl, req.url!) })), - } as any; - - responseWriter = { handleSafe: jest.fn() } as any; - - handler = new RedirectAllHttpHandler({ baseUrl, target, targetExtractor, responseWriter }); - }); - - it('rejects requests for the target.', async(): Promise => { - request.url = target; - await expect(handler.canHandle({ request, response })).rejects.toThrow(NotImplementedHttpError); - }); - - it('accepts all other requests.', async(): Promise => { - request.url = '/otherPath'; - await expect(handler.canHandle({ request, response })).resolves.toBeUndefined(); - }); - - it('writes out a redirect response.', async(): Promise => { - await expect(handler.handle({ request, response })).resolves.toBeUndefined(); - expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); - expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ - response, - result: expect.objectContaining({ statusCode: 302 }), - }); - const { metadata } = responseWriter.handleSafe.mock.calls[0][0].result; - expect(metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(absoluteTarget); - }); -}); diff --git a/test/unit/server/RedirectingHttpHandler.test.ts b/test/unit/server/util/RedirectingHttpHandler.test.ts similarity index 84% rename from test/unit/server/RedirectingHttpHandler.test.ts rename to test/unit/server/util/RedirectingHttpHandler.test.ts index 585bf7e28..5f043f789 100644 --- a/test/unit/server/RedirectingHttpHandler.test.ts +++ b/test/unit/server/util/RedirectingHttpHandler.test.ts @@ -1,11 +1,11 @@ -import type { TargetExtractor } from '../../../src/http/input/identifier/TargetExtractor'; -import type { ResponseWriter } from '../../../src/http/output/ResponseWriter'; -import type { ResourceIdentifier } from '../../../src/http/representation/ResourceIdentifier'; -import type { HttpRequest } from '../../../src/server/HttpRequest'; -import type { HttpResponse } from '../../../src/server/HttpResponse'; -import { RedirectingHttpHandler } from '../../../src/server/RedirectingHttpHandler'; -import { joinUrl } from '../../../src/util/PathUtil'; -import { SOLID_HTTP } from '../../../src/util/Vocabularies'; +import type { TargetExtractor } from '../../../../src/http/input/identifier/TargetExtractor'; +import type { ResponseWriter } from '../../../../src/http/output/ResponseWriter'; +import type { ResourceIdentifier } from '../../../../src/http/representation/ResourceIdentifier'; +import type { HttpRequest } from '../../../../src/server/HttpRequest'; +import type { HttpResponse } from '../../../../src/server/HttpResponse'; +import { RedirectingHttpHandler } from '../../../../src/server/util/RedirectingHttpHandler'; +import { joinUrl } from '../../../../src/util/PathUtil'; +import { SOLID_HTTP } from '../../../../src/util/Vocabularies'; describe('A RedirectingHttpHandler', (): void => { const baseUrl = 'http://test.com/';