From 88c7f5b489f5c4869a1d0337265cc174423e24d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Fri, 25 Oct 2024 17:22:07 -0700 Subject: [PATCH] sqlite: refactor open options Move options that are only relevant for opening the database into a self-contained class. PR-URL: https://github.com/nodejs/node/pull/55442 Reviewed-By: Benjamin Gruenbaum --- src/node_sqlite.cc | 52 +++++++++++++++++++++------------------------- src/node_sqlite.h | 33 ++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index f7025373f2c6a3..af9e77e1362bc3 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -94,17 +94,11 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) { DatabaseSync::DatabaseSync(Environment* env, Local object, - Local location, - bool open, - bool enable_foreign_keys_on_open, - bool enable_dqs_on_open) - : BaseObject(env, object) { + DatabaseOpenConfiguration&& open_config, + bool open) + : BaseObject(env, object), open_config_(std::move(open_config)) { MakeWeak(); - node::Utf8Value utf8_location(env->isolate(), location); - location_ = utf8_location.ToString(); connection_ = nullptr; - enable_foreign_keys_on_open_ = enable_foreign_keys_on_open; - enable_dqs_on_open_ = enable_dqs_on_open; if (open) { Open(); @@ -120,7 +114,9 @@ DatabaseSync::~DatabaseSync() { } void DatabaseSync::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("location", location_); + // TODO(tniessen): more accurately track the size of all fields + tracker->TrackFieldWithSize( + "open_config", sizeof(open_config_), "DatabaseOpenConfiguration"); } bool DatabaseSync::Open() { @@ -131,27 +127,29 @@ bool DatabaseSync::Open() { // TODO(cjihrig): Support additional flags. int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE; - int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, nullptr); + int r = sqlite3_open_v2( + open_config_.location().c_str(), &connection_, flags, nullptr); CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); r = sqlite3_db_config(connection_, SQLITE_DBCONFIG_DQS_DML, - static_cast(enable_dqs_on_open_), + static_cast(open_config_.get_enable_dqs()), nullptr); CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); r = sqlite3_db_config(connection_, SQLITE_DBCONFIG_DQS_DDL, - static_cast(enable_dqs_on_open_), + static_cast(open_config_.get_enable_dqs()), nullptr); CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); int foreign_keys_enabled; - r = sqlite3_db_config(connection_, - SQLITE_DBCONFIG_ENABLE_FKEY, - static_cast(enable_foreign_keys_on_open_), - &foreign_keys_enabled); + r = sqlite3_db_config( + connection_, + SQLITE_DBCONFIG_ENABLE_FKEY, + static_cast(open_config_.get_enable_foreign_keys()), + &foreign_keys_enabled); CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); - CHECK_EQ(foreign_keys_enabled, enable_foreign_keys_on_open_); + CHECK_EQ(foreign_keys_enabled, open_config_.get_enable_foreign_keys()); return true; } @@ -193,9 +191,11 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { return; } + std::string location = + node::Utf8Value(env->isolate(), args[0].As()).ToString(); + DatabaseOpenConfiguration open_config(std::move(location)); + bool open = true; - bool enable_foreign_keys = true; - bool enable_dqs = false; if (args.Length() > 1) { if (!args[1]->IsObject()) { @@ -234,7 +234,8 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { "boolean."); return; } - enable_foreign_keys = enable_foreign_keys_v.As()->Value(); + open_config.set_enable_foreign_keys( + enable_foreign_keys_v.As()->Value()); } Local enable_dqs_string = FIXED_ONE_BYTE_STRING( @@ -252,16 +253,11 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { "a boolean."); return; } - enable_dqs = enable_dqs_v.As()->Value(); + open_config.set_enable_dqs(enable_dqs_v.As()->Value()); } } - new DatabaseSync(env, - args.This(), - args[0].As(), - open, - enable_foreign_keys, - enable_dqs); + new DatabaseSync(env, args.This(), std::move(open_config), open); } void DatabaseSync::Open(const FunctionCallbackInfo& args) { diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 40abf7b0d3edc3..933d0b65a3ba08 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -14,16 +14,37 @@ namespace node { namespace sqlite { +class DatabaseOpenConfiguration { + public: + explicit DatabaseOpenConfiguration(std::string&& location) + : location_(std::move(location)) {} + + inline const std::string& location() const { return location_; } + + inline bool get_enable_foreign_keys() const { return enable_foreign_keys_; } + + inline void set_enable_foreign_keys(bool flag) { + enable_foreign_keys_ = flag; + } + + inline bool get_enable_dqs() const { return enable_dqs_; } + + inline void set_enable_dqs(bool flag) { enable_dqs_ = flag; } + + private: + std::string location_; + bool enable_foreign_keys_ = true; + bool enable_dqs_ = false; +}; + class StatementSync; class DatabaseSync : public BaseObject { public: DatabaseSync(Environment* env, v8::Local object, - v8::Local location, - bool open, - bool enable_foreign_keys_on_open, - bool enable_dqs_on_open); + DatabaseOpenConfiguration&& open_config, + bool open); void MemoryInfo(MemoryTracker* tracker) const override; static void New(const v8::FunctionCallbackInfo& args); static void Open(const v8::FunctionCallbackInfo& args); @@ -42,11 +63,9 @@ class DatabaseSync : public BaseObject { bool Open(); ~DatabaseSync() override; - std::string location_; + DatabaseOpenConfiguration open_config_; sqlite3* connection_; std::unordered_set statements_; - bool enable_foreign_keys_on_open_; - bool enable_dqs_on_open_; }; class StatementSync : public BaseObject {