fix: remove quadratic decoding of repeated elements. (#287)

Author: zrzka

lib/protox/define_decoder.ex

@@ -63,17 +63,38 @@ defmodule Protox.DefineDecoder do
       make_parse_key_value_body(fields, vars, opts)
 
     unknown_fields_name = Keyword.fetch!(opts, :unknown_fields_name)
+    finish_decode = make_finish_decode(fields, vars.msg, unknown_fields_name)
 
     quote do
       @spec parse_key_value(binary(), struct()) :: struct()
       defp parse_key_value(<<>>, msg) do
-        %{msg | unquote(unknown_fields_name) => Enum.reverse(msg.unquote(unknown_fields_name))}
+        unquote(finish_decode)
       end
 
       defp parse_key_value(bytes, msg), do: unquote(parse_key_value_body)
     end
   end
 
+  defp make_finish_decode(fields, msg_var, unknown_fields_name) do
+    repeated_field_names =
+      fields
+      |> Enum.filter(&(&1.kind in [:packed, :unpacked]))
+      |> Enum.map(& &1.name)
+      |> Enum.uniq()
+
+    updates =
+      Enum.map(repeated_field_names, fn field_name ->
+        {field_name, quote(do: Enum.reverse(unquote(msg_var).unquote(field_name)))}
+      end) ++
+        [
+          {unknown_fields_name, quote(do: Enum.reverse(unquote(msg_var).unquote(unknown_fields_name)))}
+        ]
+
+    quote do
+      %{unquote(msg_var) | unquote_splicing(updates)}
+    end
+  end
+
   defp make_parse_key_value_body(fields, vars, opts) do
     # Fragment to parse unknown fields. Those are identified with an unknown tag.
     unknown_tag_clause =
@@ -312,6 +333,19 @@ defmodule Protox.DefineDecoder do
     quote(do: {unquote(field.name), unquote(value)})
   end
 
+  defp make_update_field(value, %Field{kind: kind} = field, vars, wrap_value) when kind in [:packed, :unpacked] do
+    update_value =
+      if wrap_value do
+        quote(do: [unquote(value) | unquote(vars.msg).unquote(field.name)])
+      else
+        quote(do: Enum.reverse(unquote(value), unquote(vars.msg).unquote(field.name)))
+      end
+
+    quote do
+      {unquote(field.name), unquote(update_value)}
+    end
+  end
+
   defp make_update_field(value, %Field{} = field, vars, wrap_value) do
     value = maybe_wrap(value, wrap_value)
 

test/protox/decode_test.exs

@@ -48,6 +48,12 @@ defmodule Protox.DecodeTest do
       <<218, 2, 2, 1, 0>> <> <<218, 2, 1, 1, 218, 2, 1, 0>>,
       %TestAllTypesProto3{repeated_bool: [true, false, true, false]}
     },
+    {
+      "Repeated bool, unpacked and packed, should be concatenated (2)",
+      # unpacked <> packed
+      <<218, 2, 1, 1, 218, 2, 1, 0>> <> <<218, 2, 2, 1, 0>>,
+      %TestAllTypesProto3{repeated_bool: [true, false, true, false]}
+    },
     {
       "Repeated uint32",
       <<138, 2, 6, 0, 1, 2, 3, 144, 78>>,

test/protox/generate_test.exs

@@ -142,4 +142,29 @@ defmodule Protox.GenerateTest do
     assert Code.compile_file(namespace_directory_message1_tmp_file) != []
     assert Code.compile_file(namespace_sub_directory_message_tmp_file) != []
   end
+
+  test "Generate repeated-field decoders with prepend-and-reverse" do
+    file = Path.join(__DIR__, "../samples/google/test_messages_proto3.proto")
+    generated_file_name = "generated_repeated_decode.ex"
+
+    {:ok, files_content} =
+      Protox.Generate.generate_module_code([file], generated_file_name, false, [
+        "./test/samples/google"
+      ])
+
+    assert [%Protox.Generate.FileContent{name: ^generated_file_name, content: content}] =
+             files_content
+
+    content = IO.iodata_to_binary(content)
+
+    refute content =~ "msg.repeated_bool ++"
+    refute content =~ "msg.repeated_string ++"
+    refute content =~ "msg.repeated_nested_message ++"
+
+    assert content =~
+             ~r/repeated_string:\s*\[\s*Protox\.Decode\.validate_string!\(delimited\)\s*\|\s*msg\.repeated_string\s*\]/s
+
+    assert content =~ "repeated_string: Enum.reverse(msg.repeated_string)"
+    assert content =~ "repeated_bool: Enum.reverse(msg.repeated_bool)"
+  end
 end