diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 45c0a75a0..c2eb5cd01 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -178,6 +178,16 @@ to the symptom table below, so the next similar issue costs fewer reads. | Same nightly pattern, **CE6083** this time: `TestMxCheck_DoctypeScripts/15b-fragment-slots-examples` fails only on **10.24** (both engines) — `[CE6083] "Design property Card style is not supported by your theme"` at every `cardWrap` container. `Card style` is an **Atlas v3 design property (11.x); the 10.x Atlas theme doesn't define it** | The shared `define fragment Card` used `designproperties: ['Card style': on]` and is instantiated by every page in the file, so a `-- @version:` gate would have to gate the whole file (killing 10.24 coverage of the slot feature the example is actually about). Unlike 15c's building block, the design property was **incidental** to the example | `mdl-examples/doctype-tests/15b-fragment-slots-examples.mdl` | **Drop the incidental v3 design property**, keep `class: 'card'` (Atlas card styling works on every version) — the example demonstrates content *slots*, not design properties (those live in 12-styling, gated 11.0+). Verified: 15b passes on 10.24 **and** 11.6.3 (0 errors, both engines). **Gate vs remove rule**: if the version-specific construct IS the point of a self-contained section → `-- @version:` gate it (15c); if it's incidental and in a shared/expanded definition → remove it and use a cross-version equivalent (15b). **Proactive sweep** after any such fix: `grep -lE 'designproperties|use building block' mdl-examples/doctype-tests/*.mdl` and confirm each usage is either gated or version-safe (note the doctype test skips `*.test.mdl`/`*.tests.mdl`) | | `mxcli run --local`: when a page action throws, the browser shows the generic Mendix error dialog and there is nothing to correlate it against — the runtime's own stdout/stderr (server stack trace, microflow `LOG` output) is swallowed, so a server-side bug can't be told apart from a client one | The runtime JVM was spawned with `cmd.Stdout=log; cmd.Stderr=log` where `log` is an in-memory `syncBuffer` surfaced only on a *startup* failure; during normal operation it goes nowhere on disk | `cmd/mxcli/docker/localboot.go` (`spawnAndConfigure`, `openRuntimeLog`, `LocalRuntime.logFile`, `LocalRuntimeOptions.RuntimeLogPath`) + `cmd/mxcli/docker/runlocal.go` (default `/.mxcli/runtime.log`) + `cmd/mxcli/cmd_run.go` (`--runtime-log`) | Tee the JVM's stdout+stderr to `/.mxcli/runtime.log` via `io.MultiWriter(log, file)` (the in-memory buffer still backs startup-error reporting). Append across restarts with a `=== runtime start … ===` marker; close the handle on Stop/reopen. Default on; `--runtime-log ` relocates, `-` disables. Print the path at boot. Test `TestOpenRuntimeLog`. Findings #25 | | Follow-up to the above (#25 re-test): `run --local` writes `runtime.log` but it stays **nearly empty** — the JVM tee captures startup/JVM output only; **application** logs (microflow `LOG`, server-side exception stack traces) never reach stdout, so a page-action error still can't be diagnosed | A standalone runtime (launched via `runtimelauncher.jar`) attaches **no log subscriber** by default — unlike a Studio Pro / m2ee run, which calls `create_log_subscriber` **after** start. Mendix application logs flow to log *subscribers*, not stdout, so with none attached they go nowhere | `cmd/mxcli/docker/runtime_controller.go` (`RuntimeController.LogSubscriberFile`/`Stdout`, `attachFileLogSubscriber`, called at the end of `Start`) + `cmd/mxcli/docker/localboot.go` (`StartLocalRuntime` sets `ctrl.LogSubscriberFile` to the abs runtime-log path) | After a successful `start` (and on every restart's `Start`, since each fresh JVM has no subscriber), call the `create_log_subscriber` admin action with `{type:"file", name:"mxcli-run-local", autosubscribe:"INFO", filename:, max_size:1GiB, max_rotate:0}`. **`max_rotate:0` is load-bearing**: the JVM stdout tee holds an fd on the same file, and a rotate-rename would detach it. Best-effort (a logging failure must not fail an up runtime — warn to Stdout instead). Pass an **absolute** path (the runtime's cwd is `/runtime`, not mxcli's). Tests `TestStart_AttachesLogSubscriber`, `TestStart_NoLogSubscriberWhenUnset`, `TestStart_LogSubscriberFailureNonFatal`. Findings #25 (round 2) | +| `create or modify persistent entity` (even a byte-for-byte identical re-run) NULLs every column value on the next runtime DB sync — rows survive, values gone. Reports success (`Modified entity`), `mx check` = 0 errors; the loss only surfaces when something reads the data. Diagnose via the DB-sync count in `runtime.log` ("Executing N database synchronization command(s)") on an unchanged model | `execCreateEntity` minted a FRESH attribute ID for EVERY attribute each run, even unchanged ones. Mendix's DB synchronizer keys off attribute identity — a new ID reads as "attribute departed + new attribute added", so it drops and re-adds the column. Only the entity's own ID was preserved; ALTER ENTITY (the safe path) mutates loaded attributes in place, keeping IDs | `mdl/executor/cmd_entities.go` (`execCreateEntity`) | On CREATE OR MODIFY of an existing entity, build a name→ID map from `existingEntity.Attributes` and reuse the existing ID for retained attributes (only new attributes get a fresh ID); `attrNameToID` propagates the reused ID into validation rules + indexes. Also steer the "already exists in project" hint (`validate_duplicates.go`) toward `alter entity … add attribute` for entities. Test `TestCreateOrModifyEntity_PreservesAttributeIDs`; repro `mdl-examples/bug-tests/create-or-modify-preserves-attribute-ids.mdl`. Findings #13 | +| `alter page P { set class = 'x' on }` (lowercase property) hard-errors on EVERY built-in widget: `property "class" not found (widget has no pluggable Object)` — but `create page` accepts the same lowercase prop, and these aren't pluggable widgets | The MPR page mutator dispatched first-class props via a case-SENSITIVE switch on canonical casing ("Class"/"Caption"/"DynamicClasses"); a lowercase name missed every case and fell through to the pluggable-Object path. `create page` was fine (WidgetV3.GetStringProp is case-insensitive); the sibling MCP mutator already lowercased | `mdl/backend/pagemutator/mutator.go` (`setRawWidgetPropertyMut`) | `switch strings.ToLower(propName)` with lowercase case labels; keep the pluggable fallback (default) on the original casing (template keys are case-sensitive). Test `TestSetWidgetProperty_LowercaseFirstClassProps`; repro `mdl-examples/bug-tests/alter-page-lowercase-set-on-builtin.mdl`. Findings #1 | +| `call javascript action` (e.g. NanoflowCommons.OpenURL) passes `check --references` + `exec` but persists as `-- Empty action`; `mx check` fails CE0008 "No action defined." + CE0109 on the output variable. Affects microflows AND nanoflows | The default `modelsdk` engine's `microflowActionToGen` had no case for `*microflows.JavaScriptActionCallAction` → returned nil → ActionActivity written with no Action. Read path + legacy engine both handled it (hence check/exec looked green); nanoflows reuse the same converter | `mdl/backend/modelsdk/microflow_write.go` (`microflowActionToGen`) | Add the `JavaScriptActionCallAction` case, built directly to mirror the legacy serializer (storage keys JavaScriptAction/OutputVariableName; JS parameter mappings use the `ParameterValue` key, not the Java-style `Value`). Test `TestMicroflowActionToGen_JavaScriptActionCall`; repro `mdl-examples/bug-tests/javascript-action-call-persist.mdl`. Findings #11 | +| A widget EXPRESSION property (`dynamicclasses`/`visibleif`/`editableif`) that walks an association passes `mxcli check --references` but fails `mx check` with CE0117 "Error(s) in expression." Easy to trip: a data binding (`contentparams`) on the SAME widget can traverse the same association legitimately | No check inspected expression-typed widget property values for association steps (MDL-WIDGET04/07/etc. check placeholders/keys, not expression contents). Mendix client-side expressions cannot follow associations — only data bindings can | `mdl/executor/validate_widgets.go` (`validateWidgetExpressionAssociations`, called from `validateStaticWidget`) | New MDL-WIDGET13: for `DynamicClasses`/`VisibleIf`/`EditableIf`, regex `exprAssociationStepRe` (`/Ident.Ident/`) flags a module-qualified step between slashes; a plain attribute (`$obj/Slug`) or enum literal (`Mod.Enum.Val`, no leading slash) doesn't match. Fix for the user: denormalise the attribute onto the bound entity (calculated attribute) or use a data binding. Test `TestValidateWidgetExpressionAssociations`. Findings #4 | +| Checker HINTS send the author the wrong way: MDL001 (nested loop) recommends `retrieve $Match from $List where … limit 1` (a parse error — can't filter a list variable); MDL044 flags `count()` in an expression with "Did you mean 'round()'?" (count is an aggregate, not a typo); MDL044's hint cites `mxcli syntax expressions` (no such topic) | Message-only defects in the linter | `mdl/executor/validate_microflow.go` (MDL001 message ~216; `checkExprFunctions` ~283; `mendixAggregateFuncs`) | MDL001 → recommend `$Match = FIND($List, )` (in-memory O(N); also fix CLAUDE.md idiom #2); MDL044 → for `count/sum/average/minimum/maximum` emit "aggregate activity, not an expression function — assign to a variable first: `$n = count($List);`" instead of a did-you-mean; point the generic hint at `mxcli syntax microflow`. Findings #7/#8/#14c | +| A microflow-CALL output variable reused across a fallback chain (`$S = call M.Inner(...); if … then $S = call M.Inner(...) end if;`) — the natural "try A else try B" — fails the build CE0111 "Duplicate variable name" | Each `$Var = call microflow/create/retrieve …` is a *fresh* variable creation; reusing the name (even inside an if) is a duplicate. `check --references` runs the flowBuilder validation (`validateOutputVariable`) which catches it — bare `check` (no project) does not | `mdl/executor/cmd_microflows_builder_validate.go` (`validateOutputVariable`, `validateScopedStatements`) | Already caught by the shared create-output-var check (same fix as the retrieve/create case); locked in by `TestValidateDuplicateMicroflowCallOutputVar` (same-scope + nested-in-if). Fix for the user: one variable per call, then a plain `set` picks the winner (documented in write-microflows.md). Finding #5 (2nd trigger) | +| `textbox` silently drops `placeholder` and `onchange` (MDL-WIDGET07 warns "unrecognized property … dropped"); after `create page` + `DESCRIBE`, both are gone — the live-search box degrades to a button, the field loses its hint text | The `pages.TextBox` model already had `Placeholder`/`OnChangeAction` fields, and the modelsdk (default) engine's `widgetToGen` already serialized them — but (a) the grammar had no `onchange:`/`placeholder:` property, (b) `buildTextBoxV3` never populated the model, (c) the legacy `serializeTextBox` hardcoded empty, (d) they weren't in `staticWidgetKnownProps` | grammar `mdl/grammar/domains/MDLPage.g4` (`widgetPropertyV3`); visitor `mdl/visitor/visitor_page_v3.go`; `mdl/executor/cmd_pages_builder_v3_widgets.go` (`buildTextBoxV3`); `sdk/mpr/writer_widgets_input.go` (`serializeTextBox`) + `writer_widgets.go` (`serializePlaceholderTemplate`); `validate_widgets.go` known-props | Add `ONCHANGE COLON actionExprV3` + `PLACEHOLDER COLON STRING_LITERAL` (tokens already existed); visitor → `Properties["OnChange"]`/`["Placeholder"]`; `GetOnChange()`/`GetPlaceholder()` accessors; builder sets `tb.Placeholder` (a `*model.Text`) + `tb.OnChangeAction` (via `buildClientActionV3`, reusing the button path); legacy serializer emits `serializePlaceholderTemplate(tb.Placeholder)` + `serializeClientAction(tb.OnChangeAction)`; add `OnChange`/`Placeholder` to `staticWidgetKnownProps`. Tests `TestSerializeTextBox_PlaceholderAndOnChange`; repro `mdl-examples/bug-tests/textbox-placeholder-onchange.mdl`. Finding #9 | +| `call javascript action` / `call java action` with a wrong-cased or misspelled PARAMETER name passes `check --references` (the action itself resolves) but writes a dangling reference that fails the build with CE1613 "The selected … parameter … no longer exists". E.g. `NanoflowCommons.OpenURL(url = …)` when the parameter is `Url` | The reference checker resolved only the action NAME (`buildJava*ActionQualifiedNames` returns names, discarding params); the call's `CallArgument` names were never compared to the action's declared parameters | `mdl/executor/validate.go` (`flowRefCollector` → `codeActionCallRef`, `validateCodeActionParams`) | Carry the call's `argNames` on the collector; after the name-exists check, `ReadJavaScriptActionByName`/`ReadJavaActionByName` → `.Parameters[].Name`, diff case-sensitively; casing-only mismatch → did-you-mean. Skip `System.*` (runtime-provided) and degrade to no-error when the backend reports no params. References-mode only (needs `-p`). Tests `TestValidateCodeActionParams`. RSS-reader follow-up finding | +| `DESCRIBE PAGE` omits a textbox's `placeholder`/`onchange` even though they are written correctly (present in the .mxunit, live app works) — so a DESCRIBE round-trip is not a reliable way to confirm the write landed | The describe read path (`parseRawWidget`) only read `LabelTemplate` + `AttributeRef` for a textbox; `PlaceholderTemplate` and `OnChangeAction` were never read back into `rawWidget` | `mdl/executor/cmd_pages_describe_parse.go` (`extractPlaceholderText`), `cmd_pages_describe.go` (`rawWidget` fields), `cmd_pages_describe_output.go` (`renderClientActionMDL`/`extractOnChangeAction` + TextBox emit) | Add `Placeholder`/`OnChange` to `rawWidget`; read `PlaceholderTemplate` via `extractTextFromTemplate` (same as label) and `OnChangeAction` via a key-agnostic `renderClientActionMDL` (refactored out of `extractButtonAction`, since OnChangeAction is the same client-action type under a different key); emit `Placeholder:`/`OnChange:` in the TextBox case. Single describe path serves both engines (reads raw BSON via `GetRawUnit`). Test `TestParseRawWidget_TextBoxPlaceholderAndOnChange`. RSS-reader follow-up (verification note on #9) | +| `DESCRIBE PAGE` of a `dynamictext` bound to a NON-STRING attribute (Integer/DateTime/…) emits `ContentParams: [{1} = toString($currentObject/Attr)]`; re-applying that output fails the build with CE1613 "attribute '…toString($currentObject/Attr)' no longer exists" — the rendered expression is treated as an attribute NAME. A string binding round-trips fine (bare attribute name) | The write side converts a non-String attribute binding to `toString($currentObject/Attr)` (`resolveTemplateAttributePathFull`), stored as a ClientTemplateParameter `Expression`; the describe reader emitted the Expression verbatim instead of reversing the transform | `mdl/executor/cmd_pages_describe_output.go` (`unwrapToStringAttrParam` in `extractClientTemplateParameters`) | When a ContentParam Expression is exactly `toString($currentObject/)` or `toString($param/)` (the auto-generated forms), emit the bare `` / `$param.attr`; the write side re-derives the toString on the next apply, so it round-trips. Any other expression (extra text, hand-written toString) is left untouched. Tests `TestUnwrapToStringAttrParam`, `TestParseRawWidget_DynamicTextNonStringAttribute`. RSS-reader follow-up finding | | Follow-up to round 2 (#25 re-test): the subscriber is registered but `runtime.log` **still** holds only the 4 JVM-banner lines — a probe microflow's `log info/warning/error` and a forced runtime exception produce **zero** lines. Isolation test: boot+subscriber → 0 probe lines; then call `start_logging` → 6 lines | A standalone runtime boots with logging **not started**, so a registered subscriber sits **inert** — nothing is delivered until the logging subsystem is activated. `create_log_subscriber` alone is necessary but not sufficient | `cmd/mxcli/docker/runtime_controller.go` (`configureRuntimeLogging` — renamed from `attachFileLogSubscriber`; now also calls `start_logging`) | After `create_log_subscriber`, call `CallM2EE(c.opts, "start_logging", nil)` in the same step (order: create subscriber → start_logging). Treat an "already started" response as success (`start` re-runs on the DB-update retry / restart paths on a still-running JVM). Still best-effort (warn to Stdout, never fail an up runtime). Test `TestStart_StartLoggingAlreadyStartedIsSuccess`; `TestStart_AttachesLogSubscriber` now asserts the `[start, create_log_subscriber, start_logging]` sequence. **Diagnosis pattern**: when a registered sink receives nothing, check whether the subsystem that feeds it is even *running* — registration ≠ activation. Findings #25 (round 3) | --- diff --git a/.claude/skills/mendix/README.md b/.claude/skills/mendix/README.md index 55a972413..f15f4eee4 100644 --- a/.claude/skills/mendix/README.md +++ b/.claude/skills/mendix/README.md @@ -66,6 +66,7 @@ Page-specific patterns: | [create-custom-widget.md](create-custom-widget.md) | Custom pluggable widget AIGC | Creating custom React widgets from scratch | | [migrate-design-prototype.md](migrate-design-prototype.md) | Turn a Claude Design prototype into a themed Mendix app | Reproducing a design handoff/prototype as an SCSS theme + styled pages | | [debug-bson.md](debug-bson.md) | BSON debugging | Troubleshooting SDK issues | +| [analyze-runtime.md](analyze-runtime.md) | Analyze runtime behavior — logs, metrics, traces, catalog, and cross-source joins | Profiling a slow page/microflow, finding what hits the DB, correlating cost with model shape | --- @@ -97,6 +98,8 @@ Load skills based on the task: | "Build a pluggable widget" | `create-custom-widget.md` | | "Turn a design prototype/handoff into a Mendix app" | `migrate-design-prototype.md`, `theme-styling.md`, `create-page.md` | | "Build/apply a theme from a design" | `migrate-design-prototype.md`, `theme-styling.md` | +| "Why is this slow / profile the app / what hits the database" | `analyze-runtime.md`, `run-local.md` | +| "Trace / metrics / flame chart / correlate cost with model" | `analyze-runtime.md` | ### For Error Recovery diff --git a/.claude/skills/mendix/analyze-runtime.md b/.claude/skills/mendix/analyze-runtime.md new file mode 100644 index 000000000..b2f2397ea --- /dev/null +++ b/.claude/skills/mendix/analyze-runtime.md @@ -0,0 +1,155 @@ +# Analyze an App's Runtime Behavior — Logs, Metrics, Traces, Catalog + +## Overview + +When you need to understand what an app *actually does* at runtime — why a page is +slow, which microflow dominates cost, what hits the database, whether an error is +network or logic — the signals live in four places. This skill is the procedure for +collecting them and, crucially, **joining them**, because the useful questions cross +sources that no single tool answers alone. + +| Signal | Where | How you get it | +|--------|-------|----------------| +| **Logs** (server stack traces + your `LOG` output) | `/.mxcli/runtime.log` | `mxcli run --local` tees it automatically | +| **Metrics** (throughput, DB counts, sessions, queues) | `/prometheus` on the admin port | `mxcli run --local --metrics` | +| **Traces** (per-microflow / per-activity spans + timings) | console→`runtime.log`, or an OTLP collector | `mxcli run --local --trace` / `--trace-otlp` | +| **Model shape** (activities, complexity, refs, XPath) | `.mxcli/catalog.db` (SQLite) | `mxcli … "refresh catalog full"` then `SELECT … FROM CATALOG.*` | + +## When to Use This Skill + +- A page/microflow is slow and you need to find *where* the time goes. +- You want to know which entities/queries the app actually hits, and how often. +- A server-side error shows only a generic dialog in the browser. +- You're profiling and need a flame chart, or want cost correlated with model shape. + +Prerequisite: run the app with the fast local loop — see `run-local.md`. Everything +below assumes `mxcli run --local` (add the flags noted per signal). + +## 1. Logs — the first stop for errors + +`run --local` writes the runtime log to `/.mxcli/runtime.log` (override +`--runtime-log`, `-` disables). It carries JVM stdout/stderr **and** the application +log — server stack traces, your microflow/nanoflow `LOG` output, and the DB +synchronization counts at startup. + +```bash +mxcli run --local -p app.mpr +tail -f .mxcli/runtime.log +``` + +Gotchas: +- **Nanoflow `LOG` lands under the `Client_Nanoflow` node**, not the node name you + declared — a filter built around microflow node names silently drops it. `LOG DEBUG` + from a nanoflow is dropped server-side (browser console only). See `write-nanoflows.md`. +- A spike in "Executing N database synchronization command(s)" on an *unchanged* model + is a red flag (see the `create or modify` data-loss class of bug). + +## 2. Metrics — throughput and database pressure + +`--metrics` registers a Prometheus registry, served at +`http://127.0.0.1:/prometheus` (loopback). + +```bash +mxcli run --local -p app.mpr --metrics +curl -s http://127.0.0.1:8090/prometheus | grep -E 'connectionbus_|handler_requests|sessions_|taskqueue_' +``` + +Useful families: `connectionbus_{selects,inserts,updates,deletes,transactions}_total` +(database pressure), `handler_requests_total` (throughput), `sessions_*`, +`taskqueue_*` (background work). Merge any extra registry (otlp/influx/statsd) with +`--runtime-setting 'Metrics.Registries=[…]'`. + +## 3. Traces — where the time goes + +`--trace` attaches the bundled OpenTelemetry agent. **Default span filters ship with +it** (`OpenTelemetry._RuntimeSpanFilters`) because unfiltered per-activity tracing is +**~10× slower** and produces ~110k spans for one busy transaction — that's a +flow-*shape* debugging mode, not a timing mode. + +```bash +# console exporter → runtime.log (span names/attrs only) +mxcli run --local -p app.mpr --trace +# flame charts: export to a collector (console can't reconstruct call trees/durations) +mxcli run --local -p app.mpr --trace-otlp http://127.0.0.1:4318 +``` + +- The **console exporter omits start/end timestamps and parent span IDs** — you get + span names + attributes but no call tree and no durations. For real timing/flame + charts use `--trace-otlp ` (implies `--trace`), which sets the OTLP + exporter for you; user-set `OTEL_*` env still wins. +- `--trace-service NAME` sets `OTEL_SERVICE_NAME` (default the `.mpr` name); use + distinct names per app for multi-app correlation. Trace context (W3C `traceparent`) + crosses app boundaries automatically over `rest call`. +- To examine flow *shape* on a small flow, temporarily disable the filters with + `--runtime-setting 'OpenTelemetry._RuntimeSpanFilters=[]'`. + +## 4. Model shape — the catalog + +The catalog is a SQLite database at `.mxcli/catalog.db` describing the model. **Run +`refresh catalog full`** — plain `refresh catalog` (fast mode) leaves the analytic +tables (`CATALOG.ACTIVITIES`, `CATALOG.REFS`, `CATALOG.XPATH_EXPRESSIONS`, +`CATALOG.WIDGETS`) **empty** (a fast-mode query warns "requires refresh catalog full"). + +```bash +mxcli -p app.mpr -c "refresh catalog full" +mxcli -p app.mpr -c "SELECT MicroflowQualifiedName, COUNT(*) activities + FROM CATALOG.ACTIVITIES GROUP BY 1 ORDER BY 2 DESC LIMIT 10" +``` + +`CATALOG.ACTIVITIES.Id` is the model GUID the debugger breaks on (see +`debug-microflows.md`), with the action name and its sequence — a named, ordered +activity list per microflow. See `catalog-search.md` / `graph-analysis.md` for the +richer queries. + +## 5. The app warehouse — join the signals (external DuckDB) + +Each signal alone answers little; the useful questions cross them. Because the catalog +is a plain database and the app's dev data is Postgres, one engine can join model +shape + live data + telemetry with **no ETL**. mxcli does **not** embed DuckDB — this +is a dev-container recipe (dev data, dev telemetry, everything read-only): + +```sql +-- in duckdb, from the project dir, after `refresh catalog full` + a --trace-otlp run +ATTACH '.mxcli/catalog.db' AS cat (TYPE sqlite, READ_ONLY); +ATTACH 'dbname=app host=127.0.0.1 user=mendix' AS app (TYPE postgres, READ_ONLY); +CREATE VIEW spans AS SELECT * FROM read_json_auto('spans.jsonl'); +``` + +Two joins that are impossible in any single source: + +- **Runtime cost × model shape** — span durations per microflow joined to + `cat.activities_data` (count) and complexity. Complexity does *not* predict cost: a + 2-activity flow that delegates can cost more than a 14-activity one. A lint rule + can't see that; this join can. +- **Query time × entity × live rows** — span DB timings joined to `cat` (which entity) + and `app` (row counts). This is how you find that the task-queue poller + (`system$queuedtask`, owned by no microflow) is the app's largest DB consumer — + invisible from any per-microflow view. + +Caveats: keep every attachment **read-only** (never a production DB; even locally make +it explicit), and filter/sample traces first — unfiltered span volume is large. + +## Known gap: logs ↔ traces + +Runtime log lines carry **no trace id**, so joining logs to traces is a fuzzy +timestamp join (worst exactly under concurrency). The OTel agent populates the trace +id in the MDC, but Mendix's log pattern doesn't print it — closing the file-log side +needs an upstream `%X{trace_id}` change, not mxcli. For collector-side correlation, +export traces (and logs) via OTLP with `--trace-otlp` so the backend joins them. + +## Decision guide + +| Question | Reach for | +|----------|-----------| +| "Why did this error?" (server-side) | **Logs** (`runtime.log`) | +| "How much DB / throughput / queue work?" | **Metrics** (`--metrics`) | +| "Where does the time go in this flow?" | **Traces** (`--trace-otlp` for a flame chart) | +| "What's the flow's shape / activity list?" | **Catalog** (`refresh catalog full`) or the debugger | +| "Which cost belongs to which entity/model construct?" | **Warehouse** (catalog × spans × app DB) | + +## See Also + +- `run-local.md` — the local loop these flags hang off (`--metrics` / `--trace` / `--trace-otlp` / `--runtime-setting` reference). +- `debug-microflows.md` — interactive breakpoints/stepping when a trace isn't enough. +- `catalog-search.md`, `graph-analysis.md` — catalog query patterns and dependency analysis. +- `verify-with-oql.md` — query the running app's data directly. diff --git a/.claude/skills/mendix/run-local.md b/.claude/skills/mendix/run-local.md index 474f119fc..8e894da88 100644 --- a/.claude/skills/mendix/run-local.md +++ b/.claude/skills/mendix/run-local.md @@ -122,6 +122,7 @@ Launch `run --local` as the **sole** command in its invocation (don't chain a tr | `--metrics` | off | Register a Prometheus meter registry at boot; the runtime serves metrics at `http://127.0.0.1:/prometheus` | | `--trace` | off | Enable OpenTelemetry tracing (bundled agent, console exporter → the runtime log) with default span filters | | `--trace-service` | `.mpr` name | `OTEL_SERVICE_NAME` under `--trace` | +| `--trace-otlp` | off (console) | Export traces to this OTLP collector endpoint (e.g. `http://127.0.0.1:4318`) instead of the console. Implies `--trace`. Needed for flame charts | | `--runtime-setting Key=Value` | — | Merge an extra runtime setting into the boot config (Value parsed as JSON when possible). Repeatable. | ## Metrics and OpenTelemetry @@ -159,8 +160,19 @@ with metrics/logs exporters off. the microflow-level spans). Override with `--runtime-setting 'OpenTelemetry._RuntimeSpanFilters=[…]'`. -**Export to a collector (OTLP) instead of the console:** set the OTEL env yourself -before running — `--trace` won't override an exporter you've already set: +**Export to a collector (OTLP) instead of the console:** the console exporter +omits start/end timestamps and parent span IDs, so call trees and durations can't +be reconstructed from it — for flame charts, export to an OTLP collector. Pass +`--trace-otlp ` (implies `--trace`); mxcli sets +`OTEL_TRACES_EXPORTER=otlp`, `OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf`, and +`OTEL_EXPORTER_OTLP_ENDPOINT` for you: + +```bash +mxcli run --local -p app.mpr --trace-otlp http://127.0.0.1:4318 +``` + +You can still set the `OTEL_*` env yourself for full control — `--trace` / +`--trace-otlp` never override an exporter you've already set: ```bash export OTEL_TRACES_EXPORTER=otlp OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317 diff --git a/.claude/skills/mendix/write-microflows.md b/.claude/skills/mendix/write-microflows.md index 7c1a4b2ce..3e60908bf 100644 --- a/.claude/skills/mendix/write-microflows.md +++ b/.claude/skills/mendix/write-microflows.md @@ -987,6 +987,25 @@ retrieve $Items from Module.Entity where Active = true; **Note**: `returns type as $Var` in the microflow signature does NOT create an activity variable — it only names the return value. So `$Var = call java action ...` after `returns as $Var` is fine (one creation). +**Fallback chains reuse the same name = CE0111.** Because each `$Var = call microflow …` (or retrieve/create) is a *fresh* variable creation, the natural "try A, else try B" shape is invalid — even when the retry is inside an `if`: + +```mdl +-- WRONG: the second call re-creates $Summary — CE0111 +$Summary = call microflow M.Inner(Tag = 'description'); +if trim($Summary) = '' then + $Summary = call microflow M.Inner(Tag = 'summary'); -- CE0111! +end if; + +-- CORRECT: one variable per call, then a plain `set` picks the winner +$Summary = call microflow M.Inner(Tag = 'description'); +if trim($Summary) = '' then + $SummaryFallback = call microflow M.Inner(Tag = 'summary'); + set $Summary = $SummaryFallback; +end if; +``` + +`mxcli check --references` catches this (CE0111) before the build. + ## Legacy SOAP Web Service Calls `call web service` preserves legacy Mendix SOAP activities. Prefer REST clients @@ -1043,7 +1062,7 @@ After every `send rest request`, Mendix automatically populates `$latestHttpResp ```mdl -- ✅ CORRECT: check $latestHttpResponse $RootResult = send rest request Module.Service.GetData; -if $latestHttpResponse/content != empty then +if $latestHttpResponse/Content != empty then -- Process $RootResult (the mapped entity) end if; @@ -1052,7 +1071,7 @@ if $RootResult != empty then -- ERROR! ``` **Key attributes on `$latestHttpResponse`:** -- `content` (String) — response body as string +- `Content` (String) — response body as string. **Capital `C`** — it is inherited from the parent entity `System.HttpMessage`, so `DESCRIBE ENTITY System.HttpResponse` does not list it. Lowercase `$latestHttpResponse/content` fails CE0117. - `StatusCode` (Integer) — HTTP status code (200, 404, etc.) **Restrictions:** diff --git a/CLAUDE.md b/CLAUDE.md index 5d2b53550..e470abcf6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -491,7 +491,7 @@ Regenerate after modifying `MDLLexer.g4`, `MDLParser.g4`, or any `domains/*.g4` These rules apply whenever generating microflow or nanoflow MDL. Violations are caught by `mxcli check`. 1. **NEVER create empty list variables as loop sources.** If processing imported data, accept the list as a microflow parameter — `declare $Items list of ... = empty` followed by `loop $item in $Items` is always wrong. -2. **NEVER use nested LOOPs for list matching.** Loop over the primary list and use `retrieve $match from $TargetList where key = $item/key limit 1` for O(N) lookup. Nested loops are O(N^2). +2. **NEVER use nested LOOPs for list matching.** Loop over the primary list and use `$match = FIND($TargetList, key = $item/key)` for an O(N) in-memory lookup. A plain `retrieve … where` **cannot** filter a list variable (only a database/association source), so `retrieve $match from $TargetList where …` is a parse error — use `FIND`/`FILTER`. Nested loops are O(N^2). 3. **Use append logic when merging**, not overwrite: `$Existing/Field + '\n' + $New/Field` inside an `if $New/Field != empty` guard. 4. **Read `.claude/skills/patterns-data-processing.md`** for delta merge, batch processing, and list operation patterns. @@ -525,7 +525,7 @@ Full syntax tables for all MDL statements (microflows, pages, security, navigati - Docker build integration (`mxcli docker build`) with PAD patching (Phase 1) - Warm local dev loop (`mxcli run --local [--watch] [--screenshot]`): Docker-free `mxbuild --serve` + standalone runtime, hot `reload_model` for behavioural changes and restart+DDL for structural ones (chosen from the serve build's `restartRequired`). Bundles the browser client (`web/dist/` via mxbuild's rollup runner, which the serve Deploy target skips) so Mendix 11.x apps render in a browser. `--watch` keeps an incremental rollup bundler hot (CHOKIDAR_USEPOLLING for container fs; ~3-4s page re-bundle, skipped for model-only edits) and watches only model source (`.mpr`+`mprcontents/`). `--ensure-db` provisions the local Postgres + app database if missing; `--setup` does the non-blocking prerequisites (cache mxbuild+runtime, ensure DB) and exits — `mxcli init` wires it into a Claude Code SessionStart hook so a fresh/reaped web session self-bootstraps, and `docs-site/src/tools/bootstrap-prompt.md` is the empty-repo seed prompt. `--screenshot` captures a Playwright PNG each change (pixel-perfect page loop), with `--screenshot-url` deep links (repeatable for multi-page sets, one PNG per page) and `--screenshot-user`/`--screenshot-password` form login (session saved as Playwright storage state, reused via `screenshot --load-storage`). See `docs/11-proposals/PROPOSAL_mxcli_dev_warm_loop.md` - External browser preview (`mxcli run --hub ` + `mxcli tunnel-hub`): the app stays local and reverse-tunnels out over a single 443 connection (embedded chisel) to a static relay, so it is reachable in a browser at a public URL — works from egress-only environments (Claude Code web), verified live through the session's MITM egress proxy. `run --hub` implies `--local`, boots the runtime with `ApplicationRootUrl` set to the assigned URL (so the SPA/`originURI` work under the public origin), resolves the control proxy honouring `NO_PROXY`, and retries forever. `mxcli tunnel-hub --domain ` is the **multi-tenant** relay: a registry keyed by prefix/project/solution/branch/worktree (stable URLs on reconnect) fronts many previews at per-subdomain hosts (`[prefix-]project[-branch].`; main collapses to the project) over one 443 with per-subdomain autocert, a registration API (`/api/register|status|deregister|backends`), and a sortable availability overview at `hub./`. Client identity flags: `--hub-prefix`/`--hub-project`/`--hub-solution`/`--hub-branch`/`--hub-worktree` (project + branch auto-detected). Package: `cmd/mxcli/tunnelhub/`. See `docs/11-proposals/PROPOSAL_mxcli_dev_warm_loop.md` (slices 3–4) -- Runtime metrics + settings passthrough (`mxcli run --local --metrics` / `--runtime-setting Key=Value`): `--metrics` registers a Prometheus Micrometer registry at boot (served at `http://127.0.0.1:/prometheus`); `--runtime-setting` merges arbitrary runtime config (e.g. `Metrics.Registries` for otlp/influx/statsd, or `OpenTelemetry._RuntimeSpanFilters`) into mxcli's **single** boot `update_configuration` call — the admin action replaces rather than merges, so folding settings into the one boot call is the only safe way. OTel traces via `--trace` attach the bundled `opentelemetry-javaagent` to the runtime JVM (console exporter → the tee'd runtime log) and ship default `OpenTelemetry._RuntimeSpanFilters` (unfiltered per-activity tracing is ~10× slower); `--trace-service` sets `OTEL_SERVICE_NAME`, and user-set `OTEL_*` env (e.g. an OTLP collector) is respected. See `.claude/skills/mendix/run-local.md` +- Runtime metrics + settings passthrough (`mxcli run --local --metrics` / `--runtime-setting Key=Value`): `--metrics` registers a Prometheus Micrometer registry at boot (served at `http://127.0.0.1:/prometheus`); `--runtime-setting` merges arbitrary runtime config (e.g. `Metrics.Registries` for otlp/influx/statsd, or `OpenTelemetry._RuntimeSpanFilters`) into mxcli's **single** boot `update_configuration` call — the admin action replaces rather than merges, so folding settings into the one boot call is the only safe way. OTel traces via `--trace` attach the bundled `opentelemetry-javaagent` to the runtime JVM (console exporter → the tee'd runtime log) and ship default `OpenTelemetry._RuntimeSpanFilters` (unfiltered per-activity tracing is ~10× slower); `--trace-service` sets `OTEL_SERVICE_NAME`. The console exporter omits timestamps/parent span IDs (no flame charts), so `--trace-otlp ` (implies `--trace`) switches to the OTLP exporter (protocol `http/protobuf`) pointed at a collector; user-set `OTEL_*` env still takes precedence. See `.claude/skills/mendix/run-local.md` - OQL query execution against running runtime (`mxcli oql`) - Microflow/nanoflow debugger (`mxcli debug`): set breakpoints **by name** (activity resolved from the model), inspect paused flows + variables, step over/into/out, continue — against a `run --local` runtime. Two M2EE planes wired behind one command (admin `enable/disable/status`, app `/debugger/` session); `run --local --debug` enables it at boot. **Nanoflows** are auto-detected (uses the `nanoflow_name` breakpoint param; paused nanoflows are merged from `poll_events`, which `get_paused_microflows` omits). Nanoflow `LOG` output is rewritten to the `Client_Nanoflow` node in the runtime log. See `.claude/skills/mendix/debug-microflows.md` and `docs/11-proposals/PROPOSAL_microflow_debugger.md` - Business event services (SHOW/DESCRIBE/CREATE/DROP) diff --git a/cmd/mxcli/cmd_new.go b/cmd/mxcli/cmd_new.go index 2e9c2bbc0..c0e5514c1 100644 --- a/cmd/mxcli/cmd_new.go +++ b/cmd/mxcli/cmd_new.go @@ -136,20 +136,30 @@ Examples: os.Exit(1) } } else { - // Running on Linux — copy ourselves + // Running on Linux — link ourselves into the project. Prefer a hard link: + // it shares the inode (no ~111MB duplicated per project on the same + // filesystem), is a real ELF the devcontainer can exec, and survives the + // original binary being moved/removed (unlike a symlink). Fall back to a + // full copy across filesystems (EXDEV) or when linking isn't possible. self, err := os.Executable() - if err == nil { - selfBytes, err := os.ReadFile(self) - if err == nil { - if err := os.WriteFile(mxcliBinPath, selfBytes, 0755); err != nil { - fmt.Fprintf(os.Stderr, " Warning: could not copy mxcli binary: %v\n", err) + if err != nil { + fmt.Fprintf(os.Stderr, " Warning: could not locate mxcli binary: %v\n", err) + } else if resolved, rerr := filepath.EvalSymlinks(self); rerr == nil { + self = resolved + _ = os.Remove(mxcliBinPath) // os.Link fails if the target already exists + if err := os.Link(self, mxcliBinPath); err == nil { + fmt.Printf(" Linked mxcli to %s (shared inode, no copy)\n", mxcliBinPath) + } else if selfBytes, rerr := os.ReadFile(self); rerr == nil { + if werr := os.WriteFile(mxcliBinPath, selfBytes, 0o755); werr != nil { + fmt.Fprintf(os.Stderr, " Warning: could not copy mxcli binary: %v\n", werr) } else { fmt.Printf(" Copied mxcli to %s\n", mxcliBinPath) } + } else { + fmt.Fprintf(os.Stderr, " Warning: could not read mxcli binary: %v\n", rerr) } - } - if err != nil { - fmt.Fprintf(os.Stderr, " Warning: could not copy mxcli binary: %v\n", err) + } else { + fmt.Fprintf(os.Stderr, " Warning: could not resolve mxcli binary path: %v\n", rerr) } } diff --git a/cmd/mxcli/cmd_run.go b/cmd/mxcli/cmd_run.go index 2dc995351..a7c9823da 100644 --- a/cmd/mxcli/cmd_run.go +++ b/cmd/mxcli/cmd_run.go @@ -54,8 +54,10 @@ runtime behaviour; it is turned back off on shutdown. With --metrics, a Prometheus meter registry is registered at boot and the runtime serves metrics at http://127.0.0.1:/prometheus. With --trace, the bundled OpenTelemetry agent is attached (spans -> the runtime log via the console -exporter) with default span filters (unfiltered tracing is ~10x slower). Use ---runtime-setting Key=Value (repeatable) to merge any other runtime setting into +exporter) with default span filters (unfiltered tracing is ~10x slower). The +console exporter omits timestamps and parent span IDs, so for flame charts pass +--trace-otlp to export to an OTLP collector (e.g. http://127.0.0.1:4318) +instead. Use --runtime-setting Key=Value (repeatable) to merge any other runtime setting into the boot config (the admin config action replaces rather than merges, so mxcli folds these into its single boot call), e.g. a different Metrics.Registries type or custom OpenTelemetry span filters. @@ -120,6 +122,10 @@ Examples: runtimeSettings, _ := cmd.Flags().GetStringArray("runtime-setting") trace, _ := cmd.Flags().GetBool("trace") traceService, _ := cmd.Flags().GetString("trace-service") + traceOTLP, _ := cmd.Flags().GetString("trace-otlp") + if traceOTLP != "" { + trace = true // --trace-otlp implies --trace + } opts := docker.LocalRunOptions{ ProjectPath: projectPath, @@ -148,6 +154,7 @@ Examples: RuntimeSettings: runtimeSettings, Trace: trace, TraceService: traceService, + TraceOTLP: traceOTLP, DB: docker.DBConfig{ Host: dbHost, Name: dbName, @@ -196,5 +203,6 @@ func init() { runCmd.Flags().StringArray("runtime-setting", nil, "Extra runtime setting Key=Value merged into the boot configuration (Value parsed as JSON when possible), e.g. --runtime-setting 'OpenTelemetry._RuntimeSpanFilters=[\"Loop\",\"Gateway\"]'. Repeatable.") runCmd.Flags().Bool("trace", false, "Enable OpenTelemetry tracing: attach the bundled agent (console exporter → the runtime log) and apply default span filters (unfiltered tracing is ~10x slower)") runCmd.Flags().String("trace-service", "", "OTEL_SERVICE_NAME under --trace (default: the .mpr name)") + runCmd.Flags().String("trace-otlp", "", "Export traces to this OTLP collector endpoint (e.g. http://127.0.0.1:4318) instead of the console — needed for flame charts (the console exporter omits timestamps/parent IDs). Implies --trace") rootCmd.AddCommand(runCmd) } diff --git a/cmd/mxcli/docker/localboot.go b/cmd/mxcli/docker/localboot.go index 573a91d07..a11c9d2b1 100644 --- a/cmd/mxcli/docker/localboot.go +++ b/cmd/mxcli/docker/localboot.go @@ -78,6 +78,11 @@ type LocalRuntimeOptions struct { Trace bool // TraceServiceName is OTEL_SERVICE_NAME when Trace is set (default: the app). TraceServiceName string + // TraceOTLPEndpoint, when set, switches the traces exporter from console to + // OTLP and points it at this collector (e.g. http://127.0.0.1:4318) — needed + // for flame charts, since the console exporter omits timestamps/parent IDs. + // Implies Trace. User-set OTEL_* vars still take precedence. + TraceOTLPEndpoint string // DB is the database the runtime connects to. DB DBConfig // ReadyTimeout bounds how long StartLocalRuntime waits for the admin API @@ -162,11 +167,13 @@ func (o *LocalRuntimeOptions) otelAgentJar() (string, error) { // withTraceEnv layers the OpenTelemetry Java-agent + OTEL_* env onto base. The // agent is always attached (via JAVA_TOOL_OPTIONS, appended to any existing -// value); the OTEL_* exporters default to console traces / no metrics+logs but -// are NOT overridden if the caller already set them (so OTLP-to-a-collector via -// the user's own env still works). Traces on the console exporter land in the -// tee'd runtime log. -func withTraceEnv(base []string, agentJar, serviceName string) []string { +// value). The traces exporter defaults to the console (lands in the tee'd +// runtime log) — but when otlpEndpoint is set, it is switched to OTLP and pointed +// at that collector (protocol http/protobuf), which is what flame-chart tools +// need since the console exporter omits timestamps and parent span IDs. Metrics +// and logs exporters default to none. None of these are overridden if the caller +// already set the corresponding OTEL_* var, so a fully hand-rolled env still works. +func withTraceEnv(base []string, agentJar, serviceName, otlpEndpoint string) []string { has := func(key string) bool { for _, e := range base { if strings.HasPrefix(e, key+"=") { @@ -197,7 +204,18 @@ func withTraceEnv(base []string, agentJar, serviceName string) []string { out = append(out, "OTEL_SERVICE_NAME="+serviceName) } if !has("OTEL_TRACES_EXPORTER") { - out = append(out, "OTEL_TRACES_EXPORTER=console") + if otlpEndpoint != "" { + // Export to an OTLP collector so call trees + durations are usable. + out = append(out, "OTEL_TRACES_EXPORTER=otlp") + if !has("OTEL_EXPORTER_OTLP_PROTOCOL") { + out = append(out, "OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf") + } + if !has("OTEL_EXPORTER_OTLP_ENDPOINT") { + out = append(out, "OTEL_EXPORTER_OTLP_ENDPOINT="+otlpEndpoint) + } + } else { + out = append(out, "OTEL_TRACES_EXPORTER=console") + } } if !has("OTEL_METRICS_EXPORTER") { out = append(out, "OTEL_METRICS_EXPORTER=none") @@ -367,7 +385,7 @@ func (rt *LocalRuntime) spawnAndConfigure() error { if err != nil { return fmt.Errorf("--trace: %w", err) } - cmd.Env = withTraceEnv(cmd.Env, jar, rt.opts.TraceServiceName) + cmd.Env = withTraceEnv(cmd.Env, jar, rt.opts.TraceServiceName, rt.opts.TraceOTLPEndpoint) } PrepareMxCommand(cmd) // FreeType LD_PRELOAD workaround, layered on cmd.Env setProcessGroup(cmd) // reap any JVM child on Stop so the port is freed diff --git a/cmd/mxcli/docker/runlocal.go b/cmd/mxcli/docker/runlocal.go index 0ee417fc6..c1d622b9c 100644 --- a/cmd/mxcli/docker/runlocal.go +++ b/cmd/mxcli/docker/runlocal.go @@ -108,6 +108,11 @@ type LocalRunOptions struct { Trace bool // TraceService is OTEL_SERVICE_NAME under --trace (default: the .mpr name). TraceService string + // TraceOTLP, when set, exports traces to this OTLP collector endpoint + // (e.g. http://127.0.0.1:4318) instead of the console — needed for flame + // charts, since the console exporter omits timestamps/parent span IDs. + // Implies Trace. + TraceOTLP string // RuntimeSettings are raw "Key=Value" runtime settings merged into the boot // update_configuration payload (Value is parsed as JSON, else a string), e.g. // 'Metrics.Registries=[{"type":"otlp"}]' or @@ -568,6 +573,7 @@ func RunLocal(opts LocalRunOptions) error { RuntimeSettings: runtimeSettings, Trace: opts.Trace, TraceServiceName: traceService, + TraceOTLPEndpoint: opts.TraceOTLP, Stdout: w, Stderr: stderr, }) @@ -585,10 +591,13 @@ func RunLocal(opts LocalRunOptions) error { fmt.Fprintf(w, "Metrics (Prometheus): http://127.0.0.1:%d/prometheus\n", opts.AdminPort) } if opts.Trace { - if runtimeLog != "" { + switch { + case opts.TraceOTLP != "": + fmt.Fprintf(w, "Tracing enabled (OpenTelemetry, service %q); spans -> OTLP %s\n", traceService, opts.TraceOTLP) + case runtimeLog != "": fmt.Fprintf(w, "Tracing enabled (OpenTelemetry, service %q); spans -> %s\n", traceService, runtimeLog) - } else { - fmt.Fprintf(w, "Tracing enabled (OpenTelemetry, service %q); spans go to the console only (pass --runtime-log to capture them)\n", traceService) + default: + fmt.Fprintf(w, "Tracing enabled (OpenTelemetry, service %q); spans go to the console only (pass --runtime-log to capture them, or --trace-otlp for a collector)\n", traceService) } } diff --git a/cmd/mxcli/docker/runlocal_test.go b/cmd/mxcli/docker/runlocal_test.go index 4d1345138..06eb1cd3b 100644 --- a/cmd/mxcli/docker/runlocal_test.go +++ b/cmd/mxcli/docker/runlocal_test.go @@ -134,7 +134,7 @@ func TestBuildRuntimeSettings(t *testing.T) { func TestWithTraceEnv(t *testing.T) { base := []string{"PATH=/bin", "JAVA_TOOL_OPTIONS=-Xmx512m"} - env := withTraceEnv(base, "/agents/otel.jar", "sudoku") + env := withTraceEnv(base, "/agents/otel.jar", "sudoku", "") get := func(key string) (string, bool) { for _, e := range env { @@ -166,10 +166,32 @@ func TestWithTraceEnv(t *testing.T) { } // A user-provided OTEL_* is respected (not overridden). - env = withTraceEnv([]string{"OTEL_TRACES_EXPORTER=otlp"}, "/agents/otel.jar", "svc") + env = withTraceEnv([]string{"OTEL_TRACES_EXPORTER=otlp"}, "/agents/otel.jar", "svc", "") if v, _ := get2(env, "OTEL_TRACES_EXPORTER"); v != "otlp" { t.Errorf("user OTEL_TRACES_EXPORTER should win, got %q", v) } + + // An OTLP endpoint switches the traces exporter to otlp and wires the + // protocol + endpoint (for flame charts). (sudoku tracing follow-up) + env = withTraceEnv([]string{"PATH=/bin"}, "/agents/otel.jar", "svc", "http://127.0.0.1:4318") + if v, _ := get2(env, "OTEL_TRACES_EXPORTER"); v != "otlp" { + t.Errorf("OTEL_TRACES_EXPORTER = %q, want otlp", v) + } + if v, _ := get2(env, "OTEL_EXPORTER_OTLP_PROTOCOL"); v != "http/protobuf" { + t.Errorf("OTEL_EXPORTER_OTLP_PROTOCOL = %q, want http/protobuf", v) + } + if v, _ := get2(env, "OTEL_EXPORTER_OTLP_ENDPOINT"); v != "http://127.0.0.1:4318" { + t.Errorf("OTEL_EXPORTER_OTLP_ENDPOINT = %q, want the endpoint", v) + } + + // A user-set exporter still wins even when an OTLP endpoint is passed. + env = withTraceEnv([]string{"OTEL_TRACES_EXPORTER=console"}, "/agents/otel.jar", "svc", "http://127.0.0.1:4318") + if v, _ := get2(env, "OTEL_TRACES_EXPORTER"); v != "console" { + t.Errorf("user OTEL_TRACES_EXPORTER should win over --trace-otlp, got %q", v) + } + if _, ok := get2(env, "OTEL_EXPORTER_OTLP_ENDPOINT"); ok { + t.Error("endpoint should not be set when the user pinned a non-otlp exporter") + } } func get2(env []string, key string) (string, bool) { diff --git a/cmd/mxcli/init_claudemd.go b/cmd/mxcli/init_claudemd.go index 7fe87027a..7e486dfff 100644 --- a/cmd/mxcli/init_claudemd.go +++ b/cmd/mxcli/init_claudemd.go @@ -518,10 +518,9 @@ func generateClaudeMD(projectName, mprFile string) string { w("### Microflows - Supported Statements\n\n") w("| Statement | Syntax |\n") w("|-----------|--------|\n") - w("| Variable declaration | " + bt + "DECLARE $Var Type = value;" + bt + " |\n") - w("| Entity declaration | " + bt + "DECLARE $Entity Module.Entity;" + bt + " |\n") - w("| List declaration | " + bt + "DECLARE $List List of Module.Entity = empty;" + bt + " |\n") + w("| Variable declaration (primitive only) | " + bt + "DECLARE $Var Type = value;" + bt + " |\n") w("| Assignment | " + bt + "SET $Var = expression;" + bt + " |\n") + w("| Enum split (CASE) | " + bt + "CASE $Var/Attr AS x WHEN Enum.Value THEN ... END CASE" + bt + " |\n") w("| Create object | " + bt + "$Var = CREATE Module.Entity (Attr = value);" + bt + " |\n") w("| Change object | " + bt + "CHANGE $Entity (Attr = value);" + bt + " |\n") w("| Commit | " + bt + "COMMIT $Entity [WITH EVENTS] [REFRESH];" + bt + " |\n") @@ -547,7 +546,8 @@ func generateClaudeMD(projectName, mprFile string) string { w("### Microflows - NOT Supported (Will Cause Parse Errors)\n\n") w("| Unsupported | Use Instead |\n") w("|-------------|-------------|\n") - w("| " + bt + "CASE ... WHEN ... END CASE" + bt + " | Nested " + bt + "IF ... ELSE ... END IF" + bt + " |\n") + w("| " + bt + "DECLARE $Entity Module.Entity;" + bt + " (MDL043/CE0053) | Get the object from a parameter, a " + bt + "RETRIEVE" + bt + ", or " + bt + "$E = CREATE Module.Entity(...)" + bt + " |\n") + w("| " + bt + "DECLARE $List List of Module.Entity = empty;" + bt + " (MDL040) | Accept the list as a parameter, or " + bt + "RETRIEVE" + bt + " / " + bt + "$L = CREATE LIST OF Module.Entity" + bt + " |\n") w("| " + bt + "TRY ... CATCH" + bt + " | " + bt + "ON ERROR { ... }" + bt + " blocks |\n") w("\n") w("**Notes:**\n") diff --git a/docs-site/src/tools/catalog-use-cases.md b/docs-site/src/tools/catalog-use-cases.md index 5ebceabe0..d989213e7 100644 --- a/docs-site/src/tools/catalog-use-cases.md +++ b/docs-site/src/tools/catalog-use-cases.md @@ -211,3 +211,35 @@ WHERE Name != UPPER(SUBSTR(Name, 1, 1)) || SUBSTR(Name, 2) OR Name LIKE '%_%' OR Name LIKE '% %'; ``` + +## Query the catalog directly (and join it to other sources) + +The catalog **is** a SQLite database — `/.mxcli/catalog.db`, written +next to the `.mpr` and refreshed by `refresh catalog`. You do not need to export +it to JSON (that drags in human-readable formatting, exposes only a subset of the +tables, and goes stale immediately). Attach the file directly with any SQLite- or +DuckDB-compatible tool for ad-hoc analysis beyond what `SELECT … FROM CATALOG.*` +exposes in one query. + +> **Run `refresh catalog full` first for the analytic tables.** Plain +> `refresh catalog` (fast mode) leaves `activities` / `refs` / `xpath_expressions` +> (and `widgets`) **empty** — a fast-mode query against them warns +> *"requires refresh catalog full"*. Full mode populates them (activities carry +> the model GUIDs the debugger uses, one row per activity with its sequence). + +```bash +# from a dev container, after: mxcli -p app.mpr -c "refresh catalog full" +duckdb -c "ATTACH '.mxcli/catalog.db' AS cat (TYPE sqlite, READ_ONLY); + SELECT MicroflowQualifiedName, COUNT(*) AS activities + FROM cat.activities_data GROUP BY 1 ORDER BY 2 DESC LIMIT 10;" +``` + +### Cross-source "app warehouse" + +Because the catalog is a plain database, one engine (e.g. DuckDB — mxcli does not +embed it) can `ATTACH` it alongside the app's dev Postgres **read-only** and a trace +dump, joining model shape + live data + telemetry with no ETL to answer questions no +single source can (runtime cost × model shape, query time × entity × live rows). The +full logging/metrics/tracing/catalog analysis procedure — including this join — is the +**`analyze-runtime`** skill (`mxcli init` installs it into your project's +`.claude/skills/mendix/`). diff --git a/docs-site/src/tools/refresh-catalog.md b/docs-site/src/tools/refresh-catalog.md index 8e886de95..d3bb6e0d2 100644 --- a/docs-site/src/tools/refresh-catalog.md +++ b/docs-site/src/tools/refresh-catalog.md @@ -77,3 +77,9 @@ The catalog is cached in `.mxcli/catalog.db` next to the MPR file. On subsequent - **Before cross-reference navigation** -- Run `REFRESH CATALOG FULL` to enable CALLERS/CALLEES/IMPACT - **After making changes** -- Refresh to update the catalog with new elements - **If results seem stale** -- Use `REFRESH CATALOG FULL FORCE` to force a rebuild + +> **FULL mode populates the analytic tables.** Plain `REFRESH CATALOG` (fast mode) +> leaves `CATALOG.ACTIVITIES`, `CATALOG.REFS`, `CATALOG.XPATH_EXPRESSIONS`, and +> `CATALOG.WIDGETS` **empty** — a fast-mode query against them warns *"requires +> refresh catalog full"*. Run `REFRESH CATALOG FULL` to fill them (activity rows +> carry the model GUIDs, cross-reference edges, and XPath expressions). diff --git a/docs-site/src/tools/run-local.md b/docs-site/src/tools/run-local.md index 49a106cd7..b52fc0c4d 100644 --- a/docs-site/src/tools/run-local.md +++ b/docs-site/src/tools/run-local.md @@ -83,6 +83,7 @@ so structural changes need a restart; behavioural changes do not. | `--metrics` | off | Register a Prometheus meter registry; metrics served at `http://127.0.0.1:/prometheus` | | `--trace` | off | Enable OpenTelemetry tracing (bundled agent → runtime log) with default span filters | | `--trace-service` | `.mpr` name | `OTEL_SERVICE_NAME` under `--trace` | +| `--trace-otlp` | off (console) | Export traces to this OTLP collector endpoint (e.g. `http://127.0.0.1:4318`) instead of the console; implies `--trace` (needed for flame charts) | | `--runtime-setting Key=Value` | — | Merge an extra runtime setting into the boot config (Value parsed as JSON when possible); repeatable | ## Metrics and OpenTelemetry @@ -113,8 +114,18 @@ mxcli run --local -p app.mpr --trace tail -f .mxcli/runtime.log # microflow spans: mx.microflow.name / mx.microflow.depth ``` -To export to a collector instead of the console, set the OTEL env yourself before -running (`--trace` won't override an exporter you've set): +The console exporter omits start/end timestamps and parent span IDs, so call trees +and durations can't be reconstructed from it — for flame charts, export to an OTLP +collector with `--trace-otlp ` (implies `--trace`; sets +`OTEL_TRACES_EXPORTER=otlp`, `OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf`, and the +endpoint for you): + +```bash +mxcli run --local -p app.mpr --trace-otlp http://127.0.0.1:4318 +``` + +You can still set the OTEL env yourself for full control (`--trace` / `--trace-otlp` +won't override an exporter you've set): ```bash export OTEL_TRACES_EXPORTER=otlp OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317 diff --git a/mdl-examples/bug-tests/alter-page-lowercase-set-on-builtin.mdl b/mdl-examples/bug-tests/alter-page-lowercase-set-on-builtin.mdl new file mode 100644 index 000000000..466e215fd --- /dev/null +++ b/mdl-examples/bug-tests/alter-page-lowercase-set-on-builtin.mdl @@ -0,0 +1,58 @@ +-- ============================================================================ +-- ALTER PAGE ... SET ON failed for every built-in widget +-- ============================================================================ +-- +-- Symptom (before fix): +-- `alter page P { set class = 'x' on topBar; }` — with a lowercase property +-- name, exactly as an author naturally types it — hard-errored on every +-- built-in widget: +-- Error: failed to set: failed to set class on topBar: +-- property "class" not found (widget has no pluggable Object) +-- Reproduced on container, dynamictext and actionbutton, for class, +-- dynamicclasses and caption. Confusingly, none of these are pluggable widgets, +-- and `create page` accepts the same lowercase properties on the same widgets. +-- +-- Root cause: +-- The MPR page mutator (setRawWidgetPropertyMut) dispatched first-class widget +-- properties via a case-SENSITIVE switch on canonical casing ("Class", "Caption", +-- "DynamicClasses", ...). A lowercase name from MDL missed every case and fell +-- through to the pluggable-Object path, which errors on built-in widgets. The +-- sibling MCP backend already matched case-insensitively; the MPR backend did not. +-- (`create page` worked because WidgetV3.GetStringProp reads props case-insensitively.) +-- +-- After fix: +-- setRawWidgetPropertyMut matches first-class properties via strings.ToLower, so +-- `set class`/`set dynamicclasses`/`set caption` (any case) work on built-in +-- widgets. The pluggable fallback keeps original casing (template keys are +-- case-sensitive). +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/alter-page-lowercase-set-on-builtin.mdl -p app.mpr +-- ============================================================================ + +create entity MyFirstModule.Row ( Name: String ); + +create or replace page MyFirstModule.P_LowerSet +( + Title: 'Lowercase Set', + Layout: Atlas_Core.Atlas_Default, + Params: { $Row: MyFirstModule.Row } +) +{ + dataview dv (datasource: $Row) { + container topBar { + dynamictext rowBadge (content: 'badge') + actionbutton btnKeys (caption: 'Keys') + } + } +} + +-- Every one of these used to hard-error "widget has no pluggable Object". +alter page MyFirstModule.P_LowerSet { + set class = 'fl-topbar' on topBar; + set dynamicclasses = 'if $currentObject/Name != '''' then ''is-named'' else ''''' on topBar; + set class = 'fl-badge' on rowBadge; + set caption = 'Shortcuts' on btnKeys; +} + +describe page MyFirstModule.P_LowerSet; diff --git a/mdl-examples/bug-tests/create-or-modify-preserves-attribute-ids.mdl b/mdl-examples/bug-tests/create-or-modify-preserves-attribute-ids.mdl new file mode 100644 index 000000000..71c9d3b90 --- /dev/null +++ b/mdl-examples/bug-tests/create-or-modify-preserves-attribute-ids.mdl @@ -0,0 +1,42 @@ +-- ============================================================================ +-- `create or modify persistent entity` destroyed all column data +-- ============================================================================ +-- +-- Symptom (before fix): +-- Re-applying a byte-for-byte identical `create or modify persistent entity` +-- wiped every value in the affected columns on the next runtime DB sync. The +-- rows survived; every attribute value became NULL. On a real domain model this +-- ran 160 synchronization commands and blanked 11 feeds + 98 articles. +-- +-- Isolated repro (needs a running app to observe the sync): +-- create persistent entity Demo.ZTest ( Val: string(50) ); +-- -- start app, INSERT a row with Val='SURVIVE-ME' +-- create or modify persistent entity Demo.ZTest ( Val: string(50) ); -- identical +-- -- restart runtime -> "Executing N database synchronization command(s)" +-- -- SELECT val -> NULL (column dropped and re-added) +-- +-- Root cause: +-- execCreateEntity minted a FRESH attribute ID (GUID) for every attribute on +-- each run, even unchanged ones. Attribute identity is what Mendix's DB +-- synchronizer keys off — a new ID for an unchanged attribute reads as "old +-- attribute departed, new attribute added", so it drops and re-adds the column, +-- discarding the data. Only the entity's own ID was preserved. +-- +-- After fix: +-- On CREATE OR MODIFY of an existing entity, each retained attribute reuses its +-- existing ID (matched by name); only genuinely new attributes get a fresh ID. +-- An identical re-run is now identity-idempotent: no spurious DB sync, no data +-- loss. Regression covered by +-- TestCreateOrModifyEntity_PreservesAttributeIDs (mdl/executor). +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/create-or-modify-preserves-attribute-ids.mdl -p app.mpr +-- mxcli exec mdl-examples/bug-tests/create-or-modify-preserves-attribute-ids.mdl -p app.mpr # 2nd run must be a no-op at the attribute-identity level +-- ============================================================================ + +create or modify persistent entity MyFirstModule.ZTest +( + Val: String(50) +); + +describe entity MyFirstModule.ZTest; diff --git a/mdl-examples/bug-tests/describe-dynamictext-nonstring-attribute.mdl b/mdl-examples/bug-tests/describe-dynamictext-nonstring-attribute.mdl new file mode 100644 index 000000000..f562aff05 --- /dev/null +++ b/mdl-examples/bug-tests/describe-dynamictext-nonstring-attribute.mdl @@ -0,0 +1,45 @@ +-- ============================================================================ +-- DESCRIBE PAGE did not round-trip a non-string attribute binding +-- ============================================================================ +-- +-- Symptom (before fix): +-- A dynamictext bound to a NON-STRING attribute (Integer/DateTime/…) came back +-- from `describe page` as the rendered template expression: +-- dynamictext num (Content: '{1}', ContentParams: [{1} = toString($currentObject/CountAll)]) +-- Re-applying that made mxcli treat `toString($currentObject/CountAll)` as an +-- attribute NAME, failing the build: +-- [CE1613] "The selected attribute +-- 'Module.Entity.toString($currentObject/CountAll)' no longer exists." +-- A string binding round-tripped fine (bare attribute name). +-- +-- Root cause: +-- The write side converts a non-String attribute binding to +-- `toString($currentObject/Attr)` (dynamictext template params must be String); +-- the describe reader emitted that Expression verbatim instead of reversing it. +-- +-- After fix: +-- The describe reader unwraps the exact auto-generated `toString($currentObject/Attr)` +-- / `toString($param/Attr)` forms back to the bare attribute (or $param.Attr), so +-- DESCRIBE round-trips — the write side re-derives the toString on the next apply. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/describe-dynamictext-nonstring-attribute.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe page MyFirstModule.P_NonStringRT" # re-apply must build clean +-- ============================================================================ + +create entity MyFirstModule.Counter ( CountAll: Integer, ListTitle: String ); + +create or replace page MyFirstModule.P_NonStringRT +( + Title: 'Non-string round-trip', + Layout: Atlas_Core.Atlas_Default, + Params: { $Counter: MyFirstModule.Counter } +) +{ + dataview dv (datasource: $Counter) { + dynamictext num (attribute: CountAll) -- Integer → written as toString(...) + dynamictext str (attribute: ListTitle) -- String + } +} + +describe page MyFirstModule.P_NonStringRT; diff --git a/mdl-examples/bug-tests/javascript-action-call-persist.mdl b/mdl-examples/bug-tests/javascript-action-call-persist.mdl new file mode 100644 index 000000000..fb2dbcb30 --- /dev/null +++ b/mdl-examples/bug-tests/javascript-action-call-persist.mdl @@ -0,0 +1,47 @@ +-- ============================================================================ +-- `call javascript action` parsed + validated but never persisted +-- ============================================================================ +-- +-- Symptom (before fix): +-- A `call javascript action` in a microflow or nanoflow passed +-- `mxcli check --references` (the reference resolved) and `mxcli exec` reported +-- success — but the activity was written with NO action. DESCRIBE showed it +-- round-tripping as `-- Empty action`, and `mx check` failed: +-- [CE0008] "No action defined." at Action activity 'Activity' +-- [CE0109] "Undefined variable 'Opened'." at End event +-- (the output variable's producing activity was gone). +-- +-- Root cause: +-- The default `modelsdk` engine's microflowActionToGen had no case for +-- *microflows.JavaScriptActionCallAction, so it returned nil and the +-- ActionActivity was emitted without an Action. The read path and the legacy +-- engine both handled it (which is why check/exec looked fine). Nanoflows reuse +-- the same converter, so both flow kinds were affected. +-- +-- After fix: +-- microflowActionToGen emits Microflows$JavaScriptActionCallAction (mirroring +-- the legacy serializer), including the JS-specific ParameterValue key for +-- parameter mappings. The activity persists and round-trips. +-- +-- Usage (needs the NanoflowCommons module, i.e. an app with NanoflowCommons): +-- mxcli exec mdl-examples/bug-tests/javascript-action-call-persist.mdl -p app.mpr +-- ============================================================================ + +create entity MyFirstModule.Bookmark ( Link: String ); + +-- Microflow variant. +create or replace microflow MyFirstModule.ACT_OpenLink ($Bookmark: MyFirstModule.Bookmark) +begin + $Opened = call javascript action NanoflowCommons.OpenURL(url = $Bookmark/Link); +end; + +-- Nanoflow variant (the shape the RSS reader hit). +create or replace nanoflow MyFirstModule.NF_OpenLink ($Bookmark: MyFirstModule.Bookmark) +returns boolean as $ok +begin + $Opened = call javascript action NanoflowCommons.OpenURL(url = $Bookmark/Link); + return $Opened; +end; + +describe microflow MyFirstModule.ACT_OpenLink; +describe nanoflow MyFirstModule.NF_OpenLink; diff --git a/mdl-examples/bug-tests/textbox-placeholder-onchange.mdl b/mdl-examples/bug-tests/textbox-placeholder-onchange.mdl new file mode 100644 index 000000000..4118163c4 --- /dev/null +++ b/mdl-examples/bug-tests/textbox-placeholder-onchange.mdl @@ -0,0 +1,57 @@ +-- ============================================================================ +-- textbox silently dropped `placeholder` and `onchange` +-- ============================================================================ +-- +-- Symptom (before fix): +-- `placeholder:` and `onchange:` on a textbox passed check (with an +-- MDL-WIDGET07 "unrecognized property … will be silently dropped" warning) and +-- then vanished — `DESCRIBE PAGE` showed neither. The design's live-search box +-- degraded to a search *button* (no On change), and the field lost its hint +-- text. Both are standard Mendix TextBox properties. +-- +-- Root cause: +-- The pages.TextBox model already had Placeholder/OnChangeAction, and the +-- modelsdk (default) engine already serialized them — but the grammar had no +-- onchange:/placeholder: property, buildTextBoxV3 never populated the model, +-- the legacy serializer hardcoded empty, and they weren't in the known-props +-- list (hence the MDL-WIDGET07 warning). +-- +-- After fix: +-- `onchange: ` and `placeholder: ''` parse, populate the model, +-- and serialize on both engines (OnChange reuses the button client-action path; +-- Placeholder is a ClientTemplate like the label). +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/textbox-placeholder-onchange.mdl -p app.mpr +-- ============================================================================ + +create entity MyFirstModule.Filter ( Query: String ); + +create or replace nanoflow MyFirstModule.NF_Search ($Filter: MyFirstModule.Filter) +begin +end; + +create or replace page MyFirstModule.P_Search +( + Title: 'Search', + Layout: Atlas_Core.Atlas_Default, + Params: { $Filter: MyFirstModule.Filter } +) +{ + layoutgrid grid { + row r { + column c (desktopwidth: autofill) { + dataview dv (datasource: $Filter) { + textbox txtQuery ( + Attribute: Query, + Label: 'Search', + Placeholder: 'Search all articles', + OnChange: nanoflow MyFirstModule.NF_Search(Filter: $Filter) + ) + } + } + } + } +} + +describe page MyFirstModule.P_Search; diff --git a/mdl-examples/bug-tests/widget-arg-keyword-param-names.mdl b/mdl-examples/bug-tests/widget-arg-keyword-param-names.mdl new file mode 100644 index 000000000..e9167b8b8 --- /dev/null +++ b/mdl-examples/bug-tests/widget-arg-keyword-param-names.mdl @@ -0,0 +1,51 @@ +-- ============================================================================ +-- Widget action argument names that are keywords required quoting; call microflow +-- did not +-- ============================================================================ +-- +-- Symptom (before fix): +-- A parameter name accepted bare by `call microflow M.Foo(Source = $x)` was a +-- parse error in a widget's OnClick/Action argument list: +-- container r (OnClick: microflow M.Foo(Source: $x)) { ... } +-- -> line N:col no viable alternative at input 'OnClick' +-- Affected names: View, Source, Item, Page, Entity (all lexer keywords). +-- Quoting worked ("Source": $x), but the error also pointed at the wrong token +-- (OnClick, a valid property) making it very hard to diagnose. +-- +-- Root cause: +-- The widget-argument name rule (microflowArgV3) accepted only +-- (IDENTIFIER | QUOTED_IDENTIFIER), while call-microflow's callArgument used the +-- permissive identifierOrKeyword. Keyword-named params therefore failed the +-- widget rule but passed call-microflow — the same identifier, two grammars. +-- +-- After fix: +-- microflowArgV3 uses identifierOrKeyword, so keyword-named params parse +-- unquoted in widget action args, matching call microflow. The misattributed +-- error position also disappears for these names (they no longer fail to parse). +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/widget-arg-keyword-param-names.mdl -p app.mpr +-- ============================================================================ + +create entity MyFirstModule.Item ( Name: String ); + +create or replace microflow MyFirstModule.ACT_Select ($Source: MyFirstModule.Item) +begin +end; + +create or replace page MyFirstModule.P_KwArg +( + Title: 'Keyword Arg Names', + Layout: Atlas_Core.Atlas_Default, + Params: { $Item: MyFirstModule.Item } +) +{ + dataview dv (datasource: $Item) { + -- 'Source' is a lexer keyword; previously a parse error unless quoted. + container rowBox (OnClick: microflow MyFirstModule.ACT_Select(Source: $Item)) { + dynamictext t (content: 'open') + } + } +} + +describe page MyFirstModule.P_KwArg; diff --git a/mdl/ast/ast_page_v3.go b/mdl/ast/ast_page_v3.go index 22e633380..fd26daeb2 100644 --- a/mdl/ast/ast_page_v3.go +++ b/mdl/ast/ast_page_v3.go @@ -201,6 +201,19 @@ func (w *WidgetV3) GetAction() *ActionV3 { return nil } +// GetOnChange returns the OnChange action (input widgets) or nil. +func (w *WidgetV3) GetOnChange() *ActionV3 { + if v, ok := w.Properties["OnChange"].(*ActionV3); ok { + return v + } + return nil +} + +// GetPlaceholder returns the Placeholder text (input widgets) or empty string. +func (w *WidgetV3) GetPlaceholder() string { + return w.GetStringProp("Placeholder") +} + // GetAttribute returns the Attribute property (attribute path) or empty string. func (w *WidgetV3) GetAttribute() string { return w.GetStringProp("Attribute") diff --git a/mdl/backend/modelsdk/microflow_action_test.go b/mdl/backend/modelsdk/microflow_action_test.go index c25758d01..89cb18b89 100644 --- a/mdl/backend/modelsdk/microflow_action_test.go +++ b/mdl/backend/modelsdk/microflow_action_test.go @@ -7,6 +7,7 @@ import ( "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/modelsdk/element" "github.com/mendixlabs/mxcli/sdk/microflows" ) @@ -193,3 +194,59 @@ func TestMicroflowActionToGen_ExecuteDatabaseQuery(t *testing.T) { t.Errorf("$Type = %q, want DatabaseConnector$ExecuteDatabaseQueryAction", g.TypeName()) } } + +// TestMicroflowActionToGen_JavaScriptActionCall guards finding #11: a +// `call javascript action` (e.g. NanoflowCommons.OpenURL) must convert to a +// Microflows$JavaScriptActionCallAction. Before the fix the modelsdk engine had +// no case for it, so it returned nil → the ActionActivity was written without an +// Action (`-- Empty action`), failing the build with CE0008 "No action defined." +// and CE0109 on the now-missing output variable. This affects both microflows +// and nanoflows (nanoflows reuse microflowActionToGen). +func TestMicroflowActionToGen_JavaScriptActionCall(t *testing.T) { + g := microflowActionToGen(µflows.JavaScriptActionCallAction{ + JavaScriptAction: "NanoflowCommons.OpenURL", + OutputVariableName: "Opened", + UseReturnVariable: true, + ParameterMappings: []*microflows.JavaScriptActionParameterMapping{ + {Parameter: "NanoflowCommons.OpenURL.url", Value: µflows.ExpressionBasedCodeActionParameterValue{Expression: "$Active/Link"}}, + }, + }) + if g == nil { + t.Fatal("nil action (CE0008/CE0109 regression — JS action call dropped on write)") + } + if g.TypeName() != "Microflows$JavaScriptActionCallAction" { + t.Errorf("$Type = %q, want Microflows$JavaScriptActionCallAction", g.TypeName()) + } + // The whole subtree must encode cleanly (guards a missing list marker / mistyped property). + if _, err := (&codec.Encoder{}).Encode(g); err != nil { + t.Errorf("encode: %v", err) + } + // JS parameter mappings use the "ParameterValue" storage key, not the + // Java-style "Value" — the read side and legacy serializer both key off it. + var mappings []element.Element + for _, p := range g.Properties() { + if p.Name() == "ParameterMappings" { + if lst, ok := p.(element.ChildListProperty); ok { + mappings = lst.ChildElements() + } + } + } + if len(mappings) != 1 { + t.Fatalf("ParameterMappings = %d, want 1", len(mappings)) + } + var hasParameterValue, hasWrongValueKey bool + for _, p := range mappings[0].Properties() { + switch p.Name() { + case "ParameterValue": + hasParameterValue = true + case "Value": + hasWrongValueKey = true + } + } + if !hasParameterValue { + t.Error("mapping missing ParameterValue key (JS uses ParameterValue, not Value)") + } + if hasWrongValueKey { + t.Error("mapping emits the Java-style Value key instead of ParameterValue") + } +} diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index 52dab88a3..b80507ef5 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -501,6 +501,32 @@ func microflowActionToGen(action microflows.MicroflowAction) element.Element { } addPartList(g, "ParameterMappings", mappings) return g + case *microflows.JavaScriptActionCallAction: + // Built directly like JavaActionCallAction: the JS action call binds + // distinct storage keys (JavaScriptAction/OutputVariableName, and the + // parameter value goes under "ParameterValue", not "Value"). Mirrors the + // legacy serializer (sdk/mpr/writer_microflow_actions.go). Without this + // case the action fell through to `default: return nil`, so the + // ActionActivity was written with no Action → CE0008 "No action defined." + // and CE0109 on the (now-missing) output variable. + g := newElem("Microflows$JavaScriptActionCallAction", string(a.ID)) + addStr(g, "ErrorHandlingType", orDefault(string(a.ErrorHandlingType), "Rollback")) + addStr(g, "JavaScriptAction", a.JavaScriptAction) + addStr(g, "OutputVariableName", a.OutputVariableName) + addBool(g, "UseReturnVariable", a.UseReturnVariable) + mappings := make([]element.Element, 0, len(a.ParameterMappings)) + for _, pm := range a.ParameterMappings { + m := newElem("Microflows$JavaScriptActionParameterMapping", string(pm.ID)) + addStr(m, "Parameter", pm.Parameter) + if pm.Value != nil { + if v := codeActionParameterValueToGen(pm.Value); v != nil { + addPart(m, "ParameterValue", v) + } + } + mappings = append(mappings, m) + } + addPartList(g, "ParameterMappings", mappings) + return g case *microflows.LogMessageAction: g := genMf.NewLogMessageAction() g.SetID(element.ID(a.ID)) diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index e5f533168..46f20694c 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -1801,45 +1801,50 @@ func setWidgetConditionalSettingMut(widget bson.D, field, typeName, expression s } func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error { - switch propName { - case "Caption": + // Property names arrive verbatim from MDL (any case) — `set class on …` is as + // valid as `set Class on …`, and `create page` reads them case-insensitively + // (WidgetV3.GetStringProp). Match the first-class properties case-insensitively + // so the ALTER path behaves the same; the pluggable fallback (default) keeps the + // original casing, since pluggable property keys must match the template exactly. + switch strings.ToLower(propName) { + case "caption": return setWidgetCaptionMut(widget, value) - case "Content": + case "content": return setWidgetContentMut(widget, value) - case "Label": + case "label": return setWidgetLabelMut(widget, value) - case "ButtonStyle": + case "buttonstyle": if s, ok := value.(string); ok { bsonnav.DSet(widget, "ButtonStyle", s) } return nil - case "Class": + case "class": if appearance := bsonnav.DGetDoc(widget, "Appearance"); appearance != nil { if s, ok := value.(string); ok { bsonnav.DSet(appearance, "Class", s) } } return nil - case "Style": + case "style": if appearance := bsonnav.DGetDoc(widget, "Appearance"); appearance != nil { if s, ok := value.(string); ok { bsonnav.DSet(appearance, "Style", s) } } return nil - case "DynamicClasses": + case "dynamicclasses": if appearance := bsonnav.DGetDoc(widget, "Appearance"); appearance != nil { if s, ok := value.(string); ok { bsonnav.DSet(appearance, "DynamicClasses", s) } } return nil - case "Editable": + case "editable": if s, ok := value.(string); ok { bsonnav.DSet(widget, "Editable", s) } return nil - case "Visible": + case "visible": // A page widget has no plain boolean "Visible" field — visibility is modeled // via ConditionalVisibilitySettings. Route static booleans and expression // strings into it (previously a bare "Visible" string was written and Studio @@ -1855,7 +1860,7 @@ func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error { return fmt.Errorf("widget does not support conditional visibility") } return nil - case "VisibleIf": + case "visibleif": // Conditional visibility expression (issue #627): replace the widget's // ConditionalVisibilitySettings node (null when unset) with one carrying // the rooted expression the visitor produced. @@ -1865,19 +1870,19 @@ func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error { return fmt.Errorf("widget does not support conditional visibility") } return nil - case "EditableIf": + case "editableif": expr, _ := value.(string) if !setWidgetConditionalSettingMut(widget, "ConditionalEditabilitySettings", "Forms$ConditionalEditabilitySettings", expr, false) { return fmt.Errorf("widget does not support conditional editability (only input widgets are editable)") } return nil - case "Name": + case "name": if s, ok := value.(string); ok { bsonnav.DSet(widget, "Name", s) } return nil - case "Attribute": + case "attribute": return setWidgetAttributeRefMut(widget, value) default: // Try as pluggable widget property diff --git a/mdl/backend/pagemutator/mutator_test.go b/mdl/backend/pagemutator/mutator_test.go index 2709b878e..5563a951c 100644 --- a/mdl/backend/pagemutator/mutator_test.go +++ b/mdl/backend/pagemutator/mutator_test.go @@ -230,6 +230,59 @@ func TestSetWidgetProperty_DynamicClasses(t *testing.T) { } } +// TestSetWidgetProperty_LowercaseFirstClassProps locks in the fix for `alter page +// … set class/dynamicclasses on `: property names arrive from MDL in +// whatever case the author typed, and `create page` reads them case-insensitively, +// so the ALTER path must too. Previously a lowercase name fell through to the +// pluggable-property setter and hard-errored "widget has no pluggable Object" on +// every built-in widget. +func TestSetWidgetProperty_LowercaseFirstClassProps(t *testing.T) { + t.Run("class", func(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + if err := m.SetWidgetProperty("ctn1", "class", "fl-topbar"); err != nil { + t.Fatalf("SetWidgetProperty(class) failed: %v", err) + } + app := bsonnav.DGetDoc(findBsonWidget(rawData, "ctn1").widget, "Appearance") + if got := bsonnav.DGetString(app, "Class"); got != "fl-topbar" { + t.Errorf("Appearance.Class = %q, want %q", got, "fl-topbar") + } + }) + + t.Run("dynamicclasses", func(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + expr := "'c--' + $currentObject/Name" + if err := m.SetWidgetProperty("ctn1", "dynamicclasses", expr); err != nil { + t.Fatalf("SetWidgetProperty(dynamicclasses) failed: %v", err) + } + app := bsonnav.DGetDoc(findBsonWidget(rawData, "ctn1").widget, "Appearance") + if got := bsonnav.DGetString(app, "DynamicClasses"); got != expr { + t.Errorf("Appearance.DynamicClasses = %q, want %q", got, expr) + } + }) + + t.Run("caption on built-in", func(t *testing.T) { + w := bson.D{ + {Key: "$Type", Value: "Pages$ActionButton"}, + {Key: "Name", Value: "btnKeys"}, + {Key: "Caption", Value: bson.D{ + {Key: "$Type", Value: "Forms$TextTemplate"}, + {Key: "Text", Value: bson.D{ + {Key: "$Type", Value: "Forms$Text"}, + {Key: "Translations", Value: bson.A{int32(3)}}, + }}, + {Key: "Parameters", Value: bson.A{int32(3)}}, + }}, + } + rawData := makeRawPage(w) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + if err := m.SetWidgetProperty("btnKeys", "caption", "Keys"); err != nil { + t.Fatalf("SetWidgetProperty(caption) failed: %v", err) + } + }) +} + // makeVisibilityWidget builds a widget with a null ConditionalVisibilitySettings // slot (as Studio Pro writes it when unset), matching the widgets that support // conditional visibility. diff --git a/mdl/executor/bugfix_test.go b/mdl/executor/bugfix_test.go index b20ad422e..f29792ee0 100644 --- a/mdl/executor/bugfix_test.go +++ b/mdl/executor/bugfix_test.go @@ -104,6 +104,48 @@ end;` } } +// TestValidateDuplicateMicroflowCallOutputVar covers finding #5 (second trigger): +// a microflow-call output variable reused across a fallback chain is a fresh +// declaration each time, so the second assignment is CE0111 — the natural +// "try A, else try B" shape. Both the same-scope and the nested-in-if forms +// must be caught (the fallback is almost always written inside an if). +func TestValidateDuplicateMicroflowCallOutputVar(t *testing.T) { + t.Run("same scope", func(t *testing.T) { + input := `create microflow Test.MF_Fallback () +begin + $Summary = call microflow Test.Inner(Tag = 'description'); + $Summary = call microflow Test.Inner(Tag = 'summary'); +end;` + errors := validateMicroflowFromMDL(t, input) + if !hasDupError(errors, "Summary") { + t.Errorf("Expected duplicate variable error for $Summary, got: %v", errors) + } + }) + + t.Run("fallback inside if", func(t *testing.T) { + input := `create microflow Test.MF_FallbackIf () +begin + $Summary = call microflow Test.Inner(Tag = 'description'); + if trim($Summary) = '' then + $Summary = call microflow Test.Inner(Tag = 'summary'); + end if; +end;` + errors := validateMicroflowFromMDL(t, input) + if !hasDupError(errors, "Summary") { + t.Errorf("Expected duplicate variable error for $Summary (fallback in if), got: %v", errors) + } + }) +} + +func hasDupError(errors []string, varName string) bool { + for _, e := range errors { + if strings.Contains(e, "duplicate") && strings.Contains(e, varName) { + return true + } + } + return false +} + // TestValidateEntityReservedAttributeName verifies that persistent entity attributes // using reserved system names (CreatedDate, ChangedDate, Owner, ChangedBy) are caught. func TestValidateEntityReservedAttributeName(t *testing.T) { diff --git a/mdl/executor/cmd_catalog.go b/mdl/executor/cmd_catalog.go index a7db3458f..da55dac56 100644 --- a/mdl/executor/cmd_catalog.go +++ b/mdl/executor/cmd_catalog.go @@ -78,11 +78,12 @@ func execShowCatalogTables(ctx *ExecContext) error { // fullOnlyTables are catalog tables only populated by REFRESH CATALOG FULL. var fullOnlyTables = map[string]bool{ - "widgets": true, - "activities": true, - "refs": true, - "strings": true, - "permissions": true, + "widgets": true, + "activities": true, + "refs": true, + "xpath_expressions": true, + "strings": true, + "permissions": true, // graph_* analysis views read the refs graph, so they need full mode too. "graph_god_nodes": true, "graph_module_coupling": true, diff --git a/mdl/executor/cmd_catalog_mode_test.go b/mdl/executor/cmd_catalog_mode_test.go new file mode 100644 index 000000000..def55f9dc --- /dev/null +++ b/mdl/executor/cmd_catalog_mode_test.go @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import "testing" + +// TestFullOnlyTablesCoverage guards the "plain REFRESH leaves the analytic tables +// empty, 0 rows, no error" papercut: every catalog table populated only in FULL +// mode must be flagged as requiring `refresh catalog full`, so a query in fast +// mode warns instead of silently returning nothing. xpath_expressions was missing +// (activities/refs were covered) — a query against it returned empty with no hint. +func TestFullOnlyTablesCoverage(t *testing.T) { + // These views are populated only when the catalog is built in FULL mode + // (builder_microflows.go activities, builder_references.go refs, + // builder_xpath.go xpath_expressions, builder_pages.go widgets). + fullOnly := []string{"activities", "refs", "xpath_expressions", "widgets"} + for _, tbl := range fullOnly { + if got := tableRequiredMode(tbl); got != "refresh catalog full" { + t.Errorf("tableRequiredMode(%q) = %q, want %q — a fast-mode query would silently return 0 rows", tbl, got, "refresh catalog full") + } + } + + // A fast-mode table must NOT be flagged (no false warning). + if got := tableRequiredMode("entities"); got != "refresh catalog" { + t.Errorf("tableRequiredMode(\"entities\") = %q, want %q", got, "refresh catalog") + } +} diff --git a/mdl/executor/cmd_entities.go b/mdl/executor/cmd_entities.go index 0f38b0f57..90141cd10 100644 --- a/mdl/executor/cmd_entities.go +++ b/mdl/executor/cmd_entities.go @@ -141,6 +141,20 @@ func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error { // Also detect pseudo-types (AutoOwner, AutoChangedBy, etc.) and set entity flags var storeOwner, storeChangedBy, storeCreatedDate, storeChangedDate bool + // On CREATE OR MODIFY of an existing entity, reuse each retained attribute's + // existing ID rather than minting a fresh one. Attribute identity is what + // Mendix's DB synchronizer keys off: a new ID for an unchanged attribute reads + // as "old attribute departed, new attribute added", so it DROPS and re-adds the + // column — wiping the stored data. Reusing the ID by name makes an identical + // re-run truly idempotent (no spurious sync, no data loss). Genuinely new + // attributes still get a fresh ID. (findings #13) + existingAttrIDByName := make(map[string]model.ID) + if s.CreateOrModify && existingEntity != nil { + for _, ea := range existingEntity.Attributes { + existingAttrIDByName[ea.Name] = ea.ID + } + } + var attrs []*domainmodel.Attribute attrNameToID := make(map[string]model.ID) for _, a := range s.Attributes { @@ -171,8 +185,13 @@ func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error { doc = a.Comment } - // Generate ID for the attribute so we can reference it in validation rules/indexes + // Generate ID for the attribute so we can reference it in validation rules/indexes. + // On CREATE OR MODIFY, reuse the existing attribute's ID (by name) to preserve + // identity and avoid a destructive DB column drop/re-add. (findings #13) attrID := model.ID(types.GenerateID()) + if id, ok := existingAttrIDByName[a.Name]; ok { + attrID = id + } attrNameToID[a.Name] = attrID attr := &domainmodel.Attribute{ diff --git a/mdl/executor/cmd_entities_mock_test.go b/mdl/executor/cmd_entities_mock_test.go index 0d63655e9..f2e701935 100644 --- a/mdl/executor/cmd_entities_mock_test.go +++ b/mdl/executor/cmd_entities_mock_test.go @@ -150,6 +150,60 @@ func TestAlterEntity_AllowCreateChangeLocally_Issue534(t *testing.T) { } } +// TestCreateOrModifyEntity_PreservesAttributeIDs guards finding #13 (data loss): +// re-running an identical CREATE OR MODIFY on an existing entity must reuse each +// retained attribute's existing ID. A fresh ID reads to Mendix's DB synchronizer +// as "attribute departed + new attribute added", so it drops and re-adds the +// column — wiping the stored values. Genuinely new attributes still get a fresh ID. +func TestCreateOrModifyEntity_PreservesAttributeIDs(t *testing.T) { + mod := mkModule("M") + const existingNameID = model.ID("attr-name-STABLE") + entity := mkEntity(mod.ID, "Feed") + entity.Attributes = []*domainmodel.Attribute{ + {BaseElement: model.BaseElement{ID: existingNameID}, Name: "Name", Type: &domainmodel.StringAttributeType{}}, + } + dm := mkDomainModel(mod.ID, entity) + + var updated *domainmodel.Entity + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { return []*domainmodel.DomainModel{dm}, nil }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dm, nil }, + ListEnumerationsFunc: func() ([]*model.Enumeration, error) { return nil, nil }, + UpdateEntityFunc: func(dmID model.ID, e *domainmodel.Entity) error { updated = e; return nil }, + } + h := mkHierarchy(mod) + withContainer(h, dm.ID, mod.ID) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + // Same "Name" attribute (unchanged) plus a genuinely new "Url". + err := execCreateEntity(ctx, &ast.CreateEntityStmt{ + Name: ast.QualifiedName{Module: "M", Name: "Feed"}, + Kind: ast.EntityPersistent, + CreateOrModify: true, + Attributes: []ast.Attribute{ + {Name: "Name", Type: ast.DataType{Kind: ast.TypeString}}, + {Name: "Url", Type: ast.DataType{Kind: ast.TypeString}}, + }, + }) + assertNoError(t, err) + if updated == nil { + t.Fatal("expected UpdateEntity to be called") + } + + byName := map[string]model.ID{} + for _, a := range updated.Attributes { + byName[a.Name] = a.ID + } + if byName["Name"] != existingNameID { + t.Errorf("retained attribute 'Name' ID = %q, want preserved %q (data-loss regression)", byName["Name"], existingNameID) + } + if byName["Url"] == "" || byName["Url"] == existingNameID { + t.Errorf("new attribute 'Url' should get a fresh non-empty ID, got %q", byName["Url"]) + } +} + func TestShowEntities_JSON(t *testing.T) { mod := mkModule("App") ent := mkEntity(mod.ID, "Item") diff --git a/mdl/executor/cmd_pages_builder_v3_widgets.go b/mdl/executor/cmd_pages_builder_v3_widgets.go index d6fecd936..dd2f21242 100644 --- a/mdl/executor/cmd_pages_builder_v3_widgets.go +++ b/mdl/executor/cmd_pages_builder_v3_widgets.go @@ -298,6 +298,26 @@ func (pb *pageBuilder) buildTextBoxV3(w *ast.WidgetV3) (*pages.TextBox, error) { tb.Label = label } + // Handle Placeholder (input hint text — a ClientTemplate, like the label) + if ph := w.GetPlaceholder(); ph != "" { + tb.Placeholder = &model.Text{ + BaseElement: model.BaseElement{ + ID: model.ID(types.GenerateID()), + TypeName: "Texts$Text", + }, + Translations: map[string]string{"en_US": ph}, + } + } + + // Handle OnChange (the "On change" client action) + if action := w.GetOnChange(); action != nil { + act, err := pb.buildClientActionV3(action) + if err != nil { + return nil, err + } + tb.OnChangeAction = act + } + if err := pb.registerWidgetName(w.Name, tb.ID); err != nil { return nil, err } diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index a8e89ec1a..f24281281 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -591,6 +591,8 @@ type rawWidget struct { ReadOnlyStyle string // "Inherit", "Control", "Text" ShowLabel bool // Whether label is shown (from LabelTemplate visibility) LabelPosition string // "Left", "Top", etc. + Placeholder string // Placeholder hint text (from PlaceholderTemplate) + OnChange string // MDL rendering of the OnChangeAction client action // Filter widget properties FilterAttributes []string // Attributes to filter on FilterExpression string // Default filter expression (contains, startsWith, etc.) diff --git a/mdl/executor/cmd_pages_describe_dyntext_test.go b/mdl/executor/cmd_pages_describe_dyntext_test.go new file mode 100644 index 000000000..f90c9b444 --- /dev/null +++ b/mdl/executor/cmd_pages_describe_dyntext_test.go @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import "testing" + +// TestUnwrapToStringAttrParam covers the non-string-attribute DESCRIBE round-trip +// fix: a ContentParam auto-generated as toString($currentObject/Attr) (or +// toString($param/Attr)) must render back as the bare attribute (or $param.Attr), +// while any other expression is left untouched. +func TestUnwrapToStringAttrParam(t *testing.T) { + cases := []struct { + in string + want string + wantOK bool + }{ + {"toString($currentObject/CountAll)", "CountAll", true}, + {"toString($currentObject/Feedline.Article_Source/Slug)", "Feedline.Article_Source/Slug", true}, + {"toString($View/CountAll)", "$View.CountAll", true}, + // Not the exact auto-generated form → left as-is. + {"toString($currentObject/CountAll) + ' items'", "", false}, + {"formatDateTime($currentObject/Created, 'yyyy')", "", false}, + {"$currentObject/CountAll", "", false}, + {"'literal'", "", false}, + } + for _, c := range cases { + got, ok := unwrapToStringAttrParam(c.in) + if ok != c.wantOK || (ok && got != c.want) { + t.Errorf("unwrapToStringAttrParam(%q) = (%q, %v), want (%q, %v)", c.in, got, ok, c.want, c.wantOK) + } + } +} + +// TestParseRawWidget_DynamicTextNonStringAttribute drives the describe reader with +// a dynamictext whose ContentParam is the toString(...) form Mendix stores for a +// non-string attribute, and asserts the describe param is the bare attribute name +// (so re-applying the DESCRIBE output doesn't fail CE1613). +func TestParseRawWidget_DynamicTextNonStringAttribute(t *testing.T) { + ctx, _ := newMockCtx(t) + raw := map[string]any{ + "$Type": "Forms$DynamicText", + "Name": "num", + "Content": map[string]any{ + "$Type": "Forms$ClientTemplate", + "Template": map[string]any{ + "$Type": "Texts$Text", + "Items": []any{ + map[string]any{"$Type": "Texts$Translation", "LanguageCode": "en_US", "Text": "{1}"}, + }, + }, + "Parameters": []any{ + map[string]any{"$Type": "Forms$ClientTemplateParameter", "Expression": "toString($currentObject/CountAll)"}, + }, + }, + } + got := parseRawWidget(ctx, raw) + if len(got) != 1 { + t.Fatalf("expected 1 widget, got %d", len(got)) + } + if len(got[0].Parameters) != 1 || got[0].Parameters[0] != "CountAll" { + t.Errorf("ContentParam = %v, want [CountAll] (bare attribute, not the toString expression)", got[0].Parameters) + } +} diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 965411d2e..91ddd04cd 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "io" + "regexp" "strings" "github.com/mendixlabs/mxcli/model" @@ -365,6 +366,12 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { if w.Content != "" { props = append(props, fmt.Sprintf("Attribute: %s", w.Content)) } + if w.Placeholder != "" { + props = append(props, fmt.Sprintf("Placeholder: %s", mdlQuote(w.Placeholder))) + } + if w.OnChange != "" { + props = append(props, fmt.Sprintf("OnChange: %s", w.OnChange)) + } props = appendAppearanceProps(props, w) formatWidgetProps(ctx.Output, prefix, header, props, "\n") @@ -969,14 +976,34 @@ func extractIconRef(w map[string]any) string { } func extractButtonAction(ctx *ExecContext, w map[string]any) string { - action, ok := w["Action"].(map[string]any) - if !ok { - // Try primitive.M type - if actionM, okM := w["Action"].(primitive.M); okM { - action = map[string]any(actionM) - } else { - return "" - } + return renderClientActionMDL(ctx, actionMapForKey(w, "Action")) +} + +// extractOnChangeAction renders a widget's OnChangeAction (input widgets) as MDL, +// reusing the same client-action renderer as buttons — OnChangeAction is the same +// client-action type, just under a different BSON key. +func extractOnChangeAction(ctx *ExecContext, w map[string]any) string { + return renderClientActionMDL(ctx, actionMapForKey(w, "OnChangeAction")) +} + +// actionMapForKey unwraps a client-action node stored under key into a plain map +// (handling both map[string]any and primitive.M), or nil when absent. +func actionMapForKey(w map[string]any, key string) map[string]any { + if action, ok := w[key].(map[string]any); ok { + return action + } + if actionM, ok := w[key].(primitive.M); ok { + return map[string]any(actionM) + } + return nil +} + +// renderClientActionMDL renders a client-action map (a Forms$*ClientAction) back +// to its MDL form (microflow/nanoflow/show_page/save_changes/…). Returns "" for a +// nil action or a NoClientAction. +func renderClientActionMDL(ctx *ExecContext, action map[string]any) string { + if action == nil { + return "" } typeName, _ := action["$Type"].(string) switch typeName { @@ -1308,6 +1335,31 @@ func extractTextCaption(ctx *ExecContext, w map[string]any) string { } // extractClientTemplateParameters extracts parameter values from a ClientTemplate field (Content or Caption). +// toStringCurrentObjectParamRe matches the exact `toString($currentObject/)` +// form the write side emits for a non-String own-attribute binding. The path may +// carry association hops (Module.Assoc/Attr). +var toStringCurrentObjectParamRe = regexp.MustCompile(`^toString\(\$currentObject/([A-Za-z_][A-Za-z0-9_.]*(?:/[A-Za-z_][A-Za-z0-9_.]*)*)\)$`) + +// toStringParamRefRe matches the `toString($/)` form emitted for a +// non-String page/snippet-parameter binding. +var toStringParamRefRe = regexp.MustCompile(`^toString\(\$([A-Za-z_][A-Za-z0-9_]*)/([A-Za-z_][A-Za-z0-9_.]*)\)$`) + +// unwrapToStringAttrParam reverses the write side's non-String attribute→toString +// transform so a ContentParam round-trips as a bare attribute (or $param.attr) +// rather than a rendered expression that re-applies as a bogus attribute name. +// Only the exact auto-generated forms are unwrapped; any other expression (extra +// text, operators, a hand-written toString) is left untouched. +func unwrapToStringAttrParam(expr string) (string, bool) { + if m := toStringCurrentObjectParamRe.FindStringSubmatch(expr); m != nil { + return m[1], true + } + if m := toStringParamRefRe.FindStringSubmatch(expr); m != nil { + // $param/attr → $param.attr (the round-trip form for a parameter ref) + return "$" + m[1] + "." + m[2], true + } + return "", false +} + func extractClientTemplateParameters(ctx *ExecContext, w map[string]any, fieldName string) []string { template, ok := w[fieldName].(map[string]any) if !ok { @@ -1325,7 +1377,17 @@ func extractClientTemplateParameters(ctx *ExecContext, w map[string]any, fieldNa } // Check for Expression first (literal value) if expr, ok := pMap["Expression"].(string); ok && expr != "" { - result = append(result, expr) + // A non-String attribute binding (Integer/DateTime/…) is written as + // `toString($currentObject/Attr)` / `toString($param/Attr)` — see + // resolveTemplateAttributePathFull. Emit it back as the bare attribute + // (or $param.Attr) so DESCRIBE round-trips; re-applying the bare name + // re-derives the same toString expression on the write side. Otherwise + // the rendered expression re-applies as a bogus attribute NAME (CE1613). + if unwrapped, ok := unwrapToStringAttrParam(expr); ok { + result = append(result, unwrapped) + } else { + result = append(result, expr) + } continue } diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index df189fb2c..c1b9d241f 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -259,6 +259,8 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s case "Forms$TextBox", "Pages$TextBox": widget.Caption = extractLabelText(ctx, w) widget.Content = extractAttributeRef(ctx, w) + widget.Placeholder = extractPlaceholderText(ctx, w) + widget.OnChange = extractOnChangeAction(ctx, w) return []rawWidget{widget} case "Forms$TextArea", "Pages$TextArea": @@ -635,6 +637,16 @@ func extractLabelText(ctx *ExecContext, w map[string]any) string { return extractTextFromTemplate(ctx, labelTemplate) } +// extractPlaceholderText extracts the placeholder hint text from an input widget. +// PlaceholderTemplate is a Forms$ClientTemplate, same shape as LabelTemplate. +func extractPlaceholderText(ctx *ExecContext, w map[string]any) string { + placeholderTemplate, ok := w["PlaceholderTemplate"].(map[string]any) + if !ok { + return "" + } + return extractTextFromTemplate(ctx, placeholderTemplate) +} + // extractEditable extracts the Editable setting from an input widget. // Returns "Always", "Never", or "Conditional". func extractEditable(ctx *ExecContext, w map[string]any) string { diff --git a/mdl/executor/cmd_pages_describe_textbox_test.go b/mdl/executor/cmd_pages_describe_textbox_test.go new file mode 100644 index 000000000..ec34f35e7 --- /dev/null +++ b/mdl/executor/cmd_pages_describe_textbox_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import "testing" + +// TestParseRawWidget_TextBoxPlaceholderAndOnChange covers the DESCRIBE round-trip +// gap: a textbox's PlaceholderTemplate and OnChangeAction must be read back into +// the describe model (previously only Label + Attribute were), so +// `describe page` reflects what `create page` wrote. +func TestParseRawWidget_TextBoxPlaceholderAndOnChange(t *testing.T) { + ctx, _ := newMockCtx(t) + + raw := map[string]any{ + "$Type": "Forms$TextBox", + "Name": "txtQuery", + // PlaceholderTemplate is a Forms$ClientTemplate: Template.Items[] holds the translation. + "PlaceholderTemplate": map[string]any{ + "$Type": "Forms$ClientTemplate", + "Template": map[string]any{ + "$Type": "Texts$Text", + "Items": []any{ + map[string]any{"$Type": "Texts$Translation", "LanguageCode": "en_US", "Text": "Search all articles"}, + }, + }, + }, + // OnChangeAction is a client action, same shape a button's Action uses: + // Forms$MicroflowAction with the microflow name under MicroflowSettings. + "OnChangeAction": map[string]any{ + "$Type": "Forms$MicroflowAction", + "MicroflowSettings": map[string]any{ + "$Type": "Forms$MicroflowSettings", + "Microflow": "MyFirstModule.ACT_Search", + }, + }, + } + + got := parseRawWidget(ctx, raw) + if len(got) != 1 { + t.Fatalf("expected 1 widget, got %d", len(got)) + } + w := got[0] + if w.Placeholder != "Search all articles" { + t.Errorf("Placeholder = %q, want %q", w.Placeholder, "Search all articles") + } + if w.OnChange == "" { + t.Errorf("OnChange should render the client action, got empty") + } +} diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index d1a8be7ce..484ca431b 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "regexp" + "sort" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -560,11 +561,19 @@ func validateFlowBodyReferences(ctx *ExecContext, body []ast.MicroflowStatement, // appear in the project's MPR. Skip them to avoid false // positives — Studio Pro's `mx check` resolves these against // the runtime, which `mxcli check` cannot reach. - if isBuiltinModuleEntity(qualifiedNameModule(ref)) { + if isBuiltinModuleEntity(qualifiedNameModule(ref.name)) { continue } - if !known[ref] { - errors = append(errors, fmt.Sprintf("java action not found: %s (referenced by call java action)", ref)) + if !known[ref.name] { + errors = append(errors, fmt.Sprintf("java action not found: %s (referenced by call java action)", ref.name)) + continue + } + if ja, err := ctx.Backend.ReadJavaActionByName(ref.name); err == nil && ja != nil { + var declared []string + for _, p := range ja.Parameters { + declared = append(declared, p.Name) + } + errors = append(errors, validateCodeActionParams("java action", ref, declared)...) } } } @@ -572,11 +581,19 @@ func validateFlowBodyReferences(ctx *ExecContext, body []ast.MicroflowStatement, if len(refs.javaScriptActions) > 0 { known := buildJavaScriptActionQualifiedNames(ctx) for _, ref := range refs.javaScriptActions { - if isBuiltinModuleEntity(qualifiedNameModule(ref)) { + if isBuiltinModuleEntity(qualifiedNameModule(ref.name)) { continue } - if !known[ref] { - errors = append(errors, fmt.Sprintf("javascript action not found: %s (referenced by call javascript action)", ref)) + if !known[ref.name] { + errors = append(errors, fmt.Sprintf("javascript action not found: %s (referenced by call javascript action)", ref.name)) + continue + } + if jsa, err := ctx.Backend.ReadJavaScriptActionByName(ref.name); err == nil && jsa != nil { + var declared []string + for _, p := range jsa.Parameters { + declared = append(declared, p.Name) + } + errors = append(errors, validateCodeActionParams("javascript action", ref, declared)...) } } } @@ -700,12 +717,72 @@ type flowRefCollector struct { pages []string microflows []string nanoflows []string - javaActions []string - javaScriptActions []string + javaActions []codeActionCallRef + javaScriptActions []codeActionCallRef entities []entityRef retrieves []retrieveConstraintRef } +// codeActionCallRef is a Java / JavaScript action call: the action's qualified +// name plus the parameter names the author wrote, so both the action's existence +// and its parameter names can be validated (a wrong/mis-cased name writes a +// dangling reference that only fails at build time with CE1613). +type codeActionCallRef struct { + name string + argNames []string +} + +// callArgNames extracts the written parameter names from a code-action call's +// argument list. +func callArgNames(args []ast.CallArgument) []string { + if len(args) == 0 { + return nil + } + names := make([]string, 0, len(args)) + for _, a := range args { + if a.Name != "" { + names = append(names, a.Name) + } + } + return names +} + +// validateCodeActionParams checks that each parameter name written in a Java / +// JavaScript action call matches a declared parameter (case-sensitively — Mendix +// parameter names are case-sensitive, and a mismatch writes a dangling reference +// that fails the build with CE1613). When the mismatch is only a casing +// difference, the message suggests the correct spelling. `declared` empty means +// the backend could not report parameters — skip (degrade gracefully). +func validateCodeActionParams(kind string, ref codeActionCallRef, declared []string) []string { + if len(declared) == 0 { + return nil + } + declaredSet := make(map[string]bool, len(declared)) + for _, d := range declared { + declaredSet[d] = true + } + var errs []string + for _, written := range ref.argNames { + if declaredSet[written] { + continue + } + msg := fmt.Sprintf("%s %s has no parameter %q", kind, ref.name, written) + // Case-only mismatch → point at the correct spelling. + for _, d := range declared { + if strings.EqualFold(d, written) { + msg += fmt.Sprintf(" — did you mean %q?", d) + break + } + } + sorted := append([]string(nil), declared...) + sort.Strings(sorted) + msg += fmt.Sprintf(" (declared parameters: %s). Mendix build fails CE1613 \"The selected %s parameter … no longer exists\".", + strings.Join(sorted, ", "), kind) + errs = append(errs, msg) + } + return errs +} + // entityRef tracks an entity reference along with the statement that referenced it. type entityRef struct { name string @@ -742,11 +819,15 @@ func (c *flowRefCollector) collectFromStatements(stmts []ast.MicroflowStatement) } case *ast.CallJavaActionStmt: if s.ActionName.Module != "" { - c.javaActions = append(c.javaActions, s.ActionName.String()) + c.javaActions = append(c.javaActions, codeActionCallRef{ + name: s.ActionName.String(), argNames: callArgNames(s.Arguments), + }) } case *ast.CallJavaScriptActionStmt: if s.ActionName.Module != "" { - c.javaScriptActions = append(c.javaScriptActions, s.ActionName.String()) + c.javaScriptActions = append(c.javaScriptActions, codeActionCallRef{ + name: s.ActionName.String(), argNames: callArgNames(s.Arguments), + }) } case *ast.CallWebServiceStmt: // Web service and mapping references can be raw IDs; reference validation diff --git a/mdl/executor/validate_codeaction_params_test.go b/mdl/executor/validate_codeaction_params_test.go new file mode 100644 index 000000000..09ff9384e --- /dev/null +++ b/mdl/executor/validate_codeaction_params_test.go @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" +) + +// TestValidateCodeActionParams covers the new-finding param-name check: a Java / +// JavaScript action call whose written parameter name doesn't match a declared +// parameter is flagged (case-sensitively), with a did-you-mean on a casing-only +// mismatch. A matching name passes; an empty declared set (backend can't report) +// degrades to no error. +func TestValidateCodeActionParams(t *testing.T) { + declared := []string{"Username", "Password"} + + t.Run("case-only mismatch flagged with did-you-mean", func(t *testing.T) { + ref := codeActionCallRef{name: "NanoflowCommons.SignIn", argNames: []string{"username", "Password"}} + errs := validateCodeActionParams("javascript action", ref, declared) + if len(errs) != 1 { + t.Fatalf("expected 1 error, got %d: %v", len(errs), errs) + } + if !strings.Contains(errs[0], `no parameter "username"`) || !strings.Contains(errs[0], `did you mean "Username"?`) { + t.Errorf("expected did-you-mean for username, got: %s", errs[0]) + } + if !strings.Contains(errs[0], "CE1613") { + t.Errorf("expected CE1613 reference, got: %s", errs[0]) + } + }) + + t.Run("unknown name flagged without did-you-mean", func(t *testing.T) { + ref := codeActionCallRef{name: "M.A", argNames: []string{"Bogus"}} + errs := validateCodeActionParams("java action", ref, declared) + if len(errs) != 1 { + t.Fatalf("expected 1 error, got %d: %v", len(errs), errs) + } + if strings.Contains(errs[0], "did you mean") { + t.Errorf("no casing match exists, should not suggest one: %s", errs[0]) + } + }) + + t.Run("all names match → no error", func(t *testing.T) { + ref := codeActionCallRef{name: "M.A", argNames: []string{"Username", "Password"}} + if errs := validateCodeActionParams("javascript action", ref, declared); len(errs) != 0 { + t.Errorf("expected no errors, got: %v", errs) + } + }) + + t.Run("empty declared set → degrade to no error", func(t *testing.T) { + ref := codeActionCallRef{name: "M.A", argNames: []string{"anything"}} + if errs := validateCodeActionParams("javascript action", ref, nil); len(errs) != 0 { + t.Errorf("expected graceful skip on unknown params, got: %v", errs) + } + }) +} diff --git a/mdl/executor/validate_duplicates.go b/mdl/executor/validate_duplicates.go index 0f3f02f44..2d58eaa05 100644 --- a/mdl/executor/validate_duplicates.go +++ b/mdl/executor/validate_duplicates.go @@ -577,9 +577,17 @@ func CheckProjectConflicts(ctx *ExecContext, prog *ast.Program) []error { if !idempotent && !reg.isAlive(dt, name) && !droppedFromProject.isAlive(dt, name) { projectSet := ps.setFor(dt) if projectSet != nil && projectSet[name] { + hint := "use CREATE OR MODIFY to update it" + if dt == "entity" { + // For a persistent entity, CREATE OR MODIFY rebuilds the whole + // definition and drops any attribute this statement omits — steer + // toward the incremental, non-destructive path instead. (findings #13) + hint = "to add or change a member use 'alter entity " + name + + " add attribute ...' (leaves the rest intact); use CREATE OR MODIFY only to replace the whole definition" + } errs = append(errs, fmt.Errorf( - "statement %d: %s already exists in project: %s — use CREATE OR MODIFY to update it", - stmtNum, friendlyDocType(dt), name, + "statement %d: %s already exists in project: %s — %s", + stmtNum, friendlyDocType(dt), name, hint, )) } } diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index 7a43f880d..5dcbee53e 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -215,8 +215,8 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { if v.loopDepth > 0 { v.addViolation("MDL001", linter.SeverityWarning, "nested loop detected (loop inside a loop). "+ - "Use retrieve $Match from $List where ... limit 1 for list matching instead of nested loops (O(N^2) performance).", - "Replace nested loop with retrieve ... where ... limit 1 for O(N) lookup") + "Use FIND($List, ) for in-memory list matching instead of an inner loop (O(N) vs O(N^2)).", + "Replace the inner loop with $Match = FIND($List, key = $item/key) (a plain retrieve ... where cannot filter a list variable)") } // Check: loop over empty declared list if v.emptyListVars[stmt.ListVariable] { @@ -271,6 +271,13 @@ func (v *microflowValidator) checkNumericAssignment(targetLabel string, targetKi fmt.Sprintf("Declare '%s' as Decimal, or round the value (e.g. round(%s) or floor(%s)).", targetLabel, src, src)) } +// mendixAggregateFuncs are the list aggregates that Mendix exposes as +// activities, not expression functions. Used inside an expression they fail +// CE0117; the fix is to assign the aggregate to a variable first. +var mendixAggregateFuncs = map[string]bool{ + "count": true, "sum": true, "average": true, "minimum": true, "maximum": true, +} + // checkExprFunctions flags calls to names that are not Mendix expression // functions (e.g. a hallucinated randomInt()) — these parse and pass a naive // check but fail the build with CE0117. label describes where the expression @@ -281,9 +288,20 @@ func (v *microflowValidator) checkExprFunctions(label string, expr ast.Expressio return } for _, u := range exprcheck.UnknownFunctionCalls(src) { - suggestion := "Use a built-in Mendix expression function (see 'mxcli syntax expressions')." - if u.Suggestion != "" { - suggestion = fmt.Sprintf("Did you mean '%s()'? ", u.Suggestion) + suggestion + var suggestion string + if mendixAggregateFuncs[strings.ToLower(u.Name)] { + // count/sum/average/minimum/maximum are aggregate ACTIVITIES, not + // expression functions — a did-you-mean against an unrelated math + // function (e.g. count→round) sends the author the wrong way. Tell them + // to assign the aggregate to a variable first, then use the variable. + suggestion = fmt.Sprintf( + "'%s' is an aggregate activity, not an expression function. Assign it to a variable first: $n = %s($List); then use $n in the expression.", + u.Name, u.Name) + } else { + suggestion = "Use a built-in Mendix expression function (see 'mxcli syntax microflow')." + if u.Suggestion != "" { + suggestion = fmt.Sprintf("Did you mean '%s()'? ", u.Suggestion) + suggestion + } } v.addViolation("MDL044", linter.SeverityError, fmt.Sprintf("%s calls '%s()', which is not a Mendix expression function — "+ diff --git a/mdl/executor/validate_microflow_expr_test.go b/mdl/executor/validate_microflow_expr_test.go index d960b2301..706845bbc 100644 --- a/mdl/executor/validate_microflow_expr_test.go +++ b/mdl/executor/validate_microflow_expr_test.go @@ -42,6 +42,10 @@ func TestValidateMicroflow_UnknownFunction(t *testing.T) { {"round(random()) is known", "declare $r Integer = round(random() * 8);", false, ""}, {"nested known funcs", "declare $s String = toUpperCase(trim($x));", false, ""}, {"unknown in if condition", "if isBlank($x) then\n return;\n end if;", true, ""}, + // count/sum/... are aggregate activities, not expression functions: MDL044 + // fires, but the hint must steer to "assign to a variable first", not a + // did-you-mean against an unrelated math function (finding #7). + {"count aggregate hint", "declare $ok Boolean = if count($x) > 0 then true else false;", true, "aggregate activity"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 6239c606b..868da3d9a 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -405,7 +405,8 @@ func objectListMappingSet(def *WidgetDefinition) map[string]*ObjectListMapping { var staticWidgetKnownProps = func() map[string]bool { names := []string{ // grammar keyword properties (widgetPropertyV3) - "DataSource", "Attribute", "Binds", "Action", "OnClick", "Caption", "Label", + "DataSource", "Attribute", "Binds", "Action", "OnClick", "OnChange", "Placeholder", + "Caption", "Label", "Attr", "Content", "RenderMode", "ContentParams", "CaptionParams", "ButtonStyle", "Class", "Style", "DesktopWidth", "TabletWidth", "PhoneWidth", "Selection", "Snippet", "Params", "Attributes", "FilterType", "DesignProperties", "Width", @@ -557,6 +558,49 @@ func validateStaticWidget(w *ast.WidgetV3, locationPrefix string) []linter.Viola } } + // A widget EXPRESSION property (DynamicClasses / VisibleIf / EditableIf) that + // walks an association fails the build with CE0117 — Mendix client-side + // expressions cannot traverse associations (only data bindings such as + // contentparams/attribute can). This is statically checkable and easy to trip + // over, because a data binding on the SAME widget can traverse the same + // association legitimately. (findings #4) + out = append(out, validateWidgetExpressionAssociations(w, locationPrefix)...) + + return out +} + +// exprAssociationStepRe matches an association step inside an expression: a +// slash-delimited, module-qualified name (e.g. `/Feedline.Article_Source/`). A +// plain attribute access (`$obj/Slug`) is a single unqualified segment and does +// not match; a qualified enum literal (`Mod.Enum.Value`) has no leading slash and +// does not match either. +var exprAssociationStepRe = regexp.MustCompile(`/([A-Za-z_][A-Za-z0-9_]*\.[A-Za-z_][A-Za-z0-9_]*)/`) + +// expressionWidgetProps are the widget property keys whose value is a client-side +// Mendix expression (never a data binding). An association step in any of these +// is always CE0117. +var expressionWidgetProps = []string{"DynamicClasses", "VisibleIf", "EditableIf"} + +// validateWidgetExpressionAssociations flags an association traversal inside an +// expression-typed widget property (MDL-WIDGET13). +func validateWidgetExpressionAssociations(w *ast.WidgetV3, locationPrefix string) []linter.Violation { + var out []linter.Violation + for _, key := range expressionWidgetProps { + expr, ok := lookupWidgetProp(w, key) + if !ok || expr == "" { + continue + } + if m := exprAssociationStepRe.FindStringSubmatch(expr); m != nil { + out = append(out, linter.Violation{ + RuleID: "MDL-WIDGET13", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: widget `%s` property `%s` expression traverses association `%s` — Mendix expressions cannot follow associations (CE0117). Bind the value via a data binding (contentparams/attribute) or precompute it onto the bound entity (e.g. a calculated attribute).", + locationPrefix, w.Name, key, m[1], + ), + }) + } + } return out } diff --git a/mdl/executor/validate_widgets_test.go b/mdl/executor/validate_widgets_test.go index 819c6f7a1..a7d8a8f1b 100644 --- a/mdl/executor/validate_widgets_test.go +++ b/mdl/executor/validate_widgets_test.go @@ -154,6 +154,47 @@ func TestValidateStaticWidget_DataViewDatabaseSource(t *testing.T) { } } +// TestValidateWidgetExpressionAssociations — MDL-WIDGET13 flags an association +// step inside an expression-typed widget property (DynamicClasses/VisibleIf/ +// EditableIf). Such expressions fail the build with CE0117; a data binding on the +// same widget may traverse the same association legitimately (not flagged here). +func TestValidateWidgetExpressionAssociations(t *testing.T) { + cases := []struct { + name string + widget *ast.WidgetV3 + want bool // expect an MDL-WIDGET13 violation + }{ + {"dynamicclasses traversing association → rejected", + &ast.WidgetV3{Type: "dynamictext", Name: "rowBadge", Properties: map[string]any{ + "DynamicClasses": "'fl-tint-' + $currentObject/Feedline.Article_Source/Slug"}}, true}, + {"visibleif traversing association → rejected", + &ast.WidgetV3{Type: "container", Name: "box", Properties: map[string]any{ + "VisibleIf": "$currentObject/Feedline.Article_Source/Active"}}, true}, + {"editableif traversing association → rejected", + &ast.WidgetV3{Type: "textbox", Name: "tb", Properties: map[string]any{ + "EditableIf": "$currentObject/Feedline.Article_Source/Editable"}}, true}, + {"dynamicclasses on a plain attribute → ok", + &ast.WidgetV3{Type: "dynamictext", Name: "rowBadge", Properties: map[string]any{ + "DynamicClasses": "'fl-tint-' + $currentObject/SourceSlug"}}, false}, + {"visibleif comparing an enum literal → ok (not a traversal)", + &ast.WidgetV3{Type: "container", Name: "box", Properties: map[string]any{ + "VisibleIf": "$currentObject/Status = Feedline.State.Open"}}, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := false + for _, v := range validateStaticWidget(c.widget, "page X") { + if v.RuleID == "MDL-WIDGET13" { + got = true + } + } + if got != c.want { + t.Errorf("MDL-WIDGET13 present = %v, want %v", got, c.want) + } + }) + } +} + // TestValidateObjectListItemEnums — MDL-WIDGET08 flags an object-list item's // enumeration sub-property whose value isn't a declared member key (e.g. a Maps // marker LocationType outside {address, latlng}). Studio Pro silently defaults diff --git a/mdl/grammar/domains/MDLPage.g4 b/mdl/grammar/domains/MDLPage.g4 index 693cd60b6..5a6c09493 100644 --- a/mdl/grammar/domains/MDLPage.g4 +++ b/mdl/grammar/domains/MDLPage.g4 @@ -338,6 +338,8 @@ widgetPropertyV3 | BINDS COLON attributePathV3 // Binds: (deprecated, use Attribute:) | ACTION COLON actionExprV3 // Action: SAVE_CHANGES | SHOW_PAGE ... | ONCLICK COLON actionExprV3 // OnClick: MICROFLOW ... (alias of Action: — e.g. clickable CONTAINER, issue #603) + | ONCHANGE COLON actionExprV3 // OnChange: MICROFLOW ... (input widgets — TextBox/TextArea/DatePicker/CheckBox) + | PLACEHOLDER COLON STRING_LITERAL // Placeholder: 'Search all articles' (input widgets) | CAPTION COLON stringExprV3 // Caption: 'Save' | LABEL COLON STRING_LITERAL // Label: 'Name' | ATTR COLON attributePathV3 // Attr: (deprecated, use Attribute:) @@ -436,7 +438,9 @@ microflowArgsV3 ; microflowArgV3 - : (IDENTIFIER | QUOTED_IDENTIFIER) COLON expression // Param: $value (canonical; "Param" if reserved) + : identifierOrKeyword COLON expression // Param: $value (identifierOrKeyword so a param + // named after a keyword — View/Source/Item/Page/ + // Entity — works unquoted, matching callArgument) | VARIABLE EQUALS expression // $Param = $value (microflow-style, also accepted) ; diff --git a/mdl/visitor/visitor.go b/mdl/visitor/visitor.go index c0d15fb90..975f92571 100644 --- a/mdl/visitor/visitor.go +++ b/mdl/visitor/visitor.go @@ -134,6 +134,18 @@ func enhanceErrorMessage(msg, offendingLine string) string { " create entity Mod.Child (Name: String) extends Mod.Parent; (wrong — causes parse error)", msg) } + // Check for a string literal that runs off the end of its line BEFORE the + // apostrophe heuristic — a newline inside the offending token means the lexer + // hit end-of-line inside an unterminated literal, which the "double your + // apostrophes" hint would only make worse. (findings #6) + if looksLikeMultilineStringLiteral(msg) { + return fmt.Sprintf("%s\n\n A string literal is not terminated before the end of the line —\n"+ + " MDL string literals cannot span multiple lines. Keep the whole string on\n"+ + " one line (concatenate across lines with + if it is long):\n"+ + " dynamicclasses: 'if $x then ''a'' else ''b''' (correct — one line)\n"+ + " dynamicclasses: 'if $x then ''a''\n else ''b''' (wrong — spans lines)", msg) + } + // Check for unescaped apostrophe in string literals first. // When 'it's here' is parsed, ANTLR sees 'it' as a complete string, then // the leftover characters (like "s", "ll", "t") appear as unexpected tokens. @@ -254,6 +266,19 @@ var contractionSuffixes = map[string]bool{ "re": true, "ve": true, "ll": true, } +// looksLikeMultilineStringLiteral detects an unterminated string literal that ran +// off the end of its line. ANTLR reports "token recognition error at: ”" with the +// offending text spilling onto the next line (a newline appears in the payload). +// This must be distinguished from a plain unescaped apostrophe — the apostrophe +// hint ("double your apostrophes") would make a line-spanning string worse. +func looksLikeMultilineStringLiteral(msg string) bool { + idx := strings.Index(msg, "token recognition error at:") + if idx < 0 { + return false + } + return strings.ContainsAny(msg[idx:], "\n\r") +} + // looksLikeUnescapedApostrophe detects ANTLR errors that are likely caused by // unescaped apostrophes in string literals. When 'don't' is parsed, ANTLR sees // 'don' as a complete string, then 't' as an unexpected token, producing errors diff --git a/mdl/visitor/visitor_page_quoted_test.go b/mdl/visitor/visitor_page_quoted_test.go index f0228c8ee..a181e7615 100644 --- a/mdl/visitor/visitor_page_quoted_test.go +++ b/mdl/visitor/visitor_page_quoted_test.go @@ -230,6 +230,42 @@ func TestQuotedReservedSelectionAndArgNames(t *testing.T) { } } +// TestWidgetArgKeywordParamNames covers finding #2/#3: a widget action argument +// whose parameter name is a lexer keyword (View/Source/Item/Page/Entity) was a +// parse error unless quoted, even though `call microflow` accepts the same names +// bare. microflowArgV3 now uses identifierOrKeyword (like callArgument), so these +// parse unquoted and the visitor recovers the bare name. +func TestWidgetArgKeywordParamNames(t *testing.T) { + for _, name := range []string{"View", "Source", "Item", "Page", "Entity"} { + t.Run(name, func(t *testing.T) { + input := `CREATE PAGE M.Home ( + Layout: Atlas_Core.Atlas_Default + ) { + DATAVIEW detail (DataSource: M.Product) { + ACTIONBUTTON btnOpen ( + Caption: 'Open', + Action: MICROFLOW M.ACT_Select (` + name + `: $currentObject) + ) + } + };` + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("Parse errors for unquoted keyword arg %q: %v", name, errs) + } + stmt := prog.Statements[0].(*ast.CreatePageStmtV3) + detail := findChildByName2(stmt.Widgets, "detail") + btnOpen := findChildByName(detail, "btnOpen") + if btnOpen == nil || btnOpen.GetAction() == nil { + t.Fatal("btnOpen or its action not found") + } + args := btnOpen.GetAction().Args + if len(args) != 1 || args[0].Name != name { + t.Errorf("expected one arg named %q, got %+v", name, args) + } + }) + } +} + func findChildByName(parent *ast.WidgetV3, name string) *ast.WidgetV3 { for _, c := range parent.Children { if c.Name == name { diff --git a/mdl/visitor/visitor_page_v3.go b/mdl/visitor/visitor_page_v3.go index 7646ed44f..7aec490e9 100644 --- a/mdl/visitor/visitor_page_v3.go +++ b/mdl/visitor/visitor_page_v3.go @@ -593,6 +593,22 @@ func parseWidgetPropertyV3(ctx parser.IWidgetPropertyV3Context, widget *ast.Widg return } + // OnChange: ... (input widgets — the "On change" action) + if propCtx.ONCHANGE() != nil { + if actCtx := propCtx.ActionExprV3(); actCtx != nil { + widget.Properties["OnChange"] = buildActionV3(actCtx) + } + return + } + + // Placeholder: 'text' (input widgets) + if propCtx.PLACEHOLDER() != nil { + if str := propCtx.STRING_LITERAL(); str != nil { + widget.Properties["Placeholder"] = unquoteString(str.GetText()) + } + return + } + // Caption: ... if propCtx.CAPTION() != nil { if strCtx := propCtx.StringExprV3(); strCtx != nil { @@ -1002,12 +1018,11 @@ func buildMicroflowArgV3(ctx parser.IMicroflowArgV3Context) ast.FlowArgV3 { if v := argCtx.VARIABLE(); v != nil { // Microflow-style: $Param = $value arg.Name = strings.TrimPrefix(v.GetText(), "$") - } else if id := argCtx.IDENTIFIER(); id != nil { - // Widget-style: Param: $value - arg.Name = id.GetText() - } else if qid := argCtx.QUOTED_IDENTIFIER(); qid != nil { - // Widget-style with reserved-word param name: "Param": $value - arg.Name = unquoteIdentifier(qid.GetText()) + } else if iok := argCtx.IdentifierOrKeyword(); iok != nil { + // Widget-style: Param: $value. identifierOrKeyword accepts a bare + // keyword (View/Source/Item/Page/Entity) or a "quoted" name; + // identifierOrKeywordText unquotes as needed. + arg.Name = identifierOrKeywordText(iok) } if expr := argCtx.Expression(); expr != nil { arg.Value = expr.GetText() diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index 104dce328..f6575019a 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1303,6 +1303,32 @@ func TestEnhanceErrorMessage_Apostrophe(t *testing.T) { } } +// TestEnhanceErrorMessage_MultilineString covers finding #6: a string literal +// that runs off the end of its line must get the "cannot span multiple lines" +// hint, NOT the "double your apostrophes" advice (which would make it worse). The +// distinguishing signal is a newline inside the token-recognition-error payload. +func TestEnhanceErrorMessage_MultilineString(t *testing.T) { + // A literal spanning two lines: the offending text carries a newline. + multiline := "line 8:66 token recognition error at: ''\n'" + out := enhanceErrorMessage(multiline, "") + if !strings.Contains(out, "cannot span multiple lines") { + t.Errorf("expected multiline-string hint, got: %s", out) + } + if strings.Contains(out, "unescaped apostrophe") { + t.Errorf("multiline-string error must not show the apostrophe hint, got: %s", out) + } + + // A single-line unbalanced quote (no newline) keeps the apostrophe hint. + singleLine := "line 3:12 token recognition error at: ''" + out = enhanceErrorMessage(singleLine, "") + if strings.Contains(out, "cannot span multiple lines") { + t.Errorf("single-line error must not show the multiline hint, got: %s", out) + } + if !strings.Contains(out, "apostrophe") { + t.Errorf("expected apostrophe hint for single-line unbalanced quote, got: %s", out) + } +} + func TestParseError_UnescapedApostrophe(t *testing.T) { // This MDL contains an unescaped apostrophe in a string literal. // The parser should produce an error with an apostrophe hint. diff --git a/sdk/mpr/writer_widgets.go b/sdk/mpr/writer_widgets.go index 5dd0b0d78..d5e9654ce 100644 --- a/sdk/mpr/writer_widgets.go +++ b/sdk/mpr/writer_widgets.go @@ -530,6 +530,23 @@ func serializeEmptyPlaceholderTemplate() bson.D { } } +// serializePlaceholderTemplate creates a placeholder ClientTemplate from a Text, +// or an empty one when nil. Same Forms$ClientTemplate shape as the label template. +func serializePlaceholderTemplate(t *model.Text) bson.D { + if t == nil { + return serializeEmptyPlaceholderTemplate() + } + text := "" + for _, v := range t.Translations { + text = v + break + } + if text == "" { + return serializeEmptyPlaceholderTemplate() + } + return serializeLabelTemplate(text) +} + // serializeLabelTemplate creates a standard label template for input widgets. func serializeLabelTemplate(label string) bson.D { if label == "" { diff --git a/sdk/mpr/writer_widgets_input.go b/sdk/mpr/writer_widgets_input.go index ea967a493..d14ef60c5 100644 --- a/sdk/mpr/writer_widgets_input.go +++ b/sdk/mpr/writer_widgets_input.go @@ -30,11 +30,11 @@ func serializeTextBox(tb *pages.TextBox) bson.D { {Key: "MaxLengthCode", Value: int64(-1)}, {Key: "Name", Value: tb.Name}, {Key: "NativeAccessibilitySettings", Value: nil}, - {Key: "OnChangeAction", Value: serializeClientAction(nil)}, - {Key: "OnEnterAction", Value: serializeClientAction(nil)}, + {Key: "OnChangeAction", Value: serializeClientAction(tb.OnChangeAction)}, + {Key: "OnEnterAction", Value: serializeClientAction(tb.OnEnterAction)}, {Key: "OnEnterKeyPressAction", Value: serializeClientAction(nil)}, {Key: "OnLeaveAction", Value: serializeClientAction(nil)}, - {Key: "PlaceholderTemplate", Value: serializeEmptyPlaceholderTemplate()}, + {Key: "PlaceholderTemplate", Value: serializePlaceholderTemplate(tb.Placeholder)}, {Key: "ReadOnlyStyle", Value: "Inherit"}, {Key: "ScreenReaderLabel", Value: nil}, {Key: "SourceVariable", Value: nil}, diff --git a/sdk/mpr/writer_widgets_test.go b/sdk/mpr/writer_widgets_test.go index 078b08a21..fdf1be747 100644 --- a/sdk/mpr/writer_widgets_test.go +++ b/sdk/mpr/writer_widgets_test.go @@ -259,6 +259,90 @@ func TestSerializeTextBox(t *testing.T) { } } +// TestSerializeTextBox_PlaceholderAndOnChange guards finding #9: placeholder and +// onchange were hardcoded to empty on the legacy write path (silently dropped). +// They must now serialize from the model's Placeholder / OnChangeAction. +func TestSerializeTextBox_PlaceholderAndOnChange(t *testing.T) { + tb := &pages.TextBox{ + BaseWidget: pages.BaseWidget{ + BaseElement: model.BaseElement{ID: "tb-id", TypeName: "Forms$TextBox"}, + Name: "txtQuery", + }, + AttributePath: "M.Filter.Query", + Placeholder: &model.Text{ + BaseElement: model.BaseElement{ID: "ph-id", TypeName: "Texts$Text"}, + Translations: map[string]string{"en_US": "Search all articles"}, + }, + OnChangeAction: &pages.MicroflowClientAction{ + MicroflowName: "M.ACT_Search", + }, + } + + result := serializeTextBox(tb) + + // PlaceholderTemplate must carry the placeholder text (not the empty template). + var placeholderText string + var onChangeType string + for _, elem := range result { + switch elem.Key { + case "PlaceholderTemplate": + if d, ok := elem.Value.(bson.D); ok { + placeholderText = extractTemplateText(d) + } + case "OnChangeAction": + if d, ok := elem.Value.(bson.D); ok { + for _, e := range d { + if e.Key == "$Type" { + onChangeType, _ = e.Value.(string) + } + } + } + } + } + if placeholderText != "Search all articles" { + t.Errorf("PlaceholderTemplate text = %q, want %q", placeholderText, "Search all articles") + } + if onChangeType == "" || onChangeType == "Forms$NoAction" { + t.Errorf("OnChangeAction should be a real action, got $Type = %q", onChangeType) + } +} + +// extractTemplateText pulls the first Translation Text out of a Forms$ClientTemplate. +func extractTemplateText(ct bson.D) string { + for _, e := range ct { + if e.Key != "Template" { + continue + } + tmpl, ok := e.Value.(bson.D) + if !ok { + continue + } + for _, te := range tmpl { + if te.Key != "Items" { + continue + } + items, ok := te.Value.(bson.A) + if !ok { + continue + } + for _, it := range items { + trans, ok := it.(bson.D) + if !ok { + continue + } + for _, tr := range trans { + if tr.Key == "Text" { + if s, ok := tr.Value.(string); ok { + return s + } + } + } + } + } + } + return "" +} + func TestSerializeDataViewLabelWidth(t *testing.T) { five := 5 zero := 0