Skip to content

Commit 1774391

Browse files
araujoguiaduh95
araujogui
authored and
aduh95
committed
sqlite: add ReadOptionalString helper
Use it for the dbName, table, db, source and target options so they all treat a missing or undefined value as the default. Also mark ReadOptionalBool and ReadOptionalString as [[nodiscard]]. Signed-off-by: Guilherme Araújo <arauujogui@gmail.com> PR-URL: #67042 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent c685695 commit 1774391

5 files changed

Lines changed: 68 additions & 95 deletions

File tree

‎src/env_properties.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,6 @@
351351
V(stream_count_string, "streamCount") \
352352
V(synthetic_string, "synthetic") \
353353
V(syscall_string, "syscall") \
354-
V(table_string, "table") \
355-
V(target_string, "target") \
356354
V(then_string, "then") \
357355
V(thread_id_string, "threadId") \
358356
V(thread_name_string, "threadName") \

‎src/node_sqlite.cc‎

Lines changed: 34 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,10 +1816,10 @@ void Database::CreateTagStore(const FunctionCallbackInfo<Value>& args) {
18161816
// undefined. Returns false if a JavaScript exception is pending, either from
18171817
// the property getter or because the value is not a boolean.
18181818
template <typename T>
1819-
static bool ReadOptionalBool(Environment* env,
1820-
Local<Object> options,
1821-
const char* key,
1822-
T* out) {
1819+
[[nodiscard]] static bool ReadOptionalBool(Environment* env,
1820+
Local<Object> options,
1821+
const char* key,
1822+
T* out) {
18231823
Local<String> key_string =
18241824
OneByteString(env->isolate(), key, -1, NewStringType::kInternalized);
18251825
Local<Value> value;
@@ -1838,6 +1838,31 @@ static bool ReadOptionalBool(Environment* env,
18381838
return true;
18391839
}
18401840

1841+
// Reads options[key] into *out as a string, leaving *out unchanged if the value
1842+
// is undefined. Returns false if a JavaScript exception is pending, either from
1843+
// the property getter or because the value is not a string.
1844+
[[nodiscard]] static bool ReadOptionalString(Environment* env,
1845+
Local<Object> options,
1846+
const char* key,
1847+
std::string* out) {
1848+
Local<String> key_string =
1849+
OneByteString(env->isolate(), key, -1, NewStringType::kInternalized);
1850+
Local<Value> value;
1851+
if (!options->Get(env->context(), key_string).ToLocal(&value)) {
1852+
return false;
1853+
}
1854+
if (value->IsUndefined()) {
1855+
return true;
1856+
}
1857+
if (!value->IsString()) {
1858+
THROW_ERR_INVALID_ARG_TYPE(
1859+
env->isolate(), "The \"options.%s\" argument must be a string.", key);
1860+
return false;
1861+
}
1862+
*out = Utf8Value(env->isolate(), value.As<String>()).ToString();
1863+
return true;
1864+
}
1865+
18411866
std::optional<std::string> ValidateDatabasePath(Environment* env,
18421867
Local<Value> path,
18431868
const std::string& field_name) {
@@ -2409,22 +2434,7 @@ void Database::Deserialize(const FunctionCallbackInfo<Value>& args) {
24092434
}
24102435

24112436
Local<Object> options = args[1].As<Object>();
2412-
Local<String> db_name_key = FIXED_ONE_BYTE_STRING(env->isolate(), "dbName");
2413-
Local<Value> db_name_value;
2414-
if (!options->Get(env->context(), db_name_key).ToLocal(&db_name_value)) {
2415-
return;
2416-
}
2417-
2418-
if (!db_name_value->IsUndefined()) {
2419-
if (!db_name_value->IsString()) {
2420-
THROW_ERR_INVALID_ARG_TYPE(
2421-
env->isolate(),
2422-
"The \"options.dbName\" argument must be a string.");
2423-
return;
2424-
}
2425-
db_name =
2426-
Utf8Value(env->isolate(), db_name_value.As<String>()).ToString();
2427-
}
2437+
if (!ReadOptionalString(env, options, "dbName", &db_name)) return;
24282438
}
24292439

24302440
// Reading the options bag above can run user JavaScript through a property
@@ -2825,46 +2835,8 @@ void Database::CreateSession(const FunctionCallbackInfo<Value>& args) {
28252835
}
28262836

28272837
Local<Object> options = args[0].As<Object>();
2828-
2829-
Local<String> table_key = env->table_string();
2830-
bool hasIt;
2831-
if (!options->HasOwnProperty(env->context(), table_key).To(&hasIt)) {
2832-
return;
2833-
}
2834-
if (hasIt) {
2835-
Local<Value> table_value;
2836-
if (!options->Get(env->context(), table_key).ToLocal(&table_value)) {
2837-
return;
2838-
}
2839-
2840-
if (table_value->IsString()) {
2841-
table = Utf8Value(env->isolate(), table_value).ToString();
2842-
} else {
2843-
THROW_ERR_INVALID_ARG_TYPE(
2844-
env->isolate(), "The \"options.table\" argument must be a string.");
2845-
return;
2846-
}
2847-
}
2848-
2849-
Local<String> db_key = FIXED_ONE_BYTE_STRING(env->isolate(), "db");
2850-
2851-
if (!options->HasOwnProperty(env->context(), db_key).To(&hasIt)) {
2852-
return;
2853-
}
2854-
if (hasIt) {
2855-
Local<Value> db_value;
2856-
if (!options->Get(env->context(), db_key).ToLocal(&db_value)) {
2857-
// An error will have been scheduled.
2858-
return;
2859-
}
2860-
if (db_value->IsString()) {
2861-
db_name = Utf8Value(env->isolate(), db_value).ToString();
2862-
} else {
2863-
THROW_ERR_INVALID_ARG_TYPE(
2864-
env->isolate(), "The \"options.db\" argument must be a string.");
2865-
return;
2866-
}
2867-
}
2838+
if (!ReadOptionalString(env, options, "table", &table)) return;
2839+
if (!ReadOptionalString(env, options, "db", &db_name)) return;
28682840
}
28692841

28702842
// Reading the options bag above can run user JavaScript through a property
@@ -2942,39 +2914,8 @@ void Backup(const FunctionCallbackInfo<Value>& args) {
29422914
}
29432915
}
29442916

2945-
Local<Value> source_v;
2946-
if (!options->Get(env->context(), env->source_string())
2947-
.ToLocal(&source_v)) {
2948-
return;
2949-
}
2950-
2951-
if (!source_v->IsUndefined()) {
2952-
if (!source_v->IsString()) {
2953-
THROW_ERR_INVALID_ARG_TYPE(
2954-
env->isolate(),
2955-
"The \"options.source\" argument must be a string.");
2956-
return;
2957-
}
2958-
2959-
source_db = Utf8Value(env->isolate(), source_v.As<String>()).ToString();
2960-
}
2961-
2962-
Local<Value> target_v;
2963-
if (!options->Get(env->context(), env->target_string())
2964-
.ToLocal(&target_v)) {
2965-
return;
2966-
}
2967-
2968-
if (!target_v->IsUndefined()) {
2969-
if (!target_v->IsString()) {
2970-
THROW_ERR_INVALID_ARG_TYPE(
2971-
env->isolate(),
2972-
"The \"options.target\" argument must be a string.");
2973-
return;
2974-
}
2975-
2976-
dest_db = Utf8Value(env->isolate(), target_v.As<String>()).ToString();
2977-
}
2917+
if (!ReadOptionalString(env, options, "source", &source_db)) return;
2918+
if (!ReadOptionalString(env, options, "target", &dest_db)) return;
29782919

29792920
Local<Value> progress_v;
29802921
if (!options->Get(env->context(), env->progress_string())

‎test/parallel/test-sqlite-backup.mjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,20 @@ test('database backup', async (t) => {
195195
});
196196
});
197197

