diff --git a/.claude/skills/fix-issue/findings/mdl-backend.jsonl b/.claude/skills/fix-issue/findings/mdl-backend.jsonl index b42032884..5a3209e45 100644 --- a/.claude/skills/fix-issue/findings/mdl-backend.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-backend.jsonl @@ -149,3 +149,5 @@ {"area": "mdl/backend", "date": "2026-09-29", "symptom": "describe -> exec, CREATE OR MODIFY or an ALTER that rebuilds the document turns an API-exported document Hidden: enumerations, pages, layouts, rules, view-entity OQL source documents, import/export mappings, JSON structures, published and consumed REST services, scheduled events, workflows, database connections, business event services, data transformers, queues, regular expressions and agent-editor documents. The run reports success, mx check is clean; the module's public surface silently shrinks. A workflow's own `export level API` clause was a no-op on create and on rewrite.", "cause": "Each rewrite converter builds a fresh document and writes ExportLevel as a constant (\"Hidden\"), or passes the semantic model's value where the executor itself filled in \"Hidden\" (mappings, database connection, business events), and the unit is replaced wholesale. MDL has no export-level spelling for most of these kinds, so describe cannot print it and the executed script cannot restore it. workflowToGen ignored wf.ExportLevel entirely. The round-trip harness could not see it: every document in TestApp and PedApp is Hidden, the constant itself.", "file": "mdl/backend/modelsdk/export_level_carry.go", "fix": "One byte-level carry, keepStoredExportLevel(unitID, contents): replaces only the top-level ExportLevel element of the freshly encoded rewrite with the stored value, copying every other element verbatim, and never adds the key. Wired into every Update path that writes ExportLevel (UpdateEnumeration/Rule/Layout/ImportMapping/ExportMapping/JsonStructure/PublishedRestService/ConsumedRestService/DataTransformer/DatabaseConnection/BusinessEventService, writeCustomBlob update, WriteViewEntitySourceDocument update; page via carryStoredPageHeader). Kinds with an MDL spelling (workflow, scheduled event, queue, regular expression) use keepStoredExportLevelUnlessSet: an authored level wins. workflowToGen now writes orDefault(wf.ExportLevel, \"Hidden\").", "insight": "A fixture-driven round trip is blind to any constant that happens to equal every fixture value: 775 TestApp documents round-tripped while 10 kinds hid API documents. Set the subject to the non-default value first (here: patch ExportLevel to API on the working copy) and run both the plain describe output and an edited one, because an elided unchanged write passes a converter that still writes the constant. Carrying at the encoded-bytes level covers gen-typed, newElem-built and hand-serialized writers with one helper, where a gen setter per converter would have needed three mechanisms.", "refs": ["ako/mxcli#816", "ako/mxcli#801", "ako/mxcli#812"], "test": "mdl/backend/modelsdk/issue816_export_level_test.go (TestUpdatePaths_KeepStoredExportLevel, 18 kinds); mdl/roundtrip/export_level_test.go (TestTestAppExportLevelSurvivesRoundTrip, -tags integration)"} {"date": "2026-09-30", "area": "mdl/backend", "symptom": "ako/mxcli#859 review of PR #864: after the built comparison landed, changing or adding `show page M.P with title = 'X'` in a `create or modify microflow` reported \"Unchanged microflow\" and wrote nothing, under mdl 0 and mdl 1 (main spliced it). Nothing warned.", "cause": "builtAsStored compares the declared flow and the stored flow both READ BACK through the codec, so any property the reader drops compares equal whatever either side holds. The ShowFormAction reader never read FormSettings.TitleOverride. Probing encode(built) against encode(readback(built)) over mdl-examples found the reader also dropped ExclusiveSplit/LoopedActivity ErrorHandlingType and a REST call's bound output variable (ResultHandling.ResultVariableName -> RestCallAction.OutputVariable), plus CallWebServiceAction (#861). Before the built comparison such a loss was a visible phantom re-splice; after it, a silently dropped edit.", "fix": "ReadBackMicroflow/ReadBackNanoflow re-encode what they read back and refuse (error -> statement diff, the pre-#859 path) when it is not the document first written, $IDs aside (sameWritten). The reader now reads TitleOverride, the split's and loop's ErrorHandlingType, and a bound REST call's OutputVariable, so those flows keep matching.", "insight": "A comparison made on both sides through the same lossy reader cannot see what the reader loses; the lost property becomes a change that is never written. When equality is decided after a decode, prove the decode lossless for the value at hand (write it again and compare bytes) and fall back when it is not. The probe that found the fields: diff encode(x) with encode(decode(encode(x))) over every mdl-examples flow.", "issue": "ako/mxcli#859", "file": "mdl/backend/modelsdk/microflow_readback.go, mdl/backend/modelsdk/microflow_read_actions.go, mdl/backend/modelsdk/microflow.go, mdl/roundtrip/flow_idempotent_shapes_test.go"} {"date": "2026-09-30", "area": "mdl/backend", "symptom": "ako/mxcli#843 (rehearsal M2): under mdl 1, `create or modify nanoflow … returns Boolean as $Done` over a nanoflow stored without a return variable refuses \"the stored document has no ReturnVariableName property … set it in Studio Pro\"; the same statement on a microflow reports \"set: ReturnVariableName\".", "cause": "mfmutator.SetHeader refuses any stated header key the stored document lacks (a key the project version does not declare makes the document unopenable). mxcli's nanoflow writer omits ReturnVariableName when the statement has no `as $Var`, while the microflow writer always writes it on 10+, so only nanoflows hit the refusal.", "fix": "Optional mfmutator.PropertyDeclarer on Deps; the codec deps answer from the metamodel version data (type, then Microflows$MicroflowBase; ReturnVariableName is 10.12+) against the project version, and SetHeader inserts the key after its predecessor in the encoder's order. No answer (MCP, unknown version) keeps the refusal.", "insight": "A refusal keyed on 'the stored document lacks the key' conflates 'this version has no such property' with 'the writer left it out'; the metamodel version data separates the two. Studio Pro 11 stores ReturnVariableName on every nanoflow (PedApp: 13 of 13), so adding it matches what Studio Pro writes.", "issue": "ako/mxcli#843", "file": "mdl/backend/mfmutator/header.go"} +{"date": "2026-10-01", "area": "mdl/backend", "symptom": "ako/mxcli#628: `move entity A.Parent to B; move entity A.Child to B;` both report success, then Studio Pro cannot open the project: System.AggregateException: The given key '' was not present in the dictionary.", "cause": "MoveEntity scanned only sourceDM.AssociationsItems(); an existing cross-association (created by the first move) was invisible, so moving its FROM entity left its ParentPointer naming an element absent from the unit, and moving its TO entity left Child naming the old module. Cross-unit damage, so the #1119 write guard cannot see it.", "fix": "association_move_cross.go: own cross-associations (FROM = moved entity) travel to the target, or become a plain DomainModels$Association there when the TO entity is already in it (raw transform: $Type, Child -> ChildPointer binary id, connection points added, GUID passed through); a cross-association in the target naming the entity becomes plain; any other module's cross-association naming it is re-pointed. MovedAssociation.SameModule lets the executor report the plain conversions apart.", "insight": "Assert on the stored documents (every ParentPointer/ChildPointer resolves in its own unit, every cross Child resolves to an entity in the named module); mx check is a weak signal for this class. The four sequences (to-then-from, from-then-to, travel, re-point) are distinct shapes, not one.", "issue": "ako/mxcli#628", "file": "mdl/backend/modelsdk/association_move_cross.go"} +{"date": "2026-10-01", "area": "mdl/backend", "symptom": "ako/mxcli#627: the fluent API's AttributeModifier.Apply() (Backend.UpdateAttribute) on a Studio Pro-authored attribute is refused with \"refusing to write unit …: 1 element(s) kept their $ID but would be written with a different GUID (DomainModels$Attribute)\"; without the #1119 guard it would re-mint the GUID and drop the column on the next deploy.", "cause": "UpdateAttribute rebuilds the attribute with attributeToGen (raw == nil) and carried only the $ID, so the codec's EmitGUID default wrote GUID = $ID. No MDL statement reaches it — every ALTER ENTITY form goes through UpdateEntity, which has the carry — so it was found by enumerating the converter's call sites, not by a repro.", "fix": "UpdateAttribute carries the stored attribute's raw bytes and export level onto the rebuild via carryStoredAttribute, the helper now shared with carryAttributeIdentity. Attribute -> Attribute keeps $Type, so SetRaw suffices.", "insight": "Test on PedApp (GUID != $ID); an mxcli-created attribute re-mints the same value and cannot fail. The unfixed code fails the test via the write guard's refusal, which is the observable symptom on main.", "issue": "ako/mxcli#627", "file": "mdl/backend/modelsdk/domainmodel_alter.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index db4ac1363..bf6255a66 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -791,3 +791,4 @@ {"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#890 (rehearsal 2, R-rep): re-running a settled script wrote nothing (units_written=0) but still printed \"Granted access on ...\", \"Set project security level to ...\", \"Added module roles ... to user role ...\", \"Updated ... settings\" / \"Updated configuration ...\", and `move ... to folder` printed \"Moved ... to new location\"; the output could not serve as the #859 'second run reports Unchanged' gate. Also: a second `move` of the same document in one session failed \"microflow not found\".", "cause": "Those handlers printed their sentence with fmt.Fprintf after the backend call instead of going through ReportMutation's write-elision evidence (WriteStats offered vs written). Grants/revokes inside a program run are deferred (#872 accessRuleRun), so even ReportMutation could not see their write at statement time. The typed movers changed a container without invalidating the cached hierarchy.", "file": "mdl/executor/report_mutation.go, mdl/executor/access_rule_run.go, mdl/executor/cmd_security_write.go, mdl/executor/cmd_settings.go, mdl/executor/cmd_move.go", "fix": "ExecContext.reportWrite(unchanged, sentence...) prints the sentence or `Unchanged ` (through the run tally) on the ReportMutation evidence rule; used for project security level/demo users/strict mode/guest access, alter user role module roles, settings section/configuration/constant updates. Access-rule reports go through reportAccessRule: held on the open accessRuleRun and printed at its flush, Unchanged when the flush offered and elided (notices like 'No access rules found' print regardless). execMove short-circuits a document already in the target container (alreadyPlaced -> Unchanged) and invalidates the hierarchy after every move.", "test": "mdl/executor/noop_reporting_pedapp_test.go TestNoopRerun_ReportsUnchanged (PedApp, per statement: run 1 reports its write = control; run 2 writes no file and reports Unchanged, for grant, security level, demo users, strict mode, user role module roles, settings runtime, configuration (alter and create or modify), move) and TestNoopRerun_ProgramReportsUnchanged (program run incl. a revoke+grant reset; run 2 writes nothing and reports no write verb; run 1's net-nothing reset reports no write). Revert check: every case fails with the write sentence on run 2; the move case with 'microflow not found'.", "insight": "A report printed after a backend call is a claim about storage the handler cannot make on its own; route every write report through the write-stats evidence, and where writes are deferred, defer the report with them. The output only becomes an idempotency gate when no statement prints a write verb by construction."} {"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#840 (mxcli-ledger, finding 162): `mxcli describe` (no header, no option) wrote mdl 0 spellings that mdl 1 refuses, and its microflow output held `$N = count($Hits)` / `$x = find($L, …)` — call forms registered as deprecated (MDL-DEPR003/004), so subcommand output was neither mdl 1-runnable nor mdl 0-clean. At the freeze a second instance surfaced on TestApp: describe of a chart series' text template wrote `staticTooltipHoverTextParams: [{1} = X]`, the bracketed form MDL-DEPR124 deprecates, under both versions.", "cause": "formatListOperation / the AggregateListAction case gated the statement form on describeLanguage >= V1 and fell back to the call form for mdl 0, although the statement form parses with the same meaning and no warning under mdl 0. The object-list describer (cmd_pages_describe_objectlist.go) wrote a TextTemplate's Params as \"[\" + … + \"]\". The roundtrip test that should have caught both (describeUsesCanonicalSpellings) filtered deprecations to an R8 allowlist ('describe keeps them under mdl 0'), so any code outside the list was invisible.", "file": "mdl/executor/cmd_microflows_format_action.go, mdl/executor/cmd_pages_describe_objectlist.go, mdl/roundtrip/describe_canonical_spelling_test.go", "fix": "Describe writes the List operation / Aggregate list statement in every language; only an activity the statement cannot express falls back to the call. Object-list template parameters are written in ( ). The canonical-spelling roundtrip test now checks EVERY registered deprecation under both describe languages (describeAs V1 and V0) on PedApp and TestApp, with no code filter. Freeze: langver.Frozen = V1, every describe output starts with `mdl 1;`, `--mdl 0|1` on describe/context/diff-local.", "test": "mdl/executor/cmd_microflows_format_list_activity_test.go TestDescribeListActivityUnderMdl1 (both versions, plus the call-form control warning under mdl 0); mdl/roundtrip/describe_canonical_spelling_test.go TestTestAppDescribeUsesCanonicalSpellings (failed on Snip_TaskDashboard_Numbers & 2 more with MDL-DEPR124 before the objectlist fix).", "insight": "A test that filters warnings to a list of 'codes this test owns' hides every code added later; a 'never emits X' property must check the whole registry, and in every output language the command can be asked for."} {"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#905 part 1 (#897 item 4, rehearsal 3 G2): under `mdl 1`, `create or modify microflow` that grows a stored flow by an activity whose custom error handler ends in its own `return` (`$Ok = call microflow … on error begin log …; return; end error;`) was refused on every run, as an insert or a replace: \"an error handler in the fragment ends at an end event of its own … a return inside an error handler is not spliced yet\". Created fresh the statement worked; mdl 0 rebuilt (MDL-V1-REBUILD). On CapTrack the stub-then-real pair (13-actions stub, 30-export real ACT_Export_Excel) left the placeholder stored on every run with mx check clean. Once spliced, the second run of the real CapTrack script was refused: \"replace $Written: cannot replace the ActionActivity …: it has an error handler\".", "cause": "addErrorHandlerFlow (cmd_microflows_builder_flows.go) builds a handler body with a child flowBuilder and merged its objects and flows into the parent, but not its returnEndIDs, so cutFragment saw the handler's return end event as one the builder added and refused it. Second defect, exposed by the grow: a log/show message/validation feedback message written as an expression (`'failed for ' + $User/Name`) is stored as template '{1}' with the expression as parameter, and describe prints that form; declaredMatches compared the two spellings as different statements. Where builtAsStored is false (a spliced or Studio Pro-drawn flow), the statement diff then replaced the activity: absorbed by write elision on the main path, but refused when the message sits in a stored activity's error handler.", "file": "mdl/executor/cmd_microflows_builder_flows.go, mdl/executor/flow_declared_match.go", "fix": "addErrorHandlerFlow copies errBuilder.returnEndIDs into the parent's (lastReturnEndID untouched), so a handler's return is a new end event of the flow like a guard's (#888); placement/room checks (checkRoom/checkBranches) apply unchanged and refuse where the handler's return branch would cross a stored flow. matchValue normalises LogStmt/ShowMessageStmt/ValidationFeedbackStmt (messageAsTemplate) to the builder's stored form: a non-literal message becomes '{1}' with the expression as first parameter (a log stating its own `with (...)` keeps its message, as the builder does).", "test": "mdl/executor/cmd_alter_flow_handler_return_test.go TestCutFragment_HandlerReturnIsANewEndEvent; mdl/executor/flow_message_respelling_test.go (with controls); mdl/roundtrip/flow_splice_handler_return_test.go TestSpliceRerun_GrowByHandlerReturn (replace/insert under mdl 1, insert under mdl 0, verdict agreement check/diff/exec, twice-exec, a changed-flow control), TestSpliceRerun_GrowStudioProFlowByHandlerReturn (PedApp ShowPasswordForm, all stored IDs kept, description fixed point), TestSpliceRerun_HandlerReturnWithNoRoomIsRefused. Revert checks: returnEndIDs not copied -> every grow refused 'not spliced yet' (check predicts it); messageAsTemplate off -> second run refused 'replace $Ok … it has an error handler'. CapTrack copy: fmt --upgrade -p 13+30, exec 13, 30, 30 -> real flow stored, third run 0 units; mx check identical to baseline (0 errors). PedApp repros mx check identical to baseline.", "insight": "Child builders (error handler, loop) each keep builder state the parent's consumers rely on; when a new piece of builder state is added for the splice (returnEndIDs, #888), enumerate the child builders and decide per child whether it propagates. A grow test that only re-runs on an mxcli-authored flow can pass because builtAsStored short-circuits the statement diff; the respelling only surfaced on the real project and the Studio Pro-drawn flow, where the statement diff decides."} +{"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#571 / mendixlabs/mxcli#1206: `create published rest service` wrote only the path's {name} placeholders as operation parameters, each a String: every query and body microflow parameter failed mx check with CE0350, an Integer {id} with CE6539. `import mapping` / `export mapping` / `commit` on an operation parsed and were thrown away (CE0350 on the body, CE0354 on an object-returning microflow). Executing describe of a Studio Pro service (TestApp Services.OrdersRestApi) reported 'Modified' and broke a 0-error app with 5 errors: mappings cleared, Integer path params retyped String, body param dropped, Commit No->Yes, Basic+Session authentication turned off", "cause": "publishedRestOperationToGen built parameters from the path alone and wrote ExportMapping/ImportMapping \"\" and Commit \"Yes\" as constants; the reader never read parameters, mappings or commit, so describe could not print them and ALTER (which rewrites every operation) lost them too; the service writer also emitted constants for AuthenticationTypes / AuthenticationMicroflow / CorsConfiguration / Documentation / PublicDocumentation with no carry; Resources and operation Parameters were registered with list marker 2 where Studio Pro writes 3", "file": "mdl/executor/cmd_published_rest.go, mdl/backend/modelsdk/published_rest_write.go, mdl/backend/modelsdk/integration_read.go, mdl/backend/modelsdk/export_level_carry.go, mdl/visitor/visitor_rest.go, model/types.go", "fix": "the executor derives operation parameters from the microflow as Studio Pro does (path name -> Path, object/list -> Body, System.HttpRequest/HttpResponse -> none, else Query; the microflow parameter's type), merged over the stored parameters per bound microflow parameter so a header/renamed/described parameter survives; mappings and commit flow AST -> model -> BSON and back, describe prints them (commit when not Yes) and notes parameters MDL cannot state; an unknown commit value is refused at exec and by check (MDL-REST03); create or modify carries summary/documentation/object handling of the restated operation; UpdatePublishedRestService carries the stored service-level keys MDL cannot state (keepStoredTopLevel); list markers measured from TestApp", "test": "mdl/executor/cmd_published_rest_params_test.go; mdl/backend/modelsdk/published_rest_write_test.go TestCreatePublishedRestService_WritesParametersAndBindings, TestWithStoredTopLevel; mdl/roundtrip TestTestAppRoundTrip/published_rest_service_Services.OrdersRestApi (allowlist entry struck); mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl (TestApp copy: old binary 16 mx check errors, fixed 0, describe->exec Unchanged twice)", "insight": "A clause that parses and is then ignored is worse than a parse error: the grammar advertised import/export mapping for months while the writer hard-coded them empty. The fastest witness was the round-trip harness's own allowlist entry for the one Studio Pro published REST service in TestApp: removing it printed the whole loss set (bindings, parameter types, markers, authentication) in one diff. Studio Pro's metamodel (ped_get_schema over the MCP tunnel) gave the enum values and defaults: Commit defaults to No there, while mxcli keeps writing Yes when the clause is absent so existing scripts do not churn, and describe prints commit whenever it is not Yes."} diff --git a/.claude/skills/fix-issue/findings/modelsdk.jsonl b/.claude/skills/fix-issue/findings/modelsdk.jsonl index e55dc06ee..3cdc21513 100644 --- a/.claude/skills/fix-issue/findings/modelsdk.jsonl +++ b/.claude/skills/fix-issue/findings/modelsdk.jsonl @@ -24,3 +24,4 @@ {"area":"modelsdk/mpr","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` (data view over a nanoflow the project lacks, 11.13.0) reported \"Altered page\"; `mxcli docker check` then could not LOAD the project: ArgumentNullException setting 'Attribute' of an Attribute in a Page","cause":"The bare-AttributeRef refusal (#678) lived in encodePage/encodeSnippet only. ALTER PAGE patches the stored BSON in pagemutator and saves via UpdateRawUnit, never passing the encoder; with no entity in scope the pluggable-widget template-parameter builder (widgetobj) writes the name as given, so DomainModels$AttributeRef{Attribute:\"ImageB64\"} reached disk","file":"modelsdk/canon/attributeref.go; modelsdk/mpr/writer_core.go (updateUnit, insertUnit)","insight":"A guard placed in one encoder covers one write path; the page family has at least four (encodePage/Snippet, pagemutator Save, widget sync apply, layout/template raw writes). Put an unloadable-shape refusal at the writer beside DuplicateElementIDError, as that one already argued. Measured before refusing stored refs too: 73 of 73 AttributeRefs across all 374 units of a stock 11.13 project are qualified (71 page, 1 snippet, 1 page template, none elsewhere) — a stored bare one cannot have come from Studio Pro, so refusing ALL bare refs (not only new ones) blocks nothing legitimate. The textbox path does NOT reproduce it: attributeRefToGen nulls a bare name (a silent binding drop instead); the pluggable/column template builders are the ones that write it verbatim. The test goes through the real mutator + writer on the expr-checker fixture (InsertColumns with a bare CaptionParams ref).","refs":["#678"]} {"area": "modelsdk/widgets", "date": "2026-09-26", "symptom": "CE0463 \"The definition of this widget has changed\" on every page carrying a pluggable widget built from its .mpk whose action properties declare `` (Signature 2.1.0, Calendar 2.6.0 on 11.12.2) — even with no action configured. `mx update-widgets` clears it", "cause": "The .mpk parser had no field for `` (modelsdk/widgets/mpk/mpk.go xmlProperty/PropertyDef), and createDefaultValueType hardcoded `ActionVariables: [2]`; reconcileValueTypesFromMPK never touched the list. The typed gen class (CustomWidgets$WidgetActionVariable) existed but the map-based template pipeline never fed it", "file": "modelsdk/widgets/mpk/mpk.go (ActionVariable, toActionVariables), modelsdk/widgets/augment.go (buildActionVariablesArray, actionVariablesMatch, createDefaultValueType, reconcileValueTypesFromMPK); tests actionvariables_test.go in both packages; example mdl-examples/bug-tests/1200-mpk-action-variables.mdl", "insight": "Third instance of the same shape after #716 (onChange) and #956 (defaultType): a widget.xml attribute/element that is part of the DEFINITION, never parsed, written as its empty default. Cheapest audit: diff every key Studio Pro stores on a WidgetValueType in the embedded templates against what createDefaultValueType derives from the .mpk — the embedded combobox.json already held the correct ActionVariables entry, i.e. the oracle was in the repo. `sdk/widgets/augment.go` has no importers; the live BSON path is modelsdk/widgets. When reconciling a list from the .mpk, rewrite only on disagreement, or an agreeing template's entry $IDs churn — the Combobox augment test is the no-change control (it fails if the rewrite is unconditional).", "refs": ["mendixlabs/mxcli#1200", "mendixlabs/mxcli#956", "#716"], "ce": ["CE0463"]} {"area": "modelsdk/canon", "date": "2026-09-26", "symptom": "A page rewrite still loses translations despite CarryTranslations: an empty caption's en_US '' vanishes (Texts$Text with no items), and a label's nl_NL 'Gebruikers' is dropped because its English 'Account Overview' is also the page title's", "cause": "Positional pairing needs the whole document's text paths unchanged — one DataGrid2 rebuilt from its template breaks that. Source pairing keys on (language, text): (en_US, '') is ambiguous on any real page, a shared English source is ambiguous, and a rebuilt EMPTY text has no translation to look up by at all", "file": "modelsdk/canon/translations.go", "insight": "Address a text by the named element that owns it — ($Type, Name, path from the element) — because a widget Name is unique per document. Exact only where the path from the element crosses no list index (a rebuilt pluggable widget reorders Properties; pairing there moves a translation onto the wrong property) and where the address occurs once in each document. Order: positional when the shape is unchanged, then owning element, then source", "refs": ["ako/mxcli#705"]} +{"date": "2026-10-01", "area": "modelsdk", "symptom": "mendixlabs/mxcli#849: `mxcli exec` writes the .mpr while Studio Pro has the project open; the write succeeds, mx check passes, and Studio Pro's next save silently discards it. No warning, no error.", "cause": "No write path looked for Studio Pro at all; the rule 'close Studio Pro first' existed only in prose (README, docs-site, skills).", "fix": "modelsdk/mpr/studiopro_lock.go: Writer.guardWrite refuses every write that would reach storage (updateUnit and WriteTransaction.WriteUnit after no-op elision, insertUnit, deleteUnit, MoveUnit, UpdateUnitContainer) with StudioProOpenError while `.mpr.lock` (matched case-insensitively) is beside the .mpr; `exec --force` / MXCLI_ALLOW_STUDIO_PRO_OPEN=1 override. Reads and elided no-op writes are never refused.", "insight": "Studio Pro's own generated .gitignore is the evidence for the signal and its spelling: it lists `testapp.mpr.lock` (lower-cased) beside `TestApp.mpr`, plus `mprcontents/mprjournal*`. Guarding at the storage layer after elision, not at command level, keeps twice-exec a no-op instead of an error and covers every command that writes.", "issue": "mendixlabs/mxcli#849", "file": "modelsdk/mpr/studiopro_lock.go"} diff --git a/.claude/skills/mendix/check-syntax/SKILL.md b/.claude/skills/mendix/check-syntax/SKILL.md index 235a9f6ac..2a45fb7e0 100644 --- a/.claude/skills/mendix/check-syntax/SKILL.md +++ b/.claude/skills/mendix/check-syntax/SKILL.md @@ -625,7 +625,9 @@ does not hot-reload when an external process changes the file. So after `mxcli e - `ped_read_document` / `ped_check_errors` will show the **stale** pre-exec model until Studio Pro re-scans — call `refresh_project` first (or reload the project in the UI). - **Hazard:** if Studio Pro later saves on its own, it overwrites mxcli's disk write with - its in-memory copy, silently discarding your MDL changes. + its in-memory copy, silently discarding your MDL changes. So a file-based write is + **refused** while Studio Pro's `.mpr.lock` is beside the `.mpr`; `exec --force` + (or `MXCLI_ALLOW_STUDIO_PRO_OPEN=1`) overrides it, e.g. for a lock left by a crash. **Safest practice:** don't keep the same project open-and-saving in Studio Pro while mxcli writes it. Either close (or don't save in) Studio Pro during MDL authoring, or diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a9513a23..66b426022 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A published REST operation gets its query and body parameters, its mappings and its commit option** (ako/mxcli#571, mendixlabs/mxcli#1206) — `create published rest service` wrote only the path's `{name}` placeholders, each as a String, so a query or body microflow parameter failed `mx check` with CE0350 and an Integer `{id}` with CE6539; `import mapping`, `export mapping` and `commit` on an operation parsed and were thrown away (CE0350 on the body, CE0354 on a microflow returning an object). The parameters are now derived from the microflow as Studio Pro derives them — named in the path: path parameter; an object or a list: the body; `System.HttpRequest` / `HttpResponse`: none; anything else: query — each with the microflow parameter's type, and the bindings are written. `describe` prints the bindings (`commit` when it is not `Yes`, the value written without the clause) and names in a comment a parameter MDL cannot state (a header, a renamed or described parameter), which `create or modify` and `alter` keep. An unknown commit option is refused by `exec` and `check` (**MDL-REST03**). Executing the `describe` output of a Studio Pro service used to clear its mappings, retype its Integer path parameters, drop its body parameter, set Commit to Yes and turn its Basic and Session authentication off — measured on TestApp's `Services.OrdersRestApi`, 0 `mx check` errors before, 5 after; it now writes nothing. - **`create or modify` of a flow can change an activity's notes, and matches an explicit `on error rollback`** (ako/mxcli#859, part) — a statement that added, reworded or took off an `@annotation` on an activity (or dropped an annotated activity) was refused under the `mdl 1;` header ("the annotations on the replaced … change"; "… carries an annotation, which would be left behind") on every run, and rebuilt the whole flow without it. The stored notes of that activity are now replaced by the ones the statement states; a note shared with another activity (one `describe` gives an `id:`) is still refused. This is what made the Studio Pro-authored PedApp nanoflows `ACT_Feedback_TriggerScreenshotMode` and `ACT_Feedback_UploadImage` unrunnable when their plain description was put under `mdl 1;`: a note's `\r\n` is two characters under mdl 1, so it is a real change, now written once. Separately, `commit $E on error rollback` (any activity with a bare `on error rollback`, which `describe` never prints because it is the stored default) did not match its own stored activity, so it was dropped and written again whenever anything else in the flow changed. - **`describe microflow` prints a Show Page action's title override** (ako/mxcli#869) — `show page M.P with title = '…'` used to describe without its override, so describe → exec dropped it, and taking the override out of a `create or modify microflow` reported "Unchanged" and wrote nothing, under mdl 0 and mdl 1. - **Re-running a `call web service` statement unchanged writes nothing** (ako/mxcli#861) — the call's activity was re-spliced on every run, because its stored raw document holds the element IDs each build mints and was compared byte for byte; it is now compared with those IDs set aside. A stored call that lacks a key mxcli's structured form writes (such as `ErrorHandlingType`), or that refers to its service by ID, now describes as `call web service raw '…'` instead of a structured form that would write a different document. diff --git a/cmd/mxcli/cmd_exec.go b/cmd/mxcli/cmd_exec.go index d50f5a2d8..2300d1ce8 100644 --- a/cmd/mxcli/cmd_exec.go +++ b/cmd/mxcli/cmd_exec.go @@ -11,6 +11,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/executor" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/visitor" + mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" "github.com/spf13/cobra" ) @@ -35,6 +36,13 @@ makes a partially-applied domain script re-runnable — the already-applied statements (e.g. "attribute already exists") error individually while the not- yet-applied ones still run — without a failure masking later work. +A write is refused while Studio Pro has the project open (its .mpr.lock +is beside the .mpr): Studio Pro does not reload the model from disk, and its next +save would silently discard the change. Close the project in Studio Pro, or route +writes through it with --mcp. --force writes anyway (for a lock left behind by a +crash); MXCLI_ALLOW_STUDIO_PRO_OPEN=1 does the same for every command. Reads, and +re-running a script whose statements change nothing, are never refused. + Pass "-" as the file to read the script from standard input, so MDL can be piped or written inline as a heredoc without a temporary file. @@ -53,6 +61,13 @@ Example: projectPath, _ := cmd.Flags().GetString("project") continueOnError, _ := cmd.Flags().GetBool("continue-on-error") skipCheck, _ := cmd.Flags().GetBool("no-check") + if force, _ := cmd.Flags().GetBool("force"); force { + mmpr.AllowWritesWhileStudioProOpen = true + if lock, _ := mmpr.StudioProLockFile(projectPath); lock != "" { + fmt.Fprintf(os.Stderr, "Warning: Studio Pro appears to have this project open (%s); writing anyway (--force). "+ + "Studio Pro's next save will discard these changes unless the project is closed or reloaded first.\n", lock) + } + } depPolicy := deprecationPolicy(cmd) // Read the script (a path, or "-" for stdin) @@ -222,6 +237,8 @@ Example: func init() { execCmd.Flags().Bool("no-check", false, "Skip the pre-flight semantic checks and apply the script even if mxcli check would report errors") + execCmd.Flags().Bool("force", false, + "Write even though Studio Pro appears to have the project open (its .mpr.lock is present) — e.g. a lock left behind by a crash") execCmd.Flags().Bool("continue-on-error", false, "Run every statement, reporting each failure instead of halting at the first (exits non-zero if any failed) — makes a partially-applied script re-runnable") } diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index 13be374c1..f09c2aaf7 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -318,9 +318,18 @@ func init() { Keywords: []string{ "create published rest", "publish rest", "rest resource", "rest operation", "microflow", "path parameter", + "query parameter", "body parameter", "import mapping", "export mapping", "commit", "grant access", "revoke access", }, - Syntax: "CREATE [OR MODIFY] PUBLISHED REST SERVICE Module.Name (\n Path: 'rest/api/v1',\n Version: '1.0.0',\n ServiceName: 'My API'\n)\n{\n RESOURCE 'name' {\n GET '' MICROFLOW Module.GetAll;\n GET '{id}' MICROFLOW Module.GetById;\n POST '' MICROFLOW Module.Create;\n }\n};\n\nALTER PUBLISHED REST SERVICE Module.Name SET Version = '2.0.0';\nALTER PUBLISHED REST SERVICE Module.Name ADD RESOURCE 'items' { ... };\nALTER PUBLISHED REST SERVICE Module.Name DROP RESOURCE 'legacy';\nDROP PUBLISHED REST SERVICE Module.Name;", + Syntax: "CREATE [OR MODIFY] PUBLISHED REST SERVICE Module.Name (\n Path: 'rest/api/v1',\n Version: '1.0.0',\n ServiceName: 'My API'\n)\n{\n RESOURCE 'name' {\n GET '' MICROFLOW Module.GetAll;\n GET '{id}' MICROFLOW Module.GetById;\n POST '' MICROFLOW Module.Create\n [IMPORT MAPPING Module.IMM] [EXPORT MAPPING Module.EMM]\n [COMMIT Yes | YesWithoutEvents | No]; -- no COMMIT clause: Yes\n }\n};\n\n" + + "-- Operation parameters come from the microflow, as Studio Pro derives them:\n" + + "-- a parameter named in the path ('{id}') -> path parameter\n" + + "-- an object or a list -> the body\n" + + "-- System.HttpRequest / HttpResponse -> none (the request and response)\n" + + "-- anything else -> query parameter\n" + + "-- each with the microflow parameter's type. Create the microflow first.\n" + + "-- A header parameter, a renamed one or a description set in Studio Pro has\n" + + "-- no MDL spelling: describe notes it, CREATE OR MODIFY / ALTER keep it.\n\nALTER PUBLISHED REST SERVICE Module.Name SET Version = '2.0.0';\nALTER PUBLISHED REST SERVICE Module.Name ADD RESOURCE 'items' { ... };\nALTER PUBLISHED REST SERVICE Module.Name DROP RESOURCE 'legacy';\nDROP PUBLISHED REST SERVICE Module.Name;", Example: "mdl 1;\nCREATE PUBLISHED REST SERVICE Module.OrderAPI (\n Path: 'rest/orders/v1',\n Version: '1.0.0',\n ServiceName: 'Order API'\n)\n{\n RESOURCE 'orders' {\n GET '' MICROFLOW Module.GetAllOrders;\n GET '{id}' MICROFLOW Module.GetOrderById;\n POST '' MICROFLOW Module.CreateOrder;\n DELETE '{id}' MICROFLOW Module.DeleteOrder;\n }\n};\n\nGRANT ACCESS ON PUBLISHED REST SERVICE Module.OrderAPI\n TO Module.User, Module.Admin;", SeeAlso: []string{"rest", "rest.consumed"}, }) diff --git a/docs-site/src/examples/rest-integration.md b/docs-site/src/examples/rest-integration.md index 28dbdcdee..0f8c8309f 100644 --- a/docs-site/src/examples/rest-integration.md +++ b/docs-site/src/examples/rest-integration.md @@ -325,7 +325,37 @@ CREATE PUBLISHED REST SERVICE Module.OrderAPI ( }; ``` -**Operation paths:** Use empty string `''` for the root, `'{paramName}'` for path parameters. Do NOT start or end with `/`. Path parameters must match a microflow parameter name exactly (case-sensitive) — e.g., `'{id}'` requires the microflow to declare `$id: String`. +**Operation paths:** Use empty string `''` for the root, `'{paramName}'` for path parameters. Do NOT start or end with `/`. Path parameters must match a microflow parameter name exactly (case-sensitive) — `'{id}'` binds the microflow's `$id`, whatever its type. + +**Operation parameters** come from the microflow, the way Studio Pro derives them. Create the microflow before the service: + +| Microflow parameter | Operation parameter | +|---|---| +| named in the path (`'{id}'`) | a path parameter | +| an object or a list | the body | +| `System.HttpRequest`, `System.HttpResponse` | none — they are the request and the response | +| anything else | a query parameter | + +Each gets the microflow parameter's type. A header parameter, a renamed parameter or a description set in Studio Pro has no MDL spelling: `describe` notes it in a comment, and `CREATE OR MODIFY` / `ALTER` on that project keep it. + +**Mappings and commit:** a body that is not a file document needs an import mapping, and a microflow returning an object or a list needs an export mapping: + +```sql +mdl 1; +CREATE OR MODIFY PUBLISHED REST SERVICE Module.OrderAPI ( + Path: 'rest/orders/v1', + Version: '1.0.0', + ServiceName: 'Order API' +) +{ + RESOURCE 'orders' { + -- COMMIT: Yes (the default without the clause) | YesWithoutEvents | No + POST '' MICROFLOW Module.PRS_CreateOrder + IMPORT MAPPING Module.IMM_Order EXPORT MAPPING Module.EMM_Order + COMMIT YesWithoutEvents; + } +}; +``` ### Multiple Resources diff --git a/docs-site/src/reference/capabilities.md b/docs-site/src/reference/capabilities.md index 063d7038f..790bf5f53 100644 --- a/docs-site/src/reference/capabilities.md +++ b/docs-site/src/reference/capabilities.md @@ -142,7 +142,7 @@ Everything mxcli can do, organized by use case. |---|---|---| | Design properties (Atlas v3) | Requires Mendix 11.0+ | Use CSS classes on 10.x | | REST query parameters | Requires Mendix 11.0+ | Build query string manually on 10.x | -| Concurrent editing | Not supported | Close Studio Pro before mxcli writes | +| Concurrent editing | Not supported — a file-based write is refused while Studio Pro has the project open (`.mpr.lock` present) | Close Studio Pro before mxcli writes, or write through it with `--mcp`; `exec --force` overrides | | Widget template drift | CE0463 on version mismatch | MPK augmentation handles most cases | | Marketplace module update | Existing modules are reported, not updated in place | Update via Studio Pro (preserves local edits and entity IDs) | | 47 of 52 metamodel domains | Not yet implemented | REST, OData write, etc. pending | diff --git a/docs-site/src/tutorial/quickstart.md b/docs-site/src/tutorial/quickstart.md index 0e841f9ca..524871130 100644 --- a/docs-site/src/tutorial/quickstart.md +++ b/docs-site/src/tutorial/quickstart.md @@ -136,4 +136,4 @@ mxcli setup mxbuild -p your-app.mpr **"CGO not available"** -- mxcli uses pure Go SQLite. No C compiler needed. If you see CGO errors, ensure you're using the official pre-built binary or a `make build` from source. -**Project won't open in Studio Pro after changes** -- Close Studio Pro before running mxcli write commands, then reopen. See [F4 sync support](../appendixes/version-compatibility.md) for details. +**"refusing to write …: Studio Pro has this project open"** -- Close the project in Studio Pro before running mxcli write commands, then reopen it. Studio Pro does not reload the model from disk, so its next save would silently discard mxcli's changes. If Studio Pro is not running, the `.mpr.lock` was left behind by a crash: delete it, or pass `--force` to `exec`. diff --git a/mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl b/mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl new file mode 100644 index 000000000..600bff301 --- /dev/null +++ b/mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl @@ -0,0 +1,94 @@ +mdl 1; +-- ============================================================================ +-- ako/mxcli#571, mendixlabs/mxcli#1206: published REST operation parameters +-- and mapping bindings +-- ============================================================================ +-- +-- Before the fix only the path's {name} placeholders were written, each as a +-- String: the query and body parameters failed mx check with CE0350, an +-- Integer {id} with CE6539. The import mapping, export mapping and commit +-- clauses parsed and were thrown away, so a body that is not a file document +-- failed with CE0350 and a microflow returning an object with CE0354. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl -p app.mpr +-- mxcli docker check -p app.mpr -> 0 errors +-- mxcli exec (the same script again) -> writes nothing +-- +-- The operation parameters are derived from each microflow, as Studio Pro +-- derives them: a parameter named in the path is a path parameter, an object +-- or a list is the body, System.HttpRequest / HttpResponse are left out, and +-- anything else is a query parameter, each with the microflow parameter's type. + +create module RestQ; + +create persistent entity RestQ.Upload extends System.FileDocument ( + Label: String(100) +); + +create persistent entity RestQ.Item ( + Code: String(20), + Quantity: Integer +); + +create json structure RestQ.JSON_Item + sample '{"code": "A1", "quantity": 3}'; + +create import mapping RestQ.IMM_Item + with json structure RestQ.JSON_Item +{ + create RestQ.Item { + Code = code, + Quantity = quantity + } +}; + +create export mapping RestQ.EMM_Item + with json structure RestQ.JSON_Item +{ + RestQ.Item { + code = Code, + quantity = Quantity + } +}; + +create microflow RestQ.GetStatus ($orderNumber: String, $count: Integer, $httpRequest: System.HttpRequest) +returns String as $Result +begin + declare $Result String = $orderNumber + ':' + toString($count); + return $Result; +end; + +create microflow RestQ.GetById ($id: Integer, $verbose: Boolean) +returns String as $Result +begin + declare $Result String = toString($id); + return $Result; +end; + +create microflow RestQ.PutFile ($file: RestQ.Upload) +returns String as $Result +begin + declare $Result String = 'ok'; + return $Result; +end; + +create microflow RestQ.PostItem ($item: RestQ.Item) +returns RestQ.Item as $item +begin + return $item; +end; + +create published rest service RestQ.Orders ( + Path: 'rest/orders/v1', + Version: '1.0.0', + ServiceName: 'Orders' +) +{ + resource 'orders' { + get 'status' microflow RestQ.GetStatus; + get 'items/{id}' microflow RestQ.GetById; + post 'upload' microflow RestQ.PutFile; + post 'item' microflow RestQ.PostItem import mapping RestQ.IMM_Item export mapping RestQ.EMM_Item commit YesWithoutEvents; + } +}; diff --git a/mdl-examples/bug-tests/move-entity-503-preserves-storage-guids.mdl b/mdl-examples/bug-tests/move-entity-503-preserves-storage-guids.mdl index 898dec6bb..fd072be98 100644 --- a/mdl-examples/bug-tests/move-entity-503-preserves-storage-guids.mdl +++ b/mdl-examples/bug-tests/move-entity-503-preserves-storage-guids.mdl @@ -69,9 +69,9 @@ CREATE MODULE BugMoveGuid503B; -- -- Measured with a binary predating both fixes here, so it is a third defect on this -- command and not something these fixes introduced — but it is why this file keeps --- one endpoint of each pair put. Filed as ako/mxcli#628: MoveEntity scans --- AssociationsItems() and never CrossAssociationsItems(), so a cross-association --- that already exists is invisible to the move. +-- one endpoint of each pair put. Filed and fixed as ako/mxcli#628 (MoveEntity +-- never looked at CrossAssociationsItems()); the both-endpoints sequences are +-- exercised in move-entity-628-existing-cross-associations.mdl. -- Pair 1 — the TO side moves, so the cross-association STAYS in the source module -- and its qualified name does not change. diff --git a/mdl-examples/bug-tests/move-entity-628-existing-cross-associations.mdl b/mdl-examples/bug-tests/move-entity-628-existing-cross-associations.mdl new file mode 100644 index 000000000..c25d1eacb --- /dev/null +++ b/mdl-examples/bug-tests/move-entity-628-existing-cross-associations.mdl @@ -0,0 +1,58 @@ +mdl 1; +-- ============================================================================ +-- ako/mxcli#628 — MOVE ENTITY must see the cross-associations that already exist +-- ============================================================================ +-- +-- MoveEntity converted the plain associations of the moved entity and never +-- looked at existing cross-associations. Moving the SECOND endpoint of an +-- association to another module — the normal way to reorganise a module one +-- entity at a time — left a cross-association whose ParentPointer named an +-- element absent from its unit, and Studio Pro could no longer open the project: +-- +-- System.AggregateException: The given key '' was not present in the dictionary. +-- +-- Three shapes, each below: +-- 1. both endpoints end up in one module -> a plain association again; +-- 2. the FROM end moves on -> the cross-association travels with it; +-- 3. the TO end moves on -> the cross-association elsewhere is re-pointed. +-- +-- mxcli exec move-entity-628-existing-cross-associations.mdl -p app.mpr +-- mxcli docker check -p app.mpr +-- +-- Expected: every statement succeeds and the project checks clean. Afterwards +-- describe association Bug628B.Child_Parent; -> from Bug628B.Child to Bug628B.Parent +-- describe association Bug628E.C2_P2; -> from Bug628E.C2 to Bug628D.P2 +-- describe association Bug628C.C3_P3; -> from Bug628C.C3 to Bug628E.P3 +-- +-- The storage GUID carry through each conversion is asserted on Studio +-- Pro-authored content in mdl/backend/modelsdk/issue628_move_cross_assoc_test.go; +-- in a script like this one every element is mxcli-created (GUID == $ID), so a +-- lost GUID is not observable here. +-- ============================================================================ + +create module Bug628A; +create module Bug628B; +create module Bug628C; +create module Bug628D; +create module Bug628E; + +-- Shape 1 (the issue's reproduction). +create or modify persistent entity Bug628A.Parent ( Code: String(50) ); +create or modify persistent entity Bug628A.Child ( Descr: String(100) ); +create or modify association Bug628A.Child_Parent from Bug628A.Child to Bug628A.Parent type Reference; +move entity Bug628A.Parent to Bug628B; +move entity Bug628A.Child to Bug628B; + +-- Shape 2. +create or modify persistent entity Bug628C.P2 ( Code: String(50) ); +create or modify persistent entity Bug628C.C2 ( Descr: String(100) ); +create or modify association Bug628C.C2_P2 from Bug628C.C2 to Bug628C.P2 type Reference; +move entity Bug628C.P2 to Bug628D; +move entity Bug628C.C2 to Bug628E; + +-- Shape 3. +create or modify persistent entity Bug628C.P3 ( Code: String(50) ); +create or modify persistent entity Bug628C.C3 ( Descr: String(100) ); +create or modify association Bug628C.C3_P3 from Bug628C.C3 to Bug628C.P3 type Reference; +move entity Bug628C.P3 to Bug628D; +move entity Bug628D.P3 to Bug628E; diff --git a/mdl/backend/modelsdk/association_move_cross.go b/mdl/backend/modelsdk/association_move_cross.go new file mode 100644 index 000000000..eec264a5c --- /dev/null +++ b/mdl/backend/modelsdk/association_move_cross.go @@ -0,0 +1,242 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "fmt" + "strings" + + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/x/bsonx/bsoncore" + + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/modelsdk/element" + genDm "github.com/mendixlabs/mxcli/modelsdk/gen/domainmodels" + "github.com/mendixlabs/mxcli/modelsdk/meta" + mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// A MOVE ENTITY has to look at the cross-associations that already exist, not +// only at the plain associations it converts (ako/mxcli#628). A cross-association +// lives in the unit of its FROM entity, holds that entity by id (ParentPointer) +// and the TO entity by qualified name (Child). Moving entity E from S to T leaves +// three shapes that are not the same case: +// +// 1. Both endpoints end up in one module. The cross-association becomes a plain +// DomainModels$Association again in that module — the inverse of +// crossAssocRawFromAssoc. This is what moving the second endpoint of an +// association across, one entity at a time, produces. +// 2. E is the FROM entity and the TO entity is elsewhere. The cross-association +// travels with E to T (its ParentPointer would not resolve in S), so its +// qualified name changes S.X -> T.X. +// 3. E is the TO entity of a cross-association in ANOTHER module. That module's +// unit names `S.E`; it is re-pointed to `T.E`. +// +// Left alone, shape 1 and 2 leave a ParentPointer naming an element absent from +// its unit — Studio Pro fails to load the project ("The given key was not present +// in the dictionary") — and shape 3 leaves a dangling qualified name. Every +// conversion carries the stored document, GUID included: the runtime keys the +// association's data on that GUID (CLAUDE.md, "A GUID Is the Database's Identity"). + +// moveOwnCrossAssociations handles shapes 1 and 2: the cross-associations in the +// source unit whose FROM entity is the moved one. It removes them from sourceDM +// and adds them to targetDM, as plain associations when their TO entity lives in +// the target module. +func moveOwnCrossAssociations(sourceDM, targetDM *genDm.DomainModel, entityID model.ID, sourceModuleName, targetModuleName string) []types.MovedAssociation { + targetEntities := entityIDsByName(targetDM) + var out []types.MovedAssociation + var removeIdx []int + for i, el := range sourceDM.CrossAssociationsItems() { + ca, ok := el.(*genDm.CrossAssociation) + if !ok || string(ca.ParentRefID()) != string(entityID) { + continue + } + moved := types.MovedAssociation{ + Name: ca.Name(), + OldQualifiedName: sourceModuleName + "." + ca.Name(), + NewQualifiedName: targetModuleName + "." + ca.Name(), + } + mod, ent, _ := strings.Cut(ca.ChildQualifiedName(), ".") + if toID, ok := targetEntities[ent]; ok && mod == targetModuleName { + targetDM.AddAssociations(assocFromGenCrossAssoc(ca, toID)) + moved.SameModule = true + } else { + targetDM.AddCrossAssociations(cloneCrossAssoc(ca)) + } + removeIdx = append(removeIdx, i) + out = append(out, moved) + } + for i := len(removeIdx) - 1; i >= 0; i-- { + sourceDM.RemoveCrossAssociations(removeIdx[i]) + } + return out +} + +// convertIncomingCrossAssociations handles shape 1 seen from the other side: a +// cross-association in the TARGET unit whose TO entity is the moved one. Both +// endpoints are now in the target module, so it becomes a plain association +// there. Its qualified name does not change. +func convertIncomingCrossAssociations(targetDM *genDm.DomainModel, entityID model.ID, oldEntityQN, targetModuleName string) []types.MovedAssociation { + var out []types.MovedAssociation + var removeIdx []int + for i, el := range targetDM.CrossAssociationsItems() { + ca, ok := el.(*genDm.CrossAssociation) + if !ok || ca.ChildQualifiedName() != oldEntityQN { + continue + } + targetDM.AddAssociations(assocFromGenCrossAssoc(ca, string(entityID))) + removeIdx = append(removeIdx, i) + qn := targetModuleName + "." + ca.Name() + out = append(out, types.MovedAssociation{Name: ca.Name(), OldQualifiedName: qn, NewQualifiedName: qn, SameModule: true}) + } + for i := len(removeIdx) - 1; i >= 0; i-- { + targetDM.RemoveCrossAssociations(removeIdx[i]) + } + return out +} + +// repointCrossAssociationsElsewhere handles shape 3: every cross-association in a +// domain model other than the source and target units that names the moved entity +// as its TO entity is re-pointed to the entity's new qualified name. +func (b *Backend) repointCrossAssociationsElsewhere(sourceDMID, targetDMID model.ID, oldEntityQN, newEntityQN string) error { + dms, err := b.ListDomainModels() + if err != nil { + return fmt.Errorf("list domain models: %w", err) + } + for _, info := range dms { + if info.ID == sourceDMID || info.ID == targetDMID || string(info.ID) == meta.SystemDomainModelID { + continue + } + gdm, err := b.loadDomainModelGen(info.ID) + if err != nil { + return err + } + changed := false + for _, el := range gdm.CrossAssociationsItems() { + if ca, ok := el.(*genDm.CrossAssociation); ok && ca.ChildQualifiedName() == oldEntityQN { + ca.SetChildQualifiedName(newEntityQN) + changed = true + } + } + if changed { + if err := b.persistDM(info.ID, gdm); err != nil { + return err + } + } + } + return nil +} + +func entityIDsByName(dm *genDm.DomainModel) map[string]string { + out := map[string]string{} + for _, el := range dm.EntitiesItems() { + if e, ok := el.(*genDm.Entity); ok { + out[e.Name()] = string(e.ID()) + } + } + return out +} + +// cloneCrossAssoc re-homes a stored cross-association in another unit unchanged: +// a clean element over the stored bytes, so the encoder passes the whole document +// — GUID included — through verbatim. +func cloneCrossAssoc(ca *genDm.CrossAssociation) *genDm.CrossAssociation { + if raw := ca.Raw(); raw != nil { + out := genDm.NewCrossAssociation() + out.SetRaw(raw) + out.InitFromRaw(raw) + out.SetID(ca.ID()) + return out + } + return ca +} + +// assocFromGenCrossAssoc converts a cross-association back into the plain +// association both of whose endpoints now share its unit; childID is the TO +// entity's element id. Like the forward conversion it prefers a raw transform of +// the stored document, so the GUID and every untouched property survive; the +// property build is the fallback for a cross-association never persisted. +func assocFromGenCrossAssoc(ca *genDm.CrossAssociation, childID string) *genDm.Association { + if raw, ok := assocRawFromCrossAssoc(ca, childID); ok { + out := genDm.NewAssociation() + out.SetRaw(raw) + out.InitFromRaw(raw) + out.SetID(ca.ID()) + return out + } + + out := genDm.NewAssociation() + out.SetID(ca.ID()) + out.SetName(ca.Name()) + out.SetDocumentation(ca.Documentation()) + out.SetExportLevel(orDefault(ca.ExportLevel(), "Hidden")) + out.SetParentID(ca.ParentRefID()) + out.SetChildID(element.ID(childID)) + out.SetType(ca.Type()) + out.SetOwner(ca.Owner()) + out.SetStorageFormat(orDefault(ca.StorageFormat(), "Column")) + out.SetParentConnection(domainmodel.DefaultParentConnection) + out.SetChildConnection(domainmodel.DefaultChildConnection) + pdb, cdb := "DeleteMeButKeepReferences", "DeleteMeButKeepReferences" + if odb, ok := ca.DeleteBehavior().(*genDm.AssociationDeleteBehavior); ok { + pdb, cdb = orDefault(odb.ParentDeleteBehavior(), pdb), orDefault(odb.ChildDeleteBehavior(), cdb) + } + out.SetDeleteBehavior(deleteBehaviorToGen(pdb, cdb)) + if src, ok := ca.Source().(*genDm.OqlViewAssociationSource); ok && src != nil { + out.SetSource(oqlViewAssociationSourceToGen(src.Reference())) + } + assignID(out.DeleteBehavior()) + return out +} + +// assocRawFromCrossAssoc is the inverse of crossAssocRawFromAssoc: $Type becomes +// DomainModels$Association, Child (a qualified name) becomes ChildPointer (the TO +// entity's 16-byte id, resolvable now that it shares the unit), and the two +// connection points a plain association declares — the line's on-canvas anchors — +// are added with the defaults mxcli and Studio Pro write for a new association. +// Everything else, the GUID above all, passes through. The key set is the one +// `generated/metamodel` declares for DomainModelsAssociation. +func assocRawFromCrossAssoc(ca *genDm.CrossAssociation, childID string) (bson.Raw, bool) { + raw := ca.Raw() + if raw == nil { + return nil, false + } + elems, err := bsoncore.Document(raw).Elements() + if err != nil { + return nil, false + } + out := make(bson.D, 0, len(elems)+2) + child, parent := false, false + for _, e := range elems { + switch e.Key() { + case "$Type": + out = append(out, bson.E{Key: "$Type", Value: "DomainModels$Association"}) + case "Child": + out = append(out, + bson.E{Key: "ChildConnection", Value: domainmodel.DefaultChildConnection}, + bson.E{Key: "ChildPointer", Value: mmpr.IDToBsonBinary(childID)}) + child = true + case "ParentPointer": + v := e.Value() + out = append(out, + bson.E{Key: "ParentConnection", Value: domainmodel.DefaultParentConnection}, + bson.E{Key: "ParentPointer", Value: bson.RawValue{Type: bson.Type(v.Type), Value: v.Data}}) + parent = true + case "ChildConnection", "ParentConnection": + // Not declared on a cross-association; never carry a stray one twice. + default: + v := e.Value() + out = append(out, bson.E{Key: e.Key(), Value: bson.RawValue{Type: bson.Type(v.Type), Value: v.Data}}) + } + } + if !child || !parent { + return nil, false + } + b, err := bson.Marshal(out) + if err != nil { + return nil, false + } + return bson.Raw(b), true +} diff --git a/mdl/backend/modelsdk/association_move_write.go b/mdl/backend/modelsdk/association_move_write.go index 4f1e88384..2e84dbd91 100644 --- a/mdl/backend/modelsdk/association_move_write.go +++ b/mdl/backend/modelsdk/association_move_write.go @@ -368,6 +368,15 @@ func (b *Backend) MoveEntity(entity *domainmodel.Entity, sourceDMID, targetDMID sourceDM.RemoveAssociations(removeIdx[i]) } + // The cross-associations that already exist (ako/mxcli#628): the ones the moved + // entity is the FROM end of travel with it — or become plain associations again + // when their TO entity is already in the target — and the ones in the target + // that point at it become plain associations there. See association_move_cross.go. + oldEntityQN := sourceModuleName + "." + entity.Name + newEntityQN := targetModuleName + "." + entity.Name + converted = append(converted, moveOwnCrossAssociations(sourceDM, targetDM, entity.ID, sourceModuleName, targetModuleName)...) + converted = append(converted, convertIncomingCrossAssociations(targetDM, entity.ID, oldEntityQN, targetModuleName)...) + // Rewrite the moved entity's module-qualified refs (view source + validations). oldPrefix, newPrefix := sourceModuleName+".", targetModuleName+"." if entity.Source == "DomainModels$OqlViewEntitySource" && strings.HasPrefix(entity.SourceDocumentRef, oldPrefix) { @@ -390,9 +399,9 @@ func (b *Backend) MoveEntity(entity *domainmodel.Entity, sourceDMID, targetDMID // // An ATTRIBUTE reference always follows the entity, so its module prefix is // rewritten unconditionally. An ASSOCIATION reference is rewritten only for the - // associations this move actually sent to the target module: a pre-existing - // cross-association whose parent is the moved entity is not in the conversion - // list and does not travel, so a blanket prefix swap would break it. + // associations this move actually sent to the target module — a converted one, + // or a pre-existing cross-association the moved entity is the FROM end of + // (#628) — never by a blanket prefix swap. assocRenames := make(map[string]string, len(converted)) for _, m := range converted { if m.Moved() { @@ -438,5 +447,10 @@ func (b *Backend) MoveEntity(entity *domainmodel.Entity, sourceDMID, targetDMID if err := b.persistDM(targetDMID, targetDM); err != nil { return nil, fmt.Errorf("MoveEntity: persist target: %w", err) } + // A cross-association in any OTHER module that names the moved entity is + // re-pointed; nothing else in the move rewrites that module's unit (#628). + if err := b.repointCrossAssociationsElsewhere(sourceDMID, targetDMID, oldEntityQN, newEntityQN); err != nil { + return nil, fmt.Errorf("MoveEntity: re-point cross-associations: %w", err) + } return converted, nil } diff --git a/mdl/backend/modelsdk/domainmodel_alter.go b/mdl/backend/modelsdk/domainmodel_alter.go index ccd3abf86..bcc65de6e 100644 --- a/mdl/backend/modelsdk/domainmodel_alter.go +++ b/mdl/backend/modelsdk/domainmodel_alter.go @@ -774,6 +774,14 @@ func (b *Backend) UpdateAttribute(domainModelID, entityID model.ID, attr *domain next := attributeToGen(attr, entityIsExternal(ent)) next.SetID(items[idx].ID()) + // attributeToGen returns raw == nil, which the codec reads as a NEW element + // and mints GUID = $ID for — the runtime keys mendixsystem$attribute.id on + // that GUID and would drop the column (ako/mxcli#627, the #1119 class). + // Attribute -> Attribute keeps the $Type, so carrying the stored raw bytes + // is enough; no raw transform is needed. + if stored, ok := items[idx].(*genDm.Attribute); ok { + carryStoredAttribute(next, stored) + } // The generated list offers only Append and Remove, so an in-place replace // means rebuilding it. Order is worth the rebuild: it is the order Studio diff --git a/mdl/backend/modelsdk/domainmodel_child_identity.go b/mdl/backend/modelsdk/domainmodel_child_identity.go index b07c04bb9..ca0d91a38 100644 --- a/mdl/backend/modelsdk/domainmodel_child_identity.go +++ b/mdl/backend/modelsdk/domainmodel_child_identity.go @@ -82,15 +82,7 @@ func carryAttributeIdentity(ge, orig *genDm.Entity, entity *domainmodel.Entity) claimed := make(map[string]bool, len(storedByID)) carry := func(ga, sa *genDm.Attribute) { - ga.SetID(sa.ID()) - ga.SetRaw(sa.Raw()) - // attributeToGen sets ExportLevel "Hidden", and a property the rebuild - // sets wins over the carried raw bytes — so without this an API attribute - // became Hidden on every rewrite of its entity (ako/mxcli#801). The - // semantic attribute has no export level to take it from. - if lvl := sa.ExportLevel(); lvl != "" { - ga.SetExportLevel(lvl) - } + carryStoredAttribute(ga, sa) claimed[string(sa.ID())] = true } @@ -170,3 +162,22 @@ func carryIndexIdentity(ge, orig *genDm.Entity, entity *domainmodel.Entity) { claimed[string(si.ID())] = true } } + +// carryStoredAttribute makes the rebuilt attribute ga the stored attribute sa as +// far as identity goes: the stored $ID and raw bytes (so the GUID passes through +// instead of being re-minted as $ID — #1119, ako/mxcli#627) and the stored export +// level. Both rebuild sites use it: the entity rewrite (carryAttributeIdentity) +// and the single-attribute rewrite (Backend.UpdateAttribute). +func carryStoredAttribute(ga, sa *genDm.Attribute) { + ga.SetID(sa.ID()) + if raw := sa.Raw(); raw != nil { + ga.SetRaw(raw) + } + // attributeToGen sets ExportLevel "Hidden", and a property the rebuild + // sets wins over the carried raw bytes — so without this an API attribute + // became Hidden on every rewrite of its entity (ako/mxcli#801). The + // semantic attribute has no export level to take it from. + if lvl := sa.ExportLevel(); lvl != "" { + ga.SetExportLevel(lvl) + } +} diff --git a/mdl/backend/modelsdk/export_level_carry.go b/mdl/backend/modelsdk/export_level_carry.go index 7f1d2c9d6..5bea6df5c 100644 --- a/mdl/backend/modelsdk/export_level_carry.go +++ b/mdl/backend/modelsdk/export_level_carry.go @@ -4,6 +4,8 @@ package modelsdkbackend import ( "fmt" + "sort" + "strings" "go.mongodb.org/mongo-driver/v2/x/bsonx/bsoncore" ) @@ -97,3 +99,69 @@ func withExportLevel(doc []byte, lvl string) ([]byte, error) { } return out, nil } + +// keepStoredTopLevel returns contents with each top-level key in keys set to +// the value the stored unit holds, for a document kind whose statement cannot +// state those properties and whose writer therefore emits constants for them. +// A key the stored unit lacks is left as written; a key only the stored unit +// has is inserted before the first written key that sorts after it, the order +// Studio Pro writes. +func (b *Backend) keepStoredTopLevel(unitID string, contents []byte, keys []string) ([]byte, error) { + if b.reader == nil || unitID == "" || len(contents) == 0 { + return contents, nil + } + stored, err := b.reader.GetRawUnitBytes(unitID) + if err != nil || len(stored) == 0 { + return contents, nil + } + return withStoredTopLevel(contents, stored, keys) +} + +// withStoredTopLevel is keepStoredTopLevel on bytes. +func withStoredTopLevel(doc, stored []byte, keys []string) ([]byte, error) { + carry := map[string]bsoncore.Value{} + for _, k := range keys { + if v, err := bsoncore.Document(stored).LookupErr(k); err == nil { + carry[k] = v + } + } + if len(carry) == 0 { + return doc, nil + } + elems, err := bsoncore.Document(doc).Elements() + if err != nil { + return nil, fmt.Errorf("carry stored properties: %w", err) + } + present := map[string]bool{} + for _, el := range elems { + present[el.Key()] = true + } + var missing []string + for k := range carry { + if !present[k] { + missing = append(missing, k) + } + } + sort.Strings(missing) + idx, out := bsoncore.AppendDocumentStart(nil) + for _, el := range elems { + key := el.Key() + for len(missing) > 0 && missing[0] < key && !strings.HasPrefix(key, "$") { + out = bsoncore.AppendValueElement(out, missing[0], carry[missing[0]]) + missing = missing[1:] + } + if v, ok := carry[key]; ok { + out = bsoncore.AppendValueElement(out, key, v) + continue + } + out = append(out, el...) + } + for _, k := range missing { + out = bsoncore.AppendValueElement(out, k, carry[k]) + } + out, err = bsoncore.AppendDocumentEnd(out, idx) + if err != nil { + return nil, fmt.Errorf("carry stored properties: %w", err) + } + return out, nil +} diff --git a/mdl/backend/modelsdk/integration_read.go b/mdl/backend/modelsdk/integration_read.go index 0ce146a12..49d759174 100644 --- a/mdl/backend/modelsdk/integration_read.go +++ b/mdl/backend/modelsdk/integration_read.go @@ -10,6 +10,7 @@ import ( "github.com/mendixlabs/mxcli/modelsdk/element" genBe "github.com/mendixlabs/mxcli/modelsdk/gen/businessevents" genDb "github.com/mendixlabs/mxcli/modelsdk/gen/databaseconnector" + genDT "github.com/mendixlabs/mxcli/modelsdk/gen/datatypes" genExportMappings "github.com/mendixlabs/mxcli/modelsdk/gen/exportmappings" genImportMappings "github.com/mendixlabs/mxcli/modelsdk/gen/importmappings" genOp "github.com/mendixlabs/mxcli/modelsdk/gen/odatapublish" @@ -427,11 +428,21 @@ func (b *Backend) ListPublishedRestServices() ([]*model.PublishedRestService, er continue } operation := &model.PublishedRestOperation{ - Path: op.Path(), - HTTPMethod: op.HttpMethod(), - Summary: op.Summary(), - Microflow: op.MicroflowQualifiedName(), - Deprecated: op.Deprecated(), + Path: op.Path(), + HTTPMethod: op.HttpMethod(), + Summary: op.Summary(), + Microflow: op.MicroflowQualifiedName(), + Deprecated: op.Deprecated(), + Documentation: op.Documentation(), + ImportMapping: op.ImportMappingQualifiedName(), + ExportMapping: op.ExportMappingQualifiedName(), + Commit: op.Commit(), + ObjectHandlingBackup: op.ObjectHandlingBackup(), + } + for _, pEl := range op.ParametersItems() { + if p, ok := pEl.(*genRest.RestOperationParameter); ok { + operation.OperationParameters = append(operation.OperationParameters, publishedRestParameterFromGen(p)) + } } operation.ID = model.ID(op.ID()) operation.TypeName = "Rest$PublishedRestServiceOperation" @@ -712,3 +723,25 @@ func rawQueryTypeName(q *genDb.DatabaseQuery) string { v, _ := q.Raw().Lookup(dbconnector.TypeKey).StringValueOK() return v } + +// publishedRestParameterFromGen reads one Rest$RestOperationParameter. +func publishedRestParameterFromGen(p *genRest.RestOperationParameter) *model.PublishedRestOperationParameter { + param := &model.PublishedRestOperationParameter{ + Name: p.Name(), + ParameterType: p.ParameterType(), + MicroflowParameter: p.MicroflowParameterQualifiedName(), + Description: p.Description(), + } + if t := p.Type(); t != nil { + param.DataType = strings.TrimSuffix(strings.TrimPrefix(t.TypeName(), "DataTypes$"), "Type") + switch g := t.(type) { + case *genDT.ObjectType: + param.QualifiedName = g.EntityQualifiedName() + case *genDT.ListType: + param.QualifiedName = g.EntityQualifiedName() + case *genDT.EnumerationType: + param.QualifiedName = g.EnumerationQualifiedName() + } + } + return param +} diff --git a/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go b/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go new file mode 100644 index 000000000..f73ef0c0b --- /dev/null +++ b/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "os" + "path/filepath" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// copyPedApp copies the Studio Pro-authored PedApp fixture into a temp dir. Its +// elements have GUID != $ID, which is the only subject on which a re-minted +// storage GUID is observable (CLAUDE.md, "A GUID Is the Database's Identity"). +func copyPedApp(t *testing.T) string { + t.Helper() + dst := t.TempDir() + if err := os.CopyFS(dst, os.DirFS("../../../testdata/pedapp")); err != nil { + t.Fatalf("copy PedApp fixture: %v", err) + } + return filepath.Join(dst, "PedApp.mpr") +} + +// TestIssue627_UpdateAttributePreservesGUID guards ako/mxcli#627: the +// AttributeModifier path of the fluent API (Backend.UpdateAttribute) rebuilt the +// attribute from the semantic model with raw == nil, so the codec minted +// GUID = $ID — the #1119 data-loss class, the column dropped on the next deploy. +// The #1119 write guard refused it, which left the API unusable on any Studio +// Pro-authored attribute. +// +// The subject is a PedApp string attribute whose stored GUID differs from its +// $ID; the comparison is against the GUID read before the write, never against a +// previous run (a re-mint is stable, so a second write is elided). +func TestIssue627_UpdateAttributePreservesGUID(t *testing.T) { + proj := copyPedApp(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + + dmID, ent, attr := studioProStringAttribute(t, b) + before := attributeGUIDs(t, b, dmID, ent.ID) + ids := attributeIDs(t, b, dmID, ent.ID) + want := before[attr.Name] + if want == "" || want == ids[attr.Name] { + t.Fatalf("subject %s.%s: GUID %q, $ID %q — need a stored GUID that differs from $ID", + ent.Name, attr.Name, want, ids[attr.Name]) + } + + st := attr.Type.(*domainmodel.StringAttributeType) + st.Length += 7 + if err := b.UpdateAttribute(dmID, ent.ID, attr); err != nil { + t.Fatalf("UpdateAttribute: %v", err) + } + if err := b.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + + b2 := New() + if err := b2.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b2.Disconnect() }) + + after := attributeGUIDs(t, b2, dmID, ent.ID) + if got := after[attr.Name]; got != want { + t.Errorf("attribute %s.%s: storage GUID changed %s -> %s", ent.Name, attr.Name, want, got) + } + // The siblings pass through as stored bytes; assert it so a future rebuild + // of the whole list is caught here too. + for name, g := range before { + if after[name] != g { + t.Errorf("sibling attribute %s: storage GUID changed %s -> %s", name, g, after[name]) + } + } + // And the edit itself landed — otherwise "GUID unchanged" proves nothing. + dm, err := b2.GetDomainModel(moduleOfDM(t, b2, dmID)) + if err != nil { + t.Fatalf("GetDomainModel: %v", err) + } + var gotLen int + for _, e := range dm.Entities { + if e.ID != ent.ID { + continue + } + for _, a := range e.Attributes { + if a.Name == attr.Name { + if s, ok := a.Type.(*domainmodel.StringAttributeType); ok { + gotLen = s.Length + } + } + } + } + if gotLen != st.Length { + t.Errorf("attribute %s.%s: length = %d after UpdateAttribute, want %d", ent.Name, attr.Name, gotLen, st.Length) + } +} + +// studioProStringAttribute picks the first string attribute with a non-zero +// length in a loadable, non-System domain model. +func studioProStringAttribute(t *testing.T, b *Backend) (model.ID, *domainmodel.Entity, *domainmodel.Attribute) { + t.Helper() + dms, err := b.ListDomainModels() + if err != nil { + t.Fatalf("ListDomainModels: %v", err) + } + for _, d := range dms { + if _, err := b.loadDomainModelGen(d.ID); err != nil { + continue + } + for _, e := range d.Entities { + for _, a := range e.Attributes { + if s, ok := a.Type.(*domainmodel.StringAttributeType); ok && s.Length > 0 { + return d.ID, e, a + } + } + } + } + t.Fatal("no string attribute in a loadable PedApp domain model") + return "", nil, nil +} + +func moduleOfDM(t *testing.T, b *Backend, dmID model.ID) model.ID { + t.Helper() + dms, err := b.ListDomainModels() + if err != nil { + t.Fatalf("ListDomainModels: %v", err) + } + for _, d := range dms { + if d.ID == dmID { + return d.ContainerID + } + } + t.Fatalf("domain model %s not found", dmID) + return "" +} diff --git a/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go b/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go new file mode 100644 index 000000000..180a4dc9a --- /dev/null +++ b/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go @@ -0,0 +1,305 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + genDm "github.com/mendixlabs/mxcli/modelsdk/gen/domainmodels" +) + +// TestIssue628_MoveEntitySeesExistingCrossAssociations guards ako/mxcli#628: +// MoveEntity converted the plain associations of the moved entity and never +// looked at the cross-associations that already existed. Moving the second +// endpoint of an association across — the normal way to reorganise a module one +// entity at a time — left a cross-association whose ParentPointer named an +// element absent from its unit, and Studio Pro could no longer load the project +// ("The given key was not present in the dictionary"). +// +// The issue's honest assertion is on the stored documents: after every sequence, +// no association pointer may name an element absent from the unit it lives in, +// and no cross-association may name an entity that is not where it says. The +// association's storage GUID must survive each conversion, which only a Studio +// Pro-authored subject (PedApp, GUID != $ID) can detect. +func TestIssue628_MoveEntitySeesExistingCrossAssociations(t *testing.T) { + type step struct { + moveFrom bool // move the FROM (parent) entity; otherwise the TO (child) entity + to int // index into the three other modules + } + for _, tc := range []struct { + name string + steps []step + // wantPlainIn is the module index (into others) whose unit must hold the + // association as a plain association afterwards; -1 means it stays a + // cross-association. + wantPlainIn int + // wantCrossIn / wantChild, when the association stays cross-module: the + // unit holding it (-1 = the original module) and the module of its TO end. + wantCrossIn, wantChildIn int + }{ + // The issue's reproduction: shape 1 seen from the FROM end. + {name: "ToThenFrom_BackToPlain", steps: []step{{false, 0}, {true, 0}}, wantPlainIn: 0}, + // Shape 1 seen from the TO end: the cross-association is in the target. + {name: "FromThenTo_BackToPlain", steps: []step{{true, 0}, {false, 0}}, wantPlainIn: 0}, + // Shape 2: the FROM end moves on, the cross-association travels with it. + {name: "ToThenFromElsewhere_Travels", steps: []step{{false, 0}, {true, 1}}, wantPlainIn: -1, wantCrossIn: 1, wantChildIn: 0}, + // Shape 3: the TO end moves again; the cross-association stays in a module + // that is neither source nor target of the second move and is re-pointed. + {name: "ToThenToElsewhere_Repointed", steps: []step{{false, 0}, {false, 1}}, wantPlainIn: -1, wantCrossIn: -1, wantChildIn: 1}, + } { + t.Run(tc.name, func(t *testing.T) { + proj := copyPedAppFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + srcID, srcMod, assocName, guid, childID, parentID := associationSubjectDistinct(t, b) + if guid == "" || guid == assocRawID(t, b, srcID, assocName) { + t.Fatalf("subject %s.%s: GUID %q must exist and differ from $ID", srcMod, assocName, guid) + } + others := otherDomainModels(t, b, srcID, 2) + + where := map[model.ID]int{childID: -1, parentID: -1} // -1 = srcMod + dmOf := func(i int) (model.ID, string) { + if i < 0 { + return srcID, srcMod + } + return others[i].id, others[i].name + } + for _, s := range tc.steps { + entID := childID + if s.moveFrom { + entID = parentID + } + fromDM, fromMod := dmOf(where[entID]) + toDM, toMod := dmOf(s.to) + ent := entityByID(t, b, fromDM, entID) + if _, err := b.MoveEntity(ent, fromDM, toDM, fromMod, toMod); err != nil { + t.Fatalf("MoveEntity %s.%s -> %s: %v", fromMod, ent.Name, toMod, err) + } + where[entID] = s.to + } + if err := b.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + + b2 := New() + if err := b2.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b2.Disconnect() }) + + assertNoDanglingAssociations(t, b2) + + if tc.wantPlainIn >= 0 { + dmID, mod := dmOf(tc.wantPlainIn) + if got := crossAssocGUID(t, b2, dmID, assocName); got != "" { + t.Errorf("%s is still a cross-association in %s; both endpoints are there", assocName, mod) + } + got, ok := plainAssocGUID(t, b2, dmID, assocName) + if !ok { + t.Fatalf("%s is not a plain association in %s after both endpoints moved there", assocName, mod) + } + if got != guid { + t.Errorf("%s: storage GUID changed %s -> %s", assocName, guid, got) + } + return + } + dmID, mod := dmOf(tc.wantCrossIn) + got := crossAssocGUID(t, b2, dmID, assocName) + if got != guid { + t.Errorf("%s in %s: storage GUID %q, want %s", assocName, mod, got, guid) + } + _, childMod := dmOf(tc.wantChildIn) + if ref := crossAssocChild(t, b2, dmID, assocName); !strings.HasPrefix(ref, childMod+".") { + t.Errorf("%s in %s names TO entity %q, want one in %s", assocName, mod, ref, childMod) + } + }) + } +} + +type dmRef struct { + id model.ID + name string +} + +// associationSubjectDistinct is associationSubject restricted to an association +// between two different entities, with no other association between the pair. +func associationSubjectDistinct(t *testing.T, b *Backend) (dmID model.ID, moduleName, assocName, assocGUID string, childID, parentID model.ID) { + t.Helper() + dms, err := b.ListDomainModels() + if err != nil { + t.Fatalf("ListDomainModels: %v", err) + } + for _, d := range dms { + gdm, err := b.loadDomainModelGen(d.ID) + if err != nil { + continue + } + mod, err := b.GetModule(d.ContainerID) + if err != nil || mod == nil { + continue + } + for _, el := range gdm.AssociationsItems() { + a, ok := el.(*genDm.Association) + if !ok || a.Raw() == nil || a.ChildRefID() == a.ParentRefID() { + continue + } + return d.ID, mod.Name, a.Name(), rawKeyHex(t, a.Raw(), "GUID"), + model.ID(a.ChildRefID()), model.ID(a.ParentRefID()) + } + } + t.Fatal("no loadable domain model with an association between two entities") + return +} + +func otherDomainModels(t *testing.T, b *Backend, exclude model.ID, n int) []dmRef { + t.Helper() + dms, err := b.ListDomainModels() + if err != nil { + t.Fatalf("ListDomainModels: %v", err) + } + var out []dmRef + for _, d := range dms { + if d.ID == exclude { + continue + } + if _, err := b.loadDomainModelGen(d.ID); err != nil { + continue + } + mod, err := b.GetModule(d.ContainerID) + if err != nil || mod == nil { + continue + } + out = append(out, dmRef{d.ID, mod.Name}) + if len(out) == n { + return out + } + } + t.Fatalf("fixture has fewer than %d other loadable domain models", n) + return nil +} + +func assocRawID(t *testing.T, b *Backend, dmID model.ID, name string) string { + t.Helper() + gdm, err := b.loadDomainModelGen(dmID) + if err != nil { + t.Fatalf("loadDomainModelGen: %v", err) + } + for _, el := range gdm.AssociationsItems() { + if a, ok := el.(*genDm.Association); ok && a.Name() == name { + return rawKeyHex(t, a.Raw(), "$ID") + } + } + return "" +} + +func plainAssocGUID(t *testing.T, b *Backend, dmID model.ID, name string) (string, bool) { + t.Helper() + gdm, err := b.loadDomainModelGen(dmID) + if err != nil { + t.Fatalf("loadDomainModelGen: %v", err) + } + for _, el := range gdm.AssociationsItems() { + if a, ok := el.(*genDm.Association); ok && a.Name() == name { + return rawKeyHex(t, a.Raw(), "GUID"), true + } + } + return "", false +} + +func crossAssocChild(t *testing.T, b *Backend, dmID model.ID, name string) string { + t.Helper() + gdm, err := b.loadDomainModelGen(dmID) + if err != nil { + t.Fatalf("loadDomainModelGen: %v", err) + } + for _, el := range gdm.CrossAssociationsItems() { + if ca, ok := el.(*genDm.CrossAssociation); ok && ca.Name() == name { + return ca.ChildQualifiedName() + } + } + return "" +} + +// assertNoDanglingAssociations is the stored-document form of "the project +// opens": every association pointer resolves inside its own unit, and every +// cross-association's TO name resolves to an entity in the module it names. +func assertNoDanglingAssociations(t *testing.T, b *Backend) { + t.Helper() + dms, err := b.ListDomainModels() + if err != nil { + t.Fatalf("ListDomainModels: %v", err) + } + type unit struct { + mod string + dm *genDm.DomainModel + } + var units []unit + entitiesByModule := map[string]map[string]bool{} + for _, d := range dms { + gdm, err := b.loadDomainModelGen(d.ID) + if err != nil { + continue + } + mod, err := b.GetModule(d.ContainerID) + if err != nil || mod == nil { + continue + } + units = append(units, unit{mod.Name, gdm}) + names := map[string]bool{} + for _, el := range gdm.EntitiesItems() { + if e, ok := el.(*genDm.Entity); ok { + names[e.Name()] = true + } + } + entitiesByModule[mod.Name] = names + } + for _, u := range units { + ids := map[string]bool{} + for _, el := range u.dm.EntitiesItems() { + ids[string(el.ID())] = true + } + for _, el := range u.dm.AssociationsItems() { + if a, ok := el.(*genDm.Association); ok { + if !ids[string(a.ParentRefID())] || !ids[string(a.ChildRefID())] { + t.Errorf("association %s.%s points at an entity outside its unit", u.mod, a.Name()) + } + } + } + for _, el := range u.dm.CrossAssociationsItems() { + ca, ok := el.(*genDm.CrossAssociation) + if !ok { + continue + } + if !ids[string(ca.ParentRefID())] { + t.Errorf("cross-association %s.%s: ParentPointer names an entity absent from its unit", u.mod, ca.Name()) + } + mod, ent, _ := strings.Cut(ca.ChildQualifiedName(), ".") + if mod == "System" { + continue + } + if !entitiesByModule[mod][ent] { + t.Errorf("cross-association %s.%s: Child %q does not resolve", u.mod, ca.Name(), ca.ChildQualifiedName()) + } + if mod == u.mod { + t.Errorf("cross-association %s.%s names an entity in its own module; it should be a plain association", u.mod, ca.Name()) + } + } + } +} + +// copyPedAppFixture copies the Studio Pro-authored PedApp fixture into a temp +// dir; its elements have GUID != $ID, so a lost storage GUID is observable. +func copyPedAppFixture(t *testing.T) string { + t.Helper() + dst := t.TempDir() + if err := os.CopyFS(dst, os.DirFS("../../../testdata/pedapp")); err != nil { + t.Fatalf("copy PedApp fixture: %v", err) + } + return filepath.Join(dst, "PedApp.mpr") +} diff --git a/mdl/backend/modelsdk/published_rest_write.go b/mdl/backend/modelsdk/published_rest_write.go index 8370e4e4d..919db33b7 100644 --- a/mdl/backend/modelsdk/published_rest_write.go +++ b/mdl/backend/modelsdk/published_rest_write.go @@ -13,25 +13,27 @@ import ( "github.com/mendixlabs/mxcli/modelsdk/element" mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" "github.com/mendixlabs/mxcli/modelsdk/property" + "github.com/mendixlabs/mxcli/sdk/microflows" ) func init() { - // Resources / Operations / operation Parameters serialize with the typed-array - // marker 2 (populated keyed by child $Type; empty via MandatoryListMarkers). The - // service's AllowedRoles is a marker-1 reference-string list, AuthenticationTypes - // and Parameters are empty marker-2 lists, and CorsConfiguration is BSON null. - codec.RegisterListMarker("Rest$PublishedRestServiceResource", 2) + // List markers as Studio Pro 11 writes them (measured on ako/TestApp's + // Services.OrdersRestApi): Resources and operation Parameters use 3, + // Operations uses 2, and the service's empty Parameters list is [3]. The + // service's AllowedRoles and AuthenticationTypes are marker-1 string lists, + // and CorsConfiguration is BSON null. + codec.RegisterListMarker("Rest$PublishedRestServiceResource", 3) codec.RegisterListMarker("Rest$PublishedRestServiceOperation", 2) - codec.RegisterListMarker("Rest$RestOperationParameter", 2) + codec.RegisterListMarker("Rest$RestOperationParameter", 3) codec.RegisterTypeDefaults("Rest$PublishedRestService", codec.TypeDefaults{ - MandatoryListMarkers: map[string]int32{"AllowedRoles": 1, "AuthenticationTypes": 2, "Parameters": 2}, + MandatoryListMarkers: map[string]int32{"AllowedRoles": 1, "AuthenticationTypes": 1, "Parameters": 3}, NullFields: []string{"CorsConfiguration"}, }) codec.RegisterTypeDefaults("Rest$PublishedRestServiceResource", codec.TypeDefaults{ MandatoryListMarkers: map[string]int32{"Operations": 2}, }) codec.RegisterTypeDefaults("Rest$PublishedRestServiceOperation", codec.TypeDefaults{ - MandatoryListMarkers: map[string]int32{"Parameters": 2}, + MandatoryListMarkers: map[string]int32{"Parameters": 3}, }) } @@ -73,6 +75,12 @@ func (b *Backend) UpdatePublishedRestService(svc *model.PublishedRestService) er if err != nil { return fmt.Errorf("UpdatePublishedRestService: %w", err) } + // ... and the service-level properties MDL has no spelling for as + // constants too: carry the stored ones (ako/mxcli#571). + contents, err = b.keepStoredTopLevel(string(svc.ID), contents, publishedRestServiceUnauthored) + if err != nil { + return fmt.Errorf("UpdatePublishedRestService: %w", err) + } return b.writer.UpdateRawUnit(string(svc.ID), contents) } @@ -121,6 +129,20 @@ func (b *Backend) UpdatePublishedRestServiceRoles(unitID model.ID, roles []strin return b.writer.UpdateRawUnit(string(unitID), out) } +// publishedRestServiceUnauthored are the Rest$PublishedRestService keys a +// create or modify / alter cannot state: the writer emits a constant for each, +// so a rewrite carries the stored value instead. Without the carry, executing +// the describe output of a Studio Pro service turned its Basic and Session +// authentication off (ako/mxcli#571). +var publishedRestServiceUnauthored = []string{ + "AuthenticationMicroflow", + "AuthenticationTypes", + "CorsConfiguration", + "Documentation", + "Parameters", + "PublicDocumentation", +} + func publishedRestServiceToGen(svc *model.PublishedRestService) element.Element { g := newElem("Rest$PublishedRestService", string(svc.ID)) addStr(g, "Name", svc.Name) @@ -164,26 +186,36 @@ func publishedRestOperationToGen(op *model.PublishedRestOperation) element.Eleme addStr(g, "Microflow", op.Microflow) addStr(g, "Summary", op.Summary) addBool(g, "Deprecated", op.Deprecated) - addStr(g, "Commit", "Yes") - addStr(g, "Documentation", "") - addStr(g, "ExportMapping", "") - addStr(g, "ImportMapping", "") - addStr(g, "ObjectHandlingBackup", "Create") - // Path parameters are auto-extracted from {name} placeholders and wired to the - // matching microflow parameter (Module.Microflow.name) — without that wiring - // mx check raises CE6538 / CE0350. - params := make([]element.Element, 0) - for _, name := range extractPathParams(op.Path) { - p := newElem("Rest$RestOperationParameter", "") - addStr(p, "Name", name) - addPart(p, "Type", newElem("DataTypes$StringType", "")) - addStr(p, "ParameterType", "Path") - mfParam := "" - if op.Microflow != "" { - mfParam = op.Microflow + "." + name + addStr(g, "Commit", orDefault(op.Commit, "Yes")) + addStr(g, "Documentation", op.Documentation) + addStr(g, "ExportMapping", op.ExportMapping) + addStr(g, "ImportMapping", op.ImportMapping) + addStr(g, "ObjectHandlingBackup", orDefault(op.ObjectHandlingBackup, "Create")) + // The executor derives the parameters from the microflow (path, query, body), + // as Studio Pro does. When it could not read the microflow only the path's + // {name} placeholders are known: those are written as String path + // parameters wired to the microflow parameter of that name, since without + // that wiring mx check raises CE6538 / CE0350. + opParams := op.OperationParameters + if len(opParams) == 0 { + for _, name := range op.PathParameterNames() { + mfParam := "" + if op.Microflow != "" { + mfParam = op.Microflow + "." + name + } + opParams = append(opParams, &model.PublishedRestOperationParameter{ + Name: name, ParameterType: "Path", MicroflowParameter: mfParam, DataType: "String", + }) } - addStr(p, "MicroflowParameter", mfParam) - addStr(p, "Description", "") + } + params := make([]element.Element, 0, len(opParams)) + for _, param := range opParams { + p := newElem("Rest$RestOperationParameter", "") + addStr(p, "Name", param.Name) + addPart(p, "Type", publishedRestParameterTypeToGen(param)) + addStr(p, "ParameterType", param.ParameterType) + addStr(p, "MicroflowParameter", param.MicroflowParameter) + addStr(p, "Description", param.Description) params = append(params, p) } if len(params) > 0 { @@ -192,6 +224,49 @@ func publishedRestOperationToGen(op *model.PublishedRestOperation) element.Eleme return g } +// publishedRestParameterTypeToGen builds the DataTypes$* element of an +// operation parameter. Long is not among an operation parameter's types +// (Studio Pro 11.14's schema for Rest$RestOperationParameter.type): it is +// written as Integer, as microflowDataTypeToGen writes it. +func publishedRestParameterTypeToGen(p *model.PublishedRestOperationParameter) element.Element { + switch p.DataType { + case "Float": + return newElem("DataTypes$FloatType", "") + case "Empty": + return newElem("DataTypes$EmptyType", "") + case "Unknown": + return newElem("DataTypes$UnknownType", "") + } + return microflowDataTypeToGen(publishedRestParameterDataType(p)) +} + +// publishedRestParameterDataType is the microflow data type an operation +// parameter carries. +func publishedRestParameterDataType(p *model.PublishedRestOperationParameter) microflows.DataType { + switch p.DataType { + case "Boolean": + return µflows.BooleanType{} + case "Integer", "Long": + return µflows.IntegerType{} + case "Decimal": + return µflows.DecimalType{} + case "DateTime", "Date": + return µflows.DateTimeType{} + case "Binary": + return µflows.BinaryType{} + case "Enumeration": + return µflows.EnumerationType{EnumerationQualifiedName: p.QualifiedName} + case "Object": + return µflows.ObjectType{EntityQualifiedName: p.QualifiedName} + case "List": + return µflows.ListType{EntityQualifiedName: p.QualifiedName} + case "Void": + return nil + default: + return µflows.StringType{} + } +} + // addByNameRefList adds a marker-1 reference-string list property (qualified // names), the form Mendix uses for AllowedRoles / AllowedModuleRoles. func addByNameRefList(b *element.Base, name, targetType string, qnames []string) { @@ -202,24 +277,6 @@ func addByNameRefList(b *element.Base, name, targetType string, qnames []string) } } -// extractPathParams returns parameter names from {param} placeholders in a path. -func extractPathParams(path string) []string { - var names []string - for { - start := strings.Index(path, "{") - if start < 0 { - break - } - end := strings.Index(path[start:], "}") - if end < 0 { - break - } - names = append(names, path[start+1:start+end]) - path = path[start+end+1:] - } - return names -} - // httpMethodToMendix converts an HTTP method name to Mendix casing. func httpMethodToMendix(method string) string { switch strings.ToUpper(method) { diff --git a/mdl/backend/modelsdk/published_rest_write_test.go b/mdl/backend/modelsdk/published_rest_write_test.go index 3a0781d1d..6c4766bc6 100644 --- a/mdl/backend/modelsdk/published_rest_write_test.go +++ b/mdl/backend/modelsdk/published_rest_write_test.go @@ -3,8 +3,11 @@ package modelsdkbackend import ( + "strings" "testing" + "go.mongodb.org/mongo-driver/bson" + "github.com/mendixlabs/mxcli/model" ) @@ -72,3 +75,125 @@ func TestCreatePublishedRestService_RoundTrip(t *testing.T) { t.Errorf("operations = %d, want 2", len(got.Resources[0].Operations)) } } + +// TestCreatePublishedRestService_WritesParametersAndBindings is ako/mxcli#571 / +// mendixlabs/mxcli#1206 at the storage layer: the operation's query and body +// parameters, each with its own type, its mapping bindings and its commit +// option are written and read back. Before, only String path parameters were +// written, and ExportMapping / ImportMapping / Commit were constants. +func TestCreatePublishedRestService_WritesParametersAndBindings(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + mod, err := b.GetModuleByName("MyFirstModule") + if err != nil || mod == nil { + t.Fatalf("GetModuleByName: %v", err) + } + params := []*model.PublishedRestOperationParameter{ + {Name: "id", ParameterType: "Path", MicroflowParameter: "MyFirstModule.ACT_Put.id", DataType: "Integer"}, + {Name: "verbose", ParameterType: "Query", MicroflowParameter: "MyFirstModule.ACT_Put.verbose", DataType: "Boolean"}, + {Name: "X-Mode", ParameterType: "Header", MicroflowParameter: "MyFirstModule.ACT_Put.mode", DataType: "Enumeration", QualifiedName: "MyFirstModule.Mode", Description: "mode"}, + {Name: "body", ParameterType: "Body", MicroflowParameter: "MyFirstModule.ACT_Put.body", DataType: "Object", QualifiedName: "MyFirstModule.Thing"}, + } + svc := &model.PublishedRestService{ + ContainerID: mod.ID, Name: "ZzParams", Path: "rest/zzp/v1", + Resources: []*model.PublishedRestResource{{ + Name: "things", + Operations: []*model.PublishedRestOperation{ + { + Path: "{id}", HTTPMethod: "PUT", Microflow: "MyFirstModule.ACT_Put", + ImportMapping: "MyFirstModule.IMM_Thing", ExportMapping: "MyFirstModule.EMM_Thing", + Commit: "YesWithoutEvents", ObjectHandlingBackup: "Error", + OperationParameters: params, + }, + // No derived parameters: the path's placeholder as a String (the + // writer's fallback when the microflow could not be read). + {Path: "{key}", HTTPMethod: "GET", Microflow: "MyFirstModule.ACT_Get"}, + }, + }}, + } + if err := b.CreatePublishedRestService(svc); err != nil { + t.Fatalf("CreatePublishedRestService: %v", err) + } + + b2 := New() + if err := b2.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b2.Disconnect() }) + all, err := b2.ListPublishedRestServices() + if err != nil { + t.Fatalf("ListPublishedRestServices: %v", err) + } + var got *model.PublishedRestService + for _, s := range all { + if s.Name == "ZzParams" { + got = s + } + } + if got == nil { + t.Fatal("ZzParams not found") + } + op := got.Resources[0].Operations[0] + if op.ImportMapping != "MyFirstModule.IMM_Thing" || op.ExportMapping != "MyFirstModule.EMM_Thing" || + op.Commit != "YesWithoutEvents" || op.ObjectHandlingBackup != "Error" { + t.Errorf("bindings read back as import %q export %q commit %q handling %q", + op.ImportMapping, op.ExportMapping, op.Commit, op.ObjectHandlingBackup) + } + if len(op.OperationParameters) != len(params) { + t.Fatalf("parameters = %d, want %d: %+v", len(op.OperationParameters), len(params), op.OperationParameters) + } + for i, want := range params { + if g := *op.OperationParameters[i]; g != *want { + t.Errorf("parameter %d = %+v, want %+v", i, g, *want) + } + } + fallback := got.Resources[0].Operations[1] + if len(fallback.OperationParameters) != 1 || *fallback.OperationParameters[0] != (model.PublishedRestOperationParameter{ + Name: "key", ParameterType: "Path", MicroflowParameter: "MyFirstModule.ACT_Get.key", DataType: "String", + }) { + t.Errorf("fallback parameters = %+v", fallback.OperationParameters) + } + if fallback.Commit != "Yes" || fallback.ObjectHandlingBackup != "Create" { + t.Errorf("defaults = commit %q handling %q, want Yes / Create", fallback.Commit, fallback.ObjectHandlingBackup) + } +} + +// TestWithStoredTopLevel carries the stored values of the listed keys, +// inserts a stored-only key in sorted position, and leaves every other key as +// written. The control: an unlisted key keeps the written value. +func TestWithStoredTopLevel(t *testing.T) { + written, _ := bson.Marshal(bson.D{ + {Key: "$ID", Value: "x"}, {Key: "AuthenticationTypes", Value: bson.A{int32(1)}}, + {Key: "Name", Value: "new"}, {Key: "Version", Value: "2"}, + }) + stored, _ := bson.Marshal(bson.D{ + {Key: "$ID", Value: "x"}, {Key: "AuthenticationTypes", Value: bson.A{int32(1), "Basic"}}, + {Key: "Name", Value: "old"}, {Key: "PublicDocumentation", Value: ""}, {Key: "Version", Value: "1"}, + }) + out, err := withStoredTopLevel(written, stored, []string{"AuthenticationTypes", "PublicDocumentation", "Missing"}) + if err != nil { + t.Fatal(err) + } + var d bson.D + if err := bson.Unmarshal(out, &d); err != nil { + t.Fatal(err) + } + var keys []string + for _, e := range d { + keys = append(keys, e.Key) + } + if got := strings.Join(keys, ","); got != "$ID,AuthenticationTypes,Name,PublicDocumentation,Version" { + t.Errorf("keys = %s", got) + } + m := d.Map() + if a, ok := m["AuthenticationTypes"].(bson.A); !ok || len(a) != 2 || a[1] != "Basic" { + t.Errorf("AuthenticationTypes = %v, want the stored [1 Basic]", m["AuthenticationTypes"]) + } + if m["Name"] != "new" || m["Version"] != "2" { + t.Errorf("unlisted keys changed: Name %v Version %v", m["Name"], m["Version"]) + } +} diff --git a/mdl/executor/cmd_move.go b/mdl/executor/cmd_move.go index 79fac38e3..7778fbdeb 100644 --- a/mdl/executor/cmd_move.go +++ b/mdl/executor/cmd_move.go @@ -410,9 +410,25 @@ func moveEntity(ctx *ExecContext, name ast.QualifiedName, sourceModule, targetMo } fmt.Fprintf(ctx.Output, "Moved entity %s to %s\n", name.String(), targetModule.Name) - if len(convertedAssocs) > 0 { - fmt.Fprintf(ctx.Output, "Converted %d association(s) to cross-module associations:\n", len(convertedAssocs)) - for _, assocName := range types.MovedAssociationNames(convertedAssocs) { + // A move that brings both endpoints of a cross-association into one module turns + // it back into a plain association (ako/mxcli#628); report the two apart. + var toCross, toPlain []types.MovedAssociation + for _, a := range convertedAssocs { + if a.SameModule { + toPlain = append(toPlain, a) + } else { + toCross = append(toCross, a) + } + } + if len(toCross) > 0 { + fmt.Fprintf(ctx.Output, "Converted %d association(s) to cross-module associations:\n", len(toCross)) + for _, assocName := range types.MovedAssociationNames(toCross) { + fmt.Fprintf(ctx.Output, " - %s\n", assocName) + } + } + if len(toPlain) > 0 { + fmt.Fprintf(ctx.Output, "Converted %d cross-module association(s) to associations within %s:\n", len(toPlain), targetModule.Name) + for _, assocName := range types.MovedAssociationNames(toPlain) { fmt.Fprintf(ctx.Output, " - %s\n", assocName) } } diff --git a/mdl/executor/cmd_published_rest.go b/mdl/executor/cmd_published_rest.go index 40533f572..6533f0288 100644 --- a/mdl/executor/cmd_published_rest.go +++ b/mdl/executor/cmd_published_rest.go @@ -11,6 +11,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" ) // listPublishedRestServices handles SHOW PUBLISHED REST SERVICES [IN module] command. @@ -135,8 +136,11 @@ func describePublishedRestService(ctx *ExecContext, name ast.QualifiedName) erro if op.Path != "" { opPath = " " + mdlQuoted(op.Path) } - fmt.Fprintf(ctx.Output, " %s%s%s%s;%s\n", - strings.ToLower(op.HTTPMethod), opPath, mf, deprecated, summary) + fmt.Fprintf(ctx.Output, " %s%s%s%s%s;%s\n", + strings.ToLower(op.HTTPMethod), opPath, mf, deprecated, publishedRestBindingClauses(op), summary) + for _, note := range publishedRestParameterNotes(op) { + fmt.Fprintf(ctx.Output, " -- %s\n", note) + } } fmt.Fprintln(ctx.Output, " }") } @@ -159,6 +163,55 @@ func describePublishedRestService(ctx *ExecContext, name ast.QualifiedName) erro return mdlerrors.NewNotFound("published rest service", name.String()) } +// publishedRestBindingClauses prints an operation's mapping bindings and its +// commit option, in the grammar's order. Commit is printed when it is not +// "Yes", the value exec writes when the statement has no commit clause. +func publishedRestBindingClauses(op *model.PublishedRestOperation) string { + var b strings.Builder + if op.ImportMapping != "" { + b.WriteString(" import mapping " + op.ImportMapping) + } + if op.ExportMapping != "" { + b.WriteString(" export mapping " + op.ExportMapping) + } + if op.Commit != "" && op.Commit != "Yes" { + b.WriteString(" commit " + op.Commit) + } + return b.String() +} + +// publishedRestParameterNotes names the operation parameters MDL cannot state: +// it derives every parameter from the microflow, so a header or form +// parameter, a parameter renamed away from its microflow parameter, or one +// with a description has no spelling. create or modify on the same project +// keeps them; a fresh create derives the parameter again. +func publishedRestParameterNotes(op *model.PublishedRestOperation) []string { + var notes []string + for _, p := range op.OperationParameters { + bound := p.MicroflowParameter + if i := strings.LastIndex(bound, "."); i >= 0 { + bound = bound[i+1:] + } + var why []string + switch p.ParameterType { + case "Path", "Query", "Body": + default: + why = append(why, strings.ToLower(p.ParameterType)+" parameter") + } + if bound != p.Name { + why = append(why, fmt.Sprintf("bound to $%s", bound)) + } + if p.Description != "" { + why = append(why, "description "+mdlQuoted(strings.ReplaceAll(p.Description, "\n", " "))) + } + if len(why) > 0 { + notes = append(notes, fmt.Sprintf("parameter %s: %s (not expressible in MDL; kept by create or modify on this project)", + p.Name, strings.Join(why, ", "))) + } + } + return notes +} + // findPublishedRestService looks up a published REST service by module and name. func findPublishedRestService(ctx *ExecContext, moduleName, name string) (*model.PublishedRestService, error) { @@ -234,21 +287,16 @@ func execCreatePublishedRestService(ctx *ExecContext, s *ast.CreatePublishedRest } for _, resDef := range s.Resources { - resource := &model.PublishedRestResource{ - Name: resDef.Name, - } - for _, opDef := range resDef.Operations { - op := &model.PublishedRestOperation{ - HTTPMethod: opDef.HTTPMethod, - Path: opDef.Path, - Microflow: opDef.Microflow.String(), - Summary: "", - Deprecated: opDef.Deprecated, - } - resource.Operations = append(resource.Operations, op) + resource, err := astResourceDefToModel(resDef) + if err != nil { + return err } svc.Resources = append(svc.Resources, resource) } + if existing != nil { + carryStoredOperations(svc, existing) + } + deriveOperationParameters(ctx, svc) if existing != nil { if s.Folder == "" { @@ -309,17 +357,212 @@ func execDropPublishedRestService(ctx *ExecContext, s *ast.DropPublishedRestServ // astResourceDefToModel converts an AST PublishedRestResourceDef to the // runtime model type used by the writer. -func astResourceDefToModel(def *ast.PublishedRestResourceDef) *model.PublishedRestResource { +func astResourceDefToModel(def *ast.PublishedRestResourceDef) (*model.PublishedRestResource, error) { resource := &model.PublishedRestResource{Name: def.Name} for _, opDef := range def.Operations { + commit, err := publishedRestCommit(opDef.Commit) + if err != nil { + return nil, mdlerrors.NewValidation(fmt.Sprintf("resource '%s', operation %s %s: %v", + def.Name, opDef.HTTPMethod, opDef.Path, err)) + } resource.Operations = append(resource.Operations, &model.PublishedRestOperation{ - HTTPMethod: opDef.HTTPMethod, - Path: opDef.Path, - Microflow: opDef.Microflow.String(), - Deprecated: opDef.Deprecated, + HTTPMethod: opDef.HTTPMethod, + Path: opDef.Path, + Microflow: opDef.Microflow.String(), + Deprecated: opDef.Deprecated, + ImportMapping: opDef.ImportMapping, + ExportMapping: opDef.ExportMapping, + Commit: commit, }) } - return resource + return resource, nil +} + +// publishedRestCommitValues are the values of Rest$PublishedRestServiceOperation.Commit. +var publishedRestCommitValues = []string{"Yes", "YesWithoutEvents", "No"} + +// publishedRestCommit returns the stored spelling of an operation's commit +// clause ("" when the statement has none). Any other value used to parse and +// be thrown away; it is refused, since there is nothing correct to write. +func publishedRestCommit(v string) (string, error) { + if v == "" { + return "", nil + } + for _, c := range publishedRestCommitValues { + if strings.EqualFold(v, c) { + return c, nil + } + } + return "", fmt.Errorf("commit %s is not a commit option (allowed: %s)", v, strings.Join(publishedRestCommitValues, ", ")) +} + +// carryStoredOperations copies onto each operation a statement declares what +// MDL cannot state about it — summary, documentation, the import mapping's +// object handling, and the stored parameters (their names, kinds and +// descriptions) — from the stored operation it restates. An operation is +// matched by its resource's name, its method and its path, in order; one +// without a match is new and gets the defaults. +func carryStoredOperations(svc, stored *model.PublishedRestService) { + pool := map[string][]*model.PublishedRestOperation{} + key := func(res string, op *model.PublishedRestOperation) string { + return res + "\x00" + strings.ToUpper(op.HTTPMethod) + "\x00" + op.Path + } + for _, res := range stored.Resources { + for _, op := range res.Operations { + k := key(res.Name, op) + pool[k] = append(pool[k], op) + } + } + for _, res := range svc.Resources { + for _, op := range res.Operations { + k := key(res.Name, op) + if len(pool[k]) == 0 { + continue + } + old := pool[k][0] + pool[k] = pool[k][1:] + op.Summary = old.Summary + op.Documentation = old.Documentation + op.ObjectHandlingBackup = old.ObjectHandlingBackup + if old.Microflow == op.Microflow { + op.OperationParameters = old.OperationParameters + } + } + } +} + +// deriveOperationParameters gives every operation the parameters Studio Pro +// derives from its microflow (ako/mxcli#571, mendixlabs/mxcli#1206): a +// parameter named in the path is a path parameter, an object or a list is the +// body, System.HttpRequest and System.HttpResponse are the request and the +// response themselves, and anything else is a query parameter — each with the +// microflow parameter's type. Without the query and body parameters mx check +// reports CE0350, and a String path parameter bound to an Integer is CE6539. +// +// The operation's stored parameters (op.OperationParameters on entry) win for +// the microflow parameter they bind: Studio Pro lets a parameter be renamed, +// described or turned into a header, and MDL has no spelling for that, so a +// rewrite keeps it. Only the type follows the microflow, and a path parameter +// follows the path. +func deriveOperationParameters(ctx *ExecContext, svc *model.PublishedRestService) { + var microflowsByName map[string]*microflows.Microflow + for _, resource := range svc.Resources { + for _, op := range resource.Operations { + if op.Microflow == "" { + continue + } + if microflowsByName == nil { + microflowsByName = liveMicroflowsByQualifiedName(ctx) + } + mf := microflowsByName[op.Microflow] + if mf == nil { + if len(op.OperationParameters) == 0 && !ctx.Quiet { + fmt.Fprintf(ctx.Output, "Warning: microflow %s not found, so operation %s %s gets only its path parameters, as String -- "+ + "create the microflow before the service, or its other parameters fail mx check with CE0350\n", + op.Microflow, strings.ToUpper(op.HTTPMethod), op.Path) + } + continue + } + op.OperationParameters = mergeOperationParameters(op.OperationParameters, operationParametersOf(op.Microflow, mf, op.PathParameterNames())) + } + } +} + +// liveMicroflowsByQualifiedName indexes the project's live microflows (an +// excluded twin never shadows the live one, #914). +func liveMicroflowsByQualifiedName(ctx *ExecContext) map[string]*microflows.Microflow { + out := map[string]*microflows.Microflow{} + all, err := ctx.Backend.ListMicroflows() + if err != nil { + return out + } + h, err := getHierarchy(ctx) + if err != nil { + return out + } + for _, mf := range all { + qn := h.GetQualifiedName(mf.ContainerID, mf.Name) + if prev, ok := out[qn]; ok && !prev.Excluded { + continue + } + out[qn] = mf + } + return out +} + +// operationParametersOf maps a microflow's parameters to operation parameters +// by the rule Studio Pro applies. +func operationParametersOf(mfName string, mf *microflows.Microflow, pathNames []string) []*model.PublishedRestOperationParameter { + inPath := make(map[string]bool, len(pathNames)) + for _, name := range pathNames { + inPath[name] = true + } + var params []*model.PublishedRestOperationParameter + for _, p := range mf.Parameters { + param := &model.PublishedRestOperationParameter{ + Name: p.Name, + ParameterType: "Query", + MicroflowParameter: mfName + "." + p.Name, + } + if p.Type != nil { + param.DataType = p.Type.GetTypeName() + } + switch t := p.Type.(type) { + case *microflows.ObjectType: + if t.EntityQualifiedName == "System.HttpRequest" || t.EntityQualifiedName == "System.HttpResponse" { + continue + } + param.ParameterType, param.DataType, param.QualifiedName = "Body", "Object", t.EntityQualifiedName + case *microflows.ListType: + param.ParameterType, param.DataType, param.QualifiedName = "Body", "List", t.EntityQualifiedName + case *microflows.EnumerationType: + param.DataType, param.QualifiedName = "Enumeration", t.EnumerationQualifiedName + } + if inPath[p.Name] { + param.ParameterType = "Path" + } + params = append(params, param) + } + return params +} + +// mergeOperationParameters keeps each stored parameter whose microflow +// parameter the microflow still has — in stored order, with the derived type, +// and the derived kind where either side is a path parameter — drops the ones +// it no longer has, keeps an unbound one as stored, and appends the derived +// parameters no stored one binds. +func mergeOperationParameters(stored, derived []*model.PublishedRestOperationParameter) []*model.PublishedRestOperationParameter { + byBinding := make(map[string]*model.PublishedRestOperationParameter, len(derived)) + for _, d := range derived { + byBinding[d.MicroflowParameter] = d + } + used := map[string]bool{} + var out []*model.PublishedRestOperationParameter + for _, sp := range stored { + if sp.MicroflowParameter == "" { + out = append(out, sp) + continue + } + d, ok := byBinding[sp.MicroflowParameter] + if !ok || used[sp.MicroflowParameter] { + continue + } + used[sp.MicroflowParameter] = true + kept := *sp + if d.DataType != "" { // "" is a type the reader could not read: keep the stored one + kept.DataType, kept.QualifiedName = d.DataType, d.QualifiedName + } + if d.ParameterType == "Path" || sp.ParameterType == "Path" { + kept.Name, kept.ParameterType = d.Name, d.ParameterType + } + out = append(out, &kept) + } + for _, d := range derived { + if !used[d.MicroflowParameter] { + out = append(out, d) + } + } + return out } // execAlterPublishedRestService applies SET / ADD RESOURCE / DROP RESOURCE @@ -363,7 +606,11 @@ func execAlterPublishedRestService(ctx *ExecContext, s *ast.AlterPublishedRestSe return mdlerrors.NewAlreadyExistsMsg("resource", a.Resource.Name, fmt.Sprintf("resource '%s' already exists on %s.%s", a.Resource.Name, s.Name.Module, s.Name.Name)) } } - svc.Resources = append(svc.Resources, astResourceDefToModel(a.Resource)) + resource, err := astResourceDefToModel(a.Resource) + if err != nil { + return err + } + svc.Resources = append(svc.Resources, resource) case *ast.PublishedRestDropResourceAction: idx := -1 @@ -383,6 +630,7 @@ func execAlterPublishedRestService(ctx *ExecContext, s *ast.AlterPublishedRestSe } } + deriveOperationParameters(ctx, svc) if err := ctx.Backend.UpdatePublishedRestService(svc); err != nil { return mdlerrors.NewBackend("alter published rest service", err) } diff --git a/mdl/executor/cmd_published_rest_params_test.go b/mdl/executor/cmd_published_rest_params_test.go new file mode 100644 index 000000000..011a5c0fd --- /dev/null +++ b/mdl/executor/cmd_published_rest_params_test.go @@ -0,0 +1,314 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// publishedRestFixture is a module RestQ holding the microflows of +// mendixlabs/mxcli#1206's repro, with the given stored services. +type publishedRestFixture struct { + mod *model.Module + stored []*model.PublishedRestService + created *model.PublishedRestService + updated *model.PublishedRestService +} + +func newPublishedRestFixture(t *testing.T, stored ...*model.PublishedRestService) (*publishedRestFixture, *ExecContext, *strings.Builder) { + t.Helper() + f := &publishedRestFixture{mod: mkModule("RestQ"), stored: stored} + for _, s := range stored { + s.ContainerID = f.mod.ID + } + param := func(name string, dt microflows.DataType) *microflows.MicroflowParameter { + return µflows.MicroflowParameter{Name: name, Type: dt} + } + mf := func(name string, params ...*microflows.MicroflowParameter) *microflows.Microflow { + return µflows.Microflow{ + BaseElement: model.BaseElement{ID: nextID("mf")}, + ContainerID: f.mod.ID, Name: name, Parameters: params, + } + } + mfs := []*microflows.Microflow{ + mf("GetStatus", + param("orderNumber", µflows.StringType{}), + param("count", µflows.IntegerType{}), + param("httpRequest", µflows.ObjectType{EntityQualifiedName: "System.HttpRequest"})), + mf("GetById", + param("id", µflows.IntegerType{}), + param("verbose", µflows.BooleanType{})), + mf("PutFile", param("file", µflows.ObjectType{EntityQualifiedName: "RestQ.Upload"})), + mf("PutMany", param("items", µflows.ListType{EntityQualifiedName: "RestQ.Item"})), + } + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{f.mod}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return mfs, nil + }, + ListPublishedRestServicesFunc: func() ([]*model.PublishedRestService, error) { return f.stored, nil }, + CreatePublishedRestServiceFunc: func(svc *model.PublishedRestService) error { + f.created = svc + return nil + }, + UpdatePublishedRestServiceFunc: func(svc *model.PublishedRestService) error { + f.updated = svc + return nil + }, + } + h := mkHierarchy(f.mod) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + out := &strings.Builder{} + ctx.Output = out + return f, ctx, out +} + +func execPublishedRest(t *testing.T, ctx *ExecContext, src string) error { + t.Helper() + prog := parseMDL(t, src) + for _, stmt := range prog.Statements { + var err error + switch s := stmt.(type) { + case *ast.CreatePublishedRestServiceStmt: + err = execCreatePublishedRestService(ctx, s) + case *ast.AlterPublishedRestServiceStmt: + err = execAlterPublishedRestService(ctx, s) + default: + t.Fatalf("unexpected statement %T", stmt) + } + if err != nil { + return err + } + } + return nil +} + +func paramsString(op *model.PublishedRestOperation) string { + var parts []string + for _, p := range op.OperationParameters { + s := p.ParameterType + " " + p.Name + ":" + p.DataType + if p.QualifiedName != "" { + s += "(" + p.QualifiedName + ")" + } + s += "->" + p.MicroflowParameter + parts = append(parts, s) + } + return strings.Join(parts, "; ") +} + +// TestCreatePublishedRestService_DerivesParameters is mendixlabs/mxcli#1206: +// only the path's placeholders were written, each a String, so every query and +// body parameter failed mx check with CE0350 and an Integer {id} with CE6539. +func TestCreatePublishedRestService_DerivesParameters(t *testing.T) { + f, ctx, _ := newPublishedRestFixture(t) + assertNoError(t, execPublishedRest(t, ctx, `create published rest service RestQ.Orders (Path: 'rest/orders/v1') { + resource 'orders' { + get 'status' microflow RestQ.GetStatus; + get 'items/{id}' microflow RestQ.GetById; + post 'upload' microflow RestQ.PutFile; + put 'many' microflow RestQ.PutMany; + } +};`)) + if f.created == nil { + t.Fatal("nothing created") + } + ops := f.created.Resources[0].Operations + want := []string{ + "Query orderNumber:String->RestQ.GetStatus.orderNumber; Query count:Integer->RestQ.GetStatus.count", + "Path id:Integer->RestQ.GetById.id; Query verbose:Boolean->RestQ.GetById.verbose", + "Body file:Object(RestQ.Upload)->RestQ.PutFile.file", + "Body items:List(RestQ.Item)->RestQ.PutMany.items", + } + for i, w := range want { + if got := paramsString(ops[i]); got != w { + t.Errorf("operation %d parameters:\n got %s\nwant %s", i, got, w) + } + } +} + +// TestCreatePublishedRestService_WritesMappingBindings is ako/mxcli#571: the +// import mapping, export mapping and commit clauses parsed and were thrown away. +func TestCreatePublishedRestService_WritesMappingBindings(t *testing.T) { + f, ctx, _ := newPublishedRestFixture(t) + assertNoError(t, execPublishedRest(t, ctx, `create published rest service RestQ.Orders (Path: 'rest/orders/v1') { + resource 'orders' { + post 'upload' microflow RestQ.PutFile import mapping RestQ.IMM_Upload export mapping "RestQ"."EMM_Result" commit yeswithoutevents; + get 'status' microflow RestQ.GetStatus; + } +};`)) + op := f.created.Resources[0].Operations[0] + if op.ImportMapping != "RestQ.IMM_Upload" || op.ExportMapping != "RestQ.EMM_Result" || op.Commit != "YesWithoutEvents" { + t.Errorf("bindings = import %q export %q commit %q, want RestQ.IMM_Upload, RestQ.EMM_Result, YesWithoutEvents", + op.ImportMapping, op.ExportMapping, op.Commit) + } + if other := f.created.Resources[0].Operations[1]; other.ImportMapping != "" || other.ExportMapping != "" || other.Commit != "" { + t.Errorf("an operation without clauses got bindings: %+v", other) + } +} + +// TestCreatePublishedRestService_RefusesUnknownCommit: `commit Maybe` parsed +// and was thrown away; there is nothing correct to write, so it is refused by +// exec and by check (MDL-REST03). +func TestCreatePublishedRestService_RefusesUnknownCommit(t *testing.T) { + src := `create published rest service RestQ.Orders (Path: 'rest/orders/v1') { + resource 'orders' { post 'upload' microflow RestQ.PutFile import mapping RestQ.IMM commit Maybe; } +};` + f, ctx, _ := newPublishedRestFixture(t) + err := execPublishedRest(t, ctx, src) + if err == nil || !strings.Contains(err.Error(), "commit Maybe") { + t.Fatalf("exec error = %v, want a refusal naming commit Maybe", err) + } + if f.created != nil { + t.Error("the service was written despite the refusal") + } + v := ValidatePublishedRestCommit(parseMDL(t, src)) + if len(v) != 1 || v[0].RuleID != "MDL-REST03" { + t.Errorf("check = %+v, want one MDL-REST03", v) + } + if v := ValidatePublishedRestCommit(parseMDL(t, strings.Replace(src, "Maybe", "No", 1))); len(v) != 0 { + t.Errorf("control: commit No flagged: %+v", v) + } +} + +// TestCreateOrModifyPublishedRestService_KeepsStoredParameters: a parameter +// Studio Pro lets the user rename, describe or turn into a header has no MDL +// spelling; create or modify of the same operation keeps it, as it keeps the +// summary, documentation and object handling, while the type follows the +// microflow and a new microflow parameter is derived. +func TestCreateOrModifyPublishedRestService_KeepsStoredParameters(t *testing.T) { + stored := &model.PublishedRestService{ + BaseElement: model.BaseElement{ID: nextID("prs")}, + Name: "Orders", + Path: "rest/orders/v1", + Resources: []*model.PublishedRestResource{{ + Name: "orders", + Operations: []*model.PublishedRestOperation{{ + HTTPMethod: "Get", Path: "status", Microflow: "RestQ.GetStatus", + Summary: "Order status", Documentation: "docs", ObjectHandlingBackup: "Error", Commit: "No", + OperationParameters: []*model.PublishedRestOperationParameter{ + {Name: "X-Count", ParameterType: "Header", MicroflowParameter: "RestQ.GetStatus.count", DataType: "String", Description: "how many"}, + }, + }}, + }}, + } + f, ctx, out := newPublishedRestFixture(t, stored) + assertNoError(t, execPublishedRest(t, ctx, `create or modify published rest service RestQ.Orders (Path: 'rest/orders/v1') { + resource 'orders' { get 'status' microflow RestQ.GetStatus commit No; } +};`)) + if f.updated == nil { + t.Fatalf("nothing updated; output:\n%s", out) + } + op := f.updated.Resources[0].Operations[0] + if got, want := paramsString(op), "Header X-Count:Integer->RestQ.GetStatus.count; Query orderNumber:String->RestQ.GetStatus.orderNumber"; got != want { + t.Errorf("parameters:\n got %s\nwant %s", got, want) + } + if op.OperationParameters[0].Description != "how many" { + t.Errorf("description lost: %+v", op.OperationParameters[0]) + } + if op.Summary != "Order status" || op.Documentation != "docs" || op.ObjectHandlingBackup != "Error" { + t.Errorf("unstated operation properties not carried: %+v", op) + } +} + +// TestAlterPublishedRestService_DerivesAddedOperations: ALTER writes every +// operation again, so an added resource derives its parameters and keeps its +// bindings, and the untouched operations keep theirs. +func TestAlterPublishedRestService_DerivesAddedOperations(t *testing.T) { + stored := &model.PublishedRestService{ + BaseElement: model.BaseElement{ID: nextID("prs")}, + Name: "Orders", + Path: "rest/orders/v1", + Resources: []*model.PublishedRestResource{{ + Name: "orders", + Operations: []*model.PublishedRestOperation{{ + HTTPMethod: "Post", Path: "upload", Microflow: "RestQ.PutFile", + ImportMapping: "RestQ.IMM_Upload", Commit: "No", + OperationParameters: []*model.PublishedRestOperationParameter{ + {Name: "file", ParameterType: "Body", MicroflowParameter: "RestQ.PutFile.file", DataType: "Object", QualifiedName: "RestQ.Upload"}, + }, + }}, + }}, + } + f, ctx, _ := newPublishedRestFixture(t, stored) + assertNoError(t, execPublishedRest(t, ctx, `alter published rest service RestQ.Orders + add resource 'items' { get '{id}' microflow RestQ.GetById export mapping RestQ.EMM_Item; };`)) + if f.updated == nil { + t.Fatal("nothing updated") + } + kept := f.updated.Resources[0].Operations[0] + if kept.ImportMapping != "RestQ.IMM_Upload" || kept.Commit != "No" || paramsString(kept) != "Body file:Object(RestQ.Upload)->RestQ.PutFile.file" { + t.Errorf("untouched operation changed: %+v %s", kept, paramsString(kept)) + } + added := f.updated.Resources[1].Operations[0] + if added.ExportMapping != "RestQ.EMM_Item" { + t.Errorf("added operation export mapping = %q", added.ExportMapping) + } + if got, want := paramsString(added), "Path id:Integer->RestQ.GetById.id; Query verbose:Boolean->RestQ.GetById.verbose"; got != want { + t.Errorf("added operation parameters:\n got %s\nwant %s", got, want) + } +} + +// TestCreatePublishedRestService_MissingMicroflowWarns: without the microflow +// only the path is known; the operation keeps today's path-only parameters and +// exec says what mx check will report. +func TestCreatePublishedRestService_MissingMicroflowWarns(t *testing.T) { + f, ctx, out := newPublishedRestFixture(t) + assertNoError(t, execPublishedRest(t, ctx, `create published rest service RestQ.Orders (Path: 'rest/orders/v1') { + resource 'orders' { get '{id}' microflow RestQ.NotYet; } +};`)) + if len(f.created.Resources[0].Operations[0].OperationParameters) != 0 { + t.Errorf("parameters derived for a missing microflow: %s", paramsString(f.created.Resources[0].Operations[0])) + } + if !strings.Contains(out.String(), "microflow RestQ.NotYet not found") { + t.Errorf("no warning; output:\n%s", out) + } +} + +// TestDescribePublishedRestService_PrintsBindings: describe prints the +// bindings exec now writes, so executing its output restates them, and names +// the parameters MDL cannot state. +func TestDescribePublishedRestService_PrintsBindings(t *testing.T) { + stored := &model.PublishedRestService{ + BaseElement: model.BaseElement{ID: nextID("prs")}, + Name: "Orders", + Path: "rest/orders/v1", + Resources: []*model.PublishedRestResource{{ + Name: "orders", + Operations: []*model.PublishedRestOperation{ + { + HTTPMethod: "Post", Path: "upload", Microflow: "RestQ.PutFile", + ImportMapping: "RestQ.IMM_Upload", ExportMapping: "RestQ.EMM_Result", Commit: "YesWithoutEvents", + }, + { + HTTPMethod: "Get", Path: "status", Microflow: "RestQ.GetStatus", Commit: "Yes", + OperationParameters: []*model.PublishedRestOperationParameter{ + {Name: "X-Count", ParameterType: "Header", MicroflowParameter: "RestQ.GetStatus.count", DataType: "Integer"}, + {Name: "orderNumber", ParameterType: "Query", MicroflowParameter: "RestQ.GetStatus.orderNumber", DataType: "String"}, + }, + }, + }, + }}, + } + _, ctx, out := newPublishedRestFixture(t, stored) + assertNoError(t, describePublishedRestService(ctx, ast.QualifiedName{Module: "RestQ", Name: "Orders"})) + got := out.String() + assertContainsStr(t, got, "post 'upload' microflow RestQ.PutFile import mapping RestQ.IMM_Upload export mapping RestQ.EMM_Result commit YesWithoutEvents;") + assertContainsStr(t, got, "get 'status' microflow RestQ.GetStatus;") + assertContainsStr(t, got, "-- parameter X-Count: header parameter, bound to $count") + if strings.Contains(got, "parameter orderNumber") { + t.Errorf("a derivable parameter was flagged:\n%s", got) + } + // The output re-parses, and its bindings parse back to what is stored. + prog := parseMDL(t, got) + op := prog.Statements[0].(*ast.CreatePublishedRestServiceStmt).Resources[0].Operations[0] + if op.ImportMapping != "RestQ.IMM_Upload" || op.ExportMapping != "RestQ.EMM_Result" || op.Commit != "YesWithoutEvents" { + t.Errorf("describe output parses back to %+v", op) + } +} diff --git a/mdl/executor/validate_program.go b/mdl/executor/validate_program.go index 9eeb51bab..484912c80 100644 --- a/mdl/executor/validate_program.go +++ b/mdl/executor/validate_program.go @@ -289,6 +289,10 @@ func ValidateProgram(prog *ast.Program, projectPath string) []linter.Violation { // operation, so the mapping would be dropped in silence (#843). violations = append(violations, ValidateRestClientMappings(prog)...) + // Flag a published REST operation whose commit option is not one Mendix + // has; it used to parse and be thrown away (ako/mxcli#571). + violations = append(violations, ValidatePublishedRestCommit(prog)...) + // Flag a scheduled event whose Repeat and fields disagree (a Multiplier on // a Daily repeat, an HourOfDay of 99). Decidable from the statement, so it // runs here rather than at exec, where the script would already have diff --git a/mdl/executor/validate_rest_mapping.go b/mdl/executor/validate_rest_mapping.go index 186b777be..d4f22a697 100644 --- a/mdl/executor/validate_rest_mapping.go +++ b/mdl/executor/validate_rest_mapping.go @@ -54,3 +54,40 @@ func ValidateRestClientMappings(prog *ast.Program) []linter.Violation { } return out } + +// ValidatePublishedRestCommit reports (MDL-REST03) a published REST operation +// whose commit clause names no commit option. exec refuses the same statement +// (publishedRestCommit), so check predicts it. +func ValidatePublishedRestCommit(prog *ast.Program) []linter.Violation { + var out []linter.Violation + visit := func(res *ast.PublishedRestResourceDef) { + if res == nil { + return + } + for _, op := range res.Operations { + if _, err := publishedRestCommit(op.Commit); err != nil { + out = append(out, linter.Violation{ + RuleID: "MDL-REST03", + Severity: linter.SeverityError, + Message: "resource '" + res.Name + "', operation " + op.HTTPMethod + " " + op.Path + ": " + err.Error(), + Suggestion: "Write commit Yes, commit YesWithoutEvents or commit No, or leave the clause out (Yes).", + }) + } + } + } + for _, stmt := range prog.Statements { + switch s := stmt.(type) { + case *ast.CreatePublishedRestServiceStmt: + for _, res := range s.Resources { + visit(res) + } + case *ast.AlterPublishedRestServiceStmt: + for _, a := range s.Actions { + if add, ok := a.(*ast.PublishedRestAddResourceAction); ok { + visit(add.Resource) + } + } + } + } + return out +} diff --git a/mdl/roundtrip/testapp_allowlist_test.go b/mdl/roundtrip/testapp_allowlist_test.go index f9330a296..15ea36da4 100644 --- a/mdl/roundtrip/testapp_allowlist_test.go +++ b/mdl/roundtrip/testapp_allowlist_test.go @@ -184,7 +184,6 @@ var testAppKnownFailures = map[string]knownFailure{ "page WorkflowCommons.WorkflowUserTaskView_View": {laws: []law{lawGetPut}, issue: "#721 #826", why: "executes since describe prints snippet call Params (#826); the page Appearance keeps 1 of 3 DesignProperties (#721 C)"}, "page WorkflowCommons.Workflow_Dashboard": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.Workflow_JumpTo_Options": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "published rest service Services.OrdersRestApi": {laws: []law{lawGetPut}, issue: "#721", why: "published rest service: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rest client Clients.OrdersRestClient": {laws: []law{lawParse}, issue: "#721", why: "rest client: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rest client Mappings.RestClient": {laws: []law{lawParse}, issue: "#721", why: "rest client: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rule Rules.Rule1": {laws: []law{lawGetPut}, issue: "#721", why: "rule: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, diff --git a/mdl/types/entity_move.go b/mdl/types/entity_move.go index e02fd24d5..ee8292e9a 100644 --- a/mdl/types/entity_move.go +++ b/mdl/types/entity_move.go @@ -27,6 +27,10 @@ type MovedAssociation struct { // NewQualifiedName is what it is called after it. Equal to OldQualifiedName // when the association did not change module. NewQualifiedName string + // SameModule reports that the move brought both endpoints into one module, so + // the association is now a plain (same-module) association rather than a + // cross-module one (ako/mxcli#628). + SameModule bool } // Moved reports whether the association's qualified name changed, i.e. whether diff --git a/mdl/visitor/visitor_rest.go b/mdl/visitor/visitor_rest.go index c092688a2..ebf12ad97 100644 --- a/mdl/visitor/visitor_rest.go +++ b/mdl/visitor/visitor_rest.go @@ -450,7 +450,7 @@ func buildPublishedRestResourceDef(rc *parser.PublishedRestResourceContext) *ast // Import/Export mapping (qualifiedName after IMPORT/EXPORT MAPPING) if oc.IMPORT() != nil && len(allQN) >= 2 { - opDef.ImportMapping = allQN[1].GetText() + opDef.ImportMapping = buildQualifiedName(allQN[1]).String() } if oc.EXPORT() != nil { idx := 1 @@ -458,7 +458,7 @@ func buildPublishedRestResourceDef(rc *parser.PublishedRestResourceContext) *ast idx = 2 } if len(allQN) > idx { - opDef.ExportMapping = allQN[idx].GetText() + opDef.ExportMapping = buildQualifiedName(allQN[idx]).String() } } diff --git a/model/types.go b/model/types.go index 351913cca..fa8789a61 100644 --- a/model/types.go +++ b/model/types.go @@ -5,6 +5,7 @@ package model import ( "encoding/json" + "strings" "time" "go.mongodb.org/mongo-driver/bson" @@ -816,6 +817,62 @@ type PublishedRestOperation struct { Microflow string `json:"microflow,omitempty"` Deprecated bool `json:"deprecated,omitempty"` Parameters []string `json:"parameters,omitempty"` // path parameter names extracted from {param} in Path + + // Documentation is the operation's documentation; MDL has no spelling for + // it, so a rewrite carries the stored value. + Documentation string `json:"documentation,omitempty"` + // ImportMapping / ExportMapping are the qualified names of the mappings bound + // to the request body and the response ("" when none). + ImportMapping string `json:"importMapping,omitempty"` + ExportMapping string `json:"exportMapping,omitempty"` + // Commit is "Yes", "YesWithoutEvents" or "No" ("" is written as "Yes"). + Commit string `json:"commit,omitempty"` + // ObjectHandlingBackup is the import mapping's fallback object handling: + // "Create", "Ignore" or "Error" ("" is written as "Create"). MDL has no + // spelling for it, so a rewrite carries the stored value. + ObjectHandlingBackup string `json:"objectHandlingBackup,omitempty"` + // OperationParameters are the Rest$RestOperationParameter elements, in + // stored order. The executor derives them from the operation's microflow, + // as Studio Pro does. Empty means only the path is known: the writer then + // writes each {name} placeholder as a String path parameter. + OperationParameters []*PublishedRestOperationParameter `json:"operationParameters,omitempty"` +} + +// PathParameterNames returns the names of the {name} placeholders in the +// operation's path, in order. +func (op *PublishedRestOperation) PathParameterNames() []string { + var names []string + path := op.Path + for { + start := strings.Index(path, "{") + if start < 0 { + break + } + end := strings.Index(path[start:], "}") + if end < 0 { + break + } + names = append(names, path[start+1:start+end]) + path = path[start+end+1:] + } + return names +} + +// PublishedRestOperationParameter is one Rest$RestOperationParameter of a +// published REST operation. +type PublishedRestOperationParameter struct { + Name string `json:"name"` + // ParameterType is "Path", "Query", "Body", "Header" or "Form". + ParameterType string `json:"parameterType"` + // MicroflowParameter is the bound microflow parameter, qualified as + // Module.Microflow.Parameter ("" when unbound). + MicroflowParameter string `json:"microflowParameter,omitempty"` + Description string `json:"description,omitempty"` + // DataType is the parameter's type: "String", "Integer", "Long", + // "Decimal", "Boolean", "DateTime", "Binary", "Enumeration", "Object" or + // "List". QualifiedName is the enumeration or the entity. + DataType string `json:"dataType"` + QualifiedName string `json:"qualifiedName,omitempty"` } // ============================================================================ diff --git a/modelsdk/mpr/studiopro_lock.go b/modelsdk/mpr/studiopro_lock.go new file mode 100644 index 000000000..7e4f12ebc --- /dev/null +++ b/modelsdk/mpr/studiopro_lock.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mpr + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "time" +) + +// Studio Pro open-project guard (mendixlabs/mxcli#849). +// +// Studio Pro keeps the model in memory and does not reload it when another +// process changes the files. A write mxcli makes while the project is open is +// silently discarded the next time Studio Pro saves: the write succeeds, `mx +// check` passes, and the change is gone. So every write that would reach storage +// is refused while Studio Pro has the project open, unless the caller opted out +// (exec --force, or MXCLI_ALLOW_STUDIO_PRO_OPEN=1). Reads are never refused, and +// a write that reconciliation elides as a no-op never gets here, so re-running a +// script that is already applied keeps working. +// +// The signal is the lock file Studio Pro creates beside the .mpr while the +// project is open and removes on close: `.mpr.lock`. Studio Pro's own +// generated .gitignore lists it (with the project name LOWER-cased — TestApp's +// says `testapp.mpr.lock` beside `TestApp.mpr`), so the name is matched without +// regard to case. A Studio Pro that crashed leaves the file behind; the message +// says so and how to proceed, because failing toward noise is the point — the +// failure this guards against is silent. + +// AllowWritesWhileStudioProOpen disables the guard for this process. It is the +// `--force` of the CLI; nothing sets it implicitly. +var AllowWritesWhileStudioProOpen bool + +// AllowStudioProOpenEnv is the environment variable that disables the guard, +// for commands that have no --force of their own (REPL, -c, other writers). +const AllowStudioProOpenEnv = "MXCLI_ALLOW_STUDIO_PRO_OPEN" + +// StudioProOpenError reports a refused write. +type StudioProOpenError struct { + MprPath string + LockPath string + LockTime time.Time +} + +func (e *StudioProOpenError) Error() string { + return fmt.Sprintf("refusing to write %s: Studio Pro has this project open (lock file %s, last modified %s). "+ + "Studio Pro does not reload the model from disk, so its next save would silently discard this change. "+ + "Close the project in Studio Pro and retry, or write through Studio Pro with --mcp. "+ + "If Studio Pro is not running (a lock left behind by a crash), delete the lock file, "+ + "or re-run with --force (exec) or %s=1 to write anyway", + filepath.Base(e.MprPath), e.LockPath, e.LockTime.Format(time.RFC3339), AllowStudioProOpenEnv) +} + +// StudioProLockFile returns the path of the Studio Pro lock file beside mprPath, +// or "" when there is none. +func StudioProLockFile(mprPath string) (string, time.Time) { + if mprPath == "" { + return "", time.Time{} + } + dir, base := filepath.Split(mprPath) + if dir == "" { + dir = "." + } + want := base + ".lock" + entries, err := os.ReadDir(dir) + if err != nil { + return "", time.Time{} + } + for _, e := range entries { + if e.IsDir() || !strings.EqualFold(e.Name(), want) { + continue + } + var mod time.Time + if info, err := e.Info(); err == nil { + mod = info.ModTime() + } + return filepath.Join(dir, e.Name()), mod + } + return "", time.Time{} +} + +// studioProOpenGuard is called immediately before a write reaches storage. +func studioProOpenGuard(mprPath string) error { + if AllowWritesWhileStudioProOpen || os.Getenv(AllowStudioProOpenEnv) == "1" { + return nil + } + lock, mod := StudioProLockFile(mprPath) + if lock == "" { + return nil + } + return &StudioProOpenError{MprPath: mprPath, LockPath: lock, LockTime: mod} +} + +// guardWrite applies the guard to this writer's project. +func (w *Writer) guardWrite() error { + return studioProOpenGuard(w.reader.path) +} diff --git a/modelsdk/mpr/studiopro_lock_test.go b/modelsdk/mpr/studiopro_lock_test.go new file mode 100644 index 000000000..cd0656561 --- /dev/null +++ b/modelsdk/mpr/studiopro_lock_test.go @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mpr + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +// mendixlabs/mxcli#849: a file-based write while Studio Pro has the project open +// is silently discarded by Studio Pro's next save. The writer refuses it when +// Studio Pro's lock file is beside the .mpr. + +const lockUnitID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + +// studioProLock creates the lock file the way Studio Pro names it: the project +// name lower-cased (Studio Pro's own .gitignore lists `testapp.mpr.lock` beside +// `TestApp.mpr`), here against the fixture's `app.mpr`, so it is spelled in +// upper case to prove the match ignores case. +func studioProLock(t *testing.T, w *Writer) string { + t.Helper() + p := filepath.Join(filepath.Dir(w.reader.path), "APP.MPR.LOCK") + if err := os.WriteFile(p, nil, 0644); err != nil { + t.Fatalf("create lock: %v", err) + } + return p +} + +func TestStudioProOpen_WriteRefused(t *testing.T) { + t.Setenv(AllowStudioProOpenEnv, "") + stored := unitDoc(t, "Before") + w, unitPath := newV2WriterForCommitTest(t, lockUnitID, stored) + studioProLock(t, w) + + err := w.UpdateRawUnit(lockUnitID, unitDoc(t, "After")) + var spErr *StudioProOpenError + if !errors.As(err, &spErr) { + t.Fatalf("UpdateRawUnit with Studio Pro open: err = %v, want *StudioProOpenError", err) + } + if onDisk, _ := os.ReadFile(unitPath); string(onDisk) != string(stored) { + t.Error("a refused write reached disk") + } + if err := w.DeleteUnit(lockUnitID); !errors.As(err, &spErr) { + t.Errorf("DeleteUnit with Studio Pro open: err = %v, want *StudioProOpenError", err) + } + if err := w.InsertUnit("bbbbbbbb-bbbb-cccc-dddd-eeeeeeeeeeee", lockUnitID, "Documents", "Projects$Folder", unitDoc(t, "New")); !errors.As(err, &spErr) { + t.Errorf("InsertUnit with Studio Pro open: err = %v, want *StudioProOpenError", err) + } + + // Reads are never refused. + if got, err := w.reader.GetRawUnitBytes(lockUnitID); err != nil || string(got) != string(stored) { + t.Errorf("read with Studio Pro open: err = %v", err) + } +} + +// A write that reconciliation elides never reaches storage, so it is not +// refused: re-running an applied script with Studio Pro open stays a no-op +// rather than an error. +func TestStudioProOpen_NoOpWriteNotRefused(t *testing.T) { + t.Setenv(AllowStudioProOpenEnv, "") + stored := unitDoc(t, "Same") + w, _ := newV2WriterForCommitTest(t, lockUnitID, stored) + studioProLock(t, w) + if err := w.UpdateRawUnit(lockUnitID, unitDoc(t, "Same")); err != nil { + t.Fatalf("no-op write with Studio Pro open refused: %v", err) + } +} + +// Controls: the same write lands without a lock file, and with the override. +func TestStudioProOpen_Controls(t *testing.T) { + t.Setenv(AllowStudioProOpenEnv, "") + t.Run("NoLock", func(t *testing.T) { + w, unitPath := newV2WriterForCommitTest(t, lockUnitID, unitDoc(t, "Before")) + after := unitDoc(t, "After") + if err := w.UpdateRawUnit(lockUnitID, after); err != nil { + t.Fatalf("UpdateRawUnit without a lock: %v", err) + } + if onDisk, _ := os.ReadFile(unitPath); string(onDisk) != string(after) { + t.Error("write did not land") + } + }) + t.Run("Env", func(t *testing.T) { + t.Setenv(AllowStudioProOpenEnv, "1") + w, unitPath := newV2WriterForCommitTest(t, lockUnitID, unitDoc(t, "Before")) + studioProLock(t, w) + after := unitDoc(t, "After") + if err := w.UpdateRawUnit(lockUnitID, after); err != nil { + t.Fatalf("UpdateRawUnit with %s=1: %v", AllowStudioProOpenEnv, err) + } + if onDisk, _ := os.ReadFile(unitPath); string(onDisk) != string(after) { + t.Error("write did not land") + } + }) + t.Run("Force", func(t *testing.T) { + AllowWritesWhileStudioProOpen = true + t.Cleanup(func() { AllowWritesWhileStudioProOpen = false }) + w, _ := newV2WriterForCommitTest(t, lockUnitID, unitDoc(t, "Before")) + studioProLock(t, w) + if err := w.UpdateRawUnit(lockUnitID, unitDoc(t, "After")); err != nil { + t.Fatalf("UpdateRawUnit with --force: %v", err) + } + }) +} + +func TestStudioProLockFile_MatchesOnlyTheProjectsLock(t *testing.T) { + dir := t.TempDir() + mpr := filepath.Join(dir, "TestApp.mpr") + for _, n := range []string{"Other.mpr.lock", "TestApp.mpr.bak", "testapp.mpr.lock.old"} { + if err := os.WriteFile(filepath.Join(dir, n), nil, 0644); err != nil { + t.Fatal(err) + } + } + if got, _ := StudioProLockFile(mpr); got != "" { + t.Fatalf("StudioProLockFile = %q with no lock for this project", got) + } + if err := os.WriteFile(filepath.Join(dir, "testapp.mpr.lock"), nil, 0644); err != nil { + t.Fatal(err) + } + if got, _ := StudioProLockFile(mpr); filepath.Base(got) != "testapp.mpr.lock" { + t.Fatalf("StudioProLockFile = %q, want the lower-cased lock Studio Pro writes", got) + } +} diff --git a/modelsdk/mpr/writer_core.go b/modelsdk/mpr/writer_core.go index 983cb2a30..3c06e84cf 100644 --- a/modelsdk/mpr/writer_core.go +++ b/modelsdk/mpr/writer_core.go @@ -230,6 +230,9 @@ func (wt *WriteTransaction) WriteUnit(unitID string, contents []byte) error { if unchanged { return nil } + if err := wt.writer.guardWrite(); err != nil { + return err + } unitIDBlob := uuidToBlob(unitID) @@ -538,6 +541,9 @@ func (w *Writer) insertUnit(unitID, containerID, containmentName, unitType strin if err := canon.BareAttributeRefError(unitID, contents); err != nil { return err } + if err := w.guardWrite(); err != nil { + return err + } // Convert UUID strings to 16-byte blobs for database unitIDBlob := uuidToBlob(unitID) @@ -647,6 +653,9 @@ func (w *Writer) updateUnit(unitID string, contents []byte, opts ...canon.Option if unchanged { return nil } + if err := w.guardWrite(); err != nil { + return err + } // Convert UUID string to 16-byte blob unitIDBlob := uuidToBlob(unitID) @@ -894,6 +903,9 @@ func (w *Writer) MoveUnit(unitID, newContainerID string) error { if stored, err := w.containerOfUnit(unitID); err == nil && bytes.Equal(stored, target) { return nil } + if err := w.guardWrite(); err != nil { + return err + } _, err := w.reader.db.Exec(`UPDATE Unit SET ContainerID = ? WHERE UnitID = ?`, target, uuidToBlob(unitID)) if err == nil { @@ -919,6 +931,9 @@ func (w *Writer) deleteUnit(unitID string) error { if unitIDBlob == nil { return fmt.Errorf("invalid unit ID: %s", unitID) } + if err := w.guardWrite(); err != nil { + return err + } w.dropDeferred(unitID) w.rememberRemovedUnit(unitID) @@ -968,6 +983,9 @@ func (w *Writer) UpdateUnitContainer(unitID, newContainerID string) error { if containerIDBlob == nil { return fmt.Errorf("invalid container ID: %s", newContainerID) } + if err := w.guardWrite(); err != nil { + return err + } result, err := w.reader.db.Exec(`UPDATE Unit SET ContainerID = ? WHERE UnitID = ?`, containerIDBlob, unitIDBlob) if err != nil {