Skip to content

Commit 95c612b

Browse files
elpeteelpete
committed
fix(EagerLoading): Fix multiple eager loaded relationships
If you provided multiple eager loaded nested relationships, Quick would mistakenly reload the parent relationship and only load the last child relationship. This is fixed now.
1 parent 0efbecb commit 95c612b

2 files changed

Lines changed: 287 additions & 24 deletions

File tree

‎models/QuickBuilder.cfc‎

Lines changed: 162 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -566,52 +566,190 @@ component accessors="true" transientCache="false" {
566566
}
567567
}
568568

569-
arrayEach( variables._eagerLoad, function( relationName ) {
570-
entities = eagerLoadRelation( relationName, entities );
569+
structEach( denestEagerLoads( variables._eagerLoad ), function( relationName, nestedEagerLoads ) {
570+
entities = eagerLoadRelation(
571+
relationName,
572+
nestedEagerLoads,
573+
entities
574+
);
571575
} );
572576

573577
return arguments.entities;
574578
}
575579

580+
private struct function denestEagerLoads( required array eagerLoads ) {
581+
// this comes in as an array of items which can be:
582+
// 1. dot-delimited strings (e.g., "videos.tags")
583+
// 2. structs with the key being the dot-delimited path and the value being a callback
584+
// these dot-delimited strings can have the same parent
585+
// we want to only load each relationship and its eager loads once
586+
// Result format: { "relationName": { "callback": function, "nested": { ... } } }
587+
var result = {};
588+
589+
for ( var relationshipPath in arguments.eagerLoads ) {
590+
var callback = function() {
591+
};
592+
var pathString = "";
593+
594+
// Handle struct format: { "path.to.relation": callback }
595+
if ( isStruct( relationshipPath ) ) {
596+
for ( var key in relationshipPath ) {
597+
pathString = key;
598+
callback = relationshipPath[ key ];
599+
break;
600+
}
601+
} else {
602+
pathString = relationshipPath;
603+
}
604+
605+
var parts = listToArray( pathString, "." );
606+
var firstPart = parts[ 1 ];
607+
608+
// Initialize the entry if it doesn't exist
609+
if ( !result.keyExists( firstPart ) ) {
610+
result[ firstPart ] = {
611+
"callback" : function() {
612+
},
613+
"nested" : {}
614+
};
615+
}
616+
617+
if ( parts.len() > 1 ) {
618+
// Build the nested path with the callback attached to the deepest level
619+
var nestedPath = arraySlice( parts, 2 ).toList( "." );
620+
var nestedItem = isCustomFunction( callback ) || isClosure( callback )
621+
? { "#nestedPath#" : callback }
622+
: nestedPath;
623+
624+
var nestedResult = denestEagerLoads( [ nestedItem ] );
625+
// Merge nested results
626+
result[ firstPart ][ "nested" ] = mergeNestedEagerLoads( result[ firstPart ][ "nested" ], nestedResult );
627+
} else {
628+
// This is the target level - apply the callback here
629+
if ( isCustomFunction( callback ) || isClosure( callback ) ) {
630+
result[ firstPart ][ "callback" ] = callback;
631+
}
632+
}
633+
}
634+
635+
return result;
636+
}
637+
638+
private struct function mergeNestedEagerLoads( required struct target, required struct source ) {
639+
for ( var key in arguments.source ) {
640+
if ( !arguments.target.keyExists( key ) ) {
641+
arguments.target[ key ] = arguments.source[ key ];
642+
} else {
643+
// Merge callbacks - source callback takes precedence if it's not empty
644+
if (
645+
structKeyExists( arguments.source[ key ], "callback" ) &&
646+
(
647+
isCustomFunction( arguments.source[ key ][ "callback" ] ) || isClosure(
648+
arguments.source[ key ][ "callback" ]
649+
)
650+
)
651+
) {
652+
// Check if source callback has a body (not empty)
653+
arguments.target[ key ][ "callback" ] = arguments.source[ key ][ "callback" ];
654+
}
655+
// Merge nested
656+
if ( structKeyExists( arguments.source[ key ], "nested" ) ) {
657+
arguments.target[ key ][ "nested" ] = mergeNestedEagerLoads(
658+
arguments.target[ key ][ "nested" ],
659+
arguments.source[ key ][ "nested" ]
660+
);
661+
}
662+
}
663+
}
664+
return arguments.target;
665+
}
666+
667+
public array function renestEagerLoads( required struct additionalEagerLoads ) {
668+
// Input format: { "relationName": { "callback": fn, "nested": { ... } } }
669+
// Output format: array of strings or structs like { "path.to.relation": callback }
670+
return structReduce(
671+
arguments.additionalEagerLoads,
672+
function( acc, relationName, eagerLoadConfig ) {
673+
var callback = function() {
674+
};
675+
if ( eagerLoadConfig.keyExists( "callback" ) ) {
676+
callback = eagerLoadConfig.callback;
677+
}
678+
var nestedConfig = {};
679+
if ( eagerLoadConfig.keyExists( "nested" ) ) {
680+
nestedConfig = eagerLoadConfig.nested;
681+
}
682+
var hasCallback = isCustomFunction( callback ) || isClosure( callback );
683+
684+
// Get the renested items from nested config
685+
var nestedItems = renestEagerLoads( nestedConfig );
686+
687+
if ( nestedItems.len() > 0 ) {
688+
// There are nested items - prepend this relationName to each
689+
for ( var nestedItem in nestedItems ) {
690+
if ( isSimpleValue( nestedItem ) ) {
691+
acc.append( relationName & "." & nestedItem );
692+
} else {
693+
// It's a struct with callback - prepend relationName to the key
694+
for ( var key in nestedItem ) {
695+
acc.append( { "#relationName#.#key#" : nestedItem[ key ] } );
696+
break;
697+
}
698+
}
699+
}
700+
} else if ( hasCallback ) {
701+
// No nested, but has a callback - return as struct
702+
acc.append( { "#relationName#" : callback } );
703+
} else {
704+
// No nested, no callback - just the relation name
705+
acc.append( relationName );
706+
}
707+
708+
return acc;
709+
},
710+
[]
711+
);
712+
}
713+
576714
/**
577715
* Eager loads the given relation for the retrieved entities.
578716
* Returns the retrieved entities eager loaded with the given relation.
579717
*
580-
* @relationName The relationship to eager load.
581-
* @entities The retrieved entities or array of structs to eager load the relationship.
718+
* @relationName The relationship to eager load.
719+
* @eagerLoadConfig The config for this eager load containing callback and nested eager loads.
720+
* @entities The retrieved entities or array of structs to eager load the relationship.
582721
*
583722
* @doc_generic quick.models.BaseEntity | struct
584723
* @return [quick.models.BaseEntity] | [struct]
585724
*/
586-
private array function eagerLoadRelation( required any relationName, required array entities ) {
725+
private array function eagerLoadRelation(
726+
required string relationName,
727+
required struct eagerLoadConfig,
728+
required array entities
729+
) {
730+
// Extract callback and nested config from the eagerLoadConfig
587731
var callback = function() {
588732
};
589-
if ( !isSimpleValue( arguments.relationName ) ) {
590-
if ( !isStruct( arguments.relationName ) ) {
591-
throw(
592-
type = "QuickInvalidEagerLoadParameter",
593-
message = "Only strings or structs are supported eager load parameters. You passed [#serializeJSON( arguments.relationName )#"
594-
);
595-
}
596-
for ( var key in arguments.relationName ) {
597-
callback = arguments.relationName[ key ];
598-
arguments.relationName = key;
599-
break;
600-
}
733+
if ( arguments.eagerLoadConfig.keyExists( "callback" ) ) {
734+
callback = arguments.eagerLoadConfig.callback;
601735
}
602-
var currentRelationship = listFirst( arguments.relationName, "." );
603-
var relation = getEntity().ignoreLoadedGuard( function() {
604-
return getEntity().withoutRelationshipConstraints( currentRelationship, function() {
605-
return invoke( getEntity(), currentRelationship );
736+
var nestedEagerLoads = {};
737+
if ( arguments.eagerLoadConfig.keyExists( "nested" ) ) {
738+
nestedEagerLoads = arguments.eagerLoadConfig.nested;
739+
}
740+
741+
var relation = getEntity().ignoreLoadedGuard( function() {
742+
return getEntity().withoutRelationshipConstraints( relationName, function() {
743+
return invoke( getEntity(), relationName );
606744
} );
607745
} );
608746
callback( relation );
609747
var hasMatches = relation.addEagerConstraints( arguments.entities, getEntity() );
610-
relation.with( listRest( arguments.relationName, "." ) );
748+
relation.with( renestEagerLoads( nestedEagerLoads ) );
611749
return relation.match(
612-
relation.initRelation( arguments.entities, currentRelationship ),
750+
relation.initRelation( arguments.entities, arguments.relationName ),
613751
hasMatches ? relation.getEager( variables._asQuery, variables._withAliases ) : [],
614-
currentRelationship
752+
arguments.relationName
615753
);
616754
}
617755

‎tests/specs/integration/BaseEntity/Relationships/EagerLoadingSpec.cfc‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,131 @@ component extends="tests.resources.ModuleIntegrationSpec" {
609609
}
610610
} );
611611
} );
612+
613+
describe( "multiple nested eager loads", () => {
614+
it( "can eager load multiple nested relationships with the same parent using strings", function() {
615+
var users = getInstance( "User" )
616+
.with( [ "posts.tags", "posts.comments" ] )
617+
.latest()
618+
.get();
619+
620+
expect( users ).toBeArray();
621+
expect( users ).toHaveLength( 5, "Five users should be returned" );
622+
623+
// Find elpete who has posts with tags and comments
624+
var elpete = users[ 5 ];
625+
expect( elpete.getUsername() ).toBe( "elpete" );
626+
627+
// Verify posts relationship is loaded
628+
expect( elpete.isRelationshipLoaded( "posts" ) ).toBeTrue( "posts should be loaded" );
629+
expect( elpete.getPosts() ).toBeArray();
630+
expect( elpete.getPosts() ).toHaveLength( 2, "Two posts should belong to elpete" );
631+
632+
// Verify both nested relationships are loaded on the posts
633+
var postWithTagsAndComments = elpete.getPosts()[ 2 ]; // post_pk 1245
634+
expect( postWithTagsAndComments.getPost_Pk() ).toBe( 1245 );
635+
expect( postWithTagsAndComments.isRelationshipLoaded( "tags" ) ).toBeTrue( "tags should be loaded on post" );
636+
expect( postWithTagsAndComments.isRelationshipLoaded( "comments" ) ).toBeTrue( "comments should be loaded on post" );
637+
638+
// Verify the actual data - post 1245 has 2 tags
639+
expect( postWithTagsAndComments.getTags() ).toBeArray();
640+
expect( postWithTagsAndComments.getTags() ).toHaveLength( 2 );
641+
expect( postWithTagsAndComments.getComments() ).toBeArray();
642+
643+
// Should be 4 queries: users, posts, tags, comments
644+
expect( variables.queries ).toHaveLength(
645+
4,
646+
"Four queries should have been executed (users, posts, tags, comments). #arrayLen( variables.queries )# were instead."
647+
);
648+
} );
649+
650+
it( "can eager load multiple nested relationships with the same parent using structs with callbacks", function() {
651+
var users = getInstance( "User" )
652+
.with( [
653+
{
654+
"posts.tags" : function( q ) {
655+
return q.where( "name", "programming" );
656+
}
657+
},
658+
{
659+
"posts.comments" : function( q ) {
660+
return q.where( "designation", "public" );
661+
}
662+
}
663+
] )
664+
.latest()
665+
.get();
666+
667+
expect( users ).toBeArray();
668+
expect( users ).toHaveLength( 5, "Five users should be returned" );
669+
670+
// Find elpete who has posts with tags and comments
671+
var elpete = users[ 5 ];
672+
expect( elpete.getUsername() ).toBe( "elpete" );
673+
674+
// Verify posts relationship is loaded
675+
expect( elpete.isRelationshipLoaded( "posts" ) ).toBeTrue( "posts should be loaded" );
676+
677+
// Verify both nested relationships are loaded on the posts
678+
var postWithTagsAndComments = elpete.getPosts()[ 2 ]; // post_pk 1245
679+
expect( postWithTagsAndComments.getPost_Pk() ).toBe( 1245 );
680+
expect( postWithTagsAndComments.isRelationshipLoaded( "tags" ) ).toBeTrue( "tags should be loaded on post" );
681+
expect( postWithTagsAndComments.isRelationshipLoaded( "comments" ) ).toBeTrue( "comments should be loaded on post" );
682+
683+
// Verify the callbacks were applied - only "programming" tags
684+
var tags = postWithTagsAndComments.getTags();
685+
expect( tags ).toBeArray();
686+
for ( var tag in tags ) {
687+
expect( tag.getName() ).toBe( "programming" );
688+
}
689+
690+
// Verify the callbacks were applied - only "public" comments
691+
var comments = postWithTagsAndComments.getComments();
692+
expect( comments ).toBeArray();
693+
for ( var comment in comments ) {
694+
expect( comment.getDesignation() ).toBe( "public" );
695+
}
696+
697+
// Should be 4 queries: users, posts, tags, comments
698+
expect( variables.queries ).toHaveLength(
699+
4,
700+
"Four queries should have been executed (users, posts, tags, comments). #arrayLen( variables.queries )# were instead."
701+
);
702+
} );
703+
704+
it( "can mix string and struct eager loads with the same parent", function() {
705+
var users = getInstance( "User" )
706+
.with( [
707+
"posts.tags",
708+
{
709+
"posts.comments" : function( q ) {
710+
return q.where( "designation", "public" );
711+
}
712+
}
713+
] )
714+
.latest()
715+
.get();
716+
717+
expect( users ).toBeArray();
718+
expect( users ).toHaveLength( 5, "Five users should be returned" );
719+
720+
var elpete = users[ 5 ];
721+
expect( elpete.getUsername() ).toBe( "elpete" );
722+
723+
var postWithTagsAndComments = elpete.getPosts()[ 2 ];
724+
expect( postWithTagsAndComments.isRelationshipLoaded( "tags" ) ).toBeTrue( "tags should be loaded on post" );
725+
expect( postWithTagsAndComments.isRelationshipLoaded( "comments" ) ).toBeTrue( "comments should be loaded on post" );
726+
727+
// Tags should have all tags (no filter)
728+
expect( postWithTagsAndComments.getTags() ).toBeArray();
729+
730+
// Comments should only have public ones (callback applied)
731+
var comments = postWithTagsAndComments.getComments();
732+
for ( var comment in comments ) {
733+
expect( comment.getDesignation() ).toBe( "public" );
734+
}
735+
} );
736+
} );
612737
} );
613738
}
614739

0 commit comments

Comments
 (0)