From 773492d4581b210bd2acf6894f15c7a43fdff93c Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 2 Dec 2024 13:54:00 +0000 Subject: [PATCH 1/7] Load authenicated media --- src/Avatar.tsx | 51 +++++++++++++++++++++++++++++++++++++++------ src/utils/matrix.ts | 14 +------------ 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/src/Avatar.tsx b/src/Avatar.tsx index 29ab52362..f1d783f34 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -5,11 +5,11 @@ SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { useMemo, FC, CSSProperties } from "react"; +import { useMemo, FC, CSSProperties, useState, useEffect } from "react"; import { Avatar as CompoundAvatar } from "@vector-im/compound-web"; -import { getAvatarUrl } from "./utils/matrix"; import { useClient } from "./ClientContext"; +import { MatrixClient } from "matrix-js-sdk/src/client"; export enum Size { XS = "xs", @@ -36,6 +36,18 @@ interface Props { style?: CSSProperties; } +export function getAvatarUrl( + client: MatrixClient, + mxcUrl: string|null, + avatarSize = 96, +): string|null { + const width = Math.floor(avatarSize * window.devicePixelRatio); + const height = Math.floor(avatarSize * window.devicePixelRatio); + // scale is more suitable for larger sizes + const resizeMethod = avatarSize <= 96 ? "crop" : "scale"; + return mxcUrl ? client.mxcUrlToHttp(mxcUrl, width, height, resizeMethod, false, true, true) : null; +} + export const Avatar: FC = ({ className, id, @@ -55,9 +67,36 @@ export const Avatar: FC = ({ [size], ); - const resolvedSrc = useMemo(() => { - if (!client || !src || !sizePx) return undefined; - return src.startsWith("mxc://") ? getAvatarUrl(client, src, sizePx) : src; + const [avatarUrl, setAvatarUrl] = useState(undefined); + + useEffect(() => { + if (!client || !src || !sizePx) { + return; + }; + const token = client.getAccessToken(); + if (!token) { + return; + } + const resolveSrc = getAvatarUrl(client, src, sizePx); + if (!resolveSrc) { + setAvatarUrl(undefined); + return; + } + + let objectUrl: string|undefined; + fetch(resolveSrc, { + headers: { + 'Authorization': `Bearer ${token}` + } + }).then((req) => req.blob()).then((res) => { + objectUrl = URL.createObjectURL(res); + setAvatarUrl(objectUrl); + }).catch((ex) => { + console.warn("Failed to get avatar URL", ex); + setAvatarUrl(undefined); + }); + + () => objectUrl && URL.revokeObjectURL(objectUrl); }, [client, src, sizePx]); return ( @@ -66,7 +105,7 @@ export const Avatar: FC = ({ id={id} name={name} size={`${sizePx}px`} - src={resolvedSrc} + src={avatarUrl} style={style} {...props} /> diff --git a/src/utils/matrix.ts b/src/utils/matrix.ts index d3821a3ff..46b265289 100644 --- a/src/utils/matrix.ts +++ b/src/utils/matrix.ts @@ -332,16 +332,4 @@ export function getRelativeRoomUrl( ? "/" + roomAliasLocalpartFromRoomName(roomName) : ""; return `/room/#${roomPart}?${generateUrlSearchParams(roomId, encryptionSystem, viaServers).toString()}`; -} - -export function getAvatarUrl( - client: MatrixClient, - mxcUrl: string, - avatarSize = 96, -): string { - const width = Math.floor(avatarSize * window.devicePixelRatio); - const height = Math.floor(avatarSize * window.devicePixelRatio); - // scale is more suitable for larger sizes - const resizeMethod = avatarSize <= 96 ? "crop" : "scale"; - return mxcUrl && client.mxcUrlToHttp(mxcUrl, width, height, resizeMethod)!; -} +} \ No newline at end of file From 74fa5e4bdf48c41654b4796c5350029f614658c2 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 2 Dec 2024 13:56:55 +0000 Subject: [PATCH 2/7] lint --- src/Avatar.tsx | 53 ++++++++++++++++++++++++++++++--------------- src/utils/matrix.ts | 2 +- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/Avatar.tsx b/src/Avatar.tsx index f1d783f34..84b0ed48f 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -7,9 +7,9 @@ Please see LICENSE in the repository root for full details. import { useMemo, FC, CSSProperties, useState, useEffect } from "react"; import { Avatar as CompoundAvatar } from "@vector-im/compound-web"; +import { MatrixClient } from "matrix-js-sdk/src/client"; import { useClient } from "./ClientContext"; -import { MatrixClient } from "matrix-js-sdk/src/client"; export enum Size { XS = "xs", @@ -38,14 +38,24 @@ interface Props { export function getAvatarUrl( client: MatrixClient, - mxcUrl: string|null, + mxcUrl: string | null, avatarSize = 96, -): string|null { +): string | null { const width = Math.floor(avatarSize * window.devicePixelRatio); const height = Math.floor(avatarSize * window.devicePixelRatio); // scale is more suitable for larger sizes const resizeMethod = avatarSize <= 96 ? "crop" : "scale"; - return mxcUrl ? client.mxcUrlToHttp(mxcUrl, width, height, resizeMethod, false, true, true) : null; + return mxcUrl + ? client.mxcUrlToHttp( + mxcUrl, + width, + height, + resizeMethod, + false, + true, + true, + ) + : null; } export const Avatar: FC = ({ @@ -67,12 +77,12 @@ export const Avatar: FC = ({ [size], ); - const [avatarUrl, setAvatarUrl] = useState(undefined); - + const [avatarUrl, setAvatarUrl] = useState(undefined); + useEffect(() => { if (!client || !src || !sizePx) { return; - }; + } const token = client.getAccessToken(); if (!token) { return; @@ -83,20 +93,27 @@ export const Avatar: FC = ({ return; } - let objectUrl: string|undefined; + let objectUrl: string | undefined; fetch(resolveSrc, { headers: { - 'Authorization': `Bearer ${token}` - } - }).then((req) => req.blob()).then((res) => { - objectUrl = URL.createObjectURL(res); - setAvatarUrl(objectUrl); - }).catch((ex) => { - console.warn("Failed to get avatar URL", ex); - setAvatarUrl(undefined); - }); + Authorization: `Bearer ${token}`, + }, + }) + .then(async (req) => req.blob()) + .then((res) => { + objectUrl = URL.createObjectURL(res); + setAvatarUrl(objectUrl); + }) + .catch(() => { + // Error will likely be evident from browser logs. + setAvatarUrl(undefined); + }); - () => objectUrl && URL.revokeObjectURL(objectUrl); + return (): void => { + if (objectUrl) { + URL.revokeObjectURL(objectUrl); + } + }; }, [client, src, sizePx]); return ( diff --git a/src/utils/matrix.ts b/src/utils/matrix.ts index 46b265289..63b6ef67a 100644 --- a/src/utils/matrix.ts +++ b/src/utils/matrix.ts @@ -332,4 +332,4 @@ export function getRelativeRoomUrl( ? "/" + roomAliasLocalpartFromRoomName(roomName) : ""; return `/room/#${roomPart}?${generateUrlSearchParams(roomId, encryptionSystem, viaServers).toString()}`; -} \ No newline at end of file +} From 6bf07f0d4f74223f922231f8d7b89a487ac2e99f Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 2 Dec 2024 15:24:54 +0000 Subject: [PATCH 3/7] Add tests --- src/Avatar.test.tsx | 122 ++++++++++++++++++++++++++++++++++++++++++ src/Avatar.tsx | 7 ++- src/ClientContext.tsx | 2 + 3 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 src/Avatar.test.tsx diff --git a/src/Avatar.test.tsx b/src/Avatar.test.tsx new file mode 100644 index 000000000..9df6384c1 --- /dev/null +++ b/src/Avatar.test.tsx @@ -0,0 +1,122 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only +Please see LICENSE in the repository root for full details. +*/ + +import { afterEach, expect, test, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { MatrixClient } from "matrix-js-sdk/src/client"; +import { FC, PropsWithChildren } from "react"; + +import { ClientContextProvider } from "./ClientContext"; +import { Avatar } from "./Avatar"; +import { mockMatrixRoomMember } from "./utils/test"; + +const TestComponent: FC> = ({ + client, + children, +}) => { + return ( + + {children} + + ); +}; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +test("should just render a placeholder when the user has no avatar", () => { + const client = vi.mocked({ + getAccessToken: () => "my-access-token", + mxcUrlToHttp: () => vi.fn(), + } as unknown as MatrixClient); + + vi.spyOn(client, "mxcUrlToHttp"); + const member = mockMatrixRoomMember({ + userId: "@alice:example.org", + getMxcAvatarUrl: () => undefined, + }); + const displayName = "Alice"; + render( + + + , + ); + const element = screen.getByRole("img", { name: "@alice:example.org" }); + expect(element.tagName).toEqual("SPAN"); + expect(client.mxcUrlToHttp).toBeCalledTimes(0); +}); +test("should attempt to fetch authenticated media", async () => { + const expectedAuthUrl = "http://example.org/media/alice-avatar"; + const expectedObjectURL = "my-object-url"; + const accessToken = "my-access-token"; + const theBlob = new Blob([]); + + // vitest doesn't have a implementation of create/revokeObjectURL, so we need + // to delete the property. It's a bit odd, but it works. + Reflect.deleteProperty(global.window.URL, "createObjectURL"); + globalThis.URL.createObjectURL = vi.fn().mockReturnValue(expectedObjectURL); + Reflect.deleteProperty(global.window.URL, "revokeObjectURL"); + globalThis.URL.revokeObjectURL = vi.fn(); + + const fetchFn = vi.fn().mockResolvedValue({ + blob: async () => Promise.resolve(theBlob), + }); + vi.stubGlobal("fetch", fetchFn); + + const client = vi.mocked({ + getAccessToken: () => accessToken, + mxcUrlToHttp: () => vi.fn(), + } as unknown as MatrixClient); + + vi.spyOn(client, "mxcUrlToHttp").mockReturnValue(expectedAuthUrl); + const member = mockMatrixRoomMember({ + userId: "@alice:example.org", + getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", + }); + const displayName = "Alice"; + render( + + + , + ); + + // Fetch is asynchronous, so wait for this to resolve. + await vi.waitUntil(() => + document.querySelector(`img[src='${expectedObjectURL}']`), + ); + + expect(client.mxcUrlToHttp).toBeCalledTimes(1); + expect(globalThis.fetch).toBeCalledWith(expectedAuthUrl, { + headers: { Authorization: `Bearer ${accessToken}` }, + }); +}); diff --git a/src/Avatar.tsx b/src/Avatar.tsx index 84b0ed48f..a68c17f87 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -100,12 +100,11 @@ export const Avatar: FC = ({ }, }) .then(async (req) => req.blob()) - .then((res) => { - objectUrl = URL.createObjectURL(res); + .then((blob) => { + objectUrl = URL.createObjectURL(blob); setAvatarUrl(objectUrl); }) - .catch(() => { - // Error will likely be evident from browser logs. + .catch((ex) => { setAvatarUrl(undefined); }); diff --git a/src/ClientContext.tsx b/src/ClientContext.tsx index 8b5589d50..07b78dadf 100644 --- a/src/ClientContext.tsx +++ b/src/ClientContext.tsx @@ -71,6 +71,8 @@ export type SetClientParams = { const ClientContext = createContext(undefined); +export const ClientContextProvider = ClientContext.Provider; + export const useClientState = (): ClientState | undefined => useContext(ClientContext); From d76d316016542793f0ce32c9dbc742bbc10489cf Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 2 Dec 2024 21:22:58 +0000 Subject: [PATCH 4/7] Add widget behaviour and test. --- src/Avatar.test.tsx | 36 ++++++++++++++++++++++++++++++++---- src/Avatar.tsx | 15 +++++++++++---- src/ClientContext.tsx | 7 +++++++ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/Avatar.test.tsx b/src/Avatar.test.tsx index 9df6384c1..0de68ce2e 100644 --- a/src/Avatar.test.tsx +++ b/src/Avatar.test.tsx @@ -14,10 +14,9 @@ import { ClientContextProvider } from "./ClientContext"; import { Avatar } from "./Avatar"; import { mockMatrixRoomMember } from "./utils/test"; -const TestComponent: FC> = ({ - client, - children, -}) => { +const TestComponent: FC< + PropsWithChildren<{ client: MatrixClient; supportsThumbnails?: boolean }> +> = ({ client, children, supportsThumbnails }) => { return ( > = ({ disconnected: false, supportedFeatures: { reactions: true, + thumbnails: supportsThumbnails ?? true, }, setClient: vi.fn(), authenticated: { @@ -70,6 +70,34 @@ test("should just render a placeholder when the user has no avatar", () => { expect(element.tagName).toEqual("SPAN"); expect(client.mxcUrlToHttp).toBeCalledTimes(0); }); + +test("should just render a placeholder when thumbnaisl are not supported", () => { + const client = vi.mocked({ + getAccessToken: () => "my-access-token", + mxcUrlToHttp: () => vi.fn(), + } as unknown as MatrixClient); + + vi.spyOn(client, "mxcUrlToHttp"); + const member = mockMatrixRoomMember({ + userId: "@alice:example.org", + getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", + }); + const displayName = "Alice"; + render( + + + , + ); + const element = screen.getByRole("img", { name: "@alice:example.org" }); + expect(element.tagName).toEqual("SPAN"); + expect(client.mxcUrlToHttp).toBeCalledTimes(0); +}); + test("should attempt to fetch authenticated media", async () => { const expectedAuthUrl = "http://example.org/media/alice-avatar"; const expectedObjectURL = "my-object-url"; diff --git a/src/Avatar.tsx b/src/Avatar.tsx index a68c17f87..f3fe6cd87 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -9,7 +9,7 @@ import { useMemo, FC, CSSProperties, useState, useEffect } from "react"; import { Avatar as CompoundAvatar } from "@vector-im/compound-web"; import { MatrixClient } from "matrix-js-sdk/src/client"; -import { useClient } from "./ClientContext"; +import { useClientState } from "./ClientContext"; export enum Size { XS = "xs", @@ -67,7 +67,7 @@ export const Avatar: FC = ({ style, ...props }) => { - const { client } = useClient(); + const clientState = useClientState(); const sizePx = useMemo( () => @@ -80,9 +80,16 @@ export const Avatar: FC = ({ const [avatarUrl, setAvatarUrl] = useState(undefined); useEffect(() => { - if (!client || !src || !sizePx) { + if (clientState?.state !== "valid") { return; } + const { authenticated, supportedFeatures } = clientState; + const client = authenticated?.client; + + if (!client || !src || !sizePx || !supportedFeatures.thumbnails) { + return; + } + const token = client.getAccessToken(); if (!token) { return; @@ -113,7 +120,7 @@ export const Avatar: FC = ({ URL.revokeObjectURL(objectUrl); } }; - }, [client, src, sizePx]); + }, [clientState, src, sizePx]); return ( void; }; @@ -255,6 +256,7 @@ export const ClientProvider: FC = ({ children }) => { const [isDisconnected, setIsDisconnected] = useState(false); const [supportsReactions, setSupportsReactions] = useState(false); + const [supportsThumbnails, setSupportsThumbnails] = useState(false); const state: ClientState | undefined = useMemo(() => { if (alreadyOpenedErr) { @@ -280,6 +282,7 @@ export const ClientProvider: FC = ({ children }) => { disconnected: isDisconnected, supportedFeatures: { reactions: supportsReactions, + thumbnails: supportsThumbnails, }, }; }, [ @@ -290,6 +293,7 @@ export const ClientProvider: FC = ({ children }) => { setClient, isDisconnected, supportsReactions, + supportsThumbnails, ]); const onSync = useCallback( @@ -315,6 +319,8 @@ export const ClientProvider: FC = ({ children }) => { } if (initClientState.widgetApi) { + // There is currently no widget API for authenticated media thumbnails. + setSupportsThumbnails(false); const reactSend = initClientState.widgetApi.hasCapability( "org.matrix.msc2762.send.event:m.reaction", ); @@ -336,6 +342,7 @@ export const ClientProvider: FC = ({ children }) => { } } else { setSupportsReactions(true); + setSupportsThumbnails(true); } return (): void => { From 0b50df79275c1824d401f06aadbdec918e006503 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 6 Dec 2024 14:56:53 +0000 Subject: [PATCH 5/7] Update src/Avatar.test.tsx Co-authored-by: Hugh Nimmo-Smith --- src/Avatar.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avatar.test.tsx b/src/Avatar.test.tsx index 0de68ce2e..9814e6935 100644 --- a/src/Avatar.test.tsx +++ b/src/Avatar.test.tsx @@ -71,7 +71,7 @@ test("should just render a placeholder when the user has no avatar", () => { expect(client.mxcUrlToHttp).toBeCalledTimes(0); }); -test("should just render a placeholder when thumbnaisl are not supported", () => { +test("should just render a placeholder when thumbnails are not supported", () => { const client = vi.mocked({ getAccessToken: () => "my-access-token", mxcUrlToHttp: () => vi.fn(), From 2d0b6054bf83a7b9b2e8869e30debb64f8930c2d Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 6 Dec 2024 17:14:43 +0000 Subject: [PATCH 6/7] remove ?. --- src/Avatar.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Avatar.test.tsx b/src/Avatar.test.tsx index 9814e6935..d76a5f419 100644 --- a/src/Avatar.test.tsx +++ b/src/Avatar.test.tsx @@ -62,7 +62,7 @@ test("should just render a placeholder when the user has no avatar", () => { id={member.userId} name={displayName} size={96} - src={member?.getMxcAvatarUrl()} + src={member.getMxcAvatarUrl()} /> , ); @@ -89,7 +89,7 @@ test("should just render a placeholder when thumbnails are not supported", () => id={member.userId} name={displayName} size={96} - src={member?.getMxcAvatarUrl()} + src={member.getMxcAvatarUrl()} /> , ); @@ -133,7 +133,7 @@ test("should attempt to fetch authenticated media", async () => { id={member.userId} name={displayName} size={96} - src={member?.getMxcAvatarUrl()} + src={member.getMxcAvatarUrl()} /> , ); From 6d7e1831e7d7a56737a8870def7678fa5ae7bec5 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 6 Dec 2024 18:28:50 +0000 Subject: [PATCH 7/7] Fix tests --- src/Avatar.test.tsx | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Avatar.test.tsx b/src/Avatar.test.tsx index d76a5f419..7eee2e909 100644 --- a/src/Avatar.test.tsx +++ b/src/Avatar.test.tsx @@ -12,7 +12,7 @@ import { FC, PropsWithChildren } from "react"; import { ClientContextProvider } from "./ClientContext"; import { Avatar } from "./Avatar"; -import { mockMatrixRoomMember } from "./utils/test"; +import { mockMatrixRoomMember, mockRtcMembership } from "./utils/test"; const TestComponent: FC< PropsWithChildren<{ client: MatrixClient; supportsThumbnails?: boolean }> @@ -51,10 +51,12 @@ test("should just render a placeholder when the user has no avatar", () => { } as unknown as MatrixClient); vi.spyOn(client, "mxcUrlToHttp"); - const member = mockMatrixRoomMember({ - userId: "@alice:example.org", - getMxcAvatarUrl: () => undefined, - }); + const member = mockMatrixRoomMember( + mockRtcMembership("@alice:example.org", "AAAA"), + { + getMxcAvatarUrl: () => undefined, + }, + ); const displayName = "Alice"; render( @@ -78,10 +80,12 @@ test("should just render a placeholder when thumbnails are not supported", () => } as unknown as MatrixClient); vi.spyOn(client, "mxcUrlToHttp"); - const member = mockMatrixRoomMember({ - userId: "@alice:example.org", - getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", - }); + const member = mockMatrixRoomMember( + mockRtcMembership("@alice:example.org", "AAAA"), + { + getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", + }, + ); const displayName = "Alice"; render( @@ -122,10 +126,12 @@ test("should attempt to fetch authenticated media", async () => { } as unknown as MatrixClient); vi.spyOn(client, "mxcUrlToHttp").mockReturnValue(expectedAuthUrl); - const member = mockMatrixRoomMember({ - userId: "@alice:example.org", - getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", - }); + const member = mockMatrixRoomMember( + mockRtcMembership("@alice:example.org", "AAAA"), + { + getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", + }, + ); const displayName = "Alice"; render(