minor updates to the path.Path interface (#3384)

Swaps path.Append from a single item method to accept a variadic list of string elements.  Since 95% of all calls to path.Append were items, also adds a shorthand
AppendItem func to the interface for easy clarity. Finally, adds a Last() method to elements for getting the last element in the slice.

---

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

- [x]  No

#### Type of change

- [x] 🧹 Tech Debt/Cleanup

#### Test Plan

- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-05-11 19:29:17 -06:00 committed by GitHub
parent 674d3eec91
commit 1b417af5bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 64 additions and 29 deletions

View File

@ -435,7 +435,7 @@ func restoreCollection(
metrics.Bytes += int64(len(byteArray)) metrics.Bytes += int64(len(byteArray))
metrics.Successes++ metrics.Successes++
itemPath, err := dc.FullPath().Append(itemData.UUID(), true) itemPath, err := dc.FullPath().AppendItem(itemData.UUID())
if err != nil { if err != nil {
errs.AddRecoverable(clues.Wrap(err, "building full path with item").WithClues(ctx)) errs.AddRecoverable(clues.Wrap(err, "building full path with item").WithClues(ctx))
continue continue

View File

@ -677,7 +677,7 @@ func (c *Collections) getCollectionPath(
return nil, clues.New("folder with empty name") return nil, clues.New("folder with empty name")
} }
collectionPath, err = collectionPath.Append(name, false) collectionPath, err = collectionPath.Append(false, name)
if err != nil { if err != nil {
return nil, clues.Wrap(err, "making non-root folder path") return nil, clues.Wrap(err, "making non-root folder path")
} }

View File

@ -231,7 +231,7 @@ func RestoreCollection(
return metrics, nil return metrics, nil
} }
itemPath, err := dc.FullPath().Append(itemData.UUID(), true) itemPath, err := dc.FullPath().AppendItem(itemData.UUID())
if err != nil { if err != nil {
el.AddRecoverable(clues.Wrap(err, "appending item to full path").WithClues(ctx)) el.AddRecoverable(clues.Wrap(err, "appending item to full path").WithClues(ctx))
continue continue
@ -852,7 +852,7 @@ func AugmentRestorePaths(
el := p.StoragePath.Elements() el := p.StoragePath.Elements()
if backupVersion >= version.OneDrive6NameInMeta { if backupVersion >= version.OneDrive6NameInMeta {
mPath, err := p.StoragePath.Append(".dirmeta", true) mPath, err := p.StoragePath.AppendItem(".dirmeta")
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -861,7 +861,7 @@ func AugmentRestorePaths(
paths, paths,
path.RestorePaths{StoragePath: mPath, RestorePath: p.RestorePath}) path.RestorePaths{StoragePath: mPath, RestorePath: p.RestorePath})
} else if backupVersion >= version.OneDrive4DirIncludesPermissions { } else if backupVersion >= version.OneDrive4DirIncludesPermissions {
mPath, err := p.StoragePath.Append(el[len(el)-1]+".dirmeta", true) mPath, err := p.StoragePath.AppendItem(el.Last() + ".dirmeta")
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -875,7 +875,7 @@ func AugmentRestorePaths(
return nil, err return nil, err
} }
mPath, err := pp.Append(el[len(el)-1]+".dirmeta", true) mPath, err := pp.AppendItem(el.Last() + ".dirmeta")
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -251,7 +251,7 @@ func RestoreListCollection(
metrics.Bytes += itemInfo.SharePoint.Size metrics.Bytes += itemInfo.SharePoint.Size
itemPath, err := dc.FullPath().Append(itemData.UUID(), true) itemPath, err := dc.FullPath().AppendItem(itemData.UUID())
if err != nil { if err != nil {
el.AddRecoverable(clues.Wrap(err, "appending item to full path").WithClues(ctx)) el.AddRecoverable(clues.Wrap(err, "appending item to full path").WithClues(ctx))
continue continue
@ -339,7 +339,7 @@ func RestorePageCollection(
metrics.Bytes += itemInfo.SharePoint.Size metrics.Bytes += itemInfo.SharePoint.Size
itemPath, err := dc.FullPath().Append(itemData.UUID(), true) itemPath, err := dc.FullPath().AppendItem(itemData.UUID())
if err != nil { if err != nil {
el.AddRecoverable(clues.Wrap(err, "appending item to full path").WithClues(ctx)) el.AddRecoverable(clues.Wrap(err, "appending item to full path").WithClues(ctx))
continue continue

View File

@ -347,7 +347,7 @@ func collectionEntries(
seen[encodedName] = struct{}{} seen[encodedName] = struct{}{}
// For now assuming that item IDs don't need escaping. // For now assuming that item IDs don't need escaping.
itemPath, err := streamedEnts.FullPath().Append(e.UUID(), true) itemPath, err := streamedEnts.FullPath().AppendItem(e.UUID())
if err != nil { if err != nil {
err = clues.Wrap(err, "getting full item path") err = clues.Wrap(err, "getting full item path")
progress.errs.AddRecoverable(err) progress.errs.AddRecoverable(err)
@ -464,7 +464,7 @@ func streamBaseEntries(
} }
// For now assuming that item IDs don't need escaping. // For now assuming that item IDs don't need escaping.
itemPath, err := curPath.Append(entName, true) itemPath, err := curPath.AppendItem(entName)
if err != nil { if err != nil {
return clues.Wrap(err, "getting full item path for base entry") return clues.Wrap(err, "getting full item path for base entry")
} }
@ -473,7 +473,7 @@ func streamBaseEntries(
// backup details. If the item moved and we had only the new path, we'd be // backup details. If the item moved and we had only the new path, we'd be
// unable to find it in the old backup details because we wouldn't know what // unable to find it in the old backup details because we wouldn't know what
// to look for. // to look for.
prevItemPath, err := prevPath.Append(entName, true) prevItemPath, err := prevPath.AppendItem(entName)
if err != nil { if err != nil {
return clues.Wrap(err, "getting previous full item path for base entry") return clues.Wrap(err, "getting previous full item path for base entry")
} }

View File

@ -73,7 +73,7 @@ func testForFiles(
for s := range c.Items(ctx, fault.New(true)) { for s := range c.Items(ctx, fault.New(true)) {
count++ count++
fullPath, err := c.FullPath().Append(s.UUID(), true) fullPath, err := c.FullPath().AppendItem(s.UUID())
require.NoError(t, err, clues.ToCore(err)) require.NoError(t, err, clues.ToCore(err))
expected, ok := expected[fullPath.String()] expected, ok := expected[fullPath.String()]
@ -689,10 +689,10 @@ func (suite *KopiaIntegrationSuite) TestRestoreAfterCompressionChange() {
dc1 := exchMock.NewCollection(suite.storePath1, suite.locPath1, 1) dc1 := exchMock.NewCollection(suite.storePath1, suite.locPath1, 1)
dc2 := exchMock.NewCollection(suite.storePath2, suite.locPath2, 1) dc2 := exchMock.NewCollection(suite.storePath2, suite.locPath2, 1)
fp1, err := suite.storePath1.Append(dc1.Names[0], true) fp1, err := suite.storePath1.AppendItem(dc1.Names[0])
require.NoError(t, err, clues.ToCore(err)) require.NoError(t, err, clues.ToCore(err))
fp2, err := suite.storePath2.Append(dc2.Names[0], true) fp2, err := suite.storePath2.AppendItem(dc2.Names[0])
require.NoError(t, err, clues.ToCore(err)) require.NoError(t, err, clues.ToCore(err))
stats, _, _, err := w.ConsumeBackupCollections( stats, _, _, err := w.ConsumeBackupCollections(
@ -838,7 +838,7 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() {
// 5 file and 2 folder entries. // 5 file and 2 folder entries.
assert.Len(t, deets.Details().Entries, 5+2) assert.Len(t, deets.Details().Entries, 5+2)
failedPath, err := suite.storePath2.Append(testFileName4, true) failedPath, err := suite.storePath2.AppendItem(testFileName4)
require.NoError(t, err, clues.ToCore(err)) require.NoError(t, err, clues.ToCore(err))
ic := i64counter{} ic := i64counter{}
@ -987,7 +987,7 @@ func (suite *KopiaSimpleRepoIntegrationSuite) SetupSuite() {
} }
for _, item := range filesInfo { for _, item := range filesInfo {
pth, err := item.parentPath.Append(item.name, true) pth, err := item.parentPath.AppendItem(item.name)
require.NoError(suite.T(), err, clues.ToCore(err)) require.NoError(suite.T(), err, clues.ToCore(err))
mapKey := item.parentPath.String() mapKey := item.parentPath.String()
@ -1439,7 +1439,7 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestProduceRestoreCollections_Path
item, ok := suite.filesByPath[pth.StoragePath.String()] item, ok := suite.filesByPath[pth.StoragePath.String()]
require.True(t, ok, "getting expected file data") require.True(t, ok, "getting expected file data")
itemPath, err := pth.RestorePath.Append(pth.StoragePath.Item(), true) itemPath, err := pth.RestorePath.AppendItem(pth.StoragePath.Item())
require.NoError(t, err, "getting expected item path") require.NoError(t, err, "getting expected item path")
expected[itemPath.String()] = item.data expected[itemPath.String()] = item.data
@ -1532,7 +1532,7 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestProduceRestoreCollections_Fetc
} }
func (suite *KopiaSimpleRepoIntegrationSuite) TestProduceRestoreCollections_Errors() { func (suite *KopiaSimpleRepoIntegrationSuite) TestProduceRestoreCollections_Errors() {
itemPath, err := suite.testPath1.Append(testFileName, true) itemPath, err := suite.testPath1.AppendItem(testFileName)
require.NoError(suite.T(), err, clues.ToCore(err)) require.NoError(suite.T(), err, clues.ToCore(err))
table := []struct { table := []struct {

View File

@ -140,7 +140,7 @@ func (suite *OperationsManifestsUnitSuite) TestCollectMetadata() {
ps := make([]path.Path, 0, len(files)) ps := make([]path.Path, 0, len(files))
for _, f := range files { for _, f := range files {
p, err := emailPath.Append(f, true) p, err := emailPath.AppendItem(f)
assert.NoError(t, err, clues.ToCore(err)) assert.NoError(t, err, clues.ToCore(err))
ps = append(ps, p) ps = append(ps, p)
} }
@ -163,7 +163,7 @@ func (suite *OperationsManifestsUnitSuite) TestCollectMetadata() {
ps := make([]path.Path, 0, len(files)) ps := make([]path.Path, 0, len(files))
for _, f := range files { for _, f := range files {
p, err := emailPath.Append(f, true) p, err := emailPath.AppendItem(f)
assert.NoError(t, err, clues.ToCore(err)) assert.NoError(t, err, clues.ToCore(err))
ps = append(ps, p) ps = append(ps, p)
} }
@ -191,10 +191,10 @@ func (suite *OperationsManifestsUnitSuite) TestCollectMetadata() {
ps := make([]path.Path, 0, len(files)) ps := make([]path.Path, 0, len(files))
for _, f := range files { for _, f := range files {
p, err := emailPath.Append(f, true) p, err := emailPath.AppendItem(f)
assert.NoError(t, err, clues.ToCore(err)) assert.NoError(t, err, clues.ToCore(err))
ps = append(ps, p) ps = append(ps, p)
p, err = contactPath.Append(f, true) p, err = contactPath.AppendItem(f)
assert.NoError(t, err, clues.ToCore(err)) assert.NoError(t, err, clues.ToCore(err))
ps = append(ps, p) ps = append(ps, p)
} }
@ -222,10 +222,10 @@ func (suite *OperationsManifestsUnitSuite) TestCollectMetadata() {
ps := make([]path.Path, 0, len(files)) ps := make([]path.Path, 0, len(files))
for _, f := range files { for _, f := range files {
p, err := emailPath.Append(f, true) p, err := emailPath.AppendItem(f)
assert.NoError(t, err, clues.ToCore(err)) assert.NoError(t, err, clues.ToCore(err))
ps = append(ps, p) ps = append(ps, p)
p, err = contactPath.Append(f, true) p, err = contactPath.AppendItem(f)
assert.NoError(t, err, clues.ToCore(err)) assert.NoError(t, err, clues.ToCore(err))
ps = append(ps, p) ps = append(ps, p)
} }

View File

@ -25,7 +25,7 @@ func mustParsePath(ref string, isItem bool) path.Path {
// path with the element appended to it. Panics if the path cannot be parsed. // path with the element appended to it. Panics if the path cannot be parsed.
// Useful for simple variable assignments. // Useful for simple variable assignments.
func mustAppendPath(p path.Path, newElement string, isItem bool) path.Path { func mustAppendPath(p path.Path, newElement string, isItem bool) path.Path {
newP, err := p.Append(newElement, isItem) newP, err := p.Append(isItem, newElement)
if err != nil { if err != nil {
panic(err) panic(err)
} }

View File

@ -86,3 +86,12 @@ func (el Elements) String() string {
func (el Elements) PlainString() string { func (el Elements) PlainString() string {
return join(el) return join(el)
} }
// Last returns the last element. Returns "" if empty.
func (el Elements) Last() string {
if len(el) == 0 {
return ""
}
return el[len(el)-1]
}

View File

@ -106,7 +106,9 @@ type Path interface {
// Append returns a new Path object with the given element added to the end of // Append returns a new Path object with the given element added to the end of
// the old Path if possible. If the old Path is an item Path then Append // the old Path if possible. If the old Path is an item Path then Append
// returns an error. // returns an error.
Append(element string, isItem bool) (Path, error) Append(isItem bool, elems ...string) (Path, error)
// AppendItem is a shorthand for Append(true, someItem)
AppendItem(item string) (Path, error)
// ShortRef returns a short reference representing this path. The short // ShortRef returns a short reference representing this path. The short
// reference is guaranteed to be unique. No guarantees are made about whether // 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. // a short reference can be converted back into the Path that generated it.

View File

@ -245,6 +245,26 @@ func (suite *PathUnitSuite) TestAppend() {
} }
} }
func (suite *PathUnitSuite) TestAppendItem() {
t := suite.T()
p, err := Build("t", "ro", ExchangeService, EmailCategory, false, "foo", "bar")
require.NoError(t, err, clues.ToCore(err))
pb := p.ToBuilder()
assert.Equal(t, pb.String(), p.String())
pb = pb.Append("qux")
p, err = p.AppendItem("qux")
require.NoError(t, err, clues.ToCore(err))
assert.Equal(t, pb.String(), p.String())
_, err = p.AppendItem("fnords")
require.Error(t, err, clues.ToCore(err))
}
func (suite *PathUnitSuite) TestUnescapeAndAppend() { func (suite *PathUnitSuite) TestUnescapeAndAppend() {
table := append(append([]testData{}, genericCases...), basicEscapedInputs...) table := append(append([]testData{}, genericCases...), basicEscapedInputs...)
for _, test := range table { for _, test := range table {

View File

@ -253,21 +253,25 @@ func (rp dataLayerResourcePath) Dir() (Path, error) {
} }
func (rp dataLayerResourcePath) Append( func (rp dataLayerResourcePath) Append(
element string,
isItem bool, isItem bool,
elems ...string,
) (Path, error) { ) (Path, error) {
if rp.hasItem { if rp.hasItem {
return nil, clues.New("appending to an item path") return nil, clues.New("appending to an item path")
} }
return &dataLayerResourcePath{ return &dataLayerResourcePath{
Builder: *rp.Builder.Append(element), Builder: *rp.Builder.Append(elems...),
service: rp.service, service: rp.service,
category: rp.category, category: rp.category,
hasItem: isItem, hasItem: isItem,
}, nil }, nil
} }
func (rp dataLayerResourcePath) AppendItem(item string) (Path, error) {
return rp.Append(true, item)
}
func (rp dataLayerResourcePath) ToBuilder() *Builder { func (rp dataLayerResourcePath) ToBuilder() *Builder {
// Safe to directly return the Builder because Builders are immutable. // Safe to directly return the Builder because Builders are immutable.
return &rp.Builder return &rp.Builder

View File

@ -547,7 +547,7 @@ func (suite *PopulatedDataLayerResourcePath) TestAppend() {
suite.Run(test.name, func() { suite.Run(test.name, func() {
t := suite.T() t := suite.T()
newPath, err := suite.paths[m.isItem].Append(newElement, test.hasItem) newPath, err := suite.paths[m.isItem].Append(test.hasItem, newElement)
// Items don't allow appending. // Items don't allow appending.
if m.isItem { if m.isItem {