diff --git a/README.md b/README.md index 3df85028..263666d9 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,8 @@ To use this library, you describe the special behavior (if any) that resources o - `beforeSave` (optional): a function called on each resource provided by the client (i.e. in a `POST` or `PATCH` request) before it's sent to the adapter for saving. You can transform the data here as necessary or pre-emptively reject the request. [Usage details](https://github.com/ethanresnick/json-api-example/blob/master/src/resource-descriptions/people.js#L25). +- `beforeDelete` (optional): a function called on each resource to be deleted, allowing you to throw an error if the action is not allowed. See [Handling Errors](#handling-errors) for more information. + - `labelMappers` (optional): this lets you create urls (or, in REST terminology, resources) that map to different database items over time. For example, you could have an `/events/upcoming` resource or a `/users/me` resource. In those examples, "upcoming" and "me" are called the labels and, in labelMappers, you provide a function that maps each label to the proper database id(s) at any given time. The function can return a Promise if needed. - `parentType` (optional): this allows you to designate one resource type being a sub-type of another (its `parentType`). This is often used when you have two resource types that live in the same database table/collection, and their type is determined with a discriminator key. See the [`schools` type](https://github.com/ethanresnick/json-api-example/blob/master/src/resource-descriptions/schools.js#L2) in the example repository. @@ -107,5 +109,19 @@ This library gives you a Front controller (shown in the example) that can handle In the example above, routing is handled with Express's built-in `app[VERB]` methods, and the three parameters are set properly using express's built-in `:param` syntax. If you're looking for something more robust, you might be interested in [Express Simple Router](https://github.com/ethanresnick/express-simple-router). For authentication, check out [Express Simple Firewall](https://github.com/ethanresnick/express-simple-firewall). +## Handling Errors + +This library provides an error contructor and a handler that you can use to return JSON API-compliant errors to the user. For an example, please see the [example repo](https://github.com/ethanresnick/json-api-example/blob/master/src/index.js#L64). + +You can also throw an APIError inside `beforeSave`, `beforeRender`, and `beforeDelete` transforms to cancel the request. + +```javascript +beforeDelete: function(resource, req, res, superFn) { + if (req.user.role !== "admin") { + throw APIError(403, undefined, "You are not allowed to delete this resource."); + } +} +``` + ## Database Adapters An adapter handles all the interaction with the database. It is responsible for turning requests into standard [`Resource`](https://github.com/ethanresnick/json-api/blob/master/src/types/Resource.js) or [`Collection`](https://github.com/ethanresnick/json-api/blob/master/src/types/Collection.js) objects that the rest of the library will use. See the built-in [MongooseAdapter](https://github.com/ethanresnick/json-api/blob/master/src/db-adapters/Mongoose/MongooseAdapter.js) for an example. diff --git a/build/src/ResourceTypeRegistry.js b/build/src/ResourceTypeRegistry.js index cdfaa716..2136473e 100644 --- a/build/src/ResourceTypeRegistry.js +++ b/build/src/ResourceTypeRegistry.js @@ -24,7 +24,7 @@ var _lodashObjectMerge2 = _interopRequireDefault(_lodashObjectMerge); * following same format. Those getters/setters will take the resource type * whose property is being retrieved/set, and the value to set it to, if any. */ -var autoGetterSetterProps = ["dbAdapter", "beforeSave", "beforeRender", "labelMappers", "defaultIncludes", "info", "parentType"]; +var autoGetterSetterProps = ["dbAdapter", "beforeSave", "beforeRender", "beforeDelete", "labelMappers", "defaultIncludes", "info", "parentType"]; /** * Global defaults for resource descriptions, to be merged into defaults diff --git a/build/src/controllers/API.js b/build/src/controllers/API.js index 8aa18fcb..93982ee0 100644 --- a/build/src/controllers/API.js +++ b/build/src/controllers/API.js @@ -26,6 +26,10 @@ var _typesDocument = require("../types/Document"); var _typesDocument2 = _interopRequireDefault(_typesDocument); +var _typesResource = require("../types/Resource"); + +var _typesResource2 = _interopRequireDefault(_typesResource); + var _typesCollection = require("../types/Collection"); var _typesCollection2 = _interopRequireDefault(_typesCollection); @@ -118,7 +122,7 @@ var APIController = (function () { // Kick off the chain for generating the response. return (0, _co2["default"])(_regeneratorRuntime.mark(function callee$2$0() { - var parsedPrimary, mappedLabel, mappedIsEmptyArray, errorsArr, apiErrors; + var parsedPrimary, mappedLabel, mappedIsEmptyArray, toTransform, errorsArr, apiErrors; return _regeneratorRuntime.wrap(function callee$2$0$(context$3$0) { while (1) switch (context$3$0.prev = context$3$0.next) { case 0: @@ -210,46 +214,65 @@ var APIController = (function () { } case 32: + if (!(request.method === "delete")) { + context$3$0.next = 37; + break; + } + + toTransform = undefined; + + if (Array.isArray(request.idOrIds)) { + toTransform = new _typesCollection2["default"](request.idOrIds.map(function (id) { + return new _typesResource2["default"](request.type, id); + })); + } else if (typeof request.idOrIds === "string") { + toTransform = new _typesResource2["default"](request.type, request.idOrIds); + } + + context$3$0.next = 37; + return (0, _stepsApplyTransform2["default"])(toTransform, "beforeDelete", registry, frameworkReq, frameworkRes); + + case 37: if (!(typeof response.primary === "undefined")) { - context$3$0.next = 47; + context$3$0.next = 52; break; } context$3$0.t0 = request.method; - context$3$0.next = context$3$0.t0 === "get" ? 36 : context$3$0.t0 === "post" ? 39 : context$3$0.t0 === "patch" ? 42 : context$3$0.t0 === "delete" ? 45 : 47; + context$3$0.next = context$3$0.t0 === "get" ? 41 : context$3$0.t0 === "post" ? 44 : context$3$0.t0 === "patch" ? 47 : context$3$0.t0 === "delete" ? 50 : 52; break; - case 36: - context$3$0.next = 38; + case 41: + context$3$0.next = 43; return (0, _stepsDoQueryDoGet2["default"])(request, response, registry); - case 38: - return context$3$0.abrupt("break", 47); + case 43: + return context$3$0.abrupt("break", 52); - case 39: - context$3$0.next = 41; + case 44: + context$3$0.next = 46; return (0, _stepsDoQueryDoPost2["default"])(request, response, registry); - case 41: - return context$3$0.abrupt("break", 47); + case 46: + return context$3$0.abrupt("break", 52); - case 42: - context$3$0.next = 44; + case 47: + context$3$0.next = 49; return (0, _stepsDoQueryDoPatch2["default"])(request, response, registry); - case 44: - return context$3$0.abrupt("break", 47); + case 49: + return context$3$0.abrupt("break", 52); - case 45: - context$3$0.next = 47; + case 50: + context$3$0.next = 52; return (0, _stepsDoQueryDoDelete2["default"])(request, response, registry); - case 47: - context$3$0.next = 55; + case 52: + context$3$0.next = 60; break; - case 49: - context$3$0.prev = 49; + case 54: + context$3$0.prev = 54; context$3$0.t1 = context$3$0["catch"](0); errorsArr = Array.isArray(context$3$0.t1) ? context$3$0.t1 : [context$3$0.t1]; apiErrors = errorsArr.map(_typesAPIError2["default"].fromError); @@ -265,9 +288,9 @@ var APIController = (function () { response.errors = response.errors.concat(apiErrors); //console.log("API CONTROLLER ERRORS", errorsArr[0], errorsArr[0].stack); - case 55: + case 60: if (!response.errors.length) { - context$3$0.next = 59; + context$3$0.next = 64; break; } @@ -277,16 +300,16 @@ var APIController = (function () { response.body = new _typesDocument2["default"](response.errors).get(true); return context$3$0.abrupt("return", response); - case 59: - context$3$0.next = 61; + case 64: + context$3$0.next = 66; return (0, _stepsApplyTransform2["default"])(response.primary, "beforeRender", registry, frameworkReq, frameworkRes); - case 61: + case 66: response.primary = context$3$0.sent; - context$3$0.next = 64; + context$3$0.next = 69; return (0, _stepsApplyTransform2["default"])(response.included, "beforeRender", registry, frameworkReq, frameworkRes); - case 64: + case 69: response.included = context$3$0.sent; if (response.status !== 204) { @@ -295,11 +318,11 @@ var APIController = (function () { return context$3$0.abrupt("return", response); - case 67: + case 72: case "end": return context$3$0.stop(); } - }, callee$2$0, this, [[0, 49]]); + }, callee$2$0, this, [[0, 54]]); })); } diff --git a/build/test/integration/delete-resource/index.js b/build/test/integration/delete-resource/index.js index 1d9fee3a..5c8b518f 100644 --- a/build/test/integration/delete-resource/index.js +++ b/build/test/integration/delete-resource/index.js @@ -8,28 +8,31 @@ var _appAgent = require("../../app/agent"); var _appAgent2 = _interopRequireDefault(_appAgent); -var _fixturesCreation = require("../fixtures/creation"); +describe("Delete Resource", function () { + var Agent = undefined; -describe("Deleting a resource", function () { - - var Agent = undefined, - id = undefined; before(function (done) { _appAgent2["default"].then(function (A) { Agent = A; - return Agent.request("POST", "/schools").type("application/vnd.api+json").send({ "data": _fixturesCreation.VALID_SCHOOL_RESOURCE_NO_ID }).promise().then(function (response) { - id = response.body.data.id; - return Agent.request("DEL", "/schools/" + id).type("application/vnd.api+json").send().promise(); - }, done).then(function () { - return done(); - }, done); - }, done)["catch"](done); - }); - - it("should delete a resource by id", function (done) { - Agent.request("GET", "/schools/" + id).accept("application/vnd.api+json").promise().then(done, function (err) { - (0, _chai.expect)(err.response.statusCode).to.equal(404); done(); })["catch"](done); }); + + describe("Valid deletion", function () { + it("should return 204", function (done) { + Agent.request("DEL", "/organizations/54419d550a5069a2129ef255").promise().then(function (res) { + (0, _chai.expect)(res.status).to.equal(204); + done(); + })["catch"](done); + }); + }); + + describe("Invalid deletion", function () { + it("should return 403", function (done) { + Agent.request("DEL", "/schools/53f54dd98d1e62ff12539db4").promise().then(done, function (err) { + (0, _chai.expect)(err.status).to.equal(403); + done(); + })["catch"](done); + }); + }); }); \ No newline at end of file diff --git a/src/ResourceTypeRegistry.js b/src/ResourceTypeRegistry.js index fe3859fc..7c6a77f8 100644 --- a/src/ResourceTypeRegistry.js +++ b/src/ResourceTypeRegistry.js @@ -7,7 +7,7 @@ import merge from "lodash/object/merge"; * whose property is being retrieved/set, and the value to set it to, if any. */ const autoGetterSetterProps = ["dbAdapter", "beforeSave", "beforeRender", - "labelMappers", "defaultIncludes", "info", "parentType"]; + "beforeDelete", "labelMappers", "defaultIncludes", "info", "parentType"]; /** * Global defaults for resource descriptions, to be merged into defaults diff --git a/src/controllers/API.js b/src/controllers/API.js index df12ccfa..addfa74c 100644 --- a/src/controllers/API.js +++ b/src/controllers/API.js @@ -2,6 +2,7 @@ import co from "co"; import Response from "../types/HTTP/Response"; import Document from "../types/Document"; +import Resource from "../types/Resource"; import Collection from "../types/Collection"; import APIError from "../types/APIError"; @@ -114,6 +115,24 @@ class APIController { } } + if (request.method === "delete") { + let toTransform; + + if (Array.isArray(request.idOrIds)) { + toTransform = new Collection( + request.idOrIds.map((id) => new Resource(request.type, id)) + ); + } + + else if (typeof request.idOrIds === "string") { + toTransform = new Resource(request.type, request.idOrIds); + } + + yield applyTransform( + toTransform, "beforeDelete", registry, frameworkReq, frameworkRes + ); + } + // Actually fulfill the request! // If we've already populated the primary resources, which is possible // because the label may have mapped to no id(s), we don't need to query. diff --git a/test/app/database/index.js b/test/app/database/index.js index ca748b9d..4662ebc2 100644 --- a/test/app/database/index.js +++ b/test/app/database/index.js @@ -9,8 +9,10 @@ import OrganizationModelSchema from "./models/organization"; /*eslint-disable new-cap */ const ObjectId = mongoose.Types.ObjectId; const govtId = ObjectId("54419d550a5069a2129ef254"); +const otherGovtId = ObjectId("54419d550a5069a2129ef255"); const smithId = ObjectId("53f54dd98d1e62ff12539db2"); const doeId = ObjectId("53f54dd98d1e62ff12539db3"); +const stateCollegeId = ObjectId("53f54dd98d1e62ff12539db4"); /*eslint-enable new-cap */ const OrganizationModel = OrganizationModelSchema.model; @@ -29,11 +31,12 @@ fixtures.save("all", { { name: "Doug Wilson", gender: "male" } ], Organization: [ - {name: "State Government", description: "Representing the good people.", liaisons: [doeId, smithId], _id: govtId} + {name: "State Government", description: "Representing the good people.", liaisons: [doeId, smithId], _id: govtId}, + {name: "Other State Government", description: "Representing the other good people.", _id: otherGovtId} ], School: [ {name: "City College", description: "Just your average local college.", liaisons: [smithId]}, - {name: "State College", description: "Just your average state college."} + {name: "State College", description: "Just your average state college.", _id: stateCollegeId} ] }); diff --git a/test/app/src/resource-descriptions/schools.js b/test/app/src/resource-descriptions/schools.js index 6c71421e..7bc56285 100755 --- a/test/app/src/resource-descriptions/schools.js +++ b/test/app/src/resource-descriptions/schools.js @@ -1,4 +1,6 @@ import { Promise } from "q"; +import API from "../../../../../index"; +let APIError = API.types.Error; module.exports = { parentType: "organizations", @@ -23,10 +25,14 @@ module.exports = { } }, - beforeSave: function(resource) { + beforeSave(resource) { return new Promise((resolve, reject) => { resource.attrs.description = "Modified in a Promise"; resolve(resource); }); + }, + + beforeDelete(resource) { + throw new APIError(403, undefined, "You are not allowed to delete schools."); } }; diff --git a/test/integration/delete-resource/index.js b/test/integration/delete-resource/index.js index 6e197ba3..bb8dc90e 100644 --- a/test/integration/delete-resource/index.js +++ b/test/integration/delete-resource/index.js @@ -1,34 +1,44 @@ import {expect} from "chai"; import AgentPromise from "../../app/agent"; -import { VALID_SCHOOL_RESOURCE_NO_ID } from "../fixtures/creation"; -describe("Deleting a resource", () => { +describe("Delete Resource", () => { + let Agent; - let Agent, id; - before(done => { - AgentPromise.then(A => { + before((done) => { + AgentPromise.then((A) => { Agent = A; - return Agent.request("POST", "/schools") - .type("application/vnd.api+json") - .send({"data": VALID_SCHOOL_RESOURCE_NO_ID}) + done(); + }).catch(done); + }); + + describe("Valid deletion", () => { + it("should return 204", (done) => { + Agent.request("DEL", "/organizations/54419d550a5069a2129ef255") .promise() - .then(response => { - id = response.body.data.id; - return Agent.request("DEL", `/schools/${id}`) - .type("application/vnd.api+json") - .send() - .promise(); - }, done).then(() => done(), done); - }, done).catch(done); + .then((res) => { + expect(res.status).to.equal(204); + done(); + }).catch(done); + }); + + it("should have deleted the resource", (done) => { + Agent.request("GET", "/organizations/54419d550a5069a2129ef255") + .promise() + .then(done, (err) => { + expect(err.status).to.equal(404); + done(); + }).catch(done); + }) }); - it("should delete a resource by id", done => { - Agent.request("GET", `/schools/${id}`) - .accept("application/vnd.api+json") - .promise() - .then(done, err => { - expect(err.response.statusCode).to.equal(404); - done(); - }).catch(done); + describe("Invalid deletion", () => { + it("should return 403", (done) => { + Agent.request("DEL", "/schools/53f54dd98d1e62ff12539db4") + .promise() + .then(done, (err) => { + expect(err.status).to.equal(403); + done(); + }).catch(done); + }); }); });