Fix. Cache class. (#2755)

* Fix. Cache class.

 * `send_event` raise error so `Cache.del` did not remove key or send any event
 * use `memcache` method by default even if `cache` table does not defined in config
 * `Cache.get` did not return any data when use `memcache` method
 * `Cache.get` did not close file. (Its should not be a big problem because GC should do it by self).
 * `Cache.get` can returns some undefined global value. (if method is `file` and file not exists then method returns global `result` value)
 * `Cache.get` does not need check for file existence
 * Value escaping does not needed for `file` method
 * Needed different key escaping for `memcache` and `file` methods
 * Update self test

* Change. Use random names for temp files.

resources/install/scripts/resources/functions/cache.lua

@@ -11,7 +11,15 @@ require "resources.functions.config";
 
 -- include functions
 require "resources.functions.trim";
-require "resources.functions.file_exists";
+
+-- include file class
+local File = require "resources.functions.file";
+
+-- get logger
+local log = require "resources.functions.log".cache;
+
+-- get method for cache from config
+local cache_method = cache and cache.method or 'memcache'
 
 local api = api
 if not api then
@@ -25,17 +33,16 @@ if not api then
   end
 end
 
-local function send_event(action, key)
+local send_event
-    if (cache.method == "memcache") then
+if freeswitch then
-      local event = freeswitch.Event("CUSTOM", "fusion::memcache");
+  send_event = function (action, key)
-      event:addHeader("API-Command", "memcache");
+    local event = freeswitch.Event("CUSTOM", "fusion::" .. cache_method)
-    end
+    event:addHeader("API-Command", cache_method)
-    if (cache.method == "file") then
+    event:addHeader("API-Command-Argument", action .. " " .. key)
-      local event = freeswitch.Event("CUSTOM", "fusion::file");
-      event:addHeader("API-Command", "file");
-    end
-    event:addHeader("API-Command-Argument", action .. " " .. key);
     event:fire()
+  end
+else
+  send_event = function() end
 end
 
 local Cache = {}
@@ -54,15 +61,47 @@ local function check_error(result)
   return result
 end
 
+-- convert cache key to file path
+local function key2file(key)
+  return cache.location .. '/' .. string.gsub(key, '[:\\/]', {
+    [':']  = '.',
+    ['\\'] = '_',
+    ['/']  = '_',
+  })
+end
+
+-- generate random file name
+local function tmp_file(key)
+  -- @todo may be it worth fallback to `os.tmpname`
+  local uuid = check_error(api:execute("create_uuid", ""))
+  if uuid then key = key .. '.' .. uuid end
+  return key .. '.tmp'
+end
+
+-- convert cache key to memcache key
+local function key2key(key)
+  return (string.gsub(key, "\\", "\\\\"))
+end
+
+-- encode value to be able store it in memcache
+local function memcache_encode(value)
+  return (string.gsub(value, "'", "'"):gsub("\\", "\\\\"))
+end
+
+-- decode value retrived from memcache
+local function memcache_decode(value)
+  return (string.gsub(value, "'", "'"))
+end
+
 function Cache.support()
   -- assume it is not unloadable
   if Cache._support then
     return true
   end
-  if (cache.method == "memcache") then
+  if (cache_method == "memcache") then
     Cache._support = (trim(api:execute('module_exists', 'mod_memcache')) == 'true')
   else
-  	Cache._support = true;
+    Cache._support = true;
   end
   return Cache._support
 end
@@ -75,77 +114,103 @@ end
 -- @return[2] error string `e.g. 'NOT FOUND'
 -- @note error string does not contain `-ERR` prefix
 function Cache.get(key)
-  local key = key:gsub(":", ".")
+  local result, err = nil, 'UNSUPPORTTED'
-  if (cache.method == "memcache") then
+
-    local result, err = check_error(api:execute('memcache', 'get ' .. key))
+  if (cache_method == "memcache") then
+    result, err = check_error(api:execute('memcache', 'get ' .. key2key(key)))
+    if result then
+      result = memcache_decode(result)
+    end
   end
-  if (cache.method == "file") then
+
-    if (file_exists(cache.location .. "/" .. key)) then
+  if (cache_method == "file") then
-      --freeswitch.consoleLog("notice", "[cache] location: " .. cache.location .. "/" .. key .."\n");
+    key = key2file(key)
-      local file, err = io.open(cache.location .. "/" .. key,  "rb")
+    -- log.noticef('location: %s', key)
-      result = file:read("*all")
+    result, err = File.read(key)
-    else
+    if not result then
       err = 'NOT FOUND';
     end
   end
-  --freeswitch.consoleLog("notice", "[cache] result: " .. result .. "\n");
+
-  --file:close()
+  -- log.noticef('result: %s',  tostring(result or err))
-  if not result then return nil, err end
+  return result, err
-  return (result:gsub("'", "'"))
 end
 
 function Cache.set(key, value, expire)
-  key = key:gsub(":", ".")
+  if (cache_method == "file") then
-  value = value:gsub("'", "'"):gsub("\\", "\\\\")
+    -- To make write to cache atomic we first write to some
-  --local ok, err = check_error(write_file(cache.location .. "/" .. key, value))
+    -- temp file with uniq random name. So if there more than
-  if (cache.method == "file") then
+    -- one writers all of then write to its own temp file.
-    if (not file_exists(cache.location .. "/" .. key .. ".tmp")) then
+    -- After it done we do rename and this operation will
+    -- leave only one file. If rename fail we assume that
+    -- some one else write this cache item so we just remove our
+    -- temp file. This should works because all writers should
+    -- write same values and it does not metter which one do this first.
+    key = key2file(key)
+    if (not File.exists(key)) then
+      -- get random name for the temp file
+      local key_tmp = tmp_file(key)
       --write the temp file
-      local file, err = io.open(cache.location .. "/" .. key .. ".tmp", "wb")
+      local ok, err = File.write(key_tmp, value)
-      if not file then
+      if not ok then
-        log.err("Can not open file to write:" .. tostring(err))
+        log.errf('can not write file `%s`: %s', key_tmp, tostring(err))
         return nil, err
       end
-      file:write(value)
-      file:close()
       --move the temp file
-      os.rename(cache.location .. "/" .. key .. ".tmp", cache.location .. "/" .. key)
+      ok, err = File.rename(key_tmp, key)
+      -- if we can not rename file then assume that key already exists,
+      -- so we have to remove our temp file.
+      if not ok then File.remove(key_tmp) end
+      return ok, err
     end
+    --! @todo returns special code to show reuse value?
+    return true
   end
-  if (cache.method == "memcache") then
+
+  if (cache_method == "memcache") then
+    value = memcache_encode(value)
     expire = expire and tostring(expire) or ""
-    local ok, err = check_error(api:execute("memcache", "set " .. key .. " '" .. value .. "' " .. expire))
+    local ok, err = check_error(api:execute("memcache", "set " .. key2key(key) .. " '" .. value .. "' " .. expire))
     if not ok then return nil, err end
     return ok == '+OK'
   end
+
+  return nil, 'UNSUPPORTTED'
 end
 
 function Cache.del(key)
-  key = key:gsub(":", ".")
   send_event('delete', key)
-  if (cache.method == "memcache") then
+
-    local result, err = check_error(api:execute("memcache", "delete " .. key))
+  if (cache_method == "memcache") then
-  end
+    local result, err = check_error(api:execute("memcache", "delete " .. key2key(key)))
-  if (cache.method == "file") then
+    if not result then
-    if (file_exists(cache.location .. "/" .. key)) then
+      if err == 'NOT FOUND' then
-      os.remove(cache.location .. "/" .. key)
+        return true
-      if (file_exists(cache.location .. "/" .. key .. ".tmp")) then
-        os.remove(cache.location .. "/" .. key .. ".tmp")
       end
-    else
+      return nil, err
-      err = 'NOT FOUND'
     end
+    return result == '+OK'
   end
-  if not result then
+
-    if err == 'NOT FOUND' then
+  if (cache_method == "file") then
-      return true
+    key = key2file(key)
+    --! @todo remove file exists check. This check needs only for return `NOT FOUND` code.
+    local result, err = not File.exists(key)
+    if not result then
+      result, err = File.remove(key)
+      if not result then
+        log.errf('can not remove file `%s`: %s', key, tostring(err))
+      end
     end
-    return nil, err
+    File.remove(key .. ".tmp")
+    return result, err
   end
-  return result == '+OK'
+
+  return nil, 'UNSUPPORTTED'
 end
 
 function Cache._self_test()
+  print('cache mode: ', cache_method)
   assert(Cache.support())
   Cache.del("a")
 
@@ -158,10 +223,23 @@ function Cache._self_test()
   assert(s == Cache.get("a"))
 
   assert(true == Cache.del("a"))
+
+  local k = 'a/b\\c/d'
+  Cache.del(k)
+
+  assert(true == Cache.set(k, s))
+  assert(s == Cache.get(k))
+  assert(true == Cache.del(k))
+
+  ok, err = Cache.get(k)
+  assert(nil == ok)
+  assert(err == "NOT FOUND")
+
+  print('done')
 end
 
--- if debug.self_test then
+if debug.self_test then
---   Cache._self_test()
+  Cache._self_test()
--- end
+end
 
 return Cache

resources/install/scripts/resources/functions/file.lua

@@ -9,7 +9,7 @@ local base64 = base64
 local function write_file(fname, data, mode)
 	local file, err = io.open(fname, mode or "wb")
 	if not file then
-		log.err("Can not open file to write:" .. tostring(err))
+		-- log.err("Can not open file to write:" .. tostring(err))
 		return nil, err
 	end
 	file:write(data)
@@ -25,7 +25,7 @@ end
 local function read_file(fname, mode)
 	local file, err = io.open(fname, mode or "rb")
 	if not file then
-		log.err("Can not open file to read:" .. tostring(err))
+		-- log.err("Can not open file to read:" .. tostring(err))
 		return nil, err
 	end
 	local data = file:read("*all")
@@ -54,4 +54,5 @@ return {
 	write_base64 = write_base64;
 	exists       = file_exists;
 	remove       = os.remove;
+	rename       = os.rename;
 }