Store OneDrive directory metadata in directory (#2604)

## Description

This PR does a couple of things, some of which are to make storing the directory metadata in the directory easier:
* backup empty folders
* expand what selectors match on slightly so it includes the folder specified in the selector (e.x. match on prefix `/foo` will now include the folder `/foo` and subfolders under it)
* add restore code path for having directory metadata in the directory
* update backup code path to store directory metadata in the directory
* bump the backup version so we can tell this apart from the previous version

The above should mostly be split out by commit if that makes reviewing easier

Storing the directory metadata in the directory allows removing the data dependency between
restoring parent directories and child directories (though there may still be a dependency there for permissions inheritance). This makes it so that the order kopia returns restore data does not matter

It also unblocks some of the OneDrive delta token-based incremental backup work as we no longer have to worry about directory metadata when moving/deleting a directory

## Does this PR need a docs update or release note?

- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ No 

## Type of change

- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

## Issue(s)

* closes #2447
* closes #2532

## Test Plan

- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

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()
@@ -722,7 +729,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]
@@ -732,12 +739,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
@@ -748,7 +755,7 @@ func compareOneDriveItem(
 
 		if !restorePermissions {
 			assert.Equal(t, 0, len(itemMeta.Permissions))
-			return
+			return true
 		}
 
 		testElementsMatch(
@@ -758,19 +765,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
@@ -779,16 +786,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())
 	}
