Skip to content

Commit

Permalink
fix: resolve issue where marshal fails with cross api dependency (#348)
Browse files Browse the repository at this point in the history
* fix: resolve issue where marshal fails with cross api dependency

* Retrieve rules for cross api compatibility

* add comment with link to bug
  • Loading branch information
parthea authored Jun 12, 2023
1 parent 9270fe5 commit 0dcea18
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 7 deletions.
25 changes: 18 additions & 7 deletions proto/marshal/marshal.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,22 @@ def reset(self):
for rule_class in stringy_numbers.STRINGY_NUMBER_RULES:
self.register(rule_class._proto_type, rule_class())

def get_rule(self, proto_type):
# Rules are needed to convert values between proto-plus and pb.
# Retrieve the rule for the specified proto type.
# The NoopRule will be used when a rule is not found.
rule = self._rules.get(proto_type, self._noop)

# If we don't find a rule, also check under `_instances`
# in case there is a rule in another package.
# See https://github.com/googleapis/proto-plus-python/issues/349
if rule == self._noop and hasattr(self, "_instances"):
for _, instance in self._instances.items():
rule = instance._rules.get(proto_type, self._noop)
if rule != self._noop:
break
return rule

def to_python(self, proto_type, value, *, absent: bool = None):
# Internal protobuf has its own special type for lists of values.
# Return a view around it that implements MutableSequence.
Expand All @@ -174,10 +190,7 @@ def to_python(self, proto_type, value, *, absent: bool = None):
# Same thing for maps of messages.
if value_type in compat.map_composite_types:
return MapComposite(value, marshal=self)

# Convert ordinary values.
rule = self._rules.get(proto_type, self._noop)
return rule.to_python(value, absent=absent)
return self.get_rule(proto_type=proto_type).to_python(value, absent=absent)

def to_proto(self, proto_type, value, *, strict: bool = False):
# The protos in google/protobuf/struct.proto are exceptional cases,
Expand Down Expand Up @@ -212,9 +225,7 @@ def to_proto(self, proto_type, value, *, strict: bool = False):
recursive_type = type(proto_type().value)
return {k: self.to_proto(recursive_type, v) for k, v in value.items()}

# Convert ordinary values.
rule = self._rules.get(proto_type, self._noop)
pb_value = rule.to_proto(value)
pb_value = self.get_rule(proto_type=proto_type).to_proto(value)

# Sanity check: If we are in strict mode, did we get the value we want?
if strict and not isinstance(pb_value, proto_type):
Expand Down
32 changes: 32 additions & 0 deletions tests/test_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,38 @@ class Foo(proto.Message):
del sys.modules[__name__].__protobuf__


def test_module_package_cross_api():
sys.modules[__name__].__protobuf__ = proto.module(package="spam.eggs.v1")
try:

class Baz(proto.Message):
foo = proto.RepeatedField(proto.INT64, number=1)

marshal = proto.Marshal(name="spam.eggs.v1")

assert Baz.meta.package == "spam.eggs.v1"
assert Baz.pb() in marshal._rules

sys.modules[__name__].__protobuf__ = proto.module(package="ham.pancakes.v1")

class AnotherMessage(proto.Message):
qux = proto.Field(proto.MESSAGE, number=1, message=Baz)

marshal = proto.Marshal(name="ham.pancakes.v1")

assert AnotherMessage.meta.package == "ham.pancakes.v1"
assert AnotherMessage.pb() in marshal._rules
# Confirm that Baz.pb() is no longer present in marshal._rules
assert Baz.pb() not in marshal._rules

# Test using multiple packages together
# See https://github.com/googleapis/proto-plus-python/issues/349.
msg = AnotherMessage(qux=Baz())
assert type(msg) == AnotherMessage
finally:
del sys.modules[__name__].__protobuf__


def test_module_package_explicit_marshal():
sys.modules[__name__].__protobuf__ = proto.module(
package="spam.eggs.v1",
Expand Down

0 comments on commit 0dcea18

Please sign in to comment.