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

Fail the query with a helpful message if the variables defined do not match the expected values #3856

Merged
merged 4 commits into from
Nov 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ compatibility, the `cylc run` command will automatically symlink an existing

### Enhancements

[#3856](https://github.com/cylc/cylc-flow/pull/3856) - fail the GraphQL query
with a helpful message if the variables defined do not match the expected
values.

[#3853](https://github.com/cylc/cylc-flow/pull/3853) - Update protobuf and
pyzmq.

Expand Down
19 changes: 18 additions & 1 deletion cylc/flow/network/graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,29 @@ def __init__(self, schema, document_ast, variable_values):
for defn in document_ast.definitions:
if isinstance(defn, ast.OperationDefinition):
root_type = get_operation_root_type(schema, defn)
definition_variables = defn.variable_definitions or []
if definition_variables:
def_var_names = {
v.variable.name.value
for v in definition_variables
}
var_names_diff = def_var_names.difference({
k
for k in variable_values
if k in def_var_names
})
# check if we are missing some of the definition variables
if var_names_diff:
msg = (f'Please check your query variables. The '
f'following variables are missing: '
f'[{", ".join(var_names_diff)}]')
raise ValueError(msg)
self.operation_defs[getattr(defn.name, 'value', root_type)] = {
'definition': defn,
'parent_type': root_type,
'variables': get_variable_values(
schema,
defn.variable_definitions or [],
definition_variables,
variable_values
),
}
Expand Down
119 changes: 119 additions & 0 deletions tests/unit/network/test_graphql.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from typing import Optional

import pytest
from graphql import parse

from cylc.flow.network.graphql import AstDocArguments
from cylc.flow.network.schema import schema


@pytest.mark.parametrize(
'query,'
'variables,'
'expected_variables,'
'expected_error',
[
# a simple query with the correct variables
(
'''
query ($workflowID: ID) {
workflows (ids: [$workflowID]) {
id
}
}
''',
{
'workflowID': 'cylc|workflow'
},
{
'workflowID': 'cylc|workflow'
},
None
),
# a query with a fragment and with the correct variables
(
'''
query ($workflowID: ID) {
...WorkflowData
}
fragment WorkflowData on workflows {
workflows (ids: [$workflowID]) {
id
}
}
''',
{
'workflowID': 'cylc|workflow'
},
{
'workflowID': 'cylc|workflow'
},
None
),
# a query with the right variable definition, but missing
# variable in the provided values
(
'''
query ($workflowID: ID) {
workflows (ids: [$workflowID]) {
id
}
}
''',
{
'workflowId': 'cylc|workflow'
},
None,
ValueError
)
]
)
def test_query_variables(
query: str,
variables: dict,
expected_variables: Optional[dict],
expected_error: Optional[Exception]
):
"""Test that query variables are parsed correctly.

Args:
query (str): a valid GraphQL query (using our schema)
variables (dict): map with variable values for the query
expected_variables (dict): expected parsed variables
expected_error (Exception): expected error, if any
"""
def test():
"""Inner function to avoid duplication in if/else"""
document = parse(query)
document_arguments = AstDocArguments(
schema=schema,
document_ast=document,
variable_values=variables
)
parsed_variables = next(
iter(
document_arguments.operation_defs.values()
)
)['variables']
assert expected_variables == parsed_variables
if expected_error is not None:
with pytest.raises(expected_error):
test()
else:
test()