From 2847a2d8d0ee81704149428d07025d2b1c31a3de Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Sat, 15 Nov 2025 03:15:29 +0900 Subject: [PATCH] appview: use less `OwnerSlashRepo()` in handlers Change-Id: yurolxtlpsmzploooutrlmxlmwszowlo Signed-off-by: Seongmin Lee --- appview/issues/issues.go | 13 +++++++++---- appview/middleware/middleware.go | 4 ++-- appview/pages/repoinfo/repoinfo.go | 4 ++-- appview/pulls/pulls.go | 24 ++++++++++++++++-------- appview/repo/blob.go | 4 +++- appview/repo/tree.go | 5 +++-- appview/reporesolver/resolver.go | 12 ++++++++++++ 7 files changed, 47 insertions(+), 19 deletions(-) diff --git a/appview/issues/issues.go b/appview/issues/issues.go index 5bf5199a..9318ba55 100644 --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -312,7 +312,8 @@ func (rp *Issues) CloseIssue(w http.ResponseWriter, r *http.Request) { // notify about the issue closure rp.notifier.NewIssueState(r.Context(), syntax.DID(user.Did), issue) - rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d", f.OwnerSlashRepo(), issue.IssueId)) + ownerSlashRepo := rp.repoResolver.GetBaseRepoPath(r, &f.Repo) + rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d", ownerSlashRepo, issue.IssueId)) return } else { l.Error("user is not permitted to close issue") @@ -362,7 +363,8 @@ func (rp *Issues) ReopenIssue(w http.ResponseWriter, r *http.Request) { // notify about the issue reopen rp.notifier.NewIssueState(r.Context(), syntax.DID(user.Did), issue) - rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d", f.OwnerSlashRepo(), issue.IssueId)) + ownerSlashRepo := rp.repoResolver.GetBaseRepoPath(r, &f.Repo) + rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d", ownerSlashRepo, issue.IssueId)) return } else { l.Error("user is not the owner of the repo") @@ -466,7 +468,8 @@ func (rp *Issues) NewIssueComment(w http.ResponseWriter, r *http.Request) { } rp.notifier.NewIssueComment(r.Context(), &comment, mentions) - rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d#comment-%d", f.OwnerSlashRepo(), issue.IssueId, commentId)) + ownerSlashRepo := rp.repoResolver.GetBaseRepoPath(r, &f.Repo) + rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d#comment-%d", ownerSlashRepo, issue.IssueId, commentId)) } func (rp *Issues) IssueComment(w http.ResponseWriter, r *http.Request) { @@ -970,7 +973,9 @@ func (rp *Issues) NewIssue(w http.ResponseWriter, r *http.Request) { } } rp.notifier.NewIssue(r.Context(), issue, mentions) - rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d", f.OwnerSlashRepo(), issue.IssueId)) + + ownerSlashRepo := rp.repoResolver.GetBaseRepoPath(r, &f.Repo) + rp.pages.HxLocation(w, fmt.Sprintf("/%s/issues/%d", ownerSlashRepo, issue.IssueId)) return } } diff --git a/appview/middleware/middleware.go b/appview/middleware/middleware.go index 8ec71059..21724f86 100644 --- a/appview/middleware/middleware.go +++ b/appview/middleware/middleware.go @@ -164,7 +164,7 @@ func (mw Middleware) RepoPermissionMiddleware(requiredPerm string) middlewareFun ok, err := mw.enforcer.E.Enforce(actor.Did, f.Knot, f.DidSlashRepo(), requiredPerm) if err != nil || !ok { // we need a logged in user - log.Printf("%s does not have perms of a %s in repo %s", actor.Did, requiredPerm, f.OwnerSlashRepo()) + log.Printf("%s does not have perms of a %s in repo %s", actor.Did, requiredPerm, f.DidSlashRepo()) http.Error(w, "Forbiden", http.StatusUnauthorized) return } @@ -327,7 +327,7 @@ func (mw Middleware) GoImport() middlewareFunc { return } - fullName := f.OwnerHandle() + "/" + f.Name + fullName := mw.repoResolver.GetBaseRepoPath(r, &f.Repo) if r.Header.Get("User-Agent") == "Go-http-client/1.1" { if r.URL.Query().Get("go-get") == "1" { diff --git a/appview/pages/repoinfo/repoinfo.go b/appview/pages/repoinfo/repoinfo.go index d9aaba6b..9b5472d4 100644 --- a/appview/pages/repoinfo/repoinfo.go +++ b/appview/pages/repoinfo/repoinfo.go @@ -21,7 +21,7 @@ func (r RepoInfo) FullName() string { return path.Join(r.owner(), r.Name) } -func (r RepoInfo) OwnerWithoutAt() string { +func (r RepoInfo) ownerWithoutAt() string { if r.OwnerHandle != "" { return r.OwnerHandle } else { @@ -30,7 +30,7 @@ func (r RepoInfo) OwnerWithoutAt() string { } func (r RepoInfo) FullNameWithoutAt() string { - return path.Join(r.OwnerWithoutAt(), r.Name) + return path.Join(r.ownerWithoutAt(), r.Name) } func (r RepoInfo) GetTabs() [][]string { diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go index 572ceaa7..9311a19e 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -800,7 +800,8 @@ func (s *Pulls) PullComment(w http.ResponseWriter, r *http.Request) { } s.notifier.NewPullComment(r.Context(), comment, mentions) - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d#comment-%d", f.OwnerSlashRepo(), pull.PullId, commentId)) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d#comment-%d", ownerSlashRepo, pull.PullId, commentId)) return } } @@ -1271,7 +1272,8 @@ func (s *Pulls) createPullRequest( s.notifier.NewPull(r.Context(), pull) - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pullId)) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pullId)) } func (s *Pulls) createStackedPullRequest( @@ -1372,7 +1374,8 @@ func (s *Pulls) createStackedPullRequest( return } - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls", f.OwnerSlashRepo())) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls", ownerSlashRepo)) } func (s *Pulls) ValidatePatch(w http.ResponseWriter, r *http.Request) { @@ -1920,7 +1923,8 @@ func (s *Pulls) resubmitPullHelper( return } - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pull.PullId)) } func (s *Pulls) resubmitStackedPullHelper( @@ -2113,7 +2117,8 @@ func (s *Pulls) resubmitStackedPullHelper( return } - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pull.PullId)) } func (s *Pulls) MergePull(w http.ResponseWriter, r *http.Request) { @@ -2231,7 +2236,8 @@ func (s *Pulls) MergePull(w http.ResponseWriter, r *http.Request) { s.notifier.NewPullState(r.Context(), syntax.DID(user.Did), p) } - s.pages.HxLocation(w, fmt.Sprintf("/@%s/%s/pulls/%d", f.OwnerHandle(), f.Name, pull.PullId)) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pull.PullId)) } func (s *Pulls) ClosePull(w http.ResponseWriter, r *http.Request) { @@ -2303,7 +2309,8 @@ func (s *Pulls) ClosePull(w http.ResponseWriter, r *http.Request) { s.notifier.NewPullState(r.Context(), syntax.DID(user.Did), p) } - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pull.PullId)) } func (s *Pulls) ReopenPull(w http.ResponseWriter, r *http.Request) { @@ -2376,7 +2383,8 @@ func (s *Pulls) ReopenPull(w http.ResponseWriter, r *http.Request) { s.notifier.NewPullState(r.Context(), syntax.DID(user.Did), p) } - s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", f.OwnerSlashRepo(), pull.PullId)) + ownerSlashRepo := s.repoResolver.GetBaseRepoPath(r, &f.Repo) + s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pull.PullId)) } func newStack(f *reporesolver.ResolvedRepo, user *oauth.User, targetBranch, patch string, pullSource *models.PullSource, stackId string) (models.Stack, error) { diff --git a/appview/repo/blob.go b/appview/repo/blob.go index 481753bb..0efc9466 100644 --- a/appview/repo/blob.go +++ b/appview/repo/blob.go @@ -62,9 +62,11 @@ func (rp *Repo) Blob(w http.ResponseWriter, r *http.Request) { return } + ownerSlashRepo := rp.repoResolver.GetBaseRepoPath(r, &f.Repo) + // Use XRPC response directly instead of converting to internal types var breadcrumbs [][]string - breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", f.OwnerSlashRepo(), url.PathEscape(ref))}) + breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", ownerSlashRepo, url.PathEscape(ref))}) if filePath != "" { for idx, elem := range strings.Split(filePath, "/") { breadcrumbs = append(breadcrumbs, []string{elem, fmt.Sprintf("%s/%s", breadcrumbs[idx][1], url.PathEscape(elem))}) diff --git a/appview/repo/tree.go b/appview/repo/tree.go index 82f68352..0b54c67d 100644 --- a/appview/repo/tree.go +++ b/appview/repo/tree.go @@ -79,16 +79,17 @@ func (rp *Repo) Tree(w http.ResponseWriter, r *http.Request) { result.ReadmeFileName = xrpcResp.Readme.Filename result.Readme = xrpcResp.Readme.Contents } + ownerSlashRepo := rp.repoResolver.GetBaseRepoPath(r, &f.Repo) // redirects tree paths trying to access a blob; in this case the result.Files is unpopulated, // so we can safely redirect to the "parent" (which is the same file). if len(result.Files) == 0 && result.Parent == treePath { - redirectTo := fmt.Sprintf("/%s/blob/%s/%s", f.OwnerSlashRepo(), url.PathEscape(ref), result.Parent) + redirectTo := fmt.Sprintf("/%s/blob/%s/%s", ownerSlashRepo, url.PathEscape(ref), result.Parent) http.Redirect(w, r, redirectTo, http.StatusFound) return } user := rp.oauth.GetUser(r) var breadcrumbs [][]string - breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", f.OwnerSlashRepo(), url.PathEscape(ref))}) + breadcrumbs = append(breadcrumbs, []string{f.Name, fmt.Sprintf("/%s/tree/%s", ownerSlashRepo, url.PathEscape(ref))}) if treePath != "" { for idx, elem := range strings.Split(treePath, "/") { breadcrumbs = append(breadcrumbs, []string{elem, fmt.Sprintf("%s/%s", breadcrumbs[idx][1], url.PathEscape(elem))}) diff --git a/appview/reporesolver/resolver.go b/appview/reporesolver/resolver.go index 974bd20c..230f2e28 100644 --- a/appview/reporesolver/resolver.go +++ b/appview/reporesolver/resolver.go @@ -44,6 +44,18 @@ func New(config *config.Config, enforcer *rbac.Enforcer, resolver *idresolver.Re return &RepoResolver{config: config, enforcer: enforcer, idResolver: resolver, execer: execer} } +// NOTE: this... should not even be here. the entire package will be removed in future refactor +func (rr *RepoResolver) GetBaseRepoPath(r *http.Request, repo *models.Repo) string { + var ( + user = chi.URLParam(r, "user") + name = chi.URLParam(r, "repo") + ) + if user == "" || name == "" { + return repo.DidSlashRepo() + } + return path.Join(user, name) +} + func (rr *RepoResolver) Resolve(r *http.Request) (*ResolvedRepo, error) { repo, ok := r.Context().Value("repo").(*models.Repo) if !ok { -- 2.43.0