Support using Google's wrapper types as RPC output values

[boukeversteegh]

This PR fixes #39

Well known types as Output

Adds support for returning Google's well-known wrapper types directly from Services.

service Test {
    rpc GetInt32 (Input) returns (google.protobuf.Int32Value);
}

Supported added for

• BoolValue
• BytesValue
• DoubleValue
• FloatValue
• Int32Value
• Int64Value
• StringValue
• UInt32Value
• UInt64Value

---

Not included in this PR

• Returning well known types that are not wrappers
  • Empty — However, Importing well known types #9 can be fixed relatively easily now.
• Well known types (wrappers or not) as Input parameters.

  service Test {
      rpc GetSquare (google.protobuf.Int32Value) returns (google.protobuf.Int32Value);
  }

  It shouldn't be too hard to support this either.
• Automatically unwrapping the wrapped values when they are returned by the server

  This requires some further changes, and would be great, but for now this already is a step forward.

---

Notes:

• I was not sure about the right way to test this change. If you think a test is necessary, I'd be happy to discuss a strategy for this.
• I did add googletypes_service.proto to the test-cases, so the output can be manually verified.

Let me know if anything doesn't match style requirements, or some other changes need to be made.

[nat-n]

Left some minor suggestions on the code.

I assume that new test example would have failed before?

If you're up for it I think it would be useful to document the things you've identified as still to do in a new issue, to make it easier to pick up and finish.

[nat-n]

I'm not sure about the trade off here in keeping WRAPPER_TYPES declarative as a simple mapping at the cost of not being able to actually use it directly as a mapping.

What about replacing WRAPPER_TYPES and get_wrapper_type here with:

WRAPPER_TYPES = {
    goog_type.DESCRIPTOR.full_name: (goog_type, simple_name)
    for goog_type, simple_name in (
        (google.protobuf.wrappers_pb2.DoubleValue, "float"),
        (google.protobuf.wrappers_pb2.FloatValue, "float"),
        (google.protobuf.wrappers_pb2.Int64Value, "int"),
        (google.protobuf.wrappers_pb2.UInt64Value, "int"),
        (google.protobuf.wrappers_pb2.Int32Value, "int"),
        (google.protobuf.wrappers_pb2.UInt32Value, "int"),
        (google.protobuf.wrappers_pb2.BoolValue, "bool"),
        (google.protobuf.wrappers_pb2.StringValue, "str"),
        (google.protobuf.wrappers_pb2.BytesValue, "bytes"),
    )
}

[boukeversteegh]

I see what you mean, good suggestion! That get_wrapper_type a bit ugly indeed, and would be nice to not need it, while at the same time, I did not like the unchecked references to packages. Your idea solves both, although it embarrassingly took me a several re-reads to fully understand it.

To build on your idea, how about two maps? Same concept but less inlining..

WRAPPER_TYPES = {
    google.protobuf.wrappers_pb2.DoubleValue: "float",
    google.protobuf.wrappers_pb2.FloatValue: "float",
    google.protobuf.wrappers_pb2.Int64Value: "int",
    google.protobuf.wrappers_pb2.UInt64Value: "int",
    google.protobuf.wrappers_pb2.Int32Value: "int",
    google.protobuf.wrappers_pb2.UInt32Value: "int",
    google.protobuf.wrappers_pb2.BoolValue: "bool",
    google.protobuf.wrappers_pb2.StringValue: "str",
    google.protobuf.wrappers_pb2.BytesValue: "bytes",
}

WRAPPERS = {w.DESCRIPTOR.full_name: w for w in WRAPPER_TYPES.keys()}

# usage
wrapper = WRAPPERS['google.protobuf.UInt64Value']

print(wrapper.__module__)
print(WRAPPER_TYPES[wrapper])

the only think i don't like about it is having two similar maps with similar names.

[nat-n]

My preference is for the single WRAPPER_TYPES dict for conciseness, though I get that the idiom might not be as clear.

As you prefer.

[boukeversteegh]

Well, I found out we could also just extract the desired type from instance().value , so we don't need the tuples in the map

wrappers = {
    'google.protobuf.DoubleValue': google.protobuf.wrappers_pb2.DoubleValue,
    'google.protobuf.FloatValue' : google.protobuf.wrappers_pb2.FloatValue,
    'google.protobuf.Int64Value' : google.protobuf.wrappers_pb2.Int64Value,
    'google.protobuf.UInt64Value' : google.protobuf.wrappers_pb2.UInt64Value,
    'google.protobuf.Int32Value' : google.protobuf.wrappers_pb2.Int32Value,
    'google.protobuf.UInt32Value' : google.protobuf.wrappers_pb2.UInt32Value,
    'google.protobuf.BoolValue' : google.protobuf.wrappers_pb2.BoolValue,
    'google.protobuf.StringValue' : google.protobuf.wrappers_pb2.StringValue,
    'google.protobuf.BytesValue' : google.protobuf.wrappers_pb2.BytesValue,
}

# usage
wrapper = wrappers['google.protobuf.BytesValue']
mapped_type = type(wrapper().value) # string

[boukeversteegh]

I've implemented this concept. Please see 8f0caf1

[boukeversteegh]

Left some minor suggestions on the code.

I assume that new test example would have failed before?

The provided example indeed would not have worked before, because the generated call to the client passes Optional[int] as the response_type, which is supposed to be a protobuf message type, so somewhere down the line Exception: '_SpecialForm' object has no attribute 'FromString' is thrown. SpecialForm is refering to Optional.

    async def start_training_game(self) -> Optional[int]:
        ...
        return await self._unary_unary(
            "/anagon.ai.go.Go/StartTrainingGame", request, Optional[int],
        )

I'm currently also working on structuring the tests in such a way that these kinds of examples can be tested more easily, but for now i don't have a test for this..

If you're up for it I think it would be useful to document the things you've identified as still to do in a new issue, to make it easier to pick up and finish.

yes, certainly! i'll do that

[boukeversteegh]

**EDIT: **
The message below was intended for another PR. OOOOOPS!

⚠️ This PR is DRAFT and is not (anymore) intended to be merged, but serves as a proof of concept and discussion platform of which conclusions will be applied to the final PR.

• @nat-n and I had a discussion and we want to first make sure we can create proper test-cases for all scenarios. The PR for this is Organize test-cases #51.
• Once that is done, some of the refactors that were part of this PR will be extracted, excluding the bugfixes, into another PR.
• After that, we will work on modularizing the plugin as suggested in Split code up into multiple modules #49
• Then I will start creating one or more PR's for these importing bugs, including new test-cases. I hope to be able to align with @danielgtaylor and @cetanu on the code-generation strategy that is needed for this.

[boukeversteegh]

Well, oops. The message above was ment for #47 !
I wondered why my comment didn't show up there, and I couldn't find it back after searching everywhere.

This PR can actually be merged... sorry for the confusion