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

feat: added ParseFromString method to message for compatibility #336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 23 additions & 0 deletions src/betterproto/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,29 @@ def FromString(cls: Type[T], data: bytes) -> T:
"""
return cls().parse(data)

# For compatibility with Google protobuf official implementation.
def ParseFromString(self, data: bytes) -> T:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def ParseFromString(self, data: bytes) -> T:
def ParseFromString(self: T, data: bytes) -> T:

"""
Parse the binary encoded Protobuf into this message instance. This
returns the instance itself and is therefore assignable and chainable.

.. note::
This is a method for compatibility with other libraries,
you should really use :meth:`parse`.


Parameters
-----------
data: :class:`bytes`
The data to parse the protobuf from.

Returns
--------
:class:`Message`
The initialized message.
"""
return self.parse(data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this slightly different from the offical implementation which clears any currently set values? https://github.com/protocolbuffers/protobuf/blob/2a2a9b6e64e0cda49b0e5377cad58c790087e1c9/python/google/protobuf/message.py#L193-L199

Choose a reason for hiding this comment

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

This indeed seems to be the case.

I've added a simple example which shows the difference in result from ParseFromString with the one from this PR:

syntax = "proto3";

package some.package.name;

message Cat {
  int64 age = 1;
  int64 lives = 2;
}
def ParseFromString(self, data: bytes):
    return self.parse(data)

if __name__ == '__main__':
    # Create bytes
    cat = Cat(
        age=5,
        # lives=9,
    )

    data = cat.SerializeToString()

    # Dynamically add ParseFromString to the Cat class
    Cat.ParseFromString = ParseFromString

    # ParseFromString for the betterproto cat and the pb2 cat
    cat_betterproto = Cat(age=3, lives=8)
    cat_pb2 = Cat_pb2(age=3, lives=8)

    cat_betterproto.ParseFromString(data)
    cat_pb2.ParseFromString(data)

    print(cat_betterproto)  # Cat(age=5, lives=8), lives are not cleared
    print(cat_pb2)  # age: 5, lives are cleared
    assert cat_pb2.lives == 0


def to_dict(
self, casing: Casing = Casing.CAMEL, include_default_values: bool = False
) -> Dict[str, Any]:
Expand Down
9 changes: 9 additions & 0 deletions tests/test_features.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,12 @@ def test_service_argument__expected_parameter():
do_thing_request_parameter = sig.parameters["do_thing_request"]
assert do_thing_request_parameter.default is Parameter.empty
assert do_thing_request_parameter.annotation == "DoThingRequest"


def test_message_parse_from_string():
@dataclass
class SimpleMessage(betterproto.Message):
message: str = betterproto.string_field(1)

test_message = SimpleMessage(message="test message")
assert test_message == SimpleMessage().ParseFromString(bytes(test_message))