From 0909d3e138cbe0a49b24e40257527f815f03f8f1 Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Fri, 24 Jan 2025 12:09:33 -0300
Subject: [PATCH] sqlite,test,doc: move backup implementation to a separate
 function

---
 doc/api/sqlite.md                    |  41 ++---
 src/env_properties.h                 |   1 +
 src/node_sqlite.cc                   | 226 ++++++++++++++-------------
 src/node_sqlite.h                    |   1 -
 test/parallel/test-sqlite-backup.mjs |  51 +++---
 5 files changed, 171 insertions(+), 149 deletions(-)

diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md
index 08fea8d233aed2..742b7665e44ce4 100644
--- a/doc/api/sqlite.md
+++ b/doc/api/sqlite.md
@@ -115,26 +115,6 @@ added: v22.5.0
 
 Constructs a new `DatabaseSync` instance.
 
-### `database.backup(destination[, options])`
-
-<!-- YAML
-added: REPLACEME
--->
-
-* `destination` {string} The path where the backup will be created. If the file already exists, the contents will be
-  overwritten.
-* `options` {Object} Optional configuration for the backup. The
-  following properties are supported:
-  * `source` {string} Name of the source database. **Default:** `'main'`.
-  * `target` {string} Name of the target database. **Default:** `'main'`.
-  * `rate` {number} Number of pages to be transmitted in each batch of the backup. **Default:** `100`.
-  * `progress` {Function} Callback function that will be called with the number of pages copied and the total number of
-    pages.
-* Returns: {Promise} A promise that resolves when the backup is completed and rejects if an error occurs.
-
-This method makes a database backup. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][]
-and [`sqlite3_backup_finish()`][] functions.
-
 ### `database.close()`
 
 <!-- YAML
@@ -543,6 +523,27 @@ exception.
 | `TEXT`    | {string}             |
 | `BLOB`    | {Uint8Array}         |
 
+## `sqlite.backup(sourceDb, destination[, options])`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `sourceDb` A `DatabaseSync` object to be the source of the backup. The source database must be open.
+* `destination` {string} The path where the backup will be created. If the file already exists, the contents will be
+  overwritten.
+* `options` {Object} Optional configuration for the backup. The
+  following properties are supported:
+  * `source` {string} Name of the source database. **Default:** `'main'`.
+  * `target` {string} Name of the target database. **Default:** `'main'`.
+  * `rate` {number} Number of pages to be transmitted in each batch of the backup. **Default:** `100`.
+  * `progress` {Function} Callback function that will be called with the number of pages copied and the total number of
+    pages.
+* Returns: {Promise} A promise that resolves when the backup is completed and rejects if an error occurs.
+
+This method makes a database backup. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][]
+and [`sqlite3_backup_finish()`][] functions.
+
 ## `sqlite.constants`
 
 <!-- YAML
diff --git a/src/env_properties.h b/src/env_properties.h
index ebdf3e81759d6f..b383d75825495c 100644
--- a/src/env_properties.h
+++ b/src/env_properties.h
@@ -77,6 +77,7 @@
   V(asn1curve_string, "asn1Curve")                                             \
   V(async_ids_stack_string, "async_ids_stack")                                 \
   V(attributes_string, "attributes")                                           \
+  V(backup_string, "backup")                                                   \
   V(base_string, "base")                                                       \
   V(bits_string, "bits")                                                       \
   V(block_list_string, "blockList")                                            \
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index 85276c115b9c3b..3f8a70f4958d15 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -725,112 +725,6 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
   CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
 }
 
