Fix _attachments not forwarded across chained migration strategies

orga/changelog/fix-migration-attachments-chained-strategies.md

@@ -0,0 +1 @@
+- FIX schema migration not forwarding `_attachments` across chained migration strategies, so later strategies received the document with `_attachments` as `undefined` when an earlier strategy returned a new object without forwarding them, breaking the `WithAttachments<DocData>` contract and preventing strategies from reading or mutating attachment metadata as described in the docs

src/plugins/migration-schema/migration-helpers.ts

@@ -77,6 +77,16 @@ export function migrateDocumentData(
 
     let nextVersion = docSchemaVersion + 1;
 
+    /**
+     * Track attachments across chained strategies so that each strategy
+     * receives the document matching the public `WithAttachments<DocData>`
+     * contract, even when an earlier strategy returned a fresh object that
+     * did not forward `_attachments`. A strategy that explicitly rewrites
+     * `_attachments` still wins because its value is carried forward as
+     * the new baseline.
+     */
+    let currentAttachments = attachmentsBefore;
+
     // run the document through migrationStrategies
     let currentPromise = Promise.resolve(mutateableDocData);
     while (nextVersion <= collection.schema.version) {
@@ -85,7 +95,16 @@ export function migrateDocumentData(
             collection,
             version,
             docOrNull
-        ));
+        )).then(docOrNull => {
+            if (docOrNull !== null) {
+                if (typeof docOrNull._attachments === 'undefined') {
+                    docOrNull._attachments = currentAttachments;
+                } else {
+                    currentAttachments = docOrNull._attachments;
+                }
+            }
+            return docOrNull;
+        });
         nextVersion++;
     }
 
@@ -98,20 +117,6 @@ export function migrateDocumentData(
             doc._meta = meta;
         }
         doc._deleted = deleted;
-        /**
-         * Restore attachments in case a migration strategy returned a fresh
-         * object that did not forward the `_attachments` field. Strategies
-         * receive the document with its attachments already attached, but
-         * only internal fields (`_meta`, `_deleted`) were restored here.
-         * Without re-attaching, attachments would be silently dropped when
-         * a strategy returns a new object, which is a valid public API use.
-         * Strategies that intentionally clear attachments (e.g. by setting
-         * `_attachments = {}` on the old doc) still win because the
-         * returned object already carries the cleared value.
-         */
-        if (typeof doc._attachments === 'undefined') {
-            doc._attachments = attachmentsBefore;
-        }
         return doc;
     });
 }

test/unit/migration-schema.test.ts

@@ -2155,6 +2155,131 @@ describe('migration-schema.test.ts', function () {
             const fetchedData = await attachment.getStringData();
             assert.strictEqual(fetchedData, attachmentData);
 
+            await db2.close();
+        });
+        /**
+         * Regression test for multi-step migrations where an intermediate
+         * migration strategy returns a new object without forwarding
+         * `_attachments`. Subsequent strategies still receive the document
+         * typed as `WithAttachments<DocData>`, so `_attachments` must stay
+         * visible across chained strategies. Without forwarding
+         * `_attachments` between steps, later strategies see it as
+         * `undefined` and can no longer read or mutate attachment metadata,
+         * even though the public-API docs explicitly describe mutating
+         * `oldDoc._attachments` inside a strategy.
+         */
+        it('should preserve _attachments across chained migration strategies', async () => {
+            if (!config.storage.hasAttachments) {
+                return;
+            }
+            if (isDeno && config.storage.name === 'dexie') {
+                return;
+            }
+
+            const dbName = randomToken(10);
+            const schema0 = {
+                version: 0,
+                primaryKey: 'id',
+                type: 'object' as const,
+                properties: {
+                    id: {
+                        type: 'string' as const,
+                        maxLength: 100
+                    },
+                    name: {
+                        type: 'string' as const
+                    }
+                },
+                required: ['id', 'name'] as const,
+                attachments: {}
+            };
+            const schema2 = {
+                version: 2,
+                primaryKey: 'id',
+                type: 'object' as const,
+                properties: {
+                    id: {
+                        type: 'string' as const,
+                        maxLength: 100
+                    },
+                    name: {
+                        type: 'string' as const
+                    },
+                    hadAttachmentInStepTwo: {
+                        type: 'boolean' as const
+                    }
+                },
+                required: ['id', 'name'] as const,
+                attachments: {}
+            };
+
+            // create v0 database and insert a doc with an attachment
+            const db = await createRxDatabase({
+                name: dbName,
+                storage: config.storage.getStorage(),
+            });
+            const cols = await db.addCollections({
+                heroes: { schema: schema0 }
+            });
+            const doc = await cols.heroes.insert({ id: 'alice', name: 'Alice' });
+            const attachmentData = AsyncTestUtil.randomString(20);
+            await doc.putAttachment({
+                id: 'note.txt',
+                data: createBlob(attachmentData, 'text/plain'),
+                type: 'text/plain'
+            });
+            await db.close();
+
+            // reopen with v2 schema using two strategies.
+            // Strategy 1 returns a NEW object without forwarding
+            // `_attachments` (a valid public-API usage: the
+            // MigrationStrategy type is
+            //   `(doc, collection) => WithAttachments<DocData> | null`
+            // and `_attachments` is optional on that type).
+            // Strategy 2 must still receive `_attachments` to comply
+            // with the same contract and the docs that show
+            // mutating `oldDoc._attachments` inside a strategy.
+            const db2 = await createRxDatabase({
+                name: dbName,
+                storage: config.storage.getStorage(),
+            });
+            const cols2 = await db2.addCollections({
+                heroes: {
+                    schema: schema2,
+                    migrationStrategies: {
+                        1: (oldDoc: any) => {
+                            return {
+                                id: oldDoc.id,
+                                name: oldDoc.name
+                            };
+                        },
+                        2: (oldDoc: any) => {
+                            const hadAttachment = !!oldDoc._attachments
+                                && Object.keys(oldDoc._attachments).length > 0;
+                            return {
+                                id: oldDoc.id,
+                                name: oldDoc.name,
+                                hadAttachmentInStepTwo: hadAttachment
+                            };
+                        }
+                    }
+                }
+            });
+
+            const migratedDoc = await cols2.heroes.findOne('alice').exec(true);
+            assert.strictEqual(
+                (migratedDoc as any).hadAttachmentInStepTwo,
+                true,
+                'strategy 2 must see _attachments even when strategy 1 returned a new object'
+            );
+
+            const attachment = migratedDoc.getAttachment('note.txt');
+            assert.ok(attachment, 'attachment must still be present after migration');
+            assert.strictEqual(attachment.type, 'text/plain');
+            assert.strictEqual(attachment.length, attachmentData.length);
+            const fetchedData = await attachment.getStringData();
+            assert.strictEqual(fetchedData, attachmentData);
+
             await db2.close();
         });
     });