diff --git a/CHANGELOG.md b/CHANGELOG.md index eef1b6d8..9ebda8c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -664,4 +664,8 @@ All notable changes to this project will be documented in this file. Breaking ch ## [3.9.0] ### Added -- Added a new `returnOnConditionCheckFailure` execution option for write operations. Instead of throwing when a condition expression fails, the call resolves with `{ rejected: true }`. Pass `true` to get just the rejection flag, or `"all_old"` to also receive the existing item under `data`. The `"all_old"` mode requires AWS SDK v3, or AWS SDK v2 >= `2.1408.0`. \ No newline at end of file +- Added a new `returnOnConditionCheckFailure` execution option for write operations. Instead of throwing when a condition expression fails, the call resolves with `{ rejected: true }`. Pass `true` to get just the rejection flag, or `"all_old"` to also receive the existing item under `data`. The `"all_old"` mode requires AWS SDK v3, or AWS SDK v2 >= `2.1408.0`. + +## [3.9.1] +### Fixed +- [Issue #578](https://github.com/tywalch/electrodb/issues/578); Watchers no longer do unnecessary work for methods they don't define: a watcher only runs on `get` if it has a getter, and on `set` if it has a setter. This also fixes a setter-only `watch` attribute (e.g. an `updatedAt` timestamp) being returned as `undefined` on reads when absent from the item. \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 9eaff02d..e4fa0761 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "electrodb", - "version": "3.9.0", + "version": "3.9.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "electrodb", - "version": "3.9.0", + "version": "3.9.1", "license": "ISC", "dependencies": { "@aws-sdk/lib-dynamodb": "^3.654.0", diff --git a/package.json b/package.json index 0bb0b450..cae239b6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "electrodb", - "version": "3.9.0", + "version": "3.9.1", "description": "A library to more easily create and interact with multiple entities and heretical relationships in dynamodb", "main": "index.js", "scripts": { diff --git a/src/schema.js b/src/schema.js index 14f24754..bae55bdb 100644 --- a/src/schema.js +++ b/src/schema.js @@ -147,9 +147,17 @@ class Attribute { this.parent = { parentType: this.type, parentPath: this.path }; this.get = this._makeGet(definition.get); this.set = this._makeSet(definition.set); + this._hasMutationHandler = { + [AttributeMutationMethods.get]: typeof definition.get === "function", + [AttributeMutationMethods.set]: typeof definition.set === "function", + }; this.getClient = definition.getClient; } + hasMutationHandler(method) { + return !!this._hasMutationHandler[method]; + } + static buildChildAttributes(type, definition, parent) { let items; let properties; @@ -1601,28 +1609,38 @@ class Schema { _validateProperties() {} _formatWatchTranslations(attributes) { - let watchersToAttributes = {}; - let attributesToWatchers = {}; - let watchAllAttributes = {}; - let hasWatchers = false; - for (let name of Object.keys(attributes)) { - if (attributes[name].isWatcher()) { - hasWatchers = true; - watchersToAttributes[name] = attributes[name].watching; - } else if (attributes[name].watchAll) { - hasWatchers = true; - watchAllAttributes[name] = name; - } else { - attributesToWatchers[name] = attributesToWatchers[name] || {}; - attributesToWatchers[name] = attributes[name].watchedBy; + const translations = {}; + for (let method of Object.values(AttributeMutationMethods)) { + let watchersToAttributes = {}; + let attributesToWatchers = {}; + let watchAllAttributes = {}; + let hasWatchers = false; + for (let name of Object.keys(attributes)) { + const attribute = attributes[name]; + const hasMutationHandler = attribute.hasMutationHandler(method); + if (attribute.isWatcher() && hasMutationHandler) { + hasWatchers = true; + watchersToAttributes[name] = attribute.watching; + } else if (attribute.watchAll && hasMutationHandler) { + hasWatchers = true; + watchAllAttributes[name] = name; + } else { + attributesToWatchers[name] = {}; + for (let watcher of Object.keys(attribute.watchedBy)) { + if (attributes[watcher] && attributes[watcher].hasMutationHandler(method)) { + attributesToWatchers[name][watcher] = watcher; + } + } + } } + translations[method] = { + hasWatchers, + watchAllAttributes, + watchersToAttributes, + attributesToWatchers, + }; } - return { - hasWatchers, - watchAllAttributes, - watchersToAttributes, - attributesToWatchers, - }; + return translations; } getAttribute(path) { @@ -1667,21 +1685,21 @@ class Schema { _fulfillAttributeMutationMethod(method, payload) { let watchersToTrigger = {}; + const translation = this.translationForWatching[method]; // include: payload | We want to hit the getters/setters for any attributes coming in to be changed // avoid: watchersToAttributes | We want to avoid anything that is a watcher, even if it was included let avoid = { - ...this.translationForWatching.watchersToAttributes, - ...this.translationForWatching.watchAllAttributes, + ...translation.watchersToAttributes, + ...translation.watchAllAttributes, }; let data = this._applyAttributeMutation(method, payload, avoid, payload); // `data` here will include all the original payload values, but with the mutations applied to on non-watchers - if (!this.translationForWatching.hasWatchers) { + if (!translation.hasWatchers) { // exit early, why not return data; } for (let attribute of Object.keys(data)) { - let watchers = - this.translationForWatching.attributesToWatchers[attribute]; + let watchers = translation.attributesToWatchers[attribute]; // Any of the attributes on data have a watcher? if (watchers !== undefined) { watchersToTrigger = { ...watchersToTrigger, ...watchers }; @@ -1693,12 +1711,12 @@ class Schema { let include = { ...data, ...watchersToTrigger, - ...this.translationForWatching.watchAllAttributes, + ...translation.watchAllAttributes, }; return this._applyAttributeMutation( method, include, - this.translationForWatching.attributesToWatchers, + translation.attributesToWatchers, data, ); } diff --git a/test/offline.entity.spec.js b/test/offline.entity.spec.js index bbb2239a..91dab129 100644 --- a/test/offline.entity.spec.js +++ b/test/offline.entity.spec.js @@ -9011,5 +9011,166 @@ describe("Entity", () => { }, { ignoreOwnership: false }); expect(result.data).to.deep.equal({ id: "123", name: "test", description: "desc" }); }); + + describe("with watch attributes", () => { + const makeWatcherEntity = (updatedAt) => + new Entity( + { + model: { + entity: "watcherItem", + service: "Playground", + version: "1", + }, + attributes: { + id: { type: "string", required: true }, + name: { type: "string", required: true }, + updatedAt, + }, + indexes: { + byId: { + pk: { field: "pk", composite: ["id"] }, + sk: { field: "sk", composite: ["id"] }, + }, + }, + }, + { table: "electro" }, + ); + + const pk = "$playground#id_123"; + const sk = "$watcheritem_1#id_123"; + + it("should not add a watch-all attribute that only defines a setter when it is absent from the item", () => { + const entity = makeWatcherEntity({ + type: "number", + watch: "*", + set: () => 1234567890, + readOnly: true, + }); + + const result = entity.parse({ + Item: { pk, sk, id: "123", name: "test" }, + }); + + expect(result.data).to.deep.equal({ id: "123", name: "test" }); + expect(result.data).to.not.have.property("updatedAt"); + }); + + it("should still return a setter-only watch-all attribute when it is present on the item", () => { + const entity = makeWatcherEntity({ + type: "number", + watch: "*", + set: () => 1234567890, + }); + + const result = entity.parse({ + Item: { pk, sk, id: "123", name: "test", updatedAt: 999 }, + }); + + expect(result.data).to.deep.equal({ + id: "123", + name: "test", + updatedAt: 999, + }); + }); + + it("should still generate a setter-only watch-all attribute on the write path", () => { + const entity = makeWatcherEntity({ + type: "number", + watch: "*", + set: () => 1234567890, + readOnly: true, + }); + + const params = entity.create({ id: "123", name: "test" }).params(); + + expect(params.Item.updatedAt).to.equal(1234567890); + }); + + it("should still compute a getter-only watch-all attribute on read when it is absent from the item", () => { + const entity = makeWatcherEntity({ + type: "string", + watch: "*", + get: (_value, { id }) => `computed-${id}`, + }); + + const result = entity.parse({ + Item: { pk, sk, id: "123", name: "test" }, + }); + + expect(result.data).to.deep.equal({ + id: "123", + name: "test", + updatedAt: "computed-123", + }); + }); + + it("should run the getter of a watch-all attribute that defines both a getter and a setter on read", () => { + const entity = makeWatcherEntity({ + type: "string", + watch: "*", + get: (_value, { id }) => `computed-${id}`, + set: () => "set-value", + }); + + const result = entity.parse({ + Item: { pk, sk, id: "123", name: "test" }, + }); + + expect(result.data).to.deep.equal({ + id: "123", + name: "test", + updatedAt: "computed-123", + }); + }); + + it("should still trigger a getter on a named (non-watch-all) watcher on read", () => { + const entity = makeWatcherEntity({ + type: "string", + watch: ["name"], + get: (_value, { name }) => `named-${name}`, + }); + + const result = entity.parse({ + Item: { pk, sk, id: "123", name: "test" }, + }); + + expect(result.data).to.deep.equal({ + id: "123", + name: "test", + updatedAt: "named-test", + }); + }); + + it("should not add a named (non-watch-all) setter-only watcher when it is absent from the item", () => { + const entity = makeWatcherEntity({ + type: "string", + watch: ["name"], + set: () => "set-value", + }); + + const result = entity.parse({ + Item: { pk, sk, id: "123", name: "test" }, + }); + + expect(result.data).to.deep.equal({ id: "123", name: "test" }); + expect(result.data).to.not.have.property("updatedAt"); + }); + + it("should exclude a setter-only watch-all attribute when specific attributes are requested", () => { + const entity = makeWatcherEntity({ + type: "number", + watch: "*", + set: () => 1234567890, + }); + + const result = entity.parse( + { Item: { pk, sk, id: "123", name: "test", updatedAt: 999 } }, + { attributes: ["id"] }, + ); + + expect(result.data).to.deep.equal({ id: "123" }); + expect(result.data).to.not.have.property("updatedAt"); + }); + }); }); });