Tighten return formatter propagation

Author: elpete

models/Query/QueryBuilder.cfc

@@ -4377,7 +4377,7 @@ component displayname="QueryBuilder" accessors="true" {
      */
     private any function runQuery( required string sql, struct options = {}, string returnObject = "query" ) {
         structAppend( arguments.options, getDefaultOptions(), false );
-        normalizeQueryExecuteReturnTypeOptions( arguments.options );
+        guardAgainstReturnTypeOption( arguments.options );
         var bindings = getBindings( except = getAggregate().isEmpty() ? [] : [ "select" ] );
 
         var result = grammar.runQuery(
@@ -4435,7 +4435,8 @@ component displayname="QueryBuilder" accessors="true" {
             columnFormatter = isNull( getColumnFormatter() ) ? javacast( "null", "" ) : getColumnFormatter(),
             parentQuery = isNull( getParentQuery() ) ? javacast( "null", "" ) : getParentQuery(),
             defaultOptions = getDefaultOptions(),
-            validateQueryExecuteReturnType = getValidateQueryExecuteReturnType()
+            validateQueryExecuteReturnType = getValidateQueryExecuteReturnType(),
+            collectQueryLog = getCollectQueryLog()
         );
     }
 
@@ -4560,7 +4561,6 @@ component displayname="QueryBuilder" accessors="true" {
      * @return qb.models.Query.QueryBuilder
      */
     public QueryBuilder function setReturnFormat( required any format, struct options = {} ) {
-        structDelete( variables.defaultOptions, "returntype" );
         if ( isClosure( arguments.format ) || isCustomFunction( arguments.format ) ) {
             variables.returnFormat = format;
         } else {
@@ -4605,7 +4605,7 @@ component displayname="QueryBuilder" accessors="true" {
         return result;
     }
 
-    private void function normalizeQueryExecuteReturnTypeOptions( required struct options ) {
+    private void function guardAgainstReturnTypeOption( required struct options ) {
         if ( !arguments.options.keyExists( "returntype" ) ) {
             return;
         }

models/Query/ReturnFormatterRegistry.cfc

@@ -64,7 +64,8 @@ component accessors="true" singleton {
         }
 
         var definition = variables.returnFormatters[ arguments.name ];
-        var formatterOptions = duplicate( definition.options );
+        var formatterOptions = {};
+        structAppend( formatterOptions, definition.options, true );
         structAppend( formatterOptions, arguments.options, true );
 
         var factory = resolveFactory( definition.factory, definition.properties );

tests/specs/Query/Abstract/QueryExecutionSpec.cfc

@@ -1393,6 +1393,36 @@ component extends="testbox.system.BaseSpec" {
                 expect( results ).toBe( "account-1" );
             } );
 
+            it( "carries return formatter settings to new queries and clones", function() {
+                var registry = new qb.models.Query.ReturnFormatterRegistry();
+                registry.registerReturnFormatter( "firstId", function( options ) {
+                    return function( q ) {
+                        return q.id[ 1 ];
+                    };
+                } );
+                var builder = getMockBox()
+                    .createMock( "qb.models.Query.QueryBuilder" )
+                    .init(
+                        grammar = getMockBox().createMock( "qb.models.Grammars.BaseGrammar" ).init(),
+                        returnFormatterRegistry = registry,
+                        validateQueryExecuteReturnType = true,
+                        collectQueryLog = false
+                    );
+
+                var newBuilder = builder.newQuery();
+                var clonedBuilder = builder.clone();
+
+                expect( newBuilder.getReturnFormatterRegistry() ).toBe( registry );
+                expect( newBuilder.getValidateQueryExecuteReturnType() ).toBeTrue();
+                expect( newBuilder.getCollectQueryLog() ).toBeFalse();
+                newBuilder.setReturnFormat( "firstId" );
+
+                expect( clonedBuilder.getReturnFormatterRegistry() ).toBe( registry );
+                expect( clonedBuilder.getValidateQueryExecuteReturnType() ).toBeTrue();
+                expect( clonedBuilder.getCollectQueryLog() ).toBeFalse();
+                clonedBuilder.setReturnFormat( "firstId" );
+            } );
+
             it( "creates a default return formatter registry when none is passed", function() {
                 var builder = getBuilder();
                 builder.setReturnFormat( "none" );
@@ -1460,6 +1490,26 @@ component extends="testbox.system.BaseSpec" {
                 expect( builder.getGrammar().$callLog().runQuery[ 1 ].options ).toBe( {} );
             } );
 
+            it( "can strip native queryExecute returntype options from default options without mutating them", function() {
+                var builder = getBuilder();
+                builder.mergeDefaultOptions( { "returntype": "array", "columnkey": "id", "columnKey": "id" } );
+                builder.setReturnFormat( "query" );
+                var expectedQuery = queryNew( "id", "integer", [ { "id": 1 } ] );
+                builder
+                    .getGrammar()
+                    .$( "runQuery" )
+                    .$results( expectedQuery );
+
+                var results = builder
+                    .select( "id" )
+                    .from( "users" )
+                    .get();
+
+                expect( results ).toBe( expectedQuery );
+                expect( builder.getGrammar().$callLog().runQuery[ 1 ].options ).toBe( {} );
+                expect( builder.getDefaultOptions() ).toBe( { "returntype": "array", "columnkey": "id", "columnKey": "id" } );
+            } );
+
             it( "can validate native queryExecute returntype options", function() {
                 var builder = getMockBox()
                     .createMock( "qb.models.Query.QueryBuilder" )