198+
test('backup treats undefined source and target as main', async (t) => {
199+
const database = makeSourceDb();
200+
const destDb = nextDb();
201+
202+
await backup(database, destDb, { source: undefined, target: undefined });
203+
204+
const backupDb = new Database(destDb);
205+
t.after(() => {
206+
database.close();
207+
backupDb.close();
208+
});
209+
t.assert.strictEqual(backupDb.prepare('SELECT count(*) AS n FROM main.data').get().n, 2);
210+
});
211+
198212
test('backup database using location as URL', async (t) => {
199213
const database = makeSourceDb();
200214
const destDb = pathToFileURL(nextDb());

‎test/parallel/test-sqlite-serialize.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,18 @@ suite('Database.prototype.deserialize()', () => {
251251
db.close();
252252
});
253253

254+
test('treats undefined options.dbName as main', (t) => {
255+
const db1 = new Database(':memory:');
256+
db1.exec('CREATE TABLE t(x INTEGER)');
257+
const buf = db1.serialize();
258+
db1.close();
259+
260+
const db2 = new Database(':memory:');
261+
db2.deserialize(buf, { dbName: undefined });
262+
t.assert.strictEqual(db2.prepare('SELECT count(*) AS n FROM main.t').get().n, 0);
263+
db2.close();
264+
});
265+
254266
test('accepts a Buffer as input', (t) => {
255267
const db1 = new Database(':memory:');
256268
db1.exec('CREATE TABLE t(x INTEGER)');

‎test/parallel/test-sqlite-session.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,14 @@ test('database.createSession() - specify other database', (t) => {
540540
t.assert.strictEqual(sessionTest.changeset().length, 0);
541541
});
542542

543+
test('database.createSession() - undefined options use defaults', (t) => {
544+
const database = new Database(':memory:');
545+
const session = database.createSession({ table: undefined, db: undefined });
546+
database.exec('CREATE TABLE data (key INTEGER PRIMARY KEY)');
547+
database.exec('INSERT INTO data (key) VALUES (1)');
548+
t.assert.notStrictEqual(session.changeset().length, 0);
549+
});
550+
543551
test('database.createSession() - wrong arguments', (t) => {
544552
const database = new Database(':memory:');
545553
t.assert.throws(() => {

0 commit comments

Comments
 (0)