Big refactoring of the Inbox #443

elegaanz · 2019-02-11T16:23:33Z

We now have a type that routes an activity through the registered handlers
until one of them matches.

Each Actor/Activity/Object combination is represented by an implementation of AsObject

These combinations are then registered on the Inbox type, which will try to deserialize
the incoming activity in the requested types.

Advantages:

nicer syntax: the final API is clearer and more idiomatic
more generic: only two traits (AsActor and AsObject) instead of one for each kind of activity
we always try to dereference unknown objects
it is easier to see which activities we handle and which one we don't

(sorry for the big diff once again 😕)

We now have a type that routes an activity through the registered handlers until one of them matches. Each Actor/Activity/Object combination is represented by an implementation of AsObject These combinations are then registered on the Inbox type, which will try to deserialize the incoming activity in the requested types. Advantages: - nicer syntax: the final API is clearer and more idiomatic - more generic: only two traits (`AsActor` and `AsObject`) instead of one for each kind of activity - it is easier to see which activities we handle and which one we don't

codecov · 2019-02-11T17:37:25Z

Codecov Report

Merging #443 into master will increase coverage by 12.81%.
The diff coverage is 54.19%.

@@             Coverage Diff             @@
##           master     #443       +/-   ##
===========================================
+ Coverage   29.66%   42.48%   +12.81%     
===========================================
  Files          66       68        +2     
  Lines        7190     9314     +2124     
===========================================
+ Hits         2133     3957     +1824     
- Misses       5057     5357      +300

trinity-1686a

first batch of comments, I'll run some tests this evening or tomorrow

plume-common/src/activity_pub/inbox.rs

plume-models/src/blogs.rs

plume-models/src/lib.rs

plume-models/src/mentions.rs

And fix compilations issues that I didn't catch before ?

- Avoid panics - Don't search for AP ID infinitely - Code style issues

trinity-1686a

I'm trying to test each activities. Sending/receiving delete seems to not be working. Sending like/reshare/follow works, but not receiving them. I haven't tested some Undo because I wasn't able to create the related activity in the first place

src/routes/comments.rs

src/inbox.rs

src/routes/posts.rs

src/inbox.rs

elegaanz · 2019-03-13T15:01:09Z

Thanks for your review @fdb-hiroshima. I'll try to reproduce your issues and fix them, and maybe add tests for it in plume-models too.

It should be implemented for any AP object. It allows to look for an object in database using its AP ID, or to dereference it if it was not present in database Also moves the inbox code to plume-models to test it (and write a basic test for each activity type we handle)

plume-common/src/activity_pub/inbox.rs

plume-models/src/blogs.rs

Replace Context with PlumeRockets (and use it for the API context too while we are at it)

elegaanz · 2019-03-24T20:04:13Z

I fixed all the conflicts, and now instead of using Context we use PlumeRockets. I also deployed this branch to https://baptiste.gelez.xyz to make testing easier if needed.

igalic

weak 👀

plume-common/src/activity_pub/inbox.rs

plume-models/src/comments.rs

elegaanz · 2019-03-25T18:23:40Z

So I did some testing, and here are the results:

	Mastodon	Pleroma	Plume
Announce	✔️	✔️	✔️
Create Note	✔️	✔️	✔️
Create Article	-	-	✔️
Delete Note	✔️	✔️	✔️
Delete Article	-	-	✔️
Follow	✔️	✔️	✔️
Like	✔️	✔️	✔️
Undo Announce	✔️	Works if the Pleroma user is followed	✔️
Undo Follow	✔️	✔️	✔️
Undo Like	✔️	Works if the Pleroma user is followed	✔️
Update Article	-	-	✔️

