From ff1ea0599e0b52a2036c0a91c6b9d28079e53e71 Mon Sep 17 00:00:00 2001 From: Nick Misasi Date: Wed, 23 Nov 2022 14:06:29 -0500 Subject: [PATCH] [MM-48560] LastAccessiblePostTime not removed on upgrade to Professional (#21708) * Delete system value for LastAccessibleFileTime and LastAccessiblePostTime if the limits are 0 and a value is set * Update tests --- app/file.go | 22 +++++++++++++++ app/file_test.go | 71 ++++++++++++++++++++++++++++++++++-------------- app/post.go | 18 ++++++++++++ app/post_test.go | 6 ++-- 4 files changed, 95 insertions(+), 22 deletions(-) diff --git a/app/file.go b/app/file.go index 341c5f655e..2f0050dff6 100644 --- a/app/file.go +++ b/app/file.go @@ -1380,6 +1380,28 @@ func (a *App) ComputeLastAccessibleFileTime() error { return appErr } + if limit == 0 { + // All files are accessible - we must check if a previous value was set so we can clear it + systemValue, err := a.Srv().Store().System().GetByName(model.SystemLastAccessibleFileTime) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + // All files are already accessible + return nil + default: + return model.NewAppError("ComputeLastAccessibleFileTime", "app.system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + if systemValue != nil { + // Previous value was set, so we must clear it + if _, err := a.Srv().Store().System().PermanentDeleteByName(model.SystemLastAccessibleFileTime); err != nil { + return model.NewAppError("ComputeLastAccessibleFileTime", "app.system.permanent_delete_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + return nil + } + createdAt, err := a.Srv().GetStore().FileInfo().GetUptoNSizeFileTime(limit) if err != nil { var nfErr *store.ErrNotFound diff --git a/app/file_test.go b/app/file_test.go index 09c03e640e..b781af6483 100644 --- a/app/file_test.go +++ b/app/file_test.go @@ -591,30 +591,61 @@ func TestGetLastAccessibleFileTime(t *testing.T) { } func TestComputeLastAccessibleFileTime(t *testing.T) { - th := SetupWithStoreMock(t) - defer th.TearDown() + t.Run("Updates the time, if cloud limit is applicable", func(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() - th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) - cloud := &eMocks.CloudInterface{} - th.App.Srv().Cloud = cloud + cloud := &eMocks.CloudInterface{} + th.App.Srv().Cloud = cloud - cloud.Mock.On("GetCloudLimits", mock.Anything).Return(&model.ProductLimits{ - Files: &model.FilesLimits{ - TotalStorage: model.NewInt64(1), - }, - }, nil) + cloud.Mock.On("GetCloudLimits", mock.Anything).Return(&model.ProductLimits{ + Files: &model.FilesLimits{ + TotalStorage: model.NewInt64(1), + }, + }, nil) - mockStore := th.App.Srv().Store().(*storemocks.Store) - mockFileStore := storemocks.FileInfoStore{} - mockFileStore.On("GetUptoNSizeFileTime", mock.Anything).Return(int64(1), nil) - mockSystemStore := storemocks.SystemStore{} - mockSystemStore.On("SaveOrUpdate", mock.Anything).Return(nil) - mockStore.On("FileInfo").Return(&mockFileStore) - mockStore.On("System").Return(&mockSystemStore) + mockStore := th.App.Srv().Store().(*storemocks.Store) + mockFileStore := storemocks.FileInfoStore{} + mockFileStore.On("GetUptoNSizeFileTime", mock.Anything).Return(int64(1), nil) + mockSystemStore := storemocks.SystemStore{} + mockSystemStore.On("SaveOrUpdate", mock.Anything).Return(nil) + mockStore.On("FileInfo").Return(&mockFileStore) + mockStore.On("System").Return(&mockSystemStore) - err := th.App.ComputeLastAccessibleFileTime() - require.NoError(t, err) + err := th.App.ComputeLastAccessibleFileTime() + require.NoError(t, err) - mockSystemStore.AssertCalled(t, "SaveOrUpdate", mock.Anything) + mockSystemStore.AssertCalled(t, "SaveOrUpdate", mock.Anything) + }) + + t.Run("Removes the time, if cloud limit is not applicable", func(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + + cloud := &eMocks.CloudInterface{} + th.App.Srv().Cloud = cloud + + cloud.Mock.On("GetCloudLimits", mock.Anything).Return(nil, nil) + + mockStore := th.App.Srv().Store().(*storemocks.Store) + mockFileStore := storemocks.FileInfoStore{} + mockFileStore.On("GetUptoNSizeFileTime", mock.Anything).Return(int64(1), nil) + mockSystemStore := storemocks.SystemStore{} + mockSystemStore.On("GetByName", mock.Anything).Return(&model.System{Name: model.SystemLastAccessibleFileTime, Value: "10"}, nil) + mockSystemStore.On("PermanentDeleteByName", mock.Anything).Return(nil, nil) + mockSystemStore.On("SaveOrUpdate", mock.Anything).Return(nil) + mockStore.On("FileInfo").Return(&mockFileStore) + mockStore.On("System").Return(&mockSystemStore) + + err := th.App.ComputeLastAccessibleFileTime() + require.NoError(t, err) + + mockSystemStore.AssertNotCalled(t, "SaveOrUpdate", mock.Anything) + mockSystemStore.AssertCalled(t, "PermanentDeleteByName", mock.Anything) + + }) } diff --git a/app/post.go b/app/post.go index 78d14b16ef..301c5e9c2f 100644 --- a/app/post.go +++ b/app/post.go @@ -1441,6 +1441,24 @@ func (a *App) ComputeLastAccessiblePostTime() error { } if limit == 0 { + // All posts are accessible - we must check if a previous value was set so we can clear it + systemValue, err := a.Srv().Store().System().GetByName(model.SystemLastAccessiblePostTime) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + // There was no previous value, nothing to do + return nil + default: + return model.NewAppError("ComputeLastAccessiblePostTime", "app.system.get_by_name.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + } + if systemValue != nil { + // Previous value was set, so we must clear it + if _, err = a.Srv().Store().System().PermanentDeleteByName(model.SystemLastAccessiblePostTime); err != nil { + return model.NewAppError("ComputeLastAccessiblePostTime", "app.system.permanent_delete_by_name.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + } // Cloud limit is not applicable return nil } diff --git a/app/post_test.go b/app/post_test.go index 2ed0092a6c..74744e2363 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -2887,7 +2887,7 @@ func TestComputeLastAccessiblePostTime(t *testing.T) { mockSystemStore.AssertCalled(t, "SaveOrUpdate", mock.Anything) }) - t.Run("Do NOT update the time, if cloud limit is NOT applicable", func(t *testing.T) { + t.Run("Remove the time if cloud limit is NOT applicable", func(t *testing.T) { th := SetupWithStoreMock(t) defer th.TearDown() @@ -2901,13 +2901,15 @@ func TestComputeLastAccessiblePostTime(t *testing.T) { mockStore := th.App.Srv().Store().(*storemocks.Store) mockSystemStore := storemocks.SystemStore{} - mockSystemStore.On("SaveOrUpdate", mock.Anything).Return(nil) + mockSystemStore.On("GetByName", mock.Anything).Return(&model.System{Name: model.SystemLastAccessiblePostTime, Value: "10"}, nil) + mockSystemStore.On("PermanentDeleteByName", mock.Anything).Return(nil, nil) mockStore.On("System").Return(&mockSystemStore) err := th.App.ComputeLastAccessiblePostTime() assert.NoError(t, err) mockSystemStore.AssertNotCalled(t, "SaveOrUpdate", mock.Anything) + mockSystemStore.AssertCalled(t, "PermanentDeleteByName", mock.Anything) }) }