recap icon indicating copy to clipboard operation
recap copied to clipboard

Support `_Parent_._Type_` references in `ProtobufConverter.to_recap`

Open criccomini opened this issue 2 years ago • 1 comments

While reading the proto3 spec, I noticed this tidbit:

https://protobuf.dev/programming-guides/proto3/#nested

If you want to reuse this message type outside its parent message type, you refer to it as Parent.Type

message SomeOtherMessage {
  SearchResponse.Result result = 1;
}

This doesn't work with Recap right now. Recap will interpret SearchResponse.Result as a fully qualified namespace. I need to figure out how to deal with this.

criccomini avatar Jul 12 '23 02:07 criccomini

This turns out to be really non-trivial. Protobuf's name resolution rules are a real mess.

https://github.com/bufbuild/buf/issues/314 https://github.com/protocolbuffers/protobuf/issues/6296

Here are some tests that should work:

def test_protobuf_converter_nested_types():
    protobuf_schema = """
        syntax = "proto3";
        message SomeOtherMessage {
            SearchResponse.Result result = 1;
        }
        message SearchResponse {
            message Result {
                string url = 1;
            }
            repeated Result results = 1;
        }
    """

    recap_schema = ProtobufConverter("foo.bar").to_recap(protobuf_schema)
    assert recap_schema == StructType(
        [
            UnionType(
                [
                    NullType(),
                    StructType(
                        [
                            UnionType(
                                [
                                    NullType(),
                                    StringType(bytes_=2_147_483_648),
                                ],
                                name="url",
                                default=None,
                            )
                        ],
                        alias="foo.bar.SearchResponse.Result",
                    ),
                ],
                name="result",
                default=None,
            )
        ],
        alias="foo.bar.SomeOtherMessage",
    )


def test_protobuf_converter_namespace_resolution():
    protobuf_schema_1 = """
        syntax = "proto3";
        package foo.blah;
        message Inner {
            int32 innerValue = 1;
        }
    """
    protobuf_schema_2 = """
        syntax = "proto3";
        package foo.bar;
        message Outer {
            blah.Inner outerValue = 1;
        }
    """

    converter = ProtobufConverter()

    # Ignore return result. Just calling to load Inner into registry.
    converter.to_recap(protobuf_schema_1)

    recap_schema_2 = converter.to_recap(protobuf_schema_2)

    assert recap_schema_2 == StructType(
        [
            UnionType(
                [
                    NullType(),
                    StructType(
                        [
                            UnionType(
                                [
                                    NullType(),
                                    IntType(bits=32, signed=True),
                                ],
                                name="innerValue",
                                default=None,
                            )
                        ],
                        alias="foo.blah.Inner",
                    ),
                ],
                name="outerValue",
                default=None,
            )
        ],
        alias="_root.Outer",
    )


def test_protobuf_converter_namespace_root():
    protobuf_schema_1 = """
        syntax = "proto3";
        package foo.blah;
        message Inner {
            int32 innerValue = 1;
        }
    """
    protobuf_schema_2 = """
        syntax = "proto3";
        package foo.bar;
        message Outer {
            .foo.blah.Inner outerValue = 1;
        }
    """

    converter = ProtobufConverter()

    # Ignore return result. Just calling to load Inner into registry.
    converter.to_recap(protobuf_schema_1)

    recap_schema_2 = converter.to_recap(protobuf_schema_2)

    assert recap_schema_2 == StructType(
        [
            UnionType(
                [
                    NullType(),
                    StructType(
                        [
                            UnionType(
                                [
                                    NullType(),
                                    IntType(bits=32, signed=True),
                                ],
                                name="innerValue",
                                default=None,
                            )
                        ],
                        alias="foo.blah.Inner",
                    ),
                ],
                name="outerValue",
                default=None,
            )
        ],
        alias="_root.Outer",
    )

I'm going to leave this for now. It's a bunch of work with questionable returns for the moment.

criccomini avatar Jul 12 '23 21:07 criccomini