Skip to content

Commit

Permalink
refactor(db) rename for_* methods to page_for_*
Browse files Browse the repository at this point in the history
Be more explicit about the fact that the for_* methods of the new DAO objects
return a page only.
  • Loading branch information
hishamhm committed Aug 22, 2018
1 parent 7ceee51 commit 0db6b43
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 48 deletions.
7 changes: 4 additions & 3 deletions kong/api/endpoints.lua
Expand Up @@ -119,9 +119,10 @@ local function get_collection_endpoint(schema, foreign_schema, foreign_field_nam
end

local dao = db[schema.name]
local data, _, err_t, offset = dao["for_" .. foreign_field_name](dao, { id = foreign_entity.id },
self.args.size, self.args.offset,
op_nulls)
local method = "page_for_" .. foreign_field_name
local data, _, err_t, offset = dao[method](dao, { id = foreign_entity.id },
self.args.size, self.args.offset,
op_nulls)
if err_t then
return handle_error(err_t)
end
Expand Down
10 changes: 5 additions & 5 deletions kong/db/dao/init.lua
Expand Up @@ -36,9 +36,9 @@ local function generate_foreign_key_methods(schema)

for name, field in schema:each_field() do
if field.type == "foreign" then
local method_name = "for_" .. name

methods[method_name] = function(self, foreign_key, size, offset)
local page_method_name = "page_for_" .. name
methods[page_method_name] = function(self, foreign_key, size, offset)
if type(foreign_key) ~= "table" then
error("foreign_key must be a table", 2)
end
Expand Down Expand Up @@ -70,9 +70,9 @@ local function generate_foreign_key_methods(schema)

local strategy = self.strategy

local rows, err_t, new_offset = strategy[method_name](strategy,
foreign_key,
size, offset)
local rows, err_t, new_offset = strategy[page_method_name](strategy,
foreign_key,
size, offset)
if not rows then
return nil, tostring(err_t), err_t
end
Expand Down
2 changes: 1 addition & 1 deletion kong/db/dao/snis.lua
Expand Up @@ -80,7 +80,7 @@ end
-- Returns the name list for a given certificate
function _SNIs:list_for_certificate(cert_pk)
local name_list = setmetatable({}, cjson.empty_array_mt)
local rows, err, err_t = self:for_certificate(cert_pk)
local rows, err, err_t = self:page_for_certificate(cert_pk)
if err then
return nil, err, err_t
end
Expand Down
10 changes: 5 additions & 5 deletions kong/db/strategies/cassandra/init.lua
Expand Up @@ -361,7 +361,7 @@ function _M.new(connector, schema, errors)
queries = nil,
}

-- foreign keys constraints and for_ selector methods
-- foreign keys constraints and page_for_ selector methods

for field_name, field in schema:each_field() do
if field.type == "foreign" then
Expand All @@ -385,7 +385,7 @@ function _M.new(connector, schema, errors)
local db_columns_args_names = new_tab(#db_columns, 0)

for i = 1, #db_columns do
-- keep args_names for 'for_*' methods
-- keep args_names for 'page_for_*' methods
db_columns_args_names[i] = db_columns[i].col_name .. " = ?"
end

Expand All @@ -402,12 +402,12 @@ function _M.new(connector, schema, errors)
end
end

-- generate for_ method for inverse selection
-- e.g. routes:for_service(service_pk)
-- generate page_for_ method for inverse selection
-- e.g. routes:page_for_service(service_pk)
for field_name, field in schema:each_field() do
if field.type == "foreign" then

local method_name = "for_" .. field_name
local method_name = "page_for_" .. field_name
local db_columns = self.foreign_keys_db_columns[field_name]

local select_foreign_bind_args = {}
Expand Down
16 changes: 8 additions & 8 deletions kong/db/strategies/postgres/init.lua
Expand Up @@ -518,7 +518,7 @@ local function page(self, size, token, foreign_key, foreign_entity_name)

if token then
if foreign_entity_name then
statement_name = concat({ "for", foreign_entity_name, "page_next" }, "_")
statement_name = concat({ "page_for", foreign_entity_name, "next" }, "_")
attributes = {
[foreign_entity_name] = foreign_key,
[LIMIT] = limit,
Expand Down Expand Up @@ -547,7 +547,7 @@ local function page(self, size, token, foreign_key, foreign_entity_name)

else
if foreign_entity_name then
statement_name = concat({ "for", foreign_entity_name, "page_first" }, "_")
statement_name = concat({ "page_for", foreign_entity_name, "first" }, "_")
attributes = {
[foreign_entity_name] = foreign_key,
[LIMIT] = limit,
Expand Down Expand Up @@ -1348,13 +1348,13 @@ function _M.new(connector, schema, errors)
argn = { LIMIT },
argc = 1,
argv = single_args,
make = compile(table_name .. "_page_first" , page_first_statement),
make = compile(table_name .. "_first" , page_first_statement),
},
page_next = {
argn = page_next_names,
argc = page_next_count,
argv = page_next_args,
make = compile(table_name .. "_page_next" , page_next_statement),
make = compile(table_name .. "_next" , page_next_statement),
},
},
}
Expand Down Expand Up @@ -1415,20 +1415,20 @@ function _M.new(connector, schema, errors)
" LIMIT $", argc_next, ";"
}

