diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0be60055..f709813e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,8 +96,7 @@ them on a private corpus without copying it into the repository, are described i interface it exercises. Black-box tests through the entries and the CLI are in `test/e2e/`, shared helpers in `test/helpers/`; `test/repo-hygiene.test.ts` keeps it that way. Name a test after what it tests, never after the work step that added it. The suite also pins the documentation: the README quick start and the - examples in `docs/` must match what the CLI prints. `test/helpers/setup.ts` clears `BPMN_KEEP_FLOW_IDS` - of your shell before every file, so the tests see the CLI's defaults. + examples in `docs/` must match what the CLI prints. - `tools/`: benchmark, fuzzer, layout regression, isomorphism check, descriptor generator. The README demo (`docs/assets/demo.gif`) comes from the `product-shots` skill of the `miragon-brand` plugin ([Miragon/corporate-identity](https://github.com/Miragon/corporate-identity)), diff --git a/docs/commands.md b/docs/commands.md index 6e0231bd..60ef7727 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -45,12 +45,8 @@ keys are in [Syntax and output](reference.md). | `--force` | write despite a lossy import, new validation errors, or an existing file (`new`) | | `--backup` | copy `` to `.bak` first | | `--strict` | exit code 5 when the result has warnings | -| `--keep-flow-ids` | keep the ids of flows whose ends change; `W_FLOW_ID_STALE` names each id that no longer fits | | `--lint-config `, `--no-lint-config` | choose or ignore the project's [bpmnlint](bpmnlint.md) configuration | -`BPMN_KEEP_FLOW_IDS=1` makes `--keep-flow-ids` the default. A result whose ids it kept says so with -`note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 (...)`. - ## `new` ```sh diff --git a/docs/contract.md b/docs/contract.md index 4c419d88..f4f8ca72 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -65,10 +65,8 @@ A write rewrites only the elements the change touched, so a git diff shows only - Broken links in hand-edited files are not repaired; `bpmn validate` reports them. - The Camunda 8 profile checks FEEL syntax, not meaning. The engine finds the rest. - A flow id that names its ends is renamed when an edit changes its ends (a splice, a - `move`, `set source=`). Where flow ids are API (generated code, tests), `--keep-flow-ids` keeps - them, and `W_FLOW_ID_STALE` names each flow whose id no longer fits, with the `bpmn set` that - renames it. `BPMN_KEEP_FLOW_IDS=1` makes that the default of a shell - ([Environment](reference.md#environment)). Library: `keepFlowIds: true`. + `move`, `set source=`). Code that names flow ids (generated code, tests) follows the + `renamed:` report. ::: warning Merges can produce duplicate ids Two branches that each add an element of the same name get the same id. The next read refuses the diff --git a/docs/library.md b/docs/library.md index 18652a96..cc860df4 100644 --- a/docs/library.md +++ b/docs/library.md @@ -97,7 +97,6 @@ the CLI prints with `--json`. `renderMutation(result)` gives the CLI's text. | `layout` | `'auto'` (default), `'incremental'`, `'full'`, `false` | `--layout`, `--no-layout` | | `force` | write despite a guard | `--force` | | `platform` | `'auto'`, `'c7'`, `'c8'`, `'none'` | `--platform` | -| `keepFlowIds` | keep flow ids; `W_FLOW_ID_STALE` names each stale one | `--keep-flow-ids` | | `validators` | host validators ([Validators](validators.md)) | none | | `conventions` | declared id conventions (below) | the `.bpmnlintrc` | diff --git a/docs/ops.md b/docs/ops.md index f64b52b1..08da7126 100644 --- a/docs/ops.md +++ b/docs/ops.md @@ -100,12 +100,6 @@ creates (new node -> old successor), so the same batch can give it a condition: } ``` -## Flow ids in a batch - -With `apply --keep-flow-ids` a flow the file had keeps its id through the whole batch unless an op -removes it, and the result names stale ids with `W_FLOW_ID_STALE` instead of renaming them. -A `move` puts the moved node's out-flow back at its new place, so a flow between nodes moved -together stays theirs. ## A full example diff --git a/docs/reference.md b/docs/reference.md index 183be4fd..d721d306 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -27,8 +27,7 @@ Hashes and running numbers are never copied. An unknown id (`E_NOT_FOUND`) comes were probably meant. A flow whose id names its ends in this form is renamed when an edit changes them (`renamed:` in the -result); an older flow id (`Flow_AToB`) keeps its id. `--keep-flow-ids` keeps the id and reports -`W_FLOW_ID_STALE` with the `bpmn set` that renames it. +result); an older flow id (`Flow_AToB`) stays as it is. Names become ASCII words (`Prüfung buchen` gives `userTask_pruefungBuchen`), at most 40 characters of them: a longer name loses its last words, and a single longer word is cut at 40. An id has at @@ -167,14 +166,9 @@ fix: `bpmn kinds --section errors`. `-` as the file reads the model from stdin. | variable | effect | | --- | --- | -| `BPMN_KEEP_FLOW_IDS` | `1`, `true`, `yes`, `on`: every mutating command keeps flow ids, as with `--keep-flow-ids` | | `BPMN_LAYOUT_DEBUG` | layout diagnostics on stderr | | `BPMN_DEBUG` | the stack trace of an internal error | -Where `BPMN_KEEP_FLOW_IDS=1` changed a result, the result says so, since the reader may not see -the variable: `note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 (--no-keep-flow-ids renames flows after -their new ends for one command)`. - ## Quoting ::: warning Expressions in single quotes diff --git a/docs/testing.md b/docs/testing.md index c060e8a6..ebf1a0e1 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -25,12 +25,6 @@ bugs are open, a random campaign would hit them by chance. Layout quality is gated deterministically by the property test's fixed walks, the regression budget and `bench --compare`. -None of the layers depends on the shell's `BPMN_KEEP_FLOW_IDS`: the unit -tests clear it before every file (`test/helpers/setup.ts`), so the CLIs they spawn -rename flows as by default, and the fuzzer, the benchmark, the layout -regression and the demo run the CLI without it (`CLI_ENV` in -`tools/fuzz/lib/setup.mjs`). A test that needs it sets it for its own spawn. - ## Invariants The fuzzer and the property test check after every step @@ -632,10 +626,9 @@ conventions, else element-id-naming's defaults, whatever ids the file has; bpmn- the types the rule does not check; the guard that `DEFAULT_ID_CONVENTIONS` equals the plugin's defaults), `test/domain/document/ids/speaking-ids.test.ts` and `test/domain/document/ids/id-style.test.ts` (renamed flows and their DI, also after an end got -another id earlier in the batch; a flow id in an older form that keeps its id), +another id earlier in the batch; a flow id in an older form that stays as it is), `test/domain/document/ids/id-quality.test.ts` (cuts to 64 characters, collisions, -transliteration), `test/domain/editing/keep-flow-ids.test.ts` (flow ids kept with -`--keep-flow-ids`, `W_FLOW_ID_STALE`, an end renamed or named earlier, `--flow-id` on a splice), +transliteration), `test/domain/validation/conventions-*.test.ts` (declared conventions and the reader of the `.bpmnlintrc`), `test/domain/batch/batch-aliases.test.ts` (aliases in `apply`, on the command line too), `test/domain/document/not-found.test.ts` (the candidates of `E_NOT_FOUND`), diff --git a/plugins/bpmn-cli/skills/bpmn/SKILL.md b/plugins/bpmn-cli/skills/bpmn/SKILL.md index 92dfcf26..467dd219 100644 --- a/plugins/bpmn-cli/skills/bpmn/SKILL.md +++ b/plugins/bpmn-cli/skills/bpmn/SKILL.md @@ -119,19 +119,7 @@ installed version. Ask it instead of guessing: - `created` / `changed` / `removed` lines and `renamed: A -> B` (a flow whose id named its ends is renamed when a splice changes them: use the new id) - when flow ids are API (code generated from the model, e.g. bpmn-to-code, - or tests that name them), keep them: pass `--keep-flow-ids` to every - mutating command and `apply`, or ask the user to set `BPMN_KEEP_FLOW_IDS` - to `1` in the `env` of their Claude Code settings (an inline - `BPMN_KEEP_FLOW_IDS=1 bpmn ...` is not pre-approved). Then - `warning W_FLOW_ID_STALE ` names a flow whose id no longer names - its ends: tell the user; rename it (the `bpmn set` of its hint) only - together with the code that uses it. A result line - `note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 (...)` means the user's - setting kept them although you passed no flag, and that made this - result differ: a `W_FLOW_ID_STALE` instead of `renamed:`, or a move - that put the node's old out-flow back instead of a new flow. An - `E_NOT_FOUND` in an `apply` batch whose hint names the variable: use - the flow's old id + or tests that name them), tell the user each `renamed:` so the code follows - `warning ...` lines are only the warnings **this change added**, each with a hint that is the fixing command; `resolved:` what it fixed; old warnings are only counted @@ -306,7 +294,7 @@ Every command, selector and fix: [reference/formatting.md](reference/formatting. | `E_NO_ROOM` | Run the `bpmn space ...` command of the hint (checked to make the room), then place again; in a batch after other ops the hint names the space op to put before the place op; without one, move the shapes it names first. | | `E_UNKNOWN_ALIAS` | Define the alias (`as` / `flowAs` / `outAs` / `joinAs` / `refAs`) in an earlier op of the batch. | | `E_IMPORT_LOSSY` | The file has content the reader would drop. Show the user `bpmn validate` output and ask before `--force`; do not repair the XML by hand. | -| `E_USAGE` | Bad flag, ops key or `BPMN_KEEP_FLOW_IDS` value: the message names it; see `bpmn --help`. A bad `BPMN_KEEP_FLOW_IDS` is in the user's settings: tell them; `--keep-flow-ids` / `--no-keep-flow-ids` decide for one command. | +| `E_USAGE` | Bad flag or ops key: the message names it; see `bpmn --help`. | Every code with its fix: `bpmn kinds --section errors`. diff --git a/plugins/bpmn-cli/skills/bpmn/reference/recipes.md b/plugins/bpmn-cli/skills/bpmn/reference/recipes.md index dd4c5b59..cac623f2 100644 --- a/plugins/bpmn-cli/skills/bpmn/reference/recipes.md +++ b/plugins/bpmn-cli/skills/bpmn/reference/recipes.md @@ -76,8 +76,7 @@ printf '%s' '[ The dry run prints exactly what the write will do: the created ids, the flow that was split (`flow` splices into it; it keeps its name and condition, and a flow whose id named its old ends is renamed, here -`flow_checkOrderToOrderOk -> flow_checkOrderToCheckCreditLimit`, unless flow -ids are kept with `--keep-flow-ids`), what the +`flow_checkOrderToOrderOk -> flow_checkOrderToCheckCreditLimit`), what the layout `placed` / `moved` / `rerouted`, and `layout quality: a -> b` with any `added:` problem. When it looks right, run the same batch without `--dry-run`: diff --git a/src/adapters/in/cli/MutationCommand.ts b/src/adapters/in/cli/MutationCommand.ts index 74383e78..5c1ade8d 100644 --- a/src/adapters/in/cli/MutationCommand.ts +++ b/src/adapters/in/cli/MutationCommand.ts @@ -9,7 +9,7 @@ import type { MutationResult } from '../../../domain/batch/types.js'; import { LAYOUT_MODES, type LayoutMode } from '../../../domain/diagram/types.js'; import { RENDER_MAX_SIZE, type RenderOptions } from '../../../application/port/in/RenderSvgUseCase.js'; import { changeHighlight } from '../report/utils/highlight.js'; -import { type MutationReport, type MutationSummary } from '../report/command-report.js'; +import { type MutationReport } from '../report/command-report.js'; import { RenderReport } from './RenderReport.js'; import { isBpmnError, usageError } from '../../../domain/document/utils/errors.js'; import { CliCommand, type RawOpts } from './CliCommand.js'; @@ -33,8 +33,6 @@ export interface OutputOptions { summary?: boolean; /** --render : draw the result to this image */ render?: string; - /** BPMN_KEEP_FLOW_IDS when it, not a flag, kept the flow ids (the result then says so) */ - keepFlowIdsEnv?: string; } export type MutationFlags = FileMutationOptions & OutputOptions; @@ -61,9 +59,7 @@ export abstract class MutationCommand extends CliCommand { .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('--strict')); } protected withTriggerOptions(cmd: CommandType): CommandType { @@ -80,7 +76,6 @@ export abstract class MutationCommand extends CliCommand { } protected mutationOptions(options: RawOpts): MutationFlags { - const { keep: keepFlowIds, env: keepFlowIdsEnv } = this.keepFlowIdsOption(options); return { json: !!options['json'], pretty: !!options['pretty'], @@ -93,8 +88,6 @@ export abstract class MutationCommand extends CliCommand { show: !!options['show'], strict: !!options['strict'], ...(options['render'] !== undefined ? { render: String(options['render']) } : {}), - ...(keepFlowIds !== undefined ? { keepFlowIds } : {}), - ...(keepFlowIdsEnv !== undefined ? { keepFlowIdsEnv } : {}), ...this.lintOptions(options), }; } @@ -131,7 +124,7 @@ export abstract class MutationCommand extends CliCommand { }, ); } catch (err) { - throw this.withFile(this.keptIdsHint(err, commands, opts.keepFlowIdsEnv), file); + throw this.withFile(err, file); } } @@ -162,20 +155,15 @@ export abstract class MutationCommand extends CliCommand { await this.printMutation(result, opts, xmlOut ? this.cli.io.stderr : this.cli.io.stdout); } - /** - * --strict exits 5 on any warning; flow ids BPMN_KEEP_FLOW_IDS kept add a note, - * also to --summary. - */ + /** --strict exits 5 on any warning. */ private async printMutation(result: MutationResult, opts: OutputOptions & { show?: boolean; dryRun?: boolean }, out: CliStream): Promise { - const note = this.keptByEnvNote(result, opts.keepFlowIdsEnv); - if (note) result.changes.note(note); const report = new MutationReportBuilder(result).report(); const shot = opts.render ? await this.renderResult(result.xml, report, opts.render) : undefined; const shotText = shot ? `\n${shot.lines.join('\n')}` : ''; if (opts.summary) { const summary = new MutationSummaryBuilder(report).summary(); - if (opts.json) this.cli.printJson({ ...summary, ...(note ? { notes: [note] } : {}), ...(report.view ? { view: report.view } : {}), ...(shot ? { render: shot.json } : {}) }, opts.pretty, out); - else this.cli.print(this.summaryText(summary, { dryRun: opts.dryRun, advisory: advisoryOf((report).validation.validators) }, note) + shotText + (report.view ? `\n\n${new ModelViewText(report.view).text()}` : ''), out); + if (opts.json) this.cli.printJson({ ...summary, ...(report.view ? { view: report.view } : {}), ...(shot ? { render: shot.json } : {}) }, opts.pretty, out); + else this.cli.print(new SummaryText(summary, { dryRun: opts.dryRun, advisory: advisoryOf((report).validation.validators) }).text() + shotText + (report.view ? `\n\n${new ModelViewText(report.view).text()}` : ''), out); } else if (opts.json) this.cli.printJson(shot ? { ...report, render: shot.json } : report, opts.pretty, out); else this.cli.print(new MutationText(report, { dryRun: opts.dryRun }).text().replace(/\n$/, '') + shotText, out); if (opts.strict && new MutationReportBuilder(result).warnings().length) this.cli.exitCode = 5; @@ -198,45 +186,6 @@ export abstract class MutationCommand extends CliCommand { } } - /** - * A note when BPMN_KEEP_FLOW_IDS, not a flag, kept flow ids where the default differs: - * a stale flow id, or a move that put the moved node's old out-flow back. The variable - * sits in a shell or an agent's settings, where whoever reads the result does not see it. - */ - private keptByEnvNote(result: MutationResult, value: string | undefined): string | undefined { - if (value === undefined) return undefined; - const { created, changed, warnings, relocated } = result.changes; - const putBack = relocated.length > 0 && changed.some((change) => change.kind === 'sequenceFlow' && !created.some((creation) => creation.id === change.id)); - if (!putBack && !warnings.some((warning) => warning.code === 'W_FLOW_ID_STALE')) return undefined; - return `flow ids kept: BPMN_KEEP_FLOW_IDS=${value} (--no-keep-flow-ids renames flows after their new ends for one command)`; - } - - /** - * renderSummary with `note` after the change lines, where the full result has its notes - * (the summary prints no notes of its own). - */ - private summaryText(summary: MutationSummary, opts: ConstructorParameters[1], note: string | undefined): string { - const text = new SummaryText(summary, opts).text(); - if (!note) return text; - const { aliases: _aliases, renamed: _renamed, format: _format, ...rest } = summary; - const below = new SummaryText({ ...rest, created: {}, changed: [], removed: [] }, opts).text().split('\n').length - 1; - const lines = text.split('\n'); - lines.splice(lines.length - below, 0, `note: ${note}`); - return lines.join('\n'); - } - - /** - * With kept flow ids, a missing id may be the one the default gives a flow an earlier op - * rewired; the hint says so, as whoever reads the error may not see the variable. - */ - private keptIdsHint(err: unknown, commands: readonly EditCommand[], value: string | undefined): unknown { - if (value === undefined || commands.length < 2 || !isBpmnError(err) || err.code !== 'E_NOT_FOUND') return err; - const kept = `If it is the id a flow an earlier op gave other ends would be renamed to: flow ids are kept here (BPMN_KEEP_FLOW_IDS=${value}), so that flow kept its old id; use the old id, or --no-keep-flow-ids to rename it after its new ends.`; - const hint = err.details.hint; - err.details.hint = typeof hint === 'string' && hint ? `${hint} ${kept}` : kept; - return err; - } - /** Puts the file name into a refusal's runnable command (E_NO_ROOM) and its hint. */ private withFile(err: unknown, file: string): unknown { if (!isBpmnError(err) || typeof err.details['command'] !== 'string' || file === '-') return err; @@ -258,28 +207,4 @@ export abstract class MutationCommand extends CliCommand { if (layout === false) return false; return (layout as LayoutMode | undefined) ?? 'auto'; } - - /** - * --keep-flow-ids / --no-keep-flow-ids, else BPMN_KEEP_FLOW_IDS; `env`: the variable's - * value when it kept the ids. - */ - private keepFlowIdsOption(options: RawOpts): { keep?: boolean; env?: string } { - const flag = options['keepFlowIds'] as boolean | undefined; - if (flag !== undefined) return { keep: flag }; - const keep = this.envSwitch('BPMN_KEEP_FLOW_IDS', 'Set BPMN_KEEP_FLOW_IDS=1 (true, yes, on) to keep flow ids, BPMN_KEEP_FLOW_IDS=0 (false, no, off) or unset it to rename them after their new ends; --keep-flow-ids / --no-keep-flow-ids decide for one command.'); - if (keep === undefined) return {}; - return keep ? { keep, env: this.cli.io.env['BPMN_KEEP_FLOW_IDS']!.trim() } : { keep }; - } - - /** - * Unset or empty: undefined; 1 / true / yes / on: true; 0 / false / no / off: false. - * Anything else is E_USAGE: a typo must not silently mean on or off. - */ - private envSwitch(name: string, hint: string): boolean | undefined { - const value = this.cli.io.env[name]?.trim(); - if (!value) return undefined; - if (/^(1|true|yes|on)$/i.test(value)) return true; - if (/^(0|false|no|off)$/i.test(value)) return false; - throw usageError(`${name}=${value} is not a switch value`, { hint }); - } } diff --git a/src/adapters/in/report/AgentGuide.ts b/src/adapters/in/report/AgentGuide.ts index f9571cd2..437596de 100644 --- a/src/adapters/in/report/AgentGuide.ts +++ b/src/adapters/in/report/AgentGuide.ts @@ -156,17 +156,14 @@ const CONVENTIONS = [ const OUTPUT = [ ' A mutating command reports what changed, the warnings the change ADDED (three or more of one code as one line:', - ' `warning W_UNREACHABLE x20 [ids]: ...`; W_FLOW_ID_STALE keeps one line each), `resolved:` what it fixed, and one', + ' `warning W_UNREACHABLE x20 [ids]: ...`), `resolved:` what it fixed, and one', ' line counting the warnings the file already had (the platform profile\'s W_C7_* / W_C8_* among them; not repeated:', ' `bpmn validate ` lists them); then the layout block and the file line. A group warning (W_DUPLICATE_NAME)', ' that only shrank is the old one; an apply batch reports the warnings of its final state. The project\'s bpmnlint', ' (see BPMNLINT) is advisory: an error of it the change added is listed among the warnings', - ' (`warning [bpmnlint error] : ...`). Where', - ' BPMN_KEEP_FLOW_IDS=1 kept the flow ids (no --keep-flow-ids / --no-keep-flow-ids given) and that changed the', - ' result (W_FLOW_ID_STALE, or a move put the node\'s old out-flow back), it says so, also with --summary:', - ' `note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 (...)` (JSON: notes); an E_NOT_FOUND of an apply batch, in its hint.', + ' (`warning [bpmnlint error] : ...`).', ' --summary: `created : ids` per kind, `aliases:` (apply), `changed:`, `renamed: -> ` (a flow whose id', - ' named its old ends: use the new id; with --keep-flow-ids it keeps its id and W_FLOW_ID_STALE names it) and', + ' named its old ends: use the new id) and', ' `removed:` ids, one `format #: ...` line per format op, the added warnings, one `warnings:` count line (what', ' is already in the file includes the project\'s bpmnlint findings) and', ' one layout line (`layout: incremental, score 12 -> 14; added: crossings [flow_aToB, flow_cToD]`).', @@ -227,9 +224,7 @@ export class AgentGuide { ' taken id gets 2 (W_ID_SUFFIXED; an unnamed gateway or task says its kind first, gateway_parallelAfterCheck,', ' then the unnamed anchor it follows, task_afterCheckGateway).', ' A flow names an unnamed gateway by its id (flow_checkToAfterCheck). A flow whose id names its ends is', - ' renamed when a splice or bridge changes them (the result says so). Where flow ids are API (generated code,', - ' process tests), --keep-flow-ids or BPMN_KEEP_FLOW_IDS=1 keeps them; W_FLOW_ID_STALE then names each flow', - ' whose id no longer names its ends, with the `bpmn set` that renames it.', + ' renamed when a splice or bridge changes them (the result says so).', ' Every result names the ids it created; use them in the next command. In `apply`, name what an op creates', ' with an alias ("as": "$check"; add: "flowAs" for the flow into the node, "outAs" for the flow it creates out of', ' it; split: "joinAs"; "refAs": the message, error, signal or escalation the element references) and use the', @@ -387,8 +382,7 @@ export class AgentGuide { out.push(' validation: {errors, platform?, validators?: [{name, detail?, blocks?, errors, warnings, preexisting, resolved, counts,'); out.push(' leftOut?: [{reason, count}]}]}, importWarnings, view?, conventions?: {ids, source}}. With --summary --json: {ok, file,'); out.push(' written, unchanged, created: {: [ids]}, changed, removed, forced?, warnings: {added, resolvedCount, preexistingCount},'); - out.push(' leftOut?: [{validator, reason, count}], layout: {status, mode?, score?: {before?, after}, added?}, conventions?, notes? (only'); - out.push(' the note of flow ids BPMN_KEEP_FLOW_IDS kept, see OUTPUT)}.'); + out.push(' leftOut?: [{validator, reason, count}], layout: {status, mode?, score?: {before?, after}, added?}, conventions?}.'); out.push(' A validator\'s findings (the project\'s bpmnlint; a host\'s validators in library use) carry `validator` and'); out.push(' `severity`; in text they read "[] : " ("[bpmnlint error] ..." for a bpmnlint'); out.push(' error, "warning [bpmnlint error] ..." when a write added it: bpmnlint is advisory, `blocks: false`).'); @@ -443,7 +437,6 @@ export class AgentGuide { ' bpmn add [name] [--after|--before|--flow|--in|--on ] [--id ] [key=value ...]', ' bpmn connect|set|remove|retype|move|order|ext ... bpmn apply ops.json (one transaction, one layout)', ' Options: --summary (short result), --dry-run, -o , --json [--pretty]; `-` as reads stdin.', - ' Flow ids are API (generated code, tests)? --keep-flow-ids keeps them; W_FLOW_ID_STALE names a stale one.', ].join('\n'), ); out.push(heading('OPS JSON (bpmn apply; {"op": "", ...}; * = required)')); diff --git a/src/adapters/in/report/CommandCatalogue.ts b/src/adapters/in/report/CommandCatalogue.ts index ad496deb..d3e34a47 100644 --- a/src/adapters/in/report/CommandCatalogue.ts +++ b/src/adapters/in/report/CommandCatalogue.ts @@ -71,8 +71,6 @@ export const COMMON_OPTIONS: CommonOptionDoc[] = [ { option: '--backup', description: 'write .bak before overwriting', help: 'copy the input file to .bak before writing' }, { option: '--show', description: 'append the full model view to the result' }, { option: '--strict', description: 'exit 5 when the result has warnings', help: 'exit with code 5 when there are warnings' }, - { option: '--keep-flow-ids', description: 'keep the id of every existing sequence flow the change gives other ends, where flow ids are API (generated code, process tests); W_FLOW_ID_STALE names each flow whose id no longer names its ends, with the `bpmn set id=` that renames it. Default: a flow whose id names its old ends is renamed after its new ones (`renamed:`). BPMN_KEEP_FLOW_IDS=1 (true, yes, on) makes keeping the default, 0 (false, no, off) or unset renames, any other value is E_USAGE', help: 'keep the id of every existing sequence flow the change gives other ends, instead of renaming a flow whose id names its old ends after its new ones; W_FLOW_ID_STALE names each flow whose id no longer names its ends. BPMN_KEEP_FLOW_IDS=1 makes keeping the default' }, - { option: '--no-keep-flow-ids', description: 'rename such flows after their new ends, also when BPMN_KEEP_FLOW_IDS=1' }, ]; export function commandDoc(name: string): CommandDoc { diff --git a/src/adapters/in/report/KindsReference.ts b/src/adapters/in/report/KindsReference.ts index d5b5125a..aa8f4716 100644 --- a/src/adapters/in/report/KindsReference.ts +++ b/src/adapters/in/report/KindsReference.ts @@ -162,15 +162,8 @@ export class KindsReference { out.push('runs (E_UNKNOWN_ALIAS, listing the aliases defined so far); the result lists alias -> final id ("aliases:" line,'); out.push('`aliases` in --json) and the old -> new id of a flow it renamed after its new ends ("renamed:" with --summary, `renamed`).'); out.push('An alias follows its element through a rename and a retype (also to another type: create, retype, then configure).'); - out.push('With --keep-flow-ids a flow the file had keeps its id through the whole batch unless an op removes it (the flows of a'); - out.push('removed node, the out-flow a bridge replaces: "removed:"), so later ops can use it; a move puts the moved node\'s'); - out.push('out-flow back at its new place, with its id, documentation and extensions (its name, condition and annotations stay'); - out.push('with the bridge, its change says which, and it is no longer a default flow; a node that is not bridged, or whose'); - out.push('bridge a later node of the move removed, drops name and condition, W_LABEL_DROPPED, W_CONDITION_DROPPED, unless the'); - out.push('flow is put back between its old ends; a "flowId" on a move that prepends names a new flow node -> anchor instead).'); - out.push('The result names the stale ones (W_FLOW_ID_STALE) instead. A flow the batch created still follows its ends: name it'); - out.push('where an op creates it, with "flowAs" on an add that appends or prepends and on split nodes, "outAs" on an add that'); - out.push('splices, prepends or has "to", or "as" on connect. On a splice "flowAs" names the kept flow into the node and "outAs"'); + out.push('Name a flow where an op creates it, with "flowAs" on an add that appends or prepends and on split nodes, "outAs"'); + out.push('on an add that splices, prepends or has "to", or "as" on connect. On a splice "flowAs" names the kept flow into the node and "outAs"'); out.push('the new flow node -> old successor, so the batch that splices a gateway in can set that flow\'s condition or make it'); out.push('the default.'); out.push('\nExample (after `bpmn new order.bpmn --name "Order handling" --target camunda8`, start -> "Check invoice" -> end):'); diff --git a/src/adapters/in/report/WarningLines.ts b/src/adapters/in/report/WarningLines.ts index f0bc09b7..8dee9f1b 100644 --- a/src/adapters/in/report/WarningLines.ts +++ b/src/adapters/in/report/WarningLines.ts @@ -2,9 +2,6 @@ import type { Finding } from '../../../domain/document/Finding.js'; import { renderProblems } from './utils/result-lines.js'; import { FLOOD_MIN, floodKey, idsText, validatorTag, warningTag } from './utils/findings.js'; -/** Never flooded: each hint is the command for that one element. */ -const NEVER_FLOOD = new Set(['W_FLOW_ID_STALE']); - /** bpmn-cli's own codes about an advisory validator: not rules, one line each. */ const NO_RULE = new Set(['W_LINT_UNAVAILABLE', 'W_LINT_FAILED', 'W_CONFIG_IGNORED', 'import-warning']); @@ -27,7 +24,7 @@ export class WarningLines { for (const warning of warnings) { const key = keyOf(warning); const group = groups.get(key)!; - if (group.length < FLOOD_MIN || NEVER_FLOOD.has(warning.code)) { + if (group.length < FLOOD_MIN) { out.push(WarningLines.warningLine(warning)); continue; } diff --git a/src/adapters/in/report/guide.ts b/src/adapters/in/report/guide.ts index 75d56087..6e310fd4 100644 --- a/src/adapters/in/report/guide.ts +++ b/src/adapters/in/report/guide.ts @@ -16,7 +16,7 @@ export interface ErrorDoc { /** Usage and I/O codes. */ const USAGE_AND_IO_ERRORS: ErrorDoc[] = [ - { code: 'E_USAGE', meaning: 'Bad command line, ops JSON or environment value (unknown command, option, op, key or value type; a BPMN_KEEP_FLOW_IDS that is no switch value: 1 / true / yes / on, 0 / false / no / off).', fix: 'Read the message (it names the op index and the allowed keys), then `bpmn --help` or `bpmn guide`. A bad BPMN_KEEP_FLOW_IDS: set it to 1 or 0, or unset it, where it is set (shell, Claude Code settings `env`); --keep-flow-ids / --no-keep-flow-ids decide for one command.' }, + { code: 'E_USAGE', meaning: 'Bad command line or ops JSON (unknown command, option, op, key or value type).', fix: 'Read the message (it names the op index and the allowed keys), then `bpmn --help` or `bpmn guide`.' }, { code: 'E_UNKNOWN_ALIAS', meaning: 'An op of an `apply` batch uses a batch alias (`"after": "$check"`) that no op before it defines (exit 1; checked before anything runs). The message lists the aliases defined so far.', fix: 'Define it with "as": "$check" (add, connect, split, the nodes of a split branch), "flowAs" (add: the flow into the new node), "outAs" (add: the flow it creates out of the new node), "joinAs" (split) or "refAs" (add, connect, set, retype: the message / error / signal / escalation the element references) in an earlier op, or fix the spelling (the candidates are the defined aliases).' }, { code: 'E_DUPLICATE_ALIAS', meaning: 'Two ops of a batch define the same alias (exit 1).', fix: 'Give every element its own alias.' }, { code: 'E_FILE_NOT_FOUND', meaning: 'The .bpmn file does not exist.', fix: 'Check the path, or create the file with `bpmn new `.' }, @@ -124,9 +124,8 @@ const WRITE_TIME_ERRORS: ErrorDoc[] = [ const OP_WARNINGS: ErrorDoc[] = [ { code: 'W_ID_SUFFIXED', meaning: 'The id generated for a new element (from its name, its kind and place, or the ends of a flow) already existed (for an unnamed gateway or activity also with its kind spelled out, gateway_parallelAfterCheck, and with the kind of the unnamed anchor it follows, task_afterCheckGateway), so a suffix was appended (2/3 under camelCase and PascalCase, the default; _2/_3 under snake_case, -2/-3 under kebab-case; after an id that ends in a digit No2/No3, serviceTask_step1No2, so it never reads as another number; a long id loses whole words first, to stay within 64 characters): a name the file already has, or unnamed elements that only an index tells apart.', fix: 'Use the reported id, pass an explicit id (--id, --flow-id, "id", "flowId", "joinId") to choose one yourself, or give the element a distinct name.' }, { code: 'W_ID_PREFIX_STALE', meaning: 'A retype or a trigger change (`retype`, `set trigger=` and the other trigger keys) kept an id whose prefix the project\'s id conventions (element-id-naming of the .bpmnlintrc) give what the element was, or that they did not check for it (a type switched off), and do not accept for what it is now (serviceTask_checkInvoice, now a user task; timerEvent_x, now a message event). Ids are API, so the element keeps it; element-id-naming reports it from now on. Only the change that made it stale reports it (an apply batch: once per element, from its state before the batch to its final state). An element the same batch created (a split\'s join too) is nobody\'s API yet: it takes the new prefix instead, and the flows the batch named after it follow, without the warning.', fix: 'Keep the id while code, tests or links use it. To follow the convention, run the hint\'s `bpmn set id=` (the new prefix and the rest of the id; where the rest did not pass the rule, e.g. a body in another case, the id of what the element is now; free in the file after the batch).' }, - { code: 'W_FLOW_ID_STALE', meaning: 'With flow ids kept (--keep-flow-ids or BPMN_KEEP_FLOW_IDS=1; library: keepFlowIds) a flow the file had kept its id although the change gave it other ends (a splice, a bridge, a move, `set source=` / `target=`; also where an end got another id or name earlier in the batch): the id names ends the flow no longer has. Only the change that made it stale reports it, once per flow and against its ends after the whole batch; a flow the same change created follows its ends, one it renamed with `set id=` takes that id, and `bpmn validate` never lists it.', fix: 'Keep the id while code or tests use it. To rename it deliberately, together with them: `bpmn set id=` (the hint names the id its new ends give, or the --flow-id / "flowId" the splice into it was given; a second flow between the same ends gets `_2`, so every hint can be followed).' }, { code: 'W_KIND_MISMATCH', meaning: '`add --if-absent`: the id exists but with another kind; nothing was changed.', fix: 'Use `bpmn retype ` to change its kind, or pick another id.' }, - { code: 'W_OPTION_IGNORED', meaning: 'An option does not apply to the kind (trigger options on a task, --collapsed on a non-sub-process, --lane on a data object) or to the placement (--flow-id on a splice: the flow into the node is the existing one; one without id takes it) and was ignored.', fix: 'Drop the option, or use the kind it applies to. For --flow-id: the hint\'s `bpmn set id=` renames the existing flow; with flow ids kept, where W_FLOW_ID_STALE reports that flow, it names the command instead.' }, + { code: 'W_OPTION_IGNORED', meaning: 'An option does not apply to the kind (trigger options on a task, --collapsed on a non-sub-process, --lane on a data object) or to the placement (--flow-id on a splice: the flow into the node is the existing one; one without id takes it) and was ignored.', fix: 'Drop the option, or use the kind it applies to. For --flow-id: the hint\'s `bpmn set id=` renames the existing flow.' }, { code: 'W_LANE_INHERITED', meaning: 'A node added (or moved without a lane of its own, e.g. out of a sub-process) into a flow between two lanes (a cross-lane splice) without --lane got one of the two lanes: the lane of the row the layout puts it on (the target\'s after a branching source such as a gateway, else the anchor\'s); the message names both. A moved node that is in a lane keeps it.', fix: 'Keep it, or `bpmn move --lane ` (the hint names it); pass --lane when adding or moving to choose up front.' }, { code: 'W_LANES_WITHOUT_POOL', meaning: 'The process has lanes but no participant; lanes are only drawn inside a pool.', fix: 'Add a pool: `bpmn add participant ""` (it wraps the existing process).' }, { code: 'W_LAYOUT_*', meaning: 'The layout engine reported a warning while drawing (suffix = the layouter\'s code); the diagram was still written.', fix: 'Check the named element; usually a node the layouter could not place nicely (e.g. unreachable or oddly connected).' }, @@ -150,8 +149,8 @@ const LINT_FINDINGS: ErrorDoc[] = [ { code: 'W_DUPLICATE_NAME', meaning: 'Several elements of the same kind share a name.', fix: 'Rename with `bpmn set name=...` unless intended.' }, { code: 'W_IMPLICIT_SPLIT', meaning: 'A non-gateway node got a second outgoing flow (implicit parallel split).', fix: 'Route the branches through a gateway: a `split` op in `bpmn apply` (split exists only as an apply op, not as a command) or `bpmn add exclusiveGateway --after ` plus `connect`; ignore it when the implicit split is intended.' }, { code: 'W_IMPLICIT_JOIN', meaning: 'A non-gateway node got a second incoming flow (implicit join: every token runs the node again).', fix: 'Join through a gateway unless the repeated execution is intended.' }, - { code: 'W_LABEL_DROPPED', meaning: 'While bridging a removed node the outgoing flow label could not be carried over because the incoming flow has its own; or, with flow ids kept, a moved node that was not bridged (several in- or out-flows, or none) put its out-flow back at its new place, to another node, without the label, which nothing at the old place took.', fix: 'Set the wanted label on the surviving flow with `bpmn set name=...`.' }, - { code: 'W_CONDITION_DROPPED', meaning: 'A `retype` to a parallel or event-based gateway, or a `set source=` behind one, removed the condition of a flow (such a gateway takes no conditions). Or: While bridging a removed node the outgoing flow condition could not be carried over; or, with flow ids kept, a moved node that was not bridged put its out-flow back at its new place, to another node, without the condition, which nothing at the old place took, or as the default flow, which has none.', fix: 'Set the condition on the surviving flow with `bpmn set condition=...`.' }, + { code: 'W_LABEL_DROPPED', meaning: 'While bridging a removed node the outgoing flow label could not be carried over because the incoming flow has its own.', fix: 'Set the wanted label on the surviving flow with `bpmn set name=...`.' }, + { code: 'W_CONDITION_DROPPED', meaning: 'A `retype` to a parallel or event-based gateway, or a `set source=` behind one, removed the condition of a flow (such a gateway takes no conditions). Or: While bridging a removed node the outgoing flow condition could not be carried over.', fix: 'Set the condition on the surviving flow with `bpmn set condition=...`.' }, { code: 'W_PROPERTY_DROPPED', meaning: '`retype` dropped a property the new kind does not have (e.g. script on a userTask); a trigger kind change (`set trigger=`, `retype`) dropped vendor attributes / extension elements of the old event definition (changing only the details of the same trigger keeps the definition; on an event with several definitions `set trigger=` keeps that one and drops the others); `set loop=none|standard` (or standard -> multi-instance) dropped the vendor content of the old loop; a condition change dropped camunda:resource / the script language (an inline body replacing a script resource, a ${...} body that does not inherit a script language) or the inline body (a script resource replacing it); or `ext add` replaced a keyed item (same name / id) whose attributes, value or children the new one does not have.', fix: 'Re-add the information in a form the new kind supports (the hint says how, e.g. language= to keep a script language; for `ext add` the hint holds the `--xml` that keeps the old content), or accept the loss.' }, { code: 'W_PREEXISTING_ERROR', meaning: 'A structural error, or an error of a validator (a host\'s validator in library use: `[]`), that the model already had before this change (the message starts with its original code); it does not block the write. A validator\'s error follows its element through a rename. A result counts it with the other warnings the file already had (`N warnings already in the file`, JSON `warnings.preexistingCount`) instead of listing it; `MutationResult.validation.warnings` of the library keeps it.', fix: 'Fix it when convenient: `bpmn validate ` lists it with its own code and hint (a host validator\'s error: the host\'s own check or `validateXml(xml, { validators })` lists it).' }, ]; @@ -266,7 +265,7 @@ export const ERROR_CATALOGUE: ErrorDoc[] = [...USAGE_AND_IO_ERRORS, ...ID_LOOKUP export const EXIT_CODE_DOCS: Array<{ code: number; meaning: string }> = [ { code: 0, meaning: 'ok (warnings allowed)' }, - { code: EXIT_CODES.usage, meaning: 'usage error: unknown command/option, bad ops JSON or environment value (E_USAGE)' }, + { code: EXIT_CODES.usage, meaning: 'usage error: unknown command/option, bad ops JSON (E_USAGE)' }, { code: EXIT_CODES.model, meaning: 'model or validation error (E_NOT_FOUND, E_CROSS_SCOPE, ...); validate: also an error of the project\'s bpmnlint' }, { code: EXIT_CODES.layout, meaning: 'layout error (E_LAYOUT_*)' }, { code: EXIT_CODES.io, meaning: 'I/O or parse error (E_FILE_NOT_FOUND, E_PARSE, E_IMPORT_LOSSY, E_FILE_EXISTS, E_RENDER, E_LINT_CONFIG)' }, @@ -311,7 +310,7 @@ export const PLACEMENT_DOCS: PlacementDoc[] = [ { option: '--after ', key: 'after', rule: 'X is a gateway or has no outgoing flow: append X -> node. X has exactly one outgoing flow: splice, X -> node -> old successor. Otherwise E_HAS_SUCCESSOR (say which flow).' }, { option: '--before ', key: 'before', rule: 'Symmetric: prepend before a join gateway or a node without incoming flow, else splice into its single incoming flow; several incoming flows -> E_HAS_PREDECESSOR.' }, { option: '--after --before ', key: 'after + before', rule: 'Splice into the flow X -> Y (E_NO_FLOW / E_AMBIGUOUS_FLOW when there is none / several).' }, - { option: '--flow ', key: 'flow', rule: 'Splice into sequence flow F: A -> B becomes A -> node -> B. F keeps its name and condition and now ends at the node; it keeps its id too, unless the id named A and B (flow_aToB is renamed flow_aToNode; with --keep-flow-ids it stays, W_FLOW_ID_STALE, which suggests a --flow-id given for it).' }, + { option: '--flow ', key: 'flow', rule: 'Splice into sequence flow F: A -> B becomes A -> node -> B. F keeps its name and condition and now ends at the node; it keeps its id too, unless the id named A and B (flow_aToB is renamed flow_aToNode).' }, { option: '--in ', key: 'in', rule: 'Put the node into process / sub-process / participant S without connecting it (connect it afterwards).' }, { option: '--on ', key: 'on', rule: 'Boundary events only: attach to activity A.' }, { option: '--to ', key: 'to', rule: 'Additionally connect node -> T (a branch that re-joins the main path).' }, @@ -329,7 +328,7 @@ export const ID_CONVENTIONS = { ends: 'a flow end or an anchor is named by its name, else by the speaking part of its id (checkInvoice; an unnamed element by its own kind and place, gateway_afterCheckInvoice -> afterCheckInvoice), else by a word for its kind (Gateway, End, Timer); an id with a number and a word speaks (gateway_afterCheck2024 -> afterCheck2024); hashes, numbers, machine ids and StartEvent_1 are never copied', length: 'at most 64 characters, a collision suffix included; a name gives at most 40 characters of words (a single longer word is cut at 40), and an id within 64 is not cut again; a longer one loses whole words the same way every time: the two ends of a flow share the room (each gives up its last words, never its first word, its last word with a letter before its Join or number, a word between two numbers, or the numbers before a number it ends with (x_2_2 stays whole), and a number left at the end of the id goes too where it reads like a collision suffix, and so does a filler word a cut would end on; of the cuts that fit, the one with the most words, then the most characters), a join\'s Join, a number and a collision suffix are kept; a word is cut inside only when no whole word is left to give up (each end of a flow keeps its first word, or its last where the two begin alike), and then in the longer end, keeping its Join or number (in both ends where the longer would keep fewer than 8 characters); a cut keeps a letter (a room that ends inside a leading number starts at the first word with a letter); a cut (a name\'s 40 characters, a body, a flow\'s end) never ends on a filler word, und, oder, an, die, the, of, ... (lieferantenbestellungVersenden, not ...VersendenUnd), the first word stays, and so does one between two numbers (startAfter2Und2)', collisions: 'the same edit gives the same ids; an unnamed gateway or activity whose id is taken, or whose body another id has (the unnamed gateway it follows), first spells out its kind where the prefix does not say it (gateway_parallelAfterCheckInvoice), then names the kind of the unnamed anchor it follows (task_afterCheckInvoiceGateway after gateway_afterCheckInvoice); a flow whose two ends begin with the same word keeps each end\'s last word when cut; a taken id gets 2, 3 under camelCase / PascalCase (after an id that ends in a digit No2, No3: serviceTask_step1No2, never serviceTask_step12, the id of Step 12), _2 under snake_case, -2 under kebab-case (an id the suffix would take past 64 characters loses whole words first) and W_ID_SUFFIXED (a repeated name, or unnamed elements only an index tells apart)', - renames: 'a flow whose id names its ends in the current form (the id the conventions give a flow between them; an end also by its name, and in a batch by the id and name it had before a `set id=` / `name=` earlier in the batch) is renamed after its new ends when a splice, a bridge, a move or `set target=` changes them (its derived DI edge id follows; a flow without id gets one from its new ends, or the --flow-id of the add that splices into it, `id added`); a flow id in another form (Flow_AToB of bpmn-cli before 0.9, SequenceFlow_1, Flow_0k3x9qa) keeps it; with --keep-flow-ids (BPMN_KEEP_FLOW_IDS=1; library: keepFlowIds) a flow the file had keeps its id and W_FLOW_ID_STALE names it (a flow the same change created still follows its ends); every other id stays', + renames: 'a flow whose id names its ends in the current form (the id the conventions give a flow between them; an end also by its name, and in a batch by the id and name it had before a `set id=` / `name=` earlier in the batch) is renamed after its new ends when a splice, a bridge, a move or `set target=` changes them (its derived DI edge id follows; a flow without id gets one from its new ends, or the --flow-id of the add that splices into it, `id added`); a flow id in another form (Flow_AToB of bpmn-cli before 0.9, SequenceFlow_1, Flow_0k3x9qa) keeps it; every other id stays', transliteration: 'ä -> ae, ö -> oe, ü -> ue, ß -> ss; ø -> oe, å -> aa, æ -> ae, œ -> oe, ł -> l, þ -> th; other accents dropped (é -> e)', di: 'a full redraw keeps the ids of existing DI elements, planes and diagrams; new DI follows the DI ids of the file, cut like element ids to stay within 64 characters (a taken one gets _2); a DI shape or edge id derived from an element id (_di, BPMNShape_, Edge_) follows `set id=` as it follows a flow renamed after its ends; other DI ids, and plane and diagram ids, stay', }; diff --git a/src/api.ts b/src/api.ts index 4dce2762..f318df2e 100644 --- a/src/api.ts +++ b/src/api.ts @@ -58,8 +58,6 @@ export interface EditOptions { validators?: Validator[]; /** the document's file name, for the validators (`ctx.file`) */ file?: string; - /** keep existing flow ids when the ops give the flow other ends (`--keep-flow-ids`) */ - keepFlowIds?: boolean; /** new ids follow them where they declare a prefix, bpmn-cli's own prefix for the types they leave out; absent: element-id-naming's defaults */ conventions?: IdConventions; /** include the model view of the result (`--show`) */ diff --git a/src/domain/batch/EditBatch.ts b/src/domain/batch/EditBatch.ts index 23caa28f..deb6f9d5 100644 --- a/src/domain/batch/EditBatch.ts +++ b/src/domain/batch/EditBatch.ts @@ -34,7 +34,7 @@ export class EditBatch { private readonly opts: MutationOptions = {}, ) { const platformChoice = opts.platform && opts.platform !== 'auto' ? opts.platform : undefined; - this.edit = new EditSession(doc, { conventions: opts.conventions, platformChoice, keepFlowIds: opts.keepFlowIds }); + this.edit = new EditSession(doc, { conventions: opts.conventions, platformChoice }); } /** diff --git a/src/domain/batch/ModelCommands.ts b/src/domain/batch/ModelCommands.ts index 20d659c3..96d3cee9 100644 --- a/src/domain/batch/ModelCommands.ts +++ b/src/domain/batch/ModelCommands.ts @@ -19,7 +19,6 @@ import { ElementOrder } from '../editing/elements/ElementOrder.js'; import { ExtensionElements } from '../editing/extensions/ExtensionElements.js'; import { BranchSplit } from '../editing/connections/BranchSplit.js'; import { IdRename } from '../editing/ids/IdRename.js'; -import { StaleFlowReport } from '../editing/connections/StaleFlowReport.js'; /** The merged changes, the ops with aliases resolved (for layout and format) and alias -> id. */ export interface BatchRun { @@ -55,7 +54,6 @@ export class ModelCommands { runCommand(command: EditCommand): ChangeSet { this.edit.begin(); const cs = this.step(command); - new StaleFlowReport(this.edit).report(cs, this.edit.flowsBefore); new IdRename(this.edit).reportStalePrefixes(cs); return cs; } @@ -185,7 +183,6 @@ export class ModelCommands { all.bindings = []; all.replaced = []; all.warnings = all.warnings.filter((warning) => this.stillHolds(warning)); - new StaleFlowReport(this.edit).report(all, this.edit.flowsBefore); new IdRename(this.edit).reportStalePrefixes(all); return { changes: all, commands: batch.resolved, aliases: batch.table.toRecord() }; } diff --git a/src/domain/batch/types.ts b/src/domain/batch/types.ts index 18dbf5f7..6578dbea 100644 --- a/src/domain/batch/types.ts +++ b/src/domain/batch/types.ts @@ -21,11 +21,6 @@ export interface MutationOptions extends DrawingOptions { validators?: Validator[]; /** the file being written, for the validators; default doc.file */ file?: string; - /** - * true keeps a flow's id when an op gives it other ends (where flow ids are API, e.g. - * generated code); false renames a flow whose id names its old ends - */ - keepFlowIds?: boolean; /** declared id conventions, followed by generated ids where they declare a prefix */ conventions?: IdConventions; } diff --git a/src/domain/document/Doc.ts b/src/domain/document/Doc.ts index cda44ec5..8b5bbfb3 100644 --- a/src/domain/document/Doc.ts +++ b/src/domain/document/Doc.ts @@ -59,8 +59,8 @@ export class Doc { private declared: IdConventions | undefined; readonly layoutRequests = new LayoutRequests(); /** - * Whether a flow whose id names its ends is renamed when an op changes them. - * False keeps the ids of the flows the document had before the batch (reported W_FLOW_ID_STALE). + * Whether a flow whose id names its ends is renamed when an op changes them. Internal: + * BatchDrawing turns it off so bridged flows keep the ids their drawn edges are keyed by. */ followFlowEnds = true; diff --git a/src/domain/document/EditSession.ts b/src/domain/document/EditSession.ts index 939c3c1d..f7d758f2 100644 --- a/src/domain/document/EditSession.ts +++ b/src/domain/document/EditSession.ts @@ -16,18 +16,6 @@ export interface BatchContext { aliases: () => Record; } -export type FlowEnds = readonly [El | undefined, El | undefined]; - -/** - * `was`: the ends `A -> B` the kept id named; `preferred`: the id an op wanted while the flow - * ends at `target` (`add --flow-id` on a splice), which W_FLOW_ID_STALE suggests. - */ -export interface KeptFlow { - was: string; - id: string; - preferred?: { id: string; target: El }; -} - /** What a node was called as a flow end before `set` gave it another id or name. */ export interface FormerEnd { id: string; @@ -52,18 +40,15 @@ export interface EditSettings { conventions?: IdConventions; /** so the ops follow the same engine as the profile that reports on them */ platformChoice?: Platform; - keepFlowIds?: boolean; } /** * The state of one edit of a Doc: the call's settings and what the running batch (or a single - * op) recorded so far (ids given out, renames, kept flows, former flow ends, dropped content). + * op) recorded so far (ids given out, renames, former flow ends, dropped content). */ export class EditSession { batch: BatchContext | undefined; - private flows: ReadonlyMap | undefined; private renames: Rename[] = []; - private keptFlows = new Map(); private readonly formerEnds = new Map(); private idStates = new Map(); private readonly dropped: DroppedContent[] = []; @@ -96,10 +81,6 @@ export class EditSession { return this.doc.requireScope(id, this.batch); } - get followFlowEnds(): boolean { - return this.settings.keepFlowIds === undefined ? this.doc.followFlowEnds : !this.settings.keepFlowIds; - } - /** The document's, or one for conventions the call brings itself. */ get idStyle(): IdStyle { const conventions = this.settings.conventions; @@ -108,16 +89,9 @@ export class EditSession { } begin(): void { - this.keptFlows.clear(); this.idStates.clear(); this.allocator.begin(); this.formerEnds.clear(); - this.flows = this.followFlowEnds ? undefined : new Map(this.doc.allSequenceFlows().map((flow) => [flow, [flow.get('sourceRef'), flow.get('targetRef')]] as const)); - } - - /** With flow ids kept: the flows at begin(), the only ones keeping their id, with their ends. */ - get flowsBefore(): ReadonlyMap | undefined { - return this.flows; } /** `was`: what the old id named (a flow's old ends); `el` limits the rewrite to its entries. */ @@ -136,31 +110,6 @@ export class EditSession { return out; } - /** With flow ids kept, flows the document had before the batch keep their id. */ - keepsFlowId(flow: El): boolean { - return !this.followFlowEnds && !!flow.get('id') && (this.flows?.has(flow) ?? true); - } - - /** The first ends stay while the flow has the id it kept; a flow renamed since keeps a new id. */ - recordKeptFlow(flow: El, was: string): void { - const id = flow.get('id'); - if (this.keptFlows.get(flow)?.id !== id) this.keptFlows.set(flow, { was, id }); - } - - /** So W_FLOW_ID_STALE suggests the id the op wanted; false for a flow that was not kept. */ - preferFlowId(flow: El, id: string, target: El): boolean { - const kept = this.keptFlows.get(flow); - if (!kept || kept.id !== flow.get('id')) return false; - kept.preferred = { id, target }; - return true; - } - - takeKeptFlows(): Map { - const out = this.keptFlows; - this.keptFlows = new Map(); - return out; - } - /** A flow id naming the node by its former id or name still names its ends this batch. */ recordFormerEnd(node: El, end: FormerEnd): void { const list = this.formerEnds.get(node) ?? []; diff --git a/src/domain/editing/connections/FlowBridge.ts b/src/domain/editing/connections/FlowBridge.ts index f6fbe62f..3f1ca6ea 100644 --- a/src/domain/editing/connections/FlowBridge.ts +++ b/src/domain/editing/connections/FlowBridge.ts @@ -7,13 +7,6 @@ import { renamedNote } from '../../document/utils/renames.js'; import { EventGatewayRule } from '../events/EventGatewayRule.js'; import { SequenceFlows } from './SequenceFlows.js'; -/** The in-flow now running to the successor, the removed out-flow and what it took over. */ -export interface BridgedFlow { - flow: El; - from: El; - took: { name?: string; condition?: El; associations: El[] }; -} - /** Hints differ per command: `move` has no --no-bridge. */ export type BridgeCommand = 'remove' | 'move'; @@ -39,7 +32,7 @@ export class FlowBridge { } else this.single(node, bridge, removing); } - single(node: El, bridge: boolean, removing?: ReadonlySet): BridgedFlow | undefined { + single(node: El, bridge: boolean, removing?: ReadonlySet): void { const doc = this.edit.doc; const incoming = doc.incoming(node); const outgoing = doc.outgoing(node); @@ -51,10 +44,10 @@ export class FlowBridge { if (predecessor === successor && !FlowBridge.canLoopToItself(predecessor)) { this.detach(node, false); if (predecessor !== node) this.changes.note(`not bridged: ${idOf(predecessor)} is both predecessor and successor of ${idOf(node)} and cannot loop back to itself`); - return undefined; + return; } this.assertAllowed(node, inFlow, outFlow, removing); - return this.detach(node, true); + this.detach(node, true); } /** Exception paths of the group's boundary events are not exits: the move drops them. */ @@ -89,7 +82,7 @@ export class FlowBridge { } /** Detaches the flows of the node; a self-loop, both incoming and outgoing, is removed and reported once. */ - private detach(node: El, bridge: boolean): BridgedFlow | undefined { + private detach(node: El, bridge: boolean): void { const { changes } = this; const doc = this.edit.doc; const incoming = doc.incoming(node); @@ -97,27 +90,27 @@ export class FlowBridge { if (bridge && incoming.length === 1 && outgoing.length === 1) { const inFlow = incoming[0]!; const outFlow = outgoing[0]!; - if (outFlow.get('targetRef') !== node && inFlow.get('sourceRef') !== node) return this.carry(inFlow, outFlow, idOf(node), 'node'); + if (outFlow.get('targetRef') !== node && inFlow.get('sourceRef') !== node) { + this.carry(inFlow, outFlow, idOf(node), 'node'); + return; + } } for (const flow of new Set([...incoming, ...outgoing])) { changes.remove(ChangeSet.flowEntryOf(flow)); new SequenceFlows(this.edit).remove(flow, changes); } - return undefined; } /** Carries the in-flow on to the out-flow's target; under the group policy the out-flow's id stays claimed, so a flow the move creates afterwards does not reuse it. */ - private carry(inFlow: El, outFlow: El, via: string, policy: CarryPolicy): BridgedFlow { + private carry(inFlow: El, outFlow: El, via: string, policy: CarryPolicy): void { const { changes } = this; const doc = this.edit.doc; const predecessor = inFlow.get('sourceRef'); const successor = outFlow.get('targetRef'); const outName = outFlow.get('name'); const outCond = outFlow.get('conditionExpression'); - const took: BridgedFlow['took'] = { associations: [] }; if (outName && !inFlow.get('name')) { inFlow.set('name', outName); - took.name = outName; } else if (outName) changes.warn({ code: 'W_LABEL_DROPPED', message: `Flow label "${outName}" of ${idOf(outFlow)} was dropped while bridging ${via}`, element: idOf(outFlow) }); const ontoDefault = predecessor.get('default') === inFlow; if (outCond && !inFlow.get('conditionExpression') && ontoDefault && policy === 'node') { @@ -131,16 +124,14 @@ export class FlowBridge { } else if (outCond && !inFlow.get('conditionExpression') && !(ontoDefault && policy === 'group')) { inFlow.set('conditionExpression', outCond); if (policy === 'group') outCond.$parent = inFlow; - else took.condition = outCond; } else if (outCond) changes.warn({ code: 'W_CONDITION_DROPPED', message: `Condition of ${idOf(outFlow)} was dropped while bridging ${via}`, element: idOf(outFlow) }); - took.associations = new SequenceFlows(this.edit).carryAssociations(outFlow, inFlow, changes); + new SequenceFlows(this.edit).carryAssociations(outFlow, inFlow, changes); new SequenceFlows(this.edit).remove(outFlow, changes); changes.remove(ChangeSet.flowEntryOf(outFlow)); if (policy === 'group' && idOf(outFlow) && !doc.has(idOf(outFlow))) doc.ids.claim(idOf(outFlow)); const renamed = new SequenceFlows(this.edit).redirect(inFlow, { target: successor }); changes.change(ChangeSet.flowEntryOf(inFlow, `${idOf(predecessor)} -> ${idOf(successor)} (bridged ${via})${renamedNote(renamed)}`)); changes.note(`bridged: ${idOf(predecessor)} -> ${idOf(successor)}`); - return { flow: inFlow, from: outFlow, took }; } private join(node: El, removing: ReadonlySet): void { diff --git a/src/domain/editing/connections/FlowIdFollower.ts b/src/domain/editing/connections/FlowIdFollower.ts index bd183526..3d9dc780 100644 --- a/src/domain/editing/connections/FlowIdFollower.ts +++ b/src/domain/editing/connections/FlowIdFollower.ts @@ -8,15 +8,14 @@ import { is, many, type El, idOf } from '../../document/utils/moddle.js'; /** * Keeps speaking flow ids (flow_checkInvoiceToDone) true: a flow named after its ends is - * renamed after its new ends when they change. Other ids stay; with `--keep-flow-ids` - * the document's flows keep theirs and get reported as stale. + * renamed after its new ends when they change. Other ids stay. */ export class FlowIdFollower { constructor(private readonly edit: EditSession) {} /** * Call before `set` renames `node`: a flow still named after its old id or name - * follows (or is reported) when a later op of the batch changes its ends. + * follows when a later op of the batch changes its ends. */ recordFormerEnd(node: El): void { if (!is(node, 'bpmn:FlowNode') || !(many(node, 'incoming').length || many(node, 'outgoing').length)) return; @@ -36,10 +35,7 @@ export class FlowIdFollower { ); } - /** - * Returns the old id when it renamed the flow, `''` when it gave an id-less flow one. - * With ids kept, a flow the batch created is nobody's API yet and still follows. - */ + /** Returns the old id when it renamed the flow, `''` when it gave an id-less flow one. */ follow(flow: El, oldSource: El | undefined, oldTarget: El | undefined, wanted?: string): string | undefined { const doc = this.edit.doc; if (!is(flow, 'bpmn:SequenceFlow') || !oldSource || !oldTarget) return undefined; @@ -52,12 +48,8 @@ export class FlowIdFollower { doc.invalidate(); return ''; } - if (!this.namesEnds(old, oldSource, oldTarget)) return undefined; + if (!doc.followFlowEnds || !this.namesEnds(old, oldSource, oldTarget)) return undefined; const oldIds = [idOf(oldSource), idOf(oldTarget)] as const; - if (this.edit.keepsFlowId(flow)) { - this.edit.recordKeptFlow(flow, `${oldIds[0]} -> ${oldIds[1]}`); - return undefined; - } doc.ids.release(old); const id = this.edit.allocator.allocate(ElementLabels.flowRequest('bpmn:SequenceFlow', source, target)).id; if (id === old) return undefined; diff --git a/src/domain/editing/connections/MovedFlows.ts b/src/domain/editing/connections/MovedFlows.ts deleted file mode 100644 index ce02c86e..00000000 --- a/src/domain/editing/connections/MovedFlows.ts +++ /dev/null @@ -1,238 +0,0 @@ -import type { EditSession } from '../../document/EditSession.js'; -import type { Finding } from '../../document/Finding.js'; -import { is, type El } from '../../document/utils/moddle.js'; -import { ChangeSet } from '../../document/ChangeSet.js'; -import { idOf } from '../../document/utils/elements.js'; -import type { BridgedFlow } from './FlowBridge.js'; - -/** - * A flow the detach took out (flow ids kept). `label`: what no bridge took; given back when the - * flow returns to `target`, else dropped with a warning. - */ -interface SpareFlow { - flow: El; - target: El; - label?: FlowLabel; -} - -interface FlowLabel { - name: string | undefined; - condition: El | undefined; - isDefault: boolean; -} - -/** A dropped label and its warnings: a later node of the chain may put the flow back after all. */ -interface DroppedLabel { - node: El; - flow: El; - target: El; - label: FlowLabel; - warnings: Finding[]; -} - -interface BridgeMade { - node: El; - target: El; - bridge: BridgedFlow; -} - -interface LabelBefore { - name: string | undefined; - condition: El | undefined; - associations: El[]; -} - -/** - * The flows a `move` takes out at the old place and puts back at the new one (flow ids kept), and - * what becomes of their names, conditions and default markers: given back, kept by a bridge, or - * dropped with a warning. - */ -export class MovedFlows { - private readonly spare = new Map(); - private readonly dropped: DroppedLabel[] = []; - private readonly bridges: BridgeMade[] = []; - private readonly before: Map | undefined; - - constructor( - private readonly edit: EditSession, - private readonly changes: ChangeSet, - nodes: El[], - ) { - this.before = edit.followFlowEnds ? undefined : MovedFlows.labelsBefore(edit, nodes); - } - - /** The flow taken out at `source`, to put back where the placement connects it. */ - spareOf(source: El): El | undefined { - return this.spare.get(source)?.flow; - } - - hasSpare(source: El): boolean { - return this.spare.has(source); - } - - /** `label`: what no bridge took; given back when the flow returns to `target`. */ - takeOut(source: El, flow: El, target: El, label?: FlowLabel): void { - this.spare.set(source, { flow, target, ...(label ? { label } : {}) }); - } - - bridged(node: El, bridge: BridgedFlow): void { - this.bridges.push({ node, target: bridge.flow.get('targetRef'), bridge }); - } - - /** After the last node of the chain moved. */ - finish(): void { - this.giveBackLate(); - if (this.before) this.reportBridgesKept(this.before); - } - - static labelOf(flow: El, source: El): FlowLabel { - return { name: flow.get('name'), condition: flow.get('conditionExpression'), isDefault: source.get('default') === flow }; - } - - - /** A put-back flow is reported as one changed entry, not as removed plus created. */ - putBack(): void { - const changes = this.changes; - const spare = this.spare; - for (const [node, { flow, target, label }] of spare) { - if (this.edit.doc.get(idOf(flow)) !== flow) continue; - spare.delete(node); - const id = idOf(flow); - const back = flow.get('targetRef') === target; - const lost = label && back ? MovedFlows.giveBack(node, flow, label, changes) : ''; - changes.removed = changes.removed.filter((change) => change.id !== id); - changes.created = changes.created.filter((change) => change.id !== id); - changes.change(ChangeSet.flowEntryOf(flow, `${ChangeSet.flowEntryOf(flow).detail}${back ? lost : ` (was -> ${idOf(target)})`}`)); - if (!label || back) continue; - const why = `while moving ${idOf(node)}: ${idOf(node)} was not bridged, so nothing at its old place took it, and ${id} was put back at its new place without it`; - const warnings: Finding[] = []; - if (label.name) warnings.push({ code: 'W_LABEL_DROPPED', message: `Flow label "${label.name}" of ${id} was dropped ${why}`, element: id }); - if (label.condition) warnings.push({ code: 'W_CONDITION_DROPPED', message: `Condition of ${id} was dropped ${why}`, element: id }); - for (const warning of warnings) changes.warn(warning); - this.dropped.push({ node, flow, target, label, warnings }); - } - } - - /** A default flow has no condition, so a flow the placement made the default keeps none. */ - private static giveBack(node: El, flow: El, label: FlowLabel, changes: ChangeSet): string { - const id = idOf(flow); - if (label.name && !flow.get('name')) flow.set('name', label.name); - if (label.condition && !flow.get('conditionExpression')) { - if (node.get('default') === flow) { - changes.warn({ - code: 'W_CONDITION_DROPPED', - message: `Condition of ${id} was dropped while moving ${idOf(node)}: ${id} is now the default flow of ${idOf(node)}`, - element: id, - hint: `A default flow has no condition; use \`bpmn set ${id} condition=...\` if the condition should replace the default marker.`, - }); - } else { - flow.set('conditionExpression', label.condition); - label.condition.$parent = flow; - } - } - if (!label.isDefault || node.get('default') === flow) return ''; - if (!node.get('default') && !flow.get('conditionExpression')) { - node.set('default', flow); - return ''; - } - return ` (no longer the default flow of ${idOf(node)})`; - } - - /** A flow a later node of the chain took back between its old ends gets its label back. */ - private giveBackLate(): void { - const changes = this.changes; - for (const { node, flow, target, label, warnings } of this.dropped) { - if (this.edit.doc.get(idOf(flow)) !== flow || flow.get('sourceRef') !== node || flow.get('targetRef') !== target) continue; - changes.warnings = changes.warnings.filter((warning) => !warnings.includes(warning)); - const lost = MovedFlows.giveBack(node, flow, label, changes); - const change = ChangeSet.flowEntryOf(flow); - changes.changed = changes.changed.filter((entry) => entry.id !== change.id); - changes.change(ChangeSet.flowEntryOf(flow, `${change.detail}${lost}`)); - } - } - - private static labelsBefore(edit: EditSession, nodes: El[]): Map { - const doc = edit.doc; - const out = new Map(); - for (const movedNode of nodes) { - for (const flow of doc.outgoing(movedNode)) if (edit.keepsFlowId(flow)) out.set(flow, { name: flow.get('name'), condition: flow.get('conditionExpression'), associations: [] }); - } - if (!out.size) return out; - for (const element of doc.byId().values()) { - if (!is(element, 'bpmn:Association')) continue; - for (const end of ['sourceRef', 'targetRef']) out.get(element.get(end))?.associations.push(element); - } - return out; - } - - /** - * Tells on a put-back flow's change entry which bridge kept its label: - * `(was -> C; its name stays with the bridge flow_aToB)`. - */ - private reportBridgesKept(before: Map): void { - const changes = this.changes; - const made = this.bridges; - const alive = (el: El): boolean => this.edit.doc.get(idOf(el)) === el; - made.forEach(({ node, target, bridge }, index) => { - const flow = bridge.from; - const had = before.get(flow); - if (!had || !alive(flow)) return; - const holder = (took: (bridged: BridgedFlow) => boolean): El | undefined => { - let at = bridge.flow; - for (const later of made.slice(index + 1)) { - if (later.bridge.from !== at) continue; - if (!took(later.bridge)) return undefined; - at = later.bridge.flow; - } - return at; - }; - const kept = new Map(); - const keep = (at: El, what: string): void => { - kept.set(at, [...(kept.get(at) ?? []), what]); - }; - const lost: Array<{ what: 'name' | 'condition'; at: El }> = []; - const { name, condition, associations } = bridge.took; - if (name !== undefined && name === had.name && flow.get('name') !== name) { - const at = holder((bridged) => bridged.took.name === name); - if (at && !alive(at)) lost.push({ what: 'name', at }); - else if (at?.get('name') === name) keep(at, 'name'); - } - if (condition && condition === had.condition && flow.get('conditionExpression') !== condition) { - const at = holder((bridged) => bridged.took.condition === condition); - if (at && !alive(at)) lost.push({ what: 'condition', at }); - else if (at?.get('conditionExpression') === condition) keep(at, 'condition'); - } - for (const association of associations) { - if (!had.associations.includes(association) || !alive(association)) continue; - const at = holder((bridged) => bridged.took.associations.includes(association)); - if (at && alive(at) && (association.get('sourceRef') === at || association.get('targetRef') === at)) keep(at, 'annotation'); - } - const id = idOf(flow); - const back = flow.get('sourceRef') === node && flow.get('targetRef') === target; - const returned = lost.filter(({ what }) => back && !flow.get(what === 'name' ? 'name' : 'conditionExpression')); - if (returned.length) { - MovedFlows.giveBack(node, flow, { name: returned.some((loss) => loss.what === 'name') ? name : undefined, condition: returned.some((loss) => loss.what === 'condition') ? condition : undefined, isDefault: false }, changes); - changes.changed = changes.changed.filter((change) => change.id !== id); - changes.change(ChangeSet.flowEntryOf(flow)); - } - for (const { what, at } of lost.filter((loss) => !returned.includes(loss))) { - const why = `while moving ${idOf(node)}: the bridge ${idOf(at)} took it, and a later node of the move removed ${idOf(at)}`; - if (what === 'name') changes.warn({ code: 'W_LABEL_DROPPED', message: `Flow label "${name}" of ${id} was dropped ${why}`, element: id, related: [idOf(at)] }); - else changes.warn({ code: 'W_CONDITION_DROPPED', message: `Condition of ${id} was dropped ${why}`, element: id, related: [idOf(at)] }); - } - if (!kept.size) return; - const clause = [...kept].map(([at, what]) => { - const notes = what.filter((kind) => kind === 'annotation').length; - const items = [...what.filter((kind) => kind !== 'annotation'), ...(notes ? [notes > 1 ? 'annotations' : 'annotation'] : [])]; - const list = items.length > 1 ? `${items.slice(0, -1).join(', ')} and ${items[items.length - 1]}` : items[0]!; - return `its ${list} ${items.length > 1 || notes > 1 ? 'stay' : 'stays'} with the bridge ${idOf(at)}`; - }).join('; '); - const last = changes.changed.map((change) => change.id).lastIndexOf(id); - const entry = last === -1 ? ChangeSet.flowEntryOf(flow) : changes.changed[last]!; - const detail = entry.detail ?? ''; - const said = /\(was -> [^()]*\)$/.test(detail) ? `${detail.slice(0, -1)}; ${clause})` : `${detail}${detail ? ' ' : ''}(${clause})`; - if (last === -1) changes.change(ChangeSet.about({ ...entry, detail: said }, flow)); - else changes.changed[last] = ChangeSet.about({ ...entry, detail: said }, flow); - }); - } -} diff --git a/src/domain/editing/connections/SequenceFlows.ts b/src/domain/editing/connections/SequenceFlows.ts index 9d2867db..60894544 100644 --- a/src/domain/editing/connections/SequenceFlows.ts +++ b/src/domain/editing/connections/SequenceFlows.ts @@ -14,8 +14,6 @@ export interface FlowAttrs { condition?: string; language?: string; isDefault?: boolean; - /** A removed flow put back instead of a new one when no `id` is given; it keeps its id. */ - reuse?: El; } /** @@ -33,33 +31,19 @@ export class SequenceFlows { rules.assertCondition(attrs.condition); rules.assertConditionTaken(source, attrs.condition, attrs.id); const scope = doc.scopeOf(source)!; - const reuse = attrs.id ? undefined : attrs.reuse; - const oldSource = reuse?.get('sourceRef'); - const oldTarget = reuse?.get('targetRef'); - let flow: El; - if (reuse) { - flow = reuse; - if (idOf(flow)) doc.ids.claim(idOf(flow)); - flow.set('name', attrs.name || undefined); - flow.set('conditionExpression', attrs.condition ? this.conditionExpression(attrs.condition, attrs.language) : undefined); - flow.set('sourceRef', source); - flow.set('targetRef', target); - } else { - const id = this.edit.allocator.explicitOr(attrs.id, ElementLabels.flowRequest('bpmn:SequenceFlow', source, target)); - flow = doc.create('bpmn:SequenceFlow', { - id, - ...(attrs.name ? { name: attrs.name } : {}), - sourceRef: source, - targetRef: target, - ...(attrs.condition ? { conditionExpression: this.conditionExpression(attrs.condition, attrs.language) } : {}), - }); - } + const id = this.edit.allocator.explicitOr(attrs.id, ElementLabels.flowRequest('bpmn:SequenceFlow', source, target)); + const flow = doc.create('bpmn:SequenceFlow', { + id, + ...(attrs.name ? { name: attrs.name } : {}), + sourceRef: source, + targetRef: target, + ...(attrs.condition ? { conditionExpression: this.conditionExpression(attrs.condition, attrs.language) } : {}), + }); addTo(source, 'outgoing', flow); addTo(target, 'incoming', flow); insertAfterInScope(scope, declareAfter ?? lastFlowOf(scope, source) ?? source, flow); if (attrs.isDefault) this.setDefault(flow, true); doc.invalidate(); - if (reuse) new FlowIdFollower(this.edit).follow(flow, oldSource, oldTarget); return flow; } @@ -107,9 +91,9 @@ export class SequenceFlows { /** * A -> B becomes A -> node -> B: the original flow keeps its attributes and ends at - * `node`; a new plain flow (or `reuse`) runs node -> B. + * `node`; a new plain flow runs node -> B. */ - splice(flow: El, node: El, reuse?: El, flowId?: string): { first: El; second: El; renamedFrom?: string } { + splice(flow: El, node: El, flowId?: string): { first: El; second: El; renamedFrom?: string } { const doc = this.edit.doc; const target = flow.get('targetRef'); const scope = doc.scopeOf(flow)!; @@ -122,7 +106,7 @@ export class SequenceFlows { flow.set('targetRef', node); addTo(node, 'incoming', flow); const renamedFrom = new FlowIdFollower(this.edit).follow(flow, source, target, flowId); - const second = this.create(node, target, reuse ? { reuse } : {}, node); + const second = this.create(node, target, {}, node); doc.invalidate(); return { first: flow, second, ...(renamedFrom !== undefined ? { renamedFrom } : {}) }; } diff --git a/src/domain/editing/connections/StaleFlowReport.ts b/src/domain/editing/connections/StaleFlowReport.ts deleted file mode 100644 index cb6fe037..00000000 --- a/src/domain/editing/connections/StaleFlowReport.ts +++ /dev/null @@ -1,73 +0,0 @@ -import type { EditSession, FlowEnds, KeptFlow } from '../../document/EditSession.js'; -import { ElementLabels } from '../../document/ids/ElementLabels.js'; -import { isValidId } from '../../document/utils/slug.js'; -import { type El, idOf } from '../../document/utils/moddle.js'; -import type { ChangeSet } from '../../document/ChangeSet.js'; -import type { Finding } from '../../document/Finding.js'; -import { IdRegistry } from '../../document/ids/IdRegistry.js'; -import { FlowIdFollower } from './FlowIdFollower.js'; - -/** - * Warns W_FLOW_ID_STALE, once per batch and against its final state, for each kept flow - * id that no longer names its ends. - */ -export class StaleFlowReport { - constructor(private readonly edit: EditSession) {} - - /** - * `deferred`: the flow kept its id, so its W_FLOW_ID_STALE names the rename command; - * when no such warning comes, report puts the command back into this hint. - */ - static flowIdIgnoredHint(flowId: string, entry: string | undefined, deferred = false): string { - const rename = !entry ? '' : deferred ? `; W_FLOW_ID_STALE names the command that renames ${entry}` : `; rename the existing one with \`bpmn set ${entry} id=${flowId}\``; - return `--flow-id only names a flow that add creates (after a gateway / unconnected node, before a join / unconnected node)${rename}.`; - } - - /** - * Each suggested id is reserved for the later hints, so following every hint never - * collides. `before`: the flows' ends when the batch began. - */ - report(changes: ChangeSet, before?: ReadonlyMap): void { - const kept = this.edit.takeKeptFlows(); - if (!kept.size) return; - const hinted = new IdRegistry(this.edit.doc.ids.values()); - for (const [flow, was] of kept) { - const warning = this.warningFor(flow, was, hinted, before?.get(flow)); - if (warning) changes.warn(warning); - else if (was.preferred) this.restoreFlowIdHints(changes, flow, was.id); - } - } - - /** - * No warning when the flow is gone, renamed, names its ends again or was stale before - * the batch. Renaming stays the human's call, together with the code that uses the id. - */ - private warningFor(flow: El, { was, id: keptId, preferred }: KeptFlow, hinted: IdRegistry, began?: FlowEnds): Finding | undefined { - const doc = this.edit.doc; - const id = idOf(flow); - const source = flow.get('sourceRef'); - const target = flow.get('targetRef'); - if (id !== keptId || doc.get(id) !== flow || !source || !target || new FlowIdFollower(this.edit).namesEnds(id, source, target)) return undefined; - if (began?.[0] === source && began[1] === target) return undefined; - const wanted = preferred?.target === target && isValidId(preferred.id) && !hinted.has(preferred.id) && !doc.has(preferred.id) ? preferred.id : undefined; - const suggested = wanted ?? this.edit.idStyle.next(ElementLabels.flowRequest('bpmn:SequenceFlow', source, target), hinted).id; - hinted.claim(suggested); - return { - code: 'W_FLOW_ID_STALE', - message: `Flow ${id} kept its id, which names its old ends ${was}; it now runs ${idOf(source)} -> ${idOf(target)}`, - element: id, - related: [idOf(source), idOf(target)], - hint: `Code and tests that use the id keep working; to rename it together with them: \`bpmn set ${id} id=${suggested}\`.`, - }; - } - - /** The W_OPTION_IGNORED hints that deferred to a W_FLOW_ID_STALE that never came. */ - private restoreFlowIdHints(changes: ChangeSet, flow: El, id: string): void { - changes.warnings = changes.warnings.map((warning) => { - const flowId = warning.code === 'W_OPTION_IGNORED' ? /^--flow-id (\S+) ignored/.exec(warning.message)?.[1] : undefined; - if (!flowId || warning.hint !== StaleFlowReport.flowIdIgnoredHint(flowId, id, true)) return warning; - const still = this.edit.doc.get(id) === flow && !this.edit.doc.has(flowId); - return { ...warning, hint: StaleFlowReport.flowIdIgnoredHint(flowId, still ? id : undefined) }; - }); - } -} diff --git a/src/domain/editing/elements/ElementCreation.ts b/src/domain/editing/elements/ElementCreation.ts index 2d037ce0..41debaf9 100644 --- a/src/domain/editing/elements/ElementCreation.ts +++ b/src/domain/editing/elements/ElementCreation.ts @@ -16,7 +16,6 @@ import { LaneInheritance } from './LaneInheritance.js'; import { Camunda8Defaults } from '../../document/vendor/Camunda8Defaults.js'; import { FlowOptions } from '../connections/FlowOptions.js'; import type { AddElement, TriggerOptions } from '../../commands/types.js'; -import { StaleFlowReport } from '../connections/StaleFlowReport.js'; import { NodePlacement } from './NodePlacement.js'; import { EventGatewayRule } from '../events/EventGatewayRule.js'; import { FlowRules } from '../../document/FlowRules.js'; @@ -171,13 +170,12 @@ export class ElementCreation { const named = (change: Change): boolean => change.kind === 'sequenceFlow' && change.id === command.flowId; if (command.flowId && !changes.created.some(named) && !changes.changed.some((change) => named(change) && !!change.detail?.endsWith(renamedNote('')))) { const entry = entryFlow ?? this.edit.doc.incoming(node)[0]; - const kept = !!entry && this.edit.preferFlowId(entry, command.flowId, node); changes.warn({ code: 'W_OPTION_IGNORED', message: `--flow-id ${command.flowId} ignored: the flow into ${id} is the existing ${entry ? idOf(entry) : 'flow'}, which keeps its id`, element: id, ...(entry ? { related: [idOf(entry)] } : {}), - hint: StaleFlowReport.flowIdIgnoredHint(command.flowId, entry && idOf(entry), kept), + hint: `--flow-id only names a flow that add creates (after a gateway / unconnected node, before a join / unconnected node)${entry ? `; rename the existing one with \`bpmn set ${idOf(entry)} id=${command.flowId}\`` : ''}.`, }); } } diff --git a/src/domain/editing/elements/ElementMove.ts b/src/domain/editing/elements/ElementMove.ts index bd039f5f..a8332c3f 100644 --- a/src/domain/editing/elements/ElementMove.ts +++ b/src/domain/editing/elements/ElementMove.ts @@ -6,7 +6,6 @@ import { ChangeSet } from '../../document/ChangeSet.js'; import { Lanes } from './Lanes.js'; import { laneOf } from '../../document/utils/lanes.js'; import { FlowBridge } from '../connections/FlowBridge.js'; -import { MovedFlows } from '../connections/MovedFlows.js'; import { insertAfterInScope } from '../utils/declaration-order.js'; import { LaneInheritance } from './LaneInheritance.js'; import { IdRename } from '../ids/IdRename.js'; @@ -19,8 +18,7 @@ import { SequenceFlows } from '../connections/SequenceFlows.js'; /** * `move`: relocates nodes via --after/--before/--flow (chained in order), --in, --on or --lane. - * The old place is bridged; with flow ids kept, a moved node's out-flow is put back at the - * new place, keeping its id. A move that leaves every node where it is changes nothing. + * The old place is bridged. A move that leaves every node where it is changes nothing. */ export class ElementMove { constructor(private readonly edit: EditSession) {} @@ -46,12 +44,10 @@ export class ElementMove { changes.note(`${nodes.map(idOf).join(', ')} ${nodes.length > 1 ? 'are' : 'is'} already ${ElementMove.describePlacement(placement)}${nodes.length > 1 ? ', in this order' : ''}`); this.optionsOnInFlow(this.edit.doc.incoming(nodes[0]!)[0], placement, changes); } else { - const moved = new MovedFlows(this.edit, changes, nodes); nodes.forEach((node, index) => { const prev = nodes[index - 1]; - this.moveNode(node, prev ? { after: idOf(prev) } : placement, changes, hasLane, moved, { prev, next: nodes[index + 1] }); + this.moveNode(node, prev ? { after: idOf(prev) } : placement, changes, hasLane); }); - moved.finish(); } } if (hasLane) { @@ -274,13 +270,10 @@ export class ElementMove { } /** - * Moves a node to its placement. For `move a b` the flow a -> b is kept apart: when b is not - * bridged, the append after a puts it back as it was. The node's own and inherited lane are - * decided before the placement, as a splice may rename the flow; a gateway's out-flow waits for - * the append of the next node (a new branch). + * Moves a node to its placement. The node's own and inherited lane are decided before the + * placement, as a splice may rename the flow. */ - private moveNode(node: El, placement: PlacementOptions, changes: ChangeSet, explicitLane: boolean, moved: MovedFlows, chain: { prev?: El; next?: El } = {}): void { - const { prev, next } = chain; + private moveNode(node: El, placement: PlacementOptions, changes: ChangeSet, explicitLane: boolean): void { ElementMove.assertNotBoundary(node); const anchors = this.anchorsOf(placement); for (const anchor of anchors) ElementMove.assertNotInside(node, anchor); @@ -290,24 +283,14 @@ export class ElementMove { const incoming = this.edit.doc.incoming(node); const outgoing = this.edit.doc.outgoing(node); const flowIds = [...incoming, ...outgoing].map(idOf); - const oldOut = outgoing.length === 1 && outgoing[0]!.get('targetRef') !== node && this.edit.keepsFlowId(outgoing[0]!) ? outgoing[0] : undefined; - const oldTarget = oldOut?.get('targetRef'); - const outLabel = oldOut && MovedFlows.labelOf(oldOut, node); - const fromPrev = prev && incoming.find((flow) => flow.get('sourceRef') === prev && this.edit.keepsFlowId(flow)); - const prevLabel = fromPrev && MovedFlows.labelOf(fromPrev, prev!); - const bridge = new FlowBridge(this.edit, changes, 'move').single(node, true); - if (bridge) moved.bridged(node, bridge); + new FlowBridge(this.edit, changes, 'move').single(node, true); this.reserveIds(flowIds); - if (oldOut && oldTarget && !this.edit.doc.has(idOf(oldOut))) moved.takeOut(node, oldOut, oldTarget, bridge ? undefined : outLabel); - if (fromPrev && !this.edit.doc.has(idOf(fromPrev)) && !moved.hasSpare(prev!)) moved.takeOut(prev!, fromPrev, node, prevLabel); const laneBefore = laneOf(this.edit.doc, node); this.leaveScope([node, ...boundaries], oldScope, targetScope, changes); this.edit.doc.invalidate(); const own = laneBefore && (laneBefore.get('flowNodeRef') ?? []).includes(node) ? laneBefore : undefined; const inherited = !explicitLane && is(targetScope, 'bpmn:Process') ? LaneInheritance.of(this.edit.doc, placement) : LaneInheritance.NONE; - const holdOwn = !!next && is(node, 'bpmn:Gateway'); - new NodePlacement(this.edit, placement).place(node, changes, (source) => (source === node && holdOwn ? undefined : moved.spareOf(source))); - moved.putBack(); + new NodePlacement(this.edit, placement).place(node, changes); for (const flow of new Set([...this.edit.doc.incoming(node), ...this.edit.doc.outgoing(node)])) new EventGatewayRule(this.edit).warnFlow(flow, changes); if (boundaries.length) insertAfterInScope(targetScope, node, ...boundaries); this.fixFlows(boundaries, targetScope, changes); diff --git a/src/domain/editing/elements/NodePlacement.ts b/src/domain/editing/elements/NodePlacement.ts index 90a7fa75..7fd112e0 100644 --- a/src/domain/editing/elements/NodePlacement.ts +++ b/src/domain/editing/elements/NodePlacement.ts @@ -62,20 +62,15 @@ export class NodePlacement { /** * The node must not be connected yet; returns the flow into it. `after X` appends when X is * a gateway or has no outgoing flow, splices into its one flow, else E_HAS_SUCCESSOR; - * `before` mirrors it. `reuse`: a moved node's old out-flow, put back instead of a new one. + * `before` mirrors it. * On a prepend the flow options go to the one flow it creates (node -> anchor). */ - place(node: El, changes: ChangeSet, reuse?: (source: El) => El | undefined): El | undefined { + place(node: El, changes: ChangeSet): El | undefined { const { placement } = this; const doc = this.edit.doc; const mode = this.mode; const scope = this.scope(); let entryFlow: El | undefined; - const outFlow = reuse?.(node); - const reused = (source: El): { reuse?: El } => { - const flow = source === node ? outFlow : reuse?.(source); - return flow ? { reuse: flow } : {}; - }; if (mode === 'on') { const host = this.edit.require(placement.on!, 'bpmn:Activity', 'activity'); @@ -96,22 +91,22 @@ export class NodePlacement { insertAfterInScope(scope, undefined, node); } else if (mode === 'flow') { const flow = this.edit.require(placement.flow!, 'bpmn:SequenceFlow', 'sequence flow'); - entryFlow = this.spliceWithOptions(node, flow, changes, outFlow); + entryFlow = this.spliceWithOptions(node, flow, changes); } else if (mode === 'between') { const afterNode = this.edit.require(placement.after!, 'bpmn:FlowNode'); const beforeNode = this.edit.require(placement.before!, 'bpmn:FlowNode'); const flow = this.requireFlowBetween(afterNode, beforeNode); - entryFlow = this.spliceWithOptions(node, flow, changes, outFlow); + entryFlow = this.spliceWithOptions(node, flow, changes); } else if (mode === 'after') { const anchor = this.edit.require(placement.after!, 'bpmn:FlowNode'); const out = doc.outgoing(anchor); if (is(anchor, 'bpmn:Gateway') || out.length === 0) { insertAfterInScope(scope, lastFlowOf(scope, anchor) ?? anchor, node); - entryFlow = new SequenceFlows(this.edit).create(anchor, node, { ...new FlowOptions(placement).toAttrs(), ...reused(anchor) }, node); + entryFlow = new SequenceFlows(this.edit).create(anchor, node, new FlowOptions(placement).toAttrs(), node); changes.create(ChangeSet.flowEntryOf(entryFlow)); changes.note(`appended after ${idOf(anchor)}`); } else if (out.length === 1) { - entryFlow = this.spliceWithOptions(node, out[0]!, changes, outFlow); + entryFlow = this.spliceWithOptions(node, out[0]!, changes); } else { throw modelError('E_HAS_SUCCESSOR', `${idOf(anchor)} already has ${out.length} outgoing flows; say which one to insert into`, { element: idOf(anchor), @@ -125,7 +120,7 @@ export class NodePlacement { const isJoin = is(anchor, 'bpmn:Gateway') && inc.length >= 2; if (inc.length === 0 || isJoin) { insertBeforeInScope(scope, anchor, node); - const flow = new SequenceFlows(this.edit).create(node, anchor, { ...new FlowOptions(placement).toAttrs(), ...reused(node) }, node); + const flow = new SequenceFlows(this.edit).create(node, anchor, new FlowOptions(placement).toAttrs(), node); changes.create(ChangeSet.flowEntryOf(flow)); changes.note(`prepended before ${idOf(anchor)}`); const used = new FlowOptions(placement).flags; @@ -133,7 +128,7 @@ export class NodePlacement { changes.note(`${used.join(', ')} applied to ${idOf(flow)} (${idOf(node)} -> ${idOf(anchor)}): ${idOf(anchor)} ${isJoin ? 'is a join gateway' : 'had no incoming flow'}, so no flow into ${idOf(node)} was created`); } } else if (inc.length === 1) { - entryFlow = this.spliceWithOptions(node, inc[0]!, changes, outFlow); + entryFlow = this.spliceWithOptions(node, inc[0]!, changes); } else { throw modelError('E_HAS_PREDECESSOR', `${idOf(anchor)} already has ${inc.length} incoming flows; say which one to insert into`, { element: idOf(anchor), @@ -217,11 +212,11 @@ export class NodePlacement { } /** `flow` (now source -> node) is the flow into the node, so it takes the flow options. */ - private spliceWithOptions(node: El, flow: El, changes: ChangeSet, outFlow?: El): El { + private spliceWithOptions(node: El, flow: El, changes: ChangeSet): El { this.assertSpliceable(node, flow); const src = flow.get('sourceRef'); const tgt = flow.get('targetRef'); - const { second, renamedFrom } = new SequenceFlows(this.edit).splice(flow, node, outFlow, this.placement.flowId); + const { second, renamedFrom } = new SequenceFlows(this.edit).splice(flow, node, this.placement.flowId); changes.create(ChangeSet.flowEntryOf(second)); changes.change(ChangeSet.flowEntryOf(flow, `${idOf(src)} -> ${idOf(node)} (was -> ${idOf(tgt)})${renamedNote(renamedFrom)}`)); changes.note(`inserted between ${idOf(src)} and ${idOf(tgt)}`); diff --git a/test/adapters/in/report/report-delta.test.ts b/test/adapters/in/report/report-delta.test.ts index ddb6d24d..6ea19b1c 100644 --- a/test/adapters/in/report/report-delta.test.ts +++ b/test/adapters/in/report/report-delta.test.ts @@ -82,9 +82,6 @@ describe('warnings as a delta', () => { expect(lines[0]).toMatch(/^warning W_UNREACHABLE x4 \[T1, T2, T3, End\]: task T1 "Twin" is not reachable from any start event/); // two of a code stay two lines expect(warningLines([{ code: 'W_A', message: 'a', element: 'X' }, { code: 'W_A', message: 'b', element: 'Y' }])).toEqual(['warning W_A X: a', 'warning W_A Y: b']); - // a stale flow id never floods: each hint is the rename command of its flow - const stale = ['F1', 'F2', 'F3'].map((id) => ({ code: 'W_FLOW_ID_STALE', message: `${id} stale`, element: id, hint: `\`bpmn set ${id} id=${id}x\`` })); - expect(warningLines(stale)).toEqual(stale.map((warning) => `warning W_FLOW_ID_STALE ${warning.element}: ${warning.message} (${warning.hint})`)); }); it('a new document reports its warnings (there was no file before)', async () => { diff --git a/test/domain/batch/batch.test.ts b/test/domain/batch/batch.test.ts index ee975682..665e98ab 100644 --- a/test/domain/batch/batch.test.ts +++ b/test/domain/batch/batch.test.ts @@ -548,33 +548,16 @@ describe('guide, README and docs: documented syntax matches the CLI', () => { expect(DOCS).toMatch(/alternately below and above/); }); - it('documents keeping flow ids wherever renames are described (--keep-flow-ids, BPMN_KEEP_FLOW_IDS, W_FLOW_ID_STALE)', () => { - const keep = COMMON_OPTIONS.find((option) => option.option === '--keep-flow-ids')!; - expect(keep.description).toMatch(/W_FLOW_ID_STALE[\s\S]*BPMN_KEEP_FLOW_IDS=1/); - expect(COMMON_OPTIONS.map((option) => option.option)).toContain('--no-keep-flow-ids'); - for (const [name, text] of Object.entries({ guide: guideText(), DOCS, library: repoFile('docs/library.md') })) { - for (const word of ['--keep-flow-ids', 'W_FLOW_ID_STALE']) expect(text, `${name}: ${word}`).toContain(word); + it('documents no way to keep flow ids: a flow whose id names its ends always follows them', () => { + expect(COMMON_OPTIONS.map((option) => option.option)).not.toContain('--keep-flow-ids'); + const texts = { guide: guideText(), short: guideShort(), DOCS, library: repoFile('docs/library.md'), ops: repoFile('docs/ops.md'), skill: repoFile('plugins/bpmn-cli/skills/bpmn/SKILL.md') }; + for (const [name, text] of Object.entries(texts)) { + for (const word of ['keep-flow-ids', 'keepFlowIds', 'BPMN_KEEP_FLOW_IDS', 'W_FLOW_ID_STALE']) expect(text, `${name}: ${word}`).not.toContain(word); } - expect(DOCS).toContain('BPMN_KEEP_FLOW_IDS=1'); - expect(guideShort()).toContain('--keep-flow-ids'); - expect(catalogue('W_FLOW_ID_STALE').fix).toContain('`bpmn set id=`'); - // the first variable that can fail a command: the code an agent looks up names it - expect(catalogue('E_USAGE').meaning).toContain('BPMN_KEEP_FLOW_IDS'); - expect(DOCS).not.toMatch(/the CLI has no switch/); - // a batch keeps the ids of the flows it does not remove; a move puts the node's out-flow back; outAs names a splice's new flow for (const [name, text] of Object.entries({ guide: guideText(), ops: repoFile('docs/ops.md') })) { - expect(text, name).not.toMatch(/every flow the file had keeps its id/); - expect(text, name).toMatch(/unless an op\s+removes it/); - expect(text, name).toMatch(/move`? puts the moved\s+node's\s+out-flow back/); expect(text, name).toMatch(/"outAs"`?\s+the\s+(new\s+)?flow\s+(the\s+splice\s+creates\s+\(new\s+)?node -> old\s+successor/); expect(text, name).not.toMatch(/has no alias/); } - // the skill's pre-approved permission covers `bpmn ...`, not an inline variable in front of it - expect(repoFile('plugins/bpmn-cli/skills/bpmn/SKILL.md')).toMatch(/--keep-flow-ids[\s\S]*BPMN_KEEP_FLOW_IDS[\s\S]*not pre-approved/); - // a result whose flow ids the variable kept says so (src/adapters/in/cli/MutationCommand.ts keptByEnvNote): the reader may not see the variable - for (const [name, text] of Object.entries({ guide: guideText(), DOCS, skill: repoFile('plugins/bpmn-cli/skills/bpmn/SKILL.md') })) { - expect(text, name).toContain('`note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 ('); - } }); it('keeps usages, options and the result description in sync with the CLI', () => { diff --git a/test/domain/document/ids/drawn-ids.test.ts b/test/domain/document/ids/drawn-ids.test.ts index e3e69c89..ff5bd2e7 100644 --- a/test/domain/document/ids/drawn-ids.test.ts +++ b/test/domain/document/ids/drawn-ids.test.ts @@ -126,29 +126,25 @@ ${LINE.replace(' [...xml.matchAll(/]*?)\/>/g)].map((match) => match[1]!); it('a splice (add, split; with and without layout, with ids kept too) gives it an id from its new ends and lists it once, as changed', async () => { - for (const keepFlowIds of [false, true]) { - for (const layout of [false, undefined]) { - const outcome = await applyToXml(OUT, [{ op: 'add', kind: 'task', name: 'Log', after: 'Task_1' }], { keepFlowIds, ...(layout === false ? { layout } : {}) }); - await sound(outcome.xml); - expect(flows(outcome.xml)).toEqual(['id="Flow_1" sourceRef="Start_1" targetRef="Task_1"', 'id="flow_checkToLog" sourceRef="Task_1" targetRef="task_log"', 'id="flow_logToEnd" sourceRef="task_log" targetRef="End_1"']); - expect(outcome.result.changed.map((change) => `${change.id}: ${change.detail}`)).toEqual(['flow_checkToLog: Task_1 -> task_log (was -> End_1) (id added: it had none)']); - expect(outcome.result.created.map((change) => change.id)).toEqual(['task_log', 'flow_logToEnd']); - expect([outcome.result.renamed, outcome.result.warnings.added]).toEqual([undefined, []]); - } - const split = await applyToXml(OUT, [{ op: 'split', after: 'Task_1', name: 'Ok?', branches: [{ flowName: 'yes', nodes: [{ kind: 'task', name: 'A' }] }, { flowName: 'no', nodes: [] }] }], { keepFlowIds, layout: false }); - expect(split.result.changed.map((change) => `${change.id}: ${change.detail}`)).toContain('flow_checkToOk: Task_1 -> gateway_ok (was -> End_1) (id added: it had none)'); - expect(flows(split.xml)).toContain('id="flow_checkToOk" sourceRef="Task_1" targetRef="gateway_ok"'); + for (const layout of [false, undefined]) { + const outcome = await applyToXml(OUT, [{ op: 'add', kind: 'task', name: 'Log', after: 'Task_1' }], { ...(layout === false ? { layout } : {}) }); + await sound(outcome.xml); + expect(flows(outcome.xml)).toEqual(['id="Flow_1" sourceRef="Start_1" targetRef="Task_1"', 'id="flow_checkToLog" sourceRef="Task_1" targetRef="task_log"', 'id="flow_logToEnd" sourceRef="task_log" targetRef="End_1"']); + expect(outcome.result.changed.map((change) => `${change.id}: ${change.detail}`)).toEqual(['flow_checkToLog: Task_1 -> task_log (was -> End_1) (id added: it had none)']); + expect(outcome.result.created.map((change) => change.id)).toEqual(['task_log', 'flow_logToEnd']); + expect([outcome.result.renamed, outcome.result.warnings.added]).toEqual([undefined, []]); } + const split = await applyToXml(OUT, [{ op: 'split', after: 'Task_1', name: 'Ok?', branches: [{ flowName: 'yes', nodes: [{ kind: 'task', name: 'A' }] }, { flowName: 'no', nodes: [] }] }], { layout: false }); + expect(split.result.changed.map((change) => `${change.id}: ${change.detail}`)).toContain('flow_checkToOk: Task_1 -> gateway_ok (was -> End_1) (id added: it had none)'); + expect(flows(split.xml)).toContain('id="flow_checkToOk" sourceRef="Task_1" targetRef="gateway_ok"'); }); it('add --flow-id on a splice into it: the flow takes that id (claimed like the id of a flow add creates), without W_OPTION_IGNORED', async () => { - for (const keepFlowIds of [false, true]) { - const outcome = await applyToXml(OUT, [{ op: 'add', kind: 'task', name: 'Log', after: 'Task_1', flowId: 'Flow_IntoLog' }], { keepFlowIds, layout: false }); - expect(flows(outcome.xml)).toEqual(['id="Flow_1" sourceRef="Start_1" targetRef="Task_1"', 'id="Flow_IntoLog" sourceRef="Task_1" targetRef="task_log"', 'id="flow_logToEnd" sourceRef="task_log" targetRef="End_1"']); - expect(outcome.result.changed.map((change) => `${change.id}: ${change.detail}`)).toEqual(['Flow_IntoLog: Task_1 -> task_log (was -> End_1) (id added: it had none)']); - expect(outcome.result.warnings.added).toEqual([]); - await expect(applyToXml(OUT, [{ op: 'add', kind: 'task', name: 'Log', after: 'Task_1', flowId: 'Flow_1' }], { keepFlowIds, layout: false })).rejects.toMatchObject({ code: 'E_DUPLICATE_ID' }); - } + const outcome = await applyToXml(OUT, [{ op: 'add', kind: 'task', name: 'Log', after: 'Task_1', flowId: 'Flow_IntoLog' }], { layout: false }); + expect(flows(outcome.xml)).toEqual(['id="Flow_1" sourceRef="Start_1" targetRef="Task_1"', 'id="Flow_IntoLog" sourceRef="Task_1" targetRef="task_log"', 'id="flow_logToEnd" sourceRef="task_log" targetRef="End_1"']); + expect(outcome.result.changed.map((change) => `${change.id}: ${change.detail}`)).toEqual(['Flow_IntoLog: Task_1 -> task_log (was -> End_1) (id added: it had none)']); + expect(outcome.result.warnings.added).toEqual([]); + await expect(applyToXml(OUT, [{ op: 'add', kind: 'task', name: 'Log', after: 'Task_1', flowId: 'Flow_1' }], { layout: false })).rejects.toMatchObject({ code: 'E_DUPLICATE_ID' }); }); it('in a drawing kept as it is, the incremental layout draws the flow by its new id', async () => { @@ -158,13 +154,11 @@ ${LINE.replace('[\s\S]*?<\/bpmndi:BPMNEdge>/, '') .replace(' `/); - expect(outcome.result.changed.map((change) => change.id)).toContain('flow_checkToLog'); - } + const outcome = await applyToXml(kept, [{ op: 'add', kind: 'task', name: 'Log', after: 'Task_1' }]); + await sound(outcome.xml); + expect(outcome.result.layout.mode).toBe('incremental'); + expect(outcome.xml).toMatch(//); + expect(outcome.result.changed.map((change) => change.id)).toContain('flow_checkToLog'); }); it('a bridge gives the flow into the removed node an id; a removed flow without id prints `(no id)` and is left out of the summary', async () => { diff --git a/test/domain/document/ids/id-style.test.ts b/test/domain/document/ids/id-style.test.ts index 93febd84..4b155909 100644 --- a/test/domain/document/ids/id-style.test.ts +++ b/test/domain/document/ids/id-style.test.ts @@ -174,102 +174,6 @@ describe('new ids follow the defaults of element-id-naming', () => { }); }); -describe('flow ids kept (keepFlowIds): a stale id is recognised in the current flow form', () => { - /** A second flow between the same pair with a collision suffix. */ - const SUFFIXED = definitionsXml(` - flow_startToOk - flow_startToOkflow_okToBookflow_okToBook2 - flow_okToBookflow_okToBook2 - - - `); - - /** `old -> new` of the renames of the default, and `flow -> suggested id` of the W_FLOW_ID_STALE with ids kept. */ - async function bothModes(xml: string, ops: EditCommand[]): Promise<{ renamed: string[]; stale: string[] }> { - const follow = await Doc.fromXml(xml); - const renamed = runOps(follow, ops).renames.map((rename) => `${rename.from} -> ${rename.to}`); - const keep = await Doc.fromXml(xml); - keep.followFlowEnds = false; - const cs = runOps(keep, ops); - expect(cs.renames).toEqual([]); - const stale = cs.warnings.filter((warning) => warning.code === 'W_FLOW_ID_STALE').map((warning) => `${warning.element} -> ${/ id=(\S+)`/.exec(warning.hint!)![1]}`); - return { renamed, stale }; - } - - it('the suggestion is the id the default rename gives (also for a suffixed id)', async () => { - const cases: Array<[string, EditCommand, string]> = [ - [CAMEL, { op: 'add', kind: 'userTask', name: 'Pack goods', after: 'serviceTask_checkStock' }, 'flow_checkStockToApproveOrder -> flow_checkStockToPackGoods'], - [SUFFIXED, { op: 'add', kind: 'task', name: 'Review', flow: 'flow_okToBook2' }, 'flow_okToBook2 -> flow_okToReview'], - ]; - for (const [xml, op, line] of cases) { - const { renamed, stale } = await bothModes(xml, [op]); - expect(renamed, line).toEqual([line]); - expect(stale, line).toEqual([line]); - } - }); - - it('an end `set id=` renamed earlier in the batch still names its flows (the id it had)', async () => { - const ops: EditCommand[] = [{ op: 'set', id: 'serviceTask_checkStock', values: { id: 'serviceTask_verifyStock' } }, { op: 'add', kind: 'userTask', name: 'Pack goods', after: 'serviceTask_verifyStock' }]; - const line = 'flow_checkStockToApproveOrder -> flow_verifyStockToPackGoods'; - expect(await bothModes(CAMEL, ops)).toEqual({ renamed: [line], stale: [line] }); - }); - - it('a glued suffix with No after a digit still names an end `set` renamed earlier in the batch (flow_approveToStep1No2)', async () => { - const xml = definitionsXml(` - flow_goToApprove - flow_goToApproveflow_approveToStep1flow_approveToStep1No2 - flow_approveToStep1flow_approveToStep1No2flow_step1ToDone - flow_step1ToDone - - - - `); - const ops: EditCommand[] = [ - { op: 'set', id: 'userTask_approve', values: { id: 'userTask_review', name: 'Review' } }, - { op: 'add', kind: 'task', name: 'Check', flow: 'flow_approveToStep1No2' }, - ]; - expect(await bothModes(xml, ops)).toEqual({ renamed: ['flow_approveToStep1No2 -> flow_reviewToCheck'], stale: ['flow_approveToStep1No2 -> flow_reviewToCheck'] }); - }); - - it('flow ids that do not name their ends in the current form neither follow nor go stale (hashes, numbers, older forms)', async () => { - for (const [xml, anchor] of [[MODELER, 'Activity_0k3x9qa'], [NUMBERED, 'Task_1'], [OLD_MODELER, 'Task_1bcd234']] as const) { - expect(await bothModes(xml, [{ op: 'add', kind: 'task', name: 'Pack', after: anchor }])).toEqual({ renamed: [], stale: [] }); - } - // Flow_AToB named its ends in earlier versions: it keeps its id now - const older = chain( - [ - ['task', 'Activity_A', 'A'], - ['task', 'Activity_B', 'B'], - ], - ['Flow_AToB'], - ); - expect(await bothModes(older, [{ op: 'add', kind: 'task', name: 'Log', after: 'Activity_A' }])).toEqual({ renamed: [], stale: [] }); - }); - - it('two flows that end up between the same ends get the id and the suffix of the default rename, so every hint can be followed', async () => { - const joined = definitionsXml(` - flow_startToOk - flow_startToOkflow_okToAflow_okToB - flow_okToAflow_aToJoin - flow_okToBflow_bToJoin - flow_aToJoinflow_bToJoin - - - - - `); - const expected = ['flow_okToA -> flow_okToJoin', 'flow_okToB -> flow_okToJoin2']; - for (const ops of [[{ op: 'remove', ids: ['task_a', 'task_b'] }], [{ op: 'remove', ids: ['task_a'] }, { op: 'remove', ids: ['task_b'] }]] as EditCommand[][]) { - expect(await bothModes(joined, ops), JSON.stringify(ops)).toEqual({ renamed: expected, stale: expected }); - } - const doc = await Doc.fromXml(joined); - doc.followFlowEnds = false; - runOps(doc, [{ op: 'remove', ids: ['task_a', 'task_b'] }]); - runOps(doc, expected.map((line) => ({ op: 'set', id: line.split(' -> ')[0]!, values: { id: line.split(' -> ')[1]! } }))); - expect(flowBetween(doc, 'gateway_ok', 'gateway_join')).toMatch(/^flow_okToJoin2?$/); - }); -}); - describe('speaking ids on two branches of a file (independent edits)', () => { const base = chain( [ diff --git a/test/domain/editing/flow-ids-follow-ends.test.ts b/test/domain/editing/flow-ids-follow-ends.test.ts new file mode 100644 index 00000000..a3aa88ef --- /dev/null +++ b/test/domain/editing/flow-ids-follow-ends.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; +import { Doc } from '../../../src/domain/document/Doc.js'; +import { runOps } from '../../../src/index.js'; +import type { EditCommand } from '../../../src/domain/commands/types.js'; +import { definitionsXml } from '../../helpers/helpers.js'; + +const LINE = definitionsXml(` + flow_startToA + flow_startToAflow_aToB + flow_aToBflow_bToC + flow_bToCflow_cToDone + flow_cToDone + + + + `); + +async function renamesOf(ops: EditCommand[]): Promise { + return runOps(await Doc.fromXml(LINE), ops).renames.map((rename) => `${rename.from} -> ${rename.to}`); +} + +describe('a flow whose id names its ends is always renamed after its new ends (#126)', () => { + it.each<[string, EditCommand, string[]]>([ + ['add --after (splice)', { op: 'add', kind: 'task', name: 'X', after: 'task_a' }, ['flow_aToB -> flow_aToX']], + ['add --before (splice)', { op: 'add', kind: 'task', name: 'X', before: 'task_b' }, ['flow_aToB -> flow_aToX']], + ['add --flow (splice)', { op: 'add', kind: 'task', name: 'X', flow: 'flow_aToB' }, ['flow_aToB -> flow_aToX']], + ['set target=', { op: 'set', id: 'flow_aToB', values: { target: 'task_c' } }, ['flow_aToB -> flow_aToC']], + ['set source=', { op: 'set', id: 'flow_bToC', values: { source: 'task_a' } }, ['flow_bToC -> flow_aToC']], + ['remove with bridging', { op: 'remove', ids: ['task_b'] }, ['flow_aToB -> flow_aToC']], + ['split', { op: 'split', after: 'task_a', name: 'Ok?', branches: [{ nodes: [] }, { nodes: [{ kind: 'task', name: 'Y' }] }] }, ['flow_aToB -> flow_aToOk']], + ['move --after (bridge and splice)', { op: 'move', ids: ['task_b'], after: 'task_c' }, ['flow_aToB -> flow_aToC', 'flow_cToDone -> flow_cToB']], + ['move of a chain', { op: 'move', ids: ['task_a', 'task_b'], after: 'task_c' }, ['flow_startToA -> flow_startToB', 'flow_cToDone -> flow_cToA', 'flow_startToB -> flow_startToC', 'flow_aToDone -> flow_aToB2']], + ['move --before (bridge and splice)', { op: 'move', ids: ['task_c'], before: 'task_b' }, ['flow_bToC -> flow_bToDone', 'flow_aToB -> flow_aToC']], + ])('%s', async (_command, op, expected) => { + expect(await renamesOf([op])).toEqual(expected); + }); + + it('an id in another form keeps it', async () => { + const xml = LINE.replaceAll('flow_aToB', 'Flow_0yq6cfd'); + const changes = runOps(await Doc.fromXml(xml), [{ op: 'add', kind: 'task', name: 'X', after: 'task_a' }]); + expect(changes.renames).toEqual([]); + }); +}); diff --git a/test/domain/editing/keep-flow-ids.test.ts b/test/domain/editing/keep-flow-ids.test.ts deleted file mode 100644 index 516ddd6c..00000000 --- a/test/domain/editing/keep-flow-ids.test.ts +++ /dev/null @@ -1,907 +0,0 @@ -/** - * Flow ids kept (issue #32): where flow ids are API (generated code, process - * tests), `keepFlowIds` (`--keep-flow-ids`, BPMN_KEEP_FLOW_IDS) keeps the id - * of every flow the file had when an edit changes its ends, and the change - * names each flow whose id no longer names its ends (W_FLOW_ID_STALE, with - * the `bpmn set` that renames it). A flow the same change created still - * follows its ends. Synthetic fixtures only. - */ -import { readFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { describe, expect, it } from 'vitest'; -import { applyToXml, layoutXml, newXml } from '../../../src/api.js'; -import { Doc } from '../../../src/domain/document/Doc.js'; -import type { El } from '../../../src/domain/document/utils/moddle.js'; -import type { Finding } from '../../../src/domain/document/Finding.js'; -import { runOp, runOps } from '../../../src/index.js'; -import type { EditCommand } from '../../../src/domain/commands/types.js'; -import { mutateDoc, renderMutation } from '../../../src/index.js'; -import type { ChangeSet } from '../../../src/domain/document/ChangeSet.js'; -import { definitionsXml, flowBetween } from '../../helpers/helpers.js'; - -/** A file of the default style with `_di` DI (`bpmn`: the engine's `BPMNShape_`): start -> Check -> Ok? -> Book (default) -> Done. */ -async function drawnDefault(style: 'suffix' | 'bpmn' = 'suffix'): Promise { - const ops: EditCommand[] = [ - { op: 'add', kind: 'startEvent', name: 'Start' }, - { op: 'add', kind: 'task', name: 'Check', after: 'startEvent_start' }, - { op: 'add', kind: 'exclusiveGateway', name: 'Ok?', after: 'task_check' }, - { op: 'add', kind: 'task', name: 'Book', after: 'gateway_ok', default: true }, - { op: 'add', kind: 'endEvent', name: 'Done', after: 'task_book' }, - ]; - const xml = (await applyToXml((await newXml({ processName: 'P' })).xml, ops)).xml; - return style === 'bpmn' ? xml : xml.replace(/BPMN(Shape|Edge)_([A-Za-z0-9_]+)/g, '$2_di'); -} - -/** The same chain without DI, for the ops alone. */ -const CHAIN = definitionsXml(` - flow_startToCheck - flow_startToCheckflow_checkToOk - flow_checkToOkflow_okToBook - flow_okToBookflow_bookToDone - flow_bookToDone - - - - `); - -/** A split into A and B, joined before Done (remove --bridge-all of the join). */ -const JOINED = definitionsXml(` - flow_startToOk - flow_startToOkflow_okToAflow_okToB - flow_okToAflow_aToJoin - flow_okToBflow_bToJoin - flow_aToJoinflow_bToJoinflow_joinToDone - flow_joinToDone - - - - - - `); - -/** Start -> A -> B -> C -> Done, for a move of two nodes. */ -const LINE = definitionsXml(` - flow_startToA - flow_startToAflow_aToB - flow_aToBflow_bToC - flow_bToCflow_cToDone - flow_cToDone - - - - `); - -/** LINE with A as an exclusive gateway: a move after it appends. */ -const FORKED = LINE.replace(//, ''); - -/** LINE with a second flow out of B (B -> Done): B is not bridged, so a move has no out-flow of B to put back. */ -const TWO_OUT = LINE.replace('flow_bToC', 'flow_bToCflow_bToDone') - .replace('flow_cToDone', 'flow_cToDoneflow_bToDone') - .replace('\n C has a label and a condition: B is not bridged. */ -const JOIN_B = definitionsXml(` - flow_startToB - flow_aToB - flow_startToBflow_aToBflow_bToC - flow_bToCflow_cToDone - flow_cToDone - - - \${approved} - `); - -/** Start -> X -> Done, and apart from it A -> B (A has no flow in, so it is not bridged) with a label and a condition. */ -const APART = definitionsXml(` - flow_startToX - flow_startToXflow_xToDone - flow_xToDone - flow_aToB - flow_aToB - - - \${approved}`); - -/** A split into A and B, both ending in Done: Done has two flows in. */ -const SHARED_END = definitionsXml(` - flow_startToOk - flow_startToOkflow_okToAflow_okToB - flow_okToAflow_aToDone - flow_okToBflow_bToDone - flow_aToDoneflow_bToDone - - - - - `); - -/** A Camunda Modeler file: hashed flow ids say nothing about their ends. */ -const MODELER = definitionsXml(` - Flow_0a1b2c3 - Flow_0a1b2c3Flow_1q2w3e4 - Flow_1q2w3e4 - - `); - -const stale = (ws: readonly Finding[]): Finding[] => ws.filter((warning) => warning.code === 'W_FLOW_ID_STALE'); -/** ` -> => ` of each W_FLOW_ID_STALE. */ -const staleLines = (ws: readonly Finding[]): string[] => stale(ws).map((warning) => `${warning.element} ${warning.related!.join('->')} => ${/ id=(\S+)`/.exec(warning.hint!)![1]}`); - -/** Runs ops on CHAIN (or `xml`) with ids kept, without the pipeline. */ -async function kept(ops: EditCommand[], xml = CHAIN): Promise<{ doc: Doc; warnings: Finding[]; renames: number; cs: ChangeSet }> { - const doc = await Doc.fromXml(xml); - doc.followFlowEnds = false; - const cs = runOps(doc, ops); - return { doc, warnings: cs.warnings, renames: cs.renames.length, cs }; -} - -/** The sequence flows of a change set: `created` / `removed` ids. */ -const flowIds = (changes: ChangeSet['created']): string[] => changes.filter((change) => change.kind === 'sequenceFlow').map((change) => change.id); - -describe('keepFlowIds: a flow the file had keeps its id and the change names it', () => { - it('a splice keeps the flow id, its DI edge and the default reference; one W_FLOW_ID_STALE with the rename command, no renamed', async () => { - const xml = await drawnDefault(); - const result = await applyToXml(xml, [{ op: 'add', kind: 'task', name: 'Review', flow: 'flow_okToBook' }], { keepFlowIds: true }); - expect(result.result.created.map((change) => change.id)).toEqual(['task_review', 'flow_reviewToBook']); - expect(result.result.changed.map((change) => `${change.id}: ${change.detail}`)).toContain('flow_okToBook: gateway_ok -> task_review (was -> task_book)'); - expect(result.result.changed.some((change) => change.detail?.includes('renamed from'))).toBe(false); - expect(result.result.renamed).toBeUndefined(); - expect(result.xml).toContain(''); - expect(result.xml).toContain(''); - expect(result.xml).toContain(''); - expect(stale(result.result.warnings.added)).toEqual([ - { - code: 'W_FLOW_ID_STALE', - message: 'Flow flow_okToBook kept its id, which names its old ends gateway_ok -> task_book; it now runs gateway_ok -> task_review', - element: 'flow_okToBook', - related: ['gateway_ok', 'task_review'], - hint: 'Code and tests that use the id keep working; to rename it together with them: `bpmn set flow_okToBook id=flow_okToReview`.', - }, - ]); - // the hint's command renames it deliberately; nothing is stale any more - const renamed = await applyToXml(result.xml, [{ op: 'set', id: 'flow_okToBook', values: { id: 'flow_okToReview' } }], { keepFlowIds: true }); - expect(renamed.xml).toContain(''); - expect(stale(renamed.result.warnings.added)).toEqual([]); - }); - - it('every op that changes the ends of a flow: add --after / --before / --flow / --after --before, split, remove, set source= / target=', async () => { - const cases: Array<[EditCommand, string]> = [ - [{ op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, 'flow_checkToOk task_check->task_review => flow_checkToReview'], - [{ op: 'add', kind: 'task', name: 'Prepare', before: 'task_check' }, 'flow_startToCheck startEvent_start->task_prepare => flow_startToPrepare'], - [{ op: 'add', kind: 'task', name: 'Review', flow: 'flow_okToBook' }, 'flow_okToBook gateway_ok->task_review => flow_okToReview'], - [{ op: 'add', kind: 'task', name: 'Review', after: 'task_check', before: 'gateway_ok' }, 'flow_checkToOk task_check->task_review => flow_checkToReview'], - [{ op: 'split', after: 'task_check', name: 'Valid?', branches: [{ nodes: [{ kind: 'task', name: 'Fix' }] }, { nodes: [] }] }, 'flow_checkToOk task_check->gateway_valid => flow_checkToValid'], - [{ op: 'remove', ids: ['task_book'] }, 'flow_okToBook gateway_ok->endEvent_done => flow_okToDone'], - [{ op: 'set', id: 'flow_bookToDone', values: { target: 'task_check' } }, 'flow_bookToDone task_book->task_check => flow_bookToCheck'], - [{ op: 'set', id: 'flow_bookToDone', values: { source: 'task_check' } }, 'flow_bookToDone task_check->endEvent_done => flow_checkToDone'], - ]; - for (const [op, line] of cases) { - const result = await kept([op]); - expect(staleLines(result.warnings), JSON.stringify(op)).toEqual([line]); - expect(result.renames, JSON.stringify(op)).toBe(0); - const flow = line.split(' ')[0]!; - const [source, target] = line.split(' ')[1]!.split('->'); - expect(flowBetween(result.doc, source!, target!)).toBe(flow); - } - }); - - it('a move keeps the bridge at its old place, the splice at its new one and the node\'s out-flow, put back there; remove --bridge-all one per predecessor', async () => { - const moved = await kept([{ op: 'move', ids: ['task_book'], before: 'task_check' }]); - expect(staleLines(moved.warnings).sort()).toEqual([ - 'flow_bookToDone task_book->task_check => flow_bookToCheck', - 'flow_okToBook gateway_ok->endEvent_done => flow_okToDone', - 'flow_startToCheck startEvent_start->task_book => flow_startToBook', - ]); - // Book's out-flow is its out-flow at the new place: changed, neither removed nor created - expect(flowBetween(moved.doc, 'task_book', 'task_check')).toBe('flow_bookToDone'); - expect([flowIds(moved.cs.created), flowIds(moved.cs.removed)]).toEqual([[], []]); - expect(moved.cs.changed.map((change) => `${change.id}: ${change.detail}`)).toContain('flow_bookToDone: task_book -> task_check (was -> endEvent_done)'); - const bridged = await kept([{ op: 'remove', ids: ['gateway_join'], bridgeAll: true }], JOINED); - expect(staleLines(bridged.warnings)).toEqual(['flow_aToJoin task_a->endEvent_done => flow_aToDone', 'flow_bToJoin task_b->endEvent_done => flow_bToDone']); - }); - - it('nodes moved together keep the flow between them, also when they are appended; later ops of the batch use every id', async () => { - const result = await applyToXml(LINE, [ - { op: 'move', ids: ['task_b', 'task_c'], after: 'startEvent_start' }, - { op: 'set', id: 'flow_bToC', values: { name: 'go' } }, - ], { keepFlowIds: true, layout: false }); - expect([flowIds(result.result.created), flowIds(result.result.removed)]).toEqual([[], []]); - expect(result.xml).toContain(''); - // B -> C runs between its old ends again: no warning for it - expect(staleLines(result.result.warnings.added).sort()).toEqual([ - 'flow_aToB task_a->endEvent_done => flow_aToDone', - 'flow_cToDone task_c->task_a => flow_cToA', - 'flow_startToA startEvent_start->task_b => flow_startToB', - ]); - // after a gateway B is appended (a new flow into it); C is appended to B through B's old out-flow, C's has no place - const appended = await kept([{ op: 'move', ids: ['task_b', 'task_c'], after: 'task_a' }], FORKED); - expect(flowBetween(appended.doc, 'task_b', 'task_c')).toBe('flow_bToC'); - expect([flowIds(appended.cs.created), flowIds(appended.cs.removed)]).toEqual([['flow_aToB2'], ['flow_cToDone']]); - expect(staleLines(appended.warnings)).toEqual(['flow_aToB task_a->endEvent_done => flow_aToDone']); - }); - - it('nodes moved together keep the flow between them when the second is not bridged: an end event, a join', async () => { - // C is put back where it was, then Done: Done has no flow out, so C -> Done is taken out and put back by the append - const tail = await applyToXml(LINE, [{ op: 'move', ids: ['task_c', 'endEvent_done'], after: 'task_b' }], { keepFlowIds: true, layout: false }); - expect([flowIds(tail.result.created), flowIds(tail.result.removed)]).toEqual([[], []]); - expect(tail.result.warnings.added.map((warning) => warning.code)).toEqual([]); - expect(tail.xml).toContain(''); - // Done has two flows in: B -> Done is removed like every other flow of a node that is not bridged, A -> Done stays A's - const shared = await kept([{ op: 'move', ids: ['task_a', 'endEvent_done'], after: 'task_b' }], SHARED_END); - expect(flowBetween(shared.doc, 'task_a', 'endEvent_done')).toBe('flow_aToDone'); - expect([flowIds(shared.cs.created), flowIds(shared.cs.removed)]).toEqual([[], ['flow_okToA']]); - expect(shared.warnings.map((warning) => warning.code)).not.toContain('W_ID_SUFFIXED'); - // a gateway first: the next node is appended to it (a new branch), so its out-flow waits for that append - const gateway = await kept([{ op: 'move', ids: ['gateway_ok', 'task_book'], after: 'startEvent_start' }]); - expect(flowBetween(gateway.doc, 'gateway_ok', 'task_book')).toBe('flow_okToBook'); - expect([flowIds(gateway.cs.created), flowIds(gateway.cs.removed)]).toEqual([['flow_okToCheck'], ['flow_bookToDone']]); - // the flow keeps what it had: the name the prepend gave A -> Join, before the join moves after A - const named = await kept([{ op: 'move', ids: ['task_a', 'gateway_join'], before: 'gateway_join', flowName: 'done' }], JOINED); - expect(flowBetween(named.doc, 'task_a', 'gateway_join')).toBe('flow_aToJoin'); - expect(named.doc.get('flow_aToJoin')!.get('name')).toBe('done'); - expect(flowIds(named.cs.created)).toEqual([]); - }); - - it('a moved node that is not bridged puts its out-flow back without the label and condition no bridge took, and says so; between its old ends it keeps them', async () => { - const result = await applyToXml(JOIN_B, [{ op: 'move', ids: ['task_b'], after: 'task_c' }], { keepFlowIds: true, layout: false }); - expect(result.xml).toContain(''); - const dropped = result.result.warnings.added.filter((warning) => warning.code === 'W_LABEL_DROPPED' || warning.code === 'W_CONDITION_DROPPED'); - expect(dropped.map((warning) => `${warning.code} ${warning.element}: ${warning.message}`)).toEqual([ - 'W_LABEL_DROPPED flow_bToC: Flow label "approved" of flow_bToC was dropped while moving task_b: task_b was not bridged, so nothing at its old place took it, and flow_bToC was put back at its new place without it', - 'W_CONDITION_DROPPED flow_bToC: Condition of flow_bToC was dropped while moving task_b: task_b was not bridged, so nothing at its old place took it, and flow_bToC was put back at its new place without it', - ]); - // B before C: flow_bToC is put back between its old ends, with its label and condition - const same = await applyToXml(JOIN_B, [{ op: 'move', ids: ['task_b'], before: 'task_c' }], { keepFlowIds: true, layout: false }); - expect(same.xml).toMatch(/\s*\$\{approved\}<\/bpmn:conditionExpression>/); - expect(same.result.warnings.added.map((warning) => warning.code)).not.toContain('W_LABEL_DROPPED'); - // a bridged node: the bridge takes both (A -> C), nothing is dropped - const bridged = await applyToXml(LINE.replace('id="flow_bToC" sourceRef', 'id="flow_bToC" name="approved" sourceRef'), [{ op: 'move', ids: ['task_b'], after: 'task_c' }], { keepFlowIds: true, layout: false }); - expect(bridged.xml).toContain(''); - expect(bridged.result.warnings.added.map((warning) => warning.code)).not.toContain('W_LABEL_DROPPED'); - // by default the flow is removed and the change names it with its label, as before - const renamed = await applyToXml(JOIN_B, [{ op: 'move', ids: ['task_b'], after: 'task_c' }], { layout: false }); - expect(renamed.result.removed).toContainEqual({ id: 'flow_bToC', kind: 'sequenceFlow', name: 'approved', detail: 'task_b -> task_c' }); - expect(renamed.result.warnings.added.map((warning) => warning.code)).not.toContain('W_LABEL_DROPPED'); - }); - - it('a chain whose first node is not bridged: the flow to the next node, put back elsewhere and then between its old ends again, gets its label back', async () => { - // A's splice puts A -> B back as A -> Done, B's splice after A ends it at B again - const result = await applyToXml(APART, [{ op: 'move', ids: ['task_a', 'task_b'], after: 'task_x' }], { keepFlowIds: true, layout: false }); - expect(result.xml).toMatch(/\s*\$\{approved\}<\/bpmn:conditionExpression>/); - expect(result.result.warnings.added.map((warning) => warning.code)).toEqual(['W_FLOW_ID_STALE']); - expect(result.result.changed.filter((change) => change.id === 'flow_aToB')).toEqual([{ id: 'flow_aToB', kind: 'sequenceFlow', name: 'approved', detail: 'task_a -> task_b' }]); - // its default marker comes back the same way - const marked = APART.replace('', '').replace(//, ''); - const def = await kept([{ op: 'move', ids: ['task_a', 'task_b'], after: 'task_x' }], marked); - expect(def.doc.get('task_a')!.get('default')).toBe(def.doc.get('flow_aToB')); - expect(def.warnings.map((warning) => warning.code)).toEqual(['W_FLOW_ID_STALE']); - }); - - it('a put-back default flow never carries a condition, and a default the placement gave wins over the marker the flow had', async () => { - // B (two flows in, not bridged) before C, its old target: the prepend makes flow_bToC the default flow, so its condition stays dropped - const result = await applyToXml(JOIN_B, [{ op: 'move', ids: ['task_b'], before: 'task_c', default: true }], { keepFlowIds: true, layout: false }); - expect(result.xml).toContain(''); - expect(result.xml).toContain(''); - expect(result.result.warnings.added.filter((warning) => warning.code === 'W_CONDITION_DROPPED').map((warning) => warning.message)).toEqual([ - 'Condition of flow_bToC was dropped while moving task_b: flow_bToC is now the default flow of task_b', - ]); - // the join's default flow comes back to Done, but the prepend made Join -> Archive the default, as asked (as by default) - const archive = JOINED.replace('', '') - .replace('\n ('default')).toBe(wins.doc.get('flow_joinToArchive')); - expect(wins.cs.changed.filter((change) => change.id === 'flow_joinToDone').map((change) => change.detail)).toEqual(['gateway_join -> endEvent_done (no longer the default flow of gateway_join)']); - expect((await applyToXml(archive, ops, { layout: false })).xml).toContain(''); - }); - - it('none for a hashed id, an id that did not name its ends, ends a later op restored, or a flow a later op removed', async () => { - const hashed = await kept([{ op: 'add', kind: 'task', name: 'Pack', after: 'Activity_0k3x9qa' }], MODELER); - expect(stale(hashed.warnings)).toEqual([]); - expect(flowBetween(hashed.doc, 'Activity_0k3x9qa', 'task_pack')).toBe('Flow_1q2w3e4'); - const own = await kept([ - { op: 'connect', source: 'gateway_ok', target: 'endEvent_done', id: 'flow_shortcut', name: 'skip' }, - { op: 'add', kind: 'task', name: 'Log', flow: 'flow_shortcut' }, - ]); - expect(stale(own.warnings)).toEqual([]); - // add, then remove the new node: the bridge puts the flow back between its old ends - const restored = await kept([ - { op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, - { op: 'remove', ids: ['task_review'] }, - ]); - expect(stale(restored.warnings)).toEqual([]); - expect(flowBetween(restored.doc, 'task_check', 'gateway_ok')).toBe('flow_checkToOk'); - // flow_checkToOk goes stale, then goes with task_check; the bridge of that remove makes flow_startToCheck stale - const gone = await kept([ - { op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, - { op: 'remove', ids: ['task_check'] }, - ]); - expect(staleLines(gone.warnings)).toEqual(['flow_startToCheck startEvent_start->task_review => flow_startToReview']); - }); - - it('a flow created earlier in the same change follows its ends; one warning per flow, against its final ends', async () => { - const result = await applyToXml(await drawnDefault(), [ - { op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, - { op: 'add', kind: 'task', name: 'Approve', after: 'task_review' }, - { op: 'add', kind: 'task', name: 'Archive', before: 'task_review' }, - ], { keepFlowIds: true }); - // flow_reviewToOk (made by the first op) was spliced by the second: renamed after its new ends, listed under its final id - expect(result.result.created.map((change) => change.id)).toEqual(['task_review', 'flow_reviewToApprove', 'task_approve', 'flow_approveToOk', 'task_archive', 'flow_archiveToReview']); - expect(result.result.renamed).toBeUndefined(); - // flow_checkToOk was spliced twice (Check -> Review, then Check -> Archive): one warning, its first ends and its last - expect(stale(result.result.warnings.added).map((warning) => warning.message)).toEqual(['Flow flow_checkToOk kept its id, which names its old ends task_check -> gateway_ok; it now runs task_check -> task_archive']); - }); - - it('a flow the same change renames with `set id=` takes that id; a later splice names the ends that id named', async () => { - const chosen = await kept([ - { op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, - { op: 'set', id: 'flow_checkToOk', values: { id: 'flow_toReview' } }, - ]); - expect(stale(chosen.warnings)).toEqual([]); - const spliced = await kept([ - { op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, - { op: 'set', id: 'flow_checkToOk', values: { id: 'flow_checkToReview' } }, - { op: 'add', kind: 'task', name: 'Archive', after: 'task_check' }, - ]); - expect(stale(spliced.warnings).map((warning) => warning.message)).toEqual(['Flow flow_checkToReview kept its id, which names its old ends task_check -> task_review; it now runs task_check -> task_archive']); - }); - - it('the default renames as before and never warns', async () => { - const result = await applyToXml(await drawnDefault(), [{ op: 'add', kind: 'task', name: 'Review', after: 'task_check' }]); - expect(result.result.renamed).toEqual({ flow_checkToOk: 'flow_checkToReview' }); - expect(stale(result.result.warnings.added)).toEqual([]); - const off = await applyToXml(await drawnDefault(), [{ op: 'add', kind: 'task', name: 'Review', after: 'task_check' }], { keepFlowIds: false }); - expect(off.result.renamed).toEqual({ flow_checkToOk: 'flow_checkToReview' }); - }); -}); - -describe('keepFlowIds in the library: the option, Doc.followFlowEnds and the report', () => { - it('an explicit option wins over Doc.followFlowEnds, which is the caller\'s again afterwards (also after a refusal)', async () => { - const doc = await Doc.fromXml(CHAIN); - doc.followFlowEnds = false; - const renamed = await mutateDoc(doc, [{ op: 'add', kind: 'task', name: 'Review', after: 'task_check' }], { keepFlowIds: false, layout: false }); - expect(renamed.changes.renames.map((result) => `${result.from} -> ${result.to}`)).toEqual(['flow_checkToOk -> flow_checkToReview']); - expect(doc.followFlowEnds).toBe(false); - // undefined: the field decides - const keptDoc = await Doc.fromXml(CHAIN); - keptDoc.followFlowEnds = false; - const kept = await mutateDoc(keptDoc, [{ op: 'add', kind: 'task', name: 'Review', after: 'task_check' }], { layout: false }); - expect(stale(kept.changes.warnings).map((warning) => warning.element)).toEqual(['flow_checkToOk']); - const fresh = await Doc.fromXml(CHAIN); - await expect(mutateDoc(fresh, [{ op: 'add', kind: 'task', name: 'Review', after: 'task_nope' }], { keepFlowIds: true, layout: false })).rejects.toMatchObject({ code: 'E_NOT_FOUND' }); - expect(fresh.followFlowEnds).toBe(true); - }); - - it('runOp on its own reports the flow it kept, like a batch', async () => { - const doc = await Doc.fromXml(CHAIN); - doc.followFlowEnds = false; - const cs = runOp(doc, { op: 'remove', ids: ['task_book'] }); - expect(staleLines(cs.warnings)).toEqual(['flow_okToBook gateway_ok->endEvent_done => flow_okToDone']); - }); - - it('a flow the op or an earlier op of the batch created follows its ends, with runOp on its own too', async () => { - const move: EditCommand = { op: 'move', ids: ['task_b', 'task_c'], after: 'startEvent_start' }; - const alone = await Doc.fromXml(TWO_OUT); - alone.followFlowEnds = false; - const single = runOp(alone, move); - // B had two out-flows (not bridged, removed): the move made flow_bToA, then spliced it to B -> C - expect(single.renames.map((result) => `${result.from} -> ${result.to}`)).toEqual(['flow_bToA -> flow_bToC2']); - expect(staleLines(single.warnings).sort()).toEqual(['flow_cToDone task_c->task_a => flow_cToA', 'flow_startToA startEvent_start->task_b => flow_startToB']); - const batch = await kept([move], TWO_OUT); - expect(staleLines(batch.warnings).sort()).toEqual(staleLines(single.warnings).sort()); - // 0.6 kept every id with Doc.followFlowEnds = false; now only the flows the document had - const doc = await Doc.fromXml(CHAIN); - doc.followFlowEnds = false; - const cs = runOps(doc, [ - { op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, - { op: 'add', kind: 'task', name: 'Approve', after: 'task_review' }, - ]); - expect(cs.renames.map((result) => `${result.from} -> ${result.to}`)).toEqual(['flow_reviewToOk -> flow_reviewToApprove']); - expect(cs.created.map((change) => change.id)).toContain('flow_reviewToApprove'); - }); - - it('only the change reports it (warnings.added, never counted as the file\'s); text gives each stale flow its own line', async () => { - const result = await applyToXml(await drawnDefault(), [ - { op: 'add', kind: 'task', name: 'Review', after: 'task_check' }, - { op: 'add', kind: 'task', name: 'Prepare', before: 'task_check' }, - { op: 'add', kind: 'task', name: 'Pay', after: 'task_book' }, - ], { keepFlowIds: true }); - expect(staleLines(result.result.warnings.added).sort()).toEqual([ - 'flow_bookToDone task_book->task_pay => flow_bookToPay', - 'flow_checkToOk task_check->task_review => flow_checkToReview', - 'flow_startToCheck startEvent_start->task_prepare => flow_startToPrepare', - ]); - expect(result.result.warnings.preexistingCount).toBe(0); - const text = renderMutation(result.result); - expect(text.match(/^warning W_FLOW_ID_STALE /gm)).toHaveLength(3); - for (const cmd of ['id=flow_bookToPay', 'id=flow_checkToReview', 'id=flow_startToPrepare']) expect(text).toContain(cmd); - // the next, unrelated change does not repeat it - const next = await applyToXml(result.xml, [{ op: 'set', id: 'task_pay', values: { name: 'Pay invoice' } }], { keepFlowIds: true }); - expect(stale(next.result.warnings.added)).toEqual([]); - expect(next.result.warnings.preexistingCount).toBe(0); - }); - - it('a later change that leaves a stale flow where it was does not repeat it, although the flow passes its old ends on the way', async () => { - const first = await applyToXml(LINE, [{ op: 'add', kind: 'task', name: 'X', after: 'task_a' }], { keepFlowIds: true, layout: false }); - expect(staleLines(first.result.warnings.added)).toEqual(['flow_aToB task_a->task_x => flow_aToX']); - // moving X away and back: the bridge runs flow_aToB A -> B (the ends its id names), the splice A -> X again - const away: EditCommand[] = [{ op: 'move', ids: ['task_x'], after: 'task_c' }, { op: 'move', ids: ['task_x'], after: 'task_a' }]; - const again = await applyToXml(first.xml, away, { keepFlowIds: true, layout: false }); - expect(stale(again.result.warnings.added)).toEqual([]); - const doc = await Doc.fromXml(first.xml); - doc.followFlowEnds = false; - expect(stale(runOps(doc, away).warnings)).toEqual([]); - // a later op of the same change that gives it other ends again reports it - const moved = await kept([...away, { op: 'add', kind: 'task', name: 'Y', after: 'task_a' }], first.xml); - expect(staleLines(moved.warnings)).toEqual(['flow_aToB task_a->task_y => flow_aToY']); - }); -}); - -describe('an end renamed or named earlier in the change, and --flow-id on a splice', () => { - /** CHAIN with a Camunda Modeler id for Check (its id says nothing: it is named by its name) and an unnamed Done. */ - const HASHED = CHAIN.replace(/task_check/g, 'Activity_0abc123').replace('', ''); - - /** `old -> new` of the renames by default, and `flow source->target => suggested id` of each W_FLOW_ID_STALE with ids kept. */ - async function bothModes(xml: string, ops: EditCommand[]): Promise<{ renamed: string[]; stale: string[] }> { - const follow = await Doc.fromXml(xml); - const renamed = runOps(follow, ops).renames.map((result) => `${result.from} -> ${result.to}`); - const keep = await kept(ops, xml); - expect(keep.renames).toBe(0); - return { renamed, stale: staleLines(keep.warnings) }; - } - - it('an end `set id=` gave another id earlier in the batch: its flows follow, or with ids kept are reported, by default and with ids kept', async () => { - const renamedThenSpliced = await bothModes(CHAIN, [ - { op: 'set', id: 'task_check', values: { id: 'task_verify' } }, - { op: 'add', kind: 'task', name: 'Log', after: 'task_verify' }, - ]); - expect(renamedThenSpliced).toEqual({ renamed: ['flow_checkToOk -> flow_verifyToLog'], stale: ['flow_checkToOk task_verify->task_log => flow_verifyToLog'] }); - // --flow names the flow by its id, the target end too - expect(await bothModes(CHAIN, [ - { op: 'set', id: 'gateway_ok', values: { id: 'gateway_approved' } }, - { op: 'add', kind: 'task', name: 'Audit', flow: 'flow_checkToOk' }, - ])).toEqual({ renamed: ['flow_checkToOk -> flow_checkToAudit'], stale: ['flow_checkToOk task_check->task_audit => flow_checkToAudit'] }); - // an unnamed end: only the id it had tells (flow_bookToDone, Done renamed Finished); a remove bridges to it - expect(await bothModes(HASHED, [ - { op: 'set', id: 'endEvent_done', values: { id: 'endEvent_finished' } }, - { op: 'add', kind: 'task', name: 'Log', before: 'endEvent_finished' }, - ])).toEqual({ renamed: ['flow_bookToDone -> flow_bookToLog'], stale: ['flow_bookToDone task_book->task_log => flow_bookToLog'] }); - expect(await bothModes(HASHED, [ - { op: 'set', id: 'endEvent_done', values: { id: 'endEvent_finished' } }, - { op: 'remove', ids: ['task_book'] }, - ])).toEqual({ renamed: ['flow_okToBook -> flow_okToFinished'], stale: ['flow_okToBook gateway_ok->endEvent_finished => flow_okToFinished'] }); - }); - - it('an end `set name=` named again (its id says nothing), and `set id=` in a command before (the end keeps its name): the same', async () => { - expect(await bothModes(HASHED, [ - { op: 'set', id: 'Activity_0abc123', values: { name: 'Verify' } }, - { op: 'add', kind: 'task', name: 'Log', after: 'Activity_0abc123' }, - ])).toEqual({ renamed: ['flow_checkToOk -> flow_verifyToLog'], stale: ['flow_checkToOk Activity_0abc123->task_log => flow_verifyToLog'] }); - // two commands: the first renames Check, the second splices after it; Check is still named Check - const first = await applyToXml(await drawnDefault(), [{ op: 'set', id: 'task_check', values: { id: 'task_verify' } }]); - expect(first.xml).toContain(''); - const splice: EditCommand[] = [{ op: 'add', kind: 'task', name: 'Log', after: 'task_verify' }]; - const renamed = await applyToXml(first.xml, splice); - expect(renamed.result.renamed).toEqual({ flow_checkToOk: 'flow_verifyToLog' }); - expect(renamed.xml).toContain(''); - const keptIds = await applyToXml(first.xml, splice, { keepFlowIds: true }); - expect(staleLines(keptIds.result.warnings.added)).toEqual(['flow_checkToOk task_verify->task_log => flow_verifyToLog']); - // the warning's command renames it; nothing is stale afterwards - const followed = await applyToXml(keptIds.xml, [{ op: 'set', id: 'flow_checkToOk', values: { id: 'flow_verifyToLog' } }], { keepFlowIds: true }); - expect(stale(followed.result.warnings.added)).toEqual([]); - }); - - it('a retype between the `set` and the end change (a new element of the new type) keeps what the end was called', async () => { - for (const retype of [{ op: 'retype', id: 'Activity_0abc123', kind: 'userTask' }, { op: 'retype', id: 'Activity_0abc123', kind: 'serviceTask' }] as EditCommand[]) { - expect(await bothModes(HASHED, [ - { op: 'set', id: 'Activity_0abc123', values: { name: 'Verify' } }, - retype, - { op: 'add', kind: 'task', name: 'Log', after: 'Activity_0abc123' }, - ])).toEqual({ renamed: ['flow_checkToOk -> flow_verifyToLog'], stale: ['flow_checkToOk Activity_0abc123->task_log => flow_verifyToLog'] }); - } - }); - - it('a renamed end changes nothing else: flows whose ends stay keep their ids, and a flow whose id names no ends stays', async () => { - // Book is renamed, the splice is after Check: flow_checkToOk follows as always, flow_okToBook and flow_bookToDone keep their ids - const result = await bothModes(CHAIN, [ - { op: 'set', id: 'task_book', values: { id: 'task_post', name: 'Post' } }, - { op: 'add', kind: 'task', name: 'Log', after: 'task_check' }, - ]); - expect(result).toEqual({ renamed: ['flow_checkToOk -> flow_checkToLog'], stale: ['flow_checkToOk task_check->task_log => flow_checkToLog'] }); - const own = await bothModes(CHAIN.replace(/flow_checkToOk/g, 'flow_review'), [ - { op: 'set', id: 'task_check', values: { id: 'task_verify' } }, - { op: 'add', kind: 'task', name: 'Log', after: 'task_verify' }, - ]); - expect(own).toEqual({ renamed: [], stale: [] }); - }); - - it('--flow-id on a splice with ids kept: W_FLOW_ID_STALE suggests it, W_OPTION_IGNORED names no second command, and every hint can be followed', async () => { - for (const placement of [{ flow: 'flow_checkToOk' }, { after: 'task_check' }]) { - const result = await applyToXml(await drawnDefault(), [{ op: 'add', kind: 'task', name: 'Audit', ...placement, flowId: 'flow_intoAudit' }], { keepFlowIds: true }); - const ignored = result.result.warnings.added.filter((warning) => warning.code === 'W_OPTION_IGNORED'); - expect(ignored.map((warning) => warning.hint)).toEqual(['--flow-id only names a flow that add creates (after a gateway / unconnected node, before a join / unconnected node); W_FLOW_ID_STALE names the command that renames flow_checkToOk.']); - expect(staleLines(result.result.warnings.added)).toEqual(['flow_checkToOk task_check->task_audit => flow_intoAudit']); - // every command the warnings name, in order: one per flow, none fails - const commands = result.result.warnings.added.flatMap((warning) => [...(warning.hint ?? '').matchAll(/`bpmn set (\S+) id=(\S+)`/g)]); - expect(commands.map((command) => `${command[1]} -> ${command[2]}`)).toEqual(['flow_checkToOk -> flow_intoAudit']); - const followed = await applyToXml(result.xml, commands.map((command) => ({ op: 'set', id: command[1]!, values: { id: command[2]! } })), { keepFlowIds: true }); - expect(followed.xml).toContain(''); - } - // by default the flow follows its ends and W_OPTION_IGNORED names the one rename, as before - const renamed = await applyToXml(await drawnDefault(), [{ op: 'add', kind: 'task', name: 'Audit', flow: 'flow_checkToOk', flowId: 'flow_intoAudit' }]); - expect(renamed.result.warnings.added.map((warning) => `${warning.code}: ${warning.hint}`)).toEqual([ - 'W_OPTION_IGNORED: --flow-id only names a flow that add creates (after a gateway / unconnected node, before a join / unconnected node); rename the existing one with `bpmn set flow_checkToAudit id=flow_intoAudit`.', - ]); - }); - - it('--flow-id with ids kept: a taken id, a later splice into the same flow or a flow that does not name its ends give the id its ends give, or the old command', async () => { - // taken: the speaking id - const taken = await kept([{ op: 'add', kind: 'task', name: 'Audit', flow: 'flow_checkToOk', flowId: 'flow_startToCheck' }]); - expect(staleLines(taken.warnings)).toEqual(['flow_checkToOk task_check->task_audit => flow_checkToAudit']); - // the flow no longer ends at Audit: Log was put in between, so --flow-id no longer describes it - const later = await kept([ - { op: 'add', kind: 'task', name: 'Audit', flow: 'flow_checkToOk', flowId: 'flow_intoAudit' }, - { op: 'add', kind: 'task', name: 'Log', after: 'task_check' }, - ]); - expect(staleLines(later.warnings)).toEqual(['flow_checkToOk task_check->task_log => flow_checkToLog']); - // a hashed flow keeps its id without going stale: W_OPTION_IGNORED keeps its own command - const hashed = await kept([{ op: 'add', kind: 'task', name: 'Pack', after: 'Activity_0k3x9qa', flowId: 'flow_intoPack' }], MODELER); - expect(hashed.warnings.map((warning) => `${warning.code}: ${warning.hint}`)).toEqual([ - 'W_OPTION_IGNORED: --flow-id only names a flow that add creates (after a gateway / unconnected node, before a join / unconnected node); rename the existing one with `bpmn set Flow_1q2w3e4 id=flow_intoPack`.', - ]); - }); - - it('--flow-id with ids kept where no W_FLOW_ID_STALE comes: W_OPTION_IGNORED names its own command, or none where it could not be followed', async () => { - const hints = (ws: readonly Finding[]): string[] => ws.map((warning) => `${warning.code}: ${warning.hint}`); - const own = (flow: string, id: string): string => - `W_OPTION_IGNORED: --flow-id only names a flow that add creates (after a gateway / unconnected node, before a join / unconnected node); rename the existing one with \`bpmn set ${flow} id=${id}\`.`; - const none = 'W_OPTION_IGNORED: --flow-id only names a flow that add creates (after a gateway / unconnected node, before a join / unconnected node).'; - // the kept flow still names its new ends: a task named Ok before the gateway Ok?, an unnamed gateway before the unnamed Gateway_1 - expect(hints((await kept([{ op: 'add', kind: 'task', name: 'Ok', flow: 'flow_checkToOk', flowId: 'flow_intoOk' }])).warnings)).toEqual([own('flow_checkToOk', 'flow_intoOk')]); - const unnamed = CHAIN.replace(' hint.startsWith('W_OPTION_IGNORED'))).toEqual([none]); - }); -}); - -/** LINE where B -> C has a label and a condition. */ -const NAMED_LINE = LINE.replace( - '', - '${approved}', -); - -/** Start -> T -> Sub (SubStart -> Inner -> SubEnd) -> Done. */ -const WITH_SUB = definitionsXml(` - flow_startToT - flow_startToTflow_tToSub - flow_tToSubflow_subToDone - flow_subStartToInner - flow_subStartToInnerflow_innerToSubEnd - flow_innerToSubEnd - - - - flow_subToDone - - - `); - -/** Start -> A -> T -> Done; a timer on T leads to Remind -> Late. */ -const WITH_TIMER = definitionsXml(` - flow_startToA - flow_startToAflow_aToT - flow_aToTflow_tToDone - flow_timeoutToRemind - flow_timeoutToRemindflow_remindToLate - flow_tToDone - flow_remindToLate - - - - - `); - -/** `: ` of the changed sequence flows. */ -const changedFlows = (cs: Pick): string[] => cs.changed.filter((change) => change.kind === 'sequenceFlow').map((change) => `${change.id}: ${change.detail}`); - -describe('move: a chain anchored on its own nodes, nodes already in place, what the bridge took', () => { - it('a chain whose --after anchor or --flow source is one of its later nodes, or inside one, is refused before anything changes, in both modes', async () => { - const cases: Array<[string, EditCommand, string, string]> = [ - [JOINED, { op: 'move', ids: ['task_a', 'gateway_join', 'endEvent_done'], after: 'gateway_join' }, 'gateway_join cannot anchor the move of task_a, gateway_join, endEvent_done: it is one of the moved nodes', '`bpmn move task_a --after gateway_join`'], - [LINE, { op: 'move', ids: ['task_a', 'task_b'], flow: 'flow_bToC' }, 'flow_bToC cannot anchor the move of task_a, task_b: its source task_b is one of the moved nodes', '`bpmn move task_a --flow flow_bToC`'], - [LINE, { op: 'move', ids: ['task_b', 'task_c'], after: 'task_c' }, 'task_c cannot anchor the move of task_b, task_c: it is one of the moved nodes', '`bpmn move task_b --after task_c`'], - // C alone is already after B: the hint has the reorder instead - [LINE, { op: 'move', ids: ['task_c', 'task_b'], after: 'task_b' }, 'task_b cannot anchor the move of task_c, task_b: it is one of the moved nodes', 'task_c is already after task_b. Pick an anchor outside the moved nodes, or put task_c before task_b: `bpmn move task_c --before task_b`.'], - [LINE, { op: 'move', ids: ['task_a', 'task_b'], after: 'task_b', before: 'task_c' }, 'task_b cannot anchor the move of task_a, task_b: it is one of the moved nodes', '`bpmn move task_a --after task_b --before task_c`'], - [WITH_SUB, { op: 'move', ids: ['task_t', 'subProcess_sub'], after: 'task_inner' }, 'task_inner cannot anchor the move of task_t, subProcess_sub: it is inside subProcess_sub, one of the moved nodes', '`bpmn move task_t --after task_inner`'], - // a --before anchor inside a later node: the chain would be placed inside itself - [WITH_SUB, { op: 'move', ids: ['task_t', 'subProcess_sub'], before: 'task_inner' }, 'task_inner cannot anchor the move of task_t, subProcess_sub: it is inside subProcess_sub, one of the moved nodes', 'placed inside itself. Pick an anchor outside the moved nodes, or move task_t alone: `bpmn move task_t --before task_inner`'], - // a boundary event moves with its host - [WITH_TIMER, { op: 'move', ids: ['task_a', 'task_t'], after: 'event_timeout' }, 'event_timeout cannot anchor the move of task_a, task_t: it is a boundary event of task_t, one of the moved nodes', '`bpmn move task_a --after event_timeout`'], - [WITH_TIMER, { op: 'move', ids: ['task_a', 'task_t'], flow: 'flow_timeoutToRemind' }, 'flow_timeoutToRemind cannot anchor the move of task_a, task_t: its source event_timeout is a boundary event of task_t, one of the moved nodes', '`bpmn move task_a --flow flow_timeoutToRemind`'], - ]; - for (const [xml, op, message, command] of cases) { - for (const keepFlowIds of [true, false]) { - const what = `${JSON.stringify(op)} keep=${keepFlowIds}`; - const err = await applyToXml(xml, [op], { keepFlowIds, layout: false }).then(() => undefined, (error: unknown) => error as { code: string; message: string; details: { hint?: string } }); - expect(err, what).toMatchObject({ code: 'E_INVALID_PLACEMENT', message }); - expect(err!.details.hint, what).toContain(command); - // refused before the first node moved: the document is as it was - const doc = await Doc.fromXml(xml); - doc.followFlowEnds = !keepFlowIds; - const flowsBefore = [...doc.byId().values()].filter((element) => element.$type === 'bpmn:SequenceFlow').map((flow) => `${flow.get('id')} ${flow.get('sourceRef').get('id')}->${flow.get('targetRef').get('id')}`); - expect(() => runOp(doc, op), what).toThrow(message); - expect([...doc.byId().values()].filter((element) => element.$type === 'bpmn:SequenceFlow').map((flow) => `${flow.get('id')} ${flow.get('sourceRef').get('id')}->${flow.get('targetRef').get('id')}`), what).toEqual(flowsBefore); - } - } - // the node itself as the anchor keeps its own message, and so does its own boundary event; a --before anchor in the chain is fine (it ends A -> Join) - await expect(applyToXml(LINE, [{ op: 'move', ids: ['task_b'], after: 'task_b' }], { layout: false })).rejects.toMatchObject({ code: 'E_INVALID_PLACEMENT', message: 'task_b cannot be moved relative to task_b: that is the node itself or one of its children' }); - await expect(applyToXml(WITH_TIMER, [{ op: 'move', ids: ['task_t'], after: 'event_timeout' }], { layout: false })).rejects.toMatchObject({ code: 'E_INVALID_PLACEMENT', message: 'task_t cannot be moved relative to event_timeout: that is one of its boundary events, which move with it' }); - const afterTimer = await kept([{ op: 'move', ids: ['task_a'], after: 'event_timeout' }], WITH_TIMER); - expect([flowBetween(afterTimer.doc, 'event_timeout', 'task_a'), flowBetween(afterTimer.doc, 'task_a', 'task_remind')]).toEqual(['flow_timeoutToRemind', 'flow_aToT']); - const before = await kept([{ op: 'move', ids: ['task_a', 'gateway_join'], before: 'gateway_join' }], JOINED); - expect(flowBetween(before.doc, 'task_a', 'gateway_join')).toBe('flow_aToJoin'); - }); - - it('a move that leaves its nodes where they are changes nothing, in both modes: no write, no warning, the label and condition stay', async () => { - const cases: Array<[EditCommand, string]> = [ - [{ op: 'move', ids: ['task_b'], after: 'task_a' }, 'task_b is already after task_a'], - [{ op: 'move', ids: ['task_b', 'task_c'], after: 'task_a' }, 'task_b, task_c are already after task_a, in this order'], - [{ op: 'move', ids: ['task_b'], before: 'task_c' }, 'task_b is already before task_c'], - [{ op: 'move', ids: ['task_b', 'task_c'], before: 'endEvent_done' }, 'task_b, task_c are already before endEvent_done, in this order'], - [{ op: 'move', ids: ['task_b'], after: 'task_a', before: 'task_c' }, 'task_b is already between task_a and task_c'], - [{ op: 'move', ids: ['task_c', 'endEvent_done'], after: 'task_b' }, 'task_c, endEvent_done are already after task_b, in this order'], - ]; - for (const [op, note] of cases) { - for (const keepFlowIds of [true, false]) { - const what = `${JSON.stringify(op)} keep=${keepFlowIds}`; - const result = await applyToXml(NAMED_LINE, [op], { keepFlowIds, layout: false }); - expect(result.unchanged, what).toBe(true); - expect([result.result.created, result.result.changed, result.result.removed, result.result.renamed], what).toEqual([[], [], [], undefined]); - expect(result.result.warnings.added, what).toEqual([]); - expect(result.result.notes, what).toEqual([note]); - } - } - // a drawn file with its layout: nothing to write either - const drawn = await drawnDefault(); - for (const keepFlowIds of [true, false]) { - const result = await applyToXml(drawn, [{ op: 'move', ids: ['task_check'], after: 'startEvent_start' }], { keepFlowIds }); - expect(result.unchanged).toBe(true); - expect(result.result.notes).toContain('task_check is already after startEvent_start'); - } - // a hand-made drawing keeps every shape: the incremental layout places no node a move left where it was (--in its own scope too) - const hand = readFileSync(join(join(dirname(fileURLToPath(import.meta.url)), '..', '..'), 'fixtures', 'step1', 'far-label.bpmn'), 'utf8'); - for (const op of [{ op: 'move', ids: ['ST', 'SE'], after: 'SS' }, { op: 'move', ids: ['ST'], in: 'SUB' }] satisfies EditCommand[]) { - const result = await applyToXml(hand, [op]); - expect([result.unchanged, result.result.layout?.mode, result.result.layout?.moved], JSON.stringify(op)).toEqual([true, 'incremental', []]); - } - // flow options describe the flow into the first node, as on a splice: only that flow changes - for (const keepFlowIds of [true, false]) { - const result = await applyToXml(NAMED_LINE, [{ op: 'move', ids: ['task_b'], after: 'task_a', flowName: 'start' }], { keepFlowIds, layout: false }); - expect(changedFlows(result.result)).toEqual(['flow_aToB: task_a -> task_b']); - expect(result.xml).toContain(''); - expect(result.xml).toMatch(/\s* { - for (const keepFlowIds of [true, false]) { - const result = await applyToXml(LINE, [{ op: 'move', ids: ['endEvent_done'], after: 'task_c', flowId: 'flow_cToEnd' }], { keepFlowIds, layout: false }); - expect([result.result.created, result.result.removed, result.result.warnings.added], `keep=${keepFlowIds}`).toEqual([[], [], []]); - expect(changedFlows(result.result)).toEqual(['flow_cToEnd: task_c -> endEvent_done (renamed from flow_cToDone)']); - expect(result.result.notes).toEqual(['endEvent_done is already after task_c']); - expect(result.xml).toContain(''); - expect(result.xml).not.toContain('flow_cToDone'); - // with a name: one entry; the id of another element is refused - const named = await applyToXml(NAMED_LINE, [{ op: 'move', ids: ['task_b'], after: 'task_a', flowId: 'flow_start', flowName: 'start' }], { keepFlowIds, layout: false }); - expect(changedFlows(named.result)).toEqual(['flow_start: task_a -> task_b (renamed from flow_aToB)']); - expect(named.xml).toContain(''); - await expect(applyToXml(LINE, [{ op: 'move', ids: ['task_b'], after: 'task_a', flowId: 'flow_bToC' }], { keepFlowIds, layout: false })).rejects.toMatchObject({ code: 'E_DUPLICATE_ID' }); - } - }); - - it('with ids kept, the change of a bridged node\'s put-back out-flow names what the bridge took: its name, condition and annotations', async () => { - const move: EditCommand[] = [{ op: 'move', ids: ['task_b'], after: 'task_c' }]; - const both = await applyToXml(NAMED_LINE, move, { keepFlowIds: true, layout: false }); - expect(changedFlows(both.result)).toEqual([ - 'flow_aToB: task_a -> task_c (bridged task_b)', - 'flow_cToDone: task_c -> task_b (was -> endEvent_done)', - 'flow_bToC: task_b -> endEvent_done (was -> task_c; its name and condition stay with the bridge flow_aToB)', - ]); - expect(both.xml).toMatch(/\s* endEvent_done (was -> task_c; its name and condition stay with the bridge flow_aToB)'); - // an annotation goes with them; a name the bridge already had stays, so only the rest is named - const annotated = NAMED_LINE.replace('note\n \n endEvent_done (was -> task_c; its condition and annotation stay with the bridge flow_aToB)'); - expect(some.result.warnings.added.map((warning) => warning.code)).toContain('W_LABEL_DROPPED'); - // the bridge is the default flow of A: the condition is dropped there (W_CONDITION_DROPPED), so only the name is named - const byDefault = NAMED_LINE.replace('', ''); - const named = await applyToXml(byDefault, move, { keepFlowIds: true, layout: false }); - expect(changedFlows(named.result)).toContain('flow_bToC: task_b -> endEvent_done (was -> task_c; its name stays with the bridge flow_aToB)'); - // put back between its old ends (before the join, a prepend): the clause alone - const join = JOINED.replace(' gateway_join (its name stays with the bridge flow_okToA)'); - // nothing to name: as before; by default the out-flow is removed with its label, as before - expect(changedFlows(await applyToXml(LINE, move, { keepFlowIds: true, layout: false }).then((result) => result.result))).toContain('flow_bToC: task_b -> endEvent_done (was -> task_c)'); - const renamed = await applyToXml(NAMED_LINE, move, { layout: false }); - expect(renamed.result.removed).toContainEqual({ id: 'flow_bToC', kind: 'sequenceFlow', name: 'approved', detail: 'task_b -> task_c' }); - expect(changedFlows(renamed.result).join('\n')).not.toContain('stay'); - }); - - it('with ids kept, the clause is decided once the chain is placed: a bridge a later node removed, renamed or took it back from', async () => { - // A's bridge T -> Done takes "done", then Done's detach (an end event) removes it: flow_aToDone, back between A and Done, gets it back - const branched = definitionsXml(` - flow_startToOk - flow_startToOkflow_okToTflow_okToU - flow_okToTflow_tToA - flow_tToAflow_aToDone - flow_aToDone - flow_okToUflow_uToOther - flow_uToOther - - - - \${done} - - `); - const removed = await applyToXml(branched, [{ op: 'move', ids: ['task_a', 'endEvent_done'], after: 'gateway_ok' }], { keepFlowIds: true, layout: false }); - expect(removed.xml).toMatch(/\s* endEvent_done (bridged task_a)', 'flow_aToDone: task_a -> endEvent_done']); - expect(removed.result.removed.map((change) => change.id)).toEqual(['flow_tToA']); - expect(removed.result.warnings.added.filter((warning) => warning.code.endsWith('_DROPPED'))).toEqual([]); - // T's bridge Ok -> A takes "next", then Ok's detach (a split: not bridged) removes it, and T -> A ends at Ok now: the label is dropped, and said so - const named = branched.replace('id="flow_tToA" sourceRef', 'id="flow_tToA" name="next" sourceRef'); - const lost = await applyToXml(named, [{ op: 'move', ids: ['task_t', 'gateway_ok'], after: 'task_u' }], { keepFlowIds: true, layout: false }); - expect(lost.xml).not.toContain('"next"'); - expect(lost.result.warnings.added.filter((warning) => warning.code === 'W_LABEL_DROPPED').map((warning) => warning.message)).toEqual([ - 'Flow label "next" of flow_tToA was dropped while moving task_t: the bridge flow_okToT took it, and a later node of the move removed flow_okToT', - ]); - expect(changedFlows(lost.result).join('\n')).not.toContain('stay'); - // a bridge a later node renamed (a flow the batch created follows its ends): the clause names the id it has now - const long = NAMED_LINE.replace('flow_cToDone', 'flow_cToD\n flow_cToDflow_dToDone') - .replace('flow_cToDone', 'flow_dToDone') - .replace('', '\n '); - const batch = await applyToXml(long, [{ op: 'add', kind: 'task', name: 'X', after: 'task_a' }, { op: 'move', ids: ['task_b', 'task_c'], after: 'task_d' }], { keepFlowIds: true, layout: false }); - expect(batch.xml).toMatch(//); - expect(changedFlows(batch.result).filter((flowLine) => flowLine.includes('stay'))).toEqual(['flow_bToC: task_b -> task_c (was -> endEvent_done; its name and condition stay with the bridge flow_xToD)']); - // B and A swap places: the second bridge takes the label back to flow_bToC, so nothing is said, and each entry is listed once - const swap = await applyToXml(NAMED_LINE, [{ op: 'move', ids: ['task_b', 'task_a'], after: 'startEvent_start' }], { keepFlowIds: true, layout: false }); - expect(changedFlows(swap.result)).toEqual([ - 'flow_aToB: task_a -> task_c (bridged task_b)', - 'flow_startToA: startEvent_start -> task_b (was -> task_a)', - 'flow_bToC: task_b -> task_a (was -> task_c)', - 'flow_bToC: task_b -> task_c (bridged task_a)', - 'flow_aToB: task_a -> task_c', - ]); - expect(swap.xml).toMatch(/\s* { - /** DI id by element id. */ - const diOf = (xml: string): Map => new Map([...xml.matchAll(/ [match[2]!, match[1]!])); - const RENAMES: EditCommand[] = [ - { op: 'set', id: 'flow_checkToOk', values: { id: 'flow_checkToAudit' } }, - { op: 'set', id: 'task_check', values: { id: 'task_audit' } }, - ]; - - it('on a flow and a task, in the `_di` and `BPMN_` styles, with ids kept too; the result is unchanged', async () => { - for (const style of ['suffix', 'bpmn'] as const) { - for (const keepFlowIds of [false, true]) { - const result = await applyToXml(await drawnDefault(style), RENAMES, { keepFlowIds }); - const di = diOf(result.xml); - expect(di.get('flow_checkToAudit')).toBe(style === 'suffix' ? 'flow_checkToAudit_di' : 'BPMNEdge_flow_checkToAudit'); - expect(di.get('task_audit')).toBe(style === 'suffix' ? 'task_audit_di' : 'BPMNShape_task_audit'); - expect(result.xml).not.toMatch(/task_check|flow_checkToOk/); - expect(result.result.changed.map((change) => `${change.id}: ${change.detail}`)).toEqual(['flow_checkToAudit: renamed from flow_checkToOk', 'task_audit: renamed from task_check']); - } - } - }); - - it('a DI id not derived from the old id stays; one whose new id is taken gets _2', async () => { - const xml = (await drawnDefault()).replace('id="task_check_di"', 'id="Shape_0abc123"').replace('id="task_book_di"', 'id="endEvent_archive_di"'); - const result = await applyToXml(xml, [ - { op: 'set', id: 'task_check', values: { id: 'task_audit' } }, - { op: 'set', id: 'endEvent_done', values: { id: 'endEvent_archive' } }, - ]); - const di = diOf(result.xml); - expect(di.get('task_audit')).toBe('Shape_0abc123'); - expect(di.get('endEvent_archive')).toBe('endEvent_archive_di_2'); - expect(di.get('task_book')).toBe('endEvent_archive_di'); - }); - - it('a derived DI id with a collision suffix (_2) follows too', async () => { - const xml = (await drawnDefault('bpmn')).replace('id="BPMNShape_task_book"', 'id="BPMNShape_task_book_2"').replace('id="BPMNEdge_flow_checkToOk"', 'id="BPMNEdge_flow_checkToOk_3"'); - const result = await applyToXml(xml, RENAMES); - expect(diOf(result.xml).get('task_audit')).toBe('BPMNShape_task_audit'); - expect(diOf(result.xml).get('flow_checkToAudit')).toBe('BPMNEdge_flow_checkToAudit'); - const book = await applyToXml(xml, [{ op: 'set', id: 'task_book', values: { id: 'task_post' } }]); - expect(diOf(book.xml).get('task_post')).toBe('BPMNShape_task_post'); - }); - - it('in a batch, a DI id of an element removed before is free for the rename, as in two commands', async () => { - const ops: EditCommand[] = [ - { op: 'remove', ids: ['task_book'] }, - { op: 'set', id: 'task_check', values: { id: 'task_book' } }, - ]; - for (const style of ['suffix', 'bpmn'] as const) { - for (const layout of [undefined, false] as const) { - const opts = layout === false ? { layout } : {}; - const batch = await applyToXml(await drawnDefault(style), ops, opts); - const first = await applyToXml(await drawnDefault(style), [ops[0]!], opts); - const second = await applyToXml(first.xml, [ops[1]!], opts); - expect(diOf(batch.xml).get('task_book')).toBe(style === 'suffix' ? 'task_book_di' : 'BPMNShape_task_book'); - expect(batch.xml).toBe(second.xml); - } - } - }); - - it('the renamed element\'s DI element is written anew: a comment inside it is dropped, and the result says so', async () => { - const xml = (await drawnDefault()).replace(/()/, '$1\n '); - expect(xml).toContain(''); - for (const layout of [undefined, false] as const) { - const result = await applyToXml(xml, [{ op: 'set', id: 'task_check', values: { id: 'task_audit' } }], layout === false ? { layout } : {}); - expect(diOf(result.xml).get('task_audit')).toBe('task_audit_di'); - expect(result.xml).not.toContain(''); - expect(result.result.notes).toContain('1 XML comment(s) dropped: they were inside or next to elements the change removed or rewrote'); - } - }); - - it('following W_FLOW_ID_STALE with its `bpmn set` gives the file the default rename gives', async () => { - const splice: EditCommand[] = [{ op: 'add', kind: 'task', name: 'Review', flow: 'flow_okToBook' }]; - for (const style of ['suffix', 'bpmn'] as const) { - const renamed = await applyToXml(await drawnDefault(style), splice); - const keptIds = await applyToXml(await drawnDefault(style), splice, { keepFlowIds: true }); - const command = /`bpmn set (\S+) id=(\S+)`/.exec(stale(keptIds.result.warnings.added)[0]!.hint!)!; - const followed = await applyToXml(keptIds.xml, [{ op: 'set', id: command[1]!, values: { id: command[2]! } }], { keepFlowIds: true }); - expect(diOf(followed.xml).get('flow_okToReview')).toBe(style === 'suffix' ? 'flow_okToReview_di' : 'BPMNEdge_flow_okToReview'); - expect(followed.xml).toBe(renamed.xml); - } - }); -}); - -describe('--no-layout with ids kept: a flow whose ends changed loses its stale edge', () => { - /** The flows a drawing has a BPMNEdge for. */ - const drawnFlows = (xml: string): string[] => [...xml.matchAll(/]*bpmnElement="([^"]+)"/g)].map((match) => match[1]!).sort(); - - it('a move: every flow whose ends changed loses its edge and the note names them, like in the default mode; the next write draws them at their new ends', async () => { - const drawn = (await layoutXml(LINE)).xml; - expect(drawnFlows(drawn)).toEqual(['flow_aToB', 'flow_bToC', 'flow_cToDone', 'flow_startToA']); - const result = await applyToXml(drawn, [{ op: 'move', ids: ['task_b'], after: 'task_c' }], { keepFlowIds: true, layout: false }); - expect(drawnFlows(result.xml)).toEqual(['flow_startToA']); - expect(result.result.notes.filter((note) => note.startsWith('removed'))).toEqual(['removed 3 stale diagram element(s), among them the edge(s) of flow_aToB, flow_bToC, flow_cToDone (their ends changed; the next model edit without --no-layout draws them, or `bpmn route `)']); - expect(result.result.layout?.metrics?.added.filter((problem) => problem.kind === 'missing').map((problem) => problem.ids[0]).sort()).toEqual(['flow_aToB', 'flow_bToC', 'flow_cToDone']); - // the default mode: the bridge and the flow into B lose theirs, the removed flow's edge goes with it - const followed = await applyToXml(drawn, [{ op: 'move', ids: ['task_b'], after: 'task_c' }], { layout: false }); - expect(drawnFlows(followed.xml)).toEqual(['flow_startToA']); - expect(followed.result.notes.filter((note) => note.startsWith('removed'))).toEqual(['removed 3 stale diagram element(s), among them the edge(s) of flow_aToC, flow_cToB (their ends changed; the next model edit without --no-layout draws them, or `bpmn route `)']); - const next = await applyToXml(result.xml, [{ op: 'set', id: 'task_a', values: { name: 'A2' } }], { keepFlowIds: true }); - expect(next.result.layout?.mode).toBe('incremental'); - expect([...(next.result.layout?.placed ?? [])].sort()).toEqual(['flow_aToB', 'flow_bToC', 'flow_cToDone']); - expect(next.result.layout?.metrics?.after.counts.missing).toBe(0); - }); -}); diff --git a/test/domain/validation/conventions-declared.test.ts b/test/domain/validation/conventions-declared.test.ts index d41cb02b..1a0744a7 100644 --- a/test/domain/validation/conventions-declared.test.ts +++ b/test/domain/validation/conventions-declared.test.ts @@ -25,8 +25,8 @@ const REQUIRED = declared({ eventDefinitionQualifier: 'required' }); const createdIds = (mutation: MutationResult): string[] => mutation.changes.created.map((created) => created.id); /** `ops` on `xml` with the declared conventions (null: none). */ -async function run(xml: string, ops: unknown[], conventions: IdConventions | null = BARE, opts: { keepFlowIds?: boolean } = {}): Promise { - return mutateDoc(await Doc.fromXml(xml, 'x.bpmn'), ops as EditCommand[], { layout: false, ...(conventions ? { conventions } : {}), ...opts }); +async function run(xml: string, ops: unknown[], conventions: IdConventions | null = BARE): Promise { + return mutateDoc(await Doc.fromXml(xml, 'x.bpmn'), ops as EditCommand[], { layout: false, ...(conventions ? { conventions } : {}) }); } /** A PascalCase file: start -> two tasks -> end, two lanes, a data object. */ @@ -318,16 +318,12 @@ describe('new ids take the declared prefix and case', () => { expect(result.xml).toContain(' { + it('a flow of the declared form still names its ends and is renamed after its new ones; a flow of another form keeps its id', async () => { const ops = [{ op: 'add', kind: 'task', name: 'Pack order', flow: 'flow_checkOrderToShipOrder' }]; const renamed = await run(CONFORMING, ops); expect(renamed.changes.renames.map((rename) => `${rename.from} -> ${rename.to}`)).toEqual(['flow_checkOrderToShipOrder -> flow_checkOrderToPackOrder']); - const kept = await run(CONFORMING, ops, BARE, { keepFlowIds: true }); - expect(kept.changes.renames).toEqual([]); - expect(kept.changes.warnings.find((warning) => warning.code === 'W_FLOW_ID_STALE')?.hint).toContain('`bpmn set flow_checkOrderToShipOrder id=flow_checkOrderToPackOrder`'); const other = await run(PASCAL, [{ op: 'add', kind: 'task', name: 'Pack order', flow: 'Flow_CheckOrderToShipOrder' }]); expect(other.changes.renames).toEqual([]); - expect(other.changes.warnings.filter((warning) => warning.code === 'W_FLOW_ID_STALE')).toEqual([]); }); it('a host passes its own table (no plugin); unusable prefixes are left out; a type without one takes its type name', async () => { diff --git a/test/e2e/cli-commands.test.ts b/test/e2e/cli-commands.test.ts index 7dad448e..41185f84 100644 --- a/test/e2e/cli-commands.test.ts +++ b/test/e2e/cli-commands.test.ts @@ -58,12 +58,4 @@ describe('Cli in-process', () => { expect(JSON.parse(json.err)).toMatchObject({ ok: false, error: { code: 'E_USAGE' } }); }); - it('reads the environment it is given, not the process environment', async () => { - const io = { env: { BPMN_KEEP_FLOW_IDS: 'maybe' } }; - let err = ''; - const cli = new Cli('9.9.9', FILE_USE_CASES, { stdout: { write: () => true }, stderr: { write: (chunk: string | Uint8Array) => ((err += String(chunk)), true) }, stdin: Readable.from([]), ...io }); - await cli.run(['node', 'bpmn', 'add', join(dir, 'order.bpmn'), 'userTask', 'K']); - expect(cli.exitCode).toBe(1); - expect(err).toContain('BPMN_KEEP_FLOW_IDS=maybe is not a switch value'); - }); }); diff --git a/test/e2e/cli.test.ts b/test/e2e/cli.test.ts index 57a7096d..71189e62 100644 --- a/test/e2e/cli.test.ts +++ b/test/e2e/cli.test.ts @@ -21,12 +21,7 @@ interface Run { } function bpmn(...args: string[]): Run { - return bpmnWith({}, ...args); -} - -/** bpmn with these environment variables on top of the test's (test/helpers/setup.ts cleared the shell's BPMN_KEEP_FLOW_IDS). */ -function bpmnWith(env: Record, ...args: string[]): Run { - const spawned = spawnSync(process.execPath, [BIN, ...args], { encoding: 'utf8', cwd: dir, input: '', env: { ...process.env, ...env } }); + const spawned = spawnSync(process.execPath, [BIN, ...args], { encoding: 'utf8', cwd: dir, input: '' }); return { code: spawned.status ?? -1, out: spawned.stdout, @@ -36,11 +31,7 @@ function bpmnWith(env: Record, ...args: string[]): Run { } function ok(...args: string[]): Run { - return okWith({}, ...args); -} - -function okWith(env: Record, ...args: string[]): Run { - const result = bpmnWith(env, ...args); + const result = bpmn(...args); if (result.code !== 0) throw new Error(`bpmn ${args.join(' ')} failed (${result.code}):\n${result.err}\n${result.out}`); return result; } @@ -456,177 +447,3 @@ describe('bpmn cli contracts', () => { expect(readdirSync(dir).filter((fileName) => fileName.endsWith('.tmp'))).toEqual([]); }); }); - -describe('flow ids kept (--keep-flow-ids, BPMN_KEEP_FLOW_IDS)', () => { - let base: string; - const splice = ['serviceTask', 'Book invoice', '--after', 'userTask_checkInvoice']; - const STALE = 'warning W_FLOW_ID_STALE flow_checkInvoiceToInvoiceDone: Flow flow_checkInvoiceToInvoiceDone kept its id, which names its old ends userTask_checkInvoice -> endEvent_invoiceDone; it now runs userTask_checkInvoice -> serviceTask_bookInvoice (Code and tests that use the id keep working; to rename it together with them: `bpmn set flow_checkInvoiceToInvoiceDone id=flow_checkInvoiceToBookInvoice`.)'; - - /** A copy of start -> Check invoice -> end. */ - function fresh(name: string): string { - const copy = join(dir, name); - writeFileSync(copy, readFileSync(base, 'utf8')); - return copy; - } - - beforeAll(() => { - base = join(dir, 'invoice.bpmn'); - ok('new', base, '--name', 'Invoice'); - ok('add', base, 'startEvent', 'Invoice received'); - ok('add', base, 'userTask', 'Check invoice', '--after', 'startEvent_invoiceReceived'); - ok('add', base, 'endEvent', 'Invoice done', '--after', 'userTask_checkInvoice'); - }); - - it('--keep-flow-ids keeps the id and names the stale flow with the command that renames it; the command works', () => { - const modelFile = fresh('keep-flag.bpmn'); - const result = ok('add', modelFile, ...splice, '--keep-flow-ids'); - expect(result.out).toContain(`${STALE}\n`); - expect(result.out).not.toMatch(/renamed/); - expect(readFileSync(modelFile, 'utf8')).toContain(''); - const command = /`bpmn set (\S+) (id=\S+)`/.exec(result.out)!; - expect(ok('set', modelFile, command[1]!, command[2]!).out).toMatch(/^changed sequenceFlow flow_checkInvoiceToBookInvoice /m); - }); - - it('BPMN_KEEP_FLOW_IDS=1 makes it the default; --no-keep-flow-ids, 0 and an empty value rename', () => { - let result = okWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'add', fresh('env-on.bpmn'), ...splice, '--summary'); - expect(result.out).toMatch(/^warning W_FLOW_ID_STALE flow_checkInvoiceToInvoiceDone: /m); - expect(result.out).not.toMatch(/^renamed:/m); - for (const [env, flag] of [['1', '--no-keep-flow-ids'], ['0', undefined], ['Off', undefined], ['', undefined]] as const) { - result = okWith({ BPMN_KEEP_FLOW_IDS: env }, 'add', fresh('env-off.bpmn'), ...splice, '--summary', ...(flag ? [flag] : [])); - expect(result.out, `${env} ${flag ?? ''}`).toMatch(/^renamed: flow_checkInvoiceToInvoiceDone -> flow_checkInvoiceToBookInvoice$/m); - expect(result.out).not.toMatch(/W_FLOW_ID_STALE/); - } - result = okWith({ BPMN_KEEP_FLOW_IDS: 'yes' }, 'add', fresh('env-yes.bpmn'), ...splice, '--json'); - expect(result.json()).not.toHaveProperty('renamed'); - expect(result.json().warnings.added).toEqual([expect.objectContaining({ code: 'W_FLOW_ID_STALE', element: 'flow_checkInvoiceToInvoiceDone', related: ['userTask_checkInvoice', 'serviceTask_bookInvoice'] })]); - }); - - /** A copy of start -> Check invoice -> Book invoice -> end whose flows have ids that do not name their ends (`ids`: three). */ - function unnamedFlows(name: string, ids: readonly string[]): string { - const modelFile = fresh(name); - ok('add', modelFile, ...splice); - let xml = readFileSync(modelFile, 'utf8'); - ['flow_invoiceReceivedToCheckInvoice', 'flow_checkInvoiceToBookInvoice', 'flow_bookInvoiceToInvoiceDone'].forEach((id, flowIndex) => (xml = xml.replaceAll(id, ids[flowIndex]!))); - writeFileSync(modelFile, xml); - return modelFile; - } - - it('a result says where BPMN_KEEP_FLOW_IDS kept flow ids and that changed it: text, --summary, --json, --summary --json', () => { - const note = (value: string) => `flow ids kept: BPMN_KEEP_FLOW_IDS=${value} (--no-keep-flow-ids renames flows after their new ends for one command)`; - // a stale flow id: after the change's own notes, before the warnings - let result = okWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'add', fresh('note-text.bpmn'), ...splice); - expect(result.out).toContain(`\nnote: inserted between userTask_checkInvoice and endEvent_invoiceDone\nnote: ${note('1')}\n${STALE}\n`); - // --summary prints no notes of its own: this one follows the change lines - result = okWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'add', fresh('note-summary.bpmn'), ...splice, '--summary'); - expect(result.out).toContain(`\nchanged: flow_checkInvoiceToInvoiceDone\nnote: ${note('1')}\nwarning W_FLOW_ID_STALE flow_checkInvoiceToInvoiceDone: `); - result = okWith({ BPMN_KEEP_FLOW_IDS: 'yes' }, 'add', fresh('note-json.bpmn'), ...splice, '--json'); - expect(result.json().notes).toEqual(['inserted between userTask_checkInvoice and endEvent_invoiceDone', note('yes')]); - const ops = join(dir, 'note-ops.json'); - writeFileSync(ops, JSON.stringify([{ op: 'add', kind: 'serviceTask', name: 'Book invoice', after: 'userTask_checkInvoice' }])); - result = okWith({ BPMN_KEEP_FLOW_IDS: 'On' }, 'apply', fresh('note-summary-json.bpmn'), ops, '--summary', '--json'); - expect(result.json().notes).toEqual([note('On')]); - // a move puts the moved node's out-flow back without any warning (numbered flow ids): only the note says why - const numbered = unnamedFlows('note-numbered.bpmn', ['Flow_1', 'Flow_2', 'Flow_3']); - const move = ['move', numbered, 'userTask_checkInvoice', '--after', 'serviceTask_bookInvoice', '--summary', '--dry-run']; - result = okWith({ BPMN_KEEP_FLOW_IDS: '1' }, ...move); - expect(result.out).toMatch(/^changed: Flow_1, Flow_3, Flow_2, userTask_checkInvoice\nnote: flow ids kept: BPMN_KEEP_FLOW_IDS=1 /m); - expect(result.out).not.toMatch(/created sequenceFlow|warning/); - expect(ok(...move).out).toMatch(/^created sequenceFlow: flow_checkInvoiceToInvoiceDone\n[\s\S]*^removed: Flow_2$/m); - // the same move in a batch - writeFileSync(ops, JSON.stringify([{ op: 'move', ids: ['userTask_checkInvoice'], after: 'serviceTask_bookInvoice' }])); - expect(okWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'apply', numbered, ops, '--json', '--dry-run').json().notes).toContain(note('1')); - }); - - it('no note where a flag decides, the variable is unset or off, or the kept ids changed nothing: the output is the one without the variable', () => { - const modelFile = fresh('note-none.bpmn'); - const numbered = unnamedFlows('note-none-numbered.bpmn', ['Flow_1', 'Flow_2', 'Flow_3']); - // the ids Camunda Modeler gives - const modeler = unnamedFlows('note-none-modeler.bpmn', ['Flow_0k3x9ab', 'Flow_1q7m2zd', 'Flow_0w5n1rt']); - const ops = join(dir, 'note-none-ops.json'); - writeFileSync(ops, JSON.stringify([{ op: 'connect', source: 'startEvent_invoiceReceived', target: 'endEvent_invoiceDone', as: '$f' }, { op: 'set', id: '$f', values: { name: 'skip' } }])); - // a move in place relocates nothing: its flow options change the flow into its node the same in both modes - const inPlace = join(dir, 'note-none-in-place.json'); - const inPlaceNumbered = join(dir, 'note-none-in-place-numbered.json'); - writeFileSync(inPlace, JSON.stringify([{ op: 'move', ids: ['userTask_checkInvoice'], after: 'startEvent_invoiceReceived', flowName: 'go', flowId: 'Flow_ReceivedToCheck' }])); - writeFileSync(inPlaceNumbered, JSON.stringify([{ op: 'move', ids: ['serviceTask_bookInvoice'], after: 'userTask_checkInvoice', flowName: 'ok' }])); - expect(ok('apply', modelFile, inPlace, '--dry-run', '--summary').out).toMatch(/^changed: Flow_ReceivedToCheck$/m); - expect(ok('apply', numbered, inPlaceNumbered, '--dry-run', '--summary').out).toMatch(/^changed: Flow_2$/m); - const cases: string[][] = [ - ['apply', modelFile, inPlace, '--summary'], - ['apply', numbered, inPlaceNumbered, '--json'], - ['add', modelFile, ...splice, '--keep-flow-ids'], - ['add', modelFile, ...splice, '--no-keep-flow-ids', '--summary'], - ['set', modelFile, 'userTask_checkInvoice', 'name=Check bill'], - // a flow the change creates or removes is the same in both modes, also one a batch created and then changed - ['connect', modelFile, 'startEvent_invoiceReceived', 'endEvent_invoiceDone', '--summary'], - ['remove', modelFile, 'flow_checkInvoiceToInvoiceDone', '--json'], - ['apply', modelFile, ops, '--summary'], - // a property of a flow, also the rename W_FLOW_ID_STALE's hint gives - ['set', modelFile, 'flow_checkInvoiceToInvoiceDone', 'name=done', '--summary'], - ['set', modelFile, 'flow_checkInvoiceToInvoiceDone', 'id=flow_checkInvoiceToBookInvoice', '--summary'], - // a flow whose id does not name its ends is renamed in neither mode: a splice, a bridging remove, set target= - ...[numbered, modeler].flatMap((sample) => [ - ['add', sample, 'userTask', 'Approve invoice', '--after', 'userTask_checkInvoice', '--summary'], - ['remove', sample, 'serviceTask_bookInvoice', '--json'], - ['set', sample, sample === numbered ? 'Flow_1' : 'Flow_0k3x9ab', 'target=serviceTask_bookInvoice'], - ]), - ]; - for (const args of cases) { - const plain = ok(...args, '--dry-run').out; - expect(plain, args.join(' ')).not.toMatch(/flow ids kept/); - expect(okWith({ BPMN_KEEP_FLOW_IDS: '1' }, ...args, '--dry-run').out, args.join(' ')).toBe(plain); - } - for (const env of ['0', 'off', '']) { - expect(okWith({ BPMN_KEEP_FLOW_IDS: env }, 'add', modelFile, ...splice, '--dry-run').out, env).toBe(ok('add', modelFile, ...splice, '--dry-run').out); - } - }, 60000); - - it('an E_NOT_FOUND of a batch says that BPMN_KEEP_FLOW_IDS kept the flow ids: the id the default renames to does not exist', () => { - const ops = join(dir, 'note-error-ops.json'); - writeFileSync(ops, JSON.stringify([{ op: 'add', kind: 'serviceTask', name: 'Book invoice', after: 'userTask_checkInvoice' }, { op: 'set', id: 'flow_checkInvoiceToBookInvoice', values: { name: 'ok' } }])); - const modelFile = fresh('note-error.bpmn'); - expect(ok('apply', modelFile, ops, '--dry-run', '--summary').out).toMatch(/^renamed: flow_checkInvoiceToInvoiceDone -> flow_checkInvoiceToBookInvoice$/m); - const kept = 'If it is the id a flow an earlier op gave other ends would be renamed to: flow ids are kept here (BPMN_KEEP_FLOW_IDS=1), so that flow kept its old id; use the old id, or --no-keep-flow-ids to rename it after its new ends.'; - let result = bpmnWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'apply', modelFile, ops, '--dry-run'); - expect(result.code).toBe(2); - expect(result.err).toMatch(/^error E_NOT_FOUND: No element with id "flow_checkInvoiceToBookInvoice" \(did you mean: invoice\?\)\n/); - expect(result.err).toContain(` to list ids. ${kept}\n`); - result = bpmnWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'apply', modelFile, ops, '--dry-run', '--json'); - expect(result.json().error.hint.endsWith(` to list ids. ${kept}`)).toBe(true); - // a flag decides: the error as without the hint - const flag = bpmnWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'apply', modelFile, ops, '--dry-run', '--keep-flow-ids'); - expect(flag.err).not.toContain('BPMN_KEEP_FLOW_IDS'); - expect(bpmn('apply', modelFile, ops, '--dry-run', '--keep-flow-ids').err).toBe(flag.err); - // one op: the ids it names are the file's, whatever the mode - expect(bpmnWith({ BPMN_KEEP_FLOW_IDS: '1' }, 'set', modelFile, 'Flow_Nowhere', 'name=x').err).not.toContain('BPMN_KEEP_FLOW_IDS'); - }); - - it('apply, remove and set target= keep ids too; --strict exits 5 on the warning', () => { - const ops = join(dir, 'keep-ops.json'); - writeFileSync(ops, JSON.stringify([{ op: 'add', kind: 'serviceTask', name: 'Book invoice', after: 'userTask_checkInvoice' }])); - expect(ok('apply', fresh('keep-apply.bpmn'), ops, '--keep-flow-ids').out).toContain(STALE); - const bridged = ok('remove', fresh('keep-remove.bpmn'), 'userTask_checkInvoice', '--keep-flow-ids').out; - expect(bridged).toMatch(/^warning W_FLOW_ID_STALE flow_invoiceReceivedToCheckInvoice: .*`bpmn set flow_invoiceReceivedToCheckInvoice id=flow_invoiceReceivedToInvoiceDone`/m); - const retargeted = ok('set', fresh('keep-set.bpmn'), 'flow_invoiceReceivedToCheckInvoice', 'target=endEvent_invoiceDone', '--keep-flow-ids').out; - expect(retargeted).toMatch(/^warning W_FLOW_ID_STALE flow_invoiceReceivedToCheckInvoice: /m); - const strict = bpmn('add', fresh('keep-strict.bpmn'), ...splice, '--keep-flow-ids', '--strict'); - expect(strict.code).toBe(5); - expect(strict.out).toContain(STALE); - }); - - it('another BPMN_KEEP_FLOW_IDS value is a usage error of a mutating command (unless a flag decides); show and validate ignore it', () => { - const modelFile = fresh('env-bad.bpmn'); - const before = readFileSync(modelFile, 'utf8'); - let result = bpmnWith({ BPMN_KEEP_FLOW_IDS: 'maybe' }, 'add', modelFile, ...splice); - expect(result.code).toBe(1); - expect(result.err).toMatch(/^error E_USAGE: BPMN_KEEP_FLOW_IDS=maybe is not a switch value$/m); - expect(result.err).toMatch(/^ {2}hint: Set BPMN_KEEP_FLOW_IDS=1 \(true, yes, on\) to keep flow ids/m); - result = bpmnWith({ BPMN_KEEP_FLOW_IDS: 'maybe' }, 'add', modelFile, ...splice, '--json'); - expect(result.code).toBe(1); - expect(JSON.parse(result.err).error.code).toBe('E_USAGE'); - expect(readFileSync(modelFile, 'utf8')).toBe(before); - expect(bpmnWith({ BPMN_KEEP_FLOW_IDS: 'maybe' }, 'add', modelFile, ...splice, '--keep-flow-ids', '--dry-run').code).toBe(0); - expect(bpmnWith({ BPMN_KEEP_FLOW_IDS: 'maybe' }, 'show', modelFile).code).toBe(0); - expect(bpmnWith({ BPMN_KEEP_FLOW_IDS: 'maybe' }, 'validate', modelFile).code).toBe(0); - }); -}); diff --git a/test/e2e/isomorphic.test.ts b/test/e2e/isomorphic.test.ts index 643d7b28..bb259964 100644 --- a/test/e2e/isomorphic.test.ts +++ b/test/e2e/isomorphic.test.ts @@ -103,19 +103,6 @@ describe('the browser bundle in a context without Node globals', () => { expect(lines.some((line) => line.startsWith('[route] '))).toBe(true); }); - it('keeps flow ids (keepFlowIds) and reports the stale one like Node', async () => { - const ops = [ - { op: 'add', kind: 'start', name: 'Received' }, - { op: 'add', kind: 'task', name: 'Check', after: 'startEvent_received' }, - { op: 'add', kind: 'end', name: 'Done', after: 'task_check' }, - ]; - const xml = (await core.applyToXml((await core.newXml({ processName: 'Order' })).xml, ops)).xml; - const splice = [{ op: 'add', kind: 'task', name: 'Book', after: 'task_check' }]; - const there = await browser.applyToXml(xml, splice, { keepFlowIds: true }); - expect(there.result.warnings.added.map((warning) => `${warning.code} ${warning.element}`)).toEqual(['W_FLOW_ID_STALE flow_checkToDone']); - expect(JSON.stringify(there.result)).toBe(JSON.stringify((await core.applyToXml(xml, splice, { keepFlowIds: true })).result)); - }); - it('redraws and formats', async () => { const xml = fixture('orders.bpmn'); for (const opts of [{}, { tidy: true }]) { diff --git a/test/helpers/setup.ts b/test/helpers/setup.ts deleted file mode 100644 index 5451e81a..00000000 --- a/test/helpers/setup.ts +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Runs before every test file: the switches of a developer's or an agent's shell that change what an edit - * does are cleared, so the tests and every CLI they spawn (which inherit this environment) see the defaults. - * A test that wants one sets it for its own spawn. - */ -delete process.env['BPMN_KEEP_FLOW_IDS']; diff --git a/tools/fuzz/lib/setup.mjs b/tools/fuzz/lib/setup.mjs index 5acbe5d4..21e1aeba 100644 --- a/tools/fuzz/lib/setup.mjs +++ b/tools/fuzz/lib/setup.mjs @@ -11,9 +11,7 @@ * the PNG of `bpmn render --problems --tiles` (the walk's render oracle). * - `makeExecutor({ exec, via, workDir, bin })`: exec 'cli' (default, in * CLI_ENV) or 'lib' (dist/index.js in-process). - * - `CLI_ENV`: the environment the tools run the CLI in: this one without - * the switches that change what an edit does (BPMN_KEEP_FLOW_IDS), so a - * shell that sets one does not change the results (also the benchmark, + * - `CLI_ENV`: the environment the tools run the CLI in (also the benchmark, * tools/layout-regress.mjs and the demo). * - `corpusFiles(dirs, filter)`: every *.bpmn below the given directories, * sorted, de-duplicated. `defaultCorpus()`: test/fixtures and tools/scenarios @@ -31,7 +29,7 @@ import { hardKinds, newKinds } from './metric-kinds.mjs'; export const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); export const DEFAULT_BIN = join(ROOT, 'bin', 'bpmn.js'); -export const CLI_ENV = Object.fromEntries(Object.entries(process.env).filter(([name]) => name !== 'BPMN_KEEP_FLOW_IDS')); +export const CLI_ENV = process.env; const DIST = join(ROOT, 'dist'); async function importDist(rel) { diff --git a/vitest.config.ts b/vitest.config.ts index c9f654d9..f10ec7df 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -3,8 +3,6 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { include: ['test/**/*.test.ts'], - // clears BPMN_KEEP_FLOW_IDS of the shell for the tests and the CLIs they spawn - setupFiles: ['test/helpers/setup.ts'], // no .bpmnlintrc above the temporary directories or the repository (test/helpers/global-setup.ts) globalSetup: ['test/helpers/global-setup.ts'], testTimeout: 20000,