From 725f05b34d42b447bdce00dec66f469bb5f20cb4 Mon Sep 17 00:00:00 2001 From: Joshua Hawxwell Date: Thu, 9 Nov 2023 10:52:22 +0000 Subject: [PATCH] MLPAB-1582 Send reduced-fee-requested correctly (#827) This moves where document stuff is handled to be only happening in UploadEvidence or things it calls directly. It prevents the problem of Sent being set, then not sending the reduced-fee-requested event as no documents need sending. --- cmd/event-received/main.go | 2 +- internal/app/app.go | 8 +- internal/app/app_test.go | 4 +- internal/app/document_store.go | 92 +++--- internal/app/document_store_test.go | 201 +++++++----- internal/app/donor_store.go | 41 --- internal/app/donor_store_test.go | 204 ------------ internal/app/mock_DocumentStore_test.go | 14 - internal/app/mock_DynamoClient_test.go | 20 +- internal/app/mock_S3Client_test.go | 5 +- internal/dynamo/client.go | 54 +-- internal/dynamo/client_test.go | 124 ++----- .../page/donor/mock_DocumentStore_test.go | 14 + internal/page/donor/mock_EventClient_test.go | 11 +- internal/page/donor/mock_S3Client_test.go | 15 - internal/page/donor/payment_confirmation.go | 28 +- .../page/donor/payment_confirmation_test.go | 157 +-------- internal/page/donor/register.go | 68 ++-- internal/page/donor/register_test.go | 183 +---------- .../donor/send_us_your_evidence_by_post.go | 11 +- .../send_us_your_evidence_by_post_test.go | 40 ++- internal/page/donor/upload_evidence.go | 12 +- internal/page/donor/upload_evidence_test.go | 307 +++++++++--------- internal/s3/client.go | 27 +- internal/s3/client_test.go | 75 ++--- 25 files changed, 542 insertions(+), 1175 deletions(-) diff --git a/cmd/event-received/main.go b/cmd/event-received/main.go index 20aa771aff..974f895d11 100644 --- a/cmd/event-received/main.go +++ b/cmd/event-received/main.go @@ -116,7 +116,7 @@ func handler(ctx context.Context, event Event) error { if event.isS3Event() { s3Client := s3.NewClient(cfg, evidenceBucketName) - documentStore := app.NewDocumentStore(dynamoClient, s3Client, random.UuidString) + documentStore := app.NewDocumentStore(dynamoClient, nil, nil, nil, nil) if err := handleObjectTagsAdded(ctx, dynamoClient, event.S3Event, s3Client, documentStore); err != nil { return fmt.Errorf("ObjectTagging:Put: %w", err) diff --git a/internal/app/app.go b/internal/app/app.go index 554639b31f..f35ad28430 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -9,7 +9,6 @@ import ( "time" dynamodbtypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" - "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/google/uuid" "github.com/gorilla/sessions" "github.com/ministryofjustice/opg-go-common/logging" @@ -50,15 +49,15 @@ type DynamoClient interface { DeleteKeys(ctx context.Context, keys []dynamo.Key) error DeleteOne(ctx context.Context, pk, sk string) error Update(ctx context.Context, pk, sk string, values map[string]dynamodbtypes.AttributeValue, expression string) error - BatchPut(ctx context.Context, items []interface{}) (int, error) + BatchPut(ctx context.Context, items []interface{}) error } //go:generate mockery --testonly --inpackage --name S3Client --structname mockS3Client type S3Client interface { PutObject(context.Context, string, []byte) error - PutObjectTagging(context.Context, string, []types.Tag) error DeleteObject(context.Context, string) error DeleteObjects(ctx context.Context, keys []string) error + PutObjectTagging(context.Context, string, map[string]string) error } //go:generate mockery --testonly --inpackage --name SessionStore --structname mockSessionStore @@ -87,7 +86,7 @@ func App( s3Client S3Client, eventClient *event.Client, ) http.Handler { - documentStore := NewDocumentStore(lpaDynamoClient, s3Client, random.UuidString) + documentStore := NewDocumentStore(lpaDynamoClient, s3Client, eventClient, random.UuidString, time.Now) donorStore := &donorStore{ dynamoClient: lpaDynamoClient, @@ -186,6 +185,7 @@ func App( evidenceReceivedStore, s3Client, documentStore, + eventClient, ) return withAppData(page.ValidateCsrf(rootMux, sessionStore, random.String, errorHandler), localizer, lang, rumConfig, staticHash, oneloginURL) diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 4d2286e4d9..e847844dbc 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -8,20 +8,18 @@ import ( "github.com/gorilla/sessions" "github.com/ministryofjustice/opg-go-common/logging" "github.com/ministryofjustice/opg-go-common/template" - "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/ministryofjustice/opg-modernising-lpa/internal/localize" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/ministryofjustice/opg-modernising-lpa/internal/onelogin" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" "github.com/ministryofjustice/opg-modernising-lpa/internal/place" - "github.com/ministryofjustice/opg-modernising-lpa/internal/s3" "github.com/ministryofjustice/opg-modernising-lpa/internal/sesh" "github.com/stretchr/testify/assert" ) func TestApp(t *testing.T) { - app := App(&logging.Logger{}, &localize.Localizer{}, localize.En, template.Templates{}, nil, &dynamo.Client{}, "http://public.url", &pay.Client{}, ¬ify.Client{}, &place.Client{}, page.RumConfig{}, "?%3fNEI0t9MN", page.Paths, &onelogin.Client{}, "http://onelogin.url", &s3.Client{}, nil) + app := App(&logging.Logger{}, &localize.Localizer{}, localize.En, template.Templates{}, nil, nil, "http://public.url", &pay.Client{}, ¬ify.Client{}, &place.Client{}, page.RumConfig{}, "?%3fNEI0t9MN", page.Paths, &onelogin.Client{}, "http://onelogin.url", nil, nil) assert.Implements(t, (*http.Handler)(nil), app) } diff --git a/internal/app/document_store.go b/internal/app/document_store.go index 74a050282b..f65e26184b 100644 --- a/internal/app/document_store.go +++ b/internal/app/document_store.go @@ -3,30 +3,30 @@ package app import ( "context" "errors" - "fmt" + "time" "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" + "github.com/ministryofjustice/opg-modernising-lpa/internal/event" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" ) -type PartialBatchWriteError struct { - Written int - Expected int -} - -func (e PartialBatchWriteError) Error() string { - return fmt.Sprintf("Expected to write %d but %d were written", e.Expected, e.Written) -} - type documentStore struct { dynamoClient DynamoClient s3Client S3Client + eventClient EventClient randomUUID func() string + now func() time.Time } -func NewDocumentStore(dynamoClient DynamoClient, s3Client S3Client, randomUUID func() string) *documentStore { - return &documentStore{dynamoClient: dynamoClient, s3Client: s3Client, randomUUID: randomUUID} +func NewDocumentStore(dynamoClient DynamoClient, s3Client S3Client, eventClient EventClient, randomUUID func() string, now func() time.Time) *documentStore { + return &documentStore{ + dynamoClient: dynamoClient, + s3Client: s3Client, + eventClient: eventClient, + randomUUID: randomUUID, + now: now, + } } func (s *documentStore) Create(ctx context.Context, lpa *page.Lpa, filename string, data []byte) (page.Document, error) { @@ -79,39 +79,16 @@ func (s *documentStore) UpdateScanResults(ctx context.Context, lpaID, s3ObjectKe "set VirusDetected = :virusDetected, Scanned = :scanned") } -func (s *documentStore) BatchPut(ctx context.Context, documents []page.Document) error { - var converted []interface{} - for _, d := range documents { - converted = append(converted, d) - } - - toWrite := len(converted) - written, err := s.dynamoClient.BatchPut(ctx, converted) - - if err != nil { - return err - } else if written != toWrite { - return PartialBatchWriteError{Written: written, Expected: toWrite} - } - - return nil -} - func (s *documentStore) Put(ctx context.Context, document page.Document) error { return s.dynamoClient.Put(ctx, document) } func (s *documentStore) DeleteInfectedDocuments(ctx context.Context, documents page.Documents) error { var dynamoKeys []dynamo.Key - var s3Keys []string for _, d := range documents { if d.VirusDetected { - dynamoKeys = append(dynamoKeys, dynamo.Key{ - PK: d.PK, - SK: d.SK, - }) - s3Keys = append(s3Keys, d.Key) + dynamoKeys = append(dynamoKeys, dynamo.Key{PK: d.PK, SK: d.SK}) } } @@ -119,10 +96,6 @@ func (s *documentStore) DeleteInfectedDocuments(ctx context.Context, documents p return nil } - if err := s.s3Client.DeleteObjects(ctx, s3Keys); err != nil { - return err - } - return s.dynamoClient.DeleteKeys(ctx, dynamoKeys) } @@ -134,6 +107,45 @@ func (s *documentStore) Delete(ctx context.Context, document page.Document) erro return s.dynamoClient.DeleteOne(ctx, document.PK, document.SK) } +func (s *documentStore) Submit(ctx context.Context, lpa *page.Lpa, documents page.Documents) error { + var unsentDocuments []any + var unsentDocumentKeys []string + + tags := map[string]string{ + "replicate": "true", + "virus-scan-status": "ok", + } + + for _, document := range documents { + if document.Sent.IsZero() && !document.VirusDetected { + document.Sent = s.now() + unsentDocuments = append(unsentDocuments, document) + unsentDocumentKeys = append(unsentDocumentKeys, document.Key) + + if err := s.s3Client.PutObjectTagging(ctx, document.Key, tags); err != nil { + return err + } + } + } + + if len(unsentDocuments) > 0 { + if err := s.eventClient.SendReducedFeeRequested(ctx, event.ReducedFeeRequested{ + UID: lpa.UID, + RequestType: lpa.FeeType.String(), + Evidence: unsentDocumentKeys, + EvidenceDelivery: lpa.EvidenceDelivery.String(), + }); err != nil { + return err + } + + if err := s.dynamoClient.BatchPut(ctx, unsentDocuments); err != nil { + return err + } + } + + return nil +} + func documentKey(s3Key string) string { return "#DOCUMENT#" + s3Key } diff --git a/internal/app/document_store_test.go b/internal/app/document_store_test.go index e83761876a..8cdf335ac2 100644 --- a/internal/app/document_store_test.go +++ b/internal/app/document_store_test.go @@ -4,10 +4,13 @@ import ( "context" "encoding/json" "testing" + "time" "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" + "github.com/ministryofjustice/opg-modernising-lpa/internal/event" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" + "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -15,10 +18,11 @@ import ( func TestNewDocumentStore(t *testing.T) { dynamoClient := newMockDynamoClient(t) s3Client := newMockS3Client(t) + eventClient := newMockEventClient(t) - expected := &documentStore{dynamoClient: dynamoClient, s3Client: s3Client, randomUUID: nil} + expected := &documentStore{dynamoClient: dynamoClient, s3Client: s3Client, eventClient: eventClient} - assert.Equal(t, expected, NewDocumentStore(dynamoClient, s3Client, nil)) + assert.Equal(t, expected, NewDocumentStore(dynamoClient, s3Client, eventClient, nil, nil)) } func TestDocumentStoreGetAll(t *testing.T) { @@ -169,11 +173,6 @@ func TestDocumentStorePutWhenDynamoClientError(t *testing.T) { func TestDeleteInfectedDocuments(t *testing.T) { ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123"}) - s3Client := newMockS3Client(t) - s3Client. - On("DeleteObjects", ctx, []string{"a-key", "another-key"}). - Return(nil) - dynamoClient := newMockDynamoClient(t) dynamoClient. On("DeleteKeys", ctx, []dynamo.Key{ @@ -182,7 +181,7 @@ func TestDeleteInfectedDocuments(t *testing.T) { }). Return(nil) - documentStore := documentStore{s3Client: s3Client, dynamoClient: dynamoClient} + documentStore := documentStore{dynamoClient: dynamoClient} err := documentStore.DeleteInfectedDocuments(ctx, page.Documents{ {PK: "a-pk", SK: "a-sk", Key: "a-key", VirusDetected: true}, @@ -192,32 +191,9 @@ func TestDeleteInfectedDocuments(t *testing.T) { assert.Nil(t, err) } -func TestDeleteInfectedDocumentsWhenS3ClientError(t *testing.T) { - ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123"}) - - s3Client := newMockS3Client(t) - s3Client. - On("DeleteObjects", ctx, []string{"a-key", "another-key"}). - Return(expectedError) - - documentStore := documentStore{s3Client: s3Client} - - err := documentStore.DeleteInfectedDocuments(ctx, page.Documents{ - {PK: "a-pk", SK: "a-sk", Key: "a-key", VirusDetected: true}, - {PK: "another-pk", SK: "another-sk", Key: "another-key", VirusDetected: true}, - }) - - assert.Equal(t, expectedError, err) -} - func TestDeleteInfectedDocumentsWhenDynamoClientError(t *testing.T) { ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123"}) - s3Client := newMockS3Client(t) - s3Client. - On("DeleteObjects", ctx, []string{"a-key", "another-key"}). - Return(nil) - dynamoClient := newMockDynamoClient(t) dynamoClient. On("DeleteKeys", ctx, []dynamo.Key{ @@ -226,7 +202,7 @@ func TestDeleteInfectedDocumentsWhenDynamoClientError(t *testing.T) { }). Return(expectedError) - documentStore := documentStore{s3Client: s3Client, dynamoClient: dynamoClient} + documentStore := documentStore{dynamoClient: dynamoClient} err := documentStore.DeleteInfectedDocuments(ctx, page.Documents{ {PK: "a-pk", SK: "a-sk", Key: "a-key", VirusDetected: true}, @@ -304,69 +280,146 @@ func TestDeleteWhenDynamoClientError(t *testing.T) { assert.Equal(t, expectedError, err) } -func TestDocumentKey(t *testing.T) { - assert.Equal(t, "#DOCUMENT#key", documentKey("key")) -} +func TestDocumentStoreSubmit(t *testing.T) { + ctx := context.Background() + now := time.Now() -func TestBatchPut(t *testing.T) { - ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123"}) + lpa := &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} + documents := page.Documents{ + {PK: "a-pk", SK: "a-sk", Key: "a-key"}, + {PK: "b-pk", SK: "b-sk", Key: "b-key"}, + } + + s3Client := newMockS3Client(t) + s3Client. + On("PutObjectTagging", ctx, "a-key", map[string]string{"replicate": "true", "virus-scan-status": "ok"}). + Return(nil) + s3Client. + On("PutObjectTagging", ctx, "b-key", map[string]string{"replicate": "true", "virus-scan-status": "ok"}). + Return(nil) + + eventClient := newMockEventClient(t) + eventClient. + On("SendReducedFeeRequested", ctx, event.ReducedFeeRequested{ + UID: "lpa-uid", + RequestType: pay.HalfFee.String(), + Evidence: []string{"a-key", "b-key"}, + EvidenceDelivery: pay.Upload.String(), + }). + Return(nil) dynamoClient := newMockDynamoClient(t) dynamoClient. - On("BatchPut", ctx, []interface{}{ - page.Document{PK: "a-pk", SK: "a-sk", Key: "a-key"}, - page.Document{PK: "aanother-pk", SK: "aanother-sk", Key: "aanother-key"}, + On("BatchPut", ctx, []any{ + page.Document{PK: "a-pk", SK: "a-sk", Key: "a-key", Sent: now}, + page.Document{PK: "b-pk", SK: "b-sk", Key: "b-key", Sent: now}, }). - Return(2, nil) + Return(nil) - documentStore := documentStore{dynamoClient: dynamoClient} + documentStore := &documentStore{ + dynamoClient: dynamoClient, + eventClient: eventClient, + s3Client: s3Client, + now: func() time.Time { return now }, + } - err := documentStore.BatchPut(ctx, []page.Document{ - {PK: "a-pk", SK: "a-sk", Key: "a-key"}, - {PK: "aanother-pk", SK: "aanother-sk", Key: "aanother-key"}, - }) + err := documentStore.Submit(ctx, lpa, documents) + assert.Nil(t, err) +} + +func TestDocumentStoreSubmitWhenNoUnsentDocuments(t *testing.T) { + ctx := context.Background() + lpa := &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} + documents := page.Documents{{PK: "a-pk", SK: "a-sk", Key: "a-key", Sent: time.Now()}} + + documentStore := &documentStore{} + + err := documentStore.Submit(ctx, lpa, documents) assert.Nil(t, err) } -func TestBatchPutPartialWrite(t *testing.T) { - ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123"}) +func TestDocumentStoreSubmitWhenS3ClientErrors(t *testing.T) { + ctx := context.Background() + now := time.Now() - dynamoClient := newMockDynamoClient(t) - dynamoClient. - On("BatchPut", ctx, []interface{}{ - page.Document{PK: "a-pk", SK: "a-sk", Key: "a-key"}, - page.Document{PK: "aanother-pk", SK: "aanother-sk", Key: "aanother-key"}, - }). - Return(1, nil) + lpa := &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} + documents := page.Documents{{PK: "a-pk", SK: "a-sk", Key: "a-key"}} - documentStore := documentStore{dynamoClient: dynamoClient} + s3Client := newMockS3Client(t) + s3Client. + On("PutObjectTagging", ctx, "a-key", mock.Anything). + Return(expectedError) - err := documentStore.BatchPut(ctx, []page.Document{ - {PK: "a-pk", SK: "a-sk", Key: "a-key"}, - {PK: "aanother-pk", SK: "aanother-sk", Key: "aanother-key"}, - }) + documentStore := &documentStore{ + s3Client: s3Client, + now: func() time.Time { return now }, + } - assert.Equal(t, PartialBatchWriteError{Written: 1, Expected: 2}, err) + err := documentStore.Submit(ctx, lpa, documents) + assert.Equal(t, expectedError, err) } -func TestBatchPutWhenDynamoError(t *testing.T) { - ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123"}) +func TestDocumentStoreSubmitWhenEventClientErrors(t *testing.T) { + ctx := context.Background() + now := time.Now() + + lpa := &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} + documents := page.Documents{{PK: "a-pk", SK: "a-sk", Key: "a-key"}} + + s3Client := newMockS3Client(t) + s3Client. + On("PutObjectTagging", ctx, "a-key", mock.Anything). + Return(nil) + + eventClient := newMockEventClient(t) + eventClient. + On("SendReducedFeeRequested", ctx, mock.Anything). + Return(expectedError) + + documentStore := &documentStore{ + eventClient: eventClient, + s3Client: s3Client, + now: func() time.Time { return now }, + } + + err := documentStore.Submit(ctx, lpa, documents) + assert.Equal(t, expectedError, err) +} + +func TestDocumentStoreSubmitWhenDynamoClientErrors(t *testing.T) { + ctx := context.Background() + now := time.Now() + + lpa := &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} + documents := page.Documents{{PK: "a-pk", SK: "a-sk", Key: "a-key"}} + + s3Client := newMockS3Client(t) + s3Client. + On("PutObjectTagging", ctx, "a-key", mock.Anything). + Return(nil) + + eventClient := newMockEventClient(t) + eventClient. + On("SendReducedFeeRequested", ctx, mock.Anything). + Return(nil) dynamoClient := newMockDynamoClient(t) dynamoClient. - On("BatchPut", ctx, []interface{}{ - page.Document{PK: "a-pk", SK: "a-sk", Key: "a-key"}, - page.Document{PK: "aanother-pk", SK: "aanother-sk", Key: "aanother-key"}, - }). - Return(0, expectedError) + On("BatchPut", ctx, mock.Anything). + Return(expectedError) - documentStore := documentStore{dynamoClient: dynamoClient} - - err := documentStore.BatchPut(ctx, []page.Document{ - {PK: "a-pk", SK: "a-sk", Key: "a-key"}, - {PK: "aanother-pk", SK: "aanother-sk", Key: "aanother-key"}, - }) + documentStore := &documentStore{ + dynamoClient: dynamoClient, + eventClient: eventClient, + s3Client: s3Client, + now: func() time.Time { return now }, + } + err := documentStore.Submit(ctx, lpa, documents) assert.Equal(t, expectedError, err) } + +func TestDocumentKey(t *testing.T) { + assert.Equal(t, "#DOCUMENT#key", documentKey("key")) +} diff --git a/internal/app/donor_store.go b/internal/app/donor_store.go index d0ae7a7a85..af7e0afdac 100644 --- a/internal/app/donor_store.go +++ b/internal/app/donor_store.go @@ -30,7 +30,6 @@ type DocumentStore interface { GetAll(context.Context) (page.Documents, error) Put(context.Context, page.Document) error UpdateScanResults(context.Context, string, string, bool) error - BatchPut(context.Context, []page.Document) error } type donorStore struct { @@ -188,46 +187,6 @@ func (s *donorStore) Put(ctx context.Context, lpa *page.Lpa) error { lpa.HasSentPreviousApplicationLinkedEvent = true } - if lpa.UID != "" && lpa.Tasks.PayForLpa.IsPending() { - documents, err := s.documentStore.GetAll(ctx) - if err != nil { - s.logger.Print(err) - return s.dynamoClient.Put(ctx, lpa) - } - - var unsentKeys []string - - for _, document := range documents { - if document.Sent.IsZero() && !document.Scanned { - unsentKeys = append(unsentKeys, document.Key) - } - } - - if len(unsentKeys) > 0 { - if err := s.eventClient.SendReducedFeeRequested(ctx, event.ReducedFeeRequested{ - UID: lpa.UID, - RequestType: lpa.FeeType.String(), - Evidence: unsentKeys, - EvidenceDelivery: lpa.EvidenceDelivery.String(), - }); err != nil { - return err - } - - var updatedDocuments page.Documents - - for _, document := range documents { - if document.Sent.IsZero() && !document.Scanned { - document.Sent = s.now() - updatedDocuments = append(updatedDocuments, document) - } - } - - if err := s.documentStore.BatchPut(ctx, updatedDocuments); err != nil { - s.logger.Print(err) - } - } - } - return s.dynamoClient.Put(ctx, lpa) } diff --git a/internal/app/donor_store_test.go b/internal/app/donor_store_test.go index c53fd88f69..ba83039adc 100644 --- a/internal/app/donor_store_test.go +++ b/internal/app/donor_store_test.go @@ -12,7 +12,6 @@ import ( "github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo" "github.com/ministryofjustice/opg-modernising-lpa/internal/event" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" - "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" "github.com/ministryofjustice/opg-modernising-lpa/internal/place" "github.com/ministryofjustice/opg-modernising-lpa/internal/uid" "github.com/stretchr/testify/assert" @@ -450,209 +449,6 @@ func TestDonorStorePutWhenPreviousApplicationLinkedWhenError(t *testing.T) { assert.Equal(t, expectedError, err) } -func TestDonorStorePutWhenReducedFeeRequestedAndUnsentDocuments(t *testing.T) { - ctx := context.Background() - now := time.Now() - - dynamoClient := newMockDynamoClient(t) - dynamoClient. - On("Put", ctx, &page.Lpa{ - PK: "LPA#5", - SK: "#DONOR#an-id", - ID: "5", - UID: "M-1111", - UpdatedAt: now, - FeeType: pay.HalfFee, - Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}, - HasSentApplicationUpdatedEvent: true, - EvidenceDelivery: pay.Upload, - }). - Return(nil) - - eventClient := newMockEventClient(t) - eventClient. - On("SendReducedFeeRequested", ctx, event.ReducedFeeRequested{ - UID: "M-1111", - RequestType: "HalfFee", - Evidence: []string{"lpa-uid-evidence-a-uid"}, - EvidenceDelivery: "upload", - }). - Return(nil) - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", ctx). - Return(page.Documents{ - {Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf"}, - }, nil) - documentStore. - On("BatchPut", ctx, []page.Document{{Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf", Sent: now}}). - Return(nil) - - donorStore := &donorStore{dynamoClient: dynamoClient, eventClient: eventClient, now: func() time.Time { return now }, documentStore: documentStore} - - err := donorStore.Put(ctx, &page.Lpa{ - PK: "LPA#5", - SK: "#DONOR#an-id", - ID: "5", - UID: "M-1111", - FeeType: pay.HalfFee, - Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}, - HasSentApplicationUpdatedEvent: true, - EvidenceDelivery: pay.Upload, - }) - - assert.Nil(t, err) -} - -func TestDonorStorePutWhenReducedFeeRequestedWontResend(t *testing.T) { - ctx := context.Background() - now := time.Now() - - dynamoClient := newMockDynamoClient(t) - dynamoClient. - On("Put", ctx, mock.Anything). - Return(nil) - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", ctx). - Return(page.Documents{ - {Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf", Sent: now}, - }, nil) - - donorStore := &donorStore{dynamoClient: dynamoClient, now: func() time.Time { return now }, documentStore: documentStore} - - err := donorStore.Put(ctx, &page.Lpa{ - PK: "LPA#5", - SK: "#DONOR#an-id", - ID: "5", - UID: "M-1111", - Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}, - HasSentApplicationUpdatedEvent: true, - }) - - assert.Nil(t, err) -} - -func TestDonorStorePutWhenReducedFeeRequestedWhenDocumentStoreGetAllError(t *testing.T) { - ctx := context.Background() - now := time.Now() - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", ctx). - Return(page.Documents{}, expectedError) - - dynamoClient := newMockDynamoClient(t) - dynamoClient. - On("Put", ctx, mock.Anything). - Return(nil) - - logger := newMockLogger(t) - logger. - On("Print", expectedError) - - donorStore := &donorStore{now: func() time.Time { return now }, documentStore: documentStore, logger: logger, dynamoClient: dynamoClient} - - err := donorStore.Put(ctx, &page.Lpa{ - PK: "LPA#5", - SK: "#DONOR#an-id", - ID: "5", - UID: "M-1111", - Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}, - HasSentApplicationUpdatedEvent: true, - }) - - assert.Nil(t, err) -} - -func TestDonorStorePutWhenReducedFeeRequestedAndUnsentDocumentsWhenEventClientSendError(t *testing.T) { - ctx := context.Background() - now := time.Now() - - eventClient := newMockEventClient(t) - eventClient. - On("SendReducedFeeRequested", ctx, event.ReducedFeeRequested{ - UID: "M-1111", - RequestType: "HalfFee", - Evidence: []string{"lpa-uid-evidence-a-uid"}, - EvidenceDelivery: "upload", - }). - Return(expectedError) - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", ctx). - Return(page.Documents{ - {Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf"}, - }, nil) - - donorStore := &donorStore{eventClient: eventClient, now: func() time.Time { return now }, documentStore: documentStore} - - err := donorStore.Put(ctx, &page.Lpa{ - PK: "LPA#5", - SK: "#DONOR#an-id", - ID: "5", - UID: "M-1111", - FeeType: pay.HalfFee, - Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}, - HasSentApplicationUpdatedEvent: true, - EvidenceDelivery: pay.Upload, - }) - - assert.Equal(t, expectedError, err) -} - -func TestDonorStorePutWhenReducedFeeRequestedAndUnsentDocumentsWhenDocumentStoreBatchPutError(t *testing.T) { - ctx := context.Background() - now := time.Now() - - eventClient := newMockEventClient(t) - eventClient. - On("SendReducedFeeRequested", ctx, event.ReducedFeeRequested{ - UID: "M-1111", - RequestType: "HalfFee", - Evidence: []string{"lpa-uid-evidence-a-uid"}, - EvidenceDelivery: "upload", - }). - Return(nil) - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", ctx). - Return(page.Documents{ - {Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf"}, - }, nil) - documentStore. - On("BatchPut", ctx, []page.Document{{Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf", Sent: now}}). - Return(expectedError) - - logger := newMockLogger(t) - logger. - On("Print", expectedError) - - dynamoClient := newMockDynamoClient(t) - dynamoClient. - On("Put", ctx, mock.Anything). - Return(nil) - - donorStore := &donorStore{eventClient: eventClient, now: func() time.Time { return now }, documentStore: documentStore, dynamoClient: dynamoClient, logger: logger} - - err := donorStore.Put(ctx, &page.Lpa{ - PK: "LPA#5", - SK: "#DONOR#an-id", - ID: "5", - UID: "M-1111", - FeeType: pay.HalfFee, - Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}, - HasSentApplicationUpdatedEvent: true, - EvidenceDelivery: pay.Upload, - }) - - assert.Nil(t, err) -} - func TestDonorStoreCreate(t *testing.T) { ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{SessionID: "an-id"}) now := time.Now() diff --git a/internal/app/mock_DocumentStore_test.go b/internal/app/mock_DocumentStore_test.go index ea6eb407ca..08cf0c3b2d 100644 --- a/internal/app/mock_DocumentStore_test.go +++ b/internal/app/mock_DocumentStore_test.go @@ -14,20 +14,6 @@ type mockDocumentStore struct { mock.Mock } -// BatchPut provides a mock function with given fields: _a0, _a1 -func (_m *mockDocumentStore) BatchPut(_a0 context.Context, _a1 []page.Document) error { - ret := _m.Called(_a0, _a1) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, []page.Document) error); ok { - r0 = rf(_a0, _a1) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // GetAll provides a mock function with given fields: _a0 func (_m *mockDocumentStore) GetAll(_a0 context.Context) (page.Documents, error) { ret := _m.Called(_a0) diff --git a/internal/app/mock_DynamoClient_test.go b/internal/app/mock_DynamoClient_test.go index 882b2e2312..9ce2f7278d 100644 --- a/internal/app/mock_DynamoClient_test.go +++ b/internal/app/mock_DynamoClient_test.go @@ -97,27 +97,17 @@ func (_m *mockDynamoClient) AllKeysByPk(ctx context.Context, pk string) ([]dynam } // BatchPut provides a mock function with given fields: ctx, items -func (_m *mockDynamoClient) BatchPut(ctx context.Context, items []interface{}) (int, error) { +func (_m *mockDynamoClient) BatchPut(ctx context.Context, items []interface{}) error { ret := _m.Called(ctx, items) - var r0 int - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, []interface{}) (int, error)); ok { - return rf(ctx, items) - } - if rf, ok := ret.Get(0).(func(context.Context, []interface{}) int); ok { + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []interface{}) error); ok { r0 = rf(ctx, items) } else { - r0 = ret.Get(0).(int) - } - - if rf, ok := ret.Get(1).(func(context.Context, []interface{}) error); ok { - r1 = rf(ctx, items) - } else { - r1 = ret.Error(1) + r0 = ret.Error(0) } - return r0, r1 + return r0 } // Create provides a mock function with given fields: ctx, v diff --git a/internal/app/mock_S3Client_test.go b/internal/app/mock_S3Client_test.go index b401a6584f..1d0848f2cb 100644 --- a/internal/app/mock_S3Client_test.go +++ b/internal/app/mock_S3Client_test.go @@ -5,7 +5,6 @@ package app import ( context "context" - types "github.com/aws/aws-sdk-go-v2/service/s3/types" mock "github.com/stretchr/testify/mock" ) @@ -57,11 +56,11 @@ func (_m *mockS3Client) PutObject(_a0 context.Context, _a1 string, _a2 []byte) e } // PutObjectTagging provides a mock function with given fields: _a0, _a1, _a2 -func (_m *mockS3Client) PutObjectTagging(_a0 context.Context, _a1 string, _a2 []types.Tag) error { +func (_m *mockS3Client) PutObjectTagging(_a0 context.Context, _a1 string, _a2 map[string]string) error { ret := _m.Called(_a0, _a1, _a2) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, []types.Tag) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, string, map[string]string) error); ok { r0 = rf(_a0, _a1, _a2) } else { r0 = ret.Error(0) diff --git a/internal/dynamo/client.go b/internal/dynamo/client.go index 090f7094fa..e05a7900d6 100644 --- a/internal/dynamo/client.go +++ b/internal/dynamo/client.go @@ -347,48 +347,26 @@ func (c *Client) Update(ctx context.Context, pk, sk string, values map[string]ty return err } -func (c *Client) BatchPut(ctx context.Context, items []interface{}) (int, error) { - // courtesy of https://docs.aws.amazon.com/code-library/latest/ug/go_2_dynamodb_code_examples.html - - var err error - var itemValues map[string]types.AttributeValue - - written := 0 - batchSize := 25 // DynamoDB allows a maximum batch size of 25 items. - start := 0 - end := start + batchSize - - for start < len(items) { - var writeReqs []types.WriteRequest - if end > len(items) { - end = len(items) - } - - for _, item := range items[start:end] { - itemValues, err = attributevalue.MarshalMap(item) - if err != nil { - c.logger.Print("failed to marshal item during BatchPut: ", err) - } else { - writeReqs = append( - writeReqs, - types.WriteRequest{PutRequest: &types.PutRequest{Item: itemValues}}, - ) - } - } - - _, err := c.svc.BatchWriteItem(ctx, &dynamodb.BatchWriteItemInput{ - RequestItems: map[string][]types.WriteRequest{c.table: writeReqs}, - }) +func (c *Client) BatchPut(ctx context.Context, values []interface{}) error { + items := make([]types.TransactWriteItem, len(values)) + for i, value := range values { + v, err := attributevalue.MarshalMap(value) if err != nil { - c.logger.Print("failed to add a batch of item during BatchPut: ", err) - } else { - written += len(writeReqs) + return err } - start = end - end += batchSize + items[i] = types.TransactWriteItem{ + Put: &types.Put{ + TableName: aws.String(c.table), + Item: v, + }, + } } - return written, err + _, err := c.svc.TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{ + TransactItems: items, + }) + + return err } diff --git a/internal/dynamo/client_test.go b/internal/dynamo/client_test.go index c9fd0323d3..7756143567 100644 --- a/internal/dynamo/client_test.go +++ b/internal/dynamo/client_test.go @@ -722,114 +722,32 @@ func TestUpdateOnServiceError(t *testing.T) { func TestBatchPutOneBatch(t *testing.T) { ctx := context.Background() - type testObject struct { - Col string - } - - item, _ := attributevalue.MarshalMap(map[string]string{"Col": "Val"}) - var items []interface{} - var writeReqs []types.WriteRequest - - count := 0 - for count < 25 { - items = append(items, testObject{Col: "Val"}) - - writeReqs = append( - writeReqs, - types.WriteRequest{PutRequest: &types.PutRequest{Item: item}}, - ) - - count++ - } - - dynamoDB := newMockDynamoDB(t) - dynamoDB. - On("BatchWriteItem", ctx, &dynamodb.BatchWriteItemInput{ - RequestItems: map[string][]types.WriteRequest{"table-name": writeReqs}, - }). - Return(nil, nil). - Once() - - c := &Client{table: "table-name", svc: dynamoDB} - written, err := c.BatchPut(ctx, items) - - assert.Nil(t, err) - assert.Equal(t, 25, written) -} - -func TestBatchPutMultipleBatches(t *testing.T) { - ctx := context.Background() - - type testObject struct { - Col string - } - - item, _ := attributevalue.MarshalMap(map[string]string{"Col": "Val"}) - var items []interface{} - var writeReqs []types.WriteRequest - - count := 0 - for count < 26 { - items = append(items, testObject{Col: "Val"}) - - writeReqs = append( - writeReqs, - types.WriteRequest{PutRequest: &types.PutRequest{Item: item}}, - ) - count++ - } + values := []any{map[string]string{"a": "b"}, map[string]string{"x": "y"}} + itemA, _ := attributevalue.MarshalMap(values[0]) + itemB, _ := attributevalue.MarshalMap(values[1]) dynamoDB := newMockDynamoDB(t) dynamoDB. - On("BatchWriteItem", ctx, &dynamodb.BatchWriteItemInput{ - RequestItems: map[string][]types.WriteRequest{"table-name": writeReqs[0:25]}, - }). - Return(nil, nil). - Once() - - dynamoDB. - On("BatchWriteItem", ctx, &dynamodb.BatchWriteItemInput{ - RequestItems: map[string][]types.WriteRequest{"table-name": { - {PutRequest: &types.PutRequest{Item: item}}, - }}, - }). - Return(nil, nil). - Once() - - c := &Client{table: "table-name", svc: dynamoDB} - written, err := c.BatchPut(ctx, items) - - assert.Nil(t, err) - assert.Equal(t, 26, written) -} - -func TestBatchPutWhenDynamoBatchWriteItemError(t *testing.T) { - ctx := context.Background() - - type testObject struct { - Col string - } - - item, _ := attributevalue.MarshalMap(map[string]string{"Col": "Val"}) - items := []interface{}{testObject{Col: "Val"}} - writeReqs := []types.WriteRequest{ - {PutRequest: &types.PutRequest{Item: item}}, - } - - dynamoDB := newMockDynamoDB(t) - dynamoDB. - On("BatchWriteItem", ctx, &dynamodb.BatchWriteItemInput{ - RequestItems: map[string][]types.WriteRequest{"table-name": writeReqs}, + On("TransactWriteItems", ctx, &dynamodb.TransactWriteItemsInput{ + TransactItems: []types.TransactWriteItem{ + { + Put: &types.Put{ + TableName: aws.String("table-name"), + Item: itemA, + }, + }, + { + Put: &types.Put{ + TableName: aws.String("table-name"), + Item: itemB, + }, + }, + }, }). Return(nil, expectedError) - logger := newMockLogger(t) - logger. - On("Print", "failed to add a batch of item during BatchPut: ", expectedError) - - c := &Client{table: "table-name", svc: dynamoDB, logger: logger} - written, err := c.BatchPut(ctx, items) + c := &Client{table: "table-name", svc: dynamoDB} + err := c.BatchPut(ctx, values) - assert.Nil(t, err) - assert.Equal(t, 0, written) + assert.Equal(t, expectedError, err) } diff --git a/internal/page/donor/mock_DocumentStore_test.go b/internal/page/donor/mock_DocumentStore_test.go index d453807f79..80ee8cc80f 100644 --- a/internal/page/donor/mock_DocumentStore_test.go +++ b/internal/page/donor/mock_DocumentStore_test.go @@ -106,6 +106,20 @@ func (_m *mockDocumentStore) Put(_a0 context.Context, _a1 page.Document) error { return r0 } +// Submit provides a mock function with given fields: _a0, _a1, _a2 +func (_m *mockDocumentStore) Submit(_a0 context.Context, _a1 *page.Lpa, _a2 page.Documents) error { + ret := _m.Called(_a0, _a1, _a2) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *page.Lpa, page.Documents) error); ok { + r0 = rf(_a0, _a1, _a2) + } else { + r0 = ret.Error(0) + } + + return r0 +} + type mockConstructorTestingTnewMockDocumentStore interface { mock.TestingT Cleanup(func()) diff --git a/internal/page/donor/mock_EventClient_test.go b/internal/page/donor/mock_EventClient_test.go index a4c8674d7a..f973ca9910 100644 --- a/internal/page/donor/mock_EventClient_test.go +++ b/internal/page/donor/mock_EventClient_test.go @@ -5,6 +5,7 @@ package donor import ( context "context" + event "github.com/ministryofjustice/opg-modernising-lpa/internal/event" mock "github.com/stretchr/testify/mock" ) @@ -13,13 +14,13 @@ type mockEventClient struct { mock.Mock } -// Send provides a mock function with given fields: _a0, _a1, _a2 -func (_m *mockEventClient) Send(_a0 context.Context, _a1 string, _a2 interface{}) error { - ret := _m.Called(_a0, _a1, _a2) +// SendReducedFeeRequested provides a mock function with given fields: _a0, _a1 +func (_m *mockEventClient) SendReducedFeeRequested(_a0 context.Context, _a1 event.ReducedFeeRequested) error { + ret := _m.Called(_a0, _a1) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) error); ok { - r0 = rf(_a0, _a1, _a2) + if rf, ok := ret.Get(0).(func(context.Context, event.ReducedFeeRequested) error); ok { + r0 = rf(_a0, _a1) } else { r0 = ret.Error(0) } diff --git a/internal/page/donor/mock_S3Client_test.go b/internal/page/donor/mock_S3Client_test.go index 0809092fa4..b2df6f4132 100644 --- a/internal/page/donor/mock_S3Client_test.go +++ b/internal/page/donor/mock_S3Client_test.go @@ -5,7 +5,6 @@ package donor import ( context "context" - types "github.com/aws/aws-sdk-go-v2/service/s3/types" mock "github.com/stretchr/testify/mock" ) @@ -42,20 +41,6 @@ func (_m *mockS3Client) PutObject(_a0 context.Context, _a1 string, _a2 []byte) e return r0 } -// PutObjectTagging provides a mock function with given fields: _a0, _a1, _a2 -func (_m *mockS3Client) PutObjectTagging(_a0 context.Context, _a1 string, _a2 []types.Tag) error { - ret := _m.Called(_a0, _a1, _a2) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, []types.Tag) error); ok { - r0 = rf(_a0, _a1, _a2) - } else { - r0 = ret.Error(0) - } - - return r0 -} - type mockConstructorTestingTnewMockS3Client interface { mock.TestingT Cleanup(func()) diff --git a/internal/page/donor/payment_confirmation.go b/internal/page/donor/payment_confirmation.go index fec92ecff9..060b643146 100644 --- a/internal/page/donor/payment_confirmation.go +++ b/internal/page/donor/payment_confirmation.go @@ -3,10 +3,7 @@ package donor import ( "fmt" "net/http" - "time" - "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/gorilla/sessions" "github.com/ministryofjustice/opg-go-common/template" "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" @@ -25,7 +22,7 @@ type paymentConfirmationData struct { EvidenceDelivery pay.EvidenceDelivery } -func PaymentConfirmation(logger Logger, tmpl template.Template, payClient PayClient, donorStore DonorStore, sessionStore sessions.Store, evidenceS3Client S3Client, now func() time.Time, documentStore DocumentStore) Handler { +func PaymentConfirmation(logger Logger, tmpl template.Template, payClient PayClient, donorStore DonorStore, sessionStore sessions.Store) Handler { return func(appData page.AppData, w http.ResponseWriter, r *http.Request, lpa *page.Lpa) error { paymentSession, err := sesh.Payment(sessionStore, r) if err != nil { @@ -62,29 +59,6 @@ func PaymentConfirmation(logger Logger, tmpl template.Template, payClient PayCli lpa.Tasks.PayForLpa = actor.PaymentTaskCompleted } else { lpa.Tasks.PayForLpa = actor.PaymentTaskPending - - documents, err := documentStore.GetAll(r.Context()) - if err != nil { - return err - } - - for _, document := range documents { - if document.Sent.IsZero() { - err := evidenceS3Client.PutObjectTagging(r.Context(), document.Key, []types.Tag{ - {Key: aws.String("replicate"), Value: aws.String("true")}, - }) - - if err != nil { - logger.Print(fmt.Sprintf("error tagging evidence: %s", err.Error())) - return err - } - - document.Sent = now() - if err := documentStore.Put(r.Context(), document); err != nil { - return err - } - } - } } if err := donorStore.Put(r.Context(), lpa); err != nil { diff --git a/internal/page/donor/payment_confirmation_test.go b/internal/page/donor/payment_confirmation_test.go index 0a4d1b06f2..12af2e555d 100644 --- a/internal/page/donor/payment_confirmation_test.go +++ b/internal/page/donor/payment_confirmation_test.go @@ -5,10 +5,7 @@ import ( "net/http" "net/http/httptest" "testing" - "time" - "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/gorilla/sessions" "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" @@ -55,7 +52,7 @@ func TestGetPaymentConfirmationFullFee(t *testing.T) { }). Return(nil) - err := PaymentConfirmation(newMockLogger(t), template.Execute, payClient, donorStore, sessionStore, nil, nil, nil)(testAppData, w, r, &page.Lpa{ + err := PaymentConfirmation(newMockLogger(t), template.Execute, payClient, donorStore, sessionStore)(testAppData, w, r, &page.Lpa{ FeeType: pay.FullFee, CertificateProvider: actor.CertificateProvider{ Email: "certificateprovider@example.com", @@ -87,8 +84,6 @@ func TestGetPaymentConfirmationHalfFee(t *testing.T) { withPaySession(r). withExpiredPaySession(r, w) - now := time.Now() - donorStore := newMockDonorStore(t) donorStore. On("Put", r.Context(), &page.Lpa{ @@ -107,25 +102,7 @@ func TestGetPaymentConfirmationHalfFee(t *testing.T) { }). Return(nil) - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{ - {Key: "evidence-key"}, - {Key: "another-evidence-key", Sent: time.Date(2000, 1, 2, 0, 0, 0, 0, time.UTC)}, - }, nil) - documentStore. - On("Put", r.Context(), page.Document{Key: "evidence-key", Sent: now}). - Return(nil) - - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", r.Context(), "evidence-key", []types.Tag{ - {Key: aws.String("replicate"), Value: aws.String("true")}, - }). - Return(nil) - - err := PaymentConfirmation(newMockLogger(t), template.Execute, payClient, donorStore, sessionStore, s3Client, func() time.Time { return now }, documentStore)(testAppData, w, r, &page.Lpa{ + err := PaymentConfirmation(newMockLogger(t), template.Execute, payClient, donorStore, sessionStore)(testAppData, w, r, &page.Lpa{ FeeType: pay.HalfFee, CertificateProvider: actor.CertificateProvider{ Email: "certificateprovider@example.com", @@ -148,7 +125,7 @@ func TestGetPaymentConfirmationWhenErrorGettingSession(t *testing.T) { On("Get", r, "pay"). Return(&sessions.Session{}, expectedError) - err := PaymentConfirmation(nil, template.Execute, newMockPayClient(t), nil, sessionStore, nil, nil, nil)(testAppData, w, r, &page.Lpa{}) + err := PaymentConfirmation(nil, template.Execute, newMockPayClient(t), nil, sessionStore)(testAppData, w, r, &page.Lpa{}) resp := w.Result() assert.Equal(t, expectedError, err) @@ -174,7 +151,7 @@ func TestGetPaymentConfirmationWhenErrorGettingPayment(t *testing.T) { template := newMockTemplate(t) - err := PaymentConfirmation(logger, template.Execute, payClient, nil, sessionStore, nil, nil, nil)(testAppData, w, r, &page.Lpa{}) + err := PaymentConfirmation(logger, template.Execute, payClient, nil, sessionStore)(testAppData, w, r, &page.Lpa{}) resp := w.Result() assert.Equal(t, expectedError, err) @@ -208,7 +185,7 @@ func TestGetPaymentConfirmationWhenErrorExpiringSession(t *testing.T) { On("Execute", w, mock.Anything). Return(nil) - err := PaymentConfirmation(logger, template.Execute, payClient, donorStore, sessionStore, nil, nil, nil)(testAppData, w, r, &page.Lpa{CertificateProvider: actor.CertificateProvider{ + err := PaymentConfirmation(logger, template.Execute, payClient, donorStore, sessionStore)(testAppData, w, r, &page.Lpa{CertificateProvider: actor.CertificateProvider{ Email: "certificateprovider@example.com", }}) resp := w.Result() @@ -217,113 +194,6 @@ func TestGetPaymentConfirmationWhenErrorExpiringSession(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode) } -func TestGetPaymentConfirmationHalfFeeWhenDocumentStoreGetAllError(t *testing.T) { - w := httptest.NewRecorder() - r, _ := http.NewRequest(http.MethodGet, "/payment-confirmation", nil) - - payClient := newMockPayClient(t). - withASuccessfulPayment("abc123", "123456789012", 4100) - - sessionStore := newMockSessionStore(t). - withPaySession(r). - withExpiredPaySession(r, w) - - now := time.Now() - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{}, expectedError) - - err := PaymentConfirmation(nil, nil, payClient, nil, sessionStore, nil, func() time.Time { return now }, documentStore)(testAppData, w, r, &page.Lpa{ - FeeType: pay.HalfFee, - CertificateProvider: actor.CertificateProvider{ - Email: "certificateprovider@example.com", - }, - }) - resp := w.Result() - - assert.Equal(t, expectedError, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestGetPaymentConfirmationHalfFeeWhenS3ClientPutTaggingObjectError(t *testing.T) { - w := httptest.NewRecorder() - r, _ := http.NewRequest(http.MethodGet, "/payment-confirmation", nil) - - payClient := newMockPayClient(t). - withASuccessfulPayment("abc123", "123456789012", 4100) - - sessionStore := newMockSessionStore(t). - withPaySession(r). - withExpiredPaySession(r, w) - - now := time.Now() - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{{}}, nil) - - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", mock.Anything, mock.Anything, mock.Anything). - Return(expectedError) - - logger := newMockLogger(t) - logger. - On("Print", fmt.Sprintf("error tagging evidence: %s", expectedError)) - - err := PaymentConfirmation(logger, nil, payClient, nil, sessionStore, s3Client, func() time.Time { return now }, documentStore)(testAppData, w, r, &page.Lpa{ - FeeType: pay.HalfFee, - CertificateProvider: actor.CertificateProvider{ - Email: "certificateprovider@example.com", - }, - }) - resp := w.Result() - - assert.Equal(t, expectedError, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestGetPaymentConfirmationHalfFeeWhenDocumentStorePutError(t *testing.T) { - w := httptest.NewRecorder() - r, _ := http.NewRequest(http.MethodGet, "/payment-confirmation", nil) - - payClient := newMockPayClient(t). - withASuccessfulPayment("abc123", "123456789012", 4100) - - sessionStore := newMockSessionStore(t). - withPaySession(r). - withExpiredPaySession(r, w) - - now := time.Now() - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{{}}, nil) - documentStore. - On("Put", r.Context(), mock.Anything). - Return(expectedError) - - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", mock.Anything, mock.Anything, mock.Anything). - Return(nil) - - err := PaymentConfirmation(nil, nil, payClient, nil, sessionStore, s3Client, func() time.Time { return now }, documentStore)(testAppData, w, r, &page.Lpa{ - FeeType: pay.HalfFee, - CertificateProvider: actor.CertificateProvider{ - Email: "certificateprovider@example.com", - }, - }) - resp := w.Result() - - assert.Equal(t, expectedError, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) -} - func TestGetPaymentConfirmationHalfFeeWhenDonorStorePutError(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodGet, "/payment-confirmation", nil) @@ -335,31 +205,16 @@ func TestGetPaymentConfirmationHalfFeeWhenDonorStorePutError(t *testing.T) { withPaySession(r). withExpiredPaySession(r, w) - now := time.Now() - donorStore := newMockDonorStore(t) donorStore. On("Put", r.Context(), mock.Anything). Return(expectedError) - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{{}}, nil) - documentStore. - On("Put", r.Context(), mock.Anything). - Return(nil) - - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", mock.Anything, mock.Anything, mock.Anything). - Return(nil) - logger := newMockLogger(t) logger. On("Print", fmt.Sprintf("unable to update lpa in donorStore: %s", expectedError)) - err := PaymentConfirmation(logger, nil, payClient, donorStore, sessionStore, s3Client, func() time.Time { return now }, documentStore)(testAppData, w, r, &page.Lpa{ + err := PaymentConfirmation(logger, nil, payClient, donorStore, sessionStore)(testAppData, w, r, &page.Lpa{ FeeType: pay.HalfFee, CertificateProvider: actor.CertificateProvider{ Email: "certificateprovider@example.com", diff --git a/internal/page/donor/register.go b/internal/page/donor/register.go index 6b85c548c0..fcdc08b641 100644 --- a/internal/page/donor/register.go +++ b/internal/page/donor/register.go @@ -9,11 +9,10 @@ import ( "strings" "time" - "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/gorilla/sessions" "github.com/ministryofjustice/opg-go-common/template" "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" + "github.com/ministryofjustice/opg-modernising-lpa/internal/event" "github.com/ministryofjustice/opg-modernising-lpa/internal/identity" "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/ministryofjustice/opg-modernising-lpa/internal/onelogin" @@ -65,7 +64,6 @@ type EvidenceReceivedStore interface { //go:generate mockery --testonly --inpackage --name S3Client --structname mockS3Client type S3Client interface { PutObject(context.Context, string, []byte) error - PutObjectTagging(context.Context, string, []types.Tag) error DeleteObject(context.Context, string) error } @@ -148,6 +146,12 @@ type DocumentStore interface { Delete(context.Context, page.Document) error DeleteInfectedDocuments(context.Context, page.Documents) error Create(context.Context, *page.Lpa, string, []byte) (page.Document, error) + Submit(context.Context, *page.Lpa, page.Documents) error +} + +//go:generate mockery --testonly --inpackage --name EventClient --structname mockEventClient +type EventClient interface { + SendReducedFeeRequested(context.Context, event.ReducedFeeRequested) error } func Register( @@ -170,17 +174,15 @@ func Register( evidenceReceivedStore EvidenceReceivedStore, evidenceS3Client S3Client, documentStore DocumentStore, + eventClient EventClient, ) { payer := &payHelper{ - logger: logger, - sessionStore: sessionStore, - donorStore: donorStore, - payClient: payClient, - appPublicURL: appPublicURL, - randomString: random.String, - evidenceS3Client: evidenceS3Client, - now: time.Now, - documentStore: documentStore, + logger: logger, + sessionStore: sessionStore, + donorStore: donorStore, + payClient: payClient, + appPublicURL: appPublicURL, + randomString: random.String, } handleRoot := makeHandle(rootMux, sessionStore, None, errorHandler) @@ -321,13 +323,13 @@ func Register( handleWithLpa(page.Paths.HowWouldYouLikeToSendEvidence, CanGoBack, HowWouldYouLikeToSendEvidence(tmpls.Get("how_would_you_like_to_send_evidence.gohtml"), donorStore)) handleWithLpa(page.Paths.UploadEvidence, CanGoBack, - UploadEvidence(tmpls.Get("upload_evidence.gohtml"), payer, documentStore)) + UploadEvidence(tmpls.Get("upload_evidence.gohtml"), logger, payer, documentStore)) handleWithLpa(page.Paths.SendUsYourEvidenceByPost, CanGoBack, - SendUsYourEvidenceByPost(tmpls.Get("send_us_your_evidence_by_post.gohtml"), payer)) + SendUsYourEvidenceByPost(tmpls.Get("send_us_your_evidence_by_post.gohtml"), payer, eventClient)) handleWithLpa(page.Paths.FeeDenied, None, FeeDenied(tmpls.Get("fee_denied.gohtml"), payer)) handleWithLpa(page.Paths.PaymentConfirmation, None, - PaymentConfirmation(logger, tmpls.Get("payment_confirmation.gohtml"), payClient, donorStore, sessionStore, evidenceS3Client, time.Now, documentStore)) + PaymentConfirmation(logger, tmpls.Get("payment_confirmation.gohtml"), payClient, donorStore, sessionStore)) handleWithLpa(page.Paths.EvidenceSuccessfullyUploaded, None, Guidance(tmpls.Get("evidence_successfully_uploaded.gohtml"))) handleWithLpa(page.Paths.WhatHappensNextPostEvidence, None, @@ -468,15 +470,12 @@ func makeLpaHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOp } type payHelper struct { - logger Logger - sessionStore sessions.Store - donorStore DonorStore - payClient PayClient - appPublicURL string - randomString func(int) string - evidenceS3Client S3Client - now func() time.Time - documentStore DocumentStore + logger Logger + sessionStore sessions.Store + donorStore DonorStore + payClient PayClient + appPublicURL string + randomString func(int) string } func (p *payHelper) Pay(appData page.AppData, w http.ResponseWriter, r *http.Request, lpa *page.Lpa) error { @@ -490,27 +489,6 @@ func (p *payHelper) Pay(appData page.AppData, w http.ResponseWriter, r *http.Req return appData.Redirect(w, r, lpa, page.Paths.WhatHappensNextPostEvidence.Format(lpa.ID)) } - documents, err := p.documentStore.GetAll(r.Context()) - if err != nil { - return err - } - - for _, document := range documents { - if document.Sent.IsZero() { - if err := p.evidenceS3Client.PutObjectTagging(r.Context(), document.Key, []types.Tag{ - {Key: aws.String("replicate"), Value: aws.String("true")}, - }); err != nil { - p.logger.Print(fmt.Sprintf("error tagging evidence: %s", err.Error())) - return err - } - - document.Sent = p.now() - if err := p.documentStore.Put(r.Context(), document); err != nil { - return err - } - } - } - return appData.Redirect(w, r, lpa, page.Paths.EvidenceSuccessfullyUploaded.Format(lpa.ID)) } diff --git a/internal/page/donor/register_test.go b/internal/page/donor/register_test.go index 78f693593a..6fb20242a2 100644 --- a/internal/page/donor/register_test.go +++ b/internal/page/donor/register_test.go @@ -2,15 +2,11 @@ package donor import ( "context" - "fmt" "log" "net/http" "net/http/httptest" "testing" - "time" - "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/gorilla/sessions" "github.com/ministryofjustice/opg-go-common/template" "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" @@ -20,7 +16,6 @@ import ( "github.com/ministryofjustice/opg-modernising-lpa/internal/page" "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" "github.com/ministryofjustice/opg-modernising-lpa/internal/place" - "github.com/ministryofjustice/opg-modernising-lpa/internal/s3" "github.com/ministryofjustice/opg-modernising-lpa/internal/sesh" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -28,7 +23,7 @@ import ( func TestRegister(t *testing.T) { mux := http.NewServeMux() - Register(mux, &log.Logger{}, template.Templates{}, nil, nil, &onelogin.Client{}, &place.Client{}, "http://public.url", &pay.Client{}, nil, &mockWitnessCodeSender{}, nil, nil, nil, nil, ¬ify.Client{}, nil, &s3.Client{}, nil) + Register(mux, &log.Logger{}, template.Templates{}, nil, nil, &onelogin.Client{}, &place.Client{}, "http://public.url", &pay.Client{}, nil, &mockWitnessCodeSender{}, nil, nil, nil, nil, ¬ify.Client{}, nil, nil, nil, nil) assert.Implements(t, (*http.Handler)(nil), mux) } @@ -436,21 +431,6 @@ func TestPayHelperPayWhenPaymentNotRequired(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", nil) - now := time.Now() - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{ - {Key: "lpa-uid/evidence/a-uid", Filename: "dummy.pdf", Sent: now}, - {Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png"}, - }, nil) - documentStore. - On("Put", r.Context(), page.Document{ - Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png", Sent: now, - }). - Return(nil) - donorStore := newMockDonorStore(t) donorStore. On("Put", r.Context(), &page.Lpa{ @@ -461,18 +441,8 @@ func TestPayHelperPayWhenPaymentNotRequired(t *testing.T) { }). Return(nil) - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", r.Context(), "lpa-uid/evidence/another-uid", []types.Tag{ - {Key: aws.String("replicate"), Value: aws.String("true")}, - }). - Return(nil) - err := (&payHelper{ - donorStore: donorStore, - now: func() time.Time { return now }, - evidenceS3Client: s3Client, - documentStore: documentStore, + donorStore: donorStore, }).Pay(testAppData, w, r, &page.Lpa{ ID: "lpa-id", FeeType: feeType, @@ -498,8 +468,6 @@ func TestPayHelperPayWhenPostingEvidence(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", nil) - now := time.Now() - donorStore := newMockDonorStore(t) donorStore. On("Put", r.Context(), &page.Lpa{ @@ -512,7 +480,6 @@ func TestPayHelperPayWhenPostingEvidence(t *testing.T) { err := (&payHelper{ donorStore: donorStore, - now: func() time.Time { return now }, }).Pay(testAppData, w, r, &page.Lpa{ ID: "lpa-id", FeeType: feeType, @@ -531,21 +498,6 @@ func TestPayHelperPayWhenMoreEvidenceProvided(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", nil) - now := time.Now() - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{ - {Key: "lpa-uid/evidence/a-uid", Filename: "dummy.pdf", Sent: now}, - {Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png"}, - }, nil) - documentStore. - On("Put", r.Context(), page.Document{ - Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png", Sent: now, - }). - Return(nil) - donorStore := newMockDonorStore(t) donorStore. On("Put", r.Context(), &page.Lpa{ @@ -556,18 +508,8 @@ func TestPayHelperPayWhenMoreEvidenceProvided(t *testing.T) { }). Return(nil) - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", r.Context(), "lpa-uid/evidence/another-uid", []types.Tag{ - {Key: aws.String("replicate"), Value: aws.String("true")}, - }). - Return(nil) - err := (&payHelper{ - donorStore: donorStore, - now: func() time.Time { return now }, - evidenceS3Client: s3Client, - documentStore: documentStore, + donorStore: donorStore, }).Pay(testAppData, w, r, &page.Lpa{ ID: "lpa-id", FeeType: pay.HalfFee, @@ -581,128 +523,10 @@ func TestPayHelperPayWhenMoreEvidenceProvided(t *testing.T) { assert.Equal(t, page.Paths.EvidenceSuccessfullyUploaded.Format("lpa-id"), resp.Header.Get("Location")) } -func TestPayHelperPayWhenPaymentNotRequiredWhenDocumentStoreGetAllError(t *testing.T) { - w := httptest.NewRecorder() - r, _ := http.NewRequest(http.MethodPost, "/", nil) - - donorStore := newMockDonorStore(t) - donorStore. - On("Put", r.Context(), mock.Anything). - Return(nil) - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{}, expectedError) - - err := (&payHelper{documentStore: documentStore, donorStore: donorStore}).Pay(testAppData, w, r, &page.Lpa{ - ID: "lpa-id", - FeeType: pay.NoFee, - }) - resp := w.Result() - - assert.Equal(t, expectedError, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestPayHelperPayWhenPaymentNotRequiredWhenS3ClientPutObjectTaggingError(t *testing.T) { - w := httptest.NewRecorder() - r, _ := http.NewRequest(http.MethodPost, "/", nil) - - now := time.Now() - - donorStore := newMockDonorStore(t) - donorStore. - On("Put", r.Context(), mock.Anything). - Return(nil) - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{ - {Key: "lpa-uid/evidence/a-uid", Filename: "dummy.pdf", Sent: now}, - {Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png"}, - }, nil) - - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", r.Context(), "lpa-uid/evidence/another-uid", []types.Tag{ - {Key: aws.String("replicate"), Value: aws.String("true")}, - }). - Return(expectedError) - - logger := newMockLogger(t) - logger. - On("Print", fmt.Sprintf("error tagging evidence: %s", expectedError)) - - err := (&payHelper{ - logger: logger, - now: func() time.Time { return now }, - donorStore: donorStore, - evidenceS3Client: s3Client, - documentStore: documentStore, - }).Pay(testAppData, w, r, &page.Lpa{ - ID: "lpa-id", - FeeType: pay.NoFee, - }) - resp := w.Result() - - assert.Equal(t, expectedError, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestPayHelperPayWhenPaymentNotRequiredWhenDocumentStorePutError(t *testing.T) { - w := httptest.NewRecorder() - r, _ := http.NewRequest(http.MethodPost, "/", nil) - - now := time.Now() - - donorStore := newMockDonorStore(t) - donorStore. - On("Put", r.Context(), mock.Anything). - Return(nil) - - documentStore := newMockDocumentStore(t) - documentStore. - On("GetAll", r.Context()). - Return(page.Documents{ - {Key: "lpa-uid/evidence/a-uid", Filename: "dummy.pdf", Sent: now}, - {Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png"}, - }, nil) - documentStore. - On("Put", r.Context(), page.Document{ - Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png", Sent: now, - }). - Return(expectedError) - - s3Client := newMockS3Client(t) - s3Client. - On("PutObjectTagging", r.Context(), "lpa-uid/evidence/another-uid", []types.Tag{ - {Key: aws.String("replicate"), Value: aws.String("true")}, - }). - Return(nil) - - err := (&payHelper{ - now: func() time.Time { return now }, - donorStore: donorStore, - evidenceS3Client: s3Client, - documentStore: documentStore, - }).Pay(testAppData, w, r, &page.Lpa{ - ID: "lpa-id", - FeeType: pay.NoFee, - }) - resp := w.Result() - - assert.Equal(t, expectedError, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) -} - func TestPayHelperPayWhenPaymentNotRequiredWhenDonorStorePutError(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", nil) - now := time.Now() - donorStore := newMockDonorStore(t) donorStore. On("Put", r.Context(), &page.Lpa{ @@ -714,7 +538,6 @@ func TestPayHelperPayWhenPaymentNotRequiredWhenDonorStorePutError(t *testing.T) err := (&payHelper{ donorStore: donorStore, - now: func() time.Time { return now }, }).Pay(testAppData, w, r, &page.Lpa{ ID: "lpa-id", FeeType: pay.NoFee, diff --git a/internal/page/donor/send_us_your_evidence_by_post.go b/internal/page/donor/send_us_your_evidence_by_post.go index 8d7b60dcc0..1f639745f7 100644 --- a/internal/page/donor/send_us_your_evidence_by_post.go +++ b/internal/page/donor/send_us_your_evidence_by_post.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/ministryofjustice/opg-go-common/template" + "github.com/ministryofjustice/opg-modernising-lpa/internal/event" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" "github.com/ministryofjustice/opg-modernising-lpa/internal/validation" @@ -15,7 +16,7 @@ type sendUsYourEvidenceByPostData struct { FeeType pay.FeeType } -func SendUsYourEvidenceByPost(tmpl template.Template, payer Payer) Handler { +func SendUsYourEvidenceByPost(tmpl template.Template, payer Payer, eventClient EventClient) Handler { return func(appData page.AppData, w http.ResponseWriter, r *http.Request, lpa *page.Lpa) error { data := &sendUsYourEvidenceByPostData{ App: appData, @@ -23,6 +24,14 @@ func SendUsYourEvidenceByPost(tmpl template.Template, payer Payer) Handler { } if r.Method == http.MethodPost { + if err := eventClient.SendReducedFeeRequested(r.Context(), event.ReducedFeeRequested{ + UID: lpa.UID, + RequestType: lpa.FeeType.String(), + EvidenceDelivery: lpa.EvidenceDelivery.String(), + }); err != nil { + return err + } + return payer.Pay(appData, w, r, lpa) } diff --git a/internal/page/donor/send_us_your_evidence_by_post_test.go b/internal/page/donor/send_us_your_evidence_by_post_test.go index 0d46c83e60..5c5f59b027 100644 --- a/internal/page/donor/send_us_your_evidence_by_post_test.go +++ b/internal/page/donor/send_us_your_evidence_by_post_test.go @@ -5,8 +5,9 @@ import ( "net/http/httptest" "testing" - "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" + "github.com/ministryofjustice/opg-modernising-lpa/internal/event" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" + "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -20,7 +21,7 @@ func TestGetSendUsYourEvidenceByPost(t *testing.T) { On("Execute", w, &sendUsYourEvidenceByPostData{App: testAppData}). Return(nil) - err := SendUsYourEvidenceByPost(template.Execute, nil)(testAppData, w, r, &page.Lpa{}) + err := SendUsYourEvidenceByPost(template.Execute, nil, nil)(testAppData, w, r, &page.Lpa{}) resp := w.Result() assert.Nil(t, err) @@ -36,7 +37,7 @@ func TestGetSendUsYourEvidenceByPostWhenTemplateErrors(t *testing.T) { On("Execute", w, &sendUsYourEvidenceByPostData{App: testAppData}). Return(expectedError) - err := SendUsYourEvidenceByPost(template.Execute, nil)(testAppData, w, r, &page.Lpa{}) + err := SendUsYourEvidenceByPost(template.Execute, nil, nil)(testAppData, w, r, &page.Lpa{}) resp := w.Result() assert.Equal(t, expectedError, err) @@ -47,26 +48,53 @@ func TestPostSendUsYourEvidenceByPost(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/about-payment", nil) - lpa := &page.Lpa{ID: "lpa-id", Donor: actor.Donor{Email: "a@b.com"}} + lpa := &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Post} + + eventClient := newMockEventClient(t) + eventClient. + On("SendReducedFeeRequested", r.Context(), event.ReducedFeeRequested{ + UID: "lpa-uid", + RequestType: pay.HalfFee.String(), + EvidenceDelivery: pay.Post.String(), + }). + Return(nil) payer := newMockPayer(t) payer. On("Pay", testAppData, w, r, lpa). Return(nil) - err := SendUsYourEvidenceByPost(nil, payer)(testAppData, w, r, lpa) + err := SendUsYourEvidenceByPost(nil, payer, eventClient)(testAppData, w, r, lpa) assert.Nil(t, err) } +func TestPostSendUsYourEvidenceByPostWhenEventClientErrors(t *testing.T) { + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/about-payment", nil) + + eventClient := newMockEventClient(t) + eventClient. + On("SendReducedFeeRequested", r.Context(), mock.Anything). + Return(expectedError) + + err := SendUsYourEvidenceByPost(nil, nil, eventClient)(testAppData, w, r, &page.Lpa{}) + assert.Equal(t, expectedError, err) +} + func TestPostSendUsYourEvidenceByPostWhenPayerErrors(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/about-payment", nil) + eventClient := newMockEventClient(t) + eventClient. + On("SendReducedFeeRequested", r.Context(), mock.Anything). + Return(nil) + payer := newMockPayer(t) payer. On("Pay", testAppData, w, r, mock.Anything). Return(expectedError) - err := SendUsYourEvidenceByPost(nil, payer)(testAppData, w, r, &page.Lpa{}) + err := SendUsYourEvidenceByPost(nil, payer, eventClient)(testAppData, w, r, &page.Lpa{}) assert.Equal(t, expectedError, err) } diff --git a/internal/page/donor/upload_evidence.go b/internal/page/donor/upload_evidence.go index 390dadd048..be648480f3 100644 --- a/internal/page/donor/upload_evidence.go +++ b/internal/page/donor/upload_evidence.go @@ -57,7 +57,7 @@ type uploadEvidenceData struct { StartScan string } -func UploadEvidence(tmpl template.Template, payer Payer, documentStore DocumentStore) Handler { +func UploadEvidence(tmpl template.Template, logger Logger, payer Payer, documentStore DocumentStore) Handler { return func(appData page.AppData, w http.ResponseWriter, r *http.Request, lpa *page.Lpa) error { if lpa.Tasks.PayForLpa.IsPending() { return appData.Redirect(w, r, lpa, page.Paths.TaskList.Format(lpa.ID)) @@ -118,6 +118,16 @@ func UploadEvidence(tmpl template.Template, payer Payer, documentStore DocumentS } case "pay": + if len(documents.NotScanned()) > 0 { + logger.Print("attempt to pay with unscanned documents on lpa:", lpa.UID) + data.Errors = validation.With("upload", validation.CustomError{Label: "errorGenericUploadProblem"}) + return tmpl(w, data) + } + + if err := documentStore.Submit(r.Context(), lpa, documents); err != nil { + return err + } + return payer.Pay(appData, w, r, lpa) case "delete": diff --git a/internal/page/donor/upload_evidence_test.go b/internal/page/donor/upload_evidence_test.go index ceabec6d36..bda8a59ef8 100644 --- a/internal/page/donor/upload_evidence_test.go +++ b/internal/page/donor/upload_evidence_test.go @@ -10,15 +10,21 @@ import ( "os" "strings" "testing" + "time" "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" - pay "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" + "github.com/ministryofjustice/opg-modernising-lpa/internal/pay" "github.com/ministryofjustice/opg-modernising-lpa/internal/validation" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) +var ( + testNow = time.Now() + testNowFn = func() time.Time { return testNow } +) + func TestGetUploadEvidence(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodGet, "/", nil) @@ -39,7 +45,7 @@ func TestGetUploadEvidence(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{FeeType: pay.FullFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{FeeType: pay.FullFee}) resp := w.Result() assert.Nil(t, err) @@ -50,7 +56,7 @@ func TestGetUploadEvidenceWhenTaskPending(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodGet, "/", nil) - err := UploadEvidence(nil, nil, nil)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee, Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}}) + err := UploadEvidence(nil, nil, nil, nil)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee, Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}}) resp := w.Result() assert.Nil(t, err) @@ -72,7 +78,7 @@ func TestGetUploadEvidenceWhenTemplateErrors(t *testing.T) { On("Execute", w, mock.Anything). Return(expectedError) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{}) assert.Equal(t, expectedError, err) } @@ -92,23 +98,11 @@ func TestPostUploadEvidenceWithUploadActionAcceptedFileTypes(t *testing.T) { for _, filename := range testCases { t.Run(filename, func(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "upload") - - file := addFileToUploadField(writer, filename) - - writer.Close() - file.Close() + buf, contentType := multipartAction("upload", filename) w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -140,7 +134,7 @@ func TestPostUploadEvidenceWithUploadActionAcceptedFileTypes(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Nil(t, err) }) } @@ -150,7 +144,7 @@ func TestPostUploadEvidenceWhenTaskPending(t *testing.T) { w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", nil) - err := UploadEvidence(nil, nil, nil)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee, Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}}) + err := UploadEvidence(nil, nil, nil, nil)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee, Tasks: page.Tasks{PayForLpa: actor.PaymentTaskPending}}) resp := w.Result() assert.Nil(t, err) @@ -159,24 +153,11 @@ func TestPostUploadEvidenceWhenTaskPending(t *testing.T) { } func TestPostUploadEvidenceWithUploadActionMultipleFiles(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "upload") - - file := addFileToUploadField(writer, "dummy.pdf") - file = addFileToUploadField(writer, "dummy.png") - - writer.Close() - file.Close() + buf, contentType := multipartAction("upload", "dummy.pdf", "dummy.png") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -226,7 +207,7 @@ func TestPostUploadEvidenceWithUploadActionMultipleFiles(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Nil(t, err) } @@ -283,87 +264,147 @@ func TestPostUploadEvidenceWithUploadActionFilenameSpecialCharactersAreEscaped(t }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Nil(t, err) } func TestPostUploadEvidenceWithPayAction(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "pay") - - writer.Close() + buf, contentType := multipartAction("pay") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) + + lpa := &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} + + documents := page.Documents{{ + PK: "LPA#lpa-id", + SK: "#DOCUMENT#lpa-uid/evidence/a-uid", + Filename: "safe.file", + Key: "lpa-uid/evidence/a-uid", + Scanned: true, + }, { + PK: "LPA#lpa-id", + SK: "#DOCUMENT#lpa-uid/evidence/with-virus", + Filename: "virus.file", + Key: "lpa-uid/evidence/with-virus", + Scanned: true, + VirusDetected: true, + }} documentStore := newMockDocumentStore(t) documentStore. On("GetAll", r.Context()). - Return(page.Documents{}, nil) + Return(documents, nil) + documentStore. + On("Submit", r.Context(), lpa, documents). + Return(nil) payer := newMockPayer(t) payer. - On("Pay", testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}). + On("Pay", testAppData, w, r, lpa). Return(nil) - err := UploadEvidence(nil, payer, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(nil, nil, payer, documentStore)(testAppData, w, r, lpa) assert.Nil(t, err) } func TestPostUploadEvidenceWithPayActionWhenPayerError(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "pay") - - writer.Close() + buf, contentType := multipartAction("pay") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. On("GetAll", r.Context()). Return(page.Documents{}, nil) + documentStore. + On("Submit", r.Context(), mock.Anything, mock.Anything). + Return(nil) payer := newMockPayer(t) payer. On("Pay", testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}). Return(expectedError) - err := UploadEvidence(nil, payer, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(nil, nil, payer, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Equal(t, expectedError, err) } -func TestPostUploadEvidenceWithScanResultsActionWithInfectedFiles(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) +func TestPostUploadEvidenceWithPayActionWhenDocumentStoreSubmitErrors(t *testing.T) { + buf, contentType := multipartAction("pay") - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/", &buf) + r.Header.Set("Content-Type", contentType) - part, _ = writer.CreateFormField("action") - io.WriteString(part, "scanResults") + lpa := &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} - writer.Close() + documentStore := newMockDocumentStore(t) + documentStore. + On("GetAll", r.Context()). + Return(page.Documents{{ + PK: "LPA#lpa-id", + SK: "#DOCUMENT#lpa-uid/evidence/a-uid", + Filename: "safe.file", + Key: "lpa-uid/evidence/a-uid", + Scanned: true, + }}, nil) + documentStore. + On("Submit", r.Context(), mock.Anything, mock.Anything). + Return(expectedError) + + err := UploadEvidence(nil, nil, nil, documentStore)(testAppData, w, r, lpa) + assert.Equal(t, expectedError, err) +} + +func TestPostUploadEvidenceWithPayActionWhenUnscannedDocument(t *testing.T) { + buf, contentType := multipartAction("pay") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) + + lpa := &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee, EvidenceDelivery: pay.Upload} + + documentStore := newMockDocumentStore(t) + documentStore. + On("GetAll", r.Context()). + Return(page.Documents{{ + PK: "LPA#lpa-id", + SK: "#DOCUMENT#lpa-uid/evidence/a-uid", + Filename: "safe.file", + Key: "lpa-uid/evidence/a-uid", + }}, nil) + + template := newMockTemplate(t) + template. + On("Execute", w, mock.MatchedBy(func(data *uploadEvidenceData) bool { + return assert.Equal(t, validation.With("upload", validation.CustomError{Label: "errorGenericUploadProblem"}), data.Errors) + })). + Return(nil) + + logger := newMockLogger(t) + logger. + On("Print", "attempt to pay with unscanned documents on lpa:", "lpa-uid") + + err := UploadEvidence(template.Execute, logger, nil, documentStore)(testAppData, w, r, lpa) + resp := w.Result() + + assert.Nil(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + +func TestPostUploadEvidenceWithScanResultsActionWithInfectedFiles(t *testing.T) { + buf, contentType := multipartAction("scanResults") + + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/", &buf) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -402,25 +443,16 @@ func TestPostUploadEvidenceWithScanResultsActionWithInfectedFiles(t *testing.T) }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Nil(t, err) } func TestPostUploadEvidenceWithScanResultsActionWithoutInfectedFiles(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "scanResults") - - writer.Close() + buf, contentType := multipartAction("scanResults") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -440,25 +472,16 @@ func TestPostUploadEvidenceWithScanResultsActionWithoutInfectedFiles(t *testing. }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Nil(t, err) } func TestPostUploadEvidenceWithPayActionWithInfectedFilesWhenDocumentStoreGetAllErrors(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "scanResults") - - writer.Close() + buf, contentType := multipartAction("scanResults") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -470,25 +493,16 @@ func TestPostUploadEvidenceWithPayActionWithInfectedFilesWhenDocumentStoreGetAll {Filename: "d", VirusDetected: true}, }, expectedError) - err := UploadEvidence(nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(nil, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Equal(t, expectedError, err) } func TestPostUploadEvidenceWithScanResultsActionWithInfectedFilesWhenDocumentStoreDeleteInfectedDocumentsError(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "scanResults") - - writer.Close() + buf, contentType := multipartAction("scanResults") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -508,25 +522,16 @@ func TestPostUploadEvidenceWithScanResultsActionWithInfectedFilesWhenDocumentSto }). Return(expectedError) - err := UploadEvidence(nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(nil, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Equal(t, expectedError, err) } func TestPostUploadEvidenceWithScanResultsActionWithInfectedFilesWhenDocumentStoreGetAllAgainError(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "scanResults") - - writer.Close() + buf, contentType := multipartAction("scanResults") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -553,25 +558,16 @@ func TestPostUploadEvidenceWithScanResultsActionWithInfectedFilesWhenDocumentSto }, expectedError). Once() - err := UploadEvidence(nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(nil, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Equal(t, expectedError, err) } func TestPostUploadEvidenceWithScanResultsActionWithInfectedFilesWhenTemplateError(t *testing.T) { - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, _ := writer.CreateFormField("csrf") - io.WriteString(part, "123") - - part, _ = writer.CreateFormField("action") - io.WriteString(part, "scanResults") - - writer.Close() + buf, contentType := multipartAction("scanResults") w := httptest.NewRecorder() r, _ := http.NewRequest(http.MethodPost, "/", &buf) - r.Header.Set("Content-Type", writer.FormDataContentType()) + r.Header.Set("Content-Type", contentType) documentStore := newMockDocumentStore(t) documentStore. @@ -610,7 +606,7 @@ func TestPostUploadEvidenceWithScanResultsActionWithInfectedFilesWhenTemplateErr }). Return(expectedError) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", UID: "lpa-uid", FeeType: pay.HalfFee}) assert.Equal(t, expectedError, err) } @@ -645,7 +641,7 @@ func TestPostUploadEvidenceWhenBadCsrfField(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee}) resp := w.Result() assert.Nil(t, err) @@ -685,7 +681,7 @@ func TestPostUploadEvidenceWhenBadActionField(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee}) resp := w.Result() assert.Nil(t, err) @@ -733,7 +729,7 @@ func TestPostUploadEvidenceNumberOfFilesLimitPassed(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.FullFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.FullFee}) assert.Nil(t, err) } @@ -808,7 +804,7 @@ func TestPostUploadEvidenceWhenBadUpload(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{ID: "lpa-id", FeeType: pay.FullFee}) resp := w.Result() assert.Nil(t, err) @@ -861,7 +857,7 @@ func TestGetUploadEvidenceDeleteEvidence(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{}) assert.Nil(t, err) } @@ -906,7 +902,7 @@ func TestGetUploadEvidenceDeleteEvidenceWhenUnexpectedFieldName(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{}) assert.Nil(t, err) } @@ -941,7 +937,7 @@ func TestGetUploadEvidenceDeleteEvidenceWhenDocumentStoreDeleteError(t *testing. On("Delete", r.Context(), page.Document{Key: "lpa-uid/evidence/a-uid", Filename: "dummy.pdf"}). Return(expectedError) - err := UploadEvidence(nil, nil, documentStore)(testAppData, w, r, &page.Lpa{}) + err := UploadEvidence(nil, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{}) assert.Equal(t, expectedError, err) } @@ -989,7 +985,7 @@ func TestGetUploadEvidenceDeleteEvidenceWhenTemplateError(t *testing.T) { }). Return(expectedError) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{}) assert.Equal(t, expectedError, err) } @@ -1034,7 +1030,7 @@ func TestPostUploadEvidenceWithCloseConnectionAction(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) resp := w.Result() @@ -1069,7 +1065,7 @@ func TestPostUploadEvidenceWithCloseConnectionActionWhenDocumentStoreDeleteError On("Delete", r.Context(), page.Document{Key: "lpa-uid/evidence/another-uid", Filename: "dummy.png", Scanned: false}). Return(expectedError) - err := UploadEvidence(nil, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(nil, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) resp := w.Result() assert.Equal(t, expectedError, err) @@ -1117,7 +1113,7 @@ func TestPostUploadEvidenceWithCloseConnectionActionWhenTemplateError(t *testing }). Return(expectedError) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) resp := w.Result() assert.Equal(t, expectedError, err) @@ -1164,16 +1160,37 @@ func TestPostUploadEvidenceWithCancelUploadAction(t *testing.T) { }). Return(nil) - err := UploadEvidence(template.Execute, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) + err := UploadEvidence(template.Execute, nil, nil, documentStore)(testAppData, w, r, &page.Lpa{UID: "lpa-uid", FeeType: pay.HalfFee}) resp := w.Result() assert.Nil(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) } +func multipartAction(action string, files ...string) (bytes.Buffer, string) { + var buf bytes.Buffer + writer := multipart.NewWriter(&buf) + + part, _ := writer.CreateFormField("csrf") + io.WriteString(part, "123") + + part, _ = writer.CreateFormField("action") + io.WriteString(part, action) + + for _, file := range files { + addFileToUploadField(writer, file) + } + + writer.Close() + return buf, writer.FormDataContentType() +} + func addFileToUploadField(writer *multipart.Writer, filename string) *os.File { file, _ := os.Open("testdata/" + filename) + defer file.Close() + part, _ := writer.CreateFormFile("upload", filename) io.Copy(part, file) + return file } diff --git a/internal/s3/client.go b/internal/s3/client.go index c32bd08522..bafb7622d5 100644 --- a/internal/s3/client.go +++ b/internal/s3/client.go @@ -12,10 +12,10 @@ import ( //go:generate mockery --testonly --inpackage --name s3Service --structname mockS3Service type s3Service interface { PutObject(context.Context, *s3.PutObjectInput, ...func(*s3.Options)) (*s3.PutObjectOutput, error) - PutObjectTagging(context.Context, *s3.PutObjectTaggingInput, ...func(*s3.Options)) (*s3.PutObjectTaggingOutput, error) DeleteObject(context.Context, *s3.DeleteObjectInput, ...func(*s3.Options)) (*s3.DeleteObjectOutput, error) GetObjectTagging(context.Context, *s3.GetObjectTaggingInput, ...func(*s3.Options)) (*s3.GetObjectTaggingOutput, error) DeleteObjects(context.Context, *s3.DeleteObjectsInput, ...func(*s3.Options)) (*s3.DeleteObjectsOutput, error) + PutObjectTagging(context.Context, *s3.PutObjectTaggingInput, ...func(*s3.Options)) (*s3.PutObjectTaggingOutput, error) } type Client struct { @@ -67,16 +67,6 @@ func (c *Client) GetObjectTags(ctx context.Context, key string) ([]types.Tag, er return output.TagSet, nil } -func (c *Client) PutObjectTagging(ctx context.Context, key string, tagSet []types.Tag) error { - _, err := c.svc.PutObjectTagging(ctx, &s3.PutObjectTaggingInput{ - Bucket: aws.String(c.bucket), - Key: aws.String(key), - Tagging: &types.Tagging{TagSet: tagSet}, - }) - - return err -} - func (c *Client) PutObject(ctx context.Context, key string, body []byte) error { _, err := c.svc.PutObject(ctx, &s3.PutObjectInput{ Bucket: aws.String(c.bucket), @@ -87,3 +77,18 @@ func (c *Client) PutObject(ctx context.Context, key string, body []byte) error { return err } + +func (c *Client) PutObjectTagging(ctx context.Context, key string, tags map[string]string) error { + tagSet := make([]types.Tag, 0, len(tags)) + for k, v := range tags { + tagSet = append(tagSet, types.Tag{Key: aws.String(k), Value: aws.String(v)}) + } + + _, err := c.svc.PutObjectTagging(ctx, &s3.PutObjectTaggingInput{ + Bucket: aws.String(c.bucket), + Key: aws.String(key), + Tagging: &types.Tagging{TagSet: tagSet}, + }) + + return err +} diff --git a/internal/s3/client_test.go b/internal/s3/client_test.go index ee1524ee05..78e8d4dede 100644 --- a/internal/s3/client_test.go +++ b/internal/s3/client_test.go @@ -10,6 +10,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/stretchr/testify/assert" + mock "github.com/stretchr/testify/mock" ) var expectedError = errors.New("err") @@ -44,48 +45,6 @@ func TestDeleteObjectOnServiceError(t *testing.T) { assert.Equal(t, expectedError, err) } -func TestPutObjectTagging(t *testing.T) { - tagSet := []types.Tag{ - {Key: aws.String("a-tag-key"), Value: aws.String("a-value")}, - {Key: aws.String("another-tag-key"), Value: aws.String("another-value")}, - } - - s3Service := newMockS3Service(t) - s3Service. - On("PutObjectTagging", context.Background(), &s3.PutObjectTaggingInput{ - Bucket: aws.String("a-bucket"), - Key: aws.String("a-object-key"), - Tagging: &types.Tagging{TagSet: tagSet}, - }). - Return(nil, nil) - - client := Client{bucket: "a-bucket", svc: s3Service} - err := client.PutObjectTagging(context.Background(), "a-object-key", tagSet) - - assert.Nil(t, err) -} - -func TestPutObjectTaggingOnServiceError(t *testing.T) { - tagSet := []types.Tag{ - {Key: aws.String("a-tag-key"), Value: aws.String("a-value")}, - {Key: aws.String("another-tag-key"), Value: aws.String("another-value")}, - } - - s3Service := newMockS3Service(t) - s3Service. - On("PutObjectTagging", context.Background(), &s3.PutObjectTaggingInput{ - Bucket: aws.String("a-bucket"), - Key: aws.String("a-object-key"), - Tagging: &types.Tagging{TagSet: tagSet}, - }). - Return(nil, expectedError) - - client := Client{bucket: "a-bucket", svc: s3Service} - err := client.PutObjectTagging(context.Background(), "a-object-key", tagSet) - - assert.Equal(t, expectedError, err) -} - func TestPutObject(t *testing.T) { s3Service := newMockS3Service(t) s3Service. @@ -106,12 +65,7 @@ func TestPutObject(t *testing.T) { func TestPutObjectOnServiceError(t *testing.T) { s3Service := newMockS3Service(t) s3Service. - On("PutObject", context.Background(), &s3.PutObjectInput{ - Bucket: aws.String("a-bucket"), - Key: aws.String("a-object-key"), - Body: bytes.NewReader([]byte("a-body")), - ServerSideEncryption: types.ServerSideEncryptionAwsKms, - }). + On("PutObject", context.Background(), mock.Anything). Return(nil, expectedError) client := Client{bucket: "a-bucket", svc: s3Service} @@ -157,3 +111,28 @@ func TestGetObjectTagsOnServiceError(t *testing.T) { assert.Equal(t, expectedError, err) } + +func TestPutObjectTagging(t *testing.T) { + s3Service := newMockS3Service(t) + s3Service. + On("PutObjectTagging", context.Background(), &s3.PutObjectTaggingInput{ + Bucket: aws.String("a-bucket"), + Key: aws.String("a-object-key"), + Tagging: &types.Tagging{ + TagSet: []types.Tag{ + {Key: aws.String("a-tag-key"), Value: aws.String("a-value")}, + {Key: aws.String("another-tag-key"), Value: aws.String("another-value")}, + }, + }, + }). + Return(nil, expectedError) + + client := Client{bucket: "a-bucket", svc: s3Service} + + err := client.PutObjectTagging(context.Background(), "a-object-key", map[string]string{ + "a-tag-key": "a-value", + "another-tag-key": "another-value", + }) + + assert.Equal(t, expectedError, err) +}