Merge pull request #10959 from murgatroid99/node_rollback_protobuf6

Switch Protobuf.js dependency back to version 5

package.json

@@ -34,7 +34,7 @@
     "lodash": "^4.15.0",
     "nan": "^2.0.0",
     "node-pre-gyp": "^0.6.0",
-    "protobufjs": "^6.7.0"
+    "protobufjs": "^5.0.0"
   },
   "devDependencies": {
     "async": "^2.0.1",

src/node/index.js

@@ -64,8 +64,6 @@ grpc.setDefaultRootsPem(fs.readFileSync(SSL_ROOTS_PATH, 'ascii'));
  *   Buffers. Defaults to false
  * - longsAsStrings: deserialize long values as strings instead of objects.
  *   Defaults to true
- * - enumsAsStrings: deserialize enum values as strings instead of numbers.
- *   Defaults to true
  * - deprecatedArgumentOrder: Use the beta method argument order for client
  *   methods, with optional arguments after the callback. Defaults to false.
  *   This option is only a temporary stopgap measure to smooth an API breakage.
@@ -99,10 +97,6 @@ exports.loadObject = function loadObject(value, options) {
   switch (protobufjsVersion) {
     case 6: return protobuf_js_6_common.loadObject(value, options);
     case 5:
-    var deprecation_message = 'Calling grpc.loadObject with an object ' +
-        'generated by ProtoBuf.js 5 is deprecated. Please upgrade to ' +
-        'ProtoBuf.js 6.';
-    common.log(grpc.logVerbosity.INFO, deprecation_message);
     return protobuf_js_5_common.loadObject(value, options);
     default:
     throw new Error('Unrecognized protobufjsVersion', protobufjsVersion);
@@ -111,19 +105,6 @@ exports.loadObject = function loadObject(value, options) {
 
 var loadObject = exports.loadObject;
 
-function applyProtoRoot(filename, root) {
-  if (_.isString(filename)) {
-    return filename;
-  }
-  filename.root = path.resolve(filename.root) + '/';
-  root.resolvePath = function(originPath, importPath, alreadyNormalized) {
-    return ProtoBuf.util.path.resolve(filename.root,
-                                      importPath,
-                                      alreadyNormalized);
-  };
-  return filename.file;
-}
-
 /**
  * Load a gRPC object from a .proto file. The options object can provide the
  * following options:
@@ -133,8 +114,6 @@ function applyProtoRoot(filename, root) {
  *   Buffers. Defaults to false
  * - longsAsStrings: deserialize long values as strings instead of objects.
  *   Defaults to true
- * - enumsAsStrings: deserialize enum values as strings instead of numbers.
- *   Defaults to true
  * - deprecatedArgumentOrder: Use the beta method argument order for client
  *   methods, with optional arguments after the callback. Defaults to false.
  *   This option is only a temporary stopgap measure to smooth an API breakage.
@@ -146,17 +125,31 @@ function applyProtoRoot(filename, root) {
  * @return {Object<string, *>} The resulting gRPC object
  */
 exports.load = function load(filename, format, options) {
-  /* Note: format is currently unused, because the API for loading a proto
-     file or a JSON file is identical in Protobuf.js 6. In the future, there is
-     still the possibility of adding other formats that would be loaded
-     differently */
   options = _.defaults(options, common.defaultGrpcOptions);
-  options.protobufjs_version = 6;
+  options.protobufjsVersion = 5;
-  var root = new ProtoBuf.Root();
+  if (!format) {
-  var parse_options = {keepCase: !options.convertFieldsToCamelCase};
+    format = 'proto';
-  return loadObject(root.loadSync(applyProtoRoot(filename, root),
+  }
-                                  parse_options),
+  var convertFieldsToCamelCaseOriginal = ProtoBuf.convertFieldsToCamelCase;
-                    options);
+  if(options && options.hasOwnProperty('convertFieldsToCamelCase')) {
+    ProtoBuf.convertFieldsToCamelCase = options.convertFieldsToCamelCase;
+  }
+  var builder;
+  try {
+    switch(format) {
+      case 'proto':
+      builder = ProtoBuf.loadProtoFile(filename);
+      break;
+      case 'json':
+      builder = ProtoBuf.loadJsonFile(filename);
+      break;
+      default:
+      throw new Error('Unrecognized format "' + format + '"');
+    }
+  } finally {
+    ProtoBuf.convertFieldsToCamelCase = convertFieldsToCamelCaseOriginal;
+  }
+  return loadObject(builder.ns, options);
 };
 
 var log_template = _.template(

src/node/src/protobuf_js_5_common.js

@@ -45,8 +45,7 @@ var client = require('./client');
  *     objects. Defaults to true
  * @return {function(Buffer):cls} The deserialization function
  */
-exports.deserializeCls = function deserializeCls(cls, binaryAsBase64,
+exports.deserializeCls = function deserializeCls(cls, options) {
-                                                 longsAsStrings) {
   /**
    * Deserialize a buffer to a message object
    * @param {Buffer} arg_buf The buffer to deserialize
@@ -55,7 +54,8 @@ exports.deserializeCls = function deserializeCls(cls, binaryAsBase64,
   return function deserialize(arg_buf) {
     // Convert to a native object with binary fields as Buffers (first argument)
     // and longs as strings (second argument)
-    return cls.decode(arg_buf).toRaw(binaryAsBase64, longsAsStrings);
+    return cls.decode(arg_buf).toRaw(options.binaryAsBase64,
+                                     options.longsAsStrings);
   };
 };
 
@@ -128,10 +128,10 @@ exports.getProtobufServiceAttrs = function getProtobufServiceAttrs(service,
       responseType: method.resolvedResponseType,
       requestSerialize: serializeCls(method.resolvedRequestType.build()),
       requestDeserialize: deserializeCls(method.resolvedRequestType.build(),
-                                         binaryAsBase64, longsAsStrings),
+                                         options),
       responseSerialize: serializeCls(method.resolvedResponseType.build()),
       responseDeserialize: deserializeCls(method.resolvedResponseType.build(),
-                                          binaryAsBase64, longsAsStrings)
+                                          options)
     };
   }));
 };

src/node/src/server.js

@@ -779,6 +779,11 @@ Server.prototype.addService = function(service, implementation) {
   });
 };
 
+var logAddProtoServiceDeprecationOnce = _.once(function() {
+    common.log(grpc.logVerbosity.INFO,
+               'Server#addProtoService is deprecated. Use addService instead');
+});
+
 /**
  * Add a proto service to the server, with a corresponding implementation
  * @deprecated Use grpc.load and Server#addService instead
@@ -790,8 +795,7 @@ Server.prototype.addProtoService = function(service, implementation) {
   var options;
   var protobuf_js_5_common = require('./protobuf_js_5_common');
   var protobuf_js_6_common = require('./protobuf_js_6_common');
-  common.log(grpc.logVerbosity.INFO,
+  logAddProtoServiceDeprecationOnce();
-             'Server#addProtoService is deprecated. Use addService instead');
   if (protobuf_js_5_common.isProbablyProtobufJs5(service)) {
     options = _.defaults(service.grpc_options, common.defaultGrpcOptions);
     this.addService(

src/node/test/common_test.js

@@ -37,16 +37,15 @@ var assert = require('assert');
 var _ = require('lodash');
 
 var common = require('../src/common');
-var protobuf_js_6_common = require('../src/protobuf_js_6_common');
+var protobuf_js_5_common = require('../src/protobuf_js_5_common');
 
-var serializeCls = protobuf_js_6_common.serializeCls;
+var serializeCls = protobuf_js_5_common.serializeCls;
-var deserializeCls = protobuf_js_6_common.deserializeCls;
+var deserializeCls = protobuf_js_5_common.deserializeCls;
 
 var ProtoBuf = require('protobufjs');
 
-var messages_proto = new ProtoBuf.Root();
+var messages_proto = ProtoBuf.loadProtoFile(
-messages_proto = messages_proto.loadSync(
+    __dirname + '/test_messages.proto').build();
-    __dirname + '/test_messages.proto', {keepCase: true}).resolveAll();
 
 var default_options = common.defaultGrpcOptions;
 
@@ -101,6 +100,7 @@ describe('Proto message long int serialize and deserialize', function() {
     var longNumDeserialize = deserializeCls(messages_proto.LongValues,
                                             num_options);
     var serialized = longSerialize({int_64: pos_value});
+    console.log(longDeserialize(serialized));
     assert.strictEqual(typeof longDeserialize(serialized).int_64, 'string');
     /* With the longsAsStrings option disabled, long values are represented as
      * objects with 3 keys: low, high, and unsigned */
@@ -136,7 +136,8 @@ describe('Proto message bytes serialize and deserialize', function() {
     var serialized = sequenceSerialize({repeated_field: [10]});
     assert.strictEqual(expected_serialize.compare(serialized), 0);
   });
-  it('should deserialize packed or unpacked repeated', function() {
+  // This tests a bug that was fixed in Protobuf.js 6
+  it.skip('should deserialize packed or unpacked repeated', function() {
     var expectedDeserialize = {
       bytes_field: new Buffer(''),
       repeated_field: [10]
@@ -155,7 +156,8 @@ describe('Proto message bytes serialize and deserialize', function() {
     assert.deepEqual(unpackedDeserialized, expectedDeserialize);
   });
 });
-describe('Proto message oneof serialize and deserialize', function() {
+// This tests a bug that was fixed in Protobuf.js 6
+describe.skip('Proto message oneof serialize and deserialize', function() {
   var oneofSerialize = serializeCls(messages_proto.OneOfValues);
   var oneofDeserialize = deserializeCls(
       messages_proto.OneOfValues, default_options);
@@ -193,7 +195,8 @@ describe('Proto message enum serialize and deserialize', function() {
     assert.deepEqual(enumDeserialize(nameSerialized),
                      enumDeserialize(numberSerialized));
   });
-  it('Should deserialize as a string the enumsAsStrings option', function() {
+  // This tests a bug that was fixed in Protobuf.js 6
+  it.skip('Should correctly handle the enumsAsStrings option', function() {
     var serialized = enumSerialize({enum_value: 'TWO'});
     var nameDeserialized = enumDeserialize(serialized);
     var numberDeserialized = enumIntDeserialize(serialized);

src/node/test/surface_test.js

@@ -43,9 +43,8 @@ var ProtoBuf = require('protobufjs');
 
 var grpc = require('..');
 
-var math_proto = new ProtoBuf.Root();
+var math_proto = ProtoBuf.loadProtoFile(__dirname +
-math_proto = math_proto.loadSync(__dirname +
+    '/../../proto/math/math.proto');
-    '/../../proto/math/math.proto', {keepCase: true});
 
 var mathService = math_proto.lookup('math.Math');
 var mathServiceAttrs = grpc.loadObject(
@@ -332,9 +331,7 @@ describe('Echo service', function() {
   var server;
   var client;
   before(function() {
-    var test_proto = new ProtoBuf.Root();
+    var test_proto = ProtoBuf.loadProtoFile(__dirname + '/echo_service.proto');
-    test_proto = test_proto.loadSync(__dirname + '/echo_service.proto',
-                                         {keepCase: true});
     var echo_service = test_proto.lookup('EchoService');
     var Client = grpc.loadObject(echo_service);
     server = new grpc.Server();
@@ -357,6 +354,13 @@ describe('Echo service', function() {
       done();
     });
   });
+  it('Should convert an undefined argument to default values', function(done) {
+    client.echo(undefined, function(error, response) {
+      assert.ifError(error);
+      assert.deepEqual(response, {value: '', value2: 0});
+      done();
+    });
+  });
 });
 describe('Generic client and server', function() {
   function toString(val) {
@@ -457,9 +461,7 @@ describe('Echo metadata', function() {
   var server;
   var metadata;
   before(function() {
-    var test_proto = new ProtoBuf.Root();
+    var test_proto = ProtoBuf.loadProtoFile(__dirname + '/test_service.proto');
-    test_proto = test_proto.loadSync(__dirname + '/test_service.proto',
-                                         {keepCase: true});
     var test_service = test_proto.lookup('TestService');
     var Client = grpc.loadObject(test_service);
     server = new grpc.Server();
@@ -560,9 +562,7 @@ describe('Client malformed response handling', function() {
   var client;
   var badArg = new Buffer([0xFF]);
   before(function() {
-    var test_proto = new ProtoBuf.Root();
+    var test_proto = ProtoBuf.loadProtoFile(__dirname + '/test_service.proto');
-    test_proto = test_proto.loadSync(__dirname + '/test_service.proto',
-                                         {keepCase: true});
     var test_service = test_proto.lookup('TestService');
     var malformed_test_service = {
       unary: {
@@ -669,9 +669,7 @@ describe('Server serialization failure handling', function() {
   var client;
   var server;
   before(function() {
-    var test_proto = new ProtoBuf.Root();
+    var test_proto = ProtoBuf.loadProtoFile(__dirname + '/test_service.proto');
-    test_proto = test_proto.loadSync(__dirname + '/test_service.proto',
-                                         {keepCase: true});
     var test_service = test_proto.lookup('TestService');
     var malformed_test_service = {
       unary: {
@@ -772,16 +770,13 @@ describe('Server serialization failure handling', function() {
   });
 });
 describe('Other conditions', function() {
-  var test_service;
   var Client;
   var client;
   var server;
   var port;
   before(function() {
-    var test_proto = new ProtoBuf.Root();
+    var test_proto = ProtoBuf.loadProtoFile(__dirname + '/test_service.proto');
-    test_proto = test_proto.loadSync(__dirname + '/test_service.proto',
+    var test_service = test_proto.lookup('TestService');
-                                         {keepCase: true});
-    test_service = test_proto.lookup('TestService');
     Client = grpc.loadObject(test_service);
     server = new grpc.Server();
     var trailer_metadata = new grpc.Metadata();
@@ -1121,15 +1116,12 @@ describe('Call propagation', function() {
   var proxy;
   var proxy_impl;
 
-  var test_service;
   var Client;
   var client;
   var server;
   before(function() {
-    var test_proto = new ProtoBuf.Root();
+    var test_proto = ProtoBuf.loadProtoFile(__dirname + '/test_service.proto');
-    test_proto = test_proto.loadSync(__dirname + '/test_service.proto',
+    var test_service = test_proto.lookup('TestService');
-                                         {keepCase: true});
-    test_service = test_proto.lookup('TestService');
     server = new grpc.Server();
     Client = grpc.loadObject(test_service);
     server.addService(Client.service, {

templates/package.json.template

@@ -36,7 +36,7 @@
       "lodash": "^4.15.0",
       "nan": "^2.0.0",
       "node-pre-gyp": "^0.6.0",
-      "protobufjs": "^6.7.0"
+      "protobufjs": "^5.0.0"
     },
     "devDependencies": {
       "async": "^2.0.1",