ariadne icon indicating copy to clipboard operation
ariadne copied to clipboard

value_from_ast does not take into account SchemaDirectiveVisitor.visit_input_field_definition

Open vmarkovtsev opened this issue 2 years ago • 0 comments

Given the following spec:

directive @date on INPUT_FIELD_DEFINITION

input MetricParams {
    expiresAt String! @date
}

type Query {
    metrics(params: MetricParams!): Boolean
}

the following directive implementation

class GraphQLDateType(GraphQLString):
    def parse_literal(self, value_node, variables=None):
        # validate the string to match ISO 8601
        pass

class DateDirective(SchemaDirectiveVisitor):
    def visit_input_field_definition(
        self, field: GraphQLInputField, object_type: GraphQLInputObjectType,
    ) -> GraphQLInputField:
        """Validate the date format."""
        orig_type = field.type
        orig_named_type = get_named_type(field.type)
        assert orig_named_type.name == "String"

        field.type = GraphQLDateType
        # re-wrap with GraphQLNonNull if needed
        if isinstance(orig_type, GraphQLNonNull):
            field.type = GraphQLNonNull(field.type)
        return field

and the following query

query ($expiresAt: String!) {
  metrics(params: {
    expiresAt: $expiresAt
  }) {
    ...
  }
}

The field expiresAt does not get validated in parse_literal(). This happens because the value_from_ast() function that builds the resolver's arguments reads:

if isinstance(value_node, VariableNode):
    variable_name = value_node.name.value
    if not variables:
        return Undefined
    variable_value = variables.get(variable_name, Undefined)
    if variable_value is None and is_non_null_type(type_):
        return Undefined
    # Note: This does no further checking that this variable is correct.
    # This assumes that this query has been validated and the variable usage here
    # is of the correct type.
    return variable_value

The comment doesn't look correct: we can override the parse_literal() on the input field and should call it similarly to the if is_leaf_type(type_) branch.

vmarkovtsev avatar May 26 '22 14:05 vmarkovtsev