Merge lp:~vds/desktopcouch/mergeable_list_not_default_anymore into lp:desktopcouch

[Vincenzo Di Somma]

Mergeable lists are not the default for lists anymore, plus clean up a bit.

Mergeable lists are not the default for lists anymore, plus clean up a bit.

[Eric Casteleijn (thisfred)]

I love the smell of awesome in the morning.

review: Approve

[Manuel de la Peña (mandel)]

approaved

[dobey (dobey)]

Voting does not meet specified criteria. Required: Approve >= 2, Disapprove == 0, Needs Fixing == 0, Needs Information == 0, Resubmit == 0. Got: 1 Approve.

[John Lenton (chipaca)]

review: Approve

=== modified file 'desktopcouch/records/field_registry.py'
--- desktopcouch/records/field_registry.py	2010-04-05 17:46:53 +0000
+++ desktopcouch/records/field_registry.py	2010-11-02 23:33:40 +0000
@@ -21,6 +21,7 @@
 
 import copy
 
+from desktopcouch.records.record import MergeableList 
 ANNOTATION_NAMESPACE = 'application_annotations'
 
 
@@ -132,7 +133,8 @@
         values.update({self._field_name: value})
         if not root_list:
             # This is the first value we add to the list
-            record[self._root_list] = [values]
+            ml = MergeableList.from_list([values])
+            record[self._root_list] = ml
             uuid_key = record[self._root_list].get_uuid_for_index(-1)
             application_annotations[self._uuid_field] = uuid_key
             return
 
=== modified file 'desktopcouch/records/record.py'
--- desktopcouch/records/record.py	2010-10-31 23:20:14 +0000
+++ desktopcouch/records/record.py	2010-11-02 23:33:40 +0000
@@ -42,19 +42,8 @@
     """Create a RecordDict or RecordList from passed data."""
     if isinstance(data, dict):
         return _build_from_dict(data)
-    if isinstance(data, (list, tuple)):
-        return _build_from_list(data)
     return data
 
-def _build_from_list(data):
-    """Create a RecordList from passed data."""
-    if not data:
-        raise ValueError("Can't set empty list values.'")
-    result = MergeableList({})
-    for value in data:
-        result.append(record_factory(value))
-    return result
-
 def _build_from_dict(data):
     """Create a RecordDict from passed data."""
     result = RecordDict({})
@@ -155,7 +144,7 @@
         return value
 
     def __setitem__(self, key, item):
-        if isinstance(item, (list, tuple, dict)):
+        if isinstance(item, dict):
             item = record_factory(item)
         if hasattr(item, '_data'):
             item = item._data
@@ -258,6 +247,16 @@
 class MergeableList(RecordData):
     """An object that represents a list of complex values."""
 
+    @classmethod
+    def from_list(klass, data):
+        """Create a RecordList from passed data."""
+        if not data:
+            raise ValueError("Can't set empty list values.'")
+        result = klass({})
+        for value in data:
+            result.append(record_factory(value))
+        return result
+
     def __getitem__(self, index):
         if not isinstance(index, int):
             raise TypeError(
 
=== modified file 'desktopcouch/records/tests/test_record.py'
--- desktopcouch/records/tests/test_record.py	2010-10-31 15:52:12 +0000
+++ desktopcouch/records/tests/test_record.py	2010-11-02 23:33:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.
+# Copyright 2009-2010 Canonical Ltd.
 #
 # This file is part of desktopcouch.
 #
@@ -15,6 +15,7 @@
 # along with desktopcouch.  If not, see <http://www.gnu.org/licenses/>.
 #
 # Authors: Eric Casteleijn <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 """Tests for the RecordDict object on which the Contacts API is built."""
 
@@ -55,6 +56,8 @@
         self.record = Record(self.dict)
 
     def test_revision(self):
+        """Test document always has a revision field and that the revision
+        changes when the document is updated"""
         self.assertEquals(self.record.record_revision, None)
         def set_rev(rec): rec.record_revision = "1"
         self.assertRaises(AttributeError, set_rev, self.record)
@@ -77,6 +80,7 @@
         db.delete_record(record_id)
 
     def test_delitem(self):
+        """Test removing a field from the record"""
         def f(r):
             del r["_id"]
         self.assertRaises(KeyError, f, self.record)
@@ -84,25 +88,28 @@
         del self.record["a"]
 
     def test_iter(self):
+        """Tests it is possible to iterate over the record fields"""
         self.assertEquals(sorted(list(iter(self.record))),
             ['a', 'b', 'record_type', 'subfield', 'subfield_uuid'])
 
     def test_setitem_internal(self):
+        """Test it is not possible to set a private field on a record"""
         def f(r):
             r["_id"] = "new!"
         self.assertRaises(IllegalKeyException, f, self.record)
 
     def test_no_record_type(self):
+        """Test that creating a record with no record type fails"""
         self.assertRaises(NoRecordTypeSpecified, Record, {})
 
     def test_get_item(self):
-        "Does a RecordDict basically wrap a dict properly?"
+        """Does a RecordDict basically wrap a dict properly?"""
         self.assertEqual(self.dict["a"], self.record["a"])
         self.assertRaises(KeyError,
                           self.record.__getitem__, "application_annotations")
 
     def test_get(self):
-        "Does a RecordDict get() work?"
+        """Does a RecordDict get() work?"""
         self.assertEqual(self.dict["a"], self.record.get("a"))
         self.assertEqual(None, self.record.get("application_annotations"))
 
@@ -131,7 +138,7 @@
             my_app_data['foo'])
 
     def test_loads_dict_subdict(self):
