ravendb / ravendb-nodejs-client

RavenDB node.js client
MIT License
63 stars 32 forks source link

@nested-object-types doesn’t store type information for newly added fields on update #354

Closed jhahn closed 1 year ago

jhahn commented 1 year ago

Exposition

We’re loading existing documents and setting “new” fields to Date values. (e.g. fields that didn’t exist on the document when it was created)

When those documents are saved, their metadata is not updated with the type information for the newly added fields.

On subsequent loads, Date values are not cast, and instead returned as stings.

This seems like an oversight, as type information is correctly extracted and stored on create.

Failing test case

    it("sets correct nested-object-types", async () => {
        class DateDoc {
            public firstDate: Date
            public secondDate: Date
        }
        {
            const session = store.openSession();
            const dateDoc = new DateDoc();
            dateDoc.firstDate = new Date();

            await session.store(dateDoc, "date/1");
            await session.saveChanges();
        }
        const session = store.openSession()
        const doc = await session.load<DateDoc>("date/1")
        doc.secondDate = new Date()
        await session.saveChanges()
        const metadata = session.advanced.getMetadataFor(doc)
        assert.strictEqual(doc.firstDate instanceof Date, true)
        assert.strictEqual(doc.secondDate instanceof Date, true)
        assert.strictEqual(metadata["@nested-object-types"]["firstDate"], "date")
        assert.strictEqual(metadata["@nested-object-types"]["secondDate"], "date")

        const session2 = store.openSession()
        const loaded = await session2.load<DateDoc>("date/1")
        const loadedMetadata = session2.advanced.getMetadataFor(loaded)
        assert.strictEqual(loaded.firstDate instanceof Date, true)
        assert.strictEqual(loaded.secondDate instanceof Date, true)
        assert.strictEqual(loadedMetadata["@nested-object-types"]["firstDate"], "date")
        assert.strictEqual(loadedMetadata["@nested-object-types"]["secondDate"], "date")
    });
jhahn commented 1 year ago

Did a little more digging.

It looks like a variable scoping issue. When the object key matches the metadata constant (@metadata), skipTypes is clobbered, forever set to true until the recursive function finishes.

https://github.com/ravendb/ravendb-nodejs-client/blob/70834f630ee18e43e36e027da6549666b203457b/src/Mapping/ObjectMapper.ts#L449-L454

We were able to work around the issue by making this change:

let innerSkipTypes = skipTypes
if (!skipTypes) {
    innerSkipTypes = key === CONSTANTS.Documents.Metadata.KEY;
}

const fullPath = objPathPrefix ? `${objPathPrefix}.${nestedTypeInfoKey}` : nestedTypeInfoKey;
result[key] = this._makeObjectLiteral(obj[key], fullPath, typeInfoCallback, knownTypes, innerSkipTypes);
ml054 commented 1 year ago

https://github.com/ravendb/ravendb-nodejs-client/pull/357

ml054 commented 1 year ago

@jhahn Fixed. This for reporting this issue and very clear explanation and solution suggestion.