patch for generateFunction

[senordhuff]

We had to develop a patch for FormulaBuilder.generateFunction. We have some custom Formulas where the formula will call a custom registred function and not use any formulaVars. We found that sometimes old formulaVars were being left in the userFormula object. And, sometimes those formulaVars pointed to null values. Just one null value would cause the formula to incorrectly resolve to ".".

So, we took a look at the code for generateFunction and made the following patch. This is the relevant piece:

"if (isNaN(value) ) " +
//4/25/13..do not cause formula to fail if there is a null formulaVar
//"if (isNaN(value) || nullVars.length > 0) " +


That allows the function to still fire even if there is a null variable. The function is wrapped in a try/catch anyway in case there was a null variable that impacted the function, right? Is this a legit patch you'd consider applying on your side for future releases?

		generateFunction : function (userFormula, fields, component, allowNonNumeric, catchErrors) {


		    // Default to using a try...catch block to catch errors
		    // Note there are 2 cases we need to catch
		    // - Syntax error in the function. We have to catch this here as the "new Function()"
		    //   call will fail
		    // - Logic error in the function. We catch that within the function itself
		    if (catchErrors == null) catchErrors = true;

		    var output = isc.SB.create(),
		        formula = userFormula.text,
		        fieldIdProperty = this.getFieldIdProperty(component),
		        fieldDetails = this.getFieldDetailsFromValue(formula, userFormula.formulaVars, fields, component),
		        usedFields = fieldDetails.usedFields,
		        missingFields = fieldDetails.missingFields
		    ;

		    usedFields = usedFields.sortByProperties([ "mappingKey" ], [false],
		        [function (item, propertyName, context) {
		            var result = item[propertyName];

		            if (result.length == 1) result = '  ' + result;
		            else if (result.length == 2) result = ' ' + result;
		            return result
		        }]
		    );

		    if (missingFields.length == 0) {
		        if (usedFields.length > 0) {

		            output.append("var nullVars = [];\n");

		            // script local vars for record-values
		            for (var i = 0; i < usedFields.length; i++) {
		                var item = usedFields.get(i);
		                // The array of field objects available here should have the 'mappingKey'
		                // and fieldIdProperty set.

		                var fieldName = item[fieldIdProperty],
		                    pathCode = "isc.DataSource.getPathValue(record,'" + fieldName + "', field)"
		                ;

		                // Code to extract the appropriate value from the record based on
		                // fieldName.
		                // Note that if possible we use the DBC._getFieldValue() method -- this
		                // handles dataPaths, and "getAtomicType()" logic if present.
		                // If we don't have a component, back off to getPathValue() which
		                // will still navigate nested objects using dataPath

		                output.append("var ");
		                output.append(
		                    "field=component==null?null:component.getField('",fieldName,"'),",
		                    item.mappingKey,
		                    // use getPathValue so we can handle being passed a dataPath to navigate a
		                    // nested structure. Used by the RulesEngine / populate rule code-path.
		                    //"=field==null?isc.DataSource.getPathValue(record,'", fieldName, "')",
		                    //":isc.Canvas._getFieldValue(null, field, record, component);\n"
		                    //"=isc.DataSource.getPathValue(record,'", fieldName, "', field)\n;"

		                    item.userFormula ?
		                        "=component?component.getFormulaFieldValue(field, record):" + pathCode
		                        : item.userSummary ?
		                            "=component?component.getSummaryFieldValue(field, record):" + pathCode
		                        : "=" + pathCode,
		                    "\n;"
		                );

		                output.append("if (", item.mappingKey, " == null || (component && ",
		                    item.mappingKey, " == component.badFormulaResultValue) || (!component && ",
		                    item.mappingKey, " == '.')) nullVars.add('", item.mappingKey, "');");

		                if (userFormula.allowEscapedKeys) {
		                    formula = formula.replaceAll("#" + item.mappingKey, item.mappingKey);
		                    formula = formula.replaceAll("#{" + item.mappingKey + "}", item.mappingKey);
		                }
		            }
		            output.append("\n");
		        }

		        // script local vars for MathFunction-pointers
		        var functions = isc.MathFunction.getRegisteredFunctions();
		        if (functions && functions.length > 0) {
		            output.append("var functions=isc.MathFunction.getRegisteredFunctionIndex(),\n");
		            for (var i = 0; i < functions.length; i++) {
		                var item = functions.get(i);
		                output.append("        ");
		                output.append(item.name, "=", "functions.", item.name, ".jsFunction");
		                output.append(i == functions.length - 1 ? ";" : ",", "\n");
		            }
		            output.append("\n");
		        }

		        if (catchErrors) {
		            output.append("try{\n");
		        }

		        // If NaN, use badFormulaResultValue
		        output.append("var value=" , formula , ";");

		        if (catchErrors) {
		            var errorMessage = "Attempt to evaluate formulaFunction " + formula +
		                " failed. Error message:";

		            output.append("\n} catch (e) { (component||isc).logWarn(",
		                                errorMessage.asSource(true)," + e.message); }\n");

		        }

		        output.append(
		            (allowNonNumeric ? null :
		                "if (isNaN(value) ) " +
		                //4/25/13..do not cause formula to fail if there is a null formulaVar
		            	//"if (isNaN(value) || nullVars.length > 0) " +
		                "return (component && component.badFormulaResultValue) || '.'; "),
		            "return value;");
		    } else {
		        this.logWarn("Formula failed due to missing fields: " + missingFields.join(", ") + ".");
		        var result = (component && component.badFormulaResultValue) || ".";
		        if (result) result = "'" + result + "'";
		        output.append("return ", result, ";");
		    }

		    // return the wrapped function
		    var content = output.toString();

			isc.Log.logInfo("inside generateFunction and content=" + content);
		    //this.logWarn("content\n:" + content);
		    var func;
		    if (catchErrors) {
		        try {
		            func = new Function("record,component", content);
		        } catch (e) {
		            this.logWarn("Error attempting to convert formula text '" + formula +
		                "' to a function:" + e.message);
		            func = new Function("record,component", "return null;");
		        }
		    } else {
		        func = new Function("record,component", content);
		    }

		    return func;

		}

[Isomorphic]

We're not entirely comfortable a priori with removing the safety check.

However, if you could provide a reproduce case, including whatever steps are needed to see the issue, we might be convinced (or see another way to accomplish it). It's also possible the "old vars" problem has been resolved (especially in SC 9.0d/SGWT 4.0d), as we've made some recent improvements in that branch.