Repository: goadesign/goa
Pull request #2336 (2019-10-24 14:19:31)

Issue 2206 honour swagger example false at attribute level
Here I am proposing a fix to the fact that `swagger:example = false` is not honoured at attribute level, as reported at issue 2206.

Tests are passing alright, but I didn't manage to run the new code against any design, so I cannot tell whether I'm been missing anything, or worse.

If alright and approved, I'm more than happy to port the same changes to v2.


Patch:
diff --git a/README.md b/README.md
index cb893d7b..b38dcea8 100644
--- a/README.md
+++ b/README.md
@@ -17,7 +17,7 @@ Goa takes a different approach to building services by making it possible to
 describe the *design* of the service API using a simple Go DSL. Goa uses the
 description to generate specialized service helper code, client code and
 documentation. Goa is extensible via plugins, for example the
-[goakit](https://github.com/goadesign/plugins/tree/master/goakit) plugin
+[goakit](https://github.com/goadesign/plugins/tree/v3/goakit) plugin
 generates code that leverage the Go kit library.
 
 The service design describes the transport independent layer of the services in
diff --git a/expr/example.go b/expr/example.go
index 761e4f4a..68e76947 100644
--- a/expr/example.go
+++ b/expr/example.go
@@ -24,6 +24,11 @@ func (a *AttributeExpr) Example(r *Random) interface{} {
 		// slice.
 		return a.UserExamples[l-1].Value
 	}
+
+	if value, ok := a.Meta.Last("swagger:example"); ok && value == "false" {
+		return nil
+	}
+
 	// randomize array length first, since that's from higher level
 	if hasLengthValidation(a) {
 		return byLength(a, r)
diff --git a/expr/method.go b/expr/method.go
index d4a16744..49d212a1 100644
--- a/expr/method.go
+++ b/expr/method.go
@@ -99,55 +99,53 @@ func (m *MethodExpr) Prepare() {
 // Validate validates the method payloads, results, and errors (if any).
 func (m *MethodExpr) Validate() error {
 	verr := new(eval.ValidationErrors)
-	if m.Payload.Type != Empty {
-		verr.Merge(m.Payload.Validate("payload", m))
-		// validate security scheme requirements
-		var requirements []*SecurityExpr
-		if len(m.Requirements) > 0 {
-			requirements = m.Requirements
-		} else if len(m.Service.Requirements) > 0 {
-			requirements = m.Service.Requirements
-		}
-		for _, r := range requirements {
-			for _, s := range r.Schemes {
-				verr.Merge(s.Validate())
-				switch s.Kind {
-				case BasicAuthKind:
-					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)
-					}
-					if !hasTag(m.Payload, "security:password") {
-						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:
-					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:
-					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:
-					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)
-					}
+	verr.Merge(m.Payload.Validate("payload", m))
+	// validate security scheme requirements
+	var requirements []*SecurityExpr
+	if len(m.Requirements) > 0 {
+		requirements = m.Requirements
+	} else if len(m.Service.Requirements) > 0 {
+		requirements = m.Service.Requirements
+	}
+	for _, r := range requirements {
+		for _, s := range r.Schemes {
+			verr.Merge(s.Validate())
+			switch s.Kind {
+			case BasicAuthKind:
+				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)
+				}
+				if !hasTag(m.Payload, "security:password") {
+					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:
+				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:
+				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:
+				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)
 				}
 			}
-			for _, scope := range r.Scopes {
-				found := false
-				for _, s := range r.Schemes {
-					if s.Kind == BasicAuthKind || s.Kind == APIKeyKind || s.Kind == OAuth2Kind || s.Kind == JWTKind {
-						for _, se := range s.Scopes {
-							if se.Name == scope {
-								found = true
-								break
-							}
+		}
+		for _, scope := range r.Scopes {
+			found := false
+			for _, s := range r.Schemes {
+				if s.Kind == BasicAuthKind || s.Kind == APIKeyKind || s.Kind == OAuth2Kind || s.Kind == JWTKind {
+					for _, se := range s.Scopes {
+						if se.Name == scope {
+							found = true
+							break
 						}
 					}
 				}
-				if !found {
-					verr.Add(m, "security scope %q not found in any of the security schemes.", scope)
-				}
+			}
+			if !found {
+				verr.Add(m, "security scope %q not found in any of the security schemes.", scope)
 			}
 		}
 	}
diff --git a/expr/root.go b/expr/root.go
index daf70800..ac93b047 100644
--- a/expr/root.go
+++ b/expr/root.go
@@ -277,3 +277,19 @@ func (m MetaExpr) Merge(src MetaExpr) {
 		}
 	}
 }
+
+// Last returns the last value for a specific key, if the key exists and has
+// values; otherwise returns an empty string, with the "ok" flag set to false.
+func (m MetaExpr) Last(key string) (string, bool) {
+	v, ok := m[key]
+	if !ok {
+		return "", false
+	}
+
+	l := len(v)
+	if l < 1 {
+		return "", false
+	}
+
+	return v[l-1], true
+}


Test patch:
diff --git a/expr/example_test.go b/expr/example_test.go
index 5735e71a..26e5cc16 100644
--- a/expr/example_test.go
+++ b/expr/example_test.go
@@ -59,11 +59,13 @@ func TestExample(t *testing.T) {
 		{"with-example", testdata.WithExampleDSL, "example", ""},
 		{"with-array-example", testdata.WithArrayExampleDSL, []int{1, 2}, ""},
 		{"with-map-example", testdata.WithMapExampleDSL, map[string]int{"name": 1, "value": 2}, ""},
-		{"with-mulitple-examples", testdata.WithMultipleExamplesDSL, 100, ""},
+		{"with-multiple-examples", testdata.WithMultipleExamplesDSL, 100, ""},
 		{"overriding-example", testdata.OverridingExampleDSL, map[string]interface{}{"name": "overridden"}, ""},
 		{"with-extend", testdata.WithExtendExampleDSL, map[string]interface{}{"name": "example"}, ""},
 		{"invalid-example-type", testdata.InvalidExampleTypeDSL, nil, "example value map[int]int{1:1} is incompatible with attribute of type map in attribute"},
 		{"empty-example", testdata.EmptyExampleDSL, nil, "not enough arguments in attribute"},
+		{"hiding-example", testdata.HidingExampleDSL, nil, ""},
+		{"overriding-hidden-examples", testdata.OverridingHiddenExamplesDSL, "example", ""},
 	}
 	r := expr.NewRandom("test")
 	for _, k := range cases {
@@ -75,9 +77,12 @@ func TestExample(t *testing.T) {
 					t.Errorf("invalid example: got %v, expected %v", example, k.Expected)
 				}
 			} else {
-				err := expr.RunInvalidDSL(t, k.DSL)
-				if !strings.Contains(err.Error(), k.Error) {
-					t.Errorf("invalid error: got %q, expected %q", err.Error(), k.Error)
+				if err := expr.RunInvalidDSL(t, k.DSL); err == nil {
+					t.Error("the expected error was not returned")
+				} else {
+					if !strings.Contains(err.Error(), k.Error) {
+						t.Errorf("invalid error: got %q, expected %q", err.Error(), k.Error)
+					}
 				}
 			}
 		})
diff --git a/expr/http_endpoint_test.go b/expr/http_endpoint_test.go
index f89afc6c..a4fa587d 100644
--- a/expr/http_endpoint_test.go
+++ b/expr/http_endpoint_test.go
@@ -57,6 +57,12 @@ func TestHTTPEndpointValidation(t *testing.T) {
 				"service \"Service\" method \"Method\": payload of method \"Method\" of service \"Service\" does not define a JWT attribute, use Token to define one",
 			},
 		},
+		"endpoint-missing-token-payload": {
+			DSL: testdata.EndpointMissingTokenPayload,
+			Errors: []string{
+				"service \"Service\" method \"Method\": payload of method \"Method\" of service \"Service\" does not define a JWT attribute, use Token to define one",
+			},
+		},
 		"endpoint-missing-token-extend": {
 			DSL: testdata.EndpointExtendToken,
 		},
diff --git a/expr/root_test.go b/expr/root_test.go
index fe2f6c40..b721e2d4 100644
--- a/expr/root_test.go
+++ b/expr/root_test.go
@@ -43,3 +43,56 @@ func TestRootExprValidate(t *testing.T) {
 		}
 	}
 }
+
+func TestMetaExpr_Last(t *testing.T) {
+	tt := map[string]struct {
+		meta  MetaExpr
+		value string
+		ok    bool
+	}{
+		"no-key": {
+			MetaExpr{},
+			"",
+			false,
+		},
+		"key-no-values": {
+			MetaExpr{
+				"test:key": []string{},
+			},
+			"",
+			false,
+		},
+		"key-with-one-value": {
+			MetaExpr{
+				"test:key": []string{
+					"value-one",
+				},
+			},
+			"value-one",
+			true,
+		},
+		"key-with-multiple-values": {
+			MetaExpr{
+				"test:key": []string{
+					"value-one",
+					"value-two",
+					"value-n",
+				},
+			},
+			"value-n",
+			true,
+		},
+	}
+
+	for name, tc := range tt {
+		t.Run(name, func(t *testing.T) {
+			value, ok := tc.meta.Last("test:key")
+			if tc.ok != ok {
+				t.Errorf("expected ok to be %v, got %v", tc.ok, ok)
+			}
+			if tc.value != value {
+				t.Errorf("expected value to be %s, got %s", value, value)
+			}
+		})
+	}
+}
diff --git a/expr/testdata/endpoint_dsls.go b/expr/testdata/endpoint_dsls.go
index 99f848f2..9765fcae 100644
--- a/expr/testdata/endpoint_dsls.go
+++ b/expr/testdata/endpoint_dsls.go
@@ -125,6 +125,22 @@ var EndpointMissingToken = func() {
 	})
 }
 
