Unify backup bases fields (#4402)

Merge fields in the backup bases struct since
we assume they need to be tracked together
anyway

This PR attempts to keep the API as close to
what it currently is as possible. A future PR
will go through and update the
API/tests/mocks

---

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

* #3943

#### Test Plan

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

src/internal/kopia/backup_bases.go

@@ -5,8 +5,10 @@ import (
 
 	"github.com/alcionai/clues"
 	"github.com/kopia/kopia/repo/manifest"
+	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
 
+	"github.com/alcionai/corso/src/internal/model"
 	"github.com/alcionai/corso/src/internal/version"
 	"github.com/alcionai/corso/src/pkg/backup/identity"
 	"github.com/alcionai/corso/src/pkg/logger"
@@ -35,15 +37,16 @@ type BackupBases interface {
 	// and assist bases. If DisableAssistBases has been called then it returns
 	// nil.
 	SnapshotAssistBases() []ManifestEntry
+
+	// TODO(ashmrtn): Remove other functions and just have these once other code
+	// is updated. Here for now so changes in this file can be made.
+	NewMergeBases() []BackupBase
+	NewUniqueAssistBases() []BackupBase
 }
 
 type backupBases struct {
-	// backups and mergeBases should be modified together as they relate similar
+	mergeBases  []BackupBase
-	// data.
+	assistBases []BackupBase
-	backups       []BackupEntry
-	mergeBases    []ManifestEntry
-	assistBackups []BackupEntry
-	assistBases   []ManifestEntry
 
 	// disableAssistBases denote whether any assist bases should be returned to
 	// kopia during snapshot operation.
@@ -55,48 +58,50 @@ func (bb *backupBases) SnapshotAssistBases() []ManifestEntry {
 		return nil
 	}
 
+	res := []ManifestEntry{}
+
+	for _, ab := range bb.assistBases {
+		res = append(res, ManifestEntry{
+			Manifest: ab.ItemDataSnapshot,
+			Reasons:  ab.Reasons,
+		})
+	}
+
+	for _, mb := range bb.mergeBases {
+		res = append(res, ManifestEntry{
+			Manifest: mb.ItemDataSnapshot,
+			Reasons:  mb.Reasons,
+		})
+	}
+
 	// Need to use the actual variables here because the functions will return nil
 	// depending on what's been marked as disabled.
-	return append(slices.Clone(bb.assistBases), bb.mergeBases...)
+	return res
 }
 
 func (bb *backupBases) ConvertToAssistBase(manifestID manifest.ID) {
-	var (
-		snapshotMan ManifestEntry
-		base        BackupEntry
-		snapFound   bool
-	)
-
 	idx := slices.IndexFunc(
 		bb.mergeBases,
-		func(man ManifestEntry) bool {
+		func(base BackupBase) bool {
-			return man.ID == manifestID
+			return base.ItemDataSnapshot.ID == manifestID
 		})
 	if idx >= 0 {
-		snapFound = true
+		bb.assistBases = append(bb.assistBases, bb.mergeBases[idx])
-		snapshotMan = bb.mergeBases[idx]
 		bb.mergeBases = slices.Delete(bb.mergeBases, idx, idx+1)
 	}
-
-	idx = slices.IndexFunc(
-		bb.backups,
-		func(bup BackupEntry) bool {
-			return bup.SnapshotID == string(manifestID)
-		})
-	if idx >= 0 {
-		base = bb.backups[idx]
-		bb.backups = slices.Delete(bb.backups, idx, idx+1)
-	}
-
-	// Account for whether we found the backup.
-	if idx >= 0 && snapFound {
-		bb.assistBackups = append(bb.assistBackups, base)
-		bb.assistBases = append(bb.assistBases, snapshotMan)
-	}
 }
 
 func (bb backupBases) Backups() []BackupEntry {
-	return slices.Clone(bb.backups)
+	res := []BackupEntry{}
+
+	for _, mb := range bb.mergeBases {
+		res = append(res, BackupEntry{
+			Backup:  mb.Backup,
+			Reasons: mb.Reasons,
+		})
+	}
+
+	return res
 }
 
 func (bb backupBases) UniqueAssistBackups() []BackupEntry {
@@ -104,7 +109,16 @@ func (bb backupBases) UniqueAssistBackups() []BackupEntry {
 		return nil
 	}
 
-	return slices.Clone(bb.assistBackups)
+	res := []BackupEntry{}
+
+	for _, ab := range bb.assistBases {
+		res = append(res, BackupEntry{
+			Backup:  ab.Backup,
+			Reasons: ab.Reasons,
+		})
+	}
+
+	return res
 }
 
 func (bb *backupBases) MinBackupVersion() int {
@@ -114,9 +128,9 @@ func (bb *backupBases) MinBackupVersion() int {
 		return min
 	}
 
-	for _, bup := range bb.backups {
+	for _, base := range bb.mergeBases {
-		if min == version.NoBackup || bup.Version < min {
+		if min == version.NoBackup || base.Backup.Version < min {
-			min = bup.Version
+			min = base.Backup.Version
 		}
 	}
 
@@ -124,6 +138,19 @@ func (bb *backupBases) MinBackupVersion() int {
 }
 
 func (bb backupBases) MergeBases() []ManifestEntry {
+	res := []ManifestEntry{}
+
+	for _, mb := range bb.mergeBases {
+		res = append(res, ManifestEntry{
+			Manifest: mb.ItemDataSnapshot,
+			Reasons:  mb.Reasons,
+		})
+	}
+
+	return res
+}
+
+func (bb backupBases) NewMergeBases() []BackupBase {
 	return slices.Clone(bb.mergeBases)
 }
 
@@ -134,10 +161,8 @@ func (bb *backupBases) DisableMergeBases() {
 	// in the merge set since then we won't return the bases when merging backup
 	// details.
 	bb.assistBases = append(bb.assistBases, bb.mergeBases...)
-	bb.assistBackups = append(bb.assistBackups, bb.backups...)
 
 	bb.mergeBases = nil
-	bb.backups = nil
 }
 
 func (bb backupBases) UniqueAssistBases() []ManifestEntry {
@@ -145,6 +170,23 @@ func (bb backupBases) UniqueAssistBases() []ManifestEntry {
 		return nil
 	}
 
+	res := []ManifestEntry{}
+
+	for _, ab := range bb.assistBases {
+		res = append(res, ManifestEntry{
+			Manifest: ab.ItemDataSnapshot,
+			Reasons:  ab.Reasons,
+		})
+	}
+
+	return res
+}
+
+func (bb backupBases) NewUniqueAssistBases() []BackupBase {
+	if bb.disableAssistBases {
+		return nil
+	}
+
 	return slices.Clone(bb.assistBases)
 }
 
@@ -152,6 +194,36 @@ func (bb *backupBases) DisableAssistBases() {
 	bb.disableAssistBases = true
 }
 
+func getMissingBases(
+	reasonToKey func(identity.Reasoner) string,
+	seen map[string]struct{},
+	toCheck []BackupBase,
+) []BackupBase {
+	var res []BackupBase
+
+	for _, base := range toCheck {
+		useReasons := []identity.Reasoner{}
+
+		for _, r := range base.Reasons {
+			k := reasonToKey(r)
+			if _, ok := seen[k]; ok {
+				// This Reason is already "covered" by a previously seen base. Skip
+				// adding the Reason to the base being examined.
+				continue
+			}
+
+			useReasons = append(useReasons, r)
+		}
+
+		if len(useReasons) > 0 {
+			base.Reasons = useReasons
+			res = append(res, base)
+		}
+	}
+
+	return res
+}
+
 // MergeBackupBases reduces the two BackupBases into a single BackupBase.
 // Assumes the passed in BackupBases represents a prior backup version (across
 // some migration that disrupts lookup), and that the BackupBases used to call
@@ -178,19 +250,23 @@ func (bb *backupBases) MergeBackupBases(
 	other BackupBases,
 	reasonToKey func(reason identity.Reasoner) string,
 ) BackupBases {
-	if other == nil || (len(other.MergeBases()) == 0 && len(other.UniqueAssistBases()) == 0) {
+	if other == nil || (len(other.NewMergeBases()) == 0 && len(other.NewUniqueAssistBases()) == 0) {
 		return bb
 	}
 
-	if bb == nil || (len(bb.MergeBases()) == 0 && len(bb.UniqueAssistBases()) == 0) {
+	if bb == nil || (len(bb.NewMergeBases()) == 0 && len(bb.NewUniqueAssistBases()) == 0) {
 		return other
 	}
 
 	toMerge := map[string]struct{}{}
 	assist := map[string]struct{}{}
 
-	// Track the bases in bb.
+	// Track the bases in bb. We need to know the Reason(s) covered by merge bases
-	for _, m := range bb.mergeBases {
+	// and the Reason(s) covered by assist bases separately because the former
+	// dictates whether we need to select a merge base and an assist base from
+	// other while the latter dictates whether we need to select an assist base
+	// from other.
+	for _, m := range bb.MergeBases() {
 		for _, r := range m.Reasons {
 			k := reasonToKey(r)
 
@@ -199,248 +275,214 @@ func (bb *backupBases) MergeBackupBases(
 		}
 	}
 
-	for _, m := range bb.assistBases {
+	for _, m := range bb.UniqueAssistBases() {
 		for _, r := range m.Reasons {
 			k := reasonToKey(r)
 			assist[k] = struct{}{}
 		}
 	}
 
-	var toAdd []ManifestEntry
+	addMerge := getMissingBases(reasonToKey, toMerge, other.NewMergeBases())
-
+	addAssist := getMissingBases(reasonToKey, assist, other.NewUniqueAssistBases())
-	// Calculate the set of mergeBases to pull from other into this one.
-	for _, m := range other.MergeBases() {
-		useReasons := []identity.Reasoner{}
-
-		for _, r := range m.Reasons {
-			k := reasonToKey(r)
-			if _, ok := toMerge[k]; ok {
-				// Assume other contains prior manifest versions.
-				// We don't want to stack a prior version incomplete onto
-				// a current version's complete snapshot.
-				continue
-			}
-
-			useReasons = append(useReasons, r)
-		}
-
-		if len(useReasons) > 0 {
-			m.Reasons = useReasons
-			toAdd = append(toAdd, m)
-		}
-	}
 
 	res := &backupBases{
-		backups:     bb.Backups(),
+		mergeBases:  append(addMerge, bb.NewMergeBases()...),
-		mergeBases:  bb.MergeBases(),
+		assistBases: append(addAssist, bb.NewUniqueAssistBases()...),
-		assistBases: bb.UniqueAssistBases(),
-		// Note that assistBackups are a new feature and don't exist
-		// in prior versions where we were using UPN based reasons i.e.
-		// other won't have any assistBackups.
-		assistBackups: bb.UniqueAssistBackups(),
-	}
-
-	// Add new mergeBases and backups.
-	for _, man := range toAdd {
-		// Will get empty string if not found which is fine, it'll fail one of the
-		// other checks.
-		bID, _ := man.GetTag(TagBackupID)
-
-		bup, ok := getBackupByID(other.Backups(), bID)
-		if !ok {
-			logger.Ctx(ctx).Infow(
-				"not unioning snapshot missing backup",
-				"other_manifest_id", man.ID,
-				"other_backup_id", bID)
-
-			continue
-		}
-
-		bup.Reasons = man.Reasons
-
-		res.backups = append(res.backups, bup)
-		res.mergeBases = append(res.mergeBases, man)
 	}
 
 	return res
 }
 
-func findNonUniqueManifests(
+func fixupMinRequirements(
 	ctx context.Context,
-	manifests []ManifestEntry,
+	baseSet []BackupBase,
-) map[manifest.ID]struct{} {
+) []BackupBase {
-	// ReasonKey -> manifests with that reason.
+	res := make([]BackupBase, 0, len(baseSet))
-	reasons := map[string][]ManifestEntry{}
-	toDrop := map[manifest.ID]struct{}{}
 
-	for _, man := range manifests {
+	for _, base := range baseSet {
-		// Incomplete snapshots are used only for kopia-assisted incrementals. The
+		var (
-		// fact that we need this check here makes it seem like this should live in
+			backupID       model.StableID
-		// the kopia code. However, keeping it here allows for better debugging as
+			snapID         manifest.ID
-		// the kopia code only has access to a path builder which means it cannot
+			snapIncomplete bool
-		// remove the resource owner from the error/log output. That is also below
+			deetsID        string
-		// the point where we decide if we should do a full backup or an incremental.
+		)
-		if len(man.IncompleteReason) > 0 {
-			logger.Ctx(ctx).Infow(
-				"dropping incomplete manifest",
-				"manifest_id", man.ID)
 
-			toDrop[man.ID] = struct{}{}
+		if base.Backup != nil {
+			backupID = base.Backup.ID
 
+			deetsID = base.Backup.StreamStoreID
+			if len(deetsID) == 0 {
+				deetsID = base.Backup.DetailsID
+			}
+		}
+
+		if base.ItemDataSnapshot != nil {
+			snapID = base.ItemDataSnapshot.ID
+
+			snapIncomplete = len(base.ItemDataSnapshot.IncompleteReason) > 0
+		}
+
+		ictx := clues.Add(
+			ctx,
+			"base_backup_id", backupID,
+			"base_item_data_snapshot_id", snapID,
+			"base_details_id", deetsID)
+
+		switch {
+		case len(backupID) == 0:
+			logger.Ctx(ictx).Info("dropping base missing backup model")
+			continue
+
+		case len(snapID) == 0:
+			logger.Ctx(ictx).Info("dropping base missing item data snapshot")
+			continue
+
+		case snapIncomplete:
+			logger.Ctx(ictx).Info("dropping base with incomplete item data snapshot")
+			continue
+
+		case len(deetsID) == 0:
+			logger.Ctx(ictx).Info("dropping base missing backup details")
+			continue
+
+		case len(base.Reasons) == 0:
+			// Not sure how we'd end up here, but just to make sure we're really
+			// getting what we expect.
+			logger.Ctx(ictx).Info("dropping base with no marked Reasons")
 			continue
 		}
 
-		for _, reason := range man.Reasons {
+		res = append(res, base)
-			mapKey := reasonKey(reason)
-			reasons[mapKey] = append(reasons[mapKey], man)
-		}
 	}
 
-	for reason, mans := range reasons {
+	return res
-		ictx := clues.Add(ctx, "reason", reason)
-
-		if len(mans) == 0 {
-			// Not sure how this would happen but just in case...
-			continue
-		} else if len(mans) > 1 {
-			mIDs := make([]manifest.ID, 0, len(mans))
-			for _, m := range mans {
-				toDrop[m.ID] = struct{}{}
-				mIDs = append(mIDs, m.ID)
-			}
-
-			// TODO(ashmrtn): We should actually just remove this reason from the
-			// manifests and then if they have no reasons remaining drop them from the
-			// set.
-			logger.Ctx(ictx).Infow(
-				"dropping manifests with duplicate reason",
-				"manifest_ids", mIDs)
-
-			continue
-		}
-	}
-
-	return toDrop
 }
 
-func getBackupByID(backups []BackupEntry, bID string) (BackupEntry, bool) {
+func fixupReasons(
-	if len(bID) == 0 {
+	ctx context.Context,
-		return BackupEntry{}, false
+	baseSet []BackupBase,
+) []BackupBase {
+	// Associate a Reason with a set of bases since the basesByReason map needs a
+	// string key.
+	type baseEntry struct {
+		bases  []BackupBase
+		reason identity.Reasoner
 	}
 
-	idx := slices.IndexFunc(backups, func(b BackupEntry) bool {
+	var (
-		return string(b.ID) == bID
+		basesByReason = map[string]baseEntry{}
+		// res holds a mapping from backup ID -> base. We need this additional level
+		// of indirection when determining what to return because a base may be
+		// selected for multiple reasons. This map allows us to consolidate that
+		// into a single base result for all reasons easily.
+		res = map[model.StableID]BackupBase{}
+	)
+
+	// Organize all the base(s) by the Reason(s) they were chosen. A base can
+	// exist in multiple slices in the map if it was selected for multiple
+	// Reasons.
+	for _, base := range baseSet {
+		for _, reason := range base.Reasons {
+			foundBases := basesByReason[reasonKey(reason)]
+			foundBases.reason = reason
+			foundBases.bases = append(foundBases.bases, base)
+
+			basesByReason[reasonKey(reason)] = foundBases
+		}
+	}
+
+	// Go through the map and check that the length of each slice is 1. If it's
+	// longer than that then we somehow got multiple bases for the same Reason and
+	// should drop the extras.
+	for _, bases := range basesByReason {
+		ictx := clues.Add(
+			ctx,
+			"verify_service", bases.reason.Service().String(),
+			"verify_category", bases.reason.Category().String())
+
+		// Not sure how we'd actually get here but handle it anyway.
+		if len(bases.bases) == 0 {
+			logger.Ctx(ictx).Info("no bases found for reason")
+			continue
+		}
+
+		// We've got at least one base for this Reason. The below finds which base
+		// to keep based on the creation time of the bases. If there's multiple
+		// bases in the input slice then we'll log information about the ones that
+		// we didn't add to the result set.
+
+		// Sort in reverse chronological order so that it's easy to find the
+		// youngest base.
+		slices.SortFunc(bases.bases, func(a, b BackupBase) int {
+			return -a.Backup.CreationTime.Compare(b.Backup.CreationTime)
 		})
 
-	if idx < 0 || idx >= len(backups) {
+		keepBase := bases.bases[0]
-		return BackupEntry{}, false
+
+		// Add the youngest base to the result set. We add each Reason for selecting
+		// the base individually so that bases dropped for a particular Reason (or
+		// dropped completely because they overlap for all Reasons) happens without
+		// additional logic. The dropped (Reason, base) pair will just never be
+		// added to the result set to begin with.
+		b, ok := res[keepBase.Backup.ID]
+		if ok {
+			// We've already seen this base, just add this Reason to it as well.
+			b.Reasons = append(b.Reasons, bases.reason)
+			res[keepBase.Backup.ID] = b
+
+			continue
 		}
 
-	return backups[idx], true
+		// We haven't seen this base before. We want to clear all the Reasons for it
+		// except the one we're currently examining. That allows us to just not add
+		// bases that are duplicates for a Reason to res and still end up with the
+		// correct output.
+		keepBase.Reasons = []identity.Reasoner{bases.reason}
+		res[keepBase.Backup.ID] = keepBase
+
+		// Don't log about dropped bases if there was only one base.
+		if len(bases.bases) == 1 {
+			continue
+		}
+
+		// This is purely for debugging, but log the base(s) that we dropped for
+		// this Reason.
+		var dropped []model.StableID
+
+		for _, b := range bases.bases[1:] {
+			dropped = append(dropped, b.Backup.ID)
+		}
+
+		logger.Ctx(ictx).Infow(
+			"dropping bases for reason",
+			"dropped_backup_ids", dropped)
+	}
+
+	return maps.Values(res)
 }
 
 // fixupAndVerify goes through the set of backups and snapshots used for merging
 // and ensures:
 //   - the reasons for selecting merge snapshots are distinct
-//   - all bases used for merging have a backup model with item and details
+//   - all bases have a backup model with item and details snapshot IDs
-//     snapshot ID
+//   - all bases have both a backup and item data snapshot present
+//   - all bases have item data snapshots with no incomplete reason
 //
 // Backups that have overlapping reasons or that are not complete are removed
 // from the set. Dropping these is safe because it only affects how much data we
 // pull. On the other hand, *not* dropping them is unsafe as it will muck up
 // merging when we add stuff to kopia (possibly multiple entries for the same
 // item etc).
-//
-// TODO(pandeyabs): Refactor common code into a helper as part of #3943.
 func (bb *backupBases) fixupAndVerify(ctx context.Context) {
-	toDrop := findNonUniqueManifests(ctx, bb.mergeBases)
+	// Start off by removing bases that don't meet the minimum requirements of
+	// having a backup model and item data snapshot or having a backup details ID.
+	// These requirements apply to both merge and assist bases.
+	bb.mergeBases = fixupMinRequirements(ctx, bb.mergeBases)
+	bb.assistBases = fixupMinRequirements(ctx, bb.assistBases)
 
-	var (
+	// Remove merge bases that have overlapping Reasons. It's alright to call this
-		backupsToKeep       []BackupEntry
+	// on assist bases too because we only expect at most one assist base per
-		assistBackupsToKeep []BackupEntry
+	// Reason.
-		mergeToKeep         []ManifestEntry
+	bb.mergeBases = fixupReasons(ctx, bb.mergeBases)
-		assistToKeep        []ManifestEntry
+	bb.assistBases = fixupReasons(ctx, bb.assistBases)
-	)
-
-	for _, man := range bb.mergeBases {
-		if _, ok := toDrop[man.ID]; ok {
-			continue
-		}
-
-		bID, _ := man.GetTag(TagBackupID)
-
-		bup, ok := getBackupByID(bb.backups, bID)
-		if !ok {
-			toDrop[man.ID] = struct{}{}
-
-			logger.Ctx(ctx).Info(
-				"dropping merge base due to missing backup",
-				"manifest_id", man.ID)
-
-			continue
-		}
-
-		deetsID := bup.StreamStoreID
-		if len(deetsID) == 0 {
-			deetsID = bup.DetailsID
-		}
-
-		if len(bup.SnapshotID) == 0 || len(deetsID) == 0 {
-			toDrop[man.ID] = struct{}{}
-
-			logger.Ctx(ctx).Info(
-				"dropping merge base due to invalid backup",
-				"manifest_id", man.ID)
-
-			continue
-		}
-
-		backupsToKeep = append(backupsToKeep, bup)
-		mergeToKeep = append(mergeToKeep, man)
-	}
-
-	// Drop assist snapshots with overlapping reasons.
-	toDropAssists := findNonUniqueManifests(ctx, bb.assistBases)
-
-	for _, man := range bb.assistBases {
-		if _, ok := toDropAssists[man.ID]; ok {
-			continue
-		}
-
-		bID, _ := man.GetTag(TagBackupID)
-
-		bup, ok := getBackupByID(bb.assistBackups, bID)
-		if !ok {
-			toDrop[man.ID] = struct{}{}
-
-			logger.Ctx(ctx).Info(
-				"dropping assist base due to missing backup",
-				"manifest_id", man.ID)
-
-			continue
-		}
-
-		deetsID := bup.StreamStoreID
-		if len(deetsID) == 0 {
-			deetsID = bup.DetailsID
-		}
-
-		if len(bup.SnapshotID) == 0 || len(deetsID) == 0 {
-			toDrop[man.ID] = struct{}{}
-
-			logger.Ctx(ctx).Info(
-				"dropping assist base due to invalid backup",
-				"manifest_id", man.ID)
-
-			continue
-		}
-
-		assistBackupsToKeep = append(assistBackupsToKeep, bup)
-		assistToKeep = append(assistToKeep, man)
-	}
-
-	bb.backups = backupsToKeep
-	bb.mergeBases = mergeToKeep
-	bb.assistBases = assistToKeep
-	bb.assistBackups = assistBackupsToKeep
 }

src/internal/kopia/base_finder.go

@@ -136,7 +136,7 @@ func (b *baseFinder) getBackupModel(
 	return bup, nil
 }
 
-type backupBase struct {
+type BackupBase struct {
 	Backup           *backup.Backup
 	ItemDataSnapshot *snapshot.Manifest
 	// Reasons contains the tenant, protected resource and service/categories that
@@ -157,7 +157,7 @@ func (b *baseFinder) findBasesInSet(
 	ctx context.Context,
 	reason identity.Reasoner,
 	metas []*manifest.EntryMetadata,
-) (*backupBase, *backupBase, error) {
+) (*BackupBase, *BackupBase, error) {
 	// Sort manifests by time so we can go through them sequentially. The code in
 	// kopia appears to sort them already, but add sorting here just so we're not
 	// reliant on undocumented behavior.
@@ -166,8 +166,8 @@ func (b *baseFinder) findBasesInSet(
 	})
 
 	var (
-		mergeBase  *backupBase
+		mergeBase  *BackupBase
-		assistBase *backupBase
+		assistBase *BackupBase
 	)
 
 	for i := len(metas) - 1; i >= 0; i-- {
@@ -226,7 +226,7 @@ func (b *baseFinder) findBasesInSet(
 
 		if b.isAssistBackupModel(ictx, bup) {
 			if assistBase == nil {
-				assistBase = &backupBase{
+				assistBase = &BackupBase{
 					Backup:           bup,
 					ItemDataSnapshot: man,
 					Reasons:          []identity.Reasoner{reason},
@@ -248,7 +248,7 @@ func (b *baseFinder) findBasesInSet(
 			"search_snapshot_id", meta.ID,
 			"ssid", ssid)
 
-		mergeBase = &backupBase{
+		mergeBase = &BackupBase{
 			Backup:           bup,
 			ItemDataSnapshot: man,
 			Reasons:          []identity.Reasoner{reason},
@@ -304,7 +304,7 @@ func (b *baseFinder) getBase(
 	ctx context.Context,
 	r identity.Reasoner,
 	tags map[string]string,
-) (*backupBase, *backupBase, error) {
+) (*BackupBase, *BackupBase, error) {
 	allTags := map[string]string{}
 
 	for _, k := range tagKeys(r) {
@@ -336,8 +336,8 @@ func (b *baseFinder) FindBases(
 		// Backup models and item data snapshot manifests are 1:1 for bases so just
 		// track things by the backup ID. We need to track by ID so we can coalesce
 		// the reason for selecting something.
-		mergeBases  = map[model.StableID]backupBase{}
+		mergeBases  = map[model.StableID]BackupBase{}
-		assistBases = map[model.StableID]backupBase{}
+		assistBases = map[model.StableID]BackupBase{}
 	)
 
 	for _, searchReason := range reasons {
@@ -379,45 +379,11 @@ func (b *baseFinder) FindBases(
 		}
 	}
 
-	// Convert what we got to the format that backupBases takes right now.
+	res := &backupBases{
-	// TODO(ashmrtn): Remove when backupBases has consolidated fields.
+		mergeBases:  maps.Values(mergeBases),
-	res := &backupBases{}
+		assistBases: maps.Values(assistBases),
-	bups := make([]BackupEntry, 0, len(mergeBases))
-	snaps := make([]ManifestEntry, 0, len(mergeBases))
-
-	for _, base := range mergeBases {
-		bups = append(bups, BackupEntry{
-			Backup:  base.Backup,
-			Reasons: base.Reasons,
-		})
-
-		snaps = append(snaps, ManifestEntry{
-			Manifest: base.ItemDataSnapshot,
-			Reasons:  base.Reasons,
-		})
 	}
 
-	res.backups = bups
-	res.mergeBases = snaps
-
-	bups = make([]BackupEntry, 0, len(assistBases))
-	snaps = make([]ManifestEntry, 0, len(assistBases))
-
-	for _, base := range assistBases {
-		bups = append(bups, BackupEntry{
-			Backup:  base.Backup,
-			Reasons: base.Reasons,
-		})
-
-		snaps = append(snaps, ManifestEntry{
-			Manifest: base.ItemDataSnapshot,
-			Reasons:  base.Reasons,
-		})
-	}
-
-	res.assistBackups = bups
-	res.assistBases = snaps
-
 	res.fixupAndVerify(ctx)
 
 	return res

src/internal/kopia/mock_backup_base.go

@@ -3,9 +3,88 @@ package kopia
 import (
 	"testing"
 
+	"github.com/kopia/kopia/repo/manifest"
+	"github.com/kopia/kopia/snapshot"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/alcionai/corso/src/internal/model"
+	"github.com/alcionai/corso/src/pkg/backup"
 )
 
+// TODO(ashmrtn): Temp function until all PRs in the series merge.
+func backupsMatch(t *testing.T, expect, got []BackupEntry, dataType string) {
+	expectBups := make([]*backup.Backup, 0, len(expect))
+	gotBups := make([]*backup.Backup, 0, len(got))
+	gotBasesByID := map[model.StableID]BackupEntry{}
+
+	for _, e := range expect {
+		if e.Backup != nil {
+			expectBups = append(expectBups, e.Backup)
+		}
+	}
+
+	for _, g := range got {
+		if g.Backup != nil {
+			gotBups = append(gotBups, g.Backup)
+			gotBasesByID[g.Backup.ID] = g
+		}
+	}
+
+	assert.ElementsMatch(t, expectBups, gotBups, dataType+" backup model")
+
+	// Need to compare Reasons separately since they're also a slice.
+	for _, e := range expect {
+		if e.Backup == nil {
+			continue
+		}
+
+		b, ok := gotBasesByID[e.Backup.ID]
+		if !ok {
+			// Missing bases will be reported above.
+			continue
+		}
+
+		assert.ElementsMatch(t, e.Reasons, b.Reasons)
+	}
+}
+
+// TODO(ashmrtn): Temp function until all PRs in the series merge.
+func manifestsMatch(t *testing.T, expect, got []ManifestEntry, dataType string) {
+	expectMans := make([]*snapshot.Manifest, 0, len(expect))
+	gotMans := make([]*snapshot.Manifest, 0, len(got))
+	gotBasesByID := map[manifest.ID]ManifestEntry{}
+
+	for _, e := range expect {
+		if e.Manifest != nil {
+			expectMans = append(expectMans, e.Manifest)
+		}
+	}
+
+	for _, g := range got {
+		if g.Manifest != nil {
+			gotMans = append(gotMans, g.Manifest)
+			gotBasesByID[g.Manifest.ID] = g
+		}
+	}
+
+	assert.ElementsMatch(t, expectMans, gotMans, dataType+" item data snapshot")
+
+	// Need to compare Reasons separately since they're also a slice.
+	for _, e := range expect {
+		if e.Manifest == nil {
+			continue
+		}
+
+		b, ok := gotBasesByID[e.Manifest.ID]
+		if !ok {
+			// Missing bases will be reported above.
+			continue
+		}
+
+		assert.ElementsMatch(t, e.Reasons, b.Reasons)
+	}
+}
+
 func AssertBackupBasesEqual(t *testing.T, expect, got BackupBases) {
 	if expect == nil && got == nil {
 		return
@@ -33,11 +112,11 @@ func AssertBackupBasesEqual(t *testing.T, expect, got BackupBases) {
 		return
 	}
 
-	assert.ElementsMatch(t, expect.Backups(), got.Backups(), "backups")
+	backupsMatch(t, expect.Backups(), got.Backups(), "merge backups")
-	assert.ElementsMatch(t, expect.MergeBases(), got.MergeBases(), "merge bases")
+	manifestsMatch(t, expect.MergeBases(), got.MergeBases(), "merge manifests")
-	assert.ElementsMatch(t, expect.UniqueAssistBackups(), got.UniqueAssistBackups(), "assist backups")
+	backupsMatch(t, expect.UniqueAssistBackups(), got.UniqueAssistBackups(), "assist backups")
-	assert.ElementsMatch(t, expect.UniqueAssistBases(), got.UniqueAssistBases(), "assist bases")
+	manifestsMatch(t, expect.UniqueAssistBases(), got.UniqueAssistBases(), "assist manifests")
-	assert.ElementsMatch(t, expect.SnapshotAssistBases(), got.SnapshotAssistBases(), "snapshot assist bases")
+	manifestsMatch(t, expect.SnapshotAssistBases(), got.SnapshotAssistBases(), "snapshot assist bases")
 }
 
 func NewMockBackupBases() *MockBackupBases {
@@ -49,22 +128,63 @@ type MockBackupBases struct {
 }
 
 func (bb *MockBackupBases) WithBackups(b ...BackupEntry) *MockBackupBases {
-	bb.backupBases.backups = append(bb.Backups(), b...)
+	bases := make([]BackupBase, 0, len(b))
+	for _, base := range b {
+		bases = append(bases, BackupBase{
+			Backup:  base.Backup,
+			Reasons: base.Reasons,
+		})
+	}
+
+	bb.backupBases.mergeBases = append(bb.NewMergeBases(), bases...)
+
 	return bb
 }
 
 func (bb *MockBackupBases) WithMergeBases(m ...ManifestEntry) *MockBackupBases {
-	bb.backupBases.mergeBases = append(bb.MergeBases(), m...)
+	bases := make([]BackupBase, 0, len(m))
+	for _, base := range m {
+		bases = append(bases, BackupBase{
+			ItemDataSnapshot: base.Manifest,
+			Reasons:          base.Reasons,
+		})
+	}
+
+	bb.backupBases.mergeBases = append(bb.NewMergeBases(), bases...)
+
 	return bb
 }
 
 func (bb *MockBackupBases) WithAssistBackups(b ...BackupEntry) *MockBackupBases {
-	bb.backupBases.assistBackups = append(bb.UniqueAssistBackups(), b...)
+	bases := make([]BackupBase, 0, len(b))
+	for _, base := range b {
+		bases = append(bases, BackupBase{
+			Backup:  base.Backup,
+			Reasons: base.Reasons,
+		})
+	}
+
+	bb.backupBases.assistBases = append(bb.NewUniqueAssistBases(), bases...)
+
 	return bb
 }
 
 func (bb *MockBackupBases) WithAssistBases(m ...ManifestEntry) *MockBackupBases {
-	bb.backupBases.assistBases = append(bb.UniqueAssistBases(), m...)
+	bases := make([]BackupBase, 0, len(m))
+	for _, base := range m {
+		bases = append(bases, BackupBase{
+			ItemDataSnapshot: base.Manifest,
+			Reasons:          base.Reasons,
+		})
+	}
+
+	bb.backupBases.assistBases = append(bb.NewUniqueAssistBases(), bases...)
+
+	return bb
+}
+
+func (bb *MockBackupBases) NewWithMergeBases(b ...BackupBase) *MockBackupBases {
+	bb.backupBases.mergeBases = append(bb.NewMergeBases(), b...)
 	return bb
 }
 

src/internal/operations/manifests_test.go

@@ -639,8 +639,8 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 		}
 	}
 
-	makeBackup := func(ro, snapID string, cats ...path.CategoryType) kopia.BackupEntry {
+	makeBackupBase := func(ro, snapID, incmpl string, cats ...path.CategoryType) kopia.BackupBase {
-		return kopia.BackupEntry{
+		return kopia.BackupBase{
 			Backup: &backup.Backup{
 				BaseModel: model.BaseModel{
 					ID: model.StableID(snapID + "bup"),
@@ -648,6 +648,11 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 				SnapshotID:    snapID,
 				StreamStoreID: snapID + "store",
 			},
+			ItemDataSnapshot: &snapshot.Manifest{
+				ID:               manifest.ID(snapID),
+				IncompleteReason: incmpl,
+				Tags:             map[string]string{"tag:" + kopia.TagBackupID: snapID + "bup"},
+			},
 			Reasons: buildReasons(tid, ro, path.ExchangeService, cats...),
 		}
 	}
@@ -682,8 +687,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			bf: &mockBackupFinder{
 				data: map[string]kopia.BackupBases{
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)),
-						WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)),
 				},
 			},
 			rp:              mockRestoreProducer{},
@@ -693,17 +697,15 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			assertB:         assert.False,
 			expectDCS:       nil,
 			expectMans: kopia.NewMockBackupBases().
-				WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+				NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)).
-				WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)).
 				MockDisableMergeBases(),
 		},
 		{
 			name: "only fallbacks",
 			bf: &mockBackupFinder{
 				data: map[string]kopia.BackupBases{
-					fbro: kopia.NewMockBackupBases().WithMergeBases(
+					fbro: kopia.NewMockBackupBases().
-						makeMan(fbro, "fb_id1", "", path.EmailCategory)).WithBackups(
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)),
-						makeBackup(fbro, "fb_id1", path.EmailCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -717,16 +719,14 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			assertB:         assert.True,
 			expectDCS:       []mockColl{{id: "fb_id1"}},
 			expectMans: kopia.NewMockBackupBases().
-				WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+				NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)),
-				WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)),
 		},
 		{
 			name: "only fallbacks, drop assist",
 			bf: &mockBackupFinder{
 				data: map[string]kopia.BackupBases{
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)),
-						WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -741,8 +741,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			assertB:         assert.True,
 			expectDCS:       []mockColl{{id: "fb_id1"}},
 			expectMans: kopia.NewMockBackupBases().
-				WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+				NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)).
-				WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)).
 				MockDisableAssistBases(),
 		},
 		{
@@ -752,8 +751,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 					ro: kopia.NewMockBackupBases().
 						WithMergeBases(makeMan(ro, "id1", "", path.EmailCategory)),
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)),
-						WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -804,8 +802,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 						WithMergeBases(makeMan(ro, "id1", "", path.EmailCategory)).
 						WithAssistBases(makeMan(ro, "id2", "checkpoint", path.EmailCategory)),
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)).
-						WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)).
 						WithAssistBases(makeMan(fbro, "fb_id2", "checkpoint", path.EmailCategory)),
 				},
 			},
@@ -834,8 +831,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 					ro: kopia.NewMockBackupBases().WithAssistBases(
 						makeMan(ro, "id2", "checkpoint", path.EmailCategory)),
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)),
-						WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -851,8 +847,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			assertB:         assert.True,
 			expectDCS:       []mockColl{{id: "fb_id1"}},
 			expectMans: kopia.NewMockBackupBases().
-				WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+				NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)).
-				WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)).
 				WithAssistBases(makeMan(ro, "id2", "checkpoint", path.EmailCategory)),
 		},
 		{
@@ -862,8 +857,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 					ro: kopia.NewMockBackupBases().WithAssistBases(
 						makeMan(ro, "id2", "checkpoint", path.EmailCategory)),
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)),
-						WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -880,8 +874,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			assertB:         assert.True,
 			expectDCS:       []mockColl{{id: "fb_id1"}},
 			expectMans: kopia.NewMockBackupBases().
-				WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory)).
+				NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory)).
-				WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory)).
 				MockDisableAssistBases(),
 		},
 		{
@@ -916,8 +909,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 					ro: kopia.NewMockBackupBases().WithMergeBases(
 						makeMan(ro, "id1", "", path.EmailCategory, path.ContactsCategory)),
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.EmailCategory, path.ContactsCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory, path.ContactsCategory)),
-						WithBackups(makeBackup(fbro, "fb_id1", path.EmailCategory, path.ContactsCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -948,8 +940,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 					ro: kopia.NewMockBackupBases().WithMergeBases(
 						makeMan(ro, "id1", "", path.EmailCategory)),
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(makeMan(fbro, "fb_id1", "", path.ContactsCategory)).
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.ContactsCategory)),
-						WithBackups(makeBackup(fbro, "fb_id1", path.ContactsCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -967,10 +958,8 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			assertB:   assert.True,
 			expectDCS: []mockColl{{id: "id1"}, {id: "fb_id1"}},
 			expectMans: kopia.NewMockBackupBases().
-				WithMergeBases(
+				WithMergeBases(makeMan(ro, "id1", "", path.EmailCategory)).
-					makeMan(ro, "id1", "", path.EmailCategory),
+				NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.ContactsCategory)),
-					makeMan(fbro, "fb_id1", "", path.ContactsCategory)).
-				WithBackups(makeBackup(fbro, "fb_id1", path.ContactsCategory)),
 		},
 		{
 			name: "complete mans and complete fallbacks, fallback has superset of reasons",
@@ -979,10 +968,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 					ro: kopia.NewMockBackupBases().WithMergeBases(
 						makeMan(ro, "id1", "", path.EmailCategory)),
 					fbro: kopia.NewMockBackupBases().
-						WithMergeBases(
+						NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.EmailCategory, path.ContactsCategory)),
-							makeMan(fbro, "fb_id1", "", path.EmailCategory, path.ContactsCategory)).
-						WithBackups(
-							makeBackup(fbro, "fb_id1", path.EmailCategory, path.ContactsCategory)),
 				},
 			},
 			rp: mockRestoreProducer{
@@ -1004,11 +990,8 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_Fallb
 			assertB:   assert.True,
 			expectDCS: []mockColl{{id: "id1"}, {id: "fb_id1"}},
 			expectMans: kopia.NewMockBackupBases().
-				WithMergeBases(
+				WithMergeBases(makeMan(ro, "id1", "", path.EmailCategory)).
-					makeMan(ro, "id1", "", path.EmailCategory),
+				NewWithMergeBases(makeBackupBase(fbro, "fb_id1", "", path.ContactsCategory)),
-					makeMan(fbro, "fb_id1", "", path.ContactsCategory)).
-				WithBackups(
-					makeBackup(fbro, "fb_id1", path.ContactsCategory)),
 		},
 	}