(core) Be more careful with avoiding actions which don't change encoded values

Summary:
- For comparing for equality of encoding, do better at approximating what's equal in JSON.
- Fix loading of "RaisedException" values so that they can match an equivalent
  exception raised when the formula is re-evaluated.

Test Plan: Added another column to the test that verifies the Calculate action.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2682
pull/3/head
Dmitry S 4 years ago
parent 4f263fc7ec
commit b3a57e3b5c

@ -11,7 +11,6 @@ If an object cannot be encoded or decoded, an "UnmarshallableValue" is returned
of the form ['U', repr(obj)]. of the form ['U', repr(obj)].
""" """
# pylint: disable=too-many-return-statements # pylint: disable=too-many-return-statements
import exceptions
import traceback import traceback
from datetime import date, datetime from datetime import date, datetime
from math import isnan from math import isnan
@ -131,11 +130,19 @@ def strict_equal(a, b):
return False return False
def equal_encoding(a, b): def equal_encoding(a, b):
# pylint: disable=unidiomatic-typecheck # Compare NaNs as equal.
if isinstance(a, (str, unicode, bool, long, int)) or a is None: if isinstance(a, float) and isinstance(b, float):
return a == b or (isnan(a) and isnan(b))
# Compare bools as equal only to bools (these are distinguishable from numbers in JSON, and we
# take care to distinguish them in DB too).
if isinstance(a, bool) or isinstance(b, bool):
# pylint: disable=unidiomatic-typecheck
return type(a) == type(b) and a == b return type(a) == type(b) and a == b
if isinstance(a, float):
return type(a) == type(b) and (a == b or (isnan(a) and isnan(b))) # Note for simple types, encode_object is trivial, and will result in a non-type-specific
# comparison (e.g. 1 and 1.0 will compare equal, as would "a" and u"a"). This is to capture
# equivalence of values in their JSON representations.
return encode_object(a) == encode_object(b) return encode_object(a) == encode_object(b)
def encode_object(value): def encode_object(value):
@ -230,11 +237,15 @@ class RaisedException(object):
widely-used wrapper. To encode_args, it simply returns the entire encoded stored error, e.g. widely-used wrapper. To encode_args, it simply returns the entire encoded stored error, e.g.
RaisedException(ValueError("foo")) is encoded as ["E", "ValueError", "foo"]. RaisedException(ValueError("foo")) is encoded as ["E", "ValueError", "foo"].
""" """
def __init__(self, error, include_details=False): def __init__(self, error, include_details=False, encoded_error=None):
self.error = error self.error = error
self.details = traceback.format_exc() if include_details else None self.details = traceback.format_exc() if include_details else None
self._encoded_error = encoded_error or self._encode_error()
def encode_args(self): def encode_args(self):
return self._encoded_error
def _encode_error(self):
# TODO: We should probably return all args, to communicate the error details to the browser # TODO: We should probably return all args, to communicate the error details to the browser
# and to DB (for when we store formula results). There are two concerns: one is that it's # and to DB (for when we store formula results). There are two concerns: one is that it's
# potentially quite verbose; the other is that it's makes the tests more annoying (again b/c # potentially quite verbose; the other is that it's makes the tests more annoying (again b/c
@ -247,17 +258,8 @@ class RaisedException(object):
@classmethod @classmethod
def decode_args(cls, *args): def decode_args(cls, *args):
# Decoding of a RaisedException is currently only used in tests. # Decoding of a RaisedException is only enough to re-encode it.
name = args[0] return cls(None, encoded_error=list(args))
exc_type = getattr(exceptions, name)
assert isinstance(exc_type, type) and issubclass(exc_type, BaseException)
return cls(exc_type(*args[1:]))
def __eq__(self, other):
return isinstance(other, type(self)) and self.encode_args() == other.encode_args()
def __ne__(self, other):
return not self.__eq__(other)
class RecordList(list): class RecordList(list):

