Browse Source

Remove GetRepositoryByRef and add GetRepositoryByOwnerAndName (#3043)

* remove GetRepositoryByRef and add GetRepositoryByOwnerAndName

* fix tests

* fix tests bug

* some improvements
Lunny Xiao 1 year ago
parent
commit
35cc5b0402
7 changed files with 42 additions and 66 deletions
  1. 3 11
      cmd/serv.go
  2. 6 4
      models/error.go
  3. 7 2
      models/issue.go
  4. 15 16
      models/repo.go
  5. 3 6
      modules/context/context.go
  6. 6 11
      modules/lfs/server.go
  7. 2 16
      routers/repo/http.go

+ 3 - 11
cmd/serv.go

@@ -158,18 +158,10 @@ func runServ(c *cli.Context) error {
158 158
 	}
159 159
 	os.Setenv(models.EnvRepoName, reponame)
160 160
 
161
-	repoUser, err := models.GetUserByName(username)
162
-	if err != nil {
163
-		if models.IsErrUserNotExist(err) {
164
-			fail("Repository owner does not exist", "Unregistered owner: %s", username)
165
-		}
166
-		fail("Internal error", "Failed to get repository owner (%s): %v", username, err)
167
-	}
168
-
169
-	repo, err := models.GetRepositoryByName(repoUser.ID, reponame)
161
+	repo, err := models.GetRepositoryByOwnerAndName(username, reponame)
170 162
 	if err != nil {
171 163
 		if models.IsErrRepoNotExist(err) {
172
-			fail(accessDenied, "Repository does not exist: %s/%s", repoUser.Name, reponame)
164
+			fail(accessDenied, "Repository does not exist: %s/%s", username, reponame)
173 165
 		}
174 166
 		fail("Internal error", "Failed to get repository: %v", err)
175 167
 	}
@@ -263,7 +255,7 @@ func runServ(c *cli.Context) error {
263 255
 
264 256
 	//LFS token authentication
265 257
 	if verb == lfsAuthenticateVerb {
266
-		url := fmt.Sprintf("%s%s/%s.git/info/lfs", setting.AppURL, repoUser.Name, repo.Name)
258
+		url := fmt.Sprintf("%s%s/%s.git/info/lfs", setting.AppURL, username, repo.Name)
267 259
 
268 260
 		now := time.Now()
269 261
 		token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{

+ 6 - 4
models/error.go

@@ -572,9 +572,10 @@ func (err ErrLFSLockAlreadyExist) Error() string {
572 572
 
573 573
 // ErrRepoNotExist represents a "RepoNotExist" kind of error.
574 574
 type ErrRepoNotExist struct {
575
-	ID   int64
576
-	UID  int64
577
-	Name string
575
+	ID        int64
576
+	UID       int64
577
+	OwnerName string
578
+	Name      string
578 579
 }
579 580
 
580 581
 // IsErrRepoNotExist checks if an error is a ErrRepoNotExist.
@@ -584,7 +585,8 @@ func IsErrRepoNotExist(err error) bool {
584 585
 }
585 586
 
586 587
 func (err ErrRepoNotExist) Error() string {
587
-	return fmt.Sprintf("repository does not exist [id: %d, uid: %d, name: %s]", err.ID, err.UID, err.Name)
588
+	return fmt.Sprintf("repository does not exist [id: %d, uid: %d, owner_name: %s, name: %s]",
589
+		err.ID, err.UID, err.OwnerName, err.Name)
588 590
 }
589 591
 
590 592
 // ErrRepoAlreadyExist represents a "RepoAlreadyExist" kind of error.

+ 7 - 2
models/issue.go

@@ -964,7 +964,7 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, uuids []string)
964 964
 // GetIssueByRef returns an Issue specified by a GFM reference.
965 965
 // See https://help.github.com/articles/writing-on-github#references for more information on the syntax.
966 966
 func GetIssueByRef(ref string) (*Issue, error) {
967
-	n := strings.IndexByte(ref, byte('#'))
967
+	n := strings.IndexByte(ref, '#')
968 968
 	if n == -1 {
969 969
 		return nil, errMissingIssueNumber
970 970
 	}
@@ -974,7 +974,12 @@ func GetIssueByRef(ref string) (*Issue, error) {
974 974
 		return nil, errInvalidIssueNumber
975 975
 	}
976 976
 
977
-	repo, err := GetRepositoryByRef(ref[:n])
977
+	i := strings.IndexByte(ref[:n], '/')
978
+	if i < 2 {
979
+		return nil, ErrInvalidReference
980
+	}
981
+
982
+	repo, err := GetRepositoryByOwnerAndName(ref[:i], ref[i+1:n])
978 983
 	if err != nil {
979 984
 		return nil, err
980 985
 	}

+ 15 - 16
models/repo.go

@@ -1740,13 +1740,13 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
1740 1740
 	if err != nil {
1741 1741
 		return err
1742 1742
 	} else if !has {
1743
-		return ErrRepoNotExist{repoID, uid, ""}
1743
+		return ErrRepoNotExist{repoID, uid, "", ""}
1744 1744
 	}
1745 1745
 
1746 1746
 	if cnt, err := sess.ID(repoID).Delete(&Repository{}); err != nil {
1747 1747
 		return err
1748 1748
 	} else if cnt != 1 {
1749
-		return ErrRepoNotExist{repoID, uid, ""}
1749
+		return ErrRepoNotExist{repoID, uid, "", ""}
1750 1750
 	}
1751 1751
 
1752 1752
 	if org.IsOrganization() {
@@ -1891,21 +1891,20 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
1891 1891
 	return nil
1892 1892
 }
1893 1893
 
1894
-// GetRepositoryByRef returns a Repository specified by a GFM reference.
1895
-// See https://help.github.com/articles/writing-on-github#references for more information on the syntax.
1896
-func GetRepositoryByRef(ref string) (*Repository, error) {
1897
-	n := strings.IndexByte(ref, byte('/'))
1898
-	if n < 2 {
1899
-		return nil, ErrInvalidReference
1900
-	}
1901
-
1902
-	userName, repoName := ref[:n], ref[n+1:]
1903
-	user, err := GetUserByName(userName)
1894
+// GetRepositoryByOwnerAndName returns the repository by given ownername and reponame.
1895
+func GetRepositoryByOwnerAndName(ownerName, repoName string) (*Repository, error) {
1896
+	var repo Repository
1897
+	has, err := x.Select("repository.*").
1898
+		Join("INNER", "user", "`user`.id = repository.owner_id").
1899
+		Where("repository.lower_name = ?", strings.ToLower(repoName)).
1900
+		And("`user`.lower_name = ?", strings.ToLower(ownerName)).
1901
+		Get(&repo)
1904 1902
 	if err != nil {
1905 1903
 		return nil, err
1904
+	} else if !has {
1905
+		return nil, ErrRepoNotExist{0, 0, ownerName, repoName}
1906 1906
 	}
1907
-
1908
-	return GetRepositoryByName(user.ID, repoName)
1907
+	return &repo, nil
1909 1908
 }
1910 1909
 
1911 1910
 // GetRepositoryByName returns the repository by given name under user if exists.
@@ -1918,7 +1917,7 @@ func GetRepositoryByName(ownerID int64, name string) (*Repository, error) {
1918 1917
 	if err != nil {
1919 1918
 		return nil, err
1920 1919
 	} else if !has {
1921
-		return nil, ErrRepoNotExist{0, ownerID, name}
1920
+		return nil, ErrRepoNotExist{0, ownerID, "", name}
1922 1921
 	}
1923 1922
 	return repo, err
1924 1923
 }
@@ -1929,7 +1928,7 @@ func getRepositoryByID(e Engine, id int64) (*Repository, error) {
1929 1928
 	if err != nil {
1930 1929
 		return nil, err
1931 1930
 	} else if !has {
1932
-		return nil, ErrRepoNotExist{id, 0, ""}
1931
+		return nil, ErrRepoNotExist{id, 0, "", ""}
1933 1932
 	}
1934 1933
 	return repo, nil
1935 1934
 }

+ 3 - 6
modules/context/context.go

@@ -176,12 +176,9 @@ func Contexter() macaron.Handler {
176 176
 			repoName := c.Params(":reponame")
177 177
 			branchName := "master"
178 178
 
179
-			owner, err := models.GetUserByName(ownerName)
180
-			if err == nil {
181
-				repo, err := models.GetRepositoryByName(owner.ID, repoName)
182
-				if err == nil && len(repo.DefaultBranch) > 0 {
183
-					branchName = repo.DefaultBranch
184
-				}
179
+			repo, err := models.GetRepositoryByOwnerAndName(ownerName, repoName)
180
+			if err == nil && len(repo.DefaultBranch) > 0 {
181
+				branchName = repo.DefaultBranch
185 182
 			}
186 183
 			prefix := setting.AppURL + path.Join(ownerName, repoName, "src", "branch", branchName)
187 184
 			c.PlainText(http.StatusOK, []byte(com.Expand(`

+ 6 - 11
modules/lfs/server.go

@@ -108,10 +108,9 @@ func ObjectOidHandler(ctx *context.Context) {
108 108
 }
109 109
 
110 110
 func getAuthenticatedRepoAndMeta(ctx *context.Context, rv *RequestVars, requireWrite bool) (*models.LFSMetaObject, *models.Repository) {
111
-	repositoryString := rv.User + "/" + rv.Repo
112
-	repository, err := models.GetRepositoryByRef(repositoryString)
111
+	repository, err := models.GetRepositoryByOwnerAndName(rv.User, rv.Repo)
113 112
 	if err != nil {
114
-		log.Debug("Could not find repository: %s - %s", repositoryString, err)
113
+		log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err)
115 114
 		writeStatus(ctx, 404)
116 115
 		return nil, nil
117 116
 	}
@@ -197,7 +196,6 @@ func getMetaHandler(ctx *context.Context) {
197 196
 
198 197
 // PostHandler instructs the client how to upload data
199 198
 func PostHandler(ctx *context.Context) {
200
-
201 199
 	if !setting.LFS.StartServer {
202 200
 		writeStatus(ctx, 404)
203 201
 		return
@@ -210,10 +208,9 @@ func PostHandler(ctx *context.Context) {
210 208
 
211 209
 	rv := unpack(ctx)
212 210
 
213
-	repositoryString := rv.User + "/" + rv.Repo
214
-	repository, err := models.GetRepositoryByRef(repositoryString)
211
+	repository, err := models.GetRepositoryByOwnerAndName(rv.User, rv.Repo)
215 212
 	if err != nil {
216
-		log.Debug("Could not find repository: %s - %s", repositoryString, err)
213
+		log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err)
217 214
 		writeStatus(ctx, 404)
218 215
 		return
219 216
 	}
@@ -261,12 +258,10 @@ func BatchHandler(ctx *context.Context) {
261 258
 
262 259
 	// Create a response object
263 260
 	for _, object := range bv.Objects {
264
-
265
-		repositoryString := object.User + "/" + object.Repo
266
-		repository, err := models.GetRepositoryByRef(repositoryString)
261
+		repository, err := models.GetRepositoryByOwnerAndName(object.User, object.Repo)
267 262
 
268 263
 		if err != nil {
269
-			log.Debug("Could not find repository: %s - %s", repositoryString, err)
264
+			log.Debug("Could not find repository: %s/%s - %s", object.User, object.Repo, err)
270 265
 			writeStatus(ctx, 404)
271 266
 			return
272 267
 		}

+ 2 - 16
routers/repo/http.go

@@ -64,23 +64,9 @@ func HTTP(ctx *context.Context) {
64 64
 		reponame = reponame[:len(reponame)-5]
65 65
 	}
66 66
 
67
-	repoUser, err := models.GetUserByName(username)
67
+	repo, err := models.GetRepositoryByOwnerAndName(username, reponame)
68 68
 	if err != nil {
69
-		if models.IsErrUserNotExist(err) {
70
-			ctx.Handle(http.StatusNotFound, "GetUserByName", nil)
71
-		} else {
72
-			ctx.Handle(http.StatusInternalServerError, "GetUserByName", err)
73
-		}
74
-		return
75
-	}
76
-
77
-	repo, err := models.GetRepositoryByName(repoUser.ID, reponame)
78
-	if err != nil {
79
-		if models.IsErrRepoNotExist(err) {
80
-			ctx.Handle(http.StatusNotFound, "GetRepositoryByName", nil)
81
-		} else {
82
-			ctx.Handle(http.StatusInternalServerError, "GetRepositoryByName", err)
83
-		}
69
+		ctx.NotFoundOrServerError("GetRepositoryByOwnerAndName", models.IsErrRepoNotExist, err)
84 70
 		return
85 71
 	}
86 72