fix: roundtrip REST mapping cardinality via as list of syntax

[hjotha]

Summary

Restore correct cardinality (single object vs list) for REST-call import mappings, especially those backed by XML schemas or message definitions. The fix has three layers: (1) make the JSON-structure lookup work through folder hierarchies, (2) fix the parser so MinOccurs/MaxOccurs survive the BSON read, and (3) propagate the call-site cardinality through MDL using new as list of Module.Entity syntax instead of guessing from the import mapping shape.

Why the call site needs explicit syntax

Studio Pro stores the cardinality on the microflow's ImportMappingCall, not on the import mapping itself:

Microflow	Range.SingleObject	ForceSingleOccurrence	Authored result
PrivateCloudData.REST_GetEnvironmentByUUID	false	true	single Object
MendixSSO.RetrieveUserRoles	false	false	list

Both bind the same shape of mapping element (Kind: "Object", MaxOccurs: -1). The user's choice is at the call site. Earlier iterations of this PR tried to derive cardinality from MaxOccurs alone (-1 ⇒ list); that classified PCD as a list and produced CE0117 "Error(s) in expression." at the End event, where the microflow returns a single PrivateCloudData.Environment.

Fixes

1. Folder-nested JsonStructure lookup — GetJsonStructureByQualifiedName matched the stored ContainerID directly against the module ID, so structures inside module subfolders (Module/Sub/Json) were reported as not found. Walk the container parent chain via buildContainerModuleNameMap, the same helper GetImportMappingByQualifiedName already uses.
2. Fall back to import mapping root — when JsonStructure is empty (XML schema / message definition), use the mapping's own root element kind as the cardinality hint when MDL omits the explicit as list of form.
3. Parse MinOccurs / MaxOccurs — the MPR reader silently dropped these on ImportMappings$ObjectMappingElement / ImportMappings$ValueMappingElement because Mendix authors them as int64 while the existing JsonElement parser only handles int32. Add a small bsonInt helper that accepts both.
4. as list of Module.Entity syntax — new MDL form to mark a list result. Pipeline:
   • Grammar (MDLMicroflow.g4): adds RETURNS MAPPING qn AS LIST_OF qn ahead of the bare AS qn form.
   • AST (RestResult.IsList): records the cardinality.
   • Visitor: sets IsList when LIST_OF() is present in the parse tree.
   • Describer (formatRestCallAction): emits as list of Module.Entity when rh.SingleObject is false; as Module.Entity otherwise.
   • Builder (addRestCallAction): trusts s.Result.IsList to set both SingleObject and ForceSingleOccurrence (mirrored), so the writer reproduces the BSON shape Studio Pro emits.

MDL syntax

-- Single object — first item from a list-typed mapping (PCD pattern)
$Item = rest call get $URL
  returns mapping Module.IMM as Module.Item;

-- List result (MendixSSO pattern)
$Items = rest call get $URL
  returns mapping Module.IMM as list of Module.Item;

Reproduction

A REST call binding a single object via a list-typed mapping:

$Environment = rest call get $URL
  returns mapping PrivateCloudData.IMM_EnvironmentByUUID as PrivateCloudData.Environment;
return $Environment;

Before this PR: mx check reports CE0117 "Error(s) in expression." at End event because the result was forced to a ListType while the microflow returns a single Object. After this PR: 0 errors; the result is correctly typed and the BSON roundtrip preserves both Range.SingleObject and ForceSingleOccurrence.

Test plan

• TestGetJsonStructureByQualifiedName_ResolvesThroughFolders — folder-nested JSON structure lookup.
• TestAddRestCallAction_MappingAsEntityProducesSingleObject — as Module.Entity (no list of) ⇒ SingleObject=true, ForceSingleOccurrence=true.
• TestAddRestCallAction_MappingAsListOfEntityProducesListResult — as list of Module.Entity ⇒ SingleObject=false, ForceSingleOccurrence=false.
• TestFormatRestCallAction_MappingSingleObject — describer emits as for SingleObject=true.
• TestFormatRestCallAction_MappingListOf — describer emits as list of for SingleObject=false.
• TestAddImportFromMappingFallsBackToImportMappingRootForListResult / …ForSingleObject — heuristic fallback for IMPORT FROM MAPPING (no MDL syntax change there).
• go test ./... passes.
• Roundtrip audit on the seven microflows that exercise the touched paths (PCD REST_GetEnvironmentByUUID, MendixSSO RetrieveUserRoles, plus five others): six match with zero mx check errors; PCD now matches where it previously failed CE0117. The seventh (SBOMData.REST_GetSBOMByComponentId) was a pre-existing whitespace-only cosmetic mismatch unrelated to this change.

