From 2460207d50a0593c16cda78b467e9ec2d6094d0e Mon Sep 17 00:00:00 2001 From: Matthew Wild Date: Tue, 27 Sep 2022 17:46:27 +0100 Subject: storagemanager: Add keyval+ (combined keyval + map) store type This combines the two most common store types, which modules often end up opening with both interfaces separately anyway. As well as combining them, I've taken the opportunity to improve some of the method names to make them clearer. --- core/storagemanager.lua | 73 +++++++++++++++++++++ spec/core_storagemanager_spec.lua | 130 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+) diff --git a/core/storagemanager.lua b/core/storagemanager.lua index 856acad3..8cc19378 100644 --- a/core/storagemanager.lua +++ b/core/storagemanager.lua @@ -203,6 +203,37 @@ local map_shim_mt = { }; } +local combined_store_mt = { + __index = { + -- keyval + get = function (self, name) + return self.keyval_store:get(name); + end; + set = function (self, name, data) + return self.keyval_store:set(name, data); + end; + items = function (self) + return self.keyval_store:users(); + end; + -- map + get_key = function (self, name, key) + return self.map_store:get(name, key); + end; + set_key = function (self, name, key, value) + return self.map_store:set(name, key, value); + end; + set_keys = function (self, name, map) + return self.map_store:set_keys(name, map); + end; + get_key_from_all = function (self, key) + return self.map_store:get_all(key); + end; + delete_key_from_all = function (self, key) + return self.map_store:delete_all(key); + end; + }; +}; + local open; -- forward declaration local function create_map_shim(host, store) @@ -213,7 +244,49 @@ local function create_map_shim(host, store) }, map_shim_mt); end +local function open_combined(host, store) + local driver, driver_name = get_driver(host, store); + + -- Open keyval + local keyval_store, err = driver:open(store, "keyval"); + if not keyval_store then + if err == "unsupported-store" then + log("debug", "Storage driver %s does not support store %s (keyval), falling back to null driver", + driver_name, store); + keyval_store, err = null_storage_driver, nil; + end + end + + local map_store; + if keyval_store then + -- Open map + map_store, err = driver:open(store, "map"); + if not map_store then + if err == "unsupported-store" then + log("debug", "Storage driver %s does not support store %s (map), falling back to shim", + driver_name, store); + map_store, err = setmetatable({ keyval_store = keyval_store }, map_shim_mt), nil; + end + end + end + + if not(keyval_store and map_store) then + return nil, err; + end + local combined_store = setmetatable({ + keyval_store = keyval_store; + map_store = map_store; + remove = map_store.remove; + }, combined_store_mt); + local event_data = { host = host, store_name = store, store_type = "keyval+", store = combined_store }; + hosts[host].events.fire_event("store-opened", event_data); + return event_data.store, event_data.store_err; +end + function open(host, store, typ) + if typ == "keyval+" then -- TODO: default in some release? + return open_combined(host, store); + end local driver, driver_name = get_driver(host, store); local ret, err = driver:open(store, typ); if not ret then diff --git a/spec/core_storagemanager_spec.lua b/spec/core_storagemanager_spec.lua index 04acb1dd..55e32aaa 100644 --- a/spec/core_storagemanager_spec.lua +++ b/spec/core_storagemanager_spec.lua @@ -196,6 +196,136 @@ describe("storagemanager", function () end); end); + describe("keyval+ stores", function () + -- These tests rely on being executed in order, disable any order + -- randomization for this block + randomize(false); + + local store, kv_store, map_store; + it("may be opened", function () + store = assert(sm.open(test_host, "test-kv+", "keyval+")); + end); + + local simple_data = { foo = "bar" }; + + it("may set data for a user", function () + assert(store:set("user9999", simple_data)); + end); + + it("may get data for a user", function () + assert.same(simple_data, assert(store:get("user9999"))); + end); + + it("may be opened as a keyval store", function () + kv_store = assert(sm.open(test_host, "test-kv+", "keyval")); + assert.same(simple_data, assert(kv_store:get("user9999"))); + end); + + it("may be opened as a map store", function () + map_store = assert(sm.open(test_host, "test-kv+", "map")); + assert.same("bar", assert(map_store:get("user9999", "foo"))); + end); + + it("may remove data for a user", function () + assert(store:set("user9999", nil)); + local ret, err = store:get("user9999"); + assert.is_nil(ret); + assert.is_nil(err); + end); + + + it("may set a specific key for a user", function () + assert(store:set_key("user9999", "foo", "bar")); + assert.same(kv_store:get("user9999"), { foo = "bar" }); + end); + + it("may get a specific key for a user", function () + assert.equal("bar", store:get_key("user9999", "foo")); + end); + + it("may find all users with a specific key", function () + assert.is_function(store.get_key_from_all); + assert(store:set_key("user9999b", "bar", "bar")); + assert(store:set_key("user9999c", "foo", "blah")); + local ret, err = store:get_key_from_all("foo"); + assert.is_nil(err); + assert.same({ user9999 = "bar", user9999c = "blah" }, ret); + end); + + it("rejects empty or non-string keys to get_all", function () + assert.is_function(store.get_key_from_all); + do + local ret, err = store:get_key_from_all(""); + assert.is_nil(ret); + assert.is_not_nil(err); + end + do + local ret, err = store:get_key_from_all(true); + assert.is_nil(ret); + assert.is_not_nil(err); + end + end); + + it("rejects empty or non-string keys to delete_all", function () + assert.is_function(store.delete_key_from_all); + do + local ret, err = store:delete_key_from_all(""); + assert.is_nil(ret); + assert.is_not_nil(err); + end + do + local ret, err = store:delete_key_from_all(true); + assert.is_nil(ret); + assert.is_not_nil(err); + end + end); + + it("may delete all instances of a specific key", function () + assert.is_function(store.delete_key_from_all); + assert(store:set_key("user9999b", "foo", "hello")); + + assert(store:delete_key_from_all("bar")); + -- Ensure key was deleted + do + local ret, err = store:get_key("user9999b", "bar"); + assert.is_nil(ret); + assert.is_nil(err); + end + -- Ensure other users/keys are intact + do + local ret, err = store:get_key("user9999", "foo"); + assert.equal("bar", ret); + assert.is_nil(err); + end + do + local ret, err = store:get_key("user9999b", "foo"); + assert.equal("hello", ret); + assert.is_nil(err); + end + do + local ret, err = store:get_key("user9999c", "foo"); + assert.equal("blah", ret); + assert.is_nil(err); + end + end); + + it("may remove data for a specific key for a user", function () + assert(store:set_key("user9999", "foo", nil)); + do + local ret, err = store:get_key("user9999", "foo"); + assert.is_nil(ret); + assert.is_nil(err); + end + + assert(store:set_key("user9999b", "foo", nil)); + do + local ret, err = store:get_key("user9999b", "foo"); + assert.is_nil(ret); + assert.is_nil(err); + end + end); + end); + describe("archive stores", function () randomize(false); -- cgit v1.2.3