diff --git a/sandbox/grist/acl.py b/sandbox/grist/acl.py index c69ba99b4e..339fd17a60 100644 --- a/sandbox/grist/acl.py +++ b/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) diff --git a/sandbox/grist/dropdown_condition.py b/sandbox/grist/dropdown_condition.py index 983163dc41..a031d4b4f5 100644 --- a/sandbox/grist/dropdown_condition.py +++ b/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) diff --git a/sandbox/grist/test_predicate_formula.py b/sandbox/grist/test_predicate_formula.py index 34914c2ee0..4feb0ce155 100644 --- a/sandbox/grist/test_predicate_formula.py +++ b/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, "[(]")