From 0d7b77d3dd3f3cc6d7b9bb30bd8cd924cb55c1dc Mon Sep 17 00:00:00 2001
From: Chris Goller <goller@gmail.com>
Date: Fri, 2 Nov 2018 09:11:57 -0500
Subject: [PATCH] feat(http): add query plan validation

---
 http/query_handler.go      | 22 +++++++++-------
 http/query_handler_test.go | 52 ++++++++++++++++++++++++++++++++++++++
 write/batcher_test.go      |  1 -
 3 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/http/query_handler.go b/http/query_handler.go
index 281098356c9..b97cdeaf18f 100644
--- a/http/query_handler.go
+++ b/http/query_handler.go
@@ -201,6 +201,18 @@ type postPlanRequest struct {
 	Spec  *flux.Spec `json:"spec,omityempty"`
 }
 
+// Valid check if the plan request has a query or spec defined, but not both.
+func (p *postPlanRequest) Valid() error {
+	if p.Query == "" && p.Spec == nil {
+		return errors.MalformedDataf("query or spec required")
+	}
+
+	if p.Query != "" && p.Spec != nil {
+		return errors.MalformedDataf("cannot request both query and spec")
+	}
+	return nil
+}
+
 // postFluxPlan returns a flux plan for provided flux string
 func (h *FluxHandler) postFluxPlan(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
@@ -252,15 +264,7 @@ func decodePostPlanRequest(ctx context.Context, r *http.Request) (*postPlanReque
 		return nil, errors.MalformedDataf("invalid json: %v", err)
 	}
 
-	if req.Query == "" && req.Spec == nil {
-		return nil, errors.MalformedDataf("query or spec required")
-	}
-
-	if req.Query != "" && req.Spec != nil {
-		return nil, errors.MalformedDataf("cannot request both query and spec")
-	}
-
-	return req, nil
+	return req, req.Valid()
 }
 
 // fluxParams contain flux funciton parameters as defined by the semantic graph
diff --git a/http/query_handler_test.go b/http/query_handler_test.go
index 63be3f43c74..23d7c85688b 100644
--- a/http/query_handler_test.go
+++ b/http/query_handler_test.go
@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/influxdata/flux"
 	"github.com/influxdata/flux/csv"
 	"github.com/influxdata/flux/lang"
 	"github.com/influxdata/platform"
@@ -341,3 +342,54 @@ var crlfPattern = regexp.MustCompile(`\r?\n`)
 func toCRLF(data string) string {
 	return crlfPattern.ReplaceAllString(data, "\r\n")
 }
+
+func Test_postPlanRequest_Valid(t *testing.T) {
+	type fields struct {
+		Query string
+		Spec  *flux.Spec
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		wantErr bool
+	}{
+		{
+			name:    "no query nor spec is an error",
+			fields:  fields{},
+			wantErr: true,
+		},
+		{
+			name: "both query and spec is an error",
+			fields: fields{
+				Query: "from()|>last()",
+				Spec:  &flux.Spec{},
+			},
+			wantErr: true,
+		},
+		{
+
+			name: "request with query is valid",
+			fields: fields{
+				Query: `from(bucket:"telegraf")|> range(start: -5000h)|> filter(fn: (r) => r._measurement == "mem" AND r._field == "used_percent")|> group(by:["host"])|> mean()`,
+			},
+		},
+		{
+
+			name: "request with spec is valid",
+			fields: fields{
+				Spec: &flux.Spec{},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &postPlanRequest{
+				Query: tt.fields.Query,
+				Spec:  tt.fields.Spec,
+			}
+			if err := p.Valid(); (err != nil) != tt.wantErr {
+				t.Errorf("postPlanRequest.Valid() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
diff --git a/write/batcher_test.go b/write/batcher_test.go
index ebc47febfb5..8bd9edf0ca0 100644
--- a/write/batcher_test.go
+++ b/write/batcher_test.go
@@ -398,7 +398,6 @@ func TestBatcher_Write(t *testing.T) {
 }
 
 func TestBatcher_WriteTimeout(t *testing.T) {
-
 	// mocking the write service here to either return an error
 	// or get back all the bytes from the reader.
 	var got string