Feature/cleanup and recursion

lib/grpc_reflection/service/builder/util.ex

@@ -163,6 +163,9 @@ defmodule GrpcReflection.Service.Builder.Util do
     []
   end
 
+  # in gRPC, the leading "." signifies it is a FQDN
+  # we trim it and assume everything is a FQDN
+  # it works so far, but there may be corner cases
   def trim_symbol("." <> symbol), do: symbol
   def trim_symbol(symbol), do: symbol
 end

lib/grpc_reflection/service/state.ex

@@ -117,8 +117,72 @@ defmodule GrpcReflection.Service.State do
   end
 
   def group_symbols_by_namespace(%__MODULE__{} = state) do
-    # group symbols by namespace and combine
-    # IO.inspect(state)
-    state
+    state.symbols
+    |> Map.keys()
+    |> Enum.group_by(&GrpcReflection.Service.Builder.Util.get_package(&1))
+    |> Enum.reduce(state, fn {package, symbols}, state_acc ->
+      # each symbol in symbols is in the same package
+      # we will combine their files into a single file, and update them to
+      # reference this new file
+
+      # Step 1: Collect descriptors to be combined
+      symbol_files = Enum.map(symbols, &state_acc.symbols[&1])
+      files_to_combine = state_acc.files |> Map.take(symbol_files) |> Map.values()
+
+      # Step 2: Combine the descriptors
+      combined_file =
+        Enum.reduce(
+          files_to_combine,
+          %Google.Protobuf.FileDescriptorProto{
+            package: package,
+            name: package <> ".proto"
+          },
+          fn descriptor, acc ->
+            %{
+              acc
+              | syntax: descriptor.syntax,
+                message_type: Enum.uniq(acc.message_type ++ descriptor.message_type),
+                service: Enum.uniq(acc.service ++ descriptor.service),
+                enum_type: Enum.uniq(acc.enum_type ++ descriptor.enum_type),
+                dependency: Enum.uniq(acc.dependency ++ descriptor.dependency),
+                extension: Enum.uniq(acc.extension ++ descriptor.extension)
+            }
+          end
+        )
+
+      # Step 3: remove internal dependency refs
+      cleaned_file =
+        %{combined_file | dependency: combined_file.dependency -- symbol_files}
+
+      # Step 4: rework state around combined descriptor
+      # removing and re-adding symbols pointing to combined file
+      # removing combined file descriptors
+      # editing existing descriptors for relevant dependency entries
+      # add combined file descriptor
+      %{
+        state_acc
+        | symbols:
+            state_acc.symbols
+            |> Map.drop(symbols)
+            |> Map.merge(Map.new(symbols, &{&1, cleaned_file.name})),
+          files:
+            state_acc.files
+            |> Map.drop(symbol_files)
+            |> Map.new(fn {filename, descriptor} ->
+              if Enum.any?(descriptor.dependency, &Enum.member?(symbol_files, &1)) do
+                {
+                  filename,
+                  %{
+                    descriptor
+                    | dependency: (descriptor.dependency -- symbol_files) ++ [cleaned_file.name]
+                  }
+                }
+              else
+                {filename, descriptor}
+              end
+            end)
+            |> Map.put(cleaned_file.name, cleaned_file)
+      }
+    end)
   end
 end

test/integration/v1_reflection_test.exs

@@ -74,7 +74,7 @@ defmodule GrpcReflection.V1ReflectionTest do
     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"
+      assert response.name == "testserviceV3.proto"
     end
 
     test "type with leading period still resolves", ctx do
@@ -90,11 +90,8 @@ defmodule GrpcReflection.V1ReflectionTest do
       assert {:ok, response} = run_request(message, ctx)
       assert_response(response)
 
-      # we pretend all modules are in different files, dependencies are listed
-      assert response.dependency == [
-               "helloworld.HelloRequest.proto",
-               "helloworld.HelloReply.proto"
-             ]
+      # we pretend each namespace is in a single file, dependencies are listed
+      assert response.dependency == ["google.protobuf.proto"]
     end
 
     test "reject filename that doesn't match a reflection module", ctx do
@@ -104,93 +101,52 @@ defmodule GrpcReflection.V1ReflectionTest do
     end
 
     test "get replytype by filename", ctx do
-      filename = "helloworld.HelloReply.proto"
+      filename = "helloworld.proto"
       message = {:file_by_filename, filename}
       assert {:ok, response} = run_request(message, ctx)
       assert response.name == filename
       assert response.package == "helloworld"
