Skip to content

Commit

Permalink
Update NodeJS packaging patterns.
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
jmuk committed Sep 7, 2016
1 parent 5a4b086 commit 830bd0c
Show file tree
Hide file tree
Showing 15 changed files with 103 additions and 229 deletions.
12 changes: 0 additions & 12 deletions bin/gen-api-package
Original file line number Diff line number Diff line change
Expand Up @@ -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' ],
{
Expand Down
61 changes: 2 additions & 59 deletions lib/api_repo.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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$/);
});
Expand Down Expand Up @@ -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(
Expand All @@ -640,7 +622,6 @@ ApiRepo.prototype._buildProtos =
next();
}
);
makeModule();
}.bind(this);
this._findProtos(name, version, makeNodeModule);
} else {
Expand Down Expand Up @@ -1058,41 +1039,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;
};
43 changes: 32 additions & 11 deletions lib/packager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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);
Expand All @@ -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),
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
38 changes: 35 additions & 3 deletions templates/gax/nodejs/index.js.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -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}}};
2 changes: 1 addition & 1 deletion templates/gax/nodejs/package.json.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
"grpc": "^{{{dependencies.grpc.nodejs.version}}}",
"{{{api.dependsOn}}}-{{{api.version}}}": "^{{{api.semantic_version}}}"
},
"main": "index.js"
"main": "src/index.js"
}
9 changes: 6 additions & 3 deletions templates/nodejs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
* 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);
};
l
10 changes: 1 addition & 9 deletions templates/nodejs/package.json.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
26 changes: 0 additions & 26 deletions templates/nodejs/service.js.mustache

This file was deleted.

2 changes: 0 additions & 2 deletions test/api_repo.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,8 @@ 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() {
repo = new ApiRepo({
nodejsUsePbjs: true,
includePath: [path.join(__dirname, 'fixtures', 'include')],
languages: ['nodejs'],
templateRoot: path.join(__dirname, '..', 'templates')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
9 changes: 2 additions & 7 deletions test/fixtures/nodejs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
6 changes: 0 additions & 6 deletions test/fixtures/proto-based/nodejs/README.md

This file was deleted.

Loading

0 comments on commit 830bd0c

Please sign in to comment.