[bugfix] self-referencing collection pages for status replies

internal/ap/activitystreams_test.go

@@ -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) {
@@ -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) {
@@ -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) {
@@ -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 {

internal/ap/collections.go

@@ -20,7 +20,6 @@ package ap
 import (
 	"fmt"
 	"net/url"
-	"strconv"
 
 	"github.com/superseriousbusiness/activity/streams"
 	"github.com/superseriousbusiness/activity/streams/vocab"
@@ -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
 }
@@ -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
 }
 
@@ -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
 }
 
@@ -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)
@@ -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()
@@ -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
@@ -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 {
@@ -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 {
@@ -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
 }

internal/api/activitypub/users/repliesget.go

@@ -18,16 +18,15 @@
 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
@@ -120,46 +119,46 @@ 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
-	}
-
-	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
-	}
+	var onlyOtherAccounts bool
 
-	minID := ""
-	minIDString := c.Query(MinIDKey)
-	if minIDString != "" {
-		minID = minIDString
+	// Look for supplied 'onlyOtherAccounts' query key.
+	if raw, ok := c.GetQuery("only_other_accounts"); ok {
+		onlyOtherAccounts, _ = strconv.ParseBool(raw)
+	} else { // fallback to true.
+		onlyOtherAccounts = true
 	}
 
-	resp, errWithCode := m.processor.Fedi().StatusRepliesGet(c.Request.Context(), requestedUsername, requestedStatusID, page, onlyOtherAccounts, c.Query("only_other_accounts") != "", minID)
+	// 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
 	}
 
-	b, err := json.Marshal(resp)
-	if err != nil {
-		apiutil.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)
+	// 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
+	}
+
+	// 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
 	}
 
-	c.Data(http.StatusOK, format, b)
+	c.JSON(http.StatusOK, resp)
 }