[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>

9 files changed, 144 insertions, 34 deletions

diff --git a/internal/api/client/accounts/statuses_test.go b/internal/api/client/accounts/statuses_test.go
index 2c32e6571..f7a304967 100644
--- a/internal/api/client/accounts/statuses_test.go
+++ b/internal/api/client/accounts/statuses_test.go
@@ -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() {
diff --git a/internal/api/client/statuses/statusboost_test.go b/internal/api/client/statuses/statusboost_test.go
index 8642ba7aa..1f92d8b3f 100644
--- a/internal/api/client/statuses/statusboost_test.go
+++ b/internal/api/client/statuses/statusboost_test.go
@@ -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
diff --git a/internal/api/client/statuses/statusfave_test.go b/internal/api/client/statuses/statusfave_test.go
index fdc8741c7..bd81c0cf9 100644
--- a/internal/api/client/statuses/statusfave_test.go
+++ b/internal/api/client/statuses/statusfave_test.go
@@ -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) {
diff --git a/internal/cache/util.go b/internal/cache/util.go
index 924869aad..fde2f9ada 100644
--- a/internal/cache/util.go
+++ b/internal/cache/util.go
@@ -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,
 	)
 }
 
diff --git a/internal/cache/visibility.go b/internal/cache/visibility.go
index e280054ec..a424ca5ac 100644
--- a/internal/cache/visibility.go
+++ b/internal/cache/visibility.go
@@ -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,
 	})
 }
 
diff --git a/internal/db/bundb/interaction.go b/internal/db/bundb/interaction.go
index 78abcc763..88a044b6f 100644
--- a/internal/db/bundb/interaction.go
+++ b/internal/db/bundb/interaction.go
@@ -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
+	}
+
+	// Invalidate cached interaction request with ID.
+	i.state.Caches.DB.InteractionRequest.Invalidate("ID", id)
 
-	_, 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
+	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) {
diff --git a/internal/db/bundb/timeline_test.go b/internal/db/bundb/timeline_test.go
index 50747b50d..00df2b3a6 100644
--- a/internal/db/bundb/timeline_test.go
+++ b/internal/db/bundb/timeline_test.go
@@ -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++
 		}
 	}
diff --git a/internal/filter/visibility/status_test.go b/internal/filter/visibility/status_test.go
index 9b210e500..795441e7f 100644
--- a/internal/filter/visibility/status_test.go
+++ b/internal/filter/visibility/status_test.go
@@ -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() {
diff --git a/internal/typeutils/internaltofrontend_test.go b/internal/typeutils/internaltofrontend_test.go
index 61faf1c21..57c401e6f 100644
--- a/internal/typeutils/internaltofrontend_test.go
+++ b/internal/typeutils/internaltofrontend_test.go
@@ -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",