Review fixes

This commit is contained in:
Alex 2026-05-03 01:08:07 +02:00
parent b6103f6d54
commit 6e7d9e571c
8 changed files with 132 additions and 106 deletions

View file

@ -60,7 +60,6 @@ jobs:
test -s main.js
test -s manifest.json
test -s styles.css
test -s versions.json
- name: Create GitHub release
env:
@ -70,6 +69,5 @@ jobs:
main.js \
manifest.json \
styles.css \
versions.json \
--title "$GITHUB_REF_NAME" \
--notes "Release $GITHUB_REF_NAME"

View file

@ -3,10 +3,12 @@ import type { DataAdapter, ListedFiles, Stat } from 'obsidian'
import { PumlerDiskSvgCache } from './cache'
import type { RenderDiagramOptions } from './client'
const CACHE_DIR = 'custom-config/plugins/pumler/cache'
describe('PumlerDiskSvgCache', () => {
test('stores and reads SVGs by render options', async () => {
const adapter = new MemoryAdapter()
const cache = new PumlerDiskSvgCache(adapter, '.obsidian/plugins/pumler/cache', 30)
const cache = new PumlerDiskSvgCache(adapter, CACHE_DIR, 30)
const options = renderOptions('Alice -> Bob')
await cache.set(options, '<svg>Alice</svg>')
@ -17,7 +19,7 @@ describe('PumlerDiskSvgCache', () => {
test('keeps only the most recently used entries', async () => {
const adapter = new MemoryAdapter()
const cache = new PumlerDiskSvgCache(adapter, '.obsidian/plugins/pumler/cache', 2)
const cache = new PumlerDiskSvgCache(adapter, CACHE_DIR, 2)
const first = renderOptions('A -> B')
const second = renderOptions('B -> C')
const third = renderOptions('C -> D')
@ -35,10 +37,10 @@ describe('PumlerDiskSvgCache', () => {
test('ignores corrupt cache indexes', async () => {
const adapter = new MemoryAdapter()
await adapter.mkdir('.obsidian/plugins/pumler/cache')
await adapter.write('.obsidian/plugins/pumler/cache/index.json', 'not json')
await adapter.mkdir(CACHE_DIR)
await adapter.write(`${CACHE_DIR}/index.json`, 'not json')
const cache = new PumlerDiskSvgCache(adapter, '.obsidian/plugins/pumler/cache', 30)
const cache = new PumlerDiskSvgCache(adapter, CACHE_DIR, 30)
const options = renderOptions('A -> B')
await cache.set(options, '<svg>fresh</svg>')
@ -64,113 +66,124 @@ class MemoryAdapter implements DataAdapter {
return 'memory'
}
async exists(normalizedPath: string): Promise<boolean> {
return this.folders.has(normalizedPath) || this.files.has(normalizedPath)
exists(normalizedPath: string): Promise<boolean> {
return Promise.resolve(this.hasPath(normalizedPath))
}
async stat(normalizedPath: string): Promise<Stat | null> {
if (!await this.exists(normalizedPath)) {
return null
stat(normalizedPath: string): Promise<Stat | null> {
if (!this.hasPath(normalizedPath)) {
return Promise.resolve(null)
}
return {
return Promise.resolve({
type: this.folders.has(normalizedPath) ? 'folder' : 'file',
ctime: 0,
mtime: 0,
size: this.files.get(normalizedPath)?.length ?? 0
}
})
}
async list(normalizedPath: string): Promise<ListedFiles> {
list(normalizedPath: string): Promise<ListedFiles> {
const prefix = `${normalizedPath}/`
return {
return Promise.resolve({
files: Array.from(this.files.keys()).filter(path => path.startsWith(prefix)),
folders: Array.from(this.folders).filter(path => path.startsWith(prefix))
}
})
}
async read(normalizedPath: string): Promise<string> {
read(normalizedPath: string): Promise<string> {
const data = this.files.get(normalizedPath)
if (data === undefined) {
throw new Error(`Missing file: ${normalizedPath}`)
return Promise.reject(new Error(`Missing file: ${normalizedPath}`))
}
return data
return Promise.resolve(data)
}
async readBinary(): Promise<ArrayBuffer> {
throw new Error('Not implemented')
readBinary(): Promise<ArrayBuffer> {
return Promise.reject(new Error('Not implemented'))
}
async write(normalizedPath: string, data: string): Promise<void> {
write(normalizedPath: string, data: string): Promise<void> {
this.files.set(normalizedPath, data)
return Promise.resolve()
}
async writeBinary(): Promise<void> {
throw new Error('Not implemented')
writeBinary(): Promise<void> {
return Promise.reject(new Error('Not implemented'))
}
async append(normalizedPath: string, data: string): Promise<void> {
append(normalizedPath: string, data: string): Promise<void> {
this.files.set(normalizedPath, `${this.files.get(normalizedPath) ?? ''}${data}`)
return Promise.resolve()
}
async appendBinary(): Promise<void> {
throw new Error('Not implemented')
appendBinary(): Promise<void> {
return Promise.reject(new Error('Not implemented'))
}
async process(normalizedPath: string, fn: (data: string) => string): Promise<string> {
process(normalizedPath: string, fn: (data: string) => string): Promise<string> {
const data = fn(this.files.get(normalizedPath) ?? '')
this.files.set(normalizedPath, data)
return data
return Promise.resolve(data)
}
getResourcePath(normalizedPath: string): string {
return normalizedPath
}
async mkdir(normalizedPath: string): Promise<void> {
mkdir(normalizedPath: string): Promise<void> {
this.folders.add(normalizedPath)
return Promise.resolve()
}
async trashSystem(): Promise<boolean> {
return false
trashSystem(): Promise<boolean> {
return Promise.resolve(false)
}
async trashLocal(): Promise<void> {
throw new Error('Not implemented')
trashLocal(): Promise<void> {
return Promise.reject(new Error('Not implemented'))
}
async rmdir(normalizedPath: string): Promise<void> {
rmdir(normalizedPath: string): Promise<void> {
this.folders.delete(normalizedPath)
return Promise.resolve()
}
async remove(normalizedPath: string): Promise<void> {
remove(normalizedPath: string): Promise<void> {
this.files.delete(normalizedPath)
return Promise.resolve()
}
async rename(normalizedPath: string, normalizedNewPath: string): Promise<void> {
rename(normalizedPath: string, normalizedNewPath: string): Promise<void> {
const data = this.files.get(normalizedPath)
if (data !== undefined) {
this.files.delete(normalizedPath)
this.files.set(normalizedNewPath, data)
return
return Promise.resolve()
}
if (this.folders.delete(normalizedPath)) {
this.folders.add(normalizedNewPath)
}
return Promise.resolve()
}
async copy(normalizedPath: string, normalizedNewPath: string): Promise<void> {
copy(normalizedPath: string, normalizedNewPath: string): Promise<void> {
const data = this.files.get(normalizedPath)
if (data === undefined) {
throw new Error(`Missing file: ${normalizedPath}`)
return Promise.reject(new Error(`Missing file: ${normalizedPath}`))
}
this.files.set(normalizedNewPath, data)
return Promise.resolve()
}
svgFileCount(): number {
return Array.from(this.files.keys()).filter(path => path.endsWith('.svg')).length
}
private hasPath(normalizedPath: string): boolean {
return this.folders.has(normalizedPath) || this.files.has(normalizedPath)
}
}

View file

@ -37,7 +37,7 @@ export class PumlerDiskSvgCache implements PumlerSvgCache {
async get(options: RenderDiagramOptions): Promise<string | null> {
try {
return await this.enqueueOperation(() => this.readEntry(options))
} catch (error) {
} catch {
return null
}
}

View file

@ -1,20 +1,22 @@
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'
import { PumlerApiClient, PumlerRenderError } from './client'
import type { RequestUrlResponse } from 'obsidian'
import { PumlerApiClient, type PumlerRequestUrl } from './client'
describe('PumlerApiClient', () => {
let requestUrlMock: ReturnType<typeof vi.fn<PumlerRequestUrl>>
beforeEach(() => {
vi.stubGlobal('fetch', vi.fn())
requestUrlMock = vi.fn<PumlerRequestUrl>()
})
afterEach(() => {
vi.unstubAllGlobals()
vi.clearAllMocks()
})
test('renders a diagram through the Pumler API', async () => {
const fetchMock = vi.mocked(fetch)
fetchMock.mockResolvedValue(createResponse(200, { diagram: '<svg></svg>' }))
requestUrlMock.mockResolvedValue(createResponse(200, { diagram: '<svg></svg>' }))
const client = new PumlerApiClient()
const client = new PumlerApiClient(requestUrlMock)
const result = await client.renderDiagram({
provider: 'plantuml',
type: 'sequence',
@ -23,8 +25,11 @@ describe('PumlerApiClient', () => {
})
expect(result).toBe('<svg></svg>')
expect(fetchMock).toHaveBeenCalledWith('https://api.pumler.com/api/diagram/render', expect.objectContaining({
expect(requestUrlMock).toHaveBeenCalledWith(expect.objectContaining({
url: 'https://api.pumler.com/api/diagram/render',
method: 'POST',
contentType: 'application/json',
throw: false,
body: JSON.stringify({
source: 'Alice -> Bob',
metadata: {
@ -39,7 +44,7 @@ describe('PumlerApiClient', () => {
})
test('maps structured API errors', async () => {
vi.mocked(fetch).mockResolvedValue(createResponse(400, {
requestUrlMock.mockResolvedValue(createResponse(400, {
error: {
message: 'Syntax error',
line: 2,
@ -47,7 +52,7 @@ describe('PumlerApiClient', () => {
}
}))
const client = new PumlerApiClient()
const client = new PumlerApiClient(requestUrlMock)
await expect(client.renderDiagram({
provider: 'plantuml',
type: 'sequence',
@ -62,9 +67,9 @@ describe('PumlerApiClient', () => {
})
test('maps network failures', async () => {
vi.mocked(fetch).mockRejectedValue(new Error('offline'))
requestUrlMock.mockRejectedValue(new Error('offline'))
const client = new PumlerApiClient()
const client = new PumlerApiClient(requestUrlMock)
await expect(client.renderDiagram({
provider: 'mermaid',
type: 'flowchart',
@ -74,10 +79,9 @@ describe('PumlerApiClient', () => {
})
test('does not cache repeated render requests by itself', async () => {
const fetchMock = vi.mocked(fetch)
fetchMock.mockResolvedValue(createResponse(200, { diagram: '<svg></svg>' }))
requestUrlMock.mockResolvedValue(createResponse(200, { diagram: '<svg></svg>' }))
const client = new PumlerApiClient()
const client = new PumlerApiClient(requestUrlMock)
const options = {
provider: 'mermaid' as const,
type: 'flowchart',
@ -88,14 +92,16 @@ describe('PumlerApiClient', () => {
await client.renderDiagram(options)
await client.renderDiagram(options)
expect(fetchMock).toHaveBeenCalledTimes(2)
expect(requestUrlMock).toHaveBeenCalledTimes(2)
})
})
function createResponse(status: number, data: unknown): Response {
function createResponse(status: number, data: unknown): RequestUrlResponse {
return {
ok: status >= 200 && status < 300,
status,
json: async () => data
} as Response
headers: {},
arrayBuffer: new ArrayBuffer(0),
json: data,
text: JSON.stringify(data)
}
}

View file

@ -1,3 +1,4 @@
import type { RequestUrlParam, RequestUrlResponse } from 'obsidian'
import { PUMLER_API_URL, type Provider, type ResolvedTheme } from './constants'
export interface RenderDiagramOptions {
@ -19,6 +20,10 @@ interface PumlerErrorPayload {
}
}
export interface PumlerRequestUrl {
(request: RequestUrlParam): Promise<RequestUrlResponse>
}
export class PumlerRenderError extends Error {
readonly line?: number
readonly column?: number
@ -32,15 +37,18 @@ export class PumlerRenderError extends Error {
}
export class PumlerApiClient {
constructor(private readonly requestUrl: PumlerRequestUrl) {}
async renderDiagram(options: RenderDiagramOptions, requestOptions: RenderDiagramRequestOptions = {}): Promise<string> {
let response: Response
let response: RequestUrlResponse
try {
response = await fetch(PUMLER_API_URL, {
response = await this.requestUrl({
url: PUMLER_API_URL,
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json'
Accept: 'application/json'
},
contentType: 'application/json',
body: JSON.stringify({
source: options.source,
metadata: {
@ -51,7 +59,7 @@ export class PumlerApiClient {
}
}
}),
signal: requestOptions.signal
throw: false
})
} catch (error) {
if (requestOptions.signal?.aborted) {
@ -60,8 +68,8 @@ export class PumlerApiClient {
throw new PumlerRenderError('Network error: unable to reach the Pumler rendering API')
}
const data = await readJson(response)
if (!response.ok) {
const data = readJson(response)
if (response.status < 200 || response.status >= 300) {
throw mapApiError(data)
}
@ -84,12 +92,10 @@ export function createRenderDiagramCacheSeed(options: RenderDiagramOptions): str
])
}
async function readJson(response: Response): Promise<Record<string, unknown> | null> {
try {
return await response.json()
} catch {
return null
}
function readJson(response: RequestUrlResponse): Record<string, unknown> | null {
return response.json && typeof response.json === 'object'
? response.json as Record<string, unknown>
: null
}
function mapApiError(payload: PumlerErrorPayload | Record<string, unknown> | null): PumlerRenderError {

View file

@ -1,15 +1,16 @@
import { MarkdownRenderChild, Plugin, type App, type MarkdownPostProcessorContext } from 'obsidian'
import { MarkdownRenderChild, Plugin, requestUrl, type App, type MarkdownPostProcessorContext } from 'obsidian'
import { PumlerDiskSvgCache } from './cache'
import { PumlerApiClient } from './client'
import { parsePumlerBlock } from './parser'
import { PumlerBlockRenderer } from './renderer'
export default class PumlerPlugin extends Plugin {
async onload(): Promise<void> {
const cache = this.manifest.dir
? new PumlerDiskSvgCache(this.app.vault.adapter, `${this.manifest.dir}/cache`)
: undefined
const renderer = new PumlerBlockRenderer(new PumlerApiClient(), cache)
onload(): void {
const cache = new PumlerDiskSvgCache(
this.app.vault.adapter,
`${this.app.vault.configDir}/plugins/${this.manifest.id}/cache`
)
const renderer = new PumlerBlockRenderer(new PumlerApiClient(requestUrl), cache)
this.registerMarkdownCodeBlockProcessor('pumler', (source, element, context) => {
const debounceKey = createDebounceKey(source, element, context)

View file

@ -22,7 +22,7 @@ describe('PumlerBlockRenderer', () => {
})
test('renders sanitized SVG on success', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg onclick="bad()"><script>bad()</script><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg onclick="bad()"><script>bad()</script><circle /></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, validBlock(), element)
@ -38,7 +38,7 @@ describe('PumlerBlockRenderer', () => {
})
test('open preview action does not toggle the summary panel', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, validBlock(), element)
@ -52,7 +52,7 @@ describe('PumlerBlockRenderer', () => {
})
test('collapses and expands the diagram summary with title', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, validBlockWithTitle(), element)
@ -77,7 +77,7 @@ describe('PumlerBlockRenderer', () => {
})
test('opens and closes a large preview', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, validBlock(), element)
@ -104,7 +104,7 @@ describe('PumlerBlockRenderer', () => {
})
test('registers modal cleanup for render child unload', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>')))
const element = document.createElement('div')
const cleanups: Array<() => void> = []
@ -122,7 +122,7 @@ describe('PumlerBlockRenderer', () => {
})
test('button zoom keeps the diagram center as focal point', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, validBlock(), element)
@ -142,7 +142,7 @@ describe('PumlerBlockRenderer', () => {
})
test('zooms the large preview with modified wheel events', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, validBlock(), element)
@ -173,7 +173,7 @@ describe('PumlerBlockRenderer', () => {
})
test('wheel zoom keeps the cursor position as focal point', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, validBlock(), element)
@ -204,7 +204,7 @@ describe('PumlerBlockRenderer', () => {
})
test('renders validation errors', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg></svg>'))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg></svg>')))
const element = document.createElement('div')
await renderImmediately(renderer, 'Alice -> Bob', element)
@ -214,9 +214,7 @@ describe('PumlerBlockRenderer', () => {
})
test('renders API errors with source location', async () => {
const renderer = new PumlerBlockRenderer(createClient(async () => {
throw new PumlerRenderError('Syntax error', 3, 7)
}))
const renderer = new PumlerBlockRenderer(createClient(() => Promise.reject(new PumlerRenderError('Syntax error', 3, 7))))
const element = document.createElement('div')
await renderImmediately(renderer, validBlock(), element)
@ -228,10 +226,12 @@ describe('PumlerBlockRenderer', () => {
test('renders cached SVGs immediately without waiting for debounce', async () => {
vi.useFakeTimers()
const renderDiagram = vi.fn(async () => '<svg viewBox="0 0 10 10"><circle /></svg>')
const renderDiagram = vi.fn(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>'))
const cacheGet = vi.fn(() => Promise.resolve('<svg viewBox="0 0 20 20"><rect /></svg>'))
const cacheSet = vi.fn(() => Promise.resolve())
const cache: PumlerSvgCache = {
get: vi.fn(async () => '<svg viewBox="0 0 20 20"><rect /></svg>'),
set: vi.fn(async () => undefined)
get: cacheGet,
set: cacheSet
}
const renderer = new PumlerBlockRenderer(createClient(renderDiagram), cache)
const element = document.createElement('div')
@ -242,13 +242,13 @@ describe('PumlerBlockRenderer', () => {
})
expect(renderDiagram).not.toHaveBeenCalled()
expect(cache.get).toHaveBeenCalledWith(expect.objectContaining({
expect(cacheGet).toHaveBeenCalledWith(expect.objectContaining({
provider: 'plantuml',
type: 'sequence',
theme: 'light',
source: 'Alice -> Bob'
}))
expect(cache.set).not.toHaveBeenCalled()
expect(cacheSet).not.toHaveBeenCalled()
expect(element.querySelector('rect')).not.toBeNull()
})
@ -261,11 +261,13 @@ describe('PumlerBlockRenderer', () => {
<rect fill="url(#grad)" width="20" height="20" onclick="bad()" />
</svg>
`
const cacheGet = vi.fn(() => Promise.resolve(cachedSvg))
const cacheSet = vi.fn(() => Promise.resolve())
const cache: PumlerSvgCache = {
get: vi.fn(async () => cachedSvg),
set: vi.fn(async () => undefined)
get: cacheGet,
set: cacheSet
}
const renderer = new PumlerBlockRenderer(createClient(async () => '<svg></svg>'), cache)
const renderer = new PumlerBlockRenderer(createClient(() => Promise.resolve('<svg></svg>')), cache)
const firstElement = document.createElement('div')
const secondElement = document.createElement('div')
@ -284,10 +286,10 @@ describe('PumlerBlockRenderer', () => {
})
test('does not skip cached renders for detached Obsidian processor elements', async () => {
const renderDiagram = vi.fn(async () => '<svg viewBox="0 0 10 10"><circle /></svg>')
const renderDiagram = vi.fn(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>'))
const cache: PumlerSvgCache = {
get: vi.fn(async () => '<svg viewBox="0 0 20 20"><rect /></svg>'),
set: vi.fn(async () => undefined)
get: vi.fn(() => Promise.resolve('<svg viewBox="0 0 20 20"><rect /></svg>')),
set: vi.fn(() => Promise.resolve())
}
const renderer = new PumlerBlockRenderer(createClient(renderDiagram), cache)
const element = document.createElement('div')
@ -303,7 +305,7 @@ describe('PumlerBlockRenderer', () => {
test('debounces repeated render requests for the same block', async () => {
vi.useFakeTimers()
const renderDiagram = vi.fn(async () => '<svg viewBox="0 0 10 10"><circle /></svg>')
const renderDiagram = vi.fn(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(renderDiagram))
const element = document.createElement('div')
@ -333,7 +335,7 @@ describe('PumlerBlockRenderer', () => {
test('keeps a pending debounced render alive when its view aborts', async () => {
vi.useFakeTimers()
const renderDiagram = vi.fn(async () => '<svg viewBox="0 0 10 10"><circle /></svg>')
const renderDiagram = vi.fn(() => Promise.resolve('<svg viewBox="0 0 10 10"><circle /></svg>'))
const renderer = new PumlerBlockRenderer(createClient(renderDiagram))
const element = document.createElement('div')
const abortController = new AbortController()

View file

@ -1,5 +1,5 @@
const SIZING_STYLE_PROPERTIES = ['width', 'height', 'max-width', 'max-height', 'min-width', 'min-height']
const CSS_SELECTOR_SEPARATOR_PATTERN = /,(?![^\[]*\])/
const CSS_SELECTOR_SEPARATOR_PATTERN = /,(?![^[]*])/
const ALLOWED_ELEMENTS = new Set([
'svg',