From ffeeffd35c4a274351804d9a8d40b9483830b0b6 Mon Sep 17 00:00:00 2001 From: Kim Alvefur Date: Thu, 22 Jul 2021 21:01:11 +0200 Subject: util.pubsub: Fix behavior of persist_items disabled When set to 'false' there is no need for a persistence interface at all, since items are not persisted after being broadcast. Had started wondering if maybe the behavior was wrong, after reading parts of XEP-0060 that pointed in that direction. Some discussion of this can be found in logs of xmpp:xsf@muc.xmpp.org?join from around 2021-07-20 Thanks to Ralph for confirming. --- spec/util_pubsub_spec.lua | 19 +++++++++++++++++++ util/pubsub.lua | 29 ++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/spec/util_pubsub_spec.lua b/spec/util_pubsub_spec.lua index f876089e..5140a411 100644 --- a/spec/util_pubsub_spec.lua +++ b/spec/util_pubsub_spec.lua @@ -509,4 +509,23 @@ describe("util.pubsub", function () end); end); + describe("persist_items", function() + it("can be disabled", function() + local broadcaster = spy.new(function(notif_type, node_name, subscribers, item) -- luacheck: ignore 212 + end); + local service = pubsub.new { node_defaults = { persist_items = false }, broadcaster = broadcaster } + + local ok = service:create("node", true) + assert.truthy(ok); + + local ok = service:publish("node", true, "1", "item"); + assert.truthy(ok); + assert.spy(broadcaster).was_called(); + + local ok, items = service:get_items("node", true); + assert.truthy(ok); + assert.same(items, {}); + end); + + end) end); diff --git a/util/pubsub.lua b/util/pubsub.lua index c60da9e7..4b16dcfc 100644 --- a/util/pubsub.lua +++ b/util/pubsub.lua @@ -177,8 +177,11 @@ local function new(config) -- Load nodes from storage, if we have a store and it supports iterating over stored items if config.nodestore and config.nodestore.users then for node_name in config.nodestore:users() do - service.nodes[node_name] = load_node_from_store(service, node_name); - service.data[node_name] = config.itemstore(service.nodes[node_name].config, node_name); + local node = load_node_from_store(service, node_name); + service.nodes[node_name] = node; + if node.config.persist_items then + service.data[node_name] = config.itemstore(service.nodes[node_name].config, node_name); + end for jid in pairs(service.nodes[node_name].subscribers) do local normal_jid = service.config.normalize_jid(jid); @@ -466,7 +469,10 @@ function service:create(node, actor, options) --> ok, err end end - self.data[node] = self.config.itemstore(self.nodes[node].config, node); + if config.persist_items then + self.data[node] = self.config.itemstore(self.nodes[node].config, node); + end + self.events.fire_event("node-created", { service = self, node = node, actor = actor }); if actor ~= true then local ok, err = self:set_affiliation(node, true, actor, "owner"); @@ -564,9 +570,11 @@ function service:publish(node, actor, id, item, requested_config) --> ok, err if not self.config.itemcheck(item) then return nil, "invalid-item"; end - local node_data = self.data[node]; - if node_data then - local ok = node_data:set(id, item); + if node_obj.config.persist_items then + if not self.data[node] then + self.data[node] = self.config.itemstore(self.nodes[node].config, node); + end + local ok = self.data[node]:set(id, item); if not ok then return nil, "internal-server-error"; end @@ -816,7 +824,14 @@ function service:set_node_config(node, actor, new_config) --> ok, err end if old_config["persist_items"] ~= node_obj.config["persist_items"] then - self.data[node] = self.config.itemstore(self.nodes[node].config, node); + if node_obj.config["persist_items"] then + self.data[node] = self.config.itemstore(self.nodes[node].config, node); + elseif self.data[node] then + if self.data[node].clear then + self.data[node]:clear() + end + self.data[node] = nil; + end elseif old_config["max_items"] ~= node_obj.config["max_items"] then if self.data[node] then self.data[node]:resize(self.nodes[node].config["max_items"]); -- cgit v1.2.3