AP users: start migrating Follower from string domains/ids to User keys

for #512

activitypub.py

@@ -652,9 +652,8 @@ def follower_collection(protocol, domain, collection):
     followers, new_before, new_after = Follower.fetch_page(domain, collection)
     items = []
     for f in followers:
-        f_as2 = f.to_as2()
-        if f_as2:
-            items.append(f_as2)
+        if f.obj.as1:
+            items.append(as2.from_as1(f.obj.as1))
 
     page = {
         'type': 'CollectionPage',

follow.py

@@ -115,11 +115,11 @@ def finish(self, auth_entity, state=None):
             flash(f"Couldn't find ActivityPub profile link for {addr}")
             return redirect(g.user.user_page_path('following'))
 
-        # TODO: make this generic across protocols
-        followee = ActivityPub.load(as2_url).as2
-        id = followee.get('id')
-        inbox = followee.get('inbox')
-        if not id or not inbox:
+        # TODO(#512): generalize all this across protocols
+        followee = ActivityPub.load(as2_url)
+        followee_id = followee.as1.get('id')
+        inbox = followee.as2.get('inbox')
+        if not followee_id or not inbox:
             flash(f"AS2 profile {as2_url} missing id or inbox")
             return redirect(g.user.user_page_path('following'))
 
@@ -129,19 +129,21 @@ def finish(self, auth_entity, state=None):
             '@context': 'https://www.w3.org/ns/activitystreams',
             'type': 'Follow',
             'id': follow_id,
-            'object': followee,
+            'object': followee.as2,
             'actor': g.user.ap_actor(),
             'to': [as2.PUBLIC_AUDIENCE],
         }
-        obj = Object(id=follow_id, domains=[domain], labels=['user'],
-                     source_protocol='ui', status='complete', as2=follow_as2)
-        ActivityPub.send(obj, inbox)
+        follow_obj = Object(id=follow_id, domains=[domain], labels=['user'],
+                            source_protocol='ui', status='complete', as2=follow_as2)
+        ActivityPub.send(follow_obj, inbox)
 
-        Follower.get_or_create(dest=id, src=domain, status='active',
-                               last_follow=follow_as2)
-        obj.put()
+        followee_user = ActivityPub.get_or_create(followee_id, actor_as2=followee.as2)
+        Follower.get_or_create(from_=g.user, to=followee_user, status='active',
+                               follow=follow_obj.key)
+        follow_obj.put()
 
-        link = common.pretty_link(util.get_url(followee) or id, text=addr)
+        link = common.pretty_link(util.get_url(followee.as1) or followee_id,
+                                  text=addr)
         flash(f'Followed {link}.')
         return redirect(g.user.user_page_path('following'))
 

models.py

@@ -196,7 +196,7 @@ def name(self):
         return self.readable_or_key_id()
 
     def web_url(self):
-        """Returns this user's web URL aka web_url, eg 'https://foo.com/'.
+        """Returns this user's web URL (homepage), eg 'https://foo.com/'.
 
         To be implemented by subclasses.
 
@@ -206,7 +206,7 @@ def web_url(self):
         raise NotImplementedError()
 
     def is_web_url(self, url):
-        """Returns True if the given URL is this user's web URL (web_url).
+        """Returns True if the given URL is this user's web URL (homepage).
 
         Args:
           url: str
@@ -477,77 +477,82 @@ def create(data):
         return node
 
 
-class Follower(StringIdModel):
-    """A follower of a Bridgy Fed user.
-
-    Key name is 'TO FROM', where each part is either a domain or an AP id, eg:
-    'snarfed.org https://mastodon.social/@swentel'.
-
-    Both parts are duplicated in the src and dest properties.
-    """
+class Follower(ndb.Model):
+    """A follower of a Bridgy Fed user."""
     STATUSES = ('active', 'inactive')
 
-    src = ndb.StringProperty()
-    dest = ndb.StringProperty()
-    # Most recent AP (AS2) JSON Follow activity. If inbound, must have a
-    # composite actor object with an inbox, publicInbox, or sharedInbox.
-    last_follow = JsonProperty()
+    # these are both subclasses of User
+    from_ = ndb.KeyProperty(name='from')
+    to = ndb.KeyProperty()
+
+    follow = ndb.KeyProperty(Object)  # last follow activity
     status = ndb.StringProperty(choices=STATUSES, default='active')
 
     created = ndb.DateTimeProperty(auto_now_add=True)
     updated = ndb.DateTimeProperty(auto_now=True)
 
+    # DEPRECATED
+    src = ndb.StringProperty()
+    dest = ndb.StringProperty()
+    last_follow = JsonProperty()
+
     def _post_put_hook(self, future):
-        logger.info(f'Wrote Follower {self.key.id()} {self.status}')
+        logger.info(f'Wrote {self}')
 
     @classmethod
