Skip to content

Commit

Permalink
image filtering: don't include entire enclosing message/service (bufb…
Browse files Browse the repository at this point in the history
…uild#1659)

If an enclosing message is not part of the transitive dependency graph,
but a nested message therein is, the enclosing message will be stripped
of its fields. This produces a smaller filtered descriptor set but does not
impede the use of the results for dynamic messages or dynamic RPC.

Similarly, if a method is indicated as a type filter, its enclosing service
will be stripped of other unreferenced methods.

Finally, this attempts to fix/clarify the behavior around when custom
options are included since they are also known extensions. If known
extensions are included in the filtered set AND one of the options
message types is part of the transitive dependency graph, all of the
relevant custom options will be included. Otherwise, if custom
options are included, ones actually referenced in options on the
elements in the transitive dependency graph will be present.
  • Loading branch information
jhump committed Dec 14, 2022
1 parent f1c4765 commit 42e8e0d
Show file tree
Hide file tree
Showing 11 changed files with 862 additions and 211 deletions.
402 changes: 207 additions & 195 deletions private/bufpkg/bufimage/bufimageutil/bufimageutil.go

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions private/bufpkg/bufimage/bufimageutil/bufimageutil_test.go
Expand Up @@ -62,6 +62,12 @@ func TestOptions(t *testing.T) {
t.Run("exclude-options", func(t *testing.T) {
runDiffTest(t, "testdata/options", []string{"pkg.Foo", "pkg.FooEnum", "pkg.FooService"}, "all-exclude-options.txtar", WithExcludeCustomOptions())
})
t.Run("files", func(t *testing.T) {
runDiffTest(t, "testdata/options", []string{"Files"}, "Files.txtar")
})
t.Run("all-with-files", func(t *testing.T) {
runDiffTest(t, "testdata/options", []string{"pkg.Foo", "pkg.FooEnum", "pkg.FooService", "Files"}, "all-with-Files.txtar")
})
}

