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?

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

#### Type of change

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

#### Issue(s)

* closes #4012

#### Test Plan

- [x] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-08-16 10:37:30 -07:00 committed by GitHub
parent 8424c69934
commit 75c3e5cd33
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 7 additions and 37 deletions

View File

@ -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)
}

View File

@ -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")
})
}
}