client icon indicating copy to clipboard operation
client copied to clipboard

Filter works incorrectly if fields are splitted by several log actions

Open LexxFedoroff opened this issue 1 year ago • 4 comments
trafficstars

Hello, I've debugged this issue https://github.com/logux/examples/issues/21 and found that there is a bug in the filters. I can reproduce it with a simple test:

it('filter works correctly if sent not all fields', async () => {
  let client = new TestClient('10')
  await client.connect()
  client.log.keepActions()

  let posts = createFilter(client, Post, { authorId: '1'})
  let unbind = posts.listen(() => {})
  await allTasks()

  await client.server.sendAll({ channel: 'posts/1', type: 'logux/subscribed' })
  await client.server.sendAll({
    fields: { title: 'A' },
    id: '1',
    type: 'posts/created'
  })
  await client.server.sendAll({
    fields: { authorId: '1' },
    id: '1',
    type: 'posts/changed'
  })
  await allTasks()

  expect(ensureLoaded(posts.get()).list).toEqual([
    { id: '1', isLoading: false, title: 'A', authorId: '1' },
  ])
  unbind()
})

I want to fix it but I need some information on how that should work. As I understand we need to accumulate log actions for an object until we receive the fields required for a filter. What is the best way to achieve this? Version: 0.21.1

LexxFedoroff avatar Aug 16 '24 15:08 LexxFedoroff

Hm. I thought about this case.

This block should do the stuff https://github.com/logux/client/blob/main/create-filter/index.js#L280-L291

Can you investigate why it doesn’t work?

ai avatar Aug 16 '24 15:08 ai

I've investigated and I think the problem comes from the commit https://github.com/logux/client/commit/e2dac178fc7b62b9fde8fdb27f4764cd4c54883b Currently, the sync map is created from the last received action but this action doesn't contain all necessary fields. @euaaaio what do you think?

LexxFedoroff avatar Aug 19 '24 14:08 LexxFedoroff

I won't be able to review it closely today.

I only remember that we had a problem with the implementation, which was somehow very different from how it was designed. Could it be that the bug is in the example repository?

euaaaio avatar Aug 19 '24 14:08 euaaaio

Could it be that the bug is in the example repository?

not sure, I think the bug is in the createFilter method I'm a newbie in the logux framework and I don't know exactly how it should work but I see two options:

  • a filter must accumulate all changes of the map and when any of the filter's fields comes then creates a sync map with all changes. In this case, there are no extra sync operations but we spend more memory to keep unnecessary data.
  • a filter listens to changes of the map and when any of the filter's fields comes then creates an empty sync map that will sync again. In this case, there are extra sync operations but we don't spend more memory to keep unnecessary data.

the second option was implemented before the commit https://github.com/logux/client/commit/e2dac178fc7b62b9fde8fdb27f4764cd4c54883b and I tend that was correct

LexxFedoroff avatar Aug 20 '24 15:08 LexxFedoroff