-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
after upgrading to 0.8.2 Collections are empty in helper method after an insert #2275
Comments
Im experiencing the same behavior in a project of my own. I can recreate the behavior in the sample code by @calculon0 too. You can see this in my sample project here https://github.com/ramishka/meteor-collection-test |
The minimongo calls that cared about waiting for observe callbacks to be fired before returning use drain() on a Meteor._SynchronousQueue to do that wait. But if drain is called from within a task on the same queue, drain returned immediately, which failed to provide the desired semantics. This commit changes drain in this context to throw instead. This would now make a reentrant observeChanges (or mutator) throw, so we fix that by splitting up LocalCollection._observeQueue into two different pieces. One piece is used purely for the above "drain" purpose: this is the stack of queues LocalCollection._observeQueues. Each call to observeChanges/insert/update/remove/resumeObservers gets a new queue which it can drain, and since it's a new queue, the drain call is not within a task on the *same queue*, so the error case doesn't occur. However, just doing this would break the constraint that multiple observe callbacks for the same observe are always called in sequence (even if on the server, one yields). So we use a *separate* queue, which is per-observe (and which never gets drained) for this purpose. (This ensures that the test "mongo-livedata - minimongo observe on server" still passes.) Note that #2275 was made more obvious by df2820f, which caused the common case of findOne inside a helper inside a `#each` to end up dependent on the synchronous observeChanges contract. Reverting that commit does fix our newly added test "minimongo - fetch in observe" but it does not help "minimongo - observe in observe". Fixes #2275. .... .... except that this leads to the unacceptable behavior where the newly-added-in-this-commit test "minimongo - insert in observe" fails. That's because fundamentally, the following things cannot all be simultaneously true: (a) For a given observeChanges calls, its callbacks will be invoked in sequence; they cannot overlap either due to server-side yielding or due to re-entrancy. (b) When LocalCollection.insert() is called, all relevant observe callbacks are invoked before it returns. (c) It is legal to call insert (or another mutator) on a collection from within an observe callback in such a way that it will affect the result of the observed cursor. This commit resolves this issue by making (c) illegal. But this is probably a pretty valid use case. So we are not actually planning to merge this commit. Instead, we should probably relax (b): we should probably not drain the queue anywhere but in observeChanges. This would actually be consistent with how it works on the server, where you have to use a write fence if you'd like to wait for all observes to be called. This is a bigger change than we have time for now, because we'd have to ensure that write fences work on the client, make minimongo interact with them, and rewrite all of our minimongo tests that care about observe timing to use write fences (just like the mongo-livedata tests already do). So for now we will probably "fix" #2275 in a more ad-hoc and targeted way.
This reverts commit c05ae24 EXCEPT for the "fetch in observe" test, which I'm leaving in because we still want a test for #2275. This commit made it an error to start an observeChanges from within an observeChanges callback for the same collection, but it turns out that this is actually a somewhat common thing to do (for example, nested 'each'). Instead, we'll leave things the way they were pre-0.8.2: you can start an observeChanges from within an observeChanges callback, but it'll be subtly buggy in that you won't get synchronous 'added' callbacks. This issue is described in #2315, along with the fact that insert/update/remove/resumeObservers won't run their affected observe callbacks if they are called from within a task on the collection's queue. Eventually we'll implement the full fix (which relaxes the requirement that insert/update/remove run all their callbacks before returning) described in #2315.
I'm running into weird behavior after upgrading to 0.8.2. After inserting an object into a Collection, a helper method tries to find that object in the Collection and display it in a template.
0.8.1.3 works fine. 0.8.2 has an empty Collection in the helper method.
repo: https://github.com/calculon0/empty-collection-in-helper
Clicking the button will add an object into the Collection and it will get displayed in 0.8.1.3. In 0.8.2 and devel, errors are thrown because the Collection is empty.
I narrowed it down to commit df2820f.
The text was updated successfully, but these errors were encountered: