Skip to content

Commit

Permalink
MLPAB-2127: Resequence certificate provider enter reference number fo…
Browse files Browse the repository at this point in the history
…r fault tolerance (#1255)
  • Loading branch information
acsauk authored May 23, 2024
1 parent b5fcbfe commit 8e6f3f7
Show file tree
Hide file tree
Showing 16 changed files with 256 additions and 287 deletions.
2 changes: 1 addition & 1 deletion internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func App(
handleRoot(page.Paths.Fixtures, None,
fixtures.Donor(tmpls.Get("fixtures.gohtml"), sessionStore, donorStore, certificateProviderStore, attorneyStore, documentStore, eventClient, lpaStoreClient, shareCodeStore))
handleRoot(page.Paths.CertificateProviderFixtures, None,
fixtures.CertificateProvider(tmpls.Get("certificate_provider_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, eventClient, lpaStoreClient, lpaDynamoClient, organisationStore, memberStore))
fixtures.CertificateProvider(tmpls.Get("certificate_provider_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, eventClient, lpaStoreClient, lpaDynamoClient, organisationStore, memberStore, shareCodeStore))
handleRoot(page.Paths.AttorneyFixtures, None,
fixtures.Attorney(tmpls.Get("attorney_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, attorneyStore, eventClient, lpaStoreClient, organisationStore, memberStore, shareCodeStore))
handleRoot(page.Paths.SupporterFixtures, None,
Expand Down
2 changes: 1 addition & 1 deletion internal/app/attorney_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (s *attorneyStore) Create(ctx context.Context, shareCode actor.ShareCodeDat
ActorType: actor.TypeAttorney,
UpdatedAt: s.now(),
}).
Delete(shareCode.PK, shareCode.SK)
Delete(dynamo.Keys{PK: shareCode.PK, SK: shareCode.SK})

if err = s.dynamoClient.WriteTransaction(ctx, transaction); err != nil {
return nil, err
Expand Down
36 changes: 15 additions & 21 deletions internal/app/attorney_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"testing"
"time"

"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue"
"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
"github.com/ministryofjustice/opg-modernising-lpa/internal/actor/actoruid"
"github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo"
Expand All @@ -29,10 +27,8 @@ func TestAttorneyStoreCreate(t *testing.T) {

for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
data := &page.SessionData{LpaID: "123", SessionID: "456"}
ctx := page.ContextWithSessionData(context.Background(), data)
ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123", SessionID: "456"})
now := time.Now()
nowFormatted := now.Format(time.RFC3339Nano)
uid := actoruid.New()
details := &actor.AttorneyProvidedDetails{
PK: dynamo.LpaKey("123"),
Expand All @@ -55,24 +51,22 @@ func TestAttorneyStoreCreate(t *testing.T) {
LpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("donor")),
}

marshalledAttorney, _ := attributevalue.MarshalMap(details)

expectedTransaction := &dynamo.Transaction{
Puts: []*types.Put{
{Item: marshalledAttorney},
{Item: map[string]types.AttributeValue{
"PK": &types.AttributeValueMemberS{Value: "LPA#123"},
"SK": &types.AttributeValueMemberS{Value: "SUB#456"},
"DonorKey": &types.AttributeValueMemberS{Value: "DONOR#donor"},
"ActorType": &types.AttributeValueMemberN{Value: "2"},
"UpdatedAt": &types.AttributeValueMemberS{Value: nowFormatted},
}},
Puts: []any{
details,
lpaLink{
PK: dynamo.LpaKey("123"),
SK: dynamo.SubKey("456"),
DonorKey: dynamo.LpaOwnerKey(dynamo.DonorKey("donor")),
ActorType: actor.TypeAttorney,
UpdatedAt: now,
},
},
Deletes: []*types.Delete{
{Key: map[string]types.AttributeValue{
"PK": &types.AttributeValueMemberS{Value: shareCode.PK.PK()},
"SK": &types.AttributeValueMemberS{Value: shareCode.SK.SK()},
}},
Deletes: []dynamo.Keys{
{
PK: dynamo.ShareKey(dynamo.AttorneyShareKey("123")),
SK: dynamo.ShareSortKey(dynamo.MetadataKey("123")),
},
},
}

Expand Down
31 changes: 16 additions & 15 deletions internal/app/certificate_provider_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"time"

"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
"github.com/ministryofjustice/opg-modernising-lpa/internal/actor/actoruid"
"github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo"
"github.com/ministryofjustice/opg-modernising-lpa/internal/page"
)
Expand All @@ -17,7 +16,7 @@ type certificateProviderStore struct {
now func() time.Time
}

func (s *certificateProviderStore) Create(ctx context.Context, lpaOwnerKey dynamo.LpaOwnerKeyType, certificateProviderUID actoruid.UID, email string) (*actor.CertificateProviderProvidedDetails, error) {
func (s *certificateProviderStore) Create(ctx context.Context, shareCode actor.ShareCodeData, email string) (*actor.CertificateProviderProvidedDetails, error) {
data, err := page.SessionDataFromContext(ctx)
if err != nil {
return nil, err
Expand All @@ -27,29 +26,31 @@ func (s *certificateProviderStore) Create(ctx context.Context, lpaOwnerKey dynam
return nil, errors.New("certificateProviderStore.Create requires LpaID and SessionID")
}

cp := &actor.CertificateProviderProvidedDetails{
certificateProvider := &actor.CertificateProviderProvidedDetails{
PK: dynamo.LpaKey(data.LpaID),
SK: dynamo.CertificateProviderKey(data.SessionID),
UID: certificateProviderUID,
UID: shareCode.ActorUID,
LpaID: data.LpaID,
UpdatedAt: s.now(),
Email: email,
}

if err := s.dynamoClient.Create(ctx, cp); err != nil {
return nil, err
}
if err := s.dynamoClient.Create(ctx, lpaLink{
PK: dynamo.LpaKey(data.LpaID),
SK: dynamo.SubKey(data.SessionID),
DonorKey: lpaOwnerKey,
ActorType: actor.TypeCertificateProvider,
UpdatedAt: s.now(),
}); err != nil {
transaction := dynamo.NewTransaction().
Create(certificateProvider).
Create(lpaLink{
PK: dynamo.LpaKey(data.LpaID),
SK: dynamo.SubKey(data.SessionID),
DonorKey: shareCode.LpaOwnerKey,
ActorType: actor.TypeCertificateProvider,
UpdatedAt: s.now(),
}).
Delete(dynamo.Keys{PK: shareCode.PK, SK: shareCode.SK})

if err := s.dynamoClient.WriteTransaction(ctx, transaction); err != nil {
return nil, err
}

return cp, err
return certificateProvider, err
}

func (s *certificateProviderStore) GetAny(ctx context.Context) (*actor.CertificateProviderProvidedDetails, error) {
Expand Down
83 changes: 40 additions & 43 deletions internal/app/certificate_provider_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,40 @@ import (
)

func TestCertificateProviderStoreCreate(t *testing.T) {
ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123", SessionID: "456"})
ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "lpa-id", SessionID: "session-id"})
uid := actoruid.New()
now := time.Now()
details := &actor.CertificateProviderProvidedDetails{PK: dynamo.LpaKey("123"), SK: dynamo.CertificateProviderKey("456"), LpaID: "123", UpdatedAt: now, UID: uid, Email: "[email protected]"}
details := &actor.CertificateProviderProvidedDetails{PK: dynamo.LpaKey("lpa-id"), SK: dynamo.CertificateProviderKey("session-id"), LpaID: "lpa-id", UpdatedAt: testNow, UID: uid, Email: "[email protected]"}

shareCode := actor.ShareCodeData{
PK: dynamo.ShareKey(dynamo.CertificateProviderShareKey("share-key")),
SK: dynamo.ShareSortKey(dynamo.MetadataKey("share-key")),
ActorUID: uid,
UpdatedAt: testNow,
LpaOwnerKey: dynamo.LpaOwnerKey(dynamo.DonorKey("donor")),
}

expectedTransaction := &dynamo.Transaction{
Creates: []any{
details,
lpaLink{
PK: dynamo.LpaKey("lpa-id"),
SK: dynamo.SubKey("session-id"),
DonorKey: shareCode.LpaOwnerKey,
ActorType: actor.TypeCertificateProvider,
UpdatedAt: testNow,
},
},
Deletes: []dynamo.Keys{{PK: shareCode.PK, SK: shareCode.SK}},
}

dynamoClient := newMockDynamoClient(t)
dynamoClient.EXPECT().
Create(ctx, details).
Return(nil)
dynamoClient.EXPECT().
Create(ctx, lpaLink{PK: dynamo.LpaKey("123"), SK: dynamo.SubKey("456"), DonorKey: dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), ActorType: actor.TypeCertificateProvider, UpdatedAt: now}).
WriteTransaction(ctx, expectedTransaction).
Return(nil)

certificateProviderStore := &certificateProviderStore{dynamoClient: dynamoClient, now: func() time.Time { return now }}
certificateProviderStore := &certificateProviderStore{dynamoClient: dynamoClient, now: testNowFn}

certificateProvider, err := certificateProviderStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), uid, "[email protected]")
certificateProvider, err := certificateProviderStore.Create(ctx, shareCode, "[email protected]")
assert.Nil(t, err)
assert.Equal(t, details, certificateProvider)
}
Expand All @@ -41,7 +59,7 @@ func TestCertificateProviderStoreCreateWhenSessionMissing(t *testing.T) {

certificateProviderStore := &certificateProviderStore{dynamoClient: nil, now: nil}

_, err := certificateProviderStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), actoruid.New(), "")
_, err := certificateProviderStore.Create(ctx, actor.ShareCodeData{}, "")
assert.Equal(t, page.SessionMissingError{}, err)
}

Expand All @@ -57,49 +75,28 @@ func TestCertificateProviderStoreCreateWhenSessionDataMissing(t *testing.T) {

certificateProviderStore := &certificateProviderStore{}

_, err := certificateProviderStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), actoruid.New(), "")
_, err := certificateProviderStore.Create(ctx, actor.ShareCodeData{}, "")
assert.NotNil(t, err)
})
}
}

func TestCertificateProviderStoreCreateWhenCreateError(t *testing.T) {
func TestCertificateProviderStoreCreateWhenWriteTransactionError(t *testing.T) {
now := time.Now()
ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123", SessionID: "456"})

testcases := map[string]func(*testing.T) *mockDynamoClient{
"certificate provider record": func(t *testing.T) *mockDynamoClient {
dynamoClient := newMockDynamoClient(t)
dynamoClient.EXPECT().
Create(ctx, mock.Anything).
Return(expectedError)

return dynamoClient
},
"link record": func(t *testing.T) *mockDynamoClient {
dynamoClient := newMockDynamoClient(t)
dynamoClient.EXPECT().
Create(ctx, mock.Anything).
Return(nil).
Once()
dynamoClient.EXPECT().
Create(ctx, mock.Anything).
Return(expectedError)

return dynamoClient
},
}

for name, makeMockDataStore := range testcases {
t.Run(name, func(t *testing.T) {
dynamoClient := makeMockDataStore(t)
dynamoClient := newMockDynamoClient(t)
dynamoClient.EXPECT().
WriteTransaction(mock.Anything, mock.Anything).
Return(expectedError)

certificateProviderStore := &certificateProviderStore{dynamoClient: dynamoClient, now: func() time.Time { return now }}
certificateProviderStore := &certificateProviderStore{dynamoClient: dynamoClient, now: func() time.Time { return now }}

_, err := certificateProviderStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), actoruid.New(), "")
assert.Equal(t, expectedError, err)
})
}
_, err := certificateProviderStore.Create(ctx, actor.ShareCodeData{
PK: dynamo.ShareKey(dynamo.CertificateProviderShareKey("123")),
SK: dynamo.ShareSortKey(dynamo.MetadataKey("123")),
}, "")
assert.Equal(t, expectedError, err)
}

func TestCertificateProviderStoreGetAny(t *testing.T) {
Expand Down
69 changes: 40 additions & 29 deletions internal/dynamo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,20 +432,41 @@ func (c *Client) WriteTransaction(ctx context.Context, transaction *Transaction)
return errors.New("WriteTransaction requires at least one transaction")
}

if transaction.Errors() != nil {
return transaction.Errors()
var items []types.TransactWriteItem

for _, cr := range transaction.Creates {
values, err := attributevalue.MarshalMap(cr)
if err != nil {
return err
}

items = append(items, types.TransactWriteItem{Put: &types.Put{
TableName: aws.String(c.table),
Item: values,
ConditionExpression: aws.String("attribute_not_exists(PK) AND attribute_not_exists(SK)"),
}})
}

var items []types.TransactWriteItem
for _, p := range transaction.Puts {
values, err := attributevalue.MarshalMap(p)
if err != nil {
return err
}

for _, value := range transaction.Puts {
value.TableName = aws.String(c.table)
items = append(items, types.TransactWriteItem{Put: value})
items = append(items, types.TransactWriteItem{Put: &types.Put{
TableName: aws.String(c.table),
Item: values,
}})
}

for _, value := range transaction.Deletes {
value.TableName = aws.String(c.table)
items = append(items, types.TransactWriteItem{Delete: value})
for _, d := range transaction.Deletes {
items = append(items, types.TransactWriteItem{Delete: &types.Delete{
TableName: aws.String(c.table),
Key: map[string]types.AttributeValue{
"PK": &types.AttributeValueMemberS{Value: d.PK.PK()},
"SK": &types.AttributeValueMemberS{Value: d.SK.SK()},
},
}})
}

_, err := c.svc.TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{
Expand All @@ -456,36 +477,26 @@ func (c *Client) WriteTransaction(ctx context.Context, transaction *Transaction)
}

type Transaction struct {
Puts []*types.Put
Deletes []*types.Delete
Errs []error
Creates []any
Puts []any
Deletes []Keys
}

func NewTransaction() *Transaction {
return &Transaction{}
}

func (t *Transaction) Put(v interface{}) *Transaction {
values, err := attributevalue.MarshalMap(v)

if err != nil {
t.Errs = append(t.Errs, err)
}

t.Puts = append(t.Puts, &types.Put{Item: values})
func (t *Transaction) Create(v interface{}) *Transaction {
t.Creates = append(t.Creates, v)
return t
}

func (t *Transaction) Delete(pk PK, sk SK) *Transaction {
t.Deletes = append(t.Deletes, &types.Delete{
Key: map[string]types.AttributeValue{
"PK": &types.AttributeValueMemberS{Value: pk.PK()},
"SK": &types.AttributeValueMemberS{Value: sk.SK()},
},
})
func (t *Transaction) Put(v interface{}) *Transaction {
t.Puts = append(t.Puts, v)
return t
}

func (t *Transaction) Errors() error {
return errors.Join(t.Errs...)
func (t *Transaction) Delete(keys Keys) *Transaction {
t.Deletes = append(t.Deletes, keys)
return t
}
Loading

0 comments on commit 8e6f3f7

Please sign in to comment.