Skip to content

Commit

Permalink
fix(dao) Allow :update to unset fields if necessary
Browse files Browse the repository at this point in the history
By doing a PUT request to the API, one can now omit a field. If that
field is omitted and if the schema still validates, the value will be
set to null in the DB.
  • Loading branch information
thibaultcha committed Jun 22, 2015
1 parent 93655f2 commit 19bb9ef
Show file tree
Hide file tree
Showing 7 changed files with 177 additions and 98 deletions.
4 changes: 1 addition & 3 deletions kong/api/crud_helpers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ function _M.find_api_by_name_or_id(self, dao_factory, helpers)
}
self.params.name_or_id = nil

-- TODO: make the base_dao more flexible so we can query find_by_primary_key with key/values
-- https://github.com/Mashape/kong/issues/103
local data, err = dao_factory.apis:find_by_keys(fetch_keys)
if err then
return helpers.yield_error(err)
Expand Down Expand Up @@ -82,7 +80,7 @@ function _M.put(params, dao_collection)
end

if res then
new_entity, err = dao_collection:update(params)
new_entity, err = dao_collection:update(params, true)
if not err then
return responses.send_HTTP_OK(new_entity)
end
Expand Down
14 changes: 12 additions & 2 deletions kong/dao/cassandra/base_dao.lua
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ end
-- @param `t` A table representing the entity to insert
-- @return `result` Updated entity or nil
-- @return `error` Error if any during the execution
function BaseDao:update(t)
function BaseDao:update(t, full)
assert(t ~= nil, "Cannot update a nil element")
assert(type(t) == "table", "Entity to update must be a table")

Expand All @@ -443,7 +443,7 @@ function BaseDao:update(t)
end

-- Validate schema
errors = validations.validate(t, self, {is_update = true})
errors = validations.validate(t, self, {partial_update = not full, full_update = full})
if errors then
return nil, errors
end
Expand All @@ -464,6 +464,16 @@ function BaseDao:update(t)

-- Extract primary key from the entity
local t_primary_key, t_no_primary_key = extract_primary_key(t, self._primary_key, self._clustering_key)

-- If full, add `null` values to the SET part of the query for nil columns
if full then
for k, v in pairs(self._schema.fields) do
if not t[k] and not v.immutable then
t_no_primary_key[k] = cassandra.null
end
end
end

local update_q, columns = query_builder.update(self._table, t_no_primary_key, t_primary_key)

local _, stmt_err = self:execute(update_q, columns, self:_marshall(t))
Expand Down
34 changes: 17 additions & 17 deletions kong/dao/schemas_validation.lua
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ function _M.validate_fields(t, schema, options)
if not options then options = {} end
local errors

-- Check the given table against a given schema
for column, v in pairs(schema.fields) do
if not options.is_update then
if not options.partial_update and not options.full_update then
for column, v in pairs(schema.fields) do
-- [DEFAULT] Set default value for the field if given
if t[column] == nil and v.default ~= nil then
if type(v.default) == "function" then
Expand All @@ -52,16 +51,18 @@ function _M.validate_fields(t, schema, options)
t[column] = v.default
end
end

-- [INSERT_VALUE]
if v.dao_insert_value and type(options.dao_insert) == "function" then
t[column] = options.dao_insert(v)
end
else
-- [IMMUTABLE] check immutability of a field if updating
if options.is_update and t[column] ~= nil and v.immutable and not v.required then
errors = utils.add_error(errors, column, column.." cannot be updated")
end
end
end

-- Check the given table against a given schema
for column, v in pairs(schema.fields) do
-- [IMMUTABLE] check immutability of a field if updating
if (options.partial_update or options.full_update) and t[column] ~= nil and v.immutable and not v.required then
errors = utils.add_error(errors, column, column.." cannot be updated")
end

-- [TYPE] Check if type is valid. Boolean and Numbers as strings are accepted and converted
Expand Down Expand Up @@ -153,17 +154,16 @@ function _M.validate_fields(t, schema, options)
end
end

if not options.is_update then
-- [REQUIRED] Check that required fields are set. Now that default and most other checks
-- have been run.
if not options.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
errors = utils.add_error(errors, column, column.." is required")
end
end

if (options.is_update and t[column] ~= nil) or not options.is_update then
-- [FUNC] Check field against a custom function only if there is no error on that field already
if v.func and type(v.func) == "function" and (errors == nil or errors[column] == nil) then
if type(v.func) == "function" and (errors == nil or errors[column] == nil) then
-- [FUNC] Check field against a custom function
-- only if there is no error on that field already.
local ok, err, new_fields = v.func(t[column], t)
if not ok and err then
errors = utils.add_error(errors, column, err)
Expand Down Expand Up @@ -200,7 +200,7 @@ function _M.on_insert(t, schema, dao)
end

