Merge pull request #5479 from overleaf/bg-refresh-features-on-editor-load

refresh user features on editor load when out of date

GitOrigin-RevId: ef39b5626cfdc6ed611137a6f6eca3417d3ce73f

services/web/app/src/Features/Project/ProjectController.js

@@ -39,6 +39,7 @@ const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const Modules = require('../../infrastructure/Modules')
 const SplitTestV2Handler = require('../SplitTests/SplitTestV2Handler')
 const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI')
+const FeaturesUpdater = require('../Subscription/FeaturesUpdater')
 
 const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => {
   if (!affiliation.institution) return false
@@ -661,16 +662,21 @@ const ProjectController = {
           } else {
             User.findById(
               userId,
-              'email first_name last_name referal_id signUpDate featureSwitches features refProviders alphaProgram betaProgram isAdmin ace',
+              'email first_name last_name referal_id signUpDate featureSwitches features featuresEpoch refProviders alphaProgram betaProgram isAdmin ace',
               (err, user) => {
                 // Handle case of deleted user
                 if (user == null) {
                   UserController.logout(req, res, next)
                   return
                 }
-
+                if (err) {
+                  return cb(err)
+                }
                 logger.log({ projectId, userId }, 'got user')
-                cb(err, user)
+                if (FeaturesUpdater.featuresEpochIsCurrent(user)) {
+                  return cb(null, user)
+                }
+                ProjectController._refreshFeatures(req, user, cb)
               }
             )
           }
@@ -893,6 +899,59 @@ const ProjectController = {
     )
   },
 
