to_dict modifies the underlying message

[upcFrost]

Hi,

It seems that the to_dict method modifies the nested message.

In short, here's the code and the output (version from master, pulled 24-09-2020)

from dataclasses import dataclass
from typing import Dict

import betterproto


class Code(betterproto.Enum):
    ZERO = 0
    ONE = 1

@dataclass(eq=False, repr=False)
class Message(betterproto.Message):
    codes: Dict[str, "Nested"] = betterproto.map_field(2, betterproto.TYPE_STRING, betterproto.TYPE_MESSAGE)

    def __post_init__(self) -> None:
        super().__post_init__()


@dataclass(eq=False, repr=False)
class Nested(betterproto.Message):
    code: "Code" = betterproto.enum_field(1)

    def __post_init__(self) -> None:
        super().__post_init__()

msg = Message(codes={'test': Nested(code=Code.ONE)})
print(msg.codes['test'])
print(msg.to_dict())
print(msg.codes['test'])

Nested(code=<Code.ONE: 1>)
{'codes': {'test': {'code': 'ONE'}}}
{'code': 'ONE'}

Also, if you'll change ONE to ZERO, it won't print anything, as ZERO is a default value (even though it was set manually)

[nat-n]

Thanks for reporting this. If you could add a failing test case following the existing pattern then that would be a huge help.

[upcFrost]

Test as diff from the current master (17.10.2020, ID: bf9412e)

diff --git a/tests/test_inputs.py b/tests/test_inputs.py
index f09ee79..737d7c7 100644
--- a/tests/test_inputs.py
+++ b/tests/test_inputs.py
@@ -165,3 +165,10 @@ def test_binary_compatibility(repeat, test_data: TestData) -> None:
         assert (
             plugin_instance_from_json.to_dict() == plugin_instance_from_binary.to_dict()
         )
+
+@pytest.mark.parametrize("test_data", test_cases.messages_with_json, indirect=True)
+def test_to_dict(repeat, test_data: TestData) -> None:
+    plugin_module, reference_module, json_data = test_data
+    message = plugin_module.Test().from_json(json_data)
+    message.to_dict()
+    assert message == plugin_module.Test().from_json(json_data)

Proto and message for the test (for me this bug happens only when enum is used as a map value)

syntax = "proto3";

message Test {
  map<string, Nested> items = 1;
}

enum Nested {
  a = 0;
  b = 1;
}

{
  "items": {
    "foo": {
      "count": 1
    },
    "bar": {
      "count": 2
    }
  }
}

Result

Expected :Test(items={'foo': Nested(count=1), 'bar': Nested(count=2)})
Actual   :Test(items={'foo': {'count': 1}, 'bar': {'count': 2}})

[upcFrost]

Also I've spotted that when you call to_dict, it will initialise the next level of the nested message, though it will be initialized on __getattribute__ anyway if you'll try to query it

[jonmather]

Can confirm this occurs for any map which has a Message as a value. I think there's quite a simple fix, just need a slight modification to lines 1052-1058 of init.py, specifically line 1055.
Current code

            elif meta.proto_type == TYPE_MAP:
                for k in value:
                    if hasattr(value[k], "to_dict"):
                        value[k] = value[k].to_dict(casing, include_default_values)

                if value or include_default_values:
                    output[cased_name] = value

Proposed fix

            elif meta.proto_type == TYPE_MAP:
                output_map = {}
                for k in value:
                    if hasattr(value[k], "to_dict"):
                        output_map[k] = value[k].to_dict(casing, include_default_values)

                if value or include_default_values:
                    output[cased_name] = output_map

[Gobot1234]

Can confirm this occurs for any map which has a Message as a value. I think there's quite a simple fix, just need a slight modification to lines 1052-1058 of init.py, specifically line 1055.
Current code

            elif meta.proto_type == TYPE_MAP:
                for k in value:
                    if hasattr(value[k], "to_dict"):
                        value[k] = value[k].to_dict(casing, include_default_values)

                if value or include_default_values:
                    output[cased_name] = value

Proposed fix

            elif meta.proto_type == TYPE_MAP:
                output_map = {}
                for k in value:
                    if hasattr(value[k], "to_dict"):
                        output_map[k] = value[k].to_dict(casing, include_default_values)

                if value or include_default_values:
                    output[cased_name] = output_map

Can you test and PR this?