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

[Vincenzo Di Somma]

The class factories are not making the code look nice, are just making more confusion, plus all the tests weren't actually testing nothing.

The class factories are not making the code look nice, are just making more confusion, plus all the tests weren't actually testing nothing.

[Eric Casteleijn (thisfred)]

Big improvement, tests pass, yay!

review: Approve

[Manuel de la Peña (mandel)]

+1

review: Approve

=== modified file 'desktopcouch/contacts/record.py'
--- desktopcouch/contacts/record.py	2010-05-27 18:45:07 +0000
+++ desktopcouch/contacts/record.py	2010-11-01 17:09:45 +0000
@@ -17,51 +17,41 @@
 # Authors: Eric Casteleijn <[e-mail]>
 #          Nicola Larosa <[e-mail]>
 #          Mark G. Saye <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 """A dictionary based contact record representation."""
 
-from desktopcouch.records.record import Record
-
-CONTACT_RECORD_TYPE = (
-    'http://www.freedesktop.org/wiki/Specifications/desktopcouch/contact')
-# keep in sync with the above contacts record type
-SINGLE_FIELDS = frozenset((
-    'first_name', 'middle_name', 'last_name', 'title', 'suffix', 'birth_date',
-    'nick_name', 'spouse_name', 'wedding_date', 'company', 'department',
-    'job_title', 'manager_name', 'assistant_name', 'office',
-))
-LIST_FIELDS = frozenset((
-    'addresses', 'phone_numbers', 'email_addresses', 'urls', 'im_addresses',
-))
-ALL_FIELDS = SINGLE_FIELDS | LIST_FIELDS
-
-
-class ContactBase(Record):
-    """
-    A base for Contact records.
-
-    Use make_contact_class to create the Contact class with the required fields.
-    """
+from desktopcouch.records.record import Record, RecordField
+
+CONTACT_RECORD_TYPE = \
+    'http://www.freedesktop.org/wiki/Specifications/desktopcouch/contact'
+
+
+class Contact(Record):
+    """Contact records."""
+
+    first_name = RecordField('first_name')
+    middle_name = RecordField('middle_name')
+    last_name = RecordField('last_name')
+    title = RecordField('title')
+    suffix = RecordField('suffix')
+    birth_date = RecordField('birth_date')
+    nick_name = RecordField('nick_name')
+    spouse_name = RecordField('spouse_name')
+    wedding_date = RecordField('wedding_date')
+    company = RecordField('company')
+    department = RecordField('department')
+    job_title = RecordField('job_title')
+    manager_name = RecordField('manager_name')
+    assistant_name = RecordField('assistant_name')
+    office = RecordField('office')
+    addresses = RecordField('addresses')
+    phone_numbers = RecordField('phone_numbers')
+    email_addresses = RecordField('email_addresses')
+    urls = RecordField('urls')
+    im_addresses = RecordField('im_addresses')
 
     def __init__(self, data=None, record_id=None):
