msavin / userCache

The one simple trick that can save you millions of database requests
MIT License
35 stars 6 forks source link

Result is not reactive so should not be used within a reactive publish #12

Open wildhart opened 4 years ago

wildhart commented 4 years ago

If Meteor.user([fields]) doesn't hit the cache then the result is _original() which is reactive. However, if it hits the cache then the result is static (not reactive).

This is fine within a method, but if this is used within a reactive-publish, the resulting publication will not react to changes in the specified fields.

Somehow this package needs to detect if it is running inside an autorun and if so always return the _original() reactive function.

A common use-case for reactive-publish is to only publish documents the logged-in user has permission to see, e.g. in a multi-tenancy SaaS application, or in a chat room:

Meteor.publish("usersInChatRoom", function() {
  // Need to put this in an autorun in case user switches room
  this.autorun(function () {
    const user = Meteor.user('roomIds'); // oops, employed userCache out of habit...
    return user
      ? Meteor.users.find({roomIds: {$in: user.roomIds}}, {fields: ...})
      : null;
  });
})

In this example, when a user changes room the publication will not change. This is a contrived example and a bit of an edge-case, but it could happen. So far only luck has saved me from suffering this bug in production - fortunately I was requesting a field which isn't published to the client so userCache was never hitting the cache. If I remove the private field then I can reproduce this bug easily.

wildhart commented 4 years ago

My suggestions for resolving this:

  1. either monkey-patch Tracker.autorun to set a session variable to turn userCache off within that session...; or
  2. monkey-patch Meteor.methods to set a session variable to turn userCache on only with methods.

Or, is there a better way of detecting whether the current fibre is running in a method or publication?

wildhart commented 4 years ago

Had time for a bit more research. Seems you can tell if you're in an autorun with just Tracker.active. The docs say that it's only available on the client, but it works for me on the server even if I uninstall peerlibrary:reactive-publish.

So I suggest changing this:

Meteor.user = function (input) {
    if (typeof input === "undefined") {
        return _original();
    }

to this:

Meteor.user = function (input) {
    if (typeof input === "undefined" || (Tracker && Tracker.active)) {
        return _original();
    }

That fixes it for me.