Update: cherry pick updates from 0.5 branch

Add indexes on the auth table in the sauth.sqlite db
Add annotations for functions and code
Add caching on auth record creation
Change name of cache table to cache
Return the result of all functions directly where possible for potential future error handling
Restructure the code for the cache in get_auth 
Turn on caching on load by default - use conf setting to disable

init.lua

@@ -1,33 +1,36 @@
--- sauth mod for Minetest 0.4.x
+-- sqlite3 auth handler mod for minetest 0.4 voxel game
 -- by shivajiva101@hotmail.com
 
--- Expose auth handler functions
+-- Expose handler functions
 sauth = {}
-local auth_table = {}
+local cache = {}
 local MN = minetest.get_current_modname()
 local WP = minetest.get_worldpath()
 local ie = minetest.request_insecure_environment()
-
--- conf file settings
-local caching = minetest.setting_getbool(MN .. '.caching') or false
-local max_cache_records = tonumber(minetest.setting_get(MN .. '.cache_max')) or 500
-local ttl = tonumber(minetest.setting_get(MN..'.cache_ttl')) or 86400 -- defaults to 24 hours
+local owner_privs_cached = false
 
 if not ie then
 	error("insecure environment inaccessible"..
 		" - make sure this mod has been added to minetest.conf!")
 end
 
--- Requires library for db access
+-- read mt conf file settings
+local caching = minetest.setting_get_bool(MN .. '.caching') or true
+local max_cache_records = tonumber(minetest.setting_get(MN .. '.cache_max')) or 500
+local ttl = tonumber(minetest.setting_get(MN..'.cache_ttl')) or 86400 -- defaults to 24 hours
+local owner = minetest.setting_get("name")
+
+-- localise library for db access
 local _sql = ie.require("lsqlite3")
 
--- Prevent other mods using this instance!
+-- Prevent use of this db instance. If you want to run mods that
+-- don't secure this global make sure they load AFTER this mod!
 if sqlite3 then sqlite3 = nil end
 
 local singleplayer = minetest.is_singleplayer()
 
 -- Use conf setting to determine handler for singleplayer
-if not minetest.setting_get(MN .. '.enable_singleplayer')
+if not minetest.setting_get_bool(MN .. '.enable_singleplayer')
 and singleplayer then
 	  minetest.log("info", "singleplayer game using builtin auth handler")
 	  return
@@ -35,16 +38,24 @@ end
 
 local db = _sql.open(WP.."/sauth.sqlite") -- connection
 
--- Create db:exec wrapper for error reporting
+--- Apply statements against the current database
+--- wrapping db:exec for error reporting
+---@param stmt string
+---@return boolean
+---@return string error message
 local function db_exec(stmt)
 	if db:exec(stmt) ~= _sql.OK then
 		minetest.log("info", "Sqlite ERROR:  ", db:errmsg())
+		return false, db:errmsg()
 	end
+	return true
 end
 
 -- Cache handling
 local cap = 0
-local function fetch_cache()
+
+--- Create cache on load
+local function create_cache()
 	local q = "SELECT max(last_login) AS result FROM auth;"
 	local it, state = db:nrows(q)
 	local last = it(state)
@@ -53,7 +64,7 @@ local function fetch_cache()
 		q = ([[SELECT *	FROM auth WHERE last_login > %s LIMIT %s;
 		]]):format(last, max_cache_records)
 		for row in db:nrows(q) do
