Merge lp:~cjcurran/friends/another-test-fix-contacts-fb-import into lp:friends

[Conor Curran]

[lp:~cjcurran/friends/another-test-fix-contacts-fb-import]

50. By Conor Curran on 2012-10-31

rebase to pick up facebook id

[Robert Bruce Park (robru)]

Ok Conor, I've landed your branch, it is looking a lot better and the tests are now passing. Please delete that old branch that has the stale mp still outstanding, it looks bad just sitting there with a 'needs fixing' review seeing as it's now fixed and landed ;-)

One thing I just want to mention though, the Facebook._create_contact method is quite long, and only about half of it seems to be specific to Facebook. Ideally, what I would like to see is that you implement a method called Base._create_contact that handles all of the EBook logic, and then you'd have a very tiny wrapper around that from the Facebook plugin that just inserts the correct data into it. I'd like to see facebook.py not even import EBook at all (notice how it doesn't need to import any UOA modules despite implementing a ton of UOA logic).

Doing that will actually make it a lot easier to move forward with Twitter contacts, because you'll have more code re-use in the Base class and less code duplicated between protocols.

So when you start on the Twitter contacts, please don't even start importing EBook from twitter.py, first start working by dropping the EBook import from facebook.py and then work around that by wrapping it from the Base class.

Thanks!

review: Approve

=== modified file 'friends/errors.py'
--- friends/errors.py	2012-10-13 01:27:15 +0000
+++ friends/errors.py	2012-10-31 14:00:33 +0000
@@ -20,8 +20,7 @@
     'FriendsError',
     'UnsupportedProtocolError',
     ]
-
-
+    
 class FriendsError(Exception):
     """Base class for all internal Friends exceptions."""
 

=== modified file 'friends/protocols/facebook.py'
--- friends/protocols/facebook.py	2012-10-31 11:38:22 +0000
+++ friends/protocols/facebook.py	2012-10-31 14:00:33 +0000
@@ -15,6 +15,7 @@
 
 """The Facebook protocol plugin."""
 
+
 __all__ = [
     'Facebook',
     ]
@@ -23,7 +24,6 @@
 import logging
 
 from datetime import datetime, timedelta
-
 from gi.repository import EBook
 
 from friends.utils.avatar import Avatar
@@ -257,48 +257,90 @@
         else:
             self._unpublish(obj_id)
 
-    def fetch_contacts(self):
-        """Retrieve a list of up to 100 Facebook friends."""
-        limit = self._DOWNLOAD_LIMIT * 2
+    def _fetch_contacts(self):
+        """Retrieve a list of up to 1,000 Facebook friends."""
+        limit = 1000
         access_token = self._get_access_token()
         contacts = []
         url = ME_URL + '/friends'
         params = dict(
             access_token=access_token,
             limit=limit)
+        return self._follow_pagination(url, params, limit=limit)
 
-        return self._follow_pagination(url, params, limit)
+    def _fetch_contact(self, contact_id):
+        """Fetch the full, individual contact info."""
+        access_token = self._get_access_token()
+        url = API_BASE.format(id=contact_id)
+        params = dict(access_token=access_token)
+        return get_json(url, params)
 
     # This method can take the minimal contact information or full
     # contact info For now we only cache ID and the name in the
     # addressbook. Using custom field for name because I can't figure
     # out how econtact name works.
-    def create_contact(self, contact_json):
+    def _create_contact(self, contact_json):
+        vcard = EBook.VCard.new()
+
         vcafid = EBook.VCardAttribute.new(
             'social-networking-attributes', 'facebook-id')
         vcafid.add_value(contact_json['id'])
         vcafn = EBook.VCardAttribute.new(
             'social-networking-attributes', 'facebook-name')
         vcafn.add_value(contact_json['name'])
-        vcard = EBook.VCard.new()
+        vcauri = EBook.VCardAttribute.new(
+            'social-networking-attributes', 'X-URIS')
+        vcauri.add_value(contact_json['link'])
+
+        vcaws = EBook.VCardAttribute.new(
+            'social-networking-attributes', 'X-FOLKS-WEB-SERVICES-IDS')
+        vcaws_param = EBook.VCardAttributeParam.new('jabber')
+        vcaws_param.add_value('-{}@chat.facebook.com'.format(contact_json['id']))
+        vcaws.add_param(vcaws_param)
+        vcard.add_attribute(vcaws)
+
+        if 'gender' in contact_json.keys():
+            vcag = EBook.VCardAttribute.new(
+                'social-networking-attributes', 'X-GENDER')
+            vcag.add_value(contact_json['gender'])
+            vcard.add_attribute(vcag)
+
+        vcard.add_attribute(vcafn)
+        vcard.add_attribute(vcauri)
         vcard.add_attribute(vcafid)
