From 9238ab7e8b9ee66e961726a7b7f23039fafc25cc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 23 Sep 2021 21:39:57 +0100 Subject: [PATCH 1/7] Add repounit fixer for #16961 Unfortunately a number of people appear to have been bitten by the bug in the dump command. This PR adds a doctor command to attempt to fix the broken repo_units Fix #16961 Signed-off-by: Andrew Thornton --- models/repo_unit.go | 5 + modules/doctor/fix16961.go | 260 +++++++++++++++++++++++++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 modules/doctor/fix16961.go diff --git a/models/repo_unit.go b/models/repo_unit.go index c35312be604f8..6c7a025e4614d 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -219,3 +219,8 @@ func getUnitsByRepoID(e db.Engine, repoID int64) (units []*RepoUnit, err error) return units, nil } + +func UpdateRepoUnit(unit *RepoUnit) error { + _, err := db.GetEngine(db.DefaultContext).ID(unit.ID).Update(unit) + return err +} diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go new file mode 100644 index 0000000000000..3aa40ba60806b --- /dev/null +++ b/modules/doctor/fix16961.go @@ -0,0 +1,260 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package doctor + +import ( + "bytes" + "fmt" + "strconv" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/timeutil" + "xorm.io/builder" +) + +// #16831 revealed that the dump command that was broken in 1.14.3-1.14.6 and 1.15.0 (#15885). +// This led to repo_unit and login_source cfg not being converted to JSON in the dump +// Unfortunately although it was hoped that there were only a few users affected it +// appears that many users are affected. + +// We therefore need to provide a doctor command to fix this repeated issue #16961 + +func fixBrokenRepoUnits(logger log.Logger, autofix bool) error { + // RepoUnit describes all units of a repository + type RepoUnit struct { + ID int64 + RepoID int64 + Type models.UnitType + Config []byte + CreatedUnix timeutil.TimeStamp `xorm:"INDEX CREATED"` + } + + count := 0 + + err := db.Iterate( + db.DefaultContext, + new(RepoUnit), + builder.Eq{"1": "1"}, + func(idx int, bean interface{}) error { + unit := bean.(*RepoUnit) + + bs := unit.Config + repoUnit := &models.RepoUnit{ + ID: unit.ID, + RepoID: unit.RepoID, + Type: unit.Type, + CreatedUnix: unit.CreatedUnix, + } + + switch models.UnitType(unit.Type) { + case models.UnitTypeCode, models.UnitTypeReleases, models.UnitTypeWiki, models.UnitTypeProjects: + cfg := &models.UnitConfig{} + repoUnit.Config = cfg + + err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return nil + } + + // Handle #16961 + if string(bs) != "&{}" { + return err + } + case models.UnitTypeExternalWiki: + cfg := &models.ExternalWikiConfig{} + repoUnit.Config = cfg + err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return nil + } + + if len(bs) < 3 { + return err + } + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return err + } + cfg.ExternalWikiURL = string(bs[2 : len(bs)-1]) + case models.UnitTypeExternalTracker: + cfg := &models.ExternalTrackerConfig{} + repoUnit.Config = cfg + err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return nil + } + // Handle #16961 + if len(bs) < 3 { + return err + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return err + } + + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) != 3 { + return err + } + + cfg.ExternalTrackerURL = string(bytes.Join(parts[:len(parts)-2], []byte{' '})) + cfg.ExternalTrackerFormat = string(parts[len(parts)-2]) + cfg.ExternalTrackerStyle = string(parts[len(parts)-1]) + case models.UnitTypePullRequests: + cfg := &models.PullRequestsConfig{} + repoUnit.Config = cfg + err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return nil + } + + // Handle #16961 + if len(bs) < 3 { + return err + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return err + } + + // PullRequestsConfig was the following in 1.14 + // type PullRequestsConfig struct { + // IgnoreWhitespaceConflicts bool + // AllowMerge bool + // AllowRebase bool + // AllowRebaseMerge bool + // AllowSquash bool + // AllowManualMerge bool + // AutodetectManualMerge bool + // } + // + // 1.15 added in addition: + // DefaultDeleteBranchAfterMerge bool + // DefaultMergeStyle MergeStyle + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) < 7 { + return err + } + + var parseErr error + cfg.IgnoreWhitespaceConflicts, parseErr = strconv.ParseBool(string(parts[0])) + if parseErr != nil { + return err + } + cfg.AllowMerge, parseErr = strconv.ParseBool(string(parts[1])) + if parseErr != nil { + return err + } + cfg.AllowRebase, parseErr = strconv.ParseBool(string(parts[2])) + if parseErr != nil { + return err + } + cfg.AllowRebaseMerge, parseErr = strconv.ParseBool(string(parts[3])) + if parseErr != nil { + return err + } + cfg.AllowSquash, parseErr = strconv.ParseBool(string(parts[4])) + if parseErr != nil { + return err + } + cfg.AllowManualMerge, parseErr = strconv.ParseBool(string(parts[5])) + if parseErr != nil { + return err + } + cfg.AutodetectManualMerge, parseErr = strconv.ParseBool(string(parts[6])) + if parseErr != nil { + return err + } + + // 1.14 unit + if len(parts) == 7 { + count++ + if !autofix { + return nil + } + return models.UpdateRepoUnit(repoUnit) + } + + if len(parts) < 9 { + return err + } + + cfg.DefaultDeleteBranchAfterMerge, parseErr = strconv.ParseBool(string(parts[7])) + if parseErr != nil { + return err + } + + cfg.DefaultMergeStyle = models.MergeStyle(string(bytes.Join(parts[8:], []byte{' '}))) + case models.UnitTypeIssues: + cfg := &models.IssuesConfig{} + repoUnit.Config = cfg + err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return nil + } + + // Handle #16961 + if len(bs) < 3 { + return err + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return err + } + + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) != 3 { + return err + } + var parseErr error + cfg.EnableTimetracker, parseErr = strconv.ParseBool(string(parts[0])) + if parseErr != nil { + return err + } + cfg.AllowOnlyContributorsToTrackTime, parseErr = strconv.ParseBool(string(parts[1])) + if parseErr != nil { + return err + } + cfg.EnableDependencies, parseErr = strconv.ParseBool(string(parts[2])) + if parseErr != nil { + return err + } + default: + panic(fmt.Sprintf("unrecognized repo unit type: %v", unit.Type)) + } + + count++ + if !autofix { + return nil + } + + return models.UpdateRepoUnit(repoUnit) + }, + ) + + if err != nil { + logger.Critical("Unable to iterate acrosss repounits to fix the broken units: Error %v", err) + return err + } + + if !autofix { + logger.Warn("Found %d broken repo_units", count) + return nil + } + logger.Info("Fixed %d broken repo_units", count) + + return nil +} + +func init() { + Register(&Check{ + Title: "Check for incorrectly dumped repo_units (See #16961)", + Name: "fix-broken-repo-units", + IsDefault: false, + Run: fixBrokenRepoUnits, + Priority: 7, + }) +} From bebdd4add28f0b127a04f2bd75ccc3cb5fd72a0f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 24 Sep 2021 09:40:41 +0100 Subject: [PATCH 2/7] Begin adding testcases Signed-off-by: Andrew Thornton --- models/helper.go | 2 +- modules/doctor/fix16961.go | 392 ++++++++++++++++++-------------- modules/doctor/fix16961_test.go | 97 ++++++++ 3 files changed, 314 insertions(+), 177 deletions(-) create mode 100644 modules/doctor/fix16961_test.go diff --git a/models/helper.go b/models/helper.go index c499b5512ddc7..710c15a978371 100644 --- a/models/helper.go +++ b/models/helper.go @@ -51,7 +51,7 @@ func JSONUnmarshalHandleDoubleEncode(bs []byte, v interface{}) error { rs = append(rs, temp...) } if ok { - if rs[0] == 0xff && rs[1] == 0xfe { + if len(rs) > 1 && rs[0] == 0xff && rs[1] == 0xfe { rs = rs[2:] } err = json.Unmarshal(rs, v) diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go index 3aa40ba60806b..2f8fbbd11a743 100644 --- a/modules/doctor/fix16961.go +++ b/modules/doctor/fix16961.go @@ -23,7 +23,219 @@ import ( // We therefore need to provide a doctor command to fix this repeated issue #16961 -func fixBrokenRepoUnits(logger log.Logger, autofix bool) error { +func fixUnitConfig_16961(bs []byte, cfg *models.UnitConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + // Handle #16961 + if string(bs) != "&{}" && len(bs) != 0 { + return + } + + return true, nil +} + +func fixExternalWikiConfig_16961(bs []byte, cfg *models.ExternalWikiConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + if len(bs) < 3 { + return + } + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + cfg.ExternalWikiURL = string(bs[2 : len(bs)-1]) + return true, nil +} + +func fixExternalTrackerConfig_16961(bs []byte, cfg *models.ExternalTrackerConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + // Handle #16961 + if len(bs) < 3 { + return + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) != 3 { + return + } + + cfg.ExternalTrackerURL = string(bytes.Join(parts[:len(parts)-2], []byte{' '})) + cfg.ExternalTrackerFormat = string(parts[len(parts)-2]) + cfg.ExternalTrackerStyle = string(parts[len(parts)-1]) + return true, nil +} + +func fixPullRequestsConfig_16961(bs []byte, cfg *models.PullRequestsConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + // Handle #16961 + if len(bs) < 3 { + return + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + + // PullRequestsConfig was the following in 1.14 + // type PullRequestsConfig struct { + // IgnoreWhitespaceConflicts bool + // AllowMerge bool + // AllowRebase bool + // AllowRebaseMerge bool + // AllowSquash bool + // AllowManualMerge bool + // AutodetectManualMerge bool + // } + // + // 1.15 added in addition: + // DefaultDeleteBranchAfterMerge bool + // DefaultMergeStyle MergeStyle + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) < 7 { + return + } + + var parseErr error + cfg.IgnoreWhitespaceConflicts, parseErr = strconv.ParseBool(string(parts[0])) + if parseErr != nil { + return + } + cfg.AllowMerge, parseErr = strconv.ParseBool(string(parts[1])) + if parseErr != nil { + return + } + cfg.AllowRebase, parseErr = strconv.ParseBool(string(parts[2])) + if parseErr != nil { + return + } + cfg.AllowRebaseMerge, parseErr = strconv.ParseBool(string(parts[3])) + if parseErr != nil { + return + } + cfg.AllowSquash, parseErr = strconv.ParseBool(string(parts[4])) + if parseErr != nil { + return + } + cfg.AllowManualMerge, parseErr = strconv.ParseBool(string(parts[5])) + if parseErr != nil { + return + } + cfg.AutodetectManualMerge, parseErr = strconv.ParseBool(string(parts[6])) + if parseErr != nil { + return + } + + // 1.14 unit + if len(parts) == 7 { + return true, nil + } + + if len(parts) < 9 { + return + } + + cfg.DefaultDeleteBranchAfterMerge, parseErr = strconv.ParseBool(string(parts[7])) + if parseErr != nil { + return + } + + cfg.DefaultMergeStyle = models.MergeStyle(string(bytes.Join(parts[8:], []byte{' '}))) + return true, nil +} + +func fixIssuesConfig_16961(bs []byte, cfg *models.IssuesConfig) (fixed bool, err error) { + err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) + if err == nil { + return + } + + // Handle #16961 + if len(bs) < 3 { + return + } + + if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { + return + } + + parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) + if len(parts) != 3 { + return + } + var parseErr error + cfg.EnableTimetracker, parseErr = strconv.ParseBool(string(parts[0])) + if parseErr != nil { + return + } + cfg.AllowOnlyContributorsToTrackTime, parseErr = strconv.ParseBool(string(parts[1])) + if parseErr != nil { + return + } + cfg.EnableDependencies, parseErr = strconv.ParseBool(string(parts[2])) + if parseErr != nil { + return + } + return true, nil +} + +func fixBrokenRepoUnit_16961(repoUnit *models.RepoUnit, bs []byte) (fixed bool, err error) { + switch models.UnitType(repoUnit.Type) { + case models.UnitTypeCode, models.UnitTypeReleases, models.UnitTypeWiki, models.UnitTypeProjects: + cfg := &models.UnitConfig{} + repoUnit.Config = cfg + if fixed, err := fixUnitConfig_16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypeExternalWiki: + cfg := &models.ExternalWikiConfig{} + repoUnit.Config = cfg + + if fixed, err := fixExternalWikiConfig_16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypeExternalTracker: + cfg := &models.ExternalTrackerConfig{} + repoUnit.Config = cfg + if fixed, err := fixExternalTrackerConfig_16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypePullRequests: + cfg := &models.PullRequestsConfig{} + repoUnit.Config = cfg + + if fixed, err := fixPullRequestsConfig_16961(bs, cfg); !fixed { + return false, err + } + case models.UnitTypeIssues: + cfg := &models.IssuesConfig{} + repoUnit.Config = cfg + if fixed, err := fixIssuesConfig_16961(bs, cfg); !fixed { + return false, err + } + default: + panic(fmt.Sprintf("unrecognized repo unit type: %v", repoUnit.Type)) + } + return true, nil +} + +func fixBrokenRepoUnits_16961(logger log.Logger, autofix bool) error { // RepoUnit describes all units of a repository type RepoUnit struct { ID int64 @@ -50,180 +262,8 @@ func fixBrokenRepoUnits(logger log.Logger, autofix bool) error { CreatedUnix: unit.CreatedUnix, } - switch models.UnitType(unit.Type) { - case models.UnitTypeCode, models.UnitTypeReleases, models.UnitTypeWiki, models.UnitTypeProjects: - cfg := &models.UnitConfig{} - repoUnit.Config = cfg - - err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) - if err == nil { - return nil - } - - // Handle #16961 - if string(bs) != "&{}" { - return err - } - case models.UnitTypeExternalWiki: - cfg := &models.ExternalWikiConfig{} - repoUnit.Config = cfg - err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) - if err == nil { - return nil - } - - if len(bs) < 3 { - return err - } - if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return err - } - cfg.ExternalWikiURL = string(bs[2 : len(bs)-1]) - case models.UnitTypeExternalTracker: - cfg := &models.ExternalTrackerConfig{} - repoUnit.Config = cfg - err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) - if err == nil { - return nil - } - // Handle #16961 - if len(bs) < 3 { - return err - } - - if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return err - } - - parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) - if len(parts) != 3 { - return err - } - - cfg.ExternalTrackerURL = string(bytes.Join(parts[:len(parts)-2], []byte{' '})) - cfg.ExternalTrackerFormat = string(parts[len(parts)-2]) - cfg.ExternalTrackerStyle = string(parts[len(parts)-1]) - case models.UnitTypePullRequests: - cfg := &models.PullRequestsConfig{} - repoUnit.Config = cfg - err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) - if err == nil { - return nil - } - - // Handle #16961 - if len(bs) < 3 { - return err - } - - if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return err - } - - // PullRequestsConfig was the following in 1.14 - // type PullRequestsConfig struct { - // IgnoreWhitespaceConflicts bool - // AllowMerge bool - // AllowRebase bool - // AllowRebaseMerge bool - // AllowSquash bool - // AllowManualMerge bool - // AutodetectManualMerge bool - // } - // - // 1.15 added in addition: - // DefaultDeleteBranchAfterMerge bool - // DefaultMergeStyle MergeStyle - parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) - if len(parts) < 7 { - return err - } - - var parseErr error - cfg.IgnoreWhitespaceConflicts, parseErr = strconv.ParseBool(string(parts[0])) - if parseErr != nil { - return err - } - cfg.AllowMerge, parseErr = strconv.ParseBool(string(parts[1])) - if parseErr != nil { - return err - } - cfg.AllowRebase, parseErr = strconv.ParseBool(string(parts[2])) - if parseErr != nil { - return err - } - cfg.AllowRebaseMerge, parseErr = strconv.ParseBool(string(parts[3])) - if parseErr != nil { - return err - } - cfg.AllowSquash, parseErr = strconv.ParseBool(string(parts[4])) - if parseErr != nil { - return err - } - cfg.AllowManualMerge, parseErr = strconv.ParseBool(string(parts[5])) - if parseErr != nil { - return err - } - cfg.AutodetectManualMerge, parseErr = strconv.ParseBool(string(parts[6])) - if parseErr != nil { - return err - } - - // 1.14 unit - if len(parts) == 7 { - count++ - if !autofix { - return nil - } - return models.UpdateRepoUnit(repoUnit) - } - - if len(parts) < 9 { - return err - } - - cfg.DefaultDeleteBranchAfterMerge, parseErr = strconv.ParseBool(string(parts[7])) - if parseErr != nil { - return err - } - - cfg.DefaultMergeStyle = models.MergeStyle(string(bytes.Join(parts[8:], []byte{' '}))) - case models.UnitTypeIssues: - cfg := &models.IssuesConfig{} - repoUnit.Config = cfg - err := models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) - if err == nil { - return nil - } - - // Handle #16961 - if len(bs) < 3 { - return err - } - - if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return err - } - - parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) - if len(parts) != 3 { - return err - } - var parseErr error - cfg.EnableTimetracker, parseErr = strconv.ParseBool(string(parts[0])) - if parseErr != nil { - return err - } - cfg.AllowOnlyContributorsToTrackTime, parseErr = strconv.ParseBool(string(parts[1])) - if parseErr != nil { - return err - } - cfg.EnableDependencies, parseErr = strconv.ParseBool(string(parts[2])) - if parseErr != nil { - return err - } - default: - panic(fmt.Sprintf("unrecognized repo unit type: %v", unit.Type)) + if fixed, err := fixBrokenRepoUnit_16961(repoUnit, bs); !fixed { + return err } count++ @@ -254,7 +294,7 @@ func init() { Title: "Check for incorrectly dumped repo_units (See #16961)", Name: "fix-broken-repo-units", IsDefault: false, - Run: fixBrokenRepoUnits, + Run: fixBrokenRepoUnits_16961, Priority: 7, }) } diff --git a/modules/doctor/fix16961_test.go b/modules/doctor/fix16961_test.go new file mode 100644 index 0000000000000..db8550281ff9c --- /dev/null +++ b/modules/doctor/fix16961_test.go @@ -0,0 +1,97 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package doctor + +import ( + "testing" + + "code.gitea.io/gitea/models" +) + +func Test_fixUnitConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + wantFixed bool + wantErr bool + }{ + { + name: "empty", + bs: "", + wantFixed: true, + wantErr: false, + }, + { + name: "normal: {}", + bs: "{}", + wantFixed: false, + wantErr: false, + }, + { + name: "broken but fixable: &{}", + bs: "&{}", + wantFixed: true, + wantErr: false, + }, + { + name: "broken but unfixable: &{asdasd}", + bs: "&{asdasd}", + wantFixed: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotFixed, err := fixUnitConfig_16961([]byte(tt.bs), &models.UnitConfig{}) + if (err != nil) != tt.wantErr { + t.Errorf("fixUnitConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixUnitConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + }) + } +} + +func Test_fixExternalWikiConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + wantFixed bool + wantErr bool + }{ + { + name: "normal: {\"ExternalWikiURL\":\"http://someurl\"}", + bs: "{\"ExternalWikiURL\":\"http://someurl\"}", + wantFixed: false, + wantErr: false, + }, + { + name: "broken: &{http://someurl}", + bs: "&{http://someurl}", + wantFixed: true, + wantErr: false, + }, + { + name: "broken but unfixable: http://someurl", + bs: "http://someurl", + wantFixed: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotFixed, err := fixExternalWikiConfig_16961([]byte(tt.bs), &models.ExternalWikiConfig{}) + if (err != nil) != tt.wantErr { + t.Errorf("fixExternalWikiConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixExternalWikiConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + }) + } +} From a9e4520661356a15bb2c92ffdede454f1b86763b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 24 Sep 2021 10:30:44 +0100 Subject: [PATCH 3/7] add more tests Signed-off-by: Andrew Thornton --- modules/doctor/fix16961.go | 40 +++++--- modules/doctor/fix16961_test.go | 176 +++++++++++++++++++++++++++++++- 2 files changed, 203 insertions(+), 13 deletions(-) diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go index 2f8fbbd11a743..d97ec704f3167 100644 --- a/modules/doctor/fix16961.go +++ b/modules/doctor/fix16961.go @@ -7,7 +7,6 @@ package doctor import ( "bytes" "fmt" - "strconv" "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" @@ -23,6 +22,18 @@ import ( // We therefore need to provide a doctor command to fix this repeated issue #16961 +func parseBool_16961(bs []byte) (bool, error) { + if bytes.EqualFold(bs, []byte("%!s(bool=false)")) { + return false, nil + } + + if bytes.EqualFold(bs, []byte("%!s(bool=true)")) { + return true, nil + } + + return false, fmt.Errorf("unexpected bool format: %s", string(bs)) +} + func fixUnitConfig_16961(bs []byte, cfg *models.UnitConfig) (fixed bool, err error) { err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { @@ -113,31 +124,31 @@ func fixPullRequestsConfig_16961(bs []byte, cfg *models.PullRequestsConfig) (fix } var parseErr error - cfg.IgnoreWhitespaceConflicts, parseErr = strconv.ParseBool(string(parts[0])) + cfg.IgnoreWhitespaceConflicts, parseErr = parseBool_16961(parts[0]) if parseErr != nil { return } - cfg.AllowMerge, parseErr = strconv.ParseBool(string(parts[1])) + cfg.AllowMerge, parseErr = parseBool_16961(parts[1]) if parseErr != nil { return } - cfg.AllowRebase, parseErr = strconv.ParseBool(string(parts[2])) + cfg.AllowRebase, parseErr = parseBool_16961(parts[2]) if parseErr != nil { return } - cfg.AllowRebaseMerge, parseErr = strconv.ParseBool(string(parts[3])) + cfg.AllowRebaseMerge, parseErr = parseBool_16961(parts[3]) if parseErr != nil { return } - cfg.AllowSquash, parseErr = strconv.ParseBool(string(parts[4])) + cfg.AllowSquash, parseErr = parseBool_16961(parts[4]) if parseErr != nil { return } - cfg.AllowManualMerge, parseErr = strconv.ParseBool(string(parts[5])) + cfg.AllowManualMerge, parseErr = parseBool_16961(parts[5]) if parseErr != nil { return } - cfg.AutodetectManualMerge, parseErr = strconv.ParseBool(string(parts[6])) + cfg.AutodetectManualMerge, parseErr = parseBool_16961(parts[6]) if parseErr != nil { return } @@ -151,7 +162,7 @@ func fixPullRequestsConfig_16961(bs []byte, cfg *models.PullRequestsConfig) (fix return } - cfg.DefaultDeleteBranchAfterMerge, parseErr = strconv.ParseBool(string(parts[7])) + cfg.DefaultDeleteBranchAfterMerge, parseErr = parseBool_16961(parts[7]) if parseErr != nil { return } @@ -180,15 +191,15 @@ func fixIssuesConfig_16961(bs []byte, cfg *models.IssuesConfig) (fixed bool, err return } var parseErr error - cfg.EnableTimetracker, parseErr = strconv.ParseBool(string(parts[0])) + cfg.EnableTimetracker, parseErr = parseBool_16961(parts[0]) if parseErr != nil { return } - cfg.AllowOnlyContributorsToTrackTime, parseErr = strconv.ParseBool(string(parts[1])) + cfg.AllowOnlyContributorsToTrackTime, parseErr = parseBool_16961(parts[1]) if parseErr != nil { return } - cfg.EnableDependencies, parseErr = strconv.ParseBool(string(parts[2])) + cfg.EnableDependencies, parseErr = parseBool_16961(parts[2]) if parseErr != nil { return } @@ -196,6 +207,11 @@ func fixIssuesConfig_16961(bs []byte, cfg *models.IssuesConfig) (fixed bool, err } func fixBrokenRepoUnit_16961(repoUnit *models.RepoUnit, bs []byte) (fixed bool, err error) { + // Shortcut empty or null values + if len(bs) == 0 { + return false, nil + } + switch models.UnitType(repoUnit.Type) { case models.UnitTypeCode, models.UnitTypeReleases, models.UnitTypeWiki, models.UnitTypeProjects: cfg := &models.UnitConfig{} diff --git a/modules/doctor/fix16961_test.go b/modules/doctor/fix16961_test.go index db8550281ff9c..c074e663a2e00 100644 --- a/modules/doctor/fix16961_test.go +++ b/modules/doctor/fix16961_test.go @@ -8,6 +8,7 @@ import ( "testing" "code.gitea.io/gitea/models" + "github.com/stretchr/testify/assert" ) func Test_fixUnitConfig_16961(t *testing.T) { @@ -60,18 +61,21 @@ func Test_fixExternalWikiConfig_16961(t *testing.T) { tests := []struct { name string bs string + expected string wantFixed bool wantErr bool }{ { name: "normal: {\"ExternalWikiURL\":\"http://someurl\"}", bs: "{\"ExternalWikiURL\":\"http://someurl\"}", + expected: "http://someurl", wantFixed: false, wantErr: false, }, { name: "broken: &{http://someurl}", bs: "&{http://someurl}", + expected: "http://someurl", wantFixed: true, wantErr: false, }, @@ -84,7 +88,8 @@ func Test_fixExternalWikiConfig_16961(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotFixed, err := fixExternalWikiConfig_16961([]byte(tt.bs), &models.ExternalWikiConfig{}) + cfg := &models.ExternalWikiConfig{} + gotFixed, err := fixExternalWikiConfig_16961([]byte(tt.bs), cfg) if (err != nil) != tt.wantErr { t.Errorf("fixExternalWikiConfig_16961() error = %v, wantErr %v", err, tt.wantErr) return @@ -92,6 +97,175 @@ func Test_fixExternalWikiConfig_16961(t *testing.T) { if gotFixed != tt.wantFixed { t.Errorf("fixExternalWikiConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) } + if cfg.ExternalWikiURL != tt.expected { + t.Errorf("fixExternalWikiConfig_16961().ExternalWikiURL = %v, want %v", cfg.ExternalWikiURL, tt.expected) + } + }) + } +} + +func Test_fixExternalTrackerConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + expected models.ExternalTrackerConfig + wantFixed bool + wantErr bool + }{ + { + name: "normal", + bs: `{"ExternalTrackerURL":"a","ExternalTrackerFormat":"b","ExternalTrackerStyle":"c"}`, + expected: models.ExternalTrackerConfig{ + ExternalTrackerURL: "a", + ExternalTrackerFormat: "b", + ExternalTrackerStyle: "c", + }, + wantFixed: false, + wantErr: false, + }, + { + name: "broken", + bs: "&{a b c}", + expected: models.ExternalTrackerConfig{ + ExternalTrackerURL: "a", + ExternalTrackerFormat: "b", + ExternalTrackerStyle: "c", + }, + wantFixed: true, + wantErr: false, + }, + { + name: "broken - too many fields", + bs: "&{a b c d}", + wantFixed: false, + wantErr: true, + }, + { + name: "broken - wrong format", + bs: "a b c d}", + wantFixed: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &models.ExternalTrackerConfig{} + gotFixed, err := fixExternalTrackerConfig_16961([]byte(tt.bs), cfg) + if (err != nil) != tt.wantErr { + t.Errorf("fixExternalTrackerConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixExternalTrackerConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + if cfg.ExternalTrackerFormat != tt.expected.ExternalTrackerFormat { + t.Errorf("fixExternalTrackerConfig_16961().ExternalTrackerFormat = %v, want %v", tt.expected.ExternalTrackerFormat, cfg.ExternalTrackerFormat) + } + if cfg.ExternalTrackerStyle != tt.expected.ExternalTrackerStyle { + t.Errorf("fixExternalTrackerConfig_16961().ExternalTrackerStyle = %v, want %v", tt.expected.ExternalTrackerStyle, cfg.ExternalTrackerStyle) + } + if cfg.ExternalTrackerURL != tt.expected.ExternalTrackerURL { + t.Errorf("fixExternalTrackerConfig_16961().ExternalTrackerURL = %v, want %v", tt.expected.ExternalTrackerURL, cfg.ExternalTrackerURL) + } + }) + } +} + +func Test_fixPullRequestsConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + expected models.PullRequestsConfig + wantFixed bool + wantErr bool + }{ + { + name: "normal", + bs: `{"IgnoreWhitespaceConflicts":false,"AllowMerge":false,"AllowRebase":false,"AllowRebaseMerge":false,"AllowSquash":false,"AllowManualMerge":false,"AutodetectManualMerge":false,"DefaultDeleteBranchAfterMerge":false,"DefaultMergeStyle":""}`, + }, + { + name: "broken - 1.14", + bs: `&{%!s(bool=false) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=false) %!s(bool=false)}`, + expected: models.PullRequestsConfig{ + IgnoreWhitespaceConflicts: false, + AllowMerge: true, + AllowRebase: true, + AllowRebaseMerge: true, + AllowSquash: true, + AllowManualMerge: false, + AutodetectManualMerge: false, + }, + wantFixed: true, + }, + { + name: "broken - 1.15", + bs: `&{%!s(bool=false) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=true) %!s(bool=false) %!s(bool=false) %!s(bool=false) merge}`, + expected: models.PullRequestsConfig{ + AllowMerge: true, + AllowRebase: true, + AllowRebaseMerge: true, + AllowSquash: true, + DefaultMergeStyle: models.MergeStyleMerge, + }, + wantFixed: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &models.PullRequestsConfig{} + gotFixed, err := fixPullRequestsConfig_16961([]byte(tt.bs), cfg) + if (err != nil) != tt.wantErr { + t.Errorf("fixPullRequestsConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixPullRequestsConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + assert.EqualValues(t, &tt.expected, cfg) + }) + } +} + +func Test_fixIssuesConfig_16961(t *testing.T) { + tests := []struct { + name string + bs string + expected models.IssuesConfig + wantFixed bool + wantErr bool + }{ + { + name: "normal", + bs: `{"EnableTimetracker":true,"AllowOnlyContributorsToTrackTime":true,"EnableDependencies":true}`, + expected: models.IssuesConfig{ + EnableTimetracker: true, + AllowOnlyContributorsToTrackTime: true, + EnableDependencies: true, + }, + }, + { + name: "broken", + bs: `&{%!s(bool=true) %!s(bool=true) %!s(bool=true)}`, + expected: models.IssuesConfig{ + EnableTimetracker: true, + AllowOnlyContributorsToTrackTime: true, + EnableDependencies: true, + }, + wantFixed: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &models.IssuesConfig{} + gotFixed, err := fixIssuesConfig_16961([]byte(tt.bs), cfg) + if (err != nil) != tt.wantErr { + t.Errorf("fixIssuesConfig_16961() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotFixed != tt.wantFixed { + t.Errorf("fixIssuesConfig_16961() = %v, want %v", gotFixed, tt.wantFixed) + } + assert.EqualValues(t, &tt.expected, cfg) }) } } From a7e130fb3e4b01566ef35222eff91020b5d736fe Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 24 Sep 2021 12:31:34 +0100 Subject: [PATCH 4/7] placate lint Signed-off-by: Andrew Thornton --- models/repo_unit.go | 1 + modules/doctor/fix16961.go | 52 ++++++++++++++++----------------- modules/doctor/fix16961_test.go | 10 +++---- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/models/repo_unit.go b/models/repo_unit.go index 6c7a025e4614d..f5305c0dad9eb 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -220,6 +220,7 @@ func getUnitsByRepoID(e db.Engine, repoID int64) (units []*RepoUnit, err error) return units, nil } +// UpdateRepoUnit updates the provided repo unit func UpdateRepoUnit(unit *RepoUnit) error { _, err := db.GetEngine(db.DefaultContext).ID(unit.ID).Update(unit) return err diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go index d97ec704f3167..63f242296c2ae 100644 --- a/modules/doctor/fix16961.go +++ b/modules/doctor/fix16961.go @@ -22,7 +22,7 @@ import ( // We therefore need to provide a doctor command to fix this repeated issue #16961 -func parseBool_16961(bs []byte) (bool, error) { +func parseBool16961(bs []byte) (bool, error) { if bytes.EqualFold(bs, []byte("%!s(bool=false)")) { return false, nil } @@ -34,7 +34,7 @@ func parseBool_16961(bs []byte) (bool, error) { return false, fmt.Errorf("unexpected bool format: %s", string(bs)) } -func fixUnitConfig_16961(bs []byte, cfg *models.UnitConfig) (fixed bool, err error) { +func fixUnitConfig16961(bs []byte, cfg *models.UnitConfig) (fixed bool, err error) { err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { return @@ -48,7 +48,7 @@ func fixUnitConfig_16961(bs []byte, cfg *models.UnitConfig) (fixed bool, err err return true, nil } -func fixExternalWikiConfig_16961(bs []byte, cfg *models.ExternalWikiConfig) (fixed bool, err error) { +func fixExternalWikiConfig16961(bs []byte, cfg *models.ExternalWikiConfig) (fixed bool, err error) { err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { return @@ -64,7 +64,7 @@ func fixExternalWikiConfig_16961(bs []byte, cfg *models.ExternalWikiConfig) (fix return true, nil } -func fixExternalTrackerConfig_16961(bs []byte, cfg *models.ExternalTrackerConfig) (fixed bool, err error) { +func fixExternalTrackerConfig16961(bs []byte, cfg *models.ExternalTrackerConfig) (fixed bool, err error) { err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { return @@ -89,7 +89,7 @@ func fixExternalTrackerConfig_16961(bs []byte, cfg *models.ExternalTrackerConfig return true, nil } -func fixPullRequestsConfig_16961(bs []byte, cfg *models.PullRequestsConfig) (fixed bool, err error) { +func fixPullRequestsConfig16961(bs []byte, cfg *models.PullRequestsConfig) (fixed bool, err error) { err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { return @@ -124,31 +124,31 @@ func fixPullRequestsConfig_16961(bs []byte, cfg *models.PullRequestsConfig) (fix } var parseErr error - cfg.IgnoreWhitespaceConflicts, parseErr = parseBool_16961(parts[0]) + cfg.IgnoreWhitespaceConflicts, parseErr = parseBool16961(parts[0]) if parseErr != nil { return } - cfg.AllowMerge, parseErr = parseBool_16961(parts[1]) + cfg.AllowMerge, parseErr = parseBool16961(parts[1]) if parseErr != nil { return } - cfg.AllowRebase, parseErr = parseBool_16961(parts[2]) + cfg.AllowRebase, parseErr = parseBool16961(parts[2]) if parseErr != nil { return } - cfg.AllowRebaseMerge, parseErr = parseBool_16961(parts[3]) + cfg.AllowRebaseMerge, parseErr = parseBool16961(parts[3]) if parseErr != nil { return } - cfg.AllowSquash, parseErr = parseBool_16961(parts[4]) + cfg.AllowSquash, parseErr = parseBool16961(parts[4]) if parseErr != nil { return } - cfg.AllowManualMerge, parseErr = parseBool_16961(parts[5]) + cfg.AllowManualMerge, parseErr = parseBool16961(parts[5]) if parseErr != nil { return } - cfg.AutodetectManualMerge, parseErr = parseBool_16961(parts[6]) + cfg.AutodetectManualMerge, parseErr = parseBool16961(parts[6]) if parseErr != nil { return } @@ -162,7 +162,7 @@ func fixPullRequestsConfig_16961(bs []byte, cfg *models.PullRequestsConfig) (fix return } - cfg.DefaultDeleteBranchAfterMerge, parseErr = parseBool_16961(parts[7]) + cfg.DefaultDeleteBranchAfterMerge, parseErr = parseBool16961(parts[7]) if parseErr != nil { return } @@ -171,7 +171,7 @@ func fixPullRequestsConfig_16961(bs []byte, cfg *models.PullRequestsConfig) (fix return true, nil } -func fixIssuesConfig_16961(bs []byte, cfg *models.IssuesConfig) (fixed bool, err error) { +func fixIssuesConfig16961(bs []byte, cfg *models.IssuesConfig) (fixed bool, err error) { err = models.JSONUnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { return @@ -191,22 +191,22 @@ func fixIssuesConfig_16961(bs []byte, cfg *models.IssuesConfig) (fixed bool, err return } var parseErr error - cfg.EnableTimetracker, parseErr = parseBool_16961(parts[0]) + cfg.EnableTimetracker, parseErr = parseBool16961(parts[0]) if parseErr != nil { return } - cfg.AllowOnlyContributorsToTrackTime, parseErr = parseBool_16961(parts[1]) + cfg.AllowOnlyContributorsToTrackTime, parseErr = parseBool16961(parts[1]) if parseErr != nil { return } - cfg.EnableDependencies, parseErr = parseBool_16961(parts[2]) + cfg.EnableDependencies, parseErr = parseBool16961(parts[2]) if parseErr != nil { return } return true, nil } -func fixBrokenRepoUnit_16961(repoUnit *models.RepoUnit, bs []byte) (fixed bool, err error) { +func fixBrokenRepoUnit16961(repoUnit *models.RepoUnit, bs []byte) (fixed bool, err error) { // Shortcut empty or null values if len(bs) == 0 { return false, nil @@ -216,33 +216,33 @@ func fixBrokenRepoUnit_16961(repoUnit *models.RepoUnit, bs []byte) (fixed bool, case models.UnitTypeCode, models.UnitTypeReleases, models.UnitTypeWiki, models.UnitTypeProjects: cfg := &models.UnitConfig{} repoUnit.Config = cfg - if fixed, err := fixUnitConfig_16961(bs, cfg); !fixed { + if fixed, err := fixUnitConfig16961(bs, cfg); !fixed { return false, err } case models.UnitTypeExternalWiki: cfg := &models.ExternalWikiConfig{} repoUnit.Config = cfg - if fixed, err := fixExternalWikiConfig_16961(bs, cfg); !fixed { + if fixed, err := fixExternalWikiConfig16961(bs, cfg); !fixed { return false, err } case models.UnitTypeExternalTracker: cfg := &models.ExternalTrackerConfig{} repoUnit.Config = cfg - if fixed, err := fixExternalTrackerConfig_16961(bs, cfg); !fixed { + if fixed, err := fixExternalTrackerConfig16961(bs, cfg); !fixed { return false, err } case models.UnitTypePullRequests: cfg := &models.PullRequestsConfig{} repoUnit.Config = cfg - if fixed, err := fixPullRequestsConfig_16961(bs, cfg); !fixed { + if fixed, err := fixPullRequestsConfig16961(bs, cfg); !fixed { return false, err } case models.UnitTypeIssues: cfg := &models.IssuesConfig{} repoUnit.Config = cfg - if fixed, err := fixIssuesConfig_16961(bs, cfg); !fixed { + if fixed, err := fixIssuesConfig16961(bs, cfg); !fixed { return false, err } default: @@ -251,7 +251,7 @@ func fixBrokenRepoUnit_16961(repoUnit *models.RepoUnit, bs []byte) (fixed bool, return true, nil } -func fixBrokenRepoUnits_16961(logger log.Logger, autofix bool) error { +func fixBrokenRepoUnits16961(logger log.Logger, autofix bool) error { // RepoUnit describes all units of a repository type RepoUnit struct { ID int64 @@ -278,7 +278,7 @@ func fixBrokenRepoUnits_16961(logger log.Logger, autofix bool) error { CreatedUnix: unit.CreatedUnix, } - if fixed, err := fixBrokenRepoUnit_16961(repoUnit, bs); !fixed { + if fixed, err := fixBrokenRepoUnit16961(repoUnit, bs); !fixed { return err } @@ -310,7 +310,7 @@ func init() { Title: "Check for incorrectly dumped repo_units (See #16961)", Name: "fix-broken-repo-units", IsDefault: false, - Run: fixBrokenRepoUnits_16961, + Run: fixBrokenRepoUnits16961, Priority: 7, }) } diff --git a/modules/doctor/fix16961_test.go b/modules/doctor/fix16961_test.go index c074e663a2e00..f0225610190a1 100644 --- a/modules/doctor/fix16961_test.go +++ b/modules/doctor/fix16961_test.go @@ -45,7 +45,7 @@ func Test_fixUnitConfig_16961(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotFixed, err := fixUnitConfig_16961([]byte(tt.bs), &models.UnitConfig{}) + gotFixed, err := fixUnitConfig16961([]byte(tt.bs), &models.UnitConfig{}) if (err != nil) != tt.wantErr { t.Errorf("fixUnitConfig_16961() error = %v, wantErr %v", err, tt.wantErr) return @@ -89,7 +89,7 @@ func Test_fixExternalWikiConfig_16961(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cfg := &models.ExternalWikiConfig{} - gotFixed, err := fixExternalWikiConfig_16961([]byte(tt.bs), cfg) + gotFixed, err := fixExternalWikiConfig16961([]byte(tt.bs), cfg) if (err != nil) != tt.wantErr { t.Errorf("fixExternalWikiConfig_16961() error = %v, wantErr %v", err, tt.wantErr) return @@ -150,7 +150,7 @@ func Test_fixExternalTrackerConfig_16961(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cfg := &models.ExternalTrackerConfig{} - gotFixed, err := fixExternalTrackerConfig_16961([]byte(tt.bs), cfg) + gotFixed, err := fixExternalTrackerConfig16961([]byte(tt.bs), cfg) if (err != nil) != tt.wantErr { t.Errorf("fixExternalTrackerConfig_16961() error = %v, wantErr %v", err, tt.wantErr) return @@ -213,7 +213,7 @@ func Test_fixPullRequestsConfig_16961(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cfg := &models.PullRequestsConfig{} - gotFixed, err := fixPullRequestsConfig_16961([]byte(tt.bs), cfg) + gotFixed, err := fixPullRequestsConfig16961([]byte(tt.bs), cfg) if (err != nil) != tt.wantErr { t.Errorf("fixPullRequestsConfig_16961() error = %v, wantErr %v", err, tt.wantErr) return @@ -257,7 +257,7 @@ func Test_fixIssuesConfig_16961(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cfg := &models.IssuesConfig{} - gotFixed, err := fixIssuesConfig_16961([]byte(tt.bs), cfg) + gotFixed, err := fixIssuesConfig16961([]byte(tt.bs), cfg) if (err != nil) != tt.wantErr { t.Errorf("fixIssuesConfig_16961() error = %v, wantErr %v", err, tt.wantErr) return From 82c2bde8d1bf85f5c1166e041f7ba9ff161c35fc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 27 Sep 2021 12:12:37 +0100 Subject: [PATCH 5/7] ensure the fixer actually runs Signed-off-by: Andrew Thornton --- modules/doctor/fix16961.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go index 63f242296c2ae..a480854e04035 100644 --- a/modules/doctor/fix16961.go +++ b/modules/doctor/fix16961.go @@ -266,7 +266,9 @@ func fixBrokenRepoUnits16961(logger log.Logger, autofix bool) error { err := db.Iterate( db.DefaultContext, new(RepoUnit), - builder.Eq{"1": "1"}, + builder.Gt{ + "id": 0, + }, func(idx int, bean interface{}) error { unit := bean.(*RepoUnit) From 9a3ca24f5b9dc88e250bb4d4be6269c360d71b3f Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Mon, 27 Sep 2021 09:40:52 -0400 Subject: [PATCH 6/7] Update modules/doctor/fix16961.go --- modules/doctor/fix16961.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go index a480854e04035..60cc5ffe2fbaa 100644 --- a/modules/doctor/fix16961.go +++ b/modules/doctor/fix16961.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. From ac6a5d07fc77b5fdd0190f10d43bbbbb56a22945 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Mon, 27 Sep 2021 09:42:03 -0400 Subject: [PATCH 7/7] Update modules/doctor/fix16961_test.go --- modules/doctor/fix16961_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/doctor/fix16961_test.go b/modules/doctor/fix16961_test.go index f0225610190a1..017f585335c5f 100644 --- a/modules/doctor/fix16961_test.go +++ b/modules/doctor/fix16961_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file.