[RFC] Shall we clean up room information when we receive room events?

[windmemory]

It seems like when we receive room events, we don't clean up the cached room information, thus the room information will get staled.

Here is the code when we receive a room topic events:

case 'room-topic':
  puppet.on('room-topic', async (roomId, newTopic, oldTopic, changerId) => {
    const room = this.Room.load(roomId)
    await room.ready()

    const changer = this.Contact.load(changerId)
    await changer.ready()

    this.emit('room-topic', room, newTopic, oldTopic, changer)
    room.emit('topic', newTopic, oldTopic, changer)
  })
  break

It does call the room.ready() function to get the latest data for the room, but in /user/room.ts, the code to do the ready is below:

  public async ready (
    dirty = false,
  ): Promise<void> {
    log.verbose('Room', 'ready()')

    if (!dirty && this.isReady()) {
      return
    }

    if (dirty) {
      await this.puppet.roomPayloadDirty(this.id)
    }
    this.payload = await this.puppet.roomPayload(this.id)

    if (!this.payload) {
      throw new Error('ready() no payload')
    }

    const memberIdList = await this.puppet.roomMemberList(this.id)

    await Promise.all(
      memberIdList
        .map(id => this.wechaty.Contact.load(id))
        .map(contact => {
          contact.ready()
            .catch(() => {
              //
            })
        }),
    )
  }

So, without passing dirty parameter into the ready function, it won't refresh the data. That is to say, when some room events happened, the room won't get refreshed.

In wechaty-puppet layer, there is cache for the roomPayload, and the cache won't be refreshed in wechaty-puppet layer itself, it only get triggered in wechaty layer. Event the underline layer like wechaty-puppet-padchat updated the data for the room, it can not be popped up.

Will submit a PR later as a proposal.

[huan]

After I think about it again, I agree with you and your PR before makes sense for us.

Could you please resubmit this PR again, and replace all ready(true) to sync()?

Thanks.

[huan]

@windmemory Finally I decide to make sync() public. (and ready() should be private)

It seems we have to keep it because of the reasons we had discussed. Thank you for your thoughts about this.

BTW: and also I had decided to make roomMemberPayloadDirty() public too, because we have already had a publish API as roomMemberPayload(), so public it will not be a problem.

Please also restore your puppet PR and I'll merge it. After that, you will get all your modifications ready for use.

[windmemory]

Thanks for the discussion, they are valuable. You are the best :)