Revise error handling for invalid formulas

sandbox/grist/acl.py

@@ -145,7 +145,7 @@ def perform_acl_rule_renames(useractions, col_renames_dict):
 
     if not rule_rec.aclFormula:
       continue
-    formula = rule_rec.aclFormula
+    acl_formula = rule_rec.aclFormula
 
     def renamer(subject):
       if subject.type == 'recCol':
@@ -157,11 +157,15 @@ def renamer(subject):
       col_id = subject.name
       return col_renames_dict.get((table_id, col_id))
 
-    new_acl_formula = predicate_formula.process_renames(formula, _ACLEntityCollector(), renamer)
-    new_rule_record = {"aclFormula": new_acl_formula}
-    # No need to check for syntax errors. See dropdown_condition.py for more info.
-    new_rule_record["aclFormulaParsed"] = parse_predicate_formula_json(new_acl_formula)
-    rule_updates.append((rule_rec, new_rule_record))
+    new_acl_formula = predicate_formula.process_renames(acl_formula, _ACLEntityCollector(), renamer)
+    # No need to check for syntax errors, but this "if" statement must be present.
+    # See perform_dropdown_condition_renames for more info.
+    if new_acl_formula != acl_formula:
+      new_rule_record = {
+        "aclFormula": new_acl_formula,
+        "aclFormulaParsed": parse_predicate_formula_json(new_acl_formula)
+      }
+      rule_updates.append((rule_rec, new_rule_record))
 
   useractions.doBulkUpdateFromPairs('_grist_ACLResources', resource_updates)
   useractions.doBulkUpdateFromPairs('_grist_ACLRules', rule_updates)

sandbox/grist/dropdown_condition.py

@@ -58,12 +58,18 @@ def renamer(subject):
 
     new_dc_formula = predicate_formula.process_renames(dc_formula, _DCEntityCollector(), renamer)
 
-    widget_options["dropdownCondition"]["text"] = new_dc_formula
+    # The data engine stops processing remaining formulas when it hits an exception. Parsing a
+    # formula could potentially raise SyntaxErrors, so we must be careful not to invoke it on a
+    # possibly syntactically wrong formula.
     # Note that new_dc_formula was obtained from process_renames, where syntactically wrong formulas
-    # are left untouched. It is anticipated that renaming-induced changes will not introduce syntax
-    # errors, so we may assume that no syntax errors may occur here.
-    widget_options["dropdownCondition"]["parsed"] = parse_predicate_formula_json(new_dc_formula)
-    updates.append((col, {"widgetOptions": json.dumps(widget_options)}))
+    # are left untouched. It is anticipated that rename-induced changes will not introduce new
+    # SyntaxErrors, so if the formula text is updated, the new version must be valid, hence safe
+    # to parse without error handling.
+    # This also serves as an optimization to avoid unnecessary parsing operations.
+    if new_dc_formula != dc_formula:
+      widget_options["dropdownCondition"]["text"] = new_dc_formula
+      widget_options["dropdownCondition"]["parsed"] = parse_predicate_formula_json(new_dc_formula)
+      updates.append((col, {"widgetOptions": json.dumps(widget_options)}))
 
   # Update the dropdown condition in the database.
   useractions.doBulkUpdateFromPairs('_grist_Tables_column', updates)

sandbox/grist/test_predicate_formula.py

@@ -139,14 +139,14 @@ def test_unsupported(self):
     self.assertRaises(SyntaxError, parse_predicate_formula, "def foo(): pass")
 
     # Unsupported node type
-    self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "max(rec)")
-    self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "user.id in {1, 2, 3}")
-    self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "1 if user.IsAnon else 2")
+    self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "max(rec)")
+    self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "user.id in {1, 2, 3}")
+    self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "1 if user.IsAnon else 2")
 
     # Unsupported operation
-    self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "1 | 2")
-    self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "1 << 2")
-    self.assertRaisesRegex(ValueError, r'Unsupported syntax', parse_predicate_formula, "~test")
+    self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "1 | 2")
+    self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "1 << 2")
+    self.assertRaisesRegex(SyntaxError, r'Unsupported syntax', parse_predicate_formula, "~test")
 
     # Syntax error
     self.assertRaises(SyntaxError, parse_predicate_formula, "[(]")