From d0e66751196151e18005c982deffcfcecea97397 Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Tue, 24 Dec 2024 16:20:51 +0100 Subject: [PATCH] sqlite: pass conflict type to conflict-resulution handler --- doc/api/sqlite.md | 50 +++++++- src/node_sqlite.cc | 25 ++-- test/parallel/test-sqlite-session.js | 165 ++++++++++++++++++++++++--- 3 files changed, 208 insertions(+), 32 deletions(-) diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index 9e18a12ea90a71..6e9d7da8f32f36 100644 --- a/doc/api/sqlite.md +++ b/doc/api/sqlite.md @@ -234,10 +234,19 @@ added: * `options` {Object} The configuration options for how the changes will be applied. * `filter` {Function} Skip changes that, when targeted table name is supplied to this function, return a truthy value. By default, all changes are attempted. - * `onConflict` {number} Determines how conflicts are handled. **Default**: `SQLITE_CHANGESET_ABORT`. - * `SQLITE_CHANGESET_OMIT`: conflicting changes are omitted. - * `SQLITE_CHANGESET_REPLACE`: conflicting changes replace existing values. - * `SQLITE_CHANGESET_ABORT`: abort on conflict and roll back database. + * `onConflict` {Function} A function that determines how to handle conflicts. The function receives one argument, which can be one of the following values: + * `SQLITE_CHANGESET_DATA`: A `DELETE` or `UPDATE` change does not contain the expected "before" values. + * `SQLITE_CHANGESET_NOTFOUND`: A row matching the primary key of the `DELETE` or `UPDATE` change does not exist. + * `SQLITE_CHANGESET_CONFLICT`: An `INSERT` change results in a duplicate primary key. + * `SQLITE_CHANGESET_FOREIGN_KEY`: Applying a change would result in a foreign key violation. + * `SQLITE_CHANGESET_CONSTRAINT`: Applying a change results in a `UNIQUE`, `CHECK`, or `NOT NULL` constraint violation. + + The function should return one of the following values: + * `SQLITE_CHANGESET_OMIT`: Omit conflicting changes. + * `SQLITE_CHANGESET_REPLACE`: Replace existing values with conflicting changes (only valid with `SQLITE_CHANGESET_DATA` or `SQLITE_CHANGESET_CONFLICT` conflicts). + * `SQLITE_CHANGESET_ABORT`: Abort on conflict and roll back the database. + + **Default**: A function that returns `SQLITE_CHANGESET_ABORT`. * Returns: {boolean} Whether the changeset was applied succesfully without being aborted. An exception is thrown if the database is not @@ -498,7 +507,36 @@ The following constants are exported by the `sqlite.constants` object. #### Conflict-resolution constants -The following constants are meant for use with [`database.applyChangeset()`](#databaseapplychangesetchangeset-options). +One of the following constants is available as an argument to the `onConflict` conflict-resolution handler passed to [`database.applyChangeset()`](#databaseapplychangesetchangeset-options)). See also [Constants Passed To The Conflict Handler](https://www.sqlite.org/session/c_changeset_conflict.html) in the SQLite documentation. + + + + + + + + + + + + + + + + + + + + + + + + + + +
ConstantDescription
SQLITE_CHANGESET_DATAThe conflict handler is invoked with this constant when processing a DELETE or UPDATE change if a row with the required PRIMARY KEY fields is present in the database, but one or more other (non primary-key) fields modified by the update do not contain the expected "before" values.
SQLITE_CHANGESET_NOTFOUNDThe conflict handler is invoked with this constant when processing a DELETE or UPDATE change if a row with the required PRIMARY KEY fields is not present in the database.
SQLITE_CHANGESET_CONFLICTThis constant is passed to the conflict handler while processing an INSERT change if the operation would result in duplicate primary key values.
SQLITE_CHANGESET_CONSTRAINTIf foreign key handling is enabled, and applying a changeset leaves the database in a state containing foreign key violations, the conflict handler is invoked with this constant exactly once before the changeset is committed. If the conflict handler returns SQLITE_CHANGESET_OMIT, the changes, including those that caused the foreign key constraint violation, are committed. Or, if it returns SQLITE_CHANGESET_ABORT, the changeset is rolled back.
SQLITE_CHANGESET_FOREIGN_KEYIf any other constraint violation occurs while applying a change (i.e. a UNIQUE, CHECK or NOT NULL constraint), the conflict handler is invoked with this constant.
+ +One of the following constants must be returned from the `onConflict` conflict-resolution handler passed to [`database.applyChangeset()`](#databaseapplychangesetchangeset-options). @@ -511,7 +549,7 @@ The following constants are meant for use with [`database.applyChangeset()`](#da - + diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 7f5e2f89ce9dba..6c0df79238c8fa 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -731,11 +731,11 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo& args) { // the reason for using static functions here is that SQLite needs a // function pointer -static std::function conflictCallback; +static std::function conflictCallback; static int xConflict(void* pCtx, int eConflict, sqlite3_changeset_iter* pIter) { if (!conflictCallback) return SQLITE_CHANGESET_ABORT; - return conflictCallback(); + return conflictCallback(eConflict); } static std::function filterCallback; @@ -773,15 +773,20 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { options->Get(env->context(), env->onconflict_string()).ToLocalChecked(); if (!conflictValue->IsUndefined()) { - if (!conflictValue->IsNumber()) { + if (!conflictValue->IsFunction()) { THROW_ERR_INVALID_ARG_TYPE( env->isolate(), - "The \"options.onConflict\" argument must be a number."); + "The \"options.onConflict\" argument must be a function."); return; } - - int conflictInt = conflictValue->Int32Value(env->context()).FromJust(); - conflictCallback = [conflictInt]() -> int { return conflictInt; }; + Local conflictFunc = conflictValue.As(); + conflictCallback = [env, conflictFunc](int conflictType) -> int { + Local argv[] = {Integer::New(env->isolate(), conflictType)}; + Local result = + conflictFunc->Call(env->context(), Null(env->isolate()), 1, argv) + .ToLocalChecked(); + return result->Int32Value(env->context()).FromJust(); + }; } if (options->HasOwnProperty(env->context(), env->filter_string()) @@ -1662,6 +1667,12 @@ void DefineConstants(Local target) { NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_OMIT); NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_REPLACE); NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_ABORT); + + NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_DATA); + NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_NOTFOUND); + NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_CONFLICT); + NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_CONSTRAINT); + NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_FOREIGN_KEY); } static void Initialize(Local target, diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js index 617c0c2aa71181..8ccc4555a52ac9 100644 --- a/test/parallel/test-sqlite-session.js +++ b/test/parallel/test-sqlite-session.js @@ -128,15 +128,15 @@ test('database.createSession() - use table option to track specific table', (t) }); suite('conflict resolution', () => { + const createDataTableSql = `CREATE TABLE data ( + key INTEGER PRIMARY KEY, + value TEXT UNIQUE + ) STRICT`; + const prepareConflict = () => { const database1 = new DatabaseSync(':memory:'); const database2 = new DatabaseSync(':memory:'); - const createDataTableSql = `CREATE TABLE data ( - key INTEGER PRIMARY KEY, - value TEXT - ) STRICT - `; database1.exec(createDataTableSql); database2.exec(createDataTableSql); @@ -151,7 +151,91 @@ suite('conflict resolution', () => { }; }; - test('database.applyChangeset() - conflict with default behavior (abort)', (t) => { + const prepareDataConflict = () => { + const database1 = new DatabaseSync(':memory:'); + const database2 = new DatabaseSync(':memory:'); + + database1.exec(createDataTableSql); + database2.exec(createDataTableSql); + + const insertSql = 'INSERT INTO data (key, value) VALUES (?, ?)'; + database1.prepare(insertSql).run(1, 'hello'); + database2.prepare(insertSql).run(1, 'othervalue'); + const session = database1.createSession(); + database1.prepare('UPDATE data SET value = ? WHERE key = ?').run('foo', 1); + return { + database2, + changeset: session.changeset() + } + } + + const prepareNotFoundConflict = () => { + const database1 = new DatabaseSync(':memory:'); + const database2 = new DatabaseSync(':memory:'); + + database1.exec(createDataTableSql); + database2.exec(createDataTableSql); + + const insertSql = 'INSERT INTO data (key, value) VALUES (?, ?)'; + database1.prepare(insertSql).run(1, 'hello'); + const session = database1.createSession(); + database1.prepare('DELETE FROM data WHERE key = 1').run(); + return { + database2, + changeset: session.changeset() + } + } + + const prepareFkConflict = () => { + const database1 = new DatabaseSync(':memory:'); + const database2 = new DatabaseSync(':memory:'); + + database1.exec(createDataTableSql); + database2.exec(createDataTableSql); + const fkTableSql = `CREATE TABLE other ( + key INTEGER PRIMARY KEY, + ref REFERENCES data(key) + )`; + database1.exec(fkTableSql); + database2.exec(fkTableSql); + + const insertDataSql = 'INSERT INTO data (key, value) VALUES (?, ?)'; + const insertOtherSql = 'INSERT INTO other (key, ref) VALUES (?, ?)'; + database1.prepare(insertDataSql).run(1, 'hello'); + database2.prepare(insertDataSql).run(1, 'hello'); + database1.prepare(insertOtherSql).run(1, 1); + database2.prepare(insertOtherSql).run(1, 1); + + database1.exec('DELETE FROM other WHERE key = 1'); // so we don't get a fk violation in database1 + const session = database1.createSession(); + database1.prepare("DELETE FROM data WHERE key = 1").run(); // changeset with fk violation + database2.exec("PRAGMA foreign_keys = ON"); // needs to be supported, otherwise will fail here + + return { + database2, + changeset: session.changeset() + } + } + + const prepareConstraintConflict = () => { + const database1 = new DatabaseSync(':memory:'); + const database2 = new DatabaseSync(':memory:'); + + database1.exec(createDataTableSql); + database2.exec(createDataTableSql); + + const insertSql = 'INSERT INTO data (key, value) VALUES (?, ?)'; + const session = database1.createSession(); + database1.prepare(insertSql).run(1, 'hello'); + database2.prepare(insertSql).run(2, 'hello'); // database2 already constains hello + + return { + database2, + changeset: session.changeset() + }; + }; + + test('database.applyChangeset() - SQLITE_CHANGESET_CONFLICT conflict with default behavior (abort)', (t) => { const { database2, changeset } = prepareConflict(); // When changeset is aborted due to a conflict, applyChangeset should return false t.assert.strictEqual(database2.applyChangeset(changeset), false); @@ -160,40 +244,83 @@ suite('conflict resolution', () => { [{ value: 'world' }]); // unchanged }); - test('database.applyChangeset() - conflict with SQLITE_CHANGESET_ABORT', (t) => { + test('database.applyChangeset() - SQLITE_CHANGESET_CONFLICT conflict handled with SQLITE_CHANGESET_ABORT', (t) => { const { database2, changeset } = prepareConflict(); + let conflictType = null; const result = database2.applyChangeset(changeset, { - onConflict: constants.SQLITE_CHANGESET_ABORT + onConflict: (conflictType_) => { + conflictType = conflictType_; + return constants.SQLITE_CHANGESET_ABORT + } }); // When changeset is aborted due to a conflict, applyChangeset should return false t.assert.strictEqual(result, false); + t.assert.strictEqual(conflictType, constants.SQLITE_CHANGESET_CONFLICT); deepStrictEqual(t)( database2.prepare('SELECT value from data').all(), [{ value: 'world' }]); // unchanged }); - test('database.applyChangeset() - conflict with SQLITE_CHANGESET_REPLACE', (t) => { - const { database2, changeset } = prepareConflict(); + test('database.applyChangeset() - SQLITE_CHANGESET_DATA conflict handled with SQLITE_CHANGESET_REPLACE', (t) => { + const { database2, changeset } = prepareDataConflict(); + let conflictType = null; const result = database2.applyChangeset(changeset, { - onConflict: constants.SQLITE_CHANGESET_REPLACE + onConflict: (conflictType_) => { + conflictType = conflictType_; + return constants.SQLITE_CHANGESET_REPLACE; + } }); // Not aborted due to conflict, so should return true t.assert.strictEqual(result, true); + t.assert.strictEqual(conflictType, constants.SQLITE_CHANGESET_DATA); deepStrictEqual(t)( database2.prepare('SELECT value from data ORDER BY key').all(), - [{ value: 'hello' }, { value: 'foo' }]); // replaced + [{ value: 'foo' }]); // replaced }); - test('database.applyChangeset() - conflict with SQLITE_CHANGESET_OMIT', (t) => { - const { database2, changeset } = prepareConflict(); + test('database.applyChangeset() - SQLITE_CHANGESET_NOTFOUND conflict with SQLITE_CHANGESET_OMIT', (t) => { + const { database2, changeset } = prepareNotFoundConflict(); + let conflictType = null; const result = database2.applyChangeset(changeset, { - onConflict: constants.SQLITE_CHANGESET_OMIT + onConflict: (conflictType_) => { + conflictType = conflictType_; + return constants.SQLITE_CHANGESET_OMIT; + } }); // Not aborted due to conflict, so should return true t.assert.strictEqual(result, true); - deepStrictEqual(t)( - database2.prepare('SELECT value from data ORDER BY key ASC').all(), - [{ value: 'world' }, { value: 'foo' }]); // Conflicting change omitted + t.assert.strictEqual(conflictType, constants.SQLITE_CHANGESET_NOTFOUND); + deepStrictEqual(t)(database2.prepare('SELECT value from data').all(), []); + }); + + test('database.applyChangeset() - SQLITE_CHANGESET_FOREIGN_KEY conflict', (t) => { + const { database2, changeset } = prepareFkConflict(); + let conflictType = null; + const result = database2.applyChangeset(changeset, { + onConflict: (conflictType_) => { + conflictType = conflictType_; + return constants.SQLITE_CHANGESET_OMIT; + } + }); + // Not aborted due to conflict, so should return true + t.assert.strictEqual(result, true); + t.assert.strictEqual(conflictType, constants.SQLITE_CHANGESET_FOREIGN_KEY); + deepStrictEqual(t)(database2.prepare('SELECT value from data').all(), []); + }); + + test('database.applyChangeset() - SQLITE_CHANGESET_CONSTRAINT conflict', (t) => { + const { database2, changeset } = prepareConstraintConflict(); + let conflictType = null; + const result = database2.applyChangeset(changeset, { + onConflict: (conflictType_) => { + conflictType = conflictType_; + return constants.SQLITE_CHANGESET_OMIT; + } + }); + // Not aborted due to conflict, so should return true + t.assert.strictEqual(result, true); + t.assert.strictEqual(conflictType, constants.SQLITE_CHANGESET_CONSTRAINT); + deepStrictEqual(t)(database2.prepare('SELECT key, value from data').all(), [{ key: 2, value: 'hello' }]); }); }); @@ -299,7 +426,7 @@ test('database.applyChangeset() - wrong arguments', (t) => { }, null); }, { name: 'TypeError', - message: 'The "options.onConflict" argument must be a number.' + message: 'The "options.onConflict" argument must be a function.' }); });
SQLITE_CHANGESET_REPLACEConflicting changes replace existing values.Conflicting changes replace existing values. Note that this value can only be returned when the type of conflict is either `SQLITE_CHANGESET_DATA` or `SQLITE_CHANGESET_CONFLICT`.
SQLITE_CHANGESET_ABORT