Merged python-657 branch

cassandra/cqlengine/columns.py

@@ -47,7 +47,19 @@ class BaseValueManager(object):
         :rtype: boolean
 
         """
-        return self.value != self.previous_value
+        if self.explicit:
+            return self.value != self.previous_value
+
+        if isinstance(self.column, BaseContainerColumn):
+            default_value = self.column.get_default()
+            if self.column._val_is_null(default_value):
+                return not self.column._val_is_null(self.value) and self.value != self.previous_value
+            elif self.previous_value is None:
+                return self.value != default_value
+
+            return self.value != self.previous_value
+
+        return False
 
     def reset_previous_value(self):
         self.previous_value = deepcopy(self.value)
@@ -57,6 +69,7 @@ class BaseValueManager(object):
 
     def setval(self, val):
         self.value = val
+        self.explicit = True
 
     def delval(self):
         self.value = None

cassandra/cqlengine/models.py

@@ -407,8 +407,6 @@ class BaseModel(object):
                 value = column.to_python(value)
             value_mngr = column.value_manager(self, column, value)
             value_mngr.explicit = name in values
-            if not value_mngr.explicit and column.has_default:
-                value_mngr.previous_value = value
             self._values[name] = value_mngr
 
     def __repr__(self):
@@ -486,11 +484,12 @@ class BaseModel(object):
             klass = cls
 
         instance = klass(**values)
-        instance._set_persisted()
+        instance._set_persisted(force=True)
         return instance
 
-    def _set_persisted(self):
+    def _set_persisted(self, force=False):
-        for v in self._values.values():
+        # ensure we don't modify to any values not affected by the last save/update
+        for v in [v for v in self._values.values() if v.changed or force]:
             v.reset_previous_value()
             v.explicit = False
         self._is_persisted = True
@@ -591,6 +590,10 @@ class BaseModel(object):
 
         return cls._table_name
 
+    def _set_column_value(self, name, value):
+        """Function to change a column value without changing the value manager states"""
+        self._values[name].value = value  # internal assignement, skip the main setter
+
     def validate(self):
         """
         Cleans and validates the field values
@@ -600,7 +603,7 @@ class BaseModel(object):
             if v is None and not self._values[name].explicit and col.has_default:
                 v = col.get_default()
             val = col.validate(v)
-            setattr(self, name, val)
+            self._set_column_value(name, val)
 
     # Let an instance be used like a dict of its columns keys/values
     def __iter__(self):

cassandra/cqlengine/query.py

@@ -1457,6 +1457,9 @@ class DMLQuery(object):
                     if self.instance._values[name].changed:
                         nulled_fields.add(col.db_field_name)
                     continue
+                if col.has_default and not self.instance._values[name].changed:
+                    # Ensure default columns included in a save() are marked as explicit, to get them *persisted* properly
+                    self.instance._values[name].explicit = True
                 insert.add_assignment(col, getattr(self.instance, name, None))
 
         # skip query execution if it's empty

tests/integration/cqlengine/model/test_model_io.py

@@ -30,7 +30,6 @@ from cassandra.cqlengine.management import drop_table
 from cassandra.cqlengine.models import Model
 from cassandra.query import SimpleStatement
 from cassandra.util import Date, Time
-from cassandra.cqltypes import Int32Type
 from cassandra.cqlengine.statements import SelectStatement, DeleteStatement, WhereClause
 from cassandra.cqlengine.operators import EqualsOperator
 
@@ -483,37 +482,6 @@ class TestUpdating(BaseCassEngTestCase):
         self.assertTrue(self.instance._values['count'].previous_value is None)
         self.assertTrue(self.instance.count is None)
 
