diff --git a/src/RoomAliasCache.ts b/src/RoomAliasCache.ts index fd611a7559..ff57a70e6c 100644 --- a/src/RoomAliasCache.ts +++ b/src/RoomAliasCache.ts @@ -6,6 +6,8 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ +type CacheResult = { roomId: string; viaServers: string[] }; + /** * This is meant to be a cache of room alias to room ID so that moving between * rooms happens smoothly (for example using browser back / forward buttons). @@ -16,12 +18,12 @@ Please see LICENSE files in the repository root for full details. * A similar thing could also be achieved via `pushState` with a state object, * but keeping it separate like this seems easier in case we do want to extend. */ -const aliasToIDMap = new Map(); +const cache = new Map(); -export function storeRoomAliasInCache(alias: string, id: string): void { - aliasToIDMap.set(alias, id); +export function storeRoomAliasInCache(alias: string, roomId: string, viaServers: string[]): void { + cache.set(alias, { roomId, viaServers }); } -export function getCachedRoomIDForAlias(alias: string): string | undefined { - return aliasToIDMap.get(alias); +export function getCachedRoomIdForAlias(alias: string): CacheResult | undefined { + return cache.get(alias); } diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 62831716c3..aef4fcdaf0 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -51,7 +51,7 @@ import ThemeController from "../../settings/controllers/ThemeController"; import { startAnyRegistrationFlow } from "../../Registration"; import ResizeNotifier from "../../utils/ResizeNotifier"; import AutoDiscoveryUtils from "../../utils/AutoDiscoveryUtils"; -import { makeRoomPermalink } from "../../utils/permalinks/Permalinks"; +import { calculateRoomVia, makeRoomPermalink } from "../../utils/permalinks/Permalinks"; import ThemeWatcher, { ThemeWatcherEvent } from "../../settings/watchers/ThemeWatcher"; import { FontWatcher } from "../../settings/watchers/FontWatcher"; import { storeRoomAliasInCache } from "../../RoomAliasCache"; @@ -1026,7 +1026,7 @@ export default class MatrixChat extends React.PureComponent { presentedId = theAlias; // Store display alias of the presented room in cache to speed future // navigation. - storeRoomAliasInCache(theAlias, room.roomId); + storeRoomAliasInCache(theAlias, room.roomId, calculateRoomVia(room)); } // Store this as the ID of the last room accessed. This is so that we can diff --git a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx index 419a12df89..b85c1de29e 100644 --- a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx +++ b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx @@ -53,7 +53,7 @@ import { getKeyBindingsManager } from "../../../../KeyBindingsManager"; import { _t } from "../../../../languageHandler"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import { PosthogAnalytics } from "../../../../PosthogAnalytics"; -import { getCachedRoomIDForAlias } from "../../../../RoomAliasCache"; +import { getCachedRoomIdForAlias } from "../../../../RoomAliasCache"; import { showStartChatInviteDialog } from "../../../../RoomInvite"; import { SettingLevel } from "../../../../settings/SettingLevel"; import SettingsStore from "../../../../settings/SettingsStore"; @@ -912,7 +912,7 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n if ( trimmedQuery.startsWith("#") && trimmedQuery.includes(":") && - (!getCachedRoomIDForAlias(trimmedQuery) || !cli.getRoom(getCachedRoomIDForAlias(trimmedQuery))) + (!getCachedRoomIdForAlias(trimmedQuery) || !cli.getRoom(getCachedRoomIdForAlias(trimmedQuery)!.roomId)) ) { joinRoomSection = (
diff --git a/src/modules/Navigation.ts b/src/modules/Navigation.ts index 466cacff5d..0e7724727d 100644 --- a/src/modules/Navigation.ts +++ b/src/modules/Navigation.ts @@ -9,33 +9,31 @@ import { type NavigationApi as INavigationApi } from "@element-hq/element-web-mo import { navigateToPermalink } from "../utils/permalinks/navigator.ts"; import { parsePermalink } from "../utils/permalinks/Permalinks.ts"; -import { getCachedRoomIDForAlias } from "../RoomAliasCache.ts"; -import { MatrixClientPeg } from "../MatrixClientPeg.ts"; import dispatcher from "../dispatcher/dispatcher.ts"; import { Action } from "../dispatcher/actions.ts"; -import SettingsStore from "../settings/SettingsStore.ts"; +import type { ViewRoomPayload } from "../dispatcher/payloads/ViewRoomPayload.ts"; export class NavigationApi implements INavigationApi { public async toMatrixToLink(link: string, join = false): Promise { navigateToPermalink(link); const parts = parsePermalink(link); - if (parts?.roomIdOrAlias && join) { - let roomId: string | undefined = parts.roomIdOrAlias; - if (roomId.startsWith("#")) { - roomId = getCachedRoomIDForAlias(parts.roomIdOrAlias); - if (!roomId) { - // alias resolution failed - const result = await MatrixClientPeg.safeGet().getRoomIdForAlias(parts.roomIdOrAlias); - roomId = result.room_id; - } - } - - if (roomId) { - dispatcher.dispatch({ - action: Action.JoinRoom, - canAskToJoin: SettingsStore.getValue("feature_ask_to_join"), - roomId, + if (parts?.roomIdOrAlias) { + if (parts.roomIdOrAlias.startsWith("#")) { + dispatcher.dispatch({ + action: Action.ViewRoom, + room_alias: parts.roomIdOrAlias, + via_servers: parts.viaServers ?? undefined, + auto_join: join, + metricsTrigger: undefined, + }); + } else { + dispatcher.dispatch({ + action: Action.ViewRoom, + room_id: parts.roomIdOrAlias, + via_servers: parts.viaServers ?? undefined, + auto_join: join, + metricsTrigger: undefined, }); } } diff --git a/src/modules/ProxiedModuleApi.ts b/src/modules/ProxiedModuleApi.ts index b14b2c0c0c..86db1a9e35 100644 --- a/src/modules/ProxiedModuleApi.ts +++ b/src/modules/ProxiedModuleApi.ts @@ -28,13 +28,12 @@ import dispatcher from "../dispatcher/dispatcher"; import { navigateToPermalink } from "../utils/permalinks/navigator"; import { parsePermalink } from "../utils/permalinks/Permalinks"; import { MatrixClientPeg } from "../MatrixClientPeg"; -import { getCachedRoomIDForAlias } from "../RoomAliasCache"; import { Action } from "../dispatcher/actions"; import { type OverwriteLoginPayload } from "../dispatcher/payloads/OverwriteLoginPayload"; import { type ActionPayload } from "../dispatcher/payloads"; -import SettingsStore from "../settings/SettingsStore"; import WidgetStore, { type IApp } from "../stores/WidgetStore"; import { type Container, WidgetLayoutStore } from "../stores/widgets/WidgetLayoutStore"; +import type { ViewRoomPayload } from "../dispatcher/payloads/ViewRoomPayload.ts"; /** * Glue between the `ModuleApi` interface and the react-sdk. Anticipates one instance @@ -183,28 +182,22 @@ export class ProxiedModuleApi implements ModuleApi { navigateToPermalink(uri); const parts = parsePermalink(uri); - if (parts?.roomIdOrAlias && andJoin) { - let roomId: string | undefined = parts.roomIdOrAlias; - let servers = parts.viaServers; - if (roomId.startsWith("#")) { - roomId = getCachedRoomIDForAlias(parts.roomIdOrAlias); - if (!roomId) { - // alias resolution failed - const result = await MatrixClientPeg.safeGet().getRoomIdForAlias(parts.roomIdOrAlias); - roomId = result.room_id; - if (!servers) servers = result.servers; // use provided servers first, if available - } - } - dispatcher.dispatch({ - action: Action.ViewRoom, - room_id: roomId, - via_servers: servers, - }); - - if (andJoin) { - dispatcher.dispatch({ - action: Action.JoinRoom, - canAskToJoin: SettingsStore.getValue("feature_ask_to_join"), + if (parts?.roomIdOrAlias) { + if (parts.roomIdOrAlias.startsWith("#")) { + dispatcher.dispatch({ + action: Action.ViewRoom, + room_alias: parts.roomIdOrAlias, + via_servers: parts.viaServers ?? undefined, + auto_join: andJoin ?? false, + metricsTrigger: undefined, + }); + } else { + dispatcher.dispatch({ + action: Action.ViewRoom, + room_id: parts.roomIdOrAlias, + via_servers: parts.viaServers ?? undefined, + auto_join: andJoin ?? false, + metricsTrigger: undefined, }); } } diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index e3102e773c..50ca69eec3 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -26,7 +26,7 @@ import { type MatrixDispatcher } from "../dispatcher/dispatcher"; import { MatrixClientPeg } from "../MatrixClientPeg"; import Modal from "../Modal"; import { _t } from "../languageHandler"; -import { getCachedRoomIDForAlias, storeRoomAliasInCache } from "../RoomAliasCache"; +import { getCachedRoomIdForAlias, storeRoomAliasInCache } from "../RoomAliasCache"; import { Action } from "../dispatcher/actions"; import { retry } from "../utils/promise"; import { TimelineRenderingType } from "../contexts/RoomContext"; @@ -438,6 +438,7 @@ export class RoomViewStore extends EventEmitter { action: Action.JoinRoom, roomId: payload.room_id, metricsTrigger: payload.metricsTrigger as JoinRoomPayload["metricsTrigger"], + canAskToJoin: SettingsStore.getValue("feature_ask_to_join"), }); } @@ -445,10 +446,16 @@ export class RoomViewStore extends EventEmitter { await setMarkedUnreadState(room, MatrixClientPeg.safeGet(), false); } } else if (payload.room_alias) { + let roomId: string; + let viaServers: string[] | undefined; + // Try the room alias to room ID navigation cache first to avoid // blocking room navigation on the homeserver. - let roomId = getCachedRoomIDForAlias(payload.room_alias); - if (!roomId) { + const cachedResult = getCachedRoomIdForAlias(payload.room_alias); + if (cachedResult) { + roomId = cachedResult.roomId; + viaServers = cachedResult.viaServers; + } else { // Room alias cache miss, so let's ask the homeserver. Resolve the alias // and then do a second dispatch with the room ID acquired. this.setState({ @@ -467,8 +474,9 @@ export class RoomViewStore extends EventEmitter { }); try { const result = await MatrixClientPeg.safeGet().getRoomIdForAlias(payload.room_alias); - storeRoomAliasInCache(payload.room_alias, result.room_id); + storeRoomAliasInCache(payload.room_alias, result.room_id, result.servers); roomId = result.room_id; + viaServers = result.servers; } catch (err) { logger.error("RVS failed to get room id for alias: ", err); this.dis?.dispatch({ @@ -485,6 +493,7 @@ export class RoomViewStore extends EventEmitter { this.dis?.dispatch({ ...payload, room_id: roomId, + via_servers: viaServers, }); } } @@ -509,12 +518,13 @@ export class RoomViewStore extends EventEmitter { joining: true, }); - // take a copy of roomAlias & roomId as they may change by the time the join is complete - const { roomAlias, roomId } = this.state; - const address = payload.roomId || roomAlias || roomId!; + // take a copy of roomAlias, roomId & viaServers as they may change by the time the join is complete + const { roomAlias, roomId = payload.roomId, viaServers = [] } = this.state; + // prefer the room alias if we have one as it allows joining over federation even with no viaServers + const address = roomAlias || roomId!; const joinOpts: IJoinRoomOpts = { - viaServers: this.state.viaServers || [], + viaServers, ...(payload.opts ?? {}), }; if (SettingsStore.getValue("feature_share_history_on_invite")) { @@ -547,7 +557,7 @@ export class RoomViewStore extends EventEmitter { canAskToJoin: payload.canAskToJoin, }); - if (payload.canAskToJoin) { + if (payload.canAskToJoin && err instanceof MatrixError && err.httpStatus === 403) { this.dis?.dispatch({ action: Action.PromptAskToJoin }); } } diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 2945d486c2..bfb98f6b02 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -49,7 +49,7 @@ import { UPDATE_SUGGESTED_ROOMS, UPDATE_TOP_LEVEL_SPACES, } from "."; -import { getCachedRoomIDForAlias } from "../../RoomAliasCache"; +import { getCachedRoomIdForAlias } from "../../RoomAliasCache"; import { EffectiveMembership, getEffectiveMembership } from "../../utils/membership"; import { flattenSpaceHierarchyWithCache, @@ -1249,7 +1249,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { let roomId = payload.room_id; if (payload.room_alias && !roomId) { - roomId = getCachedRoomIDForAlias(payload.room_alias); + const result = getCachedRoomIdForAlias(payload.room_alias); + if (result) roomId = result.roomId; } if (!roomId) return; // we'll get re-fired with the room ID shortly diff --git a/test/unit-tests/modules/Navigation-test.ts b/test/unit-tests/modules/Navigation-test.ts index 691d8975fc..3fafdf0fa6 100644 --- a/test/unit-tests/modules/Navigation-test.ts +++ b/test/unit-tests/modules/Navigation-test.ts @@ -5,37 +5,35 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { mocked } from "jest-mock"; - import * as navigator from "../../../src/utils/permalinks/navigator"; import { NavigationApi } from "../../../src/modules/Navigation.ts"; -import { stubClient } from "../../test-utils"; import defaultDispatcher from "../../../src/dispatcher/dispatcher.ts"; describe("NavigationApi", () => { const api = new NavigationApi(); describe("toMatrixToLink", () => { - it("should call navigateToPermalink with the correct parameters", async () => { - const link = "https://matrix.to/#/!roomId:server.com"; + it.each([ + ["roomId", "https://matrix.to/#/!roomId:server.com"], + ["roomAlias", "https://matrix.to/#/#alias:server.com"], + ["user", "https://matrix.to/#/@user:server.com"], + ])("should call navigateToPermalink with the correct parameters for %s", async (_type, link) => { const spy = jest.spyOn(navigator, "navigateToPermalink"); await api.toMatrixToLink(link); expect(spy).toHaveBeenCalledWith(link); }); - it("should resolve the room alias to a room id when join=true", async () => { - const cli = stubClient(); - mocked(cli.getRoomIdForAlias).mockResolvedValue({ room_id: "!roomId:server.com", servers: [] }); - - const link = "https://matrix.to/#/#alias:server.com"; + it("should set auto_join to true when join=true", async () => { + const link = "https://matrix.to/#/#alias:server.com?via=server.com"; const spy = jest.spyOn(defaultDispatcher, "dispatch"); await api.toMatrixToLink(link, true); expect(spy).toHaveBeenCalledWith( expect.objectContaining({ - action: "join_room", - roomId: "!roomId:server.com", + action: "view_room", + room_alias: "#alias:server.com", + auto_join: true, }), ); }); diff --git a/test/unit-tests/modules/ProxiedModuleApi-test.tsx b/test/unit-tests/modules/ProxiedModuleApi-test.tsx index 4a325050ec..bb21f35397 100644 --- a/test/unit-tests/modules/ProxiedModuleApi-test.tsx +++ b/test/unit-tests/modules/ProxiedModuleApi-test.tsx @@ -24,6 +24,7 @@ import defaultDispatcher from "../../../src/dispatcher/dispatcher"; import { Action } from "../../../src/dispatcher/actions"; import WidgetStore, { type IApp } from "../../../src/stores/WidgetStore"; import { Container, WidgetLayoutStore } from "../../../src/stores/widgets/WidgetLayoutStore"; +import * as navigator from "../../../src/utils/permalinks/navigator.ts"; describe("ProxiedApiModule", () => { afterEach(() => { @@ -361,4 +362,30 @@ describe("ProxiedApiModule", () => { expect(WidgetLayoutStore.instance.moveToContainer).toHaveBeenCalledWith(room, app, Container.Top); }); }); + + describe("navigatePermalink", () => { + const api = new ProxiedModuleApi(); + + it("should call navigateToPermalink with the correct parameters", async () => { + const link = "https://matrix.to/#/!roomId:server.com"; + const spy = jest.spyOn(navigator, "navigateToPermalink"); + + await api.navigatePermalink(link); + expect(spy).toHaveBeenCalledWith(link); + }); + + it("should set auto_join to true when join=true", async () => { + const link = "https://matrix.to/#/#alias:server.com"; + const spy = jest.spyOn(defaultDispatcher, "dispatch"); + + await api.navigatePermalink(link, true); + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ + action: "view_room", + room_alias: "#alias:server.com", + auto_join: true, + }), + ); + }); + }); }); diff --git a/test/unit-tests/stores/RoomViewStore-test.ts b/test/unit-tests/stores/RoomViewStore-test.ts index 53840b0c32..4bd799c7aa 100644 --- a/test/unit-tests/stores/RoomViewStore-test.ts +++ b/test/unit-tests/stores/RoomViewStore-test.ts @@ -43,6 +43,7 @@ import { type IApp } from "../../../src/utils/WidgetUtils-types"; import { CallStore } from "../../../src/stores/CallStore"; import { MatrixClientPeg } from "../../../src/MatrixClientPeg"; import MediaDeviceHandler, { MediaDeviceKindEnum } from "../../../src/MediaDeviceHandler"; +import { storeRoomAliasInCache } from "../../../src/RoomAliasCache.ts"; jest.mock("../../../src/Modal"); @@ -211,6 +212,22 @@ describe("RoomViewStore", function () { expect(roomViewStore.isJoining()).toBe(true); }); + it("can be used to view a room by alias with auto_join", async () => { + const alias = "#alias12345:server"; + storeRoomAliasInCache(alias, roomId, ["server1"]); + dis.dispatch({ action: Action.ViewRoom, room_alias: alias, auto_join: true }, true); + await expect(untilDispatch(Action.ViewRoom, dis)).resolves.toEqual( + expect.objectContaining({ + action: Action.ViewRoom, + room_id: roomId, + auto_join: true, + }), + ); + await untilDispatch(Action.JoinRoomReady, dis); + expect(mockClient.joinRoom).toHaveBeenCalledWith(alias, { viaServers: ["server1"] }); + expect(roomViewStore.isJoining()).toBe(true); + }); + it("can auto-join a room", async () => { dis.dispatch({ action: Action.ViewRoom, room_id: roomId, auto_join: true }); await untilDispatch(Action.JoinRoomReady, dis); @@ -422,8 +439,8 @@ describe("RoomViewStore", function () { }); describe("Action.JoinRoom", () => { - it("dispatches Action.JoinRoomError and Action.AskToJoin when the join fails", async () => { - const err = new MatrixError(); + it("dispatches Action.JoinRoomError and Action.AskToJoin when the join fails with 403", async () => { + const err = new MatrixError({}, 403); jest.spyOn(dis, "dispatch"); jest.spyOn(mockClient, "joinRoom").mockRejectedValueOnce(err); diff --git a/test/unit-tests/stores/SpaceStore-test.ts b/test/unit-tests/stores/SpaceStore-test.ts index 6b6b6c1bc1..fc05cf5fed 100644 --- a/test/unit-tests/stores/SpaceStore-test.ts +++ b/test/unit-tests/stores/SpaceStore-test.ts @@ -41,6 +41,7 @@ import RoomListStore from "../../../src/stores/room-list/RoomListStore"; import { DefaultTagID } from "../../../src/stores/room-list/models"; import { RoomNotificationStateStore } from "../../../src/stores/notifications/RoomNotificationStateStore"; import { NotificationLevel } from "../../../src/stores/notifications/NotificationLevel"; +import { storeRoomAliasInCache } from "../../../src/RoomAliasCache.ts"; jest.useFakeTimers(); @@ -1217,6 +1218,15 @@ describe("SpaceStore", () => { viewRoom(room1); expect(store.activeSpace).toBe(MetaSpace.Home); }); + + it("should check alias cache if switching to a room by alias", async () => { + viewRoom(room2); + storeRoomAliasInCache("#alias:server", room3, []); + + store.setActiveSpace(space2, false); + defaultDispatcher.dispatch({ action: Action.ViewRoom, room_alias: "#alias:server" }, true); + expect(store.activeSpace).toBe(space1); + }); }); describe("traverseSpace", () => {