Refine Logic for Handling Nested Types (#38)

zhihuizhang17 · web-flow · commit 0e7662cbde54 · 2024-06-14T17:18:00.000-04:00
* deal with nested types

* fix unit tests

* add unit tests

* add unit tests cases

* address comments

* mix format

* add unit test cases

* bump version
diff --git a/lib/grpc_reflection/service/builder.ex b/lib/grpc_reflection/service/builder.ex
@@ -80,14 +80,29 @@ defmodule GrpcReflection.Service.Builder do
       descriptor = mod.descriptor()
       name = symbol
 
-      referenced_types = Util.types_from_descriptor(descriptor)
-      unencoded_payload = process_common(name, descriptor, Util.get_syntax(mod))
+      nested_types = Util.get_nested_types(name, descriptor)
+
+      referenced_types =
+        Util.types_from_descriptor(descriptor)
+        |> Enum.uniq()
+        |> Kernel.--(nested_types)
+
+      unencoded_payload = process_common(name, descriptor, Util.get_syntax(mod), nested_types)
       payload = FileDescriptorProto.encode(unencoded_payload)
       response = %{file_descriptor_proto: [payload]}
 
       root_symbols = %{symbol => response}
+
+      root_symbols =
+        Enum.reduce(nested_types, root_symbols, fn name, acc -> Map.put(acc, name, response) end)
+
       root_files = %{(symbol <> ".proto") => response}
 
+      root_files =
+        Enum.reduce(nested_types, root_files, fn name, acc ->
+          Map.put(acc, name <> ".proto", response)
+        end)
+
       extension_file = symbol <> "Extension.proto"
 
       {root_extensions, root_files} =
@@ -159,21 +174,20 @@ defmodule GrpcReflection.Service.Builder do
 
   defp process_extensions(_, _, _, _), do: {:ignore, {nil, nil}}
 
-  defp process_common(name, descriptor, syntax) do
-    package = Util.get_package(name)
-
+  defp process_common(name, descriptor, syntax, nested_types \\ []) do
     dependencies =
       descriptor
       |> Util.types_from_descriptor()
+      |> Enum.uniq()
+      |> Kernel.--(nested_types)
       |> Enum.map(fn name ->
         name <> ".proto"
       end)
-      |> Enum.uniq()
 
     response_stub =
       %FileDescriptorProto{
         name: name <> ".proto",
-        package: package,
+        package: Util.get_package(name),
         dependency: dependencies,
         syntax: syntax
       }
diff --git a/lib/grpc_reflection/service/builder/util.ex b/lib/grpc_reflection/service/builder/util.ex
@@ -7,6 +7,22 @@ defmodule GrpcReflection.Service.Builder.Util do
 
   @type_message Map.fetch!(Google.Protobuf.FieldDescriptorProto.Type.mapping(), :TYPE_MESSAGE)
 
+  def get_nested_types(symbol, descriptor), do: get_nested_types(symbol, descriptor, [])
+
+  def get_nested_types(_symbol, %Google.Protobuf.DescriptorProto{nested_type: []}, acc) do
+    acc
+  end
+
+  def get_nested_types(symbol, %Google.Protobuf.DescriptorProto{nested_type: nested_types}, acc) do
+    Enum.reduce(nested_types, acc, fn nested_type, acc ->
+      new_symbol = symbol <> "." <> nested_type.name
+      new_acc = [new_symbol | acc]
+      get_nested_types(new_symbol, nested_type, new_acc)
+    end)
+  end
+
+  def get_nested_types(_, _, acc), do: acc
+
   def get_package(symbol) do
     parent_symbol = symbol |> String.split(".") |> Enum.slice(0..-2//1) |> Enum.join(".")
 
diff --git a/mix.exs b/mix.exs
@@ -1,7 +1,7 @@
 defmodule GrpcReflection.MixProject do
   use Mix.Project
 
-  @version "0.1.4"
+  @version "0.1.5"
   @source_url "https://github.com/elixir-grpc/grpc-reflection"
   @description "gRPC reflection server for Elixir"
 
diff --git a/priv/protos/test_service_v3.proto b/priv/protos/test_service_v3.proto
@@ -29,6 +29,11 @@ message TestRequest {
     }
 
     Location location = 2;
+    Token token = 3;
+  }
+
+  message Token {
+    string vaule = 1;
   }
 
   Payload payload = 7;
diff --git a/test/builder/util_test.exs b/test/builder/util_test.exs
@@ -35,6 +35,19 @@ defmodule GrpcReflection.Service.Builder.UtilTest do
         Util.convert_symbol_to_module("testservice.TestRequest.Payload.Location")
       end
     end
+
+    test "get all nested types" do
+      assert [
+               "testserviceV3.TestRequest.Token",
+               "testserviceV3.TestRequest.Payload.Location",
+               "testserviceV3.TestRequest.Payload",
+               "testserviceV3.TestRequest.GEntry"
+             ] ==
+               Util.get_nested_types(
+                 "testserviceV3.TestRequest",
+                 TestserviceV3.TestRequest.descriptor()
+               )
+    end
   end
 
   describe "utils for dealing with proto2 only" do
diff --git a/test/builder_test.exs b/test/builder_test.exs
@@ -19,6 +19,7 @@ defmodule GrpcReflection.BuilderTest do
              "testserviceV3.TestRequest.GEntry.proto",
              "testserviceV3.TestRequest.Payload.Location.proto",
              "testserviceV3.TestRequest.Payload.proto",
+             "testserviceV3.TestRequest.Token.proto",
              "testserviceV3.TestRequest.proto",
              "testserviceV3.TestService.proto"
            ]
