Update NodeJS packaging patterns. (#95)

- retire use_pbjs flag -- it's not used anymore, and it does not fit with the new pattern of Gapic code. - update the grpc package template: this fits with the proto packages used by gcloud-node. - update gax package template: this will work well with the new pattern of Gapic code, as you can see in googleapis/gapic-generator#392 or googleapis/google-cloud-node#1476 - add 'env' parameter to dependency_out protoc
googleapis · Sep 8, 2016 · ae2eaab · ae2eaab
1 parent fc717b8
commit ae2eaab
Show file tree

Hide file tree

Showing 15 changed files with 110 additions and 240 deletions.
diff --git a/bin/gen-api-package b/bin/gen-api-package
@@ -197,18 +197,6 @@ var parseArgs = function parseArgs() {
       dest: 'altJava'
     }
   );
-  cli.addArgument(
-    [ '--nodejs_use_pbjs' ],
-    {
-      defaultValue: false,
-      action: 'storeTrue',
-      help: 'When set, the nodejs grpc client is generated with the json data\n'
-            + ' generated from pbjs. Otherwise the client bundles .proto\n'
-            + ' files. Note that pbjs-based client does not work currently\n'
-            + ' (see https://github.com/googleapis/packman/issues/35).',
-      dest: 'nodejsUsePbjs'
-    }
-  );
   cli.addArgument(
     [ '--override_plugins' ],
     {

diff --git a/lib/api_repo.js b/lib/api_repo.js
@@ -24,14 +24,11 @@ var fs = require('fs-extra');
 var glob = require('glob');
 var packager = require('./packager');
 var path = require('path');
-var pbjs = require('protobufjs/cli/pbjs');
-var pbjsUtil = require('protobufjs/cli/pbjs/util');
 var readline = require('readline');
 var request = require('request');
 var tmp = require('tmp');
 
 var EventEmitter = require('events').EventEmitter;
-var ProtoBuf = require('protobufjs');
 var StreamZip = require('node-stream-zip');
 
 exports.ApiRepo = ApiRepo;
@@ -219,7 +216,7 @@ ApiRepo.prototype.buildPackages =
               packageInfo: that.packageInfo,
               generated: generated
             }, templateInfo[l]);