-        vcard.add_attribute(vcafn)
-        c = EBook.Contact.new_from_vcard(vcard.to_string(EBook.VCardFormat(1)))
-        log.debug('Creating new contact for {}'.format(contact_json['name']))
-        return c
-
+
+        contact = EBook.Contact.new_from_vcard(
+            vcard.to_string(EBook.VCardFormat(1)))
+        contact.set_property('full-name', contact_json['name'])
+        if 'username' in contact_json.keys():
+            contact.set_property('nickname', contact_json['username'])
+
+        log.debug(
+            'Creating new contact for {}'.format(
+                contact.get_property('full-name')))
+        return contact
+
+    @feature
     def contacts(self):
-        contacts = self.fetch_contacts()
+        contacts = self._fetch_contacts()
+        log.debug('Size of the contacts returned {}'.format(len(contacts)))
         source = self._get_eds_source(FACEBOOK_ADDRESS_BOOK)
+        if source is None:
+            source = self._create_eds_source(FACEBOOK_ADDRESS_BOOK)
         for contact in contacts:
-            if source is not None:
-                if (Base.previously_stored_contact(
-                        source, 'facebook-id', contact['id'])):
-                    continue
-            # Let's not query the full contact info for now Show some
-            # respect for facebook and the chances are we won't be
-            # blocked.
-            eds_contact = self.create_contact(contact)
+            if self._previously_stored_contact(
+                    source, 'facebook-id', contact['id']):
+                continue
+            log.debug(
+                'Fetch full contact info for {} and id {}'.format(
+                    contact['name'], contact['id']))
+            full_contact = self._fetch_contact(contact['id'])
+            eds_contact = self._create_contact(full_contact)
             if not self._push_to_eds(FACEBOOK_ADDRESS_BOOK, eds_contact):
                 log.error(
-                    'Unable to save facebook contact {}'.format(contact['name']))
+                    'Unable to save facebook contact {}'.format(
+                        contact['name']))
+
+    def delete_contacts(self):
+        source = self._get_eds_source(FACEBOOK_ADDRESS_BOOK)
+        return self._delete_service_contacts(source)

=== modified file 'friends/testing/mocks.py'
--- friends/testing/mocks.py	2012-10-27 00:55:03 +0000
+++ friends/testing/mocks.py	2012-10-31 14:00:33 +0000
@@ -263,10 +263,11 @@
         return True
 
     def get_contacts_sync(val1, val2, val3):
-        if val1:
-            return [True, [{'name':'john doe', 'id': 11111}]]
-        else:
-            return [True, []]
+        return [True, [{'name':'john doe', 'id': 11111}]]
+    
+    def remove_contact_sync(val1, val2):
+        pass
+
 
 class EDSExtension:
     """A Extension mocker object for testing create source."""

=== modified file 'friends/tests/test_facebook.py'
--- friends/tests/test_facebook.py	2012-10-31 11:38:22 +0000
+++ friends/tests/test_facebook.py	2012-10-31 14:00:33 +0000
@@ -368,27 +368,38 @@
                 return_value=True)
     def test_fetch_contacts(self, *mocks):
         # Receive the users friends.
-        results = self.protocol.fetch_contacts()
+        results = self.protocol._fetch_contacts()
         self.assertEqual(len(results), 8)
         self.assertEqual(results[7]['name'], 'John Smith')
         self.assertEqual(results[7]['id'], '444444')
 
     def test_create_contact(self, *mocks):
         # Receive the users friends.
-        bare_contact = {'name': 'Lucy Baron', 'id': '555555555'}
-        eds_contact = self.protocol.create_contact(bare_contact)
+        bare_contact = {'name': 'Lucy Baron',
+                        'id': '555555555',
+                        'username': "lucy.baron5",
+                        'link': 'http:www.facebook.com/lucy.baron5'}
+        eds_contact = self.protocol._create_contact(bare_contact)
         facebook_id_attr = eds_contact.get_attribute('facebook-id')
         self.assertEqual(facebook_id_attr.get_value(), '555555555')
         facebook_name_attr = eds_contact.get_attribute('facebook-name')
         self.assertEqual(facebook_name_attr.get_value(), 'Lucy Baron')
