From a35714372d9e6be2cf92ce27ef8b05b37f8cb283 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 10 Jan 2023 01:19:19 +0800 Subject: [PATCH 1/3] Fix halfCommitter and WithTx (#22366) Related to #22362. I overlooked that there's always `committer.Close()`, like: ```go ctx, committer, err := db.TxContext(db.DefaultContext) if err != nil { return nil } defer committer.Close() // ... if err != nil { return nil } // ... return committer.Commit() ``` So the `Close` of `halfCommitter` should ignore `commit and close`, it's not a rollback. See: [Why `halfCommitter` and `WithTx` should rollback IMMEDIATELY or commit LATER](https://github.com/go-gitea/gitea/pull/22366#issuecomment-1374778612). Co-authored-by: techknowlogick --- models/db/context.go | 27 ++++++-- models/db/context_committer_test.go | 102 ++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 models/db/context_committer_test.go diff --git a/models/db/context.go b/models/db/context.go index 455f3d1c5de..911dbd1c6f7 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -98,19 +98,31 @@ type Committer interface { // halfCommitter is a wrapper of Committer. // It can be closed early, but can't be committed early, it is useful for reusing a transaction. type halfCommitter struct { - Committer + committer Committer + committed bool } -func (*halfCommitter) Commit() error { - // do nothing +func (c *halfCommitter) Commit() error { + c.committed = true + // should do nothing, and the parent committer will commit later return nil } +func (c *halfCommitter) Close() error { + if c.committed { + // it's "commit and close", should do nothing, and the parent committer will commit later + return nil + } + + // it's "rollback and close", let the parent committer rollback right now + return c.committer.Close() +} + // TxContext represents a transaction Context, // it will reuse the existing transaction in the parent context or create a new one. func TxContext(parentCtx context.Context) (*Context, Committer, error) { if sess, ok := inTransaction(parentCtx); ok { - return newContext(parentCtx, sess, true), &halfCommitter{Committer: sess}, nil + return newContext(parentCtx, sess, true), &halfCommitter{committer: sess}, nil } sess := x.NewSession() @@ -126,7 +138,12 @@ func TxContext(parentCtx context.Context) (*Context, Committer, error) { // this function will reuse it otherwise will create a new one and close it when finished. func WithTx(parentCtx context.Context, f func(ctx context.Context) error) error { if sess, ok := inTransaction(parentCtx); ok { - return f(newContext(parentCtx, sess, true)) + err := f(newContext(parentCtx, sess, true)) + if err != nil { + // rollback immediately, in case the caller ignores returned error and tries to commit the transaction. + _ = sess.Close() + } + return err } return txWithNoCheck(parentCtx, f) } diff --git a/models/db/context_committer_test.go b/models/db/context_committer_test.go new file mode 100644 index 00000000000..38e91f22edb --- /dev/null +++ b/models/db/context_committer_test.go @@ -0,0 +1,102 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package db // it's not db_test, because this file is for testing the private type halfCommitter + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +type MockCommitter struct { + wants []string + gots []string +} + +func NewMockCommitter(wants ...string) *MockCommitter { + return &MockCommitter{ + wants: wants, + } +} + +func (c *MockCommitter) Commit() error { + c.gots = append(c.gots, "commit") + return nil +} + +func (c *MockCommitter) Close() error { + c.gots = append(c.gots, "close") + return nil +} + +func (c *MockCommitter) Assert(t *testing.T) { + assert.Equal(t, c.wants, c.gots, "want operations %v, but got %v", c.wants, c.gots) +} + +func Test_halfCommitter(t *testing.T) { + /* + Do something like: + + ctx, committer, err := db.TxContext(db.DefaultContext) + if err != nil { + return nil + } + defer committer.Close() + + // ... + + if err != nil { + return nil + } + + // ... + + return committer.Commit() + */ + + testWithCommitter := func(committer Committer, f func(committer Committer) error) { + if err := f(&halfCommitter{committer: committer}); err == nil { + committer.Commit() + } + committer.Close() + } + + t.Run("commit and close", func(t *testing.T) { + mockCommitter := NewMockCommitter("commit", "close") + + testWithCommitter(mockCommitter, func(committer Committer) error { + defer committer.Close() + return committer.Commit() + }) + + mockCommitter.Assert(t) + }) + + t.Run("rollback and close", func(t *testing.T) { + mockCommitter := NewMockCommitter("close", "close") + + testWithCommitter(mockCommitter, func(committer Committer) error { + defer committer.Close() + if true { + return fmt.Errorf("error") + } + return committer.Commit() + }) + + mockCommitter.Assert(t) + }) + + t.Run("close and commit", func(t *testing.T) { + mockCommitter := NewMockCommitter("close", "close") + + testWithCommitter(mockCommitter, func(committer Committer) error { + committer.Close() + committer.Commit() + return fmt.Errorf("error") + }) + + mockCommitter.Assert(t) + }) +} From 50f67d7e38581fbecc8d36605d5607239e11d305 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 10 Jan 2023 10:53:11 +0800 Subject: [PATCH 2/3] Don't display stop watch top bar icon when disabled and hidden when click other place (#22374) Fix #22286 When timetracking is disabled, the stop watch top bar icon should be hidden. When the stop watch recording popup, it should be allowed to hide with some operation. Now click any place on this page will hide the popup window. --- templates/base/head_navbar.tmpl | 2 ++ web_src/js/features/stopwatch.js | 1 + 2 files changed, 3 insertions(+) diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index 4fc61cf3690..3c4670e4186 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -77,6 +77,7 @@ {{else if .IsSigned}}
+ {{if EnableTimetracking}} @@ -115,6 +116,7 @@
+ {{end}} diff --git a/web_src/js/features/stopwatch.js b/web_src/js/features/stopwatch.js index b9042fae4cc..33915a1d837 100644 --- a/web_src/js/features/stopwatch.js +++ b/web_src/js/features/stopwatch.js @@ -24,6 +24,7 @@ export function initStopwatch() { trigger: 'click', maxWidth: 'none', interactive: true, + hideOnClick: true, }); // global stop watch (in the head_navbar), it should always work in any case either the EventSource or the PeriodicPoller is used. From 3b804ff76cdafa2ba9fce559269116aeb25a84c8 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Tue, 10 Jan 2023 11:43:54 +0200 Subject: [PATCH 3/3] Fix "remember this device" case (#22388) In the title case, it should be "Remember This Device" Signed-off-by: Yarden Shoham --- options/locale/locale_en-US.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ddc0ee25d51..a7ed7706407 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -302,7 +302,7 @@ social_register_helper_msg = Already have an account? Link it now! disable_register_prompt = Registration is disabled. Please contact your site administrator. disable_register_mail = Email confirmation for registration is disabled. manual_activation_only = Contact your site administrator to complete activation. -remember_me = Remember this Device +remember_me = Remember This Device forgot_password_title= Forgot Password forgot_password = Forgot password? sign_up_now = Need an account? Register now.