-			auth_table[row.name] = {
+			cache[row.name] = {
 				password = row.password,
 				privileges = minetest.string_to_privs(row.privileges),
 				last_login = row.last_login
@@ -61,33 +72,38 @@ local function fetch_cache()
 			cap = cap + 1
 		end
 	end
-	minetest.log("action", "[sauth] cached " .. cap .. " records.")
+	minetest.log("action", "[sauth] caching " .. cap .. " records.")
 end
 
+--- Remove oldest entry in the cache
 local function trim_cache()
 	if cap < max_cache_records then return end
 	local entry = os.time()
 	local name
-	for k, v in pairs(auth_table) do
+	for k, v in pairs(cache) do
 		if v.last_login < entry then
 			entry = v.last_login
 			name = k
 		end
 	end
-	auth_table[name] = nil
+	cache[name] = nil
 	cap = cap - 1
 end
 
--- Db tables - because we need them!
+-- Define db tables
 local create_db = [[
-CREATE TABLE IF NOT EXISTS auth (name VARCHAR(32) PRIMARY KEY ON CONFLICT IGNORE,
+CREATE TABLE IF NOT EXISTS auth (name VARCHAR(32) PRIMARY KEY,
 password VARCHAR(512), privileges VARCHAR(512), last_login INTEGER);
-CREATE TABLE IF NOT EXISTS _s (import BOOLEAN, db_version VARCHAR(6));
+
+CREATE INDEX IF NOT EXISTS idx_auth_name ON auth(name);
+CREATE INDEX IF NOT EXISTS idx_auth_lastlogin ON auth(last_login);
+
+CREATE TABLE IF NOT EXISTS _s (import BOOLEAN, db_version VARCHAR (6));
 ]]
 db_exec(create_db)
 
 if caching then
-	fetch_cache()
+	create_cache()
 end
 
 --[[
@@ -96,10 +112,10 @@ end
 ###########################
 ]]
 
-local function get_record(name)
-	-- cached?
-	if auth_table[name] then return auth_table[name] end
-	-- fetch record
+--- Get Player db record
+---@param name string
+---@return table pairs
+local function get_player_record(name)
 	local query = ([[
 	    SELECT * FROM auth WHERE name = '%s' LIMIT 1;
 	]]):format(name)
@@ -108,6 +124,9 @@ local function get_record(name)
 	return row
 end
 
+--- Check db for match
+---@param name string
+---@return table or nil
 local function check_name(name)
 	local query = ([[
 		SELECT DISTINCT name
@@ -119,15 +138,23 @@ local function check_name(name)
 	return row
 end
 
-local function get_setting(column)
+--- Fetch setting
+---@param column_name string
+---@return table pairs
+local function get_setting(column_name)
 	local query = ([[
 		SELECT %s FROM _s
-	]]):format(column)
+	]]):format(column_name)
 	local it, state = db:nrows(query)
 	local row = it(state)
-	if row then return row[column] end
+	if row then return row[column_name] end
 end
 
+--- Search for records where the name is like param string
+---@param name any
+---@return table ipairs
+--- Uses sql LIKE %name% to pattern match any
+--- string that contains name
 local function search(name)
 	local r,q = {}
 	q = "SELECT name FROM auth WHERE name LIKE '%"..name.."%';"
@@ -137,6 +164,8 @@ local function search(name)
 	return r
 end
 
+--- Get pairs table of names in the database
+---@return table
 local function get_names()
 	local r,q = {}
 	q = "SELECT name FROM auth;"
@@ -146,64 +175,138 @@ local function get_names()
 	return r
 end
 
+
 --[[
-##############################
-###  Database: Statements  ###
-##############################
+###########################
+###  Database: Inserts  ###
+###########################
 ]]
 
-local function add_record(name, password, privs, last_login)
+--- Add auth record to database
+---@param name string
+---@param password string
+---@param privs string
+---@param last_login integer
+---@return boolean
+---@return string error message
+local function add_player_record(name, password, privs, last_login)
 	local stmt = ([[
 		INSERT INTO auth (
 		name,
 		password,
 		privileges,
 		last_login
-	) VALUES ('%s','%s','%s','%s')
+		) VALUES ('%s','%s','%s',%i)
 	]]):format(name, password, privs, last_login)
-	db_exec(stmt)
+	return db_exec(stmt)
 end
 
-local function add_setting(column, val)
+--- Add setting to the database
+---@param name string
+---@param val any
+---@return boolean
+---@return string error message
+local function add_setting(name, val)
 	local stmt = ([[
 		INSERT INTO _s (%s) VALUES ('%s')
-	]]):format(column, val)
-	db_exec(stmt)
+	]]):format(name, val)
+	return db_exec(stmt)
 end
 
