diff --git a/server/cmd/rename-rollback/main.go b/server/cmd/rename-rollback/main.go index bb8dee8..8ad1834 100644 --- a/server/cmd/rename-rollback/main.go +++ b/server/cmd/rename-rollback/main.go @@ -41,6 +41,15 @@ func main() { db := client.Database(*dbName) + // Drop indexes keyed on instance_id first, for the same reason the forward + // migration drops the org_id ones: a unique index treats the missing field + // as null and rejects the second document the rename touches. + for _, c := range services.ScopedCollections { + if err := services.DropIndexesKeyedOn(ctx, db, c, "instance_id"); err != nil { + log.Fatalf("%v", err) + } + } + for _, c := range services.ScopedCollections { res, err := db.Collection(c).UpdateMany(ctx, bson.M{"instance_id": bson.M{"$exists": true}}, diff --git a/server/internal/services/migrate_instance.go b/server/internal/services/migrate_instance.go index 7bbbc31..0aff678 100644 --- a/server/internal/services/migrate_instance.go +++ b/server/internal/services/migrate_instance.go @@ -93,7 +93,22 @@ func MigrateOrgToInstance(ctx context.Context, db *mongo.Database) error { log.Printf("0004: renamed collection %s to %s", r.from, r.to) } - // Step 2: rename the field. + // Step 2: drop indexes keyed on the old field name, BEFORE renaming it. + // + // Order matters and is not obvious. A unique index on org_id treats a + // missing org_id as null, so as soon as $rename strips the field from the + // second document the index reports a duplicate null and the whole update + // fails. Dropping first avoids that entirely. + // + // Dropping an index touches no documents. The boot-time index builders + // recreate the current ones against the new field name. + for _, c := range ScopedCollections { + if err := DropIndexesKeyedOn(ctx, db, c, "org_id"); err != nil { + return err + } + } + + // Step 3: rename the field. for _, c := range ScopedCollections { res, err := db.Collection(c).UpdateMany(ctx, bson.M{"org_id": bson.M{"$exists": true}}, @@ -107,7 +122,7 @@ func MigrateOrgToInstance(ctx context.Context, db *mongo.Database) error { } } - // Step 3: verify before anyone records a marker. Any mismatch aborts, and + // Step 4: verify before anyone records a marker. Any mismatch aborts, and // the migration is re-run rather than marked done. for _, c := range ScopedCollections { total, err := db.Collection(c).CountDocuments(ctx, bson.M{}) @@ -135,38 +150,62 @@ func MigrateOrgToInstance(ctx context.Context, db *mongo.Database) error { } } - // Step 4: indexes keyed on the old field name now point at a field that no - // longer exists. Drop them; the boot-time index builders recreate the - // current ones. Dropping an index touches no documents. - for _, c := range ScopedCollections { - cur, err := db.Collection(c).Indexes().List(ctx) - if err != nil { - return fmt.Errorf("list indexes on %s: %w", c, err) - } - var specs []bson.M - if err := cur.All(ctx, &specs); err != nil { - return fmt.Errorf("decode indexes on %s: %w", c, err) - } - for _, s := range specs { - name, _ := s["name"].(string) - if name == "_id_" { - continue + log.Printf("0004: verified %d collection(s)", len(ScopedCollections)) + return nil +} + +// IndexKeyedOn reports whether an index specification's key document mentions +// field. +// +// The key is checked as both bson.D and bson.M because the driver's decoding of +// a nested document depends on the target type, and getting this wrong is +// silent: the index simply is not found, and the field rename then fails on a +// duplicate null. +func IndexKeyedOn(key any, field string) bool { + switch k := key.(type) { + case bson.M: + _, ok := k[field] + return ok + case bson.D: + for _, e := range k { + if e.Key == field { + return true } - keys, ok := s["key"].(bson.M) - if !ok { - continue - } - if _, keyed := keys["org_id"]; !keyed { - continue - } - if err := db.Collection(c).Indexes().DropOne(ctx, name); err != nil { - return fmt.Errorf("drop index %s on %s: %w", name, c, err) - } - log.Printf("0004: dropped stale index %s on %s", name, c) } } + return false +} - log.Printf("0004: verified %d collection(s)", len(ScopedCollections)) +// DropIndexesKeyedOn removes every index on coll whose key mentions field, +// leaving _id_ alone. +// +// Both the migration and its rollback must do this BEFORE renaming the field. A +// unique index treats a missing field as null, so as soon as $rename strips the +// field from the second document the index reports a duplicate null and the +// whole update fails. Dropping an index touches no documents; the boot-time +// index builders recreate what is needed. +func DropIndexesKeyedOn(ctx context.Context, db *mongo.Database, coll, field string) error { + cur, err := db.Collection(coll).Indexes().List(ctx) + if err != nil { + return fmt.Errorf("list indexes on %s: %w", coll, err) + } + var specs []bson.M + if err := cur.All(ctx, &specs); err != nil { + return fmt.Errorf("decode indexes on %s: %w", coll, err) + } + for _, s := range specs { + name, _ := s["name"].(string) + if name == "_id_" { + continue + } + if !IndexKeyedOn(s["key"], field) { + continue + } + if err := db.Collection(coll).Indexes().DropOne(ctx, name); err != nil { + return fmt.Errorf("drop index %s on %s: %w", name, coll, err) + } + log.Printf("dropped stale index %s on %s", name, coll) + } return nil }