Skip to content

Commit

Permalink
refactor(db) replace deprecated cjson.empty_array_mt with cjson.array_mt
Browse files Browse the repository at this point in the history
1. `dao:rows_to_entities` will now properly set `array_mt` on returned
   entities

2. schema's `make_array` will now set `array_mt` even on non-empty
   arrays

3. schema's `make_set` will now set `array_mt`, it still sets `Set_mt`
   for non-empty arrays which the `cjson.encode` does not support, but
   it works as long as the value is not turned to e.g. hash afterwards.
  • Loading branch information
bungle committed Jun 13, 2019
1 parent 326bd9a commit 50d3840
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 20 deletions.
4 changes: 2 additions & 2 deletions kong/db/dao/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,7 @@ end
function DAO:rows_to_entities(rows, options)
local count = #rows
if count == 0 then
return setmetatable(rows, cjson.empty_array_mt)
return setmetatable(rows, cjson.array_mt)
end

local entities = new_tab(count, 0)
Expand All @@ -1032,7 +1032,7 @@ function DAO:rows_to_entities(rows, options)
entities[i] = entity
end

return entities
return setmetatable(entities, cjson.array_mt)
end


Expand Down
2 changes: 1 addition & 1 deletion kong/db/dao/snis.lua
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ end

-- Returns the name list for a given certificate
function _SNIs:list_for_certificate(cert_pk, options)
local name_list = setmetatable({}, cjson.empty_array_mt)
local name_list = setmetatable({}, cjson.array_mt)

