Update channel member notify props to use native JSON (#18114)

* Update channel member notify props to use native JSON Created a new store method that patches the notify props field. https://community-daily.mattermost.com/plugins/focalboard/workspace/zyoahc9uapdn3xdptac6jb69ic?id=285b80a3-257d-41f6-8cf4-ed80ca9d92e5&v=495cdb4d-c13a-4992-8eb9-80cfee2819a4&c=91d08676-4a0e-4f02-8dce-d24d4fc56449 ```release-note NONE ``` * cleanup ```release-note NONE ``` * forgot to commit ```release-note NONE ``` * Fix edge case ```release-note NONE ``` * address review comments ```release-note NONE ``` * fix incorrect order ```release-note NONE ``` * Address review comments ```release-note NONE ``` Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
2021-08-23 10:04:30 +05:30
--- a/app/channel.go
+++ b/app/channel.go
@@ -1206,40 +1206,51 @@ func (a *App) UpdateChannelMemberSchemeRoles(channelID string, userID string, is
 }

 func (a *App) UpdateChannelMemberNotifyProps(data map[string]string, channelID string, userID string) (*model.ChannelMember, *model.AppError) {
-	var member *model.ChannelMember
-	var err *model.AppError
-	if member, err = a.GetChannelMember(context.Background(), channelID, userID); err != nil {
-		return nil, err
-	}
+	filteredProps := make(map[string]string)

 	// update whichever notify properties have been provided, but don't change the others
 	if markUnread, exists := data[model.MarkUnreadNotifyProp]; exists {
-		member.NotifyProps[model.MarkUnreadNotifyProp] = markUnread
+		filteredProps[model.MarkUnreadNotifyProp] = markUnread
 	}

 	if desktop, exists := data[model.DesktopNotifyProp]; exists {
-		member.NotifyProps[model.DesktopNotifyProp] = desktop
+		filteredProps[model.DesktopNotifyProp] = desktop
 	}

 	if email, exists := data[model.EmailNotifyProp]; exists {
-		member.NotifyProps[model.EmailNotifyProp] = email
+		filteredProps[model.EmailNotifyProp] = email
 	}

 	if push, exists := data[model.PushNotifyProp]; exists {
-		member.NotifyProps[model.PushNotifyProp] = push
+		filteredProps[model.PushNotifyProp] = push
 	}

 	if ignoreChannelMentions, exists := data[model.IgnoreChannelMentionsNotifyProp]; exists {
-		member.NotifyProps[model.IgnoreChannelMentionsNotifyProp] = ignoreChannelMentions
+		filteredProps[model.IgnoreChannelMentionsNotifyProp] = ignoreChannelMentions
 	}

-	member, err = a.updateChannelMember(member)
+	member, err := a.Srv().Store.Channel().UpdateMemberNotifyProps(channelID, userID, filteredProps)
 	if err != nil {
-		return nil, err
+		var appErr *model.AppError
+		var nfErr *store.ErrNotFound
+		switch {
+		case errors.As(err, &appErr):
+			return nil, appErr
+		case errors.As(err, &nfErr):
+			return nil, model.NewAppError("updateMemberNotifyProps", MissingChannelMemberError, nil, nfErr.Error(), http.StatusNotFound)
+		default:
+			return nil, model.NewAppError("updateMemberNotifyProps", "app.channel.get_member.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
 	}

+	a.InvalidateCacheForUser(member.UserId)
 	a.invalidateCacheForChannelMembersNotifyProps(member.ChannelId)

+	// Notify the clients that the member notify props changed
+	evt := model.NewWebSocketEvent(model.WebsocketEventChannelMemberUpdated, "", "", member.UserId, nil)
+	evt.Add("channelMember", member.ToJson())
+	a.Publish(evt)
+
 	return member, nil
 }

--- a/store/opentracinglayer/opentracinglayer.go
+++ b/store/opentracinglayer/opentracinglayer.go
@@ -2212,6 +2212,24 @@ func (s *OpenTracingLayerChannelStore) UpdateMember(member *model.ChannelMember)
 	return result, err
 }

+func (s *OpenTracingLayerChannelStore) UpdateMemberNotifyProps(channelID string, userID string, props map[string]string) (*model.ChannelMember, error) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.UpdateMemberNotifyProps")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	result, err := s.ChannelStore.UpdateMemberNotifyProps(channelID, userID, props)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return result, err
+}
+
 func (s *OpenTracingLayerChannelStore) UpdateMembersRole(channelID string, userIDs []string) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.UpdateMembersRole")
--- a/store/retrylayer/retrylayer.go
+++ b/store/retrylayer/retrylayer.go
@@ -2348,6 +2348,26 @@ func (s *RetryLayerChannelStore) UpdateMember(member *model.ChannelMember) (*mod

 }

+func (s *RetryLayerChannelStore) UpdateMemberNotifyProps(channelID string, userID string, props map[string]string) (*model.ChannelMember, error) {
+
+	tries := 0
+	for {
+		result, err := s.ChannelStore.UpdateMemberNotifyProps(channelID, userID, props)
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
+
+}
+
 func (s *RetryLayerChannelStore) UpdateMembersRole(channelID string, userIDs []string) error {

 	tries := 0
--- a/store/sqlstore/channel_store.go
+++ b/store/sqlstore/channel_store.go
@@ -1632,6 +1632,52 @@ func (s SqlChannelStore) UpdateMember(member *model.ChannelMember) (*model.Chann
 	return updatedMembers[0], nil
 }

+func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props map[string]string) (*model.ChannelMember, error) {
+	tx, err := s.GetMaster().Begin()
+	if err != nil {
+		return nil, errors.Wrap(err, "begin_transaction")
+	}
+	defer finalizeTransaction(tx)
+
+	if s.DriverName() == model.DatabaseDriverPostgres {
+		_, err = tx.Exec(`UPDATE channelmembers
+			SET notifyprops = notifyprops || $1::jsonb
+			WHERE userid=$2 AND channelid=$3`, model.MapToJson(props), userID, channelID)
+	} else {
+		// It's difficult to construct a SQL query for MySQL
+		// to handle a case of empty map. So we just ignore it.
+		if len(props) > 0 {
+			// unpack the keys and values to pass to MySQL.
+			args, argString := constructMySQLJSONArgs(props)
+			args = append(args, userID, channelID)
+
+			// Example: UPDATE ChannelMembers
+			// SET NotifyProps = JSON_SET(NotifyProps, '$.mark_unread', '"yes"' [, ...])
+			// WHERE ...
+			_, err = tx.Exec(`UPDATE ChannelMembers
+				SET NotifyProps = JSON_SET(NotifyProps, `+argString+`)
+				WHERE UserId=? AND ChannelId=?`, args...)
+		}
+	}
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to update ChannelMember with channelID=%s and userID=%s", channelID, userID)
+	}
+
+	var dbMember channelMemberWithSchemeRoles
+	if err2 := tx.SelectOne(&dbMember, ChannelMembersWithSchemeSelectQuery+"WHERE ChannelMembers.ChannelId = :ChannelId AND ChannelMembers.UserId = :UserId", map[string]interface{}{"ChannelId": channelID, "UserId": userID}); err2 != nil {
+		if err2 == sql.ErrNoRows {
+			return nil, store.NewErrNotFound("ChannelMember", fmt.Sprintf("channelId=%s, userId=%s", channelID, userID))
+		}
+		return nil, errors.Wrapf(err2, "failed to get ChannelMember with channelId=%s and userId=%s", channelID, userID)
+	}
+
+	if err2 := tx.Commit(); err2 != nil {
+		return nil, errors.Wrap(err2, "commit_transaction")
+	}
+
+	return dbMember.ToModel(), err
+}
+
 func (s SqlChannelStore) GetMembers(channelId string, offset, limit int) (model.ChannelMembers, error) {
 	var dbMembers channelMemberWithSchemeRolesList
 	_, err := s.GetReplica().Select(&dbMembers, ChannelMembersWithSchemeSelectQuery+"WHERE ChannelId = :ChannelId LIMIT :Limit OFFSET :Offset", map[string]interface{}{"ChannelId": channelId, "Limit": limit, "Offset": offset})
--- a/store/sqlstore/utils.go
+++ b/store/sqlstore/utils.go
@@ -89,3 +89,30 @@ func isQuotedWord(s string) bool {

 	return s[0] == '"' && s[len(s)-1] == '"'
 }
+
+// constructMySQLJSONArgs returns the arg list to pass to a query along with
+// the string of placeholders which is needed to be to the JSON_SET function.
+// Use this function in this way:
+// UPDATE Table
+// SET Col = JSON_SET(Col, `+argString+`)
+// WHERE Id=?`, args...)
+// after appending the Id param to the args slice.
+func constructMySQLJSONArgs(props map[string]string) ([]interface{}, string) {
+	if len(props) == 0 {
+		return nil, ""
+	}
+
+	// Unpack the keys and values to pass to MySQL.
+	args := make([]interface{}, 0, len(props))
+	for k, v := range props {
+		args = append(args, "$."+k)
+		args = append(args, v)
+	}
+
+	// We calculate the number of ? to set in the query string.
+	argString := strings.Repeat("?, ", len(props)*2)
+	// Strip off the trailing comma.
+	argString = strings.TrimSuffix(strings.TrimSpace(argString), ",")
+
+	return args, argString
+}
--- a/store/sqlstore/utils_test.go
+++ b/store/sqlstore/utils_test.go
@@ -6,6 +6,7 @@ package sqlstore
 import (
 	"testing"

+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )

@@ -103,3 +104,32 @@ func TestRemoveNonAlphaNumericUnquotedTerms(t *testing.T) {
 		})
 	}
 }
