From 378bb147c81634128bf0918f4ede3aa48cac8b45 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 00:53:21 +0200 Subject: mod_mam: Factor out "should we store this" into a function Meant to improve readability and ease further improvements to this algorithm. --- plugins/mod_mam/mod_mam.lua | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 8229eb4e..27b4796d 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -263,11 +263,28 @@ local function strip_stanza_id(stanza, user) return stanza; end +local function should_store(stanza) --> boolean, reason: string + local orig_type = stanza.attr.type or "normal"; + -- We store chat messages or normal messages that have a body + if not(orig_type == "chat" or (orig_type == "normal" and stanza:get_child("body")) ) then + return false, "type"; + end + + -- or if hints suggest we shouldn't + if not stanza:get_child("store", "urn:xmpp:hints") then -- No hint telling us we should store + if stanza:get_child("no-permanent-store", "urn:xmpp:hints") + or stanza:get_child("no-store", "urn:xmpp:hints") then -- Hint telling us we should NOT store + return false, "hint"; + end + end + + return true, "default"; +end + -- Handle messages local function message_handler(event, c2s) local origin, stanza = event.origin, event.stanza; local log = c2s and origin.log or module._log; - local orig_type = stanza.attr.type or "normal"; local orig_from = stanza.attr.from; local orig_to = stanza.attr.to or orig_from; -- Stanza without 'to' are treated as if it was to their own bare jid @@ -280,21 +297,12 @@ local function message_handler(event, c2s) -- Filter out that claim to be from us event.stanza = strip_stanza_id(stanza, store_user); - -- We store chat messages or normal messages that have a body - if not(orig_type == "chat" or (orig_type == "normal" and stanza:get_child("body")) ) then - log("debug", "Not archiving stanza: %s (type)", stanza:top_tag()); + local should, why = should_store(stanza); + if not should then + log("debug", "Not archiving stanza: %s (%s)", stanza:top_tag(), why); return; end - -- or if hints suggest we shouldn't - if not stanza:get_child("store", "urn:xmpp:hints") then -- No hint telling us we should store - if stanza:get_child("no-permanent-store", "urn:xmpp:hints") - or stanza:get_child("no-store", "urn:xmpp:hints") then -- Hint telling us we should NOT store - log("debug", "Not archiving stanza: %s (hint)", stanza:top_tag()); - return; - end - end - local clone_for_storage; if not strip_tags:empty() then clone_for_storage = st.clone(stanza); -- cgit v1.2.3 From 9549657e06f1d565bdc11808eb10da8707b91c40 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 00:53:23 +0200 Subject: mod_mam: Log 'why' a stanza is archived Logging of 'why not' is already done. Why not both? Will make more sense when the rules evolve a bit. --- plugins/mod_mam/mod_mam.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 27b4796d..9c00cc99 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -323,7 +323,7 @@ local function message_handler(event, c2s) -- Check with the users preferences if shall_store(store_user, with) then - log("debug", "Archiving stanza: %s", stanza:top_tag()); + log("debug", "Archiving stanza: %s (%s)", stanza:top_tag(), why); -- And stash it local time = time_now(); -- cgit v1.2.3 From 5c12379ed9273e2be201476763ef51af96b43fff Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 00:56:56 +0200 Subject: mod_mam: Invert check for type This is based on code in mod_csi_simple and aiming towards being more flexible and maintainable than a couple of tests for when not to store. --- plugins/mod_mam/mod_mam.lua | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 9c00cc99..028c3b8f 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -264,11 +264,8 @@ local function strip_stanza_id(stanza, user) end local function should_store(stanza) --> boolean, reason: string - local orig_type = stanza.attr.type or "normal"; - -- We store chat messages or normal messages that have a body - if not(orig_type == "chat" or (orig_type == "normal" and stanza:get_child("body")) ) then - return false, "type"; - end + local st_type = stanza.attr.type or "normal"; + local st_to_full = (stanza.attr.to or ""):find("/"); -- or if hints suggest we shouldn't if not stanza:get_child("store", "urn:xmpp:hints") then -- No hint telling us we should store @@ -277,6 +274,17 @@ local function should_store(stanza) --> boolean, reason: string return false, "hint"; end end + if st_type == "headline" then + -- Headline messages are ephemeral by definition + return false, "headline"; + end + if st_type == "groupchat" and st_to_full then + -- MUC messages always go to the full JID, usually archived by the MUC + return false, "groupchat"; + end + if stanza:get_child("body") then + return true, "body"; + end return true, "default"; end -- cgit v1.2.3 From baef0f02e863f093af83003598949c94e9a82218 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 01:01:25 +0200 Subject: mod_mam: Rework hints handling Improved readability and early returns definite yes/no answer. --- plugins/mod_mam/mod_mam.lua | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 028c3b8f..57f399ae 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -267,13 +267,6 @@ local function should_store(stanza) --> boolean, reason: string local st_type = stanza.attr.type or "normal"; local st_to_full = (stanza.attr.to or ""):find("/"); - -- or if hints suggest we shouldn't - if not stanza:get_child("store", "urn:xmpp:hints") then -- No hint telling us we should store - if stanza:get_child("no-permanent-store", "urn:xmpp:hints") - or stanza:get_child("no-store", "urn:xmpp:hints") then -- Hint telling us we should NOT store - return false, "hint"; - end - end if st_type == "headline" then -- Headline messages are ephemeral by definition return false, "headline"; @@ -282,6 +275,12 @@ local function should_store(stanza) --> boolean, reason: string -- MUC messages always go to the full JID, usually archived by the MUC return false, "groupchat"; end + if stanza:get_child("no-permanent-store", "urn:xmpp:hints") then + return false, "hint"; + end + if stanza:get_child("store", "urn:xmpp:hints") then + return true, "hint"; + end if stanza:get_child("body") then return true, "body"; end -- cgit v1.2.3 From 1056f4bb820479c00c4123f5fea818f45c47f98c Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 01:17:55 +0200 Subject: mod_mam: Add more positive hints for storage Mostly just lifted from mod_csi_simple --- plugins/mod_mam/mod_mam.lua | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 57f399ae..385a0ad2 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -284,6 +284,18 @@ local function should_store(stanza) --> boolean, reason: string if stanza:get_child("body") then return true, "body"; end + if stanza:get_child("subject") then + -- XXX Who would send a message with a subject but with a body? + return true, "subject"; + end + if stanza:get_child("encryption", "urn:xmpp:eme:0") then + -- Since we can't know what an encrypted message contains, we assume it's important + return true, "encrypted"; + end + if stanza:get_child("x", "jabber:x:conference") + or stanza:find("{http://jabber.org/protocol/muc#user}x/invite") then + return true, "invite"; + end return true, "default"; end -- cgit v1.2.3 From 3dcfe11e54b8c4f936b701f9e57843fce39500f9 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 01:18:54 +0200 Subject: mod_mam: Store XEP-0184 receipts and requests Happy now Ge0rG? --- plugins/mod_mam/mod_mam.lua | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 385a0ad2..6e522d73 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -292,6 +292,11 @@ local function should_store(stanza) --> boolean, reason: string -- Since we can't know what an encrypted message contains, we assume it's important return true, "encrypted"; end + if stanza:get_child(nil, "urn:xmpp:receipts") then + -- If it's important enough to ask for a receipt then it's important enough to archive + -- and the same applies to the receipt + return true, "receipt"; + end if stanza:get_child("x", "jabber:x:conference") or stanza:find("{http://jabber.org/protocol/muc#user}x/invite") then return true, "invite"; -- cgit v1.2.3 From 230ceb086948cd12c258ca78cfbf8a345904fd8e Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 01:29:58 +0200 Subject: mod_mam: Check sender of error instead of receiver The intent is to capture errors to stanzas sent by the local user, so that they can see why a message failed to be delivered even if the error came after they went offline. --- plugins/mod_mam/mod_mam.lua | 3 +++ 1 file changed, 3 insertions(+) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 6e522d73..db0e7f8d 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -266,6 +266,9 @@ end local function should_store(stanza) --> boolean, reason: string local st_type = stanza.attr.type or "normal"; local st_to_full = (stanza.attr.to or ""):find("/"); + if st_type == "error" then + st_to_full = (stanza.attr.from or ""):find("/"); + end if st_type == "headline" then -- Headline messages are ephemeral by definition -- cgit v1.2.3 From ef0b90ff2393da021d6ac0f101aaba533a4825d9 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 19:41:43 +0200 Subject: mod_mam: Prefer not archiving if no interesting payloads are found --- plugins/mod_mam/mod_mam.lua | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index db0e7f8d..69282857 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -305,7 +305,9 @@ local function should_store(stanza) --> boolean, reason: string return true, "invite"; end - return true, "default"; + -- The IM-NG thing to do here would be to return `not st_to_full` + -- One day ... + return false, "default"; end -- Handle messages -- cgit v1.2.3 From 62b575639d000dcf172caa1cfd089ba12674c6dc Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Tue, 21 Apr 2020 23:06:55 +0200 Subject: mod_mam: Fix typo in comment If it is with a body then it execution does not get this far --- plugins/mod_mam/mod_mam.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 69282857..6d493131 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -288,7 +288,7 @@ local function should_store(stanza) --> boolean, reason: string return true, "body"; end if stanza:get_child("subject") then - -- XXX Who would send a message with a subject but with a body? + -- XXX Who would send a message with a subject but without a body? return true, "subject"; end if stanza:get_child("encryption", "urn:xmpp:eme:0") then -- cgit v1.2.3 From ead815ece336974e216141f5549c4ea8083b0a63 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Wed, 22 Apr 2020 18:47:06 +0200 Subject: mod_mam: Respect no-store hint (thanks Ge0rG) no-store is used in an example in XEP-0313, so obviously this is the preferred hint --- plugins/mod_mam/mod_mam.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 6d493131..7ac2a512 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -278,7 +278,8 @@ local function should_store(stanza) --> boolean, reason: string -- MUC messages always go to the full JID, usually archived by the MUC return false, "groupchat"; end - if stanza:get_child("no-permanent-store", "urn:xmpp:hints") then + if stanza:get_child("no-store", "urn:xmpp:hints") + or stanza:get_child("no-permanent-store", "urn:xmpp:hints") then return false, "hint"; end if stanza:get_child("store", "urn:xmpp:hints") then -- cgit v1.2.3 From bb2a3804d1db20f02ae445991a3d92a596431c41 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Wed, 22 Apr 2020 18:48:27 +0200 Subject: mod_mam: Keep chat markers (thanks Ge0rG) --- plugins/mod_mam/mod_mam.lua | 3 +++ 1 file changed, 3 insertions(+) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 7ac2a512..06a20f4d 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -301,6 +301,9 @@ local function should_store(stanza) --> boolean, reason: string -- and the same applies to the receipt return true, "receipt"; end + if stanza:get_child(nil, "urn:xmpp:chat-markers:0") then + return true, "marker"; + end if stanza:get_child("x", "jabber:x:conference") or stanza:find("{http://jabber.org/protocol/muc#user}x/invite") then return true, "invite"; -- cgit v1.2.3 From 4dbf52830e170867320e83f742d69803651871cb Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Wed, 22 Apr 2020 18:50:30 +0200 Subject: mod_mam: Save delivery failures (thanks Ge0rG) Makes it possible to learn of delivery failure even if it came bouncing back while you were offline. --- plugins/mod_mam/mod_mam.lua | 3 +++ 1 file changed, 3 insertions(+) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 06a20f4d..b74a92ea 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -274,6 +274,9 @@ local function should_store(stanza) --> boolean, reason: string -- Headline messages are ephemeral by definition return false, "headline"; end + if st_type == "error" then + return true, "bounce"; + end if st_type == "groupchat" and st_to_full then -- MUC messages always go to the full JID, usually archived by the MUC return false, "groupchat"; -- cgit v1.2.3 From de0caa52012ef89b63a090c6a06e5288efd3ff84 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Wed, 22 Apr 2020 18:53:50 +0200 Subject: mod_mam: Make note of Experimental (or Deferred) XEPs Since these XEPs are subject to change we may need come back and double check these in the future. --- plugins/mod_mam/mod_mam.lua | 3 +++ 1 file changed, 3 insertions(+) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index b74a92ea..a9d389f5 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -283,6 +283,7 @@ local function should_store(stanza) --> boolean, reason: string end if stanza:get_child("no-store", "urn:xmpp:hints") or stanza:get_child("no-permanent-store", "urn:xmpp:hints") then + -- XXX Experimental XEP return false, "hint"; end if stanza:get_child("store", "urn:xmpp:hints") then @@ -297,6 +298,7 @@ local function should_store(stanza) --> boolean, reason: string end if stanza:get_child("encryption", "urn:xmpp:eme:0") then -- Since we can't know what an encrypted message contains, we assume it's important + -- XXX Experimental XEP return true, "encrypted"; end if stanza:get_child(nil, "urn:xmpp:receipts") then @@ -305,6 +307,7 @@ local function should_store(stanza) --> boolean, reason: string return true, "receipt"; end if stanza:get_child(nil, "urn:xmpp:chat-markers:0") then + -- XXX Experimental XEP return true, "marker"; end if stanza:get_child("x", "jabber:x:conference") -- cgit v1.2.3 From 9644332df92d2c2d589744377cc685a405213143 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Thu, 23 Apr 2020 00:55:34 +0200 Subject: mod_mam: Don't store any groupchat messages The intent was to not store MUC groupchat messages, which are sent from the MUC to local full JIDs, while allowing for potential future account based group chat. However, since this function handles messages in both directions and outgoing MUC messages are sent to the bare room JID, those were stored. --- plugins/mod_mam/mod_mam.lua | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index a9d389f5..65859afd 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -269,6 +269,8 @@ local function should_store(stanza) --> boolean, reason: string if st_type == "error" then st_to_full = (stanza.attr.from or ""):find("/"); end + -- FIXME pass direction of stanza and use that along with bare/full JID addressing + -- for more accurate MUC / type=groupchat check if st_type == "headline" then -- Headline messages are ephemeral by definition @@ -277,7 +279,7 @@ local function should_store(stanza) --> boolean, reason: string if st_type == "error" then return true, "bounce"; end - if st_type == "groupchat" and st_to_full then + if st_type == "groupchat" then -- MUC messages always go to the full JID, usually archived by the MUC return false, "groupchat"; end -- cgit v1.2.3 From 7c3e03b3bbb8494882738f83459266579df97c72 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Thu, 23 Apr 2020 01:05:34 +0200 Subject: mod_mam: Remove unused variables [luacheck] Logic using full vs bare JID addressing may return in the future. --- plugins/mod_mam/mod_mam.lua | 4 ---- 1 file changed, 4 deletions(-) (limited to 'plugins/mod_mam') diff --git a/plugins/mod_mam/mod_mam.lua b/plugins/mod_mam/mod_mam.lua index 65859afd..d61d4883 100644 --- a/plugins/mod_mam/mod_mam.lua +++ b/plugins/mod_mam/mod_mam.lua @@ -265,10 +265,6 @@ end local function should_store(stanza) --> boolean, reason: string local st_type = stanza.attr.type or "normal"; - local st_to_full = (stanza.attr.to or ""):find("/"); - if st_type == "error" then - st_to_full = (stanza.attr.from or ""):find("/"); - end -- FIXME pass direction of stanza and use that along with bare/full JID addressing -- for more accurate MUC / type=groupchat check -- cgit v1.2.3