for sni, err, err_t in self:each_for_certificate(cert_pk, 1000, options) do
if err then
Expand Down
10 changes: 5 additions & 5 deletions kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1308,22 +1308,22 @@ local Set_mt = {


--- Sets (or replaces) metatable of an array:
-- 1. array is a proper sequence, but empty, `cjson.empty_array_mt`
-- 1. array is a proper sequence, `cjson.array_mt`
-- will be used as a metatable of the returned array.
-- 2. otherwise no modifications are made to input parameter.
-- @param array The table containing an array for which to apply the metatable.
-- @return input table (with metatable, see above)
local function make_array(array)
if is_sequence(array) and #array == 0 then
return setmetatable(array, cjson.empty_array_mt)
if is_sequence(array) then
return setmetatable(array, cjson.array_mt)
end

return array
end


--- Sets (or replaces) metatable of a set and removes duplicates:
-- 1. set is a proper sequence, but empty, `cjson.empty_array_mt`
-- 1. set is a proper sequence, but empty, `cjson.array_mt`
-- will be used as a metatable of the returned set.
-- 2. set a proper sequence, and has values, `Set_mt`
-- will be used as a metatable of the returned set.
Expand All @@ -1338,7 +1338,7 @@ local function make_set(set)
local count = #set

if count == 0 then
return setmetatable(set, cjson.empty_array_mt)
return setmetatable(set, cjson.array_mt)
end

local o = {}
Expand Down
51 changes: 39 additions & 12 deletions spec/01-unit/01-db/01-schema/01-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2347,7 +2347,21 @@ describe("schema", function()
assert.equals(5.5, tbl.fingers)
end)

it("adds cjson.empty_array_mt on empty array and set fields", function()
it("adds cjson.array_mt on non-empty array fields", function()
local Test = Schema.new({
fields = {
{ arr = { type = "array", elements = { type = "string" } } },
},
})

local tbl = Test:process_auto_fields({
arr = { "hello" },
}, "insert")

assert.same(cjson.array_mt, getmetatable(tbl.arr))
end)

it("adds cjson.array_mt on empty array and set fields", function()
local Test = Schema.new({
fields = {
{ arr = { type = "array", elements = { type = "string" } } },
Expand All @@ -2360,11 +2374,11 @@ describe("schema", function()
set = {}
}, "insert")

assert.same(cjson.empty_array_mt, getmetatable(tbl.arr))
assert.same(cjson.empty_array_mt, getmetatable(tbl.set))
assert.same(cjson.array_mt, getmetatable(tbl.arr))
assert.same(cjson.array_mt, getmetatable(tbl.set))
end)

it("adds cjson.empty_array_mt on empty array and set fields", function()
it("adds cjson.array_mt on empty array and set fields", function()
local Test = Schema.new({
fields = {
{ arr = { type = "array", elements = { type = "string" } } },
Expand All @@ -2378,8 +2392,8 @@ describe("schema", function()
set = {}
}, operation)

assert.same(cjson.empty_array_mt, getmetatable(tbl.arr))
assert.same(cjson.empty_array_mt, getmetatable(tbl.set))
assert.same(cjson.array_mt, getmetatable(tbl.arr))
assert.same(cjson.array_mt, getmetatable(tbl.set))
end
end)

Expand All @@ -2404,29 +2418,42 @@ describe("schema", function()
end
end)

it("does not add a helper metatable to arrays or maps", function()
it("does not add a helper metatable to maps", function()
local Test = Schema.new({
fields = {
{ arr = { type = "array", elements = { type = "string" } } },
{ map = { type = "map", keys = { type = "string" }, values = { type = "boolean" } } },
},
})

for _, operation in pairs{ "insert", "update", "select", "delete" } do
local tbl = Test:process_auto_fields({
arr = { "http", "https" },
map = { http = true },
}, operation)

assert.is_nil(getmetatable(tbl.arr))
assert.is_nil(getmetatable(tbl.map))
assert.is_equal("http", tbl.arr[1])
assert.is_nil(tbl.arr.http)
assert.is_true(tbl.map.http)
assert.is_nil(tbl.map.https)
end
end)

it("does add array_mt metatable to arrays", function()
local Test = Schema.new({
fields = {
{ arr = { type = "array", elements = { type = "string" } } },
},
})

for _, operation in pairs{ "insert", "update", "select", "delete" } do
local tbl = Test:process_auto_fields({
arr = { "http", "https" },
}, operation)

assert.is_equal(cjson.array_mt, getmetatable(tbl.arr))
assert.is_equal("http", tbl.arr[1])
assert.is_nil(tbl.arr.http)
end
end)

it("sets 'read_before_write' to true when updating field type record", function()
local Test = Schema.new({
name = "test",
Expand Down
29 changes: 29 additions & 0 deletions spec/02-integration/03-db/02-db_core_entities_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ local utils = require "kong.tools.utils"
local helpers = require "spec.helpers"
local cjson = require "cjson"


local fmt = string.format
local unindent = helpers.unindent

Expand All @@ -21,6 +22,8 @@ for _, strategy in helpers.each_strategy() do
"basicauth_credentials",
"upstreams",
"targets",
"certificates",
"snis",
})
end)

Expand Down Expand Up @@ -72,9 +75,11 @@ for _, strategy in helpers.each_strategy() do
it("returns a table encoding to a JSON Array when empty", function()

local rows, err, err_t, offset = db.routes:page()

assert.is_nil(err_t)
assert.is_nil(err)
assert.is_table(rows)
assert.equal(cjson.array_mt, getmetatable(rows))
assert.equal(0, #rows)
assert.is_nil(offset)
assert.equals("[]", cjson.encode(rows))
Expand All @@ -99,6 +104,7 @@ for _, strategy in helpers.each_strategy() do
assert.is_nil(err_t)
assert.is_nil(err)
assert.is_table(rows)
assert.equal(cjson.array_mt, getmetatable(rows))
assert.equal(10, #rows)
assert.is_nil(offset)
end)
Expand Down Expand Up @@ -254,6 +260,7 @@ for _, strategy in helpers.each_strategy() do
assert.is_nil(err_t)
assert.is_nil(err)
assert.is_table(rows)
assert.equal(cjson.array_mt, getmetatable(rows))
assert.equal(100, #rows)

-- invokes schema post-processing
Expand Down Expand Up @@ -1951,5 +1958,27 @@ for _, strategy in helpers.each_strategy() do
end)
end)
end)


describe("SNIs", function()
local certificate
lazy_setup(function()
certificate = db.certificates.schema:extract_pk_values(bp.certificates:insert())
end)
describe(":list_for_certificate()", function()
it("returns array with cjson.array_mt metatable", function()
local snis = assert(db.snis:list_for_certificate(certificate))
local json = cjson.encode(snis)
assert.equal("[]", json)
assert.equal(cjson.array_mt, getmetatable(snis))

db.snis:update_list(certificate, {"example.org"})
snis = assert(db.snis:list_for_certificate(certificate))
json = cjson.encode(snis)
assert.equal('["example.org"]', json)
assert.equal(cjson.array_mt, getmetatable(snis))
end)
end)
end)
end) -- kong.db [strategy]
end

0 comments on commit 50d3840

Please sign in to comment.