[bugfix] self-referencing collection pages for status replies (#2364)

This commit is contained in:
kim
2023-11-20 12:22:28 +00:00
committed by GitHub
parent efefdb1323
commit 16275853eb
24 changed files with 611 additions and 427 deletions

View File

@@ -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
@@ -120,36 +119,43 @@ 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
@@ -157,7 +163,8 @@ func (m *Module) StatusRepliesGETHandler(c *gin.Context) {
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
}

View File

@@ -18,10 +18,10 @@
package users_test
import (
"bytes"
"context"
"encoding/json"
"fmt"
"io/ioutil"
"io"
"net/http"
"net/http/httptest"
"testing"
@@ -31,6 +31,7 @@ import (
"github.com/stretchr/testify/suite"
"github.com/superseriousbusiness/activity/streams"
"github.com/superseriousbusiness/activity/streams/vocab"
"github.com/superseriousbusiness/gotosocial/internal/ap"
"github.com/superseriousbusiness/gotosocial/internal/api/activitypub/users"
"github.com/superseriousbusiness/gotosocial/testrig"
)
@@ -49,7 +50,7 @@ func (suite *RepliesGetTestSuite) TestGetReplies() {
// setup request
recorder := httptest.NewRecorder()
ctx, _ := testrig.CreateGinTestContext(recorder, nil)
ctx.Request = httptest.NewRequest(http.MethodGet, targetStatus.URI+"/replies", nil) // the endpoint we're hitting
ctx.Request = httptest.NewRequest(http.MethodGet, targetStatus.URI+"/replies?only_other_accounts=false", 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)
@@ -76,13 +77,26 @@ func (suite *RepliesGetTestSuite) TestGetReplies() {
// check response
suite.EqualValues(http.StatusOK, recorder.Code)
// Read response body.
result := recorder.Result()
defer result.Body.Close()
b, err := ioutil.ReadAll(result.Body)
b, err := io.ReadAll(result.Body)
assert.NoError(suite.T(), err)
assert.Equal(suite.T(), `{"@context":"https://www.w3.org/ns/activitystreams","first":{"id":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?page=true","next":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?only_other_accounts=false\u0026page=true","partOf":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies","type":"CollectionPage"},"id":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies","type":"Collection"}`, string(b))
// should be a Collection
// Indent JSON
// for readability.
b = indentJSON(b)
// Create JSON string of expected output.
expect := toJSON(map[string]any{
"@context": "https://www.w3.org/ns/activitystreams",
"type": "OrderedCollection",
"id": targetStatus.URI + "/replies?only_other_accounts=false",
"first": targetStatus.URI + "/replies?limit=20&only_other_accounts=false",
"totalItems": 1,
})
assert.Equal(suite.T(), expect, string(b))
m := make(map[string]interface{})
err = json.Unmarshal(b, &m)
assert.NoError(suite.T(), err)
@@ -90,7 +104,7 @@ func (suite *RepliesGetTestSuite) TestGetReplies() {
t, err := streams.ToType(context.Background(), m)
assert.NoError(suite.T(), err)
_, ok := t.(vocab.ActivityStreamsCollection)
_, ok := t.(vocab.ActivityStreamsOrderedCollection)
assert.True(suite.T(), ok)
}
@@ -131,14 +145,29 @@ func (suite *RepliesGetTestSuite) TestGetRepliesNext() {
// check response
suite.EqualValues(http.StatusOK, recorder.Code)
// Read response body.
result := recorder.Result()
defer result.Body.Close()
b, err := ioutil.ReadAll(result.Body)
b, err := io.ReadAll(result.Body)
assert.NoError(suite.T(), err)
assert.Equal(suite.T(), `{"@context":"https://www.w3.org/ns/activitystreams","id":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?page=true\u0026only_other_accounts=false","items":"http://localhost:8080/users/admin/statuses/01FF25D5Q0DH7CHD57CTRS6WK0","next":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?only_other_accounts=false\u0026page=true\u0026min_id=01FF25D5Q0DH7CHD57CTRS6WK0","partOf":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies","type":"CollectionPage"}`, string(b))
// Indent JSON
// for readability.
b = indentJSON(b)
// Create JSON string of expected output.
expect := toJSON(map[string]any{
"@context": "https://www.w3.org/ns/activitystreams",
"type": "OrderedCollectionPage",
"id": targetStatus.URI + "/replies?limit=20&only_other_accounts=false",
"partOf": targetStatus.URI + "/replies?only_other_accounts=false",
"next": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false",
"prev": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&max_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false",
"orderedItems": "http://localhost:8080/users/admin/statuses/01FF25D5Q0DH7CHD57CTRS6WK0",
"totalItems": 1,
})
assert.Equal(suite.T(), expect, string(b))
// should be a Collection
m := make(map[string]interface{})
err = json.Unmarshal(b, &m)
assert.NoError(suite.T(), err)
@@ -146,10 +175,10 @@ func (suite *RepliesGetTestSuite) TestGetRepliesNext() {
t, err := streams.ToType(context.Background(), m)
assert.NoError(suite.T(), err)
page, ok := t.(vocab.ActivityStreamsCollectionPage)
page, ok := t.(vocab.ActivityStreamsOrderedCollectionPage)
assert.True(suite.T(), ok)
assert.Equal(suite.T(), page.GetActivityStreamsItems().Len(), 1)
assert.Equal(suite.T(), page.GetActivityStreamsOrderedItems().Len(), 1)
}
func (suite *RepliesGetTestSuite) TestGetRepliesLast() {
@@ -162,7 +191,7 @@ func (suite *RepliesGetTestSuite) TestGetRepliesLast() {
// setup request
recorder := httptest.NewRecorder()
ctx, _ := testrig.CreateGinTestContext(recorder, nil)
ctx.Request = httptest.NewRequest(http.MethodGet, targetStatus.URI+"/replies?only_other_accounts=false&page=true&min_id=01FF25D5Q0DH7CHD57CTRS6WK0", nil) // the endpoint we're hitting
ctx.Request = httptest.NewRequest(http.MethodGet, targetStatus.URI+"/replies?min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false", nil)
ctx.Request.Header.Set("accept", "application/activity+json")
ctx.Request.Header.Set("Signature", signedRequest.SignatureHeader)
ctx.Request.Header.Set("Date", signedRequest.DateHeader)
@@ -189,15 +218,27 @@ func (suite *RepliesGetTestSuite) TestGetRepliesLast() {
// check response
suite.EqualValues(http.StatusOK, recorder.Code)
// Read response body.
result := recorder.Result()
defer result.Body.Close()
b, err := ioutil.ReadAll(result.Body)
b, err := io.ReadAll(result.Body)
assert.NoError(suite.T(), err)
fmt.Println(string(b))
assert.Equal(suite.T(), `{"@context":"https://www.w3.org/ns/activitystreams","id":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?page=true\u0026only_other_accounts=false\u0026min_id=01FF25D5Q0DH7CHD57CTRS6WK0","items":[],"next":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?only_other_accounts=false\u0026page=true","partOf":"http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies","type":"CollectionPage"}`, string(b))
// Indent JSON
// for readability.
b = indentJSON(b)
// Create JSON string of expected output.
expect := toJSON(map[string]any{
"@context": "https://www.w3.org/ns/activitystreams",
"type": "OrderedCollectionPage",
"id": targetStatus.URI + "/replies?min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false",
"partOf": targetStatus.URI + "/replies?only_other_accounts=false",
"orderedItems": []any{}, // empty
"totalItems": 1,
})
assert.Equal(suite.T(), expect, string(b))
// should be a Collection
m := make(map[string]interface{})
err = json.Unmarshal(b, &m)
assert.NoError(suite.T(), err)
@@ -205,12 +246,39 @@ func (suite *RepliesGetTestSuite) TestGetRepliesLast() {
t, err := streams.ToType(context.Background(), m)
assert.NoError(suite.T(), err)
page, ok := t.(vocab.ActivityStreamsCollectionPage)
page, ok := t.(vocab.ActivityStreamsOrderedCollectionPage)
assert.True(suite.T(), ok)
assert.Equal(suite.T(), page.GetActivityStreamsItems().Len(), 0)
assert.Equal(suite.T(), page.GetActivityStreamsOrderedItems().Len(), 0)
}
func TestRepliesGetTestSuite(t *testing.T) {
suite.Run(t, new(RepliesGetTestSuite))
}
// toJSON will return indented JSON serialized form of 'a'.
func toJSON(a any) string {
v, ok := a.(vocab.Type)
if ok {
m, err := ap.Serialize(v)
if err != nil {
panic(err)
}
a = m
}
b, err := json.MarshalIndent(a, "", " ")
if err != nil {
panic(err)
}
return string(b)
}
// indentJSON will return indented JSON from raw provided JSON.
func indentJSON(b []byte) []byte {
var dst bytes.Buffer
err := json.Indent(&dst, b, "", " ")
if err != nil {
panic(err)
}
return dst.Bytes()
}