Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: setting 64bit fields from strings supported #267

Merged
merged 5 commits into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions docs/marshal.rst
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,6 @@ Protocol buffer type Python type Nullable
msg_two = MyMessage(msg_dict)

assert msg == msg_pb == msg_two

.. warning::

Due to certain browser/javascript limitations, 64 bit sized fields, e.g. INT64, UINT64,
are converted to strings when marshalling messages to dictionaries or JSON.
Decoding JSON handles this correctly, but dicts must be unpacked when reconstructing messages. This is necessary to trigger a special case workaround.

.. code-block:: python

import proto

class MyMessage(proto.Message):
serial_id = proto.Field(proto.INT64, number=1)

msg = MyMessage(serial_id=12345)
msg_dict = MyMessage.to_dict(msg)

msg_2 = MyMessage(msg_dict) # Raises an exception

msg_3 = MyMessage(**msg_dict) # Works without exception
assert msg == msg_3


Wrapper types
Expand Down
11 changes: 10 additions & 1 deletion proto/marshal/rules/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ def to_proto(self, value):
if isinstance(value, self._wrapper):
return self._wrapper.pb(value)
if isinstance(value, dict) and not self.is_map:
return self._descriptor(**value)
# We need to use the wrapper's marshaling to handle
# potentially problematic nested messages.
try:
# Try the fast path first.
return self._descriptor(**value)
except TypeError as ex:
# If we have a type error,
# try the slow path in case the error
# was an int64/string issue
return self._wrapper(value)._pb
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay! that is a waaaay better devx.

return value

@property
Expand Down
39 changes: 31 additions & 8 deletions proto/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ def __new__(mcls, name, bases, attrs):
# Determine the name of the entry message.
msg_name = "{pascal_key}Entry".format(
pascal_key=re.sub(
r"_\w", lambda m: m.group()[1:].upper(), key,
r"_\w",
lambda m: m.group()[1:].upper(),
key,
).replace(key[0], key[0].upper(), 1),
)

Expand All @@ -83,20 +85,26 @@ def __new__(mcls, name, bases, attrs):
{
"__module__": attrs.get("__module__", None),
"__qualname__": "{prefix}.{name}".format(
prefix=attrs.get("__qualname__", name), name=msg_name,
prefix=attrs.get("__qualname__", name),
name=msg_name,
),
"_pb_options": {"map_entry": True},
}
)
entry_attrs["key"] = Field(field.map_key_type, number=1)
entry_attrs["value"] = Field(
field.proto_type, number=2, enum=field.enum, message=field.message,
field.proto_type,
number=2,
enum=field.enum,
message=field.message,
)
map_fields[msg_name] = MessageMeta(msg_name, (Message,), entry_attrs)

# Create the repeated field for the entry message.
map_fields[key] = RepeatedField(
ProtoType.MESSAGE, number=field.number, message=map_fields[msg_name],
ProtoType.MESSAGE,
number=field.number,
message=map_fields[msg_name],
)

# Add the new entries to the attrs
Expand Down Expand Up @@ -288,7 +296,13 @@ def pb(cls, obj=None, *, coerce: bool = False):
if coerce:
obj = cls(obj)
else:
raise TypeError("%r is not an instance of %s" % (obj, cls.__name__,))
raise TypeError(
"%r is not an instance of %s"
% (
obj,
cls.__name__,
)
)
return obj._pb

def wrap(cls, pb):
Expand Down Expand Up @@ -394,7 +408,7 @@ def to_dict(
determines whether field name representations preserve
proto case (snake_case) or use lowerCamelCase. Default is True.
including_default_value_fields (Optional(bool)): An option that
determines whether the default field values should be included in the results.
determines whether the default field values should be included in the results.
Default is True.

Returns:
Expand Down Expand Up @@ -453,7 +467,13 @@ class Message(metaclass=MessageMeta):
message.
"""

def __init__(self, mapping=None, *, ignore_unknown_fields=False, **kwargs):
def __init__(
self,
mapping=None,
*,
ignore_unknown_fields=False,
**kwargs,
):
# We accept several things for `mapping`:
# * An instance of this class.
# * An instance of the underlying protobuf descriptor class.
Expand Down Expand Up @@ -493,7 +513,10 @@ def __init__(self, mapping=None, *, ignore_unknown_fields=False, **kwargs):
# Sanity check: Did we get something not a map? Error if so.
raise TypeError(
"Invalid constructor input for %s: %r"
% (self.__class__.__name__, mapping,)
% (
self.__class__.__name__,
mapping,
)
)

params = {}
Expand Down
20 changes: 19 additions & 1 deletion tests/test_fields_int.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,24 @@ class Squid(proto.Message):

s_dict = Squid.to_dict(s)

s2 = Squid(**s_dict)
s2 = Squid(s_dict)

assert s == s2

# Double check that the conversion works with deeply nested messages.
class Clam(proto.Message):
class Shell(proto.Message):
class Pearl(proto.Message):
mass_kg = proto.Field(proto.INT64, number=1)

pearl = proto.Field(Pearl, number=1)

shell = proto.Field(Shell, number=1)

c = Clam(shell=Clam.Shell(pearl=Clam.Shell.Pearl(mass_kg=10)))

c_dict = Clam.to_dict(c)

c2 = Clam(c_dict)

assert c == c2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent roundtrip demo.