diff --git a/CHANGELOG.md b/CHANGELOG.md index cc6ce6e..b64a25d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/db/db.go b/db/db.go index a847127..5533265 100644 --- a/db/db.go +++ b/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 diff --git a/db/postgresql/db.go b/db/postgresql/db.go index f9e9628..2d6e3e9 100644 --- a/db/postgresql/db.go +++ b/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) diff --git a/db/postgresql/db_test.go b/db/postgresql/db_test.go index a8272cf..dacca65 100644 --- a/db/postgresql/db_test.go +++ b/db/postgresql/db_test.go @@ -181,7 +181,6 @@ func TestGetRecordIDs(t *testing.T) { expectedSuccessMetric float64 expectedFailureMetric float64 expectedErr error - expectedCalls int }{ { description: "Success", @@ -189,7 +188,6 @@ func TestGetRecordIDs(t *testing.T) { expectedRecords: []db.RecordToDelete{{DeathDate: 222, RecordID: 12345}}, expectedSuccessMetric: 1.0, expectedErr: nil, - expectedCalls: 1, }, { description: "Get Error", @@ -197,7 +195,6 @@ func TestGetRecordIDs(t *testing.T) { 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 { diff --git a/db/postgresql/executer.go b/db/postgresql/executer.go index fd6bc47..1f8552b 100644 --- a/db/postgresql/executer.go +++ b/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 diff --git a/db/postgresql/mocks_test.go b/db/postgresql/mocks_test.go index ef155cf..7f4ec96 100644 --- a/db/postgresql/mocks_test.go +++ b/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 } diff --git a/db/retry/retry.go b/db/retry/retry.go index 85e8229..8f93b19 100644 --- a/db/retry/retry.go +++ b/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 } diff --git a/db/retry/retry_test.go b/db/retry/retry_test.go index 8007766..1977ffc 100644 --- a/db/retry/retry_test.go +++ b/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 {