Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reflection: Fix references to symbols with no package #2678

Merged
merged 1 commit into from Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/grpc-reflection/package.json
@@ -1,6 +1,6 @@
{
"name": "@grpc/reflection",
"version": "1.0.2",
"version": "1.0.3",
"author": {
"name": "Google Inc."
},
Expand Down
1 change: 1 addition & 0 deletions packages/grpc-reflection/proto/sample/sample.proto
Expand Up @@ -8,6 +8,7 @@ import 'unscoped.proto';
service SampleService {
rpc Hello (HelloRequest) returns (HelloResponse) {}
rpc Hello2 (HelloRequest) returns (CommonMessage) {}
rpc Hello3 (ProcessRequest) returns (TopLevelMessage) {}
}

service IgnoreService {
Expand Down
10 changes: 7 additions & 3 deletions packages/grpc-reflection/src/implementations/reflection-v1.ts
Expand Up @@ -113,8 +113,12 @@ export class ReflectionV1Implementation {

let referencedFile: IFileDescriptorProto | null = null;
if (ref.startsWith('.')) {
// absolute reference -- just remove the leading '.' and use the ref directly
referencedFile = this.symbols[ref.slice(1)];
/* absolute reference -- In files with no package, symbols are
* populated in the symbols table with a leading period in the key.
* If there is a package, the symbol does not have a leading period in
* the key. For simplicity, we check without the period, then with it.
*/
referencedFile = this.symbols[ref.slice(1)] ?? this.symbols[ref];
} else {
// relative reference -- need to seek upwards up the current package scope until we find it
let pkg = pkgScope;
Expand Down Expand Up @@ -315,7 +319,7 @@ export class ReflectionV1Implementation {
private getFileDependencies(file: IFileDescriptorProto): IFileDescriptorProto[] {
const visited: Set<IFileDescriptorProto> = new Set();
const toVisit: IFileDescriptorProto[] = [...(this.fileDependencies.get(file) || [])];

while (toVisit.length > 0) {
const current = toVisit.pop();

Expand Down
Expand Up @@ -51,7 +51,7 @@ describe('GrpcReflectionService', () => {
const names = descriptors.map((desc) => desc.name);
assert.deepEqual(
new Set(names),
new Set(['sample.proto', 'vendor.proto', 'vendor_dependency.proto'])
new Set(['root.proto', 'sample.proto', 'vendor.proto', 'vendor_dependency.proto'])
);
});

Expand Down Expand Up @@ -99,7 +99,7 @@ describe('GrpcReflectionService', () => {
const names = descriptors.map((desc) => desc.name);
assert.deepEqual(
new Set(names),
new Set(['sample.proto', 'vendor.proto', 'vendor_dependency.proto']),
new Set(['root.proto', 'sample.proto', 'vendor.proto', 'vendor_dependency.proto']),
);
});

Expand Down Expand Up @@ -129,7 +129,7 @@ describe('GrpcReflectionService', () => {
const names = descriptors.map((desc) => desc.name);
assert.deepEqual(
new Set(names),
new Set(['sample.proto', 'vendor.proto', 'vendor_dependency.proto']),
new Set(['root.proto', 'sample.proto', 'vendor.proto', 'vendor_dependency.proto']),
);
});

Expand Down Expand Up @@ -169,7 +169,7 @@ describe('GrpcReflectionService', () => {
const names = descriptors.map((desc) => desc.name);
assert.deepEqual(
new Set(names),
new Set(['sample.proto', 'vendor.proto', 'vendor_dependency.proto']),
new Set(['root.proto', 'sample.proto', 'vendor.proto', 'vendor_dependency.proto']),
);
});
});
Expand Down