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

findCustomScalarOption but for repeated options? #607

Closed
Jayatubi opened this issue Nov 4, 2023 · 4 comments
Closed

findCustomScalarOption but for repeated options? #607

Jayatubi opened this issue Nov 4, 2023 · 4 comments

Comments

@Jayatubi
Copy link

Jayatubi commented Nov 4, 2023

It is valid to define a repeated options for a message such as:

extend google.protobuf.MessageOptions {
    repeated string bar = 10001;
}

message FooMessage {
   option(bar) = "hello";
   option(bar) = "world;
   ...
}

However, the function findCustomScalarOption could be used only to retrieve the first option since it was implemented as:

function createBinaryReader(desc, extensionNumber) {
    const opt = desc.proto.options;
    let reader = undefined;
    if (opt !== undefined) {
        const unknownFields = protobuf_1.proto3.bin.listUnknownFields(opt);
        /* 
           This `unknownFields` contains all the repeated options attached to a message, with a same number.
           But only the first matched one will be returned
        */
        const field = unknownFields.find((f) => f.no === extensionNumber);
        if (field) {
            reader = new protobuf_1.BinaryReader(field.data);
        }
    }
    return reader;
}

How about to provide a new function, maybe named findCustomRepeatedOptions, to retrieve all the matched options?

For example:

function createRepeatedBinaryReaders(desc, extensionNumber) {
  const opt = desc.proto.options;
  let readers = undefined;
  if (opt !== undefined) {
    const unknownFields = protobuf_1.proto3.bin.listUnknownFields(opt);
    readers = unknownFields
      .filter((f) => f.no === extensionNumber)
      .map(f => new protobuf_1.BinaryReader(field.data))
  }
  return readers;
}
@smaye81
Copy link
Member

smaye81 commented Nov 10, 2023

Hi @Jayatubi. You can retrieve repeated options using a message wrapper and the findCustomMessageOption function. For example:

// custom_options.proto
extend google.protobuf.MethodOptions {
  optional FooOptions foo_method_option = 50007;
}

message FooOptions {
  repeated string many = 1;
}

// foo_service.proto
import "custom_options.proto";

service FooService {
  rpc Get(GetRequest) returns (GetResponse) {
    option (foo_method_option) = { 
        many: ["a", "b", "c"],
    };
  }
}

You can retrieve the options using a generated type by first generating the file which defines the custom option type. Then, import and pass this type to the findCustomMessageOption function.

import { FooOptions } from "./gen/proto/custom_options_pb.js";

const option = findCustomMessageOption(method, 50007, FooOptions)

console.log(option);
/*
 * {
 *     many: ["a", "b", "c"],
 * }
 */

For more info, check out our plugin docs

@Jayatubi
Copy link
Author

Hi @Jayatubi. You can retrieve repeated options using a message wrapper and the findCustomMessageOption function. For example:

// custom_options.proto
extend google.protobuf.MethodOptions {
  optional FooOptions foo_method_option = 50007;
}

message FooOptions {
  repeated string many = 1;
}

// foo_service.proto
import "custom_options.proto";

service FooService {
  rpc Get(GetRequest) returns (GetResponse) {
    option (foo_method_option) = { 
        many: ["a", "b", "c"],
    };
  }
}

You can retrieve the options using a generated type by first generating the file which defines the custom option type. Then, import and pass this type to the findCustomMessageOption function.

import { FooOptions } from "./gen/proto/custom_options_pb.js";

const option = findCustomMessageOption(method, 50007, FooOptions)

console.log(option);
/*
 * {
 *     many: ["a", "b", "c"],
 * }
 */

For more info, check out our plugin docs

The wrapper message option looks like but I think a plain repeated options should also be useful.

No need to change any of current interfaces. Just add a new one would be better.

@smaye81
Copy link
Member

smaye81 commented Nov 10, 2023

Yeah, understood. We actually explicitly opted against this for now to keep the API simpler and feel that the message wrapper is a viable alternative. Going to close this for now, since I don't see us implementing this anytime soon, but we will definitely reopen if we decide to revisit.

@smaye81 smaye81 closed this as completed Nov 10, 2023
@timostamm
Copy link
Member

@Jayatubi, the next release will support extensions. They can be used to retrieve repeated custom options as well. See #666 and #669.

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

3 participants