[GH-10866] Preferrence.Save - Remove preference length return arg (#10962)

2019-05-29 19:24:57 +05:30
--- a/store/sqlstore/preference_store.go
+++ b/store/sqlstore/preference_store.go
@@ -54,25 +54,25 @@ func (s SqlPreferenceStore) DeleteUnusedFeatures() {
 	s.GetMaster().Exec(sql, queryParams)
 }

-func (s SqlPreferenceStore) Save(preferences *model.Preferences) (int, *model.AppError) {
+func (s SqlPreferenceStore) Save(preferences *model.Preferences) *model.AppError {
 	// wrap in a transaction so that if one fails, everything fails
 	transaction, err := s.GetMaster().Begin()
 	if err != nil {
-		return 0, model.NewAppError("SqlPreferenceStore.Save", "store.sql_preference.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return model.NewAppError("SqlPreferenceStore.Save", "store.sql_preference.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}

 	defer finalizeTransaction(transaction)
 	for _, preference := range *preferences {
 		if upsertResult := s.save(transaction, &preference); upsertResult.Err != nil {
-			return 0, upsertResult.Err
+			return upsertResult.Err
 		}
 	}

 	if err := transaction.Commit(); err != nil {
 		// don't need to rollback here since the transaction is already closed
-		return 0, model.NewAppError("SqlPreferenceStore.Save", "store.sql_preference.save.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return model.NewAppError("SqlPreferenceStore.Save", "store.sql_preference.save.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
-	return len(*preferences), nil
+	return nil
 }

 func (s SqlPreferenceStore) save(transaction *gorp.Transaction, preference *model.Preference) store.StoreResult {
--- a/store/sqlstore/preference_store_test.go
+++ b/store/sqlstore/preference_store_test.go
@@ -52,9 +52,8 @@ func TestDeleteUnusedFeatures(t *testing.T) {
 			},
 		}

-		count, err := ss.Preference().Save(&features)
+		err := ss.Preference().Save(&features)
 		require.Nil(t, err)
-		require.Equal(t, 4, count)

 		ss.Preference().(*SqlPreferenceStore).DeleteUnusedFeatures()

--- a/store/store.go
+++ b/store/store.go
@@ -428,7 +428,7 @@ type CommandWebhookStore interface {
 }

 type PreferenceStore interface {
-	Save(preferences *model.Preferences) (int, *model.AppError)
+	Save(preferences *model.Preferences) *model.AppError
 	GetCategory(userId string, category string) (model.Preferences, *model.AppError)
 	Get(userId string, category string, name string) (*model.Preference, *model.AppError)
 	GetAll(userId string) StoreChannel
--- a/store/storetest/mocks/PreferenceStore.go
+++ b/store/storetest/mocks/PreferenceStore.go
@@ -190,24 +190,17 @@ func (_m *PreferenceStore) PermanentDeleteByUser(userId string) *model.AppError
 }

 // Save provides a mock function with given fields: preferences
-func (_m *PreferenceStore) Save(preferences *model.Preferences) (int, *model.AppError) {
+func (_m *PreferenceStore) Save(preferences *model.Preferences) *model.AppError {
 	ret := _m.Called(preferences)

-	var r0 int
-	if rf, ok := ret.Get(0).(func(*model.Preferences) int); ok {
+	var r0 *model.AppError
+	if rf, ok := ret.Get(0).(func(*model.Preferences) *model.AppError); ok {
 		r0 = rf(preferences)
 	} else {
-		r0 = ret.Get(0).(int)
-	}
-
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(*model.Preferences) *model.AppError); ok {
-		r1 = rf(preferences)
-	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.AppError)
 		}
 	}

-	return r0, r1
+	return r0
 }
--- a/store/storetest/oauth_store.go
+++ b/store/storetest/oauth_store.go
@@ -335,9 +335,8 @@ func testOAuthGetAuthorizedApps(t *testing.T, ss store.Store) {
 	p.Category = model.PREFERENCE_CATEGORY_AUTHORIZED_OAUTH_APP
 	p.Name = a1.Id
 	p.Value = "true"
-	count, err := ss.Preference().Save(&model.Preferences{p})
+	err := ss.Preference().Save(&model.Preferences{p})
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	if result := <-ss.OAuth().GetAuthorizedApps(a1.CreatorId, 0, 1000); result.Err != nil {
 		t.Fatal(result.Err)
@@ -363,9 +362,8 @@ func testOAuthGetAccessDataByUserForApp(t *testing.T, ss store.Store) {
 	p.Category = model.PREFERENCE_CATEGORY_AUTHORIZED_OAUTH_APP
 	p.Name = a1.Id
 	p.Value = "true"
-	count, err := ss.Preference().Save(&model.Preferences{p})
+	err := ss.Preference().Save(&model.Preferences{p})
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	if result := <-ss.OAuth().GetAuthorizedApps(a1.CreatorId, 0, 1000); result.Err != nil {
 		t.Fatal(result.Err)
--- a/store/storetest/post_store.go
+++ b/store/storetest/post_store.go
@@ -1393,9 +1393,8 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlSupp
 		},
 	}

-	count, err := ss.Preference().Save(&preferences)
+	err = ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r2 := (<-ss.Post().GetFlaggedPostsForTeam(o1.UserId, c1.TeamId, 0, 2)).Data.(*model.PostList)

@@ -1412,9 +1411,8 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlSupp
 		},
 	}

-	count, err = ss.Preference().Save(&preferences)
+	err = ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r3 := (<-ss.Post().GetFlaggedPostsForTeam(o1.UserId, c1.TeamId, 0, 1)).Data.(*model.PostList)

@@ -1449,9 +1447,8 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlSupp
 		},
 	}

-	count, err = ss.Preference().Save(&preferences)
+	err = ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r4 = (<-ss.Post().GetFlaggedPostsForTeam(o1.UserId, c1.TeamId, 0, 2)).Data.(*model.PostList)

@@ -1467,9 +1464,8 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlSupp
 			Value:    "true",
 		},
 	}
