From 8c78b7c9e7904b021521f7887c3074581218be73 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Thu, 16 Jun 2016 11:57:23 -0400 Subject: [PATCH 01/33] Add stmt->allMarshal() method to node_sqlite3. This allows for fast fetching of all results of a query, in Python marshalling format. --- lib/sqlite3.js | 6 +++ package.json | 2 +- src/marshal.h | 108 ++++++++++++++++++++++++++++++++++++++++++++++ src/statement.cc | 109 +++++++++++++++++++++++++++++++++++++++++++++++ src/statement.h | 10 +++++ 5 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 src/marshal.h diff --git a/lib/sqlite3.js b/lib/sqlite3.js index 466b90230..68193170f 100644 --- a/lib/sqlite3.js +++ b/lib/sqlite3.js @@ -87,6 +87,12 @@ Database.prototype.all = normalizeMethod(function(statement, params) { return this; }); +Database.prototype.allMarshal = normalizeMethod(function(statement, params) { + statement.allMarshal.apply(statement, params).finalize(); + return this; +}); + + // Database#each(sql, [bind1, bind2, ...], [callback], [complete]) Database.prototype.each = normalizeMethod(function(statement, params) { statement.each.apply(statement, params).finalize(); diff --git a/package.json b/package.json index f4d36fe43..98cc0be0c 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ }, "scripts": { "prepublish":"npm ls", - "install": "node-pre-gyp install --fallback-to-build", + "install": "node-pre-gyp install --build-from-source", "pretest": "node test/support/createdb.js", "test": "mocha -R spec --timeout 480000" }, diff --git a/src/marshal.h b/src/marshal.h new file mode 100644 index 000000000..310f63b33 --- /dev/null +++ b/src/marshal.h @@ -0,0 +1,108 @@ +#include +#include +#include + +enum MarshalCode { + MARSHAL_NULL = '0', + MARSHAL_NONE = 'N', + MARSHAL_FALSE = 'F', + MARSHAL_TRUE = 'T', + MARSHAL_STOPITER = 'S', + MARSHAL_ELLIPSIS = '.', + MARSHAL_INT = 'i', + MARSHAL_INT64 = 'I', + MARSHAL_FLOAT = 'f', + MARSHAL_BFLOAT = 'g', + MARSHAL_COMPLEX = 'x', + MARSHAL_LONG = 'l', + MARSHAL_STRING = 's', + MARSHAL_INTERNED = 't', + MARSHAL_STRINGREF = 'R', + MARSHAL_TUPLE = '(', + MARSHAL_LIST = '[', + MARSHAL_DICT = '{', + MARSHAL_CODE = 'c', + MARSHAL_UNICODE = 'u', + MARSHAL_UNKNOWN = '?', + MARSHAL_SET = '<', + MARSHAL_FROZENSET = '>', +}; + +class Marshal { + private: + std::vector buffer; + + void _writeCode(MarshalCode code) { + buffer.push_back(static_cast(code)); + } + + void _writeBytes(const void *bytes, size_t nbytes) { + size_t offset = buffer.size(); + buffer.resize(buffer.size() + nbytes); + memcpy(&buffer[offset], bytes, nbytes); + } + + public: + Marshal() { + buffer.reserve(64); + } + + const std::vector &getBuffer() const { + return buffer; + } + + void append(const Marshal &marshal) { + const std::vector &buf = marshal.getBuffer(); + _writeBytes(&buf[0], buf.size()); + } + + void marshalNone() { + _writeCode(MARSHAL_NONE); + } + + void marshalString(const std::string &value) { + marshalString(&value[0], value.size()); + } + + void marshalString(const char *value, int32_t size) { + _writeCode(MARSHAL_STRING); + _writeBytes(&size, sizeof(int32_t)); + _writeBytes(value, size); + } + + void marshalInt(int32_t value) { + _writeCode(MARSHAL_INT); + _writeBytes(&value, sizeof(int32_t)); + } + + void marshalDouble(double value) { + _writeCode(MARSHAL_BFLOAT); + _writeBytes(&value, sizeof(double)); + } + + void marshalBool(bool value) { + _writeCode(value ? MARSHAL_TRUE : MARSHAL_FALSE); + } + + // To marshal a list, call marshalList with a size, followed by size more calls to marshal*. + void marshalList(int32_t size) { + _writeCode(MARSHAL_LIST); + _writeBytes(&size, sizeof(int32_t)); + } + + // To marshal a tuple, call marshalTuple with a size, followed by size more calls to marshal*. + void marshalTuple(int32_t size) { + _writeCode(MARSHAL_TUPLE); + _writeBytes(&size, sizeof(int32_t)); + } + + // To marshal a dictionary, call marshalDictBegin(), followed by an even number of calls to + // marshal* (for alternating keys and values), followed by marshalDictEnd(). + void marshalDictBegin() { + _writeCode(MARSHAL_DICT); + } + + void marshalDictEnd() { + _writeCode(MARSHAL_NULL); + } +}; diff --git a/src/statement.cc b/src/statement.cc index 0166b72dc..8e8f9e5a6 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -23,6 +23,7 @@ NAN_MODULE_INIT(Statement::Init) { Nan::SetPrototypeMethod(t, "get", Get); Nan::SetPrototypeMethod(t, "run", Run); Nan::SetPrototypeMethod(t, "all", All); + Nan::SetPrototypeMethod(t, "allMarshal", AllMarshal); Nan::SetPrototypeMethod(t, "each", Each); Nan::SetPrototypeMethod(t, "reset", Reset); Nan::SetPrototypeMethod(t, "finalize", Finalize); @@ -569,6 +570,114 @@ void Statement::Work_AfterAll(uv_work_t* req) { STATEMENT_END(); } +//---------------------------------------------------------------------- +NAN_METHOD(Statement::AllMarshal) { + Statement* stmt = Nan::ObjectWrap::Unwrap(info.This()); + + Baton* baton = stmt->Bind(info); + if (baton == NULL) { + return Nan::ThrowError("Data type is not supported"); + } else { + stmt->Schedule(Work_BeginAllMarshal, baton); + info.GetReturnValue().Set(info.This()); + } +} + +void Statement::Work_BeginAllMarshal(Baton* baton) { + STATEMENT_BEGIN(AllMarshal); +} + +void Statement::Work_AllMarshal(uv_work_t* req) { + STATEMENT_INIT(MarshalBaton); + + sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle); + sqlite3_mutex_enter(mtx); + + sqlite3_stmt* sqstmt = stmt->_handle; + + int columns = sqlite3_column_count(sqstmt); + baton->colNames.resize(columns); + baton->colData.resize(columns); + for (int i = 0; i < columns; i++) { + baton->colNames[i] = std::string(sqlite3_column_name(sqstmt, i)); + } + + // Make sure that we also reset when there are no parameters. + if (!baton->parameters.size()) { + sqlite3_reset(sqstmt); + } + + if (stmt->Bind(baton->parameters)) { + while ((stmt->status = sqlite3_step(sqstmt)) == SQLITE_ROW) { + baton->countRows++; + for (int i = 0; i < columns; i++) { + int type = sqlite3_column_type(sqstmt, i); + switch (type) { + case SQLITE_INTEGER: + baton->colData[i].marshalInt(sqlite3_column_int64(sqstmt, i)); + break; + case SQLITE_FLOAT: + baton->colData[i].marshalDouble(sqlite3_column_double(sqstmt, i)); + break; + case SQLITE_TEXT: { + const char* text = (const char*)sqlite3_column_text(sqstmt, i); + int length = sqlite3_column_bytes(sqstmt, i); + baton->colData[i].marshalString(text, length); + } break; + case SQLITE_BLOB: { + const char* blob = (const char*)sqlite3_column_blob(sqstmt, i); + int length = sqlite3_column_bytes(sqstmt, i); + // TODO: we want to treat blobs differently, but that's for a bit later. + // (We use Blobs to store objects, though we actually store them as + // human-readably as much as possible.) + baton->colData[i].marshalString(blob, length); + } break; + case SQLITE_NULL: + baton->colData[i].marshalNone(); + break; + default: + assert(false); + } + } + } + + if (stmt->status != SQLITE_DONE) { + stmt->message = std::string(sqlite3_errmsg(stmt->db->_handle)); + } + } + + sqlite3_mutex_leave(mtx); +} + +void Statement::Work_AfterAllMarshal(uv_work_t* req) { + Nan::HandleScope scope; + STATEMENT_INIT(MarshalBaton); + + if (stmt->status != SQLITE_DONE) { + Error(baton); + } else { + // Fire callbacks. + Local cb = Nan::New(baton->callback); + if (!cb.IsEmpty() && cb->IsFunction()) { + Marshal marshal; + marshal.marshalDictBegin(); + for (int i = 0; i < baton->colNames.size(); i++) { + marshal.marshalString(baton->colNames[i]); + marshal.marshalList(baton->countRows); + marshal.append(baton->colData[i]); + } + marshal.marshalDictEnd(); + const std::vector &buffer = marshal.getBuffer(); + Local result(Nan::CopyBuffer(&buffer[0], buffer.size()).ToLocalChecked()); + Local argv[] = { Nan::Null(), result }; + TRY_CATCH_CALL(stmt->handle(), cb, 2, argv); + } + } + STATEMENT_END(); +} + +//---------------------------------------------------------------------- + NAN_METHOD(Statement::Each) { Statement* stmt = Nan::ObjectWrap::Unwrap(info.This()); diff --git a/src/statement.h b/src/statement.h index 90d295b70..2fae3096e 100644 --- a/src/statement.h +++ b/src/statement.h @@ -4,6 +4,7 @@ #include "database.h" #include "threading.h" +#include "marshal.h" #include #include @@ -118,6 +119,14 @@ class Statement : public Nan::ObjectWrap { Rows rows; }; + struct MarshalBaton : Baton { + MarshalBaton(Statement* stmt_, Local cb_) : + Baton(stmt_, cb_), countRows(0) {} + std::vector colNames; + std::vector colData; + int countRows; + }; + struct Async; struct EachBaton : Baton { @@ -203,6 +212,7 @@ class Statement : public Nan::ObjectWrap { WORK_DEFINITION(Get); WORK_DEFINITION(Run); WORK_DEFINITION(All); + WORK_DEFINITION(AllMarshal); WORK_DEFINITION(Each); WORK_DEFINITION(Reset); From 65973ef78cd1717e6edff78f5eeda9dea2a0cc1f Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Fri, 17 Jun 2016 13:12:52 -0400 Subject: [PATCH 02/33] Update version to 3.1.4-marshalling to indicate custom branch --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f36d1d63f..11e2b46b7 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "sqlite3", "description": "Asynchronous, non-blocking SQLite3 bindings", - "version": "3.1.4", + "version": "3.1.4-marshalling", "homepage": "http://github.com/mapbox/node-sqlite3", "author": { "name": "MapBox", From fb0b54c7981387bf807ac3675478bd87fe7406dc Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Thu, 16 Mar 2017 15:37:47 -0400 Subject: [PATCH 03/33] Set package version to 3.1.8 without suffix, or npm avoids using it for dependencies --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a97728a5d..f669ecf04 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "sqlite3", "description": "Asynchronous, non-blocking SQLite3 bindings", - "version": "3.1.8-marshalling", + "version": "3.1.8", "homepage": "http://github.com/mapbox/node-sqlite3", "author": { "name": "MapBox", From 9b21f81987fa00a59f6ccc1a718cb2a9449c0587 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Thu, 16 Mar 2017 17:18:25 -0400 Subject: [PATCH 04/33] Always build from source when installing, since pre-built versions don't have our changes --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f669ecf04..3b8c51fa0 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,7 @@ }, "scripts": { "prepublish": "npm ls", - "install": "node-pre-gyp install --fallback-to-build", + "install": "node-pre-gyp install --build-from-source", "pretest": "node test/support/createdb.js", "test": "mocha -R spec --timeout 480000" }, From e7b5483572988162edd8967cf0e16a7dcc8f3571 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Tue, 7 Nov 2017 16:40:51 -0500 Subject: [PATCH 05/33] Fix distracting compiler warnings --- src/database.cc | 4 ++++ src/macros.h | 4 ++++ src/statement.cc | 3 ++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/database.cc b/src/database.cc index 4dbd5c6ab..d74ced367 100644 --- a/src/database.cc +++ b/src/database.cc @@ -143,6 +143,7 @@ NAN_METHOD(Database::New) { void Database::Work_BeginOpen(Baton* baton) { int status = uv_queue_work(uv_default_loop(), &baton->request, Work_Open, (uv_after_work_cb)Work_AfterOpen); + UNUSED(status); assert(status == 0); } @@ -229,6 +230,7 @@ void Database::Work_BeginClose(Baton* baton) { int status = uv_queue_work(uv_default_loop(), &baton->request, Work_Close, (uv_after_work_cb)Work_AfterClose); + UNUSED(status); assert(status == 0); } @@ -524,6 +526,7 @@ void Database::Work_BeginExec(Baton* baton) { assert(baton->db->pending == 0); int status = uv_queue_work(uv_default_loop(), &baton->request, Work_Exec, (uv_after_work_cb)Work_AfterExec); + UNUSED(status); assert(status == 0); } @@ -624,6 +627,7 @@ void Database::Work_BeginLoadExtension(Baton* baton) { assert(baton->db->pending == 0); int status = uv_queue_work(uv_default_loop(), &baton->request, Work_LoadExtension, reinterpret_cast(Work_AfterLoadExtension)); + UNUSED(status); assert(status == 0); } diff --git a/src/macros.h b/src/macros.h index 38399ee86..30ee90f26 100644 --- a/src/macros.h +++ b/src/macros.h @@ -125,6 +125,7 @@ const char* sqlite_authorizer_string(int type); int status = uv_queue_work(uv_default_loop(), \ &baton->request, \ Work_##type, reinterpret_cast(Work_After##type)); \ + UNUSED(status); \ assert(status == 0); #define STATEMENT_INIT(type) \ @@ -151,4 +152,7 @@ const char* sqlite_authorizer_string(int type); } \ } +/* Use UNUSED(x) to silence compiler warning about an unused value. */ +#define UNUSED(x) ((void)(x)) + #endif diff --git a/src/statement.cc b/src/statement.cc index a529f8847..aaa77dbf4 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -118,6 +118,7 @@ void Statement::Work_BeginPrepare(Database::Baton* baton) { baton->db->pending++; int status = uv_queue_work(uv_default_loop(), &baton->request, Work_Prepare, (uv_after_work_cb)Work_AfterPrepare); + UNUSED(status); assert(status == 0); } @@ -661,7 +662,7 @@ void Statement::Work_AfterAllMarshal(uv_work_t* req) { if (!cb.IsEmpty() && cb->IsFunction()) { Marshal marshal; marshal.marshalDictBegin(); - for (int i = 0; i < baton->colNames.size(); i++) { + for (size_t i = 0; i < baton->colNames.size(); i++) { marshal.marshalString(baton->colNames[i]); marshal.marshalList(baton->countRows); marshal.append(baton->colData[i]); From 3977721bc54c4c43aba56c605e53000a9df67ffa Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Tue, 7 Nov 2017 23:13:08 -0500 Subject: [PATCH 06/33] Add Marshal::marshalValue and a C++ unittest with everything needed to run it --- binding.gyp | 1 + package.json | 4 +++- src/marshal.cc | 53 ++++++++++++++++++++++++++++++++++++++++ src/marshal.h | 10 ++++++++ test/cpp/binding.gyp | 14 +++++++++++ test/cpp/marshal.cc | 22 +++++++++++++++++ test/marshal-test.js | 57 ++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 src/marshal.cc create mode 100644 test/cpp/binding.gyp create mode 100644 test/cpp/marshal.cc create mode 100644 test/marshal-test.js diff --git a/binding.gyp b/binding.gyp index 59c93d30a..9a4f670cf 100644 --- a/binding.gyp +++ b/binding.gyp @@ -33,6 +33,7 @@ ], "sources": [ "src/database.cc", + "src/marshal.cc", "src/node_sqlite3.cc", "src/statement.cc" ] diff --git a/package.json b/package.json index f7a4b66c7..e83d1b786 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ ], "devDependencies": { "aws-sdk": "2.x", + "bindings": "^1.3.0", "eslint": "3.5.0", "mocha": "3.x" }, @@ -52,7 +53,8 @@ "prepublishOnly": "npm ls", "install": "node-pre-gyp install --build-from-source", "pretest": "node test/support/createdb.js", - "test": "mocha -R spec --timeout 480000" + "test": "mocha -R spec --timeout 480000", + "rebuild-tests": "node-gyp rebuild --directory test/cpp" }, "license": "BSD-3-Clause", "keywords": [ diff --git a/src/marshal.cc b/src/marshal.cc new file mode 100644 index 000000000..aac0c471e --- /dev/null +++ b/src/marshal.cc @@ -0,0 +1,53 @@ +#include "marshal.h" + +typedef std::pair > StringPair; +static bool sortByFirst(const StringPair &a, const StringPair &b) { + return a.first < b.first; +} + +void Marshal::marshalValue(v8::Local val) { + if (val->IsBoolean()) { + marshalBool(val->BooleanValue()); + } else if (val->IsInt32()) { + marshalInt(val->Int32Value()); + } else if (val->IsNumber()) { + marshalDouble(val->NumberValue()); + } else if (val->IsString()) { + Nan::Utf8String strVal(val); + marshalUnicode(*strVal, strVal.length()); + } else if (val->IsArray()) { + v8::Local array = v8::Local::Cast(val); + int length = array->Length(); + marshalList(length); + for (int i = 0; i < length; i++) { + marshalValue(Nan::Get(array, i).ToLocalChecked()); + } + } else if (node::Buffer::HasInstance(val)) { + v8::Local buffer = Nan::To(val).ToLocalChecked(); + marshalString(node::Buffer::Data(buffer), node::Buffer::Length(buffer)); + } else if (val->IsObject()) { + v8::Local object = v8::Local::Cast(val); + v8::Local array = Nan::GetPropertyNames(object).ToLocalChecked(); + // Keys need to be serialized in sorted order. + int length = array->Length(); + std::vector keys; + keys.reserve(length); + for (int i = 0; i < length; i++) { + v8::Local name = Nan::Get(array, i).ToLocalChecked(); + Nan::Utf8String strVal(name); + if (*strVal) { + keys.push_back(std::make_pair(std::string(*strVal, strVal.length()), name)); + } + } + std::sort(keys.begin(), keys.end(), sortByFirst); + marshalDictBegin(); + for (size_t i = 0; i < keys.size(); i++) { + v8::Local name = keys[i].second; + marshalValue(name); + marshalValue(Nan::Get(object, name).ToLocalChecked()); + } + marshalDictEnd(); + } else { + marshalNone(); + } +} diff --git a/src/marshal.h b/src/marshal.h index 310f63b33..749fe0f18 100644 --- a/src/marshal.h +++ b/src/marshal.h @@ -1,6 +1,7 @@ #include #include #include +#include enum MarshalCode { MARSHAL_NULL = '0', @@ -56,6 +57,9 @@ class Marshal { _writeBytes(&buf[0], buf.size()); } + // Marshal the given value depending on its type. + void marshalValue(v8::Local val); + void marshalNone() { _writeCode(MARSHAL_NONE); } @@ -70,6 +74,12 @@ class Marshal { _writeBytes(value, size); } + void marshalUnicode(const char *value, int32_t size) { + _writeCode(MARSHAL_UNICODE); + _writeBytes(&size, sizeof(int32_t)); + _writeBytes(value, size); + } + void marshalInt(int32_t value) { _writeCode(MARSHAL_INT); _writeBytes(&value, sizeof(int32_t)); diff --git a/test/cpp/binding.gyp b/test/cpp/binding.gyp new file mode 100644 index 000000000..9aabb22e7 --- /dev/null +++ b/test/cpp/binding.gyp @@ -0,0 +1,14 @@ +{ + "target_defaults": + { + "cflags" : ["-Wall", "-Wextra", "-Wno-unused-parameter"], + "defines": [ "V8_DEPRECATION_WARNINGS=1" ], + "include_dirs": [" +#include +#include +#include "../../src/marshal.h" + +NAN_METHOD(Serialize) { + if (info.Length() > 0) { + Marshal m; + m.marshalValue(info[0]); + const std::vector &buffer = m.getBuffer(); + info.GetReturnValue().Set(Nan::CopyBuffer(&buffer[0], buffer.size()).ToLocalChecked()); + } +} + +NAN_MODULE_INIT(Init) { + Nan::Set(target + , Nan::New("serialize").ToLocalChecked() + , Nan::New(Serialize)->GetFunction() + ); +} + +NODE_MODULE(marshal, Init) diff --git a/test/marshal-test.js b/test/marshal-test.js new file mode 100644 index 000000000..145efd5b8 --- /dev/null +++ b/test/marshal-test.js @@ -0,0 +1,57 @@ +/* global describe, it, escape */ + +const path = require('path'); +const assert = require('assert'); +const util = require('util'); +const bindings = require('bindings'); + +const testRoot = path.resolve(__dirname, 'cpp'); +const mainRoot = path.resolve(__dirname, '..'); +bindings({ module_root: mainRoot, bindings: 'node_sqlite3' }); +const marshal = bindings({ module_root: testRoot, bindings: 'marshal' }); + +describe('marshal', function() { + function stringToArray(str) { + return new Uint8Array(new Buffer(str)); + } + const samples = [ + [null, 'N'], + [1, 'i\x01\x00\x00\x00'], + [1000000, 'i@B\x0f\x00'], + [-123456, 'i\xc0\x1d\xfe\xff'], + [1.23, 'g\xae\x47\xe1\x7a\x14\xae\xf3\x3f'], + [-625e-4, 'g\x00\x00\x00\x00\x00\x00\xb0\xbf'], + [true, 'T'], + [false, 'F'], + [stringToArray('Hello world'), 's\x0b\x00\x00\x00Hello world'], + ['Résumé', 'u\x08\x00\x00\x00R\xc3\xa9sum\xc3\xa9'], + [[1, 2, 3], + '[\x03\x00\x00\x00i\x01\x00\x00\x00i\x02\x00\x00\x00i\x03\x00\x00\x00'], + [{'This': 4, 'is': 0, 'a': stringToArray('test')}, + '{u\x04\x00\x00\x00Thisi\x04\x00\x00\x00u\x01\x00\x00\x00as\x04\x00\x00\x00testu\x02\x00\x00\x00isi\x00\x00\x00\x000'], + ]; + + it("should serialize correctly", function() { + for (const [value, expectedAsString] of samples) { + const expected = binStringToArray(expectedAsString); + const marshalled = marshal.serialize(value); + assert.deepEqual(marshalled, expected, + "Wrong serialization of " + util.inspect(value) + + "\n actual: " + escape(arrayToBinString(marshalled)) + + "\n expected: " + escape(arrayToBinString(expected))); + } + }); +}); + + +function binStringToArray(binaryString) { + var a = new Uint8Array(binaryString.length); + for (var i = 0; i < binaryString.length; i++) { + a[i] = binaryString.charCodeAt(i); + } + return a; +} + +function arrayToBinString(array) { + return String.fromCharCode.apply(String, array); +} From d746ff66871cabc04760883a725ef0cd4aabfcb1 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Wed, 8 Nov 2017 03:16:44 -0500 Subject: [PATCH 07/33] Implement unmarshalling, with unittests. Adds support for endianness, for both marshalling and unmarshalling, including a basic test. --- src/marshal.cc | 244 ++++++++++++++++++++++++++++++++++++++++++- src/marshal.h | 71 +++++++++++-- src/statement.cc | 14 +-- src/statement.h | 2 +- test/cpp/marshal.cc | 32 +++++- test/marshal-test.js | 41 ++++++++ 6 files changed, 384 insertions(+), 20 deletions(-) diff --git a/src/marshal.cc b/src/marshal.cc index aac0c471e..a814f9bdb 100644 --- a/src/marshal.cc +++ b/src/marshal.cc @@ -1,11 +1,75 @@ #include "marshal.h" +// ====================================================================== +// Endianness +// ====================================================================== +// +// Node provides similar functions for Buffer and for DataView, but actually +// using those is hard and annoying. Reimplementing turns out to be easier. + +static bool _isHostLittleEndian() { + union { uint8_t u8[2]; uint16_t u16; } endianness = { { 1, 0 } }; + return endianness.u16 == 1; +} +static bool isHostLittleEndian = _isHostLittleEndian(); + +// For testing only; when called with true, (un)marshalling will be all wrong. +void marshalTestOppositeEndianness(bool useOpposite) { + bool real = _isHostLittleEndian(); + isHostLittleEndian = useOpposite ? !real : real; +} + +// Copy sizeof(T) bytes of value to dest, with requested endianness. +template +void writeEndian(char *dest, T value, bool wantLittleEndian = true) { + union Convert { + T val; + char bytes[sizeof(T)]; + }; + union Convert convert = { value }; + if (wantLittleEndian == isHostLittleEndian) { + std::copy(&convert.bytes[0], &convert.bytes[0] + sizeof(T), dest); + } else { + std::reverse_copy(&convert.bytes[0], &convert.bytes[0] + sizeof(T), dest); + } +} + +// Return value constructed with the first sizeof(T) bytes of data, with requested endianness. +template +T readEndian(const char *data, bool wantLittleEndian = true) { + union Convert { + T val; + char bytes[sizeof(T)]; + }; + union Convert convert = { 0 }; + + if (wantLittleEndian == isHostLittleEndian) { + std::copy(data, data + sizeof(T), convert.bytes); + } else { + std::reverse_copy(data, data + sizeof(T), convert.bytes); + } + return convert.val; +} + + +// ====================================================================== +// Marshaller +// ====================================================================== + +template +void Marshaller::_writeEndian(T value, bool wantLittleEndian) { + size_t offset = buffer.size(); + buffer.resize(buffer.size() + sizeof(T)); + writeEndian(&buffer[offset], value, wantLittleEndian); +} + + typedef std::pair > StringPair; static bool sortByFirst(const StringPair &a, const StringPair &b) { return a.first < b.first; } -void Marshal::marshalValue(v8::Local val) { +void Marshaller::marshalValue(v8::Local val) { if (val->IsBoolean()) { marshalBool(val->BooleanValue()); } else if (val->IsInt32()) { @@ -51,3 +115,181 @@ void Marshal::marshalValue(v8::Local val) { marshalNone(); } } + +// ====================================================================== +// Unmarshaller +// ====================================================================== + +const char *Unmarshaller::consumeBytes(size_t numBytes) { + if (len < numBytes) { return NULL; } + const char *ret = data; + data += numBytes; + len -= numBytes; + return ret; +} + +bool Unmarshaller::readUint8(uint8_t *result) { + const char *bytes = consumeBytes(1); + if (!bytes) { return false; } + *result = bytes[0]; + return true; +} + + +bool Unmarshaller::readInt32(int32_t *result) { + const char *bytes = consumeBytes(4); + if (!bytes) { return false; } + *result = readEndian(bytes); + return true; +} + +bool Unmarshaller::readFloat64(double *result) { + const char *bytes = consumeBytes(8); + if (!bytes) { return false; } + *result = readEndian(bytes); + return true; +} + +bool Unmarshaller::readBytes(size_t len, const char **result) { + const char *bytes = consumeBytes(len); + if (!bytes) { return false; } + *result = bytes; + return true; +} + +Nan::MaybeLocal Unmarshaller::_parse() { + uint8_t code = 0; + if (!readUint8(&code)) { return fail(); } + _lastCode = code; + switch (code) { + case MARSHAL_NULL: return Nan::Null(); + case MARSHAL_NONE: return Nan::Null(); + case MARSHAL_FALSE: return Nan::False(); + case MARSHAL_TRUE: return Nan::True(); + case MARSHAL_INT: return _parseInt32(); + case MARSHAL_INT64: return _parseInt64(); + case MARSHAL_BFLOAT: return _parseBinaryFloat(); + case MARSHAL_STRING: return _parseByteString(); + case MARSHAL_TUPLE: return _parseList(); + case MARSHAL_LIST: return _parseList(); + case MARSHAL_DICT: return _parseDict(); + case MARSHAL_UNICODE: return _parseUnicode(); + case MARSHAL_INTERNED: return _parseInterned(); + case MARSHAL_STRINGREF: return _parseStringRef(); + + // We could support it, but it's unclear if we can parse consistently with + // Python, and it's a deprecated way to serialize floats anyway. + case MARSHAL_FLOAT: return Nan::Null(); + // None of the following are supported. + case MARSHAL_STOPITER: + case MARSHAL_ELLIPSIS: + case MARSHAL_COMPLEX: + case MARSHAL_LONG: + case MARSHAL_CODE: + case MARSHAL_UNKNOWN: + case MARSHAL_SET: + case MARSHAL_FROZENSET: return Nan::Null(); + default: return Nan::Null(); + } +} + + +// A shorthand usd internally below. +static inline Nan::MaybeLocal emptyValue() { + return Nan::MaybeLocal(); +} + +// Helper used internally below. +template +static Nan::MaybeLocal toMaybeLocalValue(Nan::MaybeLocal value) { + return value.IsEmpty() ? emptyValue() : Nan::MaybeLocal(value.ToLocalChecked()); +} + + +Nan::MaybeLocal Unmarshaller::_parseInt32() { + int32_t value = 0; + if (!readInt32(&value)) { return fail(); } + return Nan::New(value); +} + +Nan::MaybeLocal Unmarshaller::_parseInt64() { + int32_t low = 0, hi = 0; + if (!readInt32(&low) || !readInt32(&hi)) { return fail(); } + if ((hi == 0 && low >= 0) || (hi == -1 && low < 0)) { + return Nan::New(low); + } + // TODO We could actually support 53 bits or so, and offer imprecise doubles for larger ones. + // Or pass along a raw representation, such as https://github.com/broofa/node-int64. + return fail("int64 only supports 32-bit values for now"); +} + +Nan::MaybeLocal Unmarshaller::_parseBinaryFloat() { + double value = 0; + if (!readFloat64(&value)) { return fail(); } + return Nan::New(value); +} + + +Nan::MaybeLocal Unmarshaller::_parseByteString() { + int32_t len = 0; + const char *buf = NULL; + if (!readInt32(&len) || !readBytes(len, &buf)) { return fail(); } + return toMaybeLocalValue(Nan::CopyBuffer(buf, len)); +} + +Nan::MaybeLocal Unmarshaller::_parseUnicode() { + int32_t len = 0; + const char *buf = NULL; + if (!readInt32(&len) || !readBytes(len, &buf)) { return fail(); } + return toMaybeLocalValue(Nan::New(buf, len)); +} + +Nan::MaybeLocal Unmarshaller::_parseInterned() { + int32_t len = 0; + const char *buf = NULL; + if (!readInt32(&len) || !readBytes(len, &buf)) { return fail(); } + stringTable.push_back(std::string(buf, len)); + return toMaybeLocalValue(Nan::CopyBuffer(buf, len)); +} + +Nan::MaybeLocal Unmarshaller::_parseStringRef() { + int32_t index = 0; + if (!readInt32(&index)) { return fail(); } + if (index >= 0 && size_t(index) < stringTable.size()) { + const std::string &result = stringTable[index]; + return toMaybeLocalValue(Nan::CopyBuffer(&result[0], result.size())); + } else { + return fail("Invalid interned string reference"); + } +} + +Nan::MaybeLocal Unmarshaller::_parseList() { + int32_t len = 0; + if (!readInt32(&len)) { return fail(); } + + Nan::EscapableHandleScope scope; + v8::Local result = Nan::New(len); + for (int i = 0; i < len; i++) { + Nan::MaybeLocal item = _parse(); + if (item.IsEmpty()) { return emptyValue(); } + Nan::Set(result, i, item.ToLocalChecked()); + } + return scope.Escape(result); +} + +Nan::MaybeLocal Unmarshaller::_parseDict() { + Nan::EscapableHandleScope scope; + v8::Local result = Nan::New(); + while (true) { + Nan::MaybeLocal key = _parse(); + if (key.IsEmpty()) { return emptyValue(); } + + if (_lastCode == MARSHAL_NULL) { break; } + + Nan::MaybeLocal value = Unmarshaller::_parse(); + if (value.IsEmpty()) { return emptyValue(); } + + Nan::Set(result, key.ToLocalChecked(), value.ToLocalChecked()); + } + return scope.Escape(result); +} diff --git a/src/marshal.h b/src/marshal.h index 749fe0f18..0eefeea04 100644 --- a/src/marshal.h +++ b/src/marshal.h @@ -29,7 +29,7 @@ enum MarshalCode { MARSHAL_FROZENSET = '>', }; -class Marshal { +class Marshaller { private: std::vector buffer; @@ -43,8 +43,11 @@ class Marshal { memcpy(&buffer[offset], bytes, nbytes); } + template + void _writeEndian(T value, bool wantLittleEndian = true); + public: - Marshal() { + Marshaller() { buffer.reserve(64); } @@ -52,8 +55,8 @@ class Marshal { return buffer; } - void append(const Marshal &marshal) { - const std::vector &buf = marshal.getBuffer(); + void append(const Marshaller &marshaller) { + const std::vector &buf = marshaller.getBuffer(); _writeBytes(&buf[0], buf.size()); } @@ -70,24 +73,24 @@ class Marshal { void marshalString(const char *value, int32_t size) { _writeCode(MARSHAL_STRING); - _writeBytes(&size, sizeof(int32_t)); + _writeEndian(size); _writeBytes(value, size); } void marshalUnicode(const char *value, int32_t size) { _writeCode(MARSHAL_UNICODE); - _writeBytes(&size, sizeof(int32_t)); + _writeEndian(size); _writeBytes(value, size); } void marshalInt(int32_t value) { _writeCode(MARSHAL_INT); - _writeBytes(&value, sizeof(int32_t)); + _writeEndian(value); } void marshalDouble(double value) { _writeCode(MARSHAL_BFLOAT); - _writeBytes(&value, sizeof(double)); + _writeEndian(value); } void marshalBool(bool value) { @@ -97,13 +100,13 @@ class Marshal { // To marshal a list, call marshalList with a size, followed by size more calls to marshal*. void marshalList(int32_t size) { _writeCode(MARSHAL_LIST); - _writeBytes(&size, sizeof(int32_t)); + _writeEndian(size); } // To marshal a tuple, call marshalTuple with a size, followed by size more calls to marshal*. void marshalTuple(int32_t size) { _writeCode(MARSHAL_TUPLE); - _writeBytes(&size, sizeof(int32_t)); + _writeEndian(size); } // To marshal a dictionary, call marshalDictBegin(), followed by an even number of calls to @@ -116,3 +119,51 @@ class Marshal { _writeCode(MARSHAL_NULL); } }; + + +class Unmarshaller { + public: + static Nan::MaybeLocal parse(const char *data, size_t len) { + Unmarshaller u(data, len); + return u._parse(); + } + + private: + std::vector stringTable; // List of interned strings. + + // Data is a reference to the data passed to the constructor. The reason it's safe to avoid a + // copy is because we'll only use this object from within parse(). + const char *data; + size_t len; + uint8_t _lastCode; + + Unmarshaller(const char *_data, size_t _len) : data(_data), len(_len), _lastCode(0) {} + const char *consumeBytes(size_t numBytes); + bool readUint8(uint8_t *result); + bool readInt32(int32_t *result); + bool readFloat64(double *result); + bool readBytes(size_t len, const char **result); + + Nan::MaybeLocal fail(const char *msg = NULL) { + Nan::ThrowError(msg ? msg : "invalid or truncated marshalled data"); + return Nan::MaybeLocal(); + } + + + Nan::MaybeLocal _parse(); + Nan::MaybeLocal _parseInt32(); + Nan::MaybeLocal _parseInt64(); + Nan::MaybeLocal _parseStringFloat(); + Nan::MaybeLocal _parseBinaryFloat(); + Nan::MaybeLocal _parseByteString(); + Nan::MaybeLocal _parseInterned(); + Nan::MaybeLocal _parseStringRef(); + Nan::MaybeLocal _parseList(); + Nan::MaybeLocal _parseDict(); + Nan::MaybeLocal _parseUnicode(); +}; + +// Since we have our own endianness code, it's nice to be able to test it. This +// call switches our notion of the host endianness resulting in all incorrect +// marshalling. Obviously, this is only for testing, and is not exposed to JS. +void marshalTestOppositeEndianness(bool useOpposite); diff --git a/src/statement.cc b/src/statement.cc index aaa77dbf4..a67a20c5e 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -660,15 +660,15 @@ void Statement::Work_AfterAllMarshal(uv_work_t* req) { // Fire callbacks. Local cb = Nan::New(baton->callback); if (!cb.IsEmpty() && cb->IsFunction()) { - Marshal marshal; - marshal.marshalDictBegin(); + Marshaller marshaller; + marshaller.marshalDictBegin(); for (size_t i = 0; i < baton->colNames.size(); i++) { - marshal.marshalString(baton->colNames[i]); - marshal.marshalList(baton->countRows); - marshal.append(baton->colData[i]); + marshaller.marshalString(baton->colNames[i]); + marshaller.marshalList(baton->countRows); + marshaller.append(baton->colData[i]); } - marshal.marshalDictEnd(); - const std::vector &buffer = marshal.getBuffer(); + marshaller.marshalDictEnd(); + const std::vector &buffer = marshaller.getBuffer(); Local result(Nan::CopyBuffer(&buffer[0], buffer.size()).ToLocalChecked()); Local argv[] = { Nan::Null(), result }; TRY_CATCH_CALL(stmt->handle(), cb, 2, argv); diff --git a/src/statement.h b/src/statement.h index 2fae3096e..8c598decd 100644 --- a/src/statement.h +++ b/src/statement.h @@ -123,7 +123,7 @@ class Statement : public Nan::ObjectWrap { MarshalBaton(Statement* stmt_, Local cb_) : Baton(stmt_, cb_), countRows(0) {} std::vector colNames; - std::vector colData; + std::vector colData; int countRows; }; diff --git a/test/cpp/marshal.cc b/test/cpp/marshal.cc index 03509e10f..f3c392ab6 100644 --- a/test/cpp/marshal.cc +++ b/test/cpp/marshal.cc @@ -3,20 +3,50 @@ #include #include "../../src/marshal.h" + NAN_METHOD(Serialize) { if (info.Length() > 0) { - Marshal m; + Marshaller m; m.marshalValue(info[0]); const std::vector &buffer = m.getBuffer(); info.GetReturnValue().Set(Nan::CopyBuffer(&buffer[0], buffer.size()).ToLocalChecked()); } } +NAN_METHOD(Parse) { + if (info.Length() > 0) { + if (!node::Buffer::HasInstance(info[0])) { + Nan::ThrowError("Argument must be a buffer"); + } else { + v8::Local buffer = Nan::To(info[0]).ToLocalChecked(); + Nan::MaybeLocal result = Unmarshaller::parse( + node::Buffer::Data(buffer), node::Buffer::Length(buffer)); + if (!result.IsEmpty()) { + info.GetReturnValue().Set(result.ToLocalChecked()); + } + } + } +} + +NAN_METHOD(TestOppositeEndianness) { + if (info.Length() > 0) { + marshalTestOppositeEndianness(Nan::To(info[0]).FromJust()); + } +} + NAN_MODULE_INIT(Init) { Nan::Set(target , Nan::New("serialize").ToLocalChecked() , Nan::New(Serialize)->GetFunction() ); + Nan::Set(target + , Nan::New("parse").ToLocalChecked() + , Nan::New(Parse)->GetFunction() + ); + Nan::Set(target + , Nan::New("testOppositeEndianness").ToLocalChecked() + , Nan::New(TestOppositeEndianness)->GetFunction() + ); } NODE_MODULE(marshal, Init) diff --git a/test/marshal-test.js b/test/marshal-test.js index 145efd5b8..491fba9d0 100644 --- a/test/marshal-test.js +++ b/test/marshal-test.js @@ -29,6 +29,8 @@ describe('marshal', function() { '[\x03\x00\x00\x00i\x01\x00\x00\x00i\x02\x00\x00\x00i\x03\x00\x00\x00'], [{'This': 4, 'is': 0, 'a': stringToArray('test')}, '{u\x04\x00\x00\x00Thisi\x04\x00\x00\x00u\x01\x00\x00\x00as\x04\x00\x00\x00testu\x02\x00\x00\x00isi\x00\x00\x00\x000'], + // Limits of 32-bit integers. + [[0x7FFFFFFF, -0x80000000], '[\x02\x00\x00\x00i\xff\xff\xff\x7fi\x00\x00\x00\x80'], ]; it("should serialize correctly", function() { @@ -41,6 +43,45 @@ describe('marshal', function() { "\n expected: " + escape(arrayToBinString(expected))); } }); + + it("should deserialize correctly", function() { + for (const [expected, marshalledAsString] of samples) { + const marshalled = binStringToArray(marshalledAsString); + const parsed = marshal.parse(marshalled); + assert.deepEqual(parsed, expected, + "Wrong parsing of " + escape(marshalledAsString) + + "\n actual: " + escape(parsed) + + "\n expected: " + escape(expected)); + } + }); + + it("should parse interned strings correctly", function() { + const testData = '{t\x03\x00\x00\x00aaat\x03\x00\x00\x00bbbR\x01\x00\x00\x00R\x00\x00\x00\x000'; + assert.deepEqual(marshal.parse(binStringToArray(testData)), + { 'aaa': stringToArray('bbb'), + 'bbb': stringToArray('aaa') + }); + }); + + it("should account for host endianness", function() { + function compare(value, serialization) { + assert.deepEqual(marshal.parse(serialization), value); + assert.deepEqual(marshal.serialize(value), serialization); + } + + compare(0x01020304, binStringToArray('i\x04\x03\x02\x01')); + compare(1.23, binStringToArray('g\xae\x47\xe1\x7a\x14\xae\xf3\x3f')); + + // Reversed output. + marshal.testOppositeEndianness(true); + compare(0x01020304, binStringToArray('i\x01\x02\x03\x04')); + compare(1.23, binStringToArray('g\x3f\xf3\xae\x14\x7a\xe1\x47\xae')); + + // Restore correct serialization. + marshal.testOppositeEndianness(false); + compare(0x01020304, binStringToArray('i\x04\x03\x02\x01')); + compare(1.23, binStringToArray('g\xae\x47\xe1\x7a\x14\xae\xf3\x3f')); + }); }); From c02f45d26055d972ca3ac30147c3ff839f8a4890 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Wed, 8 Nov 2017 17:23:32 -0500 Subject: [PATCH 08/33] Distinguish strings from unicode when marshalling Text vs Blobs --- src/statement.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/statement.cc b/src/statement.cc index a67a20c5e..c84fc4fc4 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -623,14 +623,11 @@ void Statement::Work_AllMarshal(uv_work_t* req) { case SQLITE_TEXT: { const char* text = (const char*)sqlite3_column_text(sqstmt, i); int length = sqlite3_column_bytes(sqstmt, i); - baton->colData[i].marshalString(text, length); + baton->colData[i].marshalUnicode(text, length); } break; case SQLITE_BLOB: { const char* blob = (const char*)sqlite3_column_blob(sqstmt, i); int length = sqlite3_column_bytes(sqstmt, i); - // TODO: we want to treat blobs differently, but that's for a bit later. - // (We use Blobs to store objects, though we actually store them as - // human-readably as much as possible.) baton->colData[i].marshalString(blob, length); } break; case SQLITE_NULL: From e41129c5c6a04225d78c0e7ad35905fbf21f39f3 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Wed, 8 Nov 2017 18:30:40 -0500 Subject: [PATCH 09/33] Fix link errors by explicitly instantiating required templates --- src/marshal.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/marshal.cc b/src/marshal.cc index a814f9bdb..d0246bb2a 100644 --- a/src/marshal.cc +++ b/src/marshal.cc @@ -63,6 +63,9 @@ void Marshaller::_writeEndian(T value, bool wantLittleEndian) { writeEndian(&buffer[offset], value, wantLittleEndian); } +// Instantiate the types we use explicitly. +template void Marshaller::_writeEndian(int32_t value, bool wantLittleEndian); +template void Marshaller::_writeEndian(double value, bool wantLittleEndian); typedef std::pair > StringPair; static bool sortByFirst(const StringPair &a, const StringPair &b) { From 4dec6684ff0bd07921e52eec74c6440f0d999048 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Tue, 13 Mar 2018 19:10:09 -0400 Subject: [PATCH 10/33] add support for ELECTRON_VERSION When forking from an electron process, the logic in node-pre-gyp isn't quite right for finding the sqlite3 binary. There's a PR up to fix this [1]. In the meantime, this patches node-sqlite3 to respect an ELECTRON_VERSION environment variable, overriding node-pre-gyp's options when that variable is detected. --- lib/sqlite3.js | 11 ++++++++++- test/electron.test.js | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 test/electron.test.js diff --git a/lib/sqlite3.js b/lib/sqlite3.js index 8f59edbf5..ccbb60974 100644 --- a/lib/sqlite3.js +++ b/lib/sqlite3.js @@ -1,6 +1,15 @@ var binary = require('node-pre-gyp'); var path = require('path'); -var binding_path = binary.find(path.resolve(path.join(__dirname,'../package.json'))); +// Tweak to pick correct binding_path when running under an electron forked process. +// If this PR gets merged, it will no longer be necessary. +// https://github.com/mapbox/node-pre-gyp/pull/343 +var binding_options = {}; +if (process.env.ELECTRON_VERSION) { + binding_options.runtime = 'electron'; + binding_options.target = process.env.ELECTRON_VERSION; +} +var binding_path = binary.find(path.resolve(path.join(__dirname,'../package.json')), + binding_options); var binding = require(binding_path); var sqlite3 = module.exports = exports = binding; var EventEmitter = require('events').EventEmitter; diff --git a/test/electron.test.js b/test/electron.test.js new file mode 100644 index 000000000..5f592bee4 --- /dev/null +++ b/test/electron.test.js @@ -0,0 +1,9 @@ +var assert = require('assert'); + +describe('electron', function() { + it('respects ELECTRON_VERSION', function() { + process.env.ELECTRON_VERSION = '1.2.3'; + assert.throws(function() { require('..'); }, + (/Cannot find module .*\/node-sqlite3\/lib\/binding\/electron-v1.2-[^-]+-x64\/node_sqlite3.node/)); + }); +}); From de2475e7ee6d0c00290504c1e93ebb887452ac3e Mon Sep 17 00:00:00 2001 From: Janet Vorobyeva Date: Tue, 12 Jun 2018 16:56:15 -0400 Subject: [PATCH 11/33] Fixed testing on linux, readme, electron test (#2) * Fixed testing on linux, readme, electron test marshal.node failed to link on linux for marshal-test.js specified library in test/cpp/binding.gyp Electron test only worked if repo was checked out as exactly node-sqlite3 also, module was sometimes cached, breaking the test (needed to delete it to force reload) --- README.md | 4 +++- test/cpp/binding.gyp | 7 +++++-- test/electron.test.js | 6 +++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 6bdf33bf7..b3d7644f2 100644 --- a/README.md +++ b/README.md @@ -166,15 +166,17 @@ In the case of MacOS with Homebrew, the command should look like the following: npm install sqlite3 --build-from-source --sqlite_libname=sqlcipher --sqlite=`brew --prefix` --runtime=electron --target=1.7.6 --dist-url=https://atom.io/download/electron -# Testing +# Testing (updated for marshalling) [mocha](https://github.com/visionmedia/mocha) is required to run unit tests. In sqlite3's directory (where its `package.json` resides) run the following: npm install mocha + npm run rebuild-tests #rebuilds marshal hooks npm test + # Contributors * [Konstantin Käfer](https://github.com/kkaefer) diff --git a/test/cpp/binding.gyp b/test/cpp/binding.gyp index 9aabb22e7..09f3b7344 100644 --- a/test/cpp/binding.gyp +++ b/test/cpp/binding.gyp @@ -3,12 +3,15 @@ { "cflags" : ["-Wall", "-Wextra", "-Wno-unused-parameter"], "defines": [ "V8_DEPRECATION_WARNINGS=1" ], + "conditions" : [ + ["OS=='linux'", {"libraries+": ["../../../build/<(PRODUCT_DIR)/node_sqlite3.node"] } ] + ], "include_dirs": [" Date: Wed, 24 Mar 2021 18:59:47 -0400 Subject: [PATCH 12/33] Update package.json to set grist-specific version --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 0751b9491..c584e614e 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { - "name": "sqlite3", + "name": "@gristlabs/sqlite3", "description": "Asynchronous, non-blocking SQLite3 bindings", - "version": "4.0.6", + "version": "4.0.6-grist.1", "homepage": "http://github.com/mapbox/node-sqlite3", "author": { "name": "MapBox", From d78fb069f12fe31af22b2b59d18a16e991615028 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 24 Mar 2021 19:02:16 -0400 Subject: [PATCH 13/33] Fix bug in allMarshal() with ints that don't fit in 32 bits. (#5) Now will marshal them as doubles. Includes a test case that fails without the fix. --- src/statement.cc | 11 +++++-- test/allMarshal.test.js | 64 +++++++++++++++++++++++++++++++++++++++++ test/marshal-test.js | 4 +++ 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 test/allMarshal.test.js diff --git a/src/statement.cc b/src/statement.cc index c6e1f3d11..f52e03727 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -614,9 +614,16 @@ void Statement::Work_AllMarshal(uv_work_t* req) { for (int i = 0; i < columns; i++) { int type = sqlite3_column_type(sqstmt, i); switch (type) { - case SQLITE_INTEGER: - baton->colData[i].marshalInt(sqlite3_column_int64(sqstmt, i)); + case SQLITE_INTEGER: { + int64_t value = sqlite3_column_int64(sqstmt, i); + int32_t smallValue = int32_t(value); + if (value == smallValue) { + baton->colData[i].marshalInt(smallValue); + } else { + baton->colData[i].marshalDouble(value); + } break; + } case SQLITE_FLOAT: baton->colData[i].marshalDouble(sqlite3_column_double(sqstmt, i)); break; diff --git a/test/allMarshal.test.js b/test/allMarshal.test.js new file mode 100644 index 000000000..0970a1e86 --- /dev/null +++ b/test/allMarshal.test.js @@ -0,0 +1,64 @@ +/* globals describe, it, before, after */ +var sqlite3 = require('..'); +var assert = require('assert'); + +describe('Database#allMarshal', function() { + var db; + before(function(done) { db = new sqlite3.Database(':memory:', done); }); + + var testValues = [0x7FFFFFFF, Number.MAX_SAFE_INTEGER, Number.MAX_SAFE_INTEGER + 1]; + var marshalled = ['i\xff\xff\xff\x7f', 'g\xff\xff\xff\xff\xff\xff?C', 'g\x00\x00\x00\x00\x00\x00@C']; + var marshalledFloat = ['g\x00\x00\xc0\xff\xff\xff\xdfA', marshalled[1], marshalled[2]]; + + it('should create the table', function(done) { + db.run("CREATE TABLE foo (row text, num int, flt float, blb blob)", function(err) { + if (err) throw err; + var inserted = 0; + for (var i = 0; i < testValues.length; i++) { + var val = testValues[i]; + db.run("INSERT INTO foo VALUES(?, ?, ?, ?)", + 'row' + i, val, val, val, + function(err) { + if (err) throw err; + inserted++; + if (inserted === testValues.length) { + done(); + } + } + ); + } + }); + }); + + it('should retrieve all rows', function(done) { + var fields = ['num', 'flt', 'blb']; + var count = 0; + var results = []; + testValues.forEach(function(value, i) { + results[i] = {}; + fields.forEach(function(field) { + count++; + db.allMarshal("SELECT " + field + " as f FROM foo WHERE row=\"row" + i + "\"", + function(err, result) { + results[i][field] = result; + if (--count === 0) { compare(); } + } + ); + }); + }); + function compare() { + testValues.forEach(function(value, i) { + fields.forEach(function(field) { + // We query in a way so that marshalled data has the same form for all values: + var mvalue = (field === 'flt') ? marshalledFloat[i] : marshalled[i]; + var expect = Buffer.from('{s\x01\x00\x00\x00f[\x01\x00\x00\x00' + mvalue + '0', 'binary'); + var result = results[i][field]; + assert.deepEqual(result, expect); + }); + }); + done(); + } + }); + + after(function(done) { db.close(done); }); +}); diff --git a/test/marshal-test.js b/test/marshal-test.js index 16c278617..5b9ee2fb3 100644 --- a/test/marshal-test.js +++ b/test/marshal-test.js @@ -31,6 +31,10 @@ describe('marshal', function() { '{u\x04\x00\x00\x00Thisi\x04\x00\x00\x00u\x01\x00\x00\x00as\x04\x00\x00\x00testu\x02\x00\x00\x00isi\x00\x00\x00\x000'], // Limits of 32-bit integers. [[0x7FFFFFFF, -0x80000000], '[\x02\x00\x00\x00i\xff\xff\xff\x7fi\x00\x00\x00\x80'], + // Beyond 32-bit limit, we marshal numbers as doubles. + [0x80000000, 'g\x00\x00\x00\x00\x00\x00\xe0A'], + [-9007199254740991, 'g\xff\xff\xff\xff\xff\xff?\xc3'], + [9007199254740992, 'g\x00\x00\x00\x00\x00\x00@C'], ]; it("should serialize correctly", function() { From c2dc8a9f26b6a2139f037a201d321ca650940201 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 29 Apr 2021 12:53:55 -0400 Subject: [PATCH 14/33] port marshalling code to napi --- package.json | 2 +- scripts/prepare_for_test.sh | 17 +++++ src/database.cc | 10 ++- src/marshal.cc | 146 +++++++++++++++++++----------------- src/marshal.h | 40 +++++----- src/statement.cc | 27 ++++--- src/statement.h | 2 +- test/cpp/binding.gyp | 6 +- test/cpp/marshal.cc | 55 +++++++------- 9 files changed, 172 insertions(+), 133 deletions(-) create mode 100755 scripts/prepare_for_test.sh diff --git a/package.json b/package.json index ae262808f..4bffe4acf 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "scripts": { "install": "node-pre-gyp install --build-from-source", "pretest": "node test/support/createdb.js", - "test": "mocha -R spec --timeout 480000", + "test": "./scripts/prepare_for_test.sh; mocha -R spec --timeout 480000", "rebuild-tests": "node-gyp rebuild --directory test/cpp", "pack": "node-pre-gyp package" }, diff --git a/scripts/prepare_for_test.sh b/scripts/prepare_for_test.sh new file mode 100755 index 000000000..b306e2364 --- /dev/null +++ b/scripts/prepare_for_test.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +# Marshalling tests need to use built node_sqlite3.node, but bindings helper +# doesn't look in the right place. There's probably a smart way to fix this, +# but since this only affects tests of our fork, this script just makes a +# link in the place where marshalling tests expect. + +set -e + +expected=$(echo "console.log('node-v' + process.versions.modules + '-' + process.platform + '-' + process.arch)" | node) +if [ ! -e lib/binding/$expected/node_sqlite3.node ]; then + cd lib/binding + target=$(find . -iname "node_sqlite3.node" | head -n1) + mkdir -p $expected + ln -s ../$target $expected/node_sqlite3.node + echo Added lib/binding/$expected/node_sqlite3.node +fi diff --git a/src/database.cc b/src/database.cc index 15265bd30..59d31a78d 100644 --- a/src/database.cc +++ b/src/database.cc @@ -145,6 +145,7 @@ void Database::Work_BeginOpen(Baton* baton) { env, NULL, Napi::String::New(env, "sqlite3.Database.Open"), Work_Open, Work_AfterOpen, baton, &baton->request ); + UNUSED(status); assert(status == 0); napi_queue_async_work(env, baton->request); } @@ -235,8 +236,11 @@ void Database::Work_BeginClose(Baton* baton) { baton->db->RemoveCallbacks(); baton->db->closing = true; - int status = uv_queue_work(uv_default_loop(), - &baton->request, Work_Close, (uv_after_work_cb)Work_AfterClose); + Napi::Env env = baton->db->Env(); + int status = napi_create_async_work( + env, NULL, Napi::String::New(env, "sqlite3.Database.Close"), + Work_Close, Work_AfterClose, baton, &baton->request + ); UNUSED(status); assert(status == 0); napi_queue_async_work(env, baton->request); @@ -555,6 +559,7 @@ void Database::Work_BeginExec(Baton* baton) { env, NULL, Napi::String::New(env, "sqlite3.Database.Exec"), Work_Exec, Work_AfterExec, baton, &baton->request ); + UNUSED(status); assert(status == 0); napi_queue_async_work(env, baton->request); } @@ -665,6 +670,7 @@ void Database::Work_BeginLoadExtension(Baton* baton) { env, NULL, Napi::String::New(env, "sqlite3.Database.LoadExtension"), Work_LoadExtension, Work_AfterLoadExtension, baton, &baton->request ); + UNUSED(status); assert(status == 0); napi_queue_async_work(env, baton->request); } diff --git a/src/marshal.cc b/src/marshal.cc index d0246bb2a..3d2040955 100644 --- a/src/marshal.cc +++ b/src/marshal.cc @@ -1,3 +1,4 @@ +#include "macros.h" #include "marshal.h" // ====================================================================== @@ -67,51 +68,52 @@ void Marshaller::_writeEndian(T value, bool wantLittleEndian) { template void Marshaller::_writeEndian(int32_t value, bool wantLittleEndian); template void Marshaller::_writeEndian(double value, bool wantLittleEndian); -typedef std::pair > StringPair; +typedef std::pair StringPair; static bool sortByFirst(const StringPair &a, const StringPair &b) { return a.first < b.first; } -void Marshaller::marshalValue(v8::Local val) { - if (val->IsBoolean()) { - marshalBool(val->BooleanValue()); - } else if (val->IsInt32()) { - marshalInt(val->Int32Value()); - } else if (val->IsNumber()) { - marshalDouble(val->NumberValue()); - } else if (val->IsString()) { - Nan::Utf8String strVal(val); - marshalUnicode(*strVal, strVal.length()); - } else if (val->IsArray()) { - v8::Local array = v8::Local::Cast(val); - int length = array->Length(); +void Marshaller::marshalValue(Napi::Value val) { + if (val.IsBoolean()) { + marshalBool(val.As()); + } else if (val.IsNumber()) { + Napi::Number num = val.As(); + if (OtherIsInt(num)) { + marshalInt(num.Int32Value()); + } else { + marshalDouble(num.DoubleValue()); + } + } else if (val.IsString()) { + std::string strVal = val.As(); + marshalUnicode(strVal.c_str(), strVal.length()); + } else if (val.IsArray()) { + Napi::Array array = val.As(); + int length = array.Length(); marshalList(length); for (int i = 0; i < length; i++) { - marshalValue(Nan::Get(array, i).ToLocalChecked()); + marshalValue(array.Get(i)); } - } else if (node::Buffer::HasInstance(val)) { - v8::Local buffer = Nan::To(val).ToLocalChecked(); - marshalString(node::Buffer::Data(buffer), node::Buffer::Length(buffer)); - } else if (val->IsObject()) { - v8::Local object = v8::Local::Cast(val); - v8::Local array = Nan::GetPropertyNames(object).ToLocalChecked(); + } else if (val.IsBuffer()) { + Napi::Buffer buffer = val.As>(); + marshalString(buffer.Data(), buffer.Length()); + } else if (val.IsObject()) { + Napi::Object object = val.As(); + Napi::Array array = object.GetPropertyNames(); // Keys need to be serialized in sorted order. - int length = array->Length(); + int length = array.Length(); std::vector keys; keys.reserve(length); for (int i = 0; i < length; i++) { - v8::Local name = Nan::Get(array, i).ToLocalChecked(); - Nan::Utf8String strVal(name); - if (*strVal) { - keys.push_back(std::make_pair(std::string(*strVal, strVal.length()), name)); - } + Napi::Value name = array.Get(i); + std::string strVal = name.As(); + keys.push_back(std::make_pair(strVal, name)); } std::sort(keys.begin(), keys.end(), sortByFirst); marshalDictBegin(); for (size_t i = 0; i < keys.size(); i++) { - v8::Local name = keys[i].second; + Napi::Value name = keys[i].second; marshalValue(name); - marshalValue(Nan::Get(object, name).ToLocalChecked()); + marshalValue(object.Get(name)); } marshalDictEnd(); } else { @@ -160,15 +162,16 @@ bool Unmarshaller::readBytes(size_t len, const char **result) { return true; } -Nan::MaybeLocal Unmarshaller::_parse() { +Napi::Value Unmarshaller::_parse() { + Napi::Env env = info.Env(); uint8_t code = 0; if (!readUint8(&code)) { return fail(); } _lastCode = code; switch (code) { - case MARSHAL_NULL: return Nan::Null(); - case MARSHAL_NONE: return Nan::Null(); - case MARSHAL_FALSE: return Nan::False(); - case MARSHAL_TRUE: return Nan::True(); + case MARSHAL_NULL: return env.Null(); + case MARSHAL_NONE: return env.Null(); + case MARSHAL_FALSE: return Napi::Boolean::New(env, false); + case MARSHAL_TRUE: return Napi::Boolean::New(env, true); case MARSHAL_INT: return _parseInt32(); case MARSHAL_INT64: return _parseInt64(); case MARSHAL_BFLOAT: return _parseBinaryFloat(); @@ -182,7 +185,7 @@ Nan::MaybeLocal Unmarshaller::_parse() { // We could support it, but it's unclear if we can parse consistently with // Python, and it's a deprecated way to serialize floats anyway. - case MARSHAL_FLOAT: return Nan::Null(); + case MARSHAL_FLOAT: return env.Null(); // None of the following are supported. case MARSHAL_STOPITER: case MARSHAL_ELLIPSIS: @@ -191,108 +194,111 @@ Nan::MaybeLocal Unmarshaller::_parse() { case MARSHAL_CODE: case MARSHAL_UNKNOWN: case MARSHAL_SET: - case MARSHAL_FROZENSET: return Nan::Null(); - default: return Nan::Null(); + case MARSHAL_FROZENSET: return env.Null(); + default: return env.Null(); } } // A shorthand usd internally below. -static inline Nan::MaybeLocal emptyValue() { - return Nan::MaybeLocal(); -} - -// Helper used internally below. -template -static Nan::MaybeLocal toMaybeLocalValue(Nan::MaybeLocal value) { - return value.IsEmpty() ? emptyValue() : Nan::MaybeLocal(value.ToLocalChecked()); +static inline Napi::Value emptyValue() { + return Napi::Value(); } -Nan::MaybeLocal Unmarshaller::_parseInt32() { +Napi::Value Unmarshaller::_parseInt32() { int32_t value = 0; if (!readInt32(&value)) { return fail(); } - return Nan::New(value); + Napi::Env env = info.Env(); + return Napi::Number::New(env, value); } -Nan::MaybeLocal Unmarshaller::_parseInt64() { +Napi::Value Unmarshaller::_parseInt64() { int32_t low = 0, hi = 0; if (!readInt32(&low) || !readInt32(&hi)) { return fail(); } if ((hi == 0 && low >= 0) || (hi == -1 && low < 0)) { - return Nan::New(low); + Napi::Env env = info.Env(); + return Napi::Number::New(env, low); } // TODO We could actually support 53 bits or so, and offer imprecise doubles for larger ones. // Or pass along a raw representation, such as https://github.com/broofa/node-int64. return fail("int64 only supports 32-bit values for now"); } -Nan::MaybeLocal Unmarshaller::_parseBinaryFloat() { +Napi::Value Unmarshaller::_parseBinaryFloat() { double value = 0; if (!readFloat64(&value)) { return fail(); } - return Nan::New(value); + Napi::Env env = info.Env(); + return Napi::Number::New(env, value); } -Nan::MaybeLocal Unmarshaller::_parseByteString() { +Napi::Value Unmarshaller::_parseByteString() { int32_t len = 0; const char *buf = NULL; if (!readInt32(&len) || !readBytes(len, &buf)) { return fail(); } - return toMaybeLocalValue(Nan::CopyBuffer(buf, len)); + Napi::Env env = info.Env(); + return Napi::Buffer::Copy(env, buf, len); } -Nan::MaybeLocal Unmarshaller::_parseUnicode() { +Napi::Value Unmarshaller::_parseUnicode() { int32_t len = 0; const char *buf = NULL; if (!readInt32(&len) || !readBytes(len, &buf)) { return fail(); } - return toMaybeLocalValue(Nan::New(buf, len)); + Napi::Env env = info.Env(); + return Napi::String::New(env, buf, len); } -Nan::MaybeLocal Unmarshaller::_parseInterned() { +Napi::Value Unmarshaller::_parseInterned() { int32_t len = 0; const char *buf = NULL; if (!readInt32(&len) || !readBytes(len, &buf)) { return fail(); } stringTable.push_back(std::string(buf, len)); - return toMaybeLocalValue(Nan::CopyBuffer(buf, len)); + Napi::Env env = info.Env(); + return Napi::Buffer::Copy(env, buf, len); } -Nan::MaybeLocal Unmarshaller::_parseStringRef() { +Napi::Value Unmarshaller::_parseStringRef() { int32_t index = 0; if (!readInt32(&index)) { return fail(); } if (index >= 0 && size_t(index) < stringTable.size()) { const std::string &result = stringTable[index]; - return toMaybeLocalValue(Nan::CopyBuffer(&result[0], result.size())); + Napi::Env env = info.Env(); + return Napi::Buffer::Copy(env, &result[0], result.size()); } else { return fail("Invalid interned string reference"); } } -Nan::MaybeLocal Unmarshaller::_parseList() { +Napi::Value Unmarshaller::_parseList() { int32_t len = 0; if (!readInt32(&len)) { return fail(); } - Nan::EscapableHandleScope scope; - v8::Local result = Nan::New(len); + Napi::Env env = info.Env(); + Napi::EscapableHandleScope scope(env); + Napi::Array result = Napi::Array::New(env, len); for (int i = 0; i < len; i++) { - Nan::MaybeLocal item = _parse(); + Napi::Value item = _parse(); if (item.IsEmpty()) { return emptyValue(); } - Nan::Set(result, i, item.ToLocalChecked()); + result.Set(i, item); } return scope.Escape(result); } -Nan::MaybeLocal Unmarshaller::_parseDict() { - Nan::EscapableHandleScope scope; - v8::Local result = Nan::New(); +Napi::Value Unmarshaller::_parseDict() { + Napi::Env env = info.Env(); + Napi::EscapableHandleScope scope(env); + Napi::Object result = Napi::Object::New(env); while (true) { - Nan::MaybeLocal key = _parse(); + Napi::Value key = _parse(); if (key.IsEmpty()) { return emptyValue(); } if (_lastCode == MARSHAL_NULL) { break; } - Nan::MaybeLocal value = Unmarshaller::_parse(); + Napi::Value value = Unmarshaller::_parse(); if (value.IsEmpty()) { return emptyValue(); } - Nan::Set(result, key.ToLocalChecked(), value.ToLocalChecked()); + result.Set(key, value); } return scope.Escape(result); } diff --git a/src/marshal.h b/src/marshal.h index 0eefeea04..a1a00ae87 100644 --- a/src/marshal.h +++ b/src/marshal.h @@ -1,7 +1,7 @@ #include #include #include -#include +#include enum MarshalCode { MARSHAL_NULL = '0', @@ -61,7 +61,7 @@ class Marshaller { } // Marshal the given value depending on its type. - void marshalValue(v8::Local val); + void marshalValue(Napi::Value val); void marshalNone() { _writeCode(MARSHAL_NONE); @@ -123,8 +123,8 @@ class Marshaller { class Unmarshaller { public: - static Nan::MaybeLocal parse(const char *data, size_t len) { - Unmarshaller u(data, len); + static Napi::Value parse(const Napi::CallbackInfo& info, const char *data, size_t len) { + Unmarshaller u(info, data, len); return u._parse(); } @@ -136,31 +136,33 @@ class Unmarshaller { const char *data; size_t len; uint8_t _lastCode; + const Napi::CallbackInfo& info; - Unmarshaller(const char *_data, size_t _len) : data(_data), len(_len), _lastCode(0) {} + Unmarshaller(const Napi::CallbackInfo& _info, const char *_data, size_t _len) : data(_data), len(_len), _lastCode(0), info(_info) {} const char *consumeBytes(size_t numBytes); bool readUint8(uint8_t *result); bool readInt32(int32_t *result); bool readFloat64(double *result); bool readBytes(size_t len, const char **result); - Nan::MaybeLocal fail(const char *msg = NULL) { - Nan::ThrowError(msg ? msg : "invalid or truncated marshalled data"); - return Nan::MaybeLocal(); + Napi::Value fail(const char *msg = NULL) { + Napi::Env env = info.Env(); + Napi::Error::New(env, msg ? msg : "invalid or truncated marshalled data").ThrowAsJavaScriptException(); + return Napi::Value(); } - Nan::MaybeLocal _parse(); - Nan::MaybeLocal _parseInt32(); - Nan::MaybeLocal _parseInt64(); - Nan::MaybeLocal _parseStringFloat(); - Nan::MaybeLocal _parseBinaryFloat(); - Nan::MaybeLocal _parseByteString(); - Nan::MaybeLocal _parseInterned(); - Nan::MaybeLocal _parseStringRef(); - Nan::MaybeLocal _parseList(); - Nan::MaybeLocal _parseDict(); - Nan::MaybeLocal _parseUnicode(); + Napi::Value _parse(); + Napi::Value _parseInt32(); + Napi::Value _parseInt64(); + Napi::Value _parseStringFloat(); + Napi::Value _parseBinaryFloat(); + Napi::Value _parseByteString(); + Napi::Value _parseInterned(); + Napi::Value _parseStringRef(); + Napi::Value _parseList(); + Napi::Value _parseDict(); + Napi::Value _parseUnicode(); }; // Since we have our own endianness code, it's nice to be able to test it. This diff --git a/src/statement.cc b/src/statement.cc index 330af9875..b387e2ee7 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -18,6 +18,7 @@ Napi::Object Statement::Init(Napi::Env env, Napi::Object exports) { InstanceMethod("get", &Statement::Get), InstanceMethod("run", &Statement::Run), InstanceMethod("all", &Statement::All), + InstanceMethod("allMarshal", &Statement::AllMarshal), InstanceMethod("each", &Statement::Each), InstanceMethod("reset", &Statement::Reset), InstanceMethod("finalize", &Statement::Finalize_), @@ -607,15 +608,17 @@ void Statement::Work_AfterAll(napi_env e, napi_status status, void* data) { } //---------------------------------------------------------------------- -NAN_METHOD(Statement::AllMarshal) { - Statement* stmt = Nan::ObjectWrap::Unwrap(info.This()); +Napi::Value Statement::AllMarshal(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); + Statement* stmt = this; Baton* baton = stmt->Bind(info); if (baton == NULL) { - return Nan::ThrowError("Data type is not supported"); + Napi::Error::New(env, "Data type is not supported").ThrowAsJavaScriptException(); + return env.Null(); } else { stmt->Schedule(Work_BeginAllMarshal, baton); - info.GetReturnValue().Set(info.This()); + return info.This(); } } @@ -623,7 +626,7 @@ void Statement::Work_BeginAllMarshal(Baton* baton) { STATEMENT_BEGIN(AllMarshal); } -void Statement::Work_AllMarshal(uv_work_t* req) { +void Statement::Work_AllMarshal(napi_env e, void* data) { STATEMENT_INIT(MarshalBaton); sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle); @@ -689,16 +692,16 @@ void Statement::Work_AllMarshal(uv_work_t* req) { sqlite3_mutex_leave(mtx); } -void Statement::Work_AfterAllMarshal(uv_work_t* req) { - Nan::HandleScope scope; +void Statement::Work_AfterAllMarshal(napi_env e, napi_status status, void* data) { STATEMENT_INIT(MarshalBaton); + Napi::Env env = stmt->Env(); if (stmt->status != SQLITE_DONE) { Error(baton); } else { // Fire callbacks. - Local cb = Nan::New(baton->callback); - if (!cb.IsEmpty() && cb->IsFunction()) { + Napi::Function cb = baton->callback.Value(); + if (!cb.IsUndefined() && cb.IsFunction()) { Marshaller marshaller; marshaller.marshalDictBegin(); for (size_t i = 0; i < baton->colNames.size(); i++) { @@ -708,9 +711,9 @@ void Statement::Work_AfterAllMarshal(uv_work_t* req) { } marshaller.marshalDictEnd(); const std::vector &buffer = marshaller.getBuffer(); - Local result(Nan::CopyBuffer(&buffer[0], buffer.size()).ToLocalChecked()); - Local argv[] = { Nan::Null(), result }; - TRY_CATCH_CALL(stmt->handle(), cb, 2, argv); + Napi::Value result(Napi::Buffer::Copy(env, &buffer[0], buffer.size())); + Napi::Value argv[] = { env.Null(), result }; + TRY_CATCH_CALL(stmt->Value(), cb, 2, argv); } } STATEMENT_END(); diff --git a/src/statement.h b/src/statement.h index 4e1dcbf3f..9ea62cfd9 100644 --- a/src/statement.h +++ b/src/statement.h @@ -118,7 +118,7 @@ class Statement : public Napi::ObjectWrap { }; struct MarshalBaton : Baton { - MarshalBaton(Statement* stmt_, Local cb_) : + MarshalBaton(Statement* stmt_, Napi::Function cb_) : Baton(stmt_, cb_), countRows(0) {} std::vector colNames; std::vector colData; diff --git a/test/cpp/binding.gyp b/test/cpp/binding.gyp index 09f3b7344..d85a62f2f 100644 --- a/test/cpp/binding.gyp +++ b/test/cpp/binding.gyp @@ -6,12 +6,14 @@ "conditions" : [ ["OS=='linux'", {"libraries+": ["../../../build/<(PRODUCT_DIR)/node_sqlite3.node"] } ] ], - "include_dirs": [" +#include +#include #include #include #include "../../src/marshal.h" -NAN_METHOD(Serialize) { +Napi::Value Serialize(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); if (info.Length() > 0) { Marshaller m; m.marshalValue(info[0]); const std::vector &buffer = m.getBuffer(); - info.GetReturnValue().Set(Nan::CopyBuffer(&buffer[0], buffer.size()).ToLocalChecked()); + Napi::Env env = info.Env(); + return Napi::Buffer::Copy(env, &buffer[0], buffer.size()); } + return env.Null(); } -NAN_METHOD(Parse) { +Napi::Value Parse(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); if (info.Length() > 0) { - if (!node::Buffer::HasInstance(info[0])) { - Nan::ThrowError("Argument must be a buffer"); + if (!info[0].IsBuffer()) { + Napi::Error::New(env, "Argument must be a buffer").ThrowAsJavaScriptException(); + return env.Null(); } else { - v8::Local buffer = Nan::To(info[0]).ToLocalChecked(); - Nan::MaybeLocal result = Unmarshaller::parse( - node::Buffer::Data(buffer), node::Buffer::Length(buffer)); + Napi::Buffer buffer = info[0].As>(); + Napi::Value result = Unmarshaller::parse(info, buffer.Data(), buffer.Length()); if (!result.IsEmpty()) { - info.GetReturnValue().Set(result.ToLocalChecked()); + return result; } } } + return env.Null(); } -NAN_METHOD(TestOppositeEndianness) { +Napi::Value TestOppositeEndianness(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); if (info.Length() > 0) { - marshalTestOppositeEndianness(Nan::To(info[0]).FromJust()); + marshalTestOppositeEndianness(info[0].As().Value()); } + return env.Null(); } -NAN_MODULE_INIT(Init) { - Nan::Set(target - , Nan::New("serialize").ToLocalChecked() - , Nan::New(Serialize)->GetFunction() - ); - Nan::Set(target - , Nan::New("parse").ToLocalChecked() - , Nan::New(Parse)->GetFunction() - ); - Nan::Set(target - , Nan::New("testOppositeEndianness").ToLocalChecked() - , Nan::New(TestOppositeEndianness)->GetFunction() - ); +Napi::Object Init(Napi::Env env, Napi::Object exports) { + exports.Set(Napi::String::New(env, "serialize"), + Napi::Function::New(env, Serialize)); + exports.Set(Napi::String::New(env, "parse"), + Napi::Function::New(env, Parse)); + exports.Set(Napi::String::New(env, "testOppositeEndianness"), + Napi::Function::New(env, TestOppositeEndianness)); + return exports; } -NODE_MODULE(marshal, Init) +NODE_API_MODULE(marshal, Init) From 1a973d16e4ac95db4270c8b42d52bddcb7003f3a Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Mon, 19 Jul 2021 11:24:51 -0400 Subject: [PATCH 15/33] squash recent grist changes for easier rebase turn off support for ATTACH This sets the maximum number of ATTACHed databases to 0, as suggested by https://www.sqlite.org/security.html remove unneeded file delete; bump version Fix attach.test.js when running the first time Set SQLITE_ENABLE_DBSTAT_VTAB=1 to use dbstat table Bump version to .3 Add instructions to README.md Enable ATTACH again to allow VACUUM support sqlite3_limit(id, value) via db.configure('limit', id, value) This extends `db.configure` to support the `sqlite3_limit` method. Calling `db.configure('limit', sqlite3.LIMIT_XXX, value)` is equivalent to calling `sqlite3_limit(db, SQLITE_LIMIT_XXX, value)`. For example, to prohibit attaching extra databases on a given database connection, you'd call `db.configure('limit', sqlite3.LIMIT_ATTACHED, 0)`. bump version bump version --- README.md | 12 ++++++++++++ deps/sqlite3.gyp | 6 ++++-- package.json | 2 +- src/database.cc | 25 +++++++++++++++++++++++++ src/database.h | 8 ++++++++ src/node_sqlite3.cc | 13 +++++++++++++ test/attach.test.js | 25 +++++++++++++++++++++++++ test/limit.test.js | 28 ++++++++++++++++++++++++++++ 8 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 test/attach.test.js create mode 100644 test/limit.test.js diff --git a/README.md b/README.md index ef5b08b5e..1e563446b 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,15 @@ +# Quick instructions for Grist devs + +1. Run `make` to install dependencies and build. Delete the `package-lock.json` which this creates. +2. Possibly rename `build-tmp-napi-v3` or something to `build`. Don't know why it isn't just `build` and if this is system-specific. +3. Run `npm run rebuild-tests` which puts files in `test/cpp`, particularly `marshal.node`. +4. Run `npm test`. A failure in the test `respects ELECTRON_VERSION` is OK. +5. Maybe run `npm pack`? Not sure if needed. +6. Run `npm publish`. + +---- +---- + Asynchronous, non-blocking [SQLite3](https://sqlite.org/) bindings for [Node.js](http://nodejs.org/). [![NPM](https://nodei.co/npm/sqlite3.png?downloads=true&downloadRank=true)](https://nodei.co/npm/sqlite3/) diff --git a/deps/sqlite3.gyp b/deps/sqlite3.gyp index 54440690f..a382565eb 100755 --- a/deps/sqlite3.gyp +++ b/deps/sqlite3.gyp @@ -88,7 +88,8 @@ 'SQLITE_ENABLE_FTS4', 'SQLITE_ENABLE_FTS5', 'SQLITE_ENABLE_JSON1', - 'SQLITE_ENABLE_RTREE' + 'SQLITE_ENABLE_RTREE', + 'SQLITE_ENABLE_DBSTAT_VTAB=1', ], }, 'cflags_cc': [ @@ -102,7 +103,8 @@ 'SQLITE_ENABLE_FTS4', 'SQLITE_ENABLE_FTS5', 'SQLITE_ENABLE_JSON1', - 'SQLITE_ENABLE_RTREE' + 'SQLITE_ENABLE_RTREE', + 'SQLITE_ENABLE_DBSTAT_VTAB=1', ], 'export_dependent_settings': [ 'action_before_build', diff --git a/package.json b/package.json index 4bffe4acf..3e4cb2cd5 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@gristlabs/sqlite3", "description": "Asynchronous, non-blocking SQLite3 bindings", - "version": "4.1.1-grist.1", + "version": "4.1.1-grist.6", "homepage": "https://github.com/mapbox/node-sqlite3", "author": { "name": "MapBox", diff --git a/src/database.cc b/src/database.cc index 59d31a78d..5abc427e9 100644 --- a/src/database.cc +++ b/src/database.cc @@ -365,6 +365,22 @@ Napi::Value Database::Configure(const Napi::CallbackInfo& info) { baton->status = info[1].As().Int32Value(); db->Schedule(SetBusyTimeout, baton); } + else if (info[0].StrictEquals( Napi::String::New(env, "limit"))) { + REQUIRE_ARGUMENTS(3); + if (!info[1].IsNumber()) { + Napi::TypeError::New(env, "limit id must be an integer").ThrowAsJavaScriptException(); + return env.Null(); + } + if (!info[2].IsNumber()) { + Napi::TypeError::New(env, "limit value must be an integer").ThrowAsJavaScriptException(); + return env.Null(); + } + Napi::Function handle; + int id = info[1].As().Int32Value(); + int value = info[2].As().Int32Value(); + Baton* baton = new LimitBaton(db, handle, id, value); + db->Schedule(SetLimit, baton); + } else { Napi::TypeError::New(env, (StringConcat( #if V8_MAJOR_VERSION > 6 @@ -409,6 +425,15 @@ void Database::SetBusyTimeout(Baton* baton) { delete baton; } +void Database::SetLimit(Baton* b) { + std::unique_ptr baton(static_cast(b)); + + assert(baton->db->open); + assert(baton->db->_handle); + + sqlite3_limit(baton->db->_handle, baton->id, baton->value); +} + void Database::RegisterTraceCallback(Baton* baton) { assert(baton->db->open); assert(baton->db->_handle); diff --git a/src/database.h b/src/database.h index b31f68136..45063b57b 100644 --- a/src/database.h +++ b/src/database.h @@ -71,6 +71,13 @@ class Database : public Napi::ObjectWrap { Baton(db_, cb_), filename(filename_) {} }; + struct LimitBaton : Baton { + int id; + int value; + LimitBaton(Database* db_, Napi::Function cb_, int id_, int value_) : + Baton(db_, cb_), id(id_), value(value_) {} + }; + typedef void (*Work_Callback)(Baton* baton); struct Call { @@ -160,6 +167,7 @@ class Database : public Napi::ObjectWrap { Napi::Value Interrupt(const Napi::CallbackInfo& info); static void SetBusyTimeout(Baton* baton); + static void SetLimit(Baton* baton); static void RegisterTraceCallback(Baton* baton); static void TraceCallback(void* db, const char* sql); diff --git a/src/node_sqlite3.cc b/src/node_sqlite3.cc index b101b451f..6f47a68a8 100644 --- a/src/node_sqlite3.cc +++ b/src/node_sqlite3.cc @@ -61,6 +61,19 @@ Napi::Object RegisterModule(Napi::Env env, Napi::Object exports) { DEFINE_CONSTANT_INTEGER(exports, SQLITE_FORMAT, FORMAT) DEFINE_CONSTANT_INTEGER(exports, SQLITE_RANGE, RANGE) DEFINE_CONSTANT_INTEGER(exports, SQLITE_NOTADB, NOTADB) + + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_LENGTH, LIMIT_LENGTH) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_SQL_LENGTH, LIMIT_SQL_LENGTH) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_COLUMN, LIMIT_COLUMN) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_EXPR_DEPTH, LIMIT_EXPR_DEPTH) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_COMPOUND_SELECT, LIMIT_COMPOUND_SELECT) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_VDBE_OP, LIMIT_VDBE_OP) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_FUNCTION_ARG, LIMIT_FUNCTION_ARG) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_ATTACHED, LIMIT_ATTACHED) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_LIKE_PATTERN_LENGTH, LIMIT_LIKE_PATTERN_LENGTH) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_VARIABLE_NUMBER, LIMIT_VARIABLE_NUMBER) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_TRIGGER_DEPTH, LIMIT_TRIGGER_DEPTH) + DEFINE_CONSTANT_INTEGER(exports, SQLITE_LIMIT_WORKER_THREADS, LIMIT_WORKER_THREADS) }); return exports; diff --git a/test/attach.test.js b/test/attach.test.js new file mode 100644 index 000000000..d5910fc94 --- /dev/null +++ b/test/attach.test.js @@ -0,0 +1,25 @@ +var sqlite3 = require('..'); +var helper = require('./support/helper'); + +// TODO turns out that disabling ATTACH causes other problems, so it's not disabled any more, for now +describe.skip('attach', function() { + // Check that ATTACH is not supported, as part of defense in depth measures. + it ('does not permit attaching another db', function(done) { + helper.ensureExists('test/tmp/'); + helper.deleteFile('test/tmp/test_attach.db'); + var db = new sqlite3.Database('test/tmp/test_attach.db', function(err) { + if (err) throw err; + db.exec("ATTACH 'test/support/prepare.db' AS zing", function (err) { + if (!err) { + throw new Error('ATTACH should not succeed'); + } + if (err.errno === sqlite3.ERROR && + err.message === 'SQLITE_ERROR: too many attached databases - max 0') { + db.close(done); + } else { + throw err; + } + }); + }); + }); +}); diff --git a/test/limit.test.js b/test/limit.test.js new file mode 100644 index 000000000..d79f2c8df --- /dev/null +++ b/test/limit.test.js @@ -0,0 +1,28 @@ +var sqlite3 = require('..'); + +describe('limit', function() { + var db; + + before(function(done) { + db = new sqlite3.Database(':memory:', done); + }); + + it('should support applying limits via configure', function(done) { + db.configure('limit', sqlite3.LIMIT_ATTACHED, 0); + db.exec("ATTACH 'test/support/prepare.db' AS zing", function(err) { + if (!err) { + throw new Error('ATTACH should not succeed'); + } + if (err.errno === sqlite3.ERROR && + err.message === 'SQLITE_ERROR: too many attached databases - max 0') { + db.configure('limit', sqlite3.LIMIT_ATTACHED, 1); + db.exec("ATTACH 'test/support/prepare.db' AS zing", function(err) { + if (err) throw err; + db.close(done); + }); + } else { + throw err; + } + }); + }); +}); From 78d29867e9e77f161dd4365ad18628694f8bfd88 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 15:10:52 -0500 Subject: [PATCH 16/33] clean up after merge --- README.md | 6 ------ package.json | 3 ++- scripts/prepare_for_test.sh | 5 +++++ src/statement.cc | 2 +- test/electron.test.js | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index aa4fae63c..1c48dad46 100644 --- a/README.md +++ b/README.md @@ -234,7 +234,6 @@ npm install sqlite3 --build-from-source --sqlite_libname=sqlcipher --sqlite=`bre # Testing (updated for marshalling) -<<<<<<< HEAD [mocha](https://github.com/visionmedia/mocha) is required to run unit tests. In sqlite3's directory (where its `package.json` resides) run the following: @@ -242,11 +241,6 @@ In sqlite3's directory (where its `package.json` resides) run the following: npm install mocha npm run rebuild-tests #rebuilds marshal hooks npm test -======= -```bash -npm test -``` ->>>>>>> upstream/master # Contributors diff --git a/package.json b/package.json index 9e7564005..e9be73e82 100644 --- a/package.json +++ b/package.json @@ -55,6 +55,7 @@ "devDependencies": { "eslint": "6.8.0", "mocha": "7.2.0", + "bindings": "^1.3.0", "node-pre-gyp-github": "1.4.4" }, "peerDependencies": { @@ -74,7 +75,7 @@ "install": "node-pre-gyp install --fallback-to-build", "pretest": "node test/support/createdb.js", "test": "./scripts/prepare_for_test.sh; mocha -R spec --timeout 480000", - "rebuild-tests": "node-gyp rebuild --directory test/cpp", + "rebuild-tests": "./scripts/prepare_for_test.sh; node-gyp rebuild --directory test/cpp", "pack": "node-pre-gyp package" }, "license": "BSD-3-Clause", diff --git a/scripts/prepare_for_test.sh b/scripts/prepare_for_test.sh index b306e2364..ba209ef46 100755 --- a/scripts/prepare_for_test.sh +++ b/scripts/prepare_for_test.sh @@ -15,3 +15,8 @@ if [ ! -e lib/binding/$expected/node_sqlite3.node ]; then ln -s ../$target $expected/node_sqlite3.node echo Added lib/binding/$expected/node_sqlite3.node fi + +if [ ! -e build ]; then + ln -s build-tmp-napi-v6 build + echo Added build +fi diff --git a/src/statement.cc b/src/statement.cc index a5eb3186b..c882308b6 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -19,7 +19,7 @@ Napi::Object Statement::Init(Napi::Env env, Napi::Object exports) { InstanceMethod("get", &Statement::Get, napi_default_method), InstanceMethod("run", &Statement::Run, napi_default_method), InstanceMethod("all", &Statement::All, napi_default_method), - //InstanceMethod("allMarshal", &Statement::AllMarshal), + InstanceMethod("allMarshal", &Statement::AllMarshal, napi_default_method), InstanceMethod("each", &Statement::Each, napi_default_method), InstanceMethod("reset", &Statement::Reset, napi_default_method), InstanceMethod("finalize", &Statement::Finalize_, napi_default_method), diff --git a/test/electron.test.js b/test/electron.test.js index 24b7abf26..7b0386e8e 100644 --- a/test/electron.test.js +++ b/test/electron.test.js @@ -1,6 +1,6 @@ var assert = require('assert'); -describe('electron', function() { +describe.skip('electron', function() { it('respects ELECTRON_VERSION', function() { process.env.ELECTRON_VERSION = '1.2.3'; let name = require.resolve('..'); From 1caa35e41f24bf37e38e29af9a57c9c63ede591f Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 15:47:49 -0500 Subject: [PATCH 17/33] prepare for prebuilding workflow --- .github/workflows/ci.yml | 18 +++++++----------- package.json | 8 ++++---- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a68cc5e25..83d393e65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,7 @@ on: pull_request: push: branches: - - master + - grist-test tags: - '*' env: @@ -19,23 +19,19 @@ jobs: fail-fast: false matrix: os: - - macos-latest +# - macos-latest - ubuntu-20.04 - - windows-latest +# - windows-latest host: - x64 target: - x64 node: - 10 - - 11 - - 12 - - 13 - - 14 - - 15 - - 16 - - 17 - - 18 +# - 12 +# - 14 +# - 16 +# - 18 include: - os: windows-latest node: 16 diff --git a/package.json b/package.json index e9be73e82..db8c569f5 100644 --- a/package.json +++ b/package.json @@ -1,8 +1,8 @@ { - "name": "@gristlabs/sqlite3", + "name": "@paulfitz/sqlite3", "description": "Asynchronous, non-blocking SQLite3 bindings", "version": "5.1.4-grist.1", - "homepage": "https://github.com/TryGhost/node-sqlite3", + "homepage": "https://github.com/paulfitz/node-sqlite3", "author": { "name": "Mapbox", "url": "https://mapbox.com/" @@ -10,7 +10,7 @@ "binary": { "module_name": "node_sqlite3", "module_path": "./lib/binding/napi-v{napi_build_version}-{platform}-{libc}-{arch}", - "host": "https://github.com/TryGhost/node-sqlite3/releases/download/", + "host": "https://github.com/paulfitz/node-sqlite3/releases/download/", "remote_path": "v{version}", "package_name": "napi-v{napi_build_version}-{platform}-{libc}-{arch}.tar.gz", "napi_versions": [ @@ -45,7 +45,7 @@ ], "repository": { "type": "git", - "url": "https://github.com/TryGhost/node-sqlite3.git" + "url": "https://github.com/paulfitz/node-sqlite3.git" }, "dependencies": { "@mapbox/node-pre-gyp": "^1.0.0", From c348a8789820d00ec408ae9a112cd3dd2fbe04c1 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 15:48:33 -0500 Subject: [PATCH 18/33] dev branch --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83d393e65..03c3d4227 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,7 @@ on: pull_request: push: branches: - - grist-test + - dev tags: - '*' env: From 8ea197ec8a95069f1d917b0837799cd1b4541f88 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 15:52:25 -0500 Subject: [PATCH 19/33] narrow to one job to start with --- .github/workflows/ci.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 03c3d4227..38dfd2fd1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,15 +32,15 @@ jobs: # - 14 # - 16 # - 18 - include: - - os: windows-latest - node: 16 - host: x86 - target: x86 - - os: macos-m1 - node: 16 - host: arm64 - target: arm64 +# include: +# - os: windows-latest +# node: 16 +# host: x86 +# target: x86 +# - os: macos-m1 +# node: 16 +# host: arm64 +# target: arm64 name: ${{ matrix.os }} (node=${{ matrix.node }}, host=${{ matrix.host }}, target=${{ matrix.target }}) steps: - uses: actions/checkout@v3 From 4553c58ad512335d85f68e3aa63e10642d6e9ed3 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 15:57:26 -0500 Subject: [PATCH 20/33] build marshaling test code --- scripts/prepare_for_test.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/prepare_for_test.sh b/scripts/prepare_for_test.sh index ba209ef46..42757e957 100755 --- a/scripts/prepare_for_test.sh +++ b/scripts/prepare_for_test.sh @@ -14,9 +14,12 @@ if [ ! -e lib/binding/$expected/node_sqlite3.node ]; then mkdir -p $expected ln -s ../$target $expected/node_sqlite3.node echo Added lib/binding/$expected/node_sqlite3.node + cd ../.. fi if [ ! -e build ]; then ln -s build-tmp-napi-v6 build echo Added build fi + +node-gyp rebuild --directory test/cpp From 5741c7c27edc0752d0aa70bccfb69ceda1924bb0 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 16:06:44 -0500 Subject: [PATCH 21/33] tweak preinclude path for use in tests --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38dfd2fd1..79731f91a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,8 +76,8 @@ jobs: - name: Add Linux env vars if: contains(matrix.os, 'ubuntu') run: | - echo "CFLAGS=${CFLAGS:-} -include ../src/gcc-preinclude.h" >> $GITHUB_ENV - echo "CXXFLAGS=${CXXFLAGS:-} -include ../src/gcc-preinclude.h" >> $GITHUB_ENV + echo "CFLAGS=${CFLAGS:-} -include ${PWD}/src/gcc-preinclude.h" >> $GITHUB_ENV + echo "CXXFLAGS=${CXXFLAGS:-} -include ${PWD}/src/gcc-preinclude.h" >> $GITHUB_ENV - name: Configure build run: yarn node-pre-gyp configure --target_arch=${{ env.TARGET }} From f367174d8b6d99cb13f114a57e55bb82edecb80a Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 16:15:35 -0500 Subject: [PATCH 22/33] thread the needle for testing --- src/gcc-preinclude.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/gcc-preinclude.h b/src/gcc-preinclude.h index f7e5ed59f..e3681a175 100644 --- a/src/gcc-preinclude.h +++ b/src/gcc-preinclude.h @@ -2,6 +2,8 @@ #if defined(__linux__) +#ifndef _GNU_SOURCE + #define _GNU_SOURCE #include #undef _GNU_SOURCE @@ -24,5 +26,7 @@ __asm__(".symver pow,pow@GLIBC_2.17"); __asm__(".symver fcntl64,fcntl@GLIBC_2.17"); #endif +#endif + #endif #endif From c4e5e03d4adc1460c9cd753b6630ca2bf493dfeb Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 17:26:39 -0500 Subject: [PATCH 23/33] tweak how sqlite is loaded in marshalling test --- test/marshal-test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/marshal-test.js b/test/marshal-test.js index 5b9ee2fb3..325b73f0a 100644 --- a/test/marshal-test.js +++ b/test/marshal-test.js @@ -7,7 +7,8 @@ const bindings = require('bindings'); const testRoot = path.resolve(__dirname, 'cpp'); const mainRoot = path.resolve(__dirname, '..'); -bindings({ module_root: mainRoot, bindings: 'node_sqlite3' }); +var sqlite3 = require('..'); +//bindings({ module_root: mainRoot, bindings: 'node_sqlite3' }); const marshal = bindings({ module_root: testRoot, bindings: 'marshal' }); describe('marshal', function() { From 6bb4eb135d959142be24849c7263ca5555a330df Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 17:32:13 -0500 Subject: [PATCH 24/33] try windows + mac --- .github/workflows/ci.yml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79731f91a..7dfd0e3bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,9 +19,9 @@ jobs: fail-fast: false matrix: os: -# - macos-latest + - macos-latest - ubuntu-20.04 -# - windows-latest + - windows-latest host: - x64 target: @@ -32,15 +32,15 @@ jobs: # - 14 # - 16 # - 18 -# include: -# - os: windows-latest -# node: 16 -# host: x86 -# target: x86 -# - os: macos-m1 -# node: 16 -# host: arm64 -# target: arm64 + include: + - os: windows-latest + node: 16 + host: x86 + target: x86 + - os: macos-m1 + node: 16 + host: arm64 + target: arm64 name: ${{ matrix.os }} (node=${{ matrix.node }}, host=${{ matrix.host }}, target=${{ matrix.target }}) steps: - uses: actions/checkout@v3 From 480948101cc6c56450f87fbcba3c53fd4ee2742a Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 17:49:07 -0500 Subject: [PATCH 25/33] try to sneak onto windows --- .github/workflows/ci.yml | 18 +++++++++--------- package.json | 6 +++--- scripts/prepare_for_test.sh | 2 -- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7dfd0e3bf..311c32d2c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,15 +32,15 @@ jobs: # - 14 # - 16 # - 18 - include: - - os: windows-latest - node: 16 - host: x86 - target: x86 - - os: macos-m1 - node: 16 - host: arm64 - target: arm64 +# include: +# - os: windows-latest +# node: 16 +# host: x86 +# target: x86 +# - os: macos-m1 +# node: 16 +# host: arm64 +# target: arm64 name: ${{ matrix.os }} (node=${{ matrix.node }}, host=${{ matrix.host }}, target=${{ matrix.target }}) steps: - uses: actions/checkout@v3 diff --git a/package.json b/package.json index db8c569f5..b1d54cc0a 100644 --- a/package.json +++ b/package.json @@ -73,9 +73,9 @@ "build": "node-pre-gyp build", "build:debug": "node-pre-gyp build --debug", "install": "node-pre-gyp install --fallback-to-build", - "pretest": "node test/support/createdb.js", - "test": "./scripts/prepare_for_test.sh; mocha -R spec --timeout 480000", - "rebuild-tests": "./scripts/prepare_for_test.sh; node-gyp rebuild --directory test/cpp", + "pretest": "( node test/support/createdb.js && ./scripts/prepare_for_test.sh ) || node-gyp rebuild --directory test/cpp", + "test": "mocha -R spec --timeout 480000", + "rebuild-tests": "node-gyp rebuild --directory test/cpp", "pack": "node-pre-gyp package" }, "license": "BSD-3-Clause", diff --git a/scripts/prepare_for_test.sh b/scripts/prepare_for_test.sh index 42757e957..38879432c 100755 --- a/scripts/prepare_for_test.sh +++ b/scripts/prepare_for_test.sh @@ -21,5 +21,3 @@ if [ ! -e build ]; then ln -s build-tmp-napi-v6 build echo Added build fi - -node-gyp rebuild --directory test/cpp From 8a17a309339f9a744631b535588b06e544bdc4ae Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 18:17:52 -0500 Subject: [PATCH 26/33] more windows games --- package.json | 2 +- scripts/prep.cmd | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100755 scripts/prep.cmd diff --git a/package.json b/package.json index b1d54cc0a..cbf815f2d 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,7 @@ "build": "node-pre-gyp build", "build:debug": "node-pre-gyp build --debug", "install": "node-pre-gyp install --fallback-to-build", - "pretest": "( node test/support/createdb.js && ./scripts/prepare_for_test.sh ) || node-gyp rebuild --directory test/cpp", + "pretest": "node test/support/createdb.js && ./scripts/prep.cmd && node-gyp rebuild --directory test/cpp", "test": "mocha -R spec --timeout 480000", "rebuild-tests": "node-gyp rebuild --directory test/cpp", "pack": "node-pre-gyp package" diff --git a/scripts/prep.cmd b/scripts/prep.cmd new file mode 100755 index 000000000..bfbd343b0 --- /dev/null +++ b/scripts/prep.cmd @@ -0,0 +1,36 @@ +echo >/dev/null # >nul & GOTO WINDOWS & rem ^ +echo 'Processing for Linux' + +# Marshalling tests need to use built node_sqlite3.node, but bindings helper +# doesn't look in the right place. There's probably a smart way to fix this, +# but since this only affects tests of our fork, this script just makes a +# link in the place where marshalling tests expect. + +set -e + +expected=$(echo "console.log('node-v' + process.versions.modules + '-' + process.platform + '-' + process.arch)" | node) +if [ ! -e lib/binding/$expected/node_sqlite3.node ]; then + cd lib/binding + target=$(find . -iname "node_sqlite3.node" | head -n1) + mkdir -p $expected + ln -s ../$target $expected/node_sqlite3.node + echo Added lib/binding/$expected/node_sqlite3.node + cd ../.. +fi + +if [ ! -e build ]; then + ln -s build-tmp-napi-v6 build + echo Added build +fi + +exit 0 + +- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + +:WINDOWS +echo "Processing for Windows" + +REM Do Windows CMD commands here... for example: +SET StartDir=%cd% + +REM Then, when all Windows commands are complete... the script is done. From e9972db22ff4554d8b044a815ae13427f2bc3c5c Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 30 Dec 2022 18:27:18 -0500 Subject: [PATCH 27/33] linking issue for marshal tests --- .github/workflows/ci.yml | 4 ++-- test/cpp/binding.gyp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 311c32d2c..c096fd74f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,8 +19,8 @@ jobs: fail-fast: false matrix: os: - - macos-latest - - ubuntu-20.04 +# - macos-latest +# - ubuntu-20.04 - windows-latest host: - x64 diff --git a/test/cpp/binding.gyp b/test/cpp/binding.gyp index d85a62f2f..f955e6ca9 100644 --- a/test/cpp/binding.gyp +++ b/test/cpp/binding.gyp @@ -4,7 +4,8 @@ "cflags" : ["-Wall", "-Wextra", "-Wno-unused-parameter"], "defines": [ "V8_DEPRECATION_WARNINGS=1" ], "conditions" : [ - ["OS=='linux'", {"libraries+": ["../../../build/<(PRODUCT_DIR)/node_sqlite3.node"] } ] + ["OS=='linux'", {"libraries+": ["../../../build/<(PRODUCT_DIR)/node_sqlite3.node"] } ], + ["OS=='win'", {"libraries+": ["../../../build/<(PRODUCT_DIR)/node_sqlite3.node"] } ] ], "include_dirs": [ " Date: Fri, 30 Dec 2022 18:34:06 -0500 Subject: [PATCH 28/33] tweak path --- test/cpp/binding.gyp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/binding.gyp b/test/cpp/binding.gyp index f955e6ca9..d5aa51faa 100644 --- a/test/cpp/binding.gyp +++ b/test/cpp/binding.gyp @@ -5,7 +5,7 @@ "defines": [ "V8_DEPRECATION_WARNINGS=1" ], "conditions" : [ ["OS=='linux'", {"libraries+": ["../../../build/<(PRODUCT_DIR)/node_sqlite3.node"] } ], - ["OS=='win'", {"libraries+": ["../../../build/<(PRODUCT_DIR)/node_sqlite3.node"] } ] + ["OS=='win'", {"libraries+": ["<(PRODUCT_DIR)/../../../../build/Release/node_sqlite3.node"] } ] ], "include_dirs": [ " Date: Sat, 31 Dec 2022 15:14:10 -0500 Subject: [PATCH 29/33] smash test binary into library because yolo --- package.json | 3 +-- src/node_sqlite3.cc | 62 +++++++++++++++++++++++++++++++++++++++++++- test/marshal-test.js | 8 +++--- 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index cbf815f2d..adbcbbc02 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,6 @@ "devDependencies": { "eslint": "6.8.0", "mocha": "7.2.0", - "bindings": "^1.3.0", "node-pre-gyp-github": "1.4.4" }, "peerDependencies": { @@ -73,7 +72,7 @@ "build": "node-pre-gyp build", "build:debug": "node-pre-gyp build --debug", "install": "node-pre-gyp install --fallback-to-build", - "pretest": "node test/support/createdb.js && ./scripts/prep.cmd && node-gyp rebuild --directory test/cpp", + "pretest": "node test/support/createdb.js && ./scripts/prep.cmd", "test": "mocha -R spec --timeout 480000", "rebuild-tests": "node-gyp rebuild --directory test/cpp", "pack": "node-pre-gyp package" diff --git a/src/node_sqlite3.cc b/src/node_sqlite3.cc index 6f47a68a8..44b1c551a 100644 --- a/src/node_sqlite3.cc +++ b/src/node_sqlite3.cc @@ -125,4 +125,64 @@ const char* sqlite_authorizer_string(int type) { } } -NODE_API_MODULE(node_sqlite3, RegisterModule) + +/******/ + +Napi::Value Serialize(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); + if (info.Length() > 0) { + Marshaller m; + m.marshalValue(info[0]); + const std::vector &buffer = m.getBuffer(); + Napi::Env env = info.Env(); + return Napi::Buffer::Copy(env, &buffer[0], buffer.size()); + } + return env.Null(); +} + +Napi::Value Parse(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); + if (info.Length() > 0) { + if (!info[0].IsBuffer()) { + Napi::Error::New(env, "Argument must be a buffer").ThrowAsJavaScriptException(); + return env.Null(); + } else { + Napi::Buffer buffer = info[0].As>(); + Napi::Value result = Unmarshaller::parse(info, buffer.Data(), buffer.Length()); + if (!result.IsEmpty()) { + return result; + } + } + } + return env.Null(); +} + +Napi::Value TestOppositeEndianness(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); + if (info.Length() > 0) { + marshalTestOppositeEndianness(info[0].As().Value()); + } + return env.Null(); +} + +Napi::Object Init(Napi::Env env, Napi::Object exports) { + exports.Set(Napi::String::New(env, "serialize"), + Napi::Function::New(env, Serialize)); + exports.Set(Napi::String::New(env, "parse"), + Napi::Function::New(env, Parse)); + exports.Set(Napi::String::New(env, "testOppositeEndianness"), + Napi::Function::New(env, TestOppositeEndianness)); + return exports; +} + + +Napi::Object RegisterBoth(Napi::Env env, Napi::Object exports) { + RegisterModule(env, exports); + return Init(env, exports); +} + + +/******/ + + +NODE_API_MODULE(node_sqlite3, RegisterBoth) diff --git a/test/marshal-test.js b/test/marshal-test.js index 325b73f0a..05b342aff 100644 --- a/test/marshal-test.js +++ b/test/marshal-test.js @@ -5,11 +5,13 @@ const assert = require('assert'); const util = require('util'); const bindings = require('bindings'); -const testRoot = path.resolve(__dirname, 'cpp'); -const mainRoot = path.resolve(__dirname, '..'); +//const testRoot = path.resolve(__dirname, 'cpp'); +//const mainRoot = path.resolve(__dirname, '..'); var sqlite3 = require('..'); //bindings({ module_root: mainRoot, bindings: 'node_sqlite3' }); -const marshal = bindings({ module_root: testRoot, bindings: 'marshal' }); +//const marshal = bindings({ module_root: testRoot, bindings: 'marshal' }); + +const marshal = sqlite3; describe('marshal', function() { function stringToArray(str) { From 9f795266ad2d29e8abd6d88768a3fc84f748750f Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Sat, 31 Dec 2022 15:14:34 -0500 Subject: [PATCH 30/33] try on 3 --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c096fd74f..311c32d2c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,8 +19,8 @@ jobs: fail-fast: false matrix: os: -# - macos-latest -# - ubuntu-20.04 + - macos-latest + - ubuntu-20.04 - windows-latest host: - x64 From c0c8b6535562945652298d7fbcd360027e77a3de Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Sat, 31 Dec 2022 15:20:12 -0500 Subject: [PATCH 31/33] remove unneeded require --- test/marshal-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/marshal-test.js b/test/marshal-test.js index 05b342aff..2409e0faa 100644 --- a/test/marshal-test.js +++ b/test/marshal-test.js @@ -3,7 +3,7 @@ const path = require('path'); const assert = require('assert'); const util = require('util'); -const bindings = require('bindings'); +//const bindings = require('bindings'); //const testRoot = path.resolve(__dirname, 'cpp'); //const mainRoot = path.resolve(__dirname, '..'); From 9167aade1a327f3674a335189cbe674f26931260 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Sat, 31 Dec 2022 15:27:09 -0500 Subject: [PATCH 32/33] yay, working, now switch to node 16 for packaging --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 311c32d2c..f1fe78ffc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,10 +27,10 @@ jobs: target: - x64 node: - - 10 +# - 10 # - 12 # - 14 -# - 16 + - 16 # - 18 # include: # - os: windows-latest From 8951ae42a8c7a64ff31e520cfa477a525c0001d2 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Sun, 1 Jan 2023 16:52:24 -0500 Subject: [PATCH 33/33] include windows x86 --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1fe78ffc..b56431eb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,11 +32,11 @@ jobs: # - 14 - 16 # - 18 -# include: -# - os: windows-latest -# node: 16 -# host: x86 -# target: x86 + include: + - os: windows-latest + node: 16 + host: x86 + target: x86 # - os: macos-m1 # node: 16 # host: arm64