From a2874ac001cde93052bb06e64f56419b86a3bfdf Mon Sep 17 00:00:00 2001 From: Joshua Humphries Date: Fri, 18 Feb 2022 22:40:34 -0500 Subject: [PATCH] no need for srcinforeflection package (#501) Thanks to https://github.com/grpc/grpc-go/pull/5197, no longer necessary But sourceinfo.GlobalFiles does need to implement add'l methods to serve extension info --- desc/sourceinfo/registry.go | 34 ++- .../srcinforeflection/serverreflection.go | 208 ------------------ .../serverreflection_test.go | 145 ------------ desc/sourceinfo/wrappers.go | 8 + 4 files changed, 37 insertions(+), 358 deletions(-) delete mode 100644 desc/sourceinfo/srcinforeflection/serverreflection.go delete mode 100644 desc/sourceinfo/srcinforeflection/serverreflection_test.go diff --git a/desc/sourceinfo/registry.go b/desc/sourceinfo/registry.go index eba91a86..ca34136d 100644 --- a/desc/sourceinfo/registry.go +++ b/desc/sourceinfo/registry.go @@ -12,10 +12,6 @@ // // So, by using the protoc-gen-gosrcinfo plugin and this package, we can recover the // source code info and comments that were otherwise stripped by protoc-gen-go. -// -// Also see the "github.com/jhump/protoreflect/desc/srcinfo/srcinforeflection" package -// for an implementation of the gRPC server reflection service that uses this package -// return descriptors with source code info. package sourceinfo import ( @@ -34,13 +30,19 @@ var ( // // If is mean to serve as a drop-in alternative to protoregistry.GlobalFiles that // can include source code info in the returned descriptors. - GlobalFiles protodesc.Resolver = registry{} + GlobalFiles Resolver = registry{} mu sync.RWMutex sourceInfoByFile = map[string]*descriptorpb.SourceCodeInfo{} fileDescriptors = map[protoreflect.FileDescriptor]protoreflect.FileDescriptor{} ) +type Resolver interface { + protodesc.Resolver + protoregistry.ExtensionTypeResolver + RangeExtensionsByMessage(message protoreflect.FullName, f func(protoreflect.ExtensionType) bool) +} + // RegisterSourceInfo registers the given source code info for the file descriptor // with the given path/name. // @@ -138,3 +140,25 @@ func (r registry) FindDescriptorByName(name protoreflect.FullName) (protoreflect return nil, fmt.Errorf("unrecognized descriptor type: %T", d) } } + +func (r registry) FindExtensionByName(field protoreflect.FullName) (protoreflect.ExtensionType, error) { + xt, err := protoregistry.GlobalTypes.FindExtensionByName(field) + if err != nil { + return nil, err + } + return extensionType{xt}, nil +} + +func (r registry) FindExtensionByNumber(message protoreflect.FullName, field protoreflect.FieldNumber) (protoreflect.ExtensionType, error) { + xt, err := protoregistry.GlobalTypes.FindExtensionByNumber(message, field) + if err != nil { + return nil, err + } + return extensionType{xt}, nil +} + +func (r registry) RangeExtensionsByMessage(message protoreflect.FullName, fn func(protoreflect.ExtensionType) bool) { + protoregistry.GlobalTypes.RangeExtensionsByMessage(message, func(xt protoreflect.ExtensionType) bool { + return fn(extensionType{xt}) + }) +} diff --git a/desc/sourceinfo/srcinforeflection/serverreflection.go b/desc/sourceinfo/srcinforeflection/serverreflection.go deleted file mode 100644 index 1c12795a..00000000 --- a/desc/sourceinfo/srcinforeflection/serverreflection.go +++ /dev/null @@ -1,208 +0,0 @@ -// Package srcinforeflection provides an implementation of server reflection -// that includes source code info, if the protoc-gen-gosrcinfo plugin was used -// for the files that contain the descriptors being served. This allows for -// sending comment information to dynamic/reflective clients. -// -// NOTE: This package is EXPERIMENTAL! It may not see the light of day in an -// actual release. Instead, the hope is that the core gRPC reflection package -// can be updated in a way to more easily accommodate customizations, such as -// the caller wanting to use sourceinfo.GlobalFiles as the source of descriptors. -// Related: https://github.com/grpc/grpc-go/pull/5197 -package srcinforeflection - -import ( - "io" - "sort" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/reflection" - rpb "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" - "google.golang.org/grpc/status" - "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/reflect/protodesc" - "google.golang.org/protobuf/reflect/protoreflect" - "google.golang.org/protobuf/reflect/protoregistry" - - "github.com/jhump/protoreflect/desc/sourceinfo" -) - -// NB: This was forked from the implementation in google.golang.org/grpc/reflection. -// However, this implementation is very different (and MUCH more concise) since it -// uses the v2 API for protobuf. In addition to this difference, it also uses -// the sourceinfo package to load file descriptors, which will merge in source -// code information (which contains information about element locations and comments). -type serverReflectionServer struct { - s reflection.GRPCServer -} - -// Register registers the server reflection service on the given gRPC server. -func Register(s reflection.GRPCServer) { - rpb.RegisterServerReflectionServer(s, &serverReflectionServer{s: s}) -} - -func (s *serverReflectionServer) listServices() []string { - svcInfo := s.s.GetServiceInfo() - svcNames := make([]string, 0, len(svcInfo)) - for n := range svcInfo { - svcNames = append(svcNames, n) - } - sort.Strings(svcNames) - return svcNames -} - -// fileDescWithDependencies returns a slice of serialized fileDescriptors in -// wire format ([]byte). The fileDescriptors will include fd and all the -// transitive dependencies of fd with names not in sentFileDescriptors. -func fileDescWithDependencies(file string, sentFileDescriptors map[string]struct{}) ([][]byte, error) { - fd, err := sourceinfo.GlobalFiles.FindFileByPath(file) - if err != nil { - return nil, err - } - var r [][]byte - queue := []protoreflect.FileDescriptor{fd} - for len(queue) > 0 { - currentfd := queue[0] - queue = queue[1:] - if _, sent := sentFileDescriptors[currentfd.Path()]; len(r) == 0 || !sent { - sentFileDescriptors[currentfd.Path()] = struct{}{} - fdProto := protodesc.ToFileDescriptorProto(fd) - currentfdEncoded, err := proto.Marshal(fdProto) - if err != nil { - return nil, err - } - r = append(r, currentfdEncoded) - } - for i := 0; i < currentfd.Imports().Len(); i++ { - queue = append(queue, currentfd.Imports().Get(i)) - } - } - return r, nil -} - -// fileDescEncodingContainingSymbol finds the file descriptor containing the -// given symbol, finds all of its previously unsent transitive dependencies, -// does marshalling on them, and returns the marshalled result. The given symbol -// can be a type, a service or a method. -func fileDescEncodingContainingSymbol(name string, sentFileDescriptors map[string]struct{}) ([][]byte, error) { - d, err := protoregistry.GlobalFiles.FindDescriptorByName(protoreflect.FullName(name)) - if err != nil { - return nil, err - } - return fileDescWithDependencies(d.ParentFile().Path(), sentFileDescriptors) -} - -// fileDescEncodingContainingExtension finds the file descriptor containing -// given extension, finds all of its previously unsent transitive dependencies, -// does marshalling on them, and returns the marshalled result. -func fileDescEncodingContainingExtension(typeName string, extNum int32, sentFileDescriptors map[string]struct{}) ([][]byte, error) { - xt, err := protoregistry.GlobalTypes.FindExtensionByNumber(protoreflect.FullName(typeName), protoreflect.FieldNumber(extNum)) - if err != nil { - return nil, err - } - return fileDescWithDependencies(xt.TypeDescriptor().ParentFile().Path(), sentFileDescriptors) -} - -// allExtensionNumbersForTypeName returns all extension numbers for the given type. -func allExtensionNumbersForTypeName(name string) []int32 { - var numbers []int32 - protoregistry.GlobalTypes.RangeExtensionsByMessage(protoreflect.FullName(name), func(xt protoreflect.ExtensionType) bool { - numbers = append(numbers, int32(xt.TypeDescriptor().Number())) - return true - }) - sort.Slice(numbers, func(i, j int) bool { - return numbers[i] < numbers[j] - }) - return numbers -} - -// ServerReflectionInfo is the reflection service handler. -func (s *serverReflectionServer) ServerReflectionInfo(stream rpb.ServerReflection_ServerReflectionInfoServer) error { - sentFileDescriptors := map[string]struct{}{} - for { - in, err := stream.Recv() - if err == io.EOF { - return nil - } - if err != nil { - return err - } - - out := &rpb.ServerReflectionResponse{ - ValidHost: in.Host, - OriginalRequest: in, - } - switch req := in.MessageRequest.(type) { - case *rpb.ServerReflectionRequest_FileByFilename: - b, err := fileDescWithDependencies(req.FileByFilename, sentFileDescriptors) - if err != nil { - out.MessageResponse = &rpb.ServerReflectionResponse_ErrorResponse{ - ErrorResponse: &rpb.ErrorResponse{ - ErrorCode: int32(codes.NotFound), - ErrorMessage: err.Error(), - }, - } - } else { - out.MessageResponse = &rpb.ServerReflectionResponse_FileDescriptorResponse{ - FileDescriptorResponse: &rpb.FileDescriptorResponse{FileDescriptorProto: b}, - } - } - case *rpb.ServerReflectionRequest_FileContainingSymbol: - b, err := fileDescEncodingContainingSymbol(req.FileContainingSymbol, sentFileDescriptors) - if err != nil { - out.MessageResponse = &rpb.ServerReflectionResponse_ErrorResponse{ - ErrorResponse: &rpb.ErrorResponse{ - ErrorCode: int32(codes.NotFound), - ErrorMessage: err.Error(), - }, - } - } else { - out.MessageResponse = &rpb.ServerReflectionResponse_FileDescriptorResponse{ - FileDescriptorResponse: &rpb.FileDescriptorResponse{FileDescriptorProto: b}, - } - } - case *rpb.ServerReflectionRequest_FileContainingExtension: - typeName := req.FileContainingExtension.ContainingType - extNum := req.FileContainingExtension.ExtensionNumber - b, err := fileDescEncodingContainingExtension(typeName, extNum, sentFileDescriptors) - if err != nil { - out.MessageResponse = &rpb.ServerReflectionResponse_ErrorResponse{ - ErrorResponse: &rpb.ErrorResponse{ - ErrorCode: int32(codes.NotFound), - ErrorMessage: err.Error(), - }, - } - } else { - out.MessageResponse = &rpb.ServerReflectionResponse_FileDescriptorResponse{ - FileDescriptorResponse: &rpb.FileDescriptorResponse{FileDescriptorProto: b}, - } - } - case *rpb.ServerReflectionRequest_AllExtensionNumbersOfType: - extNums := allExtensionNumbersForTypeName(req.AllExtensionNumbersOfType) - out.MessageResponse = &rpb.ServerReflectionResponse_AllExtensionNumbersResponse{ - AllExtensionNumbersResponse: &rpb.ExtensionNumberResponse{ - BaseTypeName: req.AllExtensionNumbersOfType, - ExtensionNumber: extNums, - }, - } - case *rpb.ServerReflectionRequest_ListServices: - svcNames := s.listServices() - serviceResponses := make([]*rpb.ServiceResponse, len(svcNames)) - for i, n := range svcNames { - serviceResponses[i] = &rpb.ServiceResponse{ - Name: n, - } - } - out.MessageResponse = &rpb.ServerReflectionResponse_ListServicesResponse{ - ListServicesResponse: &rpb.ListServiceResponse{ - Service: serviceResponses, - }, - } - default: - return status.Errorf(codes.InvalidArgument, "invalid MessageRequest: %v", in.MessageRequest) - } - - if err := stream.Send(out); err != nil { - return err - } - } -} diff --git a/desc/sourceinfo/srcinforeflection/serverreflection_test.go b/desc/sourceinfo/srcinforeflection/serverreflection_test.go deleted file mode 100644 index 0a25b03f..00000000 --- a/desc/sourceinfo/srcinforeflection/serverreflection_test.go +++ /dev/null @@ -1,145 +0,0 @@ -package srcinforeflection - -import ( - "context" - "fmt" - "net" - "testing" - "time" - - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" - rpb "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" - "google.golang.org/protobuf/types/descriptorpb" - - "github.com/jhump/protoreflect/desc" - "github.com/jhump/protoreflect/grpcreflect" - "github.com/jhump/protoreflect/internal/testprotos" - "github.com/jhump/protoreflect/internal/testutil" -) - -func TestReflectionService(t *testing.T) { - svc := grpc.NewServer() - testprotos.RegisterRpcServiceServer(svc, &testprotos.UnimplementedRpcServiceServer{}) - Register(svc) - l, err := net.Listen("tcp", "127.0.0.1:0") - testutil.Ok(t, err) - go func() { - if err := svc.Serve(l); err != nil { - t.Logf("error from gRPC server: %v", err) - } - }() - defer func() { - _ = svc.Stop - }() - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - cc, err := grpc.DialContext(ctx, l.Addr().String(), - grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithBlock()) - testutil.Ok(t, err) - defer func() { - _ = cc.Close() - }() - - stub := rpb.NewServerReflectionClient(cc) - ctx, cancel = context.WithCancel(context.Background()) - defer cancel() - cli := grpcreflect.NewClient(ctx, stub) - defer cli.Reset() - - t.Run("ListServices", func(t *testing.T) { - svcs, err := cli.ListServices() - testutil.Ok(t, err) - testutil.Eq(t, []string{"foo.bar.RpcService", "grpc.reflection.v1alpha.ServerReflection"}, svcs) - }) - - t.Run("FileContainingSymbol", func(t *testing.T) { - fd, err := cli.FileContainingSymbol("foo.bar.RpcService") - testutil.Ok(t, err) - d := fd.FindSymbol("foo.bar.RpcService") - testutil.Eq(t, " Service comment\n", d.GetSourceInfo().GetLeadingComments()) - md := d.(*desc.ServiceDescriptor).FindMethodByName("StreamingRpc") - testutil.Eq(t, " Method comment\n", md.GetSourceInfo().GetLeadingComments()) - md = d.(*desc.ServiceDescriptor).FindMethodByName("UnaryRpc") - testutil.Eq(t, " trailer for method\n", md.GetSourceInfo().GetTrailingComments()) - }) - - t.Run("FileByFilename", func(t *testing.T) { - fd, err := cli.FileByFilename("desc_test1.proto") - testutil.Ok(t, err) - checkFileComments(t, fd) - }) - - t.Run("FileContainingExtension", func(t *testing.T) { - fd, err := cli.FileContainingExtension("testprotos.AnotherTestMessage", 100) - testutil.Ok(t, err) - testutil.Eq(t, "desc_test1.proto", fd.GetName()) - checkFileComments(t, fd) - }) - - t.Run("AllExtensionsByType", func(t *testing.T) { - nums, err := cli.AllExtensionNumbersForType("testprotos.AnotherTestMessage") - testutil.Ok(t, err) - testutil.Eq(t, []int32{100, 101, 102, 103, 200}, nums) - }) -} - -func checkFileComments(t *testing.T, fd *desc.FileDescriptor) { - for _, md := range fd.GetMessageTypes() { - checkMessageComments(t, md) - } - for _, ed := range fd.GetEnumTypes() { - checkEnumComments(t, ed) - } - for _, exd := range fd.GetExtensions() { - checkComment(t, exd) - } - for _, sd := range fd.GetServices() { - checkComment(t, sd) - for _, mtd := range sd.GetMethods() { - checkComment(t, mtd) - } - } -} - -func checkMessageComments(t *testing.T, md *desc.MessageDescriptor) { - checkComment(t, md) - - for _, fld := range md.GetFields() { - if fld.GetType() == descriptorpb.FieldDescriptorProto_TYPE_GROUP { - continue // comment is attributed to group message, not field - } - checkComment(t, fld) - } - for _, od := range md.GetOneOfs() { - checkComment(t, od) - } - - for _, nmd := range md.GetNestedMessageTypes() { - if nmd.IsMapEntry() { - // synthetic map entry messages won't have comments - continue - } - checkMessageComments(t, nmd) - } - for _, ed := range md.GetNestedEnumTypes() { - checkEnumComments(t, ed) - } - for _, exd := range md.GetNestedExtensions() { - checkComment(t, exd) - } -} - -func checkEnumComments(t *testing.T, ed *desc.EnumDescriptor) { - checkComment(t, ed) - for _, evd := range ed.GetValues() { - checkComment(t, evd) - } -} - -func checkComment(t *testing.T, d desc.Descriptor) { - cmt := fmt.Sprintf(" Comment for %s\n", d.GetName()) - testutil.Eq(t, cmt, d.GetSourceInfo().GetLeadingComments()) -} diff --git a/desc/sourceinfo/wrappers.go b/desc/sourceinfo/wrappers.go index 108c4592..134dadf4 100644 --- a/desc/sourceinfo/wrappers.go +++ b/desc/sourceinfo/wrappers.go @@ -498,3 +498,11 @@ func (m methodDescriptor) Input() protoreflect.MessageDescriptor { func (m methodDescriptor) Output() protoreflect.MessageDescriptor { return messageDescriptor{m.MethodDescriptor.Output()} } + +type extensionType struct { + protoreflect.ExtensionType +} + +func (e extensionType) TypeDescriptor() protoreflect.ExtensionTypeDescriptor { + return extensionDescriptor{e.ExtensionType.TypeDescriptor()} +}