-            if (l === 'nodejs' && !that.opts.nodejsUsePbjs) {
+            if (l === 'nodejs') {
               opts.packageInfo.protoFiles = _.filter(generated, function(file) {
                 return file.match(/\.proto$/);
               });
@@ -444,8 +441,8 @@ ApiRepo.prototype._collectProtoDeps = function _collectProtoDeps(
     includePath, protoFile, done) {
   var deps = tmp.fileSync();
   var desc = tmp.fileSync();
-  var args = this.protoCompilerArgs.concat(
-      ['--dependency_out=' + deps.name, '-o', desc.name]);
+  var args = this.protoCompilerArgs ? this.protoCompilerArgs.split(' ') : [];
+  args.push('--dependency_out=' + deps.name, '-o', desc.name);
   if (includePath) {
     includePath.forEach(function(ipath) {
       args.push('-I', ipath);
@@ -454,7 +451,7 @@ ApiRepo.prototype._collectProtoDeps = function _collectProtoDeps(
   args.push(protoFile);
   // Invokes protoc with --dependency_out to generate the dependency
   // proto files.
-  execFile(this.protoCompiler, args, {}, function(err) {
+  execFile(this.protoCompiler, args, {env: this.env}, function(err) {
     if (err) {
       done(err);
     }
@@ -582,7 +579,7 @@ ApiRepo.prototype._buildProtos =
               var outDir = path.join(langTopDir, 'proto');
               async.map(
                   fullPathProtos,
-                  this._collectProtoDeps.bind(null, includePath),
+                  this._collectProtoDeps.bind(this, includePath),
                   function(err, fileLists) {
                     if (err) {
                       findOutputs(err);
@@ -615,22 +612,7 @@ ApiRepo.prototype._buildProtos =
           var fullPathProtos = [];
           function makeModule() {
             var includePath = _.union(that.includePath, that.repoDirs);
-            if (!that.opts.nodejsUsePbjs) {
-              makeProtoBasedNodeModule(fullPathProtos, includePath);
-              return;
-            }
-            var opts = {
-              root: that.repoDirs,
-              source: 'proto',
-              path: includePath
-            };
-
-            var builder = loadProtos(fullPathProtos, opts);
-            var outDir = path.join(that.outDir, language);
-            fs.mkdirsSync(outDir);
-            var servicePath = path.join(outDir, 'service.js');
-            var commonJS = pbjs.targets.commonjs(builder, opts);
-            fs.writeFile(servicePath, commonJS, findOutputs);
+            makeProtoBasedNodeModule(fullPathProtos, includePath);
           }
 
           this._findProtos(
@@ -640,7 +622,6 @@ ApiRepo.prototype._buildProtos =
                 next();
               }
           );
-          makeModule();
         }.bind(this);
         this._findProtos(name, version, makeNodeModule);
       } else {
@@ -675,12 +656,6 @@ ApiRepo.prototype._getPluginName = function _getPluginName(
  * @param {function(?Error)} done - The callback.
  */
 ApiRepo.prototype._checkDeps = function _checkDeps(opts, done) {
-  // If nodejs is the only language, there are no dependencies.
-  if (this.languages.length === 1 && this.languages.indexOf('nodejs') !== -1) {
-    done(null);
-    return;
-  }
-
   // If gaxDir is set, are no dependencies, as protoc is not run
   if (this.gaxDir) {
     done(null);
@@ -1058,41 +1033,3 @@ ApiRepo.prototype._makeProtocFunc = function _makeProtocFunc(opts, language) {
 
   return callProtoc;
 };
-
-/**
- * Helps construct a JSON representation each proto file for the nodejs build.
- *
- * @param {string[]} filenames - The list of files.
- * @param {object} opts - provides configuration info
- * @param {object} opts.root - a virtual root folder that contains all the protos
- * @param {object} opts.path - an array of folders where other protos reside
- *
- * @return {object} a ProtoBuf.Builder containing loaded representations of the
- * protos
- */
-function loadProtos(filenames, opts) {
-  opts = opts || [];
-  var builder = ProtoBuf.newBuilder();
-  var loaded = [];
-  builder.importRoot = opts.root;
-  filenames.forEach(function(filename) {
-    var data = pbjs.sources.proto.load(filename, opts, loaded);
-    builder.import(data, filename);
-  });
-  builder.resolveAll();
-  return builder;
-}
-
-/**
- * Replace isDescriptor with a version that always returns false.
- *
- * pbjs/util.isDescriptor excludes imports that in google/protobuf.
- *
- * However, the nodejs packages need to be self-contained, so we actually want
- * to include these.
- * @param {string} name - The name of the message.
- * @return {Boolean} Whether it is a descriptor or not.
- */
-pbjsUtil.isDescriptor = function(name) {
-  return false;
-};
diff --git a/lib/packager.js b/lib/packager.js
@@ -176,6 +176,12 @@ function removeMustacheExt(filePath) {
   return filePath.slice(0, extIndex);
 }
 
+function underscoreToCamelCase(str) {
+  return _.map(str.split('_'), function(segment) {
+    return segment.charAt(0).toUpperCase() + segment.slice(1);
+  }).join('');
+}
+
 /**
  * makePythonPackage creates a new python package.
  *
@@ -360,9 +366,6 @@ function makeNodejsPackage(opts, done) {
   opts = _.merge({}, settings.nodejs, opts);
   var tasks = [];
 
-  if (!opts.packageInfo.nodejsUsePbjs) {
-    opts.templates.push('service.js.mustache');
-  }
   // Move copyable files to the top-level dir.
   opts.copyables.forEach(function(f) {
     var src = path.join(opts.templateDir, f);
@@ -378,17 +381,38 @@ function makeNodejsPackage(opts, done) {
     var dstBase = removeMustacheExt(f);
     var tmpl = path.join(opts.templateDir, f);
     var dst = path.join(opts.top, dstBase);
-    tasks.push(expand(tmpl, dst, opts.packageInfo));
+    // index.js should reside among other .js files for GAX packages.
+    if (dstBase !== 'index.js') {
+      tasks.push(expand(tmpl, dst, opts.packageInfo));
+    }
   });
 
   function runTask(err, jsFiles) {
+    function underscoreToLowerCamelCase(str) {
+      var camelCase = underscoreToCamelCase(str);
+      return camelCase.charAt(0).toLowerCase() + camelCase.slice(1);
+    }
     if (err) {
       done(err);
       return;
     }
     opts.packageInfo.apiFiles = _.map(jsFiles, function(jsFile) {
-      return jsFile.replace(/\.js$/, '');
+      var filePath = path.basename(jsFile, '.js');
+      return {
+        name: underscoreToLowerCamelCase(filePath),
+        filePath: filePath
+      };
     });
+    if (opts.packageInfo.apiFiles.length > 0) {
+      _.last(opts.packageInfo.apiFiles).last = true;
+      opts.packageInfo.serviceAddressName = opts.packageInfo.apiFiles[0].name;
+      // Creating a task to bring index.js into the same directory where
+      // other generated .js files exist.
+      tasks.push(expand(path.join(opts.templateDir, 'index.js.mustache'),
+                        path.join(path.dirname(jsFiles[0]), 'index.js'),
+                        opts.packageInfo));
+    }
+    opts.packageInfo.singleFile = (opts.packageInfo.apiFiles.length === 1);
     async.series(tasks, function(err) {
       if (!err && opts.copyables.indexOf('PUBLISHING.md') !== -1) {
         console.log('The nodejs package', pkgName(opts.packageInfo),
@@ -403,7 +427,9 @@ function makeNodejsPackage(opts, done) {
   // find lib/*.js files for gax API packages to generate index.js properly.
   if (opts.mockApiFilesForTest) {
     // For tests, mock data is passed instead of scanning the filesystem.
-    runTask(null, opts.mockApiFilesForTest);
+    runTask(null, _.map(opts.mockApiFilesForTest, function(file) {
+      return path.join(opts.top, 'src', file);
+    }));
   } else {
     glob.glob('*.js', {cwd: path.join(opts.top, 'src')}, runTask);
   }
@@ -468,11 +494,6 @@ function makeRubyPackage(opts, done) {
    * @param {function(?Error, Object[])} done - The callback.
    */
   function collectApiClasses(done) {
-    function underscoreToCamelCase(str) {
-      return _.map(str.split('_'), function(segment) {
-        return segment.charAt(0).toUpperCase() + segment.slice(1);
-      }).join('');
-    }
     function rbFileToData(rbFile) {
       var pathComponents = _.map(
           rbFile.replace(/\.rb$/, '').split('/'), underscoreToCamelCase);

diff --git a/templates/gax/nodejs/index.js.mustache b/templates/gax/nodejs/index.js.mustache
@@ -13,10 +13,42 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+'use strict';
 
 {{#apiFiles}}
-var apiClient = require('./src/{{{.}}}');
-for (var key in apiClient) {
-  exports[key] = apiClient[key];
+var {{{name}}} = require('./{{{filePath}}}');
+{{/apiFiles}}
+var gax = require('google-gax');
+var extend = require('extend');
+{{^singleFile}}
+var lodash = require('lodash');
+{{/singleFile}}
+
+function {{{api.version}}}(options) {
+  options = extend({
+    scopes: {{{api.version}}}.ALL_SCOPES
+  }, options);
+  var gaxGrpc = gax.grpc(options);
+  {{#singleFile}}
+  return {{{apiFiles[0].name}}}(gaxGrpc);
+  {{/singleFile}}
+  {{^singleFile}}
+  var result = {};
+  {{#apiFiles}}
+  extend(result, {{{name}}}(gaxGrpc));
+  {{/apiFiles}}
+  return result;
+  {{/singleFile}}
 }
+{{{api.version}}}.SERVICE_ADDRESS = {{{serviceAddressName}}}.SERVICE_ADDRESS;
+{{#singleFile}}
+{{{api.version}}}.ALL_SCOPES = {{{serviceAddressName}}}.ALL_SCOPES;
+{{/singleFile}}
+{{^singleFile}}
+{{{api.version}}}.ALL_SCOPES = lodash.union(
+{{#apiFiles}}
+  {{{name}}}.ALL_SCOPES{{^last}},{{/last}}
 {{/apiFiles}}
+);
+{{/singleFile}}
+module.exports = {{{api.version}}};
diff --git a/templates/gax/nodejs/package.json.mustache b/templates/gax/nodejs/package.json.mustache
@@ -11,5 +11,5 @@
     "grpc": "^{{{dependencies.grpc.nodejs.version}}}",
     "{{{api.dependsOn}}}-{{{api.version}}}": "^{{{api.semantic_version}}}"
   },
-  "main": "index.js"
+  "main": "src/index.js"
 }
diff --git a/templates/nodejs/index.js b/templates/nodejs/index.js
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
-var grpc = require('grpc');
-exports.client = grpc.loadObject(require('./service'));
-exports.auth = require('google-auth-library');
+var path = require('path');
+
+module.exports = function(relative) {
+  return path.join(__dirname, 'proto', relative);
+};
diff --git a/templates/nodejs/package.json.mustache b/templates/nodejs/package.json.mustache
@@ -5,18 +5,10 @@
   "author": "Google Inc.",
   "description": "GRPC library for service {{{api.name}}}-{{{api.version}}}",
   "license": "{{{api.license}}}",
-  "dependencies": {
-    "grpc": "^{{{dependencies.grpc.nodejs.version}}}",
-    "protobufjs": "^{{{dependencies.protobuf.nodejs.version}}}",
-    "google-auth-library": "^{{{dependencies.auth.nodejs.version}}}"
-  },
   "main": "index.js",
   "files": [
     "LICENSE",
-    "index.js",
-{{#nodejsUseProtos}}
     "proto/**/*",
-{{/nodejsUseProtos}}
-    "service.js"
+    "index.js"
   ]
 }
diff --git a/templates/nodejs/service.js.mustache b/templates/nodejs/service.js.mustache
diff --git a/test/api_repo.js b/test/api_repo.js
@@ -100,13 +100,14 @@ describe('ApiRepo', function() {
   describe('on the test fixture repo with no plugins', function() {
     var fakes, repo; // eslint-disable-line
     describe('configured for nodejs', function() {
-      // TODO: add a test case for nodejsUsePbjs: false.
       beforeEach(function() {
+        fakes = addFakeBinsToPath('protoc');
         repo = new ApiRepo({
-          nodejsUsePbjs: true,
+          env: {PATH: fakes.path},
           includePath: [path.join(__dirname, 'fixtures', 'include')],
           languages: ['nodejs'],
-          templateRoot: path.join(__dirname, '..', 'templates')
+          templateRoot: path.join(__dirname, '..', 'templates'),
+          protoCompiler: 'echo'
         });
         getsGoodZipFrom(repo.zipUrl);
       });