From aaa1f6f287109aaea232a2ce5c86ec0774147a6a Mon Sep 17 00:00:00 2001 From: Adnan Maolood Date: Wed, 16 Feb 2022 11:12:51 -0500 Subject: [PATCH] api/loaders: Refactor RepositoriesByOwnerRepoName --- api/graph/schema.resolvers.go | 4 +- api/loaders/generate.go | 2 +- api/loaders/middleware.go | 52 ++++++++++--------- ...a9910cd_add_owner_repo_name_custom_type.py | 29 +++++++++++ 4 files changed, 59 insertions(+), 28 deletions(-) create mode 100644 gitsrht/alembic/versions/822baa9910cd_add_owner_repo_name_custom_type.py diff --git a/api/graph/schema.resolvers.go b/api/graph/schema.resolvers.go index f77b82b..3085ed1 100644 --- a/api/graph/schema.resolvers.go +++ b/api/graph/schema.resolvers.go @@ -222,7 +222,7 @@ func (r *mutationResolver) CreateRepository(ctx context.Context, name string, vi return valid.Errorf(ctx, "cloneUrl", "Invalid username") } repo, err := loaders.ForContext(ctx). - RepositoriesByOwnerRepoName.Load([2]string{entity, repoName}) + RepositoriesByOwnerRepoName.Load(loaders.OwnerRepoName{entity, repoName}) if err != nil { panic(err) } else if repo == nil { @@ -1281,7 +1281,7 @@ func (r *treeResolver) Entries(ctx context.Context, obj *model.Tree, cursor *cor func (r *userResolver) Repository(ctx context.Context, obj *model.User, name string) (*model.Repository, error) { // TODO: Load repository with user ID instead of username. Needs a new loader. - return loaders.ForContext(ctx).RepositoriesByOwnerRepoName.Load([2]string{obj.Username, name}) + return loaders.ForContext(ctx).RepositoriesByOwnerRepoName.Load(loaders.OwnerRepoName{obj.Username, name}) } func (r *userResolver) Repositories(ctx context.Context, obj *model.User, cursor *coremodel.Cursor, filter *coremodel.Filter) (*model.RepositoryCursor, error) { diff --git a/api/loaders/generate.go b/api/loaders/generate.go index 221de67..6efb1ae 100644 --- a/api/loaders/generate.go +++ b/api/loaders/generate.go @@ -4,6 +4,6 @@ package loaders //go:generate ./gen RepositoriesByIDLoader int api/graph/model.Repository -//go:generate ./gen RepositoriesByOwnerRepoNameLoader [2]string api/graph/model.Repository +//go:generate ./gen RepositoriesByOwnerRepoNameLoader OwnerRepoName api/graph/model.Repository //go:generate ./gen UsersByIDLoader int api/graph/model.User //go:generate ./gen UsersByNameLoader string api/graph/model.User diff --git a/api/loaders/middleware.go b/api/loaders/middleware.go index e852b83..1ae4f32 100644 --- a/api/loaders/middleware.go +++ b/api/loaders/middleware.go @@ -3,7 +3,9 @@ package loaders import ( "context" "database/sql" + "database/sql/driver" "errors" + "fmt" "net/http" "time" @@ -166,37 +168,37 @@ func fetchRepositoriesByID(ctx context.Context) func(ids []int) ([]*model.Reposi } } -func fetchRepositoriesByOwnerRepoName(ctx context.Context) func(names [][2]string) ([]*model.Repository, []error) { - return func(names [][2]string) ([]*model.Repository, []error) { - repos := make([]*model.Repository, len(names)) +type OwnerRepoName struct { + Owner string + RepoName string +} + +func (or OwnerRepoName) Value() (driver.Value, error) { + return fmt.Sprintf("(%q,%q)", or.Owner, or.RepoName), nil +} + +func fetchRepositoriesByOwnerRepoName(ctx context.Context) func([]OwnerRepoName) ([]*model.Repository, []error) { + return func(ownerRepoNames []OwnerRepoName) ([]*model.Repository, []error) { + repos := make([]*model.Repository, len(ownerRepoNames)) if err := database.WithTx(ctx, &sql.TxOptions{ Isolation: 0, ReadOnly: true, }, func(tx *sql.Tx) error { var ( - err error - rows *sql.Rows - _names []string = make([]string, len(names)) + err error + rows *sql.Rows ) - for i, name := range names { - // This is a hack, but it works around limitations with PostgreSQL - // and is guaranteed to work because / is invalid in both usernames - // and repo names - _names[i] = name[0] + "/" + name[1] - } query := database. Select(ctx). - Prefix(`WITH user_repo AS ( - SELECT - substring(un for position('/' in un)-1) AS owner, - substring(un from position('/' in un)+1) AS repo - FROM unnest(?::text[]) un)`, pq.Array(_names)). + Prefix(`WITH owner_repo_names AS ( + SELECT owner, repo_name + FROM unnest(?::owner_repo_name[]))`, pq.GenericArray{ownerRepoNames}). Columns(database.Columns(ctx, (&model.Repository{}).As(`repo`))...). - Columns(`u.username`). + Columns(`o.owner`). Distinct(). - From(`user_repo ur`). - Join(`"user" u on ur.owner = u.username`). - Join(`repository repo ON ur.repo = repo.name + From(`owner_repo_names o`). + Join(`"user" u on o.owner = u.username`). + Join(`repository repo ON o.repo_name = repo.name AND u.id = repo.owner_id`). LeftJoin(`access ON repo.id = access.repo_id`). Where(sq.Or{ @@ -209,7 +211,7 @@ func fetchRepositoriesByOwnerRepoName(ctx context.Context) func(names [][2]strin } defer rows.Close() - reposByOwnerRepoName := map[[2]string]*model.Repository{} + reposByOwnerRepoName := map[OwnerRepoName]*model.Repository{} for rows.Next() { var ownerName string repo := model.Repository{} @@ -217,14 +219,14 @@ func fetchRepositoriesByOwnerRepoName(ctx context.Context) func(names [][2]strin database.Scan(ctx, &repo), &ownerName)...); err != nil { panic(err) } - reposByOwnerRepoName[[2]string{ownerName, repo.Name}] = &repo + reposByOwnerRepoName[OwnerRepoName{ownerName, repo.Name}] = &repo } if err = rows.Err(); err != nil { panic(err) } - for i, name := range names { - repos[i] = reposByOwnerRepoName[name] + for i, or := range ownerRepoNames { + repos[i] = reposByOwnerRepoName[or] } return nil }); err != nil { diff --git a/gitsrht/alembic/versions/822baa9910cd_add_owner_repo_name_custom_type.py b/gitsrht/alembic/versions/822baa9910cd_add_owner_repo_name_custom_type.py new file mode 100644 index 0000000..9767ff6 --- /dev/null +++ b/gitsrht/alembic/versions/822baa9910cd_add_owner_repo_name_custom_type.py @@ -0,0 +1,29 @@ +"""Add owner_repo_name custom type + +Revision ID: 822baa9910cd +Revises: 0a3d114e8a18 +Create Date: 2022-02-16 10:06:54.271103 + +""" + +# revision identifiers, used by Alembic. +revision = '822baa9910cd' +down_revision = '0a3d114e8a18' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.execute(""" + CREATE TYPE owner_repo_name AS ( + owner text, + repo_name text + ); + """) + + +def downgrade(): + op.execute(""" + DROP TYPE owner_repo_name; + """) -- 2.38.4