From 75c3e5cd339211402fa1bd308f033009872c737b Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Wed, 16 Aug 2023 10:37:30 -0700 Subject: [PATCH] Don't munge drive-based ShortRefs (#4009) Munging code originally existed so that SDK users would get unique ShortRefs if an item moved or was renamed. However, we no longer need to support that so this removes that munging code Manually tested making a new backup with this PR where the merge base was a backup made prior to this PR. A folder with a subfolder and items was moved between the two backups. Backup details for the new backup contained all the expected entries Restore selector logic should be unaffected as that assumes the user has passed in a ShortRef that was previously printed to the CLI by `corso backup details`. Input is compared against the ShortRef stored in the entry, `ShortRef()` is not called on any `Path` --- #### 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 - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * closes #4012 #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/pkg/backup/details/details.go | 20 -------------------- src/pkg/backup/details/details_test.go | 24 +++++++----------------- 2 files changed, 7 insertions(+), 37 deletions(-) diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index add52d809..4d224f475 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -51,26 +51,6 @@ func (d *Details) add( return entry, clues.New("item is not SharePoint or OneDrive type") } - filename := "" - if info.OneDrive != nil { - filename = info.OneDrive.ItemName - } else if info.SharePoint != nil { - filename = info.SharePoint.ItemName - } - - // Make the new path contain all display names and then the M365 item ID. - // This ensures the path will be unique, thus ensuring the ShortRef will be - // unique. - // - // If we appended the file's display name to the path then it's possible - // for a folder in the parent directory to have the same display name as the - // M365 ID of this file and also have a subfolder in the folder with a - // display name that matches the file's display name. That would result in - // duplicate ShortRefs, which we can't allow. - elements := repoRef.Elements() - elements = append(elements[:len(elements)-1], filename, repoRef.Item()) - entry.ShortRef = path.Builder{}.Append(elements...).ShortRef() - // clean metadata suffixes from item refs entry.ItemRef = withoutMetadataSuffix(entry.ItemRef) } diff --git a/src/pkg/backup/details/details_test.go b/src/pkg/backup/details/details_test.go index 6c9434867..43883ed5a 100644 --- a/src/pkg/backup/details/details_test.go +++ b/src/pkg/backup/details/details_test.go @@ -270,27 +270,18 @@ func (suite *DetailsUnitSuite) TestDetailsAdd_NoLocationFolders() { table := []struct { name string entry Entry - // shortRefEqual allows checking that OneDrive and SharePoint have their - // ShortRef updated in the returned entry. - // - // TODO(ashmrtn): Remove this when we don't need extra munging for - // OneDrive/SharePoint file name changes. - shortRefEqual assert.ComparisonAssertionFunc }{ { - name: "Exchange Email", - entry: exchangeEntry(t, itemID, 42, ExchangeMail), - shortRefEqual: assert.Equal, + name: "Exchange Email", + entry: exchangeEntry(t, itemID, 42, ExchangeMail), }, { - name: "OneDrive File", - entry: oneDriveishEntry(t, itemID, 42, OneDriveItem), - shortRefEqual: assert.NotEqual, + name: "OneDrive File", + entry: oneDriveishEntry(t, itemID, 42, OneDriveItem), }, { - name: "SharePoint File", - entry: oneDriveishEntry(t, itemID, 42, SharePointLibrary), - shortRefEqual: assert.NotEqual, + name: "SharePoint File", + entry: oneDriveishEntry(t, itemID, 42, SharePointLibrary), }, { name: "Legacy SharePoint File", @@ -300,7 +291,6 @@ func (suite *DetailsUnitSuite) TestDetailsAdd_NoLocationFolders() { return res }(), - shortRefEqual: assert.NotEqual, }, } @@ -334,7 +324,7 @@ func (suite *DetailsUnitSuite) TestDetailsAdd_NoLocationFolders() { got.ShortRef = "" assert.Equal(t, localItem, got, "DetailsEntry") - test.shortRefEqual(t, expectedShortRef, gotShortRef, "ShortRef") + assert.Equal(t, expectedShortRef, gotShortRef, "ShortRef") }) } }