Meteor-Community-Packages / meteor-collection-hooks

Meteor Collection Hooks
https://atmospherejs.com/matb33/collection-hooks
MIT License
657 stars 92 forks source link

Do collection hooks work on the server #109

Closed ghost closed 9 years ago

ghost commented 9 years ago

If you insert on the server, are the hooks meant to run? becuase mine doesnt

if(Meteor.isServer)
    Posts.insert({name: 'test'}); // does this cause the hooks to run?
dstollie commented 9 years ago

Same for me..

matb33 commented 9 years ago

They do -- if you can paste more of your code we can help you track down what to change

dstollie commented 9 years ago

I was being stupid by creating two insert hooks: one which incremented a counter and one which decremented it. This caused to counter unchanged. No probs which the package for me!

zimme commented 9 years ago

@MurWade, Maybe you've defined the hook in client code and run the insert in server code, then the hook won't run.

ghost commented 9 years ago

I have defined my hooks in common code. so client and server. I am using simple schema to combine maybe that is an issue. I will link a repo so you can reproduce it. The code runs twice when i insert from client which is expected but as soon as i insert from server no hooks run.

7etoiles commented 9 years ago

I also use simple schema in combination with collection hooks. The hooks are not called.

Collection-hooks are set up in my common code. I only use collection.insert in the server code.

7etoiles commented 9 years ago

To confirm: if I do not attach a simple schema to a collection, the hooks are called properly.

I.e. setting up multiple before.insert or before.update hooks and they are all called in the order I have attached them.

What I can see is that SimpleSchema performs its magic before collection hooks. For example, I have an insertDateTime timestamp field in the schema definition. It has denyUpdate : true set in the schema. Upon updating a collection to which this schema is attached, and having the insertDateTime in the data, SimpleSchema says the data does not validate which is correct. The collection hooks never get called in this case.

What I would like is to have the collection hooks fire before SimpleSchema. Is that possible?

zimme commented 9 years ago

If validation fails then there is no insert/update and if there isn't an insert/update the before.insert/update hooks won't run. Which I feel is correct.

wursttheke commented 9 years ago

@florasolutions you could manually validate against the schema inside a collection hook. That way you can control when which hook is fired.

7etoiles commented 9 years ago

@wursttheke I also validate in a method before calling insert/update. The result of the validation is positive at that point, as simple schema does not know whether its an insert or update, but when calling insert/update collection2/simple schema again validates, but now it knows if it's an insert or update. I can never validate in a hook in the above case.

@zimme that is true. Maybe a better hook for me would be before.insert.validation and before.update.validation to have it more semantically correct. I just have some generic processing to do before inserting/updating on common fields available in all my collections and I thought hooks were the best place to do this in order to reduce repeated code.

zimme commented 9 years ago

It still is, but it shouldn't do the processing if there isn't an insert/update because of failed validation, right?

So if the validation passes then the insert/update goes through and your hook with the processing is run.

matb33 commented 9 years ago

Out of curiosity, could the validation not be done within allow/deny?

darkship commented 9 years ago

Hello,

@matb33 There is definitely something with the hooks. In this repo https://github.com/darkship/test_hooks_with_update_fail, I do an update in the before.update hook but then after.update is never called.

matb33 commented 9 years ago

@darkship thanks for the repo. I took a quick look. Did you mean to forbid access for the client to the item2 collection? The hook behavior looks completely fine -- you may have simply made a mistake with regards to when the hooks will run (you defined them on both server and client, so you have to pay extra special attention to how things will go down).

What you're encountering is very similar to what many have experienced, and I think it's a matter of having better explanations in the readme (as per https://github.com/matb33/meteor-collection-hooks/issues/65). Another issue where the user was having issues like yours is: https://github.com/matb33/meteor-collection-hooks/issues/95

In short, what's happening is that the complex interplay between server- and client-defined hooks, mixed with allow/deny, can be hard to reason about. In your case @darkship, you're calling that update from within a client-side hook, but the client doesn't have write access and is thus denied. But yours is a fairly simple example, and in the case of #95, it became much harder to reason about, but it comes down to something similar.

I'm not sure how to go about documenting this and having it clear for everyone. Sometimes I wonder if client-side hooks should be discouraged, just to keep things clearer in people's heads...

darkship commented 9 years ago

I was too quick in my explanation. Sorry.

I didn't care of client side but I updated my repo to allow inserts and updates on item2. So when I run it, there is no function after update in my shell neither in my client's console.

matb33 commented 9 years ago

@darkship thanks for this. You helped me uncover a typo that was certainly at the root of many strange side-effects. I'm releasing an update in a few minutes.

darkship commented 9 years ago

@matb33 it works, thanks.