From 9e66f197c0ca56d5b6ebd96b633dfcf2f58c8f74 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Thu, 15 Sep 2022 18:42:01 -0700 Subject: [PATCH] Create and populate folder backup details entries during backup (#872) ## Description Other components may need to rebuild the directory hierarchy of items. As the paths Corso deals with can be hard to properly parse at times, store that information in the Corso backup details. The hierarchy can be rebuilt by following the `ParentRef` fields of items. The item at the root of the hierarchy has an empty `ParentRef` field. Also hide these folders from end-users. They are not displayed during backup list nor are they eligible as a target for restore ## Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :hamster: Trivial/Minor ## Issue(s) * closes #862 * closes #861 * closes #818 merge after: * #869 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- src/cli/backup/exchange_integration_test.go | 16 ++- src/internal/kopia/wrapper.go | 30 ++++- src/internal/kopia/wrapper_test.go | 104 ++++++++++++++--- src/internal/path/path.go | 4 +- src/internal/path/resource_path.go | 7 +- src/pkg/backup/details/details.go | 59 +++++++--- src/pkg/backup/details/details_test.go | 122 +++++++++++++------- src/pkg/selectors/scopes.go | 6 +- 8 files changed, 265 insertions(+), 83 deletions(-) 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) } }