feature: filters v2 server-side warning/hiding #2793

VyrCossont · 2024-04-01T07:19:44Z

Description

This PR implements Mastodon-4.0-compatible server-side filtering, aka v2 filtering. Statuses matching a filter with a warn action will have a filtered field added to them so that clients can display a click-through warning with context, or hide them entirely depending on user preferences. (Clients not aware of this field can still use v1 client-side filtering.) Statuses matching a filter with a hide action will not be sent to the client at all, which works for all clients regardless of supported filter APIs.

This PR doesn't implement the CRUD API for v2 filters, or allow the creation of hide filters through the v1 API. I'll do that in a followup PR. Our internal filter storage is already in the v2 format, so the CRUD API for v1 filters can be used to manage warn filters meanwhile.

Impact

Enables filtering on Phanpy and other clients that support v2 filters but not v1 filters. (Note that Phanpy doesn't have UI for managing filters, so they have to be created through another client using the v1 API.)

Doesn't affect clients that only support v1 filters (Semaphore, Feditext).

Implementation

Extends typeutils.StatusToAPIStatus to take a filter context (public or tag timeline, home or list timeline, notifications, thread, account's statuses, or none) and the filter list from the requesting user, if there is one. About half of StatusToAPIStatus calls are from a context where we would apply filters, so adding filters to that function makes sense to me (I could be convinced otherwise if this goes beyond the traditional scope of typeutils). Similarly extends typeutils.NotificationToAPINotification to take a filter list (the filter context is implicitly notifications).

Applying the filters generates either:

no match: the Status is not modified by filtering.
for a matching filter with the warn filter action, a FilterResult, which contains a filter that matched (even if multiple filters match, it can only hold one), and the list of the actual keywords and status IDs within that filter that matched. This gets attached to the returned Status so clients can put it behind a warning or hide it themselves.
for a matching filter with the hide filter action, an ErrHideStatus error, which signals to StatusToAPIStatus callers that the Status should not be included in the results at all.

A matching hide filter takes action priority over a matching warn filter, so if we match a warn filter, we will record the match but keep going through the filter list in case there are any matching hide filters. Matching a hide filter exits early and doesn't bother checking the rest of the filters.

Filters don't apply to a user's own posts.

I've extended StatusToAPIStatus callers to:

include the filter context if applicable: APIs like the user's bookmarks list don't filter, and use a named constant for the empty string to indicate no context.
fetch the requesting user's filters if applicable: a few APIs don't always require auth, so sometimes there's no user.
handle ErrHideStatus by returning early with no error (when adding a single status to a timeline) or by skipping a status (when returning several statuses).

Performance

Inherently slower since we're doing some more work server-side. How much slower depends on how many filters a user has.

Filters are fetched from the existing filter cache if possible instead of fetching from the DB, like most DB objects. However, the cache/DB representation isn't quite what the filtering code uses. We could store precompiled regexps and status ID sets per filter context, as well as lookup maps for generating the filtered attribute, at the complexity cost of adding another kind of cache and having to invalidate all of it for a user every time any of their filters change.

Alternatively, instead of caching regexps and status IDs across requests, we could fetch the user's cached DB filters per request (as this PR already does), compile them to regexps and status IDs for a given filter context, and tweak StatusToAPIStatus to take those regexps and status IDs instead of a filter list. This would save a little work when filtering multiple statuses at once, for example, when returning a timeline page, and isn't as complex as the previous option since there's no second cache.

Either of these could be done in a followup PR.

Expired filters are not purged automatically, since Mastodon intentionally doesn't do that in case the user wants to renew an expired filter later. We could add a recurring task to purge very old ones automatically in a followup PR if user filter list size becomes a problem due to expired filters specifically.

Checklist

VyrCossont · 2024-04-01T07:22:35Z

One more TODO:

when filtering a status's content, we should first convert the HTML to plain text so filters don't match HTML tags by accident.

tsmethurst · 2024-04-08T15:56:31Z

Will get to this as soon as 0.15.0 is out and other pending PRs are squerged :)

tsmethurst · 2024-04-22T08:57:06Z

Apologies for leaving this in review request limbo for a while, got caught up in a necessary refactor of the frontend stuff so I haven't got around to looking at it yet!

daenney

Honestly this seem alright to me. The tests pass, as do the new ones too so that's also a good sign (the tests fail on the Swagger gen test, it just needs regenerating).

I've left some minor comments, mostly a few places where bare empty strings are being passed in instead of the FilterContextNone.

internal/api/model/filterv2.go

daenney · 2024-05-02T09:42:08Z

internal/api/model/filterv2.go

+	// FilterActionHide filters will remove this status from API results.
+	FilterActionHide FilterAction = "hide"
+
+	FilterActionNumValues = 2


This seems unused.

It's currently unused, so I'll remove it for now., and add it back in with the rest of the public v2 filter API.

daenney · 2024-05-02T09:44:27Z

internal/processing/common/status.go

@@ -184,95 +184,14 @@ func (p *Processor) GetAPIStatus(
 	apiStatus *apimodel.Status,
 	errWithCode gtserror.WithCode,
 ) {
-	apiStatus, err := p.converter.StatusToAPIStatus(ctx, target, requester)
+	apiStatus, err := p.converter.StatusToAPIStatus(ctx, target, requester, "", nil)


Suggested change

apiStatus, err := p.converter.StatusToAPIStatus(ctx, target, requester, "", nil)

apiStatus, err := p.converter.StatusToAPIStatus(ctx, target, requester, custom.FilterContextNone, nil)

daenney · 2024-05-02T09:44:45Z

internal/processing/search/util.go

@@ -113,7 +113,7 @@ func (p *Processor) packageStatuses(
 			continue
 		}

-		apiStatus, err := p.converter.StatusToAPIStatus(ctx, status, requestingAccount)
+		apiStatus, err := p.converter.StatusToAPIStatus(ctx, status, requestingAccount, "", nil)


Suggested change

apiStatus, err := p.converter.StatusToAPIStatus(ctx, status, requestingAccount, "", nil)

apiStatus, err := p.converter.StatusToAPIStatus(ctx, status, requestingAccount, custom.FilterContextNone, nil)

daenney · 2024-05-02T09:49:35Z

internal/typeutils/internaltofrontend.go

@@ -1446,7 +1606,7 @@ func (c *Converter) ReportToAdminAPIReport(ctx context.Context, r *gtsmodel.Repo
 		}
 	}
 	for _, s := range r.Statuses {
-		status, err := c.StatusToAPIStatus(ctx, s, requestingAccount)
+		status, err := c.StatusToAPIStatus(ctx, s, requestingAccount, "", nil)


Suggested change

status, err := c.StatusToAPIStatus(ctx, s, requestingAccount, "", nil)

status, err := c.StatusToAPIStatus(ctx, s, requestingAccount, custom.FilterContextNone, nil)

daenney · 2024-05-02T09:51:16Z

internal/typeutils/internaltofrontend.go

-		ExpiresAt:    expiresAt,
-		Irreversible: filter.Action == gtsmodel.FilterActionHide,
-	}, nil
+	return ""


It might be nice to have a apimodel.FitlerActionNone here and return that from the default in the switch statement.

daenney · 2024-05-02T09:51:31Z

internal/typeutils/internaltofrontend_test.go

@@ -427,7 +428,7 @@ func (suite *InternalToFrontendTestSuite) TestLocalInstanceAccountToFrontendBloc
 func (suite *InternalToFrontendTestSuite) TestStatusToFrontend() {
 	testStatus := suite.testStatuses["admin_account_status_1"]
 	requestingAccount := suite.testAccounts["local_account_1"]
-	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount)
+	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, "", nil)


