From bc725adaf8ca69b0f360887a1aff62a8a55d7ca4 Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Wed, 12 Nov 2025 18:42:33 +0900 Subject: [PATCH] appview: diet the reporesolver Change-Id: zwpwyqwtsuvkuoznquvprmvyykvtxsqy Ideally we should completely remove it. Signed-off-by: Seongmin Lee --- appview/db/repos.go | 11 +++ appview/issues/issues.go | 25 +++---- appview/pulls/pulls.go | 6 +- appview/repo/settings.go | 32 ++++++++- appview/reporesolver/resolver.go | 113 ++++++++++--------------------- appview/state/state.go | 2 +- rbac/rbac.go | 8 +++ 7 files changed, 98 insertions(+), 99 deletions(-) diff --git a/appview/db/repos.go b/appview/db/repos.go index 09ef9840..3389b585 100644 --- a/appview/db/repos.go +++ b/appview/db/repos.go @@ -411,6 +411,17 @@ func GetRepoSource(e Execer, repoAt syntax.ATURI) (string, error) { return nullableSource.String, nil } +func GetRepoSourceRepo(e Execer, repoAt syntax.ATURI) (*models.Repo, error) { + source, err := GetRepoSource(e, repoAt) + if source == "" || errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, err + } + return GetRepoByAtUri(e, source) +} + func GetForksByDid(e Execer, did string) ([]models.Repo, error) { var repos []models.Repo diff --git a/appview/issues/issues.go b/appview/issues/issues.go index b16f11a1..b4644518 100644 --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -7,7 +7,6 @@ import ( "fmt" "log/slog" "net/http" - "slices" "time" comatproto "github.com/bluesky-social/indigo/api/atproto" @@ -286,17 +285,13 @@ func (rp *Issues) CloseIssue(w http.ResponseWriter, r *http.Request) { return } - collaborators, err := f.Collaborators(r.Context()) - if err != nil { - l.Error("failed to fetch repo collaborators", "err", err) - } - isCollaborator := slices.ContainsFunc(collaborators, func(collab pages.Collaborator) bool { - return user.Did == collab.Did - }) + roles := f.RolesInRepo(user) + isRepoOwner := roles.IsOwner() + isCollaborator := roles.IsCollaborator() isIssueOwner := user.Did == issue.Did // TODO: make this more granular - if isIssueOwner || isCollaborator { + if isIssueOwner || isRepoOwner || isCollaborator { err = db.CloseIssues( rp.db, db.FilterEq("id", issue.Id), @@ -338,16 +333,12 @@ func (rp *Issues) ReopenIssue(w http.ResponseWriter, r *http.Request) { return } - collaborators, err := f.Collaborators(r.Context()) - if err != nil { - l.Error("failed to fetch repo collaborators", "err", err) - } - isCollaborator := slices.ContainsFunc(collaborators, func(collab pages.Collaborator) bool { - return user.Did == collab.Did - }) + roles := f.RolesInRepo(user) + isRepoOwner := roles.IsOwner() + isCollaborator := roles.IsCollaborator() isIssueOwner := user.Did == issue.Did - if isCollaborator || isIssueOwner { + if isCollaborator || isRepoOwner || isIssueOwner { err := db.ReopenIssues( rp.db, db.FilterEq("id", issue.Id), diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go index 2e75157c..b3c739af 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -875,7 +875,8 @@ func (s *Pulls) NewPull(w http.ResponseWriter, r *http.Request) { } // Determine PR type based on input parameters - isPushAllowed := f.RepoInfo(user).Roles.IsPushAllowed() + roles := f.RolesInRepo(user) + isPushAllowed := roles.IsPushAllowed() isBranchBased := isPushAllowed && sourceBranch != "" && fromFork == "" isForkBased := fromFork != "" && sourceBranch != "" isPatchBased := patch != "" && !isBranchBased && !isForkBased @@ -1671,7 +1672,8 @@ func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { return } - if !f.RepoInfo(user).Roles.IsPushAllowed() { + roles := f.RolesInRepo(user) + if !roles.IsPushAllowed() { log.Println("unauthorized user") w.WriteHeader(http.StatusUnauthorized) return diff --git a/appview/repo/settings.go b/appview/repo/settings.go index bf4ebc0a..60de68b8 100644 --- a/appview/repo/settings.go +++ b/appview/repo/settings.go @@ -10,6 +10,7 @@ import ( "tangled.org/core/api/tangled" "tangled.org/core/appview/db" + "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" xrpcclient "tangled.org/core/appview/xrpcclient" @@ -271,7 +272,34 @@ func (rp *Repo) accessSettings(w http.ResponseWriter, r *http.Request) { f, err := rp.repoResolver.Resolve(r) user := rp.oauth.GetUser(r) - repoCollaborators, err := f.Collaborators(r.Context()) + collaborators, err := func(repo *models.Repo) ([]pages.Collaborator, error) { + repoCollaborators, err := rp.enforcer.E.GetImplicitUsersForResourceByDomain(repo.DidSlashRepo(), repo.Knot) + if err != nil { + return nil, err + } + var collaborators []pages.Collaborator + for _, item := range repoCollaborators { + // currently only two roles: owner and member + var role string + switch item[3] { + case "repo:owner": + role = "owner" + case "repo:collaborator": + role = "collaborator" + default: + continue + } + + did := item[0] + + c := pages.Collaborator{ + Did: did, + Role: role, + } + collaborators = append(collaborators, c) + } + return collaborators, nil + }(&f.Repo) if err != nil { l.Error("failed to get collaborators", "err", err) } @@ -281,7 +309,7 @@ func (rp *Repo) accessSettings(w http.ResponseWriter, r *http.Request) { RepoInfo: f.RepoInfo(user), Tabs: settingsTabs, Tab: "access", - Collaborators: repoCollaborators, + Collaborators: collaborators, }) } diff --git a/appview/reporesolver/resolver.go b/appview/reporesolver/resolver.go index 75630a9b..5351e3b1 100644 --- a/appview/reporesolver/resolver.go +++ b/appview/reporesolver/resolver.go @@ -1,9 +1,6 @@ package reporesolver import ( - "context" - "database/sql" - "errors" "fmt" "log" "net/http" @@ -17,9 +14,7 @@ import ( "tangled.org/core/appview/db" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" - "tangled.org/core/appview/pages" "tangled.org/core/appview/pages/repoinfo" - "tangled.org/core/idresolver" "tangled.org/core/rbac" ) @@ -33,14 +28,13 @@ type ResolvedRepo struct { } type RepoResolver struct { - config *config.Config - enforcer *rbac.Enforcer - idResolver *idresolver.Resolver - execer db.Execer + config *config.Config + enforcer *rbac.Enforcer + execer db.Execer } -func New(config *config.Config, enforcer *rbac.Enforcer, resolver *idresolver.Resolver, execer db.Execer) *RepoResolver { - return &RepoResolver{config: config, enforcer: enforcer, idResolver: resolver, execer: execer} +func New(config *config.Config, enforcer *rbac.Enforcer, execer db.Execer) *RepoResolver { + return &RepoResolver{config: config, enforcer: enforcer, execer: execer} } // NOTE: this... should not even be here. the entire package will be removed in future refactor @@ -80,37 +74,6 @@ func (rr *RepoResolver) Resolve(r *http.Request) (*ResolvedRepo, error) { }, nil } -func (f *ResolvedRepo) Collaborators(ctx context.Context) ([]pages.Collaborator, error) { - repoCollaborators, err := f.rr.enforcer.E.GetImplicitUsersForResourceByDomain(f.DidSlashRepo(), f.Knot) - if err != nil { - return nil, err - } - - var collaborators []pages.Collaborator - for _, item := range repoCollaborators { - // currently only two roles: owner and member - var role string - switch item[3] { - case "repo:owner": - role = "owner" - case "repo:collaborator": - role = "collaborator" - default: - continue - } - - did := item[0] - - c := pages.Collaborator{ - Did: did, - Role: role, - } - collaborators = append(collaborators, c) - } - - return collaborators, nil -} - // this function is a bit weird since it now returns RepoInfo from an entirely different // package. we should refactor this or get rid of RepoInfo entirely. func (f *ResolvedRepo) RepoInfo(user *oauth.User) repoinfo.RepoInfo { @@ -120,36 +83,34 @@ func (f *ResolvedRepo) RepoInfo(user *oauth.User) repoinfo.RepoInfo { isStarred = db.GetStarStatus(f.rr.execer, user.Did, repoAt) } - starCount, err := db.GetStarCount(f.rr.execer, repoAt) - if err != nil { - log.Println("failed to get star count for ", repoAt) - } - issueCount, err := db.GetIssueCount(f.rr.execer, repoAt) - if err != nil { - log.Println("failed to get issue count for ", repoAt) - } - pullCount, err := db.GetPullCount(f.rr.execer, repoAt) - if err != nil { - log.Println("failed to get issue count for ", repoAt) - } - source, err := db.GetRepoSource(f.rr.execer, repoAt) - if errors.Is(err, sql.ErrNoRows) { - source = "" - } else if err != nil { - log.Println("failed to get repo source for ", repoAt, err) - } - - var sourceRepo *models.Repo - if source != "" { - sourceRepo, err = db.GetRepoByAtUri(f.rr.execer, source) + stats := f.RepoStats + if stats == nil { + starCount, err := db.GetStarCount(f.rr.execer, repoAt) + if err != nil { + log.Println("failed to get star count for ", repoAt) + } + issueCount, err := db.GetIssueCount(f.rr.execer, repoAt) + if err != nil { + log.Println("failed to get issue count for ", repoAt) + } + pullCount, err := db.GetPullCount(f.rr.execer, repoAt) if err != nil { - log.Println("failed to get repo by at uri", err) + log.Println("failed to get pull count for ", repoAt) + } + stats = &models.RepoStats{ + StarCount: starCount, + IssueCount: issueCount, + PullCount: pullCount, } } - knot := f.Knot + sourceRepo, err := db.GetRepoSourceRepo(f.rr.execer, repoAt) + if err != nil { + log.Println("failed to get repo by at uri", err) + } repoInfo := repoinfo.RepoInfo{ + // this is basically a models.Repo OwnerDid: f.OwnerId.DID.String(), OwnerHandle: f.OwnerId.Handle.String(), Name: f.Name, @@ -157,21 +118,19 @@ func (f *ResolvedRepo) RepoInfo(user *oauth.User) repoinfo.RepoInfo { Description: f.Description, Website: f.Website, Topics: f.Topics, - IsStarred: isStarred, - Knot: knot, + Knot: f.Knot, Spindle: f.Spindle, - Roles: f.RolesInRepo(user), - Stats: models.RepoStats{ - StarCount: starCount, - IssueCount: issueCount, - PullCount: pullCount, - }, + Stats: *stats, + + // fork repo upstream + Source: sourceRepo, + CurrentDir: f.CurrentDir, Ref: f.Ref, - } - if sourceRepo != nil { - repoInfo.Source = sourceRepo + // info related to the session + IsStarred: isStarred, + Roles: f.RolesInRepo(user), } return repoInfo diff --git a/appview/state/state.go b/appview/state/state.go index ce632747..8b99fe21 100644 --- a/appview/state/state.go +++ b/appview/state/state.go @@ -96,7 +96,7 @@ func Make(ctx context.Context, config *config.Config) (*State, error) { } validator := validator.New(d, res, enforcer) - repoResolver := reporesolver.New(config, enforcer, res, d) + repoResolver := reporesolver.New(config, enforcer, d) wrapper := db.DbWrapper{Execer: d} jc, err := jetstream.NewJetstreamClient( diff --git a/rbac/rbac.go b/rbac/rbac.go index 28904c8e..2326ed1e 100644 --- a/rbac/rbac.go +++ b/rbac/rbac.go @@ -285,6 +285,14 @@ func (e *Enforcer) IsRepoDeleteAllowed(user, domain, repo string) (bool, error) return e.E.Enforce(user, domain, repo, "repo:delete") } +func (e *Enforcer) IsRepoOwner(user, domain, repo string) (bool, error) { + return e.E.Enforce(user, domain, repo, "repo:owner") +} + +func (e *Enforcer) IsRepoCollaborator(user, domain, repo string) (bool, error) { + return e.E.Enforce(user, domain, repo, "repo:collaborator") +} + func (e *Enforcer) IsPushAllowed(user, domain, repo string) (bool, error) { return e.E.Enforce(user, domain, repo, "repo:push") } -- 2.43.0