+  _refreshFeatures(req, user, callback) {
+    // If the feature refresh has failed in this session, don't retry
+    // it - require the user to log in again.
+    if (req.session.feature_refresh_failed) {
+      metrics.inc('features-refresh', 1, {
+        path: 'load-editor',
+        status: 'skipped',
+      })
+      return callback(null, user)
+    }
+    // If the refresh takes too long then return the current
+    // features. Note that the user.features property may still be
+    // updated in the background after the callback is called.
+    callback = _.once(callback)
+    const refreshTimeoutHandler = setTimeout(() => {
+      req.session.feature_refresh_failed = { reason: 'timeout', at: new Date() }
+      metrics.inc('features-refresh', 1, {
+        path: 'load-editor',
+        status: 'timeout',
+      })
+      callback(null, user)
+    }, 5000)
+    // try to refresh user features now
+    const timer = new metrics.Timer('features-refresh-on-load-editor')
+    FeaturesUpdater.refreshFeatures(
+      user._id,
+      'load-editor',
+      (err, features) => {
+        clearTimeout(refreshTimeoutHandler)
+        timer.done()
+        if (err) {
+          // keep a record to prevent unneceary retries and leave
+          // the original features unmodified if the refresh failed
+          req.session.feature_refresh_failed = {
+            reason: 'error',
+            at: new Date(),
+          }
+          metrics.inc('features-refresh', 1, {
+            path: 'load-editor',
+            status: 'error',
+          })
+        } else {
+          user.features = features
+          metrics.inc('features-refresh', 1, {
+            path: 'load-editor',
+            status: 'success',
+          })
+        }
+        return callback(null, user)
+      }
+    )
+  },
+
   _buildProjectList(allProjects, userId) {
     let project
     const {

services/web/app/src/Features/Subscription/FeaturesUpdater.js

@@ -22,6 +22,13 @@ async function scheduleRefreshFeatures(userId, reason) {
   await queue.add({ userId, reason })
 }
 
+/* Check if user features refresh if needed, based on the global featuresEpoch setting */
+function featuresEpochIsCurrent(user) {
+  return Settings.featuresEpoch
+    ? user.featuresEpoch === Settings.featuresEpoch
+    : true
+}
+
 /**
  * Refresh features for the given user
  */
@@ -197,6 +204,7 @@ function _getMatchedFeatureSet(features) {
 }
 
 module.exports = {
+  featuresEpochIsCurrent,
   computeFeatures: callbackify(computeFeatures),
   refreshFeatures: callbackifyMultiResult(refreshFeatures, [
     'features',

services/web/app/src/Features/Subscription/UserFeaturesUpdater.js

@@ -1,5 +1,6 @@
 const { User } = require('../../models/User')
 const { promisifyAll } = require('../../util/promises')
+const Settings = require('@overleaf/settings')
 
 function _featuresChanged(newFeatures, featuresBefore) {
   for (const feature in newFeatures) {
@@ -15,6 +16,10 @@ module.exports = {
     const update = {
       featuresUpdatedAt: new Date(),
     }
+    // record the system-wide features epoch, if defined
+    if (Settings.featuresEpoch) {
+      update.featuresEpoch = Settings.featuresEpoch
+    }
     for (const key in features) {
       const value = features[key]
       update[`features.${key}`] = value

services/web/app/src/models/User.js

@@ -135,6 +135,9 @@ const UserSchema = new Schema({
     },
   ],
   featuresUpdatedAt: { type: Date },
+  featuresEpoch: {
+    type: String,
+  },
   // when auto-merged from SL and must-reconfirm is set, we may end up using
   // `sharelatexHashedPassword` to recover accounts...
   sharelatexHashedPassword: String,

services/web/config/settings.defaults.js

@@ -296,6 +296,8 @@ module.exports = {
     trackChanges: true,
   }),
 
+  // featuresEpoch: 'YYYY-MM-DD',
+
   features: {
     personal: defaultFeatures,
   },

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -105,6 +105,10 @@ describe('ProjectController', function () {
     this.Features = {
       hasFeature: sinon.stub(),
     }
+    this.FeaturesUpdater = {
+      featuresEpochIsCurrent: sinon.stub().returns(true),
+      refreshFeatures: sinon.stub().yields(null, this.user),
+    }
     this.BrandVariationsHandler = {
       getBrandVariationById: sinon
         .stub()
@@ -167,6 +171,7 @@ describe('ProjectController', function () {
         '../Collaborators/CollaboratorsGetter': this.CollaboratorsGetter,
         './ProjectEntityHandler': this.ProjectEntityHandler,
         '../../infrastructure/Features': this.Features,
+        '../Subscription/FeaturesUpdater': this.FeaturesUpdater,
         '../Notifications/NotificationsBuilder': this.NotificationBuilder,
         '../User/UserGetter': this.UserGetter,
         '../BrandVariations/BrandVariationsHandler': this
@@ -1083,6 +1088,18 @@ describe('ProjectController', function () {
       this.ProjectController.loadEditor(this.req, this.res)
     })
 
+    it('should refresh the user features if the epoch is outdated', function (done) {
+      this.FeaturesUpdater.featuresEpochIsCurrent = sinon.stub().returns(false)
+      this.res.render = () => {
+        this.FeaturesUpdater.refreshFeatures.should.have.been.calledWith(
+          this.user._id,
+          'load-editor'
+        )
+        done()
+      }
+      this.ProjectController.loadEditor(this.req, this.res)
+    })
+
     describe('pdf caching feature flags', function () {
       /* eslint-disable mocha/no-identical-title */
       function showNoVariant() {

services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js

@@ -31,6 +31,7 @@ describe('UserFeaturesUpdater', function () {
         '../../models/User': {
           User: this.User,
         },
+        '@overleaf/settings': (this.Settings = {}),
       },
     })
   })
@@ -58,6 +59,31 @@ describe('UserFeaturesUpdater', function () {
           )
           expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true
           features.should.deep.equal(update)
+          expect(updateArgs[1].featuresEpoch).to.be.undefined
+          done()
+        }
+      )
+    })
+    it('should set the featuresEpoch when present', function (done) {
+      const userId = '5208dd34438842e2db000005'
+      const update = {
+        versioning: true,
+      }
+      this.Settings.featuresEpoch = 'epoch-1'
+      this.UserFeaturesUpdater.updateFeatures(
+        userId,
+        update,
+        (err, features) => {
+          const updateArgs = this.User.findByIdAndUpdate.lastCall.args
+          expect(updateArgs[0]).to.deep.equal(userId)
+          assert.equal(err, null)
+          expect(Object.keys(updateArgs[1]).length).to.equal(3)
+          expect(updateArgs[1]['features.versioning']).to.equal(
+            update.versioning
+          )
+          expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true
+          features.should.deep.equal(update)
+          expect(updateArgs[1].featuresEpoch).to.equal('epoch-1')
           done()
         }
       )