Added a way to get a list of device ids (#83)

Author: kristinapathak

CHANGELOG.md

@@ -5,7 +5,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+
+## [v0.8.0]
+- Refactored pruning
 - Added comments for godocs
+- Added a way to get a list of device ids
 
 ## [v0.7.0]
 - Added `record_id` to index
@@ -122,7 +126,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Initial creation
 - Created `db` and `xvault` package
 
-[Unreleased]: https://github.com/Comcast/codex/compare/v0.7.0..HEAD
+[Unreleased]: https://github.com/Comcast/codex/compare/v0.8.0..HEAD
+[v0.8.0]: https://github.com/Comcast/codex/compare/v0.7.0...v0.8.0
 [v0.7.0]: https://github.com/Comcast/codex/compare/v0.6.0...v0.7.0
 [v0.6.0]: https://github.com/Comcast/codex/compare/v0.5.0...v0.6.0
 [v0.5.0]: https://github.com/Comcast/codex/compare/v0.4.3...v0.5.0

db/db.go

@@ -24,13 +24,12 @@ const (
 	// TypeLabel is for labeling metrics; if there is a single metric for
 	// successful queries, the typeLabel and corresponding type can be used
 	// when incrementing the metric.
-	TypeLabel  = "type"
-	InsertType = "insert"
-	DeleteType = "delete"
-	ReadType   = "read"
-	PingType   = "ping"
-	// ListReadType is for reading from the blacklist.
-	ListReadType = "listRead"
+	TypeLabel         = "type"
+	InsertType        = "insert"
+	DeleteType        = "delete"
+	ReadType          = "read"
+	PingType          = "ping"
+	BlacklistReadType = "blacklistRead"
 )
 
 // Record is the struct used to insert an event into the database.  It includes

db/postgresql/db.go

@@ -80,14 +80,15 @@ type Config struct {
 // Connection manages the connection to the postgresql database, and maintains
 // a health check on the database connection.
 type Connection struct {
-	finder      finder
-	findList    findList
-	mutliInsert multiinserter
-	deleter     deleter
-	closer      closer
-	pinger      pinger
-	stats       stats
-	gennericDB  *sql.DB
+	finder       finder
+	findList     findList
+	deviceFinder deviceFinder
+	mutliInsert  multiinserter
+	deleter      deleter
+	closer       closer
+	pinger       pinger
+	stats        stats
+	gennericDB   *sql.DB
 
 	pruneLimit  int
 	health      *health.Health
@@ -150,6 +151,7 @@ func CreateDbConnection(config Config, provider provider.Provider, health *healt
 
 	dbConn.finder = conn
 	dbConn.findList = conn
+	dbConn.deviceFinder = conn
 	dbConn.mutliInsert = conn
 	dbConn.deleter = conn
 	dbConn.closer = conn
@@ -293,13 +295,25 @@ func (c *Connection) GetRecordsToDelete(shard int, limit int, deathDate int64) (
 func (c *Connection) GetBlacklist() (list []blacklist.BlackListedItem, err error) {
 	err = c.findList.findBlacklist(&list)
 	if err != nil {
-		c.measures.SQLQueryFailureCount.With(db.TypeLabel, db.ListReadType).Add(1.0)
+		c.measures.SQLQueryFailureCount.With(db.TypeLabel, db.BlacklistReadType).Add(1.0)
 		return []blacklist.BlackListedItem{}, emperror.WrapWith(err, "Getting records from database failed")
 	}
-	c.measures.SQLQuerySuccessCount.With(db.TypeLabel, db.ListReadType).Add(1.0)
+	c.measures.SQLQuerySuccessCount.With(db.TypeLabel, db.BlacklistReadType).Add(1.0)
 	return
 }
 
+// GetDeviceList returns a list of device ids where the device id is greater
+// than the offset device id.
+func (c *Connection) GetDeviceList(offset string, limit int) ([]string, error) {
+	list, err := c.deviceFinder.getList(offset, limit)
+	if err != nil {
+		c.measures.SQLQueryFailureCount.With(db.TypeLabel, db.ReadType).Add(1.0)
+		return []string{}, emperror.WrapWith(err, "Getting list of devices from database failed")
+	}
+	c.measures.SQLQuerySuccessCount.With(db.TypeLabel, db.ReadType).Add(1.0)
+	return list, nil
+}
+
 // DeleteRecord removes a record.
 func (c *Connection) DeleteRecord(shard int, deathDate int64, recordID int64) error {
 	rowsAffected, err := c.deleter.delete(&db.Record{}, 1, "shard = ? AND death_date = ? AND record_id = ?", shard, deathDate, recordID)

db/postgresql/db_test.go

@@ -181,23 +181,20 @@ func TestGetRecordIDs(t *testing.T) {
 		expectedSuccessMetric float64
 		expectedFailureMetric float64
 		expectedErr           error
-		expectedCalls         int
 	}{
 		{
 			description:           "Success",
 			deviceID:              "1234",
 			expectedRecords:       []db.RecordToDelete{{DeathDate: 222, RecordID: 12345}},
 			expectedSuccessMetric: 1.0,
 			expectedErr:           nil,
-			expectedCalls:         1,
 		},
 		{
 			description:           "Get Error",
 			deviceID:              "1234",
 			expectedRecords:       []db.RecordToDelete{},
 			expectedFailureMetric: 1.0,
 			expectedErr:           errors.New("test Get error"),
-			expectedCalls:         1,
 		},
 	}
 
@@ -211,9 +208,7 @@ func TestGetRecordIDs(t *testing.T) {
 				measures: m,
 				finder:   mockObj,
 			}
-			if tc.expectedCalls > 0 {
-				mockObj.On("findRecordsToDelete", mock.Anything, mock.Anything, mock.Anything).Return(tc.expectedRecords, tc.expectedErr).Times(tc.expectedCalls)
-			}
+			mockObj.On("findRecordsToDelete", mock.Anything, mock.Anything, mock.Anything).Return(tc.expectedRecords, tc.expectedErr).Once()
 			p.Assert(t, SQLQuerySuccessCounter)(xmetricstest.Value(0.0))
 			p.Assert(t, SQLQueryFailureCounter)(xmetricstest.Value(0.0))
 
@@ -231,6 +226,56 @@ func TestGetRecordIDs(t *testing.T) {
 	}
 }
 
+func TestDeviceList(t *testing.T) {
+	tests := []struct {
+		description           string
+		expectedIDs           []string
+		expectedSuccessMetric float64
+		expectedFailureMetric float64
+		expectedErr           error
+	}{
+		{
+			description:           "Success",
+			expectedIDs:           []string{"aaa", "bbb", "ccc"},
+			expectedSuccessMetric: 1.0,
+			expectedErr:           nil,
+		},
+		{
+			description:           "Get Error",
+			expectedIDs:           []string{},
+			expectedFailureMetric: 1.0,
+			expectedErr:           errors.New("test Get error"),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.description, func(t *testing.T) {
+			assert := assert.New(t)
+			mockObj := new(mockDeviceFinder)
+			p := xmetricstest.NewProvider(nil, Metrics)
+			m := NewMeasures(p)
+			dbConnection := Connection{
+				measures:     m,
+				deviceFinder: mockObj,
+			}
+			mockObj.On("getList", mock.Anything, mock.Anything, mock.Anything).Return(tc.expectedIDs, tc.expectedErr).Once()
+			p.Assert(t, SQLQuerySuccessCounter)(xmetricstest.Value(0.0))
+			p.Assert(t, SQLQueryFailureCounter)(xmetricstest.Value(0.0))
+
+			result, err := dbConnection.GetDeviceList("", 10)
+			mockObj.AssertExpectations(t)
+			p.Assert(t, SQLQuerySuccessCounter, db.TypeLabel, db.ReadType)(xmetricstest.Value(tc.expectedSuccessMetric))
+			p.Assert(t, SQLQueryFailureCounter, db.TypeLabel, db.ReadType)(xmetricstest.Value(tc.expectedFailureMetric))
+			if tc.expectedErr == nil || err == nil {
+				assert.Equal(tc.expectedErr, err)
+			} else {
+				assert.Contains(err.Error(), tc.expectedErr.Error())
+			}
+			assert.Equal(tc.expectedIDs, result)
+		})
+	}
+}
+
 func TestPruneRecords(t *testing.T) {
 	pruneTestErr := errors.New("test prune history error")
 	tests := []struct {

db/postgresql/executer.go

@@ -39,6 +39,9 @@ type (
 	findList interface {
 		findBlacklist(out *[]blacklist.BlackListedItem) error
 	}
+	deviceFinder interface {
+		getList(offset string, limit int, where ...interface{}) ([]string, error)
+	}
 	multiinserter interface {
 		insert(records []db.Record) (int64, error)
 	}
@@ -79,6 +82,14 @@ func (b *dbDecorator) findBlacklist(out *[]blacklist.BlackListedItem) error {
 	return db.Error
 }
 
+func (b *dbDecorator) getList(offset string, limit int, where ...interface{}) ([]string, error) {
+	var result []string
+	// Raw SQL
+	db := b.Raw("SELECT device_id from devices.events WHERE device_id > ? GROUP BY device_id LIMIT ?", offset, limit).Pluck("device_id", &result)
+	//db := b.Limit(limit).Select("device_id").Find(&[]Record{}, where).Group("device_id").Where("device_id > ?", offset).Pluck("device_id", &result)
+	return result, db.Error
+}
+
 func (b *dbDecorator) insert(records []db.Record) (int64, error) {
 	if len(records) == 0 {
 		return 0, errNoEvents

db/postgresql/mocks_test.go

@@ -42,6 +42,15 @@ func (f *mockFinder) findRecordsToDelete(limit int, shard int, deathDate int64)
 	return args.Get(0).([]db.RecordToDelete), args.Error(1)
 }
 
+type mockDeviceFinder struct {
+	mock.Mock
+}
+
+func (df *mockDeviceFinder) getList(offset string, limit int, where ...interface{}) ([]string, error) {
+	args := df.Called(offset, limit, where)
+	return args.Get(0).([]string), args.Error(1)
+}
+
 type mockMultiInsert struct {
 	mock.Mock
 }

db/retry/retry.go

@@ -265,7 +265,7 @@ func (ltg RetryListGService) GetBlacklist() (list []blacklist.BlackListedItem, e
 	sleepTime := ltg.config.interval
 	for i := 0; i < retries+1; i++ {
 		if i > 0 {
-			ltg.config.measures.SQLQueryRetryCount.With(db.TypeLabel, db.ListReadType).Add(1.0)
+			ltg.config.measures.SQLQueryRetryCount.With(db.TypeLabel, db.BlacklistReadType).Add(1.0)
 			ltg.config.sleep(sleepTime)
 			sleepTime = sleepTime * ltg.config.intervalMult
 		}
@@ -274,7 +274,7 @@ func (ltg RetryListGService) GetBlacklist() (list []blacklist.BlackListedItem, e
 		}
 	}
 
-	ltg.config.measures.SQLQueryEndCount.With(db.TypeLabel, db.ListReadType).Add(1.0)
+	ltg.config.measures.SQLQueryEndCount.With(db.TypeLabel, db.BlacklistReadType).Add(1.0)
 	return
 }
 

db/retry/retry_test.go

@@ -389,8 +389,8 @@ func TestRetryGetBlacklist(t *testing.T) {
 			p.Assert(t, SQLQueryEndCounter)(xmetricstest.Value(0.0))
 			_, err := retryListGService.GetBlacklist()
 			mockObj.AssertExpectations(t)
-			p.Assert(t, SQLQueryRetryCounter, db.TypeLabel, db.ListReadType)(xmetricstest.Value(tc.expectedRetryMetric))
-			p.Assert(t, SQLQueryEndCounter, db.TypeLabel, db.ListReadType)(xmetricstest.Value(1.0))
+			p.Assert(t, SQLQueryRetryCounter, db.TypeLabel, db.BlacklistReadType)(xmetricstest.Value(tc.expectedRetryMetric))
+			p.Assert(t, SQLQueryEndCounter, db.TypeLabel, db.BlacklistReadType)(xmetricstest.Value(1.0))
 			if tc.expectedErr == nil || err == nil {
 				assert.Equal(tc.expectedErr, err)
 			} else {