Skip to content

Commit

Permalink
perf(utils) use luajit's table.isarray for array detection
Browse files Browse the repository at this point in the history
### Summary

OpenResty's fork of LuaJIT has `table.isarray` library that we can use for
checking whether a table is array or not.

This commit also changes the special case of `Lapis` array to another function
called `utils.is_lapis_array` because its implementation was rather opinionated
and it might cause bugs elsewhere where this is not expected.
  • Loading branch information
bungle committed Oct 6, 2020
1 parent 7c61ef5 commit 312a7a8
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 6 deletions.
2 changes: 1 addition & 1 deletion kong/api/api_helpers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function _M.normalize_nested_params(obj)
is_array = false
if type(v) == "table" then
-- normalize arrays since Lapis parses ?key[1]=foo as {["1"]="foo"} instead of {"foo"}
if utils.is_array(v) then
if utils.is_array(v, "lapis") then
is_array = true
local arr = {}
for _, arr_v in pairs(v) do arr[#arr+1] = arr_v end
Expand Down
4 changes: 2 additions & 2 deletions kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1783,9 +1783,9 @@ function Schema:validate(input, full_check, original_input, rbw_entity)
end


-- Iterate through input fields on update and check agianst schema for
-- Iterate through input fields on update and check against schema for
-- immutable attribute. If immutable attribute is set, compare input values
-- against entity values to detirmine whether input is valid.
-- against entity values to determine whether input is valid.
-- @param input The input table.
-- @param entity The entity update will be performed on.
-- @return True on success.
Expand Down
70 changes: 69 additions & 1 deletion kong/tools/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -514,11 +514,78 @@ function _M.table_contains(arr, val)
return false
end


do
local floor = math.floor
local max = math.max

local ok, is_array_fast = pcall(require, "table.isarray")
if not ok then
is_array_fast = function(t)
for k in pairs(t) do
if type(k) ~= "number" or floor(k) ~= k then
return false
end
end
return true
end
end

local is_array_strict = function(t)
local m, c = 0, 0
for k in pairs(t) do
if type(k) ~= "number" or k < 1 or floor(k) ~= k then
return false
end
m = max(m, k)
c = c + 1
end
return c == m
end

local is_array_lapis = function(t)
if type(t) ~= "table" then
return false
end
local i = 0
for _ in pairs(t) do
i = i + 1
if t[i] == nil and t[tostring(i)] == nil then
return false
end
end
return true
end

--- Checks if a table is an array and not an associative array.
-- @param t The table to check
-- @param mode: `"strict"`: only sequential indices starting from 1 are allowed (no holes)
-- `"fast"`: OpenResty optimized version (holes and negative indices are ok)
-- `"lapis"`: Allows numeric indices as strings (no holes)
-- @return Returns `true` if the table is an array, `false` otherwise
function _M.is_array(t, mode)
if type(t) ~= "table" then
return false
end

if mode == "lapis" then
return is_array_lapis(t)
end

if mode == "fast" then
return is_array_fast(t)
end

return is_array_strict(t)
end
end


--- Checks if a table is an array and not an associative array.
-- *** NOTE *** string-keys containing integers are considered valid array entries!
-- @param t The table to check
-- @return Returns `true` if the table is an array, `false` otherwise
function _M.is_array(t)
function _M.is_lapis_array(t)
if type(t) ~= "table" then
return false
end
Expand All @@ -532,6 +599,7 @@ function _M.is_array(t)
return true
end


--- Deep copies a table into a new table.
-- Tables used as keys are also deep copied, as are metatables
-- @param orig The table to copy
Expand Down
47 changes: 45 additions & 2 deletions spec/01-unit/05-utils_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -389,14 +389,57 @@ describe("Utils", function()
end)

describe("is_array()", function()
it("should know when an array ", function()
it("should know when an array (strict)", function()
assert.True(utils.is_array({ "a", "b", "c", "d" }))
assert.True(utils.is_array({ ["1"] = "a", ["2"] = "b", ["3"] = "c", ["4"] = "d" }))
assert.False(utils.is_array({ "a", "b", nil, "c", "d" }))
assert.False(utils.is_array({ [-1] = "a", [0] = "b", [1] = "c", [2] = "d" }))
assert.False(utils.is_array({ [0] = "a", [1] = "b", [2] = "c", [3] = "d" }))
assert.True(utils.is_array({ [1] = "a", [2] = "b", [3] = "c", [4] = "d" }))
assert.True(utils.is_array({ [1.0] = "a", [2.0] = "b", [3.0] = "c", [4.0] = "d" }))
assert.False(utils.is_array({ [1] = "a", [2] = "b", nil, [3] = "c", [4] = "d" })) --luacheck: ignore
assert.False(utils.is_array({ [1] = "a", [2] = "b", nil, [4] = "c", [5] = "d" })) --luacheck: ignore
assert.False(utils.is_array({ [1.1] = "a", [2.1] = "b", [3.1] = "c", [4.1] = "d" }))
assert.False(utils.is_array({ ["1"] = "a", ["2"] = "b", ["3"] = "c", ["4"] = "d" }))
assert.False(utils.is_array({ "a", "b", "c", foo = "d" }))
assert.False(utils.is_array())
assert.False(utils.is_array(false))
assert.False(utils.is_array(true))
end)

it("should know when an array (fast)", function()
assert.True(utils.is_array({ "a", "b", "c", "d" }, "fast"))
assert.True(utils.is_array({ "a", "b", nil, "c", "d" }, "fast"))
assert.True(utils.is_array({ [-1] = "a", [0] = "b", [1] = "c", [2] = "d" }, "fast"))
assert.True(utils.is_array({ [0] = "a", [1] = "b", [2] = "c", [3] = "d" }, "fast"))
assert.True(utils.is_array({ [1] = "a", [2] = "b", [3] = "c", [4] = "d" }, "fast"))
assert.True(utils.is_array({ [1.0] = "a", [2.0] = "b", [3.0] = "c", [4.0] = "d" }, "fast"))
assert.True(utils.is_array({ [1] = "a", [2] = "b", nil, [3] = "c", [4] = "d" }, "fast")) --luacheck: ignore
assert.True(utils.is_array({ [1] = "a", [2] = "b", nil, [4] = "c", [5] = "d" }, "fast")) --luacheck: ignore
assert.False(utils.is_array({ [1.1] = "a", [2.1] = "b", [3.1] = "c", [4.1] = "d" }, "fast"))
assert.False(utils.is_array({ ["1"] = "a", ["2"] = "b", ["3"] = "c", ["4"] = "d" }, "fast"))
assert.False(utils.is_array({ "a", "b", "c", foo = "d" }, "fast"))
assert.False(utils.is_array(nil, "fast"))
assert.False(utils.is_array(false, "fast"))
assert.False(utils.is_array(true, "fast"))
end)

it("should know when an array (lapis)", function()
assert.True(utils.is_array({ "a", "b", "c", "d" }, "lapis"))
assert.False(utils.is_array({ "a", "b", nil, "c", "d" }, "lapis"))
assert.False(utils.is_array({ [-1] = "a", [0] = "b", [1] = "c", [2] = "d" }, "lapis"))
assert.False(utils.is_array({ [0] = "a", [1] = "b", [2] = "c", [3] = "d" }, "lapis"))
assert.True(utils.is_array({ [1] = "a", [2] = "b", [3] = "c", [4] = "d" }, "lapis"))
assert.True(utils.is_array({ [1.0] = "a", [2.0] = "b", [3.0] = "c", [4.0] = "d" }, "lapis"))
assert.False(utils.is_array({ [1] = "a", [2] = "b", nil, [3] = "c", [4] = "d" }, "lapis")) --luacheck: ignore
assert.False(utils.is_array({ [1] = "a", [2] = "b", nil, [4] = "c", [5] = "d" }, "lapis")) --luacheck: ignore
assert.False(utils.is_array({ [1.1] = "a", [2.1] = "b", [3.1] = "c", [4.1] = "d" }, "lapis"))
assert.True(utils.is_array({ ["1"] = "a", ["2"] = "b", ["3"] = "c", ["4"] = "d" }, "lapis"))
assert.False(utils.is_array({ "a", "b", "c", foo = "d" }, "lapis"))
assert.False(utils.is_array(nil, "lapis"))
assert.False(utils.is_array(false, "lapis"))
assert.False(utils.is_array(true, "lapis"))
end)

end)

describe("add_error()", function()
Expand Down

0 comments on commit 312a7a8

Please sign in to comment.