Răsfoiți Sursa

Don't try to fetch user when id doesn't exist

James Allen 12 ani în urmă
părinte
comite
3d1d962501

+ 10 - 2
services/track-changes/app/coffee/UpdatesManager.coffee

@@ -79,7 +79,8 @@ module.exports = UpdatesManager =
 	fillUserInfo: (updates, callback = (error, updates) ->) ->
 	fillUserInfo: (updates, callback = (error, updates) ->) ->
 		users = {}
 		users = {}
 		for update in updates
 		for update in updates
-			users[update.meta.user_id] = true
+			if UpdatesManager._validUserId(update.meta.user_id)
+				users[update.meta.user_id] = true
 
 
 		jobs = []
 		jobs = []
 		for user_id, _ of users
 		for user_id, _ of users
@@ -95,5 +96,12 @@ module.exports = UpdatesManager =
 			for update in updates
 			for update in updates
 				user_id = update.meta.user_id
 				user_id = update.meta.user_id
 				delete update.meta.user_id
 				delete update.meta.user_id
-				update.meta.user = users[user_id]
+				if UpdatesManager._validUserId(user_id)
+					update.meta.user = users[user_id]
 			callback null, updates
 			callback null, updates
+
+	_validUserId: (user_id) ->
+		if !user_id?
+			return false
+		else
+			return !!user_id.match(/^[a-f0-9]{24}$/)

+ 1 - 2
services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee

@@ -33,7 +33,7 @@ describe "Getting updates", ->
 			v: 3
 			v: 3
 		}, {
 		}, {
 			op: [{ i: "two ", p: 4 }]
 			op: [{ i: "two ", p: 4 }]
-			meta: { ts: @to - 2 * @minutes, user_id: @user_id }
+			meta: { ts: @to - 2 * @minutes }
 			v: 4
 			v: 4
 		}, {
 		}, {
 			op: [{ i: "three ", p: 8 }]
 			op: [{ i: "three ", p: 8 }]
@@ -71,6 +71,5 @@ describe "Getting updates", ->
 			meta:
 			meta:
 				start_ts: @to - 2 * @minutes
 				start_ts: @to - 2 * @minutes
 				end_ts: @to - 2 * @minutes
 				end_ts: @to - 2 * @minutes
-				user: @user
 			v: 4
 			v: 4
 		}]
 		}]

+ 83 - 55
services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee

@@ -243,64 +243,92 @@ describe "UpdatesManager", ->
 			@callback.calledWith(null, @updatesWithUserInfo).should.equal true
 			@callback.calledWith(null, @updatesWithUserInfo).should.equal true
 
 
 	describe "fillUserInfo", ->
 	describe "fillUserInfo", ->
-		beforeEach (done) ->
-			@user_id_1 = "user-id-1"
-			@user_id_2 = "user-id-2"
-			@updates = [{
-				meta:
-					user_id: @user_id_1
-				op: "mock-op-1"
-			}, {
-				meta:
-					user_id: @user_id_1
-				op: "mock-op-2"
-			}, {
-				meta:
-					user_id: @user_id_2
-				op: "mock-op-3"
-			}]
-			@user_info =
-				"user-id-1": {
-					email: "user1@sharelatex.com"
-				}
-				"user-id-2": {
-					email: "user2@sharelatex.com"
-				}
-			@WebApiManager.getUserInfo = (user_id, callback = (error, userInfo) ->) =>
-				callback null, @user_info[user_id]
-			sinon.spy @WebApiManager, "getUserInfo"
-
-			@UpdatesManager.fillUserInfo @updates, (error, @results) =>
-				done()
-
-		it "should only call getUserInfo once for each user_id", ->
-			@WebApiManager.getUserInfo.calledTwice.should.equal true
-			@WebApiManager.getUserInfo
-				.calledWith(@user_id_1)
-				.should.equal true
-			@WebApiManager.getUserInfo
-				.calledWith(@user_id_2)
-				.should.equal true
+		describe "with valid users", ->
+			beforeEach (done) ->
+				{ObjectId} = require "mongojs"
+				@user_id_1 = ObjectId().toString()
+				@user_id_2 = ObjectId().toString()
+				@updates = [{
+					meta:
+						user_id: @user_id_1
+					op: "mock-op-1"
+				}, {
+					meta:
+						user_id: @user_id_1
+					op: "mock-op-2"
+				}, {
+					meta:
+						user_id: @user_id_2
+					op: "mock-op-3"
+				}]
+				@user_info = {}
+				@user_info[@user_id_1] = email: "user1@sharelatex.com"
+				@user_info[@user_id_2] = email: "user2@sharelatex.com"
+				
+				@WebApiManager.getUserInfo = (user_id, callback = (error, userInfo) ->) =>
+					callback null, @user_info[user_id]
+				sinon.spy @WebApiManager, "getUserInfo"
+
+				@UpdatesManager.fillUserInfo @updates, (error, @results) =>
+					done()
 
 
-		it "should return the updates with the user info filled", ->
-			expect(@results).to.deep.equal [{
-				meta:
-					user:
-						email: "user1@sharelatex.com"
-				op: "mock-op-1"
-			}, {
-				meta:
-					user:
-						email: "user1@sharelatex.com"
-				op: "mock-op-2"
-			}, {
-				meta:
-					user:
-						email: "user2@sharelatex.com"
-				op: "mock-op-3"
-			}]
+			it "should only call getUserInfo once for each user_id", ->
+				@WebApiManager.getUserInfo.calledTwice.should.equal true
+				@WebApiManager.getUserInfo
+					.calledWith(@user_id_1)
+					.should.equal true
+				@WebApiManager.getUserInfo
+					.calledWith(@user_id_2)
+					.should.equal true
 
 
+			it "should return the updates with the user info filled", ->
+				expect(@results).to.deep.equal [{
+					meta:
+						user:
+							email: "user1@sharelatex.com"
+					op: "mock-op-1"
+				}, {
+					meta:
+						user:
+							email: "user1@sharelatex.com"
+					op: "mock-op-2"
+				}, {
+					meta:
+						user:
+							email: "user2@sharelatex.com"
+					op: "mock-op-3"
+				}]
+
+
+		describe "with invalid user ids", ->
+			beforeEach (done) ->
+				@updates = [{
+					meta:
+						user_id: null
+					op: "mock-op-1"
+				}, {
+					meta:
+						user_id: "anonymous-user"
+					op: "mock-op-2"
+				}]
+				@WebApiManager.getUserInfo = (user_id, callback = (error, userInfo) ->) =>
+					callback null, @user_info[user_id]
+				sinon.spy @WebApiManager, "getUserInfo"
+
+				@UpdatesManager.fillUserInfo @updates, (error, @results) =>
+					done()
 
 
+			it "should not call getUserInfo", ->
+				@WebApiManager.getUserInfo.called.should.equal false
+
+			it "should return the updates without the user info filled", ->
+				expect(@results).to.deep.equal [{
+					meta: {}
+					op: "mock-op-1"
+				}, {
+					meta: {}
+					op: "mock-op-2"
+				}]