From: nutomic Date: Thu, 5 Nov 2020 20:19:06 +0000 (+0000) Subject: Fix federation of community removal/deletion, added docs (#125) X-Git-Url: http://these/git/%7B%60%24%7BwebArchiveUrl%7D/%22%7B%7D/%24%7B%60data:application/%22https:/hacktivis.me/%7BpictrsAvatarThumbnail%28this.props.site.site.icon%29%7D?a=commitdiff_plain;h=b7d2dac9bfaa29cbc5fd7a9eb25f239cfb2bc52b;p=lemmy.git Fix federation of community removal/deletion, added docs (#125) Adding a federation test for community deletes / removes. Add missing docs for community deletion/removal (fixes #1250) Fix federation of community deletion/removal (fixes #1253) Co-authored-by: Dessalines Co-authored-by: Felix Ableitner Reviewed-on: https://yerbamate.dev/LemmyNet/lemmy/pulls/125 --- diff --git a/api_tests/src/community.spec.ts b/api_tests/src/community.spec.ts index e01d673d..7c33f82f 100644 --- a/api_tests/src/community.spec.ts +++ b/api_tests/src/community.spec.ts @@ -8,7 +8,10 @@ import { createCommunity, deleteCommunity, removeCommunity, + getCommunity, + followCommunity, delay, + longDelay, } from './shared'; import { Community, @@ -55,6 +58,21 @@ test('Create community', async () => { test('Delete community', async () => { let communityRes = await createCommunity(beta); await delay(); + + // Cache the community on Alpha + let searchShort = `!${communityRes.community.name}@lemmy-beta:8551`; + let search = await searchForCommunity(alpha, searchShort); + let communityOnAlpha = search.communities[0]; + assertCommunityFederation(communityOnAlpha, communityRes.community); + await delay(); + + // Follow the community from alpha + let follow = await followCommunity(alpha, true, communityOnAlpha.id); + + // Make sure the follow response went through + expect(follow.community.local).toBe(false); + await delay(); + let deleteCommunityRes = await deleteCommunity( beta, true, @@ -64,11 +82,9 @@ test('Delete community', async () => { await delay(); // Make sure it got deleted on A - let search = await searchForBetaCommunity(alpha); - let communityA = search.communities[0]; - // TODO this fails currently, because no updates are pushed - // expect(communityA.deleted).toBe(true); - // assertCommunityFederation(communityA, communityRes.community); + let communityOnAlphaDeleted = await getCommunity(alpha, communityOnAlpha.id); + expect(communityOnAlphaDeleted.community.deleted).toBe(true); + await delay(); // Undelete let undeleteCommunityRes = await deleteCommunity( @@ -80,29 +96,39 @@ test('Delete community', async () => { await delay(); // Make sure it got undeleted on A - let search2 = await searchForBetaCommunity(alpha); - let communityA2 = search2.communities[0]; - // TODO this fails currently, because no updates are pushed - // expect(communityA2.deleted).toBe(false); - // assertCommunityFederation(communityA2, undeleteCommunityRes.community); + let communityOnAlphaUnDeleted = await getCommunity(alpha, communityOnAlpha.id); + expect(communityOnAlphaUnDeleted.community.deleted).toBe(false); }); test('Remove community', async () => { let communityRes = await createCommunity(beta); await delay(); + + // Cache the community on Alpha + let searchShort = `!${communityRes.community.name}@lemmy-beta:8551`; + let search = await searchForCommunity(alpha, searchShort); + let communityOnAlpha = search.communities[0]; + assertCommunityFederation(communityOnAlpha, communityRes.community); + await delay(); + + // Follow the community from alpha + let follow = await followCommunity(alpha, true, communityOnAlpha.id); + + // Make sure the follow response went through + expect(follow.community.local).toBe(false); + await delay(); + let removeCommunityRes = await removeCommunity( beta, true, communityRes.community.id ); expect(removeCommunityRes.community.removed).toBe(true); + await delay(); - // Make sure it got removed on A - let search = await searchForBetaCommunity(alpha); - let communityA = search.communities[0]; - // TODO this fails currently, because no updates are pushed - // expect(communityA.removed).toBe(true); - // assertCommunityFederation(communityA, communityRes.community); + // Make sure it got Removed on A + let communityOnAlphaRemoved = await getCommunity(alpha, communityOnAlpha.id); + expect(communityOnAlphaRemoved.community.removed).toBe(true); await delay(); // unremove @@ -114,15 +140,18 @@ test('Remove community', async () => { expect(unremoveCommunityRes.community.removed).toBe(false); await delay(); - // Make sure it got unremoved on A - let search2 = await searchForBetaCommunity(alpha); - let communityA2 = search2.communities[0]; - // TODO this fails currently, because no updates are pushed - // expect(communityA2.removed).toBe(false); - // assertCommunityFederation(communityA2, unremoveCommunityRes.community); + // Make sure it got undeleted on A + let communityOnAlphaUnRemoved = await getCommunity(alpha, communityOnAlpha.id); + expect(communityOnAlphaUnRemoved.community.removed).toBe(false); }); test('Search for beta community', async () => { - let search = await searchForBetaCommunity(alpha); - expect(search.communities[0].name).toBe('main'); + let communityRes = await createCommunity(beta); + expect(communityRes.community.name).toBeDefined(); + await delay(); + + let searchShort = `!${communityRes.community.name}@lemmy-beta:8551`; + let search = await searchForCommunity(alpha, searchShort); + let communityOnAlpha = search.communities[0]; + assertCommunityFederation(communityOnAlpha, communityRes.community); }); diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 6aa5cfc4..3c9cc5d8 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -20,6 +20,7 @@ import { RemoveCommentForm, SearchForm, CommentResponse, + GetCommunityForm, CommunityForm, DeleteCommunityForm, RemoveCommunityForm, @@ -402,6 +403,16 @@ export async function createCommunity( return api.client.createCommunity(form); } +export async function getCommunity( + api: API, + id: number, +): Promise { + let form: GetCommunityForm = { + id, + }; + return api.client.getCommunity(form); +} + export async function deleteCommunity( api: API, deleted: boolean, diff --git a/docker/federation/start-local-instances.bash b/docker/federation/start-local-instances.bash index 6814fdd7..3c374e37 100755 --- a/docker/federation/start-local-instances.bash +++ b/docker/federation/start-local-instances.bash @@ -8,4 +8,14 @@ for Item in alpha beta gamma delta epsilon ; do sudo chown -R 991:991 volumes/pictrs_$Item done -sudo docker-compose up +sudo docker-compose up -d + +echo "Waiting for Lemmy to start..." +while [[ "$(curl -s -o /dev/null -w '%{http_code}' 'localhost:8541/api/v1/site')" != "200" ]]; do sleep 1; done +while [[ "$(curl -s -o /dev/null -w '%{http_code}' 'localhost:8551/api/v1/site')" != "200" ]]; do sleep 1; done +while [[ "$(curl -s -o /dev/null -w '%{http_code}' 'localhost:8561/api/v1/site')" != "200" ]]; do sleep 1; done +while [[ "$(curl -s -o /dev/null -w '%{http_code}' 'localhost:8571/api/v1/site')" != "200" ]]; do sleep 1; done +while [[ "$(curl -s -o /dev/null -w '%{http_code}' 'localhost:8581/api/v1/site')" != "200" ]]; do sleep 1; done +echo "All instances started." + +sudo docker-compose logs -f diff --git a/docs/src/contributing_apub_api_outline.md b/docs/src/contributing_apub_api_outline.md index 0556e484..f5c7a595 100644 --- a/docs/src/contributing_apub_api_outline.md +++ b/docs/src/contributing_apub_api_outline.md @@ -559,6 +559,56 @@ Sent to: User |---|---|---| | `object` | yes | Any `Create`, `Update`, `Like`, `Dislike`, `Delete` `Remove` or `Undo` activity as described above | +### Remove or Delete Community + +```json +{ + "@context": "https://www.w3.org/ns/activitystreams", + "id": "http://ds9.lemmy.ml/activities/remove/e4ca7688-af9d-48b7-864f-765e7f9f3591", + "type": "Remove", + "actor": "http://ds9.lemmy.ml/c/some_community", + "cc": [ + "http://ds9.lemmy.ml/c/some_community/followers" + ], + "to": "https://www.w3.org/ns/activitystreams#Public", + "object": "http://ds9.lemmy.ml/c/some_community" +} +``` + +| Field Name | Mandatory | Description | +|---|---|---| +| `type` | yes | Either `Remove` or `Delete` | + +### Restore Removed or Deleted Community + +```json +{ + "@context": "https://www.w3.org/ns/activitystreams", + "id": "http://ds9.lemmy.ml/activities/like/0703668c-8b09-4a85-aa7a-f93621936901", + "type": "Undo", + "actor": "http://ds9.lemmy.ml/c/some_community", + "to": "https://www.w3.org/ns/activitystreams#Public", + "cc": [ + "http://ds9.lemmy.ml/c/testcom/followers" + ], + "object": { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "http://ds9.lemmy.ml/activities/remove/1062b5e0-07e8-44fc-868c-854209935bdd", + "type": "Remove", + "actor": "http://ds9.lemmy.ml/c/some_community", + "object": "http://ds9.lemmy.ml/c/testcom", + "to": "https://www.w3.org/ns/activitystreams#Public", + "cc": [ + "http://ds9.lemmy.ml/c/testcom/followers" + ] + } +} + +``` +| Field Name | Mandatory | Description | +|---|---|---| +| `object.type` | yes | Either `Remove` or `Delete` | + ### Create or Update Private message Creates a new private message between two users. diff --git a/lemmy_api/src/community.rs b/lemmy_api/src/community.rs index 4149053f..4c9d769b 100644 --- a/lemmy_api/src/community.rs +++ b/lemmy_api/src/community.rs @@ -327,9 +327,9 @@ impl Perform for DeleteCommunity { // Send apub messages if deleted { - updated_community.send_delete(&user, context).await?; + updated_community.send_delete(context).await?; } else { - updated_community.send_undo_delete(&user, context).await?; + updated_community.send_undo_delete(context).await?; } let edit_id = data.edit_id; @@ -395,9 +395,9 @@ impl Perform for RemoveCommunity { // Apub messages if removed { - updated_community.send_remove(&user, context).await?; + updated_community.send_remove(context).await?; } else { - updated_community.send_undo_remove(&user, context).await?; + updated_community.send_undo_remove(context).await?; } let edit_id = data.edit_id; diff --git a/lemmy_apub/src/activities/send/community.rs b/lemmy_apub/src/activities/send/community.rs index 446ce6f3..cdcc9670 100644 --- a/lemmy_apub/src/activities/send/community.rs +++ b/lemmy_apub/src/activities/send/community.rs @@ -4,7 +4,6 @@ use crate::{ check_is_apub_id_valid, fetcher::get_or_fetch_and_upsert_user, ActorType, - ToApub, }; use activitystreams::{ activity::{ @@ -23,7 +22,7 @@ use activitystreams::{ }; use anyhow::Context; use itertools::Itertools; -use lemmy_db::{community::Community, community_view::CommunityFollowerView, user::User_, DbPool}; +use lemmy_db::{community::Community, community_view::CommunityFollowerView, DbPool}; use lemmy_structs::blocking; use lemmy_utils::{location_info, settings::Settings, LemmyError}; use lemmy_websocket::LemmyContext; @@ -85,10 +84,8 @@ impl ActorType for Community { } /// If the creator of a community deletes the community, send this to all followers. - async fn send_delete(&self, creator: &User_, context: &LemmyContext) -> Result<(), LemmyError> { - let group = self.to_apub(context.pool()).await?; - - let mut delete = Delete::new(creator.actor_id.to_owned(), group.into_any_base()?); + async fn send_delete(&self, context: &LemmyContext) -> Result<(), LemmyError> { + let mut delete = Delete::new(self.actor_id()?, self.actor_id()?); delete .set_context(activitystreams::context()) .set_id(generate_activity_id(DeleteType::Delete)?) @@ -100,21 +97,15 @@ impl ActorType for Community { } /// If the creator of a community reverts the deletion of a community, send this to all followers. - async fn send_undo_delete( - &self, - creator: &User_, - context: &LemmyContext, - ) -> Result<(), LemmyError> { - let group = self.to_apub(context.pool()).await?; - - let mut delete = Delete::new(creator.actor_id.to_owned(), group.into_any_base()?); + async fn send_undo_delete(&self, context: &LemmyContext) -> Result<(), LemmyError> { + let mut delete = Delete::new(self.actor_id()?, self.actor_id()?); delete .set_context(activitystreams::context()) .set_id(generate_activity_id(DeleteType::Delete)?) .set_to(public()) .set_many_ccs(vec![self.get_followers_url()?]); - let mut undo = Undo::new(creator.actor_id.to_owned(), delete.into_any_base()?); + let mut undo = Undo::new(self.actor_id()?, delete.into_any_base()?); undo .set_context(activitystreams::context()) .set_id(generate_activity_id(UndoType::Undo)?) @@ -126,8 +117,8 @@ impl ActorType for Community { } /// If an admin removes a community, send this to all followers. - async fn send_remove(&self, mod_: &User_, context: &LemmyContext) -> Result<(), LemmyError> { - let mut remove = Remove::new(mod_.actor_id.to_owned(), self.actor_id()?); + async fn send_remove(&self, context: &LemmyContext) -> Result<(), LemmyError> { + let mut remove = Remove::new(self.actor_id()?, self.actor_id()?); remove .set_context(activitystreams::context()) .set_id(generate_activity_id(RemoveType::Remove)?) @@ -139,8 +130,8 @@ impl ActorType for Community { } /// If an admin reverts the removal of a community, send this to all followers. - async fn send_undo_remove(&self, mod_: &User_, context: &LemmyContext) -> Result<(), LemmyError> { - let mut remove = Remove::new(mod_.actor_id.to_owned(), self.actor_id()?); + async fn send_undo_remove(&self, context: &LemmyContext) -> Result<(), LemmyError> { + let mut remove = Remove::new(self.actor_id()?, self.actor_id()?); remove .set_context(activitystreams::context()) .set_id(generate_activity_id(RemoveType::Remove)?) @@ -148,7 +139,7 @@ impl ActorType for Community { .set_many_ccs(vec![self.get_followers_url()?]); // Undo that fake activity - let mut undo = Undo::new(mod_.actor_id.to_owned(), remove.into_any_base()?); + let mut undo = Undo::new(self.actor_id()?, remove.into_any_base()?); undo .set_context(activitystreams::context()) .set_id(generate_activity_id(LikeType::Like)?) diff --git a/lemmy_apub/src/activities/send/user.rs b/lemmy_apub/src/activities/send/user.rs index c4d287a5..bd791e5e 100644 --- a/lemmy_apub/src/activities/send/user.rs +++ b/lemmy_apub/src/activities/send/user.rs @@ -94,27 +94,19 @@ impl ActorType for User_ { unimplemented!() } - async fn send_delete(&self, _creator: &User_, _context: &LemmyContext) -> Result<(), LemmyError> { + async fn send_delete(&self, _context: &LemmyContext) -> Result<(), LemmyError> { unimplemented!() } - async fn send_undo_delete( - &self, - _creator: &User_, - _context: &LemmyContext, - ) -> Result<(), LemmyError> { + async fn send_undo_delete(&self, _context: &LemmyContext) -> Result<(), LemmyError> { unimplemented!() } - async fn send_remove(&self, _creator: &User_, _context: &LemmyContext) -> Result<(), LemmyError> { + async fn send_remove(&self, _context: &LemmyContext) -> Result<(), LemmyError> { unimplemented!() } - async fn send_undo_remove( - &self, - _creator: &User_, - _context: &LemmyContext, - ) -> Result<(), LemmyError> { + async fn send_undo_remove(&self, _context: &LemmyContext) -> Result<(), LemmyError> { unimplemented!() } diff --git a/lemmy_apub/src/fetcher.rs b/lemmy_apub/src/fetcher.rs index 43466a0f..ff3b03d3 100644 --- a/lemmy_apub/src/fetcher.rs +++ b/lemmy_apub/src/fetcher.rs @@ -251,10 +251,14 @@ pub(crate) async fn get_or_fetch_and_upsert_user( Ok(u) if !u.local && should_refetch_actor(u.last_refreshed_at) => { debug!("Fetching and updating from remote user: {}", apub_id); let person = - fetch_remote_object::(context.client(), apub_id, recursion_counter).await?; + fetch_remote_object::(context.client(), apub_id, recursion_counter).await; + // If fetching failed, return the existing data. + if person.is_err() { + return Ok(u); + } let mut uf = UserForm::from_apub( - &person, + &person?, context, Some(apub_id.to_owned()), recursion_counter, @@ -320,7 +324,7 @@ pub(crate) async fn get_or_fetch_and_upsert_community( match community { Ok(c) if !c.local && should_refetch_actor(c.last_refreshed_at) => { debug!("Fetching and updating from remote community: {}", apub_id); - fetch_remote_community(apub_id, context, Some(c.id), recursion_counter).await + fetch_remote_community(apub_id, context, Some(c), recursion_counter).await } Ok(c) => Ok(c), Err(NotFound {}) => { @@ -331,17 +335,24 @@ pub(crate) async fn get_or_fetch_and_upsert_community( } } -/// Request a community by apub ID from a remote instance, including moderators. If `community_id`, +/// Request a community by apub ID from a remote instance, including moderators. If `old_community`, /// is set, this is an update for a community which is already known locally. If not, we don't know /// the community yet and also pull the outbox, to get some initial posts. async fn fetch_remote_community( apub_id: &Url, context: &LemmyContext, - community_id: Option, + old_community: Option, recursion_counter: &mut i32, ) -> Result { - let group = fetch_remote_object::(context.client(), apub_id, recursion_counter).await?; + let group = fetch_remote_object::(context.client(), apub_id, recursion_counter).await; + // If fetching failed, return the existing data. + if let Some(ref c) = old_community { + if group.is_err() { + return Ok(c.to_owned()); + } + } + let group = group?; let cf = CommunityForm::from_apub(&group, context, Some(apub_id.to_owned()), recursion_counter).await?; let community = blocking(context.pool(), move |conn| Community::upsert(conn, &cf)).await??; @@ -364,7 +375,7 @@ async fn fetch_remote_community( } // TODO: need to make this work to update mods of existing communities - if community_id.is_none() { + if old_community.is_none() { let community_id = community.id; blocking(context.pool(), move |conn| { for mod_ in creator_and_moderators { diff --git a/lemmy_apub/src/lib.rs b/lemmy_apub/src/lib.rs index 4449c94f..e7410ee2 100644 --- a/lemmy_apub/src/lib.rs +++ b/lemmy_apub/src/lib.rs @@ -189,15 +189,11 @@ pub trait ActorType { context: &LemmyContext, ) -> Result<(), LemmyError>; - async fn send_delete(&self, creator: &User_, context: &LemmyContext) -> Result<(), LemmyError>; - async fn send_undo_delete( - &self, - creator: &User_, - context: &LemmyContext, - ) -> Result<(), LemmyError>; + async fn send_delete(&self, context: &LemmyContext) -> Result<(), LemmyError>; + async fn send_undo_delete(&self, context: &LemmyContext) -> Result<(), LemmyError>; - async fn send_remove(&self, mod_: &User_, context: &LemmyContext) -> Result<(), LemmyError>; - async fn send_undo_remove(&self, mod_: &User_, context: &LemmyContext) -> Result<(), LemmyError>; + async fn send_remove(&self, context: &LemmyContext) -> Result<(), LemmyError>; + async fn send_undo_remove(&self, context: &LemmyContext) -> Result<(), LemmyError>; async fn send_announce( &self,