Consistent options implementation

api/v0/httpclient/client.go

@@ -13,6 +13,11 @@ import (
 // New creates a base URL and a new http.Client.  The default port is only used
 // if baseURL does not contain a port.
 func New(baseURL, resource string, options ...Option) (*url.URL, *http.Client, error) {
+	opts, err := getOpts(options)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") {
 		baseURL = "http://" + baseURL
 	}
@@ -26,16 +31,11 @@ func New(baseURL, resource string, options ...Option) (*url.URL, *http.Client, e
 	}
 	u.Path = resource
 
-	var cfg clientConfig
-	if err := cfg.apply(options...); err != nil {
-		return nil, nil, err
-	}
-
-	if cfg.client != nil {
-		return u, cfg.client, nil
+	if opts.client != nil {
+		return u, opts.client, nil
 	}
 	cl := &http.Client{
-		Timeout: cfg.timeout,
+		Timeout: opts.timeout,
 	}
 	return u, cl, nil
 }

api/v0/httpclient/options.go

@@ -6,47 +6,41 @@ import (
 	"time"
 )
 
-type clientConfig struct {
+const defaultTimeout = time.Minute
+
+type config struct {
 	timeout time.Duration
 	client  *http.Client
 }
 
-// Option is the option type for httpclient
-type Option func(*clientConfig) error
-
-var clientDefaults = func(c *clientConfig) error {
-	// As a fallback, never take more than a minute.
-	// Most client API calls should use a context.
-	c.timeout = time.Minute
-	return nil
-}
+// Option is a function that sets a value in a config.
+type Option func(*config) error
 
-// apply applies the given options to this clientConfig
-func (c *clientConfig) apply(opts ...Option) error {
-	err := clientDefaults(c)
-	if err != nil {
-		// Failure of default option should panic
-		panic("default option failed: " + err.Error())
+// getOpts creates a config and applies Options to it.
+func getOpts(opts []Option) (config, error) {
+	cfg := config{
+		timeout: defaultTimeout,
 	}
 	for i, opt := range opts {
-		if err = opt(c); err != nil {
-			return fmt.Errorf("httpclient option %d failed: %s", i, err)
+		if err := opt(&cfg); err != nil {
+			return config{}, fmt.Errorf("option %d failed: %s", i, err)
 		}
 	}
-	return nil
+	return cfg, nil
 }
 
-// Timeout configures the timeout to wait for a response
-func Timeout(timeout time.Duration) Option {
-	return func(cfg *clientConfig) error {
+// WithTimeout configures the timeout to wait for a response.
+func WithTimeout(timeout time.Duration) Option {
+	return func(cfg *config) error {
 		cfg.timeout = timeout
 		return nil
 	}
 }
 
-// WithClient allows creation of the http client using an underlying network round tripper / client
+// WithClient allows creation of the http client using an underlying network
+// round tripper / client.
 func WithClient(c *http.Client) Option {
-	return func(cfg *clientConfig) error {
+	return func(cfg *config) error {
 		cfg.client = c
 		return nil
 	}

assigner/server/option.go

@@ -20,7 +20,7 @@ type config struct {
 type Option func(*config) error
 
 // getOpts creates a config and applies Options to it.
-func getOpts(opts ...Option) (config, error) {
+func getOpts(opts []Option) (config, error) {
 	cfg := config{
 		writeTimeout: defaultWriteTimeout,
 		readTimeout:  defaultReadTimeout,

assigner/server/server.go

@@ -30,7 +30,7 @@ type Server struct {
 }
 
 func New(listen string, assigner *core.Assigner, options ...Option) (*Server, error) {
-	opts, err := getOpts(options...)
+	opts, err := getOpts(options)
 	if err != nil {
 		return nil, err
 	}

command/daemon.go

@@ -178,9 +178,9 @@ func daemonCommand(cctx *cli.Context) error {
 			return fmt.Errorf("bad finder address %s: %s", finderAddr, err)
 		}
 		finderSvr, err = httpfinderserver.New(finderNetAddr.String(), indexerCore, reg,
-			httpfinderserver.ReadTimeout(time.Duration(cfg.Finder.ApiReadTimeout)),
-			httpfinderserver.WriteTimeout(time.Duration(cfg.Finder.ApiWriteTimeout)),
-			httpfinderserver.MaxConnections(cfg.Finder.MaxConnections),
+			httpfinderserver.WithReadTimeout(time.Duration(cfg.Finder.ApiReadTimeout)),
+			httpfinderserver.WithWriteTimeout(time.Duration(cfg.Finder.ApiWriteTimeout)),
+			httpfinderserver.WithMaxConnections(cfg.Finder.MaxConnections),
 			httpfinderserver.WithHomepage(cfg.Finder.Webpage),
 			httpfinderserver.WithIndexCounts(indexCounts),
 		)

dagsync/dtsync/option.go

@@ -14,16 +14,18 @@ type config struct {
 	senders   []announce.Sender
 }
 
+// Option is a function that sets a value in a config.
 type Option func(*config) error
 
-// apply applies the given options to this config.
-func (c *config) apply(opts []Option) error {
+// getOpts creates a config and applies Options to it.
+func getOpts(opts []Option) (config, error) {
+	var cfg config
 	for i, opt := range opts {
-		if err := opt(c); err != nil {
-			return fmt.Errorf("option %d failed: %s", i, err)
+		if err := opt(&cfg); err != nil {
+			return config{}, fmt.Errorf("option %d failed: %s", i, err)
 		}
 	}
-	return nil
+	return cfg, nil
 }
 
 // WithExtraData sets the extra data to include in the pubsub message.

dagsync/dtsync/publisher.go

@@ -31,13 +31,12 @@ type publisher struct {
 
 // NewPublisher creates a new dagsync publisher.
 func NewPublisher(host host.Host, ds datastore.Batching, lsys ipld.LinkSystem, topicName string, options ...Option) (*publisher, error) {
-	cfg := config{}
-	err := cfg.apply(options)
+	opts, err := getOpts(options)
 	if err != nil {
 		return nil, err
 	}
 
-	dtManager, _, dtClose, err := makeDataTransfer(host, ds, lsys, cfg.allowPeer)
+	dtManager, _, dtClose, err := makeDataTransfer(host, ds, lsys, opts.allowPeer)
 	if err != nil {
 		return nil, err
 	}
@@ -48,10 +47,10 @@ func NewPublisher(host host.Host, ds datastore.Batching, lsys ipld.LinkSystem, t
 	return &publisher{
 		dtManager:     dtManager,
 		dtClose:       dtClose,
-		extraData:     cfg.extraData,
+		extraData:     opts.extraData,
 		headPublisher: headPublisher,
 		host:          host,
-		senders:       cfg.senders,
+		senders:       opts.senders,
 	}, nil
 }
 
@@ -70,24 +69,23 @@ func startHeadPublisher(host host.Host, topicName string, headPublisher *head.Pu
 // NewPublisherFromExisting instantiates publishing on an existing
 // data transfer instance.
 func NewPublisherFromExisting(dtManager dt.Manager, host host.Host, topicName string, lsys ipld.LinkSystem, options ...Option) (*publisher, error) {
-	cfg := config{}
-	err := cfg.apply(options)
+	opts, err := getOpts(options)
 	if err != nil {
 		return nil, err
 	}
 
-	err = configureDataTransferForDagsync(context.Background(), dtManager, lsys, cfg.allowPeer)
+	err = configureDataTransferForDagsync(context.Background(), dtManager, lsys, opts.allowPeer)
 	if err != nil {
 		return nil, fmt.Errorf("cannot configure datatransfer: %w", err)
 	}
 	headPublisher := head.NewPublisher()
 	startHeadPublisher(host, topicName, headPublisher)
 
 	return &publisher{
-		extraData:     cfg.extraData,
+		extraData:     opts.extraData,
 		headPublisher: headPublisher,
 		host:          host,
-		senders:       cfg.senders,
+		senders:       opts.senders,
 	}, nil
 }
 

internal/registry/option.go

@@ -7,24 +7,25 @@ import (
 	"github.com/ipni/storetheindex/internal/registry/discovery"
 )
 
-// Options is a structure containing all the options that can be used when constructing an http server
+// regConfig contains all options for the server.
 type regConfig struct {
 	discoverer      discovery.Discoverer
 	freezeAtPercent float64
 	valueStoreDir   string
 }
 
-// Option for registry
+// Option is a function that sets a value in a regConfig.
 type Option func(*regConfig) error
 
-// apply applies the given options to this Option.
-func (c *regConfig) apply(opts ...Option) error {
+// getOpts creates a regConfig and applies Options to it.
+func getOpts(opts []Option) (regConfig, error) {
+	var cfg regConfig
 	for i, opt := range opts {
-		if err := opt(c); err != nil {
-			return fmt.Errorf("option %d failed: %s", i, err)
+		if err := opt(&cfg); err != nil {
+			return regConfig{}, fmt.Errorf("option %d error: %s", i, err)
 		}
 	}
-	return nil
+	return cfg, nil
 }
 
 func WithDiscoverer(discoverer discovery.Discoverer) Option {

internal/registry/registry.go

@@ -254,8 +254,7 @@ func (p *ExtendedProviderInfo) UnmarshalJSON(data []byte) error {
 // configuration, a datastore to persist provider data, and a Discoverer
 // interface. The context is only used for cancellation of this function.
 func New(ctx context.Context, cfg config.Discovery, dstore datastore.Datastore, options ...Option) (*Registry, error) {
-	var opts regConfig
-	err := opts.apply(options...)
+	opts, err := getOpts(options)
 	if err != nil {
 		return nil, err
 	}

server/admin/http/options.go

@@ -6,49 +6,46 @@ import (
 )
 
 const (
-	apiWriteTimeout = 30 * time.Second
-	apiReadTimeout  = 30 * time.Second
+	defaultWriteTimeout = 30 * time.Second
+	defaultReadTimeout  = 30 * time.Second
 )
 
-// Options is a structure containing all the options that can be used when constructing an http server
-type serverConfig struct {
-	apiWriteTimeout time.Duration
-	apiReadTimeout  time.Duration
+// config contains all options for the server.
+type config struct {
+	readTimeout  time.Duration
+	writeTimeout time.Duration
 }
 
-// ServerOption for httpserver
-type ServerOption func(*serverConfig) error
+// Option is a function that sets a value in a config.
+type Option func(*config) error
 
-// defaults are the default ptions. This option will be automatically
-// prepended to any options you pass to the constructor.
-var serverDefaults = func(o *serverConfig) error {
-	o.apiWriteTimeout = apiWriteTimeout
-	o.apiReadTimeout = apiReadTimeout
-	return nil
-}
+// getOpts creates a config and applies Options to it.
+func getOpts(opts []Option) (config, error) {
+	cfg := config{
+		readTimeout:  defaultReadTimeout,
+		writeTimeout: defaultWriteTimeout,
+	}
 
-// apply applies the given options to this Option
-func (c *serverConfig) apply(opts ...ServerOption) error {
 	for i, opt := range opts {
-		if err := opt(c); err != nil {
-			return fmt.Errorf("httpserver option %d failed: %s", i, err)
+		if err := opt(&cfg); err != nil {
+			return config{}, fmt.Errorf("option %d error: %s", i, err)
 		}
 	}
-	return nil
+	return cfg, nil
 }
 
-// WriteTimeout config for API
-func WriteTimeout(t time.Duration) ServerOption {
-	return func(c *serverConfig) error {
-		c.apiWriteTimeout = t
+// WithReadTimeout configures server read timeout.
+func WithReadTimeout(t time.Duration) Option {
+	return func(c *config) error {
+		c.readTimeout = t
 		return nil
 	}
 }
 
-// ReadTimeout config for API
-func ReadTimeout(t time.Duration) ServerOption {
-	return func(c *serverConfig) error {
-		c.apiReadTimeout = t
+// WithWriteTimeout configures server write timeout.
+func WithWriteTimeout(t time.Duration) Option {
+	return func(c *config) error {
+		c.writeTimeout = t
 		return nil
 	}
 }

server/admin/http/server.go

@@ -30,12 +30,11 @@ func (s *Server) URL() string {
 	return fmt.Sprint("http://", s.listener.Addr().String())
 }
 
-func New(listen string, id peer.ID, indexer indexer.Interface, ingester *ingest.Ingester, reg *registry.Registry, reloadErrChan chan<- chan error, options ...ServerOption) (*Server, error) {
-	var cfg serverConfig
-	if err := cfg.apply(append([]ServerOption{serverDefaults}, options...)...); err != nil {
+func New(listen string, id peer.ID, indexer indexer.Interface, ingester *ingest.Ingester, reg *registry.Registry, reloadErrChan chan<- chan error, options ...Option) (*Server, error) {
+	opts, err := getOpts(options)
+	if err != nil {
 		return nil, err
 	}
-	var err error
 
 	l, err := net.Listen("tcp", listen)
 	if err != nil {
@@ -45,8 +44,8 @@ func New(listen string, id peer.ID, indexer indexer.Interface, ingester *ingest.
 	r := mux.NewRouter().StrictSlash(true)
 	server := &http.Server{
 		Handler:      r,
-		WriteTimeout: cfg.apiWriteTimeout,
-		ReadTimeout:  cfg.apiReadTimeout,
+		WriteTimeout: opts.writeTimeout,
+		ReadTimeout:  opts.readTimeout,
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())