From 8a983ac4d6eca4ccf41ad9561121a9d58d871ed6 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Tue, 5 May 2026 11:39:32 +0200 Subject: [PATCH] fix: classify reverse-Reference traversal through entity inheritance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit addRetrieveAction decided whether `retrieve $X from $V/M.Assoc` was a reverse traversal (child-side → parent-side, returning a list) by comparing the start variable's entity type to the association's child entity name *exactly*. Inheritance broke that check: when the start variable was a subclass of the association's child entity (e.g. a HttpRequest variable traversing a HttpHeaders association whose Child is HttpMessage), the comparison missed and the result variable was typed as the parent entity *singleton* instead of `List of `. Downstream `find($List, attr = value)` then dispatched to `FindByExpression` instead of `Find` because resolveListOperationMember read the variable type as a non-list. Studio Pro authors `Find` for the simple equality shape; emitting `FindByExpression` triggers `mx check` CE0117 "Error in expression at List operation activity 'Find by expression'" because the runtime can no longer see the qualified attribute path the model expects. Introduce entityIsSubtypeOf, an inheritance-aware walk (mirrors the existing resolveAttributeInEntityHierarchy traversal), and replace the exact-match `startVarType == childEntityQN` checks in addRetrieveAction with `entityIsSubtypeOf(startVarType, childEntityQN)`. The replacement covers both the expandReverseReference DatabaseRetrieveSource branch and the AssociationRetrieveSource fallback that registers the result type. Direct (non-inheriting) traversal still works the same way; the helper short-circuits on identity. Two regression tests: - reverse Reference through an inherited child registers `List of ` (covers the original bug) - reverse Reference with an exact child match still registers `List of ` (guards the non-inheritance path) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cmd_microflows_builder_actions.go | 59 ++++++- ...builder_retrieve_inherited_subtype_test.go | 150 ++++++++++++++++++ 2 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 mdl/executor/cmd_microflows_builder_retrieve_inherited_subtype_test.go diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index f1db1c8b..b8eb377c 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -541,6 +541,14 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { outputUsedAsList := fb.listInputVariables != nil && fb.listInputVariables[s.Variable] outputUsedAsObject := fb.objectInputVariables != nil && fb.objectInputVariables[s.Variable] + // startsFromChildSide is true when the retrieve's start variable is the + // child side of the association (or a subclass of it). Inheritance has + // to be honoured so traversals like `$httpRequest/System.HttpHeaders` + // — where HttpRequest extends HttpMessage and HttpHeaders has child + // HttpMessage — are still classified as reverse traversal. + startsFromChildSide := assocInfo != nil && + assocInfo.childEntityQN != "" && + fb.entityIsSubtypeOf(startVarType, assocInfo.childEntityQN) // Owner-both Reference associations need later usage context: the same // compact retrieve can be consumed as either a list or a single object. // Owner="" means metadata was unavailable, so keep the association source. @@ -549,7 +557,7 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { assocInfo.Owner != "" && assocInfo.parentPersistable && assocInfo.childEntityQN != "" && - startVarType == assocInfo.childEntityQN && + startsFromChildSide && (assocInfo.Owner != domainmodel.AssociationOwnerBoth || (outputUsedAsList && !outputUsedAsObject)) if expandReverseReference { @@ -577,10 +585,10 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { // non-persistable reverse traversal can still use association // source syntax, but keeps list typing for downstream actions. otherEntity := assocInfo.childEntityQN - if startVarType == assocInfo.childEntityQN { + if startsFromChildSide { otherEntity = assocInfo.parentEntityQN } - if startVarType == assocInfo.childEntityQN && !outputUsedAsObject { + if startsFromChildSide && !outputUsedAsObject { fb.varTypes[s.Variable] = "List of " + otherEntity } else { fb.varTypes[s.Variable] = otherEntity @@ -589,7 +597,7 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { // ReferenceSet traversal returns a list of the entity on the other side, // not a list typed as the association itself. otherEntity := assocInfo.childEntityQN - if startVarType == assocInfo.childEntityQN { + if startsFromChildSide { otherEntity = assocInfo.parentEntityQN } if otherEntity != "" { @@ -1284,6 +1292,49 @@ func (fb *flowBuilder) resolveAttributeInEntityHierarchy(entityQN, attrName stri return "", false } +// entityIsSubtypeOf reports whether candidateQN is the same as ancestorQN or +// inherits from it through the generalization chain. The walk consults the +// domain model the same way resolveAttributeInEntityHierarchy does. +func (fb *flowBuilder) entityIsSubtypeOf(candidateQN, ancestorQN string) bool { + if candidateQN == "" || ancestorQN == "" { + return false + } + if candidateQN == ancestorQN { + return true + } + if fb == nil || fb.backend == nil { + return false + } + seen := make(map[string]bool) + for currentQN := candidateQN; currentQN != ""; { + if seen[currentQN] { + return false + } + seen[currentQN] = true + if currentQN == ancestorQN { + return true + } + parts := strings.SplitN(currentQN, ".", 2) + if len(parts) != 2 { + return false + } + mod, err := fb.backend.GetModuleByName(parts[0]) + if err != nil || mod == nil { + return false + } + dm, err := fb.backend.GetDomainModel(mod.ID) + if err != nil || dm == nil { + return false + } + entity := dm.FindEntityByName(parts[1]) + if entity == nil { + return false + } + currentQN = entity.GeneralizationRef + } + return false +} + // resolveMemberChangeFallback preserves the authored member name shape when the // entity metadata is unavailable. // diff --git a/mdl/executor/cmd_microflows_builder_retrieve_inherited_subtype_test.go b/mdl/executor/cmd_microflows_builder_retrieve_inherited_subtype_test.go new file mode 100644 index 00000000..ebdf9595 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_retrieve_inherited_subtype_test.go @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// Reverse-traversal type registration must follow the entity inheritance +// chain. When a Reference association has Parent=Header / Child=Message +// and a microflow holds a $Request variable typed as a Request entity that +// extends Message, traversing $Request/Module.Headers should be classified +// as reverse traversal — yielding `List of Header` — even though +// `Request != Message` exactly. Failing this check leaves the result +// variable typed as the parent (Message) singleton, which downstream +// list-operation builders read as a non-list and emit FindByExpression +// instead of Find with a qualified attribute, triggering CE0117 in mx +// check on Studio Pro. +func TestAddRetrieveAction_ReverseRefThroughInheritedChild(t *testing.T) { + moduleID := model.ID("synthetic-module") + headerID := model.ID("header-entity") + messageID := model.ID("message-entity") + requestID := model.ID("request-entity") + fb := &flowBuilder{ + varTypes: map[string]string{ + // $Request is a Request, which extends Message + "Request": "Synthetic.Request", + }, + listInputVariables: map[string]bool{ + // Result is consumed as a list (drives the owner-both branch) + "HeaderList": true, + }, + backend: &mock.MockBackend{ + GetModuleByNameFunc: func(name string) (*model.Module, error) { + if name != "Synthetic" { + return nil, nil + } + return &model.Module{BaseElement: model.BaseElement{ID: moduleID}, Name: name}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { + if id != moduleID { + return nil, nil + } + return &domainmodel.DomainModel{ + ContainerID: moduleID, + Entities: []*domainmodel.Entity{ + { + BaseElement: model.BaseElement{ID: headerID}, + Name: "Header", + Persistable: true, + }, + { + BaseElement: model.BaseElement{ID: messageID}, + Name: "Message", + Persistable: true, + }, + { + BaseElement: model.BaseElement{ID: requestID}, + Name: "Request", + Persistable: true, + GeneralizationRef: "Synthetic.Message", + }, + }, + Associations: []*domainmodel.Association{ + { + Name: "Headers", + ParentID: headerID, + ChildID: messageID, + Type: domainmodel.AssociationTypeReference, + Owner: domainmodel.AssociationOwnerDefault, + }, + }, + }, nil + }, + }, + } + + fb.addRetrieveAction(&ast.RetrieveStmt{ + Variable: "HeaderList", + StartVariable: "Request", + Source: ast.QualifiedName{Module: "Synthetic", Name: "Headers"}, + }) + + got := fb.varTypes["HeaderList"] + want := "List of Synthetic.Header" + if got != want { + t.Errorf("varTypes[HeaderList] = %q, want %q", got, want) + } +} + +// Direct exact-match (no inheritance involved) must continue to be +// classified as reverse traversal — guards against the helper accidentally +// regressing the non-inheritance path. +func TestAddRetrieveAction_ReverseRefDirectChild(t *testing.T) { + moduleID := model.ID("synthetic-module") + headerID := model.ID("header-entity") + messageID := model.ID("message-entity") + fb := &flowBuilder{ + varTypes: map[string]string{ + "Message": "Synthetic.Message", + }, + listInputVariables: map[string]bool{ + "HeaderList": true, + }, + backend: &mock.MockBackend{ + GetModuleByNameFunc: func(name string) (*model.Module, error) { + if name != "Synthetic" { + return nil, nil + } + return &model.Module{BaseElement: model.BaseElement{ID: moduleID}, Name: name}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { + if id != moduleID { + return nil, nil + } + return &domainmodel.DomainModel{ + ContainerID: moduleID, + Entities: []*domainmodel.Entity{ + {BaseElement: model.BaseElement{ID: headerID}, Name: "Header", Persistable: true}, + {BaseElement: model.BaseElement{ID: messageID}, Name: "Message", Persistable: true}, + }, + Associations: []*domainmodel.Association{ + { + Name: "Headers", + ParentID: headerID, + ChildID: messageID, + Type: domainmodel.AssociationTypeReference, + Owner: domainmodel.AssociationOwnerDefault, + }, + }, + }, nil + }, + }, + } + + fb.addRetrieveAction(&ast.RetrieveStmt{ + Variable: "HeaderList", + StartVariable: "Message", + Source: ast.QualifiedName{Module: "Synthetic", Name: "Headers"}, + }) + + if got, want := fb.varTypes["HeaderList"], "List of Synthetic.Header"; got != want { + t.Errorf("varTypes[HeaderList] = %q, want %q", got, want) + } +}