+        web_service_addrs = eds_contact.get_attribute('X-FOLKS-WEB-SERVICES-IDS')
+        self.assertEqual(len(web_service_addrs.get_params()), 1)
+        self.assertEqual(web_service_addrs.get_params()[0].get_name(), "jabber")
+        self.assertEqual(len(web_service_addrs.get_params()[0].get_values()), 1)
+        self.assertEqual(web_service_addrs.get_params()[0].get_values()[0], "[e-mail]")
 
     @mock.patch('friends.utils.base.Base._get_eds_source',
                 return_value=True)
     @mock.patch('gi.repository.EBook.BookClient.new',
                 return_value=EDSBookClientMock())
     def test_successfull_push_to_eds(self, *mocks):
-        bare_contact = {'name': 'Lucy Baron', 'id': '555555555'}
-        eds_contact = self.protocol.create_contact(bare_contact)
+        bare_contact = {'name': 'Lucy Baron',
+                        'id': '555555555',
+                        'username': "lucy.baron5",
+                        'link': 'http:www.facebook.com/lucy.baron5'}
+        eds_contact = self.protocol._create_contact(bare_contact)
         result = self.protocol._push_to_eds('test-address-book', eds_contact)
         self.assertEqual(result, True)
 
@@ -397,22 +408,27 @@
     @mock.patch('friends.utils.base.Base._create_eds_source',
                 return_value=None)
     def test_unsuccessfull_push_to_eds(self, *mocks):
-        bare_contact = {'name': 'Lucy Baron', 'id': '555555555'}
-        eds_contact = self.protocol.create_contact(bare_contact)
+        bare_contact = {'name': 'Lucy Baron',
+                        'id': '555555555',
+                        'username': "lucy.baron5",
+                        'link': 'http:www.facebook.com/lucy.baron5'}
+        eds_contact = self.protocol._create_contact(bare_contact)
         result = self.protocol._push_to_eds('test-address-book', eds_contact)
         self.assertEqual(result, False)
 
     @mock.patch('gi.repository.EDataServer.Source.new',
                 return_value=EDSSource('foo', 'bar'))
     def test_create_eds_source(self, *mocks):
-        self.protocol._source_registry = mock.Mock()
+        regmock = self.protocol._source_registry = mock.Mock()
+        regmock.ref_source = lambda x: x
+
         result = self.protocol._create_eds_source('facebook-test-address')
         self.assertEqual(result, 'test-source-uid')
 
     @mock.patch('gi.repository.EBook.BookClient.new',
                 return_value=EDSBookClientMock())
     def test_successful_previously_stored_contact(self, *mocks):
