diff --git a/bin/gen-api-package b/bin/gen-api-package index 18b8e5c..8215855 100755 --- 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 index f596626..d72a98a 100644 --- 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 index df65e16..54e138a 100644 --- 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 index 7637610..86db335 100644 --- 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 index d9e3b93..d96304f 100644 --- 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 index 4a49055..0e8cfce 100644 --- 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 index ba05f74..4067c5d 100644 --- 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 deleted file mode 100644 index c90ab63..0000000 --- a/templates/nodejs/service.js.mustache +++ /dev/null @@ -1,26 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -var protobufjs = require('protobufjs'); -var path = require('path'); -var builder = protobufjs.newBuilder({}); -builder.importRoot = path.join(__dirname, 'proto'); -{{#protoFiles}} -protobufjs.loadProtoFile( - path.join(__dirname, '{{{.}}}'), - builder); -{{/protoFiles}} -module.exports = builder.ns; diff --git a/test/api_repo.js b/test/api_repo.js index c794ce9..3d14e0c 100644 --- 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); }); diff --git a/test/fixtures/gax/nodejs/index.js b/test/fixtures/gax/nodejs/src/v2/index.js similarity index 52% rename from test/fixtures/gax/nodejs/index.js rename to test/fixtures/gax/nodejs/src/v2/index.js index 099d724..97746f1 100644 --- a/test/fixtures/gax/nodejs/index.js +++ b/test/fixtures/gax/nodejs/src/v2/index.js @@ -13,12 +13,27 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +'use strict'; -var apiClient = require('./src/foo_api'); -for (var key in apiClient) { - exports[key] = apiClient[key]; -} -var apiClient = require('./src/bar_api'); -for (var key in apiClient) { - exports[key] = apiClient[key]; +var fooApi = require('./foo_api'); +var barApi = require('./bar_api'); +var gax = require('google-gax'); +var extend = require('extend'); +var lodash = require('lodash'); + +function v2(options) { + options = extend({ + scopes: v2.ALL_SCOPES + }, options); + var gaxGrpc = gax.grpc(options); + var result = {}; + extend(result, fooApi(gaxGrpc)); + extend(result, barApi(gaxGrpc)); + return result; } +v2.SERVICE_ADDRESS = fooApi.SERVICE_ADDRESS; +v2.ALL_SCOPES = lodash.union( + fooApi.ALL_SCOPES, + barApi.ALL_SCOPES +); +module.exports = v2; diff --git a/test/fixtures/nodejs/package.json b/test/fixtures/nodejs/package.json index 3af3756..701c788 100644 --- a/test/fixtures/nodejs/package.json +++ b/test/fixtures/nodejs/package.json @@ -5,15 +5,10 @@ "author": "Google Inc.", "description": "GRPC library for service packager-unittest-v2", "license": "BSD-3-Clause", - "dependencies": { - "grpc": "^0.10.0", - "protobufjs": "^5.0.1", - "google-auth-library": "^0.9.2" - }, "main": "index.js", "files": [ "LICENSE", - "index.js", - "service.js" + "proto/**/*", + "index.js" ] } diff --git a/test/fixtures/proto-based/nodejs/README.md b/test/fixtures/proto-based/nodejs/README.md deleted file mode 100644 index 303083d..0000000 --- a/test/fixtures/proto-based/nodejs/README.md +++ /dev/null @@ -1,6 +0,0 @@ -# gRPC library for the packager-v2 service - -packager-unittest-v2 contains the IDL-generated [grpc][] library for the service: `packager-v2` in the [googleapis][] repository. - -[googleapis]:https://github.com/google/googleapis -[grpc]:http://www.grpc.io/docs/tutorials/basic/node.html diff --git a/test/fixtures/proto-based/nodejs/package.json b/test/fixtures/proto-based/nodejs/package.json deleted file mode 100644 index d7db9ef..0000000 --- a/test/fixtures/proto-based/nodejs/package.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "url": "https://github.com/google/googleapis", - "name": "packager-unittest-v2", - "version": "1.0.0", - "author": "Google Inc.", - "description": "GRPC library for service packager-unittest-v2", - "license": "BSD-3-Clause", - "dependencies": { - "grpc": "^0.10.0", - "protobufjs": "^5.0.1", - "google-auth-library": "^0.9.2" - }, - "main": "index.js", - "files": [ - "LICENSE", - "index.js", - "proto/**/*", - "service.js" - ] -} diff --git a/test/fixtures/proto-based/nodejs/service.js b/test/fixtures/proto-based/nodejs/service.js deleted file mode 100644 index da5b244..0000000 --- a/test/fixtures/proto-based/nodejs/service.js +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -var protobufjs = require('protobufjs'); -var path = require('path'); -var builder = protobufjs.newBuilder({}); -builder.importRoot = path.join(__dirname, 'proto'); -protobufjs.loadProtoFile( - path.join(__dirname, 'proto/foo.proto'), - builder); -protobufjs.loadProtoFile( - path.join(__dirname, 'proto/bar/baz.proto'), - builder); -module.exports = builder.ns; diff --git a/test/packager.js b/test/packager.js index 6e753ad..828a58f 100644 --- a/test/packager.js +++ b/test/packager.js @@ -366,48 +366,14 @@ describe('the nodejs package builder', function() { ], done); }); - it('should construct a proto-based nodejs package', function(done) { - var opts = _.merge({ - packageInfo: testPackageInfo, - nodejsUsePbjs: false, - top: path.join(top, 'nodejs') - }, templateDirs.nodejs); - opts.packageInfo.nodejsUseProtos = true; - opts.packageInfo.protoFiles = ['proto/foo.proto', 'proto/bar/baz.proto']; - var copies = [ - 'nodejs/index.js', - 'nodejs/PUBLISHING.md' - ]; - var checkCopies = function checkCopies(next) { - var checkACopy = genCopyCompareFunc(top); - var copyTasks = _.map(copies, checkACopy); - async.parallel(copyTasks, next); - }; - var expanded = [ - 'nodejs/package.json', - 'nodejs/service.js', - 'nodejs/README.md' - ]; - var compareWithFixture = genFixtureCompareFunc(top, ['proto-based']); - var checkExpanded = function checkExpanded(next) { - var expandTasks = _.map(expanded, compareWithFixture); - async.parallel(expandTasks, next); - }; - async.series([ - packager.nodejs.bind(null, opts), - checkCopies, - checkExpanded - ], done); - }); - it('should construct a gax package', function(done) { var opts = _.merge({ packageInfo: testPackageInfo, top: path.join(top, 'nodejs') }, {templateDir: path.join(__dirname, '..', 'templates', 'gax', 'nodejs')}); - opts.mockApiFilesForTest = ['foo_api.js', 'bar_api.js']; + opts.mockApiFilesForTest = ['v2/foo_api.js', 'v2/bar_api.js']; var expanded = [ - 'nodejs/index.js' + 'nodejs/src/v2/index.js' ]; var compareWithFixture = genFixtureCompareFunc(top, ['gax']); var checkExpanded = function checkExpanded(next) {