🤖 Generated with Claude Code

[github-actions]

AI Code Review

What Looks Good

The PR presents a well-focused bug fix that correctly addresses the issue of incorrect SingleObject determination for import mappings backed by XML schemas or message definitions (which lack JsonStructure).

Key strengths:

• The fix correctly implements a fallback to check the import mapping's root element kind when JsonStructure is empty
• Applied symmetrically to both addRestCallAction and addImportFromMappingAction for consistency
• Includes comprehensive unit tests covering both Object and Array root cases
• Maintains backward compatibility by preserving the existing JSON-structure check as the primary path
• Code changes are clear, well-commented, and follow existing patterns in the file
• Verified through unit tests, full test suite, and roundtrip audit showing zero regressions
• No modifications to MDL syntax, so syntax design and full-stack consistency checks don't apply

The fix resolves the reported CE0117 expression errors by ensuring the generated BSON correctly matches the microflow's declared return type when using REST calls with schema/message-definition mappings.

Recommendation

Approve - This is a clean, focused bug fix with appropriate test coverage that resolves the issue without introducing regressions or violating any project guidelines.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

[ako]

Review

The core fix is sound: falling back to ImportMappingElement.Kind for message-definition- and XML-schema-backed mappings (no JsonStructure) correctly resolves the CE0117/CE0019/CE0136 cascade. The Object-fallback path in addRestCallAction is correct and addresses the real bug.

---

Moderate

M1 — Array fallback in addImportFromMappingAction is dead code

parseImportMappingElement only ever sets Kind to "Object" (ImportMappings$ObjectMappingElement) or "Value" (ImportMappings$ValueMappingElement). There is no ImportMappings$ArrayMappingElement branch — unrecognised $Type values fall through to default: return nil, and nil elements are never appended to im.Elements. So im.Elements[0].Kind == "Array" can never be true from real MPR data.

// sdk/mpr/parser_import_mapping.go
switch typeName {
case "ImportMappings$ObjectMappingElement":   // → Kind: "Object"
    return parseImportObjectMappingElement(raw)
case "ImportMappings$ValueMappingElement":    // → Kind: "Value"
    return parseImportValueMappingElement(raw)
default:
    return nil  // any Array element type falls here and is dropped
}

TestAddRestCallAction_MappingFallsBackToArrayKindWhenJsonStructureMissing constructs Kind: "Array" in a mock — a state the parser cannot produce — so it provides false assurance for a dead branch.

Fix options (pick one):

• Verify with a real Studio Pro BSON dump of an XML-schema-backed array-rooted import mapping. If ImportMappings$ArrayMappingElement exists as a $Type, add a parser case; the Array fallback and test then become meaningful.
• If no such $Type exists in real MPR files, remove the Array fallback from addImportFromMappingAction and drop the Array-kind test.

M2 — No test for addImportFromMappingAction with the new message-definition fallback

Two new tests cover the REST call path but none exercise addImportFromMappingAction with JsonStructure = "" and an Object-rooted element. The resolved flag + Object fallback in that function is equally new and untested.

---

Minor

m3 — Nil guard inconsistency

The new fallback guards im.Elements[0] != nil, but the immediately following pre-existing entity lookup does not:

if len(im.Elements) > 0 && im.Elements[0].Entity != "" {  // no nil guard

Low risk in production (parser never appends nil), but inconsistent with the guard added two lines above.

---

Verdict

Approve after M1 and M2. M1 in particular could mislead a future contributor into believing array-rooted message-definition mappings work end-to-end when the parser silently drops them.

[github-actions]

AI Code Review

What Looks Good:

• The PR successfully fixes a specific bug where import mappings backed by XML schema or message definitions (lacking JsonStructure) incorrectly defaulted to ListType results
• The solution is well-designed: falls back to checking the import mapping's root element kind when JsonStructure is empty, properly handling MaxOccurs to distinguish singleton vs. repeating elements
• Changes are applied consistently to both addRestCallAction and addImportFromMappingAction for symmetry
• Comprehensive test coverage added including edge cases (Object root, Array root, repeating Object with unbounded/bounded MaxOccurs)
• Existing tests still pass and roundtrip audit shows no regressions
• BSON parser enhancements (bsonInt helper, MinOccurs/MaxOccurs parsing) are necessary and well-scoped
• Code includes clear comments explaining the Studio Pro modeling behavior for repeating elements
• Follows existing patterns in the codebase (similar JSON structure lookup logic)
• No unnecessary changes outside the scope of the fix

Recommendation: Approve

The PR is focused, correctly implements the fallback logic described in the issue, includes appropriate tests, and maintains backward compatibility. The fix addresses the root cause (missing JsonStructure for schema-backed mappings) while preserving existing behavior for JSON-backed mappings. All checklist items are satisfied for this type of bug fix.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

[hjotha]

Thanks @ako — addressed in c393ddee:

M1 — dead Array branch removed. You're right: parseImportMappingElement only ever produces "Object" or "Value", so the Kind == "Array" check was unreachable from real MPR data. Both fallback sites (addRestCallAction and addImportFromMappingAction) now key on MaxOccurs == -1 || MaxOccurs > 1 only — repetition is the actual discriminator for repeating-Object roots in real authored BSON. The TestAddRestCallAction_MappingFallsBackToArrayKindWhenJsonStructureMissing test is removed alongside the dead branch (mock-only state).

M2 — addImportFromMappingAction fallback now has tests. Two new cases in cmd_microflows_builder_import_mapping_test.go:

• TestAddImportFromMappingFallsBackToImportMappingRootForListResult — Object root with MaxOccurs: -1 → List of <Entity>.
• TestAddImportFromMappingFallsBackToImportMappingRootForSingleObject — Object root with MaxOccurs: 1 → singleton.

m3 — guard parity. Added the im.Elements[0] != nil guard before the pre-existing entity lookup so it matches the guard above. (Low-risk in practice since the parser never appends nil, but eliminates the inconsistency.)

If we ever discover an ImportMappings$ArrayMappingElement (or similar) $Type in a real MPR dump, the parser dispatcher in parser_import_mapping.go is the single place that needs a new case — the fallback no longer needs to special-case it.

[ako]

Review (updated)

All three items from the first round have been addressed or superseded. Two things left.

---

Moderate

M1 — bsonInt duplicates extractInt in the same package

parser.go already defines:

func extractInt(v any) int {
    if v == nil { return 0 }
    switch val := v.(type) {
    case int32:  return int(val)
    case int64:  return int(val)
    case int:    return val
    case float64: return int(val)
    }
    return 0
}

The new bsonInt in parser_import_mapping.go is identical except it omits the float64 case and the nil guard. Both are in package mpr. Replace bsonInt with extractInt throughout parser_import_mapping.go and delete the new helper.

---

Minor

m2 — TestAddRestCallAction_MappingFallsBackToArrayKindWhenJsonStructureMissing tests an unreachable state

The test comment says "an Array root on the mapping element must yield a list-typed result handling" but exercises Kind: "Array" — a value parseImportMappingElement never produces (ImportMappings$ObjectMappingElement → "Object", ImportMappings$ValueMappingElement → "Value"; no Array branch). The practical list-from-root path is MaxOccurs > 1 / -1, which is already well-covered by the Unbounded and Bounded repeat tests. This test doesn't cause a bug but documents a non-existent input shape. Low priority — rename it or add a comment that it represents a hypothetical future parser extension.

m3 — Nil guard inconsistency (pre-existing, now more visible) im.Elements[0] != nil is checked inside the new fallback, but the immediately following entity-lookup im.Elements[0].Entity != "" (unchanged from before this PR) has no nil guard.

---

What's improved vs round 1

• Folder-hierarchy fix for GetJsonStructureByQualifiedName with its own parser-level test ✓
• MaxOccurs > 1 / -1 reliably detects list regardless of Kind, covering the real Studio Pro data ✓
• addImportFromMappingAction fallback now tested (Object singleton + unbounded list) ✓
• MinOccurs / MaxOccurs are now parsed from BSON ✓

Approve after the bsonInt → extractInt substitution.

[hjotha]

Thanks @ako — addressed in dcf5b7c3.

M1 — bsonInt → extractInt

You're right, extractInt in parser.go already handles int32 / int64 / int / float64 with a nil guard, which is a strict superset of bsonInt. Replaced both MinOccurs / MaxOccurs call sites and removed the helper.

m2 — unreachable Array-kind test

Already removed in the previous fixup c393ddee (the dead Kind == "Array" branch and its test). No leftover Array-kind code or tests.

m3 — nil guard inconsistency

Fixed in c393ddee as well — im.Elements[0] is now nil-guarded before the entity-name lookup.

The PR now also adds new MDL syntax as list of Module.Entity (commit c4febc1d) so the cardinality round-trips correctly even for cases like PCD's REST_GetEnvironmentByUUID, where the underlying mapping is list-typed (MaxOccurs=-1) but the call site binds a single Object via ForceSingleOccurrence=true. The MaxOccurs heuristic alone couldn't tell those apart.