@@ -807,11 +820,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(
@@ -832,7 +847,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
@@ -842,6 +857,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
@@ -851,10 +867,12 @@ func checkCollections(
 	gotItems := 0
 
 	for _, returned := range got {
-		startingItems := gotItems
-		service := returned.FullPath().Service()
-		category := returned.FullPath().Category()
-		expectedColData := expected[returned.FullPath().String()]
+		var (
+			hasItems        bool
+			service         = returned.FullPath().Service()
+			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
@@ -872,16 +890,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

@@ -173,7 +173,7 @@ func (c *onedriveCollection) withFile(
 			name+onedrive.DataFileSuffix,
 			fileData))
 
-	case 1, 2, 3:
+	case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker, version.OneDrive4DirIncludesPermissions:
 		c.items = append(c.items, onedriveItemWithData(
 			c.t,
 			name+onedrive.DataFileSuffix,
@@ -202,10 +202,10 @@ func (c *onedriveCollection) withFolder(
 	roles []string,
 ) *onedriveCollection {
 	switch c.backupVersion {
-	case 0:
+	case 0, version.OneDrive4DirIncludesPermissions:
 		return c
 
-	case 1, 2, 3:
+	case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker:
 		c.items = append(
 			c.items,
 			onedriveMetadata(
@@ -230,7 +230,7 @@ func (c *onedriveCollection) withPermissions(
 ) *onedriveCollection {
 	// These versions didn't store permissions for the folder or didn't store them
 	// in the folder's collection.
-	if c.backupVersion < version.OneDriveXIncludesPermissions {
+	if c.backupVersion < version.OneDrive4DirIncludesPermissions {
 		return c
 	}
 
@@ -240,15 +240,15 @@ func (c *onedriveCollection) withPermissions(
 		return c
 	}
 
-	c.items = append(
-		c.items,
-		onedriveMetadata(
-			c.t,
-			name,
-			name+onedrive.DirMetaFileSuffix,
-			user,
-			roles),
-	)
+	metadata := onedriveMetadata(
+		c.t,
+		name,
+		name+onedrive.DirMetaFileSuffix,
+		user,
+		roles)
+
+	c.items = append(c.items, metadata)
+	c.aux = append(c.aux, metadata)
 
 	return c
 }
@@ -349,7 +349,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Multip
 	table := []onedriveTest{
 		{
 			name:         "WithMetadata",
-			startVersion: 1,
+			startVersion: version.OneDrive1DataAndMetaFiles,
 			cols: []onedriveColInfo{
 				{
 					pathElements: rootPath,
@@ -375,6 +375,10 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreAndBackup_Multip
 				},
 				{
 					pathElements: folderBPath,
+					perms: permData{
+						user:  suite.secondaryUser,
+						roles: readPerm,
+					},
 					files: []itemData{
 						{
 							name: fileName,
@@ -516,7 +520,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
 		folderBName,
 	}
 
-	startVersion := 1
+	startVersion := version.OneDrive1DataAndMetaFiles
 
 	table := []onedriveTest{
 		{
@@ -731,7 +735,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsBackupAndNoR
 		suite.user,
 	)
 
-	startVersion := 1
+	startVersion := version.OneDrive1DataAndMetaFiles
 
 	table := []onedriveTest{
 		{
@@ -891,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

@@ -493,7 +493,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()
 
@@ -1000,7 +1007,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,51 +628,45 @@ 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,
-						prevPath,
-						driveID,
-						c.service,
-						c.statusUpdater,
-						c.source,
-						c.ctrl,
-						invalidPrevDelta,
-					)
-					c.CollectionMap[*item.GetId()] = col
-					c.NumContainers++
-				}
+				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 {
 				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,11 +757,7 @@ 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++
-				}
+				c.NumFiles++
 			}
 
 		default:
@@ -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

@@ -222,7 +222,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(""),
@@ -242,7 +242,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(""),
@@ -301,15 +301,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"),
-				"folder2":   expectedPath("/folder/subfolder/folder"),
+				"folder":    expectedPath(folder),
+				"subfolder": expectedPath(folderSub),
+				"folder2":   expectedPath(folderSub + folder),
 			},
 			expectedExcludes: getDelList("fileInFolder", "fileInFolder2"),
 		},
@@ -334,12 +335,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"),
+				"root":      expectedPath(""),
+				"subfolder": expectedPath(folderSub),
+				"folder2":   expectedPath(folderSub + folder),
 			},
 			expectedExcludes: getDelList("fileInFolder2"),
 		},
@@ -361,12 +363,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(""),
+				"root":      expectedPath(""),
+				"subfolder": expectedPath(folderSub),
 			},
 			expectedExcludes: getDelList("fileInSubfolder"),
 		},
@@ -377,22 +380,21 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() {
 				driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false),
 			},
 			inputFolderMap: map[string]string{
-				"folder":    expectedPath("/folder"),
-				"subfolder": expectedPath("/folder/subfolder"),
+				"folder":    expectedPath(folder),
+				"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"),
-				"subfolder": expectedPath("/folder/subfolder"),
+				"folder":    expectedPath(folder),
+				"subfolder": expectedPath(folderSub),
 			},
 			expectedExcludes: map[string]struct{}{},
 		},
@@ -409,16 +411,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"),
-				"subfolder": expectedPath("/folder/subfolder"),
+				"folder":    expectedPath(folder),
+				"subfolder": expectedPath(folderSub),
 			},
 			expectedExcludes: map[string]struct{}{},
 		},
@@ -430,20 +431,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"),
 		},
@@ -459,12 +459,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"),
@@ -482,15 +481,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"),
 		},
@@ -508,16 +506,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{}{},
@@ -536,16 +533,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{}{},
@@ -556,6 +552,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",
@@ -575,14 +574,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"),
@@ -606,12 +604,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"),
@@ -680,13 +677,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"),
@@ -752,7 +748,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)
@@ -1299,8 +1299,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1334,8 +1333,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1369,7 +1367,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,
@@ -1402,8 +1401,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
 			},
 			expectedDeltaURLs:   map[string]string{},
 			expectedFolderPaths: map[string]map[string]string{},
