From 85eada28c0aecc28af8725368a66bafa239692a7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 26 Apr 2025 09:50:45 -0700 Subject: [PATCH 1/2] Explicitly not update indexes when sync database schemas (#34281) Fix #34275 --------- Co-authored-by: wxiaoguang --- models/migrations/v1_23/v299.go | 6 ++- models/migrations/v1_23/v300.go | 6 ++- models/migrations/v1_23/v301.go | 6 ++- models/migrations/v1_23/v303.go | 6 ++- models/migrations/v1_23/v306.go | 6 ++- models/migrations/v1_23/v310.go | 6 ++- models/migrations/v1_23/v311.go | 7 ++- tests/integration/pull_status_test.go | 74 +++++++++++++++++++++++++++ 8 files changed, 109 insertions(+), 8 deletions(-) diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go index f6db960c3b41b..e5fde3749b6ce 100644 --- a/models/migrations/v1_23/v299.go +++ b/models/migrations/v1_23/v299.go @@ -14,5 +14,9 @@ func AddContentVersionToIssueAndComment(x *xorm.Engine) error { ContentVersion int `xorm:"NOT NULL DEFAULT 0"` } - return x.Sync(new(Comment), new(Issue)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(Comment), new(Issue)) + return err } diff --git a/models/migrations/v1_23/v300.go b/models/migrations/v1_23/v300.go index f1f1cccdbfb5e..51de43da5e6da 100644 --- a/models/migrations/v1_23/v300.go +++ b/models/migrations/v1_23/v300.go @@ -13,5 +13,9 @@ func AddForcePushBranchProtection(x *xorm.Engine) error { ForcePushAllowlistTeamIDs []int64 `xorm:"JSON TEXT"` ForcePushAllowlistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` } - return x.Sync(new(ProtectedBranch)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(ProtectedBranch)) + return err } diff --git a/models/migrations/v1_23/v301.go b/models/migrations/v1_23/v301.go index b7797f6c6b5eb..99c8e3d8eac2f 100644 --- a/models/migrations/v1_23/v301.go +++ b/models/migrations/v1_23/v301.go @@ -10,5 +10,9 @@ func AddSkipSecondaryAuthColumnToOAuth2ApplicationTable(x *xorm.Engine) error { type oauth2Application struct { SkipSecondaryAuthorization bool `xorm:"NOT NULL DEFAULT FALSE"` } - return x.Sync(new(oauth2Application)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(oauth2Application)) + return err } diff --git a/models/migrations/v1_23/v303.go b/models/migrations/v1_23/v303.go index adfe917d3f241..1e3638893021b 100644 --- a/models/migrations/v1_23/v303.go +++ b/models/migrations/v1_23/v303.go @@ -19,5 +19,9 @@ func AddCommentMetaDataColumn(x *xorm.Engine) error { CommentMetaData *CommentMetaData `xorm:"JSON TEXT"` // put all non-index metadata in a single field } - return x.Sync(new(Comment)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(Comment)) + return err } diff --git a/models/migrations/v1_23/v306.go b/models/migrations/v1_23/v306.go index 276b438e95bcf..a1e698fe31f03 100644 --- a/models/migrations/v1_23/v306.go +++ b/models/migrations/v1_23/v306.go @@ -9,5 +9,9 @@ func AddBlockAdminMergeOverrideBranchProtection(x *xorm.Engine) error { type ProtectedBranch struct { BlockAdminMergeOverride bool `xorm:"NOT NULL DEFAULT false"` } - return x.Sync(new(ProtectedBranch)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(ProtectedBranch)) + return err } diff --git a/models/migrations/v1_23/v310.go b/models/migrations/v1_23/v310.go index 394417f5a01b0..c856a708f9175 100644 --- a/models/migrations/v1_23/v310.go +++ b/models/migrations/v1_23/v310.go @@ -12,5 +12,9 @@ func AddPriorityToProtectedBranch(x *xorm.Engine) error { Priority int64 `xorm:"NOT NULL DEFAULT 0"` } - return x.Sync(new(ProtectedBranch)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(ProtectedBranch)) + return err } diff --git a/models/migrations/v1_23/v311.go b/models/migrations/v1_23/v311.go index 0fc1ac8c0e85c..21293d83be046 100644 --- a/models/migrations/v1_23/v311.go +++ b/models/migrations/v1_23/v311.go @@ -11,6 +11,9 @@ func AddTimeEstimateColumnToIssueTable(x *xorm.Engine) error { type Issue struct { TimeEstimate int64 `xorm:"NOT NULL DEFAULT 0"` } - - return x.Sync(new(Issue)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(Issue)) + return err } diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index ac9036ca962f9..dd147d8a9cd16 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -165,3 +165,77 @@ func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) { assert.Contains(t, text, "This branch is already included in the target branch. There is nothing to merge.") }) } +<<<<<<< HEAD +======= + +func TestPullStatusDelayCheck(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + defer test.MockVariableValue(&setting.Repository.PullRequest.DelayCheckForInactiveDays, 1)() + defer test.MockVariableValue(&pull.AddPullRequestToCheckQueue)() + + session := loginUser(t, "user2") + + run := func(t *testing.T, fn func(*testing.T)) (issue3 *issues.Issue, checkedPrID int64) { + pull.AddPullRequestToCheckQueue = func(prID int64) { + checkedPrID = prID + } + fn(t) + issue3 = unittest.AssertExistsAndLoadBean(t, &issues.Issue{RepoID: 1, Index: 3}) + _ = issue3.LoadPullRequest(t.Context()) + return issue3, checkedPrID + } + + assertReloadingInterval := func(t *testing.T, interval string) { + req := NewRequest(t, "GET", "/user2/repo1/pulls/3") + resp := session.MakeRequest(t, req, http.StatusOK) + attr := "data-pull-merge-box-reloading-interval" + if interval == "" { + assert.NotContains(t, resp.Body.String(), attr) + } else { + assert.Contains(t, resp.Body.String(), fmt.Sprintf(`%s="%v"`, attr, interval)) + } + } + + // PR issue3 is merageable at the beginning + issue3, checkedPrID := run(t, func(t *testing.T) {}) + assert.Equal(t, issues.PullRequestStatusMergeable, issue3.PullRequest.Status) + assert.Zero(t, checkedPrID) + assertReloadingInterval(t, "") // the PR is mergeable, so no need to reload the merge box + + // setting.IsProd = false // it would cause data-race because the queue handlers might be running and reading its value + // assertReloadingInterval(t, "1") // make sure dev mode always do merge box reloading, to make sure the UI logic won't break + // setting.IsProd = true + + // when base branch changes, PR status should be updated, but it is inactive for long time, so no real check + issue3, checkedPrID = run(t, func(t *testing.T) { + testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 1") + }) + assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Zero(t, checkedPrID) + assertReloadingInterval(t, "2000") // the PR status is "checking", so try to reload the merge box + + // view a PR with status=checking, it starts the real check + issue3, checkedPrID = run(t, func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo1/pulls/3") + session.MakeRequest(t, req, http.StatusOK) + }) + assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Equal(t, issue3.PullRequest.ID, checkedPrID) + + // when base branch changes, still so no real check + issue3, checkedPrID = run(t, func(t *testing.T) { + testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 2") + }) + assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Zero(t, checkedPrID) + + // then allow to check PRs without delay, when base branch changes, the PRs will be checked + setting.Repository.PullRequest.DelayCheckForInactiveDays = -1 + issue3, checkedPrID = run(t, func(t *testing.T) { + testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 3") + }) + assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Equal(t, issue3.PullRequest.ID, checkedPrID) + }) +} +>>>>>>> 44ece1e6f3 (Explicitly not update indexes when sync database schemas (#34281)) From ef9e9f1632b1e336b2791353f2e20d7b18440880 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 27 Apr 2025 12:11:16 -0700 Subject: [PATCH 2/2] Fix wrong code --- tests/integration/pull_status_test.go | 74 --------------------------- 1 file changed, 74 deletions(-) diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index dd147d8a9cd16..ac9036ca962f9 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -165,77 +165,3 @@ func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) { assert.Contains(t, text, "This branch is already included in the target branch. There is nothing to merge.") }) } -<<<<<<< HEAD -======= - -func TestPullStatusDelayCheck(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - defer test.MockVariableValue(&setting.Repository.PullRequest.DelayCheckForInactiveDays, 1)() - defer test.MockVariableValue(&pull.AddPullRequestToCheckQueue)() - - session := loginUser(t, "user2") - - run := func(t *testing.T, fn func(*testing.T)) (issue3 *issues.Issue, checkedPrID int64) { - pull.AddPullRequestToCheckQueue = func(prID int64) { - checkedPrID = prID - } - fn(t) - issue3 = unittest.AssertExistsAndLoadBean(t, &issues.Issue{RepoID: 1, Index: 3}) - _ = issue3.LoadPullRequest(t.Context()) - return issue3, checkedPrID - } - - assertReloadingInterval := func(t *testing.T, interval string) { - req := NewRequest(t, "GET", "/user2/repo1/pulls/3") - resp := session.MakeRequest(t, req, http.StatusOK) - attr := "data-pull-merge-box-reloading-interval" - if interval == "" { - assert.NotContains(t, resp.Body.String(), attr) - } else { - assert.Contains(t, resp.Body.String(), fmt.Sprintf(`%s="%v"`, attr, interval)) - } - } - - // PR issue3 is merageable at the beginning - issue3, checkedPrID := run(t, func(t *testing.T) {}) - assert.Equal(t, issues.PullRequestStatusMergeable, issue3.PullRequest.Status) - assert.Zero(t, checkedPrID) - assertReloadingInterval(t, "") // the PR is mergeable, so no need to reload the merge box - - // setting.IsProd = false // it would cause data-race because the queue handlers might be running and reading its value - // assertReloadingInterval(t, "1") // make sure dev mode always do merge box reloading, to make sure the UI logic won't break - // setting.IsProd = true - - // when base branch changes, PR status should be updated, but it is inactive for long time, so no real check - issue3, checkedPrID = run(t, func(t *testing.T) { - testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 1") - }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) - assert.Zero(t, checkedPrID) - assertReloadingInterval(t, "2000") // the PR status is "checking", so try to reload the merge box - - // view a PR with status=checking, it starts the real check - issue3, checkedPrID = run(t, func(t *testing.T) { - req := NewRequest(t, "GET", "/user2/repo1/pulls/3") - session.MakeRequest(t, req, http.StatusOK) - }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) - assert.Equal(t, issue3.PullRequest.ID, checkedPrID) - - // when base branch changes, still so no real check - issue3, checkedPrID = run(t, func(t *testing.T) { - testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 2") - }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) - assert.Zero(t, checkedPrID) - - // then allow to check PRs without delay, when base branch changes, the PRs will be checked - setting.Repository.PullRequest.DelayCheckForInactiveDays = -1 - issue3, checkedPrID = run(t, func(t *testing.T) { - testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 3") - }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) - assert.Equal(t, issue3.PullRequest.ID, checkedPrID) - }) -} ->>>>>>> 44ece1e6f3 (Explicitly not update indexes when sync database schemas (#34281))