diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index 9e18a12ea90a71..5b1831f40f2076 100644 --- a/doc/api/sqlite.md +++ b/doc/api/sqlite.md @@ -234,10 +234,27 @@ 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. + + When an error is thrown in the conflict handler or when any other value is returned from the handler, + applying the changeset is aborted and the database is rolled back. + + **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 @@ -496,9 +513,42 @@ An object containing commonly used constants for SQLite operations. The following constants are exported by the `sqlite.constants` object. -#### Conflict-resolution constants +#### Conflict resolution constants + +One of the following constants is available as an argument to the `onConflict` +conflict resolution handler passed to [`database.applyChangeset()`][]. See also +[Constants Passed To The Conflict Handler][] 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.
-The following constants are meant for use with [`database.applyChangeset()`](#databaseapplychangesetchangeset-options). +One of the following constants must be returned from the `onConflict` conflict +resolution handler passed to [`database.applyChangeset()`][]. See also +[Constants Returned From The Conflict Handler][] in the SQLite documentation. @@ -511,7 +561,7 @@ The following constants are meant for use with [`database.applyChangeset()`](#da - + @@ -520,11 +570,14 @@ The following constants are meant for use with [`database.applyChangeset()`](#da
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
[Changesets and Patchsets]: https://www.sqlite.org/sessionintro.html#changesets_and_patchsets +[Constants Passed To The Conflict Handler]: https://www.sqlite.org/session/c_changeset_conflict.html +[Constants Returned From The Conflict Handler]: https://www.sqlite.org/session/c_changeset_abort.html [SQL injection]: https://en.wikipedia.org/wiki/SQL_injection [`ATTACH DATABASE`]: https://www.sqlite.org/lang_attach.html [`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys [`SQLITE_DETERMINISTIC`]: https://www.sqlite.org/c3ref/c_deterministic.html [`SQLITE_DIRECTONLY`]: https://www.sqlite.org/c3ref/c_deterministic.html +[`database.applyChangeset()`]: #databaseapplychangesetchangeset-options [`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html [`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html [`sqlite3_create_function_v2()`]: https://www.sqlite.org/c3ref/create_function.html diff --git a/src/env_properties.h b/src/env_properties.h index 592e95b0584a87..d9e18cbc4515ac 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -144,10 +144,13 @@ V(entry_type_string, "entryType") \ V(env_pairs_string, "envPairs") \ V(env_var_settings_string, "envVarSettings") \ + V(err_sqlite_error_string, "ERR_SQLITE_ERROR") \ + V(errcode_string, "errcode") \ V(errno_string, "errno") \ V(error_string, "error") \ - V(events, "events") \ + V(errstr_string, "errstr") \ V(events_waiting, "eventsWaiting") \ + V(events, "events") \ V(exchange_string, "exchange") \ V(expire_string, "expire") \ V(exponent_string, "exponent") \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 7f5e2f89ce9dba..abd85a98c5aebb 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -42,6 +42,7 @@ using v8::Number; using v8::Object; using v8::SideEffectType; using v8::String; +using v8::TryCatch; using v8::Uint8Array; using v8::Value; @@ -66,13 +67,14 @@ inline MaybeLocal CreateSQLiteError(Isolate* isolate, const char* message) { Local js_msg; Local e; + Environment* env = Environment::GetCurrent(isolate); if (!String::NewFromUtf8(isolate, message).ToLocal(&js_msg) || !Exception::Error(js_msg) ->ToObject(isolate->GetCurrentContext()) .ToLocal(&e) || e->Set(isolate->GetCurrentContext(), - OneByteString(isolate, "code"), - OneByteString(isolate, "ERR_SQLITE_ERROR")) + env->code_string(), + env->err_sqlite_error_string()) .IsNothing()) { return MaybeLocal(); } @@ -85,15 +87,14 @@ inline MaybeLocal CreateSQLiteError(Isolate* isolate, sqlite3* db) { const char* errmsg = sqlite3_errmsg(db); Local js_errmsg; Local e; + Environment* env = Environment::GetCurrent(isolate); if (!String::NewFromUtf8(isolate, errstr).ToLocal(&js_errmsg) || !CreateSQLiteError(isolate, errmsg).ToLocal(&e) || e->Set(isolate->GetCurrentContext(), - OneByteString(isolate, "errcode"), + env->errcode_string(), Integer::New(isolate, errcode)) .IsNothing() || - e->Set(isolate->GetCurrentContext(), - OneByteString(isolate, "errstr"), - js_errmsg) + e->Set(isolate->GetCurrentContext(), env->errstr_string(), js_errmsg) .IsNothing()) { return MaybeLocal(); } @@ -114,6 +115,19 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) { } } +inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, int errcode) { + const char* errstr = sqlite3_errstr(errcode); + + Environment* env = Environment::GetCurrent(isolate); + auto error = CreateSQLiteError(isolate, errstr).ToLocalChecked(); + error + ->Set(isolate->GetCurrentContext(), + env->errcode_string(), + Integer::New(isolate, errcode)) + .ToChecked(); + isolate->ThrowException(error); +} + class UserDefinedFunction { public: explicit UserDefinedFunction(Environment* env, @@ -731,11 +745,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 +787,27 @@ 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)}; + TryCatch try_catch(env->isolate()); + Local result = + conflictFunc->Call(env->context(), Null(env->isolate()), 1, argv) + .FromMaybe(Local()); + if (try_catch.HasCaught()) { + try_catch.ReThrow(); + return SQLITE_CHANGESET_ABORT; + } + constexpr auto invalid_value = -1; + if (!result->IsInt32()) return invalid_value; + return result->Int32Value(env->context()).FromJust(); + }; } if (options->HasOwnProperty(env->context(), env->filter_string()) @@ -819,12 +845,16 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { xFilter, xConflict, nullptr); + if (r == SQLITE_OK) { + args.GetReturnValue().Set(true); + return; + } if (r == SQLITE_ABORT) { + // this is not an error, return false args.GetReturnValue().Set(false); return; } - CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void()); - args.GetReturnValue().Set(true); + THROW_ERR_SQLITE_ERROR(env->isolate(), r); } void DatabaseSync::EnableLoadExtension( @@ -1662,6 +1692,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..5cba37e337e835 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,120 @@ 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' }]); + }); + + test('conflict resolution handler returns invalid value', (t) => { + const invalidHandlers = [ + () => -1, + () => ({}), + () => null, + async () => constants.SQLITE_CHANGESET_ABORT, + ]; + + for (const invalidHandler of invalidHandlers) { + const { database2, changeset } = prepareConflict(); + t.assert.throws(() => { + database2.applyChangeset(changeset, { + onConflict: invalidHandler + }); + }, { + name: 'Error', + message: 'bad parameter or other API misuse', + errcode: 21, + code: 'ERR_SQLITE_ERROR' + }, `Did not throw expected exception when using invalid onConflict handler: ${invalidHandler}`); + } + }); + + test('conflict resolution handler throws', (t) => { + const { database2, changeset } = prepareConflict(); + t.assert.throws(() => { + database2.applyChangeset(changeset, { + onConflict: () => { + throw new Error('some error'); + } + }); + }, { + name: 'Error', + message: 'some error' + }); }); }); @@ -299,7 +463,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.' }); });