From 1ac3f130ffc440dc37d6ce0b22cab015ec03150e Mon Sep 17 00:00:00 2001 From: Wouter R Date: Wed, 10 Jul 2013 00:32:43 +0200 Subject: [PATCH] Fix response sending when init() fails; create config.lua to contain global configuration; improve API function resolution; improve API code structure. --- TODO.md | 20 +++-- src/config.lua | 10 +++ src/main.lua | 24 ++--- src/rest/api/api_network.lua | 136 +++++++++++++--------------- src/rest/api/api_test.lua | 43 ++++----- src/rest/request.lua | 167 +++++++++++++++++++++-------------- src/rest/response.lua | 13 +-- 7 files changed, 216 insertions(+), 197 deletions(-) create mode 100644 src/config.lua diff --git a/TODO.md b/TODO.md index 868daa0..dc8fdbd 100644 --- a/TODO.md +++ b/TODO.md @@ -1,10 +1,16 @@ # TODO (new functionality) - - save requested mod+func in request, as well as resolved function/pretty print version/function pointer (or subset…); then fix endpoint function name (when called with blank argument) in response objects to show pretty print name - fix init script handling as described here: http://wiki.openwrt.org/doc/devel/packages#packaging.a.service - - write a simple client script to autotest as much of the api as possible - extend the client script to run arbitrary post/get requests - - document REST API (mention rq IDs and endpoint information, list endpoints+args+CRUD type, unknown values are empty fields) - (describe fail/error difference: fail is valid rq..could not comply, while error is invalid rq _or_ system error) + - implement (automated) test code where possible + * in 'test' dir next to 'src', with API tests under 'test/www/' + * www tests check functionality of the test module + * www tests also provide an interface to run arbitrary get/post requests + - document REST API + * fail/error difference: fail is a valid rq aka 'could not comply', while error is invalid rq _or_ system error + * modules/functions prefixed with '_' are for internal use + * rq IDs and endpoint information can be supplied (but it's probably not useful after all) + * list endpoints+args+CRUD type + * success/fail/error statuses are justified by drupal api + * unknown values (e.g. in network info) are either empty or unmentioned fields - use a slightly more descriptive success/error definition (e.g. errortype=system/missing-arg/generic) - steps to take regarding versioning/updating * versioning scheme @@ -17,7 +23,9 @@ * expose through info API and/or system API; also provide a way (future) to flash a new image - dynamic AP name based on partial MAC (set once on installation and then only upon explicit request? (e.g. api/config/wifiname/default)) - require api functions which change state to be invoked as post request - - add API functions to test network connectivity in steps (ifup? hasip? resolve? ping?) to network or test + * can this be modelled like java annotations or c function attributes? + * otherwise maybe pair each function with _attribs = {…}? + - add API functions to test network connectivity in steps (any chance(e.g. ~ap)? ifup? hasip? resolve? ping?) to network or test - add more config options to package, which should act as defaults for a config file on the system; candidates: reconf.WWW_RENAME_NAME, wifihelper.{AP_ADDRESS, AP_NETMASK, (NET)} diff --git a/src/config.lua b/src/config.lua new file mode 100644 index 0000000..a9cdc28 --- /dev/null +++ b/src/config.lua @@ -0,0 +1,10 @@ +local M = {} + +--NOTE: pcall protects from invocation exceptions, which is what we need except +--during debugging. This flag replaces them with a normal call so we can inspect stack traces. +M.DEBUG_PCALLS = true + +--REST responses will contain 'module' and 'function' keys describing what was requested +M.API_INCLUDE_ENDPOINT_INFO = true + +return M diff --git a/src/main.lua b/src/main.lua index 5637107..cac9897 100644 --- a/src/main.lua +++ b/src/main.lua @@ -3,12 +3,7 @@ local RequestClass = require("rest.request") local ResponseClass = require("rest.response") local wifi = require("network.wlanconfig") local netconf = require("network.netconfig") - - ---NOTE: pcall protects from invocation exceptions, which is what we need except ---during debugging. This flag replaces them with a normal call so we can inspect stack traces. -local DEBUG_PCALLS = true - +local config = require("config") local postData = nil @@ -21,7 +16,7 @@ local function init() l:init(l.LEVEL.debug) l:setStream(io.stderr) - if DEBUG_PCALLS then l:info("Wifibox CGI handler started (pcall debugging enabled)") + if config.DEBUG_PCALLS then l:info("Wifibox CGI handler started (pcall debugging enabled)") else l:info("Wifibox CGI handler started") end @@ -41,7 +36,7 @@ local function init() end local function main() - local rq = RequestClass.new(postData, DEBUG_PCALLS) -- initializes itself using various environment variables and the arg array + local rq = RequestClass.new(postData, config.DEBUG_PCALLS) l:info("received request of type " .. rq:getRequestMethod() .. " with arguments: " .. l:dump(rq:getAll())) if rq:getRequestMethod() ~= "CMDLINE" then @@ -49,7 +44,7 @@ end l:debug("user agent: " .. rq:getUserAgent()) end - if (not DEBUG_PCALLS and rq:getRequestMethod() == "CMDLINE") then + if (not config.DEBUG_PCALLS and rq:getRequestMethod() == "CMDLINE") then if rq:get("autowifi") ~= nil then setupAutoWifiMode() else @@ -65,12 +60,17 @@ end end end +---'entry point'--- local s, msg = init() if s == false then local resp = ResponseClass.new() - resp:setError("initialization failed (" .. msg .. ")") - resp:send() --FIXME: this message does not seem to be sent - l:error("initialization failed (" .. msg .. ")") --NOTE: this assumes the logger has been inited properly, despite init() having failed + local errSuffix = msg and " (" .. msg .. ")" or "" + + resp:setError("initialization failed" .. errSuffix) + io.write ("Content-type: text/plain\r\n\r\n") + resp:send() + l:error("initialization failed" .. errSuffix) --NOTE: this assumes the logger has been inited properly, despite init() having failed + os.exit(1) else main() diff --git a/src/rest/api/api_network.lua b/src/rest/api/api_network.lua index 778c743..5360a3f 100644 --- a/src/rest/api/api_network.lua +++ b/src/rest/api/api_network.lua @@ -8,21 +8,20 @@ local M = {} M.isApi = true -function M._global(d) - local r = ResponseClass.new(d) - r:setError("not implemented") - return r +function M._global(request, response) + response:setError("not implemented") end --accepts API argument 'nofilter'(bool) to disable filtering of APs and 'self' -function M.available(d) - local r = ResponseClass.new(d) - local noFilter = u.toboolean(d:get("nofilter")) +--accepts with_raw(bool) to include raw table dump +function M.available(request, response) + local noFilter = u.toboolean(request:get("nofilter")) + local withRaw = u.toboolean(request:get("with_raw")) local sr = wifi.getScanInfo() local si, se if sr and #sr > 0 then - r:setSuccess("") + response:setSuccess("") local netInfoList = {} for _, se in ipairs(sr) do if noFilter or se.mode ~= "ap" and se.ssid ~= wifi.AP_SSID then @@ -36,26 +35,25 @@ function M.available(d) netInfo["signal"] = se.signal netInfo["quality"] = se.quality netInfo["quality_max"] = se.quality_max - --netInfo["raw"] = l:dump(se) --TEMP for debugging only + if withRaw then netInfo["_raw"] = l:dump(se) end table.insert(netInfoList, netInfo) end end - r:addData("count", #netInfoList) - r:addData("networks", netInfoList) + response:addData("count", #netInfoList) + response:addData("networks", netInfoList) else - r:setFail("No scan results or scanning not possible") + response:setFail("No scan results or scanning not possible") end - - return r end --accepts API argument 'nofilter'(bool) to disable filtering of APs and 'self' -function M.known(d) - local r = ResponseClass.new(d) - local noFilter = u.toboolean(d:get("nofilter")) +--accepts with_raw(bool) to include raw table dump +function M.known(request, response) + local noFilter = u.toboolean(request:get("nofilter")) + local withRaw = u.toboolean(request:get("with_raw")) - r:setSuccess() + response:setSuccess() local netInfoList = {} for _, net in ipairs(wifi.getConfigs()) do if noFilter or net.mode == "sta" then @@ -64,47 +62,43 @@ function M.known(d) netInfo["bssid"] = net.bssid or "" netInfo["channel"] = net.channel or "" netInfo["encryption"] = net.encryption - --netInfo["raw"] = l:dump(net) --TEMP for debugging only + if withRaw then netInfo["_raw"] = l:dump(net) end table.insert(netInfoList, netInfo) end end - r:addData("count", #netInfoList) - r:addData("networks", netInfoList) - - return r + response:addData("count", #netInfoList) + response:addData("networks", netInfoList) end -function M.state(d) - local r = ResponseClass.new(d) +--accepts with_raw(bool) to include raw table dump +function M.state(request, response) + local withRaw = u.toboolean(request:get("with_raw")) local ds = wifi.getDeviceState() - r:setSuccess() - r:addData("ssid", ds.ssid or "") - r:addData("bssid", ds.bssid or "") - r:addData("channel", ds.channel or "") - r:addData("mode", ds.mode) - r:addData("encryption", ds.encryption) - r:addData("quality", ds.quality) - r:addData("quality_max", ds.quality_max) - r:addData("txpower", ds.txpower) - r:addData("signal", ds.signal) - r:addData("noise", ds.noise) - --r:addData("raw", l:dump(ds)) --TEMP for debugging only - - return r + response:setSuccess() + response:addData("ssid", ds.ssid or "") + response:addData("bssid", ds.bssid or "") + response:addData("channel", ds.channel or "") + response:addData("mode", ds.mode) + response:addData("encryption", ds.encryption) + response:addData("quality", ds.quality) + response:addData("quality_max", ds.quality_max) + response:addData("txpower", ds.txpower) + response:addData("signal", ds.signal) + response:addData("noise", ds.noise) + if withRaw then response:addData("_raw", l:dump(ds)) end end --UNTESTED --requires ssid(string), accepts phrase(string), recreate(bool) -function M.assoc(d) - local r = ResponseClass.new(d) - local argSsid = d:get("ssid") - local argPhrase = d:get("phrase") - local argRecreate = d:get("recreate") +function M.assoc(request, response) + local argSsid = request:get("ssid") + local argPhrase = request:get("phrase") + local argRecreate = request:get("recreate") if argSsid == nil or argSsid == "" then - r:setError("missing ssid argument") - return r + response:setError("missing ssid argument") + return end local cfg = nil @@ -121,65 +115,53 @@ function M.assoc(d) wifi.createConfigFromScanInfo(scanResult, argPhrase) else --check for error - r:setFail("no wireless network with requested SSID is available") - r:addData("ssid", argSsid) + response:setFail("no wireless network with requested SSID is available") + response:addData("ssid", argSsid) + return end end wifi.activateConfig(argSsid) netconf.switchConfiguration{ wifiiface="add", apnet="rm", staticaddr="rm", dhcppool="rm", wwwredir="rm", dnsredir="rm", wwwcaptive="rm", wireless="reload" } - r:setSuccess("wlan associated") - r:addData("ssid", argSsid) - - return r + response:setSuccess("wlan associated") + response:addData("ssid", argSsid) end --UNTESTED -function M.disassoc(d) - local r = ResponseClass.new(d) - +function M.disassoc(request, response) wifi.activateConfig() local rv = wifi.restart() - r:setSuccess("all wireless networks deactivated") - r:addData("wifi_restart_result", rv) - - return r + response:setSuccess("all wireless networks deactivated") + response:addData("wifi_restart_result", rv) end --UNTESTED -function M.openap(d) - local r = ResponseClass.new(d) - +function M.openap(request, response) --add AP net, activate it, deactivate all others, reload network/wireless config, add all dhcp and captive settings and reload as needed netconf.switchConfiguration{apnet="add_noreload"} wifi.activateConfig(wifi.AP_SSID) netconf.switchConfiguration{ wifiiface="add", network="reload", staticaddr="add", dhcppool="add", wwwredir="add", dnsredir="add", wwwcaptive="add" } - r:setSuccess("switched to Access Point mode") - r:addData("ssid", wifi.AP_SSID) - - return r + response:setSuccess("switched to Access Point mode") + response:addData("ssid", wifi.AP_SSID) end --UNTESTED --requires ssid(string) -function M.rm(d) - local r = ResponseClass.new(d) - local argSsid = d:get("ssid") +function M.rm(request, response) + local argSsid = request:get("ssid") if argSsid == nil or argSsid == "" then - r:setError("missing ssid argument") - return r + response:setError("missing ssid argument") + return end if wifi.removeConfig(argSsid) then - r:setSuccess("removed wireless network with requested SSID") - r:addData("ssid", argSsid) + response:setSuccess("removed wireless network with requested SSID") + response:addData("ssid", argSsid) else - r:setFail("no wireless network with requested SSID") --this used to be a warning instead of an error... - r:addData("ssid", argSsid) + response:setFail("no wireless network with requested SSID") --this used to be a warning instead of an error... + response:addData("ssid", argSsid) end - - return r end return M diff --git a/src/rest/api/api_test.lua b/src/rest/api/api_test.lua index 242bff0..840e49c 100644 --- a/src/rest/api/api_test.lua +++ b/src/rest/api/api_test.lua @@ -5,42 +5,31 @@ local M = {} M.isApi = true -function M._global(d) - local r = ResponseClass.new(d) - local ba = d:getBlankArgument() +function M._global(request, response) + local ba = request:getBlankArgument() - r:setSuccess("REST test API - default function called with blank argument: '" .. (ba or "") .. "'") - if ba ~= nil then r:addData("blank_argument", ba) end - - return r + response:setSuccess("REST test API - default function called with blank argument: '" .. (ba or "") .. "'") + if ba ~= nil then response:addData("blank_argument", ba) end end -function M.success(d) - local r = ResponseClass.new(d) - r:setSuccess("this successful response has been generated on purpose") - r:addData("url", "http://xkcd.com/349/") - return r +function M.success(request, response) + response:setSuccess("this successful response has been generated on purpose") + response:addData("url", "http://xkcd.com/349/") end -function M.fail(d) - local r = ResponseClass.new(d) - r:setFail("this failure has been generated on purpose") - r:addData("url", "http://xkcd.com/336/") - return r +function M.fail(request, response) + response:setFail("this failure has been generated on purpose") + response:addData("url", "http://xkcd.com/336/") end -function M.error(d) - local r = ResponseClass.new(d) - r:setError("this error has been generated on purpose") - r:addData("url", "http://xkcd.com/1024/") - return r +function M.error(request, response) + response:setError("this error has been generated on purpose") + response:addData("url", "http://xkcd.com/1024/") end -function M.echo(d) - local r = ResponseClass.new(d) - r:setSuccess("request echo") - r:addData("request_data", d) - return r +function M.echo(request, response) + response:setSuccess("request echo") + response:addData("request_data", request) end return M diff --git a/src/rest/request.lua b/src/rest/request.lua index 86b2a6a..c55d0b5 100644 --- a/src/rest/request.lua +++ b/src/rest/request.lua @@ -1,4 +1,5 @@ local urlcode = require("util.urlcode") +local config = require("config") local ResponseClass = require("rest.response") local M = {} @@ -6,12 +7,24 @@ M.__index = M local GLOBAL_API_FUNCTION_NAME = "_global" + +--NOTE: requestedApi* contain what was extracted from the request data +-- regarding the other variables: either both resolvedApiFunction and realApiFunctionName +-- are nil and resolutionError is not, or exactly the other way around +M.requestedApiModule = nil +M.requestedApiFunction = nil +M.resolvedApiFunction = nil --will contain function address, or nil +M.realApiFunctionName = nil --will contain requested name, or global name, or nil +M.resolutionError = nil --non-nil means function could not be resolved + + local function kvTableFromUrlEncodedString(encodedText) local args = {} if (encodedText ~= nil) then urlcode.parsequery(encodedText, args) end return args + end local function kvTableFromArray(argArray) @@ -30,12 +43,53 @@ local function kvTableFromArray(argArray) end +--returns either a module object, or nil+errmsg +local function resolveApiModule(modname) + if modname == nil then return nil, "missing module name" end + if string.find(modname, "_") == 1 then return nil, "module names starting with '_' are preserved for internal use" end + + local reqModName = "rest.api.api_" .. modname + local ok, modObj + + if config.DEBUG_PCALLS then ok, modObj = true, require(reqModName) + else ok, modObj = pcall(require, reqModName) + end + + if ok == false then return nil, "API module does not exist" end + if modObj == nil then return nil, "API module could not be found" end + if modObj.isApi ~= true then return nil, "module is not part of the CGI API" end + + return modObj +end + +--returns funcobj+nil (usual), funcobj+number (global func with blank arg), or nil+errmsg (unresolvable or inaccessible) +local function resolveApiFunction(modname, funcname) + if funcname and string.find(funcname, "_") == 1 then return nil, "function names starting with '_' are preserved for internal use" end + + local mod, msg = resolveApiModule(modname) + + if (funcname == nil or funcname == '') then funcname = GLOBAL_API_FUNCTION_NAME end --treat empty function name as nil + local f = mod[funcname] + local funcNumber = tonumber(funcname) + + if (type(f) == "function") then + return f + elseif funcNumber ~= nil then + return mod[GLOBAL_API_FUNCTION_NAME], funcNumber + else + return nil, ("function '" .. funcname .. "' does not exist in API module '" .. modname .. "'") + end +end + + setmetatable(M, { __call = function(cls, ...) return cls.new(...) end }) +--This function initializes itself using various environment variables, the arg array and the given postData +--NOTE: if debugging is enabled, commandline arguments 'm' and 'f' override requested module and function function M.new(postData, debug) local self = setmetatable({}, M) @@ -54,30 +108,54 @@ function M.new(postData, debug) self.postArgs = kvTableFromUrlEncodedString(postData) --TEMP: until these can be extracted from the url path itself - self.apiModule = self.getArgs["m"] - self.apiFunction = self.getArgs["f"] + self.requestedApiModule = self.getArgs["m"] + self.requestedApiFunction = self.getArgs["f"] if debug then - self.apiModule = self.cmdLineArgs["m"] or self.apiModule - self.apiFunction = self.cmdLineArgs["f"] or self.apiFunction + self.requestedApiModule = self.cmdLineArgs["m"] or self.requestedApiModule + self.requestedApiFunction = self.cmdLineArgs["f"] or self.requestedApiFunction + end + + if self.requestedApiModule == "" then self.requestedApiModule = nil end + if self.requestedApiFunction == "" then self.requestedApiFunction = nil end + + + -- Perform module/function resolution + --TODO: improve naming and perhaps argument passing + local sfunc, sres = resolveApiFunction(self:getRequestedApiModule(), self:getRequestedApiFunction()) + + if sfunc ~= nil then --function (possibly the global one) could be resolved + self.resolvedApiFunction = sfunc + if sres ~= nil then --apparently it was the global one, and we received a 'blank argument' + self:setBlankArgument(sres) + self.realApiFunctionName = GLOBAL_API_FUNCTION_NAME + else --resolved without blank argument but still potentially the global function, hence the _or_ construction + self.realApiFunctionName = self:getRequestedApiFunction() or GLOBAL_API_FUNCTION_NAME + end + else + --instead of throwing an error, save the message for handle() which is expected to return a response anyway + self.resolutionError = sres end - if self.apiModule == "" then self.apiModule = nil end - if self.apiFunction == "" then self.apiFunction = nil end return self end +--GET/POST/CMDLINE function M:getRequestMethod() return self.requestMethod end -function M:getApiModule() - return self.apiModule +function M:getRequestedApiModule() + return self.requestedApiModule end -function M:getApiFunction() - return self.apiFunction +function M:getRequestedApiFunction() + return self.requestedApiFunction +end + +function M:getRealApiFunctionName() + return self.realApiFunctionName end function M:getBlankArgument() @@ -116,72 +194,29 @@ function M:getAll() end end ---returns either a module object, or nil+errmsg -local function resolveApiModule(modname) - if modname == nil then return nil, "missing module name" end - if string.find(modname, "_") == 1 then return nil, "module names starting with '_' are preserved for internal use" end - - local reqModName = "rest.api.api_" .. modname - local ok, modObj - - --TODO: create config.lua which contains DEBUG_PCALLS (nothing else for the moment) - if DEBUG_PCALLS then ok, modObj = true, require(reqModName) - else ok, modObj = pcall(require, reqModName) - end - - if ok == false then return nil, "API module does not exist" end - if modObj == nil then return nil, "API module could not be found" end - if modObj.isApi ~= true then return nil, "module is not part of the CGI API" end - - return modObj -end - ---returns funcobj+nil (usual), funcobj+number (global func with blank arg), or nil+errmsg (unresolvable or inaccessible) -local function resolveApiFunction(modname, funcname) - if funcname and string.find(funcname, "_") == 1 then return nil, "function names starting with '_' are preserved for internal use" end - - local mod, msg = resolveApiModule(modname) - - if (funcname == nil or funcname == '') then funcname = GLOBAL_API_FUNCTION_NAME end --treat empty function name as nil - local f = mod[funcname] - local funcNumber = tonumber(funcname) - - if (type(f) == "function") then - return f - elseif funcNumber ~= nil then - return mod[GLOBAL_API_FUNCTION_NAME], funcNumber - else - return nil, ("function '" .. funcname .. "' does not exist in API module '" .. modname .. "'") - end -end --returns either a response object+nil, or response object+errmsg function M:handle() - - --TEMP: should be moved to init - local mod = self:getApiModule() - local func = self:getApiFunction() - local sf, sr = resolveApiFunction(mod, func) + local modname = self:getRequestedApiModule() + local resp = ResponseClass.new(self) - local resp = ResponseClass.new(rq) --TEMP: do not do this before resolving. after resolving has been moved to init that will be automatically true - - if (sf ~= nil) then - if (sr ~= nil) then self:setBlankArgument(sr) end - + if (self.resolvedApiFunction ~= nil) then --we found a function (possible the global function) + --invoke the function local ok, r - if DEBUG_PCALLS then ok, r = true, sf(self) - else ok, r = pcall(sf, self) + if config.DEBUG_PCALLS then ok, r = true, self.resolvedApiFunction(self, resp) + else ok, r = pcall(self.resolvedApiFunction, self, resp) end - + + --handle the result if ok == true then - return r, nil + return resp, nil else - resp:setError("call to function '" .. mod .. "/" .. sr .. "' failed") - return resp, ("calling function '" .. func .. "' in API module '" .. mod .. "' somehow failed ('" .. r .. "')") + resp:setError("call to function '" .. modname .. "/" .. self.realApiFunctionName .. "' failed") + return resp, ("calling function '" .. self.realApiFunctionName .. "' in API module '" .. modname .. "' somehow failed ('" .. r .. "')") end else - resp:setError("function unknown '" .. (mod or "") .. "/" .. (func or "") .. "'") - return resp, ("could not resolve requested API function ('" .. sr .. "')") + resp:setError("function unknown '" .. (modname or "") .. "/" .. (self:getRequestedApiFunction() or "") .. "'") + return resp, ("could not resolve requested API function ('" .. self.resolutionError .. "')") end return resp diff --git a/src/rest/response.lua b/src/rest/response.lua index 0f1f564..789e1fa 100644 --- a/src/rest/response.lua +++ b/src/rest/response.lua @@ -1,10 +1,10 @@ local JSON = (loadfile "util/JSON.lua")() +local config = require("config") local M = {} M.__index = M local REQUEST_ID_ARGUMENT = "rq_id" -local INCLUDE_ENDPOINT_INFO = false setmetatable(M, { __call = function(cls, ...) @@ -22,20 +22,15 @@ function M.new(requestObject) local rqId = requestObject:get(REQUEST_ID_ARGUMENT) if rqId ~= nil then self.body[REQUEST_ID_ARGUMENT] = rqId end - if INCLUDE_ENDPOINT_INFO == true then - self.body["module"] = requestObject:getApiModule() - self.body["function"] = requestObject:getApiFunction() or "" + if config.API_INCLUDE_ENDPOINT_INFO == true then + self.body["module"] = requestObject:getRequestedApiModule() + self.body["function"] = requestObject:getRealApiFunctionName() or "" end end return self end ---use set{Success|Fail|Error}() ---function M:setStatus(s) --- self.body.status = s ---end - function M:setSuccess(msg) self.body.status = "success" if msg ~= "" then self.body.msg = msg end