feat: Add google.api.http support

[jas-chen]

Fixes #948.

This PR adds support for generating google.api.http-based metadata for developers to implement an HTTP client based on it.

To retrieve the information, google-protobuf and related vendor codes are included. They will be loaded on demand to reduce the performance hit.

It also introduces a top-level CLI option http just like the nestJs option.

[stephenh]

Hi @jas-chen ! This looks amazing! Just scanning the PR, everything looks great, but I won't have time to review more thoroughly until this weekend, if that's okay...I'll take a closer look, and maybe leave a few minor comments/suggestions then. Thank you!

[stephenh]

Hi @jas-chen ; thanks again for the PR, I had a chance to leave some feedback.

I'd really like to avoid vendoring the other code generator, and re-deserializing the request--lmk if you have ideas for how to avoid that by getting ts-proto-descriptors to have the info you need/want.

Also, per the comment, would love to have this PR (or a follow up) come with mapped types / runtime support to "just make a client" for users to use out-of-the-box.

Thanks!

[stephenh]

Hey @jas-chen , I think this is a great so far--I'm wondering if we could further and remove the need for this createApi method...

I.e. if a user has ~10 services with ~10 methods each, they''ll have to call createApi against each of them--granted, they could write a method to do that in a loop...

Like our current non-google impl looks like:

export interface MyService {
  MyMethod(request: RequestType): Promise<ResponseType>;
}

export const MyServiceServiceName = "MyService";
export class MyServiceClientImpl implements MyService {
  private readonly rpc: Rpc;
  private readonly service: string;
  constructor(rpc: Rpc, opts?: { service?: string }) {
    this.service = opts?.service || MyServiceServiceName;
    this.rpc = rpc;
    this.MyMethod = this.MyMethod.bind(this);
  }
  MyMethod(request: RequestType): Promise<ResponseType> {
    const data = RequestType.encode(request).finish();
    const promise = this.rpc.request(this.service, "MyMethod", data);
    return promise.then((data) => ResponseType.decode(_m0.Reader.create(data)));
  }
}

And it seems like it'd be nice to provide a similar sort of API to users...

I suppose with the export const Messaging, the users could provide their own mapped types & helper methods to achieve this same output, i.e. something like:

// creates the `MessagingService.MyMethod` / `MessagingService.updateMessage` interface
type MessagingService = ClientService<typeof Messaging>;

const client: MessagingService = createClient(Messaging);
await client.getMessage(...);

I'm thinking it'd be great for ts-proto to eventually provide those ClientService and createClient implementations (i.e. that could automatically parse & replace the {message_id} based on the rules in their docs).

Granted, sometimes I've just codegen-d out the export interface MyService { ... } and export class MyServiceImpl, which was a very old-school approach code generation, but it's definitely doable to use TS mapped types & runtime libraries to achieve the same effect.

Wdyt about the goal of shipping a ClientService & createClient method that took in the Messaging config? Maybe it could be like a ts-proto-google-http runtime library, which didn't bloat the codegen output. Would you be interesting in contributing that as well as this config output?

Thanks!

[jas-chen]

I updated the client implementation example code, the createApi takes a service definition now.

[jas-chen]

Hi @stephenh,

Thank you for reviewing my PR. I want to confirm this point first:

I'd really like to avoid vendoring the other code generator, and re-deserializing the request--lmk if you have ideas for how to avoid that by getting ts-proto-descriptors to have the info you need/want.

I tried using ts-proto-descriptors to generate code from google/api/annotations.proto. However, I couldn't find a way to generate code that injects HttpRule http = 72295728; to google.protobuf.MethodOptions.

https://github.com/googleapis/googleapis/blob/556515b4a1a60b38e8a23d7ac0171c9cd3a87c7a/google/api/annotations.proto#L28-L31

To be more specific, I want to make protos/google/protobuf/descriptor.ts to have additional code

// Add to the top of the file
const annotations = require('../api/annotations');
const http = require('../api/http');

// Add one more case in MethodOptions.decode
case annotations.http.number:
  if (tag !== annotations.http.tag) {
      break;
  }
  message.http = http.HttpRule.decode(reader, reader.uint32());
  continue;

