From ce7eb05ed49430701f952831d9b578d97ad5fe94 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 14 Apr 2022 11:59:51 -0400 Subject: [PATCH] (core) get user.Name through same mechanism as user.id for websocket Client Summary: This avoids an extra database query to look up the user's current name, by capturing it at the moment their user id is queried. Test Plan: existing test for user.Name changes continues to pass Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D3381 --- app/server/lib/Client.ts | 15 ++++++++++++++- app/server/lib/Comm.js | 23 ++++++----------------- app/server/lib/FlexServer.ts | 1 - 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/app/server/lib/Client.ts b/app/server/lib/Client.ts index 1c3d1cfd1d..35fe359d1b 100644 --- a/app/server/lib/Client.ts +++ b/app/server/lib/Client.ts @@ -77,6 +77,7 @@ export class Client { private _org: string|null = null; private _profile: UserProfile|null = null; private _userId: number|null = null; + private _userName: string|null = null; private _firstLoginAt: Date|null = null; private _isAnonymous: boolean = false; // Identifier for the current GristWSConnection object connected to this client. @@ -299,6 +300,7 @@ export class Client { // Unset userId, so that we look it up again on demand. (Not that userId could change in // practice via a change to profile, but let's not make any assumptions here.) this._userId = null; + this._userName = null; this._firstLoginAt = null; this._isAnonymous = !profile; } @@ -311,7 +313,16 @@ export class Client { anonymous: true, }; } - return this._profile; + // If we have a database, the user id and name will have been + // fetched before we start using the Client, so we take this + // opportunity to update the user name to use the latest user name + // in the database (important since user name is now exposed via + // user.Name in granular access support). TODO: might want to + // subscribe to changes in user name while the document is open. + return this._profile ? { + ...this._profile, + ...(this._userName && { name: this._userName }), + } : null; } public async getSessionProfile(): Promise { @@ -336,10 +347,12 @@ export class Client { if (this._profile) { const user = await this._fetchUser(dbManager); this._userId = (user && user.id) || null; + this._userName = (user && user.name) || null; this._isAnonymous = this._userId && dbManager.getAnonymousUserId() === this._userId || false; this._firstLoginAt = (user && user.firstLoginAt) || null; } else { this._userId = dbManager.getAnonymousUserId(); + this._userName = 'Anonymous'; this._isAnonymous = true; this._firstLoginAt = null; } diff --git a/app/server/lib/Comm.js b/app/server/lib/Comm.js index 202e7c2d5a..6e7eaca442 100644 --- a/app/server/lib/Comm.js +++ b/app/server/lib/Comm.js @@ -85,7 +85,6 @@ function Comm(server, options) { this._settings = options.settings; this._hosts = options.hosts; - this._dbManager = options.dbManager; // This maps method names to their implementation. this.methods = {}; @@ -156,23 +155,13 @@ Comm.prototype._broadcastMessage = function(type, data, clients) { /** * Returns a profile based on the request or session. */ -Comm.prototype._getSessionProfile = async function(scopedSession, req) { - const profile = getRequestProfile(req) || await scopedSession.getSessionProfile(); - if (this._dbManager && profile?.email) { - try { - // Use latest user name in database, since user name is now exposed via - // user.Name in granular access support. - // TODO: might want to subscribe to changes to user information while - // the document is open. - const user = await this._dbManager.getUserByLogin(profile.email, {profile}); - profile.name = user.name; - } catch (e) { - // Not an expected problem, log it and fail brutally. - log.debug(`Comm: failed to look up user in database ${profile.email}`); - throw e; - } +Comm.prototype._getSessionProfile = function(scopedSession, req) { + const profile = getRequestProfile(req); + if (profile) { + return Promise.resolve(profile); + } else { + return scopedSession.getSessionProfile(); } - return profile; }; diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 2a9e589db1..5a9182166b 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -825,7 +825,6 @@ export class FlexServer implements GristServer { sessions: this._sessions, hosts: this._hosts, httpsServer: this.httpsServer, - dbManager: this._dbManager, }); } /**