larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix up failures found when manually testing (#4825)

Browse Source 
correct the tree post-process traversal, ensuring the root folder doesn't appear twice in the path.  And provide the drive name to the collection so that the backup details can utilize it.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🐛 Bugfix

#### Issue(s)

* #4689

#### Test Plan

- [x] 💪 Manual
- [x] ⚡ Unit test
This commit is contained in:
Keepers 2023-12-14 16:28:16 -07:00 committed by GitHub
parent 505c06441a
commit e3363aaa46
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 42 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/internal/m365/collection/drive/collection.go
View File

@ -111,7 +111,7 @@ func NewCollection(
resource idname.Provider,
currPath path.Path,
prevPath path.Path,
driveID string,
driveID, driveName string,
statusUpdater support.StatusUpdater,
ctrlOpts control.Options,
isPackageOrChildOfPackage bool,
@ -139,6 +139,7 @@ func NewCollection(
currPath,
prevPath,
driveID,
driveName,
statusUpdater,
ctrlOpts,
isPackageOrChildOfPackage,
@ -157,7 +158,7 @@ func newColl(
resource idname.Provider,
currPath path.Path,
prevPath path.Path,
driveID string,
driveID, driveName string,
statusUpdater support.StatusUpdater,
ctrlOpts control.Options,
isPackageOrChildOfPackage bool,
@ -174,6 +175,7 @@ func newColl(
prevPath: prevPath,
driveItems: map[string]*custom.DriveItem{},
driveID: driveID,
driveName: driveName,
data: dataCh,
statusUpdater: statusUpdater,
ctrl: ctrlOpts,

22
src/internal/m365/collection/drive/collection_test.go
View File

@ -211,7 +211,8 @@ func (suite *CollectionUnitSuite) TestCollection() {
mbh.ProtectedResource,
folderPath,
nil,
"drive-id",
id(drivePfx),
name(drivePfx),
suite.testStatusUpdater(&wg, &collStatus),
control.Options{ToggleFeatures: control.Toggles{}},
false,
@ -303,7 +304,6 @@ func (suite *CollectionUnitSuite) TestCollectionReadError() {
stubItemID = "fakeItemID"
collStatus = support.ControllerOperationStatus{}
wg = sync.WaitGroup{}
name = "name"
size = defaultFileSize
now = time.Now()
)
@ -334,7 +334,8 @@ func (suite *CollectionUnitSuite) TestCollectionReadError() {
mbh.ProtectedResource,
folderPath,
nil,
"fakeDriveID",
id(drivePfx),
name(drivePfx),
suite.testStatusUpdater(&wg, &collStatus),
control.Options{ToggleFeatures: control.Toggles{}},
false,
@ -345,7 +346,7 @@ func (suite *CollectionUnitSuite) TestCollectionReadError() {
stubItem := odTD.NewStubDriveItem(
stubItemID,
name,
name(drivePfx),
size,
now,
now,
@ -373,7 +374,6 @@ func (suite *CollectionUnitSuite) TestCollectionReadUnauthorizedErrorRetry() {
stubItemID = "fakeItemID"
collStatus = support.ControllerOperationStatus{}
wg = sync.WaitGroup{}
name = "name"
size = defaultFileSize
now = time.Now()
)
@ -385,7 +385,7 @@ func (suite *CollectionUnitSuite) TestCollectionReadUnauthorizedErrorRetry() {
stubItem := odTD.NewStubDriveItem(
stubItemID,
name,
name(drivePfx),
size,
now,
now,
@ -413,7 +413,8 @@ func (suite *CollectionUnitSuite) TestCollectionReadUnauthorizedErrorRetry() {
mbh.ProtectedResource,
folderPath,
nil,
"fakeDriveID",
id(drivePfx),
name(drivePfx),
suite.testStatusUpdater(&wg, &collStatus),
control.Options{ToggleFeatures: control.Toggles{}},
false,
@ -470,7 +471,8 @@ func (suite *CollectionUnitSuite) TestCollectionPermissionBackupLatestModTime()
mbh.ProtectedResource,
folderPath,
nil,
"drive-id",
id(drivePfx),
name(drivePfx),
suite.testStatusUpdater(&wg, &collStatus),
control.Options{ToggleFeatures: control.Toggles{}},
false,
@ -837,7 +839,6 @@ func (suite *CollectionUnitSuite) TestItemExtensions() {
t = suite.T()
stubItemID = "itemID"
stubItemName = "name"
driveID = "driveID"
collStatus = support.ControllerOperationStatus{}
wg = sync.WaitGroup{}
now = time.Now()
@ -1002,7 +1003,8 @@ func (suite *CollectionUnitSuite) TestItemExtensions() {
mbh.ProtectedResource,
folderPath,
nil,
driveID,
id(drivePfx),
name(drivePfx),
suite.testStatusUpdater(&wg, &collStatus),
opts,
false,

6
src/internal/m365/collection/drive/collections.go
View File

@ -489,6 +489,7 @@ func (c *Collections) Get(
nil, // delete the folder
prevPath,
driveID,
driveName,
c.statusUpdater,
c.ctrl,
false,
@ -527,6 +528,7 @@ func (c *Collections) Get(
nil, // delete the drive
prevDrivePath,
driveID,
"",
c.statusUpdater,
c.ctrl,
false,
@ -698,6 +700,7 @@ func (c *Collections) handleDelete(
nil, // deletes the collection
prevPath,
driveID,
"",
c.statusUpdater,
c.ctrl,
false,
@ -706,7 +709,7 @@ func (c *Collections) handleDelete(
nil,
counter.Local())
if err != nil {
return clues.Wrap(err, "making collection").With(
return clues.WrapWC(ctx, err, "making collection").With(
"drive_id", driveID,
"item_id", itemID,
"path_string", prevPathStr)
@ -1089,6 +1092,7 @@ func (c *Collections) processItem(
collectionPath,
prevPath,
driveID,
driveName,
c.statusUpdater,
c.ctrl,
isPackage || childOfPackage,

7
src/internal/m365/collection/drive/collections_tree.go
View File

@ -220,7 +220,7 @@ func (c *Collections) makeDriveCollections(
collections, newPrevs, excludedItemIDs, err := c.turnTreeIntoCollections(
ctx,
tree,
driveID,
drv,
prevDeltaLink,
countPagesInDelta,
errs)
@ -775,7 +775,7 @@ func addPrevPathsToTree(
func (c *Collections) turnTreeIntoCollections(
ctx context.Context,
tree *folderyMcFolderFace,
driveID string,
drv models.Driveable,
prevDeltaLink string,
countPagesInDelta int,
errs *fault.Bus,
@ -796,6 +796,8 @@ func (c *Collections) turnTreeIntoCollections(
newPrevPaths = map[string]string{}
uc *urlCache
el = errs.Local()
driveID = ptr.Val(drv.GetId())
driveName = ptr.Val(drv.GetName())
)
// Attach an url cache to the drive if the number of discovered items is
@ -838,6 +840,7 @@ func (c *Collections) turnTreeIntoCollections(
cbl.currPath,
cbl.prevPath,
driveID,
driveName,
c.statusUpdater,
c.ctrl,
cbl.isPackageOrChildOfPackage,

2
src/internal/m365/collection/drive/collections_tree_test.go
View File

@ -655,7 +655,7 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_TurnTreeIntoCollections()
colls, newPrevPaths, excluded, err := c.turnTreeIntoCollections(
ctx,
tree,
d.id,
d.able,
deltaURL,
countPages,
fault.New(true))

29
src/internal/m365/collection/drive/delta_tree.go
View File

@ -381,16 +381,15 @@ type collectable struct {
files map[string]*custom.DriveItem
folderID string
isPackageOrChildOfPackage bool
loc path.Elements
prevPath path.Path
}
// produces a map of folderID -> collectable
func (face *folderyMcFolderFace) generateCollectables() (map[string]collectable, error) {
result := map[string]collectable{}
err := walkTreeAndBuildCollections(
err := face.walkTreeAndBuildCollections(
face.root,
face.prefix,
&path.Builder{},
false,
result)
@ -410,10 +409,9 @@ func (face *folderyMcFolderFace) generateCollectables() (map[string]collectable,
return result, clues.Stack(err).OrNil()
}
func walkTreeAndBuildCollections(
func (face *folderyMcFolderFace) walkTreeAndBuildCollections(
node *nodeyMcNodeFace,
pathPfx path.Path,
parentPath *path.Builder,
location *path.Builder,
isChildOfPackage bool,
result map[string]collectable,
) error {
@ -421,14 +419,16 @@ func walkTreeAndBuildCollections(
return nil
}
parentLocation := parentPath.Elements()
currentLocation := parentPath.Append(node.name)
isRoot := node == face.root
if !isRoot {
location = location.Append(node.name)
}
for _, child := range node.children {
err := walkTreeAndBuildCollections(
err := face.walkTreeAndBuildCollections(
child,
pathPfx,
currentLocation,
location,
node.isPackage || isChildOfPackage,
result)
if err != nil {
@ -436,12 +436,12 @@ func walkTreeAndBuildCollections(
}
}
collectionPath, err := pathPfx.Append(false, currentLocation.Elements()...)
collectionPath, err := face.prefix.Append(false, location.Elements()...)
if err != nil {
return clues.Wrap(err, "building collection path").
With(
"path_prefix", pathPfx,
"path_suffix", currentLocation.Elements())
"path_prefix", face.prefix,
"path_suffix", location.Elements())
}
cbl := collectable{
@ -449,7 +449,6 @@ func walkTreeAndBuildCollections(
files: node.files,
folderID: node.id,
isPackageOrChildOfPackage: node.isPackage || isChildOfPackage,
loc: parentLocation,
prevPath: node.prev,
}

18
src/internal/m365/collection/drive/delta_tree_test.go
View File

@ -1021,7 +1021,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
files: map[string]*custom.DriveItem{},
folderID: rootID,
isPackageOrChildOfPackage: false,
loc: path.Elements{},
},
},
},
@ -1037,7 +1036,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
},
folderID: rootID,
isPackageOrChildOfPackage: false,
loc: path.Elements{},
},
},
},
@ -1051,14 +1049,12 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
files: map[string]*custom.DriveItem{},
folderID: rootID,
isPackageOrChildOfPackage: false,
loc: path.Elements{},
},
folderID("parent"): {
currPath: d.fullPath(t, folderName("parent")),
files: map[string]*custom.DriveItem{},
folderID: folderID("parent"),
isPackageOrChildOfPackage: false,
loc: path.Elements{rootName},
},
folderID(): {
currPath: d.fullPath(t, folderName("parent"), folderName()),
@ -1067,7 +1063,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
},
folderID: folderID(),
isPackageOrChildOfPackage: false,
loc: path.Elements{rootName, folderName("parent")},
},
},
},
@ -1093,21 +1088,18 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
files: map[string]*custom.DriveItem{},
folderID: rootID,
isPackageOrChildOfPackage: false,
loc: path.Elements{},
},
id(pkg): {
currPath: d.fullPath(t, name(pkg)),
files: map[string]*custom.DriveItem{},
folderID: id(pkg),
isPackageOrChildOfPackage: true,
loc: path.Elements{rootName},
},
folderID(): {
currPath: d.fullPath(t, name(pkg), folderName()),
files: map[string]*custom.DriveItem{},
folderID: folderID(),
isPackageOrChildOfPackage: true,
loc: path.Elements{rootName, name(pkg)},
},
},
},
@ -1126,7 +1118,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
files: map[string]*custom.DriveItem{},
folderID: rootID,
isPackageOrChildOfPackage: false,
loc: path.Elements{},
prevPath: d.fullPath(t),
},
folderID("parent"): {
@ -1134,7 +1125,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
files: map[string]*custom.DriveItem{},
folderID: folderID("parent"),
isPackageOrChildOfPackage: false,
loc: path.Elements{rootName},
prevPath: d.fullPath(t, folderName("parent-prev")),
},
folderID(): {
@ -1144,7 +1134,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
files: map[string]*custom.DriveItem{
fileID(): custom.ToCustomDriveItem(d.fileAt("parent")),
},
loc: path.Elements{rootName, folderName("parent")},
prevPath: d.fullPath(t, folderName("parent-prev"), folderName()),
},
},
@ -1163,7 +1152,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
files: map[string]*custom.DriveItem{},
folderID: rootID,
isPackageOrChildOfPackage: false,
loc: path.Elements{},
prevPath: d.fullPath(t),
},
folderID(): {
@ -1212,12 +1200,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_GenerateCollectables()
assert.Equal(t, expect.prevPath.String(), result.prevPath.String())
}
if expect.loc == nil {
assert.Nil(t, result.loc)
} else {
assert.Equal(t, expect.loc.PlainString(), result.loc.PlainString())
}
assert.ElementsMatch(t, maps.Keys(expect.files), maps.Keys(result.files))
}
})

15
src/internal/m365/collection/drive/helper_test.go
View File

@ -440,20 +440,7 @@ func (ecs expectedCollections) requireNoUnseenCollections(t *testing.T) {
// ---------------------------------------------------------------------------
func defaultTreePfx(t *testing.T, d *deltaDrive) path.Path {
fpb := d.fullPath(t).ToBuilder()
fpe := fpb.Elements()
fpe = fpe[:len(fpe)-1]
fpb = path.Builder{}.Append(fpe...)
p, err := path.FromDataLayerPath(fpb.String(), false)
require.NoErrorf(
t,
err,
"err processing path:\n\terr %+v\n\tpath %q",
clues.ToCore(err),
fpb)
return p
return d.fullPath(t)
}
func defaultLoc() path.Elements {