Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare ?p=<id> and ?author=<id> for ActivityPub #894

Merged
merged 59 commits into from
Oct 22, 2024
Merged

Conversation

pfefferle
Copy link
Member

This way we could replace the permalink with the ID of a post or an author, to work agains the Username and Permalink issue.

Maybe fixes the caching issues in WordPress!?!

This way we could replace the permalink with the ID of a post or an author, to work agains the Username and Permalink issue.
@pfefferle pfefferle changed the title Prepare ?p= and ?author= for ActivityPub Prepare ?p=<id> and ?author=<id> for ActivityPub Sep 14, 2024
@pfefferle
Copy link
Member Author

@mattwiebe @akirk @jeherve @mediaformat @Menrath Is that the way to go? What do you think about using the example.org/?p=<ID> instead of example.org/permalink/ as ID?

@Menrath
Copy link
Contributor

Menrath commented Sep 14, 2024

I think that would be a good idea, if I remember rightly, we discussed this very thing on a call about a year ago! :)

Can you link to any discussion about the mentioned "to work agains the Username and Permalink issue". I am not completely sure, if I have the right issue in mind.

A suggestion: what about replacing author with activitypub_actor, along with a clear comment on how the relationship currently works. This would give us more flexibility for certain features in the future.

So do you think get_id return the proposed structure and get_url should still return the permalink? Or should we use that structure for both?

@pfefferle
Copy link
Member Author

Can you link to any discussion about the mentioned "to work agains the Username and Permalink issue". I am not completely sure, if I have the right issue in mind.

I am not sure if there is a discussion exactly about that, but in the end it is the same discussion that speaks against the permalink. People want to change names and if the pretty URL is also the ID, we always need to send a Move activity if a user want to change usernames.

A suggestion: what about replacing author with activitypub_actor, along with a clear comment on how the relationship currently works. This would give us more flexibility for certain features in the future.

The nice thing about using ?author= is, that all core WordPress functions (is_author for example) still work and we do not have to change too much code.

So do you think get_id return the proposed structure and get_url should still return the permalink? Or should we use that structure for both?

get_id should return example.org/?p=1 and get_url should return the pretty permaling example.org/pretty/permalink.

@github-actions github-actions bot added the Docs label Sep 15, 2024
@jeherve
Copy link
Member

jeherve commented Sep 16, 2024

It makes sense to me at first, since it follows the existing "ugly permalinks" defaults from Core itself. I'm not familiar with the issues it solves though, so I'll let others weigh in on that :)

@akirk
Copy link
Member

akirk commented Sep 16, 2024

I haven't thought it through but I just wanted to mention that also the REST API endpoints might be an option, e.g. /wp-json/wp/v2/posts/1234 instead of /?p=1234, resp. /wp-json/wp/v2/users/1234 instead of /?author=1234

@pfefferle
Copy link
Member Author

pfefferle commented Sep 16, 2024

@akirk the problem with the REST endpoint is, that the ID has to return the ActivityPub JSON when requested with the correct Accept header! Besides of that, I would love to redirect the user to the post or author permalink if loaded in the browser and I am not a fan of redirecting API requests to the frontend.

@akirk
Copy link
Member

akirk commented Sep 16, 2024

Makes sense not to use it then! I just wanted to add it as an option so that we know it was considered.

@mediaformat
Copy link
Contributor

This is very welcome!

Using ?p=<id> for ActivityPub id makes the system more robust, allowing site admins to change permalink structute and site editors/authors to change post slugs without breaking existing federated interactions.

@mattwiebe
Copy link
Contributor

The only downside of this that I can see is that you will no longer to be able to paste the pretty permalink URL into eg a Mastodon search and get the result.

But it's overall more robust and does have the possibility of bypassing some caching layers, have there been any tests along those lines? Are we looking to address caching plugins, or Cloudflare, or?

@pfefferle
Copy link
Member Author

@mattwiebe

The only downside of this that I can see is that you will no longer to be able to paste the pretty permalink URL into eg a Mastodon search and get the result.

This will still be possible! I do not want to disable the Content-Negotiation, so people will still be able to search for permalinks and ?p= links!

@pfefferle
Copy link
Member Author

But it's overall more robust and does have the possibility of bypassing some caching layers, have there been any tests along those lines? Are we looking to address caching plugins, or Cloudflare, or?

Exactly! If a caching system would not be able to support Content-Negotiation, the user can simply disable the cache for ?p=*! Then the user well indeed no longer be able to search for the URL in Mastodon, but it would at least work!

@mattwiebe
Copy link
Contributor

This is definitely sounding like a good way to go. What if any are the downsides for existing sites having all of their IDs updated in this way?

@pfefferle
Copy link
Member Author

@mattwiebe I thought about storing a date, when the plugin updates to this version, to only change the ID for posts that are newer than that date. does that make sense?

@pfefferle
Copy link
Member Author

pfefferle commented Sep 17, 2024

Interesting point: https://mastodon.social/@by_caballero/113152023836926387

I'm more worried about the query parameter in an IRI than I am about the Move activity having its target on the same domain! I'm no JSON-LD wizard but I wonder if that's valid as a URI to stick in an actor.id, and even if it's valid, I'd worry other implementations botching it or dropping the ?... somehow, because of some unwitting behavior of underlying HTTP validation libraries or whatever

@pfefferle
Copy link
Member Author

What speaks against that is, that Comments (?c=) are working fine so far!

@mattwiebe
Copy link
Contributor

@mattwiebe I thought about storing a date, when the plugin updates to this version, to only change the ID for posts that are newer than that date. does that make sense?

Yup makes sense, probably even easier to just store the highest published post ID at the time of the plugin update?

@pfefferle
Copy link
Member Author

@Menrath I tested profiles and posts with misskey, mastodon, nodebb and friendi.ca and everything worked fine so far!

No issues with query params so far!

@pfefferle
Copy link
Member Author

@mattwiebe @akirk @jeherve @mediaformat @Menrath I would love to get this merged! Does anyone have any concerns or has anyone had positive/negative effects when testing?

@akirk
Copy link
Member

akirk commented Oct 21, 2024

I didn't test this but I am unsure how this should work.

For posts, it makes sense: As I understand the code you want to maintain the old full URL until the last post when migrating and from then on the URL with the post id.

But for authors, you're changing the ActivityPub id and maintaining it for old followers. So doest this mean that for Mastodon instances they will appear as two separate authors?

@akirk
Copy link
Member

akirk commented Oct 21, 2024

Oh, I realise because of aliases in Webfinger this should "just work."

@pfefferle
Copy link
Member Author

@akirk

But for authors, you're changing the ActivityPub id and maintaining it for old followers. So doest this mean that for Mastodon instances they will appear as two separate authors?

I check if the user has followers, if so I do not change anything, if not it uses the "new" id: https://github.com/Automattic/wordpress-activitypub/pull/894/files#diff-e2183e3284e3a7161c943a36d71aa3b1becf63ffe7ed41afa0b16d4c16944f2eR368 and https://github.com/Automattic/wordpress-activitypub/pull/894/files#diff-1d75cd772f6ce6eec7428f320fbc02e1829fbf3e96194d74c451beb246fc50e7R116

Copy link
Contributor

@mattwiebe mattwiebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ready! One way to find out. :shipit:

@pfefferle pfefferle merged commit 4a12d74 into trunk Oct 22, 2024
21 checks passed
@pfefferle pfefferle deleted the add/query-as-id branch October 22, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants