Merge pull request #17 from puellanivis/double-registration-error

it is an error to attempt to register a health check twice

health.go

@@ -2,15 +2,17 @@ package health
 
 import (
 	"encoding/json"
+	"errors"
+	"fmt"
 	"net/http"
 	"runtime"
 	"sync"
 	"time"
 )
 
 var (
-	mu     sync.Mutex
-	checks []Config
+	mu       sync.Mutex
+	checkMap = make(map[string]Config)
 )
 
 const (
@@ -70,14 +72,25 @@ type (
 )
 
 // Register registers a check config to be performed.
-func Register(c Config) {
+func Register(c Config) error {
+	if c.Timeout == 0 {
+		c.Timeout = time.Second * 2
+	}
+
+	if c.Name == "" {
+		return errors.New("health check must have a name to be registered")
+	}
+
 	mu.Lock()
 	defer mu.Unlock()
 
-	if c.Timeout == 0 {
-		c.Timeout = time.Second * 2
+	if _, ok := checkMap[c.Name]; ok {
+		return fmt.Errorf("health check %s is already registered", c.Name)
 	}
-	checks = append(checks, c)
+
+	checkMap[c.Name] = c
+
+	return nil
 }
 
 // Handler returns an HTTP handler (http.HandlerFunc).
@@ -91,7 +104,7 @@ func HandlerFunc(w http.ResponseWriter, r *http.Request) {
 	defer mu.Unlock()
 
 	status := statusOK
-	total := len(checks)
+	total := len(checkMap)
 	failures := make(map[string]string)
 	resChan := make(chan checkResponse, total)
 
@@ -103,7 +116,7 @@ func HandlerFunc(w http.ResponseWriter, r *http.Request) {
 		wg.Wait()
 	}()
 
-	for _, c := range checks {
+	for _, c := range checkMap {
 		go func(c Config) {
 			defer wg.Done()
 			select {
@@ -151,7 +164,7 @@ func Reset() {
 	mu.Lock()
 	defer mu.Unlock()
 
-	checks = []Config{}
+	checkMap = make(map[string]Config)
 }
 
 func newCheck(status string, failures map[string]string) Check {

health_test.go

@@ -13,7 +13,58 @@ const (
 	checkErr = "Failed during RabbitMQ health check"
 )
 
+func TestRegisterWithNoName(t *testing.T) {
+	err := Register(Config{
+		Name: "",
+		Check: func() error {
+			return nil
+		},
+	})
+	if err == nil {
+		t.Error("health check registration with empty name should return an error, but did not get one")
+	}
+}
+
+func TestDoubleRegister(t *testing.T) {
+	Reset()
+	if len(checkMap) != 0 {
+		t.Errorf("checks lenght differes from zero: got %d", len(checkMap))
+	}
+
+	healthcheckName := "healthcheck"
+
+	conf := Config{
+		Name: healthcheckName,
+		Check: func() error {
+			return nil
+		},
+	}
+
+	err := Register(conf)
+	if err != nil {
+		// If the first registration failed, then further testing makes no sense.
+		t.Fatal("the first registration of a health check should not return an error, but got: ", err)
+	}
+
+	err = Register(conf)
+	if err == nil {
+		t.Error("the second registration of a health check config should return an error, but did not")
+	}
+
+	err = Register(Config{
+		Name: healthcheckName,
+		Check: func() error {
+			return errors.New("health checks registered")
+		},
+	})
+	if err == nil {
+		t.Error("registration with same name, but different details should still return an error, but did not")
+	}
+}
+
 func TestHealthHandler(t *testing.T) {
+	Reset()
+
 	res := httptest.NewRecorder()
 	req, err := http.NewRequest("GET", "/status", nil)
 	if err != nil {
@@ -76,7 +127,7 @@ func TestHealthHandler(t *testing.T) {
 	}
 
 	Reset()
-	if len(checks) != 0 {
-		t.Errorf("checks lenght differes from zero: got %d", len(checks))
+	if len(checkMap) != 0 {
+		t.Errorf("checks lenght differes from zero: got %d", len(checkMap))
 	}
 }