+
+func TestMySQLJSONArgs(t *testing.T) {
+	tests := []struct {
+		props     map[string]string
+		args      []interface{}
+		argString string
+	}{
+		{
+			props: map[string]string{
+				"desktop": "linux",
+				"mobile":  "android",
+				"notify":  "always",
+			},
+			args:      []interface{}{"$.desktop", "linux", "$.mobile", "android", "$.notify", "always"},
+			argString: "?, ?, ?, ?, ?, ?",
+		},
+		{
+			props:     map[string]string{},
+			args:      nil,
+			argString: "",
+		},
+	}
+
+	for _, test := range tests {
+		args, argString := constructMySQLJSONArgs(test.props)
+		assert.ElementsMatch(t, test.args, args)
+		assert.Equal(t, test.argString, argString)
+	}
+}
--- a/store/store.go
+++ b/store/store.go
@@ -194,6 +194,9 @@ type ChannelStore interface {
 	SaveMember(member *model.ChannelMember) (*model.ChannelMember, error)
 	UpdateMember(member *model.ChannelMember) (*model.ChannelMember, error)
 	UpdateMultipleMembers(members []*model.ChannelMember) ([]*model.ChannelMember, error)
+	// UpdateMemberNotifyProps patches the notifyProps field with the given props map.
+	// It replaces existing fields and creates new ones which don't exist.
+	UpdateMemberNotifyProps(channelID, userID string, props map[string]string) (*model.ChannelMember, error)
 	GetMembers(channelID string, offset, limit int) (model.ChannelMembers, error)
 	GetMember(ctx context.Context, channelID string, userID string) (*model.ChannelMember, error)
 	GetChannelMembersTimezones(channelID string) ([]model.StringMap, error)
--- a/store/storetest/channel_store.go
+++ b/store/storetest/channel_store.go
@@ -57,6 +57,7 @@ func TestChannelStore(t *testing.T, ss store.Store, s SqlStore) {
 	t.Run("SaveMember", func(t *testing.T) { testChannelSaveMember(t, ss) })
 	t.Run("SaveMultipleMembers", func(t *testing.T) { testChannelSaveMultipleMembers(t, ss) })
 	t.Run("UpdateMember", func(t *testing.T) { testChannelUpdateMember(t, ss) })
+	t.Run("UpdateMemberNotifyProps", func(t *testing.T) { testChannelUpdateMemberNotifyProps(t, ss) })
 	t.Run("UpdateMultipleMembers", func(t *testing.T) { testChannelUpdateMultipleMembers(t, ss) })
 	t.Run("RemoveMember", func(t *testing.T) { testChannelRemoveMember(t, ss) })
 	t.Run("RemoveMembers", func(t *testing.T) { testChannelRemoveMembers(t, ss) })
@@ -2955,6 +2956,48 @@ func testChannelUpdateMultipleMembers(t *testing.T, ss store.Store) {
 	})
 }

