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

Remove internal fields #833

Closed
netanel-utila opened this issue May 9, 2024 · 14 comments
Closed

Remove internal fields #833

netanel-utila opened this issue May 9, 2024 · 14 comments

Comments

@netanel-utila
Copy link

netanel-utila commented May 9, 2024

Hi, thanks for this great library. We are migrating to using it (from https://github.com/stephenh/ts-proto) and we are wondering how can we remove fields that marked as internal:

 Utila x = 50 [(google.api.field_visibility).restriction = "INTERNAL"];

Thanks.

@timostamm
Copy link
Member

I'm not aware that google/api/visibility.proto is supported anywhere outside of google. I think the best approach would be to filter fields marked as internal before generating code.

The first step is to generate code for the visibility option, for example - assuming you have @bufbuild/buf and @bufbuild/protoc-gen-es installed, and that you have a minimal buf.gen.yaml:

npx buf generate buf.build/googleapis/googleapis --path google/api/visibility.proto

This will generate the file src/gen/google/api/visibility_pb.ts, which includes the extension google.api.field_visibility.

With this extension, we can remove internal fields from a descriptorset:

// filter-internal.ts
import {readFileSync, writeFileSync} from "node:fs";
import {FileDescriptorSet, getExtension, hasExtension} from "@bufbuild/protobuf";
import {field_visibility} from "./src/gen/google/api/visibility_pb";

const set = FileDescriptorSet.fromBinary(readFileSync("descriptorset.binpb"));

for (const file of set.file) {
  for (const message of file.messageType) {
    for (const field of message.field) {
      if (field.options && hasExtension(field.options, field_visibility)) {
        const visibilityRule = getExtension(field.options, field_visibility);
        if (visibilityRule.restriction == "INTERNAL") {
          // remove the field
          message.field.splice(
            message.field.indexOf(field),
            1,
          );
        }
      }
    }
  }
}

writeFileSync("descriptorset-filtered.binpb", set.toBinary());

Instead of generating code directly from Protobuf files, we can go from Protobuf files to a descriptor-set, remove internal fields from that descriptor-set, then generate code from the filtered set:

# generate.bash
npx buf build proto -o descriptorset.binpb
npx tsx filter-internal.ts
npx buf generate descriptorset-filtered.binpb

The result is generated code where fields marked with (google.api.field_visibility).restriction = "INTERNAL" simply don't exist.

@netanel-utila
Copy link
Author

netanel-utila commented May 9, 2024

Thanks for the detailed response. I'll try it asap and let you know how it goes. Will it work also with internal rpc?

service Balances {
  rpc SumBalances(SumBalancesRequest) returns (SumBalancesResponse) {
    option (google.api.method_visibility).restriction = "INTERNAL";
  }
}

In the above example, we want to remove the SumBalances rpc.

@netanel-utila
Copy link
Author

Btw, we're not limited to use INTERNAL. If you have an easier way to achieve it, we'd love to hear.

@timostamm
Copy link
Member

The example above only considers fields. I'm sure you'll find it's quite easy to add the same filtering for RPCs if you peek into the code a bit, and look into descriptors.

@netanel-utila
Copy link
Author

@timostamm it seems like we can't use buf generate descriptorset-filtered.binpb:

Failure: descriptorset-filtered.binpb: not a directory

@timostamm
Copy link
Member

I tested the example I gave you, so if it runs for me, it can also run for you 🙂

You're probably have a typo somewhere, or a similar small issue. Double check the command you are running that errors with "not a directory".

@netanel-utila
Copy link
Author

I'm running buf generate descriptorset-filtered.binpb. I can't find in the docs that we can use binpb with the generate command. What am I missing?

@timostamm
Copy link
Member

The generate command takes an "input":

buf generate <input> [flags]

The "input" is specified here.

You're probably using an old version of buf, that does not recognizes the .binpb extension automatically. I recommend installing the CLI via npm from @bufbuild/buf in a JS / TS project. This makes it easy to update.

@netanel-utila
Copy link
Author

Works, thanks for your help :)

@netanel-utila
Copy link
Author

Hi, will it work also with v2? I just saw it and it seems like it is a different Api.

@timostamm
Copy link
Member

Hi, will it work also with v2? I just saw it and it seems like it is a different Api.

The API changes in a very straight-forward way: Methods on messages are now external functions. Here's the same script with v2:

import {readFileSync, writeFileSync} from "node:fs";
import {fromBinary, toBinary, getExtension, hasExtension} from "@bufbuild/protobuf";
import {FileDescriptorSetDesc} from "@bufbuild/protobuf/wkt";
import {field_visibility} from "./src/gen/google/api/visibility_pb";

const set = fromBinary(FileDescriptorSetDesc, readFileSync("descriptorset.binpb"));

for (const file of set.file) {
  for (const message of file.messageType) {
    for (const field of message.field) {
      if (field.options && hasExtension(field.options, field_visibility)) {
        const visibilityRule = getExtension(field.options, field_visibility);
        if (visibilityRule.restriction == "INTERNAL") {
          // remove the field
          message.field.splice(
            message.field.indexOf(field),
            1,
          );
        }
      }
    }
  }
}

writeFileSync("descriptorset-filtered.binpb", toBinary(FileDescriptorSetDesc, set));

@netanel-utila
Copy link
Author

I'm trying v2 now, and have a few issues:

  • @connectrpc/connect and @connectrpc/connect-node depends on "@bufbuild/protobuf": "^1.4.2".
  • I'm also getting an error when trying to run the connected proto:

SyntaxError: Export named 'protoBase64' not found in module '/PATH/node_modules/@bufbuild/protobuf/dist/esm/index.js'.

@timostamm
Copy link
Member

Ah, I didn't realize you use @connectrpc/connect as well. It does not support v2 yet, but we'll publish a pre-release version very soon that does.

@netanel-utila
Copy link
Author

Oh OK. We'll wait for the releases. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants