babka/mastodon-babka
2
0
Fork 0
Code Issues 15 Pull requests 10 Projects Releases 4 Packages Wiki Activity Actions

Resolve "Add Webhook at FeedManager.filter? for filtering statuses from users' timelines" #40

Open
CSDUMMI wants to merge 5 commits from 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines into babka
pull from: 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
merge into: babka:babka
Conversation 17 Commits 5 Files changed 3 +80 -1
CSDUMMI commented 2024-02-23 10:11:56 +00:00 (Migrated from gitlab.com)
Copy link

Closes #9

Closes #9
CSDUMMI commented 2024-02-23 11:00:16 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • 442246c4 - Invoke a webhook in FeedManager.filter?

Compare with previous version

added 1 commit <ul><li>442246c4 - Invoke a webhook in FeedManager.filter?</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=934220625&start_sha=1053ae25299576ccf8abeaa868023fc298b8381e)
CSDUMMI commented 2024-03-20 09:48:32 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • 53621335 - Invoke a webhook in FeedManager.filter?

Compare with previous version

added 1 commit <ul><li>53621335 - Invoke a webhook in FeedManager.filter?</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=958698286&start_sha=442246c4b6b8daaef8f20a6627f512cd60157a25)
CSDUMMI commented 2024-03-22 17:13:05 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • fee80c51 - Invoke a webhook in FeedManager.filter containing the status id and account id to filter for.

Compare with previous version

added 1 commit <ul><li>fee80c51 - Invoke a webhook in FeedManager.filter containing the status id and account id to filter for.</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=961899375&start_sha=536213355e224a2f3bfdca61ff4f6974b170dcce)
CSDUMMI commented 2024-03-22 17:14:07 +00:00 (Migrated from gitlab.com)
Copy link

The code in call_webhook works. But in testing it on a staging server, it does not seem to be invoked at any point that I can identify.

The code in call_webhook works. But in testing it on a staging server, it does not seem to be invoked at any point that I can identify.
CSDUMMI commented 2024-03-22 19:42:52 +00:00 (Migrated from gitlab.com)
Copy link

To summarize the current state of my experimentation and development on this MR:

  1. FeedManager.filter? is called by the FeedInsertWorker (and NotifyService). The FeedInsertWorker is a sidekiq job that checks if a status should be delivered to a user and if FeedManager.filter returns false, the status is added to a sorted set in redis for that specific timeline type and user (key: feed:home:<user id> for example).
  2. These sorted sets are used when generating the home feed, list feeds. Direct Feeds are still supported and use this same method, but have been deprecated and replaced by the conversation API.
  3. The public and tag timeline do not use redis sorted sets and instead load their contents directly from the database. To invoke the same webhook for these timelines an HTTP request would have to be made within the request-response loop of the API call, which could lead to delays. This is also the API that is used to present the local feed.
  4. The conversations API (app/controllers/api/v1/conversations_controller.rb:paginate_conversations) also appears to be reading directly from a database table called AccountConversation. Thus a call to the webhook would have to occur in the controller here as well. This is a more difficult problem than with 3, because conversations and direct messages are the way that spam was delivered during the last few spam waves and it is thus vital that we are able to filter these messages as well.