-local function update_login(name)
-	local ts = os.time()
+-- Add db version to settings
+if not get_setting('db_version') then
+	add_setting('db_version', '1.1')
+end
+
+
+--[[
+###########################
+###  Database: Updates  ###
+###########################
+]]
+
+--- Update last login for a player
+---@param name string
+---@param timestamp integer
+---@return boolean
+---@return string error message
+local function update_auth_login(name, timestamp)
 	local stmt = ([[
 		UPDATE auth SET last_login = %i WHERE name = '%s'
-	]]):format(ts, name)
-	db_exec(stmt)
+	]]):format(timestamp, name)
+	return db_exec(stmt)
 end
 
+--- Update password for a player
+---@param name string
+---@param password string
+---@return boolean
+---@return string error message
 local function update_password(name, password)
 	local stmt = ([[
 		UPDATE auth SET password = '%s' WHERE name = '%s'
 	]]):format(password,name)
-	db_exec(stmt)
+	return db_exec(stmt)
 end
 
+--- Update privileges for a player
+---@param name string
+---@param privs string
+---@return boolean
+---@return string error message
 local function update_privileges(name, privs)
 	local stmt = ([[
 		UPDATE auth SET privileges = '%s' WHERE name = '%s'
 	]]):format(privs,name)
-	db_exec(stmt)
+	return db_exec(stmt)
 end
 
+
+--[[
+#############################
+###  Database: Deletions  ###
+#############################
+]]
+
+--- Delete a players auth record from the database
+---@param name string
+---@return boolean
+---@return string error message
 local function del_record(name)
 	local stmt = ([[
 		DELETE FROM auth WHERE name = '%s'
 	]]):format(name)
-	db_exec(stmt)
+	return db_exec(stmt)
 end
 
-if not get_setting('db_version') then
-	add_setting('db_version', '1.1')
+
+--[[
+###################
+###  Functions  ###
+###################
+]]
+
+--- Get Player db record
+---@param name string
+---@return table pairs
+local function get_record(name)
+	-- Prioritise cache
+	if cache[name] then return cache[name] end
+	return get_player_record(name)
 end
 
