From cf8f7bf25d63c7c614f3c8bb146ba5a310dcaa7d Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Thu, 30 Jul 2026 01:26:21 +0900 Subject: [PATCH] slop: appview: rewrite spindle selector Signed-off-by: Seongmin Lee --- appview/db/spindle.go | 37 +++++++- appview/db/spindle_test.go | 63 +++++++++++++ appview/models/repo.go | 24 +++++ appview/models/repo_test.go | 33 +++++++ appview/pages/compose_parse_test.go | 9 +- appview/pages/ratchet_test.go | 1 - appview/pages/spindle_input_test.go | 88 +++++++++++++++++++ appview/pages/templates/repo/fork.html | 36 ++------ .../repo/fragments/spindleInput.html | 20 +++++ appview/pages/templates/repo/new.html | 34 ++----- .../templates/repo/settings/pipelines.html | 27 ++---- appview/pages/templates/spindles/index.html | 16 ---- appview/repo/repo.go | 44 ++++------ appview/state/state.go | 20 ++--- docs/DOCS.md | 15 +++- 15 files changed, 319 insertions(+), 148 deletions(-) create mode 100644 appview/db/spindle_test.go create mode 100644 appview/pages/spindle_input_test.go create mode 100644 appview/pages/templates/repo/fragments/spindleInput.html diff --git a/appview/db/spindle.go b/appview/db/spindle.go index 434f81135..349a5b66b 100644 --- a/appview/db/spindle.go +++ b/appview/db/spindle.go @@ -4,18 +4,49 @@ import ( "context" "database/sql" "fmt" + "slices" "strings" "time" "github.com/bluesky-social/indigo/atproto/syntax" "tangled.org/core/appview/models" + "tangled.org/core/consts" "tangled.org/core/orm" ) -// RecentSpindles lists spindles user recently used. +// RecentSpindles suggests spindles for the spindle picker: ones this user already +// pointed a repo at, most recently created first, plus the default spindle. func RecentSpindles(ctx context.Context, e Execer, user syntax.DID) ([]string, error) { - // NOTE: should I use redis instead..? - panic("unimplemented") + rows, err := e.QueryContext(ctx, ` + select spindle from repos + where did = ? and coalesce(spindle, '') != '' + group by spindle + order by max(id) desc + limit 5 + `, user) + if err != nil { + return nil, err + } + defer rows.Close() + + var spindles []string + for rows.Next() { + var spindle string + if err := rows.Scan(&spindle); err != nil { + return nil, err + } + spindles = append(spindles, spindle) + } + if err := rows.Err(); err != nil { + return nil, err + } + + // a fresh account owns no repos, and a bare text input gives it nothing to go on + if !slices.Contains(spindles, consts.DefaultSpindle) { + spindles = append(spindles, consts.DefaultSpindle) + } + + return spindles, nil } func GetSpindles(ctx context.Context, e Execer, filters ...orm.Filter) ([]models.Spindle, error) { diff --git a/appview/db/spindle_test.go b/appview/db/spindle_test.go new file mode 100644 index 000000000..00e1ac1b2 --- /dev/null +++ b/appview/db/spindle_test.go @@ -0,0 +1,63 @@ +package db + +import ( + "context" + "fmt" + "slices" + "testing" + + "github.com/bluesky-social/indigo/atproto/syntax" + "tangled.org/core/consts" +) + +func insertRepoWithSpindle(t *testing.T, d *DB, did, name, spindle string) { + t.Helper() + if _, err := d.Exec( + `insert into repos (did, name, knot, rkey, at_uri, spindle) values (?, ?, 'knot.test', ?, ?, ?)`, + did, name, name, fmt.Sprintf("at://%s/sh.tangled.repo/%s", did, name), spindle, + ); err != nil { + t.Fatalf("insert repo %q: %v", name, err) + } +} + +func TestRecentSpindles(t *testing.T) { + d := newTestDB(t) + const user = "did:plc:akshay" + + // oldest first; "one" and "three" share a spindle + insertRepoWithSpindle(t, d, user, "one", "a.spindle.test") + insertRepoWithSpindle(t, d, user, "two", "b.spindle.test") + insertRepoWithSpindle(t, d, user, "three", "a.spindle.test") + insertRepoWithSpindle(t, d, user, "no-spindle", "") + if _, err := d.Exec( + `insert into repos (did, name, knot, rkey, at_uri) values (?, 'null-spindle', 'knot.test', 'null-spindle', ?)`, + user, fmt.Sprintf("at://%s/sh.tangled.repo/null-spindle", user), + ); err != nil { + t.Fatalf("insert null-spindle repo: %v", err) + } + insertRepoWithSpindle(t, d, "did:plc:someone-else", "theirs", "other.spindle.test") + + got, err := RecentSpindles(context.Background(), d, syntax.DID(user)) + if err != nil { + t.Fatalf("RecentSpindles: %v", err) + } + + want := []string{"a.spindle.test", "b.spindle.test", consts.DefaultSpindle} + if !slices.Equal(got, want) { + t.Errorf("RecentSpindles = %v, want %v", got, want) + } +} + +func TestRecentSpindlesNoRepos(t *testing.T) { + d := newTestDB(t) + + got, err := RecentSpindles(context.Background(), d, syntax.DID("did:plc:akshay")) + if err != nil { + t.Fatalf("RecentSpindles: %v", err) + } + + // a fresh account still gets something to pick + if !slices.Equal(got, []string{consts.DefaultSpindle}) { + t.Errorf("RecentSpindles = %v, want [%s]", got, consts.DefaultSpindle) + } +} diff --git a/appview/models/repo.go b/appview/models/repo.go index 893abe205..41f89c9f8 100644 --- a/appview/models/repo.go +++ b/appview/models/repo.go @@ -9,6 +9,7 @@ import ( securejoin "github.com/cyphar/filepath-securejoin" enry "github.com/go-enry/go-enry/v2" "tangled.org/core/api/tangled" + "tangled.org/core/hostutil" ) type Repo struct { @@ -198,6 +199,29 @@ func StripGitExt(name string) string { return strings.TrimSuffix(name, ".git") } +// ValidateSpindle normalizes a user-typed spindle host. Empty means "no spindle". +// +// Membership is enforced by the spindle itself, so this only checks that the value +// is a host the appview can safely send service-auth requests to. +func ValidateSpindle(raw string, dev bool) (string, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", nil + } + + host, noTLS, err := hostutil.ParseHostname(raw) + if err != nil { + return "", fmt.Errorf("%q is not a valid spindle host", raw) + } + + // ParseHostname allows localhost:PORT, which would make the appview dial itself + if noTLS && !dev { + return "", fmt.Errorf("spindle must be a public https host") + } + + return host, nil +} + type RepoGroup struct { Repo *Repo Issues []Issue diff --git a/appview/models/repo_test.go b/appview/models/repo_test.go index d534ceb78..3f64b7d7a 100644 --- a/appview/models/repo_test.go +++ b/appview/models/repo_test.go @@ -104,3 +104,36 @@ func TestRepoSlug(t *testing.T) { }) } } + +func TestValidateSpindle(t *testing.T) { + cases := []struct { + name string + raw string + dev bool + want string + wantErr bool + }{ + {"empty means no spindle", "", false, "", false}, + {"whitespace only", " ", false, "", false}, + {"bare hostname", "spindle.example.com", false, "spindle.example.com", false}, + {"scheme and trailing slash stripped", " https://Spindle.Example.com/ ", false, "spindle.example.com", false}, + {"localhost in dev", "localhost:6555", true, "localhost:6555", false}, + {"localhost in prod", "localhost:6555", false, "", true}, + {"plain http in prod", "http://spindle.example.com", false, "", true}, + {"link-local ip", "169.254.169.254", false, "", true}, + {"port on public host", "spindle.example.com:8443", false, "", true}, + {"single word", "spindle", false, "", true}, + {"not a host", "not a host", false, "", true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := ValidateSpindle(c.raw, c.dev) + if (err != nil) != c.wantErr { + t.Fatalf("ValidateSpindle(%q, %v) error = %v, wantErr %v", c.raw, c.dev, err, c.wantErr) + } + if got != c.want { + t.Errorf("ValidateSpindle(%q, %v) = %q, want %q", c.raw, c.dev, got, c.want) + } + }) + } +} diff --git a/appview/pages/compose_parse_test.go b/appview/pages/compose_parse_test.go index 9735ec12b..e8f8beaa7 100644 --- a/appview/pages/compose_parse_test.go +++ b/appview/pages/compose_parse_test.go @@ -10,13 +10,14 @@ import ( "tangled.org/core/appview/config" "tangled.org/core/appview/models" "tangled.org/core/appview/pages/repoinfo" + "tangled.org/core/idresolver" "tangled.org/core/patchutil" "tangled.org/core/types" ) func TestPullComposeTemplatesParse(t *testing.T) { cfg := &config.Config{} - p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) + p := NewPages(cfg, idresolver.DefaultResolver("https://plc.test"), nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) cases := []struct { name string @@ -45,7 +46,7 @@ func TestPullComposeTemplatesParse(t *testing.T) { func TestPullComposeHostRender(t *testing.T) { cfg := &config.Config{} - p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) + p := NewPages(cfg, idresolver.DefaultResolver("https://plc.test"), nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) base := RepoNewPullParams{ RepoInfo: repoinfo.RepoInfo{ @@ -80,7 +81,7 @@ func TestPullComposeHostRender(t *testing.T) { func TestPullComposeHostRenderWithData(t *testing.T) { cfg := &config.Config{} - p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) + p := NewPages(cfg, idresolver.DefaultResolver("https://plc.test"), nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) sampleBranches := []types.Branch{ {Reference: types.Reference{Name: "feature"}}, @@ -209,7 +210,7 @@ index 0000000..1111111 100644 func TestPullComposeLabelStateRoundTrip(t *testing.T) { cfg := &config.Config{} - p := NewPages(cfg, nil, nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) + p := NewPages(cfg, idresolver.DefaultResolver("https://plc.test"), nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) sampleBranches := []types.Branch{ {Reference: types.Reference{Name: "feature"}}, diff --git a/appview/pages/ratchet_test.go b/appview/pages/ratchet_test.go index 635042a01..8bc49a112 100644 --- a/appview/pages/ratchet_test.go +++ b/appview/pages/ratchet_test.go @@ -44,7 +44,6 @@ func TestNoRepoRkeyInTemplates(t *testing.T) { var bareDidAllowlist = map[string]bool{ "templates/strings/string.html": true, "templates/strings/fragments/form.html": true, - "templates/spindles/dashboard.html": true, } var didCloseAsUrlSegment = regexp.MustCompile(`\.(?:Did|OwnerDid)\s*\}\}\s*/`) diff --git a/appview/pages/spindle_input_test.go b/appview/pages/spindle_input_test.go new file mode 100644 index 000000000..43da84845 --- /dev/null +++ b/appview/pages/spindle_input_test.go @@ -0,0 +1,88 @@ +package pages + +import ( + "bytes" + "io" + "log/slog" + "strings" + "testing" + + "tangled.org/core/appview/config" + "tangled.org/core/appview/oauth" + "tangled.org/core/appview/pages/repoinfo" + "tangled.org/core/idresolver" +) + +// the spindle picker is free text with a datalist of recently used spindles, and +// is shared by repo-new, repo-fork and repo-settings/pipelines. Rendering catches +// what parsing can't: a mistyped fragment name or dict key only fails on execute. +func TestSpindleInputRenders(t *testing.T) { + cfg := &config.Config{} + p := NewPages(cfg, idresolver.DefaultResolver("https://plc.test"), nil, nil, slog.New(slog.NewTextHandler(io.Discard, nil))) + + cases := []struct { + name string + stack []string + define string + params any + want []string + }{ + { + name: "repo/new", + stack: []string{"repo/new"}, + define: "spindle", + params: NewRepoParams{Spindles: []string{"spindle.example.com"}}, + want: []string{`list="spindle-options"`, `