~~The CW is not here~~
~~The notification is not deleted (see Delete notifications when deleting comments #499)~~
~~There is no error, but after refreshing the Mastodon page, the follow is not accepted (:hourglass: icon)~~

I will try to debug a bit more the errors I encountered.

igalic · 2019-03-25T19:21:50Z

it would be cool if we could add a service: mastodon and service: pleroma to Travis 😅

elegaanz · 2019-03-25T19:32:17Z

I think if we were on Gitlab we could just pull their Docker images… I really miss Gitlab CI.

trinity-1686a · 2019-03-25T19:37:50Z

can't we do it the ugly way with some bash script?

elegaanz · 2019-03-25T19:38:54Z

We could, but it would take a lot of time to install all the dependencies, and the CI already takes so long to run.

igalic · 2019-03-25T22:17:55Z

i think we're starting to discuss integration tests, and it's not really fruitful in this PR.

And validate blog names too

elegaanz · 2019-04-02T13:14:42Z

Update: I optimized the algorithm a little bit to avoid dereferencing the same object twice, fixed the space-in-username issue, and found why it was not possible to Undo Announce and Like from Pleroma. It was just not sending the activity to Plume because no one was following the Pleroma account on the Plume instance. It works fine if I follow my Pleroma account from Plume. I still created a bug report.

So I think this PR is ready for another review. 😄

plume-models/src/lib.rs

…x-refactor

trinity-1686a

👀

plume-common/src/activity_pub/inbox.rs

trinity-1686a

👀

trinity-1686a · 2019-04-07T17:26:52Z

plume-models/src/blogs.rs

+    fn from_activity(c: &PlumeRocket, acct: CustomGroup) -> Result<Self> {
+        let url = Url::parse(&acct.object.object_props.id_string()?)?;
+        let inst = url.host_str()?;
+        let instance = Instance::find_by_domain(&c.conn, inst).or_else(|_| {


out of scope for this pr, but we should consider a find_or_create of some kind instead of find_by_domain.or_else(insert), so to not tie to much code from one part of the model to another

plume-models/src/blogs.rs

plume-models/src/posts.rs

po/plume-front/en.po~

…n instead of map when possible

trinity-1686a · 2019-04-07T19:05:16Z

Federation with old Plume

Sending activities

~~Unsubscribing from a user does not seem to delete the entry in database~~
~~Sending comment don't seem to propagate CW (maybe bug from old Plume)~~ This is a bug from old Plume

Receiving

all right

Federation with itself

~~Unsubscribing from a user does not seem to delete the entry in database~~
~~Sending/Receiving Post Update does not seems to update post cover-picture~~

pleroma

Sending activities

boost (announce) post doesn't seems to work (unboost untested)
follow does not seems to work, but it might be my pleroma having problems (unfollow untested)

Receiving

again issues with follow, but again maybe my pleroma

plume-models/src/posts.rs

trinity-1686a · 2019-04-07T19:51:58Z

#443 (comment) by the way if we manage to get some certificate we can use in tests, this is now probably possible with circleci

and fix a typo in a constant name

elegaanz · 2019-04-16T14:41:24Z

@fdb-hiroshima The issues with Pleroma are normally fixed now.

trinity-1686a

🚀

elegaanz · 2019-04-17T17:31:32Z

Yay, Finally! Thank you @igalic and @fdb-hiroshima for reviewing this big PR! 😊

elegaanz added C: Enhancement New feature or request A: Federation Stuff related to Federation S: Ready for review This PR is ready to be reviewed A: Backend Code running on the server labels Feb 11, 2019

trinity-1686a reviewed Mar 4, 2019

View reviewed changes

elegaanz added 3 commits March 4, 2019 22:43

Merge branch 'master' into inbox-refactor

a4fa291

And fix compilations issues that I didn't catch before ?

Small fixes

bc4b239

- Avoid panics - Don't search for AP ID infinitely - Code style issues

Fix tests

4615f65

elegaanz added this to the Alpha 2 milestone Mar 12, 2019

trinity-1686a reviewed Mar 13, 2019

View reviewed changes

elegaanz added 2 commits March 13, 2019 20:04

Merge branch 'master' into inbox-refactor

b420cc7

igalic reviewed Mar 16, 2019

View reviewed changes

plume-common/src/activity_pub/inbox.rs Outdated Show resolved Hide resolved

plume-common/src/activity_pub/inbox.rs Outdated Show resolved Hide resolved

plume-common/src/activity_pub/inbox.rs Show resolved Hide resolved

plume-models/src/blogs.rs Show resolved Hide resolved

elegaanz added 2 commits March 16, 2019 15:26

Use if let instead of match

2a5fa53

Merge branch 'master' into inbox-refactor

0df7326

Replace Context with PlumeRockets (and use it for the API context too while we are at it)

Don't require PlumeRocket::intl for tests

fe0da84

igalic reviewed Mar 24, 2019

View reviewed changes

plume-common/src/activity_pub/inbox.rs Outdated Show resolved Hide resolved

plume-models/src/comments.rs Outdated Show resolved Hide resolved

plume-models/src/comments.rs Outdated Show resolved Hide resolved

elegaanz added 2 commits March 24, 2019 21:52

Return early and remove a forgotten dbg!

074da71

Merge branch 'master' into inbox-refactor

d1ef5f1

Merge branch 'master' into inbox-refactor

98ae386

elegaanz added 4 commits March 31, 2019 19:18

Add spaces to characters that should not be in usernames

2a1ceb0

And validate blog names too

Smarter dereferencing: only do it once for each actor/object

8df0507

Forgot some files

3f8e7b5

Merge branch 'master' into inbox-refactor

15fba89

Cargo fmt

f145749

igalic reviewed Apr 2, 2019

View reviewed changes

plume-models/src/lib.rs Outdated Show resolved Hide resolved

elegaanz added 4 commits April 6, 2019 19:20

Delete plume_test

93c54bb

Delete plume_tests

33da60d

Merge branch 'master' into inbox-refactor

2eedaa6

Merge branch 'inbox-refactor' of github.com:Plume-org/Plume into inbo…

a606245

…x-refactor

trinity-1686a reviewed Apr 6, 2019

View reviewed changes

plume-common/src/activity_pub/inbox.rs Outdated Show resolved Hide resolved

plume-common/src/activity_pub/inbox.rs Show resolved Hide resolved

elegaanz added 2 commits April 7, 2019 09:40

Update get_id docs + Remove useless : Sized

aad16b2

Appease cargo fmt

916be0d

trinity-1686a reviewed Apr 7, 2019

View reviewed changes

elegaanz added 2 commits April 7, 2019 19:31

Remove dbg! + Use as_ref instead of clone when possible + Use and_the…

e5fac47

…n instead of map when possible

Remove .po~

1dc0f4a

trinity-1686a reviewed Apr 7, 2019

View reviewed changes

plume-models/src/posts.rs Show resolved Hide resolved

trinity-1686a and others added 3 commits April 9, 2019 19:28

send unfollow to local instance

954d75c

read cover from update activity

4c50477

Make sure "cc" and "to" are never empty

4b2bc90

and fix a typo in a constant name

Cargo fmt

aeb8ecd

trinity-1686a mentioned this pull request Apr 17, 2019

Add support for remote interact #519

Merged

6 tasks

trinity-1686a approved these changes Apr 17, 2019

View reviewed changes

elegaanz merged commit 12efe72 into master Apr 17, 2019

elegaanz deleted the inbox-refactor branch April 17, 2019 17:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Big refactoring of the Inbox #443

Big refactoring of the Inbox #443

elegaanz commented Feb 11, 2019 •

edited

codecov bot commented Feb 11, 2019 •

edited

trinity-1686a left a comment

trinity-1686a left a comment •

edited by igalic

elegaanz commented Mar 13, 2019

elegaanz commented Mar 24, 2019

igalic left a comment

elegaanz commented Mar 25, 2019 •

edited

igalic commented Mar 25, 2019

elegaanz commented Mar 25, 2019

trinity-1686a commented Mar 25, 2019

elegaanz commented Mar 25, 2019

igalic commented Mar 25, 2019

elegaanz commented Apr 2, 2019 •

edited

trinity-1686a left a comment

trinity-1686a left a comment

trinity-1686a Apr 7, 2019

trinity-1686a commented Apr 7, 2019 •

edited by elegaanz

trinity-1686a commented Apr 7, 2019 •

edited

elegaanz commented Apr 16, 2019

trinity-1686a left a comment

elegaanz commented Apr 17, 2019

Big refactoring of the Inbox #443

Big refactoring of the Inbox #443

Conversation

elegaanz commented Feb 11, 2019 • edited

codecov bot commented Feb 11, 2019 • edited

Codecov Report

trinity-1686a left a comment

Choose a reason for hiding this comment

trinity-1686a left a comment • edited by igalic

Choose a reason for hiding this comment

elegaanz commented Mar 13, 2019

elegaanz commented Mar 24, 2019

igalic left a comment

Choose a reason for hiding this comment

elegaanz commented Mar 25, 2019 • edited

igalic commented Mar 25, 2019

elegaanz commented Mar 25, 2019

trinity-1686a commented Mar 25, 2019

elegaanz commented Mar 25, 2019

igalic commented Mar 25, 2019

elegaanz commented Apr 2, 2019 • edited

trinity-1686a left a comment

Choose a reason for hiding this comment

trinity-1686a left a comment

Choose a reason for hiding this comment

trinity-1686a Apr 7, 2019

Choose a reason for hiding this comment

trinity-1686a commented Apr 7, 2019 • edited by elegaanz

Federation with old Plume

Sending activities

Receiving

Federation with itself

pleroma

Sending activities

Receiving

trinity-1686a commented Apr 7, 2019 • edited

elegaanz commented Apr 16, 2019

trinity-1686a left a comment

Choose a reason for hiding this comment

elegaanz commented Apr 17, 2019

elegaanz commented Feb 11, 2019 •

edited

codecov bot commented Feb 11, 2019 •

edited

trinity-1686a left a comment •

edited by igalic

elegaanz commented Mar 25, 2019 •

edited

elegaanz commented Apr 2, 2019 •

edited

trinity-1686a commented Apr 7, 2019 •

edited by elegaanz

trinity-1686a commented Apr 7, 2019 •

edited