Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to generate validations of hash elements #1298

Merged
merged 4 commits into from Jul 12, 2017

Conversation

@ikawaha
Copy link
Contributor

@ikawaha ikawaha commented Jul 10, 2017

HashOf accepts optional DSLs as third and fourth argument which allows providing validations for the keys and values of the hash. But there is no validation codes generated.

Sample design:

package design

import (
	. "github.com/goadesign/goa/design"
	. "github.com/goadesign/goa/design/apidsl"
)

var _ = API("cellar", func() {
	Title("The virtual wine cellar")
	Description("A simple goa service")
	Scheme("http")
	Host("localhost:8080")
})

var Bottle = Type("bottle", func() {
	Param("names", ArrayOf(String, func() {
		Pattern("[a-zA-Z]+")
	}))
	Param("bottles", HashOf(String, String, func() {
		Pattern("[a-zA-Z]+")
	}, func() {
		Pattern("[0-9]+")
	}), func() {
		MinLength(1)
	})
})

var _ = Resource("bottle", func() {
	BasePath("/bottles")
	DefaultMedia(BottleMedia)

	Action("show", func() {
		Description("Get bottle by id")
		Routing(GET("/:bottleID"))
		Params(func() {
			Param("bottleID", Integer, "Bottle ID")
		})
		Response(OK)
		Response(NotFound)
	})
	Action("updateRatings", func() {
		Routing(POST(""))
		Payload(func() {
			Member("validatableHash", HashOf(String, Integer, func() {
				Pattern("[a-zA-Z]+")
			}, func() {
				Enum(12, 3, 4, 5, 6)
			}))
			Member("validatableArray", ArrayOf(String, func() {
				Pattern("[a-zA-Z]+")
			}))
		})
	})
})

var BottleMedia = MediaType("application/vnd.goa.example.bottle+json", func() {
	Description("A bottle of wine")
	Attributes(func() {
		Attribute("id", Integer, "Unique bottle ID")
		Attribute("href", String, "API href for making requests on the bottle")
		Attribute("name", String, "Name of wine")
		Required("id", "href", "name")
	})
	View("default", func() {
		Attribute("id")
		Attribute("href")
		Attribute("name")
	})
})

Above design generates following codes:

app/contexts.go

// Validate runs the validation rules defined in the design.
func (payload *UpdateRatingsBottlePayload) Validate() (err error) {
        for _, e := range payload.ValidatableArray {
                if ok := goa.ValidatePattern(`[a-zA-Z]+`, e); !ok {
                        err = goa.MergeErrors(err, goa.InvalidPatternError(`raw.validatableArray[*]`, e, `[a-zA-Z]+`))
                }
                // ← ★ hash validation codes should be here !!!
        }
        return
}

app/user_type.go

// Validate validates the Bottle type instance.
func (ut *Bottle) Validate() (err error) {
        if ut.Bottles != nil {
                if len(ut.Bottles) < 1 {
                        err = goa.MergeErrors(err, goa.InvalidLengthError(`response.bottles`, ut.Bottles, len(ut.Bottles), 1, true))
                }
        }
        for _, e := range ut.Names {
                if ok := goa.ValidatePattern(`[a-zA-Z]+`, e); !ok {
                        err = goa.MergeErrors(err, goa.InvalidPatternError(`response.names[*]`, e, `[a-zA-Z]+`))
                }
        }

        // ← ★ hash validation codes should be here !!!

        return
}

This patch is applied:

app/contexts.go

// Validate runs the validation rules defined in the design.
func (payload *UpdateRatingsBottlePayload) Validate() (err error) {
        for _, e := range payload.ValidatableArray {
                if ok := goa.ValidatePattern(`[a-zA-Z]+`, e); !ok {
                        err = goa.MergeErrors(err, goa.InvalidPatternError(`raw.validatableArray[*]`, e, `[a-zA-Z]+`))
                }
        }
        for k, e := range payload.ValidatableHash {
                if ok := goa.ValidatePattern(`[a-zA-Z]+`, k); !ok {
                        err = goa.MergeErrors(err, goa.InvalidPatternError(`raw.validatableHash[*]`, k, `[a-zA-Z]+`))
                }
                if !(e == 12 || e == 3 || e == 4 || e == 5 || e == 6) {
                        err = goa.MergeErrors(err, goa.InvalidEnumValueError(`raw.validatableHash[*]`, e, []interface{}{12, 3, 4,
5, 6}))
                }
        }
        return
}

