Skip to content

Commit

Permalink
- Extensively utilized automatic type deduction for improved code rea…
Browse files Browse the repository at this point in the history 
…dability

- Implemented RAII to ensure resource safety and exception safety
- Leveraged range-based for loops for cleaner and more efficient iteration
- Addressed and resolved all project-related warnings for enhanced code quality
  • Loading branch information
@zenon8adams @daniellockyer
zenon8adams authored and daniellockyer committed May 29, 2023
1 parent 22ac6cc commit cdf78bf
Show file tree
Hide file tree
Showing 9 changed files with 306 additions and 315 deletions.
11 changes: 5 additions & 6 deletions src/async.h
View file
Expand Up @@ -29,20 +29,19 @@ template <class Item, class Parent> class Async {
}

static void listener(uv_async_t* handle) {
Async* async = static_cast<Async*>(handle->data);
auto* async = static_cast<Async*>(handle->data);
std::vector<Item*> rows;
NODE_SQLITE3_MUTEX_LOCK(&async->mutex)
rows.swap(async->data);
NODE_SQLITE3_MUTEX_UNLOCK(&async->mutex)
for (unsigned int i = 0, size = rows.size(); i < size; i++) {
async->callback(async->parent, rows[i]);
}
for(auto row : rows)
async->callback(async->parent, row);
}

static void close(uv_handle_t* handle) {
assert(handle != NULL);
assert(handle->data != NULL);
Async* async = static_cast<Async*>(handle->data);
auto* async = static_cast<Async*>(handle->data);
delete async;
}

