Skip to content

THRIFT-5937: Fix generation of episodic nodejs for services #3361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CJCombrink wants to merge 1 commit into
base: master
Choose a base branch
from
+101 −14
Open

THRIFT-5937: Fix generation of episodic nodejs for services #3361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions compiler/cpp/src/thrift/generate/t_js_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ class t_js_generator : public t_oop_generator {
std::string render_includes();
std::string render_ts_includes();
std::string get_import_path(t_program* program);
std::string get_import_path(t_service* service);
std::string declare_field(t_field* tfield, bool init = false, bool obj = false);
std::string function_signature(t_function* tfunction,
std::string prefix = "",
Expand Down Expand Up @@ -640,6 +641,17 @@ string t_js_generator::get_import_path(t_program* program) {
return "./" + import_file_name_with_extension;
}

string t_js_generator::get_import_path(t_service* service) {
const string import_file_name(service->get_name());
const string import_file_name_with_extension = import_file_name + (gen_esm_ ? ".mjs" : ".js");

auto module_name_and_import_path_iterator = module_name_2_import_path.find(import_file_name);
if (module_name_and_import_path_iterator != module_name_2_import_path.end()) {
return module_name_and_import_path_iterator->second;
}
return "./" + import_file_name_with_extension;
}

/**
* Close up (or down) some filez.
*/
Expand Down Expand Up @@ -1275,7 +1287,9 @@ void t_js_generator::generate_service(t_service* tservice) {
f_service_ << js_includes() << '\n' << render_includes() << '\n';

if (gen_ts_) {
if (tservice->get_extends() != nullptr) {
if ((tservice->get_extends() != nullptr)
&& (module_name_2_import_path.find(tservice->get_extends()->get_name()) == module_name_2_import_path.end())) {
// Only add the reference if the service does not come from the episode
f_service_ts_ << "/// <reference path=\"" << tservice->get_extends()->get_name()
<< ".d.ts\" />" << '\n';
}
Expand Down Expand Up @@ -1328,15 +1342,15 @@ void t_js_generator::generate_service(t_service* tservice) {

if (gen_node_) {
if (tservice->get_extends() != nullptr) {
f_service_ << js_const_type_ << tservice->get_extends()->get_name() << " = require('./"
<< tservice->get_extends()->get_name() << "');" << '\n' << js_const_type_
f_service_ << js_const_type_ << tservice->get_extends()->get_name() << " = require('"
<< get_import_path(tservice->get_extends()) << "');" << '\n' << js_const_type_
<< tservice->get_extends()->get_name()
<< "Client = " << tservice->get_extends()->get_name() << ".Client;" << '\n'
<< js_const_type_ << tservice->get_extends()->get_name()
<< "Processor = " << tservice->get_extends()->get_name() << ".Processor;" << '\n';

f_service_ts_ << "import " << tservice->get_extends()->get_name() << " = require('./"
<< tservice->get_extends()->get_name() << "');" << '\n';
f_service_ts_ << "import " << tservice->get_extends()->get_name() << " = require('"
<< get_import_path(tservice->get_extends()) << "');" << '\n';
}

if (gen_esm_) {
Expand Down
36 changes: 35 additions & 1 deletion lib/nodejs/test/episodic-code-generation-test/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,32 @@ const assert = require("assert");
const test = require("tape");
const thrift = require("thrift");
const { program } = require("commander");
const tape = require("tape");

program
.option("--host <host>", "Set the thrift server host to connect", "localhost")
.option("--port <port>", "Set the thrift server port number to connect", 9090)
.option("--base <base>", "Set the base: 'pure', 'base' or 'extend'", "pure")
.parse(process.argv);

const Service = require("./gen-2/second-episode/gen-nodejs/Service");
const ServiceBase = require("types-package/first-episode/BaseService");
const ServicePure = require("./gen-2/second-episode/gen-nodejs/Service");
const ServiceExtended = require("./gen-2/second-episode/gen-nodejs/ExtendedService");
const Types = require("types-package/first-episode/Types_types");

const opts = program.opts();
const host = opts.host;
const port = opts.port;

let Service;
if (opts.base === "pure") {
Service = ServicePure;
} else if (opts.base === "base") {
Service = ServiceBase;
} else if (opts.base === "extend") {
Service = ServiceExtended;
}

const options = {
transport: thrift.TBufferedTransport,
protocol: thrift.TJSONProtocol,
Expand All @@ -58,6 +71,24 @@ const testDriver = function (client, callback) {
callback("Server successfully tested");
});
});

if (opts.base === "extend") {
test("NodeJS episodic compilation client-server extended test", function (assert) {
const type1Object = new Types.Type1();
type1Object.number = 42;
type1Object.message = "The answer";
client.testEpisodeExtend(type1Object, function (err, response) {
assert.error(err, "no callback error");
assert.equal(response.number, type1Object.number + 1);
assert.equal(
response.message,
type1Object.message + " [Hello from the extended server]",
);
assert.end();
callback("Extended Server successfully tested");
});
});
}
};

connection.on("error", function (err) {
Expand All @@ -71,6 +102,9 @@ runTests();
function runTests() {
testDriver(client, function (status) {
console.log(status);
});
tape.onFinish(function () {
console.log("Tests finished");
connection.destroy();
});
}
Expand Down
21 changes: 20 additions & 1 deletion lib/nodejs/test/episodic-code-generation-test/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,26 @@ const { program } = require("commander");

program
.option("--port <port>", "Set the thrift server port", 9090)
.option("--base <base>", "Set the base: 'pure', 'base' or 'extend'", "pure")
.parse(process.argv);

const Service = require("./gen-2/second-episode/gen-nodejs/Service");
const ServiceBase = require("types-package/first-episode/BaseService");
const ServicePure = require("./gen-2/second-episode/gen-nodejs/Service");
const ServiceExtended = require("./gen-2/second-episode/gen-nodejs/ExtendedService");
const Types = require("types-package/first-episode/Types_types");

const opts = program.opts();
const port = opts.port;

let Service;
if (opts.base === "pure") {
Service = ServicePure;
} else if (opts.base === "base") {
Service = ServiceBase;
} else if (opts.base === "extend") {
Service = ServiceExtended;
}

const options = {
transport: thrift.TBufferedTransport,
protocol: thrift.TJSONProtocol,
Expand All @@ -45,6 +57,13 @@ const ServiceHandler = {
receivedType1Object.message + " [Hello from the server]";
return type1Object;
},
testEpisodeExtend: function (receivedType1Object) {
const type1Object = new Types.Type1();
type1Object.number = receivedType1Object.number + 1;
type1Object.message =
receivedType1Object.message + " [Hello from the extended server]";
return type1Object;
},
};

const server = thrift.createServer(Service, ServiceHandler, options);
Expand Down
26 changes: 19 additions & 7 deletions lib/nodejs/test/testAll.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,23 @@ testServer()

testEpisodicCompilation()
{
local _server=$1
local _client=$2
echo " [Episodic Variant: ${_server}:${_client}] Testing Client(${_client})/Server(${_server})";
RET=0
if [ -n "${COVER}" ]; then
${ISTANBUL} cover ${EPISODIC_DIR}/server.js --dir ${REPORT_PREFIX}${COUNT} --handle-sigint &
${ISTANBUL} cover ${EPISODIC_DIR}/server.js --dir ${REPORT_PREFIX}${COUNT} --handle-sigint -- --base "${_server}" &
COUNT=$((COUNT+1))
else
node ${EPISODIC_DIR}/server.js &
node ${EPISODIC_DIR}/server.js --base "${_server}" &
fi
SERVERPID=$!
sleep 0.1
if [ -n "${COVER}" ]; then
${ISTANBUL} cover ${EPISODIC_DIR}/client.js --dir ${REPORT_PREFIX}${COUNT} || RET=1
${ISTANBUL} cover ${EPISODIC_DIR}/client.js --dir ${REPORT_PREFIX}${COUNT} -- --base "${_client}" || RET=1
COUNT=$((COUNT+1))
else
node ${EPISODIC_DIR}/client.js || RET=1
node ${EPISODIC_DIR}/client.js --base "${_client}" || RET=1
fi
kill -2 $SERVERPID || RET=1
wait $SERVERPID
Expand Down Expand Up @@ -105,17 +108,21 @@ TYPES_PACKAGE=${EPISODIC_DIR}/node_modules/types-package

# generate the first episode
mkdir --parents ${EPISODIC_DIR}/gen-1/first-episode
${THRIFT_COMPILER} -o ${EPISODIC_DIR}/gen-1/first-episode --gen js:node,thrift_package_output_directory=first-episode ${THRIFT_FILES_DIR}/Types.thrift
${THRIFT_COMPILER} -o ${EPISODIC_DIR}/gen-1/first-episode --gen js:node,ts,thrift_package_output_directory=first-episode ${THRIFT_FILES_DIR}/Types.thrift

# create a "package" from the first episode and "install" it, the episode file must be at the module root
mkdir --parents ${TYPES_PACKAGE}/first-episode
cp --force ${EPISODIC_DIR}/episodic_compilation.package.json ${TYPES_PACKAGE}/package.json
cp --force ${EPISODIC_DIR}/gen-1/first-episode/gen-nodejs/Types_types.js ${TYPES_PACKAGE}/first-episode/
cp --force ${EPISODIC_DIR}/gen-1/first-episode/gen-nodejs/Types_types.d.ts ${TYPES_PACKAGE}/first-episode/
cp --force ${EPISODIC_DIR}/gen-1/first-episode/gen-nodejs/BaseService.js ${TYPES_PACKAGE}/first-episode/
cp --force ${EPISODIC_DIR}/gen-1/first-episode/gen-nodejs/BaseService.d.ts ${TYPES_PACKAGE}/first-episode/
cp --force ${EPISODIC_DIR}/gen-1/first-episode/gen-nodejs/thrift.js.episode ${TYPES_PACKAGE}
rm --force --recursive ${EPISODIC_DIR}/gen-1

# generate the second episode
mkdir --parents ${EPISODIC_DIR}/gen-2/second-episode
${THRIFT_COMPILER} -o ${EPISODIC_DIR}/gen-2/second-episode --gen js:node,imports=${TYPES_PACKAGE} ${THRIFT_FILES_DIR}/Service.thrift
${THRIFT_COMPILER} -o ${EPISODIC_DIR}/gen-2/second-episode --gen js:node,ts,imports=${TYPES_PACKAGE} ${THRIFT_FILES_DIR}/Service.thrift
if [ -f ${EPISODIC_DIR}/gen-2/second-episode/Types_types.js ]; then
TESTOK=1
fi
Expand Down Expand Up @@ -148,7 +155,12 @@ do
done

# episodic compilation test
testEpisodicCompilation
echo "Testing Episode Compilation"
testEpisodicCompilation 'pure' 'pure' || TESTOK=1
testEpisodicCompilation 'base' 'base' || TESTOK=1
testEpisodicCompilation 'extend' 'extend' || TESTOK=1
testEpisodicCompilation 'extend' 'base' || TESTOK=1
testEpisodicCompilation 'base' 'pure' || TESTOK=1

if [ -n "${COVER}" ]; then
${ISTANBUL} report --dir "${DIR}/../coverage" --include "${DIR}/../coverage/report*/coverage.json" lcov cobertura html
Expand Down
4 changes: 4 additions & 0 deletions test/Service.thrift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ include "Types.thrift"
service Service {
Types.Type1 testEpisode(1:Types.Type1 arg)
}

service ExtendedService extends Types.BaseService {
Types.Type1 testEpisodeExtend(1:Types.Type1 arg)
}
4 changes: 4 additions & 0 deletions test/Types.thrift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,8 @@
struct Type1 {
1: i32 number,
2: string message,
}

service BaseService {
Type1 testEpisode(1:Type1 arg)
}