-    def _id(cls, dest, src):
-        assert src
-        assert dest
-        return f'{dest} {src}'
+    @ndb.transactional()
+    def get_or_create(cls, *, from_, to, **kwargs):
+        """Returns a Follower with the given from_ and to users.
 
-    @classmethod
-    def get_or_create(cls, dest, src, **kwargs):
-        follower = cls.get_or_insert(cls._id(dest, src), src=src, dest=dest, **kwargs)
-        follower.dest = dest
-        follower.src = src
-        for prop, val in kwargs.items():
-            setattr(follower, prop, val)
-        follower.put()
-        return follower
+        If a matching Follower doesn't exist in the datastore, creates it first.
 
-    def to_as1(self):
-        """Returns this follower as an AS1 actor dict, if possible."""
-        return as2.to_as1(self.to_as2())
+        Args:
+          from_: :class:`User`
+          to: :class:`User`
 
-    def to_as2(self):
-        """Returns this follower as an AS2 actor dict, if possible."""
-        if self.last_follow:
-            return self.last_follow.get('actor' if util.is_web(self.src) else 'object')
+        Returns:
+          :class:`Follower`
+        """
+        assert from_
+        assert to
+
+        follower = Follower.query(Follower.from_ == from_.key,
+                                  Follower.to == to.key,
+                                  ).get()
+        if not follower:
+            follower = Follower(from_=from_.key, to=to.key, **kwargs)
+            follower.put()
+        elif kwargs:
+            # update existing entity with new property values, eg to make an
+            # inactive Follower active again
+            for prop, val in kwargs.items():
+                setattr(follower, prop, val)
+            follower.put()
+
+        return follower
 
     @staticmethod
-    def fetch_page(domain, collection):
-        """Fetches a page of Follower entities.
+    def fetch_page(collection):
+        """Fetches a page of Follower entities for the current user.
 
         Wraps :func:`fetch_page`. Paging uses the `before` and `after` query
         parameters, if available in the request.
 
         Args:
-          domain: str, user to fetch entities for
           collection, str, 'followers' or 'following'
 
         Returns:
           (results, new_before, new_after) tuple with:
