Skip to content

Commit

Permalink
(core) get user.Name through same mechanism as user.id for websocket …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
paulfitz committed Apr 14, 2022
1 parent 64a5c79 commit ce7eb05
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
15 changes: 14 additions & 1 deletion app/server/lib/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -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<UserProfile|null|undefined> {
Expand All @@ -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;
}
Expand Down
23 changes: 6 additions & 17 deletions app/server/lib/Comm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down Expand Up @@ -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;
};


Expand Down
1 change: 0 additions & 1 deletion app/server/lib/FlexServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,6 @@ export class FlexServer implements GristServer {
sessions: this._sessions,
hosts: this._hosts,
httpsServer: this.httpsServer,
dbManager: this._dbManager,
});
}
/**
Expand Down

0 comments on commit ce7eb05

Please sign in to comment.