Expand All @@ -56,7 +55,7 @@ template <class Item, class Parent> class Async {

void add(Item* item) {
NODE_SQLITE3_MUTEX_LOCK(&mutex);
data.push_back(item);
data.emplace_back(item);
NODE_SQLITE3_MUTEX_UNLOCK(&mutex)
}

Expand Down
93 changes: 46 additions & 47 deletions src/backup.cc
View file
@@ -1,6 +1,5 @@
#include <string.h>
#include <cstring>
#include <napi.h>

#include "macros.h"
#include "database.h"
#include "backup.h"
Expand All @@ -11,9 +10,9 @@ Napi::Object Backup::Init(Napi::Env env, Napi::Object exports) {
Napi::HandleScope scope(env);

// declare napi_default_method here as it is only available in Node v14.12.0+
napi_property_attributes napi_default_method = static_cast<napi_property_attributes>(napi_writable | napi_configurable);
auto napi_default_method = static_cast<napi_property_attributes>(napi_writable | napi_configurable);

Napi::Function t = DefineClass(env, "Backup", {
auto t = DefineClass(env, "Backup", {
InstanceMethod("step", &Backup::Step, napi_default_method),
InstanceMethod("finish", &Backup::Finish, napi_default_method),
InstanceAccessor("idle", &Backup::IdleGetter, nullptr),
Expand All @@ -34,7 +33,7 @@ void Backup::Process() {
}

while (inited && !locked && !queue.empty()) {
std::unique_ptr<Call> call(queue.front());
auto call = std::move(queue.front());
queue.pop();

call->callback(call->baton);
Expand All @@ -43,19 +42,19 @@ void Backup::Process() {

void Backup::Schedule(Work_Callback callback, Baton* baton) {
if (finished) {
queue.push(new Call(callback, baton));
queue.emplace(new Call(callback, baton));
CleanQueue();
}
else if (!inited || locked || !queue.empty()) {
queue.push(new Call(callback, baton));
queue.emplace(new Call(callback, baton));
}
else {
callback(baton);
}
}

template <class T> void Backup::Error(T* baton) {
Napi::Env env = baton->backup->Env();
auto env = baton->backup->Env();
Napi::HandleScope scope(env);

Backup* backup = baton->backup;
Expand All @@ -76,7 +75,7 @@ template <class T> void Backup::Error(T* baton) {
}

void Backup::CleanQueue() {
Napi::Env env = this->Env();
auto env = this->Env();
Napi::HandleScope scope(env);

if (inited && !queue.empty()) {
Expand All @@ -88,7 +87,7 @@ void Backup::CleanQueue() {

// Clear out the queue so that this object can get GC'ed.
while (!queue.empty()) {
std::unique_ptr<Call> call(queue.front());
auto call = std::move(queue.front());
queue.pop();

std::unique_ptr<Baton> baton(call->baton);
Expand All @@ -111,7 +110,7 @@ void Backup::CleanQueue() {
else while (!queue.empty()) {
// Just delete all items in the queue; we already fired an event when
// initializing the backup failed.
std::unique_ptr<Call> call(queue.front());
auto call = std::move(queue.front());
queue.pop();

// We don't call the actual callback, so we have to make sure that
Expand All @@ -121,13 +120,13 @@ void Backup::CleanQueue() {
}

Backup::Backup(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Backup>(info) {
Napi::Env env = info.Env();
auto env = info.Env();
if (!info.IsConstructCall()) {
Napi::TypeError::New(env, "Use the new operator to create new Backup objects").ThrowAsJavaScriptException();
return;
}

int length = info.Length();
auto length = info.Length();

if (length <= 0 || !Database::HasInstance(info[0])) {
Napi::TypeError::New(env, "Database object expected").ThrowAsJavaScriptException();
Expand All @@ -154,32 +153,32 @@ Backup::Backup(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Backup>(info)
return;
}

Database* db = Napi::ObjectWrap<Database>::Unwrap(info[0].As<Napi::Object>());
Napi::String filename = info[1].As<Napi::String>();
Napi::String sourceName = info[2].As<Napi::String>();
Napi::String destName = info[3].As<Napi::String>();
Napi::Boolean filenameIsDest = info[4].As<Napi::Boolean>();
auto* database = Napi::ObjectWrap<Database>::Unwrap(info[0].As<Napi::Object>());
auto filename = info[1].As<Napi::String>();
auto sourceName = info[2].As<Napi::String>();
auto destName = info[3].As<Napi::String>();
auto filenameIsDest = info[4].As<Napi::Boolean>();

info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("filename", filename));
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("sourceName", sourceName));
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("destName", destName));
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("filenameIsDest", filenameIsDest));

init(db);
init(database);

InitializeBaton* baton = new InitializeBaton(db, info[5].As<Napi::Function>(), this);
auto* baton = new InitializeBaton(database, info[5].As<Napi::Function>(), this);
baton->filename = filename.Utf8Value();
baton->sourceName = sourceName.Utf8Value();
baton->destName = destName.Utf8Value();
baton->filenameIsDest = filenameIsDest.Value();
db->Schedule(Work_BeginInitialize, baton);
database->Schedule(Work_BeginInitialize, baton);
}

void Backup::Work_BeginInitialize(Database::Baton* baton) {
assert(baton->db->open);
baton->db->pending++;
Napi::Env env = baton->db->Env();
int status = napi_create_async_work(
auto env = baton->db->Env();
int UNUSED(status) = napi_create_async_work(
env, NULL, Napi::String::New(env, "sqlite3.Backup.Initialize"),
Work_Initialize, Work_AfterInitialize, baton, &baton->request
);
Expand All @@ -192,7 +191,7 @@ void Backup::Work_Initialize(napi_env e, void* data) {

// In case stepping fails, we use a mutex to make sure we get the associated
// error message.
sqlite3_mutex* mtx = sqlite3_db_mutex(baton->db->_handle);
auto* mtx = sqlite3_db_mutex(baton->db->_handle);
sqlite3_mutex_enter(mtx);

backup->status = sqlite3_open(baton->filename.c_str(), &backup->_otherDb);
Expand All @@ -218,9 +217,9 @@ void Backup::Work_Initialize(napi_env e, void* data) {

void Backup::Work_AfterInitialize(napi_env e, napi_status status, void* data) {
std::unique_ptr<InitializeBaton> baton(static_cast<InitializeBaton*>(data));
Backup* backup = baton->backup;
auto* backup = baton->backup;

Napi::Env env = backup->Env();
auto env = backup->Env();
Napi::HandleScope scope(env);

if (backup->status != SQLITE_OK) {
Expand All @@ -239,13 +238,13 @@ void Backup::Work_AfterInitialize(napi_env e, napi_status status, void* data) {
}

Napi::Value Backup::Step(const Napi::CallbackInfo& info) {
Backup* backup = this;
Napi::Env env = backup->Env();
auto* backup = this;
auto env = backup->Env();

REQUIRE_ARGUMENT_INTEGER(0, pages);
OPTIONAL_ARGUMENT_FUNCTION(1, callback);

StepBaton* baton = new StepBaton(backup, callback, pages);
auto* baton = new StepBaton(backup, callback, pages);
backup->GetRetryErrors(baton->retryErrorsSet);
backup->Schedule(Work_BeginStep, baton);
return info.This();
Expand Down Expand Up @@ -281,9 +280,9 @@ void Backup::Work_Step(napi_env e, void* data) {

void Backup::Work_AfterStep(napi_env e, napi_status status, void* data) {
std::unique_ptr<StepBaton> baton(static_cast<StepBaton*>(data));
Backup* backup = baton->backup;
auto* backup = baton->backup;

Napi::Env env = backup->Env();
auto env = backup->Env();
Napi::HandleScope scope(env);

if (backup->status == SQLITE_DONE) {
Expand All @@ -308,12 +307,12 @@ void Backup::Work_AfterStep(napi_env e, napi_status status, void* data) {
}

Napi::Value Backup::Finish(const Napi::CallbackInfo& info) {
Backup* backup = this;
Napi::Env env = backup->Env();
auto* backup = this;
auto env = backup->Env();

OPTIONAL_ARGUMENT_FUNCTION(0, callback);

Baton* baton = new Baton(backup, callback);
auto* baton = new Baton(backup, callback);
backup->Schedule(Work_BeginFinish, baton);
return info.This();
}
Expand All @@ -329,9 +328,9 @@ void Backup::Work_Finish(napi_env e, void* data) {

void Backup::Work_AfterFinish(napi_env e, napi_status status, void* data) {
std::unique_ptr<Baton> baton(static_cast<Baton*>(data));
Backup* backup = baton->backup;
auto* backup = baton->backup;

Napi::Env env = backup->Env();
auto env = backup->Env();
Napi::HandleScope scope(env);

backup->FinishAll();
Expand Down Expand Up @@ -369,39 +368,39 @@ void Backup::FinishSqlite() {
}

Napi::Value Backup::IdleGetter(const Napi::CallbackInfo& info) {
Backup* backup = this;
auto* backup = this;
bool idle = backup->inited && !backup->locked && backup->queue.empty();
return Napi::Boolean::New(this->Env(), idle);
}

Napi::Value Backup::CompletedGetter(const Napi::CallbackInfo& info) {
Backup* backup = this;
auto* backup = this;
return Napi::Boolean::New(this->Env(), backup->completed);
}

Napi::Value Backup::FailedGetter(const Napi::CallbackInfo& info) {
Backup* backup = this;
auto* backup = this;
return Napi::Boolean::New(this->Env(), backup->failed);
}

Napi::Value Backup::RemainingGetter(const Napi::CallbackInfo& info) {
Backup* backup = this;
auto* backup = this;
return Napi::Number::New(this->Env(), backup->remaining);
}

Napi::Value Backup::PageCountGetter(const Napi::CallbackInfo& info) {
Backup* backup = this;
auto* backup = this;
return Napi::Number::New(this->Env(), backup->pageCount);
}

Napi::Value Backup::RetryErrorGetter(const Napi::CallbackInfo& info) {
Backup* backup = this;
auto* backup = this;
return backup->retryErrors.Value();
}

void Backup::RetryErrorSetter(const Napi::CallbackInfo& info, const Napi::Value& value) {
Backup* backup = this;
Napi::Env env = backup->Env();
auto* backup = this;
auto env = backup->Env();
if (!value.IsArray()) {
Napi::Error::New(env, "retryErrors must be an array").ThrowAsJavaScriptException();
return;
Expand All @@ -413,9 +412,9 @@ void Backup::RetryErrorSetter(const Napi::CallbackInfo& info, const Napi::Value&
void Backup::GetRetryErrors(std::set<int>& retryErrorsSet) {
retryErrorsSet.clear();
Napi::Array array = retryErrors.Value();
int length = array.Length();
for (int i = 0; i < length; i++) {
Napi::Value code = (array).Get(i);
auto length = array.Length();
for (size_t i = 0; i < length; i++) {
Napi::Value code = (array).Get(static_cast<uint32_t>(i));
if (code.IsNumber()) {
retryErrorsSet.insert(code.As<Napi::Number>().Int32Value());
}
Expand Down
9 changes: 5 additions & 4 deletions src/backup.h
View file
Expand Up @@ -121,7 +121,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
Baton(db_, cb_), backup(backup_), filenameIsDest(true) {
backup->Ref();
}
virtual ~InitializeBaton() {
virtual ~InitializeBaton() override {
backup->Unref();
if (!db->IsOpen() && db->IsLocked()) {
// The database handle was closed before the backup could be opened.
Expand All @@ -135,6 +135,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
std::set<int> retryErrorsSet;
StepBaton(Backup* backup_, Napi::Function cb_, int pages_) :
Baton(backup_, cb_), pages(pages_) {}
virtual ~StepBaton() override = default;
};

typedef void (*Work_Callback)(Baton* baton);
Expand Down Expand Up @@ -169,8 +170,8 @@ class Backup : public Napi::ObjectWrap<Backup> {
retryErrors.Reset();
}

WORK_DEFINITION(Step);
WORK_DEFINITION(Finish);
WORK_DEFINITION(Step)
WORK_DEFINITION(Finish)
Napi::Value IdleGetter(const Napi::CallbackInfo& info);
Napi::Value CompletedGetter(const Napi::CallbackInfo& info);
Napi::Value FailedGetter(const Napi::CallbackInfo& info);
Expand Down Expand Up @@ -211,7 +212,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
int remaining;
int pageCount;
bool finished;
std::queue<Call*> queue;
std::queue<std::unique_ptr<Call>> queue;

Napi::Reference<Array> retryErrors;
};
Expand Down

0 comments on commit cdf78bf

Please sign in to comment.