From 1113bbdf2251634e053c1bda389b472ffdc6f733 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:02:27 +0200 Subject: mod_storage_sql: Remove compat for argument order change --- plugins/mod_storage_sql.lua | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index 3fa3230b..f48c7ae9 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -183,9 +183,6 @@ archive_store.caps = { }; archive_store.__index = archive_store function archive_store:append(username, key, value, when, with) - if type(when) ~= "number" then - when, with, value = value, when, with; - end local user,store = username,self.store; local ok, key = engine:transaction(function() if key then -- cgit v1.2.3 From 0e5546ccf1433220ef102f4da71eaa9b1880a074 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:04:15 +0200 Subject: mod_offline: Include timestamp and empty 'with' --- plugins/mod_offline.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/mod_offline.lua b/plugins/mod_offline.lua index fe4df573..487098d1 100644 --- a/plugins/mod_offline.lua +++ b/plugins/mod_offline.lua @@ -24,7 +24,7 @@ module:hook("message/offline/handle", function(event) node = origin.username; end - return offline_messages:append(node, nil, stanza); + return offline_messages:append(node, nil, stanza, os.time(), ""); end, -1); module:hook("message/offline/broadcast", function(event) -- cgit v1.2.3 From b33ad7f001f8242a828e11e9c225346f4aadd81b Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:04:54 +0200 Subject: mod_storage_sql: Sensible defaults for 'when' and 'with' arguments --- plugins/mod_storage_sql.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index f48c7ae9..122a673a 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -184,6 +184,8 @@ archive_store.caps = { archive_store.__index = archive_store function archive_store:append(username, key, value, when, with) local user,store = username,self.store; + when = when or os.time(); + with = with or ""; local ok, key = engine:transaction(function() if key then engine:delete("DELETE FROM `prosodyarchive` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=?", host, user or "", store, key); -- cgit v1.2.3 From fbd4f04b73d8137a0a00d39804d75f1ee076c934 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:25:34 +0200 Subject: mod_storage_sql: Rename variables to avoid name clashes [luacheck] --- plugins/mod_storage_sql.lua | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index 122a673a..25a9dc56 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -32,8 +32,8 @@ local function serialize(value) elseif is_stanza(value) then return "xml", tostring(value); elseif t == "table" then - local value,err = json.encode(value); - if value then return "json", value; end + local encoded,err = json.encode(value); + if value then return "json", encoded; end return nil, err; end return nil, "Unhandled value type: "..t; @@ -80,15 +80,15 @@ local function keyval_store_set(data) local extradata = {}; for key, value in pairs(data) do if type(key) == "string" and key ~= "" then - local t, value = assert(serialize(value)); - engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, user or "", store, key, t, value); + local t, encoded_value = assert(serialize(value)); + engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, user or "", store, key, t, encoded_value); else extradata[key] = value; end end if next(extradata) ~= nil then - local t, extradata = assert(serialize(extradata)); - engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, user or "", store, "", t, extradata); + local t, encoded_extradata = assert(serialize(extradata)); + engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, user or "", store, "", t, encoded_extradata); end end return true; @@ -186,18 +186,18 @@ function archive_store:append(username, key, value, when, with) local user,store = username,self.store; when = when or os.time(); with = with or ""; - local ok, key = engine:transaction(function() + local ok, ret = engine:transaction(function() if key then engine:delete("DELETE FROM `prosodyarchive` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=?", host, user or "", store, key); else key = uuid.generate(); end - local t, value = assert(serialize(value)); - engine:insert("INSERT INTO `prosodyarchive` (`host`, `user`, `store`, `when`, `with`, `key`, `type`, `value`) VALUES (?,?,?,?,?,?,?,?)", host, user or "", store, when, with, key, t, value); + local t, encoded_value = assert(serialize(value)); + engine:insert(insert_sql, host, user or "", store, when, with, key, t, encoded_value); return key; end); - if not ok then return ok, key; end - return key; + if not ok then return ok, ret; end + return ret; -- the key end -- Helpers for building the WHERE clause -- cgit v1.2.3 From 61d955084fb668b8310ed7e5ba66556b9577f193 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:26:27 +0200 Subject: mod_storage_sql: Move SQL queries into multiline strings for readability [luacheck] --- plugins/mod_storage_sql.lua | 122 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 101 insertions(+), 21 deletions(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index 25a9dc56..40a59911 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -57,7 +57,12 @@ local user, store; local function keyval_store_get() local haveany; local result = {}; - for row in engine:select("SELECT `key`,`type`,`value` FROM `prosody` WHERE `host`=? AND `user`=? AND `store`=?", host, user or "", store) do + local select_sql = [[ + SELECT `key`,`type`,`value` + FROM `prosody` + WHERE `host`=? AND `user`=? AND `store`=?; + ]] + for row in engine:select(select_sql, host, user or "", store) do haveany = true; local k = row[1]; local v = deserialize(row[2], row[3]); @@ -74,21 +79,30 @@ local function keyval_store_get() end end local function keyval_store_set(data) - engine:delete("DELETE FROM `prosody` WHERE `host`=? AND `user`=? AND `store`=?", host, user or "", store); - + local delete_sql = [[ + DELETE FROM `prosody` + WHERE `host`=? AND `user`=? AND `store`=? + ]]; + engine:delete(delete_sql, host, user or "", store); + + local insert_sql = [[ + INSERT INTO `prosody` + (`host`,`user`,`store`,`key`,`type`,`value`) + VALUES (?,?,?,?,?,?); + ]] if data and next(data) ~= nil then local extradata = {}; for key, value in pairs(data) do if type(key) == "string" and key ~= "" then local t, encoded_value = assert(serialize(value)); - engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, user or "", store, key, t, encoded_value); + engine:insert(delete_sql, host, user or "", store, key, t, encoded_value); else extradata[key] = value; end end if next(extradata) ~= nil then local t, encoded_extradata = assert(serialize(extradata)); - engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, user or "", store, "", t, encoded_extradata); + engine:insert(insert_sql, host, user or "", store, "", t, encoded_extradata); end end return true; @@ -115,7 +129,12 @@ function keyval_store:set(username, data) end function keyval_store:users() local ok, result = engine:transaction(function() - return engine:select("SELECT DISTINCT `user` FROM `prosody` WHERE `host`=? AND `store`=?", host, self.store); + local select_sql = [[ + SELECT DISTINCT `user` + FROM `prosody` + WHERE `host`=? AND `store`=?; + ]]; + return engine:select(select_sql, host, self.store); end); if not ok then return ok, result end return iterator(result); @@ -129,14 +148,20 @@ map_store.__index = map_store; map_store.remove = {}; function map_store:get(username, key) local ok, result = engine:transaction(function() + local query = [[ + SELECT `type`, `value` + FROM `prosody` + WHERE `host`=? AND `user`=? AND `store`=? AND `key`=? + LIMIT 1 + ]]; local data; if type(key) == "string" and key ~= "" then - for row in engine:select("SELECT `type`, `value` FROM `prosody` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=? LIMIT 1", host, username or "", self.store, key) do + for row in engine:select(query, host, username or "", self.store, key) do data = deserialize(row[1], row[2]); end return data; else - for row in engine:select("SELECT `type`, `value` FROM `prosody` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=? LIMIT 1", host, username or "", self.store, "") do + for row in engine:select(query, host, username or "", self.store, "") do data = deserialize(row[1], row[2]); end return data and data[key] or nil; @@ -151,24 +176,38 @@ function map_store:set(username, key, data) end function map_store:set_keys(username, keydatas) local ok, result = engine:transaction(function() + local delete_sql = [[ + DELETE FROM `prosody` + WHERE `host`=? AND `user`=? AND `store`=? AND `key`=?; + ]]; + local insert_sql = [[ + INSERT INTO `prosody` + (`host`,`user`,`store`,`key`,`type`,`value`) + VALUES (?,?,?,?,?,?); + ]]; + local select_extradata_sql = [[ + SELECT `type`, `value` + FROM `prosody` + WHERE `host`=? AND `user`=? AND `store`=? AND `key`=? + LIMIT 1; + ]]; for key, data in pairs(keydatas) do if type(key) == "string" and key ~= "" then - engine:delete("DELETE FROM `prosody` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=?", + engine:delete(delete_sql, host, username or "", self.store, key); if data ~= self.remove then local t, value = assert(serialize(data)); - engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, username or "", self.store, key, t, value); + engine:insert(insert_sql, host, username or "", self.store, key, t, value); end else local extradata = {}; - for row in engine:select("SELECT `type`, `value` FROM `prosody` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=? LIMIT 1", host, username or "", self.store, "") do + for row in engine:select(select_extradata_sql, host, username or "", self.store, "") do extradata = deserialize(row[1], row[2]); end - engine:delete("DELETE FROM `prosody` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=?", - host, username or "", self.store, ""); + engine:delete(delete_sql, host, username or "", self.store, ""); extradata[key] = data; local t, value = assert(serialize(extradata)); - engine:insert("INSERT INTO `prosody` (`host`,`user`,`store`,`key`,`type`,`value`) VALUES (?,?,?,?,?,?)", host, username or "", self.store, "", t, value); + engine:insert(insert_sql, host, username or "", self.store, "", t, value); end end return true; @@ -187,8 +226,17 @@ function archive_store:append(username, key, value, when, with) when = when or os.time(); with = with or ""; local ok, ret = engine:transaction(function() + local delete_sql = [[ + DELETE FROM `prosodyarchive` + WHERE `host`=? AND `user`=? AND `store`=? AND `key`=?; + ]]; + local insert_sql = [[ + INSERT INTO `prosodyarchive` + (`host`, `user`, `store`, `when`, `with`, `key`, `type`, `value`) + VALUES (?,?,?,?,?,?,?,?); + ]]; if key then - engine:delete("DELETE FROM `prosodyarchive` WHERE `host`=? AND `user`=? AND `store`=? AND `key`=?", host, user or "", store, key); + engine:delete(delete_sql, host, user or "", store, key); else key = uuid.generate(); end @@ -233,12 +281,33 @@ local function archive_where_id_range(query, args, where) local args_len = #args -- Before or after specific item, exclusive if query.after then -- keys better be unique! - where[#where+1] = "`sort_id` > COALESCE((SELECT `sort_id` FROM `prosodyarchive` WHERE `key` = ? AND `host` = ? AND `user` = ? AND `store` = ? LIMIT 1), 0)" + where[#where+1] = [[ + `sort_id` > COALESCE( + ( + SELECT `sort_id` + FROM `prosodyarchive` + WHERE `key` = ? AND `host` = ? AND `user` = ? AND `store` = ? + LIMIT 1 + ), 0) + ]]; args[args_len+1], args[args_len+2], args[args_len+3], args[args_len+4] = query.after, args[1], args[2], args[3]; args_len = args_len + 4 end if query.before then - where[#where+1] = "`sort_id` < COALESCE((SELECT `sort_id` FROM `prosodyarchive` WHERE `key` = ? AND `host` = ? AND `user` = ? AND `store` = ? LIMIT 1), (SELECT MAX(`sort_id`)+1 FROM `prosodyarchive`))" + where[#where+1] = [[ + `sort_id` < COALESCE( + ( + SELECT `sort_id` + FROM `prosodyarchive` + WHERE `key` = ? AND `host` = ? AND `user` = ? AND `store` = ? + LIMIT 1 + ), + ( + SELECT MAX(`sort_id`)+1 + FROM `prosodyarchive` + ) + ) + ]] args[args_len+1], args[args_len+2], args[args_len+3], args[args_len+4] = query.before, args[1], args[2], args[3]; end end @@ -248,7 +317,12 @@ function archive_store:find(username, query) local user,store = username,self.store; local total; local ok, result = engine:transaction(function() - local sql_query = "SELECT `key`, `type`, `value`, `when`, `with` FROM `prosodyarchive` WHERE %s ORDER BY `sort_id` %s%s;"; + local sql_query = [[ + SELECT `key`, `type`, `value`, `when`, `with` + FROM `prosodyarchive` + WHERE %s + ORDER BY `sort_id` %s%s; + ]]; local args = { host, user or "", store, }; local where = { "`host` = ?", "`user` = ?", "`store` = ?", }; @@ -256,7 +330,8 @@ function archive_store:find(username, query) -- Total matching if query.total then - local stats = engine:select("SELECT COUNT(*) FROM `prosodyarchive` WHERE " .. t_concat(where, " AND "), unpack(args)); + local stats = engine:select("SELECT COUNT(*) FROM `prosodyarchive` WHERE " + .. t_concat(where, " AND "), unpack(args)); if stats then for row in stats do total = row[1]; @@ -273,7 +348,8 @@ function archive_store:find(username, query) args[#args+1] = query.limit; end - sql_query = sql_query:format(t_concat(where, " AND "), query.reverse and "DESC" or "ASC", query.limit and " LIMIT ?" or ""); + sql_query = sql_query:format(t_concat(where, " AND "), query.reverse + and "DESC" or "ASC", query.limit and " LIMIT ?" or ""); return engine:select(sql_query, unpack(args)); end); if not ok then return ok, result end @@ -405,7 +481,11 @@ local function upgrade_table(params, apply_changes) end -- COMPAT w/pre-0.10: Upgrade table to UTF-8 if not already - local check_encoding_query = "SELECT `COLUMN_NAME`,`COLUMN_TYPE`,`TABLE_NAME` FROM `information_schema`.`columns` WHERE `TABLE_NAME` LIKE 'prosody%%' AND ( `CHARACTER_SET_NAME`!='%s' OR `COLLATION_NAME`!='%s_bin' );"; + local check_encoding_query = [[ + SELECT `COLUMN_NAME`,`COLUMN_TYPE`,`TABLE_NAME` + FROM `information_schema`.`columns` + WHERE `TABLE_NAME` LIKE 'prosody%%' AND ( `CHARACTER_SET_NAME`!='%s' OR `COLLATION_NAME`!='%s_bin' ); + ]]; check_encoding_query = check_encoding_query:format(engine.charset, engine.charset); success,err = engine:transaction(function() local result = engine:execute(check_encoding_query); -- cgit v1.2.3 From 29c30180ee535df52089db53287bdfa8b0c8e61b Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:39:20 +0200 Subject: mod_storage_sql: Pass SQL engine as an argument to upgrade and table creator functions --- plugins/mod_storage_sql.lua | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index 40a59911..b6b71138 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -423,7 +423,7 @@ end --- Initialization -local function create_table(name) +local function create_table(engine, name) -- luacheck: ignore 431/engine local Table, Column, Index = sql.Table, sql.Column, sql.Index; local ProsodyTable = Table { @@ -458,7 +458,7 @@ local function create_table(name) end); end -local function upgrade_table(params, apply_changes) +local function upgrade_table(engine, params, apply_changes) -- luacheck: ignore 431/engine local changes = false; if params.driver == "MySQL" then local success,err = engine:transaction(function() @@ -540,13 +540,13 @@ function module.load() engine = engines[sql.db2uri(params)]; if not engine then module:log("debug", "Creating new engine"); - engine = sql:create_engine(params, function (engine) + engine = sql:create_engine(params, function (engine) -- luacheck: ignore 431/engine if module:get_option("sql_manage_tables", true) then -- Automatically create table, ignore failure (table probably already exists) -- FIXME: we should check in information_schema, etc. - create_table(); + create_table(engine); -- Check whether the table needs upgrading - if upgrade_table(params, false) then + if upgrade_table(engine, params, false) then module:log("error", "Old database format detected. Please run: prosodyctl mod_%s upgrade", module.name); return false, "database upgrade needed"; end @@ -583,7 +583,7 @@ function module.command(arg) for _, params in pairs(uris) do print("Checking "..params.database.."..."); engine = sql:create_engine(params); - upgrade_table(params, true); + upgrade_table(engine, params, true); end print("All done!"); elseif command then -- cgit v1.2.3 From d5205416d5a586cccbf863fdea79d8f85b83a959 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:45:20 +0200 Subject: mod_storage_sql: Ignore name clash [luacheck] --- plugins/mod_storage_sql.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index b6b71138..0ca0512f 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -515,7 +515,7 @@ local function upgrade_table(engine, params, apply_changes) -- luacheck: ignore return changes; end -local function normalize_database(driver, database) +local function normalize_database(driver, database) -- luacheck: ignore 431/driver if driver == "SQLite3" and database ~= ":memory:" then return resolve_relative_path(prosody.paths.data or ".", database or "prosody.sqlite"); end -- cgit v1.2.3 From ee705afbefd752e4d0aa726d0b045b9145a4f581 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:45:37 +0200 Subject: mod_storage_sql: Split long lines [luacheck] --- plugins/mod_storage_sql.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index 0ca0512f..56d91dc6 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -524,8 +524,10 @@ end local function normalize_params(params) return { - driver = assert(params.driver, "Configuration error: Both the SQL driver and the database need to be specified"); - database = assert(normalize_database(params.driver, params.database), "Configuration error: Both the SQL driver and the database need to be specified"); + driver = assert(params.driver, + "Configuration error: Both the SQL driver and the database need to be specified"); + database = assert(normalize_database(params.driver, params.database), + "Configuration error: Both the SQL driver and the database need to be specified"); username = params.username; password = params.password; host = params.host; -- cgit v1.2.3 From dd86b147bf0e4a6a606b2c4c1b3896776e81d98e Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:45:47 +0200 Subject: mod_storage_sql: Ignore name clash [luacheck] --- plugins/mod_storage_sql.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index 56d91dc6..da0f471c 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -567,7 +567,7 @@ function module.command(arg) if command == "upgrade" then -- We need to find every unique dburi in the config local uris = {}; - for host in pairs(prosody.hosts) do + for host in pairs(prosody.hosts) do -- luacheck: ignore 431/host local params = normalize_params(config.get(host, "sql") or default_params); uris[sql.db2uri(params)] = params; end -- cgit v1.2.3 From 67572368f294b4a1f018ab78d42bdf7cf12e7d8c Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Sat, 1 Apr 2017 19:47:02 +0200 Subject: mod_storage_sql: Remove unused return values (should those actually be used?) --- plugins/mod_storage_sql.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/mod_storage_sql.lua b/plugins/mod_storage_sql.lua index da0f471c..a04a39a0 100644 --- a/plugins/mod_storage_sql.lua +++ b/plugins/mod_storage_sql.lua @@ -487,7 +487,8 @@ local function upgrade_table(engine, params, apply_changes) -- luacheck: ignore WHERE `TABLE_NAME` LIKE 'prosody%%' AND ( `CHARACTER_SET_NAME`!='%s' OR `COLLATION_NAME`!='%s_bin' ); ]]; check_encoding_query = check_encoding_query:format(engine.charset, engine.charset); - success,err = engine:transaction(function() + -- FIXME Is it ok to ignore the return values from this? + engine:transaction(function() local result = engine:execute(check_encoding_query); local n_bad_columns = result:rowcount(); if n_bad_columns > 0 then -- cgit v1.2.3