From 5765212c6dbcaeb27779707af3ca57775e535bd9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 12 Jan 2020 17:36:21 +0800 Subject: [PATCH] Add owner_name column for table repository for maintaince reason (#9717) * Add owner_name column for table repository for maintaince reason * refactor * Fix tests * fix test * fix bug when fork repository Co-authored-by: zeripath --- models/action.go | 2 +- models/fixtures/repository.yml | 48 ++++++++++++++++ models/helper_environment.go | 2 +- models/migrations/migrations.go | 2 + models/migrations/v120.go | 20 +++++++ models/pull.go | 2 +- models/repo.go | 98 ++++++++++----------------------- models/repo_generate.go | 14 ++--- models/repo_indexer.go | 6 +- models/repo_permission.go | 8 +-- models/repo_test.go | 1 + models/wiki.go | 5 +- modules/context/repo.go | 2 +- routers/admin/repos.go | 2 +- routers/repo/compare.go | 6 +- services/mailer/mail.go | 3 +- services/pull/patch.go | 8 +-- services/pull/pull.go | 2 +- 18 files changed, 129 insertions(+), 102 deletions(-) create mode 100644 models/migrations/v120.go diff --git a/models/action.go b/models/action.go index a7e04a72fd2..1754c2a353f 100644 --- a/models/action.go +++ b/models/action.go @@ -145,7 +145,7 @@ func (a *Action) GetActAvatar() string { // GetRepoUserName returns the name of the action repository owner. func (a *Action) GetRepoUserName() string { a.loadRepo() - return a.Repo.MustOwner().Name + return a.Repo.OwnerName } // ShortRepoUserName returns the name of the action repository owner diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index c7f4d4d1096..a68e63e309e 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -1,6 +1,7 @@ - id: 1 owner_id: 2 + owner_name: user2 lower_name: repo1 name: repo1 is_private: false @@ -16,6 +17,7 @@ - id: 2 owner_id: 2 + owner_name: user2 lower_name: repo2 name: repo2 is_private: true @@ -30,6 +32,7 @@ - id: 3 owner_id: 3 + owner_name: user3 lower_name: repo3 name: repo3 is_private: true @@ -43,6 +46,7 @@ - id: 4 owner_id: 5 + owner_name: user5 lower_name: repo4 name: repo4 is_private: false @@ -56,6 +60,7 @@ - id: 5 owner_id: 3 + owner_name: user3 lower_name: repo5 name: repo5 is_private: true @@ -70,6 +75,7 @@ - id: 6 owner_id: 10 + owner_name: user10 lower_name: repo6 name: repo6 is_private: true @@ -83,6 +89,7 @@ - id: 7 owner_id: 10 + owner_name: user10 lower_name: repo7 name: repo7 is_private: true @@ -96,6 +103,7 @@ - id: 8 owner_id: 10 + owner_name: user10 lower_name: repo8 name: repo8 is_private: false @@ -109,6 +117,7 @@ - id: 9 owner_id: 11 + owner_name: user11 lower_name: repo9 name: repo9 is_private: false @@ -122,6 +131,7 @@ - id: 10 owner_id: 12 + owner_name: user12 lower_name: repo10 name: repo10 is_private: false @@ -137,6 +147,7 @@ id: 11 fork_id: 10 owner_id: 13 + owner_name: user13 lower_name: repo11 name: repo11 is_private: false @@ -150,6 +161,7 @@ - id: 12 owner_id: 14 + owner_name: user14 lower_name: test_repo_12 name: test_repo_12 is_private: false @@ -163,6 +175,7 @@ - id: 13 owner_id: 14 + owner_name: user14 lower_name: test_repo_13 name: test_repo_13 is_private: true @@ -176,6 +189,7 @@ - id: 14 owner_id: 14 + owner_name: user14 lower_name: test_repo_14 name: test_repo_14 description: test_description_14 @@ -190,6 +204,7 @@ - id: 15 owner_id: 2 + owner_name: user2 lower_name: repo15 name: repo15 is_empty: true @@ -198,6 +213,7 @@ - id: 16 owner_id: 2 + owner_name: user2 lower_name: repo16 name: repo16 is_private: true @@ -211,6 +227,7 @@ - id: 17 owner_id: 15 + owner_name: user15 lower_name: big_test_public_1 name: big_test_public_1 is_private: false @@ -226,6 +243,7 @@ - id: 18 owner_id: 15 + owner_name: user15 lower_name: big_test_public_2 name: big_test_public_2 is_private: false @@ -240,6 +258,7 @@ - id: 19 owner_id: 15 + owner_name: user15 lower_name: big_test_private_1 name: big_test_private_1 is_private: true @@ -254,6 +273,7 @@ - id: 20 owner_id: 15 + owner_name: user15 lower_name: big_test_private_2 name: big_test_private_2 is_private: true @@ -268,6 +288,7 @@ - id: 21 owner_id: 16 + owner_name: user16 lower_name: big_test_public_3 name: big_test_public_3 is_private: false @@ -282,6 +303,7 @@ - id: 22 owner_id: 16 + owner_name: user16 lower_name: big_test_private_3 name: big_test_private_3 is_private: true @@ -296,6 +318,7 @@ - id: 23 owner_id: 17 + owner_name: user17 lower_name: big_test_public_4 name: big_test_public_4 is_private: false @@ -310,6 +333,7 @@ - id: 24 owner_id: 17 + owner_name: user17 lower_name: big_test_private_4 name: big_test_private_4 is_private: true @@ -324,6 +348,7 @@ - id: 25 owner_id: 20 + owner_name: user20 lower_name: big_test_public_mirror_5 name: big_test_public_mirror_5 is_private: false @@ -339,6 +364,7 @@ - id: 26 owner_id: 20 + owner_name: user20 lower_name: big_test_private_mirror_5 name: big_test_private_mirror_5 is_private: true @@ -354,6 +380,7 @@ - id: 27 owner_id: 19 + owner_name: user19 lower_name: big_test_public_mirror_6 name: big_test_public_mirror_6 is_private: false @@ -370,6 +397,7 @@ - id: 28 owner_id: 19 + owner_name: user19 lower_name: big_test_private_mirror_6 name: big_test_private_mirror_6 is_private: true @@ -387,6 +415,7 @@ id: 29 fork_id: 27 owner_id: 20 + owner_name: user20 lower_name: big_test_public_fork_7 name: big_test_public_fork_7 is_private: false @@ -402,6 +431,7 @@ id: 30 fork_id: 28 owner_id: 20 + owner_name: user20 lower_name: big_test_private_fork_7 name: big_test_private_fork_7 is_private: true @@ -416,6 +446,7 @@ - id: 31 owner_id: 2 + owner_name: user2 lower_name: repo20 name: repo20 num_stars: 0 @@ -427,6 +458,7 @@ - id: 32 # org public repo owner_id: 3 + owner_name: user3 lower_name: repo21 name: repo21 is_private: false @@ -439,6 +471,7 @@ - id: 33 owner_id: 2 + owner_name: user2 lower_name: utf8 name: utf8 is_private: false @@ -447,6 +480,7 @@ - id: 34 owner_id: 21 + owner_name: user21 lower_name: golang name: golang is_private: false @@ -459,6 +493,7 @@ - id: 35 owner_id: 21 + owner_name: user21 lower_name: graphql name: graphql is_private: false @@ -471,6 +506,7 @@ - id: 36 owner_id: 2 + owner_name: user2 lower_name: commits_search_test name: commits_search_test is_private: false @@ -483,6 +519,7 @@ - id: 37 owner_id: 2 + owner_name: user2 lower_name: git_hooks_test name: git_hooks_test is_private: false @@ -495,6 +532,7 @@ - id: 38 owner_id: 22 + owner_name: limited_org lower_name: public_repo_on_limited_org name: public_repo_on_limited_org is_private: false @@ -507,6 +545,7 @@ - id: 39 owner_id: 22 + owner_name: limited_org lower_name: private_repo_on_limited_org name: private_repo_on_limited_org is_private: true @@ -519,6 +558,7 @@ - id: 40 owner_id: 23 + owner_name: limited_org lower_name: public_repo_on_private_org name: public_repo_on_private_org is_private: false @@ -531,6 +571,7 @@ - id: 41 owner_id: 23 + owner_name: limited_org lower_name: private_repo_on_private_org name: private_repo_on_private_org is_private: true @@ -542,6 +583,7 @@ - id: 42 owner_id: 2 + owner_name: user2 lower_name: glob name: glob is_private: false @@ -554,6 +596,7 @@ - id: 43 owner_id: 26 + owner_name: org26 lower_name: repo26 name: repo26 is_private: true @@ -566,6 +609,7 @@ - id: 44 owner_id: 27 + owner_name: user27 lower_name: template1 name: template1 is_private: false @@ -579,6 +623,7 @@ - id: 45 owner_id: 27 + owner_name: user27 lower_name: template2 name: template2 is_private: false @@ -592,6 +637,7 @@ - id: 46 owner_id: 26 + owner_name: org26 lower_name: repo_external_tracker name: repo_external_tracker is_private: false @@ -604,6 +650,7 @@ - id: 47 owner_id: 26 + owner_name: org26 lower_name: repo_external_tracker_numeric name: repo_external_tracker_numeric is_private: false @@ -616,6 +663,7 @@ - id: 48 owner_id: 26 + owner_name: org26 lower_name: repo_external_tracker_alpha name: repo_external_tracker_alpha is_private: false diff --git a/models/helper_environment.go b/models/helper_environment.go index 35af17adb10..112df96823a 100644 --- a/models/helper_environment.go +++ b/models/helper_environment.go @@ -43,7 +43,7 @@ func FullPushingEnvironment(author, committer *User, repo *Repository, repoName "GIT_COMMITTER_NAME="+committerSig.Name, "GIT_COMMITTER_EMAIL="+committerSig.Email, EnvRepoName+"="+repoName, - EnvRepoUsername+"="+repo.MustOwnerName(), + EnvRepoUsername+"="+repo.OwnerName, EnvRepoIsWiki+"="+isWiki, EnvPusherName+"="+committer.Name, EnvPusherID+"="+fmt.Sprintf("%d", committer.ID), diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index dc5cc48c64e..703c168b008 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -294,6 +294,8 @@ var migrations = []Migration{ NewMigration("Add commit id and stale to reviews", addReviewCommitAndStale), // v119 -> v120 NewMigration("Fix migrated repositories' git service type", fixMigratedRepositoryServiceType), + // v120 -> v121 + NewMigration("Add owner_name on table repository", addOwnerNameOnRepository), } // Migrate database to current version diff --git a/models/migrations/v120.go b/models/migrations/v120.go new file mode 100644 index 00000000000..91d5b503f3f --- /dev/null +++ b/models/migrations/v120.go @@ -0,0 +1,20 @@ +// 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 migrations + +import ( + "xorm.io/xorm" +) + +func addOwnerNameOnRepository(x *xorm.Engine) error { + type Repository struct { + OwnerName string + } + if err := x.Sync2(new(Repository)); err != nil { + return err + } + _, err := x.Exec("UPDATE repository SET owner_name = (SELECT name FROM `user` WHERE `user`.id = repository.owner_id)") + return err +} diff --git a/models/pull.go b/models/pull.go index bf2527679f1..0435311e4e5 100644 --- a/models/pull.go +++ b/models/pull.go @@ -70,7 +70,7 @@ func (pr *PullRequest) MustHeadUserName() string { log.Error("LoadHeadRepo: %v", err) return "" } - return pr.HeadRepo.MustOwnerName() + return pr.HeadRepo.OwnerName } // Note: don't try to get Issue because will end up recursive querying. diff --git a/models/repo.go b/models/repo.go index 6c9623ea2c5..e15c22e8224 100644 --- a/models/repo.go +++ b/models/repo.go @@ -149,9 +149,9 @@ const ( // Repository represents a git repository. type Repository struct { - ID int64 `xorm:"pk autoincr"` - OwnerID int64 `xorm:"UNIQUE(s) index"` - OwnerName string `xorm:"-"` + ID int64 `xorm:"pk autoincr"` + OwnerID int64 `xorm:"UNIQUE(s) index"` + OwnerName string Owner *User `xorm:"-"` LowerName string `xorm:"UNIQUE(s) INDEX NOT NULL"` Name string `xorm:"INDEX NOT NULL"` @@ -252,17 +252,9 @@ func (repo *Repository) MustOwner() *User { return repo.mustOwner(x) } -// MustOwnerName always returns valid owner name to avoid -// conceptually impossible error handling. -// It returns "error" and logs error details when error -// occurs. -func (repo *Repository) MustOwnerName() string { - return repo.mustOwnerName(x) -} - // FullName returns the repository full name func (repo *Repository) FullName() string { - return repo.MustOwnerName() + "/" + repo.Name + return repo.OwnerName + "/" + repo.Name } // HTMLURL returns the repository HTML URL @@ -294,7 +286,7 @@ func (repo *Repository) GetCommitsCountCacheKey(contextName string, isRef bool) func (repo *Repository) innerAPIFormat(e Engine, mode AccessMode, isParent bool) *api.Repository { var parent *api.Repository - cloneLink := repo.cloneLink(e, false) + cloneLink := repo.cloneLink(false) permission := &api.Permission{ Admin: mode >= AccessModeAdmin, Push: mode >= AccessModeWrite, @@ -356,6 +348,8 @@ func (repo *Repository) innerAPIFormat(e Engine, mode AccessMode, isParent bool) allowSquash = config.AllowSquash } + repo.mustOwner(e) + return &api.Repository{ ID: repo.ID, Owner: repo.Owner.APIFormat(), @@ -533,46 +527,11 @@ func (repo *Repository) mustOwner(e Engine) *User { return repo.Owner } -func (repo *Repository) getOwnerName(e Engine) error { - if len(repo.OwnerName) > 0 { - return nil - } - - if repo.Owner != nil { - repo.OwnerName = repo.Owner.Name - return nil - } - - u := new(User) - has, err := e.ID(repo.OwnerID).Cols("name").Get(u) - if err != nil { - return err - } else if !has { - return ErrUserNotExist{repo.OwnerID, "", 0} - } - repo.OwnerName = u.Name - return nil -} - -// GetOwnerName returns the repository owner name -func (repo *Repository) GetOwnerName() error { - return repo.getOwnerName(x) -} - -func (repo *Repository) mustOwnerName(e Engine) string { - if err := repo.getOwnerName(e); err != nil { - log.Error("Error loading repository owner name: %v", err) - return "error" - } - - return repo.OwnerName -} - // ComposeMetas composes a map of metas for properly rendering issue links and external issue trackers. func (repo *Repository) ComposeMetas() map[string]string { if repo.RenderingMetas == nil { metas := map[string]string{ - "user": repo.MustOwner().Name, + "user": repo.OwnerName, "repo": repo.Name, "repoPath": repo.RepoPath(), } @@ -588,6 +547,7 @@ func (repo *Repository) ComposeMetas() map[string]string { } } + repo.MustOwner() if repo.Owner.IsOrganization() { teams := make([]string, 0, 5) _ = x.Table("team_repo"). @@ -597,7 +557,7 @@ func (repo *Repository) ComposeMetas() map[string]string { OrderBy("team.lower_name"). Find(&teams) metas["teams"] = "," + strings.Join(teams, ",") + "," - metas["org"] = repo.Owner.LowerName + metas["org"] = strings.ToLower(repo.OwnerName) } repo.RenderingMetas = metas @@ -711,13 +671,9 @@ func (repo *Repository) getTemplateRepo(e Engine) (err error) { return err } -func (repo *Repository) repoPath(e Engine) string { - return RepoPath(repo.mustOwnerName(e), repo.Name) -} - // RepoPath returns the repository path func (repo *Repository) RepoPath() string { - return repo.repoPath(x) + return RepoPath(repo.OwnerName, repo.Name) } // GitConfigPath returns the path to a repository's git config/ directory @@ -742,7 +698,7 @@ func (repo *Repository) Link() string { // ComposeCompareURL returns the repository comparison URL func (repo *Repository) ComposeCompareURL(oldCommitID, newCommitID string) string { - return fmt.Sprintf("%s/%s/compare/%s...%s", repo.MustOwner().Name, repo.Name, oldCommitID, newCommitID) + return fmt.Sprintf("%s/compare/%s...%s", repo.FullName(), oldCommitID, newCommitID) } // UpdateDefaultBranch updates the default branch @@ -757,7 +713,7 @@ func (repo *Repository) IsOwnedBy(userID int64) bool { } func (repo *Repository) updateSize(e Engine) error { - size, err := util.GetDirectorySize(repo.repoPath(e)) + size, err := util.GetDirectorySize(repo.RepoPath()) if err != nil { return fmt.Errorf("UpdateSize: %v", err) } @@ -912,7 +868,7 @@ func ComposeHTTPSCloneURL(owner, repo string) string { return fmt.Sprintf("%s%s/%s.git", setting.AppURL, url.PathEscape(owner), url.PathEscape(repo)) } -func (repo *Repository) cloneLink(e Engine, isWiki bool) *CloneLink { +func (repo *Repository) cloneLink(isWiki bool) *CloneLink { repoName := repo.Name if isWiki { repoName += ".wiki" @@ -923,22 +879,21 @@ func (repo *Repository) cloneLink(e Engine, isWiki bool) *CloneLink { sshUser = setting.SSH.BuiltinServerUser } - repo.Owner = repo.mustOwner(e) cl := new(CloneLink) if setting.SSH.Port != 22 { - cl.SSH = fmt.Sprintf("ssh://%s@%s:%d/%s/%s.git", sshUser, setting.SSH.Domain, setting.SSH.Port, repo.Owner.Name, repoName) + cl.SSH = fmt.Sprintf("ssh://%s@%s:%d/%s/%s.git", sshUser, setting.SSH.Domain, setting.SSH.Port, repo.OwnerName, repoName) } else if setting.Repository.UseCompatSSHURI { - cl.SSH = fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, setting.SSH.Domain, repo.Owner.Name, repoName) + cl.SSH = fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, setting.SSH.Domain, repo.OwnerName, repoName) } else { - cl.SSH = fmt.Sprintf("%s@%s:%s/%s.git", sshUser, setting.SSH.Domain, repo.Owner.Name, repoName) + cl.SSH = fmt.Sprintf("%s@%s:%s/%s.git", sshUser, setting.SSH.Domain, repo.OwnerName, repoName) } - cl.HTTPS = ComposeHTTPSCloneURL(repo.Owner.Name, repoName) + cl.HTTPS = ComposeHTTPSCloneURL(repo.OwnerName, repoName) return cl } // CloneLink returns clone URLs of repository. func (repo *Repository) CloneLink() (cl *CloneLink) { - return repo.cloneLink(x, false) + return repo.cloneLink(false) } // CheckCreateRepository check if could created a repository @@ -1137,7 +1092,7 @@ func prepareRepoCommit(e Engine, repo *Repository, tmpDir, repoPath string, opts return fmt.Errorf("getRepoInitFile[%s]: %v", opts.Readme, err) } - cloneLink := repo.cloneLink(e, false) + cloneLink := repo.cloneLink(false) match := map[string]string{ "Name": repo.Name, "Description": repo.Description, @@ -1210,7 +1165,7 @@ func initRepository(e Engine, repoPath string, u *User, repo *Repository, opts C if opts.AutoInit { tmpDir, err := ioutil.TempDir(os.TempDir(), "gitea-"+repo.Name) if err != nil { - return fmt.Errorf("Failed to create temp dir for repository %s: %v", repo.repoPath(e), err) + return fmt.Errorf("Failed to create temp dir for repository %s: %v", repo.RepoPath(), err) } defer os.RemoveAll(tmpDir) @@ -1366,6 +1321,7 @@ func CreateRepository(doer, u *User, opts CreateRepoOptions) (_ *Repository, err repo := &Repository{ OwnerID: u.ID, Owner: u, + OwnerName: u.Name, Name: opts.Name, LowerName: strings.ToLower(opts.Name), Description: opts.Description, @@ -1485,6 +1441,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error // new owner. repo.OwnerID = newOwner.ID repo.Owner = newOwner + repo.OwnerName = newOwner.Name // Update repository. if _, err := sess.ID(repo.ID).Update(repo); err != nil { @@ -1683,7 +1640,7 @@ func updateRepository(e Engine, repo *Repository, visibilityChanged bool) (err e } // Create/Remove git-daemon-export-ok for git-daemon... - daemonExportFile := path.Join(repo.repoPath(e), `git-daemon-export-ok`) + daemonExportFile := path.Join(repo.RepoPath(), `git-daemon-export-ok`) if repo.IsPrivate && com.IsExist(daemonExportFile) { if err = os.Remove(daemonExportFile); err != nil { log.Error("Failed to remove %s: %v", daemonExportFile, err) @@ -1905,7 +1862,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } // FIXME: Remove repository files should be executed after transaction succeed. - repoPath := repo.repoPath(sess) + repoPath := repo.RepoPath() removeAllWithNotice(sess, "Delete repository files", repoPath) err = repo.deleteWiki(sess) @@ -2290,7 +2247,7 @@ func GitGcRepos() error { SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName())). RunInDirTimeout( time.Duration(setting.Git.Timeout.GC)*time.Second, - RepoPath(repo.Owner.Name, repo.Name)); err != nil { + repo.RepoPath()); err != nil { log.Error("Repository garbage collection failed for %v. Stdout: %s\nError: %v", repo, stdout, err) return fmt.Errorf("Repository garbage collection failed: Error: %v", err) } @@ -2517,6 +2474,7 @@ func ForkRepository(doer, owner *User, oldRepo *Repository, name, desc string) ( repo := &Repository{ OwnerID: owner.ID, Owner: owner, + OwnerName: owner.Name, Name: name, LowerName: strings.ToLower(name), Description: desc, @@ -2543,7 +2501,7 @@ func ForkRepository(doer, owner *User, oldRepo *Repository, name, desc string) ( repoPath := RepoPath(owner.Name, repo.Name) if stdout, err := git.NewCommand( - "clone", "--bare", oldRepo.repoPath(sess), repoPath). + "clone", "--bare", oldRepo.RepoPath(), repoPath). SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", oldRepo.FullName(), repo.FullName())). RunInDirTimeout(10*time.Minute, ""); err != nil { log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, oldRepo, stdout, err) diff --git a/models/repo_generate.go b/models/repo_generate.go index 1b0466eaa7c..6761c1ce41d 100644 --- a/models/repo_generate.go +++ b/models/repo_generate.go @@ -107,7 +107,7 @@ func generateRepoCommit(e Engine, repo, templateRepo, generateRepo *Repository, ) // Clone to temporary path and do the init commit. - templateRepoPath := templateRepo.repoPath(e) + templateRepoPath := templateRepo.RepoPath() if err := git.Clone(templateRepoPath, tmpDir, git.CloneRepoOptions{ Depth: 1, }); err != nil { @@ -168,7 +168,7 @@ func generateRepoCommit(e Engine, repo, templateRepo, generateRepo *Repository, return err } - repoPath := repo.repoPath(e) + repoPath := repo.RepoPath() if stdout, err := git.NewCommand("remote", "add", "origin", repoPath). SetDescription(fmt.Sprintf("generateRepoCommit (git remote add): %s to %s", templateRepoPath, tmpDir)). RunInDirWithEnv(tmpDir, env); err != nil { @@ -183,7 +183,7 @@ func generateRepoCommit(e Engine, repo, templateRepo, generateRepo *Repository, func generateRepository(e Engine, repo, templateRepo, generateRepo *Repository) (err error) { tmpDir, err := ioutil.TempDir(os.TempDir(), "gitea-"+repo.Name) if err != nil { - return fmt.Errorf("Failed to create temp dir for repository %s: %v", repo.repoPath(e), err) + return fmt.Errorf("Failed to create temp dir for repository %s: %v", repo.RepoPath(), err) } defer func() { @@ -263,13 +263,13 @@ func GenerateTopics(ctx DBContext, templateRepo, generateRepo *Repository) error // GenerateGitHooks generates git hooks from a template repository func GenerateGitHooks(ctx DBContext, templateRepo, generateRepo *Repository) error { - generateGitRepo, err := git.OpenRepository(generateRepo.repoPath(ctx.e)) + generateGitRepo, err := git.OpenRepository(generateRepo.RepoPath()) if err != nil { return err } defer generateGitRepo.Close() - templateGitRepo, err := git.OpenRepository(templateRepo.repoPath(ctx.e)) + templateGitRepo, err := git.OpenRepository(templateRepo.RepoPath()) if err != nil { return err } @@ -365,9 +365,9 @@ func generateExpansion(src string, templateRepo, generateRepo *Repository) strin case "TEMPLATE_DESCRIPTION": return templateRepo.Description case "REPO_OWNER": - return generateRepo.MustOwnerName() + return generateRepo.OwnerName case "TEMPLATE_OWNER": - return templateRepo.MustOwnerName() + return templateRepo.OwnerName case "REPO_LINK": return generateRepo.Link() case "TEMPLATE_LINK": diff --git a/models/repo_indexer.go b/models/repo_indexer.go index aee3c74b35d..a9a516175d9 100644 --- a/models/repo_indexer.go +++ b/models/repo_indexer.go @@ -62,13 +62,13 @@ func (repo *Repository) GetIndexerStatus() error { // UpdateIndexerStatus updates indexer status func (repo *Repository) UpdateIndexerStatus(sha string) error { if err := repo.GetIndexerStatus(); err != nil { - return fmt.Errorf("UpdateIndexerStatus: Unable to getIndexerStatus for repo: %s/%s Error: %v", repo.MustOwnerName(), repo.Name, err) + return fmt.Errorf("UpdateIndexerStatus: Unable to getIndexerStatus for repo: %s Error: %v", repo.FullName(), err) } if len(repo.IndexerStatus.CommitSha) == 0 { repo.IndexerStatus.CommitSha = sha _, err := x.Insert(repo.IndexerStatus) if err != nil { - return fmt.Errorf("UpdateIndexerStatus: Unable to insert repoIndexerStatus for repo: %s/%s Sha: %s Error: %v", repo.MustOwnerName(), repo.Name, sha, err) + return fmt.Errorf("UpdateIndexerStatus: Unable to insert repoIndexerStatus for repo: %s Sha: %s Error: %v", repo.FullName(), sha, err) } return nil } @@ -76,7 +76,7 @@ func (repo *Repository) UpdateIndexerStatus(sha string) error { _, err := x.ID(repo.IndexerStatus.ID).Cols("commit_sha"). Update(repo.IndexerStatus) if err != nil { - return fmt.Errorf("UpdateIndexerStatus: Unable to update repoIndexerStatus for repo: %s/%s Sha: %s Error: %v", repo.MustOwnerName(), repo.Name, sha, err) + return fmt.Errorf("UpdateIndexerStatus: Unable to update repoIndexerStatus for repo: %s Sha: %s Error: %v", repo.FullName(), sha, err) } return nil } diff --git a/models/repo_permission.go b/models/repo_permission.go index 374c6f8d561..cd202249121 100644 --- a/models/repo_permission.go +++ b/models/repo_permission.go @@ -164,10 +164,6 @@ func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permiss return } - if repo.Owner == nil { - repo.mustOwner(e) - } - var isCollaborator bool if user != nil { isCollaborator, err = repo.isCollaborator(e, user.ID) @@ -176,6 +172,10 @@ func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permiss } } + if err = repo.getOwner(e); err != nil { + return + } + // Prevent strangers from checking out public repo of private orginization // Allow user if they are collaborator of a repo within a private orginization but not a member of the orginization itself if repo.Owner.IsOrganization() && !HasOrgVisible(repo.Owner, user) && !isCollaborator { diff --git a/models/repo_test.go b/models/repo_test.go index 7a2227c8202..3a6555c7666 100644 --- a/models/repo_test.go +++ b/models/repo_test.go @@ -22,6 +22,7 @@ func TestMetas(t *testing.T) { repo := &Repository{Name: "testRepo"} repo.Owner = &User{Name: "testOwner"} + repo.OwnerName = repo.Owner.Name repo.Units = nil diff --git a/models/wiki.go b/models/wiki.go index 32a0cc1627a..223abf1edc5 100644 --- a/models/wiki.go +++ b/models/wiki.go @@ -1,4 +1,5 @@ // Copyright 2015 The Gogs Authors. All rights reserved. +// 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. @@ -13,7 +14,7 @@ import ( // WikiCloneLink returns clone URLs of repository wiki. func (repo *Repository) WikiCloneLink() *CloneLink { - return repo.cloneLink(x, true) + return repo.cloneLink(true) } // WikiPath returns wiki data path by given user and repository name. @@ -23,7 +24,7 @@ func WikiPath(userName, repoName string) string { // WikiPath returns wiki data path for given repository. func (repo *Repository) WikiPath() string { - return WikiPath(repo.MustOwnerName(), repo.Name) + return WikiPath(repo.OwnerName, repo.Name) } // HasWiki returns true if repository has wiki. diff --git a/modules/context/repo.go b/modules/context/repo.go index 4c32e846eb1..86c7df2b054 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -256,7 +256,7 @@ func RedirectToRepo(ctx *Context, redirectRepoID int64) { redirectPath := strings.Replace( ctx.Req.URL.Path, fmt.Sprintf("%s/%s", ownerName, previousRepoName), - fmt.Sprintf("%s/%s", repo.MustOwnerName(), repo.Name), + repo.FullName(), 1, ) if ctx.Req.URL.RawQuery != "" { diff --git a/routers/admin/repos.go b/routers/admin/repos.go index 73aa66807b3..39a1d7596c3 100644 --- a/routers/admin/repos.go +++ b/routers/admin/repos.go @@ -43,7 +43,7 @@ func DeleteRepo(ctx *context.Context) { ctx.ServerError("DeleteRepository", err) return } - log.Trace("Repository deleted: %s/%s", repo.MustOwner().Name, repo.Name) + log.Trace("Repository deleted: %s", repo.FullName()) ctx.Flash.Success(ctx.Tr("repo.settings.deletion_success")) ctx.JSON(200, map[string]interface{}{ diff --git a/routers/repo/compare.go b/routers/repo/compare.go index d23bccd09ad..bb800f9ef70 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -361,10 +361,8 @@ func parseBaseRepoInfo(ctx *context.Context, repo *models.Repository) error { if err := repo.GetBaseRepo(); err != nil { return err } - if err := repo.BaseRepo.GetOwnerName(); err != nil { - return err - } - baseGitRepo, err := git.OpenRepository(models.RepoPath(repo.BaseRepo.OwnerName, repo.BaseRepo.Name)) + + baseGitRepo, err := git.OpenRepository(repo.BaseRepo.RepoPath()) if err != nil { return err } diff --git a/services/mailer/mail.go b/services/mailer/mail.go index fa40170d464..4b8e46715f3 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -10,7 +10,6 @@ import ( "fmt" "html/template" "mime" - "path" "regexp" "strings" texttmpl "text/template" @@ -142,7 +141,7 @@ func SendRegisterNotifyMail(locale Locale, u *models.User) { // SendCollaboratorMail sends mail notification to new collaborator. func SendCollaboratorMail(u, doer *models.User, repo *models.Repository) { - repoName := path.Join(repo.Owner.Name, repo.Name) + repoName := repo.FullName() subject := fmt.Sprintf("%s added you to %s", doer.DisplayName(), repoName) data := map[string]interface{}{ diff --git a/services/pull/patch.go b/services/pull/patch.go index 57a2997b36e..1dbeb81c011 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -55,8 +55,8 @@ func DownloadDiffOrPatch(pr *models.PullRequest, w io.Writer, patch bool) error } pr.MergeBase = strings.TrimSpace(pr.MergeBase) if err := gitRepo.GetDiffOrPatch(pr.MergeBase, "tracking", w, patch); err != nil { - log.Error("Unable to get patch file from %s to %s in %s/%s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.MustOwner().Name, pr.BaseRepo.Name, err) - return fmt.Errorf("Unable to get patch file from %s to %s in %s/%s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.MustOwner().Name, pr.BaseRepo.Name, err) + log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) + return fmt.Errorf("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) } return nil } @@ -108,8 +108,8 @@ func TestPatch(pr *models.PullRequest) error { if err := gitRepo.GetDiff(pr.MergeBase, "tracking", tmpPatchFile); err != nil { tmpPatchFile.Close() - log.Error("Unable to get patch file from %s to %s in %s/%s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.MustOwner().Name, pr.BaseRepo.Name, err) - return fmt.Errorf("Unable to get patch file from %s to %s in %s/%s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.MustOwner().Name, pr.BaseRepo.Name, err) + log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) + return fmt.Errorf("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) } stat, err := tmpPatchFile.Stat() if err != nil { diff --git a/services/pull/pull.go b/services/pull/pull.go index b459d81cf79..bc71e522135 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -256,7 +256,7 @@ func checkIfPRContentChanged(pr *models.PullRequest, oldCommitID, newCommitID st // Add a temporary remote. tmpRemote := "checkIfPRContentChanged-" + com.ToStr(time.Now().UnixNano()) - if err = headGitRepo.AddRemote(tmpRemote, models.RepoPath(pr.BaseRepo.MustOwner().Name, pr.BaseRepo.Name), true); err != nil { + if err = headGitRepo.AddRemote(tmpRemote, pr.BaseRepo.RepoPath(), true); err != nil { return false, fmt.Errorf("AddRemote: %s/%s-%s: %v", pr.HeadRepo.OwnerName, pr.HeadRepo.Name, tmpRemote, err) } defer func() {