@ -209,13 +209,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "text", {"type": "Numeric"}], ["ModifyColumn", "Types", "text", {"type": "Numeric"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"text": [0.0, 1.0, 1509556595.0, 0.0, 1.0, None]}], {"text": [0.0, 1.0, None]}],
["UpdateRecord", "_grist_Tables_column", 21, {"type": "Numeric"}], ["UpdateRecord", "_grist_Tables_column", 21, {"type": "Numeric"}],
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"text": [False, True, 1509556595, 0, 1, ""]}], {"text": [False, True, ""]}],
["ModifyColumn", "Types", "text", {"type": "Text"}], ["ModifyColumn", "Types", "text", {"type": "Text"}],
["UpdateRecord", "_grist_Tables_column", 21, {"type": "Text"}], ["UpdateRecord", "_grist_Tables_column", 21, {"type": "Text"}],
] ]
@ -233,13 +233,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "int", {"type": "Numeric"}], ["ModifyColumn", "Types", "int", {"type": "Numeric"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"int": [0.0, 1.0, 1509556595.0, 0.0, 1.0, None]}], {"int": [0.0, 1.0, None]}],
["UpdateRecord", "_grist_Tables_column", 23, {"type": "Numeric"}], ["UpdateRecord", "_grist_Tables_column", 23, {"type": "Numeric"}],
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"int": [False, True, 1509556595, 0, 1, ""]}], {"int": [False, True, ""]}],
["ModifyColumn", "Types", "int", {"type": "Int"}], ["ModifyColumn", "Types", "int", {"type": "Int"}],
["UpdateRecord", "_grist_Tables_column", 23, {"type": "Int"}], ["UpdateRecord", "_grist_Tables_column", 23, {"type": "Int"}],
] ]
@ -250,13 +250,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "bool", {"type": "Numeric"}], ["ModifyColumn", "Types", "bool", {"type": "Numeric"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 17, 18, 19],
{"bool": [0.0, 1.0, 1509556595.0, 0.0, 1.0, None]}], {"bool": [0.0, 1.0, 0.0, 1.0, None]}],
["UpdateRecord", "_grist_Tables_column", 24, {"type": "Numeric"}], ["UpdateRecord", "_grist_Tables_column", 24, {"type": "Numeric"}],
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 17, 18, 19],
{"bool": [False, True, 1509556595, False, True, ""]}], {"bool": [False, True, False, True, ""]}],
["ModifyColumn", "Types", "bool", {"type": "Bool"}], ["ModifyColumn", "Types", "bool", {"type": "Bool"}],
["UpdateRecord", "_grist_Tables_column", 24, {"type": "Bool"}], ["UpdateRecord", "_grist_Tables_column", 24, {"type": "Bool"}],
] ]
@ -324,14 +324,14 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "numeric", {"type": "Int"}], ["ModifyColumn", "Types", "numeric", {"type": "Int"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 16, 19],
{"numeric": [0, 1, 1509556595, 8, 0, 1, None]}], {"numeric": [0, 1, 8, None]}],
["UpdateRecord", "_grist_Tables_column", 22, {"type": "Int"}], ["UpdateRecord", "_grist_Tables_column", 22, {"type": "Int"}],
["UpdateRecord", "Formulas", 1, {"division": 0}], ["UpdateRecord", "Formulas", 1, {"division": 0}],
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 16, 19],
{"numeric": [False, True, 1509556595.0, 8.153, 0.0, 1.0, ""]}], {"numeric": [False, True, 8.153, ""]}],
["ModifyColumn", "Types", "numeric", {"type": "Numeric"}], ["ModifyColumn", "Types", "numeric", {"type": "Numeric"}],
["UpdateRecord", "_grist_Tables_column", 22, {"type": "Numeric"}], ["UpdateRecord", "_grist_Tables_column", 22, {"type": "Numeric"}],
["UpdateRecord", "Formulas", 1, {"division": 0.5}], ["UpdateRecord", "Formulas", 1, {"division": 0.5}],
@ -367,13 +367,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "date", {"type": "Int"}], ["ModifyColumn", "Types", "date", {"type": "Int"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 16, 19],
{"date": [0, 1, 1509556595, 8, 0, 1, None]}], {"date": [0, 1, 8, None]}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Int"}] ["UpdateRecord", "_grist_Tables_column", 25, {"type": "Int"}]
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 16, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 16, 19],
{"date": [False, True, 1509556595.0, 8.153, 0.0, 1.0, ""]}], {"date": [False, True, 8.153, ""]}],
["ModifyColumn", "Types", "date", {"type": "Date"}], ["ModifyColumn", "Types", "date", {"type": "Date"}],
["UpdateRecord", "_grist_Tables_column", 25, {"type": "Date"}] ["UpdateRecord", "_grist_Tables_column", 25, {"type": "Date"}]
] ]
@ -509,13 +509,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "text", {"type": "Date"}], ["ModifyColumn", "Types", "text", {"type": "Date"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"text": [0.0, 1.0, 1509556595.0, 0.0, 1.0, None]}], {"text": [0.0, 1.0, None]}],
["UpdateRecord", "_grist_Tables_column", 21, {"type": "Date"}], ["UpdateRecord", "_grist_Tables_column", 21, {"type": "Date"}],
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"text": [False, True, 1509556595, 0, 1, ""]}], {"text": [False, True, ""]}],
["ModifyColumn", "Types", "text", {"type": "Text"}], ["ModifyColumn", "Types", "text", {"type": "Text"}],
["UpdateRecord", "_grist_Tables_column", 21, {"type": "Text"}], ["UpdateRecord", "_grist_Tables_column", 21, {"type": "Text"}],
] ]
@ -545,13 +545,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "int", {"type": "Date"}], ["ModifyColumn", "Types", "int", {"type": "Date"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"int": [0.0, 1.0, 1509556595.0, 0.0, 1.0, None]}], {"int": [0.0, 1.0, None]}],
["UpdateRecord", "_grist_Tables_column", 23, {"type": "Date"}], ["UpdateRecord", "_grist_Tables_column", 23, {"type": "Date"}],
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 19],
{"int": [False, True, 1509556595, 0, 1, ""]}], {"int": [False, True, ""]}],
["ModifyColumn", "Types", "int", {"type": "Int"}], ["ModifyColumn", "Types", "int", {"type": "Int"}],
["UpdateRecord", "_grist_Tables_column", 23, {"type": "Int"}], ["UpdateRecord", "_grist_Tables_column", 23, {"type": "Int"}],
] ]
@ -562,13 +562,13 @@ class TestTypes(test_engine.EngineTestCase):
self.assertPartialOutActions(out_actions, { self.assertPartialOutActions(out_actions, {
"stored": [ "stored": [
["ModifyColumn", "Types", "bool", {"type": "Date"}], ["ModifyColumn", "Types", "bool", {"type": "Date"}],
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 17, 18, 19],
{"bool": [0.0, 1.0, 1509556595.0, 0.0, 1.0, None]}], {"bool": [0.0, 1.0, 0.0, 1.0, None]}],
["UpdateRecord", "_grist_Tables_column", 24, {"type": "Date"}] ["UpdateRecord", "_grist_Tables_column", 24, {"type": "Date"}]
], ],
"undo": [ "undo": [
["BulkUpdateRecord", "Types", [13, 14, 15, 17, 18, 19], ["BulkUpdateRecord", "Types", [13, 14, 17, 18, 19],
{"bool": [False, True, 1509556595, False, True, ""]}], {"bool": [False, True, False, True, ""]}],
["ModifyColumn", "Types", "bool", {"type": "Bool"}], ["ModifyColumn", "Types", "bool", {"type": "Bool"}],
["UpdateRecord", "_grist_Tables_column", 24, {"type": "Bool"}] ["UpdateRecord", "_grist_Tables_column", 24, {"type": "Bool"}]
] ]

@ -262,8 +262,8 @@
["ModifyColumn", "Schools", "numStudents", {"type": "Numeric"}], ["ModifyColumn", "Schools", "numStudents", {"type": "Numeric"}],
// Record 13 is not updated since it can't be properly converted. // Record 13 is not updated since it can't be properly converted.
["BulkUpdateRecord", "Schools", [1, 2, 5, 6, 7, 8, 9, 10, 11, 12, 14, 15], // Other records aren't updated because the converted value is equivalent for JSON and DB.
{"numStudents": [0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1e+27, 1000000.0]}], ["UpdateRecord", "Schools", 14, {"numStudents": 1e+27}],
["UpdateRecord", "_grist_Tables_column", 30, {"type": "Numeric"}], ["UpdateRecord", "_grist_Tables_column", 30, {"type": "Numeric"}],
["AddRecord", "Schools", 16, {"numStudents": "@+Infinity"}], ["AddRecord", "Schools", 16, {"numStudents": "@+Infinity"}],
["BulkUpdateRecord", "Schools", [14, 15, 16], {"name": ["14$\\$$'\\'", "15$\\$$'\\'", "16$\\$$'\\'"]}] ["BulkUpdateRecord", "Schools", [14, 15, 16], {"name": ["14$\\$$'\\'", "15$\\$$'\\'", "16$\\$$'\\'"]}]
@ -1445,7 +1445,6 @@
"addr = $university.address\naddr.stateName if addr.country == 'US' else addr.region"] "addr = $university.address\naddr.stateName if addr.country == 'US' else addr.region"]
}], }],
["ModifyColumn", "Address", "stateName", {"type": "Numeric"}], ["ModifyColumn", "Address", "stateName", {"type": "Numeric"}],
["UpdateRecord", "Address", 2, {"stateName": 73.0}],
["UpdateRecord", "_grist_Tables_column", 27, {"type": "Numeric"}] ["UpdateRecord", "_grist_Tables_column", 27, {"type": "Numeric"}]
], ],
"undo": [ "undo": [
@ -1458,7 +1457,6 @@
"formula": ["", "formula": ["",
"addr = $university.address\naddr.state if addr.country == 'US' else addr.region"] "addr = $university.address\naddr.state if addr.country == 'US' else addr.region"]
}], }],
["UpdateRecord", "Address", 2, {"stateName": 73}],
["ModifyColumn", "Address", "stateName", {"type": "Int"}], ["ModifyColumn", "Address", "stateName", {"type": "Int"}],
["UpdateRecord", "_grist_Tables_column", 27, {"type": "Int"}] ["UpdateRecord", "_grist_Tables_column", 27, {"type": "Int"}]
] ]

Loading…
Cancel
Save