From f272cb4c51cb2bb3752269faf431bcf4bfbc0686 Mon Sep 17 00:00:00 2001 From: Kaarle Ritvanen Date: Thu, 7 Mar 2013 07:14:03 +0000 Subject: forward relevant error messages to client --- acf/error.lua | 97 +++++++++++++++++++++ acf/init.lua | 3 +- acf/model/field.lua | 26 ++++-- acf/model/init.lua | 26 +++--- acf/model/model.lua | 6 +- acf/model/node.lua | 13 ++- acf/modules/awall.lua | 4 +- acf/persistence/init.lua | 5 +- acf/transaction/backend.lua | 11 ++- acf/transaction/init.lua | 7 +- acf/util.lua | 7 +- protocol.txt | 19 +++++ server.lua | 200 +++++++++++++++++++++++++------------------- 13 files changed, 305 insertions(+), 119 deletions(-) create mode 100644 acf/error.lua diff --git a/acf/error.lua b/acf/error.lua new file mode 100644 index 0000000..26dfb86 --- /dev/null +++ b/acf/error.lua @@ -0,0 +1,97 @@ +--[[ +Copyright (c) 2012-2013 Kaarle Ritvanen +See LICENSE file for license details +--]] + +module(..., package.seeall) + +local object = require('acf.object') +local class = object.class + +local util = require('acf.util') + +require 'json' + + +local function pack(...) return arg end + + +local ErrorTable = class() + +function ErrorTable:init() self.errors = {} end + +function ErrorTable:success() return not next(self.errors) end + +function ErrorTable:raise() + if not self:success() then error(json.encode(self.errors)) end +end + + +ErrorList = class(ErrorTable) + +function ErrorList:init(label) + object.super(self, ErrorList):init() + self.label = label +end + +function ErrorList:insert(msg) + table.insert(util.setdefault(self.errors, self.label, {}), msg) +end + + +ErrorDict = class(ErrorTable) + +function ErrorDict:collect(func, ...) + local function pack(success, ...) + return success, success and arg or arg[1] + end + + local success, res = pack( + xpcall( + function() return func(unpack(arg)) end, + function(err) + local _, _, data = string.find(err, '.-: (.+)') + local success, res = pcall(json.decode, data) + if success and type(res) == 'table' then return res end + return data..'\n'..debug.traceback() + end + ) + ) + + if success then return unpack(res) end + + if type(res) == 'table' then + for label, errors in pairs(res) do + for _, err in ipairs(errors) do + table.insert(util.setdefault(self.errors, label, {}), err) + end + end + else error(res) end +end + + +function raise(label, msg) + local err = ErrorList(label) + err:insert(msg) + err:raise() +end + +function relabel(label, ...) + local err = ErrorDict() + local res = pack(err:collect(unpack(arg))) + if err:success() then return unpack(res) end + + elist = ErrorList(label) + for lbl, el in pairs(err.errors) do + for _, e in ipairs(el) do elist:insert(lbl..': '..e) end + end + elist:raise() +end + +function call(...) + local err = ErrorDict() + local res = pack(err:collect(unpack(arg))) + if err:success() then return true, unpack(res) end + if err.errors.system then error(err.errors.system[1]) end + return false, err.errors +end diff --git a/acf/init.lua b/acf/init.lua index 3358571..8f89c08 100644 --- a/acf/init.lua +++ b/acf/init.lua @@ -1,5 +1,5 @@ --[[ -Copyright (c) 2012 Kaarle Ritvanen +Copyright (c) 2012-2013 Kaarle Ritvanen See LICENSE file for license details --]] @@ -8,6 +8,7 @@ module(..., package.seeall) require('acf.util').loadmods('modules') require 'acf.model' +call = require('acf.error').call require 'acf.object' require 'acf.path' require 'acf.transaction' diff --git a/acf/model/field.lua b/acf/model/field.lua index d2e62a5..e1bdeaf 100644 --- a/acf/model/field.lua +++ b/acf/model/field.lua @@ -1,10 +1,13 @@ --[[ -Copyright (c) 2012 Kaarle Ritvanen +Copyright (c) 2012-2013 Kaarle Ritvanen See LICENSE file for license details --]] module(..., package.seeall) +local err = require('acf.error') +local raise = err.raise + local node = require('acf.model.node') local object = require('acf.object') @@ -63,10 +66,10 @@ end function Field:_validate(txn, path, value) if self.required and value == nil then - error('Required value not set: '..path) + raise(path, 'Required value not set') end if self.choice and value ~= nil and not contains(self.choice, value) then - error('Invalid value for '..path..': '..value) + raise(path, 'Invalid value') end if value ~= nil then self:validate(txn, path, value) end return value @@ -92,16 +95,27 @@ function TreeNode:save(txn, path, addr, value) if object.isinstance(value, node.TreeNode) then -- TODO clone if TreeNode has wrong path - assert(node.path(value) == path) + if node.path(value) ~= path then + raise(path, 'Attempted to assign foreign object as value') + end return end txn:set(addr) + if value then - assert(type(value) == 'table') + if type(value) ~= 'table' then + raise(path, 'Cannot assign primitive value') + end + txn:set(addr, 'table') local new = self:load(txn, path, addr, true) - for k, v in pairs(value) do new[k] = v end + + local errors = err.ErrorDict() + for k, v in pairs(value) do + errors:collect(function() new[k] = v end) + end + errors:raise() end end diff --git a/acf/model/init.lua b/acf/model/init.lua index f333886..44b1f5e 100644 --- a/acf/model/init.lua +++ b/acf/model/init.lua @@ -5,6 +5,10 @@ See LICENSE file for license details module(..., package.seeall) +error = require('acf.error') +local raise = error.raise +local relabel = error.relabel + local fld = require('acf.model.field') local Field = fld.Field @@ -34,7 +38,7 @@ local Primitive = class(Field) function Primitive:validate(txn, path, value) local t = self.dtype - if type(value) ~= t then error('Not a '..t..': '..tostring(value)) end + if type(value) ~= t then raise(path, 'Not a '..t) end end @@ -48,7 +52,7 @@ end function String:validate(txn, path, value) super(self, String):validate(txn, path, value) if self['max-length'] and string.len(value) > self['max-length'] then - error('Maximum length exceeded: '..value) + raise(path, 'Maximum length exceeded') end end @@ -79,7 +83,7 @@ Integer = class(Number) function Integer:validate(txn, path, value) super(self, Integer):validate(txn, path, value) - if math.floor(value) ~= value then error('Not an integer: '..value) end + if math.floor(value) ~= value then raise(path, 'Not an integer') end end @@ -101,7 +105,7 @@ end function Range:validate(txn, path, value) local comps = stringy.split(value, '-') - if #comps > 2 then error('Invalid range') end + if #comps > 2 then raise(path, 'Invalid range') end for _, v in ipairs(comps) do to_field(self.type):_validate(txn, path, v) end @@ -124,8 +128,9 @@ function Reference:meta(txn, path, addr) local res = super(self, Reference):meta(txn, path, addr) res.scope = self:abs_scope(path) - local base = txn:search(res.scope) - local objs = base and txn:get(getmetatable(base).addr) or {} + local objs = txn:get( + getmetatable(relabel('system', txn.search, txn, res.scope)).addr + ) or {} res.choice = map(function(p) return pth.join(res.scope, p) end, objs) res['ui-choice'] = objs @@ -153,17 +158,18 @@ function Reference:_validate(txn, path, value) local scope = self:abs_scope(path) local prefix = scope..'/' if not stringy.startswith(value, prefix) then - error('Reference out of scope ('..scope..')') + raise(path, 'Reference out of scope ('..scope..')') end value = string.sub(value, string.len(prefix) + 1, -1) end -- assume one-level ref for now - assert(not string.find(value, '/')) - if not self:follow(txn, path, value) then - error('Does not exist: '..value) + if string.find(value, '/') then + raise(path, 'Subtree references not yet supported') end + -- TODO check instance type + relabel(path, self.follow, self, txn, path, value) return value end diff --git a/acf/model/model.lua b/acf/model/model.lua index 2efcb6f..6186d04 100644 --- a/acf/model/model.lua +++ b/acf/model/model.lua @@ -1,10 +1,12 @@ --[[ -Copyright (c) 2012 Kaarle Ritvanen +Copyright (c) 2012-2013 Kaarle Ritvanen See LICENSE file for license details --]] module(..., package.seeall) +local raise = require('acf.error').raise + local fld = require('acf.model.field') local Field = fld.Field @@ -88,7 +90,7 @@ function Model:init(txn, path, addr) function mt.__newindex(t, k, v) local f = mt.field(k) - if not f then error('Field named '..k..' does not exist') end + if not f then raise(mt.path, 'Field named '..k..' does not exist') end f:save(v) txn.validate[mt.path] = function() self:validate() end end diff --git a/acf/model/node.lua b/acf/model/node.lua index 083757c..3459fce 100644 --- a/acf/model/node.lua +++ b/acf/model/node.lua @@ -5,6 +5,7 @@ See LICENSE file for license details module(..., package.seeall) +local raise = require('acf.error').raise local object = require('acf.object') local class = object.class local super = object.super @@ -51,9 +52,13 @@ end function TreeNode:search(path) if #path == 0 then return self end - local next = path[1] + local name = path[1] + local next = self[name] + if next == nil then + raise(getmetatable(self).path, 'Subordinate does not exist: '..name) + end table.remove(path, 1) - return TreeNode.search(self[next], path) + return TreeNode.search(next, path) end @@ -65,7 +70,7 @@ function Collection:init(txn, path, addr, field, required) if required then txn.validate[path] = function() if #txn:get(addr) == 0 then - error('Collection cannot be empty: '..path) + raise(path, 'Collection cannot be empty') end end end @@ -100,7 +105,7 @@ function PrimitiveList:init(txn, path, addr, field, required) assert(i == tonumber(j)) if mt.field:load(i) == k then return k end end - error('Value does not exist: '..k) + raise(path, 'Value does not exist: '..k) end end diff --git a/acf/modules/awall.lua b/acf/modules/awall.lua index 0b73f9c..dbb335a 100644 --- a/acf/modules/awall.lua +++ b/acf/modules/awall.lua @@ -21,14 +21,14 @@ function IPv4Addr:validate(txn, path, value) end end if test(string.match(value, '(%d+)%.(%d+)%.(%d+)%.(%d+)')) then - error('Invalid IP address: '..value) + M.error.raise(path, 'Invalid IP address') end end Port = class(M.Integer) function Port:validate(txn, path, value) super(self, Port):validate(txn, path, value) - if value < 0 or value > 65535 then error('Invalid port: '..value) end + if value < 0 or value > 65535 then M.error.raise(path, 'Invalid port') end end PortRange = class(M.Range) diff --git a/acf/persistence/init.lua b/acf/persistence/init.lua index f674e9c..2533560 100644 --- a/acf/persistence/init.lua +++ b/acf/persistence/init.lua @@ -1,5 +1,5 @@ --[[ -Copyright (c) 2012 Kaarle Ritvanen +Copyright (c) 2012-2013 Kaarle Ritvanen See LICENSE file for license details --]] @@ -39,8 +39,7 @@ function DataStore:set_multiple(mods) for _, mod in ipairs(mods) do local path, t, value = unpack(mod) local backend, comps = self:split_path(path) - if not bms[backend] then bms[backend] = {} end - table.insert(bms[backend], {comps, t, value}) + table.insert(util.setdefault(bms, backend, {}), {comps, t, value}) end for backend, bm in pairs(bms) do backend:set(bm) end diff --git a/acf/transaction/backend.lua b/acf/transaction/backend.lua index 5b8c53f..e907c6f 100644 --- a/acf/transaction/backend.lua +++ b/acf/transaction/backend.lua @@ -1,10 +1,12 @@ --[[ -Copyright (c) 2012 Kaarle Ritvanen +Copyright (c) 2012-2013 Kaarle Ritvanen See LICENSE file for license details --]] module(..., package.seeall) +local err = require('acf.error') + -- TODO each transaction backend (i.e. persistence manager or -- transaction proper) should be implemented as a thread or have its -- internal state stored in shared storage (with appropriate locking) @@ -23,7 +25,7 @@ function TransactionBackend:init() self.mod_time = {} end function TransactionBackend:get_if_older(path, timestamp) local value, ts = self:get(path) - if ts > timestamp then error('Concurrent modification: '..path) end + if ts > timestamp then err.raise('conflict', path) end return value, ts end @@ -41,8 +43,11 @@ end -- TODO should be atomic, mutex with set_multiple function TransactionBackend:comp_and_setm(accessed, mods) + local errors = err.ErrorDict() for path, timestamp in pairs(accessed) do - self:get_if_older(path, timestamp) + errors:collect(self.get_if_older, self, path, timestamp) end + errors:raise() + self:set_multiple(mods) end diff --git a/acf/transaction/init.lua b/acf/transaction/init.lua index 7f2f6b9..99b8b9d 100644 --- a/acf/transaction/init.lua +++ b/acf/transaction/init.lua @@ -1,10 +1,11 @@ --[[ -Copyright (c) 2012 Kaarle Ritvanen +Copyright (c) 2012-2013 Kaarle Ritvanen See LICENSE file for license details --]] module(..., package.seeall) +local ErrorDict = require('acf.error').ErrorDict local RootModel = require('acf.model').RootModel local object = require('acf.object') local super = object.super @@ -121,7 +122,9 @@ function Transaction:search(path) return self.root:search(pth.split(path)) end function Transaction:commit() self:check() - for _, func in pairs(self.validate) do func() end + local errors = ErrorDict() + for path, func in pairs(self.validate) do errors:collect(func) end + errors:raise() local mods = {} local handled = {} diff --git a/acf/util.lua b/acf/util.lua index cbea072..9788c81 100644 --- a/acf/util.lua +++ b/acf/util.lua @@ -1,10 +1,15 @@ --[[ -Copyright (c) 2012 Kaarle Ritvanen +Copyright (c) 2012-2013 Kaarle Ritvanen See LICENSE file for license details --]] module(..., package.seeall) +function setdefault(t, k, v) + if t[k] == nil then t[k] = v end + return t[k] +end + function setdefaults(dst, src) for k, v in pairs(src) do if dst[k] == nil then dst[k] = v end end return dst diff --git a/protocol.txt b/protocol.txt index 018006a..0d699f3 100644 --- a/protocol.txt +++ b/protocol.txt @@ -75,3 +75,22 @@ req: POST /config// resp: action-specific JSON - for time-consuming actions, can return multiple JSON documents, each containing a status update + + +Use of HTTP status codes: + +2xx Success + +400 Bad request + resp: plain text error message + +401 Authentication error + +409 Concurrent modification + resp: JSON list of objects modified by a concurrent transaction + +422 Semantic error + resp: JSON object mapping paths to object-specific lists of + validation error messages + +500 Server error diff --git a/server.lua b/server.lua index c6fe586..cbbc82f 100644 --- a/server.lua +++ b/server.lua @@ -10,50 +10,6 @@ require 'json' require 'stringy' -local function handle(txn, method, path, data) - local parent, name - if path ~= '/' then - parent = txn:search(acf.path.parent(path)) - name = acf.path.name(path) - end - - if method == 'GET' then - local obj = txn:search(path) - if obj == nil then return 404 end - - local res - - if isinstance(obj, acf.model.node.TreeNode) then - local node = {} - for _, k in ipairs(acf.model.node.members(obj)) do - local v = obj[k] - if isinstance(v, acf.model.node.TreeNode) then - v = acf.model.node.path(v) - end - node[k] = v - end - res = {data=node, meta=acf.model.node.meta(obj)} - - else res = {data=obj, meta=acf.model.node.mmeta(parent, name)} end - - local function f() coroutine.yield(json.encode(res)) end - return 200, {['Content-Type']='application/json'}, coroutine.wrap(f) - end - - if method == 'DELETE' then - parent[name] = nil - - elseif method == 'PUT' then - parent[name] = data - - -- TODO implement POST for invoking object-specific actions - - else return 405 end - - return 200 -end - - -- TODO shared storage for login sessions local last_sid = 0 local sessions = {} @@ -70,23 +26,50 @@ return function(env) local method = env.REQUEST_METHOD local path = env.REQUEST_URI + local function wrap(code, headers, res) + if not headers then headers = {} end + + local ctype + if type(res) == 'table' then + ctype = 'application/json' + res = json.encode(res) + else + ctype = 'text/plain' + if not res then res = '' end + end + headers['Content-Type'] = ctype + + return code, headers, coroutine.wrap( + function() coroutine.yield(res) end + ) + end + local data if env.CONTENT_LENGTH then - data = json.decode(env.input:read(env.CONTENT_LENGTH)) + local success + success, data = pcall( + json.decode, + env.input:read(env.CONTENT_LENGTH) + ) + if not success then + return wrap(400, nil, 'Request not in JSON format') + end end local sid = tonumber(env.HTTP_X_ACF_AUTH_TOKEN) local user, txn_id if sid then user = sessions[sid] - if not user then return 401 end + if not user then return wrap(401) end txn_id = tonumber(env.HTTP_X_ACF_TRANSACTION_ID) end local txn if txn_id then txn = txns[txn_id] - if not txn then return 404 end + if not txn then + return wrap(400, nil, 'Invalid transaction ID') + end else txn = acf.transaction.start() end local function fetch_user(name) @@ -94,66 +77,113 @@ return function(env) end if user then fetch_user(user) - if not user then return 401 end + if not user then return wrap(401) end end if path == '/login' then if method == 'POST' then - if not data.username or not data.password then return 401 end + if not data.username or not data.password then + return wrap(401) + end fetch_user(data.username) if user and user:check_password(data.password) then last_sid = last_sid + 1 local sid = last_sid sessions[sid] = data.username - return 200, {['X-ACF-Auth-Token']=sid} + return wrap(204, {['X-ACF-Auth-Token']=sid}) end - return 401 + return wrap(401) end - if not user then return 401 end + if not user then return wrap(401) end if method == 'DELETE' then sessions[sid] = nil - return 200 + return wrap(204) end - return 405 - end - - if not user then return 401 end - - if stringy.startswith(path, '/config/') then - -- TODO catch and forward relevant errors to the client - local code, hdr, body = handle(txn, - method, - string.sub(path, 8, -1), - data) - if not txn_id and method ~= 'GET' and code == 200 then - txn:commit() - end - return code, hdr, body + return wrap(405) end - if path == '/' then - if method == 'GET' then return 301, {['Location']='/browser/'} end + if not user then return wrap(401) end + + local success, code, hdr, res = acf.call( + function() + if stringy.startswith(path, '/config/') then + path = string.sub(path, 8, -1) + + local parent, name + if path ~= '/' then + parent = txn:search(acf.path.parent(path)) + name = acf.path.name(path) + end + + if method == 'GET' then + local obj = txn:search(path) + if obj == nil then return 404 end + + local res + + if isinstance(obj, acf.model.node.TreeNode) then + local node = {} + for _, k in ipairs(acf.model.node.members(obj)) do + local v = obj[k] + if isinstance(v, acf.model.node.TreeNode) then + v = acf.model.node.path(v) + end + node[k] = v + end + res = {data=node, meta=acf.model.node.meta(obj)} + + else + res = { + data=obj, + meta=acf.model.node.mmeta(parent, name) + } + end + + return 200, nil, res + end + + -- TODO implement POST for invoking object-specific actions + if method == 'DELETE' then parent[name] = nil + elseif method == 'PUT' then parent[name] = data + else return 405 end + + if not txn_id then txn:commit() end + return 205 + end - if not ({DELETE=true, POST=true})[method] then - return 405 - end + if path == '/' then + if method == 'GET' then + return 301, {['Location']='/browser/'} + end + + if not ({DELETE=true, POST=true})[method] then + return 405 + end + + if txn_id then + if method == 'POST' then txn:commit() end + txns[txn_id] = nil + return 204 + end + + if method == 'DELETE' then return 405 end + + last_txn_id = last_txn_id + 1 + local txn_id = last_txn_id + txns[txn_id] = txn + return 204, {['X-ACF-Transaction-ID']=txn_id} + end - if txn_id then - if method == 'POST' then txn:commit() end - txns[txn_id] = nil - return 200 + return 404 end + ) - if method == 'DELETE' then return 405 end - - last_txn_id = last_txn_id + 1 - local txn_id = last_txn_id - txns[txn_id] = txn - return 200, {['X-ACF-Transaction-ID']=txn_id} - end + if success then return wrap(code, hdr, res) end - return 404 + if code.conflict then return wrap(409, nil, code.conflict) end + + return wrap(422, nil, code) end -- cgit v1.2.3