From f4fb8dbc8739e5b9ca5d388895179fbaf8944453 Mon Sep 17 00:00:00 2001 From: singuliere <35190819+singuliere@users.noreply.github.com> Date: Tue, 10 May 2022 14:05:34 +0100 Subject: [PATCH] [doctor] Add check/fix for bogus action rows (#19656) (#19669) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Loïc Dachary Conflicts: models/consistency_test.go trivial context conflict. --- models/consistency.go | 21 ++++++++++++++++ models/consistency_test.go | 44 +++++++++++++++++++++++++++++++++ modules/doctor/dbconsistency.go | 9 +++++++ 3 files changed, 74 insertions(+) diff --git a/models/consistency.go b/models/consistency.go index 0b9d9fd2c3..df8b8e48df 100644 --- a/models/consistency.go +++ b/models/consistency.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" "xorm.io/builder" ) @@ -247,3 +248,23 @@ func FixIssueLabelWithOutsideLabels() (int64, error) { return res.RowsAffected() } + +// CountActionCreatedUnixString count actions where created_unix is an empty string +func CountActionCreatedUnixString() (int64, error) { + if setting.Database.UseSQLite3 { + return db.GetEngine(db.DefaultContext).Where(`created_unix = ""`).Count(new(Action)) + } + return 0, nil +} + +// FixActionCreatedUnixString set created_unix to zero if it is an empty string +func FixActionCreatedUnixString() (int64, error) { + if setting.Database.UseSQLite3 { + res, err := db.GetEngine(db.DefaultContext).Exec(`UPDATE action SET created_unix = 0 WHERE created_unix = ""`) + if err != nil { + return 0, err + } + return res.RowsAffected() + } + return 0, nil +} diff --git a/models/consistency_test.go b/models/consistency_test.go index d49a0132f0..d7c6cf97bd 100644 --- a/models/consistency_test.go +++ b/models/consistency_test.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" ) @@ -33,3 +34,46 @@ func TestDeleteOrphanedObjects(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, countBefore, countAfter) } + +func TestConsistencyUpdateAction(t *testing.T) { + if !setting.Database.UseSQLite3 { + t.Skip("Test is only for SQLite database.") + } + assert.NoError(t, unittest.PrepareTestDatabase()) + id := 8 + unittest.AssertExistsAndLoadBean(t, &Action{ + ID: int64(id), + }) + _, err := db.GetEngine(db.DefaultContext).Exec(`UPDATE action SET created_unix = "" WHERE id = ?`, id) + assert.NoError(t, err) + actions := make([]*Action, 0, 1) + // + // XORM returns an error when created_unix is a string + // + err = db.GetEngine(db.DefaultContext).Where("id = ?", id).Find(&actions) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "type string to a int64: invalid syntax") + } + // + // Get rid of incorrectly set created_unix + // + count, err := CountActionCreatedUnixString() + assert.NoError(t, err) + assert.EqualValues(t, 1, count) + count, err = FixActionCreatedUnixString() + assert.NoError(t, err) + assert.EqualValues(t, 1, count) + + count, err = CountActionCreatedUnixString() + assert.NoError(t, err) + assert.EqualValues(t, 0, count) + count, err = FixActionCreatedUnixString() + assert.NoError(t, err) + assert.EqualValues(t, 0, count) + + // + // XORM must be happy now + // + assert.NoError(t, db.GetEngine(db.DefaultContext).Where("id = ?", id).Find(&actions)) + unittest.CheckConsistencyFor(t, &Action{}) +} diff --git a/modules/doctor/dbconsistency.go b/modules/doctor/dbconsistency.go index cd34994e1a..a7c8312e07 100644 --- a/modules/doctor/dbconsistency.go +++ b/modules/doctor/dbconsistency.go @@ -142,6 +142,12 @@ func checkDBConsistency(logger log.Logger, autofix bool) error { Fixer: models.FixIssueLabelWithOutsideLabels, FixedMessage: "Removed", }, + { + Name: "Action with created_unix set as an empty string", + Counter: models.CountActionCreatedUnixString, + Fixer: models.FixActionCreatedUnixString, + FixedMessage: "Set to zero", + }, } // TODO: function to recalc all counters @@ -177,6 +183,9 @@ func checkDBConsistency(logger log.Logger, autofix bool) error { // find access without repository genericOrphanCheck("Access entries without existing repository", "access", "repository", "access.repo_id=repository.id"), + // find action without repository + genericOrphanCheck("Action entries without existing repository", + "action", "repository", "action.repo_id=repository.id"), ) for _, c := range consistencyChecks {