From 1eb113c61ed87dbc12d9e76649e206190ccd3c4a Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 27 Aug 2015 09:26:33 -0700 Subject: [PATCH 1/4] Add metadata echo functionality to interop server, and corresponding interop test --- src/node/interop/interop_client.js | 73 ++++++++++++++++++++++++++++ src/node/interop/interop_server.js | 48 ++++++++++++++++-- src/node/test/interop_sanity_test.js | 4 ++ 3 files changed, 120 insertions(+), 5 deletions(-) diff --git a/src/node/interop/interop_client.js b/src/node/interop/interop_client.js index 8fb8d66920681..14ae045275e7b 100644 --- a/src/node/interop/interop_client.js +++ b/src/node/interop/interop_client.js @@ -49,6 +49,9 @@ var AUTH_USER = ('155450119199-3psnrh1sdr3d8cpj1v46naggf81mhdnk' + var COMPUTE_ENGINE_USER = ('155450119199-r5aaqa2vqoa9g5mv2m6s3m1l293rlmel' + '@developer.gserviceaccount.com'); +var ECHO_INITIAL_KEY = "x-grpc-test-echo-initial"; +var ECHO_TRAILING_KEY = "x-grpc-test-echo-trailing-bin"; + /** * Create a buffer filled with size zeroes * @param {number} size The length of the buffer @@ -60,6 +63,27 @@ function zeroBuffer(size) { return zeros; } +/** + * This is used for testing functions with multiple asynchronous calls that + * can happen in different orders. This should be passed the number of async + * function invocations that can occur last, and each of those should call this + * function's return value + * @param {function()} done The function that should be called when a test is + * complete. + * @param {number} count The number of calls to the resulting function if the + * test passes. + * @return {function()} The function that should be called at the end of each + * sequence of asynchronous functions. + */ +function multiDone(done, count) { + return function() { + count -= 1; + if (count <= 0) { + done(); + } + }; +} + /** * Run the empty_unary test * @param {Client} client The client to test against @@ -271,6 +295,54 @@ function timeoutOnSleepingServer(client, done) { }); } +function customMetadata(client, done) { + done = multiDone(done, 5); + var metadata = new grpc.Metadata(); + metadata.set(ECHO_INITIAL_KEY, 'test_initial_metadata_value'); + metadata.set(ECHO_TRAILING_KEY, new Buffer('ababab', 'hex')); + var arg = { + response_type: 'COMPRESSABLE', + response_size: 314159, + payload: { + body: zeroBuffer(271828) + } + }; + var streaming_arg = { + payload: { + body: zeroBuffer(271828) + } + }; + var unary = client.unaryCall(arg, function(err, resp) { + assert.ifError(err); + done(); + }, metadata); + unary.on('metadata', function(metadata) { + assert.deepEqual(metadata.get(ECHO_INITIAL_KEY), + ['test_initial_metadata_value']); + done(); + }); + unary.on('status', function(status) { + var echo_trailer = status.metadata.get(ECHO_TRAILING_KEY); + assert(echo_trailer.length > 0); + assert.strictEqual(echo_trailer.toString('hex'), 'ababab'); + done(); + }); + var stream = client.fullDuplexCall(metadata); + stream.on('metadata', function(metadata) { + assert.deepEqual(metadata.get(ECHO_INITIAL_KEY), + ['test_initial_metadata_value']); + done(); + }); + stream.on('status', function(status) { + var echo_trailer = status.metadata.get(ECHO_TRAILING_KEY); + assert(echo_trailer.length > 0); + assert.strictEqual(echo_trailer.toString('hex'), 'ababab'); + done(); + }); + stream.write(streaming_arg); + stream.end(); +} + /** * Run one of the authentication tests. * @param {string} expected_user The expected username in the response @@ -358,6 +430,7 @@ var test_cases = { cancel_after_begin: cancelAfterBegin, cancel_after_first_response: cancelAfterFirstResponse, timeout_on_sleeping_server: timeoutOnSleepingServer, + custom_metadata: customMetadata, compute_engine_creds: _.partial(authTest, COMPUTE_ENGINE_USER, null), service_account_creds: _.partial(authTest, AUTH_USER, AUTH_SCOPE), jwt_token_creds: _.partial(authTest, AUTH_USER, null), diff --git a/src/node/interop/interop_server.js b/src/node/interop/interop_server.js index 99155e99584cb..c23f237680892 100644 --- a/src/node/interop/interop_server.js +++ b/src/node/interop/interop_server.js @@ -39,6 +39,9 @@ var _ = require('lodash'); var grpc = require('..'); var testProto = grpc.load(__dirname + '/test.proto').grpc.testing; +var ECHO_INITIAL_KEY = "x-grpc-test-echo-initial"; +var ECHO_TRAILING_KEY = "x-grpc-test-echo-trailing-bin"; + /** * Create a buffer filled with size zeroes * @param {number} size The length of the buffer @@ -50,6 +53,34 @@ function zeroBuffer(size) { return zeros; } +/** + * Echos a header metadata item as specified in the interop spec. + * @param {Call} call The call to echo metadata on + */ +function echoHeader(call) { + var echo_initial = call.metadata.get(ECHO_INITIAL_KEY); + if (echo_initial.length > 0) { + var response_metadata = new grpc.Metadata(); + response_metadata.set(ECHO_INITIAL_KEY, echo_initial[0]); + call.sendMetadata(response_metadata); + } +} + +/** + * Gets the trailer metadata that should be echoed when the call is done, + * as specified in the interop spec. + * @param {Call} call The call to get metadata from + * @return {grpc.Metadata} The metadata to send as a trailer + */ +function getEchoTrailer(call) { + var echo_trailer = call.metadata.get(ECHO_TRAILING_KEY); + var response_trailer = new grpc.Metadata(); + if (echo_trailer.length > 0) { + response_trailer.set(ECHO_TRAILING_KEY, echo_trailer[0]); + } + return response_trailer; +} + /** * Respond to an empty parameter with an empty response. * NOTE: this currently does not work due to issue #137 @@ -58,7 +89,8 @@ function zeroBuffer(size) { * or error */ function handleEmpty(call, callback) { - callback(null, {}); + echoHeader(call); + callback(null, {}, getEchoTrailer(call)); } /** @@ -68,6 +100,7 @@ function handleEmpty(call, callback) { * error */ function handleUnary(call, callback) { + echoHeader(call); var req = call.request; var zeros = zeroBuffer(req.response_size); var payload_type = req.response_type; @@ -75,7 +108,8 @@ function handleUnary(call, callback) { payload_type = ['COMPRESSABLE', 'UNCOMPRESSABLE'][Math.random() < 0.5 ? 0 : 1]; } - callback(null, {payload: {type: payload_type, body: zeros}}); + callback(null, {payload: {type: payload_type, body: zeros}}, + getEchoTrailer(call)); } /** @@ -85,12 +119,14 @@ function handleUnary(call, callback) { * error */ function handleStreamingInput(call, callback) { + echoHeader(call); var aggregate_size = 0; call.on('data', function(value) { aggregate_size += value.payload.body.length; }); call.on('end', function() { - callback(null, {aggregated_payload_size: aggregate_size}); + callback(null, {aggregated_payload_size: aggregate_size}, + getEchoTrailer(call)); }); } @@ -99,6 +135,7 @@ function handleStreamingInput(call, callback) { * @param {Call} call Call to handle */ function handleStreamingOutput(call) { + echoHeader(call); var req = call.request; var payload_type = req.response_type; if (payload_type === 'RANDOM') { @@ -113,7 +150,7 @@ function handleStreamingOutput(call) { } }); }); - call.end(); + call.end(getEchoTrailer(call)); } /** @@ -122,6 +159,7 @@ function handleStreamingOutput(call) { * @param {Call} call Call to handle */ function handleFullDuplex(call) { + echoHeader(call); call.on('data', function(value) { var payload_type = value.response_type; if (payload_type === 'RANDOM') { @@ -138,7 +176,7 @@ function handleFullDuplex(call) { }); }); call.on('end', function() { - call.end(); + call.end(getEchoTrailer(call)); }); } diff --git a/src/node/test/interop_sanity_test.js b/src/node/test/interop_sanity_test.js index 2ca07c1d50d28..d7b6f3316c876 100644 --- a/src/node/test/interop_sanity_test.js +++ b/src/node/test/interop_sanity_test.js @@ -90,4 +90,8 @@ describe('Interop tests', function() { interop_client.runTest(port, name_override, 'timeout_on_sleeping_server', true, true, done); }); + it.only('should pass custom_metadata', function(done) { + interop_client.runTest(port, name_override, 'custom_metadata', + true, true, done); + }); }); From e634f9afdf86df6e89e9a4722b956bd6154b41b0 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 27 Aug 2015 10:02:00 -0700 Subject: [PATCH 2/4] Fixed the tests --- src/node/interop/interop_client.js | 8 ++++---- src/node/interop/interop_server.js | 4 ++-- src/node/test/interop_sanity_test.js | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/node/interop/interop_client.js b/src/node/interop/interop_client.js index 14ae045275e7b..7945ab190ea25 100644 --- a/src/node/interop/interop_client.js +++ b/src/node/interop/interop_client.js @@ -49,8 +49,8 @@ var AUTH_USER = ('155450119199-3psnrh1sdr3d8cpj1v46naggf81mhdnk' + var COMPUTE_ENGINE_USER = ('155450119199-r5aaqa2vqoa9g5mv2m6s3m1l293rlmel' + '@developer.gserviceaccount.com'); -var ECHO_INITIAL_KEY = "x-grpc-test-echo-initial"; -var ECHO_TRAILING_KEY = "x-grpc-test-echo-trailing-bin"; +var ECHO_INITIAL_KEY = 'x-grpc-test-echo-initial'; +var ECHO_TRAILING_KEY = 'x-grpc-test-echo-trailing-bin'; /** * Create a buffer filled with size zeroes @@ -324,7 +324,7 @@ function customMetadata(client, done) { unary.on('status', function(status) { var echo_trailer = status.metadata.get(ECHO_TRAILING_KEY); assert(echo_trailer.length > 0); - assert.strictEqual(echo_trailer.toString('hex'), 'ababab'); + assert.strictEqual(echo_trailer[0].toString('hex'), 'ababab'); done(); }); var stream = client.fullDuplexCall(metadata); @@ -336,7 +336,7 @@ function customMetadata(client, done) { stream.on('status', function(status) { var echo_trailer = status.metadata.get(ECHO_TRAILING_KEY); assert(echo_trailer.length > 0); - assert.strictEqual(echo_trailer.toString('hex'), 'ababab'); + assert.strictEqual(echo_trailer[0].toString('hex'), 'ababab'); done(); }); stream.write(streaming_arg); diff --git a/src/node/interop/interop_server.js b/src/node/interop/interop_server.js index c23f237680892..762e67001337a 100644 --- a/src/node/interop/interop_server.js +++ b/src/node/interop/interop_server.js @@ -39,8 +39,8 @@ var _ = require('lodash'); var grpc = require('..'); var testProto = grpc.load(__dirname + '/test.proto').grpc.testing; -var ECHO_INITIAL_KEY = "x-grpc-test-echo-initial"; -var ECHO_TRAILING_KEY = "x-grpc-test-echo-trailing-bin"; +var ECHO_INITIAL_KEY = 'x-grpc-test-echo-initial'; +var ECHO_TRAILING_KEY = 'x-grpc-test-echo-trailing-bin'; /** * Create a buffer filled with size zeroes diff --git a/src/node/test/interop_sanity_test.js b/src/node/test/interop_sanity_test.js index d7b6f3316c876..804c1d45e4a8c 100644 --- a/src/node/test/interop_sanity_test.js +++ b/src/node/test/interop_sanity_test.js @@ -90,7 +90,7 @@ describe('Interop tests', function() { interop_client.runTest(port, name_override, 'timeout_on_sleeping_server', true, true, done); }); - it.only('should pass custom_metadata', function(done) { + it('should pass custom_metadata', function(done) { interop_client.runTest(port, name_override, 'custom_metadata', true, true, done); }); From dd7e017e305d149bca999f670159ad254a7d9598 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 27 Aug 2015 10:02:24 -0700 Subject: [PATCH 3/4] Fixed handling of binary metadata values --- src/node/ext/call.cc | 23 ++++++++++++----------- src/node/src/metadata.js | 4 +++- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/node/ext/call.cc b/src/node/ext/call.cc index 18858fa334ad8..fddc1e214f5b7 100644 --- a/src/node/ext/call.cc +++ b/src/node/ext/call.cc @@ -111,17 +111,19 @@ bool CreateMetadataArray(Handle metadata, grpc_metadata_array *array, NanAssignPersistent(*handle, value); resources->handles.push_back(unique_ptr( new PersistentHolder(handle))); - continue; + } else { + return false; } - } - if (value->IsString()) { - Handle string_value = value->ToString(); - NanUtf8String *utf8_value = new NanUtf8String(string_value); - resources->strings.push_back(unique_ptr(utf8_value)); - current->value = **utf8_value; - current->value_length = string_value->Length(); } else { - return false; + if (value->IsString()) { + Handle string_value = value->ToString(); + NanUtf8String *utf8_value = new NanUtf8String(string_value); + resources->strings.push_back(unique_ptr(utf8_value)); + current->value = **utf8_value; + current->value_length = string_value->Length(); + } else { + return false; + } } array->count += 1; } @@ -156,8 +158,7 @@ Handle ParseMetadata(const grpc_metadata_array *metadata_array) { } if (EndsWith(elem->key, "-bin")) { array->Set(index_map[elem->key], - MakeFastBuffer( - NanNewBufferHandle(elem->value, elem->value_length))); + NanNewBufferHandle(elem->value, elem->value_length)); } else { array->Set(index_map[elem->key], NanNew(elem->value)); } diff --git a/src/node/src/metadata.js b/src/node/src/metadata.js index 65fd91f367286..c1da70b1974f6 100644 --- a/src/node/src/metadata.js +++ b/src/node/src/metadata.js @@ -147,7 +147,9 @@ Metadata.prototype.getMap = function() { */ Metadata.prototype.clone = function() { var copy = new Metadata(); - copy._internal_repr = _.cloneDeep(this._internal_repr); + _.forOwn(this._internal_repr, function(value, key) { + copy._internal_repr[key] = _.clone(value); + }); return copy; }; From f20d7db554e1d1d3085b23c1b73391d078ab0e91 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 6 Oct 2015 16:51:50 -0700 Subject: [PATCH 4/4] Fixed issues with binary metadata type checking --- src/node/ext/call.cc | 5 +++-- src/node/src/metadata.js | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/node/ext/call.cc b/src/node/ext/call.cc index b08a9f96d8120..f98fe2463b694 100644 --- a/src/node/ext/call.cc +++ b/src/node/ext/call.cc @@ -168,8 +168,9 @@ Local ParseMetadata(const grpc_metadata_array *metadata_array) { } if (EndsWith(elem->key, "-bin")) { Nan::Set(array, index_map[elem->key], - Nan::CopyBuffer(elem->value, - elem->value_length).ToLocalChecked()); + MakeFastBuffer( + Nan::CopyBuffer(elem->value, + elem->value_length).ToLocalChecked())); } else { Nan::Set(array, index_map[elem->key], Nan::New(elem->value).ToLocalChecked()); diff --git a/src/node/src/metadata.js b/src/node/src/metadata.js index c1da70b1974f6..5c24e46c9b2c8 100644 --- a/src/node/src/metadata.js +++ b/src/node/src/metadata.js @@ -59,6 +59,7 @@ function normalizeKey(key) { function validate(key, value) { if (_.endsWith(key, '-bin')) { if (!(value instanceof Buffer)) { + console.log(value.constructor.toString()); throw new Error('keys that end with \'-bin\' must have Buffer values'); } } else { @@ -173,7 +174,9 @@ Metadata.prototype._getCoreRepresentation = function() { Metadata._fromCoreRepresentation = function(metadata) { var newMetadata = new Metadata(); if (metadata) { - newMetadata._internal_repr = _.cloneDeep(metadata); + _.forOwn(metadata, function(value, key) { + newMetadata._internal_repr[key] = _.clone(value); + }); } return newMetadata; };