larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix circular dependency for store interface (#4014)

Browse Source 
Remove references to the kopia package from
`pkg/store` package so that kopia can import
that package itself. Do this by using
interfaces where needed in `pkg/store`
instead of concrete struct types

These changes will make cleaning up
incomplete backups a little neater since
that code will need to lookup both
manifests and backup models

This PR is just minor renaming and fixups,
no logic changes

---

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

- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ No

#### Type of change

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #3217

#### Test Plan

- [ ] 💪 Manual
- [x] ⚡ Unit test
- [x] 💚 E2E
This commit is contained in:
ashmrtn 2023-08-11 09:04:05 -07:00 committed by GitHub
parent 50e84b28a4
commit 893598d8ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 81 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/internal/kopia/base_finder.go
View File

@ -10,11 +10,11 @@ import (
"golang.org/x/exp/maps"
"github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/internal/operations/inject"
"github.com/alcionai/corso/src/pkg/backup"
"github.com/alcionai/corso/src/pkg/backup/identity"
"github.com/alcionai/corso/src/pkg/logger"
"github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/store"
)
const (
@ -155,19 +155,19 @@ func normalizeTagKVs(tags map[string]string) map[string]string {
type baseFinder struct {
sm snapshotManager
bg inject.GetBackuper
bg store.BackupGetter
}
func newBaseFinder(
sm snapshotManager,
bg inject.GetBackuper,
bg store.BackupGetter,
) (*baseFinder, error) {
if sm == nil {
return nil, clues.New("nil snapshotManager")
}
if bg == nil {
return nil, clues.New("nil GetBackuper")
return nil, clues.New("nil BackupGetter")
}
return &baseFinder{

3
src/internal/kopia/model_store.go
View File

@ -13,8 +13,11 @@ import (
"github.com/alcionai/corso/src/internal/data"
"github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/pkg/store"
)
var _ store.Storer = &ModelStore{}
const (
stableIDKey = "stableID"
modelVersionKey = "storeVersion"

4
src/internal/kopia/wrapper.go
View File

@ -20,7 +20,6 @@ import (
"github.com/alcionai/corso/src/internal/data"
"github.com/alcionai/corso/src/internal/diagnostics"
"github.com/alcionai/corso/src/internal/observe"
"github.com/alcionai/corso/src/internal/operations/inject"
"github.com/alcionai/corso/src/internal/stats"
"github.com/alcionai/corso/src/pkg/backup/details"
"github.com/alcionai/corso/src/pkg/backup/identity"
@ -28,6 +27,7 @@ import (
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/logger"
"github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/store"
)
const (
@ -590,7 +590,7 @@ func (w Wrapper) DeleteSnapshot(
return nil
}
func (w Wrapper) NewBaseFinder(bg inject.GetBackuper) (*baseFinder, error) {
func (w Wrapper) NewBaseFinder(bg store.BackupGetter) (*baseFinder, error) {
return newBaseFinder(w.c, bg)
}

2
src/internal/operations/backup.go
View File

@ -75,7 +75,7 @@ func NewBackupOperation(
ctx context.Context,
opts control.Options,
kw *kopia.Wrapper,
sw *store.Wrapper,
sw store.BackupStorer,
bp inject.BackupProducer,
acct account.Account,
selector selectors.Selector,

10
src/internal/operations/backup_test.go
View File

@ -364,7 +364,7 @@ func TestBackupOpUnitSuite(t *testing.T) {
func (suite *BackupOpUnitSuite) TestBackupOperation_PersistResults() {
var (
kw = &kopia.Wrapper{}
sw = &store.Wrapper{}
sw = store.NewWrapper(&kopia.ModelStore{})
ctrl = &mock.Controller{}
acct = account.Account{}
now = time.Now()
@ -1398,7 +1398,7 @@ func (suite *BackupOpIntegrationSuite) SetupSuite() {
func (suite *BackupOpIntegrationSuite) TestNewBackupOperation() {
var (
kw = &kopia.Wrapper{}
sw = &store.Wrapper{}
sw = store.NewWrapper(&kopia.ModelStore{})
ctrl = &mock.Controller{}
acct = tconfig.NewM365Account(suite.T())
opts = control.DefaultOptions()
@ -1407,7 +1407,7 @@ func (suite *BackupOpIntegrationSuite) TestNewBackupOperation() {
table := []struct {
name string
kw *kopia.Wrapper
sw *store.Wrapper
sw store.BackupStorer
bp inject.BackupProducer
acct account.Account
targets []string
@ -1447,7 +1447,7 @@ type AssistBackupIntegrationSuite struct {
kopiaCloser func(ctx context.Context)
acct account.Account
kw *kopia.Wrapper
sw *store.Wrapper
sw store.BackupStorer
ms *kopia.ModelStore
}
@ -1489,7 +1489,7 @@ func (suite *AssistBackupIntegrationSuite) SetupSuite() {
suite.ms = ms
sw := store.NewKopiaStore(ms)
sw := store.NewWrapper(ms)
suite.sw = sw
}

2
src/internal/operations/common.go
View File

@ -16,7 +16,7 @@ import (
func getBackupAndDetailsFromID(
ctx context.Context,
backupID model.StableID,
ms *store.Wrapper,
ms store.BackupStorer,
detailsStore streamstore.Reader,
errs *fault.Bus,
) (*backup.Backup, *details.Details, error) {

2
src/internal/operations/export.go
View File

@ -57,7 +57,7 @@ func NewExportOperation(
ctx context.Context,
opts control.Options,
kw *kopia.Wrapper,
sw *store.Wrapper,
sw store.BackupStorer,
ec inject.ExportConsumer,
acct account.Account,
backupID model.StableID,

2
src/internal/operations/export_test.go
View File

@ -40,7 +40,7 @@ func TestExportOpSuite(t *testing.T) {
func (suite *ExportOpSuite) TestExportOperation_PersistResults() {
var (
kw = &kopia.Wrapper{}
sw = &store.Wrapper{}
sw = store.NewWrapper(&kopia.ModelStore{})
ctrl = &mock.Controller{}
now = time.Now()
exportCfg = control.DefaultExportConfig()

9
src/internal/operations/inject/inject.go
View File

@ -6,8 +6,6 @@ import (
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/internal/common/prefixmatcher"
"github.com/alcionai/corso/src/internal/data"
"github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/pkg/backup"
"github.com/alcionai/corso/src/pkg/backup/details"
"github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/control/repository"
@ -92,11 +90,4 @@ type (
RepoMaintenancer interface {
RepoMaintenance(ctx context.Context, opts repository.Maintenance) error
}
GetBackuper interface {
GetBackup(
ctx context.Context,
backupID model.StableID,
) (*backup.Backup, error)
}
)

4
src/internal/operations/operation.go
View File

@ -57,7 +57,7 @@ type operation struct {
bus events.Eventer
kopia *kopia.Wrapper
store *store.Wrapper
store store.BackupStorer
}
func newOperation(
@ -65,7 +65,7 @@ func newOperation(
bus events.Eventer,
ctr *count.Bus,
kw *kopia.Wrapper,
sw *store.Wrapper,
sw store.BackupStorer,
) operation {
return operation{
CreatedAt: time.Now(),

4
src/internal/operations/operation_test.go
View File

@ -32,12 +32,12 @@ func (suite *OperationSuite) TestNewOperation() {
func (suite *OperationSuite) TestOperation_Validate() {
kwStub := &kopia.Wrapper{}
swStub := &store.Wrapper{}
swStub := store.NewWrapper(&kopia.ModelStore{})
table := []struct {
name string
kw *kopia.Wrapper
sw *store.Wrapper
sw store.BackupStorer
errCheck assert.ErrorAssertionFunc
}{
{"good", kwStub, swStub, assert.NoError},

2
src/internal/operations/restore.go
View File

@ -59,7 +59,7 @@ func NewRestoreOperation(
ctx context.Context,
opts control.Options,
kw *kopia.Wrapper,
sw *store.Wrapper,
sw store.BackupStorer,
rc inject.RestoreConsumer,
acct account.Account,
backupID model.StableID,

10
src/internal/operations/restore_test.go
View File

@ -50,7 +50,7 @@ func TestRestoreOpUnitSuite(t *testing.T) {
func (suite *RestoreOpUnitSuite) TestRestoreOperation_PersistResults() {
var (
kw = &kopia.Wrapper{}
sw = &store.Wrapper{}
sw = store.NewWrapper(&kopia.ModelStore{})
ctrl = &mock.Controller{}
now = time.Now()
restoreCfg = testdata.DefaultRestoreConfig("")
@ -216,7 +216,7 @@ type RestoreOpIntegrationSuite struct {
kopiaCloser func(ctx context.Context)
acct account.Account
kw *kopia.Wrapper
sw *store.Wrapper
sw store.BackupStorer
ms *kopia.ModelStore
}
@ -260,7 +260,7 @@ func (suite *RestoreOpIntegrationSuite) SetupSuite() {
suite.ms = ms
sw := store.NewKopiaStore(ms)
sw := store.NewWrapper(ms)
suite.sw = sw
}
@ -284,7 +284,7 @@ func (suite *RestoreOpIntegrationSuite) TearDownSuite() {
func (suite *RestoreOpIntegrationSuite) TestNewRestoreOperation() {
var (
kw = &kopia.Wrapper{}
sw = &store.Wrapper{}
sw = store.NewWrapper(&kopia.ModelStore{})
ctrl = &mock.Controller{}
restoreCfg = testdata.DefaultRestoreConfig("")
opts = control.DefaultOptions()
@ -293,7 +293,7 @@ func (suite *RestoreOpIntegrationSuite) TestNewRestoreOperation() {
table := []struct {
name string
kw *kopia.Wrapper
sw *store.Wrapper
sw store.BackupStorer
rc inject.RestoreConsumer
targets []string
errCheck assert.ErrorAssertionFunc

6
src/internal/operations/test/helper_test.go
View File

@ -60,7 +60,7 @@ type backupOpDependencies struct {
sel selectors.Selector
sss streamstore.Streamer
st storage.Storage
sw *store.Wrapper
sw store.BackupStorer
closer func()
}
@ -130,7 +130,7 @@ func prepNewTestBackupOp(
return operations.BackupOperation{}, nil
}
bod.sw = store.NewKopiaStore(bod.kms)
bod.sw = store.NewWrapper(bod.kms)
connectorResource := resource.Users
if sel.Service == selectors.ServiceSharePoint {
@ -235,7 +235,7 @@ func checkBackupIsInManifests(
t *testing.T,
ctx context.Context, //revive:disable-line:context-as-argument
kw *kopia.Wrapper,
sw *store.Wrapper,
sw store.BackupStorer,
bo *operations.BackupOperation,
sel selectors.Selector,
resourceOwner string,

4
src/internal/operations/test/restore_helper_test.go
View File

@ -36,7 +36,7 @@ type restoreOpDependencies struct {
sel selectors.Selector
sss streamstore.Streamer
st storage.Storage
sw *store.Wrapper
sw store.BackupStorer
closer func()
}
@ -105,7 +105,7 @@ func prepNewTestRestoreOp(
return operations.RestoreOperation{}, rod
}
rod.sw = store.NewKopiaStore(rod.kms)
rod.sw = store.NewWrapper(rod.kms)
connectorResource := resource.Users
if sel.Service == selectors.ServiceSharePoint {

18
src/pkg/repository/repository.go
View File

@ -347,7 +347,7 @@ func (r repository) NewBackupWithLookup(
ctx,
r.Opts,
r.dataLayer,
store.NewKopiaStore(r.modelStore),
store.NewWrapper(r.modelStore),
ctrl,
r.Account,
sel,
@ -371,7 +371,7 @@ func (r repository) NewExport(
ctx,
r.Opts,
r.dataLayer,
store.NewKopiaStore(r.modelStore),
store.NewWrapper(r.modelStore),
ctrl,
r.Account,
model.StableID(backupID),
@ -396,7 +396,7 @@ func (r repository) NewRestore(
ctx,
r.Opts,
r.dataLayer,
store.NewKopiaStore(r.modelStore),
store.NewWrapper(r.modelStore),
ctrl,
r.Account,
model.StableID(backupID),
@ -432,7 +432,7 @@ func (r repository) NewRetentionConfig(
// Backup retrieves a backup by id.
func (r repository) Backup(ctx context.Context, id string) (*backup.Backup, error) {
return getBackup(ctx, id, store.NewKopiaStore(r.modelStore))
return getBackup(ctx, id, store.NewWrapper(r.modelStore))
}
// getBackup handles the processing for Backup.
@ -455,7 +455,7 @@ func (r repository) Backups(ctx context.Context, ids []string) ([]*backup.Backup
var (
bups []*backup.Backup
errs = fault.New(false)
sw = store.NewKopiaStore(r.modelStore)
sw = store.NewWrapper(r.modelStore)
)
for _, id := range ids {
@ -475,7 +475,7 @@ func (r repository) Backups(ctx context.Context, ids []string) ([]*backup.Backup
// BackupsByTag lists all backups in a repository that contain all the tags
// specified.
func (r repository) BackupsByTag(ctx context.Context, fs ...store.FilterOption) ([]*backup.Backup, error) {
sw := store.NewKopiaStore(r.modelStore)
sw := store.NewWrapper(r.modelStore)
return backupsByTag(ctx, sw, fs)
}
@ -518,7 +518,7 @@ func (r repository) GetBackupDetails(
backupID,
r.Account.ID(),
r.dataLayer,
store.NewKopiaStore(r.modelStore),
store.NewWrapper(r.modelStore),
errs)
return deets, bup, errs.Fail(err)
@ -588,7 +588,7 @@ func (r repository) GetBackupErrors(
backupID,
r.Account.ID(),
r.dataLayer,
store.NewKopiaStore(r.modelStore),
store.NewWrapper(r.modelStore),
errs)
return fe, bup, errs.Fail(err)
@ -635,7 +635,7 @@ type snapshotDeleter interface {
// DeleteBackup removes the backup from both the model store and the backup storage.
func (r repository) DeleteBackup(ctx context.Context, id string) error {
return deleteBackup(ctx, id, r.dataLayer, store.NewKopiaStore(r.modelStore))
return deleteBackup(ctx, id, r.dataLayer, store.NewWrapper(r.modelStore))
}
// deleteBackup handles the processing for Backup.

6
src/pkg/repository/repository_unexported_test.go
View File

@ -434,7 +434,7 @@ type RepositoryModelIntgSuite struct {
tester.Suite
kw *kopia.Wrapper
ms *kopia.ModelStore
sw *store.Wrapper
sw store.BackupStorer
kopiaCloser func(ctx context.Context)
}
@ -476,7 +476,7 @@ func (suite *RepositoryModelIntgSuite) SetupSuite() {
suite.ms, err = kopia.NewModelStore(k)
require.NoError(t, err, clues.ToCore(err))
suite.sw = store.NewKopiaStore(suite.ms)
suite.sw = store.NewWrapper(suite.ms)
}
func (suite *RepositoryModelIntgSuite) TearDownSuite() {
@ -537,7 +537,7 @@ func writeBackup(
t *testing.T,
ctx context.Context, //revive:disable-line:context-as-argument
kw *kopia.Wrapper,
sw *store.Wrapper,
sw store.BackupStorer,
tID, snapID, backupID string,
sel selectors.Selector,
ownerID, ownerName string,

40
src/pkg/store/backup.go
View File

@ -4,6 +4,7 @@ import (
"context"
"github.com/alcionai/clues"
"github.com/kopia/kopia/repo/manifest"
"github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/pkg/backup"
@ -28,6 +29,14 @@ func (q *queryFilters) populate(qf ...FilterOption) {
}
}
// Service ensures the retrieved backups only match
// the specified service.
func Service(pst path.ServiceType) FilterOption {
return func(qf *queryFilters) {
qf.tags[model.ServiceTag] = pst.String()
}
}
type (
BackupWrapper interface {
BackupGetterDeleter
@ -49,18 +58,33 @@ type (
BackupDeleter interface {
DeleteBackup(ctx context.Context, backupID model.StableID) error
}
Storer interface {
Delete(ctx context.Context, s model.Schema, id model.StableID) error
DeleteWithModelStoreID(ctx context.Context, id manifest.ID) error
Get(ctx context.Context, s model.Schema, id model.StableID, data model.Model) error
GetIDsForType(ctx context.Context, s model.Schema, tags map[string]string) ([]*model.BaseModel, error)
GetWithModelStoreID(ctx context.Context, s model.Schema, id manifest.ID, data model.Model) error
Put(ctx context.Context, s model.Schema, m model.Model) error
Update(ctx context.Context, s model.Schema, m model.Model) error
}
BackupStorer interface {
Storer
BackupWrapper
}
)
// Service ensures the retrieved backups only match
// the specified service.
func Service(pst path.ServiceType) FilterOption {
return func(qf *queryFilters) {
qf.tags[model.ServiceTag] = pst.String()
type wrapper struct {
Storer
}
func NewWrapper(s Storer) *wrapper {
return &wrapper{Storer: s}
}
// GetBackup gets a single backup by id.
func (w Wrapper) GetBackup(ctx context.Context, backupID model.StableID) (*backup.Backup, error) {
func (w wrapper) GetBackup(ctx context.Context, backupID model.StableID) (*backup.Backup, error) {
b := backup.Backup{}
err := w.Get(ctx, model.BackupSchema, backupID, &b)
@ -72,7 +96,7 @@ func (w Wrapper) GetBackup(ctx context.Context, backupID model.StableID) (*backu
}
// GetDetailsFromBackupID retrieves all backups in the model store.
func (w Wrapper) GetBackups(
func (w wrapper) GetBackups(
ctx context.Context,
filters ...FilterOption,
) ([]*backup.Backup, error) {
@ -101,6 +125,6 @@ func (w Wrapper) GetBackups(
}
// DeleteBackup deletes the backup and its details entry from the model store.
func (w Wrapper) DeleteBackup(ctx context.Context, backupID model.StableID) error {
func (w wrapper) DeleteBackup(ctx context.Context, backupID model.StableID) error {
return w.Delete(ctx, model.BackupSchema, backupID)
}

6
src/pkg/store/backup_test.go
View File

@ -66,7 +66,7 @@ func (suite *StoreBackupUnitSuite) TestGetBackup() {
ctx, flush := tester.NewContext(t)
defer flush()
sm := &store.Wrapper{Storer: test.mock}
sm := store.NewWrapper(test.mock)
result, err := sm.GetBackup(ctx, model.StableID(uuid.NewString()))
test.expect(t, err, clues.ToCore(err))
@ -104,7 +104,7 @@ func (suite *StoreBackupUnitSuite) TestGetBackups() {
ctx, flush := tester.NewContext(t)
defer flush()
sm := &store.Wrapper{Storer: test.mock}
sm := store.NewWrapper(test.mock)
result, err := sm.GetBackups(ctx)
test.expect(t, err, clues.ToCore(err))
@ -143,7 +143,7 @@ func (suite *StoreBackupUnitSuite) TestDeleteBackup() {
ctx, flush := tester.NewContext(t)
defer flush()
sm := &store.Wrapper{Storer: test.mock}
sm := store.NewWrapper(test.mock)
err := sm.DeleteBackup(ctx, model.StableID(uuid.NewString()))
test.expect(t, err, clues.ToCore(err))

32
src/pkg/store/store.go
View File

@ -1,32 +0,0 @@
package store
import (
"context"
"github.com/kopia/kopia/repo/manifest"
"github.com/alcionai/corso/src/internal/kopia"
"github.com/alcionai/corso/src/internal/model"
)
var _ Storer = &kopia.ModelStore{}
type (
Storer interface {
Delete(ctx context.Context, s model.Schema, id model.StableID) error
DeleteWithModelStoreID(ctx context.Context, id manifest.ID) error
Get(ctx context.Context, s model.Schema, id model.StableID, data model.Model) error
GetIDsForType(ctx context.Context, s model.Schema, tags map[string]string) ([]*model.BaseModel, error)
GetWithModelStoreID(ctx context.Context, s model.Schema, id manifest.ID, data model.Model) error
Put(ctx context.Context, s model.Schema, m model.Model) error
Update(ctx context.Context, s model.Schema, m model.Model) error
}
)
type Wrapper struct {
Storer
}
func NewKopiaStore(kms *kopia.ModelStore) *Wrapper {
return &Wrapper{kms}
}