move descriptor encode to application boundary (#66)

Author: mjheilmann

lib/grpc_reflection/server/v1.ex

@@ -27,9 +27,11 @@ defmodule GrpcReflection.Server.V1 do
     end)
   end
 
-  defp build_response({:ok, {:file_descriptor_response, description}}) do
-    encoded = struct(Grpc.Reflection.V1.FileDescriptorResponse, description)
-    {:file_descriptor_response, encoded}
+  defp build_response({:ok, {:file_descriptor_response, file_descriptor}}) do
+    {:file_descriptor_response,
+     %Grpc.Reflection.V1.FileDescriptorResponse{
+       file_descriptor_proto: [Google.Protobuf.FileDescriptorProto.encode(file_descriptor)]
+     }}
   end
 
   defp build_response({:ok, {:all_extension_numbers_response, body}}) do

lib/grpc_reflection/server/v1alpha.ex

@@ -27,19 +27,19 @@ defmodule GrpcReflection.Server.V1alpha do
     end)
   end
 
-  defp build_response({:ok, {:file_descriptor_response, description}}) do
-    encoded = struct(Grpc.Reflection.V1alpha.FileDescriptorResponse, description)
-    {:file_descriptor_response, encoded}
+  defp build_response({:ok, {:file_descriptor_response, file_descriptor}}) do
+    {:file_descriptor_response,
+     %Grpc.Reflection.V1alpha.FileDescriptorResponse{
+       file_descriptor_proto: [Google.Protobuf.FileDescriptorProto.encode(file_descriptor)]
+     }}
   end
 
   defp build_response({:ok, {:all_extension_numbers_response, body}}) do
-    encoded =
-      struct(
-        Grpc.Reflection.V1alpha.ExtensionNumberResponse,
-        body
-      )
-
-    {:all_extension_numbers_response, encoded}
+    {:all_extension_numbers_response,
+     struct(
+       Grpc.Reflection.V1alpha.ExtensionNumberResponse,
+       body
+     )}
   end
 
   defp build_response({:ok, {:list_services_response, %{service: services}}}) do

lib/grpc_reflection/service/builder.ex

@@ -132,14 +132,11 @@ defmodule GrpcReflection.Service.Builder do
         syntax: syntax
       }
 
-    unencoded_payload =
-      case descriptor = descriptor do
-        %Google.Protobuf.DescriptorProto{} -> %{response_stub | message_type: [descriptor]}
-        %Google.Protobuf.ServiceDescriptorProto{} -> %{response_stub | service: [descriptor]}
-        %Google.Protobuf.EnumDescriptorProto{} -> %{response_stub | enum_type: [descriptor]}
-      end
-
-    %{file_descriptor_proto: [FileDescriptorProto.encode(unencoded_payload)]}
+    case descriptor = descriptor do
+      %Google.Protobuf.DescriptorProto{} -> %{response_stub | message_type: [descriptor]}
+      %Google.Protobuf.ServiceDescriptorProto{} -> %{response_stub | service: [descriptor]}
+      %Google.Protobuf.EnumDescriptorProto{} -> %{response_stub | enum_type: [descriptor]}
+    end
   end
 
   # protoc with the elixir generator and protobuf.generate slightly differ for how they

lib/grpc_reflection/service/builder/extensions.ex

@@ -1,7 +1,6 @@
 defmodule GrpcReflection.Service.Builder.Extensions do
   @moduledoc false
 
-  alias Google.Protobuf.FileDescriptorProto
   alias GrpcReflection.Service.Builder.Util
   alias GrpcReflection.Service.State
 
@@ -11,7 +10,7 @@ defmodule GrpcReflection.Service.Builder.Extensions do
     case process_extensions(module, symbol, extension_file, module.descriptor()) do
       {:ok, {extension_numbers, extension_payload}} ->
         state
-        |> State.add_files(%{extension_file => %{file_descriptor_proto: [extension_payload]}})
+        |> State.add_files(%{extension_file => extension_payload})
         |> State.add_extensions(%{symbol => extension_numbers})
 
       :ignore ->
@@ -50,9 +49,7 @@ defmodule GrpcReflection.Service.Builder.Extensions do
       message_type: message_list
     }
 
-    {:ok,
-     {Enum.map(extensions, fn {idx, _, _} -> idx end),
-      FileDescriptorProto.encode(unencoded_extension_payload)}}
+    {:ok, {Enum.map(extensions, fn {idx, _, _} -> idx end), unencoded_extension_payload}}
   end
 
   defp process_extensions(_, _, _, _), do: :ignore