func TestNesting(t *testing.T) {
Expand Down
Expand Up @@ -2,10 +2,7 @@
syntax = "proto3";
package pkg;
message Foo {
string x = 1;
NestedFoo nested_foo = 2;
message NestedFoo {
string nested_x = 1;
message NestedNestedFoo {
string nested_nested_x = 1;
}
Expand Down
Expand Up @@ -2,8 +2,6 @@
syntax = "proto3";
package pkg;
message Bar {
FooEnum foo_enum = 1;
Foo.NestedFoo nested_foo = 2;
enum NestedBarEnum {
NESTED_BAR_ENUM_X = 0;
NESTED_BAR_ENUM_Y = 1;
Expand All @@ -12,14 +10,3 @@ message Bar {
message Baz {
Bar.NestedBarEnum nested_bar_enum = 1;
}
message Foo {
string x = 1;
NestedFoo nested_foo = 2;
message NestedFoo {
string nested_x = 1;
}
}
enum FooEnum {
FOO_ENUM_X = 0;
FOO_ENUM_Y = 1;
}
291 changes: 291 additions & 0 deletions private/bufpkg/bufimage/bufimageutil/testdata/options/Files.txtar
@@ -0,0 +1,291 @@
-- google/protobuf/descriptor.proto --
syntax = "proto2";
package google.protobuf;
option cc_enable_arenas = true;
option csharp_namespace = "Google.Protobuf.Reflection";
option go_package = "google.golang.org/protobuf/types/descriptorpb";
option java_outer_classname = "DescriptorProtos";
option java_package = "com.google.protobuf";
option objc_class_prefix = "GPB";
option optimize_for = SPEED;
message DescriptorProto {
optional string name = 1;
repeated FieldDescriptorProto field = 2;
repeated DescriptorProto nested_type = 3;
repeated EnumDescriptorProto enum_type = 4;
repeated ExtensionRange extension_range = 5;
repeated FieldDescriptorProto extension = 6;
optional MessageOptions options = 7;
repeated OneofDescriptorProto oneof_decl = 8;
repeated ReservedRange reserved_range = 9;
repeated string reserved_name = 10;
message ExtensionRange {
optional int32 start = 1;
optional int32 end = 2;
optional ExtensionRangeOptions options = 3;
}
message ReservedRange {
optional int32 start = 1;
optional int32 end = 2;
}
}
message EnumDescriptorProto {
optional string name = 1;
repeated EnumValueDescriptorProto value = 2;
optional EnumOptions options = 3;
repeated EnumReservedRange reserved_range = 4;
repeated string reserved_name = 5;
message EnumReservedRange {
optional int32 start = 1;
optional int32 end = 2;
}
}
message EnumOptions {
optional bool allow_alias = 2;
optional bool deprecated = 3 [default = false];
repeated UninterpretedOption uninterpreted_option = 999;
extensions 1000 to max;
reserved 5;
}
message EnumValueDescriptorProto {
optional string name = 1;
optional int32 number = 2;
optional EnumValueOptions options = 3;
}
message EnumValueOptions {
optional bool deprecated = 1 [default = false];
repeated UninterpretedOption uninterpreted_option = 999;
extensions 1000 to max;
}
message ExtensionRangeOptions {
repeated UninterpretedOption uninterpreted_option = 999;
extensions 1000 to max;
}
message FieldDescriptorProto {
optional string name = 1;
optional string extendee = 2;
optional int32 number = 3;
optional Label label = 4;
optional Type type = 5;
optional string type_name = 6;
optional string default_value = 7;
optional FieldOptions options = 8;
optional int32 oneof_index = 9;
optional string json_name = 10;
optional bool proto3_optional = 17;
enum Label {
LABEL_OPTIONAL = 1;
LABEL_REQUIRED = 2;
LABEL_REPEATED = 3;
}
enum Type {
TYPE_DOUBLE = 1;
TYPE_FLOAT = 2;
TYPE_INT64 = 3;
TYPE_UINT64 = 4;
TYPE_INT32 = 5;
TYPE_FIXED64 = 6;
TYPE_FIXED32 = 7;
TYPE_BOOL = 8;
TYPE_STRING = 9;
TYPE_GROUP = 10;
TYPE_MESSAGE = 11;
TYPE_BYTES = 12;
TYPE_UINT32 = 13;
TYPE_ENUM = 14;
TYPE_SFIXED32 = 15;
TYPE_SFIXED64 = 16;
TYPE_SINT32 = 17;
TYPE_SINT64 = 18;
}
}
message FieldOptions {
optional CType ctype = 1 [default = STRING];
optional bool packed = 2;
optional bool deprecated = 3 [default = false];
optional bool lazy = 5 [default = false];
optional JSType jstype = 6 [default = JS_NORMAL];
optional bool weak = 10 [default = false];
optional bool unverified_lazy = 15 [default = false];
repeated UninterpretedOption uninterpreted_option = 999;
enum CType {
STRING = 0;
CORD = 1;
STRING_PIECE = 2;
}
enum JSType {
JS_NORMAL = 0;
JS_STRING = 1;
JS_NUMBER = 2;
}
extensions 1000 to max;
reserved 4;
}
message FileDescriptorProto {
optional string name = 1;
optional string package = 2;
repeated string dependency = 3;
repeated DescriptorProto message_type = 4;
repeated EnumDescriptorProto enum_type = 5;
repeated ServiceDescriptorProto service = 6;
repeated FieldDescriptorProto extension = 7;
optional FileOptions options = 8;
optional SourceCodeInfo source_code_info = 9;
repeated int32 public_dependency = 10;
repeated int32 weak_dependency = 11;
optional string syntax = 12;
}
message FileDescriptorSet {
repeated FileDescriptorProto file = 1;
}
message FileOptions {
optional string java_package = 1;
optional string java_outer_classname = 8;
optional OptimizeMode optimize_for = 9 [default = SPEED];
optional bool java_multiple_files = 10 [default = false];
optional string go_package = 11;
optional bool cc_generic_services = 16 [default = false];
optional bool java_generic_services = 17 [default = false];
optional bool py_generic_services = 18 [default = false];
optional bool java_generate_equals_and_hash = 20 [deprecated = true];
optional bool deprecated = 23 [default = false];
optional bool java_string_check_utf8 = 27 [default = false];
optional bool cc_enable_arenas = 31 [default = true];
optional string objc_class_prefix = 36;
optional string csharp_namespace = 37;
optional string swift_prefix = 39;
optional string php_class_prefix = 40;
optional string php_namespace = 41;
optional bool php_generic_services = 42 [default = false];
optional string php_metadata_namespace = 44;
optional string ruby_package = 45;
repeated UninterpretedOption uninterpreted_option = 999;
enum OptimizeMode {
SPEED = 1;
CODE_SIZE = 2;
LITE_RUNTIME = 3;
}
extensions 1000 to max;
reserved 38;
}
message MessageOptions {
optional bool message_set_wire_format = 1 [default = false];
optional bool no_standard_descriptor_accessor = 2 [default = false];
optional bool deprecated = 3 [default = false];
optional bool map_entry = 7;
repeated UninterpretedOption uninterpreted_option = 999;
extensions 1000 to max;
reserved 4, 5, 6, 8, 9;
}
message MethodDescriptorProto {
optional string name = 1;
optional string input_type = 2;
optional string output_type = 3;
optional MethodOptions options = 4;
optional bool client_streaming = 5 [default = false];
optional bool server_streaming = 6 [default = false];
}
message MethodOptions {
optional bool deprecated = 33 [default = false];
optional IdempotencyLevel idempotency_level = 34 [default = IDEMPOTENCY_UNKNOWN];
repeated UninterpretedOption uninterpreted_option = 999;
enum IdempotencyLevel {
IDEMPOTENCY_UNKNOWN = 0;
NO_SIDE_EFFECTS = 1;
IDEMPOTENT = 2;
}
extensions 1000 to max;
}
message OneofDescriptorProto {
optional string name = 1;
optional OneofOptions options = 2;
}
message OneofOptions {
repeated UninterpretedOption uninterpreted_option = 999;
extensions 1000 to max;
}
message ServiceDescriptorProto {
optional string name = 1;
repeated MethodDescriptorProto method = 2;
optional ServiceOptions options = 3;
}
message ServiceOptions {
optional bool deprecated = 33 [default = false];
repeated UninterpretedOption uninterpreted_option = 999;
extensions 1000 to max;
}
message SourceCodeInfo {
repeated Location location = 1;
message Location {
repeated int32 path = 1 [packed = true];
repeated int32 span = 2 [packed = true];
optional string leading_comments = 3;
optional string trailing_comments = 4;
repeated string leading_detached_comments = 6;
}
}
message UninterpretedOption {
repeated NamePart name = 2;
optional string identifier_value = 3;
optional uint64 positive_int_value = 4;
optional int64 negative_int_value = 5;
optional double double_value = 6;
optional bytes string_value = 7;
optional string aggregate_value = 8;
message NamePart {
required string name_part = 1;
required bool is_extension = 2;
}
}
-- options.proto --
syntax = "proto3";
import "google/protobuf/descriptor.proto";
message Files {
google.protobuf.FileDescriptorSet files = 1;
}
message UnusedOption {
string foo = 1;
}
message UsedOption {
string foo = 1;
extend google.protobuf.FileOptions {
optional UsedOption file_foo = 50000;
optional UnusedOption file_bar = 50001;
optional string file_baz = 50002;
}
}
extend google.protobuf.EnumOptions {
optional UsedOption enum_foo = 50000;
optional UnusedOption enum_bar = 50001;
optional string enum_baz = 50002;
}
extend google.protobuf.EnumValueOptions {
optional UsedOption enum_value_foo = 50000;
optional UnusedOption enum_value_bar = 50001;
optional string enum_value_baz = 50002;
}
extend google.protobuf.FieldOptions {
optional UsedOption field_foo = 50000;
optional UnusedOption field_bar = 50001;
optional string field_baz = 50002;
}
extend google.protobuf.MessageOptions {
optional UsedOption message_foo = 50000;
optional UnusedOption message_bar = 50001;
optional string message_baz = 50002;
}
extend google.protobuf.MethodOptions {
optional UsedOption method_foo = 50000;
optional UnusedOption method_bar = 50001;
optional string method_baz = 50002;
}
extend google.protobuf.OneofOptions {
optional UsedOption oneof_foo = 50000;
optional UnusedOption oneof_bar = 50001;
optional string oneof_baz = 50002;
}
extend google.protobuf.ServiceOptions {
optional UsedOption service_foo = 50000;
optional UnusedOption service_bar = 50001;
optional string service_baz = 50002;
}
5 changes: 5 additions & 0 deletions private/bufpkg/bufimage/bufimageutil/testdata/options/a.proto
Expand Up @@ -48,6 +48,11 @@ service FooService {
option (method_foo).foo = "str";
option (method_baz) = "str";
};

rpc DoNot(Empty) returns (Empty) {
option (method_foo).foo = "str";
option (method_baz) = "str";
};
}

extend Foo {
Expand Down
Expand Up @@ -18,6 +18,7 @@ enum FooEnum {
}
service FooService {
rpc Do ( Empty ) returns ( Empty );
rpc DoNot ( Empty ) returns ( Empty );
}
extend Foo {
optional string extension = 11;
Expand Down

0 comments on commit 42e8e0d

Please sign in to comment.