Suggested change

apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, "", nil)

apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, custom.FilterContextNone, nil)

daenney · 2024-05-02T09:52:16Z

internal/typeutils/internaltofrontend_test.go

 func (suite *InternalToFrontendTestSuite) TestStatusToFrontendUnknownAttachments() {
 	testStatus := suite.testStatuses["remote_account_2_status_1"]
 	requestingAccount := suite.testAccounts["admin_account"]

-	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount)
+	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, "", nil)


Suggested change

apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, "", nil)

apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, custom.FilterContextNone, nil)

daenney · 2024-05-02T09:52:26Z

internal/typeutils/internaltofrontend_test.go

@@ -774,7 +950,7 @@ func (suite *InternalToFrontendTestSuite) TestStatusToFrontendUnknownLanguage()
 	*testStatus = *suite.testStatuses["admin_account_status_1"]
 	testStatus.Language = ""
 	requestingAccount := suite.testAccounts["local_account_1"]
-	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount)
+	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, "", nil)


Suggested change

apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, "", nil)

apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, custom.FilterContextNone, nil)

daenney · 2024-05-02T09:57:17Z

internal/filter/custom/user.go

+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+// Package custom represents custom filters managed by the user through the API.
+package custom


I would probably rename this package to user instead of custom, since they're user filters. I think it's implicitly understood they're kinda custom/unique to a user. It reads a bit odd in other import paths to see stuff like custom.FilterContextNone, user.FilterContextNone makes it a bit more obvious for me as to what kind of filter this is.

