Add Bluesky backfeed support #1523

JoelOtter · 2023-07-24T20:15:02Z

This is the bridgy portion of the Bluesky work. It's rather simple compared to the Granary work, which this depends on along with the oauth-dropins work.

Most significantly it adds the app password submission page, which currently looks like this:

Tests to come.

snarfed · 2023-07-29T20:16:46Z

Argh, just realized I never sent my review comments. Sorry for the delay!

snarfed · 2023-07-24T22:10:21Z

bluesky.py

+from urllib.parse import quote
+
+logger = logging.getLogger(__name__)
+


very minor style nit, two line breaks.

snarfed · 2023-07-24T22:12:28Z

handlers.py

@@ -76,7 +77,7 @@ class Item(View):
  """
  source = None

-  VALID_ID = re.compile(r'^[\w.+:@=<>-]+$')


We need to keep the :, right?

Everything should be URL encoded so there shouldn't be colons - where would they come from?

We don't really try hard to guarantee URL-encoding. Eg here's the code that generates webmention source URLs, which you :

bridgy/tasks.py

Lines 782 to 798 in 1d1fc44

# generate source URL

id = activity['id']

parsed = util.parse_tag_uri(id)

post_id = parsed[1] if parsed else id

parts = [self.entity.type, g.source.SHORT_NAME, g.source.key.string_id(), post_id]

if self.entity.type != 'post':

# parse and add response id. (we know Response key ids are always tag URIs)

_, response_id = util.parse_tag_uri(self.entity.key.string_id())

reaction_id = response_id

if self.entity.type in ('like', 'react', 'repost', 'rsvp'):

response_id = response_id.split('_')[-1] # extract responder user id

parts.append(response_id)

if self.entity.type == 'react':

parts.append(reaction_id)

return util.host_url('/'.join(parts))

URL encoding is nice, but I'm not that much of a stickler when it's not necessary for correctness. I sometimes like leaving in some characters like : when possible, even if only for aesthetics. As an example, when Bridgy supported Facebook, many of its ids included colons: https://github.com/snarfed/granary/blob/4f13ce738fabfdda11d59f96a3f6878df2233390/granary/facebook.py#L2427-L2447

Ah, I see your format_for_source_url change. OK! We may still get colons in other silos' ids, but at least not Bluesky's.

snarfed · 2023-07-24T22:18:18Z

bluesky.py

+    return self.gr_source.user_url(self.name)
+
+  def format_for_source_url(self, key):
+    """Bluesky keys (AT URIs) contain slashes, so must be double-encoded."""


Hmm, why double encoded? What breaks if this is just once with safe=''?

This is due to an issue in Flask (via WSGI) wherein URL-encoded slashes are treated as slashes, which breaks routing pallets/flask#900

Woowww that's unfortunate. Could you add that explanation and link to the docstring, including that we don't have to decode them correspondingly?

snarfed · 2023-07-24T22:21:35Z

handlers.py

@@ -222,6 +223,10 @@ class Post(Item):
  def get_item(self, post_id):
    posts = None

+    # Bluesky IDs need to be URL-decoded.


minor, could these all be unified into dispatch_request, eg maybe in the existing loop over kwargs on line 136?

snarfed · 2023-07-24T22:22:52Z

models.py

@@ -483,6 +486,19 @@ def edit_template_url(self):
    """
    raise NotImplementedError()

+  def format_for_source_url(self, key):


minor nit, s/key/id/ here and below?

snarfed · 2023-07-24T22:29:32Z

templates/provide_app_password.html

+<br />
+<p class="row big">Log into Bluesky with your app password.</p>
+
+<p>App passwords can be generated from your Bluesky settings, and revoked at any time.</p>


minor, center these with class="row"?

also, ugh, they don't really have user-facing docs for app passwords. mind linking this to https://bsky.app/settings/app-passwords ?

Suggested change

App passwords can be generated from your Bluesky settings, and revoked at any time.

<a href="https://bsky.app/settings/app-passwords">App passwords can be generated from your Bluesky settings</a>, and revoked at any time.

snarfed · 2023-07-24T22:30:57Z

templates/provide_app_password.html

+<p>You should generate a new app password just for Bridgy.</p>
+
+<form id="app-password" class="row big" action="/bluesky/callback" method="post">
+  <input type="text" name="username" placeholder="username" />


Mind if we change username to handle everywhere user-facing? Hopefully avoids confusion for people with custom domain handles.

