Distinguish schema from Cassandra errors in DAO #1

spec/unit/dao/cassandra_spec.lua

@@ -68,12 +68,20 @@ describe("Cassandra DAO", function()
         -- Nil
         local api, err = dao_factory.apis:insert()
         assert.falsy(api)
-        assert.are.same("Cannot insert a nil element", err)
+        assert.truthy(err)
+        assert.True(err.is_schema)
+        assert.are.same("Cannot insert a nil element", err.error)
+
+        -- Invalid schema UNIQUE error (already existing API name)
+        local api_rows, err = dao_factory._db:execute("SELECT * FROM apis LIMIT 1;")
+        assert.falsy(err)
+        local api_t = dao_factory.faker:fake_entity("api")
+        api_t.name = api_rows[1].name
 
-        -- Invalid type
-        local api_t = dao_factory.faker:fake_entity("api", true)
         local api, err = dao_factory.apis:insert(api_t)
         assert.truthy(err)
+        assert.True(err.is_schema)
+        assert.are.same("name already exists with value "..api_t.name, err.error.name)
         assert.falsy(api)
 
         -- Duplicated name
@@ -86,7 +94,8 @@ describe("Cassandra DAO", function()
         local api, err = dao_factory.apis:insert(api_t)
         assert.falsy(api)
         assert.truthy(err)
-        assert.are.same("name already exists with value "..api_t.name, err.name)
+        assert.True(err.is_schema)
+        assert.are.same("name already exists with value "..api_t.name, err.error.name)
       end)
 
     end)
