wrap repo backup data.notfound errors (#3004)

Adds a backupNotFound sentinel error that sdk
consumers can use to easily identify when a backup did not produce any backup data.

Also adds extra testing and mocks to support
testing inside the repo package.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature
- [x] 🤖 Supportability/Tests

#### Test Plan

- [x] 💪 Manual
- [x] ⚡ Unit test

src/pkg/repository/repository.go

@@ -12,6 +12,7 @@ import (
 	"github.com/alcionai/corso/src/internal/connector"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/onedrive"
+	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/internal/events"
 	"github.com/alcionai/corso/src/internal/kopia"
 	"github.com/alcionai/corso/src/internal/model"
@@ -30,7 +31,10 @@ import (
 	"github.com/alcionai/corso/src/pkg/store"
 )
 
-var ErrorRepoAlreadyExists = clues.New("a repository was already initialized with that configuration")
+var (
+	ErrorRepoAlreadyExists = clues.New("a repository was already initialized with that configuration")
+	ErrorBackupNotFound    = clues.New("no backup exists with that id")
+)
 
 // BackupGetter deals with retrieving metadata about backups from the
 // repository.
@@ -329,10 +333,23 @@ func (r repository) NewRestore(
 		r.Bus)
 }
 
-// backups lists a backup by id
+// Backup retrieves a backup by id.
 func (r repository) Backup(ctx context.Context, id string) (*backup.Backup, error) {
-	sw := store.NewKopiaStore(r.modelStore)
+	return getBackup(ctx, id, store.NewKopiaStore(r.modelStore))
-	return sw.GetBackup(ctx, model.StableID(id))
+}
+
+// getBackup handles the processing for Backup.
+func getBackup(
+	ctx context.Context,
+	id string,
+	sw store.BackupGetter,
+) (*backup.Backup, error) {
+	b, err := sw.GetBackup(ctx, model.StableID(id))
+	if err != nil {
+		return nil, errWrapper(err)
+	}
+
+	return b, nil
 }
 
 // BackupsByID lists backups by ID. Returns as many backups as possible with
@@ -349,7 +366,7 @@ func (r repository) Backups(ctx context.Context, ids []string) ([]*backup.Backup
 
 		b, err := sw.GetBackup(ictx, model.StableID(id))
 		if err != nil {
-			errs.AddRecoverable(clues.Stack(err))
+			errs.AddRecoverable(errWrapper(err))
 		}
 
 		bups = append(bups, b)
@@ -387,12 +404,12 @@ func getBackupDetails(
 	ctx context.Context,
 	backupID, tenantID string,
 	kw *kopia.Wrapper,
-	sw *store.Wrapper,
+	sw store.BackupGetter,
 	errs *fault.Bus,
 ) (*details.Details, *backup.Backup, error) {
 	b, err := sw.GetBackup(ctx, model.StableID(backupID))
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errWrapper(err)
 	}
 
 	ssid := b.StreamStoreID
@@ -457,12 +474,12 @@ func getBackupErrors(
 	ctx context.Context,
 	backupID, tenantID string,
 	kw *kopia.Wrapper,
-	sw *store.Wrapper,
+	sw store.BackupGetter,
 	errs *fault.Bus,
 ) (*fault.Errors, *backup.Backup, error) {
 	b, err := sw.GetBackup(ctx, model.StableID(backupID))
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errWrapper(err)
 	}
 
 	ssid := b.StreamStoreID
@@ -487,31 +504,43 @@ func getBackupErrors(
 	return &fe, b, nil
 }
 
+type snapshotDeleter interface {
+	DeleteSnapshot(ctx context.Context, snapshotID string) error
+}
+
 // DeleteBackup removes the backup from both the model store and the backup storage.
 func (r repository) DeleteBackup(ctx context.Context, id string) error {
-	bu, err := r.Backup(ctx, id)
+	return deleteBackup(ctx, id, r.dataLayer, store.NewKopiaStore(r.modelStore))
+}
+
+// deleteBackup handles the processing for Backup.
+func deleteBackup(
+	ctx context.Context,
+	id string,
+	kw snapshotDeleter,
+	sw store.BackupGetterDeleter,
+) error {
+	b, err := sw.GetBackup(ctx, model.StableID(id))
 	if err != nil {
+		return errWrapper(err)
+	}
+
+	if err := kw.DeleteSnapshot(ctx, b.SnapshotID); err != nil {
 		return err
 	}
 
-	if err := r.dataLayer.DeleteSnapshot(ctx, bu.SnapshotID); err != nil {
+	if len(b.SnapshotID) > 0 {
-		return err
+		if err := kw.DeleteSnapshot(ctx, b.SnapshotID); err != nil {
-	}
-
-	if len(bu.SnapshotID) > 0 {
-		if err := r.dataLayer.DeleteSnapshot(ctx, bu.SnapshotID); err != nil {
 			return err
 		}
 	}
 
-	if len(bu.DetailsID) > 0 {
+	if len(b.DetailsID) > 0 {
-		if err := r.dataLayer.DeleteSnapshot(ctx, bu.DetailsID); err != nil {
+		if err := kw.DeleteSnapshot(ctx, b.DetailsID); err != nil {
 			return err
 		}
 	}
 
-	sw := store.NewKopiaStore(r.modelStore)
-
 	return sw.DeleteBackup(ctx, model.StableID(id))
 }
 
@@ -590,3 +619,11 @@ func connectToM365(
 
 	return gc, nil
 }
+
+func errWrapper(err error) error {
+	if errors.Is(err, data.ErrNotFound) {
+		return clues.Stack(ErrorBackupNotFound, err)
+	}
+
+	return err
+}

src/pkg/repository/repository_unexported_test.go

@@ -5,10 +5,12 @@ import (
 	"testing"
 
 	"github.com/alcionai/clues"
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/internal/kopia"
 	"github.com/alcionai/corso/src/internal/model"
 	"github.com/alcionai/corso/src/internal/operations"
@@ -20,8 +22,172 @@ import (
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/selectors"
 	"github.com/alcionai/corso/src/pkg/store"
+	"github.com/alcionai/corso/src/pkg/store/mock"
 )
 
+// ---------------------------------------------------------------------------
+// Unit
+// ---------------------------------------------------------------------------
+
+type RepositoryBackupsUnitSuite struct {
+	tester.Suite
+}
+
+func TestRepositoryBackupsUnitSuite(t *testing.T) {
+	suite.Run(t, &RepositoryBackupsUnitSuite{Suite: tester.NewUnitSuite(t)})
+}
+
+func (suite *RepositoryBackupsUnitSuite) TestGetBackup() {
+	bup := &backup.Backup{
+		BaseModel: model.BaseModel{
+			ID: model.StableID(uuid.NewString()),
+		},
+	}
+
+	table := []struct {
+		name      string
+		sw        mock.BackupWrapper
+		expectErr func(t *testing.T, result error)
+		expectID  model.StableID
+	}{
+		{
+			name: "no error",
+			sw: mock.BackupWrapper{
+				Backup:    bup,
+				GetErr:    nil,
+				DeleteErr: nil,
+			},
+			expectErr: func(t *testing.T, result error) {
+				assert.NoError(t, result, clues.ToCore(result))
+			},
+			expectID: bup.ID,
+		},
+		{
+			name: "get error",
+			sw: mock.BackupWrapper{
+				Backup:    bup,
+				GetErr:    data.ErrNotFound,
+				DeleteErr: nil,
+			},
+			expectErr: func(t *testing.T, result error) {
+				assert.ErrorIs(t, result, data.ErrNotFound, clues.ToCore(result))
+				assert.ErrorIs(t, result, ErrorBackupNotFound, clues.ToCore(result))
+			},
+			expectID: bup.ID,
+		},
+	}
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			ctx, flush := tester.NewContext()
+			defer flush()
+
+			t := suite.T()
+
+			b, err := getBackup(ctx, string(bup.ID), test.sw)
+			test.expectErr(t, err)
+
+			if err != nil {
+				return
+			}
+
+			assert.Equal(t, test.expectID, b.ID)
+		})
+	}
+}
+
+type mockSSDeleter struct {
+	err error
+}
+
+func (sd mockSSDeleter) DeleteSnapshot(_ context.Context, _ string) error {
+	return sd.err
+}
+
+func (suite *RepositoryBackupsUnitSuite) TestDeleteBackup() {
+	bup := &backup.Backup{
+		BaseModel: model.BaseModel{
+			ID: model.StableID(uuid.NewString()),
+		},
+	}
+
+	table := []struct {
+		name      string
+		sw        mock.BackupWrapper
+		kw        mockSSDeleter
+		expectErr func(t *testing.T, result error)
+		expectID  model.StableID
+	}{
+		{
+			name: "no error",
+			sw: mock.BackupWrapper{
+				Backup:    bup,
+				GetErr:    nil,
+				DeleteErr: nil,
+			},
+			kw: mockSSDeleter{},
+			expectErr: func(t *testing.T, result error) {
+				assert.NoError(t, result, clues.ToCore(result))
+			},
+			expectID: bup.ID,
+		},
+		{
+			name: "get error",
+			sw: mock.BackupWrapper{
+				Backup:    bup,
+				GetErr:    data.ErrNotFound,
+				DeleteErr: nil,
+			},
+			kw: mockSSDeleter{},
+			expectErr: func(t *testing.T, result error) {
+				assert.ErrorIs(t, result, data.ErrNotFound, clues.ToCore(result))
+				assert.ErrorIs(t, result, ErrorBackupNotFound, clues.ToCore(result))
+			},
+			expectID: bup.ID,
+		},
+		{
+			name: "delete error",
+			sw: mock.BackupWrapper{
+				Backup:    bup,
+				GetErr:    nil,
+				DeleteErr: assert.AnError,
+			},
+			kw: mockSSDeleter{},
+			expectErr: func(t *testing.T, result error) {
+				assert.ErrorIs(t, result, assert.AnError, clues.ToCore(result))
+			},
+			expectID: bup.ID,
+		},
+		{
+			name: "snapshot delete error",
+			sw: mock.BackupWrapper{
+				Backup:    bup,
+				GetErr:    nil,
+				DeleteErr: nil,
+			},
+			kw: mockSSDeleter{assert.AnError},
+			expectErr: func(t *testing.T, result error) {
+				assert.ErrorIs(t, result, assert.AnError, clues.ToCore(result))
+			},
+			expectID: bup.ID,
+		},
+	}
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			ctx, flush := tester.NewContext()
+			defer flush()
+
+			t := suite.T()
+
+			err := deleteBackup(ctx, string(bup.ID), test.kw, test.sw)
+			test.expectErr(t, err)
+		})
+	}
+}
+
+// ---------------------------------------------------------------------------
+// integration
+// ---------------------------------------------------------------------------
+
 type RepositoryModelIntgSuite struct {
 	tester.Suite
 	kw          *kopia.Wrapper

src/pkg/store/backup.go

@@ -28,6 +28,29 @@ func (q *queryFilters) populate(qf ...FilterOption) {
 	}
 }
 
+type (
+	BackupWrapper interface {
+		BackupGetterDeleter
+		GetBackups(
+			ctx context.Context,
+			filters ...FilterOption,
+		) ([]*backup.Backup, error)
+	}
+
+	BackupGetterDeleter interface {
+		BackupGetter
+		BackupDeleter
+	}
+
+	BackupGetter interface {
+		GetBackup(ctx context.Context, backupID model.StableID) (*backup.Backup, error)
+	}
+
+	BackupDeleter interface {
+		DeleteBackup(ctx context.Context, backupID model.StableID) error
+	}
+)
+
 // Service ensures the retrieved backups only match
 // the specified service.
 func Service(pst path.ServiceType) FilterOption {

src/pkg/store/backup_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/alcionai/corso/src/internal/tester"
 	"github.com/alcionai/corso/src/pkg/backup"
 	"github.com/alcionai/corso/src/pkg/store"
-	storeMock "github.com/alcionai/corso/src/pkg/store/mock"
+	"github.com/alcionai/corso/src/pkg/store/mock"
 )
 
 // ------------------------------------------------------------
@@ -48,17 +48,17 @@ func (suite *StoreBackupUnitSuite) TestGetBackup() {
 
 	table := []struct {
 		name   string
-		mock   *storeMock.MockModelStore
+		mock   *mock.ModelStore
 		expect assert.ErrorAssertionFunc
 	}{
 		{
 			name:   "gets backup",
-			mock:   storeMock.NewMock(&bu, nil),
+			mock:   mock.NewModelStoreMock(&bu, nil),
 			expect: assert.NoError,
 		},
 		{
 			name:   "errors",
-			mock:   storeMock.NewMock(&bu, assert.AnError),
+			mock:   mock.NewModelStoreMock(&bu, assert.AnError),
 			expect: assert.Error,
 		},
 	}
@@ -85,17 +85,17 @@ func (suite *StoreBackupUnitSuite) TestGetBackups() {
 
 	table := []struct {
 		name   string
-		mock   *storeMock.MockModelStore
+		mock   *mock.ModelStore
 		expect assert.ErrorAssertionFunc
 	}{
 		{
 			name:   "gets backups",
-			mock:   storeMock.NewMock(&bu, nil),
+			mock:   mock.NewModelStoreMock(&bu, nil),
 			expect: assert.NoError,
 		},
 		{
 			name:   "errors",
-			mock:   storeMock.NewMock(&bu, assert.AnError),
+			mock:   mock.NewModelStoreMock(&bu, assert.AnError),
 			expect: assert.Error,
 		},
 	}
@@ -123,17 +123,17 @@ func (suite *StoreBackupUnitSuite) TestDeleteBackup() {
 
 	table := []struct {
 		name   string
-		mock   *storeMock.MockModelStore
+		mock   *mock.ModelStore
 		expect assert.ErrorAssertionFunc
 	}{
 		{
 			name:   "deletes backup",
-			mock:   storeMock.NewMock(&bu, nil),
+			mock:   mock.NewModelStoreMock(&bu, nil),
 			expect: assert.NoError,
 		},
 		{
 			name:   "errors",
-			mock:   storeMock.NewMock(&bu, assert.AnError),
+			mock:   mock.NewModelStoreMock(&bu, assert.AnError),
 			expect: assert.Error,
 		},
 	}

src/pkg/store/mock/model_store.go

@@ -14,13 +14,13 @@ import (
 // model wrapper model store
 // ------------------------------------------------------------
 
-type MockModelStore struct {
+type ModelStore struct {
 	backup *backup.Backup
 	err    error
 }
 
-func NewMock(b *backup.Backup, err error) *MockModelStore {
+func NewModelStoreMock(b *backup.Backup, err error) *ModelStore {
-	return &MockModelStore{
+	return &ModelStore{
 		backup: b,
 		err:    err,
 	}
@@ -30,11 +30,11 @@ func NewMock(b *backup.Backup, err error) *MockModelStore {
 // deleter iface
 // ------------------------------------------------------------
 
-func (mms *MockModelStore) Delete(ctx context.Context, s model.Schema, id model.StableID) error {
+func (mms *ModelStore) Delete(ctx context.Context, s model.Schema, id model.StableID) error {
 	return mms.err
 }
 
-func (mms *MockModelStore) DeleteWithModelStoreID(ctx context.Context, id manifest.ID) error {
+func (mms *ModelStore) DeleteWithModelStoreID(ctx context.Context, id manifest.ID) error {
 	return mms.err
 }
 
@@ -42,7 +42,7 @@ func (mms *MockModelStore) DeleteWithModelStoreID(ctx context.Context, id manife
 // getter iface
 // ------------------------------------------------------------
 
-func (mms *MockModelStore) Get(
+func (mms *ModelStore) Get(
 	ctx context.Context,
 	s model.Schema,
 	id model.StableID,
@@ -64,7 +64,7 @@ func (mms *MockModelStore) Get(
 	return nil
 }
 
-func (mms *MockModelStore) GetIDsForType(
+func (mms *ModelStore) GetIDsForType(
 	ctx context.Context,
 	s model.Schema,
 	tags map[string]string,
@@ -82,7 +82,7 @@ func (mms *MockModelStore) GetIDsForType(
 	return nil, clues.New("schema not supported by mock GetIDsForType").With("schema", s)
 }
 
-func (mms *MockModelStore) GetWithModelStoreID(
+func (mms *ModelStore) GetWithModelStoreID(
 	ctx context.Context,
 	s model.Schema,
 	id manifest.ID,
@@ -108,7 +108,7 @@ func (mms *MockModelStore) GetWithModelStoreID(
 // updater iface
 // ------------------------------------------------------------
 
-func (mms *MockModelStore) Put(ctx context.Context, s model.Schema, m model.Model) error {
+func (mms *ModelStore) Put(ctx context.Context, s model.Schema, m model.Model) error {
 	switch s {
 	case model.BackupSchema:
 		bm := m.(*backup.Backup)
@@ -121,7 +121,7 @@ func (mms *MockModelStore) Put(ctx context.Context, s model.Schema, m model.Mode
 	return mms.err
 }
 
-func (mms *MockModelStore) Update(ctx context.Context, s model.Schema, m model.Model) error {
+func (mms *ModelStore) Update(ctx context.Context, s model.Schema, m model.Model) error {
 	switch s {
 	case model.BackupSchema:
 		bm := m.(*backup.Backup)

src/pkg/store/mock/wrapper.go

@@ -0,0 +1,38 @@
+package mock
+
+import (
+	"context"
+
+	"github.com/alcionai/clues"
+
+	"github.com/alcionai/corso/src/internal/model"
+	"github.com/alcionai/corso/src/pkg/backup"
+	"github.com/alcionai/corso/src/pkg/store"
+)
+
+type BackupWrapper struct {
+	Backup    *backup.Backup
+	GetErr    error
+	DeleteErr error
+}
+
+func (bw BackupWrapper) GetBackup(
+	ctx context.Context,
+	backupID model.StableID,
+) (*backup.Backup, error) {
+	return bw.Backup, bw.GetErr
+}
+
+func (bw BackupWrapper) DeleteBackup(
+	ctx context.Context,
+	backupID model.StableID,
+) error {
+	return bw.DeleteErr
+}
+
+func (bw BackupWrapper) GetBackups(
+	ctx context.Context,
+	filters ...store.FilterOption,
+) ([]*backup.Backup, error) {
+	return nil, clues.New("GetBackups mock not implemented yet")
+}