Skip to content

Commit

Permalink
fix: Handle empty package (#285)
Browse files Browse the repository at this point in the history
* handle empty protobuf package

* prettify changes

* Add testcase

* Use maybePrefixPackage to reduce copy/paste.

Co-authored-by: Daniel Kilimnik <kaleb.dk@gmail.com>
  • Loading branch information
stephenh and kilimnik authored Apr 24, 2021
1 parent 2e4f654 commit 5eadf92
Show file tree
Hide file tree
Showing 10 changed files with 179 additions and 21 deletions.
Binary file added integration/no-proto-package/no-proto-package.bin
Binary file not shown.
11 changes: 11 additions & 0 deletions integration/no-proto-package/no-proto-package.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
syntax = "proto3";

service UserState {
rpc GetUsers(Empty) returns (stream User);
}

message User {
string name = 1;
}

message Empty {}
143 changes: 143 additions & 0 deletions integration/no-proto-package/no-proto-package.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/* eslint-disable */
import { util, configure, Reader, Writer } from 'protobufjs/minimal';
import * as Long from 'long';
import { Observable } from 'rxjs';

export const protobufPackage = '';

export interface User {
name: string;
}

export interface Empty {}

const baseUser: object = { name: '' };

export const User = {
encode(message: User, writer: Writer = Writer.create()): Writer {
if (message.name !== '') {
writer.uint32(10).string(message.name);
}
return writer;
},

decode(input: Reader | Uint8Array, length?: number): User {
const reader = input instanceof Reader ? input : new Reader(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = { ...baseUser } as User;
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
case 1:
message.name = reader.string();
break;
default:
reader.skipType(tag & 7);
break;
}
}
return message;
},

fromJSON(object: any): User {
const message = { ...baseUser } as User;
if (object.name !== undefined && object.name !== null) {
message.name = String(object.name);
} else {
message.name = '';
}
return message;
},

toJSON(message: User): unknown {
const obj: any = {};
message.name !== undefined && (obj.name = message.name);
return obj;
},

fromPartial(object: DeepPartial<User>): User {
const message = { ...baseUser } as User;
if (object.name !== undefined && object.name !== null) {
message.name = object.name;
} else {
message.name = '';
}
return message;
},
};

const baseEmpty: object = {};

export const Empty = {
encode(_: Empty, writer: Writer = Writer.create()): Writer {
return writer;
},

decode(input: Reader | Uint8Array, length?: number): Empty {
const reader = input instanceof Reader ? input : new Reader(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = { ...baseEmpty } as Empty;
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
default:
reader.skipType(tag & 7);
break;
}
}
return message;
},

fromJSON(_: any): Empty {
const message = { ...baseEmpty } as Empty;
return message;
},

toJSON(_: Empty): unknown {
const obj: any = {};
return obj;
},

fromPartial(_: DeepPartial<Empty>): Empty {
const message = { ...baseEmpty } as Empty;
return message;
},
};

export interface UserState {
GetUsers(request: Empty): Observable<User>;
}

export class UserStateClientImpl implements UserState {
private readonly rpc: Rpc;
constructor(rpc: Rpc) {
this.rpc = rpc;
}
GetUsers(request: Empty): Promise<User> {
const data = Empty.encode(request).finish();
const promise = this.rpc.request('UserState', 'GetUsers', data);
return promise.then((data) => User.decode(new Reader(data)));
}
}

interface Rpc {
request(service: string, method: string, data: Uint8Array): Promise<Uint8Array>;
}

type Builtin = Date | Function | Uint8Array | string | number | undefined;
export type DeepPartial<T> = T extends Builtin
? T
: T extends Array<infer U>
? Array<DeepPartial<U>>
: T extends ReadonlyArray<infer U>
? ReadonlyArray<DeepPartial<U>>
: T extends {}
? { [K in keyof T]?: DeepPartial<T[K]> }
: Partial<T>;