Do you have any suggestions?

[stephenh]

Hi @jas-chen !

I think we can do this like this:

#1078

Unfortunately it looks like we might have a bug in our extensions support, because I'm getting this error:

[I] ~/o/t/protos (add-api-protos|✔) $ ./build.sh
google/api/annotations.ts:19:21 - error TS2345: Argument of type 'HttpRule | undefined' is not assignable to parameter of type 'HttpRule'.
  Type 'undefined' is not assignable to type 'HttpRule'.

19     HttpRule.encode(value, writer.fork()).ldelim();
                       ~~~~~


Found 1 error in google/api/annotations.ts:19

On that branch...maybe we're just missing an undefined check in the generated code?

Do you have time to look into fixing ^? It probably involves updating the extension code in src/main.ts (#808 was the original PR that extension suport)...

If not, I can try and take a look soon. Thanks!

[jas-chen]

Hi @stephenh,

After much consideration of the API design, I decided to extend the current outputServices=generic-definitions feature to support google.api.http. This way, it won't introduce any API changes, and it makes sense to only generate definitions.

We can add a client implementation later if needed.

Regarding the google/api/annotations.proto issue, thank you for providing #1078 as a reference, but I don't see protos/google/protobuf/descriptor.ts changed. I added a workaround protos/extend-descriptor.cjs in this PR as a codemod to inject HTTP-related code into it.

Please help review it again and let me know what you think; thank you!

[jas-chen]

After testing this API with a bunch of .proto files I have, I realized that the generated code from grpc and google.api.http are not compatible because google.api.http requires onlyTypes=true while grpc can't work with that option. So we can't use the same option to handle them at once 😓 .

I pushed a commit 2715391 to introduce the outputServices=generic-google-api-http-definitions option.

[DMXRoid]

This is functionality that I need badly enough that I'm using @jas-chen 's fork :p Happy to chip in and fix bugs and such if that would be at all helpful towards getting this merged in.

[jas-chen]

@stephenh, are you reviewing this PR? Is there anything I need to change?

[stephenh]

@jas-chen hello! Sorry, I've just been really busy with work lately; I definitely want to review & accept this PR--I should be able to review/accept it this weekend when I have some time. Thanks again!

[jas-chen]

Hi @stephenh,
No worries at all—take your time. I was just checking in to see if there’s any update on the status of the PR. Thank you!

[williamsk91]

@stephenh Hi 👋🏻
@jas-chen has moved company and I plan to take over his PR 🙏🏻

When you have the time can you please help check whether the direction of adding protos/google/* is a good idea or not?

Can I also ask when does protos/build.sh is supposed to be called? I looked through the codebase but couldn't find anything 🙇🏻

Also going forward, maybe it's better for me to open a new PR from my own fork referencing this PR? 🤔

[angelorc]

It seems that many of us need this PR :D
I tried the PR locally, unfortunately I am unable to generate google.http.api types

git clone https://github.com/jas-chen/ts-proto/
git checkout 948-http-support
yarn && yarn build

on the directory integration/google-api-http after the command

protoc --version # libprotoc 3.12.4

protoc \
  --plugin=../../protoc-gen-ts_proto \
  --ts_proto_out=./generated \
  --ts_proto_opt=onlyTypes=true,outputServices=generic-google-api-http-definitions \
  ./*.proto

I still get a simple.ts file without any google.http.api

export const protobufPackage = "";

export interface GetMessageRequest {
  messageId: string;
}

export interface GetMessageResponse {
  message: string;
}

export interface CreateMessageRequest {
  messageId: string;
  /** mapped to the body */
  message: string;
}

export interface CreateMessageResponse {
}

[stephenh]

Hey @williamsk91 , @angelorc , sorry, this is a larger PR in an area that I'm not very familiar (I don't personally use google.api.http), so reviewing this PR has been easy to fall to the bottom of the todo list. :-/ Sorry about that!

But, as long as you're patient with me reviewing it & landing it, yep, would definitely love to have this feature/PR ship in ts-proto.

Kinda looks like the PR needs rebased/spruced up, which is undertandable given it's probably just stale; lmk if you need anything from me, but otherwise thanks for tackling this!