@@ -33,6 +34,7 @@ defmodule GrpcReflection.BuilderTest do
              "testserviceV3.TestRequest.GEntry",
              "testserviceV3.TestRequest.Payload",
              "testserviceV3.TestRequest.Payload.Location",
+             "testserviceV3.TestRequest.Token",
              "testserviceV3.TestService",
              "testserviceV3.TestService.CallFunction"
            ]
diff --git a/test/support/protos/test_service_v3.pb.ex b/test/support/protos/test_service_v3.pb.ex
@@ -180,6 +180,20 @@ defmodule TestserviceV3.TestRequest.Payload do
           json_name: "location",
           proto3_optional: nil,
           __unknown_fields__: []
+        },
+        %Google.Protobuf.FieldDescriptorProto{
+          name: "token",
+          extendee: nil,
+          number: 3,
+          label: :LABEL_OPTIONAL,
+          type: :TYPE_MESSAGE,
+          type_name: ".testserviceV3.TestRequest.Token",
+          default_value: nil,
+          options: nil,
+          oneof_index: nil,
+          json_name: "token",
+          proto3_optional: nil,
+          __unknown_fields__: []
         }
       ],
       nested_type: [
@@ -239,6 +253,45 @@ defmodule TestserviceV3.TestRequest.Payload do
 
   field :data, 1, type: Google.Protobuf.StringValue
   field :location, 2, type: TestserviceV3.TestRequest.Payload.Location
+  field :token, 3, type: TestserviceV3.TestRequest.Token
+end
+
+defmodule TestserviceV3.TestRequest.Token do
+  use Protobuf, protoc_gen_elixir_version: "0.12.0", syntax: :proto3
+
+  def descriptor do
+    # credo:disable-for-next-line
+    %Google.Protobuf.DescriptorProto{
+      name: "Token",
+      field: [
+        %Google.Protobuf.FieldDescriptorProto{
+          name: "vaule",
+          extendee: nil,
+          number: 1,
+          label: :LABEL_OPTIONAL,
+          type: :TYPE_STRING,
+          type_name: nil,
+          default_value: nil,
+          options: nil,
+          oneof_index: nil,
+          json_name: "vaule",
+          proto3_optional: nil,
+          __unknown_fields__: []
+        }
+      ],
+      nested_type: [],
+      enum_type: [],
+      extension_range: [],
+      extension: [],
+      options: nil,
+      oneof_decl: [],
+      reserved_range: [],
+      reserved_name: [],
+      __unknown_fields__: []
+    }
+  end
+
+  field :vaule, 1, type: :string
 end
 
 defmodule TestserviceV3.TestRequest do
@@ -430,6 +483,20 @@ defmodule TestserviceV3.TestRequest do
               json_name: "location",
               proto3_optional: nil,
               __unknown_fields__: []
+            },
+            %Google.Protobuf.FieldDescriptorProto{
+              name: "token",
+              extendee: nil,
+              number: 3,
+              label: :LABEL_OPTIONAL,
+              type: :TYPE_MESSAGE,
+              type_name: ".testserviceV3.TestRequest.Token",
+              default_value: nil,
+              options: nil,
+              oneof_index: nil,
+              json_name: "token",
+              proto3_optional: nil,
+              __unknown_fields__: []
             }
           ],
           nested_type: [
@@ -484,6 +551,34 @@ defmodule TestserviceV3.TestRequest do
           reserved_range: [],
           reserved_name: [],
           __unknown_fields__: []
+        },
+        %Google.Protobuf.DescriptorProto{
+          name: "Token",
+          field: [
+            %Google.Protobuf.FieldDescriptorProto{
+              name: "vaule",
+              extendee: nil,
+              number: 1,
+              label: :LABEL_OPTIONAL,
+              type: :TYPE_STRING,
+              type_name: nil,
+              default_value: nil,
+              options: nil,
+              oneof_index: nil,
+              json_name: "vaule",
+              proto3_optional: nil,
+              __unknown_fields__: []
+            }
+          ],
+          nested_type: [],
+          enum_type: [],
+          extension_range: [],
+          extension: [],
+          options: nil,
+          oneof_decl: [],
+          reserved_range: [],
+          reserved_name: [],
+          __unknown_fields__: []
         }
       ],
       enum_type: [],
