[BACK-2866] Fix Dexcom task failing for newly created Dexcom connections #697
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| *.coverprofile | ||
| _bin/ | ||
| _tmp/ | ||
| coverprofile.out | ||
| debug | ||
| deploy/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| *.coverprofile | ||
| *.env | ||
| *.test | ||
| /_bin | ||
| /_data | ||
|
|
@@ -7,3 +8,5 @@ | |
| /deploy | ||
| /.idea | ||
| .DS_Store | ||
| .envrc | ||
| coverprofile.out | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,46 +5,73 @@ import ( | |
| "net/http" | ||
| "strings" | ||
|
|
||
| . "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" | ||
| 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" | ||
| dataStoreTest "github.com/tidepool-org/platform/data/store/test" | ||
| "github.com/tidepool-org/platform/data/summary" | ||
| "github.com/tidepool-org/platform/data/types/upload" | ||
| 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" | ||
| "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" | ||
| ) | ||
|
|
||
| var _ = Describe("UsersDataSetsCreate", func() { | ||
| 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")) | ||
| } | ||
|
|
||
| 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) | ||
|
|
||
| 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, | ||
| 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)) | ||
| 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-path-params-user-id", | ||
| }, | ||
| } | ||
|
|
||
|
|
@@ -120,7 +163,7 @@ func (c *mockDataServiceContext) AuthClient() auth.Client { | |
| } | ||
|
|
||
| func (c *mockDataServiceContext) MetricClient() metric.Client { | ||
| 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 { | ||
| return c.dataDeduplicatorFactory | ||
| } | ||
|
|
||
| func (c *mockDataServiceContext) DataRepository() dataStore.DataRepository { | ||
| 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 { | ||
| panic("not implemented") // TODO: Implement | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,10 +26,10 @@ import ( | |
| ) | ||
|
|
||
| const ( | ||
| AvailableAfterDurationMaximum = 135 * time.Minute | ||
| AvailableAfterDurationMinimum = 105 * time.Minute | ||
darinkrauss marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| DataSetSize = 2000 | ||
| TaskDurationMaximum = 10 * time.Minute | ||
darinkrauss marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
|
|
||
| var initialDataTime = time.Date(2015, 1, 1, 0, 0, 0, 0, time.UTC) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 shouldTaskFail(t) { | ||
| return | ||
| } | ||
| incrementTaskRetryCount(t) | ||
|
|
@@ -51,7 +51,7 @@ func FailTask(l log.Logger, t *task.Task, err error) error { | |
| return err | ||
| } | ||
|
|
||
| func shouldTaskFail(t *task.Task) bool { | ||
|
There was a problem hiding this comment. I don't think this function handles correctly the case where the There was a problem hiding this comment. Yes, that is correct and I intentionally did not fix that yet. If we make that fix right now, then every Dexcom task that hits a 429 will start down the "retry path" and, after retrying 3 times, will be marked as failed. I really don't want that to happen quite yet. I need to first straighten out what errors cause a formal retry (where the retry counter is incremented; e.g. Dexcom auth errors), what errors retry for "free" (no increment; e.g. Dexcom 429s), and what errors fail immediately (e.g bad data). Furthermore, I need to add code that resets the retry counter on success (as there is no code to do that currently). |
||
| if t.Data[dexcomTaskRetryField] != nil { | ||
| count, ok := t.Data[dexcomTaskRetryField].(int) | ||
| if ok { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI,
details.UserIDwill be set to the id of the backend client service account, which is a real user in Keycloak.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's not what I'm seeing. I'm getting an empty string which causes a failure during later validation (
CreatedUserIDofnilis fine, but an empty string is not).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll double check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the service session token does include the
UserIDof the backend client service account, it is not being used because the session token is marked asIsServer. Find the relevant code that ignores theUserIDfor a server session token atplatform/auth/client/external.go
Line 208 in f797630