Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make_executable_schema's omits default enum values in args and inputs #293

Closed
Tracked by #479
rafalp opened this issue Jan 10, 2020 · 14 comments · Fixed by #533
Closed
Tracked by #479

make_executable_schema's omits default enum values in args and inputs #293

rafalp opened this issue Jan 10, 2020 · 14 comments · Fixed by #533
Assignees
Labels
bug Something isn't working roadmap Feature that we want to have included

Comments

@rafalp
Copy link
Contributor

rafalp commented Jan 10, 2020

GraphQL enums are represented in Python via the GraphQLEnumType. If those types are created from the SDL, they define no Python representation for GraphQL enum items. Developers can use Ariadne-provided EnumType to specify how enum items should be represented in Python themselves, but we also include "fallback" step that fills enum values with their str representations.

However neither of those approaches takes care of enum items defined as default values for field arguments. Eg:

type Query {
    users(role: UserRole = ADMIN): [User]
}

input FooBarInput {
    foo: FooEnum! = FOO
}

We will need to add additional step to make_executable_schema that gathers all enums from schema together with their Python representations, and then introspects schema's fields values for places where field or input has default value type of enum. If this is the case, we should set correct default_value on GraphQLArgument in the schema.

@rafalp rafalp added the bug Something isn't working label Jan 10, 2020
@rafalp rafalp changed the title make_executable_schema's omits default enum values in fields args make_executable_schema's omits default enum values in args and inputs Feb 16, 2020
@Dodobibi
Copy link

Is there a workaround for this ?

@rafalp rafalp added this to the Ariadne 0.12 milestone Apr 3, 2020
@rafalp rafalp modified the milestones: Ariadne 0.12, Ariadne 0.13 Jun 16, 2020
@dkbarn
Copy link
Contributor

dkbarn commented Nov 20, 2020

Any update on this? The issue was created quite a while ago and feels like a pretty common use case. I've only been playing with ariadne for a couple days and hit the bug immediately.

@rafalp
Copy link
Contributor Author

rafalp commented Nov 20, 2020

PR's welcome :)

In the meantime workaround for this issue is to define default value in field's resolver, eg:

def resolve(_, info, *, role=Roles.ADMIN)

@dkbarn
Copy link
Contributor

dkbarn commented Jan 12, 2021

It's unclear to me why this is not already done by graphql.parse()
This is the function that interprets a raw schema string and converts it into python objects like GraphQLEnumType and GraphQLArgument, correct?
Why isn't the population of GraphQLArgument.default_value happening as part of that step?

@rafalp
Copy link
Contributor Author

rafalp commented Jan 12, 2021

graphql.parse() doesn't known about custom enum bindings defined in ariadne.EnumType.

@dkbarn
Copy link
Contributor

dkbarn commented Jan 12, 2021

The use of custom ariadne.EnumType bindings is optional, I thought. According to https://ariadnegraphql.org/docs/enums#mapping-to-internal-values - the default behaviour is for enums to be represented as strings. So if someone is not making use of custom enum bindings, can graphql.parse() take care of it?

@rafalp
Copy link
Contributor Author

rafalp commented Jan 12, 2021

That may be the case, but please remember that this issue is mainly about EnumType that Ariadne provides.

@rafalp rafalp mentioned this issue Mar 1, 2021
6 tasks
@rafalp
Copy link
Contributor Author

rafalp commented Mar 2, 2021

This is not the case as long as EnumType is not used. For this schema:

enum Role {
  ADMIN
  USER
}

type Query {
  hello(r: Role = USER): String
}

And this query:

{
  default: hello
  custom: hello(r: ADMIN)
}

Resolver that returns repr(r) gives this result:

{
  "default": "'USER'",
  "custom": "'ADMIN'"
}

However as soon as EnumType is used, python types stop working out:

{
  "default": "'USER'",
  "custom": "<Role.ADMIN: 'ADMIN'>"
}

@rafalp
Copy link
Contributor Author

rafalp commented Mar 2, 2021

Turns out there's also a gotcha with Schema validation in GraphQL core which skips default values validation against enum definition:

graphql-python/graphql-core#122

Without this its going to be super troublesome to offer useful error messages. I'll wait now to see what response to above issue will be given. If there's no, we may have to implement schema validator on our own for this case.

@rafalp rafalp self-assigned this Mar 3, 2021
@rafalp rafalp added the roadmap Feature that we want to have included label Mar 3, 2021
@rafalp rafalp removed this from the Ariadne 0.13 milestone Mar 4, 2021
@rafalp rafalp assigned kuchichan and unassigned rafalp Apr 6, 2021
@dkbarn
Copy link
Contributor

dkbarn commented Apr 6, 2021

I don't know if this is the right place to discuss this issue (it seems related to enums and default values):

There seems to be a regression in ariadne 0.13.0. Or at least an unexpected change in behaviour when I upgraded from 0.12.0.

If you run the following:

import ariadne
import enum
import pprint

type_defs = """
    enum Role {
        ADMIN
        USER
    }

    type Query {
        hello(r: Role = USER): String
    }
"""

class Role(enum.Enum):
    ADMIN = "admin"
    USER = "user"

RoleGraphQLType = ariadne.EnumType("Role", Role)

QueryGraphQLType = ariadne.QueryType()

schema = ariadne.make_executable_schema(
    type_defs,
    QueryGraphQLType,
    RoleGraphQLType,
)

query = "{__schema{types{name,fields{name,args{name,defaultValue}}}}}"

result = ariadne.graphql_sync(schema, {"query": query}, debug=True)
pprint.pprint(result)

This works fine under the environment:

ariadne 0.12.0
graphql-core 3.0.5

But when run under the environment:

ariadne 0.13.0
graphql-core 3.1.3

It produces the error:

Traceback (most recent call last):
  File "venv/lib/python3.8/site-packages/graphql/execution/execute.py", line 678, in complete_value_catching_error
    completed = self.complete_value(
  File "venv/lib/python3.8/site-packages/graphql/execution/execute.py", line 745, in complete_value
    raise result
  File "venv/lib/python3.8/site-packages/graphql/execution/execute.py", line 637, in resolve_field_value_or_error
    result = resolve_fn(source, info, **args)
  File "venv/lib/python3.8/site-packages/graphql/type/introspection.py", line 399, in default_value
    value_ast = ast_from_value(item[1].default_value, item[1].type)
  File "venv/lib/python3.8/site-packages/graphql/utilities/ast_from_value.py", line 108, in ast_from_value
    serialized = type_.serialize(value)  # type: ignore
  File "venv/lib/python3.8/site-packages/graphql/type/definition.py", line 1146, in serialize
    raise GraphQLError(
graphql.error.graphql_error.GraphQLError: Enum 'Role' cannot represent value: 'USER'

@bnaoki
Copy link

bnaoki commented Apr 12, 2021

Perhaps this is obvious but, to add on to @dkbarn's previous comment, as it stands the default introspection query used by tools like GraphiQL and GraphQL Playground do not work. This results in the documentation tab and the auto-fill being broken in those tools.

@rafalp
Copy link
Contributor Author

rafalp commented Apr 13, 2021

@dkbarn that looks like a bug all-right, but it should be separate issue on our tracker. Can you post it as separate bug? Thanks!

@dkbarn
Copy link
Contributor

dkbarn commented Apr 16, 2021

Sure thing, separate issue created here: #547

@jhuitema
Copy link

Has there been any movement on the schema validation solution/workaround that would unblock this issue? My team is working on a project that is currently using workarounds for this and we'd like to remove them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working roadmap Feature that we want to have included
Projects
None yet
6 participants