@@ -616,4 +711,4 @@ end
 
 defmodule TestserviceV3.TestService.Stub do
   use GRPC.Stub, service: TestserviceV3.TestService.Service
-end
+end
diff --git a/test/v1_reflection_test.exs b/test/v1_reflection_test.exs
@@ -65,12 +65,18 @@ defmodule GrpcReflection.V1ReflectionTest do
       assert {:error, _} = run_request(message, ctx)
     end
 
-    test "describing a type returns the type", ctx do
+    test "describing a root type returns the type", ctx do
       message = {:file_containing_symbol, "helloworld.HelloRequest"}
       assert {:ok, response} = run_request(message, ctx)
       assert_response(response)
     end
 
+    test "describing a nested type returns the root type", ctx do
+      message = {:file_containing_symbol, "testserviceV3.TestRequest.Payload"}
+      assert {:ok, response} = run_request(message, ctx)
+      assert response.name == "testserviceV3.TestRequest.proto"
+    end
+
     test "type with leading period still resolves", ctx do
       message = {:file_containing_symbol, ".helloworld.HelloRequest"}
       assert {:ok, response} = run_request(message, ctx)
@@ -173,7 +179,7 @@ defmodule GrpcReflection.V1ReflectionTest do
              ]
     end
 
-    test "ensure inclusion of nested types in file descriptor dependencies", ctx do
+    test "ensure exclusion of nested types in file descriptor dependencies", ctx do
       filename = "testserviceV3.TestRequest.proto"
       message = {:file_by_filename, filename}
       assert {:ok, response} = run_request(message, ctx)
@@ -182,11 +188,8 @@ defmodule GrpcReflection.V1ReflectionTest do
 
       assert response.dependency == [
                "testserviceV3.Enum.proto",
-               "testserviceV3.TestRequest.GEntry.proto",
                "google.protobuf.Any.proto",
-               "testserviceV3.TestRequest.Payload.proto",
-               "google.protobuf.StringValue.proto",
-               "testserviceV3.TestRequest.Payload.Location.proto"
+               "google.protobuf.StringValue.proto"
              ]
     end
   end
diff --git a/test/v1alpha_reflection_test.exs b/test/v1alpha_reflection_test.exs
@@ -65,12 +65,18 @@ defmodule GrpcReflection.V1alphaReflectionTest do
       assert {:error, _} = run_request(message, ctx)
     end
 
-    test "describing a type returns the type", ctx do
+    test "describing a root type returns the type", ctx do
       message = {:file_containing_symbol, "helloworld.HelloRequest"}
       assert {:ok, response} = run_request(message, ctx)
       assert_response(response)
     end
 
+    test "describing a nested type returns the root type", ctx do
+      message = {:file_containing_symbol, "testserviceV3.TestRequest.Payload"}
+      assert {:ok, response} = run_request(message, ctx)
+      assert response.name == "testserviceV3.TestRequest.proto"
+    end
+
     test "type with leading period still resolves", ctx do
       message = {:file_containing_symbol, ".helloworld.HelloRequest"}
       assert {:ok, response} = run_request(message, ctx)
@@ -173,7 +179,7 @@ defmodule GrpcReflection.V1alphaReflectionTest do
              ]
     end
 
-    test "ensure inclusion of nested types in file descriptor dependencies", ctx do
+    test "ensure exclusion of nested types in file descriptor dependencies", ctx do
       filename = "testserviceV3.TestRequest.proto"
       message = {:file_by_filename, filename}
       assert {:ok, response} = run_request(message, ctx)
@@ -182,11 +188,8 @@ defmodule GrpcReflection.V1alphaReflectionTest do
 
       assert response.dependency == [
                "testserviceV3.Enum.proto",
-               "testserviceV3.TestRequest.GEntry.proto",
                "google.protobuf.Any.proto",
-               "testserviceV3.TestRequest.Payload.proto",
-               "google.protobuf.StringValue.proto",
-               "testserviceV3.TestRequest.Payload.Location.proto"
+               "google.protobuf.StringValue.proto"
              ]
     end
   end

Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,11 @@ message TestRequest {`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`Location location = 2;`
	`32`	`+ Token token = 3;`
	`33`	`+ }`
	`34`	`+`
	`35`	`+ message Token {`
	`36`	`+ string vaule = 1;`
`32`	`37`	`}`
`33`	`38`
`34`	`39`	`Payload payload = 7;`