diff --git a/src/internal/kopia/wrapper.go b/src/internal/kopia/wrapper.go index 491f69c9d..3a761b313 100644 --- a/src/internal/kopia/wrapper.go +++ b/src/internal/kopia/wrapper.go @@ -26,10 +26,7 @@ const ( corsoUser = "corso" ) -var ( - errNotConnected = errors.New("not connected to repo") - errUnsupportedDir = errors.New("unsupported static children in streaming directory") -) +var errNotConnected = errors.New("not connected to repo") type BackupStats struct { SnapshotID string @@ -80,19 +77,33 @@ func (w *Wrapper) Close(ctx context.Context) error { // kopia callbacks on directory entries. It binds the directory to the given // DataCollection. func getStreamItemFunc( - collection data.Collection, + staticEnts []fs.Entry, + streamedEnts data.Collection, snapshotDetails *details.Details, ) func(context.Context, func(context.Context, fs.Entry) error) error { return func(ctx context.Context, cb func(context.Context, fs.Entry) error) error { - items := collection.Items() + // Return static entries in this directory first. + for _, d := range staticEnts { + if err := cb(ctx, d); err != nil { + return errors.Wrap(err, "executing callback on static directory") + } + } + + if streamedEnts == nil { + return nil + } + + items := streamedEnts.Items() for { select { case <-ctx.Done(): return ctx.Err() + case e, ok := <-items: if !ok { return nil } + ei, ok := e.(data.StreamInfo) if !ok { return errors.New("item does not implement DataStreamInfo") @@ -104,7 +115,7 @@ func getStreamItemFunc( } // Populate BackupDetails - ep := append(collection.FullPath(), e.UUID()) + ep := append(streamedEnts.FullPath(), e.UUID()) snapshotDetails.Add(path.Join(ep...), ei.Info()) } } @@ -112,22 +123,11 @@ func getStreamItemFunc( } // buildKopiaDirs recursively builds a directory hierarchy from the roots up. -// Returned directories are either virtualfs.StreamingDirectory or -// virtualfs.staticDirectory. +// Returned directories are virtualfs.StreamingDirectory. func buildKopiaDirs(dirName string, dir *treeMap, snapshotDetails *details.Details) (fs.Directory, error) { - // Don't support directories that have both a DataCollection and a set of - // static child directories. - if dir.collection != nil && len(dir.childDirs) > 0 { - return nil, errors.New(errUnsupportedDir.Error()) - } - - if dir.collection != nil { - return virtualfs.NewStreamingDirectory(dirName, getStreamItemFunc(dir.collection, snapshotDetails)), nil - } - // Need to build the directory tree from the leaves up because intermediate // directories need to have all their entries at creation time. - childDirs := []fs.Entry{} + var childDirs []fs.Entry for childName, childDir := range dir.childDirs { child, err := buildKopiaDirs(childName, childDir, snapshotDetails) @@ -138,7 +138,10 @@ func buildKopiaDirs(dirName string, dir *treeMap, snapshotDetails *details.Detai childDirs = append(childDirs, child) } - return virtualfs.NewStaticDirectory(dirName, childDirs), nil + return virtualfs.NewStreamingDirectory( + dirName, + getStreamItemFunc(childDirs, dir.collection, snapshotDetails), + ), nil } type treeMap struct { @@ -179,8 +182,8 @@ func inflateDirTree(ctx context.Context, collections []data.Collection, snapshot } for _, p := range itemPath[1 : len(itemPath)-1] { - newDir, ok := dir.childDirs[p] - if !ok { + newDir := dir.childDirs[p] + if newDir == nil { newDir = newTreeMap() if dir.childDirs == nil { @@ -200,13 +203,13 @@ func inflateDirTree(ctx context.Context, collections []data.Collection, snapshot end := len(itemPath) - 1 // Make sure this entry doesn't already exist. - if _, ok := dir.childDirs[itemPath[end]]; ok { - return nil, errors.New(errUnsupportedDir.Error()) + tmpDir := dir.childDirs[itemPath[end]] + if tmpDir == nil { + tmpDir = newTreeMap() + dir.childDirs[itemPath[end]] = tmpDir } - sd := newTreeMap() - sd.collection = s - dir.childDirs[itemPath[end]] = sd + tmpDir.collection = s } if len(roots) > 1 { diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index 9e111896b..0bb867a6c 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/src/internal/kopia/wrapper_test.go @@ -197,7 +197,90 @@ func (suite *KopiaUnitSuite) TestBuildDirectoryTree_NoAncestorDirs() { entries, err := fs.GetAllEntries(ctx, dirTree) require.NoError(suite.T(), err) - assert.Len(suite.T(), entries, 42) + assert.Len(suite.T(), entries, expectedFileCount) +} + +func (suite *KopiaUnitSuite) TestBuildDirectoryTree_MixedDirectory() { + ctx := context.Background() + // Test multiple orders of items because right now order can matter. Both + // orders result in a directory structure like: + // - a-tenant + // - user1 + // - emails + // - 5 separate files + // - 42 separate files + table := []struct { + name string + layout []data.Collection + }{ + { + name: "SubdirFirst", + layout: []data.Collection{ + mockconnector.NewMockExchangeCollection( + []string{testTenant, testUser, testEmailDir}, + 5, + ), + mockconnector.NewMockExchangeCollection( + []string{testTenant, testUser}, + 42, + ), + }, + }, + { + name: "SubdirLast", + layout: []data.Collection{ + mockconnector.NewMockExchangeCollection( + []string{testTenant, testUser}, + 42, + ), + mockconnector.NewMockExchangeCollection( + []string{testTenant, testUser, testEmailDir}, + 5, + ), + }, + }, + } + + for _, test := range table { + suite.T().Run(test.name, func(t *testing.T) { + snapshotDetails := &details.Details{} + + dirTree, err := inflateDirTree(ctx, test.layout, snapshotDetails) + require.NoError(t, err) + assert.Equal(t, testTenant, dirTree.Name()) + + entries, err := fs.GetAllEntries(ctx, dirTree) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, testUser, entries[0].Name()) + + d, ok := entries[0].(fs.Directory) + require.True(t, ok, "returned entry is not a directory") + + entries, err = fs.GetAllEntries(ctx, d) + require.NoError(t, err) + // 42 files and 1 subdirectory. + assert.Len(t, entries, 43) + + // One of these entries should be a subdirectory with items in it. + subDirs := []fs.Directory(nil) + for _, e := range entries { + d, ok := e.(fs.Directory) + if !ok { + continue + } + + subDirs = append(subDirs, d) + assert.Equal(t, testEmailDir, e.Name()) + } + + require.Len(t, subDirs, 1) + + entries, err = fs.GetAllEntries(ctx, subDirs[0]) + assert.NoError(t, err) + assert.Len(t, entries, 5) + }) + } } func (suite *KopiaUnitSuite) TestBuildDirectoryTree_Fails() { @@ -234,25 +317,6 @@ func (suite *KopiaUnitSuite) TestBuildDirectoryTree_Fails() { ), }, }, - { - "MixedDirectory", - // Directory structure would look like (but should return error): - // - a-tenant - // - user1 - // - emails - // - 5 separate files - // - 42 separate files - []data.Collection{ - mockconnector.NewMockExchangeCollection( - []string{"a-tenant", "user1", "emails"}, - 5, - ), - mockconnector.NewMockExchangeCollection( - []string{"a-tenant", "user1"}, - 42, - ), - }, - }, } for _, test := range table {