+--- Update last login for a player
+---@param name string
+---@param timestamp integer
+---@return boolean
+---@return string error message
+local function update_login(name)
+	local ts = os.time()
+	cache[name].last_login = ts
+	return update_auth_login(name, ts)
+end
+
+
 --[[
 ######################
 ###  Auth Handler  ###
@@ -211,93 +314,121 @@ end
 ]]
 
 sauth.auth_handler = {
+
+	--- Return auth record entry with privileges as a pair table
+	--- Prioritises cached data over repeated db searches
+	---@param name string
+	---@param add_to_cache boolean optional - default is true
+	---@return table of pairs
 	get_auth = function(name, add_to_cache)
-		-- Return password,privileges,last_login
+
+		-- Check param
 		assert(type(name) == 'string')
-		-- catch empty names for mods that do privilege checks
-		if name == nil or name == '' or name == ' ' then
-			minetest.log("info", "[sauth] Name missing in call to get_auth. Rejected.")
-			return nil
+
+		-- if an auth record exists in the cache the only
+		-- other check reqd is that the owner has all privs
+		if cache[name] then
+			if not owner_privs_cached and name == owner then
+				-- grant all privs
+				for priv, def in pairs(minetest.registered_privileges) do
+					cache[name].privileges[priv] = true
+				end
+				owner_privs_cached = true
+			end
+			return cache[name]
 		end
+
 		-- catch ' passed in name string to prevent crash
 		if name:find("%'") then return nil end
-		add_to_cache = add_to_cache or true -- Assert caching on missing param
-		local r = auth_table[name]
-		-- Check and load db record if reqd
-		if r == nil then
-			r = get_record(name)
-		end
-		-- Return nil on missing entry
-		if not r then return nil end
-		-- Figure out what privileges the player should have.
-		-- Take a copy of the players privilege table
-		local privileges, admin = {}
-		if type(r.privileges) == "string" then
-			-- db record
-			for priv, _ in pairs(minetest.string_to_privs(r.privileges)) do
-				privileges[priv] = true
-			end
+
+		-- Assert caching on missing param
+		add_to_cache = add_to_cache or true
+
+		-- Check db for matching record
+		local auth_entry = get_player_record(name)
+
+		-- Unknown name check
+		if not auth_entry then return nil end
+
+		-- Make a copy of the players privilege table.
+		-- Data originating from the db is a string
+		-- so it must be mutated to a table
+		local privileges
+		if type(auth_entry.privileges) == "string" then
+			-- Reconstruct table using minetest function
+			privileges = minetest.string_to_privs(auth_entry.privileges)
 		else
-			-- cache
-			privileges = r.privileges
+			privileges = auth_entry.privileges -- cached
 		end
-		if minetest.settings then
-			admin = minetest.settings:get("name")
-		else
-			-- use old api
-			admin = minetest.setting_get("name")
-		end
-		-- If singleplayer, grant privileges marked give_to_singleplayer = true
+
+		-- If singleplayer, grant privileges marked give_to_singleplayer
 		if minetest.is_singleplayer() then
 			for priv, def in pairs(minetest.registered_privileges) do
 				if def.give_to_singleplayer then
 					privileges[priv] = true
 				end
 			end
-		-- If admin, grant all privileges
-		elseif name == admin then
+
+		-- Grant owner all privileges
+		elseif name == owner then
 			for priv, def in pairs(minetest.registered_privileges) do
 				privileges[priv] = true
 			end
 		end
+
 		-- Construct record
 		local record = {
-			password = r.password,
+			password = auth_entry.password,
 			privileges = privileges,
-			last_login = tonumber(r.last_login)
-			}
-		-- Cache if reqd
-		if not auth_table[name] and add_to_cache then
-			auth_table[name] = record
+			last_login = tonumber(auth_entry.last_login)}
+
+		-- Cache if reqd, mt core calls this function constantly
+		-- so minimise the codes path to speed things up
+		if add_to_cache then
+			cache[name] = record
 			cap = cap + 1
+			return record
 		end
-		return record
 	end,
+
+	--- Create a new auth entry
+	---@param name string
+	---@param password string
+	---@return boolean
 	create_auth = function(name, password)
 		assert(type(name) == 'string')
 		assert(type(password) == 'string')
 		local ts = os.time()
-		local privs
-		if minetest.settings then
-			privs = minetest.settings:get("default_privs")
-		else
-			-- use old api
-			privs = minetest.setting_get("default_privs")
-		end
-		-- Params: name, password, privs, last_login
-		add_record(name,password,privs,ts)
+		local privs = minetest.setting_get("default_privs")
+		add_player_record(name,password,privs,ts)
+		cache[name] = {
+			password = password,
+			privileges = minetest.string_to_privs(privs),
+			last_login = -1 -- defer
+		}
 		return true
 	end,
+
+
+	--- Delete an auth entry
+	---@param name string
+	---@return boolean
 	delete_auth = function(name)
 		assert(type(name) == 'string')
 		local record = get_record(name)
 		if record then
 			del_record(name)
-			auth_table[name] = nil
+			cache[name] = nil
 			minetest.log("info", "[sauth] Db record for " .. name .. " was deleted!")
 			return true
 		end
+		return false
 	end,
+
+	--- Set password for an auth record
+	---@param name string
+	---@param password string
+	---@return boolean
 	set_password = function(name, password)
 		assert(type(name) == 'string')
 		assert(type(password) == 'string')
