Remove old code for finding bases (#3526)

Now that we're finding bases with a different component
remove the code that used to find bases since it's no
longer in use

---

#### 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)

* #3202

#### Test Plan

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

src/internal/kopia/base_finder.go

@@ -13,8 +13,40 @@ import (
 	"github.com/alcionai/corso/src/internal/operations/inject"
 	"github.com/alcionai/corso/src/pkg/backup"
 	"github.com/alcionai/corso/src/pkg/logger"
+	"github.com/alcionai/corso/src/pkg/path"
 )
 
+const (
+	// Kopia does not do comparisons properly for empty tags right now so add some
+	// placeholder value to them.
+	defaultTagValue = "0"
+
+	// Kopia CLI prefixes all user tags with "tag:"[1]. Maintaining this will
+	// ensure we don't accidentally take reserved tags and that tags can be
+	// displayed with kopia CLI.
+	// (permalinks)
+	// [1] https://github.com/kopia/kopia/blob/05e729a7858a6e86cb48ba29fb53cb6045efce2b/cli/command_snapshot_create.go#L169
+	userTagPrefix = "tag:"
+)
+
+type Reason struct {
+	ResourceOwner string
+	Service       path.ServiceType
+	Category      path.CategoryType
+}
+
+func (r Reason) TagKeys() []string {
+	return []string{
+		r.ResourceOwner,
+		serviceCatString(r.Service, r.Category),
+	}
+}
+
+// Key is the concatenation of the ResourceOwner, Service, and Category.
+func (r Reason) Key() string {
+	return r.ResourceOwner + r.Service.String() + r.Category.String()
+}
+
 type backupBases struct {
 	backups     []BackupEntry
 	mergeBases  []ManifestEntry
@@ -26,6 +58,63 @@ type BackupEntry struct {
 	Reasons []Reason
 }
 
+type ManifestEntry struct {
+	*snapshot.Manifest
+	// Reason contains the ResourceOwners and Service/Categories that caused this
+	// snapshot to be selected as a base. We can't reuse OwnersCats here because
+	// it's possible some ResourceOwners will have a subset of the Categories as
+	// the reason for selecting a snapshot. For example:
+	// 1. backup user1 email,contacts -> B1
+	// 2. backup user1 contacts -> B2 (uses B1 as base)
+	// 3. backup user1 email,contacts,events (uses B1 for email, B2 for contacts)
+	Reasons []Reason
+}
+
+func (me ManifestEntry) GetTag(key string) (string, bool) {
+	k, _ := makeTagKV(key)
+	v, ok := me.Tags[k]
+
+	return v, ok
+}
+
+type snapshotManager interface {
+	FindManifests(
+		ctx context.Context,
+		tags map[string]string,
+	) ([]*manifest.EntryMetadata, error)
+	LoadSnapshot(ctx context.Context, id manifest.ID) (*snapshot.Manifest, error)
+}
+
+func serviceCatString(s path.ServiceType, c path.CategoryType) string {
+	return s.String() + c.String()
+}
+
+// MakeTagKV normalizes the provided key to protect it from clobbering
+// similarly named tags from non-user input (user inputs are still open
+// to collisions amongst eachother).
+// Returns the normalized Key plus a default value.  If you're embedding a
+// key-only tag, the returned default value msut be used instead of an
+// empty string.
+func makeTagKV(k string) (string, string) {
+	return userTagPrefix + k, defaultTagValue
+}
+
+func normalizeTagKVs(tags map[string]string) map[string]string {
+	t2 := make(map[string]string, len(tags))
+
+	for k, v := range tags {
+		mk, mv := makeTagKV(k)
+
+		if len(v) == 0 {
+			v = mv
+		}
+
+		t2[mk] = v
+	}
+
+	return t2
+}
+
 type baseFinder struct {
 	sm snapshotManager
 	bg inject.GetBackuper

src/internal/kopia/base_finder_test.go

@@ -27,11 +27,9 @@ const (
 var (
 	testT1 = time.Now()
 	testT2 = testT1.Add(1 * time.Hour)
-	testT3 = testT2.Add(1 * time.Hour)
 
 	testID1 = manifest.ID("snap1")
 	testID2 = manifest.ID("snap2")
-	testID3 = manifest.ID("snap3")
 
 	testBackup1 = "backupID1"
 	testBackup2 = "backupID2"

src/internal/kopia/conn.go

@@ -418,18 +418,6 @@ func (w *conn) LoadSnapshot(
 	return man, nil
 }
 
-func (w *conn) LoadSnapshots(
-	ctx context.Context,
-	ids []manifest.ID,
-) ([]*snapshot.Manifest, error) {
-	mans, err := snapshot.LoadSnapshots(ctx, w.Repository, ids)
-	if err != nil {
-		return nil, clues.Stack(err).WithClues(ctx)
-	}
-
-	return mans, nil
-}
-
 func (w *conn) SnapshotRoot(man *snapshot.Manifest) (fs.Entry, error) {
 	return snapshotfs.SnapshotRoot(w.Repository, man)
 }

src/internal/kopia/snapshot_manager.go

@@ -1,307 +0,0 @@
-package kopia
-
-import (
-	"context"
-	"sort"
-
-	"github.com/alcionai/clues"
-	"github.com/kopia/kopia/repo/manifest"
-	"github.com/kopia/kopia/snapshot"
-	"golang.org/x/exp/maps"
-
-	"github.com/alcionai/corso/src/pkg/logger"
-	"github.com/alcionai/corso/src/pkg/path"
-)
-
-const (
-	// Kopia does not do comparisons properly for empty tags right now so add some
-	// placeholder value to them.
-	defaultTagValue = "0"
-
-	// Kopia CLI prefixes all user tags with "tag:"[1]. Maintaining this will
-	// ensure we don't accidentally take reserved tags and that tags can be
-	// displayed with kopia CLI.
-	// (permalinks)
-	// [1] https://github.com/kopia/kopia/blob/05e729a7858a6e86cb48ba29fb53cb6045efce2b/cli/command_snapshot_create.go#L169
-	userTagPrefix = "tag:"
-)
-
-type Reason struct {
-	ResourceOwner string
-	Service       path.ServiceType
-	Category      path.CategoryType
-}
-
-func (r Reason) TagKeys() []string {
-	return []string{
-		r.ResourceOwner,
-		serviceCatString(r.Service, r.Category),
-	}
-}
-
-// Key is the concatenation of the ResourceOwner, Service, and Category.
-func (r Reason) Key() string {
-	return r.ResourceOwner + r.Service.String() + r.Category.String()
-}
-
-type ManifestEntry struct {
-	*snapshot.Manifest
-	// Reason contains the ResourceOwners and Service/Categories that caused this
-	// snapshot to be selected as a base. We can't reuse OwnersCats here because
-	// it's possible some ResourceOwners will have a subset of the Categories as
-	// the reason for selecting a snapshot. For example:
-	// 1. backup user1 email,contacts -> B1
-	// 2. backup user1 contacts -> B2 (uses B1 as base)
-	// 3. backup user1 email,contacts,events (uses B1 for email, B2 for contacts)
-	Reasons []Reason
-}
-
-func (me ManifestEntry) GetTag(key string) (string, bool) {
-	k, _ := makeTagKV(key)
-	v, ok := me.Tags[k]
-
-	return v, ok
-}
-
-type snapshotManager interface {
-	FindManifests(
-		ctx context.Context,
-		tags map[string]string,
-	) ([]*manifest.EntryMetadata, error)
-	LoadSnapshot(ctx context.Context, id manifest.ID) (*snapshot.Manifest, error)
-	// TODO(ashmrtn): Remove this when we switch to the new BaseFinder.
-	LoadSnapshots(ctx context.Context, ids []manifest.ID) ([]*snapshot.Manifest, error)
-}
-
-func serviceCatString(s path.ServiceType, c path.CategoryType) string {
-	return s.String() + c.String()
-}
-
-// MakeTagKV normalizes the provided key to protect it from clobbering
-// similarly named tags from non-user input (user inputs are still open
-// to collisions amongst eachother).
-// Returns the normalized Key plus a default value.  If you're embedding a
-// key-only tag, the returned default value msut be used instead of an
-// empty string.
-func makeTagKV(k string) (string, string) {
-	return userTagPrefix + k, defaultTagValue
-}
-
-// getLastIdx searches for manifests contained in both foundMans and metas
-// and returns the most recent complete manifest index and the manifest it
-// corresponds to. If no complete manifest is in both lists returns nil, -1.
-func getLastIdx(
-	foundMans map[manifest.ID]*ManifestEntry,
-	metas []*manifest.EntryMetadata,
-) (*ManifestEntry, int) {
-	// Minor optimization: the current code seems to return the entries from
-	// earliest timestamp to latest (this is undocumented). Sort in the same
-	// fashion so that we don't incur a bunch of swaps.
-	sort.Slice(metas, func(i, j int) bool {
-		return metas[i].ModTime.Before(metas[j].ModTime)
-	})
-
-	// Search newest to oldest.
-	for i := len(metas) - 1; i >= 0; i-- {
-		m := foundMans[metas[i].ID]
-		if m == nil || len(m.IncompleteReason) > 0 {
-			continue
-		}
-
-		return m, i
-	}
-
-	return nil, -1
-}
-
-// manifestsSinceLastComplete searches through mans and returns the most recent
-// complete manifest (if one exists), maybe the most recent incomplete
-// manifest, and a bool denoting if a complete manifest was found. If the newest
-// incomplete manifest is more recent than the newest complete manifest then
-// adds it to the returned list. Otherwise no incomplete manifest is returned.
-// Returns nil if there are no complete or incomplete manifests in mans.
-func manifestsSinceLastComplete(
-	ctx context.Context,
-	mans []*snapshot.Manifest,
-) ([]*snapshot.Manifest, bool) {
-	var (
-		res             []*snapshot.Manifest
-		foundIncomplete bool
-		foundComplete   bool
-	)
-
-	// Manifests should maintain the sort order of the original IDs that were used
-	// to fetch the data, but just in case sort oldest to newest.
-	mans = snapshot.SortByTime(mans, false)
-
-	for i := len(mans) - 1; i >= 0; i-- {
-		m := mans[i]
-
-		if len(m.IncompleteReason) > 0 {
-			if !foundIncomplete {
-				res = append(res, m)
-				foundIncomplete = true
-
-				logger.Ctx(ctx).Infow("found incomplete snapshot", "snapshot_id", m.ID)
-			}
-
-			continue
-		}
-
-		// Once we find a complete snapshot we're done, even if we haven't
-		// found an incomplete one yet.
-		res = append(res, m)
-		foundComplete = true
-
-		logger.Ctx(ctx).Infow("found complete snapshot", "snapshot_id", m.ID)
-
-		break
-	}
-
-	return res, foundComplete
-}
-
-// fetchPrevManifests returns the most recent, as-of-yet unfound complete and
-// (maybe) incomplete manifests in metas. If the most recent incomplete manifest
-// is older than the most recent complete manifest no incomplete manifest is
-// returned. If only incomplete manifests exists, returns the most recent one.
-// Returns no manifests if an error occurs.
-func fetchPrevManifests(
-	ctx context.Context,
-	sm snapshotManager,
-	foundMans map[manifest.ID]*ManifestEntry,
-	reason Reason,
-	tags map[string]string,
-) ([]*snapshot.Manifest, error) {
-	allTags := map[string]string{}
-
-	for _, k := range reason.TagKeys() {
-		allTags[k] = ""
-	}
-
-	maps.Copy(allTags, tags)
-	allTags = normalizeTagKVs(allTags)
-
-	metas, err := sm.FindManifests(ctx, allTags)
-	if err != nil {
-		return nil, clues.Wrap(err, "fetching manifest metas by tag")
-	}
-
-	if len(metas) == 0 {
-		return nil, nil
-	}
-
-	man, lastCompleteIdx := getLastIdx(foundMans, metas)
-
-	// We have a complete cached snapshot and it's the most recent. No need
-	// to do anything else.
-	if lastCompleteIdx == len(metas)-1 {
-		return []*snapshot.Manifest{man.Manifest}, nil
-	}
-
-	// TODO(ashmrtn): Remainder of the function can be simplified if we can inject
-	// different tags to the snapshot checkpoints than the complete snapshot.
-
-	// Fetch all manifests newer than the oldest complete snapshot. A little
-	// wasteful as we may also re-fetch the most recent incomplete manifest, but
-	// it reduces the complexity of returning the most recent incomplete manifest
-	// if it is newer than the most recent complete manifest.
-	ids := make([]manifest.ID, 0, len(metas)-(lastCompleteIdx+1))
-	for i := lastCompleteIdx + 1; i < len(metas); i++ {
-		ids = append(ids, metas[i].ID)
-	}
-
-	mans, err := sm.LoadSnapshots(ctx, ids)
-	if err != nil {
-		return nil, clues.Wrap(err, "fetching previous manifests")
-	}
-
-	found, hasCompleted := manifestsSinceLastComplete(ctx, mans)
-
-	// If we didn't find another complete manifest then we need to mark the
-	// previous complete manifest as having this ResourceOwner, Service, Category
-	// as the reason as well.
-	if !hasCompleted && man != nil {
-		found = append(found, man.Manifest)
-		logger.Ctx(ctx).Infow(
-			"reusing cached complete snapshot",
-			"snapshot_id", man.ID)
-	}
-
-	return found, nil
-}
-
-// fetchPrevSnapshotManifests returns a set of manifests for complete and maybe
-// incomplete snapshots for the given (resource owner, service, category)
-// tuples. Up to two manifests can be returned per tuple: one complete and one
-// incomplete. An incomplete manifest may be returned if it is newer than the
-// newest complete manifest for the tuple. Manifests are deduped such that if
-// multiple tuples match the same manifest it will only be returned once.
-// External callers can access this via wrapper.FetchPrevSnapshotManifests().
-// If tags are provided, manifests must include a superset of the k:v pairs
-// specified by those tags.  Tags should pass their raw values, and will be
-// normalized inside the func using MakeTagKV.
-func fetchPrevSnapshotManifests(
-	ctx context.Context,
-	sm snapshotManager,
-	reasons []Reason,
-	tags map[string]string,
-) []*ManifestEntry {
-	mans := map[manifest.ID]*ManifestEntry{}
-
-	// For each serviceCat/resource owner pair that we will be backing up, see if
-	// there's a previous incomplete snapshot and/or a previous complete snapshot
-	// we can pass in. Can be expanded to return more than the most recent
-	// snapshots, but may require more memory at runtime.
-	for _, reason := range reasons {
-		ictx := clues.Add(ctx, "service", reason.Service.String(), "category", reason.Category.String())
-		logger.Ctx(ictx).Info("searching for previous manifests for reason")
-
-		found, err := fetchPrevManifests(ictx, sm, mans, reason, tags)
-		if err != nil {
-			logger.CtxErr(ictx, err).Info("fetching previous snapshot manifests for service/category/resource owner")
-
-			// Snapshot can still complete fine, just not as efficient.
-			continue
-		}
-
-		// If we found more recent snapshots then add them.
-		for _, m := range found {
-			man := mans[m.ID]
-			if man == nil {
-				mans[m.ID] = &ManifestEntry{
-					Manifest: m,
-					Reasons:  []Reason{reason},
-				}
-
-				continue
-			}
-
-			// This manifest has multiple reasons for being chosen. Merge them here.
-			man.Reasons = append(man.Reasons, reason)
-		}
-	}
-
-	res := make([]*ManifestEntry, 0, len(mans))
-	for _, m := range mans {
-		res = append(res, m)
-	}
-
-	return res
-}
-
-func normalizeTagKVs(tags map[string]string) map[string]string {
-	t2 := make(map[string]string, len(tags))
-
-	for k, v := range tags {
-		mk, mv := makeTagKV(k)
-
-		if len(v) == 0 {
-			v = mv
-		}
-
-		t2[mk] = v
-	}
-
-	return t2
-}

src/internal/kopia/snapshot_manager_test.go

@@ -1,932 +0,0 @@
-package kopia
-
-import (
-	"context"
-	"testing"
-	"time"
-
-	"github.com/alcionai/clues"
-	"github.com/kopia/kopia/fs"
-	"github.com/kopia/kopia/repo/manifest"
-	"github.com/kopia/kopia/snapshot"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/suite"
-
-	"github.com/alcionai/corso/src/internal/tester"
-	"github.com/alcionai/corso/src/pkg/path"
-)
-
-func newManifestInfo(
-	id manifest.ID,
-	modTime time.Time,
-	incomplete bool,
-	tags ...string,
-) manifestInfo {
-	incompleteStr := ""
-	if incomplete {
-		incompleteStr = "checkpoint"
-	}
-
-	structTags := make(map[string]string, len(tags))
-
-	for _, t := range tags {
-		tk, _ := makeTagKV(t)
-		structTags[tk] = ""
-	}
-
-	return manifestInfo{
-		tags: structTags,
-		metadata: &manifest.EntryMetadata{
-			ID:      id,
-			ModTime: modTime,
-		},
-		man: &snapshot.Manifest{
-			ID:               id,
-			StartTime:        fs.UTCTimestamp(modTime.UnixNano()),
-			IncompleteReason: incompleteStr,
-		},
-	}
-}
-
-type mockSnapshotManager struct {
-	data         []manifestInfo
-	loadCallback func(ids []manifest.ID)
-}
-
-func matchesTags(mi manifestInfo, tags map[string]string) bool {
-	for k := range tags {
-		if _, ok := mi.tags[k]; !ok {
-			return false
-		}
-	}
-
-	return true
-}
-
-func (msm *mockSnapshotManager) FindManifests(
-	ctx context.Context,
-	tags map[string]string,
-) ([]*manifest.EntryMetadata, error) {
-	if msm == nil {
-		return nil, assert.AnError
-	}
-
-	res := []*manifest.EntryMetadata{}
-
-	for _, mi := range msm.data {
-		if matchesTags(mi, tags) {
-			res = append(res, mi.metadata)
-		}
-	}
-
-	return res, nil
-}
-
-func (msm *mockSnapshotManager) LoadSnapshots(
-	ctx context.Context,
-	ids []manifest.ID,
-) ([]*snapshot.Manifest, error) {
-	if msm == nil {
-		return nil, assert.AnError
-	}
-
-	// Allow checking set of IDs passed in.
-	if msm.loadCallback != nil {
-		msm.loadCallback(ids)
-	}
-
-	res := []*snapshot.Manifest{}
-
-	for _, id := range ids {
-		for _, mi := range msm.data {
-			if mi.man.ID == id {
-				res = append(res, mi.man)
-			}
-		}
-	}
-
-	return res, nil
-}
-
-func (msm *mockSnapshotManager) LoadSnapshot(
-	ctx context.Context,
-	id manifest.ID,
-) (*snapshot.Manifest, error) {
-	return nil, clues.New("not implemented")
-}
-
-type SnapshotFetchUnitSuite struct {
-	tester.Suite
-}
-
-func TestSnapshotFetchUnitSuite(t *testing.T) {
-	suite.Run(t, &SnapshotFetchUnitSuite{Suite: tester.NewUnitSuite(t)})
-}
-
-func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshots() {
-	table := []struct {
-		name  string
-		input []Reason
-		data  []manifestInfo
-		// Use this to denote which manifests in data should be expected. Allows
-		// defining data in a table while not repeating things between data and
-		// expected.
-		expectedIdxs []int
-		// Use this to denote the Reasons a manifest is selected. The int maps to
-		// the index of the manifest in data.
-		expectedReasons map[int][]Reason
-		// Expected number of times a manifest should try to be loaded from kopia.
-		// Used to check that caching is functioning properly.
-		expectedLoadCounts map[manifest.ID]int
-	}{
-		{
-			name:  "AllOneSnapshot",
-			input: testAllUsersAllCats,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testCompleteMan,
-					testMail,
-					testEvents,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{0},
-			expectedReasons: map[int][]Reason{
-				0: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 1,
-			},
-		},
-		{
-			name:  "SplitByCategory",
-			input: testAllUsersAllCats,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testCompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testCompleteMan,
-					testEvents,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{0, 1},
-			expectedReasons: map[int][]Reason{
-				0: {
-					{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-				1: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 1,
-				testID2: 1,
-			},
-		},
-		{
-			name:  "IncompleteNewerThanComplete",
-			input: testAllUsersMail,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testCompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testIncompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{0, 1},
-			expectedReasons: map[int][]Reason{
-				0: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-				1: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 1,
-				testID2: 3,
-			},
-		},
-		{
-			name:  "IncompleteOlderThanComplete",
-			input: testAllUsersMail,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testIncompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testCompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{1},
-			expectedReasons: map[int][]Reason{
-				1: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 1,
-				testID2: 1,
-			},
-		},
-		{
-			name:  "OnlyIncomplete",
-			input: testAllUsersMail,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testIncompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{0},
-			expectedReasons: map[int][]Reason{
-				0: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 3,
-			},
-		},
-		{
-			name:  "NewestComplete",
-			input: testAllUsersMail,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testCompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testCompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{1},
-			expectedReasons: map[int][]Reason{
-				1: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 1,
-				testID2: 1,
-			},
-		},
-		{
-			name:  "NewestIncomplete",
-			input: testAllUsersMail,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testIncompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testIncompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{1},
-			expectedReasons: map[int][]Reason{
-				1: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 3,
-				testID2: 3,
-			},
-		},
-		{
-			name:  "SomeCachedSomeNewer",
-			input: testAllUsersMail,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testCompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testCompleteMan,
-					testMail,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{0, 1},
-			expectedReasons: map[int][]Reason{
-				0: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-				1: {
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 2,
-				testID2: 1,
-			},
-		},
-		{
-			name:  "SomeCachedSomeNewerDifferentCategories",
-			input: testAllUsersAllCats,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testCompleteMan,
-					testMail,
-					testEvents,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testCompleteMan,
-					testMail,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{0, 1},
-			expectedReasons: map[int][]Reason{
-				0: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EventsCategory,
-					},
-				},
-				1: {
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 2,
-				testID2: 1,
-			},
-		},
-		{
-			name:  "SomeCachedSomeNewerIncomplete",
-			input: testAllUsersMail,
-			data: []manifestInfo{
-				newManifestInfo(
-					testID1,
-					testT1,
-					testCompleteMan,
-					testMail,
-					testUser1,
-					testUser2,
-					testUser3,
-				),
-				newManifestInfo(
-					testID2,
-					testT2,
-					testIncompleteMan,
-					testMail,
-					testUser3,
-				),
-			},
-			expectedIdxs: []int{0, 1},
-			expectedReasons: map[int][]Reason{
-				0: {
-					Reason{
-						ResourceOwner: testUser1,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser2,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-				1: {
-					Reason{
-						ResourceOwner: testUser3,
-						Service:       path.ExchangeService,
-						Category:      path.EmailCategory,
-					},
-				},
-			},
-			expectedLoadCounts: map[manifest.ID]int{
-				testID1: 1,
-				testID2: 1,
-			},
-		},
-		{
-			name:         "NoMatches",
-			input:        testAllUsersMail,
-			data:         nil,
-			expectedIdxs: nil,
-			// Stop failure for nil-map comparison.
-			expectedLoadCounts: map[manifest.ID]int{},
-		},
-	}
-
-	for _, test := range table {
-		suite.Run(test.name, func() {
-			t := suite.T()
-
-			ctx, flush := tester.NewContext(t)
-			defer flush()
-
-			msm := &mockSnapshotManager{
-				data: test.data,
-			}
-
-			loadCounts := map[manifest.ID]int{}
-			msm.loadCallback = func(ids []manifest.ID) {
-				for _, id := range ids {
-					loadCounts[id]++
-				}
-			}
-
-			snaps := fetchPrevSnapshotManifests(ctx, msm, test.input, nil)
-
-			// Check the proper snapshot manifests were returned.
-			expected := make([]*snapshot.Manifest, 0, len(test.expectedIdxs))
-			for _, i := range test.expectedIdxs {
-				expected = append(expected, test.data[i].man)
-			}
-
-			got := make([]*snapshot.Manifest, 0, len(snaps))
-			for _, s := range snaps {
-				got = append(got, s.Manifest)
-			}
-
-			assert.ElementsMatch(t, expected, got)
-
-			// Check the reasons for selecting each manifest are correct.
-			expectedReasons := make(map[manifest.ID][]Reason, len(test.expectedReasons))
-			for idx, reason := range test.expectedReasons {
-				expectedReasons[test.data[idx].man.ID] = reason
-			}
-
-			for _, found := range snaps {
-				reason, ok := expectedReasons[found.ID]
-				if !ok {
-					// Missing or extra snapshots will be reported by earlier checks.
-					continue
-				}
-
-				assert.ElementsMatch(
-					t,
-					reason,
-					found.Reasons,
-					"incorrect reasons for snapshot with ID %s",
-					found.ID,
-				)
-			}
-
-			// Check number of loads to make sure caching is working properly.
-			// Need to manually check because we don't know the order the
-			// user/service/category labels will be iterated over. For some tests this
-			// could cause more loads than the ideal case.
-			assert.Len(t, loadCounts, len(test.expectedLoadCounts))
-			for id, count := range loadCounts {
-				assert.GreaterOrEqual(t, test.expectedLoadCounts[id], count)
-			}
-		})
-	}
-}
-
-func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshots_customTags() {
-	data := []manifestInfo{
-		newManifestInfo(
-			testID1,
-			testT1,
-			false,
-			testMail,
-			testUser1,
-			"fnords",
-			"smarf",
-		),
-	}
-	expectLoad1T1 := map[manifest.ID]int{
-		testID1: 1,
-	}
-
-	table := []struct {
-		name  string
-		input []Reason
-		tags  map[string]string
-		// Use this to denote which manifests in data should be expected. Allows
-		// defining data in a table while not repeating things between data and
-		// expected.
-		expectedIdxs []int
-		// Expected number of times a manifest should try to be loaded from kopia.
-		// Used to check that caching is functioning properly.
-		expectedLoadCounts map[manifest.ID]int
-	}{
-		{
-			name:               "no tags specified",
-			tags:               nil,
-			expectedIdxs:       []int{0},
-			expectedLoadCounts: expectLoad1T1,
-		},
-		{
-			name: "all custom tags",
-			tags: map[string]string{
-				"fnords": "",
-				"smarf":  "",
-			},
-			expectedIdxs:       []int{0},
-			expectedLoadCounts: expectLoad1T1,
-		},
-		{
-			name:               "subset of custom tags",
-			tags:               map[string]string{"fnords": ""},
-			expectedIdxs:       []int{0},
-			expectedLoadCounts: expectLoad1T1,
-		},
-		{
-			name:               "custom tag mismatch",
-			tags:               map[string]string{"bojangles": ""},
-			expectedIdxs:       nil,
-			expectedLoadCounts: nil,
-		},
-	}
-
-	for _, test := range table {
-		suite.Run(test.name, func() {
-			t := suite.T()
-
-			ctx, flush := tester.NewContext(t)
-			defer flush()
-
-			msm := &mockSnapshotManager{
-				data: data,
-			}
-
-			loadCounts := map[manifest.ID]int{}
-			msm.loadCallback = func(ids []manifest.ID) {
-				for _, id := range ids {
-					loadCounts[id]++
-				}
-			}
-
-			snaps := fetchPrevSnapshotManifests(ctx, msm, testAllUsersAllCats, test.tags)
-
-			expected := make([]*snapshot.Manifest, 0, len(test.expectedIdxs))
-			for _, i := range test.expectedIdxs {
-				expected = append(expected, data[i].man)
-			}
-
-			got := make([]*snapshot.Manifest, 0, len(snaps))
-			for _, s := range snaps {
-				got = append(got, s.Manifest)
-			}
-
-			assert.ElementsMatch(t, expected, got)
-
-			// Need to manually check because we don't know the order the
-			// user/service/category labels will be iterated over. For some tests this
-			// could cause more loads than the ideal case.
-			assert.Len(t, loadCounts, len(test.expectedLoadCounts))
-			for id, count := range loadCounts {
-				assert.GreaterOrEqual(t, test.expectedLoadCounts[id], count)
-			}
-		})
-	}
-}
-
-// mockErrorSnapshotManager returns an error the first time LoadSnapshot and
-// FindSnapshot are called. After that it passes the calls through to the
-// contained snapshotManager.
-type mockErrorSnapshotManager struct {
-	retFindErr bool
-	retLoadErr bool
-	sm         snapshotManager
-}
-
-func (msm *mockErrorSnapshotManager) FindManifests(
-	ctx context.Context,
-	tags map[string]string,
-) ([]*manifest.EntryMetadata, error) {
-	if !msm.retFindErr {
-		msm.retFindErr = true
-		return nil, assert.AnError
-	}
-
-	return msm.sm.FindManifests(ctx, tags)
-}
-
-func (msm *mockErrorSnapshotManager) LoadSnapshots(
-	ctx context.Context,
-	ids []manifest.ID,
-) ([]*snapshot.Manifest, error) {
-	if !msm.retLoadErr {
-		msm.retLoadErr = true
-		return nil, assert.AnError
-	}
-
-	return msm.sm.LoadSnapshots(ctx, ids)
-}
-
-func (msm *mockErrorSnapshotManager) LoadSnapshot(
-	ctx context.Context,
-	id manifest.ID,
-) (*snapshot.Manifest, error) {
-	return nil, clues.New("not implemented")
-}
-
-func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshots_withErrors() {
-	t := suite.T()
-
-	ctx, flush := tester.NewContext(t)
-	defer flush()
-
-	input := testAllUsersMail
-	mockData := []manifestInfo{
-		newManifestInfo(
-			testID1,
-			testT1,
-			testCompleteMan,
-			testMail,
-			testUser1,
-		),
-		newManifestInfo(
-			testID2,
-			testT2,
-			testCompleteMan,
-			testMail,
-			testUser2,
-		),
-		newManifestInfo(
-			testID3,
-			testT3,
-			testCompleteMan,
-			testMail,
-			testUser3,
-		),
-	}
-
-	msm := &mockErrorSnapshotManager{
-		sm: &mockSnapshotManager{
-			data: mockData,
-		},
-	}
-
-	snaps := fetchPrevSnapshotManifests(ctx, msm, input, nil)
-
-	// Only 1 snapshot should be chosen because the other two attempts fail.
-	// However, which one is returned is non-deterministic because maps are used.
-	assert.Len(t, snaps, 1)
-}

src/internal/kopia/wrapper.go

@@ -596,27 +596,6 @@ func (w Wrapper) DeleteSnapshot(
 	return nil
 }
 
-// FetchPrevSnapshotManifests returns a set of manifests for complete and maybe
-// incomplete snapshots for the given (resource owner, service, category)
-// tuples. Up to two manifests can be returned per tuple: one complete and one
-// incomplete. An incomplete manifest may be returned if it is newer than the
-// newest complete manifest for the tuple. Manifests are deduped such that if
-// multiple tuples match the same manifest it will only be returned once.
-// If tags are provided, manifests must include a superset of the k:v pairs
-// specified by those tags.  Tags should pass their raw values, and will be
-// normalized inside the func using MakeTagKV.
-func (w Wrapper) FetchPrevSnapshotManifests(
-	ctx context.Context,
-	reasons []Reason,
-	tags map[string]string,
-) ([]*ManifestEntry, error) {
-	if w.c == nil {
-		return nil, clues.Stack(errNotConnected).WithClues(ctx)
-	}
-
-	return fetchPrevSnapshotManifests(ctx, w.c, reasons, tags), nil
-}
-
 func (w Wrapper) NewBaseFinder(bg inject.GetBackuper) (*baseFinder, error) {
 	return newBaseFinder(w.c, bg)
 }

src/internal/kopia/wrapper_test.go

@@ -1101,14 +1101,8 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupExcludeItem() {
 
 	subtreePath := subtreePathTmp.ToBuilder().Dir()
 
-	manifests, err := suite.w.FetchPrevSnapshotManifests(
+	man, err := suite.w.c.LoadSnapshot(suite.ctx, suite.snapshotID)
-		suite.ctx,
+	require.NoError(suite.T(), err, "getting base snapshot: %v", clues.ToCore(err))
-		[]Reason{reason},
-		nil,
-	)
-	require.NoError(suite.T(), err, clues.ToCore(err))
-	require.Len(suite.T(), manifests, 1)
-	require.Equal(suite.T(), suite.snapshotID, manifests[0].ID)
 
 	tags := map[string]string{}
 
@@ -1206,7 +1200,7 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupExcludeItem() {
 				suite.ctx,
 				[]IncrementalBase{
 					{
-						Manifest: manifests[0].Manifest,
+						Manifest: man,
 						SubtreePaths: []*path.Builder{
 							subtreePath,
 						},

src/internal/operations/backup_integration_test.go

@@ -226,7 +226,10 @@ func checkBackupIsInManifests(
 				found bool
 			)
 
-			mans, err := kw.FetchPrevSnapshotManifests(ctx, reasons, tags)
+			bf, err := kw.NewBaseFinder(bo.store)
+			require.NoError(t, err, clues.ToCore(err))
+
+			mans, err := bf.FindBases(ctx, reasons, tags)
 			require.NoError(t, err, clues.ToCore(err))
 
 			for _, man := range mans {