diff --git a/apps/server/src/lib/create-app.test.ts b/apps/server/src/lib/create-app.test.ts new file mode 100644 index 0000000..1a3f6bf --- /dev/null +++ b/apps/server/src/lib/create-app.test.ts @@ -0,0 +1,48 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { envMock, rateLimitMock } = vi.hoisted(() => ({ + envMock: { + BODY_SIZE_LIMIT: 4_194_304, + CORS_ORIGINS: 'http://localhost:3001', + isDevelopment: true, + isProduction: false + }, + rateLimitMock: vi.fn(async (_context: unknown, next: () => Promise) => next()) +})); + +vi.mock('./env-config.js', () => ({ env: envMock })); +vi.mock('../middlewares/rate-limit.js', () => ({ rateLimit: rateLimitMock })); + +import createApp from './create-app.js'; + +async function requestProbe() { + const app = createApp(); + app.get('/probe', (c) => c.text('ok')); + + return app.request('/probe'); +} + +describe('createApp global rate limiting', () => { + beforeEach(() => { + envMock.isDevelopment = true; + envMock.isProduction = false; + rateLimitMock.mockClear(); + }); + + it('skips the global rate limiter in development', async () => { + const response = await requestProbe(); + + expect(response.status).toBe(200); + expect(rateLimitMock).not.toHaveBeenCalled(); + }); + + it('applies the global rate limiter in production', async () => { + envMock.isDevelopment = false; + envMock.isProduction = true; + + const response = await requestProbe(); + + expect(response.status).toBe(200); + expect(rateLimitMock).toHaveBeenCalledOnce(); + }); +}); diff --git a/apps/server/src/lib/create-app.ts b/apps/server/src/lib/create-app.ts index c52d743..708937c 100644 --- a/apps/server/src/lib/create-app.ts +++ b/apps/server/src/lib/create-app.ts @@ -69,7 +69,7 @@ export default function createApp() { ) ); - if (!env.isTest) { + if (env.isProduction) { app.use('*', async (c, next) => { if (c.req.path === '/health') { return next(); diff --git a/apps/server/src/services/browser.service.test.ts b/apps/server/src/services/browser.service.test.ts new file mode 100644 index 0000000..14d40b0 --- /dev/null +++ b/apps/server/src/services/browser.service.test.ts @@ -0,0 +1,81 @@ +import type { BrowserContext, Page, Response } from 'playwright-core'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../index.js', () => ({ getIsShuttingDown: () => false })); + +import { browserService } from './browser.service.js'; + +type Deferred = { + promise: Promise; + resolve: () => void; +}; + +function deferred(): Deferred { + let resolve = () => {}; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + + return { promise, resolve }; +} + +function challengeResponse(): Response { + return { + headers: () => ({ 'x-amzn-waf-action': 'challenge' }) + } as unknown as Response; +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('browserService.crawlPage', () => { + it('waits for challenged article content before extracting the page', async () => { + const ready = deferred(); + const content = vi.fn().mockResolvedValue('
Ready
'); + const waitForFunction = vi.fn(() => ready.promise); + const page = { + content, + goto: vi.fn().mockResolvedValue(challengeResponse()), + waitForFunction, + waitForLoadState: vi.fn() + } as unknown as Page; + const context = { newPage: vi.fn().mockResolvedValue(page) } as unknown as BrowserContext; + + vi.spyOn(browserService, 'acquireContext').mockResolvedValue(context); + vi.spyOn(browserService, 'releaseContext').mockResolvedValue(); + + const crawl = browserService.crawlPage('https://arstechnica.com/example'); + await vi.waitFor(() => expect(waitForFunction).toHaveBeenCalledOnce()); + + expect(content).not.toHaveBeenCalled(); + + ready.resolve(); + await expect(crawl).resolves.toEqual({ html: '
Ready
', isPaywalled: false }); + expect(content).toHaveBeenCalledOnce(); + expect(waitForFunction).toHaveBeenCalledWith(expect.any(Function), undefined, { + polling: 250, + timeout: 15_000 + }); + }); + + it('releases the browser context when challenge readiness times out', async () => { + const challengeError = new Error('Challenge readiness timed out'); + const page = { + content: vi.fn(), + goto: vi.fn().mockResolvedValue(challengeResponse()), + waitForFunction: vi.fn().mockRejectedValue(challengeError), + waitForLoadState: vi.fn() + } as unknown as Page; + const context = { newPage: vi.fn().mockResolvedValue(page) } as unknown as BrowserContext; + + vi.spyOn(browserService, 'acquireContext').mockResolvedValue(context); + const releaseContext = vi.spyOn(browserService, 'releaseContext').mockResolvedValue(); + + await expect(browserService.crawlPage('https://arstechnica.com/example')).rejects.toThrow( + challengeError + ); + expect(releaseContext).toHaveBeenCalledWith(context); + expect(page.content).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/services/browser.service.ts b/apps/server/src/services/browser.service.ts index b68482b..2fef27d 100644 --- a/apps/server/src/services/browser.service.ts +++ b/apps/server/src/services/browser.service.ts @@ -96,19 +96,32 @@ class BrowserService { logger.info(`[Crawler] Navigating to ${url}...`); - await page.goto(url, { + const response = await page.goto(url, { timeout: 30_000, waitUntil: 'domcontentloaded' }); - logger.info(`[Crawler] Successfully navigated to ${url}. Waiting for page to load...`); - - await Promise.race([ - page.waitForLoadState('networkidle', { timeout: 5000 }).catch(() => {}), - new Promise((resolve) => setTimeout(resolve, 5000)) - ]); - - logger.info(`[Crawler] Successfully loaded ${url}. Extracting content...`); + const wafAction = response?.headers()['x-amzn-waf-action']; + + if (wafAction === 'challenge') { + await page.waitForFunction( + () => { + const articleText = document.querySelector('article')?.textContent?.trim() ?? ''; + const bodyText = document.body?.innerText.trim() ?? ''; + + return ( + document.title.length > 0 && Math.max(articleText.length, bodyText.length) >= 500 + ); + }, + undefined, + { + polling: 250, + timeout: 15_000 + } + ); + } else { + await page.waitForLoadState('networkidle', { timeout: 5_000 }).catch(() => {}); + } const html = await page.content(); diff --git a/apps/server/src/services/content-extraction-bookmark.integration.test.ts b/apps/server/src/services/content-extraction-bookmark.integration.test.ts new file mode 100644 index 0000000..272f744 --- /dev/null +++ b/apps/server/src/services/content-extraction-bookmark.integration.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; + +import { contentExtractionService } from './content-extraction.service.js'; +import { markdownService } from './markdown.service.js'; + +describe('Ghost bookmark extraction pipeline', () => { + it('preserves bookmark metadata through Readability and Markdown conversion', async () => { + const sourceUrl = 'https://publisher.example/articles/story'; + const html = ` + + + Example article + + + + + `; + + const readable = await contentExtractionService.extractReadableContent(html, sourceUrl); + + expect(readable.content).toContain('kg-bookmark-card'); + + const markdown = markdownService.convertToMarkdown(readable.content ?? '', { + baseUrl: sourceUrl, + title: readable.title + }); + + expect(markdown).toContain('[**Example bookmark**](https://example.com/bookmark-target)'); + expect(markdown).toContain('Bookmark description.'); + }); +}); diff --git a/apps/server/src/services/content-extraction.service.ts b/apps/server/src/services/content-extraction.service.ts index b2053ee..d8d1824 100644 --- a/apps/server/src/services/content-extraction.service.ts +++ b/apps/server/src/services/content-extraction.service.ts @@ -3,6 +3,14 @@ import { parseHTML } from 'linkedom'; import { logger } from '@/lib/logger.js'; +const GHOST_BOOKMARK_CLASSES = [ + 'kg-bookmark-card', + 'kg-bookmark-content', + 'kg-bookmark-title', + 'kg-bookmark-description', + 'kg-bookmark-thumbnail' +]; + class ContentExtraction { async extractReadableContent(htmlContent: string, url: string) { const { document } = parseHTML(htmlContent); @@ -21,6 +29,7 @@ class ContentExtraction { try { const article = new Readability(document, { + classesToPreserve: GHOST_BOOKMARK_CLASSES, // @ts-expect-error: missing type definition linkDensityModifier: 0.1 }).parse(); diff --git a/apps/server/src/services/markdown.service.test.ts b/apps/server/src/services/markdown.service.test.ts new file mode 100644 index 0000000..245a581 --- /dev/null +++ b/apps/server/src/services/markdown.service.test.ts @@ -0,0 +1,223 @@ +import { describe, expect, it } from 'vitest'; + +import { markdownService, type MarkdownConversionContext } from './markdown.service.js'; + +const defaultContext: MarkdownConversionContext = { + baseUrl: 'https://publisher.example/articles/story', + title: 'Extracted article title' +}; + +function convertToMarkdown(html: string, context = defaultContext) { + return markdownService.convertToMarkdown(html, context); +} + +describe('markdownService.convertToMarkdown', () => { + describe('content preservation', () => { + it('preserves navigational, fragment, commentary, and following links', () => { + const html = ` +

+ Jump to section + Footnote + Commentary + Following +

+ `; + + expect(convertToMarkdown(html)).toBe( + '[Jump to section](https://publisher.example/articles/story#section) [Footnote](https://publisher.example/articles/story#fn-1) [Commentary](https://publisher.example/commentary) [Following](https://publisher.example/following)' + ); + }); + + it('preserves prose in unrelated author and most-read class names', () => { + const html = ` +

Authoritative analysis stays.

+

Most readers need this context.

+ `; + + expect(convertToMarkdown(html)).toBe( + 'Authoritative analysis stays.\n\nMost readers need this context.' + ); + }); + + it('preserves standalone non-Latin and emoji content', () => { + const html = ` +

你好世界

+

日本語

+

Привет мир

+

مرحبا بالعالم

+

🙂

+ `; + + expect(convertToMarkdown(html)).toBe( + '你好世界\n\n日本語\n\nПривет мир\n\nمرحبا بالعالم\n\n🙂' + ); + }); + + it('preserves Ghost bookmark title, description, and destination', () => { + const html = ` +
+ + +

GitHub - example/bookmark-target

+

Contribute to example/bookmark-target development by creating an account on GitHub.

+ +
+ `; + + expect(convertToMarkdown(html)).toBe( + '[**GitHub - example/bookmark-target**](https://github.com/example/bookmark-target?ref=publisher.example)\n\nContribute to example/bookmark-target development by creating an account on GitHub.' + ); + }); + + it('supports Ghost bookmark title and description elements', () => { + const html = ` +
+ +
+
Example bookmark
+
Bookmark description.
+
+
+ +
+ `; + + expect(convertToMarkdown(html)).toBe( + '[**Example bookmark**](https://publisher.example/bookmark-target)\n\n*Bookmark description.*' + ); + }); + + it('does not treat ordinary linked figures as Ghost bookmarks', () => { + const html = ` +
+ + Figure +

Figure title

+ +
+ `; + + expect(convertToMarkdown(html)).toBe('![Figure](/figure.png)'); + }); + + it('does not emit an active link for an unsafe bookmark destination', () => { + const html = ` +
+ + Bookmark icon +

Unsafe bookmark

+

Bookmark description.

+ +
+ `; + + expect(convertToMarkdown(html)).toBe('![Bookmark icon](/icon.png)'); + }); + + it('escapes bookmark link delimiters in metadata', () => { + const html = ` +
+ + +

Example [*bookmark*_] `code`~

+

First line
Second line.

+ +
+ `; + + expect(convertToMarkdown(html)).toBe( + '[**Example \\[\\*bookmark\\*\\_\\] \\`code\\`\\~**](https://example.com/bookmark-\\(target\\))\n\nFirst line \nSecond line.' + ); + }); + + it('continues removing executable and styling elements', () => { + const html = ` + + +

Readable article content.

+ `; + + expect(convertToMarkdown(html)).toBe('Readable article content.'); + }); + }); + + describe('document context', () => { + const context = { + baseUrl: 'https://publisher.example/articles/story', + title: 'Expected article title' + }; + + it('resolves publisher-relative links while preserving document fragments', () => { + const html = ` +

+ Docs + Guide + Section + CDN +

+ `; + + expect(convertToMarkdown(html, context)).toBe( + '[Docs](https://publisher.example/docs) [Guide](https://publisher.example/guide) [Section](https://publisher.example/articles/story#section) [CDN](https://cdn.example.com/file)' + ); + }); + + it('strips active-content links while preserving safe contact links', () => { + const html = ` +

Unsafe script

+

Unsafe data

+

Email the author

+ `; + + expect(convertToMarkdown(html, context)).toBe( + 'Unsafe script\n\nUnsafe data\n\n[Email the author](mailto:author@publisher.example)' + ); + }); + + it('removes a leading H1 only when it duplicates the extracted title', () => { + const html = '

Expected article title

Opening paragraph.

'; + + expect(convertToMarkdown(html, context)).toBe('Opening paragraph.'); + }); + + it('removes a duplicate leading H1 when it is the entire document', () => { + expect(convertToMarkdown('

Expected article title

', context)).toBe(''); + }); + + it('preserves a leading H1 that differs from the extracted title', () => { + const html = '

A meaningful section heading

Opening paragraph.

'; + + expect(convertToMarkdown(html, context)).toBe( + '# A meaningful section heading\n\nOpening paragraph.' + ); + }); + + it('preserves linked heading structure and destination', () => { + const html = ` + +

Read the investigation

+

Background and supporting details.

+ + `; + + expect(convertToMarkdown(html, context)).toBe( + '## [Read the investigation](https://publisher.example/story)\n\nBackground and supporting details.' + ); + }); + + it('keeps multiple blocks after a linked heading as valid Markdown', () => { + const html = ` + +

Read the investigation

+

First supporting paragraph.

+

Second supporting paragraph.

+
  • Supporting evidence
+ + `; + + expect(convertToMarkdown(html, context)).toBe( + '## [Read the investigation](https://publisher.example/story)\n\nFirst supporting paragraph.\n\nSecond supporting paragraph.\n\n- Supporting evidence' + ); + }); + }); +}); diff --git a/apps/server/src/services/markdown.service.ts b/apps/server/src/services/markdown.service.ts index 349ee7e..f1b1104 100644 --- a/apps/server/src/services/markdown.service.ts +++ b/apps/server/src/services/markdown.service.ts @@ -1,5 +1,11 @@ +import { parseHTML } from 'linkedom'; import TurndownService from 'turndown'; +export interface MarkdownConversionContext { + baseUrl: string; + title: string; +} + interface GenericElement { classList?: { contains: (className: string) => boolean; @@ -31,6 +37,62 @@ function isGenericElement(node: unknown): node is GenericElement { return node !== null && typeof node === 'object' && 'getAttribute' in node; } +export function resolveArticleHref(href: string, baseUrl: string) { + try { + const resolved = new URL(href, baseUrl); + if (resolved.protocol === 'http:' || resolved.protocol === 'https:') { + return resolved.href; + } + + return resolved.protocol === 'mailto:' || resolved.protocol === 'tel:' ? href : null; + } catch { + return null; + } +} + +export interface BookmarkCardElement { + classList?: { + contains: (className: string) => boolean; + }; + getAttribute: (name: string) => string | null; + nodeName: string; + querySelector: (selector: string) => Element | null; +} + +export function isGhostBookmarkCard(node: BookmarkCardElement): boolean { + if (node.nodeName !== 'FIGURE' || !node.classList?.contains('kg-bookmark-card')) { + return false; + } + + const anchor = node.querySelector('a[href]'); + return Boolean(anchor?.textContent?.trim() && anchor.querySelector('img')); +} + +function normalizeTitle(value: string) { + return value.normalize('NFKC').replaceAll(/\s+/g, ' ').trim().toLocaleLowerCase(); +} + +function prepareHtmlForConversion(htmlContent: string, context: MarkdownConversionContext) { + const { document } = parseHTML(htmlContent); + const leadingH1Text = document.querySelector('h1')?.textContent?.trim() ?? null; + + for (const anchor of document.querySelectorAll('a[href]')) { + const href = anchor.getAttribute('href'); + if (!href) { + continue; + } + + const resolvedHref = resolveArticleHref(href, context.baseUrl); + if (resolvedHref) { + anchor.setAttribute('href', resolvedHref); + } else { + anchor.removeAttribute('href'); + } + } + + return { html: document.toString(), leadingH1Text }; +} + // generic element detection, highlight, strikethrough, list // listItem, table, and complexLinkStructure rules // below are adapted from defuddle @@ -103,15 +165,56 @@ turndownService.addRule('figure', { caption = tagText ? `${tagText} ${captionMarkdown}`.trim() : captionMarkdown; } - // Handle references in the caption - caption = caption.replaceAll(/\[([^\]]+)]\(([^)]+)\)/g, (_match, text, href) => { - return `[${text}](${href})`; - }); - return `![${alt}](${src})\n\n${caption}\n\n`; } }); +turndownService.addRule('bookmarkCard', { + filter(node) { + return isGenericElement(node) && isGhostBookmarkCard(node); + }, + replacement(content, node) { + if (!isGenericElement(node)) { + return content; + } + + const anchor = node.querySelector('a[href]'); + if (!anchor || !isGenericElement(anchor)) { + return content; + } + + const href = anchor.getAttribute('href'); + const paragraphs = Array.from(anchor.querySelectorAll('p')); + const titleElement = anchor.querySelector('.kg-bookmark-title') ?? paragraphs[0]; + const descriptionElement = anchor.querySelector('.kg-bookmark-description') ?? paragraphs[1]; + const title = titleElement?.textContent?.trim() ?? ''; + const description = + descriptionElement && isGenericElement(descriptionElement) + ? turndownService.turndown(descriptionElement.innerHTML || '').trim() + : ''; + + if (!href || !title) { + return content; + } + + const safeTitle = title + .replaceAll('\\', '\\\\') + .replaceAll('[', '\\[') + .replaceAll(']', '\\]') + .replaceAll('*', '\\*') + .replaceAll('_', '\\_') + .replaceAll('`', '\\`') + .replaceAll('~', '\\~'); + const safeHref = href.replaceAll('\\', '\\\\').replaceAll('(', '\\(').replaceAll(')', '\\)'); + const bookmarkLines = [`[**${safeTitle}**](${safeHref})`]; + if (description) { + bookmarkLines.push('', description); + } + + return `\n\n${bookmarkLines.join('\n')}\n\n`; + } +}); + turndownService.addRule('highlight', { filter: 'mark', replacement(content) { @@ -250,17 +353,25 @@ turndownService.addRule('complexLinkStructure', { if (!isGenericElement(node)) { return content; } - const href = node.getAttribute('href') || ''; + const headingEl = node.querySelector('h1, h2, h3, h4, h5, h6'); if (!headingEl || !isGenericElement(headingEl)) { return content; } - const headingMd = turndownService.turndown(headingEl.innerHTML || ''); - // Remove heading text from content to get the remaining link text - const remaining = content.replace(headingMd, '').trim(); - const linkPart = href ? `[${remaining || 'View'}](${href})` : remaining; - return `\n\n${headingMd}\n\n${linkPart}\n\n`; + const headingLevel = Number(headingEl.nodeName.slice(1)); + const headingContent = turndownService.turndown(headingEl.innerHTML || '').trim(); + const href = node.getAttribute('href') || ''; + const clonedLink = node.cloneNode(true); + const remaining = isGenericElement(clonedLink) + ? (() => { + const clonedHeading = clonedLink.querySelector('h1, h2, h3, h4, h5, h6'); + clonedHeading?.remove(); + return turndownService.turndown(clonedLink.innerHTML || '').trim(); + })() + : ''; + const heading = `${'#'.repeat(headingLevel)} ${href ? `[${headingContent}](${href})` : headingContent}`; + return `\n\n${heading}${remaining ? `\n\n${remaining}` : ''}\n\n`; } }); @@ -346,67 +457,20 @@ turndownService.addRule('removeMediumSubscription', { replacement: () => '' }); -turndownService.addRule('removeButtons', { +turndownService.addRule('removeControls', { filter(node: HTMLElement): boolean { - // Remove actual button tags if (node.nodeName === 'BUTTON') { return true; } - // Remove anchor tags that behave like buttons - if (node.nodeName === 'A') { - const className = node.getAttribute('class') || ''; - const role = node.getAttribute('role') || ''; - const ariaLabel = node.getAttribute('aria-label') || ''; - const href = node.getAttribute('href') || ''; - - // Check for common button-like indicators - const isButtonLike = - className.includes('btn') || - className.includes('button') || - role === 'button' || - ariaLabel.toLowerCase().includes('button') || - href === '#' || - href === 'javascript:void(0)' || - href === 'javascript:;'; - - // Also check if it has minimal content (typical for buttons) - const hasMinimalContent = - node.textContent?.trim().length === 0 || - (node.textContent.trim().length < 20 && - (className.includes('icon') || className.includes('button'))); - - // Check for comment links (specific pattern) - const isCommentLink = - className.includes('comments') || - href.includes('#comments') || - node.textContent?.toLowerCase().includes('comment'); - - // Check for social interaction links - const isSocialInteraction = - className.includes('share') || - className.includes('like') || - className.includes('follow') || - className.includes('subscribe') || - (node.textContent && /(?:share|like|follow|subscribe|comment)s?/i.test(node.textContent)); - - // Also remove if it points to the same page with a hash - const isSamePageLink = href.startsWith('#') && href.length > 1; - - return ( - isButtonLike || - isCommentLink || - isSocialInteraction || - isSamePageLink || - (hasMinimalContent && (!href || href === '#' || href.startsWith('javascript'))) - ); + if (node.nodeName !== 'A') { + return false; } - return false; + const href = node.getAttribute('href')?.trim().toLowerCase() ?? ''; + return href === 'javascript:void(0)' || href === 'javascript:;'; }, - replacement() { - return ''; - } + replacement: () => '' }); turndownService.remove(['style', 'script']); @@ -520,37 +584,17 @@ turndownService.addRule('embedToMarkdown', { } }); -turndownService.addRule('removeAuthor', { - filter: (node) => { - if (node.nodeName === 'DIV') { - const className = node.getAttribute('class') || ''; - return className.includes('author'); - } - - return false; - }, - replacement: () => '' -}); - -turndownService.addRule('removeMostRead', { - filter: (node) => { - if (node.nodeName === 'DIV') { - const className = node.getAttribute('class') || ''; - return className.includes('most-read'); - } - - return false; - }, - replacement: () => '' -}); - class MarkdownService { - convertToMarkdown(htmlContent: string) { - let markdown = turndownService.turndown(htmlContent); - - // remove the title from the beginning of the content if it exists - const titleMatch = markdown.match(/^# .+\n+/); - if (titleMatch) { + convertToMarkdown(htmlContent: string, context: MarkdownConversionContext) { + const prepared = prepareHtmlForConversion(htmlContent, context); + let markdown = turndownService.turndown(prepared.html); + + const titleMatch = markdown.match(/^# .+(?:\n+|$)/); + if ( + titleMatch && + prepared.leadingH1Text && + normalizeTitle(prepared.leadingH1Text) === normalizeTitle(context.title) + ) { markdown = markdown.slice(titleMatch[0].length); } @@ -561,10 +605,6 @@ class MarkdownService { // remove any consecutive newlines more than two markdown = markdown.replaceAll(/\n{3,}/g, '\n\n'); - // clean up any remaining lines that are just punctuation or empty - // exclude lines containing code block markers (backticks) - markdown = markdown.replaceAll(/^\s*(?![^\s\w]*`[^\s\w]*)(?:[^\s\w]+\s*)?$/gm, ''); - return markdown.trim(); } } diff --git a/apps/server/src/workers/content-extraction.worker.behavior.test.ts b/apps/server/src/workers/content-extraction.worker.behavior.test.ts index 16f5435..10e95ed 100644 --- a/apps/server/src/workers/content-extraction.worker.behavior.test.ts +++ b/apps/server/src/workers/content-extraction.worker.behavior.test.ts @@ -88,9 +88,18 @@ vi.mock('@/services/browser.service.js', () => ({ browserService: browserService vi.mock('@/services/content-extraction.service.js', () => ({ contentExtractionService: contentExtractionServiceMock })); -vi.mock('@/services/markdown.service.js', () => ({ markdownService: markdownServiceMock })); +vi.mock('@/services/markdown.service.js', async () => ({ + ...(await vi.importActual( + '@/services/markdown.service.js' + )), + markdownService: markdownServiceMock +})); vi.mock('@/services/storage.service.js', () => ({ storageService: storageServiceMock })); +const { contentExtractionService: actualContentExtractionService } = await vi.importActual< + typeof import('@/services/content-extraction.service.js') +>('@/services/content-extraction.service.js'); + await importWithEnv({ DEMO_MODE: 'false' }, async () => import('./content-extraction.worker.js')); beforeEach(() => { @@ -206,9 +215,73 @@ describe('content extraction worker behavior', () => { 'https://example.com/body.png', { userId: 'user-1' } ); + expect(markdownServiceMock.convertToMarkdown).toHaveBeenCalledWith(expect.any(String), { + baseUrl: 'https://example.com/article', + title: 'Readable Title' + }); expect(setTimeoutSpy.mock.calls.some(([, delay]) => delay === 500)).toBe(false); }); + it('does not upload images used only by Ghost bookmark cards', async () => { + expect(contentExtractionJobHandler).toBeDefined(); + + const readable = await actualContentExtractionService.extractReadableContent( + ` + + + Readable Title + + + + + `, + 'https://example.com/article' + ); + contentExtractionServiceMock.extractReadableContent.mockResolvedValueOnce(readable); + storageServiceMock.uploadImageFromUrl.mockReset(); + storageServiceMock.uploadImageFromUrl.mockResolvedValue({ + key: 'user-user-1/articles/body.png', + url: 'https://cdn/body.png' + }); + + const result = await contentExtractionJobHandler!(job()); + + expect(result).toMatchObject({ status: 'success', imagesProcessed: 1, imagesFailed: 0 }); + expect(storageServiceMock.uploadImageFromUrl).toHaveBeenCalledTimes(2); + expect(storageServiceMock.uploadImageFromUrl).toHaveBeenNthCalledWith( + 1, + 'https://example.com/cover.png', + { userId: 'user-1' } + ); + expect(storageServiceMock.uploadImageFromUrl).toHaveBeenNthCalledWith( + 2, + 'https://example.com/body.png', + { userId: 'user-1' } + ); + expect(storageServiceMock.uploadImageFromUrl).not.toHaveBeenCalledWith( + 'https://example.com/bookmark-icon.png', + expect.anything() + ); + expect(storageServiceMock.uploadImageFromUrl).not.toHaveBeenCalledWith( + 'https://example.com/bookmark-thumbnail.png', + expect.anything() + ); + }); + it('resumes stale processing jobs instead of skipping them', async () => { linksAdapterMock.findById.mockResolvedValueOnce( makeLink({ diff --git a/apps/server/src/workers/content-extraction.worker.ts b/apps/server/src/workers/content-extraction.worker.ts index 5f17af3..da489af 100644 --- a/apps/server/src/workers/content-extraction.worker.ts +++ b/apps/server/src/workers/content-extraction.worker.ts @@ -17,7 +17,11 @@ import { createDrizzleLinksAdapter } from '@/repositories/links.repository.js'; import { browserService } from '@/services/browser.service.js'; import { contentExtractionService } from '@/services/content-extraction.service.js'; -import { markdownService } from '@/services/markdown.service.js'; +import { + isGhostBookmarkCard, + markdownService, + resolveArticleHref +} from '@/services/markdown.service.js'; import { storageService } from '@/services/storage.service.js'; const links = createDrizzleLinksAdapter(db); @@ -28,6 +32,20 @@ const jobTimeoutMs = 5 * 60 * 1000; const isDataURI = (uri: string): boolean => uri.startsWith('data:'); +function isBookmarkCardImage(image: Element, baseUrl: string): boolean { + const figure = image.closest('figure'); + const anchor = figure?.querySelector('a[href]'); + const href = anchor?.getAttribute('href'); + + return Boolean( + figure && + isGhostBookmarkCard(figure) && + href && + resolveArticleHref(href, baseUrl) && + image.closest('a[href]') === anchor + ); +} + function abortPromise(signal: AbortSignal): Promise { if (signal.aborted) { const p = Promise.reject(signal.reason ?? new Error('AbortError')); @@ -180,7 +198,9 @@ async function contentExtractionJob(job: Job): Promise const { document } = parseHTML(htmlContent); - const images = Array.from(document.querySelectorAll('img')); + const allImages = Array.from(document.querySelectorAll('img')); + const images = allImages.filter((image) => !isBookmarkCardImage(image, articleUrl)); + const skippedBookmarkImages = allImages.length - images.length; let coverImage = null; @@ -203,7 +223,11 @@ async function contentExtractionJob(job: Job): Promise } } - logger.info(`Found ${images.length} images in the article`); + logger.info( + `Found ${images.length} images in the article${ + skippedBookmarkImages > 0 ? `; skipped ${skippedBookmarkImages} bookmark images` : '' + }` + ); let currentImageProgress = 0; @@ -255,7 +279,10 @@ async function contentExtractionJob(job: Job): Promise htmlContent = document.toString(); - const content = markdownService.convertToMarkdown(htmlContent); + const content = markdownService.convertToMarkdown(htmlContent, { + baseUrl: articleUrl, + title: readableContent.title + }); const readingTime = estimateReadingTime(readableContent.textContent as string); diff --git a/apps/web/src/components/common/user-menu.test.tsx b/apps/web/src/components/common/user-menu.test.tsx new file mode 100644 index 0000000..81be0fb --- /dev/null +++ b/apps/web/src/components/common/user-menu.test.tsx @@ -0,0 +1,90 @@ +import userEvent from '@testing-library/user-event'; +import type { ReactNode } from 'react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { render, screen } from '@/tests/test-utils'; + +const navigate = vi.hoisted(() => vi.fn()); +const setTheme = vi.hoisted(() => vi.fn()); +const toggleTheme = vi.hoisted(() => vi.fn()); + +vi.mock('@tanstack/react-router', async (importOriginal) => { + const actual = await importOriginal(); + + return { + ...actual, + useNavigate: () => navigate + }; +}); + +vi.mock('next-themes', () => ({ + useTheme: () => ({ setTheme, theme: 'light' }) +})); + +vi.mock('react-i18next', () => ({ + initReactI18next: { init: vi.fn(), type: '3rdParty' }, + useTranslation: () => ({ t: (key: string) => key }) +})); + +vi.mock('@/features/auth/api/get-user', () => ({ + useGetUser: () => ({ + data: { + result: { + avatar: null, + displayName: 'Reader', + email: 'reader@example.com', + role: 'user' + } + } + }) +})); + +vi.mock('@/features/auth/api/logout', () => ({ + useLogout: () => ({ mutate: vi.fn() }) +})); + +vi.mock('@/hooks/use-theme-config', () => ({ + useThemeConfig: (selector: (state: { toggleTheme: typeof toggleTheme }) => unknown) => + selector({ toggleTheme }) +})); + +vi.mock('../ui/dropdown-menu', () => { + const Wrapper = ({ children }: { children?: ReactNode }) =>
{children}
; + + return { + DropdownMenu: Wrapper, + DropdownMenuContent: Wrapper, + DropdownMenuGroup: Wrapper, + DropdownMenuItem: ({ children, onClick }: { children?: ReactNode; onClick?: () => void }) => ( + + ), + DropdownMenuLabel: Wrapper, + DropdownMenuPortal: Wrapper, + DropdownMenuSeparator: () =>
, + DropdownMenuSub: Wrapper, + DropdownMenuSubContent: Wrapper, + DropdownMenuSubTrigger: Wrapper, + DropdownMenuTrigger: ({ render }: { render: ReactNode }) => render + }; +}); + +import { UserMenu } from './user-menu'; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('UserMenu', () => { + it('applies and persists theme changes from the menu', async () => { + const user = userEvent.setup(); + + render(Open user menu} />); + + await user.click(screen.getByRole('button', { name: 'userMenu.themes.dark' })); + + expect(setTheme).toHaveBeenCalledWith('dark'); + expect(toggleTheme).toHaveBeenCalledWith('dark'); + }); +}); diff --git a/apps/web/src/components/common/user-menu.tsx b/apps/web/src/components/common/user-menu.tsx index 47adcee..177fef5 100644 --- a/apps/web/src/components/common/user-menu.tsx +++ b/apps/web/src/components/common/user-menu.tsx @@ -17,6 +17,8 @@ import { useTranslation } from 'react-i18next'; import { useGetUser } from '@/features/auth/api/get-user'; import { useLogout } from '@/features/auth/api/logout'; +import { useThemeConfig } from '@/hooks/use-theme-config'; + import { cn } from '@/lib/utils'; import { Avatar, AvatarFallback, AvatarImage } from '../ui/avatar'; @@ -60,6 +62,7 @@ export function UserMenu({ align = 'end', contentClassName, trigger }: UserMenuP const { t } = useTranslation(); const { data: user } = useGetUser(); const { setTheme, theme } = useTheme(); + const toggleTheme = useThemeConfig((state) => state.toggleTheme); const navigate = useNavigate(); const queryClient = useQueryClient(); @@ -110,7 +113,10 @@ export function UserMenu({ align = 'end', contentClassName, trigger }: UserMenuP setTheme(value)} + onClick={() => { + setTheme(value); + toggleTheme(value); + }} > {t(labelKey)} diff --git a/apps/web/src/features/reader/components/reader-content.tsx b/apps/web/src/features/reader/components/reader-content.tsx index ad8f913..a1c4248 100644 --- a/apps/web/src/features/reader/components/reader-content.tsx +++ b/apps/web/src/features/reader/components/reader-content.tsx @@ -73,7 +73,7 @@ export function ReaderContent({ ) }} remarkPlugins={[remarkGfm]} - urlTransform={(url) => sanitizeUrl(url ?? '')} + urlTransform={(url, key) => sanitizeUrl(url ?? '', { allowDataImage: key === 'src' })} > {textContent} diff --git a/apps/web/src/lib/utils.test.ts b/apps/web/src/lib/utils.test.ts index 01197da..00b2de7 100644 --- a/apps/web/src/lib/utils.test.ts +++ b/apps/web/src/lib/utils.test.ts @@ -13,9 +13,17 @@ describe('getUrlName', () => { }); describe('sanitizeUrl', () => { - it('should return data URIs unchanged', () => { + it('should reject data URIs by default', () => { + expect(sanitizeUrl('data:text/html,')).toBe(''); + }); + + it('should allow raster data images only when requested for image sources', () => { const dataUri = 'data:image/png;base64,abc123'; - expect(sanitizeUrl(dataUri)).toBe(dataUri); + + expect(sanitizeUrl(dataUri, { allowDataImage: true })).toBe(dataUri); + expect(sanitizeUrl('data:text/html,', { allowDataImage: true })).toBe( + '' + ); }); it('should normalize http URLs', () => { diff --git a/apps/web/src/lib/utils.ts b/apps/web/src/lib/utils.ts index 4433ce6..ba2a6ae 100644 --- a/apps/web/src/lib/utils.ts +++ b/apps/web/src/lib/utils.ts @@ -10,10 +10,12 @@ export function getUrlName(url: string) { return urlObject.hostname.replace('www.', ''); } -export function sanitizeUrl(url: string): string { +const safeDataImagePattern = /^data:image\/(?:avif|gif|jpeg|png|webp);base64,/i; + +export function sanitizeUrl(url: string, options: { allowDataImage?: boolean } = {}): string { try { if (url.startsWith('data:')) { - return url; + return options.allowDataImage && safeDataImagePattern.test(url) ? url : ''; } const parsedUrl = new URL(url, window.location.origin); diff --git a/apps/web/src/pages/feeds.test.tsx b/apps/web/src/pages/feeds.test.tsx index bc31339..b83b179 100644 --- a/apps/web/src/pages/feeds.test.tsx +++ b/apps/web/src/pages/feeds.test.tsx @@ -1,20 +1,27 @@ -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { HttpResponse, http } from 'msw'; import type { ReactNode } from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { server } from '@/tests/mocks/server'; +import { render } from '@/tests/test-utils'; + const createFeedMutate = vi.hoisted(() => vi.fn()); const deleteFeedMutate = vi.hoisted(() => vi.fn()); const saveFeedMutate = vi.hoisted(() => vi.fn()); const dismissFeedMutate = vi.hoisted(() => vi.fn()); const updateFeedMutate = vi.hoisted(() => vi.fn()); +const navigate = vi.hoisted(() => vi.fn()); vi.mock('@tanstack/react-router', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - Link: ({ children }: { children: ReactNode }) => {children} + Link: ({ children }: { children: ReactNode }) => {children}, + useNavigate: () => navigate, + useSearch: () => ({}) }; }); @@ -99,8 +106,9 @@ import { import type { FeedItem, FeedSubscription } from '@/types/feeds'; -import { AddFeedForm, FeedItemCard } from './feeds'; +import FeedsPage, { AddFeedForm, FeedItemCard } from './feeds'; +const API_URL = 'http://localhost:3000'; const NOW = '2026-06-28T12:00:00.000Z'; function subscription(overrides: Partial = {}): FeedSubscription { @@ -205,6 +213,26 @@ describe('feeds page helpers', () => { }); describe('feed page components', () => { + it('shows the first-use empty state without requesting feed items', async () => { + let itemRequests = 0; + + server.use( + http.get(`${API_URL}/feeds/subscriptions`, () => + HttpResponse.json({ message: 'ok', result: [], status: 200 }) + ), + http.get(`${API_URL}/feeds/items`, () => { + itemRequests++; + return HttpResponse.json({ message: 'failed', status: 400 }, { status: 400 }); + }) + ); + + render(); + + expect(await screen.findByText('feeds.empty.title')).toBeInTheDocument(); + expect(screen.queryByText('feeds.error.title')).not.toBeInTheDocument(); + expect(itemRequests).toBe(0); + }); + it('validates add feed input before submitting', async () => { const user = userEvent.setup(); render(); diff --git a/apps/web/src/pages/feeds.tsx b/apps/web/src/pages/feeds.tsx index 2b1dc1c..de5a48e 100644 --- a/apps/web/src/pages/feeds.tsx +++ b/apps/web/src/pages/feeds.tsx @@ -73,25 +73,31 @@ function FeedsPage() { selectedSubscriptionId === 'all' ? undefined : selectedSubscriptionId; const subscriptionsQuery = useFeedSubscriptions(); + const subscriptions = subscriptionsQuery.data?.result ?? []; + const hasSubscriptions = subscriptions.length > 0; const itemsQuery = useFeedItems({ filters: { sort, state: activeState, ...(selectedSubscriptionFilter ? { subscriptionId: selectedSubscriptionFilter } : {}) - } + }, + queryConfig: { enabled: hasSubscriptions } + }); + const pendingItemsQuery = useFeedItems({ + filters: { state: 'new' }, + queryConfig: { enabled: hasSubscriptions } }); - const pendingItemsQuery = useFeedItems({ filters: { state: 'new' } }); - const subscriptions = subscriptionsQuery.data?.result ?? []; const items = itemsQuery.data ?? []; const activeFeeds = subscriptions.filter((subscription) => subscription.status === 'active'); const sourceTitleBySubscriptionId = useMemo( () => new Map(subscriptions.map((subscription) => [subscription.id, subscription.title])), [subscriptions] ); - const isLoading = subscriptionsQuery.isLoading || itemsQuery.isLoading; - const isError = subscriptionsQuery.isError || itemsQuery.isError; - const isOverviewLoading = subscriptionsQuery.isLoading || pendingItemsQuery.isLoading; + const isLoading = subscriptionsQuery.isLoading || (hasSubscriptions && itemsQuery.isLoading); + const isError = subscriptionsQuery.isError || (hasSubscriptions && itemsQuery.isError); + const isOverviewLoading = + subscriptionsQuery.isLoading || (hasSubscriptions && pendingItemsQuery.isLoading); const isManageOpen = search.manage === true || search.tab === 'feeds'; const managerStatus = search.manageStatus ?? 'all'; @@ -117,6 +123,11 @@ function FeedsPage() { }; const retryFeeds = () => { + if (!hasSubscriptions) { + void subscriptionsQuery.refetch(); + return; + } + void Promise.all([ subscriptionsQuery.refetch(), itemsQuery.refetch(), @@ -259,13 +270,9 @@ function FeedsPage() { <>
-
-

- {t('feeds.title')} -

-

- {t('feeds.description')} -

+
+

{t('feeds.title')}

+

{t('feeds.description')}

diff --git a/apps/web/src/typography.css b/apps/web/src/typography.css index 9957a6b..ee01913 100644 --- a/apps/web/src/typography.css +++ b/apps/web/src/typography.css @@ -188,6 +188,7 @@ background-color: var(--color-zinc-950); border-radius: 0.25rem; font-size: 0.875rem; + overflow-wrap: anywhere; } :where(h2, h3, h4) code:where(:not(.not-prose, .not-prose *)) { @@ -225,6 +226,7 @@ font-family: var(--font-mono); font-size: var(--text-sm); line-height: 2; + overflow-wrap: normal; } table:where(:not(.not-prose, .not-prose *)) {