From f3c98c8fb71e4e29d06a74b6cb1f56ca10103733 Mon Sep 17 00:00:00 2001
From: Abin Simon <abinsimon10@gmail.com>
Date: Tue, 14 Feb 2023 10:43:52 +0530
Subject: [PATCH] Fix OneDrive permissions not getting backed up (#2473)

## Description

When we do a new backup, we look up the old permissions assuming they are accurate, but the permissions from the previous backup might be incorrect as we may have run it with permissions backup disabled.

It can also happen the other way round where even if we ask it to disable permissions backup, it can still end up having permissions which are pulled from the previous backup.

This fixes this behavior by setting the mod time for the backup info to the current time.

- [x] Add tests
- [x] Address TODOs in the PR

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

- [ ] :white_check_mark: Yes, it's included
- [ ] :clock1: Yes, but in a later PR
- [x] :no_entry: No

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Test
- [ ] :computer: CI/Deployment
- [ ] :broom: Tech Debt/Cleanup

## Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* fixes https://github.com/alcionai/corso/issues/2472

## Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] :muscle: Manual
- [x] :zap: Unit test
- [ ] :green_heart: E2E
---
 src/internal/connector/onedrive/collection.go | 19 +++-
 .../connector/onedrive/collection_test.go     | 90 +++++++++++++++++++
 2 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go
index 54f66fda3..59494856b 100644
--- a/src/internal/connector/onedrive/collection.go
+++ b/src/internal/connector/onedrive/collection.go
@@ -402,10 +402,27 @@ func (oc *Collection) populateItems(ctx context.Context) {
 					return progReader, nil
 				})
 
+				// TODO(meain): Remove this once we change to always
+				// backing up permissions. Until then we cannot rely
+				// on weather the previous data is what we need as the
+				// user might have not backup up permissions in the
+				// previous run.
+				metaItemInfo := details.ItemInfo{}
+				metaItemInfo.OneDrive = &details.OneDriveInfo{
+					Created:    itemInfo.OneDrive.Created,
+					ItemName:   itemInfo.OneDrive.ItemName,
+					DriveName:  itemInfo.OneDrive.DriveName,
+					ItemType:   itemInfo.OneDrive.ItemType,
+					Modified:   time.Now(), // set to current time to always refresh
+					Owner:      itemInfo.OneDrive.Owner,
+					ParentPath: itemInfo.OneDrive.ParentPath,
+					Size:       itemInfo.OneDrive.Size,
+				}
+
 				oc.data <- &Item{
 					id:   itemName + metaSuffix,
 					data: metaReader,
-					info: itemInfo,
+					info: metaItemInfo,
 				}
 			}
 
diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go
index 4ca6a9850..5379a53e2 100644
--- a/src/internal/connector/onedrive/collection_test.go
+++ b/src/internal/connector/onedrive/collection_test.go
@@ -428,3 +428,93 @@ func (suite *CollectionUnitTestSuite) TestCollectionDisablePermissionsBackup() {
 		})
 	}
 }
+
+// TODO(meain): Remove this test once we start always backing up permissions
+func (suite *CollectionUnitTestSuite) TestCollectionPermissionBackupLatestModTime() {
+	table := []struct {
+		name   string
+		source driveSource
+	}{
+		{
+			name:   "oneDrive",
+			source: OneDriveSource,
+		},
+	}
+	for _, test := range table {
+		suite.T().Run(test.name, func(t *testing.T) {
+			var (
+				testItemID   = "fakeItemID"
+				testItemName = "Fake Item"
+				testItemSize = int64(10)
+
+				collStatus = support.ConnectorOperationStatus{}
+				wg         = sync.WaitGroup{}
+			)
+
+			wg.Add(1)
+
+			folderPath, err := GetCanonicalPath("drive/driveID1/root:/folderPath", "a-tenant", "a-user", test.source)
+			require.NoError(t, err)
+
+			coll := NewCollection(
+				graph.HTTPClient(graph.NoTimeout()),
+				folderPath,
+				nil,
+				"drive-id",
+				suite,
+				suite.testStatusUpdater(&wg, &collStatus),
+				test.source,
+				control.Options{ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}},
+				true)
+
+			mtime := time.Now().AddDate(0, -1, 0)
+			mockItem := models.NewDriveItem()
+			mockItem.SetFile(models.NewFile())
+			mockItem.SetId(&testItemID)
+			mockItem.SetName(&testItemName)
+			mockItem.SetSize(&testItemSize)
+			mockItem.SetCreatedDateTime(&mtime)
+			mockItem.SetLastModifiedDateTime(&mtime)
+			coll.Add(mockItem)
+
+			coll.itemReader = func(
+				*http.Client,
+				models.DriveItemable,
+			) (details.ItemInfo, io.ReadCloser, error) {
+				return details.ItemInfo{OneDrive: &details.OneDriveInfo{ItemName: "fakeName", Modified: time.Now()}},
+					io.NopCloser(strings.NewReader("Fake Data!")),
+					nil
+			}
+
+			coll.itemMetaReader = func(_ context.Context,
+				_ graph.Servicer,
+				_ string,
+				_ models.DriveItemable,
+			) (io.ReadCloser, int, error) {
+				return io.NopCloser(strings.NewReader(`{}`)), 16, nil
+			}
+
+			readItems := []data.Stream{}
+			for item := range coll.Items() {
+				readItems = append(readItems, item)
+			}
+
+			wg.Wait()
+
+			// Expect no items
+			require.Equal(t, 1, collStatus.ObjectCount)
+			require.Equal(t, 1, collStatus.Successful)
+
+			for _, i := range readItems {
+				if strings.HasSuffix(i.UUID(), MetaFileSuffix) {
+					content, err := io.ReadAll(i.ToReader())
+					require.NoError(t, err)
+					require.Equal(t, content, []byte("{}"))
+					im, ok := i.(data.StreamModTime)
+					require.Equal(t, ok, true, "modtime interface")
+					require.Greater(t, im.ModTime(), mtime, "permissions time greater than mod time")
+				}
+			}
+		})
+	}
+}