refactor(schema) on_insert becomes self_check

kong/dao/cassandra/base_dao.lua

@@ -351,10 +351,11 @@ function BaseDao:insert(t)
   assert(t ~= nil, "Cannot insert a nil element")
   assert(type(t) == "table", "Entity to insert must be a table")
 
-  local ok, db_err, errors
+  local ok, db_err, errors, self_err
 
   -- Populate the entity with any default/overriden values and validate it
-  errors = validations.validate(t, self, {
+  ok, errors, self_err = validations.validate_entity(t, self._schema, {
+    dao = self._factory,
     dao_insert = function(field)
       if field.type == "id" then
         return uuid()
@@ -363,13 +364,10 @@ function BaseDao:insert(t)
       end
     end
   })
-  if errors then
-    return nil, errors
-  end
-
-  ok, errors = validations.on_insert(t, self._schema, self._factory)
-  if not ok then
-    return nil, errors
+  if self_err then
+    return nil, self_err
+  elseif not ok then
+    return nil, DaoError(errors, error_types.SCHEMA)
   end
 
   ok, errors, db_err = self:check_unique_fields(t)
@@ -440,7 +438,7 @@ function BaseDao:update(t, full)
   assert(t ~= nil, "Cannot update a nil element")
   assert(type(t) == "table", "Entity to update must be a table")
 
-  local ok, db_err, errors
+  local ok, db_err, errors, self_err
 
   -- Check if exists to prevent upsert
   local res, err = self:find_by_primary_key(t)
@@ -455,9 +453,15 @@ function BaseDao:update(t, full)
   end
 
   -- Validate schema
-  errors = validations.validate(t, self, {partial_update = not full, full_update = full})
-  if errors then
-    return nil, errors
+  ok, errors, self_err = validations.validate_entity(t, self._schema, {
+    partial_update = not full,
+    full_update = full,
+    dao = self._factory
+  })
+  if self_err then
+    return nil, self_err
+  elseif not ok then
+    return nil, DaoError(errors, error_types.SCHEMA)
   end
 
   ok, errors, db_err = self:check_unique_fields(t, true)

kong/dao/schemas/plugins_configurations.lua

@@ -26,32 +26,32 @@ return {
     value = { type = "table", schema = load_value_schema },
     enabled = { type = "boolean", default = true }
   },
-  on_insert = function(plugin_t, dao, schema)
+  self_check = function(self, plugin_t, dao, is_update)
     -- Load the value schema
-    local value_schema, err = schema.fields.value.schema(plugin_t)
+    local value_schema, err = self.fields.value.schema(plugin_t)
     if err then
-      return false, err
+      return false, DaoError(err, constants.DATABASE_ERROR_TYPES.SCHEMA)
     end
 
     -- Check if the schema has a `no_consumer` field
     if value_schema.no_consumer and plugin_t.consumer_id ~= nil and plugin_t.consumer_id ~= constants.DATABASE_NULL_ID then
       return false, DaoError("No consumer can be configured for that plugin", constants.DATABASE_ERROR_TYPES.SCHEMA)
     end
 
-    local res, err = dao.plugins_configurations:find_by_keys({
-      name = plugin_t.name,
-      api_id = plugin_t.api_id,
-      consumer_id = plugin_t.consumer_id
-    })
+    if not is_update then
+      local res, err = dao.plugins_configurations:find_by_keys({
+        name = plugin_t.name,
+        api_id = plugin_t.api_id,
+        consumer_id = plugin_t.consumer_id
+      })
 
-    if err then
-      return nil, DaoError(err, constants.DATABASE_ERROR_TYPES.DATABASE)
-    end
+      if err then
+        return nil, DaoError(err, constants.DATABASE_ERROR_TYPES.DATABASE)
+      end
 
-    if res and #res > 0 then
-      return false, DaoError("Plugin configuration already exists", constants.DATABASE_ERROR_TYPES.UNIQUE)
-    else
-      return true
+      if res and #res > 0 then
+        return false, DaoError("Plugin configuration already exists", constants.DATABASE_ERROR_TYPES.UNIQUE)
+      end
     end
   end
 }

kong/dao/schemas_validation.lua

@@ -44,7 +44,7 @@ local _M = {}
 --           `is_update`  For an entity update, check immutable fields. Set to true.
 -- @return `valid`     Success of validation. True or false.
 -- @return `errors`    A list of encountered errors during the validation.
-function _M.validate_fields(t, schema, options)
+function _M.validate_entity(t, schema, options)
   if not options then options = {} end
   local errors
 
@@ -151,7 +151,7 @@ function _M.validate_fields(t, schema, options)
 
         if t[column] and type(t[column]) == "table" then
           -- Actually validating the sub-schema
-          local s_ok, s_errors = _M.validate_fields(t[column], sub_schema, options)
+          local s_ok, s_errors = _M.validate_entity(t[column], sub_schema, options)
           if not s_ok then
             for s_k, s_v in pairs(s_errors) do
               errors = utils.add_error(errors, column.."."..s_k, s_v)
@@ -190,29 +190,14 @@ function _M.validate_fields(t, schema, options)
     end
   end
 
-  return errors == nil, errors
-end
-
-function _M.on_insert(t, schema, dao)
-  if schema.on_insert and type(schema.on_insert) == "function" then
-    local valid, err = schema.on_insert(t, dao, schema)
-    if not valid or err then
-      return false, err
-    else
-      return true
+  if errors == nil and type(schema.self_check) == "function" then
+    local ok, err = schema.self_check(schema, t, options.dao, (options.partial_update or options.full_update))
+    if ok == false then
+      return false, nil, err
     end
-  else
-    return true
   end
-end
 
-function _M.validate(t, dao, options)
-  local ok, errors
-
-  ok, errors = _M.validate_fields(t, dao._schema, options)
-  if not ok then
-    return DaoError(errors, error_types.SCHEMA)
-  end
+  return errors == nil, errors
 end
 
 local digit = "[0-9a-f]"

spec/unit/dao/entities_schemas_spec.lua

@@ -2,7 +2,7 @@ local api_schema = require "kong.dao.schemas.apis"
 local consumer_schema = require "kong.dao.schemas.consumers"
 local plugins_configurations_schema = require "kong.dao.schemas.plugins_configurations"
 local validations = require "kong.dao.schemas_validation"
-local validate_fields = validations.validate_fields
+local validate_entity = validations.validate_entity
 
 require "kong.tools.ngx_stub"
 
@@ -26,7 +26,7 @@ describe("Entities Schemas", function()
   describe("APIs", function()
 
     it("should return error with wrong target_url", function()
-      local valid, errors = validate_fields({
+      local valid, errors = validate_entity({
         public_dns = "mockbin.com",
         target_url = "asdasd"
       }, api_schema)
@@ -35,7 +35,7 @@ describe("Entities Schemas", function()
     end)
 
     it("should return error with wrong target_url protocol", function()
-      local valid, errors = validate_fields({
+      local valid, errors = validate_entity({
         public_dns = "mockbin.com",
         target_url = "wot://mockbin.com/"
       }, api_schema)
@@ -44,7 +44,7 @@ describe("Entities Schemas", function()
     end)
 
     it("should validate without a path", function()
-      local valid, errors = validate_fields({
+      local valid, errors = validate_entity({
         public_dns = "mockbin.com",
         target_url = "http://mockbin.com"
       }, api_schema)
@@ -53,7 +53,7 @@ describe("Entities Schemas", function()
     end)
 
     it("should validate with upper case protocol", function()
-      local valid, errors = validate_fields({
+      local valid, errors = validate_entity({
         public_dns = "mockbin.com",
         target_url = "HTTP://mockbin.com/world"
       }, api_schema)
@@ -62,14 +62,14 @@ describe("Entities Schemas", function()
     end)
 
     it("should complain if missing `public_dns` and `path`", function()
-      local valid, errors = validate_fields({
+      local valid, errors = validate_entity({
         name = "mockbin"
       }, api_schema)
       assert.False(valid)
       assert.equal("At least a 'public_dns' or a 'path' must be specified", errors.path)
       assert.equal("At least a 'public_dns' or a 'path' must be specified", errors.public_dns)
 
-      local valid, errors = validate_fields({
+      local valid, errors = validate_entity({
         name = "mockbin",
         path = true
       }, api_schema)
@@ -81,29 +81,29 @@ describe("Entities Schemas", function()
     it("should set the name from public_dns if not set", function()
       local t = { public_dns = "mockbin.com", target_url = "http://mockbin.com" }
 
-      local valid, errors = validate_fields(t, api_schema)
+      local valid, errors = validate_entity(t, api_schema)
       assert.falsy(errors)
       assert.True(valid)
       assert.equal("mockbin.com", t.name)
     end)
 
     it("should only accept alphanumeric `path`", function()
-      local valid, errors = validate_fields({
+      local valid, errors = validate_entity({
         name = "mockbin",
         path = "/[a-zA-Z]{3}",
         target_url = "http://mockbin.com"
       }, api_schema)
       assert.equal("path must only contain alphanumeric and '. -, _, ~, /' characters", errors.path)
       assert.False(valid)
 
-      valid = validate_fields({
+      valid = validate_entity({
         name = "mockbin",
         path = "/status/",
         target_url = "http://mockbin.com"
       }, api_schema)
       assert.True(valid)
 
-      valid = validate_fields({
+      valid = validate_entity({
         name = "mockbin",
         path = "/abcd~user-2",
         target_url = "http://mockbin.com"
@@ -113,42 +113,42 @@ describe("Entities Schemas", function()
 
     it("should prefix a `path` with a slash and remove trailing slash", function()
       local api_t = { name = "mockbin", path = "status", target_url = "http://mockbin.com" }
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/status", api_t.path)
 
       api_t.path = "/status"
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/status", api_t.path)
 
       api_t.path = "status/"
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/status", api_t.path)
 
       api_t.path = "/status/"
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/status", api_t.path)
 
       api_t.path = "/deep/nested/status/"
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/deep/nested/status", api_t.path)
 
       api_t.path = "deep/nested/status"
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/deep/nested/status", api_t.path)
 
       -- Strip all leading slashes
       api_t.path = "//deep/nested/status"
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/deep/nested/status", api_t.path)
 
       -- Strip all trailing slashes
       api_t.path = "/deep/nested/status//"
-      validate_fields(api_t, api_schema)
+      validate_entity(api_t, api_schema)
       assert.equal("/deep/nested/status", api_t.path)
 
       -- Error if invalid path
       api_t.path = "/deep//nested/status"
-      local _, errors = validate_fields(api_t, api_schema)
+      local _, errors = validate_entity(api_t, api_schema)
       assert.equal("path is invalid: /deep//nested/status", errors.path)
     end)
 
@@ -157,17 +157,17 @@ describe("Entities Schemas", function()
   describe("Consumers", function()
 
     it("should require a `custom_id` or `username`", function()
-      local valid, errors = validate_fields({}, consumer_schema)
+      local valid, errors = validate_entity({}, consumer_schema)
       assert.False(valid)
       assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.username)
       assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.custom_id)
 
-      valid, errors = validate_fields({ username = "" }, consumer_schema)
+      valid, errors = validate_entity({ username = "" }, consumer_schema)
       assert.False(valid)
       assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.username)
       assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.custom_id)
 
-      valid, errors = validate_fields({ username = true }, consumer_schema)
+      valid, errors = validate_entity({ username = true }, consumer_schema)
       assert.False(valid)
       assert.equal("username is not a string", errors.username)
       assert.equal("At least a 'custom_id' or a 'username' must be specified", errors.custom_id)
@@ -177,22 +177,30 @@ describe("Entities Schemas", function()
 
   describe("Plugins Configurations", function()
 
+    local dao_stub = {
+      plugins_configurations = {
+        find_by_keys = function()
+          return nil
+        end
+      }
+    }
+
     it("should not validate if the plugin doesn't exist (not installed)", function()
-      local valid, errors = validate_fields({name = "world domination"}, plugins_configurations_schema)
+      local valid, errors = validate_entity({name = "world domination"}, plugins_configurations_schema)
       assert.False(valid)
       assert.equal("Plugin \"world domination\" not found", errors.value)
     end)
 
     it("should validate a plugin configuration's `value` field", function()
       -- Success
       local plugin = {name = "keyauth", api_id = "stub", value = {key_names = {"x-kong-key"}}}
-      local valid = validate_fields(plugin, plugins_configurations_schema)
+      local valid = validate_entity(plugin, plugins_configurations_schema, {dao = dao_stub})
       assert.True(valid)
 
       -- Failure
       plugin = {name = "ratelimiting", api_id = "stub", value = {period = "hello"}}
 
-      local valid, errors = validate_fields(plugin, plugins_configurations_schema)
+      local valid, errors = validate_entity(plugin, plugins_configurations_schema, {dao = dao_stub})
       assert.False(valid)
       assert.equal("limit is required", errors["value.limit"])
       assert.equal("\"hello\" is not allowed. Allowed values are: \"second\", \"minute\", \"hour\", \"day\", \"month\", \"year\"", errors["value.period"])
@@ -211,19 +219,11 @@ describe("Entities Schemas", function()
           return stub_value_schema
         end
 
-        local valid, err = validations.on_insert({name = "stub", api_id = "0000", consumer_id = "0000", value = {string = "foo"}}, plugins_configurations_schema)
+        local valid, errors, err = validate_entity({name = "stub", api_id = "0000", consumer_id = "0000", value = {string = "foo"}}, plugins_configurations_schema)
         assert.False(valid)
         assert.equal("No consumer can be configured for that plugin", err.message)
 
-        local dao_stub = {
-          plugins_configurations = {
-            find_by_keys = function()
-              return nil
-            end
-          }
-        }
-
-        valid, err = validations.on_insert({name = "stub", api_id = "0000", value = {string = "foo"}}, plugins_configurations_schema, dao_stub)
+        valid, err = validate_entity({name = "stub", api_id = "0000", value = {string = "foo"}}, plugins_configurations_schema, {dao = dao_stub})
         assert.True(valid)
         assert.falsy(err)
       end)