Add a flag to toggle OneDrive delta incrementals

This will help us while this is still in development. We should be
able to add in features without affecting anything else.

Once we have it in a more or less stable state this can be removed
completely.

src/cli/backup/onedrive.go

@@ -81,6 +81,7 @@ func addOneDriveCommands(cmd *cobra.Command) *cobra.Command {
 	case createCommand:
 		c, fs = utils.AddCommand(cmd, oneDriveCreateCmd())
 		options.AddFeatureToggle(cmd, options.EnablePermissionsBackup())
+		options.AddFeatureToggle(cmd, options.EnableOneDriveDeltaIncrementals())
 
 		c.Use = c.Use + " " + oneDriveServiceCommandCreateUseSuffix
 		c.Example = oneDriveServiceCommandCreateExamples

src/cli/options/options.go

@@ -16,6 +16,7 @@ func Control() control.Options {
 	opt.RestorePermissions = restorePermissions
 	opt.ToggleFeatures.DisableIncrementals = disableIncrementals
 	opt.ToggleFeatures.EnablePermissionsBackup = enablePermissionsBackup
+	opt.ToggleFeatures.EnableOneDriveDeltaIncrementals = enableOneDriveDeltaIncrentals
 
 	return opt
 }
@@ -59,6 +60,7 @@ func AddRestorePermissionsFlag(cmd *cobra.Command) {
 var (
 	disableIncrementals           bool
 	enablePermissionsBackup       bool
+	enableOneDriveDeltaIncrentals bool
 )
 
 type exposeFeatureFlag func(*pflag.FlagSet)
@@ -97,3 +99,16 @@ func EnablePermissionsBackup() func(*pflag.FlagSet) {
 		cobra.CheckErr(fs.MarkHidden("enable-permissions-backup"))
 	}
 }
+
+// Adds the hidden '--enable-onedrive-delta-incrementals' cli flag which, when
+// set, enables delta incrementals for OneDrive.
+func EnableOneDriveDeltaIncrementals() func(*pflag.FlagSet) {
+	return func(fs *pflag.FlagSet) {
+		fs.BoolVar(
+			&enableOneDriveDeltaIncrentals,
+			"enable-onedrive-delta-incrementals",
+			false,
+			"Enables delta based incrementals for OneDrive")
+		cobra.CheckErr(fs.MarkHidden("enable-onedrive-delta-incrementals"))
+	}
+}

src/internal/connector/graph_connector_helper_test.go

@@ -704,11 +704,18 @@ func compareOneDriveItem(
 	t *testing.T,
 	expected map[string][]byte,
 	item data.Stream,
+	dest control.RestoreDestination,
 	restorePermissions bool,
-) {
+) bool {
+	// Skip OneDrive permissions in the folder that used to be the root. We don't
+	// have a good way to materialize these in the test right now.
+	if item.UUID() == dest.ContainerName+onedrive.DirMetaFileSuffix {
+		return false
+	}
+
 	buf, err := io.ReadAll(item.ToReader())
 	if !assert.NoError(t, err) {
-		return
+		return true
 	}
 
 	name := item.UUID()
@@ -721,7 +728,7 @@ func compareOneDriveItem(
 
 		err = json.Unmarshal(buf, &itemMeta)
 		if !assert.NoErrorf(t, err, "unmarshalling retrieved metadata for file %s", name) {
-			return
+			return true
 		}
 
 		expectedData := expected[name]
@@ -731,12 +738,12 @@ func compareOneDriveItem(
 			"unexpected metadata file with name %s",
 			name,
 		) {
-			return
+			return true
 		}
 
 		err = json.Unmarshal(expectedData, &expectedMeta)
 		if !assert.NoError(t, err, "unmarshalling expected metadata") {
-			return
+			return true
 		}
 
 		// Only compare file names if we're using a version that expects them to be
@@ -747,7 +754,7 @@ func compareOneDriveItem(
 
 		if !restorePermissions {
 			assert.Equal(t, 0, len(itemMeta.Permissions))
-			return
+			return true
 		}
 
 		testElementsMatch(
@@ -757,19 +764,19 @@ func compareOneDriveItem(
 			permissionEqual,
 		)
 
-		return
+		return true
 	}
 
 	var fileData testOneDriveData
 
 	err = json.Unmarshal(buf, &fileData)
 	if !assert.NoErrorf(t, err, "unmarshalling file data for file %s", name) {
-		return
+		return true
 	}
 
 	expectedData := expected[fileData.FileName]
 	if !assert.NotNil(t, expectedData, "unexpected file with name %s", name) {
-		return
+		return true
 	}
 
 	// OneDrive data items are just byte buffers of the data. Nothing special to
@@ -778,16 +785,22 @@ func compareOneDriveItem(
 	// Compare against the version with the file name embedded because that's what
 	// the auto-generated expected data has.
 	assert.Equal(t, expectedData, buf)
+
+	return true
 }
 
+// compareItem compares the data returned by backup with the expected data.
+// Returns true if a comparison was done else false. Bool return is mostly used
+// to exclude OneDrive permissions for the root right now.
 func compareItem(
 	t *testing.T,
 	expected map[string][]byte,
 	service path.ServiceType,
 	category path.CategoryType,
 	item data.Stream,
+	dest control.RestoreDestination,
 	restorePermissions bool,
-) {
+) bool {
 	if mt, ok := item.(data.StreamModTime); ok {
 		assert.NotZero(t, mt.ModTime())
 	}
@@ -806,11 +819,13 @@ func compareItem(
 		}
 
 	case path.OneDriveService:
-		compareOneDriveItem(t, expected, item, restorePermissions)
+		return compareOneDriveItem(t, expected, item, dest, restorePermissions)
 
 	default:
 		assert.FailNowf(t, "unexpected service: %s", service.String())
 	}
+
+	return true
 }
 
 func checkHasCollections(
@@ -831,7 +846,7 @@ func checkHasCollections(
 		gotNames = append(gotNames, g.FullPath().String())
 	}
 
-	assert.ElementsMatch(t, expectedNames, gotNames)
+	assert.ElementsMatch(t, expectedNames, gotNames, "returned collections")
 }
 
 //revive:disable:context-as-argument
@@ -841,6 +856,7 @@ func checkCollections(
 	expectedItems int,
 	expected map[string]map[string][]byte,
 	got []data.BackupCollection,
+	dest control.RestoreDestination,
 	restorePermissions bool,
 ) int {
 	//revive:enable:context-as-argument
@@ -850,10 +866,12 @@ func checkCollections(
 	gotItems := 0
 
 	for _, returned := range got {
-		startingItems := gotItems
+		var (
-		service := returned.FullPath().Service()
+			hasItems        bool
-		category := returned.FullPath().Category()
+			service         = returned.FullPath().Service()
-		expectedColData := expected[returned.FullPath().String()]
+			category        = returned.FullPath().Category()
+			expectedColData = expected[returned.FullPath().String()]
+		)
 
 		// Need to iterate through all items even if we don't expect to find a match
 		// because otherwise we'll deadlock waiting for GC status. Unexpected or
@@ -871,16 +889,19 @@ func checkCollections(
 				continue
 			}
 
+			hasItems = true
 			gotItems++
 
 			if expectedColData == nil {
 				continue
 			}
 
-			compareItem(t, expectedColData, service, category, item, restorePermissions)
+			if !compareItem(t, expectedColData, service, category, item, dest, restorePermissions) {
+				gotItems--
+			}
 		}
 
-		if gotItems != startingItems {
+		if hasItems {
 			collectionsWithItems = append(collectionsWithItems, returned)
 		}
 	}

src/internal/connector/graph_connector_onedrive_test.go

@@ -175,9 +175,7 @@ func (c *onedriveCollection) withFile(
 			name+onedrive.DataFileSuffix,
 			fileData))
 
-	case 1:
+	case 1, 2, 3:
-		fallthrough
-	case 2:
 		c.items = append(c.items, onedriveItemWithData(
 			c.t,
 			name+onedrive.DataFileSuffix,
@@ -206,12 +204,10 @@ func (c *onedriveCollection) withFolder(
 	roles []string,
 ) *onedriveCollection {
 	switch c.backupVersion {
-	case 0:
+	case 0, 3:
 		return c
 
-	case 1:
+	case 1, 2:
-		fallthrough
-	case 2:
 		c.items = append(
 			c.items,
 			onedriveMetadata(
@@ -247,15 +243,15 @@ func (c *onedriveCollection) withPermissions(
 		return c
 	}
 
-	c.items = append(
+	metadata := onedriveMetadata(
-		c.items,
-		onedriveMetadata(
 		c.t,
 		name,
 		name+onedrive.DirMetaFileSuffix,
 		user,
-			roles),
+		roles)
-	)
+
+	c.items = append(c.items, metadata)
+	c.aux = append(c.aux, metadata)
 
 	return c
 }
@@ -382,6 +378,10 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Multip
 				},
 				{
 					pathElements: folderBPath,
+					perms: permData{
+						user:  suite.secondaryUser,
+						roles: readPerm,
+					},
 					files: []itemData{
 						{
 							name: fileName,
@@ -895,6 +895,12 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
 					"",
 					nil,
 				).
+				// Call this to generate a meta file with the folder name that we can
+				// check.
+				withPermissions(
+					"",
+					nil,
+				).
 				collection(),
 		},
 	}

src/internal/connector/graph_connector_test.go

@@ -490,7 +490,14 @@ func runBackupAndCompare(
 
 	// Pull the data prior to waiting for the status as otherwise it will
 	// deadlock.
-	skipped := checkCollections(t, ctx, totalKopiaItems, expectedData, dcs, config.opts.RestorePermissions)
+	skipped := checkCollections(
+		t,
+		ctx,
+		totalKopiaItems,
+		expectedData,
+		dcs,
+		config.dest,
+		config.opts.RestorePermissions)
 
 	status := backupGC.AwaitStatus()
 
@@ -995,7 +1002,15 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames
 
 			// Pull the data prior to waiting for the status as otherwise it will
 			// deadlock.
-			skipped := checkCollections(t, ctx, allItems, allExpectedData, dcs, true)
+			skipped := checkCollections(
+				t,
+				ctx,
+				allItems,
+				allExpectedData,
+				dcs,
+				// Alright to be empty, needed for OneDrive.
+				control.RestoreDestination{},
+				true)
 
 			status := backupGC.AwaitStatus()
 			assert.Equal(t, allItems+skipped, status.ObjectCount, "status.ObjectCount")

src/internal/connector/onedrive/collections.go

@@ -13,6 +13,7 @@ import (
 	"github.com/pkg/errors"
 	"golang.org/x/exp/maps"
 
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/support"
 	"github.com/alcionai/corso/src/internal/data"
@@ -562,8 +563,26 @@ func (c *Collections) UpdateCollections(
 			return err
 		}
 
+		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, collectionPath, c.matcher, driveName) {
+		if shouldSkipDrive(ctx, itemPath, c.matcher, driveName) &&
+			shouldSkipDrive(ctx, collectionPath, c.matcher, driveName) {
 			logger.Ctx(ctx).Infof("Skipping path %s", collectionPath.String())
 			continue
 		}
@@ -584,13 +603,6 @@ func (c *Collections) UpdateCollections(
 				// the deleted folder/package.
 				delete(newPaths, *item.GetId())
 
-				// TODO(meain): Directory metadata files should be
-				// moved into the directory instead of having a
-				// `.dirmeta` file at the same level as the
-				// directory. This way we can make sure it is moved
-				// and deleted along with the directory and don't have
-				// to be handled separately.
-
 				if prevPath == nil {
 					// It is possible that an item was created and
 					// deleted between two delta invocations. In
@@ -616,33 +628,20 @@ func (c *Collections) UpdateCollections(
 				break
 			}
 
-			// Deletions of folders are handled in this case so we may as well start
-			// off by saving the path.Path of the item instead of just the OneDrive
-			// parentRef or such.
-			folderPath, err := collectionPath.Append(*item.GetName(), false)
-			if err != nil {
-				logger.Ctx(ctx).Errorw("failed building collection path", "error", err)
-				return err
-			}
-
 			// 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(), folderPath.String())
+			updatePath(newPaths, *item.GetId(), itemPath.String())
 
-			found, err := updateCollectionPaths(*item.GetId(), c.CollectionMap, folderPath)
+			found, err := updateCollectionPaths(*item.GetId(), c.CollectionMap, itemPath)
 			if err != nil {
 				return err
 			}
 
 			if !found {
-				// We only create collections for folder that are not
-				// new. This is so as to not create collections for
-				// new folders without any files within them.
-				if prevPath != nil {
 				col := NewCollection(
 					c.itemClient,
-						folderPath,
+					itemPath,
 					prevPath,
 					driveID,
 					c.service,
@@ -654,13 +653,20 @@ func (c *Collections) UpdateCollections(
 				c.CollectionMap[*item.GetId()] = col
 				c.NumContainers++
 			}
-			}
 
 			if c.source != OneDriveSource {
 				continue
 			}
 
-			fallthrough
+			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++
+				}
+			}
 
 		case item.GetFile() != nil:
 			if !invalidPrevDelta && item.GetFile() != nil {
@@ -702,6 +708,11 @@ func (c *Collections) UpdateCollections(
 
 			col, 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,
@@ -739,14 +750,6 @@ func (c *Collections) UpdateCollections(
 				if !removed {
 					return clues.New("removing from prev collection").With("item_id", *item.GetId())
 				}
-
-				// If that was the only item in that collection and is
-				// not getting added back, delete the collection
-				if itemColID != collectionID &&
-					pcollection.IsEmpty() &&
-					pcollection.State() == data.NewState {
-					delete(c.CollectionMap, itemColID)
-				}
 			}
 
 			itemCollection[*item.GetId()] = collectionID
@@ -754,12 +757,8 @@ func (c *Collections) UpdateCollections(
 
 			if collection.Add(item) {
 				c.NumItems++
-				if item.GetFile() != nil {
-					// This is necessary as we have a fallthrough for
-					// folders and packages
 				c.NumFiles++
 			}
-			}
 
 		default:
 			return errors.Errorf("item type not supported. item name : %s", *item.GetName())
@@ -770,6 +769,10 @@ 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)
 }

src/internal/connector/onedrive/collections_test.go

@@ -220,7 +220,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{
 				"root":   expectedPath(""),
@@ -240,7 +240,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{
 				"root":    expectedPath(""),
@@ -299,15 +299,16 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 				"subfolder": expectedStatePath(data.NewState, folderSub),
 				"folder2":   expectedStatePath(data.NewState, folderSub+folder),
 			},
-			expectedItemCount:      4,
+			expectedItemCount:      5,
 			expectedFileCount:      2,
 			expectedContainerCount: 3,
 			// 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(""),
-				"subfolder": expectedPath("/folder/subfolder"),
+				"folder":    expectedPath(folder),
-				"folder2":   expectedPath("/folder/subfolder/folder"),
+				"subfolder": expectedPath(folderSub),
+				"folder2":   expectedPath(folderSub + folder),
 			},
 			expectedExcludes: getDelList("fileInFolder", "fileInFolder2"),
 		},
@@ -332,12 +333,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 				"subfolder": expectedStatePath(data.NewState, folderSub),
 				"folder2":   expectedStatePath(data.NewState, folderSub+folder),
 			},
-			expectedItemCount:      2,
+			expectedItemCount:      3,
 			expectedFileCount:      1,
 			expectedContainerCount: 2,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
-				"folder2": expectedPath("/folder/subfolder/folder"),
+				"subfolder": expectedPath(folderSub),
+				"folder2":   expectedPath(folderSub + folder),
 			},
 			expectedExcludes: getDelList("fileInFolder2"),
 		},
@@ -359,12 +361,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 			expectedCollectionIDs: map[string]statePath{
 				"subfolder": expectedStatePath(data.NewState, folderSub),
 			},
-			expectedItemCount:      1,
+			expectedItemCount:      2,
 			expectedFileCount:      1,
 			expectedContainerCount: 1,
 			// No child folders for subfolder so nothing here.
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
+				"subfolder": expectedPath(folderSub),
 			},
 			expectedExcludes: getDelList("fileInSubfolder"),
 		},
@@ -375,22 +378,21 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 				driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false),
 			},
 			inputFolderMap: map[string]string{
-				"folder":    expectedPath("/folder"),
+				"folder":    expectedPath(folder),
-				"subfolder": expectedPath("/folder/subfolder"),
+				"subfolder": expectedPath(folderSub),
 			},
 			scope:  anyFolder,
 			expect: assert.NoError,
 			expectedCollectionIDs: map[string]statePath{
-				"root":   expectedStatePath(data.NotMovedState, ""),
+				"folder": expectedStatePath(data.NotMovedState, folder),
-				"folder": expectedStatePath(data.NotMovedState, "/folder"),
 			},
 			expectedItemCount:      1,
 			expectedFileCount:      0,
-			expectedContainerCount: 2,
+			expectedContainerCount: 1,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
-				"folder":    expectedPath("/folder"),
+				"folder":    expectedPath(folder),
-				"subfolder": expectedPath("/folder/subfolder"),
+				"subfolder": expectedPath(folderSub),
 			},
 			expectedExcludes: map[string]struct{}{},
 		},
@@ -407,16 +409,15 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 			scope:  anyFolder,
 			expect: assert.NoError,
 			expectedCollectionIDs: map[string]statePath{
-				"root":   expectedStatePath(data.NotMovedState, ""),
+				"folder": expectedStatePath(data.MovedState, folder, "/a-folder"),
-				"folder": expectedStatePath(data.MovedState, "/folder", "/a-folder"),
 			},
 			expectedItemCount:      1,
 			expectedFileCount:      0,
-			expectedContainerCount: 2,
+			expectedContainerCount: 1,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
-				"folder":    expectedPath("/folder"),
+				"folder":    expectedPath(folder),
-				"subfolder": expectedPath("/folder/subfolder"),
+				"subfolder": expectedPath(folderSub),
 			},
 			expectedExcludes: map[string]struct{}{},
 		},
@@ -428,20 +429,19 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 				driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false),
 			},
 			inputFolderMap: map[string]string{
-				"folder": expectedPath("/folder"),
+				"folder": expectedPath(folder),
 			},
 			scope:  anyFolder,
 			expect: assert.NoError,
 			expectedCollectionIDs: map[string]statePath{
-				"root":   expectedStatePath(data.NotMovedState, ""),
+				"folder": expectedStatePath(data.NotMovedState, folder),
-				"folder": expectedStatePath(data.NotMovedState, "/folder"),
 			},
 			expectedItemCount:      2,
 			expectedFileCount:      1,
