Replace public InitFromOptions with private ConfigureFromOptions

plugin/storage/badger/factory.go

@@ -99,7 +99,7 @@ func NewFactoryWithConfig(
 	logger *zap.Logger,
 ) (*Factory, error) {
 	f := NewFactory()
-	f.InitFromOptions(Options{Primary: cfg})
+	f.configureFromOptions(&Options{Primary: cfg})
 	err := f.Initialize(metricsFactory, logger)
 	if err != nil {
 		return nil, err
@@ -115,11 +115,12 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
 // InitFromViper implements plugin.Configurable
 func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
 	f.Options.InitFromViper(v, logger)
+	f.configureFromOptions(f.Options)
 }
 
 // InitFromOptions initializes Factory from supplied options
-func (f *Factory) InitFromOptions(opts Options) {
-	f.Options = &opts
+func (f *Factory) configureFromOptions(opts *Options) {
+	f.Options = opts
 }
 
 // Initialize implements storage.Factory

plugin/storage/badger/factory_test.go

@@ -194,11 +194,15 @@ func TestBadgerMetrics(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func TestInitFromOptions(t *testing.T) {
+func TestConfigureFromOptions(t *testing.T) {
 	f := NewFactory()
-	opts := Options{}
-	f.InitFromOptions(opts)
-	assert.Equal(t, &opts, f.Options)
+	opts := &Options{
+		Primary: NamespaceConfig{
+			MaintenanceInterval: 42 * time.Second,
+		},
+	}
+	f.configureFromOptions(opts)
+	assert.Equal(t, opts, f.Options)
 }
 
 func TestBadgerStorageFactoryWithConfig(t *testing.T) {

plugin/storage/cassandra/factory.go

@@ -82,15 +82,15 @@ func NewFactory() *Factory {
 
 // NewFactoryWithConfig initializes factory with Config.
 func NewFactoryWithConfig(
-	cfg Options,
+	opts Options,
 	metricsFactory metrics.Factory,
 	logger *zap.Logger,
 ) (*Factory, error) {
 	f := NewFactory()
 	// use this to help with testing
 	b := &withConfigBuilder{
 		f:              f,
-		cfg:            &cfg,
+		opts:           &opts,
 		metricsFactory: metricsFactory,
 		logger:         logger,
 		initializer:    f.Initialize, // this can be mocked in tests
@@ -100,15 +100,15 @@ func NewFactoryWithConfig(
 
 type withConfigBuilder struct {
 	f              *Factory
-	cfg            *Options
+	opts           *Options
 	metricsFactory metrics.Factory
 	logger         *zap.Logger
 	initializer    func(metricsFactory metrics.Factory, logger *zap.Logger) error
 }
 
 func (b *withConfigBuilder) build() (*Factory, error) {
-	b.f.InitFromOptions(b.cfg)
-	if err := b.cfg.Primary.Validate(); err != nil {
+	b.f.configureFromOptions(b.opts)
+	if err := b.opts.Primary.Validate(); err != nil {
 		return nil, err
 	}
 	err := b.initializer(b.metricsFactory, b.logger)
@@ -126,14 +126,11 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
 // InitFromViper implements plugin.Configurable
 func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
 	f.Options.InitFromViper(v)
-	f.primaryConfig = f.Options.GetPrimary()
-	if cfg := f.Options.Get(archiveStorageConfig); cfg != nil {
-		f.archiveConfig = cfg // this is so stupid - see https://golang.org/doc/faq#nil_error
-	}
+	f.configureFromOptions(f.Options)
 }
 
 // InitFromOptions initializes factory from options.
-func (f *Factory) InitFromOptions(o *Options) {
+func (f *Factory) configureFromOptions(o *Options) {
 	f.Options = o
 	// TODO this is a hack because we do not define defaults in Options
 	if o.others == nil {

plugin/storage/cassandra/factory_test.go

@@ -191,19 +191,19 @@ func TestWriterOptions(t *testing.T) {
 	assert.Empty(t, options)
 }
 
-func TestInitFromOptions(t *testing.T) {
+func TestConfigureFromOptions(t *testing.T) {
 	f := NewFactory()
 	o := NewOptions("foo", archiveStorageConfig)
 	o.others[archiveStorageConfig].Enabled = true
-	f.InitFromOptions(o)
+	f.configureFromOptions(o)
 	assert.Equal(t, o, f.Options)
 	assert.Equal(t, o.GetPrimary(), f.primaryConfig)
 	assert.Equal(t, o.Get(archiveStorageConfig), f.archiveConfig)
 }
 
 func TestNewFactoryWithConfig(t *testing.T) {
 	t.Run("valid configuration", func(t *testing.T) {
-		cfg := Options{
+		opts := &Options{
 			Primary: namespaceConfig{
 				Configuration: cassandraCfg.Configuration{
 					Servers: []string{"localhost:9200"},
@@ -213,7 +213,7 @@ func TestNewFactoryWithConfig(t *testing.T) {
 		f := NewFactory()
 		b := &withConfigBuilder{
 			f:              f,
-			cfg:            &cfg,
+			opts:           opts,
 			metricsFactory: metrics.NullFactory,
 			logger:         zap.NewNop(),
 			initializer:    func(metricsFactory metrics.Factory, logger *zap.Logger) error { return nil },
@@ -223,7 +223,7 @@ func TestNewFactoryWithConfig(t *testing.T) {
 	})
 	t.Run("connection error", func(t *testing.T) {
 		expErr := errors.New("made-up error")
-		cfg := Options{
+		opts := &Options{
 			Primary: namespaceConfig{
 				Configuration: cassandraCfg.Configuration{
 					Servers: []string{"localhost:9200"},
@@ -233,7 +233,7 @@ func TestNewFactoryWithConfig(t *testing.T) {
 		f := NewFactory()
 		b := &withConfigBuilder{
 			f:              f,
-			cfg:            &cfg,
+			opts:           opts,
 			metricsFactory: metrics.NullFactory,
 			logger:         zap.NewNop(),
 			initializer:    func(metricsFactory metrics.Factory, logger *zap.Logger) error { return expErr },

plugin/storage/es/factory.go

@@ -106,7 +106,7 @@ func NewFactoryWithConfig(
 	}
 
 	f := NewFactory()
-	f.InitFromOptions(Options{
+	f.configureFromOptions(&Options{
 		Primary: namespaceConfig{
 			Configuration: cfg,
 			namespace:     primaryNamespace,
@@ -128,13 +128,12 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
 // InitFromViper implements plugin.Configurable
 func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
 	f.Options.InitFromViper(v)
-	f.primaryConfig = f.Options.GetPrimary()
-	f.archiveConfig = f.Options.Get(archiveNamespace)
+	f.configureFromOptions(f.Options)
 }
 
-// InitFromOptions configures factory from Options struct.
-func (f *Factory) InitFromOptions(o Options) {
-	f.Options = &o
+// configureFromOptions configures factory from Options struct.
+func (f *Factory) configureFromOptions(o *Options) {
+	f.Options = o
 	f.primaryConfig = f.Options.GetPrimary()
 	f.archiveConfig = f.Options.Get(archiveNamespace)
 }

plugin/storage/es/factory_test.go

@@ -261,13 +261,13 @@ func TestArchiveEnabled(t *testing.T) {
 	assert.NotNil(t, r)
 }
 
-func TestInitFromOptions(t *testing.T) {
+func TestConfigureFromOptions(t *testing.T) {
 	f := NewFactory()
-	o := Options{
+	o := &Options{
 		Primary: namespaceConfig{Configuration: escfg.Configuration{Servers: []string{"server"}}},
 		others:  map[string]*namespaceConfig{"es-archive": {Configuration: escfg.Configuration{Servers: []string{"server2"}}}},
 	}
-	f.InitFromOptions(o)
+	f.configureFromOptions(o)
 	assert.Equal(t, o.GetPrimary(), f.primaryConfig)
 	assert.Equal(t, o.Get(archiveNamespace), f.archiveConfig)
 }

plugin/storage/grpc/factory.go

@@ -70,7 +70,7 @@ func NewFactoryWithConfig(
 	logger *zap.Logger,
 ) (*Factory, error) {
 	f := NewFactory()
-	f.InitFromOptions(Options{Configuration: cfg})
+	f.configureFromOptions(Options{Configuration: cfg})
 	err := f.Initialize(metricsFactory, logger)
 	if err != nil {
 		return nil, err
@@ -91,8 +91,8 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
 	f.builder = &f.options.Configuration
 }
 
-// InitFromOptions initializes factory from options
-func (f *Factory) InitFromOptions(opts Options) {
+// configureFromOptions initializes factory from options
+func (f *Factory) configureFromOptions(opts Options) {
 	f.options = opts
 	f.builder = &f.options.Configuration
 }

plugin/storage/grpc/factory_test.go

@@ -315,14 +315,14 @@ func TestWithConfiguration(t *testing.T) {
 	require.NoError(t, f.Close())
 }
 
-func TestInitFromOptions(t *testing.T) {
+func TestConfigureFromOptions(t *testing.T) {
 	f := Factory{}
 	o := Options{
 		Configuration: grpcConfig.Configuration{
 			PluginLogLevel: "info",
 		},
 	}
-	f.InitFromOptions(o)
+	f.configureFromOptions(o)
 	assert.Equal(t, o, f.options)
 	assert.Equal(t, &o.Configuration, f.builder)
 }

plugin/storage/kafka/factory.go

@@ -62,11 +62,11 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
 // InitFromViper implements plugin.Configurable
 func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
 	f.options.InitFromViper(v)
-	f.Builder = &f.options.Config
+	f.configureFromOptions(f.options)
 }
 
-// InitFromOptions initializes factory from options.
-func (f *Factory) InitFromOptions(o Options) {
+// configureFromOptions initializes factory from options.
+func (f *Factory) configureFromOptions(o Options) {
 	f.options = o
 	f.Builder = &f.options.Config
 }

plugin/storage/kafka/factory_test.go

@@ -158,10 +158,10 @@ func TestKafkaFactoryDoesNotLogPassword(t *testing.T) {
 	}
 }
 
-func TestInitFromOptions(t *testing.T) {
+func TestConfigureFromOptions(t *testing.T) {
 	f := NewFactory()
 	o := Options{Topic: "testTopic", Config: kafkaConfig.Configuration{Brokers: []string{"host"}}}
-	f.InitFromOptions(o)
+	f.configureFromOptions(o)
 	assert.Equal(t, o, f.options)
 	assert.Equal(t, &o.Config, f.Builder)
 }

plugin/storage/memory/factory.go

@@ -58,7 +58,7 @@ func NewFactoryWithConfig(
 	logger *zap.Logger,
 ) *Factory {
 	f := NewFactory()
-	f.InitFromOptions(Options{Configuration: cfg})
+	f.configureFromOptions(Options{Configuration: cfg})
 	_ = f.Initialize(metricsFactory, logger)
 	return f
 }
@@ -73,8 +73,8 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) {
 	f.options.InitFromViper(v)
 }
 
-// InitFromOptions initializes factory from the supplied options
-func (f *Factory) InitFromOptions(opts Options) {
+// configureFromOptions initializes factory from the supplied options
+func (f *Factory) configureFromOptions(opts Options) {
 	f.options = opts
 }
 

plugin/storage/memory/factory_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/jaegertracing/jaeger/internal/metrics/fork"
 	"github.com/jaegertracing/jaeger/internal/metricstest"
 	"github.com/jaegertracing/jaeger/pkg/config"
+	memCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 	"github.com/jaegertracing/jaeger/storage"
 )
@@ -61,11 +62,12 @@ func TestWithConfiguration(t *testing.T) {
 	assert.Equal(t, 100, f.options.Configuration.MaxTraces)
 }
 
-func TestInitFromOptions(t *testing.T) {
-	o := Options{}
-	f := Factory{}
-	f.InitFromOptions(o)
-	assert.Equal(t, o, f.options)
+func TestNewFactoryWithConfig(t *testing.T) {
+	cfg := memCfg.Configuration{
+		MaxTraces: 42,
+	}
+	f := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop())
+	assert.Equal(t, cfg, f.options.Configuration)
 }
 
 func TestPublishOpts(t *testing.T) {