fix public timeline bug (#150)

internal/db/bundb/account.go

@@ -110,7 +110,7 @@ func (a *accountDB) GetInstanceAccount(ctx context.Context, domain string) (*gts
 	} else {
 		q = q.
 			Where("account.username = ?", domain).
-			Where("? IS NULL", bun.Ident("domain"))
+			WhereGroup(" AND ", whereEmptyOrNull("domain"))
 	}
 
 	err := processErrorResponse(q.Scan(ctx))
@@ -172,7 +172,7 @@ func (a *accountDB) GetLocalAccountByUsername(ctx context.Context, username stri
 
 	q := a.newAccountQ(account).
 		Where("username = ?", username).
-		Where("? IS NULL", bun.Ident("domain"))
+		WhereGroup(" AND ", whereEmptyOrNull("domain"))
 
 	err := processErrorResponse(q.Scan(ctx))
 
@@ -218,7 +218,7 @@ func (a *accountDB) GetAccountStatuses(ctx context.Context, accountID string, li
 	}
 
 	if excludeReplies {
-		q = q.Where("? IS NULL", bun.Ident("in_reply_to_id"))
+		q = q.WhereGroup(" AND ", whereEmptyOrNull("in_reply_to_id"))
 	}
 
 	if pinnedOnly {

internal/db/bundb/admin.go

@@ -97,7 +97,7 @@ func (a *adminDB) NewSignup(ctx context.Context, username string, reason string,
 	err = a.conn.NewSelect().
 		Model(acct).
 		Where("username = ?", username).
-		Where("? IS NULL", bun.Ident("domain")).
+		WhereGroup(" AND ", whereEmptyOrNull("domain")).
 		Scan(ctx)
 	if err != nil {
 		// we just don't have an account yet create one
@@ -181,7 +181,7 @@ func (a *adminDB) CreateInstanceAccount(ctx context.Context) db.Error {
 		NewSelect().
 		Model(&gtsmodel.Account{}).
 		Where("username = ?", username).
-		Where("? IS NULL", bun.Ident("domain"))
+		WhereGroup(" AND ", whereEmptyOrNull("domain"))
 	count, err := existsQ.Count(ctx)
 	if err != nil && count == 1 {
 		a.log.Infof("instance account %s already exists", username)

internal/db/bundb/basic_test.go

@@ -63,6 +63,13 @@ func (suite *BasicTestSuite) TestGetAccountByID() {
 	suite.NoError(err)
 }
 
+func (suite *BasicTestSuite) TestGetAllStatuses() {
+	s := []*gtsmodel.Status{}
+	err := suite.db.GetAll(context.Background(), &s)
+	suite.NoError(err)
+	suite.Len(s, 12)
+}
+
 func TestBasicTestSuite(t *testing.T) {
 	suite.Run(t, new(BasicTestSuite))
 }

internal/db/bundb/instance.go

@@ -41,7 +41,7 @@ func (i *instanceDB) CountInstanceUsers(ctx context.Context, domain string) (int
 
 	if domain == i.config.Host {
 		// if the domain is *this* domain, just count where the domain field is null
-		q = q.Where("? IS NULL", bun.Ident("domain"))
+		q = q.WhereGroup(" AND ", whereEmptyOrNull("domain"))
 	} else {
 		q = q.Where("domain = ?", domain)
 	}
@@ -83,7 +83,9 @@ func (i *instanceDB) CountInstanceDomains(ctx context.Context, domain string) (i
 	if domain == i.config.Host {
 		// if the domain is *this* domain, just count other instances it knows about
 		// exclude domains that are blocked
-		q = q.Where("domain != ?", domain).Where("? IS NULL", bun.Ident("suspended_at"))
+		q = q.
+			Where("domain != ?", domain).
+			Where("? IS NULL", bun.Ident("suspended_at"))
 	} else {
 		// TODO: implement federated domain counting properly for remote domains
 		return 0, nil

internal/db/bundb/relationship.go

@@ -294,18 +294,10 @@ func (r *relationshipDB) GetAccountFollowedBy(ctx context.Context, accountID str
 		Model(&follows)
 
 	if localOnly {
-		// for local accounts let's get where domain is null OR where domain is an empty string, just to be safe
-		whereGroup := func(q *bun.SelectQuery) *bun.SelectQuery {
-			q = q.
-				WhereOr("? IS NULL", bun.Ident("a.domain")).
-				WhereOr("a.domain = ?", "")
-			return q
-		}
-
 		q = q.ColumnExpr("follow.*").
 			Join("JOIN accounts AS a ON follow.account_id = TEXT(a.id)").
 			Where("follow.target_account_id = ?", accountID).
-			WhereGroup(" AND ", whereGroup)
+			WhereGroup(" AND ", whereEmptyOrNull("a.domain"))
 	} else {
 		q = q.Where("target_account_id = ?", accountID)
 	}

internal/db/bundb/timeline.go

@@ -96,9 +96,9 @@ func (t *timelineDB) GetPublicTimeline(ctx context.Context, accountID string, ma
 		NewSelect().
 		Model(&statuses).
 		Where("visibility = ?", gtsmodel.VisibilityPublic).
-		Where("? IS NULL", bun.Ident("in_reply_to_id")).
-		Where("? IS NULL", bun.Ident("in_reply_to_uri")).
-		Where("? IS NULL", bun.Ident("boost_of_id")).
+		WhereGroup(" AND ", whereEmptyOrNull("in_reply_to_id")).
+		WhereGroup(" AND ", whereEmptyOrNull("in_reply_to_uri")).
+		WhereGroup(" AND ", whereEmptyOrNull("boost_of_id")).
 		Order("status.id DESC")
 
 	if maxID != "" {

internal/db/bundb/timeline_test.go

@@ -0,0 +1,68 @@
+/*
+   GoToSocial
+   Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org
+
+   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 <http://www.gnu.org/licenses/>.
+*/
+
+package bundb_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/testrig"
+)
+
+type TimelineTestSuite struct {
+	BunDBStandardTestSuite
+}
+
+func (suite *TimelineTestSuite) SetupSuite() {
+	suite.testTokens = testrig.NewTestTokens()
+	suite.testClients = testrig.NewTestClients()
+	suite.testApplications = testrig.NewTestApplications()
+	suite.testUsers = testrig.NewTestUsers()
+	suite.testAccounts = testrig.NewTestAccounts()
+	suite.testAttachments = testrig.NewTestAttachments()
+	suite.testStatuses = testrig.NewTestStatuses()
+	suite.testTags = testrig.NewTestTags()
+	suite.testMentions = testrig.NewTestMentions()
+}
+
+func (suite *TimelineTestSuite) SetupTest() {
+	suite.config = testrig.NewTestConfig()
+	suite.db = testrig.NewTestDB()
+	suite.log = testrig.NewTestLog()
+
+	testrig.StandardDBSetup(suite.db, suite.testAccounts)
+}
+
+func (suite *TimelineTestSuite) TearDownTest() {
+	testrig.StandardDBTeardown(suite.db)
+}
+
+func (suite *TimelineTestSuite) TestGetPublicTimeline() {
+	viewingAccount := suite.testAccounts["local_account_1"]
+
+	s, err := suite.db.GetPublicTimeline(context.Background(), viewingAccount.ID, "", "", "", 20, false)
+	suite.NoError(err)
+
+	suite.Len(s, 6)
+}
+
+func TestTimelineTestSuite(t *testing.T) {
+	suite.Run(t, new(TimelineTestSuite))
+}

internal/db/bundb/util.go

@@ -76,3 +76,17 @@ func notExists(ctx context.Context, q *bun.SelectQuery) (bool, db.Error) {
 
 	return notExists, nil
 }
+
+// whereEmptyOrNull is a convenience function to return a bun WhereGroup that specifies
+// that the given column should be EITHER an empty string OR null.
+//
+// Use it as follows:
+//
+//   q = q.WhereGroup(" AND ", whereEmptyOrNull("whatever_column"))
+func whereEmptyOrNull(column string) func(*bun.SelectQuery) *bun.SelectQuery {
+	return func(q *bun.SelectQuery) *bun.SelectQuery {
+		return q.
+			WhereOr("? IS NULL", bun.Ident(column)).
+			WhereOr("? = ''", bun.Ident(column))
+	}
+}

internal/gtsmodel/account.go

@@ -48,20 +48,20 @@ type Account struct {
 	AvatarMediaAttachmentID string           `bun:"type:CHAR(26),nullzero"`
 	AvatarMediaAttachment   *MediaAttachment `bun:"rel:belongs-to"`
 	// For a non-local account, where can the header be fetched?
-	AvatarRemoteURL string
+	AvatarRemoteURL string `bun:",nullzero"`
 	// ID of the header as a media attachment
 	HeaderMediaAttachmentID string           `bun:"type:CHAR(26),nullzero"`
 	HeaderMediaAttachment   *MediaAttachment `bun:"rel:belongs-to"`
 	// For a non-local account, where can the header be fetched?
-	HeaderRemoteURL string
+	HeaderRemoteURL string `bun:",nullzero"`
 	// DisplayName for this account. Can be empty, then just the Username will be used for display purposes.
-	DisplayName string
+	DisplayName string `bun:",nullzero"`
 	// a key/value map of fields that this account has added to their profile
 	Fields []Field
 	// A note that this account has on their profile (ie., the account's bio/description of themselves)
-	Note string
+	Note string `bun:",nullzero"`
 	// Is this a memorial account, ie., has the user passed away?
-	Memorial bool
+	Memorial bool `bun:",nullzero"`
 	// This account has moved this account id in the database
 	MovedToAccountID string `bun:"type:CHAR(26),nullzero"`
 	// When was this account created?
@@ -71,7 +71,7 @@ type Account struct {
 	// Does this account identify itself as a bot?
 	Bot bool
 	// What reason was given for signing up when this account was created?
-	Reason string
+	Reason string `bun:",nullzero"`
 
 	/*
 		USER AND PRIVACY PREFERENCES
@@ -109,9 +109,9 @@ type Account struct {
 	// URL for getting the featured collection list of this account
 	FeaturedCollectionURI string `bun:",unique,nullzero"`
 	// What type of activitypub actor is this account?
-	ActorType string
+	ActorType string `bun:",nullzero"`
 	// This account is associated with x account id
-	AlsoKnownAs string
+	AlsoKnownAs string `bun:",nullzero"`
 
 	/*
 		CRYPTO FIELDS
@@ -122,7 +122,7 @@ type Account struct {
 	// Publickey for encoding activitypub requests, will be defined for both local and remote accounts
 	PublicKey *rsa.PublicKey
 	// Web-reachable location of this account's public key
-	PublicKeyURI string
+	PublicKeyURI string `bun:",nullzero"`
 
 	/*
 		ADMIN FIELDS

internal/gtsmodel/application.go

@@ -24,17 +24,17 @@ type Application struct {
 	// id of this application in the db
 	ID string `bun:"type:CHAR(26),pk,notnull"`
 	// name of the application given when it was created (eg., 'tusky')
-	Name string
+	Name string `bun:",nullzero"`
 	// website for the application given when it was created (eg., 'https://tusky.app')
-	Website string
+	Website string `bun:",nullzero"`
 	// redirect uri requested by the application for oauth2 flow
-	RedirectURI string
+	RedirectURI string `bun:",nullzero"`
 	// id of the associated oauth client entity in the db
-	ClientID string `bun:"type:CHAR(26)"`
+	ClientID string `bun:"type:CHAR(26),nullzero"`
 	// secret of the associated oauth client entity in the db
-	ClientSecret string
+	ClientSecret string `bun:",nullzero"`
 	// scopes requested when this app was created
-	Scopes string
+	Scopes string `bun:",nullzero"`
 	// a vapid key generated for this app when it was created
-	VapidKey string
+	VapidKey string `bun:",nullzero"`
 }

internal/gtsmodel/domainblock.go

@@ -34,9 +34,9 @@ type DomainBlock struct {
 	CreatedByAccountID string   `bun:"type:CHAR(26),notnull"`
 	CreatedByAccount   *Account `bun:"rel:belongs-to"`
 	// Private comment on this block, viewable to admins
-	PrivateComment string
+	PrivateComment string `bun:",nullzero"`
 	// Public comment on this block, viewable (optionally) by everyone
-	PublicComment string
+	PublicComment string `bun:",nullzero"`
 	// whether the domain name should appear obfuscated when displaying it publicly
 	Obfuscate bool
 	// if this block was created through a subscription, what's the subscription ID?

internal/gtsmodel/emoji.go

@@ -36,19 +36,19 @@ type Emoji struct {
 	// Where can this emoji be retrieved remotely? Null for local emojis.
 	// For remote emojis, it'll be something like:
 	// https://hackers.town/system/custom_emojis/images/000/049/842/original/1b74481204feabfd.png
-	ImageRemoteURL string
+	ImageRemoteURL string `bun:",nullzero"`
 	// Where can a static / non-animated version of this emoji be retrieved remotely? Null for local emojis.
 	// For remote emojis, it'll be something like:
 	// https://hackers.town/system/custom_emojis/images/000/049/842/static/1b74481204feabfd.png
-	ImageStaticRemoteURL string
+	ImageStaticRemoteURL string `bun:",nullzero"`
 	// Where can this emoji be retrieved from the local server? Null for remote emojis.
 	// Assuming our server is hosted at 'example.org', this will be something like:
 	// 'https://example.org/fileserver/6339820e-ef65-4166-a262-5a9f46adb1a7/emoji/original/bfa6c9c5-6c25-4ea4-98b4-d78b8126fb52.png'
-	ImageURL string
+	ImageURL string `bun:",nullzero"`
 	// Where can a static version of this emoji be retrieved from the local server? Null for remote emojis.
 	// Assuming our server is hosted at 'example.org', this will be something like:
 	// 'https://example.org/fileserver/6339820e-ef65-4166-a262-5a9f46adb1a7/emoji/small/bfa6c9c5-6c25-4ea4-98b4-d78b8126fb52.png'
-	ImageStaticURL string
+	ImageStaticURL string `bun:",nullzero"`
 	// Path of the emoji image in the server storage system. Will be something like:
 	// '/gotosocial/storage/6339820e-ef65-4166-a262-5a9f46adb1a7/emoji/original/bfa6c9c5-6c25-4ea4-98b4-d78b8126fb52.png'
 	ImagePath string `bun:",notnull"`

internal/gtsmodel/follow.go

@@ -37,7 +37,7 @@ type Follow struct {
 	// Does this follow also want to see reblogs and not just posts?
 	ShowReblogs bool `bun:"default:true"`
 	// What is the activitypub URI of this follow?
-	URI string `bun:",unique"`
+	URI string `bun:",unique,nullzero"`
 	// does the following account want to be notified when the followed account posts?
 	Notify bool
 }

internal/gtsmodel/instance.go

@@ -9,7 +9,7 @@ type Instance struct {
 	// Instance domain eg example.org
 	Domain string `bun:",pk,notnull,unique"`
 	// Title of this instance as it would like to be displayed.
-	Title string
+	Title string `bun:",nullzero"`
 	// base URI of this instance eg https://example.org
 	URI string `bun:",notnull,unique"`
 	// When was this instance created in the db?
@@ -22,20 +22,20 @@ type Instance struct {
 	DomainBlockID string       `bun:"type:CHAR(26),nullzero"`
 	DomainBlock   *DomainBlock `bun:"rel:belongs-to"`
 	// Short description of this instance
-	ShortDescription string
+	ShortDescription string `bun:",nullzero"`
 	// Longer description of this instance
-	Description string
+	Description string `bun:",nullzero"`
 	// Terms and conditions of this instance
-	Terms string
+	Terms string `bun:",nullzero"`
 	// Contact email address for this instance
-	ContactEmail string
+	ContactEmail string `bun:",nullzero"`
 	// Username of the contact account for this instance
-	ContactAccountUsername string
+	ContactAccountUsername string `bun:",nullzero"`
 	// Contact account ID in the database for this instance
 	ContactAccountID string   `bun:"type:CHAR(26),nullzero"`
 	ContactAccount   *Account `bun:"rel:belongs-to"`
 	// Reputation score of this instance
 	Reputation int64 `bun:",notnull,default:0"`
 	// Version of the software used on this instance
-	Version string
+	Version string `bun:",nullzero"`
 }

internal/gtsmodel/mediaattachment.go

@@ -30,9 +30,9 @@ type MediaAttachment struct {
 	// ID of the status to which this is attached
 	StatusID string `bun:"type:CHAR(26),nullzero"`
 	// Where can the attachment be retrieved on *this* server
-	URL string
+	URL string `bun:",nullzero"`
 	// Where can the attachment be retrieved on a remote server (empty for local media)
-	RemoteURL string
+	RemoteURL string `bun:",nullzero"`
 	// When was the attachment created
 	CreatedAt time.Time `bun:",nullzero,notnull,default:current_timestamp"`
 	// When was the attachment last updated
@@ -45,11 +45,11 @@ type MediaAttachment struct {
 	AccountID string   `bun:"type:CHAR(26),notnull"`
 	Account   *Account `bun:"rel:has-one"`
 	// Description of the attachment (for screenreaders)
-	Description string
+	Description string `bun:",nullzero"`
 	// To which scheduled status does this attachment belong
 	ScheduledStatusID string `bun:"type:CHAR(26),nullzero"`
 	// What is the generated blurhash of this attachment
-	Blurhash string
+	Blurhash string `bun:",nullzero"`
 	// What is the processing status of this attachment
 	Processing ProcessingStatus
 	// metadata for the whole file
@@ -65,29 +65,29 @@ type MediaAttachment struct {
 // File refers to the metadata for the whole file
 type File struct {
 	// What is the path of the file in storage.
-	Path string
+	Path string `bun:",nullzero"`
 	// What is the MIME content type of the file.
-	ContentType string
+	ContentType string `bun:",nullzero"`
 	// What is the size of the file in bytes.
 	FileSize int
 	// When was the file last updated.
-	UpdatedAt time.Time `bun:"type:timestamp,notnull,default:current_timestamp"`
+	UpdatedAt time.Time `bun:",notnull,default:current_timestamp"`
 }
 
 // Thumbnail refers to a small image thumbnail derived from a larger image, video, or audio file.
 type Thumbnail struct {
 	// What is the path of the file in storage
-	Path string
+	Path string `bun:",nullzero"`
 	// What is the MIME content type of the file.
-	ContentType string
+	ContentType string `bun:",nullzero"`
 	// What is the size of the file in bytes
 	FileSize int
 	// When was the file last updated
-	UpdatedAt time.Time `bun:"type:timestamp,notnull,default:current_timestamp"`
+	UpdatedAt time.Time `bun:",notnull,default:current_timestamp"`
 	// What is the URL of the thumbnail on the local server
-	URL string
+	URL string `bun:",nullzero"`
 	// What is the remote URL of the thumbnail (empty for local media)
-	RemoteURL string
+	RemoteURL string `bun:",nullzero"`
 }
 
 // ProcessingStatus refers to how far along in the processing stage the attachment is.

internal/gtsmodel/status.go

@@ -31,7 +31,7 @@ type Status struct {
 	// web url for viewing this status
 	URL string `bun:",unique,nullzero"`
 	// the html-formatted content of this status
-	Content string
+	Content string `bun:",nullzero"`
 	// Database IDs of any media attachments associated with this status
 	AttachmentIDs []string           `bun:"attachments,array"`
 	Attachments   []*MediaAttachment `bun:"attached_media,rel:has-many"`
@@ -54,12 +54,12 @@ type Status struct {
 	AccountID string   `bun:"type:CHAR(26),notnull"`
 	Account   *Account `bun:"rel:belongs-to"`
 	// AP uri of the owner of this status
-	AccountURI string
+	AccountURI string `bun:",nullzero"`
 	// id of the status this status is a reply to
 	InReplyToID string  `bun:"type:CHAR(26),nullzero"`
 	InReplyTo   *Status `bun:"-"`
 	// AP uri of the status this status is a reply to
-	InReplyToURI string
+	InReplyToURI string `bun:",nullzero"`
 	// id of the account that this status replies to
 	InReplyToAccountID string   `bun:"type:CHAR(26),nullzero"`
 	InReplyToAccount   *Account `bun:"rel:belongs-to"`
@@ -70,13 +70,13 @@ type Status struct {
 	BoostOfAccountID string   `bun:"type:CHAR(26),nullzero"`
 	BoostOfAccount   *Account `bun:"rel:belongs-to"`
 	// cw string for this status
-	ContentWarning string
+	ContentWarning string `bun:",nullzero"`
 	// visibility entry for this status
 	Visibility Visibility `bun:",notnull"`
 	// mark the status as sensitive?
 	Sensitive bool
 	// what language is this status written in?
-	Language string
+	Language string `bun:",nullzero"`
 	// Which application was used to create this status?
 	CreatedWithApplicationID string       `bun:"type:CHAR(26),nullzero"`
 	CreatedWithApplication   *Application `bun:"rel:belongs-to"`
@@ -84,9 +84,9 @@ type Status struct {
 	VisibilityAdvanced *VisibilityAdvanced
 	// What is the activitystreams type of this status? See: https://www.w3.org/TR/activitystreams-vocabulary/#object-types
 	// Will probably almost always be Note but who knows!.
-	ActivityStreamsType string
+	ActivityStreamsType string `bun:",nullzero"`
 	// Original text of the status without formatting
-	Text string
+	Text string `bun:",nullzero"`
 	// Has this status been pinned by its owner?
 	Pinned bool
 }

internal/gtsmodel/tag.go

@@ -25,7 +25,7 @@ type Tag struct {
 	// id of this tag in the database
 	ID string `bun:",unique,type:CHAR(26),pk,notnull"`
 	// Href of this tag, eg https://example.org/tags/somehashtag
-	URL string
+	URL string `bun:",nullzero"`
 	// name of this tag -- the tag without the hash part
 	Name string `bun:",unique,notnull"`
 	// Which account ID is the first one we saw using this tag?

internal/gtsmodel/user.go

@@ -67,7 +67,7 @@ type User struct {
 	// What languages does this user not want to see?
 	FilteredLanguages []string
 	// In what timezone/locale is this user located?
-	Locale string
+	Locale string `bun:",nullzero"`
 	// Which application id created this user? See gtsmodel.Application
 	CreatedByApplicationID string       `bun:"type:CHAR(26),nullzero"`
 	CreatedByApplication   *Application `bun:"rel:belongs-to"`
@@ -79,13 +79,13 @@ type User struct {
 	*/
 
 	// What confirmation token did we send this user/what are we expecting back?
-	ConfirmationToken string
+	ConfirmationToken string `bun:",nullzero"`
 	// When did the user confirm their email address
 	ConfirmedAt time.Time `bun:",nullzero"`
 	// When did we send email confirmation to this user?
 	ConfirmationSentAt time.Time `bun:",nullzero"`
 	// Email address that hasn't yet been confirmed
-	UnconfirmedEmail string
+	UnconfirmedEmail string `bun:",nullzero"`
 
 	/*
 		ACL FLAGS
@@ -105,18 +105,18 @@ type User struct {
 	*/
 
 	// The generated token that the user can use to reset their password
-	ResetPasswordToken string
+	ResetPasswordToken string `bun:",nullzero"`
 	// When did we email the user their reset-password email?
 	ResetPasswordSentAt time.Time `bun:",nullzero"`
 
-	EncryptedOTPSecret     string
-	EncryptedOTPSecretIv   string
-	EncryptedOTPSecretSalt string
+	EncryptedOTPSecret     string `bun:",nullzero"`
+	EncryptedOTPSecretIv   string `bun:",nullzero"`
+	EncryptedOTPSecretSalt string `bun:",nullzero"`
 	OTPRequiredForLogin    bool
 	OTPBackupCodes         []string
 	ConsumedTimestamp      int
-	RememberToken          string
-	SignInToken            string
+	RememberToken          string    `bun:",nullzero"`
+	SignInToken            string    `bun:",nullzero"`
 	SignInTokenSentAt      time.Time `bun:",nullzero"`
-	WebauthnID             string
+	WebauthnID             string    `bun:",nullzero"`
 }