Skip to content

Commit

Permalink
Support nested definitions in GetMessageClassesForFiles
Browse files Browse the repository at this point in the history
Otherwise downstream callers must explicitly fetch and generate message classes for nested definitions. (For example [here](foxglove/mcap#1321).)

Example:
```
message NestedTypeMessage {
  message NestedType {
      string data = 1;
    }

  NestedType nested = 1;
}
```

Currently `GetMessageClassesForFiles()` will only return `NestedTypeMessage`, but miss `NestedTypeMessage.NestedType` (and any arbitrary level of nested definitions).

This is an unexpected deviation from the comment that says that it will find and resolve all dependencies.

PiperOrigin-RevId: 719854337
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Jan 26, 2025
1 parent ae3015c commit c50d93a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
25 changes: 25 additions & 0 deletions python/google/protobuf/internal/message_factory_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,29 @@ def testExtensionValueInDifferentFile(self):
self.assertEqual(234, m.Extensions[ext1].setting)
self.assertEqual(345, m.Extensions[ext2].setting)

def testNestedDefinition(self):
f = descriptor_pb2.FileDescriptorProto(
name='google/protobuf/internal/meta_class.proto',
package='google.protobuf.python.internal')
msg_proto = f.message_type.add(name='Empty')
msg_proto.nested_type.add(name='Nested')
msg_proto.field.add(name='nested_field',
number=1,
label=descriptor.FieldDescriptor.LABEL_REPEATED,
type=descriptor.FieldDescriptor.TYPE_MESSAGE,
type_name='Nested')

msg_proto.nested_type[0].nested_type.add(name='DoublyNested')
msg_proto.nested_type[0].field.add(name='doubly_nested_field',
number=2,
label=descriptor.FieldDescriptor.LABEL_REPEATED,
type=descriptor.FieldDescriptor.TYPE_MESSAGE,
type_name='DoublyNested')

messages = message_factory.GetMessages([f])
self.assertIn('google.protobuf.python.internal.Empty.Nested', messages)
self.assertIn('google.protobuf.python.internal.Empty.Nested.DoublyNested', messages)

def testDescriptorKeepConcreteClass(self):
def loadFile():
f= descriptor_pb2.FileDescriptorProto(
Expand All @@ -310,6 +333,8 @@ def loadFile():

messages = loadFile()
for des, meta_class in messages.items():
if des == "google.protobuf.python.internal.Empty.Nested":
continue
message = meta_class()
nested_des = message.DESCRIPTOR.nested_types_by_name['Nested']
nested_msg = nested_des._concrete_class()
Expand Down
9 changes: 9 additions & 0 deletions python/google/protobuf/message_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ def GetMessageClassesForFiles(files, pool):
This will find and resolve dependencies, failing if the descriptor
pool cannot satisfy them.
This will also recursively find any nested definitions.
Args:
files: The file names to extract messages from.
pool: The descriptor pool to find the files including the dependent files.
Expand All @@ -69,6 +71,13 @@ def GetMessageClassesForFiles(files, pool):
for desc in file_desc.message_types_by_name.values():
result[desc.full_name] = GetMessageClass(desc)

# Recursively load protos for nested definitions.
nested_descriptions = list(desc.nested_types_by_name.values())
while nested_descriptions:
nested_desc = nested_descriptions.pop()
result[nested_desc.full_name] = GetMessageClass(nested_desc)
nested_descriptions.extend(nested_desc.nested_types_by_name.values())

# While the extension FieldDescriptors are created by the descriptor pool,
# the python classes created in the factory need them to be registered
# explicitly, which is done below.
Expand Down

0 comments on commit c50d93a

Please sign in to comment.