WORK IN PROGRESS -- PLEASE IGNORE ME -- Asyncify mysql_db.js and CacheAndBufferLayer.js, and other cleanups

[rhansen]

Multiple commits:

• Use this instead of self, _this, or that
• Simplify write callback conditional
• Avoid iterating over inherited properties
• Avoid repeated key lookups
• Invert the dirty check to reduce indent level (for readability)
• Asyncify CacheAndBufferLayer.js
• Asyncify mysql_db.js

Before this change many errors were silently ignored. Now they are propagated up, partially addressing #153.

[JohnMcLear]

Awesome stuff will look at this tomorrow.

[webzwo0i]

Did a small test run:

 TypeError [ERR_INVALID_ARG_TYPE]: The last argument must be of type function. Received undefined
    at new NodeError (node:internal/errors:278:15)
    at bound Callbackified (node:util:206:13)
    at exports.database.setSub (/dev/shm/etherpad-lite/src/node_modules/ueberdb2/lib/CacheAndBufferLayer.js:283:51)
    at exports.channels.doOperation [as operatorFunction] (/dev/shm/etherpad-lite/src/node_modules/ueberdb2/index.js:146:18)
    at exports.channels.emit (/dev/shm/etherpad-lite/src/node_modules/ueberdb2/node_modules/channels/channels.js:49:11)
    at exports.database.setSub (/dev/shm/etherpad-lite/src/node_modules/ueberdb2/index.js:91:17)
    at node:internal/util:297:30
    at new Promise (<anonymous>)
    at Object.bound  [as setSub] (node:internal/util:296:12)
    at mapAuthorWithDBKey (/dev/shm/etherpad-lite/src/node/db/AuthorManager.js:156:12)

when running with mysql
I don't know what bufferCallback and writeCallback are about, just adding an empty function seems to "work around" the problem - but for sure I would need to study the code to understand the nature of those callbacks:

diff --git a/index.js b/index.js
index 1135934..159e009 100644
--- a/index.js
+++ b/index.js
@@ -93,6 +93,9 @@ exports.database.prototype.setSub = function (key, sub, value, bufferCallback, w
 };
 
 const doOperation = (operation, callback) => {
+  if (!operation.writeCallback) {
+    operation.writeCallback = () => {};
+  }
   if (operation.type === 'get') {
     operation.db.get(operation.key, (err, value) => {
       // clone the value

Maybe it's a problem with my setup, but on backend tests:

      Import authorization checks
        6) authn user modify !exist -> fail
        7) authn user readonly !exist -> fail
        8) authn user create exist -> replace
        9) authn user modify exist -> replace
        10) authn user readonly exist -> fail
    Abnormal access attempts
      11) authn anonymous /p/pad -> 401, error
      12) authn !cookie -> error
      13) authorization bypass attempt -> error
    Authorization levels via authorize hook
      14) level='create' -> can create
      15) level=true -> can create
      16) level='modify' -> can modify
      17) level='create' settings.editOnly=true -> unable to create
      18) level='modify' settings.editOnly=false -> unable to create
      19) level='readOnly' -> unable to create
      20) level='readOnly' -> unable to modify
    Authorization levels via user settings
      21) user.canCreate = true -> can create and modify
      22) user.canCreate = false -> unable to create
      23) user.readOnly = true -> unable to create
      24) user.readOnly = true -> unable to modify
      25) user.readOnly = false -> can create and modify
      26) user.readOnly = true, user.canCreate = true -> unable to create

I noticed it's very slow on mysql - I usually don't run my tests with mysql (probably I should prepare a better test setup here...), so I don't know if this PR or something else introduced the slowness.

With dirtydb backend the following tests fail:

        6) authn user modify !exist -> fail
        7) authn user readonly !exist -> fail
        8) authn user create exist -> replace
        9) authn user modify exist -> replace
        10) authn user readonly exist -> fail

Maybe the changes exposed some problems (error handling?) that we need to fix in etherpad-core?
When setting writeInterval to 0, so that everything is directly written to database, the number of errors increases for mysql. (Probably timeouts).

[rhansen]

Did a small test run:

Thanks for testing! I pushed a fix and tested it against Etherpad with MySQL and dirty to confirm that it works.

I noticed it's very slow on mysql

My unscientific tests showed MySQL to be a bit faster than dirty (46s vs. 54s for backend tests on my slow machine). This PR probably does add some overhead, but it shouldn't be significant.

[JohnMcLear]

I have to conclude that this work has introduced some additional slowness. I only tested MySQL, other databases should probably also be checked..

Before (master)

no-cache
┌──────────┬──────────────────┬──────────────────┐
│ op       │ Acceptable ms/op │ Actual ms/op     │
├──────────┼──────────────────┼──────────────────┤
│ set      │ 3                │ 0.338            │
├──────────┼──────────────────┼──────────────────┤
│ get      │ 0.1              │ 0.125            │
├──────────┼──────────────────┼──────────────────┤
│ findKey  │ 3                │ 7.008            │
├──────────┼──────────────────┼──────────────────┤
│ remove   │ 1                │ 0.727            │
└──────────┴──────────────────┴──────────────────┘
cache
┌──────────┬──────────────────┬──────────────────┐
│ op       │ Acceptable ms/op │ Actual ms/op     │
├──────────┼──────────────────┼──────────────────┤
│ set      │ 3                │ 0.343            │
├──────────┼──────────────────┼──────────────────┤
│ get      │ 0.1              │ 0.123            │
├──────────┼──────────────────┼──────────────────┤
│ findKey  │ 3                │ 6.887            │
├──────────┼──────────────────┼──────────────────┤
│ remove   │ 1                │ 0.708            │
└──────────┴──────────────────┴──────────────────┘

After

no-cache
┌──────────┬──────────────────┬──────────────────┐
│ op       │ Acceptable ms/op │ Actual ms/op     │
├──────────┼──────────────────┼──────────────────┤
│ set      │ 3                │ 0.461            │
├──────────┼──────────────────┼──────────────────┤
│ get      │ 0.1              │ 0.204            │
├──────────┼──────────────────┼──────────────────┤
│ findKey  │ 3                │ 8.379            │
├──────────┼──────────────────┼──────────────────┤
│ remove   │ 1                │ 0.84             │
└──────────┴──────────────────┴──────────────────┘
cache
┌──────────┬──────────────────┬──────────────────┐
│ op       │ Acceptable ms/op │ Actual ms/op     │
├──────────┼──────────────────┼──────────────────┤
│ set      │ 3                │ 0.456            │
├──────────┼──────────────────┼──────────────────┤
│ get      │ 0.1              │ 0.248            │
├──────────┼──────────────────┼──────────────────┤
│ findKey  │ 3                │ 9.824            │
├──────────┼──────────────────┼──────────────────┤
│ remove   │ 1                │ 0.837            │
└──────────┴──────────────────┴──────────────────┘