diff --git a/src/cli/backup/exchange_integration_test.go b/src/cli/backup/exchange_integration_test.go index 6a92e8d4f..31e56f53e 100644 --- a/src/cli/backup/exchange_integration_test.go +++ b/src/cli/backup/exchange_integration_test.go @@ -270,11 +270,25 @@ func (suite *PreparedBackupExchangeIntegrationSuite) TestExchangeDetailsCmd() { // compare the output result := recorder.String() - for i, ent := range deets.Entries { + i := 0 + foundFolders := 0 + + for _, ent := range deets.Entries { + // Skip folders as they don't mean anything to the end user. + if ent.Folder != nil { + foundFolders++ + continue + } + t.Run(fmt.Sprintf("detail %d", i), func(t *testing.T) { assert.Contains(t, result, ent.ShortRef) }) + + i++ } + + // At least the prefix of the path should be encoded as folders. + assert.Greater(suite.T(), foundFolders, 4) }) } } diff --git a/src/internal/kopia/wrapper.go b/src/internal/kopia/wrapper.go index 4c5b8d33d..0b39f1960 100644 --- a/src/internal/kopia/wrapper.go +++ b/src/internal/kopia/wrapper.go @@ -89,7 +89,35 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) { return } - cp.deets.Add(d.repoPath.String(), d.repoPath.ShortRef(), d.info) + parent := d.repoPath.ToBuilder().Dir() + + cp.deets.Add( + d.repoPath.String(), + d.repoPath.ShortRef(), + parent.ShortRef(), + d.info, + ) + + folders := []details.FolderEntry{} + + for len(parent.Elements()) > 0 { + nextParent := parent.Dir() + + folders = append(folders, details.FolderEntry{ + RepoRef: parent.String(), + ShortRef: parent.ShortRef(), + ParentRef: nextParent.ShortRef(), + Info: details.ItemInfo{ + Folder: &details.FolderInfo{ + DisplayName: parent.Elements()[len(parent.Elements())-1], + }, + }, + }) + + parent = nextParent + } + + cp.deets.AddFolders(folders) } func (cp *corsoProgress) put(k string, v *itemDetails) { diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index 95ee0b2aa..ef85283f1 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/src/internal/kopia/wrapper_test.go @@ -132,20 +132,17 @@ func getDirEntriesForEntry( // --------------- type CorsoProgressUnitSuite struct { suite.Suite + targetFilePath path.Path + targetFileName string } func TestCorsoProgressUnitSuite(t *testing.T) { suite.Run(t, new(CorsoProgressUnitSuite)) } -func (suite *CorsoProgressUnitSuite) TestFinishedFile() { - type testInfo struct { - info *itemDetails - err error - } - - targetFilePath, err := path.Builder{}.Append( - "Inbox", +func (suite *CorsoProgressUnitSuite) SetupSuite() { + p, err := path.Builder{}.Append( + testInboxDir, "testFile", ).ToDataLayerExchangePathForCategory( testTenant, @@ -155,11 +152,17 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFile() { ) require.NoError(suite.T(), err) - relativePath, err := targetFilePath.Dir() - require.NoError(suite.T(), err) + suite.targetFilePath = p + suite.targetFileName = suite.targetFilePath.ToBuilder().Dir().String() +} - targetFileName := relativePath.String() - deets := &itemDetails{details.ItemInfo{}, targetFilePath} +func (suite *CorsoProgressUnitSuite) TestFinishedFile() { + type testInfo struct { + info *itemDetails + err error + } + + deets := &itemDetails{details.ItemInfo{}, suite.targetFilePath} table := []struct { name string @@ -170,17 +173,18 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFile() { { name: "DetailsExist", cachedItems: map[string]testInfo{ - targetFileName: { + suite.targetFileName: { info: deets, err: nil, }, }, - expectedLen: 1, + // 1 file and 5 folders. + expectedLen: 6, }, { name: "PendingNoDetails", cachedItems: map[string]testInfo{ - targetFileName: { + suite.targetFileName: { info: nil, err: nil, }, @@ -190,7 +194,7 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFile() { { name: "HadError", cachedItems: map[string]testInfo{ - targetFileName: { + suite.targetFileName: { info: deets, err: assert.AnError, }, @@ -227,6 +231,65 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFile() { } } +func (suite *CorsoProgressUnitSuite) TestFinishedFileBuildsHierarchy() { + t := suite.T() + // Order of folders in hierarchy from root to leaf (excluding the item). + expectedFolderOrder := suite.targetFilePath.ToBuilder().Dir().Elements() + + // Setup stuff. + bd := &details.Details{} + cp := corsoProgress{ + UploadProgress: &snapshotfs.NullUploadProgress{}, + deets: bd, + pending: map[string]*itemDetails{}, + } + + deets := &itemDetails{details.ItemInfo{}, suite.targetFilePath} + cp.put(suite.targetFileName, deets) + require.Len(t, cp.pending, 1) + + cp.FinishedFile(suite.targetFileName, nil) + + // Gather information about the current state. + var ( + curRef *details.DetailsEntry + refToEntry = map[string]*details.DetailsEntry{} + ) + + for i := 0; i < len(bd.Entries); i++ { + e := &bd.Entries[i] + if e.Folder == nil { + continue + } + + refToEntry[e.ShortRef] = e + + if e.Folder.DisplayName == expectedFolderOrder[len(expectedFolderOrder)-1] { + curRef = e + } + } + + // Actual tests start here. + var rootRef *details.DetailsEntry + + // Traverse the details entries from leaf to root, following the ParentRef + // fields. At the end rootRef should point to the root of the path. + for i := len(expectedFolderOrder) - 1; i >= 0; i-- { + name := expectedFolderOrder[i] + + require.NotNil(t, curRef) + assert.Equal(t, name, curRef.Folder.DisplayName) + + rootRef = curRef + curRef = refToEntry[curRef.ParentRef] + } + + // Hierarchy root's ParentRef = "" and map will return nil. + assert.Nil(t, curRef) + require.NotNil(t, rootRef) + assert.Empty(t, rootRef.ParentRef) +} + type KopiaUnitSuite struct { suite.Suite testPath path.Path @@ -595,7 +658,8 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() { assert.Equal(t, stats.IgnoredErrorCount, 0) assert.Equal(t, stats.ErrorCount, 0) assert.False(t, stats.Incomplete) - assert.Len(t, rp.Entries, 47) + // 47 file and 6 folder entries. + assert.Len(t, rp.Entries, 47+6) } func (suite *KopiaIntegrationSuite) TestRestoreAfterCompressionChange() { @@ -690,7 +754,8 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { assert.Equal(t, 6, stats.TotalDirectoryCount) assert.Equal(t, 1, stats.IgnoredErrorCount) assert.False(t, stats.Incomplete) - assert.Len(t, rp.Entries, 5) + // 5 file and 6 folder entries. + assert.Len(t, rp.Entries, 5+6) } type KopiaSimpleRepoIntegrationSuite struct { @@ -794,7 +859,8 @@ func (suite *KopiaSimpleRepoIntegrationSuite) SetupTest() { require.Equal(t, stats.TotalDirectoryCount, 6) require.Equal(t, stats.IgnoredErrorCount, 0) require.False(t, stats.Incomplete) - assert.Len(t, rp.Entries, 6) + // 6 file and 6 folder entries. + assert.Len(t, rp.Entries, 6+6) suite.snapshotID = manifest.ID(stats.SnapshotID) diff --git a/src/internal/path/path.go b/src/internal/path/path.go index efe675fdd..2c0b0d3e2 100644 --- a/src/internal/path/path.go +++ b/src/internal/path/path.go @@ -91,6 +91,8 @@ type Path interface { // reference is guaranteed to be unique. No guarantees are made about whether // a short reference can be converted back into the Path that generated it. ShortRef() string + // ToBuilder returns a Builder instance that represents the current Path. + ToBuilder() *Builder } // Builder is a simple path representation that only tracks path elements. It @@ -174,7 +176,7 @@ func (pb Builder) PopFront() *Builder { } } -func (pb Builder) dir() *Builder { +func (pb Builder) Dir() *Builder { if len(pb.elements) <= 1 { return &Builder{} } diff --git a/src/internal/path/resource_path.go b/src/internal/path/resource_path.go index 9c1ec856e..e9194a638 100644 --- a/src/internal/path/resource_path.go +++ b/src/internal/path/resource_path.go @@ -171,7 +171,7 @@ func (rp dataLayerResourcePath) Dir() (Path, error) { } return &dataLayerResourcePath{ - Builder: *rp.dir(), + Builder: *rp.Builder.Dir(), service: rp.service, category: rp.category, hasItem: false, @@ -193,3 +193,8 @@ func (rp dataLayerResourcePath) Append( hasItem: isItem, }, nil } + +func (rp dataLayerResourcePath) ToBuilder() *Builder { + // Safe to directly return the Builder because Builders are immutable. + return &rp.Builder +} diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index 133ddea7b..a506b20bf 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -12,9 +12,10 @@ import ( ) type FolderEntry struct { - RepoRef string - ShortRef string - Info ItemInfo + RepoRef string + ShortRef string + ParentRef string + Info ItemInfo } // -------------------------------------------------------------------------------- @@ -66,18 +67,39 @@ func printJSON(ctx context.Context, dm DetailsModel) { print.All(ctx, ents...) } -// Paths returns the list of Paths extracted from the Entries slice. +// Paths returns the list of Paths for non-folder items extracted from the +// Entries slice. func (dm DetailsModel) Paths() []string { - ents := dm.Entries - r := make([]string, len(ents)) + r := make([]string, 0, len(dm.Entries)) - for i := range ents { - r[i] = ents[i].RepoRef + for _, ent := range dm.Entries { + if ent.Folder != nil { + continue + } + + r = append(r, ent.RepoRef) } return r } +// Items returns a slice of *ItemInfo that does not contain any FolderInfo +// entries. Required because not all folders in the details are valid resource +// paths. +func (dm DetailsModel) Items() []*DetailsEntry { + res := make([]*DetailsEntry, 0, len(dm.Entries)) + + for i := 0; i < len(dm.Entries); i++ { + if dm.Entries[i].Folder != nil { + continue + } + + res = append(res, &dm.Entries[i]) + } + + return res +} + // -------------------------------------------------------------------------------- // Details // -------------------------------------------------------------------------------- @@ -93,13 +115,14 @@ type Details struct { knownFolders map[string]struct{} `json:"-"` } -func (d *Details) Add(repoRef, shortRef string, info ItemInfo) { +func (d *Details) Add(repoRef, shortRef, parentRef string, info ItemInfo) { d.mu.Lock() defer d.mu.Unlock() d.Entries = append(d.Entries, DetailsEntry{ - RepoRef: repoRef, - ShortRef: shortRef, - ItemInfo: info, + RepoRef: repoRef, + ShortRef: shortRef, + ParentRef: parentRef, + ItemInfo: info, }) } @@ -121,9 +144,10 @@ func (d *Details) AddFolders(folders []FolderEntry) { d.knownFolders[folder.ShortRef] = struct{}{} d.Entries = append(d.Entries, DetailsEntry{ - RepoRef: folder.RepoRef, - ShortRef: folder.ShortRef, - ItemInfo: folder.Info, + RepoRef: folder.RepoRef, + ShortRef: folder.ShortRef, + ParentRef: folder.ParentRef, + ItemInfo: folder.Info, }) } } @@ -136,8 +160,9 @@ func (d *Details) AddFolders(folders []FolderEntry) { type DetailsEntry struct { // TODO: `RepoRef` is currently the full path to the item in Kopia // This can be optimized. - RepoRef string `json:"repoRef"` - ShortRef string `json:"shortRef"` + RepoRef string `json:"repoRef"` + ShortRef string `json:"shortRef"` + ParentRef string `json:"parentRef,omitempty"` ItemInfo } diff --git a/src/pkg/backup/details/details_test.go b/src/pkg/backup/details/details_test.go index f78e7106e..6cd047908 100644 --- a/src/pkg/backup/details/details_test.go +++ b/src/pkg/backup/details/details_test.go @@ -132,41 +132,77 @@ func (suite *DetailsUnitSuite) TestDetailsEntry_HeadersValues() { } } +var pathItemsTable = []struct { + name string + ents []details.DetailsEntry + expectRefs []string +}{ + { + name: "nil entries", + ents: nil, + expectRefs: []string{}, + }, + { + name: "single entry", + ents: []details.DetailsEntry{ + {RepoRef: "abcde"}, + }, + expectRefs: []string{"abcde"}, + }, + { + name: "multiple entries", + ents: []details.DetailsEntry{ + {RepoRef: "abcde"}, + {RepoRef: "12345"}, + }, + expectRefs: []string{"abcde", "12345"}, + }, + { + name: "multiple entries with folder", + ents: []details.DetailsEntry{ + {RepoRef: "abcde"}, + {RepoRef: "12345"}, + { + RepoRef: "deadbeef", + ItemInfo: details.ItemInfo{ + Folder: &details.FolderInfo{ + DisplayName: "test folder", + }, + }, + }, + }, + expectRefs: []string{"abcde", "12345"}, + }, +} + func (suite *DetailsUnitSuite) TestDetailsModel_Path() { - table := []struct { - name string - ents []details.DetailsEntry - expect []string - }{ - { - name: "nil entries", - ents: nil, - expect: []string{}, - }, - { - name: "single entry", - ents: []details.DetailsEntry{ - {RepoRef: "abcde"}, - }, - expect: []string{"abcde"}, - }, - { - name: "multiple entries", - ents: []details.DetailsEntry{ - {RepoRef: "abcde"}, - {RepoRef: "12345"}, - }, - expect: []string{"abcde", "12345"}, - }, - } - for _, test := range table { + for _, test := range pathItemsTable { suite.T().Run(test.name, func(t *testing.T) { d := details.Details{ DetailsModel: details.DetailsModel{ Entries: test.ents, }, } - assert.Equal(t, test.expect, d.Paths()) + assert.Equal(t, test.expectRefs, d.Paths()) + }) + } +} + +func (suite *DetailsUnitSuite) TestDetailsModel_Items() { + for _, test := range pathItemsTable { + suite.T().Run(test.name, func(t *testing.T) { + d := details.Details{ + DetailsModel: details.DetailsModel{ + Entries: test.ents, + }, + } + + ents := d.Items() + assert.Len(t, ents, len(test.expectRefs)) + + for _, e := range ents { + assert.Contains(t, test.expectRefs, e.RepoRef) + } }) } } @@ -181,12 +217,14 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { name: "MultipleFolders", folders: []details.FolderEntry{ { - RepoRef: "rr1", - ShortRef: "sr1", + RepoRef: "rr1", + ShortRef: "sr1", + ParentRef: "pr1", }, { - RepoRef: "rr2", - ShortRef: "sr2", + RepoRef: "rr2", + ShortRef: "sr2", + ParentRef: "pr2", }, }, expectedShortRefs: []string{"sr1", "sr2"}, @@ -195,20 +233,24 @@ func (suite *DetailsUnitSuite) TestDetails_AddFolders() { name: "MultipleFoldersWithRepeats", folders: []details.FolderEntry{ { - RepoRef: "rr1", - ShortRef: "sr1", + RepoRef: "rr1", + ShortRef: "sr1", + ParentRef: "pr1", }, { - RepoRef: "rr2", - ShortRef: "sr2", + RepoRef: "rr2", + ShortRef: "sr2", + ParentRef: "pr2", }, { - RepoRef: "rr1", - ShortRef: "sr1", + RepoRef: "rr1", + ShortRef: "sr1", + ParentRef: "pr1", }, { - RepoRef: "rr3", - ShortRef: "sr3", + RepoRef: "rr3", + ShortRef: "sr3", + ParentRef: "pr3", }, }, expectedShortRefs: []string{"sr1", "sr2", "sr3"}, diff --git a/src/pkg/selectors/scopes.go b/src/pkg/selectors/scopes.go index ed16628f2..5e1ef1f62 100644 --- a/src/pkg/selectors/scopes.go +++ b/src/pkg/selectors/scopes.go @@ -227,7 +227,7 @@ func reduce[T scopeT, C categoryT]( ents := []details.DetailsEntry{} // for each entry, compare that entry against the scopes of the same data type - for _, ent := range deets.Entries { + for _, ent := range deets.Items() { repoPath, err := path.FromDataLayerPath(ent.RepoRef, true) if err != nil { logger.Ctx(ctx).Debugw("transforming repoRef to path", "err", err) @@ -242,13 +242,13 @@ func reduce[T scopeT, C categoryT]( passed := passes( dc, dc.pathValues(repoPath), - ent, + *ent, excls[dc], filts[dc], incls[dc], ) if passed { - ents = append(ents, ent) + ents = append(ents, *ent) } }