Skip to content

Commit d168d1b

Browse files
Merge pull request #249 from stanley-cheung/codegen-fixes
Various fixes to codegen plugin
2 parents 30c30e4 + 01d3b63 commit d168d1b

File tree

13 files changed

+643
-52
lines changed

13 files changed

+643
-52
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,5 @@ bazel-genfiles
33
bazel-grpc-web
44
bazel-out
55
bazel-testlogs
6+
*.o
7+
protoc-gen-*

javascript/net/grpc/web/grpc_generator.cc

Lines changed: 147 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,120 @@ string LowercaseFirstLetter(string s) {
9090
return s;
9191
}
9292

93+
94+
// The following 5 functions were copied from
95+
// google/protobuf/src/google/protobuf/stubs/strutil.h
96+
97+
inline bool HasPrefixString(const string& str,
98+
const string& prefix) {
99+
return str.size() >= prefix.size() &&
100+
str.compare(0, prefix.size(), prefix) == 0;
101+
}
102+
103+
inline string StripPrefixString(const string& str, const string& prefix) {
104+
if (HasPrefixString(str, prefix)) {
105+
return str.substr(prefix.size());
106+
} else {
107+
return str;
108+
}
109+
}
110+
111+
inline bool HasSuffixString(const string& str,
112+
const string& suffix) {
113+
return str.size() >= suffix.size() &&
114+
str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0;
115+
}
116+
117+
inline string StripSuffixString(const string& str, const string& suffix) {
118+
if (HasSuffixString(str, suffix)) {
119+
return str.substr(0, str.size() - suffix.size());
120+
} else {
121+
return str;
122+
}
123+
}
124+
125+
void ReplaceCharacters(string *s, const char *remove, char replacewith) {
126+
const char *str_start = s->c_str();
127+
const char *str = str_start;
128+
for (str = strpbrk(str, remove);
129+
str != nullptr;
130+
str = strpbrk(str + 1, remove)) {
131+
(*s)[str - str_start] = replacewith;
132+
}
133+
}
134+
135+
136+
// The following function was copied from
137+
// google/protobuf/src/google/protobuf/compiler/cpp/cpp_helpers.cc
138+
139+
string StripProto(const string& filename) {
140+
if (HasSuffixString(filename, ".protodevel")) {
141+
return StripSuffixString(filename, ".protodevel");
142+
} else {
143+
return StripSuffixString(filename, ".proto");
144+
}
145+
}
146+
147+
148+
// The following 3 functions were copied from
149+
// google/protobuf/src/google/protobuf/compiler/js/js_generator.cc
150+
151+
// Returns the name of the message with a leading dot and taking into account
152+
// nesting, for example ".OuterMessage.InnerMessage", or returns empty if
153+
// descriptor is null. This function does not handle namespacing, only message
154+
// nesting.
155+
string GetNestedMessageName(const Descriptor* descriptor) {
156+
if (descriptor == nullptr) {
157+
return "";
158+
}
159+
string result = StripPrefixString(descriptor->full_name(),
160+
descriptor->file()->package());
161+
// Add a leading dot if one is not already present.
162+
if (!result.empty() && result[0] != '.') {
163+
result = "." + result;
164+
}
165+
return result;
166+
}
167+
168+
// Given a filename like foo/bar/baz.proto, returns the root directory
169+
// path ../../
170+
string GetRootPath(const string& from_filename, const string& to_filename) {
171+
if (HasPrefixString(to_filename, "google/protobuf")) {
172+
// Well-known types (.proto files in the google/protobuf directory) are
173+
// assumed to come from the 'google-protobuf' npm package. We may want to
174+
// generalize this exception later by letting others put generated code in
175+
// their own npm packages.
176+
return "google-protobuf/";
177+
}
178+
179+
size_t slashes = std::count(from_filename.begin(), from_filename.end(), '/');
180+
if (slashes == 0) {
181+
return "./";
182+
}
183+
string result = "";
184+
for (size_t i = 0; i < slashes; i++) {
185+
result += "../";
186+
}
187+
return result;
188+
}
189+
190+
// Returns the alias we assign to the module of the given .proto filename
191+
// when importing.
192+
string ModuleAlias(const string& filename) {
193+
// This scheme could technically cause problems if a file includes any 2 of:
194+
// foo/bar_baz.proto
195+
// foo_bar_baz.proto
196+
// foo_bar/baz.proto
197+
//
198+
// We'll worry about this problem if/when we actually see it. This name isn't
199+
// exposed to users so we can change it later if we need to.
200+
string basename = StripProto(filename);
201+
ReplaceCharacters(&basename, "-", '$');
202+
ReplaceCharacters(&basename, "/", '_');
203+
ReplaceCharacters(&basename, ".", '_');
204+
return basename + "_pb";
205+
}
206+
93207
/* Finds all message types used in all services in the file, and returns them
94208
* as a map of fully qualified message type name to message descriptor */
95209
std::map<string, const Descriptor*> GetAllMessages(const FileDescriptor* file) {
@@ -126,14 +240,20 @@ void PrintMessagesDeps(Printer* printer, const FileDescriptor* file) {
126240
}
127241

128242
void PrintCommonJsMessagesDeps(Printer* printer, const FileDescriptor* file) {
129-
std::map<string, const Descriptor*> messages = GetAllMessages(file);
130243
std::map<string, string> vars;
244+
245+
for (int i = 0; i < file->dependency_count(); i++) {
246+
const string& name = file->dependency(i)->name();
247+
vars["alias"] = ModuleAlias(name);
248+
vars["dep_filename"] = GetRootPath(file->name(), name) + StripProto(name);
249+
// we need to give each cross-file import an alias
250+
printer->Print(
251+
vars,
252+
"\nvar $alias$ = require('$dep_filename$_pb.js')\n");
253+
}
254+
131255
string package = file->package();
132-
string filename = file->name();
133-
// Remove .proto extension
134-
filename = filename.substr(0, filename.size() - 6);
135256
vars["package_name"] = package;
136-
vars["filename"] = filename;
137257

138258
printer->Print(vars, "const proto = {};\n");
139259
if (!package.empty()) {
@@ -149,6 +269,14 @@ void PrintCommonJsMessagesDeps(Printer* printer, const FileDescriptor* file) {
149269
}
150270
}
151271

272+
// need to import the messages from our own file
273+
string filename = StripProto(file->name());
274+
size_t last_slash = filename.find_last_of('/');
275+
if (last_slash != string::npos) {
276+
filename = filename.substr(last_slash + 1);
277+
}
278+
vars["filename"] = filename;
279+
152280
printer->Print(
153281
vars,
154282
"proto.$package_name$ = require('./$filename$_pb.js');\n\n");
@@ -221,7 +349,7 @@ void PrintMethodInfo(Printer* printer, std::map<string, string> vars) {
221349
printer->Indent();
222350
printer->Print(
223351
vars,
224-
"proto.$out$,\n"
352+
"$out_type$,\n"
225353
"/** @param {!proto.$in$} request */\n"
226354
"function(request) {\n");
227355
printer->Print(
@@ -230,7 +358,7 @@ void PrintMethodInfo(Printer* printer, std::map<string, string> vars) {
230358
printer->Print("},\n");
231359
printer->Print(
232360
vars,
233-
("proto.$out$." + GetDeserializeMethodName(vars["mode"]) +
361+
("$out_type$." + GetDeserializeMethodName(vars["mode"]) +
234362
"\n").c_str());
235363
printer->Outdent();
236364
printer->Print(
@@ -349,8 +477,7 @@ class GrpcCodeGenerator : public CodeGenerator {
349477
}
350478

351479
if (file_name.empty()) {
352-
*error = "options: out is required";
353-
return false;
480+
file_name = StripProto(file->name()) + "_grpc_web_pb.js";
354481
}
355482
if (mode.empty()) {
356483
*error = "options: mode is required";
@@ -398,8 +525,8 @@ class GrpcCodeGenerator : public CodeGenerator {
398525
switch (import_style) {
399526
case ImportStyle::CLOSURE:
400527
printer.Print(
401-
vars,
402-
"goog.provide('proto.$package_dot$$service_name$Client');\n");
528+
vars,
529+
"goog.provide('proto.$package_dot$$service_name$Client');\n");
403530
break;
404531
case ImportStyle::COMMONJS:
405532
break;
@@ -439,6 +566,15 @@ class GrpcCodeGenerator : public CodeGenerator {
439566
vars["method_name"] = method->name();
440567
vars["in"] = method->input_type()->full_name();
441568
vars["out"] = method->output_type()->full_name();
569+
if (import_style == ImportStyle::COMMONJS &&
570+
method->output_type()->file() != file) {
571+
// Cross-file ref in CommonJS needs to use the module alias instead
572+
// of the global name.
573+
vars["out_type"] = ModuleAlias(method->output_type()->file()->name())
574+
+ GetNestedMessageName(method->output_type());
575+
} else {
576+
vars["out_type"] = "proto."+method->output_type()->full_name();
577+
}
442578

443579
// Client streaming is not supported yet
444580
if (!method->client_streaming()) {

packages/grpc-web/package.json

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
{
22
"name": "grpc-web",
33
"version": "0.3.0",
4-
"description": "gRPC web runtime",
4+
"author": "Google Inc.",
5+
"description": "gRPC-Web JS Client Runtime Library",
6+
"homepage": "https://grpc.io/",
7+
"repository": {
8+
"type": "git",
9+
"url": "https://github.com/grpc/grpc-web.git"
10+
},
11+
"bugs": "https://github.com/grpc/grpc-web/issues",
512
"main": "index.js",
613
"scripts": {
714
"build": "node scripts/build.js",
815
"prepare": "npm run build && require-self",
916
"prepublishOnly": "npm run build",
10-
"test": "mocha"
17+
"test": "mocha --timeout 8000 \"./test/**/*_test.js\""
1118
},
1219
"license": "Apache-2.0",
1320
"devDependencies": {

packages/grpc-web/scripts/build.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
/**
2+
*
3+
* Copyright 2018 Google LLC
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
119
const path = require("path");
220
const {exec} = require("child_process");
321

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
*
3+
* Copyright 2018 Google LLC
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
goog.provide('proto.grpc.gateway.testing.EchoAppClient');
20+
21+
goog.require('proto.grpc.gateway.testing.EchoServiceClient');
22+
goog.require('proto.grpc.gateway.testing.EchoRequest');
23+
24+
25+
proto.grpc.gateway.testing.EchoAppClient = function() {
26+
this.client =
27+
new proto.grpc.gateway.testing.EchoServiceClient('MyHostname', null, null);
28+
}
29+
30+
proto.grpc.gateway.testing.EchoAppClient.prototype.echo = function(msg, cb) {
31+
var request = new proto.grpc.gateway.testing.EchoRequest();
32+
request.setMessage(msg);
33+
this.client.echo(request, {}, cb);
34+
}

packages/grpc-web/test/common.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
*
3+
* Copyright 2018 Google LLC
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
const fs = require('fs');
20+
const GENERATED_CODE_PATH = './generated';
21+
22+
var removeDirectory = function(path) {
23+
if (fs.existsSync(path)) {
24+
fs.readdirSync(path).forEach(function(file, index) {
25+
var curPath = path + "/" + file;
26+
if (fs.lstatSync(curPath).isDirectory()) {
27+
removeDirectory(curPath);
28+
} else {
29+
fs.unlinkSync(curPath);
30+
}
31+
});
32+
fs.rmdirSync(path);
33+
}
34+
}
35+
36+
exports.removeDirectory = removeDirectory;
37+
exports.GENERATED_CODE_PATH = GENERATED_CODE_PATH;

0 commit comments

Comments
 (0)