diff --git a/database/application.go b/database/application.go index ccad458b8..73757de3c 100644 --- a/database/application.go +++ b/database/application.go @@ -72,7 +72,7 @@ func (d *GormDatabase) GetApplicationsByUser(userID uint) ([]*model.Application, // UpdateApplication updates an application. func (d *GormDatabase) UpdateApplication(app *model.Application) error { - return d.DB.Save(app).Error + return d.updateExisting(app) } // UpdateApplicationTokenLastUsed updates the last used time of the application token. diff --git a/database/application_test.go b/database/application_test.go index 2c38e7561..50881aa1f 100644 --- a/database/application_test.go +++ b/database/application_test.go @@ -5,6 +5,7 @@ import ( "github.com/gotify/server/v3/model" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func (s *DatabaseSuite) TestApplication() { @@ -99,3 +100,16 @@ func (s *DatabaseSuite) TestDeleteAppDeletesMessages() { assert.NotEmpty(s.T(), msgs) } } + +func (s *DatabaseSuite) TestUpdateDeletedApplicationDoesNotRecreateIt() { + app := &model.Application{UserID: 1, Token: "ASTALE", Name: "backup"} + require.NoError(s.T(), s.db.CreateApplication(app)) + require.NoError(s.T(), s.db.DeleteApplicationByID(app.ID)) + + app.Name = "renamed" + assert.NoError(s.T(), s.db.UpdateApplication(app)) + + if a, err := s.db.GetApplicationByID(app.ID); assert.NoError(s.T(), err) { + assert.Nil(s.T(), a) + } +} diff --git a/database/client.go b/database/client.go index aa87b37c2..25e11c13c 100644 --- a/database/client.go +++ b/database/client.go @@ -60,7 +60,7 @@ func (d *GormDatabase) DeleteClientByID(id uint) error { // UpdateClient updates a client. func (d *GormDatabase) UpdateClient(client *model.Client) error { client.PopulateExpiresAt() - return d.DB.Save(client).Error + return d.updateExisting(client) } // UpdateClientTokensLastUsed updates the last used timestamp of clients and @@ -74,7 +74,7 @@ func (d *GormDatabase) UpdateClientTokensLastUsedAndExpiresAt(tokens []string, t for _, c := range clients { c.LastUsed = t c.PopulateExpiresAt() - if err := tx.Save(c).Error; err != nil { + if err := tx.Model(c).Select("last_used", "expires_at").Updates(c).Error; err != nil { return err } } diff --git a/database/client_test.go b/database/client_test.go index 6d6edd6bd..45e7f8cd5 100644 --- a/database/client_test.go +++ b/database/client_test.go @@ -5,6 +5,7 @@ import ( "github.com/gotify/server/v3/model" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func (s *DatabaseSuite) TestClient() { @@ -194,3 +195,19 @@ func (s *DatabaseSuite) TestClientSelectHidesExpired() { assert.Equal(s.T(), valid.Token, cs[0].Token) } } + +func (s *DatabaseSuite) TestUpdateDeletedClientDoesNotRecreateIt() { + user := &model.User{Name: "stale", Pass: []byte{1}} + require.NoError(s.T(), s.db.CreateUser(user)) + + client := &model.Client{UserID: user.ID, Token: "CSTALE", Name: "android"} + require.NoError(s.T(), s.db.CreateClient(client)) + require.NoError(s.T(), s.db.DeleteClientByID(client.ID)) + + client.Name = "renamed" + assert.NoError(s.T(), s.db.UpdateClient(client)) + + if c, err := s.db.GetClientByID(client.ID); assert.NoError(s.T(), err) { + assert.Nil(s.T(), c) + } +} diff --git a/database/database.go b/database/database.go index 8da809064..b99e261b6 100644 --- a/database/database.go +++ b/database/database.go @@ -112,6 +112,10 @@ func New(dialect, connection, defaultUser, defaultPass string, strength int, cre return &GormDatabase{DB: db}, nil } +func (d *GormDatabase) updateExisting(value any) error { + return d.DB.Select("*").Updates(value).Error +} + func fillMissingCreatedAt(db *gorm.DB, now time.Time) error { models := []any{ new(model.User), diff --git a/database/plugin.go b/database/plugin.go index 32ea5951d..04f051ace 100644 --- a/database/plugin.go +++ b/database/plugin.go @@ -74,7 +74,7 @@ func (d *GormDatabase) GetPluginConfByID(id uint) (*model.PluginConf, error) { // UpdatePluginConf updates plugin configuration. func (d *GormDatabase) UpdatePluginConf(p *model.PluginConf) error { - return d.DB.Save(p).Error + return d.updateExisting(p) } // DeletePluginConfByID deletes a plugin configuration by its id. diff --git a/database/plugin_test.go b/database/plugin_test.go index 9948b4584..e449195a1 100644 --- a/database/plugin_test.go +++ b/database/plugin_test.go @@ -68,3 +68,16 @@ func (s *DatabaseSuite) TestPluginConf() { assert.Equal(s.T(), false, pluginConf.Enabled) assert.Equal(s.T(), testConf, string(pluginConf.Config)) } + +func (s *DatabaseSuite) TestUpdateDeletedPluginConfDoesNotRecreateIt() { + conf := &model.PluginConf{UserID: 1, ModulePath: "github.com/gotify/stale", Token: "PSTALE"} + require.NoError(s.T(), s.db.CreatePluginConf(conf)) + require.NoError(s.T(), s.db.DeletePluginConfByID(conf.ID)) + + conf.Enabled = true + assert.NoError(s.T(), s.db.UpdatePluginConf(conf)) + + if c, err := s.db.GetPluginConfByToken(conf.Token); assert.NoError(s.T(), err) { + assert.Nil(s.T(), c) + } +} diff --git a/database/user.go b/database/user.go index a2bdcde14..02fd5c6a0 100644 --- a/database/user.go +++ b/database/user.go @@ -83,7 +83,7 @@ func (d *GormDatabase) DeleteUserByID(id uint) error { // UpdateUser updates a user. func (d *GormDatabase) UpdateUser(user *model.User) error { - return d.DB.Save(user).Error + return d.updateExisting(user) } // CreateUser creates a user. diff --git a/database/user_test.go b/database/user_test.go index c10d97a89..9051a4764 100644 --- a/database/user_test.go +++ b/database/user_test.go @@ -180,3 +180,16 @@ func (s *DatabaseSuite) TestDeleteUserDeletesApplicationsAndClientsAndPluginConf require.NoError(s.T(), err) assert.NotNil(s.T(), msg) } + +func (s *DatabaseSuite) TestUpdateDeletedUserDoesNotRecreateIt() { + user := &model.User{Name: "stale", Pass: []byte{1}} + require.NoError(s.T(), s.db.CreateUser(user)) + require.NoError(s.T(), s.db.DeleteUserByID(user.ID)) + + user.Admin = true + assert.NoError(s.T(), s.db.UpdateUser(user)) + + if u, err := s.db.GetUserByID(user.ID); assert.NoError(s.T(), err) { + assert.Nil(s.T(), u) + } +}