-			expectedContainerCount: 2,
+			expectedContainerCount: 1,
 			expectedMetadataPaths: map[string]string{
 				"root":   expectedPath(""),
-				"folder": expectedPath("/folder"),
+				"folder": expectedPath(folder),
 			},
 			expectedExcludes: getDelList("file"),
 		},
@@ -457,12 +457,11 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 			scope:          anyFolder,
 			expect:         assert.NoError,
 			expectedCollectionIDs: map[string]statePath{
-				"root":   expectedStatePath(data.NotMovedState, ""),
 				"folder": expectedStatePath(data.NewState, "/folder2"),
 			},
-			expectedItemCount:      3, // permissions gets saved twice for folder
+			expectedItemCount:      2,
 			expectedFileCount:      1,
-			expectedContainerCount: 2,
+			expectedContainerCount: 1,
 			expectedMetadataPaths: map[string]string{
 				"root":   expectedPath(""),
 				"folder": expectedPath("/folder2"),
@@ -480,15 +479,14 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 			scope:          anyFolder,
 			expect:         assert.NoError,
 			expectedCollectionIDs: map[string]statePath{
-				"root":   expectedStatePath(data.NotMovedState, ""),
+				"folder": expectedStatePath(data.NewState, folder),
-				"folder": expectedStatePath(data.NewState, "/folder"),
 			},
 			expectedItemCount:      2,
 			expectedFileCount:      1,
-			expectedContainerCount: 2,
+			expectedContainerCount: 1,
 			expectedMetadataPaths: map[string]string{
 				"root":   expectedPath(""),
-				"folder": expectedPath("/folder"),
+				"folder": expectedPath(folder),
 			},
 			expectedExcludes: getDelList("file"),
 		},
