Merge pull request #1937 from wheels-dev/peter/throw-on-column-not-found-main

Add throwOnColumnNotFound config setting

Author: bpamiri

core/src/wheels/events/onapplicationstart.cfc

@@ -321,6 +321,7 @@ component {
 		application.$wheels.deletePluginDirectories = true;
 		application.$wheels.loadIncompatiblePlugins = true;
 		application.$wheels.automaticValidations = true;
+		application.$wheels.throwOnColumnNotFound = true;
 		application.$wheels.setUpdatedAtOnCreate = true;
 		application.$wheels.useExpandedColumnAliases = false;
 		application.$wheels.lowerCaseTableNames = false;

core/src/wheels/model/sql.cfc

@@ -274,12 +274,20 @@ component {
 								}
 							}
 						}
-						if (application.wheels.showErrorInformation && !Len(local.toAdd)) {
-							Throw(
-								type = "Wheels.ColumnNotFound",
-								message = "Wheels looked for the column mapped to the `#local.property#` property but couldn't find it in the database table.",
-								extendedInfo = "Verify the `order` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct."
-							);
+						if (!Len(local.toAdd)) {
+							if (application.wheels.throwOnColumnNotFound) {
+								Throw(
+									type = "Wheels.ColumnNotFound",
+									message = "Wheels looked for the column mapped to the `#local.property#` property but couldn't find it in the database table.",
+									extendedInfo = "Verify the `order` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct."
+								);
+							} else {
+								writeLog(
+									text = "ColumnNotFound: column mapped to `#local.property#` not found in database table (order clause). Set throwOnColumnNotFound=true to throw an exception.",
+									type = "warning",
+									file = "wheels_columnnotfound"
+								);
+							}
 						}
 					}
 				}
@@ -527,12 +535,20 @@ component {
 					Added an exception in case the column specified in the select or group argument does not exist in the database.
 					This will only be in case when not using "table.column" or "column AS something" since in those cases Wheels passes through the select clause unchanged.
 				*/
-				if (application.wheels.showErrorInformation && !Len(local.toAppend) && arguments.clause == "select" && ListFindNoCase(local.addedPropertiesByModel[local.associationKey], local.iItem) EQ 0) {
-					Throw(
-						type = "Wheels.ColumnNotFound",
-						message = "Wheels looked for the column mapped to the `#local.iItem#` property but couldn't find it in the database table.",
-						extendedInfo = "Verify the `#arguments.clause#` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct."
-					);
+				if (!Len(local.toAppend) && arguments.clause == "select" && ListFindNoCase(local.addedPropertiesByModel[local.associationKey], local.iItem) EQ 0) {
+					if (application.wheels.throwOnColumnNotFound) {
+						Throw(
+							type = "Wheels.ColumnNotFound",
+							message = "Wheels looked for the column mapped to the `#local.iItem#` property but couldn't find it in the database table.",
+							extendedInfo = "Verify the `#arguments.clause#` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct."
+						);
+					} else {
+						writeLog(
+							text = "ColumnNotFound: column mapped to `#local.iItem#` not found in database table (#arguments.clause# clause). Set throwOnColumnNotFound=true to throw an exception.",
+							type = "warning",
+							file = "wheels_columnnotfound"
+						);
+					}
 				}
 
 				if (Len(local.toAppend)) {
@@ -762,12 +778,24 @@ component {
 							}
 						}
 					}
-					if (application.wheels.showErrorInformation && !StructKeyExists(local.param, "column")) {
-						Throw(
-							type = "Wheels.ColumnNotFound",
-							message = "Wheels looked for the column mapped to the `#local.param.property#` property but couldn't find it in the database table.",
-							extendedInfo = "Verify the `where` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct."
-						);
+					if (!StructKeyExists(local.param, "column")) {
+						if (application.wheels.throwOnColumnNotFound) {
+							Throw(
+								type = "Wheels.ColumnNotFound",
+								message = "Wheels looked for the column mapped to the `#local.param.property#` property but couldn't find it in the database table.",
+								extendedInfo = "Verify the `where` argument and/or your property to column mappings done with the `property` method inside the model's `config` method to make sure everything is correct."
+							);
+						} else {
+							writeLog(
+								text = "ColumnNotFound: column mapped to `#local.param.property#` not found in database table (where clause). Set throwOnColumnNotFound=true to throw an exception.",
+								type = "warning",
+								file = "wheels_columnnotfound"
+							);
+							// Undo the ? replacement so where/params arrays stay in sync.
+							// The raw column name passes through to the database as-is.
+							local.where = Replace(local.where, Replace(local.element, local.elementDataPart, "?", "one"), local.element);
+							continue;
+						}
 					}
 					local.temp = ReFind(
 						"^[a-zA-Z0-9-_\.]* ?#variables.wheels.class.RESQLOperators#",

core/src/wheels/tests_testbox/specs/model/raisedErrorsSpec.cfc

@@ -45,6 +45,18 @@ component extends="wheels.Testbox" {
 					g.model("user").findAll(select="id,email,firstname,lastname,createdat,foo")
 				}).toThrow("Wheels.ColumnNotFound")
 			})