@@ -112,7 +121,8 @@ describe("Cassandra DAO", function()
         local app, err = dao_factory.applications:insert(app_t)
         assert.falsy(app)
         assert.truthy(err)
-        assert.are.same("account_id is required", err.account_id)
+        assert.True(err.is_schema)
+        assert.are.same("account_id is required", err.error.account_id)
 
         -- With an invalid account_id, it's an EXISTS error
         local app_t = dao_factory.faker:fake_entity("application")
@@ -121,7 +131,8 @@ describe("Cassandra DAO", function()
         local app, err = dao_factory.applications:insert(app_t)
         assert.falsy(app)
         assert.truthy(err)
-        assert.are.same("account_id "..app_t.account_id.." does not exist", err.account_id)
+        assert.True(err.is_schema)
+        assert.are.same("account_id "..app_t.account_id.." does not exist", err.error.account_id)
       end)
 
       it("should insert in DB and add generated values", function()
@@ -158,7 +169,8 @@ describe("Cassandra DAO", function()
         local plugin, err = dao_factory.plugins:insert(plugin_t)
         assert.falsy(plugin)
         assert.truthy(err)
-        assert.are.same("api_id "..plugin_t.api_id.." does not exist", err.api_id)
+        assert.True(err.is_schema)
+        assert.are.same("api_id "..plugin_t.api_id.." does not exist", err.error.api_id)
 
         -- With invalid api_id and application_id, it's an EXISTS error
         local plugin_t = dao_factory.faker:fake_entity("plugin")
@@ -168,8 +180,9 @@ describe("Cassandra DAO", function()
         local plugin, err = dao_factory.plugins:insert(plugin_t)
         assert.falsy(plugin)
         assert.truthy(err)
-        assert.are.same("api_id "..plugin_t.api_id.." does not exist", err.api_id)
-        assert.are.same("application_id "..plugin_t.application_id.." does not exist", err.application_id)
+        assert.True(err.is_schema)
+        assert.are.same("api_id "..plugin_t.api_id.." does not exist", err.error.api_id)
+        assert.are.same("application_id "..plugin_t.application_id.." does not exist", err.error.application_id)
       end)
 
       it("should insert a plugin in DB and add generated values", function()
@@ -232,7 +245,8 @@ describe("Cassandra DAO", function()
         local entity, err = dao_factory[collection]:update(t)
         assert.falsy(entity)
         assert.truthy(err)
-        assert.are.same("Entity to update not found", err)
+        assert.True(err.is_schema)
+        assert.are.same("Entity to update not found", err.error)
       end)
 
     end)
@@ -282,7 +296,8 @@ describe("Cassandra DAO", function()
         local api, err = dao_factory.apis:update(api_t)
         assert.falsy(api)
         assert.truthy(err)
-        assert.are.same("public_dns already exists with value "..api_t.public_dns, err.public_dns)
+        assert.True(err.is_schema)
+        assert.are.same("public_dns already exists with value "..api_t.public_dns, err.error.public_dns)
       end)
 
     end)
@@ -465,10 +480,11 @@ describe("Cassandra DAO", function()
 
         local results, err = dao_factory[collection]:find_by_keys(t)
         assert.truthy(err)
+        assert.True(err.is_schema)
         assert.falsy(results)
 
         -- All those fields are indeed non queryable
-        for k,v in pairs(err) do
+        for k,v in pairs(err.error) do
           assert.is_not_true(dao_factory[collection]._schema[k].queryable)
         end
       end)

src/kong/dao/cassandra/base_dao.lua

@@ -10,6 +10,14 @@ local cjson = require "cjson"
 
 local validate = schemas.validate
 
+local error_types = {
+  SCHEMA = "schema",
+  CASSANDRA = "database"
+}
+
+--
+--
+--
 local BaseDao = Object:extend()
 
 function BaseDao:new(database)
@@ -56,10 +64,16 @@ local function encode_cassandra_values(schema, t, parameters)
   return values_to_bind
 end
 
+local function build_error(type, err)
+  if not err then
+    return nil
+  end
 
------------
--- UTILS --
------------
+  return {
+    is_schema = type == error_types.SCHEMA,
+    error = err
+  }
+end
 
 -- Run a statement and check if the result exists
 --
@@ -177,7 +191,7 @@ function BaseDao:_execute_prepared_stmt(statement, values_to_bind, options)
   local results, err = self._db:execute(statement.query, values_to_bind, options)
 
   if results and results.type == "ROWS" then
-    -- return deserialized content for encoded values (plugins)
+    -- return deserialized content for encoded values (plugin value column)
     if self._deserialize then
       for _,row in ipairs(results) do
         for k,v in pairs(row) do
@@ -188,15 +202,13 @@ function BaseDao:_execute_prepared_stmt(statement, values_to_bind, options)
       end
     end
 
-    -- erase this property to only return an ordered list
-    results.type = nil
-
     -- do we have more pages to fetch?
     if results.meta.has_more_pages then
       results.next_page = results.meta.paging_state
     end
 
     results.meta = nil
+    results.type = nil
 
     return results, err
   elseif results and results.type == "VOID" then
@@ -245,7 +257,9 @@ end
 -- @return {table|nil} Inserted entity or nil
 -- @return {table|nil} Error if any
 function BaseDao:insert(t)
-  if not t then return nil, "Cannot insert a nil element" end
+  if not t then
+    return nil, build_error(error_types.SCHEMA, "Cannot insert a nil element")
+  end
 
   -- Override created_at and id by default value
   t.created_at = utils.get_utc() * 1000
@@ -254,29 +268,28 @@ function BaseDao:insert(t)
   -- Validate schema
   local valid_schema, errors = validate(t, self._schema)
   if not valid_schema then
-    return nil, errors
+    return nil, build_error(error_types.SCHEMA, errors)
   end
 
   -- Check UNIQUE values
   local unique, err, errors = self:_check_all_unique(t)
   if err then
-    return nil, err
+    return nil, build_error(error_types.CASSANDRA, err)
   elseif not unique then
-    return nil, errors
+    return nil, build_error(error_types.SCHEMA, errors)
   end
 
   -- Check foreign entities EXIST
   local exists, err, errors = self:_check_all_exists(t)
   if err then
-    return nil, err
+    return nil, build_error(error_types.CASSANDRA, err)
   elseif not exists then
-    return nil, errors
+    return nil, build_error(error_types.SCHEMA, errors)
   end
 
   local _, err = self:_execute_prepared_stmt(self._statements.insert, t)
-
   if err then
-    return nil, err
+    return nil, build_error(error_types.CASSANDRA, err)
   else
     return t
   end
@@ -289,16 +302,18 @@ end
 -- @return {table|nil} Updated entity or nil
 -- @return {table|nil} Error if any
 function BaseDao:update(t)
-  if not t then return nil, "Cannot update a nil element" end
+  if not t then
+    return nil, build_error(error_types.SCHEMA, "Cannot update a nil element")
+  end
 
   -- Check if exists to prevent upsert
   -- and set UNSET values
   -- (pfffff...)
   local exists, err, results = self:_check_exists(self._statements.select_one, t)
   if err then
-    return nil, err
+    return nil, build_error(error_types.CASSANDRA, err)
   elseif not exists then
-    return nil, "Entity to update not found"
+    return nil, build_error(error_types.SCHEMA, "Entity to update not found")
   else
     -- Set UNSET values to prevent cassandra from setting to NULL
     -- @see Test case
@@ -313,29 +328,29 @@ function BaseDao:update(t)
   -- Validate schema
   local valid_schema, errors = validate(t, self._schema)
   if not valid_schema then
-    return nil, errors
+    return nil, build_error(error_types.SCHEMA, errors)
   end
 
   -- Check UNIQUE with update
   local unique, err, errors = self:_check_all_unique(t, true)
   if err then
-    return nil, err
+    return nil, build_error(error_types.CASSANDRA, err)
   elseif not unique then
-    return nil, errors
+    return nil, build_error(error_types.SCHEMA, errors)
   end
 
   -- Check foreign entities EXIST
   local exists, err, errors = self:_check_all_exists(t)
   if err then
-    return nil, err
+    return nil, build_error(error_types.CASSANDRA, err)
   elseif not exists then
-    return nil, errors
+    return nil, build_error(error_types.SCHEMA, errors)
   end
 
   local _, err = self:_execute_prepared_stmt(self._statements.update, t)
 
   if err then
-    return nil, err
+    return nil, build_error(error_types.CASSANDRA, err)
   else
     return t
   end
@@ -347,11 +362,15 @@ end
 -- @return _execute_prepared_stmt()
 function BaseDao:find_one(id)
   local data, err = self:_execute_prepared_stmt(self._statements.select_one, { id = id })
-  if not data or (data and utils.table_size(data) == 0) then
-    return nil, err
+
+  -- Return the 1st and only element of the result set
+  if data and utils.table_size(data) > 0 then
+    data = table.remove(data, 1)
   else
-    return table.remove(data, 1), err
+    data = nil
   end
+
+  return data, build_error(error_types.CASSANDRA, err)
 end
 
 -- Execute a SELECT statement with special WHERE values
@@ -382,18 +401,19 @@ function BaseDao:find_by_keys(t, page_size, paging_state)
     end
 
     if errors then
-      return nil, errors
+      return nil, build_error(error_types.SCHEMA, errors)
     end
 
     where_str = "WHERE "..table.concat(where, " AND ").." ALLOW FILTERING"
   end
 
   local select_query = string.format(self._queries.select.query, where_str)
 
+  -- prepare query in a statement cache
   if not self._statements_cache[select_query] then
     local stmt, err = self._db:prepare(select_query)
     if err then
-      return nil, err
+      return nil, build_error(error_types.CASSANDRA, err)
     end
 
     self._statements_cache[select_query] = {
@@ -402,10 +422,12 @@ function BaseDao:find_by_keys(t, page_size, paging_state)
     }
   end
 
-  return self:_execute_prepared_stmt(self._statements_cache[select_query], t, {
+  local rows, err = self:_execute_prepared_stmt(self._statements_cache[select_query], t, {
     page_size = page_size,
     paging_state = paging_state
   })
+
+  return rows, build_error(error_types.CASSANDRA, err)
 end
 
 -- Execute the prepared SELECT statement as it is
@@ -432,7 +454,9 @@ function BaseDao:delete(id)
     return false, "Entity to delete not found"
   end
 
-  return self:_execute_prepared_stmt(self._statements.delete, { id = id })
+  local ok, err = self:_execute_prepared_stmt(self._statements.delete, { id = id })
+
+  return ok, build_error(error_types.CASSANDRA, err)
 end
 
 return BaseDao

src/kong/tools/faker.lua

@@ -37,45 +37,29 @@ local Faker = Object:extend()
 
 function Faker:new(dao_factory)
   self.dao_factory = dao_factory
-  self.inserted_entities = {}
+  self:clear()
 end
 
 function Faker:clear()
-  self.entities_to_insert = {}
   self.inserted_entities = {}
 end
 
 -- Generate a fake entity
 --
 -- @param {string} type Type of the entity to generate
--- @param {boolean} invalid If true, generated entity will have an invalid schema on purpose
 -- @return {table} An entity schema
-function Faker:fake_entity(type, invalid)
+function Faker:fake_entity(type)
   local r = math.random(1, 1000000000)
 
   if type == "api" then
-    local name
-    if invalid then
-      name = 123456
-    else
-      name = "random"..r
-    end
-
     return {
-      name = name,
+      name = "random"..r,
       public_dns = "random"..r..".com",
       target_url = "http://random"..r..".com"
     }
   elseif type == "account" then
-    local provider_id
-    if invalid then
-      provider_id = "provider_123"
-    else
-      provider_id = "random_provider_id_"..r
-    end
-
     return {
-      provider_id = provider_id
+      provider_id = "random_provider_id_"..r
     }
   elseif type == "application" then
     return {