Repository: goadesign/goa
Pull request #2354 (2019-11-03 01:47:48)

v3: Fix a bug that caused a build error when a variable of a payload is not referenced
In gRPC, a payload variable is always prepared 
even though the payload reference might be unnecessary.

**An example of design**

```go
package design

import (
	. "goa.design/goa/v3/dsl"
)

var _ = Service("calc", func() {
	Method("add", func() {
		StreamingPayload(func() {
			Field(1, "a", Int, "Left operand")
			Field(2, "b", Int, "Right operand")
			Required("a", "b")
		})
		Result(Int)
		GRPC(func() {
		})
	})
```

**Error**

```
$ go run ./cmd/calc
# calc/gen/grpc/calc/server
gen/grpc/calc/server/server.go:56:2: p declared and not used
```

**Generated code**

```go
// Add implements the "Add" method in calcpb.CalcServer interface.
func (s *Server) Add(stream calcpb.Calc_AddServer) error {
	ctx := stream.Context()
	ctx = context.WithValue(ctx, goa.MethodKey, "add")
	ctx = context.WithValue(ctx, goa.ServiceKey, "calc")
	p, err := s.AddH.Decode(ctx, nil)  // ← var p is not referenced anywhere
	if err != nil {
		return goagrpc.EncodeError(err)
	}
	ep := &calc.AddEndpointInput{
		Stream: &AddServerStream{stream: stream},
	}
	err = s.AddH.Handle(ctx, ep)
	if err != nil {
		return goagrpc.EncodeError(err)
	}
	return nil
}
```


Patch:
diff --git a/expr/attribute.go b/expr/attribute.go
index 7217bd4a..d4bafe76 100644
--- a/expr/attribute.go
+++ b/expr/attribute.go
@@ -2,6 +2,7 @@ package expr
 
 import (
 	"fmt"
+	"strings"
 
 	"goa.design/goa/v3/eval"
 )
@@ -389,6 +390,26 @@ func (a *AttributeExpr) HasTag(tag string) bool {
 	return false
 }
 
