From 27fc664e27fff1226a29192fc2c3c4b5f9f0e5dc Mon Sep 17 00:00:00 2001 From: Victor Petrovykh Date: Tue, 30 Apr 2024 12:16:11 -0400 Subject: [PATCH] Fix handling of enums in arrays and multi properties. Declaring an enum array property or a multi property should not result in an error in GraphQL. Both of these are valid types and can be inserted and updated. This is a purely GraphQL fix. Fixes #3990 --- edb/graphql/translator.py | 8 +- edb/graphql/types.py | 13 +- tests/schemas/graphql_other.esdl | 2 + tests/test_http_graphql_mutation.py | 240 ++++++++++++++++++++++++++++ 4 files changed, 258 insertions(+), 5 deletions(-) diff --git a/edb/graphql/translator.py b/edb/graphql/translator.py index e6873ffa702..7860394691a 100644 --- a/edb/graphql/translator.py +++ b/edb/graphql/translator.py @@ -1683,12 +1683,14 @@ def visit_VariableNode(self, node): optional = False if self.is_list_type(vartype): - if target.is_array: - raise errors.QueryError(err_msg) - elif target.is_multirange: + if target.is_multirange: subtype = target.edb_base.get_subtypes(target.edb_schema)[0] st_name = subtype.get_name(target.edb_schema) castname = qlast.ObjectRef(name=str(st_name)) + else: + # So far the only list allowed is a multirange + # representation. + raise errors.QueryError(err_msg) elif vartype.name.value in gt.GQL_TO_EDB_SCALARS_MAP: castname = qlast.ObjectRef( diff --git a/edb/graphql/types.py b/edb/graphql/types.py index 5c1a0c1677f..c13545d7308 100644 --- a/edb/graphql/types.py +++ b/edb/graphql/types.py @@ -876,8 +876,17 @@ def get_insert_fields( continue if isinstance(target, GraphQLList): - inobjtype = self._gql_inobjtypes.get( - f'Insert{target.of_type.of_type.name}') + # Check whether the edb_target is an array of enums, + # because enums need slightly different handling. + assert isinstance(edb_target, s_types.Array) + el = edb_target.get_element_type(self.edb_schema) + if el.is_enum(self.edb_schema): + tname = el.get_name(self.edb_schema) + assert isinstance(tname, s_name.QualName) + else: + tname = target.of_type.of_type.name + + inobjtype = self._gql_inobjtypes.get(f'Insert{tname}') assert inobjtype is not None intype = GraphQLList(GraphQLNonNull(inobjtype)) diff --git a/tests/schemas/graphql_other.esdl b/tests/schemas/graphql_other.esdl index 29f22bacc78..d2a5e359ceb 100644 --- a/tests/schemas/graphql_other.esdl +++ b/tests/schemas/graphql_other.esdl @@ -27,6 +27,8 @@ type Foo { property `select` -> str; property after -> str; required property color -> ColorEnum; + multi property multi_color -> ColorEnum; + property color_array -> array; # Testing linking to the same type multi link foos -> Foo { diff --git a/tests/test_http_graphql_mutation.py b/tests/test_http_graphql_mutation.py index 8276ba3d09c..b756a89f150 100644 --- a/tests/test_http_graphql_mutation.py +++ b/tests/test_http_graphql_mutation.py @@ -547,6 +547,124 @@ def test_graphql_mutation_insert_enum_02(self): "other__Foo": [] }) + def test_graphql_mutation_insert_enum_03(self): + # This tests enum values in insertion. + data = { + 'select': 'New EnumTest05', + 'color': 'GREEN', + 'color_array': ['RED', 'GREEN', 'RED'], + } + + validation_query = r""" + query { + other__Foo(filter: {select: {eq: "New EnumTest05"}}) { + select + color + color_array + } + } + """ + + self.assert_graphql_query_result(r""" + mutation insert_other__Foo { + insert_other__Foo( + data: [{ + select: "New EnumTest05", + color: GREEN, + color_array: [RED, GREEN, RED], + }] + ) { + select + color + color_array + } + } + """, { + "insert_other__Foo": [data] + }) + + self.assert_graphql_query_result(validation_query, { + "other__Foo": [data] + }) + + self.assert_graphql_query_result(r""" + mutation delete_other__Foo { + delete_other__Foo( + filter: {select: {eq: "New EnumTest05"}} + ) { + select + color + color_array + } + } + """, { + "delete_other__Foo": [data] + }) + + # validate that the deletion worked + self.assert_graphql_query_result(validation_query, { + "other__Foo": [] + }) + + def test_graphql_mutation_insert_enum_04(self): + # This tests enum values in insertion. + data = { + 'select': 'New EnumTest03', + 'color': 'GREEN', + 'multi_color': {'RED', 'GREEN'}, + } + + validation_query = r""" + query { + other__Foo(filter: {select: {eq: "New EnumTest03"}}) { + select + color + multi_color + } + } + """ + + self.assert_graphql_query_result(r""" + mutation insert_other__Foo { + insert_other__Foo( + data: [{ + select: "New EnumTest03", + color: GREEN, + multi_color: [RED, GREEN], + }] + ) { + select + color + multi_color + } + } + """, { + "insert_other__Foo": [data] + }) + + self.assert_graphql_query_result(validation_query, { + "other__Foo": [data] + }) + + self.assert_graphql_query_result(r""" + mutation delete_other__Foo { + delete_other__Foo( + filter: {select: {eq: "New EnumTest03"}} + ) { + select + color + multi_color + } + } + """, { + "delete_other__Foo": [data] + }) + + # validate that the deletion worked + self.assert_graphql_query_result(validation_query, { + "other__Foo": [] + }) + def test_graphql_mutation_insert_range_01(self): # This tests range and multirange values in insertion. data = { @@ -3213,6 +3331,128 @@ def test_graphql_mutation_update_enum_02(self): }] }) + def test_graphql_mutation_update_enum_03(self): + # This tests enum values in updates. + + self.assert_graphql_query_result(r""" + mutation insert_other__Foo { + insert_other__Foo( + data: [{ + select: "Update EnumTest03", + color: BLUE, + color_array: [RED, BLUE, BLUE] + }] + ) { + select + color + color_array + } + } + """, { + "insert_other__Foo": [{ + 'select': 'Update EnumTest03', + 'color': 'BLUE', + 'color_array': ['RED', 'BLUE', 'BLUE'], + }] + }) + + self.assert_graphql_query_result(r""" + mutation update_other__Foo { + update_other__Foo( + filter: {select: {eq: "Update EnumTest03"}} + data: { + color_array: {append: [GREEN]} + } + ) { + select + color_array + } + } + """, { + "update_other__Foo": [{ + 'select': 'Update EnumTest03', + 'color_array': ['RED', 'BLUE', 'BLUE', 'GREEN'], + }] + }) + + # clean up + self.assert_graphql_query_result(r""" + mutation delete_other__Foo { + delete_other__Foo( + filter: {select: {eq: "Update EnumTest03"}} + ) { + select + color_array + } + } + """, { + "delete_other__Foo": [{ + 'select': 'Update EnumTest03', + 'color_array': ['RED', 'BLUE', 'BLUE', 'GREEN'], + }] + }) + + def test_graphql_mutation_update_enum_04(self): + # This tests enum values in updates. + + self.assert_graphql_query_result(r""" + mutation insert_other__Foo { + insert_other__Foo( + data: [{ + select: "Update EnumTest04", + color: BLUE, + multi_color: [RED, BLUE] + }] + ) { + select + color + multi_color + } + } + """, { + "insert_other__Foo": [{ + 'select': 'Update EnumTest04', + 'color': 'BLUE', + 'multi_color': ['RED', 'BLUE'], + }] + }) + + self.assert_graphql_query_result(r""" + mutation update_other__Foo { + update_other__Foo( + filter: {select: {eq: "Update EnumTest04"}} + data: { + multi_color: {add: GREEN} + } + ) { + select + multi_color + } + } + """, { + "update_other__Foo": [{ + 'select': 'Update EnumTest04', + 'multi_color': {'RED', 'BLUE', 'GREEN'}, + }] + }) + + # clean up + self.assert_graphql_query_result(r""" + mutation delete_other__Foo { + delete_other__Foo( + filter: {select: {eq: "Update EnumTest04"}} + ) { + select + multi_color + } + } + """, { + "delete_other__Foo": [{ + 'select': 'Update EnumTest04', + 'multi_color': {'RED', 'BLUE', 'GREEN'}, + }] + }) + def test_graphql_mutation_update_range_01(self): # This tests range and multirange values in updates. self.assert_graphql_query_result(r"""