[MM-16791] Migrate Plugin.Delete to Sync by default (#11580)

* Migrate Plugin.Delete to Sync by default

* remove unused import

app/plugin_key_value_store.go

@@ -40,8 +40,8 @@ func (a *App) SetPluginKeyWithExpiry(pluginId string, key string, value []byte,
 	}
 
 	// Clean up a previous entry using the hashed key, if it exists.
-	if result := <-a.Srv.Store.Plugin().Delete(pluginId, getKeyHash(key)); result.Err != nil {
+	if err := a.Srv.Store.Plugin().Delete(pluginId, getKeyHash(key)); err != nil {
-		mlog.Error("Failed to clean up previously hashed plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(result.Err))
+		mlog.Error("Failed to clean up previously hashed plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
 	}
 
 	return nil
@@ -61,8 +61,8 @@ func (a *App) CompareAndSetPluginKey(pluginId string, key string, oldValue, newV
 	}
 
 	// Clean up a previous entry using the hashed key, if it exists.
-	if result := <-a.Srv.Store.Plugin().Delete(pluginId, getKeyHash(key)); result.Err != nil {
+	if err := a.Srv.Store.Plugin().Delete(pluginId, getKeyHash(key)); err != nil {
-		mlog.Error("Failed to clean up previously hashed plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(result.Err))
+		mlog.Error("Failed to clean up previously hashed plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
 	}
 
 	return updated, nil
@@ -88,15 +88,15 @@ func (a *App) GetPluginKey(pluginId string, key string) ([]byte, *model.AppError
 }
 
 func (a *App) DeletePluginKey(pluginId string, key string) *model.AppError {
-	if result := <-a.Srv.Store.Plugin().Delete(pluginId, getKeyHash(key)); result.Err != nil {
+	if err := a.Srv.Store.Plugin().Delete(pluginId, getKeyHash(key)); err != nil {
-		mlog.Error("Failed to delete plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(result.Err))
+		mlog.Error("Failed to delete plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return result.Err
+		return err
 	}
 
 	// Also delete the key without hashing
-	if result := <-a.Srv.Store.Plugin().Delete(pluginId, key); result.Err != nil {
+	if err := a.Srv.Store.Plugin().Delete(pluginId, key); err != nil {
-		mlog.Error("Failed to delete plugin key value using hashed key", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(result.Err))
+		mlog.Error("Failed to delete plugin key value using hashed key", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return result.Err
+		return err
 	}
 
 	return nil

store/sqlstore/plugin_store.go

@@ -124,14 +124,11 @@ func (ps SqlPluginStore) Get(pluginId, key string) (*model.PluginKeyValue, *mode
 	return kv, nil
 }
 
-func (ps SqlPluginStore) Delete(pluginId, key string) store.StoreChannel {
+func (ps SqlPluginStore) Delete(pluginId, key string) *model.AppError {
-	return store.Do(func(result *store.StoreResult) {
+	if _, err := ps.GetMaster().Exec("DELETE FROM PluginKeyValueStore WHERE PluginId = :PluginId AND PKey = :Key", map[string]interface{}{"PluginId": pluginId, "Key": key}); err != nil {
-		if _, err := ps.GetMaster().Exec("DELETE FROM PluginKeyValueStore WHERE PluginId = :PluginId AND PKey = :Key", map[string]interface{}{"PluginId": pluginId, "Key": key}); err != nil {
+		return model.NewAppError("SqlPluginStore.Delete", "store.sql_plugin_store.delete.app_error", nil, fmt.Sprintf("plugin_id=%v, key=%v, err=%v", pluginId, key, err.Error()), http.StatusInternalServerError)
-			result.Err = model.NewAppError("SqlPluginStore.Delete", "store.sql_plugin_store.delete.app_error", nil, fmt.Sprintf("plugin_id=%v, key=%v, err=%v", pluginId, key, err.Error()), http.StatusInternalServerError)
+	}
-		} else {
+	return nil
-			result.Data = true
-		}
-	})
 }
 
 func (ps SqlPluginStore) DeleteAllForPlugin(pluginId string) *model.AppError {

store/store.go

@@ -538,7 +538,7 @@ type PluginStore interface {
 	SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError)
 	CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError)
 	Get(pluginId, key string) (*model.PluginKeyValue, *model.AppError)
-	Delete(pluginId, key string) StoreChannel
+	Delete(pluginId, key string) *model.AppError
 	DeleteAllForPlugin(PluginId string) *model.AppError
 	DeleteAllExpired() *model.AppError
 	List(pluginId string, page, perPage int) ([]string, *model.AppError)

store/storetest/mocks/PluginStore.go

@@ -6,7 +6,6 @@ package mocks
 
 import mock "github.com/stretchr/testify/mock"
 import model "github.com/mattermost/mattermost-server/model"
-import store "github.com/mattermost/mattermost-server/store"
 
 // PluginStore is an autogenerated mock type for the PluginStore type
 type PluginStore struct {
@@ -37,15 +36,15 @@ func (_m *PluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []by
 }
 
 // Delete provides a mock function with given fields: pluginId, key
-func (_m *PluginStore) Delete(pluginId string, key string) store.StoreChannel {
+func (_m *PluginStore) Delete(pluginId string, key string) *model.AppError {
 	ret := _m.Called(pluginId, key)
 
-	var r0 store.StoreChannel
+	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok {
 		r0 = rf(pluginId, key)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.AppError)
 		}
 	}
 

store/storetest/plugin_store.go

@@ -34,7 +34,7 @@ func testPluginSaveGet(t *testing.T, ss store.Store) {
 	}
 
 	defer func() {
-		<-ss.Plugin().Delete(kv.PluginId, kv.Key)
+		_ = ss.Plugin().Delete(kv.PluginId, kv.Key)
 	}()
 
 	if received, err := ss.Plugin().Get(kv.PluginId, kv.Key); err != nil {
@@ -74,7 +74,7 @@ func testPluginSaveGetExpiry(t *testing.T, ss store.Store) {
 	}
 
 	defer func() {
-		<-ss.Plugin().Delete(kv.PluginId, kv.Key)
+		_ = ss.Plugin().Delete(kv.PluginId, kv.Key)
 	}()
 
 	if received, err := ss.Plugin().Get(kv.PluginId, kv.Key); err != nil {
@@ -98,7 +98,7 @@ func testPluginSaveGetExpiry(t *testing.T, ss store.Store) {
 	}
 
 	defer func() {
-		<-ss.Plugin().Delete(kv.PluginId, kv.Key)
+		_ = ss.Plugin().Delete(kv.PluginId, kv.Key)
 	}()
 
 	if _, err := ss.Plugin().Get(kv.PluginId, kv.Key); err == nil {
@@ -114,8 +114,8 @@ func testPluginDelete(t *testing.T, ss store.Store) {
 	})
 	require.Nil(t, err)
 
-	if result := <-ss.Plugin().Delete(kv.PluginId, kv.Key); result.Err != nil {
+	if err := ss.Plugin().Delete(kv.PluginId, kv.Key); err != nil {
-		t.Fatal(result.Err)
+		t.Fatal(err)
 	}
 }