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

Design code and gen code case miss match for headers #1242

Closed
lockwobr opened this issue Jun 1, 2017 · 6 comments
Closed

Design code and gen code case miss match for headers #1242

lockwobr opened this issue Jun 1, 2017 · 6 comments
Labels

Comments

@lockwobr
Copy link

lockwobr commented Jun 1, 2017

Running into an issue with Headers and case sensitivity and the generated code, meaning if I add something like the design below to to a resource the code in context.go that gen makes does not match cases everywhere making the ctx.ETag nil in the controller. Some places the gen code uses ETag and other is Etag... if i change the design to Etag then the gen code cases matchup. I noticed this problem when writing unit tests using the testing generated helpers. So the bug could be in that part of the code pipeline, but seems the be in the the Context constructor, headerETag := req.Header["Etag"] the case for ETag does not match, resulting in the ETag field on the context object being nil.

Design:

Action("update", func() {
	UseTrait("versioned")
	Headers(func() {
		Header("ETag", UUID, "ETag is part of http standard, and is used for detecting resource conflicts")
	})
       ....

context.go:

func NewUpdateUserProfileContext(ctx context.Context, r *http.Request, service *goa.Service) (*UpdateUserProfileContext, error) {
.....
    headerETag := req.Header["Etag"]
    if len(headerETag) > 0 {
        rawETag := headerETag[0]
        req.Params["ETag"] = []string{rawETag}
        if eTag, err2 := uuid.FromString(rawETag); err2 == nil {
            tmp7 := &eTag
            rctx.ETag = tmp7
        } else {
            err = goa.MergeErrors(err, goa.InvalidParamTypeError("ETag", rawETag, "uuid"))
        }
    }
.....

}

testing.go:

       if eTag != nil {
		sliceVal := []string{fmt.Sprintf("%v", *eTag)}
		req.Header["ETag"] = sliceVal
	}
	if xApiVersion != nil {
		sliceVal := []string{*xApiVersion}
		req.Header["X-Api-Version"] = sliceVal
	}
	prms := url.Values{}
	if ctx == nil {
		ctx = context.Background()
	}
	goaCtx := goa.NewContext(goa.WithAction(ctx, "UserProfileTest"), rw, req, prms)
	updateCtx, __err := app.NewUpdateUserProfileContext(goaCtx, req, service)
	if __err != nil {
		panic("invalid test data " + __err.Error()) // bug
	}
	updateCtx.Payload = payload
@raphael
Copy link
Member

raphael commented Jun 1, 2017

I took a look and the difference is that the code generator for the controller calls http.CanonicalHeaderKey where the code generator for the tests does not. So it seems the controller is "correct" and that the code should be request.Header["Etag"] but before I fix the test generator I wanted to make sure that this would work, can you please confirm?

@lockwobr
Copy link
Author

lockwobr commented Jun 1, 2017

I can test something but not 100% sure I understand what you're asking me to do or test. Guessing some with how I am testing my controller?

@raphael
Copy link
Member

raphael commented Jun 1, 2017

Sorry my response wasn't super clear :) I mean if the generated test code was:

if eTag != nil {
		sliceVal := []string{fmt.Sprintf("%v", *eTag)}
		req.Header["Etag"] = sliceVal // Etag instead of ETag
	}

instead of:

if eTag != nil {
		sliceVal := []string{fmt.Sprintf("%v", *eTag)}
		req.Header["ETag"] = sliceVal
	}

Would that fix your issue? Just want to make sure the problem is not in the controller generated code but only in the test generated code.

@lockwobr
Copy link
Author

lockwobr commented Jun 1, 2017

@raphael yes if I make that change in the testing code, my tests work pass!

On a side note, I now understand what you were first asking/saying. Your point was that the casing in the controller code was right based on standards and the test code did not match the standards but it did match the design, which is why I was thinking the controller was where the problem is, but in fact it was by design that the controller did not match the design. Also makes sense that this was not reported before, but I am guessing less consumers of goa are using the testing code, so it went unnoticed where if the problem was in the controller I am sure would have been reported before. Thanks for the help.

@raphael
Copy link
Member

raphael commented Jun 1, 2017

You got it, I'll work a fix soon. Thank you for confirming!

@raphael
Copy link
Member

raphael commented Jun 2, 2017

This is now fixed, thanks for the detailed report!

@raphael raphael added bug and removed need more info labels Jun 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants