fix missing scalar alias, add Accept header and improve response deco…

…ding (#43)

* Fix missing scalar type alias from primitive scalars.
* Add the Accept request header with the value from the response content type.
* Decode the response in JSON/XML format if the content type contains +json or +xml suffix.
* Mask sensitive headers that are forwarded from the v3-engine

connector/connector_test.go

@@ -143,7 +143,7 @@ func TestHTTPConnector_authentication(t *testing.T) {
 		assertHTTPResponse(t, res, http.StatusOK, schema.ExplainResponse{
 			Details: schema.ExplainResponseDetails{
 				"url":     server.URL + "/pet",
-				"headers": `{"Api_key":["ran*******(14)"],"Content-Type":["application/json"]}`,
+				"headers": `{"Accept":["application/json"],"Api_key":["ran*******(14)"],"Content-Type":["application/json"]}`,
 			},
 		})
 	})
@@ -201,7 +201,7 @@ func TestHTTPConnector_authentication(t *testing.T) {
 		assertHTTPResponse(t, res, http.StatusOK, schema.ExplainResponse{
 			Details: schema.ExplainResponseDetails{
 				"url":     server.URL + "/pet",
-				"headers": `{"Api_key":["ran*******(14)"],"Content-Type":["application/json"]}`,
+				"headers": `{"Accept":["application/json"],"Api_key":["ran*******(14)"],"Content-Type":["application/json"]}`,
 				"body":    `{"name":"pet"}`,
 			},
 		})
@@ -253,7 +253,7 @@ func TestHTTPConnector_authentication(t *testing.T) {
 		assertHTTPResponse(t, res, http.StatusOK, schema.ExplainResponse{
 			Details: schema.ExplainResponseDetails{
 				"url":     server.URL + "/pet/findByStatus?status=available",
-				"headers": `{"Authorization":["Bearer ran*******(19)"],"Content-Type":["application/json"],"X-Custom-Header":["This is a test"]}`,
+				"headers": `{"Accept":["application/json"],"Authorization":["Bearer ran*******(19)"],"Content-Type":["application/json"],"X-Custom-Header":["This is a test"]}`,
 			},
 		})
 	})

connector/internal/client.go

@@ -373,7 +373,7 @@ func (client *HTTPClient) evalHTTPResponse(ctx context.Context, span trace.Span,
 		}
 
 		result = string(respBody)