-          results: list of Follower entities
+          results: list of :class:`Follower` entities
           new_before, new_after: str query param values for `before` and `after`
             to fetch the previous and next pages, respectively
         """
         assert collection in ('followers', 'following'), collection
 
-        domain_prop = Follower.dest if collection == 'followers' else Follower.src
+        prop = Follower.to if collection == 'followers' else Follower.from_
         query = Follower.query(
             Follower.status == 'active',
-            domain_prop == domain,
+            prop == g.user.key,
         ).order(-Follower.updated)
         return fetch_page(query, Follower)
 

pages.py

@@ -7,6 +7,7 @@
 import urllib.parse
 
 from flask import g, redirect, render_template, request
+from google.cloud.ndb.model import get_multi
 from google.cloud.ndb.stats import KindStat
 from granary import as1, as2, atom, microformats2, rss
 import humanize
@@ -118,15 +119,18 @@ def user(protocol, id):
 def followers_or_following(protocol, id, collection):
     load_user(protocol, id)
 
-    followers, before, after = Follower.fetch_page(id, collection)
+    followers, before, after = Follower.fetch_page(collection)
+    users = {
+        u.key: u for u in get_multi(f.from_ if collection == 'followers' else f.to
+                                    for f in followers)
+    }
 
     for f in followers:
-        f.url = f.src if collection == 'followers' else f.dest
-        person = f.to_as1()
-        f.handle = as2.address(as2.from_as1(person) or f.url) or f.url
-        if person and isinstance(person, dict):
-            f.name = person.get('name') or ''
-            f.picture = util.get_url(person, 'icon') or util.get_url(person, 'image')
+        user = users[f.from_ if collection == 'followers' else f.to]
+        f.url = user.web_url()
+        f.as1 = as2.to_as1(user.actor_as2)
+        f.handle = as2.address(user.actor_as2 or f.url) or f.url
+        f.picture = util.get_url(f.as1, 'icon') or util.get_url(f.as1, 'image')
 
     return render_template(
         f'{collection}.html',

templates/_followers.html

@@ -7,7 +7,7 @@
     {% if f.picture %}
       <img class="profile u-photo" src="{{ f.picture }}" width="48px">
     {% endif %}
-    {{ f.name or '' }}
+    {{ f.as1.get('displayName') or '' }}
     {{ f.handle }}
   </a>
   {% if page_name == 'following' %}

tests/test_follow.py

@@ -11,8 +11,9 @@
 from oauth_dropins.webutil.util import json_dumps, json_loads
 
 # import first so that Fake is defined before URL routes are registered
-from . import testutil
+from .testutil import Fake, TestCase
 
+from activitypub import ActivityPub
 import common
 from common import redirect_unwrap
 from models import Follower, Object, User
@@ -58,7 +59,7 @@
 
 
 @patch('requests.get')
-class RemoteFollowTest(testutil.TestCase):
+class RemoteFollowTest(TestCase):
 
     def setUp(self):
         super().setUp()
@@ -132,7 +133,7 @@ def test_webfinger_returns_not_json(self, mock_get):
 
 @patch('requests.post')
 @patch('requests.get')
-class FollowTest(testutil.TestCase):
+class FollowTest(TestCase):
 
     def setUp(self):
         super().setUp()
@@ -206,15 +207,16 @@ def check(self, input, resp, expected_follow, mock_get, mock_post):
         self.assertTrue(sig_template.startswith('keyId="http://localhost/alice.com"'),
                         sig_template)
 
+        follow_id = f'http://localhost/web/alice.com/following#2022-01-02T03:04:05-{input}'
+
         followers = Follower.query().fetch()
         self.assert_entities_equal(
-            Follower(id='https://bar/id alice.com', last_follow=expected_follow,
-                     src='alice.com', dest='https://bar/id', status='active'),
+            Follower(from_=self.user.key, to=ActivityPub(id='https://bar/id').key,
+                     follow=Object(id=follow_id).key, status='active'),
             followers,
             ignore=['created', 'updated'])
 
-        id = f'http://localhost/web/alice.com/following#2022-01-02T03:04:05-{input}'
-        self.assert_object(id, domains=['alice.com'], status='complete',
+        self.assert_object(follow_id, domains=['alice.com'], status='complete',
                            labels=['user', 'activity'], source_protocol='ui',
                            as2=expected_follow, as1=as2.to_as1(expected_follow))
 
@@ -256,17 +258,18 @@ def test_callback_user_use_instead(self, mock_get, mock_post):
             'object': FOLLOWEE,
             'to': [as2.PUBLIC_AUDIENCE],
         }
+        follow_obj = self.assert_object(
+            id, domains=['www.alice.com'], status='complete',
+            labels=['user', 'activity'], source_protocol='ui', as2=expected_follow,
+            as1=as2.to_as1(expected_follow))
+
         followers = Follower.query().fetch()
         self.assert_entities_equal(
-            Follower(id='https://bar/id www.alice.com', last_follow=expected_follow,
-                     src='www.alice.com', dest='https://bar/id', status='active'),
+            Follower(from_=user.key, to=ActivityPub(id='https://bar/id').key,
+                     follow=follow_obj.key, status='active'),
             followers,
             ignore=['created', 'updated'])
 
-        self.assert_object(id, domains=['www.alice.com'], status='complete',
-                           labels=['user', 'activity'], source_protocol='ui',
-                           as2=expected_follow, as1=as2.to_as1(expected_follow))
-
     def test_indieauthed_session(self, mock_get, mock_post):
         mock_get.side_effect = (
             self.as2_resp(FOLLOWEE),
@@ -303,7 +306,7 @@ def test_indieauthed_session_wrong_me(self, mock_get, mock_post):
 
 @patch('requests.post')
 @patch('requests.get')
-class UnfollowTest(testutil.TestCase):
+class UnfollowTest(TestCase):
 
     def setUp(self):
         super().setUp()

tests/test_models.py

@@ -213,22 +213,29 @@ class FollowerTest(TestCase):
 
     def setUp(self):
         super().setUp()
-        self.inbound = Follower(dest='user.com', src='http://mas.to/@baz',
-                                last_follow={'actor': ACTOR})
-        self.outbound = Follower(dest='http://mas.to/@baz', src='user.com',
-                                 last_follow={'object': ACTOR})
-
-    def test_to_as1(self):
-        self.assertEqual({}, Follower().to_as1())
-
-        as1_actor = as2.to_as1(ACTOR)
-        self.assertEqual(as1_actor, self.inbound.to_as1())
-        self.assertEqual(as1_actor, self.outbound.to_as1())
-
-    def test_to_as2(self):
-        self.assertIsNone(Follower().to_as2())
-        self.assertEqual(ACTOR, self.inbound.to_as2())
-        self.assertEqual(ACTOR, self.outbound.to_as2())
+        g.user = self.make_user('foo', cls=Fake)
+        self.other_user = self.make_user('bar', cls=Fake)
+
+    def test_get_or_create(self):
+        follower = Follower.get_or_create(from_=g.user, to=self.other_user)
+
+        self.assertEqual(g.user.key, follower.from_)
+        self.assertEqual(self.other_user.key, follower.to)
+        self.assertEqual(1, Follower.query().count())
+
+        follower2 = Follower.get_or_create(from_=g.user, to=self.other_user)
+        self.assert_entities_equal(follower, follower2)
+        self.assertEqual(1, Follower.query().count())
+
+        Follower.get_or_create(to=g.user, from_=self.other_user)
+        Follower.get_or_create(from_=g.user, to=self.make_user('baz', cls=Fake))
+        self.assertEqual(3, Follower.query().count())
+
+        # check that kwargs get set on existing entity
+        follower = Follower.get_or_create(from_=g.user, to=self.other_user,
+                                          status='inactive')
+        got = follower.key.get()
+        self.assertEqual('inactive', got.status)
 
 
 class AtpNodeTest(TestCase):