From faeee2e5a44e8c08823e9fec39e83322721e2f84 Mon Sep 17 00:00:00 2001 From: Denis Tingaikin Date: Sat, 25 Jan 2025 07:42:35 +0300 Subject: [PATCH] Address PR review comments: correctly handle edge cases in canDisplayLinkPreview (#7796) --- .../src/___tests___/link-preview.test.ts | 146 ++++++++++++++++++ packages/presentation/src/link-preview.ts | 27 ++-- 2 files changed, 163 insertions(+), 10 deletions(-) create mode 100644 packages/presentation/src/___tests___/link-preview.test.ts diff --git a/packages/presentation/src/___tests___/link-preview.test.ts b/packages/presentation/src/___tests___/link-preview.test.ts new file mode 100644 index 0000000000..6db691fb03 --- /dev/null +++ b/packages/presentation/src/___tests___/link-preview.test.ts @@ -0,0 +1,146 @@ +// +// Copyright © 2025 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public 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 https://www.eclipse.org/legal/epl-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 { canDisplayLinkPreview, fetchLinkPreviewDetails, type LinkPreviewDetails } from '../link-preview' +import { setMetadata } from '@hcengineering/platform' +import plugin from '../plugin' + +const fechFunc = + (responseFunc: (input: RequestInfo | URL, init?: RequestInit) => Response) => + async (input: RequestInfo | URL, init?: RequestInit): Promise => { + return responseFunc(input, init) + } + +describe('canDisplayLinkPreview', () => { + it('should return false if hostname & title are undefined', () => { + const linkDetails: LinkPreviewDetails = { + image: 'test-image.jpg', + description: 'Test Description' + } + expect(canDisplayLinkPreview(linkDetails)).toBe(false) + }) + + it('should return false if both image and description are missing', () => { + const linkDetails: LinkPreviewDetails = { + hostname: 'www.example.com', + title: 'Test Title' + } + expect(canDisplayLinkPreview(linkDetails)).toBe(false) + }) + + it('should return false if both title and description are empty', () => { + const linkDetails: LinkPreviewDetails = { + hostname: 'www.example.com', + image: 'test-image.jpg', + title: '', + description: '' + } + expect(canDisplayLinkPreview(linkDetails)).toBe(false) + }) + + it('should return true if hostname, image, and title are present', () => { + const linkDetails: LinkPreviewDetails = { + hostname: 'www.example.com', + image: 'test-image.jpg', + title: 'Test Title' + } + expect(canDisplayLinkPreview(linkDetails)).toBe(true) + }) + + it('should return true if hostname, description, and title are present', () => { + const linkDetails: LinkPreviewDetails = { + hostname: 'www.example.com', + title: 'Test Title', + description: 'Test Description' + } + expect(canDisplayLinkPreview(linkDetails)).toBe(true) + }) + + it('should return false if hostname, image, and description are present', () => { + const linkDetails: LinkPreviewDetails = { + hostname: 'www.example.com', + image: 'test-image.jpg', + description: 'Test Description' + } + expect(canDisplayLinkPreview(linkDetails)).toBe(false) + }) + + it('should handle whitespace in title and description', () => { + const linkDetails: LinkPreviewDetails = { + hostname: 'www.example.com', + image: 'test-image.jpg', + title: ' ', + description: ' ' + } + expect(canDisplayLinkPreview(linkDetails)).toBe(false) + }) + + it('should fetch link preview details successfully', async () => { + const fetcher = fechFunc((url) => + Response.json({ + title: 'Test Title', + description: 'Test Description', + url: 'https://www.example.com', + icon: 'https://www.example.com/favicon.ico', + image: 'https://www.example.com/image.jpg' + }) + ) + const linkDetails = await fetchLinkPreviewDetails('https://www.example.com', 5000, fetcher) + expect(linkDetails).toEqual({ + title: 'Test Title', + description: 'Test Description', + url: 'https://www.example.com', + icon: 'https://www.example.com/favicon.ico', + image: 'https://www.example.com/image.jpg' + }) + }) + + it('should handle errors during fetching', async () => { + const fetcher = fechFunc(() => { + throw new Error('something went wrong') + }) + expect(await fetchLinkPreviewDetails('https://www.example.com', 5000, fetcher)).toEqual({}) + }) + + it('should handle timeout', async () => { + let fetcherCalled = false + const fetcher = fechFunc((req, params) => { + if (params?.signal === undefined) { + fail('missed signal') + } + fetcherCalled = true + return Response.json({}) + }) + await fetchLinkPreviewDetails('https://www.example.com', 50, fetcher) + expect(fetcherCalled).toBe(true) + }) + + it('should add auth token', async () => { + let tokenProvided = false + setMetadata(plugin.metadata.Token, 'token') + const fetcher = fechFunc((req, params) => { + if (!(params?.headers instanceof Headers)) { + fail('missed headers') + } + if (params.headers.get('Authorization') !== 'Bearer token') { + fail('missed token') + } + tokenProvided = true + return Response.json({}) + }) + await fetchLinkPreviewDetails('https://www.example.com', 50, fetcher) + expect(tokenProvided).toBe(true) + }) +}) diff --git a/packages/presentation/src/link-preview.ts b/packages/presentation/src/link-preview.ts index 02c4e82e2a..8028c5eb20 100644 --- a/packages/presentation/src/link-preview.ts +++ b/packages/presentation/src/link-preview.ts @@ -30,27 +30,30 @@ export interface LinkPreviewDetails { } export function canDisplayLinkPreview (val: LinkPreviewDetails): boolean { - if (val.hostname === undefined) { + if (isEmpty(val.host) && isEmpty(val.title)) { return false } - if (val.image === undefined && val.description?.trim() === '') { - return false - } - if (val.title?.trim() === '' && val.description?.trim() === '') { + if (isEmpty(val.description) && isEmpty(val.image)) { return false } + return true } -export async function fetchLinkPreviewDetails (url: string, timeoutMs = 15000): Promise { +export async function fetchLinkPreviewDetails ( + url: string, + timeoutMs = 5000, + fetcher = fetch +): Promise { try { const linkPreviewUrl = getMetadata(plugin.metadata.LinkPreviewUrl) - let token: string = '' + const headers = new Headers() if (getMetadata(plugin.metadata.Token) !== undefined) { - token = getMetadata(plugin.metadata.Token) as string + const token = getMetadata(plugin.metadata.Token) as string + headers.set('Authorization', 'Bearer ' + token) } - const response = await fetch(`${linkPreviewUrl}/details?q=${url}`, { - headers: { Authorization: 'Bearer ' + token }, + const response = await fetcher(`${linkPreviewUrl}/details?q=${url}`, { + headers, signal: AbortSignal.timeout(timeoutMs) }) const res = (await response.json()) as LinkPreviewDetails @@ -60,3 +63,7 @@ export async function fetchLinkPreviewDetails (url: string, timeoutMs = 15000): return {} } } + +function isEmpty (str: string | undefined): boolean { + return str === undefined || str.trim() === '' +}