Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Commit c0687f3

Browse files
committed
Fix GraphiQL warning on the pcre argument
The problem was in incorrect schema: sub-arguments of the pcre argument for non-null strings was generated as non-null that triggers a warning in GraphiQL in case when the sub-argument is omitted. Graphql-lua (graphql/core/validate.lua) does not check for this kind of inconsistency between a schema and a query, so lack of the sub-argument works as expected in test/common/pcre.test.lua. The added test with immediate sub-arguments of the pcre argument does not catch the bug, but I decided to leave it in the commit. Fixes #156.
1 parent 9d342b9 commit c0687f3

File tree

4 files changed

+112
-51
lines changed

4 files changed

+112
-51
lines changed

graphql/accessor_general.lua

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ if rex == nil then
1515
-- fallback to libpcre
1616
rex, is_pcre2 = utils.optional_require('rex_pcre'), false
1717
end
18+
local avro_helpers = require('graphql.avro_helpers')
1819

1920
-- XXX: consider using [1] when it will be mature enough;
2021
-- look into [2] for the status.
@@ -1215,7 +1216,10 @@ local function get_pcre_argument_type(self, collection_name)
12151216

12161217
for _, field in ipairs(schema.fields) do
12171218
if field.type == 'string' or field.type == 'string*' then
1218-
string_fields[#string_fields + 1] = table.copy(field)
1219+
local field = table.copy(field)
1220+
field.type = avro_helpers.make_avro_type_nullable(
1221+
field.type, {raise_on_nullable = false})
1222+
table.insert(string_fields, field)
12191223
end
12201224
end
12211225

graphql/avro_helpers.lua

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
--- The module us collection of helpers to simplify avro-schema related tasks.
2+
3+
local json = require('json')
4+
5+
local avro_helpers = {}
6+
7+
--- The function converts avro type to the corresponding nullable type in
8+
--- place and returns the result.
9+
---
10+
--- We make changes in place in case of table input (`avro`) because of
11+
--- performance reasons, but we returns the result because an input (`avro`)
12+
--- can be a string. Strings in Lua are immutable.
13+
---
14+
--- In the current tarantool/avro-schema implementation we simply add '*' to
15+
--- the end of a type name or, in case of union, add 'null' branch.
16+
---
17+
--- If the type is already nullable the function leaves it as is if
18+
--- `opts.raise_on_nullable` is false or omitted. If `opts.raise_on_nullable`
19+
--- is true the function will raise an error.
20+
---
21+
--- @tparam table avro avro schema node to be converted to nullable one
22+
---
23+
--- @tparam[opt] table opts the following options:
24+
---
25+
--- * `raise_on_nullable` (boolean) raise an error on nullable type
26+
---
27+
--- @result `result` (string or table) nullable avro type
28+
function avro_helpers.make_avro_type_nullable(avro, opts)
29+
assert(avro ~= nil, "avro must not be nil")
30+
local opts = opts or {}
31+
assert(type(opts) == 'table',
32+
'opts must be nil or a table, got ' .. type(opts))
33+
local raise_on_nullable = opts.raise_on_nullable or false
34+
assert(type(raise_on_nullable) == 'boolean',
35+
'opts.raise_on_nullable must be nil or a boolean, got ' ..
36+
type(raise_on_nullable))
37+
38+
local value_type = type(avro)
39+
40+
if value_type == "string" then
41+
local is_nullable = avro:endswith("*")
42+
if raise_on_nullable and is_nullable then
43+
error('expected non-null type, got the nullable one: ' ..
44+
json.encode(avro))
45+
end
46+
return is_nullable and avro or (avro .. '*')
47+
elseif value_type == 'table' and #avro > 0 then -- union
48+
local is_nullable = false
49+
for _, branch in ipairs(avro) do
50+
if branch == 'null' then
51+
is_nullable = true
52+
break
53+
end
54+
end
55+
if raise_on_nullable and is_nullable then
56+
error('expected non-null type, got the nullable one: ' ..
57+
json.encode(avro))
58+
end
59+
-- We add 'nil' branch to the end because here we don't know whether
60+
-- the following things matter for a caller:
61+
-- * a default value (it must have the type of the first branch);
62+
-- * {,un,x}flatten data layout.
63+
if not is_nullable then
64+
table.insert(avro, 'null')
65+
end
66+
return avro
67+
elseif value_type == 'table' and #avro == 0 then
68+
return avro_helpers.make_avro_type_nullable(avro.type, opts)
69+
end
70+
71+
error("avro should be a string or a table, got " .. value_type)
72+
end
73+
74+
return avro_helpers

graphql/query_to_avro.lua

Lines changed: 5 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
--- * The best way to use this module is to just call `avro_schema` method on
66
--- compiled query object.
77

8-
local json = require('json')
98
local path = "graphql.core"
109
local introspection = require(path .. '.introspection')
1110
local query_util = require(path .. '.query_util')
11+
local avro_helpers = require('graphql.avro_helpers')
1212

1313
-- module functions
1414
local query_to_avro = {}
@@ -25,6 +25,7 @@ local gql_scalar_to_avro_index = {
2525
Float = "double",
2626
Boolean = "boolean"
2727
}
28+
2829
local function gql_scalar_to_avro(fieldType)
2930
assert(fieldType.__type == "Scalar", "GraphQL scalar field expected")
3031
assert(fieldType.name ~= "Map", "Map type is not supported")
@@ -33,53 +34,6 @@ local function gql_scalar_to_avro(fieldType)
3334
return result
3435
end
3536

36-
--- The function converts avro type to the corresponding nullable type in
37-
--- place and returns the result.
38-
---
39-
--- We make changes in place in case of table input (`avro`) because of
40-
--- performance reasons, but we returns the result because an input (`avro`)
41-
--- can be a string. Strings in Lua are immutable.
42-
---
43-
--- In the current tarantool/avro-schema implementation we simply add '*' to
44-
--- the end of a type name.
45-
---
46-
--- If the type is already nullable the function leaves it as if
47-
--- `opts.raise_on_nullable` is false or omitted. If `opts.raise_on_nullable`
48-
--- is true the function will raise an error.
49-
---
50-
--- @tparam table avro avro schema node to be converted to nullable one
51-
---
52-
--- @tparam[opt] table opts the following options:
53-
---
54-
--- * `raise_on_nullable` (boolean) raise an error on nullable type
55-
---
56-
--- @result `result` (string or table) nullable avro type
57-
local function make_avro_type_nullable(avro, opts)
58-
assert(avro ~= nil, "avro must not be nil")
59-
local opts = opts or {}
60-
assert(type(opts) == 'table',
61-
'opts must be nil or a table, got ' .. type(opts))
62-
local raise_on_nullable = opts.raise_on_nullable or false
63-
assert(type(raise_on_nullable) == 'boolean',
64-
'opts.raise_on_nullable must be nil or a boolean, got ' ..
65-
type(raise_on_nullable))
66-
67-
local value_type = type(avro)
68-
69-
if value_type == "string" then
70-
local is_nullable = avro:endswith("*")
71-
if raise_on_nullable and is_nullable then
72-
error('expected non-null type, got the nullable one: ' ..
73-
json.encode(avro))
74-
end
75-
return is_nullable and avro or (avro .. '*')
76-
elseif value_type == "table" then
77-
return make_avro_type_nullable(avro.type, opts)
78-
end
79-
80-
error("avro should be a string or a table, got " .. value_type)
81-
end
82-
8337
--- Convert GraphQL type to avro-schema with selecting fields.
8438
---
8539
--- @tparam table fieldType GraphQL type
@@ -123,7 +77,9 @@ local function gql_type_to_avro(fieldType, subSelections, context)
12377
end
12478

12579
if not isNonNull then
126-
result = make_avro_type_nullable(result, {raise_on_nullable = true})
80+
result = avro_helpers.make_avro_type_nullable(result, {
81+
raise_on_nullable = true,
82+
})
12783
end
12884
return result
12985
end

test/common/pcre.test.lua

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ local testdata = require('test.testdata.common_testdata')
1414

1515
local function run_queries(gql_wrapper)
1616
local test = tap.test('pcre')
17-
test:plan(3)
17+
test:plan(4)
1818

1919
local query_1 = [[
2020
query users($offset: String, $first_name_re: String,
@@ -98,6 +98,33 @@ local function run_queries(gql_wrapper)
9898

9999
-- }}}
100100

101+
-- {{{ regexp match with immediate arguments
102+
103+
local query_1i = [[
104+
query users {
105+
user_collection(pcre: {
106+
first_name: "(?i)^i",
107+
middle_name: "ich$",
108+
}) {
109+
first_name
110+
middle_name
111+
last_name
112+
}
113+
}
114+
]]
115+
116+
local gql_query_1i = utils.show_trace(function()
117+
return gql_wrapper:compile(query_1i)
118+
end)
119+
120+
local result_1i_1 = utils.show_trace(function()
121+
return gql_query_1i:execute({})
122+
end)
123+
124+
test:is_deeply(result_1i_1, exp_result_1_1, '1i_1')
125+
126+
-- }}}
127+
101128
assert(test:check(), 'check plan')
102129
end
103130

0 commit comments

Comments
 (0)