diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a6c6f37..5fdcc4d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,9 +20,9 @@ jobs: fail-fast: false matrix: node_version: - - 12 - 14 - 16 + - 18 opendds_branch: - master - latest-release @@ -33,6 +33,25 @@ jobs: - {os: macos-latest, dds_security: 0} - {os: windows-2019, dds_security: 1} - {os: windows-2019, dds_security: 0} + include: + - node_version: 8 + opendds_branch: latest-release + m: {os: ubuntu-latest, security: 0} + - node_version: 8 + opendds_branch: latest-release + m: {os: windows-2019, security: 0} + - node_version: 10 + opendds_branch: latest-release + m: {os: ubuntu-latest, security: 1} + - node_version: 10 + opendds_branch: latest-release + m: {os: windows-2019, security: 1} + - node_version: 12 + opendds_branch: latest-release + m: {os: ubuntu-latest, security: 1} + - node_version: 12 + opendds_branch: latest-release + m: {os: windows-2019, security: 1} runs-on: ${{ matrix.m.os }} steps: @@ -57,6 +76,10 @@ jobs: ref: ${{ matrix.opendds_branch }} path: OpenDDS fetch-depth: 1 + - name: 'Set Up node' + uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.node_version }} - name: 'Install ssl and xerces (ubuntu)' if: ${{ matrix.m.dds_security == 1 && matrix.m.os == 'ubuntu-latest' }} run: |- @@ -82,11 +105,6 @@ jobs: - name: 'Set Up Problem Matcher (ubuntu / macos)' if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }} uses: ammaraskar/gcc-problem-matcher@0.1 - - name: 'Set Up node (windows)' - if: ${{ matrix.m.os == 'windows-2019' }} - uses: actions/setup-node@v3 - with: - node-version: ${{ matrix.node_version }} - name: 'Set environment variables (ubuntu / macos)' if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }} shell: bash @@ -95,7 +113,6 @@ jobs: echo "TAO_ROOT=$GITHUB_WORKSPACE/ACE_TAO/TAO" >> $GITHUB_ENV echo "DDS_ROOT=$GITHUB_WORKSPACE/OpenDDS" >> $GITHUB_ENV echo "MPC_ROOT=$GITHUB_WORKSPACE/MPC" >> $GITHUB_ENV - echo "NVM_DIR=$GITHUB_WORKSPACE/nvm" >> $GITHUB_ENV echo "npm_config_devdir=$GITHUB_WORKSPACE/opendds-node-gyp-devdir" >> $GITHUB_ENV echo "LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$GITHUB_WORKSPACE/ACE_TAO/ACE/lib:$GITHUB_WORKSPACE/OpenDDS/lib" >> $GITHUB_ENV if [ ${{ matrix.node_version }} == 16 ]; then @@ -120,7 +137,6 @@ jobs: echo "TAO_ROOT=$GITHUB_WORKSPACE\\ACE_TAO\\TAO" >> $GITHUB_ENV echo "DDS_ROOT=$GITHUB_WORKSPACE\\OpenDDS" >> $GITHUB_ENV echo "MPC_ROOT=$GITHUB_WORKSPACE\\MPC" >> $GITHUB_ENV - echo "NVM_DIR=$GITHUB_WORKSPACE\\nvm" >> $GITHUB_ENV echo "npm_config_devdir=$GITHUB_WORKSPACE\\opendds-node-gyp-devdir" >> $GITHUB_ENV if [ ${{ matrix.m.dds_security }} == 1 ]; then CONFIG_OPTIONS+=" --security"; @@ -154,25 +170,10 @@ jobs: cd OpenDDS call setenv.cmd msbuild -p:Configuration=Debug,Platform=x64 -t:${{ env.BUILD_TARGETS }} -m DDS_TAOv2.sln - - name: 'Checkout nvm' - uses: actions/checkout@v3 - with: - repository: nvm-sh/nvm.git - path: ${{ env.NVM_DIR }} - ref: v0.39.0 - fetch-depth: 1 - - name: 'Install node, Build (ubuntu / macos)' + - name: 'Install (ubuntu / macos)' if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }} shell: bash run: |- - set -e - cd ${{ env.NVM_DIR }} - [ -s "./nvm.sh" ] && \. "./nvm.sh" # loads nvm - cd $GITHUB_WORKSPACE - nvm install ${{ matrix.node_version }} - nvm use ${{ matrix.node_version }} - echo "node:$(node -v) npm:$(npm -v)" - echo "Build ========== ========== ==========" npm install ${{ env.ACE_ROOT }}/bin/mwc.pl -type gnuace -exclude ACE_TAO,OpenDDS make -j2 || make @@ -180,12 +181,6 @@ jobs: if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }} shell: bash run: |- - set -e - cd ${{ env.NVM_DIR }} - [ -s "./nvm.sh" ] && \. "./nvm.sh" # loads nvm - cd $GITHUB_WORKSPACE - nvm use ${{ matrix.node_version }} - echo "node:$(node -v) npm:$(npm -v)" cd test ./run_test.pl cpp2node ./run_test.pl node2cpp @@ -197,12 +192,6 @@ jobs: if: ${{ (matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest') && matrix.m.dds_security == 1 }} shell: bash run: |- - set -e - cd ${{ env.NVM_DIR }} - [ -s "./nvm.sh" ] && \. "./nvm.sh" # loads nvm - cd $GITHUB_WORKSPACE - nvm use ${{ matrix.node_version }} - echo "node:$(node -v) npm:$(npm -v)" cd test ./run_test.pl cpp2node --rtps --secure ./run_test.pl node2cpp --rtps --secure diff --git a/README.md b/README.md index e57459d..1d61b5f 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ ** If using security, you may need to ensure that OpenDDS is built with the same version of OpenSSL used by Node.js ## Tested platforms: -* Node LTS versions 12, 14, 16 +* Node LTS versions 8, 10, 12, 14, 16, 18 * Linux (Ubuntu 20.04) x86_64 using gcc 9.4.0 * Linux (Ubuntu 22.04) x86_64 using gcc 11.2.0 (w/ openssl-1.1.1q) * Windows (Server 2022) x86_64 using Visual Studio Enterprise 2022 @@ -40,9 +40,11 @@ $ node-gyp configure build * Remove requirement for V8 TypeSupport generation and NodeQosConversion ** uses ValueReader/Writer Implementation for NaN / Node.js -** requires OpenDDS 3.22 +** requires OpenDDS 3.22, will not build with older versions of OpenDDS ** slightly changes QoS object formatting to align with IDL types -* Adds support and test coverage for Linux / MacOS / Windows +* Improved version support and CI test coverage +** Good Coverage of Node.js 14, 16, 18 (Linux / macOS / Windows) +** Limited Coverage of Node.js 8, 10, 12 (Linux / Windows) ### Version 0.1.1 diff --git a/src/NodeValueReader.cpp b/src/NodeValueReader.cpp index 8166add..84e8899 100644 --- a/src/NodeValueReader.cpp +++ b/src/NodeValueReader.cpp @@ -1,5 +1,31 @@ #include "NodeValueReader.h" +#include +#include + +#if NODE_MAJOR_VERSION > 10 || (NODE_MAJOR_VERSION == 10 && NODE_MINOR_VERSION >= 4) +#define HAS_BIGINT +#endif + +#ifdef HAS_BIGINT +// This is a bit of a hack to make Nan::To work for BigInt +// It should be OK as long as we avoid trying to reference BigInt before 10.4 +namespace Nan { +namespace imp { + +template<> +struct ToFactory : ToFactoryBase { + static inline return_t convert(v8::Local val) { + v8::Isolate *isolate = v8::Isolate::GetCurrent(); + v8::EscapableHandleScope scope(isolate); + return scope.Escape(val->ToBigInt(isolate->GetCurrentContext()).FromMaybe(v8::Local())); + } +}; + +} +} +#endif + namespace NodeOpenDDS { NodeValueReader::NodeValueReader(v8::Local obj) @@ -29,13 +55,9 @@ bool NodeValueReader::begin_struct_member(OpenDDS::XTypes::MemberId& member_id, Nan::MaybeLocal mlv = Nan::Get(property_names_.ToLocalChecked(), current_index_); if (!mlv.IsEmpty()) { current_property_name_ = mlv.ToLocalChecked(); - v8::Local name = v8::Local::Cast(current_property_name_); - if (!name.IsEmpty()) { - std::string str(name->Utf8Length(v8::Isolate::GetCurrent()), 0); - name->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]); - if (helper.get_value(member_id, str.c_str())) { - return true; - } + Nan::Utf8String name(current_property_name_); + if (helper.get_value(member_id, *name)) { + return true; } } ++current_index_; @@ -67,13 +89,9 @@ bool NodeValueReader::begin_discriminator() Nan::MaybeLocal mlv = Nan::Get(property_names_.ToLocalChecked(), current_index_); if (!mlv.IsEmpty()) { current_property_name_ = mlv.ToLocalChecked(); - v8::Local name = v8::Local::Cast(current_property_name_); - if (!name.IsEmpty()) { - std::string str(name->Utf8Length(v8::Isolate::GetCurrent()), 0); - name->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]); - if (str == "$discriminator") { - return true; - } + Nan::Utf8String name(current_property_name_); + if (std::strcmp(*name, "$discriminator") == 0) { + return true; } } ++current_index_; @@ -95,13 +113,9 @@ bool NodeValueReader::begin_union_member() Nan::MaybeLocal mlv = Nan::Get(property_names_.ToLocalChecked(), current_index_); if (!mlv.IsEmpty()) { current_property_name_ = mlv.ToLocalChecked(); - v8::Local name = v8::Local::Cast(current_property_name_); - if (!name.IsEmpty()) { - std::string str(name->Utf8Length(v8::Isolate::GetCurrent()), 0); - name->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]); - if (str != "$discriminator") { - return true; - } + Nan::Utf8String name(current_property_name_); + if (std::strcmp(*name, "$discriminator") != 0) { + return true; } } ++current_index_; @@ -192,10 +206,7 @@ bool NodeValueReader::read_uint16(ACE_CDR::UShort& value) bool NodeValueReader::read_int32(ACE_CDR::Long& value) { - bool result = primitive_helper(value, &v8::Value::IsNumber, strtol); - if (result) { - } - return result; + return primitive_helper(value, &v8::Value::IsNumber, strtol); } bool NodeValueReader::read_uint32(ACE_CDR::ULong& value) @@ -207,15 +218,15 @@ bool NodeValueReader::read_int64(ACE_CDR::LongLong& value) { Nan::MaybeLocal mlvai = use_name_ ? Nan::Get(current_object_, current_property_name_) : Nan::Get(current_object_, current_index_); if (!mlvai.IsEmpty()) { - // This doesn't work because NaN seems to lack conversion support to BigInt - // Not super surprising, since BigInt support wasn't introduced until Node.js 10.4 - //{ - // Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); - // if (!tov.IsEmpty()) { - // value = static_cast(tov.ToLocalChecked()->Int64Value()); - // return true; - // } - //} +#ifdef HAS_BIGINT + if (!mlvai.ToLocalChecked()->IsNumber()) { + Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); + if (!tov.IsEmpty()) { + value = static_cast(tov.ToLocalChecked()->Int64Value()); + return true; + } + } +#endif { Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); if (!tov.IsEmpty()) { @@ -233,15 +244,15 @@ bool NodeValueReader::read_uint64(ACE_CDR::ULongLong& value) { Nan::MaybeLocal mlvai = use_name_ ? Nan::Get(current_object_, current_property_name_) : Nan::Get(current_object_, current_index_); if (!mlvai.IsEmpty()) { - // This doesn't work because NaN seems to lack conversion support to BigInt - // Not super surprising, since BigInt support wasn't introduced until Node.js 10.4 - //{ - // Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); - // if (!tov.IsEmpty()) { - // value = static_cast(tov.ToLocalChecked()->UInt64Value()); - // return true; - // } - //} +#ifdef HAS_BIGINT + if (!mlvai.ToLocalChecked()->IsNumber()) { + Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); + if (!tov.IsEmpty()) { + value = static_cast(tov.ToLocalChecked()->Uint64Value()); + return true; + } + } +#endif { Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); if (!tov.IsEmpty()) { @@ -310,8 +321,8 @@ bool NodeValueReader::read_string(std::string& value) if (!mlvai.IsEmpty()) { Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); if (!tov.IsEmpty()) { - value.resize(tov.ToLocalChecked()->Utf8Length(v8::Isolate::GetCurrent()), 0); - tov.ToLocalChecked()->WriteUtf8(v8::Isolate::GetCurrent(), &value[0], -1, 0, v8::String::NO_NULL_TERMINATION); + Nan::Utf8String str(tov.ToLocalChecked()); + value = *str; return true; } } @@ -324,12 +335,9 @@ bool NodeValueReader::read_wstring(std::wstring& value) if (!mlvai.IsEmpty()) { Nan::MaybeLocal tov = Nan::To(mlvai.ToLocalChecked()); if (!tov.IsEmpty()) { - std::vector temp(tov.ToLocalChecked()->Length(), 0); - tov.ToLocalChecked()->Write(v8::Isolate::GetCurrent(), &temp[0], 0, -1, v8::String::NO_NULL_TERMINATION); - value.resize(temp.size(), 0); - for (size_t i = 0; i < temp.size(); ++i) { - value[i] = static_cast(temp[i]); - } + Nan::Utf8String str(tov.ToLocalChecked()); + std::wstring_convert, wchar_t> wconv; + value = wconv.from_bytes(*str); return true; } } diff --git a/src/NodeValueReader.h b/src/NodeValueReader.h index 44611bb..c5eb36e 100644 --- a/src/NodeValueReader.h +++ b/src/NodeValueReader.h @@ -114,10 +114,8 @@ class NodeValueReader : public OpenDDS::DCPS::ValueReader { } } if (lvai->IsString()) { - v8::Local ls = v8::Local::Cast(lvai); - std::string str(ls->Utf8Length(v8::Isolate::GetCurrent()), 0); - ls->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]); - safe_assign(value, (*str_conv)(str.c_str(), 0, str.find("0x") != std::string::npos ? 16 : 10)); + Nan::Utf8String str(lvai); + safe_assign(value, (*str_conv)(*str, 0, std::string(*str).find("0x") != std::string::npos ? 16 : 10)); return true; } } @@ -138,10 +136,8 @@ class NodeValueReader : public OpenDDS::DCPS::ValueReader { } } if (lvai->IsString()) { - v8::Local ls = v8::Local::Cast(lvai); - std::string str(ls->Utf8Length(v8::Isolate::GetCurrent()), 0); - ls->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]); - safe_assign(value, (*str_conv)(str.c_str(), 0)); + Nan::Utf8String str(lvai); + safe_assign(value, (*str_conv)(*str, 0)); return true; } } diff --git a/src/NodeValueWriter.cpp b/src/NodeValueWriter.cpp index 9c6af78..d9d2962 100644 --- a/src/NodeValueWriter.cpp +++ b/src/NodeValueWriter.cpp @@ -1,13 +1,24 @@ #include "NodeValueWriter.h" +#if NODE_MAJOR_VERSION > 10 || (NODE_MAJOR_VERSION == 10 && NODE_MINOR_VERSION >= 4) +#define HAS_BIGINT +#endif + namespace NodeOpenDDS { namespace { const int64_t NODE_MAX_SAFE_INT = 9007199254740991; } -NodeValueWriter::NodeValueWriter() : next_index_(0) +NodeValueWriter::NodeValueWriter() + : next_index_(0) + , use_bigint_(true) +{ +} + +void NodeValueWriter::use_bigint(bool value) { + use_bigint_ = value; } void NodeValueWriter::begin_struct() @@ -148,19 +159,19 @@ void NodeValueWriter::write_int64(ACE_CDR::LongLong value) return; } - // If we decide not to use BigInt - char buff[21]; // 2^63 is 19 characters long in decimal representation, plus optional sign, plus null -#ifndef ACE_LINUX - std::sprintf(buff, "%lld", value); -#else - std::sprintf(buff, "%ld", value); +#ifdef HAS_BIGINT + if (use_bigint_) { + v8::MaybeLocal v8_value = v8::BigInt::New(v8::Isolate::GetCurrent(), static_cast(value)); + value_helper(v8_value); + } else { +#endif + char buff[21]; // 2^63 is 19 characters long in decimal representation, plus optional sign, plus null + std::sprintf(buff, ACE_INT64_FORMAT_SPECIFIER_ASCII, value); + v8::MaybeLocal v8_value = Nan::New(buff, std::strlen(buff)); + value_helper(v8_value); +#ifdef HAS_BIGINT + } #endif - v8::MaybeLocal v8_value = Nan::New(buff, std::strlen(buff)); - value_helper(v8_value); - - // If we decide to use BigInt - //v8::MaybeLocal v8_value = v8::BigInt::New(v8::Isolate::GetCurrent(), static_cast(value)); - //value_helper(v8_value); } void NodeValueWriter::write_uint64(ACE_CDR::ULongLong value) @@ -170,19 +181,19 @@ void NodeValueWriter::write_uint64(ACE_CDR::ULongLong value) return; } - // If we decide not to use BigInt - char buff[21]; // 2^64 is 20 characters long in decimal representation, plus null -#ifndef ACE_LINUX - std::sprintf(buff, "%llu", value); -#else - std::sprintf(buff, "%lu", value); +#ifdef HAS_BIGINT + if (use_bigint_) { + v8::MaybeLocal v8_value = v8::BigInt::NewFromUnsigned(v8::Isolate::GetCurrent(), static_cast(value)); + value_helper(v8_value); + } else { +#endif + char buff[21]; // 2^64 is 20 characters long in decimal representation, plus null + std::sprintf(buff, ACE_UINT64_FORMAT_SPECIFIER_ASCII, value); + v8::MaybeLocal v8_value = Nan::New(buff, std::strlen(buff)); + value_helper(v8_value); +#ifdef HAS_BIGINT + } #endif - v8::MaybeLocal v8_value = Nan::New(buff, std::strlen(buff)); - value_helper(v8_value); - - // If we decide to use BigInt - //v8::MaybeLocal v8_value = v8::BigInt::NewFromUnsigned(v8::Isolate::GetCurrent(), static_cast(value)); - //value_helper(v8_value); } void NodeValueWriter::write_float32(ACE_CDR::Float value) diff --git a/src/NodeValueWriter.h b/src/NodeValueWriter.h index 7cc8ebd..d82e346 100644 --- a/src/NodeValueWriter.h +++ b/src/NodeValueWriter.h @@ -15,6 +15,8 @@ class NodeValueWriter : public OpenDDS::DCPS::ValueWriter { public: NodeValueWriter(); + void use_bigint(bool value); + void begin_struct(); void end_struct(); void begin_struct_member(const DDS::MemberDescriptor& descriptor); @@ -115,6 +117,7 @@ class NodeValueWriter : public OpenDDS::DCPS::ValueWriter { OPENDDS_VECTOR(v8::Local) object_stack_; std::string next_key_; size_t next_index_; + bool use_bigint_; }; } // NodeOpenDDS