+// HasTagPrefix returns true if the attribute is an object that has an attribute with
+// the given tag prefix.
+func (a *AttributeExpr) HasTagPrefix(prefix string) bool {
+	if a == nil {
+		return false
+	}
+	obj := AsObject(a.Type)
+	if obj == nil {
+		return false
+	}
+	for _, at := range *obj {
+		for k := range at.Attribute.Meta {
+			if strings.HasPrefix(k, prefix) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // FieldTag returns the field tag if the attribute is a field.
 func (a *AttributeExpr) FieldTag() (tag string, found bool) {
 	if a == nil {
diff --git a/expr/method.go b/expr/method.go
index 49d212a1..ada71ddd 100644
--- a/expr/method.go
+++ b/expr/method.go
@@ -107,11 +107,18 @@ func (m *MethodExpr) Validate() error {
 	} else if len(m.Service.Requirements) > 0 {
 		requirements = m.Service.Requirements
 	}
+	var (
+		hasBasicAuth bool
+		hasAPIKey    bool
+		hasJWT       bool
+		hasOAuth     bool
+	)
 	for _, r := range requirements {
 		for _, s := range r.Schemes {
 			verr.Merge(s.Validate())
 			switch s.Kind {
 			case BasicAuthKind:
+				hasBasicAuth = true
 				if !hasTag(m.Payload, "security:username") {
 					verr.Add(m, "payload of method %q of service %q does not define a username attribute, use Username to define one", m.Name, m.Service.Name)
 				}
@@ -119,14 +126,17 @@ func (m *MethodExpr) Validate() error {
 					verr.Add(m, "payload of method %q of service %q does not define a password attribute, use Password to define one", m.Name, m.Service.Name)
 				}
 			case APIKeyKind:
+				hasAPIKey = true
 				if !hasTag(m.Payload, "security:apikey:"+s.SchemeName) {
 					verr.Add(m, "payload of method %q of service %q does not define an API key attribute, use APIKey to define one", m.Name, m.Service.Name)
 				}
 			case JWTKind:
+				hasJWT = true
 				if !hasTag(m.Payload, "security:token") {
 					verr.Add(m, "payload of method %q of service %q does not define a JWT attribute, use Token to define one", m.Name, m.Service.Name)
 				}
 			case OAuth2Kind:
+				hasOAuth = true
 				if !hasTag(m.Payload, "security:accesstoken") {
 					verr.Add(m, "payload of method %q of service %q does not define a OAuth2 access token attribute, use AccessToken to define one", m.Name, m.Service.Name)
 				}
@@ -149,6 +159,29 @@ func (m *MethodExpr) Validate() error {
 			}
 		}
 	}
+	if !hasBasicAuth {
+		if hasTag(m.Payload, "security:username") {
+			verr.Add(m, "payload of method %q of service %q defines a username attribute, but no basic auth security scheme exist", m.Name, m.Service.Name)
+		}
+		if hasTag(m.Payload, "security:password") {
+			verr.Add(m, "payload of method %q of service %q defines a password attribute, but no basic auth security scheme exist", m.Name, m.Service.Name)
+		}
+	}
+	if !hasAPIKey {
+		if hasTagPrefix(m.Payload, "security:apikey") {
+			verr.Add(m, "payload of method %q of service %q defines an API key attribute, but no APIKey security scheme exist", m.Name, m.Service.Name)
+		}
+	}
+	if !hasJWT {
+		if hasTag(m.Payload, "security:token") {
+			verr.Add(m, "payload of method %q of service %q defines a JWT token attribute, but no JWT auth security scheme exist", m.Name, m.Service.Name)
+		}
+	}
+	if !hasOAuth {
+		if hasTag(m.Payload, "security:accesstoken") {
+			verr.Add(m, "payload of method %q of service %q defines a OAuth2 access token attribute, but no OAuth2 security scheme exist", m.Name, m.Service.Name)
+		}
+	}
 	if m.StreamingPayload.Type != Empty {
 		verr.Merge(m.StreamingPayload.Validate("streaming_payload", m))
 	}
@@ -203,6 +236,26 @@ func hasTag(p *AttributeExpr, tag string) bool {
 	return false
 }
 
+// hasTag is a helper function that traverses the given attribute and all its
+// bases recursively looking for an attribute with the given tag meta prefix. This
+// recursion is only needed for attributes that have not been finalized yet.
+func hasTagPrefix(p *AttributeExpr, prefix string) bool {
+	if p.HasTagPrefix(prefix) {
+		return true
+	}
+	for _, base := range p.Bases {
+		ut, ok := base.(UserType)
+		if !ok {
+			continue
+		}
+		return hasTagPrefix(ut.Attribute(), prefix)
+	}
+	if ut, ok := p.Type.(UserType); ok {
+		return hasTagPrefix(ut.Attribute(), prefix)
+	}
+	return false
+}
+
 // Finalize makes sure the method payload and result types are set. It also
 // projects the result if it is a result type and a view is explicitly set in
 // the design or a result type having at most one view.
diff --git a/grpc/codegen/server.go b/grpc/codegen/server.go
index bfb46766..b028ef08 100644
--- a/grpc/codegen/server.go
+++ b/grpc/codegen/server.go
@@ -242,7 +242,7 @@ func (s *{{ .ServerStruct }}) {{ .Method.VarName }}(
 	ctx = context.WithValue(ctx, goa.ServiceKey, {{ printf "%q" .ServiceName }})
 
 {{- if .ServerStream }}
-	p, err := s.{{ .Method.VarName }}H.Decode(ctx, {{ if .Method.StreamingPayload }}nil{{ else }}message{{ end }})
+	{{if .PayloadRef }}p{{ else }}_{{ end }}, err := s.{{ .Method.VarName }}H.Decode(ctx, {{ if .Method.StreamingPayload }}nil{{ else }}message{{ end }})
 	{{- template "handle_error" . }}
 	ep := &{{ .ServicePkgName }}.{{ .Method.VarName }}EndpointInput{
 		Stream: &{{ .ServerStream.VarName }}{stream: stream},


Test patch:
diff --git a/expr/attribute_test.go b/expr/attribute_test.go
index 0639e898..b54932bf 100644
--- a/expr/attribute_test.go
+++ b/expr/attribute_test.go
@@ -613,6 +613,64 @@ func TestAttributeExprHasTag(t *testing.T) {
 	}
 }
 
+func TestAttributeExprHasTagPrefix(t *testing.T) {
+	var (
+		tag    = "security:apikey:api_key"
+		prefix = "security:apikey"
+	)
+	cases := map[string]struct {
+		attribute *AttributeExpr
+		prefix    string
+		expected  bool
+	}{
+		"has tag prefix": {
+			attribute: &AttributeExpr{
+				Type: &Object{
+					&NamedAttributeExpr{
+						Name: "foo",
+						Attribute: &AttributeExpr{
+							Meta: MetaExpr{
+								tag: []string{"key"},
+							},
+						},
+					},
+				},
+			},
+			prefix:   prefix,
+			expected: true,
+		},
+		"attribute expr is nil": {
+			attribute: nil,
+			prefix:    prefix,
+			expected:  false,
+		},
+		"not object": {
+			attribute: &AttributeExpr{
+				Type: String,
+			},
+			prefix:   prefix,
+			expected: false,
+		},
+		"object but has no tag": {
+			attribute: &AttributeExpr{
+				Type: &Object{
+					&NamedAttributeExpr{
+						Name:      "foo",
+						Attribute: &AttributeExpr{},
+					},
+				},
+			},
+			prefix: prefix,
+		},
+	}
+
+	for k, tc := range cases {
+		if actual := tc.attribute.HasTagPrefix(tc.prefix); tc.expected != actual {
+			t.Errorf("%s: got %#v, expected %#v", k, actual, tc.expected)
+		}
+	}
+}
+
 func TestAttributeExprHasDefaultValue(t *testing.T) {
 	var (
 		object = &Object{
diff --git a/expr/method_test.go b/expr/method_test.go
index 423b2a18..0d97b847 100644
--- a/expr/method_test.go
+++ b/expr/method_test.go
@@ -24,7 +24,12 @@ flow authorization_code: invalid authorization URL "http://^authorization": pars
 flow authorization_code: invalid refresh URL "http://refresh^": parse http://refresh^: invalid character "^" in host name
 service "InvalidSecuritySchemesService" method "InheritedSecureMethod": payload of method "InheritedSecureMethod" of service "InvalidSecuritySchemesService" does not define a OAuth2 access token attribute, use AccessToken to define one
 service "InvalidSecuritySchemesService" method "InheritedSecureMethod": payload of method "InheritedSecureMethod" of service "InvalidSecuritySchemesService" does not define an API key attribute, use APIKey to define one
-service "InvalidSecuritySchemesService" method "InheritedSecureMethod": security scope "not:found" not found in any of the security schemes.`,
+service "InvalidSecuritySchemesService" method "InheritedSecureMethod": security scope "not:found" not found in any of the security schemes.
+service "AnotherInvalidSecuritySchemesService" method "Method": payload of method "Method" of service "AnotherInvalidSecuritySchemesService" defines a username attribute, but no basic auth security scheme exist
+service "AnotherInvalidSecuritySchemesService" method "Method": payload of method "Method" of service "AnotherInvalidSecuritySchemesService" defines a password attribute, but no basic auth security scheme exist
+service "AnotherInvalidSecuritySchemesService" method "Method": payload of method "Method" of service "AnotherInvalidSecuritySchemesService" defines an API key attribute, but no APIKey security scheme exist
+service "AnotherInvalidSecuritySchemesService" method "Method": payload of method "Method" of service "AnotherInvalidSecuritySchemesService" defines a JWT token attribute, but no JWT auth security scheme exist
+service "AnotherInvalidSecuritySchemesService" method "Method": payload of method "Method" of service "AnotherInvalidSecuritySchemesService" defines a OAuth2 access token attribute, but no OAuth2 security scheme exist`,
 		},
 	}
 	for _, tc := range cases {
diff --git a/expr/testdata/endpoint_dsls.go b/expr/testdata/endpoint_dsls.go
index 30c587a8..2837e10b 100644
--- a/expr/testdata/endpoint_dsls.go
+++ b/expr/testdata/endpoint_dsls.go
@@ -262,6 +262,8 @@ var FinalizeEndpointBodyAsExtendedTypeDSL = func() {
 }
 
 var FinalizeEndpointBodyAsPropWithExtendedTypeDSL = func() {
+	var OAuth2 = OAuth2Security("authCode")
+
 	var EntityData = Type("EntityData", func() {
 		Attribute("name", String)
 	})
@@ -274,6 +276,7 @@ var FinalizeEndpointBodyAsPropWithExtendedTypeDSL = func() {
 
 	Service("Service", func() {
 		Method("Method", func() {
+			Security(OAuth2)
 			Payload(func() {
 				AccessToken("token", String)
 				Attribute("payload", Entity)
diff --git a/expr/testdata/method_validate_dsls.go b/expr/testdata/method_validate_dsls.go
index 35e9612a..96f34d75 100644
--- a/expr/testdata/method_validate_dsls.go
+++ b/expr/testdata/method_validate_dsls.go
@@ -39,4 +39,16 @@ var InvalidSecuritySchemesDSL = func() {
 			})
 		})
 	})
+	Service("AnotherInvalidSecuritySchemesService", func() {
+		Method("Method", func() {
+			Payload(func() {
+				Username("user", String)
+				Password("pass", String)
+				APIKey("key_key", "key", String)
+				Token("token", String)
+				AccessToken("access_token", String)
+			})
+			// invalid: missing security scheme
+		})
+	})
 }
diff --git a/grpc/codegen/testdata/server_interface_code.go b/grpc/codegen/testdata/server_interface_code.go
index 7ebc28b6..566f8595 100644
--- a/grpc/codegen/testdata/server_interface_code.go
+++ b/grpc/codegen/testdata/server_interface_code.go
@@ -130,7 +130,7 @@ func (s *Server) MethodClientStreamingRPC(stream service_client_streaming_rpcpb.
 	ctx := stream.Context()
 	ctx = context.WithValue(ctx, goa.MethodKey, "MethodClientStreamingRPC")
 	ctx = context.WithValue(ctx, goa.ServiceKey, "ServiceClientStreamingRPC")
-	p, err := s.MethodClientStreamingRPCH.Decode(ctx, nil)
+	_, err := s.MethodClientStreamingRPCH.Decode(ctx, nil)
 	if err != nil {
 		return goagrpc.EncodeError(err)
 	}
@@ -177,7 +177,7 @@ func (s *Server) MethodBidirectionalStreamingRPC(stream service_bidirectional_st
 	ctx := stream.Context()
 	ctx = context.WithValue(ctx, goa.MethodKey, "MethodBidirectionalStreamingRPC")
 	ctx = context.WithValue(ctx, goa.ServiceKey, "ServiceBidirectionalStreamingRPC")
-	p, err := s.MethodBidirectionalStreamingRPCH.Decode(ctx, nil)
+	_, err := s.MethodBidirectionalStreamingRPCH.Decode(ctx, nil)
 	if err != nil {
 		return goagrpc.EncodeError(err)
 	}
@@ -224,7 +224,7 @@ func (s *Server) MethodBidirectionalStreamingRPCWithErrors(stream service_bidire
 	ctx := stream.Context()
 	ctx = context.WithValue(ctx, goa.MethodKey, "MethodBidirectionalStreamingRPCWithErrors")
 	ctx = context.WithValue(ctx, goa.ServiceKey, "ServiceBidirectionalStreamingRPCWithErrors")
-	p, err := s.MethodBidirectionalStreamingRPCWithErrorsH.Decode(ctx, nil)
+	_, err := s.MethodBidirectionalStreamingRPCWithErrorsH.Decode(ctx, nil)
 	if err != nil {
 		if en, ok := err.(ErrorNamer); ok {
 			switch en.ErrorName() {