Pydantic validation error when creating instance without parameter of message with oneof

AdrienVannson commented 2 months ago

Summary

Decoding a message with oneof fails.

Reproduction Steps

To show the problem, here is a simple example. Compile the following proto messages:

message X {
    int32 v = 1;
}

message Y {
    int32 v = 1;
}

message Choice {
    oneof choice {
        X x = 1;
        Y y = 2;
    }
}

In a Python script, creating an instance without any parameter ( Choice() ) fails with a validation error: Value error, Group choice has no value; all fields are None.

This error comes from _validate_field_groups in src/betterproto/__init__.py.

Expected Results

In this simple case, the error might seem expected. However, betterproto creates instances of messages without any parameter a bit everywhere in the code. For example, parsing a message is done as follows: value = cls().parse(value). An instance is created without any parameter, which fails if the message contains a oneof field.

Actual Results

I would have expected receiving messages with oneof fields to work.

I see two potential solutions:

rewrite the code to avoid creating instances of messages without parameters. I think parse, from_dict , etc should probably be class methods instead. It would probably need many changes in the source code, and would also need an update to the rust codec
Remove the check that at least one field in the oneof is specified. It would be easier, but it would it would remove a check that is useful. Plus, I would say that the methods like parse have no reason to be called on an actual instance of the object (they are always called on an instance created just for the call).

What do you think?

System Information

libprotoc 3.12.4 Python 3.12.5 Name: betterproto Version: 2.0.0b7 Summary: A better Protobuf / gRPC generator & library Home-page: https://github.com/danielgtaylor/python-betterproto Author: Daniel G. Taylor Author-email: danielgtaylor@gmail.com License: MIT Location: XXX Requires: grpclib, python-dateutil, typing-extensions Required-by: pygrpc-poc

Checklist

[X] I have searched the issues for duplicates.
[X] I have shown the entire traceback, if possible.
[X] I have verified this issue occurs on the latest prelease of betterproto which can be installed using pip install -U --pre betterproto, if possible.

Gobot1234 commented 2 months ago

The parse methods used to be hybridmethods not sure why this code has disappeared

AdrienVannson commented 2 months ago

I don't think so, if I do a git blame, I see that the function was last updated here: https://github.com/danielgtaylor/python-betterproto/commit/8659c511

This commit creates the parse function directly as a standard method. However, from_dict is hybrid.

danielgtaylor / python-betterproto