From fab7d170319856993fe15563c801b3084def0e85 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Sat, 19 Oct 2024 11:04:07 +0200 Subject: [PATCH] [bugfix] Fix filter title unique constraint (#3458) --- internal/db/bundb/filter_test.go | 48 +++++++ .../migrations/20240126064004_add_filters.go | 2 +- .../20240126064004_add_filters/filter.go | 65 +++++++++ .../20241018151036_filter_unique_fix.go | 131 ++++++++++++++++++ internal/gtsmodel/filter.go | 28 ++-- 5 files changed, 259 insertions(+), 15 deletions(-) create mode 100644 internal/db/bundb/migrations/20240126064004_add_filters/filter.go create mode 100644 internal/db/bundb/migrations/20241018151036_filter_unique_fix.go diff --git a/internal/db/bundb/filter_test.go b/internal/db/bundb/filter_test.go index d1249d16b..fbb2a6bfb 100644 --- a/internal/db/bundb/filter_test.go +++ b/internal/db/bundb/filter_test.go @@ -247,6 +247,54 @@ func (suite *FilterTestSuite) TestFilterCRUD() { } } +func (suite *FilterTestSuite) TestFilterTitleOverlap() { + var ( + ctx = context.Background() + account1 = "01HNEJXCPRTJVJY9MV0VVHGD47" + account2 = "01JAG5BRJPJYA0FSA5HR2MMFJH" + ) + + // Create an empty filter for account 1. + account1filter1 := >smodel.Filter{ + ID: "01HNEJNVZZVXJTRB3FX3K2B1YF", + AccountID: account1, + Title: "my filter", + Action: gtsmodel.FilterActionWarn, + ContextHome: util.Ptr(true), + } + if err := suite.db.PutFilter(ctx, account1filter1); err != nil { + suite.FailNow("", "error putting account1filter1: %s", err) + } + + // Create a filter for account 2 with + // the same title, should be no issue. + account2filter1 := >smodel.Filter{ + ID: "01JAG5GPXG7H5Y4ZP78GV1F2ET", + AccountID: account2, + Title: "my filter", + Action: gtsmodel.FilterActionWarn, + ContextHome: util.Ptr(true), + } + if err := suite.db.PutFilter(ctx, account2filter1); err != nil { + suite.FailNow("", "error putting account2filter1: %s", err) + } + + // Try to create another filter for + // account 1 with the same name as + // an existing filter of theirs. + account1filter2 := >smodel.Filter{ + ID: "01JAG5J8NYKQE2KYCD28Y4P05V", + AccountID: account1, + Title: "my filter", + Action: gtsmodel.FilterActionWarn, + ContextHome: util.Ptr(true), + } + err := suite.db.PutFilter(ctx, account1filter2) + if !errors.Is(err, db.ErrAlreadyExists) { + suite.FailNow("", "wanted ErrAlreadyExists, got %s", err) + } +} + func TestFilterTestSuite(t *testing.T) { suite.Run(t, new(FilterTestSuite)) } diff --git a/internal/db/bundb/migrations/20240126064004_add_filters.go b/internal/db/bundb/migrations/20240126064004_add_filters.go index 3ad22f9d8..3ea9ae63c 100644 --- a/internal/db/bundb/migrations/20240126064004_add_filters.go +++ b/internal/db/bundb/migrations/20240126064004_add_filters.go @@ -20,7 +20,7 @@ package migrations import ( "context" - gtsmodel "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + gtsmodel "github.com/superseriousbusiness/gotosocial/internal/db/bundb/migrations/20240126064004_add_filters" "github.com/uptrace/bun" ) diff --git a/internal/db/bundb/migrations/20240126064004_add_filters/filter.go b/internal/db/bundb/migrations/20240126064004_add_filters/filter.go new file mode 100644 index 000000000..5c0338019 --- /dev/null +++ b/internal/db/bundb/migrations/20240126064004_add_filters/filter.go @@ -0,0 +1,65 @@ +// GoToSocial +// Copyright (C) GoToSocial Authors admin@gotosocial.org +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package gtsmodel + +import ( + "regexp" + "time" +) + +// Filter stores a filter created by a local account. +type Filter struct { + ID string `bun:"type:CHAR(26),pk,nullzero,notnull,unique"` // id of this item in the database + CreatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item created + UpdatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item last updated + ExpiresAt time.Time `bun:"type:timestamptz,nullzero"` // Time filter should expire. If null, should not expire. + AccountID string `bun:"type:CHAR(26),notnull,nullzero"` // ID of the local account that created the filter. + Title string `bun:",nullzero,notnull,unique"` // The name of the filter. + Action string `bun:",nullzero,notnull"` // The action to take. + Keywords []*FilterKeyword `bun:"-"` // Keywords for this filter. + Statuses []*FilterStatus `bun:"-"` // Statuses for this filter. + ContextHome *bool `bun:",nullzero,notnull,default:false"` // Apply filter to home timeline and lists. + ContextNotifications *bool `bun:",nullzero,notnull,default:false"` // Apply filter to notifications. + ContextPublic *bool `bun:",nullzero,notnull,default:false"` // Apply filter to home timeline and lists. + ContextThread *bool `bun:",nullzero,notnull,default:false"` // Apply filter when viewing a status's associated thread. + ContextAccount *bool `bun:",nullzero,notnull,default:false"` // Apply filter when viewing an account profile. +} + +// FilterKeyword stores a single keyword to filter statuses against. +type FilterKeyword struct { + ID string `bun:"type:CHAR(26),pk,nullzero,notnull,unique"` // id of this item in the database + CreatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item created + UpdatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item last updated + AccountID string `bun:"type:CHAR(26),notnull,nullzero"` // ID of the local account that created the filter keyword. + FilterID string `bun:"type:CHAR(26),notnull,nullzero,unique:filter_keywords_filter_id_keyword_uniq"` // ID of the filter that this keyword belongs to. + Filter *Filter `bun:"-"` // Filter corresponding to FilterID + Keyword string `bun:",nullzero,notnull,unique:filter_keywords_filter_id_keyword_uniq"` // The keyword or phrase to filter against. + WholeWord *bool `bun:",nullzero,notnull,default:false"` // Should the filter consider word boundaries? + Regexp *regexp.Regexp `bun:"-"` // pre-prepared regular expression +} + +// FilterStatus stores a single status to filter. +type FilterStatus struct { + ID string `bun:"type:CHAR(26),pk,nullzero,notnull,unique"` // id of this item in the database + CreatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item created + UpdatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item last updated + AccountID string `bun:"type:CHAR(26),notnull,nullzero"` // ID of the local account that created the filter keyword. + FilterID string `bun:"type:CHAR(26),notnull,nullzero,unique:filter_statuses_filter_id_status_id_uniq"` // ID of the filter that this keyword belongs to. + Filter *Filter `bun:"-"` // Filter corresponding to FilterID + StatusID string `bun:"type:CHAR(26),notnull,nullzero,unique:filter_statuses_filter_id_status_id_uniq"` // ID of the status to filter. +} diff --git a/internal/db/bundb/migrations/20241018151036_filter_unique_fix.go b/internal/db/bundb/migrations/20241018151036_filter_unique_fix.go new file mode 100644 index 000000000..f2ead0b92 --- /dev/null +++ b/internal/db/bundb/migrations/20241018151036_filter_unique_fix.go @@ -0,0 +1,131 @@ +// GoToSocial +// Copyright (C) GoToSocial Authors admin@gotosocial.org +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package migrations + +import ( + "context" + + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/uptrace/bun" + "github.com/uptrace/bun/dialect" +) + +func init() { + up := func(ctx context.Context, db *bun.DB) error { + return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error { + + // Create the new filters table + // with the unique constraint + // set on AccountID + Title. + if _, err := tx. + NewCreateTable(). + ModelTableExpr("new_filters"). + Model((*gtsmodel.Filter)(nil)). + Exec(ctx); err != nil { + return err + } + + // Explicitly specify columns to bring + // from old table to new, to avoid any + // potential Postgres shenanigans. + columns := []string{ + "id", + "created_at", + "updated_at", + "expires_at", + "account_id", + "title", + "action", + "context_home", + "context_notifications", + "context_public", + "context_thread", + "context_account", + } + + // Copy all data for existing + // filters to the new table. + if _, err := tx. + NewInsert(). + Table("new_filters"). + Table("filters"). + Column(columns...). + Exec(ctx); err != nil { + return err + } + + // Drop the old table. + if _, err := tx. + NewDropTable(). + Table("filters"). + Exec(ctx); err != nil { + return err + } + + // Rename new table to old table. + if _, err := tx. + ExecContext( + ctx, + "ALTER TABLE ? RENAME TO ?", + bun.Ident("new_filters"), + bun.Ident("filters"), + ); err != nil { + return err + } + + // Index the new version + // of the filters table. + if _, err := tx. + NewCreateIndex(). + Table("filters"). + Index("filters_account_id_idx"). + Column("account_id"). + IfNotExists(). + Exec(ctx); err != nil { + return err + } + + if db.Dialect().Name() == dialect.PG { + // Rename "new_filters_pkey" from the + // new table to just "filters_pkey". + // This is only necessary on Postgres. + if _, err := tx.ExecContext( + ctx, + "ALTER TABLE ? RENAME CONSTRAINT ? TO ?", + bun.Ident("public.filters"), + bun.Safe("new_filters_pkey"), + bun.Safe("filters_pkey"), + ); err != nil { + return err + } + } + + return nil + }) + } + + down := func(ctx context.Context, db *bun.DB) error { + return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error { + return nil + }) + } + + if err := Migrations.Register(up, down); err != nil { + panic(err) + } +} diff --git a/internal/gtsmodel/filter.go b/internal/gtsmodel/filter.go index 95047a44f..593918ec1 100644 --- a/internal/gtsmodel/filter.go +++ b/internal/gtsmodel/filter.go @@ -26,20 +26,20 @@ import ( // Filter stores a filter created by a local account. type Filter struct { - ID string `bun:"type:CHAR(26),pk,nullzero,notnull,unique"` // id of this item in the database - CreatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item created - UpdatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item last updated - ExpiresAt time.Time `bun:"type:timestamptz,nullzero"` // Time filter should expire. If null, should not expire. - AccountID string `bun:"type:CHAR(26),notnull,nullzero"` // ID of the local account that created the filter. - Title string `bun:",nullzero,notnull,unique"` // The name of the filter. - Action FilterAction `bun:",nullzero,notnull"` // The action to take. - Keywords []*FilterKeyword `bun:"-"` // Keywords for this filter. - Statuses []*FilterStatus `bun:"-"` // Statuses for this filter. - ContextHome *bool `bun:",nullzero,notnull,default:false"` // Apply filter to home timeline and lists. - ContextNotifications *bool `bun:",nullzero,notnull,default:false"` // Apply filter to notifications. - ContextPublic *bool `bun:",nullzero,notnull,default:false"` // Apply filter to home timeline and lists. - ContextThread *bool `bun:",nullzero,notnull,default:false"` // Apply filter when viewing a status's associated thread. - ContextAccount *bool `bun:",nullzero,notnull,default:false"` // Apply filter when viewing an account profile. + ID string `bun:"type:CHAR(26),pk,nullzero,notnull,unique"` // id of this item in the database + CreatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item created + UpdatedAt time.Time `bun:"type:timestamptz,nullzero,notnull,default:current_timestamp"` // when was item last updated + ExpiresAt time.Time `bun:"type:timestamptz,nullzero"` // Time filter should expire. If null, should not expire. + AccountID string `bun:"type:CHAR(26),notnull,nullzero,unique:filters_account_id_title_uniq"` // ID of the local account that created the filter. + Title string `bun:",nullzero,notnull,unique:filters_account_id_title_uniq"` // The name of the filter. + Action FilterAction `bun:",nullzero,notnull"` // The action to take. + Keywords []*FilterKeyword `bun:"-"` // Keywords for this filter. + Statuses []*FilterStatus `bun:"-"` // Statuses for this filter. + ContextHome *bool `bun:",nullzero,notnull,default:false"` // Apply filter to home timeline and lists. + ContextNotifications *bool `bun:",nullzero,notnull,default:false"` // Apply filter to notifications. + ContextPublic *bool `bun:",nullzero,notnull,default:false"` // Apply filter to home timeline and lists. + ContextThread *bool `bun:",nullzero,notnull,default:false"` // Apply filter when viewing a status's associated thread. + ContextAccount *bool `bun:",nullzero,notnull,default:false"` // Apply filter when viewing an account profile. } // Expired returns whether the filter has expired at a given time.