Skip to content

Commit

Permalink
Enforcing required fields on full-updates
Browse files Browse the repository at this point in the history
  • Loading branch information
subnetmarco committed Apr 26, 2016
1 parent eacc686 commit 0fb6de2
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 27 deletions.
4 changes: 2 additions & 2 deletions kong/dao/schemas/apis.lua
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ return {
table = "apis",
primary_key = {"id"},
fields = {
id = {type = "id", dao_insert_value = true},
created_at = {type = "timestamp", immutable = true, dao_insert_value = true},
id = {type = "id", dao_insert_value = true, required = true},
created_at = {type = "timestamp", immutable = true, dao_insert_value = true, required = true},
name = {type = "string", unique = true, default = default_name, func = check_name},
request_host = {type = "string", unique = true, func = check_request_host},
request_path = {type = "string", unique = true, func = check_request_path},
Expand Down
8 changes: 4 additions & 4 deletions kong/dao/schemas/consumers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ return {
table = "consumers",
primary_key = {"id"},
fields = {
id = { type = "id", dao_insert_value = true },
created_at = { type = "timestamp", immutable = true, dao_insert_value = true },
custom_id = { type = "string", unique = true, func = check_custom_id_and_username },
username = { type = "string", unique = true, func = check_custom_id_and_username }
id = {type = "id", dao_insert_value = true, required = true},
created_at = {type = "timestamp", immutable = true, dao_insert_value = true, required = true},
custom_id = {type = "string", unique = true, func = check_custom_id_and_username},
username = {type = "string", unique = true, func = check_custom_id_and_username}
},
marshall_event = function(self, t)
return { id = t.id }
Expand Down
6 changes: 3 additions & 3 deletions kong/dao/schemas/nodes.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ return {
table = "nodes",
primary_key = {"name"},
fields = {
name = { type = "string" },
created_at = { type = "timestamp", dao_insert_value = true },
cluster_listening_address = { type = "string", required = true }
name = {type = "string"},
created_at = {type = "timestamp", dao_insert_value = true,required = true},
cluster_listening_address = {type = "string", required = true}
},
marshall_event = function(self, t)
return { name = t.name }
Expand Down
6 changes: 4 additions & 2 deletions kong/dao/schemas/plugins.lua
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ return {
fields = {
id = {
type = "id",
dao_insert_value = true
dao_insert_value = true,
required = true
},
created_at = {
type = "timestamp",
immutable = true,
dao_insert_value = true
dao_insert_value = true,
required = true
},
api_id = {
type = "id",
Expand Down
13 changes: 9 additions & 4 deletions kong/dao/schemas_validation.lua
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function _M.validate_entity(tbl, schema, options)

-- Check the given table against a given schema
for column, v in pairs(schema.fields) do
-- [TYPE] Check if type is valid. Booleans and Numbers as strings are accepted and converted
-- [TYPE] Check if type is valid. Booleans and Numbers as strings are accepted and converted
if t[column] ~= nil and v.type ~= nil then
local is_valid_type
-- ALIASES: number, timestamp, boolean and array can be passed as strings and will be converted
Expand Down Expand Up @@ -171,7 +171,7 @@ function _M.validate_entity(tbl, schema, options)
for sub_field_k, sub_field in pairs(sub_schema.fields) do
if sub_field.default ~= nil then -- Sub-value has a default, be polite and pre-assign the sub-value
t[column] = {}
elseif sub_field.required then -- Only check required if field doesn't have a default
elseif sub_field.required then -- Only check required if field doesn't have a default and dao_insert_value
errors = utils.add_error(errors, error_prefix..column, column.."."..sub_field_k.." is required")
end
end
Expand All @@ -193,10 +193,15 @@ function _M.validate_entity(tbl, schema, options)
end
end

-- Check that full updates still meet the REQUIRED contraints
if options.full_update and v.required and (t[column] == nil or t[column] == "") then
errors = utils.add_error(errors, error_prefix..column, column.." is required")
end

if not partial_update or t[column] ~= nil then
-- [REQUIRED] Check that required fields are set.
-- Now that default and most other checks have been run.
if v.required and (t[column] == nil or t[column] == "") then
if v.required and not v.dao_insert_value and (t[column] == nil or t[column] == "") then
errors = utils.add_error(errors, error_prefix..column, column.." is required")
end

Expand Down Expand Up @@ -262,4 +267,4 @@ function _M.is_schema_subset(tbl, schema)
return errors == nil, errors
end

return _M
return _M
2 changes: 2 additions & 0 deletions kong/plugins/response-ratelimiting/header_filter.lua
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ local function parse_header(header_value, limits)
end

function _M.execute(conf)
ngx.ctx.increments = {}

if utils.table_size(conf.limits) <= 0 then
return
end
Expand Down
37 changes: 29 additions & 8 deletions spec/integration/04-admin_api/apis_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,32 @@ describe("Admin API", function()
assert.equal(201, status)
end
end)
it_content_types("should update if exists", function(content_type)
it_content_types("#only should not update if some required fields are missing", function(content_type)
return function()
local response, status = http_client.put(BASE_URL, {
id = api.id,
name = "api-PUT-tests-updated",
request_host = "updated-api.mockbin.com",
upstream_url = "http://mockbin.com"
}, {["content-type"] = content_type})
assert.equal(400, status)
local body = json.decode(response)
assert.equal("created_at is required", body.created_at)
end
end)
it_content_types("#only should update if exists", function(content_type)
return function()
local response, status = http_client.put(BASE_URL, {
id = api.id,
name = "api-PUT-tests-updated",
request_host = "updated-api.mockbin.com",
upstream_url = "http://mockbin.com",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)
local body = json.decode(response)
assert.equal("api-PUT-tests-updated", body.name)
assert.truthy(body.created_at)
end
end)
describe("errors", function()
Expand Down Expand Up @@ -412,7 +427,8 @@ describe("Admin API", function()
local response, status = http_client.put(PLUGIN_BASE_URL, {
id = plugin.id,
name = "key-auth",
["config.key_names"] = "updated_apikey"
["config.key_names"] = "updated_apikey",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)
local body = json.decode(response)
Expand All @@ -433,7 +449,8 @@ describe("Admin API", function()

local response, status = http_client.put(PLUGIN_BASE_URL, {
id = plugin.id,
name = "key-auth"
name = "key-auth",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)
local body = json.decode(response)
Expand Down Expand Up @@ -464,7 +481,8 @@ describe("Admin API", function()
id = plugin.id,
api_id = api.id,
name = "rate-limiting",
["config.minute"] = 3
["config.minute"] = 3,
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)
local body = json.decode(response)
Expand All @@ -487,7 +505,8 @@ describe("Admin API", function()
local response, status = http_client.put(PLUGIN_BASE_URL, {
id = plugin.id,
name = "key-auth",
["config.key_names"] = "api_key_updated"
["config.key_names"] = "api_key_updated",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)
local body = json.decode(response)
Expand All @@ -501,7 +520,8 @@ describe("Admin API", function()
id = plugin.id,
name = "key-auth",
enabled = false,
["config.key_names"] = "apikey,key"
["config.key_names"] = "apikey,key",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)

Expand All @@ -513,7 +533,8 @@ describe("Admin API", function()
id = plugin.id,
name = "key-auth",
enabled = true,
["config.key_names"] = "apikey,key"
["config.key_names"] = "apikey,key",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)

Expand Down Expand Up @@ -669,4 +690,4 @@ describe("Admin API", function()
end)
end)
end)
end)
end)
10 changes: 7 additions & 3 deletions spec/integration/04-admin_api/consumers_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,16 @@ describe("Admin API", function()
local response, status = http_client.put(BASE_URL, {
id = consumer_id,
username = "updated",
custom_id = "5678"
custom_id = "5678",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)
local body = json.decode(response)
assert.same({
id = consumer_id,
username = "updated",
custom_id = "5678"
custom_id = "5678",
created_at = 1461276890000
}, body)
end
end)
Expand All @@ -99,12 +101,14 @@ describe("Admin API", function()
local response, status = http_client.put(BASE_URL, {
id = consumer_id,
username = "updated",
created_at = 1461276890000
}, {["content-type"] = content_type})
assert.equal(200, status)
local body = json.decode(response)
assert.same({
id = consumer_id,
username = "updated",
created_at = 1461276890000
}, body)
end
end)
Expand Down Expand Up @@ -280,4 +284,4 @@ describe("Admin API", function()
end)
end)
end)
end)
end)
2 changes: 1 addition & 1 deletion spec/unit/10-schema_validation_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ describe("Schemas", function()
local valid, err = validate_entity(values, schema, {update = true, full_update = true})
assert.False(valid)
assert.truthy(err)
assert.equal("string is required", err.string)
assert.are.same({"string is required", "string is required"}, err.string)
end)
it("should complete default fields", function()
local values = {string = "foo", date = 123456}
Expand Down

0 comments on commit 0fb6de2

Please sign in to comment.