diff --git a/docs/api/swagger.yaml b/docs/api/swagger.yaml index ab54bebd..540a7dbc 100644 --- a/docs/api/swagger.yaml +++ b/docs/api/swagger.yaml @@ -4574,6 +4574,18 @@ paths: ```` operationId: notifications parameters: + - description: Return only notifications *OLDER* than the given max notification ID. The notification with the specified ID will not be included in the response. + in: query + name: max_id + type: string + - description: Return only notifications *newer* than the given since notification ID. The notification with the specified ID will not be included in the response. + in: query + name: since_id + type: string + - description: Return only notifications *immediately newer* than the given since notification ID. The notification with the specified ID will not be included in the response. + in: query + name: min_id + type: string - default: 20 description: Number of notifications to return. in: query @@ -4584,16 +4596,6 @@ paths: type: string name: exclude_types type: array - - description: Return only notifications *OLDER* than the given max status ID. The status with the specified ID will not be included in the response. - in: query - name: max_id - type: string - - description: |- - Return only notifications *NEWER* than the given since status ID. - The status with the specified ID will not be included in the response. - in: query - name: since_id - type: string produces: - application/json responses: diff --git a/internal/api/client/notifications/notifications.go b/internal/api/client/notifications/notifications.go index 52b06f38..8e089046 100644 --- a/internal/api/client/notifications/notifications.go +++ b/internal/api/client/notifications/notifications.go @@ -36,12 +36,10 @@ const ( // ExcludeTypes is an array specifying notification types to exclude ExcludeTypesKey = "exclude_types[]" - // MaxIDKey is the url query for setting a max notification ID to return - MaxIDKey = "max_id" - // LimitKey is for specifying maximum number of notifications to return. - LimitKey = "limit" - // SinceIDKey is for specifying the minimum notification ID to return. - SinceIDKey = "since_id" + MaxIDKey = "max_id" + LimitKey = "limit" + SinceIDKey = "since_id" + MinIDKey = "min_id" ) type Module struct { diff --git a/internal/api/client/notifications/notificationsget.go b/internal/api/client/notifications/notificationsget.go index adfc1bcc..6ce8adca 100644 --- a/internal/api/client/notifications/notificationsget.go +++ b/internal/api/client/notifications/notificationsget.go @@ -50,6 +50,29 @@ import ( // // parameters: // - +// name: max_id +// type: string +// description: >- +// Return only notifications *OLDER* than the given max notification ID. +// The notification with the specified ID will not be included in the response. +// in: query +// required: false +// - +// name: since_id +// type: string +// description: >- +// Return only notifications *newer* than the given since notification ID. +// The notification with the specified ID will not be included in the response. +// in: query +// - +// name: min_id +// type: string +// description: >- +// Return only notifications *immediately newer* than the given since notification ID. +// The notification with the specified ID will not be included in the response. +// in: query +// required: false +// - // name: limit // type: integer // description: Number of notifications to return. @@ -64,22 +87,6 @@ import ( // description: Array of types of notifications to exclude (follow, favourite, reblog, mention, poll, follow_request) // in: query // required: false -// - -// name: max_id -// type: string -// description: >- -// Return only notifications *OLDER* than the given max status ID. -// The status with the specified ID will not be included in the response. -// in: query -// required: false -// - -// name: since_id -// type: string -// description: |- -// Return only notifications *NEWER* than the given since status ID. -// The status with the specified ID will not be included in the response. -// in: query -// required: false // // security: // - OAuth2 Bearer: @@ -131,21 +138,15 @@ func (m *Module) NotificationsGETHandler(c *gin.Context) { limit = int(i) } - maxID := "" - maxIDString := c.Query(MaxIDKey) - if maxIDString != "" { - maxID = maxIDString - } - - sinceID := "" - sinceIDString := c.Query(SinceIDKey) - if sinceIDString != "" { - sinceID = sinceIDString - } - - excludeTypes := c.QueryArray(ExcludeTypesKey) - - resp, errWithCode := m.processor.NotificationsGet(c.Request.Context(), authed, excludeTypes, limit, maxID, sinceID) + resp, errWithCode := m.processor.NotificationsGet( + c.Request.Context(), + authed, + c.Query(MaxIDKey), + c.Query(SinceIDKey), + c.Query(MinIDKey), + limit, + c.QueryArray(ExcludeTypesKey), + ) if errWithCode != nil { apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) return diff --git a/internal/db/bundb/notification.go b/internal/db/bundb/notification.go index 1cc286f4..f2ff60b9 100644 --- a/internal/db/bundb/notification.go +++ b/internal/db/bundb/notification.go @@ -23,6 +23,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/id" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/state" "github.com/uptrace/bun" @@ -73,53 +74,93 @@ func (n *notificationDB) GetNotification( }, notificationType, targetAccountID, originAccountID, statusID) } -func (n *notificationDB) GetAccountNotifications(ctx context.Context, accountID string, excludeTypes []string, limit int, maxID string, sinceID string) ([]*gtsmodel.Notification, db.Error) { +func (n *notificationDB) GetAccountNotifications( + ctx context.Context, + accountID string, + maxID string, + sinceID string, + minID string, + limit int, + excludeTypes []string, +) ([]*gtsmodel.Notification, db.Error) { // Ensure reasonable if limit < 0 { limit = 0 } - // Make a guess for slice size - notifIDs := make([]string, 0, limit) + // Make educated guess for slice size + var ( + notifIDs = make([]string, 0, limit) + frontToBack = true + ) q := n.conn. NewSelect(). TableExpr("? AS ?", bun.Ident("notifications"), bun.Ident("notification")). Column("notification.id") - if maxID != "" { - q = q.Where("? < ?", bun.Ident("notification.id"), maxID) + if maxID == "" { + maxID = id.Highest } + // Return only notifs LOWER (ie., older) than maxID. + q = q.Where("? < ?", bun.Ident("notification.id"), maxID) + if sinceID != "" { + // Return only notifs HIGHER (ie., newer) than sinceID. q = q.Where("? > ?", bun.Ident("notification.id"), sinceID) } + if minID != "" { + // Return only notifs HIGHER (ie., newer) than minID. + q = q.Where("? > ?", bun.Ident("notification.id"), minID) + + frontToBack = false // page up + } + for _, excludeType := range excludeTypes { + // Filter out unwanted notif types. q = q.Where("? != ?", bun.Ident("notification.notification_type"), excludeType) } - q = q. - Where("? = ?", bun.Ident("notification.target_account_id"), accountID). - Order("notification.id DESC") + // Return only notifs for this account. + q = q.Where("? = ?", bun.Ident("notification.target_account_id"), accountID) - if limit != 0 { + if limit > 0 { q = q.Limit(limit) } + if frontToBack { + // Page down. + q = q.Order("notification.id DESC") + } else { + // Page up. + q = q.Order("notification.id ASC") + } + if err := q.Scan(ctx, ¬ifIDs); err != nil { return nil, n.conn.ProcessError(err) } - notifs := make([]*gtsmodel.Notification, 0, limit) + if len(notifIDs) == 0 { + return nil, nil + } - // now we have the IDs, select the notifs one by one - // reason for this is that for each notif, we can instead get it from our cache if it's cached + // If we're paging up, we still want notifications + // to be sorted by ID desc, so reverse ids slice. + // https://zchee.github.io/golang-wiki/SliceTricks/#reversing + if !frontToBack { + for l, r := 0, len(notifIDs)-1; l < r; l, r = l+1, r-1 { + notifIDs[l], notifIDs[r] = notifIDs[r], notifIDs[l] + } + } + + notifs := make([]*gtsmodel.Notification, 0, len(notifIDs)) for _, id := range notifIDs { // Attempt fetch from DB notif, err := n.GetNotificationByID(ctx, id) if err != nil { - log.Errorf(ctx, "error getting notification %q: %v", id, err) + log.Errorf(ctx, "error fetching notification %q: %v", id, err) continue } diff --git a/internal/db/bundb/notification_test.go b/internal/db/bundb/notification_test.go index bdee911b..8bbea78a 100644 --- a/internal/db/bundb/notification_test.go +++ b/internal/db/bundb/notification_test.go @@ -89,7 +89,7 @@ func (suite *NotificationTestSuite) TestGetAccountNotificationsWithSpam() { suite.spamNotifs() testAccount := suite.testAccounts["local_account_1"] before := time.Now() - notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, []string{}, 20, id.Highest, id.Lowest) + notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, id.Highest, id.Lowest, "", 20, nil) suite.NoError(err) timeTaken := time.Since(before) fmt.Printf("\n\n\n withSpam: got %d notifications in %s\n\n\n", len(notifications), timeTaken) @@ -103,7 +103,7 @@ func (suite *NotificationTestSuite) TestGetAccountNotificationsWithSpam() { func (suite *NotificationTestSuite) TestGetAccountNotificationsWithoutSpam() { testAccount := suite.testAccounts["local_account_1"] before := time.Now() - notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, []string{}, 20, id.Highest, id.Lowest) + notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, id.Highest, id.Lowest, "", 20, nil) suite.NoError(err) timeTaken := time.Since(before) fmt.Printf("\n\n\n withoutSpam: got %d notifications in %s\n\n\n", len(notifications), timeTaken) @@ -120,9 +120,9 @@ func (suite *NotificationTestSuite) TestDeleteNotificationsWithSpam() { err := suite.db.DeleteNotifications(context.Background(), nil, testAccount.ID, "") suite.NoError(err) - notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, []string{}, 20, id.Highest, id.Lowest) + notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, id.Highest, id.Lowest, "", 20, nil) suite.NoError(err) - suite.NotNil(notifications) + suite.Nil(notifications) suite.Empty(notifications) } @@ -132,9 +132,9 @@ func (suite *NotificationTestSuite) TestDeleteNotificationsWithTwoAccounts() { err := suite.db.DeleteNotifications(context.Background(), nil, testAccount.ID, "") suite.NoError(err) - notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, []string{}, 20, id.Highest, id.Lowest) + notifications, err := suite.db.GetAccountNotifications(context.Background(), testAccount.ID, id.Highest, id.Lowest, "", 20, nil) suite.NoError(err) - suite.NotNil(notifications) + suite.Nil(notifications) suite.Empty(notifications) notif := []*gtsmodel.Notification{} diff --git a/internal/db/notification.go b/internal/db/notification.go index c4860093..c17cf3d9 100644 --- a/internal/db/notification.go +++ b/internal/db/notification.go @@ -28,7 +28,7 @@ type Notification interface { // GetNotifications returns a slice of notifications that pertain to the given accountID. // // Returned notifications will be ordered ID descending (ie., highest/newest to lowest/oldest). - GetAccountNotifications(ctx context.Context, accountID string, excludeTypes []string, limit int, maxID string, sinceID string) ([]*gtsmodel.Notification, Error) + GetAccountNotifications(ctx context.Context, accountID string, maxID string, sinceID string, minID string, limit int, excludeTypes []string) ([]*gtsmodel.Notification, Error) // GetNotification returns one notification according to its id. GetNotificationByID(ctx context.Context, id string) (*gtsmodel.Notification, Error) diff --git a/internal/processing/notification.go b/internal/processing/notification.go index 1cfe71b2..2e4e1788 100644 --- a/internal/processing/notification.go +++ b/internal/processing/notification.go @@ -31,34 +31,69 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/util" ) -func (p *Processor) NotificationsGet(ctx context.Context, authed *oauth.Auth, excludeTypes []string, limit int, maxID string, sinceID string) (*apimodel.PageableResponse, gtserror.WithCode) { - notifs, err := p.state.DB.GetAccountNotifications(ctx, authed.Account.ID, excludeTypes, limit, maxID, sinceID) +func (p *Processor) NotificationsGet(ctx context.Context, authed *oauth.Auth, maxID string, sinceID string, minID string, limit int, excludeTypes []string) (*apimodel.PageableResponse, gtserror.WithCode) { + notifs, err := p.state.DB.GetAccountNotifications(ctx, authed.Account.ID, maxID, sinceID, minID, limit, excludeTypes) if err != nil { + if errors.Is(err, db.ErrNoEntries) { + // No notifs (left). + return util.EmptyPageableResponse(), nil + } + // An actual error has occurred. + err = fmt.Errorf("NotificationsGet: db error getting notifications: %w", err) return nil, gtserror.NewErrorInternalError(err) } count := len(notifs) - if count == 0 { return util.EmptyPageableResponse(), nil } - items := make([]interface{}, 0, count) - nextMaxIDValue := "" - prevMinIDValue := "" - for i, n := range notifs { - item, err := p.tc.NotificationToAPINotification(ctx, n) - if err != nil { - log.Debugf(ctx, "got an error converting a notification to api, will skip it: %s", err) - continue - } + var ( + items = make([]interface{}, 0, count) + nextMaxIDValue string + prevMinIDValue string + ) + for i, n := range notifs { + // Set next + prev values before filtering and API + // converting, so caller can still page properly. if i == count-1 { - nextMaxIDValue = item.GetID() + nextMaxIDValue = n.ID } if i == 0 { - prevMinIDValue = item.GetID() + prevMinIDValue = n.ID + } + + // Ensure this notification should be shown to requester. + if n.OriginAccount != nil { + // Account is set, ensure it's visible to notif target. + visible, err := p.filter.AccountVisible(ctx, authed.Account, n.OriginAccount) + if err != nil { + log.Debugf(ctx, "skipping notification %s because of an error checking notification visibility: %s", n.ID, err) + continue + } + if !visible { + continue + } + } + + if n.Status != nil { + // Status is set, ensure it's visible to notif target. + visible, err := p.filter.StatusVisible(ctx, authed.Account, n.Status) + if err != nil { + log.Debugf(ctx, "skipping notification %s because of an error checking notification visibility: %s", n.ID, err) + continue + } + if !visible { + continue + } + } + + item, err := p.tc.NotificationToAPINotification(ctx, n) + if err != nil { + log.Debugf(ctx, "skipping notification %s because it couldn't be converted to its api representation: %s", n.ID, err) + continue } items = append(items, item) @@ -68,7 +103,6 @@ func (p *Processor) NotificationsGet(ctx context.Context, authed *oauth.Auth, ex Items: items, Path: "api/v1/notifications", NextMaxIDValue: nextMaxIDValue, - PrevMinIDKey: "since_id", PrevMinIDValue: prevMinIDValue, Limit: limit, }) diff --git a/internal/processing/notification_test.go b/internal/processing/notification_test.go index 054f5215..bf69fc9b 100644 --- a/internal/processing/notification_test.go +++ b/internal/processing/notification_test.go @@ -32,7 +32,7 @@ type NotificationTestSuite struct { // get a notification where someone has liked our status func (suite *NotificationTestSuite) TestGetNotifications() { receivingAccount := suite.testAccounts["local_account_1"] - notifsResponse, err := suite.processor.NotificationsGet(context.Background(), suite.testAutheds["local_account_1"], []string{}, 10, "", "") + notifsResponse, err := suite.processor.NotificationsGet(context.Background(), suite.testAutheds["local_account_1"], "", "", "", 10, nil) suite.NoError(err) suite.Len(notifsResponse.Items, 1) notif, ok := notifsResponse.Items[0].(*apimodel.Notification) @@ -44,7 +44,7 @@ func (suite *NotificationTestSuite) TestGetNotifications() { suite.NotNil(notif.Status) suite.NotNil(notif.Status.Account) suite.Equal(receivingAccount.ID, notif.Status.Account.ID) - suite.Equal(`; rel="next", ; rel="prev"`, notifsResponse.LinkHeader) + suite.Equal(`; rel="next", ; rel="prev"`, notifsResponse.LinkHeader) } func TestNotificationTestSuite(t *testing.T) {