[bugfix] visibility after implicit approval not getting invalidated (#3370)

* replicate issue

* update go-structr to v0.8.10 with internal linked-list fix, small tweaks to caching of interaction requests

* remove debug function

---------

Co-authored-by: tobi <tobi.smethurst@protonmail.com>
This commit is contained in:
kim 2024-09-28 20:47:46 +00:00 committed by GitHub
commit 095663f5cc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 239 additions and 136 deletions

View file

@ -73,7 +73,7 @@ func (suite *AccountStatusesTestSuite) TestGetStatusesPublicOnly() {
suite.Equal(apimodel.VisibilityPublic, s.Visibility)
}
suite.Equal(`<http://localhost:8080/api/v1/accounts/01F8MH17FWEB39HZJ76B6VXSKF/statuses?limit=20&max_id=01F8MH75CBF9JFX4ZAD54N0W0R&exclude_replies=false&exclude_reblogs=false&pinned=false&only_media=false&only_public=true>; rel="next", <http://localhost:8080/api/v1/accounts/01F8MH17FWEB39HZJ76B6VXSKF/statuses?limit=20&min_id=01G36SF3V6Y6V5BF9P4R7PQG7G&exclude_replies=false&exclude_reblogs=false&pinned=false&only_media=false&only_public=true>; rel="prev"`, result.Header.Get("link"))
suite.Equal(`<http://localhost:8080/api/v1/accounts/01F8MH17FWEB39HZJ76B6VXSKF/statuses?limit=20&max_id=01F8MH75CBF9JFX4ZAD54N0W0R&exclude_replies=false&exclude_reblogs=false&pinned=false&only_media=false&only_public=true>; rel="next", <http://localhost:8080/api/v1/accounts/01F8MH17FWEB39HZJ76B6VXSKF/statuses?limit=20&min_id=01J5QVB9VC76NPPRQ207GG4DRZ&exclude_replies=false&exclude_reblogs=false&pinned=false&only_media=false&only_public=true>; rel="prev"`, result.Header.Get("link"))
}
func (suite *AccountStatusesTestSuite) TestGetStatusesPublicOnlyMediaOnly() {

View file

@ -591,7 +591,7 @@ func (suite *StatusBoostTestSuite) TestPostBoostImplicitAccept() {
"text": "Hi @1happyturtle, can I reply?",
"uri": "http://localhost:8080/some/determinate/url",
"url": "http://localhost:8080/some/determinate/url",
"visibility": "unlisted"
"visibility": "public"
},
"reblogged": true,
"reblogs_count": 0,
@ -601,7 +601,7 @@ func (suite *StatusBoostTestSuite) TestPostBoostImplicitAccept() {
"tags": [],
"uri": "http://localhost:8080/some/determinate/url",
"url": "http://localhost:8080/some/determinate/url",
"visibility": "unlisted"
"visibility": "public"
}`, out)
// Target status should no

View file

@ -27,6 +27,7 @@ import (
"github.com/gin-gonic/gin"
"github.com/stretchr/testify/suite"
"github.com/superseriousbusiness/gotosocial/internal/api/client/statuses"
"github.com/superseriousbusiness/gotosocial/internal/filter/visibility"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
@ -185,13 +186,24 @@ func (suite *StatusFaveTestSuite) TestPostUnfaveable() {
// Fave a status that's pending approval by us.
func (suite *StatusFaveTestSuite) TestPostFaveImplicitAccept() {
var (
ctx = context.Background()
targetStatus = suite.testStatuses["admin_account_status_5"]
app = suite.testApplications["application_1"]
token = suite.testTokens["local_account_2"]
user = suite.testUsers["local_account_2"]
account = suite.testAccounts["local_account_2"]
visFilter = visibility.NewFilter(&suite.state)
)
// Check visibility of status to public before posting fave.
visible, err := visFilter.StatusVisible(ctx, nil, targetStatus)
if err != nil {
suite.FailNow(err.Error())
}
if visible {
suite.FailNow("status should not be visible yet")
}
out, recorder := suite.postStatusFave(
targetStatus.ID,
app,
@ -268,30 +280,40 @@ func (suite *StatusFaveTestSuite) TestPostFaveImplicitAccept() {
"text": "Hi @1happyturtle, can I reply?",
"uri": "http://localhost:8080/some/determinate/url",
"url": "http://localhost:8080/some/determinate/url",
"visibility": "unlisted"
"visibility": "public"
}`, out)
// Target status should no
// longer be pending approval.
dbStatus, err := suite.state.DB.GetStatusByID(
context.Background(),
ctx,
targetStatus.ID,
)
if err != nil {
suite.FailNow(err.Error())
}
suite.False(*dbStatus.PendingApproval)
suite.NotEmpty(dbStatus.ApprovedByURI)
// There should be an Accept
// stored for the target status.
intReq, err := suite.state.DB.GetInteractionRequestByInteractionURI(
context.Background(), targetStatus.URI,
ctx, targetStatus.URI,
)
if err != nil {
suite.FailNow(err.Error())
}
suite.NotZero(intReq.AcceptedAt)
suite.NotEmpty(intReq.URI)
// Check visibility of status to public after posting fave.
visible, err = visFilter.StatusVisible(ctx, nil, dbStatus)
if err != nil {
suite.FailNow(err.Error())
}
if !visible {
suite.FailNow("status should be visible")
}
}
func TestStatusFaveTestSuite(t *testing.T) {

View file

@ -18,7 +18,6 @@
package cache
import (
"database/sql"
"errors"
"time"
@ -42,7 +41,6 @@ func ignoreErrors(err error) bool {
// (until invalidation).
db.ErrNoEntries,
db.ErrAlreadyExists,
sql.ErrNoRows,
)
}

View file

@ -48,9 +48,15 @@ func (c *Caches) initVisibility() {
{Fields: "RequesterID", Multiple: true},
{Fields: "Type,RequesterID,ItemID"},
},
MaxSize: cap,
IgnoreErr: ignoreErrors,
Copy: copyF,
MaxSize: cap,
IgnoreErr: func(err error) bool {
// don't cache any errors,
// it gets a little too tricky
// otherwise with ensuring
// errors are cleared out
return true
},
Copy: copyF,
})
}