+var EndpointMissingTokenPayload = func() {
+	var JWT = JWTSecurity("JWT", func() {
+		Scope("api:read", "Read access")
+	})
+	Service("Service", func() {
+		Security(JWT, func() {
+			Scope("api:read")
+		})
+		Method("Method", func() {
+			HTTP(func() {
+				POST("/")
+			})
+		})
+	})
+}
+
 var EndpointExtendToken = func() {
 	var CommonAttributes = Type("Common", func() {
 		Token("token", String)
diff --git a/expr/testdata/example_test_dsls.go b/expr/testdata/example_test_dsls.go
index d0cf4a5b..f81b19eb 100644
--- a/expr/testdata/example_test_dsls.go
+++ b/expr/testdata/example_test_dsls.go
@@ -95,3 +95,24 @@ var EmptyExampleDSL = func() {
 		})
 	})
 }
+
+var HidingExampleDSL = func() {
+	Service("HidingExample", func() {
+		Method("Method", func() {
+			Payload(String, func() {
+				Meta("swagger:example", "false")
+			})
+		})
+	})
+}
+
+var OverridingHiddenExamplesDSL = func() {
+	Service("OverridingHiddenExamples", func() {
+		Meta("swagger:example", "false")
+		Method("Method", func() {
+			Payload(String, func() {
+				Example("example")
+			})
+		})
+	})
+}