Refactor inbox, simplify and split into multiple files #72
Loading…
Reference in New Issue
No description provided.
Delete Branch "inbox-refactoring"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Need to figure out how to convert activities to the proper type, same way as asonix' ap-relay does. Unfortunately that repo is down...
Okay this is (mostly) working now. Tests are passing, but votes are not working because of what seems to be a bug in our code. Essentially,
activitystreams
now checks the domain of the actor field against the domain of the ID field (for security reasons). But this check fails when upvoting a remote post, because we use the community ID for creating the ID of the Like activity. So if a user on instance alpha upvotes a post in a community on instance beta, the upvote activity will have different domains in the actor and id fields which is not supposed to happen. The problem is here, but probably also in other places. Hope this makes sense. Its probably better to fix that in a separate PR, as it will require a general rework of activity ID generation.Besides, I havent tested all federation functionality, so there may be more problems.
Edit: To be clear, this domain check was added in a new activitystreams version, I think we just never tested that particular functionality since then. So it has nothing to do with this PR. I think the proper fix would be to add a helper function that generates activity IDs like
https://local-domain/activity/create/uuid
.Hrm.... I spose its no problem to use the actor domain for all the id generations.
I'll test and merge this PR in a min, and also add a unit test for federated like, to make sure that it does fail.
WIP: Refactor inbox, simplify and split into multiple filesto Refactor inbox, simplify and split into multiple filesSry this took me a bit, I adding the unit test now.
I'm getting a lot of these errors btw:
Seems to be trying to send or fetch something to itself too:
Okay the unit test is done, run
git merge inbox-refactoring-dessalines
Okay I fixed a bug in the community fetching (the one in your second log, have to remove the
/followers
at the end so its the proper community ID and can be found in the DB). I also renamed the fetch_remote_xxx methods because we actually use them for local objects as well.Also I disabled the ID check in FromApub by using
id_insecure()
, until we figure out how to do that properly.Not sure about the missing field error, can you tell me how to reproduce that?
@ -115,3 +121,1 @@
self.updated,
NoteType::Note.to_string(),
)
create_tombstone(self.deleted, &self.ap_id, self.updated, NoteType::Note)
This is nice that we don't have to use the string types anymore.
@ -179,3 +182,3 @@
updated: note.updated().map(|u| u.to_owned().naive_local()),
deleted: None,
ap_id: note.id(actor_id.domain().unwrap())?.unwrap().to_string(),
ap_id: note.id_unchecked().unwrap().to_string(),
Did you wanna try to fix our id generation in this ticket? Or just leave a TODO for later.
Maybe seeing the full object would help me think about this.
@ -372,3 +361,1 @@
.id(actor_id.domain().unwrap())?
.unwrap()
.to_string(),
actor_id: group.inner.id_unchecked().unwrap().to_string(),
Might be good to add a TODO here also.
@ -0,0 +42,4 @@
dbg!(create.object().as_single_kind_str());
match create.object().as_single_kind_str() {
Some("Page") => receive_create_post(create, client, pool, chat_server).await,
Some("Note") => receive_create_comment(create, client, pool, chat_server).await,
This looks good, much better organized.
@ -69,2 +71,4 @@
Ok(())
}
pub(in crate::apub) fn generate_activity_id<T>(kind: T) -> Result<Url, ParseError>
I still need to see the full objects to make sense of this, but if I'm getting this right, in the example of liking a federated comment:
To me I don't see why the domain of the activity needs to match the domain of the object. I'm gonna bring asonix into this.