Skip to content

Commit

Permalink
apply suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
eternal-flame-AD committed Jul 28, 2023
1 parent 2c01982 commit af298d3
Show file tree
Hide file tree
Showing 18 changed files with 165 additions and 55 deletions.
5 changes: 3 additions & 2 deletions api/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ func (s *ApplicationSuite) Test_ensureApplicationHasCorrectJsonRepresentation()
Description: "mydesc",
Image: "asd",
Internal: true,
LastUsed: nil,
}
test.JSONEquals(s.T(), actual, `{"id":1,"token":"Aasdasfgeeg","name":"myapp","description":"mydesc", "image": "asd", "internal":true, "defaultPriority":0}`)
test.JSONEquals(s.T(), actual, `{"id":1,"token":"Aasdasfgeeg","name":"myapp","description":"mydesc", "image": "asd", "internal":true, "defaultPriority":0, "lastUsed":null}`)
}

func (s *ApplicationSuite) Test_CreateApplication_expectBadRequestOnEmptyName() {
Expand Down Expand Up @@ -302,7 +303,7 @@ func (s *ApplicationSuite) Test_UploadAppImage_OtherErrors_expectServerError() {
s.a.UploadApplicationImage(s.ctx)

assert.Equal(s.T(), 500, s.recorder.Code)
assert.Equal(s.T(), s.ctx.Errors[0].Err, errors.New("multipart: NextPart: EOF"))
assert.Contains(s.T(), s.ctx.Errors[0].Err.Error(), "multipart: NextPart: EOF")
}

func (s *ApplicationSuite) Test_UploadAppImage_WithImageFile_expectSuccess() {
Expand Down
2 changes: 1 addition & 1 deletion api/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (s *ClientSuite) AfterTest(suiteName, testName string) {

func (s *ClientSuite) Test_ensureClientHasCorrectJsonRepresentation() {
actual := &model.Client{ID: 1, UserID: 2, Token: "Casdasfgeeg", Name: "myclient"}
test.JSONEquals(s.T(), actual, `{"id":1,"token":"Casdasfgeeg","name":"myclient","lastSeen":null}`)
test.JSONEquals(s.T(), actual, `{"id":1,"token":"Casdasfgeeg","name":"myclient","lastUsed":null}`)
}

func (s *ClientSuite) Test_CreateClient_mapAllParameters() {
Expand Down
48 changes: 35 additions & 13 deletions api/stream/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,39 @@ import (

// The API provides a handler for a WebSocket stream API.
type API struct {
clients map[uint][]*client
lock sync.RWMutex
pingPeriod time.Duration
pongTimeout time.Duration
onClientPong func(userID uint, token string)
upgrader *websocket.Upgrader
clients map[uint][]*client
lock sync.RWMutex
pingPeriod time.Duration
pongTimeout time.Duration
upgrader *websocket.Upgrader
}

// New creates a new instance of API.
// pingPeriod: is the interval, in which is server sends the a ping to the client.
// pongTimeout: is the duration after the connection will be terminated, when the client does not respond with the
// pong command.
func New(pingPeriod, pongTimeout time.Duration, onClientPong func(userID uint, token string), allowedWebSocketOrigins []string) *API {
func New(pingPeriod, pongTimeout time.Duration, allowedWebSocketOrigins []string) *API {
return &API{
clients: make(map[uint][]*client),
pingPeriod: pingPeriod,
pongTimeout: pingPeriod + pongTimeout,
onClientPong: onClientPong,
upgrader: newUpgrader(allowedWebSocketOrigins),
clients: make(map[uint][]*client),
pingPeriod: pingPeriod,
pongTimeout: pingPeriod + pongTimeout,
upgrader: newUpgrader(allowedWebSocketOrigins),
}
}

// CollectConnectedClientTokens returns all tokens of the connected clients.
func (a *API) CollectConnectedClientTokens() []string {
a.lock.RLock()
defer a.lock.RUnlock()
var clients []string
for _, cs := range a.clients {
for _, c := range cs {
clients = append(clients, c.token)
}
}
return uniq(clients)
}

// NotifyDeletedUser closes existing connections for the given user.
func (a *API) NotifyDeletedUser(userID uint) error {
a.lock.Lock()
Expand Down Expand Up @@ -95,7 +106,6 @@ func (a *API) remove(remove *client) {
func (a *API) register(client *client) {
a.lock.Lock()
defer a.lock.Unlock()
a.onClientPong(client.userID, client.token)
a.clients[client.userID] = append(a.clients[client.userID], client)
}

Expand Down Expand Up @@ -158,6 +168,18 @@ func (a *API) Close() {
}
}

func uniq[T comparable](s []T) []T {
m := make(map[T]struct{})
for _, v := range s {
m[v] = struct{}{}
}
var r []T
for k := range m {
r = append(r, k)
}
return r
}

func isAllowedOrigin(r *http.Request, allowedOrigins []*regexp.Regexp) bool {
origin := r.Header.Get("origin")
if origin == "" {
Expand Down
36 changes: 35 additions & 1 deletion api/stream/stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"sort"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -322,6 +323,39 @@ func TestDeleteUser(t *testing.T) {
api.Close()
}

func TestCollectConnectedClientTokens(t *testing.T) {
mode.Set(mode.TestDev)

defer leaktest.Check(t)()
userIDs := []uint{1, 1, 1, 2, 2}
tokens := []string{"1-1", "1-2", "1-2", "2-1", "2-2"}
i := 0
server, api := bootTestServer(func(context *gin.Context) {
auth.RegisterAuthentication(context, nil, userIDs[i], tokens[i])
i++
})
defer server.Close()

wsURL := wsURL(server.URL)
userOneConnOne := testClient(t, wsURL)
defer userOneConnOne.conn.Close()
userOneConnTwo := testClient(t, wsURL)
defer userOneConnTwo.conn.Close()
userOneConnThree := testClient(t, wsURL)
defer userOneConnThree.conn.Close()
ret := api.CollectConnectedClientTokens()
sort.Strings(ret)
assert.Equal(t, []string{"1-1", "1-2"}, ret)

userTwoConnOne := testClient(t, wsURL)
defer userTwoConnOne.conn.Close()
userTwoConnTwo := testClient(t, wsURL)
defer userTwoConnTwo.conn.Close()
ret = api.CollectConnectedClientTokens()
sort.Strings(ret)
assert.Equal(t, []string{"1-1", "1-2", "2-1", "2-2"}, ret)
}

func TestMultipleClients(t *testing.T) {
mode.Set(mode.TestDev)

Expand Down Expand Up @@ -544,7 +578,7 @@ func bootTestServer(handlerFunc gin.HandlerFunc) (*httptest.Server, *API) {
r := gin.New()
r.Use(handlerFunc)
// ping every 500 ms, and the client has 500 ms to respond
api := New(500*time.Millisecond, 500*time.Millisecond, func(userID uint, token string) {}, []string{})
api := New(500*time.Millisecond, 500*time.Millisecond, []string{})

r.GET("/", api.Handle)
server := httptest.NewServer(r)
Expand Down
13 changes: 10 additions & 3 deletions auth/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package auth
import (
"errors"
"strings"
"time"

"github.com/gin-gonic/gin"
"github.com/gotify/server/v2/auth/password"
Expand All @@ -20,6 +21,8 @@ type Database interface {
GetPluginConfByToken(token string) (*model.PluginConf, error)
GetUserByName(name string) (*model.User, error)
GetUserByID(id uint) (*model.User, error)
UpdateClientTokensLastUsed(tokens []string, t *time.Time) error
UpdateApplicationTokenLastUsed(token string, t *time.Time) error
}

// Auth is the provider for authentication middleware.
Expand Down Expand Up @@ -56,10 +59,12 @@ func (a *Auth) RequireClient() gin.HandlerFunc {
if user != nil {
return true, true, user.ID, nil
}
if token, err := a.DB.GetClientByToken(tokenID); err != nil {
if client, err := a.DB.GetClientByToken(tokenID); err != nil {
return false, false, 0, err
} else if token != nil {
return true, true, token.UserID, nil
} else if client != nil {
now := time.Now()
a.DB.UpdateClientTokensLastUsed([]string{tokenID}, &now)
return true, true, client.UserID, nil
}
return false, false, 0, nil
})
Expand All @@ -74,6 +79,8 @@ func (a *Auth) RequireApplicationToken() gin.HandlerFunc {
if token, err := a.DB.GetApplicationByToken(tokenID); err != nil {
return false, false, 0, err
} else if token != nil {
now := time.Now()
a.DB.UpdateApplicationTokenLastUsed(tokenID, &now)
return true, true, token.UserID, nil
}
return false, false, 0, nil
Expand Down
7 changes: 7 additions & 0 deletions database/application.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package database

import (
"time"

"github.com/gotify/server/v2/model"
"github.com/jinzhu/gorm"
)
Expand Down Expand Up @@ -56,3 +58,8 @@ func (d *GormDatabase) GetApplicationsByUser(userID uint) ([]*model.Application,
func (d *GormDatabase) UpdateApplication(app *model.Application) error {
return d.DB.Save(app).Error
}

// UpdateApplicationTokenLastUsed updates the last used time of the application token.
func (d *GormDatabase) UpdateApplicationTokenLastUsed(token string, t *time.Time) error {
return d.DB.Model(&model.Application{}).Where("token = ?", token).Update("last_used", t).Error
}
10 changes: 10 additions & 0 deletions database/application_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package database

import (
"time"

"github.com/gotify/server/v2/model"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -40,6 +42,14 @@ func (s *DatabaseSuite) TestApplication() {
assert.Equal(s.T(), app, newApp)
}

lastUsed := time.Now().Add(-time.Hour)
s.db.UpdateApplicationTokenLastUsed(app.Token, &lastUsed)
newApp, err = s.db.GetApplicationByID(app.ID)
if assert.NoError(s.T(), err) {
assert.Equal(s.T(), lastUsed.Unix(), newApp.LastUsed.Unix())
}
app.LastUsed = &lastUsed

newApp.Image = "asdasd"
assert.NoError(s.T(), s.db.UpdateApplication(newApp))

Expand Down
7 changes: 7 additions & 0 deletions database/client.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package database

import (
"time"

"github.com/gotify/server/v2/model"
"github.com/jinzhu/gorm"
)
Expand Down Expand Up @@ -55,3 +57,8 @@ func (d *GormDatabase) DeleteClientByID(id uint) error {
func (d *GormDatabase) UpdateClient(client *model.Client) error {
return d.DB.Save(client).Error
}

// UpdateClientTokensLastUsed updates the last used timestamp of clients

Check failure on line 61 in database/client.go

View workflow job for this annotation

GitHub Actions / gotify

Comment should end in a period (godot)
func (d *GormDatabase) UpdateClientTokensLastUsed(tokens []string, t *time.Time) error {
return d.DB.Model(&model.Client{}).Where("token IN (?)", tokens).Update("last_used", t).Error
}
9 changes: 9 additions & 0 deletions database/client_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package database

import (
"time"

"github.com/gotify/server/v2/model"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -44,6 +46,13 @@ func (s *DatabaseSuite) TestClient() {
assert.Equal(s.T(), updateClient, updatedClient)
}

lastUsed := time.Now().Add(-time.Hour)
s.db.UpdateClientTokensLastUsed([]string{client.Token}, &lastUsed)
newClient, err = s.db.GetClientByID(client.ID)
if assert.NoError(s.T(), err) {
assert.Equal(s.T(), lastUsed.Unix(), newClient.LastUsed.Unix())
}

s.db.DeleteClientByID(client.ID)

if clients, err := s.db.GetClientsByUser(user.ID); assert.NoError(s.T(), err) {
Expand Down
14 changes: 11 additions & 3 deletions docs/spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -2098,6 +2098,14 @@
"readOnly": true,
"example": false
},
"lastUsed": {
"description": "The last time the application token was used.",
"type": "string",
"format": "date-time",
"x-go-name": "LastUsed",
"readOnly": true,
"example": "2019-01-01T00:00:00Z"
},
"name": {
"description": "The application name. This is how the application should be displayed to the user.",
"type": "string",
Expand Down Expand Up @@ -2162,11 +2170,11 @@
"readOnly": true,
"example": 5
},
"lastSeen": {
"description": "The last time the client was seen.",
"lastUsed": {
"description": "The last time the client token was used.",
"type": "string",
"format": "date-time",
"x-go-name": "LastSeen",
"x-go-name": "LastUsed",
"readOnly": true,
"example": "2019-01-01T00:00:00Z"
},
Expand Down
7 changes: 7 additions & 0 deletions model/application.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package model

import "time"

// Application Model
//
// The Application holds information about an app which can send notifications.
Expand Down Expand Up @@ -47,4 +49,9 @@ type Application struct {
// required: false
// example: 4
DefaultPriority int `form:"defaultPriority" query:"defaultPriority" json:"defaultPriority"`
// The last time the application token was used.
//
// read only: true
// example: 2019-01-01T00:00:00Z
LastUsed *time.Time `json:"lastUsed"`
}
4 changes: 2 additions & 2 deletions model/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ type Client struct {
// required: true
// example: Android Phone
Name string `gorm:"type:text" form:"name" query:"name" json:"name" binding:"required"`
// The last time the client was seen.
// The last time the client token was used.
//
// read only: true
// example: 2019-01-01T00:00:00Z
LastSeen *time.Time `json:"lastSeen"`
LastUsed *time.Time `json:"lastUsed"`
}
25 changes: 8 additions & 17 deletions router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package router

import (
"fmt"
"log"
"net/http"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -31,23 +30,15 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co
g.NoRoute(gerror.NotFound())

streamHandler := stream.New(
time.Duration(conf.Server.Stream.PingPeriodSeconds)*time.Second, 15*time.Second,
func(userID uint, token string) {
client, err := db.GetClientByToken(token)
if err != nil {
log.Printf("could not get client by token: %v", err)
return
}
if client.UserID != userID {
log.Printf("client %d does not belong to user %d", client.ID, userID)
return
}
time.Duration(conf.Server.Stream.PingPeriodSeconds)*time.Second, 15*time.Second, conf.Server.Stream.AllowedOrigins)
go func() {
ticker := time.NewTicker(5 * time.Minute)
for range ticker.C {
connectedTokens := streamHandler.CollectConnectedClientTokens()
now := time.Now()
client.LastSeen = &now
if err := db.UpdateClient(client); err != nil {
log.Printf("could not update client: %v", err)
}
}, conf.Server.Stream.AllowedOrigins)
db.UpdateClientTokensLastUsed(connectedTokens, &now)
}
}()
authentication := auth.Auth{DB: db}
messageHandler := api.MessageAPI{Notifier: streamHandler, DB: db}
healthHandler := api.HealthAPI{DB: db}
Expand Down

0 comments on commit af298d3

Please sign in to comment.