From 9d3277e998fc3216807b5c9f9bd9e62860f347c9 Mon Sep 17 00:00:00 2001 From: oppiliappan Date: Wed, 10 Sep 2025 16:09:54 +0100 Subject: [PATCH] appview/db: refactor GetRepo to use db.Filter Signed-off-by: oppiliappan --- appview/db/repos.go | 49 ++++++++------------- appview/issues/router.go | 2 +- appview/middleware/middleware.go | 75 ++++++++++++++++---------------- appview/pulls/pulls.go | 8 +++- appview/repo/repo.go | 6 ++- appview/state/state.go | 6 ++- spindle/ingester.go | 2 +- 7 files changed, 74 insertions(+), 74 deletions(-) diff --git a/appview/db/repos.go b/appview/db/repos.go index b9e1f2b6..432b3da5 100644 --- a/appview/db/repos.go +++ b/appview/db/repos.go @@ -334,6 +334,24 @@ func GetRepos(e Execer, limit int, filters ...filter) ([]Repo, error) { return repos, nil } +// helper to get exactly one repo +func GetRepo(e Execer, filters ...filter) (*Repo, error) { + repos, err := GetRepos(e, 0, filters...) + if err != nil { + return nil, err + } + + if repos == nil { + return nil, sql.ErrNoRows + } + + if len(repos) != 1 { + return nil, fmt.Errorf("too many rows returned") + } + + return &repos[0], nil +} + func CountRepos(e Execer, filters ...filter) (int64, error) { var conditions []string var args []any @@ -358,37 +376,6 @@ func CountRepos(e Execer, filters ...filter) (int64, error) { return count, nil } -func GetRepo(e Execer, did, name string) (*Repo, error) { - var repo Repo - var description, spindle sql.NullString - - row := e.QueryRow(` - select did, name, knot, created, description, spindle, rkey - from repos - where did = ? and name = ? - `, - did, - name, - ) - - var createdAt string - if err := row.Scan(&repo.Did, &repo.Name, &repo.Knot, &createdAt, &description, &spindle, &repo.Rkey); err != nil { - return nil, err - } - createdAtTime, _ := time.Parse(time.RFC3339, createdAt) - repo.Created = createdAtTime - - if description.Valid { - repo.Description = description.String - } - - if spindle.Valid { - repo.Spindle = spindle.String - } - - return &repo, nil -} - func GetRepoByAtUri(e Execer, atUri string) (*Repo, error) { var repo Repo var nullableDescription sql.NullString diff --git a/appview/issues/router.go b/appview/issues/router.go index 58492585..8c6751a7 100644 --- a/appview/issues/router.go +++ b/appview/issues/router.go @@ -14,7 +14,7 @@ func (i *Issues) Router(mw *middleware.Middleware) http.Handler { r.With(middleware.Paginate).Get("/", i.RepoIssues) r.Route("/{issue}", func(r chi.Router) { - r.Use(mw.ResolveIssue()) + r.Use(mw.ResolveIssue) r.Get("/", i.RepoSingleIssue) // authenticated routes diff --git a/appview/middleware/middleware.go b/appview/middleware/middleware.go index 7bb4eee6..4bea33eb 100644 --- a/appview/middleware/middleware.go +++ b/appview/middleware/middleware.go @@ -213,10 +213,13 @@ func (mw Middleware) ResolveRepo() middlewareFunc { return } - repo, err := db.GetRepo(mw.db, id.DID.String(), repoName) + repo, err := db.GetRepo( + mw.db, + db.FilterEq("did", id.DID.String()), + db.FilterEq("name", repoName), + ) if err != nil { - // invalid did or handle - log.Println("failed to resolve repo") + log.Println("failed to resolve repo", "err", err) mw.pages.ErrorKnot404(w) return } @@ -276,43 +279,41 @@ func (mw Middleware) ResolvePull() middlewareFunc { } // middleware that is tacked on top of /{user}/{repo}/issues/{issue} -func (mw Middleware) ResolveIssue() middlewareFunc { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - f, err := mw.repoResolver.Resolve(r) - if err != nil { - log.Println("failed to fully resolve repo", err) - mw.pages.ErrorKnot404(w) - return - } +func (mw Middleware) ResolveIssue(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f, err := mw.repoResolver.Resolve(r) + if err != nil { + log.Println("failed to fully resolve repo", err) + mw.pages.ErrorKnot404(w) + return + } - issueIdStr := chi.URLParam(r, "issue") - issueId, err := strconv.Atoi(issueIdStr) - if err != nil { - log.Println("failed to fully resolve issue ID", err) - mw.pages.ErrorKnot404(w) - return - } + issueIdStr := chi.URLParam(r, "issue") + issueId, err := strconv.Atoi(issueIdStr) + if err != nil { + log.Println("failed to fully resolve issue ID", err) + mw.pages.ErrorKnot404(w) + return + } - issues, err := db.GetIssues( - mw.db, - db.FilterEq("repo_at", f.RepoAt()), - db.FilterEq("issue_id", issueId), - ) - if err != nil { - log.Println("failed to get issues", "err", err) - return - } - if len(issues) != 1 { - log.Println("got incorrect number of issues", "len(issuse)", len(issues)) - return - } - issue := issues[0] + issues, err := db.GetIssues( + mw.db, + db.FilterEq("repo_at", f.RepoAt()), + db.FilterEq("issue_id", issueId), + ) + if err != nil { + log.Println("failed to get issues", "err", err) + return + } + if len(issues) != 1 { + log.Println("got incorrect number of issues", "len(issuse)", len(issues)) + return + } + issue := issues[0] - ctx := context.WithValue(r.Context(), "issue", &issue) - next.ServeHTTP(w, r.WithContext(ctx)) - }) - } + ctx := context.WithValue(r.Context(), "issue", &issue) + next.ServeHTTP(w, r.WithContext(ctx)) + }) } // this should serve the go-import meta tag even if the path is technically diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go index c7e9b366..fd6c3cc5 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -1364,9 +1364,13 @@ func (s *Pulls) CompareForksBranchesFragment(w http.ResponseWriter, r *http.Requ forkOwnerDid := repoString[0] forkName := repoString[1] // fork repo - repo, err := db.GetRepo(s.db, forkOwnerDid, forkName) + repo, err := db.GetRepo( + s.db, + db.FilterEq("did", forkOwnerDid), + db.FilterEq("name", forkName), + ) if err != nil { - log.Println("failed to get repo", user.Did, forkVal) + log.Println("failed to get repo", "did", forkOwnerDid, "name", forkName, "err", err) return } diff --git a/appview/repo/repo.go b/appview/repo/repo.go index 763ff55f..b77b70dd 100644 --- a/appview/repo/repo.go +++ b/appview/repo/repo.go @@ -1580,7 +1580,11 @@ func (rp *Repo) ForkRepo(w http.ResponseWriter, r *http.Request) { forkName := f.Name // this check is *only* to see if the forked repo name already exists // in the user's account. - existingRepo, err := db.GetRepo(rp.db, user.Did, f.Name) + existingRepo, err := db.GetRepo( + rp.db, + db.FilterEq("did", user.Did), + db.FilterEq("name", f.Name), + ) if err != nil { if errors.Is(err, sql.ErrNoRows) { // no existing repo with this name found, we can use the name as is diff --git a/appview/state/state.go b/appview/state/state.go index 2490f03e..2dfb6180 100644 --- a/appview/state/state.go +++ b/appview/state/state.go @@ -419,7 +419,11 @@ func (s *State) NewRepo(w http.ResponseWriter, r *http.Request) { } // Check for existing repos - existingRepo, err := db.GetRepo(s.db, user.Did, repoName) + existingRepo, err := db.GetRepo( + s.db, + db.FilterEq("did", user.Did), + db.FilterEq("name", repoName), + ) if err == nil && existingRepo != nil { l.Info("repo exists") s.pages.Notice(w, "repo", fmt.Sprintf("You already have a repository by this name on %s", existingRepo.Knot)) diff --git a/spindle/ingester.go b/spindle/ingester.go index dfc6c333..80825a94 100644 --- a/spindle/ingester.go +++ b/spindle/ingester.go @@ -146,7 +146,7 @@ func (s *Spindle) ingestRepo(ctx context.Context, e *models.Event) error { l := s.l.With("component", "ingester", "record", tangled.RepoNSID) - l.Info("ingesting repo record") + l.Info("ingesting repo record", "did", did) switch e.Commit.Operation { case models.CommitOperationCreate, models.CommitOperationUpdate: -- 2.51.2