snarfed · 2023-08-02T18:41:15Z

(this is re: #1453)

JoelOtter · 2023-09-03T21:11:34Z

@snarfed Think I've addressed all comments - just needs tests added and the Granary change merged for it to pass on CI. I'm having a couple of tests fail locally and not sure why, would you be able to offer guidance?

======================================================================
FAIL: test_social_user_page_mf2 (tests.test_pages.PagesTest)
Check the custom mf2 we render on social user pages.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/joel/Code/bridgy/local/lib/python3.9/site-packages/mox3/mox.py", line 2139, in new_method
    func(self, *args, **kwargs)
  File "/Users/joel/Code/bridgy/tests/test_pages.py", line 328, in test_social_user_page_mf2
    self.assertEqual(200, response.status_code)
AssertionError: 200 != 500

======================================================================
FAIL: test_user_page_escapes_html_chars (tests.test_pages.PagesTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/joel/Code/bridgy/local/lib/python3.9/site-packages/mox3/mox.py", line 2139, in new_method
    func(self, *args, **kwargs)
  File "/Users/joel/Code/bridgy/tests/test_pages.py", line 418, in test_user_page_escapes_html_chars
    self.assertEqual(200, resp.status_code)
AssertionError: 200 != 500

snarfed · 2023-09-04T05:26:13Z

Awesome! I'll look tomorrow morning. For the tests, try running just one of them with -v, eg python -m unittest -v -k test_social_user_page_mf2, you should see the stack trace where it's crashing along with full logging.

JoelOtter · 2023-09-04T08:12:21Z

Awesome! I'll look tomorrow morning. For the tests, try running just one of them with -v, eg python -m unittest -v -k test_social_user_page_mf2, you should see the stack trace where it's crashing along with full logging.

This worked great, thanks!

snarfed · 2023-09-04T14:52:15Z

This and the granary change are both looking great! You're right, the last remaining thing I'd probably look for is new tests. Not necessarily the most fun part, but I pretty much always find more bugs when I write them. Most new tests would probably go into granary; some here too, but mostly just logic, eg the URL-encoding/decoding in handlers.py and PropagateResponse.source_url, no need to worry too much about trying to test UI stuff. Feel free to copy and paste liberally from existing tests, obviously!

JoelOtter · 2023-10-24T21:44:29Z

@snarfed How's this for tests? More needed? I kind of followed the Mastodon tests

JoelOtter · 2023-10-24T21:54:45Z

CI errors appear to be hitting that old version of lexrpc again 🙃

cc #1523

snarfed · 2023-10-24T22:37:35Z

CI errors appear to be hitting that old version of lexrpc again 🙃

Oops true, sorry about that! Hadn't fixed that in this repo yet. I've done that now, feel free to rebase and try again.

JoelOtter · 2023-10-25T08:37:57Z

Hmm, seem to be getting the same thing:

TypeError: Client.__init__() missing 1 required positional argument: 'lexicons'

snarfed · 2023-10-25T13:36:32Z

Huh, true, that's odd. Looking.

snarfed · 2023-10-25T13:38:29Z

Ah, it wasn't installing lexrpc from github. Fixed that! I'll look at the PR itself now...

snarfed

This is looking pretty ready! Couple minor nits, but nothing big. Ready to merge after those and CI pass!

snarfed · 2023-10-25T13:44:30Z

bluesky.py

+    gr_source = gr_bluesky.Bluesky(user.get('handle'), app_password=app_password)
+    actor = gr_source.user_to_actor(user)
+    return Bluesky(id=auth_entity.key_id(),
+                   username=auth_entity.key_id(),


this should probably be handle, right?

Are you saying the value for username should be the Bluesky handle rather than the DID, or I should be passing a kwarg handle into the constructor?

I think I'm being stupid here and you definitely mean the former, will change

snarfed · 2023-10-25T13:45:39Z

bluesky.py

+    return Bluesky(id=auth_entity.key_id(),
+                   username=auth_entity.key_id(),
+                   auth_entity=auth_entity.key,
+                   name=user.get('handle'),


and then this should be displayName?

It could be but I think that would actually be inconsistent with other silos? Bridgy current seems to use username rather than display name for this in Mastodon and Twitter (RIP):

Hmm, unless this is something that changed recently? Looking at the source code it appears to be as you say, but that isn't reflected in my actual brid.gy account

right, Bridgy shows silo-specific usernames/handles in the UI, but those come from label_name(), not from the name property. I think we want name to be the human-readable name.

bridgy/templates/profile_link.html

Line 12 in 33b4a71

{{ source.label_name() }}

bridgy/models.py

Line 143 in 33b4a71

name = ndb.StringProperty() # full human-readable name

Checking in a few other silos...yup, name should be human name.

OK I'm a bit baffled by this, running it locally gives the same result, but the name field is clearly my display name, it just doesn't get used in the display, but in Bluesky it does!

I guess the problem is somewhere else?

Ah, so I should change the implementation of label_name()?

Ahh sorry, the missing piece is that Bluesky itself should override label_name() with whatever we want there. Other examples:

bridgy/mastodon.py

Lines 119 to 121 in 33b4a71

def label_name(self):

"""Returns the fully qualified address."""

return self.username

bridgy/github.py

Lines 74 to 76 in 33b4a71

def label_name(self):

"""Returns the username."""

return self.username

so yeah I think we want to do that and return handle.

Sorry for the confusion!

I wish GitHub had auto-refresh 😆 Will get that changed!

snarfed · 2023-10-25T13:48:17Z

bluesky.py

+  OAUTH_START = oauth_bluesky.Start
+  AUTH_MODEL = oauth_bluesky.BlueskyAuth
+  URL_CANONICALIZER = util.UrlCanonicalizer(
+          domain=GR_CLASS.DOMAIN,


this makes me think we need to support multiple domains in UrlCanonicalizer, so we can canonicalize staging.bsky.app => bsky.app. Obviously not needed in this PR though!

Ugh yeah federation/multiple PDSs is just not something I've thought about yet, likely to be a headache but hoping we can use a lot of the same stuff Mastodon does?

Oh this isn't federation, that's just the main official PSS's old hostname, there are still syndication links to it out there in the wild, eg on my site 😁

Agreed though, let's definitely punt on federation itself for now!

snarfed · 2023-10-25T13:49:25Z

bluesky.py

+
+  @classmethod
+  def button_html(cls, feature):
+    """Override oauth-dropins's button_html() to not show the instance text box."""


minor nit: update docstring? no instance text box, right?

snarfed · 2023-10-25T13:55:13Z

Oh, one last small thing, could you center the handle + app password text boxes on the login page?

snarfed · 2023-10-25T15:43:52Z

Oh btw, I tried running this locally, and login and poll worked great! Haven't tried propagate yet, but still, good sign!

JoelOtter · 2023-10-25T20:18:04Z

Should be all addressed now!

snarfed · 2023-10-25T20:33:28Z

Woo, looks great! Thanks for your patience. Merging and deploying! Can't wait to try it in prod!

Next step, docs? Not much, only a few sections on https://brid.gy/about and in the README that call out specific silos. Then maybe a blog post? You should get to do the honors!

snarfed · 2023-10-25T20:59:59Z

Background service isn't starting up ok. 😕 Not sure why, I'm guessing not related to this PR, maybe a bad new dependency that we upgraded since deploying last a couple weeks ago. Looking now...

snarfed · 2023-10-25T21:01:53Z

Startup command is gunicorn --workers 1 --threads 10 --timeout 600 -b :$PORT background:app, that runs fine locally for me on macOS. Hrm.

snarfed · 2023-10-25T21:08:44Z

Same build config for both services' containers, and the default (ie frontend) service is up and serving fine. Can't tell why background isn't. And I'm not seeing any useful logs, it just closes the /_ah/start request's connection after 5s, which makes the watchdog terminate it. Hrm.

snarfed · 2023-10-25T21:09:09Z

Rolling back for now, still investigating.

snarfed · 2023-10-25T21:11:08Z

Aha, background.yaml is still on runtime: python39, missed upgrading that to 3.11 a while back. Fingers crossed that fixes it...

snarfed · 2023-10-25T21:14:05Z

It's alive, it's aliiiive!

JoelOtter · 2023-10-26T08:09:19Z

Next step, docs? Not much, only a few sections on https://brid.gy/about and in the README that call out specific silos. Then maybe a blog post? You should get to do the honors!

Very happy to write some docs. :) What would a blog post look like, do you mean on my own blog?

snarfed · 2023-10-26T15:23:15Z

Sure, at least if you want! Just announcing the launch, and maybe syndicated to https://news.indieweb.org/en . Totally optional, of course.

JoelOtter · 2023-10-26T15:28:48Z

Cool! I'm guessing the indie web way ™️ would be to write it on my own site, do a u-syndication to news.indieweb, do a u-mention-of to yourself, then send the webmentions accordingly?

snarfed · 2023-10-26T16:02:47Z

u-syndication to IndieNews, yes! u-mention-of to...me? Not necessary but OK if you want! Except I don't think u-mention-of exists, mentions are just links, no need to mark up with any specific mf2.

snarfed reviewed Jul 29, 2023

View reviewed changes

JoelOtter force-pushed the main branch from af2583c to eb8d09e Compare August 25, 2023 18:35

JoelOtter force-pushed the main branch from 5bbdaba to 7141d95 Compare October 7, 2023 13:24

JoelOtter force-pushed the main branch from 8f8fea8 to 55df4a3 Compare October 24, 2023 17:31

JoelOtter requested a review from snarfed October 24, 2023 21:44

snarfed added a commit that referenced this pull request Oct 24, 2023

circle: force install arroba, lexrpc at head

a3c3bc8

cc #1523

JoelOtter force-pushed the main branch from 2b78944 to 4b39a7c Compare October 25, 2023 07:36

snarfed reviewed Oct 25, 2023

View reviewed changes

JoelOtter added 8 commits October 25, 2023 20:31

Add Bluesky backfeed support

cb163d2

Add URL canonicalizer directly, fix image parsing

27b078c

Handle URL image list in handler

37ba15c

newline

9ce7afc

Rename key to id

aa0e1bc

Center, add link

c7045f1

Re-add colon

679f632

Add docstring

f1efab0

JoelOtter added 5 commits October 25, 2023 20:31

Move unquote to dispatch_request

e8c1a23

Change username to handle on form

1ef008f

Ensure image object on actor even if no URL

ff55114

Fixes after rebase

885b831

Add Bluesky tests

62e4472

JoelOtter force-pushed the main branch from 4b39a7c to 62e4472 Compare October 25, 2023 19:32

JoelOtter added 3 commits October 25, 2023 20:45

Center input fields

79ad8df

Make name/username consistent, implement label_name

48ed5b0

Fix docstring

9ca9ec5

JoelOtter requested a review from snarfed October 25, 2023 20:17

snarfed merged commit 9e7e489 into snarfed:main Oct 25, 2023
4 checks passed

		from urllib.parse import quote

		logger = logging.getLogger(__name__)

	# generate source URL
	id = activity['id']
	parsed = util.parse_tag_uri(id)
	post_id = parsed[1] if parsed else id
	parts = [self.entity.type, g.source.SHORT_NAME, g.source.key.string_id(), post_id]

	if self.entity.type != 'post':
	# parse and add response id. (we know Response key ids are always tag URIs)
	_, response_id = util.parse_tag_uri(self.entity.key.string_id())
	reaction_id = response_id
	if self.entity.type in ('like', 'react', 'repost', 'rsvp'):
	response_id = response_id.split('_')[-1] # extract responder user id
	parts.append(response_id)
	if self.entity.type == 'react':
	parts.append(reaction_id)

	return util.host_url('/'.join(parts))

	<p>App passwords can be generated from your Bluesky settings, and revoked at any time.</p>
	<p class="row"><a href="https://bsky.app/settings/app-passwords">App passwords can be generated from your Bluesky settings</a>, and revoked at any time.</p>

	def label_name(self):
	"""Returns the fully qualified address."""
	return self.username

	def label_name(self):
	"""Returns the username."""
	return self.username

Add Bluesky backfeed support #1523

Add Bluesky backfeed support #1523

Conversation

JoelOtter commented Jul 24, 2023

snarfed commented Jul 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snarfed Aug 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snarfed commented Aug 2, 2023

JoelOtter commented Sep 3, 2023

snarfed commented Sep 4, 2023

JoelOtter commented Sep 4, 2023

snarfed commented Sep 4, 2023

JoelOtter commented Oct 24, 2023

JoelOtter commented Oct 24, 2023

snarfed commented Oct 24, 2023

JoelOtter commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snarfed Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

JoelOtter commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

snarfed commented Oct 25, 2023

JoelOtter commented Oct 26, 2023

snarfed commented Oct 26, 2023

JoelOtter commented Oct 26, 2023

snarfed commented Oct 26, 2023

snarfed Aug 30, 2023 •

edited

Loading

snarfed Oct 25, 2023 •

edited

Loading