From c517c72bca7b29a47ec7f8a4841c58ab092adc1b Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Wed, 10 Jun 2026 15:42:49 +0000 Subject: [PATCH] Update `NewClient` to accept a pointer to `Options` instead of a value and add unit tests --- client/mirror/client.go | 4 +- client/mirror/client_test.go | 234 +++++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 2 deletions(-) diff --git a/client/mirror/client.go b/client/mirror/client.go index c106541b0..c76b2b22d 100644 --- a/client/mirror/client.go +++ b/client/mirror/client.go @@ -131,12 +131,12 @@ type Client struct { } // NewClient creates a new Client with the provided options. -func NewClient(_ context.Context, opts Options) (*Client, error) { +func NewClient(_ context.Context, opts *Options) (*Client, error) { if err := opts.validate(); err != nil { return nil, err } - return &Client{opts: &opts}, nil + return &Client{opts: opts}, nil } // ErrConflict is returned by tlog-mirror client operations when the mirror returns a 409 Conflict. diff --git a/client/mirror/client_test.go b/client/mirror/client_test.go index 67e194310..66e77262b 100644 --- a/client/mirror/client_test.go +++ b/client/mirror/client_test.go @@ -16,10 +16,15 @@ package mirror import ( "bytes" + "context" "encoding/base64" "errors" "io" + "net/http" + "net/url" "testing" + + "github.com/transparency-dev/tessera/client" ) func TestParseConflict(t *testing.T) { @@ -169,3 +174,232 @@ func TestBuildCheckpointRequestBody(t *testing.T) { }) } } + +// TestNewOptions tests that NewOptions returns an Options struct with correct default values. +func TestNewOptions(t *testing.T) { + opts := NewOptions() + if opts.httpClient != http.DefaultClient { + t.Errorf("NewOptions().httpClient = %v, want http.DefaultClient", opts.httpClient) + } + if opts.mirrorURL != nil { + t.Errorf("NewOptions().mirrorURL = %v, want nil", opts.mirrorURL) + } + if opts.logOrigin != "" { + t.Errorf("NewOptions().logOrigin = %q, want empty", opts.logOrigin) + } + if opts.tileFetcher != nil { + t.Errorf("NewOptions().tileFetcher = %v, want nil", opts.tileFetcher) + } + if opts.bundleFetcher != nil { + t.Errorf("NewOptions().bundleFetcher = %v, want nil", opts.bundleFetcher) + } + if opts.mirrorCheckpointFetcher != nil { + t.Errorf("NewOptions().mirrorCheckpointFetcher = %v, want nil", opts.mirrorCheckpointFetcher) + } + if opts.packageProver != nil { + t.Errorf("NewOptions().packageProver = %v, want nil", opts.packageProver) + } +} + +// TestOptionsBuilders tests that each of the With... builder methods correctly sets the corresponding field in Options. +func TestOptionsBuilders(t *testing.T) { + u, err := url.Parse("https://example.com") + if err != nil { + t.Fatalf("failed to parse test URL: %v", err) + } + httpClient := &http.Client{} + logOrigin := "test-origin" + var tileFetcher client.TileFetcherFunc = func(ctx context.Context, level, index uint64, p uint8) ([]byte, error) { + return nil, nil + } + var bundleFetcher client.EntryBundleFetcherFunc = func(ctx context.Context, index uint64, size uint8) ([]byte, error) { + return nil, nil + } + var mirrorCheckpointFetcher client.CheckpointFetcherFunc = func(ctx context.Context) ([]byte, error) { + return nil, nil + } + var packageProver PackageProverFunc = func(ctx context.Context, start, end uint64) ([][]byte, error) { + return nil, nil + } + + opts := NewOptions(). + WithMirrorURL(u). + WithHTTPClient(httpClient). + WithLogOrigin(logOrigin). + WithTileFetcher(tileFetcher). + WithBundleFetcher(bundleFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher). + WithPackageProver(packageProver) + + if opts.mirrorURL != u { + t.Errorf("WithMirrorURL() = %v, want %v", opts.mirrorURL, u) + } + if opts.httpClient != httpClient { + t.Errorf("WithHTTPClient() = %v, want %v", opts.httpClient, httpClient) + } + if opts.logOrigin != logOrigin { + t.Errorf("WithLogOrigin() = %q, want %q", opts.logOrigin, logOrigin) + } + if opts.tileFetcher == nil { + t.Errorf("WithTileFetcher() tileFetcher is nil") + } + if opts.bundleFetcher == nil { + t.Errorf("WithBundleFetcher() bundleFetcher is nil") + } + if opts.mirrorCheckpointFetcher == nil { + t.Errorf("WithMirrorCheckpointFetcher() mirrorCheckpointFetcher is nil") + } + if opts.packageProver == nil { + t.Errorf("WithPackageProver() packageProver is nil") + } +} + +// TestNewClientValidation tests that NewClient validates the provided Options and returns errors when required fields are missing. +func TestNewClientValidation(t *testing.T) { + u, _ := url.Parse("https://example.com") + httpClient := &http.Client{} + logOrigin := "test-origin" + var tileFetcher client.TileFetcherFunc = func(ctx context.Context, level, index uint64, p uint8) ([]byte, error) { return nil, nil } + var bundleFetcher client.EntryBundleFetcherFunc = func(ctx context.Context, index uint64, size uint8) ([]byte, error) { return nil, nil } + var mirrorCheckpointFetcher client.CheckpointFetcherFunc = func(ctx context.Context) ([]byte, error) { return nil, nil } + var packageProver PackageProverFunc = func(ctx context.Context, start, end uint64) ([][]byte, error) { return nil, nil } + + tests := []struct { + desc string + setup func() *Options + wantErr string + }{ + { + desc: "valid options", + setup: func() *Options { + return NewOptions(). + WithMirrorURL(u). + WithHTTPClient(httpClient). + WithLogOrigin(logOrigin). + WithTileFetcher(tileFetcher). + WithBundleFetcher(bundleFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher). + WithPackageProver(packageProver) + }, + }, + { + desc: "missing mirror URL", + setup: func() *Options { + return NewOptions(). + WithHTTPClient(httpClient). + WithLogOrigin(logOrigin). + WithTileFetcher(tileFetcher). + WithBundleFetcher(bundleFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher). + WithPackageProver(packageProver) + }, + wantErr: "mirror URL is required", + }, + { + desc: "missing HTTP client", + setup: func() *Options { + opts := NewOptions(). + WithMirrorURL(u). + WithLogOrigin(logOrigin). + WithTileFetcher(tileFetcher). + WithBundleFetcher(bundleFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher). + WithPackageProver(packageProver) + opts.httpClient = nil + return opts + }, + wantErr: "HTTP client is required", + }, + { + desc: "missing log origin", + setup: func() *Options { + return NewOptions(). + WithMirrorURL(u). + WithHTTPClient(httpClient). + WithTileFetcher(tileFetcher). + WithBundleFetcher(bundleFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher). + WithPackageProver(packageProver) + }, + wantErr: "log origin is required", + }, + { + desc: "missing tile fetcher", + setup: func() *Options { + return NewOptions(). + WithMirrorURL(u). + WithHTTPClient(httpClient). + WithLogOrigin(logOrigin). + WithBundleFetcher(bundleFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher). + WithPackageProver(packageProver) + }, + wantErr: "tile fetcher is required", + }, + { + desc: "missing bundle fetcher", + setup: func() *Options { + return NewOptions(). + WithMirrorURL(u). + WithHTTPClient(httpClient). + WithLogOrigin(logOrigin). + WithTileFetcher(tileFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher). + WithPackageProver(packageProver) + }, + wantErr: "bundle fetcher is required", + }, + { + desc: "missing mirror checkpoint fetcher", + setup: func() *Options { + return NewOptions(). + WithMirrorURL(u). + WithHTTPClient(httpClient). + WithLogOrigin(logOrigin). + WithTileFetcher(tileFetcher). + WithBundleFetcher(bundleFetcher). + WithPackageProver(packageProver) + }, + wantErr: "mirror checkpoint fetcher is required", + }, + { + desc: "missing package prover", + setup: func() *Options { + return NewOptions(). + WithMirrorURL(u). + WithHTTPClient(httpClient). + WithLogOrigin(logOrigin). + WithTileFetcher(tileFetcher). + WithBundleFetcher(bundleFetcher). + WithMirrorCheckpointFetcher(mirrorCheckpointFetcher) + }, + wantErr: "package prover is required", + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + opts := tc.setup() + ctx := context.Background() + client, err := NewClient(ctx, opts) + if tc.wantErr != "" { + if err == nil { + t.Fatalf("NewClient() succeeded, want error containing %q", tc.wantErr) + } + if got, want := err.Error(), tc.wantErr; got != want { + t.Errorf("NewClient() err = %q, want %q", got, want) + } + if client != nil { + t.Errorf("NewClient() returned non-nil client on error: %v", client) + } + } else { + if err != nil { + t.Fatalf("NewClient() failed: %v", err) + } + if client == nil { + t.Errorf("NewClient() returned nil client on success") + } + } + }) + } +}