feat: basic validation to Metric struct (#18)

aerogear-attic · Feb 23, 2018 · a2bade2 · a2bade2
1 parent bc1ee5d
commit a2bade2
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 17 deletions.
diff --git a/pkg/mobile/metrics_test.go b/pkg/mobile/metrics_test.go
@@ -30,13 +30,13 @@ func TestCreateCallsDAOWithCorrectArgs(t *testing.T) {
 
 	metric := Metric{
 		ClientId: "org.aerogear.metrics.tests",
-		Data: MetricData{
-			App: AppMetric{
+		Data: &MetricData{
+			App: &AppMetric{
 				ID:         "12345678",
 				SDKVersion: "1.0.0",
 				AppVersion: "1",
 			},
-			Device: DeviceMetric{
+			Device: &DeviceMetric{
 				Platform:        "Android",
 				PlatformVersion: "27",
 			},
@@ -69,13 +69,13 @@ func TestCreateReturnsErrorFromDAO(t *testing.T) {
 
 	metric := Metric{
 		ClientId: "org.aerogear.metrics.tests",
-		Data: MetricData{
-			App: AppMetric{
+		Data: &MetricData{
+			App: &AppMetric{
 				ID:         "12345678",
 				SDKVersion: "1.0.0",
 				AppVersion: "1",
 			},
-			Device: DeviceMetric{
+			Device: &DeviceMetric{
 				Platform:        "Android",
 				PlatformVersion: "27",
 			},

diff --git a/pkg/mobile/types.go b/pkg/mobile/types.go
@@ -7,14 +7,14 @@ type AppConfig struct {
 // ClientMetric struct is what the client payload should be parsed into
 // Need to figure out how to structure this
 type Metric struct {
-	ClientTimestamp int64      `json:"timestamp"`
-	ClientId        string     `json:"clientId"`
-	Data            MetricData `json:"data"`
+	ClientTimestamp int64       `json:"timestamp,omitempty"`
+	ClientId        string      `json:"clientId"`
+	Data            *MetricData `json:"data,omitempty"`
 }
 
 type MetricData struct {
-	App    AppMetric    `json:"app"`
-	Device DeviceMetric `json:"device"`
+	App    *AppMetric    `json:"app,omitempty"`
+	Device *DeviceMetric `json:"device,omitempty"`
 }
 
 type AppMetric struct {
@@ -27,3 +27,15 @@ type DeviceMetric struct {
 	Platform        string `json:"platform"`
 	PlatformVersion string `json:"platformVersion"`
 }
+
+func (m *Metric) Validate() (valid bool, reason string) {
+	if m.ClientId == "" {
+		return false, "missing clientId in payload"
+	}
+
+	// check if data field was missing or empty object
+	if m.Data == nil || (MetricData{}) == *m.Data {
+		return false, "missing metrics data in payload"
+	}
+	return true, ""
+}
diff --git a/pkg/mobile/types_test.go b/pkg/mobile/types_test.go
@@ -0,0 +1,59 @@
+package mobile
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestMetricValidate(t *testing.T) {
+
+	testCases := []struct {
+		Name           string
+		Metric         Metric
+		Valid          bool
+		ExpectedReason string
+	}{
+		{
+			Name:           "Empty metric should be invalid",
+			Metric:         Metric{},
+			Valid:          false,
+			ExpectedReason: "missing clientId in payload",
+		},
+		{
+			Name:           "Metric with no clientId should be invalid",
+			Metric:         Metric{Data: &MetricData{App: &AppMetric{SDKVersion: "1"}}},
+			Valid:          false,
+			ExpectedReason: "missing clientId in payload",
+		},
+		{
+			Name:           "Metric with no Data should be invalid",
+			Metric:         Metric{ClientId: "org.aerogear.metrics.testing"},
+			Valid:          false,
+			ExpectedReason: "missing metrics data in payload",
+		},
+		{
+			Name:           "Metric with empty Data should be invalid",
+			Metric:         Metric{ClientId: "org.aerogear.metrics.testing", Data: &MetricData{}},
+			Valid:          false,
+			ExpectedReason: "missing metrics data in payload",
+		},
+		{
+			Name:           "Metric with ClientId and Some Data should be valid",
+			Metric:         Metric{ClientId: "org.aerogear.metrics.testing", Data: &MetricData{App: &AppMetric{SDKVersion: "1"}}},
+			Valid:          true,
+			ExpectedReason: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		valid, reason := tc.Metric.Validate()
+
+		if valid != tc.Valid {
+			fmt.Errorf("case failed: %s. Expected: %v, got %v", tc.Name, tc.Valid, valid)
+		}
+
+		if reason != tc.ExpectedReason {
+			fmt.Errorf("case failed: %s. Expected: %v, got %v", tc.Name, tc.ExpectedReason, reason)
+		}
+	}
+}
diff --git a/pkg/web/metricsHandler.go b/pkg/web/metricsHandler.go
@@ -27,6 +27,11 @@ func (mh *metricsHandler) CreateMetric(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if valid, reason := metric.Validate(); !valid {
+		boom.BadRequest(w, reason)
+		return
+	}
+
 	// create the record in the db
 	result, err := mh.metricService.Create(metric)
 

diff --git a/pkg/web/metricsHandler_test.go b/pkg/web/metricsHandler_test.go
@@ -35,13 +35,13 @@ func TestMetricsEndpointShouldPassReceivedDataToMetricsService(t *testing.T) {
 	metric := mobile.Metric{
 		ClientTimestamp: 1234,
 		ClientId:        "client123",
-		Data: mobile.MetricData{
-			App: mobile.AppMetric{
+		Data: &mobile.MetricData{
+			App: &mobile.AppMetric{
 				ID:         "deadbeef",
 				SDKVersion: "1.2.3",
 				AppVersion: "27",
 			},
-			Device: mobile.DeviceMetric{
+			Device: &mobile.DeviceMetric{
 				Platform:        "android",
 				PlatformVersion: "19",
 			},
@@ -73,13 +73,13 @@ func TestMetricsEndpointShouldReturn500WhenThereIsAnErrorInMetricsService(t *tes
 	metric := mobile.Metric{
 		ClientTimestamp: 1234,
 		ClientId:        "client123",
-		Data: mobile.MetricData{
-			App: mobile.AppMetric{
+		Data: &mobile.MetricData{
+			App: &mobile.AppMetric{
 				ID:         "deadbeef",
 				SDKVersion: "1.2.3",
 				AppVersion: "27",
 			},
-			Device: mobile.DeviceMetric{
+			Device: &mobile.DeviceMetric{
 				Platform:        "android",
 				PlatformVersion: "19",
 			},