// If you get a compile-error about 'Constructor<Long> and ... have no overlap',
// add '--ts_proto_opt=esModuleInterop=true' as a flag when calling 'protoc'.
if (util.Long !== Long) {
util.Long = Long as any;
configure();
}
6 changes: 3 additions & 3 deletions src/generate-grpc-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { camelCase } from './case';
import { Context } from './context';
import SourceInfo, { Fields } from './sourceInfo';
import { messageToTypeName } from './types';
import { maybeAddComment } from './utils';
import { maybeAddComment, maybePrefixPackage } from './utils';

const CallOptions = imp('CallOptions@@grpc/grpc-js');
const ChannelCredentials = imp('ChannelCredentials@@grpc/grpc-js');
Expand Down Expand Up @@ -67,7 +67,7 @@ function generateServiceDefinition(

chunks.push(code`
${camelCase(methodDesc.name)}: {
path: '/${fileDesc.package}.${serviceDesc.name}/${methodDesc.name}',
path: '/${maybePrefixPackage(fileDesc, serviceDesc.name)}/${methodDesc.name}',
requestStream: ${methodDesc.clientStreaming},
responseStream: ${methodDesc.serverStreaming},
requestSerialize: (value: ${inputType}) =>
Expand Down Expand Up @@ -209,7 +209,7 @@ function generateClientConstructor(fileDesc: FileDescriptorProto, serviceDesc: S
return code`
export const ${def(`${serviceDesc.name}Client`)} = ${makeGenericClientConstructor}(
${serviceDesc.name}Service,
'${fileDesc.package}.${serviceDesc.name}'
'${maybePrefixPackage(fileDesc, serviceDesc.name)}'
) as unknown as {
new (
address: string,
Expand Down
3 changes: 2 additions & 1 deletion src/generate-grpc-web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { MethodDescriptorProto, FileDescriptorProto, ServiceDescriptorProto } fr
import { requestType, responseObservable, responsePromise, responseType } from './types';
import { Code, code, imp, joinCode } from 'ts-poet';
import { Context } from './context';
import { maybePrefixPackage } from './utils';

const grpc = imp('grpc@@improbable-eng/grpc-web');
const share = imp('share@rxjs/operators');
Expand Down Expand Up @@ -68,7 +69,7 @@ function generateRpcMethod(ctx: Context, serviceDesc: ServiceDescriptorProto, me
export function generateGrpcServiceDesc(fileDesc: FileDescriptorProto, serviceDesc: ServiceDescriptorProto): Code {
return code`
export const ${serviceDesc.name}Desc = {
serviceName: "${fileDesc.package}.${serviceDesc.name}",
serviceName: "${maybePrefixPackage(fileDesc, serviceDesc.name)}",
};
`;
}
Expand Down
10 changes: 6 additions & 4 deletions src/generate-services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
responsePromise,
responseType,
} from './types';
import { maybeAddComment, singular } from './utils';
import { maybeAddComment, maybePrefixPackage, singular } from './utils';
import SourceInfo, { Fields } from './sourceInfo';
import { camelCase } from './case';
import { contextTypeVar } from './main';
Expand Down Expand Up @@ -122,7 +122,7 @@ function generateRegularRpcMethod(
const data = ${inputType}.encode(request).finish();
const promise = this.rpc.request(
${maybeCtx}
"${fileDesc.package}.${serviceDesc.name}",
"${maybePrefixPackage(fileDesc, serviceDesc.name)}",
"${methodDesc.name}",
data
);
Expand Down Expand Up @@ -230,12 +230,14 @@ function generateCachingRpcMethod(
): Code {
const inputType = requestType(ctx, methodDesc);
const outputType = responseType(ctx, methodDesc);
const uniqueIdentifier = `${fileDesc.package}.${serviceDesc.name}.${methodDesc.name}`;
const uniqueIdentifier = `${maybePrefixPackage(fileDesc, serviceDesc.name)}.${methodDesc.name}`;
const lambda = code`
(requests) => {
const responses = requests.map(async request => {
const data = ${inputType}.encode(request).finish()
const response = await this.rpc.request(ctx, "${fileDesc.package}.${serviceDesc.name}", "${methodDesc.name}", data);
const response = await this.rpc.request(ctx, "${maybePrefixPackage(fileDesc, serviceDesc.name)}", "${
methodDesc.name
}", data);
return ${outputType}.decode(new ${Reader}(response));
});
return Promise.all(responses);
Expand Down
12 changes: 3 additions & 9 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
valueTypeName,
} from './types';
import SourceInfo, { Fields } from './sourceInfo';
import { maybeAddComment } from './utils';
import { maybeAddComment, maybePrefixPackage } from './utils';
import { camelToSnake, capitalize, maybeSnakeToCamel } from './case';
import {
generateNestjsGrpcServiceMethodsDecorator,
Expand Down Expand Up @@ -94,13 +94,7 @@ export function generateFile(ctx: Context, fileDesc: FileDescriptorProto): [stri
sourceInfo,
(fullName, message, sInfo, fullProtoTypeName) => {
chunks.push(
generateInterfaceDeclaration(
ctx,
fullName,
message,
sInfo,
fileDesc.package ? `${fileDesc.package}.${fullProtoTypeName}` : fullProtoTypeName
)
generateInterfaceDeclaration(ctx, fullName, message, sInfo, maybePrefixPackage(fileDesc, fullProtoTypeName))
);
},
options,
Expand All @@ -121,7 +115,7 @@ export function generateFile(ctx: Context, fileDesc: FileDescriptorProto): [stri
fileDesc,
sourceInfo,
(fullName, message, sInfo, fullProtoTypeName) => {
const fullTypeName = fileDesc.package ? `${fileDesc.package}.${fullProtoTypeName}` : fullProtoTypeName;
const fullTypeName = maybePrefixPackage(fileDesc, fullProtoTypeName);

chunks.push(generateBaseInstance(ctx, fullName, message, fullTypeName));

Expand Down
3 changes: 2 additions & 1 deletion src/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { imp, code, Code, joinCode, def } from 'ts-poet';
import { visit, visitServices } from './visit';
import { Context } from './context';
import SourceInfo from './sourceInfo';
import { maybePrefixPackage } from './utils';

const fileDescriptorProto = imp('FileDescriptorProto@ts-proto-descriptors');

Expand All @@ -20,7 +21,7 @@ export function generateSchema(ctx: Context, fileDesc: FileDescriptorProto, sour

const references: Code[] = [];
function addReference(localName: string, symbol: string): void {
references.push(code`'.${fileDesc.package}.${localName.replace(/_/g, '.')}': ${symbol}`);
references.push(code`'.${maybePrefixPackage(fileDesc, localName.replace(/_/g, '.'))}': ${symbol}`);
}

visit(
Expand Down
4 changes: 2 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { code, Code, imp, Import } from 'ts-poet';
import { DateOption, EnvOption, LongOption, OneofOption, Options } from './options';
import { visit } from './visit';
import { fail } from './utils';
import { fail, maybePrefixPackage } from './utils';
import SourceInfo from './sourceInfo';
import { camelCase } from './case';
import { Context } from './context';
Expand Down Expand Up @@ -575,7 +575,7 @@ export function detectBatchMethod(
if (mapType) {
outputType = mapType.valueType;
}
const uniqueIdentifier = `${fileDesc.package}.${serviceDesc.name}.${methodDesc.name}`;
const uniqueIdentifier = `${maybePrefixPackage(fileDesc, serviceDesc.name)}.${methodDesc.name}`;
return {
methodDesc,
uniqueIdentifier,
Expand Down
8 changes: 7 additions & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { code, Code } from 'ts-poet';
import { FileDescriptorProto } from 'ts-proto-descriptors';
import ReadStream = NodeJS.ReadStream;
import { SourceDescription } from './sourceInfo';
import { code, Code } from 'ts-poet';

export function readToBuffer(stream: ReadStream): Promise<Buffer> {
return new Promise((resolve) => {
Expand Down Expand Up @@ -87,3 +88,8 @@ export function maybeAddComment(
export function prefixDisableLinter(spec: string): string {
return `/* eslint-disable */\n${spec}`;
}

export function maybePrefixPackage(fileDesc: FileDescriptorProto, rest: string): string {
const prefix = fileDesc.package === '' ? '' : `${fileDesc.package}.`;
return `${prefix}${rest}`;
}

0 comments on commit 5eadf92

Please sign in to comment.