-        "Are subdicts supported?"
+        """Are subdicts supported?"""
         self.assertEqual(2, len(self.record["subfield"]))
         subfield_single = self.record["subfield"]
         self.assertEqual(
@@ -139,7 +146,7 @@
             self.dict["subfield"]["field11s"])
 
     def test_loads_dict_multi_subdict(self):
-        "Are subdicts with multiple entries supported?"
+        """Are subdicts with multiple entries supported?"""
         self.assertEqual(2, len(self.record["subfield_uuid"]))
 
     def test_mergeable_list_index(self):
@@ -212,21 +219,22 @@
         """Test missing data rmeoval"""
         self.assertRaises(ValueError, self.record["subfield_uuid"].remove, 
             "missing_data")
+
     def test_mergeable_list_remove_last(self):
         """Test that exception is raised when removing last item."""
-        self.record["subfield_uuid"] = [1]
+        self.record["subfield_uuid"] = MergeableList.from_list([1])
         self.assertRaises(ValueError, self.record["subfield_uuid"].remove, 1)
         
     def test_mergeable_list_pop_correct_index(self):
         """Test the pop method when working with a correct index."""
         value = [1, 2, 3, 4, 5]
-        self.record["subfield_uuid"] = value
+        self.record["subfield_uuid"] = MergeableList.from_list(value)
         # test with negative index
-        poped_value = self.record["subfield_uuid"].pop(-2)
-        self.assertEqual(value[-2], poped_value)
+        popped_value = self.record["subfield_uuid"].pop(-2)
+        self.assertEqual(value[-2], popped_value)
         # test with positive index
-        poped_value = self.record["subfield_uuid"].pop(1)
-        self.assertEqual(value[1], poped_value)
+        popped_value = self.record["subfield_uuid"].pop(1)
+        self.assertEqual(value[1], popped_value)
         
     def test_mergeable_list_pop_wrong_index(self):
         """Test pop when index is out or range."""
@@ -237,20 +245,20 @@
     
     def test_mergeable_list_pop_last(self):
         """Test that exception is raised when poping last item"""
-        self.record["subfield_uuid"] = [1]
+        self.record["subfield_uuid"] = MergeableList.from_list([1])
         self.assertRaises(ValueError, self.record["subfield_uuid"].pop, 0)
         
-    def test_tuple(self):
-        """Test assigning tuples to a key results in mergeable lists."""
-        rec = Record({'record_type': 'http://fnord.org/smorgasbord'})
-        rec['key'] = (1, 2, 3, 4)
-        self.assert_(isinstance(rec['key'], MergeableList))
-        self.assertEqual([1, 2, 3, 4], [value for value in rec['key']])
-
     def test_list(self):
         """Test assigning lists to a key results in mergeable lists."""
         rec = Record({'record_type': 'http://fnord.org/smorgasbord'})
         rec['key'] = [1, 2, 3, 4]
+        self.assert_(isinstance(rec['key'], list))
+        self.assertEqual([1, 2, 3, 4], [value for value in rec['key']])
+
+    def test_mergeable_list(self):
+        """Test assigning lists to a key results in mergeable lists."""
+        rec = Record({'record_type': 'http://fnord.org/smorgasbord'})
+        rec['key'] = MergeableList.from_list([1, 2, 3, 4])
         self.assert_(isinstance(rec['key'], MergeableList))
         self.assertEqual([1, 2, 3, 4], [value for value in rec['key']])
 
@@ -291,12 +299,14 @@
           self.record.record_type)
 
     def test_run_doctests(self):
+        """Run all doc tests from here to set the proper context (ctx)"""
         ctx = test_environment.test_context
         globs = { "db": CouchDatabase('testing', create=True, ctx=ctx) }
         results = doctest.testfile('../doc/records.txt', globs=globs)
         self.assertEqual(0, results.failed)
 
     def test_record_id(self):
+        """Test all passible way to assign a record id"""
         data = {"_id":"recordid"}
         record = Record(data, record_type="url")
         self.assertEqual(data["_id"], record.record_id)
@@ -309,6 +319,7 @@
             Record, data, record_id=record_id, record_type="url")
 
     def test_record_type_version(self):
+        """Test record type version support"""
         data = {"_id":"recordid"}
         record1 = Record(data, record_type="url")
         self.assertIs(None, record1.record_type_version)