test/service/builder_test.exs

@@ -39,10 +39,7 @@ defmodule GrpcReflection.Service.BuilderTest do
              "testserviceV3.TestService.CallFunction"
            ]
 
-    (Map.values(tree.files) ++ Map.values(tree.symbols))
-    |> Enum.flat_map(&Map.get(&1, :file_descriptor_proto))
-    |> Enum.map(&Google.Protobuf.FileDescriptorProto.decode/1)
-    |> Enum.each(fn payload ->
+    Enum.each(Map.values(tree.files) ++ Map.values(tree.symbols), fn payload ->
       assert payload.syntax == "proto3"
     end)
   end
@@ -80,13 +77,9 @@ defmodule GrpcReflection.Service.BuilderTest do
     # this is a bitstring that may contain whitespace characters
     assert extensions |> to_string() |> String.trim() == ""
 
-    (Map.values(tree.files) ++ Map.values(tree.symbols))
-    |> Enum.flat_map(&Map.get(&1, :file_descriptor_proto))
-    |> Enum.map(&Google.Protobuf.FileDescriptorProto.decode/1)
-    |> Enum.each(fn
-      # the google types are proto3
-      %{name: "google" <> _} = payload -> assert payload.syntax == "proto3"
-      payload -> assert payload.syntax == "proto2"
+    Enum.each(Map.values(tree.files) ++ Map.values(tree.symbols), fn
+      %{name: "google" <> _, syntax: syntax} -> assert syntax == "proto3"
+      %{name: _, syntax: syntax} -> assert syntax == "proto2"
     end)
   end
 
@@ -95,8 +88,6 @@ defmodule GrpcReflection.Service.BuilderTest do
     assert %State{services: [EmptyService.Service]} = tree
 
     (Map.values(tree.files) ++ Map.values(tree.symbols))
-    |> Enum.flat_map(&Map.get(&1, :file_descriptor_proto))
-    |> Enum.map(&Google.Protobuf.FileDescriptorProto.decode/1)
     |> Enum.each(fn payload ->
       # empty services default to proto2
       assert payload.syntax == "proto2"
@@ -121,8 +112,6 @@ defmodule GrpcReflection.Service.BuilderTest do
 
     names =
       (Map.values(tree.files) ++ Map.values(tree.symbols))
-      |> Enum.flat_map(&Map.get(&1, :file_descriptor_proto))
-      |> Enum.map(&Google.Protobuf.FileDescriptorProto.decode/1)
       |> Enum.map(& &1.name)
 
     assert names ==

test/service/server_test.exs

@@ -20,10 +20,10 @@ defmodule GrpcReflection.ServerTest do
 
     assert Service.list_services() == ["helloworld.Greeter"]
 
-    assert {:ok, %{file_descriptor_proto: proto}} =
+    assert {:ok, descriptor} =
              Service.get_by_symbol("helloworld.Greeter")
 
-    assert {:ok, %{file_descriptor_proto: ^proto}} =
+    assert {:ok, ^descriptor} =
              Service.get_by_filename("helloworld.Greeter.proto")
   end
 
@@ -45,26 +45,26 @@ defmodule GrpcReflection.ServerTest do
     end
 
     test "method files return service descriptors" do
-      assert {:ok, %{file_descriptor_proto: proto}} =
+      assert {:ok, descriptor} =
                Service.get_by_symbol("helloworld.Greeter")
 
-      assert {:ok, %{file_descriptor_proto: ^proto}} =
+      assert {:ok, ^descriptor} =
                Service.get_by_symbol("helloworld.Greeter.SayHello")
     end
 
     test "describing a type returns the type" do
-      assert {:ok, %{file_descriptor_proto: proto}} =
+      assert {:ok, descriptor} =
                Service.get_by_symbol("helloworld.HelloRequest")
 
-      assert {:ok, %{file_descriptor_proto: ^proto}} =
+      assert {:ok, ^descriptor} =
                Service.get_by_filename("helloworld.HelloRequest.proto")
     end
 
     test "type with leading period still resolves" do
-      assert {:ok, %{file_descriptor_proto: proto}} =
+      assert {:ok, descriptor} =
                Service.get_by_symbol(".helloworld.HelloRequest")
 
-      assert {:ok, %{file_descriptor_proto: ^proto}} =
+      assert {:ok, ^descriptor} =
                Service.get_by_filename("helloworld.HelloRequest.proto")
     end
   end