Make Open() idempotent and required

Open() wasn't a hard requirement, so it was a little surprising to need
to use it when creating an enterprise.Client in some circumstances but
not others. This returns an error when Querying if Open() was not
called, preventing panics which would otherwise result. Granted, this
would only be encountered by developers, but I believe a helpful error
is sometimes more useful than a mysterious panic when making libs that
devs actually enjoy using. Furthermore, a preflight check to see whether
dataNodes was initialized makes the Open() method idempotent.

chronograf.go

@@ -18,6 +18,7 @@ const (
 	ErrLayoutInvalid       = Error("layout is invalid")
 	ErrAlertNotFound       = Error("alert not found")
 	ErrAuthentication      = Error("user not authenticated")
+	ErrUninitialized       = Error("client uninitialized. Call Open() method")
 )
 
 // Error is a domain error encountered while processing chronograf requests
@@ -238,13 +239,13 @@ type Dashboard struct {
 
 // DashboardCell holds visual and query information for a cell
 type DashboardCell struct {
-	X       int32    `json:"x"`
-	Y       int32    `json:"y"`
-	W       int32    `json:"w"`
-	H       int32    `json:"h"`
-	Name    string   `json:"name"`
+	X       int32   `json:"x"`
+	Y       int32   `json:"y"`
+	W       int32   `json:"w"`
+	H       int32   `json:"h"`
+	Name    string  `json:"name"`
 	Queries []Query `json:"queries"`
-	Type    string   `json:"type"`
+	Type    string  `json:"type"`
 }
 
 // DashboardsStore is the storage and retrieval of dashboards

enterprise/enterprise.go

@@ -23,10 +23,10 @@ type Client struct {
 	}
 
 	dataNodes *ring.Ring
+	opened    bool
 }
 
 // NewClientWithTimeSeries initializes a Client with a known set of TimeSeries.
-// It is not necessary to call Open when creating a Client using this function
 func NewClientWithTimeSeries(series ...mrfusion.TimeSeries) *Client {
 	c := &Client{}
 
@@ -57,6 +57,11 @@ func NewClientWithURL(mu string, tls bool) (*Client, error) {
 
 // Open prepares a Client to process queries. It must be called prior to calling Query
 func (c *Client) Open() error {
+	c.opened = true
+	// return early if we already have dataNodes
+	if c.dataNodes != nil {
+		return nil
+	}
 	cluster, err := c.Ctrl.ShowCluster()
 	if err != nil {
 		return err
@@ -78,6 +83,9 @@ func (c *Client) Open() error {
 // Query retrieves timeseries information pertaining to a specified query. It
 // can be cancelled by using a provided context.
 func (c *Client) Query(ctx context.Context, q mrfusion.Query) (mrfusion.Response, error) {
+	if !c.opened {
+		return nil, mrfusion.ErrUninitialized
+	}
 	return c.nextDataNode().Query(ctx, q)
 }
 

enterprise/enterprise_test.go

@@ -72,7 +72,12 @@ func Test_Enterprise_AdvancesDataNodes(t *testing.T) {
 	m2 := mock.NewTimeSeries([]string{"http://host-2.example.com:8086"}, &mock.Response{})
 	cl := enterprise.NewClientWithTimeSeries(mrfusion.TimeSeries(m1), mrfusion.TimeSeries(m2))
 
-	_, err := cl.Query(context.Background(), mrfusion.Query{"show shards", "_internal", "autogen"})
+	err := cl.Open()
+	if err != nil {
+		t.Error("Unexpected error while initializing client: err:", err)
+	}
+
+	_, err = cl.Query(context.Background(), mrfusion.Query{"show shards", "_internal", "autogen"})
 	if err != nil {
 		t.Fatal("Unexpected error while issuing query: err:", err)
 	}
@@ -116,3 +121,12 @@ func Test_Enterprise_NewClientWithURL(t *testing.T) {
 		}
 	}
 }
+
+func Test_Enterprise_ComplainsIfNotOpened(t *testing.T) {
+	m1 := mock.NewTimeSeries([]string{"http://host-1.example.com:8086"}, &mock.Response{})
+	cl := enterprise.NewClientWithTimeSeries(mrfusion.TimeSeries(m1))
+	_, err := cl.Query(context.Background(), mrfusion.Query{"show shards", "_internal", "autogen"})
+	if err != mrfusion.ErrUninitialized {
+		t.Error("Expected ErrUnitialized, but was this err:", err)
+	}
+}