app/user_type.go

// Validate validates the Bottle type instance.
func (ut *Bottle) Validate() (err error) {
        if ut.Bottles != nil {
                if len(ut.Bottles) < 1 {
                        err = goa.MergeErrors(err, goa.InvalidLengthError(`response.bottles`, ut.Bottles, len(ut.Bottles), 1, true
))
                }
        }
        for k, e := range ut.Bottles {
                if ok := goa.ValidatePattern(`[a-zA-Z]+`, k); !ok {
                        err = goa.MergeErrors(err, goa.InvalidPatternError(`response.bottles[*]`, k, `[a-zA-Z]+`))
                }
                if ok := goa.ValidatePattern(`[0-9]+`, e); !ok {
                        err = goa.MergeErrors(err, goa.InvalidPatternError(`response.bottles[*]`, e, `[0-9]+`))
                }
        }
        for _, e := range ut.Names {
                if ok := goa.ValidatePattern(`[a-zA-Z]+`, e); !ok {
                        err = goa.MergeErrors(err, goa.InvalidPatternError(`response.names[*]`, e, `[a-zA-Z]+`))
                }
        }
        return
}
Copy link
Member

@raphael raphael left a comment

Thank you for doing that! I left a couple of questions in the review.

})
keyVal = fmt.Sprintf("%sif e != nil {\n%s\n%s}", Tabs(depth+1), val, Tabs(depth+1))
}
elemVal := v.Code(h.ElemType, true, false, false, "e", context+"[*]", depth+1, false)

This comment has been minimized.

@raphael

raphael Jul 10, 2017
Member

Shouldn't that be outside of the if val != "" { ? There may be validations that apply to the elements even if no validation applies to the keys. It seems the code should compute validations for the keys and values separately and render both separately.

@@ -386,6 +437,11 @@ const (
{{ .validation }}
{{ tabs .depth }}}`

hashValTmpl = `{{ tabs .depth }}for k, {{ if .elemValidation }}e{{ else }}_{{ end }} := range {{ .target }} {
{{ .keyValidation }}
{{ if .elemValidation }}{{ .elemValidation }}{{ end }}

This comment has been minimized.

@raphael

raphael Jul 10, 2017
Member

Is the if .elemValidation needed?

@ikawaha
Copy link
Contributor Author

@ikawaha ikawaha commented Jul 12, 2017

Thank you for your review. I misunderstood the specification.
I fixed the part pointed out in the review.

@@ -386,6 +438,12 @@ const (
{{ .validation }}
{{ tabs .depth }}}`

hashValTmpl = `{{ tabs .depth }}for {{ if .keyValidation }}k{{ else }}_{{ end }}, {{ if .elemValidation }}e{{ else }}_{{ end }} := range {{ .target }} {
{{- if .keyValidation }}

This comment has been minimized.

@ikawaha

ikawaha Jul 12, 2017
Author Contributor

if .keyValidation and if .elemValidation are used for linefeed control.

@raphael
Copy link
Member

@raphael raphael commented Jul 12, 2017

Awesome PR, thank you! Do you think you could backport to v1?

@raphael raphael merged commit 0f4fc0b into goadesign:master Jul 12, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
ikawaha added a commit to ikawaha/goa that referenced this pull request Jul 12, 2017
* Fix to generate validations of hash elements

* Fix to compute validations for the keys and values separately and render both separately

* Add tests for hash validations

* Fix cyclo
raphael added a commit that referenced this pull request Jul 12, 2017
* Fix to generate validations of hash elements

* Fix to compute validations for the keys and values separately and render both separately

* Add tests for hash validations

* Fix cyclo
@ikawaha ikawaha deleted the ikawaha:fix/hash_validation branch Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants