ether / etherpad-lite

Etherpad: A modern really-real-time collaborative document editor.
http://docs.etherpad.org/
Apache License 2.0
16.73k stars 2.86k forks source link

padUpdate missing padId #5814

Open danielpetri1 opened 1 year ago

danielpetri1 commented 1 year ago

Describe the bug The padUpdate hook does not appear to include the pad.id attribute as it did in 1.8.17.

Sample:

{
    "body": {
        "pad": {
            "atext": {
                "text": "​s\n",
                "attribs": "+1*0+1|1+1"
            },
            "pool": {
                "numToAttrib": {
                    "0": [
                        "author",
                        "a.i2WSCwOkL5ZS19pD"
                    ]
                },
                "nextNum": 1
            },
            "head": 1,
            "chatHead": -1,
            "publicStatus": false,
            "savedRevisions": []
        },
        "authorId": "a.i2WSCwOkL5ZS19pD",
        "author": "a.i2WSCwOkL5ZS19pD",
        "revs": 1,
        "changeset": "Z:2>1=1*0+1$s"
    }
}

Expected behavior The hook should provide the padId in the pad object so that it is possible to know which pad was modified.

Server (please complete the following information):

Additional context Pad was created via the HTTP API. It may be related to the open issues/PR surrounding the appendRevision method. I tried the suggested fix but it did not resolve this issue.

webzwo0i commented 1 year ago

Cannot reproduce. In your example the "db" key is missing and also the "id" field. This is how it looks like on my side. Because you have a "body" field I assume you're doing some serialization.

{
  pad: Pad {
    db: {
      db: [Database],
      init: [AsyncFunction (anonymous)],
      shutdown: [AsyncFunction (anonymous)],
      get: [AsyncFunction: get] Function,
      set: [AsyncFunction: set] Function,
      findKeys: [AsyncFunction: findKeys] Function,
      getSub: [AsyncFunction: getSub] Function,
      setSub: [AsyncFunction: setSub] Function,
      remove: [AsyncFunction: remove] Function
    },
    atext: {
      text: 'vnbWelcome to Etherpadf!c\n' +
        '\n' +
        'This pad text is synchronized as you type, so that everyone viewing this page sees the same text. This allows you to collaborate seamlessly on documents!\n' +
        '\n' +
        'Get involved with Etherpad at https://etherpad.org\n' +
        '\n' +
        'Warning: DirtyDB is used. This is not recommended for production. -- To suppress these warning messages change suppressErrorsInPadText to true in your settings.json\n' +
        '\n',
      attribs: '*0+3+j*0+1+1*0+1|8+af'
    },
    pool: AttributePool {
      numToAttrib: [Object],
      attribToNum: [Object],
      nextNum: 1
    },
    head: 5,
    chatHead: -1,
    publicStatus: false,
    id: '-GylVTuDq6n445f4jnMN',
    savedRevisions: []
  },
  authorId: 'a.79CsRs7RLFAsPPxx',
  author: [Getter/Setter],
  revs: 5,
  changeset: 'Z:b3>1*0+1$v'
}
webzwo0i commented 1 year ago

Do you use padUpdate hook in a public plugin so we can take a look at the code? As seen above it should be context.pad.id

danielpetri1 commented 1 year ago

Hi, thank you for the swift response. Thanks for the info that the db key was missing too, as this pointed me in the right direction. I am indeed using a public plugin, namely ep_redis_publisher, which explains why "db" is missing.

The events are then read in by BigBlueButton's pads manager with context.pad.id already missing at that point. As you couldn't reproduce it, it's most likely not an Etherpad issue. Sorry for the spam and appreciate your time.

danielpetri1 commented 1 year ago

I believe the issue is that the Pad class has its own toJSON implementation which removes the id. So when calling e.g. JSON.stringify(), the padId is removed despite being present in the object. Was this an intentional change?

https://github.com/ether/etherpad-lite/blob/13330c45f8b17f002e76d70beed22aeadb901801/src/node/db/Pad.js#L123

danielpetri1 commented 1 year ago

Introduced in https://github.com/ether/etherpad-lite/commit/79e7697c1cff44fb607d0b123c486f52d55e805d

SamTV12345 commented 1 year ago

True. Id and db are deleted. But it seems like that was also dropped before with a blacklist.