MM-27883 - remove deep copies from the cache layer (#15271)

When we are reading from or putting to the cache, there is no need to deep copy
objects now because the cache is already serialized.

We also delete some lines from tests because the mock store directly returns the pointers
whereas actually they would be returned from the database where a serialization occurs again.

So we would be testing for the wrong thing by unnecessarily keeping the deep copies for DB reads.

https://mattermost.atlassian.net/browse/MM-27883

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

store/localcachelayer/channel_layer.go

@@ -159,8 +159,7 @@ func (s LocalCacheChannelStore) Get(id string, allowFromCache bool) (*model.Chan
 	if allowFromCache {
 		var cacheItem *model.Channel
 		if err := s.rootStore.doStandardReadCache(s.rootStore.channelByIdCache, id, &cacheItem); err == nil {
-			ch := cacheItem.DeepCopy()
+			return cacheItem, nil
-			return ch, nil
 		}
 	}
 

store/localcachelayer/user_layer.go

@@ -75,7 +75,7 @@ func (s LocalCacheUserStore) GetAllProfilesInChannel(channelId string, allowFrom
 	if allowFromCache {
 		var cachedMap map[string]*model.User
 		if err := s.rootStore.doStandardReadCache(s.rootStore.profilesInChannelCache, channelId, &cachedMap); err == nil {
-			return deepCopyUserMap(cachedMap), nil
+			return cachedMap, nil
 		}
 	}
 
@@ -85,7 +85,7 @@ func (s LocalCacheUserStore) GetAllProfilesInChannel(channelId string, allowFrom
 	}
 
 	if allowFromCache {
-		s.rootStore.doStandardAddToCache(s.rootStore.profilesInChannelCache, channelId, deepCopyUserMap(userMap))
+		s.rootStore.doStandardAddToCache(s.rootStore.profilesInChannelCache, channelId, userMap)
 	}
 
 	return userMap, nil
@@ -107,7 +107,7 @@ func (s LocalCacheUserStore) GetProfileByIds(userIds []string, options *store.Us
 		var cacheItem *model.User
 		if err := s.rootStore.doStandardReadCache(s.rootStore.userProfileByIdsCache, userId, &cacheItem); err == nil {
 			if options.Since == 0 || cacheItem.UpdateAt > options.Since {
-				users = append(users, cacheItem.DeepCopy())
+				users = append(users, cacheItem)
 			}
 		} else {
 			remainingUserIds = append(remainingUserIds, userId)
@@ -126,7 +126,7 @@ func (s LocalCacheUserStore) GetProfileByIds(userIds []string, options *store.Us
 		}
 		for _, user := range remainingUsers {
 			s.rootStore.doStandardAddToCache(s.rootStore.userProfileByIdsCache, user.Id, user)
-			users = append(users, user.DeepCopy())
+			users = append(users, user)
 		}
 	}
 
@@ -143,7 +143,7 @@ func (s LocalCacheUserStore) Get(id string) (*model.User, *model.AppError) {
 		if s.rootStore.metrics != nil {
 			s.rootStore.metrics.AddMemCacheHitCounter("Profile By Id", float64(1))
 		}
-		return cacheItem.DeepCopy(), nil
+		return cacheItem, nil
 	}
 	if s.rootStore.metrics != nil {
 		s.rootStore.metrics.AddMemCacheMissCounter("Profile By Id", float64(1))
@@ -152,17 +152,6 @@ func (s LocalCacheUserStore) Get(id string) (*model.User, *model.AppError) {
 	if err != nil {
 		return nil, err
 	}
-	u := user.DeepCopy()
+	s.rootStore.doStandardAddToCache(s.rootStore.userProfileByIdsCache, id, user)
-	s.rootStore.doStandardAddToCache(s.rootStore.userProfileByIdsCache, id, u)
+	return user, nil
-	return user.DeepCopy(), nil
-}
-
-func deepCopyUserMap(users map[string]*model.User) map[string]*model.User {
-	copyOfUsers := make(map[string]*model.User)
-
-	for id, user := range users {
-		copyOfUsers[id] = user.DeepCopy()
-	}
-
-	return copyOfUsers
 }

store/localcachelayer/user_layer_test.go

@@ -91,11 +91,6 @@ func TestUserStoreCache(t *testing.T) {
 
 		for i := 0; i < len(storedUsers); i++ {
 			assert.Equal(t, storedUsers[i].Id, cachedUsers[i].Id)
-			if storedUsers[i] == cachedUsers[i] {
-				assert.Fail(t, "should be different pointers")
-			}
-			cachedUsers[i].NotifyProps["key"] = "othervalue"
-			assert.NotEqual(t, storedUsers[i], cachedUsers[i])
 		}
 
 		cachedUsers, err = cachedStore.User().GetProfileByIds(fakeUserIds, &store.UserGetByIdsOpts{}, true)
@@ -237,11 +232,6 @@ func TestUserStoreGetCache(t *testing.T) {
 		cachedUser, err := cachedStore.User().Get(fakeUserId)
 		require.Nil(t, err)
 		assert.Equal(t, storedUser, cachedUser)
-		if storedUser == cachedUser {
-			assert.Fail(t, "should be different pointers")
-		}
-		cachedUser.NotifyProps["key"] = "othervalue"
-		assert.NotEqual(t, storedUser, cachedUser)
 
 		storedUser.Props = model.StringMap{}
 		storedUser.Timezone = model.StringMap{}