fix: save client timestamp in database if present (#31)

* fix: use varchar(128) for clientId * fix: enforce clientId max length at app layer * fix: save client timestamp in database * fix: typo in comment * fix: update tests
aerogear-attic · Mar 1, 2018 · dcc91f8 · dcc91f8
1 parent 1478ca7
commit dcc91f8
Show file tree

Hide file tree

Showing 11 changed files with 125 additions and 19 deletions.
diff --git a/Gopkg.lock b/Gopkg.lock
diff --git a/pkg/dao/dao.go b/pkg/dao/dao.go
@@ -2,15 +2,16 @@ package dao
 
 import (
 	"database/sql"
+	"time"
 )
 
 type MetricsDAO struct {
 	db *sql.DB
 }
 
 // Create a metrics record
-func (m *MetricsDAO) Create(clientId string, metricsData []byte) error {
-	_, err := m.db.Exec("INSERT INTO mobileappmetrics(clientId, data) VALUES($1, $2)", clientId, metricsData)
+func (m *MetricsDAO) Create(clientId string, metricsData []byte, clientTime *time.Time) error {
+	_, err := m.db.Exec("INSERT INTO mobileappmetrics(clientId, data, client_time) VALUES($1, $2, $3)", clientId, metricsData, clientTime)
 	return err
 }
 

diff --git a/pkg/dao/dao_test.go b/pkg/dao/dao_test.go
@@ -3,6 +3,7 @@ package dao
 
 import (
 	"testing"
+	"time"
 
 	"github.com/aerogear/aerogear-app-metrics/pkg/config"
 	"github.com/stretchr/testify/mock"
@@ -81,7 +82,7 @@ func TestCreate(t *testing.T) {
 	clientId := "org.aerogear.metrics.testing"
 	metricsData := []byte("{\"app\":{\"id\":\"com.example.someApp\",\"sdkVersion\":\"2.4.6\",\"appVersion\":\"256\"},\"device\":{\"platform\":\"android\",\"platformVersion\":\"27\"}}")
 
-	err = dao.Create(clientId, metricsData)
+	err = dao.Create(clientId, metricsData, nil)
 
 	if err != nil {
 		t.Errorf("Create() returned an error %s", err.Error())
@@ -103,7 +104,7 @@ func TestCreateBadJSON(t *testing.T) {
 	clientId := "org.aerogear.metrics.testing"
 	metricsData := []byte("InvalidJSON")
 
-	err = dao.Create(clientId, metricsData)
+	err = dao.Create(clientId, metricsData, nil)
 
 	if err == nil {
 		t.Errorf("Create() with invalid JSON did not return an error")
@@ -131,9 +132,38 @@ func TestCreateEmptyClientID(t *testing.T) {
 	clientId := ""
 	metricsData := []byte("{\"app\":{\"id\":\"com.example.someApp\",\"sdkVersion\":\"2.4.6\",\"appVersion\":\"256\"},\"device\":{\"platform\":\"android\",\"platformVersion\":\"27\"}}")
 
-	err = dao.Create(clientId, metricsData)
+	err = dao.Create(clientId, metricsData, nil)
 
 	if err == nil {
 		t.Errorf("Create() with empty clientId did not return an error")
 	}
 }
+
+func TestCreateClientTimestamp(t *testing.T) {
+	config := config.GetConfig()
+	dbHandler := DatabaseHandler{}
+
+	err := dbHandler.Connect(config.DBConnectionString, config.DBMaxConnections)
+
+	if err != nil {
+		t.Errorf("Connect() returned an error: %s", err.Error())
+	}
+
+	dbHandler.DoInitialSetup()
+
+	if err != nil {
+		t.Errorf("Connect() returned an error: %s", err.Error())
+	}
+
+	dao := NewMetricsDAO(dbHandler.DB)
+
+	clientId := "org.aerogear.metrics.testing"
+	metricsData := []byte("{\"app\":{\"id\":\"com.example.someApp\",\"sdkVersion\":\"2.4.6\",\"appVersion\":\"256\"},\"device\":{\"platform\":\"android\",\"platformVersion\":\"27\"}}")
+	time := time.Now()
+
+	err = dao.Create(clientId, metricsData, &time)
+
+	if err != nil {
+		t.Errorf("Create() returned an error %s", err.Error())
+	}
+}
diff --git a/pkg/dao/db.go b/pkg/dao/db.go
@@ -52,7 +52,7 @@ func (handler *DatabaseHandler) DoInitialSetup() error {
 	if handler.DB == nil {
 		return errors.New("cannot setup database, must call Connect() first")
 	}
-	if _, err := handler.DB.Exec("CREATE TABLE IF NOT EXISTS mobileappmetrics(clientId varchar NOT NULL CHECK (clientId <> ''), event_time timestamptz NOT NULL DEFAULT now() Not NULL, data jsonb)"); err != nil {
+	if _, err := handler.DB.Exec("CREATE TABLE IF NOT EXISTS mobileappmetrics(clientId varchar NOT NULL CHECK (clientId <> ''), event_time timestamptz NOT NULL DEFAULT now(), client_time timestamptz DEFAULT now(), data jsonb NOT NULL)"); err != nil {
 		return err
 	}
 	return nil

diff --git a/pkg/mobile/interfaces.go b/pkg/mobile/interfaces.go
@@ -1,6 +1,8 @@
 package mobile
 
+import "time"
+
 // MetricCreator defines how a metric can be created
 type MetricCreator interface {
-	Create(clientId string, metricsData []byte) error
+	Create(clientId string, metricsData []byte, clientTime *time.Time) error
 }
diff --git a/pkg/mobile/metrics.go b/pkg/mobile/metrics.go
@@ -1,6 +1,9 @@
 package mobile
 
-import "encoding/json"
+import (
+	"encoding/json"
+	"time"
+)
 
 type MetricsService struct {
 	mdao MetricCreator
@@ -17,5 +20,15 @@ func (m MetricsService) Create(metric Metric) (Metric, error) {
 		return metric, err
 	}
 
-	return metric, m.mdao.Create(metric.ClientId, metricsData)
+	t, err := metric.ClientTimestamp.Int64()
+
+	// happens if timestamp is empty
+	if err != nil {
+		return metric, m.mdao.Create(metric.ClientId, metricsData, nil)
+	}
+
+	// convert to time object
+	clientTime := time.Unix(t, 0)
+
+	return metric, m.mdao.Create(metric.ClientId, metricsData, &clientTime)
 }
diff --git a/pkg/mobile/metrics_test.go b/pkg/mobile/metrics_test.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/mock"
 )
@@ -13,8 +14,8 @@ type MetricsDAOMock struct {
 	mock.Mock
 }
 
-func (m *MetricsDAOMock) Create(clientId string, metricsData []byte) error {
-	args := m.Called(clientId, metricsData)
+func (m *MetricsDAOMock) Create(clientId string, metricsData []byte, clientTime *time.Time) error {
+	args := m.Called(clientId, metricsData, clientTime)
 	return args.Error(0)
 }
 
@@ -50,7 +51,7 @@ func TestCreateCallsDAOWithCorrectArgs(t *testing.T) {
 
 	mdaoMock, ms := newTestMetricsService()
 
-	mdaoMock.On("Create", metric.ClientId, expectedMetricsData).Return(nil)
+	mdaoMock.On("Create", metric.ClientId, expectedMetricsData, (*time.Time)(nil)).Return(nil)
 
 	res, err := ms.Create(metric)
 
@@ -90,7 +91,7 @@ func TestCreateReturnsErrorFromDAO(t *testing.T) {
 	mdaoMock, ms := newTestMetricsService()
 
 	daoError := errors.New("problem connecting to db")
-	mdaoMock.On("Create", metric.ClientId, expectedMetricsData).Return(daoError)
+	mdaoMock.On("Create", metric.ClientId, expectedMetricsData, (*time.Time)(nil)).Return(daoError)
 
 	_, err = ms.Create(metric)
 
@@ -100,3 +101,43 @@ func TestCreateReturnsErrorFromDAO(t *testing.T) {
 
 	mdaoMock.AssertExpectations(t)
 }
+
+func TestCreateCallsDaoWithCorrectTimestamp(t *testing.T) {
+
+	metric := Metric{
+		ClientId:        "org.aerogear.metrics.tests",
+		ClientTimestamp: "12345",
+		Data: &MetricData{
+			App: &AppMetric{
+				ID:         "12345678",
+				SDKVersion: "1.0.0",
+				AppVersion: "1",
+			},
+			Device: &DeviceMetric{
+				Platform:        "Android",
+				PlatformVersion: "27",
+			},
+		},
+	}
+	expectedMetricsData, err := json.Marshal(metric.Data)
+	expectedTimestamp := time.Unix(12345, 0)
+
+	if err != nil {
+		t.Errorf("could not encode metric object to JSON")
+	}
+
+	mdaoMock, ms := newTestMetricsService()
+
+	mdaoMock.On("Create", metric.ClientId, expectedMetricsData, &expectedTimestamp).Return(nil)
+	res, err := ms.Create(metric)
+
+	if err != nil {
+		t.Errorf("Metrics Service should not have returned an error")
+	}
+
+	if reflect.DeepEqual(reflect.ValueOf(metric), reflect.ValueOf(res)) {
+		t.Errorf("failed")
+	}
+
+	mdaoMock.AssertExpectations(t)
+}
diff --git a/pkg/mobile/types.go b/pkg/mobile/types.go
@@ -1,6 +1,7 @@
 package mobile
 
 import (
+	"encoding/json"
 	"fmt"
 )
 
@@ -11,7 +12,7 @@ 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,omitempty"`
+	ClientTimestamp json.Number `json:"timestamp,omitempty"`
 	ClientId        string      `json:"clientId"`
 	Data            *MetricData `json:"data,omitempty"`
 }
@@ -45,6 +46,12 @@ func (m *Metric) Validate() (valid bool, reason string) {
 		return false, clientIdLengthError
 	}
 
+	if m.ClientTimestamp != "" {
+		if _, err := m.ClientTimestamp.Int64(); err != nil {
+			return false, "timestamp must be a valid number"
+		}
+	}
+
 	// check if data field was missing or empty object
 	if m.Data == nil || (MetricData{}) == *m.Data {
 		return false, "missing metrics data in payload"

diff --git a/pkg/mobile/types_test.go b/pkg/mobile/types_test.go
@@ -50,6 +50,18 @@ func TestMetricValidate(t *testing.T) {
 			Valid:          true,
 			ExpectedReason: "",
 		},
+		{
+			Name:           "Metric with bad timestamp should be invalid",
+			Metric:         Metric{ClientId: "org.aerogear.metrics.testing", ClientTimestamp: "invalid", Data: &MetricData{App: &AppMetric{SDKVersion: "1"}}},
+			Valid:          false,
+			ExpectedReason: "timestamp must be a valid number",
+		},
+		{
+			Name:           "Metric with valid timestamp should be valid",
+			Metric:         Metric{ClientId: "org.aerogear.metrics.testing", ClientTimestamp: "12345", Data: &MetricData{App: &AppMetric{SDKVersion: "1"}}},
+			Valid:          true,
+			ExpectedReason: "",
+		},
 	}
 
 	for _, tc := range testCases {

diff --git a/pkg/web/metricsHandler.go b/pkg/web/metricsHandler.go
@@ -42,7 +42,7 @@ func (mh *metricsHandler) CreateMetric(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := withJSON(w, 200, result); err != nil {
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		boom.BadImplementation(w)
 		return
 	}
 }
diff --git a/pkg/web/metricsHandler_test.go b/pkg/web/metricsHandler_test.go
@@ -33,7 +33,7 @@ func setupMetricsHandler(service MetricsServiceInterface) *httptest.Server {
 
 func TestMetricsEndpointShouldPassReceivedDataToMetricsService(t *testing.T) {
 	metric := mobile.Metric{
-		ClientTimestamp: 1234,
+		ClientTimestamp: "1234",
 		ClientId:        "client123",
 		Data: &mobile.MetricData{
 			App: &mobile.AppMetric{
@@ -71,7 +71,7 @@ func TestMetricsEndpointShouldPassReceivedDataToMetricsService(t *testing.T) {
 
 func TestMetricsEndpointShouldReturn500WhenThereIsAnErrorInMetricsService(t *testing.T) {
 	metric := mobile.Metric{
-		ClientTimestamp: 1234,
+		ClientTimestamp: "1234",
 		ClientId:        "client123",
 		Data: &mobile.MetricData{
 			App: &mobile.AppMetric{