Skip to content

Commit 088f4f8

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 4652345 commit 088f4f8

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
@@ -237,16 +237,19 @@ void JSValueToSQLiteResult(Isolate* isolate,
237237
}
238238
}
239239

240+
inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, sqlite3* db) {
241+
Local<Object> e;
242+
if (CreateSQLiteError(isolate, db).ToLocal(&e)) {
243+
isolate->ThrowException(e);
244+
}
245+
}
240246
inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, DatabaseSync* db) {
241247
if (db->ShouldIgnoreSQLiteError()) {
242248
db->SetIgnoreNextSQLiteError(false);
243249
return;
244250
}
245251

246-
Local<Object> e;
247-
if (CreateSQLiteError(isolate, db->Connection()).ToLocal(&e)) {
248-
isolate->ThrowException(e);
249-
}
252+
THROW_ERR_SQLITE_ERROR(isolate, db->Connection());
250253
}
251254

252255
inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) {
@@ -281,6 +284,28 @@ inline MaybeLocal<Value> NullableSQLiteStringToValue(Isolate* isolate,
281284
.As<Value>();
282285
}
283286

287+
DatabaseCommon::DatabaseCommon(Environment* env,
288+
Local<Object> object,
289+
DatabaseOpenConfiguration&& open_config,
290+
bool allow_load_extension)
291+
: BaseObject(env, object),
292+
open_config_(std::move(open_config)),
293+
allow_load_extension_(allow_load_extension),
294+
enable_load_extension_(allow_load_extension) {
295+
MakeWeak();
296+
}
297+
298+
namespace {
299+
void AddDatabaseCommonMethodsToTemplate(Isolate* isolate,
300+
Local<FunctionTemplate> tmpl) {
301+
SetProtoMethod(isolate, tmpl, "open", DatabaseCommon::Open);
302+
SetSideEffectFreeGetter(isolate,
303+
tmpl,
304+
FIXED_ONE_BYTE_STRING(isolate, "isOpen"),
305+
DatabaseCommon::IsOpenGetter);
306+
}
307+
} // namespace
308+
284309
class CustomAggregate {
285310
public:
286311
explicit CustomAggregate(Environment* env,
@@ -688,11 +713,9 @@ DatabaseSync::DatabaseSync(Environment* env,
688713
DatabaseOpenConfiguration&& open_config,
689714
bool open,
690715
bool allow_load_extension)
691-
: BaseObject(env, object), open_config_(std::move(open_config)) {
716+
: DatabaseCommon(
717+
env, object, std::move(open_config), allow_load_extension) {
692718
MakeWeak();
693-
connection_ = nullptr;
694-
allow_load_extension_ = allow_load_extension;
695-
enable_load_extension_ = allow_load_extension;
696719
ignore_next_sqlite_error_ = false;
697720

698721
if (open) {
@@ -744,7 +767,8 @@ v8::Local<v8::FunctionTemplate> CreateDatabaseSyncConstructorTemplate(
744767
tmpl->InstanceTemplate()->SetInternalFieldCount(
745768
DatabaseSync::kInternalFieldCount);
746769

747-
SetProtoMethod(isolate, tmpl, "open", DatabaseSync::Open);
770+
AddDatabaseCommonMethodsToTemplate(isolate, tmpl);
771+
748772
SetProtoMethod(isolate, tmpl, "close", DatabaseSync::Close);
749773
SetProtoDispose(isolate, tmpl, DatabaseSync::Dispose);
750774
SetProtoMethod(isolate, tmpl, "prepare", DatabaseSync::Prepare);
@@ -761,10 +785,6 @@ v8::Local<v8::FunctionTemplate> CreateDatabaseSyncConstructorTemplate(
761785
isolate, tmpl, "enableDefensive", DatabaseSync::EnableDefensive);
762786
SetProtoMethod(isolate, tmpl, "loadExtension", DatabaseSync::LoadExtension);
763787
SetProtoMethod(isolate, tmpl, "setAuthorizer", DatabaseSync::SetAuthorizer);
764-
SetSideEffectFreeGetter(isolate,
765-
tmpl,
766-
FIXED_ONE_BYTE_STRING(isolate, "isOpen"),
767-
DatabaseSync::IsOpenGetter);
768788
SetSideEffectFreeGetter(isolate,
769789
tmpl,
770790
FIXED_ONE_BYTE_STRING(isolate, "isTransaction"),
@@ -780,7 +800,7 @@ v8::Local<v8::FunctionTemplate> CreateDatabaseSyncConstructorTemplate(
780800
}
781801
} // namespace
782802

783-
bool DatabaseSync::Open() {
803+
bool DatabaseCommon::Open() {
784804
if (IsOpen()) {
785805
THROW_ERR_INVALID_STATE(env(), "database is already open");
786806
return false;
@@ -795,34 +815,34 @@ bool DatabaseSync::Open() {
795815
&connection_,
796816
flags | default_flags,
797817
nullptr);
798-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
818+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
799819

800820
r = sqlite3_db_config(connection_,
801821
SQLITE_DBCONFIG_DQS_DML,
802822
static_cast<int>(open_config_.get_enable_dqs()),
803823
nullptr);
804-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
824+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
805825
r = sqlite3_db_config(connection_,
806826
SQLITE_DBCONFIG_DQS_DDL,
807827
static_cast<int>(open_config_.get_enable_dqs()),
808828
nullptr);
809-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
829+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
810830

811831
int foreign_keys_enabled;
812832
r = sqlite3_db_config(
813833
connection_,
814834
SQLITE_DBCONFIG_ENABLE_FKEY,
815835
static_cast<int>(open_config_.get_enable_foreign_keys()),
816836
&foreign_keys_enabled);
817-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
837+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
818838
CHECK_EQ(foreign_keys_enabled, open_config_.get_enable_foreign_keys());
819839

820840
int defensive_enabled;
821841
r = sqlite3_db_config(connection_,
822842
SQLITE_DBCONFIG_DEFENSIVE,
823843
static_cast<int>(open_config_.get_enable_defensive()),
824844
&defensive_enabled);
825-
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
845+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
826846
CHECK_EQ(defensive_enabled, open_config_.get_enable_defensive());
827847

828848
sqlite3_busy_timeout(connection_, open_config_.get_timeout());
@@ -837,7 +857,7 @@ bool DatabaseSync::Open() {
837857
const int load_extension_ret = sqlite3_db_config(
838858
connection_, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, nullptr);
839859
CHECK_ERROR_OR_THROW(
840-
env()->isolate(), this, load_extension_ret, SQLITE_OK, false);
860+
env()->isolate(), connection_, load_extension_ret, SQLITE_OK, false);
841861
}
842862

843863
return true;
@@ -866,11 +886,11 @@ void DatabaseSync::UntrackStatement(StatementSync* statement) {
866886
}
867887
}
868888

869-
inline bool DatabaseSync::IsOpen() {
889+
inline bool DatabaseCommon::IsOpen() {
870890
return connection_ != nullptr;
871891
}
872892

873-
inline sqlite3* DatabaseSync::Connection() {
893+
inline sqlite3* DatabaseCommon::Connection() {
874894
return connection_;
875895
}
876896

@@ -1158,13 +1178,13 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
11581178
env, args.This(), std::move(open_config), open, allow_load_extension);
11591179
}
11601180

1161-
void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {
1181+
void DatabaseCommon::Open(const FunctionCallbackInfo<Value>& args) {
11621182
DatabaseSync* db;
11631183
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
11641184
db->Open();
11651185
}
11661186

1167-
void DatabaseSync::IsOpenGetter(const FunctionCallbackInfo<Value>& args) {
1187+
void DatabaseCommon::IsOpenGetter(const FunctionCallbackInfo<Value>& args) {
11681188
DatabaseSync* db;
11691189
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
11701190
args.GetReturnValue().Set(db->IsOpen());

‎src/node_sqlite.h‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,39 @@ class StatementExecutionHelper {
114114
bool use_big_ints);
115115
};
116116

117-
class DatabaseSync : public BaseObject {
117+
class DatabaseCommon : public BaseObject {
118+
public:
119+
DatabaseCommon(Environment* env,
120+
v8::Local<v8::Object> object,
121+
DatabaseOpenConfiguration&& open_config,
122+
bool allow_load_extension);
123+
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
124+
static void IsOpenGetter(const v8::FunctionCallbackInfo<v8::Value>& args);
125+
bool IsOpen();
126+
bool use_big_ints() const { return open_config_.get_use_big_ints(); }
127+
bool return_arrays() const { return open_config_.get_return_arrays(); }
128+
bool allow_bare_named_params() const {
129+
return open_config_.get_allow_bare_named_params();
130+
}
131+
bool allow_unknown_named_params() const {
132+
return open_config_.get_allow_unknown_named_params();
133+
}
134+
sqlite3* Connection();
135+
136+
protected:
137+
~DatabaseCommon() override = default;
138+
bool Open();
139+
140+
DatabaseOpenConfiguration open_config_;
141+
sqlite3* connection_ = nullptr;
142+
bool allow_load_extension_;
143+
bool enable_load_extension_;
144+
};
145+
146+
class DatabaseSync : public DatabaseCommon {
118147
public:
119148
enum InternalFields {
120-
kAuthorizerCallback = BaseObject::kInternalFieldCount,
149+
kAuthorizerCallback = DatabaseCommon::kInternalFieldCount,
121150
kInternalFieldCount
122151
};
123152

@@ -128,8 +157,6 @@ class DatabaseSync : public BaseObject {
128157
bool allow_load_extension);
129158
void MemoryInfo(MemoryTracker* tracker) const override;
130159
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
131-
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
132-
static void IsOpenGetter(const v8::FunctionCallbackInfo<v8::Value>& args);
133160
static void IsTransactionGetter(
134161
const v8::FunctionCallbackInfo<v8::Value>& args);
135162
static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -159,16 +186,6 @@ class DatabaseSync : public BaseObject {
159186
void AddBackup(BackupJob* backup);
160187
void FinalizeBackups();
161188
void UntrackStatement(StatementSync* statement);
162-
bool IsOpen();
163-
bool use_big_ints() const { return open_config_.get_use_big_ints(); }
164-
bool return_arrays() const { return open_config_.get_return_arrays(); }
165-
bool allow_bare_named_params() const {
166-
return open_config_.get_allow_bare_named_params();
167-
}
168-
bool allow_unknown_named_params() const {
169-
return open_config_.get_allow_unknown_named_params();
170-
}
171-
sqlite3* Connection();
172189

173190
// In some situations, such as when using custom functions, it is possible
174191
// that SQLite reports an error while JavaScript already has a pending
@@ -181,14 +198,9 @@ class DatabaseSync : public BaseObject {
181198
SET_SELF_SIZE(DatabaseSync)
182199

183200
private:
184-
bool Open();
185201
void DeleteSessions();
186202

187203
~DatabaseSync() override;
188-
DatabaseOpenConfiguration open_config_;
189-
bool allow_load_extension_;
190-
bool enable_load_extension_;
191-
sqlite3* connection_;
192204
bool ignore_next_sqlite_error_;
193205

194206
std::set<BackupJob*> backups_;

0 commit comments

Comments
 (0)