-	case contentType == rest.ContentTypeXML:
+	case contentType == rest.ContentTypeXML || strings.HasSuffix(contentType, "+xml"):
 		field, extractErr := client.extractResultType(resultType)
 		if extractErr != nil {
 			return nil, nil, extractErr
@@ -384,7 +384,7 @@ func (client *HTTPClient) evalHTTPResponse(ctx context.Context, span trace.Span,
 		if err != nil {
 			return nil, nil, schema.NewConnectorError(http.StatusInternalServerError, err.Error(), nil)
 		}
-	case contentType == rest.ContentTypeJSON:
+	case contentType == rest.ContentTypeJSON || strings.HasSuffix(contentType, "+json"):
 		if len(resultType) > 0 {
 			namedType, err := resultType.AsNamed()
 			if err == nil && namedType.Name == string(rest.ScalarString) {

connector/internal/request_builder.go

@@ -56,6 +56,10 @@ func (c *RequestBuilder) Build() (*RetryableRequest, error) {
 		return nil, err
 	}
 
+	if rawRequest.Response.ContentType != "" && request.Headers.Get(acceptHeader) == "" {
+		request.Headers.Set(acceptHeader, evalAcceptContentType(rawRequest.Response.ContentType))
+	}
+
 	if rawRequest.RuntimeSettings != nil {
 		if rawRequest.RuntimeSettings.Timeout > 0 {
 			request.Runtime.Timeout = rawRequest.RuntimeSettings.Timeout

connector/internal/request_builder_test.go

@@ -77,6 +77,57 @@ func TestEvalURLAndHeaderParameters(t *testing.T) {
 	}
 }
 
+func TestEvalURLAndHeaderParametersOAS2(t *testing.T) {
+	testCases := []struct {
+		name         string
+		rawArguments string
+		expectedURL  string
+		errorMsg     string
+		headers      map[string]string
+	}{
+		{
+			name: "get_subject",
+			rawArguments: `{
+				"identifier": "thesauri/material/AAT.11914"
+			}`,
+			expectedURL: "/id/thesauri/material/AAT.11914",
+		},
+	}
+
+	ndcSchema := createMockSchemaOAS2(t)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var info *rest.OperationInfo
+			for key, f := range ndcSchema.Functions {
+				if key == tc.name {
+					info = &f
+					break
+				}
+			}
+			var arguments map[string]any
+			assert.NilError(t, json.Unmarshal([]byte(tc.rawArguments), &arguments))
+
+			builder := RequestBuilder{
+				Schema:    ndcSchema,
+				Operation: info,
+				Arguments: arguments,
+			}
+			result, headers, err := builder.evalURLAndHeaderParameters()
+			if tc.errorMsg != "" {
+				assert.ErrorContains(t, err, tc.errorMsg)
+			} else {
+				assert.NilError(t, err)
+				decodedValue, err := url.QueryUnescape(result.String())
+				assert.NilError(t, err)
+				assert.Equal(t, tc.expectedURL, decodedValue)
+				for k, v := range tc.headers {
+					assert.Equal(t, v, headers.Get(k))
+				}
+			}
+		})
+	}
+}
+
 func createMockSchema(t *testing.T) *rest.NDCHttpSchema {
 	var ndcSchema rest.NDCHttpSchema
 	rawSchemaBytes, err := os.ReadFile("../../ndc-http-schema/openapi/testdata/petstore3/expected.json")
@@ -85,3 +136,12 @@ func createMockSchema(t *testing.T) *rest.NDCHttpSchema {
 
 	return &ndcSchema
 }
+
+func createMockSchemaOAS2(t *testing.T) *rest.NDCHttpSchema {
+	var ndcSchema rest.NDCHttpSchema
+	rawSchemaBytes, err := os.ReadFile("../../ndc-http-schema/openapi/testdata/petstore2/expected.json")
+	assert.NilError(t, err)
+	assert.NilError(t, json.Unmarshal(rawSchemaBytes, &ndcSchema))
+
+	return &ndcSchema
+}

connector/internal/types.go

@@ -12,6 +12,7 @@ import (
 )
 
 const (
+	acceptHeader               = "Accept"
 	contentTypeHeader          = "Content-Type"
 	defaultTimeoutSeconds uint = 30
 	defaultRetryDelays    uint = 1000
@@ -30,7 +31,6 @@ type HTTPOptions struct {
 	Servers  []string `json:"serverIds" yaml:"serverIds"`
 	Parallel bool     `json:"parallel"  yaml:"parallel"`
 
-	Explain     bool `json:"-" yaml:"-"`
 	Distributed bool `json:"-" yaml:"-"`
 	Concurrency uint `json:"-" yaml:"-"`
 }

connector/internal/utils.go

@@ -10,13 +10,29 @@ import (
 	"go.opentelemetry.io/otel/trace"
 )
 
+// IsSensitiveHeader checks if the header name is sensitive.
+func IsSensitiveHeader(name string) bool {
+	return sensitiveHeaderRegex.MatchString(strings.ToLower(name))
+}
+
+func evalAcceptContentType(contentType string) string {
+	switch {
+	case strings.HasPrefix(contentType, "image/"):
+		return "image/*"
+	case strings.HasPrefix(contentType, "video/"):
+		return "video/*"
+	default:
+		return contentType
+	}
+}
+
 func setHeaderAttributes(span trace.Span, prefix string, httpHeaders http.Header) {
 	for key, headers := range httpHeaders {
 		if len(headers) == 0 {
 			continue
 		}
 		values := headers
-		if sensitiveHeaderRegex.MatchString(strings.ToLower(key)) {
+		if IsSensitiveHeader(key) {
 			values = make([]string, len(headers))
 			for i, header := range headers {
 				values[i] = utils.MaskString(header)

connector/internal/utils_test.go

@@ -0,0 +1,13 @@
+package internal
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestEvalAcceptContentType(t *testing.T) {
+	assert.Equal(t, "image/*", evalAcceptContentType("image/jpeg"))
+	assert.Equal(t, "video/*", evalAcceptContentType("video/mp4"))
+	assert.Equal(t, "application/json", evalAcceptContentType("application/json"))
+}

connector/query.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/hasura/ndc-http/connector/internal"
 	"github.com/hasura/ndc-http/ndc-http-schema/configuration"
+	restUtils "github.com/hasura/ndc-http/ndc-http-schema/utils"
 	"github.com/hasura/ndc-sdk-go/schema"
 	"github.com/hasura/ndc-sdk-go/utils"
 	"go.opentelemetry.io/otel/codes"
@@ -175,6 +176,13 @@ func (c *HTTPConnector) serializeExplainResponse(ctx context.Context, requests *
 	}
 	defer cancel()
 
+	// mask sensitive forwarded headers if exists
+	for key := range req.Header {
+		if internal.IsSensitiveHeader(key) {
+			req.Header.Set(key, restUtils.MaskString(req.Header.Get(key)))
+		}
+	}
+
 	c.upstreams.InjectMockRequestSettings(req, requests.Schema.Name, httpRequest.RawRequest.Security)
 
 	explainResp.Details["url"] = req.URL.String()

ndc-http-schema/openapi/internal/oas2.go

@@ -210,9 +210,16 @@ func (oc *OAS2Builder) convertComponentSchemas(schemaItem orderedmap.Pair[string
 	typeSchema := typeValue.Schema()
 
 	oc.Logger.Debug("component schema", slog.String("name", typeKey))
+	if _, ok := oc.schema.ObjectTypes[typeKey]; ok {
+		return nil
+	}
+	if _, ok := oc.schema.ScalarTypes[typeKey]; ok {
+		return nil
+	}
 	if typeSchema == nil {
 		return nil
 	}
+
 	typeEncoder, schemaResult, err := newOAS2SchemaBuilder(oc, "", rest.InBody).getSchemaType(typeSchema, []string{typeKey})
 
 	var typeName string
@@ -229,6 +236,13 @@ func (oc *OAS2Builder) convertComponentSchemas(schemaItem orderedmap.Pair[string
 		}
 	}
 
+	// If the result type is a scalar, the builder returns the raw scalar name (String, Int).
+	// We should check and add the alias type to scalar objects
+	pascalTypeName := utils.ToPascalCase(typeKey)
+	if scalarType, ok := oc.schema.ScalarTypes[typeName]; ok && pascalTypeName != typeName {
+		oc.schema.ScalarTypes[pascalTypeName] = scalarType
+	}
+
 	cacheKey := "#/definitions/" + typeKey
 	// treat no-property objects as a Arbitrary JSON scalar
 	if typeEncoder == nil || typeName == string(rest.ScalarJSON) {

ndc-http-schema/openapi/internal/oas3.go

@@ -262,6 +262,10 @@ func (oc *OAS3Builder) convertComponentSchemas(schemaItem orderedmap.Pair[string
 	if _, ok := oc.schema.ObjectTypes[typeKey]; ok {
 		return nil
 	}
+	if _, ok := oc.schema.ScalarTypes[typeKey]; ok {
+		return nil
+	}
+
 	typeEncoder, schemaResult, err := newOAS3SchemaBuilder(oc, "", rest.InBody, false).
 		getSchemaType(typeSchema, []string{typeKey})
 	if err != nil {
@@ -282,6 +286,13 @@ func (oc *OAS3Builder) convertComponentSchemas(schemaItem orderedmap.Pair[string
 		}
 	}
 
+	// If the result type is a scalar, the builder returns the raw scalar name (String, Int).
+	// We should check and add the alias type to scalar objects
+	pascalTypeName := utils.ToPascalCase(typeKey)
+	if scalarType, ok := oc.schema.ScalarTypes[typeName]; ok && pascalTypeName != typeName {
+		oc.schema.ScalarTypes[pascalTypeName] = scalarType
+	}
+
 	cacheKey := "#/components/schemas/" + typeKey
 	// treat no-property objects as a Arbitrary JSON scalar
 	if typeEncoder == nil || typeName == string(rest.ScalarJSON) {

ndc-http-schema/openapi/testdata/petstore2/expected.json

@@ -564,6 +564,38 @@
         "type": "named"
       }
     },
+    "get_subject": {
+      "request": {
+        "url": "/id/{identifier}",
+        "method": "get",
+        "response": {
+          "contentType": "application/json"
+        }
+      },
+      "arguments": {
+        "identifier": {
+          "description": "The identifier path of the `subject` you're looking for",
+          "type": {
+            "name": "String",
+            "type": "named"
+          },
+          "http": {
+            "name": "identifier",
+            "in": "path",
+            "schema": {
+              "type": [
+                "string"
+              ]
+            }
+          }
+        }
+      },
+      "description": "Explore details about a given subject node",
+      "result_type": {
+        "name": "JSON",
+        "type": "named"
+      }
+    },
     "listOAuth2Clients": {
       "request": {
         "url": "/clients",

ndc-http-schema/openapi/testdata/petstore2/schema.json

@@ -260,6 +260,23 @@
         "type": "named"
       }
     },
+    {
+      "arguments": {
+        "identifier": {
+          "description": "The identifier path of the `subject` you're looking for",
+          "type": {
+            "name": "String",
+            "type": "named"
+          }
+        }
+      },
+      "description": "Explore details about a given subject node",
+      "name": "get_subject",
+      "result_type": {
+        "name": "JSON",
+        "type": "named"
+      }
+    },
     {
       "arguments": {
         "client_name": {

ndc-http-schema/openapi/testdata/petstore2/swagger.json

@@ -904,6 +904,30 @@
           }
         ]
       }
+    },
+    "/id/{identifier}": {
+      "get": {
+        "operationId": "get subject",
+        "parameters": [
+          {
+            "description": "The identifier path of the `subject` you're looking for\n",
+            "in": "path",
+            "name": "identifier",
+            "required": true,
+            "type": "string"
+          }
+        ],
+        "produces": ["text/html", "application/ld+json", "application/json"],
+        "responses": {
+          "200": {
+            "description": "`subject` found\n"
+          },
+          "404": {
+            "description": "`subject` not found\n"
+          }
+        },
+        "summary": "Explore details about a given subject node"
+      }
     }
   },
   "securityDefinitions": {