View file

@ -26,8 +26,10 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/paging"
"github.com/superseriousbusiness/gotosocial/internal/state"
"github.com/superseriousbusiness/gotosocial/internal/util"
"github.com/uptrace/bun"
)
@ -84,6 +86,53 @@ func (i *interactionDB) GetInteractionRequestByURI(ctx context.Context, uri stri
)
}
func (i *interactionDB) GetInteractionRequestsByIDs(ctx context.Context, ids []string) ([]*gtsmodel.InteractionRequest, error) {
// Load all interaction request IDs via cache loader callbacks.
requests, err := i.state.Caches.DB.InteractionRequest.LoadIDs("ID",
ids,
func(uncached []string) ([]*gtsmodel.InteractionRequest, error) {
// Preallocate expected length of uncached interaction requests.
requests := make([]*gtsmodel.InteractionRequest, 0, len(uncached))
// Perform database query scanning
// the remaining (uncached) IDs.
if err := i.db.NewSelect().
Model(&requests).
Where("? IN (?)", bun.Ident("id"), bun.In(uncached)).
Scan(ctx); err != nil {
return nil, err
}
return requests, nil
},
)
if err != nil {
return nil, err
}
// Reorder the requests by their
// IDs to ensure in correct order.
getID := func(r *gtsmodel.InteractionRequest) string { return r.ID }
util.OrderBy(requests, ids, getID)
if gtscontext.Barebones(ctx) {
// no need to fully populate.
return requests, nil
}
// Populate all loaded interaction requests, removing those we
// fail to populate (removes needing so many nil checks everywhere).
requests = slices.DeleteFunc(requests, func(request *gtsmodel.InteractionRequest) bool {
if err := i.PopulateInteractionRequest(ctx, request); err != nil {
log.Errorf(ctx, "error populating %s: %v", request.ID, err)
return true
}
return false
})
return requests, nil
}
func (i *interactionDB) getInteractionRequest(
ctx context.Context,
lookup string,
@ -205,13 +254,18 @@ func (i *interactionDB) UpdateInteractionRequest(ctx context.Context, request *g
}
func (i *interactionDB) DeleteInteractionRequestByID(ctx context.Context, id string) error {
defer i.state.Caches.DB.InteractionRequest.Invalidate("ID", id)
// Delete interaction request by ID.
if _, err := i.db.NewDelete().
Table("interaction_requests").
Where("? = ?", bun.Ident("id"), id).
Exec(ctx); err != nil {
return err
}
_, err := i.db.NewDelete().
TableExpr("? AS ?", bun.Ident("interaction_requests"), bun.Ident("interaction_request")).
Where("? = ?", bun.Ident("interaction_request.id"), id).
Exec(ctx)
return err
// Invalidate cached interaction request with ID.
i.state.Caches.DB.InteractionRequest.Invalidate("ID", id)
return nil
}
func (i *interactionDB) GetInteractionsRequestsForAcct(
@ -317,19 +371,8 @@ func (i *interactionDB) GetInteractionsRequestsForAcct(
slices.Reverse(reqIDs)
}
// For each interaction request ID,
// select the interaction request.
reqs := make([]*gtsmodel.InteractionRequest, 0, len(reqIDs))
for _, id := range reqIDs {
req, err := i.GetInteractionRequestByID(ctx, id)
if err != nil {
return nil, err
}
reqs = append(reqs, req)
}
return reqs, nil
// Load all interaction requests by their IDs.
return i.GetInteractionRequestsByIDs(ctx, reqIDs)
}
func (i *interactionDB) IsInteractionRejected(ctx context.Context, interactionURI string) (bool, error) {

View file

@ -74,7 +74,8 @@ func (suite *TimelineTestSuite) publicCount() int {
var publicCount int
for _, status := range suite.testStatuses {
if status.Visibility == gtsmodel.VisibilityPublic &&
status.BoostOfID == "" {
status.BoostOfID == "" &&
!util.PtrOrZero(status.PendingApproval) {
publicCount++
}
}

View file

@ -168,6 +168,9 @@ func (suite *StatusVisibleTestSuite) TestVisiblePending() {
testStatus := new(gtsmodel.Status)
*testStatus = *suite.testStatuses["admin_account_status_3"]
testStatus.PendingApproval = util.Ptr(true)
if err := suite.state.DB.UpdateStatus(ctx, testStatus); err != nil {
suite.FailNow(err.Error())
}
for _, testCase := range []struct {
acct *gtsmodel.Account
@ -198,6 +201,43 @@ func (suite *StatusVisibleTestSuite) TestVisiblePending() {
suite.NoError(err)
suite.Equal(testCase.visible, visible)
}
// Update the status to mark it as approved.
testStatus.PendingApproval = util.Ptr(false)
testStatus.ApprovedByURI = "http://localhost:8080/some/accept/uri"
if err := suite.state.DB.UpdateStatus(ctx, testStatus); err != nil {
suite.FailNow(err.Error())
}
for _, testCase := range []struct {
acct *gtsmodel.Account
visible bool
}{
{
acct: suite.testAccounts["admin_account"],
visible: true, // Own status, always visible.
},
{
acct: suite.testAccounts["local_account_1"],
visible: true, // Reply to zork, always visible.
},
{
acct: suite.testAccounts["local_account_2"],
visible: true, // Should be visible now.
},
{
acct: suite.testAccounts["remote_account_1"],
visible: true, // Should be visible now.
},
{
acct: nil, // Unauthed request.
visible: true, // Should be visible now (public status).
},
} {
visible, err := suite.filter.StatusVisible(ctx, testCase.acct, testStatus)
suite.NoError(err)
suite.Equal(testCase.visible, visible)
}
}
func (suite *StatusVisibleTestSuite) TestVisibleLocalOnly() {

View file

@ -1744,7 +1744,7 @@ func (suite *InternalToFrontendTestSuite) TestStatusToAPIStatusPendingApproval()
"in_reply_to_account_id": "01F8MH5NBDF2MV7CTC4Q5128HF",
"sensitive": false,
"spoiler_text": "",
"visibility": "unlisted",
"visibility": "public",
"language": null,
"uri": "http://localhost:8080/users/admin/statuses/01J5QVB9VC76NPPRQ207GG4DRZ",
"url": "http://localhost:8080/@admin/statuses/01J5QVB9VC76NPPRQ207GG4DRZ",
@ -3177,7 +3177,7 @@ func (suite *InternalToFrontendTestSuite) TestIntReqToAPI() {
"in_reply_to_account_id": null,
"sensitive": true,
"spoiler_text": "you won't be able to reply to this without my approval",
"visibility": "unlisted",
"visibility": "public",
"language": "en",
"uri": "http://localhost:8080/users/1happyturtle/statuses/01F8MHC8VWDRBQR0N1BATDDEM5",
"url": "http://localhost:8080/@1happyturtle/statuses/01F8MHC8VWDRBQR0N1BATDDEM5",
@ -3269,7 +3269,7 @@ func (suite *InternalToFrontendTestSuite) TestIntReqToAPI() {
"in_reply_to_account_id": "01F8MH5NBDF2MV7CTC4Q5128HF",
"sensitive": false,
"spoiler_text": "",
"visibility": "unlisted",
"visibility": "public",
"language": null,
"uri": "http://localhost:8080/users/admin/statuses/01J5QVB9VC76NPPRQ207GG4DRZ",
"url": "http://localhost:8080/@admin/statuses/01J5QVB9VC76NPPRQ207GG4DRZ",