diff --git a/src/client/pjax.ts b/src/client/pjax.ts index 2049f90..2c7713d 100644 --- a/src/client/pjax.ts +++ b/src/client/pjax.ts @@ -22,9 +22,6 @@ import { StoreValsContext, middleware as storeValsMiddleware} from '../middlewar /** * This is a helper class to make PJAX easier. PJAX is a clean way of improving the performance of webpages * by progressively turning standard HTML links into AJAX requests for portions of a web page. - * - * @todo Form support - * @todo tests */ export default class RoadsPjax { protected _road: Road; @@ -73,7 +70,6 @@ export default class RoadsPjax { return next().then((response) => { if (this.getVal) { - // TODO: I'm not sure I like title key in here. Maybe we should have it passed in? pjaxObj._page_title = this.getVal(titleKey) as string; } @@ -91,8 +87,7 @@ export default class RoadsPjax { * This function call enables PJAX on the current page. */ register (): void { - // Handle navigation changes besides pushState. TODO: don' blow out existing onpopstate's - // TODO: If a request is in process during the popstate, we should kill it and use the new url + // Handle navigation changes besides pushState. this._window.onpopstate = (event: PopStateEvent) => { if (event.state.pjax) { // if the popped state was generated via pjax, execute the appropriate route @@ -156,7 +151,6 @@ export default class RoadsPjax { if (event.target instanceof HTMLAnchorElement && event.target.dataset['roadsPjax'] === 'link' && !event.ctrlKey) { event.preventDefault(); this._roadsLinkEvent(event.target as HTMLAnchorElement); - // TODO: Change this to a on submit event? } else if ((event.target instanceof HTMLInputElement || event.target instanceof HTMLButtonElement) && event.target.dataset['roadsPjax'] === 'submit' && event.target.form && event.target.form.dataset['roadsPjax'] === 'form') { @@ -205,7 +199,6 @@ export default class RoadsPjax { ) .then((response: Response) => { if ([301, 302, 303, 307, 308].includes(response.status) && typeof response.headers?.location === 'string') { - // todo: location can be an array via code, but I don't think it's vaild to the spec? return this._road.request('GET', response.headers.location); } else { return response; diff --git a/src/client/request.ts b/src/client/request.ts index 1735f8f..d2b69b5 100644 --- a/src/client/request.ts +++ b/src/client/request.ts @@ -14,8 +14,6 @@ import Response from '../core/response'; * This class is a helper with making HTTP requests that look like roads requests. * The function signature matches the roads "request" method to allow the details of a request to be abstracted * away from the client. Sometimes the request may route internally, sometimes it may be an HTTP request. - * - * @todo tests */ export default class Request { protected secure: boolean; @@ -23,8 +21,6 @@ export default class Request { protected port: number; /** - * @todo: port should just be part of the host - * * @param {boolean} secure - Whether or not this request should use HTTPS * @param {string} host - The hostname of all requests made by this function * @param {number} port - The post of all requests made by this function @@ -76,9 +72,6 @@ export default class Request { headers: newHeaders, redirect: 'manual', referrerPolicy: 'no-referrer', - - // TODO: What should we do with these? - // origin-when-cross-origin, same-origin, strict-origin, strict-origin-when-cross-origin, unsafe-url body }); diff --git a/src/core/requestChain.ts b/src/core/requestChain.ts new file mode 100644 index 0000000..02e3655 --- /dev/null +++ b/src/core/requestChain.ts @@ -0,0 +1,61 @@ +/* eslint-disable max-len */ +import Response from './response'; +import { Context } from './road'; + +export interface NextCallback { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (): Promise +} + +// eslint-disable-next-line @typescript-eslint/ban-types +export class RequestChain { + /** + * The request chain is an array of functions that are executed in order when `run` is called + */ + protected _function_chain: fn[]; + + /** + * RequestChain Constructor + * + * Creates a new RequestChain object + */ + constructor (initial?: fn[]) { + this._function_chain = initial || []; + } + + add (fn: fn) { + this._function_chain.push(fn); + } + + length () { + return this._function_chain.length; + } + + /** + * The `run` function is used to execute the function chain. + * + * The function chain is executed in order, with each function in the chain being called with the parameters provided to the function. + * + * The function function returns a Promise that resolves to a Response object or a string. + * + */ + getChainStart () { + + let progress = 0; + const next: (context: Context, ...args: any[]) => Promise = async (context, ...args) => { + + if (this._function_chain.length && this._function_chain[progress]) { + return this._function_chain[progress].call(context, ...args, () => { + progress += 1; + return next(context, ...args); + }); + } + + // If next is called and there is nothing next, we should still return a promise, + // it just shouldn't do anything + return new Response('Page not found', 404); + }; + + return next; + } +} \ No newline at end of file diff --git a/src/core/road.ts b/src/core/road.ts index 59a4772..7202282 100644 --- a/src/core/road.ts +++ b/src/core/road.ts @@ -9,7 +9,7 @@ import * as response_lib from './response'; import Response from './response'; - +import { NextCallback, RequestChain } from './requestChain'; export interface IncomingHeaders extends Record | undefined> {} @@ -19,10 +19,6 @@ export interface Middleware { next: NextCallback): Promise | Response | string } -export interface NextCallback { - (): Promise -} - export interface Context extends Record {} /** @@ -31,7 +27,7 @@ export interface Context extends Record {} * @name Road */ export default class Road { - protected _request_chain: Middleware[]; + protected _request_chain: RequestChain>; /** * Road Constructor @@ -39,7 +35,7 @@ export default class Road { * Creates a new Road object */ constructor () { - this._request_chain = []; + this._request_chain = new RequestChain(); } /** @@ -73,7 +69,7 @@ export default class Road { use (fn: Middleware): Road { // Currently we pass everything through the coroutine wrapper to be save. Let that library decide // what does and does not actually need to be wrapped - this._request_chain.push(fn); + this._request_chain.add(fn); return this; } @@ -94,40 +90,6 @@ export default class Road { * @returns {Promise} this promise will resolve to a Response object */ request (method: string, url: string, body?: string, headers?: IncomingHeaders): Promise { - return response_lib.wrap(this._buildNext({}, method, url, body, headers)()); - } - - /** - * Turn an HTTP request into an executable function with a useful request context. Will also incorporate the entire - * request handler chain - * - * @param {Context} context - Request context - * @param {string} request_method - HTTP request method - * @param {string} path - HTTP request path - * @param {string} request_body - HTTP request body - * @param {object} request_headers - HTTP request headers - * @returns {NextMiddleware} A function that will start (or continue) the request chain - */ - protected _buildNext (context: Context, request_method: string, path: string, request_body?: string, - request_headers?: IncomingHeaders - ): NextCallback { - - let progress = 0; - const next: NextCallback = async () => { - - if (this._request_chain.length && this._request_chain[progress]) { - return this._request_chain[progress].call(context, - request_method, path, request_body, request_headers, () => { - progress += 1; - return next(); - }); - } - - // If next is called and there is nothing next, we should still return a promise, - // it just shouldn't do anything - return new Response('Page not found', 404); - }; - - return next; + return response_lib.wrap(this._request_chain.getChainStart()({}, method, url, body, headers)); } } \ No newline at end of file diff --git a/src/middleware/applyToContext.ts b/src/middleware/applyToContext.ts index 84bb456..9ea3f8c 100644 --- a/src/middleware/applyToContext.ts +++ b/src/middleware/applyToContext.ts @@ -12,15 +12,12 @@ import { Context, Middleware } from '../core/road'; /** * This is a very simple middleware to apply a single value to the request context with a single line of code. * - * TODO: Get better typing on this. I think we need to wait for https://github.com/Microsoft/TypeScript/pull/26797. - * In the meanwhile if you use this middleware you should create your own context. - * * @param {string} key - The key that should store the value on the request context. * @param {any} val - The value to apply to the request context. * @returns {Middleware} The middleware function to apply to the road.use(fn) method. */ -export function build (key: string, val: unknown): Middleware { - const applyToContext: Middleware = function (method, url, body, headers, next) { +export function build (key: K, val: T[K]): Middleware { + const applyToContext: Middleware = function (method, url, body, headers, next) { this[key] = val; return next(); }; diff --git a/src/middleware/basicRouter.ts b/src/middleware/basicRouter.ts index 73438cb..55f9620 100644 --- a/src/middleware/basicRouter.ts +++ b/src/middleware/basicRouter.ts @@ -8,18 +8,19 @@ */ import parse from 'url-parse'; -import { IncomingHeaders, NextCallback } from '../core/road'; +import { IncomingHeaders } from '../core/road'; import Road, {Context} from '../core/road'; import Response from '../core/response'; +import { NextCallback, RequestChain } from '../core/requestChain'; export interface Route { - (this: ContextType, path: BasicRouterURL, body: string, + (this: ContextType, method: string, path: BasicRouterURL, body: string, headers: IncomingHeaders, next: NextCallback): Promise } interface RouteDetails { - route: Route, + route: Route | RequestChain>, path: string, method: string } @@ -98,7 +99,9 @@ export class BasicRouter { * @param {(string|array)} paths - One or many URL paths that will trigger the provided function * @param {function} fn - The function containing all of your route logic */ - addRoute (method: string, paths: string | string[], fn: Route): void { + addRoute ( + method: string, paths: string | string[], fn: Route | Route[] + ): void { if (!Array.isArray(paths)) { paths = [paths]; } @@ -107,7 +110,7 @@ export class BasicRouter { this._routes.push({ path: path, method: method, - route: fn + route: Array.isArray(fn) ? new RequestChain(fn) : fn }); }); } @@ -137,16 +140,17 @@ export class BasicRouter { * Slightly non-standard roads middleware to execute the functions in this router when requests are received by the road * The first method is the routes to ensure that we can properly use this router once we loose the "this" value * from the roads context - * - * @todo there might be a better way to do this */ - protected _middleware (routes: RouteDetails[], request_method: string, request_url: string, request_body: string, - request_headers: IncomingHeaders, next: NextCallback): Promise { + protected async _middleware ( + this: Context, routes: RouteDetails[], request_method: string, request_url: string, request_body: string, + request_headers: IncomingHeaders, next: () => Promise + ): Promise { let realMethod = request_method; let response = null; let hit = false; + let routeHitMethodFail = false; const parsed_url = parse(request_url, true); @@ -165,10 +169,20 @@ export class BasicRouter { for (let i = 0; i < routes.length; i++) { const route = routes[i]; - if (compareRouteAndApplyArgs(route, parsed_url, realMethod)) { - response = (route.route).call(this, parsed_url, request_body, request_headers, next); - hit = true; - break; + if (compareRouteAndApplyArgs(route, parsed_url)) { + // Check the method last, so we can give proper status codes + if (route.method === realMethod) { + if (route.route instanceof RequestChain) { + response = route.route.getChainStart()(this, realMethod, parsed_url, request_body, request_headers); + } else { + response = (route.route).call(this, realMethod, parsed_url, request_body, request_headers, next); + } + + hit = true; + break; + } else { + routeHitMethodFail = true; + } } } @@ -176,6 +190,10 @@ export class BasicRouter { return response; } + if (routeHitMethodFail) { + return new Response('Method Not Allowed', 405); + } + return next(); } } @@ -190,10 +208,9 @@ export class BasicRouter { * @param {string} request_method - HTTP request method * @returns {boolean} */ -function compareRouteAndApplyArgs (route: {method: string, path: string}, request_url: ReturnType, - request_method: string): boolean { +function compareRouteAndApplyArgs (route: {method: string, path: string}, request_url: ReturnType): boolean { - if (route.method !== request_method || !request_url.pathname) { + if (!request_url.pathname) { return false; } @@ -270,7 +287,6 @@ function applyArg(request_url: BasicRouterURL, template_part: string, actual_par /** * Applies a prefix to paths of route files * - * @todo I'm pretty sure there's an existing library that will do this more accurately * @param {string} path - The HTTP path of a route * @param {string} [prefix] - An optional prefix for the HTTP path * @returns {string} diff --git a/src/middleware/cookieMiddleware.ts b/src/middleware/cookieMiddleware.ts index c09942b..5db8726 100644 --- a/src/middleware/cookieMiddleware.ts +++ b/src/middleware/cookieMiddleware.ts @@ -80,7 +80,6 @@ function (route_method, route_path, route_body, route_headers, next) { // Find the cookies from the request and store them locally if (route_headers && route_headers.cookie) { cookies = cookie.parse( - // todo: hmm... Can we get an array of cookies? I don't think so... this handles it properly if we do though. Array.isArray(route_headers.cookie) ? route_headers.cookie.join('; ') : route_headers.cookie); } @@ -108,15 +107,18 @@ function (route_method, route_path, route_body, route_headers, next) { response = new Response(response); } - // Initalize the header + // Force the set cookie response header to be an array, for ease of applying them below. if (!response.headers['Set-Cookie']) { response.headers['Set-Cookie'] = []; } + if (typeof response.headers['Set-Cookie'] === 'string') { + response.headers['Set-Cookie'] = [response.headers['Set-Cookie']]; + } + // Apply all the cookies for (let i = 0; i < cookieKeys.length; i++) { - // TODO: Don't have this "as" - (response.headers['Set-Cookie'] as Array).push( + (response.headers['Set-Cookie']).push( cookie.serialize(cookieKeys[i], this.newCookies[cookieKeys[i]].value, this.newCookies[cookieKeys[i]].options)); } diff --git a/src/middleware/cors.ts b/src/middleware/cors.ts index 0944b82..380b42d 100644 --- a/src/middleware/cors.ts +++ b/src/middleware/cors.ts @@ -49,7 +49,6 @@ export function build (options: { const supportsCredentials = options.supportsCredentials || false; const responseHeaders = options.responseHeaders || []; const requestHeaders = options.requestHeaders || []; - // todo: lowercase all valid methods const validMethods = options.validMethods || []; const cacheMaxAge = options.cacheMaxAge || null; const logger = options.logger || { log: () => { /* do nothing */ } }; @@ -124,9 +123,9 @@ export function build (options: { Note: Always matching is acceptable since the list of methods can be unbounded. */ - - // todo: lowercase valid methods and cors method - if (corsMethod && validMethods.indexOf(corsMethod) === -1) { + if (corsMethod && validMethods.findIndex((value) => { + return value.toLowerCase() === corsMethod.toLowerCase(); + })) { logger.log('CORS Error: bad method', corsMethod); return next(); } diff --git a/src/middleware/parseBody.ts b/src/middleware/parseBody.ts index d8c6daa..f8459ca 100644 --- a/src/middleware/parseBody.ts +++ b/src/middleware/parseBody.ts @@ -44,7 +44,6 @@ function getSingleHeader(headers: IncomingHeaders | OutgoingHeaders, key: string * @param {string} body - request body * @param {string} content_type - media type of the body * @returns {(object|string)} parsed body - * @todo Actually do something with the parameters, such as charset */ function parseRequestBody (body: string | undefined, contentType?: string): unknown { if (contentType && body) { diff --git a/src/middleware/removeTrailingSlash.ts b/src/middleware/removeTrailingSlash.ts index bef0934..0cf91a9 100644 --- a/src/middleware/removeTrailingSlash.ts +++ b/src/middleware/removeTrailingSlash.ts @@ -18,7 +18,6 @@ import Response from '../core/response'; * Any requests with trailing slashes will immediately return a Response object redirecting to a non-trailing-slash path */ export const middleware: Middleware = function (method, url, body, headers, next) { - // TODO: parse is deprecated, but the URL object that replaces it doesn't do what I need it to const parsedUrl = parse(url); const parsedPath = parsedUrl.pathname; diff --git a/src/middleware/reroute.ts b/src/middleware/reroute.ts index 2ca4f3c..1c24376 100644 --- a/src/middleware/reroute.ts +++ b/src/middleware/reroute.ts @@ -14,7 +14,6 @@ import Road from '../core/road'; * Applies a method to the request context that allows you to make requests into another roads object. * This is useful when you're running two servers locally. One example is a webserver and a separate API server. * - * TODO: Should this just use applytocontext? * * @param {string} key - The name of the key in the request context that will store the roads request. * @param {road} road - The roads object that you will interact with. diff --git a/test/__tests__/middleware/basicRouter.test.ts b/test/__tests__/middleware/basicRouter.test.ts index b4f5125..03f4c9c 100644 --- a/test/__tests__/middleware/basicRouter.test.ts +++ b/test/__tests__/middleware/basicRouter.test.ts @@ -3,9 +3,10 @@ import parse from 'url-parse'; import { BasicRouter, Route, BasicRouterURL } from '../../../src/middleware/basicRouter'; import Road from '../../../src/core/road'; import Response from '../../../src/core/response'; -import { Context, NextCallback } from '../../../src/core/road'; +import { Context } from '../../../src/core/road'; import { describe, expect, test, assert } from 'vitest'; +import { NextCallback } from '../../../src/core/requestChain'; const router_file_test_path = `${__dirname }/../../resources/_router_file_test.js`; @@ -39,7 +40,7 @@ describe('Basic Router Tests', () => { const router = new BasicRouter(); router.applyMiddleware(road); - expect(road['_request_chain'].length).toEqual(1); + expect(road['_request_chain'].length()).toEqual(1); }); /** @@ -62,11 +63,26 @@ describe('Basic Router Tests', () => { }; router.addRoute(method, path, fn); - router['_middleware'](router['_routes'], method, path, '', {}, next); + router['_middleware'].call({}, router['_routes'], method, path, '', {}, next); expect(route_hit).toEqual(true); }); + test('test middleware function 405s when route exists but method is not present', () => { + expect.assertions(1); + + const router = new BasicRouter(); + const path = '/'; + + const next: NextCallback = () => { + return Promise.resolve(new Response('')); + }; + + router.addRoute('POST', path, () => assert.fail('POST route should not run')); + return expect(router['_middleware'].call({}, router['_routes'], 'GET', path, '', {}, next)) + .resolves.toEqual(new Response('Method Not Allowed', 405)); + }); + test('test middleware function routes successfully to successful routes with x-http-method-override header', () => { expect.assertions(1); @@ -85,7 +101,7 @@ describe('Basic Router Tests', () => { router.addRoute('PUT', path, fn); router.addRoute('POST', path, () => assert.fail('POST route should not run')); - router['_middleware'](router['_routes'], method, path, '', { + router['_middleware'].call({}, router['_routes'], method, path, '', { 'x-http-method-override': 'PUT' }, next); @@ -110,7 +126,7 @@ describe('Basic Router Tests', () => { router.addRoute('GET', path, fn); router.addRoute('PUT', path, () => assert.fail('PUT route should not run')); - router['_middleware'](router['_routes'], method, path, '', { + router['_middleware'].call({}, router['_routes'], method, path, '', { 'x-http-method-override': 'PUT' }, next); @@ -135,7 +151,7 @@ describe('Basic Router Tests', () => { router.addRoute('PUT', path, fn); router.addRoute('POST', path, () => assert.fail('POST route should not run')); - router['_middleware'](router['_routes'], method, `${path}?_method=PUT`, '', {}, next); + router['_middleware'].call({}, router['_routes'], method, `${path}?_method=PUT`, '', {}, next); expect(route_hit).toEqual(true); }); @@ -159,7 +175,7 @@ describe('Basic Router Tests', () => { router.addRoute('GET', path, fn); router.addRoute('PUT', path, () => assert.fail('PUT route should not run')); - router['_middleware'](router['_routes'], method, `${path}?_method=PUT`, '', {}, next); + router['_middleware'].call({}, router['_routes'], method, `${path}?_method=PUT`, '', {}, next); expect(route_hit).toEqual(true); }); @@ -190,7 +206,7 @@ describe('Basic Router Tests', () => { router.addRoute(method, path, fn); router.addRoute(method, path, fn2); - router['_middleware'](router['_routes'], method, path, '', {}, next); + router['_middleware'].call({}, router['_routes'], method, path, '', {}, next); expect(route_hit).toEqual(true); }); @@ -198,7 +214,7 @@ describe('Basic Router Tests', () => { /** * */ - test('test middleware function routes to next on a missed url', () => { + test('test middleware function routes to next on a missed url', () => { expect.assertions(2); const router = new BasicRouter(); const path = '/'; @@ -211,7 +227,7 @@ describe('Basic Router Tests', () => { }; router.addRoute('/foo', method, fn); - router['_middleware'](router['_routes'], method, path, '', {}, () => { + router['_middleware'].call({}, router['_routes'], method, path, '', {}, () => { next_hit = true; return Promise.resolve(new Response('')); }); @@ -236,7 +252,7 @@ describe('Basic Router Tests', () => { }; router.addRoute(path, 'PUT', fn); - router['_middleware'](router['_routes'], method, path, '', {}, () => { + router['_middleware'].call({}, router['_routes'], method, path, '', {}, () => { next_hit = true; return Promise.resolve(new Response('')); }); @@ -249,7 +265,7 @@ describe('Basic Router Tests', () => { * */ test('test route function with no template gets the proper context and arguments', () => { - expect.assertions(3); + expect.assertions(4); const router = new BasicRouter(); const path = '/'; @@ -257,7 +273,8 @@ describe('Basic Router Tests', () => { const body = '{"harvey": "birdman"}'; const headers = {bojack: 'horseman'}; - const route: Route = (request_url, request_body, request_headers) => { + const route: Route = (request_method, request_url, request_body, request_headers) => { + expect(request_method).toEqual(method); // parsed url expect(request_url).toEqual(parse(path, true)); // passthrough request body @@ -270,7 +287,7 @@ describe('Basic Router Tests', () => { router.addRoute(method, path,route); - router['_middleware'](router['_routes'], method, path, body, headers, () => { + router['_middleware'].call({}, router['_routes'], method, path, body, headers, () => { return Promise.resolve(new Response('')); }); }); @@ -279,7 +296,7 @@ describe('Basic Router Tests', () => { * */ test('test route function with numeric template gets the proper context and arguments', () => { - expect.assertions(3); + expect.assertions(4); const router = new BasicRouter(); const path = '/#numeric'; const req_path = '/12345'; @@ -287,7 +304,8 @@ describe('Basic Router Tests', () => { const body = '{"harvey": "birdman"}'; const headers = {bojack: 'horseman'}; - const route: Route = (request_url, request_body, request_headers) => { + const route: Route = (request_method, request_url, request_body, request_headers) => { + expect(request_method).toEqual(method); // parsed url const parsed_url: BasicRouterURL = parse(req_path, true); parsed_url.args = {numeric: 12345}; @@ -302,7 +320,7 @@ describe('Basic Router Tests', () => { router.addRoute(method, path, route); - router['_middleware'](router['_routes'], method, req_path, body, headers, () => { + router['_middleware'].call({}, router['_routes'], method, req_path, body, headers, () => { return Promise.resolve(new Response('')); }); }); @@ -311,7 +329,7 @@ describe('Basic Router Tests', () => { * */ test('test route function with string template gets the proper context and arguments', () => { - expect.assertions(3); + expect.assertions(4); const router = new BasicRouter(); const path = '/$string'; const req_path = '/hello'; @@ -319,7 +337,8 @@ describe('Basic Router Tests', () => { const body = '{"harvey": "birdman"}'; const headers = {bojack: 'horseman'}; - const route: Route = (request_url, request_body, request_headers) => { + const route: Route = (request_method, request_url, request_body, request_headers) => { + expect(request_method).toEqual(method); // parsed url const parsed_url: BasicRouterURL = parse(req_path, true); parsed_url.args = {string: 'hello'}; @@ -333,7 +352,7 @@ describe('Basic Router Tests', () => { router.addRoute(method, path, route); - router['_middleware'](router['_routes'], method, req_path, body, headers, () => { + router['_middleware'].call({}, router['_routes'], method, req_path, body, headers, () => { return Promise.resolve(new Response('')); }); }); @@ -352,11 +371,11 @@ describe('Basic Router Tests', () => { }; router.addRoute(method, path, fn); - expect(() => { - router['_middleware'](router['_routes'], method, path, '', {}, () => { + return expect(() => { + return router['_middleware'].call({}, router['_routes'], method, path, '', {}, () => { return Promise.resolve(new Response('')); }); - }).toThrow(new Error(error_message)); + }).rejects.toThrow(new Error(error_message)); }); /** @@ -372,9 +391,11 @@ describe('Basic Router Tests', () => { }; router.addRoute(method, path, fn); - const route_hit: Promise = router['_middleware'](router['_routes'], method, path, '', {}, () => { - return Promise.resolve(new Response('')); - }); + const route_hit: Promise = router['_middleware'].call( + {}, router['_routes'], method, path, '', {}, () => { + return Promise.resolve(new Response('')); + } + ); route_hit.then((response: Response | string) => { expect(response).toBeInstanceOf(Response); @@ -395,9 +416,11 @@ describe('Basic Router Tests', () => { }; router.addRoute(path, 'PUT', fn); - const route_hit: Promise = router['_middleware'](router['_routes'], method, path, '', {}, () => { - return Promise.resolve(new Response('next')); - }); + const route_hit: Promise = router['_middleware'].call( + {}, router['_routes'], method, path, '', {}, () => { + return Promise.resolve(new Response('next')); + } + ); route_hit.then((response: Response | string) => { expect(response).toBeInstanceOf(Response); @@ -476,7 +499,7 @@ describe('Basic Router Tests', () => { expect(results[1]).toEqual(new Response('root post successful')); expect(results[2]).toEqual(new Response('test get successful')); - expect(results[3]).toEqual(new Response('Page not found', 404)); + expect(results[3]).toEqual(new Response('Method Not Allowed', 405)); expect(results[4]).toEqual(new Response('Page not found', 404)); }); @@ -505,10 +528,45 @@ describe('Basic Router Tests', () => { expect(results[1]).toEqual(new Response('root post successful')); expect(results[2]).toEqual(new Response('test get successful')); - expect(results[3]).toEqual(new Response('Page not found', 404)); + expect(results[3]).toEqual(new Response('Method Not Allowed', 405)); expect(results[4]).toEqual(new Response('Page not found', 404)); }); }); }); + + test('test routes using a request chain successfully progress through the chain', () => { + expect.assertions(3); + const road = new Road(); + const router = new BasicRouter(); + router.applyMiddleware(road); + + const path = '/'; + const method = 'GET'; + let route_hit = ''; + const fn: Route = async (method, url, body, headers, next) => { + // this logged undefined undefined [Function (anonymous)] undefined undefined + console.log(method, url, body, headers, next); + const result = await next(); + + if (result instanceof Response) { + return result; + } + + return new Response(result); + }; + + const fn2: Route = (method, url, body, headers, next) => { + route_hit = 'route'; + return Promise.resolve(new Response('')); + }; + + router.addRoute(method, path, [fn, fn2]); + + return road.request(method, path).then((response: Response) => { + expect(route_hit).toEqual('route'); + expect(response).toBeInstanceOf(Response); + expect(response.body).toEqual(''); + }); + }); }); \ No newline at end of file diff --git a/test/__tests__/middleware/cookie.test.ts b/test/__tests__/middleware/cookie.test.ts index 902e1a1..c30b481 100644 --- a/test/__tests__/middleware/cookie.test.ts +++ b/test/__tests__/middleware/cookie.test.ts @@ -21,7 +21,7 @@ describe('cookie tests', () => { expect(context.getCookies().abc).toEqual('def'); }); - test('test cookie middleware will update the response headers', () => { + test('test cookie middleware will update the response headers', async () => { expect.assertions(1); const context = { Response: Response @@ -33,14 +33,14 @@ describe('cookie tests', () => { }; // eslint-disable-next-line @typescript-eslint/no-empty-function - expect(serverMiddleware.call(context, 'a', 'b', 'c', {}, next.bind(context))) - .resolves.toEqual(new Response('test', 200, { + expect(await serverMiddleware.call(context, 'a', 'b', 'c', {}, next.bind(context))) + .toEqual(new Response('test', 200, { 'Set-Cookie': ['foo=bar'] })); }); - test('test that getCookies merges new and old cookies together and properly sets outgoing header', () => { + test('test that getCookies merges new and old cookies together and properly sets outgoing header', async () => { expect.assertions(2); const context = { Response: Response @@ -56,10 +56,10 @@ describe('cookie tests', () => { return Promise.resolve('test'); }; - expect(serverMiddleware.call(context, 'a', 'b', 'c', { + expect(await serverMiddleware.call(context, 'a', 'b', 'c', { cookie: 'abc=def' }, next.bind(context))) - .resolves.toEqual(new Response('test', 200, { + .toEqual(new Response('test', 200, { 'Set-Cookie': ['foo=bar'] })); diff --git a/test/__tests__/road/buildNext.test.ts b/test/__tests__/road/buildNext.test.ts deleted file mode 100644 index e27b76b..0000000 --- a/test/__tests__/road/buildNext.test.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { Road } from '../../../src/index'; -import { Response } from '../../../src/index'; - -import { describe, expect, test } from 'vitest'; - -// Note: This file used to have many more tests, but a recent roads change invalidated most of them, and the -// migration to jest made it clear that many of them were -// covered by other tests (context, multi use, etc) -describe('road buildNext test', () => { - /** - * Test buildNext success when a route does not have an onRequest handler - */ - test('build next hits', () => { - expect.assertions(1); - - const road = new Road(); - return expect(road['_buildNext']({ - request: function() { return Promise.resolve(new Response('')); }, - Response: Response - }, 'GET', '/', '', {}, )()).resolves.toEqual(new Response('Page not found', 404, {})); - }); -}); \ No newline at end of file diff --git a/test/__tests__/road/roadContext.test.ts b/test/__tests__/road/roadContext.test.ts index e378815..defea9c 100644 --- a/test/__tests__/road/roadContext.test.ts +++ b/test/__tests__/road/roadContext.test.ts @@ -77,6 +77,6 @@ describe('Road Context', () => { return await next(); }); - expect(road['_request_chain'].length).toEqual(1); + expect(road['_request_chain'].length()).toEqual(1); }); }); \ No newline at end of file diff --git a/todo.md b/todo.md index 6282c03..9724294 100644 --- a/todo.md +++ b/todo.md @@ -1,7 +1,8 @@ +# Future 1. Figure out a way to have better typing on headers -2. Add a "this request only" request chain to the simple router, allowing you to bind a series of functions to a single url. Have it work like the request chain in road. -3. Have the package-lock automatically updated pre-commit -4. Ensure we have JS and Typescript examples for everything. -5. Improve test coverage (cors, simple router, pjax, probably much more) -6. The current system doesn't properly handle bad http methods. If the route exists, but the method doesn't, we don't 405. If the method is something we know nothing about (e.g. "BanAnANanN"), it should 501 -7. Scan for todos and mark them in this file! \ No newline at end of file +2. Ensure we have JS and Typescript examples for everything. +3. Improve test coverage (cors, simple router, pjax, client request class, probably much more) +4. In pjax window.onpopstate (where we manage fake page navigation), if there's an existing navigation in progress, we should cancel it (and the http request) and then execute the following one. +5. reroute middleware probably can just use applyToContext +6. parseRequestBody should respect more of the content type parameter values, such as charset6. in client request, we accept host and port params. I think port would be better as part of the host... or we could accept either option +7. It would be cool if the router could fork a route off of a query param \ No newline at end of file