diff --git a/.deadcode-out b/.deadcode-out index 940551da04..5e6f81780f 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -66,6 +66,7 @@ package "code.gitea.io/gitea/models/migrations/base" func MainTest package "code.gitea.io/gitea/models/organization" + func GetTeamNamesByID func UpdateTeamUnits func (SearchMembersOptions).ToConds func UsersInTeamsCount @@ -131,6 +132,7 @@ package "code.gitea.io/gitea/models/user" func GetUserAllSettings func DeleteUserSetting func GetUserEmailsByNames + func GetUserNamesByIDs package "code.gitea.io/gitea/modules/activitypub" func CurrentTime diff --git a/Makefile b/Makefile index fec96de982..fe989d2add 100644 --- a/Makefile +++ b/Makefile @@ -121,7 +121,6 @@ LDFLAGS := $(LDFLAGS) -X "main.ReleaseVersion=$(RELEASE_VERSION)" -X "main.MakeV LINUX_ARCHS ?= linux/amd64,linux/386,linux/arm-5,linux/arm-6,linux/arm64 ifeq ($(HAS_GO), yes) - GO_PACKAGES ?= $(filter-out code.gitea.io/gitea/tests/integration/migration-test code.gitea.io/gitea/tests code.gitea.io/gitea/tests/integration code.gitea.io/gitea/tests/e2e,$(shell $(GO) list ./... | grep -v /vendor/)) GO_TEST_PACKAGES ?= $(filter-out $(shell $(GO) list code.gitea.io/gitea/models/migrations/...) $(shell $(GO) list code.gitea.io/gitea/models/forgejo_migrations/...) code.gitea.io/gitea/tests/integration/migration-test code.gitea.io/gitea/tests code.gitea.io/gitea/tests/integration code.gitea.io/gitea/tests/e2e,$(shell $(GO) list ./... | grep -v /vendor/)) endif @@ -457,7 +456,7 @@ lint-go-windows: .PHONY: lint-go-vet lint-go-vet: @echo "Running go vet..." - @$(GO) vet $(GO_PACKAGES) + @$(GO) vet ./... .PHONY: lint-editorconfig lint-editorconfig: @@ -823,7 +822,7 @@ generate-backend: $(TAGS_PREREQ) generate-go .PHONY: generate-go generate-go: $(TAGS_PREREQ) @echo "Running go generate..." - @CC= GOOS= GOARCH= $(GO) generate -tags '$(TAGS)' $(GO_PACKAGES) + @CC= GOOS= GOARCH= $(GO) generate -tags '$(TAGS)' ./... .PHONY: merge-locales merge-locales: diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go index 824d66d112..bd9063a8e4 100644 --- a/cmd/admin_user_change_password.go +++ b/cmd/admin_user_change_password.go @@ -36,6 +36,7 @@ var microcmdUserChangePassword = &cli.Command{ &cli.BoolFlag{ Name: "must-change-password", Usage: "User must change password", + Value: true, }, }, } @@ -57,23 +58,18 @@ func runChangePassword(c *cli.Context) error { return err } - var mustChangePassword optional.Option[bool] - if c.IsSet("must-change-password") { - mustChangePassword = optional.Some(c.Bool("must-change-password")) - } - opts := &user_service.UpdateAuthOptions{ Password: optional.Some(c.String("password")), - MustChangePassword: mustChangePassword, + MustChangePassword: optional.Some(c.Bool("must-change-password")), } if err := user_service.UpdateAuth(ctx, user, opts); err != nil { switch { case errors.Is(err, password.ErrMinLength): - return fmt.Errorf("Password is not long enough. Needs to be at least %d", setting.MinPasswordLength) + return fmt.Errorf("password is not long enough, needs to be at least %d characters", setting.MinPasswordLength) case errors.Is(err, password.ErrComplexity): - return errors.New("Password does not meet complexity requirements") + return errors.New("password does not meet complexity requirements") case errors.Is(err, password.ErrIsPwned): - return errors.New("The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password.\nFor more details, see https://haveibeenpwned.com/Passwords") + return errors.New("the password is in a list of stolen passwords previously exposed in public data breaches, please try again with a different password, to see more details: https://haveibeenpwned.com/Passwords") default: return err } diff --git a/cmd/admin_user_create.go b/cmd/admin_user_create.go index 10965c7e8f..caafef536c 100644 --- a/cmd/admin_user_create.go +++ b/cmd/admin_user_create.go @@ -8,6 +8,7 @@ import ( "fmt" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" pwd "code.gitea.io/gitea/modules/auth/password" "code.gitea.io/gitea/modules/optional" @@ -46,9 +47,10 @@ var microcmdUserCreate = &cli.Command{ Usage: "Generate a random password for the user", }, &cli.BoolFlag{ - Name: "must-change-password", - Usage: "Set this option to false to prevent forcing the user to change their password after initial login", - Value: true, + Name: "must-change-password", + Usage: "Set this option to false to prevent forcing the user to change their password after initial login", + Value: true, + DisableDefaultText: true, }, &cli.IntFlag{ Name: "random-password-length", @@ -72,10 +74,10 @@ func runCreateUser(c *cli.Context) error { } if c.IsSet("name") && c.IsSet("username") { - return errors.New("Cannot set both --name and --username flags") + return errors.New("cannot set both --name and --username flags") } if !c.IsSet("name") && !c.IsSet("username") { - return errors.New("One of --name or --username flags must be set") + return errors.New("one of --name or --username flags must be set") } if c.IsSet("password") && c.IsSet("random-password") { @@ -111,12 +113,21 @@ func runCreateUser(c *cli.Context) error { return errors.New("must set either password or random-password flag") } - changePassword := c.Bool("must-change-password") - - // If this is the first user being created. - // Take it as the admin and don't force a password update. - if n := user_model.CountUsers(ctx, nil); n == 0 { - changePassword = false + isAdmin := c.Bool("admin") + mustChangePassword := true // always default to true + if c.IsSet("must-change-password") { + // if the flag is set, use the value provided by the user + mustChangePassword = c.Bool("must-change-password") + } else { + // check whether there are users in the database + hasUserRecord, err := db.IsTableNotEmpty(&user_model.User{}) + if err != nil { + return fmt.Errorf("IsTableNotEmpty: %w", err) + } + if !hasUserRecord && isAdmin { + // if this is the first admin being created, don't force to change password (keep the old behavior) + mustChangePassword = false + } } restricted := optional.None[bool]() @@ -132,8 +143,8 @@ func runCreateUser(c *cli.Context) error { Name: username, Email: c.String("email"), Passwd: password, - IsAdmin: c.Bool("admin"), - MustChangePassword: changePassword, + IsAdmin: isAdmin, + MustChangePassword: mustChangePassword, Visibility: visibility, } diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 74999b5bb3..72dcaddb60 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2387,22 +2387,6 @@ LEVEL = Info ;; Enable issue by repository metrics; default is false ;ENABLED_ISSUE_BY_REPOSITORY = false -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;[task] -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; -;; Task queue type, could be `channel` or `redis`. -;QUEUE_TYPE = channel -;; -;; Task queue length, available only when `QUEUE_TYPE` is `channel`. -;QUEUE_LENGTH = 1000 -;; -;; Task queue connection string, available only when `QUEUE_TYPE` is `redis`. -;; If there is a password of redis, use `redis://127.0.0.1:6379/0?pool_size=100&idle_timeout=180s` or `redis+cluster://127.0.0.1:6379/0?pool_size=100&idle_timeout=180s` for `redis-clsuter`. -;QUEUE_CONN_STR = "redis://127.0.0.1:6379/0?pool_size=100&idle_timeout=180s" - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;[migrations] diff --git a/models/db/engine.go b/models/db/engine.go index 27e5fb9e1a..b3a4171e3f 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -296,8 +296,8 @@ func MaxBatchInsertSize(bean any) int { } // IsTableNotEmpty returns true if table has at least one record -func IsTableNotEmpty(tableName string) (bool, error) { - return x.Table(tableName).Exist() +func IsTableNotEmpty(beanOrTableName any) (bool, error) { + return x.Table(beanOrTableName).Exist() } // DeleteAllRecords will delete all the records of this table diff --git a/models/packages/package_version.go b/models/packages/package_version.go index 505dbaa0a5..278e8e3a86 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -287,9 +287,10 @@ func (opts *PackageSearchOptions) configureOrderBy(e db.Engine) { // SearchVersions gets all versions of packages matching the search options func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { sess := db.GetEngine(ctx). - Where(opts.ToConds()). + Select("package_version.*"). Table("package_version"). - Join("INNER", "package", "package.id = package_version.package_id") + Join("INNER", "package", "package.id = package_version.package_id"). + Where(opts.ToConds()) opts.configureOrderBy(sess) @@ -304,19 +305,18 @@ func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*Package // SearchLatestVersions gets the latest version of every package matching the search options func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { - cond := opts.ToConds(). - And(builder.Expr("pv2.id IS NULL")) - - joinCond := builder.Expr("package_version.package_id = pv2.package_id AND (package_version.created_unix < pv2.created_unix OR (package_version.created_unix = pv2.created_unix AND package_version.id < pv2.id))") - if opts.IsInternal.Has() { - joinCond = joinCond.And(builder.Eq{"pv2.is_internal": opts.IsInternal.Value()}) - } + in := builder. + Select("MAX(package_version.id)"). + From("package_version"). + InnerJoin("package", "package.id = package_version.package_id"). + Where(opts.ToConds()). + GroupBy("package_version.package_id") sess := db.GetEngine(ctx). + Select("package_version.*"). Table("package_version"). - Join("LEFT", "package_version pv2", joinCond). Join("INNER", "package", "package.id = package_version.package_id"). - Where(cond) + Where(builder.In("package_version.id", in)) opts.configureOrderBy(sess) diff --git a/modules/session/store.go b/modules/session/store.go index 4fa4d2848f..70988fcdc5 100644 --- a/modules/session/store.go +++ b/modules/session/store.go @@ -18,6 +18,12 @@ type Store interface { // RegenerateSession regenerates the underlying session and returns the new store func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) { + for _, f := range BeforeRegenerateSession { + f(resp, req) + } s, err := session.RegenerateSession(resp, req) return s, err } + +// BeforeRegenerateSession is a list of functions that are called before a session is regenerated. +var BeforeRegenerateSession []func(http.ResponseWriter, *http.Request) diff --git a/modules/web/middleware/cookie.go b/modules/web/middleware/cookie.go index 621640895b..ec6b06f993 100644 --- a/modules/web/middleware/cookie.go +++ b/modules/web/middleware/cookie.go @@ -9,6 +9,7 @@ import ( "net/url" "strings" + "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" ) @@ -45,10 +46,40 @@ func SetSiteCookie(resp http.ResponseWriter, name, value string, maxAge int) { SameSite: setting.SessionConfig.SameSite, } resp.Header().Add("Set-Cookie", cookie.String()) - if maxAge < 0 { - // There was a bug in "setting.SessionConfig.CookiePath" code, the old default value of it was empty "". - // So we have to delete the cookie on path="" again, because some old code leaves cookies on path="". - cookie.Path = strings.TrimSuffix(setting.SessionConfig.CookiePath, "/") - resp.Header().Add("Set-Cookie", cookie.String()) - } + // Previous versions would use a cookie path with a trailing /. + // These are more specific than cookies without a trailing /, so + // we need to delete these if they exist. + deleteLegacySiteCookie(resp, name) +} + +// deleteLegacySiteCookie deletes the cookie with the given name at the cookie +// path with a trailing /, which would unintentionally override the cookie. +func deleteLegacySiteCookie(resp http.ResponseWriter, name string) { + if setting.SessionConfig.CookiePath == "" || strings.HasSuffix(setting.SessionConfig.CookiePath, "/") { + // If the cookie path ends with /, no legacy cookies will take + // precedence, so do nothing. The exception is that cookies with no + // path could override other cookies, but it's complicated and we don't + // currently handle that. + return + } + + cookie := &http.Cookie{ + Name: name, + Value: "", + MaxAge: -1, + Path: setting.SessionConfig.CookiePath + "/", + Domain: setting.SessionConfig.Domain, + Secure: setting.SessionConfig.Secure, + HttpOnly: true, + SameSite: setting.SessionConfig.SameSite, + } + resp.Header().Add("Set-Cookie", cookie.String()) +} + +func init() { + session.BeforeRegenerateSession = append(session.BeforeRegenerateSession, func(resp http.ResponseWriter, _ *http.Request) { + // Ensure that a cookie with a trailing slash does not take precedence over + // the cookie written by the middleware. + deleteLegacySiteCookie(resp, setting.SessionConfig.CookieName) + }) } diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 12da8a9597..98c9a2697f 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -30,7 +30,7 @@ import ( user_service "code.gitea.io/gitea/services/user" ) -func parseAuthSource(ctx *context.APIContext, u *user_model.User, sourceID int64, loginName string) { +func parseAuthSource(ctx *context.APIContext, u *user_model.User, sourceID int64) { if sourceID == 0 { return } @@ -47,7 +47,6 @@ func parseAuthSource(ctx *context.APIContext, u *user_model.User, sourceID int64 u.LoginType = source.Type u.LoginSource = source.ID - u.LoginName = loginName } // CreateUser create a user @@ -83,12 +82,13 @@ func CreateUser(ctx *context.APIContext) { Passwd: form.Password, MustChangePassword: true, LoginType: auth.Plain, + LoginName: form.LoginName, } if form.MustChangePassword != nil { u.MustChangePassword = *form.MustChangePassword } - parseAuthSource(ctx, u, form.SourceID, form.LoginName) + parseAuthSource(ctx, u, form.SourceID) if ctx.Written() { return } diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index c33beee0ae..852b7a2ee0 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -437,7 +437,7 @@ func GetBranchProtection(ctx *context.APIContext) { return } - ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp)) + ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp, repo)) } // ListBranchProtections list branch protections for a repo @@ -470,7 +470,7 @@ func ListBranchProtections(ctx *context.APIContext) { } apiBps := make([]*api.BranchProtection, len(bps)) for i := range bps { - apiBps[i] = convert.ToBranchProtection(ctx, bps[i]) + apiBps[i] = convert.ToBranchProtection(ctx, bps[i], repo) } ctx.JSON(http.StatusOK, apiBps) @@ -682,7 +682,7 @@ func CreateBranchProtection(ctx *context.APIContext) { return } - ctx.JSON(http.StatusCreated, convert.ToBranchProtection(ctx, bp)) + ctx.JSON(http.StatusCreated, convert.ToBranchProtection(ctx, bp, repo)) } // EditBranchProtection edits a branch protection for a repo @@ -964,7 +964,7 @@ func EditBranchProtection(ctx *context.APIContext) { return } - ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp)) + ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp, repo)) } // DeleteBranchProtection deletes a branch protection for a repo diff --git a/routers/web/web.go b/routers/web/web.go index 40f4ffc018..1d085a37cb 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -258,7 +258,7 @@ func Routes() *web.Route { routes.Get("/metrics", append(mid, Metrics)...) } - routes.Get("/robots.txt", append(mid, misc.RobotsTxt)...) + routes.Methods("GET,HEAD", "/robots.txt", append(mid, misc.RobotsTxt)...) routes.Get("/ssh_info", misc.SSHInfo) routes.Get("/api/healthz", healthcheck.Check) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index ef37ff87ee..365212d9c2 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -80,6 +80,11 @@ func newNotifyInput(repo *repo_model.Repository, doer *user_model.User, event we } } +func newNotifyInputForSchedules(repo *repo_model.Repository) *notifyInput { + // the doer here will be ignored as we force using action user when handling schedules + return newNotifyInput(repo, user_model.NewActionsUser(), webhook_module.HookEventSchedule) +} + func (input *notifyInput) WithDoer(doer *user_model.User) *notifyInput { input.Doer = doer return input @@ -562,7 +567,7 @@ func DetectAndHandleSchedules(ctx context.Context, repo *repo_model.Repository) // We need a notifyInput to call handleSchedules // if repo is a mirror, commit author maybe an external user, // so we use action user as the Doer of the notifyInput - notifyInput := newNotifyInput(repo, user_model.NewActionsUser(), webhook_module.HookEventSchedule) + notifyInput := newNotifyInputForSchedules(repo) return handleSchedules(ctx, scheduleWorkflows, commit, notifyInput, repo.DefaultBranch) } diff --git a/services/auth/source/oauth2/store.go b/services/auth/source/oauth2/store.go index 394bf99463..90fa965602 100644 --- a/services/auth/source/oauth2/store.go +++ b/services/auth/source/oauth2/store.go @@ -9,6 +9,7 @@ import ( "net/http" "code.gitea.io/gitea/modules/log" + session_module "code.gitea.io/gitea/modules/session" chiSession "gitea.com/go-chi/session" "github.com/gorilla/sessions" @@ -65,7 +66,7 @@ func (st *SessionsStore) Save(r *http.Request, w http.ResponseWriter, session *s chiStore := chiSession.GetSession(r) if session.IsNew { - _, _ = chiSession.RegenerateSession(w, r) + _, _ = session_module.RegenerateSession(w, r) session.IsNew = false } diff --git a/services/convert/convert.go b/services/convert/convert.go index dd2239458e..70ca5da2ec 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -21,6 +21,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" @@ -105,33 +106,46 @@ func ToBranch(ctx context.Context, repo *repo_model.Repository, branchName strin return branch, nil } +// getWhitelistEntities returns the names of the entities that are in the whitelist +func getWhitelistEntities[T *user_model.User | *organization.Team](entities []T, whitelistIDs []int64) []string { + whitelistUserIDsSet := container.SetOf(whitelistIDs...) + whitelistNames := make([]string, 0) + for _, entity := range entities { + switch v := any(entity).(type) { + case *user_model.User: + if whitelistUserIDsSet.Contains(v.ID) { + whitelistNames = append(whitelistNames, v.Name) + } + case *organization.Team: + if whitelistUserIDsSet.Contains(v.ID) { + whitelistNames = append(whitelistNames, v.Name) + } + } + } + + return whitelistNames +} + // ToBranchProtection convert a ProtectedBranch to api.BranchProtection -func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch) *api.BranchProtection { - pushWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.WhitelistUserIDs) +func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo *repo_model.Repository) *api.BranchProtection { + readers, err := access_model.GetRepoReaders(ctx, repo) if err != nil { - log.Error("GetUserNamesByIDs (WhitelistUserIDs): %v", err) + log.Error("GetRepoReaders: %v", err) } - mergeWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.MergeWhitelistUserIDs) + + pushWhitelistUsernames := getWhitelistEntities(readers, bp.WhitelistUserIDs) + mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs) + approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs) + + teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead) if err != nil { - log.Error("GetUserNamesByIDs (MergeWhitelistUserIDs): %v", err) - } - approvalsWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.ApprovalsWhitelistUserIDs) - if err != nil { - log.Error("GetUserNamesByIDs (ApprovalsWhitelistUserIDs): %v", err) - } - pushWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.WhitelistTeamIDs) - if err != nil { - log.Error("GetTeamNamesByID (WhitelistTeamIDs): %v", err) - } - mergeWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.MergeWhitelistTeamIDs) - if err != nil { - log.Error("GetTeamNamesByID (MergeWhitelistTeamIDs): %v", err) - } - approvalsWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.ApprovalsWhitelistTeamIDs) - if err != nil { - log.Error("GetTeamNamesByID (ApprovalsWhitelistTeamIDs): %v", err) + log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err) } + pushWhitelistTeams := getWhitelistEntities(teamReaders, bp.WhitelistTeamIDs) + mergeWhitelistTeams := getWhitelistEntities(teamReaders, bp.MergeWhitelistTeamIDs) + approvalsWhitelistTeams := getWhitelistEntities(teamReaders, bp.ApprovalsWhitelistTeamIDs) + branchName := "" if !git_model.IsRuleNameSpecial(bp.RuleName) { branchName = bp.RuleName diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index 0bb738e2ad..992889c454 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/services/automerge" @@ -25,12 +26,41 @@ func getCacheKey(repoID int64, brancheName string) string { return fmt.Sprintf("commit_status:%x", hashBytes) } -func updateCommitStatusCache(ctx context.Context, repoID int64, branchName string, status api.CommitStatusState) error { - c := cache.GetCache() - return c.Put(getCacheKey(repoID, branchName), string(status), 3*24*60) +type commitStatusCacheValue struct { + State string `json:"state"` + TargetURL string `json:"target_url"` } -func deleteCommitStatusCache(ctx context.Context, repoID int64, branchName string) error { +func getCommitStatusCache(repoID int64, branchName string) *commitStatusCacheValue { + c := cache.GetCache() + statusStr, ok := c.Get(getCacheKey(repoID, branchName)).(string) + if ok && statusStr != "" { + var cv commitStatusCacheValue + err := json.Unmarshal([]byte(statusStr), &cv) + if err == nil && cv.State != "" { + return &cv + } + if err != nil { + log.Warn("getCommitStatusCache: json.Unmarshal failed: %v", err) + } + } + return nil +} + +func updateCommitStatusCache(repoID int64, branchName string, state api.CommitStatusState, targetURL string) error { + c := cache.GetCache() + bs, err := json.Marshal(commitStatusCacheValue{ + State: state.String(), + TargetURL: targetURL, + }) + if err != nil { + log.Warn("updateCommitStatusCache: json.Marshal failed: %v", err) + return nil + } + return c.Put(getCacheKey(repoID, branchName), string(bs), 3*24*60) +} + +func deleteCommitStatusCache(repoID int64, branchName string) error { c := cache.GetCache() return c.Delete(getCacheKey(repoID, branchName)) } @@ -74,7 +104,7 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato } if commit.ID.String() == defaultBranchCommit.ID.String() { // since one commit status updated, the combined commit status should be invalid - if err := deleteCommitStatusCache(ctx, repo.ID, repo.DefaultBranch); err != nil { + if err := deleteCommitStatusCache(repo.ID, repo.DefaultBranch); err != nil { log.Error("deleteCommitStatusCache[%d:%s] failed: %v", repo.ID, repo.DefaultBranch, err) } } @@ -91,12 +121,12 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato // FindReposLastestCommitStatuses loading repository default branch latest combinded commit status with cache func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Repository) ([]*git_model.CommitStatus, error) { results := make([]*git_model.CommitStatus, len(repos)) - c := cache.GetCache() - for i, repo := range repos { - status, ok := c.Get(getCacheKey(repo.ID, repo.DefaultBranch)).(string) - if ok && status != "" { - results[i] = &git_model.CommitStatus{State: api.CommitStatusState(status)} + if cv := getCommitStatusCache(repo.ID, repo.DefaultBranch); cv != nil { + results[i] = &git_model.CommitStatus{ + State: api.CommitStatusState(cv.State), + TargetURL: cv.TargetURL, + } } } @@ -123,8 +153,8 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep for i, repo := range repos { if results[i] == nil { results[i] = git_model.CalcCommitStatus(repoToItsLatestCommitStatuses[repo.ID]) - if results[i] != nil { - if err := updateCommitStatusCache(ctx, repo.ID, repo.DefaultBranch, results[i].State); err != nil { + if results[i] != nil && results[i].State != "" { + if err := updateCommitStatusCache(repo.ID, repo.DefaultBranch, results[i].State, results[i].TargetURL); err != nil { log.Error("updateCommitStatusCache[%d:%s] failed: %v", repo.ID, repo.DefaultBranch, err) } } diff --git a/templates/repo/actions/runs_list.tmpl b/templates/repo/actions/runs_list.tmpl index ac5049cf56..20330b5d62 100644 --- a/templates/repo/actions/runs_list.tmpl +++ b/templates/repo/actions/runs_list.tmpl @@ -1,4 +1,4 @@ -