Add additional unit test

Rename backup model variables that holds the protected resource ID and
name. The variable name has changed from ResourceOwner[ID|Name] to
ProtectedResource[ID|Name].

The previous version of the variable in the model of the form
ProtectedResourceX wasn't getting populated and never had any data
persisted for it even though other code sometimes relied on it having a
value.

The current update ** DOES NOT ** change the JSON tag associated with
the renamed variable. This means that golang's stdlib JSON serialization
with continue to use the old name. Thus there's no need for additional
code to handle previously serialized backup models.

src/internal/kopia/cleanup_backups_test.go

@@ -371,7 +371,7 @@ func (suite *BackupCleanupUnitSuite) TestCleanupOrphanedData() {
 
 	backupWithLegacyResource := func(protectedResource string, b *backup.Backup) *backup.Backup {
 		res := *b
-		res.ResourceOwnerID = protectedResource
+		res.ProtectedResourceID = protectedResource
 
 		return &res
 	}
@@ -706,14 +706,48 @@ func (suite *BackupCleanupUnitSuite) TestCleanupOrphanedData() {
 		},
 		{
 			// Test that an assist base that has the same Reasons as a newer merge
-			// base but the merge base is from an older version of corso for some
+			// base but the merge base is missing the protected resource ID for some
 			// reason doesn't cause the assist base to be garbage collected. This is
-			// not ideal, but some older versions of corso didn't even populate the
-			// resource owner ID.
+			// not ideal, but at least we're not accidentally removing assist bases
+			// that could be useful later on.
 			//
-			// Worst case, the assist base will be cleaned up when the user upgrades
-			// corso and generates either a new assist base or merge base with the
-			// same reason.
+			// Worst case, the assist base will be cleaned up when there's a  merge
+			// base that does populated the protected resource ID field.
+			name: "AssistAndMergeMissingResourceIDBases NotYoungest Noops",
+			snapshots: []*manifest.EntryMetadata{
+				manifestWithReasons(
+					manifestWithTime(baseTime, snapCurrent()),
+					"tenant1",
+					NewReason("", "ro", path.ExchangeService, path.EmailCategory)),
+				manifestWithTime(baseTime, deetsCurrent()),
+
+				manifestWithReasons(
+					manifestWithTime(baseTime.Add(time.Second), snapCurrent2()),
+					"tenant1",
+					NewReason("", "ro", path.ExchangeService, path.EmailCategory)),
+				manifestWithTime(baseTime.Add(time.Second), deetsCurrent2()),
+			},
+			backups: []backupRes{
+				{bup: backupWithResource("ro", true, backupWithTime(baseTime, bupCurrent()))},
+				{bup: func() *backup.Backup {
+					res := backupWithResource(
+						"ro",
+						false,
+						backupWithTime(baseTime.Add(time.Second), bupCurrent2()))
+					res.ProtectedResourceID = ""
+
+					return res
+				}()},
+			},
+			time:      baseTime.Add(48 * time.Hour),
+			buffer:    24 * time.Hour,
+			expectErr: assert.NoError,
+		},
+		{
+			// Test that an assist base that has the same Reasons as a newer merge
+			// base but the merge base is from an older version of corso that doesn't
+			// add merge or assist base tags for some reason causes the assist base to
+			// be garbage collected.
 			name: "AssistAndLegacyMergeBases NotYoungest Noops",
 			snapshots: []*manifest.EntryMetadata{
 				manifestWithReasons(
@@ -732,8 +766,13 @@ func (suite *BackupCleanupUnitSuite) TestCleanupOrphanedData() {
 				{bup: backupWithResource("ro", true, backupWithTime(baseTime, bupCurrent()))},
 				{bup: backupWithLegacyResource("ro", backupWithTime(baseTime.Add(time.Second), bupCurrent2()))},
 			},
-			time:      baseTime.Add(48 * time.Hour),
-			buffer:    24 * time.Hour,
+			time:   baseTime.Add(48 * time.Hour),
+			buffer: 24 * time.Hour,
+			expectDeleteIDs: []manifest.ID{
+				snapCurrent().ID,
+				deetsCurrent().ID,
+				manifest.ID(bupCurrent().ModelStoreID),
+			},
 			expectErr: assert.NoError,
 		},
 		{

src/pkg/backup/backup.go

@@ -36,10 +36,12 @@ type Backup struct {
 	// Selector used in this operation
 	Selector selectors.Selector `json:"selectors"`
 
-	// TODO: in process of gaining support, most cases will still use
-	// ResourceOwner and ResourceOwnerName.
-	ProtectedResourceID   string `json:"protectedResourceID,omitempty"`
-	ProtectedResourceName string `json:"protectedResourceName,omitempty"`
+	// ** DO NOT CHANGE JSON TAG NAMES **
+	// These are in-memory only variable renames of previously persisted fields.
+	// ** CHANGING THE JSON TAGS WILL BREAK THINGS BECAUSE THE MODEL WON'T **
+	// ** DESERIALIZE PROPERLY **
+	ProtectedResourceID   string `json:"resourceOwnerID,omitempty"`
+	ProtectedResourceName string `json:"resourceOwnerName,omitempty"`
 
 	// Version represents the version of the backup format
 	Version int `json:"version"`
@@ -61,10 +63,6 @@ type Backup struct {
 	// Reference to the backup details storage location.
 	// Used to read backup.Details from the streamstore.
 	DetailsID string `json:"detailsID"`
-
-	// prefer protectedResource
-	ResourceOwnerID   string `json:"resourceOwnerID,omitempty"`
-	ResourceOwnerName string `json:"resourceOwnerName,omitempty"`
 }
 
 // interface compliance checks
@@ -115,8 +113,8 @@ func New(
 			Tags: tags,
 		},
 
-		ResourceOwnerID:   ownerID,
-		ResourceOwnerName: ownerName,
+		ProtectedResourceID:   ownerID,
+		ProtectedResourceName: ownerName,
 
 		Version:       version,
 		SnapshotID:    snapshotID,
@@ -253,9 +251,7 @@ func (b Backup) Values() []string {
 
 	name := str.First(
 		b.ProtectedResourceName,
-		b.ResourceOwnerName,
 		b.ProtectedResourceID,
-		b.ResourceOwnerID,
 		b.Selector.Name())
 
 	bs := b.toStats()

src/pkg/backup/backup_test.go

@@ -40,10 +40,8 @@ func stubBackup(t time.Time, ownerID, ownerName string) backup.Backup {
 		CreationTime:          t,
 		SnapshotID:            "snapshot",
 		DetailsID:             "details",
-		ProtectedResourceID:   ownerID + "-pr",
-		ProtectedResourceName: ownerName + "-pr",
-		ResourceOwnerID:       ownerID + "-ro",
-		ResourceOwnerName:     ownerName + "-ro",
+		ProtectedResourceID:   ownerID + "-ro",
+		ProtectedResourceName: ownerName + "-ro",
 		Status:                "status",
 		Selector:              sel.Selector,
 		ErrorCount:            2,
@@ -86,7 +84,7 @@ func (suite *BackupUnitSuite) TestBackup_HeadersValues() {
 			nowFmt,
 			"1m0s",
 			"status (2 errors, 1 skipped: 1 malware)",
-			"name-pr",
+			"name-ro",
 		}
 	)
 
@@ -123,9 +121,6 @@ func (suite *BackupUnitSuite) TestBackup_HeadersValues_onlyResourceOwners() {
 		}
 	)
 
-	b.ProtectedResourceID = ""
-	b.ProtectedResourceName = ""
-
 	b.StartAndEndTime.CompletedAt = later
 
 	// single skipped malware