Merge pull request #197 from jhernand/honor_http_annotation_in_query_…

…parameters

Honor `@http` annotation for query parameters

pkg/generators/golang/clients_generator.go

@@ -452,7 +452,7 @@ func (g *ClientsGenerator) generateResourceClient(resource *concepts.Resource) e
 		Function("locatorSegment", g.binding.LocatorSegment).
 		Function("methodName", g.methodName).
 		Function("methodSegment", g.binding.MethodSegment).
-		Function("parameterName", g.binding.ParameterName).
+		Function("parameterName", g.binding.QueryParameterName).
 		Function("pollRequestName", g.pollRequestName).
 		Function("pollResponseName", g.pollResponseName).
 		Function("readResponseFunc", g.readResponseFunc).

pkg/generators/golang/json_generator.go

@@ -809,7 +809,7 @@ func (g *JSONSupportGenerator) generateResourceSupport(resource *concepts.Resour
 		Function("generateWriteValue", g.generateWriteValue).
 		Function("marshalTypeFunc", g.marshalTypeFunc).
 		Function("parameterFieldName", g.parameterFieldName).
-		Function("parameterFieldTag", g.binding.ParameterName).
+		Function("parameterQueryName", g.binding.QueryParameterName).
 		Function("readResponseFunc", g.readResponseFunc).
 		Function("readTypeFunc", g.readTypeFunc).
 		Function("requestBodyParameters", g.binding.RequestBodyParameters).
@@ -1216,11 +1216,11 @@ func (g *JSONSupportGenerator) generateReadQueryParameter(parameter *concepts.Pa
 		return ""
 	}
 	field := g.parameterFieldName(parameter)
-	tag := g.binding.ParameterName(parameter)
+	tag := g.binding.QueryParameterName(parameter)
 	kind := typ.Name().Camel()
 	return g.buffer.Eval(`
 		{{ $field := parameterFieldName .Parameter }}
-		{{ $tag := parameterFieldTag .Parameter }}
+		{{ $tag := parameterQueryName .Parameter }}
 		{{ $kind := .Parameter.Type.Name.Camel }}
 
 		request.{{ $field }}, err = helpers.Parse{{ $kind }}(query, "{{ $tag }}")
@@ -1243,7 +1243,7 @@ func (g *JSONSupportGenerator) generateReadQueryParameter(parameter *concepts.Pa
 func (g *JSONSupportGenerator) generateReadBodyParameter(object string, parameter *concepts.
 	Parameter) string {
 	field := g.parameterFieldName(parameter)
-	tag := g.binding.ParameterName(parameter)
+	tag := g.binding.BodyParameterName(parameter)
 	typ := parameter.Type()
 	return g.buffer.Eval(`
 		case "{{ .Tag }}":
@@ -1336,7 +1336,7 @@ func (g *JSONSupportGenerator) generateWriteBodyParameter(object string,
 	parameter *concepts.Parameter) string {
 	typ := parameter.Type()
 	field := g.parameterFieldName(parameter)
-	tag := g.binding.ParameterName(parameter)
+	tag := g.binding.BodyParameterName(parameter)
 	var value string
 	if typ.IsScalar() && !typ.IsInterface() {
 		value = g.buffer.Eval(

pkg/generators/openapi/openapi_generator.go

@@ -378,7 +378,7 @@ func (g *OpenAPIGenerator) generatePathParameter(locator *concepts.Locator) {
 
 func (g *OpenAPIGenerator) generateQueryParameter(parameter *concepts.Parameter) {
 	g.buffer.StartObject()
-	g.buffer.Field("name", g.binding.ParameterName(parameter))
+	g.buffer.Field("name", g.binding.QueryParameterName(parameter))
 	g.generateDescription(parameter.Doc())
 	g.buffer.Field("in", "query")
 	g.buffer.StartObject("schema")

pkg/http/binding_calculator.go

@@ -190,9 +190,27 @@ func (c *BindingCalculator) AttributeName(attribute *concepts.Attribute) string
 	return name
 }
 
-// ParameterName returns the name of the field or query parameter corresponding to the given  model
+// QueryParameterName returns the name of the query parameter corresponding to the given model
 // method parameter.
-func (c *BindingCalculator) ParameterName(parameter *concepts.Parameter) string {
+func (c *BindingCalculator) QueryParameterName(parameter *concepts.Parameter) string {
+	name := annotations.HTTPName(parameter)
+	if name == "" {
+		// We check also the JSON annotation here for "bug compatibility". In the past we
+		// used to check only the JSON name, but that is incorrect because this is about
+		// HTTP query parameters, not JSON field names. But some models (the 'dryRun'
+		// parameter of the accounts management service, for example) are already using that
+		// bug as it was a feature.
+		name = annotations.JSONName(parameter)
+		if name == "" {
+			name = parameter.Name().Snake()
+		}
+	}
+	return name
+}
+
+// BodyParameterName returns the name of the body field corresponding to the given  model method
+// parameter.
+func (c *BindingCalculator) BodyParameterName(parameter *concepts.Parameter) string {
 	name := annotations.JSONName(parameter)
 	if name == "" {
 		name = parameter.Name().Snake()

tests/go/clients_test.go

@@ -527,4 +527,90 @@ var _ = Describe("Client", func() {
 		Expect(response.Status()).To(Equal(http.StatusNoContent))
 		Expect(response.Body()).To(BeNil())
 	})
+
+	It("Honors @http in query parameter", func() {
+		// Prepare the server:
+		server.AppendHandlers(
+			CombineHandlers(
+				VerifyFormKV("dryRun", "true"),
+				RespondWith(http.StatusOK, `{}`),
+			),
+		)
+
+		// Prepare the body:
+		body, err := cmv1.NewCluster().
+			Name("my").
+			Build()
+		Expect(err).ToNot(HaveOccurred())
+
+		// Send the request:
+		client := cmv1.NewClustersClient(transport, "/api/clusters_mgmt/v1/clusters")
+		response, err := client.Add().
+			DryRun(true).
+			Body(body).
+			Send()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(response).ToNot(BeNil())
+	})
+
+	It("Honors @http in path segment", func() {
+		// Prepare the server:
+		server.AppendHandlers(
+			CombineHandlers(
+				VerifyRequest(
+					http.MethodPost,
+					"/api/clusters_mgmt/v1/clusters/my_test",
+				),
+				RespondWith(http.StatusOK, `{}`),
+			),
+		)
+
+		// Send the request:
+		client := cmv1.NewClustersClient(transport, "/api/clusters_mgmt/v1/clusters")
+		response, err := client.TestAnnotations().Send()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(response).ToNot(BeNil())
+	})
+
+	It("Honors @json and ignores @http in request body parameter", func() {
+		// Prepare the server:
+		server.AppendHandlers(
+			CombineHandlers(
+				VerifyJSON(`{
+					"my_json": true
+				}`),
+				RespondWith(http.StatusOK, `{}`),
+			),
+		)
+
+		// Send the request:
+		client := cmv1.NewClustersClient(transport, "/api/clusters_mgmt/v1/clusters")
+		response, err := client.TestAnnotations().
+			My(true).
+			Send()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(response).ToNot(BeNil())
+	})
+
+	It("Honors @json and ignores @http in response body parameter", func() {
+		// Prepare the server:
+		server.AppendHandlers(
+			CombineHandlers(
+				RespondWith(http.StatusOK, `{
+					"my_json": true
+				}`),
+			),
+		)
+
+		// Send the request:
+		client := cmv1.NewClustersClient(transport, "/api/clusters_mgmt/v1/clusters")
+		response, err := client.TestAnnotations().
+			My(true).
+			Send()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(response).ToNot(BeNil())
+		value, ok := response.GetMy()
+		Expect(ok).To(BeTrue())
+		Expect(value).To(BeTrue())
+	})
 })

tests/model/clusters_mgmt/v1/clusters_resource.model

@@ -67,10 +67,21 @@ resource Clusters {
 	//
 	// See the `register_cluster` method for adding an existing cluster.
 	method Add {
+                @http(name = "dryRun")
+                in DryRun Boolean
+
 		// Description of the cluster.
 		in out Body Cluster
 	}
 
+	// This is used in tests to verify that annotations are interpreted correctly.
+	@http(name = "my_test")
+	method TestAnnotations {
+                @json(name = "my_json") // Should be honored.
+                @http(name = "my_http") // Should be ignored.
+                in out My Boolean
+	}
+
 	// Returns a reference to the service that manages an specific cluster.
 	locator Cluster {
 		target Cluster