Merge 3b22bf0 into cb34ae5

CHANGELOG.md

@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+### Added
+- Struct validation for path id
+
 ## [4.0.1] - 2021-12-13
 ### Added
 - Typings build from JSDoc

README.md

@@ -68,8 +68,42 @@ module.exports = MyApiGet;
 
 All methods are optional
 
-# Get APIs with parents
+## Get APIs with parents
 
 If you have for example, a get API for a sub-entity of one specific record, the parent will be automatically be added as a filter.
 
 For example, the following endpoint: `/api/parent-entity/1/sub-entity/2`, will be a get of the sub-entity, and `parentEntity: '1'` will be set as a filter.
+
+## ✔️ Path ID validation
+Validation for path ID is available. By implementing `validateId()` method, you can make sure ID format is correct. If received ID is invalid, the API will return a ***400*** error by default and no request will be made to database.
+
+### Default behavior
+1. This validation will only be performed if database driver has `idStruct` getter implemented.
+2. If received ID is invalid, the API will return a ***400*** error. 
+3. Validation applies only to main record ID (eg: For `/api/parent-entity/1/sub-entity/2` the ID validation will be applied only to `2`).
+
+### Customization
+
+❗In case you want to set a different behaviour or validation, you can do it by overriding the `validateId` method.
+
+**eg: Adding validation for parent `ids`**
+```javascript
+	validateId() {
+
+		Object.values(this.parents).forEach(parentId => {
+			struct('string&!empty')(parentId)
+		});
+
+		struct('objectId')(this.recordId)
+	}
+```
+
+#### How to disable validation
+In case database driver has an `idsStruct` defined and you want to disable validation, you can do it by overriding the `validateId` method.
+
+**eg:**
+```javascript
+	validateId() {
+		// Do nothing
+	}
+```

lib/api-get.js

