pipeline: disallow Register() name dupes (#72)

cmd/conduit/main.go

@@ -6,11 +6,10 @@ import (
 
 	"github.com/spf13/cobra/doc"
 
-	"github.com/algorand/conduit/pkg/cli"
-
 	_ "github.com/algorand/conduit/conduit/plugins/exporters/all"
 	_ "github.com/algorand/conduit/conduit/plugins/importers/all"
 	_ "github.com/algorand/conduit/conduit/plugins/processors/all"
+	"github.com/algorand/conduit/pkg/cli"
 )
 
 func main() {

conduit/plugins/exporters/exporter_factory.go

@@ -27,6 +27,10 @@ var Exporters = make(map[string]ExporterConstructor)
 // for loose coupling between the configuration and the implementation. It is extremely similar to the way sql.DB
 // drivers are configured and used.
 func Register(name string, constructor ExporterConstructor) {
+	if _, ok := Exporters[name]; ok {
+		panic(fmt.Errorf("exporter %s already registered", name))
+	}
+
 	Exporters[name] = constructor
 }
 

conduit/plugins/exporters/exporter_factory_test.go

@@ -1,6 +1,7 @@
 package exporters
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -47,3 +48,17 @@ func TestExporterByNameNotFound(t *testing.T) {
 	expectedErr := "no Exporter Constructor for barfoo"
 	assert.EqualError(t, err, expectedErr)
 }
+
+// TestRegister verifies that Register works as expected.
+func TestRegister(t *testing.T) {
+	mockName := "____mock"
+	assert.NotContains(t, Exporters, mockName)
+
+	Register(mockName, &mockExporterConstructor{})
+	assert.Contains(t, Exporters, mockName)
+
+	panicMsg := fmt.Sprintf("exporter %s already registered", mockName)
+	assert.PanicsWithError(t, panicMsg, func() {
+		Register(mockName, &mockExporterConstructor{})
+	})
+}

conduit/plugins/exporters/noop/noop_exporter_test.go

@@ -20,7 +20,8 @@ var nc = exporters.ExporterConstructorFunc(func() exporters.Exporter {
 var ne = nc.New()
 
 func TestExporterBuilderByName(t *testing.T) {
-	exporters.Register(metadata.Name, nc)
+	// init() has already registered the noop exporter
+	assert.Contains(t, exporters.Exporters, metadata.Name)
 	neBuilder, err := exporters.ExporterBuilderByName(metadata.Name)
 	assert.NoError(t, err)
 	ne := neBuilder.New()

conduit/plugins/importers/algod/algod_importer.go

@@ -241,19 +241,18 @@ func (algodImp *algodImporter) catchupNode(catchpoint string, targetRound uint64
 
 		if runCatchup, err := checkRounds(algodImp.logger, uint64(cpRound), nStatus.LastRound, targetRound); !runCatchup || err != nil {
 			return err
-		} else {
-			algodImp.logger.Infof("Starting catchpoint catchup with label %s", catchpoint)
+		}
+		algodImp.logger.Infof("Starting catchpoint catchup with label %s", catchpoint)
 
-			err = algodImp.startCatchpointCatchup(catchpoint)
-			if err != nil {
-				return err
-			}
+		err = algodImp.startCatchpointCatchup(catchpoint)
+		if err != nil {
+			return err
+		}
 
-			// Wait for algod to catchup
-			err = algodImp.monitorCatchpointCatchup()
-			if err != nil {
-				return err
-			}
+		// Wait for algod to catchup
+		err = algodImp.monitorCatchpointCatchup()
+		if err != nil {
+			return err
 		}
 	}
 

conduit/plugins/importers/importer_factory.go

@@ -27,6 +27,9 @@ var Importers = make(map[string]Constructor)
 // for loose coupling between the configuration and the implementation. It is extremely similar to the way sql.DB
 // drivers are configured and used.
 func Register(name string, constructor Constructor) {
+	if _, ok := Importers[name]; ok {
+		panic(fmt.Errorf("importer %s already registered", name))
+	}
 	Importers[name] = constructor
 }
 

conduit/plugins/importers/importer_factory_test.go

@@ -0,0 +1,53 @@
+package importers
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/algorand/conduit/conduit/data"
+	"github.com/algorand/conduit/conduit/plugins"
+	sdk "github.com/algorand/go-algorand-sdk/v2/types"
+)
+
+// MockImporter and MockImporterConstructor:
+
+type mockImporter struct{}
+
+func (i *mockImporter) Metadata() plugins.Metadata {
+	return plugins.Metadata{
+		Name:         "Awesome Importer",
+		Description:  "",
+		Deprecated:   false,
+		SampleConfig: "",
+	}
+}
+func (i *mockImporter) Init(ctx context.Context, initProvider data.InitProvider, cfg plugins.PluginConfig, logger *logrus.Logger) (*sdk.Genesis, error) {
+	return &sdk.Genesis{}, nil
+}
+func (i *mockImporter) Config() string                              { return "" }
+func (i *mockImporter) Close() error                                { return nil }
+func (i *mockImporter) GetBlock(rnd uint64) (data.BlockData, error) { return data.BlockData{}, nil }
+
+type mockImporterConstructor struct{}
+
+func (c *mockImporterConstructor) New() Importer {
+	return &mockImporter{}
+}
+
+// TestRegister verifies that Register works as expected.
+func TestRegister(t *testing.T) {
+	mockName := "____mock"
+	assert.NotContains(t, Importers, mockName)
+
+	Register(mockName, &mockImporterConstructor{})
+	assert.Contains(t, Importers, mockName)
+
+	panicMsg := fmt.Sprintf("importer %s already registered", mockName)
+	assert.PanicsWithError(t, panicMsg, func() {
+		Register(mockName, &mockImporterConstructor{})
+	})
+}

conduit/plugins/processors/processor_factory.go

@@ -27,6 +27,9 @@ var Processors = make(map[string]ProcessorConstructor)
 // for loose coupling between the configuration and the implementation. It is extremely similar to the way sql.DB
 // drivers are configured and used.
 func Register(name string, constructor ProcessorConstructor) {
+	if _, ok := Processors[name]; ok {
+		panic(fmt.Errorf("processor %s already registered", name))
+	}
 	Processors[name] = constructor
 }
 

conduit/plugins/processors/processor_factory_test.go

@@ -1,6 +1,7 @@
 package processors
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -52,3 +53,17 @@ func TestProcessorBuilderByNameNotFound(t *testing.T) {
 	expectedErr := "no Processor Constructor for barfoo"
 	assert.EqualError(t, err, expectedErr)
 }
+
+// TestRegister verifies that Register works as expected.
+func TestRegister(t *testing.T) {
+	mockName := "____mock"
+	assert.NotContains(t, Processors, mockName)
+
+	Register(mockName, &mockProcessorConstructor{})
+	assert.Contains(t, Processors, mockName)
+
+	panicMsg := fmt.Sprintf("processor %s already registered", mockName)
+	assert.PanicsWithError(t, panicMsg, func() {
+		Register(mockName, &mockProcessorConstructor{})
+	})
+}

go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/sirupsen/logrus v1.8.1
 	github.com/spf13/cobra v1.3.0
 	github.com/stretchr/testify v1.8.1
+	github.com/yuin/goldmark v1.5.4
 	gopkg.in/yaml.v3 v3.0.1
 
 )
@@ -67,7 +68,6 @@ require (
 	github.com/stretchr/objx v0.5.0 // indirect
 	github.com/valyala/bytebufferpool v1.0.0 // indirect
 	github.com/valyala/fasttemplate v1.2.1 // indirect
-	github.com/yuin/goldmark v1.5.4 // indirect
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
 	go.uber.org/zap v1.17.0 // indirect

pkg/cli/cli.go

@@ -19,9 +19,12 @@ import (
 )
 
 var (
-	logger     *log.Logger
-	conduitCmd = MakeConduitCmd()
+	logger *log.Logger
 
+	// ConduitCmd is the root command for conduit
+	ConduitCmd = MakeConduitCmd()
+
+	//Banner is the banner for conduit's pipeline
 	//go:embed banner.txt
 	Banner string
 )
@@ -102,6 +105,7 @@ func runConduitCmdWithConfig(args *data.Args) error {
 	return pipeline.Error()
 }
 
+// MakeConduitCmdWithUtilities creates the main cobra command with all utilities
 func MakeConduitCmdWithUtilities() *cobra.Command {
 	cmd := MakeConduitCmd()
 	cmd.AddCommand(initialize.InitCommand)