@@ -506,16 +504,15 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 			scope:  anyFolder,
 			expect: assert.NoError,
 			expectedCollectionIDs: map[string]statePath{
-				"root":      expectedStatePath(data.NotMovedState, ""),
+				"folder":    expectedStatePath(data.MovedState, folder, "/a-folder"),
-				"folder":    expectedStatePath(data.MovedState, "/folder", "/a-folder"),
 				"subfolder": expectedStatePath(data.MovedState, "/subfolder", "/a-folder/subfolder"),
 			},
 			expectedItemCount:      2,
 			expectedFileCount:      0,
-			expectedContainerCount: 3,
+			expectedContainerCount: 2,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
-				"folder":    expectedPath("/folder"),
+				"folder":    expectedPath(folder),
 				"subfolder": expectedPath("/subfolder"),
 			},
 			expectedExcludes: map[string]struct{}{},
@@ -534,16 +531,15 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 			scope:  anyFolder,
 			expect: assert.NoError,
 			expectedCollectionIDs: map[string]statePath{
-				"root":      expectedStatePath(data.NotMovedState, ""),
+				"folder":    expectedStatePath(data.MovedState, folder, "/a-folder"),
-				"folder":    expectedStatePath(data.MovedState, "/folder", "/a-folder"),
 				"subfolder": expectedStatePath(data.MovedState, "/subfolder", "/a-folder/subfolder"),
 			},
 			expectedItemCount:      2,
 			expectedFileCount:      0,