TL;DR: This MR works for the Home and List feed, but not for the public, local, tag feed or direct messages. These later four cases would have to invoke the webhook inside the controller, which leads to performance problems (though these could be reduced by using Mastodon's own redis cache).

To summarize the current state of my experimentation and development on this MR: 1. FeedManager.filter? is called by the `FeedInsertWorker` (and `NotifyService`). The `FeedInsertWorker` is a sidekiq job that checks if a status should be delivered to a user and if FeedManager.filter returns false, the status is added to a sorted set in redis for that specific timeline type and user (key: `feed:home:<user id>` for example). 2. These sorted sets are used when generating the home feed, list feeds. Direct Feeds are still supported and use this same method, but have been deprecated and replaced by the conversation API. 3. The public and tag timeline do not use redis sorted sets and instead load their contents directly from the database. To invoke the same webhook for these timelines an HTTP request would have to be made within the request-response loop of the API call, which could lead to delays. This is also the API that is used to present the local feed. 4. The conversations API (app/controllers/api/v1/conversations_controller.rb:paginate_conversations) also appears to be reading directly from a database table called `AccountConversation`. Thus a call to the webhook would have to occur in the controller here as well. This is a more difficult problem than with 3, because conversations and direct messages are the way that spam was delivered during the last few spam waves and it is thus vital that we are able to filter these messages as well. TL;DR: This MR works for the Home and List feed, but not for the public, local, tag feed or direct messages. These later four cases would have to invoke the webhook inside the controller, which leads to performance problems (though these could be reduced by using Mastodon's own redis cache).
CSDUMMI commented 2024-03-23 14:35:00 +00:00 (Migrated from gitlab.com)
Copy link

Upon reflection, I think it might be possible to invoke the webhook when an AccountConversation object is created (all relevant information should already be available) and only create the AccountConversation if the webhook succeeds.

Upon reflection, I think it might be possible to invoke the webhook when an `AccountConversation` object is created (all relevant information should already be available) and only create the `AccountConversation` if the webhook succeeds.
CSDUMMI commented 2024-03-23 15:34:08 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • d02a9376 - Invoke a webhook in FeedManager.filter containing the status id and account id to filter for

Compare with previous version

added 1 commit <ul><li>d02a9376 - Invoke a webhook in FeedManager.filter containing the status id and account id to filter for</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=962230336&start_sha=fee80c5187d0aa79a92e6d000d6709c1b7c8144d)
CSDUMMI commented 2024-03-23 15:45:56 +00:00 (Migrated from gitlab.com)
Copy link

I can report that this MR works on the testing instance for the:

  • Home Feed
  • List Feed
  • Direct Message Feed
  • Notifications

And does not work for:

  • Local Feed
  • Federated Feed
  • Other Servers Feed

These last three use the /api/v1/timelines/public with different parameters and directly query the SQL database. To implement the webhook for these feeds a request would have to be made from within the controller for every status that is being returned (defaults to 20 statuses to check per API call, 20 webhook calls per API call).

The question for the next meeting is thus:
Should we try to find a work-around for the public timelines or deploy this MR as is with Wispwot to babka.social?

I can report that this MR works on the testing instance for the: - [x] Home Feed - [x] List Feed - [x] Direct Message Feed - [x] Notifications And does not work for: - [ ] Local Feed - [ ] Federated Feed - [ ] Other Servers Feed These last three use the [`/api/v1/timelines/public`](https://gitlab.com/babka_net/mastodon-babka/-/blob/babka/app/controllers/api/v1/timelines/public_controller.rb?ref_type=heads) with different parameters and directly query the SQL database. To implement the webhook for these feeds a request would have to be made from within the controller for every status that is being returned (defaults to 20 statuses to check per API call, 20 webhook calls per API call). The question for the next meeting is thus: Should we try to find a work-around for the public timelines or deploy this MR as is with Wispwot to babka.social?
CSDUMMI commented 2024-03-29 08:11:22 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • dd8e2a26 - Add CI job to run rspec unit tests

Compare with previous version

added 1 commit <ul><li>dd8e2a26 - Add CI job to run rspec unit tests</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=967770086&start_sha=d02a9376d4f48935d8b1f38c89445ddb1aeeca66)
CSDUMMI commented 2024-03-29 08:12:15 +00:00 (Migrated from gitlab.com)
Copy link

mentioned in merge request !24

mentioned in merge request !24
CSDUMMI commented 2024-03-29 08:22:33 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • 0deb6f78 - Add test case for FeedManager.filter testing the webhook overwrites other tests

Compare with previous version

added 1 commit <ul><li>0deb6f78 - Add test case for FeedManager.filter testing the webhook overwrites other tests</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=967777056&start_sha=dd8e2a2658ff7f05cdc7058ffdd56993d65d899c)
CSDUMMI commented 2024-03-29 10:04:36 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • 1187b18d - Add test case for FeedManager.filter testing the webhook overwrites other tests

Compare with previous version

added 1 commit <ul><li>1187b18d - Add test case for FeedManager.filter testing the webhook overwrites other tests</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=967853061&start_sha=0deb6f785d578f2806d08e7c97f81844270d9b1a)
CSDUMMI commented 2024-03-29 10:49:55 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • f4a48dc1 - Add rspec test for FeedManager webhook

Compare with previous version

added 1 commit <ul><li>f4a48dc1 - Add rspec test for FeedManager webhook</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=967888940&start_sha=1187b18d3b863bd0131f60711c46c35fe72bcfc1)
CSDUMMI commented 2024-03-29 11:04:54 +00:00 (Migrated from gitlab.com)
Copy link

added 1 commit

  • f2f38d65 - Build precompiled assets in separate stage and store it in artifacts to cache the build process

Compare with previous version

added 1 commit <ul><li>f2f38d65 - Build precompiled assets in separate stage and store it in artifacts to cache the build process</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=967900145&start_sha=f4a48dc130d8feff5fa42ffa32b274a9a76bebf1)
CSDUMMI commented 2024-03-29 11:33:52 +00:00 (Migrated from gitlab.com)
Copy link

I've been able to add a passing unit test for this Webhook.

I've been able to add a passing unit test for this Webhook.
CSDUMMI commented 2024-03-29 11:34:02 +00:00 (Migrated from gitlab.com)
Copy link

marked this merge request as ready

marked this merge request as **ready**
CSDUMMI commented 2024-05-09 12:30:17 +00:00 (Migrated from gitlab.com)
Copy link

added 2 commits

  • cc073628 - Add event field to feedmanager webhook
  • 1f3928ef - Add sender field with ID of the account sending a status to check in webhook

Compare with previous version

added 2 commits <ul><li>cc073628 - Add event field to feedmanager webhook</li><li>1f3928ef - Add sender field with ID of the account sending a status to check in webhook</li></ul> [Compare with previous version](/babka_net/mastodon-babka/-/merge_requests/22/diffs?diff_id=1004181634&start_sha=f4a48dc130d8feff5fa42ffa32b274a9a76bebf1)
This pull request has changes conflicting with the target branch.
  • .gitlab-ci.yml
  • app/lib/feed_manager.rb
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines:9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git switch 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch babka
git merge --no-ff 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git switch 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git rebase babka
git switch babka
git merge --ff-only 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git switch 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git rebase babka
git switch babka
git merge --no-ff 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git switch babka
git merge --squash 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git switch babka
git merge --ff-only 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git switch babka
git merge 9-add-webhook-at-feedmanager-filter-for-filtering-statuses-from-users-timelines
git push origin babka
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

   
confirmed

   
critical

   
discussion

   
documentation

   
enhancement

   
merge-upstream

   
suggestion

   
support
No labels
bug
confirmed
critical
discussion
documentation
enhancement
merge-upstream
suggestion
support
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
babka/mastodon-babka!40
No description provided.