From ade2bc33cf339f17213b8bb1baf415f475a24537 Mon Sep 17 00:00:00 2001 From: Jamie Alexandre Date: Thu, 9 Apr 2015 12:30:37 -0700 Subject: [PATCH 1/4] Minimal regression test for #2470. --- python-packages/securesync/tests/crypto_tests.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/python-packages/securesync/tests/crypto_tests.py b/python-packages/securesync/tests/crypto_tests.py index 57a37f25fd..c7c5123256 100755 --- a/python-packages/securesync/tests/crypto_tests.py +++ b/python-packages/securesync/tests/crypto_tests.py @@ -253,3 +253,17 @@ def test_facility_group_save(self): group.save() assert group.zone_fallback is not None, "Centrally created FacilityGroup was not assigned a zone." + +class TestHashableFieldsAndSerialization(unittest.TestCase): + + def test_description_exclusion_regression_bug_2470(self): + + d = Device(name="Test", description="Test") + + good_serialization = d._hashable_representation() + + g = FacilityGroup() + + possibly_bad_serialization = d._hashable_representation() + + self.assertEqual(good_serialization, possibly_bad_serialization, "Instatiating a FacilityGroup changed hashable representation of Device") \ No newline at end of file From b0fc255392c959ff4a4f44d0aeba772c7b8102df Mon Sep 17 00:00:00 2001 From: Jamie Alexandre Date: Thu, 9 Apr 2015 12:37:05 -0700 Subject: [PATCH 2/4] Turn class-level attributes on SyncedModel into immutable tuples. --- python-packages/securesync/engine/models.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/python-packages/securesync/engine/models.py b/python-packages/securesync/engine/models.py index 4cbdbec82b..9994146ae1 100755 --- a/python-packages/securesync/engine/models.py +++ b/python-packages/securesync/engine/models.py @@ -190,10 +190,9 @@ class SyncedModel(ExtendedModel): objects = SyncedModelManager() all_objects = SyncedModelManager(show_deleted=True) - _unhashable_fields = ["signature", "signed_by"] # fields of this class to avoid serializing - _always_hash_fields = ["signed_version", "id"] # fields of this class to always serialize (see note above for signed_version) - _import_excluded_validation_fields = [] # fields that should not be validated upon import - + _unhashable_fields = ("signature", "signed_by") # fields of this class to avoid serializing + _always_hash_fields = ("signed_version", "id") # fields of this class to always serialize (see note above for signed_version) + _import_excluded_validation_fields = tuple() # fields that should not be validated upon import __metaclass__ = SyncedModelMetaclass From 656e3cf7a3733b957378eaf9592c09ee708ee330 Mon Sep 17 00:00:00 2001 From: Jamie Alexandre Date: Thu, 9 Apr 2015 12:37:58 -0700 Subject: [PATCH 3/4] Fixes #2470: Overwrite _unhashable_fields rather than updating in place. --- kalite/facility/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kalite/facility/models.py b/kalite/facility/models.py index 2ee9eb1cdd..a740ee3143 100755 --- a/kalite/facility/models.py +++ b/kalite/facility/models.py @@ -90,7 +90,7 @@ class FacilityGroup(DeferredCountSyncedModel): def __init__(self, *args, **kwargs): super(FacilityGroup, self).__init__(*args, **kwargs) - self._unhashable_fields.append("description") # since it's being stripped out by minversion, we can't include it in the signature + self._unhashable_fields += ("description",) # since it's being stripped out by minversion, we can't include it in the signature class Meta: app_label = "securesync" # for back-compat reasons From d1d5e02b2e387a5726a41b36e518a5e0e332c85e Mon Sep 17 00:00:00 2001 From: Jamie Alexandre Date: Thu, 9 Apr 2015 14:56:42 -0700 Subject: [PATCH 4/4] Update test for #2470 to properly catch error even on Travis. --- python-packages/securesync/tests/crypto_tests.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/python-packages/securesync/tests/crypto_tests.py b/python-packages/securesync/tests/crypto_tests.py index c7c5123256..793728528e 100755 --- a/python-packages/securesync/tests/crypto_tests.py +++ b/python-packages/securesync/tests/crypto_tests.py @@ -257,13 +257,14 @@ def test_facility_group_save(self): class TestHashableFieldsAndSerialization(unittest.TestCase): def test_description_exclusion_regression_bug_2470(self): + """FacilityGroup.__init__ used to append "description" to _unhashable_fields, which affected other classes as well. + This test ensures that the description is not being excluded from Device._hashable_representation, even after + instantiating a FacilityGroup.""" d = Device(name="Test", description="Test") - - good_serialization = d._hashable_representation() - - g = FacilityGroup() - possibly_bad_serialization = d._hashable_representation() + self.assertIn("description=Test", possibly_bad_serialization, "Hashable representation of Device did not include description") - self.assertEqual(good_serialization, possibly_bad_serialization, "Instatiating a FacilityGroup changed hashable representation of Device") \ No newline at end of file + g = FacilityGroup() + possibly_worse_serialization = d._hashable_representation() + self.assertIn("description=Test", possibly_worse_serialization, "Instantiating a FacilityGroup changed hashable representation of Device") \ No newline at end of file