Repository: polygraph-python/polygraph

Issue:
`Union(Union(String, Int), Float)` should be the same as `Union(String, Int, Float)`

Discussion:

Patch:
diff --git a/polygraph/types/api.py b/polygraph/types/api.py
new file mode 100644
index 0000000..8ffc397
--- /dev/null
+++ b/polygraph/types/api.py
@@ -0,0 +1,2 @@
+def typedef(type_):
+    return type_.__type
diff --git a/polygraph/types/basic_type.py b/polygraph/types/basic_type.py
index 26e54b8..6cbce15 100644
--- a/polygraph/types/basic_type.py
+++ b/polygraph/types/basic_type.py
@@ -1,30 +1,9 @@
-from functools import wraps
-
-from polygraph.exceptions import PolygraphSchemaError, PolygraphValueError
+from polygraph.exceptions import PolygraphValueError
+from polygraph.types.api import typedef
 from polygraph.types.definitions import TypeDefinition, TypeKind
 from polygraph.utils.trim_docstring import trim_docstring
 
 
-def typedef(type_):
-    return type_.__type
-
-
-type_builder_registry = {}
-
-
-def type_builder_cache(method):
-    @wraps(method)
-    def wrapper(cls, *args):
-        unique_args = frozenset(args)
-        if (cls, unique_args) in type_builder_registry:
-            return type_builder_registry[(cls, unique_args)]
-        else:
-            return_val = method(cls, *args)
-            type_builder_registry[(cls, unique_args)] = return_val
-            return return_val
-    return wrapper
-
-
 class PolygraphTypeMeta(type):
     def __new__(cls, name, bases, namespace):
         default_description = trim_docstring(namespace.get("__doc__", ""))
@@ -62,6 +41,7 @@ class PolygraphTypeMeta(type):
 
         > x = String | Int
         """
+        from polygraph.types.type_builder import Union
         return Union(self, other)
 
 
@@ -107,103 +87,6 @@ class Interface(PolygraphOutputType, PolygraphType):
         kind = TypeKind.INTERFACE
 
 
-class Union(PolygraphOutputType, PolygraphType):
-    """
-    GraphQL Unions represent an object that could be one of a list of
-    GraphQL Object types, but provides for no guaranteed fields between
-    those types.
-    """
-    @type_builder_cache
-    def __new__(cls, *types):
-        types = set(types)
-        assert len(types) >= 2, "Unions must consist of more than 1 type"
-        bad_types = [t for t in types if not issubclass(t, PolygraphType)]
-        if bad_types:
-            message = "All types must be subclasses of PolygraphType. Invalid values: "\
-                      "{}".format(", ".join(bad_types))
-            raise PolygraphSchemaError(message)
-        type_names = [typedef(t).name for t in types]
-
-        def __new_from_value__(cls, value):
-            if not any(isinstance(value, t) for t in types):
-                valid_types = ", ".join(type_names)
-                message = "{} is an invalid value type. "\
-                          "Valid types: {}".format(type(value), valid_types)
-                raise PolygraphValueError(message)
-            return value
-
-        class Type:
-            name = "|".join(type_names)
-            description = "One of {}".format(", ".join(type_names))
-            possible_types = types
-            kind = TypeKind.UNION
-
-        name = "Union__" + "_".join(type_names)
-        bases = (Union, )
-        attrs = {"__new__": __new_from_value__}
-        return type(name, bases, attrs)
-
-
-class List(PolygraphType):
-    """
-    A GraphQL list is a special collection type which declares the
-    type of each item in the List (referred to as the item type of
-    the list).
-
-    List values are serialized as ordered lists, where
-    each item in the list is serialized as per the item type.
-    """
-
-    @type_builder_cache
-    def __new__(cls, type_):
-        type_name = typedef(type_).name
-
-        def __new_from_value__(cls, value):
-            if value is None:
-                return None
-            ret_val = [type_(v) for v in value]
-            return list.__new__(cls, ret_val)
-
-        class Type:
-            name = "[{}]".format(type_name)
-            description = "A list of {}".format(type_name)
-            kind = TypeKind.LIST
-            of_type = type_
-
-        name = "List__" + type_name
-        bases = (List, list)
-        attrs = {"__new__": __new_from_value__, "Type": Type}
-        return type(name, bases, attrs)
-
-
-class NonNull(PolygraphType):
-    """
-    Represents a type for which null is not a valid result.
-    """
-    @type_builder_cache
-    def __new__(cls, type_):
-        type_name = typedef(type_).name
-
-        if issubclass(type, NonNull):
-            raise TypeError("NonNull cannot modify NonNull types")
-
-        class Type:
-            name = type_name + "!"
-            description = "A non-nullable version of {}".format(type_name)
-            kind = TypeKind.NON_NULL
-            of_type = type_
-
-        def __new_from_value__(cls, value):
-            if value is None:
-                raise PolygraphValueError("Non-nullable value cannot be None")
-            return type_.__new__(cls, value)
-
-        name = "NonNull__" + type_name
-        bases = (NonNull, type_, )
-        attrs = {"__new__": __new_from_value__, "Type": Type}
-        return type(name, bases, attrs)
-
-
 class InputObject(PolygraphInputType, PolygraphType):
     """
     An Input Object defines a set of input fields; the input fields
