sqlite: pass object to progress function instead of two args

nodejs · Jan 14, 2025 · a050cd6 · a050cd6
1 parent 5ad3f48
commit a050cd6
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 8 deletions.
diff --git a/src/env_properties.h b/src/env_properties.h
@@ -73,6 +73,8 @@
   V(address_string, "address")                                                 \
   V(source_db_string, "sourceDb")                                              \
   V(target_db_string, "targetDb")                                              \
+  V(total_pages_string, "totalPages")                                          \
+  V(remaining_pages_string, "remainingPages")                                  \
   V(progress_string, "progress")                                               \
   V(aliases_string, "aliases")                                                 \
   V(alpn_callback_string, "ALPNCallback")                                      \

diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
@@ -247,13 +247,25 @@ class BackupJob : public ThreadPoolWork {
           Local<Function>::New(env()->isolate(), progressFunc_);
 
       if (!fn.IsEmpty()) {
-        Local<Value> argv[] = {
-            Integer::New(env()->isolate(), total_pages),
-            Integer::New(env()->isolate(), remaining_pages),
-        };
+        Local<Object> progress_info = Object::New(env()->isolate());
+
+        if (progress_info
+                ->Set(env()->context(),
+                      env()->total_pages_string(),
+                      Integer::New(env()->isolate(), total_pages))
+                .IsNothing() ||
+            progress_info
+                ->Set(env()->context(),
+                      env()->remaining_pages_string(),
+                      Integer::New(env()->isolate(), remaining_pages))
+                .IsNothing()) {
+          return;
+        }
+
+        Local<Value> argv[] = {progress_info};
 
         TryCatch try_catch(env()->isolate());
-        fn->Call(env()->context(), Null(env()->isolate()), 2, argv)
+        fn->Call(env()->context(), Null(env()->isolate()), 1, argv)
             .FromMaybe(Local<Value>());
 
         if (try_catch.HasCaught()) {
@@ -742,7 +754,7 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
   CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
 }
 
-// database.backup(destination, { sourceDb, targetDb, rate, progress: (total,
+// database.backup(path, { sourceDb, targetDb, rate, progress: (total,
 // remaining) => {} )
 void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
@@ -762,7 +774,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
 
   THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
 
-  Utf8Value destFilename(env->isolate(), args[0].As<String>());
+  Utf8Value destPath(env->isolate(), args[0].As<String>());
   Local<Function> progressFunc = Local<Function>();
 
   if (args.Length() > 1) {
@@ -852,7 +864,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(resolver->GetPromise());
 
   BackupJob* job = new BackupJob(
-      env, db, resolver, source_db, *destFilename, dest_db, rate, progressFunc);
+      env, db, resolver, source_db, *destPath, dest_db, rate, progressFunc);
   db->backups_.insert(job);
   job->ScheduleBackup();
 }

diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
@@ -127,6 +127,27 @@ test('database backup', async (t) => {
   // so the progress function should be called once (the last call is not made since
   // the promise resolves)
   t.assert.strictEqual(progressFn.mock.calls.length, 1);
+  t.assert.deepStrictEqual(progressFn.mock.calls[0].arguments, [{ totalPages: 2, remainingPages: 1 }]);
+  t.assert.deepStrictEqual(rows, [
+    { __proto__: null, key: 1, value: 'value-1' },
+    { __proto__: null, key: 2, value: 'value-2' },
+  ]);
+});
+
+test('database backup in a single call', async (t) => {
+  const progressFn = t.mock.fn();
+  const database = makeSourceDb();
+  const destDb = nextDb();
+
+  // Let rate to be default (100) to backup in a single call
+  await database.backup(destDb, {
+    progress: progressFn,
+  });
+
+  const backup = new DatabaseSync(destDb);
+  const rows = backup.prepare('SELECT * FROM data').all();
+
+  t.assert.strictEqual(progressFn.mock.calls.length, 0);
   t.assert.deepStrictEqual(rows, [
     { __proto__: null, key: 1, value: 'value-1' },
     { __proto__: null, key: 2, value: 'value-2' },