From 694653c1f6ded86ddd7ba6641f07e59a6780e316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominique=20J=C3=A4ggi?= Date: Mon, 23 Dec 2024 11:17:08 +0100 Subject: [PATCH] chore: pr review --- .../src/v2/models/base/base.collection.js | 7 +++++-- .../src/v2/models/base/base.model.js | 18 ++++++++++++----- .../src/v2/models/base/entity.registry.js | 3 ++- .../src/v2/models/base/schema.builder.js | 20 +++++++++---------- .../src/v2/models/base/schema.js | 3 ++- .../unit/v2/models/base/base.model.test.js | 4 ++-- 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/packages/spacecat-shared-data-access/src/v2/models/base/base.collection.js b/packages/spacecat-shared-data-access/src/v2/models/base/base.collection.js index 99e666f5..8d7f8f53 100755 --- a/packages/spacecat-shared-data-access/src/v2/models/base/base.collection.js +++ b/packages/spacecat-shared-data-access/src/v2/models/base/base.collection.js @@ -296,6 +296,7 @@ class BaseCollection { * @param {QueryOptions} [options] - Additional options for the query. * Additional options for the query. * @return {Promise} + * @throws {DataAccessError} - Throws an error if the sort keys are not provided. */ async findByAll(sortKeys = {}, options = {}) { if (!isObject(sortKeys)) { @@ -315,7 +316,7 @@ class BaseCollection { * @param {string} id - The unique identifier of the entity to be found. * @returns {Promise} - A promise that resolves to an instance of * the model if found, otherwise null. - * @throws {Error} - Throws an error if the ID is not provided. + * @throws {ValidationError} - Throws an error if the ID is not provided. */ async findById(id) { guardId(this.idName, id, this.entityName); @@ -330,6 +331,7 @@ class BaseCollection { * @param {Object} keys - The index keys to use for the query. * @param {{index?: string, attributes?: string[]}} [options] - Additional options for the query. * @returns {Promise} - A promise that resolves to the model instance or null. + * @throws {DataAccessError} - Throws an error if retrieving the entity fails. * @async */ async findByIndexKeys(keys, options = {}) { @@ -343,7 +345,8 @@ class BaseCollection { * @async * @param {Object} item - The data for the entity to be created. * @returns {Promise} - A promise that resolves to the created model instance. - * @throws {Error} - Throws an error if the data is invalid or if the creation process fails. + * @throws {DataAccessError} - Throws an error if the data is invalid or if the + * creation process fails. */ async create(item) { if (!isNonEmptyObject(item)) { diff --git a/packages/spacecat-shared-data-access/src/v2/models/base/base.model.js b/packages/spacecat-shared-data-access/src/v2/models/base/base.model.js index af50a1ac..fa4f5904 100755 --- a/packages/spacecat-shared-data-access/src/v2/models/base/base.model.js +++ b/packages/spacecat-shared-data-access/src/v2/models/base/base.model.js @@ -223,7 +223,7 @@ class BaseModel { * @async * @returns {Promise} - A promise that resolves to the current instance of the entity * after it and its dependents have been removed. - * @throws {Error} - Throws an error if the schema does not allow removal + * @throws {DataAccessError} - Throws an error if the schema does not allow removal * or if the removal operation fails. */ async remove() { @@ -239,7 +239,7 @@ class BaseModel { * This method does not check if the schema allows removal in order to be able to remove * dependents even if the schema does not allow removal. * @return {Promise} - * @throws {Error} - Throws an error if the removal operation fails. + * @throws {DataAccessError} - Throws an error if the removal operation fails. * @protected */ async _remove() { @@ -258,7 +258,11 @@ class BaseModel { return this; } catch (error) { this.log.error('Failed to remove record', error); - throw error; + throw new DataAccessError( + `Failed to remove entity ${this.entityName} with ID ${this.getId()}`, + this, + error, + ); } } @@ -268,7 +272,7 @@ class BaseModel { * @async * @returns {Promise} - A promise that resolves to the current instance of the entity * after it has been saved. - * @throws {Error} - Throws an error if the save operation fails. + * @throws {DataAccessError} - Throws an error if the save operation fails. */ async save() { // todo: validate associations @@ -281,7 +285,11 @@ class BaseModel { return this; } catch (error) { this.log.error('Failed to save record', error); - throw error; + throw new DataAccessError( + `Failed to to save entity ${this.entityName} with ID ${this.getId()}`, + this, + error, + ); } } diff --git a/packages/spacecat-shared-data-access/src/v2/models/base/entity.registry.js b/packages/spacecat-shared-data-access/src/v2/models/base/entity.registry.js index 5cfdded7..a3a40eea 100755 --- a/packages/spacecat-shared-data-access/src/v2/models/base/entity.registry.js +++ b/packages/spacecat-shared-data-access/src/v2/models/base/entity.registry.js @@ -93,7 +93,8 @@ class EntityRegistry { * Gets a collection instance by its name. * @param {string} collectionName - The name of the collection to retrieve. * @returns {Object} - The requested collection instance. - * @throws {Error} - Throws an error if the collection with the specified name is not found. + * @throws {DataAccessError} - Throws an error if the collection with the + * specified name is not found. */ getCollection(collectionName) { const collection = this.collections.get(collectionName); diff --git a/packages/spacecat-shared-data-access/src/v2/models/base/schema.builder.js b/packages/spacecat-shared-data-access/src/v2/models/base/schema.builder.js index 6614186c..a2eee88f 100755 --- a/packages/spacecat-shared-data-access/src/v2/models/base/schema.builder.js +++ b/packages/spacecat-shared-data-access/src/v2/models/base/schema.builder.js @@ -88,9 +88,9 @@ class SchemaBuilder { * @param {BaseModel} modelClass - The model class for this entity. * @param {BaseCollection} collectionClass - The collection class for this entity. * @param {number} schemaVersion - A positive integer representing the schema's version. - * @throws {Error} If entityName is not a non-empty string. - * @throws {Error} If schemaVersion is not a positive integer. - * @throws {Error} If serviceName is not a non-empty string. + * @throws {SchemaBuilderError} If entityName is not a non-empty string. + * @throws {SchemaBuilderError} If schemaVersion is not a positive integer. + * @throws {SchemaBuilderError} If serviceName is not a non-empty string. */ constructor(modelClass, collectionClass, schemaVersion = 1) { if (!modelClass || !(modelClass.prototype instanceof BaseModel)) { @@ -174,7 +174,7 @@ class SchemaBuilder { * This should only be used in special cases. * * @param {Array} sortKeys - The attributes to form the sort key. - * @throws {Error} If sortKeys are not provided or are not a non-empty array. + * @throws {SchemaBuilderError} If sortKeys are not provided or are not a non-empty array. * @return {SchemaBuilder} */ withPrimarySortKeys(sortKeys) { @@ -194,7 +194,7 @@ class SchemaBuilder { * remove is called implicitly when the entity is removed * as part of parent entity remove (dependents). * @param {boolean} allow - Whether to allow removes. - * @throws {Error} If allow is not a boolean. + * @throws {SchemaBuilderError} If allow is not a boolean. * @return {SchemaBuilder} */ allowRemove(allow) { @@ -212,7 +212,7 @@ class SchemaBuilder { * not prevent updates at the database level, but rather * at the application level. * @param {boolean} allow - Whether to allow updates. - * @throws {Error} If allow is not a boolean. + * @throws {SchemaBuilderError} If allow is not a boolean. * @return {SchemaBuilder} */ allowUpdates(allow) { @@ -230,7 +230,7 @@ class SchemaBuilder { * @param {string} name - The attribute name. * @param {object} data - The attribute definition (type, required, validation, etc.). * @returns {SchemaBuilder} Returns this builder for method chaining. - * @throws {Error} If name is not non-empty or data is not an object. + * @throws {SchemaBuilderError} If name is not non-empty or data is not an object. */ addAttribute(name, data) { if (!hasText(name)) { @@ -253,7 +253,7 @@ class SchemaBuilder { * * @param {Array} sortKeys - The attributes to form the sort key. * @returns {SchemaBuilder} Returns this builder for method chaining. - * @throws {Error} If composite attribute names or template are not provided. + * @throws {SchemaBuilderError} If composite attribute names or template are not provided. */ addAllIndex(sortKeys) { if (!isNonEmptyArray(sortKeys)) { @@ -276,7 +276,7 @@ class SchemaBuilder { * (e.g., { composite: [attributeName] }). * @param {object} sortKey - The sort key definition. * @returns {SchemaBuilder} Returns this builder for method chaining. - * @throws {Error} If index name is reserved or pk/sk configs are invalid. + * @throws {SchemaBuilderError} If index name is reserved or pk/sk configs are invalid. */ addIndex(partitionKey, sortKey) { if (!isNonEmptyObject(partitionKey)) { @@ -304,7 +304,7 @@ class SchemaBuilder { * @param {boolean} [options.removeDependents=false] - Whether to remove dependent entities * on delete. Only applies to HAS_MANY and HAS_ONE references. * @returns {SchemaBuilder} Returns this builder for method chaining. - * @throws {Error} If type or entityName are invalid. + * @throws {SchemaBuilderError} If type or entityName are invalid. */ addReference(type, entityName, sortKeys = [], options = {}) { if (!Reference.isValidType(type)) { diff --git a/packages/spacecat-shared-data-access/src/v2/models/base/schema.js b/packages/spacecat-shared-data-access/src/v2/models/base/schema.js index af5a2a2d..63f941fc 100644 --- a/packages/spacecat-shared-data-access/src/v2/models/base/schema.js +++ b/packages/spacecat-shared-data-access/src/v2/models/base/schema.js @@ -188,7 +188,7 @@ class Schema { * index is found, we fall back to the "all" index, then the "primary". * * @param {Object} keys - The keys to search for. - * @return {*|string} - The index name. + * @return {string} - The index name. */ findIndexNameByKeys(keys) { const { ALL, PRIMARY } = this.getIndexTypes(); @@ -313,6 +313,7 @@ class Schema { * * @param {BaseModel|BaseCollection} entity - The entity for which to generate accessors. * @param {Object} [log] - The logger to use for logging information + * @throws {SchemaError} - Throws an error if the entity is not a BaseModel or BaseCollection. * @return {Object[]} */ toAccessorConfigs(entity, log = console) { diff --git a/packages/spacecat-shared-data-access/test/unit/v2/models/base/base.model.test.js b/packages/spacecat-shared-data-access/test/unit/v2/models/base/base.model.test.js index 79900c9b..8e3500d2 100755 --- a/packages/spacecat-shared-data-access/test/unit/v2/models/base/base.model.test.js +++ b/packages/spacecat-shared-data-access/test/unit/v2/models/base/base.model.test.js @@ -250,7 +250,7 @@ describe('BaseModel', () => { /* eslint-disable no-underscore-dangle */ const error = new Error('Remove failed'); mockElectroService.entities.opportunity.remove.returns({ go: () => Promise.reject(error) }); - await expect(baseModelInstance.remove()).to.be.rejectedWith('Remove failed'); + await expect(baseModelInstance.remove()).to.be.rejectedWith('Failed to remove entity opportunity with ID 12345'); expect(mockLogger.error.calledOnce).to.be.true; }); @@ -274,7 +274,7 @@ describe('BaseModel', () => { /* eslint-disable no-underscore-dangle */ const error = new Error('Save failed'); baseModelInstance.patcher.save = stub().returns(Promise.reject(error)); - await expect(baseModelInstance.save()).to.be.rejectedWith('Save failed'); + await expect(baseModelInstance.save()).to.be.rejectedWith('Failed to to save entity opportunity with ID 12345'); expect(mockLogger.error.calledOnce).to.be.true; }); });