-void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
-  DatabaseSync* db;
-  ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
-  Environment* env = Environment::GetCurrent(args);
-  THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
-
-  if (!args[0]->IsString()) {
-    THROW_ERR_INVALID_ARG_TYPE(
-        env->isolate(), "The \"destination\" argument must be a string.");
-    return;
-  }
-
-  int rate = 100;
-  std::string source_db = "main";
-  std::string dest_db = "main";
-
-  Utf8Value dest_path(env->isolate(), args[0].As<String>());
-  Local<Function> progressFunc = Local<Function>();
-
-  if (args.Length() > 1) {
-    if (!args[1]->IsObject()) {
-      THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
-                                 "The \"options\" argument must be an object.");
-      return;
-    }
-
-    Local<Object> options = args[1].As<Object>();
-    Local<Value> rate_v;
-    if (!options->Get(env->context(), env->rate_string()).ToLocal(&rate_v)) {
-      return;
-    }
-
-    if (!rate_v->IsUndefined()) {
-      if (!rate_v->IsInt32()) {
-        THROW_ERR_INVALID_ARG_TYPE(
-            env->isolate(),
-            "The \"options.rate\" argument must be an integer.");
-        return;
-      }
-      rate = rate_v.As<Int32>()->Value();
-    }
-
-    Local<Value> source_v;
-    if (!options->Get(env->context(), env->source_string())
-             .ToLocal(&source_v)) {
-      return;
-    }
-
-    if (!source_v->IsUndefined()) {
-      if (!source_v->IsString()) {
-        THROW_ERR_INVALID_ARG_TYPE(
-            env->isolate(),
-            "The \"options.source\" argument must be a string.");
-        return;
-      }
-
-      source_db = Utf8Value(env->isolate(), source_v.As<String>()).ToString();
-    }
-
-    Local<Value> target_v;
-    if (!options->Get(env->context(), env->target_string())
-             .ToLocal(&target_v)) {
-      return;
-    }
-
-    if (!target_v->IsUndefined()) {
-      if (!target_v->IsString()) {
-        THROW_ERR_INVALID_ARG_TYPE(
-            env->isolate(),
-            "The \"options.target\" argument must be a string.");
-        return;
-      }
-
-      dest_db = Utf8Value(env->isolate(), target_v.As<String>()).ToString();
-    }
-
-    Local<Value> progress_v;
-    if (!options->Get(env->context(), env->progress_string())
-             .ToLocal(&progress_v)) {
-      return;
-    }
-
-    if (!progress_v->IsUndefined()) {
-      if (!progress_v->IsFunction()) {
-        THROW_ERR_INVALID_ARG_TYPE(
-            env->isolate(),
-            "The \"options.progress\" argument must be a function.");
-        return;
-      }
-      progressFunc = progress_v.As<Function>();
-    }
-  }
-
-  Local<Promise::Resolver> resolver;
-  if (!Promise::Resolver::New(env->context()).ToLocal(&resolver)) {
-    return;
-  }
-
-  args.GetReturnValue().Set(resolver->GetPromise());
-
-  BackupJob* job = new BackupJob(
-      env, db, resolver, source_db, *dest_path, dest_db, rate, progressFunc);
-  db->backups_.insert(job);
-  job->ScheduleBackup();
-}
-
 void DatabaseSync::CustomFunction(const FunctionCallbackInfo<Value>& args) {
   DatabaseSync* db;
   ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
@@ -1041,6 +935,117 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(session->object());
 }
 
+void Backup(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  if (args.Length() < 1 || !args[0]->IsObject()) {
+    THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
+                               "The \"sourceDb\" argument must be an object.");
+    return;
+  }
+
+  DatabaseSync* db;
+  ASSIGN_OR_RETURN_UNWRAP(&db, args[0].As<Object>());
+  THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
+  if (!args[1]->IsString()) {
+    THROW_ERR_INVALID_ARG_TYPE(
+        env->isolate(), "The \"destination\" argument must be a string.");
+    return;
+  }
+
+  int rate = 100;
+  std::string source_db = "main";
+  std::string dest_db = "main";
+
+  Utf8Value dest_path(env->isolate(), args[1].As<String>());
+  Local<Function> progressFunc = Local<Function>();
+
+  if (args.Length() > 2) {
+    if (!args[2]->IsObject()) {
+      THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
+                                 "The \"options\" argument must be an object.");
+      return;
+    }
+
+    Local<Object> options = args[2].As<Object>();
+    Local<Value> rate_v;
+    if (!options->Get(env->context(), env->rate_string()).ToLocal(&rate_v)) {
+      return;
+    }
+
+    if (!rate_v->IsUndefined()) {
+      if (!rate_v->IsInt32()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.rate\" argument must be an integer.");
+        return;
+      }
+      rate = rate_v.As<Int32>()->Value();
+    }
+
+    Local<Value> source_v;
+    if (!options->Get(env->context(), env->source_string())
+             .ToLocal(&source_v)) {
+      return;
+    }
+
+    if (!source_v->IsUndefined()) {
+      if (!source_v->IsString()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.source\" argument must be a string.");
+        return;
+      }
+
+      source_db = Utf8Value(env->isolate(), source_v.As<String>()).ToString();
+    }
+
+    Local<Value> target_v;
+    if (!options->Get(env->context(), env->target_string())
+             .ToLocal(&target_v)) {
+      return;
+    }
+
+    if (!target_v->IsUndefined()) {
+      if (!target_v->IsString()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.target\" argument must be a string.");
+        return;
+      }
+
+      dest_db = Utf8Value(env->isolate(), target_v.As<String>()).ToString();
+    }
+
+    Local<Value> progress_v;
+    if (!options->Get(env->context(), env->progress_string())
+             .ToLocal(&progress_v)) {
+      return;
+    }
+
+    if (!progress_v->IsUndefined()) {
+      if (!progress_v->IsFunction()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.progress\" argument must be a function.");
+        return;
+      }
+      progressFunc = progress_v.As<Function>();
+    }
+  }
+
+  Local<Promise::Resolver> resolver;
+  if (!Promise::Resolver::New(env->context()).ToLocal(&resolver)) {
+    return;
+  }
+
+  args.GetReturnValue().Set(resolver->GetPromise());
+
+  BackupJob* job = new BackupJob(
+      env, db, resolver, source_db, *dest_path, dest_db, rate, progressFunc);
+  db->backups_.insert(job);
+  job->ScheduleBackup();
+}
+
 // the reason for using static functions here is that SQLite needs a
 // function pointer
 static std::function<int(int)> conflictCallback;
