From a85528e956bcbe4034435b38de16675427b7813b Mon Sep 17 00:00:00 2001
From: peteruithoven <peter@peteruithoven.nl>
Date: Thu, 17 Oct 2013 14:04:13 +0200
Subject: [PATCH] More helpfull validation errors

---
 src/rest/api/api_config.lua | 10 ++++++--
 src/util/settings.lua       | 47 +++++++++++++++++++++++--------------
 2 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/src/rest/api/api_config.lua b/src/rest/api/api_config.lua
index ba07751..48d0b72 100644
--- a/src/rest/api/api_config.lua
+++ b/src/rest/api/api_config.lua
@@ -1,4 +1,5 @@
 local log = require('util.logger')
+local utils = require('util.utils')
 local settings = require('util.settings')
 local printer = require('util.printer')
 local signin = require('network.signin')
@@ -19,12 +20,17 @@ function M._global_GET(request, response)
 end
 
 function M._global_POST(request, response)
+	--log:info("API:config:set")
 	response:setSuccess()
 	for k,v in pairs(request:getAll()) do
+		--log:info("  "..k..": "..v);
 		local r,m = settings.set(k, v)
 		
-		if r then response:addData(k, "ok")
-		else response:addData(k, "could not set key ('" .. m .. "')")
+		if r then 
+			response:addData(k, "ok")
+		else 
+			response:addData(k, "could not save setting ('" .. m .. "')")
+			log:info("  m: "..utils.dump(m))
 		end
 	end
 	
diff --git a/src/util/settings.lua b/src/util/settings.lua
index 5d264d6..4ee1725 100644
--- a/src/util/settings.lua
+++ b/src/util/settings.lua
@@ -13,6 +13,8 @@
 local uci = require('uci').cursor()
 local utils = require('util.utils')
 local baseconfig = require('conf_defaults')
+local utils = require('util.utils')
+local log = require('util.logger')
 
 local M = {}
 
@@ -107,18 +109,25 @@ local function isValid(value, baseTable)
 		
 	elseif varType == 'int' or varType == 'float' then
 		local numValue = tonumber(value)
-		local ok = numValue and true or false
-		ok = ok and (varType == 'float' or math.floor(numValue) == numValue)
-		if min then ok = ok and numValue >= min end
-		if max then ok = ok and numValue <= max end
-		return ok or nil,"invalid int/float value or out of range"
+		if numValue == nil then
+			return nil, "invalid number"
+		elseif varType == 'int' and math.floor(numValue) ~= numValue then
+			return nil, "invalid int"
+		elseif min and numValue < min then
+			return nil, "to low"
+		elseif max and numValue > max then
+			return nil, "to high"
+		end
 		
 	elseif varType == 'string' then
 		local ok = true
-		if min then ok = ok and value:len() >= min end
-		if max then ok = ok and value:len() <= max end
-		if regex then ok = ok and value:match(regex) ~= nil end
-		return ok or nil,"invalid string value"
+		if min and value:len() < min then
+			return nil,"to short"
+		elseif max and value:len() > max then
+			return nil,"to long"
+		elseif regex and value:match(regex) == nil then
+			return nil,"invalid value"
+		end 
 	end
 
 	return true
@@ -191,6 +200,7 @@ end
 -- @treturn bool|nil True if everything went well, nil in case of error.
 -- @treturn ?string Error message in case first return value is nil (invalid key).
 function M.set(key, value)
+	--log:info("settings:set: "..utils.dump(key))
 	key = replaceDots(key)
 
 	local r = utils.create(UCI_CONFIG_FILE)
@@ -198,11 +208,11 @@ function M.set(key, value)
 	
 	local base = getBaseKeyTable(key)
 	if not base then return nil,ERR_NO_SUCH_KEY end
-
+	
 	if M.isDefault(key) and value == nil then return true end -- key is default already
-	
+	--log:info("  not default")
 	local current = uci:get(UCI_CONFIG_NAME, UCI_CONFIG_SECTION, key)
-	
+	--log:info("  base.type: "..utils.dump(base.type))
 	if base.type == 'bool' then
 		if value ~= "" then
 			value = utils.toboolean(value)
@@ -216,15 +226,16 @@ function M.set(key, value)
 		end
 	end
 
+
+	local valid,m = isValid(value, base)
+	if not valid then
+		return nil,m
+	end
+		
 	if fromUciValue(current, base.type) == value then return true end
 
 	if value ~= nil then
-		local valid,m = isValid(value, base)
-		if (valid) then
-			uci:set(UCI_CONFIG_NAME, UCI_CONFIG_SECTION, key, toUciValue(value, base.type))
-		else
-			return nil,m
-		end
+		uci:set(UCI_CONFIG_NAME, UCI_CONFIG_SECTION, key, toUciValue(value, base.type))
 	else
 		uci:delete(UCI_CONFIG_NAME, UCI_CONFIG_SECTION, key)
 	end