From 35a376ff0222f19ea4f4635ac3123b30ef761236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sevilla?= Date: Fri, 3 May 2024 11:44:22 +0200 Subject: [PATCH] Return a new indexer rather than a pointer to the same object (#45) Signed-off-by: Raul Sevilla --- indexers/elastic.go | 22 +++++++++------------- indexers/elastic_test.go | 12 ++++++------ indexers/factory.go | 23 +++++++++++------------ indexers/local.go | 18 +++++++----------- indexers/local_test.go | 5 ++--- indexers/opensearch.go | 22 +++++++++------------- indexers/opensearch_test.go | 10 +++++----- indexers/types.go | 1 - 8 files changed, 49 insertions(+), 64 deletions(-) diff --git a/indexers/elastic.go b/indexers/elastic.go index 52090a7..78ec9ce 100644 --- a/indexers/elastic.go +++ b/indexers/elastic.go @@ -40,16 +40,12 @@ type Elastic struct { // ESClient elasticsearch client instance var ESClient *elasticsearch.Client -// Init function -func init() { - indexerMap[ElasticIndexer] = &Elastic{} -} - -// Returns new indexer for elastic search -func (esIndexer *Elastic) new(indexerConfig IndexerConfig) error { +// Returns new indexer for Elastic +func NewElasticIndexer(indexerConfig IndexerConfig) (*Elastic, error) { var err error + var esIndexer Elastic if indexerConfig.Index == "" { - return fmt.Errorf("index name not specified") + return &esIndexer, fmt.Errorf("index name not specified") } esIndex := strings.ToLower(indexerConfig.Index) cfg := elasticsearch.Config{ @@ -58,24 +54,24 @@ func (esIndexer *Elastic) new(indexerConfig IndexerConfig) error { } ESClient, err = elasticsearch.NewClient(cfg) if err != nil { - return fmt.Errorf("error creating the ES client: %s", err) + return &esIndexer, fmt.Errorf("error creating the ES client: %s", err) } r, err := ESClient.Cluster.Health() if err != nil { - return fmt.Errorf("ES health check failed: %s", err) + return &esIndexer, fmt.Errorf("ES health check failed: %s", err) } if r.StatusCode != 200 { - return fmt.Errorf("unexpected ES status code: %d", r.StatusCode) + return &esIndexer, fmt.Errorf("unexpected ES status code: %d", r.StatusCode) } esIndexer.index = esIndex r, _ = ESClient.Indices.Exists([]string{esIndex}) if r.IsError() { r, _ = ESClient.Indices.Create(esIndex) if r.IsError() { - return fmt.Errorf("error creating index %s on ES: %s", esIndex, r.String()) + return &esIndexer, fmt.Errorf("error creating index %s on ES: %s", esIndex, r.String()) } } - return nil + return &esIndexer, nil } // Index uses bulkIndexer to index the documents in the given index diff --git a/indexers/elastic_test.go b/indexers/elastic_test.go index c72a097..8116143 100644 --- a/indexers/elastic_test.go +++ b/indexers/elastic_test.go @@ -2,10 +2,10 @@ package indexers import ( "errors" + "log" "net/http" "net/http/httptest" "os" - "log" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -40,12 +40,12 @@ var _ = Describe("Tests for elastic.go", func() { })) defer testcase.mockServer.Close() testcase.indexerConfig.Servers = []string{testcase.mockServer.URL} - err := indexer.new(testcase.indexerConfig) + _, err := NewElasticIndexer(testcase.indexerConfig) Expect(err).To(BeEquivalentTo(errors.New("unexpected ES status code: 400"))) }) It("when no url is passed", func() { - err := indexer.new(testcase.indexerConfig) + _, err := NewElasticIndexer(testcase.indexerConfig) testcase.mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusGatewayTimeout) })) @@ -58,7 +58,7 @@ var _ = Describe("Tests for elastic.go", func() { os.Setenv("ELASTICSEARCH_URL", "not a valid url:port") defer os.Unsetenv("ELASTICSEARCH_URL") defer testcase.mockServer.Close() - err := indexer.new(testcase.indexerConfig) + _, err := NewElasticIndexer(testcase.indexerConfig) Expect(err).To(BeEquivalentTo(errors.New("error creating the ES client: cannot create client: cannot parse url: parse \"not a valid url:port\": first path segment in URL cannot contain colon"))) }) @@ -66,7 +66,7 @@ var _ = Describe("Tests for elastic.go", func() { defer testcase.mockServer.Close() testcase.indexerConfig.Servers = []string{testcase.mockServer.URL} testcase.indexerConfig.Index = "" - err := indexer.new(testcase.indexerConfig) + _, err := NewElasticIndexer(testcase.indexerConfig) Expect(err).To(BeEquivalentTo(errors.New("index name not specified"))) }) @@ -116,7 +116,7 @@ var _ = Describe("Tests for elastic.go", func() { _, err := indexer.Index(testcase.documents, testcase.opts) Expect(err).To(BeNil()) }) - + It("err returned docs not processed", func() { testcase.documents = append(testcase.documents, make(chan string)) _, err := indexer.Index(testcase.documents, testcase.opts) diff --git a/indexers/factory.go b/indexers/factory.go index fc23c45..ab4a5c5 100644 --- a/indexers/factory.go +++ b/indexers/factory.go @@ -18,20 +18,19 @@ import ( "fmt" ) -var indexerMap = make(map[IndexerType]Indexer) - // NewIndexer creates a new Indexer with the specified IndexerConfig func NewIndexer(indexerConfig IndexerConfig) (*Indexer, error) { var indexer Indexer - var exists bool - cfg := indexerConfig - if indexer, exists = indexerMap[cfg.Type]; exists { - err := indexer.new(indexerConfig) - if err != nil { - return &indexer, err - } - } else { - return &indexer, fmt.Errorf("Indexer not found: %s", cfg.Type) + var err error + switch indexerConfig.Type { + case LocalIndexer: + indexer, err = NewLocalIndexer(indexerConfig) + case ElasticIndexer: + indexer, err = NewElasticIndexer(indexerConfig) + case OpenSearchIndexer: + indexer, err = NewOpenSearchIndexer(indexerConfig) + default: + return &indexer, fmt.Errorf("Indexer not found: %s", indexerConfig.Type) } - return &indexer, nil + return &indexer, err } diff --git a/indexers/local.go b/indexers/local.go index dcabe85..e0ade7a 100644 --- a/indexers/local.go +++ b/indexers/local.go @@ -26,19 +26,15 @@ type Local struct { metricsDirectory string } -// Init function -func init() { - indexerMap[LocalIndexer] = &Local{} -} - -// Prepares local indexing directory -func (l *Local) new(indexerConfig IndexerConfig) error { +// NewLocalIndexer returns a new Local Indexer +func NewLocalIndexer(indexerConfig IndexerConfig) (*Local, error) { + var localIndexer Local if indexerConfig.MetricsDirectory == "" { - return fmt.Errorf("directory name not specified") + return &localIndexer, fmt.Errorf("directory name not specified") } - l.metricsDirectory = indexerConfig.MetricsDirectory - err := os.MkdirAll(l.metricsDirectory, 0744) - return err + localIndexer.metricsDirectory = indexerConfig.MetricsDirectory + err := os.MkdirAll(localIndexer.metricsDirectory, 0744) + return &localIndexer, err } // Index uses generates a local file with the given name and metrics diff --git a/indexers/local_test.go b/indexers/local_test.go index 9bce27e..87e66fa 100644 --- a/indexers/local_test.go +++ b/indexers/local_test.go @@ -18,7 +18,6 @@ var _ = Describe("Tests for local.go", func() { indexerconfig IndexerConfig } var testcase newtestcase - var localIndexer Local BeforeEach(func() { testcase = newtestcase{ indexerconfig: IndexerConfig{Type: "local", @@ -30,13 +29,13 @@ var _ = Describe("Tests for local.go", func() { }) It("returns err no metrics directory", func() { - err := localIndexer.new(testcase.indexerconfig) + _, err := NewLocalIndexer(testcase.indexerconfig) Expect(err).To(BeEquivalentTo(errors.New("directory name not specified"))) }) It("returns nil as error", func() { testcase.indexerconfig.MetricsDirectory = "placeholder" - err := localIndexer.new(testcase.indexerconfig) + _, err := NewLocalIndexer(testcase.indexerconfig) Expect(err).To(BeNil()) }) }) diff --git a/indexers/opensearch.go b/indexers/opensearch.go index 35fde7b..4d4bf7d 100644 --- a/indexers/opensearch.go +++ b/indexers/opensearch.go @@ -40,16 +40,12 @@ type OpenSearch struct { index string } -// Init function -func init() { - indexerMap[OpenSearchIndexer] = &OpenSearch{} -} - // Returns new indexer for OpenSearch -func (OpenSearchIndexer *OpenSearch) new(indexerConfig IndexerConfig) error { +func NewOpenSearchIndexer(indexerConfig IndexerConfig) (*OpenSearch, error) { var err error + var osIndexer OpenSearch if indexerConfig.Index == "" { - return fmt.Errorf("index name not specified") + return &osIndexer, fmt.Errorf("index name not specified") } OpenSearchIndex := strings.ToLower(indexerConfig.Index) cfg := opensearch.Config{ @@ -58,24 +54,24 @@ func (OpenSearchIndexer *OpenSearch) new(indexerConfig IndexerConfig) error { } OSClient, err = opensearch.NewClient(cfg) if err != nil { - return fmt.Errorf("error creating the OpenSearch client: %s", err) + return &osIndexer, fmt.Errorf("error creating the OpenSearch client: %s", err) } r, err := OSClient.Cluster.Health() if err != nil { - return fmt.Errorf("OpenSearch health check failed: %s", err) + return &osIndexer, fmt.Errorf("OpenSearch health check failed: %s", err) } if r.StatusCode != 200 { - return fmt.Errorf("unexpected OpenSearch status code: %d", r.StatusCode) + return &osIndexer, fmt.Errorf("unexpected OpenSearch status code: %d", r.StatusCode) } - OpenSearchIndexer.index = OpenSearchIndex + osIndexer.index = OpenSearchIndex r, _ = OSClient.Indices.Exists([]string{OpenSearchIndex}) if r.IsError() { r, _ = OSClient.Indices.Create(OpenSearchIndex) if r.IsError() { - return fmt.Errorf("error creating index %s on OpenSearch: %s", OpenSearchIndex, r.String()) + return &osIndexer, fmt.Errorf("error creating index %s on OpenSearch: %s", OpenSearchIndex, r.String()) } } - return nil + return &osIndexer, nil } // Index uses bulkIndexer to index the documents in the given index diff --git a/indexers/opensearch_test.go b/indexers/opensearch_test.go index 3a4ed3c..3c709cc 100644 --- a/indexers/opensearch_test.go +++ b/indexers/opensearch_test.go @@ -2,10 +2,10 @@ package indexers import ( "errors" + "log" "net/http" "net/http/httptest" "os" - "log" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -39,12 +39,12 @@ var _ = Describe("Tests for opensearch.go", func() { })) defer testcase.mockServer.Close() testcase.indexerConfig.Servers = []string{testcase.mockServer.URL} - err := indexer.new(testcase.indexerConfig) + _, err := NewOpenSearchIndexer(testcase.indexerConfig) Expect(err).To(BeEquivalentTo(errors.New("OpenSearch health check failed: cannot retrieve information from OpenSearch"))) }) It("when no url is passed", func() { - err := indexer.new(testcase.indexerConfig) + _, err := NewOpenSearchIndexer(testcase.indexerConfig) testcase.mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusGatewayTimeout) })) @@ -56,7 +56,7 @@ var _ = Describe("Tests for opensearch.go", func() { os.Setenv("ELASTICSEARCH_URL", "not a valid url:port") defer os.Unsetenv("ELASTICSEARCH_URL") defer testcase.mockServer.Close() - err := indexer.new(testcase.indexerConfig) + _, err := NewOpenSearchIndexer(testcase.indexerConfig) Expect(err).To(BeEquivalentTo(errors.New("error creating the OpenSearch client: cannot create client: cannot parse url: parse \"not a valid url:port\": first path segment in URL cannot contain colon"))) }) @@ -64,7 +64,7 @@ var _ = Describe("Tests for opensearch.go", func() { defer testcase.mockServer.Close() testcase.indexerConfig.Servers = []string{testcase.mockServer.URL} testcase.indexerConfig.Index = "" - err := indexer.new(testcase.indexerConfig) + _, err := NewOpenSearchIndexer(testcase.indexerConfig) Expect(err).To(BeEquivalentTo(errors.New("index name not specified"))) }) diff --git a/indexers/types.go b/indexers/types.go index bcb044e..edb5886 100644 --- a/indexers/types.go +++ b/indexers/types.go @@ -27,7 +27,6 @@ const ( // Indexer interface type Indexer interface { Index([]interface{}, IndexingOpts) (string, error) - new(IndexerConfig) error } // Indexing options