Skip to content

feat: allow pagination of message by date #890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eternal-flame-AD wants to merge 3 commits into
base: master
Choose a base branch
from
+231 −35
Open

feat: allow pagination of message by date #890

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5715adc
feat: allow pagination of message by date
eternal-flame-AD Dec 9, 2025
576f4f4
more gofumpt
eternal-flame-AD Dec 9, 2025
ac8c478
clear a stray print
eternal-flame-AD Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions api/message.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package api
import (
"encoding/json"
"errors"
"log"
"strconv"
"strings"
"time"
Expand All @@ -16,9 +17,9 @@ import (

// The MessageDatabase interface for encapsulating database access.
type MessageDatabase interface {
GetMessagesByApplicationSince(appID uint, limit int, since uint) ([]*model.Message, error)
GetMessagesByApplicationPaginated(appID uint, limit int, since, after uint64, by string) ([]*model.Message, error)
GetApplicationByID(id uint) (*model.Application, error)
GetMessagesByUserSince(userID uint, limit int, since uint) ([]*model.Message, error)
GetMessagesByUserPaginated(userID uint, limit int, since, after uint64, by string) ([]*model.Message, error)
DeleteMessageByID(id uint) error
GetMessageByID(id uint) (*model.Message, error)
DeleteMessagesByUser(userID uint) error
Expand All @@ -41,8 +42,10 @@ type MessageAPI struct {
}

type pagingParams struct {
Limit int `form:"limit" binding:"min=1,max=200"`
Since uint `form:"since" binding:"min=0"`
Limit int `form:"limit" binding:"min=1,max=200"`
Since uint64 `form:"since" binding:"min=0"`
Copy link
Member

@jmattheis jmattheis Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other date-time related fields use RFC3339 format, this would break the consistency with it being a unix timestamp.

I think I'd prefer either something like an ISO interval e.g. &interval=2025-10-10T23:00Z/2025-10-15T23:00Z and then allowing to use the after/before id offset, to page in this daterange to prevent the bugs described in #889 (comment)

or have separate sinceTime and afterTime fields which accept an RFC3339 datetime.

What do you think? I don't dislike this current solution, so I'm open for any arguments (:.

Copy link

@cxtal cxtal Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or have separate sinceTime and afterTime fields which accept an RFC3339 datetime.

+1 I prefer ISO dates to unix timestamps

After uint64 `form:"after" binding:"min=0"`
By string `form:"by" binding:"oneof=id date"`
}

// GetMessages returns all messages from a user.
Expand All @@ -69,6 +72,27 @@ type pagingParams struct {
// required: false
// type: integer
// format: int64
// - name: by
// in: query
// description: the field to order by
// required: false
// type: string
// enum: [id, date]
// default: id
// - name: after
// in: query
// description: return all messages with an cursor value greater than or equal to this value
// minimum: 0
// required: false
// type: integer
// format: int64
// - name: since
// in: query
// description: return all messages with an cursor value less than this value
// minimum: 0
// required: false
// type: integer
// format: int64
// responses:
// 200:
// description: Ok
Expand All @@ -90,7 +114,7 @@ func (a *MessageAPI) GetMessages(ctx *gin.Context) {
userID := auth.GetUserID(ctx)
withPaging(ctx, func(params *pagingParams) {
// the +1 is used to check if there are more messages and will be removed on buildWithPaging
messages, err := a.DB.GetMessagesByUserSince(userID, params.Limit+1, params.Since)
messages, err := a.DB.GetMessagesByUserPaginated(userID, params.Limit+1, params.Since, params.After, params.By)
if success := successOrAbort(ctx, 500, err); !success {
return
}
Expand Down Expand Up @@ -120,8 +144,9 @@ func buildWithPaging(ctx *gin.Context, paging *pagingParams, messages []*model.M
}

func withPaging(ctx *gin.Context, f func(pagingParams *pagingParams)) {
params := &pagingParams{Limit: 100}
params := &pagingParams{Limit: 100, By: "id"}
if err := ctx.MustBindWith(params, binding.Query); err == nil {
log.Printf("Paging params: %+v", params)
Copy link
Member

@jmattheis jmattheis Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not needed right?

f(params)
}
}
Expand Down Expand Up @@ -151,11 +176,25 @@ func withPaging(ctx *gin.Context, f func(pagingParams *pagingParams)) {
// type: integer
// - name: since
// in: query
// description: return all messages with an ID less than this value
// description: return all messages with an cursor value less than this value
// minimum: 0
// required: false
// type: integer
// format: int64
// - name: after
// in: query
// description: return all messages with an cursor value greater than or equal to this value
// minimum: 0
// required: false
// type: integer
// format: int64
// - name: by
// in: query
// description: the field to order by
// required: false
// type: string
// enum: [id, date]
// default: id
// responses:
// 200:
// description: Ok
Expand Down Expand Up @@ -186,7 +225,7 @@ func (a *MessageAPI) GetMessagesWithApplication(ctx *gin.Context) {
}
if app != nil && app.UserID == auth.GetUserID(ctx) {
// the +1 is used to check if there are more messages and will be removed on buildWithPaging
messages, err := a.DB.GetMessagesByApplicationSince(id, params.Limit+1, params.Since)
messages, err := a.DB.GetMessagesByApplicationPaginated(id, params.Limit+1, params.Since, params.After, params.By)
if success := successOrAbort(ctx, 500, err); !success {
return
}
Expand Down
57 changes: 49 additions & 8 deletions database/message.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package database

import (
"time"

"github.com/gotify/server/v2/model"
"gorm.io/gorm"
"gorm.io/gorm/clause"
)

// GetMessageByID returns the messages for the given id or nil.
Expand Down Expand Up @@ -34,14 +37,33 @@ func (d *GormDatabase) GetMessagesByUser(userID uint) ([]*model.Message, error)
return messages, err
}

// GetMessagesByUserSince returns limited messages from a user.
// GetMessagesByUserPaginated returns limited messages from a user.
// If since is 0 it will be ignored.
func (d *GormDatabase) GetMessagesByUserSince(userID uint, limit int, since uint) ([]*model.Message, error) {
func (d *GormDatabase) GetMessagesByUserPaginated(userID uint, limit int, since, after uint64, by string) ([]*model.Message, error) {
var messages []*model.Message
db := d.DB.Joins("JOIN applications ON applications.user_id = ?", userID).
Where("messages.application_id = applications.id").Order("messages.id desc").Limit(limit)
Where("messages.application_id = applications.id").Order(clause.OrderBy{Columns: []clause.OrderByColumn{
{
Column: clause.Column{
Table: "messages",
Name: by,
},
Desc: since != 0 || after == 0,
Comment on lines +47 to +51
Copy link
Member

@jmattheis jmattheis Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean, the messages are returned in another order based on if since or after is set. I feel like this is unexpected if both parameters are set. It should be another parameter to make it more explicit.

Either way, it should always use sorting by id, as this will be more precise than ordering by date, as there could be messages with the same date.

Copy link
Member Author

@eternal-flame-AD eternal-flame-AD Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is not ideal, the reason I used this scheme is mostly:

The computational complexity of finding inequalities on two keys is O(N) time O(N) space (or you can build 4 indices, regardless the complexity is squared and as if no indices apply), this is made worse by the fact that the database will not be permitted to assume id and date are monotonic to each other.

So I am trying to design the API to prevent making it possible to make queries like id > 100 and timestamp < 200, to which the only viable query plan will be traverse idx_id upwards and collect 50 messages satisfying timestamp < 200 which may be expensive depending on the selectivity of the timestamp < 200 to this query.

Copy link
Member Author

@eternal-flame-AD eternal-flame-AD Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am open to using distinct keys and just enforce they are mutually exclusive to each other though, but the tradeoff is vocabulary bloat instead of this "switch pagination key" semantics.

Copy link
Member

@jmattheis jmattheis Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm currently a little unresponsive, and probably will continue to be for the rest of the month.

Ahh yeah, that's tough.

From a general point of view, do we need the new "after" filter. If we want to query a specific date range e.g. 2025-01-01 to 2025-01-31, I don't think it makes much difference if you start fetching the messages from the newest (2025-01-31) or oldest (2025-01-01 messages.

The user would be responsible to fetch as many pages until all messages in the range were fetched. The only thing to consider would be big date ranges, and wanting to lazy load and starting with the oldest message, but I'm not sure if we need to support this.

I'm probably also okay with just not having an index for the features not used in the official clients. I don't think the difference it performance should be too visible, because the query is still pretty simple, and we're probably not talking about billions of messages in gotify.

Another idea would be for an intermediate api to query the message id boundaries. Something like

Get /message/idrange?interval=2025-10-10T23:00Z/2025-10-15T23:00Z
{"begin": 42, "end: 1337}

We'd only query messages with the actual ids, so it should make the pagination simpler. Maybe we could do this also internally in our pagination api, if a date is provided we translate it to a message. And if both is provided we do a min(translated date id, provided since message id) or similar.

What do you think about any of that?

Copy link
Member Author

@eternal-flame-AD eternal-flame-AD Dec 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah honestly I don't really care about the perf degredation for my own use case - I am just worried about it becoming a hard to resolve issue once somebody opens a ticket saying they have DB timeout for what looks like a simple query why (had this problem in another project I am engaged in)

This final proposal looks best to me because it inherits existing schemes , but we also need to implement the same thing again in the /application path, probably needs some refactoring, but the API presentations looks okay to me.

@cxtal Do you have any input on this? It should work for your case and respect your preference for long timestamps.

Copy link

@cxtal cxtal Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi again @eternal-flame-AD while you were at it, I added some stuff to #889 but I think the point is that my request was more along the lines of "filters" than pagination so there shouldn't be any conflicts with the current way of paginating.

Even if the algorithm is a dumb linear-search when returning data, having the ability to filter the data sent should prevent cases where the application would stall because it is waiting for too much data or that it receives too much data.

I guess it would be possible to optimize it like having buckets for date-ranges and so on but just the ability to only return matching messages is already a huge performance gain.

RFC-dates are more elegant to work with and also more stable given that Unix time is sometimes measured in milliseconds and other times measured in seconds otherwise it does not matter much :-)

Thank you for adding :-) Looking forward to rip a recursive function apart.

},
}}).Limit(limit)
if since != 0 {
db = db.Where("messages.id < ?", since)
sinceVal := any(since)
if by == "date" {
sinceVal = time.Unix(int64(since), 0)
}
db = db.Where(clause.Lt{Column: clause.Column{Table: "messages", Name: by}, Value: sinceVal})
}
if after != 0 {
afterVal := any(after)
if by == "date" {
afterVal = time.Unix(int64(after), 0)
}
db = db.Where(clause.Gte{Column: clause.Column{Table: "messages", Name: by}, Value: afterVal})
}
err := db.Find(&messages).Error
if err == gorm.ErrRecordNotFound {
Expand All @@ -60,13 +82,32 @@ func (d *GormDatabase) GetMessagesByApplication(tokenID uint) ([]*model.Message,
return messages, err
}

// GetMessagesByApplicationSince returns limited messages from an application.
// GetMessagesByApplicationPaginated returns limited messages from an application.
// If since is 0 it will be ignored.
func (d *GormDatabase) GetMessagesByApplicationSince(appID uint, limit int, since uint) ([]*model.Message, error) {
func (d *GormDatabase) GetMessagesByApplicationPaginated(appID uint, limit int, since, after uint64, by string) ([]*model.Message, error) {
var messages []*model.Message
db := d.DB.Where("application_id = ?", appID).Order("messages.id desc").Limit(limit)
db := d.DB.Where("application_id = ?", appID).Order(clause.OrderBy{Columns: []clause.OrderByColumn{
{
Copy link
Member

@jmattheis jmattheis Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this can be unified between the two *Paginated apis, the only difference is the additional filtering for application id or?

Copy link
Member Author

@eternal-flame-AD eternal-flame-AD Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes .. I don't think I wrote this part (?) so I am not sure the exact rationale but I assume it's to accommodate it being mounted on a different API path. It can be just one interface at the data model layer.

Column: clause.Column{
Table: "messages",
Name: by,
},
Desc: since != 0 || after == 0,
},
}}).Limit(limit)
if since != 0 {
db = db.Where("messages.id < ?", since)
sinceVal := any(since)
if by == "date" {
sinceVal = time.Unix(int64(since), 0)
}
db = db.Where(clause.Lt{Column: clause.Column{Table: "messages", Name: by}, Value: sinceVal})
}
if after != 0 {
afterVal := any(after)
if by == "date" {
afterVal = time.Unix(int64(after), 0)
}
db = db.Where(clause.Gte{Column: clause.Column{Table: "messages", Name: by}, Value: afterVal})
}
err := db.Find(&messages).Error
if err == gorm.ErrRecordNotFound {
Expand Down
102 changes: 87 additions & 15 deletions database/message_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package database

import (
"slices"
"testing"
"time"

Expand Down Expand Up @@ -156,71 +157,142 @@ func (s *DatabaseSuite) TestGetMessagesSince() {
require.NoError(s.T(), s.db.CreateApplication(app))
require.NoError(s.T(), s.db.CreateApplication(app2))

curDate := time.Now()
curDate := time.Unix(time.Now().Unix(), 0)
for i := 1; i <= 500; i++ {
s.db.CreateMessage(&model.Message{ApplicationID: app.ID, Message: "abc", Date: curDate.Add(time.Duration(i) * time.Second)})
s.db.CreateMessage(&model.Message{ApplicationID: app2.ID, Message: "abc", Date: curDate.Add(time.Duration(i) * time.Second)})
s.db.CreateMessage(&model.Message{ApplicationID: app.ID, Message: "abc", Date: curDate.Add(time.Duration(i*2) * time.Second)})
s.db.CreateMessage(&model.Message{ApplicationID: app2.ID, Message: "abc", Date: curDate.Add(time.Duration(i*2+1) * time.Second)})
}

actual, err := s.db.GetMessagesByUserSince(user.ID, 50, 0)
actual, err := s.db.GetMessagesByUserPaginated(user.ID, 50, 0, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 1000, 951, 1)

actual, err = s.db.GetMessagesByUserSince(user.ID, 50, 951)
actual, err = s.db.GetMessagesByUserPaginated(user.ID, 50, 0, 0, "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 1000, 951, 1)

actual, err = s.db.GetMessagesByUserPaginated(user.ID, 50, 951, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 950, 901, 1)

actual, err = s.db.GetMessagesByUserPaginated(user.ID, 50, 0, 901, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
slices.Reverse(actual)
hasIDInclusiveBetween(s.T(), actual, 950, 901, 1)

actual, err = s.db.GetMessagesByUserSince(user.ID, 100, 951)
actual, err = s.db.GetMessagesByUserPaginated(user.ID, 100, 951, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 100)
hasIDInclusiveBetween(s.T(), actual, 950, 851, 1)

actual, err = s.db.GetMessagesByUserSince(user.ID, 100, 51)
actual, err = s.db.GetMessagesByUserPaginated(user.ID, 100, 51, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 50, 1, 1)

actual, err = s.db.GetMessagesByApplicationSince(app.ID, 50, 0)
actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 50, 0, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 999, 901, 2)

actual, err = s.db.GetMessagesByApplicationSince(app.ID, 50, 901)
actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 50, 0, 0, "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 999, 901, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 50, uint64(curDate.Unix()+50), 0, "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 24)
hasIDInclusiveBetween(s.T(), actual, 47, 1, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 50, uint64(curDate.Unix()+50), uint64(curDate.Unix()+10), "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 20)
hasIDInclusiveBetween(s.T(), actual, 47, 47-20*2+2, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 50, 0, uint64(curDate.Unix()+950), "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 26)
slices.Reverse(actual)
hasIDInclusiveBetween(s.T(), actual, 999, 949, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 50, 901, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 899, 801, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 50, 0, 801, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
slices.Reverse(actual)
hasIDInclusiveBetween(s.T(), actual, 899, 801, 2)

actual, err = s.db.GetMessagesByApplicationSince(app.ID, 100, 666)
actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 100, 666, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 100)
hasIDInclusiveBetween(s.T(), actual, 665, 467, 2)

actual, err = s.db.GetMessagesByApplicationSince(app.ID, 100, 101)
actual, err = s.db.GetMessagesByApplicationPaginated(app.ID, 100, 101, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 99, 1, 2)

actual, err = s.db.GetMessagesByApplicationSince(app2.ID, 50, 0)
actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, 0, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 1000, 902, 2)

actual, err = s.db.GetMessagesByApplicationSince(app2.ID, 50, 902)
actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, 0, 0, "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 1000, 902, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, uint64(curDate.Unix()+50), 0, "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 24)
hasIDInclusiveBetween(s.T(), actual, 48, 2, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, uint64(curDate.Unix()+50), uint64(curDate.Unix()+10), "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 20)
hasIDInclusiveBetween(s.T(), actual, 48, 48-20*2+2, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, 0, uint64(curDate.Unix()+950), "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 26)
slices.Reverse(actual)
hasIDInclusiveBetween(s.T(), actual, 1000, 950, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, 902, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 900, 802, 2)

actual, err = s.db.GetMessagesByApplicationSince(app2.ID, 100, 667)
actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, 0, 802, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
slices.Reverse(actual)
hasIDInclusiveBetween(s.T(), actual, 900, 802, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 100, 667, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 100)
hasIDInclusiveBetween(s.T(), actual, 666, 468, 2)

actual, err = s.db.GetMessagesByApplicationSince(app2.ID, 100, 102)
actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 100, 102, 0, "id")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 50)
hasIDInclusiveBetween(s.T(), actual, 100, 2, 2)

actual, err = s.db.GetMessagesByApplicationPaginated(app2.ID, 50, 0, uint64(curDate.Unix()+950), "date")
require.NoError(s.T(), err)
assert.Len(s.T(), actual, 26)
slices.Reverse(actual)
hasIDInclusiveBetween(s.T(), actual, 1000, 950, 2)
}

func hasIDInclusiveBetween(t *testing.T, msgs []*model.Message, from, to, decrement int) {
Expand Down
Loading
Loading