Record paths handle root-level array indices inconsistently

[jdmnd]

In the js client:

let r = client.record.getRecord('foo')
r.set([0,1,2,3])
r.get() // [0, 1, 2, 3]
r.set('[4]', 4)
r.set('.5', 5)
r.get() // [0, 1, 2, 3, 4, 5]
r.discard()
client.record.snapshot('foo', (err, record) => console.log(record)) // [0, 1, 2, 3, null, 5]

In the example above, local and remote record state diverge due to different handling of the path. The client believes the '[4]' path accesses the fourth index of the array, the server believes the path is invalid.

Perhaps this issue should be raised against the client instead, since it's supposed to replicate the behaviour of the server, but in this case I believe this is a due to a regression in server behaviour.

[jdmnd]

Related: #651

[derrandz]

It seems that in v4, the states no longer diverge, I've tested so with the same script as John's in v4:

const deepstream = require('deepstream.io-client-js')
const BBPromise = require('bluebird').Promise
const server = require('./server')

const expect = require('chai').expect

const run = async() => {
  await server.start()

  const client = deepstream('localhost:6020', {
    reconnectIntervalIncrement: 100,
    maxReconnectInterval: 1000,
    maxReconnectAttempts: 999,
  })

  await client.login({})

  const record = client.record.getRecord('posts/a')
  await record.whenReady()

  record.set([0, 1, 2, 3])
  await BBPromise.delay(500)

  record.set('[4]', 4)
  await BBPromise.delay(500)

  record.set('.5', 5)
  await BBPromise.delay(500)

  const localSnapshot = record.get() // [0, 1, 2, 3, null, 5]

  record.discard()
  await BBPromise.delay(500)

  const snapshot = await client.record.snapshot('posts/a')  // [0, 1, 2, 3, null, 5]

  let exceptionCaught = false
  try {
    expect(snapshot).to.deep.equal(localSnapshot)
  } catch (e) {
    exceptionCaught = true
  } finally {
    const message = exceptionCaught
      ? 'failed: path with array indices does not update values properly'
      : 'succeeded: path with array indices updates values correctly'

    console.log('Assertion', message)
    process.exit(exceptionCaught)
  }
}

run()

So this is fixed in v4 since both the local version of the record and the server's have the value [0, 1, 2, 3, null, 5],

However as you can see, the client does not handle properly root level array indices path, resulting in the null in the fourth position. Not sure if it is a client or server issue

---

You can find the script in: https://github.com/deepstreamIO/test-scripts/commit/8361b68fe79f6521fa7ffb0951dbb1a5d0a230ca

[AlexBHarley]

In this script it doesn't look like you wait for the discardTimeout, so when you snapshot you're just loading the same data again.

I would recommend connecting another client and doing a snapshot to confirm data is the same between client and server

[derrandz]

Thank you Alex for pointing out the discard timeout option, that escaped my mind.

Here is the updated version of the script, which also reveals that the states no longer diverge, so we are only left with the issue of not being able to set a root level array using brackets notation, so statements like record.set('[4]', 4) fail to update the root level array correctly.

const deepstream = require('deepstream.io-client-js')
const BBPromise = require('bluebird').Promise
const server = require('./server')

const expect = require('chai').expect
const assert = require('./assert').assert

const run = async() => {
  const snapshots = {
    clientA: {
      local: null,
      remote: null,
    },
    clientB: {
      remote: null
    }
  }

  const discardTimeout = 1000

  await server.start()

  const clientA = deepstream('localhost:6020', {
    reconnectIntervalIncrement: 100,
    maxReconnectInterval: 1000,
    maxReconnectAttempts: 999,
    discardTimeout
  })

  const clientB = deepstream('localhost:6020', {
    reconnectIntervalIncrement: 100,
    maxReconnectInterval: 1000,
    maxReconnectAttempts: 999,
  })

  await clientA.login({})
  await clientB.login({})

  const record = clientA.record.getRecord('posts/a')
  await record.whenReady()

  record.set([0, 1, 2, 3])
  await BBPromise.delay(500)

  record.set('[4]', 4)
  await BBPromise.delay(500)

  record.set('.5', 5)
  await BBPromise.delay(500)

  snapshots.clientA.local = record.get()

  record.discard()

  await BBPromise.delay(discardTimeout)

  snapshots.clientA.remote = await clientA.record.snapshot('posts/a')
  snapshots.clientB.remote = await clientB.record.snapshot('posts/a')

  const assertion = assert(() => {
    expect(snapshots.clientA.local)
      .to.deep.equal(snapshots.clientA.remote)

    expect(snapshots.clientA.local)
      .to.deep.equal(snapshots.clientB.remote)
  }, {
      success: 'path with array indices does not update values properly',
      failure: 'path with array indices updates values correctly'
    })
  
  await BBPromise.delay(100)
  process.exit(assertion)
}

run()

[yasserf]

Closing in favour of #1046