diff --git a/src/internal/connector/data_collections_test.go b/src/internal/connector/data_collections_test.go index 336e6f713..eda0bd9a0 100644 --- a/src/internal/connector/data_collections_test.go +++ b/src/internal/connector/data_collections_test.go @@ -349,7 +349,10 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar for _, collection := range cols { t.Logf("Path: %s\n", collection.FullPath().String()) - assert.Equal(t, path.SharePointMetadataService, collection.FullPath().Service()) + assert.Equal( + t, + path.SharePointMetadataService.String(), + collection.FullPath().Service().String()) } } diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index a3a7484ee..9f0a6c6e4 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -70,7 +70,7 @@ type Collections struct { // collectionMap allows lookup of the data.BackupCollection // for a OneDrive folder - CollectionMap map[string]data.BackupCollection + CollectionMap map[string]*Collection // Not the most ideal, but allows us to change the pager function for testing // as needed. This will allow us to mock out some scenarios during testing. @@ -107,7 +107,7 @@ func NewCollections( resourceOwner: resourceOwner, source: source, matcher: matcher, - CollectionMap: map[string]data.BackupCollection{}, + CollectionMap: map[string]*Collection{}, drivePagerFunc: PagerForSource, itemPagerFunc: defaultItemPager, service: service, @@ -433,24 +433,19 @@ func (c *Collections) Get( func updateCollectionPaths( id string, - cmap map[string]data.BackupCollection, + cmap map[string]*Collection, curPath path.Path, ) (bool, error) { var initialCurPath path.Path col, found := cmap[id] if found { - ocol, ok := col.(*Collection) - if !ok { - return found, clues.New("unable to cast onedrive collection") - } - - initialCurPath = ocol.FullPath() + initialCurPath = col.FullPath() if initialCurPath.String() == curPath.String() { return found, nil } - ocol.SetFullPath(curPath) + col.SetFullPath(curPath) } if initialCurPath == nil { @@ -462,23 +457,134 @@ func updateCollectionPaths( continue } - ocol, ok := c.(*Collection) - if !ok { - return found, clues.New("unable to cast onedrive collection") - } - colPath := c.FullPath() // Only updates if initialCurPath parent of colPath updated := colPath.UpdateParent(initialCurPath, curPath) if updated { - ocol.SetFullPath(colPath) + c.SetFullPath(colPath) } } return found, nil } +func (c *Collections) handleDelete( + itemID, driveID string, + oldPaths, newPaths map[string]string, + isFolder bool, + excluded map[string]struct{}, +) error { + if !isFolder { + excluded[itemID+DataFileSuffix] = struct{}{} + excluded[itemID+MetaFileSuffix] = struct{}{} + // Exchange counts items streamed through it which includes deletions so + // add that here too. + c.NumFiles++ + c.NumItems++ + + return nil + } + + var prevPath path.Path + + prevPathStr, ok := oldPaths[itemID] + if ok { + var err error + + prevPath, err = path.FromDataLayerPath(prevPathStr, false) + if err != nil { + return clues.Wrap(err, "invalid previous path"). + With("collection_id", itemID, "path_string", prevPathStr) + } + } + + // Nested folders also return deleted delta results so we don't have to + // worry about doing a prefix search in the map to remove the subtree of + // the deleted folder/package. + delete(newPaths, itemID) + + if prevPath == nil { + // It is possible that an item was created and + // deleted between two delta invocations. In + // that case, it will only produce a single + // delete entry in the delta response. + return nil + } + + col := NewCollection( + c.itemClient, + nil, + prevPath, + driveID, + c.service, + c.statusUpdater, + c.source, + c.ctrl, + // DoNotMerge is not checked for deleted items. + false, + ) + + c.CollectionMap[itemID] = col + + return nil +} + +func (c *Collections) getCollectionPath( + driveID string, + item models.DriveItemable, +) (path.Path, error) { + var ( + collectionPathStr string + isRoot = item.GetRoot() != nil + isFile = item.GetFile() != nil + ) + + if isRoot { + collectionPathStr = fmt.Sprintf(rootDrivePattern, driveID) + } else { + if item.GetParentReference() == nil || + item.GetParentReference().GetPath() == nil { + err := clues.New("no parent reference"). + With("item_name", ptr.Val(item.GetName())) + + return nil, err + } + + collectionPathStr = ptr.Val(item.GetParentReference().GetPath()) + } + + collectionPath, err := GetCanonicalPath( + collectionPathStr, + c.tenant, + c.resourceOwner, + c.source, + ) + if err != nil { + return nil, clues.Wrap(err, "making item path") + } + + if isRoot || isFile { + return collectionPath, nil + } + + // Append folder name to path since we want the path for the collection, not + // the path for the parent of the collection. The root and files don't need + // to append an extra element because the root already refers to itself and + // the collection containing the item is the parent path. + name := ptr.Val(item.GetName()) + if len(name) == 0 { + return nil, clues.New("folder with empty name") + } + + collectionPath, err = collectionPath.Append(name, false) + if err != nil { + return nil, clues.Wrap(err, "making non-root folder path") + } + + return collectionPath, nil +} + // UpdateCollections initializes and adds the provided drive items to Collections // A new collection is created for every drive folder (or package). // oldPaths is the unchanged data that was loaded from the metadata file. @@ -503,97 +609,43 @@ func (c *Collections) UpdateCollections( } var ( - prevPath path.Path - prevCollectionPath path.Path - ok bool + itemID = ptr.Val(item.GetId()) + ictx = clues.Add(ctx, "update_item_id", itemID) + isFolder = item.GetFolder() != nil || item.GetPackage() != nil ) - if item.GetRoot() != nil { - rootPath, err := GetCanonicalPath( - fmt.Sprintf(rootDrivePattern, driveID), - c.tenant, - c.resourceOwner, - c.source, - ) - if err != nil { - return err + // Deleted file or folder. + if item.GetDeleted() != nil { + if err := c.handleDelete( + itemID, + driveID, + oldPaths, + newPaths, + isFolder, + excluded, + ); err != nil { + return clues.Stack(err).WithClues(ictx) } - // Add root path so as to have root folder information in - // path metadata. Root id -> path map is necessary in - // following delta incremental backups. - updatePath(newPaths, *item.GetId(), rootPath.String()) - continue } - var ( - itemID = ptr.Val(item.GetId()) - ictx = clues.Add(ctx, "update_item_id", itemID) - ) - - if item.GetParentReference() == nil || - item.GetParentReference().GetId() == nil || - (item.GetDeleted() == nil && item.GetParentReference().GetPath() == nil) { - et.Add(clues.New("item missing parent reference"). - WithClues(ictx). - With("item_id", itemID, "item_name", ptr.Val(item.GetName())). - Label(fault.LabelForceNoBackupCreation)) - - continue - } - - // Create a collection for the parent of this item - collectionID := ptr.Val(item.GetParentReference().GetId()) - ictx = clues.Add(ictx, "collection_id", collectionID) - - var collectionPathStr string - if item.GetDeleted() == nil { - collectionPathStr = ptr.Val(item.GetParentReference().GetPath()) - } else { - collectionPathStr, ok = oldPaths[ptr.Val(item.GetParentReference().GetId())] - if !ok { - // This collection was created and destroyed in - // between the current and previous invocation - continue - } - } - - collectionPath, err := GetCanonicalPath( - collectionPathStr, - c.tenant, - c.resourceOwner, - c.source) + collectionPath, err := c.getCollectionPath(driveID, item) if err != nil { return clues.Stack(err).WithClues(ictx) } - var ( - itemPath path.Path - isFolder = item.GetFolder() != nil || item.GetPackage() != nil - ) - - if item.GetDeleted() == nil { - name := ptr.Val(item.GetName()) - if len(name) == 0 { - return clues.New("non-deleted item with empty name").With("item_id", name) - } - - itemPath, err = collectionPath.Append(name, !isFolder) - if err != nil { - return err - } - } - // Skip items that don't match the folder selectors we were given. - if shouldSkipDrive(ctx, itemPath, c.matcher, driveName) && - shouldSkipDrive(ctx, collectionPath, c.matcher, driveName) { + if shouldSkipDrive(ctx, collectionPath, c.matcher, driveName) { logger.Ctx(ictx).Infow("Skipping path", "skipped_path", collectionPath.String()) continue } switch { - case item.GetFolder() != nil, item.GetPackage() != nil: + case isFolder: + // Deletions are handled above so this is just moves/renames. + var prevPath path.Path + prevPathStr, ok := oldPaths[itemID] if ok { prevPath, err = path.FromDataLayerPath(prevPathStr, false) @@ -602,172 +654,103 @@ func (c *Collections) UpdateCollections( WithClues(ictx). With("path_string", prevPathStr)) } - } - - if item.GetDeleted() != nil { - // Nested folders also return deleted delta results so we don't have to - // worry about doing a prefix search in the map to remove the subtree of - // the deleted folder/package. - delete(newPaths, itemID) - - if prevPath == nil { - // It is possible that an item was created and - // deleted between two delta invocations. In - // that case, it will only produce a single - // delete entry in the delta response. - continue - } - - col := NewCollection( - c.itemClient, - nil, - prevPath, - driveID, - c.service, - c.statusUpdater, - c.source, - c.ctrl, - invalidPrevDelta) - - c.CollectionMap[itemID] = col - - break + } else if item.GetRoot() != nil { + // Root doesn't move or get renamed. + prevPath = collectionPath } // Moved folders don't cause delta results for any subfolders nested in // them. We need to go through and update paths to handle that. We only // update newPaths so we don't accidentally clobber previous deletes. - updatePath(newPaths, *item.GetId(), itemPath.String()) + updatePath(newPaths, itemID, collectionPath.String()) - found, err := updateCollectionPaths(*item.GetId(), c.CollectionMap, itemPath) + found, err := updateCollectionPaths(itemID, c.CollectionMap, collectionPath) if err != nil { - return clues.Stack(err).WithClues(ctx) + return clues.Stack(err).WithClues(ictx) } - if !found { - col := NewCollection( - c.itemClient, - itemPath, - prevPath, - driveID, - c.service, - c.statusUpdater, - c.source, - c.ctrl, - invalidPrevDelta, - ) - c.CollectionMap[*item.GetId()] = col - c.NumContainers++ - } - - if c.source != OneDriveSource { + if found { continue } - if col := c.CollectionMap[*item.GetId()]; col != nil { - // Add an entry to fetch permissions into this collection. This assumes - // that OneDrive always returns all folders on the path of an item - // before the item. This seems to hold true for now at least. - collection := col.(*Collection) - if collection.Add(item) { - c.NumItems++ - } + col := NewCollection( + c.itemClient, + collectionPath, + prevPath, + driveID, + c.service, + c.statusUpdater, + c.source, + c.ctrl, + invalidPrevDelta, + ) + c.CollectionMap[itemID] = col + c.NumContainers++ + + if c.source != OneDriveSource || item.GetRoot() != nil { + continue + } + + // Add an entry to fetch permissions into this collection. This assumes + // that OneDrive always returns all folders on the path of an item + // before the item. This seems to hold true for now at least. + if col.Add(item) { + c.NumItems++ } case item.GetFile() != nil: - if !invalidPrevDelta && item.GetFile() != nil { - // Always add a file to the excluded list. If it was - // deleted, we want to avoid it. If it was - // renamed/moved/modified, we still have to drop the - // original one and download a fresh copy. - excluded[itemID+DataFileSuffix] = struct{}{} - excluded[itemID+MetaFileSuffix] = struct{}{} + // Deletions are handled above so this is just moves/renames. + if len(ptr.Val(item.GetParentReference().GetId())) == 0 { + return clues.New("file without parent ID").WithClues(ictx) } - if item.GetDeleted() != nil { - // Exchange counts items streamed through it which includes deletions so - // add that here too. - c.NumFiles++ - c.NumItems++ + // Get the collection for this item. + collectionID := ptr.Val(item.GetParentReference().GetId()) + ictx = clues.Add(ictx, "collection_id", collectionID) - continue - } - - oneDrivePath, err := path.ToOneDrivePath(collectionPath) - if err != nil { - return clues.Wrap(err, "invalid path for backup") - } - - if len(oneDrivePath.Folders) == 0 { - // path for root will never change - prevCollectionPath = collectionPath - } else { - prevCollectionPathStr, ok := oldPaths[collectionID] - if ok { - prevCollectionPath, err = path.FromDataLayerPath(prevCollectionPathStr, false) - if err != nil { - return clues.Wrap(err, "invalid previous path").With("path_string", prevCollectionPathStr) - } - } - - } - - col, found := c.CollectionMap[collectionID] + collection, found := c.CollectionMap[collectionID] if !found { - // TODO(ashmrtn): We should probably tighten the restrictions on this - // and just make it return an error if the collection doesn't already - // exist. Graph seems pretty consistent about returning all folders on - // the path from the root to the item in question. Removing this will - // also ensure we always add an entry to get the folder metadata. - col = NewCollection( - c.itemClient, - collectionPath, - prevCollectionPath, - driveID, - c.service, - c.statusUpdater, - c.source, - c.ctrl, - invalidPrevDelta, - ) - - c.CollectionMap[collectionID] = col - c.NumContainers++ + return clues.New("item seen before parent folder").WithClues(ictx) } - // TODO(meain): If a folder gets renamed/moved multiple - // times within a single delta response, we might end up - // storing the permissions multiple times. Switching the - // files to IDs should fix this. - // Delete the file from previous collection. This will // only kick in if the file was moved multiple times // within a single delta query - itemColID, found := itemCollection[*item.GetId()] + itemColID, found := itemCollection[itemID] if found { - pcol, found := c.CollectionMap[itemColID] + pcollection, found := c.CollectionMap[itemColID] if !found { - return clues.New("previous collection not found").With("item_id", *item.GetId()) + return clues.New("previous collection not found").WithClues(ictx) } - pcollection := pcol.(*Collection) - removed := pcollection.Remove(item) if !removed { - return clues.New("removing from prev collection").With("item_id", *item.GetId()) + return clues.New("removing from prev collection").WithClues(ictx) } } - itemCollection[*item.GetId()] = collectionID - collection := col.(*Collection) + itemCollection[itemID] = collectionID if collection.Add(item) { c.NumItems++ c.NumFiles++ } + // Do this after adding the file to the collection so if we fail to add + // the item to the collection for some reason and we're using best effort + // we don't just end up deleting the item in the resulting backup. The + // resulting backup will be slightly incorrect, but it will have the most + // data that we were able to preserve. + if !invalidPrevDelta { + // Always add a file to the excluded list. The file may have been + // renamed/moved/modified, so we still have to drop the + // original one and download a fresh copy. + excluded[itemID+DataFileSuffix] = struct{}{} + excluded[itemID+MetaFileSuffix] = struct{}{} + } + default: - return clues.New("item type not supported").WithClues(ctx) + return clues.New("item type not supported").WithClues(ictx) } } @@ -775,10 +758,6 @@ func (c *Collections) UpdateCollections( } func shouldSkipDrive(ctx context.Context, drivePath path.Path, m folderMatcher, driveName string) bool { - if drivePath == nil { - return false - } - return !includePath(ctx, m, drivePath) || (drivePath.Category() == path.LibrariesCategory && restrictedDirectory == driveName) } diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 079b7efb7..165394508 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -187,6 +187,10 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { inputFolderMap: map[string]string{}, scope: anyFolder, expect: assert.Error, + expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), + }, + expectedContainerCount: 1, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), }, @@ -223,6 +227,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.NewState, folder), }, expectedMetadataPaths: map[string]string{ @@ -230,7 +235,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { "folder": expectedPath("/folder"), }, expectedItemCount: 1, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedExcludes: map[string]struct{}{}, }, { @@ -243,6 +248,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "package": expectedStatePath(data.NewState, pkg), }, expectedMetadataPaths: map[string]string{ @@ -250,7 +256,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { "package": expectedPath("/package"), }, expectedItemCount: 1, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedExcludes: map[string]struct{}{}, }, { @@ -308,7 +314,6 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { // just "folder" isn't added here because the include check is done on the // parent path since we only check later if something is a folder or not. expectedMetadataPaths: map[string]string{ - "root": expectedPath(""), "folder": expectedPath(folder), "subfolder": expectedPath(folderSub), "folder2": expectedPath(folderSub + folder), @@ -340,7 +345,6 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { expectedFileCount: 1, expectedContainerCount: 2, expectedMetadataPaths: map[string]string{ - "root": expectedPath(""), "subfolder": expectedPath(folderSub), "folder2": expectedPath(folderSub + folder), }, @@ -369,7 +373,6 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { expectedContainerCount: 1, // No child folders for subfolder so nothing here. expectedMetadataPaths: map[string]string{ - "root": expectedPath(""), "subfolder": expectedPath(folderSub), }, expectedExcludes: getDelList("fileInSubfolder"), @@ -387,11 +390,12 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.NotMovedState, folder), }, expectedItemCount: 1, expectedFileCount: 0, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath(folder), @@ -412,11 +416,12 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.MovedState, folder, "/a-folder"), }, expectedItemCount: 1, expectedFileCount: 0, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath(folder), @@ -424,30 +429,6 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { }, expectedExcludes: map[string]struct{}{}, }, - { - testCase: "moved folder tree with file with file first", - items: []models.DriveItemable{ - driveRootItem("root"), - driveItem("file", "file", testBaseDrivePath+"/folder", "folder", true, false, false), - driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), - }, - inputFolderMap: map[string]string{ - "folder": expectedPath(folder), - }, - scope: anyFolder, - expect: assert.NoError, - expectedCollectionIDs: map[string]statePath{ - "folder": expectedStatePath(data.NotMovedState, folder), - }, - expectedItemCount: 2, - expectedFileCount: 1, - expectedContainerCount: 1, - expectedMetadataPaths: map[string]string{ - "root": expectedPath(""), - "folder": expectedPath(folder), - }, - expectedExcludes: getDelList("file"), - }, { testCase: "moved folder tree with file no previous", items: []models.DriveItemable{ @@ -460,11 +441,12 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.NewState, "/folder2"), }, expectedItemCount: 2, expectedFileCount: 1, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath("/folder2"), @@ -482,11 +464,12 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.NewState, folder), }, expectedItemCount: 2, expectedFileCount: 1, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath(folder), @@ -507,12 +490,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.MovedState, folder, "/a-folder"), "subfolder": expectedStatePath(data.MovedState, "/subfolder", "/a-folder/subfolder"), }, expectedItemCount: 2, expectedFileCount: 0, - expectedContainerCount: 2, + expectedContainerCount: 3, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath(folder), @@ -534,12 +518,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.MovedState, folder, "/a-folder"), "subfolder": expectedStatePath(data.MovedState, "/subfolder", "/a-folder/subfolder"), }, expectedItemCount: 2, expectedFileCount: 0, - expectedContainerCount: 2, + expectedContainerCount: 3, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath(folder), @@ -575,13 +560,14 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.MovedState, folder, "/a-folder"), "folder2": expectedStatePath(data.NewState, "/folder2"), "subfolder": expectedStatePath(data.MovedState, folderSub, "/a-folder/subfolder"), }, expectedItemCount: 5, expectedFileCount: 2, - expectedContainerCount: 3, + expectedContainerCount: 4, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath("/folder"), @@ -605,11 +591,12 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.MovedState, "/folder2", "/a-folder"), }, expectedItemCount: 2, expectedFileCount: 1, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "folder": expectedPath("/folder2"), @@ -632,12 +619,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.DeletedState, folder), "package": expectedStatePath(data.DeletedState, pkg), }, expectedItemCount: 0, expectedFileCount: 0, - expectedContainerCount: 0, + expectedContainerCount: 1, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), }, @@ -652,12 +640,14 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { inputFolderMap: map[string]string{ "root": expectedPath(""), }, - scope: anyFolder, - expect: assert.NoError, - expectedCollectionIDs: map[string]statePath{}, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), + }, expectedItemCount: 0, expectedFileCount: 0, - expectedContainerCount: 0, + expectedContainerCount: 1, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), }, @@ -678,12 +668,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { scope: anyFolder, expect: assert.NoError, expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), "folder": expectedStatePath(data.DeletedState, folder), "subfolder": expectedStatePath(data.MovedState, "/subfolder", folderSub), }, expectedItemCount: 1, expectedFileCount: 0, - expectedContainerCount: 1, + expectedContainerCount: 2, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), "subfolder": expectedPath("/subfolder"), @@ -693,21 +684,46 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "delete file", items: []models.DriveItemable{ + driveRootItem("root"), delItem("item", testBaseDrivePath, "root", true, false, false), }, inputFolderMap: map[string]string{ "root": expectedPath(""), }, - scope: anyFolder, - expect: assert.NoError, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), + }, expectedItemCount: 1, expectedFileCount: 1, - expectedContainerCount: 0, + expectedContainerCount: 1, expectedMetadataPaths: map[string]string{ "root": expectedPath(""), }, expectedExcludes: getDelList("item"), }, + { + testCase: "item before parent errors", + items: []models.DriveItemable{ + driveRootItem("root"), + driveItem("file", "file", testBaseDrivePath+"/folder", "folder", true, false, false), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), + }, + inputFolderMap: map[string]string{}, + scope: anyFolder, + expect: assert.Error, + expectedCollectionIDs: map[string]statePath{ + "root": expectedStatePath(data.NotMovedState, ""), + }, + expectedItemCount: 0, + expectedFileCount: 0, + expectedContainerCount: 1, + expectedMetadataPaths: map[string]string{ + "root": expectedPath(""), + }, + expectedExcludes: map[string]struct{}{}, + }, } for _, tt := range tests { @@ -1241,7 +1257,9 @@ func (suite *OneDriveCollectionsSuite) TestGet() { prevFolderPaths: map[string]map[string]string{ driveID1: {"root": rootFolderPath1}, }, - expectedCollections: map[string]map[data.CollectionState][]string{}, + expectedCollections: map[string]map[data.CollectionState][]string{ + rootFolderPath1: {data.NotMovedState: {}}, + }, expectedDeltaURLs: map[string]string{ driveID1: delta, }, @@ -1266,10 +1284,10 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, errCheck: assert.NoError, prevFolderPaths: map[string]map[string]string{ - driveID1: {"root": expectedPath1("")}, + driveID1: {"root": rootFolderPath1}, }, expectedCollections: map[string]map[data.CollectionState][]string{ - expectedPath1(""): {data.NotMovedState: {"file"}}, + rootFolderPath1: {data.NotMovedState: {"file"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1299,7 +1317,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ - folderPath1: {data.NewState: {"folder", "file"}}, + rootFolderPath1: {data.NewState: {}}, + folderPath1: {data.NewState: {"folder", "file"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1333,7 +1352,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ - folderPath1: {data.NewState: {"folder", "file"}}, + rootFolderPath1: {data.NewState: {}}, + folderPath1: {data.NewState: {"folder", "file"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1401,7 +1421,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ - folderPath1: {data.NewState: {"folder", "file"}}, + rootFolderPath1: {data.NewState: {}}, + folderPath1: {data.NewState: {"folder", "file"}}, }, expectedDeltaURLs: map[string]string{}, expectedFolderPaths: map[string]map[string]string{}, @@ -1435,7 +1456,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ - folderPath1: {data.NewState: {"folder", "file", "file2"}}, + rootFolderPath1: {data.NewState: {}}, + folderPath1: {data.NewState: {"folder", "file", "file2"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1482,8 +1504,10 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID2: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ - folderPath1: {data.NewState: {"folder", "file"}}, - folderPath2: {data.NewState: {"folder2", "file2"}}, + rootFolderPath1: {data.NewState: {}}, + folderPath1: {data.NewState: {"folder", "file"}}, + rootFolderPath2: {data.NewState: {}}, + folderPath2: {data.NewState: {"folder2", "file2"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1539,7 +1563,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, errCheck: assert.NoError, expectedCollections: map[string]map[data.CollectionState][]string{ - expectedPath1(""): {data.NotMovedState: {"file"}}, + rootFolderPath1: {data.NotMovedState: {"file"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1579,7 +1603,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, errCheck: assert.NoError, expectedCollections: map[string]map[data.CollectionState][]string{ - expectedPath1(""): {data.NotMovedState: {"file"}}, + rootFolderPath1: {data.NotMovedState: {"file"}}, expectedPath1("/folder"): {data.NewState: {"folder", "file2"}}, }, expectedDeltaURLs: map[string]string{ @@ -1621,7 +1645,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: {}, }, expectedCollections: map[string]map[data.CollectionState][]string{ - expectedPath1(""): {data.NotMovedState: {"file"}}, + rootFolderPath1: {data.NotMovedState: {"file"}}, expectedPath1("/folder"): {data.NewState: {"folder", "file2"}}, }, expectedDeltaURLs: map[string]string{ @@ -1662,6 +1686,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, }, expectedCollections: map[string]map[data.CollectionState][]string{ + rootFolderPath1: {data.NewState: {}}, expectedPath1("/folder"): {data.DeletedState: {}}, expectedPath1("/folder2"): {data.NewState: {"folder2", "file"}}, }, @@ -1703,6 +1728,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, }, expectedCollections: map[string]map[data.CollectionState][]string{ + rootFolderPath1: {data.NewState: {}}, expectedPath1("/folder"): {data.NewState: {"folder2", "file"}}, }, expectedDeltaURLs: map[string]string{ @@ -1894,6 +1920,7 @@ func driveRootItem(id string) models.DriveItemable { item.SetName(&name) item.SetId(&id) item.SetRoot(models.NewRoot()) + item.SetFolder(models.NewFolder()) return item } diff --git a/src/internal/connector/sharepoint/data_collections_test.go b/src/internal/connector/sharepoint/data_collections_test.go index 32959a9df..dde5c8fcc 100644 --- a/src/internal/connector/sharepoint/data_collections_test.go +++ b/src/internal/connector/sharepoint/data_collections_test.go @@ -156,6 +156,7 @@ func driveRootItem(id string) models.DriveItemable { item.SetName(&name) item.SetId(&id) item.SetRoot(models.NewRoot()) + item.SetFolder(models.NewFolder()) return item }