Skip to content

Commit 05473f5

Browse files
authored
Use other fallback coders for protobuf message base class (#33432)
* Use other fallback coders for protobuf Message base class. * Minor change on comments * Revise the comments based on review. * Move import out of if condition. * Fix lints
1 parent 4a32482 commit 05473f5

File tree

2 files changed

+28
-3
lines changed

2 files changed

+28
-3
lines changed

sdks/python/apache_beam/coders/coders.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555

5656
import google.protobuf.wrappers_pb2
5757
import proto
58+
from google.protobuf import message
5859

5960
from apache_beam.coders import coder_impl
6061
from apache_beam.coders.avro_record import AvroRecord
@@ -65,7 +66,6 @@
6566
from apache_beam.utils import proto_utils
6667

6768
if TYPE_CHECKING:
68-
from google.protobuf import message # pylint: disable=ungrouped-imports
6969
from apache_beam.coders.typecoders import CoderRegistry
7070
from apache_beam.runners.pipeline_context import PipelineContext
7171

@@ -1039,11 +1039,18 @@ def __hash__(self):
10391039

10401040
@classmethod
10411041
def from_type_hint(cls, typehint, unused_registry):
1042-
if issubclass(typehint, proto_utils.message_types):
1042+
# The typehint must be a strict subclass of google.protobuf.message.Message.
1043+
# ProtoCoder cannot work with message.Message itself, as deserialization of
1044+
# a serialized proto requires knowledge of the desired concrete proto
1045+
# subclass which is not stored in the encoded bytes themselves. If this
1046+
# occurs, an error is raised and the system defaults to other fallback
1047+
# coders.
1048+
if (issubclass(typehint, proto_utils.message_types) and
1049+
typehint != message.Message):
10431050
return cls(typehint)
10441051
else:
10451052
raise ValueError((
1046-
'Expected a subclass of google.protobuf.message.Message'
1053+
'Expected a strict subclass of google.protobuf.message.Message'
10471054
', but got a %s' % typehint))
10481055

10491056
def to_type_hint(self):

sdks/python/apache_beam/coders/coders_test.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import proto
2424
import pytest
25+
from google.protobuf import message
2526

2627
import apache_beam as beam
2728
from apache_beam import typehints
@@ -86,6 +87,23 @@ def test_proto_coder(self):
8687
self.assertEqual(ma, real_coder.decode(real_coder.encode(ma)))
8788
self.assertEqual(ma.__class__, real_coder.to_type_hint())
8889

90+
def test_proto_coder_on_protobuf_message_subclasses(self):
91+
# This replicates a scenario where users provide message.Message as the
92+
# output typehint for a Map function, even though the actual output messages
93+
# are subclasses of message.Message.
94+
ma = test_message.MessageA()
95+
mb = ma.field2.add()
96+
mb.field1 = True
97+
ma.field1 = 'hello world'
98+
99+
coder = coders_registry.get_coder(message.Message)
100+
# For messages of google.protobuf.message.Message, the fallback coder will
101+
# be FastPrimitivesCoder rather than ProtoCoder.
102+
# See the comment on ProtoCoder.from_type_hint() for further details.
103+
self.assertEqual(coder, coders.FastPrimitivesCoder())
104+
105+
self.assertEqual(ma, coder.decode(coder.encode(ma)))
106+
89107

90108
class DeterministicProtoCoderTest(unittest.TestCase):
91109
def test_deterministic_proto_coder(self):

0 commit comments

Comments
 (0)