standardize more on 'handle' and 'id' terms

* User.readable_id => handle
* User.readable_or_key_id => handle_or_id

README.md

@@ -58,8 +58,9 @@ How to add a new protocol
 ---
 
 1. Determine [how you'll map the new protocol to other existing Bridgy Fed protocols](https://fed.brid.gy/docs#translate), specifically identity, protocol inference, events, and operations. [Add those to the existing tables in the docs](https://github.com/snarfed/bridgy-fed/blob/main/templates/docs.html) in a PR. This is an important step before you start writing code.
-1. If the new protocol uses a new data format - which is likely - add that format to [granary](https://github.com/snarfed/granary) in a new file with functions that convert to/from [ActivityStreams 1](https://activitystrea.ms/specs/json/1.0/) and tests. See [`nostr.py`](https://github.com/snarfed/granary/blob/main/granary/nostr.py#L542) and [`test_nostr.py`](https://github.com/snarfed/granary/blob/main/granary/tests/test_nostr.py#) for examples.
-1. Implement the protocol in a new `.py` file as a subclass of both [`Protocol`](https://github.com/snarfed/bridgy-fed/blob/main/protocol.py) 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`, `ap_address`, and `ap_actor` from `User` .
+1. Implement the id and handle conversions in [`ids.py`](https://github.com/snarfed/bridgy-fed/blob/main/ids.py).
+1. If the new protocol uses a new data format - which is likely - add that format to [granary](https://github.com/snarfed/granary) in a new file with functions that convert to/from [ActivityStreams 1](https://activitystrea.ms/specs/json/1.0/) and tests. See [`nostr.py`](https://github.com/snarfed/granary/blob/main/granary/nostr.py) and [`test_nostr.py`](https://github.com/snarfed/granary/blob/main/granary/tests/test_nostr.py) for examples.
+1. Implement the protocol in a new `.py` file as a subclass of both [`Protocol`](https://github.com/snarfed/bridgy-fed/blob/main/protocol.py) and [`User`](https://github.com/snarfed/bridgy-fed/blob/main/models.py). Implement the `send`, `fetch`, `serve`, and `target_for` methods from `Protocol` and `handle` and `web_url` from `User` .
 1. TODO: add a new usage section to the docs for the new protocol.
 1. TODO: does the new protocol need any new UI or signup functionality? Unusual, but not impossible. Add that if necessary.
 1. Add the new protocol's logo to `static/`, use it in [`templates/user.html`](https://github.com/snarfed/bridgy-fed/blob/main/templates/user.html).

activitypub.py

@@ -62,11 +62,6 @@ class ActivityPub(User, Protocol):
     """
     ABBREV = 'ap'
 
-    @ndb.ComputedProperty
-    def readable_id(self):
-        """Returns fediverse handle ie WebFinger address, eg '@me@snarfed.org'."""
-        return self.ap_address()
-
     def _pre_put_hook(self):
         """Validate id, require URL, don't allow Bridgy Fed domains.
 
@@ -106,7 +101,7 @@ class ActivityPub(User, Protocol):
 
         Eg 'https://foo.com/@user'
         """
-        return self.key.id()
+        return self.key.id() + (f'/{rest}' if rest else '')
 
     @classmethod
     def owns_id(cls, id):

atproto.py

@@ -45,11 +45,6 @@ class ATProto(User, Protocol):
     """
     ABBREV = 'atproto'
 
-    @ndb.ComputedProperty
-    def readable_id(self):
-        """Prefers handle, then DID."""
-        return self.handle() or self.key.id()
-
     def _pre_put_hook(self):
         """Validate id, require did:plc or non-blocklisted did:web.
 
@@ -81,11 +76,11 @@ class ATProto(User, Protocol):
                 return handle
 
     def web_url(self):
-        return bluesky.Bluesky.user_url(self.readable_id)
+        return bluesky.Bluesky.user_url(self.handle_or_id())
 
     def ap_address(self):
         """Returns this user's AP address, eg '@handle.com@bsky.brid.gy'."""
-        return f'@{self.readable_id}@{self.ABBREV}{common.SUPERDOMAIN}'
+        return f'@{self.handle_or_id()}@{self.ABBREV}{common.SUPERDOMAIN}'
 
     @classmethod
     def owns_id(cls, id):

models.py

@@ -290,10 +290,8 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
         """This user's human-readable unique id, eg ``@me@snarfed.org``.
 
         TODO: rename to handle! And keep readable_id in queries for backcompat
-
-        To be implemented by subclasses.
         """
-        return None
+        return self.handle()
 
     def handle(self):
         """Returns this user's handle, eg ``@me@snarfed.org``.
@@ -332,9 +330,9 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
         return ids.convert_id(id=self.key.id(), from_proto=self.__class__,
                                   to_proto=to_proto)
 
-    def readable_or_key_id(self):
-        """Returns readable_id if set, otherwise key id."""
-        return self.readable_id or self.key.id()
+    def handle_or_id(self):
+        """Returns handle if we know it, otherwise id."""
+        return self.handle() or self.key.id()
 
     def href(self):
         return f'data:application/magic-public-key,RSA.{self.mod}.{self.public_exponent}'
@@ -360,7 +358,7 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
             if name:
                 return name
 
-        return self.readable_or_key_id()
+        return self.handle_or_id()
 
     def web_url(self):
         """Returns this user's web URL (homepage), eg 'https://foo.com/'.
@@ -446,7 +444,7 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
 
     def user_page_path(self, rest=None):
         """Returns the user's Bridgy Fed user page path."""
-        path = f'/{self.ABBREV}/{self.readable_or_key_id()}'
+        path = f'/{self.ABBREV}/{self.handle_or_id()}'
 
         if rest:
             if not rest.startswith('?'):

pages.py

@@ -50,7 +50,7 @@ def load_user(protocol, id):
             if g.user and g.user.use_instead:
                 g.user = g.user.use_instead.get()
 
-        if g.user and id != g.user.readable_or_key_id():
+        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

protocol.py

@@ -655,8 +655,7 @@ class Protocol:
 
             # send accept. note that this is one accept for the whole follow, even
             # if it has multiple followees!
-            id = common.host_url(to_user.user_page_path(
-                f'followers#accept-{obj.key.id()}'))
+            id = to_user.ap_actor(f'followers#accept-{obj.key.id()}')
             accept = Object.get_or_create(id, our_as1={
                 'id': id,
                 'objectType': 'activity',

templates/activities.html

@@ -17,7 +17,7 @@
       {% if loop.index0 == 3 %}
         <span id="more-users" style="display: none">
       {% endif %}
-      <a href="{{ user.user_page_path() }}">🌐 {{ user.readable_or_key_id() }}</a>
+      <a href="{{ user.user_page_path() }}">🌐 {{ user.handle_or_id() }}</a>
       <br>
     {% endfor %}
     {% if obj.users|length > 3 %}

templates/feed.html

@@ -1,6 +1,6 @@
 {% extends "base.html" %}
 
-{% block title %}{{ g.user.readable_or_key_id() }}'s feed - Bridgy Fed{% endblock %}
+{% block title %}{{ g.user.handle_or_id() }}'s feed - Bridgy Fed{% endblock %}
 
 {% block content %}
 

templates/followers.html

@@ -1,6 +1,6 @@
 {% extends "base.html" %}
 
-{% block title %}{{ g.user.readable_or_key_id() }}'s followers - Bridgy Fed{% endblock %}
+{% block title %}{{ g.user.handle_or_id() }}'s followers - Bridgy Fed{% endblock %}
 
 {% block content %}
 

templates/following.html

@@ -1,6 +1,6 @@
 {% extends "base.html" %}
 
-{% block title %}{{ g.user.readable_or_key_id() }}'s following - Bridgy Fed{% endblock %}
+{% block title %}{{ g.user.handle_or_id() }}'s following - Bridgy Fed{% endblock %}
 
 {% block content %}
 

tests/test_activitypub.py

@@ -76,14 +76,14 @@ ACTOR_BASE_FULL = {
 ACTOR_FAKE = {
     '@context': ['https://w3id.org/security/v1'],
     'type': 'Person',
-    'id': 'http://bf/fake/fake:user/ap',
+    'id': 'http://localhost/ap/fa/fake:user',
     'preferredUsername': 'fake:user',
     'url': 'http://localhost/r/fake:user',
     'summary': '',
-    'inbox': 'http://bf/fake/fake:user/ap/inbox',
-    'outbox': 'http://bf/fake/fake:user/ap/outbox',
-    'following': 'http://bf/fake/fake:user/ap/following',
-    'followers': 'http://bf/fake/fake:user/ap/followers',
+    'inbox': 'http://localhost/ap/fa/fake:user/inbox',
+    'outbox': 'http://localhost/ap/fa/fake:user/outbox',
+    'following': 'http://localhost/ap/fa/fake:user/following',
+    'followers': 'http://localhost/ap/fa/fake:user/followers',
     'endpoints': {'sharedInbox': 'http://localhost/ap/sharedInbox'},
     'publicKey': {
         'id': 'http://localhost/fake#key',
@@ -218,7 +218,7 @@ ACCEPT_FOLLOW['object'] = 'http://localhost/user.com'
 ACCEPT = {
     '@context': 'https://www.w3.org/ns/activitystreams',
     'type': 'Accept',
-    'id': 'http://localhost/web/user.com/followers#accept-https://mas.to/6d1a',
+    'id': 'http://localhost/user.com/followers#accept-https://mas.to/6d1a',
     'actor': 'http://localhost/user.com',
     'object': {
         **ACCEPT_FOLLOW,
@@ -1593,21 +1593,21 @@ class ActivityPubUtilsTest(TestCase):
                 'id': 'baj',
                 'preferredUsername': 'site',
                 'url': 'http://localhost/r/site',
-                'inbox': 'http://bf/fake/site/ap/inbox',
-                'outbox': 'http://bf/fake/site/ap/outbox',
+                'inbox': 'http://localhost/ap/fa/site/inbox',
+                'outbox': 'http://localhost/ap/fa/site/outbox',
             },
             'attributedTo': [{
                 'id': 'bar',
                 'preferredUsername': 'site',
                 'url': 'http://localhost/r/site',
-                'inbox': 'http://bf/fake/site/ap/inbox',
-                'outbox': 'http://bf/fake/site/ap/outbox',
+                'inbox': 'http://localhost/ap/fa/site/inbox',
+                'outbox': 'http://localhost/ap/fa/site/outbox',
             }, {
                 'id': 'baz',
                 'preferredUsername': 'site',
                 'url': 'http://localhost/r/site',
-                'inbox': 'http://bf/fake/site/ap/inbox',
-                'outbox': 'http://bf/fake/site/ap/outbox',
+                'inbox': 'http://localhost/ap/fa/site/inbox',
+                'outbox': 'http://localhost/ap/fa/site/outbox',
             }],
             'to': [as2.PUBLIC_AUDIENCE],
         }, postprocess_as2({
@@ -1960,11 +1960,11 @@ class ActivityPubUtilsTest(TestCase):
     def test_readable_id(self):
         user = self.make_user('http://foo', cls=ActivityPub)
         self.assertIsNone(user.readable_id)
-        self.assertEqual('http://foo', user.readable_or_key_id())
+        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.readable_or_key_id())
+        self.assertEqual('@swentel@mas.to', user.handle_or_id())
 
     @skip
     def test_target_for_not_activitypub(self):

tests/test_atproto.py

@@ -226,14 +226,14 @@ class ATProtoTest(TestCase):
         self.assertEqual('https://bsky.app/profile/han.dull', user.web_url())
 
     @patch('requests.get', return_value=requests_response('', status=404))
-    def test_handle_and_readable_id(self, mock_get):
+    def test_handle_or_id(self, mock_get):
         user = self.make_user('did:plc:foo', cls=ATProto)
         self.assertIsNone(user.handle())
-        self.assertEqual('did:plc:foo', user.readable_id)
+        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.readable_id)
+        self.assertEqual('han.dull', user.handle_or_id())
 
     def test_ap_address(self):
         user = self.make_user('did:plc:foo', cls=ATProto)

tests/test_models.py

@@ -28,7 +28,7 @@ class UserTest(TestCase):
 
     def setUp(self):
         super().setUp()
-        g.user = self.make_user('y.z')
+        g.user = self.make_user('y.z', cls=Web)
 
     def test_get_or_create(self):
         user = Fake.get_or_create('a.b')
@@ -114,7 +114,7 @@ class UserTest(TestCase):
         self.assertEqual('alice', g.user.name())
 
     def test_readable_id(self):
-        self.assertIsNone(g.user.readable_id)
+        self.assertEqual('y.z', g.user.readable_id)
 
     def test_as2(self):
         self.assertEqual({}, g.user.as2())

tests/test_protocol.py

@@ -1031,7 +1031,7 @@ class ProtocolReceiveTest(TestCase):
                                         delivered=['fake:user:target'],
                                         )
 
-        accept_id = 'http://localhost/fa/fake:user/followers#accept-fake:follow'
+        accept_id = 'http://localhost/ap/fa/fake:user/followers#accept-fake:follow'
         accept_as1 = {
             'id': accept_id,
             'objectType': 'activity',
@@ -1233,10 +1233,10 @@ class ProtocolReceiveTest(TestCase):
             Fake.receive_as1(follow_as1)
 
         (bob_obj, bob_target), (eve_obj, eve_target) = Fake.sent
-        self.assertEqual('http://localhost/fa/http://x.com/bob/followers#accept-http://x.com/follow',
+        self.assertEqual('http://localhost/ap/fa/http://x.com/bob/followers#accept-http://x.com/follow',
                          bob_obj.key.id())
         self.assertEqual('http://x.com/alice:target', bob_target)
-        self.assertEqual('http://localhost/fa/http://x.com/eve/followers#accept-http://x.com/follow',
+        self.assertEqual('http://localhost/ap/fa/http://x.com/eve/followers#accept-http://x.com/follow',
                          eve_obj.key.id())
         self.assertEqual('http://x.com/alice:target', eve_target)
 

tests/test_webfinger.py

@@ -90,11 +90,11 @@ WEBFINGER_FAKE = {
     }, {
         'rel': 'self',
         'type': 'application/activity+json',
-        'href': 'http://bf/fake/fake:user/ap',
+        'href': 'http://localhost/ap/fa/fake:user',
     }, {
         'rel': 'inbox',
         'type': 'application/activity+json',
-        'href': 'http://bf/fake/fake:user/ap/inbox',
+        'href': 'http://localhost/ap/fa/fake:user/inbox',
     }, {
         'rel': 'sharedInbox',
         'type': 'application/activity+json',
@@ -105,7 +105,9 @@ WEBFINGER_FAKE = {
     }],
 }
 WEBFINGER_FAKE_FED_BRID_GY = copy.deepcopy(WEBFINGER_FAKE)
-WEBFINGER_FAKE_FED_BRID_GY['links'][3]['href'] = 'https://fed.brid.gy/ap/sharedInbox'
+for link in WEBFINGER_FAKE_FED_BRID_GY['links']:
+    if 'href' in link:
+        link['href'] = link['href'].replace('http://localhost', 'https://fed.brid.gy')
 WEBFINGER_FAKE_FED_BRID_GY['links'][4]['template'] = 'https://fed.brid.gy/fa/fake:user?url={uri}'
 
 

tests/testutil.py

@@ -79,9 +79,6 @@ class Fake(User, protocol.Protocol):
     def ap_address(self):
         return f'@{self.key.id()}@fake'
 
-    def ap_actor(self, rest=None):
-        return f'http://bf/fake/{self.key.id()}/ap' + (f'/{rest}' if rest else '')
-
     @classmethod
     def owns_id(cls, id):
         if id.startswith('nope') or id == 'fake:nope':

web.py

@@ -46,7 +46,7 @@ def is_valid_domain(domain):
     Valid means TLD is ok, not blacklisted, etc.
     """
     if not re.match(DOMAIN_RE, domain):
-        logger.debug(f"{id} doesn't look like a domain")
+        logger.debug(f"{domain} doesn't look like a domain")
         return False
 
     if Web.is_blocklisted(domain):
@@ -77,13 +77,6 @@ class Web(User, Protocol):
     def _get_kind(cls):
         return 'MagicKey'
 
-    @ComputedProperty
-    def readable_id(self):
-        # prettify if domain, noop if username
-        username = self.username()
-        if username != self.key.id():
-            return util.domain_from_link(username, minimize=False)
-
     def _pre_put_hook(self):
         """Validate domain id, don't allow upper case or invalid characters."""
         super()._pre_put_hook()
@@ -101,8 +94,12 @@ class Web(User, Protocol):
         return super().get_or_create(id.lower().strip('.'), **kwargs)
 
     def handle(self):
-        """Returns this user's domain, eg ``user.com``."""
-        return self.key.id()
+        """Returns this user's chosen username or domain, eg ``user.com``."""
+        # prettify if domain, noop if username
+        username = self.username()
+        if username != self.key.id():
+            return util.domain_from_link(username, minimize=False)
+        return username
 
     def web_url(self):
         """Returns this user's web URL aka web_url, eg ``https://foo.com/``."""

webfinger.py

@@ -144,7 +144,7 @@ class Webfinger(flask_util.XrdOrJrd):
                 'rel': 'http://ostatus.org/schema/1.0/subscribe',
                 # TODO: switch to:
                 # 'template': common.host_url(g.user.user_page_path('?url={uri}')),
-                # the problem is that user_page_path() uses readable_id, which uses
+                # the problem is that user_page_path() uses handle_or_id, which uses
                 # custom username instead of domain, which may not be unique
                 'template': common.host_url(f'{cls.ABBREV}/{id}?url={{uri}}'),
             }]