@@ -21,11 +21,34 @@ module.exports = class ApiGet extends API {
 	async validate() {
 
 		this._parseEndpoint();
+
 		this._validateModel();
 
+		await this.validateId();
+
 		return this.postValidate();
 	}
 
+	/**
+	 * Validates if path ID type is valid.
+	 * Validation will only be performed if database driver has `idStruct` getter implemented.
+	 * @returns {void}
+	 */
+	async validateId() {
+
+		let idStruct;
+
+		try {
+			idStruct = await this.model.getIdStruct();
+		} catch(error) {
+			this.setCode(500);
+			throw error;
+		}
+
+		if(idStruct)
+			idStruct(this.recordId);
+	}
+
 	/**
 	 *  It is to perform extra validations
 	 * @returns {*}

package.json

@@ -28,7 +28,8 @@
     "mock-require": "^3.0.3",
     "nyc": "^15.1.0",
     "sinon": "^12.0.1",
-    "typescript": "^4.5.2"
+    "typescript": "^4.5.2",
+    "@janiscommerce/superstruct": "^1.2.0"
   },
   "files": [
     "lib/",

tests/api-get.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const { struct } = require('@janiscommerce/superstruct');
 const assert = require('assert');
 const path = require('path');
 
@@ -15,6 +16,9 @@ describe('ApiGet', () => {
 	});
 
 	class Model {
+		async getIdStruct() {
+			return undefined;
+		}
 	}
 
 	const modelPath = path.join(process.cwd(), process.env.MS_PATH || '', 'models', 'some-entity');
@@ -58,8 +62,74 @@ describe('ApiGet', () => {
 			await assert.rejects(() => apiGet.validate(), ApiGetError);
 		});
 
-		it('Should validate if a valid model and ID is passed', async () => {
+		it('Should reject if model fails on getting idStruct', async () => {
+
+			sinon.restore();
+			class Model2 {
+				async getIdStruct() {
+					return struct('objectId');
+				}
+			}
+
+			mockRequire(modelPath, Model2);
+
+			sinon.stub(Model2.prototype, 'getIdStruct')
+				.rejects(new Error('Internal Error'));
+
+			const apiGet = new ApiGet();
+			apiGet.endpoint = '/some-entity/10';
+			apiGet.pathParameters = ['10'];
+
+			await assert.rejects(apiGet.validate(), { message: 'Internal Error' });
+		});
+
+		it('Should reject if invalid ID is passed', async () => {
+
+			sinon.restore();
+			class Model2 {
+				async getIdStruct() {
+					return struct('objectId');
+				}
+			}
+
+			mockRequire(modelPath, Model2);
+
+			const apiGet = new ApiGet();
+			apiGet.endpoint = '/some-entity/10';
+			apiGet.pathParameters = ['10'];
+
+			await assert.rejects(apiGet.validate(), { message: 'Expected a value of type `objectId` but received `"10"`.' });
+		});
+
+		it('Should use validation defined in extended API', async () => {
+
+			sinon.restore();
+			class Model2 {
+				async getIdStruct() {
+					return struct('objectId');
+				}
+			}
+
+			class TestApi extends ApiGet {
+				async getIdStruct() {
+					return struct('string');
+				}
+			}
+
+			mockRequire(modelPath, Model2);
+			mockRequire(modelPath, TestApi);
+
+			const apiGet = new TestApi();
+			apiGet.endpoint = '/some-entity/10';
+			apiGet.pathParameters = ['10'];
+
+			const validation = await apiGet.validate();
+
+			assert.strictEqual(validation, true);
+		});
 
+		it('Should validate if a valid model and ID is passed', async () => {
+			mockRequire(modelPath, Model);
 			const apiGet = new ApiGet();
 			apiGet.endpoint = '/some-entity/10';
 			apiGet.pathParameters = ['10'];
@@ -68,6 +138,35 @@ describe('ApiGet', () => {
 
 			assert.strictEqual(validation, true);
 		});
+
+		it('Should not reject if model Database driver has no idStruct defined', async () => {
+
+			mockRequire(modelPath, Model);
+
+			const apiGet = new ApiGet();
+			apiGet.endpoint = '/some-entity/10';
+			apiGet.pathParameters = ['10'];
+
+			assert.deepStrictEqual(await apiGet.validate(), true);
+		});
+
+		it('Should not reject if invalid parent ID is passed', async () => {
+
+			sinon.restore();
+			class Model2 {
+				async getIdStruct() {
+					return struct('objectId');
+				}
+			}
+
+			mockRequire(modelPath, Model2);
+
+			const apiGet = new ApiGet();
+			apiGet.endpoint = '/some-parent/10/some-entity/6282c2484f64bffff55bcd7c';
+			apiGet.pathParameters = ['10', '6282c2484f64bffff55bcd7c'];
+
+			assert.deepStrictEqual(await apiGet.validate(), true);
+		});
 	});
 
 	describe('Process', () => {
@@ -78,6 +177,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -111,6 +214,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -155,6 +262,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -190,6 +301,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -233,6 +348,11 @@ describe('ApiGet', () => {
 		it('Should throw an internal error if get fails', async () => {
 
 			mockRequire(modelPath, class MyModel {
+
+				async getIdStruct() {
+					return undefined;
+				}
+
 				async get() {
 					throw new Error('Some internal error');
 				}
@@ -255,6 +375,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -297,6 +421,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [dbRecord];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -351,6 +479,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [dbRecord];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -385,6 +517,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -451,6 +587,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [dbRecord];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -507,6 +647,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [dbRecord];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -547,6 +691,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			mockRequire(modelPath, MyModel);
@@ -574,6 +722,10 @@ describe('ApiGet', () => {
 				async get() {
 					return [];
 				}
+
+				async getIdStruct() {
+					return undefined;
+				}
 			}
 
 			const pathOtherEntity = path.join(process.cwd(), process.env.MS_PATH || '', 'models', 'other-entity');
@@ -590,5 +742,4 @@ describe('ApiGet', () => {
 			mockRequire.stop(modelPath);
 		});
 	});
-
 });