Attempting to update a document with revision comparison in 7.2.0 causes conflict

[robross0606]

This unit test fails:

  it.only('Document updates with revision checking causes conflict', async () => {
    const testCount = 10

    const db = arangoService.getInstance()
    const collection = await db.createCollection('test')
    const collectionData = await collection.get()
    expect(collectionData.type).toBe(CollectionType.DOCUMENT_COLLECTION)

    // Create documents
    const created = []
    for (let i = 0; i < testCount; i++) {
      created.push(await collection.save({ test: 'update-failure', count: 1 }, { returnNew: true }))
    }
    expect(created).toBeArrayOfObjects()
    expect(created).toBeArrayOfSize(testCount)

    // Now try to update them
    const updated = []
    for (const doc of created) {
      updated.push(
        await collection.update(
          doc._id,
          { _rev: doc._rev, updated: true },
          { returnOld: true, returnNew: true, ignoreRevs: false }
        )
      )
    }
    expect(updated).toBeArrayOfObjects()
    expect(updated).toBeArrayOfSize(testCount)
  })

The error is:

    ArangoError: conflict

      at new ArangoError (node_modules/src/error.ts:143:17)
      at Object.resolve (node_modules/src/connection.ts:900:20)
      at callback (node_modules/src/connection.ts:628:16)

If ignoreRevs is set to true or the _rev property is not provided on the document content, the test passes.

[robross0606]

I have also tried with a single document update which also fails with the same error:

  it.only('Document update with revision checking causes conflict', async () => {
    const db = arangoService.getInstance()
    const collection = await db.createCollection('test')
    const collectionData = await collection.get()
    expect(collectionData.type).toBe(CollectionType.DOCUMENT_COLLECTION)

    // Create document
    const created = await collection.save({ test: 'update-failure' }, { returnNew: true })
    expect(created).toBeNonEmptyObject()
    expect(created._id).toBeNonEmptyString()
    expect(created._rev).toBeNonEmptyString()
    expect(created.new.test).toBe('update-failure')
    expect(created.new.updated).toBeFalsy()

    // Now try to update
    const updated = await collection.update(
      created._id,
      { _rev: created._rev, updated: true },
      { returnOld: true, returnNew: true, ignoreRevs: false }
    )
    expect(updated).toBeNonEmptyObject()
    expect(updated._id).toBe(created._id)
    expect(updated._rev).not.toBe(created._rev)
    expect(updated.new.test).toBe(created.new.test)
    expect(updated.new.updated).toBeTruthy()
  })

Failure:

    ArangoError: conflict

      at new ArangoError (node_modules/src/error.ts:143:17)
      at Object.resolve (node_modules/src/connection.ts:900:20)
      at callback (node_modules/src/connection.ts:628:16)
      at IncomingMessage.<anonymous> (node_modules/src/lib/request.node.ts:165:15)

[robross0606]

With the old driver where rev was passed in as an option, this resulted in an HTTP API header value:

if-match: _bkexiY----

However, with the new API, there is no if-match header value. The value is passed in as a _rev in the body and this fails for some reason even though the HTTP API docs read for all the world like this should succeed:

If the If-Match header is specified and the revision of the document in the database is unequal to the given revision, the precondition is violated.

If If-Match is not given and ignoreRevs is false and there is a _rev attribute in the body and its value does not match the revision of the document in the database, the precondition is violated.

I have attached two Wireshark PCAP captures showing the difference between the two drivers using effectively the same unit test.

PCAP.zip

[robross0606]

I think the documentation on HTTP API is incorrect, or at least misleading. I tried this directly from the REST "Try It" feature on the Support page of the web UI and it fails in a similar manner. If using the If-Match header it works fine. If using a _rev stored in the body of the input, it fails:

[pluma]

Looks like the HTTP API docs are incorrect or misleading. It's probably easiest to restore the if-match behavior of v6, though I think the new option should be called ifMatch rather than rev.

[robross0606]

My only issue with calling it ifMatch is you're intentionally breaking backward compatibility with previous versions of your library. Putting back rev would make it backward compatible. This isn't a new option. It is an old option that was removed. Either way, I'll take it if it works. :)

[robross0606]

FYI, underlying Arango bug (arangodb/arangodb#13232) is confirmed.

[Simran-B]

The server-side issue is fixed in 3.6.12 and 3.7.7, but it may still be desirable to have a driver-side workaround for earlier versions.

[robross0606]

@Simran-B, trying to plan for our own projects. Any idea if/when a "driver-side workaround for earlier versions" might be implemented?

[robross0606]

@Simran-B, I'm trying to figure out if this change now makes the ArangoJS 7.x driver incompatible with older servers. This would appear to be the case as attempting to put _rev in the input with ignoreRevs: false works okay with 3.7.9 but breaks against a 3.6.5 server with a " precondition failed" error. Is there some way to use the 7.x driver update() call with revision checks that is compatible with both 3.6.x and 3.7.x servers?