diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index c339efb07..ea2ef6308 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -318,6 +318,7 @@ func (i *Item) ModTime() time.Time { return i.info.Modified() } // getDriveItemContent fetch drive item's contents with retries func (oc *Collection) getDriveItemContent( ctx context.Context, + driveID string, item models.DriveItemable, errs *fault.Bus, ) (io.ReadCloser, error) { @@ -338,14 +339,14 @@ func (oc *Collection) getDriveItemContent( if err != nil { if clues.HasLabel(err, graph.LabelsMalware) || (item != nil && item.GetMalware() != nil) { logger.CtxErr(ctx, err).With("skipped_reason", fault.SkipMalware).Info("item flagged as malware") - el.AddSkip(fault.FileSkip(fault.SkipMalware, itemID, itemName, graph.ItemInfo(item))) + el.AddSkip(fault.FileSkip(fault.SkipMalware, driveID, itemID, itemName, graph.ItemInfo(item))) return nil, clues.Wrap(err, "malware item").Label(graph.LabelsSkippable) } if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || graph.IsErrDeletedInFlight(err) { logger.CtxErr(ctx, err).With("skipped_reason", fault.SkipNotFound).Info("item not found") - el.AddSkip(fault.FileSkip(fault.SkipNotFound, itemID, itemName, graph.ItemInfo(item))) + el.AddSkip(fault.FileSkip(fault.SkipNotFound, driveID, itemID, itemName, graph.ItemInfo(item))) return nil, clues.Wrap(err, "deleted item").Label(graph.LabelsSkippable) } @@ -360,7 +361,7 @@ func (oc *Collection) getDriveItemContent( // restore, or we have to handle it separately by somehow // deleting the entire collection. logger.CtxErr(ctx, err).With("skipped_reason", fault.SkipBigOneNote).Info("max OneNote file size exceeded") - el.AddSkip(fault.FileSkip(fault.SkipBigOneNote, itemID, itemName, graph.ItemInfo(item))) + el.AddSkip(fault.FileSkip(fault.SkipBigOneNote, driveID, itemID, itemName, graph.ItemInfo(item))) return nil, clues.Wrap(err, "max oneNote item").Label(graph.LabelsSkippable) } @@ -528,7 +529,7 @@ func (oc *Collection) populateItems(ctx context.Context, errs *fault.Bus) { // attempts to read bytes. Assumption is that kopia will check things // like file modtimes before attempting to read. itemReader := lazy.NewLazyReadCloser(func() (io.ReadCloser, error) { - itemData, err := oc.getDriveItemContent(ctx, item, errs) + itemData, err := oc.getDriveItemContent(ctx, oc.driveID, item, errs) if err != nil { return nil, err } diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index 48dcc0f67..095348812 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -710,7 +710,7 @@ func (suite *GetDriveItemUnitTestSuite) TestGetDriveItem_error() { return item, nil } - _, err := col.getDriveItemContent(ctx, item, errs) + _, err := col.getDriveItemContent(ctx, "driveID", item, errs) if test.err == nil { assert.NoError(t, err, clues.ToCore(err)) return diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index d7503949e..5faff46e9 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -717,10 +717,10 @@ func (c *Collections) UpdateCollections( if item.GetMalware() != nil { addtl := graph.ItemInfo(item) - skip := fault.FileSkip(fault.SkipMalware, itemID, itemName, addtl) + skip := fault.FileSkip(fault.SkipMalware, driveID, itemID, itemName, addtl) if isFolder { - skip = fault.ContainerSkip(fault.SkipMalware, itemID, itemName, addtl) + skip = fault.ContainerSkip(fault.SkipMalware, driveID, itemID, itemName, addtl) } errs.AddSkip(skip) diff --git a/src/internal/streamstore/collectables_test.go b/src/internal/streamstore/collectables_test.go index 6cefb804a..8cd45aea9 100644 --- a/src/internal/streamstore/collectables_test.go +++ b/src/internal/streamstore/collectables_test.go @@ -108,8 +108,8 @@ func (suite *StreamStoreIntgSuite) TestStreamer() { bus := fault.New(false) bus.Fail(clues.New("foo")) bus.AddRecoverable(clues.New("bar")) - bus.AddRecoverable(fault.FileErr(clues.New("file"), "file-id", "file-name", map[string]any{"foo": "bar"})) - bus.AddSkip(fault.FileSkip(fault.SkipMalware, "file-id", "file-name", map[string]any{"foo": "bar"})) + bus.AddRecoverable(fault.FileErr(clues.New("file"), "ns", "file-id", "file-name", map[string]any{"foo": "bar"})) + bus.AddSkip(fault.FileSkip(fault.SkipMalware, "ns", "file-id", "file-name", map[string]any{"foo": "bar"})) fe := bus.Errors() return fe @@ -137,8 +137,8 @@ func (suite *StreamStoreIntgSuite) TestStreamer() { bus := fault.New(false) bus.Fail(clues.New("foo")) bus.AddRecoverable(clues.New("bar")) - bus.AddRecoverable(fault.FileErr(clues.New("file"), "file-id", "file-name", map[string]any{"foo": "bar"})) - bus.AddSkip(fault.FileSkip(fault.SkipMalware, "file-id", "file-name", map[string]any{"foo": "bar"})) + bus.AddRecoverable(fault.FileErr(clues.New("file"), "ns", "file-id", "file-name", map[string]any{"foo": "bar"})) + bus.AddSkip(fault.FileSkip(fault.SkipMalware, "ns", "file-id", "file-name", map[string]any{"foo": "bar"})) fe := bus.Errors() return fe diff --git a/src/pkg/fault/example_fault_test.go b/src/pkg/fault/example_fault_test.go index 222d52270..c830a9aa9 100644 --- a/src/pkg/fault/example_fault_test.go +++ b/src/pkg/fault/example_fault_test.go @@ -428,6 +428,7 @@ func ExampleBus_AddSkip() { // error. Our only option is to skip it. errs.AddSkip(fault.FileSkip( fault.SkipMalware, + "deduplication-namespace", "file-id", "file-name", map[string]any{"foo": "bar"}, diff --git a/src/pkg/fault/fault.go b/src/pkg/fault/fault.go index c3601bd14..041e3d4e2 100644 --- a/src/pkg/fault/fault.go +++ b/src/pkg/fault/fault.go @@ -204,7 +204,7 @@ type Errors struct { // Items are the reduction of all errors (both the failure and the // recovered values) in the Errors struct into a slice of items, - // deduplicated by their ID. + // deduplicated by their Namespace + ID. Items []Item `json:"items"` // Skipped is the accumulation of skipped items. Skipped items @@ -218,7 +218,8 @@ type Errors struct { } // itemsIn reduces all errors (both the failure and recovered values) -// in the Errors struct into a slice of items, deduplicated by their ID. +// in the Errors struct into a slice of items, deduplicated by their +// Namespace + ID. // Any non-item error is serialized to a clues.ErrCore and returned in // the second list. func itemsIn(failure error, recovered []error) ([]Item, []*clues.ErrCore) { @@ -234,12 +235,12 @@ func itemsIn(failure error, recovered []error) ([]Item, []*clues.ErrCore) { continue } - is[ie.ID] = *ie + is[ie.dedupeID()] = *ie } var ie *Item if errors.As(failure, &ie) { - is[ie.ID] = *ie + is[ie.dedupeID()] = *ie } return maps.Values(is), non diff --git a/src/pkg/fault/fault_test.go b/src/pkg/fault/fault_test.go index 3370e3d3e..4d731ede1 100644 --- a/src/pkg/fault/fault_test.go +++ b/src/pkg/fault/fault_test.go @@ -193,7 +193,7 @@ func (suite *FaultErrorsUnitSuite) TestAddSkip() { n.AddRecoverable(assert.AnError) assert.Len(t, n.Skipped(), 0) - n.AddSkip(fault.OwnerSkip(fault.SkipMalware, "id", "name", nil)) + n.AddSkip(fault.OwnerSkip(fault.SkipMalware, "ns", "id", "name", nil)) assert.Len(t, n.Skipped(), 1) } @@ -262,12 +262,12 @@ func (suite *FaultErrorsUnitSuite) TestErrors_Items() { name: "failure item", errs: func() *fault.Errors { b := fault.New(false) - b.Fail(fault.OwnerErr(ae, "id", "name", addtl)) + b.Fail(fault.OwnerErr(ae, "ns", "id", "name", addtl)) b.AddRecoverable(ae) return b.Errors() }, - expectItems: []fault.Item{*fault.OwnerErr(ae, "id", "name", addtl)}, + expectItems: []fault.Item{*fault.OwnerErr(ae, "ns", "id", "name", addtl)}, expectRecoverable: noncore, }, { @@ -275,25 +275,40 @@ func (suite *FaultErrorsUnitSuite) TestErrors_Items() { errs: func() *fault.Errors { b := fault.New(false) b.Fail(ae) - b.AddRecoverable(fault.OwnerErr(ae, "id", "name", addtl)) + b.AddRecoverable(fault.OwnerErr(ae, "ns", "id", "name", addtl)) return b.Errors() }, - expectItems: []fault.Item{*fault.OwnerErr(ae, "id", "name", addtl)}, + expectItems: []fault.Item{*fault.OwnerErr(ae, "ns", "id", "name", addtl)}, expectRecoverable: []*clues.ErrCore{}, }, { name: "two items", errs: func() *fault.Errors { b := fault.New(false) - b.Fail(fault.OwnerErr(ae, "oid", "name", addtl)) - b.AddRecoverable(fault.FileErr(ae, "fid", "name", addtl)) + b.Fail(fault.OwnerErr(ae, "ns", "oid", "name", addtl)) + b.AddRecoverable(fault.FileErr(ae, "ns", "fid", "name", addtl)) return b.Errors() }, expectItems: []fault.Item{ - *fault.OwnerErr(ae, "oid", "name", addtl), - *fault.FileErr(ae, "fid", "name", addtl), + *fault.OwnerErr(ae, "ns", "oid", "name", addtl), + *fault.FileErr(ae, "ns", "fid", "name", addtl), + }, + expectRecoverable: []*clues.ErrCore{}, + }, + { + name: "two items - diff namespace same id", + errs: func() *fault.Errors { + b := fault.New(false) + b.Fail(fault.OwnerErr(ae, "ns", "id", "name", addtl)) + b.AddRecoverable(fault.FileErr(ae, "ns2", "id", "name", addtl)) + + return b.Errors() + }, + expectItems: []fault.Item{ + *fault.OwnerErr(ae, "ns", "id", "name", addtl), + *fault.FileErr(ae, "ns2", "id", "name", addtl), }, expectRecoverable: []*clues.ErrCore{}, }, @@ -301,13 +316,13 @@ func (suite *FaultErrorsUnitSuite) TestErrors_Items() { name: "duplicate items - failure priority", errs: func() *fault.Errors { b := fault.New(false) - b.Fail(fault.OwnerErr(ae, "id", "name", addtl)) - b.AddRecoverable(fault.FileErr(ae, "id", "name", addtl)) + b.Fail(fault.OwnerErr(ae, "ns", "id", "name", addtl)) + b.AddRecoverable(fault.FileErr(ae, "ns", "id", "name", addtl)) return b.Errors() }, expectItems: []fault.Item{ - *fault.OwnerErr(ae, "id", "name", addtl), + *fault.OwnerErr(ae, "ns", "id", "name", addtl), }, expectRecoverable: []*clues.ErrCore{}, }, @@ -316,13 +331,13 @@ func (suite *FaultErrorsUnitSuite) TestErrors_Items() { errs: func() *fault.Errors { b := fault.New(false) b.Fail(ae) - b.AddRecoverable(fault.FileErr(ae, "fid", "name", addtl)) - b.AddRecoverable(fault.FileErr(ae, "fid", "name2", addtl)) + b.AddRecoverable(fault.FileErr(ae, "ns", "fid", "name", addtl)) + b.AddRecoverable(fault.FileErr(ae, "ns", "fid", "name2", addtl)) return b.Errors() }, expectItems: []fault.Item{ - *fault.FileErr(ae, "fid", "name2", addtl), + *fault.FileErr(ae, "ns", "fid", "name2", addtl), }, expectRecoverable: []*clues.ErrCore{}, }, @@ -331,13 +346,13 @@ func (suite *FaultErrorsUnitSuite) TestErrors_Items() { errs: func() *fault.Errors { b := fault.New(false) b.Fail(ae) - b.AddRecoverable(fault.FileErr(ae, "fid", "name", addtl)) + b.AddRecoverable(fault.FileErr(ae, "ns", "fid", "name", addtl)) b.AddRecoverable(ae) return b.Errors() }, expectItems: []fault.Item{ - *fault.FileErr(ae, "fid", "name", addtl), + *fault.FileErr(ae, "ns", "fid", "name", addtl), }, expectRecoverable: noncore, }, diff --git a/src/pkg/fault/item.go b/src/pkg/fault/item.go index 6aaa07416..8b5f8c929 100644 --- a/src/pkg/fault/item.go +++ b/src/pkg/fault/item.go @@ -49,6 +49,12 @@ var ( // by the end user (cli or sdk) for surfacing human-readable and // identifiable points of failure. type Item struct { + // deduplication namespace; the maximally-unique boundary of the + // item ID. The scope of this boundary depends on the service. + // ex: exchange items are unique within their category, drive items + // are only unique within a given drive. + Namespace string `json:"namespace"` + // deduplication identifier; the ID of the observed item. ID string `json:"id"` @@ -72,6 +78,12 @@ type Item struct { Additional map[string]any `json:"additional"` } +// dedupeID is the id used to deduplicate items when aggreagating +// errors in fault.Errors(). +func (i *Item) dedupeID() string { + return i.Namespace + i.ID +} + // Error complies with the error interface. func (i *Item) Error() string { if i == nil { @@ -111,23 +123,24 @@ func (i Item) Values() []string { } // ContainerErr produces a Container-type Item for tracking erroneous items -func ContainerErr(cause error, id, name string, addtl map[string]any) *Item { - return itemErr(ContainerType, cause, id, name, addtl) +func ContainerErr(cause error, namespace, id, name string, addtl map[string]any) *Item { + return itemErr(ContainerType, cause, namespace, id, name, addtl) } // FileErr produces a File-type Item for tracking erroneous items. -func FileErr(cause error, id, name string, addtl map[string]any) *Item { - return itemErr(FileType, cause, id, name, addtl) +func FileErr(cause error, namespace, id, name string, addtl map[string]any) *Item { + return itemErr(FileType, cause, namespace, id, name, addtl) } // OnwerErr produces a ResourceOwner-type Item for tracking erroneous items. -func OwnerErr(cause error, id, name string, addtl map[string]any) *Item { - return itemErr(ResourceOwnerType, cause, id, name, addtl) +func OwnerErr(cause error, namespace, id, name string, addtl map[string]any) *Item { + return itemErr(ResourceOwnerType, cause, namespace, id, name, addtl) } // itemErr produces a Item of the provided type for tracking erroneous items. -func itemErr(t itemType, cause error, id, name string, addtl map[string]any) *Item { +func itemErr(t itemType, cause error, namespace, id, name string, addtl map[string]any) *Item { return &Item{ + Namespace: namespace, ID: id, Name: name, Type: t, @@ -228,24 +241,25 @@ func (s Skipped) Values() []string { } // ContainerSkip produces a Container-kind Item for tracking skipped items. -func ContainerSkip(cause skipCause, id, name string, addtl map[string]any) *Skipped { - return itemSkip(ContainerType, cause, id, name, addtl) +func ContainerSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped { + return itemSkip(ContainerType, cause, namespace, id, name, addtl) } // FileSkip produces a File-kind Item for tracking skipped items. -func FileSkip(cause skipCause, id, name string, addtl map[string]any) *Skipped { - return itemSkip(FileType, cause, id, name, addtl) +func FileSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped { + return itemSkip(FileType, cause, namespace, id, name, addtl) } // OnwerSkip produces a ResourceOwner-kind Item for tracking skipped items. -func OwnerSkip(cause skipCause, id, name string, addtl map[string]any) *Skipped { - return itemSkip(ResourceOwnerType, cause, id, name, addtl) +func OwnerSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped { + return itemSkip(ResourceOwnerType, cause, namespace, id, name, addtl) } // itemSkip produces a Item of the provided type for tracking skipped items. -func itemSkip(t itemType, cause skipCause, id, name string, addtl map[string]any) *Skipped { +func itemSkip(t itemType, cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped { return &Skipped{ Item: Item{ + Namespace: namespace, ID: id, Name: name, Type: t, diff --git a/src/pkg/fault/item_test.go b/src/pkg/fault/item_test.go index 18fce66f7..b597121ee 100644 --- a/src/pkg/fault/item_test.go +++ b/src/pkg/fault/item_test.go @@ -36,9 +36,10 @@ func (suite *ItemUnitSuite) TestItem_Error() { func (suite *ItemUnitSuite) TestContainerErr() { t := suite.T() addtl := map[string]any{"foo": "bar"} - i := ContainerErr(clues.New("foo"), "id", "name", addtl) + i := ContainerErr(clues.New("foo"), "ns", "id", "name", addtl) expect := Item{ + Namespace: "ns", ID: "id", Name: "name", Type: ContainerType, @@ -52,9 +53,10 @@ func (suite *ItemUnitSuite) TestContainerErr() { func (suite *ItemUnitSuite) TestFileErr() { t := suite.T() addtl := map[string]any{"foo": "bar"} - i := FileErr(clues.New("foo"), "id", "name", addtl) + i := FileErr(clues.New("foo"), "ns", "id", "name", addtl) expect := Item{ + Namespace: "ns", ID: "id", Name: "name", Type: FileType, @@ -68,9 +70,10 @@ func (suite *ItemUnitSuite) TestFileErr() { func (suite *ItemUnitSuite) TestOwnerErr() { t := suite.T() addtl := map[string]any{"foo": "bar"} - i := OwnerErr(clues.New("foo"), "id", "name", addtl) + i := OwnerErr(clues.New("foo"), "ns", "id", "name", addtl) expect := Item{ + Namespace: "ns", ID: "id", Name: "name", Type: ResourceOwnerType, @@ -127,17 +130,17 @@ func (suite *ItemUnitSuite) TestItem_HeadersValues() { }{ { name: "file", - item: FileErr(assert.AnError, "id", "name", addtl), + item: FileErr(assert.AnError, "ns", "id", "name", addtl), expect: []string{"Error", FileType.Printable(), "name", "cname", cause}, }, { name: "container", - item: ContainerErr(assert.AnError, "id", "name", addtl), + item: ContainerErr(assert.AnError, "ns", "id", "name", addtl), expect: []string{"Error", ContainerType.Printable(), "name", "cname", cause}, }, { name: "owner", - item: OwnerErr(assert.AnError, "id", "name", nil), + item: OwnerErr(assert.AnError, "ns", "id", "name", nil), expect: []string{"Error", ResourceOwnerType.Printable(), "name", "", cause}, }, } @@ -169,9 +172,10 @@ func (suite *ItemUnitSuite) TestSkipped_String() { func (suite *ItemUnitSuite) TestContainerSkip() { t := suite.T() addtl := map[string]any{"foo": "bar"} - i := ContainerSkip(SkipMalware, "id", "name", addtl) + i := ContainerSkip(SkipMalware, "ns", "id", "name", addtl) expect := Item{ + Namespace: "ns", ID: "id", Name: "name", Type: ContainerType, @@ -185,9 +189,10 @@ func (suite *ItemUnitSuite) TestContainerSkip() { func (suite *ItemUnitSuite) TestFileSkip() { t := suite.T() addtl := map[string]any{"foo": "bar"} - i := FileSkip(SkipMalware, "id", "name", addtl) + i := FileSkip(SkipMalware, "ns", "id", "name", addtl) expect := Item{ + Namespace: "ns", ID: "id", Name: "name", Type: FileType, @@ -201,9 +206,10 @@ func (suite *ItemUnitSuite) TestFileSkip() { func (suite *ItemUnitSuite) TestOwnerSkip() { t := suite.T() addtl := map[string]any{"foo": "bar"} - i := OwnerSkip(SkipMalware, "id", "name", addtl) + i := OwnerSkip(SkipMalware, "ns", "id", "name", addtl) expect := Item{ + Namespace: "ns", ID: "id", Name: "name", Type: ResourceOwnerType, @@ -227,17 +233,17 @@ func (suite *ItemUnitSuite) TestSkipped_HeadersValues() { }{ { name: "file", - skip: FileSkip(SkipMalware, "id", "name", addtl), + skip: FileSkip(SkipMalware, "ns", "id", "name", addtl), expect: []string{"Skip", FileType.Printable(), "name", "cname", string(SkipMalware)}, }, { name: "container", - skip: ContainerSkip(SkipMalware, "id", "name", addtl), + skip: ContainerSkip(SkipMalware, "ns", "id", "name", addtl), expect: []string{"Skip", ContainerType.Printable(), "name", "cname", string(SkipMalware)}, }, { name: "owner", - skip: OwnerSkip(SkipMalware, "id", "name", nil), + skip: OwnerSkip(SkipMalware, "ns", "id", "name", nil), expect: []string{"Skip", ResourceOwnerType.Printable(), "name", "", string(SkipMalware)}, }, } diff --git a/src/pkg/fault/testdata/testdata.go b/src/pkg/fault/testdata/testdata.go index 8b3cf7bb8..a3a0e48dc 100644 --- a/src/pkg/fault/testdata/testdata.go +++ b/src/pkg/fault/testdata/testdata.go @@ -19,7 +19,7 @@ func MakeErrors(failure, recovered, skipped bool) fault.Errors { } if skipped { - fe.Skipped = []fault.Skipped{*fault.FileSkip(fault.SkipMalware, "id", "name", nil)} + fe.Skipped = []fault.Skipped{*fault.FileSkip(fault.SkipMalware, "ns", "id", "name", nil)} } return fe diff --git a/src/pkg/repository/repository_unexported_test.go b/src/pkg/repository/repository_unexported_test.go index 92fbd86d5..6860f6250 100644 --- a/src/pkg/repository/repository_unexported_test.go +++ b/src/pkg/repository/repository_unexported_test.go @@ -434,8 +434,8 @@ func (suite *RepositoryModelIntgSuite) TestGetBackupErrors() { var ( err = clues.Wrap(assert.AnError, "wrap") cec = err.Core() - item = fault.FileErr(err, "file-id", "file-name", map[string]any{"foo": "bar"}) - skip = fault.FileSkip(fault.SkipMalware, "s-file-id", "s-file-name", map[string]any{"foo": "bar"}) + item = fault.FileErr(err, "ns", "file-id", "file-name", map[string]any{"foo": "bar"}) + skip = fault.FileSkip(fault.SkipMalware, "ns", "s-file-id", "s-file-name", map[string]any{"foo": "bar"}) info = details.ItemInfo{ Exchange: &details.ExchangeInfo{ ItemType: details.ExchangeMail,