address review comments

.editorconfig

@@ -12,3 +12,4 @@ indent_size              = 4
 
 [Makefile]
 indent_style             = tab
+indent_size              = 4

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
 
 

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()