* remove obsolete TODOs, commented out code
* remove obsolete circular imports of per-protocol modules
* minimize Object put in Protocol.load
* remove duplicated Protocol.load tests in test_activitypub
* re-enable rest of ActivityPubUtilsTest.test_postprocess_as2_idempotent
* drop default cls=Web in TestCase.make_user
pull/729/head
Ryan Barrett 2023-11-15 14:23:08 -08:00
rodzic 4c61770017
commit 4d095fa3d9
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: 6BE31FDF4776E9D4
19 zmienionych plików z 49 dodań i 159 usunięć

Wyświetl plik

@ -156,10 +156,6 @@ class ActivityPub(User, Protocol):
@classmethod
def target_for(cls, obj, shared=False):
"""Returns ``obj``'s or its author's/actor's inbox, if available."""
# TODO: we have entities in prod that fail this, eg
# https://indieweb.social/users/bismark has source_protocol webmention
# assert obj.source_protocol in (cls.LABEL, cls.ABBREV, 'ui', None), str(obj)
if not obj.as1:
return None

13
app.py
Wyświetl plik

@ -6,18 +6,7 @@ registered.
from flask_app import app
# import all modules to register their Flask handlers
import (
activitypub,
atproto,
convert,
follow,
pages,
redirect,
superfeedr,
ui,
webfinger,
web,
)
import activitypub, atproto, convert, follow, pages, redirect, superfeedr, ui, webfinger, web
import models
models.reset_protocol_properties()

Wyświetl plik

@ -449,7 +449,6 @@ def poll_notifications():
for cls in set(PROTOCOLS.values())
if cls and cls != ATProto))
# TODO: convert to Session for connection pipelining!
client = Client(f'https://{os.environ["APPVIEW_HOST"]}',
headers={'User-Agent': USER_AGENT})
@ -504,7 +503,6 @@ def poll_posts():
for cls in set(PROTOCOLS.values())
if cls and cls != ATProto))
# TODO: convert to Session for connection pipelining!
client = Client(f'https://{os.environ["APPVIEW_HOST"]}',
headers={'User-Agent': USER_AGENT})

3
ids.py
Wyświetl plik

@ -111,7 +111,8 @@ def translate_handle(*, handle, from_proto, to_proto):
case 'activitypub', 'web':
user, instance = handle.lstrip('@').split('@')
return f'instance/@user' # TODO
# TODO: get this from the actor object's url field?
return f'https://{instance}/@{user}'
case _, 'web':
return handle

Wyświetl plik

@ -544,8 +544,7 @@ class Object(StringIdModel):
status = ndb.StringProperty(choices=STATUSES)
# choices is populated in app, after all User subclasses are created,
# so that PROTOCOLS is fully populated
# TODO: remove? is this redundant with the protocol-specific data fields below?
# TODO: otherwise, nail down whether this is ABBREV or LABEL
# TODO: nail down whether this is ABBREV or LABEL
source_protocol = ndb.StringProperty(choices=[])
labels = ndb.StringProperty(repeated=True, choices=LABELS)
@ -588,13 +587,6 @@ class Object(StringIdModel):
@ComputedJsonProperty
def as1(self):
# TODO: bring back log or assert? we have prod entities that currently
# fail this though.
# assert (self.as2 is not None) ^ (self.bsky is not None) ^ (self.mf2 is not None), \
# f'{self.as2} {self.bsky} {self.mf2}'
# if bool(self.as2) + bool(self.bsky) + bool(self.mf2) > 1:
# logger.warning(f'{self.key} has multiple! {bool(self.as2)} {bool(self.bsky)} {bool(self.mf2)}')
if self.our_as1:
obj = self.our_as1
@ -603,7 +595,7 @@ class Object(StringIdModel):
elif self.bsky:
owner, _, _ = parse_at_uri(self.key.id())
ATProto = PROTOCOLS['atproto'] # TODO: circular import :( ???
ATProto = PROTOCOLS['atproto']
handle = ATProto(id=owner).handle
obj = bluesky.to_as1(self.bsky, repo_did=owner, repo_handle=handle,
uri=self.key.id(), pds=ATProto.target_for(self))

Wyświetl plik

