repository: Properly set id for finalized index

As MergeFinalIndex and index uploads can occur concurrently, it is
necessary for MergeFinalIndex to check whether the IDs for an index were
already set before merging it. Otherwise, we'd loose the ID of an index
which is set _after_ uploading it.

internal/repository/master_index.go

@@ -287,7 +287,9 @@ func (mi *MasterIndex) MergeFinalIndexes() error {
 		idx := mi.idx[i]
 		// clear reference in masterindex as it may become stale
 		mi.idx[i] = nil
-		if !idx.Final() {
+		// do not merge indexes that have no id set
+		ids, _ := idx.IDs()
+		if !idx.Final() || len(ids) == 0 {
 			newIdx = append(newIdx, idx)
 		} else {
 			err := mi.idx[0].merge(idx)
@@ -404,7 +406,13 @@ func SaveIndex(ctx context.Context, repo restic.SaverUnpacked, index *Index) (re
 		return restic.ID{}, err
 	}
 
-	return repo.SaveUnpacked(ctx, restic.IndexFile, buf.Bytes())
+	id, err := repo.SaveUnpacked(ctx, restic.IndexFile, buf.Bytes())
+	ierr := index.SetID(id)
+	if ierr != nil {
+		// logic bug
+		panic(ierr)
+	}
+	return id, err
 }
 
 // saveIndex saves all indexes in the backend.

internal/repository/master_index_test.go

@@ -157,16 +157,9 @@ func TestMasterMergeFinalIndexes(t *testing.T) {
 	mIdx.Insert(idx1)
 	mIdx.Insert(idx2)
 
-	finalIndexes := mIdx.FinalizeNotFinalIndexes()
+	finalIndexes, idxCount := repository.TestMergeIndex(t, mIdx)
 	rtest.Equals(t, []*repository.Index{idx1, idx2}, finalIndexes)
-
-	err := mIdx.MergeFinalIndexes()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	allIndexes := mIdx.All()
-	rtest.Equals(t, 1, len(allIndexes))
+	rtest.Equals(t, 1, idxCount)
 
 	blobCount := 0
 	for range mIdx.Each(context.TODO()) {
@@ -189,16 +182,9 @@ func TestMasterMergeFinalIndexes(t *testing.T) {
 	idx3.StorePack(blob2.PackID, []restic.Blob{blob2.Blob})
 
 	mIdx.Insert(idx3)
-	finalIndexes = mIdx.FinalizeNotFinalIndexes()
+	finalIndexes, idxCount = repository.TestMergeIndex(t, mIdx)
 	rtest.Equals(t, []*repository.Index{idx3}, finalIndexes)
-
-	err = mIdx.MergeFinalIndexes()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	allIndexes = mIdx.All()
-	rtest.Equals(t, 1, len(allIndexes))
+	rtest.Equals(t, 1, idxCount)
 
 	// Index should have same entries as before!
 	blobs = mIdx.Lookup(bhInIdx1)

internal/repository/repository.go

@@ -582,12 +582,6 @@ func (r *Repository) LoadIndex(ctx context.Context) error {
 		if err != nil {
 			return err
 		}
-
-		_, err = idx.IDs()
-		if err != nil {
-			return err
-		}
-
 		r.idx.Insert(idx)
 		return nil
 	})

internal/repository/testing.go

@@ -132,3 +132,13 @@ func BenchmarkAllVersions(b *testing.B, bench VersionedBenchmark) {
 		})
 	}
 }
+
+func TestMergeIndex(t testing.TB, mi *MasterIndex) ([]*Index, int) {
+	finalIndexes := mi.FinalizeNotFinalIndexes()
+	for _, idx := range finalIndexes {
+		test.OK(t, idx.SetID(restic.NewRandomID()))
+	}
+
+	test.OK(t, mi.MergeFinalIndexes())
+	return finalIndexes, len(mi.idx)
+}