Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating an entity using wrap(existing).assign({...new_contents}) does not update its BlobType properties. #1406

Closed
jjhbw opened this issue Feb 9, 2021 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@jjhbw
Copy link

jjhbw commented Feb 9, 2021

Thanks for this excellent lib! I found something that may be a bug.

Describe the bug
Updating an entity using wrap(existing).assign({...new_contents}) does not seem to update its BlobType properties. The below code is a representation of what goes on in the backend i'm working on. I am wrap-ing the existing entity and using .assign() to assign the new properties to it. Obviously this can be simplified if you only want to assign a new contents property using book.contents = new_contents, but I also want it to update the other properties that my entity has so I'm using wrap().assign(). Updating the name property through this mechanism (which is just a varchar) seems to work fine.

Be sure to let me know if I'm misunderstanding something here!

To Reproduce

import { BlobType, Entity, MikroORM, PrimaryKey, Property, wrap } from "@mikro-orm/core"

@Entity()
export class Book {
    constructor(name: string, contents: Buffer) {
        this.name = name
        this.contents = contents
    }

    @PrimaryKey()
    id!: number

    @Property({ type: 'varchar' })
    name: string

    @Property({ type: BlobType })
    contents: Buffer
}

const test = async () => {
    const connection = await MikroORM.init({
        entities: [
            Book,
        ],
        dbName: ':memory:',
        type: 'sqlite',
        debug: true,
    })

    const generator = connection.getSchemaGenerator()
    await generator.updateSchema()
    {
        const contents = Buffer.from('abcdefg')
        const book1 = new Book('initial name', contents)
        await connection.em.fork().persistAndFlush(book1)
    }

    const new_contents_value = '123456'
    const new_name = 'updated name'
    {
        const new_version = {
            id: 1,
            name: new_name,
            contents: Buffer.from(new_contents_value),
        }
        const em = connection.em.fork()
        const existing = await em.findOneOrFail(Book, 1)
        const to_save = wrap(existing).assign({ ...new_version }, { mergeObjects: true })
        await em.persistAndFlush(to_save)
    }

    {
        const book1 = await connection.em.fork().findOneOrFail(Book, 1)
        if (book1.name !== new_name) {
            throw new Error(`expected name to be ${new_name} but instead received ${book1.name}`)
        }
        const conts = book1.contents.toString()
        if (conts !== new_contents_value) {
            throw new Error(`expected contents to be ${new_contents_value} but instead received ${conts}`)
        }
    }
}

test().catch(console.log)

Expected behavior
I would expect the BlobType buffer to contain the new value.

Versions

Dependency Version
node v12.20.0
typescript 4.1.3
mikro-orm 4.4.2
your-driver sqlite3 5.0.1
@B4nan
Copy link
Member

B4nan commented Feb 9, 2021

Drop the mergeObjects and it will work.

@B4nan B4nan added the bug Something isn't working label Feb 9, 2021
@jjhbw
Copy link
Author

jjhbw commented Feb 9, 2021

Awesome, thanks. I'll use that for now.

@B4nan
Copy link
Member

B4nan commented Feb 9, 2021

Btw few notes about the code:

const existing = await em.findOneOrFail(Book, 1) // here you load the book - therefore it is a managed entity
const to_save = wrap(existing).assign({ ...new_version }, { mergeObjects: true }) // assign will return the very same entity (`to_save === existing`), also it won't mutate your data
await em.persistAndFlush(to_save) // as the entity is managed, calling flush is enough - only new entities need to be persisted

So in other words, its equivalent to this:

const book = await em.findOneOrFail(Book, 1);
wrap(book).assign(new_version, { mergeObjects: true }); // or even better `em.assign(book, new_version, { mergeObjects: true })`
await em.flush();

@jjhbw
Copy link
Author

jjhbw commented Feb 9, 2021

Thanks again! Very useful to see assign() used in context.

@B4nan B4nan closed this as completed in c5bbcee Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants