Validate macro edge does not exist on sub/superclass

graphql_compiler/macros/__init__.py

@@ -1,5 +1,6 @@
 # Copyright 2019-present Kensho Technologies, LLC.
 from collections import namedtuple
+from itertools import chain
 import six
 
 from graphql.language.ast import (FieldDefinition, Name, NamedType,
@@ -16,7 +17,7 @@
 from .macro_edge.helpers import get_type_at_macro_edge_target
 from .macro_edge.directives import MacroEdgeDirective
 from .macro_expansion import expand_macros_in_query_ast
-from ..exceptions import GraphQLValidationError
+from ..exceptions import GraphQLInvalidMacroError, GraphQLValidationError
 
 
 MacroRegistry = namedtuple(
@@ -74,27 +75,41 @@ def register_macro_edge(macro_registry, macro_edge_graphql, macro_edge_args):
         macro_edge_args: dict mapping strings to any type, containing any arguments the macro edge
                          requires in order to function.
     """
-    class_name, macro_edge_name, macro_descriptor = make_macro_edge_descriptor(
+    new_macro_edge_class_name, macro_edge_name, macro_descriptor = make_macro_edge_descriptor(
         macro_registry.schema_without_macros, macro_edge_graphql, macro_edge_args,
         type_equivalence_hints=macro_registry.type_equivalence_hints)
 
     # Ensure this new macro edge does not conflict with any previous descriptor.
-    macro_edges_for_class = macro_registry.macro_edges.get(class_name, dict())
+    macro_edges_for_class = macro_registry.macro_edges.get(new_macro_edge_class_name, dict())
     existing_descriptor = macro_edges_for_class.get(macro_edge_name, None)
 
     if existing_descriptor is not None:
         raise AssertionError(
             u'Attempting to redefine an already registered macro edge: '
             u'class {}, macro edge {}, new GraphQL descriptor {}, new args {}.'
-            .format(class_name, macro_edge_name, macro_edge_graphql, macro_edge_args))
-
-    # TODO(predrag): Write a more stringent check that makes sure that two types A and B,
-    #                where A is a superclass of B, cannot define the same macro edge.
-    #                Right now, both A and B can independently define a macro edge out_Foo,
-    #                which would result in an illegal schema as B would be required to have
-    #                two different descriptors for the same out_Foo edge.
-
-    macro_registry.macro_edges.setdefault(class_name, dict())[macro_edge_name] = macro_descriptor
+            .format(new_macro_edge_class_name, macro_edge_name, macro_edge_graphql, macro_edge_args))
+
+    # Ensure there's no conflict with macro edges defined on subclasses and superclasses.
+    class_sets_to_check = (
+        ('subclass', macro_registry.subclass_sets[new_macro_edge_class_name]),
+        ('superclass', {
+            class_name
+            for class_name, class_subclasses in six.iteritems(macro_registry.subclass_sets)
+            if new_macro_edge_class_name in class_subclasses
+        }),
+    )
+    for relationship, class_names in class_sets_to_check:
+        for class_name in class_names:
+            macros_on_class = macro_registry.macro_edges.get(class_name, dict())
+            if macro_edge_name in macros_on_class:
+                raise GraphQLInvalidMacroError(
+                    u'A macro edge with name {} already exists on {}, which is'
+                    u'a {} of {}. new GraphQL descriptor {}, new args {}'
+                    .format(macro_edge_name, class_name, relationship,
+                            new_macro_edge_class_name, macro_edge_graphql, macro_edge_args))
+
+    macro_registry.macro_edges.setdefault(
+        new_macro_edge_class_name, dict())[macro_edge_name] = macro_descriptor
 
 
 def get_schema_with_macros(macro_registry):

graphql_compiler/tests/test_macro_validation.py

@@ -416,7 +416,6 @@ def test_macro_edge_duplicate_definition_on_target(self):
         with self.assertRaises(AssertionError):
             register_macro_edge(macro_registry, duplicate_query, args)
 
-    @pytest.mark.skip(reason='not implemented')
     def test_macro_edge_dulpicate_definition_on_subclass(self):
         query = '''{
             Entity @macro_edge_definition(name: "out__RelatedOfRelated") {
@@ -441,13 +440,13 @@ def test_macro_edge_dulpicate_definition_on_subclass(self):
         # Try registering on the superclass first
         macro_registry = get_empty_test_macro_registry()
         register_macro_edge(macro_registry, query, args)
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(GraphQLInvalidMacroError):
             register_macro_edge(macro_registry, query_on_subclass, args)
 
         # Try registering on the subclass first
         macro_registry = get_empty_test_macro_registry()
         register_macro_edge(macro_registry, query_on_subclass, args)
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(GraphQLInvalidMacroError):
             register_macro_edge(macro_registry, query, args)
 
     @pytest.mark.skip(reason='not implemented')