Skip to content

Commit

Permalink
[bugfix] Deref stats async, serve stub collections if handshaking (#2990
Browse files Browse the repository at this point in the history
)

* [bugfix] Deref stats async, allow peek if handshaking

* don't return totalItems when handshaking or hiding collections

* use GetLimit()

* use StubAccountStats
  • Loading branch information
tsmethurst authored Jun 11, 2024
1 parent fd6637d commit 611f9de
Show file tree
Hide file tree
Showing 14 changed files with 391 additions and 240 deletions.
37 changes: 31 additions & 6 deletions internal/ap/activitystreams_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/superseriousbusiness/activity/streams/vocab"
"github.com/superseriousbusiness/gotosocial/internal/ap"
"github.com/superseriousbusiness/gotosocial/internal/paging"
"github.com/superseriousbusiness/gotosocial/internal/util"
)

func TestASCollection(t *testing.T) {
Expand All @@ -51,7 +52,7 @@ func TestASCollection(t *testing.T) {
ID: parseURI(idURI),
First: new(paging.Page),
Query: url.Values{"limit": []string{"40"}},
Total: total,
Total: util.Ptr(total),
})

// Serialize collection.
Expand Down Expand Up @@ -82,7 +83,7 @@ func TestASCollectionTotalOnly(t *testing.T) {
// Create new collection using builder function.
c := ap.NewASCollection(ap.CollectionParams{
ID: parseURI(idURI),
Total: total,
Total: util.Ptr(total),
})

// Serialize collection.
Expand Down Expand Up @@ -128,7 +129,7 @@ func TestASCollectionPage(t *testing.T) {
p := ap.NewASCollectionPage(ap.CollectionPageParams{
CollectionParams: ap.CollectionParams{
ID: parseURI(idURI),
Total: total,
Total: util.Ptr(total),
},

Current: currPg,
Expand Down Expand Up @@ -166,7 +167,7 @@ func TestASOrderedCollection(t *testing.T) {
ID: parseURI(idURI),
First: new(paging.Page),
Query: url.Values{"limit": []string{"40"}},
Total: total,
Total: util.Ptr(total),
})

// Serialize collection.
Expand All @@ -193,7 +194,31 @@ func TestASOrderedCollectionTotalOnly(t *testing.T) {
// Create new collection using builder function.
c := ap.NewASOrderedCollection(ap.CollectionParams{
ID: parseURI(idURI),
Total: total,
Total: util.Ptr(total),
})

// Serialize collection.
s := toJSON(c)

// Ensure outputs are equal.
assert.Equal(t, expect, s)
}

func TestASOrderedCollectionNoTotal(t *testing.T) {
const (
idURI = "https://zorg.flabormagorg.xyz/users/itsa_me_mario"
)

// Create JSON string of expected output.
expect := toJSON(map[string]any{
"@context": "https://www.w3.org/ns/activitystreams",
"type": "OrderedCollection",
"id": idURI,
})

// Create new collection using builder function.
c := ap.NewASOrderedCollection(ap.CollectionParams{
ID: parseURI(idURI),
})

// Serialize collection.
Expand Down Expand Up @@ -239,7 +264,7 @@ func TestASOrderedCollectionPage(t *testing.T) {
p := ap.NewASOrderedCollectionPage(ap.CollectionPageParams{
CollectionParams: ap.CollectionParams{
ID: parseURI(idURI),
Total: total,
Total: util.Ptr(total),
},

Current: currPg,
Expand Down
20 changes: 13 additions & 7 deletions internal/ap/collections.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ type CollectionParams struct {
Query url.Values

// Total no. items.
Total int
// Omitted if nil.
Total *int
}

type CollectionPageParams struct {
Expand Down Expand Up @@ -367,6 +368,7 @@ type CollectionPageBuilder interface {
// vocab.ActivityStreamsOrderedItemsProperty
type ItemsPropertyBuilder interface {
AppendIRI(*url.URL)
AppendActivityStreamsCreate(vocab.ActivityStreamsCreate)

// NOTE: add more of the items-property-like interface
// functions here as you require them for building pages.
Expand Down Expand Up @@ -409,9 +411,11 @@ func buildCollection[C CollectionBuilder](collection C, params CollectionParams)
collection.SetJSONLDId(idProp)

// Add the collection totalItems count property.
totalItems := streams.NewActivityStreamsTotalItemsProperty()
totalItems.Set(params.Total)
collection.SetActivityStreamsTotalItems(totalItems)
if params.Total != nil {
totalItems := streams.NewActivityStreamsTotalItemsProperty()
totalItems.Set(*params.Total)
collection.SetActivityStreamsTotalItems(totalItems)
}

// No First page means we're done.
if params.First == nil {
Expand Down Expand Up @@ -497,9 +501,11 @@ func buildCollectionPage[C CollectionPageBuilder, I ItemsPropertyBuilder](collec
}

// Add the collection totalItems count property.
totalItems := streams.NewActivityStreamsTotalItemsProperty()
totalItems.Set(params.Total)
collectionPage.SetActivityStreamsTotalItems(totalItems)
if params.Total != nil {
totalItems := streams.NewActivityStreamsTotalItemsProperty()
totalItems.Set(*params.Total)
collectionPage.SetActivityStreamsTotalItems(totalItems)
}

if params.Append == nil {
// nil check outside the for loop.
Expand Down
34 changes: 10 additions & 24 deletions internal/api/activitypub/users/outboxget.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@ package users

import (
"errors"
"fmt"
"net/http"
"strconv"
"strings"

"github.com/gin-gonic/gin"
apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/paging"
)

// OutboxGETHandler swagger:operation GET /users/{username}/outbox s2sOutboxGet
Expand Down Expand Up @@ -105,30 +104,17 @@ func (m *Module) OutboxGETHandler(c *gin.Context) {
return
}

var page bool
if pageString := c.Query(PageKey); pageString != "" {
i, err := strconv.ParseBool(pageString)
if err != nil {
err := fmt.Errorf("error parsing %s: %s", PageKey, err)
apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
return
}
page = i
}

minID := ""
minIDString := c.Query(MinIDKey)
if minIDString != "" {
minID = minIDString
}

maxID := ""
maxIDString := c.Query(MaxIDKey)
if maxIDString != "" {
maxID = maxIDString
page, errWithCode := paging.ParseIDPage(c,
1, // min limit
80, // max limit
0, // default = disabled
)
if errWithCode != nil {
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
}

resp, errWithCode := m.processor.Fedi().OutboxGet(c.Request.Context(), requestedUsername, page, maxID, minID)
resp, errWithCode := m.processor.Fedi().OutboxGet(c.Request.Context(), requestedUsername, page)
if errWithCode != nil {
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
Expand Down
17 changes: 10 additions & 7 deletions internal/api/activitypub/users/outboxget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ func (suite *OutboxGetTestSuite) TestGetOutbox() {
suite.NoError(err)
suite.Equal(`{
"@context": "https://www.w3.org/ns/activitystreams",
"first": "http://localhost:8080/users/the_mighty_zork/outbox?page=true",
"first": "http://localhost:8080/users/the_mighty_zork/outbox?limit=40",
"id": "http://localhost:8080/users/the_mighty_zork/outbox",
"totalItems": 7,
"type": "OrderedCollection"
}`, dst.String())

Expand All @@ -105,7 +106,7 @@ func (suite *OutboxGetTestSuite) TestGetOutboxFirstPage() {
// setup request
recorder := httptest.NewRecorder()
ctx, _ := testrig.CreateGinTestContext(recorder, nil)
ctx.Request = httptest.NewRequest(http.MethodGet, targetAccount.OutboxURI+"?page=true", nil) // the endpoint we're hitting
ctx.Request = httptest.NewRequest(http.MethodGet, targetAccount.OutboxURI+"?limit=40", nil) // the endpoint we're hitting
ctx.Request.Header.Set("accept", "application/activity+json")
ctx.Request.Header.Set("Signature", signedRequest.SignatureHeader)
ctx.Request.Header.Set("Date", signedRequest.DateHeader)
Expand Down Expand Up @@ -138,8 +139,8 @@ func (suite *OutboxGetTestSuite) TestGetOutboxFirstPage() {
suite.NoError(err)
suite.Equal(`{
"@context": "https://www.w3.org/ns/activitystreams",
"id": "http://localhost:8080/users/the_mighty_zork/outbox?page=true",
"next": "http://localhost:8080/users/the_mighty_zork/outbox?page=true\u0026max_id=01F8MHAMCHF6Y650WCRSCP4WMY",
"id": "http://localhost:8080/users/the_mighty_zork/outbox?limit=40",
"next": "http://localhost:8080/users/the_mighty_zork/outbox?limit=40\u0026max_id=01F8MHAMCHF6Y650WCRSCP4WMY",
"orderedItems": [
{
"actor": "http://localhost:8080/users/the_mighty_zork",
Expand All @@ -159,7 +160,8 @@ func (suite *OutboxGetTestSuite) TestGetOutboxFirstPage() {
}
],
"partOf": "http://localhost:8080/users/the_mighty_zork/outbox",
"prev": "http://localhost:8080/users/the_mighty_zork/outbox?page=true\u0026min_id=01HH9KYNQPA416TNJ53NSATP40",
"prev": "http://localhost:8080/users/the_mighty_zork/outbox?limit=40\u0026min_id=01HH9KYNQPA416TNJ53NSATP40",
"totalItems": 7,
"type": "OrderedCollectionPage"
}`, dst.String())

Expand All @@ -183,7 +185,7 @@ func (suite *OutboxGetTestSuite) TestGetOutboxNextPage() {
// setup request
recorder := httptest.NewRecorder()
ctx, _ := testrig.CreateGinTestContext(recorder, nil)
ctx.Request = httptest.NewRequest(http.MethodGet, targetAccount.OutboxURI+"?page=true&max_id=01F8MHAMCHF6Y650WCRSCP4WMY", nil) // the endpoint we're hitting
ctx.Request = httptest.NewRequest(http.MethodGet, targetAccount.OutboxURI+"?limit=40&max_id=01F8MHAMCHF6Y650WCRSCP4WMY", nil) // the endpoint we're hitting
ctx.Request.Header.Set("accept", "application/activity+json")
ctx.Request.Header.Set("Signature", signedRequest.SignatureHeader)
ctx.Request.Header.Set("Date", signedRequest.DateHeader)
Expand Down Expand Up @@ -219,9 +221,10 @@ func (suite *OutboxGetTestSuite) TestGetOutboxNextPage() {
suite.NoError(err)
suite.Equal(`{
"@context": "https://www.w3.org/ns/activitystreams",
"id": "http://localhost:8080/users/the_mighty_zork/outbox?page=true&maxID=01F8MHAMCHF6Y650WCRSCP4WMY",
"id": "http://localhost:8080/users/the_mighty_zork/outbox?limit=40&max_id=01F8MHAMCHF6Y650WCRSCP4WMY",
"orderedItems": [],
"partOf": "http://localhost:8080/users/the_mighty_zork/outbox",
"totalItems": 7,
"type": "OrderedCollectionPage"
}`, dst.String())

Expand Down
17 changes: 15 additions & 2 deletions internal/db/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,23 @@ type Account interface {
// Update local account settings.
UpdateAccountSettings(ctx context.Context, settings *gtsmodel.AccountSettings, columns ...string) error

// PopulateAccountStats gets (or creates and gets) account stats for
// the given account, and attaches them to the account model.
// PopulateAccountStats either creates account stats for the given
// account by performing COUNT(*) database queries, or retrieves
// existing stats from the database, and attaches stats to account.
//
// If account is local and stats were last regenerated > 48 hours ago,
// stats will always be regenerated using COUNT(*) queries, to prevent drift.
PopulateAccountStats(ctx context.Context, account *gtsmodel.Account) error

// StubAccountStats creates zeroed account stats for the given account,
// skipping COUNT(*) queries, upserts them in the DB, and attaches them
// to the account model.
//
// Useful following fresh dereference of a remote account, or fresh
// creation of a local account, when you know all COUNT(*) queries
// would return 0 anyway.
StubAccountStats(ctx context.Context, account *gtsmodel.Account) error

// RegenerateAccountStats creates, upserts, and returns stats
// for the given account, and attaches them to the account model.
//
Expand Down
29 changes: 29 additions & 0 deletions internal/db/bundb/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,35 @@ func (a *accountDB) PopulateAccountStats(ctx context.Context, account *gtsmodel.
return nil
}

func (a *accountDB) StubAccountStats(ctx context.Context, account *gtsmodel.Account) error {
stats := &gtsmodel.AccountStats{
AccountID: account.ID,
RegeneratedAt: time.Now(),
FollowersCount: util.Ptr(0),
FollowingCount: util.Ptr(0),
FollowRequestsCount: util.Ptr(0),
StatusesCount: util.Ptr(0),
StatusesPinnedCount: util.Ptr(0),
}

// Upsert this stats in case a race
// meant someone else inserted it first.
if err := a.state.Caches.GTS.AccountStats.Store(stats, func() error {
if _, err := NewUpsert(a.db).
Model(stats).
Constraint("account_id").
Exec(ctx); err != nil {
return err
}
return nil
}); err != nil {
return err
}

account.Stats = stats
return nil
}

func (a *accountDB) RegenerateAccountStats(ctx context.Context, account *gtsmodel.Account) error {
// Initialize a new stats struct.
stats := &gtsmodel.AccountStats{
Expand Down
25 changes: 14 additions & 11 deletions internal/db/bundb/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,6 @@ func (a *adminDB) NewSignup(ctx context.Context, newSignup gtsmodel.NewSignup) (
return nil, err
}

settings := &gtsmodel.AccountSettings{
AccountID: accountID,
Privacy: gtsmodel.VisibilityDefault,
}

// Insert the settings!
if err := a.state.DB.PutAccountSettings(ctx, settings); err != nil {
return nil, err
}

account = &gtsmodel.Account{
ID: accountID,
Username: newSignup.Username,
Expand All @@ -145,13 +135,26 @@ func (a *adminDB) NewSignup(ctx context.Context, newSignup gtsmodel.NewSignup) (
PrivateKey: privKey,
PublicKey: &privKey.PublicKey,
PublicKeyURI: uris.PublicKeyURI,
Settings: settings,
}

// Insert the new account!
if err := a.state.DB.PutAccount(ctx, account); err != nil {
return nil, err
}

// Insert basic settings for new account.
account.Settings = &gtsmodel.AccountSettings{
AccountID: accountID,
Privacy: gtsmodel.VisibilityDefault,
}
if err := a.state.DB.PutAccountSettings(ctx, account.Settings); err != nil {
return nil, err
}

// Stub empty stats for new account.
if err := a.state.DB.StubAccountStats(ctx, account); err != nil {
return nil, err
}
}

// Created or already had an account.
Expand Down
Loading

0 comments on commit 611f9de

Please sign in to comment.