-    def test_value_override_with_default(self):
-        """
-        Updating a row with a new Model instance shouldn't set columns to defaults
-
-        @since 3.9
-        @jira_ticket PYTHON-657
-        @expected_result column value should not change
-
-        @test_category object_mapper
-        """
-        class ModelWithDefault(Model):
-            id = columns.Integer(primary_key=True)
-            mf = columns.Map(columns.Integer, columns.Integer)
-            dummy = columns.Integer(default=42)
-        sync_table(ModelWithDefault)
-
-        initial = ModelWithDefault(id=1, mf={0: 0}, dummy=0)
-        initial.save()
-
-        session = cassandra.cluster.Cluster().connect()
-        session.execute('USE ' + DEFAULT_KEYSPACE)
-        self.assertEqual(
-            list(session.execute('SELECT * from model_with_default'))[0].dummy, 0
-        )
-
-        second = ModelWithDefault(id=1)
-        second.update(mf={0: 1})
-        self.assertEqual(
-            list(session.execute('SELECT * from model_with_default'))[0].dummy, 0
-        )
-
     def test_previous_value_tracking_on_instantiation_with_default(self):
 
         class TestDefaultValueTracking(Model):
@@ -546,16 +514,12 @@ class TestUpdating(BaseCassEngTestCase):
         # yet.
         self.assertTrue(instance._values['id'].previous_value is None)
         self.assertTrue(instance._values['int1'].previous_value is None)
+        self.assertTrue(instance._values['int2'].previous_value is None)
         self.assertTrue(instance._values['int3'].previous_value is None)
+        self.assertTrue(instance._values['int4'].previous_value is None)
         self.assertTrue(instance._values['int5'].previous_value is None)
         self.assertTrue(instance._values['int6'].previous_value is None)
 
