diff --git a/graphql_compiler/compiler/compiler_frontend.py b/graphql_compiler/compiler/compiler_frontend.py index 048c69b85..228aa89ae 100644 --- a/graphql_compiler/compiler/compiler_frontend.py +++ b/graphql_compiler/compiler/compiler_frontend.py @@ -20,8 +20,8 @@ property fields and vertex fields; all property fields must precede the vertex fields for the AST to be valid; - step 1. apply all @filter directives, if present on the current AST node - (see _compile_ast_node_to_ir()) + step 1. apply all @filter directives that apply to the current field + (see _compile_ast_node_to_ir() and directive_helpers.get_local_filter_directives()) We now proceed with one of three cases (P, V and F), depending on whether the current AST node is a property AST node, vertex AST node, or inline fragment, respectively. @@ -36,8 +36,8 @@ (property fields cannot have property fields of their own, see _compile_vertex_ast()). step V-3. Property field processing complete: (see _compile_vertex_ast()) - - mark the current location in the query, since all @filter directives on this AST node - have already been processed; + - mark the current location in the query, since all @filter directives that apply to the + current field have already been processed; - process the output_source directive, if it exists step V-4. Recurse into any vertex field children of the current AST node: @@ -66,8 +66,9 @@ from ..exceptions import GraphQLCompilationError, GraphQLParsingError, GraphQLValidationError from .context_helpers import (has_encountered_output_source, is_in_fold_scope, is_in_optional_scope, validate_context_for_visiting_vertex_field) -from .directive_helpers import (get_directives, validate_property_directives, - validate_root_vertex_directives, validate_vertex_directives, +from .directive_helpers import (get_local_filter_directives, get_unique_directives, + validate_property_directives, validate_root_vertex_directives, + validate_vertex_directives, validate_vertex_field_directive_interactions) from .filters import process_filter_directive from .helpers import (FoldScopeLocation, Location, get_ast_field_name, get_field_type_from_schema, @@ -167,7 +168,7 @@ def _mark_location(location): def _process_output_source_directive(schema, current_schema_type, ast, - location, context, local_directives): + location, context, local_unique_directives): """Process the output_source directive, modifying the context as appropriate. Args: @@ -177,14 +178,14 @@ def _process_output_source_directive(schema, current_schema_type, ast, location: Location object representing the current location in the query context: dict, various per-compilation data (e.g. declared tags, whether the current block is optional, etc.). May be mutated in-place in this function! - local_directives: dict, directive name string -> directive object, containing the - directives present on the current AST node *only* + local_unique_directives: dict, directive name string -> directive object, containing + unique directives present on the current AST node *only* Returns: an OutputSource block, if one should be emitted, or None otherwise """ # The 'ast' variable is only for function signature uniformity, and is currently not used. - output_source_directive = local_directives.get('output_source', None) + output_source_directive = local_unique_directives.get('output_source', None) if output_source_directive: if has_encountered_output_source(context): raise GraphQLCompilationError(u'Cannot have more than one output source!') @@ -196,7 +197,8 @@ def _process_output_source_directive(schema, current_schema_type, ast, return None -def _compile_property_ast(schema, current_schema_type, ast, location, context, local_directives): +def _compile_property_ast(schema, current_schema_type, ast, location, + context, unique_local_directives): """Process property directives at this AST node, updating the query context as appropriate. Args: @@ -207,13 +209,13 @@ def _compile_property_ast(schema, current_schema_type, ast, location, context, l location: Location object representing the current location in the query context: dict, various per-compilation data (e.g. declared tags, whether the current block is optional, etc.). May be mutated in-place in this function! - local_directives: dict, directive name string -> directive object, containing the - directives present on the current AST node *only* + unique_local_directives: dict, directive name string -> directive object, containing + unique directives present on the current AST node *only* """ - validate_property_directives(local_directives) + validate_property_directives(unique_local_directives) # step P-2: process property-only directives - tag_directive = local_directives.get('tag', None) + tag_directive = unique_local_directives.get('tag', None) if tag_directive: if is_in_fold_scope(context): raise GraphQLCompilationError(u'Tagging values within a @fold vertex field is ' @@ -230,7 +232,7 @@ def _compile_property_ast(schema, current_schema_type, ast, location, context, l 'type': strip_non_null_from_type(current_schema_type), } - output_directive = local_directives.get('output', None) + output_directive = unique_local_directives.get('output', None) if output_directive: # Schema validation has ensured that the fields below exist. output_name = output_directive.arguments[0].value.value @@ -304,7 +306,7 @@ def _get_edge_direction_and_name(vertex_field_name): def _compile_vertex_ast(schema, current_schema_type, ast, - location, context, local_directives, fields): + location, context, unique_local_directives, fields): """Return a list of basic blocks corresponding to the vertex AST node. Args: @@ -314,8 +316,8 @@ def _compile_vertex_ast(schema, current_schema_type, ast, location: Location object representing the current location in the query context: dict, various per-compilation data (e.g. declared tags, whether the current block is optional, etc.). May be mutated in-place in this function! - local_directives: dict, directive name string -> directive object, containing the - directives present on the current AST node *only* + unique_local_directives: dict, directive name string -> directive object, containing + unique directives present on the current AST node *only* fields: tuple of lists (property_fields, vertex_fields), with lists of field objects present on the current vertex AST node @@ -325,7 +327,7 @@ def _compile_vertex_ast(schema, current_schema_type, ast, basic_blocks = [] vertex_fields, property_fields = fields - validate_vertex_directives(local_directives) + validate_vertex_directives(unique_local_directives) # step V-2: step into property fields for field_ast in property_fields: @@ -345,7 +347,7 @@ def _compile_vertex_ast(schema, current_schema_type, ast, basic_blocks.append(_mark_location(location)) output_source = _process_output_source_directive(schema, current_schema_type, ast, - location, context, local_directives) + location, context, unique_local_directives) if output_source: basic_blocks.append(output_source) @@ -367,12 +369,12 @@ def _compile_vertex_ast(schema, current_schema_type, ast, edge_schema_type)) field_schema_type = edge_schema_type.of_type - inner_directives = get_directives(field_ast) - validate_vertex_field_directive_interactions(inner_location, inner_directives) + inner_unique_directives = get_unique_directives(field_ast) + validate_vertex_field_directive_interactions(inner_location, inner_unique_directives) - recurse_directive = inner_directives.get('recurse', None) - optional_directive = inner_directives.get('optional', None) - fold_directive = inner_directives.get('fold', None) + recurse_directive = inner_unique_directives.get('recurse', None) + optional_directive = inner_unique_directives.get('optional', None) + fold_directive = inner_unique_directives.get('fold', None) in_topmost_optional_block = False edge_traversal_is_optional = optional_directive is not None @@ -400,7 +402,7 @@ def _compile_vertex_ast(schema, current_schema_type, ast, basic_blocks.append(fold_block) context['fold'] = fold_scope_location elif recurse_directive: - recurse_depth = _get_recurse_directive_depth(field_name, inner_directives) + recurse_depth = _get_recurse_directive_depth(field_name, inner_unique_directives) _validate_recurse_directive_types(current_schema_type, field_schema_type) basic_blocks.append(blocks.Recurse(edge_direction, edge_name, recurse_depth)) else: @@ -513,10 +515,11 @@ def _compile_ast_node_to_ir(schema, current_schema_type, ast, location, context) basic_blocks = [] # step 0: preprocessing - local_directives = get_directives(ast) + local_unique_directives = get_unique_directives(ast) fields = _get_fields(ast) vertex_fields, property_fields = fields fragment = _get_inline_fragment(ast) + local_filters_directives = get_local_filter_directives(ast, vertex_fields) # We don't support type coercion while at the same time selecting fields. # Either there are no fields, or there is no fragment, otherwise we raise a compilation error. @@ -539,17 +542,15 @@ def _compile_ast_node_to_ir(schema, current_schema_type, ast, location, context) u'{} {}'.format(location, property_fields)) # step 1: apply local filter, if any - filter_directives = local_directives.get('filter', None) - if filter_directives: - for filter_directive in filter_directives: - basic_blocks.append( - process_filter_directive(schema, current_schema_type, - ast, context, filter_directive)) + for filter_directive in local_filters_directives: + basic_blocks.append( + process_filter_directive(schema, current_schema_type, + ast, context, filter_directive)) if location.field is not None: # The location is at a property, compile the property data following P-steps. _compile_property_ast(schema, current_schema_type, ast, - location, context, local_directives) + location, context, local_unique_directives) else: # The location is at a vertex. if fragment_exists: @@ -564,7 +565,7 @@ def _compile_ast_node_to_ir(schema, current_schema_type, ast, location, context) # Compile the vertex data following V-steps. basic_blocks.extend( _compile_vertex_ast(schema, current_schema_type, ast, - location, context, local_directives, fields)) + location, context, local_unique_directives, fields)) return basic_blocks @@ -625,7 +626,7 @@ def _compile_root_ast_to_ir(schema, ast, type_equivalence_hints=None): # Ensure the GraphQL query root doesn't have any vertex directives # that are disallowed on the root node. - validate_root_vertex_directives(get_directives(base_ast)) + validate_root_vertex_directives(base_ast) # Compile and add the basic blocks for the query's base AST vertex. new_basic_blocks = _compile_ast_node_to_ir( diff --git a/graphql_compiler/compiler/directive_helpers.py b/graphql_compiler/compiler/directive_helpers.py index 7a1987899..ef8f045d7 100644 --- a/graphql_compiler/compiler/directive_helpers.py +++ b/graphql_compiler/compiler/directive_helpers.py @@ -4,6 +4,7 @@ import six from ..exceptions import GraphQLCompilationError +from .filters import is_filter_with_outer_scope_vertex_field_operator ALLOWED_DUPLICATED_DIRECTIVES = frozenset({'filter'}) @@ -18,19 +19,18 @@ u'{}'.format(VERTEX_DIRECTIVES_PROHIBITED_ON_ROOT, VERTEX_ONLY_DIRECTIVES)) -def get_directives(ast): +def get_unique_directives(ast): """Return a dict of directive name to directive object for the given AST node. - Also verifies that each directive is only present once on any given AST node, - raising GraphQLCompilationError if that is not the case. + Any directives that are allowed to exist more than once on any AST node are ignored. + For any directives that can only exist up to once, we verify that they are not duplicated + raising GraphQLCompilationError in case we find them more than once on the AST node. Args: ast: GraphQL AST node, obtained from the graphql library Returns: - dict of string to: - - directive object, if the directive is only allowed to appear at most once, or - - list of directive objects, if the directive is allowed to appear multiple times + dict of string to directive object """ if not ast.directives: return dict() @@ -39,7 +39,7 @@ def get_directives(ast): for directive_obj in ast.directives: directive_name = directive_obj.name.value if directive_name in ALLOWED_DUPLICATED_DIRECTIVES: - result.setdefault(directive_name, []).append(directive_obj) + pass # We don't return these. elif directive_name in result: raise GraphQLCompilationError(u'Directive was unexpectedly applied twice in the same ' u'location: {} {}'.format(directive_name, ast.directives)) @@ -49,6 +49,48 @@ def get_directives(ast): return result +def get_local_filter_directives(ast, inner_vertex_fields): + """Get all filter directives that apply to the current field. + + This helper abstracts away the fact that some vertex field filtering operators apply on the + inner scope (the scope of the inner vertex field on which they are applied), whereas some apply + on the outer scope (the scope that contains the inner vertex field). + See filters.py for more information. + + Args: + ast: a GraphQL AST object for which to load local filters, from the graphql library + inner_vertex_fields: a list of inner AST objects representing vertex fields that are within + the current field. If currently processing a property field (i.e. + there are no inner vertex fields), this argument may be set to None. + + Returns: + list of filter directive objects + """ + result = [] + if ast.directives: # it'll be None if the AST has no directives at that node + for directive_obj in ast.directives: + if directive_obj.name.value != 'filter': + continue + + # Of all filters that appear *on the field itself*, only the ones that apply + # to the outer scope are not considered "local" and are not to be returned. + if not is_filter_with_outer_scope_vertex_field_operator(directive_obj): + result.append(directive_obj) + + if inner_vertex_fields: # allow the argument to be None + for inner_ast in inner_vertex_fields: + for directive_obj in inner_ast.directives: + if directive_obj.name.value != 'filter': + continue + + # Of all filters that appear on an inner vertex field, only the ones that apply + # to the outer scope are "local" to the outer field and therefore to be returned. + if is_filter_with_outer_scope_vertex_field_operator(directive_obj): + result.append(directive_obj) + + return result + + def validate_property_directives(directives): """Validate the directives that appear at a property field.""" for directive_name in six.iterkeys(directives): @@ -65,9 +107,13 @@ def validate_vertex_directives(directives): u'Found property-only directive {} set on vertex.'.format(directive_name)) -def validate_root_vertex_directives(directives): +def validate_root_vertex_directives(root_ast): """Validate the directives that appear at the root vertex field.""" - directives_present_at_root = set(six.iterkeys(directives)) + directives_present_at_root = set() + for directive_obj in root_ast.directives: + directive_name = directive_obj.name.value + directives_present_at_root.add(directive_name) + disallowed_directives = directives_present_at_root & VERTEX_DIRECTIVES_PROHIBITED_ON_ROOT if disallowed_directives: raise GraphQLCompilationError(u'Found prohibited directives on root vertex: ' diff --git a/graphql_compiler/compiler/filters.py b/graphql_compiler/compiler/filters.py index bcb9345cc..648a5f502 100644 --- a/graphql_compiler/compiler/filters.py +++ b/graphql_compiler/compiler/filters.py @@ -438,10 +438,62 @@ def _process_contains_filter_directive(schema, current_schema_type, ast, return blocks.Filter(filter_predicate) +def _get_filter_op_name_and_values(directive): + """Extract the (op_name, operator_params) tuple from a directive object.""" + args = get_uniquely_named_objects_by_name(directive.arguments) + if 'op_name' not in args: + raise AssertionError(u'op_name not found in filter directive arguments!' + u'Validation should have caught this: {}'.format(directive)) + + # HACK(predrag): Workaround for graphql-core validation issue + # https://github.com/graphql-python/graphql-core/issues/97 + if not isinstance(args['value'].value, ListValue): + raise GraphQLValidationError(u'Filter directive value was not a list: {}'.format(directive)) + + op_name = args['op_name'].value.value + operator_params = [x.value for x in args['value'].value.values] + + return (op_name, operator_params) + + ### # Public API ### +COMPARISON_OPERATORS = frozenset({u'=', u'!=', u'>', u'<', u'>=', u'<='}) +PROPERTY_FIELD_OPERATORS = COMPARISON_OPERATORS | frozenset({ + u'between', + u'in_collection', + u'contains', + u'has_substring', +}) + +# Vertex field filtering operators can apply to the inner scope or the outer scope. +# Consider: +# { +# Foo { +# out_Foo_Bar @filter(op_name: "...", value: [...]) { +# ... +# } +# } +# } +# +# If the filter on out_Foo_Bar filters the Foo, we say that it filters the outer scope. +# Instead, if the filter filters the Bar connected to the Foo, it filters the inner scope. +INNER_SCOPE_VERTEX_FIELD_OPERATORS = frozenset({'name_or_alias'}) +OUTER_SCOPE_VERTEX_FIELD_OPERATORS = frozenset() + +VERTEX_FIELD_OPERATORS = INNER_SCOPE_VERTEX_FIELD_OPERATORS | OUTER_SCOPE_VERTEX_FIELD_OPERATORS + +ALL_OPERATORS = PROPERTY_FIELD_OPERATORS | VERTEX_FIELD_OPERATORS + + +def is_filter_with_outer_scope_vertex_field_operator(directive): + """Return True if the directive's operator is an outer scope vertex field operator.""" + op_name, _ = _get_filter_op_name_and_values(directive) + return op_name in OUTER_SCOPE_VERTEX_FIELD_OPERATORS + + def process_filter_directive(schema, current_schema_type, ast, context, directive): """Return a Filter basic block that corresponds to the filter operation in the directive. @@ -456,32 +508,25 @@ def process_filter_directive(schema, current_schema_type, ast, context, directiv Returns: a Filter basic block that performs the requested filtering operation """ - args = get_uniquely_named_objects_by_name(directive.arguments) - if 'op_name' not in args: - raise AssertionError(u'op_name not found in filter directive arguments!' - u'Validation should have caught this: {}'.format(directive)) - - # HACK(predrag): Workaround for graphql-core validation issue - # https://github.com/graphql-python/graphql-core/issues/97 - if not isinstance(args['value'].value, ListValue): - raise GraphQLValidationError(u'Filter directive value was not a list: {}'.format(directive)) - - op_name = args['op_name'].value.value - operator_params = [x.value for x in args['value'].value.values] - - comparison_operators = {u'=', u'!=', u'>', u'<', u'>=', u'<='} - - if op_name in comparison_operators: + op_name, operator_params = _get_filter_op_name_and_values(directive) + + non_comparison_filters = { + u'name_or_alias': _process_name_or_alias_filter_directive, + u'between': _process_between_filter_directive, + u'in_collection': _process_in_collection_filter_directive, + u'has_substring': _process_has_substring_filter_directive, + u'contains': _process_contains_filter_directive, + } + all_recognized_filters = frozenset(non_comparison_filters.keys()) | COMPARISON_OPERATORS + if all_recognized_filters != ALL_OPERATORS: + unrecognized_filters = ALL_OPERATORS - all_recognized_filters + raise AssertionError(u'Some filtering operators are defined but do not have an associated ' + u'processing function. This is a bug: {}'.format(unrecognized_filters)) + + if op_name in COMPARISON_OPERATORS: process_func = partial(_process_comparison_filter_directive, operator=op_name) else: - known_filter_types = { - u'name_or_alias': _process_name_or_alias_filter_directive, - u'between': _process_between_filter_directive, - u'in_collection': _process_in_collection_filter_directive, - u'has_substring': _process_has_substring_filter_directive, - u'contains': _process_contains_filter_directive, - } - process_func = known_filter_types.get(op_name, None) + process_func = non_comparison_filters.get(op_name, None) if process_func is None: raise GraphQLCompilationError(u'Unknown op_name for filter directive: {}'.format(op_name))