vadimdemedes / mongorito

🍹 MongoDB ODM for Node.js apps based on Redux
1.38k stars 90 forks source link

Hooks running more than once #113

Closed ghost closed 7 years ago

ghost commented 8 years ago

I have a user model that runs three hooks before save: validateEmail, validatePassword, encryptPassword.

In the test file, I do:

const user = new User({ email: 'user@example.com', password: '' }) yield user.save() // assert throws error user.set('password', 'Example123') yield user.save() // test should pass

But then the hooks run like this: Validate email Validate password Encrypt password Validate email Validate password

So when the encrypt password hook is called, a hashed version of the password is set using

user.set('password', yield bcrypt.hash(this.get('password'), 10))

the validate password hook fails because password is too long.

screen shot 2016-05-02 at 10 34 34 screen shot 2016-05-02 at 10 34 58 screen shot 2016-05-02 at 10 35 11 screen shot 2016-05-02 at 15 04 08

vadimdemedes commented 8 years ago

But then the hooks run like this: Validate email Validate password Encrypt password Validate email Validate password

It looks correct. What is the issue specifically?

ghost commented 8 years ago

It is duplicating hooks... It should run Validate Email => Validate Password => Encrypt Password. But instead it is running Validate Email and Validate Password after Encrypt Password. Encrypt Password should be the last hook to run

vadimdemedes commented 8 years ago

Could you do a PR with a failing test?

zaaack commented 8 years ago

https://github.com/vdemedes/mongorito/blob/master/lib/mongorito.js#L521

I just find out the _getHooks() method might cause a leak when there are hooks in save action, would it be related?

zaaack commented 8 years ago

@vdemedes And there are a lot of test failures in the main branch, is that OK?

zaaack commented 8 years ago

Looks like mongodb client

vadimdemedes commented 7 years ago

Should be fixed now thanks to @zaaack. Published on npm as 2.2.0.