From 8f5028a038ef3a47c915b36ebb10bf5736a1c666 Mon Sep 17 00:00:00 2001
From: Eric Peterson <eric@elpete.com>
Date: Mon, 4 May 2020 10:42:15 -0600
Subject: [PATCH] fix(JoinClause): Prevent duplicate joins when using closure
 syntax

---
 ModuleConfig.cfc                              |  2 +-
 models/Query/QueryBuilder.cfc                 | 32 ++++++++++++-------
 tests/specs/Query/Abstract/JoinClauseSpec.cfc | 10 ++++++
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/ModuleConfig.cfc b/ModuleConfig.cfc
index 862730fa..3ce09761 100644
--- a/ModuleConfig.cfc
+++ b/ModuleConfig.cfc
@@ -23,7 +23,7 @@ component {
             .to( "qb.models.Query.QueryBuilder" )
             .initArg( name = "grammar", ref = settings.defaultGrammar )
             .initArg( name = "utils", ref = "QueryUtils@qb" )
-            .initArg( name = "preventDuplicateJoints", value = settings.preventDuplicateJoins ) 
+            .initArg( name = "preventDuplicateJoints", value = settings.preventDuplicateJoins )
             .initArg( name = "returnFormat", value = settings.defaultReturnFormat );
 
         binder.map( alias = "SchemaBuilder@qb", force = true )
diff --git a/models/Query/QueryBuilder.cfc b/models/Query/QueryBuilder.cfc
index 32af42d3..6dacb19e 100644
--- a/models/Query/QueryBuilder.cfc
+++ b/models/Query/QueryBuilder.cfc
@@ -529,19 +529,17 @@ component displayname="QueryBuilder" accessors="true" {
 
         var join = new qb.models.Query.JoinClause( parentQuery = this, type = arguments.type, table = arguments.table );
 
-        if ( arguments.preventDuplicateJoins ) {
-            var hasThisJoin = variables.joins.find( function( existingJoin ) {
-                return existingJoin.isEqualTo( join );
-            } );
-
-            if ( hasThisJoin ) {
-                return this;
-            }
-        }
-
-
         if ( isClosure( arguments.first ) || isCustomFunction( arguments.first ) ) {
             first( join );
+            if ( arguments.preventDuplicateJoins ) {
+                var hasThisJoin = variables.joins.find( function( existingJoin ) {
+                    return existingJoin.isEqualTo( join );
+                } );
+
+                if ( hasThisJoin ) {
+                    return this;
+                }
+            }
             variables.joins.append( join );
             addBindings( join.getBindings(), "join" );
             return this;
@@ -550,7 +548,17 @@ component displayname="QueryBuilder" accessors="true" {
         var method = where ? "where" : "on";
         arguments.column = arguments.first;
         arguments.value = isNull( arguments.second ) ? javacast( "null", "" ) : arguments.second;
-        variables.joins.append( invoke( join, method, arguments ) );
+        join = invoke( join, method, arguments );
+        if ( arguments.preventDuplicateJoins ) {
+            var hasThisJoin = variables.joins.find( function( existingJoin ) {
+                return existingJoin.isEqualTo( join );
+            } );
+
+            if ( hasThisJoin ) {
+                return this;
+            }
+        }
+        variables.joins.append( join );
         addBindings( join.getBindings(), "join" );
 
         return this;
diff --git a/tests/specs/Query/Abstract/JoinClauseSpec.cfc b/tests/specs/Query/Abstract/JoinClauseSpec.cfc
index 72050d27..f11c462c 100644
--- a/tests/specs/Query/Abstract/JoinClauseSpec.cfc
+++ b/tests/specs/Query/Abstract/JoinClauseSpec.cfc
@@ -260,6 +260,16 @@ component extends="testbox.system.BaseSpec" {
                         expect( variables.qb.getJoins().len() ).toBe( 1 );
                     } );
 
+                    it( "will prevent an identical join from being added using the closure syntax when preventDuplicateJoins is true", function() {
+                        variables.qb.join( "secondTable", function( j ) {
+                            j.on( "secondTable.id", "firstTable.secondId" );
+                        } );
+                        variables.qb.join( "secondTable", function( j ) {
+                            j.on( "secondTable.id", "firstTable.secondId" );
+                        } );
+                        expect( variables.qb.getJoins().len() ).toBe( 1 );
+                    } );
+
                     it( "will allow an identical join from being added when preventDuplicateJoins is false", function() {
                         variables.qb.setPreventDuplicateJoins( false );
                         variables.qb.join( variables.join );