Skip to content

/ sequelize Public

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

Jump to bottom

model.bulkCreate modifies the "options" by assigning "model" as an attribute to the object #12406

Open
2 tasks done
sandeepraheja opened this issue Jun 22, 2020 · 5 comments
Open
2 tasks done

model.bulkCreate modifies the "options" by assigning "model" as an attribute to the object #12406

sandeepraheja opened this issue Jun 22, 2020 · 5 comments
Labels
good first issue type: bug

Comments

@sandeepraheja
Copy link

@sandeepraheja sandeepraheja commented Jun 22, 2020 •
edited

Issue Description

While using model.bulkCreate, as we pass the options, an object which contains transaction, it comes modified after the execution

What are you doing?

We have called bulkCreate on a model object. The options object which is prepared as an argument to the function, if we check it after execution, it contains the reference to the model object.

Here is the link to the SSCCE for this issue: LINK-HERE 

// You can delete this code block if you have included a link to your SSCCE above!
// MINIMAL, SELF-CONTAINED code here (SSCCE/MCVE/reprex)

What do you expect to happen?

The options object should not have been modified.

What is actually happening?

The options object now have a reference to "model" object.

Output here

Additional context

When we checked the code in sequelize -> model.js, it reflected the assignment on the passed in argument, instead of using the cloned copy of "options"

image

Environment

  • Sequelize version: 5.21.10
  • Node.js version: 12.16.3
  • Operating System: MacOS Catalina, Ubuntu 18.04
  • If TypeScript related: TypeScript version: 4.28.9

Issue Template Checklist

How does this problem relate to dialects?

  • I think this problem happens regardless of the dialect.

Would you be willing to resolve this issue by submitting a Pull Request?

  • No, I don't have the time, although I believe I could do it if I had the time...
@walkerps
Copy link

@walkerps walkerps commented Jun 25, 2020 •
edited

Hey!, It's my first time contributing . I would like to take a stab at this issue.

Please correct me if i'm wrong here , in this issue all we need to do is create a copy of Model's object and assign it to option.model rather than assigning the reference to model's object right?

@sandeepraheja
Copy link
Author

@sandeepraheja sandeepraheja commented Jun 25, 2020

Nope, I think you need to clone options (just like other implementations in the same file) using Object.assign,
Assign the cloned object to options.
and then let the model assignment in options work as it is.

This will make sure the passed in object reference is not polluted

@walkerps
Copy link

@walkerps walkerps commented Jun 25, 2020

oh yeah, thanks. On it

@JuarezLustosa
Copy link
Contributor

@JuarezLustosa JuarezLustosa commented Jul 9, 2020

Hello, @walkerps are still working on this?

@auipc auipc mentioned this issue Jul 18, 2020
Closed
6 tasks
@auipc
Copy link

@auipc auipc commented Jul 18, 2020

@JuarezLustosa I went ahead and raised a fix. Do review it

@LaurentVB LaurentVB mentioned this issue Apr 9, 2021
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue type: bug
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

fix(model): clone options object instead of modifying
5 participants
@JuarezLustosa @sandeepraheja @sushantdhiman @walkerps @auipc