Skip to content
This repository was archived by the owner on Feb 6, 2025. It is now read-only.

Commit 3f1afb1

Browse files
committed
Permit field renaming for interface implementations
This depends on some other code being merged into main, such as the code for interface implementation suppression in PR #1002, so this is just a rough draft at the moment and just contains the rough outline for what this code would look like.
1 parent c346068 commit 3f1afb1

File tree

2 files changed

+122
-30
lines changed

2 files changed

+122
-30
lines changed

graphql_compiler/schema_transformation/rename_schema.py

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@
149149
get_query_type_name,
150150
is_valid_nonreserved_name,
151151
)
152-
152+
from ..compiler.helpers import get_only_element_from_collection
153153

154154
RenamedSchemaDescriptor = namedtuple(
155155
"RenamedSchemaDescriptor",
@@ -451,20 +451,51 @@ def _rename_and_suppress_types_and_fields(
451451
]
452452
raise InvalidNameError("\n".join([i for i in error_message_components if i is not None]))
453453
if (
454-
visitor.type_name_conflicts
455-
or visitor.type_renamed_to_builtin_scalar_conflicts
456-
or visitor.field_name_conflicts
454+
visitor.type_name_conflicts
455+
or visitor.type_renamed_to_builtin_scalar_conflicts
456+
or visitor.field_name_conflicts
457457
):
458458
raise SchemaRenameNameConflictError(
459459
visitor.type_name_conflicts,
460460
visitor.type_renamed_to_builtin_scalar_conflicts,
461461
visitor.field_name_conflicts,
462462
)
463-
if visitor.types_involving_interfaces_with_field_renamings:
463+
if visitor.interfaces_with_field_renamings:
464464
raise NotImplementedError(
465-
f"Field renaming for interfaces or types implementing interfaces is not supported, but "
466-
f"they exist for the following types and should be removed: "
467-
f"{visitor.types_involving_interfaces_with_field_renamings}"
465+
f"Field renaming for interface types is not supported yet, but they exist for the "
466+
f"following types and should be removed: "
467+
f"{visitor.interfaces_with_field_renamings}"
468+
)
469+
470+
# Check that, for all field renaming for types that implement interfaces, the field renamings don't affect any fields included in the interfaces implemented by those types.
471+
# TODO: A variant of this mapping also exists in the code for interface implementation suppression-- will need to adjust this code once that PR gets merged.
472+
interface_and_object_name_to_definition_node_map = {
473+
node.name.value: node
474+
for node in schema_ast.definitions
475+
if isinstance(node, (InterfaceTypeDefinitionNode, ObjectTypeDefinitionNode))
476+
}
477+
# A more convenient way to find out whether any interface contains this particular field or not. The existing InterfaceTypeDefinitionNode contains a list of FieldDefinitionNodes which we'd have to perform a linear search through every single time we want to check for field membership
478+
field_name_to_interface_name_map = {}
479+
for node in schema_ast.definitions:
480+
if not isinstance(node, InterfaceTypeDefinitionNode):
481+
continue
482+
for field_node in node.fields:
483+
field_name = field_node.name.value
484+
field_name_to_interface_name_map.setdefault(field_name, set()).add(node.name.value)
485+
# If any fields belonging to a type named "Foo" that were renamed also appear in an interface, interface_fields_renamed will map the "Foo" to a dict which maps the interface name to the field name.
486+
interface_fields_renamed: Dict[str, Dict[str, str]] = {}
487+
for type_name, fields_renamed in visitor.interface_implementations_field_renamings.items():
488+
type_node = interface_and_object_name_to_definition_node_map[type_name]
489+
interfaces_implemented_by_type_node = {interface_node.name.value for interface_node in type_node.interfaces}
490+
for field_name in fields_renamed:
491+
interfaces_with_field_name = field_name_to_interface_name_map.get(field_name, set())
492+
interface_intersection = interfaces_with_field_name.intersection(interfaces_implemented_by_type_node)
493+
if interface_intersection:
494+
conflicting_interface_name = get_only_element_from_collection(interface_intersection)
495+
interface_fields_renamed.setdefault(type_name, {})[conflicting_interface_name] = field_name
496+
if interface_fields_renamed:
497+
raise NotImplementedError(
498+
f"Fields can be renamed in object types only if those fields do not appear in any interfaces that the object type implements. However, field_renamings contained renamings that violate this rule. The following maps object type names to a dictionary, which maps the interface name containing the field name to the field name, if the field appears in both the object type and the interface: {interface_fields_renamed}. To fix this, remove these renamings from field_renamings."
468499
)
469500
if visitor.illegal_builtin_scalar_renamings:
470501
raise NotImplementedError(
@@ -488,12 +519,12 @@ def _rename_and_suppress_types_and_fields(
488519
# nonexistent_types_with_field_renamings is the set of all object type names that aren't in the
489520
# original schema but appeared in field_renamings anyways.
490521
nonexistent_types_with_field_renamings = (
491-
set(field_renamings) - visitor.types_with_field_renamings_processed
522+
set(field_renamings) - visitor.types_with_field_renamings_processed
492523
)
493524
if (
494-
no_op_type_renames
495-
or visitor.no_op_field_renamings
496-
or nonexistent_types_with_field_renamings
525+
no_op_type_renames
526+
or visitor.no_op_field_renamings
527+
or nonexistent_types_with_field_renamings
497528
):
498529
raise NoOpRenamingError(
499530
no_op_type_renames,
@@ -678,11 +709,16 @@ class RenameSchemaTypesVisitor(Visitor):
678709
# schema that would be renamed to "foo".
679710
field_name_conflicts: Dict[str, Dict[str, Set[str]]]
680711

681-
# Collects names of types who have entries in field_renamings if the type is an interface
682-
# or if the type is an object type implementing an interface because field renamings involving
683-
# interfaces haven't been implemented yet. If field renamings has field renamings for such a
684-
# type named "Foo", types_involving_interfaces_with_field_renamings will contain "Foo".
685-
types_involving_interfaces_with_field_renamings: Set[str]
712+
# Collects names of interfaces who have entries in field_renamings. If field renamings has field
713+
# renamings for an interface named "Foo", interfaces_with_field_renamings will contain "Foo".
714+
interfaces_with_field_renamings: Set[str]
715+
716+
# Maps types that implement interfaces to their renamed fields. This will be used to ensure that
717+
# the only fields that can be renamed for that type are fields that don't appear in the
718+
# interfaces it implements, avoiding many tricky edge cases for now. If a type named "Foo" has a
719+
# field named "bar" that gets renamed, interface_implementations_field_renamings will map "Foo"
720+
# to a set containing "bar".
721+
interface_implementations_field_renamings: Dict[str, Set[str]]
686722

687723
def __init__(
688724
self,
@@ -719,7 +755,8 @@ def __init__(
719755
self.types_with_field_renamings_processed = set()
720756
self.invalid_field_names = {}
721757
self.field_name_conflicts = {}
722-
self.types_involving_interfaces_with_field_renamings = set()
758+
self.interfaces_with_field_renamings = set()
759+
self.interface_implementations_field_renamings = {}
723760

724761
def _rename_or_suppress_or_ignore_name_and_add_to_record(
725762
self, node: RenameTypesT
@@ -782,7 +819,7 @@ def _rename_or_suppress_or_ignore_name_and_add_to_record(
782819
isinstance(fields_renamed_node, InterfaceTypeDefinitionNode)
783820
and fields_renamed_node.name.value in self.field_renamings
784821
):
785-
self.types_involving_interfaces_with_field_renamings.add(fields_renamed_node.name.value)
822+
self.interfaces_with_field_renamings.add(fields_renamed_node.name.value)
786823
self.reverse_name_map[desired_type_name] = type_name
787824
if desired_type_name == type_name:
788825
return fields_renamed_node
@@ -798,7 +835,7 @@ def _rename_fields(self, node: ObjectTypeDefinitionNode) -> ObjectTypeDefinition
798835
if type_name not in self.field_renamings:
799836
return node
800837
if node.interfaces:
801-
self.types_involving_interfaces_with_field_renamings.add(type_name)
838+
self.interface_implementations_field_renamings[type_name] = set(self.field_renamings.get(type_name, {}))
802839
current_type_field_renamings = self.field_renamings[type_name]
803840
self.types_with_field_renamings_processed.add(type_name)
804841
# Need to create a set of field nodes that the type will have after the field renamings,

graphql_compiler/tests/schema_transformation_tests/test_rename_schema.py

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,71 @@ def test_field_renaming_illegal_noop_rename_fields_of_suppressed_type(self) -> N
463463
str(e.exception),
464464
)
465465

466+
def test_field_rename_in_interface_implementation(self) -> None:
467+
# Field renaming is permitted for object types that implement interfaces as long as the
468+
# fields being renamed don't also appear as fields in any interface the object type
469+
# implements.
470+
renamed_schema = rename_schema(parse(ISS.various_types_schema), {}, {"Human": {"name": {"name", "new_name"}}})
471+
renamed_schema_string = dedent(
472+
"""\
473+
schema {
474+
query: SchemaQuery
475+
}
476+
477+
scalar Date
478+
479+
enum Height {
480+
TALL
481+
SHORT
482+
}
483+
484+
interface Character {
485+
id: String
486+
}
487+
488+
type Human implements Character {
489+
id: String
490+
name: String
491+
new_name: String
492+
birthday: Date
493+
}
494+
495+
type Giraffe implements Character {
496+
id: String
497+
height: Height
498+
}
499+
500+
type Dog {
501+
nickname: String
502+
}
503+
504+
directive @stitch(source_field: String!, sink_field: String!) on FIELD_DEFINITION
505+
506+
type SchemaQuery {
507+
Human: Human
508+
Giraffe: Giraffe
509+
Dog: Dog
510+
}
511+
"""
512+
)
513+
compare_schema_texts_order_independently(
514+
self, renamed_schema_string, print_ast(renamed_schema.schema_ast)
515+
)
516+
self.assertEqual({}, renamed_schema.reverse_name_map)
517+
self.assertEqual({"Human": {"new_name": "name"}}, renamed_schema.reverse_field_name_map)
518+
519+
with self.assertRaises(NotImplementedError):
520+
# Cannot rename Human's fields because Human implements an interface and field_renamings
521+
# for object types that implement interfaces isn't supported yet.
522+
rename_schema(parse(ISS.multiple_interfaces_schema), {}, {"Human": {"id": {"new_id"}}})
523+
with self.assertRaises(NotImplementedError):
524+
rename_schema(
525+
parse(ISS.multiple_interfaces_schema), {}, {"Human": {"id": {"id", "new_id"}}}
526+
)
527+
with self.assertRaises(NotImplementedError):
528+
rename_schema(parse(ISS.multiple_interfaces_schema), {}, {"Human": {"id": set()}})
529+
530+
466531
def test_field_renaming_in_interfaces(self) -> None:
467532
with self.assertRaises(NotImplementedError):
468533
rename_schema(
@@ -474,16 +539,6 @@ def test_field_renaming_in_interfaces(self) -> None:
474539
)
475540
with self.assertRaises(NotImplementedError):
476541
rename_schema(parse(ISS.multiple_interfaces_schema), {}, {"Character": {"id": set()}})
477-
with self.assertRaises(NotImplementedError):
478-
# Cannot rename Human's fields because Human implements an interface and field_renamings
479-
# for object types that implement interfaces isn't supported yet.
480-
rename_schema(parse(ISS.multiple_interfaces_schema), {}, {"Human": {"id": {"new_id"}}})
481-
with self.assertRaises(NotImplementedError):
482-
rename_schema(
483-
parse(ISS.multiple_interfaces_schema), {}, {"Human": {"id": {"id", "new_id"}}}
484-
)
485-
with self.assertRaises(NotImplementedError):
486-
rename_schema(parse(ISS.multiple_interfaces_schema), {}, {"Human": {"id": set()}})
487542

488543
def test_enum_rename(self) -> None:
489544
renamed_schema = rename_schema(

0 commit comments

Comments
 (0)