From bef07ce118cbed1e41ff4108360f4f3a36d73621 Mon Sep 17 00:00:00 2001 From: Sergey Ostrovskiy Date: Wed, 8 Nov 2023 23:05:29 +0300 Subject: [PATCH] add delete secret to sync --- .../infrastrucrure/filemanager/filemanager.go | 30 ++++++ .../filemanager/filemanager_test.go | 4 +- .../infrastrucrure/filemanager/mocks/mock.go | 16 ++- .../filemanager/userauth_test.go | 15 +-- .../client/infrastrucrure/ui/mocks/mock.go | 10 +- internal/client/usecase/create_test.go | 2 +- internal/client/usecase/delete_test.go | 2 +- internal/client/usecase/get_test.go | 2 +- internal/client/usecase/sync.go | 34 ++++-- internal/client/usecase/sync_test.go | 100 ++++++++++++++++++ internal/client/usecase/update_test.go | 2 +- internal/client/usecase/usecase.go | 2 +- internal/models/usermeta.go | 6 ++ .../middleware_grpc_controller_test.go | 20 ++-- 14 files changed, 205 insertions(+), 40 deletions(-) diff --git a/internal/client/infrastrucrure/filemanager/filemanager.go b/internal/client/infrastrucrure/filemanager/filemanager.go index 81b8e77..e74d37b 100644 --- a/internal/client/infrastrucrure/filemanager/filemanager.go +++ b/internal/client/infrastrucrure/filemanager/filemanager.go @@ -31,6 +31,7 @@ type FileStorage interface { ReadEncryptedByName(name string) (chan []byte, error) UpdateDataByName(name string, data Data) error UpdateInfoByName(name string, info models.DataInfo) error + DeleteByIDs(ids []string) error DeleteByName(name string) error } @@ -213,6 +214,35 @@ func (fm *FileManager) DeleteByName(name string) error { return fm.saveMetaData() } +func (fm *FileManager) DeleteByIDs(ids []string) error { + var delErr error + for _, id := range ids { + if _, err := os.Stat(filepath.Join(fm.storageDir, id)); !os.IsNotExist(err) { + err := os.Remove(filepath.Join(fm.storageDir, id)) + if err != nil { + delErr = fmt.Errorf("%w, %s: %w", delErr, id, err) + } + } + + finder: + for name, info := range fm.Meta.Data { + if info.DataID == id { + delete(fm.Meta.Data, name) + + break finder + } + } + } + if err := fm.saveMetaData(); err != nil { + delErr = fmt.Errorf("%w, save meta err: %w", delErr, err) + } + if delErr != nil { + return fmt.Errorf("%w", delErr) + } + + return nil +} + func (fm *FileManager) saveMetaData() error { data, err := json.Marshal(fm.Meta) if err != nil { diff --git a/internal/client/infrastrucrure/filemanager/filemanager_test.go b/internal/client/infrastrucrure/filemanager/filemanager_test.go index 984c859..7f5ccf6 100644 --- a/internal/client/infrastrucrure/filemanager/filemanager_test.go +++ b/internal/client/infrastrucrure/filemanager/filemanager_test.go @@ -489,7 +489,7 @@ func TestFileManager_DeleteByName(t *testing.T) { { name: "ok", storage: func() filemanager.FileStorage { - meta := models.UserMeta{Data: make(models.MetaData)} + meta := models.UserMeta{Data: make(models.MetaData), DeletedData: make(models.Deleted)} testInfo := info testInfo.DataID = filename meta.Data["testData"] = testInfo @@ -509,7 +509,7 @@ func TestFileManager_DeleteByName(t *testing.T) { { name: "failed if secret not exist", storage: func() filemanager.FileStorage { - meta := models.UserMeta{Data: make(models.MetaData)} + meta := models.UserMeta{Data: make(models.MetaData), DeletedData: make(models.Deleted)} fs, err := filemanager.NewFileManager(storageDir, userDir, userDir, meta, testKey) if err != nil { t.Fatalf("Failed to create FileManager: %v", err) diff --git a/internal/client/infrastrucrure/filemanager/mocks/mock.go b/internal/client/infrastrucrure/filemanager/mocks/mock.go index 095fc2e..d0f5ca3 100644 --- a/internal/client/infrastrucrure/filemanager/mocks/mock.go +++ b/internal/client/infrastrucrure/filemanager/mocks/mock.go @@ -7,9 +7,9 @@ package mock_filemanager import ( reflect "reflect" + gomock "github.com/golang/mock/gomock" filemanager "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager" models "github.com/kripsy/GophKeeper/internal/models" - gomock "go.uber.org/mock/gomock" ) // MockFileStorage is a mock of FileStorage interface. @@ -63,6 +63,20 @@ func (mr *MockFileStorageMockRecorder) AddToStorage(name, data, info interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddToStorage", reflect.TypeOf((*MockFileStorage)(nil).AddToStorage), name, data, info) } +// DeleteByIOs mocks base method. +func (m *MockFileStorage) DeleteByIDs(ids []string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteByIDs", ids) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteByIOs indicates an expected call of DeleteByIOs. +func (mr *MockFileStorageMockRecorder) DeleteByIOs(ids interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteByIDs", reflect.TypeOf((*MockFileStorage)(nil).DeleteByIDs), ids) +} + // DeleteByName mocks base method. func (m *MockFileStorage) DeleteByName(name string) error { m.ctrl.T.Helper() diff --git a/internal/client/infrastrucrure/filemanager/userauth_test.go b/internal/client/infrastrucrure/filemanager/userauth_test.go index b5b3837..f1d20b2 100644 --- a/internal/client/infrastrucrure/filemanager/userauth_test.go +++ b/internal/client/infrastrucrure/filemanager/userauth_test.go @@ -38,8 +38,9 @@ func Test_userAuth_GetUser(t *testing.T) { t.Fatalf("Failed prepare user: %v", err) } }, - user: &models.User{Username: Login, Password: Password}, - want: models.UserMeta{Username: Login, IsSyncStorage: true, Data: make(models.MetaData)}, + user: &models.User{Username: Login, Password: Password}, + want: models.UserMeta{Username: Login, IsSyncStorage: true, + Data: make(models.MetaData), DeletedData: make(models.Deleted)}, wantErr: false, }, { @@ -64,7 +65,6 @@ func Test_userAuth_GetUser(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tt.prepareFunc() - got, err := tt.auth.GetUser(tt.user) if (err != nil) != tt.wantErr { t.Errorf("GetUser() error = %v, wantErr %v", err, tt.wantErr) @@ -93,10 +93,11 @@ func Test_userAuth_CreateUser(t *testing.T) { wantErr bool }{ { - name: "ok", - auth: auth, - user: &models.User{Username: Login, Password: Password}, - want: models.UserMeta{Username: Login, IsSyncStorage: true, Data: make(models.MetaData)}, + name: "ok", + auth: auth, + user: &models.User{Username: Login, Password: Password}, + want: models.UserMeta{Username: Login, IsSyncStorage: true, + Data: make(models.MetaData), DeletedData: make(models.Deleted)}, IsSyncStorage: true, wantErr: false, }, diff --git a/internal/client/infrastrucrure/ui/mocks/mock.go b/internal/client/infrastrucrure/ui/mocks/mock.go index 4ed6beb..212e7da 100644 --- a/internal/client/infrastrucrure/ui/mocks/mock.go +++ b/internal/client/infrastrucrure/ui/mocks/mock.go @@ -7,9 +7,9 @@ package mock_ui import ( reflect "reflect" + gomock "github.com/golang/mock/gomock" filemanager "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager" models "github.com/kripsy/GophKeeper/internal/models" - gomock "go.uber.org/mock/gomock" ) // MockUserInterface is a mock of UserInterface interface. @@ -223,17 +223,17 @@ func (mr *MockUserInterfaceMockRecorder) IsLocalStorage() *gomock.Call { } // Menu mocks base method. -func (m *MockUserInterface) Menu(isLocalStorage bool) int { +func (m *MockUserInterface) Menu(isSyncStorage bool) int { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Menu", isLocalStorage) + ret := m.ctrl.Call(m, "Menu", isSyncStorage) ret0, _ := ret[0].(int) return ret0 } // Menu indicates an expected call of Menu. -func (mr *MockUserInterfaceMockRecorder) Menu(isLocalStorage interface{}) *gomock.Call { +func (mr *MockUserInterfaceMockRecorder) Menu(isSyncStorage interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Menu", reflect.TypeOf((*MockUserInterface)(nil).Menu), isLocalStorage) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Menu", reflect.TypeOf((*MockUserInterface)(nil).Menu), isSyncStorage) } // PrintErr mocks base method. diff --git a/internal/client/usecase/create_test.go b/internal/client/usecase/create_test.go index 7b91a0c..f1a100f 100644 --- a/internal/client/usecase/create_test.go +++ b/internal/client/usecase/create_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/golang/mock/gomock" "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager" mock_filemanager "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager/mocks" "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/ui" @@ -13,7 +14,6 @@ import ( "github.com/kripsy/GophKeeper/internal/client/permissions" "github.com/kripsy/GophKeeper/internal/models" "github.com/rs/zerolog" - "go.uber.org/mock/gomock" ) func TestClientUsecase_createSecret(t *testing.T) { diff --git a/internal/client/usecase/delete_test.go b/internal/client/usecase/delete_test.go index a576c07..0cd9cc7 100644 --- a/internal/client/usecase/delete_test.go +++ b/internal/client/usecase/delete_test.go @@ -6,12 +6,12 @@ import ( "os" "testing" + "github.com/golang/mock/gomock" mock_filemanager "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager/mocks" "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/ui" mock_ui "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/ui/mocks" "github.com/kripsy/GophKeeper/internal/models" "github.com/rs/zerolog" - "go.uber.org/mock/gomock" ) func TestClientUsecase_deleteSecret(t *testing.T) { diff --git a/internal/client/usecase/get_test.go b/internal/client/usecase/get_test.go index 6c52c08..19fd8bf 100644 --- a/internal/client/usecase/get_test.go +++ b/internal/client/usecase/get_test.go @@ -7,13 +7,13 @@ import ( "os" "testing" + "github.com/golang/mock/gomock" "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager" mock_filemanager "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager/mocks" "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/ui" mock_ui "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/ui/mocks" "github.com/kripsy/GophKeeper/internal/models" "github.com/rs/zerolog" - "go.uber.org/mock/gomock" ) func TestClientUsecase_getSecrets(t *testing.T) { diff --git a/internal/client/usecase/sync.go b/internal/client/usecase/sync.go index 323e480..323a0ff 100644 --- a/internal/client/usecase/sync.go +++ b/internal/client/usecase/sync.go @@ -64,6 +64,8 @@ func (c *ClientUsecase) synchronizeWithServer(ctx context.Context, syncKey strin return nil } + defer c.syncDeletedSecret(serverMeta.DeletedData) + toDownload, toUpload := findDifferences(c.userData.Meta, *serverMeta) if len(toUpload) == 0 && len(toDownload) == 0 { return nil @@ -101,14 +103,21 @@ func (c *ClientUsecase) uploadMeta(ctx context.Context, syncKey string) error { } func (c *ClientUsecase) syncDeletedSecret(deleted models.Deleted) { - // var needDeleted []string + var needDeleted []string for dataID := range deleted { if _, ok := c.userData.Meta.DeletedData[dataID]; !ok { c.userData.Meta.DeletedData[dataID] = struct{}{} - // needDeleted = append(needDeleted, dataID) + needDeleted = append(needDeleted, dataID) } } + if len(needDeleted) == 0 { + return + } + + if err := c.fileManager.DeleteByIDs(needDeleted); err != nil { + c.log.Err(err).Msg("failed sync deleted secret") + } } func (c *ClientUsecase) uploadSecrets(ctx context.Context, syncKey string, toUpload models.MetaData) error { @@ -255,23 +264,28 @@ func findDifferences(local, server models.UserMeta) (needDownload, needUpload mo serverData[data.DataID] = data } - // проверяем данные сервера, если данные не обнаружены или устарели добавляем в список на скачивание + // проверяем данные сервера, пропускаем удаленные локально. + // если данные не обнаружены или устарели добавляем в список на скачивание for dataID, sData := range serverData { + if local.DeletedData.IsDeleted(dataID) { + continue + } lData, found := localData[dataID] if !found || lData.UpdatedAt.Before(sData.UpdatedAt) { - if _, deleted := local.DeletedData[dataID]; !deleted { - needDownload[dataID] = sData - } + needDownload[dataID] = sData } } - // проверяем локальные данные, если данные не обнаружены или устарели добавляем в список на выгрузку + // проверяем локальные данные, пропускаем удаленные на сервере. + // если данные не обнаружены или устарели добавляем в список на выгрузку for dataID, lData := range localData { + if server.DeletedData.IsDeleted(dataID) { + continue + } + sData, found := serverData[dataID] if !found || sData.UpdatedAt.Before(lData.UpdatedAt) { - if _, deleted := server.DeletedData[dataID]; !deleted { - needUpload[dataID] = lData - } + needUpload[dataID] = lData } } diff --git a/internal/client/usecase/sync_test.go b/internal/client/usecase/sync_test.go index 3be248a..09261fa 100644 --- a/internal/client/usecase/sync_test.go +++ b/internal/client/usecase/sync_test.go @@ -1,2 +1,102 @@ //nolint:testpackage package usecase + +import ( + "reflect" + "testing" + "time" + + "github.com/kripsy/GophKeeper/internal/models" +) + +func Test_findDifferences(t *testing.T) { + testTime := time.Now() + tests := []struct { + name string + local models.UserMeta + server models.UserMeta + wantNeedDownload models.MetaData + wantNeedUpload models.MetaData + }{ + { + name: "download & upload", + local: models.UserMeta{Data: models.MetaData{ + "data1": models.DataInfo{DataID: "data1", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime}}}, + server: models.UserMeta{Data: models.MetaData{ + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data4": models.DataInfo{DataID: "data4", UpdatedAt: testTime}}}, + wantNeedDownload: models.MetaData{ + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data4": models.DataInfo{DataID: "data4", UpdatedAt: testTime}, + }, + wantNeedUpload: models.MetaData{ + "data1": models.DataInfo{DataID: "data1", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime}, + }, + }, + { + name: "upload", + local: models.UserMeta{Data: models.MetaData{ + "data1": models.DataInfo{DataID: "data1", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime}}}, + server: models.UserMeta{Data: models.MetaData{}}, + wantNeedDownload: models.MetaData{}, + wantNeedUpload: models.MetaData{ + "data1": models.DataInfo{DataID: "data1", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime}}, + }, + { + name: "download", + local: models.UserMeta{Data: models.MetaData{}}, + server: models.UserMeta{Data: models.MetaData{ + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data4": models.DataInfo{DataID: "data4", UpdatedAt: testTime}}}, + wantNeedDownload: models.MetaData{ + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data4": models.DataInfo{DataID: "data4", UpdatedAt: testTime}}, + wantNeedUpload: models.MetaData{}, + }, + { + name: "upload with deleted data", + local: models.UserMeta{Data: models.MetaData{ + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-2 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime}}, + }, + server: models.UserMeta{DeletedData: models.Deleted{"data2": struct{}{}}}, + wantNeedDownload: models.MetaData{}, + wantNeedUpload: models.MetaData{ + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime}}, + }, + { + name: "download with deleted", + local: models.UserMeta{Data: models.MetaData{}, + DeletedData: models.Deleted{"data2": struct{}{}}}, + server: models.UserMeta{Data: models.MetaData{ + "data2": models.DataInfo{DataID: "data2", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data4": models.DataInfo{DataID: "data4", UpdatedAt: testTime}}}, + wantNeedDownload: models.MetaData{ + "data3": models.DataInfo{DataID: "data3", UpdatedAt: testTime.Add(-1 * time.Hour)}, + "data4": models.DataInfo{DataID: "data4", UpdatedAt: testTime}}, + wantNeedUpload: models.MetaData{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotNeedDownload, gotNeedUpload := findDifferences(tt.local, tt.server) + if !reflect.DeepEqual(gotNeedDownload, tt.wantNeedDownload) { + t.Errorf("findDifferences() gotNeedDownload = %v, want %v", gotNeedDownload, tt.wantNeedDownload) + } + if !reflect.DeepEqual(gotNeedUpload, tt.wantNeedUpload) { + t.Errorf("findDifferences() gotNeedUpload = %v, want %v", gotNeedUpload, tt.wantNeedUpload) + } + }) + } +} diff --git a/internal/client/usecase/update_test.go b/internal/client/usecase/update_test.go index 21312bb..01968c0 100644 --- a/internal/client/usecase/update_test.go +++ b/internal/client/usecase/update_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/golang/mock/gomock" "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager" mock_filemanager "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/filemanager/mocks" "github.com/kripsy/GophKeeper/internal/client/infrastrucrure/ui" @@ -13,7 +14,6 @@ import ( "github.com/kripsy/GophKeeper/internal/client/permissions" "github.com/kripsy/GophKeeper/internal/models" "github.com/rs/zerolog" - "go.uber.org/mock/gomock" ) func TestClientUsecase_updateSecret(t *testing.T) { diff --git a/internal/client/usecase/usecase.go b/internal/client/usecase/usecase.go index 406f75e..13b11c3 100644 --- a/internal/client/usecase/usecase.go +++ b/internal/client/usecase/usecase.go @@ -119,7 +119,7 @@ func (c *ClientUsecase) checkUserOnServer(userAuth filemanager.Auth) bool { return false } - meta, err := userAuth.CreateUser(&c.userData.User, false) + meta, err := userAuth.CreateUser(&c.userData.User, true) if err != nil { return false } diff --git a/internal/models/usermeta.go b/internal/models/usermeta.go index 56acb82..a10a78e 100644 --- a/internal/models/usermeta.go +++ b/internal/models/usermeta.go @@ -50,3 +50,9 @@ func (md *UserMeta) GetHash() error { // todo delete return nil } + +func (d Deleted) IsDeleted(dataID string) bool { + _, deleted := d[dataID] + + return deleted +} diff --git a/internal/server/controller/middleware_grpc_controller_test.go b/internal/server/controller/middleware_grpc_controller_test.go index 824bf3e..64819ba 100644 --- a/internal/server/controller/middleware_grpc_controller_test.go +++ b/internal/server/controller/middleware_grpc_controller_test.go @@ -10,7 +10,7 @@ import ( "github.com/kripsy/GophKeeper/internal/server/controller" "github.com/kripsy/GophKeeper/internal/server/controller/mocks" "github.com/kripsy/GophKeeper/internal/utils" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -36,11 +36,11 @@ func TestInitMyMiddleware(t *testing.T) { // Переинициализация одиночки перед каждым тестом middleware := controller.InitMyMiddleware(mockLogger, tt.secret) - assert.NotNil(t, middleware) + require.NotNil(t, middleware) // Проверяем, что повторный вызов возвращает тот же экземпляр sameMiddleware := controller.InitMyMiddleware(mockLogger, "newSecret") - assert.Equal(t, middleware, sameMiddleware) + require.Equal(t, middleware, sameMiddleware) }) } } @@ -61,10 +61,10 @@ func TestAuthInterceptor(t *testing.T) { userID := 1 userName := "testuser" validToken, err := utils.BuildJWTString(userID, userName, secret, time.Hour) - assert.NoError(t, err) + require.NoError(t, err) invalidToken, err := utils.BuildJWTString(userID, userName, secret+"fake", time.Hour) - assert.NoError(t, err) + require.NoError(t, err) tests := []struct { name string @@ -163,9 +163,9 @@ func TestAuthInterceptor(t *testing.T) { _, err := middleware.AuthInterceptor(ctx, nil, info, testHandler) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) } }) } @@ -258,12 +258,12 @@ func TestStreamAuthInterceptor(t *testing.T) { }) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) if err != nil { - assert.Equal(t, tt.wantCode, status.Code(err)) + require.Equal(t, tt.wantCode, status.Code(err)) } } else { - assert.NoError(t, err) + require.NoError(t, err) } }) }