diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index d3d4f0a9f..9215bc0d5 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -559,31 +559,6 @@ func getNewPathRefs( repoRef path.Path, backupVersion int, ) (path.Path, *path.Builder, error) { - // Right now we can't guarantee that we have an old location in the - // previous details entry so first try a lookup without a location to see - // if it matches so we don't need to try parsing from the old entry. - // - // TODO(ashmrtn): In the future we can remove this first check as we'll be - // able to assume we always have the location in the previous entry. We'll end - // up doing some extra parsing, but it will simplify this code. - if repoRef.Service() == path.ExchangeService { - newPath, newLoc, err := dataFromBackup.GetNewPathRefs( - repoRef.ToBuilder(), - entry.Modified(), - nil) - if err != nil { - return nil, nil, clues.Wrap(err, "getting new paths") - } else if newPath == nil { - // This entry doesn't need merging. - return nil, nil, nil - } else if newLoc == nil { - return nil, nil, clues.New("unable to find new exchange location") - } - - return newPath, newLoc, nil - } - - // We didn't have an exact entry, so retry with a location. locRef, err := entry.ToLocationIDer(backupVersion) if err != nil { return nil, nil, clues.Wrap(err, "getting previous item location") diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index 5b489e769..e8970ea1c 100644 --- a/src/internal/operations/backup_test.go +++ b/src/internal/operations/backup_test.go @@ -606,6 +606,24 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems time1 = time.Now() time2 = time1.Add(time.Hour) + + exchangeItemPath1 = makePath( + suite.T(), + []string{ + tenant, + path.ExchangeService.String(), + ro, + path.EmailCategory.String(), + "work", + "item1", + }, + true) + exchangeLocationPath1 = path.Builder{}.Append("work-display-name") + exchangePathReason1 = kopia.NewReason( + "", + exchangeItemPath1.ResourceOwner(), + exchangeItemPath1.Service(), + exchangeItemPath1.Category()) ) itemParents1, err := path.GetDriveFolderPath(itemPath1) @@ -803,6 +821,36 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems makeDetailsEntry(suite.T(), itemPath1, locationPath1, 42, false), }, }, + { + name: "ExchangeItemMerged", + mdm: func() *mockDetailsMergeInfoer { + res := newMockDetailsMergeInfoer() + res.add(exchangeItemPath1, exchangeItemPath1, exchangeLocationPath1) + + return res + }(), + inputBackups: []kopia.BackupEntry{ + { + Backup: &backup1, + Reasons: []identity.Reasoner{ + exchangePathReason1, + }, + }, + }, + populatedDetails: map[string]*details.Details{ + backup1.DetailsID: { + DetailsModel: details.DetailsModel{ + Entries: []details.Entry{ + *makeDetailsEntry(suite.T(), exchangeItemPath1, exchangeLocationPath1, 42, false), + }, + }, + }, + }, + errCheck: assert.NoError, + expectedEntries: []*details.Entry{ + makeDetailsEntry(suite.T(), exchangeItemPath1, exchangeLocationPath1, 42, false), + }, + }, { name: "ItemMergedSameLocation", mdm: func() *mockDetailsMergeInfoer { diff --git a/src/pkg/backup/details/details_test.go b/src/pkg/backup/details/details_test.go index 43883ed5a..1d5e57035 100644 --- a/src/pkg/backup/details/details_test.go +++ b/src/pkg/backup/details/details_test.go @@ -1353,7 +1353,7 @@ func (suite *DetailsUnitSuite) TestLocationIDer_FromEntry() { expectedUniqueLoc: fmt.Sprintf(expectedExchangeUniqueLocFmt, path.EmailCategory), }, { - name: "Exchange Email Without LocationRef Old Version Errors", + name: "Exchange Email Without LocationRef Old Version", service: path.ExchangeService.String(), category: path.EmailCategory.String(), itemInfo: ItemInfo{ @@ -1361,11 +1361,13 @@ func (suite *DetailsUnitSuite) TestLocationIDer_FromEntry() { ItemType: ExchangeMail, }, }, - backupVersion: version.OneDrive7LocationRef - 1, - expectedErr: require.Error, + backupVersion: version.OneDrive7LocationRef - 1, + hasLocRef: true, + expectedErr: require.NoError, + expectedUniqueLoc: fmt.Sprintf(expectedExchangeUniqueLocFmt, path.EmailCategory), }, { - name: "Exchange Email Without LocationRef New Version Errors", + name: "Exchange Email Without LocationRef New Version", service: path.ExchangeService.String(), category: path.EmailCategory.String(), itemInfo: ItemInfo{ @@ -1373,9 +1375,49 @@ func (suite *DetailsUnitSuite) TestLocationIDer_FromEntry() { ItemType: ExchangeMail, }, }, + backupVersion: version.OneDrive7LocationRef, + hasLocRef: true, + expectedErr: require.NoError, + expectedUniqueLoc: fmt.Sprintf(expectedExchangeUniqueLocFmt, path.EmailCategory), + }, + { + name: "Exchange Email Bad RepoRef Fails", + service: path.OneDriveService.String(), + category: path.EmailCategory.String(), + itemInfo: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeMail, + }, + }, backupVersion: version.OneDrive7LocationRef, expectedErr: require.Error, }, + { + name: "Exchange Event Empty LocationRef New Version Fails", + service: path.ExchangeService.String(), + category: path.EventsCategory.String(), + itemInfo: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeEvent, + }, + }, + backupVersion: 2, + expectedErr: require.Error, + }, + { + name: "Exchange Event Empty LocationRef Old Version", + service: path.ExchangeService.String(), + category: path.EventsCategory.String(), + itemInfo: ItemInfo{ + Exchange: &ExchangeInfo{ + ItemType: ExchangeEvent, + }, + }, + backupVersion: version.OneDrive1DataAndMetaFiles, + hasLocRef: true, + expectedErr: require.NoError, + expectedUniqueLoc: fmt.Sprintf(expectedExchangeUniqueLocFmt, path.EventsCategory), + }, } for _, test := range table { diff --git a/src/pkg/backup/details/entry.go b/src/pkg/backup/details/entry.go index 47e2d5196..cfadd8641 100644 --- a/src/pkg/backup/details/entry.go +++ b/src/pkg/backup/details/entry.go @@ -56,6 +56,9 @@ type Entry struct { // ToLocationIDer takes a backup version and produces the unique location for // this entry if possible. Reasons it may not be possible to produce the unique // location include an unsupported backup version or missing information. +// +// TODO(ashmrtn): Remove this function completely if we ever decide to sunset +// older corso versions that didn't populate LocationRef. func (de Entry) ToLocationIDer(backupVersion int) (LocationIDer, error) { if len(de.LocationRef) > 0 { baseLoc, err := path.Builder{}.SplitUnescapeAppend(de.LocationRef) @@ -68,32 +71,44 @@ func (de Entry) ToLocationIDer(backupVersion int) (LocationIDer, error) { return de.ItemInfo.uniqueLocation(baseLoc) } - if backupVersion >= version.OneDrive7LocationRef || - (de.ItemInfo.infoType() != OneDriveItem && - de.ItemInfo.infoType() != SharePointLibrary) { - return nil, clues.New("no previous location for entry") - } - - // This is a little hacky, but we only want to try to extract the old - // location if it's OneDrive or SharePoint libraries and it's known to - // be an older backup version. - // - // TODO(ashmrtn): Remove this code once OneDrive/SharePoint libraries - // LocationRef code has been out long enough that all delta tokens for - // previous backup versions will have expired. At that point, either - // we'll do a full backup (token expired, no newer backups) or have a - // backup of a higher version with the information we need. rr, err := path.FromDataLayerPath(de.RepoRef, true) if err != nil { - return nil, clues.Wrap(err, "getting item RepoRef") + return nil, clues.Wrap(err, "getting item RepoRef"). + With("repo_ref", de.RepoRef) } - p, err := path.ToDrivePath(rr) - if err != nil { - return nil, clues.New("converting RepoRef to drive path") + var baseLoc *path.Builder + + switch de.ItemInfo.infoType() { + case ExchangeEvent: + if backupVersion >= 2 { + return nil, clues.New("no previous location for calendar entry"). + With("repo_ref", rr) + } + + fallthrough + case ExchangeMail, ExchangeContact: + baseLoc = path.Builder{}.Append(rr.Folders()...) + + case OneDriveItem, SharePointLibrary: + if backupVersion >= version.OneDrive7LocationRef { + return nil, clues.New("no previous location for drive entry"). + With("repo_ref", rr) + } + + p, err := path.ToDrivePath(rr) + if err != nil { + return nil, clues.New("converting RepoRef to drive path"). + With("repo_ref", rr) + } + + baseLoc = path.Builder{}.Append(p.Root).Append(p.Folders...) } - baseLoc := path.Builder{}.Append(p.Root).Append(p.Folders...) + if baseLoc == nil { + return nil, clues.New("unable to extract LocationRef from RepoRef"). + With("repo_ref", rr) + } // Individual services may add additional info to the base and return that. return de.ItemInfo.uniqueLocation(baseLoc)