TOOLS-3697 Fix mongorestore’s import of indexes.

buildscript/sa.go

@@ -33,7 +33,7 @@ const (
 	golinesVersion      = "0.12.2"
 	gosecVersion        = "2.20.0"
 	preciousVersion     = "0.7.2"
-	ubiVersion          = "0.0.18"
+	ubiVersion          = "0.0.29"
 	prettierVersion     = "3.3.1"
 )
 

common/bsonutil/indexes.go

@@ -80,47 +80,16 @@ func IsIndexKeysEqual(indexKey1 bson.D, indexKey2 bson.D) bool {
 // will cause the index build to fail. See TOOLS-2412 for more information.
 //
 // This function logs the keys that are converted.
+//
+// For a “quiet” variant of this function, see NormalizeIndexKeyValue.
 func ConvertLegacyIndexKeys(indexKey bson.D, ns string) {
 	var converted bool
 	originalJSONString := CreateExtJSONString(indexKey)
 	for j, elem := range indexKey {
-		switch v := elem.Value.(type) {
-		case int:
-			if v == 0 {
-				indexKey[j].Value = int32(1)
-				converted = true
-			}
-		case int32:
-			if v == int32(0) {
-				indexKey[j].Value = int32(1)
-				converted = true
-			}
-		case int64:
-			if v == int64(0) {
-				indexKey[j].Value = int32(1)
-				converted = true
-			}
-		case float64:
-			if math.Abs(v-float64(0)) < epsilon {
-				indexKey[j].Value = int32(1)
-				converted = true
-			}
-		case primitive.Decimal128:
-			if bi, _, err := v.BigInt(); err == nil {
-				if bi.Cmp(big.NewInt(0)) == 0 {
-					indexKey[j].Value = int32(1)
-					converted = true
-				}
-			}
-		case string:
-			// Only convert an empty string
-			if v == "" {
-				indexKey[j].Value = int32(1)
-				converted = true
-			}
-		default:
-			// Convert all types that aren't strings or numbers
-			indexKey[j].Value = int32(1)
+		newValue, convertedThis := ConvertLegacyIndexKeyValue(elem.Value)
+
+		if convertedThis {
+			indexKey[j].Value = newValue
 			converted = true
 		}
 	}
@@ -136,6 +105,52 @@ func ConvertLegacyIndexKeys(indexKey bson.D, ns string) {
 	}
 }
 
+// ConvertLegacyIndexKeyValue provides ConvertLegacyIndexKeys’s implementation
+// without logging or mutating inputs. It just returns the normalized value
+// and a boolean that indicates whether the value was normalized/converted.
+func ConvertLegacyIndexKeyValue(value any) (any, bool) {
+	var needsConversion bool
+
+	switch v := value.(type) {
+	case int:
+		if v == 0 {
+			needsConversion = true
+		}
+	case int32:
+		if v == int32(0) {
+			needsConversion = true
+		}
+	case int64:
+		if v == int64(0) {
+			needsConversion = true
+		}
+	case float64:
+		if math.Abs(v-float64(0)) < epsilon {
+			needsConversion = true
+		}
+	case primitive.Decimal128:
+		if bi, _, err := v.BigInt(); err == nil {
+			if bi.Cmp(big.NewInt(0)) == 0 {
+				needsConversion = true
+			}
+		}
+	case string:
+		// Only convert an empty string
+		if v == "" {
+			needsConversion = true
+		}
+	default:
+		// Convert all types that aren't strings or numbers
+		needsConversion = true
+	}
+
+	if needsConversion {
+		value = int32(1)
+	}
+
+	return value, needsConversion
+}
+
 // ConvertLegacyIndexOptions removes options that don't match a known list of index options.
 // It is preferable to use the ignoreUnknownIndexOptions on the createIndex command to
 // force the server to do this task. But that option was only added in 4.1.9. So for

common/idx/idx.go

@@ -9,6 +9,8 @@ package idx
 import (
 	"fmt"
 
+	"github.com/mongodb/mongo-tools/common/bsonutil"
+	"github.com/mongodb/mongo-tools/common/util"
 	"go.mongodb.org/mongo-driver/bson"
 )
 
@@ -46,3 +48,36 @@ func NewIndexDocumentFromD(doc bson.D) (*IndexDocument, error) {
 
 	return &indexDoc, nil
 }
+
+// IsDefaultIdIndex indicates whether the IndexDocument represents its
+// collection’s default _id index.
+func (id *IndexDocument) IsDefaultIdIndex() bool {
+
+	// Default indexes can’t have partial filters.
+	if id.PartialFilterExpression != nil {
+		return false
+	}
+
+	indexKeyIsIdOnly := len(id.Key) == 1 && id.Key[0].Key == "_id"
+
+	if !indexKeyIsIdOnly {
+		return false
+	}
+
+	// We need to ignore special indexes like hashed or 2dsphere. Historically
+	// “non-special” indexes weren’t always persisted with 1 as the value,
+	// so before we check for “special” we normalize.
+	normalizedVal, _ := bsonutil.ConvertLegacyIndexKeyValue(id.Key[0].Value)
+
+	// Default indexes are always { _id:1 }. They’re probably always int32(1),
+	// but let’s be more permissive than that.
+	normalizedAsF64, err := util.ToFloat64(normalizedVal)
+
+	// An error here just means that the value can‘t be cast to a float64
+	// (e.g., is a string).
+	if err != nil {
+		return false
+	}
+
+	return normalizedAsF64 == 1
+}

common/idx/idx_test.go

@@ -0,0 +1,49 @@
+package idx
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"go.mongodb.org/mongo-driver/bson"
+)
+
+func TestIsDefaultIdIndex(t *testing.T) {
+	cases := []struct {
+		Document  IndexDocument
+		IsDefault bool
+	}{
+		{
+			Document: IndexDocument{
+				Key: bson.D{{"_id", int32(1)}},
+			},
+			IsDefault: true,
+		},
+		{
+			Document: IndexDocument{
+				Key: bson.D{{"_id", 1}},
+			},
+			IsDefault: true,
+		},
+		{
+			Document: IndexDocument{
+				Key: bson.D{{"_id", ""}}, // legacy
+			},
+			IsDefault: true,
+		},
+		{
+			Document: IndexDocument{
+				Key: bson.D{{"_id", "hashed"}},
+			},
+			IsDefault: false,
+		},
+	}
+
+	for _, curCase := range cases {
+		assert.Equal(
+			t,
+			curCase.IsDefault,
+			curCase.Document.IsDefaultIdIndex(),
+			"%+v", curCase.Document,
+		)
+	}
+}

mongorestore/mongorestore_test.go

@@ -27,10 +27,12 @@ import (
 	"github.com/mongodb/mongo-tools/common/testtype"
 	"github.com/mongodb/mongo-tools/common/testutil"
 	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.mongodb.org/mongo-driver/bson"
 	"go.mongodb.org/mongo-driver/bson/primitive"
 	"go.mongodb.org/mongo-driver/mongo"
+	moptions "go.mongodb.org/mongo-driver/mongo/options"
 	"go.mongodb.org/mongo-driver/mongo/writeconcern"
 )
 
@@ -2854,6 +2856,113 @@ func TestRestoreColumnstoreIndex(t *testing.T) {
 	})
 }
 
