From 23549abfe41cb10f28cf24d8bf071e0de023f618 Mon Sep 17 00:00:00 2001 From: Darin Krauss Date: Tue, 6 Feb 2024 18:45:11 -0800 Subject: [PATCH 1/2] [BACK-2866] Fix Dexcom task failing for newly created Dexcom connections - Fix Dexcom task failing for newly created Dexcom connections - Only set CreatedUserID in UsersDataSetsCreate if user AuthDetails - Always add error to Dexcom tasks whether or not task should fail - Update tests - Update .gitignore and .dockerignore - https://tidepool.atlassian.net/browse/BACK-2866 --- .dockerignore | 1 + .gitignore | 3 + data/service/api/v1/users_datasets_create.go | 4 +- .../api/v1/users_datasets_create_test.go | 107 +++++++++++------- dexcom/fetch/task.go | 6 +- dexcom/fetch/task_test.go | 30 ++--- 6 files changed, 94 insertions(+), 57 deletions(-) diff --git a/.dockerignore b/.dockerignore index 36e828ee9..9bea9007d 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,5 +1,6 @@ *.coverprofile _bin/ _tmp/ +coverprofile.out debug deploy/ diff --git a/.gitignore b/.gitignore index 18555ba6e..c72a35ee3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.coverprofile +*.env *.test /_bin /_data @@ -7,3 +8,5 @@ /deploy /.idea .DS_Store +.envrc +coverprofile.out diff --git a/data/service/api/v1/users_datasets_create.go b/data/service/api/v1/users_datasets_create.go index 875ce6334..f83878a6e 100644 --- a/data/service/api/v1/users_datasets_create.go +++ b/data/service/api/v1/users_datasets_create.go @@ -69,7 +69,9 @@ func UsersDataSetsCreate(dataServiceContext dataService.Context) { } dataSet.SetUserID(&targetUserID) - dataSet.SetCreatedUserID(pointer.FromString(details.UserID())) + if details.IsUser() { + dataSet.SetCreatedUserID(pointer.FromString(details.UserID())) + } dataSet.Normalize(normalizer) diff --git a/data/service/api/v1/users_datasets_create_test.go b/data/service/api/v1/users_datasets_create_test.go index 588abb1af..839bf5eef 100644 --- a/data/service/api/v1/users_datasets_create_test.go +++ b/data/service/api/v1/users_datasets_create_test.go @@ -5,46 +5,73 @@ import ( "net/http" "strings" - "github.com/ant0ine/go-json-rest/rest" . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/ant0ine/go-json-rest/rest" "github.com/tidepool-org/platform/alerts" "github.com/tidepool-org/platform/auth" dataClient "github.com/tidepool-org/platform/data/client" "github.com/tidepool-org/platform/data/deduplicator" - deduplicatortest "github.com/tidepool-org/platform/data/deduplicator/test" + dataDeduplicatorTest "github.com/tidepool-org/platform/data/deduplicator/test" v1 "github.com/tidepool-org/platform/data/service/api/v1" "github.com/tidepool-org/platform/data/service/api/v1/mocks" dataSource "github.com/tidepool-org/platform/data/source" dataStore "github.com/tidepool-org/platform/data/store" - datatest "github.com/tidepool-org/platform/data/store/test" + dataStoreTest "github.com/tidepool-org/platform/data/store/test" "github.com/tidepool-org/platform/data/summary" "github.com/tidepool-org/platform/data/types/upload" - uploadtest "github.com/tidepool-org/platform/data/types/upload/test" + dataTypesUploadTest "github.com/tidepool-org/platform/data/types/upload/test" "github.com/tidepool-org/platform/log" logtest "github.com/tidepool-org/platform/log/test" "github.com/tidepool-org/platform/metric" - metrictest "github.com/tidepool-org/platform/metric/test" + metricTest "github.com/tidepool-org/platform/metric/test" "github.com/tidepool-org/platform/permission" "github.com/tidepool-org/platform/pointer" "github.com/tidepool-org/platform/request" "github.com/tidepool-org/platform/service" - syncTaskStore "github.com/tidepool-org/platform/synctask/store" + synctaskStore "github.com/tidepool-org/platform/synctask/store" ) var _ = Describe("UsersDataSetsCreate", func() { - It("sets the CreatedUserID", func() { - dataServiceContext := newMockDataServiceContext(GinkgoT()) - dataServiceContext.UploadTester = func(t testingT, up *upload.Upload) { - exp := "testuser001" - if up.CreatedUserID == nil { - t.Fatalf("expected %q, got %v", exp, up.CreatedUserID) + Context("CreatedUserID", func() { + It("does set the CreatedUserID if the auth details are for a user", func() { + dataServiceContext := newMockDataServiceContext(GinkgoT()) + dataServiceContext.AuthDetails = request.NewAuthDetails(request.MethodAccessToken, "test-auth-details-user-id", "token") + dataServiceContext.UploadTester = func(t testingT, up *upload.Upload) { + Expect(up.CreatedUserID).ToNot(BeNil()) + Expect(*up.CreatedUserID).To(Equal("test-deduplicator-created-user-id")) } - if *up.CreatedUserID != exp { - t.Errorf("expected %q, got %q", exp, *up.CreatedUserID) + + v1.UsersDataSetsCreate(dataServiceContext) + + Expect(dataServiceContext.dataDeduplicatorFactory.NewInputs).To(HaveLen(1)) + Expect(dataServiceContext.dataDeduplicatorFactory.NewInputs[0]).ToNot(BeNil()) + Expect(dataServiceContext.dataDeduplicatorFactory.NewInputs[0].CreatedUserID).ToNot(BeNil()) + Expect(*dataServiceContext.dataDeduplicatorFactory.NewInputs[0].CreatedUserID).To(Equal("test-auth-details-user-id")) + + dataServiceContext.dataDeduplicator.AssertOutputsEmpty() + dataServiceContext.dataDeduplicatorFactory.AssertOutputsEmpty() + }) + + It("does not set the CreatedUserID if the auth details are not for a user", func() { + dataServiceContext := newMockDataServiceContext(GinkgoT()) + dataServiceContext.AuthDetails = request.NewAuthDetails(request.MethodServiceSecret, "", "token") + dataServiceContext.UploadTester = func(t testingT, up *upload.Upload) { + Expect(up.CreatedUserID).ToNot(BeNil()) + Expect(*up.CreatedUserID).To(Equal("test-deduplicator-created-user-id")) } - } - v1.UsersDataSetsCreate(dataServiceContext) + + v1.UsersDataSetsCreate(dataServiceContext) + + Expect(dataServiceContext.dataDeduplicatorFactory.NewInputs).To(HaveLen(1)) + Expect(dataServiceContext.dataDeduplicatorFactory.NewInputs[0]).ToNot(BeNil()) + Expect(dataServiceContext.dataDeduplicatorFactory.NewInputs[0].CreatedUserID).To(BeNil()) + + dataServiceContext.dataDeduplicator.AssertOutputsEmpty() + dataServiceContext.dataDeduplicatorFactory.AssertOutputsEmpty() + }) }) }) @@ -55,13 +82,30 @@ type testingT interface { type mockDataServiceContext struct { t testingT + + dataDeduplicator *dataDeduplicatorTest.Deduplicator + dataDeduplicatorFactory *dataDeduplicatorTest.Factory + + AuthDetails request.AuthDetails + // UploadTester tests the resulting upload. UploadTester func(testingT, *upload.Upload) } func newMockDataServiceContext(t testingT) *mockDataServiceContext { + dataSet := dataTypesUploadTest.RandomUpload() + dataSet.CreatedUserID = pointer.FromString("test-deduplicator-created-user-id") + + dataDeduplicator := dataDeduplicatorTest.NewDeduplicator() + dataDeduplicator.OpenOutputs = []dataDeduplicatorTest.OpenOutput{{DataSet: dataSet, Error: nil}} + + dataDeduplicatorFactory := dataDeduplicatorTest.NewFactory() + dataDeduplicatorFactory.NewOutput = &dataDeduplicatorTest.NewOutput{Deduplicator: dataDeduplicator, Error: nil} + return &mockDataServiceContext{ - t: t, + t: t, + dataDeduplicator: dataDeduplicator, + dataDeduplicatorFactory: dataDeduplicatorFactory, } } @@ -77,15 +121,14 @@ func (c *mockDataServiceContext) Request() *rest.Request { testLogger := logtest.NewLogger() r = r.WithContext(log.NewContextWithLogger(r.Context(), testLogger)) - authDetails := request.NewAuthDetails("method", "test", "token") - r = r.WithContext(request.NewContextWithAuthDetails(r.Context(), authDetails)) + r = r.WithContext(request.NewContextWithAuthDetails(r.Context(), c.AuthDetails)) r.Body = io.NopCloser(strings.NewReader(`{}`)) rr := &rest.Request{ Request: r, PathParams: map[string]string{ - "userId": "test", + "userId": "test-path-params-user-id", }, } @@ -120,7 +163,7 @@ func (c *mockDataServiceContext) AuthClient() auth.Client { } func (c *mockDataServiceContext) MetricClient() metric.Client { - mc := metrictest.NewClient() + mc := metricTest.NewClient() mc.RecordMetricOutputs = []error{nil} return mc } @@ -137,27 +180,11 @@ func (c *mockDataServiceContext) PermissionClient() permission.Client { } func (c *mockDataServiceContext) DataDeduplicatorFactory() deduplicator.Factory { - d := deduplicatortest.NewDeduplicator() - up := uploadtest.RandomUpload() - up.CreatedUserID = pointer.FromString("testuser001") - d.OpenOutputs = []deduplicatortest.OpenOutput{ - { - DataSet: up, - Error: nil, - }, - } - f := deduplicatortest.NewFactory() - f.NewOutputs = []deduplicatortest.NewOutput{ - { - Deduplicator: d, - Error: nil, - }, - } - return f + return c.dataDeduplicatorFactory } func (c *mockDataServiceContext) DataRepository() dataStore.DataRepository { - r := datatest.NewDataRepository() + r := dataStoreTest.NewDataRepository() r.CreateDataSetOutputs = []error{nil} return r } @@ -166,7 +193,7 @@ func (c *mockDataServiceContext) SummaryRepository() dataStore.SummaryRepository panic("not implemented") // TODO: Implement } -func (c *mockDataServiceContext) SyncTaskRepository() syncTaskStore.SyncTaskRepository { +func (c *mockDataServiceContext) SyncTaskRepository() synctaskStore.SyncTaskRepository { panic("not implemented") // TODO: Implement } diff --git a/dexcom/fetch/task.go b/dexcom/fetch/task.go index 9cd039726..67a56b134 100644 --- a/dexcom/fetch/task.go +++ b/dexcom/fetch/task.go @@ -35,9 +35,9 @@ func NewTaskCreate(providerSessionID string, dataSourceID string) (*task.TaskCre } func ErrorOrRetryTask(t *task.Task, err error) { + t.AppendError(err) if t.IsFailed() { - if shouldTaskError(t) { - t.AppendError(err) + if shouldTaskFail(t) { return } incrementTaskRetryCount(t) @@ -51,7 +51,7 @@ func FailTask(l log.Logger, t *task.Task, err error) error { return err } -func shouldTaskError(t *task.Task) bool { +func shouldTaskFail(t *task.Task) bool { if t.Data[dexcomTaskRetryField] != nil { count, ok := t.Data[dexcomTaskRetryField].(int) if ok { diff --git a/dexcom/fetch/task_test.go b/dexcom/fetch/task_test.go index b757b665d..8c2d0337c 100644 --- a/dexcom/fetch/task_test.go +++ b/dexcom/fetch/task_test.go @@ -28,15 +28,15 @@ var _ = Describe("Task", func() { } Context("ErrorOrRetryTask", func() { - DescribeTable("will not append error", + DescribeTable("will append error, but not fail", func(setupFunc func() (*task.Task, int)) { tsk, startCount := setupFunc() - Expect(tsk.IsFailed()).To(Equal(true)) + Expect(tsk.IsFailed()).To(BeTrue()) Expect(tsk.Data["retryCount"]).To(Equal(startCount)) fetch.ErrorOrRetryTask(tsk, errors.New("some error")) - Expect(tsk.HasError()).To(Equal(false)) + Expect(tsk.HasError()).To(BeTrue()) + Expect(tsk.IsFailed()).To(BeFalse()) Expect(tsk.Data["retryCount"]).To(Equal(startCount + 1)) - Expect(tsk.IsFailed()).To(Equal(false)) }, Entry("if zero retries", func() (*task.Task, int) { retryCount := 0 @@ -51,13 +51,15 @@ var _ = Describe("Task", func() { return getTask(retryCount, true), retryCount }), ) - DescribeTable("will append error", + DescribeTable("will append error and fail", func(setupFunc func() (*task.Task, int)) { tsk, startCount := setupFunc() + Expect(tsk.IsFailed()).To(BeTrue()) Expect(tsk.Data["retryCount"]).To(Equal(startCount)) fetch.ErrorOrRetryTask(tsk, errors.New("some error")) - Expect(tsk.HasError()).To(Equal(true)) - Expect(tsk.IsFailed()).To(Equal(true)) + Expect(tsk.HasError()).To(BeTrue()) + Expect(tsk.IsFailed()).To(BeTrue()) + Expect(tsk.Data["retryCount"]).To(Equal(startCount)) }, Entry("when 3rd retry", func() (*task.Task, int) { retryCount := 3 @@ -68,12 +70,14 @@ var _ = Describe("Task", func() { return getTask(retryCount, true), retryCount }), ) - DescribeTable("will ignore if the task is not been failed", + DescribeTable("will set the error even if the task is not failed", func(setupFunc func() (*task.Task, int)) { tsk, startCount := setupFunc() + Expect(tsk.IsFailed()).To(BeFalse()) Expect(tsk.Data["retryCount"]).To(Equal(startCount)) fetch.ErrorOrRetryTask(tsk, errors.New("some error")) - Expect(tsk.HasError()).To(Equal(false)) + Expect(tsk.HasError()).To(BeTrue()) + Expect(tsk.IsFailed()).To(BeFalse()) Expect(tsk.Data["retryCount"]).To(Equal(startCount)) }, Entry("when 3rd retry", func() (*task.Task, int) { @@ -94,16 +98,16 @@ var _ = Describe("Task", func() { }) It("will set the task to have failed", func() { tsk := getTask(0, false) - Expect(tsk.IsFailed()).To(Equal(false)) + Expect(tsk.IsFailed()).To(BeFalse()) fetch.FailTask(logger, tsk, errors.New("some error")) - Expect(tsk.IsFailed()).To(Equal(true)) + Expect(tsk.IsFailed()).To(BeTrue()) }) It("will not change the failure status if already set", func() { tsk := getTask(0, false) tsk.SetFailed() - Expect(tsk.IsFailed()).To(Equal(true)) + Expect(tsk.IsFailed()).To(BeTrue()) fetch.FailTask(logger, tsk, errors.New("some error")) - Expect(tsk.IsFailed()).To(Equal(true)) + Expect(tsk.IsFailed()).To(BeTrue()) }) }) From 2a425fc6f4f10e41c88aa28b496739d87b034493 Mon Sep 17 00:00:00 2001 From: Darin Krauss Date: Wed, 7 Feb 2024 13:24:48 -0800 Subject: [PATCH 2/2] Increase Dexcom task duration maximum and available period - Doubled average period between Dexcom task executions from 60 minutes to 120 minutes - Doubled task duration to account for a couple of long running errors observed in logs - Minor changes for local debugging --- .vscode/launch.json | 34 ++++++++++++++-------------------- application/provider.go | 4 ++-- dexcom/fetch/runner.go | 6 +++--- 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index dd1f1a89f..27a9a4886 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -6,11 +6,9 @@ "type": "go", "request": "launch", "mode": "debug", - "host": "127.0.0.1", - "port": 19222, "program": "${workspaceRoot}/services/auth", - "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application/version.Base=0.0.0 -X github.com/tidepool-org/platform/application/version.ShortCommit=00000000 -X github.com/tidepool-org/platform/application/version.FullCommit=0000000000000000000000000000000000000000'", - "envFile": "${workspaceRoot}/env.sh", + "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application.VersionBase=0.0.0 -X github.com/tidepool-org/platform/application.VersionShortCommit=00000000 -X github.com/tidepool-org/platform/application.VersionFullCommit=0000000000000000000000000000000000000000'", + "envFile": "${workspaceRoot}/auth.env", "env": { "TIDEPOOL_DEBUG_NAME": "auth" } @@ -20,11 +18,9 @@ "type": "go", "request": "launch", "mode": "debug", - "host": "127.0.0.1", - "port": 19220, "program": "${workspaceRoot}/services/data", - "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application/version.Base=0.0.0 -X github.com/tidepool-org/platform/application/version.ShortCommit=00000000 -X github.com/tidepool-org/platform/application/version.FullCommit=0000000000000000000000000000000000000000'", - "envFile": "${workspaceRoot}/env.sh", + "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application.VersionBase=0.0.0 -X github.com/tidepool-org/platform/application.VersionShortCommit=00000000 -X github.com/tidepool-org/platform/application.VersionFullCommit=0000000000000000000000000000000000000000'", + "envFile": "${workspaceRoot}/data.env", "env": { "TIDEPOOL_DEBUG_NAME": "data" } @@ -34,11 +30,9 @@ "type": "go", "request": "launch", "mode": "debug", - "host": "127.0.0.1", - "port": 19223, "program": "${workspaceRoot}/services/notification", - "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application/version.Base=0.0.0 -X github.com/tidepool-org/platform/application/version.ShortCommit=00000000 -X github.com/tidepool-org/platform/application/version.FullCommit=0000000000000000000000000000000000000000'", - "envFile": "${workspaceRoot}/env.sh", + "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application.VersionBase=0.0.0 -X github.com/tidepool-org/platform/application.VersionShortCommit=00000000 -X github.com/tidepool-org/platform/application.VersionFullCommit=0000000000000000000000000000000000000000'", + "envFile": "${workspaceRoot}/notification.env", "env": { "TIDEPOOL_DEBUG_NAME": "notification" } @@ -48,11 +42,9 @@ "type": "go", "request": "launch", "mode": "debug", - "host": "127.0.0.1", - "port": 19224, "program": "${workspaceRoot}/services/task", - "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application/version.Base=0.0.0 -X github.com/tidepool-org/platform/application/version.ShortCommit=00000000 -X github.com/tidepool-org/platform/application/version.FullCommit=0000000000000000000000000000000000000000'", - "envFile": "${workspaceRoot}/env.sh", + "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application.VersionBase=0.0.0 -X github.com/tidepool-org/platform/application.VersionShortCommit=00000000 -X github.com/tidepool-org/platform/application.VersionFullCommit=0000000000000000000000000000000000000000'", + "envFile": "${workspaceRoot}/task.env", "env": { "TIDEPOOL_DEBUG_NAME": "task" } @@ -62,17 +54,19 @@ "type": "go", "request": "launch", "mode": "debug", - "host": "127.0.0.1", - "port": 19221, "program": "${workspaceRoot}/services/user", - "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application/version.Base=0.0.0 -X github.com/tidepool-org/platform/application/version.ShortCommit=00000000 -X github.com/tidepool-org/platform/application/version.FullCommit=0000000000000000000000000000000000000000'", - "envFile": "${workspaceRoot}/env.sh", + "buildFlags": "-ldflags '-X github.com/tidepool-org/platform/application.VersionBase=0.0.0 -X github.com/tidepool-org/platform/application.VersionShortCommit=00000000 -X github.com/tidepool-org/platform/application.VersionFullCommit=0000000000000000000000000000000000000000'", + "envFile": "${workspaceRoot}/user.env", "env": { "TIDEPOOL_DEBUG_NAME": "user" } } ], "compounds": [ + { + "name": "auth, task", + "configurations": ["auth", "task"] + }, { "name": "auth, data, task", "configurations": ["auth", "data", "task"] diff --git a/application/provider.go b/application/provider.go index 736d68111..8d742e8e7 100644 --- a/application/provider.go +++ b/application/provider.go @@ -88,8 +88,8 @@ func NewProvider(prefix string, scopes ...string) (*ProviderImpl, error) { } name := filepath.Base(os.Args[0]) - if strings.EqualFold(name, "debug") { - name = configReporter.WithScopes(name).GetWithDefault("name", name) + if strings.EqualFold(name, "debug") || strings.HasPrefix(name, "__debug_bin") { + name = configReporter.WithScopes("debug").GetWithDefault("name", name) } configReporter = configReporter.WithScopes(name).WithScopes(scopes...) diff --git a/dexcom/fetch/runner.go b/dexcom/fetch/runner.go index ceb727098..59360116b 100644 --- a/dexcom/fetch/runner.go +++ b/dexcom/fetch/runner.go @@ -26,10 +26,10 @@ import ( ) const ( - AvailableAfterDurationMaximum = 75 * time.Minute - AvailableAfterDurationMinimum = 45 * time.Minute + AvailableAfterDurationMaximum = 135 * time.Minute + AvailableAfterDurationMinimum = 105 * time.Minute DataSetSize = 2000 - TaskDurationMaximum = 5 * time.Minute + TaskDurationMaximum = 10 * time.Minute ) var initialDataTime = time.Date(2015, 1, 1, 0, 0, 0, 0, time.UTC)