From aad9ebfc8b2c1ec12457b1c279f04e026e623581 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Mon, 9 Feb 2026 22:39:38 -0600 Subject: [PATCH] fix lint and unit tests --- pkg/atproto/client.go | 12 ++++++------ pkg/hold/admin/auth.go | 16 ---------------- pkg/hold/admin/handlers_crew_io.go | 5 ++++- pkg/hold/config.go | 1 - pkg/hold/config_test.go | 6 ++++++ pkg/hold/metadata.go | 5 ++++- pkg/hold/pds/captain_test.go | 1 + pkg/hold/pds/delete_test.go | 12 ++++-------- pkg/hold/pds/layer_test.go | 27 +++++++++------------------ pkg/hold/pds/server.go | 6 ++++++ pkg/hold/pds/status_test.go | 2 +- 11 files changed, 41 insertions(+), 52 deletions(-) diff --git a/pkg/atproto/client.go b/pkg/atproto/client.go index bf700bf..dd2bb6b 100644 --- a/pkg/atproto/client.go +++ b/pkg/atproto/client.go @@ -104,7 +104,7 @@ func NewClient(pdsEndpoint, did, accessToken string) *Client { pdsEndpoint: pdsEndpoint, did: did, clientProvider: NewBasicAuthClientProvider(pdsEndpoint, did, accessToken), - httpClient: &http.Client{}, + httpClient: &http.Client{Timeout: 30 * time.Second}, } } @@ -121,7 +121,7 @@ func NewClientWithSessionProvider(pdsEndpoint, did string, sessionProvider Sessi pdsEndpoint: pdsEndpoint, did: did, clientProvider: NewOAuthClientProvider(sessionProvider), - httpClient: &http.Client{}, + httpClient: &http.Client{Timeout: 30 * time.Second}, } } @@ -259,7 +259,7 @@ func (c *Client) UploadBlob(ctx context.Context, data []byte, mimeType string) ( // Note: This is a sync endpoint that returns raw binary data func (c *Client) GetBlob(ctx context.Context, cid string) ([]byte, error) { // Public endpoint - no auth required - client := &xrpc.Client{Host: c.pdsEndpoint} + client := &xrpc.Client{Host: c.pdsEndpoint, Client: c.httpClient} data, err := comatproto.SyncGetBlob(ctx, client, cid, c.did) if err != nil { var xrpcErr *xrpc.Error @@ -294,7 +294,7 @@ func (c *Client) ListReposByCollection(ctx context.Context, collection string, l } // Public endpoint - no auth required - client := &xrpc.Client{Host: c.pdsEndpoint} + client := &xrpc.Client{Host: c.pdsEndpoint, Client: c.httpClient} var result ListReposByCollectionResult err := client.LexDo(ctx, "GET", "", "com.atproto.sync.listReposByCollection", params, nil, &result) if err != nil { @@ -318,7 +318,7 @@ func (c *Client) ListRecordsForRepo(ctx context.Context, repoDID, collection str } // Public endpoint - no auth required - client := &xrpc.Client{Host: c.pdsEndpoint} + client := &xrpc.Client{Host: c.pdsEndpoint, Client: c.httpClient} var result struct { Records []Record `json:"records"` Cursor string `json:"cursor,omitempty"` @@ -352,7 +352,7 @@ type ProfileRecord struct { // The actor parameter can be a DID or handle func (c *Client) GetActorProfile(ctx context.Context, actor string) (*ActorProfile, error) { // Public endpoint - doesn't require auth - client := &xrpc.Client{Host: c.pdsEndpoint} + client := &xrpc.Client{Host: c.pdsEndpoint, Client: c.httpClient} resp, err := appbsky.ActorGetProfile(ctx, client, actor) if err != nil { diff --git a/pkg/hold/admin/auth.go b/pkg/hold/admin/auth.go index 003a356..7356bdf 100644 --- a/pkg/hold/admin/auth.go +++ b/pkg/hold/admin/auth.go @@ -112,19 +112,3 @@ func (ui *AdminUI) renderTemplate(w http.ResponseWriter, name string, data any) http.Error(w, "Internal server error", http.StatusInternalServerError) } } - -// renderError renders an error page -func (ui *AdminUI) renderError(w http.ResponseWriter, r *http.Request, message string, statusCode int) { - w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.WriteHeader(statusCode) - - data := struct { - PageData - Error string - }{ - PageData: ui.newPageData(r, "Error", ""), - Error: message, - } - - ui.renderTemplate(w, "pages/error.html", data) -} diff --git a/pkg/hold/admin/handlers_crew_io.go b/pkg/hold/admin/handlers_crew_io.go index 30c38ac..4c70f29 100644 --- a/pkg/hold/admin/handlers_crew_io.go +++ b/pkg/hold/admin/handlers_crew_io.go @@ -67,7 +67,10 @@ func (ui *AdminUI) handleCrewExport(w http.ResponseWriter, r *http.Request) { enc := json.NewEncoder(w) enc.SetIndent("", " ") - enc.Encode(export) + if err := enc.Encode(export); err != nil { + slog.Error("Failed to encode crew export", "error", err) + return + } session := getSessionFromContext(ctx) slog.Info("Crew exported via admin panel", diff --git a/pkg/hold/config.go b/pkg/hold/config.go index 476a3f9..51b041f 100644 --- a/pkg/hold/config.go +++ b/pkg/hold/config.go @@ -292,4 +292,3 @@ func buildStorageConfigFromFields(sc StorageConfig) configuration.Storage { return storageCfg } - diff --git a/pkg/hold/config_test.go b/pkg/hold/config_test.go index 3dd3327..82b9320 100644 --- a/pkg/hold/config_test.go +++ b/pkg/hold/config_test.go @@ -8,6 +8,12 @@ import ( "time" ) +func init() { + // Point metadata endpoint to a closed listener so it fails instantly instead of + // waiting 2s for the real 169.254.169.254 to timeout on non-cloud machines. + metadataEndpoint = "http://127.0.0.1:1" +} + // setupEnv sets environment variables for testing and returns a cleanup function func setupEnv(t *testing.T, vars map[string]string) func() { // Save original env diff --git a/pkg/hold/metadata.go b/pkg/hold/metadata.go index e4d5a2c..4cf5e9b 100644 --- a/pkg/hold/metadata.go +++ b/pkg/hold/metadata.go @@ -13,6 +13,9 @@ type CloudMetadata struct { Region string } +// metadataEndpoint is the cloud metadata service URL. Package-level var for test override. +var metadataEndpoint = "http://169.254.169.254" + // DetectCloudMetadata queries the instance metadata service (169.254.169.254) // Currently supports UpCloud. Others can be added via PR. func DetectCloudMetadata(ctx context.Context) (*CloudMetadata, error) { @@ -32,7 +35,7 @@ func DetectCloudMetadata(ctx context.Context) (*CloudMetadata, error) { // detectUpCloud queries UpCloud's metadata service func detectUpCloud(ctx context.Context) (*CloudMetadata, error) { - req, err := http.NewRequestWithContext(ctx, "GET", "http://169.254.169.254/metadata/v1.json", nil) + req, err := http.NewRequestWithContext(ctx, "GET", metadataEndpoint+"/metadata/v1.json", nil) if err != nil { return nil, err } diff --git a/pkg/hold/pds/captain_test.go b/pkg/hold/pds/captain_test.go index 0bb463c..4170ff0 100644 --- a/pkg/hold/pds/captain_test.go +++ b/pkg/hold/pds/captain_test.go @@ -40,6 +40,7 @@ func setupTestPDS(t *testing.T) (*HoldPDS, context.Context) { t.Fatalf("Failed to initialize test repo: %v", err) } + t.Cleanup(func() { pds.Close() }) return pds, ctx } diff --git a/pkg/hold/pds/delete_test.go b/pkg/hold/pds/delete_test.go index 07698fc..4c77f4e 100644 --- a/pkg/hold/pds/delete_test.go +++ b/pkg/hold/pds/delete_test.go @@ -150,8 +150,7 @@ func TestPostMentionsUser(t *testing.T) { func TestDeleteBlueskyPosts_NoPosts(t *testing.T) { // Create a test PDS with records index - pds, cleanup := setupTestPDSWithIndex(t, "did:plc:testowner") - defer cleanup() + pds := setupTestPDSWithIndex(t, "did:plc:testowner") ctx := sharedCtx @@ -168,8 +167,7 @@ func TestDeleteBlueskyPosts_NoPosts(t *testing.T) { func TestListBlueskyPostsForUser_NoPosts(t *testing.T) { // Create a test PDS with records index - pds, cleanup := setupTestPDSWithIndex(t, "did:plc:testowner") - defer cleanup() + pds := setupTestPDSWithIndex(t, "did:plc:testowner") ctx := sharedCtx @@ -186,8 +184,7 @@ func TestListBlueskyPostsForUser_NoPosts(t *testing.T) { func TestDeleteAndListBlueskyPosts_WithPosts(t *testing.T) { // Create a test PDS with records index - pds, cleanup := setupTestPDSWithIndex(t, "did:plc:testowner") - defer cleanup() + pds := setupTestPDSWithIndex(t, "did:plc:testowner") ctx := sharedCtx @@ -276,8 +273,7 @@ func TestDeleteAndListBlueskyPosts_WithPosts(t *testing.T) { func TestDeleteUserData_IncludesPosts(t *testing.T) { // Create a test PDS with records index - pds, cleanup := setupTestPDSWithIndex(t, "did:plc:testowner") - defer cleanup() + pds := setupTestPDSWithIndex(t, "did:plc:testowner") ctx := sharedCtx diff --git a/pkg/hold/pds/layer_test.go b/pkg/hold/pds/layer_test.go index d054f2e..f0d6c79 100644 --- a/pkg/hold/pds/layer_test.go +++ b/pkg/hold/pds/layer_test.go @@ -287,7 +287,7 @@ func TestLayerRecord_FieldValidation(t *testing.T) { // setupTestPDSWithIndex creates a PDS with file-based database (enables RecordsIndex) // and bootstraps it with the given owner. Required for quota tests. -func setupTestPDSWithIndex(t *testing.T, ownerDID string) (*HoldPDS, func()) { +func setupTestPDSWithIndex(t *testing.T, ownerDID string) *HoldPDS { t.Helper() ctx := sharedCtx @@ -321,11 +321,8 @@ func setupTestPDSWithIndex(t *testing.T, ownerDID string) (*HoldPDS, func()) { t.Fatalf("Failed to backfill records index: %v", err) } - cleanup := func() { - pds.Close() - } - - return pds, cleanup + t.Cleanup(func() { pds.Close() }) + return pds } // addCrewMemberWithTier adds a crew member with a specific tier @@ -349,8 +346,7 @@ func addCrewMemberWithTier(t *testing.T, pds *HoldPDS, memberDID, role string, p func TestGetQuotaForUserWithTier_OwnerUnlimited(t *testing.T) { ownerDID := "did:plc:owner123" - pds, cleanup := setupTestPDSWithIndex(t, ownerDID) - defer cleanup() + pds := setupTestPDSWithIndex(t, ownerDID) ctx := sharedCtx @@ -423,8 +419,7 @@ defaults: func TestGetQuotaForUserWithTier_CrewWithDefaultTier(t *testing.T) { ownerDID := "did:plc:owner456" crewDID := "did:plc:crew123" - pds, cleanup := setupTestPDSWithIndex(t, ownerDID) - defer cleanup() + pds := setupTestPDSWithIndex(t, ownerDID) ctx := sharedCtx @@ -498,8 +493,7 @@ defaults: func TestGetQuotaForUserWithTier_CrewWithExplicitTier(t *testing.T) { ownerDID := "did:plc:owner789" crewDID := "did:plc:bosuncrew456" - pds, cleanup := setupTestPDSWithIndex(t, ownerDID) - defer cleanup() + pds := setupTestPDSWithIndex(t, ownerDID) ctx := sharedCtx @@ -566,8 +560,7 @@ defaults: func TestGetQuotaForUserWithTier_NoQuotaManager(t *testing.T) { ownerDID := "did:plc:ownerabc" crewDID := "did:plc:crewabc" - pds, cleanup := setupTestPDSWithIndex(t, ownerDID) - defer cleanup() + pds := setupTestPDSWithIndex(t, ownerDID) ctx := sharedCtx @@ -608,8 +601,7 @@ func TestGetQuotaForUserWithTier_NoQuotaManager(t *testing.T) { func TestGetQuotaForUserWithTier_DisabledQuotas(t *testing.T) { ownerDID := "did:plc:ownerdef" crewDID := "did:plc:crewdef" - pds, cleanup := setupTestPDSWithIndex(t, ownerDID) - defer cleanup() + pds := setupTestPDSWithIndex(t, ownerDID) ctx := sharedCtx @@ -655,8 +647,7 @@ func TestGetQuotaForUserWithTier_DisabledQuotas(t *testing.T) { func TestGetQuotaForUserWithTier_DeduplicatesBlobs(t *testing.T) { ownerDID := "did:plc:ownerghi" crewDID := "did:plc:crewghi" - pds, cleanup := setupTestPDSWithIndex(t, ownerDID) - defer cleanup() + pds := setupTestPDSWithIndex(t, ownerDID) ctx := sharedCtx diff --git a/pkg/hold/pds/server.go b/pkg/hold/pds/server.go index cc68671..f2da17f 100644 --- a/pkg/hold/pds/server.go +++ b/pkg/hold/pds/server.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "io" "log/slog" "os" "path/filepath" @@ -388,6 +389,11 @@ func (p *HoldPDS) Close() error { return fmt.Errorf("failed to close records index: %w", err) } } + if closer, ok := p.carstore.(io.Closer); ok { + if err := closer.Close(); err != nil { + return fmt.Errorf("failed to close carstore: %w", err) + } + } return nil } diff --git a/pkg/hold/pds/status_test.go b/pkg/hold/pds/status_test.go index 15851a7..ef7b49c 100644 --- a/pkg/hold/pds/status_test.go +++ b/pkg/hold/pds/status_test.go @@ -288,6 +288,6 @@ func TestMain(m *testing.M) { // Run tests code := m.Run() - // Cleanup is automatic with t.TempDir() + sharedPDS.Close() os.Exit(code) }