[bugfix] Log + ignore unknown notification types (#3577)

* [bugfix] Log + ignore unknown notification types

* pass context to ParseNotificationTypes

internal/api/client/notifications/notificationsget.go

@@ -18,11 +18,14 @@
 package notifications
 
 import (
+	"context"
 	"net/http"
 
 	"github.com/gin-gonic/gin"
 	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
+	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+	"github.com/superseriousbusiness/gotosocial/internal/log"
 	"github.com/superseriousbusiness/gotosocial/internal/oauth"
 	"github.com/superseriousbusiness/gotosocial/internal/paging"
 )
@@ -151,18 +154,6 @@ func (m *Module) NotificationsGETHandler(c *gin.Context) {
 		return
 	}
 
-	types, errWithCode := apiutil.ParseNotificationTypes(c.QueryArray(TypesKey))
-	if errWithCode != nil {
-		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
-		return
-	}
-
-	exclTypes, errWithCode := apiutil.ParseNotificationTypes(c.QueryArray(ExcludeTypesKey))
-	if errWithCode != nil {
-		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
-		return
-	}
-
 	page, errWithCode := paging.ParseIDPage(c,
 		1,  // min limit
 		80, // max limit
@@ -173,12 +164,13 @@ func (m *Module) NotificationsGETHandler(c *gin.Context) {
 		return
 	}
 
+	ctx := c.Request.Context()
 	resp, errWithCode := m.processor.Timeline().NotificationsGet(
-		c.Request.Context(),
+		ctx,
 		authed,
 		page,
-		types,
-		exclTypes,
+		ParseNotificationTypes(ctx, c.QueryArray(TypesKey)),        // Include types.
+		ParseNotificationTypes(ctx, c.QueryArray(ExcludeTypesKey)), // Exclude types.
 	)
 	if errWithCode != nil {
 		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
@@ -191,3 +183,28 @@ func (m *Module) NotificationsGETHandler(c *gin.Context) {
 
 	apiutil.JSON(c, http.StatusOK, resp.Items)
 }
+
+// ParseNotificationTypes converts the given slice of string values
+// to gtsmodel notification types, logging + skipping unknown types.
+func ParseNotificationTypes(
+	ctx context.Context,
+	values []string,
+) []gtsmodel.NotificationType {
+	if len(values) == 0 {
+		return nil
+	}
+
+	ntypes := make([]gtsmodel.NotificationType, 0, len(values))
+	for _, value := range values {
+		ntype := gtsmodel.NewNotificationType(value)
+		if ntype == gtsmodel.NotificationUnknown {
+			// Type we don't know about (yet), log and ignore it.
+			log.Debugf(ctx, "ignoring unknown type %s", value)
+			continue
+		}
+
+		ntypes = append(ntypes, ntype)
+	}
+
+	return ntypes
+}

internal/api/client/notifications/notificationsget_test.go

@@ -248,6 +248,45 @@ func (suite *NotificationsTestSuite) TestGetNotificationsIncludeOneType() {
 	}
 }
 
+// Test including an unknown notification type, it should be ignored.
+func (suite *NotificationsTestSuite) TestGetNotificationsIncludeUnknownType() {
+	testAccount := suite.testAccounts["local_account_1"]
+	testToken := suite.testTokens["local_account_1"]
+	testUser := suite.testUsers["local_account_1"]
+
+	suite.addMoreNotifications(testAccount)
+
+	maxID := ""
+	minID := ""
+	limit := 10
+	types := []string{"favourite", "something.weird"}
+	excludeTypes := []string(nil)
+	expectedHTTPStatus := http.StatusOK
+	expectedBody := ""
+
+	notifications, _, err := suite.getNotifications(
+		testAccount,
+		testToken,
+		testUser,
+		maxID,
+		minID,
+		limit,
+		types,
+		excludeTypes,
+		expectedHTTPStatus,
+		expectedBody,
+	)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	// This should only include the fav notification.
+	suite.Len(notifications, 1)
+	for _, notification := range notifications {
+		suite.Equal("favourite", notification.Type)
+	}
+}
+
 func TestBookmarkTestSuite(t *testing.T) {
 	suite.Run(t, new(NotificationsTestSuite))
 }

internal/api/util/parsequery.go

@@ -18,13 +18,11 @@
 package util
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 	"strings"
 
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
-	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 )
 
 const (
@@ -218,51 +216,6 @@ func ParseInteractionReblogs(value string, defaultValue bool) (bool, gtserror.Wi
 	return parseBool(value, defaultValue, InteractionReblogsKey)
 }
 
-func ParseNotificationType(value string) (gtsmodel.NotificationType, gtserror.WithCode) {
-	switch strings.ToLower(value) {
-	case "follow":
-		return gtsmodel.NotificationFollow, nil
-	case "follow_request":
-		return gtsmodel.NotificationFollowRequest, nil
-	case "mention":
-		return gtsmodel.NotificationMention, nil
-	case "reblog":
-		return gtsmodel.NotificationReblog, nil
-	case "favourite":
-		return gtsmodel.NotificationFave, nil
-	case "poll":
-		return gtsmodel.NotificationPoll, nil
-	case "status":
-		return gtsmodel.NotificationStatus, nil
-	case "admin.sign_up":
-		return gtsmodel.NotificationSignup, nil
-	case "pending.favourite":
-		return gtsmodel.NotificationPendingFave, nil
-	case "pending.reply":
-		return gtsmodel.NotificationPendingReply, nil
-	case "pending.reblog":
-		return gtsmodel.NotificationPendingReblog, nil
-	default:
-		text := fmt.Sprintf("unrecognized notification type %s", value)
-		return 0, gtserror.NewErrorBadRequest(errors.New(text), text)
-	}
-}
-
-func ParseNotificationTypes(values []string) ([]gtsmodel.NotificationType, gtserror.WithCode) {
-	if len(values) == 0 {
-		return nil, nil
-	}
-	ntypes := make([]gtsmodel.NotificationType, len(values))
-	for i, value := range values {
-		ntype, errWithCode := ParseNotificationType(value)
-		if errWithCode != nil {
-			return nil, errWithCode
-		}
-		ntypes[i] = ntype
-	}
-	return ntypes, nil
-}
-
 /*
 	Parse functions for *REQUIRED* parameters.
 */

internal/gtsmodel/notification.go

@@ -17,7 +17,10 @@
 
 package gtsmodel
 
-import "time"
+import (
+	"strings"
+	"time"
+)
 
 // Notification models an alert/notification sent to an account about something like a reblog, like, new follow request, etc.
 type Notification struct {
@@ -40,6 +43,7 @@ type NotificationType enumType
 
 const (
 	// Notification Types
+	NotificationUnknown       NotificationType = 0  // NotificationUnknown -- unknown notification type, error if this occurs
 	NotificationFollow        NotificationType = 1  // NotificationFollow -- someone followed you
 	NotificationFollowRequest NotificationType = 2  // NotificationFollowRequest -- someone requested to follow you
 	NotificationMention       NotificationType = 3  // NotificationMention -- someone mentioned you in their status
@@ -82,3 +86,33 @@ func (t NotificationType) String() string {
 		panic("invalid notification type")
 	}
 }
+
+// NewNotificationType returns a notification type from the given value.
+func NewNotificationType(in string) NotificationType {
+	switch strings.ToLower(in) {
+	case "follow":
+		return NotificationFollow
+	case "follow_request":
+		return NotificationFollowRequest
+	case "mention":
+		return NotificationMention
+	case "reblog":
+		return NotificationReblog
+	case "favourite":
+		return NotificationFave
+	case "poll":
+		return NotificationPoll
+	case "status":
+		return NotificationStatus
+	case "admin.sign_up":
+		return NotificationSignup
+	case "pending.favourite":
+		return NotificationPendingFave
+	case "pending.reply":
+		return NotificationPendingReply
+	case "pending.reblog":
+		return NotificationPendingReblog
+	default:
+		return NotificationUnknown
+	}
+}