@@ -306,55 +437,61 @@ sauth.auth_handler = {
 			sauth.auth_handler.create_auth(name, password)
 		else
 			update_password(name, password)
-			if auth_table[name] then auth_table[name].password = password end
+			if cache[name] then cache[name].password = password end
 		end
 		return true
 	end,
-	set_privileges = function(name, privs)
+
+	--- Set privileges for an auth record
+	---@param name string
+	---@param privileges string
+	---@return boolean
+	set_privileges = function(name, privileges)
 		assert(type(name) == 'string')
-		assert(type(privs) == 'table')
-		if not sauth.auth_handler.get_auth(name) then
-			-- create the record
-			if minetest.settings then
-				sauth.auth_handler.create_auth(name,
-					minetest.get_password_hash(name,
-						minetest.settings:get("default_password")))
-			else
-				sauth.auth_handler.create_auth(name,
+		assert(type(privileges) == 'table')
+		local auth_entry = sauth.auth_handler.get_auth(name)
+		if not auth_entry then
+			sauth.auth_handler.create_auth(name,
 					minetest.get_password_hash(name,
 						minetest.setting_get("default_password")))
-			end
+
 		end
-		local admin
-		if minetest.settings then
-			admin = minetest.settings:get("name")
-		else
-			-- use old api method
-			admin = minetest.setting_get("name")
-		end
-		if name == admin then privs.privs = true end
-		update_privileges(name, minetest.privs_to_string(privs))
-		if auth_table[name] then auth_table[name].privileges = privs end
+		-- Ensure owner has ability to grant
+		if name == owner then privileges.privs = true end
+		-- Update record
+		update_privileges(name, minetest.privs_to_string(privileges))
+		if cache[name] then cache[name].privileges = privileges end
 		minetest.notify_authentication_modified(name)
 		return true
 	end,
+
+	--- Reload database
+	---@param return boolean
 	reload = function()
+		-- deprecated due to the change in storage mechanism but maybe useful
+		-- for cache regeneration
 		return true
 	end,
+
+	--- Records the last login timestamp
+	---@param name string
+	---@return boolean
+	---@return string error message
 	record_login = function(name)
 		assert(type(name) == 'string')
-		update_login(name)
-
-		local auth = auth_table[name]
-		if auth then
-			auth.last_login = os.time()
-		end
-		return true
+		return update_login(name)
 	end,
+
+	--- Searches for names like param
+	---@param name string
+	---@return table ipairs
 	name_search = function(name)
 		assert(type(name) == 'string')
 		return search(name)
 	end,
+
+	--- Return an iterator function for the auth table names
+	---@return function iterator
 	iterate = function()
 		local names = get_names()
 		return pairs(names)
@@ -453,7 +590,7 @@ if get_setting("import") == nil then
 		end
 		for name, stuff in pairs(importauth) do
 			if name ~= player_name then
-				add_record(name,stuff.password,stuff.privileges,stuff.last_login)
+				add_player_record(name,stuff.password,stuff.privileges,stuff.last_login)
 			else
 				update_privileges(name, stuff.privileges)
 				update_password(name, stuff.password)
@@ -478,10 +615,6 @@ if get_setting("import") == nil then
 		if get_setting("import") == nil then export_auth() end -- dump to sql
 		-- rename auth.txt otherwise it will still load!
 		ie.os.rename(WP.."/auth.txt", WP.."/auth.txt.bak")
-		-- removed from later versions of minetest
-		--if minetest.auth_table then
-			--minetest.auth_table = {} -- unload redundant data
-		--end
 		minetest.notify_authentication_modified()
 	end
 	minetest.after(5, task)
@@ -492,9 +625,12 @@ end
 ###  Register hooks  ###
 ########################
 ]]
+
 -- Register auth handler
 minetest.register_authentication_handler(sauth.auth_handler)
-minetest.log('action', MN .. ": Registered auth handler")
+
+-- Log event as minetest registers silently
+minetest.log('action', "[sauth] now registered as the authentication handler")
 
 minetest.register_on_prejoinplayer(function(name, ip)
 	local r = get_record(name)