AP users: promote ActivityPub.label_id computed prop to User.readable_id

and drop ActivityPub.get_by_id() override, move logic to user() page handler. fixes 'Only ancestor queries are allowed inside transactions.'

for #512

activitypub.py

@@ -52,25 +52,14 @@ class ActivityPub(User, Protocol):
     """ActivityPub protocol class.
 
     Key id is AP/AS2 actor id URL. (*Not* fediverse/WebFinger @-@ handle!)
-
-    Includes extra `address` computed property for querying entities by handle.
     """
     LABEL = 'activitypub'
 
     @ndb.ComputedProperty
-    def address(self):
+    def readable_id(self):
+        """Returns fediverse handle ie WebFinger address, eg '@me@snarfed.org'."""
         return self.ap_address()
 
-    @classmethod
-    def get_by_id(cls, id):
-        """Override User.get_by_id to fall back to querying by address."""
-        return (super(User, ActivityPub).get_by_id(id)
-                or ActivityPub.query(ActivityPub.address == id).get())
-
-    def label_id(self):
-        """Returns this user's human-readable unique id, eg '@me@snarfed.org'."""
-        return self.address or self.key.id()
-
     def web_url(self):
         """Returns this user's web URL aka web_url, eg 'https://foo.com/'."""
         return util.get_url(self.actor_as2) or self.ap_actor()

models.py

@@ -98,6 +98,18 @@ class User(StringIdModel, metaclass=ProtocolUserMeta):
     created = ndb.DateTimeProperty(auto_now_add=True)
     updated = ndb.DateTimeProperty(auto_now=True)
 
+    @ndb.ComputedProperty
+    def readable_id(self):
+        """This user's human-readable unique id, eg '@me@snarfed.org'.
+
+        To be implemented by subclasses.
+        """
+        return None
+
+    def readable_or_key_id(self):
+        """Returns readable_id if set, otherwise key id."""
+        return self.readable_id or self.key.id()
+
     @classmethod
     def new(cls, **kwargs):
         """Try to prevent instantiation. Use subclasses instead."""
@@ -181,11 +193,7 @@ def name(self):
             if name:
                 return name
 
-        return self.label_id()
-
-    def label_id(self):
-        """Returns this user's human-readable unique id, eg '@me@snarfed.org'."""
-        return self.key.id()
+        return self.readable_or_key_id()
 
     def username(self):
         """Returns the user's preferred username.
@@ -270,7 +278,7 @@ def ap_actor(self, rest=None):
 
     def user_page_path(self, rest=None):
         """Returns the user's Bridgy Fed user page path."""
-        path = f'/{self.LABEL}/{self.label_id()}'
+        path = f'/{self.LABEL}/{self.readable_or_key_id()}'
 
         if rest:
             if not rest.startswith('?'):

pages.py

@@ -55,10 +55,14 @@ def web_user_redirects(**kwargs):
 
 @app.get(f'/<any({",".join(PROTOCOLS)}):protocol>/<id>')
 def user(protocol, id):
-    g.user = PROTOCOLS[protocol].get_by_id(id)
+    cls = PROTOCOLS[protocol]
+    g.user = cls.get_by_id(id)
+    if not g.user:
+        g.user = cls.query(cls.readable_id == id).get()
+
     if not g.user or not g.user.direct:
         return USER_NOT_FOUND_HTML, 404
-    elif id != g.user.label_id():
+    elif id != g.user.readable_or_key_id():  # this also handles  use_instead
         return redirect(g.user.user_page_path(), code=301)
 
     assert not g.user.use_instead

tests/test_activitypub.py

@@ -1585,15 +1585,15 @@ def test_postprocess_as2_idempotent(self):
     def test_ap_address(self):
         user = ActivityPub(actor_as2={**ACTOR, 'preferredUsername': 'me'})
         self.assertEqual('@me@mas.to', user.ap_address())
-        self.assertEqual('@me@mas.to', user.address)
+        self.assertEqual('@me@mas.to', user.readable_id)
 
         user = ActivityPub(actor_as2=ACTOR)
         self.assertEqual('@swentel@mas.to', user.ap_address())
-        self.assertEqual('@swentel@mas.to', user.address)
+        self.assertEqual('@swentel@mas.to', user.readable_id)
 
         user = ActivityPub(id='https://mas.to/users/alice')
         self.assertEqual('@alice@mas.to', user.ap_address())
-        self.assertEqual('@alice@mas.to', user.address)
+        self.assertEqual('@alice@mas.to', user.readable_id)
 
     def test_ap_actor(self):
         user = self.make_user('http://foo/actor', cls=ActivityPub)
