appy-one / acebase-server

A fast, low memory, transactional, index & query enabled NoSQL database server for node.js that easily syncs with browser and node.js clients and servers
MIT License
32 stars 14 forks source link

update() doesn't accept nested keys #95

Open fahhem opened 1 year ago

fahhem commented 1 year ago

I'm not 100% sure this is the cause, since it's crashing the server rather than the client, but if I comment out this line then I don't get the error:

const updates = { 'sweepRepositories/interval': 1234};
ref.child('queues/cron').update(updates);
failed to update "queues/cron": Error: Invalid key "sweepRepositories/interval" for object to store at path "queues/cron". Keys cannot contain control characters or any of the following characters: \ / [ ]
    at /reviewable/node_modules/acebase/dist/cjs/storage/binary/index.js:3527:23
    at Array.forEach (<anonymous>)                                                        
    at _writeNode (/reviewable/node_modules/acebase/dist/cjs/storage/binary/index.js:3525:28)
    at _lockAndWriteNode (/reviewable/node_modules/acebase/dist/cjs/storage/binary/index.js:3461:34)
    at async Promise.all (index 0)
    at async _writeNode (/reviewable/node_modules/acebase/dist/cjs/storage/binary/index.js:3563:5)
    at async _mergeNode (/reviewable/node_modules/acebase/dist/cjs/storage/binary/index.js:3436:25)
    at async Object.write [as _customWriteFunction] (/reviewable/node_modules/acebase/dist/cjs/storage/binary/index.js:1945:28)
    at async AceBaseStorage._writeNodeWithTracking (/reviewable/node_modules/acebase/dist/cjs/storage/index.js:660:25)

It seems update(), set(), etc should all handle paths with /'s in them, right?

This is from trying to migrate an existing app, so Firebase's SDK supports this

appy-one commented 11 months ago

@fahhem AceBase doesn't support this yet - it is a cool feature though so I will definitely consider implementing it!

appy-one commented 11 months ago

This code should be refactored to use

const updates = { interval: 1234 };
ref.child('queues/cron/sweepRepositories').update(updates);
fahhem commented 11 months ago

That is just an example, the code in question actually has multiple updates in the dict at different levels of nesting, which means I can't simply refactor it like that. Also, this is another non-Firebase-compatible API. Is there no interest in matching Firebase's API anymore?