From 738313384609a8ed05072c0a97723c0a9350d648 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 19 Oct 2018 13:30:38 -0600 Subject: [PATCH 01/12] Support parsing matrix.to links in the timeline with ?via= in them This ends up being translated to ?server_name= in the matrix-js-sdk, although that has a bug at the time of writing. It converts `server_name: ['a', 'b']` to `?server_name=a,b` instead of `?server_name=a&server_name=b` For reference: the `viaServers` option is routed through the 'join_room' action to RoomViewStore#_joinRoom which is passed directly to the js-sdk http-api#joinRoom function. Next steps: * Fix the js-sdk parsing * Make the SDK generate matrix.to links with ?via= --- src/components/structures/LoggedInView.js | 4 ++++ src/components/structures/MatrixChat.js | 13 +++++++++++++ src/components/structures/RoomView.js | 7 +++++-- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index 180a3484345..d95d5cd6524 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -64,6 +64,9 @@ const LoggedInView = React.createClass({ teamToken: PropTypes.string, + // Used by the RoomView to handle joining rooms + viaServers: PropTypes.arrayOf(PropTypes.string), + // and lots and lots of other stuff. }, @@ -389,6 +392,7 @@ const LoggedInView = React.createClass({ onRegistered={this.props.onRegistered} thirdPartyInvite={this.props.thirdPartyInvite} oobData={this.props.roomOobData} + viaServers={this.props.viaServers} eventPixelOffset={this.props.initialEventPixelOffset} key={this.props.currentRoomId || 'roomview'} disabled={this.props.middleDisabled} diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index f385aacd404..5dbafbd2a70 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -840,6 +840,7 @@ export default React.createClass({ page_type: PageTypes.RoomView, thirdPartyInvite: roomInfo.third_party_invite, roomOobData: roomInfo.oob_data, + viaServers: roomInfo.via_servers, }; if (roomInfo.room_alias) { @@ -1488,9 +1489,21 @@ export default React.createClass({ inviterName: params.inviter_name, }; + // on our URLs there might be a ?via=matrix.org or similar to help + // joins to the room succeed. We'll pass these through as an array + // to other levels. If there's just one ?via= then params.via is a + // single string. If someone does something like ?via=one.com&via=two.com + // then params.via is an array of strings. + let via = []; + if (params.via) { + if (typeof(params.via) === 'string') via = [params.via]; + else via = params.via; + } + const payload = { action: 'view_room', event_id: eventId, + via_servers: via, // If an event ID is given in the URL hash, notify RoomViewStore to mark // it as highlighted, which will propagate to RoomView and highlight the // associated EventTile. diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 8e226bdfcf7..5d51b9f9a0b 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -88,6 +88,9 @@ module.exports = React.createClass({ // is the RightPanel collapsed? collapsedRhs: PropTypes.bool, + + // Servers the RoomView can use to try and assist joins + viaServers: PropTypes.arrayOf(PropTypes.string), }, getInitialState: function() { @@ -833,7 +836,7 @@ module.exports = React.createClass({ action: 'do_after_sync_prepared', deferred_action: { action: 'join_room', - opts: { inviteSignUrl: signUrl }, + opts: { inviteSignUrl: signUrl, viaServers: this.props.viaServers }, }, }); @@ -875,7 +878,7 @@ module.exports = React.createClass({ this.props.thirdPartyInvite.inviteSignUrl : undefined; dis.dispatch({ action: 'join_room', - opts: { inviteSignUrl: signUrl }, + opts: { inviteSignUrl: signUrl, viaServers: this.props.viaServers }, }); return Promise.resolve(); }); From a8782120fe8a591fb126cee2dc16b8c9dba18c0e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 24 Oct 2018 16:57:16 -0600 Subject: [PATCH 02/12] Install memfs because webpack is made of fail --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index dabaefe0ad1..03311a50e3b 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "lodash": "^4.13.1", "lolex": "2.3.2", "matrix-js-sdk": "matrix-org/matrix-js-sdk#develop", + "memfs": "^2.10.1", "optimist": "^0.6.1", "pako": "^1.0.5", "prop-types": "^15.5.8", From e8cb636631b8830002f3b429d896608ce5182e6c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 24 Oct 2018 18:01:08 -0600 Subject: [PATCH 03/12] Pick servers for ?via on matrix.to links based on some heuristics --- src/matrix-to.js | 84 ++++++++++++++++++++++- test/matrix-to-test.js | 148 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 2 deletions(-) create mode 100644 test/matrix-to-test.js diff --git a/src/matrix-to.js b/src/matrix-to.js index 90b0a66090e..706363a251d 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -14,11 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. */ +import MatrixClientPeg from "./MatrixClientPeg"; + export const host = "matrix.to"; export const baseUrl = `https://${host}`; export function makeEventPermalink(roomId, eventId) { - return `${baseUrl}/#/${roomId}/${eventId}`; + const serverCandidates = pickServerCandidates(roomId); + return `${baseUrl}/#/${roomId}/${eventId}?${encodeServerCandidates(serverCandidates)}`; } export function makeUserPermalink(userId) { @@ -26,9 +29,86 @@ export function makeUserPermalink(userId) { } export function makeRoomPermalink(roomId) { - return `${baseUrl}/#/${roomId}`; + const serverCandidates = pickServerCandidates(roomId); + return `${baseUrl}/#/${roomId}?${encodeServerCandidates(serverCandidates)}`; } export function makeGroupPermalink(groupId) { return `${baseUrl}/#/${groupId}`; } + +export function encodeServerCandidates(candidates) { + if (!candidates) return ''; + return `via=${candidates.map(c => encodeURIComponent(c)).join("&via=")}` +} + +export function pickServerCandidates(roomId) { + const client = MatrixClientPeg.get(); + const room = client.getRoom(roomId); + if (!room) return []; + + // Permalinks can have servers appended to them so that the user + // receiving them can have a fighting chance at joining the room. + // These servers are called "candidates" at this point because + // it is unclear whether they are going to be useful to actually + // join in the future. + // + // We pick 3 servers based on the following criteria: + // + // Server 1: The highest power level user in the room, provided + // they are at least PL 50. We don't calculate "what is a moderator" + // here because it is less relevant for the vast majority of rooms. + // We also want to ensure that we get an admin or high-ranking mod + // as they are less likely to leave the room. If no user happens + // to meet this criteria, we'll pick the most popular server in the + // room. + // + // Server 2: The next most popular server in the room (in user + // distribution). This will probably be matrix.org in most cases + // although it is certainly possible to be some other server. This + // cannot be the same as Server 1. If no other servers are available + // then we'll only return Server 1. + // + // Server 3: The next most popular server by user distribution. This + // has the same rules as Server 2, with the added exception that it + // must be unique from Server 1 and 2. + + // Rationale for popular servers: It's hard to get rid of people when + // they keep flocking in from a particular server. Sure, the server could + // be ACL'd in the future or for some reason be evicted from the room + // however an event like that is unlikely the larger the room gets. + + // Note: Users receiving permalinks that happen to have all 3 potential + // servers fail them (in terms of joining) are somewhat expected to hunt + // down the person who gave them the link to ask for a participating server. + // The receiving user can then manually append the known-good server to + // the list and magically have the link work. + + const populationMap: {[server:string]:number} = {}; + const highestPlUser = {userId:null, powerLevel: 0, serverName: null}; + + for (const member of room.getJoinedMembers()) { + const serverName = member.userId.split(":").splice(1).join(":"); + if (member.powerLevel > highestPlUser.powerLevel) { + highestPlUser.userId = member.userId; + highestPlUser.powerLevel = member.powerLevel; + highestPlUser.serverName = serverName; + } + + if (!populationMap[serverName]) populationMap[serverName] = 0; + populationMap[serverName]++; + } + + const candidates = []; + if (highestPlUser.powerLevel >= 50) candidates.push(highestPlUser.serverName); + + const maxCandidates = 3; + const serversByPopulation = Object.keys(populationMap) + .sort((a, b) => populationMap[a] - populationMap[b]) + .filter(a => !candidates.includes(a)); + while(candidates.length < maxCandidates && candidates.length <= serversByPopulation.length) { + candidates.push(serversByPopulation[Math.max(0, candidates.length - 1)]); + } + + return candidates; +} \ No newline at end of file diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js new file mode 100644 index 00000000000..9b17a378603 --- /dev/null +++ b/test/matrix-to-test.js @@ -0,0 +1,148 @@ +/* +Copyright 2018 New Vector 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. +*/ + +import expect from 'expect'; +import peg from '../src/MatrixClientPeg'; +import {pickServerCandidates} from "../src/matrix-to"; + + +describe('matrix-to', function () { + it('should pick no candidate servers when the room is not found', function () { + //peg.getRoom = () => null; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(0); + }); + it('should pick no candidate servers when the room has no members', function () { + peg.getRoom = () => { + return { + getJoinedMembers: () => [], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(0); + }); + it('should pick no candidate servers when no users have enough power level', function () { + peg.getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:example.org", + powerLevel: 0, + }, + { + userId: "@bob:example.org", + powerLevel: 25, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(0); + }); + it('should pick a candidate server for the highest power level user in the room', function () { + peg.getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:pl_50", + powerLevel: 50, + }, + { + userId: "@alice:pl_75", + powerLevel: 75, + }, + { + userId: "@alice:pl_95", + powerLevel: 95, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(3); + expect(pickedServers[0]).toBe("pl_95"); + // we don't check the 2nd and 3rd servers because that is done by the next test + }); + it('should pick candidate servers based on user population', function () { + peg.getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:first", + powerLevel: 0, + }, + { + userId: "@bob:first", + powerLevel: 0, + }, + { + userId: "@charlie:first", + powerLevel: 0, + }, + { + userId: "@alice:second", + powerLevel: 0, + }, + { + userId: "@bob:second", + powerLevel: 0, + }, + { + userId: "@charlie:third", + powerLevel: 0, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(3); + expect(pickedServers[0]).toBe("first"); + expect(pickedServers[1]).toBe("second"); + expect(pickedServers[2]).toBe("third"); + }); + it('should pick prefer candidate servers with higher power levels', function () { + peg.getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:first", + powerLevel: 100, + }, + { + userId: "@alice:second", + powerLevel: 0, + }, + { + userId: "@bob:second", + powerLevel: 0, + }, + { + userId: "@charlie:third", + powerLevel: 0, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(3); + expect(pickedServers[0]).toBe("first"); + expect(pickedServers[1]).toBe("second"); + expect(pickedServers[2]).toBe("third"); + }); +}); From 54ff5d8f256cfb1c34f518430ab62ab6df9c6ba0 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 25 Oct 2018 15:21:18 -0600 Subject: [PATCH 04/12] Fix Karma/Webpack so it can build the tests --- karma.conf.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/karma.conf.js b/karma.conf.js index 4d699599cb6..41ddbdf249e 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -199,12 +199,25 @@ module.exports = function (config) { 'matrix-react-sdk': path.resolve('test/skinned-sdk.js'), 'sinon': 'sinon/pkg/sinon.js', + + // To make webpack happy + // Related: https://github.com/request/request/issues/1529 + // (there's no mock available for fs, so we fake a mock by using + // an in-memory version of fs) + "fs": "memfs", }, modules: [ path.resolve('./test'), "node_modules" ], }, + node: { + // Because webpack is made of fail + // https://github.com/request/request/issues/1529 + // Note: 'mock' is the new 'empty' + net: 'mock', + tls: 'mock' + }, devtool: 'inline-source-map', externals: { // Don't try to bundle electron: leave it as a commonjs dependency From b9bfbdc22d70ef8af96a3b331d6f24a09ee1363b Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 25 Oct 2018 15:22:28 -0600 Subject: [PATCH 05/12] Fix the tests so they actually test something --- test/matrix-to-test.js | 46 ++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js index 9b17a378603..3bf6b5af6f4 100644 --- a/test/matrix-to-test.js +++ b/test/matrix-to-test.js @@ -14,17 +14,31 @@ limitations under the License. import expect from 'expect'; import peg from '../src/MatrixClientPeg'; import {pickServerCandidates} from "../src/matrix-to"; +import * as testUtils from "./test-utils"; describe('matrix-to', function () { + let sandbox; + + beforeEach(function() { + testUtils.beforeEach(this); + sandbox = testUtils.stubClient(); + peg.get().credentials = { userId: "@test:example.com" }; + }); + + afterEach(function() { + sandbox.restore(); + }); + it('should pick no candidate servers when the room is not found', function () { - //peg.getRoom = () => null; + peg.get().getRoom = () => null; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); expect(pickedServers.length).toBe(0); }); + it('should pick no candidate servers when the room has no members', function () { - peg.getRoom = () => { + peg.get().getRoom = () => { return { getJoinedMembers: () => [], } @@ -33,27 +47,9 @@ describe('matrix-to', function () { expect(pickedServers).toExist(); expect(pickedServers.length).toBe(0); }); - it('should pick no candidate servers when no users have enough power level', function () { - peg.getRoom = () => { - return { - getJoinedMembers: () => [ - { - userId: "@alice:example.org", - powerLevel: 0, - }, - { - userId: "@bob:example.org", - powerLevel: 25, - } - ], - } - }; - const pickedServers = pickServerCandidates("!somewhere:example.org"); - expect(pickedServers).toExist(); - expect(pickedServers.length).toBe(0); - }); + it('should pick a candidate server for the highest power level user in the room', function () { - peg.getRoom = () => { + peg.get().getRoom = () => { return { getJoinedMembers: () => [ { @@ -77,8 +73,9 @@ describe('matrix-to', function () { expect(pickedServers[0]).toBe("pl_95"); // we don't check the 2nd and 3rd servers because that is done by the next test }); + it('should pick candidate servers based on user population', function () { - peg.getRoom = () => { + peg.get().getRoom = () => { return { getJoinedMembers: () => [ { @@ -115,8 +112,9 @@ describe('matrix-to', function () { expect(pickedServers[1]).toBe("second"); expect(pickedServers[2]).toBe("third"); }); + it('should pick prefer candidate servers with higher power levels', function () { - peg.getRoom = () => { + peg.get().getRoom = () => { return { getJoinedMembers: () => [ { From 43980addd008aa78d353bc632c43f1ca3d3751e5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 25 Oct 2018 15:22:52 -0600 Subject: [PATCH 06/12] Add hostname sanity tests In the event someone changes how the hostname parsing works. --- test/matrix-to-test.js | 85 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js index 3bf6b5af6f4..dbe7fa55ff8 100644 --- a/test/matrix-to-test.js +++ b/test/matrix-to-test.js @@ -143,4 +143,89 @@ describe('matrix-to', function () { expect(pickedServers[1]).toBe("second"); expect(pickedServers[2]).toBe("third"); }); + + it('should work with IPv4 hostnames', function () { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:127.0.0.1", + powerLevel: 100, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(1); + expect(pickedServers[0]).toBe("127.0.0.1"); + }); + + it('should work with IPv6 hostnames', function () { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:[::1]", + powerLevel: 100, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(1); + expect(pickedServers[0]).toBe("[::1]"); + }); + + it('should work with IPv4 hostnames with ports', function () { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:127.0.0.1:8448", + powerLevel: 100, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(1); + expect(pickedServers[0]).toBe("127.0.0.1:8448"); + }); + + it('should work with IPv6 hostnames with ports', function () { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:[::1]:8448", + powerLevel: 100, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(1); + expect(pickedServers[0]).toBe("[::1]:8448"); + }); + + it('should work with hostnames with ports', function () { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:example.org:8448", + powerLevel: 100, + } + ], + } + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(1); + expect(pickedServers[0]).toBe("example.org:8448"); + }); }); From 52094964a01897a0c65ca82f1ef8123808efc6f3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 25 Oct 2018 15:23:15 -0600 Subject: [PATCH 07/12] Fix the server candidate picker to actually work Tests do wonders. --- src/matrix-to.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/matrix-to.js b/src/matrix-to.js index 706363a251d..881972410ff 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -102,12 +102,15 @@ export function pickServerCandidates(roomId) { const candidates = []; if (highestPlUser.powerLevel >= 50) candidates.push(highestPlUser.serverName); + const beforePopulation = candidates.length; const maxCandidates = 3; const serversByPopulation = Object.keys(populationMap) - .sort((a, b) => populationMap[a] - populationMap[b]) + .sort((a, b) => populationMap[b] - populationMap[a]) .filter(a => !candidates.includes(a)); - while(candidates.length < maxCandidates && candidates.length <= serversByPopulation.length) { - candidates.push(serversByPopulation[Math.max(0, candidates.length - 1)]); + for (let i = beforePopulation; i <= maxCandidates; i++) { + const idx = i - beforePopulation; + if (idx >= serversByPopulation.length) break; + candidates.push(serversByPopulation[idx]); } return candidates; From ef8c9246aaef1bd1308fc58974c1a70758a8d82f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 10:07:21 -0600 Subject: [PATCH 08/12] Maybe fix UserSettings? --- src/components/structures/UserSettings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/UserSettings.js b/src/components/structures/UserSettings.js index f32026511b1..772eb70dde0 100644 --- a/src/components/structures/UserSettings.js +++ b/src/components/structures/UserSettings.js @@ -1298,7 +1298,7 @@ module.exports = React.createClass({ // If the olmVersion is not defined then either crypto is disabled, or // we are using a version old version of olm. We assume the former. let olmVersionString = ""; - if (olmVersion !== undefined) { + if (olmVersion) { olmVersionString = `${olmVersion[0]}.${olmVersion[1]}.${olmVersion[2]}`; } From c389540522067978298eaf487596ee0fd30948a2 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 10:22:18 -0600 Subject: [PATCH 09/12] Appease the linter --- src/matrix-to.js | 6 ++--- test/matrix-to-test.js | 56 +++++++++++++++++++++--------------------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/matrix-to.js b/src/matrix-to.js index 881972410ff..e513cbb14c7 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -39,7 +39,7 @@ export function makeGroupPermalink(groupId) { export function encodeServerCandidates(candidates) { if (!candidates) return ''; - return `via=${candidates.map(c => encodeURIComponent(c)).join("&via=")}` + return `via=${candidates.map(c => encodeURIComponent(c)).join("&via=")}`; } export function pickServerCandidates(roomId) { @@ -85,7 +85,7 @@ export function pickServerCandidates(roomId) { // the list and magically have the link work. const populationMap: {[server:string]:number} = {}; - const highestPlUser = {userId:null, powerLevel: 0, serverName: null}; + const highestPlUser = {userId: null, powerLevel: 0, serverName: null}; for (const member of room.getJoinedMembers()) { const serverName = member.userId.split(":").splice(1).join(":"); @@ -114,4 +114,4 @@ export function pickServerCandidates(roomId) { } return candidates; -} \ No newline at end of file +} diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js index dbe7fa55ff8..d199588b9a3 100644 --- a/test/matrix-to-test.js +++ b/test/matrix-to-test.js @@ -17,7 +17,7 @@ import {pickServerCandidates} from "../src/matrix-to"; import * as testUtils from "./test-utils"; -describe('matrix-to', function () { +describe('matrix-to', function() { let sandbox; beforeEach(function() { @@ -30,25 +30,25 @@ describe('matrix-to', function () { sandbox.restore(); }); - it('should pick no candidate servers when the room is not found', function () { + it('should pick no candidate servers when the room is not found', function() { peg.get().getRoom = () => null; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); expect(pickedServers.length).toBe(0); }); - it('should pick no candidate servers when the room has no members', function () { + it('should pick no candidate servers when the room has no members', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); expect(pickedServers.length).toBe(0); }); - it('should pick a candidate server for the highest power level user in the room', function () { + it('should pick a candidate server for the highest power level user in the room', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -63,9 +63,9 @@ describe('matrix-to', function () { { userId: "@alice:pl_95", powerLevel: 95, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); @@ -74,7 +74,7 @@ describe('matrix-to', function () { // we don't check the 2nd and 3rd servers because that is done by the next test }); - it('should pick candidate servers based on user population', function () { + it('should pick candidate servers based on user population', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -101,9 +101,9 @@ describe('matrix-to', function () { { userId: "@charlie:third", powerLevel: 0, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); @@ -113,7 +113,7 @@ describe('matrix-to', function () { expect(pickedServers[2]).toBe("third"); }); - it('should pick prefer candidate servers with higher power levels', function () { + it('should pick prefer candidate servers with higher power levels', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -132,9 +132,9 @@ describe('matrix-to', function () { { userId: "@charlie:third", powerLevel: 0, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); @@ -144,16 +144,16 @@ describe('matrix-to', function () { expect(pickedServers[2]).toBe("third"); }); - it('should work with IPv4 hostnames', function () { + it('should work with IPv4 hostnames', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ { userId: "@alice:127.0.0.1", powerLevel: 100, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); @@ -161,16 +161,16 @@ describe('matrix-to', function () { expect(pickedServers[0]).toBe("127.0.0.1"); }); - it('should work with IPv6 hostnames', function () { + it('should work with IPv6 hostnames', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ { userId: "@alice:[::1]", powerLevel: 100, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); @@ -178,16 +178,16 @@ describe('matrix-to', function () { expect(pickedServers[0]).toBe("[::1]"); }); - it('should work with IPv4 hostnames with ports', function () { + it('should work with IPv4 hostnames with ports', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ { userId: "@alice:127.0.0.1:8448", powerLevel: 100, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); @@ -195,16 +195,16 @@ describe('matrix-to', function () { expect(pickedServers[0]).toBe("127.0.0.1:8448"); }); - it('should work with IPv6 hostnames with ports', function () { + it('should work with IPv6 hostnames with ports', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ { userId: "@alice:[::1]:8448", powerLevel: 100, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); @@ -212,16 +212,16 @@ describe('matrix-to', function () { expect(pickedServers[0]).toBe("[::1]:8448"); }); - it('should work with hostnames with ports', function () { + it('should work with hostnames with ports', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ { userId: "@alice:example.org:8448", powerLevel: 100, - } + }, ], - } + }; }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); From d802ee0fa2ce0a68986f848b9924ac352ededf54 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 10:25:23 -0600 Subject: [PATCH 10/12] Move the max candidates constant out of the function --- src/matrix-to.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/matrix-to.js b/src/matrix-to.js index e513cbb14c7..adcbdf81117 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -19,6 +19,10 @@ import MatrixClientPeg from "./MatrixClientPeg"; export const host = "matrix.to"; export const baseUrl = `https://${host}`; +// The maximum number of servers to pick when working out which servers +// to add to permalinks. The servers are appended as ?via=example.org +const MAX_SERVER_CANDIDATES = 3; + export function makeEventPermalink(roomId, eventId) { const serverCandidates = pickServerCandidates(roomId); return `${baseUrl}/#/${roomId}/${eventId}?${encodeServerCandidates(serverCandidates)}`; @@ -103,11 +107,10 @@ export function pickServerCandidates(roomId) { if (highestPlUser.powerLevel >= 50) candidates.push(highestPlUser.serverName); const beforePopulation = candidates.length; - const maxCandidates = 3; const serversByPopulation = Object.keys(populationMap) .sort((a, b) => populationMap[b] - populationMap[a]) .filter(a => !candidates.includes(a)); - for (let i = beforePopulation; i <= maxCandidates; i++) { + for (let i = beforePopulation; i <= MAX_SERVER_CANDIDATES; i++) { const idx = i - beforePopulation; if (idx >= serversByPopulation.length) break; candidates.push(serversByPopulation[idx]); From 3734c8ab9169d861b9935d254b13838dfc9f3f4a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 12:21:02 -0600 Subject: [PATCH 11/12] Don't mention that matrix.org is likely to be popular This is untrue for quite a few real scenarios, including private federations and a ton of rooms. --- src/matrix-to.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/matrix-to.js b/src/matrix-to.js index adcbdf81117..62ad7c3e6fa 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -68,10 +68,8 @@ export function pickServerCandidates(roomId) { // room. // // Server 2: The next most popular server in the room (in user - // distribution). This will probably be matrix.org in most cases - // although it is certainly possible to be some other server. This - // cannot be the same as Server 1. If no other servers are available - // then we'll only return Server 1. + // distribution). This cannot be the same as Server 1. If no other + // servers are available then we'll only return Server 1. // // Server 3: The next most popular server by user distribution. This // has the same rules as Server 2, with the added exception that it From 0857e2c5e9f88657b4f792507233721613fc51d5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 12:21:27 -0600 Subject: [PATCH 12/12] Clarify why we pick popular servers over the one that created the room --- src/matrix-to.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/matrix-to.js b/src/matrix-to.js index 62ad7c3e6fa..944446c4ccf 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -80,6 +80,12 @@ export function pickServerCandidates(roomId) { // be ACL'd in the future or for some reason be evicted from the room // however an event like that is unlikely the larger the room gets. + // Note: we don't pick the server the room was created on because the + // homeserver should already be using that server as a last ditch attempt + // and there's less of a guarantee that the server is a resident server. + // Instead, we actively figure out which servers are likely to be residents + // in the future and try to use those. + // Note: Users receiving permalinks that happen to have all 3 potential // servers fail them (in terms of joining) are somewhat expected to hunt // down the person who gave them the link to ask for a participating server.