From 4ea2a4c6acd050ce7dfa9e4a37e6c124a8066935 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:07:54 +0100 Subject: [PATCH 01/11] chore: drop the unused playwright dependency, the superseded render script and a dead test gate --- docs/testing.md | 6 ++--- knip.json | 2 +- package-lock.json | 30 --------------------- package.json | 1 - test/domain/editing/ops-mutate.test.ts | 37 +++----------------------- tools/render.sh | 21 --------------- 6 files changed, 7 insertions(+), 90 deletions(-) delete mode 100755 tools/render.sh diff --git a/docs/testing.md b/docs/testing.md index eec664ac..823d4a7d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -787,9 +787,9 @@ node bin/bpmn.js add file.bpmn task "X" --after --render /tmp/png/after.png reviews and blind judging, render with real bpmn-js: ``` -tools/render.sh /tmp/png file1.bpmn file2.bpmn # any files +tools/bench/render.sh /tmp/png file1.bpmn file2.bpmn # any files tools/bench/render.sh /tmp/png tools/bench/results/latest/work/edits////*/model.bpmn ``` -Both need Google Chrome (`PUPPETEER_EXECUTABLE_PATH` overrides the macOS -default path) and fetch `bpmn-to-image` with npx. +It needs Google Chrome (`PUPPETEER_EXECUTABLE_PATH` overrides the macOS +default path) and fetches `bpmn-to-image` with npx. diff --git a/knip.json b/knip.json index 6acb6faf..d868c3a1 100644 --- a/knip.json +++ b/knip.json @@ -2,5 +2,5 @@ "$schema": "https://unpkg.com/knip@6/schema.json", "entry": ["src/api.ts", "tools/**/*.mjs", "test/**/*.test.ts"], "project": ["src/**/*.ts", "tools/**/*.mjs", "test/**/*.ts"], - "ignoreDependencies": ["@resvg/resvg-wasm", "playwright"] + "ignoreDependencies": ["@resvg/resvg-wasm"] } diff --git a/package-lock.json b/package-lock.json index 1d9ce46d..5b4abed8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26,7 +26,6 @@ "camunda-bpmn-moddle": "8.0.2", "esbuild": "^0.28.2", "knip": "6.43.0", - "playwright": "1.64.1", "tsx": "^4.23.0", "typescript": "^7.0.2", "vitest": "^5.0.2", @@ -3445,35 +3444,6 @@ "plantuml-parser": "dist/bin/cli.js" } }, - "node_modules/playwright": { - "version": "1.64.1", - "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.64.1.tgz", - "integrity": "sha512-HCrVhHr5QQuwYoswEUGTb7gGp522yflbv/ST8nF4sFWGuLjQCt17Q/EubpGGSNKYn1tsTCPZxqHhHCfVURu7UA==", - "dev": true, - "license": "Apache-2.0", - "dependencies": { - "playwright-core": "1.64.1" - }, - "bin": { - "playwright": "cli.js" - }, - "engines": { - "node": ">=20" - } - }, - "node_modules/playwright-core": { - "version": "1.64.1", - "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.64.1.tgz", - "integrity": "sha512-BC4s8y6SEVA6Q1rSxZTto8hI1KUI3xXlHGZgvGqP4IqPdI/a6FX9H29IDNz7vKo9l21b6LfYosScPL6M3xxyDQ==", - "dev": true, - "license": "Apache-2.0", - "bin": { - "playwright-core": "cli.js" - }, - "engines": { - "node": ">=20" - } - }, "node_modules/pluralize": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/pluralize/-/pluralize-8.0.0.tgz", diff --git a/package.json b/package.json index eddf575a..caeacad1 100644 --- a/package.json +++ b/package.json @@ -79,7 +79,6 @@ "camunda-bpmn-moddle": "8.0.2", "esbuild": "^0.28.2", "knip": "6.43.0", - "playwright": "1.64.1", "tsx": "^4.23.0", "typescript": "^7.0.2", "vitest": "^5.0.2", diff --git a/test/domain/editing/ops-mutate.test.ts b/test/domain/editing/ops-mutate.test.ts index c7ae3ff5..a75a736d 100644 --- a/test/domain/editing/ops-mutate.test.ts +++ b/test/domain/editing/ops-mutate.test.ts @@ -2,14 +2,12 @@ import { describe, expect, it } from 'vitest'; import { Doc } from '../../../src/domain/document/Doc.js'; import { isCliError } from '../../../src/domain/document/utils/errors.js'; import { is, many, type El } from '../../../src/domain/document/utils/moddle.js'; -import { LaneService } from '../../../src/domain/editing/participants/LaneService.js'; import { listExtensions } from '../../../src/index.js'; import { ExtensionElementService } from '../../../src/domain/editing/extensions/ExtensionElementService.js'; import { ElementOrderService } from '../../../src/domain/editing/ElementOrderService.js'; import { IdRenameService } from '../../../src/domain/editing/IdRenameService.js'; import { ChangeSet } from '../../../src/domain/document/ChangeSet.js'; import { definitionsXml, docFromXml, linearDoc } from '../../helpers/helpers.js'; -import { EventDefinitionService } from '../../../src/domain/editing/events/EventDefinitionService.js'; import { ElementRemovalService } from '../../../src/domain/editing/ElementRemovalService.js'; import { ElementTypeChangeService } from '../../../src/domain/editing/ElementTypeChangeService.js'; import { ElementMoveService } from '../../../src/domain/editing/ElementMoveService.js'; @@ -126,35 +124,6 @@ function errorCode(fn: () => unknown): string | undefined { return undefined; } -/** Functions of concurrently implemented packages: their tests become it.todo while they are stubs. */ -function implemented(fn: () => void): boolean { - try { - fn(); - return true; - } catch (err) { - return !/not implemented/.test((err as Error).message); - } -} - -const triggersReady = implemented(() => { - const doc = Doc.create(); - const ev = doc.create('bpmn:StartEvent', { id: 'Probe' }); - many(doc.defaultScope(), 'flowElements').push(ev); - ev.$parent = doc.defaultScope(); - new EventDefinitionService(doc, new EditContext(doc), ev).apply('timer', { timer: 'PT1M' }); -}); - -const lanesReady = implemented(() => { - const doc = Doc.create(); - const task = doc.create('bpmn:Task', { id: 'Probe' }); - many(doc.defaultScope(), 'flowElements').push(task); - task.$parent = doc.defaultScope(); - new LaneService(doc, new EditContext(doc)).assign(task, undefined, new ChangeSet()); -}); - -const itTriggers = triggersReady ? it : it.todo; -const itLanes = lanesReady ? it : it.todo; - /* ------------------------------------------------------------------ */ /* set */ /* ------------------------------------------------------------------ */ @@ -345,7 +314,7 @@ describe('set: event keys', () => { expect(errorCode(() => new PropertyService(doc, new EditContext(doc)).set({ op: 'set', id: 'Task_A', values: { timer: 'PT1M' } }))).toBe('E_UNKNOWN_KEY'); }); - itTriggers('changes triggers through applyTrigger (timer value, message, trigger=none)', async () => { + it('changes triggers through applyTrigger (timer value, message, trigger=none)', async () => { const doc = await fullDoc(); new PropertyService(doc, new EditContext(doc)).set({ op: 'set', id: 'Timer_A', values: { timer: 'PT2D' } }); expect(new ElementProperties(doc).read(doc.require('Timer_A'))).toMatchObject({ trigger: 'timer', timer: 'PT2D' }); @@ -746,7 +715,7 @@ describe('retype', () => { await expectRoundTrip(doc); }); - itTriggers('changes an event trigger via applyTrigger', async () => { + it('changes an event trigger via applyTrigger', async () => { const doc = await fullDoc(); const cs = new ElementTypeChangeService(doc, new EditContext(doc)).retype({ op: 'retype', id: 'Timer_A', kind: 'boundaryEvent:message', message: 'Reminder' }); const ev = doc.require('Timer_A'); @@ -907,7 +876,7 @@ describe('move', () => { expect(ids(doc.outgoing(doc.require('Task_B')))).toEqual([]); }); - itLanes('move --lane assigns lanes (alone and combined with a placement)', async () => { + it('move --lane assigns lanes (alone and combined with a placement)', async () => { const doc = await fullDoc(); new ElementMoveService(doc, new EditContext(doc)).move({ op: 'move', ids: ['Task_B'], lane: 'Lane_Sales' }); expect(ids(doc.lanesOf(doc.require('Task_B')))).toEqual(['Lane_Sales']); diff --git a/tools/render.sh b/tools/render.sh deleted file mode 100755 index acad3a8a..00000000 --- a/tools/render.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/zsh -# Renders .bpmn files to PNG with real bpmn-js, so you can look at a layout. -# -# tools/render.sh out/ file1.bpmn file2.bpmn ... -# -# Needs Google Chrome installed; bpmn-to-image is fetched on demand with npx. -# Typical use after a layout change: -# npm run build -# node tools/layout-regress.mjs --filter claim --keep -# tools/render.sh /tmp/png tools/.regress-work/*claim*.bpmn -set -e -out=$1 -shift -mkdir -p "$out" -pairs=() -for f in "$@"; do - pairs+=("$f:$out/$(basename ${f%.bpmn}).png") -done -PUPPETEER_EXECUTABLE_PATH="${PUPPETEER_EXECUTABLE_PATH:-/Applications/Google Chrome.app/Contents/MacOS/Google Chrome}" \ - npx -y bpmn-to-image@0.10.0 "${pairs[@]}" -echo "wrote ${#pairs[@]} png to $out" From f0bf64b76df79acce03498faa16191eff4bfc8b9 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:10:30 +0100 Subject: [PATCH 02/11] refactor: let Cli.run report every command error and drop the unreachable port guards --- docs/architecture.md | 28 ++++--- src/adapters/in/cli/AddElementCommand.ts | 78 +++++++++---------- src/adapters/in/cli/AlignCommand.ts | 14 ++-- src/adapters/in/cli/ApplyOpsCommand.ts | 6 +- src/adapters/in/cli/Cli.ts | 8 -- src/adapters/in/cli/ColorCommand.ts | 10 +-- src/adapters/in/cli/ConnectCommand.ts | 30 ++++--- .../in/cli/ExtensionElementsCommand.ts | 48 +++++------- src/adapters/in/cli/FindCommand.ts | 14 ++-- src/adapters/in/cli/FormatCommand.ts | 4 +- src/adapters/in/cli/GuideCommand.ts | 12 ++- src/adapters/in/cli/InsertSpaceCommand.ts | 12 ++- src/adapters/in/cli/KindsReferenceCommand.ts | 14 ++-- src/adapters/in/cli/LayoutCommand.ts | 20 +++-- src/adapters/in/cli/LayoutMetricsCommand.ts | 14 ++-- src/adapters/in/cli/MoveCommand.ts | 32 ++++---- src/adapters/in/cli/NewFileCommand.ts | 26 +++---- src/adapters/in/cli/PlaceCommand.ts | 26 +++---- src/adapters/in/cli/RemoveElementsCommand.ts | 20 +++-- src/adapters/in/cli/RenderCommand.ts | 2 +- src/adapters/in/cli/ReorderCommand.ts | 6 +- src/adapters/in/cli/RetypeCommand.ts | 6 +- src/adapters/in/cli/SetPropertiesCommand.ts | 10 +-- src/adapters/in/cli/ShowCommand.ts | 2 +- src/adapters/in/cli/TidyCommand.ts | 10 +-- src/adapters/in/cli/ValidateCommand.ts | 26 +++---- .../service/CreateDocumentService.ts | 7 +- .../service/EditDocumentService.ts | 14 +--- .../service/ReadDocumentService.ts | 9 +-- .../service/RelayoutDiagramService.ts | 14 +--- .../service/ValidateDocumentService.ts | 8 +- 31 files changed, 217 insertions(+), 313 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index eca263d6..5f4a022a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -100,23 +100,21 @@ export class AddElementCommand extends MutationCommand { options: RawOpts, ) { const opts = this.mutationOptions(options) - await this.cli.guarded(async () => { - this.checkPlacementFlags(options) - const op: AddElement = { - op: 'add', - kind, // 'userTask' - name, // 'Check invoice' - after: options['after'], // 'Event_OrderReceived' - // … - } - await this.mutate(file, [op], opts) - }, opts.json) + this.checkPlacementFlags(options) + const op: AddElement = { + op: 'add', + kind, // 'userTask' + name, // 'Check invoice' + after: options['after'], // 'Event_OrderReceived' + // … + } + await this.mutate(file, [op], opts) } } ``` -`guarded()` catches any error and prints it with its code and hint. From here on nobody knows -about flags any more. +An error thrown here reaches `Cli.run`, which prints it with its code and hint. From here on +nobody knows about flags any more. ### 2. Through the port @@ -174,7 +172,7 @@ export class EditDocumentService implements EditDocumentUseCase { // step 7: read the file const doc = 'doc' in source ? source.doc - : await Doc.fromXml(await this.loadPort(source.file).read(source.file), source.file) + : await Doc.fromXml(await this.load!.read(source.file), source.file) const target = opts.out ?? doc.file // the project's .bpmnlintrc, if any @@ -187,7 +185,7 @@ export class EditDocumentService implements EditDocumentUseCase { // step 7: write the file if (!opts.dryRun) { - result.written = await this.savePort().save(target, result.xml, { … }) + result.written = await this.save!.save(target, result.xml, { … }) } return result } diff --git a/src/adapters/in/cli/AddElementCommand.ts b/src/adapters/in/cli/AddElementCommand.ts index 25e8fd7d..73e63e61 100644 --- a/src/adapters/in/cli/AddElementCommand.ts +++ b/src/adapters/in/cli/AddElementCommand.ts @@ -39,45 +39,43 @@ export class AddElementCommand extends MutationCommand { private async run(file: string, kind: string, name: string | undefined, keyValues: string[], options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const kv = [...keyValues]; - if (name && /^[A-Za-z_][\w:.-]*=/.test(name)) { - kv.unshift(name); - name = undefined; - } - this.checkPlacementFlags(options); - this.checkFlowFlags(options, true); - if (options['ifAbsent'] && !options['id']) { - throw usageError('--if-absent needs an explicit --id to check for', { hint: 'Pass --id _ (the id add would generate), or drop --if-absent.' }); - } - const op: AddElement = { - op: 'add', - kind, - ...(name !== undefined ? { name } : {}), - ...(options['id'] ? { id: String(options['id']) } : {}), - ...(options['after'] ? { after: String(options['after']) } : {}), - ...(options['before'] ? { before: String(options['before']) } : {}), - ...(options['flow'] ? { flow: String(options['flow']) } : {}), - ...(options['in'] ? { in: String(options['in']) } : {}), - ...(options['on'] ? { on: String(options['on']) } : {}), - ...(options['to'] ? { to: String(options['to']) } : {}), - ...(options['lane'] ? { lane: String(options['lane']) } : {}), - ...(options['flowName'] ? { flowName: String(options['flowName']) } : {}), - ...(options['flowId'] ? { flowId: String(options['flowId']) } : {}), - ...(options['condition'] !== undefined ? { condition: String(options['condition']) } : {}), - ...(options['language'] ? { language: String(options['language']) } : {}), - ...(options['default'] ? { default: true } : {}), - ...(options['collapsed'] ? { collapsed: true } : {}), - ...(options['ifAbsent'] ? { ifAbsent: true } : {}), - ...(options['doc'] ? { doc: String(options['doc']) } : {}), - ...(options['text'] ? { text: String(options['text']) } : {}), - ...(options['process'] ? { process: String(options['process']) } : {}), - ...(options['blackBox'] ? { blackBox: true } : {}), - ...(options['members'] ? { members: splitList(String(options['members'])) } : {}), - ...(kv.length ? { set: parseKeyValues(kv) } : {}), - ...this.triggerOptions(options), - }; - await this.mutate(file, [op], opts); - }, opts.json); + const kv = [...keyValues]; + if (name && /^[A-Za-z_][\w:.-]*=/.test(name)) { + kv.unshift(name); + name = undefined; + } + this.checkPlacementFlags(options); + this.checkFlowFlags(options, true); + if (options['ifAbsent'] && !options['id']) { + throw usageError('--if-absent needs an explicit --id to check for', { hint: 'Pass --id _ (the id add would generate), or drop --if-absent.' }); + } + const op: AddElement = { + op: 'add', + kind, + ...(name !== undefined ? { name } : {}), + ...(options['id'] ? { id: String(options['id']) } : {}), + ...(options['after'] ? { after: String(options['after']) } : {}), + ...(options['before'] ? { before: String(options['before']) } : {}), + ...(options['flow'] ? { flow: String(options['flow']) } : {}), + ...(options['in'] ? { in: String(options['in']) } : {}), + ...(options['on'] ? { on: String(options['on']) } : {}), + ...(options['to'] ? { to: String(options['to']) } : {}), + ...(options['lane'] ? { lane: String(options['lane']) } : {}), + ...(options['flowName'] ? { flowName: String(options['flowName']) } : {}), + ...(options['flowId'] ? { flowId: String(options['flowId']) } : {}), + ...(options['condition'] !== undefined ? { condition: String(options['condition']) } : {}), + ...(options['language'] ? { language: String(options['language']) } : {}), + ...(options['default'] ? { default: true } : {}), + ...(options['collapsed'] ? { collapsed: true } : {}), + ...(options['ifAbsent'] ? { ifAbsent: true } : {}), + ...(options['doc'] ? { doc: String(options['doc']) } : {}), + ...(options['text'] ? { text: String(options['text']) } : {}), + ...(options['process'] ? { process: String(options['process']) } : {}), + ...(options['blackBox'] ? { blackBox: true } : {}), + ...(options['members'] ? { members: splitList(String(options['members'])) } : {}), + ...(kv.length ? { set: parseKeyValues(kv) } : {}), + ...this.triggerOptions(options), + }; + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/AlignCommand.ts b/src/adapters/in/cli/AlignCommand.ts index b764bb7e..48dbc1fc 100644 --- a/src/adapters/in/cli/AlignCommand.ts +++ b/src/adapters/in/cli/AlignCommand.ts @@ -15,13 +15,11 @@ export class AlignCommand extends FormatCommand { .addOption(new Option('--axis ').makeOptionMandatory()) .option('--to '), ), - ).action((file: string, ids: string[], options: RawOpts) => - this.cli.guarded(async () => { - const sel = this.selectorsOf(ids, options, true); - if (!sel.path && !sel.kind && !sel.branch && ids.length < 2 && !options['to']) throw usageError('align needs two ids, or one id and --to '); - const op: AlignShapes = { op: 'align', ...sel, axis: options['axis'] as AlignShapes['axis'], ...this.pick(options, ['to'] as const) }; - await this.runFormat(file, op, options); - }, !!options['json']), - ); + ).action(async (file: string, ids: string[], options: RawOpts) => { + const sel = this.selectorsOf(ids, options, true); + if (!sel.path && !sel.kind && !sel.branch && ids.length < 2 && !options['to']) throw usageError('align needs two ids, or one id and --to '); + const op: AlignShapes = { op: 'align', ...sel, axis: options['axis'] as AlignShapes['axis'], ...this.pick(options, ['to'] as const) }; + await this.runFormat(file, op, options); + }); } } diff --git a/src/adapters/in/cli/ApplyOpsCommand.ts b/src/adapters/in/cli/ApplyOpsCommand.ts index d0bcf07f..e8f6ab59 100644 --- a/src/adapters/in/cli/ApplyOpsCommand.ts +++ b/src/adapters/in/cli/ApplyOpsCommand.ts @@ -17,10 +17,8 @@ export class ApplyOpsCommand extends MutationCommand { private async run(file: string, opsFile: string | undefined, options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const ops: EditCommand[] = new CommandParser().parse(parseOpsJson(await this.readOpsText(file, opsFile))); - await this.mutate(file, ops, opts); - }, opts.json); + const ops: EditCommand[] = new CommandParser().parse(parseOpsJson(await this.readOpsText(file, opsFile))); + await this.mutate(file, ops, opts); } private async readOpsText(file: string, opsFile: string | undefined): Promise { diff --git a/src/adapters/in/cli/Cli.ts b/src/adapters/in/cli/Cli.ts index 7a8f1f3e..fd4b85c2 100644 --- a/src/adapters/in/cli/Cli.ts +++ b/src/adapters/in/cli/Cli.ts @@ -150,14 +150,6 @@ export class Cli { } } - async guarded(fn: () => Promise, json?: boolean): Promise { - try { - await fn(); - } catch (err) { - this.printError(err, json); - } - } - print(text: string, out: CliStream = this.io.stdout): void { out.write(text.endsWith('\n') ? text : `${text}\n`); } diff --git a/src/adapters/in/cli/ColorCommand.ts b/src/adapters/in/cli/ColorCommand.ts index 6c3d7ee5..766e59a3 100644 --- a/src/adapters/in/cli/ColorCommand.ts +++ b/src/adapters/in/cli/ColorCommand.ts @@ -10,11 +10,9 @@ export class ColorCommand extends FormatCommand { register(program: CommandType): void { this.withMutationOptions( this.withSelectors(this.define(program, 'color').addOption(new Option('--color ').makeOptionMandatory())), - ).action((file: string, ids: string[], options: RawOpts) => - this.cli.guarded(async () => { - const op: ColorElements = { op: 'color', ...this.selectorsOf(ids, options, true), color: options['color'] as ColorElements['color'] }; - await this.runFormat(file, op, options); - }, !!options['json']), - ); + ).action(async (file: string, ids: string[], options: RawOpts) => { + const op: ColorElements = { op: 'color', ...this.selectorsOf(ids, options, true), color: options['color'] as ColorElements['color'] }; + await this.runFormat(file, op, options); + }); } } diff --git a/src/adapters/in/cli/ConnectCommand.ts b/src/adapters/in/cli/ConnectCommand.ts index 2cf95357..9f720440 100644 --- a/src/adapters/in/cli/ConnectCommand.ts +++ b/src/adapters/in/cli/ConnectCommand.ts @@ -22,21 +22,19 @@ export class ConnectCommand extends MutationCommand { private async run(file: string, source: string, target: string, options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - this.checkFlowFlags(options, false); - const op: ConnectElements = { - op: 'connect', - source, - target, - ...(options['name'] ? { name: String(options['name']) } : {}), - ...(options['id'] ? { id: String(options['id']) } : {}), - ...(options['condition'] !== undefined ? { condition: String(options['condition']) } : {}), - ...(options['language'] ? { language: String(options['language']) } : {}), - ...(options['default'] ? { default: true } : {}), - ...(options['message'] ? { message: String(options['message']) } : {}), - ...(options['ifAbsent'] ? { ifAbsent: true } : {}), - }; - await this.mutate(file, [op], opts); - }, opts.json); + this.checkFlowFlags(options, false); + const op: ConnectElements = { + op: 'connect', + source, + target, + ...(options['name'] ? { name: String(options['name']) } : {}), + ...(options['id'] ? { id: String(options['id']) } : {}), + ...(options['condition'] !== undefined ? { condition: String(options['condition']) } : {}), + ...(options['language'] ? { language: String(options['language']) } : {}), + ...(options['default'] ? { default: true } : {}), + ...(options['message'] ? { message: String(options['message']) } : {}), + ...(options['ifAbsent'] ? { ifAbsent: true } : {}), + }; + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/ExtensionElementsCommand.ts b/src/adapters/in/cli/ExtensionElementsCommand.ts index e53d09b5..6e1270d8 100644 --- a/src/adapters/in/cli/ExtensionElementsCommand.ts +++ b/src/adapters/in/cli/ExtensionElementsCommand.ts @@ -28,40 +28,34 @@ export class ExtensionElementsCommand extends MutationCommand { private async add(file: string, id: string, type: string, attrs: string[], options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const op: EditExtensionElements = { - op: 'ext', - id, - action: 'add', - type, - ...(attrs.length ? { attrs: parseKeyValues(attrs, 'attribute') } : {}), - ...(options['body'] ? { body: String(options['body']) } : {}), - ...(options['xml'] ? { xml: String(options['xml']) } : {}), - ...(options['replace'] ? { replace: true } : {}), - }; - await this.mutate(file, [op], opts); - }, opts.json); + const op: EditExtensionElements = { + op: 'ext', + id, + action: 'add', + type, + ...(attrs.length ? { attrs: parseKeyValues(attrs, 'attribute') } : {}), + ...(options['body'] ? { body: String(options['body']) } : {}), + ...(options['xml'] ? { xml: String(options['xml']) } : {}), + ...(options['replace'] ? { replace: true } : {}), + }; + await this.mutate(file, [op], opts); } /** `2` or `loop.2` is an index into the extension elements; anything else a selector. */ private async remove(file: string, id: string, typeOrIndex: string, options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const indexed = /^(?:(definition|loop|condition)\.)?(\d+)$/.exec(typeOrIndex.trim()); - const op: EditExtensionElements = indexed - ? { op: 'ext', id, action: 'remove', index: Number(indexed[2]), ...(indexed[1] ? { slot: indexed[1] as EditExtensionElements['slot'] } : {}) } - : { op: 'ext', id, action: 'remove', type: typeOrIndex }; - await this.mutate(file, [op], opts); - }, opts.json); + const indexed = /^(?:(definition|loop|condition)\.)?(\d+)$/.exec(typeOrIndex.trim()); + const op: EditExtensionElements = indexed + ? { op: 'ext', id, action: 'remove', index: Number(indexed[2]), ...(indexed[1] ? { slot: indexed[1] as EditExtensionElements['slot'] } : {}) } + : { op: 'ext', id, action: 'remove', type: typeOrIndex }; + await this.mutate(file, [op], opts); } private async list(file: string, id: string, options: RawOpts): Promise { - await this.cli.guarded(async () => { - const doc = await this.cli.readDocArg(file); - const el = doc.require(id); - const items = new ExtensionLister().listAll(el); - if (options['json']) this.cli.printJson(items, !!options['pretty']); - else this.cli.print(renderExtensionList(items)); - }, !!options['json']); + const doc = await this.cli.readDocArg(file); + const el = doc.require(id); + const items = new ExtensionLister().listAll(el); + if (options['json']) this.cli.printJson(items, !!options['pretty']); + else this.cli.print(renderExtensionList(items)); } } diff --git a/src/adapters/in/cli/FindCommand.ts b/src/adapters/in/cli/FindCommand.ts index e3133e1f..ce90100a 100644 --- a/src/adapters/in/cli/FindCommand.ts +++ b/src/adapters/in/cli/FindCommand.ts @@ -8,14 +8,12 @@ import { renderFind } from '../report/utils/view-lines.js'; */ export class FindCommand extends CliCommand { register(program: CommandType): void { - this.withJsonOptions(this.define(program, 'find').option('--kind ', 'restrict to a kind', (raw: string) => this.kindArg(raw))).action((file: string, text: string, options: RawOpts) => - this.cli.guarded(async () => { - const doc = await this.cli.readDocArg(file); - const hits = this.cli.useCases.findElements.find({ doc, text, ...(options['kind'] !== undefined ? { kind: options['kind'] as string } : {}) }); - if (options['json']) this.cli.printJson(hits, !!options['pretty']); - else this.cli.print(renderFind(hits)); - }, !!options['json']), - ); + this.withJsonOptions(this.define(program, 'find').option('--kind ', 'restrict to a kind', (raw: string) => this.kindArg(raw))).action(async (file: string, text: string, options: RawOpts) => { + const doc = await this.cli.readDocArg(file); + const hits = this.cli.useCases.findElements.find({ doc, text, ...(options['kind'] !== undefined ? { kind: options['kind'] as string } : {}) }); + if (options['json']) this.cli.printJson(hits, !!options['pretty']); + else this.cli.print(renderFind(hits)); + }); } /** Kind tokens fail with the documented kind codes (like `add`), not commander's text. */ diff --git a/src/adapters/in/cli/FormatCommand.ts b/src/adapters/in/cli/FormatCommand.ts index 760a6aa9..34353ecc 100644 --- a/src/adapters/in/cli/FormatCommand.ts +++ b/src/adapters/in/cli/FormatCommand.ts @@ -40,8 +40,6 @@ export abstract class FormatCommand extends MutationCommand { protected async runFormat(file: string, op: EditCommand, options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - await this.mutate(file, [op], opts); - }, opts.json); + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/GuideCommand.ts b/src/adapters/in/cli/GuideCommand.ts index d98b2bee..6b8ea17e 100644 --- a/src/adapters/in/cli/GuideCommand.ts +++ b/src/adapters/in/cli/GuideCommand.ts @@ -10,12 +10,10 @@ export class GuideCommand extends CliCommand { register(program: CommandType): void { this.define(program, 'guide') .option('--short', 'the core: contract, reading, ops keys, placement, top errors (at most 5 KB)') - .action((topic: string | undefined, options: RawOpts) => - this.cli.guarded(async () => { - if (topic !== undefined && options['short']) throw usageError('--short and a topic cannot be combined', { hint: 'Use `bpmn guide --short` or `bpmn guide `.' }); - const guide = new AgentGuide(); - this.cli.print(options['short'] ? guide.short() : topic !== undefined ? guide.topic(topic) : guide.text()); - }), - ); + .action(async (topic: string | undefined, options: RawOpts) => { + if (topic !== undefined && options['short']) throw usageError('--short and a topic cannot be combined', { hint: 'Use `bpmn guide --short` or `bpmn guide `.' }); + const guide = new AgentGuide(); + this.cli.print(options['short'] ? guide.short() : topic !== undefined ? guide.topic(topic) : guide.text()); + }); } } diff --git a/src/adapters/in/cli/InsertSpaceCommand.ts b/src/adapters/in/cli/InsertSpaceCommand.ts index 3e32ee2b..16a2da97 100644 --- a/src/adapters/in/cli/InsertSpaceCommand.ts +++ b/src/adapters/in/cli/InsertSpaceCommand.ts @@ -18,12 +18,10 @@ export class InsertSpaceCommand extends FormatCommand { if (!parseSpaceAmount(raw)) throw new InvalidArgumentError('expected column, row, col, row (2col: two columns) or pixels (80, 80px); negative (-column, -2col, -80) to close space'); return /^\s*-?\d+\s*$/.test(raw) ? Number(raw) : raw; }), - ).action((file: string, options: RawOpts) => - this.cli.guarded(async () => { - this.checkFlagGroups(options, [['after', 'below']], 'space needs --after (horizontal space) or --below (vertical space)'); - const op: InsertSpace = { op: 'space', ...this.pick(options, ['after', 'below'] as const), ...(options['by'] !== undefined ? { by: options['by'] as InsertSpace['by'] } : {}) }; - await this.runFormat(file, op, options); - }, !!options['json']), - ); + ).action(async (file: string, options: RawOpts) => { + this.checkFlagGroups(options, [['after', 'below']], 'space needs --after (horizontal space) or --below (vertical space)'); + const op: InsertSpace = { op: 'space', ...this.pick(options, ['after', 'below'] as const), ...(options['by'] !== undefined ? { by: options['by'] as InsertSpace['by'] } : {}) }; + await this.runFormat(file, op, options); + }); } } diff --git a/src/adapters/in/cli/KindsReferenceCommand.ts b/src/adapters/in/cli/KindsReferenceCommand.ts index 688ee532..1f2ef0e2 100644 --- a/src/adapters/in/cli/KindsReferenceCommand.ts +++ b/src/adapters/in/cli/KindsReferenceCommand.ts @@ -8,13 +8,11 @@ import { CliCommand, type RawOpts } from './CliCommand.js'; */ export class KindsReferenceCommand extends CliCommand { register(program: CommandType): void { - this.withJsonOptions(this.define(program, 'kinds').option('--section ', `only these parts (comma-separated): ${KINDS_SECTION_NAMES}`)).action((options: RawOpts) => - this.cli.guarded(async () => { - const kinds = new KindsReference(); - const sections = options['section'] !== undefined ? kinds.sections(String(options['section'])) : undefined; - if (options['json']) this.cli.printJson(sections ? kinds.sectionJson(sections) : kinds.json(), !!options['pretty']); - else this.cli.print(sections ? kinds.sectionText(sections) : kinds.text()); - }, !!options['json']), - ); + this.withJsonOptions(this.define(program, 'kinds').option('--section ', `only these parts (comma-separated): ${KINDS_SECTION_NAMES}`)).action(async (options: RawOpts) => { + const kinds = new KindsReference(); + const sections = options['section'] !== undefined ? kinds.sections(String(options['section'])) : undefined; + if (options['json']) this.cli.printJson(sections ? kinds.sectionJson(sections) : kinds.json(), !!options['pretty']); + else this.cli.print(sections ? kinds.sectionText(sections) : kinds.text()); + }); } } diff --git a/src/adapters/in/cli/LayoutCommand.ts b/src/adapters/in/cli/LayoutCommand.ts index 25c02d50..f9c3b91d 100644 --- a/src/adapters/in/cli/LayoutCommand.ts +++ b/src/adapters/in/cli/LayoutCommand.ts @@ -20,16 +20,14 @@ export class LayoutCommand extends MutationCommand { /** --expand / --collapse are validated like `set expanded=`: unknown ids and non-sub-processes are errors. */ private async run(file: string, options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const expand = splitList(options['expand'] as string | undefined); - const collapse = splitList(options['collapse'] as string | undefined); - const layoutOpts = { tidy: !!options['tidy'], ...(expand ? { expand } : {}), ...(collapse ? { collapse } : {}) }; - await this.runMutation( - file, - opts, - (fo) => this.cli.useCases.relayoutDiagram.relayout({ ...fo, ...layoutOpts, source: { file } }), - (doc, fo) => this.cli.useCases.relayoutDiagram.relayout({ ...fo, ...layoutOpts, source: { doc } }), - ); - }, opts.json); + const expand = splitList(options['expand'] as string | undefined); + const collapse = splitList(options['collapse'] as string | undefined); + const layoutOpts = { tidy: !!options['tidy'], ...(expand ? { expand } : {}), ...(collapse ? { collapse } : {}) }; + await this.runMutation( + file, + opts, + (fo) => this.cli.useCases.relayoutDiagram.relayout({ ...fo, ...layoutOpts, source: { file } }), + (doc, fo) => this.cli.useCases.relayoutDiagram.relayout({ ...fo, ...layoutOpts, source: { doc } }), + ); } } diff --git a/src/adapters/in/cli/LayoutMetricsCommand.ts b/src/adapters/in/cli/LayoutMetricsCommand.ts index 9ab47e92..5efbe99a 100644 --- a/src/adapters/in/cli/LayoutMetricsCommand.ts +++ b/src/adapters/in/cli/LayoutMetricsCommand.ts @@ -10,13 +10,11 @@ import { MetricsText } from '../report/MetricsText.js'; */ export class LayoutMetricsCommand extends CliCommand { register(program: CommandType): void { - this.withJsonOptions(this.define(program, 'metrics')).action((file: string, options: RawOpts) => - this.cli.guarded(async () => { - const doc = await this.cli.readDocArg(file); - const metrics = LayoutAnalysis.of(doc.definitions); - if (options['json']) this.cli.printJson({ file, ...metricsJson(metrics) }, !!options['pretty']); - else this.cli.print(new MetricsText(metrics).text()); - }, !!options['json']), - ); + this.withJsonOptions(this.define(program, 'metrics')).action(async (file: string, options: RawOpts) => { + const doc = await this.cli.readDocArg(file); + const metrics = LayoutAnalysis.of(doc.definitions); + if (options['json']) this.cli.printJson({ file, ...metricsJson(metrics) }, !!options['pretty']); + else this.cli.print(new MetricsText(metrics).text()); + }); } } diff --git a/src/adapters/in/cli/MoveCommand.ts b/src/adapters/in/cli/MoveCommand.ts index 57238389..4123ad6f 100644 --- a/src/adapters/in/cli/MoveCommand.ts +++ b/src/adapters/in/cli/MoveCommand.ts @@ -22,22 +22,20 @@ export class MoveCommand extends MutationCommand { private async run(file: string, ids: string[], options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - this.checkPlacementFlags(options); - const op: MoveElements = { - op: 'move', - ids, - ...(options['after'] ? { after: String(options['after']) } : {}), - ...(options['before'] ? { before: String(options['before']) } : {}), - ...(options['flow'] ? { flow: String(options['flow']) } : {}), - ...(options['in'] ? { in: String(options['in']) } : {}), - ...(options['on'] ? { on: String(options['on']) } : {}), - ...(options['lane'] !== undefined ? { lane: String(options['lane']) } : {}), - }; - if (!op.after && !op.before && !op.flow && !op.in && !op.on && op.lane === undefined) { - throw usageError('move needs one of --after, --before, --flow, --in, --on or --lane'); - } - await this.mutate(file, [op], opts); - }, opts.json); + this.checkPlacementFlags(options); + const op: MoveElements = { + op: 'move', + ids, + ...(options['after'] ? { after: String(options['after']) } : {}), + ...(options['before'] ? { before: String(options['before']) } : {}), + ...(options['flow'] ? { flow: String(options['flow']) } : {}), + ...(options['in'] ? { in: String(options['in']) } : {}), + ...(options['on'] ? { on: String(options['on']) } : {}), + ...(options['lane'] !== undefined ? { lane: String(options['lane']) } : {}), + }; + if (!op.after && !op.before && !op.flow && !op.in && !op.on && op.lane === undefined) { + throw usageError('move needs one of --after, --before, --flow, --in, --on or --lane'); + } + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/NewFileCommand.ts b/src/adapters/in/cli/NewFileCommand.ts index 9a7d57ae..9b842e05 100644 --- a/src/adapters/in/cli/NewFileCommand.ts +++ b/src/adapters/in/cli/NewFileCommand.ts @@ -28,19 +28,17 @@ export class NewFileCommand extends MutationCommand { /** `new -` (or -o -) writes the new model to stdout. */ private async run(file: string, options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const stdout = file === '-' || opts.out === '-'; - const result = await this.cli.useCases.createDocument.create({ - ...opts, - ...(file === '-' ? {} : { destination: file }), - processName: options['name'] as string | undefined, - processId: options['id'] as string | undefined, - executable: options['executable'] !== false, - target: options['target'] as Target | undefined, - mustNotExist: true, - ...(stdout ? { out: undefined, dryRun: true } : {}), - }); - await this.emit(result, opts, stdout); - }, opts.json); + const stdout = file === '-' || opts.out === '-'; + const result = await this.cli.useCases.createDocument.create({ + ...opts, + ...(file === '-' ? {} : { destination: file }), + processName: options['name'] as string | undefined, + processId: options['id'] as string | undefined, + executable: options['executable'] !== false, + target: options['target'] as Target | undefined, + mustNotExist: true, + ...(stdout ? { out: undefined, dryRun: true } : {}), + }); + await this.emit(result, opts, stdout); } } diff --git a/src/adapters/in/cli/PlaceCommand.ts b/src/adapters/in/cli/PlaceCommand.ts index 57ca22f7..701e0165 100644 --- a/src/adapters/in/cli/PlaceCommand.ts +++ b/src/adapters/in/cli/PlaceCommand.ts @@ -20,19 +20,17 @@ export class PlaceCommand extends FormatCommand { .option('--after ') .option('--before ') .option('--side ')), - ).action((file: string, ids: string[], options: RawOpts) => - this.cli.guarded(async () => { - if (options['side'] !== undefined) { - const given = ['rowOf', 'below', 'above', 'columnOf', 'after', 'before', 'path', 'kind', 'branch'].filter((flag) => options[flag] !== undefined); - if (given.length) throw usageError(`--side cannot be combined with ${given.map(flag).join(', ')}`, { hint: '`bpmn place --side top` moves a boundary event to the top border of its host; place the host to move both.' }); - if (ids.length !== 1) throw usageError('--side takes exactly one id: the boundary event', { hint: 'Example: `bpmn place order.bpmn Event_PaymentOverdue --side top`.' }); - await this.runFormat(file, { op: 'place', ids, side: options['side'] as PlaceShapes['side'] }, options); - return; - } - this.checkFlagGroups(options, [['rowOf', 'below', 'above'], ['columnOf', 'after', 'before']], 'place needs a row (--row-of, --below, --above) and/or a column (--column-of, --after, --before), or --side for a boundary event'); - const op: PlaceShapes = { op: 'place', ...this.selectorsOf(ids, options, true), ...this.pick(options, ['rowOf', 'below', 'above', 'columnOf', 'after', 'before'] as const) }; - await this.runFormat(file, op, options); - }, !!options['json']), - ); + ).action(async (file: string, ids: string[], options: RawOpts) => { + if (options['side'] !== undefined) { + const given = ['rowOf', 'below', 'above', 'columnOf', 'after', 'before', 'path', 'kind', 'branch'].filter((flag) => options[flag] !== undefined); + if (given.length) throw usageError(`--side cannot be combined with ${given.map(flag).join(', ')}`, { hint: '`bpmn place --side top` moves a boundary event to the top border of its host; place the host to move both.' }); + if (ids.length !== 1) throw usageError('--side takes exactly one id: the boundary event', { hint: 'Example: `bpmn place order.bpmn Event_PaymentOverdue --side top`.' }); + await this.runFormat(file, { op: 'place', ids, side: options['side'] as PlaceShapes['side'] }, options); + return; + } + this.checkFlagGroups(options, [['rowOf', 'below', 'above'], ['columnOf', 'after', 'before']], 'place needs a row (--row-of, --below, --above) and/or a column (--column-of, --after, --before), or --side for a boundary event'); + const op: PlaceShapes = { op: 'place', ...this.selectorsOf(ids, options, true), ...this.pick(options, ['rowOf', 'below', 'above', 'columnOf', 'after', 'before'] as const) }; + await this.runFormat(file, op, options); + }); } } diff --git a/src/adapters/in/cli/RemoveElementsCommand.ts b/src/adapters/in/cli/RemoveElementsCommand.ts index a83a2266..71bd132a 100644 --- a/src/adapters/in/cli/RemoveElementsCommand.ts +++ b/src/adapters/in/cli/RemoveElementsCommand.ts @@ -19,16 +19,14 @@ export class RemoveElementsCommand extends MutationCommand { private async run(file: string, ids: string[], options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const op: RemoveElements = { - op: 'remove', - ids, - bridge: options['bridge'] !== false, - ...(options['bridgeAll'] ? { bridgeAll: true } : {}), - ...(options['withBranch'] ? { withBranch: true } : {}), - ...(options['ifExists'] ? { ifExists: true } : {}), - }; - await this.mutate(file, [op], opts); - }, opts.json); + const op: RemoveElements = { + op: 'remove', + ids, + bridge: options['bridge'] !== false, + ...(options['bridgeAll'] ? { bridgeAll: true } : {}), + ...(options['withBranch'] ? { withBranch: true } : {}), + ...(options['ifExists'] ? { ifExists: true } : {}), + }; + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/RenderCommand.ts b/src/adapters/in/cli/RenderCommand.ts index b2e08e8b..5a0bd41a 100644 --- a/src/adapters/in/cli/RenderCommand.ts +++ b/src/adapters/in/cli/RenderCommand.ts @@ -31,7 +31,7 @@ export class RenderCommand extends CliCommand { .option('--tiles', 'when the image would be unreadable (labels below 0.75): split it at the diagram columns into readable tiles') .option('--max-size ', `the longest edge in px (default ${RENDER_MAX_SIZE}); a larger drawing is scaled down`, this.boundedNumber('px', 50, 20000, true)) .option('--scale ', 'zoom factor (default 1); the image still stays within --max-size', this.boundedNumber('zoom', 0.05, 10, false)), - ).action((file: string, ids: string[], options: RawOpts) => this.cli.guarded(() => this.run(file, ids, options), !!options['json'])); + ).action((file: string, ids: string[], options: RawOpts) => this.run(file, ids, options)); } private async run(file: string, ids: string[], options: RawOpts): Promise { diff --git a/src/adapters/in/cli/ReorderCommand.ts b/src/adapters/in/cli/ReorderCommand.ts index 1828753e..67e1b167 100644 --- a/src/adapters/in/cli/ReorderCommand.ts +++ b/src/adapters/in/cli/ReorderCommand.ts @@ -14,9 +14,7 @@ export class ReorderCommand extends MutationCommand { private async run(file: string, nodeId: string, flowIds: string[], options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const op: ReorderElements = { op: 'order', id: nodeId, flows: flowIds }; - await this.mutate(file, [op], opts); - }, opts.json); + const op: ReorderElements = { op: 'order', id: nodeId, flows: flowIds }; + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/RetypeCommand.ts b/src/adapters/in/cli/RetypeCommand.ts index 38f0af84..8d927e07 100644 --- a/src/adapters/in/cli/RetypeCommand.ts +++ b/src/adapters/in/cli/RetypeCommand.ts @@ -13,9 +13,7 @@ export class RetypeCommand extends MutationCommand { private async run(file: string, id: string, kind: string, options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const op: ChangeElementType = { op: 'retype', id, kind, ...this.triggerOptions(options) }; - await this.mutate(file, [op], opts); - }, opts.json); + const op: ChangeElementType = { op: 'retype', id, kind, ...this.triggerOptions(options) }; + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/SetPropertiesCommand.ts b/src/adapters/in/cli/SetPropertiesCommand.ts index c2add565..233ac336 100644 --- a/src/adapters/in/cli/SetPropertiesCommand.ts +++ b/src/adapters/in/cli/SetPropertiesCommand.ts @@ -17,11 +17,9 @@ export class SetPropertiesCommand extends MutationCommand { private async run(file: string, id: string, keyValues: string[], options: RawOpts): Promise { const opts = this.mutationOptions(options); - await this.cli.guarded(async () => { - const unset = (options['unset'] as string[]) ?? []; - if (!keyValues.length && !unset.length) throw new CliError('E_USAGE', 'Nothing to set: pass key=value pairs or --unset ', 'usage'); - const op: SetProperties = { op: 'set', id, values: parseKeyValues(keyValues), ...(unset.length ? { unset } : {}) }; - await this.mutate(file, [op], opts); - }, opts.json); + const unset = (options['unset'] as string[]) ?? []; + if (!keyValues.length && !unset.length) throw new CliError('E_USAGE', 'Nothing to set: pass key=value pairs or --unset ', 'usage'); + const op: SetProperties = { op: 'set', id, values: parseKeyValues(keyValues), ...(unset.length ? { unset } : {}) }; + await this.mutate(file, [op], opts); } } diff --git a/src/adapters/in/cli/ShowCommand.ts b/src/adapters/in/cli/ShowCommand.ts index 5bbc872a..1262f2f8 100644 --- a/src/adapters/in/cli/ShowCommand.ts +++ b/src/adapters/in/cli/ShowCommand.ts @@ -25,7 +25,7 @@ export class ShowCommand extends CliCommand { .option('--conventions', "the id conventions new ids follow here: element-id-naming of the .bpmnlintrc (prefixes, case, qualifier) and the file's own style; a file that does not exist yet: those of a new model there") .option('--lint-config ', "with --conventions: the project's bpmnlint configuration (default: the nearest .bpmnlintrc above the file, up to the repository root, whoever owns it; outside one only in the file's directory; stdin: from the working directory)") .option('--no-lint-config', 'with --conventions: do not read the .bpmnlintrc (no id conventions from element-id-naming)'), - ).action((file: string, id: string | undefined, options: RawOpts) => this.cli.guarded(() => (options['conventions'] ? this.conventions(file, id, options) : this.view(file, id, options)), !!options['json'])); + ).action((file: string, id: string | undefined, options: RawOpts) => (options['conventions'] ? this.conventions(file, id, options) : this.view(file, id, options))); } /** A file that does not exist yet gets the conventions a new model there gets (before `bpmn new`). */ diff --git a/src/adapters/in/cli/TidyCommand.ts b/src/adapters/in/cli/TidyCommand.ts index 007c0ca9..048e1879 100644 --- a/src/adapters/in/cli/TidyCommand.ts +++ b/src/adapters/in/cli/TidyCommand.ts @@ -8,11 +8,9 @@ import { FormatCommand } from './FormatCommand.js'; */ export class TidyCommand extends FormatCommand { register(program: CommandType): void { - this.withMutationOptions(this.withSelectors(this.define(program, 'tidy'))).action((file: string, ids: string[], options: RawOpts) => - this.cli.guarded(async () => { - const op: TidyShapes = { op: 'tidy', ...this.selectorsOf(ids, options, false) }; - await this.runFormat(file, op, options); - }, !!options['json']), - ); + this.withMutationOptions(this.withSelectors(this.define(program, 'tidy'))).action(async (file: string, ids: string[], options: RawOpts) => { + const op: TidyShapes = { op: 'tidy', ...this.selectorsOf(ids, options, false) }; + await this.runFormat(file, op, options); + }); } } diff --git a/src/adapters/in/cli/ValidateCommand.ts b/src/adapters/in/cli/ValidateCommand.ts index 3193e175..1589df18 100644 --- a/src/adapters/in/cli/ValidateCommand.ts +++ b/src/adapters/in/cli/ValidateCommand.ts @@ -18,19 +18,17 @@ export class ValidateCommand extends CliCommand { if (!(PLATFORM_CHOICES as readonly string[]).includes(raw)) throw new InvalidArgumentError(`expected ${PLATFORM_CHOICES.join(', ')}`); return raw; }), - ).action((file: string, options: RawOpts) => - this.cli.guarded(async () => { - const checkOpts = { platform: (options['platform'] as PlatformChoice | undefined) ?? 'auto', ...this.lintOptions(options) }; - const report = new ValidationReportBuilder(await this.cli.useCases.validateDocument.validate({ ...checkOpts, source: file === '-' ? { doc: await this.cli.readDocArg(file) } : { file } })).report(); - if (options['json']) { - const { ok, ...rest } = report; - this.cli.printJson({ ok, file, ...rest }, !!options['pretty']); - } else { - this.cli.print(new ValidationText(report).text()); - } - if (report.errors.length) this.cli.exitCode = 2; - else if (options['strict'] && (report.warnings.length || report.importWarnings.length)) this.cli.exitCode = 5; - }, !!options['json']), - ); + ).action(async (file: string, options: RawOpts) => { + const checkOpts = { platform: (options['platform'] as PlatformChoice | undefined) ?? 'auto', ...this.lintOptions(options) }; + const report = new ValidationReportBuilder(await this.cli.useCases.validateDocument.validate({ ...checkOpts, source: file === '-' ? { doc: await this.cli.readDocArg(file) } : { file } })).report(); + if (options['json']) { + const { ok, ...rest } = report; + this.cli.printJson({ ok, file, ...rest }, !!options['pretty']); + } else { + this.cli.print(new ValidationText(report).text()); + } + if (report.errors.length) this.cli.exitCode = 2; + else if (options['strict'] && (report.warnings.length || report.importWarnings.length)) this.cli.exitCode = 5; + }); } } diff --git a/src/application/service/CreateDocumentService.ts b/src/application/service/CreateDocumentService.ts index 03f119a4..d08528c3 100644 --- a/src/application/service/CreateDocumentService.ts +++ b/src/application/service/CreateDocumentService.ts @@ -12,11 +12,6 @@ export class CreateDocumentService implements CreateDocumentUseCase { private readonly save?: SaveDocumentPort, ) {} - private savePort(): SaveDocumentPort { - if (!this.save) throw ioError('E_NO_FILE', 'No output file given'); - return this.save; - } - /** An existing file fails before the .bpmnlintrc is read. */ async create(command: CreateDocumentUseCase.Command): Promise { const { destination, ...opts } = command; @@ -28,7 +23,7 @@ export class CreateDocumentService implements CreateDocumentUseCase { const conventions = opts.conventions ?? (await this.settings.idConventions(target, opts))?.conventions; const doc = Doc.create({ processName: opts.processName, processId: opts.processId, executable: opts.executable, target: opts.target, ...(conventions ? { conventions } : {}) }, destination); const result = await new EditBatchService(doc, [], lint.applyTo(opts, target)).run(target); - if (!opts.dryRun && target) result.written = await this.savePort().save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); + if (!opts.dryRun && target) result.written = await this.save!.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); result.changes.create(newProcessChange(doc, opts.processName)); return result; } diff --git a/src/application/service/EditDocumentService.ts b/src/application/service/EditDocumentService.ts index 64a4ac9e..8f9cc50f 100644 --- a/src/application/service/EditDocumentService.ts +++ b/src/application/service/EditDocumentService.ts @@ -14,26 +14,16 @@ export class EditDocumentService implements EditDocumentUseCase { private readonly save?: SaveDocumentPort, ) {} - private loadPort(file: string): LoadDocumentPort { - if (!this.load) throw ioError('E_FILE_NOT_FOUND', `Cannot read ${file}: no such document`, { file }); - return this.load; - } - - private savePort(): SaveDocumentPort { - if (!this.save) throw ioError('E_NO_FILE', 'No output file given'); - return this.save; - } - async edit(command: EditDocumentUseCase.Command): Promise { const { source, commands, ...opts } = command; - const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.loadPort(source.file).read(source.file), source.file); + const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load!.read(source.file), source.file); doc.assertLossless(opts); const target = opts.out ?? doc.file; if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); if (opts.mustNotExist) await this.save?.assertNew(target, opts); const lint = await this.settings.lint(opts.file ?? target, opts); const result = await new EditBatchService(doc, commands, lint.applyTo(opts, target)).run(target); - if (!opts.dryRun && target) result.written = await this.savePort().save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); + if (!opts.dryRun && target) result.written = await this.save!.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); return result; } diff --git a/src/application/service/ReadDocumentService.ts b/src/application/service/ReadDocumentService.ts index 840bb437..9f142924 100644 --- a/src/application/service/ReadDocumentService.ts +++ b/src/application/service/ReadDocumentService.ts @@ -1,18 +1,11 @@ import { Doc } from '../../domain/document/Doc.js'; import type { ReadDocumentUseCase } from '../port/in/ReadDocumentUseCase.js'; -import { ioError } from '../../domain/document/utils/errors.js'; import type { LoadDocumentPort } from '../port/out/LoadDocumentPort.js'; /** Decoded in the encoding the XML declaration names, so ISO-8859-1 and Windows-1252 characters survive. */ export class ReadDocumentService implements ReadDocumentUseCase { constructor(private readonly load?: LoadDocumentPort) {} - private loadPort(file: string): LoadDocumentPort { - if (!this.load) throw ioError('E_FILE_NOT_FOUND', `Cannot read ${file}: no such document`, { file }); - return this.load; - } - - async read(command: ReadDocumentUseCase.Command): Promise { const doc = await this.parse(command.source); if (command.lossless) doc.assertLossless(command); @@ -25,6 +18,6 @@ export class ReadDocumentService implements ReadDocumentUseCase { private async parse(source: ReadDocumentUseCase.Command['source']): Promise { if ('xml' in source) return Doc.fromXml(source.xml, source.file); - return 'file' in source ? Doc.fromXml(await this.loadPort(source.file).read(source.file), source.file) : Doc.fromXml(this.loadPort(source.name).decode(source.bytes, source.name)); + return 'file' in source ? Doc.fromXml(await this.load!.read(source.file), source.file) : Doc.fromXml(this.load!.decode(source.bytes, source.name)); } } diff --git a/src/application/service/RelayoutDiagramService.ts b/src/application/service/RelayoutDiagramService.ts index 0366fe9b..3a4aa3ba 100644 --- a/src/application/service/RelayoutDiagramService.ts +++ b/src/application/service/RelayoutDiagramService.ts @@ -20,20 +20,10 @@ export class RelayoutDiagramService implements RelayoutDiagramUseCase { private readonly save?: SaveDocumentPort, ) {} - private loadPort(file: string): LoadDocumentPort { - if (!this.load) throw ioError('E_FILE_NOT_FOUND', `Cannot read ${file}: no such document`, { file }); - return this.load; - } - - private savePort(): SaveDocumentPort { - if (!this.save) throw ioError('E_NO_FILE', 'No output file given'); - return this.save; - } - async relayout(command: RelayoutDiagramUseCase.Command): Promise { const { source, ...opts } = command; RelayoutDiagramService.assertOptions(opts); - const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.loadPort(source.file).read(source.file), source.file); + const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load!.read(source.file), source.file); doc.assertLossless(opts); const target = opts.out ?? doc.file; if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); @@ -76,7 +66,7 @@ export class RelayoutDiagramService implements RelayoutDiagramUseCase { private async write(doc: Doc, batch: EditBatchService, target: string | undefined, opts: FileWriteOptions): Promise { const result = await batch.run(target); - if (!opts.dryRun && target) result.written = await this.savePort().save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); + if (!opts.dryRun && target) result.written = await this.save!.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); return result; } } diff --git a/src/application/service/ValidateDocumentService.ts b/src/application/service/ValidateDocumentService.ts index 2e3d2c24..5307fe51 100644 --- a/src/application/service/ValidateDocumentService.ts +++ b/src/application/service/ValidateDocumentService.ts @@ -16,7 +16,6 @@ import { once } from '../../domain/validation/utils/once.js'; import type { ValidatorReport, ValidatorRun } from '../../domain/validation/validators.js'; import { toCliError } from '../../domain/document/utils/cli-error.js'; import type { CheckOptions, CheckResult, ValidateDocumentUseCase } from '../port/in/ValidateDocumentUseCase.js'; -import { ioError } from '../../domain/document/utils/errors.js'; import type { LoadDocumentPort } from '../port/out/LoadDocumentPort.js'; import type { LoadProjectSettingsPort } from '../port/out/LoadProjectSettingsPort.js'; @@ -27,14 +26,9 @@ export class ValidateDocumentService implements ValidateDocumentUseCase { private readonly load?: LoadDocumentPort, ) {} - private loadPort(file: string): LoadDocumentPort { - if (!this.load) throw ioError('E_FILE_NOT_FOUND', `Cannot read ${file}: no such document`, { file }); - return this.load; - } - async validate(command: ValidateDocumentUseCase.Command): Promise { const { source, ...opts } = command; - const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.loadPort(source.file).read(source.file), source.file); + const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load!.read(source.file), source.file); return this.check(doc, (await this.settings.lint(opts.file ?? doc.file, opts)).applyTo(opts, doc.file)); } From 4aa4c53371d8edd69a7386dae1050ad154e61fae Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:14:56 +0100 Subject: [PATCH 03/11] refactor: share one set of geometry helpers across the layout engines and metrics --- src/domain/diagram/di/PlaneAnalysis.ts | 23 ++++----- src/domain/diagram/di/ProblemLocator.ts | 15 +++--- .../diagram/layout/clean/LabelDeclutter.ts | 8 +-- .../diagram/layout/clean/MessageFlowRouter.ts | 3 +- src/domain/diagram/layout/clean/NotePlacer.ts | 9 ++-- .../layout/clean/ScopeArtifactPlacer.ts | 8 +-- .../diagram/layout/clean/ScopeLabels.ts | 4 +- .../diagram/layout/clean/TopBorderTrial.ts | 10 ++-- .../diagram/layout/incremental/Drawing.ts | 11 +---- .../diagram/layout/incremental/FrameEditor.ts | 22 ++------- .../diagram/layout/incremental/NodePlacer.ts | 41 ++-------------- .../layout/incremental/OrthogonalRouter.ts | 4 +- .../diagram/layout/incremental/SpaceTool.ts | 20 ++------ src/domain/diagram/utils/geom.ts | 23 +++++++-- src/domain/diagram/utils/geometry.ts | 6 --- src/domain/diagram/utils/harness.ts | 49 +++---------------- src/domain/diagram/utils/routing.ts | 11 +---- src/domain/diagram/utils/scope.ts | 41 ++++------------ 18 files changed, 92 insertions(+), 216 deletions(-) diff --git a/src/domain/diagram/di/PlaneAnalysis.ts b/src/domain/diagram/di/PlaneAnalysis.ts index 7e5a0131..d9e836b0 100644 --- a/src/domain/diagram/di/PlaneAnalysis.ts +++ b/src/domain/diagram/di/PlaneAnalysis.ts @@ -1,7 +1,8 @@ import { is, type El } from '../../document/utils/moddle.js'; import type { Box, Point } from './types.js'; -import { cx, cy, segmentsCross } from '../utils/geom.js'; -import { collinear, cuts, drawnEnds, enclosingProcess, glyph, hits, inside, joinTail, lookup, overlap, ptIn, readDrawnPlane, subTitleOf, type Edge, type MetricIndex, type Seg, type Shape } from '../utils/harness.js'; +import { containsPoint, cx, cy, overlaps, segmentHits, segmentsCross } from '../utils/geom.js'; +import type { Segment } from '../utils/geometry.js'; +import { collinear, drawnEnds, enclosingProcess, glyph, hits, inside, joinTail, lookup, readDrawnPlane, subTitleOf, type Edge, type MetricIndex, type Shape } from '../utils/harness.js'; import { idOf, raw, rawList } from '../utils/raw.js'; import type { MetricKey } from '../utils/problems.js'; @@ -72,7 +73,7 @@ export class PlaneAnalysis { const first = leaf[firstIndex]!; const second = leaf[secondIndex]!; const attached = (is(first.el, 'bpmn:BoundaryEvent') && this.hostOf(first.el) === second.id) || (is(second.el, 'bpmn:BoundaryEvent') && this.hostOf(second.el) === first.id); - if (!attached && overlap(first.bounds, second.bounds)) add('overlaps', `${first.id}~${second.id}`, [first.id, second.id]); + if (!attached && overlaps(first.bounds, second.bounds)) add('overlaps', `${first.id}~${second.id}`, [first.id, second.id]); } } } @@ -87,7 +88,7 @@ export class PlaneAnalysis { for (const sh of leaf) { if (edge.ends.has(sh.id)) continue; if (is(sh.el, 'bpmn:BoundaryEvent') && edge.ends.has(this.hostOf(sh.el))) continue; - if (cuts(segment, sh.bounds)) add('through', `${edge.id}->${sh.id}`, [edge.id, sh.id]); + if (segmentHits(...segment, sh.bounds)) add('through', `${edge.id}->${sh.id}`, [edge.id, sh.id]); } } } @@ -119,11 +120,11 @@ export class PlaneAnalysis { for (let labelIndex = 0; labelIndex < labels.length; labelIndex++) { const label = labels[labelIndex]!; for (const shape of leaf) { - if (shape.id !== label.owner && overlap(label.box, shape.bounds)) add('labelClash', `${label.owner}#L~${shape.id}`, [label.owner, shape.id], 'label on shape'); + if (shape.id !== label.owner && overlaps(label.box, shape.bounds)) add('labelClash', `${label.owner}#L~${shape.id}`, [label.owner, shape.id], 'label on shape'); } for (let otherIndex = labelIndex + 1; otherIndex < labels.length; otherIndex++) { const other = labels[otherIndex]!; - if (overlap(label.box, other.box)) add('labelClash', `${label.owner}#L~${other.owner}#L`, [label.owner, other.owner], 'label on label'); + if (overlaps(label.box, other.box)) add('labelClash', `${label.owner}#L~${other.owner}#L`, [label.owner, other.owner], 'label on label'); } for (const edge of edges) if (edge.segs.some((segment) => hits(segment, label.text, 0))) add('labelOnLine', `${label.owner}#L~${edge.id}`, [label.owner, edge.id]); } @@ -193,11 +194,11 @@ export class PlaneAnalysis { private checkSequenceFlow(edge: Edge): void { const { sem, byId, add } = this; const frame = this.flowFrame(edge.id); - if (frame && edge.pts.some((point) => !ptIn(point, frame.bounds))) add('edgeOutside', edge.id, [edge.id, frame.id]); + if (frame && edge.pts.some((point) => !containsPoint(frame.bounds, point))) add('edgeOutside', edge.id, [edge.id, frame.id]); const sl = lookup(sem.laneOf, idOf(raw(edge.el, 'sourceRef'))); const tl = lookup(sem.laneOf, idOf(raw(edge.el, 'targetRef'))); const lane = sl && sl === tl ? lookup(byId, idOf(sl)) : undefined; - if (lane && edge.pts.some((point) => !ptIn(point, lane.bounds))) add('edgeLeavesLane', edge.id, [edge.id, lane.id]); + if (lane && edge.pts.some((point) => !containsPoint(lane.bounds, point))) add('edgeLeavesLane', edge.id, [edge.id, lane.id]); } private checkMessageFlow(edge: Edge): void { @@ -452,12 +453,12 @@ export class PlaneAnalysis { const owners: Array<{ id: string; el: El; label?: Box }> = [...shapes.filter((shape) => this.externallyLabelled(shape.el)), ...edges.filter((edge) => is(edge.el, 'bpmn:SequenceFlow') || is(edge.el, 'bpmn:MessageFlow'))]; for (const owner of owners) { if (!owner.label || !raw(owner.el, 'name')?.trim()) continue; - for (const title of titles) if (overlap(owner.label, title.box)) add('labelOnTitle', `${owner.id}#L^${title.id}`, [owner.id, title.id]); + for (const title of titles) if (overlaps(owner.label, title.box)) add('labelOnTitle', `${owner.id}#L^${title.id}`, [owner.id, title.id]); } } - private segmentBoxDistance([start, end]: Seg, box: Box): number { - if (ptIn(start, box) || ptIn(end, box) || cuts([start, end], box, 0)) return 0; + private segmentBoxDistance([start, end]: Segment, box: Box): number { + if (containsPoint(box, start) || containsPoint(box, end) || segmentHits(start, end, box, 0)) return 0; const pointBox = (point: Point): number => Math.hypot(Math.max(box.x - point.x, 0, point.x - box.x - box.width), Math.max(box.y - point.y, 0, point.y - box.y - box.height)); const pointSeg = (point: Point): number => { const dx = end.x - start.x; diff --git a/src/domain/diagram/di/ProblemLocator.ts b/src/domain/diagram/di/ProblemLocator.ts index 134f6386..6d65384a 100644 --- a/src/domain/diagram/di/ProblemLocator.ts +++ b/src/domain/diagram/di/ProblemLocator.ts @@ -1,7 +1,8 @@ import { is, type El } from '../../document/utils/moddle.js'; import type { Box, Point } from './types.js'; -import { cx, cy, segmentsCross } from '../utils/geom.js'; -import { collinear, cuts, drawnEnds, glyph, hits, joinTail, ptIn, readDrawnPlane, segs, subTitleOf, type Edge, type Seg, type Shape } from '../utils/harness.js'; +import { containsPoint, cx, cy, segmentHits, segmentsCross } from '../utils/geom.js'; +import { toSegments, type Segment } from '../utils/geometry.js'; +import { collinear, drawnEnds, glyph, hits, joinTail, readDrawnPlane, subTitleOf, type Edge, type Shape } from '../utils/harness.js'; import { idOf, raw, rawList } from '../utils/raw.js'; import type { LayoutProblem } from '../utils/problems.js'; @@ -56,14 +57,14 @@ export class ProblemLocator { return width >= 0 && height >= 0 ? { x: left + width / 2, y: top + height / 2 } : this.centreOf(first); } - private crossPoint([firstStart, firstEnd]: Seg, [secondStart, secondEnd]: Seg): Point { + private crossPoint([firstStart, firstEnd]: Segment, [secondStart, secondEnd]: Segment): Point { const den = (firstEnd.x - firstStart.x) * (secondEnd.y - secondStart.y) - (firstEnd.y - firstStart.y) * (secondEnd.x - secondStart.x); const progress = den ? ((secondStart.x - firstStart.x) * (secondEnd.y - secondStart.y) - (secondStart.y - firstStart.y) * (secondEnd.x - secondStart.x)) / den : 0.5; return { x: firstStart.x + progress * (firstEnd.x - firstStart.x), y: firstStart.y + progress * (firstEnd.y - firstStart.y) }; } /** the middle of the part of a segment inside the box shrunk by `shrink` */ - private clipMiddle(seg: Seg, box: Box, shrink: number): Point { + private clipMiddle(seg: Segment, box: Box, shrink: number): Point { const [segmentStart, segmentEnd] = seg; const dx = segmentEnd.x - segmentStart.x; const dy = segmentEnd.y - segmentStart.y; @@ -99,7 +100,7 @@ export class ProblemLocator { } private halfway(pts: Point[]): Point | undefined { - const parts = segs(pts); + const parts = toSegments(pts); let rest = parts.reduce((sum, [start, end]) => sum + Math.hypot(end.x - start.x, end.y - start.y), 0) / 2; for (const [start, end] of parts) { const len = Math.hypot(end.x - start.x, end.y - start.y); @@ -134,7 +135,7 @@ export class ProblemLocator { case 'outsideSub': return sa ? this.centreOf(sa.bounds) : undefined; case 'through': { - const seg = sb ? ea?.segs.find((segment) => cuts(segment, sb.bounds)) : undefined; + const seg = sb ? ea?.segs.find((segment) => segmentHits(...segment, sb.bounds)) : undefined; return seg && sb ? this.clipMiddle(seg, sb.bounds, 2) : undefined; } case 'msgThroughPool': { @@ -171,7 +172,7 @@ export class ProblemLocator { return ea && eb ? this.sharedAt(ea, eb, joinTail(ea, eb, pl.shapes.get(problem.ids[2] ?? ''))) : undefined; case 'edgeLeavesLane': case 'edgeOutside': - return sb ? ea?.pts.find((point) => !ptIn(point, sb.bounds)) : undefined; + return sb ? ea?.pts.find((point) => !containsPoint(sb.bounds, point)) : undefined; case 'laneGap': if (!sa || !sb) return undefined; return problem.detail === 'bottom' ? { x: cx(sa.bounds), y: (sa.bounds.y + sa.bounds.height + sb.bounds.y + sb.bounds.height) / 2 } : { x: cx(sa.bounds), y: (sa.bounds.y + sb.bounds.y) / 2 }; diff --git a/src/domain/diagram/layout/clean/LabelDeclutter.ts b/src/domain/diagram/layout/clean/LabelDeclutter.ts index c0815b42..1a94d989 100644 --- a/src/domain/diagram/layout/clean/LabelDeclutter.ts +++ b/src/domain/diagram/layout/clean/LabelDeclutter.ts @@ -1,5 +1,5 @@ import { titleBox } from '../../utils/text.js'; -import { boxesOverlap } from '../../utils/geom.js'; +import { overlaps } from '../../utils/geom.js'; import type { Box, Point } from '../../di/types.js'; import { is, many, type El } from '../../../document/utils/moddle.js'; import { toSegments, type Segment } from '../../utils/geometry.js'; @@ -151,10 +151,10 @@ export class LabelDeclutter { /** Whether `box` hits a shape, a placed label, a title or a line (the text would sit on it). */ private clashes(box: Box, own?: Box): boolean { return ( - this.shapes.some((shape) => shape !== own && boxesOverlap(shape, box, 2)) || - this.placed.some((other) => boxesOverlap(other, box, 2)) || + this.shapes.some((shape) => shape !== own && overlaps(shape, box, 2)) || + this.placed.some((other) => overlaps(other, box, 2)) || this.lines.some((seg) => this.hitsBox(seg, box, seg[0].y === seg[1].y ? -RUN_KEEP : 1)) || - this.titleBoxes.some((title) => boxesOverlap(title, box, 2)) + this.titleBoxes.some((title) => overlaps(title, box, 2)) ); } } diff --git a/src/domain/diagram/layout/clean/MessageFlowRouter.ts b/src/domain/diagram/layout/clean/MessageFlowRouter.ts index a1f714ac..96927807 100644 --- a/src/domain/diagram/layout/clean/MessageFlowRouter.ts +++ b/src/domain/diagram/layout/clean/MessageFlowRouter.ts @@ -2,7 +2,8 @@ import { is, type El } from '../../../document/utils/moddle.js'; import { SIZES } from '../../utils/sizes.js'; import { center } from '../../utils/geom.js'; import type { Box, Point } from '../../di/types.js'; -import { crossings, properCross, toSegments, type Segment } from '../../utils/geometry.js'; +import { properCross, toSegments, type Segment } from '../../utils/geometry.js'; +import { crossings } from '../../utils/routing.js'; export interface MessagePool { box: Box; diff --git a/src/domain/diagram/layout/clean/NotePlacer.ts b/src/domain/diagram/layout/clean/NotePlacer.ts index 5f9285e8..a51f4b25 100644 --- a/src/domain/diagram/layout/clean/NotePlacer.ts +++ b/src/domain/diagram/layout/clean/NotePlacer.ts @@ -1,4 +1,5 @@ import type { Box } from '../../di/types.js'; +import { overlaps } from '../../utils/geom.js'; import type { Segment } from '../../utils/geometry.js'; export interface NoteJob { @@ -39,8 +40,8 @@ export class NotePlacer { let fallbackY = top; const free = (job: NoteJob, box: Box): boolean => { if (box.x < 0 || box.y < 0) return false; - if (job.pool ? !this.inside(box, job.pool) : world.pools.some((pool) => this.overlaps(box, pool, 0))) return false; - if ([...world.obstacles, ...taken].some((obstacle) => this.overlaps(box, obstacle, MARGIN))) return false; + if (job.pool ? !this.inside(box, job.pool) : world.pools.some((pool) => overlaps(box, pool, 0))) return false; + if ([...world.obstacles, ...taken].some((obstacle) => overlaps(box, obstacle, MARGIN))) return false; return !world.lines.some((line) => this.lineThrough(line, box)); }; return jobs.map((job) => { @@ -56,10 +57,6 @@ export class NotePlacer { }); } - private overlaps(first: Box, second: Box, margin: number): boolean { - return first.x < second.x + second.width + margin && first.x + first.width + margin > second.x && first.y < second.y + second.height + margin && first.y + first.height + margin > second.y; - } - private inside(inner: Box, outer: Box): boolean { return inner.x >= outer.x && inner.y >= outer.y && inner.x + inner.width <= outer.x + outer.width && inner.y + inner.height <= outer.y + outer.height; } diff --git a/src/domain/diagram/layout/clean/ScopeArtifactPlacer.ts b/src/domain/diagram/layout/clean/ScopeArtifactPlacer.ts index a36e08f4..1e090dd3 100644 --- a/src/domain/diagram/layout/clean/ScopeArtifactPlacer.ts +++ b/src/domain/diagram/layout/clean/ScopeArtifactPlacer.ts @@ -1,6 +1,6 @@ import { is, many, type El, idOf } from '../../../document/utils/moddle.js'; import { METRICS, SIZES, annotationSize } from '../../utils/sizes.js'; -import { boxesOverlap } from '../../utils/geom.js'; +import { overlaps } from '../../utils/geom.js'; import type { Box, GArtifact, GEdge, GNode, LinkEnd, Point, ScopeLayout } from '../../di/types.js'; import { ChannelRouter } from './ChannelRouter.js'; import { ScopeLabels } from './ScopeLabels.js'; @@ -98,7 +98,7 @@ export class ScopeArtifactPlacer { placeAttached(): void { const { layout } = this; const taken: Box[] = []; - const free = (box: Box): boolean => !taken.some((takenBox) => boxesOverlap(takenBox, box, 10)); + const free = (box: Box): boolean => !taken.some((takenBox) => overlaps(takenBox, box, 10)); for (const artifact of layout.artifacts) { const node = artifact.anchorNode; if (!node) continue; @@ -147,7 +147,7 @@ export class ScopeArtifactPlacer { for (const edge of layout.edges) if (edge.label) boxes.push(edge.label); const segments = routedSegments(layout); const collides = (box: Box): boolean => - boxes.some((obstacle) => boxesOverlap(obstacle, box, 6)) || + boxes.some((obstacle) => overlaps(obstacle, box, 6)) || segments.some(([start, end]) => Math.min(start.x, end.x) < box.x + box.width + 4 && Math.max(start.x, end.x) > box.x - 4 && Math.min(start.y, end.y) < box.y + box.height + 4 && Math.max(start.y, end.y) > box.y - 4); const settle = (note: GArtifact, cx: number, top: number, check: (box: Box) => boolean): Box | undefined => { for (let step = 0; step <= 8; step++) { @@ -258,7 +258,7 @@ export class ScopeArtifactPlacer { }; if (!blockers.length) return fallback(); const others = nodes.filter((box) => box !== host.box && box !== event.box); - const clear = (box: Box): boolean => free(box) && !blockers.some((blocker) => boxesOverlap(blocker, box, 4)) && !others.some((blocker) => boxesOverlap(blocker, box, 8)); + const clear = (box: Box): boolean => free(box) && !blockers.some((blocker) => overlaps(blocker, box, 4)) && !others.some((blocker) => overlaps(blocker, box, 8)); const label = ScopeLabels.of(event, 'boundary'); const right = { x: event.box.x + event.box.width + 8, y: Math.max(y0, label ? label.y + label.height + 6 : y0) }; const far = Math.min(near, Math.min(...blockers.map((blocker) => blocker.x)) - 8 - note.width); diff --git a/src/domain/diagram/layout/clean/ScopeLabels.ts b/src/domain/diagram/layout/clean/ScopeLabels.ts index c3800390..71f864bf 100644 --- a/src/domain/diagram/layout/clean/ScopeLabels.ts +++ b/src/domain/diagram/layout/clean/ScopeLabels.ts @@ -1,5 +1,5 @@ import { LABEL_LINE, labelSize } from '../../utils/text.js'; -import { boxesOverlap } from '../../utils/geom.js'; +import { overlaps } from '../../utils/geom.js'; import type { Box, GArtifact, GNode, ScopeLayout } from '../../di/types.js'; import { bottom } from '../../utils/geom.js'; import { nameOf } from '../../utils/names.js'; @@ -56,7 +56,7 @@ export class ScopeLabels { const label = artifact.kind === 'annotation' ? undefined : ScopeLabels.of(artifact, 'data'); if (!label) continue; const spots = [...[0, -0.6, 0.6, -1.2, 1.2].map((dx) => ({ ...label, x: Math.round(label.x + dx * label.width) })), ...[1, 2, 3].map((step) => ({ ...label, y: label.y + step * (label.height + 4) }))]; - out.push([spots.find((spot) => !out.some(([placed]) => boxesOverlap(placed, spot, 2))) ?? label, artifact]); + out.push([spots.find((spot) => !out.some(([placed]) => overlaps(placed, spot, 2))) ?? label, artifact]); } return out; } diff --git a/src/domain/diagram/layout/clean/TopBorderTrial.ts b/src/domain/diagram/layout/clean/TopBorderTrial.ts index d2c94c57..381e124d 100644 --- a/src/domain/diagram/layout/clean/TopBorderTrial.ts +++ b/src/domain/diagram/layout/clean/TopBorderTrial.ts @@ -1,6 +1,6 @@ import { LayoutDebug } from '../LayoutDebug.js'; import { titleBox } from '../../utils/text.js'; -import { boxesOverlap, center, equalBoxes, segmentHitsBox } from '../../utils/geom.js'; +import { center, equalBoxes, overlaps, segmentHitsBox } from '../../utils/geom.js'; import type { Box, GEdge, GNode, LaneBandInfo, LinkEnd, Point } from '../../di/types.js'; import { is, many, type El, idOf } from '../../../document/utils/moddle.js'; import { ArrangementCheckpoint } from './ArrangementCheckpoint.js'; @@ -64,9 +64,9 @@ export class TopBorderTrial extends ArrangementTrial { const box = !segments.some(([edge, pieces]) => (!edge || !mine.has(edge)) && pieces.some(([start, end]) => segmentHitsBox(start, end, boundaryEvent.box))); const clear = !label || - (!shapes.some((shape) => shape !== boundaryEvent.box && boxesOverlap(shape, label, 2)) && - !labels.some((otherLabel) => !equalBoxes(otherLabel, label) && boxesOverlap(otherLabel, label, 2)) && - !flowLabels.some((flowLabel) => boxesOverlap(flowLabel, label, 2)) && + (!shapes.some((shape) => shape !== boundaryEvent.box && overlaps(shape, label, 2)) && + !labels.some((otherLabel) => !equalBoxes(otherLabel, label) && overlaps(otherLabel, label, 2)) && + !flowLabels.some((flowLabel) => overlaps(flowLabel, label, 2)) && !segments.some(([, pieces]) => pieces.some(([start, end]) => segmentHitsBox(start, end, label)))); ways.set(boundaryEvent, { length, bends, up, box, label: clear, worth: false }); continue; @@ -88,7 +88,7 @@ export class TopBorderTrial extends ArrangementTrial { boundaryEvent.onTop = true; ScopeGrid.placeBoundaryEvents(host); const title = host.child ? titleBox(host.box, nameOf(host)) : undefined; - if (title && boxesOverlap(title, boundaryEvent.box, 2)) { + if (title && overlaps(title, boundaryEvent.box, 2)) { if (LayoutDebug.on()) LayoutDebug.log(`[boundary] ${boundaryEvent.id} stays on the bottom border: the title of ${host.id} is in the way`); checkpoint.restore(); continue; diff --git a/src/domain/diagram/layout/incremental/Drawing.ts b/src/domain/diagram/layout/incremental/Drawing.ts index c66e7130..76193b86 100644 --- a/src/domain/diagram/layout/incremental/Drawing.ts +++ b/src/domain/diagram/layout/incremental/Drawing.ts @@ -9,7 +9,7 @@ import type { DrawingSnapshot } from './DrawingSnapshot.js'; import type { TrialGuard } from '../../di/TrialGuard.js'; import type { Box, DEdge, DShape, Plane, Point, Side } from '../../di/types.js'; import type { Semantics } from '../../di/Semantics.js'; -import { segmentHits } from '../../utils/geom.js'; +import { segmentHits, translateEdge } from '../../utils/geom.js'; import { hasExternalLabel, labelSideOf, type LabelSide } from '../../utils/labels.js'; import { raw } from '../../utils/raw.js'; @@ -165,14 +165,7 @@ export class Drawing { const sourceDelta = edge.sourceId ? delta.get(edge.sourceId) : undefined; const targetDelta = edge.targetId ? delta.get(edge.targetId) : undefined; if (!sourceDelta || !targetDelta || sourceDelta.dx !== targetDelta.dx || sourceDelta.dy !== targetDelta.dy) continue; - for (const point of edge.points) { - point.x += sourceDelta.dx; - point.y += sourceDelta.dy; - } - if (edge.label) { - edge.label.x += sourceDelta.dx; - edge.label.y += sourceDelta.dy; - } + translateEdge(edge, sourceDelta.dx, sourceDelta.dy); } const rerouted: DEdge[] = []; for (const edge of this.routeOrder(this.plane.edges.values())) { diff --git a/src/domain/diagram/layout/incremental/FrameEditor.ts b/src/domain/diagram/layout/incremental/FrameEditor.ts index 920f33ff..f0228733 100644 --- a/src/domain/diagram/layout/incremental/FrameEditor.ts +++ b/src/domain/diagram/layout/incremental/FrameEditor.ts @@ -1,5 +1,5 @@ import { SIZES } from '../../utils/sizes.js'; -import { bottom, type Box, copyBox, cx, cy, median, right, union } from '../../utils/geom.js'; +import { bottom, type Box, copyBox, cx, cy, median, right, translateEdge, translateShape, union } from '../../utils/geom.js'; import { PlaneQuery } from '../../di/PlaneQuery.js'; import type { DEdge, DShape, Plane } from '../../di/types.js'; import type { Semantics } from '../../di/Semantics.js'; @@ -142,24 +142,8 @@ export class FrameEditor { } private shiftAll(shapes: DShape[], edges: DEdge[], dx: number, dy: number): void { - for (const shape of shapes) { - shape.bounds.x += dx; - shape.bounds.y += dy; - if (shape.label) { - shape.label.x += dx; - shape.label.y += dy; - } - } - for (const edge of edges) { - for (const point of edge.points) { - point.x += dx; - point.y += dy; - } - if (edge.label) { - edge.label.x += dx; - edge.label.y += dy; - } - } + for (const shape of shapes) translateShape(shape, dx, dy); + for (const edge of edges) translateEdge(edge, dx, dy); } /** diff --git a/src/domain/diagram/layout/incremental/NodePlacer.ts b/src/domain/diagram/layout/incremental/NodePlacer.ts index 0916a2f7..967914b6 100644 --- a/src/domain/diagram/layout/incremental/NodePlacer.ts +++ b/src/domain/diagram/layout/incremental/NodePlacer.ts @@ -1,7 +1,7 @@ import { LayoutDebug } from '../LayoutDebug.js'; import { annotationSize, SIZES } from '../../utils/sizes.js'; import { is, type El } from '../../../document/utils/moddle.js'; -import { bottom, clamp, cx, cy, median, right, type Box } from '../../utils/geom.js'; +import { bottom, clamp, cx, cy, median, right, translateEdge, translateShape, type Box } from '../../utils/geom.js'; import { defaultLabelSide, hasExternalLabel, labelAt } from '../../utils/labels.js'; import { idOf, raw } from '../../utils/raw.js'; import { DiagramReader } from '../../di/DiagramReader.js'; @@ -40,19 +40,7 @@ export interface PlacementHooks { } /** What a placement run changes in its placer, to undo it. */ -export interface PlacerState { - placed: Set; - moved: Set; - spacing: Map; - reuse: Map; - blockSize: Map; - spliced?: Map; - joins?: Map; - relabel?: Set; - heads?: Map; - skip?: Map; - continued?: Set; -} +export type PlacerState = Pick; export interface Probe { id: string; @@ -252,14 +240,7 @@ export class NodePlacer { const inner = Frames.interior(frame, 15); const dx = Math.max(0, Math.round(inner.x - shape.bounds.x)); const dy = Math.max(0, Math.round(inner.y - shape.bounds.y)); - if (dx || dy) { - shape.bounds.x += dx; - shape.bounds.y += dy; - if (shape.label) { - shape.label.x += dx; - shape.label.y += dy; - } - } + if (dx || dy) translateShape(shape, dx, dy); } plane.shapes.set(shape.id, shape); this.placed.add(shape.id); @@ -269,26 +250,14 @@ export class NodePlacer { const dx = shape.bounds.x - old.x; const dy = shape.bounds.y - old.y; const translate = (moving: DShape): void => { - moving.bounds.x += dx; - moving.bounds.y += dy; - if (moving.label) { - moving.label.x += dx; - moving.label.y += dy; - } + translateShape(moving, dx, dy); plane.shapes.set(moving.id, moving); this.moved.add(moving.id); }; reuse.boundaries.forEach(translate); for (const child of reuse.content?.shapes ?? []) translate(child); for (const edge of reuse.content?.edges ?? []) { - for (const point of edge.points) { - point.x += dx; - point.y += dy; - } - if (edge.label) { - edge.label.x += dx; - edge.label.y += dy; - } + translateEdge(edge, dx, dy); if (reuse.from !== plane) { reuse.from.edges.delete(edge.id); plane.edges.set(edge.id, edge); diff --git a/src/domain/diagram/layout/incremental/OrthogonalRouter.ts b/src/domain/diagram/layout/incremental/OrthogonalRouter.ts index b547f44a..75ce5d2c 100644 --- a/src/domain/diagram/layout/incremental/OrthogonalRouter.ts +++ b/src/domain/diagram/layout/incremental/OrthogonalRouter.ts @@ -1,9 +1,9 @@ -import { bottom, cy, equalBoxes, grow, right, segmentHitsBox, union } from '../../utils/geom.js'; +import { bottom, cy, equalBoxes, grow, right, segmentHitsBox, segmentsCross, union } from '../../utils/geom.js'; import type { Box, Point, Side } from '../../di/types.js'; import { SOURCE_DEFAULT, TARGET_DEFAULT } from '../../utils/sides.js'; import { RouteGrid } from './RouteGrid.js'; import { SearchHeap } from './SearchHeap.js'; -import { AVOID, BEND, CROSS, DX, DY, EPS, HIT, MIN_STUB, NEAR, OUTWARD, OVERLAP, OVERLAP_TOL, PORT_OFFSET, PORT_SLOPE, REGION_PAD, SRC_RANK, TGT_RANK, THIN_MARGIN, pointOn, segmentsCross, simplify, span, type Port, type Seg } from '../../utils/routing.js'; +import { AVOID, BEND, CROSS, DX, DY, EPS, HIT, MIN_STUB, NEAR, OUTWARD, OVERLAP, OVERLAP_TOL, PORT_OFFSET, PORT_SLOPE, REGION_PAD, SRC_RANK, TGT_RANK, THIN_MARGIN, pointOn, simplify, span, type Port, type Seg } from '../../utils/routing.js'; export interface RouteRequest { source: Box; diff --git a/src/domain/diagram/layout/incremental/SpaceTool.ts b/src/domain/diagram/layout/incremental/SpaceTool.ts index 1b4a3e08..3ae78651 100644 --- a/src/domain/diagram/layout/incremental/SpaceTool.ts +++ b/src/domain/diagram/layout/incremental/SpaceTool.ts @@ -1,5 +1,5 @@ import { LayoutDebug } from '../LayoutDebug.js'; -import { bottom, type Box, containsPoint, cx, cy, overlaps, right } from '../../utils/geom.js'; +import { bottom, type Box, containsPoint, cx, cy, overlaps, right, translateEdge, translateShape } from '../../utils/geom.js'; import { Frames } from '../../di/Frames.js'; import { PlaneQuery } from '../../di/PlaneQuery.js'; import { defaultLabelSide, hasExternalLabel, labelAt } from '../../utils/labels.js'; @@ -76,14 +76,7 @@ export class SpaceTool { const shape = this.plane.shapes.get(id); if (!shape || (!dx && !dy)) return []; const moved = [id]; - const move = (moving: DShape): void => { - moving.bounds.x += dx; - moving.bounds.y += dy; - if (moving.label) { - moving.label.x += dx; - moving.label.y += dy; - } - }; + const move = (moving: DShape): void => translateShape(moving, dx, dy); move(shape); for (const boundary of PlaneQuery.boundariesOf(this.plane, id)) { move(boundary); @@ -98,14 +91,7 @@ export class SpaceTool { const inside = new Set(shapes.map((child) => child.id)); for (const edge of edges) { if (!(edge.sourceId && inside.has(edge.sourceId)) || !(edge.targetId && inside.has(edge.targetId))) continue; - for (const point of edge.points) { - point.x += dx; - point.y += dy; - } - if (edge.label) { - edge.label.x += dx; - edge.label.y += dy; - } + translateEdge(edge, dx, dy); } } return moved; diff --git a/src/domain/diagram/utils/geom.ts b/src/domain/diagram/utils/geom.ts index b9dc8848..84c2dc0b 100644 --- a/src/domain/diagram/utils/geom.ts +++ b/src/domain/diagram/utils/geom.ts @@ -7,10 +7,6 @@ export function center(box: Box): Point { return { x: box.x + box.width / 2, y: box.y + box.height / 2 }; } -export function boxesOverlap(first: Box, second: Box, margin = 0): boolean { - return first.x < second.x + second.width + margin && first.x + first.width + margin > second.x && first.y < second.y + second.height + margin && first.y + first.height + margin > second.y; -} - export const right = (box: Box): number => box.x + box.width; export const bottom = (box: Box): number => box.y + box.height; export const cx = (box: Box): number => box.x + box.width / 2; @@ -62,6 +58,25 @@ export function unionAll(boxes: readonly Box[]): Box | undefined { return out; } +export function translate(at: Point, dx: number, dy: number): void { + at.x += dx; + at.y += dy; +} + +export function translatePoints(points: readonly Point[], dx: number, dy: number): void { + for (const point of points) translate(point, dx, dy); +} + +export function translateShape(shape: { bounds: Box; label?: Box | undefined }, dx: number, dy: number): void { + translate(shape.bounds, dx, dy); + if (shape.label) translate(shape.label, dx, dy); +} + +export function translateEdge(edge: { points: Point[]; label?: Box | undefined }, dx: number, dy: number): void { + translatePoints(edge.points, dx, dy); + if (edge.label) translate(edge.label, dx, dy); +} + export function grow(box: Box, by: number): Box { return { x: box.x - by, y: box.y - by, width: box.width + 2 * by, height: box.height + 2 * by }; } diff --git a/src/domain/diagram/utils/geometry.ts b/src/domain/diagram/utils/geometry.ts index a95a033b..001f62f3 100644 --- a/src/domain/diagram/utils/geometry.ts +++ b/src/domain/diagram/utils/geometry.ts @@ -47,9 +47,3 @@ export function pathHits(points: Point[], boxes: Box[] | XIndex, ignore: Bo export function properCross([firstStart, firstEnd]: Segment, [secondStart, secondEnd]: Segment): boolean { return segmentsCross(firstStart, firstEnd, secondStart, secondEnd); } - -export function crossings(points: Point[], lines: Segment[]): number { - let count = 0; - for (const segment of toSegments(points)) for (const line of lines) if (properCross(segment, line)) count++; - return count; -} diff --git a/src/domain/diagram/utils/harness.ts b/src/domain/diagram/utils/harness.ts index acd42ddc..3091dce8 100644 --- a/src/domain/diagram/utils/harness.ts +++ b/src/domain/diagram/utils/harness.ts @@ -7,51 +7,18 @@ import { is, type El } from '../../document/utils/moddle.js'; import type { Box, Point } from '../di/types.js'; import { idOf, raw, rawList } from './raw.js'; import { titleBox } from './text.js'; +import { toSegments, type Segment } from './geometry.js'; -export type Seg = [Point, Point]; - -export function segs(pts: Point[]): Seg[] { - const out: Seg[] = []; - for (let index = 0; index + 1 < pts.length; index++) out.push([pts[index]!, pts[index + 1]!]); - return out; -} - -export const hits = ([start, end]: Seg, box: Box, shrink = 2): boolean => +export const hits = ([start, end]: Segment, box: Box, shrink = 2): boolean => Math.min(start.x, end.x) < box.x + box.width - shrink && Math.max(start.x, end.x) > box.x + shrink && Math.min(start.y, end.y) < box.y + box.height - shrink && Math.max(start.y, end.y) > box.y + shrink; -function slab(origin: number, direction: number, lo: number, hi: number): [number, number] { - const lowExit = (lo - origin) / direction; - const highExit = (hi - origin) / direction; - return lowExit < highExit ? [lowExit, highExit] : [highExit, lowExit]; -} - -/** Liang-Barsky on the open box; for axis-parallel segments the same as `hits`. */ -export function cuts(seg: Seg, box: Box, shrink = 2): boolean { - const [start, end] = seg; - if (start.x === end.x || start.y === end.y) return hits(seg, box, shrink); - const x0 = box.x + shrink; - const x1 = box.x + box.width - shrink; - const y0 = box.y + shrink; - const y1 = box.y + box.height - shrink; - if (x0 >= x1 || y0 >= y1) return false; - const [ax, bx] = slab(start.x, end.x - start.x, x0, x1); - const [ay, by] = slab(start.y, end.y - start.y, y0, y1); - const lo = Math.max(ax, ay); - const hi = Math.min(bx, by); - return lo < hi && lo < 1 && hi > 0; -} - -export const overlap = (first: Box, second: Box): boolean => first.x < second.x + second.width && second.x < first.x + first.width && first.y < second.y + second.height && second.y < first.y + first.height; - export const inside = (inner: Box, outer: Box): boolean => inner.x >= outer.x && inner.y >= outer.y && inner.x + inner.width <= outer.x + outer.width && inner.y + inner.height <= outer.y + outer.height; -export const ptIn = (point: Point, box: Box): boolean => point.x >= box.x && point.x <= box.x + box.width && point.y >= box.y && point.y <= box.y + box.height; - /** ≤ 0: none */ -export function collinear([firstStart, firstEnd]: Seg, [secondStart, secondEnd]: Seg): number { +export function collinear([firstStart, firstEnd]: Segment, [secondStart, secondEnd]: Segment): number { if (firstStart.x === firstEnd.x && secondStart.x === secondEnd.x && firstStart.x === secondStart.x) { return Math.min(Math.max(firstStart.y, firstEnd.y), Math.max(secondStart.y, secondEnd.y)) - Math.max(Math.min(firstStart.y, firstEnd.y), Math.min(secondStart.y, secondEnd.y)); } @@ -109,7 +76,7 @@ export interface Edge { id: string; el: El; pts: Point[]; - segs: Seg[]; + segs: Segment[]; /** `undefined` is an unresolved end and makes edges related, as in the harness */ ends: Set; label?: Box; @@ -141,7 +108,7 @@ export function readDrawnPlane(plane: unknown, seen: Set): { shapes: Sha shapes.push({ id, el, bounds, container, expanded, ...(label ? { label } : {}) }); } else if (is(pe, 'bpmndi:BPMNEdge')) { const pts = rawList(pe, 'waypoint').map((waypoint) => ({ x: raw(waypoint, 'x')!, y: raw(waypoint, 'y')! })); - edges.push({ id, el, pts, segs: segs(pts), ends: endsOf(el), ...(label ? { label } : {}) }); + edges.push({ id, el, pts, segs: toSegments(pts), ends: endsOf(el), ...(label ? { label } : {}) }); } } return { shapes, edges }; @@ -155,14 +122,14 @@ export function drawnEnds(el: El): [string | undefined, string | undefined] { } interface Run { - seg: Seg; + seg: Segment; idx: number[]; } /** A zero-length segment joins the run before it; leading ones join the first run. */ -function runs(list: Seg[]): Run[] { +function runs(list: Segment[]): Run[] { const out: Run[] = []; - const way = ([start, end]: Seg): string => `${Math.sign(end.x - start.x)},${Math.sign(end.y - start.y)}`; + const way = ([start, end]: Segment): string => `${Math.sign(end.x - start.x)},${Math.sign(end.y - start.y)}`; const lead: number[] = []; list.forEach((segment, segmentIndex) => { const last = out[out.length - 1]; diff --git a/src/domain/diagram/utils/routing.ts b/src/domain/diagram/utils/routing.ts index 0eca2771..a4fba0ac 100644 --- a/src/domain/diagram/utils/routing.ts +++ b/src/domain/diagram/utils/routing.ts @@ -1,6 +1,6 @@ /** Costs, directions and segment helpers the orthogonal router and its grid share. */ import type { Box, Point, Side } from '../di/types.js'; -import { bottom, right } from './geom.js'; +import { bottom, right, segmentsCross } from './geom.js'; export const BEND = 150; /** a port on a side another connection of the shape takes */ @@ -28,15 +28,6 @@ export const DX = [1, 0, -1, 0]; export const DY = [0, 1, 0, -1]; export const OUTWARD: Record = { right: 0, bottom: 1, left: 2, top: 3 }; -function orient(first: Point, second: Point, third: Point): number { - return Math.sign((second.x - first.x) * (third.y - first.y) - (second.y - first.y) * (third.x - first.x)); -} - -export function segmentsCross(firstStart: Point, firstEnd: Point, secondStart: Point, secondEnd: Point): boolean { - const o1 = orient(firstStart, firstEnd, secondStart), o2 = orient(firstStart, firstEnd, secondEnd), o3 = orient(secondStart, secondEnd, firstStart), o4 = orient(secondStart, secondEnd, firstEnd); - return o1 * o2 < 0 && o3 * o4 < 0; -} - /** The router's crossing cost uses the same test. */ export function crossings(points: readonly Point[], lines: readonly Point[][]): number { let crossings = 0; diff --git a/src/domain/diagram/utils/scope.ts b/src/domain/diagram/utils/scope.ts index c14b5338..e4fbd336 100644 --- a/src/domain/diagram/utils/scope.ts +++ b/src/domain/diagram/utils/scope.ts @@ -1,5 +1,6 @@ import type { Box, GArtifact, GNode, LinkEnd, ScopeLayout } from '../di/types.js'; import { toSegments, type Segment } from './geometry.js'; +import { translate, translateEdge, translatePoints } from './geom.js'; /** data objects and notes of boundary events are placed before routing */ export function routedArtifacts(layout: ScopeLayout): GArtifact[] { @@ -40,27 +41,19 @@ export function growToFit(layout: ScopeLayout): void { /** Shifts the whole scope; a flow anchor is the same object as the link end using it and moves once. */ export function shiftScope(layout: ScopeLayout, dx: number, dy: number): void { for (const node of layout.nodes) shiftNode(node, dx, dy); - for (const es of layout.eventSubs) { - es.box.x += dx; - es.box.y += dy; - } + for (const es of layout.eventSubs) translate(es.box, dx, dy); const movedEnds = new Set(); const moveEnd = (end: LinkEnd | undefined): void => { if (!end?.flow || movedEnds.has(end)) return; movedEnds.add(end); - end.box.x += dx; - end.box.y += dy; + translate(end.box, dx, dy); }; for (const artifact of layout.artifacts) { - artifact.box.x += dx; - artifact.box.y += dy; + translate(artifact.box, dx, dy); for (const link of artifact.links) { moveEnd(link.from); moveEnd(link.to); - for (const point of link.points) { - point.x += dx; - point.y += dy; - } + translatePoints(link.points, dx, dy); } moveEnd(artifact.anchor); } @@ -71,29 +64,13 @@ export function shiftScope(layout: ScopeLayout, dx: number, dy: number): void { function shiftEdges(layout: ScopeLayout, dx: number, dy: number): void { for (const edge of layout.edges) { - for (const point of edge.points) { - point.x += dx; - point.y += dy; - } - if (edge.label) { - edge.label.x += dx; - edge.label.y += dy; - } + translateEdge(edge, dx, dy); if (edge.bandY !== undefined) edge.bandY += dy; } - for (const compensation of layout.compensations) { - for (const point of compensation.points) { - point.x += dx; - point.y += dy; - } - } + for (const compensation of layout.compensations) translatePoints(compensation.points, dx, dy); } export function shiftNode(node: GNode, dx: number, dy: number): void { - node.box.x += dx; - node.box.y += dy; - for (const boundaryEvent of node.boundary) { - boundaryEvent.box.x += dx; - boundaryEvent.box.y += dy; - } + translate(node.box, dx, dy); + for (const boundaryEvent of node.boundary) translate(boundaryEvent.box, dx, dy); } From 9f7bb60a3849718ad3374ee6295d7691ccf45054 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:18:16 +0100 Subject: [PATCH 04/11] refactor: check typed split branches with the command parser and share one parent-slot helper --- src/domain/commands/CommandParser.ts | 10 +++-- src/domain/document/ids/ConventionsTable.ts | 6 +-- .../editing/ElementTypeChangeService.ts | 23 +---------- .../editing/flows/BranchSplitService.ts | 41 +------------------ src/domain/editing/utils/containment.ts | 17 ++++---- 5 files changed, 22 insertions(+), 75 deletions(-) diff --git a/src/domain/commands/CommandParser.ts b/src/domain/commands/CommandParser.ts index 804a753d..4b1b6d0f 100644 --- a/src/domain/commands/CommandParser.ts +++ b/src/domain/commands/CommandParser.ts @@ -152,7 +152,7 @@ export class CommandParser { out[key] = this.checkMap(ctx, key, raw); break; case 'branches': - out[key] = this.checkBranches(ctx, key, spec, raw); + out[key] = this.checkBranches(ctx, key, raw, spec.minItems); break; case 'nodes': out[key] = this.checkNodes(ctx, key, raw); @@ -182,9 +182,9 @@ export class CommandParser { }); } - private checkBranches(ctx: CommandLocation, key: string, spec: FieldSpec, value: unknown): SplitBranch[] { + private checkBranches(ctx: CommandLocation, key: string, value: unknown, minItems?: number): SplitBranch[] { if (!Array.isArray(value)) throw this.fail(ctx, `"${key}" must be an array of branch objects, got ${this.describe(value)}`, 'Write "branches": [{"flowName": "yes", "nodes": [{"kind": "userTask", "name": "..."}]}].'); - if (spec.minItems && value.length < spec.minItems) throw this.fail(ctx, `"${key}" needs at least ${spec.minItems} branch`); + if (minItems && value.length < minItems) throw this.fail(ctx, `"${key}" needs at least ${minItems} branch`); return value.map((branch, index) => { const bctx: CommandLocation = { index: ctx.index, path: `${ctx.path} ${key}[${index}]` }; const out = this.checkFields(bctx, branch, BRANCH_FIELDS); @@ -213,6 +213,10 @@ export class CommandParser { } /** Validates the ops JSON (an array or `{ ops: [...] }`) and returns typed ops. */ + parseBranches(branches: unknown): SplitBranch[] { + return this.checkBranches({ index: 0, path: 'split' }, 'branches', branches); + } + parse(input: unknown): EditCommand[] { let ops: unknown; if (Array.isArray(input)) ops = input; diff --git a/src/domain/document/ids/ConventionsTable.ts b/src/domain/document/ids/ConventionsTable.ts index 4f616658..68a4b768 100644 --- a/src/domain/document/ids/ConventionsTable.ts +++ b/src/domain/document/ids/ConventionsTable.ts @@ -2,7 +2,7 @@ import type { BpmnlintSeverity } from './bpmnlint-rule-setting.js'; import type { Doc } from '../Doc.js'; import { KINDS } from '../utils/kinds.js'; import type { ValidatorFinding } from '../validator-finding.js'; -import { IdStyle, type Body, type FlowForm, type PrefixSource } from './IdStyle.js'; +import { IdStyle, type FlowForm, type PrefixSource } from './IdStyle.js'; import type { IdConventions, QualifierMode } from './IdConventionsReader.js'; import { kindRequest, typeRequest, type IdRequest } from '../utils/requests.js'; import { UNQUALIFIED_NOTE } from '../utils/conventions-text.js'; @@ -41,8 +41,6 @@ export interface ConventionsView { notes: ValidatorFinding[]; } -const CASE_WORD: Record = { pascal: 'pascal', camel: 'camel', snake: 'snake', pascalSnake: 'pascalSnake' }; - const IGNORED_NOTE = 'checked; prefix ignored (W_CONFIG_IGNORED)'; /** @@ -96,7 +94,7 @@ export class ConventionsTable { private element(kind: string, type: string, req: IdRequest): void { const { head, source, body } = this.style.explain(req); const own = head.toLowerCase() === `${kind.toLowerCase()}_` ? (/^[a-z]/.test(head) ? '_' : '_') : head; - const tail = `{name:${CASE_WORD[body]}}`; + const tail = `{name:${body}}`; this.add(kind, `${head}${tail}`, `${own}${tail}`, source, source !== 'lint' ? this.unnamed(type) : this.unqualified(type)); } diff --git a/src/domain/editing/ElementTypeChangeService.ts b/src/domain/editing/ElementTypeChangeService.ts index c80bf325..78472fd8 100644 --- a/src/domain/editing/ElementTypeChangeService.ts +++ b/src/domain/editing/ElementTypeChangeService.ts @@ -17,6 +17,7 @@ import { ZEEBE, ZEEBE_URI } from '../document/vendor/ZeebeDescriptor.js'; import { SequenceFlowService } from './flows/SequenceFlowService.js'; import { RootElementService } from './events/RootElementService.js'; import { ElementRemovalService } from './ElementRemovalService.js'; +import { detachFromParent } from './utils/containment.js'; const ACTIVITY_FAMILIES = new Set(['task', 'subProcess', 'callActivity']); @@ -234,25 +235,6 @@ export class ElementTypeChangeService { } } - private static replaceInParent(parent: El, el: El, next: El): void { - for (const property of descriptorOf(parent).properties ?? []) { - if (property.isReference) continue; - const propValue = ownValue(parent, property.name); - if (Array.isArray(propValue)) { - const index = propValue.indexOf(el); - if (index !== -1) { - propValue[index] = next; - next.$parent = parent; - return; - } - } else if (propValue === el) { - parent.set(property.name, next); - next.$parent = parent; - return; - } - } - } - /** A kind prop is not copied over: applyKindProps sets it for the new kind. */ private static isReappliedKindProp(name: string): boolean { return name === 'triggeredByEvent'; @@ -274,8 +256,7 @@ export class ElementTypeChangeService { else this.dropProp(el, property, value, to, cs); } Object.assign(next.$attrs, el.$attrs); - const parent = el.$parent as El | undefined; - if (parent) ElementTypeChangeService.replaceInParent(parent, el, next); + detachFromParent(el, next); for (const ref of findReferences(this.doc.definitions, el)) { if (is(ref.holder, 'bpmndi:BPMNPlane') && !is(next, 'bpmn:SubProcess')) { this.dropDiagram(ref.holder.$parent as El | undefined); diff --git a/src/domain/editing/flows/BranchSplitService.ts b/src/domain/editing/flows/BranchSplitService.ts index f315c232..22481ea3 100644 --- a/src/domain/editing/flows/BranchSplitService.ts +++ b/src/domain/editing/flows/BranchSplitService.ts @@ -1,14 +1,13 @@ import type { Doc } from '../../document/Doc.js'; import type { EditContext } from '../../document/EditContext.js'; import { modelError, usageError } from '../../document/utils/errors.js'; -import type { CliError } from '../../document/CliError.js'; import { gatewayKindList, parseKind } from '../../document/utils/kinds.js'; import { KindError } from '../../document/KindError.js'; import { is, type El, idOf } from '../../document/utils/moddle.js'; import { ChangeSet } from '../../document/ChangeSet.js'; import { IdStyle } from '../../document/ids/IdStyle.js'; import { ElementLabels } from '../../document/ids/ElementLabels.js'; -import { BRANCH_FIELDS, NODE_FIELDS } from '../../commands/utils/op-fields.js'; +import { CommandParser } from '../../commands/CommandParser.js'; import { laneOf } from '../../document/utils/lanes.js'; import { renamedNote } from '../../document/utils/renames.js'; import { insertAfterInScope } from '../utils/declaration-order.js'; @@ -73,42 +72,6 @@ export class BranchSplitService { return { id, base }; } - private static describe(input: unknown): string { - if (input === null) return 'null'; - if (Array.isArray(input)) return 'an array'; - if (typeof input === 'object') return 'an object'; - return `${typeof input} ${JSON.stringify(input)}`; - } - - private static isObject(candidate: unknown): candidate is Record { - return typeof candidate === 'object' && candidate !== null && !Array.isArray(candidate); - } - - /** - * The shape parseOps checks, in its words, for typed library ops that skip it. - * A branch without `nodes` is no empty branch: the CLI refuses it too. - */ - private static checkShape(branches: unknown): void { - if (branches === undefined || branches === null) return; - if (!Array.isArray(branches)) { - throw usageError(`split: "branches" must be an array of branch objects, got ${BranchSplitService.describe(branches)}`, { hint: 'Write "branches": [{"flowName": "yes", "nodes": [{"kind": "userTask", "name": "..."}]}].' }); - } - const missing = (path: string, key: string, description: string): CliError => usageError(`${path}: missing required key "${key}" (${description})`); - branches.forEach((branch: unknown, branchIndex) => { - if (!BranchSplitService.isObject(branch)) throw usageError(`split branches[${branchIndex}]: expected an object, got ${BranchSplitService.describe(branch)}`); - const nodes = branch['nodes']; - if (nodes === undefined || nodes === null) throw missing(`split branches[${branchIndex}]`, 'nodes', BRANCH_FIELDS.nodes.description); - if (!Array.isArray(nodes)) throw usageError(`split branches[${branchIndex}]: "nodes" must be an array of node objects, got ${BranchSplitService.describe(nodes)}`, { hint: 'An empty array is allowed: the branch is then a direct gateway -> join flow.' }); - nodes.forEach((node: unknown, nodeIndex) => { - const path = `split branches[${branchIndex}].nodes[${nodeIndex}]`; - if (!BranchSplitService.isObject(node)) throw usageError(`${path}: expected an object, got ${BranchSplitService.describe(node)}`); - const kind = node['kind']; - if (kind === undefined || kind === null) throw missing(path, 'kind', NODE_FIELDS.kind.description); - if (typeof kind !== 'string') throw usageError(`${path}: "kind" must be a string, got ${BranchSplitService.describe(kind)}`, { hint: 'Write it as a JSON string, e.g. "kind": "...".' }); - }); - }); - } - /** * Splits after op.after into a gateway, its branches and a join. Branches ending in an end * event terminate there instead of reaching the join; a generated join id is marked generated, @@ -117,7 +80,7 @@ export class BranchSplitService { split(op: SplitIntoBranches): ChangeSet { const cs = new ChangeSet(); const kind = BranchSplitService.gatewayKind(op.kind); - BranchSplitService.checkShape(op.branches); + if (op.branches !== undefined && op.branches !== null) new CommandParser().parseBranches(op.branches); if (!op.branches?.length) { throw modelError('E_INVALID_VALUE', 'split needs at least one branch', { hint: 'Pass branches: [{ nodes: [...] }, ...].' }); } diff --git a/src/domain/editing/utils/containment.ts b/src/domain/editing/utils/containment.ts index c0be3fad..667a94e3 100644 --- a/src/domain/editing/utils/containment.ts +++ b/src/domain/editing/utils/containment.ts @@ -15,7 +15,7 @@ export function contains(parent: El, child: El): boolean { return false; } -export function detachFromParent(el: El): El | undefined { +export function detachFromParent(el: El, replacement?: El): El | undefined { const parent = el.$parent as El | undefined; if (!parent) return undefined; for (const property of descriptorOf(parent).properties ?? []) { @@ -23,14 +23,15 @@ export function detachFromParent(el: El): El | undefined { const propValue = ownValue(parent, property.name); if (Array.isArray(propValue)) { const index = propValue.indexOf(el); - if (index !== -1) { - propValue.splice(index, 1); - return parent; - } - } else if (propValue === el) { - parent.set(property.name, undefined); - return parent; + if (index === -1) continue; + if (replacement) propValue[index] = replacement; + else propValue.splice(index, 1); + } else { + if (propValue !== el) continue; + parent.set(property.name, replacement); } + if (replacement) replacement.$parent = parent; + return parent; } return parent; } From a7371e4d8f8695b0693b0e8a671f49132e4766d1 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:20:43 +0100 Subject: [PATCH 05/11] refactor: build CLI op fields from flag lists and read the command catalogue through functions --- src/adapters/in/cli/AddElementCommand.ts | 22 ++----------- src/adapters/in/cli/CliCommand.ts | 9 +++-- src/adapters/in/cli/ConnectCommand.ts | 9 ++--- .../in/cli/ExtensionElementsCommand.ts | 12 +++---- src/adapters/in/cli/MoveCommand.ts | 7 ++-- src/adapters/in/cli/MutationCommand.ts | 28 ++++++++-------- src/adapters/in/cli/RemoveElementsCommand.ts | 5 ++- src/adapters/in/cli/ValidateCommand.ts | 3 +- src/adapters/in/cli/utils/flags.ts | 12 +++++++ src/adapters/in/report/CommandCatalogue.ts | 33 ++++++------------- 10 files changed, 58 insertions(+), 82 deletions(-) diff --git a/src/adapters/in/cli/AddElementCommand.ts b/src/adapters/in/cli/AddElementCommand.ts index 73e63e61..1397e477 100644 --- a/src/adapters/in/cli/AddElementCommand.ts +++ b/src/adapters/in/cli/AddElementCommand.ts @@ -3,7 +3,7 @@ import type { AddElement } from '../../../domain/commands/types.js'; import { usageError } from '../../../domain/document/utils/errors.js'; import type { RawOpts } from './CliCommand.js'; import { MutationCommand } from './MutationCommand.js'; -import { parseKeyValues, splitList } from './utils/flags.js'; +import { givenFlags, givenStrings, parseKeyValues, splitList } from './utils/flags.js'; /** * Creates an element and wires it into the process (`bpmn add`). @@ -53,25 +53,9 @@ export class AddElementCommand extends MutationCommand { op: 'add', kind, ...(name !== undefined ? { name } : {}), - ...(options['id'] ? { id: String(options['id']) } : {}), - ...(options['after'] ? { after: String(options['after']) } : {}), - ...(options['before'] ? { before: String(options['before']) } : {}), - ...(options['flow'] ? { flow: String(options['flow']) } : {}), - ...(options['in'] ? { in: String(options['in']) } : {}), - ...(options['on'] ? { on: String(options['on']) } : {}), - ...(options['to'] ? { to: String(options['to']) } : {}), - ...(options['lane'] ? { lane: String(options['lane']) } : {}), - ...(options['flowName'] ? { flowName: String(options['flowName']) } : {}), - ...(options['flowId'] ? { flowId: String(options['flowId']) } : {}), + ...givenStrings(options, ['id', 'after', 'before', 'flow', 'in', 'on', 'to', 'lane', 'flowName', 'flowId', 'language', 'doc', 'text', 'process']), + ...givenFlags(options, ['default', 'collapsed', 'ifAbsent', 'blackBox']), ...(options['condition'] !== undefined ? { condition: String(options['condition']) } : {}), - ...(options['language'] ? { language: String(options['language']) } : {}), - ...(options['default'] ? { default: true } : {}), - ...(options['collapsed'] ? { collapsed: true } : {}), - ...(options['ifAbsent'] ? { ifAbsent: true } : {}), - ...(options['doc'] ? { doc: String(options['doc']) } : {}), - ...(options['text'] ? { text: String(options['text']) } : {}), - ...(options['process'] ? { process: String(options['process']) } : {}), - ...(options['blackBox'] ? { blackBox: true } : {}), ...(options['members'] ? { members: splitList(String(options['members'])) } : {}), ...(kv.length ? { set: parseKeyValues(kv) } : {}), ...this.triggerOptions(options), diff --git a/src/adapters/in/cli/CliCommand.ts b/src/adapters/in/cli/CliCommand.ts index f611c74f..46de5da7 100644 --- a/src/adapters/in/cli/CliCommand.ts +++ b/src/adapters/in/cli/CliCommand.ts @@ -1,6 +1,6 @@ import type { Command as CommandType } from 'commander'; import type { FileMutationOptions } from '../../../application/port/in/EditDocumentUseCase.js'; -import { CommandCatalogue, type CommandSignature } from '../report/CommandCatalogue.js'; +import { commandDoc, commonOption, type CommandSignature } from '../report/CommandCatalogue.js'; import type { Cli } from './Cli.js'; export interface RawOpts { @@ -12,14 +12,13 @@ export interface RawOpts { * and maps the flags all commands share. */ export abstract class CliCommand { - protected static readonly catalogue = new CommandCatalogue(); constructor(protected readonly cli: Cli) {} abstract register(program: CommandType): void; protected define(parent: CommandType, name: string): CommandType { - return this.defineSignature(parent, CliCommand.catalogue.command(name)); + return this.defineSignature(parent, commandDoc(name)); } protected defineSignature(parent: CommandType, doc: CommandSignature): CommandType { @@ -29,12 +28,12 @@ export abstract class CliCommand { } protected withJsonOptions(cmd: CommandType): CommandType { - return cmd.option(...CliCommand.catalogue.commonOption('--json')).option(...CliCommand.catalogue.commonOption('--pretty')); + return cmd.option(...commonOption('--json')).option(...commonOption('--pretty')); } /** In this order, so that the default stays undefined (like --layout / --no-layout). */ protected withLintOptions(cmd: CommandType): CommandType { - return cmd.option(...CliCommand.catalogue.commonOption('--lint-config')).option(...CliCommand.catalogue.commonOption('--no-lint-config')); + return cmd.option(...commonOption('--lint-config')).option(...commonOption('--no-lint-config')); } /** Paths and stdin resolve from the working directory; project plugins run. */ diff --git a/src/adapters/in/cli/ConnectCommand.ts b/src/adapters/in/cli/ConnectCommand.ts index 9f720440..5fdabd8a 100644 --- a/src/adapters/in/cli/ConnectCommand.ts +++ b/src/adapters/in/cli/ConnectCommand.ts @@ -2,6 +2,7 @@ import type { Command as CommandType } from 'commander'; import type { ConnectElements } from '../../../domain/commands/types.js'; import type { RawOpts } from './CliCommand.js'; import { MutationCommand } from './MutationCommand.js'; +import { givenFlags, givenStrings } from './utils/flags.js'; /** * Connects two elements; the kind of connection is inferred (`bpmn connect`). @@ -27,13 +28,9 @@ export class ConnectCommand extends MutationCommand { op: 'connect', source, target, - ...(options['name'] ? { name: String(options['name']) } : {}), - ...(options['id'] ? { id: String(options['id']) } : {}), + ...givenStrings(options, ['name', 'id', 'language', 'message']), + ...givenFlags(options, ['default', 'ifAbsent']), ...(options['condition'] !== undefined ? { condition: String(options['condition']) } : {}), - ...(options['language'] ? { language: String(options['language']) } : {}), - ...(options['default'] ? { default: true } : {}), - ...(options['message'] ? { message: String(options['message']) } : {}), - ...(options['ifAbsent'] ? { ifAbsent: true } : {}), }; await this.mutate(file, [op], opts); } diff --git a/src/adapters/in/cli/ExtensionElementsCommand.ts b/src/adapters/in/cli/ExtensionElementsCommand.ts index 6e1270d8..733753f5 100644 --- a/src/adapters/in/cli/ExtensionElementsCommand.ts +++ b/src/adapters/in/cli/ExtensionElementsCommand.ts @@ -1,9 +1,10 @@ import type { Command as CommandType } from 'commander'; import { ExtensionLister } from '../../../domain/document/ExtensionLister.js'; import type { EditExtensionElements } from '../../../domain/commands/types.js'; -import { CliCommand, type RawOpts } from './CliCommand.js'; +import type { RawOpts } from './CliCommand.js'; import { MutationCommand } from './MutationCommand.js'; -import { parseKeyValues } from './utils/flags.js'; +import { commandDoc } from '../report/CommandCatalogue.js'; +import { givenFlags, givenStrings, parseKeyValues } from './utils/flags.js'; import { renderExtensionList } from '../report/utils/view-lines.js'; /** @@ -23,7 +24,7 @@ export class ExtensionElementsCommand extends MutationCommand { } private subcommand(ext: CommandType, name: string): CommandType { - return this.defineSignature(ext, CliCommand.catalogue.command('ext').subcommands!.find((subcommand) => subcommand.name === name)!); + return this.defineSignature(ext, commandDoc('ext').subcommands!.find((subcommand) => subcommand.name === name)!); } private async add(file: string, id: string, type: string, attrs: string[], options: RawOpts): Promise { @@ -34,9 +35,8 @@ export class ExtensionElementsCommand extends MutationCommand { action: 'add', type, ...(attrs.length ? { attrs: parseKeyValues(attrs, 'attribute') } : {}), - ...(options['body'] ? { body: String(options['body']) } : {}), - ...(options['xml'] ? { xml: String(options['xml']) } : {}), - ...(options['replace'] ? { replace: true } : {}), + ...givenStrings(options, ['body', 'xml']), + ...givenFlags(options, ['replace']), }; await this.mutate(file, [op], opts); } diff --git a/src/adapters/in/cli/MoveCommand.ts b/src/adapters/in/cli/MoveCommand.ts index 4123ad6f..436311ba 100644 --- a/src/adapters/in/cli/MoveCommand.ts +++ b/src/adapters/in/cli/MoveCommand.ts @@ -3,6 +3,7 @@ import type { MoveElements } from '../../../domain/commands/types.js'; import { usageError } from '../../../domain/document/utils/errors.js'; import type { RawOpts } from './CliCommand.js'; import { MutationCommand } from './MutationCommand.js'; +import { givenStrings } from './utils/flags.js'; /** * Relocates nodes to another flow, scope, activity or lane (`bpmn move`). @@ -26,11 +27,7 @@ export class MoveCommand extends MutationCommand { const op: MoveElements = { op: 'move', ids, - ...(options['after'] ? { after: String(options['after']) } : {}), - ...(options['before'] ? { before: String(options['before']) } : {}), - ...(options['flow'] ? { flow: String(options['flow']) } : {}), - ...(options['in'] ? { in: String(options['in']) } : {}), - ...(options['on'] ? { on: String(options['on']) } : {}), + ...givenStrings(options, ['after', 'before', 'flow', 'in', 'on']), ...(options['lane'] !== undefined ? { lane: String(options['lane']) } : {}), }; if (!op.after && !op.before && !op.flow && !op.in && !op.on && op.lane === undefined) { diff --git a/src/adapters/in/cli/MutationCommand.ts b/src/adapters/in/cli/MutationCommand.ts index a42826db..f3101a16 100644 --- a/src/adapters/in/cli/MutationCommand.ts +++ b/src/adapters/in/cli/MutationCommand.ts @@ -15,7 +15,8 @@ import { isCliError, usageError } from '../../../domain/document/utils/errors.js import { CliCommand, type RawOpts } from './CliCommand.js'; import type { CliStream } from './Cli.js'; import { renderFormat, renderTarget } from './utils/flags.js'; -import { flag } from '../report/utils/option-flags.js'; +import { flag, triggerFlag } from '../report/utils/option-flags.js'; +import { commonOption } from '../report/CommandCatalogue.js'; import { ModelViewText } from '../report/ModelViewText.js'; import { MutationReportBuilder } from '../report/MutationReportBuilder.js'; import { MutationSummaryBuilder } from '../report/MutationSummaryBuilder.js'; @@ -46,32 +47,31 @@ const FLOW_FLAGS: Record = { flowName: '--flow-name', flowId: '- */ export abstract class MutationCommand extends CliCommand { protected withMutationOptions(cmd: CommandType, { layoutToggle = true } = {}): CommandType { - const option = (long: string) => CliCommand.catalogue.commonOption(long); - this.withJsonOptions(cmd).option(...option('--summary')).option(...option('--out')).option(...option('--dry-run')).option(...option('--render'), renderTarget); + this.withJsonOptions(cmd).option(...commonOption('--summary')).option(...commonOption('--out')).option(...commonOption('--dry-run')).option(...commonOption('--render'), renderTarget); if (layoutToggle) { cmd - .option(...option('--layout'), (raw: string) => { + .option(...commonOption('--layout'), (raw: string) => { if (!(LAYOUT_MODES as readonly string[]).includes(raw)) throw new InvalidArgumentError(`expected ${LAYOUT_MODES.join(', ')}`); return raw; }) - .option(...option('--relayout')) - .option(...option('--no-layout')); + .option(...commonOption('--relayout')) + .option(...commonOption('--no-layout')); } return this.withLintOptions(cmd) - .option(...option('--force')) - .option(...option('--backup')) - .option(...option('--show')) - .option(...option('--strict')) - .option(...option('--keep-flow-ids')) - .option(...option('--no-keep-flow-ids')) - .option(...option('--engine'), (raw: string) => { + .option(...commonOption('--force')) + .option(...commonOption('--backup')) + .option(...commonOption('--show')) + .option(...commonOption('--strict')) + .option(...commonOption('--keep-flow-ids')) + .option(...commonOption('--no-keep-flow-ids')) + .option(...commonOption('--engine'), (raw: string) => { if (raw !== 'clean' && raw !== 'auto') throw new InvalidArgumentError('expected clean or auto'); return raw; }); } protected withTriggerOptions(cmd: CommandType): CommandType { - for (const key of TRIGGER_KEYS) cmd.option(...CliCommand.catalogue.triggerOption(key)); + for (const key of TRIGGER_KEYS) cmd.option(...triggerFlag(key)); return cmd; } diff --git a/src/adapters/in/cli/RemoveElementsCommand.ts b/src/adapters/in/cli/RemoveElementsCommand.ts index 71bd132a..116fb653 100644 --- a/src/adapters/in/cli/RemoveElementsCommand.ts +++ b/src/adapters/in/cli/RemoveElementsCommand.ts @@ -2,6 +2,7 @@ import type { Command as CommandType } from 'commander'; import type { RemoveElements } from '../../../domain/commands/types.js'; import type { RawOpts } from './CliCommand.js'; import { MutationCommand } from './MutationCommand.js'; +import { givenFlags } from './utils/flags.js'; /** * Removes elements and bridges the gaps they leave (`bpmn remove`). @@ -23,9 +24,7 @@ export class RemoveElementsCommand extends MutationCommand { op: 'remove', ids, bridge: options['bridge'] !== false, - ...(options['bridgeAll'] ? { bridgeAll: true } : {}), - ...(options['withBranch'] ? { withBranch: true } : {}), - ...(options['ifExists'] ? { ifExists: true } : {}), + ...givenFlags(options, ['bridgeAll', 'withBranch', 'ifExists']), }; await this.mutate(file, [op], opts); } diff --git a/src/adapters/in/cli/ValidateCommand.ts b/src/adapters/in/cli/ValidateCommand.ts index 1589df18..cebf2764 100644 --- a/src/adapters/in/cli/ValidateCommand.ts +++ b/src/adapters/in/cli/ValidateCommand.ts @@ -1,6 +1,7 @@ import { InvalidArgumentError, type Command as CommandType } from 'commander'; import { PLATFORM_CHOICES, type PlatformChoice } from '../../../domain/document/vendor/PlatformDetection.js'; import { CliCommand, type RawOpts } from './CliCommand.js'; +import { commonOption } from '../report/CommandCatalogue.js'; import { ValidationReportBuilder } from '../report/ValidationReportBuilder.js'; import { ValidationText } from '../report/ValidationText.js'; @@ -13,7 +14,7 @@ export class ValidateCommand extends CliCommand { register(program: CommandType): void { this.withLintOptions( this.withJsonOptions(this.define(program, 'validate')) - .option(...CliCommand.catalogue.commonOption('--strict')) + .option(...commonOption('--strict')) .option('--platform ', 'engine profile: auto (default: detected from modeler:executionPlatform / the vendor namespace), c7, c8 or none', (raw: string) => { if (!(PLATFORM_CHOICES as readonly string[]).includes(raw)) throw new InvalidArgumentError(`expected ${PLATFORM_CHOICES.join(', ')}`); return raw; diff --git a/src/adapters/in/cli/utils/flags.ts b/src/adapters/in/cli/utils/flags.ts index 78965894..1b73cdbc 100644 --- a/src/adapters/in/cli/utils/flags.ts +++ b/src/adapters/in/cli/utils/flags.ts @@ -14,6 +14,18 @@ export function parseKeyValues(items: string[], what = 'property'): Record(options: Record, keys: readonly TKey[]): Partial> { + const out: Partial> = {}; + for (const key of keys) if (options[key]) out[key] = String(options[key]); + return out; +} + +export function givenFlags(options: Record, keys: readonly TKey[]): Partial> { + const out: Partial> = {}; + for (const key of keys) if (options[key]) out[key] = true; + return out; +} + export function splitList(value: string | undefined): string[] | undefined { if (!value) return undefined; return value diff --git a/src/adapters/in/report/CommandCatalogue.ts b/src/adapters/in/report/CommandCatalogue.ts index 7d9d2de6..f4ce4dbc 100644 --- a/src/adapters/in/report/CommandCatalogue.ts +++ b/src/adapters/in/report/CommandCatalogue.ts @@ -1,6 +1,4 @@ import { SWATCHES } from '../../../domain/diagram/utils/colors.js'; -import type { TriggerOptions } from '../../../domain/commands/types.js'; -import { triggerFlag } from './utils/option-flags.js'; import { LAYOUT_MODES } from '../../../domain/diagram/types.js'; export interface CommandSignature { @@ -17,6 +15,10 @@ export interface CommandDoc extends CommandSignature { examples: string[]; } +/** + * The commands of `bpmn`, registered with commander and listed by `bpmn guide`. Where the two + * read differently an entry keeps both: `help` for `--help`, `usage` / `summary` for the guide. + */ export const COMMANDS: CommandDoc[] = [ { name: 'new', arguments: '', help: 'create a new .bpmn file with one empty process', usage: 'bpmn new [--name ] [--id ] [--no-executable] [--target camunda8|camunda7]', summary: 'Create a file with one empty process. --target camunda7 (also for CIB seven and Operaton) writes what Camunda Modeler writes: the camunda / modeler namespaces, the platform version and camunda:historyTimeToLive=180 (the engines refuse an executable process without it). --target camunda8 writes the zeebe / modeler namespaces and the platform (Camunda Cloud, 8.9.0); in a Camunda 8 file new user tasks are Camunda user tasks (zeebe:userTask) and new event definitions get an id, like the Modeler. The process id is Process_ (Process_1 without a name) unless element-id-naming of the .bpmnlintrc names a bpmn:Process prefix (process_orderHandling, process_main without a name; see CONVENTIONS).', examples: ['bpmn new order.bpmn --name "Order handling" --target camunda8', 'bpmn new order.bpmn --name "Order handling" --target camunda7'] }, { name: 'show', arguments: ' [id]', help: 'print the model (or one element) as the AI sees it: no coordinates (file - = stdin)', usage: 'bpmn show [ [--context]] [--json] [--scope ] | bpmn show --around [--depth ] [--inner] [--json] | bpmn show --layout [--json] | bpmn show --conventions [--json] [--lint-config | --no-lint-config]', summary: 'Print the model in flow order (no coordinates), or every detail of one element. Every node carries its lane (lane=); vendor attributes appear with their values next to the properties (nested ones under their set keys: loop.camunda:collection=${items}), extension types as `ext: camunda:taskListener x3`; the process line carries the process\'s own; a business rule task shows its decision link as calledDecision= in every spelling; message flows name their endpoints, the collaboration lists its annotations. --around prints only the neighbourhood of an element (--depth flow steps both ways, default 2; see READING LARGE MODELS), --context the element in its context (where, lane, before / after, catching events, message flows). --layout prints the drawing instead: per pool / lane the rows of node ids (left to right) with their column across the diagram (c0..cN; " … " where a wide gap splits a row), the wide empty gaps between columns, colours, labels off their default side and the layout problems with ids. --conventions prints the id conventions new ids follow in the file: per pattern the kinds and where it comes from (element-id-naming of the .bpmnlintrc, the file\'s ids, bpmn-cli\'s default), also for a file that does not exist yet (see CONVENTIONS). `-` as the file reads stdin.', examples: ['bpmn show order.bpmn', 'bpmn show order.bpmn --around Activity_CheckInvoice', 'bpmn show order.bpmn Activity_CheckInvoice --context', 'bpmn show order.bpmn Activity_CheckInvoice --json', 'bpmn show order.bpmn --layout', 'bpmn show order.bpmn --conventions'] }, @@ -74,26 +76,11 @@ export const COMMON_OPTIONS: CommonOptionDoc[] = [ { option: '--engine ', description: 'layout engine: clean (built-in, default: straight happy path, branches as bands below, loops underneath) or auto (bpmn-auto-layout)', flags: '--engine ', help: 'layout engine: clean (default) or auto (bpmn-auto-layout)' }, ]; -/** - * The commands of `bpmn`, registered with commander and listed by `bpmn guide`. Where the two - * read differently an entry keeps both: `help` for `--help`, `usage` / `summary` for the guide. - */ -export class CommandCatalogue { - constructor( - readonly commands: readonly CommandDoc[] = COMMANDS, - readonly commonOptions: readonly CommonOptionDoc[] = COMMON_OPTIONS, - ) {} - - command(name: string): CommandDoc { - return this.commands.find((command) => command.name === name)!; - } - - commonOption(longFlag: string): [flags: string, help: string] { - const doc = this.commonOptions.find((option) => (option.flags ?? option.option).split(/[ ,]+/).includes(longFlag))!; - return [doc.flags ?? doc.option, doc.help ?? doc.description]; - } +export function commandDoc(name: string): CommandDoc { + return COMMANDS.find((command) => command.name === name)!; +} - triggerOption(key: keyof TriggerOptions): [flags: string, help: string] { - return triggerFlag(key); - } +export function commonOption(longFlag: string): [flags: string, help: string] { + const doc = COMMON_OPTIONS.find((option) => (option.flags ?? option.option).split(/[ ,]+/).includes(longFlag))!; + return [doc.flags ?? doc.option, doc.help ?? doc.description]; } From dd1e764cd641aa362508714a28974a75ef81000d Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:25:02 +0100 Subject: [PATCH 06/11] refactor!: drop thirteen undocumented library exports and the use case methods only they reached BREAKING CHANGE: mutationWarnings, opWarnings, renderAround, renderContext, renderMetrics, decisionLinkOf, scopeView, assertLossless, kindsSectionJson, runFormatOps, viewDoc and renderShown are no longer exported from @miragon/bpmn-cli, and layoutDocToFile no longer from @miragon/bpmn-cli/node. None of them was documented in docs/library.md. --- src/api.ts | 7 +--- .../port/in/ReadDocumentUseCase.ts | 10 ----- src/application/port/in/RunCommandsUseCase.ts | 15 -------- src/application/port/in/ShowModelUseCase.ts | 17 --------- .../service/ReadDocumentService.ts | 4 -- src/application/service/RunCommandsService.ts | 5 --- src/application/service/ShowModelService.ts | 9 ----- src/index.ts | 37 +------------------ src/node.ts | 2 - 9 files changed, 2 insertions(+), 104 deletions(-) diff --git a/src/api.ts b/src/api.ts index 96fd504a..8b9dab8e 100644 --- a/src/api.ts +++ b/src/api.ts @@ -137,13 +137,8 @@ export async function validateXml(xml: string, opts: ValidateXmlOptions = {}): P return new ValidationReportBuilder(await new ValidateDocumentService(NO_PROJECT).validate({ ...opts, source: { doc: await parseXml(xml, opts.file) } })).report(); } -/** `bpmn show` on a parsed document. */ -export function viewDoc(doc: Doc, opts: ViewOptions = {}): ShownView { - return new ShowModelService().show({ ...opts, doc }); -} - /** The text of a view as `bpmn show` prints it, without the final newline. */ -export function renderShown(shown: ShownView): string { +function renderShown(shown: ShownView): string { return new ShownText(shown).text(); } diff --git a/src/application/port/in/ReadDocumentUseCase.ts b/src/application/port/in/ReadDocumentUseCase.ts index 9c693571..88f9ba30 100644 --- a/src/application/port/in/ReadDocumentUseCase.ts +++ b/src/application/port/in/ReadDocumentUseCase.ts @@ -4,9 +4,6 @@ import type { Doc } from '../../../domain/document/Doc.js'; export interface ReadDocumentUseCase { read(command: ReadDocumentUseCase.Command): Promise; - - /** Throws E_IMPORT_LOSSY when bpmn-moddle dropped content on import, unless `force`. */ - assertLossless(command: ReadDocumentUseCase.LosslessCommand): void; } export declare namespace ReadDocumentUseCase { @@ -28,11 +25,4 @@ export declare namespace ReadDocumentUseCase { /** The opened document. */ type Result = Doc; - - /** The document to check for content dropped on import. */ - interface LosslessCommand { - doc: Doc; - /** accept a lossy import */ - force?: boolean; - } } diff --git a/src/application/port/in/RunCommandsUseCase.ts b/src/application/port/in/RunCommandsUseCase.ts index 97b4eb48..8adc118e 100644 --- a/src/application/port/in/RunCommandsUseCase.ts +++ b/src/application/port/in/RunCommandsUseCase.ts @@ -1,7 +1,6 @@ import type { Doc } from '../../../domain/document/Doc.js'; import type { ChangeSet } from '../../../domain/document/ChangeSet.js'; import type { EditCommand } from '../../../domain/commands/types.js'; -import type { FormatEntry, FormatResult } from '../../../domain/diagram/DiagramCommandService.js'; /** * The commands of a batch run on a document in memory on their own: @@ -15,12 +14,6 @@ export interface RunCommandsUseCase { /** Runs model commands as one batch and returns what changed. */ runModelCommands(command: RunCommandsUseCase.ModelCommands): ChangeSet; - /** - * Runs the diagram commands of a batch on the drawing; with `asFile` (a format-only batch) - * the first command's refusal may name a runnable command. - */ - runDiagramCommands(command: RunCommandsUseCase.DiagramCommands): FormatResult[]; - /** Validates the ops (an array or `{ ops }`); a usage error names the op index. */ parseCommands(command: RunCommandsUseCase.ParseCommand): EditCommand[]; @@ -42,14 +35,6 @@ export declare namespace RunCommandsUseCase { commands: EditCommand[]; } - /** Diagram commands of a batch and the document they draw on. */ - interface DiagramCommands { - doc: Doc; - entries: readonly FormatEntry[]; - /** a format-only batch: the first command's refusal may name a runnable command */ - asFile?: boolean; - } - /** Ops as parsed JSON: an array or `{ ops }`. */ interface ParseCommand { input: unknown; diff --git a/src/application/port/in/ShowModelUseCase.ts b/src/application/port/in/ShowModelUseCase.ts index 410d4a36..73e32862 100644 --- a/src/application/port/in/ShowModelUseCase.ts +++ b/src/application/port/in/ShowModelUseCase.ts @@ -1,6 +1,5 @@ import type { Doc } from '../../../domain/document/Doc.js'; import type { ExtensionInfo } from '../../../domain/document/ExtensionLister.js'; -import type { DecisionLink } from '../../../domain/document/DecisionLinkReader.js'; import type { El } from '../../../domain/document/utils/moddle.js'; import type { AroundOptions, AroundView, ElementContext, Implementation } from '../../../domain/views/context.js'; import type { LayoutView } from '../../../domain/views/layout-view.js'; @@ -14,9 +13,6 @@ export interface ShowModelUseCase { modelView(command: ShowModelUseCase.DocumentCommand): ModelView; - /** The view narrowed to one scope (`show --scope`). */ - scopeView(command: ShowModelUseCase.ScopeCommand): ModelView; - elementDetail(command: ShowModelUseCase.DetailCommand): ElementDetail; aroundView(command: ShowModelUseCase.AroundCommand): AroundView; @@ -27,8 +23,6 @@ export interface ShowModelUseCase { implementation(command: ShowModelUseCase.ElementCommand): Implementation | undefined; - decisionLink(command: ShowModelUseCase.DecisionLinkCommand): DecisionLink | undefined; - /** The extension elements of an element, in order. */ extensions(command: ShowModelUseCase.ElementCommand): ExtensionInfo[]; @@ -51,12 +45,6 @@ export declare namespace ShowModelUseCase { doc: Doc; } - /** A model view to narrow to one scope. */ - interface ScopeCommand extends DocumentCommand { - view: ModelView; - scopeId: string; - } - /** An element of the document to detail. */ interface DetailCommand extends DocumentCommand { element: El; @@ -79,9 +67,4 @@ export declare namespace ShowModelUseCase { interface ElementCommand { element: El; } - - /** An element whose decision link is resolved, in its document if given. */ - interface DecisionLinkCommand extends ElementCommand { - doc?: Doc; - } } diff --git a/src/application/service/ReadDocumentService.ts b/src/application/service/ReadDocumentService.ts index 9f142924..b2855b9a 100644 --- a/src/application/service/ReadDocumentService.ts +++ b/src/application/service/ReadDocumentService.ts @@ -12,10 +12,6 @@ export class ReadDocumentService implements ReadDocumentUseCase { return doc; } - assertLossless(command: ReadDocumentUseCase.LosslessCommand): void { - command.doc.assertLossless(command); - } - private async parse(source: ReadDocumentUseCase.Command['source']): Promise { if ('xml' in source) return Doc.fromXml(source.xml, source.file); return 'file' in source ? Doc.fromXml(await this.load!.read(source.file), source.file) : Doc.fromXml(this.load!.decode(source.bytes, source.name)); diff --git a/src/application/service/RunCommandsService.ts b/src/application/service/RunCommandsService.ts index 7f48d5da..b5477686 100644 --- a/src/application/service/RunCommandsService.ts +++ b/src/application/service/RunCommandsService.ts @@ -4,7 +4,6 @@ import type { EditCommand } from '../../domain/commands/types.js'; import { CommandParser } from '../../domain/commands/CommandParser.js'; import { parseOpsJson } from '../../domain/commands/utils/ops-json.js'; import { ModelCommandService } from '../../domain/batch/ModelCommandService.js'; -import { DiagramCommandService, type FormatResult } from '../../domain/diagram/DiagramCommandService.js'; export class RunCommandsService implements RunCommandsUseCase { runModelCommand(command: RunCommandsUseCase.ModelCommand): ChangeSet { @@ -15,10 +14,6 @@ export class RunCommandsService implements RunCommandsUseCase { return new ModelCommandService(command.doc).runBatch(command.commands).changes; } - runDiagramCommands(command: RunCommandsUseCase.DiagramCommands): FormatResult[] { - return new DiagramCommandService(command.doc).run(command.entries, command.asFile ?? false); - } - parseCommands(command: RunCommandsUseCase.ParseCommand): EditCommand[] { return new CommandParser().parse(command.input); } diff --git a/src/application/service/ShowModelService.ts b/src/application/service/ShowModelService.ts index 193a0a18..9615c1a2 100644 --- a/src/application/service/ShowModelService.ts +++ b/src/application/service/ShowModelService.ts @@ -6,7 +6,6 @@ import { AroundWindow } from '../../domain/views/AroundWindow.js'; import { ElementContextBuilder } from '../../domain/views/ElementContextBuilder.js'; import { LayoutViewBuilder } from '../../domain/views/LayoutViewBuilder.js'; import { ElementFacts } from '../../domain/views/ElementFacts.js'; -import { DecisionLinkReader, type DecisionLink } from '../../domain/document/DecisionLinkReader.js'; import { ExtensionLister, type ExtensionInfo } from '../../domain/document/ExtensionLister.js'; import type { AroundView, ElementContext, Implementation } from '../../domain/views/context.js'; import type { LayoutView } from '../../domain/views/layout-view.js'; @@ -22,10 +21,6 @@ export class ShowModelService implements ShowModelUseCase { return new ModelViewBuilder(command.doc).build(); } - scopeView(command: ShowModelUseCase.ScopeCommand): ModelView { - return new ModelViewBuilder(command.doc).scope(command.view, command.scopeId); - } - elementDetail(command: ShowModelUseCase.DetailCommand): ElementDetail { return new ElementDetailBuilder(command.doc, command.element).build(); } @@ -47,10 +42,6 @@ export class ShowModelService implements ShowModelUseCase { return new ElementFacts(command.element).implementation(); } - decisionLink(command: ShowModelUseCase.DecisionLinkCommand): DecisionLink | undefined { - return new DecisionLinkReader(command.doc).linkOf(command.element); - } - extensions(command: ShowModelUseCase.ElementCommand): ExtensionInfo[] { return new ExtensionLister().list(command.element); } diff --git a/src/index.ts b/src/index.ts index 740d03a1..c1eff10a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -36,13 +36,9 @@ import { AnalyzeLayoutService } from './application/service/AnalyzeLayoutService import { RenderSvgService } from './application/service/RenderSvgService.js'; import { EditDocumentService } from './application/service/EditDocumentService.js'; import { RelayoutDiagramService } from './application/service/RelayoutDiagramService.js'; -import { ReadDocumentService } from './application/service/ReadDocumentService.js'; import { NoProjectSettings } from './adapters/out/memory/NoProjectSettings.js'; -import { AroundText } from './adapters/in/report/AroundText.js'; -import { ContextText } from './adapters/in/report/ContextText.js'; import { DetailText } from './adapters/in/report/DetailText.js'; import { LayoutText } from './adapters/in/report/LayoutText.js'; -import { MetricsText } from './adapters/in/report/MetricsText.js'; import { ModelViewText } from './adapters/in/report/ModelViewText.js'; import type { MutationReport, MutationReportLike, MutationSummary, ValidationReport } from './adapters/in/report/command-report.js'; import { MutationReportBuilder } from './adapters/in/report/MutationReportBuilder.js'; @@ -53,18 +49,14 @@ import { ValidationReportBuilder } from './adapters/in/report/ValidationReportBu import { ValidationText } from './adapters/in/report/ValidationText.js'; import { WarningLines } from './adapters/in/report/WarningLines.js'; import { advisoryOf } from './adapters/in/report/utils/findings.js'; -import type { DecisionLink } from './domain/document/DecisionLinkReader.js'; import type { ExtensionInfo } from './domain/document/ExtensionLister.js'; import type { LayoutOptions, LayoutResult } from './domain/diagram/layout/FullLayout.js'; -import type { FormatEntry, FormatResult } from './domain/diagram/DiagramCommandService.js'; const NO_PROJECT = new NoProjectSettings(); /* the in-memory API: strings in, strings / data out */ -export { mutateDoc, layoutDoc, checkDoc, applyToXml, newXml, layoutXml, validateXml, viewXml, showXml, conventionsXml, metricsXml, findXml, extensionsXml, viewDoc, renderShown, type EditOptions, type EditResult, type EditReport, type LayoutXmlOptions, type ShownView, type ValidateXmlOptions, type ViewOptions } from './api.js'; +export { mutateDoc, layoutDoc, checkDoc, applyToXml, newXml, layoutXml, validateXml, viewXml, showXml, conventionsXml, metricsXml, findXml, extensionsXml, type EditOptions, type EditResult, type EditReport, type LayoutXmlOptions, type ShownView, type ValidateXmlOptions, type ViewOptions } from './api.js'; export { type MutationReport, type MutationReportLike, type MutationSummary, type ValidationReport, type WarningReport } from './adapters/in/report/command-report.js'; export { conventionsLines } from './adapters/in/report/utils/mutation-lines.js'; export function mutationReport(result: MutationResult): MutationReport { return new MutationReportBuilder(result).report(); } -export function mutationWarnings(result: MutationResult): Warning[] { return new MutationReportBuilder(result).warnings(); } -export function opWarnings(result: MutationResult): Warning[] { return new MutationReportBuilder(result).opWarnings(); } export function renderMutation(result: MutationReportLike, opts: { dryRun?: boolean } = {}): string { return new MutationText(result, opts).text(); } export function mutationSummary(report: MutationReportLike): MutationSummary { return new MutationSummaryBuilder(report).summary(); } export function renderSummary(summary: MutationSummary, opts: { dryRun?: boolean; advisory?: ReadonlySet } = {}): string { return new SummaryText(summary, opts).text(); } @@ -76,10 +68,7 @@ export { renderFind, renderExtensionList } from './adapters/in/report/utils/view export { renderProblems } from './adapters/in/report/utils/result-lines.js'; export function renderView(view: ModelView): string { return new ModelViewText(view).text(); } export function renderDetail(detail: ElementDetail): string { return new DetailText(detail).text(); } -export function renderAround(view: AroundView): string { return new AroundText(view).text(); } -export function renderContext(context: ElementContext): string { return new ContextText(context).text(); } export function renderLayoutView(view: LayoutView): string { return new LayoutText(view).text(); } -export function renderMetrics(metrics: LayoutMetrics): string { return new MetricsText(metrics).text(); } export { setLayoutDebug } from './api.js'; export { type DebugSink } from './domain/diagram/layout/LayoutDebug.js'; export { GUIDE_TOPICS } from './adapters/in/report/AgentGuide.js'; @@ -158,11 +147,6 @@ export function conventionsView( export type { DecisionLink } from './domain/document/DecisionLinkReader.js'; -/** The decision link of a business rule task in any spelling. */ -export function decisionLinkOf(el: El, doc?: Doc): DecisionLink | undefined { - return new ShowModelService().decisionLink({ element: el, ...(doc !== undefined ? { doc } : {}) }); -} - export { ChangeSet, type Change } from './domain/document/ChangeSet.js'; export type { ValidateOptions, ValidationResult } from './domain/validation/StructuralValidator.js'; /** Structural validation and lint of a document, with the platform profile on request. */ @@ -176,7 +160,6 @@ export { type ModelView, type ElementDetail, type DetailMessageFlow, type Detail export function buildView(doc: Doc): ModelView { return new ShowModelService().modelView({ doc }); } export function elementDetail(doc: Doc, el: El): ElementDetail { return new ShowModelService().elementDetail({ doc, element: el }); } export function findElements(doc: Doc, text: string, kind?: string): FindHit[] { return new FindElementsService().find({ doc, text, ...(kind !== undefined ? { kind } : {}) }); } -export function scopeView(doc: Doc, view: ModelView, scopeId: string): ModelView { return new ShowModelService().scopeView({ doc, view, scopeId }); } export { OPS_SCHEMA } from './domain/commands/OpsSchemaBuilder.js'; /* the diagram API: read-only views of a drawing and the format operations */ export { diffProblems, metricsDelta, KEYS as METRIC_KEYS, WEIGHTS as METRIC_WEIGHTS, type LayoutMetrics, type LayoutProblem, type ProblemLocation, type MetricKey, type MetricsDelta, type MetricsSummary } from './domain/diagram/utils/problems.js'; @@ -218,11 +201,6 @@ export function parseOps(input: unknown): EditCommand[] { return new RunCommandsService().parseCommands({ input }); } -/** Throws E_IMPORT_LOSSY when bpmn-moddle dropped content on import, unless `force`. */ -export function assertLossless(doc: Doc, opts: { force?: boolean } = {}): void { - new ReadDocumentService().assertLossless({ ...opts, doc }); -} - /** * Loads the engine rules of `platform` (cached), needed before the synchronous runProfile and * validateDoc; the async API loads them itself. A failed load (E_PROFILE_LOAD) is retried later. @@ -284,11 +262,6 @@ export function kindsSectionText(sections: string[]): string { return new KindsReference().sectionText(sections); } -/** `bpmn kinds --section --json`: only those keys of kindsJson(). */ -export function kindsSectionJson(sections: string[]): Record { - return new KindsReference().sectionJson(sections); -} - /** The layout problems of every diagram; `importWarnings` count as one `failed` problem. */ export function layoutProblems(defs: El, opts: { importWarnings?: number } = {}): LayoutMetrics { return new AnalyzeLayoutService().measure({ ...opts, definitions: defs }); @@ -304,14 +277,6 @@ export function locateProblems(defs: El, problems: readonly LayoutProblem[]): La return new AnalyzeLayoutService().locateProblems({ definitions: defs, problems }); } -/** - * Runs the format ops of a batch on the drawing of `doc`; with `asFile` (a format-only batch) - * the first op's refusal may name a runnable command. - */ -export function runFormatOps(doc: Doc, entries: readonly FormatEntry[], asFile = false): FormatResult[] { - return new RunCommandsService().runDiagramCommands({ doc, entries, asFile }); -} - /** * Lays out the model. Its in-memory DI is replaced by the hint diagram as a side effect: * treat the returned XML as the truth and re-parse it if you need DI. diff --git a/src/node.ts b/src/node.ts index 2d4619ef..de705554 100644 --- a/src/node.ts +++ b/src/node.ts @@ -42,8 +42,6 @@ export function mutateFile(file: string, ops: EditCommand[], opts: FileMutationO export function mutateDocToFile(doc: Doc, ops: EditCommand[], opts: FileMutationOptions = {}): Promise { return new EditDocumentService(PROJECT, FILES, FILES).edit({ ...opts, source: { doc }, commands: ops }); } /** `bpmn layout`: redraws a file (or with `tidy` only removes overlaps) and writes it back. */ export function layoutFile(file: string, opts: FileLayoutOptions = {}): Promise { return new RelayoutDiagramService(PROJECT, FILES, FILES).relayout({ ...opts, source: { file } }); } -/** `bpmn layout` on a loaded document, written to opts.out ?? doc.file unless dryRun. */ -export function layoutDocToFile(doc: Doc, opts: FileLayoutOptions = {}): Promise { return new RelayoutDiagramService(PROJECT, FILES, FILES).relayout({ ...opts, source: { doc } }); } /** `bpmn validate`: validates a file and dry-runs the layout without writing anything. */ export function checkFile(file: string, opts: FileCheckOptions = {}): Promise { return new ValidateDocumentService(PROJECT, FILES).validate({ ...opts, source: { file } }); } /** From 16be2083e3a6421bf3cbcae7eec5ffb94c4cd275 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:31:20 +0100 Subject: [PATCH 07/11] refactor!: stop recognising the ids bpmn-cli 0.6 cut BREAKING CHANGE: flow and DI ids that bpmn-cli 0.6 cut at the 64-character limit in its own way are no longer recognised as generated, so they keep their id when their ends change. Ids 0.7 and later wrote, and uncut ids of any version, are not affected. --- docs/testing.md | 4 +- src/domain/diagram/utils/di-ids.ts | 16 ++-- src/domain/document/ids/IdFitter.ts | 54 +++--------- src/domain/document/ids/IdStyle.ts | 48 +++++----- src/domain/document/ids/LearnedStyle.ts | 30 +------ src/domain/document/utils/slug.ts | 49 ++++------- test/domain/diagram/conventions-di.test.ts | 27 +----- test/domain/document/conventions-ids.test.ts | 26 ------ test/domain/document/id-quality.test.ts | 92 +------------------- 9 files changed, 69 insertions(+), 277 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 823d4a7d..880dea70 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -234,9 +234,7 @@ ending on a number that reads like a suffix (`Phase1To...` kept), a cut inside a word that keeps the numbers, a camelCase end without its first word, a suffix within the cap, none skipped, also on an id that names the anchor's kind, a file prefix or scope too long for the cap in every flow -form, cut flows voting for their form, flows 0.6 cut still following their -ends, also with `_100` and a leading separator, ends of any length cut -fast: #31), +form, cut flows voting for their form, ends of any length cut fast: #31), transliteration beyond German, the definitions id of `new`, and the warnings delta: platform findings counted, a shrinking `W_DUPLICATE_NAME`, a bridge that takes over a removed flow's id, warnings of a batch's final state, `new` diff --git a/src/domain/diagram/utils/di-ids.ts b/src/domain/diagram/utils/di-ids.ts index 744a8fa4..e681b45a 100644 --- a/src/domain/diagram/utils/di-ids.ts +++ b/src/domain/diagram/utils/di-ids.ts @@ -71,30 +71,30 @@ const MAX_DI = MAX_ID - 3; /** * A long element id is cut at a word boundary like element ids, leaving room for a collision - * suffix. `legacy` cuts as 0.6 did, to recognise the DI ids it wrote. + * suffix. */ -export function styledId(style: Style, kind: 'Shape' | 'Edge', id: string, legacy = false): string { +export function styledId(style: Style, kind: 'Shape' | 'Edge', id: string): string { const [head, tail] = style === 'suffix' ? ['', '_di'] : style === 'bpmn' ? [`BPMN${kind}_`, ''] : [`${kind}_`, '']; const room = MAX_DI - head.length - tail.length; - return `${head}${id.length > room ? cutAt(id, room, legacy) : id}${tail}`; + return `${head}${id.length > room ? cutAt(id, room) : id}${tail}`; } function isOrSuffixed(diId: string, base: string): boolean { return diId === base || (diId.startsWith(`${base}_`) && /^\d+$/.test(diId.slice(base.length + 1))); } -/** Recognises cut ids (now or as 0.6 cut) and collision suffixes too. */ +/** Recognises cut ids and collision suffixes too. */ function diStyleOf(diId: string, elId: string, kind: 'Shape' | 'Edge'): Style | undefined { - return styleOf(diId, elId, kind) ?? (['suffix', 'bpmn', 'short'] as Style[]).find((style) => isOrSuffixed(diId, styledId(style, kind, elId)) || isOrSuffixed(diId, styledId(style, kind, elId, true))); + return styleOf(diId, elId, kind) ?? (['suffix', 'bpmn', 'short'] as Style[]).find((style) => isOrSuffixed(diId, styledId(style, kind, elId))); } -export function rootDiId(kind: 'Plane' | 'Diagram', root: string, legacy = false): string { +export function rootDiId(kind: 'Plane' | 'Diagram', root: string): string { const head = `BPMN${kind}_`; - return `${head}${cutAt(root, MAX_DI - head.length, legacy)}`; + return `${head}${cutAt(root, MAX_DI - head.length)}`; } export function namesRoot(kind: 'Plane' | 'Diagram', diId: string | undefined, root: string): boolean { - return diId === `BPMN${kind}_${root}` || diId === rootDiId(kind, root) || diId === rootDiId(kind, root, true); + return diId === `BPMN${kind}_${root}` || diId === rootDiId(kind, root); } /** Then DiIdMemo keeps the engine's ids for a drawing without old DI. */ diff --git a/src/domain/document/ids/IdFitter.ts b/src/domain/document/ids/IdFitter.ts index 0b94b551..ee6a33cd 100644 --- a/src/domain/document/ids/IdFitter.ts +++ b/src/domain/document/ids/IdFitter.ts @@ -1,4 +1,4 @@ -import { camelSlug, cutAt, idWords, isFiller, keptTail, MAX_ID, nameWords, pascalSnakeSlug, readsApart, slugify, snakeSlug, withLetter } from '../utils/slug.js'; +import { camelSlug, idWords, isFiller, keptTail, MAX_ID, nameWords, pascalSnakeSlug, readsApart, slugify, snakeSlug, withLetter } from '../utils/slug.js'; import { beginAlike, lcfirst, speakingStem, stem, ucfirst } from '../utils/id-words.js'; import type { Body } from './IdStyle.js'; import type { IdRequest } from '../utils/requests.js'; @@ -7,9 +7,6 @@ export type IdForm = 'stemTo' | 'idPair' | 'stemSnake' | 'idSnake'; export const FORM_JOIN: Record = { stemTo: 'To', idPair: '_', stemSnake: '_to_', idSnake: '_to_' }; export const ID_FORMS = Object.keys(FORM_JOIN) as IdForm[]; -/** The longest base id 0.6 generated, so the flows it cut are still recognised. */ -export const LEGACY_BASE = MAX_ID - 3; - interface EndForm { text: string; words: number; @@ -21,48 +18,45 @@ interface EndForm { */ export class IdFitter { private readonly body: Body; - private readonly legacy: boolean; /** the numbers the closing end does not end on, as they would look like a collision suffix */ private readonly closes: RegExp; private readonly names: string[] | undefined; - constructor(opts: { body?: Body; legacy?: boolean; suffixSep?: string; names?: string[] } = {}) { + constructor(opts: { body?: Body; suffixSep?: string; names?: string[] } = {}) { this.body = opts.body ?? 'pascal'; - this.legacy = opts.legacy ?? false; this.closes = (opts.suffixSep ?? '_') === '' ? /^[_.-]*\d+$/ : /^_\d+$/; this.names = opts.names; } ends(source: string, target: string, glued: boolean): [string, string, string] { - const legacy = this.legacy; switch (this.body) { case 'camel': { - const sourceSlug = camelSlug(source, legacy); - return [glued ? ucfirst(sourceSlug) : sourceSlug, 'To', slugify(target, legacy)]; + const sourceSlug = camelSlug(source); + return [glued ? ucfirst(sourceSlug) : sourceSlug, 'To', slugify(target)]; } case 'snake': - return [snakeSlug(source, legacy), '_to_', snakeSlug(target, legacy)]; + return [snakeSlug(source), '_to_', snakeSlug(target)]; case 'pascalSnake': - return [pascalSnakeSlug(source, legacy), '_To_', pascalSnakeSlug(target, legacy)]; + return [pascalSnakeSlug(source), '_To_', pascalSnakeSlug(target)]; default: - return [slugify(source, legacy), 'To', slugify(target, legacy)]; + return [slugify(source), 'To', slugify(target)]; } } /** * `long`: the head leaves the ends fewer than 16 characters, so it stays only where they fit - * beside it at whole words (else Flow_; 0.6 always kept it). `glued`: a lower-case prefix + * beside it at whole words (else Flow_). `glued`: a lower-case prefix * without separator glues the words too (flowCheckToShip); an upper-case one reads apart (F_CheckToShip). */ namedHead(prefix: string, sep: string | undefined): { head: string; glued: boolean; long: boolean } { const glued = sep === '' && /[a-z]/.test(prefix); const joined = this.body === 'snake' || this.body === 'pascalSnake' ? 4 : 2; - return { head: `${prefix}${glued ? '' : '_'}`, glued, long: !this.legacy && MAX_ID - prefix.length - (glued ? 0 : 1) - joined < 16 }; + return { head: `${prefix}${glued ? '' : '_'}`, glued, long: MAX_ID - prefix.length - (glued ? 0 : 1) - joined < 16 }; } pair(head: string, sourceEnd: string, join: string, targetEnd: string, max: number): string { const room = max - head.length - join.length; - const [fittedSource, fittedTarget] = this.legacy ? this.legacyFitPair(sourceEnd, targetEnd, room) : this.fitPair(sourceEnd, targetEnd, Math.max(room, 0)); + const [fittedSource, fittedTarget] = this.fitPair(sourceEnd, targetEnd, Math.max(room, 0)); return `${head}${fittedSource}${join}${fittedTarget}`; } @@ -84,7 +78,7 @@ export class IdFitter { const pb = targetId.length - stem(targetId).length; const room = max - head.length - join.length - pa - pb; const fitter = this.ofForm(form); - if (this.legacy || room < 16) return fitter.pair(head, sourceId, join, targetId, max); + if (room < 16) return fitter.pair(head, sourceId, join, targetId, max); const [fittedSource, fittedTarget] = fitter.fitPair(stem(sourceId), stem(targetId), room); return `${head}${sourceId.slice(0, pa)}${fittedSource}${join}${targetId.slice(0, pb)}${fittedTarget}`; } @@ -107,7 +101,7 @@ export class IdFitter { /** An end's id stem when that says something, else its label. */ stemEnds(form: 'stemTo' | 'stemSnake', req: IdRequest): { sourceEnd: string; targetEnd: string; names?: string[] } { - const slug = (text: string): string => (form === 'stemTo' ? camelSlug(text, this.legacy) : snakeSlug(text, this.legacy)); + const slug = (text: string): string => (form === 'stemTo' ? camelSlug(text) : snakeSlug(text)); const byLabel = (id: string, label: string | undefined): label is string => speakingStem(id) === undefined && !!label && !!slug(label); const end = (id: string, label: string | undefined): string => speakingStem(id) ?? (byLabel(id, label) ? slug(label) : stem(id)); const [sourceEnd, targetEnd] = [end(req.source!, req.sourceLabel), end(req.target!, req.targetLabel)]; @@ -123,12 +117,12 @@ export class IdFitter { const segments = id.split('_'); if (segments.length > 1 && new RegExp(`^${terminal}(Event)?$`, 'i').test(segments[0]!)) return terminal; const rest = segments.length > 1 ? segments.slice(1).filter((segment) => segment !== scope) : segments; - const word = (words: string[]): string | undefined => (this.legacy ? words[0] : words.find((candidate) => /[A-Za-z]/.test(candidate))); + const word = (words: string[]): string | undefined => words.find((candidate) => /[A-Za-z]/.test(candidate)); return ucfirst(word(nameWords(rest.join(' '))) ?? word(nameWords(id)) ?? terminal); } private ofForm(form: IdForm, names?: string[]): IdFitter { - return new IdFitter({ legacy: this.legacy, suffixSep: form === 'stemTo' ? '' : '_', ...(names ? { names } : {}) }); + return new IdFitter({ suffixSep: form === 'stemTo' ? '' : '_', ...(names ? { names } : {}) }); } /** @@ -247,30 +241,10 @@ export class IdFitter { return withLetter(slug, slug.slice(0, Math.max(max, 0)).replace(/[_.-]+$/, ''), (rest) => this.cutInside(rest, max)); } - /** - * fitPair as 0.6 cut the ends, to recognise the flows it named. Ends that begin alike differ at - * their ends, so each keeps its last word. - */ - private legacyFitPair(sourceEnd: string, targetEnd: string, room: number): [string, string] { - if (sourceEnd.length + targetEnd.length <= room) return [sourceEnd, targetEnd]; - return this.sharePair(sourceEnd, targetEnd, room, beginAlike(sourceEnd, targetEnd) ? (slug, max) => this.cutKeepingLast(slug, max) : (slug, max) => this.legacyCut(slug, max)); - } - /** The first end gets at least half, the second the rest, the first what the second leaves. */ private sharePair(sourceEnd: string, targetEnd: string, room: number, cut: (slug: string, max: number) => string): [string, string] { const first = cut(sourceEnd, Math.max(Math.floor(room / 2), room - targetEnd.length)); const second = cut(targetEnd, room - first.length); return [cut(sourceEnd, room - second.length), second]; } - - private cutKeepingLast(slug: string, max: number): string { - if (slug.length <= max) return slug; - const match = /^(.*[a-z0-9])([_.-][A-Za-z0-9]+|[A-Z][a-z0-9]+)$/.exec(slug); - if (!match || match[2]!.length + 8 > max) return this.legacyCut(slug, max); - return `${this.legacyCut(match[1]!, max - match[2]!.length)}${match[2]}`; - } - - private legacyCut(slug: string, max: number): string { - return cutAt(slug, max, true); - } } diff --git a/src/domain/document/ids/IdStyle.ts b/src/domain/document/ids/IdStyle.ts index 4319a5cd..1eac5e63 100644 --- a/src/domain/document/ids/IdStyle.ts +++ b/src/domain/document/ids/IdStyle.ts @@ -1,6 +1,6 @@ import { camelSlug, cutAt, cutName, hasIdWords, idWords, isFiller, isValidId, MAX_ID, MAX_SLUG, nameWords, pascalSnakeSlug, slugify, snakeSlug, withSuffix } from '../utils/slug.js'; import type { IdRegistry } from './IdRegistry.js'; -import { FORM_JOIN, IdFitter, LEGACY_BASE } from './IdFitter.js'; +import { FORM_JOIN, IdFitter } from './IdFitter.js'; import { caseLike, CONTEXT_WORDS, kindWord, PREFIXED, speakingStem, stemWords } from '../utils/id-words.js'; import { CONNECTIONS, connectionRequest, kindRequest, typeRequest, type IdRequest } from '../utils/requests.js'; import type { IdConventions } from './IdConventionsReader.js'; @@ -283,8 +283,8 @@ export class IdStyle { } /** - * Every id the request may have been given before (uncut, cut for a suffix, as 0.6 cut it, in - * every other flow style the file shows), so a flow named earlier still names its ends. + * Every id the request may have been given before (uncut, cut for a suffix, in every other + * flow style the file shows), so a flow named earlier still names its ends. * `id` limits this to the styles it can belong to. */ derivedBases(req: IdRequest, id?: string): string[] { @@ -304,10 +304,9 @@ export class IdStyle { } private connectionBases(req: IdRequest, style: FlowStyle): string[] { - const at = (max: number, legacy = false): string => this.connectionBase(req, max, legacy, style).base; + const at = (max: number): string => this.connectionBase(req, max, style).base; const uncut = at(Infinity); - const legacy = at(LEGACY_BASE, true); - return [...(uncut.length <= MAX_ID - 4 ? [] : [0, 1, 2, 3, 4].map((suffix) => at(MAX_ID - suffix))), legacy, cutAt(legacy, MAX_ID - 4, true), at(Infinity, true), uncut]; + return [...(uncut.length <= MAX_ID - 4 ? [] : [0, 1, 2, 3, 4].map((suffix) => at(MAX_ID - suffix))), uncut]; } private wanted(req: IdRequest, max = MAX_ID): { base: string; sep: string } { @@ -426,29 +425,28 @@ export class IdStyle { /** * A head that leaves the ends under 16 characters stays per flow where they fit beside it at - * whole words, else gives way to Flow_, so one file can hold both forms (0.6 kept the head). + * whole words, else gives way to Flow_, so one file can hold both forms. */ - private connectionBase(req: IdRequest, max: number, legacy = false, style = this.flowOf(req.key)): { base: string; sep: string } { + private connectionBase(req: IdRequest, max: number, style = this.flowOf(req.key)): { base: string; sep: string } { const { form, prefix, sep, scope, head } = style; - if (head !== undefined) return this.declaredFlowBase(req, max, form, head, legacy); - if (form === 'named') return { base: this.namedBase(req, max, prefix, sep, legacy), sep: '_' }; - const long = (head: string, join: string): boolean => !legacy && MAX_ID - head.length - join.length < 16; + if (head !== undefined) return this.declaredFlowBase(req, max, form, head); + if (form === 'named') return { base: this.namedBase(req, max, prefix, sep), sep: '_' }; + const long = (head: string, join: string): boolean => MAX_ID - head.length - join.length < 16; if (form === 'scopedTo') { const sourceId = req.source!; const targetId = req.target!; const own = this.scopeOf(req, scope!); const head = `${prefix}_${own}_`; - const fitter = new IdFitter({ legacy, suffixSep: '' }); + const fitter = new IdFitter({ suffixSep: '' }); const [sourceWord, targetWord] = [fitter.endWord(sourceId, own, 'Start'), fitter.endWord(targetId, own, 'End')]; - if (legacy) return { base: cutAt(`${head}${sourceWord}To${targetWord}`, max, true), sep: '' }; const base = long(head, 'To') ? fitter.whole(head, sourceWord, 'To', targetWord, max) : fitter.pair(head, sourceWord, 'To', targetWord, max); - return base !== undefined ? { base, sep: '' } : { base: this.namedBase({ ...req, sourceLabel: sourceWord, targetLabel: targetWord }, max, DEFAULT_FLOW.prefix, undefined, false), sep: '_' }; + return base !== undefined ? { base, sep: '' } : { base: this.namedBase({ ...req, sourceLabel: sourceWord, targetLabel: targetWord }, max, DEFAULT_FLOW.prefix, undefined), sep: '_' }; } const own = form === 'stemTo' ? '' : '_'; - const fitter = new IdFitter({ legacy }); + const fitter = new IdFitter(); if (!long(`${prefix}_`, FORM_JOIN[form])) return { base: fitter.idForm(form, `${prefix}_`, req, max), sep: own }; const base = fitter.wholeIdForm(form, `${prefix}_`, req, max); - return base !== undefined ? { base, sep: own } : { base: this.namedBase(req, max, DEFAULT_FLOW.prefix, undefined, false), sep: '_' }; + return base !== undefined ? { base, sep: own } : { base: this.namedBase(req, max, DEFAULT_FLOW.prefix, undefined), sep: '_' }; } /** The scope an end names in its id (KotO in serviceTask_KotO_Validate), else the most used. */ @@ -461,7 +459,7 @@ export class IdStyle { * A declared head never gives way to Flow_. The ends leave out the file's scope segment, and * one that begins with a number is written `From...` where the case wants a letter first. */ - private declaredFlowBase(req: IdRequest, max: number, form: FlowForm, head: string, legacy: boolean): { base: string; sep: string } { + private declaredFlowBase(req: IdRequest, max: number, form: FlowForm, head: string): { base: string; sep: string } { const sep = this.sepFor(req); if (form === 'named') { const bodyCase = this.caseFor(req); @@ -472,29 +470,29 @@ export class IdStyle { }; const source = label(req.source!, req.sourceLabel); const target = label(req.target!, req.targetLabel); - const fitter = new IdFitter({ body: bodyCase, legacy, suffixSep: sep, ...(bodyCase === 'camel' ? { names: nameWords(source) } : {}) }); + const fitter = new IdFitter({ body: bodyCase, suffixSep: sep, ...(bodyCase === 'camel' ? { names: nameWords(source) } : {}) }); let [sourceEnd, join, targetEnd] = fitter.ends(source, target, false); if (this.lettersOnly(req) && /^[0-9]/.test(sourceEnd)) sourceEnd = fitter.ends(`From ${source}`, target, false)[0]; return { base: fitter.pair(head, sourceEnd, join, targetEnd, max), sep }; } if (form === 'scopedTo') { const own = this.scopeOf(req, this.flowOf(req.key).scope!); - const fitter = new IdFitter({ legacy, suffixSep: sep }); + const fitter = new IdFitter({ suffixSep: sep }); const [sourceWord, targetWord] = [fitter.endWord(req.source!, own, 'Start'), fitter.endWord(req.target!, own, 'End')]; - return { base: legacy ? cutAt(`${head}${own}_${sourceWord}To${targetWord}`, max, true) : fitter.pair(`${head}${own}_`, sourceWord, 'To', targetWord, max), sep }; + return { base: fitter.pair(`${head}${own}_`, sourceWord, 'To', targetWord, max), sep }; } - return { base: new IdFitter({ legacy }).idForm(form, head, req, max), sep }; + return { base: new IdFitter().idForm(form, head, req, max), sep }; } - private namedBase(req: IdRequest, max: number, prefix: string, sep: string | undefined, legacy: boolean): string { + private namedBase(req: IdRequest, max: number, prefix: string, sep: string | undefined): string { const label = (id: string, given: string | undefined): string => (given && nameWords(given).length ? given : (speakingStem(id) ?? id)); const bodyCase = this.caseFor(req); - const { head, glued, long } = new IdFitter({ body: bodyCase, legacy }).namedHead(prefix, sep); + const { head, glued, long } = new IdFitter({ body: bodyCase }).namedHead(prefix, sep); const source = label(req.source!, req.sourceLabel); - const fitter = new IdFitter({ body: bodyCase, legacy, ...(bodyCase === 'camel' && !glued ? { names: nameWords(source) } : {}) }); + const fitter = new IdFitter({ body: bodyCase, ...(bodyCase === 'camel' && !glued ? { names: nameWords(source) } : {}) }); const [sourceEnd, join, targetEnd] = fitter.ends(source, label(req.target!, req.targetLabel), glued); if (!long) return fitter.pair(head, sourceEnd, join, targetEnd, max); - return fitter.whole(head, sourceEnd, join, targetEnd, max) ?? this.namedBase(req, max, DEFAULT_FLOW.prefix, undefined, legacy); + return fitter.whole(head, sourceEnd, join, targetEnd, max) ?? this.namedBase(req, max, DEFAULT_FLOW.prefix, undefined); } private isConnection(req: IdRequest): boolean { diff --git a/src/domain/document/ids/LearnedStyle.ts b/src/domain/document/ids/LearnedStyle.ts index 0752d384..66e9dacf 100644 --- a/src/domain/document/ids/LearnedStyle.ts +++ b/src/domain/document/ids/LearnedStyle.ts @@ -1,10 +1,10 @@ import { kindOf, triggerOf } from '../utils/kinds.js'; import { is, isBpmnElement, walk, type El } from '../utils/moddle.js'; -import { cutAt, isSuffix, MAX_ID, nameWords, pascalSnakeSlug, snakeSlug, SUFFIX_WORD } from '../utils/slug.js'; +import { isSuffix, MAX_ID, nameWords, pascalSnakeSlug, snakeSlug, SUFFIX_WORD } from '../utils/slug.js'; import { beginAlike, caseLike, GENERIC, GLUED, HASH, lcfirst, localName, PREFIXED, prefixClass, stem, TOOL_DEFAULTS, ucfirst, type PrefixClass } from '../utils/id-words.js'; import { CONNECTIONS, connectionRequest, kindTypeNames, type IdRequest } from '../utils/requests.js'; import { ElementLabels } from './ElementLabels.js'; -import { ID_FORMS, IdFitter, LEGACY_BASE, FORM_JOIN, type IdForm } from './IdFitter.js'; +import { ID_FORMS, IdFitter, FORM_JOIN, type IdForm } from './IdFitter.js'; import { MIN_EVIDENCE, Tally } from './Tally.js'; import type { DeclaredConventions, Votes } from './DeclaredConventions.js'; import type { Body, FlowForm, FlowStyle, PrefixSource } from './IdStyle.js'; @@ -21,8 +21,6 @@ const GLUED_SUFFIXED = /^[A-Za-z][A-Za-z0-9]*To[A-Z0-9][A-Za-z0-9]*_\d+$/; const GLUED_NAMED = /^[A-Za-z][A-Za-z0-9]*To(?![a-z])[A-Za-z0-9]*$/; /** May be a glued flow in a snake_case / Pascal_Snake body (`fweigh_parcel_to_label_parcel`). */ const GLUED_JOINED = /^[a-z]\w*_[tT]o_\w*$/; -/** A scopedTo flow 0.6 cut to an empty target word (Flow_KotO_WarenwirtschaftssystemabgleichTo). */ -const SCOPED_CUT = /^([A-Za-z][A-Za-z0-9]*)_([A-Z][A-Za-z0-9]*?)To\d*$/; type BodyClass = Body | 'hash' | 'numbered' | 'lower' | 'other'; @@ -392,8 +390,6 @@ export class LearnedStyle { let req: IdRequest | undefined; const request = (): IdRequest => (req ??= connectionRequest(el.$type as 'bpmn:SequenceFlow' | 'bpmn:MessageFlow', source, target, { sourceLabel: new ElementLabels(el.get('sourceRef')).label(), targetLabel: new ElementLabels(el.get('targetRef')).label() })); for (const form of ID_FORMS) if (!seen.some((seenForm) => seenForm.form === form) && this.cutOfForm(id, form, prefix, request)) seen.push({ form, prefix }); - const scope = seen.some((seenForm) => seenForm.form === 'scopedTo') ? undefined : this.legacyScopedCut(id, prefix, source, target); - if (scope) seen.push({ form: 'scopedTo', prefix, scope }); } for (const seenForm of seen) { votes.forms.add(seenForm.form); @@ -483,18 +479,16 @@ export class LearnedStyle { for (const [join, slug] of [['_to_', snakeSlug], ['_To_', pascalSnakeSlug]] as const) { if (!id.includes(join) || id.charAt(prefix.length) !== slug(ends.source()).charAt(0)) continue; if (id.startsWith(`${prefix}${slug(ends.source())}${join}`)) return true; - const at = (max: number, legacy = false): string => new IdFitter({ legacy }).pair(prefix, slug(ends.source(), legacy), join, slug(ends.target(), legacy), max); + const at = (max: number): string => new IdFitter().pair(prefix, slug(ends.source()), join, slug(ends.target()), max); const uncut = at(Infinity); if (fits(uncut) || (uncut.length > MAX_ID - 4 && [0, 1, 2, 3, 4].some((trim) => fits(at(MAX_ID - trim))))) return true; - const legacy = at(LEGACY_BASE, true); - if (fits(legacy) || fits(cutAt(legacy, MAX_ID - 4, true))) return true; } return false; } private otherStyles(votes: FlowVotes, style: FlowStyle): FlowStyle[] { const out = new Map(); - const key = (flowStyle: FlowStyle): string => (flowStyle.form === 'named' ? `named|${new IdFitter({ legacy: true }).namedHead(flowStyle.prefix, flowStyle.sep).head}` : `${flowStyle.form}|${flowStyle.prefix}|${flowStyle.scope ?? ''}`); + const key = (flowStyle: FlowStyle): string => (flowStyle.form === 'named' ? `named|${new IdFitter().namedHead(flowStyle.prefix, flowStyle.sep).head}` : `${flowStyle.form}|${flowStyle.prefix}|${flowStyle.scope ?? ''}`); const add = (flowStyle: FlowStyle): void => { if (!out.has(key(flowStyle))) out.set(key(flowStyle), flowStyle); }; @@ -529,9 +523,6 @@ export class LearnedStyle { const fitter = new IdFitter(); if (fitter.idForm(form, `${prefix}_`, req, Infinity).length <= MAX_ID - 4) return false; const sep = form === 'stemTo' ? '' : '_'; - const suffixed = (base: string): boolean => id.startsWith(base) && isSuffix(base, id.slice(base.length), sep); - const legacy = new IdFitter({ legacy: true }).idForm(form, `${prefix}_`, req, LEGACY_BASE); - if (id === legacy || suffixed(legacy) || suffixed(cutAt(legacy, MAX_ID - 4, true))) return true; if (id === fitter.idForm(form, `${prefix}_`, req, MAX_ID)) return true; const digits = (sep === '' ? /\d+$/ : /_\d+$/).exec(id)?.[0] ?? ''; const ks: number[] = []; @@ -544,17 +535,4 @@ export class LearnedStyle { } return false; } - - /** SCOPED_TO does not read a flow 0.6 cut to an empty target word, so this does. */ - private legacyScopedCut(id: string, prefix: string, source: string, target: string): string | undefined { - const match = SCOPED_CUT.exec(id.slice(prefix.length + 1)); - if (!match) return undefined; - const scope = match[1]!; - const fitter = new IdFitter({ legacy: true }); - const sourceWord = fitter.endWord(source, scope, 'Start'); - if (!match[2]!.startsWith(sourceWord)) return undefined; - const legacy = cutAt(`${prefix}_${scope}_${sourceWord}To${fitter.endWord(target, scope, 'End')}`, LEGACY_BASE, true); - const suffixed = (base: string): boolean => id.startsWith(base) && isSuffix(base, id.slice(base.length), ''); - return id === legacy || suffixed(legacy) || suffixed(cutAt(legacy, MAX_ID - 4, true)) ? scope : undefined; - } } diff --git a/src/domain/document/utils/slug.ts b/src/domain/document/utils/slug.ts index 76e8a119..8839fc3f 100644 --- a/src/domain/document/utils/slug.ts +++ b/src/domain/document/utils/slug.ts @@ -79,27 +79,26 @@ const KEPT_TAIL = /(?:(?<=[a-z0-9])Join|_[Jj]oin)?(?:(?<=[A-Za-z])\d+|_\d+)?$/; /** * Cuts at the last word boundary that fits, keeping a join or number tail. A cut keeps a letter - * and never runs a number into the kept one (`1010`). `legacy` cuts as 0.6 did, to recognise - * the ids it wrote. + * and never runs a number into the kept one (`1010`). * A filler word between a number and the number tail keeps them apart (`StartAfter2Und2`). */ -export function cutAt(slug: string, max: number, legacy = false, names = false): string { +export function cutAt(slug: string, max: number, names = false): string { if (slug.length <= max) return slug; const tail = keptTail(slug); if (tail && max - tail.length >= 8) { const head = slug.slice(0, -tail.length); - const cut = cutWords(head, max - tail.length, legacy, names); - if (legacy || readsApart(cut, tail)) return `${cut}${tail}`; + const cut = cutWords(head, max - tail.length, names); + if (readsApart(cut, tail)) return `${cut}${tail}`; const kept = names ? cutWords(head, max - tail.length) : cut; if (readsApart(kept, tail)) return `${kept}${tail}`; const inside = trimSeparators(head.slice(0, max - tail.length)); if (readsApart(inside, tail)) return `${inside}${tail}`; } - return cutWords(slug, max, legacy, names); + return cutWords(slug, max, names); } export function cutName(slug: string, max: number): string { - return cutAt(slug, max, false, true); + return cutAt(slug, max, true); } /** The head has a letter and does not run into a number tail as one number (`Stufe2` + `3`). */ @@ -109,9 +108,8 @@ export function readsApart(head: string, tail: string): boolean { const trimSeparators = (slug: string): string => slug.replace(/[_.-]+$/, ''); -function cutWords(slug: string, max: number, legacy = false, names = false): string { +function cutWords(slug: string, max: number, names = false): string { if (slug.length <= max) return slug; - if (legacy) return legacyCutWords(slug, max); const words: string[] = []; let length = 0; for (const word of idWords(slug)) { @@ -122,7 +120,7 @@ function cutWords(slug: string, max: number, legacy = false, names = false): str if (names) while (words.length > 1 && isFiller(words[words.length - 1]!) && words.slice(0, -1).some((word) => /[A-Za-z]/.test(word))) words.pop(); const out = words.join(''); if (out && /[A-Za-z]/.test(out)) return trimSeparators(out); - return trimSeparators(withLetter(slug, slug.slice(0, Math.max(max, 0)), (rest) => cutWords(rest, max, false, names))); + return trimSeparators(withLetter(slug, slug.slice(0, Math.max(max, 0)), (rest) => cutWords(rest, max, names))); } /** A cut that keeps no letter (a leading number) starts at the first word with a letter instead. */ @@ -133,18 +131,6 @@ export function withLetter(slug: string, cut: string, again: (rest: string) => s return at > 0 ? again(words.slice(at).join('').replace(/^[_.-]+/, '')) : cut; } -/** cutWords as 0.6 cut: also at the boundary right after a separator. */ -function legacyCutWords(slug: string, max: number): string { - let best = 0; - for (let index = 1; index <= max; index++) { - const prev = slug.charAt(index - 1); - const character = slug.charAt(index); - const boundary = /[_.-]/.test(character) || (/[A-Z]/.test(character) && /[a-z0-9]/.test(prev)) || (/[0-9]/.test(character) && /[A-Za-z]/.test(prev)) || /[_.-]/.test(prev); - if (boundary) best = index; - } - return trimSeparators(best > 0 ? slug.slice(0, best) : slug.slice(0, max)); -} - /** Each word keeps the separator before it, so joined they give the slug again. */ export function idWords(slug: string): string[] { const out: string[] = []; @@ -168,33 +154,32 @@ export function keptTail(slug: string): string { return tail.length < slug.length ? tail : ''; } -function cut(slug: string, legacy = false): string { - return legacy ? cutAt(slug, MAX_SLUG, true) : cutName(slug, MAX_SLUG); +function cut(slug: string): string { + return cutName(slug, MAX_SLUG); } -export function slugify(name: string | undefined, legacy = false): string { - return cut(nameWords(name).map(cap).join(''), legacy); +export function slugify(name: string | undefined): string { + return cut(nameWords(name).map(cap).join('')); } -export function camelSlug(name: string | undefined, legacy = false): string { +export function camelSlug(name: string | undefined): string { const words = nameWords(name); if (!words.length) return ''; const [first, ...rest] = words; const head = /^[A-Z0-9]+$/.test(first!) ? first!.toLowerCase() : first!.charAt(0).toLowerCase() + first!.slice(1); - return cut(head + rest.map(cap).join(''), legacy); + return cut(head + rest.map(cap).join('')); } -export function snakeSlug(name: string | undefined, legacy = false): string { +export function snakeSlug(name: string | undefined): string { return cut( nameWords(name) .map((word) => word.toLowerCase()) .join('_'), - legacy, ); } -export function pascalSnakeSlug(name: string | undefined, legacy = false): string { - return cut(nameWords(name).map(cap).join('_'), legacy); +export function pascalSnakeSlug(name: string | undefined): string { + return cut(nameWords(name).map(cap).join('_')); } /** `step1No2`, never `step12`, which reads as the id of Step 12. */ diff --git a/test/domain/diagram/conventions-di.test.ts b/test/domain/diagram/conventions-di.test.ts index 32ca9cbf..b82ee19e 100644 --- a/test/domain/diagram/conventions-di.test.ts +++ b/test/domain/diagram/conventions-di.test.ts @@ -194,7 +194,7 @@ describe('every DI id bpmn-cli writes stays within 64 characters', () => { expect(diOf(await write(xml, [], { layout: 'full' })).shapes.get('A')).toBe(long); }); - it('a plane and diagram id cut from a long process id follow a new collaboration root, also as 0.6 cut them (both engines)', async () => { + it('a plane and diagram id cut from a long process id follow a new collaboration root (both engines)', async () => { const ops: EditCommand[] = [ { op: 'add', kind: 'startEvent', id: 'S', name: 'Start' }, { op: 'add', kind: 'task', id: 'A', name: 'Check', after: 'S' }, @@ -212,13 +212,6 @@ describe('every DI id bpmn-cli writes stays within 64 characters', () => { } } } - // a plane 0.6 cut in an incremental drawing (diIds), its root ending in a number - const pid = 'Process_Pruefung10Lieferantenbestellungsuebersichtsverwaltung20'; - const fresh = (await mutateDoc(Doc.create({ processId: pid }), ops, { dryRun: true })).xml; - const legacy = fresh.replace(/id="BPMNPlane_[^"]+"/, `id="BPMNPlane_${cutAt(pid, 51, true)}"`).replace(/id="BPMNDiagram_[^"]+"/, `id="BPMNDiagram_${cutAt(pid, 49, true)}"`); - expect(cutAt(pid, 51, true)).not.toBe(cutAt(pid, 51)); - const after = diOf(await write(legacy, [pool], { layout: 'full' })); - expect(after.planes[0]).toMatch(/^BPMNPlane_Collaboration_/); }); it('a sub-process collapsed without a drawing (--no-layout) records it under a plane and diagram id within 64 characters', async () => { @@ -226,22 +219,4 @@ describe('every DI id bpmn-cli writes stays within 64 characters', () => { expect(diOf(outcome.xml).planes).toEqual([`BPMNPlane_${cutAt(LONG_PROCESS, 51)}`]); expect(diOf(outcome.xml).diagrams).toEqual([`BPMNDiagram_${cutAt(LONG_PROCESS, 49)}`]); }); - - it('a DI edge id 0.6 cut to bare numbers (BPMNEdge_Flow_Pruefung10To2030) follows its flow renamed after new ends', async () => { - const ops: EditCommand[] = [ - { op: 'add', kind: 'task', name: 'Prüfung 10 Lieferantenbestellungsuebersicht', as: '$a' }, - { op: 'add', kind: 'task', name: '20 Lieferantenbestellungsuebersicht 30', after: '$a' }, - ]; - const drawn = (await mutateDoc(Doc.create({ processId: 'P' }), ops, { dryRun: true })).xml; - const flow = 'Flow_Pruefung10To20Lieferantenbestellungsuebersicht30'; - // as 0.6 cut it (now: BPMNEdge_Flow_Pruefung10To20Lieferantenbestellungsuebersich30) - const old = drawn.replace(/id="BPMNEdge_Flow_Pruefung10To[^"]+"/, 'id="BPMNEdge_Flow_Pruefung10To2030"'); - expect(diOf(old).shapes.get(flow)).toBe('BPMNEdge_Flow_Pruefung10To2030'); - const outcome = await mutateDoc(await Doc.fromXml(old, 'x.bpmn'), [{ op: 'add', kind: 'task', name: 'Freigabe', flow }], { dryRun: true }); - expect(outcome.xml).toContain(''); - expect(diOf(outcome.xml).shapes.get('Flow_Pruefung10ToFreigabe')).toBe('BPMNEdge_Flow_Pruefung10ToFreigabe'); - // and with `set id=` - const set = await mutateDoc(await Doc.fromXml(old, 'x.bpmn'), [{ op: 'set', id: flow, values: { id: 'Flow_Check' } }], { dryRun: true }); - expect(diOf(set.xml).shapes.get('Flow_Check')).toBe('BPMNEdge_Flow_Check'); - }); }); diff --git a/test/domain/document/conventions-ids.test.ts b/test/domain/document/conventions-ids.test.ts index 7a247a86..5ba8e199 100644 --- a/test/domain/document/conventions-ids.test.ts +++ b/test/domain/document/conventions-ids.test.ts @@ -627,32 +627,6 @@ describe('a file\'s flow form stays put, and flows named after their ends follow const moved = runOps(await Doc.fromXml(forms), [{ op: 'add', kind: 'task', name: 'Log', after: 'Activity_E' }]); expect(moved.renames).toEqual([expect.objectContaining({ from: 'SF_Activity_E_Activity_F', to: 'Flow_EToLog' })]); }); - - it("0.6's scopedTo flows cut to an empty target word vote for the form: a 0.6 file keeps it, and they follow their ends", async () => { - const long = ['Warenwirtschaftssystemabgleich', 'Kundenstammdatenaktualisierung', 'Lieferantenbestellungsuebersicht', 'Zahlungseingangsbestaetigung']; - const short = ['Validate', 'Reserve', 'Ship']; - const els: string[] = []; - const flows: Array<[string, string, string]> = []; - short.forEach((word, wordIndex) => { - els.push(``, ``); - flows.push([`Flow_KotO_${word}${wordIndex}xTo${short[(wordIndex + 1) % 3]}Y${wordIndex}`, `serviceTask_KotO_${word}${wordIndex}x`, `serviceTask_KotO_${short[(wordIndex + 1) % 3]}Y${wordIndex}`]); - }); - long.forEach((word, wordIndex) => { - els.push(``, ``); - // 0.6 cut Flow_KotO_To to 61 characters at whole words: the target went - flows.push([`Flow_KotO_${word}To`, `serviceTask_KotO_${word}_${wordIndex}`, `serviceTask_KotO_${long[(wordIndex + 1) % 4]}_${wordIndex}`]); - }); - const xml = definitionsXml([...els, ...flows.map(([id, source, target]) => ``)].join('\n')); - // before: four of seven flows matched no form, and the file read as named Flow_ - const doc = await Doc.fromXml(xml); - expect(doc.ownIdStyle.info().sequenceFlow).toEqual({ form: 'scopedTo', prefix: 'Flow', scope: 'KotO' }); - const cs = runOps(doc, [{ op: 'add', kind: 'serviceTask', id: 'serviceTask_KotO_Pack', name: 'Pack', flow: 'Flow_KotO_WarenwirtschaftssystemabgleichTo' }]); - expect(cs.renames).toEqual([expect.objectContaining({ from: 'Flow_KotO_WarenwirtschaftssystemabgleichTo', to: 'Flow_KotO_WarenwirtschaftssystemabgleichToPack' })]); - expect(cs.created.map((created) => created.id)).toEqual(['serviceTask_KotO_Pack', 'Flow_KotO_PackToKundenstammdatenaktualisierung']); - // a shape alone is no vote: `_To` whose A is not the source's end word - const other = xml.replace(/Flow_KotO_([A-Za-z]+)To"/g, 'Flow_KotO_Other$1To"'); - expect((await Doc.fromXml(other)).ownIdStyle.info().sequenceFlow).toEqual({ form: 'named', prefix: 'Flow' }); - }); }); describe('flow ids kept (keepFlowIds): a stale id is recognised in every flow form', () => { diff --git a/test/domain/document/id-quality.test.ts b/test/domain/document/id-quality.test.ts index 381f8c3f..3e2f93f2 100644 --- a/test/domain/document/id-quality.test.ts +++ b/test/domain/document/id-quality.test.ts @@ -812,33 +812,13 @@ describe('id quality: context once, length cap, transliteration', () => { expect(spliced.result.renamed).toEqual({ Flow_billOrderToCheckOrderJoin: 'Flow_billOrderToArchiveOrder' }); }); - it('#31: flows 0.6 named still follow their ends where the file now reads as another form, also as 0.6 cut them', async () => { + it('#31: flows 0.6 named still follow their ends where the file now reads as another form', async () => { /** a chain of tasks ([id, name]) joined by the given flows */ const chainXml = (tasks: Array<[string, string]>, flows: string[]): string => definitionsXml( tasks.map(([id, name], index) => `${index ? `${flows[index - 1]}` : ''}${index < flows.length ? `${flows[index]}` : ''}`).join('\n') + flows.map((id, index) => ``).join('\n'), ); - // an idPair file 0.6 read as `named` once it had cut three of its flows, and gave two named flows; the cut ones vote - // for idPair again (before: Flow_PackOrderToArchiveOrder kept its id on the flow to Activity_WeighParcel) - const idPair = chainXml( - [ - ['Activity_CheckOrder', 'Check order'], - ['Activity_ShipOrder', 'Ship order'], - ['Activity_BillOrder', 'Bill order'], - ['Activity_ZahlungseingangsbestaetigungenPruefen', 'Zahlungseingangsbestaetigungen pruefen'], - ['Activity_Lieferantenbestellungsuebersicht', 'Lieferantenbestellungsuebersicht erstellen'], - ['Activity_KundenstammdatenaktualisierungAnstossen', 'Kundenstammdatenaktualisierung anstossen'], - ['Activity_PackOrder', 'Pack order'], - ['Activity_ArchiveOrder', 'Archive order'], - ], - ['Flow_Activity_CheckOrder_Activity_ShipOrder', 'Flow_Activity_ShipOrder_Activity_BillOrder', 'Flow_Activity_BillOrder_ActivityPruefen', 'Flow_Activity_ZahlungseingangsbestaetigungenPruefen_Activity', 'Flow_Activity_ActivityAnstossen', 'Flow_KundenstammdatenaktualisierungAnstossenToPackOrder', 'Flow_PackOrderToArchiveOrder'], - ); - expect((await Doc.fromXml(idPair)).ownIdStyle.info().sequenceFlow).toEqual({ form: 'idPair', prefix: 'Flow' }); - const pack = await applyToXml(idPair, [{ op: 'add', kind: 'task', name: 'Weigh parcel', after: 'Activity_PackOrder' }], { layout: false }); - expect(pack.result.renamed).toEqual({ Flow_PackOrderToArchiveOrder: 'Flow_Activity_PackOrder_Activity_WeighParcel' }); - const before = await applyToXml(idPair, [{ op: 'add', kind: 'task', name: 'Weigh parcel', flow: 'Flow_KundenstammdatenaktualisierungAnstossenToPackOrder' }], { layout: false }); - expect(Object.keys(before.result.renamed ?? {})).toEqual(['Flow_KundenstammdatenaktualisierungAnstossenToPackOrder']); // an idSnake file with a flow 0.6 named (Flow_pay_to_done) const idSnake = chainXml( [['Task_check', 'Check'], ['Task_ship', 'Ship'], ['Task_bill', 'Bill'], ['Task_pay', 'Pay'], ['Task_done', 'Done']], @@ -847,32 +827,6 @@ describe('id quality: context once, length cap, transliteration', () => { expect((await Doc.fromXml(idSnake)).ownIdStyle.info().sequenceFlow).toEqual({ form: 'idSnake', prefix: 'Flow' }); const log = await applyToXml(idSnake, [{ op: 'add', kind: 'task', name: 'Log', flow: 'Flow_pay_to_done' }], { layout: false }); expect(log.result.renamed).toEqual({ Flow_pay_to_done: 'Flow_Task_pay_to_Task_log' }); - // a camelCase file that reads as stemTo, with flows 0.6 named and cut at 61 characters to an end with a suffix - // (before: both kept their ids) - const stem = chainXml( - [ - ['Activity_checkOrder', 'Check order'], - ['Activity_shipOrder', 'Ship order'], - ['Activity_billOrder', 'Bill order'], - ['Activity_lieferantenbestellungsuebersicht', 'Lieferantenbestellungsuebersicht erstellen'], - ['Activity_lieferantenbestellungsuebersicht_2', 'Lieferantenbestellungsuebersicht erstellen'], - ['Activity_kundenstammdatenaktualisierungAnstossen', 'Kundenstammdatenaktualisierung anstossen'], - ], - ['Flow_checkOrderToShipOrder', 'Flow_shipOrderToBillOrder', 'Flow_billOrderToLieferantenbestellungsuebersicht', 'Flow_lieferantenbestellungsueberToLieferantenbestellungsuebe2', 'Flow_lieferantenbestellungsuebe2ToKundenstammdatenaktualisier'], - ); - expect((await Doc.fromXml(stem)).ownIdStyle.info().sequenceFlow).toEqual({ form: 'stemTo', prefix: 'Flow' }); - const weigh = await applyToXml( - stem, - [ - { op: 'add', kind: 'task', name: 'Weigh parcel', after: 'Activity_lieferantenbestellungsuebersicht' }, - { op: 'add', kind: 'task', name: 'Scan', after: 'Activity_lieferantenbestellungsuebersicht_2' }, - ], - { layout: false }, - ); - expect(weigh.result.renamed).toEqual({ - Flow_lieferantenbestellungsueberToLieferantenbestellungsuebe2: 'Flow_lieferantenbestellungsuebersichtToWeighParcel', - Flow_lieferantenbestellungsuebe2ToKundenstammdatenaktualisier: 'Flow_lieferantenbestellungsuebersicht_2ToScan', - }); // an id an author wrote in the `named` form follows too: nothing tells it from one bpmn-cli wrote (ID_CONVENTIONS.renames) const handWritten = definitionsXml(` flow_eventOrderReceivedToCheckOrder @@ -998,50 +952,6 @@ describe('id quality: context once, length cap, transliteration', () => { expect(times.sort((first, second) => first - second)[1]).toBeLessThan(100); }); - it('#31: a flow 0.6 cut inside a word still follows its ends', async () => { - const syncTask = 'Activity_ConsignationsSynchronisieren'; - const logTask = 'Activity_ConsignationsProtokollieren'; - const old = 'Flow_ConsignationSynchronisierenToConsignationsProtokollieren'; - const doc = await Doc.fromXml( - definitionsXml(` - ${old} - ${old} - `), - ); - runOps(doc, [{ op: 'add', kind: 'serviceTask', name: 'Consignations prüfen', after: syncTask }]); - expect(flowBetween(doc, syncTask, 'Activity_ConsignationsPruefen')).toBe('Flow_ConsignationsSynchronisierenToConsignationsPruefen'); - // a suffix of 4 characters made 0.6 cut its base of 61 again (`_100`) - const [syncSource, arrivedTarget] = ['Activity_ProtokollierenSynchronisieren', 'Activity_ProtokollierenEingegangen']; - const hundredth = 'Flow_ProtokollierenSynchronisierenToProtokollieren_100'; - const doc2 = await Doc.fromXml( - definitionsXml(` - ${hundredth} - ${hundredth} - `), - ); - runOps(doc2, [{ op: 'add', kind: 'serviceTask', name: 'Protokollieren prüfen', after: syncSource }]); - expect(flowBetween(doc2, syncSource, 'Activity_ProtokollierenPruefen')).toBe('Flow_ProtokollierenSynchronisierenToProtokollierenPruefen'); - // 0.6 cut an end that begins with a separator and whose first word did not fit to nothing (cutAt with `legacy`) - expect(cutAt('_lieferantenbestellungsuebersichtenpruefung', 26, true)).toBe(''); - expect(cutAt('_lieferantenbestellungsuebersichtenpruefung', 26)).toBe('_lieferantenbestellungsueb'); - const [reviewTask, reconcileTask] = ['Task__lieferantenbestellungsuebersichtenpruefung', 'Task__zahlungseingangsbestaetigungenabgleichung']; - const separated = 'Flow__to__zahlungseingangsbestaetigungenabgleichung'; - const doc3 = await Doc.fromXml( - definitionsXml(` - Flow_check_order_to_ship_order - Flow_check_order_to_ship_orderFlow_ship_order_to_bill_order - Flow_ship_order_to_bill_order - ${separated} - ${separated} - - - `), - ); - expect(doc3.ownIdStyle.info().sequenceFlow.form).toBe('stemSnake'); - const cs = runOps(doc3, [{ op: 'add', kind: 'task', name: 'Scan', after: reviewTask }]); - expect(cs.renames).toEqual([expect.objectContaining({ from: separated, to: 'Flow__lieferantenbestellungsuebersichtenpruefung_to_scan' })]); - }); - it('#31: a collision suffix fits into 64 characters: the ends are cut again at whole words, and the flow follows its ends', async () => { const applied = await applyToXml((await newXml({ processName: 'Sync' })).xml, [ { op: 'add', kind: 'startEvent', name: 'Go', in: 'Process_Sync', as: '$s' }, From ba415d80ec52f740e125f58fbdefa1b648eb3db2 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:49:16 +0100 Subject: [PATCH 08/11] refactor: share the lane walk and the id index with the auto engine's lane workaround --- .../diagram/layout/TemporaryLaneMembers.ts | 24 ++++--------------- src/domain/document/Doc.ts | 17 ++----------- src/domain/document/utils/lanes.ts | 16 ++++++++++++- src/domain/validation/structure/LaneChecks.ts | 3 ++- test/domain/editing/ops-create.test.ts | 3 ++- 5 files changed, 25 insertions(+), 38 deletions(-) diff --git a/src/domain/diagram/layout/TemporaryLaneMembers.ts b/src/domain/diagram/layout/TemporaryLaneMembers.ts index 75cde4b1..ac6c8611 100644 --- a/src/domain/diagram/layout/TemporaryLaneMembers.ts +++ b/src/domain/diagram/layout/TemporaryLaneMembers.ts @@ -1,4 +1,5 @@ -import { addTo, is, many, processes, removeFrom, walk, type El, idOf } from '../../document/utils/moddle.js'; +import { addTo, idOf, indexById, is, many, processes, removeFrom, type El } from '../../document/utils/moddle.js'; +import { allLanes } from '../../document/utils/lanes.js'; interface TempMember { laneId: string; @@ -16,19 +17,6 @@ export class TemporaryLaneMembers { return this.members.length; } - private static allLanes(process: El): El[] { - const out: El[] = []; - const visit = (laneSet: El | undefined): void => { - if (!laneSet) return; - for (const lane of many(laneSet, 'lanes')) { - out.push(lane); - visit(lane.get('childLaneSet')); - } - }; - for (const ls of many(process, 'laneSets')) visit(ls); - return out; - } - private static isLeafLane(lane: El): boolean { const child = lane.get('childLaneSet'); return !child || many(child, 'lanes').length === 0; @@ -56,7 +44,7 @@ export class TemporaryLaneMembers { static add(defs: El): TemporaryLaneMembers { const added: TempMember[] = []; for (const process of processes(defs)) { - const lanes = TemporaryLaneMembers.allLanes(process); + const lanes = allLanes(process); if (!lanes.length) continue; const leafLanes = lanes.filter(TemporaryLaneMembers.isLeafLane); if (!leafLanes.length) continue; @@ -76,11 +64,7 @@ export class TemporaryLaneMembers { withdraw(defs: El): void { if (!this.members.length) return; - const byId = new Map(); - for (const el of walk(defs)) { - const id = el.get('id'); - if (id) byId.set(id, el); - } + const byId = indexById(defs); for (const { laneId, nodeId } of this.members) { const lane = byId.get(laneId); const node = byId.get(nodeId); diff --git a/src/domain/document/Doc.ts b/src/domain/document/Doc.ts index c9ffdfe2..e686ba1e 100644 --- a/src/domain/document/Doc.ts +++ b/src/domain/document/Doc.ts @@ -12,6 +12,7 @@ import { LayoutRequests } from './LayoutRequests.js'; import { kindLabel } from './utils/kinds.js'; import { IdSuggestions } from './IdSuggestions.js'; import { completeMirrorLists, takeMirrorSnapshot, type MirrorSnapshot } from './utils/mirror.js'; +import { allLanes } from './utils/lanes.js'; import { createDefinitions, createModdle, indexById, is, isBpmnElement, many, parseXml, processes as rootProcesses, collaboration as rootCollaboration, layoutRoot as pickLayoutRoot, serialize, walk, type El, type Model } from './utils/moddle.js'; import { ModelError } from './ModelError.js'; import { C7_DEFAULT_TTL, C7_PLATFORM_VERSION } from './vendor/CamundaDescriptor.js'; @@ -436,24 +437,10 @@ export class Doc { return this.flowElements(scope).filter((element) => is(element, 'bpmn:BoundaryEvent') && element.get('attachedToRef') === activity); } - /** Nested lanes included, deepest last. */ - allLanes(process: El): El[] { - const out: El[] = []; - const visit = (laneSet: El | undefined): void => { - if (!laneSet) return; - for (const lane of many(laneSet, 'lanes')) { - out.push(lane); - visit(lane.get('childLaneSet')); - } - }; - for (const ls of many(process, 'laneSets')) visit(ls); - return out; - } - lanesOf(node: El): El[] { const process = this.processOf(node); if (!process) return []; - return this.allLanes(process).filter((lane) => (lane.get('flowNodeRef') ?? []).includes(node)); + return allLanes(process).filter((lane) => (lane.get('flowNodeRef') ?? []).includes(node)); } messageFlows(): El[] { diff --git a/src/domain/document/utils/lanes.ts b/src/domain/document/utils/lanes.ts index 810f98e6..e11c759c 100644 --- a/src/domain/document/utils/lanes.ts +++ b/src/domain/document/utils/lanes.ts @@ -1,5 +1,19 @@ import type { Doc } from '../Doc.js'; -import { is, type El } from './moddle.js'; +import { is, many, type El } from './moddle.js'; + +/** Nested lanes included, deepest last. */ +export function allLanes(process: El): El[] { + const out: El[] = []; + const visit = (laneSet: El | undefined): void => { + if (!laneSet) return; + for (const lane of many(laneSet, 'lanes')) { + out.push(lane); + visit(lane.get('childLaneSet')); + } + }; + for (const ls of many(process, 'laneSets')) visit(ls); + return out; +} export function laneSetOf(owner: El): El | undefined { if (is(owner, 'bpmn:Lane')) return owner.get('childLaneSet'); diff --git a/src/domain/validation/structure/LaneChecks.ts b/src/domain/validation/structure/LaneChecks.ts index 8e3d826a..74e0e4c4 100644 --- a/src/domain/validation/structure/LaneChecks.ts +++ b/src/domain/validation/structure/LaneChecks.ts @@ -1,5 +1,6 @@ import type { Doc } from '../../document/Doc.js'; import { is, type El, list, shownId } from '../../document/utils/moddle.js'; +import { allLanes } from '../../document/utils/lanes.js'; import type { StructureFindings } from './StructureFindings.js'; /** Lane membership: flow nodes of the process only, one lane each, none left out. */ @@ -11,7 +12,7 @@ export class LaneChecks { checkLanes(process: El): void { const { doc } = this; - const lanes = doc.allLanes(process); + const lanes = allLanes(process); if (!lanes.length) return; const direct = doc.flowElements(process); const seen = new Map(); diff --git a/test/domain/editing/ops-create.test.ts b/test/domain/editing/ops-create.test.ts index 0e580c10..b1eb73c6 100644 --- a/test/domain/editing/ops-create.test.ts +++ b/test/domain/editing/ops-create.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; +import { allLanes } from '../../../src/domain/document/utils/lanes.js'; import { Doc } from '../../../src/domain/document/Doc.js'; import { is, many, type El } from '../../../src/domain/document/utils/moddle.js'; import { AssociationService, PLACEHOLDER_PROPERTY } from '../../../src/domain/editing/AssociationService.js'; @@ -543,7 +544,7 @@ describe('lanes', () => { add(doc, { kind: 'lane', name: 'Outbound', in: 'Lane_Sales', members: ['Task_A'] }); expect(ids(many(doc.require('Lane_Inbound'), 'flowNodeRef'))).toEqual(['Start']); expect(laneOf(doc, doc.require('Task_A'))?.get('id')).toBe('Lane_Outbound'); - expect(ids(doc.allLanes(doc.require('Process_1')))).toEqual(['Lane_Sales', 'Lane_Inbound', 'Lane_Outbound', 'Lane_Backoffice']); + expect(ids(allLanes(doc.require('Process_1')))).toEqual(['Lane_Sales', 'Lane_Inbound', 'Lane_Outbound', 'Lane_Backoffice']); // parent lanes with children cannot take members directly expect(codeOf(() => new LaneService(doc, new EditContext(doc)).assign(doc.require('End'), doc.require('Lane_Sales'), new ChangeSet()))).toBe('E_INVALID_LANE_MEMBERSHIP'); // nodes added after a member inherit the nested lane From 2d50f11c7de840337289b1bec093b227d37a0f62 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 17:54:38 +0100 Subject: [PATCH 09/11] chore: gate on a duplication budget and on code only tests reach (knip production mode) --- .jscpd.json | 8 + CONTRIBUTING.md | 6 +- knip.json | 4 +- package-lock.json | 151 ++++++++++++++++++ package.json | 3 +- src/domain/diagram/utils/problems.ts | 6 +- src/domain/diagram/utils/text.ts | 2 + .../document/vendor/camunda-descriptor.ts | 3 +- .../document/vendor/zeebe-descriptor.ts | 3 +- src/domain/editing/AssociationService.ts | 1 + tools/gen-camunda-descriptor.mjs | 3 +- 11 files changed, 179 insertions(+), 11 deletions(-) create mode 100644 .jscpd.json diff --git a/.jscpd.json b/.jscpd.json new file mode 100644 index 00000000..a1de27d0 --- /dev/null +++ b/.jscpd.json @@ -0,0 +1,8 @@ +{ + "path": ["src"], + "format": ["typescript"], + "minLines": 6, + "threshold": 0.2, + "reporters": ["console"], + "gitignore": true +} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e35bea01..1a01b90b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,8 +28,10 @@ npm test # the vitest suite npm run gate # everything a change has to pass before it is handed over ``` -`npm run gate` builds, runs knip and the tests, the isomorphism check of the browser-safe core, the -layout-regression budget and a short fuzz campaign. CI runs `typecheck` and `gate` on every pull +`npm run gate` builds, runs knip twice (the second time in production mode: code in `src/` that only +tests use fails unless its export is tagged `@internal`), the duplication budget of `src/` (jscpd, +`.jscpd.json`), the tests, the isomorphism check of the browser-safe core, the layout-regression +budget and a short fuzz campaign. CI runs `typecheck` and `gate` on every pull request and every push to `main`. The test layers, the benchmark and the fuzzer, and how to run them on a private corpus without copying it into the repository, are described in [docs/testing.md](docs/testing.md). diff --git a/knip.json b/knip.json index d868c3a1..35a5d2c2 100644 --- a/knip.json +++ b/knip.json @@ -1,6 +1,6 @@ { "$schema": "https://unpkg.com/knip@6/schema.json", - "entry": ["src/api.ts", "tools/**/*.mjs", "test/**/*.test.ts"], - "project": ["src/**/*.ts", "tools/**/*.mjs", "test/**/*.ts"], + "entry": ["src/api.ts!", "tools/**/*.mjs", "test/**/*.test.ts"], + "project": ["src/**/*.ts!", "tools/**/*.mjs", "test/**/*.ts"], "ignoreDependencies": ["@resvg/resvg-wasm"] } diff --git a/package-lock.json b/package-lock.json index 5b4abed8..99c0456f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,6 +25,7 @@ "archunit": "2.5.4", "camunda-bpmn-moddle": "8.0.2", "esbuild": "^0.28.2", + "jscpd": "5.4.1", "knip": "6.43.0", "tsx": "^4.23.0", "typescript": "^7.0.2", @@ -2800,6 +2801,156 @@ "jiti": "lib/jiti-cli.mjs" } }, + "node_modules/jscpd": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd/-/jscpd-5.4.1.tgz", + "integrity": "sha512-afUt+LSQMmzSPYrWZlNGTwj8X4tmwVJbMNmr92qxPf4uUF/naLwG7SQyMPEZtrHqGgzGxmAzNjiwlVdjls2MyQ==", + "dev": true, + "license": "MIT", + "bin": { + "jscpd": "run-jscpd.js" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://opencollective.com/jscpd" + }, + "optionalDependencies": { + "jscpd-darwin-arm64": "5.4.1", + "jscpd-darwin-x64": "5.4.1", + "jscpd-linux-arm64-gnu": "5.4.1", + "jscpd-linux-arm64-musl": "5.4.1", + "jscpd-linux-x64-gnu": "5.4.1", + "jscpd-linux-x64-musl": "5.4.1", + "jscpd-windows-arm64-msvc": "5.4.1", + "jscpd-windows-x64-msvc": "5.4.1" + } + }, + "node_modules/jscpd-darwin-arm64": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-darwin-arm64/-/jscpd-darwin-arm64-5.4.1.tgz", + "integrity": "sha512-LXHVeH60C9BsozcVIMPmXWvj97KqFKbCJYmYG4hDwfwIwS1kz+uF7yiObckP6h2xo12pD88nI4XjWTB5frjZUw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/jscpd-darwin-x64": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-darwin-x64/-/jscpd-darwin-x64-5.4.1.tgz", + "integrity": "sha512-Gqck4tcMZjv4IvYHU2Vcikb885zTADo5T/YfEqeu1i5W5tfFpLdViiTYrh5wbrW+uxBqC33gHpG1hy0xbGYknQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/jscpd-linux-arm64-gnu": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-linux-arm64-gnu/-/jscpd-linux-arm64-gnu-5.4.1.tgz", + "integrity": "sha512-XUYZJZVxSY7JIBEoyOOMTgvJwmymkns4wG55tu6I/vRKXnS5QtaTESQfRLt6xj1+62aflAvr01FAhPAy+du6tA==", + "cpu": [ + "arm64" + ], + "dev": true, + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/jscpd-linux-arm64-musl": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-linux-arm64-musl/-/jscpd-linux-arm64-musl-5.4.1.tgz", + "integrity": "sha512-h2sH8Bk++yBxWqq2QAy4fvPzXkw1vecDSyOmzLwPfnRQoQ2Q/EWxCNOrgJSb+SNMLWoHLLzfqx8m9wt2GG6a8w==", + "cpu": [ + "arm64" + ], + "dev": true, + "libc": [ + "musl" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/jscpd-linux-x64-gnu": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-linux-x64-gnu/-/jscpd-linux-x64-gnu-5.4.1.tgz", + "integrity": "sha512-UiC4YoGAVVLy6hPVz8A6cndo7W5br4FAO2hADnFeinhBPnXPu0QFh1q/s/8B3ldeQHQkpYd/l5cjqNBMrbm78w==", + "cpu": [ + "x64" + ], + "dev": true, + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/jscpd-linux-x64-musl": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-linux-x64-musl/-/jscpd-linux-x64-musl-5.4.1.tgz", + "integrity": "sha512-toAafl5roqfFotPrDJNMY9noNVZ9dReBqcO/nT5pJ8+7aq+opbscaLUcV4+8jxTq5btUD6CiO72+Fu+TyUZv3Q==", + "cpu": [ + "x64" + ], + "dev": true, + "libc": [ + "musl" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/jscpd-windows-arm64-msvc": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-windows-arm64-msvc/-/jscpd-windows-arm64-msvc-5.4.1.tgz", + "integrity": "sha512-JLMga/NIwv/OPurPsXA2/d5OdWfdwrsBk2D4NhyKnrV/uNjo1BIi2zXztzDyTQVKMK/TEK6GPgLD143GG1fZ0A==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, + "node_modules/jscpd-windows-x64-msvc": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/jscpd-windows-x64-msvc/-/jscpd-windows-x64-msvc-5.4.1.tgz", + "integrity": "sha512-sN57Y9YP3ijrqc1IsWDceOFHUwbukg9/mU2NSDHTryKKc5CUvWz+o9GId8uKTiJIhkKfR/Ce5GKkCepTwB6kaw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, "node_modules/json-colorizer": { "version": "2.2.2", "resolved": "https://registry.npmjs.org/json-colorizer/-/json-colorizer-2.2.2.tgz", diff --git a/package.json b/package.json index caeacad1..977c6fca 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "docs:dev": "npm --prefix docs run dev", "docs:build": "npm --prefix docs run build", "docs:preview": "npm --prefix docs run preview", - "gate": "npm run build && knip && vitest run && node tools/iso/check.mjs && node tools/bench/regress-gate.mjs && node tools/fuzz/run.mjs --walks 12 --steps 15 --seed 1 --hard warn --out tools/fuzz/out/gate", + "gate": "npm run build && knip && knip --production --exclude dependencies && jscpd && vitest run && node tools/iso/check.mjs && node tools/bench/regress-gate.mjs && node tools/fuzz/run.mjs --walks 12 --steps 15 --seed 1 --hard warn --out tools/fuzz/out/gate", "prepublishOnly": "npm run build" }, "dependencies": { @@ -78,6 +78,7 @@ "archunit": "2.5.4", "camunda-bpmn-moddle": "8.0.2", "esbuild": "^0.28.2", + "jscpd": "5.4.1", "knip": "6.43.0", "tsx": "^4.23.0", "typescript": "^7.0.2", diff --git a/src/domain/diagram/utils/problems.ts b/src/domain/diagram/utils/problems.ts index e99eeed0..93f13c64 100644 --- a/src/domain/diagram/utils/problems.ts +++ b/src/domain/diagram/utils/problems.ts @@ -1,8 +1,8 @@ /** Layout problem kinds, their weights and the comparison of two measurements. */ -/** structural defects the regression harness does not measure, weighted like an overlap */ +/** structural defects the regression harness does not measure, weighted like an overlap @internal */ export const EXTRA_KEYS = ['frameIntrusion', 'frameOverlap', 'degenerateEdge', 'detachedEdge'] as const; -/** soft kinds the harness does not measure; weights below 6, so they never count as hard */ +/** soft kinds the harness does not measure; weights below 6, so they never count as hard @internal */ export const QUALITY_KEYS = ['backwardFlow', 'segmentOverlap', 'labelOutsideFrame', 'labelOnTitle', 'messageLabelFar'] as const; export const KEYS = [ @@ -36,7 +36,7 @@ export const KEYS = [ export type MetricKey = (typeof KEYS)[number]; -/** the regression harness's kinds, in its order */ +/** the regression harness's kinds, in its order @internal */ export const HARNESS_KEYS: readonly MetricKey[] = KEYS.filter((key) => !(EXTRA_KEYS as readonly string[]).includes(key) && !(QUALITY_KEYS as readonly string[]).includes(key)); export const WEIGHTS: Readonly> = { diff --git a/src/domain/diagram/utils/text.ts b/src/domain/diagram/utils/text.ts index 6b9da65d..cfa1f926 100644 --- a/src/domain/diagram/utils/text.ts +++ b/src/domain/diagram/utils/text.ts @@ -77,6 +77,7 @@ export function wrapLines(text: string, maxWidth: number): string[] { const PADDING = 7; +/** @internal */ export const innerLines = (name: string, width: number): number => wrapLines(name, width - 2 * PADDING).length; /** Where bpmn-js draws an expanded sub-process's name: centred, starting 7 px below the top. */ @@ -97,6 +98,7 @@ export function titleDepth(name: string | undefined, width: number): number { */ export const BOTTOM_ROOM = 17; +/** @internal */ export function fitsInside(name: string, size: { width: number; height: number }, bottomRoom = 0): boolean { return innerLines(name, size.width) * LINE_HEIGHT <= size.height - 2 * Math.max(4, bottomRoom) + 0.01; } diff --git a/src/domain/document/vendor/camunda-descriptor.ts b/src/domain/document/vendor/camunda-descriptor.ts index ce2657f4..07a4211c 100644 --- a/src/domain/document/vendor/camunda-descriptor.ts +++ b/src/domain/document/vendor/camunda-descriptor.ts @@ -29,9 +29,10 @@ * THE SOFTWARE. */ -/** camunda-bpmn-moddle's descriptor, verbatim. */ +/** @internal */ export const CAMUNDA_DESCRIPTOR_VERSION = "8.0.2"; +/** camunda-bpmn-moddle's descriptor, verbatim. */ export const CAMUNDA_DESCRIPTOR: { types: unknown[]; [key: string]: unknown } = { "name": "Camunda", "uri": "http://camunda.org/schema/1.0/bpmn", diff --git a/src/domain/document/vendor/zeebe-descriptor.ts b/src/domain/document/vendor/zeebe-descriptor.ts index b4827b61..93a25fe1 100644 --- a/src/domain/document/vendor/zeebe-descriptor.ts +++ b/src/domain/document/vendor/zeebe-descriptor.ts @@ -29,9 +29,10 @@ * THE SOFTWARE. */ -/** zeebe-bpmn-moddle's descriptor, verbatim. */ +/** @internal */ export const ZEEBE_DESCRIPTOR_VERSION = "2.0.0"; +/** zeebe-bpmn-moddle's descriptor, verbatim. */ export const ZEEBE_DESCRIPTOR: { types: unknown[]; [key: string]: unknown } = { "name": "zeebe", "prefix": "zeebe", diff --git a/src/domain/editing/AssociationService.ts b/src/domain/editing/AssociationService.ts index 2cc9ad11..53915419 100644 --- a/src/domain/editing/AssociationService.ts +++ b/src/domain/editing/AssociationService.ts @@ -9,6 +9,7 @@ import type { ChangeSet } from '../document/ChangeSet.js'; import { allocateId } from './utils/allocate-id.js'; import { artifactContainerOf, isDataReference } from './utils/artifacts.js'; +/** @internal */ export const PLACEHOLDER_PROPERTY = '__targetRef_placeholder'; /** diff --git a/tools/gen-camunda-descriptor.mjs b/tools/gen-camunda-descriptor.mjs index 2c346c74..5b622303 100644 --- a/tools/gen-camunda-descriptor.mjs +++ b/tools/gen-camunda-descriptor.mjs @@ -64,9 +64,10 @@ function generate(target) { ${licence.split('\n').map((licenceLine) => (licenceLine ? ` * ${licenceLine}` : ' *')).join('\n')} */ -/** ${target.pkg}'s descriptor, verbatim. */ +/** @internal */ export const ${target.constant}_VERSION = ${JSON.stringify(version)}; +/** ${target.pkg}'s descriptor, verbatim. */ export const ${target.constant}: { types: unknown[]; [key: string]: unknown } = ${render(json)}; `; return { text, version, types: json.types.length, out: join(ROOT, 'src', 'domain', 'document', 'vendor', target.file) }; From 65755601097166ea13936645f76deb220acda9d1 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 18:07:38 +0100 Subject: [PATCH 10/11] refactor: services load, call the domain and save in plain sight, with mandatory ports Every port of EditDocumentService, RelayoutDiagramService, CreateDocumentService, ValidateDocumentService and ReadDocumentService is required now, so no `!` reaches into an optional one. The in-memory API hands them NoDocumentFiles (adapters/out/memory), the counterpart of NoProjectSettings: nothing to read, nothing written. The public method of each service reads as open, call the domain on a line of its own, write. The check of `bpmn validate` moves out of ValidateDocumentService into the domain object DocumentCheck, which owns CheckOptions and CheckResult; the port re-exports them. The jscpd budget goes from 0.2 % to 0.25 %: the small open / target / write steps repeat in three services, as the architecture keeps helpers out of application/. --- .jscpd.json | 2 +- CONTRIBUTING.md | 2 +- docs/architecture.md | 39 +++----- src/adapters/out/memory/NoDocumentFiles.ts | 20 ++++ src/api.ts | 20 ++-- .../port/in/ValidateDocumentUseCase.ts | 29 +----- .../service/CreateDocumentService.ts | 25 +++-- .../service/EditDocumentService.ts | 38 +++++--- .../service/ReadDocumentService.ts | 4 +- .../service/RelayoutDiagramService.ts | 46 +++++---- .../service/ValidateDocumentService.ts | 63 ++---------- src/domain/validation/DocumentCheck.ts | 95 +++++++++++++++++++ src/index.ts | 20 ++-- 13 files changed, 242 insertions(+), 161 deletions(-) create mode 100644 src/adapters/out/memory/NoDocumentFiles.ts create mode 100644 src/domain/validation/DocumentCheck.ts diff --git a/.jscpd.json b/.jscpd.json index a1de27d0..abfa5e66 100644 --- a/.jscpd.json +++ b/.jscpd.json @@ -2,7 +2,7 @@ "path": ["src"], "format": ["typescript"], "minLines": 6, - "threshold": 0.2, + "threshold": 0.25, "reporters": ["console"], "gitignore": true } diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1a01b90b..683da86a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,7 +62,7 @@ them on a private corpus without copying it into the repository, are described i | `adapters/in/cli/` | the driving adapter: the `Cli` (commander, streams, exit codes, errors) and one class per command; it knows the use cases only by their contracts | | `adapters/in/report/` | what a use case's result looks like to the one who asked: text and JSON reports (`MutationText`, `MutationReportBuilder`, `ValidationText`, ...), the views as text, and the reference the guide and `bpmn kinds` print; the CLI prints them and `index.ts` / `api.ts` return them | | `adapters/out/files/` | the driven adapters on disk, Node only: `FileDocumentStore`, `BpmnlintProjectSettings`, `FileImageWriter` (PNG) | - | `adapters/out/memory/` | the driven adapters of the in-memory API: `InMemoryDocumentStore`, `NoProjectSettings` | + | `adapters/out/memory/` | the driven adapters of the in-memory API: `NoDocumentFiles`, `NoProjectSettings` | | `types/` | type shims for the bpmn-io packages that ship no usable declarations | Where new code goes: a new command is a type in `domain/commands/` and a `...Service` of the diff --git a/docs/architecture.md b/docs/architecture.md index 5f4a022a..a0a921e1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -23,7 +23,7 @@ flowchart TB service --> domain["Domain
domain/"] service --> portOut{{"Port
application/port/out"}} portOut --> files["Files
adapters/out/files"] - portOut --> memory["No project
adapters/out/memory"] + portOut --> memory["No project, no files
adapters/out/memory"] classDef adapter stroke:#2f80ed,stroke-width:2px classDef app stroke:#f2994a,stroke-width:2px @@ -156,44 +156,33 @@ export declare namespace EditDocumentUseCase { ### 3. Load, run, save The service is short on purpose: get the document, let the domain work, write the result. -`settings`, `load` and `save` are outbound ports, handed in by the constructor; without `load` -or `save` the service takes only a document and writes nothing. +`settings`, `load` and `save` are outbound ports, handed in by the constructor. The call into +the domain stands on a line of its own; reading and writing the file are small steps around it. ```ts // src/application/service/EditDocumentService.ts export class EditDocumentService implements EditDocumentUseCase { constructor( private readonly settings: LoadProjectSettingsPort, - private readonly load?: LoadDocumentPort, - private readonly save?: SaveDocumentPort, + private readonly load: LoadDocumentPort, + private readonly save: SaveDocumentPort, ) {} - async edit({ source, commands, ...opts }: EditDocumentUseCase.Command) { - // step 7: read the file - const doc = 'doc' in source - ? source.doc - : await Doc.fromXml(await this.load!.read(source.file), source.file) - const target = opts.out ?? doc.file - - // the project's .bpmnlintrc, if any - const lint = await this.settings.lint(target, opts) - - // step 4: the domain does the work - const batchOpts = lint.applyTo(opts, target) - const batch = new EditBatchService(doc, commands, batchOpts) - const result = await batch.run(target) - - // step 7: write the file - if (!opts.dryRun) { - result.written = await this.save!.save(target, result.xml, { … }) - } + async edit(command: EditDocumentUseCase.Command) { + const { source, commands, ...opts } = command + const doc = await this.open(source, opts) // step 7: read the file + const target = await this.targetOf(doc, opts) + const project = (await this.settings.lint(opts.file ?? target, opts)).applyTo(opts, target) + const result = await new EditBatchService(doc, commands, project).run(target) // step 4: the domain + await this.write(result, doc, target, opts) // step 7: write the file return result } } ``` In the CLI the ports are a `FileDocumentStore` and `BpmnlintProjectSettings`; `applyToXml()` -passes only `NoProjectSettings` and a `{ doc }` with `dryRun`, so nothing is read or written. +passes `NoProjectSettings`, `NoDocumentFiles` and a `{ doc }` with `dryRun`, so nothing is read +or written. ### 4. The batch diff --git a/src/adapters/out/memory/NoDocumentFiles.ts b/src/adapters/out/memory/NoDocumentFiles.ts new file mode 100644 index 00000000..e7c29b0c --- /dev/null +++ b/src/adapters/out/memory/NoDocumentFiles.ts @@ -0,0 +1,20 @@ +import type { LoadDocumentPort } from '../../../application/port/out/LoadDocumentPort.js'; +import type { SaveDocumentPort } from '../../../application/port/out/SaveDocumentPort.js'; +import { ioError } from '../../../domain/document/utils/errors.js'; + +/** A document in memory has no file: there is none to read, and none is written or taken. */ +export class NoDocumentFiles implements LoadDocumentPort, SaveDocumentPort { + async read(file: string): Promise { + throw ioError('E_FILE_NOT_FOUND', `Cannot read ${file}: no such document`, { file }); + } + + decode(_bytes: Uint8Array, source: string): string { + throw ioError('E_FILE_NOT_FOUND', `Cannot read ${source}: no such document`, { file: source }); + } + + async save(): Promise { + throw ioError('E_NO_FILE', 'No output file given'); + } + + async assertNew(): Promise {} +} diff --git a/src/api.ts b/src/api.ts index 8b9dab8e..c0f95b5f 100644 --- a/src/api.ts +++ b/src/api.ts @@ -20,6 +20,7 @@ import type { CheckOptions, CheckResult } from './application/port/in/ValidateDo import type { LayoutDocOptions } from './application/port/in/RelayoutDiagramUseCase.js'; import { EditDocumentService } from './application/service/EditDocumentService.js'; import { NoProjectSettings } from './adapters/out/memory/NoProjectSettings.js'; +import { NoDocumentFiles } from './adapters/out/memory/NoDocumentFiles.js'; import { RelayoutDiagramService } from './application/service/RelayoutDiagramService.js'; import { ValidateDocumentService } from './application/service/ValidateDocumentService.js'; import type { PlatformChoice } from './domain/document/vendor/PlatformDetection.js'; @@ -41,7 +42,8 @@ import { ShownText } from './adapters/in/report/ShownText.js'; /** The in-memory API runs the use cases without files and without a project around the document. */ const NO_PROJECT = new NoProjectSettings(); -const READ_DOCUMENT = new ReadDocumentService(); +const NO_FILES = new NoDocumentFiles(); +const READ_DOCUMENT = new ReadDocumentService(NO_FILES); export type { ShownView, ViewOptions } from './domain/views/ViewRequest.js'; @@ -102,12 +104,12 @@ function parseXml(xml: string, file?: string): Promise { export async function applyToXml(xml: string, ops: unknown, opts: EditOptions = {}): Promise { const list = readOps(ops); const doc = await loadXml(xml, opts); - return toEditResult(xml, await new EditDocumentService(NO_PROJECT).edit({ ...opts, source: { doc }, commands: list, dryRun: true })); + return toEditResult(xml, await new EditDocumentService(NO_PROJECT, NO_FILES, NO_FILES).edit({ ...opts, source: { doc }, commands: list, dryRun: true })); } /** `bpmn new`: a laid-out document with one empty process. */ export async function newXml(opts: NewDocOptions & EditOptions = {}): Promise { - return toEditResult(undefined, await new CreateDocumentService(NO_PROJECT).create({ ...opts, dryRun: true })); + return toEditResult(undefined, await new CreateDocumentService(NO_PROJECT, NO_FILES).create({ ...opts, dryRun: true })); } export interface LayoutXmlOptions extends Omit { @@ -121,7 +123,7 @@ export interface LayoutXmlOptions extends Omit { export async function layoutXml(xml: string, opts: LayoutXmlOptions = {}): Promise { RelayoutDiagramService.assertOptions(opts); const doc = await loadXml(xml, opts); - return toEditResult(xml, await new RelayoutDiagramService(NO_PROJECT).relayout({ ...opts, source: { doc }, dryRun: true })); + return toEditResult(xml, await new RelayoutDiagramService(NO_PROJECT, NO_FILES, NO_FILES).relayout({ ...opts, source: { doc }, dryRun: true })); } export interface ValidateXmlOptions { @@ -134,7 +136,7 @@ export interface ValidateXmlOptions { /** `bpmn validate --json`: structure, lint, platform profile, validators and a layout dry run. */ export async function validateXml(xml: string, opts: ValidateXmlOptions = {}): Promise { - return new ValidationReportBuilder(await new ValidateDocumentService(NO_PROJECT).validate({ ...opts, source: { doc: await parseXml(xml, opts.file) } })).report(); + return new ValidationReportBuilder(await new ValidateDocumentService(NO_PROJECT, NO_FILES).validate({ ...opts, source: { doc: await parseXml(xml, opts.file) } })).report(); } /** The text of a view as `bpmn show` prints it, without the final newline. */ @@ -180,20 +182,20 @@ export async function extensionsXml(xml: string, id: string): Promise { - return new EditDocumentService(NO_PROJECT).edit({ ...opts, source: { doc }, commands: ops, dryRun: true }); + return new EditDocumentService(NO_PROJECT, NO_FILES, NO_FILES).edit({ ...opts, source: { doc }, commands: ops, dryRun: true }); } /** `bpmn layout` on an in-memory document. */ export function layoutDoc(doc: Doc, opts: LayoutDocOptions = {}): Promise { - return new RelayoutDiagramService(NO_PROJECT).relayout({ ...opts, source: { doc }, dryRun: true }); + return new RelayoutDiagramService(NO_PROJECT, NO_FILES, NO_FILES).relayout({ ...opts, source: { doc }, dryRun: true }); } /** What `bpmn validate` finds in a document. */ export function checkDoc(doc: Doc, opts: CheckOptions = {}): Promise { - return new ValidateDocumentService(NO_PROJECT).validate({ ...opts, source: { doc } }); + return new ValidateDocumentService(NO_PROJECT, NO_FILES).validate({ ...opts, source: { doc } }); } diff --git a/src/application/port/in/ValidateDocumentUseCase.ts b/src/application/port/in/ValidateDocumentUseCase.ts index b292eaa8..619a18d5 100644 --- a/src/application/port/in/ValidateDocumentUseCase.ts +++ b/src/application/port/in/ValidateDocumentUseCase.ts @@ -1,15 +1,13 @@ -import type { Warning } from '../../../domain/document/CliError.js'; -import type { DebugSink } from '../../../domain/diagram/layout/LayoutDebug.js'; import type { Doc } from '../../../domain/document/Doc.js'; import type { Platform, PlatformChoice, PlatformInfo } from '../../../domain/document/vendor/PlatformDetection.js'; import type { ValidateOptions, ValidationResult } from '../../../domain/validation/StructuralValidator.js'; -import type { NamedValidator, Validator } from '../../../domain/validation/validators.js'; +import type { NamedValidator } from '../../../domain/validation/validators.js'; import type { ProfileReport } from '../../../domain/validation/profiles/PlatformProfiles.js'; import type { BpmnlintValidatorOptions } from '../../../domain/validation/BpmnlintValidator.js'; import type { BpmnlintConfig, BpmnlintEngine, ResolvedBpmnlintConfig } from '../../../domain/validation/bpmnlint-validator.js'; import type { FileBpmnlintOptions } from '../out/LoadProjectSettingsPort.js'; +import type { CheckOptions, CheckResult } from '../../../domain/validation/DocumentCheck.js'; import type { DocumentSource } from './DocumentSource.js'; -import type { LayoutStatus } from '../../../domain/diagram/types.js'; /** * `bpmn validate`: what is wrong with a document, @@ -88,28 +86,7 @@ export declare namespace ValidateDocumentUseCase { } } -/** Options of `bpmn validate` (checkDoc, checkFile, validateXml). */ -export interface CheckOptions { - - platform?: PlatformChoice; - - /** their errors fail the check, an advisory one's too */ - validators?: Validator[]; - - /** the file being checked, for the validators (default doc.file) */ - file?: string; - - /** receives the layout engines' diagnostic lines during this call */ - debug?: DebugSink; -} - -/** What `bpmn validate` finds in a document (checkDoc). */ -export interface CheckResult { - - validation: ValidationResult; - layout: LayoutStatus | { status: 'failed'; error: Warning }; - importWarnings: string[]; -} +export type { CheckOptions, CheckResult }; /** Everything `bpmn validate` takes for a file. */ export interface FileCheckOptions extends CheckOptions, FileBpmnlintOptions {} diff --git a/src/application/service/CreateDocumentService.ts b/src/application/service/CreateDocumentService.ts index d08528c3..7c49556f 100644 --- a/src/application/service/CreateDocumentService.ts +++ b/src/application/service/CreateDocumentService.ts @@ -5,26 +5,37 @@ import type { CreateDocumentUseCase } from '../port/in/CreateDocumentUseCase.js' import type { LoadProjectSettingsPort } from '../port/out/LoadProjectSettingsPort.js'; import type { SaveDocumentPort } from '../port/out/SaveDocumentPort.js'; import { EditBatchService } from '../../domain/batch/EditBatchService.js'; +import type { MutationResult } from '../../domain/batch/types.js'; +import type { FileMutationOptions, FileWriteOptions } from '../port/in/EditDocumentUseCase.js'; export class CreateDocumentService implements CreateDocumentUseCase { constructor( private readonly settings: LoadProjectSettingsPort, - private readonly save?: SaveDocumentPort, + private readonly save: SaveDocumentPort, ) {} /** An existing file fails before the .bpmnlintrc is read. */ async create(command: CreateDocumentUseCase.Command): Promise { const { destination, ...opts } = command; if (opts.processId !== undefined) Doc.assertProcessId(opts.processId); - const target = opts.out ?? destination; - if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); - if (opts.mustNotExist) await this.save?.assertNew(target, opts); - const lint = await this.settings.lint(opts.file ?? target, opts); + const target = await this.targetOf(destination, opts); + const project = (await this.settings.lint(opts.file ?? target, opts)).applyTo(opts, target); const conventions = opts.conventions ?? (await this.settings.idConventions(target, opts))?.conventions; const doc = Doc.create({ processName: opts.processName, processId: opts.processId, executable: opts.executable, target: opts.target, ...(conventions ? { conventions } : {}) }, destination); - const result = await new EditBatchService(doc, [], lint.applyTo(opts, target)).run(target); - if (!opts.dryRun && target) result.written = await this.save!.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); + const result = await new EditBatchService(doc, [], project).run(target); result.changes.create(newProcessChange(doc, opts.processName)); + await this.write(result, doc, target, opts); return result; } + + private async targetOf(destination: string | undefined, opts: FileMutationOptions): Promise { + const target = opts.out ?? destination; + if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); + if (opts.mustNotExist) await this.save.assertNew(target, opts); + return target; + } + + private async write(result: MutationResult, doc: Doc, target: string | undefined, opts: FileWriteOptions): Promise { + if (!opts.dryRun && target) result.written = await this.save.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); + } } diff --git a/src/application/service/EditDocumentService.ts b/src/application/service/EditDocumentService.ts index 8f9cc50f..a0fcc207 100644 --- a/src/application/service/EditDocumentService.ts +++ b/src/application/service/EditDocumentService.ts @@ -1,33 +1,49 @@ import { Doc } from '../../domain/document/Doc.js'; import { ioError } from '../../domain/document/utils/errors.js'; -import type { EditDocumentUseCase } from '../port/in/EditDocumentUseCase.js'; +import type { DocumentSource } from '../port/in/DocumentSource.js'; +import type { EditDocumentUseCase, FileMutationOptions, FileWriteOptions } from '../port/in/EditDocumentUseCase.js'; import type { LoadDocumentPort } from '../port/out/LoadDocumentPort.js'; import type { LoadProjectSettingsPort } from '../port/out/LoadProjectSettingsPort.js'; import type { SaveDocumentPort } from '../port/out/SaveDocumentPort.js'; import { EditBatchService } from '../../domain/batch/EditBatchService.js'; +import type { MutationResult } from '../../domain/batch/types.js'; import { TextPreservation, type PreservedText } from '../../domain/document/xml/TextPreservation.js'; export class EditDocumentService implements EditDocumentUseCase { constructor( private readonly settings: LoadProjectSettingsPort, - private readonly load?: LoadDocumentPort, - private readonly save?: SaveDocumentPort, + private readonly load: LoadDocumentPort, + private readonly save: SaveDocumentPort, ) {} async edit(command: EditDocumentUseCase.Command): Promise { const { source, commands, ...opts } = command; - const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load!.read(source.file), source.file); - doc.assertLossless(opts); - const target = opts.out ?? doc.file; - if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); - if (opts.mustNotExist) await this.save?.assertNew(target, opts); - const lint = await this.settings.lint(opts.file ?? target, opts); - const result = await new EditBatchService(doc, commands, lint.applyTo(opts, target)).run(target); - if (!opts.dryRun && target) result.written = await this.save!.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); + const doc = await this.open(source, opts); + const target = await this.targetOf(doc, opts); + const project = (await this.settings.lint(opts.file ?? target, opts)).applyTo(opts, target); + const result = await new EditBatchService(doc, commands, project).run(target); + await this.write(result, doc, target, opts); return result; } preserveText(command: EditDocumentUseCase.PreserveTextCommand): Promise { return new TextPreservation(command.original, command.baseline, command.next).result(); } + + private async open(source: DocumentSource, opts: { force?: boolean }): Promise { + const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load.read(source.file), source.file); + doc.assertLossless(opts); + return doc; + } + + private async targetOf(doc: Doc, opts: FileMutationOptions): Promise { + const target = opts.out ?? doc.file; + if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); + if (opts.mustNotExist) await this.save.assertNew(target, opts); + return target; + } + + private async write(result: MutationResult, doc: Doc, target: string | undefined, opts: FileWriteOptions): Promise { + if (!opts.dryRun && target) result.written = await this.save.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); + } } diff --git a/src/application/service/ReadDocumentService.ts b/src/application/service/ReadDocumentService.ts index b2855b9a..1f0765b7 100644 --- a/src/application/service/ReadDocumentService.ts +++ b/src/application/service/ReadDocumentService.ts @@ -4,7 +4,7 @@ import type { LoadDocumentPort } from '../port/out/LoadDocumentPort.js'; /** Decoded in the encoding the XML declaration names, so ISO-8859-1 and Windows-1252 characters survive. */ export class ReadDocumentService implements ReadDocumentUseCase { - constructor(private readonly load?: LoadDocumentPort) {} + constructor(private readonly load: LoadDocumentPort) {} async read(command: ReadDocumentUseCase.Command): Promise { const doc = await this.parse(command.source); @@ -14,6 +14,6 @@ export class ReadDocumentService implements ReadDocumentUseCase { private async parse(source: ReadDocumentUseCase.Command['source']): Promise { if ('xml' in source) return Doc.fromXml(source.xml, source.file); - return 'file' in source ? Doc.fromXml(await this.load!.read(source.file), source.file) : Doc.fromXml(this.load!.decode(source.bytes, source.name)); + return 'file' in source ? Doc.fromXml(await this.load.read(source.file), source.file) : Doc.fromXml(this.load.decode(source.bytes, source.name)); } } diff --git a/src/application/service/RelayoutDiagramService.ts b/src/application/service/RelayoutDiagramService.ts index 3a4aa3ba..6451a8a7 100644 --- a/src/application/service/RelayoutDiagramService.ts +++ b/src/application/service/RelayoutDiagramService.ts @@ -5,7 +5,8 @@ import { kindLabel } from '../../domain/document/utils/kinds.js'; import { EditBatchService } from '../../domain/batch/EditBatchService.js'; import { FullLayout, type LayoutResult } from '../../domain/diagram/layout/FullLayout.js'; import { LayoutDebug } from '../../domain/diagram/layout/LayoutDebug.js'; -import type { LayoutDocOptions, RelayoutDiagramUseCase } from '../port/in/RelayoutDiagramUseCase.js'; +import type { FileLayoutOptions, LayoutDocOptions, RelayoutDiagramUseCase } from '../port/in/RelayoutDiagramUseCase.js'; +import type { DocumentSource } from '../port/in/DocumentSource.js'; import type { FileWriteOptions } from '../port/in/EditDocumentUseCase.js'; import type { MutationResult } from '../../domain/batch/types.js'; import type { LoadDocumentPort } from '../port/out/LoadDocumentPort.js'; @@ -16,21 +17,19 @@ import type { SaveDocumentPort } from '../port/out/SaveDocumentPort.js'; export class RelayoutDiagramService implements RelayoutDiagramUseCase { constructor( private readonly settings: LoadProjectSettingsPort, - private readonly load?: LoadDocumentPort, - private readonly save?: SaveDocumentPort, + private readonly load: LoadDocumentPort, + private readonly save: SaveDocumentPort, ) {} async relayout(command: RelayoutDiagramUseCase.Command): Promise { const { source, ...opts } = command; RelayoutDiagramService.assertOptions(opts); - const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load!.read(source.file), source.file); - doc.assertLossless(opts); - const target = opts.out ?? doc.file; - if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); - if (opts.mustNotExist) await this.save?.assertNew(target, opts); + const doc = await this.open(source, opts); + const target = await this.targetOf(doc, opts); const project = (await this.settings.lint(opts.file ?? target, opts)).applyTo(opts, target); - if (project.tidy) return this.write(doc, new EditBatchService(doc, [{ op: 'tidy' }], { ...project, layout: 'auto' }), target, opts); - return this.redraw(doc, project, target, opts); + const result = project.tidy ? await this.tidy(doc, project, target) : await this.redraw(doc, project, target); + await this.write(result, doc, target, opts); + return result; } layoutModel(command: RelayoutDiagramUseCase.ModelCommand): Promise { @@ -49,13 +48,17 @@ export class RelayoutDiagramService implements RelayoutDiagramUseCase { } } - private async redraw(doc: Doc, opts: LayoutDocOptions, target: string | undefined, write: FileWriteOptions): Promise { + private tidy(doc: Doc, opts: LayoutDocOptions, target: string | undefined): Promise { + return new EditBatchService(doc, [{ op: 'tidy' }], { ...opts, layout: 'auto' }).run(target); + } + + private async redraw(doc: Doc, opts: LayoutDocOptions, target: string | undefined): Promise { const expand = opts.expand ?? []; const collapse = opts.collapse ?? []; const subs = new Map([...expand, ...collapse].map((id) => [id, doc.require(id, SUB_PROCESS_TYPES, 'sub-process')])); const both = expand.filter((id) => collapse.includes(id)); if (both.length) throw usageError(`${both.join(', ')} given to both --expand and --collapse`); - const result = await this.write(doc, new EditBatchService(doc, [], { ...opts, layout: 'full', expand, collapse }), target, write); + const result = await new EditBatchService(doc, [], { ...opts, layout: 'full', expand, collapse }).run(target); for (const [id, expanded] of [...expand.map((id) => [id, true] as const), ...collapse.map((id) => [id, false] as const)]) { const sub = subs.get(id)!; const name = sub.get('name'); @@ -64,9 +67,20 @@ export class RelayoutDiagramService implements RelayoutDiagramUseCase { return result; } - private async write(doc: Doc, batch: EditBatchService, target: string | undefined, opts: FileWriteOptions): Promise { - const result = await batch.run(target); - if (!opts.dryRun && target) result.written = await this.save!.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); - return result; + private async open(source: DocumentSource, opts: { force?: boolean }): Promise { + const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load.read(source.file), source.file); + doc.assertLossless(opts); + return doc; + } + + private async targetOf(doc: Doc, opts: FileLayoutOptions): Promise { + const target = opts.out ?? doc.file; + if (!opts.dryRun && !target) throw ioError('E_NO_FILE', 'No output file given'); + if (opts.mustNotExist) await this.save.assertNew(target, opts); + return target; + } + + private async write(result: MutationResult, doc: Doc, target: string | undefined, opts: FileWriteOptions): Promise { + if (!opts.dryRun && target) result.written = await this.save.save(target, result.xml, { source: doc.file, unchanged: result.unchanged, backup: opts.backup }); } } diff --git a/src/application/service/ValidateDocumentService.ts b/src/application/service/ValidateDocumentService.ts index 5307fe51..4181da98 100644 --- a/src/application/service/ValidateDocumentService.ts +++ b/src/application/service/ValidateDocumentService.ts @@ -1,35 +1,28 @@ import { Doc } from '../../domain/document/Doc.js'; -import type { CliError } from '../../domain/document/CliError.js'; -import { LayoutDebug } from '../../domain/diagram/layout/LayoutDebug.js'; -import { FullLayout } from '../../domain/diagram/layout/FullLayout.js'; -import { reportedImportWarnings } from '../../domain/editing/utils/decision-link-warnings.js'; import { PlatformDetection, type PlatformInfo } from '../../domain/document/vendor/PlatformDetection.js'; import { PLATFORM_PROFILES, type ProfileReport } from '../../domain/validation/profiles/PlatformProfiles.js'; import { StructuralValidator, type ValidationResult } from '../../domain/validation/StructuralValidator.js'; import { BpmnlintValidator } from '../../domain/validation/BpmnlintValidator.js'; import { BpmnlintConfigResolution } from '../../domain/validation/BpmnlintConfigResolution.js'; +import { DocumentCheck } from '../../domain/validation/DocumentCheck.js'; import type { NamedValidator } from '../../domain/validation/validators.js'; import type { ResolvedBpmnlintConfig } from '../../domain/validation/bpmnlint-validator.js'; -import { ValidatorSet } from '../../domain/validation/ValidatorSet.js'; -import { counts, withValidatorReports } from '../../domain/validation/utils/validator-issues.js'; -import { once } from '../../domain/validation/utils/once.js'; -import type { ValidatorReport, ValidatorRun } from '../../domain/validation/validators.js'; -import { toCliError } from '../../domain/document/utils/cli-error.js'; -import type { CheckOptions, CheckResult, ValidateDocumentUseCase } from '../port/in/ValidateDocumentUseCase.js'; +import type { DocumentSource } from '../port/in/DocumentSource.js'; +import type { ValidateDocumentUseCase } from '../port/in/ValidateDocumentUseCase.js'; import type { LoadDocumentPort } from '../port/out/LoadDocumentPort.js'; import type { LoadProjectSettingsPort } from '../port/out/LoadProjectSettingsPort.js'; -/** The layout dry run redraws the in-memory document: pass a document of its own. */ export class ValidateDocumentService implements ValidateDocumentUseCase { constructor( private readonly settings: LoadProjectSettingsPort, - private readonly load?: LoadDocumentPort, + private readonly load: LoadDocumentPort, ) {} async validate(command: ValidateDocumentUseCase.Command): Promise { const { source, ...opts } = command; - const doc = 'doc' in source ? source.doc : await Doc.fromXml(await this.load!.read(source.file), source.file); - return this.check(doc, (await this.settings.lint(opts.file ?? doc.file, opts)).applyTo(opts, doc.file)); + const doc = await this.open(source); + const project = (await this.settings.lint(opts.file ?? doc.file, opts)).applyTo(opts, doc.file); + return new DocumentCheck(doc, project).run(); } validateStructure(command: ValidateDocumentUseCase.StructureCommand): ValidationResult { @@ -63,45 +56,7 @@ export class ValidateDocumentService implements ValidateDocumentUseCase { return new BpmnlintConfigResolution(engine, config, opts).resolve(); } - /** Validators run before the layout dry run redraws the model, so they see the document's diagram. */ - private async check(doc: Doc, opts: CheckOptions): Promise { - return LayoutDebug.during(opts.debug, async () => { - await PLATFORM_PROFILES.ensure(doc, opts.platform ?? 'auto'); - let validation = new StructuralValidator(doc).validate({ platform: opts.platform ?? 'auto' }); - const structuralErrors = validation.errors.length; - const importWarnings = reportedImportWarnings(doc).map((warning) => warning.message.split('\n')[0]!); - const file = opts.file ?? doc.file; - const validators = new ValidatorSet(opts.validators); - if (validators.size) { - const runs = await validators.run({ xml: once(async () => doc.source?.text ?? (await doc.toXml())), doc: async () => doc }, { ...(file ? { file } : {}), phase: 'check', platform: PlatformDetection.of(doc).resolve(opts.platform ?? 'auto').platform, ops: [] }); - validation = withValidatorReports(validation, ValidateDocumentService.reports(runs), runs); - } - let layout: CheckResult['layout']; - if (structuralErrors) { - layout = { status: 'skipped', warnings: [], expanded: [] }; - } else { - try { - const layoutResult = await new FullLayout(doc.model, {}).run(); - layout = { status: 'ok', warnings: layoutResult.warnings, expanded: layoutResult.expanded }; - } catch (err) { - const cliError = toCliError(err) as CliError; - layout = { status: 'failed', error: { code: cliError.code, message: cliError.message, element: cliError.details?.element as string | undefined } }; - } - } - return { validation, layout, importWarnings }; - }); - } - - private static reports(runs: ValidatorRun[]): ValidatorReport[] { - return runs.map((run) => ({ - name: run.name, - ...(run.detail ? { detail: run.detail } : {}), - ...(run.blocks === false ? { blocks: false as const } : {}), - errors: run.issues.filter((issue) => issue.severity === 'error'), - warnings: run.issues.filter((issue) => issue.severity === 'warning'), - preexisting: [], - resolved: [], - counts: counts(run.issues), - })); + private async open(source: DocumentSource): Promise { + return 'doc' in source ? source.doc : Doc.fromXml(await this.load.read(source.file), source.file); } } diff --git a/src/domain/validation/DocumentCheck.ts b/src/domain/validation/DocumentCheck.ts new file mode 100644 index 00000000..d2eddfbc --- /dev/null +++ b/src/domain/validation/DocumentCheck.ts @@ -0,0 +1,95 @@ +import type { Doc } from '../document/Doc.js'; +import type { CliError, Warning } from '../document/CliError.js'; +import type { LayoutStatus } from '../diagram/types.js'; +import { LayoutDebug, type DebugSink } from '../diagram/layout/LayoutDebug.js'; +import { FullLayout } from '../diagram/layout/FullLayout.js'; +import { reportedImportWarnings } from '../editing/utils/decision-link-warnings.js'; +import { PlatformDetection, type PlatformChoice } from '../document/vendor/PlatformDetection.js'; +import { PLATFORM_PROFILES } from './profiles/PlatformProfiles.js'; +import { StructuralValidator, type ValidationResult } from './StructuralValidator.js'; +import { ValidatorSet } from './ValidatorSet.js'; +import { counts, withValidatorReports } from './utils/validator-issues.js'; +import { once } from './utils/once.js'; +import type { Validator, ValidatorReport, ValidatorRun } from './validators.js'; +import { toCliError } from '../document/utils/cli-error.js'; + +/** Options of `bpmn validate` (checkDoc, checkFile, validateXml). */ +export interface CheckOptions { + + platform?: PlatformChoice; + + /** their errors fail the check, an advisory one's too */ + validators?: Validator[]; + + /** the file being checked, for the validators (default doc.file) */ + file?: string; + + /** receives the layout engines' diagnostic lines during this call */ + debug?: DebugSink; +} + +/** What `bpmn validate` finds in a document (checkDoc). */ +export interface CheckResult { + + validation: ValidationResult; + layout: LayoutStatus | { status: 'failed'; error: Warning }; + importWarnings: string[]; +} + +/** + * `bpmn validate` on one document: its structure, the validators, and a layout dry run. The + * validators run before the dry run redraws the model, so they see the document's diagram; the + * dry run redraws the in-memory document, so pass a document of its own. + */ +export class DocumentCheck { + constructor( + private readonly doc: Doc, + private readonly opts: CheckOptions, + ) {} + + run(): Promise { + return LayoutDebug.during(this.opts.debug, async () => { + const platform = this.opts.platform ?? 'auto'; + await PLATFORM_PROFILES.ensure(this.doc, platform); + let validation = new StructuralValidator(this.doc).validate({ platform }); + const structuralErrors = validation.errors.length; + const importWarnings = reportedImportWarnings(this.doc).map((warning) => warning.message.split('\n')[0]!); + const validators = new ValidatorSet(this.opts.validators); + if (validators.size) { + const runs = await this.runValidators(validators, platform); + validation = withValidatorReports(validation, DocumentCheck.reports(runs), runs); + } + const layout = structuralErrors ? { status: 'skipped' as const, warnings: [], expanded: [] } : await this.layoutDryRun(); + return { validation, layout, importWarnings }; + }); + } + + private runValidators(validators: ValidatorSet, platform: PlatformChoice): Promise { + const doc = this.doc; + const file = this.opts.file ?? doc.file; + return validators.run({ xml: once(async () => doc.source?.text ?? (await doc.toXml())), doc: async () => doc }, { ...(file ? { file } : {}), phase: 'check', platform: PlatformDetection.of(doc).resolve(platform).platform, ops: [] }); + } + + private async layoutDryRun(): Promise { + try { + const layoutResult = await new FullLayout(this.doc.model, {}).run(); + return { status: 'ok', warnings: layoutResult.warnings, expanded: layoutResult.expanded }; + } catch (err) { + const cliError = toCliError(err) as CliError; + return { status: 'failed', error: { code: cliError.code, message: cliError.message, element: cliError.details?.element as string | undefined } }; + } + } + + private static reports(runs: ValidatorRun[]): ValidatorReport[] { + return runs.map((run) => ({ + name: run.name, + ...(run.detail ? { detail: run.detail } : {}), + ...(run.blocks === false ? { blocks: false as const } : {}), + errors: run.issues.filter((issue) => issue.severity === 'error'), + warnings: run.issues.filter((issue) => issue.severity === 'warning'), + preexisting: [], + resolved: [], + counts: counts(run.issues), + })); + } +} diff --git a/src/index.ts b/src/index.ts index c1eff10a..50d8960f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -37,6 +37,7 @@ import { RenderSvgService } from './application/service/RenderSvgService.js'; import { EditDocumentService } from './application/service/EditDocumentService.js'; import { RelayoutDiagramService } from './application/service/RelayoutDiagramService.js'; import { NoProjectSettings } from './adapters/out/memory/NoProjectSettings.js'; +import { NoDocumentFiles } from './adapters/out/memory/NoDocumentFiles.js'; import { DetailText } from './adapters/in/report/DetailText.js'; import { LayoutText } from './adapters/in/report/LayoutText.js'; import { ModelViewText } from './adapters/in/report/ModelViewText.js'; @@ -52,6 +53,7 @@ import { advisoryOf } from './adapters/in/report/utils/findings.js'; import type { ExtensionInfo } from './domain/document/ExtensionLister.js'; import type { LayoutOptions, LayoutResult } from './domain/diagram/layout/FullLayout.js'; const NO_PROJECT = new NoProjectSettings(); +const NO_FILES = new NoDocumentFiles(); /* the in-memory API: strings in, strings / data out */ export { mutateDoc, layoutDoc, checkDoc, applyToXml, newXml, layoutXml, validateXml, viewXml, showXml, conventionsXml, metricsXml, findXml, extensionsXml, type EditOptions, type EditResult, type EditReport, type LayoutXmlOptions, type ShownView, type ValidateXmlOptions, type ViewOptions } from './api.js'; export { type MutationReport, type MutationReportLike, type MutationSummary, type ValidationReport, type WarningReport } from './adapters/in/report/command-report.js'; @@ -110,19 +112,19 @@ export { type LayoutDocOptions } from './application/port/in/RelayoutDiagramUseC export type { ValidationDelta } from './domain/validation/FindingBaseline.js'; /* text-preserving output, for hosts that serialise models themselves */ export { type PreservedText } from './domain/document/xml/TextPreservation.js'; -export function preserveText(original: string, baseline: string, next: string): Promise { return new EditDocumentService(NO_PROJECT).preserveText({ original, baseline, next }); } +export function preserveText(original: string, baseline: string, next: string): Promise { return new EditDocumentService(NO_PROJECT, NO_FILES, NO_FILES).preserveText({ original, baseline, next }); } /* a host's validators, run inside the write transaction and by validate */ export type { NamedValidator, Validator, ValidatorContext, ValidatorFinding, ValidatorFn, ValidatorIssue, ValidatorReport, ValidatorSeverity } from './domain/validation/validators.js'; /* the project's bpmnlint configuration as an advisory validator; bpmnlint itself is injected */ export { BPMNLINT_COVERS, BPMNLINT_VALIDATOR, type BpmnlintValidatorOptions } from './domain/validation/BpmnlintValidator.js'; export type { BpmnlintConfig, BpmnlintEngine, BpmnlintLinter, BpmnlintLinterClass, BpmnlintReport, BpmnlintResolver, BpmnlintRule, BpmnlintRuleSetting, BpmnlintSeverity, BpmnlintUnavailable, ResolvedBpmnlintConfig } from './domain/validation/bpmnlint-validator.js'; /** The advisory validator of a resolved bpmnlint configuration. */ -export function bpmnlintValidator(engine: BpmnlintEngine, resolved: ResolvedBpmnlintConfig, opts: BpmnlintValidatorOptions = {}): NamedValidator { return new ValidateDocumentService(NO_PROJECT).bpmnlintValidator({ ...opts, engine, resolved }); } +export function bpmnlintValidator(engine: BpmnlintEngine, resolved: ResolvedBpmnlintConfig, opts: BpmnlintValidatorOptions = {}): NamedValidator { return new ValidateDocumentService(NO_PROJECT, NO_FILES).bpmnlintValidator({ ...opts, engine, resolved }); } /** * The effective bpmnlint configuration; `opts.moddleExtensions` holds the descriptors the host * resolved for the module paths of `config.moddleExtensions`. */ -export async function resolveBpmnlintConfig(engine: BpmnlintEngine, config: BpmnlintConfig, opts: { moddleExtensions?: Record } = {}): Promise { return new ValidateDocumentService(NO_PROJECT).resolveBpmnlintConfig({ ...opts, engine, config }); } +export async function resolveBpmnlintConfig(engine: BpmnlintEngine, config: BpmnlintConfig, opts: { moddleExtensions?: Record } = {}): Promise { return new ValidateDocumentService(NO_PROJECT, NO_FILES).resolveBpmnlintConfig({ ...opts, engine, config }); } /* declared id conventions: what element-id-naming in the .bpmnlintrc says about new ids */ export { renderConventions } from './domain/document/utils/conventions-text.js'; export { ID_NAMING_RULE, type IdConventions, type IdNamingHelpers, type QualifierMode } from './domain/document/ids/IdConventionsReader.js'; @@ -150,7 +152,7 @@ export type { DecisionLink } from './domain/document/DecisionLinkReader.js'; export { ChangeSet, type Change } from './domain/document/ChangeSet.js'; export type { ValidateOptions, ValidationResult } from './domain/validation/StructuralValidator.js'; /** Structural validation and lint of a document, with the platform profile on request. */ -export function validateDoc(doc: Doc, opts: ValidateOptions = {}): ValidationResult { return new ValidateDocumentService(NO_PROJECT).validateStructure({ ...opts, doc }); } +export function validateDoc(doc: Doc, opts: ValidateOptions = {}): ValidationResult { return new ValidateDocumentService(NO_PROJECT, NO_FILES).validateStructure({ ...opts, doc }); } /* the platform profile: Camunda 7 / 8 rules, loaded on demand */ export { PLATFORM_CHOICES, TARGETS, type Platform, type PlatformChoice, type PlatformInfo } from './domain/document/vendor/PlatformDetection.js'; export type { PlatformSummary, ProfileReport } from './domain/validation/profiles/PlatformProfiles.js'; @@ -206,17 +208,17 @@ export function parseOps(input: unknown): EditCommand[] { * validateDoc; the async API loads them itself. A failed load (E_PROFILE_LOAD) is retried later. */ export function loadProfile(platform: Platform): Promise { - return new ValidateDocumentService(NO_PROJECT).loadProfile({ platform }); + return new ValidateDocumentService(NO_PROJECT, NO_FILES).loadProfile({ platform }); } /** Loads the engine rules of several platforms (default: all). */ export function loadProfiles(platforms?: readonly Exclude[]): Promise { - return new ValidateDocumentService(NO_PROJECT).loadProfiles(platforms !== undefined ? { platforms } : {}); + return new ValidateDocumentService(NO_PROJECT, NO_FILES).loadProfiles(platforms !== undefined ? { platforms } : {}); } /** Runs the profile of the document's platform (or `choice`); its rules must be loaded. */ export function runProfile(doc: Doc, choice?: PlatformChoice): ProfileReport { - return new ValidateDocumentService(NO_PROJECT).runProfile({ doc, ...(choice !== undefined ? { choice } : {}) }); + return new ValidateDocumentService(NO_PROJECT, NO_FILES).runProfile({ doc, ...(choice !== undefined ? { choice } : {}) }); } /** @@ -224,7 +226,7 @@ export function runProfile(doc: Doc, choice?: PlatformChoice): ProfileReport { * uses, else a declared one. */ export function detectPlatform(doc: Doc): PlatformInfo { - return new ValidateDocumentService(NO_PROJECT).detectPlatform({ doc }); + return new ValidateDocumentService(NO_PROJECT, NO_FILES).detectPlatform({ doc }); } /** `bpmn guide`: the agent cheat sheet. */ @@ -282,5 +284,5 @@ export function locateProblems(defs: El, problems: readonly LayoutProblem[]): La * treat the returned XML as the truth and re-parse it if you need DI. */ export function layoutModel(model: Model, opts: LayoutOptions = {}): Promise { - return new RelayoutDiagramService(NO_PROJECT).layoutModel({ ...opts, model }); + return new RelayoutDiagramService(NO_PROJECT, NO_FILES, NO_FILES).layoutModel({ ...opts, model }); } From b46c3907976869fc9da7a9e9b69e14573fd30474 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Tue, 27 Oct 2026 18:19:03 +0100 Subject: [PATCH 11/11] refactor: RenderDiagramService reads the text in a step of its own before it writes the image --- src/application/service/RenderDiagramService.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/application/service/RenderDiagramService.ts b/src/application/service/RenderDiagramService.ts index ba32387a..e12bdc44 100644 --- a/src/application/service/RenderDiagramService.ts +++ b/src/application/service/RenderDiagramService.ts @@ -10,7 +10,12 @@ export class RenderDiagramService implements RenderDiagramUseCase { async render(command: RenderDiagramUseCase.Command): Promise { const { source, ...request } = command; - const xml = 'xml' in source ? source.xml : 'file' in source ? await this.load.read(source.file) : this.load.decode(source.bytes, source.name); + const xml = await this.textOf(source); return this.images.write({ xml, ...request }); } + + private async textOf(source: RenderDiagramUseCase.Command['source']): Promise { + if ('xml' in source) return source.xml; + return 'file' in source ? this.load.read(source.file) : this.load.decode(source.bytes, source.name); + } }