-        # When a column has a default value and that field has no explicit value specified at
-        # the instance creation, the previous_value should be set to the default value to
-        # avoid any undesired update
-        self.assertEqual(instance._values['int2'].previous_value, 456)
-        self.assertIsNotNone(instance._values['int4'])
-
         # All explicitely set columns, and those with default values are
         # flagged has changed.
         self.assertTrue(set(instance.get_changed_columns()) == set([

tests/integration/cqlengine/model/test_updates.py

@@ -134,3 +134,157 @@ class ModelUpdateTests(BaseCassEngTestCase):
         m0 = TestUpdateModel.create(count=5, text='monkey')
         with self.assertRaises(ValidationError):
             m0.update(partition=uuid4())
+
+
+class ModelWithDefault(Model):
+    id          = columns.Integer(primary_key=True)
+    mf          = columns.Map(columns.Integer, columns.Integer)
+    dummy       = columns.Integer(default=42)
+
+
+class ModelWithDefaultCollection(Model):
+    id          = columns.Integer(primary_key=True)
+    mf          = columns.Map(columns.Integer, columns.Integer, default={2:2})
+    dummy       = columns.Integer(default=42)
+
+
+class ModelWithDefaultTests(BaseCassEngTestCase):
+    def setUp(self):
+        sync_table(ModelWithDefault)
+
+    def tearDown(self):
+        drop_table(ModelWithDefault)
+
+    def test_value_override_with_default(self):
+        """
+        Updating a row with a new Model instance shouldn't set columns to defaults
+
+        @since 3.9
+        @jira_ticket PYTHON-657
+        @expected_result column value should not change
+
+        @test_category object_mapper
+        """
+        initial = ModelWithDefault(id=1, mf={0: 0}, dummy=0)
+        initial.save()
+
+        self.assertEqual(ModelWithDefault.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': 0, 'mf': {0: 0}})
+
+        second = ModelWithDefault(id=1)
+        second.update(mf={0: 1})
+
+        self.assertEqual(ModelWithDefault.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': 0, 'mf': {0: 1}})
+
+    def test_value_is_written_if_is_default(self):
+        """
+        Check if the we try to update with the default value, the update
+        happens correctly
+        @since 3.9
+        @jira_ticket PYTHON-657
+        @expected_result column value should be updated
+
+        @test_category object_mapper
+        :return:
+        """
+        initial = ModelWithDefault(id=1)
+        initial.mf = {0: 0}
+        initial.dummy = 42
+        initial.update()
+
+        self.assertEqual(ModelWithDefault.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': 42, 'mf': {0: 0}})
+
+    def test_null_update_is_respected(self):
+        """
+        Check if the we try to update with None under particular
+        circumstances, it works correctly
+        @since 3.9
+        @jira_ticket PYTHON-657
+        @expected_result column value should be updated to None
+
+        @test_category object_mapper
+        :return:
+        """
+        ModelWithDefault.create(id=1, mf={0: 0}).save()
+
+        q = ModelWithDefault.objects.all().allow_filtering()
+        obj = q.filter(id=1).get()
+
+        obj.update(dummy=None)
+
+        self.assertEqual(ModelWithDefault.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': None, 'mf': {0: 0}})
+
+    def test_only_set_values_is_updated(self):
+        """
+        Test the updates work as expected when an object is deleted
+        @since 3.9
+        @jira_ticket PYTHON-657
+        @expected_result the non updated column is None and the
+        updated column has the set value
+
+        @test_category object_mapper
+        """
+
+        ModelWithDefault.create(id=1, mf={1: 1}, dummy=1).save()
+
+        item = ModelWithDefault.filter(id=1).first()
+        ModelWithDefault.objects(id=1).delete()
+        item.mf = {1: 2}
+
+        item.save()
+
+        self.assertEqual(ModelWithDefault.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': None, 'mf': {1: 2}})
+
+    def test_collections(self):
+        """
+        Test the updates work as expected when an object is deleted
+        @since 3.9
+        @jira_ticket PYTHON-657
+        @expected_result the non updated column is None and the
+        updated column has the set value
+
+        @test_category object_mapper
+        """
+        ModelWithDefault.create(id=1, mf={1: 1, 2: 1}, dummy=1).save()
+        item = ModelWithDefault.filter(id=1).first()
+
+        item.update(mf={2:1})
+        self.assertEqual(ModelWithDefault.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': 1, 'mf': {2: 1}})
+
+    def test_collection_with_default(self):
+        """
+        Test the updates work as expected when an object is deleted
+        @since 3.9
+        @jira_ticket PYTHON-657
+        @expected_result the non updated column is None and the
+        updated column has the set value
+
+        @test_category object_mapper
+        """
+        sync_table(ModelWithDefaultCollection)
+        item = ModelWithDefaultCollection.create(id=1, mf={1: 1}, dummy=1).save()
+        self.assertEqual(ModelWithDefaultCollection.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': 1, 'mf': {1: 1}})
+
+        item.update(mf={2: 2})
+        self.assertEqual(ModelWithDefaultCollection.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': 1, 'mf': {2: 2}})
+
+        item.update(mf=None)
+        self.assertEqual(ModelWithDefaultCollection.objects().all().get()._as_dict(),
+                         {'id': 1, 'dummy': 1, 'mf': {}})
+
+        item = ModelWithDefaultCollection.create(id=2, dummy=2).save()
+        self.assertEqual(ModelWithDefaultCollection.objects().all().get(id=2)._as_dict(),
+                         {'id': 2, 'dummy': 2, 'mf': {2: 2}})
+
+        item.update(mf={1: 1, 4: 4})
+        self.assertEqual(ModelWithDefaultCollection.objects().all().get(id=2)._as_dict(),
+                         {'id': 2, 'dummy': 2, 'mf': {1: 1, 4: 4}})
+
+        drop_table(ModelWithDefaultCollection)

tests/integration/cqlengine/query/test_updates.py

@@ -290,6 +290,34 @@ class QueryUpdateTests(BaseCassEngTestCase):
                 text_map__remove=["bar"]
             )
 
+    @execute_count(3)
+    def test_an_extra_delete_is_not_sent(self):
+        """
+        Test to ensure that an extra DELETE is not sent if an object is read
+        from the DB with a None value
+
+        @since 3.9
+        @jira_ticket PYTHON-719
+        @expected_result only three queries are executed, the first one for
+        inserting the object, the second one for reading it, and the third
+        one for updating it
+
+        @test_category object_mapper
+        """
+        partition = uuid4()
+        cluster = 1
+
+        TestQueryUpdateModel.objects.create(
+            partition=partition, cluster=cluster)
+
+        obj = TestQueryUpdateModel.objects(
+            partition=partition, cluster=cluster).first()
+
+        self.assertFalse(any([obj._values[column].deleted for column in obj._values]))
+
+        obj.text = 'foo'
+        obj.save()
+        
 
 class StaticDeleteModel(Model):
     example_id = columns.Integer(partition_key=True, primary_key=True, default=uuid4)