+
+			it("skips invalid select column when throwOnColumnNotFound is false", () => {
+				application.wheels.throwOnColumnNotFound = false;
+				try {
+					result = g.model("user").findAll(select="id,firstname,nonexistentcolumn");
+					expect(result.recordcount).toBeGTE(0);
+					expect(result.columnList).toInclude("id");
+					expect(result.columnList).toInclude("firstname");
+				} finally {
+					application.wheels.throwOnColumnNotFound = true;
+				}
+			})
 		})
 	}
-}
+}

core/src/wheels/tests_testbox/specs/model/sqlSpec.cfc

@@ -23,7 +23,7 @@ component extends="wheels.Testbox" {
 					expect(result).toHaveLength(3)
 					expect(result[3].type).toBe("cf_sql_integer")
 					expect(Right(result[2], Len(i))).toBe(i)
-					
+
 					result = g.model("author").$whereClause(where = "id #i#999")
 
 					expect(result[2]).toHaveLength(19+len(i))
@@ -43,7 +43,7 @@ component extends="wheels.Testbox" {
 				expect(Right(result[2], 6)).toBe("NOT IN")
 
 				result = g.model("author").$whereClause(where = "lastName LIKE 'Djurner'")
-				
+
 				expect(Right(result[2], 4)).toBe("LIKE")
 
 				result = g.model("author").$whereClause(where = "lastName NOT LIKE 'Djurner'")
@@ -60,13 +60,13 @@ component extends="wheels.Testbox" {
 				expect(datatypes).toHaveKey(result[4].datatype)
 
 				result = g.model("post").$whereClause(where = "averagerating NOT IN(3.6,3.2)")
-				
+
 				expect(arraylen(result)).toBeGTE(4)
 				expect(result[4]).toBeStruct()
 				expect(datatypes).toHaveKey(result[4].datatype)
 
 				result = g.model("post").$whereClause(where = "averagerating = 3.6")
-				
+
 				expect(arraylen(result)).toBeGTE(4)
 				expect(result[4]).toBeStruct()
 				expect(datatypes).toHaveKey(result[4].datatype)
@@ -99,7 +99,7 @@ component extends="wheels.Testbox" {
 					g.model("user").findall(where="username = '#badparams.username#' AND password = '#badparams.password#'", parameterize=2)
 				}).toThrow("Wheels.ParameterMismatch")
 			})
-			
+
 			it("protects against SQL Injection with Parameterize and Pagination", () => {
 				badparams = {username = "tonyp", password = "tonyp123' OR password!='tonyp123"}
 
@@ -127,6 +127,18 @@ component extends="wheels.Testbox" {
 				}).toThrow("Wheels.ColumnNotFound");
 
 			});
+
+			it( "skips invalid select column in CONCAT when throwOnColumnNotFound is false", function(){
+				application.wheels.throwOnColumnNotFound = false;
+				try {
+					actual = g.model("user").findAll(where = "username='tonyp'", select = "id,username,nonexistentcolumn");
+					expect( actual.recordcount ).toBeGTE(1);
+					expect( actual.columnList ).toInclude("id");
+					expect( actual.columnList ).toInclude("username");
+				} finally {
+					application.wheels.throwOnColumnNotFound = true;
+				}
+			});
 		})
 	}
-}
+}