@@ -1437,8 +1435,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file", "file2"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
+				folderPath1: {data.NewState: {"folder", "file", "file2"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1485,10 +1482,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID2: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				folderPath1:     {data.NewState: {"file"}},
-				folderPath2:     {data.NewState: {"file2"}},
-				rootFolderPath1: {data.NotMovedState: {"folder"}},
-				rootFolderPath2: {data.NotMovedState: {"folder2"}},
+				folderPath1: {data.NewState: {"folder", "file"}},
+				folderPath2: {data.NewState: {"folder2", "file2"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1584,8 +1579,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 			},
 			errCheck: assert.NoError,
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				expectedPath1(""):        {data.NotMovedState: {"file", "folder"}},
-				expectedPath1("/folder"): {data.NewState: {"file2"}},
+				expectedPath1(""):        {data.NotMovedState: {"file"}},
+				expectedPath1("/folder"): {data.NewState: {"folder", "file2"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1626,8 +1621,8 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				driveID1: {},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				expectedPath1(""):        {data.NotMovedState: {"file", "folder"}},
-				expectedPath1("/folder"): {data.NewState: {"file2"}},
+				expectedPath1(""):        {data.NotMovedState: {"file"}},
+				expectedPath1("/folder"): {data.NewState: {"folder", "file2"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1667,9 +1662,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,
@@ -1709,8 +1703,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 				},
 			},
 			expectedCollections: map[string]map[data.CollectionState][]string{
-				expectedPath1(""):        {data.NotMovedState: {"folder2"}},
-				expectedPath1("/folder"): {data.NewState: {"file"}},
+				expectedPath1("/folder"): {data.NewState: {"folder2", "file"}},
 			},
 			expectedDeltaURLs: map[string]string{
 				driveID1: delta,
@@ -1842,9 +1835,9 @@ func (suite *OneDriveCollectionsSuite) TestGet() {
 					t,
 					test.expectedCollections[folderPath][baseCol.State()],
 					itemIDs,
-					"items in collection %s",
-					folderPath,
-				)
+					"state: %d, path: %s",
+					baseCol.State(),
+					folderPath)
 				assert.Equal(t, test.doNotMergeItems, baseCol.DoNotMergeItems(), "DoNotMergeItems")
 			}
 

src/internal/connector/onedrive/permission.go

@@ -4,11 +4,14 @@ import (
 	"context"
 
 	"github.com/alcionai/clues"
-	"github.com/alcionai/corso/src/internal/connector/graph"
-	"github.com/alcionai/corso/src/pkg/path"
 	msdrive "github.com/microsoftgraph/msgraph-sdk-go/drive"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"github.com/pkg/errors"
+
+	"github.com/alcionai/corso/src/internal/connector/graph"
+	"github.com/alcionai/corso/src/internal/data"
+	"github.com/alcionai/corso/src/internal/version"
+	"github.com/alcionai/corso/src/pkg/path"
 )
 
 func getParentPermissions(
@@ -33,16 +36,23 @@ 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 < version.OneDrive1DataAndMetaFiles {
 		return nil, nil, nil
 	}
 
-	var parentPerms []UserPermission
+	var (
+		err            error
+		parentPerms    []UserPermission
+		colPerms       []UserPermission
+		collectionPath = dc.FullPath()
+	)
 
 	// Only get parent permissions if we're not restoring the root.
 	if len(drivePath.Folders) > 0 {
@@ -57,11 +67,24 @@ func getParentAndCollectionPermissions(
 		}
 	}
 
-	// TODO(ashmrtn): For versions after this pull the permissions from the
-	// current collection with Fetch().
-	colPerms, err := getParentPermissions(collectionPath, permissions)
-	if err != nil {
-		return nil, nil, clues.Wrap(err, "getting collection permissions")
+	if backupVersion < version.OneDrive4DirIncludesPermissions {
+		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

src/internal/connector/onedrive/restore.go

@@ -170,9 +170,11 @@ func RestoreCollection(
 	logger.Ctx(ctx).Info("restoring onedrive collection")
 
 	parentPerms, colPerms, err := getParentAndCollectionPermissions(
+		ctx,
 		drivePath,
-		dc.FullPath(),
+		dc,
 		parentPermissions,
+		backupVersion,
 		restorePerms)
 	if err != nil {
 		return metrics, folderPerms, permissionIDMappings, clues.Wrap(err, "getting permissions").WithClues(ctx)
@@ -278,7 +280,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 version.OneDrive1DataAndMetaFiles needed to deserialize the
+					// permission for child folders here. Later versions can request
+					// permissions inline when processing the collection.
+					if !restorePerms || backupVersion >= version.OneDrive4DirIncludesPermissions {
 						continue
 					}
 

src/internal/version/backup.go

@@ -2,7 +2,7 @@ package version
 
 import "math"
 
-const Backup = 3
+const Backup = 4
 
 // Various labels to refer to important version changes.
 // Labels don't need 1:1 service:version representation.  Add a new
@@ -21,12 +21,9 @@ const (
 	// specifies if the file is a meta file or a data file.
 	OneDrive3IsMetaMarker = 3
 
-	// OneDrive4IncludesPermissions includes permissions in the backup.
-	// Note that this is larger than the current backup version.  That's
-	// because it isn't implemented yet.  But we have tests based on this,
-	// so maybe we just keep bumping the verson ahead of the backup until
-	// it gets implemented.
-	OneDriveXIncludesPermissions = Backup + 1
+	// OneDrive4IncludesPermissions includes permissions for folders in the same
+	// collection as the folder itself.
+	OneDrive4DirIncludesPermissions = 4
 
 	// OneDriveXNameInMeta points to the backup format version where we begin
 	// storing files in kopia with their item ID instead of their OneDrive file