REP-6088 Fix bugs from PR #117

- Mismatches were previously shown in an indeterminate order. Now they’re
  consistently sorted by the mismatched documents’ `_id`.
- Documents with missing fields were being logged as entirely missing.
  That logic is corrected here.
- The logic to create the table of missing/changed documents previously
  iterated through the _ids persisted in the task rather than the actual
  missing/changed documents. This was appropriate when that list stored
  mismatches but is no longer correct since the list now always stores
  the list of documents to check in the task. Thus, if there were only a
  handful of missing documents in a recheck task that contained thousands
  of document IDs, all of that task’s document IDs would be logged as
  missing. This was an oversight from PR #117, which should have updated
  the logic to build that table as it migrated that for the
  mismatched-documents table. This changeset does the necessary update.

Author: FGasper

internal/verifier/migration_verifier_test.go

@@ -15,6 +15,7 @@ import (
 	"regexp"
 	"slices"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 
@@ -27,6 +28,7 @@ import (
 	"github.com/10gen/migration-verifier/mslices"
 	"github.com/cespare/permute/v2"
 	"github.com/rs/zerolog"
+	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
@@ -1240,6 +1242,88 @@ func (suite *IntegrationTestSuite) TestVerifierCompareIndexes() {
 	}
 }
 
+func (suite *IntegrationTestSuite) TestVerifierDocMismatches() {
+	ctx := suite.Context()
+
+	suite.Require().NoError(
+		suite.srcMongoClient.
+			Database("test").
+			Collection("coll").Drop(ctx),
+	)
+	suite.Require().NoError(
+		suite.dstMongoClient.
+			Database("test").
+			Collection("coll").Drop(ctx),
+	)
+
+	_, err := suite.srcMongoClient.
+		Database("test").
+		Collection("coll").
+		InsertMany(
+			ctx,
+			lo.RepeatBy(
+				20,
+				func(index int) any {
+					return bson.D{
+						{"_id", 100000 + index},
+						{"foo", 3},
+					}
+				},
+			),
+		)
+	suite.Require().NoError(err)
+
+	// The first has a mismatched `foo` value,
+	// and the 2nd lacks `foo` entirely.
+	_, err = suite.dstMongoClient.
+		Database("test").
+		Collection("coll").
+		InsertMany(ctx, lo.ToAnySlice([]bson.D{
+			{{"_id", 100000}, {"foo", 1}},
+			{{"_id", 100001}},
+		}))
+	suite.Require().NoError(err)
+
+	verifier := suite.BuildVerifier()
+	verifier.failureDisplaySize = 10
+
+	ns := "test.coll"
+	verifier.SetSrcNamespaces([]string{ns})
+	verifier.SetDstNamespaces([]string{ns})
+	verifier.SetNamespaceMap()
+
+	runner := RunVerifierCheck(ctx, suite.T(), verifier)
+	suite.Require().NoError(runner.AwaitGenerationEnd())
+
+	builder := &strings.Builder{}
+	_, _, err = verifier.reportDocumentMismatches(ctx, builder)
+	suite.Require().NoError(err)
+
+	suite.Assert().Contains(
+		builder.String(),
+		"100009",
+		"summary should show an early mismatch",
+	)
+
+	suite.Assert().Contains(
+		builder.String(),
+		" 10 ",
+		"summary should show the # of missing docs shown",
+	)
+
+	suite.Assert().Contains(
+		builder.String(),
+		" 18 ",
+		"summary should show the total # of missing/changed documents",
+	)
+
+	suite.Assert().NotContains(
+		builder.String(),
+		"100019",
+		"summary should NOT show a late mismatch",
+	)
+}
+
 func (suite *IntegrationTestSuite) TestVerifierCompareIndexSpecs() {
 	ctx := suite.Context()
 	verifier := suite.BuildVerifier()

internal/verifier/mismatches.go

@@ -9,6 +9,7 @@ import (
 	"go.mongodb.org/mongo-driver/bson"
 	"go.mongodb.org/mongo-driver/bson/primitive"
 	"go.mongodb.org/mongo-driver/mongo"
+	"go.mongodb.org/mongo-driver/mongo/options"
 )
 
 const (
@@ -49,6 +50,11 @@ func getMismatchesForTasks(
 		bson.D{
 			{"task", bson.D{{"$in", taskIDs}}},
 		},
+		options.Find().SetSort(
+			bson.D{
+				{"detail.id", 1},
+			},
+		),
 	)
 
 	if err != nil {

internal/verifier/result.go

@@ -36,6 +36,12 @@ type VerificationResult struct {
 	DstTimestamp option.Option[primitive.Timestamp]
 }
 
-func (vr VerificationResult) IsMissing() bool {
-	return vr.Details == Missing
+// DocumentIsMissing returns a boolean that indicates whether the
+// VerificationResult indicates a document that is missing on either
+// source or destination.
+func (vr VerificationResult) DocumentIsMissing() bool {
+	// NB: Missing gets set as the Details value when a field is missing
+	// but the document exists. To ascertain that the document is entirely
+	// absent we have to check Field as well.
+	return vr.Details == Missing && vr.Field == ""
 }

internal/verifier/summary.go

@@ -144,7 +144,7 @@ func (verifier *Verifier) reportDocumentMismatches(ctx context.Context, strBuild
 		missingCount := lo.CountBy(
 			discrepancies,
 			func(d VerificationResult) bool {
-				return d.IsMissing()
+				return d.DocumentIsMissing()
 			},
 		)
 
@@ -171,7 +171,7 @@ func (verifier *Verifier) reportDocumentMismatches(ctx context.Context, strBuild
 OUTA:
 	for _, task := range failedTasks {
 		for _, d := range taskDiscrepancies[task.PrimaryKey] {
-			if d.IsMissing() {
+			if d.DocumentIsMissing() {
 				continue
 			}
 
@@ -207,14 +207,18 @@ OUTA:
 	printAll = int64(missingOrChangedCount) < (verifier.failureDisplaySize + int64(0.25*float32(verifier.failureDisplaySize)))
 OUTB:
 	for _, task := range failedTasks {
-		for _, _id := range task.Ids {
+		for _, d := range taskDiscrepancies[task.PrimaryKey] {
+			if !d.DocumentIsMissing() {
+				continue
+			}
+
 			if !printAll && missingOrChangedDocsTableRows >= verifier.failureDisplaySize {
 				break OUTB
 			}
 
 			missingOrChangedDocsTableRows++
 			missingOrChangedDocsTable.Append([]string{
-				fmt.Sprintf("%v", _id),
+				fmt.Sprintf("%v", d.ID),
 				fmt.Sprintf("%v", task.QueryFilter.Namespace),
 				fmt.Sprintf("%v", task.QueryFilter.To),
 			})