From ad4c3d91f107df16b92390e67b2502ba55307c91 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 14 Oct 2026 19:08:58 +0200 Subject: [PATCH] Show the prefix page's origin names from the prefix query (closes #67) The prefix page looked up each origin AS's details only to show its handle and description, and that lookup reads every live route the AS originates. `GetPrefixDetailsContext` already selected the handle and description and dropped them; it now returns them keyed by AS number, and the page uses them. The origin list now also shows the description after the handle, which it never rendered. The JSON prefix answer is unchanged. The test serves the page from a store whose AS lookup always fails. Model: opus-5-5 --- internal/database/database.go | 21 ++++--- internal/database/interface.go | 4 +- internal/routewatch/app_integration_test.go | 8 ++- internal/routewatch/prefixhandler_test.go | 2 +- internal/server/handlers.go | 16 ++--- internal/server/handlers_test.go | 70 +++++++++++++++++++++ internal/templates/prefix_detail.html | 1 + 7 files changed, 97 insertions(+), 25 deletions(-) diff --git a/internal/database/database.go b/internal/database/database.go index ed62eb8..468bd2b 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -1632,16 +1632,17 @@ func (d *Database) GetASPeersContext(ctx context.Context, asn int) ([]ASPeer, er } // GetPrefixDetails returns detailed information about a prefix -func (d *Database) GetPrefixDetails(prefix string) ([]LiveRoute, error) { +func (d *Database) GetPrefixDetails(prefix string) ([]LiveRoute, map[int]ASN, error) { return d.GetPrefixDetailsContext(context.Background(), prefix) } -// GetPrefixDetailsContext returns detailed information about a prefix with context support -func (d *Database) GetPrefixDetailsContext(ctx context.Context, prefix string) ([]LiveRoute, error) { +// GetPrefixDetailsContext returns the live routes for a prefix and, keyed by +// AS number, the handle and description of each AS that originates it. +func (d *Database) GetPrefixDetailsContext(ctx context.Context, prefix string) ([]LiveRoute, map[int]ASN, error) { // Determine if it's IPv4 or IPv6 by parsing the prefix _, ipnet, err := net.ParseCIDR(prefix) if err != nil { - return nil, fmt.Errorf("invalid prefix format: %w", err) + return nil, nil, fmt.Errorf("invalid prefix format: %w", err) } tableName := "live_routes_v4" @@ -1663,11 +1664,12 @@ func (d *Database) GetPrefixDetailsContext(ctx context.Context, prefix string) ( rows, err := d.db.QueryContext(ctx, query, prefix) if err != nil { - return nil, fmt.Errorf("failed to query prefix details: %w", err) + return nil, nil, fmt.Errorf("failed to query prefix details: %w", err) } defer func() { _ = rows.Close() }() var routes []LiveRoute + origins := make(map[int]ASN) for rows.Next() { var route LiveRoute var pathJSON string @@ -1690,13 +1692,18 @@ func (d *Database) GetPrefixDetailsContext(ctx context.Context, prefix string) ( route.IPVersion = ipVersion route.MaskLength, _ = ipnet.Mask.Size() routes = append(routes, route) + origins[route.OriginASN] = ASN{ + ASN: route.OriginASN, + Handle: handle.String, + Description: description.String, + } } if len(routes) == 0 { - return nil, fmt.Errorf("%w: %s", ErrNoRoute, prefix) + return nil, nil, fmt.Errorf("%w: %s", ErrNoRoute, prefix) } - return routes, nil + return routes, origins, nil } // GetRandomPrefixesByLength returns a random sample of prefixes with the specified mask length diff --git a/internal/database/interface.go b/internal/database/interface.go index 89cd4f0..504c252 100644 --- a/internal/database/interface.go +++ b/internal/database/interface.go @@ -76,8 +76,8 @@ type Store interface { GetASDetailsContext(ctx context.Context, asn int) (*ASN, []LiveRoute, error) GetASPeers(asn int) ([]ASPeer, error) GetASPeersContext(ctx context.Context, asn int) ([]ASPeer, error) - GetPrefixDetails(prefix string) ([]LiveRoute, error) - GetPrefixDetailsContext(ctx context.Context, prefix string) ([]LiveRoute, error) + GetPrefixDetails(prefix string) ([]LiveRoute, map[int]ASN, error) + GetPrefixDetailsContext(ctx context.Context, prefix string) ([]LiveRoute, map[int]ASN, error) GetRandomPrefixesByLength(maskLength, ipVersion, limit int) ([]LiveRoute, error) GetRandomPrefixesByLengthContext(ctx context.Context, maskLength, ipVersion, limit int) ([]LiveRoute, error) diff --git a/internal/routewatch/app_integration_test.go b/internal/routewatch/app_integration_test.go index d53c9fe..a359d03 100644 --- a/internal/routewatch/app_integration_test.go +++ b/internal/routewatch/app_integration_test.go @@ -252,13 +252,15 @@ func (m *mockStore) GetASDetailsContext(ctx context.Context, asn int) (*database } // GetPrefixDetails mock implementation -func (m *mockStore) GetPrefixDetails(prefix string) ([]database.LiveRoute, error) { +func (m *mockStore) GetPrefixDetails(prefix string) ([]database.LiveRoute, map[int]database.ASN, error) { // Return empty routes for now - return []database.LiveRoute{}, nil + return []database.LiveRoute{}, map[int]database.ASN{}, nil } // GetPrefixDetailsContext mock implementation with context support -func (m *mockStore) GetPrefixDetailsContext(ctx context.Context, prefix string) ([]database.LiveRoute, error) { +func (m *mockStore) GetPrefixDetailsContext( + ctx context.Context, prefix string, +) ([]database.LiveRoute, map[int]database.ASN, error) { return m.GetPrefixDetails(prefix) } diff --git a/internal/routewatch/prefixhandler_test.go b/internal/routewatch/prefixhandler_test.go index 0d30e77..fcfa9a3 100644 --- a/internal/routewatch/prefixhandler_test.go +++ b/internal/routewatch/prefixhandler_test.go @@ -170,7 +170,7 @@ func TestPrefixHandlerAppliesEachPeersLastUpdateInABatch(t *testing.T) { } ctx := context.Background() - routes, err := db.GetPrefixDetailsContext(ctx, prefix) + routes, _, err := db.GetPrefixDetailsContext(ctx, prefix) if err != nil && !errors.Is(err, database.ErrNoRoute) { t.Fatalf("GetPrefixDetailsContext: %v", err) } diff --git a/internal/server/handlers.go b/internal/server/handlers.go index eed2271..0856bd2 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -759,7 +759,7 @@ func (s *Server) handlePrefixDetailJSON() http.HandlerFunc { } prefix := parsed.Masked().String() - routes, err := s.db.GetPrefixDetailsContext(r.Context(), prefix) + routes, _, err := s.db.GetPrefixDetailsContext(r.Context(), prefix) if err != nil { if errors.Is(err, database.ErrNoRoute) { writeJSONError(w, http.StatusNotFound, err.Error()) @@ -930,7 +930,7 @@ func (s *Server) handlePrefixDetail() http.HandlerFunc { } prefix := parsed.Masked().String() - routes, err := s.db.GetPrefixDetailsContext(r.Context(), prefix) + routes, originASNs, err := s.db.GetPrefixDetailsContext(r.Context(), prefix) if err != nil { if errors.Is(err, database.ErrNoRoute) { http.Error(w, "Prefix not found", http.StatusNotFound) @@ -952,18 +952,10 @@ func (s *Server) handlePrefixDetail() http.HandlerFunc { originMap := make(map[int]*ASNInfo) for _, route := range routes { if _, exists := originMap[route.OriginASN]; !exists { - // Get AS info from database - asInfo, _, _ := s.db.GetASDetailsContext(r.Context(), route.OriginASN) - handle := "" - description := "" - if asInfo != nil { - handle = asInfo.Handle - description = asInfo.Description - } originMap[route.OriginASN] = &ASNInfo{ ASN: route.OriginASN, - Handle: handle, - Description: description, + Handle: originASNs[route.OriginASN].Handle, + Description: originASNs[route.OriginASN].Description, PeerCount: 0, } } diff --git a/internal/server/handlers_test.go b/internal/server/handlers_test.go index 927938c..3821654 100644 --- a/internal/server/handlers_test.go +++ b/internal/server/handlers_test.go @@ -3,10 +3,13 @@ package server import ( "context" "encoding/json" + "errors" + "html" "net/http" "net/http/httptest" "runtime" "slices" + "strings" "testing" "time" @@ -15,6 +18,7 @@ import ( "git.eeqj.de/sneak/routewatch/internal/logger" "git.eeqj.de/sneak/routewatch/internal/metrics" "git.eeqj.de/sneak/routewatch/internal/streamer" + "git.eeqj.de/sneak/routewatch/pkg/asinfo" "github.com/google/uuid" ) @@ -200,6 +204,72 @@ func TestPrefixDetailHandlersParsePrefix(t *testing.T) { } } +// failingASDetailsDB wraps a real store and fails every AS details lookup. +type failingASDetailsDB struct { + database.Store +} + +func (failingASDetailsDB) GetASDetailsContext(_ context.Context, _ int) (*database.ASN, []database.LiveRoute, error) { + return nil, nil, errors.New("AS details lookup failed") +} + +// TestPrefixDetailShowsOriginNames checks that the prefix page shows each +// origin AS's handle and description without looking up the AS's details, +// which also read every route the AS originates. Before the fix the page did +// that lookup once per origin, so with it failing the names were missing +// (https://git.eeqj.de/sneak/routewatch/issues/67). The AS path column shows +// handles too, so the test looks for the origin list's "(handle)" form. +func TestPrefixDetailShowsOriginNames(t *testing.T) { + db, err := database.New(&config.Config{StateDir: t.TempDir()}, logger.New()) + if err != nil { + t.Fatalf("database.New: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + + ts := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + origins := []int{15169, 13335} + for _, asn := range origins { + if _, err := db.GetOrCreateASN(asn, ts); err != nil { + t.Fatalf("GetOrCreateASN(%d): %v", asn, err) + } + } + if err := db.UpsertLiveRouteBatch([]*database.LiveRoute{ + { + ID: uuid.New(), Prefix: "192.0.2.0/24", MaskLength: 24, IPVersion: 4, + OriginASN: origins[0], PeerIP: "198.51.100.1", ASPath: []int{origins[0]}, NextHop: "198.51.100.1", + LastUpdated: ts, + }, + { + ID: uuid.New(), Prefix: "192.0.2.0/24", MaskLength: 24, IPVersion: 4, + OriginASN: origins[1], PeerIP: "198.51.100.2", ASPath: []int{origins[1]}, NextHop: "198.51.100.2", + LastUpdated: ts, + }, + }); err != nil { + t.Fatalf("UpsertLiveRouteBatch: %v", err) + } + + s := New(failingASDetailsDB{db}, streamer.New(logger.New(), metrics.New()), logger.New(), &config.Config{}) + rec := httptest.NewRecorder() + s.router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/prefix/192.0.2.0/24", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("status %d, want %d; body %s", rec.Code, http.StatusOK, rec.Body) + } + + page := rec.Body.String() + for _, asn := range origins { + info, ok := asinfo.Get(asn) + if !ok { + t.Fatalf("AS%d is not in the AS info data", asn) + } + if want := "(" + html.EscapeString(info.Handle) + ")"; !strings.Contains(page, want) { + t.Errorf("page does not show AS%d's handle %q", asn, want) + } + if want := html.EscapeString(info.Description); !strings.Contains(page, want) { + t.Errorf("page does not show AS%d's description %q", asn, want) + } + } +} + // settledGoroutineCount lets transient goroutines finish, then reports the // current count. func settledGoroutineCount() int { diff --git a/internal/templates/prefix_detail.html b/internal/templates/prefix_detail.html index 5ab9bf3..cb12cb8 100644 --- a/internal/templates/prefix_detail.html +++ b/internal/templates/prefix_detail.html @@ -283,6 +283,7 @@
AS{{.ASN}} {{if .Handle}} ({{.Handle}}){{end}} + {{if .Description}} {{.Description}}{{end}} {{.PeerCount}} peer{{if ne .PeerCount 1}}s{{end}}
{{end}}