From 9644bd9944d04f2d714a3c0f12db8ec0cf522799 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Tue, 14 Jan 2025 18:09:44 -0300 Subject: [PATCH 1/8] Add Protobuf.Text * Allows encoding to textproto * Run conformance for text, fixes for conformance, add unknown fields support Most of the conformance tests in the text suite are for decoding, and some of them are for unknown fields encoding rules, so we only got value out of five of them. Still, they led to significant improvements in the implementation. For one, I figured that the "outer message" **can't** use brackets ("{", "}"). I thought they were optional and just not used in the examples. So we went from: ```elixir Protobuf.Text.encode(%MyMessage{a: 1, b: 2}) ``` To: ```elixir Protobuf.Text.encode(%MyMessage{a: 1, b: 2}) ``` I also added unknown fields support, but wasn't able to pass all the unknown fields tests, so I skipped them for now. The current version does the job of printing them reasonably well, so I'm not too bothered with it. --- conformance/exemptions.txt | 7 ++ conformance/protobuf/runner.ex | 12 +- conformance/text-exemptions.txt | 4 + lib/protobuf/text.ex | 207 ++++++++++++++++++++++++++++++++ mix.exs | 2 + test/protobuf/text_test.exs | 150 +++++++++++++++++++++++ 6 files changed, 380 insertions(+), 2 deletions(-) create mode 100644 conformance/text-exemptions.txt create mode 100644 lib/protobuf/text.ex create mode 100644 test/protobuf/text_test.exs diff --git a/conformance/exemptions.txt b/conformance/exemptions.txt index adc2e572..be224bf6 100644 --- a/conformance/exemptions.txt +++ b/conformance/exemptions.txt @@ -1,4 +1,6 @@ Recommended.Proto2.JsonInput.FieldNameExtension.Validator + +# We raise if find unknown enum values Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput @@ -9,3 +11,8 @@ Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.Protobu Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput + +# Any +Required.Proto3.JsonInput.AnyWithNoType.JsonOutput +Required.Proto3.JsonInput.AnyWktRepresentationWithBadType +Required.Proto3.JsonInput.AnyWktRepresentationWithEmptyTypeAndValue diff --git a/conformance/protobuf/runner.ex b/conformance/protobuf/runner.ex index e9d2edd5..c63c2cb7 100644 --- a/conformance/protobuf/runner.ex +++ b/conformance/protobuf/runner.ex @@ -77,10 +77,11 @@ defmodule Conformance.Protobuf.Runner do defp handle_conformance_request(%mod{ requested_output_format: requested_output_format, message_type: message_type, - payload: {payload_kind, msg} + payload: {payload_kind, msg}, + print_unknown_fields: print_unknown_fields }) when mod == Conformance.ConformanceRequest and - requested_output_format in [:PROTOBUF, :JSON] and + requested_output_format in [:PROTOBUF, :JSON, :TEXT_FORMAT] and payload_kind in [:protobuf_payload, :json_payload] do test_proto_type = to_test_proto_type(message_type) @@ -94,6 +95,7 @@ defmodule Conformance.Protobuf.Runner do case requested_output_format do :PROTOBUF -> {&safe_encode/1, :protobuf_payload} :JSON -> {&Protobuf.JSON.encode/1, :json_payload} + :TEXT_FORMAT -> {&safe_text_encode(&1, print_unknown_fields), :text_payload} end with {:decode, {:ok, decoded_msg}} <- {:decode, decode_fun.(msg, test_proto_type)}, @@ -138,4 +140,10 @@ defmodule Conformance.Protobuf.Runner do rescue exception -> {:error, exception, __STACKTRACE__} end + + defp safe_text_encode(struct, print_unknown_fields?) do + {:ok, Protobuf.Text.encode(struct, print_unknown_fields?: print_unknown_fields?)} + rescue + exception -> {:error, exception, __STACKTRACE__} + end end diff --git a/conformance/text-exemptions.txt b/conformance/text-exemptions.txt new file mode 100644 index 00000000..768431ca --- /dev/null +++ b/conformance/text-exemptions.txt @@ -0,0 +1,4 @@ +# We do a best effort for printing unknown values but don't try to expand them +Recommended.Proto3.ProtobufInput.GroupUnknownFields_Print.TextFormatOutput +Recommended.Proto3.ProtobufInput.MessageUnknownFields_Print.TextFormatOutput +Recommended.Proto3.ProtobufInput.RepeatedUnknownFields_Print.TextFormatOutput diff --git a/lib/protobuf/text.ex b/lib/protobuf/text.ex new file mode 100644 index 00000000..c0bd6bfe --- /dev/null +++ b/lib/protobuf/text.ex @@ -0,0 +1,207 @@ +defmodule Protobuf.Text do + @moduledoc """ + Text encoding of Protobufs + + Compliant with the + [textproto spec](https://protobuf.dev/reference/protobuf/textformat-spec/), + but without extensions or `Google.Protobuf.Any` support (yet). + + Useful for inspecting Protobuf data. + """ + + alias Protobuf.FieldProps + alias Protobuf.MessageProps + alias Inspect.Algebra + + @doc """ + Encodes a Protobuf struct to text. + + Accepts the following options: + + - `:max_line_width` - maximum line width, in columns. Defaults to 80. Lines + may still be bigger than the limit, depending on the data. + - `:print_unknown_fields?` - if `true`, prints unknown fields. Notice this makes + the output impossible to decode, per current Protobuf spec. Defaults to `false`. + + Doesn't perform type validations. If input data is invalid, it produces + undecodable output. + """ + @spec encode(struct(), Keyword.t()) :: binary() + def encode(%mod{} = struct, opts \\ []) do + max_line_width = Keyword.get(opts, :max_line_width, 80) + message_props = mod.__message_props__() + print_unknown? = Keyword.get(opts, :print_unknown_fields?, false) + + struct + |> transform_module(mod) + |> encode_struct(message_props, true, print_unknown?) + |> Algebra.format(max_line_width) + |> IO.iodata_to_binary() + end + + @spec encode_struct(struct(), MessageProps.t(), boolean(), boolean()) :: + Algebra.t() + defp encode_struct(%_{} = struct, message_props, root?, print_unknown?) do + %{syntax: syntax} = message_props + + fields = prepare_fields(struct, print_unknown?) + + if root? do + empty = Algebra.empty() + + fields + |> Enum.reduce([], fn value, acc -> + case encode_struct_field(value, syntax, message_props, print_unknown?) do + ^empty -> acc + doc -> [Algebra.break(", "), doc | acc] + end + end) + |> safe_tail() + |> Enum.reverse() + |> Algebra.concat() + |> Algebra.group() + else + fun = fn value, _opts -> + encode_struct_field(value, syntax, message_props, print_unknown?) + end + + Algebra.container_doc("{", fields, "}", inspect_opts(), fun, break: :strict) + end + end + + defp prepare_fields(struct, print_unknown?) do + unknown_fields = + if print_unknown? do + Enum.map(struct.__unknown_fields__, fn {field_number, _wire_type, value} -> + {field_number, value} + end) + else + [] + end + + struct + |> Map.drop([:__unknown_fields__, :__struct__, :__pb_extensions__]) + |> Enum.sort() + |> Kernel.++(unknown_fields) + end + + defp safe_tail([]), do: [] + defp safe_tail([_head | tail]), do: tail + + @spec encode_struct_field( + {atom() | integer(), term()}, + :proto2 | :proto3, + MessageProps.t(), + boolean() + ) :: Algebra.t() + defp encode_struct_field({name, value}, syntax, message_props, print_unknown?) + when is_atom(name) do + case Enum.find(message_props.field_props, fn {_, prop} -> prop.name_atom == name end) do + {_fnum, field_prop} -> + if skip_field?(syntax, value, field_prop) do + Algebra.empty() + else + Algebra.concat([ + to_string(name), + ": ", + encode_value(value, syntax, field_prop, print_unknown?) + ]) + end + + nil -> + if Enum.any?(message_props.oneof, fn {oneof_name, _} -> name == oneof_name end) do + case value do + {field_name, field_value} -> + encode_struct_field( + {field_name, field_value}, + syntax, + message_props, + print_unknown? + ) + + nil -> + Algebra.empty() + + _ -> + raise Protobuf.EncodeError, message: "invalid value for oneof `#{inspect(name)}`: #{inspect(value)}" + end + else + raise Protobuf.EncodeError, message: "unknown field #{inspect(name)}" + end + end + end + + # unknown fields + defp encode_struct_field({number, value}, _syntax, _, _) when is_integer(number) do + Algebra.concat([to_string(number), ": ", inspect(value)]) + end + + @spec encode_value(term(), :proto2 | :proto3, FieldProps.t(), boolean()) :: Algebra.t() + defp encode_value(value, syntax, %{repeated?: true} = field_prop, print_unknown?) + when is_list(value) do + encode_list(value, syntax, field_prop, print_unknown?) + end + + defp encode_value(value, syntax, %{map?: true} = field_prop, print_unknown?) do + as_list = + Enum.map(value, fn {k, v} -> struct(field_prop.type, key: k, value: v) end) + + encode_list(as_list, syntax, field_prop, print_unknown?) + end + + defp encode_value(value, _syntax, %{embedded?: true, type: mod}, print_unknown?) do + value + |> transform_module(mod) + |> encode_struct(mod.__message_props__(), false, print_unknown?) + end + + defp encode_value(nil, :proto2, %FieldProps{required?: true, name_atom: name}, _) do + raise Protobuf.EncodeError, message: "field #{inspect(name)} is required" + end + + defp encode_value(value, _, _, _) when is_atom(value) do + to_string(value) + end + + defp encode_value(value, _, _, _) do + inspect(value) + end + + defp encode_list(list, syntax, field_prop, print_unknown?) do + fun = fn val, _opts -> encode_value(val, syntax, %{field_prop | repeated?: false, map?: false}, print_unknown?) end + Algebra.container_doc("[", list, "]", inspect_opts(), fun, break: :strict) + end + + defp transform_module(message, module) do + if transform_module = module.transform_module() do + transform_module.encode(message, module) + else + message + end + end + + # Copied from Protobuf.Encoder. Should it be shared? + defp skip_field?(_syntax, [], _prop), do: true + defp skip_field?(_syntax, val, _prop) when is_map(val), do: map_size(val) == 0 + defp skip_field?(:proto2, nil, %FieldProps{optional?: optional?}), do: optional? + defp skip_field?(:proto2, value, %FieldProps{default: value, oneof: nil}), do: true + defp skip_field?(:proto3, val, %FieldProps{proto3_optional?: true}), do: is_nil(val) + + defp skip_field?(:proto3, nil, _prop), do: true + defp skip_field?(:proto3, 0, %FieldProps{oneof: nil}), do: true + defp skip_field?(:proto3, +0.0, %FieldProps{oneof: nil}), do: true + defp skip_field?(:proto3, "", %FieldProps{oneof: nil}), do: true + defp skip_field?(:proto3, false, %FieldProps{oneof: nil}), do: true + + # This is actually new. Should it be ported to Protobuf.Encoder? + defp skip_field?(:proto3, value, %FieldProps{type: {:enum, enum_mod}, oneof: nil}) do + enum_props = enum_mod.__message_props__() + %{name_atom: name_atom, name: name, json_name: name_json} = enum_props.field_props[0] + + value == name_atom or value == name or value == name_json + end + + defp skip_field?(_, _, _), do: false + + defp inspect_opts(), do: %Inspect.Opts{limit: :infinity} +end diff --git a/mix.exs b/mix.exs index c430670f..16b353e0 100644 --- a/mix.exs +++ b/mix.exs @@ -279,6 +279,8 @@ defmodule Protobuf.Mixfile do "--enforce_recommended", "--failure_list", "conformance/exemptions.txt", + "--text_format_failure_list", + "conformance/text-exemptions.txt", "./conformance/runner.sh" ] diff --git a/test/protobuf/text_test.exs b/test/protobuf/text_test.exs new file mode 100644 index 00000000..7b84852e --- /dev/null +++ b/test/protobuf/text_test.exs @@ -0,0 +1,150 @@ +defmodule Protobuf.TextTest do + use ExUnit.Case, async: true + + alias Protobuf.Text + + test "default fields aren't encoded" do + # proto3 + assert "" == Text.encode(%TestMsg.Foo{}) + + # proto2 + assert "a: 1" == Text.encode(%TestMsg.Foo2{a: 1}) + end + + test "encoding basic types" do + assert ~S(a: 1, b: "foo") == Text.encode(%TestMsg.Foo.Bar{a: 1, b: "foo"}) + end + + test "encoding enums" do + assert ~S(j: D) == Text.encode(%TestMsg.Foo{j: :D}) + end + + test "encoding repeated" do + assert ~S(g: [1, 2, 3, 4, 5, 6]) == Text.encode(%TestMsg.Foo{g: [1, 2, 3, 4, 5, 6]}) + end + + test "encoding large repeated breaks lines" do + result = Text.encode(%TestMsg.Foo{a: 1, g: List.duplicate(1_111_111_111, 7), j: :D}) + + assert result == """ + a: 1 + g: [ + 1111111111, + 1111111111, + 1111111111, + 1111111111, + 1111111111, + 1111111111, + 1111111111 + ] + j: D\ + """ + end + + test "encoding nested structs" do + result = Text.encode(%TestMsg.Foo{e: nil}) + assert result == ~S() + + result = Text.encode(%TestMsg.Foo{e: %TestMsg.Foo.Bar{}}) + assert result == ~S(e: {}) + + result = Text.encode(%TestMsg.Foo{e: %TestMsg.Foo.Bar{a: 1, b: "Hello"}}) + + assert result == ~S(e: {a: 1, b: "Hello"}) + end + + test "encoding large nested structs" do + result = + Text.encode(%TestMsg.Foo{ + a: 1, + e: %TestMsg.Foo.Bar{a: 1, b: String.duplicate("Hello", 15)}, + h: [ + %TestMsg.Foo.Bar{a: 5}, + %TestMsg.Foo.Bar{a: 1, b: String.duplicate("Hello", 15)}, + %TestMsg.Foo.Bar{a: 7} + ] + }) + + assert result == """ + a: 1 + e: { + a: 1, + b: "HelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHello" + } + h: [ + {a: 5}, + { + a: 1, + b: "HelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHello" + }, + {a: 7} + ]\ + """ + end + + test "respects max line width option" do + input = %TestMsg.Foo{ + a: 1, + e: %TestMsg.Foo.Bar{a: 1, b: String.duplicate("Hello", 15)}, + h: [ + %TestMsg.Foo.Bar{a: 5, b: "hi"}, + %TestMsg.Foo.Bar{a: 1, b: String.duplicate("Hello", 15)}, + %TestMsg.Foo.Bar{a: 7} + ] + } + + result_with_large_limit = Text.encode(input, max_line_width: 1000) + + assert result_with_large_limit == """ + a: 1, \ + e: {\ + a: 1, \ + b: "HelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHello"\ + }, \ + h: [\ + {a: 5, b: "hi"}, \ + {\ + a: 1, \ + b: "HelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHello"\ + }, \ + {a: 7}\ + ]\ + """ + + result_with_small_limit = Text.encode(input, max_line_width: 10) + + assert result_with_small_limit == """ + a: 1 + e: { + a: 1, + b: "HelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHello" + } + h: [ + { + a: 5, + b: "hi" + }, + { + a: 1, + b: "HelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHelloHello" + }, + {a: 7} + ]\ + """ + end + + test "encoding oneofs" do + assert "a: 50" == Text.encode(%TestMsg.Oneof{first: {:a, 50}}) + end + + test "encoding maps" do + assert ~S(l: [{key: "a", value: 1}, {key: "b", value: 2}]) == + Text.encode(%TestMsg.Foo{l: %{"a" => 1, "b" => 2}}) + end + + test "raises on absent proto2 required" do + assert_raise RuntimeError, "field :a is required", fn -> + Text.encode(%TestMsg.Foo2{}) + end + end +end From 50642e93a0a89ba51775221a110dd5fce3681dd8 Mon Sep 17 00:00:00 2001 From: felipe stival <14948182+v0idpwn@users.noreply.github.com> Date: Tue, 15 Apr 2025 18:25:11 -0300 Subject: [PATCH 2/8] Create Protobuf.Presence module (#403) * Create Protobuf.Presence module Unifies presence tracking between encoders. Provides a public API for checking field presence * Add test for messages, fix proto3 * Improve docs --- lib/protobuf/encoder.ex | 40 ++----- lib/protobuf/json/encode.ex | 25 +++-- lib/protobuf/presence.ex | 179 ++++++++++++++++++++++++++++++++ lib/protobuf/text.ex | 28 ++--- test/protobuf/presence_test.exs | 146 ++++++++++++++++++++++++++ test/protobuf/text_test.exs | 2 +- test/support/test_msg.ex | 4 +- 7 files changed, 357 insertions(+), 67 deletions(-) create mode 100644 lib/protobuf/presence.ex create mode 100644 test/protobuf/presence_test.exs diff --git a/lib/protobuf/encoder.ex b/lib/protobuf/encoder.ex index 4c426e94..18bdc7ee 100644 --- a/lib/protobuf/encoder.ex +++ b/lib/protobuf/encoder.ex @@ -83,20 +83,14 @@ defmodule Protobuf.Encoder do "Got error when encoding #{inspect(struct_mod)}##{prop.name_atom}: #{Exception.format(:error, error)}" end - defp skip_field?(_syntax, [], _prop), do: true - defp skip_field?(_syntax, val, _prop) when is_map(val), do: map_size(val) == 0 - defp skip_field?(:proto2, nil, %FieldProps{optional?: optional?}), do: optional? - defp skip_field?(:proto2, value, %FieldProps{default: value, oneof: nil}), do: true - - defp skip_field?(:proto3, val, %FieldProps{proto3_optional?: true}), - do: is_nil(val) - - defp skip_field?(:proto3, nil, _prop), do: true - defp skip_field?(:proto3, 0, %FieldProps{oneof: nil}), do: true - defp skip_field?(:proto3, +0.0, %FieldProps{oneof: nil}), do: true - defp skip_field?(:proto3, "", %FieldProps{oneof: nil}), do: true - defp skip_field?(:proto3, false, %FieldProps{oneof: nil}), do: true - defp skip_field?(_syntax, _val, _prop), do: false + defp skip_field?(syntax, value, field_prop) do + case Protobuf.Presence.get_field_presence(syntax, value, field_prop) do + :present -> false + # Proto2 required isn't skipped even if not present + :maybe -> not(syntax == :proto2 && field_prop.required?) + :not_present -> not(syntax == :proto2 && field_prop.required?) + end + end defp do_encode_field( :normal, @@ -104,7 +98,7 @@ defmodule Protobuf.Encoder do syntax, %FieldProps{encoded_fnum: fnum, type: type, repeated?: repeated?} = prop ) do - if skip_field?(syntax, val, prop) or skip_enum?(syntax, val, prop) do + if skip_field?(syntax, val, prop) do :skip else iodata = apply_or_map(val, repeated?, &[fnum | Wire.encode(type, &1)]) @@ -142,7 +136,7 @@ defmodule Protobuf.Encoder do end defp do_encode_field(:packed, val, syntax, %FieldProps{type: type, encoded_fnum: fnum} = prop) do - if skip_field?(syntax, val, prop) or skip_enum?(syntax, val, prop) do + if skip_field?(syntax, val, prop) do :skip else encoded = Enum.map(val, &Wire.encode(type, &1)) @@ -197,20 +191,6 @@ defmodule Protobuf.Encoder do defp apply_or_map(val, _repeated? = true, func), do: Enum.map(val, func) defp apply_or_map(val, _repeated? = false, func), do: func.(val) - defp skip_enum?(:proto2, _value, _prop), do: false - defp skip_enum?(:proto3, _value, %FieldProps{proto3_optional?: true}), do: false - defp skip_enum?(_syntax, _value, %FieldProps{enum?: false}), do: false - - defp skip_enum?(_syntax, _value, %FieldProps{enum?: true, oneof: oneof}) when not is_nil(oneof), - do: false - - defp skip_enum?(_syntax, _value, %FieldProps{required?: true}), do: false - defp skip_enum?(_syntax, value, %FieldProps{type: type}), do: enum_default?(type, value) - - defp enum_default?({:enum, enum_mod}, val) when is_atom(val), do: enum_mod.value(val) == 0 - defp enum_default?({:enum, _enum_mod}, val) when is_integer(val), do: val == 0 - defp enum_default?({:enum, _enum_mod}, list) when is_list(list), do: false - # Returns a map of %{field_name => field_value} from oneofs. For example, if you have: # oneof body { # string a = 1; diff --git a/lib/protobuf/json/encode.ex b/lib/protobuf/json/encode.ex index 13e97e0c..ad031c9f 100644 --- a/lib/protobuf/json/encode.ex +++ b/lib/protobuf/json/encode.ex @@ -162,7 +162,7 @@ defmodule Protobuf.JSON.Encode do defp encode_regular_fields(struct, %{field_props: field_props, syntax: syntax}, opts) do for {_field_num, %{name_atom: name, oneof: nil} = prop} <- field_props, %{^name => value} = struct, - emit?(syntax, prop, value) || opts[:emit_unpopulated] do + emit?(syntax, prop, value, opts[:emit_unpopulated]) do encode_field(prop, value, opts) end end @@ -301,18 +301,17 @@ defmodule Protobuf.JSON.Encode do defp maybe_repeat(%{repeated?: false}, val, fun), do: fun.(val) defp maybe_repeat(%{repeated?: true}, val, fun), do: Enum.map(val, fun) - defp emit?(:proto2, %{default: value}, value), do: false - defp emit?(:proto2, %{optional?: true}, val), do: not is_nil(val) - defp emit?(:proto3, %{proto3_optional?: true}, val), do: not is_nil(val) - defp emit?(_syntax, _prop, +0.0), do: false - defp emit?(_syntax, _prop, nil), do: false - defp emit?(_syntax, _prop, 0), do: false - defp emit?(_syntax, _prop, false), do: false - defp emit?(_syntax, _prop, []), do: false - defp emit?(_syntax, _prop, ""), do: false - defp emit?(_syntax, _prop, %{} = map) when map_size(map) == 0, do: false - defp emit?(_syntax, %{type: {:enum, enum}}, key) when is_atom(key), do: enum.value(key) != 0 - defp emit?(_syntax, _prop, _value), do: true + defp emit?(_syntax, _field_prop, _value, true = _emit_unpopulated?) do + true + end + + defp emit?(syntax, field_prop, value, _emit_unpopulated?) do + case Protobuf.Presence.get_field_presence(syntax, value, field_prop) do + :present -> true + :maybe -> false + :not_present -> false + end + end defp transform_module(message, module) do if transform_module = module.transform_module() do diff --git a/lib/protobuf/presence.ex b/lib/protobuf/presence.ex new file mode 100644 index 00000000..409845bf --- /dev/null +++ b/lib/protobuf/presence.ex @@ -0,0 +1,179 @@ +defmodule Protobuf.Presence do + @moduledoc """ + Helpers for determining Protobuf field presence. + """ + + alias Protobuf.FieldProps + + @doc """ + Returns whether a field or oneof is present, not present, or maybe present + + `:present` and `:not present` mean that a field is **explicitly** present or not, + respectively. + + Some values may be implicitly present. For example, lists in `repeated` fields + always have implicit presence. In these cases, if the presence is ambiguous, + returns `:maybe`. + + For more information about field presence tracking rules, refer to the official + [Field Presence docs](https://protobuf.dev/programming-guides/field_presence/). + + + ## Examples + + # Non-optional proto3 field: + Protobuf.Presence(%MyMessage{foo: 42}, :foo) + #=> :present + + Protobuf.Presence(%MyMessage{foo: 0}, :foo) + #=> :maybe + + Protobuf.Presence(%MyMessage{}, :foo) + #=> :maybe + + # Optional proto3 field: + Protobuf.Presence(%MyMessage{bar: 42}, :bar) + #=> :present + + Protobuf.Presence(%MyMessage{bar: 0}, :bar) + #=> :present + + Protobuf.Presence(%MyMessage{}, :bar) + #=> :not_present + + """ + @spec field_presence(message :: struct(), field :: atom()) :: :present | :not_present | :maybe + def field_presence(%mod{} = message, field) do + message_props = mod.__message_props__() + transformed_message = transform_module(message, mod) + fnum = Map.fetch!(message_props.field_tags, field) + field_prop = Map.fetch!(message_props.field_props, fnum) + value = get_oneof_value(transformed_message, message_props, field, field_prop) + + transformed_value = + case field_prop do + %{embedded: true, type: mod} -> transform_module(value, mod) + _ -> value + end + + get_field_presence(message_props.syntax, transformed_value, field_prop) + end + + defp get_oneof_value(message, message_props, field, field_prop) do + case field_prop.oneof do + nil -> + Map.fetch!(message, field) + + oneof_num -> + {oneof_field, _} = Enum.find(message_props.oneof, fn {_name, tag} -> tag == oneof_num end) + + case Map.fetch!(message, oneof_field) do + {^field, value} -> value + _ -> nil + end + end + end + + defp transform_module(message, module) do + if transform_module = module.transform_module() do + transform_module.encode(message, module) + else + message + end + end + + # We probably want to make this public eventually, but it makes sense to hold + # it until we add editions support, since we definitely don't want to add + # `syntax` in a public API + @doc false + @spec get_field_presence(:proto2 | :proto3, term(), FieldProps.t()) :: :present | :not_present | :maybe + def get_field_presence(syntax, value, field_prop) + + # Repeated and maps are always implicit. + def get_field_presence(_syntax, [], _prop) do + :maybe + end + + def get_field_presence(_syntax, val, _prop) when is_map(val) do + if map_size(val) == 0 do + :maybe + else + :present + end + end + + # For proto2 singular cardinality fields: + # + # - Non-one_of fields with default values have implicit presence + # - Others have explicit presence + def get_field_presence(:proto2, nil, _prop) do + :not_present + end + + def get_field_presence(:proto2, value, %FieldProps{default: value, oneof: nil}) do + :maybe + end + + def get_field_presence(:proto2, _value, _props) do + :present + end + + # For proto3 singular cardinality fields: + # + # - Optional and Oneof fields have explicit presence tracking + # - Other fields have implicit presence tracking + def get_field_presence(:proto3, nil, %FieldProps{proto3_optional?: true}) do + :not_present + end + + def get_field_presence(:proto3, _, %FieldProps{proto3_optional?: true}) do + :present + end + + def get_field_presence(_syntax, value, %FieldProps{oneof: oneof}) when not is_nil(oneof) do + if is_nil(value) do + :not_present + else + :present + end + end + + # Messages have explicit presence tracking in proto3 + def get_field_presence(:proto3, nil, _prop) do + :not_present + end + + # Defaults for different field types: implicit presence means they are maybe set + def get_field_presence(:proto3, 0, _prop) do + :maybe + end + + def get_field_presence(:proto3, +0.0, _prop) do + :maybe + end + + def get_field_presence(:proto3, "", _prop) do + :maybe + end + + def get_field_presence(:proto3, false, _prop) do + :maybe + end + + def get_field_presence(_syntax, value, %FieldProps{type: {:enum, enum_mod}}) do + if enum_default?(enum_mod, value) do + :maybe + else + :present + end + end + + # Finally, everything else. + def get_field_presence(_syntax, _val, _prop) do + :present + end + + defp enum_default?(enum_mod, val) when is_atom(val), do: enum_mod.value(val) == 0 + defp enum_default?(_enum_mod, val) when is_integer(val), do: val == 0 + defp enum_default?(_enum_mod, list) when is_list(list), do: false +end diff --git a/lib/protobuf/text.ex b/lib/protobuf/text.ex index c0bd6bfe..92675ac9 100644 --- a/lib/protobuf/text.ex +++ b/lib/protobuf/text.ex @@ -180,28 +180,14 @@ defmodule Protobuf.Text do end end - # Copied from Protobuf.Encoder. Should it be shared? - defp skip_field?(_syntax, [], _prop), do: true - defp skip_field?(_syntax, val, _prop) when is_map(val), do: map_size(val) == 0 - defp skip_field?(:proto2, nil, %FieldProps{optional?: optional?}), do: optional? - defp skip_field?(:proto2, value, %FieldProps{default: value, oneof: nil}), do: true - defp skip_field?(:proto3, val, %FieldProps{proto3_optional?: true}), do: is_nil(val) - - defp skip_field?(:proto3, nil, _prop), do: true - defp skip_field?(:proto3, 0, %FieldProps{oneof: nil}), do: true - defp skip_field?(:proto3, +0.0, %FieldProps{oneof: nil}), do: true - defp skip_field?(:proto3, "", %FieldProps{oneof: nil}), do: true - defp skip_field?(:proto3, false, %FieldProps{oneof: nil}), do: true - - # This is actually new. Should it be ported to Protobuf.Encoder? - defp skip_field?(:proto3, value, %FieldProps{type: {:enum, enum_mod}, oneof: nil}) do - enum_props = enum_mod.__message_props__() - %{name_atom: name_atom, name: name, json_name: name_json} = enum_props.field_props[0] - - value == name_atom or value == name or value == name_json + defp skip_field?(syntax, value, field_prop) do + case Protobuf.Presence.get_field_presence(syntax, value, field_prop) do + :present -> false + # Proto2 required isn't skipped even if not present + :maybe -> not(syntax == :proto2 && field_prop.required?) + :not_present -> not(syntax == :proto2 && field_prop.required?) + end end - defp skip_field?(_, _, _), do: false - defp inspect_opts(), do: %Inspect.Opts{limit: :infinity} end diff --git a/test/protobuf/presence_test.exs b/test/protobuf/presence_test.exs new file mode 100644 index 00000000..ef43dd51 --- /dev/null +++ b/test/protobuf/presence_test.exs @@ -0,0 +1,146 @@ +defmodule Protobuf.PresenceTest do + use ExUnit.Case, async: true + + alias Protobuf.Presence + alias TestMsg.{Foo, Foo2, Proto3Optional, Oneof, OneofProto3, ContainsTransformModule} + + describe "field_presence/2 for proto3" do + test "singular non-optional fields have implicit presence" do + msg = %Foo{} + assert Presence.field_presence(msg, :a) == :maybe + assert Presence.field_presence(msg, :c) == :maybe + assert Presence.field_presence(msg, :k) == :maybe + + msg = %Foo{a: 42, c: "hello", k: true, j: :A} + assert Presence.field_presence(msg, :a) == :present + assert Presence.field_presence(msg, :c) == :present + assert Presence.field_presence(msg, :k) == :present + assert Presence.field_presence(msg, :j) == :present + + msg = %Foo{a: 0, c: "", k: false, j: :UNKNOWN} + assert Presence.field_presence(msg, :a) == :maybe + assert Presence.field_presence(msg, :c) == :maybe + assert Presence.field_presence(msg, :k) == :maybe + assert Presence.field_presence(msg, :j) == :maybe + end + + test "optional fields have explicit presence" do + msg = %Proto3Optional{} + assert Presence.field_presence(msg, :a) == :not_present + assert Presence.field_presence(msg, :c) == :not_present + + msg = %Proto3Optional{a: 50, c: "hello"} + assert Presence.field_presence(msg, :a) == :present + assert Presence.field_presence(msg, :c) == :present + + msg = %Proto3Optional{a: 0, c: ""} + assert Presence.field_presence(msg, :a) == :present + assert Presence.field_presence(msg, :c) == :present + end + + test "oneof fields have explicit presence" do + msg = %OneofProto3{} + assert Presence.field_presence(msg, :a) == :not_present + assert Presence.field_presence(msg, :b) == :not_present + + msg = %OneofProto3{first: {:a, 42}} + assert Presence.field_presence(msg, :a) == :present + assert Presence.field_presence(msg, :b) == :not_present + + msg = %OneofProto3{first: {:a, 0}} + assert Presence.field_presence(msg, :a) == :present + + msg = %OneofProto3{first: {:e, :UNKNOWN}} + assert Presence.field_presence(msg, :e) == :present + end + + test "message fields have explicit presence" do + msg = %Foo{} + assert Presence.field_presence(msg, :e) == :not_present + + msg = %Foo{e: %Foo.Bar{}} + assert Presence.field_presence(msg, :e) == :present + end + end + + describe "field_presence/2 for proto2" do + test "singular fields have explicit presence" do + msg = %Foo2{} + assert Presence.field_presence(msg, :a) == :not_present + assert Presence.field_presence(msg, :c) == :not_present + assert Presence.field_presence(msg, :k) == :not_present + + msg = %Foo2{a: 42, c: "hello", k: true, j: :A} + assert Presence.field_presence(msg, :a) == :present + assert Presence.field_presence(msg, :c) == :present + assert Presence.field_presence(msg, :k) == :present + assert Presence.field_presence(msg, :j) == :present + + msg = %Foo2{a: 0, c: "", k: false, j: :UNKNOWN} + assert Presence.field_presence(msg, :a) == :present + assert Presence.field_presence(msg, :c) == :present + assert Presence.field_presence(msg, :k) == :present + assert Presence.field_presence(msg, :j) == :present + end + + test "singular fields with default have implicit presence" do + msg = %Foo2{} + assert Presence.field_presence(msg, :b) == :maybe + + msg = %Foo2{b: 5} # 5 is the default value for :b + assert Presence.field_presence(msg, :b) == :maybe + + msg = %Foo2{b: 6} + assert Presence.field_presence(msg, :b) == :present + end + + test "oneof fields have explicit presence" do + msg = %Oneof{} + assert Presence.field_presence(msg, :a) == :not_present + assert Presence.field_presence(msg, :b) == :not_present + + msg = %Oneof{first: {:a, 42}} + assert Presence.field_presence(msg, :a) == :present + assert Presence.field_presence(msg, :b) == :not_present + + # Even if the value is default, it is present + msg = %Oneof{first: {:a, 0}} + assert Presence.field_presence(msg, :a) == :present + + msg = %Oneof{first: {:e, :UNKNOWN}} + assert Presence.field_presence(msg, :e) == :present + end + + test "message fields have explicit presence" do + msg = %Foo2{} + assert Presence.field_presence(msg, :e) == :not_present + + msg = %Foo2{e: %Foo.Bar{}} + assert Presence.field_presence(msg, :e) == :present + end + end + + describe "field_presence/2" do + test "repeated fields have implicit presence" do + msg = %Foo{g: []} + assert Presence.field_presence(msg, :g) == :maybe + + msg = %Foo{g: [1, 2, 3]} + assert Presence.field_presence(msg, :g) == :present + end + + test "maps have implicit presence" do + msg = %Foo{l: %{}} + assert Presence.field_presence(msg, :l) == :maybe + + msg = %Foo{l: %{"key" => 123}} + assert Presence.field_presence(msg, :l) == :present + end + + # Transform module tests + test "field_presence works with transform modules" do + msg = %ContainsTransformModule{field: 42} + assert Presence.field_presence(msg, :field) == :present + end + end +end diff --git a/test/protobuf/text_test.exs b/test/protobuf/text_test.exs index 7b84852e..7fec38ed 100644 --- a/test/protobuf/text_test.exs +++ b/test/protobuf/text_test.exs @@ -143,7 +143,7 @@ defmodule Protobuf.TextTest do end test "raises on absent proto2 required" do - assert_raise RuntimeError, "field :a is required", fn -> + assert_raise Protobuf.EncodeError, "field :a is required", fn -> Text.encode(%TestMsg.Foo2{}) end end diff --git a/test/support/test_msg.ex b/test/support/test_msg.ex index e90b77b3..6a911495 100644 --- a/test/support/test_msg.ex +++ b/test/support/test_msg.ex @@ -82,8 +82,8 @@ defmodule TestMsg do field :g, 8, repeated: true, type: :int32 # field :h, 9, repeated: true, type: Foo.Bar field :i, 10, repeated: true, type: :int32, packed: true - # field :j, 11, optional: true, type: EnumFoo, enum: true - # field :k, 12, optional: true, type: :bool + field :j, 11, optional: true, type: EnumFoo, enum: true + field :k, 12, optional: true, type: :bool field :l, 13, repeated: true, type: MapFoo, map: true field :non_matched, 101, type: :int32, optional: true end From 1cb4cb00ab1d01b015074b560a6eaac445df86b5 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Thu, 10 Apr 2025 12:58:33 -0300 Subject: [PATCH 3/8] Bump google protos version --- lib/google/protobuf/descriptor.pb.ex | 17 +++++++++++++++++ mix.exs | 2 +- mix.lock | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/google/protobuf/descriptor.pb.ex b/lib/google/protobuf/descriptor.pb.ex index 5311dcf9..8d318745 100644 --- a/lib/google/protobuf/descriptor.pb.ex +++ b/lib/google/protobuf/descriptor.pb.ex @@ -189,6 +189,16 @@ defmodule Google.Protobuf.FeatureSet.JsonFormat do field :LEGACY_BEST_EFFORT, 2 end +defmodule Google.Protobuf.FeatureSet.EnforceNamingStyle do + @moduledoc false + + use Protobuf, enum: true, protoc_gen_elixir_version: "0.13.0", syntax: :proto2 + + field :ENFORCE_NAMING_STYLE_UNKNOWN, 0 + field :STYLE2024, 1 + field :STYLE_LEGACY, 2 +end + defmodule Google.Protobuf.GeneratedCodeInfo.Annotation.Semantic do @moduledoc false @@ -822,6 +832,13 @@ defmodule Google.Protobuf.FeatureSet do enum: true, deprecated: false + field :enforce_naming_style, 7, + optional: true, + type: Google.Protobuf.FeatureSet.EnforceNamingStyle, + json_name: "enforceNamingStyle", + enum: true, + deprecated: false + extensions [{1000, 9995}, {9995, 10000}, {10000, 10001}] end diff --git a/mix.exs b/mix.exs index 16b353e0..10cb3dcc 100644 --- a/mix.exs +++ b/mix.exs @@ -57,7 +57,7 @@ defmodule Protobuf.Mixfile do # and make sure it's there for tests without Git submodules or anything like that. {:google_protobuf, github: "protocolbuffers/protobuf", - ref: "b407e8416e3893036aee5af9a12bd9b6a0e2b2e6", + ref: "43e1626812c1b543e56a7bec59dc09eb18248bd2", submodules: true, app: false, compile: false, diff --git a/mix.lock b/mix.lock index 149deac8..4b086b30 100644 --- a/mix.lock +++ b/mix.lock @@ -8,7 +8,7 @@ "ex_doc": {:hex, :ex_doc, "0.34.2", "13eedf3844ccdce25cfd837b99bea9ad92c4e511233199440488d217c92571e8", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "5ce5f16b41208a50106afed3de6a2ed34f4acfd65715b82a0b84b49d995f95c1"}, "excoveralls": {:hex, :excoveralls, "0.14.6", "610e921e25b180a8538229ef547957f7e04bd3d3e9a55c7c5b7d24354abbba70", [:mix], [{:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "0eceddaa9785cfcefbf3cd37812705f9d8ad34a758e513bb975b081dce4eb11e"}, "file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"}, - "google_protobuf": {:git, "https://github.com/protocolbuffers/protobuf.git", "b407e8416e3893036aee5af9a12bd9b6a0e2b2e6", [ref: "b407e8416e3893036aee5af9a12bd9b6a0e2b2e6", submodules: true]}, + "google_protobuf": {:git, "https://github.com/protocolbuffers/protobuf.git", "43e1626812c1b543e56a7bec59dc09eb18248bd2", [ref: "43e1626812c1b543e56a7bec59dc09eb18248bd2", submodules: true]}, "hackney": {:hex, :hackney, "1.20.1", "8d97aec62ddddd757d128bfd1df6c5861093419f8f7a4223823537bad5d064e2", [:rebar3], [{:certifi, "~> 2.12.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~> 6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~> 1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.4.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "fe9094e5f1a2a2c0a7d10918fee36bfec0ec2a979994cff8cfe8058cd9af38e3"}, "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, From 248a8de2ae5f32d8a4eb708d67d68d7b3bd39304 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Wed, 30 Apr 2025 13:08:13 -0300 Subject: [PATCH 4/8] Move the `field_presence` function to the `Protobuf` module Also fix examples, and add an extra example about repeated --- lib/protobuf.ex | 50 ++++++++++++++++++++++++++++++++++++++++ lib/protobuf/presence.ex | 41 +------------------------------- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/lib/protobuf.ex b/lib/protobuf.ex index 5a3980a2..3480f0bf 100644 --- a/lib/protobuf.ex +++ b/lib/protobuf.ex @@ -285,6 +285,56 @@ defmodule Protobuf do "version that introduced implicit struct generation" end + @doc """ + Returns whether a field or oneof is present, not present, or maybe present + + `:present` and `:not present` mean that a field is **explicitly** present or not, + respectively. + + Some values may be implicitly present. For example, lists in `repeated` fields + always have implicit presence. In these cases, if the presence is ambiguous, + returns `:maybe`. + + For more information about field presence tracking rules, refer to the official + [Field Presence docs](https://protobuf.dev/programming-guides/field_presence/). + + + ## Examples + + # Non-optional proto3 field: + Protobuf.field_presence(%MyMessage{foo: 42}, :foo) + #=> :present + + Protobuf.field_presence(%MyMessage{foo: 0}, :foo) + #=> :maybe + + Protobuf.field_presence(%MyMessage{}, :foo) + #=> :maybe + + # Optional proto3 field: + Protobuf.field_presence(%MyMessage{bar: 42}, :bar) + #=> :present + + Protobuf.field_presence(%MyMessage{bar: 0}, :bar) + #=> :present + + Protobuf.field_presence(%MyMessage{}, :bar) + #=> :not_present + + # Lists + Protobuf.field_presence(%MyMessage{repeated_field: []}, :repeated_field) + #=> :maybe + + Protobuf.field_presence(%MyMessage{repeated_field: [1}, :repeated_field) + #=> :present + + """ + @doc since: "0.15.0" + @spec field_presence(message :: struct(), field :: atom()) :: :present | :not_present | :maybe + def field_presence(message, field) do + Protobuf.Presence.field_presence(message, field) + end + @doc """ Loads extensions modules. diff --git a/lib/protobuf/presence.ex b/lib/protobuf/presence.ex index 409845bf..9d2a2463 100644 --- a/lib/protobuf/presence.ex +++ b/lib/protobuf/presence.ex @@ -1,47 +1,8 @@ defmodule Protobuf.Presence do - @moduledoc """ - Helpers for determining Protobuf field presence. - """ + @moduledoc false alias Protobuf.FieldProps - @doc """ - Returns whether a field or oneof is present, not present, or maybe present - - `:present` and `:not present` mean that a field is **explicitly** present or not, - respectively. - - Some values may be implicitly present. For example, lists in `repeated` fields - always have implicit presence. In these cases, if the presence is ambiguous, - returns `:maybe`. - - For more information about field presence tracking rules, refer to the official - [Field Presence docs](https://protobuf.dev/programming-guides/field_presence/). - - - ## Examples - - # Non-optional proto3 field: - Protobuf.Presence(%MyMessage{foo: 42}, :foo) - #=> :present - - Protobuf.Presence(%MyMessage{foo: 0}, :foo) - #=> :maybe - - Protobuf.Presence(%MyMessage{}, :foo) - #=> :maybe - - # Optional proto3 field: - Protobuf.Presence(%MyMessage{bar: 42}, :bar) - #=> :present - - Protobuf.Presence(%MyMessage{bar: 0}, :bar) - #=> :present - - Protobuf.Presence(%MyMessage{}, :bar) - #=> :not_present - - """ @spec field_presence(message :: struct(), field :: atom()) :: :present | :not_present | :maybe def field_presence(%mod{} = message, field) do message_props = mod.__message_props__() From 7560e91eb9ff617c3af7e705012c2926583016a5 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Wed, 30 Apr 2025 13:11:38 -0300 Subject: [PATCH 5/8] Add @doc since to Protobuf.Text --- lib/protobuf/text.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/protobuf/text.ex b/lib/protobuf/text.ex index 92675ac9..054cb929 100644 --- a/lib/protobuf/text.ex +++ b/lib/protobuf/text.ex @@ -26,6 +26,7 @@ defmodule Protobuf.Text do Doesn't perform type validations. If input data is invalid, it produces undecodable output. """ + @doc since: "0.15.0" @spec encode(struct(), Keyword.t()) :: binary() def encode(%mod{} = struct, opts \\ []) do max_line_width = Keyword.get(opts, :max_line_width, 80) From 2e91c8af5c537eea38e528af31a7ed7fd16cdfcb Mon Sep 17 00:00:00 2001 From: felipe stival <14948182+v0idpwn@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:38:49 -0300 Subject: [PATCH 6/8] Doc improvement in Protobuf.field_presence/2 --- lib/protobuf.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/protobuf.ex b/lib/protobuf.ex index 3480f0bf..44612d93 100644 --- a/lib/protobuf.ex +++ b/lib/protobuf.ex @@ -321,7 +321,7 @@ defmodule Protobuf do Protobuf.field_presence(%MyMessage{}, :bar) #=> :not_present - # Lists + # Repeated Protobuf.field_presence(%MyMessage{repeated_field: []}, :repeated_field) #=> :maybe From b5258459ca9f8f761688dd78cace76ea954b4446 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Sat, 3 May 2025 10:57:24 -0300 Subject: [PATCH 7/8] Consistent map handling Maps should be considered present when used for messages --- lib/protobuf/presence.ex | 2 +- test/protobuf/presence_test.exs | 17 ++++++++++++++++- test/support/test_msg.ex | 2 ++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/protobuf/presence.ex b/lib/protobuf/presence.ex index 9d2a2463..3a838556 100644 --- a/lib/protobuf/presence.ex +++ b/lib/protobuf/presence.ex @@ -55,7 +55,7 @@ defmodule Protobuf.Presence do :maybe end - def get_field_presence(_syntax, val, _prop) when is_map(val) do + def get_field_presence(_syntax, val, %FieldProps{map?: true}) when is_map(val) do if map_size(val) == 0 do :maybe else diff --git a/test/protobuf/presence_test.exs b/test/protobuf/presence_test.exs index ef43dd51..d3ad200d 100644 --- a/test/protobuf/presence_test.exs +++ b/test/protobuf/presence_test.exs @@ -50,6 +50,9 @@ defmodule Protobuf.PresenceTest do msg = %OneofProto3{first: {:a, 0}} assert Presence.field_presence(msg, :a) == :present + msg = %OneofProto3{first: {:f, %{}}} + assert Presence.field_presence(msg, :f) == :present + msg = %OneofProto3{first: {:e, :UNKNOWN}} assert Presence.field_presence(msg, :e) == :present end @@ -60,6 +63,11 @@ defmodule Protobuf.PresenceTest do msg = %Foo{e: %Foo.Bar{}} assert Presence.field_presence(msg, :e) == :present + + # We (still) accept maps and cast them into structs, so empty maps must + # be present + msg = %Foo{e: %{}} + assert Presence.field_presence(msg, :e) == :present end end @@ -103,10 +111,12 @@ defmodule Protobuf.PresenceTest do assert Presence.field_presence(msg, :a) == :present assert Presence.field_presence(msg, :b) == :not_present - # Even if the value is default, it is present msg = %Oneof{first: {:a, 0}} assert Presence.field_presence(msg, :a) == :present + msg = %Oneof{first: {:f, %{}}} + assert Presence.field_presence(msg, :f) == :present + msg = %Oneof{first: {:e, :UNKNOWN}} assert Presence.field_presence(msg, :e) == :present end @@ -117,6 +127,11 @@ defmodule Protobuf.PresenceTest do msg = %Foo2{e: %Foo.Bar{}} assert Presence.field_presence(msg, :e) == :present + + # We (still) accept maps and cast them into structs, so empty maps must + # be present + msg = %Foo2{e: %{}} + assert Presence.field_presence(msg, :e) == :present end end diff --git a/test/support/test_msg.ex b/test/support/test_msg.ex index 6a911495..ae8a708f 100644 --- a/test/support/test_msg.ex +++ b/test/support/test_msg.ex @@ -113,6 +113,7 @@ defmodule TestMsg do field :c, 3, optional: true, type: :int32, oneof: 1 field :d, 4, optional: true, type: :string, oneof: 1 field :e, 6, optional: true, type: EnumFoo, enum: true, default: :A, oneof: 0 + field :f, 7, type: Foo2, oneof: 0 field :other, 5, optional: true, type: :string end @@ -127,6 +128,7 @@ defmodule TestMsg do field :c, 3, optional: true, type: :int32, oneof: 1 field :d, 4, optional: true, type: :string, oneof: 1 field :e, 6, type: EnumFoo, enum: true, oneof: 0 + field :f, 7, type: Foo, oneof: 0 field :other, 5, optional: true, type: :string end From f76e03a997a220ef9b84092c19aca4f4def0ac44 Mon Sep 17 00:00:00 2001 From: v0idpwn Date: Sat, 3 May 2025 10:57:36 -0300 Subject: [PATCH 8/8] format --- lib/protobuf/encoder.ex | 4 ++-- lib/protobuf/presence.ex | 3 ++- lib/protobuf/text.ex | 12 ++++++++---- test/protobuf/presence_test.exs | 3 ++- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/protobuf/encoder.ex b/lib/protobuf/encoder.ex index 18bdc7ee..c1ab45cf 100644 --- a/lib/protobuf/encoder.ex +++ b/lib/protobuf/encoder.ex @@ -87,8 +87,8 @@ defmodule Protobuf.Encoder do case Protobuf.Presence.get_field_presence(syntax, value, field_prop) do :present -> false # Proto2 required isn't skipped even if not present - :maybe -> not(syntax == :proto2 && field_prop.required?) - :not_present -> not(syntax == :proto2 && field_prop.required?) + :maybe -> not (syntax == :proto2 && field_prop.required?) + :not_present -> not (syntax == :proto2 && field_prop.required?) end end diff --git a/lib/protobuf/presence.ex b/lib/protobuf/presence.ex index 3a838556..32e6ec50 100644 --- a/lib/protobuf/presence.ex +++ b/lib/protobuf/presence.ex @@ -47,7 +47,8 @@ defmodule Protobuf.Presence do # it until we add editions support, since we definitely don't want to add # `syntax` in a public API @doc false - @spec get_field_presence(:proto2 | :proto3, term(), FieldProps.t()) :: :present | :not_present | :maybe + @spec get_field_presence(:proto2 | :proto3, term(), FieldProps.t()) :: + :present | :not_present | :maybe def get_field_presence(syntax, value, field_prop) # Repeated and maps are always implicit. diff --git a/lib/protobuf/text.ex b/lib/protobuf/text.ex index 054cb929..835c391b 100644 --- a/lib/protobuf/text.ex +++ b/lib/protobuf/text.ex @@ -124,7 +124,8 @@ defmodule Protobuf.Text do Algebra.empty() _ -> - raise Protobuf.EncodeError, message: "invalid value for oneof `#{inspect(name)}`: #{inspect(value)}" + raise Protobuf.EncodeError, + message: "invalid value for oneof `#{inspect(name)}`: #{inspect(value)}" end else raise Protobuf.EncodeError, message: "unknown field #{inspect(name)}" @@ -169,7 +170,10 @@ defmodule Protobuf.Text do end defp encode_list(list, syntax, field_prop, print_unknown?) do - fun = fn val, _opts -> encode_value(val, syntax, %{field_prop | repeated?: false, map?: false}, print_unknown?) end + fun = fn val, _opts -> + encode_value(val, syntax, %{field_prop | repeated?: false, map?: false}, print_unknown?) + end + Algebra.container_doc("[", list, "]", inspect_opts(), fun, break: :strict) end @@ -185,8 +189,8 @@ defmodule Protobuf.Text do case Protobuf.Presence.get_field_presence(syntax, value, field_prop) do :present -> false # Proto2 required isn't skipped even if not present - :maybe -> not(syntax == :proto2 && field_prop.required?) - :not_present -> not(syntax == :proto2 && field_prop.required?) + :maybe -> not (syntax == :proto2 && field_prop.required?) + :not_present -> not (syntax == :proto2 && field_prop.required?) end end diff --git a/test/protobuf/presence_test.exs b/test/protobuf/presence_test.exs index d3ad200d..80403f76 100644 --- a/test/protobuf/presence_test.exs +++ b/test/protobuf/presence_test.exs @@ -95,7 +95,8 @@ defmodule Protobuf.PresenceTest do msg = %Foo2{} assert Presence.field_presence(msg, :b) == :maybe - msg = %Foo2{b: 5} # 5 is the default value for :b + # 5 is the default value for :b + msg = %Foo2{b: 5} assert Presence.field_presence(msg, :b) == :maybe msg = %Foo2{b: 6}