From 569fa643aca1d1add64402495179ebb939d40b7a Mon Sep 17 00:00:00 2001 From: Ethan Resnick Date: Mon, 8 Jan 2018 16:09:11 -0500 Subject: [PATCH] API controller: support user providing a query factory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the `handle` method took in a request object, did some extra parsing and validation, built a query from the parsed + validated request, and then transformed that query with the user’s transform function. Finally, it ran the new query to get a Result, and returned an HTTPResponse created from that result. I’ve now broken this up over many separate functions (that are then optionally still called in `handle`), which is a good thing in a general/abstract sense, but also enables a specific use case. In particular, the idea is that, rather than the library constructing the initial query and then the user getting to transform that, the user should be able to control how the initial query is constructed. This may seem like a distinction without a difference, because you’d imagine that the user could take the initial query (passed to their transform function) and just discard it in the transform to replace it with whatever query they want. But the problem with that is request validation. Imagine if the user wants to handle some request like `POST /sign-in`, where the credentials are passed in an Authorization header. Even though this is a POST request, the user wants to map it to a FindQuery that’ll read the credentials and use them to look up the resulting user, or return an error. The problem is that, to invoke their query transform, the library first has to construct an initial query that it can hand the query transform as an argument. And, fundamentally, it has no idea how to construct a query for a POST request with no body and no `type` parameter, since it’s basically assuming all POST requests result in CreateQuerys or AddToRelationshipQuerys. So, the library tries to validate the request in preparation for creating such a query, and the lack of a body causes the library to throw an error, so the user’s transform never runs. So, we replace the idea of a query transform with a query factory -- a function the user can provide to build the query -- and we make a separate `APIController.makeQuery` function public, so that the user can simply compose their own function with `makeQuery` to recreate the effect of query transforms. But this reveals another problem, which is present whether the final query comes from running the user's transform function (as before) or a user factory function. Namely: when and how beforeSave/beforeRender transforms should run and generally interact with query construction. In short, `beforeSave` transforms have to run as part of/prior to constructing the query, because we have no way to take a query, once constructed, and know conceptually which parts of it were constructed with info from the request document that should be transformed. (E.g., in a bulk delete query, resource identifier objects from the document end up in the query's Criteria.) In other words, we really can't we really can't apply `beforeSave` to a query -- but only to an incoming request document, where we know what parts are resources and resource identifiers. Moreover, it might be confusing/inelegant for the query that the query factory returns to not actually be the query that runs, which is another argument for not trying to apply beforeSave to a returned query. But, in our old query transform world, applying `beforeSave` as part of query construction posed a problem: the user's code only ran after the initial query was constructed, which made it impossible for them to access the raw, untransformed data as the end user input it in the request. So, now we solve this by giving the query factory access to the untransformed request document, to read the raw data, and a function for transforming a document, so it can get the transformed dat it needs and put it in the query. That runs into another small problem, though, namely: that the `Resource` class is mutable, so running `beforeSave` produces big side effects; at the moment you do the transform, the untransformed resources that get passed to the factory are mutated too. That's really icky/not ideal, but it's tolerable: the factory function just has to read the data it needs off the untransformed doc before calling the transform function or `makeQuery`, which has to call the transform function. So, that solves how to apply `beforeSave`. Then, the question is `beforeRender`. It turns out there's an asymmetry here, because we can take a query and transform it to be the same query but with `beforeRender` applied. That's because beforeRender would be applied in `query.resulting`, and we always have a document that we're applying it to at that point. That made me tempted to take the query factory's result and wrap it in `beforeRender` automatically, as that would ensure that the user never forgets to call beforeRender on the result, which could be a security issue. However, I ultimately decided against this because: 1) I again didn't like the idea that the query returned by the the query factory isn't actually the query that's run and; 2) when the user's making their own queries (which should hopefully be rare), they already have to remember to apply beforeSave to be safe, so having to remember to apply beforeRender (which will be automatic if they're using `makeQuery` as a base and composing the `query.returning` function it generates) seemed like not too much extra burden. Note: a big nice outcome of this commit is that all the library’s built-in query construction logic is unit testable, because APIController.makeQuery is a pure function. Before we’d reified the idea of the query and broken construction apart from running, we’d have needed to do an integration test where we spied on adapter methods. This commit also includes another of other changes; see UPGRADING.md diff in this commit. --- README.md | 46 +- UPGRADING.md | 20 +- package-lock.json | 84 +++- package.json | 4 +- src/controllers/API.ts | 429 +++++++++++++------ src/controllers/Documentation.ts | 10 +- src/http-strategies/Base.ts | 30 +- src/http-strategies/Express.ts | 177 +++++--- src/http-strategies/Koa.ts | 4 +- src/steps/apply-transform.ts | 26 +- src/steps/make-query/make-delete.ts | 14 +- src/steps/make-query/make-get.ts | 4 +- src/steps/make-query/make-patch.ts | 10 +- src/steps/make-query/make-post.ts | 8 +- src/types/index.ts | 18 +- test/app/src/index.ts | 111 +++-- test/app/src/resource-descriptions/people.ts | 6 +- test/integration/fetch-resource/index.ts | 11 + test/integration/http-compliance/index.ts | 42 +- 19 files changed, 718 insertions(+), 336 deletions(-) diff --git a/README.md b/README.md index 57ca8225..06085e62 100755 --- a/README.md +++ b/README.md @@ -25,43 +25,43 @@ Check out the [full, working v3 example repo](https://github.com/ethanresnick/js var adapter = new API.dbAdapters.Mongoose(models); var registry = new API.ResourceTypeRegistry({ "people": { - urlTemplates: { - "self": "/people/{id}" - }, beforeRender: function(resource, req, res) { if(!userIsAdmin(req)) resource.removeAttr("password"); return resource; } }, - "places": { - urlTemplates: {"self": "/places/{id}"} - } + "places": {} }, { - "dbAdapter": adapter + "dbAdapter": adapter, + "urlTemplates": { + "self": "/{type}/{id}" + } }); - // Initialize the automatic documentation. - var DocsController = new API.controllers.Documentation(registry, {name: 'Example API'}); - // Tell the lib the host name our API is served from; needed for security. - var opts = { host: 'example.com' } + const opts = { host: 'example.com' }; + + // Set up a front controller, passing it controllers that'll be used + // to handle requests for API resources and for the auto-generated docs. + var Front = new API.httpStrategies.Express( + new API.controllers.API(registry), + new API.controllers.Documentation(registry, {name: 'Example API'}) + ); - // Set up our controllers - var APIController = new API.controllers.API(registry); - var Front = new API.httpStrategies.Express(APIController, DocsController, opts); - var requestHandler = Front.apiRequest.bind(Front); + // Render the docs at / + app.get("/", Front.docsRequest); // Add routes for basic list, read, create, update, delete operations - app.get("/:type(people|places)", requestHandler); - app.get("/:type(people|places)/:id", requestHandler); - app.post("/:type(people|places)", requestHandler); - app.patch("/:type(people|places)/:id", requestHandler); - app.delete("/:type(people|places)/:id", requestHandler); + app.get("/:type(people|places)", Front.apiRequest); + app.get("/:type(people|places)/:id", Front.apiRequest); + app.post("/:type(people|places)", Front.apiRequest); + app.patch("/:type(people|places)/:id", Front.apiRequest); + app.delete("/:type(people|places)/:id", Front.apiRequest); // Add routes for adding to, removing from, or updating resource relationships - app.post("/:type(people|places)/:id/relationships/:relationship", requestHandler); - app.patch("/:type(people|places)/:id/relationships/:relationship", requestHandler); - app.delete("/:type(people|places)/:id/relationships/:relationship", requestHandler); + app.post("/:type(people|places)/:id/relationships/:relationship", Front.apiRequest); + app.patch("/:type(people|places)/:id/relationships/:relationship", Front.apiRequest); + app.delete("/:type(people|places)/:id/relationships/:relationship", Front.apiRequest); app.listen(3000); diff --git a/UPGRADING.md b/UPGRADING.md index 7dabf24d..79429a10 100755 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,7 +1,25 @@ # 3.0.0-beta.4 +## New Features +- The APIController, and the ExpressStrategy, now have functions for sending a JSON:API any `Result` back to the user. This is convenient if you ahve a `Result` that you created manually or that otherwise came from somewhere other than executing a query in `APIController.handle`. + +- The public methods on the Express strategy are now bound methods, so you no-longer have to bind their `this` when passing them around. + +- The library now prints deprecation warnings using the `depd` module. + ## Breaking Changes -- Query.returning and query.catch can now be async (returning a `Promise` rather than a `Result` directly). Accordingly, if you were wrapping one of those functions, you'll now have to `await` the return value. +- Query.returning and query.catch can now be async (returning a `Promise` rather than a `Result` directly). Accordingly, if you were composing with one of those functions, you'll now have to `await` the return value. + +- Request body and query parsing now happen slightly earlier than before. Accordingly, if the user's request had multiple errors (e.g. an invalid body and an invalid method), the error returned may be different than before, because the first check that fails is the one whose error is reported. + +- The library's internal `Request` object no longer has a `primary` key; instead it has a `document` key which holds the whole request body parsed as a `Document` instance (or undefined for requests with no body). This lays the ground work for supporting sideposting and other features where there's query-related data in the request document outside of the primary data. + +- In the fifth argument passed to user-defined `beforeSave` and `beforeRender` functions, which is an object, the request and response objects generated by your HTTP framework (e.g., express, hapi, node) should now be accessed at the `serverReq` and `serverRes` properties, rather than `frameworkReq` and `frameworkRes`. Those old names are deprecated and will be removed when v3 launches. + +- `ExpressStrategy.sendError` should now be provided with the `next` function as an argument; not providing this will be an error when v3 is finalized. + +- On `APIController` signature of `handle` method has changed, and the `responseFromExternalError` method has been renamed to `responseFromError`. These changes should only effect you if you have written a custom HTTP strategy. + # 3.0.0-beta.3 ## Bugfixes diff --git a/package-lock.json b/package-lock.json index ddd3811c..06f45fe3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -458,6 +458,14 @@ "has-ansi": "2.0.0", "strip-ansi": "3.0.1", "supports-color": "2.0.0" + }, + "dependencies": { + "supports-color": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-2.0.0.tgz", + "integrity": "sha1-U10EXOa2Nj+kARcIRimZXp3zJMc=", + "dev": true + } } }, "character-parser": { @@ -733,9 +741,9 @@ "dev": true }, "debug": { - "version": "2.6.8", - "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.8.tgz", - "integrity": "sha1-5zFTHKLt4n0YgiJCfaF4IdaP9Pw=", + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", + "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", "requires": { "ms": "2.0.0" } @@ -1053,7 +1061,7 @@ "requires": { "chalk": "1.1.3", "concat-stream": "1.6.0", - "debug": "2.6.8", + "debug": "2.6.9", "doctrine": "0.6.4", "escape-string-regexp": "1.0.5", "escope": "3.6.0", @@ -1075,6 +1083,15 @@ "xml-escape": "1.0.0" }, "dependencies": { + "debug": { + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", + "dev": true, + "requires": { + "ms": "2.0.0" + } + }, "espree": { "version": "2.2.5", "resolved": "https://registry.npmjs.org/espree/-/espree-2.2.5.tgz", @@ -1211,6 +1228,17 @@ "type-is": "1.6.15", "utils-merge": "1.0.0", "vary": "1.1.1" + }, + "dependencies": { + "debug": { + "version": "2.6.8", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.8.tgz", + "integrity": "sha1-5zFTHKLt4n0YgiJCfaF4IdaP9Pw=", + "dev": true, + "requires": { + "ms": "2.0.0" + } + } } }, "extend": { @@ -1334,6 +1362,17 @@ "parseurl": "1.3.1", "statuses": "1.3.1", "unpipe": "1.0.0" + }, + "dependencies": { + "debug": { + "version": "2.6.8", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.8.tgz", + "integrity": "sha1-5zFTHKLt4n0YgiJCfaF4IdaP9Pw=", + "dev": true, + "requires": { + "ms": "2.0.0" + } + } } }, "find-index": { @@ -2826,6 +2865,15 @@ "sliced": "0.0.5" }, "dependencies": { + "debug": { + "version": "2.6.8", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.8.tgz", + "integrity": "sha1-5zFTHKLt4n0YgiJCfaF4IdaP9Pw=", + "dev": true, + "requires": { + "ms": "2.0.0" + } + }, "sliced": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/sliced/-/sliced-0.0.5.tgz", @@ -5094,6 +5142,17 @@ "on-finished": "2.3.0", "range-parser": "1.2.0", "statuses": "1.3.1" + }, + "dependencies": { + "debug": { + "version": "2.6.8", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.8.tgz", + "integrity": "sha1-5zFTHKLt4n0YgiJCfaF4IdaP9Pw=", + "dev": true, + "requires": { + "ms": "2.0.0" + } + } } }, "sequencify": { @@ -5315,10 +5374,19 @@ } }, "supports-color": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-2.0.0.tgz", - "integrity": "sha1-U10EXOa2Nj+kARcIRimZXp3zJMc=", - "dev": true + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-4.5.0.tgz", + "integrity": "sha1-vnoN5ITexcXN34s9WRJQRJEvY1s=", + "requires": { + "has-flag": "2.0.0" + }, + "dependencies": { + "has-flag": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-2.0.0.tgz", + "integrity": "sha1-6CB68cx7MNRGzHC3NLXovhj4jVE=" + } + } }, "text-table": { "version": "0.2.0", diff --git a/package.json b/package.json index 1053eb67..a3456db8 100755 --- a/package.json +++ b/package.json @@ -51,7 +51,8 @@ "@types/url-template": "^2.0.28", "content-type": "1.x.x", "dasherize": "2.0.x", - "debug": "^2.6.8", + "debug": "^3.1.0", + "depd": "^1.1.1", "flat": "^1.2.1", "immutable": "^3.7.5", "jade": "1.11.x", @@ -61,6 +62,7 @@ "qs": "^6.5.0", "ramda": "^0.24.1", "raw-body": "2.x.x", + "supports-color": "^4.5.0", "url-template": "^2.0.4", "vary": "^1.1.1" }, diff --git a/src/controllers/API.ts b/src/controllers/API.ts index 082c8bfb..f0401adc 100755 --- a/src/controllers/API.ts +++ b/src/controllers/API.ts @@ -1,9 +1,10 @@ import templating = require("url-template"); +import R = require("ramda"); import { - Request, Result, HTTPResponse, + Request, FinalizedRequest, Result, HTTPResponse, + ServerReq, ServerRes, Predicate, FieldConstraint, - UrlTemplateFnsByType, - makeDoc + UrlTemplateFnsByType, makeDocument } from "../types"; import Query from "../types/Query/Query"; import ResourceTypeRegistry from '../ResourceTypeRegistry'; @@ -12,6 +13,9 @@ import APIError from "../types/APIError"; import Data from "../types/Generic/Data"; import Resource from '../types/Resource'; import ResourceIdentifier from "../types/ResourceIdentifier"; +import ResourceSet from '../types/ResourceSet'; +import ResourceIdentifierSet from "../types/ResourceIdentifierSet"; +import Relationship from "../types/Relationship"; import logger from '../util/logger'; import { mapObject } from '../util/type-handling'; @@ -24,13 +28,26 @@ import validateRequestDocument from "../steps/pre-query/validate-document"; import validateRequestResources from "../steps/pre-query/validate-resources"; import parseQueryParams from "../steps/pre-query/parse-query-params"; import filterParamParser, { getFilterList } from "../steps/pre-query/filter-param-parser"; -import applyTransform from "../steps/apply-transform"; +import applyTransform, { TransformMode, Extras } from "../steps/apply-transform"; import makeGET from "../steps/make-query/make-get"; import makePOST from "../steps/make-query/make-post"; import makePATCH from "../steps/make-query/make-patch"; import makeDELETE from "../steps/make-query/make-delete"; +// Import to export +import { IncomingMessage, ServerResponse } from "http"; +import CreateQuery from "../types/Query/CreateQuery"; +import FindQuery from "../types/Query/FindQuery"; +import UpdateQuery from "../types/Query/UpdateQuery"; +import DeleteQuery from "../types/Query/DeleteQuery"; +import AddToRelationshipQuery from "../types/Query/AddToRelationshipQuery"; +import RemoveFromRelationshipQuery from "../types/Query/RemoveFromRelationshipQuery"; +export { + CreateQuery, FindQuery, UpdateQuery, DeleteQuery, + AddToRelationshipQuery, RemoveFromRelationshipQuery, + IncomingMessage, ServerResponse +}; export type ErrOrErrArr = Error | APIError | Error[] | APIError[]; @@ -38,6 +55,22 @@ export type APIControllerOpts = { filterParser?: filterParamParser }; +export type QueryFactory = (opts: QueryBuildingContext) => Query | Promise; + +export type QueryBuildingContext = { + request: FinalizedRequest, + serverReq: ServerReq, + serverRes: ServerRes, + transformDocument: (doc: Document, mode: TransformMode) => Promise, + registry: ResourceTypeRegistry, + makeDocument: makeDocument, + makeQuery: QueryFactory +}; + +export type RequestOpts = { + queryFactory?: QueryFactory; +} + export type filterParamParser = ( legalUnaryOpts: string[], legalBinaryOpts: string[], @@ -46,7 +79,7 @@ export type filterParamParser = ( ) => (Predicate|FieldConstraint)[] | undefined; -class APIController { +export default class APIController { private registry: ResourceTypeRegistry; private filterParamParser: filterParamParser; private urlTemplateFns: UrlTemplateFnsByType; @@ -61,110 +94,207 @@ class APIController { }); } + protected makeDoc = (data: DocumentData) => + new Document({ urlTemplates: this.urlTemplateFns, ...data }); + + protected async finalizeRequest(request: Request): Promise { + // Parse any query params to finalize the request object. + // We might not actually use the parse result to construct our query, + // so we could (e.g.) put this in a lazily-evaluated getter, but we + // always have to make it appear like the final params are available + // in case someone tries to access them in beforeSave or a query transform. + // If we can't find the adapter for this request, we can't know the valid + // filter param operators, so we act conservatively to say there are no + // valid operators. That may lead to some parse errors, but it's probably + // better than erroring unconditionally (even if no filters are in use). + const { unaryFilterOperators, binaryFilterOperators } = + this.registry.hasType(request.type) + ? this.registry.dbAdapter(request.type).constructor + : { unaryFilterOperators: [], binaryFilterOperators: [] }; + + const finalizedRequest: FinalizedRequest = { + ...request, + queryParams: { + ...parseQueryParams(request.queryParams), + filter: this.filterParamParser( + unaryFilterOperators, + binaryFilterOperators, + request.rawQueryString, + request.queryParams + ), + }, + document: undefined + } + + if(request.body !== undefined) { + const linksJSONToTemplates = (linksJSON) => + mapObject(linksJSON || {}, v => () => v); + + const parsedPrimary = await (async () => { + await validateContentType(request, (this.constructor).supportedExt); + await validateRequestDocument(request.body); + return parseRequestPrimary(request.body.data, parseAsLinkage(request)); + })(); + + finalizedRequest.document = this.makeDoc({ + primary: parseAsLinkage(request) + ? (isBulkDelete(request) + ? ResourceIdentifierSet.of({ + data: parsedPrimary as Data, + links: linksJSONToTemplates(request.body.links) + }) + : Relationship.of({ + data: parsedPrimary as Data, + links: linksJSONToTemplates(request.body.links), + owner: { + type: request.type, + id: request.id, + path: request.relationship + } + })) + : ResourceSet.of({ + data: parsedPrimary as Data, + links: linksJSONToTemplates(request.body.links), + }), + meta: request.body.meta + }); + } + + return finalizedRequest; + } + + /** + * Makes the default query the library will run, given a finalized request + * and some options. + * + * Note: this function should be pure, including in the sense of not needing + * access to anything on `this.`, as everything needed to construct the query + * should be in the same option set that we give user-provided query factory + * functions. + * + * @param {QueryBuildingContext} opts [description] + */ + public async makeQuery(opts: QueryBuildingContext) { + const { request } = opts; + let requestAfterBeforeSave = request; + + // check that a valid method is in use + // and that, if the body is supposed to be present, it is (or vice-versa). + // We await these in order to be deterministic about the error message. + await requestValidators.checkMethod(request); + await requestValidators.checkBodyExistence(request); + + if(request.document && request.document.primary) { + // validate the request's resources, if any + if(!parseAsLinkage(request)) { + await validateRequestResources( + request.type, + (request.document.primary as any).data as Data, + opts.registry + ); + } + + // Apply beforeSave. Note: we arguably should be passing the + // post-beforeSave primary data to the make query functions separately, + // rather than override the original primary and pass that. But the + // make query functions are still looking for the transformed data at + // request.document, so we do this for simplicity. TODO: Should we + // create ResourceIdentifier's from URL params and transform those too? + requestAfterBeforeSave = { + ...request, + document: await opts.transformDocument(request.document, "beforeSave") + }; + } + + const baseQuery = await (() => { + switch(<"get"|"post"|"patch"|"delete">request.method) { + case "get": + return makeGET(requestAfterBeforeSave, opts.registry, opts.makeDocument) + case "post": + return makePOST(requestAfterBeforeSave, opts.registry, opts.makeDocument) + case "patch": + return makePATCH(requestAfterBeforeSave, opts.registry, opts.makeDocument) + case "delete": + return makeDELETE(requestAfterBeforeSave, opts.registry, opts.makeDocument) + } + })(); + + // Apply beforeRender and add default catch + const origReturning = baseQuery.returning; + const origCatch = baseQuery.catch || makeResultFromErrors.bind(null, opts.makeDocument); + + const transformResultDocument = async (result: Result) => { + result.document = result.document && + await opts.transformDocument(result.document, "beforeRender"); + + return result; + }; + + return baseQuery.resultsIn( + R.pipe(origReturning, transformResultDocument), + R.pipe(origCatch, transformResultDocument), + ); + } + /** * @param {Request} request The Request this controller will use to generate * the HTTPResponse. - * @param {Object} frameworkReq This should be the request object generated by - * the framework that you're using. But, really, it can be absolutely - * anything, as this controller won't use it for anything except passing it - * to user-provided functions that it calls (like transforms and id mappers). - * @param {Object} frameworkRes Theoretically, the response objcet generated + * @param {ServerReq} serverReq This should be the request object generated + * by the server framework that you're using. But, really, it can be + * absolutely anything, as this controller won't use it for anything except + * passing it to user-provided functions that it calls (like transforms). + * @param {ServerRes} serverRes Theoretically, the response objcet generated * by your http framework but, like with frameworkReq, it can be anything. */ - async handle( + public handle = async ( request: Request, - frameworkReq: any, - frameworkRes: any, - queryTransform?: (q: Query) => Query | Promise - ) { - const registry = this.registry; - const makeDoc = (data: DocumentData) => - new Document({ urlTemplates: this.urlTemplateFns, ...data }); - - let jsonAPIResult: Result = {}; + serverReq: ServerReq, + serverRes: ServerRes, + opts: RequestOpts = {} + ) => { let contentType: string | undefined; + let jsonAPIResult: Result = {}; // Kick off the chain for generating the response. try { - // check that a valid method is in use - await requestValidators.checkMethod(request); - - // throw if the body is supposed to be present but isn't (or vice-versa). - await requestValidators.checkBodyExistence(request); - // Attempt to negotiate the content type. Will be json-api, or standard // application/json if that's all the client supports, or will error. // Better to do this early so we exit fast if the client can't support anything. + logger.info('Negotiating response content-type'); contentType = await negotiateContentType(request.accepts, ["application/vnd.api+json"]) - // If the type requested in the endpoint hasn't been registered, we 404. - if(!registry.hasType(request.type)) { - throw new APIError(404, undefined, `${request.type} is not a valid type.`); - } - - // Parse any query params and mutate the request object to have the parse - // results. Arguably, this could be done a bit more lazily, since we only - // need to first parse the params to construct get queries (atm, anyway). - // Still, we do this here so that any any transforms (like beforeSave) - // see the finished request object. - const adapter = registry.dbAdapter(request.type); - const { unaryFilterOperators, binaryFilterOperators } = adapter.constructor; + logger.info('Parsing request body/query parameters'); + const finalizedRequest = await this.finalizeRequest(request); - request.queryParams = { - ...parseQueryParams(request.queryParams), - filter: this.filterParamParser( - unaryFilterOperators, - binaryFilterOperators, - request.rawQueryString, - request.queryParams - ) - } - - // If the request has a body, validate it and parse its resources. - if(typeof request.body !== 'undefined') { - await validateContentType(request, (this.constructor).supportedExt); - await validateRequestDocument(request.body); - - const isBulkDelete = - request.method === "delete" && !request.id && !request.aboutRelationship; - - const parsedPrimary = await parseRequestPrimary( - request.body.data, request.aboutRelationship || isBulkDelete - ); - - // validate the request's resources. - if(!request.aboutRelationship) { - await validateRequestResources(request.type, >parsedPrimary, registry); - } + // Actually fulfill the request! + const queryFactory = opts.queryFactory || this.makeQuery; + + const transformExtras = { + request: finalizedRequest, + registry: this.registry, + serverReq, + serverRes + }; + + logger.info('Creating request query'); + const query = await queryFactory({ + ...transformExtras, + makeDocument: this.makeDoc, // already this bound. + transformDocument: R.partialRight(transformDoc, [transformExtras]), + makeQuery: this.makeQuery + }); - request.primary = await applyTransform( - parsedPrimary as Data, - "beforeSave", - { frameworkReq, frameworkRes, request, registry } - ) as (Data | Data); + // If the type in the request hasn't been registered, + // we can't look up it's adapter to run the query, so we 404. + if(!this.registry.hasType(query.type)) { + throw new APIError(404, undefined, `${request.type} is not a valid type.`); } - // Actually fulfill the request! - const query = await (() => { - queryTransform = queryTransform || ((it: any) => it); - - switch(<"get"|"post"|"patch"|"delete">request.method) { - case "get": - return queryTransform(makeGET(request, registry, makeDoc)) - case "post": - return queryTransform(makePOST(request, registry, makeDoc)) - case "patch": - return queryTransform(makePATCH(request, registry, makeDoc)) - case "delete": - return queryTransform(makeDELETE(request, registry, makeDoc)) - } - })(); - - jsonAPIResult = await adapter.doQuery(query).then( - query.returning, - query.catch || makeResultFromErrors.bind(null, makeDoc) - ); + logger.info('Executing request query'); + const adapter = this.registry.dbAdapter(query.type); + jsonAPIResult = + await adapter.doQuery(query).then(query.returning, query.catch); // add top level self link pre send. if(jsonAPIResult.document && jsonAPIResult.document.primary) { @@ -173,32 +303,6 @@ class APIController { ...jsonAPIResult.document.primary.links }; } - - // apply transforms pre-send - if(jsonAPIResult.document) { - // Read and transform private internal Data if it exists. - const primaryData = jsonAPIResult.document.primary && - (jsonAPIResult.document.primary).data; - - const includedData = jsonAPIResult.document.included && - Data.of(jsonAPIResult.document.included); - - if(primaryData) { - (jsonAPIResult.document.primary).data = await applyTransform( - primaryData, - "beforeRender", - { frameworkReq, frameworkRes, request, registry } - ); - } - - if(includedData) { - jsonAPIResult.document.included = (await applyTransform( - includedData, - "beforeRender", - { frameworkReq, frameworkRes, request, registry } - )).values as Resource[]; - } - } } // If any errors occurred, convert them to a Response. Might be needed if, @@ -206,20 +310,15 @@ class APIController { // one of prior steps or the user couldn't throw an APIError for // compatibility with other code. catch (err) { - jsonAPIResult = makeResultFromErrors(makeDoc, err); - - // I'm pretty sure err is always one err and not an array, - // but this code was here before, so keep it for now just in case. - const errorsArr = Array.isArray(err) ? err : [err]; - errorsArr.forEach(err => { - logger.info("API Controller caught error", err, err.stack); - }); + logger.warn("API Controller caught error", err, err.stack); + jsonAPIResult = makeResultFromErrors(this.makeDoc, err); } // Convert jsonApiResponse to httpResponse. Atm, this is simply about // copying over a couple properties. In the future, though, one HTTP request // might generate multiple queries, and then multiple jsonAPIResponses, // which would be merged into a single HTTP Response. + logger.info('Creating HTTPResponse'); return resultToHTTPResponse(jsonAPIResult, contentType); } @@ -227,23 +326,51 @@ class APIController { * Builds a response from errors. Allows errors that occur outside of the * library to be handled and returned in JSON API-compiant fashion. * - * @param {} errors Error or array of errors + * @param {ErrOrErrArr} errors Error or array of errors * @param {string} requestAccepts Request's Accepts header */ - static async responseFromExternalError(errors: ErrOrErrArr, requestAccepts) { - let contentType; + static async responseFromError(errors: ErrOrErrArr, requestAccepts) { + return this.responseFromResult( + makeResultFromErrors((data: DocumentData) => new Document(data), errors), + requestAccepts, + false + ); + } + + /** + * Produces an HTTPResponse object from a JSON:API Result. + * This is useful when you produce the result from outside the library, + * rather than as the product of running a Query. + * + * @param {Result} result The result to send + * @param {string} reqAccepts The `Accept` header of the user's request + * @param {boolean= true} allow406 If no acceptable Content-Type can be + * negotiated given reqAccets, whether to send a 406 and replace the + * body of the resulting HTTPResponse with a 406 error message, or + * whether to ignore the user's `Accept` and send the result as is + * with Content-Type: application/vnd.api+json. + * @returns {HTTPResponse} + */ + static async responseFromResult( + result: Result, + reqAccepts?: string, + allow406: boolean = true + ) { + let contentType: string; + try { - contentType = await negotiateContentType(requestAccepts, ["application/vnd.api+json"]) - } catch (e) { - // if we couldn't find any acceptable content-type, - // just ignore the accept header, as http allows. - contentType = "application/vnd.api+json"; + contentType = await negotiateContentType(reqAccepts, ["application/vnd.api+json"]); + return resultToHTTPResponse(result, contentType); } - return resultToHTTPResponse( - makeResultFromErrors((data: DocumentData) => new Document(data), errors), - contentType - ); + catch (e) { + // If we allow406, replace result with the 406 error message. + const finalResult = allow406 + ? makeResultFromErrors((data: DocumentData) => new Document(data), e) + : result; + + return resultToHTTPResponse(finalResult, "application/vnd.api+json"); + } } public static supportedExt = Object.freeze([]); @@ -255,13 +382,11 @@ class APIController { } } -export default APIController; - /** * Creates a JSON:API Result from an error or array of errors. */ -function makeResultFromErrors(makeDoc: makeDoc, errors: ErrOrErrArr): Result { +function makeResultFromErrors(makeDoc: makeDocument, errors: ErrOrErrArr): Result { const errorsArray = (Array.isArray(errors) ? errors : [errors]) .map(<(v: any) => APIError>APIError.fromError.bind(APIError)); @@ -318,3 +443,31 @@ function resultToHTTPResponse(response: Result, negotiatedMediaType?: string): H function pickStatus(errStatuses) { return errStatuses[0]; } + +function isBulkDelete(request: Request) { + return request.method === "delete" && !request.id && !request.aboutRelationship; +} + +function parseAsLinkage(request: Request) { + return request.aboutRelationship || isBulkDelete(request); +} + + +async function transformDoc(doc: Document, mode: TransformMode, extras: Extras) { + // Create Data, or read private internal Data if it exists, and transform it. + const res = doc.clone(); + const primaryData = res.primary && (res.primary).data; + const includedData = doc.included && Data.of(doc.included); + + if(primaryData) { + (res.primary).data = + await applyTransform(primaryData, mode, extras); + } + + if(includedData) { + res.included = + (await applyTransform(includedData, mode, extras)).values; + } + + return res; +} diff --git a/src/controllers/Documentation.ts b/src/controllers/Documentation.ts index d46d72d3..f824a226 100755 --- a/src/controllers/Documentation.ts +++ b/src/controllers/Documentation.ts @@ -6,10 +6,12 @@ import dasherize = require("dasherize"); import mapValues = require("lodash/object/mapValues"); import ResourceTypeRegistry, { ResourceTypeDescription, ResourceTypeInfo } from "../ResourceTypeRegistry"; -import { HTTPResponse } from "../types"; +import { HTTPResponse, ServerReq, ServerRes, Request } from "../types"; import ResourceSet from "../types/ResourceSet"; import Document from "../types/Document"; import Resource from "../types/Resource"; +import { IncomingMessage, ServerResponse } from 'http'; +export { IncomingMessage, ServerResponse }; export default class DocumentationController { private registry: ResourceTypeRegistry; @@ -41,7 +43,7 @@ export default class DocumentationController { this.templateData = data; } - handle(request, frameworkReq, frameworkRes) { + handle = async (request: Request, serverReq: ServerReq, serverRes: ServerRes) => { const response: HTTPResponse = { headers: {}, body: undefined, status: 200 }; const negotiator = new Negotiator({headers: {accept: request.accepts}}); const contentType = negotiator.mediaType(["text/html", "application/vnd.api+json"]); @@ -53,7 +55,7 @@ export default class DocumentationController { // process templateData (just the type infos for now) for this particular request. const templateData = _.cloneDeep(this.templateData, cloneCustomizer); templateData.resourcesMap = mapValues(templateData.resourcesMap, (typeInfo, typeName) => { - return this.transformTypeInfo(typeName, typeInfo, request, response, frameworkReq, frameworkRes); + return this.transformTypeInfo(typeName, typeInfo, request, response, serverReq, serverRes); }); if(contentType && contentType.toLowerCase() === "text/html") { @@ -76,7 +78,7 @@ export default class DocumentationController { }).toString(); } - return Promise.resolve(response); + return response; } // Clients can extend this if, say, the adapter can't infer diff --git a/src/http-strategies/Base.ts b/src/http-strategies/Base.ts index 4f2df888..ee23c59e 100755 --- a/src/http-strategies/Base.ts +++ b/src/http-strategies/Base.ts @@ -2,7 +2,7 @@ import qs = require("qs"); import getRawBody = require("raw-body"); import logger from "../util/logger"; import APIError from "../types/APIError"; -import { Request } from "../types/"; +import { Request, ServerReq, ServerRes, HTTPResponse } from "../types/"; import APIController from "../controllers/API"; import DocsController from "../controllers/Documentation"; @@ -12,6 +12,9 @@ export type HTTPStrategyOptions = { host?: string }; +export type Controller = + (request: Request, req: ServerReq, res: ServerRes) => Promise; + /** * This controller is the base for http strategy classes. It's built around * the premise that most if not all http frameworks are built on top of the @@ -41,7 +44,11 @@ export default class BaseStrategy { protected docs: DocsController; protected config: HTTPStrategyOptions; - constructor(apiController: APIController, docsController: DocsController, options?: HTTPStrategyOptions) { + constructor( + apiController: APIController, + docsController: DocsController, + options?: HTTPStrategyOptions + ) { this.api = apiController; this.docs = docsController; @@ -57,7 +64,6 @@ export default class BaseStrategy { "unless you have reason to trust the (X-Forwarded-)Host header." ); } - } /** @@ -73,14 +79,20 @@ export default class BaseStrategy { * @param {Object} params object containing url parameters * @param {Object} [parsedQuery] object containing pre-parsed query parameters */ - protected async buildRequestObject(req, protocol, fallbackHost, params, parsedQuery?): Promise { + protected async buildRequestObject( + req, + protocol, + fallbackHost, + params, + parsedQuery? + ): Promise { const queryStartIndex = req.url.indexOf("?"); const hasQuery = queryStartIndex !== -1; const rawQueryString = hasQuery && req.url.substr(queryStartIndex + 1); const protocolGuess = protocol || (req.connection.encrypted ? "https" : "http"); const host = this.config.host || fallbackHost; - const body = await this.getParsedBodyJSON(req); + const body = await this.getParsedBodyJSON(req); // could throw, rejecting promise. return { // Handle route & query params @@ -106,10 +118,10 @@ export default class BaseStrategy { } else if(requestedMethod) { - const msg = - `Cannot tunnel to the method "${requestedMethod.toUpperCase()}".`; - - throw new APIError(400, undefined, msg); + throw new APIError({ + status: 400, + title: `Cannot tunnel to method "${requestedMethod.toUpperCase()}".` + }); } return usedMethod; diff --git a/src/http-strategies/Express.ts b/src/http-strategies/Express.ts index 5b793b80..6517fcaa 100755 --- a/src/http-strategies/Express.ts +++ b/src/http-strategies/Express.ts @@ -1,8 +1,13 @@ import varyLib = require("vary"); -import API from "../controllers/API"; -import Base, { HTTPStrategyOptions } from "./Base"; +import depd = require('depd') +import R = require('ramda'); +import logger from '../util/logger'; +import API, { RequestOpts, QueryBuildingContext } from "../controllers/API"; +import Base, { HTTPStrategyOptions, Controller } from "./Base"; import Query from "../types/Query/Query"; -import { Request } from "express"; +import { HTTPResponse, Request as JSONAPIRequest, Result } from "../types"; +import { Request, Response, NextFunction } from "express"; +const deprecate = depd("json-api"); // Note: however, having one object type with both possible callback signatures // in it doesn't work, but splitting these function signatures into separate @@ -12,16 +17,16 @@ import { Request } from "express"; // this behavior is probably subject to change (e.g., might be effected by // https://github.com/Microsoft/TypeScript/pull/17819). However, it's the // approach that the express typings seem to use, so I imagine it's safe enough. -export type QueryTransformCurried = { +export type DeprecatedQueryTransformNoReq = { (first: Query): Query; }; -export type QueryTransformWithReq = { +export type DeprecatedQueryTransformWithReq = { (first: Request, second: Query): Query } -export type QueryTransform = - QueryTransformCurried | QueryTransformWithReq; +export type DeprecatedQueryTransform = + DeprecatedQueryTransformNoReq | DeprecatedQueryTransformWithReq; /** * This controller receives requests directly from express and sends responses @@ -54,42 +59,29 @@ export default class ExpressStrategy extends Base { super(apiController, docsController, options); } - // For requests for the documentation. - // Note: this will ignore any port number if you're using Express 4. - // See: https://expressjs.com/en/guide/migrating-5.html#req.host - // The workaround is to use the host configuration option. - docsRequest(req, res, next) { - this.buildRequestObject(req, req.protocol, req.host, req.params, req.query) - .then((requestObject) => { - return this.docs.handle(requestObject, req, res) - .then((responseObject) => { - this.sendResources(responseObject, res, next); - }); - }) - .catch((err) => { - this.sendError(err, req, res); - }); + protected buildRequestObject(req: Request): Promise { + return super.buildRequestObject(req, req.protocol, req.host, req.params, req.query) } - sendResources(responseObject, res, next) { - const { vary, ...otherHeaders } = responseObject.headers; + protected sendResponse(response: HTTPResponse, res: Response, next: NextFunction) { + const { vary, ...otherHeaders } = response.headers; if(vary) { varyLib(res, vary); } - if(responseObject.status === 406 && !this.config.handleContentNegotiation) { + if(response.status === 406 && !this.config.handleContentNegotiation) { return next(); } - res.status(responseObject.status || 200); + res.status(response.status || 200); Object.keys(otherHeaders).forEach(k => { res.set(k, otherHeaders[k]); }) - if(responseObject.body !== undefined) { - res.send(new Buffer(responseObject.body)).end(); + if(response.body !== undefined) { + res.send(new Buffer(response.body)).end(); } else { @@ -97,6 +89,74 @@ export default class ExpressStrategy extends Base { } } + protected doRequest = async ( + controller: Controller, + req: Request, + res: Response, + next: NextFunction + ) => { + try { + const requestObj = await this.buildRequestObject(req); + const responseObj = await controller(requestObj, req, res); + this.sendResponse(responseObj, res, next); + } + catch (err) { + // This case should only occur if building a request object fails, as the + // controller should catch any internal errors and always returns a response. + this.sendError(err, req, res, next); + } + } + + /** + * A middleware to handle requests for the documentation. + * Note: this will ignore any port number if you're using Express 4. + * See: https://expressjs.com/en/guide/migrating-5.html#req.host + * The workaround is to use the host configuration option. + */ + docsRequest = R.partial(this.doRequest, [this.docs.handle]); + + /** + * A middleware to handle supported API requests. + * + * Supported requests included: GET /:type, GET /:type/:id/:relationship, + * POST /:type, PATCH /:type/:id, PATCH /:type, DELETE /:type/:id, + * DELETE /:type, GET /:type/:id/relationships/:relationship, + * PATCH /:type/:id/relationships/:relationship, + * POST /:type/:id/relationships/:relationship, and + * DELETE /:type/:id/relationships/:relationship. + * + * Note: this will ignore any port number if you're using Express 4. + * See: https://expressjs.com/en/guide/migrating-5.html#req.host + * The workaround is to use the host configuration option. + */ + apiRequest = R.partial(this.doRequest, [this.api.handle]); + + /** + * Takes arguments for how to build the query, and returns a middleware + * function that will respond to incoming requests with those query settings. + * + * @param {RequestCustomizationOpts} opts + * @returns {RequestHandler} A middleware for fulfilling API requests. + */ + customAPIRequest = (opts: RequestOpts) => + R.partial(this.doRequest, [ + (request, req, res) => this.api.handle(request, req, res, opts) + ]); + + transformedAPIRequest = (queryTransform: DeprecatedQueryTransform) => { + deprecate('transformedAPIRequest: use customAPIRequest instead.'); + + return this.customAPIRequest({ + queryFactory: async (opts: QueryBuildingContext) => { + const req = opts.serverReq as Request; + const query = await this.api.makeQuery(opts); + return queryTransform.length > 1 + ? (queryTransform as DeprecatedQueryTransformWithReq)(req, query) + : (queryTransform as DeprecatedQueryTransformNoReq)(query); + } + }); + } + /** * A user of this library may wish to send an error response for an exception * that originated outside of the JSON API Pipeline and that's outside the @@ -107,48 +167,33 @@ export default class ExpressStrategy extends Base { * @param {Object} req Express's request object * @param {Object} res Express's response object */ - sendError(errors, req, res) { - API.responseFromExternalError(errors, req.headers.accept).then( - (responseObject) => this.sendResources(responseObject, res, () => {}) - ).catch((err) => { - // if we hit an error generating our error... - res.status(err.status).send(err.message); - }); - } + sendError = async (errors, req, res, next) => { + if(!next) { + deprecate("sendError with 3 arguments: must now provide next function."); + next = (err: any) => {} + } - apiRequest(req, res, next) { - return this.apiRequestWithTransform(undefined, req, res, next); - } + try { + const responseObj = await API.responseFromError(errors, req.headers.accept); + this.sendResponse(responseObj, res, next) + } - transformedAPIRequest(queryTransform: QueryTransform) { - return this.apiRequestWithTransform.bind(this, queryTransform); + catch(err) { + logger.warn("Hit an unexpected error creating or sending response. This shouldn't happen."); + next(err); + } } - // For requests like GET /:type, GET /:type/:id/:relationship, - // POST /:type PATCH /:type/:id, PATCH /:type, DELETE /:type/:idOrLabel, - // DELETE /:type, GET /:type/:id/links/:relationship, - // PATCH /:type/:id/links/:relationship, POST /:type/:id/links/:relationship, - // and DELETE /:type/:id/links/:relationship. - // Note: this will ignore any port number if you're using Express 4. - // See: https://expressjs.com/en/guide/migrating-5.html#req.host - // The workaround is to use the host configuration option. - private apiRequestWithTransform(queryTransform, req, res, next) { - // Support query transform functions where the query is the only argument, - // but also ones that expect (req, query). - queryTransform = queryTransform && queryTransform.length > 1 - ? queryTransform.bind(undefined, req) - : queryTransform; - - this.buildRequestObject(req, req.protocol, req.host, req.params, req.query) - .then((requestObject) => { - return this.api.handle(requestObject, req, res, queryTransform) - .then((responseObject) => { - this.sendResources(responseObject, res, next); - }); - }) - .catch((err) => { - this.sendError(err, req, res); - }); + sendResult = async (result: Result, req, res, next) => { + try { + const responseObj = await API.responseFromResult(result, req.headers.accept, true); + this.sendResponse(responseObj, res, next) + } + + catch(err) { + logger.warn("Hit an unexpected error creating or sending response. This shouldn't happen."); + next(err); + } } /** diff --git a/src/http-strategies/Koa.ts b/src/http-strategies/Koa.ts index 63fef0b8..136eb802 100755 --- a/src/http-strategies/Koa.ts +++ b/src/http-strategies/Koa.ts @@ -69,7 +69,7 @@ export default class KoaStrategy extends Base { }; } - sendResources(responseObject, ctx): void | true { + protected sendResources(responseObject, ctx): void | true { const { vary, ...otherHeaders } = responseObject.headers; if(vary) { @@ -101,7 +101,7 @@ export default class KoaStrategy extends Base { * @param {Object} ctx Koa's context object */ sendError(errors, ctx) { - API.responseFromExternalError(errors, ctx.headers.accept).then( + API.responseFromError(errors, ctx.headers.accept).then( (responseObject) => this.sendResources(responseObject, ctx) ).catch((err) => { // if we hit an error generating our error... diff --git a/src/steps/apply-transform.ts b/src/steps/apply-transform.ts index d4d065bc..596fceab 100755 --- a/src/steps/apply-transform.ts +++ b/src/steps/apply-transform.ts @@ -1,13 +1,17 @@ +import depd = require("depd"); import ResourceTypeRegistry from "../ResourceTypeRegistry"; -import { Request } from '../types/'; +import { FinalizedRequest, ServerReq, ServerRes } from '../types/'; import Data from "../types/Generic/Data"; import Resource from "../types/Resource"; import ResourceIdentifier from "../types/ResourceIdentifier"; +const deprecate = depd("json-api"); export type Extras = { - frameworkReq: any, - frameworkRes: any, - request: Request, + request: FinalizedRequest, + serverReq: ServerReq, + serverRes: ServerRes, + frameworkReq?: ServerReq, + frameworkRes?: ServerRes, registry: ResourceTypeRegistry }; @@ -15,8 +19,8 @@ export type Transformable = Resource | ResourceIdentifier; export type TransformMode = 'beforeSave' | 'beforeRender'; export type TransformFn = ( resourceOrIdentifier: T, - frameworkReq: Extras['frameworkReq'], - frameworkRes: Extras['frameworkRes'], + serverReq: Extras['serverReq'], + serverRes: Extras['serverRes'], superFn: TransformFn, extras: Extras ) => T | undefined | Promise; @@ -31,6 +35,12 @@ export default function transform( ): Promise> { const { registry } = extras; + // Copy properties for backwards compatibility, with deprecation warning. + extras.frameworkReq = extras.serverReq; + extras.frameworkRes = extras.serverRes; + deprecate.property(extras, 'frameworkReq', 'frameworkReq: use serverReq prop instead.'); + deprecate.property(extras, 'frameworkRes', 'frameworkRes: use serverReq prop instead.'); + // Whether to skip transforming a given item. const skipTransform = (it: Transformable, typeForTransform: string) => it instanceof ResourceIdentifier && !registry.transformLinkage(typeForTransform); @@ -81,8 +91,8 @@ export default function transform( const transformed = await transformFn( it, - extras.frameworkReq, - extras.frameworkRes, + extras.serverReq, + extras.serverRes, superFn, extras ); diff --git a/src/steps/make-query/make-delete.ts b/src/steps/make-query/make-delete.ts index 134027eb..c196d14b 100755 --- a/src/steps/make-query/make-delete.ts +++ b/src/steps/make-query/make-delete.ts @@ -1,13 +1,15 @@ import APIError from "../../types/APIError"; -import { Request, makeDoc } from "../../types"; +import { FinalizedRequest, makeDocument } from "../../types"; import ResourceTypeRegistry from "../../ResourceTypeRegistry"; import Data from "../../types/Generic/Data"; import ResourceIdentifier from "../../types/ResourceIdentifier"; import DeleteQuery from "../../types/Query/DeleteQuery"; import RemoveFromRelationshipQuery from "../../types/Query/RemoveFromRelationshipQuery"; -export default function(request: Request, registry: ResourceTypeRegistry, makeDoc: makeDoc) { +export default function(request: FinalizedRequest, registry: ResourceTypeRegistry, makeDoc: makeDocument) { const type = request.type; + // There may not be a document, but if there is it'll have primary data. + const primary = request.document && (request.document.primary as any).data; if(request.aboutRelationship) { if(!request.id || !request.relationship) { @@ -22,7 +24,7 @@ export default function(request: Request, registry: ResourceTypeRegistry, makeDo type: type, id: request.id, relationshipName: request.relationship, - linkage: >request.primary, + linkage: >primary, returning: () => ({ status: 204 }) }); } @@ -30,11 +32,11 @@ export default function(request: Request, registry: ResourceTypeRegistry, makeDo // Bulk delete of resources const bulkDelete = !request.id; if(bulkDelete) { - if(!request.primary) { + if(!primary) { throw new Error("Bulk delete without a body is not possible.") } - if(request.primary.isSingular) { + if(primary.isSingular) { const title = "You must provide an array of resource identifier objects to do a bulk delete."; throw new APIError(400, undefined, title); } @@ -45,7 +47,7 @@ export default function(request: Request, registry: ResourceTypeRegistry, makeDo returning: () => ({ status: 204 }), [bulkDelete ? 'ids' : 'id']: bulkDelete - ? (>request.primary).map((it) => it.id).values + ? (>primary).map((it) => it.id).values : request.id }); } diff --git a/src/steps/make-query/make-get.ts b/src/steps/make-query/make-get.ts index 7107240c..604f6723 100755 --- a/src/steps/make-query/make-get.ts +++ b/src/steps/make-query/make-get.ts @@ -1,11 +1,11 @@ import APIError from "../../types/APIError"; -import { Request, makeDoc } from "../../types"; +import { FinalizedRequest, makeDocument } from "../../types"; import Resource from "../../types/Resource"; import ResourceSet from "../../types/ResourceSet"; import ResourceTypeRegistry from "../../ResourceTypeRegistry"; import FindQuery from "../../types/Query/FindQuery"; -export default function(request: Request, registry: ResourceTypeRegistry, makeDoc: makeDoc) { +export default function(request: FinalizedRequest, registry: ResourceTypeRegistry, makeDoc: makeDocument) { const type = request.type; // Handle fields, sorts, includes and filters. diff --git a/src/steps/make-query/make-patch.ts b/src/steps/make-query/make-patch.ts index 6c5b9d86..9cda0e59 100755 --- a/src/steps/make-query/make-patch.ts +++ b/src/steps/make-query/make-patch.ts @@ -6,11 +6,11 @@ import ResourceTypeRegistry from "../../ResourceTypeRegistry"; import Data from "../../types/Generic/Data"; import ResourceSet from "../../types/ResourceSet"; import ResourceIdentifier from "../../types/ResourceIdentifier"; -import { Request, makeDoc } from "../../types"; +import { FinalizedRequest, makeDocument } from "../../types"; -export default function(request: Request, registry: ResourceTypeRegistry, makeDoc: makeDoc) { - const primary = | Data>request.primary; - const type = request.type; +export default function(request: FinalizedRequest, registry: ResourceTypeRegistry, makeDoc: makeDocument) { + const type = request.type; + const primary = | Data>(request.document!.primary as any).data; let changedResourceData; if(!request.aboutRelationship) { @@ -50,7 +50,7 @@ export default function(request: Request, registry: ResourceTypeRegistry, makeDo undefined, { [request.relationship]: Relationship.of({ - data: >request.primary, + data: >primary, owner: { type: request.type, id: request.id, path: request.relationship } }) } diff --git a/src/steps/make-query/make-post.ts b/src/steps/make-query/make-post.ts index 0f8e5f61..d84c93ca 100755 --- a/src/steps/make-query/make-post.ts +++ b/src/steps/make-query/make-post.ts @@ -2,21 +2,21 @@ import APIError from "../../types/APIError"; import Resource from "../../types/Resource"; import CreateQuery from "../../types/Query/CreateQuery"; import AddToRelationshipQuery from '../../types/Query/AddToRelationshipQuery'; -import { Request, Result } from "../../types"; +import { FinalizedRequest, Result } from "../../types"; import Data from "../../types/Generic/Data"; import ResourceSet from "../../types/ResourceSet"; import ResourceIdentifier from "../../types/ResourceIdentifier"; import ResourceTypeRegistry from "../../ResourceTypeRegistry"; import templating = require("url-template"); -export default function(request: Request, registry: ResourceTypeRegistry, makeDoc) { - const primary = request.primary; +export default function(request: FinalizedRequest, registry: ResourceTypeRegistry, makeDoc) { + const primary = (request.document!.primary as any).data; const type = request.type; // We're going to do an adapter.create, below, EXCEPT if we're adding to // an existing toMany relationship, which uses a different adapter method. if(request.aboutRelationship) { - if((request.primary as Data).isSingular) { + if((primary as Data).isSingular) { throw new APIError( 400, undefined, diff --git a/src/types/index.ts b/src/types/index.ts index 17c63797..92a5b46f 100755 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -2,6 +2,8 @@ import Resource, { ResourceJSON } from './Resource'; import ResourceIdentifier, { ResourceIdentifierJSON } from "./ResourceIdentifier"; import Document, { DocumentData } from "./Document"; import Data from "./Generic/Data"; +import { ParsedQueryParams } from '../steps/pre-query/parse-query-params'; +import { IncomingMessage, ServerResponse } from "http"; // A helper type to capture the ability of // a given type T to appear singularly or in an array. @@ -25,6 +27,10 @@ export type PredicateFn = (it: T, i: number, arr: T[]) => boolean; export type Mapper = (it: T, i: number, arr: T[]) => U; export type AsyncMapper = (it: T, i: number, arr: T[]) => U | Promise; +// Types for interacting with the underlying server +export type ServerReq = IncomingMessage; +export type ServerRes = ServerResponse; + // Types related to queries export type Sort = { field: string; direction: 'ASC' | 'DESC' }; @@ -96,11 +102,13 @@ export type Request = { // The prop below will be used in the future with the JSON:API ext system. //ext: string[]; +} - // Any primary data included in the request's body. - // Necessary for creating and updating our resources. - // Not present when request first created (parsed later from body). - primary?: Data | Data; +// A request, with its query parameters parsed for their +// JSON:API-specific format, and the body (if any) parsed into a Document. +export type FinalizedRequest = Request & { + queryParams: ParsedQueryParams, + document: Document | undefined } // Result is different than HTTPResponse in that it contains details of the @@ -126,4 +134,4 @@ export interface HTTPResponse { body?: string; } -export type makeDoc = (data: DocumentData) => Document; +export type makeDocument = (data: DocumentData) => Document; diff --git a/test/app/src/index.ts b/test/app/src/index.ts index 8240b89d..94e80488 100755 --- a/test/app/src/index.ts +++ b/test/app/src/index.ts @@ -1,4 +1,5 @@ import express = require("express"); +import R = require("ramda"); import API = require("../../../src/index"); import Document from "../../../src/types/Document"; import APIError from "../../../src/types/APIError"; @@ -6,8 +7,9 @@ import Query from "../../../src/types/Query/Query"; import FindQuery from "../../../src/types/Query/FindQuery"; import ResourceSet from "../../../src/types/ResourceSet"; import database from "../database/index"; +import { QueryBuildingContext } from "../../../src/controllers/API"; import { Express } from "express"; -export { Express } from "express"; +export { Express, QueryBuildingContext }; /** * Export a promise for the app. @@ -80,8 +82,7 @@ export default database.then(function(dbModule) { ) ); - app.get('/:type(people)/custom-filter-test/', - FrontWithCustomFilterSupport.apiRequest.bind(FrontWithCustomFilterSupport)); + app.get('/:type(people)/custom-filter-test/', FrontWithCustomFilterSupport.apiRequest); // Apply a query transform that puts all the names in meta app.get('/:type(people)/with-names', @@ -100,30 +101,86 @@ export default database.then(function(dbModule) { }) ); -// Add a route that delegates to next route on 406. -app.get('/:type(organizations)/no-id/406-delegation-test', - FrontWith406Delegation.apiRequest.bind(FrontWith406Delegation), - (req, res, next) => { - res.header('Content-Type', 'text/plain') - res.send("Hello from express"); - }) + // Add a route that delegates to next route on 406. + app.get('/:type(organizations)/no-id/406-delegation-test', + FrontWith406Delegation.apiRequest, + (req, res, next) => { + res.header('Content-Type', 'text/plain') + res.send("Hello from express"); + }) -// Now, add the routes. -// Note: below, express incorrectly passes requests using PUT and other -// unknown methods into the API Controller at some routes. We're doing this -// here just to test that the controller rejects them properly. -app.get("/", Front.docsRequest.bind(Front)); -app.route("/:type(people|organizations|schools)").all(apiReqHandler); -app.route("/:type(people|organizations|schools)/:id") - .get(apiReqHandler).patch(apiReqHandler).delete(apiReqHandler); -app.route("/:type(organizations|schools)/:id/:related") // not supported yet. - .get(apiReqHandler); -app.route("/:type(people|organizations|schools)/:id/relationships/:relationship") - .get(apiReqHandler).post(apiReqHandler).patch(apiReqHandler); - -app.use(function(req, res, next) { - Front.sendError({ "message": "Not Found", "status": 404 }, req, res); -}); + app.get('/hardcoded-result', R.partial( + Front.sendResult, [{ + status: 201, + document: new Document({ meta: { "hardcoded result": true } }) + }] + )); + + app.post('/sign-in', Front.customAPIRequest({ queryFactory: makeSignInQuery })); + + app.post('/sign-in/with-before-render', Front.customAPIRequest({ + queryFactory: (opts) => { + const signInQuery = makeSignInQuery(opts); + + return signInQuery.resultsIn( + R.pipe(signInQuery.returning, async (origResultPromise) => { + const origResult = await origResultPromise; + + if(origResult.document) { + origResult.document = + await opts.transformDocument(origResult.document, 'beforeRender'); + } -return app; + return origResult; + }) + ); + } + })) + + // Now, add the routes. + // Note: below, express incorrectly passes requests using PUT and other + // unknown methods into the API Controller at some routes. We're doing this + // here just to test that the controller rejects them properly. + app.get("/", Front.docsRequest); + app.route("/:type(people|organizations|schools)").all(Front.apiRequest); + app.route("/:type(people|organizations|schools)/:id") + .get(apiReqHandler).patch(apiReqHandler).delete(apiReqHandler); + app.route("/:type(organizations|schools)/:id/:related") // not supported yet. + .get(apiReqHandler); + app.route("/:type(people|organizations|schools)/:id/relationships/:relationship") + .get(apiReqHandler).post(apiReqHandler).patch(apiReqHandler); + + app.use(function(req, res, next) { + Front.sendError({ "message": "Not Found", "status": 404 }, req, res, next); + }); + + return app; }); + +function makeSignInQuery(opts: QueryBuildingContext) { + const { serverReq } = opts; + let authHeader = serverReq.headers.authorization; + + if(!authHeader) { + throw new APIError({ status: 400, title: "Missing user info." }); + } + + authHeader = Array.isArray(authHeader) ? authHeader[0] : authHeader; + const [user, pass] = Buffer.from(authHeader.substr(6), 'base64').toString().split(':'); + + return new FindQuery({ + type: "people", + singular: true, + filters: [{ field: "name", operator: "eq", value: user }], + returning: ([userData]) => { + if(pass !== 'password') { + throw new APIError(401); + } + + return { + // Note lack of beforeRender. + document: new Document({ primary: ResourceSet.of({ data: userData }) }) + } + } + }); +} diff --git a/test/app/src/resource-descriptions/people.ts b/test/app/src/resource-descriptions/people.ts index 257e199e..c36b2920 100755 --- a/test/app/src/resource-descriptions/people.ts +++ b/test/app/src/resource-descriptions/people.ts @@ -4,13 +4,17 @@ module.exports = { "relationship": "http://127.0.0.1:3000/people/{ownerId}/relationships/{path}" }, - beforeRender(it) { + beforeRender(it, frameworkReq) { // Hide the invisible sorcerer resource, and remove pointers to it as well // This id check works whether `it` is a Resource or ResourceIdentifier if(it.id === "59af14d3bbd18cd55ea08ea2") { return undefined; } + if(frameworkReq.url.startsWith('/sign-in')) { + it.attrs.signInBeforeRender = true; + } + return it; }, diff --git a/test/integration/fetch-resource/index.ts b/test/integration/fetch-resource/index.ts index 24ee0876..0cf244db 100755 --- a/test/integration/fetch-resource/index.ts +++ b/test/integration/fetch-resource/index.ts @@ -78,4 +78,15 @@ describe("Fetching Resources", () => { ]); }); }); + + describe("Fetching a static/hardcoded resource", () => { + it("should work", () => { + return Agent.request("GET", '/hardcoded-result') + .accept("application/vnd.api+json") + .then((res) => { + expect(res.body.meta).to.deep.equal({ "hardcoded result": true }); + expect(res.status).to.equal(201); + }); + }); + }); }); diff --git a/test/integration/http-compliance/index.ts b/test/integration/http-compliance/index.ts index b6d673b5..8074a454 100755 --- a/test/integration/http-compliance/index.ts +++ b/test/integration/http-compliance/index.ts @@ -2,37 +2,27 @@ import {expect} from "chai"; import AgentPromise from "../../app/agent"; describe("HTTP Compliance", () => { - let Agent; - before((done) => { - AgentPromise.then(A => { - Agent = A; - done(); - }, done); + before(() => { + return AgentPromise.then(A => { Agent = A; }); }); - it("should reject PUT with a PUT-specific message", (done) => { - Agent.request("PUT", "/organizations").send({}).promise().then( - (res) => { - done(new Error("Shouldn't run since response should be an error")); - }, - (err) => { - expect(err.response.status).to.equal(405); - expect(err.response.body.errors[0].detail).to.match(/PUT.+jsonapi\.org/i); - } - ).then(done, done); + it("should reject PUT with a PUT-specific message", () => { + return Agent.request("PUT", "/organizations").then((res) => { + throw new Error("Shouldn't run since response should be an error"); + }, (err) => { + expect(err.response.status).to.equal(405); + expect(err.response.body.errors[0].detail).to.match(/PUT.+jsonapi\.org/i); + }); }); - it("should reject other unknown methods too", (done) => { - Agent.request("LOCK", "/organizations").send({}).promise().then( - (res) => { - done(new Error("Shouldn't run since response should be an error")); - }, - (err) => { - expect(err.response.status).to.equal(405); - expect(err.response.body.errors[0].detail).to.match(/lock/i); - } - ).then(done, done); + it("should reject other unknown methods too", () => { + return Agent.request("LOCK", "/organizations").then(() => { + throw new Error("Shouldn't run since response should be an error"); + }, (err) => { + expect(err.response.status).to.equal(405); + expect(err.response.body.errors[0].detail).to.match(/lock/i); + }); }); });