-        result = Facebook.previously_stored_contact(
+        result = self.protocol._previously_stored_contact(
             True, 'facebook-id', '11111')
         self.assertEqual(result, True)
 
@@ -422,7 +438,7 @@
                 return 'test-facebook-contacts'
             def get_uid(self):
                 return 1345245
-
+                
         reg_mock = self.protocol._source_registry = mock.Mock()
         reg_mock.list_sources.return_value = [FakeSource()]
         reg_mock.ref_source = lambda x: x

=== modified file 'friends/utils/base.py'
--- friends/utils/base.py	2012-10-31 10:42:53 +0000
+++ friends/utils/base.py	2012-10-31 14:00:33 +0000
@@ -31,7 +31,7 @@
 
 from datetime import datetime
 
-from gi.repository import EDataServer, EBook, Gio
+from gi.repository import EDataServer, EBook
 
 from friends.utils.authentication import Authentication
 from friends.utils.model import COLUMN_INDICES, SCHEMA, DEFAULTS
@@ -94,16 +94,16 @@
     punctuation for the sake of comparing the strings.  For example:
 
     Fred uses Friends to post identical messages on Twitter and Google+
-    (pretend that we support G+ for a moment).  Fred writes "Hey jimbob, been
-    to http://example.com lately?", and this message might show up on Twitter
-    like "Hey @jimbob, been to example.com lately?", but it might show up on
-    G+ like "Hey +jimbob, been to http://example.com lately?".  So we need to
+    (pretend that we support G+ for a moment).  Fred writes 'Hey jimbob, been
+    to http://example.com lately?', and this message might show up on Twitter
+    like 'Hey @jimbob, been to example.com lately?', but it might show up on
+    G+ like 'Hey +jimbob, been to http://example.com lately?'.  So we need to
     strip out all the possibly different bits in order to identify that these
     messages are the same for our purposes.  In both of these cases, the
-    string is converted into "Heyjimbobbeentoexamplecomlately" and then they
+    string is converted into 'Heyjimbobbeentoexamplecomlately' and then they
     compare equally, so we've identified a duplicate message.
     """
-    # Given a "row" of data, the sender and message fields are concatenated
+    # Given a 'row' of data, the sender and message fields are concatenated
     # together to form the raw key.  Then we strip out details such as url
     # schemes, punctuation, and whitespace, that allow for the fuzzy matching.
     key = SCHEME_RE.sub('', row[SENDER_IDX] + row[MESSAGE_IDX])
@@ -357,23 +357,12 @@
     def _push_to_eds(self, online_service, contact):
         source_match = self._get_eds_source(online_service)
         if source_match is None:
-            new_source_uid = self._create_eds_source(online_service)
-            if new_source_uid is None:
-                log.error(
-                    'Could not create a new source for {}'.format(
-                        online_service))
-                return False
-            else:
-                # https://bugzilla.gnome.org/show_bug.cgi?id=685986
-                # Potential race condition - need to sleep for a
-                # couple of cycles to ensure the registry will return
-                # a valid source object after commiting Evolution fix
-                # on the way but for now we need this.
-                time.sleep(1)
-                source_match = self._source_registry.ref_source(new_source_uid)
+            log.error("Push to EDS failed because we have been handed an online" + 
+                      "service (%s) which does not have an address book", online_service)
+            return False
         client = EBook.BookClient.new(source_match)
         client.open_sync(False, None)
-        return client.add_contact_sync(contact, Gio.Cancellable())
+        return client.add_contact_sync(contact, None)
 
     def _create_eds_source(self, online_service):
         source = EDataServer.Source.new(None, None)
@@ -382,26 +371,48 @@
         extension = source.get_extension(
             EDataServer.SOURCE_EXTENSION_ADDRESS_BOOK)
         extension.set_backend_name('local')
-        if self._source_registry.commit_source_sync(source, Gio.Cancellable()):
-            return source.get_uid()
+        if self._source_registry.commit_source_sync(source, None):
+            # https://bugzilla.gnome.org/show_bug.cgi?id=685986
+            # Potential race condition - need to sleep for a
+            # couple of cycles to ensure the registry will return
+            # a valid source object after commiting. Evolution fix
+            # on the way but for now we need this.
+            time.sleep(2)
+            return self._source_registry.ref_source(source.get_uid())
 
     def _get_eds_source(self, online_service):
         for previous_source in self._source_registry.list_sources(None):
             if previous_source.get_display_name() == online_service:
                 return self._source_registry.ref_source(
                     previous_source.get_uid())
+        return None
 
-    @classmethod
-    def previously_stored_contact(cls, source, field, search_term):
+    def _previously_stored_contact(self, source, field, search_term):
         client = EBook.BookClient.new(source)
         client.open_sync(False, None)
         query = EBook.book_query_vcard_field_test(
             field, EBook.BookQueryTest(0), search_term)
-        cs = client.get_contacts_sync(query.to_string(), Gio.Cancellable())
-        log.debug('Search string: {}'.format(query.to_string()))
-        if not cs[0]:
-           return False # is this right ...
-        return len(cs[1]) > 0
+        success, result = client.get_contacts_sync(query.to_string(), None)
+        if not success:
+            log.error('EDS Search failed on field %s', field)
+            return False
+        return len(result) > 0
+
+    def _delete_service_contacts(self, source):
+        client = EBook.BookClient.new(source)
+        client.open_sync(False, None)
+        query = EBook.book_query_any_field_contains('')
+        success, results = client.get_contacts_sync(query.to_string(), None)
+        if not success:
+            log.error('EDS search for delete all contacts failed')
+            return False
+        log.debug('Found {} contacts to delete'.format(len(results)))
+        for contact in results:
+            log.debug(
+                'Deleting contact {}'.format(
+                    contact.get_property('full-name')))
+            client.remove_contact_sync(contact, None)
+        return True
 
     @classmethod
     def get_features(cls):