Refactor queryToCsv() parallelization to fail gracefully when paralle…

…l loops not supported
cfsimplicity · Nov 1, 2023 · 6e279e1 · 6e279e1
1 parent f98e62a
commit 6e279e1
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 13 deletions.
diff --git a/Spreadsheet.cfc b/Spreadsheet.cfc
@@ -166,6 +166,10 @@ component accessors="true"{
 		return getFormatHelper().getCachedCellStyles();
 	}
 
+	public boolean function engineSupportsParallelLoopProcessing(){
+		return ( !this.getIsACF() || ( this.getIsACF() && ( server.coldfusion.productVersion.ListFirst() >= 2021 ) ) );
+	}
+
 	/* MAIN PUBLIC API */
 
 	public Spreadsheet function addAutofilter( required workbook, string cellRange="", numeric row=1 ){

diff --git a/helpers/csv.cfc b/helpers/csv.cfc
@@ -25,23 +25,26 @@ component extends="base"{
 	}
 
 	/* row order is not guaranteed if using more than one thread */
-	array function queryToArrayForCsv( required query query, required boolean includeHeaderRow, numeric threads=1){
+	array function queryToArrayForCsv( required query query, required boolean includeHeaderRow, numeric threads=1 ){
 		var result = [];
 		var columns = getQueryHelper()._QueryColumnArray( arguments.query );
 		if( arguments.includeHeaderRow )
 			result.Append( columns );
-		queryEach(arguments.query, function(row){
-			var rowValues = [];
-			arrayEach(columns, function(column){
-				var cellValue = row[ column ];
-				if( getDateHelper().isDateObject( cellValue ) )
-					cellValue = DateTimeFormat( cellValue, library().getDateFormats().DATETIME );
-				if( IsValid( "integer", cellValue ) )
-					cellValue = JavaCast( "string", cellValue );// prevent CSV writer converting 1 to 1.0
-				rowValues.Append( cellValue );
-			});
-			result.Append( rowValues );
-		}, arguments.threads gt 1, arguments.threads)
+		if( ( arguments.threads > 1 ) && !library().engineSupportsParallelLoopProcessing() )
+			getExceptionHelper().throwParallelOptionNotSupportedException();
+		if( arguments.threads > 1 ){
+			arguments.query.Each(
+				function( row ){
+					result.Append( getQueryRowValues( row, columns ) );
+				}
+				,true
+				,arguments.threads
+			);
+			return result;			
+		}
+		for( var row IN arguments.query ){
+			result.Append( getQueryRowValues( row, columns ) );
+		}
 		return result;
 	}
 
@@ -95,4 +98,17 @@ component extends="base"{
 		return result;
 	}
 
+	private array function getQueryRowValues( required row, required array columns ){
+		var rowValues = [];
+		for( var column IN arguments.columns ){
+			var cellValue = arguments.row[ column ];
+			if( getDateHelper().isDateObject( cellValue ) )
+				cellValue = DateTimeFormat( cellValue, library().getDateFormats().DATETIME );
+			if( IsValid( "integer", cellValue ) )
+				cellValue = JavaCast( "string", cellValue );// prevent CSV writer converting 1 to 1.0
+			rowValues.Append( cellValue );
+		};
+		return rowValues;
+	}
+
 }
diff --git a/helpers/exception.cfc b/helpers/exception.cfc
@@ -30,4 +30,10 @@ component extends="base"{
 		Throw( type=library().getExceptionType() & ".nonExistentRow", message="Non-existent row", detail="Row #arguments.rowNumber# doesn't exist. You may need to create the row first by adding data to it." );
 	}
 
+	void function throwParallelOptionNotSupportedException(){
+		Throw( type=library().getExceptionType() & ".parallelOptionNotSupported", message="Parallel threads option not supported", detail="Your ColdFusion engine does not support parallel processing of loops via threads" );
+	}
+
+
+
 }
diff --git a/test/specs/queryToCsv.cfm b/test/specs/queryToCsv.cfm
@@ -60,5 +60,36 @@ describe( "queryToCsv", function(){
 		expect( s.queryToCsv( data ) ).toBe( expected );
 	});
 
+	it(
+		title="can process rows in parallel if the engine supports it"
+		,body=function(){
+			//can't test if using threads, just that there are no errors
+			var data = QueryNew( "column1,column2", "VarChar,VarChar", [ [ "a", "a" ], [ "a", "a" ] ] );
+			var expected = 'a,a#crlf#a,a';//same values because order is not guaranteed
+			expect( s.queryToCsv( query=data, threads=2 ) ).toBe( expected );
+		}
+		,skip=function(){
+			//20231031: ACF 2021 and 2023 won't run the whole suite if this test is included: testbox errors thrown
+			//running just the queryToCsv tests works fine though. Lucee is fine with the whole suite.
+			return s.getIsACF();
+		}
+	);
+
+	describe( "queryToCsv throws an exception if", function(){
+
+		it(
+			title="parallel threads are specified and the engine does not support it"
+			,body=function(){
+				expect( function(){
+					s.queryToCsv( query=data, threads=2 );
+				}).toThrow( type="cfsimplicity.spreadsheet.parallelOptionNotSupported" );
+			}
+			,skip=function(){
+				return s.engineSupportsParallelLoopProcessing();
+			}
+		);
+
+	});
+
 });
 </cfscript>