From 61bee44ffa96a8d6740ab27e388ad672c019d07c Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 01:04:57 +0300 Subject: [PATCH 01/20] Return result in standard GraphQL format Success case: {data = ...} Error case: {errors = {message = ..., ...}} compiled_query:execute(...) becomes exception-safe (performs pcall internally), but graphql.new(...):compile() and graphql.new(...).execute(...) still can throw an exception. Enabled 5_2 test in common.test.lua, it fails before fix for #135 ( PR #178). Prerequisite for #71. Prerequisite for #134. --- graphql/impl.lua | 11 ++- graphql/server/server.lua | 10 +-- graphql/utils.lua | 23 ++++++ test/bench/bench.lua | 4 +- test/common/directives.test.lua | 8 +-- test/common/introspection.test.lua | 2 +- test/common/limit_result.test.lua | 25 ++++--- test/common/mutation.test.lua | 40 +++++------ test/common/pcre.test.lua | 11 +-- test/common/query_timeout.test.lua | 15 ++-- test/extra/to_avro_arrays.test.lua | 6 +- test/extra/to_avro_huge.test.lua | 6 +- test/extra/to_avro_nested.test.lua | 6 +- test/extra/to_avro_nullable.test.lua | 6 +- test/space/complemented_config.test.lua | 2 +- test/space/default_instance.test.lua | 4 +- test/space/nested_args.test.lua | 4 +- test/space/unflatten_tuple.test.lua | 2 +- test/space/zero_config.test.lua | 2 +- test/testdata/array_and_map_testdata.lua | 2 +- test/testdata/avro_refs_testdata.lua | 8 +-- test/testdata/common_testdata.lua | 62 ++++++++-------- test/testdata/compound_index_testdata.lua | 62 ++++++---------- test/testdata/multihead_conn_testdata.lua | 4 +- test/testdata/nested_record_testdata.lua | 4 +- test/testdata/nullable_1_1_conn_testdata.lua | 76 ++++++++------------ test/testdata/nullable_index_testdata.lua | 14 ++-- test/testdata/union_testdata.lua | 4 +- 28 files changed, 207 insertions(+), 216 deletions(-) diff --git a/graphql/impl.lua b/graphql/impl.lua index 83ed138..3e73a3d 100644 --- a/graphql/impl.lua +++ b/graphql/impl.lua @@ -41,8 +41,15 @@ local function gql_execute(qstate, variables, operation_name) local root_value = {} - return execute(state.schema, qstate.ast, root_value, variables, - operation_name) + local ok, data = pcall(execute, state.schema, qstate.ast, root_value, + variables, operation_name) + if not ok then + local err = utils.serialize_error(data) + return {errors = {err}} + end + return { + data = data, + } end --- Compile a query and execute an operation. diff --git a/graphql/server/server.lua b/graphql/server/server.lua index 0cf42b3..b7ece3f 100644 --- a/graphql/server/server.lua +++ b/graphql/server/server.lua @@ -114,16 +114,8 @@ function server.init(graphql, host, port) operation_name = nil end - local ok, result = pcall(compiled_query.execute, compiled_query, - variables, operation_name) - if not ok then - return { - status = 200, - body = json.encode({error = {{message = result}}}) - } - end + local result = compiled_query:execute(variables, operation_name) - result = {data = result} return { status = 200, headers = { diff --git a/graphql/utils.lua b/graphql/utils.lua index e07fd0b..a46cf7a 100644 --- a/graphql/utils.lua +++ b/graphql/utils.lua @@ -1,6 +1,8 @@ --- Various utility function used across the graphql module sources and tests. +local json = require('json') local log = require('log') +local ffi = require('ffi') local utils = {} @@ -217,4 +219,25 @@ function utils.optional_require_rex() return rex, is_pcre2 end +function utils.serialize_error(err) + if type(err) == 'string' then + return {message = err} + elseif type(err) == 'cdata' and + tostring(ffi.typeof(err)) == 'ctype' then + return {message = tostring(err)} + elseif type(err) == 'table' and type(err.message) == 'string' then + return err + end + + local message = 'internal error: unknown error format' + local encode_use_tostring_orig = json.cfg.encode_use_tostring + json.cfg({encode_use_tostring = true}) + local orig_error = json.encode(err) + json.cfg({encode_use_tostring = encode_use_tostring_orig}) + return { + message = message, + orig_error = orig_error, + } +end + return utils diff --git a/test/bench/bench.lua b/test/bench/bench.lua index 8f0a246..aaefa6d 100644 --- a/test/bench/bench.lua +++ b/test/bench/bench.lua @@ -49,13 +49,13 @@ local function workload(shard, bench_prepare, bench_iter, opts) -- first iteration; print result and update checksum local result = bench_iter(state) - local result_str = yaml.encode(result) + local result_str = yaml.encode(result.data) checksum:update(result_str .. '1') -- the rest iterations; just update checksum for i = 2, iterations do local result = bench_iter(state) - local result_str = yaml.encode(result) + local result_str = yaml.encode(result.data) checksum:update(result_str .. tostring(i)) if i % 100 == 0 then fiber.yield() diff --git a/test/common/directives.test.lua b/test/common/directives.test.lua index 35a4d87..6fd357b 100755 --- a/test/common/directives.test.lua +++ b/test/common/directives.test.lua @@ -59,7 +59,7 @@ local function run_queries(gql_wrapper) first_name: Ivan ]]):strip()) - test:is_deeply(result_1_1, exp_result_1_1, '1_1') + test:is_deeply(result_1_1.data, exp_result_1_1, '1_1') -- }}} -- {{{ 1_2 @@ -81,7 +81,7 @@ local function run_queries(gql_wrapper) description: first order of Ivan ]]):strip()) - test:is_deeply(result_1_2, exp_result_1_2, '1_2') + test:is_deeply(result_1_2.data, exp_result_1_2, '1_2') -- }}} @@ -122,7 +122,7 @@ local function run_queries(gql_wrapper) description: first order of Ivan ]]):strip()) - test:is_deeply(result_2_1, exp_result_2_1, '2_1') + test:is_deeply(result_2_1.data, exp_result_2_1, '2_1') -- }}} -- {{{ 2_2 @@ -148,7 +148,7 @@ local function run_queries(gql_wrapper) first_name: Ivan ]]):strip()) - test:is_deeply(result_2_2, exp_result_2_2, '2_2') + test:is_deeply(result_2_2.data, exp_result_2_2, '2_2') -- }}} diff --git a/test/common/introspection.test.lua b/test/common/introspection.test.lua index 9a9cdae..c004321 100755 --- a/test/common/introspection.test.lua +++ b/test/common/introspection.test.lua @@ -3113,7 +3113,7 @@ local function run_queries(gql_wrapper) test_utils.show_trace(function() local gql_query = gql_wrapper:compile(query) local result = gql_query:execute({}) - test:is_deeply(result, exp_result, 'introspection query') + test:is_deeply(result.data, exp_result, 'introspection query') end) assert(test:check(), 'check plan') diff --git a/test/common/limit_result.test.lua b/test/common/limit_result.test.lua index 8b00feb..6b00009 100755 --- a/test/common/limit_result.test.lua +++ b/test/common/limit_result.test.lua @@ -33,20 +33,25 @@ local function run_queries(gql_wrapper) local variables = { user_id = 5, } - local ok, result = pcall(gql_query.execute, gql_query, variables) - assert(ok == false, "this test should fail") - test:like(result, - 'count%[4%] exceeds limit%[3%] %(`resulting_object_cnt_max`', - 'resulting_object_cnt_max test') + local result = gql_query:execute(variables) + assert(result.data == nil, "this test should fail") + assert(result.errors ~= nil, "this test should fail") + local err = test_utils.strip_error(result.errors[1].message) + test:like(err, + 'count%[4%] exceeds limit%[3%] %(`resulting_object_cnt_max`', + 'resulting_object_cnt_max test') + variables = { user_id = 5, description = "no such description" } - ok, result = pcall(gql_query.execute, gql_query, variables) - assert(ok == false, "this test should fail") - test:like(result, - 'count%[6%] exceeds limit%[5%] %(`fetched_object_cnt_max`', - 'resulting_object_cnt_max test') + local result = gql_query:execute(variables) + assert(result.data == nil, "this test should fail") + assert(result.errors ~= nil, "this test should fail") + local err = test_utils.strip_error(result.errors[1].message) + test:like(err, + 'count%[6%] exceeds limit%[5%] %(`fetched_object_cnt_max`', + 'resulting_object_cnt_max test') assert(test:check(), 'check plan') end diff --git a/test/common/mutation.test.lua b/test/common/mutation.test.lua index 77d2e91..b0707f3 100755 --- a/test/common/mutation.test.lua +++ b/test/common/mutation.test.lua @@ -92,7 +92,7 @@ local function check_insert(test, gql_wrapper, virtbox, mutation_insert, -- check mutation result from graphql local result = gql_mutation_insert:execute(dont_pass_variables and {} or variables_insert) - test:is_deeply(result, exp_result_insert, 'insert result') + test:is_deeply(result.data, exp_result_insert, 'insert result') -- check inserted user local tuple = get_tuple(virtbox, 'user_collection', {user_id}) test:ok(tuple ~= nil, 'tuple was inserted') @@ -134,7 +134,7 @@ local function check_insert_order_metainfo(test, gql_wrapper, virtbox, -- check mutation result local gql_mutation_insert = gql_wrapper:compile(mutation_insert) local result = gql_mutation_insert:execute(variables) - test:is_deeply(result, exp_result_insert, 'insert result') + test:is_deeply(result.data, exp_result_insert, 'insert result') -- check inserted tuple local EXTERNAL_ID_STRING = 1 -- 0 is for int @@ -218,7 +218,7 @@ local function check_update(test, gql_wrapper, virtbox, mutation_update, -- check mutation result from graphql local result = gql_mutation_update:execute(dont_pass_variables and {} or variables_update) - test:is_deeply(result, exp_result_update, 'update result') + test:is_deeply(result.data, exp_result_update, 'update result') -- check updated user local tuple = get_tuple(virtbox, 'user_collection', {user_id}) test:ok(tuple ~= nil, 'updated tuple exists') @@ -282,7 +282,7 @@ local function check_update_order_metainfo(test, gql_wrapper, virtbox, -- check mutation result local gql_mutation_update = gql_wrapper:compile(mutation_update) local result = gql_mutation_update:execute(variables) - test:is_deeply(result, exp_result_update, 'update result') + test:is_deeply(result.data, exp_result_update, 'update result') -- check updated tuple local tuple = get_tuple(virtbox, 'order_metainfo_collection', @@ -334,7 +334,7 @@ local function check_delete(test, gql_wrapper, virtbox, mutation_delete, -- check mutation result from graphql local result = gql_mutation_delete:execute(dont_pass_variables and {} or variables_delete) - test:is_deeply(result, exp_result_delete, 'delete result') + test:is_deeply(result.data, exp_result_delete, 'delete result') -- check the user was deleted local tuple = get_tuple(virtbox, 'user_collection', {user_id}) @@ -468,10 +468,10 @@ local function run_queries(gql_wrapper, virtbox, meta) } ]] local gql_mutation_insert_3i = gql_wrapper:compile(mutation_insert_3i) - local ok, err = pcall(gql_mutation_insert_3i.execute, - gql_mutation_insert_3i, {}) + local result = gql_mutation_insert_3i:execute({}) + local err = test_utils.strip_error(result.errors[1].message) local err_exp = '"insert" must be the only argument when it is present' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is(err, err_exp, '"insert" argument is forbidden with other filters (object arguments)') -- test "insert" argument is forbidden with list arguments @@ -489,10 +489,10 @@ local function run_queries(gql_wrapper, virtbox, meta) } ]] local gql_mutation_insert_4i = gql_wrapper:compile(mutation_insert_4i) - local ok, err = pcall(gql_mutation_insert_4i.execute, - gql_mutation_insert_4i, {}) + local result = gql_mutation_insert_4i:execute({}) + local err = test_utils.strip_error(result.errors[1].message) local err_exp = '"insert" must be the only argument when it is present' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is(err, err_exp, '"insert" argument is forbidden with other filters (list arguments)') -- test "insert" argument is forbidden with other extra argument @@ -510,10 +510,10 @@ local function run_queries(gql_wrapper, virtbox, meta) } ]] local gql_mutation_insert_5i = gql_wrapper:compile(mutation_insert_5i) - local ok, err = pcall(gql_mutation_insert_5i.execute, - gql_mutation_insert_5i, {}) + local result = gql_mutation_insert_5i:execute({}) + local err = test_utils.strip_error(result.errors[1].message) local err_exp = '"insert" must be the only argument when it is present' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is(err, err_exp, '"insert" argument is forbidden with other filters (extra arguments)') -- test inserting an object into a collection with subrecord, union, array @@ -1134,11 +1134,11 @@ local function run_queries(gql_wrapper, virtbox, meta) user_id = 'user_id_201', } } - local ok, err = pcall(gql_mutation_update_4.execute, gql_mutation_update_4, - variables_update_4) + local result = gql_mutation_update_4:execute(variables_update_4) + local err = test_utils.strip_error(result.errors[1].message) local err_exp = "Attempt to modify a tuple field which is part of index " .. "'user_id_index' in space 'user_collection'" - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is(err, err_exp, 'updating of a field of a primary key when it is NOT shard key field') local mutation_update_5 = [[ @@ -1159,11 +1159,11 @@ local function run_queries(gql_wrapper, virtbox, meta) order_id = 'order_id_4001', } } - local ok, err = pcall(gql_mutation_update_5.execute, gql_mutation_update_5, - variables_update_5) + local result = gql_mutation_update_5:execute(variables_update_5) + local err = test_utils.strip_error(result.errors[1].message) local err_exp = "Attempt to modify a tuple field which is part of index " .. "'order_id_index' in space 'order_collection'" - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is(err, err_exp, 'updating of a field of a primary key when it is shard key field') -- }}} diff --git a/test/common/pcre.test.lua b/test/common/pcre.test.lua index 32abd25..b6d6a1c 100755 --- a/test/common/pcre.test.lua +++ b/test/common/pcre.test.lua @@ -50,7 +50,7 @@ local function run_queries(gql_wrapper) middle_name: Ivanovich ]]):strip()) - test:is_deeply(result_1_1, exp_result_1_1, '1_1') + test:is_deeply(result_1_1.data, exp_result_1_1, '1_1') -- }}} -- {{{ offset + regexp match @@ -71,7 +71,7 @@ local function run_queries(gql_wrapper) first_name: Vasiliy ]]):strip()) - test:is_deeply(result_1_2, exp_result_1_2, '1_2') + test:is_deeply(result_1_2.data, exp_result_1_2, '1_2') -- }}} -- {{{ UTF-8 in regexp @@ -93,7 +93,7 @@ local function run_queries(gql_wrapper) middle_name: Иванович ]]):strip()) - test:is_deeply(result_1_3, exp_result_1_3, '1_3') + test:is_deeply(result_1_3.data, exp_result_1_3, '1_3') -- }}} @@ -120,7 +120,7 @@ local function run_queries(gql_wrapper) return gql_query_1i:execute({}) end) - test:is_deeply(result_1i_1, exp_result_1_1, '1i_1') + test:is_deeply(result_1i_1.data, exp_result_1_1, '1i_1') -- }}} @@ -154,7 +154,8 @@ local function run_queries(gql_wrapper) return gql_query_2:execute({}) end) - test:is_deeply(result_2, exp_result_2, 'regexp match by a subrecord field') + test:is_deeply(result_2.data, exp_result_2, + 'regexp match by a subrecord field') -- }}} diff --git a/test/common/query_timeout.test.lua b/test/common/query_timeout.test.lua index e41b20b..17eea35 100755 --- a/test/common/query_timeout.test.lua +++ b/test/common/query_timeout.test.lua @@ -7,7 +7,7 @@ package.path = fio.abspath(debug.getinfo(1).source:match("@?(.*/)") :gsub('/./', '/'):gsub('/+$', '')) .. '/../../?.lua' .. ';' .. package.path local tap = require('tap') -local utils = require('test.test_utils') +local test_utils = require('test.test_utils') local testdata = require('test.testdata.user_order_item_testdata') local function run_queries(gql_wrapper) @@ -30,18 +30,19 @@ local function run_queries(gql_wrapper) ]] local gql_query = gql_wrapper:compile(query) - local variables = { - } - local ok, result = pcall(gql_query.execute, gql_query, variables) - assert(ok == false, 'this test should fail') - test:like(result, 'query execution timeout exceeded', 'timeout test') + local variables = {} + local result = gql_query:execute(variables) + assert(result.data == nil, "this test should fail") + assert(result.errors ~= nil, "this test should fail") + local err = test_utils.strip_error(result.errors[1].message) + test:like(err, 'query execution timeout exceeded', 'timeout test') assert(test:check(), 'check plan') end box.cfg({}) -utils.run_testdata(testdata, { +test_utils.run_testdata(testdata, { run_queries = run_queries, graphql_opts = { timeout_ms = 0.001, diff --git a/test/extra/to_avro_arrays.test.lua b/test/extra/to_avro_arrays.test.lua index 90419ae..32cc7f0 100755 --- a/test/extra/to_avro_arrays.test.lua +++ b/test/extra/to_avro_arrays.test.lua @@ -96,16 +96,16 @@ user_collection: ]] result_expected = yaml.decode(result_expected) local result = gql_query:execute(variables) -test:is_deeply(result, result_expected, 'graphql query exec result') +test:is_deeply(result.data, result_expected, 'graphql query exec result') local ok, ash, r, fs, _ ok, ash = avro.create(avros) assert(ok) -ok, _ = avro.validate(ash, result) +ok, _ = avro.validate(ash, result.data) assert(ok) test:is(ok, true, 'gql result validation by avro') ok, fs = avro.compile(ash) assert(ok) -ok, r = fs.flatten(result) +ok, r = fs.flatten(result.data) assert(ok) ok, r = fs.unflatten(r) assert(ok) diff --git a/test/extra/to_avro_huge.test.lua b/test/extra/to_avro_huge.test.lua index 15ea2a7..4f824c1 100755 --- a/test/extra/to_avro_huge.test.lua +++ b/test/extra/to_avro_huge.test.lua @@ -248,16 +248,16 @@ order_collection: ]] result_expected = yaml.decode(result_expected) local result = gql_query:execute(variables) -test:is_deeply(result, result_expected, 'graphql query exec result') +test:is_deeply(result.data, result_expected, 'graphql query exec result') local ok, ash, r, fs, _ ok, ash = avro.create(avros) assert(ok) -ok, _ = avro.validate(ash, result) +ok, _ = avro.validate(ash, result.data) assert(ok) test:is(ok, true, 'gql result validation by avro') ok, fs = avro.compile(ash) assert(ok) -ok, r = fs.flatten(result) +ok, r = fs.flatten(result.data) assert(ok) ok, r = fs.unflatten(r) -- The test can fail if wrong avro-schema version is installed. diff --git a/test/extra/to_avro_nested.test.lua b/test/extra/to_avro_nested.test.lua index 2f7491e..99937af 100755 --- a/test/extra/to_avro_nested.test.lua +++ b/test/extra/to_avro_nested.test.lua @@ -94,16 +94,16 @@ user: ]] result_expected = yaml.decode(result_expected) local result = gql_query:execute(variables) -test:is_deeply(result, result_expected, 'graphql query exec result') +test:is_deeply(result.data, result_expected, 'graphql query exec result') local ok, ash, r, fs, _ ok, ash = avro.create(avros) assert(ok) -ok, _ = avro.validate(ash, result) +ok, _ = avro.validate(ash, result.data) assert(ok) test:is(ok, true, 'gql result validation by avro') ok, fs = avro.compile(ash) assert(ok) -ok, r = fs.flatten(result) +ok, r = fs.flatten(result.data) assert(ok) ok, r = fs.unflatten(r) assert(ok) diff --git a/test/extra/to_avro_nullable.test.lua b/test/extra/to_avro_nullable.test.lua index 453f8ff..bc24333 100755 --- a/test/extra/to_avro_nullable.test.lua +++ b/test/extra/to_avro_nullable.test.lua @@ -82,15 +82,15 @@ bar: ]] result_expected = yaml.decode(result_expected) local result = gql_query:execute(variables) -test:is_deeply(result, result_expected, 'graphql query exec result') +test:is_deeply(result.data, result_expected, 'graphql query exec result') local ok, ash = avro.create(avros) assert(ok, tostring(ash)) -local ok, err = avro.validate(ash, result) +local ok, err = avro.validate(ash, result.data) assert(ok, tostring(err)) test:is(ok, true, 'gql result validation by avro') local ok, fs = avro.compile(ash) assert(ok, tostring(fs)) -local ok, r = fs.flatten(result) +local ok, r = fs.flatten(result.data) assert(ok, tostring(r)) local ok, r = fs.unflatten(r) assert(ok, tostring(r)) diff --git a/test/space/complemented_config.test.lua b/test/space/complemented_config.test.lua index 59f6ad2..d5e3761 100755 --- a/test/space/complemented_config.test.lua +++ b/test/space/complemented_config.test.lua @@ -90,7 +90,7 @@ local function run_queries(gql_wrapper) order_id: order_id_1 description: Ivan order ]]):strip()) - test:is_deeply(result_1_1, exp_result_1_1, '1_1') + test:is_deeply(result_1_1.data, exp_result_1_1, '1_1') local cfg = gql_wrapper.internal.cfg cfg.accessor = nil diff --git a/test/space/default_instance.test.lua b/test/space/default_instance.test.lua index 843e6a5..95daaab 100755 --- a/test/space/default_instance.test.lua +++ b/test/space/default_instance.test.lua @@ -52,7 +52,7 @@ test_utils.show_trace(function() - user_id: user_id_1 name: Ivan ]]):strip()) - test:is_deeply(result, exp_result, '1') + test:is_deeply(result.data, exp_result, '1') end) -- test require('graphql').execute(query) @@ -65,7 +65,7 @@ test_utils.show_trace(function() - user_id: user_id_2 name: Vasiliy ]]):strip()) - test:is_deeply(result, exp_result, '2') + test:is_deeply(result.data, exp_result, '2') end) -- test server diff --git a/test/space/nested_args.test.lua b/test/space/nested_args.test.lua index 16ad75c..b6a3689 100755 --- a/test/space/nested_args.test.lua +++ b/test/space/nested_args.test.lua @@ -114,7 +114,7 @@ local function run_common_queries(gql_wrapper) last_name: Ivanov first_name: Ivan ]]):strip()) - test:is_deeply(result_1, exp_result_1, '1') + test:is_deeply(result_1.data, exp_result_1, '1') end local function run_emails_queries(gql_wrapper) @@ -162,7 +162,7 @@ local function run_emails_queries(gql_wrapper) in_reply_to: body: a ]]):strip()) - test:is_deeply(result_upside, exp_result_upside, 'upside') + test:is_deeply(result_upside.data, exp_result_upside, 'upside') end run_common_queries(common_gql_wrapper) diff --git a/test/space/unflatten_tuple.test.lua b/test/space/unflatten_tuple.test.lua index fad778d..67cf7d0 100755 --- a/test/space/unflatten_tuple.test.lua +++ b/test/space/unflatten_tuple.test.lua @@ -119,7 +119,7 @@ local function run_queries(gql_wrapper) return gql_query_1:execute(variables_1) end) - test:is_deeply(result, exp_result_1, '1') + test:is_deeply(result.data, exp_result_1, '1') assert(test:check(), 'check plan') end diff --git a/test/space/zero_config.test.lua b/test/space/zero_config.test.lua index a2f67b4..8b37507 100755 --- a/test/space/zero_config.test.lua +++ b/test/space/zero_config.test.lua @@ -67,7 +67,7 @@ local function run_queries(gql_wrapper) age: 42 name: Ivan ]]):strip()) - test:is_deeply(result_1_1, exp_result_1_1, '1_1') + test:is_deeply(result_1_1.data, exp_result_1_1, '1_1') local result_1_2 = test_utils.show_trace(function() local cfg = gql_wrapper.internal.cfg diff --git a/test/testdata/array_and_map_testdata.lua b/test/testdata/array_and_map_testdata.lua index bcc2ef2..7958df9 100644 --- a/test/testdata/array_and_map_testdata.lua +++ b/test/testdata/array_and_map_testdata.lua @@ -143,7 +143,7 @@ function array_testdata.run_queries(gql_wrapper) customer_balances: {'salary': {'value': 333}, 'deposit': {'value': 444}} ]]):strip()) - test:is_deeply(result_1, exp_result_1, '1') + test:is_deeply(result_1.data, exp_result_1, '1') assert(test:check(), 'check plan') end diff --git a/test/testdata/avro_refs_testdata.lua b/test/testdata/avro_refs_testdata.lua index 5657092..1c314c8 100644 --- a/test/testdata/avro_refs_testdata.lua +++ b/test/testdata/avro_refs_testdata.lua @@ -351,8 +351,8 @@ function testdata.run_queries(gql_wrapper) end local exp_result_1_1_p = {foo_2 = exp_result_1_1.foo} - test:is_deeply(result_1_1, exp_result_1_1, '1_1') - test:is_deeply(result_1_1_p, exp_result_1_1_p, '1_1_p') + test:is_deeply(result_1_1.data, exp_result_1_1, '1_1') + test:is_deeply(result_1_1_p.data, exp_result_1_1_p, '1_1_p') if avro_version == 2 then assert(test:check(), 'check plan') @@ -392,8 +392,8 @@ function testdata.run_queries(gql_wrapper) ]]):strip()) local exp_result_1_2_p = {foo_2 = exp_result_1_2.foo} - test:is_deeply(result_1_2, exp_result_1_2, '1_2') - test:is_deeply(result_1_2_p, exp_result_1_2_p, '1_2_p') + test:is_deeply(result_1_2.data, exp_result_1_2, '1_2') + test:is_deeply(result_1_2_p.data, exp_result_1_2_p, '1_2_p') assert(test:check(), 'check plan') end diff --git a/test/testdata/common_testdata.lua b/test/testdata/common_testdata.lua index f0e7e33..2a7ca9b 100644 --- a/test/testdata/common_testdata.lua +++ b/test/testdata/common_testdata.lua @@ -359,7 +359,7 @@ end function common_testdata.run_queries(gql_wrapper) local test = tap.test('common') - test:plan(26) + test:plan(27) local query_1 = [[ query user_by_order($order_id: String) { @@ -391,7 +391,7 @@ function common_testdata.run_queries(gql_wrapper) test_utils.show_trace(function() local gql_query_1 = gql_wrapper:compile(query_1) local result = gql_query_1:execute(variables_1) - test:is_deeply(result, exp_result_1, '1') + test:is_deeply(result.data, exp_result_1, '1') end) local query_1n = [[ @@ -411,7 +411,7 @@ function common_testdata.run_queries(gql_wrapper) test_utils.show_trace(function() local gql_query_1n = gql_wrapper:compile(query_1n) local result = gql_query_1n:execute(variables_1) - test:is_deeply(result, exp_result_1, '1n') + test:is_deeply(result.data, exp_result_1, '1n') end) local query_1inn = [[ @@ -431,7 +431,7 @@ function common_testdata.run_queries(gql_wrapper) test_utils.show_trace(function() local gql_query_1inn = gql_wrapper:compile(query_1inn) local result = gql_query_1inn:execute({}) - test:is_deeply(result, exp_result_1, '1inn') + test:is_deeply(result.data, exp_result_1, '1inn') end) local query_1tn = [[ @@ -481,19 +481,18 @@ function common_testdata.run_queries(gql_wrapper) local err_exp = 'Operation name must be specified if more than one ' .. 'operation exists.' - local ok, err = pcall(gql_query_1t.execute, gql_query_1t, {}) - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, - 'non-determined query name should give an error') + local result = gql_query_1t:execute({}) + local err = test_utils.strip_error(result.errors[1].message) + test:is(err, err_exp, 'non-determined query name should give an error') local err_exp = 'Unknown operation "non_existent_operation"' - local ok, err = pcall(gql_query_1t.execute, gql_query_1t, {}, - 'non_existent_operation') - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, - 'wrong operation name should give an error') + local result = gql_query_1t:execute({}, 'non_existent_operation') + local err = test_utils.strip_error(result.errors[1].message) + test:is(err, err_exp, 'wrong operation name should give an error') test_utils.show_trace(function() local result = gql_query_1t:execute({}, 'user_by_order') - test:is_deeply(result, exp_result_1, 'execute an operation by name') + test:is_deeply(result.data, exp_result_1, 'execute an operation by name') end) local query_2 = [[ @@ -531,7 +530,7 @@ function common_testdata.run_queries(gql_wrapper) test_utils.show_trace(function() local variables_2_1 = {user_id = 'user_id_1'} local result = gql_query_2:execute(variables_2_1) - test:is_deeply(result, exp_result_2_1, '2_1') + test:is_deeply(result.data, exp_result_2_1, '2_1') end) local exp_result_2_2 = yaml.decode(([[ @@ -570,7 +569,7 @@ function common_testdata.run_queries(gql_wrapper) offset = 'order_id_1573', -- 10th } local result = gql_query_2:execute(variables_2_2) - test:is_deeply(result, exp_result_2_2, '2_2') + test:is_deeply(result.data, exp_result_2_2, '2_2') end) local exp_result_2_3 = yaml.decode(([[ @@ -593,7 +592,7 @@ function common_testdata.run_queries(gql_wrapper) offset = 'order_id_1601', -- 38th } local result = gql_query_2:execute(variables_2_3) - test:is_deeply(result, exp_result_2_3, '2_3') + test:is_deeply(result.data, exp_result_2_3, '2_3') end) local exp_result_2_4 = yaml.decode(([[ @@ -614,7 +613,7 @@ function common_testdata.run_queries(gql_wrapper) offset = 'order_id_1602', -- 39th } local result = gql_query_2:execute(variables_2_4) - test:is_deeply(result, exp_result_2_4, '2_4') + test:is_deeply(result.data, exp_result_2_4, '2_4') end) local exp_result_2_5 = yaml.decode(([[ @@ -710,7 +709,7 @@ function common_testdata.run_queries(gql_wrapper) test_utils.show_trace(function() local variables_2_5 = {user_id = 'user_id_42'} local result = gql_query_2:execute(variables_2_5) - test:is_deeply(result, exp_result_2_5, '2_5') + test:is_deeply(result.data, exp_result_2_5, '2_5') end) local query_3 = [[ @@ -765,7 +764,7 @@ function common_testdata.run_queries(gql_wrapper) } local gql_query_3 = gql_wrapper:compile(query_3) local result = gql_query_3:execute(variables_3) - test:is_deeply(result, exp_result_3, '3') + test:is_deeply(result.data, exp_result_3, '3') end) -- extra filter for 1:N connection @@ -807,7 +806,7 @@ function common_testdata.run_queries(gql_wrapper) description = 'first order of Ivan', } local result = gql_query_4:execute(variables_4_1) - test:is_deeply(result, exp_result_4_1, '4_1') + test:is_deeply(result.data, exp_result_4_1, '4_1') end) local exp_result_4_2 = yaml.decode(([[ @@ -826,7 +825,7 @@ function common_testdata.run_queries(gql_wrapper) description = 'non-existent order', } local result = gql_query_4:execute(variables_4_2) - test:is_deeply(result, exp_result_4_2, '4_2') + test:is_deeply(result.data, exp_result_4_2, '4_2') end) -- extra filter for 1:1 connection @@ -868,24 +867,25 @@ function common_testdata.run_queries(gql_wrapper) description = 'first order of Ivan', } local result = gql_query_5:execute(variables_5_1) - test:is_deeply(result, exp_result_5_1, '5_1') + test:is_deeply(result.data, exp_result_5_1, '5_1') end) - --[=[ local exp_result_5_2 = yaml.decode(([[ - --- [] + --- + order_collection: + - order_id: order_id_1 + description: first order of Ivan ]]):strip()) - -- should match no users (or give an error?) + -- should match no users test_utils.show_trace(function() local variables_5_2 = { first_name = 'non-existent user', description = 'first order of Ivan', } local result = gql_query_5:execute(variables_5_2) - test:is_deeply(result, exp_result_5_2, '5_2') + test:is_deeply(result.data, exp_result_5_2, '5_2') end) - ]=]-- -- {{{ float, double @@ -998,7 +998,7 @@ function common_testdata.run_queries(gql_wrapper) local result = gql_query_6:execute(variables_6_1) local exp_result_6_1 = deeply_number_tostring(exp_result_6_1) local result = deeply_number_tostring(result) - test:is_deeply(result, exp_result_6_1, '6_1') + test:is_deeply(result.data, exp_result_6_1, '6_1') end) local exp_result_6_2 = yaml.decode(([[ @@ -1062,12 +1062,12 @@ function common_testdata.run_queries(gql_wrapper) local variables_6_2 = {limit = 10, in_stock = true} local result = gql_query_6:execute(variables_6_2) local result = deeply_number_tostring(result) - test:is_deeply(result, exp_result_6_2, '6_2') + test:is_deeply(result.data, exp_result_6_2, '6_2') local variables_6_2 = {limit = 10} local result = gql_query_6_i_true:execute(variables_6_2) local result = deeply_number_tostring(result) - test:is_deeply(result, exp_result_6_2, '6_2') + test:is_deeply(result.data, exp_result_6_2, '6_2') end) local exp_result_6_3 = yaml.decode(([[ @@ -1131,12 +1131,12 @@ function common_testdata.run_queries(gql_wrapper) local variables_6_3 = {limit = 10, in_stock = false} local result = gql_query_6:execute(variables_6_3) local result = deeply_number_tostring(result) - test:is_deeply(result, exp_result_6_3, '6_3') + test:is_deeply(result.data, exp_result_6_3, '6_3') local variables_6_3 = {limit = 10} local result = gql_query_6_i_false:execute(variables_6_3) local result = deeply_number_tostring(result) - test:is_deeply(result, exp_result_6_3, '6_3') + test:is_deeply(result.data, exp_result_6_3, '6_3') end) -- should fail diff --git a/test/testdata/compound_index_testdata.lua b/test/testdata/compound_index_testdata.lua index 6919d67..eec65c5 100644 --- a/test/testdata/compound_index_testdata.lua +++ b/test/testdata/compound_index_testdata.lua @@ -222,7 +222,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_num: 12 ]]):strip()) - test:is_deeply(result_1_1, exp_result_1_1, '1_1') + test:is_deeply(result_1_1.data, exp_result_1_1, '1_1') -- }}} -- {{{ get a top-level object by a full compound primary key plus filter @@ -241,7 +241,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_collection: [] ]]):strip()) - test:is_deeply(result_1_2, exp_result_1_2, '1_2') + test:is_deeply(result_1_2.data, exp_result_1_2, '1_2') -- }}} -- {{{ select top-level objects by a partial compound primary key (or maybe @@ -277,7 +277,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_num: 12 ]]):strip()) - test:is_deeply(result_1_3, exp_result_1_3, '1_3') + test:is_deeply(result_1_3.data, exp_result_1_3, '1_3') local result_1_4 = test_utils.show_trace(function() local variables_1_4 = {user_str = 'user_str_b'} @@ -369,7 +369,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_num: 20 ]]):strip()) - test:is_deeply(result_1_4, exp_result_1_4, '1_4') + test:is_deeply(result_1_4.data, exp_result_1_4, '1_4') -- }}} -- {{{ select top-level objects by a partial compound primary key plus @@ -388,7 +388,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_collection: [] ]]):strip()) - test:is_deeply(result_1_5, exp_result_1_5, '1_5') + test:is_deeply(result_1_5.data, exp_result_1_5, '1_5') local result_1_6 = test_utils.show_trace(function() local variables_1_6 = { @@ -403,7 +403,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_collection: [] ]]):strip()) - test:is_deeply(result_1_6, exp_result_1_6, '1_6') + test:is_deeply(result_1_6.data, exp_result_1_6, '1_6') -- }}} -- {{{ select objects by a connection by a full compound index @@ -473,7 +473,7 @@ function compound_index_testdata.run_queries(gql_wrapper) last_name: last name b ]]):strip()) - test:is_deeply(result_2_1, exp_result_2_1, '2_1') + test:is_deeply(result_2_1.data, exp_result_2_1, '2_1') -- }}} -- {{{ select objects by a connection by a full compound index plus filter @@ -497,7 +497,7 @@ function compound_index_testdata.run_queries(gql_wrapper) last_name: last name b ]]):strip()) - test:is_deeply(result_2_2, exp_result_2_2, '2_2') + test:is_deeply(result_2_2.data, exp_result_2_2, '2_2') -- }}} -- {{{ select object by a connection by a partial compound index @@ -1139,9 +1139,10 @@ function compound_index_testdata.run_queries(gql_wrapper) local function comparator(a, b) return a.order_num < b.order_num end - table.sort(result_3.user_collection[1].order_str_connection, comparator) + table.sort(result_3.data.user_collection[1].order_str_connection, + comparator) table.sort(exp_result_3.user_collection[1].order_str_connection, comparator) - test:is_deeply(result_3, exp_result_3, '3') + test:is_deeply(result_3.data, exp_result_3, '3') -- }}} -- {{{ offset on top-level by a full compound primary key @@ -1217,7 +1218,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_num: 2 ]]):strip()) - test:is_deeply(result_4_1, exp_result_4_1, '4_1') + test:is_deeply(result_4_1.data, exp_result_4_1, '4_1') -- }}} -- {{{ offset on top-level by a partial compound primary key (expected to @@ -1229,19 +1230,10 @@ function compound_index_testdata.run_queries(gql_wrapper) user_str = 'user_str_b', } } - local ok, err = pcall(function() - return gql_query_4:execute(variables_4_2) - end) - - local result_4_2 = {ok = ok, err = test_utils.strip_error(err)} - - local exp_result_4_2 = yaml.decode(([[ - --- - ok: false - err: offset by a partial key is forbidden - ]]):strip()) - - test:is_deeply(result_4_2, exp_result_4_2, '4_2') + local result = gql_query_4:execute(variables_4_2) + local err = test_utils.strip_error(result.errors[1].message) + local exp_err = 'offset by a partial key is forbidden' + test:is(err, exp_err, '4_2') -- }}} -- {{{ offset when using a connection by a full compound primary key @@ -1302,7 +1294,7 @@ function compound_index_testdata.run_queries(gql_wrapper) last_name: last name b ]]):strip()) - test:is_deeply(result_5_1, exp_result_5_1, '5_1') + test:is_deeply(result_5_1.data, exp_result_5_1, '5_1') -- }}} -- {{{ offset when using a connection by a partial compound primary key @@ -1316,19 +1308,11 @@ function compound_index_testdata.run_queries(gql_wrapper) order_str = 'order_str_b_2', } } - local ok, err = pcall(function() - return gql_query_5:execute(variables_5_2) - end) - - local result_5_2 = {ok = ok, err = test_utils.strip_error(err)} - - local exp_result_5_2 = yaml.decode(([[ - --- - ok: false - err: 'offset by a partial key is forbidden: expected "order_num" field' - ]]):strip()) - - test:is_deeply(result_5_2, exp_result_5_2, '5_2') + local result = gql_query_5:execute(variables_5_2) + local err = test_utils.strip_error(result.errors[1].message) + local exp_err = 'offset by a partial key is forbidden: ' .. + 'expected "order_num" field' + test:is(err, exp_err, '5_2') -- }}} -- {{{ compound offset argument constructed from separate variables @@ -1401,7 +1385,7 @@ function compound_index_testdata.run_queries(gql_wrapper) user_num: 2 ]]):strip()) - test:is_deeply(result_6, exp_result_6, '6') + test:is_deeply(result_6.data, exp_result_6, '6') -- }}} diff --git a/test/testdata/multihead_conn_testdata.lua b/test/testdata/multihead_conn_testdata.lua index 6680b95..0108fa6 100644 --- a/test/testdata/multihead_conn_testdata.lua +++ b/test/testdata/multihead_conn_testdata.lua @@ -384,7 +384,7 @@ function multihead_conn_testdata.run_queries(gql_wrapper) - account_id: credit_account_id_3 hero_id: hero_id_1 ]]):strip()) - test:is_deeply(result_1_1, exp_result_1_1, '1_1') + test:is_deeply(result_1_1.data, exp_result_1_1, '1_1') local variables_1_2 = {hero_id = 'hero_id_2'} local result_1_2 = test_utils.show_trace(function() @@ -407,7 +407,7 @@ function multihead_conn_testdata.run_queries(gql_wrapper) - account_id: dublon_account_id_3 hero_id: hero_id_2 ]]):strip()) - test:is_deeply(result_1_2, exp_result_1_2, '1_2') + test:is_deeply(result_1_2.data, exp_result_1_2, '1_2') assert(test:check(), 'check plan') end diff --git a/test/testdata/nested_record_testdata.lua b/test/testdata/nested_record_testdata.lua index 5ad89ff..4866f23 100644 --- a/test/testdata/nested_record_testdata.lua +++ b/test/testdata/nested_record_testdata.lua @@ -122,7 +122,7 @@ function testdata.run_queries(gql_wrapper) y: 2005 ]]):strip()) - test:is_deeply(result_1, exp_result_1, '1') + test:is_deeply(result_1.data, exp_result_1, '1') local query_2 = [[ query getUserByX($x: Long) { @@ -155,7 +155,7 @@ function testdata.run_queries(gql_wrapper) y: 2005 ]]):strip()) - test:is_deeply(result_2, exp_result_2, '2') + test:is_deeply(result_2.data, exp_result_2, '2') assert(test:check(), 'check plan') end diff --git a/test/testdata/nullable_1_1_conn_testdata.lua b/test/testdata/nullable_1_1_conn_testdata.lua index 077d4b2..6f9863e 100644 --- a/test/testdata/nullable_1_1_conn_testdata.lua +++ b/test/testdata/nullable_1_1_conn_testdata.lua @@ -324,7 +324,7 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) body: a ]]):strip()) - test:is_deeply(result, exp_result, 'downside_a') + test:is_deeply(result.data, exp_result, 'downside_a') local result = test_utils.show_trace(function() local variables_downside_h = {body = 'h'} @@ -344,7 +344,7 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) body: h ]]):strip()) - test:is_deeply(result, exp_result, 'downside_h') + test:is_deeply(result.data, exp_result, 'downside_h') -- }}} -- {{{ upside traversal (1:1 connections) @@ -383,7 +383,7 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) body: a ]]):strip()) - test:is_deeply(result, exp_result, 'upside') + test:is_deeply(result.data, exp_result, 'upside') -- }}} -- {{{ FULL MATCH constraint @@ -392,40 +392,26 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) -- to fail local variables_upside_x = {body = 'x'} - local ok, err = pcall(function() - return gql_query_upside:execute(variables_upside_x) - end) - - local result = {ok = ok, err = test_utils.strip_error(err)} - local exp_result = yaml.decode(([[ - --- - ok: false - err: 'FULL MATCH constraint was failed: connection key parts must be - all non-nulls or all nulls; object: {"domain":"graphql.tarantool.org", - "localpart":"062b56b1885c71c51153ccb880ac7315","body":"x", - "in_reply_to_domain":"graphql.tarantool.org", - "in_reply_to_localpart":null}' - ]]):strip()) - exp_result.err = exp_result.err:gsub(', ', ',') - test:is_deeply(result, exp_result, 'upside_x') + local result = gql_query_upside:execute(variables_upside_x) + local err = test_utils.strip_error(result.errors[1].message) + local exp_err = 'FULL MATCH constraint was failed: connection key parts ' .. + 'must be all non-nulls or all nulls; object: ' .. + '{"domain":"graphql.tarantool.org",' .. + '"localpart":"062b56b1885c71c51153ccb880ac7315","body":"x",' .. + '"in_reply_to_domain":"graphql.tarantool.org",' .. + '"in_reply_to_localpart":null}' + test:is(err, exp_err, 'upside_x') local variables_upside_y = {body = 'y'} - local ok, err = pcall(function() - return gql_query_upside:execute(variables_upside_y) - end) - - local result = {ok = ok, err = test_utils.strip_error(err)} - local exp_result = yaml.decode(([[ - --- - ok: false - err: 'FULL MATCH constraint was failed: connection key parts must be - all non-nulls or all nulls; object: {"domain":"graphql.tarantool.org", - "localpart":"1f70391f6ba858129413bd801b12acbf","body":"y", - "in_reply_to_domain":null, - "in_reply_to_localpart":"1f70391f6ba858129413bd801b12acbf"}' - ]]):strip()) - exp_result.err = exp_result.err:gsub(', ', ',') - test:is_deeply(result, exp_result, 'upside_y') + local result = gql_query_upside:execute(variables_upside_y) + local err = test_utils.strip_error(result.errors[1].message) + local exp_err = 'FULL MATCH constraint was failed: connection key parts ' .. + 'must be all non-nulls or all nulls; object: ' .. + '{"domain":"graphql.tarantool.org",' .. + '"localpart":"1f70391f6ba858129413bd801b12acbf","body":"y",' .. + '"in_reply_to_domain":null,' .. + '"in_reply_to_localpart":"1f70391f6ba858129413bd801b12acbf"}' + test:is(err, exp_err, 'upside_y') -- Check we get an error when trying to use dangling 1:1 connection. Check -- we don't get this error when `disable_dangling_check` is set. @@ -441,20 +427,14 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) - body: z ]]):strip()) - test:is_deeply(result, exp_result, 'upside_z disabled constraint check') + test:is_deeply(result.data, exp_result, 'upside_z disabled constraint check') else local variables_upside_z = {body = 'z'} - local ok, err = pcall(function() - return gql_query_upside:execute(variables_upside_z) - end) - - local result = {ok = ok, err = test_utils.strip_error(err)} - local exp_result = yaml.decode(([[ - --- - ok: false - err: "FULL MATCH constraint was failed: we expect 1 tuples, got 0" - ]]):strip()) - test:is_deeply(result, exp_result, 'upside_z constraint violation') + local result = gql_query_upside:execute(variables_upside_z) + local err = test_utils.strip_error(result.errors[1].message) + local exp_err = 'FULL MATCH constraint was failed: we expect 1 ' .. + 'tuples, got 0' + test:is(err, exp_err, 'upside_z constraint violation') end -- We can got zero objects by 1:1 connection when use filters, it is not @@ -471,7 +451,7 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) - body: f ]]):strip()) - test:is_deeply(result, exp_result, 'upside_f filter child') + test:is_deeply(result.data, exp_result, 'upside_f filter child') assert(test:check(), 'check plan') end diff --git a/test/testdata/nullable_index_testdata.lua b/test/testdata/nullable_index_testdata.lua index 9bd2275..b19396b 100644 --- a/test/testdata/nullable_index_testdata.lua +++ b/test/testdata/nullable_index_testdata.lua @@ -316,9 +316,7 @@ function nullable_index_testdata.run_queries(gql_wrapper) ]] local ok, err = pcall(function() - local gql_query_1 = gql_wrapper:compile(query_1) - local variables_1 = {} - return gql_query_1:execute(variables_1) + return gql_wrapper:compile(query_1) end) local result = {ok = ok, err = test_utils.strip_error(err)} @@ -414,7 +412,7 @@ function nullable_index_testdata.run_queries(gql_wrapper) id_or_null_2: '13' id: '13' ]]):strip()) - test:is_deeply(result, exp_result, '2_1') + test:is_deeply(result.data, exp_result, '2_1') -- lookup by the unique index; expected to see only the object with ID 42 local result = test_utils.show_trace(function() @@ -432,7 +430,7 @@ function nullable_index_testdata.run_queries(gql_wrapper) id_or_null_2: '42' id: '42' ]]):strip()) - test:is_deeply(result, exp_result, '2_2') + test:is_deeply(result.data, exp_result, '2_2') -- lookup by the non-unique index; expected to see only the object with ID -- 42 @@ -451,7 +449,7 @@ function nullable_index_testdata.run_queries(gql_wrapper) id_or_null_2: '42' id: '42' ]]):strip()) - test:is_deeply(result, exp_result, '2_3') + test:is_deeply(result.data, exp_result, '2_3') -- }}} -- {{{ connection: partial match with compound secondary index (nullable @@ -488,7 +486,7 @@ function nullable_index_testdata.run_queries(gql_wrapper) - id: '42' id: '42' ]]):strip()) - test:is_deeply(result_3_1, exp_result_3_1, '3_1') + test:is_deeply(result_3_1.data, exp_result_3_1, '3_1') local variables_3_2 = {id = '103'} local result_3_2 = test_utils.show_trace(function() @@ -502,7 +500,7 @@ function nullable_index_testdata.run_queries(gql_wrapper) bar_partial_non_unique: [] id: '103' ]]):strip()) - test:is_deeply(result_3_2, exp_result_3_2, '3_2') + test:is_deeply(result_3_2.data, exp_result_3_2, '3_2') -- }}} diff --git a/test/testdata/union_testdata.lua b/test/testdata/union_testdata.lua index 9637318..5837ae7 100644 --- a/test/testdata/union_testdata.lua +++ b/test/testdata/union_testdata.lua @@ -190,13 +190,13 @@ function union_testdata.run_queries(gql_wrapper) - {'salary': 'string salary', 'deposit': 'string deposit'} ]]):strip()) - test:is_deeply(result_1, exp_result_1, '1') + test:is_deeply(result_1.data, exp_result_1, '1') -- validating results with initial avro-schema local schemas = union_testdata_schemas local ok, schema = avro.create(schemas.user_collection) assert(ok) - for i, user in ipairs(result_1.user_collection) do + for i, user in ipairs(result_1.data.user_collection) do local ok, res = avro.validate(schema, user) test:ok(ok, ('validate %dth user'):format(i), res) end From 541bb8743fcac71b0a5e8209bc4ba6ffdd65f958 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 02:46:15 +0300 Subject: [PATCH 02/20] Fix make bench for avro-schema-3* --- test/bench/bench.lua | 4 ++-- test/bench/nesting-1-1.test.lua | 3 ++- test/bench/nesting-1-100.test.lua | 3 ++- test/bench/nesting-2-1-1.test.lua | 3 ++- test/bench/nesting-2-100-1.test.lua | 3 ++- test/bench/nesting-3-1-1-1.test.lua | 3 ++- test/bench/nesting-3-100-100-1.test.lua | 3 ++- test/testdata/bench_testdata.lua | 28 ++++++++++++------------- 8 files changed, 27 insertions(+), 23 deletions(-) diff --git a/test/bench/bench.lua b/test/bench/bench.lua index aaefa6d..d277087 100644 --- a/test/bench/bench.lua +++ b/test/bench/bench.lua @@ -157,8 +157,8 @@ function bench.run(test_name, opts) end -- helper for preparing benchmarking environment -function bench.bench_prepare_helper(testdata, shard) - testdata.fill_test_data(shard or box.space) +function bench.bench_prepare_helper(testdata, shard, meta) + testdata.fill_test_data(shard or box.space, meta) return test_utils.graphql_from_testdata(testdata, shard, { graphql_opts = { timeout_ms = graphql.TIMEOUT_INFINITY, diff --git a/test/bench/nesting-1-1.test.lua b/test/bench/nesting-1-1.test.lua index f88ba50..3400566 100755 --- a/test/bench/nesting-1-1.test.lua +++ b/test/bench/nesting-1-1.test.lua @@ -16,7 +16,8 @@ local testdata = require('test.testdata.bench_testdata') -- --------- local function bench_prepare(state) - state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard) + local meta = testdata.meta or testdata.get_test_metadata() + state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard, meta) local query = [[ query match_by_user_id($user_id: String) { user(user_id: $user_id) { diff --git a/test/bench/nesting-1-100.test.lua b/test/bench/nesting-1-100.test.lua index e077c7c..28a267c 100755 --- a/test/bench/nesting-1-100.test.lua +++ b/test/bench/nesting-1-100.test.lua @@ -16,7 +16,8 @@ local testdata = require('test.testdata.bench_testdata') -- --------- local function bench_prepare(state) - state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard) + local meta = testdata.meta or testdata.get_test_metadata() + state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard, meta) local query = [[ query match_users { user { diff --git a/test/bench/nesting-2-1-1.test.lua b/test/bench/nesting-2-1-1.test.lua index a8fc6ea..ad08cd0 100755 --- a/test/bench/nesting-2-1-1.test.lua +++ b/test/bench/nesting-2-1-1.test.lua @@ -16,7 +16,8 @@ local testdata = require('test.testdata.bench_testdata') -- --------- local function bench_prepare(state) - state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard) + local meta = testdata.meta or testdata.get_test_metadata() + state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard, meta) local query = [[ query match_by_user_and_passport_id($user_id: String, $passport_id: String) { diff --git a/test/bench/nesting-2-100-1.test.lua b/test/bench/nesting-2-100-1.test.lua index 42ca5f2..4d108ac 100755 --- a/test/bench/nesting-2-100-1.test.lua +++ b/test/bench/nesting-2-100-1.test.lua @@ -16,7 +16,8 @@ local testdata = require('test.testdata.bench_testdata') -- --------- local function bench_prepare(state) - state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard) + local meta = testdata.meta or testdata.get_test_metadata() + state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard, meta) local query = [[ query match_by_passport_id($passport_id: String) { user(user_to_passport_c: {passport_id: $passport_id}) { diff --git a/test/bench/nesting-3-1-1-1.test.lua b/test/bench/nesting-3-1-1-1.test.lua index 023db0c..0d8d3ac 100755 --- a/test/bench/nesting-3-1-1-1.test.lua +++ b/test/bench/nesting-3-1-1-1.test.lua @@ -16,7 +16,8 @@ local testdata = require('test.testdata.bench_testdata') -- --------- local function bench_prepare(state) - state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard) + local meta = testdata.meta or testdata.get_test_metadata() + state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard, meta) local query = [[ query match_by_user_and_passport($user_id: String, $number: String) { user(user_id: $user_id, user_to_passport_c: { diff --git a/test/bench/nesting-3-100-100-1.test.lua b/test/bench/nesting-3-100-100-1.test.lua index e299164..24ff41b 100755 --- a/test/bench/nesting-3-100-100-1.test.lua +++ b/test/bench/nesting-3-100-100-1.test.lua @@ -16,7 +16,8 @@ local testdata = require('test.testdata.bench_testdata') -- --------- local function bench_prepare(state) - state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard) + local meta = testdata.meta or testdata.get_test_metadata() + state.gql_wrapper = bench.bench_prepare_helper(testdata, state.shard, meta) local query = [[ query match_by_passport($number: String) { user(user_to_passport_c: {passport_c: {number: $number}}) { diff --git a/test/testdata/bench_testdata.lua b/test/testdata/bench_testdata.lua index f3e33ad..ea12bf8 100644 --- a/test/testdata/bench_testdata.lua +++ b/test/testdata/bench_testdata.lua @@ -1,4 +1,5 @@ local json = require('json') +local test_utils = require('test.test_utils') local bench_testdata = {} @@ -174,27 +175,24 @@ function bench_testdata.init_spaces() end) end -function bench_testdata.fill_test_data(shard) +function bench_testdata.fill_test_data(shard, meta) local virtbox = shard or box.space - local NULL_T = 0 - local STRING_T = 1 --luacheck: ignore - for i = 1, 100 do local s = tostring(i) - virtbox.user:replace({ - 'user_id_' .. s, - 'first name ' .. s, - NULL_T, box.NULL, - 'last name ' .. s, + test_utils.replace_object(virtbox, meta, 'user', { + user_id = 'user_id_' .. s, + first_name = 'first name ' .. s, + middle_name = box.NULL, + last_name = 'last name ' .. s, }) - virtbox.user_to_passport:replace({ - 'user_id_' .. s, - 'passport_id_' .. s, + test_utils.replace_object(virtbox, meta, 'user_to_passport', { + user_id = 'user_id_' .. s, + passport_id = 'passport_id_' .. s, }) - virtbox.passport:replace({ - 'passport_id_' .. s, - 'number_' .. s, + test_utils.replace_object(virtbox, meta, 'passport', { + passport_id = 'passport_id_' .. s, + number = 'number_' .. s, }) end end From 98f8b8677e83ee36b32a18b981ad04f6871a1edd Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 03:06:03 +0300 Subject: [PATCH 03/20] Fix make bench for hold traversal order It is follow up of 8ba78a68a2a2d716445c1399c25ba7ffda4e1dad. --- test/bench/nesting-2-1-1.test.lua | 4 ++-- test/bench/nesting-2-100-1.test.lua | 4 ++-- test/bench/nesting-3-1-1-1.test.lua | 4 ++-- test/bench/nesting-3-100-100-1.test.lua | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/bench/nesting-2-1-1.test.lua b/test/bench/nesting-2-1-1.test.lua index ad08cd0..7abe014 100755 --- a/test/bench/nesting-2-1-1.test.lua +++ b/test/bench/nesting-2-1-1.test.lua @@ -59,8 +59,8 @@ bench.run('nesting-2-1-1', { shard = 10000, }, checksums = { - space = 1500062808, - shard = 478898394, + space = 1462678117, + shard = 134534674, }, }) diff --git a/test/bench/nesting-2-100-1.test.lua b/test/bench/nesting-2-100-1.test.lua index 4d108ac..d4c0e7d 100755 --- a/test/bench/nesting-2-100-1.test.lua +++ b/test/bench/nesting-2-100-1.test.lua @@ -56,8 +56,8 @@ bench.run('nesting-2-100-1', { shard = 1000, }, checksums = { - space = 478898394, - shard = 946670351, + space = 134534674, + shard = 3070331298, }, }) diff --git a/test/bench/nesting-3-1-1-1.test.lua b/test/bench/nesting-3-1-1-1.test.lua index 0d8d3ac..aabb017 100755 --- a/test/bench/nesting-3-1-1-1.test.lua +++ b/test/bench/nesting-3-1-1-1.test.lua @@ -61,8 +61,8 @@ bench.run('nesting-3-1-1-1', { shard = 10000, }, checksums = { - space = 839993960, - shard = 922069577, + space = 3707454999, + shard = 4073260869, }, }) diff --git a/test/bench/nesting-3-100-100-1.test.lua b/test/bench/nesting-3-100-100-1.test.lua index 24ff41b..05a9340 100755 --- a/test/bench/nesting-3-100-100-1.test.lua +++ b/test/bench/nesting-3-100-100-1.test.lua @@ -59,8 +59,8 @@ bench.run('nesting-3-100-100-1', { shard = 1000, }, checksums = { - space = 922069577, - shard = 1286959955, + space = 4073260869, + shard = 538565788, }, }) From 1ff97b2edc7946d1a4e6d4bdae0214ecd72a44fd Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 12:17:38 +0300 Subject: [PATCH 04/20] Strip file:line from a string error --- graphql/utils.lua | 8 +++++++- test/common/limit_result.test.lua | 4 ++-- test/common/mutation.test.lua | 21 ++++++++++---------- test/common/query_timeout.test.lua | 2 +- test/space/init_fail.test.lua | 6 +++--- test/test_utils.lua | 6 ------ test/testdata/common_testdata.lua | 17 ++++++++-------- test/testdata/compound_index_testdata.lua | 4 ++-- test/testdata/nullable_1_1_conn_testdata.lua | 6 +++--- test/testdata/nullable_index_testdata.lua | 3 ++- 10 files changed, 40 insertions(+), 37 deletions(-) diff --git a/graphql/utils.lua b/graphql/utils.lua index a46cf7a..281c728 100644 --- a/graphql/utils.lua +++ b/graphql/utils.lua @@ -6,6 +6,12 @@ local ffi = require('ffi') local utils = {} +--- Return an error w/o file name and line number. +function utils.strip_error(err) + local res = tostring(err):gsub('^.-:.-: (.*)$', '%1') + return res +end + --- Recursively checks whether `sub` fields values are match `t` ones. function utils.is_subtable(t, sub) for k, v in pairs(sub) do @@ -221,7 +227,7 @@ end function utils.serialize_error(err) if type(err) == 'string' then - return {message = err} + return {message = utils.strip_error(err)} elseif type(err) == 'cdata' and tostring(ffi.typeof(err)) == 'ctype' then return {message = tostring(err)} diff --git a/test/common/limit_result.test.lua b/test/common/limit_result.test.lua index 6b00009..bd4e18c 100755 --- a/test/common/limit_result.test.lua +++ b/test/common/limit_result.test.lua @@ -36,7 +36,7 @@ local function run_queries(gql_wrapper) local result = gql_query:execute(variables) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message test:like(err, 'count%[4%] exceeds limit%[3%] %(`resulting_object_cnt_max`', 'resulting_object_cnt_max test') @@ -48,7 +48,7 @@ local function run_queries(gql_wrapper) local result = gql_query:execute(variables) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message test:like(err, 'count%[6%] exceeds limit%[5%] %(`fetched_object_cnt_max`', 'resulting_object_cnt_max test') diff --git a/test/common/mutation.test.lua b/test/common/mutation.test.lua index b0707f3..1437501 100755 --- a/test/common/mutation.test.lua +++ b/test/common/mutation.test.lua @@ -8,6 +8,7 @@ package.path = fio.abspath(debug.getinfo(1).source:match("@?(.*/)") local tap = require('tap') local yaml = require('yaml') +local utils = require('graphql.utils') local test_utils = require('test.test_utils') local testdata = require('test.testdata.common_testdata') @@ -435,7 +436,7 @@ local function run_queries(gql_wrapper, virtbox, meta) ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, mutation_insert_2) local err_exp = 'Non-existent argument "insert"' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, '"insert" argument is forbidden in a non-top level field') -- test "insert" argument is forbidden in a query @@ -450,7 +451,7 @@ local function run_queries(gql_wrapper, virtbox, meta) ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_insert) local err_exp = 'Non-existent argument "insert"' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, '"insert" argument is forbidden in a query') -- test "insert" argument is forbidden with object arguments @@ -469,7 +470,7 @@ local function run_queries(gql_wrapper, virtbox, meta) ]] local gql_mutation_insert_3i = gql_wrapper:compile(mutation_insert_3i) local result = gql_mutation_insert_3i:execute({}) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local err_exp = '"insert" must be the only argument when it is present' test:is(err, err_exp, '"insert" argument is forbidden with other filters (object arguments)') @@ -490,7 +491,7 @@ local function run_queries(gql_wrapper, virtbox, meta) ]] local gql_mutation_insert_4i = gql_wrapper:compile(mutation_insert_4i) local result = gql_mutation_insert_4i:execute({}) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local err_exp = '"insert" must be the only argument when it is present' test:is(err, err_exp, '"insert" argument is forbidden with other filters (list arguments)') @@ -511,7 +512,7 @@ local function run_queries(gql_wrapper, virtbox, meta) ]] local gql_mutation_insert_5i = gql_wrapper:compile(mutation_insert_5i) local result = gql_mutation_insert_5i:execute({}) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local err_exp = '"insert" must be the only argument when it is present' test:is(err, err_exp, '"insert" argument is forbidden with other filters (extra arguments)') @@ -977,7 +978,7 @@ local function run_queries(gql_wrapper, virtbox, meta) ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_update) local err_exp = 'Non-existent argument "update"' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, '"update" argument is forbidden in a query') -- test updating of a field by which a shard key is calculated (it is the @@ -1135,7 +1136,7 @@ local function run_queries(gql_wrapper, virtbox, meta) } } local result = gql_mutation_update_4:execute(variables_update_4) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local err_exp = "Attempt to modify a tuple field which is part of index " .. "'user_id_index' in space 'user_collection'" test:is(err, err_exp, @@ -1160,7 +1161,7 @@ local function run_queries(gql_wrapper, virtbox, meta) } } local result = gql_mutation_update_5:execute(variables_update_5) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local err_exp = "Attempt to modify a tuple field which is part of index " .. "'order_id_index' in space 'order_collection'" test:is(err, err_exp, @@ -1278,7 +1279,7 @@ local function run_queries(gql_wrapper, virtbox, meta) ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_delete) local err_exp = 'Non-existent argument "delete"' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, '"delete" argument is forbidden in a query') -- }}} @@ -1306,7 +1307,7 @@ local function run_queries_avro_schema_2(test, enable_mutations, gql_wrapper, else local err_exp = 'Variable specifies unknown type ' .. '"user_collection_insert"' - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, 'mutations are forbidden for avro-schema-2*') end end diff --git a/test/common/query_timeout.test.lua b/test/common/query_timeout.test.lua index 17eea35..536501c 100755 --- a/test/common/query_timeout.test.lua +++ b/test/common/query_timeout.test.lua @@ -34,7 +34,7 @@ local function run_queries(gql_wrapper) local result = gql_query:execute(variables) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message test:like(err, 'query execution timeout exceeded', 'timeout test') assert(test:check(), 'check plan') diff --git a/test/space/init_fail.test.lua b/test/space/init_fail.test.lua index 8b9d616..78504f1 100755 --- a/test/space/init_fail.test.lua +++ b/test/space/init_fail.test.lua @@ -9,8 +9,8 @@ package.path = fio.abspath(debug.getinfo(1).source:match("@?(.*/)") local tap = require('tap') local graphql = require('graphql') +local utils = require('graphql.utils') local testdata = require('test.testdata.compound_index_testdata') -local test_utils = require('test.test_utils') -- init box, upload test data and acquire metadata -- ----------------------------------------------- @@ -58,7 +58,7 @@ local err_exp = '1:1 connection "user_connection" of collection ' .. '"order_collection" has less fields than the index of ' .. '"user_str_num_index" collection (cannot prove uniqueness of the partial ' .. 'index)' -test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, +test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, 'not enough fields') -- restore back cut part @@ -83,7 +83,7 @@ local ok, err = pcall(create_gql_wrapper, metadata) local err_exp = 'several indexes were marked as primary in the ' .. '"user_collection" collection, at least "user_str_num_index" and ' .. '"user_str_index"' -test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, +test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, 'multiple primary indexes') -- restore metadata back diff --git a/test/test_utils.lua b/test/test_utils.lua index a72bf99..8664a96 100644 --- a/test/test_utils.lua +++ b/test/test_utils.lua @@ -99,12 +99,6 @@ function test_utils.major_avro_schema_version() return model.get_types == nil and 2 or 3 end --- return an error w/o file name and line number -function test_utils.strip_error(err) - local res = tostring(err):gsub('^.-:.-: (.*)$', '%1') - return res -end - function test_utils.graphql_from_testdata(testdata, shard, graphql_opts) local graphql_opts = graphql_opts or {} local meta = testdata.meta or testdata.get_test_metadata() diff --git a/test/testdata/common_testdata.lua b/test/testdata/common_testdata.lua index 2a7ca9b..464d664 100644 --- a/test/testdata/common_testdata.lua +++ b/test/testdata/common_testdata.lua @@ -1,6 +1,7 @@ local tap = require('tap') local json = require('json') local yaml = require('yaml') +local utils = require('graphql.utils') local test_utils = require('test.test_utils') local common_testdata = {} @@ -452,7 +453,7 @@ function common_testdata.run_queries(gql_wrapper) local err_exp = 'Cannot have more than one operation when using ' .. 'anonymous operations' local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_1tn) - test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, 'unnamed query should be a single one') local query_1t = [[ @@ -482,12 +483,12 @@ function common_testdata.run_queries(gql_wrapper) local err_exp = 'Operation name must be specified if more than one ' .. 'operation exists.' local result = gql_query_1t:execute({}) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message test:is(err, err_exp, 'non-determined query name should give an error') local err_exp = 'Unknown operation "non_existent_operation"' local result = gql_query_1t:execute({}, 'non_existent_operation') - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message test:is(err, err_exp, 'wrong operation name should give an error') test_utils.show_trace(function() @@ -1162,7 +1163,7 @@ function common_testdata.run_queries(gql_wrapper) return gql_wrapper:compile(query_7) end) - local result = {ok = ok, err = test_utils.strip_error(err)} + local result = {ok = ok, err = utils.strip_error(err)} test:is_deeply(result, exp_result_7, '7') -- should fail @@ -1188,7 +1189,7 @@ function common_testdata.run_queries(gql_wrapper) return gql_wrapper:compile(query_8) end) - local result = {ok = ok, err = test_utils.strip_error(err)} + local result = {ok = ok, err = utils.strip_error(err)} test:is_deeply(result, exp_result_8, '8') -- should fail @@ -1214,7 +1215,7 @@ function common_testdata.run_queries(gql_wrapper) return gql_wrapper:compile(query_9) end) - local result = {ok = ok, err = test_utils.strip_error(err)} + local result = {ok = ok, err = utils.strip_error(err)} test:is_deeply(result, exp_result_9, '9') -- }}} @@ -1240,7 +1241,7 @@ function common_testdata.run_queries(gql_wrapper) return gql_wrapper:compile(query_10) end) - local result = {ok = ok, err = test_utils.strip_error(err)} + local result = {ok = ok, err = utils.strip_error(err)} test:is_deeply(result, exp_result_10, 'scalar with fields is forbidden') local query_11 = [[ @@ -1261,7 +1262,7 @@ function common_testdata.run_queries(gql_wrapper) return gql_wrapper:compile(query_11) end) - local result = {ok = ok, err = test_utils.strip_error(err)} + local result = {ok = ok, err = utils.strip_error(err)} test:is_deeply(result, exp_result_11, 'complex without fields is forbidden') -- }}} diff --git a/test/testdata/compound_index_testdata.lua b/test/testdata/compound_index_testdata.lua index eec65c5..917c05b 100644 --- a/test/testdata/compound_index_testdata.lua +++ b/test/testdata/compound_index_testdata.lua @@ -1231,7 +1231,7 @@ function compound_index_testdata.run_queries(gql_wrapper) } } local result = gql_query_4:execute(variables_4_2) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local exp_err = 'offset by a partial key is forbidden' test:is(err, exp_err, '4_2') @@ -1309,7 +1309,7 @@ function compound_index_testdata.run_queries(gql_wrapper) } } local result = gql_query_5:execute(variables_5_2) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local exp_err = 'offset by a partial key is forbidden: ' .. 'expected "order_num" field' test:is(err, exp_err, '5_2') diff --git a/test/testdata/nullable_1_1_conn_testdata.lua b/test/testdata/nullable_1_1_conn_testdata.lua index 6f9863e..b086c3f 100644 --- a/test/testdata/nullable_1_1_conn_testdata.lua +++ b/test/testdata/nullable_1_1_conn_testdata.lua @@ -393,7 +393,7 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) local variables_upside_x = {body = 'x'} local result = gql_query_upside:execute(variables_upside_x) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local exp_err = 'FULL MATCH constraint was failed: connection key parts ' .. 'must be all non-nulls or all nulls; object: ' .. '{"domain":"graphql.tarantool.org",' .. @@ -404,7 +404,7 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) local variables_upside_y = {body = 'y'} local result = gql_query_upside:execute(variables_upside_y) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local exp_err = 'FULL MATCH constraint was failed: connection key parts ' .. 'must be all non-nulls or all nulls; object: ' .. '{"domain":"graphql.tarantool.org",' .. @@ -431,7 +431,7 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) else local variables_upside_z = {body = 'z'} local result = gql_query_upside:execute(variables_upside_z) - local err = test_utils.strip_error(result.errors[1].message) + local err = result.errors[1].message local exp_err = 'FULL MATCH constraint was failed: we expect 1 ' .. 'tuples, got 0' test:is(err, exp_err, 'upside_z constraint violation') diff --git a/test/testdata/nullable_index_testdata.lua b/test/testdata/nullable_index_testdata.lua index b19396b..aef41ed 100644 --- a/test/testdata/nullable_index_testdata.lua +++ b/test/testdata/nullable_index_testdata.lua @@ -6,6 +6,7 @@ local tap = require('tap') local json = require('json') local yaml = require('yaml') +local utils = require('graphql.utils') local test_utils = require('test.test_utils') local nullable_index_testdata = {} @@ -319,7 +320,7 @@ function nullable_index_testdata.run_queries(gql_wrapper) return gql_wrapper:compile(query_1) end) - local result = {ok = ok, err = test_utils.strip_error(err)} + local result = {ok = ok, err = utils.strip_error(err)} local exp_result = yaml.decode(([[ --- ok: false From aa1ee349840000f8466072afef1829db108fd48e Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 12:54:55 +0300 Subject: [PATCH 05/20] More strict test for disable_dangling_check flag We lean on gql_wrapper.disable_dangling_check value before, but now we don't. --- .../common/nullable_1_1_conn_nocheck.test.lua | 44 +++++++++++++++++++ test/testdata/nullable_1_1_conn_testdata.lua | 32 ++++---------- 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/test/common/nullable_1_1_conn_nocheck.test.lua b/test/common/nullable_1_1_conn_nocheck.test.lua index 1d2b69b..7a7e5f4 100755 --- a/test/common/nullable_1_1_conn_nocheck.test.lua +++ b/test/common/nullable_1_1_conn_nocheck.test.lua @@ -6,12 +6,56 @@ local fio = require('fio') package.path = fio.abspath(debug.getinfo(1).source:match("@?(.*/)") :gsub('/./', '/'):gsub('/+$', '')) .. '/../../?.lua' .. ';' .. package.path +local tap = require('tap') +local yaml = require('yaml') local test_utils = require('test.test_utils') local testdata = require('test.testdata.nullable_1_1_conn_testdata') box.cfg({}) +local function run_queries(gql_wrapper) + local test = tap.test('nullable_1_1_conn_nocheck') + test:plan(1) + + local query_upside = [[ + query emails_trace_upside($body: String, $child_domain: String) { + email(body: $body) { + body + in_reply_to(domain: $child_domain) { + body + in_reply_to { + body + in_reply_to { + body + } + } + } + } + } + ]] + + local gql_query_upside = gql_wrapper:compile(query_upside) + + -- Check we don't get an error re dangling 1:1 connection when + -- `disable_dangling_check` is set. + local variables_upside_z = {body = 'z'} + local result = test_utils.show_trace(function() + return gql_query_upside:execute(variables_upside_z) + end) + + local exp_result = yaml.decode(([[ + --- + email: + - body: z + ]]):strip()) + + test:is_deeply(result.data, exp_result, 'upside_z disabled constraint check') + + assert(test:check(), 'check plan') +end + test_utils.run_testdata(testdata, { + run_queries = run_queries, graphql_opts = { disable_dangling_check = true, }, diff --git a/test/testdata/nullable_1_1_conn_testdata.lua b/test/testdata/nullable_1_1_conn_testdata.lua index b086c3f..629b029 100644 --- a/test/testdata/nullable_1_1_conn_testdata.lua +++ b/test/testdata/nullable_1_1_conn_testdata.lua @@ -413,29 +413,15 @@ function nullable_1_1_conn_testdata.run_queries(gql_wrapper) '"in_reply_to_localpart":"1f70391f6ba858129413bd801b12acbf"}' test:is(err, exp_err, 'upside_y') - -- Check we get an error when trying to use dangling 1:1 connection. Check - -- we don't get this error when `disable_dangling_check` is set. - if gql_wrapper.disable_dangling_check then - local variables_upside_z = {body = 'z'} - local result = test_utils.show_trace(function() - return gql_query_upside:execute(variables_upside_z) - end) - - local exp_result = yaml.decode(([[ - --- - email: - - body: z - ]]):strip()) - - test:is_deeply(result.data, exp_result, 'upside_z disabled constraint check') - else - local variables_upside_z = {body = 'z'} - local result = gql_query_upside:execute(variables_upside_z) - local err = result.errors[1].message - local exp_err = 'FULL MATCH constraint was failed: we expect 1 ' .. - 'tuples, got 0' - test:is(err, exp_err, 'upside_z constraint violation') - end + -- Check we get an error when trying to use dangling 1:1 connection. + -- See nullable_1_1_conn_nocheck.test.lua for the case when + -- `disable_dangling_check` is set. + local variables_upside_z = {body = 'z'} + local result = gql_query_upside:execute(variables_upside_z) + local err = result.errors[1].message + local exp_err = 'FULL MATCH constraint was failed: we expect 1 ' .. + 'tuples, got 0' + test:is(err, exp_err, 'upside_z constraint violation') -- We can got zero objects by 1:1 connection when use filters, it is not -- violation of FULL MATCH constraint, because we found corresponding From 6bc887ee677cc3627a348795a6583b60f5737b84 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 13:18:56 +0300 Subject: [PATCH 06/20] Rephrased limit and timeout error messages --- graphql/accessor_general.lua | 14 +++++++------- test/common/limit_result.test.lua | 12 ++++++------ test/common/query_timeout.test.lua | 3 ++- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/graphql/accessor_general.lua b/graphql/accessor_general.lua index d3bdce3..26a1fc5 100644 --- a/graphql/accessor_general.lua +++ b/graphql/accessor_general.lua @@ -857,11 +857,11 @@ local function process_tuple(self, state, tuple, opts) local fetched_object_cnt_max = opts.fetched_object_cnt_max qstats.fetched_object_cnt = qstats.fetched_object_cnt + 1 assert(qstats.fetched_object_cnt <= fetched_object_cnt_max, - ('fetched object count[%d] exceeds limit[%d] ' .. - '(`fetched_object_cnt_max` in accessor)'):format( - qstats.fetched_object_cnt, fetched_object_cnt_max)) + ('fetched object count (%d) exceeds fetched_object_cnt_max limit (%d)') + :format(qstats.fetched_object_cnt, fetched_object_cnt_max)) assert(qcontext.deadline_clock > clock.monotonic64(), - 'query execution timeout exceeded, use `timeout_ms` to increase it') + ('query execution timeout exceeded timeout_ms limit (%s ms)'):format( + tostring(self.settings.timeout_ms))) local collection_name = opts.collection_name local pcre = opts.pcre local resolveField = opts.resolveField @@ -906,9 +906,9 @@ local function process_tuple(self, state, tuple, opts) state.count = state.count + 1 qstats.resulting_object_cnt = qstats.resulting_object_cnt + 1 assert(qstats.resulting_object_cnt <= resulting_object_cnt_max, - ('returning object count[%d] exceeds limit[%d] ' .. - '(`resulting_object_cnt_max` in accessor)'):format( - qstats.resulting_object_cnt, resulting_object_cnt_max)) + ('resulting objects count (%d) exceeds resulting_object_cnt_max ' .. + 'limit (%d)'):format(qstats.resulting_object_cnt, + resulting_object_cnt_max)) if limit ~= nil and state.count >= limit then return false end diff --git a/test/common/limit_result.test.lua b/test/common/limit_result.test.lua index bd4e18c..df4250e 100755 --- a/test/common/limit_result.test.lua +++ b/test/common/limit_result.test.lua @@ -37,9 +37,9 @@ local function run_queries(gql_wrapper) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") local err = result.errors[1].message - test:like(err, - 'count%[4%] exceeds limit%[3%] %(`resulting_object_cnt_max`', - 'resulting_object_cnt_max test') + test:is(err, + 'resulting objects count (4) exceeds resulting_object_cnt_max ' .. + 'limit (3)', 'resulting_object_cnt_max test') variables = { user_id = 5, @@ -49,9 +49,9 @@ local function run_queries(gql_wrapper) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") local err = result.errors[1].message - test:like(err, - 'count%[6%] exceeds limit%[5%] %(`fetched_object_cnt_max`', - 'resulting_object_cnt_max test') + test:is(err, + 'fetched object count (6) exceeds fetched_object_cnt_max limit (5)', + 'fetched_object_cnt_max test') assert(test:check(), 'check plan') end diff --git a/test/common/query_timeout.test.lua b/test/common/query_timeout.test.lua index 536501c..6c543ad 100755 --- a/test/common/query_timeout.test.lua +++ b/test/common/query_timeout.test.lua @@ -35,7 +35,8 @@ local function run_queries(gql_wrapper) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") local err = result.errors[1].message - test:like(err, 'query execution timeout exceeded', 'timeout test') + test:is(err, 'query execution timeout exceeded timeout_ms limit (0.001 ms)', + 'timeout test') assert(test:check(), 'check plan') end From 778807bf7a74ae252dd69185e3aae5c1e9cc7cbe Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 13:39:27 +0300 Subject: [PATCH 07/20] Add traceback to an execute error --- graphql/impl.lua | 12 +++++++++--- graphql/utils.lua | 15 ++++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/graphql/impl.lua b/graphql/impl.lua index 3e73a3d..93a8062 100644 --- a/graphql/impl.lua +++ b/graphql/impl.lua @@ -41,10 +41,16 @@ local function gql_execute(qstate, variables, operation_name) local root_value = {} - local ok, data = pcall(execute, state.schema, qstate.ast, root_value, - variables, operation_name) + local traceback + local ok, data = xpcall(function() + return execute(state.schema, qstate.ast, root_value, variables, + operation_name) + end, function(err) + traceback = debug.traceback() + return err + end) if not ok then - local err = utils.serialize_error(data) + local err = utils.serialize_error(data, traceback) return {errors = {err}} end return { diff --git a/graphql/utils.lua b/graphql/utils.lua index 281c728..2982818 100644 --- a/graphql/utils.lua +++ b/graphql/utils.lua @@ -225,13 +225,21 @@ function utils.optional_require_rex() return rex, is_pcre2 end -function utils.serialize_error(err) +function utils.serialize_error(err, traceback) if type(err) == 'string' then - return {message = utils.strip_error(err)} + return { + message = utils.strip_error(err), + traceback = traceback, + } elseif type(err) == 'cdata' and tostring(ffi.typeof(err)) == 'ctype' then - return {message = tostring(err)} + return { + message = tostring(err), + traceback = traceback, + } elseif type(err) == 'table' and type(err.message) == 'string' then + local err = table.copy(err) + err.traceback = traceback return err end @@ -243,6 +251,7 @@ function utils.serialize_error(err) return { message = message, orig_error = orig_error, + traceback = traceback, } end From 69040ca963866da7eba1fed03c8818e904241e99 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Thu, 21 Jun 2018 18:46:23 +0300 Subject: [PATCH 08/20] Use `extensions` field for custom error fields It is needed to avoid possible name clash with future versions of GraphQL standard. June, 2018 version of the standard strongly recommends to use `extensions` field for custom fields of an error. [1]: https://github.com/facebook/graphql/pull/407/files [2]: https://github.com/facebook/graphql/releases/tag/June2018 --- graphql/server/server.lua | 12 ++++++++++-- graphql/utils.lua | 11 ++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/graphql/server/server.lua b/graphql/server/server.lua index b7ece3f..999ef93 100644 --- a/graphql/server/server.lua +++ b/graphql/server/server.lua @@ -1,4 +1,5 @@ local fio = require('fio') +local graphql_utils = require('graphql.utils') local utils = require('graphql.server.utils') local json = require('json') @@ -100,11 +101,18 @@ function server.init(graphql, host, port) local query = parsed.query - local ok, compiled_query = pcall(graphql.compile, graphql, query) + local traceback + local ok, compiled_query = xpcall(function() + return graphql:compile(query) + end, function(err) + traceback = debug.traceback() + return err + end) if not ok then + local err = graphql_utils.serialize_error(compiled_query, traceback) return { status = 200, - body = json.encode({errors = {{message = compiled_query}}}) + body = json.encode({errors = {err}}) } end diff --git a/graphql/utils.lua b/graphql/utils.lua index 2982818..eb46e0b 100644 --- a/graphql/utils.lua +++ b/graphql/utils.lua @@ -226,20 +226,21 @@ function utils.optional_require_rex() end function utils.serialize_error(err, traceback) + local extensions = {traceback = traceback} if type(err) == 'string' then return { message = utils.strip_error(err), - traceback = traceback, + extensions = extensions, } elseif type(err) == 'cdata' and tostring(ffi.typeof(err)) == 'ctype' then return { message = tostring(err), - traceback = traceback, + extensions = extensions, } elseif type(err) == 'table' and type(err.message) == 'string' then local err = table.copy(err) - err.traceback = traceback + err.extensions = extensions return err end @@ -248,10 +249,10 @@ function utils.serialize_error(err, traceback) json.cfg({encode_use_tostring = true}) local orig_error = json.encode(err) json.cfg({encode_use_tostring = encode_use_tostring_orig}) + extensions.orig_error = orig_error return { message = message, - orig_error = orig_error, - traceback = traceback, + extensions = extensions, } end From 60ad23ee0a3dcc29a52a8a6ecd9ed12eccb40b32 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Mon, 25 Jun 2018 16:50:57 +0300 Subject: [PATCH 09/20] Show statistics in a query result meta Fixed resulting_object_cnt counter in case of a filter by 1:1 connection. Part of #71. --- graphql/accessor_general.lua | 26 +++++++++++++++++++++++++- graphql/core/execute.lua | 5 +++-- graphql/impl.lua | 6 +++++- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/graphql/accessor_general.lua b/graphql/accessor_general.lua index 26a1fc5..c3fbb08 100644 --- a/graphql/accessor_general.lua +++ b/graphql/accessor_general.lua @@ -878,6 +878,12 @@ local function process_tuple(self, state, tuple, opts) return true -- skip pivot item too end + -- Don't count subrequest resulting objects (needed for filtering) into + -- count of object we show to an user as a result. + -- XXX: It is better to have an option to control whether selected objects + -- will be counted as resulting ones. + local saved_resulting_object_cnt = qstats.resulting_object_cnt + -- make subrequests if needed for k, v in pairs(filter) do if obj[k] == nil then @@ -891,6 +897,8 @@ local function process_tuple(self, state, tuple, opts) end end + qstats.resulting_object_cnt = saved_resulting_object_cnt + -- filter out non-matching objects local match = utils.is_subtable(obj, filter) and match_using_re(obj, pcre) @@ -1060,6 +1068,13 @@ local function select_internal(self, collection_name, from, filter, args, extra) -- fullscan local primary_index = self.funcs.get_primary_index(self, collection_name) + + -- count full scan select request + extra.qcontext.statistics.select_requests_cnt = + extra.qcontext.statistics.select_requests_cnt + 1 + extra.qcontext.statistics.full_scan_select_requests_cnt = + extra.qcontext.statistics.full_scan_select_requests_cnt + 1 + for _, tuple in primary_index:pairs() do assert(pivot == nil, 'offset for top-level objects must use a primary index') @@ -1102,6 +1117,12 @@ local function select_internal(self, collection_name, from, filter, args, extra) local tuple_count = 0 + -- count index select request + extra.qcontext.statistics.select_requests_cnt = + extra.qcontext.statistics.select_requests_cnt + 1 + extra.qcontext.statistics.index_select_requests_cnt = + extra.qcontext.statistics.index_select_requests_cnt + 1 + for _, tuple in index:pairs(index_value, iterator_opts) do tuple_count = tuple_count + 1 -- check full match constraint @@ -1287,7 +1308,10 @@ local function init_qcontext(accessor, qcontext) local settings = accessor.settings qcontext.statistics = { resulting_object_cnt = 0, - fetched_object_cnt = 0 + fetched_object_cnt = 0, + select_requests_cnt = 0, + full_scan_select_requests_cnt = 0, + index_select_requests_cnt = 0, } qcontext.deadline_clock = clock.monotonic64() + settings.timeout_ms * 1000 * 1000 diff --git a/graphql/core/execute.lua b/graphql/core/execute.lua index 7a10099..d1bfe47 100644 --- a/graphql/core/execute.lua +++ b/graphql/core/execute.lua @@ -125,11 +125,12 @@ evaluateSelections = function(objectType, object, selections, context) return result end -return function(schema, tree, rootValue, variables, operationName) +return function(schema, tree, rootValue, variables, operationName, opts) + local opts = opts or {} local context = query_util.buildContext(schema, tree, rootValue, variables, operationName) -- The field is passed to resolve function within info attribute. -- Can be used to store any data within one query. - context.qcontext = {} + context.qcontext = opts.qcontext or {} local rootType = schema[context.operation.operation] if not rootType then diff --git a/graphql/impl.lua b/graphql/impl.lua index 93a8062..366017a 100644 --- a/graphql/impl.lua +++ b/graphql/impl.lua @@ -40,11 +40,12 @@ local function gql_execute(qstate, variables, operation_name) check(operation_name, 'operation_name', 'string', 'nil') local root_value = {} + local qcontext = {} local traceback local ok, data = xpcall(function() return execute(state.schema, qstate.ast, root_value, variables, - operation_name) + operation_name, {qcontext = qcontext}) end, function(err) traceback = debug.traceback() return err @@ -55,6 +56,9 @@ local function gql_execute(qstate, variables, operation_name) end return { data = data, + meta = { + statistics = qcontext.statistics, + } } end From 2f383fec83971bfc4b7c4aaaa3bf4f052ebdc343 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Tue, 26 Jun 2018 11:18:37 +0300 Subject: [PATCH 10/20] Add execute.lua to luacheck, fix luacheck warnings --- Makefile | 1 + graphql/core/execute.lua | 30 +++++++++++++++++++----------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index b3ab03b..d4f461c 100644 --- a/Makefile +++ b/Makefile @@ -9,6 +9,7 @@ default: .PHONY: lint lint: luacheck graphql/*.lua \ + graphql/core/execute.lua \ graphql/convert_schema/*.lua \ graphql/server/*.lua \ test/bench/*.lua \ diff --git a/graphql/core/execute.lua b/graphql/core/execute.lua index d1bfe47..2513d96 100644 --- a/graphql/core/execute.lua +++ b/graphql/core/execute.lua @@ -1,5 +1,4 @@ local path = (...):gsub('%.[^%.]+$', '') -local types = require(path .. '.types') local util = require(path .. '.util') local introspection = require(path .. '.introspection') local query_util = require(path .. '.query_util') @@ -10,13 +9,21 @@ end local evaluateSelections --- @param[opt] resolvedType a type to be used instead of one returned by --- `fieldType.resolveType(result)` in case when the `fieldType` is Interface or --- Union; that is needed to increase flexibility of an union type resolving --- (e.g. resolving by a parent object instead of a current object) via --- returning it from the `fieldType.resolve` function, which called before --- `resolvedType` and may need to determine the type itself for its needs -local function completeValue(fieldType, result, subSelections, context, resolvedType) +-- @tparam[opt] table opts the following options: +-- +-- * fieldName (string; optional) +-- +-- * resolvedType (table; optional) resolvedType a type to be used instead of +-- one returned by `fieldType.resolveType(result)` in case when the +-- `fieldType` is Interface or Union; that is needed to increase flexibility +-- of an union type resolving (e.g. resolving by a parent object instead of a +-- current object) via returning it from the `fieldType.resolve` function, +-- which called before `resolvedType` and may need to determine the type +-- itself for its needs +local function completeValue(fieldType, result, subSelections, context, opts) + local opts = opts or {} + local resolvedType = opts.resolvedType + local fieldName = opts.fieldName or '???' local fieldTypeName = fieldType.__type if fieldTypeName == 'NonNull' then @@ -65,13 +72,13 @@ local function completeValue(fieldType, result, subSelections, context, resolved return evaluateSelections(objectType, result, subSelections, context) end - error('Unknown type "' .. fieldTypeName .. '" for field "' .. field.name .. '"') + error('Unknown type "' .. fieldTypeName .. '" for field "' .. fieldName .. '"') end local function getFieldEntry(objectType, object, fields, context) local firstField = fields[1] local fieldName = firstField.name.value - local responseKey = query_util.getFieldResponseKey(firstField) + -- local responseKey = query_util.getFieldResponseKey(firstField) local fieldType = introspection.fieldMap[fieldName] or objectType.fields[fieldName] if fieldType == nil then @@ -110,7 +117,8 @@ local function getFieldEntry(objectType, object, fields, context) local resolvedObject, resolvedType = (fieldType.resolve or defaultResolver)(object, arguments, info) local subSelections = query_util.mergeSelectionSets(fields) - return completeValue(fieldType.kind, resolvedObject, subSelections, context, resolvedType) + return completeValue(fieldType.kind, resolvedObject, subSelections, context, + {resolvedType = resolvedType}) end evaluateSelections = function(objectType, object, selections, context) From d2c2dceb4ef5913adf1cbfd161e020f1716389bb Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Tue, 26 Jun 2018 11:23:53 +0300 Subject: [PATCH 11/20] Deduplicate typeFromAST code --- graphql/core/query_util.lua | 4 ++-- graphql/core/rules.lua | 18 +++--------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/graphql/core/query_util.lua b/graphql/core/query_util.lua index 06d69d2..e26aec1 100644 --- a/graphql/core/query_util.lua +++ b/graphql/core/query_util.lua @@ -7,10 +7,10 @@ local query_util = {} function query_util.typeFromAST(node, schema) local innerType if node.kind == 'listType' then - innerType = query_util.typeFromAST(node.type) + innerType = query_util.typeFromAST(node.type, schema) return innerType and types.list(innerType) elseif node.kind == 'nonNullType' then - innerType = query_util.typeFromAST(node.type) + innerType = query_util.typeFromAST(node.type, schema) return innerType and types.nonNull(innerType) else assert(node.kind == 'namedType', 'Variable must be a named type') diff --git a/graphql/core/rules.lua b/graphql/core/rules.lua index e324d54..76f1d3b 100644 --- a/graphql/core/rules.lua +++ b/graphql/core/rules.lua @@ -3,6 +3,7 @@ local types = require(path .. '.types') local util = require(path .. '.util') local schema = require(path .. '.schema') local introspection = require(path .. '.introspection') +local query_util = require(path .. '.query_util') local function getParentField(context, name, count) if introspection.fieldMap[name] then return introspection.fieldMap[name] end @@ -490,21 +491,8 @@ function rules.variableUsageAllowed(node, context) local variableDefinition = variableMap[variableName] local hasDefault = variableDefinition.defaultValue ~= nil - local function typeFromAST(variable) - local innerType - if variable.kind == 'listType' then - innerType = typeFromAST(variable.type) - return innerType and types.list(innerType) - elseif variable.kind == 'nonNullType' then - innerType = typeFromAST(variable.type) - return innerType and types.nonNull(innerType) - else - assert(variable.kind == 'namedType', 'Variable must be a named type') - return context.schema:getType(variable.name.value) - end - end - - local variableType = typeFromAST(variableDefinition.type) + local variableType = query_util.typeFromAST(variableDefinition.type, + context.schema) if hasDefault and variableType.__type ~= 'NonNull' then variableType = types.nonNull(variableType) From 38fc560a725eed6485df7790654045cc5ab30802 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Tue, 26 Jun 2018 11:27:31 +0300 Subject: [PATCH 12/20] Validate a variable type Fixed Int type to be between -2^31 and 2^31-1 instead of -2^32 and 2^32-1. Forbid implicit fraction part removing from Int immediate value. Fixed Long type to forbid variable value of 'number' Lua type that cannot preciselly represent integral value (allow only ones inside -2^53 and 2^53 exclusivelly). Related to #159. Related to #160. --- Makefile | 1 + graphql/convert_schema/union.lua | 11 +- graphql/core/execute.lua | 3 + graphql/core/query_util.lua | 9 +- graphql/core/types.lua | 98 +++++++-- graphql/core/validate_variables.lua | 129 +++++++++++ graphql/gen_arguments.lua | 57 ++++- graphql/utils.lua | 5 +- test/common/introspection.test.lua | 6 +- test/common/mutation.test.lua | 8 +- test/common/pcre.test.lua | 1 - test/testdata/common_testdata.lua | 247 +++++++++++++++++++++- test/testdata/compound_index_testdata.lua | 5 +- 13 files changed, 540 insertions(+), 40 deletions(-) create mode 100644 graphql/core/validate_variables.lua diff --git a/Makefile b/Makefile index d4f461c..e42aaae 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,7 @@ default: lint: luacheck graphql/*.lua \ graphql/core/execute.lua \ + graphql/core/validate_variables.lua \ graphql/convert_schema/*.lua \ graphql/server/*.lua \ test/bench/*.lua \ diff --git a/graphql/convert_schema/union.lua b/graphql/convert_schema/union.lua index a9537a1..801160d 100644 --- a/graphql/convert_schema/union.lua +++ b/graphql/convert_schema/union.lua @@ -301,15 +301,18 @@ function union.convert(avro_schema, opts) types = union_types, name = helpers.full_name(union_name, context), resolveType = function(result) + assert(type(result) == 'table', + 'union value must be a map with one field, got ' .. + type(result)) + assert(next(result) ~= nil and next(result, next(result)) == nil, + 'union value must have only one field') for determinant, type in pairs(determinant_to_type) do if result[determinant] ~= nil then return type end end - error(('result object has no determinant field matching ' .. - 'determinants for this union\nresult object:\n%s' .. - 'determinants:\n%s'):format(yaml.encode(result), - yaml.encode(determinant_to_type))) + local field_name = tostring(next(result)) + error(('unexpected union value field: %s'):format(field_name)) end, resolveNodeType = function(node) assert(#node.values == 1, diff --git a/graphql/core/execute.lua b/graphql/core/execute.lua index 2513d96..6eadcb1 100644 --- a/graphql/core/execute.lua +++ b/graphql/core/execute.lua @@ -2,6 +2,7 @@ local path = (...):gsub('%.[^%.]+$', '') local util = require(path .. '.util') local introspection = require(path .. '.introspection') local query_util = require(path .. '.query_util') +local validate_variables = require(path .. '.validate_variables') local function defaultResolver(object, arguments, info) return object[info.fieldASTs[1].name.value] @@ -145,5 +146,7 @@ return function(schema, tree, rootValue, variables, operationName, opts) error('Unsupported operation "' .. context.operation.operation .. '"') end + validate_variables.validate_variables(context) + return evaluateSelections(rootType, rootValue, context.operation.selectionSet.selections, context) end diff --git a/graphql/core/query_util.lua b/graphql/core/query_util.lua index e26aec1..16f13db 100644 --- a/graphql/core/query_util.lua +++ b/graphql/core/query_util.lua @@ -111,7 +111,8 @@ function query_util.buildContext(schema, tree, rootValue, variables, operationNa rootValue = rootValue, variables = variables, operation = nil, - fragmentMap = {} + fragmentMap = {}, + variableTypes = {}, } for _, definition in ipairs(tree.definitions) do @@ -136,6 +137,12 @@ function query_util.buildContext(schema, tree, rootValue, variables, operationNa end end + -- Save variableTypes for the operation. + for _, definition in ipairs(context.operation.variableDefinitions or {}) do + context.variableTypes[definition.variable.name.value] = + query_util.typeFromAST(definition.type, context.schema) + end + return context end diff --git a/graphql/core/types.lua b/graphql/core/types.lua index f233257..743575f 100644 --- a/graphql/core/types.lua +++ b/graphql/core/types.lua @@ -1,3 +1,4 @@ +local ffi = require('ffi') local path = (...):gsub('%.[^%.]+$', '') local util = require(path .. '.util') @@ -55,7 +56,8 @@ function types.scalar(config) description = config.description, serialize = config.serialize, parseValue = config.parseValue, - parseLiteral = config.parseLiteral + parseLiteral = config.parseLiteral, + isValueOfTheType = config.isValueOfTheType, } instance.nonNull = types.nonNull(instance) @@ -253,6 +255,7 @@ function types.inputUnion(config) error('Literal parsing is implemented in util.coerceValue; ' .. 'we should not go here') end, + resolveType = config.resolveType, resolveNodeType = config.resolveNodeType, } @@ -261,14 +264,58 @@ function types.inputUnion(config) return instance end -local coerceInt = function(value) - value = tonumber(value) +-- Based on the code from tarantool/checks. +local function isInt(value) + if type(value) == 'number' then + return value >= -2^31 and value < 2^31 and math.floor(value) == value + end - if not value then return end + if type(value) == 'cdata' then + if ffi.istype('int64_t', value) then + return value >= -2^31 and value < 2^31 + elseif ffi.istype('uint64_t', value) then + return value < 2^31 + end + end + + return false +end - if value == value and value < 2 ^ 32 and value >= -2 ^ 32 then - return value < 0 and math.ceil(value) or math.floor(value) +-- The code from tarantool/checks. +local function isLong(value) + if type(value) == 'number' then + -- Double floating point format has 52 fraction bits. If we want to keep + -- integer precision, the number must be less than 2^53. + return value > -2^53 and value < 2^53 and math.floor(value) == value end + + if type(value) == 'cdata' then + if ffi.istype('int64_t', value) then + return true + elseif ffi.istype('uint64_t', value) then + return value < 2^63 + end + end + + return false +end + +local function coerceInt(value) + local value = tonumber(value) + + if value == nil then return end + if not isInt(value) then return end + + return value +end + +local function coerceLong(value) + local value = tonumber64(value) + + if value == nil then return end + if not isLong(value) then return end + + return value end types.int = types.scalar({ @@ -280,20 +327,22 @@ types.int = types.scalar({ if node.kind == 'int' then return coerceInt(node.value) end - end + end, + isValueOfTheType = isInt, }) types.long = types.scalar({ name = 'Long', - description = 'Long is non-bounded integral type', - serialize = function(value) return tonumber(value) end, - parseValue = function(value) return tonumber(value) end, + description = "The `Long` scalar type represents non-fractional signed whole numeric values. Long can represent values between -(2^63) and 2^63 - 1.", + serialize = coerceLong, + parseValue = coerceLong, parseLiteral = function(node) -- 'int' is name of the immediate value type if node.kind == 'int' then - return tonumber(node.value) + return coerceLong(node.value) end - end + end, + isValueOfTheType = isLong, }) types.float = types.scalar({ @@ -304,7 +353,10 @@ types.float = types.scalar({ if node.kind == 'float' or node.kind == 'int' then return tonumber(node.value) end - end + end, + isValueOfTheType = function(value) + return type(value) == 'number' + end, }) types.double = types.scalar({ @@ -316,7 +368,10 @@ types.double = types.scalar({ if node.kind == 'float' or node.kind == 'int' then return tonumber(node.value) end - end + end, + isValueOfTheType = function(value) + return type(value) == 'number' + end, }) types.string = types.scalar({ @@ -328,7 +383,10 @@ types.string = types.scalar({ if node.kind == 'string' then return node.value end - end + end, + isValueOfTheType = function(value) + return type(value) == 'string' + end, }) local function toboolean(x) @@ -346,7 +404,10 @@ types.boolean = types.scalar({ else return nil end - end + end, + isValueOfTheType = function(value) + return type(value) == 'boolean' + end, }) types.id = types.scalar({ @@ -355,7 +416,10 @@ types.id = types.scalar({ parseValue = tostring, parseLiteral = function(node) return node.kind == 'string' or node.kind == 'int' and node.value or nil - end + end, + isValueOfTheType = function(value) + error('NIY') + end, }) function types.directive(config) diff --git a/graphql/core/validate_variables.lua b/graphql/core/validate_variables.lua new file mode 100644 index 0000000..4d1894b --- /dev/null +++ b/graphql/core/validate_variables.lua @@ -0,0 +1,129 @@ +local types = require('graphql.core.types') +local graphql_utils = require('graphql.utils') + +local check = graphql_utils.check + +local validate_variables = {} + +-- Traverse type more or less likewise util.coerceValue do. +local function checkVariableValue(variableName, value, variableType) + check(variableName, 'variableName', 'string') + check(variableType, 'variableType', 'table') + + local isNonNull = variableType.__type == 'NonNull' + + if isNonNull then + variableType = types.nullable(variableType) + assert(value ~= nil, + ('Variable "%s" expected to be non-null'):format(variableName)) + end + + local isList = variableType.__type == 'List' + local isScalar = variableType.__type == 'Scalar' + local isInputObject = variableType.__type == 'InputObject' + local isInputMap = isScalar and variableType.subtype == 'InputMap' + local isInputUnion = isScalar and variableType.subtype == 'InputUnion' + + -- Nullable variable type + null value case: value can be nil only when + -- isNonNull is false. + if value == nil then return end + + if isList then + assert(type(value) == 'table', + ('Variable "%s" for a List must be a Lua table, got %s') + :format(variableName, type(value))) + assert(graphql_utils.is_array(value), + ('Variable "%s" for a List must be an array, got map') + :format(variableName)) + assert(variableType.ofType ~= nil, 'variableType.ofType must not be nil') + for i, item in ipairs(value) do + local itemName = variableName .. '[' .. tostring(i) .. ']' + checkVariableValue(itemName, item, variableType.ofType) + end + return + end + + if isInputObject then + assert(type(value) == 'table', + ('Variable "%s" for the InputObject "%s" must be a Lua table, ' .. + 'got %s'):format(variableName, variableType.name, type(value))) + + -- check all fields: as from value as well as from schema + local fieldNameSet = {} + for fieldName, _ in pairs(value) do + fieldNameSet[fieldName] = true + end + for fieldName, _ in pairs(variableType.fields) do + fieldNameSet[fieldName] = true + end + + for fieldName, _ in pairs(fieldNameSet) do + local fieldValue = value[fieldName] + assert(type(fieldName) == 'string', + ('Field key of the variable "%s" for the InputObject "%s" ' .. + 'must be a string, got %s'):format(variableName, variableType.name, + type(fieldName))) + assert(type(variableType.fields[fieldName]) ~= 'nil', + ('Unknown field "%s" of the variable "%s" for the ' .. + 'InputObject "%s"'):format(fieldName, variableName, variableType.name)) + + local childType = variableType.fields[fieldName].kind + local childName = variableName .. '.' .. fieldName + checkVariableValue(childName, fieldValue, childType) + end + + return + end + + if isInputMap then + assert(type(value) == 'table', + ('Variable "%s" for the InputMap "%s" must be a Lua table, got %s') + :format(variableName, variableType.name, type(value))) + + for fieldName, fieldValue in pairs(value) do + assert(type(fieldName) == 'string', + ('Field key of the variable "%s" for the InputMap "%s" must be a ' .. + 'string, got %s'):format(variableName, variableType.name, + type(fieldName))) + local childType = variableType.values + local childName = variableName .. '.' .. fieldName + checkVariableValue(childName, fieldValue, childType) + end + + return + end + + -- XXX: Enum + + if isInputUnion then + local childType = variableType.resolveType(value) + checkVariableValue(variableName, value, childType) + return + end + + if isScalar then + check(variableType.isValueOfTheType, 'isValueOfTheType', 'function') + assert(variableType.isValueOfTheType(value), + ('Wrong variable "%s" for the Scalar "%s"'):format( + variableName, variableType.name)) + return + end + + error(('Unknown type of the variable "%s"'):format(variableName)) +end + +function validate_variables.validate_variables(context) + -- check that all variable values have corresponding variable declaration + for variableName, _ in pairs(context.variables or {}) do + assert(context.variableTypes[variableName] ~= nil, + ('There is no declaration for the variable "%s"'):format(variableName)) + end + + -- check that variable values have correct type + for variableName, variableType in pairs(context.variableTypes) do + local value = (context.variables or {})[variableName] + checkVariableValue(variableName, value, variableType) + end +end + +return validate_variables diff --git a/graphql/gen_arguments.lua b/graphql/gen_arguments.lua index e6f1f5d..ee04a66 100644 --- a/graphql/gen_arguments.lua +++ b/graphql/gen_arguments.lua @@ -72,7 +72,8 @@ local function get_primary_key_type(db_schema, collection_name) end --- Make schema types deep nullable down to scalar, union, array or map ---- (matches xflatten input syntax). +--- (matches xflatten input syntax) and remove default values (from fields of +--- record and record* types). --- --- @param e_schema (table or string) avro-schema with expanded references --- @@ -97,6 +98,7 @@ local function recursive_nullable(e_schema, skip_cond) if new_type ~= nil then local field = table.copy(field) field.type = new_type + field.default = nil table.insert(res.fields, field) end end @@ -105,6 +107,7 @@ local function recursive_nullable(e_schema, skip_cond) elseif avro_t == 'union' or avro_t == 'array' or avro_t == 'array*' or avro_t == 'map' or avro_t == 'map*' then + -- it is non-recursive intentionally to match current xflatten semantics e_schema = table.copy(e_schema) return avro_helpers.make_avro_type_nullable(e_schema, {raise_on_nullable = false}) @@ -113,6 +116,56 @@ local function recursive_nullable(e_schema, skip_cond) error('unrecognized avro-schema type: ' .. json.encode(e_schema)) end +--- Remove default values from passed avro-schema (from fields of record and +--- record* types) and make they nullable. +--- +--- @param e_schema (table or string) avro-schema with expanded references +--- +--- @return transformed avro-schema +local function recursive_replace_default_with_nullable(e_schema) + local avro_t = avro_helpers.avro_type(e_schema) + + if avro_helpers.is_scalar_type(avro_t) then + return e_schema + elseif avro_t == 'record' or avro_t == 'record*' then + local res = table.copy(e_schema) + res.fields = {} + + for _, field in ipairs(e_schema.fields) do + field = table.copy(field) + if type(field.default) ~= 'nil' then + field.default = nil + field.type = avro_helpers.make_avro_type_nullable(field.type, + {raise_on_nullable = false}) + end + field.type = recursive_replace_default_with_nullable(field.type) + table.insert(res.fields, field) + end + + return res + elseif avro_t == 'union' then + local res = {} + + for _, child in ipairs(e_schema) do + local new_child_type = + recursive_replace_default_with_nullable(child) + table.insert(res, new_child_type) + end + + return res + elseif avro_t == 'array' or avro_t == 'array*' then + local res = table.copy(e_schema) + res.items = recursive_replace_default_with_nullable(e_schema.items) + return res + elseif avro_t == 'map' or avro_t == 'map*' then + local res = table.copy(e_schema) + res.values = recursive_replace_default_with_nullable(e_schema.values) + return res + end + + error('unrecognized avro-schema type: ' .. json.encode(e_schema)) +end + --- Whether we can compare the type for equallity. --- --- @tparam string avro_schema_type @@ -320,7 +373,7 @@ function gen_arguments.extra_args(db_schema, collection_name, opts) collection_name) local e_schema = db_schema.e_schemas[schema_name] - local schema_insert = table.copy(e_schema) + local schema_insert = recursive_replace_default_with_nullable(e_schema) schema_insert.name = collection_name .. '_insert' schema_insert.type = 'record*' -- make the record nullable diff --git a/graphql/utils.lua b/graphql/utils.lua index eb46e0b..89f40e0 100644 --- a/graphql/utils.lua +++ b/graphql/utils.lua @@ -39,9 +39,8 @@ end --- case) --- @return[2] `false` otherwise function utils.is_array(table) - if type(table) ~= 'table' then - return false - end + utils.check(table, 'table', 'table') + local max = 0 local count = 0 for k, _ in pairs(table) do diff --git a/test/common/introspection.test.lua b/test/common/introspection.test.lua index c004321..791e883 100755 --- a/test/common/introspection.test.lua +++ b/test/common/introspection.test.lua @@ -512,10 +512,8 @@ local function run_queries(gql_wrapper) kind: NON_NULL name: price - type: - ofType: - name: Boolean - kind: SCALAR - kind: NON_NULL + name: Boolean + kind: SCALAR name: in_stock - type: ofType: diff --git a/test/common/mutation.test.lua b/test/common/mutation.test.lua index 1437501..7b06e29 100755 --- a/test/common/mutation.test.lua +++ b/test/common/mutation.test.lua @@ -1137,8 +1137,8 @@ local function run_queries(gql_wrapper, virtbox, meta) } local result = gql_mutation_update_4:execute(variables_update_4) local err = result.errors[1].message - local err_exp = "Attempt to modify a tuple field which is part of index " .. - "'user_id_index' in space 'user_collection'" + local err_exp = 'Unknown field "user_id" of the variable "xuser" ' .. + 'for the InputObject "user_collection_update"' test:is(err, err_exp, 'updating of a field of a primary key when it is NOT shard key field') @@ -1162,8 +1162,8 @@ local function run_queries(gql_wrapper, virtbox, meta) } local result = gql_mutation_update_5:execute(variables_update_5) local err = result.errors[1].message - local err_exp = "Attempt to modify a tuple field which is part of index " .. - "'order_id_index' in space 'order_collection'" + local err_exp = 'Unknown field "order_id" of the variable "xorder" ' .. + 'for the InputObject "order_collection_update"' test:is(err, err_exp, 'updating of a field of a primary key when it is shard key field') diff --git a/test/common/pcre.test.lua b/test/common/pcre.test.lua index b6d6a1c..4d9395f 100755 --- a/test/common/pcre.test.lua +++ b/test/common/pcre.test.lua @@ -56,7 +56,6 @@ local function run_queries(gql_wrapper) -- {{{ offset + regexp match local variables_1_2 = { - user_id = 'user_id_1', first_name_re = '^V', } diff --git a/test/testdata/common_testdata.lua b/test/testdata/common_testdata.lua index 464d664..9f7f435 100644 --- a/test/testdata/common_testdata.lua +++ b/test/testdata/common_testdata.lua @@ -360,7 +360,7 @@ end function common_testdata.run_queries(gql_wrapper) local test = tap.test('common') - test:plan(27) + test:plan(46) local query_1 = [[ query user_by_order($order_id: String) { @@ -1267,6 +1267,251 @@ function common_testdata.run_queries(gql_wrapper) -- }}} + -- {{{ fail cases for a variable type + + local query_12 = [[ + query order($order_id: String!) { + order_collection(order_id: $order_id) { + order_id + } + } + ]] + local gql_query_12 = test_utils.show_trace(function() + return gql_wrapper:compile(query_12) + end) + + -- {{{ non-null, scalar type mismatch + + local variables_12_1 = {} + local exp_err = 'Variable "order_id" expected to be non-null' + local result = gql_query_12:execute(variables_12_1) + local err = result.errors[1].message + test:is(err, exp_err, 'nil for a non-null type') + + local variables_12_2 = {order_id = box.NULL} + local exp_err = 'Variable "order_id" expected to be non-null' + local result = gql_query_12:execute(variables_12_2) + local err = result.errors[1].message + test:is(err, exp_err, 'box.NULL for a non-null type') + + local variables_12_3 = {order_id = 42} + local exp_err = 'Wrong variable "order_id" for the Scalar "String"' + local result = gql_query_12:execute(variables_12_3) + local err = result.errors[1].message + test:is(err, exp_err, 'Int for a String type') + + -- }}} + + local query_13 = [[ + mutation($xorder_metainfo: order_metainfo_collection_update) { + order_metainfo_collection(update: $xorder_metainfo, limit: 1) { + order_metainfo_id + } + } + ]] + local gql_query_13 = test_utils.show_trace(function() + return gql_wrapper:compile(query_13) + end) + + -- {{{ List + + local variables_13_1 = {xorder_metainfo = {store = {tags = '123'}}} + local exp_err = 'Variable "xorder_metainfo.store.tags" for a List ' .. + 'must be a Lua table, got string' + local result = gql_query_13:execute(variables_13_1) + local err = result.errors[1].message + test:is(err, exp_err, 'String for a List type') + + local variables_13_2 = {xorder_metainfo = {store = {tags = {1, 2, 3}}}} + local exp_err = 'Wrong variable "xorder_metainfo.store.tags[1]" for ' .. + 'the Scalar "String"' + local result = gql_query_13:execute(variables_13_2) + local err = result.errors[1].message + test:is(err, exp_err, 'wrong List value type') + + local variables_13_3 = {xorder_metainfo = {store = {tags = {foo = 'bar'}}}} + local exp_err = 'Variable "xorder_metainfo.store.tags" for a List ' .. + 'must be an array, got map' + local result = gql_query_13:execute(variables_13_3) + local err = result.errors[1].message + test:is(err, exp_err, 'map for a List type') + + -- }}} + -- {{{ InputObject + + local variables_13_4 = {xorder_metainfo = {store = {address = 42}}} + local exp_err = 'Variable "xorder_metainfo.store.address" for the ' .. + 'InputObject "arguments___order_metainfo_collection___update' .. + '___order_metainfo_collection_update___store___store___address' .. + '___address" must be a Lua table, got number' + local result = gql_query_13:execute(variables_13_4) + local err = result.errors[1].message + test:is(err, exp_err, 'Int for an InputObject type') + + local variables_13_5 = {xorder_metainfo = {store = { + address = {'foo', 'bar', 'baz'}}}} + local exp_err = 'Field key of the variable "xorder_metainfo.store.' .. + 'address" for the InputObject "arguments___order_metainfo_' .. + 'collection___update___order_metainfo_collection_update___' .. + 'store___store___address___address" must be a string, got number' + local result = gql_query_13:execute(variables_13_5) + local err = result.errors[1].message + test:is(err, exp_err, 'List for an InputObject type') + + local variables_13_6 = {xorder_metainfo = {store = { + address = { + street = 'street', + city = 'city', + state = 'state', + zip = 42, + } + }}} + local exp_err = 'Wrong variable "xorder_metainfo.store.address.zip" ' .. + 'for the Scalar "String"' + local result = gql_query_13:execute(variables_13_6) + local err = result.errors[1].message + test:is(err, exp_err, 'wrong type for an InputObject field') + + local variables_13_7 = {xorder_metainfo = {store = { + address = { + street = 'street', + city = 'city', + state = 'state', + zip = 'zip', + foo = 'foo', + } + }}} + local exp_err = 'Unknown field "foo" of the variable "xorder_metainfo.' .. + 'store.address" for the InputObject "arguments___order_metainfo_' .. + 'collection___update___order_metainfo_collection_update___store' .. + '___store___address___address"' + local result = gql_query_13:execute(variables_13_7) + local err = result.errors[1].message + test:is(err, exp_err, 'extra field for an InputObject type') + + -- }}} + + -- {{{ InputMap + + local variables_13_8 = {xorder_metainfo = {store = { + parametrized_tags = 42}}} + local exp_err = 'Variable "xorder_metainfo.store.parametrized_tags" ' .. + 'for the InputMap "arguments___order_metainfo_collection___update' .. + '___order_metainfo_collection_update___store___store___' .. + 'parametrized_tags___InputMap" must be a Lua table, got number' + local result = gql_query_13:execute(variables_13_8) + local err = result.errors[1].message + test:is(err, exp_err, 'Int for an InputMap type') + + local variables_13_9 = {xorder_metainfo = {store = { + parametrized_tags = {'foo', 'bar', 'baz'}}}} + local exp_err = 'Field key of the variable "xorder_metainfo.store.' .. + 'parametrized_tags" for the InputMap "arguments___order_metainfo_' .. + 'collection___update___order_metainfo_collection_update___store___' .. + 'store___parametrized_tags___InputMap" must be a string, got number' + local result = gql_query_13:execute(variables_13_9) + local err = result.errors[1].message + test:is(err, exp_err, 'List for an InputMap type') + + local variables_13_10 = {xorder_metainfo = {store = { + parametrized_tags = {int_tag = 42}}}} + local exp_err = 'Wrong variable "xorder_metainfo.store.' .. + 'parametrized_tags.int_tag" for the Scalar "String"' + local result = gql_query_13:execute(variables_13_10) + local err = result.errors[1].message + test:is(err, exp_err, 'wrong type for an InputMap field') + + -- }}} + + -- {{{ InputUnion + + local variables_13_11 = {xorder_metainfo = {store = { + external_id = 42}}} + local exp_err = 'union value must be a map with one field, got number' + local result = gql_query_13:execute(variables_13_11) + local err = result.errors[1].message + test:is(err, exp_err, 'non-map value type for an InputUnion type') + + local variables_13_12 = {xorder_metainfo = {store = { + external_id = {int = 42.2}}}} + local exp_err = 'Wrong variable "xorder_metainfo.store.external_id.int" ' .. + 'for the Scalar "Int"' + local result = gql_query_13:execute(variables_13_12) + local err = result.errors[1].message + test:is(err, exp_err, 'wrong value type for an InputUnion type') + + local variables_13_13 = {xorder_metainfo = {store = { + external_id = {integer = 42}}}} -- integer instead of int + local exp_err = 'unexpected union value field: integer' + local result = gql_query_13:execute(variables_13_13) + local err = result.errors[1].message + test:is(err, exp_err, 'wrong object field name for an InputUnion type') + + local variables_13_14 = {xorder_metainfo = {store = { + external_id = {a = 1, b = 2, c = 3}}}} + local exp_err = 'union value must have only one field' + local result = gql_query_13:execute(variables_13_14) + local err = result.errors[1].message + test:is(err, exp_err, 'object with several fields for an InputUnion type') + + local variables_13_15 = {xorder_metainfo = {store = { + external_id = {}}}} + local exp_err = 'union value must have only one field' + local result = gql_query_13:execute(variables_13_15) + local err = result.errors[1].message + test:is(err, exp_err, 'object with no fields for an InputUnion type') + + -- }}} + + local query_14 = [[ + mutation($order_metainfo: order_metainfo_collection_insert) { + order_metainfo_collection(insert: $order_metainfo) { + order_metainfo_id + } + } + ]] + local gql_query_14 = test_utils.show_trace(function() + return gql_wrapper:compile(query_14) + end) + + -- {{{ InputObject (no mandatory field) + + local variables_14_1 = { + order_metainfo = { + metainfo = 'order metainfo', + order_metainfo_id = 'order_metainfo_id_14_1', + order_id = 'order_id', + store = { + name = 'store', + address = { + street = 'street', + city = 'city', + state = 'state', + -- no zip field + }, + second_address = { + street = 'second street', + city = 'second city', + state = 'second state', + zip = 'second zip', + }, + external_id = {string = 'eid'}, + tags = {'slow'}, + parametrized_tags = { + size = 'small', + } + } + } + } + local exp_err = 'Variable "order_metainfo.store.address.zip" expected ' .. + 'to be non-null' + local result = gql_query_14:execute(variables_14_1) + local err = result.errors[1].message + test:is(err, exp_err, 'Lack of non-null field for an InputObject type') + + -- }}} + -- }}} + assert(test:check(), 'check plan') end diff --git a/test/testdata/compound_index_testdata.lua b/test/testdata/compound_index_testdata.lua index 917c05b..f312804 100644 --- a/test/testdata/compound_index_testdata.lua +++ b/test/testdata/compound_index_testdata.lua @@ -1232,7 +1232,7 @@ function compound_index_testdata.run_queries(gql_wrapper) } local result = gql_query_4:execute(variables_4_2) local err = result.errors[1].message - local exp_err = 'offset by a partial key is forbidden' + local exp_err = 'Variable "offset.user_num" expected to be non-null' test:is(err, exp_err, '4_2') -- }}} @@ -1310,8 +1310,7 @@ function compound_index_testdata.run_queries(gql_wrapper) } local result = gql_query_5:execute(variables_5_2) local err = result.errors[1].message - local exp_err = 'offset by a partial key is forbidden: ' .. - 'expected "order_num" field' + local exp_err = 'Variable "offset.order_num" expected to be non-null' test:is(err, exp_err, '5_2') -- }}} From bfc8bea18e2b0010490d7b8d023b084a242757f1 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Tue, 26 Jun 2018 20:14:05 +0300 Subject: [PATCH 13/20] Fix grammar mistakes in README --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 0b28f12..765358b 100644 --- a/README.md +++ b/README.md @@ -141,23 +141,23 @@ be consistent. Mutations are disabled in the resharding state of a shard cluster. There are three types of modifications: insert, update and delete. Several -modifications are allowed in an one GraphQL request, but they will be processed -in non-transactional way. +modifications are allowed in one GraphQL request, but they will be processed in +non-transactional way. In the case of shard accessor the following constraints can guarantee that data -will be changed in atomic way or, in other words, in an one shard request (but +will be changed in atomic way or, in other words, in one shard request (but foregoing and upcoming selects can see other data): * One insert / update / delete argument over the entire GraphQL request. * For update / delete: either the argument is for 1:1 connection or `limit: 1` - is used for a collection (a topmost field) or 1:N connection (a nested + is used for a collection (a upmost field) or 1:N connection (a nested field). * No update of a first field of a **tuple** (shard key is calculated by it). It is the first field of upmost record in the schema for a collection in case when there are no service fields. If there are service fields, the first field of a tuple cannot be changed by a mutation GraphQL request. -Data can be changed between shard requests which are part of the one GraphQL +Data can be changed between shard requests which are part of one GraphQL request, so the result can observe inconsistent state. We'll don't show all possible cases, but give an idea what going on in the following paragraph. @@ -229,7 +229,7 @@ Consider the following details: #### Update Example with an update statement passed from a variable. Note that here we -update an object given by a connection (inside an one of nested fields of a +update an object given by a connection (inside one of nested fields of a request): ``` @@ -336,7 +336,7 @@ Consider the following details: * The `delete` argument is forbidden with `insert` or `update` arguments. * The `delete` argument is forbidden in `query` requests. * The same fields traversal order and 'select -> change -> select connected' - order of operations for an one field are applied likewise for the `update` + order of operations for one field are applied likewise for the `update` argument. * The `limit` argument can be used to define how many objects are subject to deletion and `offset` can help with adjusting start point of multi-object From 680adbe8c49368a6a2b6eb25488db15a9c4bf5e0 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Tue, 26 Jun 2018 20:14:44 +0300 Subject: [PATCH 14/20] Make mutation order description more clear --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 765358b..64fb9e4 100644 --- a/README.md +++ b/README.md @@ -300,9 +300,10 @@ Consider the following details: `update` argument, then connected objects are selected. * The `limit` and `offset` arguments applied before update, so a user can use `limit: 1` to update only first match. -* Objects traversed in deep-first up-first order as it written in a mutation - request. So an `update` argument potentially changes those fields that are - follows the updated object in this order. +* Objects are traversed in pre-order depth-first way, object's fields are + traversed in an order as they are written in a mutation request. So an + `update` argument potentially changes those fields that are follows the + updated object in this order. * Filters by connected objects are performed before update. Resulting connected objects given after the update (it is matter when a field(s) of the parent objects by whose the connection is made is subject to change). From 58f896288d027993768aa23800208669eccefe45 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Wed, 27 Jun 2018 00:17:36 +0300 Subject: [PATCH 15/20] Remove trailings spaces from types descriptions --- graphql/core/types.lua | 2 +- graphql/core/util.lua | 2 +- test/common/introspection.test.lua | 64 +++++++++++++++--------------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/graphql/core/types.lua b/graphql/core/types.lua index 743575f..f91eb15 100644 --- a/graphql/core/types.lua +++ b/graphql/core/types.lua @@ -320,7 +320,7 @@ end types.int = types.scalar({ name = 'Int', - description = "The `Int` scalar type represents non-fractional signed whole numeric values. Int can represent values between -(2^31) and 2^31 - 1. ", + description = "The `Int` scalar type represents non-fractional signed whole numeric values. Int can represent values between -(2^31) and 2^31 - 1.", serialize = coerceInt, parseValue = coerceInt, parseLiteral = function(node) diff --git a/graphql/core/util.lua b/graphql/core/util.lua index f9f7218..01dc2d9 100644 --- a/graphql/core/util.lua +++ b/graphql/core/util.lua @@ -42,7 +42,7 @@ function util.bind1(func, x) end function util.trim(s) - return s:gsub('^%s+', ''):gsub('%s$', ''):gsub('%s%s+', ' ') + return s:gsub('^%s+', ''):gsub('%s+$', ''):gsub('%s%s+', ' ') end function util.coerceValue(node, schemaType, variables) diff --git a/test/common/introspection.test.lua b/test/common/introspection.test.lua index 791e883..ab8a923 100755 --- a/test/common/introspection.test.lua +++ b/test/common/introspection.test.lua @@ -595,8 +595,8 @@ local function run_queries(gql_wrapper) name: INLINE_FRAGMENT description: Location adjacent to an inline fragment. name: __DirectiveLocation - description: 'A Directive can be adjacent to many parts of the GraphQL language, - a __DirectiveLocation describes one such possible adjacencies. ' + description: A Directive can be adjacent to many parts of the GraphQL language, + a __DirectiveLocation describes one such possible adjacencies. - interfaces: *0 fields: - isDeprecated: false @@ -709,11 +709,11 @@ local function run_queries(gql_wrapper) name: args kind: OBJECT name: __Directive - description: 'A Directive provides a way to describe alternate runtime execution + description: A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document. In some cases, you need to provide options to alter GraphQL’s execution behavior in ways field arguments will not suffice, such as conditionally including or skipping a field. Directives - provide this by describing additional information to the executor. ' + provide this by describing additional information to the executor. - interfaces: *0 fields: - isDeprecated: false @@ -766,8 +766,8 @@ local function run_queries(gql_wrapper) name: description kind: OBJECT name: __Field - description: 'Object and Interface types are described by a list of Fields, each - of which has a name, potentially a list of arguments, and a return type. ' + description: Object and Interface types are described by a list of Fields, each + of which has a name, potentially a list of arguments, and a return type. - interfaces: *0 fields: - isDeprecated: false @@ -880,9 +880,9 @@ local function run_queries(gql_wrapper) input value. kind: OBJECT name: __InputValue - description: 'Arguments provided to Fields or Directives and the input fields + description: Arguments provided to Fields or Directives and the input fields of an InputObject are represented as Input Values which describe their type - and optionally a default value. ' + and optionally a default value. - kind: INPUT_OBJECT inputFields: - type: @@ -1135,9 +1135,9 @@ local function run_queries(gql_wrapper) description: A list of all directives supported by this server. kind: OBJECT name: __Schema - description: 'A GraphQL Schema defines the capabilities of a GraphQL server. It + description: A GraphQL Schema defines the capabilities of a GraphQL server. It exposes all available types and directives on the server, as well as the entry - points for query and mutation operations. ' + points for query and mutation operations. - kind: INPUT_OBJECT inputFields: - type: @@ -1272,9 +1272,9 @@ local function run_queries(gql_wrapper) name: description kind: OBJECT name: __EnumValue - description: 'One possible value for a given Enum. Enum values are unique values, + description: One possible value for a given Enum. Enum values are unique values, not a placeholder for a string or numeric value. However an Enum value is returned - in a JSON response as a string. ' + in a JSON response as a string. - interfaces: *0 fields: - isDeprecated: false @@ -1365,13 +1365,13 @@ local function run_queries(gql_wrapper) name: description kind: OBJECT name: __Type - description: 'The fundamental unit of any GraphQL Schema is the type. There are + description: The fundamental unit of any GraphQL Schema is the type. There are many kinds of types in GraphQL as represented by the `__TypeKind` enum. Depending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name and description, while Enum types provide their values. Object and Interface types provide the fields they describe. Abstract types, Union and Interface, provide the Object types possible - at runtime. List and NonNull types compose other types. ' + at runtime. List and NonNull types compose other types. - kind: INPUT_OBJECT inputFields: - type: @@ -1402,8 +1402,8 @@ local function run_queries(gql_wrapper) description: generated from avro-schema for order_metainfo_collection_insert - kind: SCALAR name: Int - description: 'The `Int` scalar type represents non-fractional signed whole numeric - values. Int can represent values between -(2^31) and 2^31 - 1. ' + description: The `Int` scalar type represents non-fractional signed whole numeric + values. Int can represent values between -(2^31) and 2^31 - 1. - kind: INPUT_OBJECT inputFields: - type: @@ -2194,11 +2194,11 @@ local function run_queries(gql_wrapper) name: args kind: OBJECT name: __Directive - description: 'A Directive provides a way to describe alternate runtime execution + description: A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document. In some cases, you need to provide options to alter GraphQL’s execution behavior in ways field arguments will not suffice, such as conditionally including or skipping a field. Directives - provide this by describing additional information to the executor. ' + provide this by describing additional information to the executor. - interfaces: *0 fields: - isDeprecated: false @@ -2570,9 +2570,9 @@ local function run_queries(gql_wrapper) description: A list of all directives supported by this server. kind: OBJECT name: __Schema - description: 'A GraphQL Schema defines the capabilities of a GraphQL server. It + description: A GraphQL Schema defines the capabilities of a GraphQL server. It exposes all available types and directives on the server, as well as the entry - points for query and mutation operations. ' + points for query and mutation operations. - interfaces: *0 fields: - isDeprecated: false @@ -2605,9 +2605,9 @@ local function run_queries(gql_wrapper) name: description kind: OBJECT name: __EnumValue - description: 'One possible value for a given Enum. Enum values are unique values, + description: One possible value for a given Enum. Enum values are unique values, not a placeholder for a string or numeric value. However an Enum value is returned - in a JSON response as a string. ' + in a JSON response as a string. - interfaces: *0 fields: - isDeprecated: false @@ -2698,13 +2698,13 @@ local function run_queries(gql_wrapper) name: description kind: OBJECT name: __Type - description: 'The fundamental unit of any GraphQL Schema is the type. There are + description: The fundamental unit of any GraphQL Schema is the type. There are many kinds of types in GraphQL as represented by the `__TypeKind` enum. Depending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name and description, while Enum types provide their values. Object and Interface types provide the fields they describe. Abstract types, Union and Interface, provide the Object types possible - at runtime. List and NonNull types compose other types. ' + at runtime. List and NonNull types compose other types. - kind: INPUT_OBJECT inputFields: - type: @@ -2759,9 +2759,9 @@ local function run_queries(gql_wrapper) input value. kind: OBJECT name: __InputValue - description: 'Arguments provided to Fields or Directives and the input fields + description: Arguments provided to Fields or Directives and the input fields of an InputObject are represented as Input Values which describe their type - and optionally a default value. ' + and optionally a default value. - interfaces: *0 fields: - isDeprecated: false @@ -2814,8 +2814,8 @@ local function run_queries(gql_wrapper) name: description kind: OBJECT name: __Field - description: 'Object and Interface types are described by a list of Fields, each - of which has a name, potentially a list of arguments, and a return type. ' + description: Object and Interface types are described by a list of Fields, each + of which has a name, potentially a list of arguments, and a return type. - name: Double kind: SCALAR - kind: SCALAR @@ -2823,8 +2823,8 @@ local function run_queries(gql_wrapper) description: The `Boolean` scalar type represents `true` or `false`. - kind: SCALAR name: Int - description: 'The `Int` scalar type represents non-fractional signed whole numeric - values. Int can represent values between -(2^31) and 2^31 - 1. ' + description: The `Int` scalar type represents non-fractional signed whole numeric + values. Int can represent values between -(2^31) and 2^31 - 1. - kind: SCALAR name: String description: The `String` scalar type represents textual data, represented as @@ -3066,8 +3066,8 @@ local function run_queries(gql_wrapper) name: INLINE_FRAGMENT description: Location adjacent to an inline fragment. name: __DirectiveLocation - description: 'A Directive can be adjacent to many parts of the GraphQL language, - a __DirectiveLocation describes one such possible adjacencies. ' + description: A Directive can be adjacent to many parts of the GraphQL language, + a __DirectiveLocation describes one such possible adjacencies. queryType: name: Query directives: From e7a826a231f1856a030bab39d8595317d98820f5 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Sat, 30 Jun 2018 22:38:24 +0300 Subject: [PATCH 16/20] Validate a nested variable by an argument type --- Makefile | 1 + graphql/convert_schema/union.lua | 4 +- graphql/core/rules.lua | 273 +++++---- graphql/core/schema.lua | 7 +- graphql/core/types.lua | 1 + test/common/introspection.test.lua | 684 ++++++++++++---------- test/testdata/common_testdata.lua | 131 ++++- test/testdata/compound_index_testdata.lua | 2 +- 8 files changed, 679 insertions(+), 424 deletions(-) diff --git a/Makefile b/Makefile index e42aaae..ee93810 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,7 @@ default: lint: luacheck graphql/*.lua \ graphql/core/execute.lua \ + graphql/core/rules.lua \ graphql/core/validate_variables.lua \ graphql/convert_schema/*.lua \ graphql/server/*.lua \ diff --git a/graphql/convert_schema/union.lua b/graphql/convert_schema/union.lua index 801160d..156adb6 100644 --- a/graphql/convert_schema/union.lua +++ b/graphql/convert_schema/union.lua @@ -98,8 +98,10 @@ local function create_union_types(avro_schema, opts) if type == 'null' then is_nullable = true else - local variant_type = convert(type, {context = context}) local box_field_name = type.name or avro_helpers.avro_type(type) + table.insert(context.path, box_field_name) + local variant_type = convert(type, {context = context}) + table.remove(context.path, #context.path) union_types[#union_types + 1] = box_type(variant_type, box_field_name, { gen_argument = gen_argument, diff --git a/graphql/core/rules.lua b/graphql/core/rules.lua index 76f1d3b..96a9614 100644 --- a/graphql/core/rules.lua +++ b/graphql/core/rules.lua @@ -1,9 +1,10 @@ +local yaml = require('yaml') local path = (...):gsub('%.[^%.]+$', '') local types = require(path .. '.types') local util = require(path .. '.util') -local schema = require(path .. '.schema') local introspection = require(path .. '.introspection') local query_util = require(path .. '.query_util') +local graphql_utils = require('graphql.utils') local function getParentField(context, name, count) if introspection.fieldMap[name] then return introspection.fieldMap[name] end @@ -162,7 +163,8 @@ function rules.unambiguousSelections(node, context) validateField(key, fieldEntry) elseif selection.kind == 'inlineFragment' then - local parentType = selection.typeCondition and context.schema:getType(selection.typeCondition.name.value) or parentType + local parentType = selection.typeCondition and context.schema:getType( + selection.typeCondition.name.value) or parentType validateSelectionSet(selection.selectionSet, parentType) elseif selection.kind == 'fragmentSpread' then local fragmentDefinition = context.fragmentMap[selection.name.value] @@ -436,117 +438,192 @@ function rules.variablesAreDefined(node, context) end end -function rules.variableUsageAllowed(node, context) - if context.currentOperation then - local variableMap = {} - for _, definition in ipairs(context.currentOperation.variableDefinitions or {}) do - variableMap[definition.variable.name.value] = definition - end - - local arguments - - if node.kind == 'field' then - arguments = { [node.name.value] = node.arguments } - elseif node.kind == 'fragmentSpread' then - local seen = {} - local function collectArguments(referencedNode) - if referencedNode.kind == 'selectionSet' then - for _, selection in ipairs(referencedNode.selections) do - if not seen[selection] then - seen[selection] = true - collectArguments(selection) - end - end - elseif referencedNode.kind == 'field' and referencedNode.arguments then - local fieldName = referencedNode.name.value - arguments[fieldName] = arguments[fieldName] or {} - for _, argument in ipairs(referencedNode.arguments) do - table.insert(arguments[fieldName], argument) - end - elseif referencedNode.kind == 'inlineFragment' then - return collectArguments(referencedNode.selectionSet) - elseif referencedNode.kind == 'fragmentSpread' then - local fragment = context.fragmentMap[referencedNode.name.value] - return fragment and collectArguments(fragment.selectionSet) - end +-- {{{ variableUsageAllowed + +local function collectArguments(referencedNode, context, seen, arguments) + if referencedNode.kind == 'selectionSet' then + for _, selection in ipairs(referencedNode.selections) do + if not seen[selection] then + seen[selection] = true + collectArguments(selection, context, seen, arguments) end + end + elseif referencedNode.kind == 'field' and referencedNode.arguments then + local fieldName = referencedNode.name.value + arguments[fieldName] = arguments[fieldName] or {} + for _, argument in ipairs(referencedNode.arguments) do + table.insert(arguments[fieldName], argument) + end + elseif referencedNode.kind == 'inlineFragment' then + return collectArguments(referencedNode.selectionSet, context, seen, + arguments) + elseif referencedNode.kind == 'fragmentSpread' then + local fragment = context.fragmentMap[referencedNode.name.value] + return fragment and collectArguments(fragment.selectionSet, context, seen, + arguments) + end +end + +-- http://facebook.github.io/graphql/June2018/#AreTypesCompatible() +local function isTypeSubTypeOf(subType, superType, context) + if subType == superType then return true end + + if superType.__type == 'NonNull' then + if subType.__type == 'NonNull' then + return isTypeSubTypeOf(subType.ofType, superType.ofType, context) + end + + return false + elseif subType.__type == 'NonNull' then + return isTypeSubTypeOf(subType.ofType, superType, context) + end + + if superType.__type == 'List' then + if subType.__type == 'List' then + return isTypeSubTypeOf(subType.ofType, superType.ofType, context) + end + + return false + elseif subType.__type == 'List' then + return false + end - local fragment = context.fragmentMap[node.name.value] - if fragment then - arguments = {} - collectArguments(fragment.selectionSet) + if superType.__type == 'Scalar' and superType.subtype == 'InputUnion' then + local types = superType.types + for i = 1, #types do + if types[i] == subType then + return true end end - if not arguments then return end + return false + end + + return false +end + +local function getTypeName(t) + if t.name ~= nil then + if t.name == 'Scalar' and t.subtype == 'InputMap' then + return ('InputMap(%s)'):format(getTypeName(t.values)) + elseif t.name == 'Scalar' and t.subtype == 'InputUnion' then + local typeNames = {} + for _, child in ipairs(t.types) do + table.insert(typeNames, getTypeName(child)) + end + return ('InputUnion(%s)'):format(table.concat(typeNames, ',')) + end + return t.name + elseif t.__type == 'NonNull' then + return ('NonNull(%s)'):format(getTypeName(t.ofType)) + elseif t.__type == 'List' then + return ('List(%s)'):format(getTypeName(t.ofType)) + end - for field in pairs(arguments) do - local parentField = getParentField(context, field) - for i = 1, #arguments[field] do - local argument = arguments[field][i] - if argument.value.kind == 'variable' then - local argumentType = parentField.arguments[argument.name.value] + local orig_encode_use_tostring = yaml.cfg.encode_use_tostring + local err = ('Internal error: unknown type:\n%s'):format(yaml.encode(t)) + yaml.cfg({encode_use_tostring = orig_encode_use_tostring}) + error(err) +end - local variableName = argument.value.name.value - local variableDefinition = variableMap[variableName] - local hasDefault = variableDefinition.defaultValue ~= nil +local function isVariableTypesValid(argument, argumentType, context, + variableMap) + if argument.value.kind == 'variable' then + -- found a variable, check types compatibility + local variableName = argument.value.name.value + local variableDefinition = variableMap[variableName] + local hasDefault = variableDefinition.defaultValue ~= nil - local variableType = query_util.typeFromAST(variableDefinition.type, - context.schema) + local variableType = query_util.typeFromAST(variableDefinition.type, + context.schema) - if hasDefault and variableType.__type ~= 'NonNull' then - variableType = types.nonNull(variableType) - end + if hasDefault and variableType.__type ~= 'NonNull' then + variableType = types.nonNull(variableType) + end - local function isTypeSubTypeOf(subType, superType) - if subType == superType then return true end - - if superType.__type == 'NonNull' then - if subType.__type == 'NonNull' then - return isTypeSubTypeOf(subType.ofType, superType.ofType) - end - - return false - elseif subType.__type == 'NonNull' then - return isTypeSubTypeOf(subType.ofType, superType) - end - - if superType.__type == 'List' then - if subType.__type == 'List' then - return isTypeSubTypeOf(subType.ofType, superType.ofType) - end - - return false - elseif subType.__type == 'List' then - return false - end - - if subType.__type ~= 'Object' then return false end - - if superType.__type == 'Interface' then - local implementors = context.schema:getImplementors(superType.name) - return implementors and implementors[context.schema:getType(subType.name)] - elseif superType.__type == 'Union' then - local types = superType.types - for i = 1, #types do - if types[i] == subType then - return true - end - end - - return false - end - - return false - end + if not isTypeSubTypeOf(variableType, argumentType, context) then + return false, ('Variable "%s" type mismatch: the variable type "%s" ' .. + 'is not compatible with the argument type "%s"'):format(variableName, + getTypeName(variableType), getTypeName(argumentType)) + end + elseif argument.value.kind == 'inputObject' then + -- find variables deeper + for _, child in ipairs(argument.value.values) do + local isInputObject = argumentType.__type == 'InputObject' + local isInputMap = argumentType.__type == 'Scalar' and + argumentType.subtype == 'InputMap' + local isInputUnion = argumentType.__type == 'Scalar' and + argumentType.subtype == 'InputUnion' + + if isInputObject then + local childArgumentType = argumentType.fields[child.name].kind + local ok, err = isVariableTypesValid(child, childArgumentType, context, + variableMap) + if not ok then return false, err end + elseif isInputMap then + local childArgumentType = argumentType.values + local ok, err = isVariableTypesValid(child, childArgumentType, context, + variableMap) + if not ok then return false, err end + elseif isInputUnion then + local has_ok = false + local first_err + + for _, childArgumentType in ipairs(argumentType.types) do + local ok, err = isVariableTypesValid(child, + childArgumentType, context, variableMap) + has_ok = has_ok or ok + first_err = first_err or graphql_utils.strip_error(err) + if ok then break end + end - if not isTypeSubTypeOf(variableType, argumentType) then - error('Variable type mismatch') - end + if not has_ok then + return false, first_err end end end end + return true +end + +function rules.variableUsageAllowed(node, context) + if not context.currentOperation then return end + + local variableMap = {} + local variableDefinitions = context.currentOperation.variableDefinitions + for _, definition in ipairs(variableDefinitions or {}) do + variableMap[definition.variable.name.value] = definition + end + + local arguments + + if node.kind == 'field' then + arguments = { [node.name.value] = node.arguments } + elseif node.kind == 'fragmentSpread' then + local seen = {} + local fragment = context.fragmentMap[node.name.value] + if fragment then + arguments = {} + collectArguments(fragment.selectionSet, context, seen, arguments) + end + end + + if not arguments then return end + + for field in pairs(arguments) do + local parentField = getParentField(context, field) + for i = 1, #arguments[field] do + local argument = arguments[field][i] + local argumentType = parentField.arguments[argument.name.value] + local ok, err = isVariableTypesValid(argument, argumentType, context, + variableMap) + if not ok then + error(err) + end + end + end end +-- }}} + return rules diff --git a/graphql/core/schema.lua b/graphql/core/schema.lua index a320335..21f0ae3 100644 --- a/graphql/core/schema.lua +++ b/graphql/core/schema.lua @@ -50,7 +50,8 @@ function schema:generateTypeMap(node) node.fields = type(node.fields) == 'function' and node.fields() or node.fields self.typeMap[node.name] = node - if node.__type == 'Union' then + if node.__type == 'Union' or (node.__type == 'Scalar' and + node.subtype == 'InputUnion') then for _, type in ipairs(node.types) do self:generateTypeMap(type) end @@ -77,6 +78,10 @@ function schema:generateTypeMap(node) self:generateTypeMap(field.kind) end end + + if node.type == 'Scalar' and node.subtype == 'InputMap' then + self:generateTypeMap(node.values) + end end function schema:generateDirectiveMap() diff --git a/graphql/core/types.lua b/graphql/core/types.lua index f91eb15..31999b7 100644 --- a/graphql/core/types.lua +++ b/graphql/core/types.lua @@ -249,6 +249,7 @@ function types.inputUnion(config) __type = 'Scalar', subtype = 'InputUnion', name = config.name, + types = config.types, serialize = function(value) return value end, parseValue = function(value) return value end, parseLiteral = function(node) diff --git a/test/common/introspection.test.lua b/test/common/introspection.test.lua index ab8a923..dea6ce0 100755 --- a/test/common/introspection.test.lua +++ b/test/common/introspection.test.lua @@ -112,7 +112,7 @@ local function run_queries(gql_wrapper) } ]] - -- luacheck: push max line length 152 + -- luacheck: push max line length 156 local exp_result_avro_schema_3 = yaml.decode(([[ --- __schema: @@ -472,6 +472,76 @@ local function run_queries(gql_wrapper) description: generated from avro-schema for address - name: arguments___order_metainfo_collection___update___order_metainfo_collection_update___store___store___parametrized_tags___InputMap kind: SCALAR + - kind: ENUM + enumValues: + - isDeprecated: false + name: FRAGMENT_SPREAD + description: Location adjacent to a fragment spread. + - isDeprecated: false + name: MUTATION + description: Location adjacent to a mutation operation. + - isDeprecated: false + name: FRAGMENT_DEFINITION + description: Location adjacent to a fragment definition. + - isDeprecated: false + name: FIELD + description: Location adjacent to a field. + - isDeprecated: false + name: QUERY + description: Location adjacent to a query operation. + - isDeprecated: false + name: INLINE_FRAGMENT + description: Location adjacent to an inline fragment. + name: __DirectiveLocation + description: A Directive can be adjacent to many parts of the GraphQL language, + a __DirectiveLocation describes one such possible adjacencies. + - interfaces: *0 + fields: + - isDeprecated: false + args: *0 + type: + name: String + kind: SCALAR + name: description + - isDeprecated: false + args: *0 + type: + ofType: + ofType: + ofType: + name: __DirectiveLocation + kind: ENUM + kind: NON_NULL + kind: LIST + kind: NON_NULL + name: locations + - isDeprecated: false + args: *0 + type: + ofType: + name: String + kind: SCALAR + kind: NON_NULL + name: name + - isDeprecated: false + args: *0 + type: + ofType: + ofType: + ofType: + name: __InputValue + kind: OBJECT + kind: NON_NULL + kind: LIST + kind: NON_NULL + name: args + kind: OBJECT + name: __Directive + description: A Directive provides a way to describe alternate runtime execution + and type validation behavior in a GraphQL document. In some cases, you need + to provide options to alter GraphQL’s execution behavior in ways field arguments + will not suffice, such as conditionally including or skipping a field. Directives + provide this by describing additional information to the executor. - kind: ENUM enumValues: - isDeprecated: false @@ -541,6 +611,60 @@ local function run_queries(gql_wrapper) name: description name: order_collection_insert description: generated from avro-schema for order_collection_insert + - interfaces: *0 + fields: + - isDeprecated: false + args: *0 + type: + ofType: + name: Boolean + kind: SCALAR + kind: NON_NULL + name: isDeprecated + - isDeprecated: false + args: *0 + type: + name: String + kind: SCALAR + name: deprecationReason + - isDeprecated: false + args: *0 + type: + ofType: + ofType: + ofType: + name: __InputValue + kind: OBJECT + kind: NON_NULL + kind: LIST + kind: NON_NULL + name: args + - isDeprecated: false + args: *0 + type: + ofType: + name: __Type + kind: OBJECT + kind: NON_NULL + name: type + - isDeprecated: false + args: *0 + type: + ofType: + name: String + kind: SCALAR + kind: NON_NULL + name: name + - isDeprecated: false + args: *0 + type: + name: String + kind: SCALAR + name: description + kind: OBJECT + name: __Field + description: Object and Interface types are described by a list of Fields, each + of which has a name, potentially a list of arguments, and a return type. - interfaces: *0 fields: - isDeprecated: false @@ -554,6 +678,43 @@ local function run_queries(gql_wrapper) kind: OBJECT name: Int_box description: Box (wrapper) around union variant + - interfaces: *0 + fields: + - isDeprecated: false + args: *0 + type: + ofType: + name: __Type + kind: OBJECT + kind: NON_NULL + name: type + - isDeprecated: false + args: *0 + type: + name: String + kind: SCALAR + name: description + - isDeprecated: false + args: *0 + type: + ofType: + name: String + kind: SCALAR + kind: NON_NULL + name: name + - isDeprecated: false + args: *0 + type: + name: String + kind: SCALAR + name: defaultValue + description: A GraphQL-formatted string representing the default value for + this input value. + kind: OBJECT + name: __InputValue + description: Arguments provided to Fields or Directives and the input fields + of an InputObject are represented as Input Values which describe their type + and optionally a default value. - kind: INPUT_OBJECT inputFields: - type: @@ -574,29 +735,50 @@ local function run_queries(gql_wrapper) name: street name: arguments___order_metainfo_collection___store___store___second_address___address description: generated from avro-schema for address - - kind: ENUM - enumValues: - - isDeprecated: false - name: FRAGMENT_SPREAD - description: Location adjacent to a fragment spread. - - isDeprecated: false - name: MUTATION - description: Location adjacent to a mutation operation. - - isDeprecated: false - name: FRAGMENT_DEFINITION - description: Location adjacent to a fragment definition. - - isDeprecated: false - name: FIELD - description: Location adjacent to a field. - - isDeprecated: false - name: QUERY - description: Location adjacent to a query operation. - - isDeprecated: false - name: INLINE_FRAGMENT - description: Location adjacent to an inline fragment. - name: __DirectiveLocation - description: A Directive can be adjacent to many parts of the GraphQL language, - a __DirectiveLocation describes one such possible adjacencies. + - kind: INPUT_OBJECT + inputFields: + - type: + ofType: + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___address___address + kind: INPUT_OBJECT + kind: NON_NULL + name: address + - type: + ofType: + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___second_address___address + kind: INPUT_OBJECT + kind: NON_NULL + name: second_address + - type: + ofType: + ofType: + ofType: + name: String + kind: SCALAR + kind: NON_NULL + kind: LIST + kind: NON_NULL + name: tags + - type: + ofType: + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___external_id___external_id + kind: SCALAR + kind: NON_NULL + name: external_id + - type: + ofType: + name: String + kind: SCALAR + kind: NON_NULL + name: name + - type: + ofType: + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___parametrized_tags___InputMap + kind: SCALAR + kind: NON_NULL + name: parametrized_tags + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store + description: generated from avro-schema for store - interfaces: *0 fields: - isDeprecated: false @@ -667,107 +849,44 @@ local function run_queries(gql_wrapper) kind: OBJECT name: String_box description: Box (wrapper) around union variant - - interfaces: *0 - fields: - - isDeprecated: false - args: *0 - type: - name: String - kind: SCALAR - name: description - - isDeprecated: false - args: *0 - type: - ofType: - ofType: - ofType: - name: __DirectiveLocation - kind: ENUM - kind: NON_NULL - kind: LIST - kind: NON_NULL - name: locations - - isDeprecated: false - args: *0 - type: + - kind: INPUT_OBJECT + inputFields: + - type: ofType: name: String kind: SCALAR kind: NON_NULL - name: name - - isDeprecated: false - args: *0 - type: - ofType: - ofType: - ofType: - name: __InputValue - kind: OBJECT - kind: NON_NULL - kind: LIST - kind: NON_NULL - name: args - kind: OBJECT - name: __Directive - description: A Directive provides a way to describe alternate runtime execution - and type validation behavior in a GraphQL document. In some cases, you need - to provide options to alter GraphQL’s execution behavior in ways field arguments - will not suffice, such as conditionally including or skipping a field. Directives - provide this by describing additional information to the executor. - - interfaces: *0 - fields: - - isDeprecated: false - args: *0 - type: + name: order_metainfo_id + - type: ofType: - name: Boolean + name: String kind: SCALAR kind: NON_NULL - name: isDeprecated - - isDeprecated: false - args: *0 - type: - name: String - kind: SCALAR - name: deprecationReason - - isDeprecated: false - args: *0 - type: + name: metainfo + - type: ofType: - ofType: - ofType: - name: __InputValue - kind: OBJECT - kind: NON_NULL - kind: LIST + name: String + kind: SCALAR kind: NON_NULL - name: args - - isDeprecated: false - args: *0 - type: + name: order_id + - type: ofType: - name: __Type - kind: OBJECT + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store + kind: INPUT_OBJECT kind: NON_NULL - name: type - - isDeprecated: false - args: *0 - type: + name: store + name: order_metainfo_collection_insert + description: generated from avro-schema for order_metainfo_collection_insert + - kind: INPUT_OBJECT + inputFields: + - type: ofType: - name: String + name: Int kind: SCALAR kind: NON_NULL - name: name - - isDeprecated: false - args: *0 - type: - name: String - kind: SCALAR - name: description - kind: OBJECT - name: __Field - description: Object and Interface types are described by a list of Fields, each - of which has a name, potentially a list of arguments, and a return type. + name: int + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___external_id___external_id___Int_box + description: Box (wrapper) around union variant - interfaces: *0 fields: - isDeprecated: false @@ -846,43 +965,8 @@ local function run_queries(gql_wrapper) name: user_connection description: generated from the connection "user_connection" of collection "order_collection" using collection "user_collection" - - interfaces: *0 - fields: - - isDeprecated: false - args: *0 - type: - ofType: - name: __Type - kind: OBJECT - kind: NON_NULL - name: type - - isDeprecated: false - args: *0 - type: - name: String - kind: SCALAR - name: description - - isDeprecated: false - args: *0 - type: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - name: name - - isDeprecated: false - args: *0 - type: - name: String - kind: SCALAR - name: defaultValue - description: A GraphQL-formatted string representing the default value for this - input value. - kind: OBJECT - name: __InputValue - description: Arguments provided to Fields or Directives and the input fields - of an InputObject are represented as Input Values which describe their type - and optionally a default value. + - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___parametrized_tags___InputMap + kind: SCALAR - kind: INPUT_OBJECT inputFields: - type: @@ -890,27 +974,9 @@ local function run_queries(gql_wrapper) name: String kind: SCALAR kind: NON_NULL - name: state - - type: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - name: zip - - type: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - name: city - - type: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - name: street - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___address___address - description: generated from avro-schema for address + name: string + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___external_id___external_id___String_box + description: Box (wrapper) around union variant - interfaces: *0 fields: - isDeprecated: false @@ -1135,9 +1201,9 @@ local function run_queries(gql_wrapper) description: A list of all directives supported by this server. kind: OBJECT name: __Schema - description: A GraphQL Schema defines the capabilities of a GraphQL server. It - exposes all available types and directives on the server, as well as the entry - points for query and mutation operations. + description: A GraphQL Schema defines the capabilities of a GraphQL server. + It exposes all available types and directives on the server, as well as the + entry points for query and mutation operations. - kind: INPUT_OBJECT inputFields: - type: @@ -1163,55 +1229,47 @@ local function run_queries(gql_wrapper) kind: SCALAR name: middle_name name: user_collection_insert - description: generated from avro-schema for user_collection_insert - - kind: INPUT_OBJECT - inputFields: - - type: - ofType: - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___address___address - kind: INPUT_OBJECT - kind: NON_NULL - name: address + description: generated from avro-schema for user_collection_insert + - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___external_id___external_id + kind: SCALAR + - kind: INPUT_OBJECT + inputFields: - type: ofType: - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___second_address___address - kind: INPUT_OBJECT + name: Int + kind: SCALAR kind: NON_NULL - name: second_address + name: int + name: arguments___order_metainfo_collection___update___order_metainfo_collection_update___store___store___external_id___external_id___Int_box + description: Box (wrapper) around union variant + - kind: INPUT_OBJECT + inputFields: - type: ofType: - ofType: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - kind: LIST + name: String + kind: SCALAR kind: NON_NULL - name: tags + name: state - type: ofType: - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___external_id___external_id + name: String kind: SCALAR kind: NON_NULL - name: external_id + name: zip - type: ofType: name: String kind: SCALAR kind: NON_NULL - name: name + name: city - type: ofType: - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___parametrized_tags___InputMap + name: String kind: SCALAR kind: NON_NULL - name: parametrized_tags - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store - description: generated from avro-schema for store - - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___parametrized_tags___InputMap - kind: SCALAR - - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___external_id___external_id - kind: SCALAR + name: street + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___second_address___address + description: generated from avro-schema for address - kind: INPUT_OBJECT inputFields: - type: @@ -1238,7 +1296,7 @@ local function run_queries(gql_wrapper) kind: SCALAR kind: NON_NULL name: street - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___second_address___address + name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store___address___address description: generated from avro-schema for address - interfaces: *0 fields: @@ -1273,8 +1331,8 @@ local function run_queries(gql_wrapper) kind: OBJECT name: __EnumValue description: One possible value for a given Enum. Enum values are unique values, - not a placeholder for a string or numeric value. However an Enum value is returned - in a JSON response as a string. + not a placeholder for a string or numeric value. However an Enum value is + returned in a JSON response as a string. - interfaces: *0 fields: - isDeprecated: false @@ -1369,41 +1427,87 @@ local function run_queries(gql_wrapper) many kinds of types in GraphQL as represented by the `__TypeKind` enum. Depending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name and description, while Enum - types provide their values. Object and Interface types provide the fields they - describe. Abstract types, Union and Interface, provide the Object types possible - at runtime. List and NonNull types compose other types. + types provide their values. Object and Interface types provide the fields + they describe. Abstract types, Union and Interface, provide the Object types + possible at runtime. List and NonNull types compose other types. + - kind: INPUT_OBJECT + inputFields: + - type: + name: String + kind: SCALAR + name: order_metainfo_id + - type: + name: String + kind: SCALAR + name: metainfo + - type: + name: String + kind: SCALAR + name: order_id + - type: + name: arguments___order_metainfo_collection___pcre___order_metainfo_collection_pcre___store___store + kind: INPUT_OBJECT + name: store + name: order_metainfo_collection_pcre + description: generated from avro-schema for order_metainfo_collection_pcre - kind: INPUT_OBJECT inputFields: - type: + name: arguments___order_metainfo_collection___store___store + kind: INPUT_OBJECT + name: store + - type: + name: String + kind: SCALAR + name: metainfo + - type: + name: String + kind: SCALAR + name: order_id + - type: + name: String + kind: SCALAR + name: order_metainfo_id + name: order_metainfo_connection + description: generated from the connection "order_metainfo_connection" of collection + "order_collection" using collection "order_metainfo_collection" + - interfaces: *0 + fields: + - isDeprecated: false + args: *0 + type: ofType: name: String kind: SCALAR kind: NON_NULL - name: order_metainfo_id - - type: + name: state + - isDeprecated: false + args: *0 + type: ofType: name: String kind: SCALAR kind: NON_NULL - name: metainfo - - type: + name: zip + - isDeprecated: false + args: *0 + type: ofType: name: String kind: SCALAR kind: NON_NULL - name: order_id - - type: + name: city + - isDeprecated: false + args: *0 + type: ofType: - name: arguments___order_metainfo_collection___insert___order_metainfo_collection_insert___store___store - kind: INPUT_OBJECT + name: String + kind: SCALAR kind: NON_NULL - name: store - name: order_metainfo_collection_insert - description: generated from avro-schema for order_metainfo_collection_insert - - kind: SCALAR - name: Int - description: The `Int` scalar type represents non-fractional signed whole numeric - values. Int can represent values between -(2^31) and 2^31 - 1. + name: street + kind: OBJECT + name: order_metainfo_collection___store___store___address___address + description: generated from avro-schema for address - kind: INPUT_OBJECT inputFields: - type: @@ -1537,49 +1641,28 @@ local function run_queries(gql_wrapper) kind: OBJECT name: order_collection description: generated from avro-schema for order - - name: Float - kind: SCALAR - kind: INPUT_OBJECT inputFields: - type: name: String kind: SCALAR - name: order_metainfo_id - - type: - name: String - kind: SCALAR - name: metainfo - - type: - name: String - kind: SCALAR - name: order_id - - type: - name: arguments___order_metainfo_collection___pcre___order_metainfo_collection_pcre___store___store - kind: INPUT_OBJECT - name: store - name: order_metainfo_collection_pcre - description: generated from avro-schema for order_metainfo_collection_pcre - - kind: INPUT_OBJECT - inputFields: - - type: - name: arguments___order_metainfo_collection___store___store - kind: INPUT_OBJECT - name: store + name: user_id - type: name: String kind: SCALAR - name: metainfo + name: last_name - type: name: String kind: SCALAR - name: order_id + name: first_name - type: name: String kind: SCALAR - name: order_metainfo_id - name: order_metainfo_connection - description: generated from the connection "order_metainfo_connection" of collection - "order_collection" using collection "order_metainfo_collection" + name: middle_name + name: user_collection_pcre + description: generated from avro-schema for user_collection_pcre + - name: Float + kind: SCALAR - kind: INPUT_OBJECT inputFields: - type: @@ -1607,6 +1690,22 @@ local function run_queries(gql_wrapper) name: Map description: Map is a dictionary with string keys and values of arbitrary but same among all values type + - kind: INPUT_OBJECT + inputFields: + - type: + name: String + kind: SCALAR + name: metainfo + - type: + name: String + kind: SCALAR + name: order_id + - type: + name: arguments___order_metainfo_collection___update___order_metainfo_collection_update___store___store + kind: INPUT_OBJECT + name: store + name: order_metainfo_collection_update + description: generated from avro-schema for order_metainfo_collection_update - kind: INPUT_OBJECT inputFields: - type: @@ -1639,26 +1738,6 @@ local function run_queries(gql_wrapper) name: parametrized_tags name: arguments___order_metainfo_collection___update___order_metainfo_collection_update___store___store description: generated from avro-schema for store - - kind: INPUT_OBJECT - inputFields: - - type: - name: String - kind: SCALAR - name: user_id - - type: - name: String - kind: SCALAR - name: last_name - - type: - name: String - kind: SCALAR - name: first_name - - type: - name: String - kind: SCALAR - name: middle_name - name: user_collection_pcre - description: generated from avro-schema for user_collection_pcre - kind: INPUT_OBJECT inputFields: - type: @@ -1679,62 +1758,23 @@ local function run_queries(gql_wrapper) name: street name: arguments___order_metainfo_collection___update___order_metainfo_collection_update___store___store___address___address description: generated from avro-schema for address - - kind: INPUT_OBJECT - inputFields: - - type: - name: String - kind: SCALAR - name: metainfo - - type: - name: String - kind: SCALAR - name: order_id - - type: - name: arguments___order_metainfo_collection___update___order_metainfo_collection_update___store___store - kind: INPUT_OBJECT - name: store - name: order_metainfo_collection_update - description: generated from avro-schema for order_metainfo_collection_update - kind: SCALAR name: Boolean description: The `Boolean` scalar type represents `true` or `false`. - - interfaces: *0 - fields: - - isDeprecated: false - args: *0 - type: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - name: state - - isDeprecated: false - args: *0 - type: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - name: zip - - isDeprecated: false - args: *0 - type: - ofType: - name: String - kind: SCALAR - kind: NON_NULL - name: city - - isDeprecated: false - args: *0 - type: + - kind: SCALAR + name: Int + description: The `Int` scalar type represents non-fractional signed whole numeric + values. Int can represent values between -(2^31) and 2^31 - 1. + - kind: INPUT_OBJECT + inputFields: + - type: ofType: name: String kind: SCALAR kind: NON_NULL - name: street - kind: OBJECT - name: order_metainfo_collection___store___store___address___address - description: generated from avro-schema for address + name: string + name: arguments___order_metainfo_collection___update___order_metainfo_collection_update___store___store___external_id___external_id___String_box + description: Box (wrapper) around union variant - possibleTypes: - name: Int_box kind: OBJECT @@ -1802,7 +1842,7 @@ local function run_queries(gql_wrapper) ]]):strip()) -- luacheck: pop - -- luacheck: push max line length 152 + -- luacheck: push max line length 156 local exp_result_avro_schema_2 = yaml.decode(([[ --- __schema: diff --git a/test/testdata/common_testdata.lua b/test/testdata/common_testdata.lua index 9f7f435..9845825 100644 --- a/test/testdata/common_testdata.lua +++ b/test/testdata/common_testdata.lua @@ -360,7 +360,7 @@ end function common_testdata.run_queries(gql_wrapper) local test = tap.test('common') - test:plan(46) + test:plan(53) local query_1 = [[ query user_by_order($order_id: String) { @@ -1512,6 +1512,135 @@ function common_testdata.run_queries(gql_wrapper) -- }}} -- }}} + -- {{{ check variable type againts argument type + + local query_15 = [[ + mutation($tags: [Int!]) { + order_metainfo_collection( + update: {store: {tags: $tags}} + limit: 1 + ) { + order_metainfo_id + store { tags } + } + } + ]] + local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_15) + local err_exp = 'Variable "tags" type mismatch: the variable type ' .. + '"List(NonNull(Int))" is not compatible with the argument type ' .. + '"List(NonNull(String))"' + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + 'variable usage inside InputObject') + + local query_16 = [[ + mutation($tag_value: Int) { + order_metainfo_collection( + update: {store: {parametrized_tags: {foo: $tag_value}}} + limit: 1 + ) { + order_metainfo_id + } + } + ]] + local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_16) + local err_exp = 'Variable "tag_value" type mismatch: the variable type ' .. + '"Int" is not compatible with the argument type "NonNull(String)"' + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + 'variable usage inside InputMap') + + local query_17 = [[ + mutation($map_value: Int) { + order_metainfo_collection( + update: {store: {parametrized_tags: $map_value}} + limit: 1 + ) { + order_metainfo_id + } + } + ]] + local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_17) + local err_exp = 'Variable "map_value" type mismatch: the variable type ' .. + '"Int" is not compatible with the argument type "arguments___' .. + 'order_metainfo_collection___update___order_metainfo_collection_' .. + 'update___store___store___parametrized_tags___InputMap"' + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + 'variable usage as InputMap') + + local query_18 = [[ + mutation($id_value: Float!) { + order_metainfo_collection( + update: {store: {external_id: {int: $id_value}}} + limit: 1 + ) { + order_metainfo_id + } + } + ]] + local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_18) + local err_exp = 'Variable "id_value" type mismatch: the variable type ' .. + '"NonNull(Float)" is not compatible with the argument type ' .. + '"arguments___order_metainfo_collection___update___order_metainfo_' .. + 'collection_update___store___store___external_id___external_id___' .. + 'Int_box"' + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + 'variable usage inside InputUnion') + + local query_19 = [[ + mutation($id_box_value: Int) { + order_metainfo_collection( + update: {store: {external_id: $id_box_value}} + limit: 1 + ) { + order_metainfo_id + } + } + ]] + local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_19) + local err_exp = 'Variable "id_box_value" type mismatch: the variable ' .. + 'type "Int" is not compatible with the argument type "arguments___' .. + 'order_metainfo_collection___update___order_metainfo_collection_' .. + 'update___store___store___external_id___external_id"' + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + 'variable usage as InputUnion') + + local box_t = 'arguments___order_metainfo_collection___update___' .. + 'order_metainfo_collection_update___store___store___external_id___' .. + 'external_id___Int_box' + local query_20 = [[ + mutation($id_box_value: ]] .. box_t .. [[) { + order_metainfo_collection( + update: {store: {external_id: $id_box_value}} + limit: 1 + ) { + order_metainfo_id + } + } + ]] + local ok, _ = pcall(gql_wrapper.compile, gql_wrapper, query_20) + test:is(ok, true, 'correct variable usage as InputUnion') + + -- {{{ nullable variable for NonNull argument + local query_21 = [[ + mutation($first_name: String) { + user_collection(insert: { + user_id: "user_id_new" + first_name: $first_name + middle_name: "middle name new" + last_name: "last name new" + }) { + user_id + } + } + ]] + local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_21) + local err_exp = 'Variable "first_name" type mismatch: the variable type ' .. + '"String" is not compatible with the argument type "NonNull(String)"' + test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + 'nullable variable for non-null argument') + -- }}} + + -- }}} + assert(test:check(), 'check plan') end diff --git a/test/testdata/compound_index_testdata.lua b/test/testdata/compound_index_testdata.lua index f312804..a58a713 100644 --- a/test/testdata/compound_index_testdata.lua +++ b/test/testdata/compound_index_testdata.lua @@ -1318,7 +1318,7 @@ function compound_index_testdata.run_queries(gql_wrapper) -- (top-level collection, full primary key) local query_6 = [[ - query users($limit: Int, $user_str: String, $user_num: Long) { + query users($limit: Int, $user_str: String!, $user_num: Long!) { user_collection(limit: $limit, offset: {user_str: $user_str, user_num: $user_num}) { user_str From 9d681cdaa5afdd3121b067004e4af273c804f7c8 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Mon, 2 Jul 2018 05:59:51 +0300 Subject: [PATCH 17/20] Disable variable check cases on avro-schema-2* It is because the cases use mutation arguments, which is disabled on avro-schema-2*. --- test/testdata/common_testdata.lua | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/testdata/common_testdata.lua b/test/testdata/common_testdata.lua index 9845825..e00fae0 100644 --- a/test/testdata/common_testdata.lua +++ b/test/testdata/common_testdata.lua @@ -359,8 +359,9 @@ function common_testdata.drop_spaces() end function common_testdata.run_queries(gql_wrapper) + local avro_version = test_utils.major_avro_schema_version() local test = tap.test('common') - test:plan(53) + test:plan(avro_version == 3 and 53 or 30) local query_1 = [[ query user_by_order($order_id: String) { @@ -1302,6 +1303,11 @@ function common_testdata.run_queries(gql_wrapper) -- }}} + if avro_version == 2 then + assert(test:check(), 'check plan') + return + end + local query_13 = [[ mutation($xorder_metainfo: order_metainfo_collection_update) { order_metainfo_collection(update: $xorder_metainfo, limit: 1) { From a8b3650bd798bd73722a2efb119be2d90fee32ea Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Mon, 2 Jul 2018 07:26:35 +0300 Subject: [PATCH 18/20] Runtime check for nested non-null arguments It would be better to implement this check as compile-time via rules.requiredArgumentsPresent or a similar rule for argument/InputObject. Related to #159. Related to #160. --- graphql/core/execute.lua | 3 +- graphql/core/rules.lua | 27 +--------- graphql/core/util.lua | 85 ++++++++++++++++++++++++++----- test/testdata/common_testdata.lua | 25 ++++++++- 4 files changed, 99 insertions(+), 41 deletions(-) diff --git a/graphql/core/execute.lua b/graphql/core/execute.lua index 6eadcb1..657b578 100644 --- a/graphql/core/execute.lua +++ b/graphql/core/execute.lua @@ -93,7 +93,8 @@ local function getFieldEntry(objectType, object, fields, context) local arguments = util.map(fieldType.arguments or {}, function(argument, name) local supplied = argumentMap[name] and argumentMap[name].value - supplied = supplied and util.coerceValue(supplied, argument, context.variables) + supplied = util.coerceValue(supplied, argument, context.variables, + {strict_non_null = true}) if supplied ~= nil then return supplied else diff --git a/graphql/core/rules.lua b/graphql/core/rules.lua index 96a9614..97636fa 100644 --- a/graphql/core/rules.lua +++ b/graphql/core/rules.lua @@ -1,4 +1,3 @@ -local yaml = require('yaml') local path = (...):gsub('%.[^%.]+$', '') local types = require(path .. '.types') local util = require(path .. '.util') @@ -502,30 +501,6 @@ local function isTypeSubTypeOf(subType, superType, context) return false end -local function getTypeName(t) - if t.name ~= nil then - if t.name == 'Scalar' and t.subtype == 'InputMap' then - return ('InputMap(%s)'):format(getTypeName(t.values)) - elseif t.name == 'Scalar' and t.subtype == 'InputUnion' then - local typeNames = {} - for _, child in ipairs(t.types) do - table.insert(typeNames, getTypeName(child)) - end - return ('InputUnion(%s)'):format(table.concat(typeNames, ',')) - end - return t.name - elseif t.__type == 'NonNull' then - return ('NonNull(%s)'):format(getTypeName(t.ofType)) - elseif t.__type == 'List' then - return ('List(%s)'):format(getTypeName(t.ofType)) - end - - local orig_encode_use_tostring = yaml.cfg.encode_use_tostring - local err = ('Internal error: unknown type:\n%s'):format(yaml.encode(t)) - yaml.cfg({encode_use_tostring = orig_encode_use_tostring}) - error(err) -end - local function isVariableTypesValid(argument, argumentType, context, variableMap) if argument.value.kind == 'variable' then @@ -544,7 +519,7 @@ local function isVariableTypesValid(argument, argumentType, context, if not isTypeSubTypeOf(variableType, argumentType, context) then return false, ('Variable "%s" type mismatch: the variable type "%s" ' .. 'is not compatible with the argument type "%s"'):format(variableName, - getTypeName(variableType), getTypeName(argumentType)) + util.getTypeName(variableType), util.getTypeName(argumentType)) end elseif argument.value.kind == 'inputObject' then -- find variables deeper diff --git a/graphql/core/util.lua b/graphql/core/util.lua index 01dc2d9..4d49400 100644 --- a/graphql/core/util.lua +++ b/graphql/core/util.lua @@ -1,3 +1,5 @@ +local yaml = require('yaml') + local util = {} function util.map(t, fn) @@ -45,11 +47,42 @@ function util.trim(s) return s:gsub('^%s+', ''):gsub('%s+$', ''):gsub('%s%s+', ' ') end -function util.coerceValue(node, schemaType, variables) - variables = variables or {} +function util.getTypeName(t) + if t.name ~= nil then + if t.name == 'Scalar' and t.subtype == 'InputMap' then + return ('InputMap(%s)'):format(util.getTypeName(t.values)) + elseif t.name == 'Scalar' and t.subtype == 'InputUnion' then + local typeNames = {} + for _, child in ipairs(t.types) do + table.insert(typeNames, util.getTypeName(child)) + end + return ('InputUnion(%s)'):format(table.concat(typeNames, ',')) + end + return t.name + elseif t.__type == 'NonNull' then + return ('NonNull(%s)'):format(util.getTypeName(t.ofType)) + elseif t.__type == 'List' then + return ('List(%s)'):format(util.getTypeName(t.ofType)) + end + + local orig_encode_use_tostring = yaml.cfg.encode_use_tostring + local err = ('Internal error: unknown type:\n%s'):format(yaml.encode(t)) + yaml.cfg({encode_use_tostring = orig_encode_use_tostring}) + error(err) +end + +function util.coerceValue(node, schemaType, variables, opts) + local variables = variables or {} + local opts = opts or {} + local strict_non_null = opts.strict_non_null or false if schemaType.__type == 'NonNull' then - return util.coerceValue(node, schemaType.ofType, variables) + local res = util.coerceValue(node, schemaType.ofType, variables, opts) + if strict_non_null and res == nil then + error(('Expected non-null for "%s", got null'):format( + util.getTypeName(schemaType))) + end + return res end if not node then @@ -66,7 +99,7 @@ function util.coerceValue(node, schemaType, variables) end return util.map(node.values, function(value) - return util.coerceValue(value, schemaType.ofType, variables) + return util.coerceValue(value, schemaType.ofType, variables, opts) end) end @@ -76,25 +109,51 @@ function util.coerceValue(node, schemaType, variables) local isInputUnion = schemaType.__type == 'Scalar' and schemaType.subtype == 'InputUnion' - if isInputObject or isInputMap then + if isInputObject then if node.kind ~= 'inputObject' then error('Expected an input object') end + -- check all fields: as from value as well as from schema + local fieldNameSet = {} + local fieldValues = {} + for _, field in ipairs(node.values) do + fieldNameSet[field.name] = true + fieldValues[field.name] = field.value + end + for fieldName, _ in pairs(schemaType.fields) do + fieldNameSet[fieldName] = true + end + local inputObjectValue = {} - for _, field in pairs(node.values) do - if isInputObject and not schemaType.fields[field.name] then - error('Unknown input object field "' .. field.name .. '"') + for fieldName, _ in pairs(fieldNameSet) do + if not schemaType.fields[fieldName] then + error('Unknown input object field "' .. fieldName .. '"') end - local child_type = isInputObject and schemaType.fields[field.name].kind or - schemaType.values - inputObjectValue[field.name] = util.coerceValue(field.value, child_type, - variables) + local childValue = fieldValues[fieldName] + local childType = schemaType.fields[fieldName].kind + inputObjectValue[fieldName] = util.coerceValue(childValue, childType, + variables, opts) end + return inputObjectValue end + if isInputMap then + if node.kind ~= 'inputObject' then + error('Expected an input object') + end + + local inputMapValue = {} + for _, field in pairs(node.values) do + local childType = schemaType.values + inputMapValue[field.name] = util.coerceValue(field.value, childType, + variables, opts) + end + return inputMapValue + end + if schemaType.__type == 'Enum' then if node.kind ~= 'enum' then error('Expected enum value, got ' .. node.kind) @@ -109,7 +168,7 @@ function util.coerceValue(node, schemaType, variables) if isInputUnion then local child_type = schemaType.resolveNodeType(node) - return util.coerceValue(node, child_type, variables) + return util.coerceValue(node, child_type, variables, opts) end if schemaType.__type == 'Scalar' then diff --git a/test/testdata/common_testdata.lua b/test/testdata/common_testdata.lua index e00fae0..e474be5 100644 --- a/test/testdata/common_testdata.lua +++ b/test/testdata/common_testdata.lua @@ -361,7 +361,7 @@ end function common_testdata.run_queries(gql_wrapper) local avro_version = test_utils.major_avro_schema_version() local test = tap.test('common') - test:plan(avro_version == 3 and 53 or 30) + test:plan(avro_version == 3 and 54 or 30) local query_1 = [[ query user_by_order($order_id: String) { @@ -1645,6 +1645,29 @@ function common_testdata.run_queries(gql_wrapper) 'nullable variable for non-null argument') -- }}} + -- {{{ lack of non-null argument + + local query_22 = [[ + mutation { + user_collection(insert: { + user_id: "user_id_new" + # no first_name field + middle_name: "middle name new" + last_name: "last name new" + }) { + user_id + } + } + ]] + local gql_query_22 = test_utils.show_trace(function() + return gql_wrapper:compile(query_22) + end) + local result = gql_query_22:execute({}) + local err = result.errors[1].message + local err_exp = 'Expected non-null for "NonNull(String)", got null' + test:is(err, err_exp, 'lack of non-null argument') + -- }}} + -- }}} assert(test:check(), 'check plan') From 7fc9c49575dbe507c2058f9a0a7d93503c7405ec Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Mon, 2 Jul 2018 16:46:56 +0300 Subject: [PATCH 19/20] Add TYPE_MISMATCH and WRONG_VALUE error codes Part of #134. --- graphql/convert_schema/union.lua | 32 +++--- graphql/core/rules.lua | 5 +- graphql/core/util.lua | 23 ++-- graphql/core/validate_variables.lua | 77 ++++++++------ graphql/error_codes.lua | 24 +++++ graphql/init.lua | 9 ++ graphql/utils.lua | 41 ++++++-- test/common/mutation.test.lua | 40 +++---- test/space/init_fail.test.lua | 8 +- test/testdata/common_testdata.lua | 158 ++++++++++++++++++++-------- 10 files changed, 285 insertions(+), 132 deletions(-) create mode 100644 graphql/error_codes.lua diff --git a/graphql/convert_schema/union.lua b/graphql/convert_schema/union.lua index 156adb6..bf886c4 100644 --- a/graphql/convert_schema/union.lua +++ b/graphql/convert_schema/union.lua @@ -2,9 +2,11 @@ local yaml = require('yaml') local core_types = require('graphql.core.types') local avro_helpers = require('graphql.avro_helpers') local helpers = require('graphql.convert_schema.helpers') - +local error_codes = require('graphql.error_codes') local utils = require('graphql.utils') + local check = utils.check +local e = error_codes local union = {} @@ -303,28 +305,34 @@ function union.convert(avro_schema, opts) types = union_types, name = helpers.full_name(union_name, context), resolveType = function(result) - assert(type(result) == 'table', - 'union value must be a map with one field, got ' .. - type(result)) - assert(next(result) ~= nil and next(result, next(result)) == nil, - 'union value must have only one field') + if type(result) ~= 'table' then + error(e.wrong_value('union value must be a map with one ' .. + 'field, got ' .. type(result))) + end + if next(result) == nil or next(result, next(result)) ~= nil then + error(e.wrong_value('union value must have only one field')) + end for determinant, type in pairs(determinant_to_type) do if result[determinant] ~= nil then return type end end local field_name = tostring(next(result)) - error(('unexpected union value field: %s'):format(field_name)) + error(e.wrong_value(('unexpected union value field: %s'):format( + field_name))) end, resolveNodeType = function(node) - assert(#node.values == 1, - ('box object with more then one field: %d'):format( - #node.values)) + if #node.values ~= 1 then + error(e.wrong_value('box object with more then one field: %d') + :format(#node.values)) + end local determinant = node.values[1].name check(determinant, 'determinant', 'string') local res = determinant_to_type[determinant] - assert(determinant ~= nil, - ('the union has no "%s" field'):format(determinant)) + if res == nil then + error(e.wrong_value('the union has no "%s" field'):format( + determinant)) + end return res end, }) diff --git a/graphql/core/rules.lua b/graphql/core/rules.lua index 97636fa..2078569 100644 --- a/graphql/core/rules.lua +++ b/graphql/core/rules.lua @@ -3,8 +3,11 @@ local types = require(path .. '.types') local util = require(path .. '.util') local introspection = require(path .. '.introspection') local query_util = require(path .. '.query_util') +local graphql_error_codes = require('graphql.error_codes') local graphql_utils = require('graphql.utils') +local e = graphql_error_codes + local function getParentField(context, name, count) if introspection.fieldMap[name] then return introspection.fieldMap[name] end @@ -593,7 +596,7 @@ function rules.variableUsageAllowed(node, context) local ok, err = isVariableTypesValid(argument, argumentType, context, variableMap) if not ok then - error(err) + error(e.type_mismatch(err)) end end end diff --git a/graphql/core/util.lua b/graphql/core/util.lua index 4d49400..9f6e826 100644 --- a/graphql/core/util.lua +++ b/graphql/core/util.lua @@ -1,4 +1,7 @@ local yaml = require('yaml') +local graphql_error_codes = require('graphql.error_codes') + +local e = graphql_error_codes local util = {} @@ -79,8 +82,8 @@ function util.coerceValue(node, schemaType, variables, opts) if schemaType.__type == 'NonNull' then local res = util.coerceValue(node, schemaType.ofType, variables, opts) if strict_non_null and res == nil then - error(('Expected non-null for "%s", got null'):format( - util.getTypeName(schemaType))) + error(e.wrong_value(('Expected non-null for "%s", got null'):format( + util.getTypeName(schemaType)))) end return res end @@ -95,7 +98,7 @@ function util.coerceValue(node, schemaType, variables, opts) if schemaType.__type == 'List' then if node.kind ~= 'list' then - error('Expected a list') + error(e.wrong_value('Expected a list')) end return util.map(node.values, function(value) @@ -111,7 +114,7 @@ function util.coerceValue(node, schemaType, variables, opts) if isInputObject then if node.kind ~= 'inputObject' then - error('Expected an input object') + error(e.wrong_value('Expected an input object')) end -- check all fields: as from value as well as from schema @@ -128,7 +131,8 @@ function util.coerceValue(node, schemaType, variables, opts) local inputObjectValue = {} for fieldName, _ in pairs(fieldNameSet) do if not schemaType.fields[fieldName] then - error('Unknown input object field "' .. fieldName .. '"') + error(e.wrong_value(('Unknown input object field "%s"'):format( + fieldName))) end local childValue = fieldValues[fieldName] @@ -142,7 +146,7 @@ function util.coerceValue(node, schemaType, variables, opts) if isInputMap then if node.kind ~= 'inputObject' then - error('Expected an input object') + error(e.wrong_value('Expected an input object')) end local inputMapValue = {} @@ -156,11 +160,11 @@ function util.coerceValue(node, schemaType, variables, opts) if schemaType.__type == 'Enum' then if node.kind ~= 'enum' then - error('Expected enum value, got ' .. node.kind) + error(e.wrong_value('Expected enum value, got %s'):format(node.kind)) end if not schemaType.values[node.value] then - error('Invalid enum value "' .. node.value .. '"') + error(e.wrong_value('Invalid enum value "%s"'):format(node.value)) end return node.value @@ -173,7 +177,8 @@ function util.coerceValue(node, schemaType, variables, opts) if schemaType.__type == 'Scalar' then if schemaType.parseLiteral(node) == nil then - error('Could not coerce "' .. tostring(node.value) .. '" to "' .. schemaType.name .. '"') + error(e.wrong_value('Could not coerce "%s" to "%s"'):format( + tostring(node.value), schemaType.name)) end return schemaType.parseLiteral(node) diff --git a/graphql/core/validate_variables.lua b/graphql/core/validate_variables.lua index 4d1894b..8bc2bda 100644 --- a/graphql/core/validate_variables.lua +++ b/graphql/core/validate_variables.lua @@ -1,7 +1,9 @@ local types = require('graphql.core.types') local graphql_utils = require('graphql.utils') +local graphql_error_codes = require('graphql.error_codes') local check = graphql_utils.check +local e = graphql_error_codes local validate_variables = {} @@ -14,8 +16,10 @@ local function checkVariableValue(variableName, value, variableType) if isNonNull then variableType = types.nullable(variableType) - assert(value ~= nil, - ('Variable "%s" expected to be non-null'):format(variableName)) + if value == nil then + error(e.wrong_value(('Variable "%s" expected to be non-null'):format( + variableName))) + end end local isList = variableType.__type == 'List' @@ -29,12 +33,14 @@ local function checkVariableValue(variableName, value, variableType) if value == nil then return end if isList then - assert(type(value) == 'table', - ('Variable "%s" for a List must be a Lua table, got %s') - :format(variableName, type(value))) - assert(graphql_utils.is_array(value), - ('Variable "%s" for a List must be an array, got map') - :format(variableName)) + if type(value) ~= 'table' then + error(e.wrong_value(('Variable "%s" for a List must be a Lua ' .. + 'table, got %s'):format(variableName, type(value)))) + end + if not graphql_utils.is_array(value) then + error(e.wrong_value(('Variable "%s" for a List must be an array, ' .. + 'got map'):format(variableName))) + end assert(variableType.ofType ~= nil, 'variableType.ofType must not be nil') for i, item in ipairs(value) do local itemName = variableName .. '[' .. tostring(i) .. ']' @@ -44,9 +50,11 @@ local function checkVariableValue(variableName, value, variableType) end if isInputObject then - assert(type(value) == 'table', - ('Variable "%s" for the InputObject "%s" must be a Lua table, ' .. - 'got %s'):format(variableName, variableType.name, type(value))) + if type(value) ~= 'table' then + error(e.wrong_value(('Variable "%s" for the InputObject "%s" must ' .. + 'be a Lua table, got %s'):format(variableName, variableType.name, + type(value)))) + end -- check all fields: as from value as well as from schema local fieldNameSet = {} @@ -59,13 +67,16 @@ local function checkVariableValue(variableName, value, variableType) for fieldName, _ in pairs(fieldNameSet) do local fieldValue = value[fieldName] - assert(type(fieldName) == 'string', - ('Field key of the variable "%s" for the InputObject "%s" ' .. - 'must be a string, got %s'):format(variableName, variableType.name, - type(fieldName))) - assert(type(variableType.fields[fieldName]) ~= 'nil', - ('Unknown field "%s" of the variable "%s" for the ' .. - 'InputObject "%s"'):format(fieldName, variableName, variableType.name)) + if type(fieldName) ~= 'string' then + error(e.wrong_value(('Field key of the variable "%s" for the ' .. + 'InputObject "%s" must be a string, got %s'):format(variableName, + variableType.name, type(fieldName)))) + end + if type(variableType.fields[fieldName]) == 'nil' then + error(e.wrong_value(('Unknown field "%s" of the variable "%s" ' .. + 'for the InputObject "%s"'):format(fieldName, variableName, + variableType.name))) + end local childType = variableType.fields[fieldName].kind local childName = variableName .. '.' .. fieldName @@ -76,15 +87,18 @@ local function checkVariableValue(variableName, value, variableType) end if isInputMap then - assert(type(value) == 'table', - ('Variable "%s" for the InputMap "%s" must be a Lua table, got %s') - :format(variableName, variableType.name, type(value))) + if type(value) ~= 'table' then + error(e.wrong_value(('Variable "%s" for the InputMap "%s" must be a ' .. + 'Lua table, got %s'):format(variableName, variableType.name, + type(value)))) + end for fieldName, fieldValue in pairs(value) do - assert(type(fieldName) == 'string', - ('Field key of the variable "%s" for the InputMap "%s" must be a ' .. - 'string, got %s'):format(variableName, variableType.name, - type(fieldName))) + if type(fieldName) ~= 'string' then + error(e.wrong_value(('Field key of the variable "%s" for the ' .. + 'InputMap "%s" must be a string, got %s'):format(variableName, + variableType.name, type(fieldName)))) + end local childType = variableType.values local childName = variableName .. '.' .. fieldName checkVariableValue(childName, fieldValue, childType) @@ -103,9 +117,10 @@ local function checkVariableValue(variableName, value, variableType) if isScalar then check(variableType.isValueOfTheType, 'isValueOfTheType', 'function') - assert(variableType.isValueOfTheType(value), - ('Wrong variable "%s" for the Scalar "%s"'):format( - variableName, variableType.name)) + if not variableType.isValueOfTheType(value) then + error(e.wrong_value(('Wrong variable "%s" for the Scalar "%s"'):format( + variableName, variableType.name))) + end return end @@ -115,8 +130,10 @@ end function validate_variables.validate_variables(context) -- check that all variable values have corresponding variable declaration for variableName, _ in pairs(context.variables or {}) do - assert(context.variableTypes[variableName] ~= nil, - ('There is no declaration for the variable "%s"'):format(variableName)) + if context.variableTypes[variableName] == nil then + error(e.wrong_value(('There is no declaration for the variable "%s"') + :format(variableName))) + end end -- check that variable values have correct type diff --git a/graphql/error_codes.lua b/graphql/error_codes.lua new file mode 100644 index 0000000..a1294c5 --- /dev/null +++ b/graphql/error_codes.lua @@ -0,0 +1,24 @@ +local error_codes = {} + +error_codes.TYPE_MISMATCH = 1 +error_codes.WRONG_VALUE = 2 + +function error_codes.type_mismatch(message) + return { + message = message, + extensions = { + error_code = error_codes.TYPE_MISMATCH, + } + } +end + +function error_codes.wrong_value(message) + return { + message = message, + extensions = { + error_code = error_codes.WRONG_VALUE, + } + } +end + +return error_codes diff --git a/graphql/init.lua b/graphql/init.lua index 9042ee3..39f0e39 100644 --- a/graphql/init.lua +++ b/graphql/init.lua @@ -34,12 +34,21 @@ local accessor_general = require('graphql.accessor_general') local accessor_space = require('graphql.accessor_space') local accessor_shard = require('graphql.accessor_shard') local impl = require('graphql.impl') +local error_codes = require('graphql.error_codes') local graphql = {} -- constants graphql.TIMEOUT_INFINITY = accessor_general.TIMEOUT_INFINITY +-- error codes +graphql.error_codes = {} +for k, v in pairs(error_codes) do + if type(v) == 'number' then + graphql.error_codes[k] = v + end +end + -- for backward compatibility graphql.accessor_general = accessor_general graphql.accessor_space = accessor_space diff --git a/graphql/utils.lua b/graphql/utils.lua index 89f40e0..2e946b2 100644 --- a/graphql/utils.lua +++ b/graphql/utils.lua @@ -225,22 +225,41 @@ function utils.optional_require_rex() end function utils.serialize_error(err, traceback) - local extensions = {traceback = traceback} + local def_extensions = {traceback = traceback} if type(err) == 'string' then return { message = utils.strip_error(err), - extensions = extensions, + extensions = def_extensions, } elseif type(err) == 'cdata' and tostring(ffi.typeof(err)) == 'ctype' then return { message = tostring(err), - extensions = extensions, + extensions = def_extensions, } - elseif type(err) == 'table' and type(err.message) == 'string' then - local err = table.copy(err) - err.extensions = extensions - return err + elseif type(err) == 'table' then + local res = {} + local ok = true + for k, v in pairs(err) do + if k == 'message' then + ok = ok and type(v) == 'string' + res.message = v + elseif k == 'extensions' then + ok = ok and type(v) == 'table' + res.extensions = table.copy(v) + -- add def_extensions fields to res.extensions + for k, v in pairs(def_extensions) do + if res.extensions[k] == nil then + res.extensions[k] = v + end + end + else + ok = false + end + end + if ok then + return res + end end local message = 'internal error: unknown error format' @@ -248,11 +267,13 @@ function utils.serialize_error(err, traceback) json.cfg({encode_use_tostring = true}) local orig_error = json.encode(err) json.cfg({encode_use_tostring = encode_use_tostring_orig}) - extensions.orig_error = orig_error - return { + + local res = { message = message, - extensions = extensions, + extensions = def_extensions, } + res.extensions.orig_error = orig_error + return res end return utils diff --git a/test/common/mutation.test.lua b/test/common/mutation.test.lua index 7b06e29..d218e21 100755 --- a/test/common/mutation.test.lua +++ b/test/common/mutation.test.lua @@ -435,8 +435,8 @@ local function run_queries(gql_wrapper, virtbox, meta) } ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, mutation_insert_2) - local err_exp = 'Non-existent argument "insert"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + local exp_err = 'Non-existent argument "insert"' + test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, '"insert" argument is forbidden in a non-top level field') -- test "insert" argument is forbidden in a query @@ -450,8 +450,8 @@ local function run_queries(gql_wrapper, virtbox, meta) } ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_insert) - local err_exp = 'Non-existent argument "insert"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + local exp_err = 'Non-existent argument "insert"' + test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, '"insert" argument is forbidden in a query') -- test "insert" argument is forbidden with object arguments @@ -471,8 +471,8 @@ local function run_queries(gql_wrapper, virtbox, meta) local gql_mutation_insert_3i = gql_wrapper:compile(mutation_insert_3i) local result = gql_mutation_insert_3i:execute({}) local err = result.errors[1].message - local err_exp = '"insert" must be the only argument when it is present' - test:is(err, err_exp, + local exp_err = '"insert" must be the only argument when it is present' + test:is(err, exp_err, '"insert" argument is forbidden with other filters (object arguments)') -- test "insert" argument is forbidden with list arguments @@ -492,8 +492,8 @@ local function run_queries(gql_wrapper, virtbox, meta) local gql_mutation_insert_4i = gql_wrapper:compile(mutation_insert_4i) local result = gql_mutation_insert_4i:execute({}) local err = result.errors[1].message - local err_exp = '"insert" must be the only argument when it is present' - test:is(err, err_exp, + local exp_err = '"insert" must be the only argument when it is present' + test:is(err, exp_err, '"insert" argument is forbidden with other filters (list arguments)') -- test "insert" argument is forbidden with other extra argument @@ -513,8 +513,8 @@ local function run_queries(gql_wrapper, virtbox, meta) local gql_mutation_insert_5i = gql_wrapper:compile(mutation_insert_5i) local result = gql_mutation_insert_5i:execute({}) local err = result.errors[1].message - local err_exp = '"insert" must be the only argument when it is present' - test:is(err, err_exp, + local exp_err = '"insert" must be the only argument when it is present' + test:is(err, exp_err, '"insert" argument is forbidden with other filters (extra arguments)') -- test inserting an object into a collection with subrecord, union, array @@ -977,8 +977,8 @@ local function run_queries(gql_wrapper, virtbox, meta) } ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_update) - local err_exp = 'Non-existent argument "update"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + local exp_err = 'Non-existent argument "update"' + test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, '"update" argument is forbidden in a query') -- test updating of a field by which a shard key is calculated (it is the @@ -1137,9 +1137,9 @@ local function run_queries(gql_wrapper, virtbox, meta) } local result = gql_mutation_update_4:execute(variables_update_4) local err = result.errors[1].message - local err_exp = 'Unknown field "user_id" of the variable "xuser" ' .. + local exp_err = 'Unknown field "user_id" of the variable "xuser" ' .. 'for the InputObject "user_collection_update"' - test:is(err, err_exp, + test:is(err, exp_err, 'updating of a field of a primary key when it is NOT shard key field') local mutation_update_5 = [[ @@ -1162,9 +1162,9 @@ local function run_queries(gql_wrapper, virtbox, meta) } local result = gql_mutation_update_5:execute(variables_update_5) local err = result.errors[1].message - local err_exp = 'Unknown field "order_id" of the variable "xorder" ' .. + local exp_err = 'Unknown field "order_id" of the variable "xorder" ' .. 'for the InputObject "order_collection_update"' - test:is(err, err_exp, + test:is(err, exp_err, 'updating of a field of a primary key when it is shard key field') -- }}} @@ -1278,8 +1278,8 @@ local function run_queries(gql_wrapper, virtbox, meta) } ]] local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_delete) - local err_exp = 'Non-existent argument "delete"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + local exp_err = 'Non-existent argument "delete"' + test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, '"delete" argument is forbidden in a query') -- }}} @@ -1305,9 +1305,9 @@ local function run_queries_avro_schema_2(test, enable_mutations, gql_wrapper, if enable_mutations then test:ok(ok, 'mutations are enabled with the enable_mutations flag') else - local err_exp = 'Variable specifies unknown type ' .. + local exp_err = 'Variable specifies unknown type ' .. '"user_collection_insert"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, 'mutations are forbidden for avro-schema-2*') end end diff --git a/test/space/init_fail.test.lua b/test/space/init_fail.test.lua index 78504f1..7ee1c73 100755 --- a/test/space/init_fail.test.lua +++ b/test/space/init_fail.test.lua @@ -54,11 +54,11 @@ local test = tap.test('init_fail') test:plan(3) local ok, err = pcall(create_gql_wrapper, metadata) -local err_exp = '1:1 connection "user_connection" of collection ' .. +local exp_err = '1:1 connection "user_connection" of collection ' .. '"order_collection" has less fields than the index of ' .. '"user_str_num_index" collection (cannot prove uniqueness of the partial ' .. 'index)' -test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, +test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, 'not enough fields') -- restore back cut part @@ -80,10 +80,10 @@ metadata.indexes.user_collection.user_str_index = { } local ok, err = pcall(create_gql_wrapper, metadata) -local err_exp = 'several indexes were marked as primary in the ' .. +local exp_err = 'several indexes were marked as primary in the ' .. '"user_collection" collection, at least "user_str_num_index" and ' .. '"user_str_index"' -test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, +test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, 'multiple primary indexes') -- restore metadata back diff --git a/test/testdata/common_testdata.lua b/test/testdata/common_testdata.lua index e474be5..355634f 100644 --- a/test/testdata/common_testdata.lua +++ b/test/testdata/common_testdata.lua @@ -3,9 +3,15 @@ local json = require('json') local yaml = require('yaml') local utils = require('graphql.utils') local test_utils = require('test.test_utils') +local graphql = require('graphql') + +local e = graphql.error_codes local common_testdata = {} +-- forward declaration +local type_mismatch_cases + -- needed to compare a dump with floats/doubles, because, say, -- `tonumber(tostring(1/3)) == 1/3` is `false` local function deeply_number_tostring(t) @@ -451,10 +457,10 @@ function common_testdata.run_queries(gql_wrapper) } ]] - local err_exp = 'Cannot have more than one operation when using ' .. + local exp_err = 'Cannot have more than one operation when using ' .. 'anonymous operations' local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_1tn) - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, utils.strip_error(err)}, {false, exp_err}, 'unnamed query should be a single one') local query_1t = [[ @@ -481,16 +487,16 @@ function common_testdata.run_queries(gql_wrapper) return gql_wrapper:compile(query_1t) end) - local err_exp = 'Operation name must be specified if more than one ' .. + local exp_err = 'Operation name must be specified if more than one ' .. 'operation exists.' local result = gql_query_1t:execute({}) local err = result.errors[1].message - test:is(err, err_exp, 'non-determined query name should give an error') + test:is(err, exp_err, 'non-determined query name should give an error') - local err_exp = 'Unknown operation "non_existent_operation"' + local exp_err = 'Unknown operation "non_existent_operation"' local result = gql_query_1t:execute({}, 'non_existent_operation') local err = result.errors[1].message - test:is(err, err_exp, 'wrong operation name should give an error') + test:is(err, exp_err, 'wrong operation name should give an error') test_utils.show_trace(function() local result = gql_query_1t:execute({}, 'user_by_order') @@ -1268,6 +1274,14 @@ function common_testdata.run_queries(gql_wrapper) -- }}} + type_mismatch_cases(gql_wrapper, test) +end + +-- extracted from run_queries to prevent 'function at line NNN has more than +-- 200 local variables' error +type_mismatch_cases = function(gql_wrapper, test) + local avro_version = test_utils.major_avro_schema_version() + -- {{{ fail cases for a variable type local query_12 = [[ @@ -1287,19 +1301,25 @@ function common_testdata.run_queries(gql_wrapper) local exp_err = 'Variable "order_id" expected to be non-null' local result = gql_query_12:execute(variables_12_1) local err = result.errors[1].message - test:is(err, exp_err, 'nil for a non-null type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'nil for a non-null type') local variables_12_2 = {order_id = box.NULL} local exp_err = 'Variable "order_id" expected to be non-null' local result = gql_query_12:execute(variables_12_2) local err = result.errors[1].message - test:is(err, exp_err, 'box.NULL for a non-null type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'box.NULL for a non-null type') local variables_12_3 = {order_id = 42} local exp_err = 'Wrong variable "order_id" for the Scalar "String"' local result = gql_query_12:execute(variables_12_3) local err = result.errors[1].message - test:is(err, exp_err, 'Int for a String type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'Int for a String type') -- }}} @@ -1326,21 +1346,27 @@ function common_testdata.run_queries(gql_wrapper) 'must be a Lua table, got string' local result = gql_query_13:execute(variables_13_1) local err = result.errors[1].message - test:is(err, exp_err, 'String for a List type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'String for a List type') local variables_13_2 = {xorder_metainfo = {store = {tags = {1, 2, 3}}}} local exp_err = 'Wrong variable "xorder_metainfo.store.tags[1]" for ' .. 'the Scalar "String"' local result = gql_query_13:execute(variables_13_2) local err = result.errors[1].message - test:is(err, exp_err, 'wrong List value type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'wrong List value type') local variables_13_3 = {xorder_metainfo = {store = {tags = {foo = 'bar'}}}} local exp_err = 'Variable "xorder_metainfo.store.tags" for a List ' .. 'must be an array, got map' local result = gql_query_13:execute(variables_13_3) local err = result.errors[1].message - test:is(err, exp_err, 'map for a List type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'map for a List type') -- }}} -- {{{ InputObject @@ -1352,7 +1378,9 @@ function common_testdata.run_queries(gql_wrapper) '___address" must be a Lua table, got number' local result = gql_query_13:execute(variables_13_4) local err = result.errors[1].message - test:is(err, exp_err, 'Int for an InputObject type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'Int for an InputObject type') local variables_13_5 = {xorder_metainfo = {store = { address = {'foo', 'bar', 'baz'}}}} @@ -1362,7 +1390,9 @@ function common_testdata.run_queries(gql_wrapper) 'store___store___address___address" must be a string, got number' local result = gql_query_13:execute(variables_13_5) local err = result.errors[1].message - test:is(err, exp_err, 'List for an InputObject type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'List for an InputObject type') local variables_13_6 = {xorder_metainfo = {store = { address = { @@ -1376,7 +1406,9 @@ function common_testdata.run_queries(gql_wrapper) 'for the Scalar "String"' local result = gql_query_13:execute(variables_13_6) local err = result.errors[1].message - test:is(err, exp_err, 'wrong type for an InputObject field') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'wrong type for an InputObject field') local variables_13_7 = {xorder_metainfo = {store = { address = { @@ -1393,7 +1425,9 @@ function common_testdata.run_queries(gql_wrapper) '___store___address___address"' local result = gql_query_13:execute(variables_13_7) local err = result.errors[1].message - test:is(err, exp_err, 'extra field for an InputObject type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'extra field for an InputObject type') -- }}} @@ -1407,7 +1441,9 @@ function common_testdata.run_queries(gql_wrapper) 'parametrized_tags___InputMap" must be a Lua table, got number' local result = gql_query_13:execute(variables_13_8) local err = result.errors[1].message - test:is(err, exp_err, 'Int for an InputMap type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'Int for an InputMap type') local variables_13_9 = {xorder_metainfo = {store = { parametrized_tags = {'foo', 'bar', 'baz'}}}} @@ -1417,7 +1453,9 @@ function common_testdata.run_queries(gql_wrapper) 'store___parametrized_tags___InputMap" must be a string, got number' local result = gql_query_13:execute(variables_13_9) local err = result.errors[1].message - test:is(err, exp_err, 'List for an InputMap type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'List for an InputMap type') local variables_13_10 = {xorder_metainfo = {store = { parametrized_tags = {int_tag = 42}}}} @@ -1425,7 +1463,9 @@ function common_testdata.run_queries(gql_wrapper) 'parametrized_tags.int_tag" for the Scalar "String"' local result = gql_query_13:execute(variables_13_10) local err = result.errors[1].message - test:is(err, exp_err, 'wrong type for an InputMap field') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'wrong type for an InputMap field') -- }}} @@ -1436,7 +1476,9 @@ function common_testdata.run_queries(gql_wrapper) local exp_err = 'union value must be a map with one field, got number' local result = gql_query_13:execute(variables_13_11) local err = result.errors[1].message - test:is(err, exp_err, 'non-map value type for an InputUnion type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'non-map value type for an InputUnion type') local variables_13_12 = {xorder_metainfo = {store = { external_id = {int = 42.2}}}} @@ -1444,28 +1486,36 @@ function common_testdata.run_queries(gql_wrapper) 'for the Scalar "Int"' local result = gql_query_13:execute(variables_13_12) local err = result.errors[1].message - test:is(err, exp_err, 'wrong value type for an InputUnion type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'wrong value type for an InputUnion type') local variables_13_13 = {xorder_metainfo = {store = { external_id = {integer = 42}}}} -- integer instead of int local exp_err = 'unexpected union value field: integer' local result = gql_query_13:execute(variables_13_13) local err = result.errors[1].message - test:is(err, exp_err, 'wrong object field name for an InputUnion type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'wrong object field name for an InputUnion type') local variables_13_14 = {xorder_metainfo = {store = { external_id = {a = 1, b = 2, c = 3}}}} local exp_err = 'union value must have only one field' local result = gql_query_13:execute(variables_13_14) local err = result.errors[1].message - test:is(err, exp_err, 'object with several fields for an InputUnion type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'object with several fields for an InputUnion type') local variables_13_15 = {xorder_metainfo = {store = { external_id = {}}}} local exp_err = 'union value must have only one field' local result = gql_query_13:execute(variables_13_15) local err = result.errors[1].message - test:is(err, exp_err, 'object with no fields for an InputUnion type') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'object with no fields for an InputUnion type') -- }}} @@ -1512,13 +1562,15 @@ function common_testdata.run_queries(gql_wrapper) local exp_err = 'Variable "order_metainfo.store.address.zip" expected ' .. 'to be non-null' local result = gql_query_14:execute(variables_14_1) + local code = result.errors[1].extensions.error_code local err = result.errors[1].message - test:is(err, exp_err, 'Lack of non-null field for an InputObject type') + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'Lack of a non-null field for an InputObject type') -- }}} -- }}} - -- {{{ check variable type againts argument type + -- {{{ check a variable type against an argument type local query_15 = [[ mutation($tags: [Int!]) { @@ -1531,11 +1583,13 @@ function common_testdata.run_queries(gql_wrapper) } } ]] - local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_15) - local err_exp = 'Variable "tags" type mismatch: the variable type ' .. + local ok, res = pcall(gql_wrapper.compile, gql_wrapper, query_15) + local err = utils.strip_error(res.message) + local code = res.extensions.error_code + local exp_err = 'Variable "tags" type mismatch: the variable type ' .. '"List(NonNull(Int))" is not compatible with the argument type ' .. '"List(NonNull(String))"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, err, code}, {false, exp_err, e.TYPE_MISMATCH}, 'variable usage inside InputObject') local query_16 = [[ @@ -1548,10 +1602,12 @@ function common_testdata.run_queries(gql_wrapper) } } ]] - local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_16) - local err_exp = 'Variable "tag_value" type mismatch: the variable type ' .. + local ok, res = pcall(gql_wrapper.compile, gql_wrapper, query_16) + local err = utils.strip_error(res.message) + local code = res.extensions.error_code + local exp_err = 'Variable "tag_value" type mismatch: the variable type ' .. '"Int" is not compatible with the argument type "NonNull(String)"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, err, code}, {false, exp_err, e.TYPE_MISMATCH}, 'variable usage inside InputMap') local query_17 = [[ @@ -1564,12 +1620,14 @@ function common_testdata.run_queries(gql_wrapper) } } ]] - local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_17) - local err_exp = 'Variable "map_value" type mismatch: the variable type ' .. + local ok, res = pcall(gql_wrapper.compile, gql_wrapper, query_17) + local err = utils.strip_error(res.message) + local code = res.extensions.error_code + local exp_err = 'Variable "map_value" type mismatch: the variable type ' .. '"Int" is not compatible with the argument type "arguments___' .. 'order_metainfo_collection___update___order_metainfo_collection_' .. 'update___store___store___parametrized_tags___InputMap"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, err, code}, {false, exp_err, e.TYPE_MISMATCH}, 'variable usage as InputMap') local query_18 = [[ @@ -1582,13 +1640,15 @@ function common_testdata.run_queries(gql_wrapper) } } ]] - local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_18) - local err_exp = 'Variable "id_value" type mismatch: the variable type ' .. + local ok, res = pcall(gql_wrapper.compile, gql_wrapper, query_18) + local err = utils.strip_error(res.message) + local code = res.extensions.error_code + local exp_err = 'Variable "id_value" type mismatch: the variable type ' .. '"NonNull(Float)" is not compatible with the argument type ' .. '"arguments___order_metainfo_collection___update___order_metainfo_' .. 'collection_update___store___store___external_id___external_id___' .. 'Int_box"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, err, code}, {false, exp_err, e.TYPE_MISMATCH}, 'variable usage inside InputUnion') local query_19 = [[ @@ -1601,12 +1661,14 @@ function common_testdata.run_queries(gql_wrapper) } } ]] - local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_19) - local err_exp = 'Variable "id_box_value" type mismatch: the variable ' .. + local ok, res = pcall(gql_wrapper.compile, gql_wrapper, query_19) + local err = utils.strip_error(res.message) + local code = res.extensions.error_code + local exp_err = 'Variable "id_box_value" type mismatch: the variable ' .. 'type "Int" is not compatible with the argument type "arguments___' .. 'order_metainfo_collection___update___order_metainfo_collection_' .. 'update___store___store___external_id___external_id"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, err, code}, {false, exp_err, e.TYPE_MISMATCH}, 'variable usage as InputUnion') local box_t = 'arguments___order_metainfo_collection___update___' .. @@ -1638,10 +1700,12 @@ function common_testdata.run_queries(gql_wrapper) } } ]] - local ok, err = pcall(gql_wrapper.compile, gql_wrapper, query_21) - local err_exp = 'Variable "first_name" type mismatch: the variable type ' .. + local ok, res = pcall(gql_wrapper.compile, gql_wrapper, query_21) + local err = utils.strip_error(res.message) + local code = res.extensions.error_code + local exp_err = 'Variable "first_name" type mismatch: the variable type ' .. '"String" is not compatible with the argument type "NonNull(String)"' - test:is_deeply({ok, utils.strip_error(err)}, {false, err_exp}, + test:is_deeply({ok, err, code}, {false, exp_err, e.TYPE_MISMATCH}, 'nullable variable for non-null argument') -- }}} @@ -1664,8 +1728,10 @@ function common_testdata.run_queries(gql_wrapper) end) local result = gql_query_22:execute({}) local err = result.errors[1].message - local err_exp = 'Expected non-null for "NonNull(String)", got null' - test:is(err, err_exp, 'lack of non-null argument') + local code = result.errors[1].extensions.error_code + local exp_err = 'Expected non-null for "NonNull(String)", got null' + test:is_deeply({err, code}, {exp_err, e.WRONG_VALUE}, + 'lack of non-null argument') -- }}} -- }}} From 5510e3064573a5cc57f5096cc971cfab20575d83 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Mon, 2 Jul 2018 18:54:20 +0300 Subject: [PATCH 20/20] Add limit reaching error codes Fixes #134. --- graphql/accessor_general.lua | 29 ++++++++++++++++--------- graphql/error_codes.lua | 35 +++++++++++++++++++++++------- test/common/limit_result.test.lua | 17 ++++++++++----- test/common/query_timeout.test.lua | 8 +++++-- 4 files changed, 64 insertions(+), 25 deletions(-) diff --git a/graphql/accessor_general.lua b/graphql/accessor_general.lua index c3fbb08..aa24dbe 100644 --- a/graphql/accessor_general.lua +++ b/graphql/accessor_general.lua @@ -13,8 +13,10 @@ local bit = require('bit') local rex, is_pcre2 = utils.optional_require_rex() local avro_helpers = require('graphql.avro_helpers') local db_schema_helpers = require('graphql.db_schema_helpers') +local error_codes = require('graphql.error_codes') local check = utils.check +local e = error_codes -- XXX: consider using [1] when it will be mature enough; -- look into [2] for the status. @@ -856,12 +858,17 @@ local function process_tuple(self, state, tuple, opts) local resulting_object_cnt_max = opts.resulting_object_cnt_max local fetched_object_cnt_max = opts.fetched_object_cnt_max qstats.fetched_object_cnt = qstats.fetched_object_cnt + 1 - assert(qstats.fetched_object_cnt <= fetched_object_cnt_max, - ('fetched object count (%d) exceeds fetched_object_cnt_max limit (%d)') - :format(qstats.fetched_object_cnt, fetched_object_cnt_max)) - assert(qcontext.deadline_clock > clock.monotonic64(), - ('query execution timeout exceeded timeout_ms limit (%s ms)'):format( - tostring(self.settings.timeout_ms))) + if qstats.fetched_object_cnt > fetched_object_cnt_max then + error(e.fetched_objects_limit_exceeded( + ('fetched objects count (%d) exceeds fetched_object_cnt_max ' .. + 'limit (%d)'):format(qstats.fetched_object_cnt, + fetched_object_cnt_max))) + end + if clock.monotonic64() > qcontext.deadline_clock then + error(e.timeout_exceeded(( + 'query execution timeout exceeded timeout_ms limit (%s ms)'):format( + tostring(self.settings.timeout_ms)))) + end local collection_name = opts.collection_name local pcre = opts.pcre local resolveField = opts.resolveField @@ -913,10 +920,12 @@ local function process_tuple(self, state, tuple, opts) state.objs[#state.objs + 1] = obj state.count = state.count + 1 qstats.resulting_object_cnt = qstats.resulting_object_cnt + 1 - assert(qstats.resulting_object_cnt <= resulting_object_cnt_max, - ('resulting objects count (%d) exceeds resulting_object_cnt_max ' .. - 'limit (%d)'):format(qstats.resulting_object_cnt, - resulting_object_cnt_max)) + if qstats.resulting_object_cnt > resulting_object_cnt_max then + error(e.resulting_objects_limit_exceeded( + ('resulting objects count (%d) exceeds resulting_object_cnt_max ' .. + 'limit (%d)'):format(qstats.resulting_object_cnt, + resulting_object_cnt_max))) + end if limit ~= nil and state.count >= limit then return false end diff --git a/graphql/error_codes.lua b/graphql/error_codes.lua index a1294c5..1ed13a8 100644 --- a/graphql/error_codes.lua +++ b/graphql/error_codes.lua @@ -2,23 +2,42 @@ local error_codes = {} error_codes.TYPE_MISMATCH = 1 error_codes.WRONG_VALUE = 2 +error_codes.TIMEOUT_EXCEEDED = 3 +error_codes.FETCHED_OBJECTS_LIMIT_EXCEEDED = 4 +error_codes.RESULTING_OBJECTS_LIMIT_EXCEEDED = 5 -function error_codes.type_mismatch(message) +local function message_and_error_code_error(message, error_code) return { message = message, extensions = { - error_code = error_codes.TYPE_MISMATCH, + error_code = error_code, } } end +function error_codes.type_mismatch(message) + local error_code = error_codes.TYPE_MISMATCH + return message_and_error_code_error(message, error_code) +end + function error_codes.wrong_value(message) - return { - message = message, - extensions = { - error_code = error_codes.WRONG_VALUE, - } - } + local error_code = error_codes.WRONG_VALUE + return message_and_error_code_error(message, error_code) +end + +function error_codes.timeout_exceeded(message) + local error_code = error_codes.TIMEOUT_EXCEEDED + return message_and_error_code_error(message, error_code) +end + +function error_codes.fetched_objects_limit_exceeded(message) + local error_code = error_codes.FETCHED_OBJECTS_LIMIT_EXCEEDED + return message_and_error_code_error(message, error_code) +end + +function error_codes.resulting_objects_limit_exceeded(message) + local error_code = error_codes.RESULTING_OBJECTS_LIMIT_EXCEEDED + return message_and_error_code_error(message, error_code) end return error_codes diff --git a/test/common/limit_result.test.lua b/test/common/limit_result.test.lua index df4250e..342c706 100755 --- a/test/common/limit_result.test.lua +++ b/test/common/limit_result.test.lua @@ -9,6 +9,9 @@ package.path = fio.abspath(debug.getinfo(1).source:match("@?(.*/)") local tap = require('tap') local test_utils = require('test.test_utils') local testdata = require('test.testdata.user_order_item_testdata') +local graphql = require('graphql') + +local e = graphql.error_codes local function run_queries(gql_wrapper) local test = tap.test('result cnt') @@ -36,10 +39,12 @@ local function run_queries(gql_wrapper) local result = gql_query:execute(variables) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") + local exp_err = 'resulting objects count (4) exceeds ' .. + 'resulting_object_cnt_max limit (3)' local err = result.errors[1].message - test:is(err, - 'resulting objects count (4) exceeds resulting_object_cnt_max ' .. - 'limit (3)', 'resulting_object_cnt_max test') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.RESULTING_OBJECTS_LIMIT_EXCEEDED}, + 'resulting_object_cnt_max test') variables = { user_id = 5, @@ -48,9 +53,11 @@ local function run_queries(gql_wrapper) local result = gql_query:execute(variables) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") + local exp_err = 'fetched objects count (6) exceeds ' .. + 'fetched_object_cnt_max limit (5)' local err = result.errors[1].message - test:is(err, - 'fetched object count (6) exceeds fetched_object_cnt_max limit (5)', + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.FETCHED_OBJECTS_LIMIT_EXCEEDED}, 'fetched_object_cnt_max test') assert(test:check(), 'check plan') diff --git a/test/common/query_timeout.test.lua b/test/common/query_timeout.test.lua index 6c543ad..c54e334 100755 --- a/test/common/query_timeout.test.lua +++ b/test/common/query_timeout.test.lua @@ -9,6 +9,9 @@ package.path = fio.abspath(debug.getinfo(1).source:match("@?(.*/)") local tap = require('tap') local test_utils = require('test.test_utils') local testdata = require('test.testdata.user_order_item_testdata') +local graphql = require('graphql') + +local e = graphql.error_codes local function run_queries(gql_wrapper) local test = tap.test('result cnt') @@ -34,9 +37,10 @@ local function run_queries(gql_wrapper) local result = gql_query:execute(variables) assert(result.data == nil, "this test should fail") assert(result.errors ~= nil, "this test should fail") + local exp_err = 'query execution timeout exceeded timeout_ms limit (0.001 ms)' local err = result.errors[1].message - test:is(err, 'query execution timeout exceeded timeout_ms limit (0.001 ms)', - 'timeout test') + local code = result.errors[1].extensions.error_code + test:is_deeply({err, code}, {exp_err, e.TIMEOUT_EXCEEDED}, 'timeout test') assert(test:check(), 'check plan') end