Refactor changes by introducing a parser

.github/workflows/env-test.yml

@@ -35,7 +35,7 @@ jobs:
           fetch-depth: "0"
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.20"
+          go-version: "1.21"
       - uses: unfor19/install-aws-cli-action@v1
       - name: Configure AWS
         uses: aws-actions/configure-aws-credentials@v4

docker-compose.yml

@@ -81,7 +81,7 @@ services:
     working_dir: /go/src/app
     volumes:
       - ./:/go/src/app
-      - ./.cache/golangci-lint/v1.53.3:/root/.cache
+      - ./.cache/golangci-lint/v1.55.2:/root/.cache
     command: golangci-lint run --timeout 5m --out-format=github-actions ./lambda/get/... ./lambda/create/... ./lambda/update/...
 
   gosec:

go.mod

@@ -1,6 +1,6 @@
 module github.com/ministryofjustice/opg-data-lpa-store
 
-go 1.20
+go 1.21
 
 require (
 	github.com/aws/aws-lambda-go v1.43.0

go.sum

@@ -1,4 +1,5 @@
 github.com/DATA-DOG/go-sqlmock v1.4.1 h1:ThlnYciV1iM/V0OSF/dtkqWb6xo5qITT1TJBG1MRDJM=
+github.com/DATA-DOG/go-sqlmock v1.4.1/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM=
 github.com/andybalholm/brotli v1.0.6 h1:Yf9fFpf49Zrxb9NlQaluyE92/+X7UVHlhMNJN2sxfOI=
 github.com/andybalholm/brotli v1.0.6/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig=
 github.com/aws/aws-lambda-go v1.43.0 h1:Tdu7SnMB5bD+CbdnSq1Dg4sM68vEuGIDcQFZ+IjUfx0=
@@ -18,9 +19,11 @@ github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg
 github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
+github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/uuid v1.5.0 h1:1p67kYwdtXjb0gL0BPiP1Av9wiZPo5A8z2cWkTZ+eyU=
 github.com/google/uuid v1.5.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 h1:+9834+KizmvFV7pXQGSXQTsaWhq2GjuNUt0aUU0YBYw=
+github.com/grpc-ecosystem/go-grpc-middleware v1.3.0/go.mod h1:z0ButlSOZa5vEBq9m2m2hlwIgKw+rp3sdCBRoJY+30Y=
 github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
 github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
 github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=

lambda/update/attorney_sign.go

@@ -1,15 +1,11 @@
 package main
 
 import (
-	"bytes"
-	"encoding/json"
-	"fmt"
-	"strconv"
-	"strings"
 	"time"
 
 	"github.com/ministryofjustice/opg-data-lpa-store/internal/shared"
 	"github.com/ministryofjustice/opg-data-lpa-store/internal/validate"
+	"github.com/ministryofjustice/opg-data-lpa-store/lambda/update/parse"
 )
 
 type AttorneySign struct {
@@ -31,67 +27,31 @@ func (a AttorneySign) Apply(lpa *shared.Lpa) []shared.FieldError {
 	return nil
 }
 
-func validateAttorneySign(changes []shared.Change) (data AttorneySign, errors []shared.FieldError) {
-	for i, change := range changes {
-		after, ok := strings.CutPrefix(change.Key, "/attorneys/")
-		if !ok {
-			errors = append(errors, shared.FieldError{Source: fmt.Sprintf("/changes/%d", i), Detail: "change not allowed for type"})
-			continue
-		}
-
-		if !bytes.Equal(change.Old, []byte("null")) {
-			errors = append(errors, shared.FieldError{Source: fmt.Sprintf("/changes/%d/old", i), Detail: "field must be null"})
-		}
-
-		parts := strings.SplitN(after, "/", 2)
-		attorneyIndex, err := strconv.Atoi(parts[0])
-		if err != nil {
-			errors = append(errors, shared.FieldError{Source: fmt.Sprintf("/changes/%d", i), Detail: "change not allowed for type"})
-			continue
-		}
-		if data.Index != nil && *data.Index != attorneyIndex {
-			errors = append(errors, shared.FieldError{Source: fmt.Sprintf("/changes/%d/key", i), Detail: "must be for same attorney"})
-			continue
-		} else {
-			data.Index = &attorneyIndex
-		}
-
-		newKey := fmt.Sprintf("/changes/%d/new", i)
-		switch parts[1] {
-		case "mobile":
-			if err := json.Unmarshal(change.New, &data.Mobile); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			}
-		case "signedAt":
-			if err := json.Unmarshal(change.New, &data.SignedAt); err != nil {
-				errors = errorMustBeDateTime(errors, newKey)
-			}
-		case "contactLanguagePreference":
-			if err := json.Unmarshal(change.New, &data.ContactLanguagePreference); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			} else {
-				errors = append(errors, validate.IsValid(newKey, data.ContactLanguagePreference)...)
-			}
-		default:
-			errors = append(errors, shared.FieldError{Source: fmt.Sprintf("/changes/%d", i), Detail: "change not allowed for type"})
-		}
-	}
-
-	if data.Index == nil {
-		errors = append(errors, shared.FieldError{Source: "/changes", Detail: "must be specified"})
-	} else {
-		if data.Mobile == "" {
-			errors = errorMissing(errors, fmt.Sprintf("/attorneys/%d/mobile", *data.Index))
-		}
-
-		if data.SignedAt.IsZero() {
-			errors = errorMissing(errors, fmt.Sprintf("/attorneys/%d/signedAt", *data.Index))
-		}
-
-		if data.ContactLanguagePreference == shared.Lang("") {
-			errors = errorMissing(errors, fmt.Sprintf("/attorneys/%d/contactLanguagePreference", *data.Index))
-		}
-	}
+func validateAttorneySign(changes []shared.Change) (AttorneySign, []shared.FieldError) {
+	var data AttorneySign
+
+	errors := parse.Changes(changes).
+		Prefix("/attorneys", func(p *parse.Parser) []shared.FieldError {
+			return p.
+				Each(func(i int, p *parse.Parser) []shared.FieldError {
+					if data.Index != nil && *data.Index != i {
+						return p.OutOfRange()
+					}
+
+					data.Index = &i
+					return p.
+						Field("/mobile", &data.Mobile).
+						Field("/signedAt", &data.SignedAt, parse.Validate(func() []shared.FieldError {
+							return validate.Time("", data.SignedAt)
+						})).
+						Field("/contactLanguagePreference", &data.ContactLanguagePreference, parse.Validate(func() []shared.FieldError {
+							return validate.IsValid("", data.ContactLanguagePreference)
+						})).
+						Consumed()
+				}).
+				Consumed()
+		}).
+		Consumed()
 
 	return data, errors
 }

lambda/update/attorney_sign_test.go

@@ -73,7 +73,7 @@ func TestValidateUpdateAttorneySign(t *testing.T) {
 		"missing all": {
 			update: shared.Update{Type: "ATTORNEY_SIGN"},
 			errors: []shared.FieldError{
-				{Source: "/changes", Detail: "must be specified"},
+				{Source: "/changes", Detail: "missing /attorneys/..."},
 			},
 		},
 		"extra fields": {
@@ -108,9 +108,9 @@ func TestValidateUpdateAttorneySign(t *testing.T) {
 				},
 			},
 			errors: []shared.FieldError{
-				{Source: "/changes/2/old", Detail: "field must be null"},
-				{Source: "/changes/3", Detail: "change not allowed for type"},
-				{Source: "/changes/4", Detail: "change not allowed for type"},
+				{Source: "/changes/2/old", Detail: "must be null"},
+				{Source: "/changes/3", Detail: "unexpected change provided"},
+				{Source: "/changes/4", Detail: "unexpected change provided"},
 			},
 		},
 		"invalid contact language": {
@@ -160,7 +160,7 @@ func TestValidateUpdateAttorneySign(t *testing.T) {
 				},
 			},
 			errors: []shared.FieldError{
-				{Source: "/changes/1/key", Detail: "must be for same attorney"},
+				{Source: "/changes/1/key", Detail: "index out of range"},
 				{Source: "/changes", Detail: "missing /attorneys/0/signedAt"},
 			},
 		},