-        super(ContactBase, self).__init__(
+        super(Contact, self).__init__(
             record_id=record_id, data=data, record_type=CONTACT_RECORD_TYPE)
-
-
-def make_contact_class(field_names):
-    """Contact class factory function. field_names is a strings iterable."""
-    ContactClass = type('Contact', (ContactBase,), {})
-    for field_name in field_names:
-
-        # without the _field_name param, only the last one gets used
-        def fget(self, _field_name=field_name):
-            return self.get(_field_name)
-
-        def fset(self, value, _field_name=field_name):
-            self[_field_name] = value
-
-        setattr(ContactClass, field_name, property(fget, fset))
-    return ContactClass
-
-
-Contact = make_contact_class(ALL_FIELDS)
+        

=== modified file 'desktopcouch/contacts/tests/test_record.py'
--- desktopcouch/contacts/tests/test_record.py	2010-05-27 18:45:07 +0000
+++ desktopcouch/contacts/tests/test_record.py	2010-11-01 17:09:45 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.
+# Copyright 2009-2010 Canonical Ltd.
 #
 # This file is part of desktopcouch-contacts.
 #
@@ -15,32 +15,37 @@
 # along with desktopcouch.  If not, see <http://www.gnu.org/licenses/>.
 #
 # Authors: Nicola Larosa <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 """Tests for the ContactDocument class"""
 
 import testtools
 
-from desktopcouch.contacts import record as contact_mod
+from desktopcouch.contacts.record import Contact
 
+SINGLE_FIELDS = frozenset((
+    'first_name', 'middle_name', 'last_name', 'title', 'suffix', 'birth_date',
+    'nick_name', 'spouse_name', 'wedding_date', 'company', 'department',
+    'job_title', 'manager_name', 'assistant_name', 'office'))
+LIST_FIELDS = frozenset((
+    'addresses', 'phone_numbers', 'email_addresses', 'urls', 'im_addresses'))
+ALL_FIELDS = SINGLE_FIELDS | LIST_FIELDS
 
 class TestContactRecord(testtools.TestCase):
     """Test the Contact Record object."""
 
     def test_contact_record(self):
         """Test that we get the correct record type."""
-        contact = contact_mod.Contact()
-        self.assertEqual(contact.record_type, contact_mod.CONTACT_RECORD_TYPE)
-        for field_name in contact_mod.SINGLE_FIELDS:
-            setattr(contact, field_name, field_name)
-            self.assertEqual(getattr(contact, field_name), field_name)
-            # direct access to internal representation
-            self.assertEqual(contact._data[field_name], field_name)
-        for field_name in contact_mod.LIST_FIELDS:
-            setattr(contact, field_name, [field_name])
-            self.assertEqual(getattr(contact, field_name), [field_name])
-            # direct access to internal representation
-            self.assert_(field_name in contact._data[field_name].values())
-        # transversal check
-        all_keys = set(contact_mod.ALL_FIELDS)
-        all_keys.add('record_type')
-        self.assertEqual(set(contact.keys()), all_keys)
+        contact = Contact()
+        for field_name in SINGLE_FIELDS:
+            self.assert_(hasattr(contact, field_name))
+            # direct access to internal representation
+        for field_name in LIST_FIELDS:
+            self.assert_(hasattr(contact, field_name))
+            # direct access to internal representation
+        first_name = 'manuel'
+        contact.first_name = first_name
+        self.assertEqual(first_name, contact.first_name)
+        self.assertEqual(set(['record_type', 'first_name']),
+                         set(contact.keys()))
+        

=== modified file 'desktopcouch/notes/record.py'
--- desktopcouch/notes/record.py	2010-05-28 08:30:50 +0000
+++ desktopcouch/notes/record.py	2010-11-01 17:09:45 +0000
@@ -15,53 +15,24 @@
 # along with desktopcouch.  If not, see <http://www.gnu.org/licenses/>.
 #
 # Authors: Rodrigo Moya <[e-mail]>
-
-"""A dictionary based note record representation."""
-
-from time import strptime
-from desktopcouch.records.record import Record
-
-NOTE_RECORD_TYPE = 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/note'
-# keep in sync with the above notes record type
-FIELDS = {
-    # String fields
-    'note_format': 'string',
-    'title': 'string',
-    'content': 'string',
-    # Date fields
-    'last_change_date': 'date',
-    'create_date': 'date'
-}
-
-class NoteBase(Record):
-    """
-    A base for Note records.
-
-    Use make_note_class to create the Note class with the required fields.
-    """
+#          Vincenzo Di Somma <[e-mail]>
+
+"""Note record representation."""
+
+from desktopcouch.records.record import Record, RecordField, DateRecordField
+
+NOTE_RECORD_TYPE = \
+          'http://www.freedesktop.org/wiki/Specifications/desktopcouch/note'
+
+class Note(Record):
+    """Note record """
+
+    note_format = RecordField('note_format')
+    title = RecordField('title')
+    content = RecordField('content')
+    last_change_date = DateRecordField('last_change_date')
+    create_date = DateRecordField('create_date')
 
     def __init__(self, data=None, record_id=None):
-        super(NoteBase, self).__init__(
+        super(Note, self).__init__(
             record_id=record_id, data=data, record_type=NOTE_RECORD_TYPE)
-
-def make_note_class(fields):
-    """Note class factory function. field_names is a list of strings."""
-    NoteClass = type('Note', (NoteBase,), {})
-    for field_name in fields:
-
-        def fget(self, _field_name=field_name):
-            return self.get(_field_name)
-
-        def fset(self, value, _field_name=field_name):
-            field_type = fields[_field_name]
-            if field_type == 'date':
-                # Check that it is a date with the correct format
-                date_value = strptime(value, "%Y-%m-%dT%H:%M:%S")
-                if date_value is None:
-                    return
-            self[_field_name] = value
-
-        setattr(NoteClass, field_name, property(fget, fset))
-    return NoteClass
-
-Note = make_note_class(FIELDS)

=== modified file 'desktopcouch/notes/tests/test_record.py'
--- desktopcouch/notes/tests/test_record.py	2010-05-28 09:51:46 +0000
+++ desktopcouch/notes/tests/test_record.py	2010-11-01 17:09:45 +0000
@@ -16,35 +16,40 @@
 #
 # Authors: Nicola Larosa <[e-mail]>
 #          Rodrigo Moya <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 """Tests for the NoteDocument class"""
 
 import testtools
-from time import (strftime, gmtime)
-from desktopcouch.notes import record as note_mod
+from time import strftime, gmtime
+from desktopcouch.notes.record import Note, NOTE_RECORD_TYPE
+
+FIELDS = {
+    # String fields
+    'note_format': 'string',
+    'title': 'string',
+    'content': 'string',
+    # Date fields
+    'last_change_date': 'date',
+    'create_date': 'date'
+}
 
 class TestNoteRecord(testtools.TestCase):
     """Test the Note Record object."""
 
     def test_note_record(self):
         """Test that we get the correct record type."""
-        note = note_mod.Note()
-        self.assertEqual(note_mod.NOTE_RECORD_TYPE, note.record_type)
-        for field_name in note_mod.FIELDS:
-            field_type = note_mod.FIELDS[field_name]
-            if field_type == 'string':
-                setattr(note, field_name, 'value')
-                self.assertEqual('value', note._data[field_name])
-                self.assertEqual(getattr(note, field_name), 'value')
-            elif field_type == 'date':
-                current_time = strftime("%Y-%m-%dT%H:%M:%S", gmtime())
-                setattr(note, field_name, current_time)
-                self.assertEqual(current_time, note._data[field_name])
-                self.assertEqual(getattr(note, field_name), current_time)
-            else:
-                self.fail('Unknown field type: %s' % field_type)
-
-        # Check all keys
-        all_keys = set(note_mod.FIELDS)
-        all_keys.add('record_type')
-        self.assertEqual(set(note.keys()), all_keys)
+        note = Note()
+        self.assertEqual(NOTE_RECORD_TYPE, note.record_type)
+        for field_name in FIELDS:
+            self.assert_(hasattr(note, field_name))
+        now = gmtime()
+        now_str = strftime("%Y-%m-%dT%H:%M:%S", now)
+        note.last_change_date = now_str
+        # For some reason that I ignore strptime and strftime return
+        # different timezones so we remove the timezone value
+        # before asserting equal
+        self.assertEqual(now[0:-1], note.last_change_date[0:-1])
+
+
+

=== modified file 'desktopcouch/records/record.py'
--- desktopcouch/records/record.py	2010-10-31 15:52:12 +0000
+++ desktopcouch/records/record.py	2010-11-01 17:09:45 +0000
@@ -23,6 +23,7 @@
 
 import re
 import uuid
+from time import strptime
 
 RX = re.compile('[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}')
 
@@ -76,6 +77,27 @@
         for value in data.values():
             validate(value)
 
+
+class RecordField(object):
+    """Descriptor to manage record fields"""
+
+    def __init__(self, field_name):
+        self.field_name = field_name
+
+    def __get__(self, obj, type=None):
+        return obj.get(self.field_name)
+
+    def __set__(self, obj, value):
+        obj[self.field_name] = value
+
+            
+class DateRecordField(RecordField):
+    """Descriptor to manage data record fields"""
+
+    def __set__(self, obj, value):
+        obj[self.field_name] = strptime(value, "%Y-%m-%dT%H:%M:%S")
+
+
 class IllegalKeyException(Exception):
     """Exception when attempting to set a key we don't allow."""
     pass

=== modified file 'desktopcouch/tasks/record.py'
--- desktopcouch/tasks/record.py	2010-05-28 08:30:50 +0000
+++ desktopcouch/tasks/record.py	2010-11-01 17:09:45 +0000
@@ -15,41 +15,22 @@
 # along with desktopcouch.  If not, see <http://www.gnu.org/licenses/>.
 #
 # Authors: Rodrigo Moya <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 
 """A dictionary based task record representation."""
 
-from desktopcouch.records.record import Record
-
-TASK_RECORD_TYPE = 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/task'
-# keep in sync with the above tasks record type
-FIELD_NAMES = (
-    'summary'
-)
-
-class TaskBase(Record):
-    """
-    A base for Task records.
-
-    Use make_task_class to create the Note class with the required fields.
-    """
+from desktopcouch.records.record import Record, RecordField
+
+TASK_RECORD_TYPE = \
+            'http://www.freedesktop.org/wiki/Specifications/desktopcouch/task'
+
+class Task(Record):
+    """ Task records."""
+
+    summary = RecordField('summary')
 
     def __init__(self, data=None, record_id=None):
-        super(TaskBase, self).__init__(
+        super(Task, self).__init__(
             record_id=record_id, data=data, record_type=TASK_RECORD_TYPE)
 
-def make_task_class(field_names):
-    """Task class factory function. field_names is a list of strings."""
-    TaskClass = type('Task', (TaskBase,), {})
-    for field_name in field_names:
-
-        def fget(self, _field_name=field_name):
-            return self.get(_field_name)
-
-        def fset(self, value, _field_name=field_name):
-            self[_field_name] = value
-
-        setattr(TaskClass, field_name, property(fget, fset))
-    return TaskClass
-
-Task = make_task_class(FIELD_NAMES)

=== modified file 'desktopcouch/tasks/tests/test_record.py'
--- desktopcouch/tasks/tests/test_record.py	2010-05-28 09:51:46 +0000
+++ desktopcouch/tasks/tests/test_record.py	2010-11-01 17:09:45 +0000
@@ -15,26 +15,23 @@
 # along with desktopcouch.  If not, see <http://www.gnu.org/licenses/>.
 #
 # Authors: Rodrigo Moya <[e-mail]>
+#          Vincenzo Di Somma <[e-mail]>
 
 """Tests for the Tasks record classes"""
 
 import testtools
 
-from desktopcouch.tasks import record as task_mod
+from desktopcouch.tasks.record import Task, TASK_RECORD_TYPE
 
 class TestTaskRecord(testtools.TestCase):
     """Test the Task Record object."""
 
     def test_task_record(self):
         """Test that we get the correct record type."""
-        task = task_mod.Task()
-        self.assertEqual(task_mod.TASK_RECORD_TYPE, task.record_type)
-        for field_name in task_mod.FIELD_NAMES:
-            setattr(task, field_name, 'value')
-            self.assertEqual('value', task._data[field_name])
-            self.assertEqual(getattr(task, field_name), 'value')
+        task = Task()
+        self.assertEqual(TASK_RECORD_TYPE, task.record_type)
+        self.assert_(hasattr(task, 'summary'))
+        summary = "manuel"
+        task.summary = summary
+        self.assertEqual(summary, task.summary)
 
-        # Check all keys
-        all_keys = set(task_mod.FIELD_NAMES)
-        all_keys.add('record_type')
-        self.assertEqual(set(task.keys()), all_keys)