From d9d0158b6f51dcbf5fb4589718e8890e064b481a Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Thu, 9 Feb 2023 10:30:04 -0800 Subject: [PATCH] Break file order dependency for OneDrive .meta files (#2450) ## Description Begin using the Fetch() interface to retrieve OneDrive meta files inline when restoring the file. This removes the ordering dependency between .data and .meta files This does not stop .meta files from being returned over the Items() channel. That can be disabled in a future PR ## Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No ## Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * #2447 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../connector/graph_connector_helper_test.go | 43 +++++- .../connector/graph_connector_test.go | 140 ++++++++++++++++++ src/internal/connector/onedrive/restore.go | 48 +++--- 3 files changed, 205 insertions(+), 26 deletions(-) diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index cdd9806b1..7e49f2c08 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -1,6 +1,7 @@ package connector import ( + "bytes" "context" "encoding/json" "io" @@ -165,6 +166,10 @@ type colInfo struct { pathElements []string category path.CategoryType items []itemInfo + // auxItems are items that can be retrieved with Fetch but won't be returned + // by Items(). These files do not directly participate in comparisosn at the + // end of a test. + auxItems []itemInfo } type restoreBackupInfo struct { @@ -969,6 +974,25 @@ func backupOutputPathFromRestore( ) } +// TODO(ashmrtn): Make this an actual mock class that can be used in other +// packages. +type mockRestoreCollection struct { + data.Collection + auxItems map[string]data.Stream +} + +func (rc mockRestoreCollection) Fetch( + ctx context.Context, + name string, +) (data.Stream, error) { + res := rc.auxItems[name] + if res == nil { + return nil, data.ErrNotFound + } + + return res, nil +} + func collectionsForInfo( t *testing.T, service path.ServiceType, @@ -991,7 +1015,7 @@ func collectionsForInfo( info.pathElements, false, ) - c := mockconnector.NewMockExchangeCollection(pth, len(info.items)) + mc := mockconnector.NewMockExchangeCollection(pth, len(info.items)) baseDestPath := backupOutputPathFromRestore(t, dest, pth) baseExpected := expectedData[baseDestPath.String()] @@ -1001,8 +1025,8 @@ func collectionsForInfo( } for i := 0; i < len(info.items); i++ { - c.Names[i] = info.items[i].name - c.Data[i] = info.items[i].data + mc.Names[i] = info.items[i].name + mc.Data[i] = info.items[i].data baseExpected[info.items[i].lookupKey] = info.items[i].data @@ -1014,9 +1038,16 @@ func collectionsForInfo( } } - collections = append(collections, data.NotFoundRestoreCollection{ - Collection: c, - }) + c := mockRestoreCollection{Collection: mc, auxItems: map[string]data.Stream{}} + + for _, aux := range info.auxItems { + c.auxItems[aux.name] = &mockconnector.MockExchangeData{ + ID: aux.name, + Reader: io.NopCloser(bytes.NewReader(aux.data)), + } + } + + collections = append(collections, c) kopiaEntries += len(info.items) } diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 87c2343d9..8a2a2a39e 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -898,6 +898,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { lookupKey: "b" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -924,6 +931,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { lookupKey: "b" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -951,6 +965,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { lookupKey: "folder-a" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -974,6 +995,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -995,6 +1023,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1027,6 +1062,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { lookupKey: "b" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -1048,6 +1090,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"read"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1203,6 +1252,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackupVersion0() { lookupKey: "b" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -1229,6 +1285,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackupVersion0() { lookupKey: "b" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -1256,6 +1319,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackupVersion0() { lookupKey: "folder-a" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -1279,6 +1349,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackupVersion0() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -1300,6 +1377,13 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackupVersion0() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1521,6 +1605,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1554,6 +1645,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { lookupKey: "b" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -1575,6 +1673,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"read"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1608,6 +1713,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { lookupKey: "b" + onedrive.DirMetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, { pathElements: []string{ @@ -1629,6 +1741,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"read"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1673,6 +1792,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1717,6 +1843,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: []byte("{}"), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, @@ -1775,6 +1908,13 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsBackupAndNoRestore() lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, }, }, + auxItems: []itemInfo{ + { + name: "test-file.txt" + onedrive.MetaFileSuffix, + data: getTestMetaJSON(suite.T(), suite.secondaryUser, []string{"write"}), + lookupKey: "test-file.txt" + onedrive.MetaFileSuffix, + }, + }, }, }, }, diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index e2029f4cc..d5df9dedc 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -9,6 +9,7 @@ import ( "sort" "strings" + "github.com/alcionai/clues" msdrive "github.com/microsoftgraph/msgraph-sdk-go/drive" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/pkg/errors" @@ -164,7 +165,6 @@ func RestoreCollection( metrics = support.CollectionMetrics{} copyBuffer = make([]byte, copyBufferSize) directory = dc.FullPath() - restoredIDs = map[string]string{} itemInfo details.ItemInfo itemID string folderPerms = map[string][]UserPermission{} @@ -226,37 +226,44 @@ func RestoreCollection( metrics.TotalBytes += int64(len(copyBuffer)) trimmedName := strings.TrimSuffix(name, DataFileSuffix) - itemID, itemInfo, err = restoreData(ctx, service, trimmedName, itemData, - drivePath.DriveID, restoreFolderID, copyBuffer, source) + itemID, itemInfo, err = restoreData( + ctx, + service, + trimmedName, + itemData, + drivePath.DriveID, + restoreFolderID, + copyBuffer, + source) if err != nil { errUpdater(itemData.UUID(), err) continue } - restoredIDs[trimmedName] = itemID - deets.Add(itemPath.String(), itemPath.ShortRef(), "", true, itemInfo) // Mark it as success without processing .meta // file if we are not restoring permissions if !restorePerms { metrics.Successes++ - } - } else if strings.HasSuffix(name, MetaFileSuffix) { - if !restorePerms { continue } - meta, err := getMetadata(itemData.ToReader()) + // Fetch item permissions from the collection and restore them. + metaName := trimmedName + MetaFileSuffix + + permsFile, err := dc.Fetch(ctx, metaName) if err != nil { - errUpdater(itemData.UUID(), err) + errUpdater(metaName, clues.Wrap(err, "getting item metadata")) continue } - trimmedName := strings.TrimSuffix(name, MetaFileSuffix) - restoreID, ok := restoredIDs[trimmedName] - if !ok { - errUpdater(itemData.UUID(), fmt.Errorf("item not available to restore permissions")) + metaReader := permsFile.ToReader() + meta, err := getMetadata(metaReader) + metaReader.Close() + + if err != nil { + errUpdater(metaName, clues.Wrap(err, "deserializing item metadata")) continue } @@ -264,21 +271,22 @@ func RestoreCollection( ctx, service, drivePath.DriveID, - restoreID, + itemID, parentPerms, meta.Permissions, permissionIDMappings, ) if err != nil { - errUpdater(itemData.UUID(), err) + errUpdater(trimmedName, clues.Wrap(err, "restoring item permissions")) continue } - // Objects count is incremented when we restore a - // data file and success count is incremented when - // we restore a meta file as every data file - // should have an associated meta file metrics.Successes++ + } else if strings.HasSuffix(name, MetaFileSuffix) { + // Just skip this for the moment since we moved the code to the above + // item restore path. We haven't yet stopped fetching these items in + // RestoreOp, so we still need to handle them in some way. + continue } else if strings.HasSuffix(name, DirMetaFileSuffix) { trimmedName := strings.TrimSuffix(name, DirMetaFileSuffix) folderID, err := createRestoreFolder(