From fb945d1e4c2eacb345a41918a7f9f2167dae2a3b Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Thu, 21 Nov 2024 11:41:45 +0000 Subject: [PATCH 01/14] MLPAB-2321 persist LPA UID on scheduled tasks, ignore scheduled task if cannot register --- cmd/mlpa/main.go | 4 +- cmd/schedule-runner/main.go | 20 +- docker/docker-compose.yml | 4 +- docker/localstack/localstack-init.sh | 2 +- .../identity_with_one_login_callback.go | 1 + .../identity_with_one_login_callback_test.go | 1 + internal/scheduled/event.go | 2 + .../scheduled/mock_LpaStoreClient_test.go | 96 +++++++++ internal/scheduled/runner.go | 64 ++++-- internal/scheduled/runner_test.go | 200 ++++++++++++++---- 10 files changed, 319 insertions(+), 75 deletions(-) create mode 100644 internal/scheduled/mock_LpaStoreClient_test.go diff --git a/cmd/mlpa/main.go b/cmd/mlpa/main.go index 4eb887a788..8dacfc1ac6 100644 --- a/cmd/mlpa/main.go +++ b/cmd/mlpa/main.go @@ -263,9 +263,11 @@ func run(ctx context.Context, logger *slog.Logger) error { evidenceS3Client := s3.NewClient(cfg, evidenceBucketName) lambdaClient := lambda.New(cfg, v4.NewSigner(), httpClient, time.Now) - uidClient := uid.New(uidBaseURL, lambdaClient) + lpaStoreClient := lpastore.New(lpaStoreBaseURL, secretsClient, lpaStoreSecretARN, lambdaClient) + uidClient := uid.New(uidBaseURL, lambdaClient) + mux := http.NewServeMux() mux.HandleFunc(page.PathHealthCheckService.String(), func(w http.ResponseWriter, r *http.Request) {}) mux.Handle(page.PathHealthCheckDependency.String(), page.DependencyHealthCheck(map[string]page.HealthChecker{ diff --git a/cmd/schedule-runner/main.go b/cmd/schedule-runner/main.go index 801ea014f6..01a0df7cb8 100644 --- a/cmd/schedule-runner/main.go +++ b/cmd/schedule-runner/main.go @@ -11,12 +11,15 @@ import ( "github.com/aws/aws-lambda-go/lambda" "github.com/aws/aws-sdk-go-v2/aws" + v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/cloudwatch" "github.com/ministryofjustice/opg-modernising-lpa/internal/donor" "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/ministryofjustice/opg-modernising-lpa/internal/event" + lambdaclient "github.com/ministryofjustice/opg-modernising-lpa/internal/lambda" "github.com/ministryofjustice/opg-modernising-lpa/internal/localize" + "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/ministryofjustice/opg-modernising-lpa/internal/scheduled" "github.com/ministryofjustice/opg-modernising-lpa/internal/search" @@ -29,8 +32,12 @@ import ( ) var ( - awsBaseURL = os.Getenv("AWS_BASE_URL") - eventBusName = cmp.Or(os.Getenv("EVENT_BUS_NAME"), "default") + awsBaseURL = os.Getenv("AWS_BASE_URL") + eventBusName = cmp.Or(os.Getenv("EVENT_BUS_NAME"), "default") + lpaStoreBaseURL = cmp.Or(os.Getenv("LPA_STORE_BASE_URL"), "http://mock-lpa-store:8080") + lpaStoreSecretARN = os.Getenv("LPA_STORE_SECRET_ARN") + // TODO remove in MLPAB-2690 + metricsEnabled = os.Getenv("METRICS_ENABLED") == "1" notifyBaseURL = os.Getenv("GOVUK_NOTIFY_BASE_URL") notifyIsProduction = os.Getenv("GOVUK_NOTIFY_IS_PRODUCTION") == "1" searchEndpoint = os.Getenv("SEARCH_ENDPOINT") @@ -38,8 +45,6 @@ var ( searchIndexingEnabled = os.Getenv("SEARCH_INDEXING_DISABLED") != "1" tableName = os.Getenv("LPAS_TABLE") xrayEnabled = os.Getenv("XRAY_ENABLED") == "1" - // TODO remove in MLPAB-2690 - metricsEnabled = os.Getenv("METRICS_ENABLED") == "1" Tag string @@ -89,10 +94,13 @@ func handleRunSchedule(ctx context.Context) error { } client := cloudwatch.NewFromConfig(cfg) - metricsClient := telemetry.NewMetricsClient(client, Tag) - runner := scheduled.NewRunner(logger, scheduledStore, donorStore, notifyClient, metricsClient, metricsEnabled) + lambdaClient := lambdaclient.New(cfg, v4.NewSigner(), httpClient, time.Now) + + lpaStoreClient := lpastore.New(lpaStoreBaseURL, secretsClient, lpaStoreSecretARN, lambdaClient) + + runner := scheduled.NewRunner(logger, scheduledStore, donorStore, notifyClient, metricsClient, metricsEnabled, lpaStoreClient) if err = runner.Run(ctx); err != nil { logger.Error("runner error", slog.Any("err", err)) diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 8519c43d54..75418778df 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -29,7 +29,7 @@ services: - SEARCH_INDEXING_DISABLED=0 - DEV_MODE=1 - SCHEDULED_RUNNER_PERIOD=1m - - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key + - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key::arn event-logger: build: @@ -69,6 +69,8 @@ services: - AWS_REGION=eu-west-1 - AWS_SECRET_ACCESS_KEY=fakeAccessKey - OPENSEARCH_CUSTOM_BACKEND=http://opensearch:9200 + - LPA_STORE_BASE_URL=http://mock-lpa-store:8080 + - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key::arn - LPAS_TABLE=lpas - TAG=0.0.0 - GOVUK_NOTIFY_BASE_URL=http://mock-notify:8080 diff --git a/docker/localstack/localstack-init.sh b/docker/localstack/localstack-init.sh index 24e5f42f4b..112e12ec9b 100644 --- a/docker/localstack/localstack-init.sh +++ b/docker/localstack/localstack-init.sh @@ -85,7 +85,7 @@ awslocal lambda wait function-active-v2 --region eu-west-1 --function-name event echo 'creating schedule-runner lambda' awslocal lambda create-function \ - --environment Variables="{AWS_BASE_URL=$AWS_BASE_URL,TAG=$TAG,LPAS_TABLE=$LPAS_TABLE,GOVUK_NOTIFY_IS_PRODUCTION=$GOVUK_NOTIFY_IS_PRODUCTION,GOVUK_NOTIFY_BASE_URL=$GOVUK_NOTIFY_BASE_URL,SEARCH_ENDPOINT=$SEARCH_ENDPOINT,SEARCH_INDEXING_DISABLED=$SEARCH_INDEXING_DISABLED,SEARCH_INDEX_NAME=$SEARCH_INDEX_NAME,XRAY_ENABLED=$XRAY_ENABLED,METRICS_ENABLED=$METRICS_ENABLED}" \ + --environment Variables="{AWS_BASE_URL=$AWS_BASE_URL,TAG=$TAG,LPAS_TABLE=$LPAS_TABLE,GOVUK_NOTIFY_IS_PRODUCTION=$GOVUK_NOTIFY_IS_PRODUCTION,GOVUK_NOTIFY_BASE_URL=$GOVUK_NOTIFY_BASE_URL,SEARCH_ENDPOINT=$SEARCH_ENDPOINT,SEARCH_INDEXING_DISABLED=$SEARCH_INDEXING_DISABLED,SEARCH_INDEX_NAME=$SEARCH_INDEX_NAME,XRAY_ENABLED=$XRAY_ENABLED,METRICS_ENABLED=$METRICS_ENABLED,LPA_STORE_BASE_URL=$LPA_STORE_BASE_URL,LPA_STORE_SECRET_ARN=$LPA_STORE_SECRET_ARN}" \ --region eu-west-1 \ --function-name schedule-runner \ --handler bootstrap \ diff --git a/internal/donor/donorpage/identity_with_one_login_callback.go b/internal/donor/donorpage/identity_with_one_login_callback.go index 5b9b89c7d9..a898564ffc 100644 --- a/internal/donor/donorpage/identity_with_one_login_callback.go +++ b/internal/donor/donorpage/identity_with_one_login_callback.go @@ -85,6 +85,7 @@ func IdentityWithOneLoginCallback(oneLoginClient OneLoginClient, sessionStore Se Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: provided.PK, TargetLpaOwnerKey: provided.SK, + TargetLpaUID: provided.LpaUID, }); err != nil { return err } diff --git a/internal/donor/donorpage/identity_with_one_login_callback_test.go b/internal/donor/donorpage/identity_with_one_login_callback_test.go index bb7e36a09b..c47a714027 100644 --- a/internal/donor/donorpage/identity_with_one_login_callback_test.go +++ b/internal/donor/donorpage/identity_with_one_login_callback_test.go @@ -127,6 +127,7 @@ func TestGetIdentityWithOneLoginCallbackWhenIdentityMismatched(t *testing.T) { Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: dynamo.LpaKey("hey"), TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("oh")), + TargetLpaUID: "lpa-uid", }). Return(nil) diff --git a/internal/scheduled/event.go b/internal/scheduled/event.go index ffee5e63ec..2074fe5f70 100644 --- a/internal/scheduled/event.go +++ b/internal/scheduled/event.go @@ -20,4 +20,6 @@ type Event struct { TargetLpaKey dynamo.LpaKeyType // TargetLpaOwnerKey is used to specify the target of the action TargetLpaOwnerKey dynamo.LpaOwnerKeyType + // TargetLpaUID is the UID of the action target + TargetLpaUID string } diff --git a/internal/scheduled/mock_LpaStoreClient_test.go b/internal/scheduled/mock_LpaStoreClient_test.go new file mode 100644 index 0000000000..329a221268 --- /dev/null +++ b/internal/scheduled/mock_LpaStoreClient_test.go @@ -0,0 +1,96 @@ +// Code generated by mockery. DO NOT EDIT. + +package scheduled + +import ( + context "context" + + lpadata "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore/lpadata" + mock "github.com/stretchr/testify/mock" +) + +// mockLpaStoreClient is an autogenerated mock type for the LpaStoreClient type +type mockLpaStoreClient struct { + mock.Mock +} + +type mockLpaStoreClient_Expecter struct { + mock *mock.Mock +} + +func (_m *mockLpaStoreClient) EXPECT() *mockLpaStoreClient_Expecter { + return &mockLpaStoreClient_Expecter{mock: &_m.Mock} +} + +// Lpa provides a mock function with given fields: ctx, lpaUID +func (_m *mockLpaStoreClient) Lpa(ctx context.Context, lpaUID string) (*lpadata.Lpa, error) { + ret := _m.Called(ctx, lpaUID) + + if len(ret) == 0 { + panic("no return value specified for Lpa") + } + + var r0 *lpadata.Lpa + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (*lpadata.Lpa, error)); ok { + return rf(ctx, lpaUID) + } + if rf, ok := ret.Get(0).(func(context.Context, string) *lpadata.Lpa); ok { + r0 = rf(ctx, lpaUID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*lpadata.Lpa) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, lpaUID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// mockLpaStoreClient_Lpa_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Lpa' +type mockLpaStoreClient_Lpa_Call struct { + *mock.Call +} + +// Lpa is a helper method to define mock.On call +// - ctx context.Context +// - lpaUID string +func (_e *mockLpaStoreClient_Expecter) Lpa(ctx interface{}, lpaUID interface{}) *mockLpaStoreClient_Lpa_Call { + return &mockLpaStoreClient_Lpa_Call{Call: _e.mock.On("Lpa", ctx, lpaUID)} +} + +func (_c *mockLpaStoreClient_Lpa_Call) Run(run func(ctx context.Context, lpaUID string)) *mockLpaStoreClient_Lpa_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *mockLpaStoreClient_Lpa_Call) Return(_a0 *lpadata.Lpa, _a1 error) *mockLpaStoreClient_Lpa_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *mockLpaStoreClient_Lpa_Call) RunAndReturn(run func(context.Context, string) (*lpadata.Lpa, error)) *mockLpaStoreClient_Lpa_Call { + _c.Call.Return(run) + return _c +} + +// newMockLpaStoreClient creates a new instance of mockLpaStoreClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func newMockLpaStoreClient(t interface { + mock.TestingT + Cleanup(func()) +}) *mockLpaStoreClient { + mock := &mockLpaStoreClient{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/internal/scheduled/runner.go b/internal/scheduled/runner.go index 419a17e46c..3bc12a84a3 100644 --- a/internal/scheduled/runner.go +++ b/internal/scheduled/runner.go @@ -14,6 +14,7 @@ import ( "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/ministryofjustice/opg-modernising-lpa/internal/identity" "github.com/ministryofjustice/opg-modernising-lpa/internal/localize" + "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore/lpadata" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/ministryofjustice/opg-modernising-lpa/internal/task" ) @@ -50,6 +51,10 @@ type MetricsClient interface { PutMetrics(ctx context.Context, input *cloudwatch.PutMetricDataInput) error } +type LpaStoreClient interface { + Lpa(ctx context.Context, lpaUID string) (*lpadata.Lpa, error) +} + type Runner struct { logger Logger store ScheduledStore @@ -65,9 +70,10 @@ type Runner struct { errored float64 // TODO remove in MLPAB-2690 metricsEnabled bool + lpaStoreClient LpaStoreClient } -func NewRunner(logger Logger, store ScheduledStore, donorStore DonorStore, notifyClient NotifyClient, metricsClient MetricsClient, metricsEnabled bool) *Runner { +func NewRunner(logger Logger, store ScheduledStore, donorStore DonorStore, notifyClient NotifyClient, metricsClient MetricsClient, metricsEnabled bool, lpaStoreClient LpaStoreClient) *Runner { r := &Runner{ logger: logger, store: store, @@ -78,6 +84,7 @@ func NewRunner(logger Logger, store ScheduledStore, donorStore DonorStore, notif waiter: &waiter{backoff: time.Second, sleep: time.Sleep, maxRetries: 10}, metricsClient: metricsClient, metricsEnabled: metricsEnabled, + lpaStoreClient: lpaStoreClient, } r.actions = map[Action]ActionFunc{ @@ -87,15 +94,31 @@ func NewRunner(logger Logger, store ScheduledStore, donorStore DonorStore, notif return r } -func (r *Runner) Processed() { +func (r *Runner) Processed(ctx context.Context, row *Event) { + r.logger.InfoContext(ctx, "runner action success", + slog.String("action", row.Action.String()), + slog.String("target_pk", row.TargetLpaKey.PK()), + slog.String("target_sk", row.TargetLpaOwnerKey.SK())) + r.processed++ } -func (r *Runner) Ignored() { +func (r *Runner) Ignored(ctx context.Context, row *Event) { + r.logger.InfoContext(ctx, "runner action ignored", + slog.String("action", row.Action.String()), + slog.String("target_pk", row.TargetLpaKey.PK()), + slog.String("target_sk", row.TargetLpaOwnerKey.SK())) + r.ignored++ } -func (r *Runner) Errored() { +func (r *Runner) Errored(ctx context.Context, row *Event, err error) { + r.logger.ErrorContext(ctx, "runner action error", + slog.String("action", row.Action.String()), + slog.String("target_pk", row.TargetLpaKey.PK()), + slog.String("target_sk", row.TargetLpaOwnerKey.SK()), + slog.Any("err", err)) + r.errored++ } @@ -164,30 +187,25 @@ func (r *Runner) Run(ctx context.Context) error { ) if fn, ok := r.actions[row.Action]; ok { + lpa, err := r.lpaStoreClient.Lpa(ctx, row.TargetLpaUID) + if err != nil { + r.Errored(ctx, row, err) + continue + } + + if lpa.CannotRegister { + r.Ignored(ctx, row) + continue + } + if err := fn(ctx, row); err != nil { if errors.Is(err, errStepIgnored) { - r.logger.InfoContext(ctx, "runner action ignored", - slog.String("action", row.Action.String()), - slog.String("target_pk", row.TargetLpaKey.PK()), - slog.String("target_sk", row.TargetLpaOwnerKey.SK())) - - r.Ignored() + r.Ignored(ctx, row) } else { - r.logger.ErrorContext(ctx, "runner action error", - slog.String("action", row.Action.String()), - slog.String("target_pk", row.TargetLpaKey.PK()), - slog.String("target_sk", row.TargetLpaOwnerKey.SK()), - slog.Any("err", err)) - - r.Errored() + r.Errored(ctx, row, err) } } else { - r.logger.InfoContext(ctx, "runner action success", - slog.String("action", row.Action.String()), - slog.String("target_pk", row.TargetLpaKey.PK()), - slog.String("target_sk", row.TargetLpaOwnerKey.SK())) - - r.Processed() + r.Processed(ctx, row) } } } diff --git a/internal/scheduled/runner_test.go b/internal/scheduled/runner_test.go index 2fcf99adb9..d1843714e1 100644 --- a/internal/scheduled/runner_test.go +++ b/internal/scheduled/runner_test.go @@ -14,6 +14,7 @@ import ( "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/ministryofjustice/opg-modernising-lpa/internal/identity" "github.com/ministryofjustice/opg-modernising-lpa/internal/localize" + "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore/lpadata" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -26,6 +27,12 @@ var ( testNowFn = func() time.Time { return testNow } testSinceDuration = time.Millisecond * 5 testSinceFn = func(t time.Time) time.Duration { return testSinceDuration } + testEvent = &Event{ + Action: 99, + TargetLpaKey: dynamo.LpaKey("an-lpa"), + TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), + TargetLpaUID: "lpa-uid", + } ) func (m *mockScheduledStore) ExpectPops(returns ...any) { @@ -50,8 +57,9 @@ func TestNewRunner(t *testing.T) { donorStore := newMockDonorStore(t) notifyClient := newMockNotifyClient(t) metricsClient := newMockMetricsClient(t) + lpaStoreClient := newMockLpaStoreClient(t) - runner := NewRunner(logger, store, donorStore, notifyClient, metricsClient, true) + runner := NewRunner(logger, store, donorStore, notifyClient, metricsClient, true, lpaStoreClient) assert.Equal(t, logger, runner.logger) assert.Equal(t, store, runner.store) @@ -59,6 +67,7 @@ func TestNewRunner(t *testing.T) { assert.Equal(t, notifyClient, runner.notifyClient) assert.Equal(t, metricsClient, runner.metricsClient) assert.Equal(t, true, runner.metricsEnabled) + assert.Equal(t, lpaStoreClient, runner.lpaStoreClient) } func (m *mockMetricsClient) assertPutMetrics(processed, ignored, errored float64, err error) { @@ -94,12 +103,6 @@ func (m *mockMetricsClient) assertPutMetrics(processed, ignored, errored float64 } func TestRunnerRun(t *testing.T) { - event := &Event{ - Action: 99, - TargetLpaKey: dynamo.LpaKey("an-lpa"), - TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), - } - logger := newMockLogger(t) logger.EXPECT(). InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) @@ -114,7 +117,7 @@ func TestRunnerRun(t *testing.T) { store := newMockScheduledStore(t) store.EXPECT(). Pop(ctx, testNow). - Return(event, nil). + Return(testEvent, nil). Once() store.EXPECT(). Pop(ctx, testNow). @@ -126,12 +129,17 @@ func TestRunnerRun(t *testing.T) { actionFunc := newMockActionFunc(t) actionFunc.EXPECT(). - Execute(ctx, event). + Execute(ctx, testEvent). Return(nil) metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(1, 0, 0, nil) + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, "lpa-uid"). + Return(&lpadata.Lpa{}, nil) + runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -143,6 +151,7 @@ func TestRunnerRun(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, + lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -178,13 +187,60 @@ func TestRunnerRunWhenStepErrors(t *testing.T) { assert.Equal(t, expectedError, err) } -func TestRunnerRunWhenActionIgnored(t *testing.T) { - event := &Event{ - Action: 99, - TargetLpaKey: dynamo.LpaKey("an-lpa"), - TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), +func TestRunnerRunWhenLpaStoreClientErrors(t *testing.T) { + logger := newMockLogger(t) + logger.EXPECT(). + InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) + logger.EXPECT(). + ErrorContext(ctx, "runner action error", + slog.String("action", "Action(99)"), + slog.String("target_pk", "LPA#an-lpa"), + slog.String("target_sk", "DONOR#a-donor"), + slog.Any("err", expectedError)) + logger.EXPECT(). + InfoContext(ctx, "no scheduled tasks to process") + + store := newMockScheduledStore(t) + store.EXPECT(). + Pop(ctx, testNow). + Return(testEvent, nil). + Once() + store.EXPECT(). + Pop(ctx, testNow). + Return(nil, dynamo.NotFoundError{}). + Once() + + waiter := newMockWaiter(t) + waiter.EXPECT().Reset() + + metricsClient := newMockMetricsClient(t) + metricsClient.assertPutMetrics(0, 0, 1, nil) + + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, "lpa-uid"). + Return(&lpadata.Lpa{}, expectedError) + + runner := &Runner{ + now: testNowFn, + since: testSinceFn, + logger: logger, + store: store, + waiter: waiter, + actions: map[Action]ActionFunc{ + 99: nil, + }, + metricsClient: metricsClient, + metricsEnabled: true, + lpaStoreClient: lpaStoreClient, } + err := runner.Run(ctx) + + assert.Nil(t, err) +} + +func TestRunnerRunWhenLpaStatusIsCannotRegister(t *testing.T) { logger := newMockLogger(t) logger.EXPECT(). InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) @@ -199,7 +255,59 @@ func TestRunnerRunWhenActionIgnored(t *testing.T) { store := newMockScheduledStore(t) store.EXPECT(). Pop(ctx, testNow). - Return(event, nil). + Return(testEvent, nil). + Once() + store.EXPECT(). + Pop(ctx, testNow). + Return(nil, dynamo.NotFoundError{}). + Once() + + waiter := newMockWaiter(t) + waiter.EXPECT().Reset() + + metricsClient := newMockMetricsClient(t) + metricsClient.assertPutMetrics(0, 1, 0, nil) + + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, "lpa-uid"). + Return(&lpadata.Lpa{CannotRegister: true}, nil) + + runner := &Runner{ + now: testNowFn, + since: testSinceFn, + logger: logger, + store: store, + waiter: waiter, + actions: map[Action]ActionFunc{ + 99: nil, + }, + metricsClient: metricsClient, + metricsEnabled: true, + lpaStoreClient: lpaStoreClient, + } + + err := runner.Run(ctx) + + assert.Nil(t, err) +} + +func TestRunnerRunWhenActionIgnored(t *testing.T) { + logger := newMockLogger(t) + logger.EXPECT(). + InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) + logger.EXPECT(). + InfoContext(ctx, "runner action ignored", + slog.String("action", "Action(99)"), + slog.String("target_pk", "LPA#an-lpa"), + slog.String("target_sk", "DONOR#a-donor")) + logger.EXPECT(). + InfoContext(ctx, "no scheduled tasks to process") + + store := newMockScheduledStore(t) + store.EXPECT(). + Pop(ctx, testNow). + Return(testEvent, nil). Once() store.EXPECT(). Pop(ctx, testNow). @@ -217,6 +325,11 @@ func TestRunnerRunWhenActionIgnored(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(0, 1, 0, nil) + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, "lpa-uid"). + Return(&lpadata.Lpa{}, nil) + runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -228,6 +341,7 @@ func TestRunnerRunWhenActionIgnored(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, + lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -235,12 +349,6 @@ func TestRunnerRunWhenActionIgnored(t *testing.T) { } func TestRunnerRunWhenActionErrors(t *testing.T) { - event := &Event{ - Action: 99, - TargetLpaKey: dynamo.LpaKey("an-lpa"), - TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), - } - logger := newMockLogger(t) logger.EXPECT(). InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) @@ -256,7 +364,7 @@ func TestRunnerRunWhenActionErrors(t *testing.T) { store := newMockScheduledStore(t) store.EXPECT(). Pop(ctx, testNow). - Return(event, nil). + Return(testEvent, nil). Once() store.EXPECT(). Pop(ctx, testNow). @@ -274,6 +382,11 @@ func TestRunnerRunWhenActionErrors(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(0, 0, 1, nil) + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, "lpa-uid"). + Return(&lpadata.Lpa{}, nil) + runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -285,6 +398,7 @@ func TestRunnerRunWhenActionErrors(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, + lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -292,12 +406,6 @@ func TestRunnerRunWhenActionErrors(t *testing.T) { } func TestRunnerRunWhenWaitingError(t *testing.T) { - event := &Event{ - Action: 99, - TargetLpaKey: dynamo.LpaKey("an-lpa"), - TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), - } - logger := newMockLogger(t) logger.EXPECT(). InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) @@ -314,7 +422,7 @@ func TestRunnerRunWhenWaitingError(t *testing.T) { store := newMockScheduledStore(t) store.ExpectPops( nil, expectedError, - event, nil, + testEvent, nil, nil, dynamo.NotFoundError{}) waiter := newMockWaiter(t) @@ -329,6 +437,11 @@ func TestRunnerRunWhenWaitingError(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(1, 0, 0, nil) + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, "lpa-uid"). + Return(&lpadata.Lpa{}, nil) + runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -340,6 +453,7 @@ func TestRunnerRunWhenWaitingError(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, + lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -348,12 +462,6 @@ func TestRunnerRunWhenWaitingError(t *testing.T) { } func TestRunnerRunWhenMetricsDisabled(t *testing.T) { - event := &Event{ - Action: 99, - TargetLpaKey: dynamo.LpaKey("an-lpa"), - TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), - } - logger := newMockLogger(t) logger.EXPECT(). InfoContext(ctx, mock.Anything, mock.Anything) @@ -365,7 +473,7 @@ func TestRunnerRunWhenMetricsDisabled(t *testing.T) { store := newMockScheduledStore(t) store.EXPECT(). Pop(ctx, mock.Anything). - Return(event, nil). + Return(testEvent, nil). Once() store.EXPECT(). Pop(ctx, mock.Anything). @@ -380,6 +488,11 @@ func TestRunnerRunWhenMetricsDisabled(t *testing.T) { Execute(ctx, mock.Anything). Return(nil) + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, mock.Anything). + Return(&lpadata.Lpa{}, nil) + runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -391,6 +504,7 @@ func TestRunnerRunWhenMetricsDisabled(t *testing.T) { }, metricsClient: nil, metricsEnabled: false, + lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -399,12 +513,6 @@ func TestRunnerRunWhenMetricsDisabled(t *testing.T) { } func TestRunnerRunWhenMetricsClientError(t *testing.T) { - event := &Event{ - Action: 99, - TargetLpaKey: dynamo.LpaKey("an-lpa"), - TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), - } - logger := newMockLogger(t) logger.EXPECT(). InfoContext(ctx, mock.Anything, mock.Anything) @@ -418,7 +526,7 @@ func TestRunnerRunWhenMetricsClientError(t *testing.T) { store := newMockScheduledStore(t) store.EXPECT(). Pop(ctx, mock.Anything). - Return(event, nil). + Return(testEvent, nil). Once() store.EXPECT(). Pop(ctx, mock.Anything). @@ -436,6 +544,11 @@ func TestRunnerRunWhenMetricsClientError(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(1, 0, 0, expectedError) + lpaStoreClient := newMockLpaStoreClient(t) + lpaStoreClient.EXPECT(). + Lpa(ctx, "lpa-uid"). + Return(&lpadata.Lpa{}, nil) + runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -447,6 +560,7 @@ func TestRunnerRunWhenMetricsClientError(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, + lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) From 1a05010baca788055e43bbdf8f1553739175dd2a Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Thu, 21 Nov 2024 12:22:11 +0000 Subject: [PATCH 02/14] MLPAB-2321 revert secret name --- docker/docker-compose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 75418778df..e85f6a0c8c 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -29,7 +29,7 @@ services: - SEARCH_INDEXING_DISABLED=0 - DEV_MODE=1 - SCHEDULED_RUNNER_PERIOD=1m - - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key::arn + - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key event-logger: build: @@ -70,7 +70,7 @@ services: - AWS_SECRET_ACCESS_KEY=fakeAccessKey - OPENSEARCH_CUSTOM_BACKEND=http://opensearch:9200 - LPA_STORE_BASE_URL=http://mock-lpa-store:8080 - - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key::arn + - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key - LPAS_TABLE=lpas - TAG=0.0.0 - GOVUK_NOTIFY_BASE_URL=http://mock-notify:8080 From 70b1e004f1164395891c5cc293f4c81ea170e460 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Mon, 25 Nov 2024 21:00:05 +0000 Subject: [PATCH 03/14] MLPAB-2321 delete all scheduled event when LPA cannot be registered --- Makefile | 49 +++-- cmd/event-received/factory.go | 10 + cmd/event-received/lpastore_event_handler.go | 40 ++-- cmd/event-received/main.go | 13 +- cmd/scheduled-task-adder/main.go | 10 +- internal/app/app.go | 2 + .../identity_with_one_login_callback.go | 2 +- .../identity_with_one_login_callback_test.go | 2 +- internal/dynamo/client.go | 52 ++++- internal/dynamo/client_test.go | 207 +++++++++++------- internal/scheduled/event.go | 4 +- internal/scheduled/runner.go | 2 +- internal/scheduled/runner_test.go | 2 +- internal/scheduled/store.go | 23 +- 14 files changed, 293 insertions(+), 125 deletions(-) diff --git a/Makefile b/Makefile index b05b64bcad..80f49c2299 100644 --- a/Makefile +++ b/Makefile @@ -127,32 +127,59 @@ delete-all-items: ##@dynamodb deletes and recreates lpas dynamodb table --global-secondary-indexes file://dynamodb-lpa-gsi-schema.json emit-evidence-received: ##@events emits an evidence-received event with the given LpaUID e.g. emit-evidence-received uid=abc-123 + $(eval BODY := $(shell echo '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"evidence-received","source":"opg.poas.sirius","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"UID":"$(uid)"}}' | sed 's/"/\\"/g')) + docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ --endpoint-url=http://localhost:4566 \ --region eu-west-1 \ --function-name event-received text \ - --payload '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"evidence-received","source":"opg.poas.sirius","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"UID":"$(uid)"}}' + --payload '{"Records": [{"messageId": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78", "body": "$(BODY)"}]}' emit-reduced-fee-approved: ##@events emits a reduced-fee-approved event with the given LpaUID e.g. emit-reduced-fee-approved uid=abc-123 - docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ - --endpoint-url=http://localhost:4566 \ - --region eu-west-1 \ - --function-name event-received text \ - --payload '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"reduced-fee-approved","source":"opg.poas.sirius","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"UID":"$(uid)"}}' + $(eval BODY := $(shell echo '{"version":"0","id":"abcdef01-2345-6789-abcd-ef0123456789","detail-type":"reduced-fee-approved","source":"opg.poas.sirius","account":"653761790766","time":"2024-01-01T12:00:00Z","region":"eu-west-1","resources":[],"detail":{"uid":"$(uid)"}}' | sed 's/"/\\"/g')) + + docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ + --endpoint-url=http://localhost:4566 \ + --region eu-west-1 \ + --function-name event-received text \ + --payload '{"Records": [{"messageId": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78", "body": "$(BODY)"}]}' emit-reduced-fee-declined: ##@events emits a reduced-fee-declined event with the given LpaUID e.g. emit-reduced-fee-declined uid=abc-123 + $(eval BODY := $(shell echo '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"reduced-fee-declined","source":"opg.poas.sirius","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"UID":"$(uid)"}}' | sed 's/"/\\"/g')) + docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ --endpoint-url=http://localhost:4566 \ --region eu-west-1 \ --function-name event-received text \ - --payload '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"reduced-fee-declined","source":"opg.poas.sirius","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"UID":"$(uid)"}}' + --payload '{"Records": [{"messageId": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78", "body": "$(BODY)"}]}' + emit-more-evidence-required: ##@events emits a more-evidence-required event with the given LpaUID e.g. emit-more-evidence-required uid=abc-123 + $(eval BODY := $(shell echo '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"more-evidence-required","source":"opg.poas.sirius","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"UID":"$(uid)"}}' | sed 's/"/\\"/g')) + docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ --endpoint-url=http://localhost:4566 \ --region eu-west-1 \ --function-name event-received text \ - --payload '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"more-evidence-required","source":"opg.poas.sirius","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"UID":"$(uid)"}}' + --payload '{"Records": [{"messageId": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78", "body": "$(BODY)"}]}' + +emit-uid-requested: ##@events emits a uid-requested event with the given detail e.g. emit-uid-requested lpaId=abc sessionId=xyz + $(eval BODY := $(shell echo '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"uid-requested","source":"opg.poas.makeregister","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"LpaID":"$(lpaId)","DonorSessionID":"$(sessionId)","Type":"property-and-affairs","Donor":{"Name":"abc","Dob":"2000-01-01","Postcode":"F1 1FF"}}}' | sed 's/"/\\"/g')) + + docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ + --endpoint-url=http://localhost:4566 \ + --region eu-west-1 \ + --function-name event-received text \ + --payload '{"Records": [{"messageId": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78", "body": "$(BODY)"}]}' + +emit-lpa-updated-event: ##@events emits an lpa-updated event with the given change type e.g. emit-uid-requested uid=abc-123 changeType=CANNOT_REGISTER + $(eval BODY := $(shell echo '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"lpa-updated","source":"opg.poas.lpastore","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"uid":"$(uid)","changeType":"$(changeType)"}}' | sed 's/"/\\"/g')) + + docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ + --endpoint-url=http://localhost:4566 \ + --region eu-west-1 \ + --function-name event-received text \ + --payload '{"Records": [{"messageId": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78", "body": "$(BODY)"}]}' emit-object-tags-added-with-virus: ##@events emits a ObjectTagging:Put event with the given S3 key e.g. emit-object-tags-added-with-virus key=doc/key. Also ensures a tag with virus-scan-status exists on an existing object set to infected docker compose -f docker/docker-compose.yml exec localstack awslocal s3api \ @@ -174,12 +201,6 @@ emit-object-tags-added-without-virus: ##@events emits a ObjectTagging:Put event --function-name event-received text \ --payload '{"Records":[{"eventSource":"aws:s3","eventTime":"2023-10-23T15:58:33.081Z","eventName":"ObjectTagging:Put","s3":{"bucket":{"name":"uploads-opg-modernising-lpa-eu-west-1"},"object":{"key":"$(key)"}}}]}' -emit-uid-requested: ##@events emits a uid-requested event with the given detail e.g. emit-uid-requested lpaId=abc sessionId=xyz - docker compose -f docker/docker-compose.yml exec localstack awslocal lambda invoke \ - --endpoint-url=http://localhost:4566 \ - --region eu-west-1 \ - --function-name event-received text \ - --payload '{"version":"0","id":"63eb7e5f-1f10-4744-bba9-e16d327c3b98","detail-type":"uid-requested","source":"opg.poas.makeregister","account":"653761790766","time":"2023-08-30T13:40:30Z","region":"eu-west-1","resources":[],"detail":{"LpaID":"$(lpaId)","DonorSessionID":"$(sessionId)","Type":"property-and-affairs","Donor":{"Name":"abc","Dob":"2000-01-01","Postcode":"F1 1FF"}}}' set-uploads-clean: ##@events calls emit-object-tags-added-without-virus for all documents on a given lpa e.g. set-uploads-clean lpaId=abc for k in $$(docker compose -f docker/docker-compose.yml exec localstack awslocal dynamodb --region eu-west-1 query --table-name lpas --key-condition-expression 'PK = :pk and begins_with(SK, :sk)' --expression-attribute-values '{":pk": {"S": "LPA#$(lpaId)"}, ":sk": {"S": "DOCUMENT#"}}' | jq -c -r '.Items[] | .Key[]'); do \ diff --git a/cmd/event-received/factory.go b/cmd/event-received/factory.go index e839f17fd9..800edd3ea3 100644 --- a/cmd/event-received/factory.go +++ b/cmd/event-received/factory.go @@ -19,6 +19,7 @@ import ( "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore/lpadata" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/ministryofjustice/opg-modernising-lpa/internal/random" + "github.com/ministryofjustice/opg-modernising-lpa/internal/scheduled" "github.com/ministryofjustice/opg-modernising-lpa/internal/search" "github.com/ministryofjustice/opg-modernising-lpa/internal/secrets" "github.com/ministryofjustice/opg-modernising-lpa/internal/sharecode" @@ -80,6 +81,7 @@ type Factory struct { lpaStoreClient LpaStoreClient uidStore UidStore uidClient UidClient + scheduledStore ScheduledStore } func (f *Factory) Now() func() time.Time { @@ -211,3 +213,11 @@ func (f *Factory) EventClient() EventClient { return f.eventClient } + +func (f *Factory) ScheduledStore() ScheduledStore { + if f.scheduledStore == nil { + f.scheduledStore = scheduled.NewStore(f.dynamoClient) + } + + return f.scheduledStore +} diff --git a/cmd/event-received/lpastore_event_handler.go b/cmd/event-received/lpastore_event_handler.go index e01ee79979..cc81ac1d75 100644 --- a/cmd/event-received/lpastore_event_handler.go +++ b/cmd/event-received/lpastore_event_handler.go @@ -11,32 +11,32 @@ import ( type lpastoreEventHandler struct{} -func (h *lpastoreEventHandler) Handle(ctx context.Context, factory factory, cloudWatchEvent *events.CloudWatchEvent) error { - switch cloudWatchEvent.DetailType { - case "lpa-updated": - return handleLpaUpdated(ctx, factory.DynamoClient(), cloudWatchEvent, factory.Now()) - - default: - return fmt.Errorf("unknown lpastore event") - } -} - type lpaUpdatedEvent struct { UID string `json:"uid"` ChangeType string `json:"changeType"` } -func handleLpaUpdated(ctx context.Context, client dynamodbClient, event *events.CloudWatchEvent, now func() time.Time) error { - var v lpaUpdatedEvent - if err := json.Unmarshal(event.Detail, &v); err != nil { - return fmt.Errorf("failed to unmarshal detail: %w", err) +func (h *lpastoreEventHandler) Handle(ctx context.Context, factory factory, cloudWatchEvent *events.CloudWatchEvent) error { + if cloudWatchEvent.DetailType == "lpa-updated" { + var v lpaUpdatedEvent + if err := json.Unmarshal(cloudWatchEvent.Detail, &v); err != nil { + return fmt.Errorf("failed to unmarshal detail: %w", err) + } + + switch v.ChangeType { + case "STATUTORY_WAITING_PERIOD": + return handleStatutoryWaitingPeriod(ctx, factory.DynamoClient(), factory.Now(), v) + + case "CANNOT_REGISTER": + return handleCannotRegister(ctx, factory.ScheduledStore(), v) + } } - if v.ChangeType != "STATUTORY_WAITING_PERIOD" { - return nil - } + return fmt.Errorf("unknown lpastore event") +} - donor, err := getDonorByLpaUID(ctx, client, v.UID) +func handleStatutoryWaitingPeriod(ctx context.Context, client dynamodbClient, now func() time.Time, event lpaUpdatedEvent) error { + donor, err := getDonorByLpaUID(ctx, client, event.UID) if err != nil { return err } @@ -49,3 +49,7 @@ func handleLpaUpdated(ctx context.Context, client dynamodbClient, event *events. return nil } + +func handleCannotRegister(ctx context.Context, store ScheduledStore, event lpaUpdatedEvent) error { + return store.DeleteAllByUID(ctx, event.UID) +} diff --git a/cmd/event-received/main.go b/cmd/event-received/main.go index ba29929ce8..f8961e8243 100644 --- a/cmd/event-received/main.go +++ b/cmd/event-received/main.go @@ -65,6 +65,7 @@ type factory interface { UidStore() (UidStore, error) UidClient() UidClient EventClient() EventClient + ScheduledStore() ScheduledStore } type Handler interface { @@ -76,13 +77,17 @@ type uidEvent struct { } type dynamodbClient interface { + AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error + AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error + CreateOnly(ctx context.Context, v interface{}) error + DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error + DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error + Move(ctx context.Context, oldKeys dynamo.Keys, value any) error One(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) error OneByUID(ctx context.Context, uid string, v interface{}) error OneByPK(ctx context.Context, pk dynamo.PK, v interface{}) error OneBySK(ctx context.Context, sk dynamo.SK, v interface{}) error Put(ctx context.Context, v interface{}) error - DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error - CreateOnly(ctx context.Context, v interface{}) error WriteTransaction(ctx context.Context, transaction *dynamo.Transaction) error } @@ -99,6 +104,10 @@ type EventClient interface { SendCertificateProviderStarted(ctx context.Context, event event.CertificateProviderStarted) error } +type ScheduledStore interface { + DeleteAllByUID(ctx context.Context, uid string) error +} + type Event struct { S3Event *events.S3Event SQSEvent *events.SQSEvent diff --git a/cmd/scheduled-task-adder/main.go b/cmd/scheduled-task-adder/main.go index 27615992c6..9c4e7bda98 100644 --- a/cmd/scheduled-task-adder/main.go +++ b/cmd/scheduled-task-adder/main.go @@ -73,19 +73,25 @@ func handleAddScheduledTasks(ctx context.Context, taskCountEvent TaskCountEvent) for i := 0; i < taskCount; i++ { now = now.Add(time.Second * 1) + lpaUID := uuid.NewString() + lpaID := uuid.NewString() + donor := &donordata.Provided{ - LpaUID: uuid.NewString(), - PK: dynamo.LpaKey(uuid.NewString()), + LpaUID: lpaUID, + PK: dynamo.LpaKey(lpaID), SK: dynamo.LpaOwnerKey(dynamo.DonorKey(uuid.NewString())), IdentityUserData: identity.UserData{Status: identity.StatusConfirmed}, Donor: donordata.Donor{Email: "a@b.com"}, + LpaID: lpaID, } event := scheduled.Event{ + CreatedAt: now, At: now, Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: donor.PK, TargetLpaOwnerKey: donor.SK, + LpaUID: lpaUID, PK: dynamo.ScheduledDayKey(now), SK: dynamo.ScheduledKey(now, int(scheduled.ActionExpireDonorIdentity)), } diff --git a/internal/app/app.go b/internal/app/app.go index c34b903c53..ecb7426a32 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -52,6 +52,7 @@ type DynamoClient interface { AllByKeys(ctx context.Context, keys []dynamo.Keys) ([]map[string]dynamodbtypes.AttributeValue, error) AllByPartialSK(ctx context.Context, pk dynamo.PK, partialSK dynamo.SK, v interface{}) error AllBySK(ctx context.Context, sk dynamo.SK, v interface{}) error + AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error AllKeysByPK(ctx context.Context, pk dynamo.PK) ([]dynamo.Keys, error) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error BatchPut(ctx context.Context, items []interface{}) error @@ -59,6 +60,7 @@ type DynamoClient interface { CreateOnly(ctx context.Context, v interface{}) error DeleteKeys(ctx context.Context, keys []dynamo.Keys) error DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error + DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error LatestForActor(ctx context.Context, sk dynamo.SK, v interface{}) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error One(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) error diff --git a/internal/donor/donorpage/identity_with_one_login_callback.go b/internal/donor/donorpage/identity_with_one_login_callback.go index a898564ffc..44caa71187 100644 --- a/internal/donor/donorpage/identity_with_one_login_callback.go +++ b/internal/donor/donorpage/identity_with_one_login_callback.go @@ -85,7 +85,7 @@ func IdentityWithOneLoginCallback(oneLoginClient OneLoginClient, sessionStore Se Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: provided.PK, TargetLpaOwnerKey: provided.SK, - TargetLpaUID: provided.LpaUID, + LpaUID: provided.LpaUID, }); err != nil { return err } diff --git a/internal/donor/donorpage/identity_with_one_login_callback_test.go b/internal/donor/donorpage/identity_with_one_login_callback_test.go index c47a714027..e28bfe24ab 100644 --- a/internal/donor/donorpage/identity_with_one_login_callback_test.go +++ b/internal/donor/donorpage/identity_with_one_login_callback_test.go @@ -127,7 +127,7 @@ func TestGetIdentityWithOneLoginCallbackWhenIdentityMismatched(t *testing.T) { Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: dynamo.LpaKey("hey"), TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("oh")), - TargetLpaUID: "lpa-uid", + LpaUID: "lpa-uid", }). Return(nil) diff --git a/internal/dynamo/client.go b/internal/dynamo/client.go index 5e5c4a49f4..85500aa7be 100644 --- a/internal/dynamo/client.go +++ b/internal/dynamo/client.go @@ -24,7 +24,6 @@ type dynamoDB interface { TransactWriteItems(context.Context, *dynamodb.TransactWriteItemsInput, ...func(*dynamodb.Options)) (*dynamodb.TransactWriteItemsOutput, error) DeleteItem(context.Context, *dynamodb.DeleteItemInput, ...func(*dynamodb.Options)) (*dynamodb.DeleteItemOutput, error) UpdateItem(context.Context, *dynamodb.UpdateItemInput, ...func(*dynamodb.Options)) (*dynamodb.UpdateItemOutput, error) - BatchWriteItem(ctx context.Context, params *dynamodb.BatchWriteItemInput, optFns ...func(*dynamodb.Options)) (*dynamodb.BatchWriteItemOutput, error) } type Client struct { @@ -87,6 +86,31 @@ func (c *Client) OneByUID(ctx context.Context, uid string, v interface{}) error return attributevalue.UnmarshalMap(response.Items[0], v) } +func (c *Client) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { + response, err := c.svc.Query(ctx, &dynamodb.QueryInput{ + TableName: aws.String(c.table), + IndexName: aws.String(lpaUIDIndex), + ExpressionAttributeNames: map[string]string{ + "#LpaUID": "LpaUID", + "#SK": "SK", + }, + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":LpaUID": &types.AttributeValueMemberS{Value: uid}, + ":SK": &types.AttributeValueMemberS{Value: scheduledPrefix}, + }, + KeyConditionExpression: aws.String("#LpaUID = :LpaUID"), + FilterExpression: aws.String("begins_with(#SK, :SK)"), + }) + if err != nil { + return fmt.Errorf("failed to query scheduled event by UID: %w", err) + } + if len(response.Items) == 0 { + return NotFoundError{} + } + + return attributevalue.UnmarshalListOfMaps(response.Items, v) +} + func (c *Client) AllBySK(ctx context.Context, sk SK, v interface{}) error { response, err := c.svc.Query(ctx, &dynamodb.QueryInput{ TableName: aws.String(c.table), @@ -375,6 +399,32 @@ func (c *Client) DeleteOne(ctx context.Context, pk PK, sk SK) error { return err } +func (c *Client) DeleteManyByUID(ctx context.Context, keys []Keys, uid string) error { + items := make([]types.TransactWriteItem, len(keys)) + + for i, key := range keys { + items[i] = types.TransactWriteItem{ + Delete: &types.Delete{ + TableName: aws.String(c.table), + Key: map[string]types.AttributeValue{ + "PK": &types.AttributeValueMemberS{Value: key.PK.PK()}, + "SK": &types.AttributeValueMemberS{Value: key.SK.SK()}, + }, + ConditionExpression: aws.String("LpaUID = :uid"), + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":uid": &types.AttributeValueMemberS{Value: uid}, + }, + }, + } + } + + _, err := c.svc.TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{ + TransactItems: items, + }) + + return err +} + func (c *Client) Update(ctx context.Context, pk PK, sk SK, values map[string]types.AttributeValue, expression string) error { _, err := c.svc.UpdateItem(ctx, &dynamodb.UpdateItemInput{ TableName: aws.String(c.table), diff --git a/internal/dynamo/client_test.go b/internal/dynamo/client_test.go index b47cce7f82..92984cb001 100644 --- a/internal/dynamo/client_test.go +++ b/internal/dynamo/client_test.go @@ -29,8 +29,6 @@ var ( ) func TestOne(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} pkey, _ := attributevalue.Marshal("a-pk") skey, _ := attributevalue.Marshal("a-sk") @@ -53,7 +51,6 @@ func TestOne(t *testing.T) { } func TestOneWhenError(t *testing.T) { - ctx := context.Background() pkey, _ := attributevalue.Marshal("a-pk") skey, _ := attributevalue.Marshal("a-sk") @@ -74,7 +71,6 @@ func TestOneWhenError(t *testing.T) { } func TestOneWhenNotFound(t *testing.T) { - ctx := context.Background() pkey, _ := attributevalue.Marshal("a-pk") skey, _ := attributevalue.Marshal("a-sk") @@ -95,8 +91,6 @@ func TestOneWhenNotFound(t *testing.T) { } func TestOneByUID(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, &dynamodb.QueryInput{ @@ -124,8 +118,6 @@ func TestOneByUID(t *testing.T) { } func TestOneByUIDWhenQueryError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -139,8 +131,6 @@ func TestOneByUIDWhenQueryError(t *testing.T) { } func TestOneByUIDWhenNoItems(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -153,8 +143,6 @@ func TestOneByUIDWhenNoItems(t *testing.T) { } func TestOneByUIDWhenUnmarshalError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -175,8 +163,6 @@ func TestOneByUIDWhenUnmarshalError(t *testing.T) { } func TestOneByPK(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} pkey, _ := attributevalue.Marshal("a-pk") data, _ := attributevalue.MarshalMap(expected) @@ -201,8 +187,6 @@ func TestOneByPK(t *testing.T) { } func TestOneByPKOnQueryError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -216,8 +200,6 @@ func TestOneByPKOnQueryError(t *testing.T) { } func TestOneByPKWhenNotFound(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -231,8 +213,6 @@ func TestOneByPKWhenNotFound(t *testing.T) { } func TestOneByPartialSK(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} pkey, _ := attributevalue.Marshal("a-pk") skey, _ := attributevalue.Marshal("a-partial-sk") @@ -258,8 +238,6 @@ func TestOneByPartialSK(t *testing.T) { } func TestOneByPartialSKOnQueryError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -273,8 +251,6 @@ func TestOneByPartialSKOnQueryError(t *testing.T) { } func TestOneByPartialSKWhenNotFound(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -288,8 +264,6 @@ func TestOneByPartialSKWhenNotFound(t *testing.T) { } func TestAllByPartialSK(t *testing.T) { - ctx := context.Background() - expected := []map[string]string{{"Col": "Val"}, {"Other": "Thing"}} pkey, _ := attributevalue.Marshal("a-pk") skey, _ := attributevalue.Marshal("a-partial-sk") @@ -315,8 +289,6 @@ func TestAllByPartialSK(t *testing.T) { } func TestAllByPartialSKOnQueryError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -330,8 +302,6 @@ func TestAllByPartialSKOnQueryError(t *testing.T) { } func TestAllForActor(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} skey, _ := attributevalue.Marshal("a-partial-sk") data, _ := attributevalue.MarshalMap(expected) @@ -356,8 +326,6 @@ func TestAllForActor(t *testing.T) { } func TestAllForActorWhenNotFound(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -372,8 +340,6 @@ func TestAllForActorWhenNotFound(t *testing.T) { } func TestAllForActorOnQueryError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -387,8 +353,6 @@ func TestAllForActorOnQueryError(t *testing.T) { } func TestLatestForActor(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} skey, _ := attributevalue.Marshal("a-partial-sk") updated, _ := attributevalue.Marshal("2") @@ -416,8 +380,6 @@ func TestLatestForActor(t *testing.T) { } func TestLatestForActorWhenNotFound(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -447,8 +409,6 @@ func TestLatestForActorOnQueryError(t *testing.T) { } func TestAllKeysByPK(t *testing.T) { - ctx := context.Background() - keys := []Keys{ {PK: LpaKey("pk"), SK: OrganisationKey("sk1")}, {PK: LpaKey("pk"), SK: DonorKey("sk2")}, @@ -478,8 +438,6 @@ func TestAllKeysByPK(t *testing.T) { } func TestAllKeysByPKWhenError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -492,8 +450,6 @@ func TestAllKeysByPKWhenError(t *testing.T) { } func TestAllByKeys(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} data, _ := attributevalue.MarshalMap(expected) @@ -523,8 +479,6 @@ func TestAllByKeys(t *testing.T) { } func TestAllByKeysWhenQueryErrors(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). BatchGetItem(ctx, mock.Anything). @@ -544,7 +498,6 @@ func TestPut(t *testing.T) { for name, dataMap := range testCases { t.Run(name, func(t *testing.T) { - ctx := context.Background() data, _ := attributevalue.MarshalMap(dataMap) dynamoDB := newMockDynamoDB(t) @@ -564,7 +517,6 @@ func TestPut(t *testing.T) { } func TestPutWhenStructHasVersion(t *testing.T) { - ctx := context.Background() data, _ := attributevalue.MarshalMap(map[string]any{"Col": "Val", "Version": 2}) dynamoDB := newMockDynamoDB(t) @@ -584,7 +536,6 @@ func TestPutWhenStructHasVersion(t *testing.T) { } func TestPutWhenConditionalCheckFailedException(t *testing.T) { - ctx := context.Background() data, _ := attributevalue.MarshalMap(map[string]any{"Col": "Val", "Version": 2}) dynamoDB := newMockDynamoDB(t) @@ -604,8 +555,6 @@ func TestPutWhenConditionalCheckFailedException(t *testing.T) { } func TestPutWhenError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). PutItem(ctx, mock.Anything). @@ -618,8 +567,6 @@ func TestPutWhenError(t *testing.T) { } func TestPutWhenUnmarshalError(t *testing.T) { - ctx := context.Background() - c := &Client{table: "this", svc: newMockDynamoDB(t)} err := c.Put(ctx, map[string]string{"Col": "Val", "Version": "not an int"}) @@ -627,7 +574,6 @@ func TestPutWhenUnmarshalError(t *testing.T) { } func TestCreate(t *testing.T) { - ctx := context.Background() data, _ := attributevalue.MarshalMap(map[string]string{"Col": "Val"}) dynamoDB := newMockDynamoDB(t) @@ -646,8 +592,6 @@ func TestCreate(t *testing.T) { } func TestCreateWhenError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). PutItem(ctx, mock.Anything). @@ -660,7 +604,6 @@ func TestCreateWhenError(t *testing.T) { } func TestCreateOnly(t *testing.T) { - ctx := context.Background() data, _ := attributevalue.MarshalMap(map[string]string{"Col": "Val"}) dynamoDB := newMockDynamoDB(t) @@ -679,8 +622,6 @@ func TestCreateOnly(t *testing.T) { } func TestCreateOnlyWhenError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). PutItem(ctx, mock.Anything). @@ -693,8 +634,6 @@ func TestCreateOnlyWhenError(t *testing.T) { } func TestDeleteKeys(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{ @@ -728,8 +667,6 @@ func TestDeleteKeys(t *testing.T) { } func TestDeleteOne(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). DeleteItem(ctx, &dynamodb.DeleteItemInput{ @@ -749,8 +686,6 @@ func TestDeleteOne(t *testing.T) { } func TestUpdate(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). UpdateItem(ctx, &dynamodb.UpdateItemInput{ @@ -772,8 +707,6 @@ func TestUpdate(t *testing.T) { } func TestUpdateOnServiceError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). UpdateItem(ctx, &dynamodb.UpdateItemInput{ @@ -795,8 +728,6 @@ func TestUpdateOnServiceError(t *testing.T) { } func TestBatchPutOneBatch(t *testing.T) { - ctx := context.Background() - values := []any{map[string]string{"a": "b"}, map[string]string{"x": "y"}} itemA, _ := attributevalue.MarshalMap(values[0]) itemB, _ := attributevalue.MarshalMap(values[1]) @@ -828,8 +759,6 @@ func TestBatchPutOneBatch(t *testing.T) { } func TestOneBySk(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} skey, _ := attributevalue.Marshal("sk") data, _ := attributevalue.MarshalMap(expected) @@ -854,8 +783,6 @@ func TestOneBySk(t *testing.T) { } func TestOneBySKWhenNotOneResult(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} data, _ := attributevalue.MarshalMap(expected) @@ -890,8 +817,6 @@ func TestOneBySKWhenNotOneResult(t *testing.T) { } func TestOneBySkWhenQueryError(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} data, _ := attributevalue.MarshalMap(expected) @@ -984,8 +909,6 @@ func TestMoveWhenOtherCancellation(t *testing.T) { } func TestAnyByPK(t *testing.T) { - ctx := context.Background() - expected := map[string]string{"Col": "Val"} pkey, _ := attributevalue.Marshal("a-pk") data, _ := attributevalue.MarshalMap(expected) @@ -1010,8 +933,6 @@ func TestAnyByPK(t *testing.T) { } func TestAnyByPKOnQueryError(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -1025,8 +946,6 @@ func TestAnyByPKOnQueryError(t *testing.T) { } func TestAnyByPKWhenNotFound(t *testing.T) { - ctx := context.Background() - dynamoDB := newMockDynamoDB(t) dynamoDB.EXPECT(). Query(ctx, mock.Anything). @@ -1038,3 +957,129 @@ func TestAnyByPKWhenNotFound(t *testing.T) { err := c.AnyByPK(ctx, testPK("a-pk"), &v) assert.Equal(t, NotFoundError{}, err) } + +func TestAllScheduledEventsByUID(t *testing.T) { + expected := []map[string]string{{"Col": "Val"}, {"Other": "Thing"}} + data, _ := attributevalue.MarshalMap(expected[0]) + data2, _ := attributevalue.MarshalMap(expected[1]) + + dynamoDB := newMockDynamoDB(t) + dynamoDB.EXPECT(). + Query(ctx, &dynamodb.QueryInput{ + TableName: aws.String("this"), + IndexName: aws.String(lpaUIDIndex), + ExpressionAttributeNames: map[string]string{ + "#LpaUID": "LpaUID", + "#SK": "SK", + }, + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":LpaUID": &types.AttributeValueMemberS{Value: "lpa-uid"}, + ":SK": &types.AttributeValueMemberS{Value: scheduledPrefix}, + }, + KeyConditionExpression: aws.String("#LpaUID = :LpaUID"), + FilterExpression: aws.String("begins_with(#SK, :SK)"), + }). + Return(&dynamodb.QueryOutput{Items: []map[string]types.AttributeValue{data, data2}}, nil) + + c := &Client{table: "this", svc: dynamoDB} + + var v []map[string]string + err := c.AllScheduledEventsByUID(ctx, "lpa-uid", &v) + assert.Nil(t, err) +} + +func TestAllScheduledEventsByUIDWhenQueryError(t *testing.T) { + dynamoDB := newMockDynamoDB(t) + dynamoDB.EXPECT(). + Query(ctx, mock.Anything). + Return(&dynamodb.QueryOutput{Items: []map[string]types.AttributeValue{}}, expectedError) + + c := &Client{table: "this", svc: dynamoDB} + + var v []map[string]string + err := c.AllScheduledEventsByUID(ctx, "lpa-uid", &v) + assert.Equal(t, fmt.Errorf("failed to query scheduled event by UID: %w", expectedError), err) +} + +func TestAllScheduledEventsByUIDWhenNoResults(t *testing.T) { + dynamoDB := newMockDynamoDB(t) + dynamoDB.EXPECT(). + Query(ctx, mock.Anything). + Return(&dynamodb.QueryOutput{Items: []map[string]types.AttributeValue{}}, nil) + + c := &Client{table: "this", svc: dynamoDB} + + var v []map[string]string + err := c.AllScheduledEventsByUID(ctx, "lpa-uid", &v) + assert.Equal(t, NotFoundError{}, err) +} + +func TestAllScheduledEventsByUIDWhenUnmarshalError(t *testing.T) { + expected := []map[string]string{{"Col": "Val"}} + data, _ := attributevalue.MarshalMap(expected[0]) + + dynamoDB := newMockDynamoDB(t) + dynamoDB.EXPECT(). + Query(ctx, mock.Anything). + Return(&dynamodb.QueryOutput{Items: []map[string]types.AttributeValue{data}}, nil) + + c := &Client{table: "this", svc: dynamoDB} + + var v []map[string]string + err := c.AllScheduledEventsByUID(ctx, "lpa-uid", v) + assert.Error(t, err) +} + +func TestDeleteManyByUID(t *testing.T) { + items := []types.TransactWriteItem{ + {Delete: &types.Delete{ + TableName: aws.String("this"), + Key: map[string]types.AttributeValue{ + "PK": &types.AttributeValueMemberS{Value: "a-pk1"}, + "SK": &types.AttributeValueMemberS{Value: "a-sk1"}, + }, + ConditionExpression: aws.String("LpaUID = :uid"), + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":uid": &types.AttributeValueMemberS{Value: "lpa-uid"}, + }, + }}, + {Delete: &types.Delete{ + TableName: aws.String("this"), + Key: map[string]types.AttributeValue{ + "PK": &types.AttributeValueMemberS{Value: "a-pk2"}, + "SK": &types.AttributeValueMemberS{Value: "a-sk2"}, + }, + ConditionExpression: aws.String("LpaUID = :uid"), + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":uid": &types.AttributeValueMemberS{Value: "lpa-uid"}, + }, + }}, + } + + dynamoDB := newMockDynamoDB(t) + dynamoDB.EXPECT(). + TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{ + TransactItems: items, + }). + Return(&dynamodb.TransactWriteItemsOutput{}, nil) + + c := &Client{table: "this", svc: dynamoDB} + err := c.DeleteManyByUID(ctx, []Keys{ + {PK: testPK("a-pk1"), SK: testSK("a-sk1")}, + {PK: testPK("a-pk2"), SK: testSK("a-sk2")}, + }, "lpa-uid") + + assert.Nil(t, err) +} + +func TestDeleteManyByUIDWhenWriteItemsError(t *testing.T) { + dynamoDB := newMockDynamoDB(t) + dynamoDB.EXPECT(). + TransactWriteItems(ctx, mock.Anything). + Return(&dynamodb.TransactWriteItemsOutput{}, expectedError) + + c := &Client{table: "this", svc: dynamoDB} + err := c.DeleteManyByUID(ctx, []Keys{}, "lpa-uid") + + assert.Equal(t, expectedError, err) +} diff --git a/internal/scheduled/event.go b/internal/scheduled/event.go index 2074fe5f70..3656917b42 100644 --- a/internal/scheduled/event.go +++ b/internal/scheduled/event.go @@ -20,6 +20,6 @@ type Event struct { TargetLpaKey dynamo.LpaKeyType // TargetLpaOwnerKey is used to specify the target of the action TargetLpaOwnerKey dynamo.LpaOwnerKeyType - // TargetLpaUID is the UID of the action target - TargetLpaUID string + // LpaUID is the LPA UID the action target relates to + LpaUID string } diff --git a/internal/scheduled/runner.go b/internal/scheduled/runner.go index 3bc12a84a3..742e4627a8 100644 --- a/internal/scheduled/runner.go +++ b/internal/scheduled/runner.go @@ -187,7 +187,7 @@ func (r *Runner) Run(ctx context.Context) error { ) if fn, ok := r.actions[row.Action]; ok { - lpa, err := r.lpaStoreClient.Lpa(ctx, row.TargetLpaUID) + lpa, err := r.lpaStoreClient.Lpa(ctx, row.LpaUID) if err != nil { r.Errored(ctx, row, err) continue diff --git a/internal/scheduled/runner_test.go b/internal/scheduled/runner_test.go index d1843714e1..e6808a8c83 100644 --- a/internal/scheduled/runner_test.go +++ b/internal/scheduled/runner_test.go @@ -31,7 +31,7 @@ var ( Action: 99, TargetLpaKey: dynamo.LpaKey("an-lpa"), TargetLpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("a-donor")), - TargetLpaUID: "lpa-uid", + LpaUID: "lpa-uid", } ) diff --git a/internal/scheduled/store.go b/internal/scheduled/store.go index c2c1e0f3f0..f418b249bb 100644 --- a/internal/scheduled/store.go +++ b/internal/scheduled/store.go @@ -2,14 +2,17 @@ package scheduled import ( "context" + "fmt" "time" "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" ) type DynamoClient interface { - Move(ctx context.Context, oldKeys dynamo.Keys, value any) error + AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error + DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error + Move(ctx context.Context, oldKeys dynamo.Keys, value any) error Put(ctx context.Context, v interface{}) error } @@ -48,3 +51,21 @@ func (s *Store) Put(ctx context.Context, row Event) error { return s.dynamoClient.Put(ctx, row) } + +func (s *Store) DeleteAllByUID(ctx context.Context, uid string) error { + var events []Event + if err := s.dynamoClient.AllScheduledEventsByUID(ctx, uid, &events); err != nil { + return err + } + + if len(events) == 0 { + return fmt.Errorf("no scheduled events found for UID %s", uid) + } + + var keys []dynamo.Keys + for _, e := range events { + keys = append(keys, dynamo.Keys{PK: e.PK, SK: e.SK}) + } + + return s.dynamoClient.DeleteManyByUID(ctx, keys, uid) +} From 405514ce54b20c502f05a8de46c9149e8eddd061 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Mon, 25 Nov 2024 21:00:59 +0000 Subject: [PATCH 04/14] MLPAB-2321 gen --- .../mock_ScheduledStore_test.go | 83 ++++++++ .../mock_dynamodbClient_test.go | 192 ++++++++++++++++++ cmd/event-received/mock_factory_test.go | 47 +++++ internal/app/mock_DynamoClient_test.go | 96 +++++++++ .../mock_Handler_test.go | 23 ++- internal/dynamo/mock_dynamoDB_test.go | 74 ------- internal/scheduled/mock_DynamoClient_test.go | 96 +++++++++ 7 files changed, 527 insertions(+), 84 deletions(-) create mode 100644 cmd/event-received/mock_ScheduledStore_test.go diff --git a/cmd/event-received/mock_ScheduledStore_test.go b/cmd/event-received/mock_ScheduledStore_test.go new file mode 100644 index 0000000000..e8f3ccb93f --- /dev/null +++ b/cmd/event-received/mock_ScheduledStore_test.go @@ -0,0 +1,83 @@ +// Code generated by mockery. DO NOT EDIT. + +package main + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// mockScheduledStore is an autogenerated mock type for the ScheduledStore type +type mockScheduledStore struct { + mock.Mock +} + +type mockScheduledStore_Expecter struct { + mock *mock.Mock +} + +func (_m *mockScheduledStore) EXPECT() *mockScheduledStore_Expecter { + return &mockScheduledStore_Expecter{mock: &_m.Mock} +} + +// DeleteAllByUID provides a mock function with given fields: ctx, uid +func (_m *mockScheduledStore) DeleteAllByUID(ctx context.Context, uid string) error { + ret := _m.Called(ctx, uid) + + if len(ret) == 0 { + panic("no return value specified for DeleteAllByUID") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, uid) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockScheduledStore_DeleteAllByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteAllByUID' +type mockScheduledStore_DeleteAllByUID_Call struct { + *mock.Call +} + +// DeleteAllByUID is a helper method to define mock.On call +// - ctx context.Context +// - uid string +func (_e *mockScheduledStore_Expecter) DeleteAllByUID(ctx interface{}, uid interface{}) *mockScheduledStore_DeleteAllByUID_Call { + return &mockScheduledStore_DeleteAllByUID_Call{Call: _e.mock.On("DeleteAllByUID", ctx, uid)} +} + +func (_c *mockScheduledStore_DeleteAllByUID_Call) Run(run func(ctx context.Context, uid string)) *mockScheduledStore_DeleteAllByUID_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *mockScheduledStore_DeleteAllByUID_Call) Return(_a0 error) *mockScheduledStore_DeleteAllByUID_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockScheduledStore_DeleteAllByUID_Call) RunAndReturn(run func(context.Context, string) error) *mockScheduledStore_DeleteAllByUID_Call { + _c.Call.Return(run) + return _c +} + +// newMockScheduledStore creates a new instance of mockScheduledStore. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func newMockScheduledStore(t interface { + mock.TestingT + Cleanup(func()) +}) *mockScheduledStore { + mock := &mockScheduledStore{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/cmd/event-received/mock_dynamodbClient_test.go b/cmd/event-received/mock_dynamodbClient_test.go index e5c37c2341..020d6a0e58 100644 --- a/cmd/event-received/mock_dynamodbClient_test.go +++ b/cmd/event-received/mock_dynamodbClient_test.go @@ -22,6 +22,102 @@ func (_m *mockDynamodbClient) EXPECT() *mockDynamodbClient_Expecter { return &mockDynamodbClient_Expecter{mock: &_m.Mock} } +// AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v +func (_m *mockDynamodbClient) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { + ret := _m.Called(ctx, uid, v) + + if len(ret) == 0 { + panic("no return value specified for AllScheduledEventsByUID") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) error); ok { + r0 = rf(ctx, uid, v) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamodbClient_AllScheduledEventsByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllScheduledEventsByUID' +type mockDynamodbClient_AllScheduledEventsByUID_Call struct { + *mock.Call +} + +// AllScheduledEventsByUID is a helper method to define mock.On call +// - ctx context.Context +// - uid string +// - v interface{} +func (_e *mockDynamodbClient_Expecter) AllScheduledEventsByUID(ctx interface{}, uid interface{}, v interface{}) *mockDynamodbClient_AllScheduledEventsByUID_Call { + return &mockDynamodbClient_AllScheduledEventsByUID_Call{Call: _e.mock.On("AllScheduledEventsByUID", ctx, uid, v)} +} + +func (_c *mockDynamodbClient_AllScheduledEventsByUID_Call) Run(run func(ctx context.Context, uid string, v interface{})) *mockDynamodbClient_AllScheduledEventsByUID_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(interface{})) + }) + return _c +} + +func (_c *mockDynamodbClient_AllScheduledEventsByUID_Call) Return(_a0 error) *mockDynamodbClient_AllScheduledEventsByUID_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamodbClient_AllScheduledEventsByUID_Call) RunAndReturn(run func(context.Context, string, interface{}) error) *mockDynamodbClient_AllScheduledEventsByUID_Call { + _c.Call.Return(run) + return _c +} + +// AnyByPK provides a mock function with given fields: ctx, pk, v +func (_m *mockDynamodbClient) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error { + ret := _m.Called(ctx, pk, v) + + if len(ret) == 0 { + panic("no return value specified for AnyByPK") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, dynamo.PK, interface{}) error); ok { + r0 = rf(ctx, pk, v) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamodbClient_AnyByPK_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AnyByPK' +type mockDynamodbClient_AnyByPK_Call struct { + *mock.Call +} + +// AnyByPK is a helper method to define mock.On call +// - ctx context.Context +// - pk dynamo.PK +// - v interface{} +func (_e *mockDynamodbClient_Expecter) AnyByPK(ctx interface{}, pk interface{}, v interface{}) *mockDynamodbClient_AnyByPK_Call { + return &mockDynamodbClient_AnyByPK_Call{Call: _e.mock.On("AnyByPK", ctx, pk, v)} +} + +func (_c *mockDynamodbClient_AnyByPK_Call) Run(run func(ctx context.Context, pk dynamo.PK, v interface{})) *mockDynamodbClient_AnyByPK_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(dynamo.PK), args[2].(interface{})) + }) + return _c +} + +func (_c *mockDynamodbClient_AnyByPK_Call) Return(_a0 error) *mockDynamodbClient_AnyByPK_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamodbClient_AnyByPK_Call) RunAndReturn(run func(context.Context, dynamo.PK, interface{}) error) *mockDynamodbClient_AnyByPK_Call { + _c.Call.Return(run) + return _c +} + // CreateOnly provides a mock function with given fields: ctx, v func (_m *mockDynamodbClient) CreateOnly(ctx context.Context, v interface{}) error { ret := _m.Called(ctx, v) @@ -69,6 +165,54 @@ func (_c *mockDynamodbClient_CreateOnly_Call) RunAndReturn(run func(context.Cont return _c } +// DeleteManyByUID provides a mock function with given fields: ctx, keys, uid +func (_m *mockDynamodbClient) DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error { + ret := _m.Called(ctx, keys, uid) + + if len(ret) == 0 { + panic("no return value specified for DeleteManyByUID") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys, string) error); ok { + r0 = rf(ctx, keys, uid) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamodbClient_DeleteManyByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteManyByUID' +type mockDynamodbClient_DeleteManyByUID_Call struct { + *mock.Call +} + +// DeleteManyByUID is a helper method to define mock.On call +// - ctx context.Context +// - keys []dynamo.Keys +// - uid string +func (_e *mockDynamodbClient_Expecter) DeleteManyByUID(ctx interface{}, keys interface{}, uid interface{}) *mockDynamodbClient_DeleteManyByUID_Call { + return &mockDynamodbClient_DeleteManyByUID_Call{Call: _e.mock.On("DeleteManyByUID", ctx, keys, uid)} +} + +func (_c *mockDynamodbClient_DeleteManyByUID_Call) Run(run func(ctx context.Context, keys []dynamo.Keys, uid string)) *mockDynamodbClient_DeleteManyByUID_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].([]dynamo.Keys), args[2].(string)) + }) + return _c +} + +func (_c *mockDynamodbClient_DeleteManyByUID_Call) Return(_a0 error) *mockDynamodbClient_DeleteManyByUID_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamodbClient_DeleteManyByUID_Call) RunAndReturn(run func(context.Context, []dynamo.Keys, string) error) *mockDynamodbClient_DeleteManyByUID_Call { + _c.Call.Return(run) + return _c +} + // DeleteOne provides a mock function with given fields: ctx, pk, sk func (_m *mockDynamodbClient) DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error { ret := _m.Called(ctx, pk, sk) @@ -117,6 +261,54 @@ func (_c *mockDynamodbClient_DeleteOne_Call) RunAndReturn(run func(context.Conte return _c } +// Move provides a mock function with given fields: ctx, oldKeys, value +func (_m *mockDynamodbClient) Move(ctx context.Context, oldKeys dynamo.Keys, value any) error { + ret := _m.Called(ctx, oldKeys, value) + + if len(ret) == 0 { + panic("no return value specified for Move") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, dynamo.Keys, any) error); ok { + r0 = rf(ctx, oldKeys, value) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamodbClient_Move_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Move' +type mockDynamodbClient_Move_Call struct { + *mock.Call +} + +// Move is a helper method to define mock.On call +// - ctx context.Context +// - oldKeys dynamo.Keys +// - value any +func (_e *mockDynamodbClient_Expecter) Move(ctx interface{}, oldKeys interface{}, value interface{}) *mockDynamodbClient_Move_Call { + return &mockDynamodbClient_Move_Call{Call: _e.mock.On("Move", ctx, oldKeys, value)} +} + +func (_c *mockDynamodbClient_Move_Call) Run(run func(ctx context.Context, oldKeys dynamo.Keys, value any)) *mockDynamodbClient_Move_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(dynamo.Keys), args[2].(any)) + }) + return _c +} + +func (_c *mockDynamodbClient_Move_Call) Return(_a0 error) *mockDynamodbClient_Move_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamodbClient_Move_Call) RunAndReturn(run func(context.Context, dynamo.Keys, any) error) *mockDynamodbClient_Move_Call { + _c.Call.Return(run) + return _c +} + // One provides a mock function with given fields: ctx, pk, sk, v func (_m *mockDynamodbClient) One(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) error { ret := _m.Called(ctx, pk, sk, v) diff --git a/cmd/event-received/mock_factory_test.go b/cmd/event-received/mock_factory_test.go index d0c39b8c09..55a242cb81 100644 --- a/cmd/event-received/mock_factory_test.go +++ b/cmd/event-received/mock_factory_test.go @@ -278,6 +278,53 @@ func (_c *mockFactory_Now_Call) RunAndReturn(run func() func() time.Time) *mockF return _c } +// ScheduledStore provides a mock function with given fields: +func (_m *mockFactory) ScheduledStore() ScheduledStore { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for ScheduledStore") + } + + var r0 ScheduledStore + if rf, ok := ret.Get(0).(func() ScheduledStore); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(ScheduledStore) + } + } + + return r0 +} + +// mockFactory_ScheduledStore_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ScheduledStore' +type mockFactory_ScheduledStore_Call struct { + *mock.Call +} + +// ScheduledStore is a helper method to define mock.On call +func (_e *mockFactory_Expecter) ScheduledStore() *mockFactory_ScheduledStore_Call { + return &mockFactory_ScheduledStore_Call{Call: _e.mock.On("ScheduledStore")} +} + +func (_c *mockFactory_ScheduledStore_Call) Run(run func()) *mockFactory_ScheduledStore_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *mockFactory_ScheduledStore_Call) Return(_a0 ScheduledStore) *mockFactory_ScheduledStore_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockFactory_ScheduledStore_Call) RunAndReturn(run func() ScheduledStore) *mockFactory_ScheduledStore_Call { + _c.Call.Return(run) + return _c +} + // ShareCodeSender provides a mock function with given fields: ctx func (_m *mockFactory) ShareCodeSender(ctx context.Context) (ShareCodeSender, error) { ret := _m.Called(ctx) diff --git a/internal/app/mock_DynamoClient_test.go b/internal/app/mock_DynamoClient_test.go index cfcc7dc72e..7ea1c6c02c 100644 --- a/internal/app/mock_DynamoClient_test.go +++ b/internal/app/mock_DynamoClient_test.go @@ -239,6 +239,54 @@ func (_c *mockDynamoClient_AllKeysByPK_Call) RunAndReturn(run func(context.Conte return _c } +// AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v +func (_m *mockDynamoClient) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { + ret := _m.Called(ctx, uid, v) + + if len(ret) == 0 { + panic("no return value specified for AllScheduledEventsByUID") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) error); ok { + r0 = rf(ctx, uid, v) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamoClient_AllScheduledEventsByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllScheduledEventsByUID' +type mockDynamoClient_AllScheduledEventsByUID_Call struct { + *mock.Call +} + +// AllScheduledEventsByUID is a helper method to define mock.On call +// - ctx context.Context +// - uid string +// - v interface{} +func (_e *mockDynamoClient_Expecter) AllScheduledEventsByUID(ctx interface{}, uid interface{}, v interface{}) *mockDynamoClient_AllScheduledEventsByUID_Call { + return &mockDynamoClient_AllScheduledEventsByUID_Call{Call: _e.mock.On("AllScheduledEventsByUID", ctx, uid, v)} +} + +func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Run(run func(ctx context.Context, uid string, v interface{})) *mockDynamoClient_AllScheduledEventsByUID_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(interface{})) + }) + return _c +} + +func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Return(_a0 error) *mockDynamoClient_AllScheduledEventsByUID_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) RunAndReturn(run func(context.Context, string, interface{}) error) *mockDynamoClient_AllScheduledEventsByUID_Call { + _c.Call.Return(run) + return _c +} + // AnyByPK provides a mock function with given fields: ctx, pk, v func (_m *mockDynamoClient) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error { ret := _m.Called(ctx, pk, v) @@ -475,6 +523,54 @@ func (_c *mockDynamoClient_DeleteKeys_Call) RunAndReturn(run func(context.Contex return _c } +// DeleteManyByUID provides a mock function with given fields: ctx, keys, uid +func (_m *mockDynamoClient) DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error { + ret := _m.Called(ctx, keys, uid) + + if len(ret) == 0 { + panic("no return value specified for DeleteManyByUID") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys, string) error); ok { + r0 = rf(ctx, keys, uid) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamoClient_DeleteManyByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteManyByUID' +type mockDynamoClient_DeleteManyByUID_Call struct { + *mock.Call +} + +// DeleteManyByUID is a helper method to define mock.On call +// - ctx context.Context +// - keys []dynamo.Keys +// - uid string +func (_e *mockDynamoClient_Expecter) DeleteManyByUID(ctx interface{}, keys interface{}, uid interface{}) *mockDynamoClient_DeleteManyByUID_Call { + return &mockDynamoClient_DeleteManyByUID_Call{Call: _e.mock.On("DeleteManyByUID", ctx, keys, uid)} +} + +func (_c *mockDynamoClient_DeleteManyByUID_Call) Run(run func(ctx context.Context, keys []dynamo.Keys, uid string)) *mockDynamoClient_DeleteManyByUID_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].([]dynamo.Keys), args[2].(string)) + }) + return _c +} + +func (_c *mockDynamoClient_DeleteManyByUID_Call) Return(_a0 error) *mockDynamoClient_DeleteManyByUID_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamoClient_DeleteManyByUID_Call) RunAndReturn(run func(context.Context, []dynamo.Keys, string) error) *mockDynamoClient_DeleteManyByUID_Call { + _c.Call.Return(run) + return _c +} + // DeleteOne provides a mock function with given fields: ctx, pk, sk func (_m *mockDynamoClient) DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error { ret := _m.Called(ctx, pk, sk) diff --git a/internal/certificateprovider/certificateproviderpage/mock_Handler_test.go b/internal/certificateprovider/certificateproviderpage/mock_Handler_test.go index a7b20112b9..7bd9902047 100644 --- a/internal/certificateprovider/certificateproviderpage/mock_Handler_test.go +++ b/internal/certificateprovider/certificateproviderpage/mock_Handler_test.go @@ -8,6 +8,8 @@ import ( appcontext "github.com/ministryofjustice/opg-modernising-lpa/internal/appcontext" certificateproviderdata "github.com/ministryofjustice/opg-modernising-lpa/internal/certificateprovider/certificateproviderdata" + lpadata "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore/lpadata" + mock "github.com/stretchr/testify/mock" ) @@ -24,17 +26,17 @@ func (_m *mockHandler) EXPECT() *mockHandler_Expecter { return &mockHandler_Expecter{mock: &_m.Mock} } -// Execute provides a mock function with given fields: data, w, r, details -func (_m *mockHandler) Execute(data appcontext.Data, w http.ResponseWriter, r *http.Request, details *certificateproviderdata.Provided) error { - ret := _m.Called(data, w, r, details) +// Execute provides a mock function with given fields: data, w, r, details, lpa +func (_m *mockHandler) Execute(data appcontext.Data, w http.ResponseWriter, r *http.Request, details *certificateproviderdata.Provided, lpa *lpadata.Lpa) error { + ret := _m.Called(data, w, r, details, lpa) if len(ret) == 0 { panic("no return value specified for Execute") } var r0 error - if rf, ok := ret.Get(0).(func(appcontext.Data, http.ResponseWriter, *http.Request, *certificateproviderdata.Provided) error); ok { - r0 = rf(data, w, r, details) + if rf, ok := ret.Get(0).(func(appcontext.Data, http.ResponseWriter, *http.Request, *certificateproviderdata.Provided, *lpadata.Lpa) error); ok { + r0 = rf(data, w, r, details, lpa) } else { r0 = ret.Error(0) } @@ -52,13 +54,14 @@ type mockHandler_Execute_Call struct { // - w http.ResponseWriter // - r *http.Request // - details *certificateproviderdata.Provided -func (_e *mockHandler_Expecter) Execute(data interface{}, w interface{}, r interface{}, details interface{}) *mockHandler_Execute_Call { - return &mockHandler_Execute_Call{Call: _e.mock.On("Execute", data, w, r, details)} +// - lpa *lpadata.Lpa +func (_e *mockHandler_Expecter) Execute(data interface{}, w interface{}, r interface{}, details interface{}, lpa interface{}) *mockHandler_Execute_Call { + return &mockHandler_Execute_Call{Call: _e.mock.On("Execute", data, w, r, details, lpa)} } -func (_c *mockHandler_Execute_Call) Run(run func(data appcontext.Data, w http.ResponseWriter, r *http.Request, details *certificateproviderdata.Provided)) *mockHandler_Execute_Call { +func (_c *mockHandler_Execute_Call) Run(run func(data appcontext.Data, w http.ResponseWriter, r *http.Request, details *certificateproviderdata.Provided, lpa *lpadata.Lpa)) *mockHandler_Execute_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(appcontext.Data), args[1].(http.ResponseWriter), args[2].(*http.Request), args[3].(*certificateproviderdata.Provided)) + run(args[0].(appcontext.Data), args[1].(http.ResponseWriter), args[2].(*http.Request), args[3].(*certificateproviderdata.Provided), args[4].(*lpadata.Lpa)) }) return _c } @@ -68,7 +71,7 @@ func (_c *mockHandler_Execute_Call) Return(_a0 error) *mockHandler_Execute_Call return _c } -func (_c *mockHandler_Execute_Call) RunAndReturn(run func(appcontext.Data, http.ResponseWriter, *http.Request, *certificateproviderdata.Provided) error) *mockHandler_Execute_Call { +func (_c *mockHandler_Execute_Call) RunAndReturn(run func(appcontext.Data, http.ResponseWriter, *http.Request, *certificateproviderdata.Provided, *lpadata.Lpa) error) *mockHandler_Execute_Call { _c.Call.Return(run) return _c } diff --git a/internal/dynamo/mock_dynamoDB_test.go b/internal/dynamo/mock_dynamoDB_test.go index 33d5734ef9..66808e0dca 100644 --- a/internal/dynamo/mock_dynamoDB_test.go +++ b/internal/dynamo/mock_dynamoDB_test.go @@ -96,80 +96,6 @@ func (_c *mockDynamoDB_BatchGetItem_Call) RunAndReturn(run func(context.Context, return _c } -// BatchWriteItem provides a mock function with given fields: ctx, params, optFns -func (_m *mockDynamoDB) BatchWriteItem(ctx context.Context, params *dynamodb.BatchWriteItemInput, optFns ...func(*dynamodb.Options)) (*dynamodb.BatchWriteItemOutput, error) { - _va := make([]interface{}, len(optFns)) - for _i := range optFns { - _va[_i] = optFns[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, params) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - if len(ret) == 0 { - panic("no return value specified for BatchWriteItem") - } - - var r0 *dynamodb.BatchWriteItemOutput - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *dynamodb.BatchWriteItemInput, ...func(*dynamodb.Options)) (*dynamodb.BatchWriteItemOutput, error)); ok { - return rf(ctx, params, optFns...) - } - if rf, ok := ret.Get(0).(func(context.Context, *dynamodb.BatchWriteItemInput, ...func(*dynamodb.Options)) *dynamodb.BatchWriteItemOutput); ok { - r0 = rf(ctx, params, optFns...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*dynamodb.BatchWriteItemOutput) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, *dynamodb.BatchWriteItemInput, ...func(*dynamodb.Options)) error); ok { - r1 = rf(ctx, params, optFns...) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// mockDynamoDB_BatchWriteItem_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'BatchWriteItem' -type mockDynamoDB_BatchWriteItem_Call struct { - *mock.Call -} - -// BatchWriteItem is a helper method to define mock.On call -// - ctx context.Context -// - params *dynamodb.BatchWriteItemInput -// - optFns ...func(*dynamodb.Options) -func (_e *mockDynamoDB_Expecter) BatchWriteItem(ctx interface{}, params interface{}, optFns ...interface{}) *mockDynamoDB_BatchWriteItem_Call { - return &mockDynamoDB_BatchWriteItem_Call{Call: _e.mock.On("BatchWriteItem", - append([]interface{}{ctx, params}, optFns...)...)} -} - -func (_c *mockDynamoDB_BatchWriteItem_Call) Run(run func(ctx context.Context, params *dynamodb.BatchWriteItemInput, optFns ...func(*dynamodb.Options))) *mockDynamoDB_BatchWriteItem_Call { - _c.Call.Run(func(args mock.Arguments) { - variadicArgs := make([]func(*dynamodb.Options), len(args)-2) - for i, a := range args[2:] { - if a != nil { - variadicArgs[i] = a.(func(*dynamodb.Options)) - } - } - run(args[0].(context.Context), args[1].(*dynamodb.BatchWriteItemInput), variadicArgs...) - }) - return _c -} - -func (_c *mockDynamoDB_BatchWriteItem_Call) Return(_a0 *dynamodb.BatchWriteItemOutput, _a1 error) *mockDynamoDB_BatchWriteItem_Call { - _c.Call.Return(_a0, _a1) - return _c -} - -func (_c *mockDynamoDB_BatchWriteItem_Call) RunAndReturn(run func(context.Context, *dynamodb.BatchWriteItemInput, ...func(*dynamodb.Options)) (*dynamodb.BatchWriteItemOutput, error)) *mockDynamoDB_BatchWriteItem_Call { - _c.Call.Return(run) - return _c -} - // DeleteItem provides a mock function with given fields: _a0, _a1, _a2 func (_m *mockDynamoDB) DeleteItem(_a0 context.Context, _a1 *dynamodb.DeleteItemInput, _a2 ...func(*dynamodb.Options)) (*dynamodb.DeleteItemOutput, error) { _va := make([]interface{}, len(_a2)) diff --git a/internal/scheduled/mock_DynamoClient_test.go b/internal/scheduled/mock_DynamoClient_test.go index e28acd70be..5334e0e7dd 100644 --- a/internal/scheduled/mock_DynamoClient_test.go +++ b/internal/scheduled/mock_DynamoClient_test.go @@ -22,6 +22,54 @@ func (_m *mockDynamoClient) EXPECT() *mockDynamoClient_Expecter { return &mockDynamoClient_Expecter{mock: &_m.Mock} } +// AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v +func (_m *mockDynamoClient) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { + ret := _m.Called(ctx, uid, v) + + if len(ret) == 0 { + panic("no return value specified for AllScheduledEventsByUID") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) error); ok { + r0 = rf(ctx, uid, v) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamoClient_AllScheduledEventsByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllScheduledEventsByUID' +type mockDynamoClient_AllScheduledEventsByUID_Call struct { + *mock.Call +} + +// AllScheduledEventsByUID is a helper method to define mock.On call +// - ctx context.Context +// - uid string +// - v interface{} +func (_e *mockDynamoClient_Expecter) AllScheduledEventsByUID(ctx interface{}, uid interface{}, v interface{}) *mockDynamoClient_AllScheduledEventsByUID_Call { + return &mockDynamoClient_AllScheduledEventsByUID_Call{Call: _e.mock.On("AllScheduledEventsByUID", ctx, uid, v)} +} + +func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Run(run func(ctx context.Context, uid string, v interface{})) *mockDynamoClient_AllScheduledEventsByUID_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(interface{})) + }) + return _c +} + +func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Return(_a0 error) *mockDynamoClient_AllScheduledEventsByUID_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) RunAndReturn(run func(context.Context, string, interface{}) error) *mockDynamoClient_AllScheduledEventsByUID_Call { + _c.Call.Return(run) + return _c +} + // AnyByPK provides a mock function with given fields: ctx, pk, v func (_m *mockDynamoClient) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error { ret := _m.Called(ctx, pk, v) @@ -70,6 +118,54 @@ func (_c *mockDynamoClient_AnyByPK_Call) RunAndReturn(run func(context.Context, return _c } +// DeleteManyByUID provides a mock function with given fields: ctx, keys, uid +func (_m *mockDynamoClient) DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error { + ret := _m.Called(ctx, keys, uid) + + if len(ret) == 0 { + panic("no return value specified for DeleteManyByUID") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys, string) error); ok { + r0 = rf(ctx, keys, uid) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamoClient_DeleteManyByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteManyByUID' +type mockDynamoClient_DeleteManyByUID_Call struct { + *mock.Call +} + +// DeleteManyByUID is a helper method to define mock.On call +// - ctx context.Context +// - keys []dynamo.Keys +// - uid string +func (_e *mockDynamoClient_Expecter) DeleteManyByUID(ctx interface{}, keys interface{}, uid interface{}) *mockDynamoClient_DeleteManyByUID_Call { + return &mockDynamoClient_DeleteManyByUID_Call{Call: _e.mock.On("DeleteManyByUID", ctx, keys, uid)} +} + +func (_c *mockDynamoClient_DeleteManyByUID_Call) Run(run func(ctx context.Context, keys []dynamo.Keys, uid string)) *mockDynamoClient_DeleteManyByUID_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].([]dynamo.Keys), args[2].(string)) + }) + return _c +} + +func (_c *mockDynamoClient_DeleteManyByUID_Call) Return(_a0 error) *mockDynamoClient_DeleteManyByUID_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamoClient_DeleteManyByUID_Call) RunAndReturn(run func(context.Context, []dynamo.Keys, string) error) *mockDynamoClient_DeleteManyByUID_Call { + _c.Call.Return(run) + return _c +} + // Move provides a mock function with given fields: ctx, oldKeys, value func (_m *mockDynamoClient) Move(ctx context.Context, oldKeys dynamo.Keys, value any) error { ret := _m.Called(ctx, oldKeys, value) From 703c9be359f6bb34968aaef3f45f72edaca851ad Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 26 Nov 2024 11:44:41 +0000 Subject: [PATCH 05/14] MLPAB-2321 dont error on unexpected ChangeType --- cmd/event-received/lpastore_event_handler.go | 3 + .../lpastore_event_handler_test.go | 183 ++++++++++-------- .../makeregister_event_handler_test.go | 7 - cmd/event-received/mock_test.go | 22 +++ internal/scheduled/mock_test.go | 14 ++ internal/scheduled/store_test.go | 66 +++++++ 6 files changed, 209 insertions(+), 86 deletions(-) create mode 100644 cmd/event-received/mock_test.go create mode 100644 internal/scheduled/mock_test.go diff --git a/cmd/event-received/lpastore_event_handler.go b/cmd/event-received/lpastore_event_handler.go index cc81ac1d75..ee5db73544 100644 --- a/cmd/event-received/lpastore_event_handler.go +++ b/cmd/event-received/lpastore_event_handler.go @@ -29,6 +29,9 @@ func (h *lpastoreEventHandler) Handle(ctx context.Context, factory factory, clou case "CANNOT_REGISTER": return handleCannotRegister(ctx, factory.ScheduledStore(), v) + + default: + return nil } } diff --git a/cmd/event-received/lpastore_event_handler_test.go b/cmd/event-received/lpastore_event_handler_test.go index 8046740278..5bf6d1d483 100644 --- a/cmd/event-received/lpastore_event_handler_test.go +++ b/cmd/event-received/lpastore_event_handler_test.go @@ -1,7 +1,6 @@ package main import ( - "context" "encoding/json" "fmt" "testing" @@ -20,7 +19,19 @@ func TestLpaStoreEventHandlerHandleUnknownEvent(t *testing.T) { assert.Equal(t, fmt.Errorf("unknown lpastore event"), err) } -func TestLpaStoreEventHandlerHandleLpaUpdated(t *testing.T) { +func TestLpaStoreEventHandlerHandleLpaUpdatedWhenChangeTypeNotExpected(t *testing.T) { + event := &events.CloudWatchEvent{ + DetailType: "lpa-updated", + Detail: json.RawMessage(`{"uid":"M-1111-2222-3333","changeType":"WHAT"}`), + } + + handler := &lpastoreEventHandler{} + + err := handler.Handle(ctx, nil, event) + assert.Nil(t, err) +} + +func TestLpaStoreEventHandlerHandleLpaUpdatedStatutoryWaitingPeriod(t *testing.T) { event := &events.CloudWatchEvent{ DetailType: "lpa-updated", Detail: json.RawMessage(`{"uid":"M-1111-2222-3333","changeType":"STATUTORY_WAITING_PERIOD"}`), @@ -35,20 +46,14 @@ func TestLpaStoreEventHandlerHandleLpaUpdated(t *testing.T) { updated.UpdateHash() client := newMockDynamodbClient(t) - client. - On("OneByUID", ctx, "M-1111-2222-3333", mock.Anything). - Return(func(ctx context.Context, uid string, v interface{}) error { - b, _ := json.Marshal(dynamo.Keys{PK: dynamo.LpaKey("123"), SK: dynamo.DonorKey("456")}) - json.Unmarshal(b, v) - return nil - }) - client. - On("One", ctx, dynamo.LpaKey("123"), dynamo.DonorKey("456"), mock.Anything). - Return(func(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) error { - b, _ := json.Marshal(donordata.Provided{PK: dynamo.LpaKey("123"), SK: dynamo.LpaOwnerKey(dynamo.DonorKey("456"))}) - json.Unmarshal(b, v) - return nil - }) + client.EXPECT(). + OneByUID(ctx, "M-1111-2222-3333", mock.Anything). + Return(nil). + SetData(dynamo.Keys{PK: dynamo.LpaKey("123"), SK: dynamo.DonorKey("456")}) + client.EXPECT(). + One(ctx, dynamo.LpaKey("123"), dynamo.DonorKey("456"), mock.Anything). + Return(nil). + SetData(donordata.Provided{PK: dynamo.LpaKey("123"), SK: dynamo.LpaOwnerKey(dynamo.DonorKey("456"))}) client.EXPECT(). Put(ctx, updated). Return(nil) @@ -63,28 +68,7 @@ func TestLpaStoreEventHandlerHandleLpaUpdated(t *testing.T) { assert.Nil(t, err) } -func TestLpaStoreEventHandlerHandleLpaUpdatedWhenChangeTypeNotStatutoryWaitingPeriod(t *testing.T) { - event := &events.CloudWatchEvent{ - DetailType: "lpa-updated", - Detail: json.RawMessage(`{"uid":"M-1111-2222-3333","changeType":"WHAT"}`), - } - - factory := newMockFactory(t) - factory.EXPECT().DynamoClient().Return(nil) - factory.EXPECT().Now().Return(testNowFn) - - handler := &lpastoreEventHandler{} - - err := handler.Handle(ctx, factory, event) - assert.Nil(t, err) -} - -func TestLpaStoreEventHandlerHandleLpaUpdatedWhenDynamoGetErrors(t *testing.T) { - event := &events.CloudWatchEvent{ - DetailType: "lpa-updated", - Detail: json.RawMessage(`{"uid":"M-1111-2222-3333","changeType":"STATUTORY_WAITING_PERIOD"}`), - } - +func TestHandleStatutoryWaitingPeriodWhenDynamoErrors(t *testing.T) { updated := &donordata.Provided{ PK: dynamo.LpaKey("123"), SK: dynamo.LpaOwnerKey(dynamo.DonorKey("456")), @@ -93,60 +77,101 @@ func TestLpaStoreEventHandlerHandleLpaUpdatedWhenDynamoGetErrors(t *testing.T) { } updated.UpdateHash() - client := newMockDynamodbClient(t) - client. - On("OneByUID", ctx, "M-1111-2222-3333", mock.Anything). - Return(expectedError) - - factory := newMockFactory(t) - factory.EXPECT().DynamoClient().Return(client) - factory.EXPECT().Now().Return(testNowFn) + testcases := map[string]struct { + dynamoClient func() *mockDynamodbClient + expectedError error + }{ + "OneByUID": { + dynamoClient: func() *mockDynamodbClient { + client := newMockDynamodbClient(t) + client.EXPECT(). + OneByUID(ctx, mock.Anything, mock.Anything). + Return(expectedError) + + return client + }, + expectedError: fmt.Errorf("failed to resolve uid: %w", expectedError), + }, + "One": { + dynamoClient: func() *mockDynamodbClient { + client := newMockDynamodbClient(t) + client.EXPECT(). + OneByUID(mock.Anything, mock.Anything, mock.Anything). + Return(nil). + SetData(dynamo.Keys{PK: dynamo.LpaKey("pk"), SK: dynamo.DonorKey("sk")}) + client.EXPECT(). + One(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(expectedError) + + return client + }, + expectedError: fmt.Errorf("failed to get LPA: %w", expectedError), + }, + "Put": { + dynamoClient: func() *mockDynamodbClient { + client := newMockDynamodbClient(t) + client.EXPECT(). + OneByUID(mock.Anything, mock.Anything, mock.Anything). + Return(nil). + SetData(dynamo.Keys{PK: dynamo.LpaKey("pk"), SK: dynamo.DonorKey("sk")}) + client.EXPECT(). + One(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil). + SetData(updated) + client.EXPECT(). + Put(mock.Anything, updated). + Return(expectedError) + + return client + }, + expectedError: fmt.Errorf("failed to update donor details: %w", expectedError), + }, + } - handler := &lpastoreEventHandler{} + for testName, tc := range testcases { + t.Run(testName, func(t *testing.T) { + event := lpaUpdatedEvent{ + UID: "M-1111-2222-3333", + ChangeType: "STATUTORY_WAITING_PERIOD", + } - err := handler.Handle(ctx, factory, event) - assert.ErrorIs(t, err, expectedError) + err := handleStatutoryWaitingPeriod(ctx, tc.dynamoClient(), testNowFn, event) + assert.ErrorIs(t, err, expectedError) + }) + } } -func TestLpaStoreEventHandlerHandleLpaUpdatedWhenDynamoPutErrors(t *testing.T) { +func TestLpaStoreEventHandlerHandleLpaUpdatedCannotRegister(t *testing.T) { event := &events.CloudWatchEvent{ DetailType: "lpa-updated", - Detail: json.RawMessage(`{"uid":"M-1111-2222-3333","changeType":"STATUTORY_WAITING_PERIOD"}`), + Detail: json.RawMessage(`{"uid":"M-1111-2222-3333","changeType":"CANNOT_REGISTER"}`), } - updated := &donordata.Provided{ - PK: dynamo.LpaKey("123"), - SK: dynamo.LpaOwnerKey(dynamo.DonorKey("456")), - StatutoryWaitingPeriodAt: testNow, - UpdatedAt: testNow, - } - updated.UpdateHash() - - client := newMockDynamodbClient(t) - client. - On("OneByUID", ctx, "M-1111-2222-3333", mock.Anything). - Return(func(ctx context.Context, uid string, v interface{}) error { - b, _ := json.Marshal(dynamo.Keys{PK: dynamo.LpaKey("123"), SK: dynamo.DonorKey("456")}) - json.Unmarshal(b, v) - return nil - }) - client. - On("One", ctx, dynamo.LpaKey("123"), dynamo.DonorKey("456"), mock.Anything). - Return(func(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) error { - b, _ := json.Marshal(donordata.Provided{PK: dynamo.LpaKey("123"), SK: dynamo.LpaOwnerKey(dynamo.DonorKey("456"))}) - json.Unmarshal(b, v) - return nil - }) - client.EXPECT(). - Put(ctx, updated). - Return(expectedError) + scheduledStore := newMockScheduledStore(t) + scheduledStore.EXPECT(). + DeleteAllByUID(ctx, "M-1111-2222-3333"). + Return(nil) factory := newMockFactory(t) - factory.EXPECT().DynamoClient().Return(client) - factory.EXPECT().Now().Return(testNowFn) + factory.EXPECT().ScheduledStore().Return(scheduledStore) handler := &lpastoreEventHandler{} err := handler.Handle(ctx, factory, event) + assert.Nil(t, err) +} + +func TestHandleCannotRegisterWhenStoreErrors(t *testing.T) { + event := lpaUpdatedEvent{ + UID: "M-1111-2222-3333", + ChangeType: "CANNOT_REGISTER", + } + + scheduledStore := newMockScheduledStore(t) + scheduledStore.EXPECT(). + DeleteAllByUID(mock.Anything, mock.Anything). + Return(expectedError) + + err := handleCannotRegister(ctx, scheduledStore, event) assert.ErrorIs(t, err, expectedError) } diff --git a/cmd/event-received/makeregister_event_handler_test.go b/cmd/event-received/makeregister_event_handler_test.go index aac084f9ef..5c959d9e5d 100644 --- a/cmd/event-received/makeregister_event_handler_test.go +++ b/cmd/event-received/makeregister_event_handler_test.go @@ -19,13 +19,6 @@ import ( "github.com/stretchr/testify/mock" ) -func (c *mockDynamodbClient_One_Call) SetData(data any) { - c.Run(func(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) { - b, _ := attributevalue.Marshal(data) - attributevalue.Unmarshal(b, v) - }) -} - func TestMakeRegisterHandlerHandleUnknownEvent(t *testing.T) { handler := &makeregisterEventHandler{} diff --git a/cmd/event-received/mock_test.go b/cmd/event-received/mock_test.go new file mode 100644 index 0000000000..ea102fb2a7 --- /dev/null +++ b/cmd/event-received/mock_test.go @@ -0,0 +1,22 @@ +package main + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue" + "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" +) + +func (c *mockDynamodbClient_OneByUID_Call) SetData(data any) { + c.Run(func(_ context.Context, _ string, v any) { + b, _ := attributevalue.Marshal(data) + attributevalue.Unmarshal(b, v) + }) +} + +func (c *mockDynamodbClient_One_Call) SetData(data any) { + c.Run(func(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) { + b, _ := attributevalue.Marshal(data) + attributevalue.Unmarshal(b, v) + }) +} diff --git a/internal/scheduled/mock_test.go b/internal/scheduled/mock_test.go new file mode 100644 index 0000000000..5250972fc8 --- /dev/null +++ b/internal/scheduled/mock_test.go @@ -0,0 +1,14 @@ +package scheduled + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue" +) + +func (c *mockDynamoClient_AllScheduledEventsByUID_Call) SetData(data any) { + c.Run(func(_ context.Context, _ string, v any) { + b, _ := attributevalue.Marshal(data) + attributevalue.Unmarshal(b, v) + }) +} diff --git a/internal/scheduled/store_test.go b/internal/scheduled/store_test.go index 2230700dd7..d82af359f2 100644 --- a/internal/scheduled/store_test.go +++ b/internal/scheduled/store_test.go @@ -105,3 +105,69 @@ func TestStorePut(t *testing.T) { err := store.Put(ctx, Event{At: at, Action: 99}) assert.Equal(t, expectedError, err) } + +func TestDeleteAllByUID(t *testing.T) { + now := time.Now() + yesterday := now.Add(-24 * time.Hour) + + dynamoClient := newMockDynamoClient(t) + dynamoClient.EXPECT(). + AllScheduledEventsByUID(ctx, "lpa-uid", mock.Anything). + Return(nil). + SetData([]Event{ + {LpaUID: "lpa-uid", PK: dynamo.ScheduledDayKey(now), SK: dynamo.ScheduledKey(now, 98)}, + {LpaUID: "lpa-uid", PK: dynamo.ScheduledDayKey(yesterday), SK: dynamo.ScheduledKey(yesterday, 99)}, + }) + dynamoClient.EXPECT(). + DeleteManyByUID(ctx, []dynamo.Keys{ + {PK: dynamo.ScheduledDayKey(now), SK: dynamo.ScheduledKey(now, 98)}, + {PK: dynamo.ScheduledDayKey(yesterday), SK: dynamo.ScheduledKey(yesterday, 99)}, + }, "lpa-uid"). + Return(nil) + + store := &Store{dynamoClient: dynamoClient, now: testNowFn} + err := store.DeleteAllByUID(ctx, "lpa-uid") + + assert.Nil(t, err) +} + +func TestDeleteAllByUIDWhenAllScheduledEventsByUIDErrors(t *testing.T) { + dynamoClient := newMockDynamoClient(t) + dynamoClient.EXPECT(). + AllScheduledEventsByUID(ctx, mock.Anything, mock.Anything). + Return(expectedError) + + store := &Store{dynamoClient: dynamoClient, now: testNowFn} + err := store.DeleteAllByUID(ctx, "lpa-uid") + + assert.Equal(t, expectedError, err) +} + +func TestDeleteAllByUIDWhenNoEventsFound(t *testing.T) { + dynamoClient := newMockDynamoClient(t) + dynamoClient.EXPECT(). + AllScheduledEventsByUID(ctx, mock.Anything, mock.Anything). + Return(nil). + SetData([]Event{}) + + store := &Store{dynamoClient: dynamoClient, now: testNowFn} + err := store.DeleteAllByUID(ctx, "lpa-uid") + + assert.ErrorContains(t, err, "no scheduled events found for UID lpa-uid") +} + +func TestDeleteAllByUIDWhenDeleteManyByUIDErrors(t *testing.T) { + dynamoClient := newMockDynamoClient(t) + dynamoClient.EXPECT(). + AllScheduledEventsByUID(ctx, mock.Anything, mock.Anything). + Return(nil). + SetData([]Event{{LpaUID: "lpa-uid"}}) + dynamoClient.EXPECT(). + DeleteManyByUID(mock.Anything, mock.Anything, mock.Anything). + Return(expectedError) + + store := &Store{dynamoClient: dynamoClient, now: testNowFn} + err := store.DeleteAllByUID(ctx, "lpa-uid") + + assert.Equal(t, expectedError, err) +} From 823d6458b1927758b827554107028318a8782cd6 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 26 Nov 2024 12:34:15 +0000 Subject: [PATCH 06/14] MLPAB-2321 revert ignoring cannot register LPAs in task runner --- .codecov.yml | 1 + cmd/schedule-runner/main.go | 15 +-- docker/docker-compose.yml | 2 - docker/localstack/localstack-init.sh | 2 +- internal/scheduled/runner.go | 15 +-- internal/scheduled/runner_test.go | 146 +-------------------------- 6 files changed, 7 insertions(+), 174 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 1d3856c7f1..9cb5b4e5c7 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -13,6 +13,7 @@ coverage: - "./cmd/mock-onelogin/main.go" - "./cmd/mock-os-api/main.go" - "./cmd/schedule-runner/main.go" + - "./cmd/scheduled-task-adder/main.go" - "./internal/identity/yoti*" - "./internal/notify/email.go" - "./internal/notify/sms.go" diff --git a/cmd/schedule-runner/main.go b/cmd/schedule-runner/main.go index 01a0df7cb8..370c09608c 100644 --- a/cmd/schedule-runner/main.go +++ b/cmd/schedule-runner/main.go @@ -11,15 +11,12 @@ import ( "github.com/aws/aws-lambda-go/lambda" "github.com/aws/aws-sdk-go-v2/aws" - v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/cloudwatch" "github.com/ministryofjustice/opg-modernising-lpa/internal/donor" "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/ministryofjustice/opg-modernising-lpa/internal/event" - lambdaclient "github.com/ministryofjustice/opg-modernising-lpa/internal/lambda" "github.com/ministryofjustice/opg-modernising-lpa/internal/localize" - "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/ministryofjustice/opg-modernising-lpa/internal/scheduled" "github.com/ministryofjustice/opg-modernising-lpa/internal/search" @@ -32,10 +29,8 @@ import ( ) var ( - awsBaseURL = os.Getenv("AWS_BASE_URL") - eventBusName = cmp.Or(os.Getenv("EVENT_BUS_NAME"), "default") - lpaStoreBaseURL = cmp.Or(os.Getenv("LPA_STORE_BASE_URL"), "http://mock-lpa-store:8080") - lpaStoreSecretARN = os.Getenv("LPA_STORE_SECRET_ARN") + awsBaseURL = os.Getenv("AWS_BASE_URL") + eventBusName = cmp.Or(os.Getenv("EVENT_BUS_NAME"), "default") // TODO remove in MLPAB-2690 metricsEnabled = os.Getenv("METRICS_ENABLED") == "1" notifyBaseURL = os.Getenv("GOVUK_NOTIFY_BASE_URL") @@ -96,11 +91,7 @@ func handleRunSchedule(ctx context.Context) error { client := cloudwatch.NewFromConfig(cfg) metricsClient := telemetry.NewMetricsClient(client, Tag) - lambdaClient := lambdaclient.New(cfg, v4.NewSigner(), httpClient, time.Now) - - lpaStoreClient := lpastore.New(lpaStoreBaseURL, secretsClient, lpaStoreSecretARN, lambdaClient) - - runner := scheduled.NewRunner(logger, scheduledStore, donorStore, notifyClient, metricsClient, metricsEnabled, lpaStoreClient) + runner := scheduled.NewRunner(logger, scheduledStore, donorStore, notifyClient, metricsClient, metricsEnabled) if err = runner.Run(ctx); err != nil { logger.Error("runner error", slog.Any("err", err)) diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index e85f6a0c8c..8519c43d54 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -69,8 +69,6 @@ services: - AWS_REGION=eu-west-1 - AWS_SECRET_ACCESS_KEY=fakeAccessKey - OPENSEARCH_CUSTOM_BACKEND=http://opensearch:9200 - - LPA_STORE_BASE_URL=http://mock-lpa-store:8080 - - LPA_STORE_SECRET_ARN=lpa-store-jwt-secret-key - LPAS_TABLE=lpas - TAG=0.0.0 - GOVUK_NOTIFY_BASE_URL=http://mock-notify:8080 diff --git a/docker/localstack/localstack-init.sh b/docker/localstack/localstack-init.sh index 112e12ec9b..24e5f42f4b 100644 --- a/docker/localstack/localstack-init.sh +++ b/docker/localstack/localstack-init.sh @@ -85,7 +85,7 @@ awslocal lambda wait function-active-v2 --region eu-west-1 --function-name event echo 'creating schedule-runner lambda' awslocal lambda create-function \ - --environment Variables="{AWS_BASE_URL=$AWS_BASE_URL,TAG=$TAG,LPAS_TABLE=$LPAS_TABLE,GOVUK_NOTIFY_IS_PRODUCTION=$GOVUK_NOTIFY_IS_PRODUCTION,GOVUK_NOTIFY_BASE_URL=$GOVUK_NOTIFY_BASE_URL,SEARCH_ENDPOINT=$SEARCH_ENDPOINT,SEARCH_INDEXING_DISABLED=$SEARCH_INDEXING_DISABLED,SEARCH_INDEX_NAME=$SEARCH_INDEX_NAME,XRAY_ENABLED=$XRAY_ENABLED,METRICS_ENABLED=$METRICS_ENABLED,LPA_STORE_BASE_URL=$LPA_STORE_BASE_URL,LPA_STORE_SECRET_ARN=$LPA_STORE_SECRET_ARN}" \ + --environment Variables="{AWS_BASE_URL=$AWS_BASE_URL,TAG=$TAG,LPAS_TABLE=$LPAS_TABLE,GOVUK_NOTIFY_IS_PRODUCTION=$GOVUK_NOTIFY_IS_PRODUCTION,GOVUK_NOTIFY_BASE_URL=$GOVUK_NOTIFY_BASE_URL,SEARCH_ENDPOINT=$SEARCH_ENDPOINT,SEARCH_INDEXING_DISABLED=$SEARCH_INDEXING_DISABLED,SEARCH_INDEX_NAME=$SEARCH_INDEX_NAME,XRAY_ENABLED=$XRAY_ENABLED,METRICS_ENABLED=$METRICS_ENABLED}" \ --region eu-west-1 \ --function-name schedule-runner \ --handler bootstrap \ diff --git a/internal/scheduled/runner.go b/internal/scheduled/runner.go index 742e4627a8..1b6fa4bc8c 100644 --- a/internal/scheduled/runner.go +++ b/internal/scheduled/runner.go @@ -70,10 +70,9 @@ type Runner struct { errored float64 // TODO remove in MLPAB-2690 metricsEnabled bool - lpaStoreClient LpaStoreClient } -func NewRunner(logger Logger, store ScheduledStore, donorStore DonorStore, notifyClient NotifyClient, metricsClient MetricsClient, metricsEnabled bool, lpaStoreClient LpaStoreClient) *Runner { +func NewRunner(logger Logger, store ScheduledStore, donorStore DonorStore, notifyClient NotifyClient, metricsClient MetricsClient, metricsEnabled bool) *Runner { r := &Runner{ logger: logger, store: store, @@ -84,7 +83,6 @@ func NewRunner(logger Logger, store ScheduledStore, donorStore DonorStore, notif waiter: &waiter{backoff: time.Second, sleep: time.Sleep, maxRetries: 10}, metricsClient: metricsClient, metricsEnabled: metricsEnabled, - lpaStoreClient: lpaStoreClient, } r.actions = map[Action]ActionFunc{ @@ -187,17 +185,6 @@ func (r *Runner) Run(ctx context.Context) error { ) if fn, ok := r.actions[row.Action]; ok { - lpa, err := r.lpaStoreClient.Lpa(ctx, row.LpaUID) - if err != nil { - r.Errored(ctx, row, err) - continue - } - - if lpa.CannotRegister { - r.Ignored(ctx, row) - continue - } - if err := fn(ctx, row); err != nil { if errors.Is(err, errStepIgnored) { r.Ignored(ctx, row) diff --git a/internal/scheduled/runner_test.go b/internal/scheduled/runner_test.go index e6808a8c83..b0d79167a5 100644 --- a/internal/scheduled/runner_test.go +++ b/internal/scheduled/runner_test.go @@ -14,7 +14,6 @@ import ( "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/ministryofjustice/opg-modernising-lpa/internal/identity" "github.com/ministryofjustice/opg-modernising-lpa/internal/localize" - "github.com/ministryofjustice/opg-modernising-lpa/internal/lpastore/lpadata" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -57,9 +56,8 @@ func TestNewRunner(t *testing.T) { donorStore := newMockDonorStore(t) notifyClient := newMockNotifyClient(t) metricsClient := newMockMetricsClient(t) - lpaStoreClient := newMockLpaStoreClient(t) - runner := NewRunner(logger, store, donorStore, notifyClient, metricsClient, true, lpaStoreClient) + runner := NewRunner(logger, store, donorStore, notifyClient, metricsClient, true) assert.Equal(t, logger, runner.logger) assert.Equal(t, store, runner.store) @@ -67,7 +65,6 @@ func TestNewRunner(t *testing.T) { assert.Equal(t, notifyClient, runner.notifyClient) assert.Equal(t, metricsClient, runner.metricsClient) assert.Equal(t, true, runner.metricsEnabled) - assert.Equal(t, lpaStoreClient, runner.lpaStoreClient) } func (m *mockMetricsClient) assertPutMetrics(processed, ignored, errored float64, err error) { @@ -135,11 +132,6 @@ func TestRunnerRun(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(1, 0, 0, nil) - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, "lpa-uid"). - Return(&lpadata.Lpa{}, nil) - runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -151,7 +143,6 @@ func TestRunnerRun(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, - lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -187,111 +178,6 @@ func TestRunnerRunWhenStepErrors(t *testing.T) { assert.Equal(t, expectedError, err) } -func TestRunnerRunWhenLpaStoreClientErrors(t *testing.T) { - logger := newMockLogger(t) - logger.EXPECT(). - InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) - logger.EXPECT(). - ErrorContext(ctx, "runner action error", - slog.String("action", "Action(99)"), - slog.String("target_pk", "LPA#an-lpa"), - slog.String("target_sk", "DONOR#a-donor"), - slog.Any("err", expectedError)) - logger.EXPECT(). - InfoContext(ctx, "no scheduled tasks to process") - - store := newMockScheduledStore(t) - store.EXPECT(). - Pop(ctx, testNow). - Return(testEvent, nil). - Once() - store.EXPECT(). - Pop(ctx, testNow). - Return(nil, dynamo.NotFoundError{}). - Once() - - waiter := newMockWaiter(t) - waiter.EXPECT().Reset() - - metricsClient := newMockMetricsClient(t) - metricsClient.assertPutMetrics(0, 0, 1, nil) - - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, "lpa-uid"). - Return(&lpadata.Lpa{}, expectedError) - - runner := &Runner{ - now: testNowFn, - since: testSinceFn, - logger: logger, - store: store, - waiter: waiter, - actions: map[Action]ActionFunc{ - 99: nil, - }, - metricsClient: metricsClient, - metricsEnabled: true, - lpaStoreClient: lpaStoreClient, - } - - err := runner.Run(ctx) - - assert.Nil(t, err) -} - -func TestRunnerRunWhenLpaStatusIsCannotRegister(t *testing.T) { - logger := newMockLogger(t) - logger.EXPECT(). - InfoContext(ctx, "runner action", slog.String("action", "Action(99)")) - logger.EXPECT(). - InfoContext(ctx, "runner action ignored", - slog.String("action", "Action(99)"), - slog.String("target_pk", "LPA#an-lpa"), - slog.String("target_sk", "DONOR#a-donor")) - logger.EXPECT(). - InfoContext(ctx, "no scheduled tasks to process") - - store := newMockScheduledStore(t) - store.EXPECT(). - Pop(ctx, testNow). - Return(testEvent, nil). - Once() - store.EXPECT(). - Pop(ctx, testNow). - Return(nil, dynamo.NotFoundError{}). - Once() - - waiter := newMockWaiter(t) - waiter.EXPECT().Reset() - - metricsClient := newMockMetricsClient(t) - metricsClient.assertPutMetrics(0, 1, 0, nil) - - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, "lpa-uid"). - Return(&lpadata.Lpa{CannotRegister: true}, nil) - - runner := &Runner{ - now: testNowFn, - since: testSinceFn, - logger: logger, - store: store, - waiter: waiter, - actions: map[Action]ActionFunc{ - 99: nil, - }, - metricsClient: metricsClient, - metricsEnabled: true, - lpaStoreClient: lpaStoreClient, - } - - err := runner.Run(ctx) - - assert.Nil(t, err) -} - func TestRunnerRunWhenActionIgnored(t *testing.T) { logger := newMockLogger(t) logger.EXPECT(). @@ -325,11 +211,6 @@ func TestRunnerRunWhenActionIgnored(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(0, 1, 0, nil) - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, "lpa-uid"). - Return(&lpadata.Lpa{}, nil) - runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -341,7 +222,6 @@ func TestRunnerRunWhenActionIgnored(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, - lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -382,11 +262,6 @@ func TestRunnerRunWhenActionErrors(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(0, 0, 1, nil) - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, "lpa-uid"). - Return(&lpadata.Lpa{}, nil) - runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -398,7 +273,6 @@ func TestRunnerRunWhenActionErrors(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, - lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -437,11 +311,6 @@ func TestRunnerRunWhenWaitingError(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(1, 0, 0, nil) - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, "lpa-uid"). - Return(&lpadata.Lpa{}, nil) - runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -453,7 +322,6 @@ func TestRunnerRunWhenWaitingError(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, - lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -488,11 +356,6 @@ func TestRunnerRunWhenMetricsDisabled(t *testing.T) { Execute(ctx, mock.Anything). Return(nil) - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, mock.Anything). - Return(&lpadata.Lpa{}, nil) - runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -504,7 +367,6 @@ func TestRunnerRunWhenMetricsDisabled(t *testing.T) { }, metricsClient: nil, metricsEnabled: false, - lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) @@ -544,11 +406,6 @@ func TestRunnerRunWhenMetricsClientError(t *testing.T) { metricsClient := newMockMetricsClient(t) metricsClient.assertPutMetrics(1, 0, 0, expectedError) - lpaStoreClient := newMockLpaStoreClient(t) - lpaStoreClient.EXPECT(). - Lpa(ctx, "lpa-uid"). - Return(&lpadata.Lpa{}, nil) - runner := &Runner{ now: testNowFn, since: testSinceFn, @@ -560,7 +417,6 @@ func TestRunnerRunWhenMetricsClientError(t *testing.T) { }, metricsClient: metricsClient, metricsEnabled: true, - lpaStoreClient: lpaStoreClient, } err := runner.Run(ctx) From ead6eebe6ce5e917a8cf5e6c699e428bf7057c86 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 26 Nov 2024 13:08:57 +0000 Subject: [PATCH 07/14] MLPAB-2321 bump cov --- cmd/event-received/factory_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cmd/event-received/factory_test.go b/cmd/event-received/factory_test.go index b0301d510c..573eb0b8c7 100644 --- a/cmd/event-received/factory_test.go +++ b/cmd/event-received/factory_test.go @@ -212,3 +212,18 @@ func TestEventClientWhenSet(t *testing.T) { client := factory.EventClient() assert.Equal(t, expected, client) } + +func TestScheduledStore(t *testing.T) { + factory := &Factory{} + + client := factory.ScheduledStore() + assert.NotNil(t, client) +} + +func TestScheduledStoreWhenSet(t *testing.T) { + expected := newMockScheduledStore(t) + factory := &Factory{scheduledStore: expected} + + client := factory.ScheduledStore() + assert.Equal(t, expected, client) +} From d8d2792774d1987db8283a95224cd95541a0d02b Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 26 Nov 2024 13:29:17 +0000 Subject: [PATCH 08/14] MLPAB-2321 give event-received permission to delete items --- terraform/environment/region/modules/event_received/main.tf | 1 + 1 file changed, 1 insertion(+) diff --git a/terraform/environment/region/modules/event_received/main.tf b/terraform/environment/region/modules/event_received/main.tf index 21ae987e2e..df31c6efca 100644 --- a/terraform/environment/region/modules/event_received/main.tf +++ b/terraform/environment/region/modules/event_received/main.tf @@ -249,6 +249,7 @@ data "aws_iam_policy_document" "event_received" { "dynamodb:Query", "dynamodb:GetItem", "dynamodb:UpdateItem", + "dynamodb:DeleteItem", ] resources = [ From 0853539e6a7aa351537a7ad746a9d75956a02630 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 26 Nov 2024 16:07:25 +0000 Subject: [PATCH 09/14] MLPAB-2321 generalise AllScheduledEventsByUID --- cmd/event-received/main.go | 2 +- cmd/event-received/mock_dynamodbClient_test.go | 2 +- internal/app/app.go | 2 +- internal/app/mock_DynamoClient_test.go | 2 +- internal/dynamo/client.go | 4 ++-- internal/dynamo/client_test.go | 8 ++++---- internal/dynamo/keys.go | 4 ++++ internal/scheduled/store.go | 5 +++-- 8 files changed, 17 insertions(+), 12 deletions(-) diff --git a/cmd/event-received/main.go b/cmd/event-received/main.go index f8961e8243..cd130ce5a1 100644 --- a/cmd/event-received/main.go +++ b/cmd/event-received/main.go @@ -78,7 +78,7 @@ type uidEvent struct { type dynamodbClient interface { AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error - AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error CreateOnly(ctx context.Context, v interface{}) error DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error diff --git a/cmd/event-received/mock_dynamodbClient_test.go b/cmd/event-received/mock_dynamodbClient_test.go index 020d6a0e58..4b8208c1a9 100644 --- a/cmd/event-received/mock_dynamodbClient_test.go +++ b/cmd/event-received/mock_dynamodbClient_test.go @@ -23,7 +23,7 @@ func (_m *mockDynamodbClient) EXPECT() *mockDynamodbClient_Expecter { } // AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v -func (_m *mockDynamodbClient) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { +func (_m *mockDynamodbClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error { ret := _m.Called(ctx, uid, v) if len(ret) == 0 { diff --git a/internal/app/app.go b/internal/app/app.go index ecb7426a32..14b722da1b 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -52,7 +52,7 @@ type DynamoClient interface { AllByKeys(ctx context.Context, keys []dynamo.Keys) ([]map[string]dynamodbtypes.AttributeValue, error) AllByPartialSK(ctx context.Context, pk dynamo.PK, partialSK dynamo.SK, v interface{}) error AllBySK(ctx context.Context, sk dynamo.SK, v interface{}) error - AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error AllKeysByPK(ctx context.Context, pk dynamo.PK) ([]dynamo.Keys, error) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error BatchPut(ctx context.Context, items []interface{}) error diff --git a/internal/app/mock_DynamoClient_test.go b/internal/app/mock_DynamoClient_test.go index 7ea1c6c02c..a980bfd8e0 100644 --- a/internal/app/mock_DynamoClient_test.go +++ b/internal/app/mock_DynamoClient_test.go @@ -240,7 +240,7 @@ func (_c *mockDynamoClient_AllKeysByPK_Call) RunAndReturn(run func(context.Conte } // AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v -func (_m *mockDynamoClient) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { +func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error { ret := _m.Called(ctx, uid, v) if len(ret) == 0 { diff --git a/internal/dynamo/client.go b/internal/dynamo/client.go index 85500aa7be..9c58b87bd7 100644 --- a/internal/dynamo/client.go +++ b/internal/dynamo/client.go @@ -86,7 +86,7 @@ func (c *Client) OneByUID(ctx context.Context, uid string, v interface{}) error return attributevalue.UnmarshalMap(response.Items[0], v) } -func (c *Client) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { +func (c *Client) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { response, err := c.svc.Query(ctx, &dynamodb.QueryInput{ TableName: aws.String(c.table), IndexName: aws.String(lpaUIDIndex), @@ -96,7 +96,7 @@ func (c *Client) AllScheduledEventsByUID(ctx context.Context, uid string, v inte }, ExpressionAttributeValues: map[string]types.AttributeValue{ ":LpaUID": &types.AttributeValueMemberS{Value: uid}, - ":SK": &types.AttributeValueMemberS{Value: scheduledPrefix}, + ":SK": &types.AttributeValueMemberS{Value: partialSK}, }, KeyConditionExpression: aws.String("#LpaUID = :LpaUID"), FilterExpression: aws.String("begins_with(#SK, :SK)"), diff --git a/internal/dynamo/client_test.go b/internal/dynamo/client_test.go index 92984cb001..ccedaa2371 100644 --- a/internal/dynamo/client_test.go +++ b/internal/dynamo/client_test.go @@ -984,7 +984,7 @@ func TestAllScheduledEventsByUID(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllScheduledEventsByUID(ctx, "lpa-uid", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", &v) assert.Nil(t, err) } @@ -997,7 +997,7 @@ func TestAllScheduledEventsByUIDWhenQueryError(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllScheduledEventsByUID(ctx, "lpa-uid", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", &v) assert.Equal(t, fmt.Errorf("failed to query scheduled event by UID: %w", expectedError), err) } @@ -1010,7 +1010,7 @@ func TestAllScheduledEventsByUIDWhenNoResults(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllScheduledEventsByUID(ctx, "lpa-uid", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", &v) assert.Equal(t, NotFoundError{}, err) } @@ -1026,7 +1026,7 @@ func TestAllScheduledEventsByUIDWhenUnmarshalError(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllScheduledEventsByUID(ctx, "lpa-uid", v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", v) assert.Error(t, err) } diff --git a/internal/dynamo/keys.go b/internal/dynamo/keys.go index b96d3324a4..951c3cd8cc 100644 --- a/internal/dynamo/keys.go +++ b/internal/dynamo/keys.go @@ -322,6 +322,10 @@ func ScheduledKey(at time.Time, action int) ScheduledKeyType { return ScheduledKeyType(scheduledPrefix + "#" + at.Format(time.RFC3339) + "#" + strconv.Itoa(action)) } +func PartialScheduleKey() string { + return scheduledPrefix + "#" +} + type ReservedKeyType string func (t ReservedKeyType) SK() string { return string(t) } diff --git a/internal/scheduled/store.go b/internal/scheduled/store.go index f418b249bb..50f0dd0d4b 100644 --- a/internal/scheduled/store.go +++ b/internal/scheduled/store.go @@ -9,7 +9,7 @@ import ( ) type DynamoClient interface { - AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error @@ -54,7 +54,8 @@ func (s *Store) Put(ctx context.Context, row Event) error { func (s *Store) DeleteAllByUID(ctx context.Context, uid string) error { var events []Event - if err := s.dynamoClient.AllScheduledEventsByUID(ctx, uid, &events); err != nil { + + if err := s.dynamoClient.AllByLpaUIDAndPartialSK(ctx, uid, dynamo.PartialScheduleKey(), &events); err != nil { return err } From dc071c9cb5c713c7d0d15002942d1ca7167659a6 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 26 Nov 2024 16:10:07 +0000 Subject: [PATCH 10/14] MLPAB-2321 fix interfaces --- cmd/event-received/main.go | 2 +- internal/app/app.go | 2 +- internal/dynamo/client.go | 2 +- internal/scheduled/store.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/event-received/main.go b/cmd/event-received/main.go index cd130ce5a1..afac6395e6 100644 --- a/cmd/event-received/main.go +++ b/cmd/event-received/main.go @@ -78,7 +78,7 @@ type uidEvent struct { type dynamodbClient interface { AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error - AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error CreateOnly(ctx context.Context, v interface{}) error DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error diff --git a/internal/app/app.go b/internal/app/app.go index 14b722da1b..f6deb782fa 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -52,7 +52,7 @@ type DynamoClient interface { AllByKeys(ctx context.Context, keys []dynamo.Keys) ([]map[string]dynamodbtypes.AttributeValue, error) AllByPartialSK(ctx context.Context, pk dynamo.PK, partialSK dynamo.SK, v interface{}) error AllBySK(ctx context.Context, sk dynamo.SK, v interface{}) error - AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error AllKeysByPK(ctx context.Context, pk dynamo.PK) ([]dynamo.Keys, error) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error BatchPut(ctx context.Context, items []interface{}) error diff --git a/internal/dynamo/client.go b/internal/dynamo/client.go index 9c58b87bd7..050f5fcbf7 100644 --- a/internal/dynamo/client.go +++ b/internal/dynamo/client.go @@ -86,7 +86,7 @@ func (c *Client) OneByUID(ctx context.Context, uid string, v interface{}) error return attributevalue.UnmarshalMap(response.Items[0], v) } -func (c *Client) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { +func (c *Client) AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error { response, err := c.svc.Query(ctx, &dynamodb.QueryInput{ TableName: aws.String(c.table), IndexName: aws.String(lpaUIDIndex), diff --git a/internal/scheduled/store.go b/internal/scheduled/store.go index 50f0dd0d4b..4857fe2ae4 100644 --- a/internal/scheduled/store.go +++ b/internal/scheduled/store.go @@ -9,7 +9,7 @@ import ( ) type DynamoClient interface { - AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error From 4a79bc188413286d0f26e9aee843805f0bab50d0 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 26 Nov 2024 16:18:03 +0000 Subject: [PATCH 11/14] MLPAB-2321 test fix --- .../mock_dynamodbClient_test.go | 31 +++--- internal/app/mock_DynamoClient_test.go | 97 ++++++++++--------- internal/dynamo/client_test.go | 10 +- internal/scheduled/mock_DynamoClient_test.go | 31 +++--- internal/scheduled/mock_test.go | 12 ++- internal/scheduled/store_test.go | 19 +--- 6 files changed, 101 insertions(+), 99 deletions(-) diff --git a/cmd/event-received/mock_dynamodbClient_test.go b/cmd/event-received/mock_dynamodbClient_test.go index 4b8208c1a9..5757bf2a08 100644 --- a/cmd/event-received/mock_dynamodbClient_test.go +++ b/cmd/event-received/mock_dynamodbClient_test.go @@ -22,17 +22,17 @@ func (_m *mockDynamodbClient) EXPECT() *mockDynamodbClient_Expecter { return &mockDynamodbClient_Expecter{mock: &_m.Mock} } -// AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v -func (_m *mockDynamodbClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error { - ret := _m.Called(ctx, uid, v) +// AllByLpaUIDAndPartialSK provides a mock function with given fields: ctx, uid, partialSK, v +func (_m *mockDynamodbClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { + ret := _m.Called(ctx, uid, partialSK, v) if len(ret) == 0 { - panic("no return value specified for AllScheduledEventsByUID") + panic("no return value specified for AllByLpaUIDAndPartialSK") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) error); ok { - r0 = rf(ctx, uid, v) + if rf, ok := ret.Get(0).(func(context.Context, string, string, interface{}) error); ok { + r0 = rf(ctx, uid, partialSK, v) } else { r0 = ret.Error(0) } @@ -40,32 +40,33 @@ func (_m *mockDynamodbClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid s return r0 } -// mockDynamodbClient_AllScheduledEventsByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllScheduledEventsByUID' -type mockDynamodbClient_AllScheduledEventsByUID_Call struct { +// mockDynamodbClient_AllByLpaUIDAndPartialSK_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllByLpaUIDAndPartialSK' +type mockDynamodbClient_AllByLpaUIDAndPartialSK_Call struct { *mock.Call } -// AllScheduledEventsByUID is a helper method to define mock.On call +// AllByLpaUIDAndPartialSK is a helper method to define mock.On call // - ctx context.Context // - uid string +// - partialSK string // - v interface{} -func (_e *mockDynamodbClient_Expecter) AllScheduledEventsByUID(ctx interface{}, uid interface{}, v interface{}) *mockDynamodbClient_AllScheduledEventsByUID_Call { - return &mockDynamodbClient_AllScheduledEventsByUID_Call{Call: _e.mock.On("AllScheduledEventsByUID", ctx, uid, v)} +func (_e *mockDynamodbClient_Expecter) AllByLpaUIDAndPartialSK(ctx interface{}, uid interface{}, partialSK interface{}, v interface{}) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { + return &mockDynamodbClient_AllByLpaUIDAndPartialSK_Call{Call: _e.mock.On("AllByLpaUIDAndPartialSK", ctx, uid, partialSK, v)} } -func (_c *mockDynamodbClient_AllScheduledEventsByUID_Call) Run(run func(ctx context.Context, uid string, v interface{})) *mockDynamodbClient_AllScheduledEventsByUID_Call { +func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK string, v interface{})) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(interface{})) + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(interface{})) }) return _c } -func (_c *mockDynamodbClient_AllScheduledEventsByUID_Call) Return(_a0 error) *mockDynamodbClient_AllScheduledEventsByUID_Call { +func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) Return(_a0 error) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Return(_a0) return _c } -func (_c *mockDynamodbClient_AllScheduledEventsByUID_Call) RunAndReturn(run func(context.Context, string, interface{}) error) *mockDynamodbClient_AllScheduledEventsByUID_Call { +func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, string, interface{}) error) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Return(run) return _c } diff --git a/internal/app/mock_DynamoClient_test.go b/internal/app/mock_DynamoClient_test.go index a980bfd8e0..7a3030d4d9 100644 --- a/internal/app/mock_DynamoClient_test.go +++ b/internal/app/mock_DynamoClient_test.go @@ -83,6 +83,55 @@ func (_c *mockDynamoClient_AllByKeys_Call) RunAndReturn(run func(context.Context return _c } +// AllByLpaUIDAndPartialSK provides a mock function with given fields: ctx, uid, partialSK, v +func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { + ret := _m.Called(ctx, uid, partialSK, v) + + if len(ret) == 0 { + panic("no return value specified for AllByLpaUIDAndPartialSK") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, interface{}) error); ok { + r0 = rf(ctx, uid, partialSK, v) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamoClient_AllByLpaUIDAndPartialSK_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllByLpaUIDAndPartialSK' +type mockDynamoClient_AllByLpaUIDAndPartialSK_Call struct { + *mock.Call +} + +// AllByLpaUIDAndPartialSK is a helper method to define mock.On call +// - ctx context.Context +// - uid string +// - partialSK string +// - v interface{} +func (_e *mockDynamoClient_Expecter) AllByLpaUIDAndPartialSK(ctx interface{}, uid interface{}, partialSK interface{}, v interface{}) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { + return &mockDynamoClient_AllByLpaUIDAndPartialSK_Call{Call: _e.mock.On("AllByLpaUIDAndPartialSK", ctx, uid, partialSK, v)} +} + +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK string, v interface{})) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(interface{})) + }) + return _c +} + +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Return(_a0 error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, string, interface{}) error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { + _c.Call.Return(run) + return _c +} + // AllByPartialSK provides a mock function with given fields: ctx, pk, partialSK, v func (_m *mockDynamoClient) AllByPartialSK(ctx context.Context, pk dynamo.PK, partialSK dynamo.SK, v interface{}) error { ret := _m.Called(ctx, pk, partialSK, v) @@ -239,54 +288,6 @@ func (_c *mockDynamoClient_AllKeysByPK_Call) RunAndReturn(run func(context.Conte return _c } -// AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v -func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, v interface{}) error { - ret := _m.Called(ctx, uid, v) - - if len(ret) == 0 { - panic("no return value specified for AllScheduledEventsByUID") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) error); ok { - r0 = rf(ctx, uid, v) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// mockDynamoClient_AllScheduledEventsByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllScheduledEventsByUID' -type mockDynamoClient_AllScheduledEventsByUID_Call struct { - *mock.Call -} - -// AllScheduledEventsByUID is a helper method to define mock.On call -// - ctx context.Context -// - uid string -// - v interface{} -func (_e *mockDynamoClient_Expecter) AllScheduledEventsByUID(ctx interface{}, uid interface{}, v interface{}) *mockDynamoClient_AllScheduledEventsByUID_Call { - return &mockDynamoClient_AllScheduledEventsByUID_Call{Call: _e.mock.On("AllScheduledEventsByUID", ctx, uid, v)} -} - -func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Run(run func(ctx context.Context, uid string, v interface{})) *mockDynamoClient_AllScheduledEventsByUID_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(interface{})) - }) - return _c -} - -func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Return(_a0 error) *mockDynamoClient_AllScheduledEventsByUID_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) RunAndReturn(run func(context.Context, string, interface{}) error) *mockDynamoClient_AllScheduledEventsByUID_Call { - _c.Call.Return(run) - return _c -} - // AnyByPK provides a mock function with given fields: ctx, pk, v func (_m *mockDynamoClient) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error { ret := _m.Called(ctx, pk, v) diff --git a/internal/dynamo/client_test.go b/internal/dynamo/client_test.go index ccedaa2371..430e525935 100644 --- a/internal/dynamo/client_test.go +++ b/internal/dynamo/client_test.go @@ -974,7 +974,7 @@ func TestAllScheduledEventsByUID(t *testing.T) { }, ExpressionAttributeValues: map[string]types.AttributeValue{ ":LpaUID": &types.AttributeValueMemberS{Value: "lpa-uid"}, - ":SK": &types.AttributeValueMemberS{Value: scheduledPrefix}, + ":SK": &types.AttributeValueMemberS{Value: "partial-sk"}, }, KeyConditionExpression: aws.String("#LpaUID = :LpaUID"), FilterExpression: aws.String("begins_with(#SK, :SK)"), @@ -984,7 +984,7 @@ func TestAllScheduledEventsByUID(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", &v) assert.Nil(t, err) } @@ -997,7 +997,7 @@ func TestAllScheduledEventsByUIDWhenQueryError(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", &v) assert.Equal(t, fmt.Errorf("failed to query scheduled event by UID: %w", expectedError), err) } @@ -1010,7 +1010,7 @@ func TestAllScheduledEventsByUIDWhenNoResults(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", &v) assert.Equal(t, NotFoundError{}, err) } @@ -1026,7 +1026,7 @@ func TestAllScheduledEventsByUIDWhenUnmarshalError(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", v) assert.Error(t, err) } diff --git a/internal/scheduled/mock_DynamoClient_test.go b/internal/scheduled/mock_DynamoClient_test.go index 5334e0e7dd..786930bd32 100644 --- a/internal/scheduled/mock_DynamoClient_test.go +++ b/internal/scheduled/mock_DynamoClient_test.go @@ -22,17 +22,17 @@ func (_m *mockDynamoClient) EXPECT() *mockDynamoClient_Expecter { return &mockDynamoClient_Expecter{mock: &_m.Mock} } -// AllScheduledEventsByUID provides a mock function with given fields: ctx, uid, v -func (_m *mockDynamoClient) AllScheduledEventsByUID(ctx context.Context, uid string, v interface{}) error { - ret := _m.Called(ctx, uid, v) +// AllByLpaUIDAndPartialSK provides a mock function with given fields: ctx, uid, partialSK, v +func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { + ret := _m.Called(ctx, uid, partialSK, v) if len(ret) == 0 { - panic("no return value specified for AllScheduledEventsByUID") + panic("no return value specified for AllByLpaUIDAndPartialSK") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) error); ok { - r0 = rf(ctx, uid, v) + if rf, ok := ret.Get(0).(func(context.Context, string, string, interface{}) error); ok { + r0 = rf(ctx, uid, partialSK, v) } else { r0 = ret.Error(0) } @@ -40,32 +40,33 @@ func (_m *mockDynamoClient) AllScheduledEventsByUID(ctx context.Context, uid str return r0 } -// mockDynamoClient_AllScheduledEventsByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllScheduledEventsByUID' -type mockDynamoClient_AllScheduledEventsByUID_Call struct { +// mockDynamoClient_AllByLpaUIDAndPartialSK_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllByLpaUIDAndPartialSK' +type mockDynamoClient_AllByLpaUIDAndPartialSK_Call struct { *mock.Call } -// AllScheduledEventsByUID is a helper method to define mock.On call +// AllByLpaUIDAndPartialSK is a helper method to define mock.On call // - ctx context.Context // - uid string +// - partialSK string // - v interface{} -func (_e *mockDynamoClient_Expecter) AllScheduledEventsByUID(ctx interface{}, uid interface{}, v interface{}) *mockDynamoClient_AllScheduledEventsByUID_Call { - return &mockDynamoClient_AllScheduledEventsByUID_Call{Call: _e.mock.On("AllScheduledEventsByUID", ctx, uid, v)} +func (_e *mockDynamoClient_Expecter) AllByLpaUIDAndPartialSK(ctx interface{}, uid interface{}, partialSK interface{}, v interface{}) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { + return &mockDynamoClient_AllByLpaUIDAndPartialSK_Call{Call: _e.mock.On("AllByLpaUIDAndPartialSK", ctx, uid, partialSK, v)} } -func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Run(run func(ctx context.Context, uid string, v interface{})) *mockDynamoClient_AllScheduledEventsByUID_Call { +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK string, v interface{})) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(interface{})) + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(interface{})) }) return _c } -func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) Return(_a0 error) *mockDynamoClient_AllScheduledEventsByUID_Call { +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Return(_a0 error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Return(_a0) return _c } -func (_c *mockDynamoClient_AllScheduledEventsByUID_Call) RunAndReturn(run func(context.Context, string, interface{}) error) *mockDynamoClient_AllScheduledEventsByUID_Call { +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, string, interface{}) error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Return(run) return _c } diff --git a/internal/scheduled/mock_test.go b/internal/scheduled/mock_test.go index 5250972fc8..26c86cf83a 100644 --- a/internal/scheduled/mock_test.go +++ b/internal/scheduled/mock_test.go @@ -4,11 +4,19 @@ import ( "context" "github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue" + "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" ) -func (c *mockDynamoClient_AllScheduledEventsByUID_Call) SetData(data any) { - c.Run(func(_ context.Context, _ string, v any) { +func (c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) SetData(data any) { + c.Run(func(_ context.Context, _, _ string, v any) { b, _ := attributevalue.Marshal(data) attributevalue.Unmarshal(b, v) }) } + +func (c *mockDynamoClient_AnyByPK_Call) SetData(row *Event) { + c.Run(func(_ context.Context, _ dynamo.PK, v any) { + b, _ := attributevalue.Marshal(row) + attributevalue.Unmarshal(b, v) + }) +} diff --git a/internal/scheduled/store_test.go b/internal/scheduled/store_test.go index d82af359f2..ff86b3263c 100644 --- a/internal/scheduled/store_test.go +++ b/internal/scheduled/store_test.go @@ -1,23 +1,14 @@ package scheduled import ( - "context" "testing" "time" - "github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue" "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) -func (c *mockDynamoClient_AnyByPK_Call) SetData(row *Event) { - c.Run(func(_ context.Context, _ dynamo.PK, v any) { - b, _ := attributevalue.Marshal(row) - attributevalue.Unmarshal(b, v) - }) -} - func TestNewStore(t *testing.T) { dynamoClient := newMockDynamoClient(t) store := NewStore(dynamoClient) @@ -112,7 +103,7 @@ func TestDeleteAllByUID(t *testing.T) { dynamoClient := newMockDynamoClient(t) dynamoClient.EXPECT(). - AllScheduledEventsByUID(ctx, "lpa-uid", mock.Anything). + AllByLpaUIDAndPartialSK(ctx, "lpa-uid", dynamo.PartialScheduleKey(), mock.Anything). Return(nil). SetData([]Event{ {LpaUID: "lpa-uid", PK: dynamo.ScheduledDayKey(now), SK: dynamo.ScheduledKey(now, 98)}, @@ -131,10 +122,10 @@ func TestDeleteAllByUID(t *testing.T) { assert.Nil(t, err) } -func TestDeleteAllByUIDWhenAllScheduledEventsByUIDErrors(t *testing.T) { +func TestDeleteAllByUIDWhenAllByLpaUIDAndPartialSKErrors(t *testing.T) { dynamoClient := newMockDynamoClient(t) dynamoClient.EXPECT(). - AllScheduledEventsByUID(ctx, mock.Anything, mock.Anything). + AllByLpaUIDAndPartialSK(ctx, mock.Anything, mock.Anything, mock.Anything). Return(expectedError) store := &Store{dynamoClient: dynamoClient, now: testNowFn} @@ -146,7 +137,7 @@ func TestDeleteAllByUIDWhenAllScheduledEventsByUIDErrors(t *testing.T) { func TestDeleteAllByUIDWhenNoEventsFound(t *testing.T) { dynamoClient := newMockDynamoClient(t) dynamoClient.EXPECT(). - AllScheduledEventsByUID(ctx, mock.Anything, mock.Anything). + AllByLpaUIDAndPartialSK(ctx, mock.Anything, mock.Anything, mock.Anything). Return(nil). SetData([]Event{}) @@ -159,7 +150,7 @@ func TestDeleteAllByUIDWhenNoEventsFound(t *testing.T) { func TestDeleteAllByUIDWhenDeleteManyByUIDErrors(t *testing.T) { dynamoClient := newMockDynamoClient(t) dynamoClient.EXPECT(). - AllScheduledEventsByUID(ctx, mock.Anything, mock.Anything). + AllByLpaUIDAndPartialSK(ctx, mock.Anything, mock.Anything, mock.Anything). Return(nil). SetData([]Event{{LpaUID: "lpa-uid"}}) dynamoClient.EXPECT(). From 5c1380132e748a3ae0840527a49afd80d43e7f48 Mon Sep 17 00:00:00 2001 From: Joshua Hawxwell Date: Wed, 27 Nov 2024 08:11:09 +0000 Subject: [PATCH 12/14] Use Create to make scheduled events not Put --- cmd/event-received/main.go | 1 + .../mock_dynamodbClient_test.go | 47 ++++++++++ .../identity_with_one_login_callback.go | 2 +- .../identity_with_one_login_callback_test.go | 6 +- .../donorpage/mock_ScheduledStore_test.go | 22 ++--- internal/donor/donorpage/register.go | 2 +- internal/scheduled/mock_DynamoClient_test.go | 94 +++++++++---------- internal/scheduled/store.go | 6 +- internal/scheduled/store_test.go | 6 +- 9 files changed, 117 insertions(+), 69 deletions(-) diff --git a/cmd/event-received/main.go b/cmd/event-received/main.go index afac6395e6..a390d6cb68 100644 --- a/cmd/event-received/main.go +++ b/cmd/event-received/main.go @@ -79,6 +79,7 @@ type uidEvent struct { type dynamodbClient interface { AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error + Create(ctx context.Context, v interface{}) error CreateOnly(ctx context.Context, v interface{}) error DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error diff --git a/cmd/event-received/mock_dynamodbClient_test.go b/cmd/event-received/mock_dynamodbClient_test.go index 5757bf2a08..2d2fe5893c 100644 --- a/cmd/event-received/mock_dynamodbClient_test.go +++ b/cmd/event-received/mock_dynamodbClient_test.go @@ -119,6 +119,53 @@ func (_c *mockDynamodbClient_AnyByPK_Call) RunAndReturn(run func(context.Context return _c } +// Create provides a mock function with given fields: ctx, v +func (_m *mockDynamodbClient) Create(ctx context.Context, v interface{}) error { + ret := _m.Called(ctx, v) + + if len(ret) == 0 { + panic("no return value specified for Create") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, interface{}) error); ok { + r0 = rf(ctx, v) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamodbClient_Create_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Create' +type mockDynamodbClient_Create_Call struct { + *mock.Call +} + +// Create is a helper method to define mock.On call +// - ctx context.Context +// - v interface{} +func (_e *mockDynamodbClient_Expecter) Create(ctx interface{}, v interface{}) *mockDynamodbClient_Create_Call { + return &mockDynamodbClient_Create_Call{Call: _e.mock.On("Create", ctx, v)} +} + +func (_c *mockDynamodbClient_Create_Call) Run(run func(ctx context.Context, v interface{})) *mockDynamodbClient_Create_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(interface{})) + }) + return _c +} + +func (_c *mockDynamodbClient_Create_Call) Return(_a0 error) *mockDynamodbClient_Create_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamodbClient_Create_Call) RunAndReturn(run func(context.Context, interface{}) error) *mockDynamodbClient_Create_Call { + _c.Call.Return(run) + return _c +} + // CreateOnly provides a mock function with given fields: ctx, v func (_m *mockDynamodbClient) CreateOnly(ctx context.Context, v interface{}) error { ret := _m.Called(ctx, v) diff --git a/internal/donor/donorpage/identity_with_one_login_callback.go b/internal/donor/donorpage/identity_with_one_login_callback.go index 44caa71187..04be5266fa 100644 --- a/internal/donor/donorpage/identity_with_one_login_callback.go +++ b/internal/donor/donorpage/identity_with_one_login_callback.go @@ -80,7 +80,7 @@ func IdentityWithOneLoginCallback(oneLoginClient OneLoginClient, sessionStore Se case identity.StatusInsufficientEvidence: return donor.PathUnableToConfirmIdentity.Redirect(w, r, appData, provided) default: - if err := scheduledStore.Put(r.Context(), scheduled.Event{ + if err := scheduledStore.Create(r.Context(), scheduled.Event{ At: userData.CheckedAt.AddDate(0, 6, 0), Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: provided.PK, diff --git a/internal/donor/donorpage/identity_with_one_login_callback_test.go b/internal/donor/donorpage/identity_with_one_login_callback_test.go index e28bfe24ab..e5dff71584 100644 --- a/internal/donor/donorpage/identity_with_one_login_callback_test.go +++ b/internal/donor/donorpage/identity_with_one_login_callback_test.go @@ -60,7 +60,7 @@ func TestGetIdentityWithOneLoginCallback(t *testing.T) { scheduledStore := newMockScheduledStore(t) scheduledStore.EXPECT(). - Put(r.Context(), scheduled.Event{ + Create(r.Context(), scheduled.Event{ At: now.AddDate(0, 6, 0), Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: dynamo.LpaKey("hey"), @@ -122,7 +122,7 @@ func TestGetIdentityWithOneLoginCallbackWhenIdentityMismatched(t *testing.T) { scheduledStore := newMockScheduledStore(t) scheduledStore.EXPECT(). - Put(r.Context(), scheduled.Event{ + Create(r.Context(), scheduled.Event{ At: now.AddDate(0, 6, 0), Action: scheduled.ActionExpireDonorIdentity, TargetLpaKey: dynamo.LpaKey("hey"), @@ -234,7 +234,7 @@ func TestGetIdentityWithOneLoginCallbackWhenScheduledStoreErrors(t *testing.T) { scheduledStore := newMockScheduledStore(t) scheduledStore.EXPECT(). - Put(mock.Anything, mock.Anything). + Create(mock.Anything, mock.Anything). Return(expectedError) err := IdentityWithOneLoginCallback(oneLoginClient, sessionStore, donorStore, scheduledStore, nil)(testAppData, w, r, &donordata.Provided{ diff --git a/internal/donor/donorpage/mock_ScheduledStore_test.go b/internal/donor/donorpage/mock_ScheduledStore_test.go index 0ec8810bdb..cbf5610744 100644 --- a/internal/donor/donorpage/mock_ScheduledStore_test.go +++ b/internal/donor/donorpage/mock_ScheduledStore_test.go @@ -22,12 +22,12 @@ func (_m *mockScheduledStore) EXPECT() *mockScheduledStore_Expecter { return &mockScheduledStore_Expecter{mock: &_m.Mock} } -// Put provides a mock function with given fields: ctx, row -func (_m *mockScheduledStore) Put(ctx context.Context, row scheduled.Event) error { +// Create provides a mock function with given fields: ctx, row +func (_m *mockScheduledStore) Create(ctx context.Context, row scheduled.Event) error { ret := _m.Called(ctx, row) if len(ret) == 0 { - panic("no return value specified for Put") + panic("no return value specified for Create") } var r0 error @@ -40,31 +40,31 @@ func (_m *mockScheduledStore) Put(ctx context.Context, row scheduled.Event) erro return r0 } -// mockScheduledStore_Put_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Put' -type mockScheduledStore_Put_Call struct { +// mockScheduledStore_Create_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Create' +type mockScheduledStore_Create_Call struct { *mock.Call } -// Put is a helper method to define mock.On call +// Create is a helper method to define mock.On call // - ctx context.Context // - row scheduled.Event -func (_e *mockScheduledStore_Expecter) Put(ctx interface{}, row interface{}) *mockScheduledStore_Put_Call { - return &mockScheduledStore_Put_Call{Call: _e.mock.On("Put", ctx, row)} +func (_e *mockScheduledStore_Expecter) Create(ctx interface{}, row interface{}) *mockScheduledStore_Create_Call { + return &mockScheduledStore_Create_Call{Call: _e.mock.On("Create", ctx, row)} } -func (_c *mockScheduledStore_Put_Call) Run(run func(ctx context.Context, row scheduled.Event)) *mockScheduledStore_Put_Call { +func (_c *mockScheduledStore_Create_Call) Run(run func(ctx context.Context, row scheduled.Event)) *mockScheduledStore_Create_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(context.Context), args[1].(scheduled.Event)) }) return _c } -func (_c *mockScheduledStore_Put_Call) Return(_a0 error) *mockScheduledStore_Put_Call { +func (_c *mockScheduledStore_Create_Call) Return(_a0 error) *mockScheduledStore_Create_Call { _c.Call.Return(_a0) return _c } -func (_c *mockScheduledStore_Put_Call) RunAndReturn(run func(context.Context, scheduled.Event) error) *mockScheduledStore_Put_Call { +func (_c *mockScheduledStore_Create_Call) RunAndReturn(run func(context.Context, scheduled.Event) error) *mockScheduledStore_Create_Call { _c.Call.Return(run) return _c } diff --git a/internal/donor/donorpage/register.go b/internal/donor/donorpage/register.go index 2e0fa47d0c..0b82faa0f8 100644 --- a/internal/donor/donorpage/register.go +++ b/internal/donor/donorpage/register.go @@ -163,7 +163,7 @@ type ShareCodeStore interface { } type ScheduledStore interface { - Put(ctx context.Context, row scheduled.Event) error + Create(ctx context.Context, row scheduled.Event) error } type ErrorHandler func(http.ResponseWriter, *http.Request, error) diff --git a/internal/scheduled/mock_DynamoClient_test.go b/internal/scheduled/mock_DynamoClient_test.go index 786930bd32..28b2e57155 100644 --- a/internal/scheduled/mock_DynamoClient_test.go +++ b/internal/scheduled/mock_DynamoClient_test.go @@ -119,6 +119,53 @@ func (_c *mockDynamoClient_AnyByPK_Call) RunAndReturn(run func(context.Context, return _c } +// Create provides a mock function with given fields: ctx, v +func (_m *mockDynamoClient) Create(ctx context.Context, v interface{}) error { + ret := _m.Called(ctx, v) + + if len(ret) == 0 { + panic("no return value specified for Create") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, interface{}) error); ok { + r0 = rf(ctx, v) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockDynamoClient_Create_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Create' +type mockDynamoClient_Create_Call struct { + *mock.Call +} + +// Create is a helper method to define mock.On call +// - ctx context.Context +// - v interface{} +func (_e *mockDynamoClient_Expecter) Create(ctx interface{}, v interface{}) *mockDynamoClient_Create_Call { + return &mockDynamoClient_Create_Call{Call: _e.mock.On("Create", ctx, v)} +} + +func (_c *mockDynamoClient_Create_Call) Run(run func(ctx context.Context, v interface{})) *mockDynamoClient_Create_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(interface{})) + }) + return _c +} + +func (_c *mockDynamoClient_Create_Call) Return(_a0 error) *mockDynamoClient_Create_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockDynamoClient_Create_Call) RunAndReturn(run func(context.Context, interface{}) error) *mockDynamoClient_Create_Call { + _c.Call.Return(run) + return _c +} + // DeleteManyByUID provides a mock function with given fields: ctx, keys, uid func (_m *mockDynamoClient) DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error { ret := _m.Called(ctx, keys, uid) @@ -215,53 +262,6 @@ func (_c *mockDynamoClient_Move_Call) RunAndReturn(run func(context.Context, dyn return _c } -// Put provides a mock function with given fields: ctx, v -func (_m *mockDynamoClient) Put(ctx context.Context, v interface{}) error { - ret := _m.Called(ctx, v) - - if len(ret) == 0 { - panic("no return value specified for Put") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, interface{}) error); ok { - r0 = rf(ctx, v) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// mockDynamoClient_Put_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Put' -type mockDynamoClient_Put_Call struct { - *mock.Call -} - -// Put is a helper method to define mock.On call -// - ctx context.Context -// - v interface{} -func (_e *mockDynamoClient_Expecter) Put(ctx interface{}, v interface{}) *mockDynamoClient_Put_Call { - return &mockDynamoClient_Put_Call{Call: _e.mock.On("Put", ctx, v)} -} - -func (_c *mockDynamoClient_Put_Call) Run(run func(ctx context.Context, v interface{})) *mockDynamoClient_Put_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(interface{})) - }) - return _c -} - -func (_c *mockDynamoClient_Put_Call) Return(_a0 error) *mockDynamoClient_Put_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *mockDynamoClient_Put_Call) RunAndReturn(run func(context.Context, interface{}) error) *mockDynamoClient_Put_Call { - _c.Call.Return(run) - return _c -} - // newMockDynamoClient creates a new instance of mockDynamoClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func newMockDynamoClient(t interface { diff --git a/internal/scheduled/store.go b/internal/scheduled/store.go index 4857fe2ae4..126992167b 100644 --- a/internal/scheduled/store.go +++ b/internal/scheduled/store.go @@ -13,7 +13,7 @@ type DynamoClient interface { AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error - Put(ctx context.Context, v interface{}) error + Create(ctx context.Context, v interface{}) error } type Store struct { @@ -44,12 +44,12 @@ func (s *Store) Pop(ctx context.Context, day time.Time) (*Event, error) { return &row, nil } -func (s *Store) Put(ctx context.Context, row Event) error { +func (s *Store) Create(ctx context.Context, row Event) error { row.PK = dynamo.ScheduledDayKey(row.At) row.SK = dynamo.ScheduledKey(row.At, int(row.Action)) row.CreatedAt = s.now() - return s.dynamoClient.Put(ctx, row) + return s.dynamoClient.Create(ctx, row) } func (s *Store) DeleteAllByUID(ctx context.Context, uid string) error { diff --git a/internal/scheduled/store_test.go b/internal/scheduled/store_test.go index ff86b3263c..9866b22112 100644 --- a/internal/scheduled/store_test.go +++ b/internal/scheduled/store_test.go @@ -78,12 +78,12 @@ func TestStorePopWhenDeleteOneErrors(t *testing.T) { assert.Equal(t, expectedError, err) } -func TestStorePut(t *testing.T) { +func TestStoreCreate(t *testing.T) { at := time.Date(2024, time.January, 1, 12, 13, 14, 5, time.UTC) dynamoClient := newMockDynamoClient(t) dynamoClient.EXPECT(). - Put(ctx, Event{ + Create(ctx, Event{ PK: dynamo.ScheduledDayKey(at), SK: dynamo.ScheduledKey(at, 99), CreatedAt: testNow, @@ -93,7 +93,7 @@ func TestStorePut(t *testing.T) { Return(expectedError) store := &Store{dynamoClient: dynamoClient, now: testNowFn} - err := store.Put(ctx, Event{At: at, Action: 99}) + err := store.Create(ctx, Event{At: at, Action: 99}) assert.Equal(t, expectedError, err) } From 8c609007414ee6732a5e76a48f1b7367d917accb Mon Sep 17 00:00:00 2001 From: Joshua Hawxwell Date: Wed, 27 Nov 2024 08:13:29 +0000 Subject: [PATCH 13/14] Use DeleteKeys instead of DeleteManyByUID --- cmd/event-received/main.go | 2 +- .../mock_dynamodbClient_test.go | 31 ++++++----- internal/app/app.go | 1 - internal/app/mock_DynamoClient_test.go | 48 ----------------- internal/dynamo/client.go | 26 --------- internal/dynamo/client_test.go | 54 ------------------- internal/scheduled/mock_DynamoClient_test.go | 31 ++++++----- internal/scheduled/store.go | 4 +- internal/scheduled/store_test.go | 8 +-- 9 files changed, 37 insertions(+), 168 deletions(-) diff --git a/cmd/event-received/main.go b/cmd/event-received/main.go index a390d6cb68..fdb3286e2d 100644 --- a/cmd/event-received/main.go +++ b/cmd/event-received/main.go @@ -82,7 +82,7 @@ type dynamodbClient interface { Create(ctx context.Context, v interface{}) error CreateOnly(ctx context.Context, v interface{}) error DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error - DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error + DeleteKeys(ctx context.Context, keys []dynamo.Keys) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error One(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) error OneByUID(ctx context.Context, uid string, v interface{}) error diff --git a/cmd/event-received/mock_dynamodbClient_test.go b/cmd/event-received/mock_dynamodbClient_test.go index 2d2fe5893c..c8eb8d6ee2 100644 --- a/cmd/event-received/mock_dynamodbClient_test.go +++ b/cmd/event-received/mock_dynamodbClient_test.go @@ -213,17 +213,17 @@ func (_c *mockDynamodbClient_CreateOnly_Call) RunAndReturn(run func(context.Cont return _c } -// DeleteManyByUID provides a mock function with given fields: ctx, keys, uid -func (_m *mockDynamodbClient) DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error { - ret := _m.Called(ctx, keys, uid) +// DeleteKeys provides a mock function with given fields: ctx, keys +func (_m *mockDynamodbClient) DeleteKeys(ctx context.Context, keys []dynamo.Keys) error { + ret := _m.Called(ctx, keys) if len(ret) == 0 { - panic("no return value specified for DeleteManyByUID") + panic("no return value specified for DeleteKeys") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys, string) error); ok { - r0 = rf(ctx, keys, uid) + if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys) error); ok { + r0 = rf(ctx, keys) } else { r0 = ret.Error(0) } @@ -231,32 +231,31 @@ func (_m *mockDynamodbClient) DeleteManyByUID(ctx context.Context, keys []dynamo return r0 } -// mockDynamodbClient_DeleteManyByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteManyByUID' -type mockDynamodbClient_DeleteManyByUID_Call struct { +// mockDynamodbClient_DeleteKeys_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteKeys' +type mockDynamodbClient_DeleteKeys_Call struct { *mock.Call } -// DeleteManyByUID is a helper method to define mock.On call +// DeleteKeys is a helper method to define mock.On call // - ctx context.Context // - keys []dynamo.Keys -// - uid string -func (_e *mockDynamodbClient_Expecter) DeleteManyByUID(ctx interface{}, keys interface{}, uid interface{}) *mockDynamodbClient_DeleteManyByUID_Call { - return &mockDynamodbClient_DeleteManyByUID_Call{Call: _e.mock.On("DeleteManyByUID", ctx, keys, uid)} +func (_e *mockDynamodbClient_Expecter) DeleteKeys(ctx interface{}, keys interface{}) *mockDynamodbClient_DeleteKeys_Call { + return &mockDynamodbClient_DeleteKeys_Call{Call: _e.mock.On("DeleteKeys", ctx, keys)} } -func (_c *mockDynamodbClient_DeleteManyByUID_Call) Run(run func(ctx context.Context, keys []dynamo.Keys, uid string)) *mockDynamodbClient_DeleteManyByUID_Call { +func (_c *mockDynamodbClient_DeleteKeys_Call) Run(run func(ctx context.Context, keys []dynamo.Keys)) *mockDynamodbClient_DeleteKeys_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].([]dynamo.Keys), args[2].(string)) + run(args[0].(context.Context), args[1].([]dynamo.Keys)) }) return _c } -func (_c *mockDynamodbClient_DeleteManyByUID_Call) Return(_a0 error) *mockDynamodbClient_DeleteManyByUID_Call { +func (_c *mockDynamodbClient_DeleteKeys_Call) Return(_a0 error) *mockDynamodbClient_DeleteKeys_Call { _c.Call.Return(_a0) return _c } -func (_c *mockDynamodbClient_DeleteManyByUID_Call) RunAndReturn(run func(context.Context, []dynamo.Keys, string) error) *mockDynamodbClient_DeleteManyByUID_Call { +func (_c *mockDynamodbClient_DeleteKeys_Call) RunAndReturn(run func(context.Context, []dynamo.Keys) error) *mockDynamodbClient_DeleteKeys_Call { _c.Call.Return(run) return _c } diff --git a/internal/app/app.go b/internal/app/app.go index f6deb782fa..4bca9499ac 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -60,7 +60,6 @@ type DynamoClient interface { CreateOnly(ctx context.Context, v interface{}) error DeleteKeys(ctx context.Context, keys []dynamo.Keys) error DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error - DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error LatestForActor(ctx context.Context, sk dynamo.SK, v interface{}) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error One(ctx context.Context, pk dynamo.PK, sk dynamo.SK, v interface{}) error diff --git a/internal/app/mock_DynamoClient_test.go b/internal/app/mock_DynamoClient_test.go index 7a3030d4d9..c5fb6cd6fd 100644 --- a/internal/app/mock_DynamoClient_test.go +++ b/internal/app/mock_DynamoClient_test.go @@ -524,54 +524,6 @@ func (_c *mockDynamoClient_DeleteKeys_Call) RunAndReturn(run func(context.Contex return _c } -// DeleteManyByUID provides a mock function with given fields: ctx, keys, uid -func (_m *mockDynamoClient) DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error { - ret := _m.Called(ctx, keys, uid) - - if len(ret) == 0 { - panic("no return value specified for DeleteManyByUID") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys, string) error); ok { - r0 = rf(ctx, keys, uid) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// mockDynamoClient_DeleteManyByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteManyByUID' -type mockDynamoClient_DeleteManyByUID_Call struct { - *mock.Call -} - -// DeleteManyByUID is a helper method to define mock.On call -// - ctx context.Context -// - keys []dynamo.Keys -// - uid string -func (_e *mockDynamoClient_Expecter) DeleteManyByUID(ctx interface{}, keys interface{}, uid interface{}) *mockDynamoClient_DeleteManyByUID_Call { - return &mockDynamoClient_DeleteManyByUID_Call{Call: _e.mock.On("DeleteManyByUID", ctx, keys, uid)} -} - -func (_c *mockDynamoClient_DeleteManyByUID_Call) Run(run func(ctx context.Context, keys []dynamo.Keys, uid string)) *mockDynamoClient_DeleteManyByUID_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].([]dynamo.Keys), args[2].(string)) - }) - return _c -} - -func (_c *mockDynamoClient_DeleteManyByUID_Call) Return(_a0 error) *mockDynamoClient_DeleteManyByUID_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *mockDynamoClient_DeleteManyByUID_Call) RunAndReturn(run func(context.Context, []dynamo.Keys, string) error) *mockDynamoClient_DeleteManyByUID_Call { - _c.Call.Return(run) - return _c -} - // DeleteOne provides a mock function with given fields: ctx, pk, sk func (_m *mockDynamoClient) DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error { ret := _m.Called(ctx, pk, sk) diff --git a/internal/dynamo/client.go b/internal/dynamo/client.go index 050f5fcbf7..3cf2afba0a 100644 --- a/internal/dynamo/client.go +++ b/internal/dynamo/client.go @@ -399,32 +399,6 @@ func (c *Client) DeleteOne(ctx context.Context, pk PK, sk SK) error { return err } -func (c *Client) DeleteManyByUID(ctx context.Context, keys []Keys, uid string) error { - items := make([]types.TransactWriteItem, len(keys)) - - for i, key := range keys { - items[i] = types.TransactWriteItem{ - Delete: &types.Delete{ - TableName: aws.String(c.table), - Key: map[string]types.AttributeValue{ - "PK": &types.AttributeValueMemberS{Value: key.PK.PK()}, - "SK": &types.AttributeValueMemberS{Value: key.SK.SK()}, - }, - ConditionExpression: aws.String("LpaUID = :uid"), - ExpressionAttributeValues: map[string]types.AttributeValue{ - ":uid": &types.AttributeValueMemberS{Value: uid}, - }, - }, - } - } - - _, err := c.svc.TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{ - TransactItems: items, - }) - - return err -} - func (c *Client) Update(ctx context.Context, pk PK, sk SK, values map[string]types.AttributeValue, expression string) error { _, err := c.svc.UpdateItem(ctx, &dynamodb.UpdateItemInput{ TableName: aws.String(c.table), diff --git a/internal/dynamo/client_test.go b/internal/dynamo/client_test.go index 430e525935..088c04ab40 100644 --- a/internal/dynamo/client_test.go +++ b/internal/dynamo/client_test.go @@ -1029,57 +1029,3 @@ func TestAllScheduledEventsByUIDWhenUnmarshalError(t *testing.T) { err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", v) assert.Error(t, err) } - -func TestDeleteManyByUID(t *testing.T) { - items := []types.TransactWriteItem{ - {Delete: &types.Delete{ - TableName: aws.String("this"), - Key: map[string]types.AttributeValue{ - "PK": &types.AttributeValueMemberS{Value: "a-pk1"}, - "SK": &types.AttributeValueMemberS{Value: "a-sk1"}, - }, - ConditionExpression: aws.String("LpaUID = :uid"), - ExpressionAttributeValues: map[string]types.AttributeValue{ - ":uid": &types.AttributeValueMemberS{Value: "lpa-uid"}, - }, - }}, - {Delete: &types.Delete{ - TableName: aws.String("this"), - Key: map[string]types.AttributeValue{ - "PK": &types.AttributeValueMemberS{Value: "a-pk2"}, - "SK": &types.AttributeValueMemberS{Value: "a-sk2"}, - }, - ConditionExpression: aws.String("LpaUID = :uid"), - ExpressionAttributeValues: map[string]types.AttributeValue{ - ":uid": &types.AttributeValueMemberS{Value: "lpa-uid"}, - }, - }}, - } - - dynamoDB := newMockDynamoDB(t) - dynamoDB.EXPECT(). - TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{ - TransactItems: items, - }). - Return(&dynamodb.TransactWriteItemsOutput{}, nil) - - c := &Client{table: "this", svc: dynamoDB} - err := c.DeleteManyByUID(ctx, []Keys{ - {PK: testPK("a-pk1"), SK: testSK("a-sk1")}, - {PK: testPK("a-pk2"), SK: testSK("a-sk2")}, - }, "lpa-uid") - - assert.Nil(t, err) -} - -func TestDeleteManyByUIDWhenWriteItemsError(t *testing.T) { - dynamoDB := newMockDynamoDB(t) - dynamoDB.EXPECT(). - TransactWriteItems(ctx, mock.Anything). - Return(&dynamodb.TransactWriteItemsOutput{}, expectedError) - - c := &Client{table: "this", svc: dynamoDB} - err := c.DeleteManyByUID(ctx, []Keys{}, "lpa-uid") - - assert.Equal(t, expectedError, err) -} diff --git a/internal/scheduled/mock_DynamoClient_test.go b/internal/scheduled/mock_DynamoClient_test.go index 28b2e57155..d752914fb4 100644 --- a/internal/scheduled/mock_DynamoClient_test.go +++ b/internal/scheduled/mock_DynamoClient_test.go @@ -166,17 +166,17 @@ func (_c *mockDynamoClient_Create_Call) RunAndReturn(run func(context.Context, i return _c } -// DeleteManyByUID provides a mock function with given fields: ctx, keys, uid -func (_m *mockDynamoClient) DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error { - ret := _m.Called(ctx, keys, uid) +// DeleteKeys provides a mock function with given fields: ctx, keys +func (_m *mockDynamoClient) DeleteKeys(ctx context.Context, keys []dynamo.Keys) error { + ret := _m.Called(ctx, keys) if len(ret) == 0 { - panic("no return value specified for DeleteManyByUID") + panic("no return value specified for DeleteKeys") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys, string) error); ok { - r0 = rf(ctx, keys, uid) + if rf, ok := ret.Get(0).(func(context.Context, []dynamo.Keys) error); ok { + r0 = rf(ctx, keys) } else { r0 = ret.Error(0) } @@ -184,32 +184,31 @@ func (_m *mockDynamoClient) DeleteManyByUID(ctx context.Context, keys []dynamo.K return r0 } -// mockDynamoClient_DeleteManyByUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteManyByUID' -type mockDynamoClient_DeleteManyByUID_Call struct { +// mockDynamoClient_DeleteKeys_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteKeys' +type mockDynamoClient_DeleteKeys_Call struct { *mock.Call } -// DeleteManyByUID is a helper method to define mock.On call +// DeleteKeys is a helper method to define mock.On call // - ctx context.Context // - keys []dynamo.Keys -// - uid string -func (_e *mockDynamoClient_Expecter) DeleteManyByUID(ctx interface{}, keys interface{}, uid interface{}) *mockDynamoClient_DeleteManyByUID_Call { - return &mockDynamoClient_DeleteManyByUID_Call{Call: _e.mock.On("DeleteManyByUID", ctx, keys, uid)} +func (_e *mockDynamoClient_Expecter) DeleteKeys(ctx interface{}, keys interface{}) *mockDynamoClient_DeleteKeys_Call { + return &mockDynamoClient_DeleteKeys_Call{Call: _e.mock.On("DeleteKeys", ctx, keys)} } -func (_c *mockDynamoClient_DeleteManyByUID_Call) Run(run func(ctx context.Context, keys []dynamo.Keys, uid string)) *mockDynamoClient_DeleteManyByUID_Call { +func (_c *mockDynamoClient_DeleteKeys_Call) Run(run func(ctx context.Context, keys []dynamo.Keys)) *mockDynamoClient_DeleteKeys_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].([]dynamo.Keys), args[2].(string)) + run(args[0].(context.Context), args[1].([]dynamo.Keys)) }) return _c } -func (_c *mockDynamoClient_DeleteManyByUID_Call) Return(_a0 error) *mockDynamoClient_DeleteManyByUID_Call { +func (_c *mockDynamoClient_DeleteKeys_Call) Return(_a0 error) *mockDynamoClient_DeleteKeys_Call { _c.Call.Return(_a0) return _c } -func (_c *mockDynamoClient_DeleteManyByUID_Call) RunAndReturn(run func(context.Context, []dynamo.Keys, string) error) *mockDynamoClient_DeleteManyByUID_Call { +func (_c *mockDynamoClient_DeleteKeys_Call) RunAndReturn(run func(context.Context, []dynamo.Keys) error) *mockDynamoClient_DeleteKeys_Call { _c.Call.Return(run) return _c } diff --git a/internal/scheduled/store.go b/internal/scheduled/store.go index 126992167b..0a582d388f 100644 --- a/internal/scheduled/store.go +++ b/internal/scheduled/store.go @@ -11,8 +11,8 @@ import ( type DynamoClient interface { AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error - DeleteManyByUID(ctx context.Context, keys []dynamo.Keys, uid string) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error + DeleteKeys(ctx context.Context, keys []dynamo.Keys) error Create(ctx context.Context, v interface{}) error } @@ -68,5 +68,5 @@ func (s *Store) DeleteAllByUID(ctx context.Context, uid string) error { keys = append(keys, dynamo.Keys{PK: e.PK, SK: e.SK}) } - return s.dynamoClient.DeleteManyByUID(ctx, keys, uid) + return s.dynamoClient.DeleteKeys(ctx, keys) } diff --git a/internal/scheduled/store_test.go b/internal/scheduled/store_test.go index 9866b22112..45e2ad5100 100644 --- a/internal/scheduled/store_test.go +++ b/internal/scheduled/store_test.go @@ -110,10 +110,10 @@ func TestDeleteAllByUID(t *testing.T) { {LpaUID: "lpa-uid", PK: dynamo.ScheduledDayKey(yesterday), SK: dynamo.ScheduledKey(yesterday, 99)}, }) dynamoClient.EXPECT(). - DeleteManyByUID(ctx, []dynamo.Keys{ + DeleteKeys(ctx, []dynamo.Keys{ {PK: dynamo.ScheduledDayKey(now), SK: dynamo.ScheduledKey(now, 98)}, {PK: dynamo.ScheduledDayKey(yesterday), SK: dynamo.ScheduledKey(yesterday, 99)}, - }, "lpa-uid"). + }). Return(nil) store := &Store{dynamoClient: dynamoClient, now: testNowFn} @@ -147,14 +147,14 @@ func TestDeleteAllByUIDWhenNoEventsFound(t *testing.T) { assert.ErrorContains(t, err, "no scheduled events found for UID lpa-uid") } -func TestDeleteAllByUIDWhenDeleteManyByUIDErrors(t *testing.T) { +func TestDeleteAllByUIDWhenDeleteKeysErrors(t *testing.T) { dynamoClient := newMockDynamoClient(t) dynamoClient.EXPECT(). AllByLpaUIDAndPartialSK(ctx, mock.Anything, mock.Anything, mock.Anything). Return(nil). SetData([]Event{{LpaUID: "lpa-uid"}}) dynamoClient.EXPECT(). - DeleteManyByUID(mock.Anything, mock.Anything, mock.Anything). + DeleteKeys(mock.Anything, mock.Anything). Return(expectedError) store := &Store{dynamoClient: dynamoClient, now: testNowFn} From 5675ae5bf92dfc4d9a2cdaec8d541e975a78f9a9 Mon Sep 17 00:00:00 2001 From: Joshua Hawxwell Date: Wed, 27 Nov 2024 08:20:08 +0000 Subject: [PATCH 14/14] Pass a dynamo.SK to PartialSK method --- cmd/event-received/main.go | 2 +- cmd/event-received/mock_dynamodbClient_test.go | 12 ++++++------ internal/app/app.go | 2 +- internal/app/mock_DynamoClient_test.go | 12 ++++++------ internal/dynamo/client.go | 4 ++-- internal/dynamo/client_test.go | 10 +++++----- internal/dynamo/keys.go | 4 ++-- internal/dynamo/keys_test.go | 1 + internal/scheduled/mock_DynamoClient_test.go | 12 ++++++------ internal/scheduled/mock_test.go | 2 +- internal/scheduled/store.go | 4 ++-- internal/scheduled/store_test.go | 2 +- 12 files changed, 34 insertions(+), 33 deletions(-) diff --git a/cmd/event-received/main.go b/cmd/event-received/main.go index fdb3286e2d..294be97bbb 100644 --- a/cmd/event-received/main.go +++ b/cmd/event-received/main.go @@ -78,7 +78,7 @@ type uidEvent struct { type dynamodbClient interface { AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error - AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK dynamo.SK, v interface{}) error Create(ctx context.Context, v interface{}) error CreateOnly(ctx context.Context, v interface{}) error DeleteOne(ctx context.Context, pk dynamo.PK, sk dynamo.SK) error diff --git a/cmd/event-received/mock_dynamodbClient_test.go b/cmd/event-received/mock_dynamodbClient_test.go index c8eb8d6ee2..ca9820e532 100644 --- a/cmd/event-received/mock_dynamodbClient_test.go +++ b/cmd/event-received/mock_dynamodbClient_test.go @@ -23,7 +23,7 @@ func (_m *mockDynamodbClient) EXPECT() *mockDynamodbClient_Expecter { } // AllByLpaUIDAndPartialSK provides a mock function with given fields: ctx, uid, partialSK, v -func (_m *mockDynamodbClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { +func (_m *mockDynamodbClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK dynamo.SK, v interface{}) error { ret := _m.Called(ctx, uid, partialSK, v) if len(ret) == 0 { @@ -31,7 +31,7 @@ func (_m *mockDynamodbClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid s } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, interface{}) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, dynamo.SK, interface{}) error); ok { r0 = rf(ctx, uid, partialSK, v) } else { r0 = ret.Error(0) @@ -48,15 +48,15 @@ type mockDynamodbClient_AllByLpaUIDAndPartialSK_Call struct { // AllByLpaUIDAndPartialSK is a helper method to define mock.On call // - ctx context.Context // - uid string -// - partialSK string +// - partialSK dynamo.SK // - v interface{} func (_e *mockDynamodbClient_Expecter) AllByLpaUIDAndPartialSK(ctx interface{}, uid interface{}, partialSK interface{}, v interface{}) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { return &mockDynamodbClient_AllByLpaUIDAndPartialSK_Call{Call: _e.mock.On("AllByLpaUIDAndPartialSK", ctx, uid, partialSK, v)} } -func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK string, v interface{})) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { +func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK dynamo.SK, v interface{})) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(interface{})) + run(args[0].(context.Context), args[1].(string), args[2].(dynamo.SK), args[3].(interface{})) }) return _c } @@ -66,7 +66,7 @@ func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) Return(_a0 error) *mo return _c } -func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, string, interface{}) error) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { +func (_c *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, dynamo.SK, interface{}) error) *mockDynamodbClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Return(run) return _c } diff --git a/internal/app/app.go b/internal/app/app.go index 4bca9499ac..25acfe8c77 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -52,7 +52,7 @@ type DynamoClient interface { AllByKeys(ctx context.Context, keys []dynamo.Keys) ([]map[string]dynamodbtypes.AttributeValue, error) AllByPartialSK(ctx context.Context, pk dynamo.PK, partialSK dynamo.SK, v interface{}) error AllBySK(ctx context.Context, sk dynamo.SK, v interface{}) error - AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK dynamo.SK, v interface{}) error AllKeysByPK(ctx context.Context, pk dynamo.PK) ([]dynamo.Keys, error) AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error BatchPut(ctx context.Context, items []interface{}) error diff --git a/internal/app/mock_DynamoClient_test.go b/internal/app/mock_DynamoClient_test.go index c5fb6cd6fd..2aa5651009 100644 --- a/internal/app/mock_DynamoClient_test.go +++ b/internal/app/mock_DynamoClient_test.go @@ -84,7 +84,7 @@ func (_c *mockDynamoClient_AllByKeys_Call) RunAndReturn(run func(context.Context } // AllByLpaUIDAndPartialSK provides a mock function with given fields: ctx, uid, partialSK, v -func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { +func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK dynamo.SK, v interface{}) error { ret := _m.Called(ctx, uid, partialSK, v) if len(ret) == 0 { @@ -92,7 +92,7 @@ func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid str } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, interface{}) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, dynamo.SK, interface{}) error); ok { r0 = rf(ctx, uid, partialSK, v) } else { r0 = ret.Error(0) @@ -109,15 +109,15 @@ type mockDynamoClient_AllByLpaUIDAndPartialSK_Call struct { // AllByLpaUIDAndPartialSK is a helper method to define mock.On call // - ctx context.Context // - uid string -// - partialSK string +// - partialSK dynamo.SK // - v interface{} func (_e *mockDynamoClient_Expecter) AllByLpaUIDAndPartialSK(ctx interface{}, uid interface{}, partialSK interface{}, v interface{}) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { return &mockDynamoClient_AllByLpaUIDAndPartialSK_Call{Call: _e.mock.On("AllByLpaUIDAndPartialSK", ctx, uid, partialSK, v)} } -func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK string, v interface{})) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK dynamo.SK, v interface{})) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(interface{})) + run(args[0].(context.Context), args[1].(string), args[2].(dynamo.SK), args[3].(interface{})) }) return _c } @@ -127,7 +127,7 @@ func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Return(_a0 error) *mock return _c } -func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, string, interface{}) error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, dynamo.SK, interface{}) error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Return(run) return _c } diff --git a/internal/dynamo/client.go b/internal/dynamo/client.go index 3cf2afba0a..3a7e42f328 100644 --- a/internal/dynamo/client.go +++ b/internal/dynamo/client.go @@ -86,7 +86,7 @@ func (c *Client) OneByUID(ctx context.Context, uid string, v interface{}) error return attributevalue.UnmarshalMap(response.Items[0], v) } -func (c *Client) AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error { +func (c *Client) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK SK, v interface{}) error { response, err := c.svc.Query(ctx, &dynamodb.QueryInput{ TableName: aws.String(c.table), IndexName: aws.String(lpaUIDIndex), @@ -96,7 +96,7 @@ func (c *Client) AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK str }, ExpressionAttributeValues: map[string]types.AttributeValue{ ":LpaUID": &types.AttributeValueMemberS{Value: uid}, - ":SK": &types.AttributeValueMemberS{Value: partialSK}, + ":SK": &types.AttributeValueMemberS{Value: partialSK.SK()}, }, KeyConditionExpression: aws.String("#LpaUID = :LpaUID"), FilterExpression: aws.String("begins_with(#SK, :SK)"), diff --git a/internal/dynamo/client_test.go b/internal/dynamo/client_test.go index 088c04ab40..df6c5872f4 100644 --- a/internal/dynamo/client_test.go +++ b/internal/dynamo/client_test.go @@ -974,7 +974,7 @@ func TestAllScheduledEventsByUID(t *testing.T) { }, ExpressionAttributeValues: map[string]types.AttributeValue{ ":LpaUID": &types.AttributeValueMemberS{Value: "lpa-uid"}, - ":SK": &types.AttributeValueMemberS{Value: "partial-sk"}, + ":SK": &types.AttributeValueMemberS{Value: "a-partial-sk"}, }, KeyConditionExpression: aws.String("#LpaUID = :LpaUID"), FilterExpression: aws.String("begins_with(#SK, :SK)"), @@ -984,7 +984,7 @@ func TestAllScheduledEventsByUID(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", testSK("a-partial-sk"), &v) assert.Nil(t, err) } @@ -997,7 +997,7 @@ func TestAllScheduledEventsByUIDWhenQueryError(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", testSK("a-partial-sk"), &v) assert.Equal(t, fmt.Errorf("failed to query scheduled event by UID: %w", expectedError), err) } @@ -1010,7 +1010,7 @@ func TestAllScheduledEventsByUIDWhenNoResults(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", &v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", testSK("a-partial-sk"), &v) assert.Equal(t, NotFoundError{}, err) } @@ -1026,6 +1026,6 @@ func TestAllScheduledEventsByUIDWhenUnmarshalError(t *testing.T) { c := &Client{table: "this", svc: dynamoDB} var v []map[string]string - err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", "partial-sk", v) + err := c.AllByLpaUIDAndPartialSK(ctx, "lpa-uid", testSK("a-partial-sk"), v) assert.Error(t, err) } diff --git a/internal/dynamo/keys.go b/internal/dynamo/keys.go index 951c3cd8cc..cec455a581 100644 --- a/internal/dynamo/keys.go +++ b/internal/dynamo/keys.go @@ -322,8 +322,8 @@ func ScheduledKey(at time.Time, action int) ScheduledKeyType { return ScheduledKeyType(scheduledPrefix + "#" + at.Format(time.RFC3339) + "#" + strconv.Itoa(action)) } -func PartialScheduleKey() string { - return scheduledPrefix + "#" +func PartialScheduledKey() ScheduledKeyType { + return ScheduledKeyType(scheduledPrefix + "#") } type ReservedKeyType string diff --git a/internal/dynamo/keys_test.go b/internal/dynamo/keys_test.go index 59a9198633..2a0b540c40 100644 --- a/internal/dynamo/keys_test.go +++ b/internal/dynamo/keys_test.go @@ -87,6 +87,7 @@ func TestSK(t *testing.T) { "VoucherKey": {VoucherKey("S"), "VOUCHER#S"}, "ScheduledKey": {ScheduledKey(time.Date(2024, time.January, 2, 12, 13, 14, 15, time.UTC), 99), "SCHEDULED#2024-01-02T12:13:14Z#99"}, "ReservedKey": {ReservedKey(VoucherKey), "RESERVED#VOUCHER#"}, + "PartialScheduledKey": {PartialScheduledKey(), "SCHEDULED#"}, } for name, tc := range testcases { diff --git a/internal/scheduled/mock_DynamoClient_test.go b/internal/scheduled/mock_DynamoClient_test.go index d752914fb4..68b55cee05 100644 --- a/internal/scheduled/mock_DynamoClient_test.go +++ b/internal/scheduled/mock_DynamoClient_test.go @@ -23,7 +23,7 @@ func (_m *mockDynamoClient) EXPECT() *mockDynamoClient_Expecter { } // AllByLpaUIDAndPartialSK provides a mock function with given fields: ctx, uid, partialSK, v -func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK string, v interface{}) error { +func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK dynamo.SK, v interface{}) error { ret := _m.Called(ctx, uid, partialSK, v) if len(ret) == 0 { @@ -31,7 +31,7 @@ func (_m *mockDynamoClient) AllByLpaUIDAndPartialSK(ctx context.Context, uid str } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, interface{}) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, dynamo.SK, interface{}) error); ok { r0 = rf(ctx, uid, partialSK, v) } else { r0 = ret.Error(0) @@ -48,15 +48,15 @@ type mockDynamoClient_AllByLpaUIDAndPartialSK_Call struct { // AllByLpaUIDAndPartialSK is a helper method to define mock.On call // - ctx context.Context // - uid string -// - partialSK string +// - partialSK dynamo.SK // - v interface{} func (_e *mockDynamoClient_Expecter) AllByLpaUIDAndPartialSK(ctx interface{}, uid interface{}, partialSK interface{}, v interface{}) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { return &mockDynamoClient_AllByLpaUIDAndPartialSK_Call{Call: _e.mock.On("AllByLpaUIDAndPartialSK", ctx, uid, partialSK, v)} } -func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK string, v interface{})) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Run(run func(ctx context.Context, uid string, partialSK dynamo.SK, v interface{})) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(interface{})) + run(args[0].(context.Context), args[1].(string), args[2].(dynamo.SK), args[3].(interface{})) }) return _c } @@ -66,7 +66,7 @@ func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) Return(_a0 error) *mock return _c } -func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, string, interface{}) error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { +func (_c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) RunAndReturn(run func(context.Context, string, dynamo.SK, interface{}) error) *mockDynamoClient_AllByLpaUIDAndPartialSK_Call { _c.Call.Return(run) return _c } diff --git a/internal/scheduled/mock_test.go b/internal/scheduled/mock_test.go index 26c86cf83a..3c8be16c97 100644 --- a/internal/scheduled/mock_test.go +++ b/internal/scheduled/mock_test.go @@ -8,7 +8,7 @@ import ( ) func (c *mockDynamoClient_AllByLpaUIDAndPartialSK_Call) SetData(data any) { - c.Run(func(_ context.Context, _, _ string, v any) { + c.Run(func(_ context.Context, _ string, _ dynamo.SK, v any) { b, _ := attributevalue.Marshal(data) attributevalue.Unmarshal(b, v) }) diff --git a/internal/scheduled/store.go b/internal/scheduled/store.go index 0a582d388f..0df050fa3c 100644 --- a/internal/scheduled/store.go +++ b/internal/scheduled/store.go @@ -9,7 +9,7 @@ import ( ) type DynamoClient interface { - AllByLpaUIDAndPartialSK(ctx context.Context, uid, partialSK string, v interface{}) error + AllByLpaUIDAndPartialSK(ctx context.Context, uid string, partialSK dynamo.SK, v interface{}) error AnyByPK(ctx context.Context, pk dynamo.PK, v interface{}) error Move(ctx context.Context, oldKeys dynamo.Keys, value any) error DeleteKeys(ctx context.Context, keys []dynamo.Keys) error @@ -55,7 +55,7 @@ func (s *Store) Create(ctx context.Context, row Event) error { func (s *Store) DeleteAllByUID(ctx context.Context, uid string) error { var events []Event - if err := s.dynamoClient.AllByLpaUIDAndPartialSK(ctx, uid, dynamo.PartialScheduleKey(), &events); err != nil { + if err := s.dynamoClient.AllByLpaUIDAndPartialSK(ctx, uid, dynamo.PartialScheduledKey(), &events); err != nil { return err } diff --git a/internal/scheduled/store_test.go b/internal/scheduled/store_test.go index 45e2ad5100..600f2c6f8f 100644 --- a/internal/scheduled/store_test.go +++ b/internal/scheduled/store_test.go @@ -103,7 +103,7 @@ func TestDeleteAllByUID(t *testing.T) { dynamoClient := newMockDynamoClient(t) dynamoClient.EXPECT(). - AllByLpaUIDAndPartialSK(ctx, "lpa-uid", dynamo.PartialScheduleKey(), mock.Anything). + AllByLpaUIDAndPartialSK(ctx, "lpa-uid", dynamo.PartialScheduledKey(), mock.Anything). Return(nil). SetData([]Event{ {LpaUID: "lpa-uid", PK: dynamo.ScheduledDayKey(now), SK: dynamo.ScheduledKey(now, 98)},