@ -50,7 +50,6 @@ objects_cache_lock = Lock()
logger = logging.getLogger(__name__)
# TODO: merge Protocol and User classes?
class Protocol:
"""Base protocol class. Not to be instantiated; classmethods only.
@ -652,10 +651,8 @@ class Protocol:
error(f'Undo of Follow requires actor id and object id. Got: {actor_id} {inner_obj_id} {obj.as1}')
# deactivate Follower
# TODO: avoid import?
from web import Web
from_ = from_cls.key_for(actor_id)
to_cls = Protocol.for_id(inner_obj_id) or Web
to_cls = Protocol.for_id(inner_obj_id)
to = to_cls.key_for(inner_obj_id)
follower = Follower.query(Follower.to == to,
Follower.from_ == from_,
@ -1149,9 +1146,10 @@ class Protocol:
if obj.new is False:
obj.changed = obj.activity_changed(orig_as1)
obj.source_protocol = cls.LABEL
# TODO: drop this?
obj.put()
if obj.source_protocol not in (cls.LABEL, cls.ABBREV):
assert not obj.source_protocol
obj.source_protocol = cls.LABEL
obj.put()
with objects_cache_lock:
objects_cache[id] = obj

Wyświetl plik

@ -27,7 +27,6 @@ class Activity(StringIdModel):
source_atom = ndb.TextProperty()
target_as2 = ndb.TextProperty() # JSON
# TODO: uncomment
created = ndb.DateTimeProperty(auto_now_add=True)
updated = ndb.DateTimeProperty(auto_now=True)

Wyświetl plik

@ -136,24 +136,6 @@ RewriteRule ^.well-known/(host-meta|webfinger).* https://fed.brid.gy/$0 [redire
</li>
<!--
<em><a href="https://www.blogger.com/">Blogger</a></em>:
Not to other domains
https://helplogger.blogspot.com/2014/07/how-to-set-custom-redirects-for-blogger-post.html
<em><a href="https://medium.com/">Medium</a>: TODO</em>
Redirects but not custom
https://help.medium.com/hc/en-us/articles/213475208-301-Redirects
<em><a href="http://www.tumblr.com/">Tumblr</a></em>:
Haven't found how yet. "Link pages" here mention redirects but aren't what we need:
https://tumblr.zendesk.com/hc/en-us/articles/231449328-Redirect-pages
<em><a href="http://wordpress.com/">WordPress.com</a></em>:
Site Redirect, but not per URL
https://en.support.wordpress.com/site-redirect/
-->
</ul>
<li>Add <a href="https://webmention.net/">webmention</a> support to your site. This is strongly recommended, but technically optional. You don't have to automate the webmentions to Bridgy Fed to federate your posts, and you don't have to accept the inbound webmentions that Bridgy Fed sends, but you'll have a much better experience if you do. <a href="https://indieweb.org/webmention#Publishing_Software">Check out the IndieWeb wiki</a> for instructions for your web server.</li>

Wyświetl plik

@ -299,7 +299,8 @@ class ActivityPubTest(TestCase):
super().setUp()
self.request_context.push()
self.user = self.make_user('user.com', has_hcard=True, has_redirects=True,
self.user = self.make_user('user.com', cls=Web, has_hcard=True,
has_redirects=True,
obj_as2={**ACTOR, 'id': 'https://user.com/'})
self.swentel_key = ndb.Key(ActivityPub, 'https://mas.to/users/swentel')
self.masto_actor_key = ndb.Key(ActivityPub, 'https://mas.to/actor')
@ -952,7 +953,7 @@ class ActivityPubTest(TestCase):
has_hcard=True, has_redirects=True)
def test_inbox_follow_use_instead_strip_www(self, mock_head, mock_get, mock_post):
self.make_user('www.user.com', use_instead=self.user.key)
self.make_user('www.user.com', cls=Web, use_instead=self.user.key)
mock_head.return_value = requests_response(url='https://www.user.com/')
mock_get.side_effect = [
@ -1573,7 +1574,7 @@ class ActivityPubTest(TestCase):
class ActivityPubUtilsTest(TestCase):
def setUp(self):
super().setUp()
g.user = self.make_user('user.com', has_hcard=True, obj_as2=ACTOR)
g.user = self.make_user('user.com', cls=Web, has_hcard=True, obj_as2=ACTOR)
def test_put_validates_id(self, *_):
for bad in (
@ -1764,80 +1765,6 @@ class ActivityPubUtilsTest(TestCase):
self.assertEqual(['https://masto.foo/@other'],
postprocess_as2(obj)['cc'])
# TODO: make these generic and use Fake
@patch('requests.get')
def test_load_http(self, mock_get):
mock_get.return_value = AS2
id = 'http://the/id'
self.assertIsNone(Object.get_by_id(id))
# first time fetches over HTTP
got = ActivityPub.load(id)
self.assert_equals(id, got.key.id())
self.assert_equals(AS2_OBJ, got.as2)
mock_get.assert_has_calls([self.as2_req(id)])
# second time is in cache
got.key.delete()
mock_get.reset_mock()
got = ActivityPub.load(id)
self.assert_equals(id, got.key.id())
self.assert_equals(AS2_OBJ, got.as2)
mock_get.assert_not_called()
@patch('requests.get')
def test_load_datastore(self, mock_get):
id = 'http://the/id'
stored = Object(id=id, as2=AS2_OBJ)
stored.put()
protocol.objects_cache.clear()
# first time loads from datastore
got = ActivityPub.load(id)
self.assert_entities_equal(stored, got)
mock_get.assert_not_called()
# second time is in cache
stored.key.delete()
got = ActivityPub.load(id)
self.assert_entities_equal(stored, got)
mock_get.assert_not_called()
@patch('requests.get')
def test_load_preserves_fragment(self, mock_get):
stored = Object(id='http://the/id#frag', as2=AS2_OBJ)
stored.put()
protocol.objects_cache.clear()
got = ActivityPub.load('http://the/id#frag')
self.assert_entities_equal(stored, got)
mock_get.assert_not_called()
@patch('requests.get')
def test_load_datastore_no_as2(self, mock_get):
"""If the stored Object has no as2, we should fall back to HTTP."""
id = 'http://the/id'
stored = Object(id=id, as2={}, status='in progress')
stored.put()
protocol.objects_cache.clear()
mock_get.return_value = AS2
got = ActivityPub.load(id)
mock_get.assert_has_calls([self.as2_req(id)])
self.assert_equals(id, got.key.id())
self.assert_equals(AS2_OBJ, got.as2)
mock_get.assert_has_calls([self.as2_req(id)])
self.assert_object(id,
as2=AS2_OBJ,
as1={**AS2_OBJ, 'id': id},
source_protocol='activitypub',
# check that it reused our original Object
status='in progress')
@patch('requests.get')
def test_signed_get_redirects_manually_with_new_sig_headers(self, mock_get):
mock_get.side_effect = [
@ -2005,15 +1932,14 @@ class ActivityPubUtilsTest(TestCase):
}, ActivityPub.convert(obj))
def test_postprocess_as2_idempotent(self):
g.user = self.make_user('foo.com')
g.user = self.make_user('foo.com', cls=Web)
for obj in (ACTOR, REPLY_OBJECT, REPLY_OBJECT_WRAPPED, REPLY,
NOTE_OBJECT, NOTE, MENTION_OBJECT, MENTION, LIKE,
LIKE_WRAPPED, REPOST, FOLLOW, FOLLOW_WRAPPED, ACCEPT,
UNDO_FOLLOW_WRAPPED, DELETE, UPDATE_NOTE,
# TODO: these currently fail
# LIKE_WITH_ACTOR, REPOST_FULL, FOLLOW_WITH_ACTOR,
# FOLLOW_WRAPPED_WITH_ACTOR, FOLLOW_WITH_OBJECT, UPDATE_PERSON,
LIKE_WITH_ACTOR, REPOST_FULL, FOLLOW_WITH_ACTOR,
FOLLOW_WRAPPED_WITH_ACTOR, FOLLOW_WITH_OBJECT, UPDATE_PERSON,
):
with self.subTest(obj=obj):
obj = copy.deepcopy(obj)

Wyświetl plik

@ -967,7 +967,7 @@ class ATProtoTest(TestCase):
self.assert_task(mock_create_task, 'receive', '/queue/receive',
obj=post_obj.key.urlsafe(), authed_as='did:plc:a')
# TODO
# TODO: https://github.com/snarfed/bridgy-fed/issues/728
# repost_obj = Object.get_by_id('at://did:plc:d/app.bsky.feed.post/456')
# self.assertEqual(repost, repost_obj.bsky)
# self.assert_task(mock_create_task, 'receive', '/queue/receive',

Wyświetl plik

@ -15,6 +15,7 @@ from .testutil import Fake, OtherFake
from activitypub import ActivityPub
from common import CONTENT_TYPE_HTML
from web import Web
COMMENT_AS2 = {
**as2.from_as1(COMMENT),
@ -271,7 +272,7 @@ A ☕ reply
def test_web_to_activitypub_object(self, mock_get):
mock_get.return_value = requests_response(HTML)
self.make_user('user.com')
self.make_user('user.com', cls=Web)
url = 'https://user.com/bar?baz=baj&biff'
Object(id=url, mf2=parse_mf2(HTML)['items'][0]).put()
@ -284,7 +285,7 @@ A ☕ reply
def test_web_to_activitypub_fetch(self, mock_get):
mock_get.return_value = requests_response(HTML)
url = 'https://user.com/bar?baz=baj&biff'
self.make_user('user.com')
self.make_user('user.com', cls=Web)
Object(id=url, mf2=parse_mf2(HTML)['items'][0]).put()
@ -306,7 +307,7 @@ A ☕ reply
"""https://github.com/snarfed/bridgy-fed/issues/581"""
mock_get.return_value = requests_response(HTML)
self.make_user('user.com')
self.make_user('user.com', cls=Web)
self.store_object(id='http://user.com/a#b', mf2=parse_mf2(HTML)['items'][0])
resp = self.client.get(f'/convert/ap/http://user.com/a%23b',

Wyświetl plik

@ -15,6 +15,7 @@ from .testutil import Fake, TestCase
from activitypub import ActivityPub
from models import Follower, Object
from web import Web
WEBFINGER = requests_response({
'subject': 'acct:foo@bar',
@ -61,7 +62,7 @@ class RemoteFollowTest(TestCase):
def setUp(self):
super().setUp()
self.make_user('user.com')
self.make_user('user.com', cls=Web)
def test_no_domain(self, _):
got = self.client.post('/remote-follow?address=@foo@bar&protocol=web')
@ -135,7 +136,7 @@ class FollowTest(TestCase):
def setUp(self):
super().setUp()
self.user = self.make_user('alice.com')
self.user = self.make_user('alice.com', cls=Web)
self.state = {
'endpoint': 'http://auth/endpoint',
'me': 'https://alice.com',
@ -289,7 +290,7 @@ class FollowTest(TestCase):
self.assertEqual(400, resp.status_code)
def test_callback_user_use_instead(self, mock_get, mock_post):
user = self.make_user('www.alice.com')
user = self.make_user('www.alice.com', cls=Web)
self.user.use_instead = user.key
self.user.put()
@ -395,7 +396,7 @@ class UnfollowTest(TestCase):
def setUp(self):
super().setUp()
self.user = self.make_user('alice.com')
self.user = self.make_user('alice.com', cls=Web)
self.follower = Follower.get_or_create(
from_=self.user,
to=self.make_user('https://bar/id', cls=ActivityPub, obj_as2=FOLLOWEE),
@ -484,7 +485,7 @@ class UnfollowTest(TestCase):
self.assertEqual('https://alice.com', session['indieauthed-me'])
def test_callback_user_use_instead(self, mock_get, mock_post):
user = self.make_user('www.alice.com')
user = self.make_user('www.alice.com', cls=Web)
self.user.use_instead = user.key
self.user.put()

Wyświetl plik

@ -4,8 +4,8 @@ from atproto import ATProto
from flask_app import app
from ids import translate_handle, translate_object_id, translate_user_id
from models import Target
from web import Web
from .testutil import Fake, TestCase
from web import Web
class IdsTest(TestCase):
@ -92,7 +92,7 @@ class IdsTest(TestCase):
(ActivityPub, '@user@instance', ActivityPub, '@user@instance'),
(ActivityPub, '@user@instance', ATProto, 'user.instance.ap.brid.gy'),
(ActivityPub, '@user@instance', Fake, 'fake:handle:@user@instance'),
(ActivityPub, '@user@instance', Web, 'instance/@user'),
(ActivityPub, '@user@instance', Web, 'https://instance/@user'),
# # # TODO: enhanced
# (ActivityPub, '@user@instance', Web, 'https://instance/user'),
# (ActivityPub, '@user@instance', Fake,

Wyświetl plik

@ -43,7 +43,7 @@ class PagesTest(TestCase):
def setUp(self):
super().setUp()
self.user = self.make_user('user.com')
self.user = self.make_user('user.com', cls=Web)
def test_user(self):
got = self.client.get('/web/user.com', base_url='https://fed.brid.gy/')
@ -114,7 +114,7 @@ class PagesTest(TestCase):
self.assert_equals('/web/user.com', got.headers['Location'])
def test_user_use_instead(self):
self.make_user('bar.com', use_instead=self.user.key)
self.make_user('bar.com', cls=Web, use_instead=self.user.key)
got = self.client.get('/web/bar.com')
self.assert_equals(302, got.status_code)

Wyświetl plik

@ -12,6 +12,7 @@ from oauth_dropins.webutil import appengine_info
from oauth_dropins.webutil.flask_util import CLOUD_TASKS_QUEUE_HEADER, NoContent
from oauth_dropins.webutil.testutil import requests_response
import requests
from werkzeug.exceptions import BadRequest
# import first so that Fake is defined before URL routes are registered
from .testutil import Fake, OtherFake, TestCase
@ -25,7 +26,6 @@ import protocol
from protocol import Protocol
from ui import UIProtocol
from web import Web
from werkzeug.exceptions import BadRequest
from .test_activitypub import ACTOR
from .test_atproto import DID_DOC
@ -36,7 +36,7 @@ class ProtocolTest(TestCase):
def setUp(self):
super().setUp()
self.user = self.make_user('foo.com', has_hcard=True)
self.user = self.make_user('foo.com', cls=Web, has_hcard=True)
g.user = None
def tearDown(self):
@ -210,12 +210,14 @@ class ProtocolTest(TestCase):
def test_load_remote_true_existing_empty(self):
Fake.fetchable['foo'] = {'x': 'y'}
Object(id='foo').put()
Object(id='foo', our_as1={}, status='in progress').put()
loaded = Fake.load('foo', remote=True)
self.assertEqual({'id': 'foo', 'x': 'y'}, loaded.as1)
self.assertTrue(loaded.changed)
self.assertFalse(loaded.new)
# check that it merged in fields like status
self.assertEqual('in progress', loaded.status)
self.assertEqual(['foo'], Fake.fetched)
def test_load_remote_true_new_empty(self):
@ -316,6 +318,12 @@ class ProtocolTest(TestCase):
'object': 'other:bob',
}, obj.our_as1)
def test_load_preserves_fragment(self):
stored = self.store_object(id='http://the/id#frag', our_as1={'foo': 'bar'})
got = ActivityPub.load('http://the/id#frag')
self.assert_entities_equal(stored, got)
self.assertEqual([], Fake.fetched)
def test_actor_key(self):
user = self.make_user(id='fake:a', cls=Fake)
a_key = user.key

Wyświetl plik

@ -33,7 +33,7 @@ class RedirectTest(testutil.TestCase):
def setUp(self):
super().setUp()
self.user = self.make_user('user.com')
self.user = self.make_user('user.com', cls=Web)
def test_redirect(self):
got = self.client.get('/r/https://user.com/bar?baz=baj&biff')

Wyświetl plik

@ -396,7 +396,7 @@ class WebTest(TestCase):
obj = Object(id='https://user.com/', mf2=ACTOR_MF2, source_protocol='web')
obj.put()
g.user = self.make_user('user.com', has_redirects=True, obj=obj)
g.user = self.make_user('user.com', cls=Web, has_redirects=True, obj=obj)
self.mrs_foo = ndb.Key(ActivityPub, 'https://mas.to/mrs-foo')
@ -1113,7 +1113,7 @@ class WebTest(TestCase):
},
}
g.user.obj.put()
self.make_user('www.user.com', use_instead=g.user.key)
self.make_user('www.user.com', cls=Web, use_instead=g.user.key)
self.make_followers()
note_html = NOTE_HTML.replace('https://user.com/', 'https://www.user.com/')
@ -1774,7 +1774,7 @@ http://this/404s
})
def test_verify_www_redirect(self, mock_get, _):
www_user = self.make_user('www.user.com')
www_user = self.make_user('www.user.com', cls=Web)
empty = requests_response('')
mock_get.side_effect = [

Wyświetl plik

@ -141,7 +141,7 @@ class WebfingerTest(TestCase):
def setUp(self):
super().setUp()
self.user = self.make_user('user.com', has_hcard=True, obj_as2={
self.user = self.make_user('user.com', cls=Web, has_hcard=True, obj_as2={
'@context': 'https://www.w3.org/ns/activitystreams',
'type': 'Person',
'url': 'https://user.com/about-me',

Wyświetl plik

@ -276,8 +276,7 @@ class TestCase(unittest.TestCase, testutil.Asserts):
kwargs.setdefault('headers', {})[flask_util.CLOUD_TASKS_QUEUE_HEADER] = ''
return client.post(url, **kwargs)
# TODO: switch default to Fake, start using that more
def make_user(self, id, cls=Web, **kwargs):
def make_user(self, id, cls, **kwargs):
"""Reuse RSA key across Users because generating it is expensive."""
obj_key = None