Skip to content

Commit

Permalink
Always use MustMatchExisting behaviour
Browse files Browse the repository at this point in the history
  • Loading branch information
hawx committed Jun 13, 2024
1 parent 4a80290 commit 2e67ea6
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 54 deletions.
10 changes: 5 additions & 5 deletions lambda/update/attorney_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ func validateAttorneySign(changes []shared.Change, lpa *shared.Lpa) (AttorneySig
}

return p.
Field("/mobile", &data.Mobile, parse.MustMatchExisting()).
Field("/mobile", &data.Mobile).
Field("/signedAt", &data.SignedAt, parse.Validate(func() []shared.FieldError {
return validate.Time("", data.SignedAt)
}), parse.MustMatchExisting()).
})).
Field("/contactLanguagePreference", &data.ContactLanguagePreference, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.ContactLanguagePreference)
}), parse.MustMatchExisting()).
})).
Field("/channel", &data.Channel, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.Channel)
}), parse.MustMatchExisting(), parse.Optional()).
Field("/email", &data.Email, parse.MustMatchExisting(), parse.Optional()).
}), parse.Optional()).
Field("/email", &data.Email, parse.Optional()).
Consumed()
}).
Consumed()
Expand Down
20 changes: 10 additions & 10 deletions lambda/update/certificate_provider_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,28 +45,28 @@ func validateCertificateProviderSign(changes []shared.Change, lpa *shared.Lpa) (
errors := parse.Changes(changes).
Prefix("/certificateProvider/address", func(p *parse.Parser) []shared.FieldError {
return p.
Field("/line1", &data.Address.Line1, parse.MustMatchExisting()).
Field("/line2", &data.Address.Line2, parse.Optional(), parse.MustMatchExisting()).
Field("/line3", &data.Address.Line3, parse.Optional(), parse.MustMatchExisting()).
Field("/town", &data.Address.Town, parse.MustMatchExisting()).
Field("/postcode", &data.Address.Postcode, parse.Optional(), parse.MustMatchExisting()).
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)
}), parse.MustMatchExisting()).
})).
Consumed()
}, parse.Optional()).
Field("/certificateProvider/signedAt", &data.SignedAt, parse.Validate(func() []shared.FieldError {
return validate.Time("", data.SignedAt)
}), parse.MustMatchExisting()).
})).
Field("/certificateProvider/contactLanguagePreference", &data.ContactLanguagePreference, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.ContactLanguagePreference)
}), parse.MustMatchExisting()).
})).
Field("/certificateProvider/email", &data.Email, parse.Validate(func() []shared.FieldError {
return validate.Required("", data.Email)
}), parse.Optional(), parse.MustMatchExisting()).
}), parse.Optional()).
Field("/certificateProvider/channel", &data.Channel, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.Channel)
}), parse.Optional(), parse.MustMatchExisting()).
}), parse.Optional()).
Consumed()

return data, errors
Expand Down
49 changes: 21 additions & 28 deletions lambda/update/parse/changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ func (p *Parser) Errors() []shared.FieldError {
type Option func(fieldOpts) fieldOpts

type fieldOpts struct {
optional bool
mustMatchExisting bool
validator func() []shared.FieldError
optional bool
validator func() []shared.FieldError
}

// Optional stops [Parser.Field] or [Parser.Prefix] from adding an error when the expected key is missing.
Expand All @@ -75,14 +74,6 @@ func Optional() func(fieldOpts) fieldOpts {
}
}

// MustMatchExisting [Parser.Field] will add an error when the old value does not match the existing value.
func MustMatchExisting() func(fieldOpts) fieldOpts {
return func(f fieldOpts) fieldOpts {
f.mustMatchExisting = true
return f
}
}