diff --git a/polygraph/types/type_builder.py b/polygraph/types/type_builder.py
new file mode 100644
index 0000000..943b40d
--- /dev/null
+++ b/polygraph/types/type_builder.py
@@ -0,0 +1,137 @@
+from functools import wraps
+
+from polygraph.exceptions import PolygraphSchemaError, PolygraphValueError
+from polygraph.types.api import typedef
+from polygraph.types.basic_type import PolygraphOutputType, PolygraphType
+from polygraph.types.definitions import TypeKind
+from polygraph.utils.deduplicate import deduplicate
+
+
+type_builder_registry = {}
+
+
+def flatten(iterable):
+    for arg in iterable:
+        if issubclass(arg, Union):
+            yield from flatten(arg.__type.possible_types)
+        else:
+            yield arg
+
+
+def deduplicate_union_args(method):
+    @wraps(method)
+    def wrapper(cls, *types):
+        types = list(deduplicate(flatten(types)))
+        return method(cls, *types)
+    return wrapper
+
+
+def type_builder_cache(method):
+    @wraps(method)
+    def wrapper(cls, *args):
+        unique_args = frozenset(args)
+        if (cls, unique_args) in type_builder_registry:
+            return type_builder_registry[(cls, unique_args)]
+        else:
+            return_val = method(cls, *args)
+            type_builder_registry[(cls, unique_args)] = return_val
+            return return_val
+    return wrapper
+
+
+class Union(PolygraphOutputType, PolygraphType):
+    """
+    GraphQL Unions represent an object that could be one of a list of
+    GraphQL Object types, but provides for no guaranteed fields between
+    those types.
+    """
+
+    @deduplicate_union_args
+    @type_builder_cache
+    def __new__(cls, *types):
+        assert len(types) >= 2, "Unions must consist of more than 1 type"
+        bad_types = [t for t in types if not issubclass(t, PolygraphType)]
+        if bad_types:
+            message = "All types must be subclasses of PolygraphType. Invalid values: "\
+                      "{}".format(", ".join(bad_types))
+            raise PolygraphSchemaError(message)
+        type_names = [typedef(t).name for t in types]
+
+        def __new_from_value__(cls, value):
+            if not any(isinstance(value, t) for t in types):
+                valid_types = ", ".join(type_names)
+                message = "{} is an invalid value type. "\
+                          "Valid types: {}".format(type(value), valid_types)
+                raise PolygraphValueError(message)
+            return value
+
+        class Type:
+            name = "|".join(type_names)
+            description = "One of {}".format(", ".join(type_names))
+            possible_types = types
+            kind = TypeKind.UNION
+
+        name = "Union__" + "_".join(type_names)
+        bases = (Union, )
+        attrs = {"__new__": __new_from_value__, "Type": Type}
+        return type(name, bases, attrs)
+
+
+class List(PolygraphType):
+    """
+    A GraphQL list is a special collection type which declares the
+    type of each item in the List (referred to as the item type of
+    the list).
+
+    List values are serialized as ordered lists, where
+    each item in the list is serialized as per the item type.
+    """
+
+    @type_builder_cache
+    def __new__(cls, type_):
+        type_name = typedef(type_).name
+
+        def __new_from_value__(cls, value):
+            if value is None:
+                return None
+            ret_val = [type_(v) for v in value]
+            return list.__new__(cls, ret_val)
+
+        class Type:
+            name = "[{}]".format(type_name)
+            description = "A list of {}".format(type_name)
+            kind = TypeKind.LIST
+            of_type = type_
+
+        name = "List__" + type_name
+        bases = (List, list)
+        attrs = {"__new__": __new_from_value__, "Type": Type}
+        return type(name, bases, attrs)
+
+
+class NonNull(PolygraphType):
+    """
+    Represents a type for which null is not a valid result.
+    """
+    @type_builder_cache
+    def __new__(cls, type_):
+        type_name = typedef(type_).name
+
+        if issubclass(type, NonNull):
+            raise TypeError("NonNull cannot modify NonNull types")
+
+        class Type:
+            name = type_name + "!"
+            description = "A non-nullable version of {}".format(type_name)
+            kind = TypeKind.NON_NULL
+            of_type = type_
+
+        def __new_from_value__(cls, value):
+            if value is None:
+                raise PolygraphValueError("Non-nullable value cannot be None")
+            return type_.__new__(cls, value)
+
+        name = "NonNull__" + type_name
+        bases = (NonNull, type_, )
+        attrs = {"__new__": __new_from_value__, "Type": Type}
+        return type(name, bases, attrs)
diff --git a/polygraph/utils/deduplicate.py b/polygraph/utils/deduplicate.py
new file mode 100644
index 0000000..1554394
--- /dev/null
+++ b/polygraph/utils/deduplicate.py
@@ -0,0 +1,11 @@
+def deduplicate(iterable):
+    """
+    Yields deduplicated values, in original order.
+
+    The values of iterable must be hashable
+    """
+    seen = set()
+    for val in iterable:
+        if val not in seen:
+            seen.add(val)
+            yield val