@@ -2016,7 +2021,6 @@ static void Initialize(Local<Object> target,
   SetProtoMethod(isolate, db_tmpl, "close", DatabaseSync::Close);
   SetProtoMethod(isolate, db_tmpl, "prepare", DatabaseSync::Prepare);
   SetProtoMethod(isolate, db_tmpl, "exec", DatabaseSync::Exec);
-  SetProtoMethod(isolate, db_tmpl, "backup", DatabaseSync::Backup);
   SetProtoMethod(isolate, db_tmpl, "function", DatabaseSync::CustomFunction);
   SetProtoMethod(
       isolate, db_tmpl, "createSession", DatabaseSync::CreateSession);
@@ -2035,6 +2039,14 @@ static void Initialize(Local<Object> target,
                          StatementSync::GetConstructorTemplate(env));
 
   target->Set(context, env->constants_string(), constants).Check();
+
+  Local<Function> backup_function;
+
+  if (!Function::New(context, Backup).ToLocal(&backup_function)) {
+    return;
+  }
+
+  target->Set(context, env->backup_string(), backup_function).Check();
 }
 
 }  // namespace sqlite
diff --git a/src/node_sqlite.h b/src/node_sqlite.h
index 05fe0bdb143d99..98deee02500eed 100644
--- a/src/node_sqlite.h
+++ b/src/node_sqlite.h
@@ -58,7 +58,6 @@ class DatabaseSync : public BaseObject {
   static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Prepare(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Exec(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void Backup(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void CustomFunction(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void CreateSession(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void ApplyChangeset(const v8::FunctionCallbackInfo<v8::Value>& args);
diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
index c4390a895f74f5..0dce49365a65a1 100644
--- a/test/parallel/test-sqlite-backup.mjs
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -1,7 +1,7 @@
 import '../common/index.mjs';
 import tmpdir from '../common/tmpdir.js';
 import { join } from 'node:path';
-import { DatabaseSync } from 'node:sqlite';
+import { backup, DatabaseSync } from 'node:sqlite';
 import { describe, test } from 'node:test';
 import { writeFileSync } from 'node:fs';
 
@@ -32,19 +32,28 @@ function makeSourceDb() {
   return database;
 }
 
-describe('DatabaseSync.prototype.backup()', () => {
-  test('throws if path is not a string', async (t) => {
+describe('backup()', () => {
+  test('throws if the source database is not provided', (t) => {
+    t.assert.throws(() => {
+      backup();
+    }, {
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "sourceDb" argument must be an object.'
+    });
+  });
+
+  test('throws if path is not a string', (t) => {
     const database = makeSourceDb();
 
     t.assert.throws(() => {
-      database.backup();
+      backup(database);
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "destination" argument must be a string.'
     });
 
     t.assert.throws(() => {
-      database.backup({});
+      backup(database, {});
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "destination" argument must be a string.'
@@ -55,7 +64,7 @@ describe('DatabaseSync.prototype.backup()', () => {
     const database = makeSourceDb();
 
     t.assert.throws(() => {
-      database.backup('hello.db', 'invalid');
+      backup(database, 'hello.db', 'invalid');
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "options" argument must be an object.'
@@ -66,7 +75,7 @@ describe('DatabaseSync.prototype.backup()', () => {
     const database = makeSourceDb();
 
     t.assert.throws(() => {
-      database.backup('hello.db', {
+      backup(database, 'hello.db', {
         source: 42
       });
     }, {
@@ -75,7 +84,7 @@ describe('DatabaseSync.prototype.backup()', () => {
     });
 
     t.assert.throws(() => {
-      database.backup('hello.db', {
+      backup(database, 'hello.db', {
         target: 42
       });
     }, {
@@ -84,7 +93,7 @@ describe('DatabaseSync.prototype.backup()', () => {
     });
 
     t.assert.throws(() => {
-      database.backup('hello.db', {
+      backup(database, 'hello.db', {
         rate: 'invalid'
       });
     }, {
@@ -93,7 +102,7 @@ describe('DatabaseSync.prototype.backup()', () => {
     });
 
     t.assert.throws(() => {
-      database.backup('hello.db', {
+      backup(database, 'hello.db', {
         progress: 'invalid'
       });
     }, {
@@ -108,13 +117,13 @@ test('database backup', async (t) => {
   const database = makeSourceDb();
   const destDb = nextDb();
 
-  await database.backup(destDb, {
+  await backup(database, destDb, {
     rate: 1,
     progress: progressFn,
   });
 
-  const backup = new DatabaseSync(destDb);
-  const rows = backup.prepare('SELECT * FROM data').all();
+  const backupDb = new DatabaseSync(destDb);
+  const rows = backupDb.prepare('SELECT * FROM data').all();
 
   // The source database has two pages - using the default page size -,
   // so the progress function should be called once (the last call is not made since
@@ -133,12 +142,12 @@ test('database backup in a single call', async (t) => {
   const destDb = nextDb();
 
   // Let rate to be default (100) to backup in a single call
-  await database.backup(destDb, {
+  await backup(database, destDb, {
     progress: progressFn,
   });
 
-  const backup = new DatabaseSync(destDb);
-  const rows = backup.prepare('SELECT * FROM data').all();
+  const backupDb = new DatabaseSync(destDb);
+  const rows = backupDb.prepare('SELECT * FROM data').all();
 
   t.assert.strictEqual(progressFn.mock.calls.length, 0);
   t.assert.deepStrictEqual(rows, [
@@ -153,7 +162,7 @@ test('throws exception when trying to start backup from a closed database', asyn
 
     database.close();
 
-    database.backup('backup.db');
+    backup(database, 'backup.db');
   }, {
     code: 'ERR_INVALID_STATE',
     message: 'database is not open'
@@ -167,7 +176,7 @@ test('database backup fails when dest file is not writable', async (t) => {
   const database = makeSourceDb();
 
   await t.assert.rejects(async () => {
-    await database.backup(readonlyDestDb);
+    await backup(database, readonlyDestDb);
   }, {
     code: 'ERR_SQLITE_ERROR',
     message: 'attempt to write a readonly database'
@@ -183,7 +192,7 @@ test('backup fails when progress function throws', async (t) => {
   });
 
   await t.assert.rejects(async () => {
-    await database.backup(destDb, {
+    await backup(database, destDb, {
       rate: 1,
       progress: progressFn,
     });
@@ -197,7 +206,7 @@ test('backup fails when source db is invalid', async (t) => {
   const destDb = nextDb();
 
   await t.assert.rejects(async () => {
-    await database.backup(destDb, {
+    await backup(database, destDb, {
       rate: 1,
       source: 'invalid',
     });
@@ -210,7 +219,7 @@ test('backup fails when destination cannot be opened', async (t) => {
   const database = makeSourceDb();
 
   await t.assert.rejects(async () => {
-    await database.backup(`${tmpdir.path}/invalid/backup.db`);
+    await backup(database, `${tmpdir.path}/invalid/backup.db`);
   }, {
     message: 'unable to open database file'
   });