Add few validation checks, increase test coverage

tnajdek · Aug 28, 2023 · d75fb57 · d75fb57
1 parent 93dee36
commit d75fb57
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 10 deletions.
diff --git a/jest.config.mjs b/jest.config.mjs
@@ -0,0 +1,12 @@
+export default {
+    maxWorkers: "50%",
+    restoreMocks: true,
+    coverageProvider: "v8",
+    coverageDirectory: "coverage",
+    coverageReporters: ['lcov', ['text', { skipFull: false }]],
+    coveragePathIgnorePatterns: [
+        "/node_modules/",
+        "test/fixtures/",
+        "test/mocks/",
+    ],
+}
diff --git a/src/api.js b/src/api.js
@@ -140,9 +140,12 @@ const api = function() {
 	 * @chainable
 	 */
 	const itemTypeFields = function(itemType) {
+		if (!itemType) {
+			throw new Error('itemTypeFields() requires an itemType argument');
+		}
 		return efr.bind(this)({
 			itemTypeFields: null
-		}, itemType ? { itemType } : {});
+		}, { itemType });
 	};
 
 	/**
@@ -155,9 +158,12 @@ const api = function() {
 	 * @chainable
 	 */
 	const itemTypeCreatorTypes = function(itemType) {
+		if (!itemType) {
+			throw new Error('itemTypeCreatorTypes() requires an itemType argument');
+		}
 		return efr.bind(this)({
 			itemTypeCreatorTypes: null
-		}, itemType ? { itemType } : {});
+		}, { itemType });
 	};
 
 	/**
@@ -171,6 +177,9 @@ const api = function() {
 	 * @chainable
 	 */
 	const template = function(itemType, subType) {
+		if (!itemType) {
+			throw new Error('template() requires an itemType argument');
+		}
 		const subTypeOpts = {};
 
 		if (subType && itemType === 'annotation') {
@@ -181,7 +190,7 @@ const api = function() {
 
 		return efr.bind(this)({
 			template: null
-		}, itemType ? { itemType, ...subTypeOpts } : {});
+		}, { itemType, ...subTypeOpts });
 	};
 
 	/**
@@ -354,10 +363,11 @@ const api = function() {
 	 * @return {Object} Partially configured api functions
 	 * @chainable
 	 */
-	const version = function(version = null) {
-		return ef.bind(this)({
-			version: version
-		})
+	const version = function(version) {
+		if(typeof(version) !== 'number' || isNaN(version)) {
+			throw new Error('version() requires a number argument');
+		}
+		return ef.bind(this)({ version });
 	};
 
 	/**
@@ -641,7 +651,6 @@ const api = function() {
 		var relevantSearchKey;
 		var requestConfig, keysToDelete;
 		switch(method) {
-			default:
 			case 'get':
 				requestConfig = { ...config, ...opts, method };
 				if('version' in requestConfig) {

diff --git a/test/api.spec.js b/test/api.spec.js
@@ -92,6 +92,8 @@ describe('Zotero Api Client', () => {
 
 			api(KEY).library(LIBRARY_KEY).items().version(42).post([]);
 			assert.equal(lrc.ifUnmodifiedSinceVersion, 42);
+			api(KEY).library(LIBRARY_KEY).items().version(0).post([]);
+			assert.equal(lrc.ifUnmodifiedSinceVersion, 0);
 
 			api(KEY).library(LIBRARY_KEY).items('AABBCCDD').version(42).put({});
 			assert.equal(lrc.ifUnmodifiedSinceVersion, 42);
@@ -598,7 +600,7 @@ describe('Zotero Api Client', () => {
 	});
 
 	describe('Handles pretend calls', () => {
-			it('handles api.library.items(I).pretend()', () => {
+		it('handles api.library.items(I).pretend()', () => {
 			api(KEY).library(LIBRARY_KEY).items(ITEM_KEY).pretend();
 			assert.equal(lrc.method, 'get');
 			assert.equal(lrc.resource.library, LIBRARY_KEY);
@@ -661,6 +663,27 @@ describe('Zotero Api Client', () => {
 			let configuredApi = api(KEY).library(LIBRARY_KEY).settings(SETTING_KEY);
 			assert.throws(configuredApi.delete.bind(configuredApi, []), 'Arguments to delete() not supported when deleting settings');
 		});
+
+		it('throws when api.itemTypeFields() is called without an itemType', () => {
+			let configuredApi = api();
+			assert.throws(configuredApi.itemTypeFields.bind(configuredApi), 'itemTypeFields() requires an itemType argument');
+		});
+
+		it('throws when api.itemTypeCreatorTypes() is called without an itemType', () => {
+			let configuredApi = api();
+			assert.throws(configuredApi.itemTypeCreatorTypes.bind(configuredApi), 'itemTypeCreatorTypes() requires an itemType argument');
+		});
+
+		it('throws when api.template() is called without an itemType', () => {
+			let configuredApi = api();
+			assert.throws(configuredApi.template.bind(configuredApi), 'template() requires an itemType argument');
+		});
+
+		it('throws when api.version() is called with invalid argument', () => {
+			let configuredApi = api();
+			assert.throws(configuredApi.version.bind(configuredApi), 'version() requires a number argument');
+		});
+
 	});
 
 	describe('Handles extensions', () => {

diff --git a/test/request.spec.js b/test/request.spec.js
@@ -135,7 +135,10 @@ describe('ZoteroJS request', () => {
 		it('should get a single item', () => {
 			fetchMock.mock(
 				'begin:https://api.zotero.org/users/475425/items/X42A7DEE',
-				singleGetResponseFixture
+				{
+					headers: { 'Last-Modified-Version': 1 },
+					body: singleGetResponseFixture
+				}
 			);
 
 			return request({
@@ -148,6 +151,7 @@ describe('ZoteroJS request', () => {
 				assert.strictEqual(response.getResponseType(), 'SingleReadResponse');
 				assert.strictEqual(response.getLinks().self.href, 'https://api.zotero.org/users/475425/items/X42A7DEE');
 				assert.strictEqual(response.getMeta().parsedDate, '1993');
+				assert.strictEqual(response.getVersion(), 1);
 				assert.strictEqual(Object.keys(response.getLinks()).length, 2);
 				assert.strictEqual(response.getData().key, 'X42A7DEE');
 			});
@@ -1867,6 +1871,7 @@ describe('ZoteroJS request', () => {
 				assert.instanceOf(response, FileUrlResponse);
 				assert.strictEqual(response.getResponseType(), 'FileUrlResponse');
 				assert.strictEqual(response.getData(), 'https://files.zotero.org/some-file');
+				assert.isNull(response.getVersion());
 			});
 		});
 	})