-			expectedContainerCount: 3,
+			expectedContainerCount: 2,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
-				"folder":    expectedPath("/folder"),
+				"folder":    expectedPath(folder),
 				"subfolder": expectedPath("/subfolder"),
 			},
 			expectedExcludes: map[string]struct{}{},
@@ -554,6 +550,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 				driveRootItem("root"),
 				driveItem("folder2", "folder2", testBaseDrivePath, "root", false, true, false),
 				driveItem("itemInFolder2", "itemInFolder2", testBaseDrivePath+"/folder2", "folder2", true, false, false),
+				// Need to see the parent folder first (expected since that's what Graph
+				// consistently returns).
+				driveItem("folder", "a-folder", testBaseDrivePath, "root", false, true, false),
 				driveItem("subfolder", "subfolder", testBaseDrivePath+"/a-folder", "folder", false, true, false),
 				driveItem(
 					"itemInSubfolder",
@@ -573,14 +572,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"),
-				"folder":    expectedStatePath(data.MovedState, "/folder", "/a-folder"),
 				"folder2":   expectedStatePath(data.NewState, "/folder2"),
-				"subfolder": expectedStatePath(data.MovedState, "/folder/subfolder", "/a-folder/subfolder"),
+				"subfolder": expectedStatePath(data.MovedState, folderSub, "/a-folder/subfolder"),
 			},
 			expectedItemCount:      5,
 			expectedFileCount:      2,
