Skip to content
This repository has been archived by the owner on Jan 15, 2020. It is now read-only.

Optimize getUserForContext to fetch the minimun required user fields #118

Open
j0nd0n7 opened this issue Mar 27, 2018 · 3 comments
Open
Labels
feature Feature: new addition or enhancement to existing solutions help wanted

Comments

@j0nd0n7
Copy link

j0nd0n7 commented Mar 27, 2018

For every request made by a logged in user getUserForContext fetch the entire user doc.
Normally only the userId (+ a role flied) is relevant for access controlling.

It would be more performant if getUserForContext fetch the minimum required fields for this package to work properly as well as allowing the developer to configure additional fields to fetch wich are required by the app.

/label feature

@lorensr lorensr added feature Feature: new addition or enhancement to existing solutions help wanted labels Mar 28, 2018
@j0nd0n7
Copy link
Author

j0nd0n7 commented Oct 15, 2018

Hi @lorensr since Meteor "recently" supports $expr by Mongo 3.6 and now 4.0, I'm using this query to delegate token checking to mongo and fetch only the minimum required user fields.

export const getUser = async (loginToken, fields) => {
  if (!loginToken) return null;
  check(loginToken, String);

  const hashedToken = Accounts._hashLoginToken(loginToken);
  const tokenLifetime = Accounts._getTokenLifetimeMs();
  return await Meteor.users.rawCollection().findOne(
    {
      'services.resume.loginTokens.hashedToken': hashedToken,
      $expr: {
        $ne: [
          {
            $size: {
              $ifNull: [
                {
                  $filter: {
                    input: '$services.resume.loginTokens',
                    as: 'token',
                    cond: {
                      $and: [
                        {$eq: ['$$token.hashedToken', hashedToken]},
                        {$lt: [new Date(), {$add: ['$$token.when', tokenLifetime]}]},
                      ],
                    },
                  },
                },
                [],
              ],
            },
          },
          0,
        ],
      },
    },
    {fields},
  );
};

I think it would be a nice addition to integrate something like this using either:

  • an if to check which version of mongo is using the app (I don't know how to check it),
  • provide 2 getUser functions, and let the developer pick the prefered one
  • a flag to let the developer select the checking+fetch method.

Let me know what do you think.
Regards

@lorensr
Copy link
Contributor

lorensr commented Oct 15, 2018

Oh cool! Didn't know you could do that with Mongo. Not sure if the speed improvement is worth the added complexity—perhaps the rare cases in which the JS logic is a speed problem can find this issue and use your code.

I like the fields, maybe as

export const getUser = async (loginToken, { fields } = {}) => {

to provide for future options in the second arg

@j0nd0n7
Copy link
Author

j0nd0n7 commented Oct 18, 2018

Note it's not only js processing, you save the network bandwidth required to retrieve the user object, for all the request that require token verification. Depending on the application, that requests could be the majority.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Feature: new addition or enhancement to existing solutions help wanted
Projects
None yet
Development

No branches or pull requests

2 participants