Skip to content

Commit

Permalink
[bugfix] self-referencing collection pages for status replies (#2364)
Browse files Browse the repository at this point in the history
  • Loading branch information
@NyaaaWhatsUpDoc
NyaaaWhatsUpDoc committed Nov 20, 2023
1 parent efefdb1 commit 1627585
Show file tree
Hide file tree
Showing 24 changed files with 610 additions and 426 deletions.
10 changes: 6 additions & 4 deletions internal/ap/activitystreams_test.go
View file
Expand Up @@ -49,14 +49,15 @@ func TestASCollection(t *testing.T) {
// Create new collection using builder function.
c := ap.NewASCollection(ap.CollectionParams{
ID: parseURI(idURI),
Query: url.Values{"limit": []string{"40"}},
Total: total,
})

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

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

func TestASCollectionPage(t *testing.T) {
Expand Down Expand Up @@ -110,7 +111,7 @@ func TestASCollectionPage(t *testing.T) {
s := toJSON(p)

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

func TestASOrderedCollection(t *testing.T) {
Expand All @@ -131,14 +132,15 @@ func TestASOrderedCollection(t *testing.T) {
// Create new collection using builder function.
c := ap.NewASOrderedCollection(ap.CollectionParams{
ID: parseURI(idURI),
Query: url.Values{"limit": []string{"40"}},
Total: total,
})

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

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

func TestASOrderedCollectionPage(t *testing.T) {
Expand Down Expand Up @@ -192,7 +194,7 @@ func TestASOrderedCollectionPage(t *testing.T) {
s := toJSON(p)

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

func parseURI(s string) *url.URL {
Expand Down
59 changes: 38 additions & 21 deletions internal/ap/collections.go
View file
Expand Up @@ -20,7 +20,6 @@ package ap
import (
"fmt"
"net/url"
"strconv"

"github.com/superseriousbusiness/activity/streams"
"github.com/superseriousbusiness/activity/streams/vocab"
Expand Down Expand Up @@ -169,6 +168,10 @@ type CollectionParams struct {
// ID (i.e. NOT the page).
ID *url.URL

// First page details.
First paging.Page
Query url.Values

// Total no. items.
Total int
}
Expand Down Expand Up @@ -224,7 +227,7 @@ type ItemsPropertyBuilder interface {
// NewASCollection builds and returns a new ActivityStreams Collection from given parameters.
func NewASCollection(params CollectionParams) vocab.ActivityStreamsCollection {
collection := streams.NewActivityStreamsCollection()
buildCollection(collection, params, 40)
buildCollection(collection, params)
return collection
}

Expand All @@ -239,7 +242,7 @@ func NewASCollectionPage(params CollectionPageParams) vocab.ActivityStreamsColle
// NewASOrderedCollection builds and returns a new ActivityStreams OrderedCollection from given parameters.
func NewASOrderedCollection(params CollectionParams) vocab.ActivityStreamsOrderedCollection {
collection := streams.NewActivityStreamsOrderedCollection()
buildCollection(collection, params, 40)
buildCollection(collection, params)
return collection
}

Expand All @@ -251,7 +254,7 @@ func NewASOrderedCollectionPage(params CollectionPageParams) vocab.ActivityStrea
return collectionPage
}

func buildCollection[C CollectionBuilder](collection C, params CollectionParams, pageLimit int) {
func buildCollection[C CollectionBuilder](collection C, params CollectionParams) {
// Add the collection ID property.
idProp := streams.NewJSONLDIdProperty()
idProp.SetIRI(params.ID)
Expand All @@ -262,15 +265,20 @@ func buildCollection[C CollectionBuilder](collection C, params CollectionParams,
totalItems.Set(params.Total)
collection.SetActivityStreamsTotalItems(totalItems)

// Clone the collection ID page
// to add first page query data.
firstIRI := new(url.URL)
*firstIRI = *params.ID
// Append paging query params
// to those already in ID prop.
pageQueryParams := appendQuery(
params.Query,
params.ID.Query(),
)

// Note that simply adding a limit signals to our
// endpoint to use paging (which will start at beginning).
limit := "limit=" + strconv.Itoa(pageLimit)
firstIRI.RawQuery = appendQuery(firstIRI.RawQuery, limit)
// Build the first page link IRI.
firstIRI := params.First.ToLinkURL(
params.ID.Scheme,
params.ID.Host,
params.ID.Path,
pageQueryParams,
)

// Add the collection first IRI property.
first := streams.NewActivityStreamsFirstProperty()
Expand All @@ -284,12 +292,19 @@ func buildCollectionPage[C CollectionPageBuilder, I ItemsPropertyBuilder](collec
partOfProp.SetIRI(params.ID)
collectionPage.SetActivityStreamsPartOf(partOfProp)

// Append paging query params
// to those already in ID prop.
pageQueryParams := appendQuery(
params.Query,
params.ID.Query(),
)

// Build the current page link IRI.
currentIRI := params.Current.ToLinkURL(
params.ID.Scheme,
params.ID.Host,
params.ID.Path,
params.Query,
pageQueryParams,
)

// Add the collection ID property for
Expand All @@ -303,7 +318,7 @@ func buildCollectionPage[C CollectionPageBuilder, I ItemsPropertyBuilder](collec
params.ID.Scheme,
params.ID.Host,
params.ID.Path,
params.Query,
pageQueryParams,
)

if nextIRI != nil {
Expand All @@ -318,7 +333,7 @@ func buildCollectionPage[C CollectionPageBuilder, I ItemsPropertyBuilder](collec
params.ID.Scheme,
params.ID.Host,
params.ID.Path,
params.Query,
pageQueryParams,
)

if prevIRI != nil {
Expand Down Expand Up @@ -349,11 +364,13 @@ func buildCollectionPage[C CollectionPageBuilder, I ItemsPropertyBuilder](collec
setItems(itemsProp)
}

// appendQuery appends part to an existing raw
// query with ampersand, else just returning part.
func appendQuery(raw, part string) string {
if raw != "" {
return raw + "&" + part
// appendQuery appends query values in 'src' to 'dst', returning 'dst'.
func appendQuery(dst, src url.Values) url.Values {
if dst == nil {
return src
}
for k, vs := range src {
dst[k] = append(dst[k], vs...)
}
return part
return dst
}
61 changes: 34 additions & 27 deletions internal/api/activitypub/users/repliesget.go
View file
Expand Up @@ -20,14 +20,13 @@ package users
import (
"encoding/json"
"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"
)

// StatusRepliesGETHandler swagger:operation GET /users/{username}/statuses/{status}/replies s2sRepliesGet
Expand Down Expand Up @@ -120,44 +119,52 @@ func (m *Module) StatusRepliesGETHandler(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
// Look for supplied 'only_other_accounts' query key.
onlyOtherAccounts, errWithCode := apiutil.ParseOnlyOtherAccounts(
c.Query(apiutil.OnlyOtherAccountsKey),
true, // default = enabled
)
if errWithCode != nil {
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
}

onlyOtherAccounts := false
onlyOtherAccountsString := c.Query(OnlyOtherAccountsKey)
if onlyOtherAccountsString != "" {
i, err := strconv.ParseBool(onlyOtherAccountsString)
if err != nil {
err := fmt.Errorf("error parsing %s: %s", OnlyOtherAccountsKey, err)
apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
return
}
onlyOtherAccounts = i
// Look for given paging query parameters.
page, errWithCode := paging.ParseIDPage(c,
1, // min limit
40, // max limit
0, // default = disabled
)
if errWithCode != nil {
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
}

minID := ""
minIDString := c.Query(MinIDKey)
if minIDString != "" {
minID = minIDString
// COMPATIBILITY FIX: 'page=true' enables paging.
if page == nil && c.Query("page") == "true" {
page = new(paging.Page)
page.Max = paging.MaxID("")
page.Min = paging.MinID("")
page.Limit = 20 // default
}

resp, errWithCode := m.processor.Fedi().StatusRepliesGet(c.Request.Context(), requestedUsername, requestedStatusID, page, onlyOtherAccounts, c.Query("only_other_accounts") != "", minID)
// Fetch serialized status replies response for input status.
resp, errWithCode := m.processor.Fedi().StatusRepliesGet(
c.Request.Context(),
requestedUsername,
requestedStatusID,
page,
onlyOtherAccounts,
)
if errWithCode != nil {
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
}

b, err := json.Marshal(resp)
if err != nil {
apiutil.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)
errWithCode := gtserror.NewErrorInternalError(err)
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
}

Expand Down

0 comments on commit 1627585

Please sign in to comment.