diff --git a/.eslintrc b/.eslintrc index 569f060b..99ad4231 100644 --- a/.eslintrc +++ b/.eslintrc @@ -4,7 +4,7 @@ "strict": 0, "curly": 0, - "indent": [2, 2, {indentSwitchCase: true}], + "indent": [2, 2, {"SwitchCase": 1}], "brace-style": [2, "stroustrup", { "allowSingleLine": true }], "new-cap": [2, {"capIsNewExceptions": ["Q", "Promise", "ValueObject", "Maybe", "mongoose.Schema", "Schema"]}], "quotes": [2, "double", "avoid-escape"], diff --git a/package.json b/package.json index 73f1f6a4..10e06b5a 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "license": "LGPL-3.0", "main": "index.js", "dependencies": { - "babel-runtime": "5.6.17", + "babel-runtime": "^5.8.35", "co": "4.5.x", "content-type": "1.x.x", "dasherize": "2.0.x", @@ -32,11 +32,11 @@ }, "devDependencies": { "babel": "5.6.14", - "babel-eslint": "3.x.x", + "babel-eslint": "^5.0.0", "chai": "^1.9.2", - "coveralls": "^2.11.4", "chai-subset": "^1.1.0", - "eslint": "0.x.x", + "coveralls": "^2.11.4", + "eslint": "~2.2.0", "express": "4.x.x", "gulp": "^3.8.6", "istanbul": "0.3.x", diff --git a/src/db-adapters/Mongoose/MongooseAdapter.js b/src/db-adapters/Mongoose/MongooseAdapter.js index a83b3c50..a20c7e63 100644 --- a/src/db-adapters/Mongoose/MongooseAdapter.js +++ b/src/db-adapters/Mongoose/MongooseAdapter.js @@ -32,108 +32,108 @@ export default class MongooseAdapter { find(type, idOrIds, fields, sorts, filters, includePaths) { const model = this.getModel(this.constructor.getModelName(type)); const queryBuilder = new mongoose.Query(null, null, model, model.collection); - const [mode, idQuery] = this.constructor.getIdQueryType(idOrIds); const pluralizer = this.inflector.plural; let primaryDocumentsPromise, includedResourcesPromise = Q(null); + return this.constructor.getIdQueryType(idOrIds, model).then(([ mode, idQuery]) => { + queryBuilder[mode](idQuery); - queryBuilder[mode](idQuery); - - // do sorting - if(Array.isArray(sorts)) { - queryBuilder.sort(sorts.join(" ")); - } - - // filter out invalid records with simple fields equality. - // note that there's a non-trivial risk of sql-like injection here. - // we're mostly protected by the fact that we're treating the filter's - // value as a single string, though, and not parsing as JSON. - if(typeof filters === "object" && !Array.isArray(filters)) { - queryBuilder.where(filters); - } - - // in an ideal world, we'd use mongoose here to filter the fields before - // querying. But, because the fields to filter can be scoped by type and - // we don't always know about a document's type until after query (becuase - // of discriminator keys), and because filtering out fields can really - // complicate population for includes, we don't yet filter at query time but - // instead just hide filtered fields in @docToResource. There is a more- - // efficient way to do this down the road, though--something like taking the - // provided fields and expanding them just enough (by looking at the type - // heirarachy and the relationship paths) to make sure that we're not going - // to run into any of the problems outlined above, while still querying for - // less data than we would without any fields restriction. For reference, the - // code for safely using the user's `fields` input, by putting them into a - // mongoose `.select()` object so that the user can't prefix a field with a - // minus on input to affect the query, is below. - // Reference: http://mongoosejs.com/docs/api.html#query_Query-select. - // let arrToSelectObject = (prev, curr) => { prev[curr] = 1; return prev; }; - // for(let type in fields) { - // fields[type] = fields[type].reduce(arrToSelectObject, {}); - // } - - // support includes, but only a level deep for now (recursive includes, - // especially if done in an efficient way query wise, are a pain in the ass). - if(includePaths) { - const populatedPaths = []; - const refPaths = util.getReferencePaths(model); - - includePaths = includePaths.map((it) => it.split(".")); - includePaths.forEach((pathParts) => { - // first, check that the include path is valid. - if(!arrayContains(refPaths, pathParts[0])) { - let title = "Invalid include path."; - let detail = `Resources of type "${type}" don't have a(n) "${pathParts[0]}" relationship.`; - throw new APIError(400, undefined, title, detail); - } + // do sorting + if(Array.isArray(sorts)) { + queryBuilder.sort(sorts.join(" ")); + } - if(pathParts.length > 1) { - throw new APIError(501, undefined, "Multi-level include paths aren't yet supported."); - } + // filter out invalid records with simple fields equality. + // note that there's a non-trivial risk of sql-like injection here. + // we're mostly protected by the fact that we're treating the filter's + // value as a single string, though, and not parsing as JSON. + if(typeof filters === "object" && !Array.isArray(filters)) { + queryBuilder.where(filters); + } - // Finally, do the population - populatedPaths.push(pathParts[0]); - queryBuilder.populate(pathParts[0]); - }); + // in an ideal world, we'd use mongoose here to filter the fields before + // querying. But, because the fields to filter can be scoped by type and + // we don't always know about a document's type until after query (becuase + // of discriminator keys), and because filtering out fields can really + // complicate population for includes, we don't yet filter at query time but + // instead just hide filtered fields in @docToResource. There is a more- + // efficient way to do this down the road, though--something like taking the + // provided fields and expanding them just enough (by looking at the type + // heirarachy and the relationship paths) to make sure that we're not going + // to run into any of the problems outlined above, while still querying for + // less data than we would without any fields restriction. For reference, the + // code for safely using the user's `fields` input, by putting them into a + // mongoose `.select()` object so that the user can't prefix a field with a + // minus on input to affect the query, is below. + // Reference: http://mongoosejs.com/docs/api.html#query_Query-select. + // let arrToSelectObject = (prev, curr) => { prev[curr] = 1; return prev; }; + // for(let type in fields) { + // fields[type] = fields[type].reduce(arrToSelectObject, {}); + // } + + // support includes, but only a level deep for now (recursive includes, + // especially if done in an efficient way query wise, are a pain in the ass). + if(includePaths) { + const populatedPaths = []; + const refPaths = util.getReferencePaths(model); + + includePaths = includePaths.map((it) => it.split(".")); + includePaths.forEach((pathParts) => { + // first, check that the include path is valid. + if(!arrayContains(refPaths, pathParts[0])) { + let title = "Invalid include path."; + let detail = `Resources of type "${type}" don't have a(n) "${pathParts[0]}" relationship.`; + throw new APIError(400, undefined, title, detail); + } + + if(pathParts.length > 1) { + throw new APIError(501, undefined, "Multi-level include paths aren't yet supported."); + } + + // Finally, do the population + populatedPaths.push(pathParts[0]); + queryBuilder.populate(pathParts[0]); + }); - let includedResources = []; - primaryDocumentsPromise = Q(queryBuilder.exec()).then((docs) => { - forEachArrayOrVal(docs, (doc) => { - // There's no gaurantee that the doc (or every doc) was found - // and we can't populate paths on a non-existent doc. - if(!doc) return; - - populatedPaths.forEach((path) => { - // if it's a toOne relationship, doc[path] will be a doc or undefined; - // if it's a toMany relationship, we have an array (or undefined). - let refDocs = Array.isArray(doc[path]) ? doc[path] : [doc[path]]; - refDocs.forEach((it) => { - // only include if it's not undefined. - if(it) { - includedResources.push( - this.constructor.docToResource(it, pluralizer, fields) - ); - } + let includedResources = []; + primaryDocumentsPromise = Q(queryBuilder.exec()).then((docs) => { + forEachArrayOrVal(docs, (doc) => { + // There's no gaurantee that the doc (or every doc) was found + // and we can't populate paths on a non-existent doc. + if(!doc) return; + + populatedPaths.forEach((path) => { + // if it's a toOne relationship, doc[path] will be a doc or undefined; + // if it's a toMany relationship, we have an array (or undefined). + let refDocs = Array.isArray(doc[path]) ? doc[path] : [doc[path]]; + refDocs.forEach((it) => { + // only include if it's not undefined. + if(it) { + includedResources.push( + this.constructor.docToResource(it, pluralizer, fields) + ); + } + }); }); }); - }); - return docs; - }); + return docs; + }); - includedResourcesPromise = primaryDocumentsPromise.then(() => - new Collection(includedResources) - ); - } + includedResourcesPromise = primaryDocumentsPromise.then(() => + new Collection(includedResources) + ); + } - else { - primaryDocumentsPromise = Q(queryBuilder.exec()); - } + else { + primaryDocumentsPromise = Q(queryBuilder.exec()); + } - return Q.all([primaryDocumentsPromise.then((it) => { - const makeCollection = !idOrIds || Array.isArray(idOrIds) ? true : false; - return this.constructor.docsToResourceOrCollection(it, makeCollection, pluralizer, fields); - }), includedResourcesPromise]).catch(util.errorHandler); + return Q.all([primaryDocumentsPromise.then((it) => { + const makeCollection = !idOrIds || Array.isArray(idOrIds) ? true : false; + return this.constructor.docsToResourceOrCollection(it, makeCollection, pluralizer, fields); + }), includedResourcesPromise]).catch(util.errorHandler); + }); } /** @@ -202,88 +202,89 @@ export default class MongooseAdapter { return it.id; }); - const [mode, idQuery] = this.constructor.getIdQueryType(idOrIds); - - return Q(model[mode](idQuery).exec()).then((docs) => { - const successfulSavesPromises = []; - - // if some ids were invalid/deleted/not found, we can't let *any* update - // succeed. this is the beginning of our simulation of transactions. - // There are two types of invalid cases here: we looked up one or more - // docs and got none back (i.e. docs === null) or we looked up an array of - // docs and got back docs that were missing some requested ids. - if(docs === null) { - throw new APIError(404, undefined, "No matching resource found."); - } - else { - const idOrIdsAsArray = Array.isArray(idOrIds) ? idOrIds : [idOrIds]; - const docIdOrIdsAsArray = Array.isArray(docs) ? docs.map(it => it.id) : [docs.id]; - - if(!arrayValuesMatch(idOrIdsAsArray, docIdOrIdsAsArray)) { - let title = "Some of the resources you're trying to update could not be found."; - throw new APIError(404, undefined, title); + return this.constructor.getIdQueryType(idOrIds, model) + .then(([ mode, idQuery]) => model[mode](idQuery).exec()) + .then((docs) => { + const successfulSavesPromises = []; + + // if some ids were invalid/deleted/not found, we can't let *any* update + // succeed. this is the beginning of our simulation of transactions. + // There are two types of invalid cases here: we looked up one or more + // docs and got none back (i.e. docs === null) or we looked up an array of + // docs and got back docs that were missing some requested ids. + if(docs === null) { + throw new APIError(404, undefined, "No matching resource found."); } - } + else { + const idOrIdsAsArray = Array.isArray(idOrIds) ? idOrIds : [idOrIds]; + const docIdOrIdsAsArray = Array.isArray(docs) ? docs.map(it => it.id) : [docs.id]; - forEachArrayOrVal(docs, (currDoc) => { - let newResource = changeSets[currDoc.id]; - - // Allowing the type to change is a bit of a pain. If the type's - // changed, it means the mongoose Model representing the doc must be - // different too. So we have to get the data from the old doc with - // .toObject(), change its discriminator, and then create an instance - // of the new model with that data. We also have to mark that new - // instance as not representing a new document, so that mongoose will - // do an update query rather than a save. Finally, we have to do all - // this before updating other attributes, so that they're correctly - // marked as modified when changed. - const currentModelName = currDoc.constructor.modelName; - const newModelName = this.constructor.getModelName(newResource.type, singular); - if(currentModelName !== newModelName) { - const newDoc = currDoc.toObject({virtuals: true, getters: true}); - const NewModelConstructor = this.getModel(newModelName); - newDoc[currDoc.constructor.schema.options.discriminatorKey] = newModelName; - - // replace the currDoc with our new creation. - currDoc = new NewModelConstructor(newDoc); - currDoc.isNew = false; + if(!arrayValuesMatch(idOrIdsAsArray, docIdOrIdsAsArray)) { + let title = "Some of the resources you're trying to update could not be found."; + throw new APIError(404, undefined, title); + } } - // update all attributes and links provided, ignoring type/meta/id. - currDoc.set(util.resourceToDocObject(newResource)); - - successfulSavesPromises.push( - Q.Promise(function (resolve, reject) { - currDoc.save((err, doc) => { - if(err) reject(err); - resolve(doc); - }); - }) - ); - }); + forEachArrayOrVal(docs, (currDoc) => { + let newResource = changeSets[currDoc.id]; + + // Allowing the type to change is a bit of a pain. If the type's + // changed, it means the mongoose Model representing the doc must be + // different too. So we have to get the data from the old doc with + // .toObject(), change its discriminator, and then create an instance + // of the new model with that data. We also have to mark that new + // instance as not representing a new document, so that mongoose will + // do an update query rather than a save. Finally, we have to do all + // this before updating other attributes, so that they're correctly + // marked as modified when changed. + const currentModelName = currDoc.constructor.modelName; + const newModelName = this.constructor.getModelName(newResource.type, singular); + if(currentModelName !== newModelName) { + const newDoc = currDoc.toObject({virtuals: true, getters: true}); + const NewModelConstructor = this.getModel(newModelName); + newDoc[currDoc.constructor.schema.options.discriminatorKey] = newModelName; + + // replace the currDoc with our new creation. + currDoc = new NewModelConstructor(newDoc); + currDoc.isNew = false; + } + + // update all attributes and links provided, ignoring type/meta/id. + currDoc.set(util.resourceToDocObject(newResource)); + + successfulSavesPromises.push( + Q.Promise(function (resolve, reject) { + currDoc.save((err, doc) => { + if(err) reject(err); + resolve(doc); + }); + }) + ); + }); - return Q.all(successfulSavesPromises); - }).then((docs) => { - let makeCollection = resourceOrCollection instanceof Collection; - return this.constructor.docsToResourceOrCollection(docs, makeCollection, plural); - }).catch(util.errorHandler); + return Q.all(successfulSavesPromises); + }).then((docs) => { + let makeCollection = resourceOrCollection instanceof Collection; + return this.constructor.docsToResourceOrCollection(docs, makeCollection, plural); + }).catch(util.errorHandler); } delete(parentType, idOrIds) { const model = this.getModel(this.constructor.getModelName(parentType)); - const [mode, idQuery] = this.constructor.getIdQueryType(idOrIds); - if(!idOrIds) { - return Q.Promise((resolve, reject) => { - reject(new APIError(400, undefined, "You must specify some resources to delete")); - }); - } + return this.constructor.getIdQueryType(idOrIds, model).then(([ mode, idQuery ]) => { + if(!idOrIds) { + return Q.Promise((resolve, reject) => { + reject(new APIError(400, undefined, "You must specify some resources to delete")); + }); + } - return Q(model[mode](idQuery).exec()).then((docs) => { - if(!docs) throw new APIError(404, undefined, "No matching resource found."); - forEachArrayOrVal(docs, (it) => { it.remove(); }); - return docs; - }).catch(util.errorHandler); + return Q(model[mode](idQuery).exec()).then((docs) => { + if(!docs) throw new APIError(404, undefined, "No matching resource found."); + forEachArrayOrVal(docs, (it) => { it.remove(); }); + return docs; + }).catch(util.errorHandler); + }); } /** @@ -603,30 +604,37 @@ export default class MongooseAdapter { return words.join(" "); } - static getIdQueryType(idOrIds) { - const mode = typeof idOrIds === "string" ? "findOne" : "find"; - let idQuery; + static getIdQueryType(idOrIds, Model) { + if (idOrIds == null) { + return Q([ "find", undefined ]); + } + + const [ mode, ids ] = Array.isArray(idOrIds) ? [ "find", idOrIds ] : [ "findOne", [ idOrIds ] ]; + const idValidationPromises = ids.map(id => this.validateId(id, Model)); - if (typeof idOrIds === "string") { - if (!this.idIsValid(idOrIds)) { + return Q.all(idValidationPromises).then(() => { + const idQuery = { _id: mode === "find" ? { $in: ids } : ids[0] }; + return [ mode, idOrIds != null ? idQuery : undefined ]; + }, err => { + if (mode === "findOne") { throw new APIError(404, undefined, "No matching resource found.", "Invalid ID."); } - - idQuery = {_id: idOrIds}; - } - - else if (Array.isArray(idOrIds)) { - if (!idOrIds.every(this.idIsValid)) { + else { throw new APIError(400, undefined, "Invalid ID."); } - - idQuery = {_id: {"$in": idOrIds}}; - } - - return [mode, idQuery]; + }); } - static idIsValid(id) { - return typeof id === "string" && /^[0-9a-fA-F]{24}$/.test(id); + static validateId(id, Model) { + return Q.Promise((resolve, reject) => { + if (id == null) { + return reject(); + } + + return (new Model({ _id: id })).validate().then( + (res) => resolve(true), + (err) => err.errors && err.errors._id ? reject(err.errors._id) : resolve(true) + ); + }); } } diff --git a/src/steps/pre-query/parse-request-primary.js b/src/steps/pre-query/parse-request-primary.js index 4eaedc56..44f3e949 100644 --- a/src/steps/pre-query/parse-request-primary.js +++ b/src/steps/pre-query/parse-request-primary.js @@ -37,7 +37,7 @@ export default function(data, parseAsLinkage) { function relationshipFromJSON(json) { if (typeof json.data === "undefined") { - throw new APIError(400, undefined, `Missing relationship linkage.`); + throw new APIError(400, undefined, "Missing relationship linkage."); } return new Relationship(linkageFromJSON(json.data)); diff --git a/test/app/database/index.js b/test/app/database/index.js index ca748b9d..31679cad 100644 --- a/test/app/database/index.js +++ b/test/app/database/index.js @@ -5,6 +5,8 @@ import fixtures from "node-mongoose-fixtures"; import PersonModel from "./models/person"; import makeSchoolModelConstructor from "./models/school"; import OrganizationModelSchema from "./models/organization"; +import NumericId from "./models/numeric-id"; +import StringId from "./models/string-id"; /*eslint-disable new-cap */ const ObjectId = mongoose.Types.ObjectId; @@ -19,7 +21,9 @@ const OrganizationSchema = OrganizationModelSchema.schema; const models = { Person: PersonModel, Organization: OrganizationModel, - School: makeSchoolModelConstructor(OrganizationModel, OrganizationSchema) + School: makeSchoolModelConstructor(OrganizationModel, OrganizationSchema), + NumericId, + StringId }; fixtures.save("all", { diff --git a/test/app/database/models/numeric-id.js b/test/app/database/models/numeric-id.js new file mode 100755 index 00000000..da1fac82 --- /dev/null +++ b/test/app/database/models/numeric-id.js @@ -0,0 +1,10 @@ +import mongoose from "mongoose"; + +const schema = mongoose.Schema({ + _id: { + type: Number, + required: true + } +}); + +export default mongoose.model("NumericId", schema); diff --git a/test/app/database/models/string-id.js b/test/app/database/models/string-id.js new file mode 100755 index 00000000..fb8deadf --- /dev/null +++ b/test/app/database/models/string-id.js @@ -0,0 +1,10 @@ +import mongoose from "mongoose"; + +const schema = mongoose.Schema({ + _id: { + type: String, + required: true + } +}); + +export default mongoose.model("StringId", schema); diff --git a/test/integration/patch-relationship/index.js b/test/integration/patch-relationship/index.js index a5f5e60c..513db9f5 100644 --- a/test/integration/patch-relationship/index.js +++ b/test/integration/patch-relationship/index.js @@ -15,7 +15,7 @@ describe("Patching a relationship", () => { }).then(done, done); }); - it("should support full replacement at a to-many relationship endpoint", (done) => { + it("should support full replacement at a to-many relationship endpoint", function(done) { const url = `/organizations/${orgId}/relationships/liaisons`; const setRelationship = (data, url) => { //eslint-disable-line no-shadow diff --git a/test/unit/db-adapters/Mongoose/MongooseAdapter.js b/test/unit/db-adapters/Mongoose/MongooseAdapter.js index 36f7af8d..b9e3823e 100644 --- a/test/unit/db-adapters/Mongoose/MongooseAdapter.js +++ b/test/unit/db-adapters/Mongoose/MongooseAdapter.js @@ -1,8 +1,13 @@ +import Q from "q"; import {expect} from "chai"; import APIError from "../../../../src/types/APIError"; import mongoose from "mongoose"; import MongooseAdapter from "../../../../src/db-adapters/Mongoose/MongooseAdapter"; +const School = mongoose.model("School"); +const NumericId = mongoose.model("NumericId"); +const StringId = mongoose.model("StringId"); + describe("Mongoose Adapter", () => { describe("its instances methods", () => { describe("getModel", () => { @@ -69,68 +74,199 @@ describe("Mongoose Adapter", () => { }); describe("getIdQueryType", () => { - it("should handle null input", () => { - const res = MongooseAdapter.getIdQueryType(); - expect(res[0]).to.equal("find"); - expect(res[1]).to.be.undefined; + it("should handle null input", function(done) { + MongooseAdapter.getIdQueryType(null, School).then(([ mode, idQuery ]) => { + expect(mode).to.equal("find"); + expect(idQuery).to.be.undefined; + done(); + }).catch(done); }); describe("string", () => { - it("should throw on invalid input", () => { - const fn = function() { MongooseAdapter.getIdQueryType("1"); }; - expect(fn).to.throw(APIError); + it("should reject on invalid ObjectId input", function(done) { + MongooseAdapter.getIdQueryType("1", School).then(res => { + expect(false).to.be.ok; + }, err => { + expect(err).to.be.instanceof(APIError); + done(); + }).catch(done); + }); + + it("should reject on invalid numeric ID input", function(done) { + MongooseAdapter.getIdQueryType("123abc", NumericId).then(res => { + expect(false).to.be.ok; + }, err => { + expect(err).to.be.instanceof(APIError); + done(); + }).catch(done); + }); + + it("should produce query on valid ObjectId input", function(done) { + MongooseAdapter.getIdQueryType("552c5e1c604d41e5836bb174", School).then(([ mode, idQuery ]) => { + expect(mode).to.equal("findOne"); + expect(idQuery._id).to.equal("552c5e1c604d41e5836bb174"); + done(); + }).catch(done); }); - it("should produce query on valid input", () => { - const res = MongooseAdapter.getIdQueryType("552c5e1c604d41e5836bb174"); - expect(res[0]).to.equal("findOne"); - expect(res[1]._id).to.equal("552c5e1c604d41e5836bb174"); + it("should produce query on valid numeric ID input", function(done) { + MongooseAdapter.getIdQueryType("0", NumericId).then(([ mode, idQuery ]) => { + expect(mode).to.equal("findOne"); + expect(idQuery._id).to.equal("0"); + done(); + }).catch(done); + }); + + it("should produce query on valid string ID input", function(done) { + MongooseAdapter.getIdQueryType("null", StringId).then(([ mode, idQuery ]) => { + expect(mode).to.equal("findOne"); + expect(idQuery._id).to.equal("null"); + done(); + }).catch(done); }); }); describe("array", () => { - it("should throw if any ids are invalid", () => { - const fn = function() { MongooseAdapter.getIdQueryType(["1", "552c5e1c604d41e5836bb174"]); }; - expect(fn).to.throw(APIError); + it("should throw if any ObjectIds are invalid", function(done) { + MongooseAdapter.getIdQueryType(["1", "552c5e1c604d41e5836bb174"], School).then(res => { + expect(false).to.be.ok; + }, err => { + expect(err).to.be.instanceof(APIError); + done(); + }).catch(done); + }); + + it("should throw if any numeric ids are invalid", function(done) { + MongooseAdapter.getIdQueryType(["abc", "123"], NumericId).then(res => { + expect(false).to.be.ok; + }, err => { + expect(err).to.be.instanceof(APIError); + done(); + }).catch(done); }); - it("should produce query on valid input", () => { - const res = MongooseAdapter.getIdQueryType(["552c5e1c604d41e5836bb174", "552c5e1c604d41e5836bb175"]); - expect(res[0]).to.equal("find"); - expect(res[1]._id.$in).to.be.an.Array; - expect(res[1]._id.$in[0]).to.equal("552c5e1c604d41e5836bb174"); - expect(res[1]._id.$in[1]).to.equal("552c5e1c604d41e5836bb175"); + it("should produce query on valid ObjectId input", function(done) { + MongooseAdapter.getIdQueryType(["552c5e1c604d41e5836bb174", "552c5e1c604d41e5836bb175"], School).then(([ mode, idQuery ]) => { + expect(mode).to.equal("find"); + expect(idQuery._id.$in).to.be.an.Array; + expect(idQuery._id.$in[0]).to.equal("552c5e1c604d41e5836bb174"); + expect(idQuery._id.$in[1]).to.equal("552c5e1c604d41e5836bb175"); + done(); + }).catch(done); + }); + + it("should produce query on valid numeric ID input", function(done) { + MongooseAdapter.getIdQueryType(["0", "1"], NumericId).then(([ mode, idQuery ]) => { + expect(mode).to.equal("find"); + expect(idQuery._id.$in).to.be.an.Array; + expect(idQuery._id.$in[0]).to.equal("0"); + expect(idQuery._id.$in[1]).to.equal("1"); + done(); + }).catch(done); + }); + + it("should produce query on valid string ID input", function(done) { + MongooseAdapter.getIdQueryType(["a", "null"], StringId).then(([ mode, idQuery ]) => { + expect(mode).to.equal("find"); + expect(idQuery._id.$in).to.be.an.Array; + expect(idQuery._id.$in[0]).to.equal("a"); + expect(idQuery._id.$in[1]).to.equal("null"); + done(); + }).catch(done); + }); + + it("should produce an empty query when passed an empty array of ids", function(done) { + MongooseAdapter.getIdQueryType([], School).then(([ mode, idQuery ]) => { + expect(mode).to.equal("find"); + expect(idQuery._id.$in).to.be.an.Array + expect(idQuery._id.$in).to.have.lengthOf(0); + done(); + }).catch(done); }); }); }); - describe("idIsValid", () => { - it("should reject all == null input", () => { - expect(MongooseAdapter.idIsValid()).to.not.be.ok; - expect(MongooseAdapter.idIsValid(null)).to.not.be.ok; - expect(MongooseAdapter.idIsValid(undefined)).to.not.be.ok; + describe("validateId", () => { + it("should refuse all == null input", function(done) { + const tests = [ + MongooseAdapter.validateId(null, School), + MongooseAdapter.validateId(null, NumericId), + MongooseAdapter.validateId(null, StringId), + MongooseAdapter.validateId(undefined, School), + MongooseAdapter.validateId(undefined, NumericId), + MongooseAdapter.validateId(undefined, StringId) + ]; + + Q.allSettled(tests).then((res) => { + res.forEach(result => expect(result.state).to.equal("rejected")); + done(); + }).catch(done); }); - it("should reject bad input type", () => { - expect(MongooseAdapter.idIsValid(true)).to.not.be.ok; + it("should refuse a bad input type", function(done) { + const tests = [ + MongooseAdapter.validateId(true, School), + MongooseAdapter.validateId(false, School), + MongooseAdapter.validateId("not hex", School), + MongooseAdapter.validateId([], School), + MongooseAdapter.validateId("1234567890abcdef", School), + MongooseAdapter.validateId(1234567890, School), + MongooseAdapter.validateId("NaN", NumericId), + MongooseAdapter.validateId("one", NumericId), + MongooseAdapter.validateId([], NumericId), + MongooseAdapter.validateId(true, NumericId), + MongooseAdapter.validateId(false, NumericId) + // StringId should except anything != null + ]; + + Q.allSettled(tests).then((res) => { + res.forEach(result => { + expect(result.state).to.equal("rejected"); + expect(result.reason.name).to.equal("CastError"); + expect(result.reason.path).to.equal("_id"); + }); + + done(); + }).catch(done); }); - it("should reject empty string", () => { - expect(MongooseAdapter.idIsValid("")).to.not.be.ok; + it("should refuse an empty string", function(done) { + const tests = [ + MongooseAdapter.validateId("", School), + MongooseAdapter.validateId("", NumericId), + MongooseAdapter.validateId("", StringId) + ]; + + Q.allSettled(tests).then((res) => { + res.forEach(result => { + expect(result.state).to.equal("rejected"); + expect(result.reason.name).to.match(/ValidatorError|CastError/); + expect(result.reason.path).to.equal("_id"); + }); + done(); + }).catch(done); }); // the string coming into the MongooseAdapter needs to be the 24-character, // hex encoded version of the ObjectId, not an arbitrary 12 byte string. - it("should reject 12-character strings", () => { - expect(MongooseAdapter.idIsValid("aaabbbccc111")).to.not.be.ok; - }); - - it("should reject numbers", () => { - expect(MongooseAdapter.idIsValid(1)).to.not.be.ok; + it("should reject 12-character strings", function(done) { + MongooseAdapter.validateId("aaabbbccc111", School) + .then(() => expect(false).to.be.ok) + .catch(() => done()); }); - it("should accpet valid hex string", () => { - expect(MongooseAdapter.idIsValid("552c5e1c604d41e5836bb175")).to.be.ok; + it("should accpet valid IDs", function(done) { + const tests = [ + MongooseAdapter.validateId("552c5e1c604d41e5836bb175", School), + MongooseAdapter.validateId("123", NumericId), + MongooseAdapter.validateId(123, NumericId), + MongooseAdapter.validateId("0", NumericId), + MongooseAdapter.validateId(0, NumericId), + MongooseAdapter.validateId("0", StringId), + MongooseAdapter.validateId("null", StringId) + ]; + + Q.all(tests).then((res) => done(), done); }); });