From df31748148746a29ec7cfcb10c072270cee87791 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 10 Nov 2017 15:50:17 +0000 Subject: [PATCH 1/4] Distinguish between separate percentage rollouts --- .../Features/Project/ProjectController.coffee | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index de323c54f5..07d74538c8 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -24,13 +24,16 @@ AnalyticsManager = require "../Analytics/AnalyticsManager" Sources = require "../Authorization/Sources" TokenAccessHandler = require '../TokenAccess/TokenAccessHandler' CollaboratorsHandler = require '../Collaborators/CollaboratorsHandler' +crypto = require 'crypto' module.exports = ProjectController = - _isInPercentageRollout: (objectId, percentage) -> + _isInPercentageRollout: (rolloutName, objectId, percentage) -> if Settings.bypassPercentageRollouts = true return true - counter = parseInt(objectId.toString().substring(18, 24), 16) + data = "#{rolloutName}:#{objectId.toString()}" + md5hash = crypto.createHash('md5').update(data).digest('hex') + counter = parseInt(md5hash.slice(26, 32), 16) return (counter % 100) < percentage updateProjectSettings: (req, res, next) -> @@ -274,7 +277,7 @@ module.exports = ProjectController = timestamp = parseInt(user_id.toString().substring(0, 8), 16) rolloutPercentage = 60 # Percentage of users to roll out to - if !ProjectController._isInPercentageRollout(user_id, rolloutPercentage) + if !ProjectController._isInPercentageRollout('autocompile', user_id, rolloutPercentage) # Don't show if user is not part of roll out return cb(null, { enabled: false, showOnboarding: false }) userSignupDate = new Date(timestamp * 1000) @@ -305,7 +308,11 @@ module.exports = ProjectController = token = TokenAccessHandler.getRequestToken(req, project_id) isTokenMember = results.isTokenMember - enableTokenAccessUI = ProjectController._isInPercentageRollout(project.owner_ref, 0) + enableTokenAccessUI = ProjectController._isInPercentageRollout( + 'linksharing', + project.owner_ref, + 0 + ) AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel)-> return next(error) if error? if !privilegeLevel? or privilegeLevel == PrivilegeLevels.NONE From 3a4edeaf4c99ba590ab9096dc8c6652ca6922b3d Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 10 Nov 2017 16:13:27 +0000 Subject: [PATCH 2/4] Add a unit test for `_isInPercentageRollout` --- .../Project/ProjectControllerTests.coffee | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee index 6a8a596d72..6d75f9f18e 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee @@ -468,3 +468,56 @@ describe "ProjectController", -> opts.showTrackChangesOnboarding.should.equal false done() @ProjectController.loadEditor @req, @res + + describe '_isInPercentageRollout', -> + before -> + @ids = [ + '5a05cd7621f9fe22be131740', + '5a05cd7821f9fe22be131741', + '5a05cd7921f9fe22be131742', + '5a05cd7a21f9fe22be131743', + '5a05cd7b21f9fe22be131744', + '5a05cd7c21f9fe22be131745', + '5a05cd7d21f9fe22be131746', + '5a05cd7e21f9fe22be131747', + '5a05cd7f21f9fe22be131748', + '5a05cd8021f9fe22be131749', + '5a05cd8021f9fe22be13174a', + '5a05cd8121f9fe22be13174b', + '5a05cd8221f9fe22be13174c', + '5a05cd8221f9fe22be13174d', + '5a05cd8321f9fe22be13174e', + '5a05cd8321f9fe22be13174f', + '5a05cd8421f9fe22be131750', + '5a05cd8421f9fe22be131751', + '5a05cd8421f9fe22be131752', + '5a05cd8521f9fe22be131753' + ] + + it 'should produce the expected results', -> + result = @ids.map (i) => + @ProjectController._isInPercentageRollout('abcd', i, 50) + expect(result).to.deep.equal [ + false, + false, + false, + false, + false, + false, + true, + false, + true, + true, + true, + true, + true, + true, + false, + false, + false, + true, + false, + true + ] + + From 600191b2af8c143f1584871d184e9904208feb9b Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 10 Nov 2017 16:19:43 +0000 Subject: [PATCH 3/4] Ensure that different features produce different results --- .../Project/ProjectControllerTests.coffee | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee index 6d75f9f18e..f358a94171 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee @@ -497,7 +497,10 @@ describe "ProjectController", -> it 'should produce the expected results', -> result = @ids.map (i) => @ProjectController._isInPercentageRollout('abcd', i, 50) - expect(result).to.deep.equal [ + expect( + @ids.map (i) => + @ProjectController._isInPercentageRollout('abcd', i, 50) + ).to.deep.equal [ false, false, false, @@ -518,6 +521,31 @@ describe "ProjectController", -> true, false, true + ] + expect( + @ids.map (i) => + @ProjectController._isInPercentageRollout('efgh', i, 50) + ).to.deep.equal [ + false, + false, + false, + false, + true, + false, + false, + true, + false, + false, + true, + true, + true, + false, + true, + false, + true, + true, + false, + false ] From cac39134cd3893a091bd8fa4ba381136debcd00c Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 10 Nov 2017 16:20:59 +0000 Subject: [PATCH 4/4] Remove stray test lines --- .../test/UnitTests/coffee/Project/ProjectControllerTests.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee index f358a94171..498090f790 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee @@ -495,8 +495,6 @@ describe "ProjectController", -> ] it 'should produce the expected results', -> - result = @ids.map (i) => - @ProjectController._isInPercentageRollout('abcd', i, 50) expect( @ids.map (i) => @ProjectController._isInPercentageRollout('abcd', i, 50)