Visitar URL original
fix: modules referenced in MapField message type are properly aliased by software-dov · Pull Request #654 · googleapis/gapic-generator-python · GitHub
Skip to content
This repository was archived by the owner on Aug 25, 2026. It is now read-only.

fix: modules referenced in MapField message type are properly aliased #654

Merged
software-dov merged 1 commit into from
Oct 13, 2020
Merged

fix: modules referenced in MapField message type are properly aliased #654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions gapic/schema/metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,20 @@ def __str__(self) -> str:
# Return the Python identifier.
return '.'.join(self.parent + (self.name,))

def __repr__(self) -> str:
return "({})".format(
", ".join(
(
self.name,
self.module,
str(self.module_path),
str(self.package),
str(self.parent),
str(self.api_naming),
)
)
)

@property
def module_alias(self) -> str:
"""Return an appropriate module alias if necessary.
Expand Down
26 changes: 21 additions & 5 deletions gapic/schema/wrappers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,12 @@ def type(self) -> Union['MessageType', 'EnumType', 'PrimitiveType']:
raise TypeError(f'Unrecognized protobuf type: {self.field_pb.type}. '
'This code should not be reachable; please file a bug.')

def with_context(self, *, collisions: FrozenSet[str]) -> 'Field':
def with_context(
self,
*,
collisions: FrozenSet[str],
visited_messages: FrozenSet["MessageType"],
) -> 'Field':
Comment thread
busunkim96 marked this conversation as resolved.
Resolved
"""Return a derivative of this field with the provided context.

This method is used to address naming collisions. The returned
Expand All @@ -233,7 +238,8 @@ def with_context(self, *, collisions: FrozenSet[str]) -> 'Field':
self,
message=self.message.with_context(
collisions=collisions,
skip_fields=True,
skip_fields=self.message in visited_messages,
visited_messages=visited_messages,
) if self.message else None,
enum=self.enum.with_context(collisions=collisions)
if self.enum else None,
Expand Down Expand Up @@ -406,7 +412,10 @@ def get_field(self, *field_path: str,

# Base case: If this is the last field in the path, return it outright.
if len(field_path) == 1:
return cursor.with_context(collisions=collisions)
return cursor.with_context(
collisions=collisions,
visited_messages=frozenset({self}),
)

# Sanity check: If cursor is a repeated field, then raise an exception.
# Repeated fields are only permitted in the terminal position.
Expand All @@ -433,6 +442,7 @@ def get_field(self, *field_path: str,
def with_context(self, *,
collisions: FrozenSet[str],
skip_fields: bool = False,
visited_messages: FrozenSet["MessageType"] = frozenset(),
) -> 'MessageType':
"""Return a derivative of this message with the provided context.

Expand All @@ -444,10 +454,14 @@ def with_context(self, *,
underlying fields. This provides for an "exit" in the case of circular
references.
"""
visited_messages = visited_messages | {self}
Comment thread
busunkim96 marked this conversation as resolved.
Resolved
return dataclasses.replace(
self,
fields=collections.OrderedDict(
(k, v.with_context(collisions=collisions))
(k, v.with_context(
collisions=collisions,
visited_messages=visited_messages
))
for k, v in self.fields.items()
) if not skip_fields else self.fields,
nested_enums=collections.OrderedDict(
Expand All @@ -457,7 +471,9 @@ def with_context(self, *,
nested_messages=collections.OrderedDict(
(k, v.with_context(
collisions=collisions,
skip_fields=skip_fields,))
skip_fields=skip_fields,
visited_messages=visited_messages,
))
for k, v in self.nested_messages.items()),
meta=self.meta.with_context(collisions=collisions),
)
Expand Down
73 changes: 73 additions & 0 deletions tests/unit/schema/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1214,3 +1214,76 @@ def test_resources_referenced_but_not_typed(reference_attr="type"):

def test_resources_referenced_but_not_typed_child_type():
test_resources_referenced_but_not_typed("child_type")


def test_map_field_name_disambiguation():
squid_file_pb = descriptor_pb2.FileDescriptorProto(
name="mollusc.proto",
package="animalia.mollusca.v2",
message_type=(
descriptor_pb2.DescriptorProto(
name="Mollusc",
),
),
)
method_types_file_pb = descriptor_pb2.FileDescriptorProto(
name="mollusc_service.proto",
package="animalia.mollusca.v2",
message_type=(
descriptor_pb2.DescriptorProto(
name="CreateMolluscRequest",
field=(
descriptor_pb2.FieldDescriptorProto(
name="mollusc",
type="TYPE_MESSAGE",
type_name=".animalia.mollusca.v2.Mollusc",
number=1,
),
descriptor_pb2.FieldDescriptorProto(
name="molluscs_map",
type="TYPE_MESSAGE",
number=2,
type_name=".animalia.mollusca.v2.CreateMolluscRequest.MolluscsMapEntry",
label="LABEL_REPEATED",
),
),
nested_type=(
descriptor_pb2.DescriptorProto(
name="MolluscsMapEntry",
field=(
descriptor_pb2.FieldDescriptorProto(
name="key",
type="TYPE_STRING",
number=1,
),
descriptor_pb2.FieldDescriptorProto(
name="value",
type="TYPE_MESSAGE",
number=2,
# We use the same type for the map value as for
# the singleton above to better highlight the
# problem raised in
# https://github.com/googleapis/gapic-generator-python/issues/618.
# The module _is_ disambiguated for singleton
# fields but NOT for map fields.
type_name=".animalia.mollusca.v2.Mollusc"
),
),
options=descriptor_pb2.MessageOptions(map_entry=True),
),
),
),
),
)
my_api = api.API.build(
file_descriptors=[squid_file_pb, method_types_file_pb],
)
create = my_api.messages['animalia.mollusca.v2.CreateMolluscRequest']
mollusc = create.fields['mollusc']
molluscs_map = create.fields['molluscs_map']
mollusc_ident = str(mollusc.type.ident)
mollusc_map_ident = str(molluscs_map.message.fields['value'].type.ident)

# The same module used in the same place should have the same import alias.
# Because there's a "mollusc" name used, the import should be disambiguated.
assert mollusc_ident == mollusc_map_ident == "am_mollusc.Mollusc"