From fd3c98802bfdf288e55dfd3af8de4661f34c3571 Mon Sep 17 00:00:00 2001 From: Adnan Maolood Date: Wed, 2 Feb 2022 10:17:06 -0500 Subject: [PATCH] api/graph: Remove usage of database.Apply --- api/graph/schema.resolvers.go | 115 +++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 51 deletions(-) diff --git a/api/graph/schema.resolvers.go b/api/graph/schema.resolvers.go index a6a4f37..6ac17b9 100644 --- a/api/graph/schema.resolvers.go +++ b/api/graph/schema.resolvers.go @@ -280,53 +280,24 @@ func (r *mutationResolver) UpdateRepository(ctx context.Context, id int, input m if err := database.WithTx(ctx, nil, func(tx *sql.Tx) error { user := auth.ForContext(ctx) - if val, ok := input["visibility"]; ok { - delete(input, "visibility") - if val == nil { - return fmt.Errorf("Visibility cannot be null") - } - vis := model.Visibility(val.(string)) - if !vis.IsValid() { - return fmt.Errorf("Invalid visibility '%s'", val) - } - switch vis { - case model.VisibilityPublic: - input["visibility"] = "public" - case model.VisibilityUnlisted: - input["visibility"] = "unlisted" - case model.VisibilityPrivate: - input["visibility"] = "private" - default: - panic("Invariant broken; unknown visibility") - } - } - - query := database.Apply(&repo, input). - Where(`id = ?`, id). - Where(`owner_id = ?`, auth.ForContext(ctx).UserID). - Set(`updated`, sq.Expr(`now() at time zone 'utc'`)). - Suffix(`RETURNING - id, created, updated, name, description, visibility, - upstream_uri, path, owner_id`) - - if n, ok := input["name"]; ok { - if n == nil { - return fmt.Errorf("Name cannot be null") - } - name, ok := n.(string) - if !ok { - return fmt.Errorf("Invalid type for 'name' field (expected string)") - } - - if !repoNameRE.MatchString(name) { - return fmt.Errorf("Invalid repository name '%s' (must match %s)", - name, repoNameRE.String()) - } - if name == "." || name == ".." { - return fmt.Errorf("Invalid repository name '%s' (must not be . or ..)", name) - } - if name == ".git" || name == ".hg" { - return fmt.Errorf("Invalid repository name '%s' (must not be .git or .hg)", name) + query := sq.Update(repo.Table()). + PlaceholderFormat(sq.Dollar) + + valid := valid.New(ctx).WithInput(input) + + valid.OptionalString("name", func(name string) { + valid.Expect(repoNameRE.MatchString(name), + "Invalid repository name '%s' (must match %s)", + name, repoNameRE.String()). + WithField("name") + valid.Expect(name != "." && name != "..", + "Invalid repository name '%s' (must not be . or ..)", name). + WithField("name") + valid.Expect(name != ".git" && name != ".hg", + "Invalid repository name '%s' (must not be .git or .hg)", name). + WithField("name") + if !valid.Ok() { + return } var origPath string @@ -342,9 +313,13 @@ func (r *mutationResolver) UpdateRepository(ctx context.Context, id int, input m `, id, auth.ForContext(ctx).UserID) if err := row.Scan(&origPath); err != nil { if err == sql.ErrNoRows { - return fmt.Errorf("No repository by ID %d found for this user", id) + // TODO: Ability to return errors from OptionalString callback? + valid.Error("No repository by ID %d found for this user", id). + WithField("id") + return } - return err + valid.Error(err.Error()) + return } conf := config.ForContext(ctx) @@ -356,14 +331,52 @@ func (r *mutationResolver) UpdateRepository(ctx context.Context, id int, input m repoPath := path.Join(repoStore, "~"+user.Username, name) err := os.Rename(origPath, repoPath) if errors.Is(err, os.ErrExist) { - return fmt.Errorf("A repository with this name already exists.") + valid.Error("A repository with this name already exists."). + WithField("name") } else if err != nil { - return err + valid.Error(err.Error()) + return } moved = true + query = query.Set(`name`, name) query = query.Set(`path`, repoPath) + }) + + valid.NullableString("description", func(description *string) { + if description == nil { + query = query.Set(`description`, nil) + } else { + // XXX: Should we allow an empty description? + query = query.Set(`description`, *description) + } + }) + + valid.OptionalString("visibility", func(vis string) { + valid.Expect(model.Visibility(vis).IsValid(), + "Invalid visibility '%s'", vis) + query = query.Set(`visibility`, strings.ToLower(vis)) // TODO: Can we use uppercase here? + }) + + valid.NullableString("readme", func(readme *string) { + if readme == nil { + query = query.Set(`readme`, nil) + } else { + query = query.Set(`readme`, *readme) + } + }) + + if !valid.Ok() { + return errors.New("placeholder") // TODO: Avoid surfacing placeholder error } + query = query. + Where(`id = ?`, id). + Where(`owner_id = ?`, auth.ForContext(ctx).UserID). + Set(`updated`, sq.Expr(`now() at time zone 'utc'`)). + Suffix(`RETURNING + id, created, updated, name, description, visibility, + upstream_uri, path, owner_id`) + row := query.RunWith(tx).QueryRowContext(ctx) if err := row.Scan(&repo.ID, &repo.Created, &repo.Updated, &repo.Name, &repo.Description, &repo.RawVisibility, -- 2.38.4