+func testChannelUpdateMemberNotifyProps(t *testing.T, ss store.Store) {
+	u1, err := ss.User().Save(&model.User{Username: model.NewId(), Email: MakeEmail()})
+	require.NoError(t, err)
+	defaultNotifyProps := model.GetDefaultChannelNotifyProps()
+
+	team := &model.Team{
+		DisplayName: "Name",
+		Name:        NewTestId(),
+		Email:       MakeEmail(),
+		Type:        model.TeamOpen,
+	}
+
+	team, nErr := ss.Team().Save(team)
+	require.NoError(t, nErr)
+
+	channel := &model.Channel{
+		DisplayName: "DisplayName",
+		Name:        NewTestId(),
+		Type:        model.ChannelTypeOpen,
+		TeamId:      team.Id,
+	}
+	channel, nErr = ss.Channel().Save(channel, -1)
+	require.NoError(t, nErr)
+	defer func() { ss.Channel().PermanentDelete(channel.Id) }()
+
+	member := &model.ChannelMember{
+		ChannelId:   channel.Id,
+		UserId:      u1.Id,
+		NotifyProps: defaultNotifyProps,
+	}
+	member, nErr = ss.Channel().SaveMember(member)
+	require.NoError(t, nErr)
+
+	props := member.NotifyProps
+	props["hello"] = "world"
+	props[model.DesktopNotifyProp] = model.ChannelNotifyAll
+	member, nErr = ss.Channel().UpdateMemberNotifyProps(member.ChannelId, member.UserId, props)
+	require.NoError(t, nErr)
+	// Verify props.
+	assert.Equal(t, props, member.NotifyProps)
+}
+
 func testChannelRemoveMember(t *testing.T, ss store.Store) {
 	u1, err := ss.User().Save(&model.User{Username: model.NewId(), Email: MakeEmail()})
 	require.NoError(t, err)
--- a/store/storetest/mocks/ChannelStore.go
+++ b/store/storetest/mocks/ChannelStore.go
@@ -1907,6 +1907,29 @@ func (_m *ChannelStore) UpdateMember(member *model.ChannelMember) (*model.Channe
 	return r0, r1
 }

+// UpdateMemberNotifyProps provides a mock function with given fields: channelID, userID, props
+func (_m *ChannelStore) UpdateMemberNotifyProps(channelID string, userID string, props map[string]string) (*model.ChannelMember, error) {
+	ret := _m.Called(channelID, userID, props)
+
+	var r0 *model.ChannelMember
+	if rf, ok := ret.Get(0).(func(string, string, map[string]string) *model.ChannelMember); ok {
+		r0 = rf(channelID, userID, props)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.ChannelMember)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string, string, map[string]string) error); ok {
+		r1 = rf(channelID, userID, props)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // UpdateMembersRole provides a mock function with given fields: channelID, userIDs
 func (_m *ChannelStore) UpdateMembersRole(channelID string, userIDs []string) error {
 	ret := _m.Called(channelID, userIDs)
--- a/store/timerlayer/timerlayer.go
+++ b/store/timerlayer/timerlayer.go
@@ -2046,6 +2046,22 @@ func (s *TimerLayerChannelStore) UpdateMember(member *model.ChannelMember) (*mod
 	return result, err
 }

+func (s *TimerLayerChannelStore) UpdateMemberNotifyProps(channelID string, userID string, props map[string]string) (*model.ChannelMember, error) {
+	start := timemodule.Now()
+
+	result, err := s.ChannelStore.UpdateMemberNotifyProps(channelID, userID, props)
+
+	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.UpdateMemberNotifyProps", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerChannelStore) UpdateMembersRole(channelID string, userIDs []string) error {
 	start := timemodule.Now()