-	count, err = ss.Preference().Save(&preferences)
+	err = ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r4 = (<-ss.Post().GetFlaggedPostsForTeam(o1.UserId, c1.TeamId, 0, 2)).Data.(*model.PostList)

@@ -1491,9 +1487,8 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlSupp
 			Value:    "true",
 		},
 	}
-	count, err = ss.Preference().Save(&preferences)
+	err = ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r4 = (<-ss.Post().GetFlaggedPostsForTeam(o1.UserId, c1.TeamId, 0, 10)).Data.(*model.PostList)

@@ -1543,9 +1538,8 @@ func testPostStoreGetFlaggedPosts(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err := ss.Preference().Save(&preferences)
+	err := ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r2 := (<-ss.Post().GetFlaggedPosts(o1.UserId, 0, 2)).Data.(*model.PostList)

@@ -1562,9 +1556,8 @@ func testPostStoreGetFlaggedPosts(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err = ss.Preference().Save(&preferences)
+	err = ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r3 := (<-ss.Post().GetFlaggedPosts(o1.UserId, 0, 1)).Data.(*model.PostList)

@@ -1599,9 +1592,8 @@ func testPostStoreGetFlaggedPosts(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err = ss.Preference().Save(&preferences)
+	err = ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r4 = (<-ss.Post().GetFlaggedPosts(o1.UserId, 0, 2)).Data.(*model.PostList)

@@ -1654,9 +1646,8 @@ func testPostStoreGetFlaggedPostsForChannel(t *testing.T, ss store.Store) {
 		Value:    "true",
 	}

-	count, err := ss.Preference().Save(&model.Preferences{preference})
+	err := ss.Preference().Save(&model.Preferences{preference})
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r = (<-ss.Post().GetFlaggedPostsForChannel(o1.UserId, o1.ChannelId, 0, 10)).Data.(*model.PostList)

@@ -1665,14 +1656,12 @@ func testPostStoreGetFlaggedPostsForChannel(t *testing.T, ss store.Store) {
 	}

 	preference.Name = o2.Id
-	count, err = ss.Preference().Save(&model.Preferences{preference})
+	err = ss.Preference().Save(&model.Preferences{preference})
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	preference.Name = o3.Id
-	count, err = ss.Preference().Save(&model.Preferences{preference})
+	err = ss.Preference().Save(&model.Preferences{preference})
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r = (<-ss.Post().GetFlaggedPostsForChannel(o1.UserId, o1.ChannelId, 0, 1)).Data.(*model.PostList)

@@ -1699,9 +1688,8 @@ func testPostStoreGetFlaggedPostsForChannel(t *testing.T, ss store.Store) {
 	}

 	preference.Name = o4.Id
-	count, err = ss.Preference().Save(&model.Preferences{preference})
+	err = ss.Preference().Save(&model.Preferences{preference})
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	r = (<-ss.Post().GetFlaggedPostsForChannel(o1.UserId, o4.ChannelId, 0, 10)).Data.(*model.PostList)

--- a/store/storetest/preference_store.go
+++ b/store/storetest/preference_store.go
@@ -44,9 +44,7 @@ func testPreferenceSave(t *testing.T, ss store.Store) {
 			Value:    "value1b",
 		},
 	}
-	if count, err := ss.Preference().Save(&preferences); count != 2 {
-		t.Fatal("got incorrect number of rows saved")
-	} else if err != nil {
+	if err := ss.Preference().Save(&preferences); err != nil {
 		t.Fatal("saving preference returned error")
 	}

@@ -58,9 +56,7 @@ func testPreferenceSave(t *testing.T, ss store.Store) {

 	preferences[0].Value = "value2a"
 	preferences[1].Value = "value2b"
-	if count, err := ss.Preference().Save(&preferences); count != 2 {
-		t.Fatal("got incorrect number of rows saved")
-	} else if err != nil {
+	if err := ss.Preference().Save(&preferences); err != nil {
 		t.Fatal("saving preference returned error")
 	}

@@ -99,9 +95,8 @@ func testPreferenceGet(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err := ss.Preference().Save(&preferences)
+	err := ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 4, count)

 	if data, err := ss.Preference().Get(userId, category, name); err != nil {
 		t.Fatal(err)
@@ -146,9 +141,8 @@ func testPreferenceGetCategory(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err := ss.Preference().Save(&preferences)
+	err := ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 4, count)

 	if preferencesByCategory, err := ss.Preference().GetCategory(userId, category); err != nil {
 		t.Fatal(err)
@@ -197,9 +191,8 @@ func testPreferenceGetAll(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err := ss.Preference().Save(&preferences)
+	err := ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 4, count)

 	if result := <-ss.Preference().GetAll(userId); result.Err != nil {
 		t.Fatal(result.Err)
@@ -245,9 +238,8 @@ func testPreferenceDeleteByUser(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err := ss.Preference().Save(&preferences)
+	err := ss.Preference().Save(&preferences)
 	require.Nil(t, err)
-	require.Equal(t, 4, count)

 	if err := ss.Preference().PermanentDeleteByUser(userId); err != nil {
 		t.Fatal(err)
@@ -295,9 +287,8 @@ func testIsFeatureEnabled(t *testing.T, ss store.Store) {
 		},
 	}

-	count, err := ss.Preference().Save(&features)
+	err := ss.Preference().Save(&features)
 	require.Nil(t, err)
-	require.Equal(t, 5, count)

 	if data, err := ss.Preference().IsFeatureEnabled(feature1, userId); err != nil {
 		t.Fatal(err)
@@ -334,9 +325,8 @@ func testPreferenceDelete(t *testing.T, ss store.Store) {
 		Value:    "value1a",
 	}

-	count, err := ss.Preference().Save(&model.Preferences{preference})
+	err := ss.Preference().Save(&model.Preferences{preference})
 	require.Nil(t, err)
-	require.Equal(t, 1, count)

 	if prefs := store.Must(ss.Preference().GetAll(preference.UserId)).(model.Preferences); len([]model.Preference(prefs)) != 1 {
 		t.Fatal("should've returned 1 preference")
@@ -369,9 +359,8 @@ func testPreferenceDeleteCategory(t *testing.T, ss store.Store) {
 		Value:    "value1a",
 	}

-	count, err := ss.Preference().Save(&model.Preferences{preference1, preference2})
+	err := ss.Preference().Save(&model.Preferences{preference1, preference2})
 	require.Nil(t, err)
-	require.Equal(t, 2, count)

 	if prefs := store.Must(ss.Preference().GetAll(userId)).(model.Preferences); len([]model.Preference(prefs)) != 2 {
 		t.Fatal("should've returned 2 preferences")
@@ -406,9 +395,8 @@ func testPreferenceDeleteCategoryAndName(t *testing.T, ss store.Store) {
 		Value:    "value1a",
 	}

-	count, err := ss.Preference().Save(&model.Preferences{preference1, preference2})
+	err := ss.Preference().Save(&model.Preferences{preference1, preference2})
 	require.Nil(t, err)
-	require.Equal(t, 2, count)

 	if prefs := store.Must(ss.Preference().GetAll(userId)).(model.Preferences); len([]model.Preference(prefs)) != 1 {
 		t.Fatal("should've returned 1 preference")
@@ -456,9 +444,8 @@ func testPreferenceCleanupFlagsBatch(t *testing.T, ss store.Store) {
 		Value:    "true",
 	}

-	count, err := ss.Preference().Save(&model.Preferences{preference1, preference2})
+	err := ss.Preference().Save(&model.Preferences{preference1, preference2})
 	require.Nil(t, err)
-	require.Equal(t, 2, count)

 	_, err = ss.Preference().CleanupFlagsBatch(10000)
 	assert.Nil(t, err)