diff --git a/services/track-changes/app/coffee/DiffManager.coffee b/services/track-changes/app/coffee/DiffManager.coffee index 4294e48d11..e517a5ef4e 100644 --- a/services/track-changes/app/coffee/DiffManager.coffee +++ b/services/track-changes/app/coffee/DiffManager.coffee @@ -5,7 +5,7 @@ logger = require "logger-sharelatex" module.exports = DiffManager = getLatestDocAndUpdates: (project_id, doc_id, fromVersion, toVersion, callback = (error, lines, version, updates) ->) -> - UpdatesManager.getUpdates doc_id, from: fromVersion, to: toVersion, (error, updates) -> + UpdatesManager.getUpdatesWithUserInfo doc_id, from: fromVersion, to: toVersion, (error, updates) -> return callback(error) if error? DocumentUpdaterManager.getDocument project_id, doc_id, (error, lines, version) -> return callback(error) if error? diff --git a/services/track-changes/app/coffee/HttpController.coffee b/services/track-changes/app/coffee/HttpController.coffee index 6f8a428a6e..5d7bcf6c72 100644 --- a/services/track-changes/app/coffee/HttpController.coffee +++ b/services/track-changes/app/coffee/HttpController.coffee @@ -37,7 +37,7 @@ module.exports = HttpController = if req.query.limit? limit = parseInt(req.query.limit, 10) - UpdatesManager.getUpdates doc_id, to: to, limit: limit, (error, updates) -> + UpdatesManager.getUpdatesWithUserInfo doc_id, to: to, limit: limit, (error, updates) -> return next(error) if error? formattedUpdates = for update in updates { @@ -45,5 +45,3 @@ module.exports = HttpController = v: update.v } res.send JSON.stringify updates: formattedUpdates - - diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee index 158688d03b..ee9bb12655 100644 --- a/services/track-changes/app/coffee/UpdatesManager.coffee +++ b/services/track-changes/app/coffee/UpdatesManager.coffee @@ -2,7 +2,9 @@ MongoManager = require "./MongoManager" RedisManager = require "./RedisManager" UpdateCompressor = require "./UpdateCompressor" LockManager = require "./LockManager" +WebApiManager = require "./WebApiManager" logger = require "logger-sharelatex" +async = require "async" module.exports = UpdatesManager = compressAndSaveRawUpdates: (doc_id, rawUpdates, callback = (error) ->) -> @@ -70,3 +72,31 @@ module.exports = UpdatesManager = return callback(error) if error? MongoManager.getUpdates doc_id, options, callback + getUpdatesWithUserInfo: (doc_id, options = {}, callback = (error, updates) ->) -> + UpdatesManager.getUpdates doc_id, options, (error, updates) -> + return callback(error) if error? + UpdatesManager.fillUserInfo updates, (error, updates) -> + return callback(error) if error? + callback null, updates + + fillUserInfo: (updates, callback = (error, updates) ->) -> + users = {} + for update in updates + users[update.meta.user_id] = true + + jobs = [] + for user_id, _ of users + do (user_id) -> + jobs.push (callback) -> + WebApiManager.getUserInfo user_id, (error, userInfo) -> + return callback(error) if error? + users[user_id] = userInfo + callback() + + async.series jobs, (error) -> + return callback(error) if error? + for update in updates + user_id = update.meta.user_id + delete update.meta.user_id + update.meta.user = users[user_id] + callback null, updates diff --git a/services/track-changes/app/coffee/WebApiManager.coffee b/services/track-changes/app/coffee/WebApiManager.coffee new file mode 100644 index 0000000000..fc4eda9729 --- /dev/null +++ b/services/track-changes/app/coffee/WebApiManager.coffee @@ -0,0 +1,32 @@ +request = require "request" +logger = require "logger-sharelatex" +Settings = require "settings-sharelatex" + +module.exports = WebApiManager = + getUserInfo: (user_id, callback = (error, userInfo) ->) -> + url = "#{Settings.apis.web.url}/user/#{user_id}/personal_info" + logger.log user_id: user_id, "getting user info from web" + request.get { + url: url + auth: + user: Settings.apis.web.user + pass: Settings.apis.web.pass + sendImmediately: true + }, (error, res, body)-> + if error? + return callback(error) + if res.statusCode >= 200 and res.statusCode < 300 + try + user = JSON.parse(body) + catch error + return callback(error) + callback null, { + id: user.id + email: user.email + first_name: user.first_name + last_name: user.last_name + } + else + error = new Error("web returned a non-success status code: #{res.statusCode}") + logger.error err: error, user_id: user_id, url: url, "error accessing web" + callback error \ No newline at end of file diff --git a/services/track-changes/config/settings.development.coffee b/services/track-changes/config/settings.development.coffee index d3cf2978ec..f79f657fa6 100755 --- a/services/track-changes/config/settings.development.coffee +++ b/services/track-changes/config/settings.development.coffee @@ -8,3 +8,7 @@ module.exports = apis: documentupdater: url: "http://localhost:3003" + web: + url: "http://localhost:3000" + user: "sharelatex" + pass: "password" diff --git a/services/track-changes/test/acceptance/coffee/GettingADiffTests.coffee b/services/track-changes/test/acceptance/coffee/GettingADiffTests.coffee index 8284b425aa..ed0e0e42d6 100644 --- a/services/track-changes/test/acceptance/coffee/GettingADiffTests.coffee +++ b/services/track-changes/test/acceptance/coffee/GettingADiffTests.coffee @@ -9,6 +9,7 @@ Settings = require "settings-sharelatex" TrackChangesClient = require "./helpers/TrackChangesClient" MockDocUpdaterApi = require "./helpers/MockDocUpdaterApi" +MockWebApi = require "./helpers/MockWebApi" describe "Getting a diff", -> before (done) -> @@ -21,6 +22,13 @@ describe "Getting a diff", -> @doc_id = ObjectId().toString() @project_id = ObjectId().toString() + MockWebApi.users[@user_id] = @user = + email: "user@sharelatex.com" + first_name: "Leo" + last_name: "Lion" + id: @user_id + sinon.spy MockWebApi, "getUser" + twoMinutes = 2 * 60 * 1000 @updates = [{ @@ -43,8 +51,8 @@ describe "Getting a diff", -> @lines = ["one two three four"] @expected_diff = [ { u: "one " } - { i: "two ", meta: { start_ts: @from + twoMinutes, end_ts: @from + twoMinutes, user_id: @user_id } } - { i: "three ", meta: { start_ts: @to - twoMinutes, end_ts: @to - twoMinutes, user_id: @user_id } } + { i: "two ", meta: { start_ts: @from + twoMinutes, end_ts: @from + twoMinutes, user: @user } } + { i: "three ", meta: { start_ts: @to - twoMinutes, end_ts: @to - twoMinutes, user: @user } } ] MockDocUpdaterApi.docs[@doc_id] = @@ -60,6 +68,7 @@ describe "Getting a diff", -> after () -> MockDocUpdaterApi.getDoc.restore() + MockWebApi.getUser.restore() it "should return the diff", -> expect(@diff).to.deep.equal @expected_diff diff --git a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee index 751b098cb7..6720548b26 100644 --- a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee +++ b/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee @@ -8,6 +8,7 @@ ObjectId = mongojs.ObjectId Settings = require "settings-sharelatex" TrackChangesClient = require "./helpers/TrackChangesClient" +MockWebApi = require "./helpers/MockWebApi" describe "Getting updates", -> before (done) -> @@ -19,6 +20,13 @@ describe "Getting updates", -> @minutes = 60 * 1000 + MockWebApi.users[@user_id] = @user = + email: "user@sharelatex.com" + first_name: "Leo" + last_name: "Lion" + id: @user_id + sinon.spy MockWebApi, "getUser" + @updates = [{ op: [{ i: "one ", p: 0 }] meta: { ts: @to - 4 * @minutes, user_id: @user_id } @@ -44,17 +52,25 @@ describe "Getting updates", -> @updates = body.updates done() - it "should return the diff", -> + after: () -> + MockWebApi.getUser.restore() + + it "should fetch the user details from the web api", -> + MockWebApi.getUser + .calledWith(@user_id) + .should.equal true + + it "should return the updates", -> expect(@updates).to.deep.equal [{ meta: start_ts: @to end_ts: @to - user_id: @user_id + user: @user v: 5 }, { meta: start_ts: @to - 2 * @minutes end_ts: @to - 2 * @minutes - user_id: @user_id + user: @user v: 4 }] diff --git a/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee new file mode 100644 index 0000000000..04ee281541 --- /dev/null +++ b/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -0,0 +1,24 @@ +express = require("express") +app = express() + +module.exports = MockWebApi = + users: {} + + getUser: (user_id, callback = (error) ->) -> + callback null, @users[user_id] + + run: () -> + app.get "/user/:user_id/personal_info", (req, res, next) => + @getUser req.params.user_id, (error, user) -> + if error? + res.send 500 + if !user? + res.send 404 + else + res.send JSON.stringify user + + app.listen 3000, (error) -> + throw error if error? + +MockWebApi.run() + diff --git a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee index e12d4b2085..7b1ce33af1 100644 --- a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee @@ -25,7 +25,7 @@ describe "DiffManager", -> @updates = [ "mock-update-1", "mock-update-2" ] @DocumentUpdaterManager.getDocument = sinon.stub().callsArgWith(2, null, @lines, @version) - @UpdatesManager.getUpdates = sinon.stub().callsArgWith(2, null, @updates) + @UpdatesManager.getUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @updates) @DiffManager.getLatestDocAndUpdates @project_id, @doc_id, @from, @to, @callback it "should get the latest version of the doc", -> @@ -34,7 +34,7 @@ describe "DiffManager", -> .should.equal true it "should get the latest updates", -> - @UpdatesManager.getUpdates + @UpdatesManager.getUpdatesWithUserInfo .calledWith(@doc_id, from: @from, to: @to) .should.equal true diff --git a/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee b/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee index 667e40193a..8e57ea2bd3 100644 --- a/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee +++ b/services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee @@ -78,15 +78,15 @@ describe "HttpController", -> meta: @meta = "mock-meta" doc_id: @doc_id }] - @UpdatesManager.getUpdates = sinon.stub().callsArgWith(2, null, @rawUpdates) + @UpdatesManager.getUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @rawUpdates) @HttpController.getUpdates @req, @res, @next it "should get the updates", -> - @UpdatesManager.getUpdates + @UpdatesManager.getUpdatesWithUserInfo .calledWith(@doc_id, to: @to, limit: @limit) .should.equal true - it "should return the updates", -> + it "should return the formatted updates", -> updates = for update in @rawUpdates { meta: @meta diff --git a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee index 7dfc96169e..fd2056e38d 100644 --- a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee @@ -12,6 +12,7 @@ describe "UpdatesManager", -> "./MongoManager" : @MongoManager = {} "./RedisManager" : @RedisManager = {} "./LockManager" : @LockManager = {} + "./WebApiManager": @WebApiManager = {} "logger-sharelatex": { log: sinon.stub(), error: sinon.stub() } @doc_id = "doc-id-123" @callback = sinon.stub() @@ -219,3 +220,87 @@ describe "UpdatesManager", -> .calledWith(null, @updates) .should.equal true + describe "getUpdatesWithUserInfo", -> + beforeEach -> + @updates = ["mock-updates"] + @options = { to: "mock-to", limit: "mock-limit" } + @updatesWithUserInfo = ["updates-with-user-info"] + @UpdatesManager.getUpdates = sinon.stub().callsArgWith(2, null, @updates) + @UpdatesManager.fillUserInfo = sinon.stub().callsArgWith(1, null, @updatesWithUserInfo) + @UpdatesManager.getUpdatesWithUserInfo @doc_id, @options, @callback + + it "should get the updates", -> + @UpdatesManager.getUpdates + .calledWith(@doc_id, @options) + .should.equal true + + it "should file the updates with the user info", -> + @UpdatesManager.fillUserInfo + .calledWith(@updates) + .should.equal true + + it "shoudl return the updates with the filled details", -> + @callback.calledWith(null, @updatesWithUserInfo).should.equal true + + 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 + + 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" + }] + + + + + diff --git a/services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee b/services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee new file mode 100644 index 0000000000..589dfeb7e5 --- /dev/null +++ b/services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee @@ -0,0 +1,71 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/WebApiManager.js" +SandboxedModule = require('sandboxed-module') + +describe "WebApiManager", -> + beforeEach -> + @WebApiManager = SandboxedModule.require modulePath, requires: + "request": @request = {} + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } + 'settings-sharelatex': @settings = + apis: + web: + url: "http://example.com" + user: "sharelatex" + pass: "password" + @callback = sinon.stub() + @user_id = "mock-user-id" + @user_info = + email: "leo@sharelatex.com" + id: @user_id + first_name: "Leo" + last_nane: "Lion" + extra_param: "blah" + + describe "getUserInfo", -> + describe "successfully", -> + beforeEach -> + @body = JSON.stringify @user_info + @request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body) + @WebApiManager.getUserInfo @user_id, @callback + + it 'should get the user from the web api', -> + url = + @request.get + .calledWith({ + url: "#{@settings.apis.web.url}/user/#{@user_id}/personal_info" + auth: + user: @settings.apis.web.user + pass: @settings.apis.web.pass + sendImmediately: true + }) + .should.equal true + + it "should call the callback with only the email, id and names", -> + @callback.calledWith(null, { + id: @user_id + email: @user_info.email + first_name: @user_info.first_name + last_name: @user_info.last_name + }).should.equal true + + describe "when the web API returns an error", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null) + @WebApiManager.getUserInfo @user_id, @callback + + it "should return an error to the callback", -> + @callback.calledWith(@error).should.equal true + + describe "when the web returns a failure error code", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "") + @WebApiManager.getUserInfo @user_id, @callback + + it "should return the callback with an error", -> + @callback + .calledWith(new Error("web returned failure status code: 500")) + .should.equal true \ No newline at end of file