Skip to content

Commit

Permalink
Fix handling of enums in arrays and multi properties.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@vpetrovykh
vpetrovykh committed May 6, 2024
1 parent cf8069f commit 27fc664
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 5 deletions.
8 changes: 5 additions & 3 deletions edb/graphql/translator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 11 additions & 2 deletions edb/graphql/types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
2 changes: 2 additions & 0 deletions tests/schemas/graphql_other.esdl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ColorEnum>;

# Testing linking to the same type
multi link foos -> Foo {
Expand Down
240 changes: 240 additions & 0 deletions tests/test_http_graphql_mutation.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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"""
Expand Down

0 comments on commit 27fc664

Please sign in to comment.