replace User.readable_id computed property with handle

activitypub.py

@@ -8,6 +8,7 @@ from urllib.parse import quote_plus, urljoin
 
 from flask import abort, g, request
 from google.cloud import ndb
+from google.cloud.ndb.query import OR
 from granary import as1, as2
 from httpsig import HeaderVerifier
 from httpsig.requests_auth import HTTPSignatureAuth
@@ -85,6 +86,7 @@ class ActivityPub(User, Protocol):
 
         return self.ap_actor()
 
+    @ndb.ComputedProperty
     def handle(self):
         """Returns this user's ActivityPub address, eg ``@user@foo.com``."""
         if self.obj and self.obj.as1:
@@ -94,7 +96,8 @@ class ActivityPub(User, Protocol):
 
         return as2.address(self.key.id())
 
-    ap_address = handle
+    def ap_address(self):
+        return self.handle
 
     def ap_actor(self, rest=None):
         """Returns this user's actor id URL, eg ``https://foo.com/@user``."""
@@ -136,7 +139,9 @@ class ActivityPub(User, Protocol):
         if not handle.startswith('@'):
             handle = '@' + handle
 
-        user = ActivityPub.query(ActivityPub.readable_id == handle).get()
+        user = ActivityPub.query(OR(ActivityPub.handle == handle,
+                                    ActivityPub.readable_id == handle),
+                                 ).get()
         if user:
             return user.key.id()
 

atproto.py

@@ -66,6 +66,7 @@ class ATProto(User, Protocol):
         assert not self.atproto_did, \
             f"{self.key} shouldn't have atproto_did {self.atproto_did}"
 
+    @ndb.ComputedProperty
     def handle(self):
         """Returns handle if the DID document includes one, otherwise None."""
         did_obj = ATProto.load(self.key.id(), remote=False)
@@ -94,7 +95,7 @@ class ATProto(User, Protocol):
     def handle_to_id(cls, handle):
         assert cls.owns_handle(handle) is not False
 
-        user = ATProto.query(ATProto.readable_id == handle).get()
+        user = ATProto.query(ATProto.handle == handle).get()
         if user:
             return user.key.id()
 

docs/index.rst

@@ -105,7 +105,7 @@ How to add a new protocol
    and
    `User <https://github.com/snarfed/bridgy-fed/blob/main/models.py>`__.
    Implement the ``send``, ``fetch``, ``serve``, and ``target_for``
-   methods from ``Protocol`` and ``readable_id``, ``web_url``,
+   methods from ``Protocol`` and ``handle``, ``web_url``,
    ``ap_address``, and ``ap_actor`` from ``User`` .
 4. TODO: add a new usage section to the docs for the new protocol.
 5. TODO: does the new protocol need any new UI or signup functionality?

models.py

@@ -286,20 +286,18 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
         return self.obj.as_as2() if self.obj else {}
 
     @ndb.ComputedProperty
-    def readable_id(self):
-        """This user's human-readable unique id, eg ``@me@snarfed.org``.
-
-        TODO: rename to handle! And keep readable_id in queries for backcompat
-        """
-        return self.handle()
-
     def handle(self):
-        """Returns this user's handle, eg ``@me@snarfed.org``.
+        """This user's unique, human-chosen handle, eg ``@me@snarfed.org``.
 
         To be implemented by subclasses.
         """
         raise NotImplementedError()
 
+    @ndb.ComputedProperty
+    def readable_id(self):
+        """DEPRECATED: replaced by handle. Kept for backward compatibility."""
+        return None
+
     def handle_as(self, to_proto):
         """Returns this user's handle in a different protocol.
 
@@ -312,7 +310,7 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
         if isinstance(to_proto, str):
             to_proto = PROTOCOLS[to_proto]
 
-        return ids.convert_handle(handle=self.handle(), from_proto=self.__class__,
+        return ids.convert_handle(handle=self.handle, from_proto=self.__class__,
                                   to_proto=to_proto)
 
     def id_as(self, to_proto):
@@ -332,7 +330,7 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
 
     def handle_or_id(self):
         """Returns handle if we know it, otherwise id."""
-        return self.handle() or self.key.id()
+        return self.handle or self.key.id()
 
     def href(self):
         return f'data:application/magic-public-key,RSA.{self.mod}.{self.public_exponent}'
@@ -543,7 +541,7 @@ class Object(StringIdModel):
         elif self.bsky:
             owner, _, _ = arroba.util.parse_at_uri(self.key.id())
             ATProto = PROTOCOLS['atproto']
-            handle = ATProto(id=owner).handle()
+            handle = ATProto(id=owner).handle
             obj = bluesky.to_as1(self.bsky, repo_did=owner, repo_handle=handle,
                                  pds=ATProto.target_for(self))
 

pages.py

@@ -46,11 +46,13 @@ def load_user(protocol, id):
 
     if protocol != 'web':
         if not g.user:
-            g.user = cls.query(cls.readable_id == id).get()
+            g.user = cls.query(OR(cls.handle == id,
+                                  cls.readable_id == id),
+                               ).get()
             if g.user and g.user.use_instead:
                 g.user = g.user.use_instead.get()
 
-        if g.user and id not in (g.user.key.id(), g.user.handle()):
+        if g.user and id not in (g.user.key.id(), g.user.handle):
             error('', status=302, location=g.user.user_page_path())
 
     elif g.user and id != g.user.key.id():  # use_instead redirect

tests/test_activitypub.py

@@ -1926,15 +1926,15 @@ class ActivityPubUtilsTest(TestCase):
             'preferredUsername': 'me',
         }))
         self.assertEqual('@me@mas.to', user.ap_address())
