[chore] Harden up boolptr logic on Accounts, warn if not set (#2544)

internal/ap/extract.go

@@ -492,18 +492,6 @@ func ExtractFields(i WithAttachment) []*gtsmodel.Field {
 	return fields
 }
 
-// ExtractDiscoverable extracts the Discoverable boolean
-// of the given WithDiscoverable interface. Will return
-// an error if Discoverable was nil.
-func ExtractDiscoverable(i WithDiscoverable) (bool, error) {
-	discoverableProp := i.GetTootDiscoverable()
-	if discoverableProp == nil {
-		return false, gtserror.New("discoverable was nil")
-	}
-
-	return discoverableProp.Get(), nil
-}
-
 // ExtractURL extracts the first URI it can find from the
 // given WithURL interface, or an error if no URL was set.
 // The ID of a type will not work, this function wants a URI

internal/ap/properties.go

@@ -424,6 +424,8 @@ func SetVotersCount(with WithVotersCount, count int) {
 }
 
 // GetDiscoverable returns the boolean contained in the Discoverable property of 'with'.
+//
+// Returns default 'false' if property unusable or not set.
 func GetDiscoverable(with WithDiscoverable) bool {
 	discoverProp := with.GetTootDiscoverable()
 	if discoverProp == nil || !discoverProp.IsXMLSchemaBoolean() {
@@ -442,6 +444,27 @@ func SetDiscoverable(with WithDiscoverable, discoverable bool) {
 	discoverProp.Set(discoverable)
 }
 
+// GetManuallyApprovesFollowers returns the boolean contained in the ManuallyApprovesFollowers property of 'with'.
+//
+// Returns default 'true' if property unusable or not set.
+func GetManuallyApprovesFollowers(with WithManuallyApprovesFollowers) bool {
+	mafProp := with.GetActivityStreamsManuallyApprovesFollowers()
+	if mafProp == nil || !mafProp.IsXMLSchemaBoolean() {
+		return true
+	}
+	return mafProp.Get()
+}
+
+// SetManuallyApprovesFollowers sets the given boolean on the ManuallyApprovesFollowers property of 'with'.
+func SetManuallyApprovesFollowers(with WithManuallyApprovesFollowers, manuallyApprovesFollowers bool) {
+	mafProp := with.GetActivityStreamsManuallyApprovesFollowers()
+	if mafProp == nil {
+		mafProp = streams.NewActivityStreamsManuallyApprovesFollowersProperty()
+		with.SetActivityStreamsManuallyApprovesFollowers(mafProp)
+	}
+	mafProp.Set(manuallyApprovesFollowers)
+}
+
 func getIRIs[T TypeOrIRI](prop Property[T]) []*url.URL {
 	if prop == nil || prop.Len() == 0 {
 		return nil

internal/db/bundb/account_test.go

@@ -336,6 +336,7 @@ func (suite *AccountTestSuite) TestInsertAccountWithDefaults() {
 	suite.Equal("en", newAccount.Language)
 	suite.WithinDuration(time.Now(), newAccount.CreatedAt, 30*time.Second)
 	suite.WithinDuration(time.Now(), newAccount.UpdatedAt, 30*time.Second)
+	suite.True(*newAccount.Locked)
 	suite.False(*newAccount.Memorial)
 	suite.False(*newAccount.Bot)
 	suite.False(*newAccount.Discoverable)

internal/typeutils/astointernal.go

@@ -126,23 +126,9 @@ func (c *Converter) ASRepresentationToAccount(ctx context.Context, accountable a
 	acct.Sensitive = util.Ptr(false)
 	acct.HideCollections = util.Ptr(false)
 
-	// Extract 'manuallyApprovesFollowers', (i.e. locked account)
-	maf := accountable.GetActivityStreamsManuallyApprovesFollowers()
-
-	switch {
-	case maf != nil && !maf.IsXMLSchemaBoolean():
-		log.Warnf(ctx, "unusable manuallyApprovesFollowers for %s", uri)
-		fallthrough
-
-	case maf == nil:
-		// None given, use default.
-		acct.Locked = util.Ptr(true)
-
-	default:
-		// Valid bool provided.
-		locked := maf.Get()
-		acct.Locked = &locked
-	}
+	// Extract 'manuallyApprovesFollowers' aka locked account (default = true).
+	manuallyApprovesFollowers := ap.GetManuallyApprovesFollowers(accountable)
+	acct.Locked = &manuallyApprovesFollowers
 
 	// Extract account discoverability (default = false).
 	discoverable := ap.GetDiscoverable(accountable)

internal/typeutils/astointernal_test.go

@@ -27,6 +27,7 @@
 	"github.com/superseriousbusiness/activity/streams"
 	"github.com/superseriousbusiness/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
+	"github.com/superseriousbusiness/gotosocial/internal/cache"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 )
 
@@ -35,6 +36,17 @@ type ASToInternalTestSuite struct {
 }
 
 func (suite *ASToInternalTestSuite) jsonToType(in string) vocab.Type {
+	ctx := context.Background()
+	b := []byte(in)
+
+	if accountable, err := ap.ResolveAccountable(ctx, b); err == nil {
+		return accountable
+	}
+
+	if statusable, err := ap.ResolveStatusable(ctx, b); err == nil {
+		return statusable
+	}
+
 	m := make(map[string]interface{})
 	if err := json.Unmarshal([]byte(in), &m); err != nil {
 		suite.FailNow(err.Error())
@@ -45,10 +57,6 @@ func (suite *ASToInternalTestSuite) jsonToType(in string) vocab.Type {
 		suite.FailNow(err.Error())
 	}
 
-	if statusable, ok := t.(ap.Statusable); ok {
-		ap.NormalizeIncomingContent(statusable, m)
-	}
-
 	return t
 }
 
@@ -195,7 +203,7 @@ func (suite *ASToInternalTestSuite) TestParseOwncastService() {
 	suite.Equal("https://owncast.example.org/logo/external", acct.AvatarRemoteURL)
 	suite.Equal("https://owncast.example.org/logo/external", acct.HeaderRemoteURL)
 	suite.Equal("Rob's Owncast Server", acct.DisplayName)
-	suite.Equal("linux audio stuff ", acct.Note)
+	suite.Equal("linux audio stuff", acct.Note)
 	suite.True(*acct.Bot)
 	suite.False(*acct.Locked)
 	suite.True(*acct.Discoverable)
@@ -503,6 +511,145 @@ func (suite *ASToInternalTestSuite) TestParseAnnounce() {
 	suite.Nil(boost.BoostOfAccount)
 }
 
+func (suite *ASToInternalTestSuite) TestParseHonkAccount() {
+	// Hopefully comprehensive checks for
+	// https://github.com/superseriousbusiness/gotosocial/issues/2527.
+
+	const honk_user = `{
+  "@context": "https://www.w3.org/ns/activitystreams",
+  "chatKeyV0": "vIT5wj9bJqGkvwBxhmaz4Lh4eZIeOKnsSIQifShmJUY=",
+  "followers": "https://honk.example.org/u/honk_user/followers",
+  "following": "https://honk.example.org/u/honk_user/following",
+  "icon": {
+    "mediaType": "image/png",
+    "type": "Image",
+    "url": "https://honk.example.org/a?a=https%3A%2F%2Fhonk.example.org%2Fu%2Fhonk_user"
+  },
+  "id": "https://honk.example.org/u/honk_user",
+  "inbox": "https://honk.example.org/u/honk_user/inbox",
+  "name": "honk_user",
+  "outbox": "https://honk.example.org/u/honk_user/outbox",
+  "preferredUsername": "honk_user",
+  "publicKey": {
+    "id": "https://honk.example.org/u/honk_user#key",
+    "owner": "https://honk.example.org/u/honk_user",
+    "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA593GZ9TYrvWgMaMKQ6k6\ngkItUapUgNnNXzU9J63GRtYZ7CE/Zi39Kgpsxu77hHBj34vwjr1Oc9AMrVDIMfu9\nEirW1RWxPvrjThBU56VgkpkAXVsieaffJo80BA00QzV4x69Jgat6OT7ox/HMvMxR\nyZ6CXNCPKQALYqQF6v1fX1kO9lhIA+mPd0JN/qMKvZfd1NXABEk9nORUneH7Audt\nIHNdJzKMHC6wPSQWC7SmXT0/nq6o5mR2SgvwTI/JUx6T5r8NDrwSaqB69e+EMJqR\nxKOh9N4A1ba/AQOQZbO/YkFyYY2VE4HWbvS9XpYL74yT9D6Fp4cUovJiXC+ziam0\nNwIDAQAB\n-----END PUBLIC KEY-----\n"
+  },
+  "summary": "<p>Honk account</p>",
+  "type": "Person",
+  "url": "https://honk.example.org/u/honk_user"
+}`
+
+	t := suite.jsonToType(honk_user)
+	rep, ok := t.(ap.Accountable)
+	if !ok {
+		suite.FailNow("type not coercible")
+	}
+
+	acct, err := suite.typeconverter.ASRepresentationToAccount(context.Background(), rep, "")
+	suite.NoError(err)
+	suite.Equal("https://honk.example.org/u/honk_user/followers", acct.FollowersURI)
+	suite.Equal("https://honk.example.org/u/honk_user/following", acct.FollowingURI)
+	suite.Equal(`https://honk.example.org/a?a=https%3A%2F%2Fhonk.example.org%2Fu%2Fhonk_user`, acct.AvatarRemoteURL)
+	suite.Equal("<p>Honk account</p>", acct.Note)
+	suite.Equal("https://honk.example.org/u/honk_user", acct.URI)
+	suite.Equal("https://honk.example.org/u/honk_user", acct.URL)
+	suite.Equal("honk_user", acct.Username)
+	suite.Equal("honk.example.org", acct.Domain)
+	suite.True(*acct.Locked)
+	suite.False(*acct.Discoverable)
+
+	// Store the account representation.
+	acct.ID = "01HMGRMAVQMYQC3DDQ29TPQKJ3" // <- needs an ID
+	ctx := context.Background()
+	if err := suite.db.PutAccount(ctx, acct); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Double check fields.
+	suite.Equal("https://honk.example.org/u/honk_user/followers", acct.FollowersURI)
+	suite.Equal("https://honk.example.org/u/honk_user/following", acct.FollowingURI)
+	suite.Equal(`https://honk.example.org/a?a=https%3A%2F%2Fhonk.example.org%2Fu%2Fhonk_user`, acct.AvatarRemoteURL)
+	suite.Equal("<p>Honk account</p>", acct.Note)
+	suite.Equal("https://honk.example.org/u/honk_user", acct.URI)
+	suite.Equal("https://honk.example.org/u/honk_user", acct.URL)
+	suite.Equal("honk_user", acct.Username)
+	suite.Equal("honk.example.org", acct.Domain)
+	suite.True(*acct.Locked)
+	suite.False(*acct.Discoverable)
+
+	// Check DB version.
+	dbAcct, err := suite.db.GetAccountByID(ctx, acct.ID)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.Equal("https://honk.example.org/u/honk_user/followers", dbAcct.FollowersURI)
+	suite.Equal("https://honk.example.org/u/honk_user/following", dbAcct.FollowingURI)
+	suite.Equal(`https://honk.example.org/a?a=https%3A%2F%2Fhonk.example.org%2Fu%2Fhonk_user`, dbAcct.AvatarRemoteURL)
+	suite.Equal("<p>Honk account</p>", dbAcct.Note)
+	suite.Equal("https://honk.example.org/u/honk_user", dbAcct.URI)
+	suite.Equal("https://honk.example.org/u/honk_user", dbAcct.URL)
+	suite.Equal("honk_user", dbAcct.Username)
+	suite.Equal("honk.example.org", dbAcct.Domain)
+	suite.True(*dbAcct.Locked)
+	suite.False(*dbAcct.Discoverable)
+
+	// Update the account.
+	if err := suite.db.UpdateAccount(ctx, acct); err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// Double check fields.
+	suite.Equal("https://honk.example.org/u/honk_user/followers", acct.FollowersURI)
+	suite.Equal("https://honk.example.org/u/honk_user/following", acct.FollowingURI)
+	suite.Equal(`https://honk.example.org/a?a=https%3A%2F%2Fhonk.example.org%2Fu%2Fhonk_user`, acct.AvatarRemoteURL)
+	suite.Equal("<p>Honk account</p>", acct.Note)
+	suite.Equal("https://honk.example.org/u/honk_user", acct.URI)
+	suite.Equal("https://honk.example.org/u/honk_user", acct.URL)
+	suite.Equal("honk_user", acct.Username)
+	suite.Equal("honk.example.org", acct.Domain)
+	suite.True(*acct.Locked)
+	suite.False(*acct.Discoverable)
+
+	// Check DB version.
+	dbAcct, err = suite.db.GetAccountByID(ctx, acct.ID)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.Equal("https://honk.example.org/u/honk_user/followers", dbAcct.FollowersURI)
+	suite.Equal("https://honk.example.org/u/honk_user/following", dbAcct.FollowingURI)
+	suite.Equal(`https://honk.example.org/a?a=https%3A%2F%2Fhonk.example.org%2Fu%2Fhonk_user`, dbAcct.AvatarRemoteURL)
+	suite.Equal("<p>Honk account</p>", dbAcct.Note)
+	suite.Equal("https://honk.example.org/u/honk_user", dbAcct.URI)
+	suite.Equal("https://honk.example.org/u/honk_user", dbAcct.URL)
+	suite.Equal("honk_user", dbAcct.Username)
+	suite.Equal("honk.example.org", dbAcct.Domain)
+	suite.True(*dbAcct.Locked)
+	suite.False(*dbAcct.Discoverable)
+
+	// Clear caches.
+	suite.state.Caches.GTS = cache.GTSCaches{}
+	suite.state.Caches.GTS.Init()
+
+	dbAcct, err = suite.db.GetAccountByID(ctx, acct.ID)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.Equal("https://honk.example.org/u/honk_user/followers", dbAcct.FollowersURI)
+	suite.Equal("https://honk.example.org/u/honk_user/following", dbAcct.FollowingURI)
+	suite.Equal(`https://honk.example.org/a?a=https%3A%2F%2Fhonk.example.org%2Fu%2Fhonk_user`, dbAcct.AvatarRemoteURL)
+	suite.Equal("<p>Honk account</p>", dbAcct.Note)
+	suite.Equal("https://honk.example.org/u/honk_user", dbAcct.URI)
+	suite.Equal("https://honk.example.org/u/honk_user", dbAcct.URL)
+	suite.Equal("honk_user", dbAcct.Username)
+	suite.Equal("honk.example.org", dbAcct.Domain)
+	suite.True(*dbAcct.Locked)
+	suite.False(*dbAcct.Discoverable)
+}
+
 func TestASToInternalTestSuite(t *testing.T) {
 	suite.Run(t, new(ASToInternalTestSuite))
 }

internal/typeutils/internaltofrontend.go

@@ -217,6 +217,31 @@ func (c *Converter) AccountToAPIAccountPublic(ctx context.Context, a *gtsmodel.A
 		}
 	}
 
+	// Bool ptrs should be set, but warn
+	// and use a default if they're not.
+	var boolPtrDef = func(
+		pName string,
+		p *bool,
+		d bool,
+	) bool {
+		if p != nil {
+			return *p
+		}
+
+		log.Warnf(ctx,
+			"%s ptr was nil, using default %t",
+			pName, d,
+		)
+		return d
+	}
+
+	var (
+		locked       = boolPtrDef("locked", a.Locked, true)
+		discoverable = boolPtrDef("discoverable", a.Discoverable, false)
+		bot          = boolPtrDef("bot", a.Bot, false)
+		enableRSS    = boolPtrDef("enableRSS", a.EnableRSS, false)
+	)
+
 	// Remaining properties are simple and
 	// can be populated directly below.
 
@@ -225,9 +250,9 @@ func (c *Converter) AccountToAPIAccountPublic(ctx context.Context, a *gtsmodel.A
 		Username:       a.Username,
 		Acct:           acct,
 		DisplayName:    a.DisplayName,
-		Locked:         *a.Locked,
-		Discoverable:   *a.Discoverable,
-		Bot:            *a.Bot,
+		Locked:         locked,
+		Discoverable:   discoverable,
+		Bot:            bot,
 		CreatedAt:      util.FormatISO8601(a.CreatedAt),
 		Note:           a.Note,
 		URL:            a.URL,
@@ -243,7 +268,7 @@ func (c *Converter) AccountToAPIAccountPublic(ctx context.Context, a *gtsmodel.A
 		Fields:         fields,
 		Suspended:      !a.SuspendedAt.IsZero(),
 		CustomCSS:      a.CustomCSS,
-		EnableRSS:      *a.EnableRSS,
+		EnableRSS:      enableRSS,
 		Role:           role,
 		Moved:          moved,
 	}