// Validate runs fn on the [Parser.Field] after unmarshalling. It has no effect when passed to [Parser.Prefix].
func Validate(fn func() []shared.FieldError) Option {
return func(f fieldOpts) fieldOpts {
Expand All @@ -109,25 +100,20 @@ func (p *Parser) Field(key string, existing any, opts ...Option) *Parser {

for i, change := range p.changes {
if change.Key == key {
// Adding for expand phase - will be removed once consumers are updated
if options.mustMatchExisting {
var old any
if err := json.Unmarshal(change.Old, &old); err != nil {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/old"), Detail: "error marshalling old value"})
}

if !oldEqualsExisting(old, existing) {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/old"), Detail: "does not match existing value"})

return p
}
var old any
if err := json.Unmarshal(change.Old, &old); err != nil {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/old"), Detail: "error marshalling old value"})
}

if err := json.Unmarshal(change.New, existing); err != nil {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/new"), Detail: "unexpected type"})
} else if options.validator != nil {
for _, error := range options.validator() {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/new"), Detail: error.Detail})
if !oldEqualsExisting(old, existing) {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/old"), Detail: "does not match existing value"})
} else {
if err := json.Unmarshal(change.New, existing); err != nil {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/new"), Detail: "unexpected type"})
} else if options.validator != nil {
for _, error := range options.validator() {
p.errors = append(p.errors, shared.FieldError{Source: change.Source("/new"), Detail: error.Detail})
}
}
}

Expand Down Expand Up @@ -177,6 +163,13 @@ func oldEqualsExisting(old any, existing any) bool {

return old.(string) == *v

case *int:
if old == nil {
return *v == 0
}

return old.(int) == *v

default:
return false
}
Expand Down
18 changes: 9 additions & 9 deletions lambda/update/parse/changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,48 +88,48 @@ func TestFieldValidateWhenInvalid(t *testing.T) {
assert.Equal(t, []shared.FieldError{{Source: "/changes/0/new", Detail: "invalid"}}, errors)
}

func TestFieldMustMatchExistingString(t *testing.T) {
func TestFieldOldString(t *testing.T) {
changes := []shared.Change{
{Key: "/thing", New: json.RawMessage(`"new val"`), Old: json.RawMessage(`"old val"`)},
}

v := "old val"
Changes(changes).Field("/thing", &v, MustMatchExisting())
Changes(changes).Field("/thing", &v)

assert.Equal(t, "new val", v)
}

func TestFieldMustMatchExistingTime(t *testing.T) {
func TestFieldOldTime(t *testing.T) {
now := time.Now()
yesterday := time.Now().Add(-24 * time.Hour)

changes := []shared.Change{
{Key: "/thing", New: json.RawMessage(`"` + now.Format(time.RFC3339Nano) + `"`), Old: json.RawMessage(`"` + yesterday.Format(time.RFC3339Nano) + `"`)},
}

Changes(changes).Field("/thing", &yesterday, MustMatchExisting())
Changes(changes).Field("/thing", &yesterday)

assert.WithinDuration(t, now, yesterday, time.Second)
}

func TestFieldMustMatchExistingLang(t *testing.T) {
func TestFieldOldLang(t *testing.T) {
changes := []shared.Change{
{Key: "/thing", New: json.RawMessage(`"cy"`), Old: json.RawMessage(`"en"`)},
}

v := shared.LangEn
Changes(changes).Field("/thing", &v, MustMatchExisting())
Changes(changes).Field("/thing", &v)

assert.Equal(t, shared.LangCy, v)
}

func TestFieldMustMatchExistingChannel(t *testing.T) {
func TestFieldOldChannel(t *testing.T) {
changes := []shared.Change{
{Key: "/thing", New: json.RawMessage(`"online"`), Old: json.RawMessage(`"paper"`)},
}

v := shared.ChannelPaper
Changes(changes).Field("/thing", &v, MustMatchExisting())
Changes(changes).Field("/thing", &v)

assert.Equal(t, shared.ChannelOnline, v)
}
Expand All @@ -147,7 +147,7 @@ func TestFieldWhenOldDoesNotMatchExisting(t *testing.T) {
}

v := "existing"
errors := Changes(changes).Field("/thing", &v, MustMatchExisting()).Errors()
errors := Changes(changes).Field("/thing", &v).Errors()

assert.Equal(t, []shared.FieldError{{Source: "/changes/0/old", Detail: "does not match existing value"}}, errors)
})
Expand Down
4 changes: 2 additions & 2 deletions lambda/update/trust_corporation_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ func validateTrustCorporationSign(changes []shared.Change, lpa *shared.Lpa) (Tru
Field("/contactLanguagePreference", &data.ContactLanguagePreference, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.ContactLanguagePreference)
})).
Field("/email", &data.Email, parse.Optional(), parse.MustMatchExisting()).
Field("/email", &data.Email, parse.Optional()).
Field("/channel", &data.Channel, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.Channel)
}), parse.Optional(), parse.MustMatchExisting()).
}), parse.Optional()).
Prefix("/signatories", func(prefix *parse.Parser) []shared.FieldError {
return prefix.
Each(func(i int, each *parse.Parser) []shared.FieldError {
Expand Down

0 comments on commit 2e67ea6

Please sign in to comment.