From ea5e021d8d09a32e6591f53f63589c8452fbbeab Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 21 Jul 2016 17:57:55 +0100 Subject: [PATCH 1/5] Refactor MatrixClientPeg Should be functionally identical --- src/MatrixClientPeg.js | 138 +++++++++++++++++++++-------------------- 1 file changed, 71 insertions(+), 67 deletions(-) diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 7c1c5b34d7..bf1659e25c 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -16,13 +16,10 @@ limitations under the License. 'use strict'; -// A thing that holds your Matrix Client -var Matrix = require("matrix-js-sdk"); -var GuestAccess = require("./GuestAccess"); +import Matrix from 'matrix-js-sdk'; +import GuestAccess from './GuestAccess'; -let matrixClient: MatrixClient = null; - -var localStorage = window.localStorage; +const localStorage = window.localStorage; function deviceId() { // XXX: is Math.random()'s deterministicity a problem here? @@ -35,73 +32,22 @@ function deviceId() { return id; } -function createClientForPeg(hs_url, is_url, user_id, access_token, guestAccess) { - var opts = { - baseUrl: hs_url, - idBaseUrl: is_url, - accessToken: access_token, - userId: user_id, - timelineSupport: true, - }; - - if (localStorage) { - opts.sessionStore = new Matrix.WebStorageSessionStore(localStorage); - opts.deviceId = deviceId(); - } - - matrixClient = Matrix.createClient(opts); - - // we're going to add eventlisteners for each matrix event tile, so the - // potential number of event listeners is quite high. - matrixClient.setMaxListeners(500); - - if (guestAccess) { - console.log("Guest: %s", guestAccess.isGuest()); - matrixClient.setGuest(guestAccess.isGuest()); - var peekedRoomId = guestAccess.getPeekedRoom(); - if (peekedRoomId) { - console.log("Peeking in room %s", peekedRoomId); - matrixClient.peekInRoom(peekedRoomId); - } - } -} - -if (localStorage) { - var hs_url = localStorage.getItem("mx_hs_url"); - var is_url = localStorage.getItem("mx_is_url") || 'https://matrix.org'; - var access_token = localStorage.getItem("mx_access_token"); - var user_id = localStorage.getItem("mx_user_id"); - var guestAccess = new GuestAccess(localStorage); - if (access_token && user_id && hs_url) { - console.log("Restoring session for %s", user_id); - createClientForPeg(hs_url, is_url, user_id, access_token, guestAccess); - } - else { - console.log("Session not found."); - } -} - -class MatrixClient { - +// A thing that holds your Matrix Client +// Also magically works across sessions through the power of localstorage +class MatrixClientPeg { constructor(guestAccess) { + this.matrixClient = null; this.guestAccess = guestAccess; } get(): MatrixClient { - return matrixClient; + return this.matrixClient; } unset() { - matrixClient = null; + this.matrixClient = null; } - // FIXME, XXX: this all seems very convoluted :( - // - // Why do we have this peg wrapper rather than just MatrixClient.get()? - // Why do we name MatrixClient as MatrixClientPeg when we export it? - // - // -matthew - replaceUsingUrls(hs_url, is_url) { this.replaceClient(hs_url, is_url); } @@ -119,7 +65,7 @@ class MatrixClient { } } this.guestAccess.markAsGuest(Boolean(isGuest)); - createClientForPeg(hs_url, is_url, user_id, access_token, this.guestAccess); + this._createClient(hs_url, is_url, user_id, access_token); if (localStorage) { try { localStorage.setItem("mx_hs_url", hs_url); @@ -134,9 +80,67 @@ class MatrixClient { console.warn("No local storage available: can't persist session!"); } } + + getCredentials() { + return [ + this.matrixClient.baseUrl, + this.matrixClient.idBaseUrl, + this.matrixClient.credentials.userId, + this.matrixClient.getAccessToken(), + this.guestAccess.isGuest(), + ]; + } + + tryRestore() { + if (localStorage) { + const hs_url = localStorage.getItem("mx_hs_url"); + const is_url = localStorage.getItem("mx_is_url") || 'https://matrix.org'; + const access_token = localStorage.getItem("mx_access_token"); + const user_id = localStorage.getItem("mx_user_id"); + const guestAccess = new GuestAccess(localStorage); + if (access_token && user_id && hs_url) { + console.log("Restoring session for %s", user_id); + this._createClient(hs_url, is_url, user_id, access_token, guestAccess); + } else { + console.log("Session not found."); + } + } + } + + _createClient(hs_url, is_url, user_id, access_token) { + var opts = { + baseUrl: hs_url, + idBaseUrl: is_url, + accessToken: access_token, + userId: user_id, + timelineSupport: true, + }; + + if (localStorage) { + opts.sessionStore = new Matrix.WebStorageSessionStore(localStorage); + opts.deviceId = deviceId(); + } + + this.matrixClient = Matrix.createClient(opts); + + // we're going to add eventlisteners for each matrix event tile, so the + // potential number of event listeners is quite high. + this.matrixClient.setMaxListeners(500); + + if (this.guestAccess) { + console.log("Guest: %s", this.guestAccess.isGuest()); + this.matrixClient.setGuest(this.guestAccess.isGuest()); + var peekedRoomId = this.guestAccess.getPeekedRoom(); + if (peekedRoomId) { + console.log("Peeking in room %s", peekedRoomId); + this.matrixClient.peekInRoom(peekedRoomId); + } + } + } } -if (!global.mxMatrixClient) { - global.mxMatrixClient = new MatrixClient(new GuestAccess(localStorage)); +if (!global.mxMatrixClientPeg) { + global.mxMatrixClientPeg = new MatrixClientPeg(new GuestAccess(localStorage)); + global.mxMatrixClientPeg.tryRestore(); } -module.exports = global.mxMatrixClient; +module.exports = global.mxMatrixClientPeg; From b7e95b3883163b4ee1cbeed246b63d5b9b23069e Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 22 Jul 2016 14:00:23 +0100 Subject: [PATCH 2/5] Remove other guestAccess arg --- src/MatrixClientPeg.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index bf1659e25c..77ebbaa2ba 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -100,7 +100,7 @@ class MatrixClientPeg { const guestAccess = new GuestAccess(localStorage); if (access_token && user_id && hs_url) { console.log("Restoring session for %s", user_id); - this._createClient(hs_url, is_url, user_id, access_token, guestAccess); + this._createClient(hs_url, is_url, user_id, access_token); } else { console.log("Session not found."); } From ddbac8c73a61a3d730e4e6a6414840d3d3e89fd1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 22 Jul 2016 15:47:47 +0100 Subject: [PATCH 3/5] More refactoring of MatrixClientPeg Including getting rid of GuestAccess as it was basically doing nothing apart from remembering if we were a guest which may as well be done in the same place we save/restore everything else --- src/GuestAccess.js | 51 --------------------------------- src/MatrixClientPeg.js | 64 ++++++++++++++++++++++++------------------ 2 files changed, 37 insertions(+), 78 deletions(-) delete mode 100644 src/GuestAccess.js diff --git a/src/GuestAccess.js b/src/GuestAccess.js deleted file mode 100644 index ef48d23ded..0000000000 --- a/src/GuestAccess.js +++ /dev/null @@ -1,51 +0,0 @@ -/* -Copyright 2015 OpenMarket Ltd - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ -const IS_GUEST_KEY = "matrix-is-guest"; - -class GuestAccess { - - constructor(localStorage) { - this.localStorage = localStorage; - try { - this._isGuest = localStorage.getItem(IS_GUEST_KEY) === "true"; - } - catch (e) {} // don't care - } - - setPeekedRoom(roomId) { - // we purposefully do not persist this to local storage as peeking is - // entirely transient. - this._peekedRoomId = roomId; - } - - getPeekedRoom() { - return this._peekedRoomId; - } - - isGuest() { - return this._isGuest; - } - - markAsGuest(isGuest) { - try { - this.localStorage.setItem(IS_GUEST_KEY, JSON.stringify(isGuest)); - } catch (e) {} // ignore. If they don't do LS, they'll just get a new account. - this._isGuest = isGuest; - this._peekedRoomId = null; - } -} - -module.exports = GuestAccess; diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 77ebbaa2ba..8d155143fc 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -17,7 +17,6 @@ limitations under the License. 'use strict'; import Matrix from 'matrix-js-sdk'; -import GuestAccess from './GuestAccess'; const localStorage = window.localStorage; @@ -32,12 +31,15 @@ function deviceId() { return id; } -// A thing that holds your Matrix Client -// Also magically works across sessions through the power of localstorage +/** + * Wrapper object for handling the js-sdk Matrix Client object in the react-sdk + * Handles the creation/initialisation of client objects. + * This module provides a singleton instance of this class so the 'current' + * Matrix Client object is available easily. + */ class MatrixClientPeg { - constructor(guestAccess) { + constructor() { this.matrixClient = null; - this.guestAccess = guestAccess; } get(): MatrixClient { @@ -48,15 +50,19 @@ class MatrixClientPeg { this.matrixClient = null; } + /** + * Replace this MatrixClientPeg's client with a client instance that has + * Home Server / Identity Server URLs but no credentials + */ replaceUsingUrls(hs_url, is_url) { - this.replaceClient(hs_url, is_url); + this.replaceUsingAccessToken(hs_url, is_url); } + /** + * Replace this MatrixClientPeg's client with a client instance that has + * Home Server / Identity Server URLs and active credentials + */ replaceUsingAccessToken(hs_url, is_url, user_id, access_token, isGuest) { - this.replaceClient(hs_url, is_url, user_id, access_token, isGuest); - } - - replaceClient(hs_url, is_url, user_id, access_token, isGuest) { if (localStorage) { try { localStorage.clear(); @@ -64,15 +70,20 @@ class MatrixClientPeg { console.warn("Error clearing local storage", e); } } - this.guestAccess.markAsGuest(Boolean(isGuest)); this._createClient(hs_url, is_url, user_id, access_token); + this.matrixClient.setGuest(Boolean(isGuest)); + if (localStorage) { try { localStorage.setItem("mx_hs_url", hs_url); localStorage.setItem("mx_is_url", is_url); - localStorage.setItem("mx_user_id", user_id); - localStorage.setItem("mx_access_token", access_token); - console.log("Session persisted for %s", user_id); + + if (user_id !== undefined && access_token !== undefined) { + localStorage.setItem("mx_user_id", user_id); + localStorage.setItem("mx_access_token", access_token); + localStorage.setItem("mx_is_guest", JSON.stringify(isGuest)); + console.log("Session persisted for %s", user_id); + } } catch (e) { console.warn("Error using local storage: can't persist session!", e); } @@ -87,7 +98,7 @@ class MatrixClientPeg { this.matrixClient.idBaseUrl, this.matrixClient.credentials.userId, this.matrixClient.getAccessToken(), - this.guestAccess.isGuest(), + this.matrixClient.isGuest(), ]; } @@ -97,10 +108,19 @@ class MatrixClientPeg { const is_url = localStorage.getItem("mx_is_url") || 'https://matrix.org'; const access_token = localStorage.getItem("mx_access_token"); const user_id = localStorage.getItem("mx_user_id"); - const guestAccess = new GuestAccess(localStorage); + + let is_guest; + if (localStorage.getItem("mx_is_guest") !== null) { + is_guest = localStorage.getItem("mx_is_guest") === "true"; + } else { + // legacy key name + is_guest = localStorage.getItem("matrix-is-guest") === "true"; + } + if (access_token && user_id && hs_url) { console.log("Restoring session for %s", user_id); this._createClient(hs_url, is_url, user_id, access_token); + this.matrixClient.setGuest(is_guest); } else { console.log("Session not found."); } @@ -126,21 +146,11 @@ class MatrixClientPeg { // we're going to add eventlisteners for each matrix event tile, so the // potential number of event listeners is quite high. this.matrixClient.setMaxListeners(500); - - if (this.guestAccess) { - console.log("Guest: %s", this.guestAccess.isGuest()); - this.matrixClient.setGuest(this.guestAccess.isGuest()); - var peekedRoomId = this.guestAccess.getPeekedRoom(); - if (peekedRoomId) { - console.log("Peeking in room %s", peekedRoomId); - this.matrixClient.peekInRoom(peekedRoomId); - } - } } } if (!global.mxMatrixClientPeg) { - global.mxMatrixClientPeg = new MatrixClientPeg(new GuestAccess(localStorage)); + global.mxMatrixClientPeg = new MatrixClientPeg(); global.mxMatrixClientPeg.tryRestore(); } module.exports = global.mxMatrixClientPeg; From 587a86441fad3f7172ae195b9367d2ef80713118 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 25 Jul 2016 16:20:03 +0100 Subject: [PATCH 4/5] This may as wel go in createclient --- src/MatrixClientPeg.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 8d155143fc..b7feca60ef 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -71,7 +71,6 @@ class MatrixClientPeg { } } this._createClient(hs_url, is_url, user_id, access_token); - this.matrixClient.setGuest(Boolean(isGuest)); if (localStorage) { try { @@ -146,6 +145,8 @@ class MatrixClientPeg { // we're going to add eventlisteners for each matrix event tile, so the // potential number of event listeners is quite high. this.matrixClient.setMaxListeners(500); + + this.matrixClient.setGuest(Boolean(isGuest)); } } From cbf10bfff6cc1fe06e044c32436ded090f45a2d2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 25 Jul 2016 16:28:28 +0100 Subject: [PATCH 5/5] PR feedback Reintroduce replaceClient so we're not calling replaceUsingAccessToken without access tokens which is a bit silly. Fix bug from previous commit (pass isGuest through) --- src/MatrixClientPeg.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index b7feca60ef..ce4b5ba743 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -55,7 +55,7 @@ class MatrixClientPeg { * Home Server / Identity Server URLs but no credentials */ replaceUsingUrls(hs_url, is_url) { - this.replaceUsingAccessToken(hs_url, is_url); + this._replaceClient(hs_url, is_url); } /** @@ -63,6 +63,10 @@ class MatrixClientPeg { * Home Server / Identity Server URLs and active credentials */ replaceUsingAccessToken(hs_url, is_url, user_id, access_token, isGuest) { + this._replaceClient(hs_url, is_url, user_id, access_token, isGuest); + } + + _replaceClient(hs_url, is_url, user_id, access_token, isGuest) { if (localStorage) { try { localStorage.clear(); @@ -70,7 +74,7 @@ class MatrixClientPeg { console.warn("Error clearing local storage", e); } } - this._createClient(hs_url, is_url, user_id, access_token); + this._createClient(hs_url, is_url, user_id, access_token, isGuest); if (localStorage) { try { @@ -126,7 +130,7 @@ class MatrixClientPeg { } } - _createClient(hs_url, is_url, user_id, access_token) { + _createClient(hs_url, is_url, user_id, access_token, isGuest) { var opts = { baseUrl: hs_url, idBaseUrl: is_url,