diff --git a/cmd/cmd.go b/cmd/cmd.go index fd713b9affd95..3846a8690020f 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -31,7 +31,7 @@ func argsSet(c *cli.Context, args ...string) error { return errors.New(a + " is not set") } - if util.IsEmptyString(a) { + if util.IsEmptyString(c.String(a)) { return errors.New(a + " is required") } } diff --git a/docs/content/doc/developers/guidelines-backend.md b/docs/content/doc/developers/guidelines-backend.md index d249465453f61..1248d4143233f 100644 --- a/docs/content/doc/developers/guidelines-backend.md +++ b/docs/content/doc/developers/guidelines-backend.md @@ -42,7 +42,7 @@ To maintain understandable code and avoid circular dependencies it is important - `modules/setting`: Store all system configurations read from ini files and has been referenced by everywhere. But they should be used as function parameters when possible. - `modules/git`: Package to interactive with `Git` command line or Gogit package. - `public`: Compiled frontend files (javascript, images, css, etc.) -- `routers`: Handling of server requests. As it uses other Gitea packages to serve the request, other packages (models, modules or services) shall not depend on routers. +- `routers`: Handling of server requests. As it uses other Gitea packages to serve the request, other packages (models, modules or services) must not depend on routers. - `routers/api` Contains routers for `/api/v1` aims to handle RESTful API requests. - `routers/install` Could only respond when system is in INSTALL mode (INSTALL_LOCK=false). - `routers/private` will only be invoked by internal sub commands, especially `serv` and `hooks`. @@ -106,10 +106,20 @@ i.e. `servcies/user`, `models/repository`. Since there are some packages which use the same package name, it is possible that you find packages like `modules/user`, `models/user`, and `services/user`. When these packages are imported in one Go file, it's difficult to know which package we are using and if it's a variable name or an import name. So, we always recommend to use import aliases. To differ from package variables which are commonly in camelCase, just use **snake_case** for import aliases. i.e. `import user_service "code.gitea.io/gitea/services/user"` +### Important Gotchas + +- Never write `x.Update(exemplar)` without an explicit `WHERE` clause: + - This will cause all rows in the table to be updated with the non-zero values of the exemplar - including IDs. + - You should usually write `x.ID(id).Update(exemplar)`. +- If during a migration you are inserting into a table using `x.Insert(exemplar)` where the ID is preset: + - You will need to ``SET IDENTITY_INSERT `table` ON`` for the MSSQL variant (the migration will fail otherwise) + - However, you will also need to update the id sequence for postgres - the migration will silently pass here but later insertions will fail: + ``SELECT setval('table_name_id_seq', COALESCE((SELECT MAX(id)+1 FROM `table_name`), 1), false)`` + ### Future Tasks Currently, we are creating some refactors to do the following things: - Correct that codes which doesn't follow the rules. - There are too many files in `models`, so we are moving some of them into a sub package `models/xxx`. -- Some `modules` sub packages should be moved to `services` because they depends on `models`. \ No newline at end of file +- Some `modules` sub packages should be moved to `services` because they depend on `models`. diff --git a/models/action.go b/models/action.go index 26d05730c5f6d..f2723a20147cc 100644 --- a/models/action.go +++ b/models/action.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -315,19 +316,21 @@ func (a *Action) GetIssueContent() string { // GetFeedsOptions options for retrieving feeds type GetFeedsOptions struct { - RequestedUser *user_model.User // the user we want activity for - RequestedTeam *Team // the team we want activity for - Actor *user_model.User // the user viewing the activity - IncludePrivate bool // include private actions - OnlyPerformedBy bool // only actions performed by requested user - IncludeDeleted bool // include deleted actions - Date string // the day we want activity for: YYYY-MM-DD + db.ListOptions + RequestedUser *user_model.User // the user we want activity for + RequestedTeam *Team // the team we want activity for + RequestedRepo *repo_model.Repository // the repo we want activity for + Actor *user_model.User // the user viewing the activity + IncludePrivate bool // include private actions + OnlyPerformedBy bool // only actions performed by requested user + IncludeDeleted bool // include deleted actions + Date string // the day we want activity for: YYYY-MM-DD } // GetFeeds returns actions according to the provided options func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { - if !activityReadable(opts.RequestedUser, opts.Actor) { - return make([]*Action, 0), nil + if opts.RequestedUser == nil && opts.RequestedTeam == nil && opts.RequestedRepo == nil { + return nil, fmt.Errorf("need at least one of these filters: RequestedUser, RequestedTeam, RequestedRepo") } cond, err := activityQueryCondition(opts) @@ -335,9 +338,14 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { return nil, err } - actions := make([]*Action, 0, setting.UI.FeedPagingNum) + sess := db.GetEngine(db.DefaultContext).Where(cond) - if err := db.GetEngine(db.DefaultContext).Limit(setting.UI.FeedPagingNum).Desc("created_unix").Where(cond).Find(&actions); err != nil { + opts.SetDefaultValues() + sess = db.SetSessionPagination(sess, &opts) + + actions := make([]*Action, 0, opts.PageSize) + + if err := sess.Desc("created_unix").Find(&actions); err != nil { return nil, fmt.Errorf("Find: %v", err) } @@ -349,41 +357,44 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { } func activityReadable(user, doer *user_model.User) bool { - var doerID int64 - if doer != nil { - doerID = doer.ID - } - if doer == nil || !doer.IsAdmin { - if user.KeepActivityPrivate && doerID != user.ID { - return false - } - } - return true + return !user.KeepActivityPrivate || + doer != nil && (doer.IsAdmin || user.ID == doer.ID) } func activityQueryCondition(opts GetFeedsOptions) (builder.Cond, error) { cond := builder.NewCond() - var repoIDs []int64 - var actorID int64 - if opts.Actor != nil { - actorID = opts.Actor.ID + if opts.RequestedTeam != nil && opts.RequestedUser == nil { + org, err := user_model.GetUserByID(opts.RequestedTeam.OrgID) + if err != nil { + return nil, err + } + opts.RequestedUser = org + } + + // check activity visibility for actor ( similar to activityReadable() ) + if opts.Actor == nil { + cond = cond.And(builder.In("act_user_id", + builder.Select("`user`.id").Where( + builder.Eq{"keep_activity_private": false, "visibility": structs.VisibleTypePublic}, + ).From("`user`"), + )) + } else if !opts.Actor.IsAdmin { + cond = cond.And(builder.In("act_user_id", + builder.Select("`user`.id").Where( + builder.Eq{"keep_activity_private": false}. + And(builder.In("visibility", structs.VisibleTypePublic, structs.VisibleTypeLimited))). + Or(builder.Eq{"id": opts.Actor.ID}).From("`user`"), + )) } // check readable repositories by doer/actor if opts.Actor == nil || !opts.Actor.IsAdmin { - if opts.RequestedUser.IsOrganization() { - env, err := OrgFromUser(opts.RequestedUser).AccessibleReposEnv(actorID) - if err != nil { - return nil, fmt.Errorf("AccessibleReposEnv: %v", err) - } - if repoIDs, err = env.RepoIDs(1, opts.RequestedUser.NumRepos); err != nil { - return nil, fmt.Errorf("GetUserRepositories: %v", err) - } - cond = cond.And(builder.In("repo_id", repoIDs)) - } else { - cond = cond.And(builder.In("repo_id", AccessibleRepoIDsQuery(opts.Actor))) - } + cond = cond.And(builder.In("repo_id", AccessibleRepoIDsQuery(opts.Actor))) + } + + if opts.RequestedRepo != nil { + cond = cond.And(builder.Eq{"repo_id": opts.RequestedRepo.ID}) } if opts.RequestedTeam != nil { @@ -395,11 +406,14 @@ func activityQueryCondition(opts GetFeedsOptions) (builder.Cond, error) { cond = cond.And(builder.In("repo_id", teamRepoIDs)) } - cond = cond.And(builder.Eq{"user_id": opts.RequestedUser.ID}) + if opts.RequestedUser != nil { + cond = cond.And(builder.Eq{"user_id": opts.RequestedUser.ID}) - if opts.OnlyPerformedBy { - cond = cond.And(builder.Eq{"act_user_id": opts.RequestedUser.ID}) + if opts.OnlyPerformedBy { + cond = cond.And(builder.Eq{"act_user_id": opts.RequestedUser.ID}) + } } + if !opts.IncludePrivate { cond = cond.And(builder.Eq{"is_private": false}) } diff --git a/models/action_test.go b/models/action_test.go index 306d382364c44..0ce9183b9658e 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -93,6 +93,46 @@ func TestGetFeeds2(t *testing.T) { assert.Len(t, actions, 0) } +func TestActivityReadable(t *testing.T) { + tt := []struct { + desc string + user *user_model.User + doer *user_model.User + result bool + }{{ + desc: "user should see own activity", + user: &user_model.User{ID: 1}, + doer: &user_model.User{ID: 1}, + result: true, + }, { + desc: "anon should see activity if public", + user: &user_model.User{ID: 1}, + result: true, + }, { + desc: "anon should NOT see activity", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + result: false, + }, { + desc: "user should see own activity if private too", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + doer: &user_model.User{ID: 1}, + result: true, + }, { + desc: "other user should NOT see activity", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + doer: &user_model.User{ID: 2}, + result: false, + }, { + desc: "admin should see activity", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + doer: &user_model.User{ID: 2, IsAdmin: true}, + result: true, + }} + for _, test := range tt { + assert.Equal(t, test.result, activityReadable(test.user, test.doer), test.desc) + } +} + func TestNotifyWatchers(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/lfs.go b/models/lfs.go index e0c16767f71e6..037ed8556b758 100644 --- a/models/lfs.go +++ b/models/lfs.go @@ -193,12 +193,13 @@ func LFSAutoAssociate(metas []*LFSMetaObject, user *user_model.User, repoID int6 // admin can associate any LFS object to any repository, and we do not care about errors (eg: duplicated unique key), // even if error occurs, it won't hurt users and won't make things worse for i := range metas { + p := lfs.Pointer{Oid: metas[i].Oid, Size: metas[i].Size} _, err = sess.Insert(&LFSMetaObject{ - Pointer: lfs.Pointer{Oid: metas[i].Oid, Size: metas[i].Size}, + Pointer: p, RepositoryID: repoID, }) if err != nil { - log.Warn("failed to insert LFS meta object into database, err=%v", err) + log.Warn("failed to insert LFS meta object %-v for repo_id: %d into database, err=%v", p, repoID, err) } } } diff --git a/models/migrations/v210.go b/models/migrations/v210.go index dafe355fe211a..f32fae77bacc9 100644 --- a/models/migrations/v210.go +++ b/models/migrations/v210.go @@ -174,5 +174,11 @@ func remigrateU2FCredentials(x *xorm.Engine) error { regs = regs[:0] } + if x.Dialect().URI().DBType == schemas.POSTGRES { + if _, err := x.Exec("SELECT setval('webauthn_credential_id_seq', COALESCE((SELECT MAX(id)+1 FROM `webauthn_credential`), 1), false)"); err != nil { + return err + } + } + return nil } diff --git a/models/repo.go b/models/repo.go index 53199bcca355f..d20e5f81d31be 100644 --- a/models/repo.go +++ b/models/repo.go @@ -218,50 +218,44 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post return nil, err } - var users []*user_model.User - e := db.GetEngine(ctx) + cond := builder.And(builder.Neq{"`user`.id": posterID}) if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { // This a private repository: // Anyone who can read the repository is a requestable reviewer - if err := e. - SQL("SELECT * FROM `user` WHERE id in ("+ - "SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? AND user_id != ?"+ // private org repos - ") ORDER BY name", - repo.ID, perm.AccessModeRead, - posterID). - Find(&users); err != nil { - return nil, err - } + + cond = cond.And(builder.In("`user`.id", + builder.Select("user_id").From("access").Where( + builder.Eq{"repo_id": repo.ID}. + And(builder.Gte{"mode": perm.AccessModeRead}), + ), + )) if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID { // as private *user* repos don't generate an entry in the `access` table, // the owner of a private repo needs to be explicitly added. - users = append(users, repo.Owner) + cond = cond.Or(builder.Eq{"`user`.id": repo.Owner.ID}) } - return users, nil - } - - // This is a "public" repository: - // Any user that has read access, is a watcher or organization member can be requested to review - if err := e. - SQL("SELECT * FROM `user` WHERE id IN ( "+ - "SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? "+ - "UNION "+ - "SELECT user_id FROM `watch` WHERE repo_id = ? AND mode IN (?, ?) "+ - "UNION "+ - "SELECT uid AS user_id FROM `org_user` WHERE org_id = ? "+ - ") AND id != ? ORDER BY name", - repo.ID, perm.AccessModeRead, - repo.ID, repo_model.WatchModeNormal, repo_model.WatchModeAuto, - repo.OwnerID, - posterID). - Find(&users); err != nil { - return nil, err - } - - return users, nil + } else { + // This is a "public" repository: + // Any user that has read access, is a watcher or organization member can be requested to review + cond = cond.And(builder.And(builder.In("`user`.id", + builder.Select("user_id").From("access"). + Where(builder.Eq{"repo_id": repo.ID}. + And(builder.Gte{"mode": perm.AccessModeRead})), + ).Or(builder.In("`user`.id", + builder.Select("user_id").From("watch"). + Where(builder.Eq{"repo_id": repo.ID}. + And(builder.In("mode", repo_model.WatchModeNormal, repo_model.WatchModeAuto))), + ).Or(builder.In("`user`.id", + builder.Select("uid").From("org_user"). + Where(builder.Eq{"org_id": repo.OwnerID}), + ))))) + } + + users := make([]*user_model.User, 0, 8) + return users, db.GetEngine(ctx).Where(cond).OrderBy("name").Find(&users) } // GetReviewers get all users can be requested to review: diff --git a/models/user_heatmap_test.go b/models/user_heatmap_test.go index 7d2997648df4a..7915363d9574a 100644 --- a/models/user_heatmap_test.go +++ b/models/user_heatmap_test.go @@ -19,25 +19,40 @@ import ( func TestGetUserHeatmapDataByUser(t *testing.T) { testCases := []struct { + desc string userID int64 doerID int64 CountResult int JSONResult string }{ - // self looks at action in private repo - {2, 2, 1, `[{"timestamp":1603227600,"contributions":1}]`}, - // admin looks at action in private repo - {2, 1, 1, `[{"timestamp":1603227600,"contributions":1}]`}, - // other user looks at action in private repo - {2, 3, 0, `[]`}, - // nobody looks at action in private repo - {2, 0, 0, `[]`}, - // collaborator looks at action in private repo - {16, 15, 1, `[{"timestamp":1603267200,"contributions":1}]`}, - // no action action not performed by target user - {3, 3, 0, `[]`}, - // multiple actions performed with two grouped together - {10, 10, 3, `[{"timestamp":1603009800,"contributions":1},{"timestamp":1603010700,"contributions":2}]`}, + { + "self looks at action in private repo", + 2, 2, 1, `[{"timestamp":1603227600,"contributions":1}]`, + }, + { + "admin looks at action in private repo", + 2, 1, 1, `[{"timestamp":1603227600,"contributions":1}]`, + }, + { + "other user looks at action in private repo", + 2, 3, 0, `[]`, + }, + { + "nobody looks at action in private repo", + 2, 0, 0, `[]`, + }, + { + "collaborator looks at action in private repo", + 16, 15, 1, `[{"timestamp":1603267200,"contributions":1}]`, + }, + { + "no action action not performed by target user", + 3, 3, 0, `[]`, + }, + { + "multiple actions performed with two grouped together", + 10, 10, 3, `[{"timestamp":1603009800,"contributions":1},{"timestamp":1603010700,"contributions":2}]`, + }, } // Prepare assert.NoError(t, unittest.PrepareTestDatabase()) @@ -46,7 +61,7 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { timeutil.Set(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)) defer timeutil.Unset() - for i, tc := range testCases { + for _, tc := range testCases { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: tc.userID}).(*user_model.User) doer := &user_model.User{ID: tc.doerID} @@ -74,7 +89,7 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { } assert.NoError(t, err) assert.Len(t, actions, contributions, "invalid action count: did the test data became too old?") - assert.Equal(t, tc.CountResult, contributions, fmt.Sprintf("testcase %d", i)) + assert.Equal(t, tc.CountResult, contributions, fmt.Sprintf("testcase '%s'", tc.desc)) // Test JSON rendering jsonData, err := json.Marshal(heatmap) diff --git a/modules/context/context.go b/modules/context/context.go index 6aeeb9e694676..8e50e154a14d5 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -269,7 +269,7 @@ func (ctx *Context) ServerError(logMsg string, logErr error) { func (ctx *Context) serverErrorInternal(logMsg string, logErr error) { if logErr != nil { log.ErrorWithSkip(2, "%s: %v", logMsg, logErr) - if errors.Is(logErr, &net.OpError{}) { + if _, ok := logErr.(*net.OpError); ok || errors.Is(logErr, &net.OpError{}) { // This is an error within the underlying connection // and further rendering will not work so just return return diff --git a/modules/lfs/pointer.go b/modules/lfs/pointer.go index 975b5e7dc6d7e..2a3a2116b4138 100644 --- a/modules/lfs/pointer.go +++ b/modules/lfs/pointer.go @@ -14,6 +14,8 @@ import ( "regexp" "strconv" "strings" + + "code.gitea.io/gitea/modules/log" ) const ( @@ -111,6 +113,17 @@ func (p Pointer) RelativePath() string { return path.Join(p.Oid[0:2], p.Oid[2:4], p.Oid[4:]) } +// ColorFormat provides a basic color format for a Team +func (p Pointer) ColorFormat(s fmt.State) { + if p.Oid == "" && p.Size == 0 { + log.ColorFprintf(s, "") + return + } + log.ColorFprintf(s, "%s:%d", + log.NewColoredIDValue(p.Oid), + p.Size) +} + // GeneratePointer generates a pointer for arbitrary content func GeneratePointer(content io.Reader) (Pointer, error) { h := sha256.New() diff --git a/modules/repository/repo.go b/modules/repository/repo.go index ff022f0aeddf6..ed9ed508be4c6 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -254,7 +254,7 @@ func SyncReleasesWithTags(repo *repo_model.Repository, gitRepo *git.Repository) opts.Page = page rels, err := models.GetReleasesByRepoID(repo.ID, opts) if err != nil { - return fmt.Errorf("GetReleasesByRepoID: %v", err) + return fmt.Errorf("unable to GetReleasesByRepoID in Repo[%d:%s/%s]: %w", repo.ID, repo.OwnerName, repo.Name, err) } if len(rels) == 0 { break @@ -265,11 +265,11 @@ func SyncReleasesWithTags(repo *repo_model.Repository, gitRepo *git.Repository) } commitID, err := gitRepo.GetTagCommitID(rel.TagName) if err != nil && !git.IsErrNotExist(err) { - return fmt.Errorf("GetTagCommitID: %s: %v", rel.TagName, err) + return fmt.Errorf("unable to GetTagCommitID for %q in Repo[%d:%s/%s]: %w", rel.TagName, repo.ID, repo.OwnerName, repo.Name, err) } if git.IsErrNotExist(err) || commitID != rel.Sha1 { if err := models.PushUpdateDeleteTag(repo, rel.TagName); err != nil { - return fmt.Errorf("PushUpdateDeleteTag: %s: %v", rel.TagName, err) + return fmt.Errorf("unable to PushUpdateDeleteTag: %q in Repo[%d:%s/%s]: %w", rel.TagName, repo.ID, repo.OwnerName, repo.Name, err) } } else { existingRelTags[strings.ToLower(rel.TagName)] = struct{}{} @@ -278,12 +278,12 @@ func SyncReleasesWithTags(repo *repo_model.Repository, gitRepo *git.Repository) } tags, err := gitRepo.GetTags(0, 0) if err != nil { - return fmt.Errorf("GetTags: %v", err) + return fmt.Errorf("unable to GetTags in Repo[%d:%s/%s]: %w", repo.ID, repo.OwnerName, repo.Name, err) } for _, tagName := range tags { if _, ok := existingRelTags[strings.ToLower(tagName)]; !ok { if err := PushUpdateAddTag(repo, gitRepo, tagName); err != nil { - return fmt.Errorf("pushUpdateAddTag: %v", err) + return fmt.Errorf("unable to PushUpdateAddTag: %q to Repo[%d:%s/%s]: %w", tagName, repo.ID, repo.OwnerName, repo.Name, err) } } } @@ -294,11 +294,11 @@ func SyncReleasesWithTags(repo *repo_model.Repository, gitRepo *git.Repository) func PushUpdateAddTag(repo *repo_model.Repository, gitRepo *git.Repository, tagName string) error { tag, err := gitRepo.GetTag(tagName) if err != nil { - return fmt.Errorf("GetTag: %v", err) + return fmt.Errorf("unable to GetTag: %w", err) } commit, err := tag.Commit(gitRepo) if err != nil { - return fmt.Errorf("Commit: %v", err) + return fmt.Errorf("unable to get tag Commit: %w", err) } sig := tag.Tagger @@ -315,14 +315,14 @@ func PushUpdateAddTag(repo *repo_model.Repository, gitRepo *git.Repository, tagN if sig != nil { author, err = user_model.GetUserByEmail(sig.Email) if err != nil && !user_model.IsErrUserNotExist(err) { - return fmt.Errorf("GetUserByEmail: %v", err) + return fmt.Errorf("unable to GetUserByEmail for %q: %w", sig.Email, err) } createdAt = sig.When } commitsCount, err := commit.CommitsCount() if err != nil { - return fmt.Errorf("CommitsCount: %v", err) + return fmt.Errorf("unable to get CommitsCount: %w", err) } rel := models.Release{ @@ -359,14 +359,14 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repo.ID}) if err != nil { - log.Error("Error creating LFS meta object %v: %v", p, err) + log.Error("Repo[%-v]: Error creating LFS meta object %-v: %v", repo, p, err) return err } if err := contentStore.Put(p, content); err != nil { - log.Error("Error storing content for LFS meta object %v: %v", p, err) + log.Error("Repo[%-v]: Error storing content for LFS meta object %-v: %v", repo, p, err) if _, err2 := models.RemoveLFSMetaObjectByOid(repo.ID, p.Oid); err2 != nil { - log.Error("Error removing LFS meta object %v: %v", p, err2) + log.Error("Repo[%-v]: Error removing LFS meta object %-v: %v", repo, p, err2) } return err } @@ -386,32 +386,32 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re for pointerBlob := range pointerChan { meta, err := models.GetLFSMetaObjectByOid(repo.ID, pointerBlob.Oid) if err != nil && err != models.ErrLFSObjectNotExist { - log.Error("Error querying LFS meta object %v: %v", pointerBlob.Pointer, err) + log.Error("Repo[%-v]: Error querying LFS meta object %-v: %v", repo, pointerBlob.Pointer, err) return err } if meta != nil { - log.Trace("Skipping unknown LFS meta object %v", pointerBlob.Pointer) + log.Trace("Repo[%-v]: Skipping unknown LFS meta object %-v", repo, pointerBlob.Pointer) continue } - log.Trace("LFS object %v not present in repository %s", pointerBlob.Pointer, repo.FullName()) + log.Trace("Repo[%-v]: LFS object %-v not present in repository", repo, pointerBlob.Pointer) exist, err := contentStore.Exists(pointerBlob.Pointer) if err != nil { - log.Error("Error checking if LFS object %v exists: %v", pointerBlob.Pointer, err) + log.Error("Repo[%-v]: Error checking if LFS object %-v exists: %v", repo, pointerBlob.Pointer, err) return err } if exist { - log.Trace("LFS object %v already present; creating meta object", pointerBlob.Pointer) + log.Trace("Repo[%-v]: LFS object %-v already present; creating meta object", repo, pointerBlob.Pointer) _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: pointerBlob.Pointer, RepositoryID: repo.ID}) if err != nil { - log.Error("Error creating LFS meta object %v: %v", pointerBlob.Pointer, err) + log.Error("Repo[%-v]: Error creating LFS meta object %-v: %v", repo, pointerBlob.Pointer, err) return err } } else { if setting.LFS.MaxFileSize > 0 && pointerBlob.Size > setting.LFS.MaxFileSize { - log.Info("LFS object %v download denied because of LFS_MAX_FILE_SIZE=%d < size %d", pointerBlob.Pointer, setting.LFS.MaxFileSize, pointerBlob.Size) + log.Info("Repo[%-v]: LFS object %-v download denied because of LFS_MAX_FILE_SIZE=%d < size %d", repo, pointerBlob.Pointer, setting.LFS.MaxFileSize, pointerBlob.Size) continue } @@ -432,7 +432,7 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re err, has := <-errChan if has { - log.Error("Error enumerating LFS objects for repository: %v", err) + log.Error("Repo[%-v]: Error enumerating LFS objects for repository: %v", repo, err) return err } diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index a8e0cd37b6a62..4c77a169ae1c6 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -93,7 +93,7 @@ func NewAuthSource(ctx *context.Context) { ctx.Data["PageIsAdmin"] = true ctx.Data["PageIsAdminAuthentications"] = true - ctx.Data["type"] = auth.LDAP + ctx.Data["type"] = auth.LDAP.Int() ctx.Data["CurrentTypeName"] = auth.Names[auth.LDAP] ctx.Data["CurrentSecurityProtocol"] = ldap.SecurityProtocolNames[ldap.SecurityProtocolUnencrypted] ctx.Data["smtp_auth"] = "PLAIN" @@ -112,7 +112,7 @@ func NewAuthSource(ctx *context.Context) { ctx.Data["SSPIDefaultLanguage"] = "" // only the first as default - ctx.Data["oauth2_provider"] = oauth2providers[0] + ctx.Data["oauth2_provider"] = oauth2providers[0].Name ctx.HTML(http.StatusOK, tplAuthNew) } @@ -253,9 +253,6 @@ func NewAuthSourcePost(ctx *context.Context) { ctx.Data["SSPISeparatorReplacement"] = "_" ctx.Data["SSPIDefaultLanguage"] = "" - // FIXME: most error path to render tplAuthNew will fail and result in 500 - // * template: admin/auth/new:17:68: executing "admin/auth/new" at <.type.Int>: can't evaluate field Int in type interface {} - // * template: admin/auth/source/oauth:5:93: executing "admin/auth/source/oauth" at <.oauth2_provider.Name>: can't evaluate field Name in type interface {} hasTLS := false var config convert.Conversion switch auth.Type(form.Type) { diff --git a/routers/web/feed/profile.go b/routers/web/feed/profile.go index 1a7f4ad24b21e..4c1eff04a9d94 100644 --- a/routers/web/feed/profile.go +++ b/routers/web/feed/profile.go @@ -23,31 +23,34 @@ func RetrieveFeeds(ctx *context.Context, options models.GetFeedsOptions) []*mode return nil } - userCache := map[int64]*user_model.User{options.RequestedUser.ID: options.RequestedUser} - if ctx.User != nil { - userCache[ctx.User.ID] = ctx.User - } - for _, act := range actions { - if act.ActUser != nil { - userCache[act.ActUserID] = act.ActUser + // TODO: move load repoOwner of act.Repo into models.GetFeeds->loadAttributes() + { + userCache := map[int64]*user_model.User{options.RequestedUser.ID: options.RequestedUser} + if ctx.User != nil { + userCache[ctx.User.ID] = ctx.User } - } - - for _, act := range actions { - repoOwner, ok := userCache[act.Repo.OwnerID] - if !ok { - repoOwner, err = user_model.GetUserByID(act.Repo.OwnerID) - if err != nil { - if user_model.IsErrUserNotExist(err) { - continue + for _, act := range actions { + if act.ActUser != nil { + userCache[act.ActUserID] = act.ActUser + } + } + for _, act := range actions { + repoOwner, ok := userCache[act.Repo.OwnerID] + if !ok { + repoOwner, err = user_model.GetUserByID(act.Repo.OwnerID) + if err != nil { + if user_model.IsErrUserNotExist(err) { + continue + } + ctx.ServerError("GetUserByID", err) + return nil } - ctx.ServerError("GetUserByID", err) - return nil + userCache[repoOwner.ID] = repoOwner } - userCache[repoOwner.ID] = repoOwner + act.Repo.Owner = repoOwner } - act.Repo.Owner = repoOwner } + return actions } @@ -57,7 +60,7 @@ func ShowUserFeed(ctx *context.Context, ctxUser *user_model.User, formatType str RequestedUser: ctxUser, Actor: ctx.User, IncludePrivate: false, - OnlyPerformedBy: true, + OnlyPerformedBy: !ctxUser.IsOrganization(), IncludeDeleted: false, Date: ctx.FormString("date"), }) diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 9c0ce10dae6e3..b4198ef8fd8b8 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -94,14 +94,11 @@ func Profile(ctx *context.Context) { } if ctxUser.IsOrganization() { - /* - // TODO: enable after rss.RetrieveFeeds() do handle org correctly - // Show Org RSS feed - if len(showFeedType) != 0 { - rss.ShowUserFeed(ctx, ctxUser, showFeedType) - return - } - */ + // Show Org RSS feed + if len(showFeedType) != 0 { + feed.ShowUserFeed(ctx, ctxUser, showFeedType) + return + } org.Home(ctx) return diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index c86f15efc0210..6c9c4a04838d2 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -201,7 +201,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo remoteAddr, remoteErr := git.GetRemoteAddress(ctx, repoPath, m.GetRemoteName()) if remoteErr != nil { - log.Error("GetRemoteAddress Error %v", remoteErr) + log.Error("SyncMirrors [repo: %-v]: GetRemoteAddress Error %v", m.Repo, remoteErr) } stdoutBuilder := strings.Builder{} @@ -225,7 +225,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo // Now check if the error is a resolve reference due to broken reference if strings.Contains(stderr, "unable to resolve reference") && strings.Contains(stderr, "reference broken") { - log.Warn("Failed to update mirror repository %-v due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) + log.Warn("SyncMirrors [repo: %-v]: failed to update mirror repository due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) err = nil // Attempt prune @@ -255,7 +255,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo // If there is still an error (or there always was an error) if err != nil { - log.Error("Failed to update mirror repository %-v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdoutMessage, stderrMessage, err) + log.Error("SyncMirrors [repo: %-v]: failed to update mirror repository:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdoutMessage, stderrMessage, err) desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, stderrMessage) if err = admin_model.CreateRepositoryNotice(desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) @@ -267,13 +267,13 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo gitRepo, err := git.OpenRepositoryCtx(ctx, repoPath) if err != nil { - log.Error("OpenRepository: %v", err) + log.Error("SyncMirrors [repo: %-v]: failed to OpenRepository: %v", m.Repo, err) return nil, false } log.Trace("SyncMirrors [repo: %-v]: syncing releases with tags...", m.Repo) if err = repo_module.SyncReleasesWithTags(m.Repo, gitRepo); err != nil { - log.Error("Failed to synchronize tags to releases for repository: %v", err) + log.Error("SyncMirrors [repo: %-v]: failed to synchronize tags to releases: %v", m.Repo, err) } if m.LFS && setting.LFS.StartServer { @@ -281,14 +281,14 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo endpoint := lfs.DetermineEndpoint(remoteAddr.String(), m.LFSEndpoint) lfsClient := lfs.NewClient(endpoint, nil) if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, m.Repo, gitRepo, lfsClient); err != nil { - log.Error("Failed to synchronize LFS objects for repository: %v", err) + log.Error("SyncMirrors [repo: %-v]: failed to synchronize LFS objects for repository: %v", m.Repo, err) } } gitRepo.Close() log.Trace("SyncMirrors [repo: %-v]: updating size of repository", m.Repo) if err := models.UpdateRepoSize(db.DefaultContext, m.Repo); err != nil { - log.Error("Failed to update size for mirror repository: %v", err) + log.Error("SyncMirrors [repo: %-v]: failed to update size for mirror repository: %v", m.Repo, err) } if m.Repo.HasWiki() { @@ -311,7 +311,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo remoteAddr, remoteErr := git.GetRemoteAddress(ctx, wikiPath, m.GetRemoteName()) if remoteErr != nil { - log.Error("GetRemoteAddress Error %v", remoteErr) + log.Error("SyncMirrors [repo: %-v Wiki]: unable to get GetRemoteAddress Error %v", m.Repo, remoteErr) } // sanitize the output, since it may contain the remote address, which may @@ -322,7 +322,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo // Now check if the error is a resolve reference due to broken reference if strings.Contains(stderrMessage, "unable to resolve reference") && strings.Contains(stderrMessage, "reference broken") { - log.Warn("Failed to update mirror wiki repository %-v due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) + log.Warn("SyncMirrors [repo: %-v Wiki]: failed to update mirror wiki repository due to broken references:\nStdout: %s\nStderr: %s\nErr: %v\nAttempting Prune", m.Repo, stdoutMessage, stderrMessage, err) err = nil // Attempt prune @@ -350,7 +350,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo // If there is still an error (or there always was an error) if err != nil { - log.Error("Failed to update mirror repository wiki %-v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdoutMessage, stderrMessage, err) + log.Error("SyncMirrors [repo: %-v Wiki]: failed to update mirror repository wiki:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdoutMessage, stderrMessage, err) desc := fmt.Sprintf("Failed to update mirror repository wiki '%s': %s", wikiPath, stderrMessage) if err = admin_model.CreateRepositoryNotice(desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) @@ -364,7 +364,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo log.Trace("SyncMirrors [repo: %-v]: invalidating mirror branch caches...", m.Repo) branches, _, err := git.GetBranchesByPath(ctx, m.Repo.RepoPath(), 0, 0) if err != nil { - log.Error("GetBranches: %v", err) + log.Error("SyncMirrors [repo: %-v]: failed to GetBranches: %v", m.Repo, err) return nil, false } @@ -385,12 +385,12 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { return } // There was a panic whilst syncMirrors... - log.Error("PANIC whilst syncMirrors[%d] Panic: %v\nStacktrace: %s", repoID, err, log.Stack(2)) + log.Error("PANIC whilst SyncMirrors[repo_id: %d] Panic: %v\nStacktrace: %s", repoID, err, log.Stack(2)) }() m, err := repo_model.GetMirrorByRepoID(repoID) if err != nil { - log.Error("GetMirrorByRepoID [%d]: %v", repoID, err) + log.Error("SyncMirrors [repo_id: %v]: unable to GetMirrorByRepoID: %v", repoID, err) return false } @@ -406,7 +406,7 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { log.Trace("SyncMirrors [repo: %-v]: Scheduling next update", m.Repo) m.ScheduleNextUpdate() if err = repo_model.UpdateMirror(m); err != nil { - log.Error("UpdateMirror [%d]: %v", m.RepoID, err) + log.Error("SyncMirrors [repo: %-v]: failed to UpdateMirror with next update date: %v", m.Repo, err) return false } @@ -417,7 +417,7 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { log.Trace("SyncMirrors [repo: %-v]: %d branches updated", m.Repo, len(results)) gitRepo, err = git.OpenRepositoryCtx(ctx, m.Repo.RepoPath()) if err != nil { - log.Error("OpenRepository [%d]: %v", m.RepoID, err) + log.Error("SyncMirrors [repo: %-v]: unable to OpenRepository: %v", m.Repo, err) return false } defer gitRepo.Close() @@ -444,7 +444,7 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { } commitID, err := gitRepo.GetRefCommitID(result.refName) if err != nil { - log.Error("gitRepo.GetRefCommitID [repo_id: %d, ref_name: %s]: %v", m.RepoID, result.refName, err) + log.Error("SyncMirrors [repo: %-v]: unable to GetRefCommitID [ref_name: %s]: %v", m.Repo, result.refName, err) continue } notification.NotifySyncPushCommits(m.Repo.MustOwner(), m.Repo, &repo_module.PushUpdateOptions{ @@ -465,17 +465,17 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { // Push commits oldCommitID, err := git.GetFullCommitID(gitRepo.Ctx, gitRepo.Path, result.oldCommitID) if err != nil { - log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) + log.Error("SyncMirrors [repo: %-v]: unable to get GetFullCommitID[%s]: %v", m.Repo, result.oldCommitID, err) continue } newCommitID, err := git.GetFullCommitID(gitRepo.Ctx, gitRepo.Path, result.newCommitID) if err != nil { - log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) + log.Error("SyncMirrors [repo: %-v]: unable to get GetFullCommitID [%s]: %v", m.Repo, result.newCommitID, err) continue } commits, err := gitRepo.CommitsBetweenIDs(newCommitID, oldCommitID) if err != nil { - log.Error("CommitsBetweenIDs [repo_id: %d, new_commit_id: %s, old_commit_id: %s]: %v", m.RepoID, newCommitID, oldCommitID, err) + log.Error("SyncMirrors [repo: %-v]: unable to get CommitsBetweenIDs [new_commit_id: %s, old_commit_id: %s]: %v", m.Repo, newCommitID, oldCommitID, err) continue } @@ -497,12 +497,12 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { // Get latest commit date and update to current repository updated time commitDate, err := git.GetLatestCommitTime(ctx, m.Repo.RepoPath()) if err != nil { - log.Error("GetLatestCommitDate [%d]: %v", m.RepoID, err) + log.Error("SyncMirrors [repo: %-v]: unable to GetLatestCommitDate: %v", m.Repo, err) return false } if err = repo_model.UpdateRepositoryUpdatedTime(m.RepoID, commitDate); err != nil { - log.Error("Update repository 'updated_unix' [%d]: %v", m.RepoID, err) + log.Error("SyncMirrors [repo: %-v]: unable to update repository 'updated_unix': %v", m.Repo, err) return false } diff --git a/templates/admin/auth/new.tmpl b/templates/admin/auth/new.tmpl index b8e80dbcaaa01..9882cde03bcd4 100644 --- a/templates/admin/auth/new.tmpl +++ b/templates/admin/auth/new.tmpl @@ -14,7 +14,7 @@
- +
{{.CurrentTypeName}}
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index 85c7cb6166d3c..3991dc2170792 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -2,8 +2,8 @@
- -
{{.oauth2_provider.Name}}
+ +
{{.oauth2_provider}}
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
{{range .OAuth2Providers}}