function _M.validate(t, dao, options)
local ok, errors
local ok, errors

ok, errors = _M.validate_fields(t, dao._schema, options)
if not ok then
Expand Down
16 changes: 8 additions & 8 deletions spec/integration/admin_api/consumers_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ describe("Admin API", function()

it("[SUCCESS] should create a Consumer", function()
send_content_types(BASE_URL, "POST", {
username="consumer POST tests"
}, 201, nil, {drop_db=true})
username = "consumer POST tests"
}, 201, nil, {drop_db = true})
end)

it("[FAILURE] should return proper errors", function()
Expand All @@ -31,7 +31,7 @@ describe("Admin API", function()
'{"custom_id":"At least a \'custom_id\' or a \'username\' must be specified","username":"At least a \'custom_id\' or a \'username\' must be specified"}')

send_content_types(BASE_URL, "POST", {
username="consumer POST tests"
username = "consumer POST tests"
}, 409, '{"username":"username already exists with value \'consumer POST tests\'"}')
end)

Expand All @@ -41,12 +41,12 @@ describe("Admin API", function()

it("[SUCCESS] should create and update", function()
local consumer = send_content_types(BASE_URL, "PUT", {
username="consumer PUT tests"
username = "consumer PUT tests"
}, 201, nil, {drop_db=true})

consumer = send_content_types(BASE_URL, "PUT", {
id=consumer.id,
username="consumer PUT tests updated",
id = consumer.id,
username = "consumer PUT tests updated",
}, 200)
assert.equal("consumer PUT tests updated", consumer.username)
end)
Expand All @@ -57,7 +57,7 @@ describe("Admin API", function()
'{"custom_id":"At least a \'custom_id\' or a \'username\' must be specified","username":"At least a \'custom_id\' or a \'username\' must be specified"}')

send_content_types(BASE_URL, "PUT", {
username="consumer PUT tests updated",
username = "consumer PUT tests updated",
}, 409, '{"username":"username already exists with value \'consumer PUT tests updated\'"}')
end)

Expand Down Expand Up @@ -112,7 +112,7 @@ describe("Admin API", function()
setup(function()
spec_helper.drop_db()
local fixtures = spec_helper.insert_fixtures {
consumer = {{ username="get_consumer_tests" }}
consumer = {{username = "get_consumer_tests"}}
}
consumer = fixtures.consumer[1]
end)
Expand Down
45 changes: 45 additions & 0 deletions spec/unit/dao/cassandra/base_dao_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,51 @@ describe("Cassandra", function()
assert.equal("public_dns already exists with value '"..api_t.public_dns.."'", err.message.public_dns)
end)

describe("full", function()

it("should set to NULL if a field is not specified", function()
local api_t = faker:fake_entity("api")
api_t.path = "/path"

local api, err = dao_factory.apis:insert(api_t)
assert.falsy(err)
assert.truthy(api_t.path)

-- Update
api.path = nil
api, err = dao_factory.apis:update(api, true)
assert.falsy(err)
assert.truthy(api)
assert.falsy(api.path)

-- Check update
api, err = session:execute("SELECT * FROM apis WHERE id = ?", {cassandra.uuid(api.id)})
assert.falsy(err)
assert.falsy(api.path)
end)

it("should still check the validity of the schema", function()
local api_t = faker:fake_entity("api")

local api, err = dao_factory.apis:insert(api_t)
assert.falsy(err)
assert.truthy(api_t)

-- Update
api.public_dns = nil

local nil_api, err = dao_factory.apis:update(api, true)
assert.truthy(err)
assert.falsy(nil_api)

-- Check update failed
api, err = session:execute("SELECT * FROM apis WHERE id = ?", {cassandra.uuid(api.id)})
assert.falsy(err)
assert.truthy(api[1].name)
assert.truthy(api[1].public_dns)
end)

end)
end) -- describe :update()

describe(":find_by_keys()", function()
Expand Down
16 changes: 16 additions & 0 deletions spec/unit/dao/entities_schemas_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,22 @@ local validate_fields = require("kong.dao.schemas_validation").validate_fields
require "kong.tools.ngx_stub"

describe("Entities Schemas", function()

for k, schema in pairs({api = api_schema,
consumer = consumer_schema,
plugins_configurations = plugins_configurations_schema}) do
it(k.." schema should have some required properties", function()
assert.truthy(schema.name)
assert.equal("string", type(schema.name))

assert.truthy(schema.primary_key)
assert.equal("table", type(schema.primary_key))

assert.truthy(schema.fields)
assert.equal("table", type(schema.fields))
end)
end

describe("APIs", function()

it("should return error with wrong target_url", function()
Expand Down
Loading

0 comments on commit 19bb9ef

Please sign in to comment.