Test patch:
diff --git a/polygraph/types/tests/test_object_type.py b/polygraph/types/tests/test_object_type.py
index e328e6f..9ffd4d4 100644
--- a/polygraph/types/tests/test_object_type.py
+++ b/polygraph/types/tests/test_object_type.py
@@ -1,10 +1,10 @@
 from unittest import TestCase
 
 from polygraph.exceptions import PolygraphValueError
-from polygraph.types.basic_type import NonNull
 from polygraph.types.decorators import field
 from polygraph.types.object_type import ObjectType
 from polygraph.types.scalar import Int, String
+from polygraph.types.type_builder import NonNull
 
 
 class HelloWorldObject(ObjectType):
diff --git a/polygraph/types/tests/test_scalars.py b/polygraph/types/tests/test_scalars.py
index 8eecd6a..1097619 100644
--- a/polygraph/types/tests/test_scalars.py
+++ b/polygraph/types/tests/test_scalars.py
@@ -1,6 +1,6 @@
 from unittest import TestCase
 
-from polygraph.types.basic_type import typedef
+from polygraph.types.api import typedef
 from polygraph.types.scalar import ID, Boolean, Float, Int, String
 
 
diff --git a/polygraph/types/tests/test_type_definitions.py b/polygraph/types/tests/test_type_definitions.py
new file mode 100644
index 0000000..6e67345
--- /dev/null
+++ b/polygraph/types/tests/test_type_definitions.py
@@ -0,0 +1,27 @@
+from unittest import TestCase
+
+from polygraph.types.api import typedef
+from polygraph.types.scalar import Float, Int, String
+from polygraph.types.type_builder import List, NonNull, Union
+
+
+class TestTypeDefinition(TestCase):
+
+    def test_names_of_scalars(self):
+        type_names = [
+            (String, "String"),
+            (Int, "Int"),
+            (Float, "Float"),
+        ]
+        for type_, name in type_names:
+            self.assertEqual(typedef(type_).name, name)
+
+    def test_names_of_built_types(self):
+        type_names = [
+            (List(String), "[String]"),
+            (Union(Int, String, List(String)), "Int|String|[String]"),
+            (NonNull(Int), "Int!"),
+            (NonNull(List(String)), "[String]!")
+        ]
+        for type_, name in type_names:
+            self.assertEqual(typedef(type_).name, name)
diff --git a/polygraph/types/tests/test_types.py b/polygraph/types/tests/test_types.py
index aa4bd3a..d08d30f 100644
--- a/polygraph/types/tests/test_types.py
+++ b/polygraph/types/tests/test_types.py
@@ -1,8 +1,9 @@
 from unittest import TestCase, skip
 
 from polygraph.exceptions import PolygraphValueError
-from polygraph.types.basic_type import List, NonNull, typedef
+from polygraph.types.api import typedef
 from polygraph.types.scalar import Boolean, Int, String
+from polygraph.types.type_builder import List, NonNull
 from polygraph.utils.trim_docstring import trim_docstring
 
 
diff --git a/polygraph/types/tests/test_union.py b/polygraph/types/tests/test_union.py
index 07abe3d..b04ac5e 100644
--- a/polygraph/types/tests/test_union.py
+++ b/polygraph/types/tests/test_union.py
@@ -1,15 +1,15 @@
 from unittest import TestCase, skip
 
 from polygraph.exceptions import PolygraphValueError
-from polygraph.types.basic_type import Union
+from polygraph.types.type_builder import Union
 from polygraph.types.scalar import Float, Int, String
 
 
-# @skip  # FIXME
 class UnionTypeTest(TestCase):
+
     def test_commutativity(self):
         self.assertEqual(Union(String, Int), Union(Int, String))
-        self.assertEqual(Union(String, Int, Float), Union(Float, String, Int, String))
+        self.assertEqual(Union(String, Int, Float), Union(Float, String, Int))
 
     @skip
     def test_associativity(self):
@@ -24,7 +24,6 @@ class UnionTypeTest(TestCase):
             Union(String, Int),
         )
 
-    @skip
     def test_pipe_operator_with_more_than_two_types(self):
         self.assertEqual(
             String | Int | Float,
diff --git a/polygraph/utils/tests/test_deduplicate.py b/polygraph/utils/tests/test_deduplicate.py
new file mode 100644
index 0000000..5d0761b
--- /dev/null
+++ b/polygraph/utils/tests/test_deduplicate.py
@@ -0,0 +1,11 @@
+from polygraph.utils.deduplicate import deduplicate
+from unittest import TestCase
+
+
+class DeduplicateTest(TestCase):
+    def test_deduplicate(self):
+        args = ['d', 'e', 'd', 'u', 'p', 'l', 'i', 'c', 'a', 't', 'e']
+        self.assertEqual(
+            list(deduplicate(args)),
+            ['d', 'e', 'u', 'p', 'l', 'i', 'c', 'a', 't'],
+        )