Sure, no problem.

user sounds like it could be a filter for users as well as a filter managed by a user, so we ended up going with status, imported as statusfilter to avoid conflicts with variables named status.

tsmethurst · 2024-05-02T10:22:23Z

Once I've finished fiddling with this bugfix I'm working on, I'll push to this to bring it up to date with main to save you the trouble @VyrCossont :)

VyrCossont · 2024-05-02T17:02:09Z

Ready for any necessary further review.

NyaaaWhatsUpDoc · 2024-05-06T11:49:03Z

great work! thank you so much @VyrCossont !

VyrCossont added 8 commits March 31, 2024 14:02

Remove dead code

fc2b641

Filter statuses when converting to frontend representation

125b433

status.filtered is an array

be78b7e

Make matching case-insensitive

36a156c

Remove TODOs that don't need to be done now

fbd5e44

Add missing filter check for notification

5f03f63

lint: rename ErrHideStatus

6f37930

APIFilterActionToFilterAction not used yet

7ebbbb4

CDN18 mentioned this pull request Apr 17, 2024

[bug] fedilab shows error after boosting post #2844

Closed

Merge branch 'main' into filters-v2

4c195cf

daenney reviewed May 2, 2024

View reviewed changes

tsmethurst and others added 4 commits May 2, 2024 16:32

Merge branch 'main' into filters-v2

02f2147

swaggerino docseroni

3f6ecd1

Address review comments

ef20a4e

Add apimodel.FilterActionNone

9ca8293

NyaaaWhatsUpDoc merged commit 45f4afe into superseriousbusiness:main May 6, 2024
2 checks passed

VyrCossont deleted the filters-v2 branch May 15, 2024 21:50

VyrCossont mentioned this pull request May 15, 2024

Support server-side filtering feditext/feditext#57

Open

VyrCossont mentioned this pull request May 27, 2024

[feature] Implement Filter API v2 #2936

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: filters v2 server-side warning/hiding #2793

feature: filters v2 server-side warning/hiding #2793

VyrCossont commented Apr 1, 2024 •

edited

VyrCossont commented Apr 1, 2024 •

edited

tsmethurst commented Apr 8, 2024

tsmethurst commented Apr 22, 2024

daenney left a comment •

edited

daenney May 2, 2024

VyrCossont May 2, 2024

daenney May 2, 2024

daenney May 2, 2024

daenney May 2, 2024

daenney May 2, 2024

VyrCossont May 2, 2024

daenney May 2, 2024

daenney May 2, 2024

daenney May 2, 2024

daenney May 2, 2024

VyrCossont May 2, 2024

VyrCossont May 2, 2024

tsmethurst commented May 2, 2024 •

edited

VyrCossont commented May 2, 2024

NyaaaWhatsUpDoc commented May 6, 2024

	apiStatus, err := p.converter.StatusToAPIStatus(ctx, target, requester, "", nil)
	apiStatus, err := p.converter.StatusToAPIStatus(ctx, target, requester, custom.FilterContextNone, nil)

	status, err := c.StatusToAPIStatus(ctx, s, requestingAccount, "", nil)
	status, err := c.StatusToAPIStatus(ctx, s, requestingAccount, custom.FilterContextNone, nil)

	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, "", nil)
	apiStatus, err := suite.typeconverter.StatusToAPIStatus(context.Background(), testStatus, requestingAccount, custom.FilterContextNone, nil)

feature: filters v2 server-side warning/hiding #2793

feature: filters v2 server-side warning/hiding #2793

Conversation

VyrCossont commented Apr 1, 2024 • edited

Description

Impact

Implementation

Performance

Checklist

VyrCossont commented Apr 1, 2024 • edited

tsmethurst commented Apr 8, 2024

tsmethurst commented Apr 22, 2024

daenney left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsmethurst commented May 2, 2024 • edited

VyrCossont commented May 2, 2024

NyaaaWhatsUpDoc commented May 6, 2024

VyrCossont commented Apr 1, 2024 •

edited

VyrCossont commented Apr 1, 2024 •

edited

daenney left a comment •

edited

tsmethurst commented May 2, 2024 •

edited