-        self.assertEqual('@me@mas.to', user.readable_id)
+        self.assertEqual('@me@mas.to', user.handle)
 
         user.obj.as2 = ACTOR
         self.assertEqual('@swentel@mas.to', user.ap_address())
-        self.assertEqual('@swentel@mas.to', user.readable_id)
+        self.assertEqual('@swentel@mas.to', user.handle)
 
         user = ActivityPub(id='https://mas.to/users/alice')
         self.assertEqual('@alice@mas.to', user.ap_address())
-        self.assertEqual('@alice@mas.to', user.readable_id)
+        self.assertEqual('@alice@mas.to', user.handle)
 
     def test_ap_actor(self):
         user = self.make_user('http://foo/actor', cls=ActivityPub)
@@ -1957,13 +1957,13 @@ class ActivityPubUtilsTest(TestCase):
         user.obj.as2['url'] = ['http://my/url']
         self.assertEqual('http://my/url', user.web_url())
 
-    def test_readable_id(self):
+    def test_handle(self):
         user = self.make_user('http://foo', cls=ActivityPub)
-        self.assertIsNone(user.readable_id)
+        self.assertIsNone(user.handle)
         self.assertEqual('http://foo', user.handle_or_id())
 
         user.obj = Object(id='a', as2=ACTOR)
-        self.assertEqual('@swentel@mas.to', user.readable_id)
+        self.assertEqual('@swentel@mas.to', user.handle)
         self.assertEqual('@swentel@mas.to', user.handle_or_id())
 
     @skip

tests/test_atproto.py

@@ -228,11 +228,11 @@ class ATProtoTest(TestCase):
     @patch('requests.get', return_value=requests_response('', status=404))
     def test_handle_or_id(self, mock_get):
         user = self.make_user('did:plc:foo', cls=ATProto)
-        self.assertIsNone(user.handle())
+        self.assertIsNone(user.handle)
         self.assertEqual('did:plc:foo', user.handle_or_id())
 
         self.store_object(id='did:plc:foo', raw=DID_DOC)
-        self.assertEqual('han.dull', user.handle())
+        self.assertEqual('han.dull', user.handle)
         self.assertEqual('han.dull', user.handle_or_id())
 
     def test_ap_address(self):

tests/test_models.py

@@ -113,8 +113,8 @@ class UserTest(TestCase):
         g.user.obj = Object(id='a', as2={'name': 'alice'})
         self.assertEqual('alice', g.user.name())
 
-    def test_readable_id(self):
-        self.assertEqual('y.z', g.user.readable_id)
+    def test_handle(self):
+        self.assertEqual('y.z', g.user.handle)
 
     def test_as2(self):
         self.assertEqual({}, g.user.as2())

tests/test_pages.py

@@ -40,7 +40,7 @@ class PagesTest(TestCase):
         got = self.client.get('/fa/foo.com')
         self.assert_equals(200, got.status_code)
 
-    def test_user_readable_id_activitypub_address(self):
+    def test_user_page_handle(self):
         user = self.make_user('http://foo', cls=ActivityPub,
                               obj_as2=ACTOR_WITH_PREFERRED_USERNAME)
         self.assertEqual('@me@plus.google.com', user.ap_address())

tests/test_web.py

@@ -1892,7 +1892,7 @@ class WebUtilTest(TestCase):
                 self.assertIsNone(Web.key_for(bad))
 
     def test_handle(self, *_):
-        self.assertEqual('user.com', g.user.handle())
+        self.assertEqual('user.com', g.user.handle)
 
     def test_owns_id(self, *_):
         self.assertIsNone(Web.owns_id('http://foo.com'))

tests/testutil.py

@@ -70,6 +70,7 @@ class Fake(User, protocol.Protocol):
     # in-order list of ids
     fetched = []
 
+    @ndb.ComputedProperty
     def handle(self):
         return self.key.id().replace('fake:', 'fake:handle:')
 
@@ -435,8 +436,8 @@ class TestCase(unittest.TestCase, testutil.Asserts):
             self.assert_equals(obj_as2, got.as2())
 
         # generated, computed, etc
-        ignore = ['created', 'mod', 'obj_key', 'private_exponent',
-                  'public_exponent', 'readable_id', 'updated']
+        ignore = ['created', 'mod', 'handle', 'obj_key', 'private_exponent',
+                  'public_exponent', 'updated']
         for prop in ignore:
             assert prop not in props
 

web.py

@@ -81,7 +81,7 @@ class Web(User, Protocol):
         """Validate domain id, don't allow upper case or invalid characters."""
         super()._pre_put_hook()
         id = self.key.id()
-        assert is_valid_domain(id)
+        assert is_valid_domain(id), id
         assert id.lower() == id, f'upper case is not allowed in Web key id: {id}'
 
     @classmethod
@@ -93,6 +93,7 @@ class Web(User, Protocol):
         """
         return super().get_or_create(id.lower().strip('.'), **kwargs)
 
+    @ndb.ComputedProperty
     def handle(self):
         """Returns this user's chosen username or domain, eg ``user.com``."""
         # prettify if domain, noop if username