Merge pull request #512 from lucemia/property-with-empty-list-4

Ignore Entity property with empty list

gcloud/datastore/connection.py

@@ -425,6 +425,10 @@ def save_entity(self, dataset_id, key_pb, properties,
            will be replaced by those passed in ``properties``;  properties
            not passed in ``properties`` no longer be set for the entity.
 
+        .. note::
+           When saving an entity to the backend, a property value set as
+           an empty list cannot be saved and will be ignored.
+
         :type dataset_id: string
         :param dataset_id: The ID of the dataset in which to save the entity.
 
@@ -470,19 +474,8 @@ def save_entity(self, dataset_id, key_pb, properties,
         insert.key.CopyFrom(key_pb)
 
         for name, value in properties.items():
-            prop = insert.property.add()
-            # Set the name of the property.
-            prop.name = name
-
-            # Set the appropriate value.
-            helpers._set_protobuf_value(prop.value, value)
-
-            if name in exclude_from_indexes:
-                if not isinstance(value, list):
-                    prop.value.indexed = False
-
-                for sub_value in prop.value.list_value:
-                    sub_value.indexed = False
+            helpers._set_protobuf_property(insert.property, name, value,
+                                           name not in exclude_from_indexes)
 
         # If this is in a transaction, we should just return True. The
         # transaction will handle assigning any keys as necessary.

gcloud/datastore/helpers.py

@@ -218,6 +218,44 @@ def _get_value_from_property_pb(property_pb):
     return _get_value_from_value_pb(property_pb.value)
 
 
+def _set_protobuf_property(property_pb, name, value, indexed):
+    """Assign 'name', 'value', 'indexed' to the correct 'property_pb'.
+
+    Some value (empty list) cannot be directly assigned; this function handles
+    them correctly.
+
+    :type property_pb: :class:`gcloud.datastore.datastore_v1_pb2.Property`
+    :param property_pb: The value protobuf to which the value is being
+                        assigned.
+
+    :type name: string
+    :param name: The name to be assigned.
+
+    :type value: `datetime.datetime`, boolean, float, integer, string,
+                 :class:`gcloud.datastore.key.Key`,
+                 :class:`gcloud.datastore.entity.Entity`,
+    :param value: The value to be assigned.
+
+    :type indexed: boolean
+    :param indexed: The flag indicates the property should to be indexed or
+                    not.
+    """
+    if isinstance(value, list) and len(value) == 0:
+        return
+
+    prop = property_pb.add()
+    prop.name = name
+
+    _set_protobuf_value(prop.value, value)
+
+    if not indexed:
+        if not isinstance(value, list):
+            prop.value.indexed = False
+
+        for sub_value in prop.value.list_value:
+            sub_value.indexed = False
+
+
 def _set_protobuf_value(value_pb, val):
     """Assign 'val' to the correct subfield of 'value_pb'.
 

gcloud/datastore/test_connection.py

@@ -906,6 +906,45 @@ def test_allocate_ids_non_empty(self):
         for key_before, key_after in zip(before_key_pbs, request.key):
             _compare_key_pb_after_request(self, key_before, key_after)
 
+    def test_save_entity_w_empty_list_value(self):
+        from gcloud.datastore import datastore_v1_pb2 as datastore_pb
+
+        DATASET_ID = 'DATASET'
+        key_pb = self._make_key_pb(DATASET_ID)
+        rsp_pb = datastore_pb.CommitResponse()
+        conn = self._makeOne()
+        URI = '/'.join([
+            conn.API_BASE_URL,
+            'datastore',
+            conn.API_VERSION,
+            'datasets',
+            DATASET_ID,
+            'commit',
+        ])
+        http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
+        result = conn.save_entity(DATASET_ID, key_pb,
+                                  {'foo': u'Foo', 'bar': []})
+        self.assertEqual(result, (False, None))
+        cw = http._called_with
+        self._verifyProtobufCall(cw, URI, conn)
+        rq_class = datastore_pb.CommitRequest
+        request = rq_class()
+        request.ParseFromString(cw['body'])
+        self.assertEqual(request.transaction, '')
+        mutation = request.mutation
+        self.assertEqual(len(mutation.insert_auto_id), 0)
+        upserts = list(mutation.upsert)
+        self.assertEqual(len(upserts), 1)
+        upsert = upserts[0]
+        _compare_key_pb_after_request(self, key_pb, upsert.key)
+        props = list(upsert.property)
+        self.assertEqual(len(props), 1)
+        self.assertEqual(props[0].name, 'foo')
+        self.assertEqual(props[0].value.string_value, u'Foo')
+        self.assertEqual(props[0].value.indexed, True)
+        self.assertEqual(len(mutation.delete), 0)
+        self.assertEqual(request.mode, rq_class.NON_TRANSACTIONAL)
+
     def test_save_entity_wo_transaction_w_upsert(self):
         from gcloud.datastore import datastore_v1_pb2 as datastore_pb