local statement_name = "for_" .. foreign_entity_name
local statement_name = "page_for_" .. foreign_entity_name

statements[statement_name .. "_page_first"] = {
statements[statement_name .. "_first"] = {
argn = argn_first,
argc = argc_first,
argv = argv_first,
make = compile(concat({ table_name, statement_name, "page_first" }, "_"), page_first_statement)
}

statements[statement_name .. "_page_next"] = {
statements[statement_name .. "_next"] = {
argn = argn_next,
argc = argc_next,
argv = argv_next,
make = compile(concat({ table_name, statement_name, "page_next" }, "_"), page_next_statement)
make = compile(concat({ table_name, statement_name, "_next" }, "_"), page_next_statement)
}

self[statement_name] = make_select_for(foreign_entity_name)
Expand Down
2 changes: 1 addition & 1 deletion kong/runloop/handler.lua
Expand Up @@ -271,7 +271,7 @@ return {
log(DEBUG, "[events] SSL cert updated, invalidating cached certificates")
local certificate = data.entity

local rows, err = db.snis:for_certificate({
local rows, err = db.snis:page_for_certificate({
id = certificate.id
})
if not rows then
Expand Down
50 changes: 25 additions & 25 deletions spec/02-integration/000-new-dao/02-db_core_entities_spec.lua
Expand Up @@ -25,7 +25,7 @@ for _, strategy in helpers.each_strategy() do
db.routes:select(primary_key)
db.routes:update(primary_key, entity)
db.routes:delete(primary_key)
db.routes:for_service(service_id)
db.routes:page_for_service(service_id)
--]]
describe("Routes", function()
Expand Down Expand Up @@ -1421,29 +1421,29 @@ for _, strategy in helpers.each_strategy() do
assert.same(service, service_in_db)
end)

describe("routes:for_service()", function()
describe("routes:page_for_service()", function()
-- no I/O
it("errors out if invalid arguments", function()
assert.has_error(function()
db.routes:for_service(nil)
db.routes:page_for_service(nil)
end, "foreign_key must be a table")

assert.has_error(function()
db.routes:for_service({ id = 123 }, "100")
db.routes:page_for_service({ id = 123 }, "100")
end, "size must be a number")

assert.has_error(function()
db.routes:for_service({ id = 123 }, -100)
db.routes:page_for_service({ id = 123 }, -100)
end, "size must be a positive number")

assert.has_error(function()
db.routes:for_service({ id = 123 }, 100, 12345)
db.routes:page_for_service({ id = 123 }, 100, 12345)
end, "offset must be a string")
end)

-- I/O
it("lists no Routes associated to an inexsistent Service", function()
local rows, err, err_t = db.routes:for_service {
local rows, err, err_t = db.routes:page_for_service {
id = a_blank_uuid,
}
assert.is_nil(err_t)
Expand All @@ -1452,7 +1452,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("returns a table encoding to a JSON Array when empty", function()
local rows, err, err_t = db.routes:for_service {
local rows, err, err_t = db.routes:page_for_service {
id = a_blank_uuid,
}
assert.is_nil(err_t)
Expand All @@ -1478,7 +1478,7 @@ for _, strategy in helpers.each_strategy() do
-- different service
}

local rows, err, err_t = db.routes:for_service {
local rows, err, err_t = db.routes:page_for_service {
id = service.id,
}
assert.is_nil(err_t)
Expand All @@ -1496,7 +1496,7 @@ for _, strategy in helpers.each_strategy() do
methods = { "GET" },
}

local rows, err, err_t = db.routes:for_service {
local rows, err, err_t = db.routes:page_for_service {
id = service.id,
}
assert.is_nil(err_t)
Expand Down Expand Up @@ -1530,7 +1530,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("defaults page_size = 100", function()
local rows, err, err_t = db.routes:for_service {
local rows, err, err_t = db.routes:page_for_service {
id = service.id,
}
assert.is_nil(err_t)
Expand All @@ -1539,7 +1539,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("max page_size = 1000", function()
local rows, err, err_t = db.routes:for_service({
local rows, err, err_t = db.routes:page_for_service({
id = service.id,
}, 1002)
assert.is_nil(err_t)
Expand All @@ -1563,7 +1563,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("fetches all rows in one page", function()
local rows, err, err_t, offset = db.routes:for_service {
local rows, err, err_t, offset = db.routes:page_for_service {
id = service.id,
}
assert.is_nil(err_t)
Expand All @@ -1573,7 +1573,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("fetched rows are returned in a table without hash part", function()
local rows, err, err_t = db.routes:for_service {
local rows, err, err_t = db.routes:page_for_service {
id = service.id,
}
assert.is_nil(err_t)
Expand All @@ -1590,15 +1590,15 @@ for _, strategy in helpers.each_strategy() do
end)

it("fetches rows always in same order", function()
local rows1 = db.routes:for_service { id = service.id }
local rows2 = db.routes:for_service { id = service.id }
local rows1 = db.routes:page_for_service { id = service.id }
local rows2 = db.routes:page_for_service { id = service.id }
assert.is_table(rows1)
assert.is_table(rows2)
assert.same(rows1, rows2)
end)

it("returns offset when page_size < total", function()
local rows, err, err_t, offset = db.routes:for_service({
local rows, err, err_t, offset = db.routes:page_for_service({
id = service.id,
}, 5)
assert.is_nil(err_t)
Expand All @@ -1609,7 +1609,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("fetches subsequent pages with offset", function()
local rows_1, err, err_t, offset = db.routes:for_service({
local rows_1, err, err_t, offset = db.routes:page_for_service({
id = service.id,
}, 5)
assert.is_nil(err_t)
Expand All @@ -1626,7 +1626,7 @@ for _, strategy in helpers.each_strategy() do
page_size = page_size + 1
end

local rows_2, err, err_t, offset = db.routes:for_service({
local rows_2, err, err_t, offset = db.routes:page_for_service({
id = service.id,
}, page_size, offset)

Expand All @@ -1646,22 +1646,22 @@ for _, strategy in helpers.each_strategy() do
end)

it("fetches same page with same offset", function()
local _, err, err_t, offset = db.routes:for_service({
local _, err, err_t, offset = db.routes:page_for_service({
id = service.id,
}, 3)
assert.is_nil(err_t)
assert.is_nil(err)
assert.is_string(offset)

local rows_a, err, err_t = db.routes:for_service({
local rows_a, err, err_t = db.routes:page_for_service({
id = service.id,
}, 3, offset)
assert.is_nil(err_t)
assert.is_nil(err)
assert.is_table(rows_a)
assert.equal(3, #rows_a)

local rows_b, err, err_t = db.routes:for_service({
local rows_b, err, err_t = db.routes:page_for_service({
id = service.id,
}, 3, offset)
assert.is_nil(err_t)
Expand All @@ -1680,7 +1680,7 @@ for _, strategy in helpers.each_strategy() do
repeat
local err, err_t

rows, err, err_t, offset = db.routes:for_service({
rows, err, err_t, offset = db.routes:page_for_service({
id = service.id,
}, 3, offset)
assert.is_nil(err_t)
Expand All @@ -1695,7 +1695,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("fetches first page with invalid offset", function()
local rows, err, err_t = db.routes:for_service({
local rows, err, err_t = db.routes:page_for_service({
id = service.id,
}, 3, "hello")
assert.is_nil(rows)
Expand All @@ -1711,7 +1711,7 @@ for _, strategy in helpers.each_strategy() do
end)
end)
end) -- paginates
end) -- routes:for_service()
end) -- routes:page_for_service()
end) -- Services and Routes association
end) -- kong.db [strategy]
end

0 comments on commit 0db6b43

Please sign in to comment.