diff --git a/app/plugin_test.go b/app/plugin_test.go index bdc28a6a69..7a8922447c 100644 --- a/app/plugin_test.go +++ b/app/plugin_test.go @@ -190,16 +190,12 @@ func TestPluginKeyValueStoreCompareAndSet(t *testing.T) { } func TestPluginKeyValueStoreSetWithOptionsJSON(t *testing.T) { - th := Setup(t).InitBasic() - defer th.TearDown() - pluginId := "testpluginid" - defer func() { - assert.Nil(t, th.App.DeletePluginKey(pluginId, "key")) - }() - t.Run("storing a value without providing options works", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + result, err := th.App.SetPluginKeyWithOptions(pluginId, "key", []byte("value-1"), model.PluginKVSetOptions{}) assert.True(t, result) assert.Nil(t, err) @@ -211,6 +207,9 @@ func TestPluginKeyValueStoreSetWithOptionsJSON(t *testing.T) { }) t.Run("test that setting it atomic when it doesn't match doesn't change anything", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + err := th.App.SetPluginKey(pluginId, "key", []byte("value-1")) require.Nil(t, err) @@ -228,6 +227,9 @@ func TestPluginKeyValueStoreSetWithOptionsJSON(t *testing.T) { }) t.Run("test the atomic change with the proper old value", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + err := th.App.SetPluginKey(pluginId, "key", []byte("value-2")) require.Nil(t, err) @@ -245,6 +247,9 @@ func TestPluginKeyValueStoreSetWithOptionsJSON(t *testing.T) { }) t.Run("when new value is nil and old value matches with the current, it should delete the currently set value", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + // first set a value. result, err := th.App.SetPluginKeyWithOptions(pluginId, "nil-test-key-2", []byte("value-1"), model.PluginKVSetOptions{}) require.Nil(t, err) @@ -264,6 +269,9 @@ func TestPluginKeyValueStoreSetWithOptionsJSON(t *testing.T) { }) t.Run("when new value is nil and there is a value set for the key already, it should delete the currently set value", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + // first set a value. result, err := th.App.SetPluginKeyWithOptions(pluginId, "nil-test-key-3", []byte("value-1"), model.PluginKVSetOptions{}) require.Nil(t, err) @@ -274,12 +282,21 @@ func TestPluginKeyValueStoreSetWithOptionsJSON(t *testing.T) { assert.Nil(t, err) assert.True(t, result) + // verify a nil value is returned ret, err := th.App.GetPluginKey(pluginId, "nil-test-key-3") assert.Nil(t, err) assert.Nil(t, ret) + + // verify the row is actually gone + list, err := th.App.ListPluginKeys(pluginId, 0, 1) + assert.Nil(t, err) + assert.Empty(t, list) }) t.Run("when old value is nil and there is no value set for the key before, it should set the new value", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + result, err := th.App.SetPluginKeyWithOptions(pluginId, "nil-test-key-4", []byte("value-1"), model.PluginKVSetOptions{ Atomic: true, OldValue: nil, @@ -293,6 +310,9 @@ func TestPluginKeyValueStoreSetWithOptionsJSON(t *testing.T) { }) t.Run("test that value is set and unset with ExpireInSeconds", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + result, err := th.App.SetPluginKeyWithOptions(pluginId, "key", []byte("value-1"), model.PluginKVSetOptions{ ExpireInSeconds: 1, }) diff --git a/store/sqlstore/plugin_store.go b/store/sqlstore/plugin_store.go index 26b20414df..defd2276e4 100644 --- a/store/sqlstore/plugin_store.go +++ b/store/sqlstore/plugin_store.go @@ -42,6 +42,16 @@ func (ps SqlPluginStore) SaveOrUpdate(kv *model.PluginKeyValue) (*model.PluginKe return nil, err } + if kv.Value == nil { + // Setting a key to nil is the same as removing it + err := ps.Delete(kv.PluginId, kv.Key) + if err != nil { + return nil, err + } + + return kv, nil + } + if ps.DriverName() == model.DATABASE_DRIVER_POSTGRES { // Unfortunately PostgreSQL pre-9.5 does not have an atomic upsert, so we use // separate update and insert queries to accomplish our upsert