Refactor changes by introducing a parser

ministryofjustice · Jan 5, 2024 · aab2467 · aab2467
1 parent fda8dd8
commit aab2467
Show file tree

Hide file tree

Showing 8 changed files with 620 additions and 273 deletions.
diff --git a/lambda/update/attorney_sign.go b/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
 }
diff --git a/lambda/update/attorney_sign_test.go b/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 not be provided"},
+				{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"},
 			},
 		},

diff --git a/lambda/update/certificate_provider_sign.go b/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
 }
diff --git a/lambda/update/certificate_provider_sign_test.go b/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: "wrong 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 not be provided"},
+				{Source: "/changes/2", Detail: "unexpected change provided"},
 			},
 		},
 		"invalid contact language": {