bufbuild / protobuf-es

Protocol Buffers for ECMAScript. The only JavaScript Protobuf library that is fully-compliant with Protobuf conformance tests.
Apache License 2.0
960 stars 55 forks source link

Remove internal fields #833

Closed netanel-utila closed 2 weeks ago

netanel-utila commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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

timostamm commented 3 weeks ago

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 commented 3 weeks ago

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

Failure: descriptorset-filtered.binpb: not a directory

timostamm commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

Works, thanks for your help :)

netanel-utila commented 2 weeks ago

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

timostamm commented 2 weeks ago

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 commented 2 weeks ago

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

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

timostamm commented 2 weeks ago

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 commented 2 weeks ago

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