+func TestRestoreMultipleIDIndexes(t *testing.T) {
+	testtype.SkipUnlessTestType(t, testtype.IntegrationTestType)
+
+	session, err := testutil.GetBareSession()
+	require.NoError(t, err, "can connect to server")
+
+	dbName := uniqueDBName()
+	testDB := session.Database(dbName)
+
+	coll := testDB.Collection("mycoll")
+
+	ctx := context.Background()
+
+	cases := []struct {
+		Label   string
+		Indexes []mongo.IndexModel
+	}{
+		{
+			Label: "multiple hashed + 2dsphere",
+			Indexes: []mongo.IndexModel{
+				{Keys: bson.D{{"_id", "hashed"}}},
+				{
+					Keys: bson.D{{"_id", "hashed"}},
+					Options: moptions.Index().
+						SetName("_id_hashed_de").
+						SetCollation(&moptions.Collation{Locale: "de"}),
+				},
+				{
+					Keys: bson.D{{"_id", "hashed"}},
+					Options: moptions.Index().
+						SetName("_id_hashed_ar").
+						SetCollation(&moptions.Collation{Locale: "ar"}),
+				},
+				{Keys: bson.D{{"_id", "2dsphere"}}},
+			},
+		},
+	}
+
+	for _, curCase := range cases {
+		indexesToCreate := curCase.Indexes
+
+		t.Run(
+			curCase.Label,
+			func(t *testing.T) {
+				_, err = coll.Indexes().CreateMany(ctx, indexesToCreate)
+				require.NoError(t, err, "indexes should be created")
+
+				archivedIndexes := []bson.M{}
+				require.NoError(t, listIndexes(ctx, coll, &archivedIndexes), "should list indexes")
+
+				withBSONMongodumpForCollection(t, testDB.Name(), "mycoll", func(dir string) {
+					restore, err := getRestoreWithArgs(
+						DropOption,
+						dir,
+					)
+					require.NoError(t, err)
+					defer restore.Close()
+
+					result := restore.Restore()
+					require.NoError(
+						t,
+						result.Err,
+						"%s: mongorestore should finish OK",
+						curCase.Label,
+					)
+					require.EqualValues(
+						t,
+						0,
+						result.Failures,
+						"%s: mongorestore should report 0 failures",
+						curCase.Label,
+					)
+				})
+
+				restoredIndexes := []bson.M{}
+				require.NoError(t, listIndexes(ctx, coll, &restoredIndexes), "should list indexes")
+
+				assert.ElementsMatch(
+					t,
+					archivedIndexes,
+					restoredIndexes,
+					"indexes should round-trip dump/restore",
+				)
+			},
+		)
+
+	}
+}
+
+// ListSpecifications returns IndexSpecifications, which don’t describe all
+// parts of the index. So we need to List() the indexes directly and marshal
+// them to something that lets us compare everything.
+func listIndexes[T any](ctx context.Context, coll *mongo.Collection, target *T) error {
+	ns := coll.Database().Name() + "." + coll.Name()
+
+	cursor, err := coll.Indexes().List(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to start listing indexes for %#q: %w", ns, err)
+	}
+	err = cursor.All(ctx, target)
+	if err != nil {
+		return fmt.Errorf("failed to list indexes for %#q: %w", ns, err)
+	}
+
+	return nil
+}
+
 // testRestoreColumnstoreIndexFromDump tests restoring Columnstore Indexes from dump files.
 func testRestoreColumnstoreIndexFromDump(t *testing.T) {
 	require := require.New(t)