-      assert response.dependency == ["google.protobuf.Timestamp.proto"]
+      assert response.dependency == ["google.protobuf.proto"]
 
       assert [
-               %Google.Protobuf.DescriptorProto{
-                 name: "HelloReply",
-                 field: [
-                   %Google.Protobuf.FieldDescriptorProto{
-                     name: "message",
-                     number: 1,
-                     label: :LABEL_OPTIONAL,
-                     type: :TYPE_STRING,
-                     json_name: "message"
-                   },
-                   %Google.Protobuf.FieldDescriptorProto{
-                     name: "today",
-                     number: 2,
-                     label: :LABEL_OPTIONAL,
-                     type: :TYPE_MESSAGE,
-                     type_name: ".google.protobuf.Timestamp",
-                     json_name: "today"
-                   }
-                 ]
-               }
+               %Google.Protobuf.DescriptorProto{name: "HelloReply"},
+               %Google.Protobuf.DescriptorProto{name: "HelloRequest"}
              ] = response.message_type
     end
 
     test "get external by filename", ctx do
-      filename = "google.protobuf.Timestamp.proto"
+      filename = "google.protobuf.proto"
       message = {:file_by_filename, filename}
       assert {:ok, response} = run_request(message, ctx)
       assert response.name == filename
       assert response.package == "google.protobuf"
       assert response.dependency == []
 
       assert [
-               %Google.Protobuf.DescriptorProto{
-                 field: [
-                   %Google.Protobuf.FieldDescriptorProto{
-                     json_name: "seconds",
-                     label: :LABEL_OPTIONAL,
-                     name: "seconds",
-                     number: 1,
-                     type: :TYPE_INT64
-                   },
-                   %Google.Protobuf.FieldDescriptorProto{
-                     json_name: "nanos",
-                     label: :LABEL_OPTIONAL,
-                     name: "nanos",
-                     number: 2,
-                     type: :TYPE_INT32
-                   }
-                 ],
-                 name: "Timestamp"
-               }
+               %Google.Protobuf.DescriptorProto{name: "Any"},
+               %Google.Protobuf.DescriptorProto{name: "StringValue"},
+               %Google.Protobuf.DescriptorProto{name: "Timestamp"}
              ] = response.message_type
     end
 
     test "ensures file descriptor dependencies are unique", ctx do
-      filename = "testserviceV3.TestReply.proto"
+      filename = "testserviceV3.proto"
       message = {:file_by_filename, filename}
       assert {:ok, response} = run_request(message, ctx)
       assert response.name == filename
       assert response.package == "testserviceV3"
 
-      assert response.dependency == [
-               "google.protobuf.Timestamp.proto",
-               "google.protobuf.StringValue.proto"
-             ]
+      assert response.dependency == ["google.protobuf.proto"]
     end
 
     test "ensure exclusion of nested types in file descriptor dependencies", ctx do
-      filename = "testserviceV3.TestRequest.proto"
+      filename = "testserviceV3.proto"
       message = {:file_by_filename, filename}
       assert {:ok, response} = run_request(message, ctx)
       assert response.name == filename
       assert response.package == "testserviceV3"
 
-      assert response.dependency == [
-               "testserviceV3.Enum.proto",
-               "google.protobuf.Any.proto",
-               "google.protobuf.StringValue.proto"
-             ]
+      assert response.dependency == ["google.protobuf.proto"]
     end
   end
 
@@ -213,49 +169,18 @@ defmodule GrpcReflection.V1ReflectionTest do
       assert {:ok, response} = run_request(message, ctx)
       assert response.name == extendee <> "Extension.proto"
       assert response.package == "testserviceV2"
-      assert response.dependency == [extendee <> ".proto"]
-
-      assert response.extension == [
-               %Google.Protobuf.FieldDescriptorProto{
-                 name: "data",
-                 extendee: extendee,
-                 number: 10,
-                 label: :LABEL_OPTIONAL,
-                 type: :TYPE_STRING,
-                 type_name: nil
-               },
-               %Google.Protobuf.FieldDescriptorProto{
-                 name: "location",
-                 extendee: extendee,
-                 number: 11,
-                 label: :LABEL_OPTIONAL,
-                 type: :TYPE_MESSAGE,
-                 type_name: "testserviceV2.Location"
-               }
-             ]
+      assert response.dependency == ["testserviceV2.proto"]
+
+      assert [
+               %Google.Protobuf.FieldDescriptorProto{name: "data"},
+               %Google.Protobuf.FieldDescriptorProto{name: "location"}
+             ] = response.extension
 
-      assert response.message_type == [
+      assert [
                %Google.Protobuf.DescriptorProto{
-                 name: "Location",
-                 field: [
-                   %Google.Protobuf.FieldDescriptorProto{
-                     name: "latitude",
-                     number: 1,
-                     label: :LABEL_OPTIONAL,
-                     type: :TYPE_DOUBLE,
-                     json_name: "latitude"
-                   },
-                   %Google.Protobuf.FieldDescriptorProto{
-                     name: "longitude",
-                     extendee: nil,
-                     number: 2,
-                     label: :LABEL_OPTIONAL,
-                     type: :TYPE_DOUBLE,
-                     json_name: "longitude"
-                   }
-                 ]
+                 name: "Location"
                }
-             ]
+             ] = response.message_type
     end
   end
 end