Skip to content

Commit 9e288e9

Browse files
BurningEnlightenmentBurningEnlightenment
committed
sqlite: extract async-agnostic code into a common base
The database opening and configuration logic can be shared between the sync and async API variants, therefore extract the shared implementation into a common base class.
1 parent 5d64e3d commit 9e288e9

2 files changed

Lines changed: 75 additions & 43 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -255,16 +255,19 @@ void JSValueToSQLiteResult(Isolate* isolate,
255255
}
256256
}
257257

258+
inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, sqlite3* db) {
259+
Local<Object> e;
260+
if (CreateSQLiteError(isolate, db).ToLocal(&e)) {
261+
isolate->ThrowException(e);
262+
}
263+
}
258264
inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, DatabaseSync* db) {
259265
if (db->ShouldIgnoreSQLiteError()) {
260266
db->SetIgnoreNextSQLiteError(false);
261267
return;
262268
}
263269

264-
Local<Object> e;
265-
if (CreateSQLiteError(isolate, db->Connection()).ToLocal(&e)) {
266-
isolate->ThrowException(e);
267-
}
270+
THROW_ERR_SQLITE_ERROR(isolate, db->Connection());
268271
}
269272

270273
inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) {
@@ -299,6 +302,28 @@ inline MaybeLocal<Value> NullableSQLiteStringToValue(Isolate* isolate,
299302
.As<Value>();
300303
}
301304

305+
DatabaseCommon::DatabaseCommon(Environment* env,
306+
Local<Object> object,
307+
DatabaseOpenConfiguration&& open_config,
308+
bool allow_load_extension)
309+
: BaseObject(env, object),
310+
open_config_(std::move(open_config)),
311+
allow_load_extension_(allow_load_extension),
312+
enable_load_extension_(allow_load_extension) {
313+
MakeWeak();
314+
}
315+
316+
namespace {
317+
void AddDatabaseCommonMethodsToTemplate(Isolate* isolate,
318+
Local<FunctionTemplate> tmpl) {
319+
SetProtoMethod(isolate, tmpl, "open", DatabaseCommon::Open);
320+
SetSideEffectFreeGetter(isolate,
321+
tmpl,
322+
FIXED_ONE_BYTE_STRING(isolate, "isOpen"),
323+
DatabaseCommon::IsOpenGetter);
324+
}
325+
} // namespace
326+
302327
class CustomAggregate {
303328
public:
304329
explicit CustomAggregate(Environment* env,
@@ -875,11 +900,9 @@ DatabaseSync::DatabaseSync(Environment* env,
875900
DatabaseOpenConfiguration&& open_config,
876901
bool open,
877902
bool allow_load_extension)
878-
: BaseObject(env, object), open_config_(std::move(open_config)) {
903+
: DatabaseCommon(
904+
env, object, std::move(open_config), allow_load_extension) {
879905
MakeWeak();
880-
connection_ = nullptr;
881-
allow_load_extension_ = allow_load_extension;
882-
enable_load_extension_ = allow_load_extension;
883906
ignore_next_sqlite_error_ = false;
884907

885908
if (open) {
@@ -931,7 +954,8 @@ v8::Local<v8::FunctionTemplate> CreateDatabaseSyncConstructorTemplate(
931954
tmpl->InstanceTemplate()->SetInternalFieldCount(
932955
DatabaseSync::kInternalFieldCount);
933956

934-
SetProtoMethod(isolate, tmpl, "open", DatabaseSync::Open);
957+
AddDatabaseCommonMethodsToTemplate(isolate, tmpl);
958+
935959
SetProtoMethod(isolate, tmpl, "close", DatabaseSync::Close);
936960
SetProtoDispose(isolate, tmpl, DatabaseSync::Dispose);
937961
SetProtoMethod(isolate, tmpl, "prepare", DatabaseSync::Prepare);
@@ -948,10 +972,6 @@ v8::Local<v8::FunctionTemplate> CreateDatabaseSyncConstructorTemplate(
948972
isolate, tmpl, "enableDefensive", DatabaseSync::EnableDefensive);
949973
SetProtoMethod(isolate, tmpl, "loadExtension", DatabaseSync::LoadExtension);
950974
SetProtoMethod(isolate, tmpl, "setAuthorizer", DatabaseSync::SetAuthorizer);
951-
SetSideEffectFreeGetter(isolate,
952-
tmpl,
953-
FIXED_ONE_BYTE_STRING(isolate, "isOpen"),
954-
DatabaseSync::IsOpenGetter);
955975
SetSideEffectFreeGetter(isolate,
956976
tmpl,
957977
FIXED_ONE_BYTE_STRING(isolate, "isTransaction"),
@@ -971,7 +991,7 @@ v8::Local<v8::FunctionTemplate> CreateDatabaseSyncConstructorTemplate(
971991
}
972992
} // namespace
973993

974-
bool DatabaseSync::Open() {
994+
bool DatabaseCommon::Open() {
975995
if (IsOpen()) {
976996
THROW_ERR_INVALID_STATE(env(), "database is already open");
977997
return false;
@@ -986,34 +1006,34 @@ bool DatabaseSync::Open() {
9861006
&connection_,
9871007
flags | default_flags,
9881008
nullptr);
989-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1009+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
9901010

9911011
r = sqlite3_db_config(connection_,
9921012
SQLITE_DBCONFIG_DQS_DML,
9931013
static_cast<int>(open_config_.get_enable_dqs()),
9941014
nullptr);
995-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1015+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
9961016
r = sqlite3_db_config(connection_,
9971017
SQLITE_DBCONFIG_DQS_DDL,
9981018
static_cast<int>(open_config_.get_enable_dqs()),
9991019
nullptr);
1000-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1020+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
10011021

10021022
int foreign_keys_enabled;
10031023
r = sqlite3_db_config(
10041024
connection_,
10051025
SQLITE_DBCONFIG_ENABLE_FKEY,
10061026
static_cast<int>(open_config_.get_enable_foreign_keys()),
10071027
&foreign_keys_enabled);
1008-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1028+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
10091029
CHECK_EQ(foreign_keys_enabled, open_config_.get_enable_foreign_keys());
10101030

10111031
int defensive_enabled;
10121032
r = sqlite3_db_config(connection_,
10131033
SQLITE_DBCONFIG_DEFENSIVE,
10141034
static_cast<int>(open_config_.get_enable_defensive()),
10151035
&defensive_enabled);
1016-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1036+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
10171037
CHECK_EQ(defensive_enabled, open_config_.get_enable_defensive());
10181038

10191039
sqlite3_busy_timeout(connection_, open_config_.get_timeout());
@@ -1036,7 +1056,7 @@ bool DatabaseSync::Open() {
10361056
const int load_extension_ret = sqlite3_db_config(
10371057
connection_, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, nullptr);
10381058
CHECK_ERROR_OR_THROW(
1039-
env()->isolate(), this, load_extension_ret, SQLITE_OK, false);
1059+
env()->isolate(), connection_, load_extension_ret, SQLITE_OK, false);
10401060
}
10411061

10421062
return true;
@@ -1065,11 +1085,11 @@ void DatabaseSync::UntrackStatement(StatementSync* statement) {
10651085
}
10661086
}
10671087

1068-
inline bool DatabaseSync::IsOpen() {
1088+
inline bool DatabaseCommon::IsOpen() {
10691089
return connection_ != nullptr;
10701090
}
10711091

1072-
inline sqlite3* DatabaseSync::Connection() {
1092+
inline sqlite3* DatabaseCommon::Connection() {
10731093
return connection_;
10741094
}
10751095

@@ -1410,13 +1430,13 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
14101430
env, args.This(), std::move(open_config), open, allow_load_extension);
14111431
}
14121432

1413-
void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {
1433+
void DatabaseCommon::Open(const FunctionCallbackInfo<Value>& args) {
14141434
DatabaseSync* db;
14151435
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
14161436
db->Open();
14171437
}
14181438

1419-
void DatabaseSync::IsOpenGetter(const FunctionCallbackInfo<Value>& args) {
1439+
void DatabaseCommon::IsOpenGetter(const FunctionCallbackInfo<Value>& args) {
14201440
DatabaseSync* db;
14211441
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
14221442
args.GetReturnValue().Set(db->IsOpen());

‎src/node_sqlite.h‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,39 @@ class StatementExecutionHelper {
160160
bool use_big_ints);
161161
};
162162

163-
class DatabaseSync : public BaseObject {
163+
class DatabaseCommon : public BaseObject {
164+
public:
165+
DatabaseCommon(Environment* env,
166+
v8::Local<v8::Object> object,
167+
DatabaseOpenConfiguration&& open_config,
168+
bool allow_load_extension);
169+
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
170+
static void IsOpenGetter(const v8::FunctionCallbackInfo<v8::Value>& args);
171+
bool IsOpen();
172+
bool use_big_ints() const { return open_config_.get_use_big_ints(); }
173+
bool return_arrays() const { return open_config_.get_return_arrays(); }
174+
bool allow_bare_named_params() const {
175+
return open_config_.get_allow_bare_named_params();
176+
}
177+
bool allow_unknown_named_params() const {
178+
return open_config_.get_allow_unknown_named_params();
179+
}
180+
sqlite3* Connection();
181+
182+
protected:
183+
~DatabaseCommon() override = default;
184+
bool Open();
185+
186+
DatabaseOpenConfiguration open_config_;
187+
sqlite3* connection_ = nullptr;
188+
bool allow_load_extension_;
189+
bool enable_load_extension_;
190+
};
191+
192+
class DatabaseSync : public DatabaseCommon {
164193
public:
165194
enum InternalFields {
166-
kAuthorizerCallback = BaseObject::kInternalFieldCount,
195+
kAuthorizerCallback = DatabaseCommon::kInternalFieldCount,
167196
kLimitsObject,
168197
kInternalFieldCount
169198
};
@@ -175,8 +204,6 @@ class DatabaseSync : public BaseObject {
175204
bool allow_load_extension);
176205
void MemoryInfo(MemoryTracker* tracker) const override;
177206
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
178-
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
179-
static void IsOpenGetter(const v8::FunctionCallbackInfo<v8::Value>& args);
180207
static void IsTransactionGetter(
181208
const v8::FunctionCallbackInfo<v8::Value>& args);
182209
static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -207,16 +234,6 @@ class DatabaseSync : public BaseObject {
207234
void AddBackup(BackupJob* backup);
208235
void FinalizeBackups();
209236
void UntrackStatement(StatementSync* statement);
210-
bool IsOpen();
211-
bool use_big_ints() const { return open_config_.get_use_big_ints(); }
212-
bool return_arrays() const { return open_config_.get_return_arrays(); }
213-
bool allow_bare_named_params() const {
214-
return open_config_.get_allow_bare_named_params();
215-
}
216-
bool allow_unknown_named_params() const {
217-
return open_config_.get_allow_unknown_named_params();
218-
}
219-
sqlite3* Connection();
220237

221238
// In some situations, such as when using custom functions, it is possible
222239
// that SQLite reports an error while JavaScript already has a pending
@@ -229,14 +246,9 @@ class DatabaseSync : public BaseObject {
229246
SET_SELF_SIZE(DatabaseSync)
230247

231248
private:
232-
bool Open();
233249
void DeleteSessions();
234250

235251
~DatabaseSync() override;
236-
DatabaseOpenConfiguration open_config_;
237-
bool allow_load_extension_;
238-
bool enable_load_extension_;
239-
sqlite3* connection_;
240252
bool ignore_next_sqlite_error_;
241253

242254
std::set<BackupJob*> backups_;

0 commit comments

Comments
 (0)