Skip to content

Commit

Permalink
Allow middleware hooks to return objects that contained field names t…
Browse files Browse the repository at this point in the history
…hat contain dotted strings (#1130)

* Cleanup: ensure db.folks update hook is removed after unit test completes

* Do not extract nested paths from the object keys contained in middleware hook return values

Fixes #966

* Additional test for #1130

* Nit: trailing space

* Post-merge fixup

* Add explicit check for keys as literal property names on objects before intepreting them as paths

* Alternative approach: push logic down into setByKeyPath function

* Revert "Alternative approach: push logic down into setByKeyPath function"

This reverts commit d610245.

Co-authored-by: dfahlander <david.fahlander@gmail.com>
  • Loading branch information
jayaddison and dfahlander committed Sep 29, 2020
1 parent b88f66f commit b2335f9
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/hooks/hooks-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
DBCoreKeyRange,
} from "../public/types/dbcore";
import { nop } from '../functions/chaining-functions';
import { getObjectDiff, setByKeyPath } from '../functions/utils';
import { getObjectDiff, hasOwn, setByKeyPath } from '../functions/utils';
import { PSD } from '../helpers/promise';
//import { LockableTableMiddleware } from '../dbcore/lockable-table-middleware';
import { getEffectiveKeys, getExistingValues } from '../dbcore/get-effective-keys';
Expand Down Expand Up @@ -87,7 +87,13 @@ export const hooksMiddleware: Middleware<DBCore> = {
if (additionalChanges) {
const requestedValue = req.values[i];
Object.keys(additionalChanges).forEach(keyPath => {
setByKeyPath(requestedValue, keyPath, additionalChanges[keyPath]);
if (hasOwn(requestedValue, keyPath)) {
// keyPath is already present as a literal property of the object
requestedValue[keyPath] = additionalChanges[keyPath];
} else {
// keyPath represents a new or existing path into the object
setByKeyPath(requestedValue, keyPath, additionalChanges[keyPath]);
}
});
}
}
Expand Down
45 changes: 45 additions & 0 deletions test/tests-table.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,51 @@ promisedTest("Issue #841 - put() ignores date changes", async ()=> {
db.folks.hook("updating").unsubscribe(updateAssertions);
});

promisedTest("Issue #966 - put() with dotted field in update hook", async () => {
const updateAssertions = (mods) => {
equal(mods["nested.field"], "value", "mods.nested.field should contain 'value'");
equal(mods.nested, undefined, "mods.nested field should be empty");
return {...mods};
};
db.folks.hook("updating", updateAssertions);

const id = await db.folks.add({first: "first", last: "last"});
await db.folks.put({first: "first", last: "last", "nested.field": "value"}, id);

let obj = await db.folks.get(id);
equal(obj["nested.field"], "value", "obj.nested.field should have been successfully updated to 'value'");
equal(obj.nested, undefined, "obj.nested field should have remained undefined");

db.folks.hook("updating").unsubscribe(updateAssertions);
});

promisedTest("Verify #1130 doesn't break contract of hook('updating')", async ()=>{
const updateHook = (mods) => {
return {"address.postalCode": 111};
};
try {
const id = await db.folks.add({
first: "Foo",
last: "Bar",
address: {
city: "Stockholm",
street: "Folkungagatan"
}
});
db.folks.hook("updating", updateHook);
await db.folks.update(id, {
"address.streetNo": 23
});
let foo = await db.folks.get(id);
equal(foo.address.city, "Stockholm", "Correct city Stockholm");
equal(foo.address.street, "Folkungagatan", "Correct street Folkungagatan");
equal(foo.address.streetNo, 23, "Correct streetNo: 23");
equal(foo.address.postalCode, 111, "Hooks should have added postal code");
} finally {
db.folks.hook("updating").unsubscribe(updateHook);
}
});

asyncTest("get", 4, function () {
db.table("users").get(idOfFirstUser).then(function(obj) {
equal(obj.first, "David", "Got the first object");
Expand Down

0 comments on commit b2335f9

Please sign in to comment.