-			expectedContainerCount: 4,
+			expectedContainerCount: 3,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
 				"folder":    expectedPath("/folder"),
@@ -604,12 +602,11 @@ 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:      3,
+			expectedItemCount:      2,
 			expectedFileCount:      1,
-			expectedContainerCount: 2,
+			expectedContainerCount: 1,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
 				"folder":    expectedPath("/folder2"),
@@ -678,13 +675,12 @@ 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", "/folder/subfolder"),
+				"subfolder": expectedStatePath(data.MovedState, "/subfolder", folderSub),
 			},
 			expectedItemCount:      1,
 			expectedFileCount:      0,
-			expectedContainerCount: 2,
+			expectedContainerCount: 1,
 			expectedMetadataPaths: map[string]string{
 				"root":      expectedPath(""),
 				"subfolder": expectedPath("/subfolder"),
@@ -748,7 +744,11 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 			assert.Equal(t, tt.expectedContainerCount, c.NumContainers, "container count")
 
 			for id, sp := range tt.expectedCollectionIDs {
-				assert.Containsf(t, c.CollectionMap, id, "contains collection with id %s", id)
+				if !assert.Containsf(t, c.CollectionMap, id, "missing collection with id %s", id) {
+					// Skip collections we don't find so we don't get an NPE.
+					continue
+				}
+
 				assert.Equalf(t, sp.state, c.CollectionMap[id].State(), "state for collection %s", id)
 				assert.Equalf(t, sp.curPath, c.CollectionMap[id].FullPath(), "current path for collection %s", id)
 				assert.Equalf(t, sp.prevPath, c.CollectionMap[id].PreviousPath(), "prev path for collection %s", id)
@@ -1294,8 +1294,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1329,8 +1328,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1364,7 +1362,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				rootFolderPath1: {data.NotMovedState: {"folder", "file"}},
+				rootFolderPath1: {data.NotMovedState: {"file"}},
+				folderPath1:     {data.NewState: {"folder"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1397,8 +1396,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
 			},
 			expectedDeltaURLs:   map[string]string{},
 			expectedFolderPaths: map[string]map[string]string{},
@@ -1432,8 +1430,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file", "file2"}},
+				folderPath1: {data.NewState: {"folder", "file", "file2"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1480,10 +1477,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID2: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
-				folderPath2:     {data.NewState: {"file2"}},
+				folderPath2: {data.NewState: {"folder2", "file2"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
-				rootFolderPath2: {data.NotMovedState: {"folder2"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1579,8 +1574,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 			},
 			errCheck: assert.NoError,
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				expectedPath1(""):        {data.NotMovedState: {"file", "folder"}},
+				expectedPath1(""):        {data.NotMovedState: {"file"}},
-				expectedPath1("/folder"): {data.NewState: {"file2"}},
+				expectedPath1("/folder"): {data.NewState: {"folder", "file2"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1621,8 +1616,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				expectedPath1(""):        {data.NotMovedState: {"file", "folder"}},
+				expectedPath1(""):        {data.NotMovedState: {"file"}},
-				expectedPath1("/folder"): {data.NewState: {"file2"}},
+				expectedPath1("/folder"): {data.NewState: {"folder", "file2"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1662,9 +1657,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				expectedPath1(""):         {data.NotMovedState: {"folder2"}},
 				expectedPath1("/folder"):  {data.DeletedState: {}},
-				expectedPath1("/folder2"): {data.NewState: {"file"}},
+				expectedPath1("/folder2"): {data.NewState: {"folder2", "file"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1704,8 +1698,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				expectedPath1(""):        {data.NotMovedState: {"folder2"}},
+				expectedPath1("/folder"): {data.NewState: {"folder2", "file"}},
-				expectedPath1("/folder"): {data.NewState: {"file"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1835,9 +1828,9 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 					t,
 					test.expectedCollections[folderPath][baseCol.State()],
 					itemIDs,
-					"items in collection %s",
+					"state: %d, path: %s",
-					folderPath,
+					baseCol.State(),
-				)
+					folderPath)
 				assert.Equal(t, test.doNotMergeItems, baseCol.DoNotMergeItems(), "DoNotMergeItems")
 			}
 

src/internal/connector/onedrive/restore.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"math"
 	"runtime/trace"
 	"sort"
 	"strings"
@@ -39,9 +38,11 @@ const (
 	// versionWithNameInMeta points to the backup format version where we begin
 	// storing files in kopia with their item ID instead of their OneDrive file
 	// name.
-	// TODO(ashmrtn): Update this to a real value when we merge the file name
+	versionWithNameInMeta = 5
-	// change. Set to MAXINT for now to keep the if-check using it working.
+	// versionWithDataAndMetaFilesInDir moves the .dirmeta entries to the
-	versionWithNameInMeta = math.MaxInt
+	// directory they belong to instead of being in the parent of the directory
+	// they belong to.
+	versionWithDataAndMetaFilesInDir = 3
 )
 
 func getParentPermissions(
@@ -66,12 +67,14 @@ func getParentPermissions(
 }
 
 func getParentAndCollectionPermissions(
+	ctx context.Context,
 	drivePath *path.DrivePath,
-	collectionPath path.Path,
+	dc data.RestoreCollection,
 	permissions map[string][]UserPermission,
+	backupVersion int,
 	restorePerms bool,
 ) ([]UserPermission, []UserPermission, error) {
-	if !restorePerms {
+	if !restorePerms || backupVersion < versionWithDataAndMetaFiles {
 		return nil, nil, nil
 	}
 
@@ -79,6 +82,7 @@ func getParentAndCollectionPermissions(
 		err            error
 		parentPerms    []UserPermission
 		colPerms       []UserPermission
+		collectionPath = dc.FullPath()
 	)
 
 	// Only get parent permissions if we're not restoring the root.
@@ -94,12 +98,25 @@ func getParentAndCollectionPermissions(
 		}
 	}
 
-	// TODO(ashmrtn): For versions after this pull the permissions from the
+	if backupVersion < versionWithDataAndMetaFilesInDir {
-	// current collection with Fetch().
 		colPerms, err = getParentPermissions(collectionPath, permissions)
 		if err != nil {
 			return nil, nil, clues.Wrap(err, "getting collection permissions")
 		}
+	} else if len(drivePath.Folders) > 0 {
+		// Root folder doesn't have a metadata file associated with it.
+		folders := collectionPath.Folders()
+
+		meta, err := fetchAndReadMetadata(
+			ctx,
+			dc,
+			folders[len(folders)-1]+DirMetaFileSuffix)
+		if err != nil {
+			return nil, nil, clues.Wrap(err, "collection permissions")
+		}
+
+		colPerms = meta.Permissions
+	}
 
 	return parentPerms, colPerms, nil
 }
@@ -223,9 +240,11 @@ func RestoreCollection(
 		"destination", restoreFolderElements)
 
 	parentPerms, colPerms, err := getParentAndCollectionPermissions(
+		ctx,
 		drivePath,
-		dc.FullPath(),
+		dc,
 		parentPermissions,
+		backupVersion,
 		restorePerms)
 	if err != nil {
 		errUpdater(directory.String(), err)
@@ -330,7 +349,10 @@ func RestoreCollection(
 					// RestoreOp, so we still need to handle them in some way.
 					continue
 				} else if strings.HasSuffix(name, DirMetaFileSuffix) {
-					if !restorePerms {
+					// Only the versionWithDataAndMetaFiles needed to deserialize the
+					// permission for child folders here. Later versions can request
+					// permissions inline when processing the collection.
+					if !restorePerms || backupVersion >= versionWithDataAndMetaFilesInDir {
 						continue
 					}
 

src/internal/operations/backup.go

@@ -289,6 +289,11 @@ func (op *BackupOperation) do(
 // checker to see if conditions are correct for incremental backup behavior such as
 // retrieving metadata like delta tokens and previous paths.
 func useIncrementalBackup(sel selectors.Selector, opts control.Options) bool {
+	// TODO(meain): remove this once we stabilize delta incrementals for OneDrive
+	if sel.Service == selectors.ServiceOneDrive {
+		return opts.ToggleFeatures.EnableOneDriveDeltaIncrementals
+	}
+
 	// Delta-based incrementals currently only supported for Exchange
 	if sel.Service != selectors.ServiceExchange {
 		return false

src/pkg/backup/backup.go

@@ -14,7 +14,7 @@ import (
 	"github.com/alcionai/corso/src/pkg/selectors"
 )
 
-const Version = 2
+const Version = 5
 
 // Backup represents the result of a backup operation
 type Backup struct {

src/pkg/control/options.go

@@ -80,4 +80,10 @@ type Toggles struct {
 	// permissions. Permission metadata increases graph api call count,
 	// so disabling their retrieval when not needed is advised.
 	EnablePermissionsBackup bool `json:"enablePermissionsBackup,omitempty"`
+
+	// EnableOneDriveDeltaIncrementals is used to enable OneDrive
+	// delta incrementals. It is set to false by default as OneDrive
+	// delta incrementals is still in development. This flag works
+	// independent of DisableIncrementals.
+	EnableOneDriveDeltaIncrementals bool `json:"enableOneDriveDeltaIncrementals,omitempty"`
 }