@@ -1609,19 +1609,11 @@ def test_web_url(self):
         user.actor_as2['url'] = ['http://my/url']
         self.assertEqual('http://my/url', user.web_url())
 
-    def test_label_id(self):
+    def test_readable_id(self):
         user = self.make_user('http://foo', cls=ActivityPub)
-        self.assertEqual('http://foo', user.label_id())
+        self.assertIsNone(user.readable_id)
+        self.assertEqual('http://foo', user.readable_or_key_id())
 
         user.actor_as2 = ACTOR
-        self.assertEqual('@swentel@mas.to', user.label_id())
-
-    def test_get_by_id(self):
-        user = self.make_user('http://foo', cls=ActivityPub)
-        self.assert_entities_equal(user, ActivityPub.get_by_id('http://foo'))
-        self.assertIsNone(ActivityPub.get_by_id('@swentel@mas.to'))
-
-        user.actor_as2 = ACTOR
-        user.put()
-        self.assert_entities_equal(user, ActivityPub.get_by_id('http://foo'))
-        self.assert_entities_equal(user, ActivityPub.get_by_id('@swentel@mas.to'))
+        self.assertEqual('@swentel@mas.to', user.readable_id)
+        self.assertEqual('@swentel@mas.to', user.readable_or_key_id())

tests/test_models.py

@@ -103,8 +103,8 @@ def test_name(self):
         g.user.actor_as2 = {'name': 'alice'}
         self.assertEqual('alice', g.user.name())
 
-    def test_label_id(self):
-        self.assertEqual('y.z', g.user.label_id())
+    def test_readable_id(self):
+        self.assertIsNone(g.user.readable_id)
 
 
 class ObjectTest(TestCase):

tests/test_pages.py

@@ -47,10 +47,10 @@ def test_user_fake(self):
         got = self.client.get('/fake/foo.com')
         self.assert_equals(200, got.status_code)
 
-    def test_user_activitypub_address(self):
+    def test_user_readable_id_activitypub_address(self):
         user = self.make_user('foo', cls=ActivityPub,
                               actor_as2=ACTOR_WITH_PREFERRED_USERNAME)
-        self.assertEqual('@me@plus.google.com', user.address)
+        self.assertEqual('@me@plus.google.com', user.ap_address())
 
         got = self.client.get('/activitypub/@me@plus.google.com')
         self.assert_equals(200, got.status_code)
@@ -74,7 +74,7 @@ def test_user_not_direct(self):
         got = self.client.get('/web/user.com')
         self.assert_equals(404, got.status_code)
 
-    def test_user_redirect(self):
+    def test_user_web_redirect(self):
         got = self.client.get('/user/user.com')
         self.assert_equals(301, got.status_code)
         self.assert_equals('/web/user.com', got.headers['Location'])

tests/test_web.py

@@ -1514,9 +1514,6 @@ def test_verify_override_preferredUsername(self, mock_get, _):
             'preferredUsername': 'user.com',
         })
 
-    def test_label_id(self, _, __):
-        self.assertEqual('user.com', self.user.label_id())
-
     def test_web_url(self, _, __):
         self.assertEqual('https://user.com/', self.user.web_url())
 

web.py

@@ -7,7 +7,7 @@
 import feedparser
 from flask import g, redirect, render_template, request
 from flask.views import View
-from google.cloud.ndb import Key
+from google.cloud.ndb import ComputedProperty, Key
 from granary import as1, as2, microformats2
 import mf2util
 from oauth_dropins.webutil import flask_util, util
@@ -54,9 +54,12 @@ class Web(User, Protocol):
     def _get_kind(cls):
         return 'MagicKey'
 
-    def label_id(self):
+    @ComputedProperty
+    def readable_id(self):
         # prettify if domain, noop if username
-        return util.domain_from_link(self.username(), minimize=False)
+        username = self.username()
+        if username != self.key.id():
+            return util.domain_from_link(username, minimize=False)
 
     def web_url(self):
         """Returns this user's web URL aka web_url, eg 'https://foo.com/'."""

webfinger.py

@@ -112,7 +112,7 @@ def template_vars(self, domain=None, allow_indirect=False):
                 'rel': 'http://ostatus.org/schema/1.0/subscribe',
                 # TODO(#512) switch to:
                 # 'template': common.host_url(g.user.user_page_path('?url={uri}')),
-                # the problem is that user_page_path() uses label_id(), which uses
+                # the problem is that user_page_path() uses readable_id, which uses
                 # custom username instead of domain, which may not be unique
                 'template': common.host_url(f'web/{domain}?url={{uri}}'),
             }]