From 065371814bbd203b8d59a39f87ec6f85803beae4 Mon Sep 17 00:00:00 2001 From: Mateo Date: Sun, 5 Jul 2026 21:34:41 +0200 Subject: [PATCH] fix(link-preview): implement batch link preview to avoid multiple requests & improve data loading for tasks by showing skeleton after a timetout to avoid flashbang --- .../Controllers/Api/LinkPreviewController.php | 102 +++++++++++------- .../js/components/tasks/LinkPreviewCard.vue | 8 +- resources/js/pages/Tasks.vue | 8 +- resources/js/utils/linkPreviewBatcher.ts | 82 ++++++++++++++ routes/tasks.php | 2 +- tests/Feature/LinkPreviewTest.php | 67 +++++------- 6 files changed, 179 insertions(+), 90 deletions(-) create mode 100644 resources/js/utils/linkPreviewBatcher.ts diff --git a/app/Http/Controllers/Api/LinkPreviewController.php b/app/Http/Controllers/Api/LinkPreviewController.php index 0df6f08..8a0b04d 100644 --- a/app/Http/Controllers/Api/LinkPreviewController.php +++ b/app/Http/Controllers/Api/LinkPreviewController.php @@ -9,67 +9,87 @@ class LinkPreviewController extends Controller { - public function show(Request $request) + public function batch(Request $request) { $request->validate([ - 'url' => 'required|url', + 'urls' => 'required|array', + 'urls.*' => 'required|url', ]); - $url = $request->input('url'); + $urls = array_unique($request->input('urls')); + $results = []; + $urlsToFetch = []; - $host = strtolower(parse_url($url, PHP_URL_HOST) ?? ''); $localHosts = ['localhost', '127.0.0.1', '127.0.0.2', '::1', strtolower(parse_url(config('app.url', ''), PHP_URL_HOST) ?? '')]; - $serverHost = strtolower(parse_url($request->getSchemeAndHttpHost(), PHP_URL_HOST) ?? ''); if ($serverHost) { $localHosts[] = $serverHost; } - if (in_array($host, $localHosts)) { - return response()->json([ - 'url' => $url, - 'title' => $host ?: 'Local Link', - 'description' => 'Link to local page', - 'image' => null, - 'favicon' => null, - ]); - } - - // Cache the preview for 24 hours - $cacheKey = 'link_preview_' . md5($url); - - $data = Cache::remember($cacheKey, now()->addDay(), function () use ($url) { - try { - return $this->fetchPreviewData($url); - } catch (\Exception $e) { - // Cache the fallback response on failure to prevent repeated timeouts - // that could exhaust server workers if a link is dead. - return [ + foreach ($urls as $url) { + $host = strtolower(parse_url($url, PHP_URL_HOST) ?? ''); + + if (in_array($host, $localHosts)) { + $results[$url] = [ 'url' => $url, - 'title' => parse_url($url, PHP_URL_HOST), - 'description' => 'Failed to fetch link preview', + 'title' => $host ?: 'Local Link', + 'description' => 'Link to local page', 'image' => null, 'favicon' => null, ]; + continue; } - }); - return response()->json($data); - } + $cacheKey = 'link_preview_' . md5($url); + $cached = Cache::get($cacheKey); - private function fetchPreviewData(string $url): array - { - // Set short timeout to prevent blocking server threads - $response = Http::timeout(3) - ->withHeaders([ - 'User-Agent' => 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36', - ]) - ->get($url); - - if (!$response->successful()) { - throw new \Exception('HTTP request failed with status ' . $response->status()); + if ($cached) { + $results[$url] = $cached; + } else { + $urlsToFetch[] = $url; + } + } + + if (!empty($urlsToFetch)) { + $responses = Http::pool(function ($pool) use ($urlsToFetch) { + return array_map(function ($url) use ($pool) { + return $pool->timeout(3) + ->withHeaders([ + 'User-Agent' => 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36', + ]) + ->get($url); + }, $urlsToFetch); + }); + + foreach ($urlsToFetch as $index => $url) { + $response = $responses[$index]; + $cacheKey = 'link_preview_' . md5($url); + + try { + if ($response instanceof \Exception || !$response->successful()) { + throw new \Exception('Failed to fetch ' . $url); + } + $data = $this->parseResponse($response, $url); + } catch (\Exception $e) { + $data = [ + 'url' => $url, + 'title' => parse_url($url, PHP_URL_HOST), + 'description' => '', + 'image' => null, + 'favicon' => null, + ]; + } + + Cache::put($cacheKey, $data, now()->addDay()); + $results[$url] = $data; + } } + return response()->json($results); + } + + private function parseResponse($response, string $url): array + { $contentType = $response->header('Content-Type', ''); if (strpos($contentType, 'text/html') === false) { return [ diff --git a/resources/js/components/tasks/LinkPreviewCard.vue b/resources/js/components/tasks/LinkPreviewCard.vue index a1fa12e..f6629d2 100644 --- a/resources/js/components/tasks/LinkPreviewCard.vue +++ b/resources/js/components/tasks/LinkPreviewCard.vue @@ -2,6 +2,7 @@ import { onMounted, ref } from 'vue'; import type { PreviewData } from '@/types'; import { X } from 'lucide-vue-next'; +import { fetchPreview } from '@/utils/linkPreviewBatcher'; const props = defineProps<{ url: string }>(); const emit = defineEmits<{ (e: 'dismiss'): void }>(); @@ -12,12 +13,7 @@ const failed = ref(false); onMounted(async () => { try { - const res = await fetch(`/link-preview?url=${encodeURIComponent(props.url)}`, { - headers: { Accept: 'application/json' }, - }); - if (!res.ok) throw new Error(`HTTP ${res.status}`); - const data: PreviewData = await res.json(); - preview.value = data; + preview.value = await fetchPreview(props.url); } catch { failed.value = true; } finally { diff --git a/resources/js/pages/Tasks.vue b/resources/js/pages/Tasks.vue index 4542f07..59e8ee3 100644 --- a/resources/js/pages/Tasks.vue +++ b/resources/js/pages/Tasks.vue @@ -32,6 +32,7 @@ const breadcrumbs: BreadcrumbItem[] = [ const isEditModalOpen = ref(false); const taskToEdit = ref(null); const localColumns = ref(null); +const startTime = Date.now(); const page = usePage(); const taskIdFromUrl = computed(() => { @@ -63,12 +64,15 @@ const handleFilterChange = (newFilters: any) => { }); }; - watch( () => props.columns, (newCols) => { if (newCols !== undefined) { - localColumns.value = newCols; + const elapsed = Date.now() - startTime; + const remaining = Math.max(0, 500 - elapsed); + setTimeout(() => { + localColumns.value = newCols; + }, remaining); } }, { immediate: true }, diff --git a/resources/js/utils/linkPreviewBatcher.ts b/resources/js/utils/linkPreviewBatcher.ts new file mode 100644 index 0000000..70becdc --- /dev/null +++ b/resources/js/utils/linkPreviewBatcher.ts @@ -0,0 +1,82 @@ +import type { PreviewData } from '@/types'; + +// Client-side batch cache to avoid fetching the same link twice in the same session +const cache = new Map(); + +const queue = new Set(); + +type PromiseCallbacks = { + resolve: (data: PreviewData) => void; + reject: (err: any) => void; +}; +const pending = new Map(); + +let flushTimeoutId: ReturnType | null = null; + +const flushQueue = async () => { + flushTimeoutId = null; + + const urlsToFetch = Array.from(queue); + queue.clear(); + + if (urlsToFetch.length === 0) return; + + try { + const params = new URLSearchParams(); + urlsToFetch.forEach((url) => { + params.append('urls[]', url); + }); + + const response = await fetch(`/link-previews/batch?${params.toString()}`, { + headers: { + Accept: 'application/json', + }, + }); + + if (!response.ok) { + throw new Error(`Batch request failed: HTTP ${response.status}`); + } + + const data: Record = await response.json(); + + urlsToFetch.forEach((url) => { + const callbacks = pending.get(url) || []; + pending.delete(url); + + const result = data[url]; + if (result) { + cache.set(url, result); + callbacks.forEach(({ resolve }) => resolve(result)); + } else { + callbacks.forEach(({ reject }) => reject(new Error('No preview returned'))); + } + }); + } catch (err) { + urlsToFetch.forEach((url) => { + const callbacks = pending.get(url) || []; + pending.delete(url); + callbacks.forEach(({ reject }) => reject(err)); + }); + } +}; + +const scheduleFlush = () => { + if (flushTimeoutId !== null) return; + flushTimeoutId = setTimeout(flushQueue, 50); +}; + +export const fetchPreview = (url: string): Promise => { + const cached = cache.get(url); + if (cached) { + return Promise.resolve(cached); + } + + return new Promise((resolve, reject) => { + const callbacks = pending.get(url) || []; + callbacks.push({ resolve, reject }); + pending.set(url, callbacks); + + queue.add(url); + scheduleFlush(); + }); +}; diff --git a/routes/tasks.php b/routes/tasks.php index bd6348c..d7ecad2 100644 --- a/routes/tasks.php +++ b/routes/tasks.php @@ -28,7 +28,7 @@ Route::put('tasks/{task}/sequence', [TaskSequenceController::class, 'update'])->name('tasks.sequence.update'); Route::get('columns/{column}/tasks', [ColumnTaskController::class, 'index'])->name('columns.tasks.index'); - Route::get('link-preview', [LinkPreviewController::class, 'show'])->name('link-preview.show'); + Route::get('link-previews/batch', [LinkPreviewController::class, 'batch'])->name('link-preview.batch'); Route::get('attachments/{attachment}', [TaskAttachmentController::class, 'show'])->name('attachments.show'); Route::delete('attachments/{attachment}', [TaskAttachmentController::class, 'destroy'])->name('attachments.destroy'); diff --git a/tests/Feature/LinkPreviewTest.php b/tests/Feature/LinkPreviewTest.php index 114346c..5d0583d 100644 --- a/tests/Feature/LinkPreviewTest.php +++ b/tests/Feature/LinkPreviewTest.php @@ -25,71 +25,58 @@ protected function setUp(): void ]); } - public function test_unauthenticated_users_cannot_access_link_preview() + public function test_unauthenticated_users_cannot_access_link_preview_batch() { - $response = $this->getJson('/link-preview?url=' . urlencode('https://example.com')); + $response = $this->getJson('/link-previews/batch?urls[]=' . urlencode('https://example.com')); $response->assertStatus(401); } - public function test_url_parameter_is_required_and_must_be_valid() + public function test_batch_endpoint_requires_urls_array_and_valid_urls() { $response = $this->actingAs($this->user) - ->getJson('/link-preview'); + ->getJson('/link-previews/batch'); $response->assertStatus(422) - ->assertJsonValidationErrors(['url']); + ->assertJsonValidationErrors(['urls']); $response = $this->actingAs($this->user) - ->getJson('/link-preview?url=not-a-url'); + ->getJson('/link-previews/batch?urls[]=not-a-url'); $response->assertStatus(422) - ->assertJsonValidationErrors(['url']); + ->assertJsonValidationErrors(['urls.0']); } - public function test_can_fetch_and_parse_link_preview() + public function test_can_fetch_link_previews_in_batch() { Http::fake([ - 'https://example.com*' => Http::response( - ' - - Example Domain - - - - - Hello - ', + 'https://example1.com*' => Http::response( + 'Title One', 200, ['Content-Type' => 'text/html'] - ) + ), + 'https://example2.com*' => Http::response( + 'Title Two', + 200, + ['Content-Type' => 'text/html'] + ), ]); $response = $this->actingAs($this->user) - ->getJson('/link-preview?url=' . urlencode('https://example.com/some-page')); - - $response->assertStatus(200) - ->assertJson([ - 'url' => 'https://example.com/some-page', - 'title' => 'Example Domain', - 'description' => 'This is an example description', - 'image' => 'https://example.com/images/og.png', - 'favicon' => 'https://example.com/favicon.ico', - ]); - } - - public function test_local_urls_are_not_fetched_and_return_mock_data() - { - Http::preventStrayRequests(); - - $response = $this->actingAs($this->user) - ->getJson('/link-preview?url=' . urlencode('http://127.0.0.1:8000/some-local-path')); + ->getJson('/link-previews/batch?urls[]=' . urlencode('https://example1.com') . '&urls[]=' . urlencode('https://example2.com')); $response->assertStatus(200) ->assertJson([ - 'title' => '127.0.0.1', - 'description' => 'Link to local page', - 'image' => null, + 'https://example1.com' => [ + 'url' => 'https://example1.com', + 'title' => 'Title One', + 'description' => 'Desc One', + ], + 'https://example2.com' => [ + 'url' => 'https://example2.com', + 'title' => 'Title Two', + 'description' => 'Desc Two', + ], ]); } }