lambda/update/certificate_provider_sign.go

@@ -1,13 +1,11 @@
 package main
 
 import (
-	"bytes"
-	"encoding/json"
-	"fmt"
 	"time"
 
 	"github.com/ministryofjustice/opg-data-lpa-store/internal/shared"
 	"github.com/ministryofjustice/opg-data-lpa-store/internal/validate"
+	"github.com/ministryofjustice/opg-data-lpa-store/lambda/update/parse"
 )
 
 type CertificateProviderSign struct {
@@ -28,76 +26,29 @@ func (c CertificateProviderSign) Apply(lpa *shared.Lpa) []shared.FieldError {
 	return nil
 }
 
-func validateCertificateProviderSign(changes []shared.Change) (data CertificateProviderSign, errors []shared.FieldError) {
-	for i, change := range changes {
-		if !bytes.Equal(change.Old, []byte("null")) {
-			errors = append(errors, shared.FieldError{Source: fmt.Sprintf("/changes/%d/old", i), Detail: "field must be null"})
-		}
-
-		newKey := fmt.Sprintf("/changes/%d/new", i)
-		switch change.Key {
-		case "/certificateProvider/address/line1":
-			if err := json.Unmarshal(change.New, &data.Address.Line1); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			}
-		case "/certificateProvider/address/line2":
-			if err := json.Unmarshal(change.New, &data.Address.Line2); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			}
-		case "/certificateProvider/address/line3":
-			if err := json.Unmarshal(change.New, &data.Address.Line3); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			}
-		case "/certificateProvider/address/town":
-			if err := json.Unmarshal(change.New, &data.Address.Town); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			}
-		case "/certificateProvider/address/postcode":
-			if err := json.Unmarshal(change.New, &data.Address.Postcode); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			}
-		case "/certificateProvider/address/country":
-			if err := json.Unmarshal(change.New, &data.Address.Country); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			} else {
-				errors = append(errors, validate.Country(newKey, data.Address.Country)...)
-			}
-		case "/certificateProvider/signedAt":
-			if err := json.Unmarshal(change.New, &data.SignedAt); err != nil {
-				errors = errorMustBeDateTime(errors, newKey)
-			}
-		case "/certificateProvider/contactLanguagePreference":
-			if err := json.Unmarshal(change.New, &data.ContactLanguagePreference); err != nil {
-				errors = errorMustBeString(errors, newKey)
-			} else {
-				errors = append(errors, validate.IsValid(newKey, data.ContactLanguagePreference)...)
-			}
-		default:
-			errors = append(errors, shared.FieldError{Source: fmt.Sprintf("/changes/%d", i), Detail: "change not allowed for type"})
-		}
-	}
-
-	if !data.Address.IsZero() {
-		if data.Address.Line1 == "" {
-			errors = errorMissing(errors, "/certificateProvider/address/line1")
-		}
-
-		if data.Address.Town == "" {
-			errors = errorMissing(errors, "/certificateProvider/address/town")
-		}
-
-		if data.Address.Country == "" {
-			errors = errorMissing(errors, "/certificateProvider/address/country")
-		}
-	}
-
-	if data.SignedAt.IsZero() {
-		errors = errorMissing(errors, "/certificateProvider/signedAt")
-	}
-
-	if data.ContactLanguagePreference == shared.Lang("") {
-		errors = errorMissing(errors, "/certificateProvider/contactLanguagePreference")
-	}
+func validateCertificateProviderSign(changes []shared.Change) (CertificateProviderSign, []shared.FieldError) {
+	var data CertificateProviderSign
+
+	errors := parse.Changes(changes).
+		Prefix("/certificateProvider/address", func(p *parse.Parser) []shared.FieldError {
+			return p.
+				Field("/line1", &data.Address.Line1).
+				Field("/line2", &data.Address.Line2, parse.Optional()).
+				Field("/line3", &data.Address.Line3, parse.Optional()).
+				Field("/town", &data.Address.Town).
+				Field("/postcode", &data.Address.Postcode, parse.Optional()).
+				Field("/country", &data.Address.Country, parse.Validate(func() []shared.FieldError {
+					return validate.Country("", data.Address.Country)
+				})).
+				Consumed()
+		}, parse.Optional()).
+		Field("/certificateProvider/signedAt", &data.SignedAt, parse.Validate(func() []shared.FieldError {
+			return validate.Time("", data.SignedAt)
+		})).
+		Field("/certificateProvider/contactLanguagePreference", &data.ContactLanguagePreference, parse.Validate(func() []shared.FieldError {
+			return validate.IsValid("", data.ContactLanguagePreference)
+		})).
+		Consumed()
 
 	return data, errors
 }

lambda/update/certificate_provider_sign_test.go

@@ -95,7 +95,7 @@ func TestValidateUpdateCertificateProviderSign(t *testing.T) {
 				},
 			},
 			errors: []shared.FieldError{
-				{Source: "/changes/0/new", Detail: "must be a string"},
+				{Source: "/changes/0/new", Detail: "unexpected type"},
 				{Source: "/changes", Detail: "missing /certificateProvider/address/line1"},
 				{Source: "/changes", Detail: "missing /certificateProvider/address/town"},
 				{Source: "/changes/1/new", Detail: "must be a valid ISO-3166-1 country code"},
@@ -125,8 +125,8 @@ func TestValidateUpdateCertificateProviderSign(t *testing.T) {
 				},
 			},
 			errors: []shared.FieldError{
-				{Source: "/changes/1/old", Detail: "field must be null"},
-				{Source: "/changes/2", Detail: "change not allowed for type"},
+				{Source: "/changes/1/old", Detail: "must be null"},
+				{Source: "/changes/2", Detail: "unexpected change provided"},
 			},
 		},
 		"invalid contact language": {