From d46e9d5ba50e85fcf5a541bbd2e9e6f8a97e1a85 2012-04-05 18:42:12
From: MinRK <benjaminrk@gmail.com>
Date: 2012-04-05 18:42:12
Subject: [PATCH] dictdb queries should [shallow] copy records

pop/edit actions on the results of queries should not affect the records themselves.

Hub.db_query pops buffers out of results in some situations, which would break future result_requests.  

Associated tests are included.
---

diff --git a/IPython/parallel/controller/dictdb.py b/IPython/parallel/controller/dictdb.py
index 10ce73e..0b4a3c7 100644
--- a/IPython/parallel/controller/dictdb.py
+++ b/IPython/parallel/controller/dictdb.py
@@ -46,7 +46,7 @@ We support a subset of mongodb operators:
 #  the file COPYING, distributed as part of this software.
 #-----------------------------------------------------------------------------
 
-
+from copy import copy
 from datetime import datetime
 
 from IPython.config.configurable import LoggingConfigurable
@@ -120,7 +120,7 @@ class DictDB(BaseDB):
 
         for rec in self._records.itervalues():
             if self._match_one(rec, tests):
-                matches.append(rec)
+                matches.append(copy(rec))
         return matches
 
     def _extract_subdict(self, rec, keys):
@@ -139,9 +139,9 @@ class DictDB(BaseDB):
 
     def get_record(self, msg_id):
         """Get a specific Task Record, by msg_id."""
-        if not self._records.has_key(msg_id):
+        if not msg_id in self._records:
             raise KeyError("No such msg_id %r"%(msg_id))
-        return self._records[msg_id]
+        return copy(self._records[msg_id])
 
     def update_record(self, msg_id, rec):
         """Update the data in an existing record."""
diff --git a/IPython/parallel/tests/test_client.py b/IPython/parallel/tests/test_client.py
index bd6ab3a..b79b0dc 100644
--- a/IPython/parallel/tests/test_client.py
+++ b/IPython/parallel/tests/test_client.py
@@ -238,6 +238,18 @@ class TestClient(ClusterTestCase):
         for rec in found:
             self.assertTrue('msg_id' in rec.keys())
     
+    def test_db_query_get_result(self):
+        """pop in db_query shouldn't pop from result itself"""
+        self.client[:].apply_sync(lambda : 1)
+        found = self.client.db_query({'msg_id': {'$ne' : ''}})
+        rc2 = clientmod.Client(profile='iptest')
+        # If this bug is not fixed, this call will hang:
+        ar = rc2.get_result(self.client.history[-1])
+        ar.wait(2)
+        self.assertTrue(ar.ready())
+        ar.get()
+        rc2.close()
+    
     def test_db_query_in(self):
         """test db query with '$in','$nin' operators"""
         hist = self.client.hub_history()
diff --git a/IPython/parallel/tests/test_db.py b/IPython/parallel/tests/test_db.py
index 66a4057..cf0d65a 100644
--- a/IPython/parallel/tests/test_db.py
+++ b/IPython/parallel/tests/test_db.py
@@ -182,6 +182,36 @@ class TestDictBackend(TestCase):
         query = {'msg_id' : {'$ne' : None}}
         recs = self.db.find_records(query)
         self.assertTrue(len(recs) >= 10)
+    
+    def test_pop_safe_get(self):
+        """editing query results shouldn't affect record [get]"""
+        msg_id = self.db.get_history()[-1]
+        rec = self.db.get_record(msg_id)
+        rec.pop('buffers')
+        rec['garbage'] = 'hello'
+        rec2 = self.db.get_record(msg_id)
+        self.assertTrue('buffers' in rec2)
+        self.assertFalse('garbage' in rec2)
+    
+    def test_pop_safe_find(self):
+        """editing query results shouldn't affect record [find]"""
+        msg_id = self.db.get_history()[-1]
+        rec = self.db.find_records({'msg_id' : msg_id})[0]
+        rec.pop('buffers')
+        rec['garbage'] = 'hello'
+        rec2 = self.db.find_records({'msg_id' : msg_id})[0]
+        self.assertTrue('buffers' in rec2)
+        self.assertFalse('garbage' in rec2)
+
+    def test_pop_safe_find_keys(self):
+        """editing query results shouldn't affect record [find+keys]"""
+        msg_id = self.db.get_history()[-1]
+        rec = self.db.find_records({'msg_id' : msg_id}, keys=['buffers'])[0]
+        rec.pop('buffers')
+        rec['garbage'] = 'hello'
+        rec2 = self.db.find_records({'msg_id' : msg_id})[0]
+        self.assertTrue('buffers' in rec2)
+        self.assertFalse('garbage' in rec2)
 
 
 class TestSQLiteBackend(TestDictBackend):