From 572724b7f2612b83f8e4d37d89a690e8cb70e366 Mon Sep 17 00:00:00 2001 From: Thijs Schreijer Date: Tue, 9 Aug 2022 00:28:55 +0200 Subject: [PATCH] address review comments --- .editorconfig | 1 + mqtt/init.lua | 246 ++++++++++++++++++++++-------------------- tests/spec/topics.lua | 12 +-- 3 files changed, 134 insertions(+), 125 deletions(-) diff --git a/.editorconfig b/.editorconfig index d646b37..0709883 100644 --- a/.editorconfig +++ b/.editorconfig @@ -12,3 +12,4 @@ indent_size = 4 [Makefile] indent_style = tab +indent_size = 4 diff --git a/mqtt/init.lua b/mqtt/init.lua index fca8353..00ff9aa 100644 --- a/mqtt/init.lua +++ b/mqtt/init.lua @@ -83,23 +83,27 @@ end --- Validates a topic with wildcards. --- @param t (string) wildcard topic to validate +-- @tparam string t wildcard topic to validate -- @return topic, or false+error +-- @usage +-- local t = "invalid/#/subscribe/#/topic" +-- local topic = assert(mqtt.validate_subscribe_topic(t)) function mqtt.validate_subscribe_topic(t) if type(t) ~= "string" then - return false, "not a string" + return false, "bad subscribe-topic; expected topic to be a string, got: "..type(t) end if #t < 1 then - return false, "minimum topic length is 1" + return false, "bad subscribe-topic; expected minimum topic length of 1" end do local _, count = t:gsub("#", "") if count > 1 then - return false, "wildcard '#' may only appear once" + return false, "bad subscribe-topic; wildcard '#' may only appear once, got: '"..t.."'" end if count == 1 then if t ~= "#" and not t:find("/#$") then - return false, "wildcard '#' must be the last character, and be prefixed with '/' (unless the topic is '#')" + return false, "bad subscribe-topic; wildcard '#' must be the last character, and " .. + "be prefixed with '/' (unless the topic is '#'), got: '"..t.."'" end end end @@ -110,7 +114,8 @@ function mqtt.validate_subscribe_topic(t) i = t1:find("+", i) if i then if t1:sub(i-1, i+1) ~= "/+/" then - return false, "wildcard '+' must be enclosed between '/' (except at start/end)" + return false, "bad subscribe-topic; wildcard '+' must be enclosed between '/' " .. + "(except at start/end), got: '"..t.."'" end i = i + 1 end @@ -120,141 +125,144 @@ function mqtt.validate_subscribe_topic(t) end --- Validates a topic without wildcards. --- @param t (string) topic to validate +-- @tparam string t topic to validate -- @return topic, or false+error +-- @usage +-- local t = "invalid/#/publish/+/topic" +-- local topic = assert(mqtt.validate_publish_topic(t)) function mqtt.validate_publish_topic(t) if type(t) ~= "string" then - return false, "not a string" + return false, "bad publish-topic; expected topic to be a string, got: "..type(t) end if #t < 1 then - return false, "minimum topic length is 1" + return false, "bad publish-topic; expected minimum topic length of 1" end if t:find("+", nil, true) or t:find("#", nil, true) then - return false, "wildcards '#', and '+' are not allowed when publishing" + return false, "bad publish-topic; wildcards '#', and '+' are not allowed when publishing, got: '"..t.."'" end return t end ---- Returns a Lua pattern from topic. --- Takes a wildcarded-topic and returns a Lua pattern that can be used --- to validate if a received topic matches the wildcard-topic --- @param t (string) the wildcard topic --- @return Lua-pattern (string) or false+err --- @usage --- local patt = compile_topic_pattern("homes/+/+/#") --- --- local topic = "homes/myhome/living/mainlights/brightness" --- local homeid, roomid, varargs = topic:match(patt) -function mqtt.compile_topic_pattern(t) - local ok, err = mqtt.validate_subscribe_topic(t) - if not ok then - return ok, err - end - if t == "#" then - t = "(.+)" -- matches anything at least 1 character long - else - t = t:gsub("#","(.-)") -- match anything, can be empty - t = t:gsub("%+","([^/]-)") -- match anything between '/', can be empty +do + local MATCH_ALL = "(.+)" -- matches anything at least 1 character long + local MATCH_HASH = "(.-)" -- match anything, can be empty + local MATCH_PLUS = "([^/]-)" -- match anything between '/', can be empty + + --- Returns a Lua pattern from topic. + -- Takes a wildcarded-topic and returns a Lua pattern that can be used + -- to validate if a received topic matches the wildcard-topic + -- @tparam string t the wildcard topic + -- @return Lua-pattern (string) or throws error on invalid input + -- @usage + -- local patt = compile_topic_pattern("homes/+/+/#") + -- + -- local incoming_topic = "homes/myhome/living/mainlights/brightness" + -- local homeid, roomid, varargs = incoming_topic:match(patt) + function mqtt.compile_topic_pattern(t) + t = assert(mqtt.validate_subscribe_topic(t)) + if t == "#" then + t = MATCH_ALL + else + t = t:gsub("#", MATCH_HASH) + t = t:gsub("%+", MATCH_PLUS) + end + return "^"..t.."$" end - return "^"..t.."$" end ---- Parses wildcards in a topic into a table. --- Options include: --- --- - `opts.topic`: the wild-carded topic to match against (optional if `opts.pattern` is given) --- --- - `opts.pattern`: the compiled pattern for the wild-carded topic (optional if `opts.topic` --- is given). If not given then topic will be compiled and the result will be --- stored in this field for future use (cache). --- --- - `opts.keys`: (optional) array of field names. The order must be the same as the --- order of the wildcards in `topic` --- --- Returned tables: --- --- - `fields` table: the array part will have the values of the wildcards, in --- the order they appeared. The hash part, will have the field names provided --- in `opts.keys`, with the values of the corresponding wildcard. If a `#` --- wildcard was used, that one will be the last in the table. --- --- - `varargs` table: will only be returned if the wildcard topic contained the --- `#` wildcard. The returned table is an array, with all segments that were --- matched by the `#` wildcard. --- @param topic (string) incoming topic string (required) --- @param opts (table) with options (required) --- @return fields (table) + varargs (table or nil), or false+err on error. --- @usage --- local opts = { --- topic = "homes/+/+/#", --- keys = { "homeid", "roomid", "varargs"}, --- } --- local fields, varargs = topic_match("homes/myhome/living/mainlights/brightness", opts) --- --- print(fields[1], fields.homeid) -- "myhome myhome" --- print(fields[2], fields.roomid) -- "living living" --- print(fields[3], fields.varargs) -- "mainlights/brightness mainlights/brightness" --- --- print(varargs[1]) -- "mainlights" --- print(varargs[2]) -- "brightness" -function mqtt.topic_match(topic, opts) - if type(topic) ~= "string" then - return false, "expected topic to be a string" - end - if type(opts) ~= "table" then - return false, "expected optionss to be a table" - end - local pattern = opts.pattern - if not pattern then - local ptopic = opts.topic - if not ptopic then - return false, "either 'opts.topic' or 'opts.pattern' must set" +do + local HAS_VARARG_PATTERN = "%(%.[%-%+]%)%$$" -- matches patterns that have a vararg matcher + + --- Parses wildcards in a topic into a table. + -- Options include: + -- + -- - `opts.topic`: the wild-carded topic to match against (optional if `opts.pattern` is given) + -- + -- - `opts.pattern`: the compiled pattern for the wild-carded topic (optional if `opts.topic` + -- is given). If not given then topic will be compiled and the result will be + -- stored in this field for future use (cache). + -- + -- - `opts.keys`: (optional) array of field names. The order must be the same as the + -- order of the wildcards in `topic` + -- + -- Returned tables: + -- + -- - `fields` table: the array part will have the values of the wildcards, in + -- the order they appeared. The hash part, will have the field names provided + -- in `opts.keys`, with the values of the corresponding wildcard. If a `#` + -- wildcard was used, that one will be the last in the table. + -- + -- - `varargs` table: will only be returned if the wildcard topic contained the + -- `#` wildcard. The returned table is an array, with all segments that were + -- matched by the `#` wildcard. + -- @tparam string topic incoming topic string (required) + -- @tparam table opts options table(required) + -- @return fields (table) + varargs (table or nil), or false+err on error. + -- @usage + -- local opts = { + -- topic = "homes/+/+/#", + -- keys = { "homeid", "roomid", "varargs"}, + -- } + -- local fields, varargs = topic_match("homes/myhome/living/mainlights/brightness", opts) + -- + -- print(fields[1], fields.homeid) -- "myhome myhome" + -- print(fields[2], fields.roomid) -- "living living" + -- print(fields[3], fields.varargs) -- "mainlights/brightness mainlights/brightness" + -- + -- print(varargs[1]) -- "mainlights" + -- print(varargs[2]) -- "brightness" + function mqtt.topic_match(topic, opts) + if type(topic) ~= "string" then + error("expected topic to be a string, got: "..type(topic)) end - local err - pattern, err = mqtt.compile_topic_pattern(ptopic) + if type(opts) ~= "table" then + error("expected options to be a table, got: "..type(opts)) + end + local pattern = opts.pattern if not pattern then - return false, "failed to compile 'opts.topic' into pattern: "..tostring(err) + local ptopic = assert(opts.topic, "either 'opts.topic' or 'opts.pattern' must set") + pattern = assert(mqtt.compile_topic_pattern(ptopic)) + -- store/cache compiled pattern for next time + opts.pattern = pattern end - -- store/cache compiled pattern for next time - opts.pattern = pattern - end - local values = { topic:match(pattern) } - if values[1] == nil then - return false, "topic does not match wildcard pattern" - end - local keys = opts.keys - if keys ~= nil then - if type(keys) ~= "table" then - return false, "expected 'opts.keys' to be a table (array)" + local values = { topic:match(pattern) } + if values[1] == nil then + return false, "topic does not match wildcard pattern" end - -- we have a table with keys, copy values to fields - for i, value in ipairs(values) do - local key = keys[i] - if key ~= nil then - values[key] = value + local keys = opts.keys + if keys ~= nil then + if type(keys) ~= "table" then + error("expected 'opts.keys' to be a table (array), got: "..type(keys)) + end + -- we have a table with keys, copy values to fields + for i, value in ipairs(values) do + local key = keys[i] + if key ~= nil then + values[key] = value + end end end - end - if not pattern:find("%(%.[%-%+]%)%$$") then -- pattern for "#" as last char - -- we're done - return values - end - -- we have a '#' wildcard - local vararg = values[#values] - local varargs = {} - local i = 0 - local ni = 0 - while ni do - ni = vararg:find("/", i, true) - if ni then - varargs[#varargs + 1] = vararg:sub(i, ni-1) - i = ni + 1 - else - varargs[#varargs + 1] = vararg:sub(i, -1) + if not pattern:find(HAS_VARARG_PATTERN) then -- pattern for "#" as last char + -- we're done + return values + end + -- we have a '#' wildcard + local vararg = values[#values] + local varargs = {} + local i = 0 + local ni = 0 + while ni do + ni = vararg:find("/", i, true) + if ni then + varargs[#varargs + 1] = vararg:sub(i, ni-1) + i = ni + 1 + else + varargs[#varargs + 1] = vararg:sub(i, -1) + end end - end - return values, varargs + return values, varargs + end end diff --git a/tests/spec/topics.lua b/tests/spec/topics.lua index 36070ce..7b8e568 100644 --- a/tests/spec/topics.lua +++ b/tests/spec/topics.lua @@ -167,9 +167,9 @@ describe("topics", function() pattern = nil, keys = { "hello", "world"} } - local ok, err = mqtt.topic_match(nil, opts) - assert.is_false(ok) - assert.is_string(err) + assert.has.error(function() + mqtt.topic_match(nil, opts) + end, "expected topic to be a string, got: nil") end) it("wildcard topic or pattern is required", function() @@ -178,9 +178,9 @@ describe("topics", function() pattern = nil, keys = { "hello", "world"} } - local ok, err = mqtt.topic_match("hello/world", opts) - assert.is_false(ok) - assert.is_string(err) + assert.has.error(function() + mqtt.topic_match("hello/world", opts) + end, "either 'opts.topic' or 'opts.pattern' must set") end) it("pattern must match", function()