From 6038995fef5567a90ffe1813e98808b538c9e878 Mon Sep 17 00:00:00 2001 From: Vincent Marceau Date: Mon, 13 Apr 2026 10:08:21 -0400 Subject: [PATCH 1/4] perf: add schema cache lookup to parameter validation --- parameters/validate_parameter.go | 117 ++++++++++++++----- parameters/validate_parameter_test.go | 159 ++++++++++++++++++++++++++ 2 files changed, 245 insertions(+), 31 deletions(-) diff --git a/parameters/validate_parameter.go b/parameters/validate_parameter.go index 1b7de9ca..ec32fa81 100644 --- a/parameters/validate_parameter.go +++ b/parameters/validate_parameter.go @@ -18,6 +18,7 @@ import ( stdError "errors" + "github.com/pb33f/libopenapi-validator/cache" "github.com/pb33f/libopenapi-validator/config" "github.com/pb33f/libopenapi-validator/errors" "github.com/pb33f/libopenapi-validator/helpers" @@ -35,16 +36,41 @@ func ValidateSingleParameterSchema( pathTemplate string, operation string, ) (validationErrors []*errors.ValidationError) { - // Get the JSON Schema for the parameter definition. - jsonSchema, err := buildJsonRender(schema) - if err != nil { - return validationErrors + var jsch *jsonschema.Schema + var jsonSchema []byte + + // Try cache lookup first - avoids expensive schema compilation on each request + if o != nil && o.SchemaCache != nil && schema != nil && schema.GoLow() != nil { + hash := schema.GoLow().Hash() + if cached, ok := o.SchemaCache.Load(hash); ok && cached != nil && cached.CompiledSchema != nil { + jsch = cached.CompiledSchema + } } - // Attempt to compile the JSON Schema - jsch, err := helpers.NewCompiledSchema(name, jsonSchema, o) - if err != nil { - return validationErrors + // Cache miss - compile the schema + if jsch == nil { + // Get the JSON Schema for the parameter definition. + var err error + jsonSchema, err = buildJsonRender(schema) + if err != nil { + return validationErrors + } + + // Attempt to compile the JSON Schema + jsch, err = helpers.NewCompiledSchema(name, jsonSchema, o) + if err != nil { + return validationErrors + } + + // Store in cache for future requests + if o != nil && o.SchemaCache != nil && schema != nil && schema.GoLow() != nil { + hash := schema.GoLow().Hash() + o.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ + Schema: schema, + RenderedJSON: jsonSchema, + CompiledSchema: jsch, + }) + } } // Validate the object and report any errors. @@ -94,13 +120,60 @@ func ValidateParameterSchema( validationOptions *config.ValidationOptions, ) []*errors.ValidationError { var validationErrors []*errors.ValidationError + var jsch *jsonschema.Schema + var jsonSchema []byte - // 1. build a JSON render of the schema. - renderCtx := base.NewInlineRenderContextForValidation() - renderedSchema, _ := schema.RenderInlineWithContext(renderCtx) - jsonSchema, _ := utils.ConvertYAMLtoJSON(renderedSchema) + // Try cache lookup first - avoids expensive schema compilation on each request + if validationOptions != nil && validationOptions.SchemaCache != nil && schema != nil && schema.GoLow() != nil { + hash := schema.GoLow().Hash() + if cached, ok := validationOptions.SchemaCache.Load(hash); ok && cached != nil && cached.CompiledSchema != nil { + jsch = cached.CompiledSchema + jsonSchema = cached.RenderedJSON + } + } + + // Cache miss - render and compile the schema + if jsch == nil { + // 1. build a JSON render of the schema. + renderCtx := base.NewInlineRenderContextForValidation() + renderedSchema, _ := schema.RenderInlineWithContext(renderCtx) + referenceSchema := string(renderedSchema) + jsonSchema, _ = utils.ConvertYAMLtoJSON(renderedSchema) + + // 2. create a new json schema compiler and add the schema to it + var err error + jsch, err = helpers.NewCompiledSchema(name, jsonSchema, validationOptions) + if err != nil { + // schema compilation failed, return validation error instead of panicking + validationErrors = append(validationErrors, &errors.ValidationError{ + ValidationType: validationType, + ValidationSubType: subValType, + Message: fmt.Sprintf("%s '%s' failed schema compilation", entity, name), + Reason: fmt.Sprintf("%s '%s' schema compilation failed: %s", + reasonEntity, name, err.Error()), + SpecLine: 1, + SpecCol: 0, + ParameterName: name, + HowToFix: "check the parameter schema for invalid JSON Schema syntax, complex regex patterns, or unsupported schema constructs", + Context: string(jsonSchema), + }) + return validationErrors + } - // 2. decode the object into a json blob. + // Store in cache for future requests + if validationOptions != nil && validationOptions.SchemaCache != nil && schema != nil && schema.GoLow() != nil { + hash := schema.GoLow().Hash() + validationOptions.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ + Schema: schema, + RenderedInline: renderedSchema, + ReferenceSchema: referenceSchema, + RenderedJSON: jsonSchema, + CompiledSchema: jsch, + }) + } + } + + // 3. decode the object into a json blob. var decodedObj interface{} rawIsMap := false validEncoding := false @@ -125,24 +198,6 @@ func ValidateParameterSchema( } validEncoding = true } - // 3. create a new json schema compiler and add the schema to it - jsch, err := helpers.NewCompiledSchema(name, jsonSchema, validationOptions) - if err != nil { - // schema compilation failed, return validation error instead of panicking - validationErrors = append(validationErrors, &errors.ValidationError{ - ValidationType: validationType, - ValidationSubType: subValType, - Message: fmt.Sprintf("%s '%s' failed schema compilation", entity, name), - Reason: fmt.Sprintf("%s '%s' schema compilation failed: %s", - reasonEntity, name, err.Error()), - SpecLine: 1, - SpecCol: 0, - ParameterName: name, - HowToFix: "check the parameter schema for invalid JSON Schema syntax, complex regex patterns, or unsupported schema constructs", - Context: string(jsonSchema), - }) - return validationErrors - } // 4. validate the object against the schema var scErrs error diff --git a/parameters/validate_parameter_test.go b/parameters/validate_parameter_test.go index 9c51395a..4e26ab8e 100644 --- a/parameters/validate_parameter_test.go +++ b/parameters/validate_parameter_test.go @@ -12,6 +12,7 @@ import ( lowv3 "github.com/pb33f/libopenapi/datamodel/low/v3" + "github.com/pb33f/libopenapi-validator/cache" "github.com/pb33f/libopenapi-validator/config" "github.com/pb33f/libopenapi-validator/helpers" ) @@ -678,3 +679,161 @@ func BenchmarkValidationWithRegexCache(b *testing.B) { validator.ValidatePathParams(req) } } + +// cacheTestSpec is an OpenAPI spec for testing cache behavior +var cacheTestSpec = []byte(`{ + "openapi": "3.1.0", + "info": { + "title": "Cache Test API", + "version": "1.0.0" + }, + "paths": { + "/items/{id}": { + "get": { + "operationId": "getItem", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string", + "minLength": 1, + "maxLength": 64 + } + }, + { + "name": "limit", + "in": "query", + "schema": { + "type": "integer", + "minimum": 1, + "maximum": 100 + } + } + ], + "responses": { + "200": { + "description": "OK" + } + } + } + } + } +}`) + +// Test_ParameterValidation_CacheUsage verifies that parameter validation uses the schema cache. +// This test validates that: +// 1. Cache is populated after the first validation +// 2. Subsequent validations reuse the cached compiled schemas +// 3. Validation still produces correct results when using cached schemas +func Test_ParameterValidation_CacheUsage(t *testing.T) { + doc, err := libopenapi.NewDocument(cacheTestSpec) + require.NoError(t, err, "Failed to create document") + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs, "Failed to build v3 model") + + // Create options with cache (default behavior) + opts := config.NewValidationOptions() + require.NotNil(t, opts.SchemaCache, "Schema cache should be initialized by default") + + validator := NewParameterValidator(&v3Model.Model, config.WithExistingOpts(opts)) + + // First request - should populate cache + req1, _ := http.NewRequest("GET", "/items/abc123?limit=50", nil) + isSuccess1, errors1 := validator.ValidateQueryParams(req1) + assert.True(t, isSuccess1, "First validation should succeed") + assert.Empty(t, errors1, "First validation should have no errors") + + // Count cached entries (should have at least the limit parameter schema) + cacheCount := 0 + opts.SchemaCache.Range(func(key uint64, value *cache.SchemaCacheEntry) bool { + cacheCount++ + return true + }) + assert.Greater(t, cacheCount, 0, "Cache should have entries after first validation") + + // Second request with different valid value - should use cached schema + req2, _ := http.NewRequest("GET", "/items/xyz789?limit=75", nil) + isSuccess2, errors2 := validator.ValidateQueryParams(req2) + assert.True(t, isSuccess2, "Second validation should succeed") + assert.Empty(t, errors2, "Second validation should have no errors") + + // Third request with invalid value - should still use cached schema but fail validation + req3, _ := http.NewRequest("GET", "/items/test?limit=999", nil) + isSuccess3, errors3 := validator.ValidateQueryParams(req3) + assert.False(t, isSuccess3, "Third validation should fail (limit > maximum)") + assert.NotEmpty(t, errors3, "Third validation should have errors") +} + +// Test_ParameterValidation_WithoutCache verifies that validation works when cache is disabled. +func Test_ParameterValidation_WithoutCache(t *testing.T) { + doc, err := libopenapi.NewDocument(cacheTestSpec) + require.NoError(t, err, "Failed to create document") + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs, "Failed to build v3 model") + + // Create options without cache + opts := config.NewValidationOptions(config.WithSchemaCache(nil)) + require.Nil(t, opts.SchemaCache, "Schema cache should be nil") + + validator := NewParameterValidator(&v3Model.Model, config.WithExistingOpts(opts)) + + // Validation should still work without cache + req, _ := http.NewRequest("GET", "/items/abc123?limit=50", nil) + isSuccess, errors := validator.ValidateQueryParams(req) + assert.True(t, isSuccess, "Validation should succeed without cache") + assert.Empty(t, errors, "Validation should have no errors") + + // Validation with invalid value should fail + req2, _ := http.NewRequest("GET", "/items/abc123?limit=999", nil) + isSuccess2, errors2 := validator.ValidateQueryParams(req2) + assert.False(t, isSuccess2, "Validation should fail for invalid value") + assert.NotEmpty(t, errors2, "Validation should report errors") +} + +// Test_ParameterValidation_CacheConsistency verifies that cached schemas produce +// the same validation results as freshly compiled schemas. +func Test_ParameterValidation_CacheConsistency(t *testing.T) { + doc, err := libopenapi.NewDocument(cacheTestSpec) + require.NoError(t, err, "Failed to create document") + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs, "Failed to build v3 model") + + // Run the same validations with and without cache + testCases := []struct { + name string + url string + expected bool + }{ + {"valid_limit", "/items/abc?limit=50", true}, + {"limit_at_max", "/items/abc?limit=100", true}, + {"limit_at_min", "/items/abc?limit=1", true}, + {"limit_too_high", "/items/abc?limit=101", false}, + {"limit_too_low", "/items/abc?limit=0", false}, + } + + // First run with cache + optsWithCache := config.NewValidationOptions() + validatorWithCache := NewParameterValidator(&v3Model.Model, config.WithExistingOpts(optsWithCache)) + + // Second run without cache + optsNoCache := config.NewValidationOptions(config.WithSchemaCache(nil)) + validatorNoCache := NewParameterValidator(&v3Model.Model, config.WithExistingOpts(optsNoCache)) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", tc.url, nil) + + successWithCache, errorsWithCache := validatorWithCache.ValidateQueryParams(req) + successNoCache, errorsNoCache := validatorNoCache.ValidateQueryParams(req) + + assert.Equal(t, tc.expected, successWithCache, "Cached validation result mismatch for %s", tc.name) + assert.Equal(t, successWithCache, successNoCache, "Cache vs no-cache results should match for %s", tc.name) + assert.Equal(t, len(errorsWithCache), len(errorsNoCache), "Error count should match for %s", tc.name) + }) + } +} From 526d9254a33837232ad447088e3649c0bc2ef3cc Mon Sep 17 00:00:00 2001 From: Vincent Marceau Date: Mon, 13 Apr 2026 10:20:25 -0400 Subject: [PATCH 2/4] perf: use cached rendered schemas for error messages in parameter validation --- parameters/cookie_parameters.go | 10 +- parameters/header_parameters.go | 19 +- parameters/path_parameters.go | 19 +- parameters/query_parameters.go | 17 +- parameters/validate_parameter.go | 23 ++- parameters/validate_parameter_test.go | 283 +++++++++++++++++++++++++- parameters/validation_functions.go | 8 +- 7 files changed, 319 insertions(+), 60 deletions(-) diff --git a/parameters/cookie_parameters.go b/parameters/cookie_parameters.go index 46d7a2ab..c92883a5 100644 --- a/parameters/cookie_parameters.go +++ b/parameters/cookie_parameters.go @@ -4,7 +4,6 @@ package parameters import ( - "encoding/json" "fmt" "net/http" "strconv" @@ -70,13 +69,8 @@ func (v *paramValidator) ValidateCookieParamsWithPathItem(request *http.Request, sch = p.Schema.Schema() } - // Render schema once for ReferenceSchema field in errors - var renderedSchema string - if sch != nil { - rendered, _ := sch.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedSchema = string(schemaBytes) - } + // Get rendered schema for ReferenceSchema field in errors (uses cache if available) + renderedSchema := GetRenderedSchema(sch, v.options) pType := sch.Type diff --git a/parameters/header_parameters.go b/parameters/header_parameters.go index a924f754..624d07a2 100644 --- a/parameters/header_parameters.go +++ b/parameters/header_parameters.go @@ -4,7 +4,6 @@ package parameters import ( - "encoding/json" "fmt" "net/http" "strconv" @@ -60,13 +59,8 @@ func (v *paramValidator) ValidateHeaderParamsWithPathItem(request *http.Request, sch = p.Schema.Schema() } - // Render schema once for ReferenceSchema field in errors - var renderedSchema string - if sch != nil { - rendered, _ := sch.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedSchema = string(schemaBytes) - } + // Get rendered schema for ReferenceSchema field in errors (uses cache if available) + renderedSchema := GetRenderedSchema(sch, v.options) pType := sch.Type @@ -204,15 +198,10 @@ func (v *paramValidator) ValidateHeaderParamsWithPathItem(request *http.Request, } } else { if p.Required != nil && *p.Required { - // Render schema for missing required parameter + // Get rendered schema for missing required parameter (uses cache if available) var renderedSchema string if p.Schema != nil { - sch := p.Schema.Schema() - if sch != nil { - rendered, _ := sch.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedSchema = string(schemaBytes) - } + renderedSchema = GetRenderedSchema(p.Schema.Schema(), v.options) } validationErrors = append(validationErrors, errors.HeaderParameterMissing(p, pathValue, operation, renderedSchema)) } diff --git a/parameters/path_parameters.go b/parameters/path_parameters.go index bf7005cd..c5fd7ac6 100644 --- a/parameters/path_parameters.go +++ b/parameters/path_parameters.go @@ -4,7 +4,6 @@ package parameters import ( - "encoding/json" "fmt" "net/http" "net/url" @@ -142,13 +141,8 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p // extract the schema from the parameter sch := p.Schema.Schema() - // Render schema once for ReferenceSchema field in errors - var renderedSchema string - if sch != nil { - rendered, _ := sch.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedSchema = string(schemaBytes) - } + // Get rendered schema for ReferenceSchema field in errors (uses cache if available) + renderedSchema := GetRenderedSchema(sch, v.options) // check enum (if present) enumCheck := func(decodedValue string) { @@ -309,13 +303,8 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p if sch.Items != nil && sch.Items.IsA() { iSch := sch.Items.A.Schema() - // Render items schema once for ReferenceSchema field in array errors - var renderedItemsSchema string - if iSch != nil { - rendered, _ := iSch.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedItemsSchema = string(schemaBytes) - } + // Get rendered items schema for ReferenceSchema field in errors (uses cache if available) + renderedItemsSchema := GetRenderedSchema(iSch, v.options) for n := range iSch.Type { // determine how to explode the array diff --git a/parameters/query_parameters.go b/parameters/query_parameters.go index 40074d43..23f5a752 100644 --- a/parameters/query_parameters.go +++ b/parameters/query_parameters.go @@ -121,13 +121,8 @@ doneLooking: } } - // Render schema once for ReferenceSchema field in errors - var renderedSchema string - if sch != nil { - rendered, _ := sch.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedSchema = string(schemaBytes) - } + // Get rendered schema for ReferenceSchema field in errors (uses cache if available) + renderedSchema := GetRenderedSchema(sch, v.options) pType := sch.Type @@ -263,12 +258,8 @@ doneLooking: break } } - var renderedSchema string - if sch != nil { - rendered, _ := sch.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedSchema = string(schemaBytes) - } + // Get rendered schema for ReferenceSchema field in errors (uses cache if available) + renderedSchema := GetRenderedSchema(sch, v.options) validationErrors = append(validationErrors, errors.QueryParameterMissing(params[p], pathValue, operation, renderedSchema)) } } diff --git a/parameters/validate_parameter.go b/parameters/validate_parameter.go index ec32fa81..f2554af9 100644 --- a/parameters/validate_parameter.go +++ b/parameters/validate_parameter.go @@ -97,6 +97,28 @@ func buildJsonRender(schema *base.Schema) ([]byte, error) { return utils.ConvertYAMLtoJSON(renderedSchema) } +// GetRenderedSchema returns a JSON string representation of the schema for error messages. +// It first checks the schema cache for a pre-rendered version, falling back to fresh rendering. +// This avoids expensive re-rendering on each validation when the cache is available. +func GetRenderedSchema(schema *base.Schema, opts *config.ValidationOptions) string { + if schema == nil { + return "" + } + + // Try cache lookup first + if opts != nil && opts.SchemaCache != nil && schema.GoLow() != nil { + hash := schema.GoLow().Hash() + if cached, ok := opts.SchemaCache.Load(hash); ok && cached != nil && len(cached.RenderedJSON) > 0 { + return string(cached.RenderedJSON) + } + } + + // Cache miss - render fresh + rendered, _ := schema.RenderInline() + schemaBytes, _ := json.Marshal(rendered) + return string(schemaBytes) +} + // ValidateParameterSchema will validate a parameter against a raw object, or a blob of json/yaml. // It will return a list of validation errors, if any. // @@ -128,7 +150,6 @@ func ValidateParameterSchema( hash := schema.GoLow().Hash() if cached, ok := validationOptions.SchemaCache.Load(hash); ok && cached != nil && cached.CompiledSchema != nil { jsch = cached.CompiledSchema - jsonSchema = cached.RenderedJSON } } diff --git a/parameters/validate_parameter_test.go b/parameters/validate_parameter_test.go index 4e26ab8e..215f14a1 100644 --- a/parameters/validate_parameter_test.go +++ b/parameters/validate_parameter_test.go @@ -7,11 +7,10 @@ import ( "testing" "github.com/pb33f/libopenapi" + lowv3 "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - lowv3 "github.com/pb33f/libopenapi/datamodel/low/v3" - "github.com/pb33f/libopenapi-validator/cache" "github.com/pb33f/libopenapi-validator/config" "github.com/pb33f/libopenapi-validator/helpers" @@ -837,3 +836,283 @@ func Test_ParameterValidation_CacheConsistency(t *testing.T) { }) } } + +// Test_GetRenderedSchema_NilSchema verifies GetRenderedSchema handles nil schema gracefully. +func Test_GetRenderedSchema_NilSchema(t *testing.T) { + opts := config.NewValidationOptions() + result := GetRenderedSchema(nil, opts) + assert.Empty(t, result, "GetRenderedSchema should return empty string for nil schema") +} + +// Test_GetRenderedSchema_NilOptions verifies GetRenderedSchema works without options. +func Test_GetRenderedSchema_NilOptions(t *testing.T) { + // Parse a document to get a properly initialized schema + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "id", + "in": "query", + "schema": {"type": "string", "minLength": 1} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + // Get the parameter schema + pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") + param := pathItem.Get.Parameters[0] + schema := param.Schema.Schema() + + // Call with nil options - should still render the schema (returns some representation) + result := GetRenderedSchema(schema, nil) + assert.NotEmpty(t, result, "GetRenderedSchema should render schema even with nil options") +} + +// Test_GetRenderedSchema_CacheHit verifies GetRenderedSchema uses cached data when available. +func Test_GetRenderedSchema_CacheHit(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "id", + "in": "query", + "schema": {"type": "integer", "minimum": 1} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + // Get the parameter schema + pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") + param := pathItem.Get.Parameters[0] + schema := param.Schema.Schema() + + // Create options with cache and pre-populate with known value + opts := config.NewValidationOptions() + hash := schema.GoLow().Hash() + testCachedJSON := []byte(`{"type":"integer","minimum":1,"cached":true}`) + opts.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ + Schema: schema, + RenderedJSON: testCachedJSON, + }) + + // GetRenderedSchema should return the cached value + result := GetRenderedSchema(schema, opts) + assert.Equal(t, string(testCachedJSON), result, "GetRenderedSchema should return cached JSON") +} + +// Test_GetRenderedSchema_NilCache verifies GetRenderedSchema works when cache is disabled. +func Test_GetRenderedSchema_NilCache(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "id", + "in": "query", + "schema": {"type": "boolean"} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + // Get the parameter schema + pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") + param := pathItem.Get.Parameters[0] + schema := param.Schema.Schema() + + // Create options with cache disabled + opts := config.NewValidationOptions(config.WithSchemaCache(nil)) + require.Nil(t, opts.SchemaCache) + + // GetRenderedSchema should still work by rendering fresh (returns some representation) + result := GetRenderedSchema(schema, opts) + assert.NotEmpty(t, result, "GetRenderedSchema should render schema even with nil cache") +} + +// Test_GetRenderedSchema_CacheMiss verifies GetRenderedSchema renders fresh when cache entry has empty RenderedJSON. +// This tests the code path where cache lookup succeeds but RenderedJSON is empty. +func Test_GetRenderedSchema_CacheMiss(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "id", + "in": "query", + "schema": {"type": "integer"} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + // Get the parameter schema + pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") + param := pathItem.Get.Parameters[0] + schema := param.Schema.Schema() + + // Create options with cache enabled + opts := config.NewValidationOptions() + require.NotNil(t, opts.SchemaCache) + + // Store an entry with empty RenderedJSON to simulate cache miss scenario + hash := schema.GoLow().Hash() + opts.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ + Schema: schema, + RenderedJSON: nil, // Empty - should trigger fresh rendering + }) + + // GetRenderedSchema should render fresh since RenderedJSON is empty + result := GetRenderedSchema(schema, opts) + assert.NotEmpty(t, result, "GetRenderedSchema should render fresh when cached RenderedJSON is empty") +} + +// Test_ValidateSingleParameterSchema_CacheMissCompiledSchema tests the path where cache entry +// exists but CompiledSchema is nil, forcing recompilation. +func Test_ValidateSingleParameterSchema_CacheMissCompiledSchema(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test/{id}": { + "get": { + "parameters": [{ + "name": "id", + "in": "path", + "required": true, + "schema": {"type": "integer", "minimum": 1} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + // Get the parameter schema + pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test/{id}") + param := pathItem.Get.Parameters[0] + schema := param.Schema.Schema() + + // Create options with cache enabled + opts := config.NewValidationOptions() + require.NotNil(t, opts.SchemaCache) + + // Store an entry with nil CompiledSchema to force recompilation + hash := schema.GoLow().Hash() + opts.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ + Schema: schema, + CompiledSchema: nil, // nil - should trigger recompilation + }) + + // Validate should still work by recompiling the schema + result := ValidateSingleParameterSchema( + schema, + int64(5), // valid integer + "Path parameter", + "The path parameter", + "id", + helpers.ParameterValidation, + helpers.ParameterValidationPath, + opts, + "/test/{id}", + "get", + ) + assert.Empty(t, result, "Validation should pass for valid integer") + + // Now verify the cache was populated with the compiled schema + cached, ok := opts.SchemaCache.Load(hash) + assert.True(t, ok, "Cache entry should exist") + assert.NotNil(t, cached.CompiledSchema, "CompiledSchema should be populated after validation") +} + +// arrayValidationSpec is used to test array parameter validation with the updated function signatures +var arrayValidationSpec = []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Array Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "ids", + "in": "query", + "schema": { + "type": "array", + "items": {"type": "integer", "minimum": 1} + } + }], + "responses": {"200": {"description": "OK"}} + } + } + } +}`) + +// Test_ArrayValidation_ErrorContainsRenderedSchema verifies that array validation errors +// still contain the rendered schema after the rendering optimization. +func Test_ArrayValidation_ErrorContainsRenderedSchema(t *testing.T) { + doc, err := libopenapi.NewDocument(arrayValidationSpec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + validator := NewParameterValidator(&v3Model.Model) + + // Request with invalid array values (strings instead of integers) + req, _ := http.NewRequest("GET", "/test?ids=abc,def", nil) + + success, validationErrors := validator.ValidateQueryParams(req) + assert.False(t, success, "Validation should fail for non-integer array values") + assert.NotEmpty(t, validationErrors, "Should have validation errors") + + // Verify error message is properly formatted + assert.Contains(t, validationErrors[0].Message, "ids", "Error should reference parameter name") +} diff --git a/parameters/validation_functions.go b/parameters/validation_functions.go index f931588f..ff4749eb 100644 --- a/parameters/validation_functions.go +++ b/parameters/validation_functions.go @@ -119,12 +119,8 @@ func ValidateQueryArray( var validationErrors []*errors.ValidationError itemsSchema := sch.Items.A.Schema() - var renderedItemsSchema string - if itemsSchema != nil { - rendered, _ := itemsSchema.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - renderedItemsSchema = string(schemaBytes) - } + // Get rendered items schema for ReferenceSchema field in errors (uses cache if available) + renderedItemsSchema := GetRenderedSchema(itemsSchema, validationOptions) // check for an exploded bit on the schema. // if it's exploded, then we need to check each item in the array From 7271dd30d1bb6f385049ca2d528669dd6bbd274c Mon Sep 17 00:00:00 2001 From: Vincent Marceau Date: Wed, 15 Apr 2026 12:03:19 -0400 Subject: [PATCH 3/4] fix: parameter validation populates complete cache entry --- parameters/validate_parameter.go | 23 +++++++++++-- parameters/validate_parameter_test.go | 49 +++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/parameters/validate_parameter.go b/parameters/validate_parameter.go index f2554af9..f5521322 100644 --- a/parameters/validate_parameter.go +++ b/parameters/validate_parameter.go @@ -13,6 +13,7 @@ import ( "github.com/pb33f/libopenapi/datamodel/high/base" "github.com/pb33f/libopenapi/utils" "github.com/santhosh-tekuri/jsonschema/v6" + "go.yaml.in/yaml/v4" "golang.org/x/text/language" "golang.org/x/text/message" @@ -65,10 +66,21 @@ func ValidateSingleParameterSchema( // Store in cache for future requests if o != nil && o.SchemaCache != nil && schema != nil && schema.GoLow() != nil { hash := schema.GoLow().Hash() + + renderCtx := base.NewInlineRenderContextForValidation() + renderedInline, _ := schema.RenderInlineWithContext(renderCtx) + referenceSchema := string(renderedInline) + + var renderedNode yaml.Node + _ = yaml.Unmarshal(renderedInline, &renderedNode) + o.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ - Schema: schema, - RenderedJSON: jsonSchema, - CompiledSchema: jsch, + Schema: schema, + RenderedInline: renderedInline, + ReferenceSchema: referenceSchema, + RenderedJSON: jsonSchema, + CompiledSchema: jsch, + RenderedNode: &renderedNode, }) } } @@ -184,12 +196,17 @@ func ValidateParameterSchema( // Store in cache for future requests if validationOptions != nil && validationOptions.SchemaCache != nil && schema != nil && schema.GoLow() != nil { hash := schema.GoLow().Hash() + + var renderedNode yaml.Node + _ = yaml.Unmarshal(renderedSchema, &renderedNode) + validationOptions.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ Schema: schema, RenderedInline: renderedSchema, ReferenceSchema: referenceSchema, RenderedJSON: jsonSchema, CompiledSchema: jsch, + RenderedNode: &renderedNode, }) } } diff --git a/parameters/validate_parameter_test.go b/parameters/validate_parameter_test.go index 215f14a1..964f7822 100644 --- a/parameters/validate_parameter_test.go +++ b/parameters/validate_parameter_test.go @@ -1116,3 +1116,52 @@ func Test_ArrayValidation_ErrorContainsRenderedSchema(t *testing.T) { // Verify error message is properly formatted assert.Contains(t, validationErrors[0].Message, "ids", "Error should reference parameter name") } + +// Test_ParameterValidation_CompleteCacheEntry verifies that parameter validation +// writes complete cache entries. +func Test_ParameterValidation_CompleteCacheEntry(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "id", + "in": "query", + "schema": {"type": "string", "minLength": 1} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + opts := config.NewValidationOptions() + validator := NewParameterValidator(&v3Model.Model, config.WithExistingOpts(opts)) + + req, _ := http.NewRequest("GET", "/test?id=abc", nil) + valid, _ := validator.ValidateQueryParams(req) + assert.True(t, valid) + + pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") + schema := pathItem.Get.Parameters[0].Schema.Schema() + hash := schema.GoLow().Hash() + + cached, ok := opts.SchemaCache.Load(hash) + require.True(t, ok, "Cache entry should exist") + + // Check that all fields of the cache entry are populated + assert.NotNil(t, cached.Schema, "Schema should be populated") + assert.NotEmpty(t, cached.RenderedInline, "RenderedInline should be populated") + assert.NotEmpty(t, cached.ReferenceSchema, "ReferenceSchema should be populated") + assert.NotEmpty(t, cached.RenderedJSON, "RenderedJSON should be populated") + assert.NotNil(t, cached.CompiledSchema, "CompiledSchema should be populated") + assert.NotNil(t, cached.RenderedNode, "RenderedNode should be populated") +} From 97ddd743c1d08149cfdc9bcf22b42f1d120eef3d Mon Sep 17 00:00:00 2001 From: Vincent Marceau Date: Wed, 15 Apr 2026 12:35:27 -0400 Subject: [PATCH 4/4] fix: GetRenderedSchema returns YAML in all cases --- parameters/validate_parameter.go | 14 +-- parameters/validate_parameter_test.go | 168 ++++++++++++++++++++------ 2 files changed, 138 insertions(+), 44 deletions(-) diff --git a/parameters/validate_parameter.go b/parameters/validate_parameter.go index f5521322..51f1d538 100644 --- a/parameters/validate_parameter.go +++ b/parameters/validate_parameter.go @@ -109,7 +109,7 @@ func buildJsonRender(schema *base.Schema) ([]byte, error) { return utils.ConvertYAMLtoJSON(renderedSchema) } -// GetRenderedSchema returns a JSON string representation of the schema for error messages. +// GetRenderedSchema returns a YAML string representation of the schema for error messages. // It first checks the schema cache for a pre-rendered version, falling back to fresh rendering. // This avoids expensive re-rendering on each validation when the cache is available. func GetRenderedSchema(schema *base.Schema, opts *config.ValidationOptions) string { @@ -120,15 +120,14 @@ func GetRenderedSchema(schema *base.Schema, opts *config.ValidationOptions) stri // Try cache lookup first if opts != nil && opts.SchemaCache != nil && schema.GoLow() != nil { hash := schema.GoLow().Hash() - if cached, ok := opts.SchemaCache.Load(hash); ok && cached != nil && len(cached.RenderedJSON) > 0 { - return string(cached.RenderedJSON) + if cached, ok := opts.SchemaCache.Load(hash); ok && cached != nil && len(cached.RenderedInline) > 0 { + return string(cached.RenderedInline) } } - // Cache miss - render fresh + // Cache miss - render fresh as YAML rendered, _ := schema.RenderInline() - schemaBytes, _ := json.Marshal(rendered) - return string(schemaBytes) + return string(rendered) } // ValidateParameterSchema will validate a parameter against a raw object, or a blob of json/yaml. @@ -334,8 +333,7 @@ func formatJsonSchemaValidationError(schema *base.Schema, scErrs *jsonschema.Val renderCtx := base.NewInlineRenderContextForValidation() rendered, err := schema.RenderInlineWithContext(renderCtx) if err == nil && rendered != nil { - renderedBytes, _ := json.Marshal(rendered) - fail.ReferenceSchema = string(renderedBytes) + fail.ReferenceSchema = string(rendered) } } schemaValidationErrors = append(schemaValidationErrors, fail) diff --git a/parameters/validate_parameter_test.go b/parameters/validate_parameter_test.go index 964f7822..edaed8a2 100644 --- a/parameters/validate_parameter_test.go +++ b/parameters/validate_parameter_test.go @@ -846,7 +846,6 @@ func Test_GetRenderedSchema_NilSchema(t *testing.T) { // Test_GetRenderedSchema_NilOptions verifies GetRenderedSchema works without options. func Test_GetRenderedSchema_NilOptions(t *testing.T) { - // Parse a document to get a properly initialized schema spec := []byte(`{ "openapi": "3.1.0", "info": {"title": "Test", "version": "1.0.0"}, @@ -870,14 +869,16 @@ func Test_GetRenderedSchema_NilOptions(t *testing.T) { v3Model, errs := doc.BuildV3Model() require.Nil(t, errs) - // Get the parameter schema pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") - param := pathItem.Get.Parameters[0] - schema := param.Schema.Schema() + schema := pathItem.Get.Parameters[0].Schema.Schema() + + // Ground truth + rendered, _ := schema.RenderInline() + expected := string(rendered) - // Call with nil options - should still render the schema (returns some representation) + // With nil options should match ground truth result := GetRenderedSchema(schema, nil) - assert.NotEmpty(t, result, "GetRenderedSchema should render schema even with nil options") + assert.Equal(t, expected, result) } // Test_GetRenderedSchema_CacheHit verifies GetRenderedSchema uses cached data when available. @@ -905,23 +906,24 @@ func Test_GetRenderedSchema_CacheHit(t *testing.T) { v3Model, errs := doc.BuildV3Model() require.Nil(t, errs) - // Get the parameter schema pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") - param := pathItem.Get.Parameters[0] - schema := param.Schema.Schema() + schema := pathItem.Get.Parameters[0].Schema.Schema() - // Create options with cache and pre-populate with known value + // Ground truth + rendered, _ := schema.RenderInline() + expected := string(rendered) + + // Pre-populate cache with RenderedInline opts := config.NewValidationOptions() hash := schema.GoLow().Hash() - testCachedJSON := []byte(`{"type":"integer","minimum":1,"cached":true}`) opts.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ - Schema: schema, - RenderedJSON: testCachedJSON, + Schema: schema, + RenderedInline: rendered, }) - // GetRenderedSchema should return the cached value + // Cache hit should match ground truth result := GetRenderedSchema(schema, opts) - assert.Equal(t, string(testCachedJSON), result, "GetRenderedSchema should return cached JSON") + assert.Equal(t, expected, result) } // Test_GetRenderedSchema_NilCache verifies GetRenderedSchema works when cache is disabled. @@ -949,22 +951,21 @@ func Test_GetRenderedSchema_NilCache(t *testing.T) { v3Model, errs := doc.BuildV3Model() require.Nil(t, errs) - // Get the parameter schema pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") - param := pathItem.Get.Parameters[0] - schema := param.Schema.Schema() + schema := pathItem.Get.Parameters[0].Schema.Schema() - // Create options with cache disabled - opts := config.NewValidationOptions(config.WithSchemaCache(nil)) - require.Nil(t, opts.SchemaCache) + // Ground truth + rendered, _ := schema.RenderInline() + expected := string(rendered) - // GetRenderedSchema should still work by rendering fresh (returns some representation) + // With nil cache should match ground truth + opts := config.NewValidationOptions(config.WithSchemaCache(nil)) result := GetRenderedSchema(schema, opts) - assert.NotEmpty(t, result, "GetRenderedSchema should render schema even with nil cache") + assert.Equal(t, expected, result) } -// Test_GetRenderedSchema_CacheMiss verifies GetRenderedSchema renders fresh when cache entry has empty RenderedJSON. -// This tests the code path where cache lookup succeeds but RenderedJSON is empty. +// Test_GetRenderedSchema_CacheMiss verifies GetRenderedSchema renders fresh when cache entry has empty RenderedInline. +// This tests the code path where cache lookup succeeds but RenderedInline is empty. func Test_GetRenderedSchema_CacheMiss(t *testing.T) { spec := []byte(`{ "openapi": "3.1.0", @@ -989,25 +990,72 @@ func Test_GetRenderedSchema_CacheMiss(t *testing.T) { v3Model, errs := doc.BuildV3Model() require.Nil(t, errs) - // Get the parameter schema pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") - param := pathItem.Get.Parameters[0] - schema := param.Schema.Schema() + schema := pathItem.Get.Parameters[0].Schema.Schema() - // Create options with cache enabled - opts := config.NewValidationOptions() - require.NotNil(t, opts.SchemaCache) + // Ground truth + rendered, _ := schema.RenderInline() + expected := string(rendered) - // Store an entry with empty RenderedJSON to simulate cache miss scenario + // Store entry with empty RenderedInline to force cache miss + opts := config.NewValidationOptions() hash := schema.GoLow().Hash() opts.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ - Schema: schema, - RenderedJSON: nil, // Empty - should trigger fresh rendering + Schema: schema, + RenderedInline: nil, // Empty - should trigger fresh rendering }) - // GetRenderedSchema should render fresh since RenderedJSON is empty + // Cache miss should still match ground truth result := GetRenderedSchema(schema, opts) - assert.NotEmpty(t, result, "GetRenderedSchema should render fresh when cached RenderedJSON is empty") + assert.Equal(t, expected, result) +} + +// Test_GetRenderedSchema_Deterministic verifies that GetRenderedSchema returns the same +// output regardless of cache state (cache hit vs cache miss). +func Test_GetRenderedSchema_Deterministic(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "status", + "in": "query", + "schema": {"type": "string", "enum": ["active", "inactive"]} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test") + schema := pathItem.Get.Parameters[0].Schema.Schema() + + // Ground truth + rendered, _ := schema.RenderInline() + expected := string(rendered) + + // Cache miss path (no cache) + optsNoCache := config.NewValidationOptions(config.WithSchemaCache(nil)) + resultMiss := GetRenderedSchema(schema, optsNoCache) + assert.Equal(t, expected, resultMiss) + + // Cache hit path (pre-populated cache) + optsWithCache := config.NewValidationOptions() + hash := schema.GoLow().Hash() + optsWithCache.SchemaCache.Store(hash, &cache.SchemaCacheEntry{ + RenderedInline: rendered, + }) + resultHit := GetRenderedSchema(schema, optsWithCache) + assert.Equal(t, expected, resultHit) } // Test_ValidateSingleParameterSchema_CacheMissCompiledSchema tests the path where cache entry @@ -1165,3 +1213,51 @@ func Test_ParameterValidation_CompleteCacheEntry(t *testing.T) { assert.NotNil(t, cached.CompiledSchema, "CompiledSchema should be populated") assert.NotNil(t, cached.RenderedNode, "RenderedNode should be populated") } + +// Test_ReferenceSchema_ConsistentFormat verifies that ReferenceSchema has the same +// format whether the error comes from GetRenderedSchema or formatJsonSchemaValidationError. +func Test_ReferenceSchema_ConsistentFormat(t *testing.T) { + spec := []byte(`{ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0.0"}, + "paths": { + "/test": { + "get": { + "parameters": [{ + "name": "count", + "in": "query", + "required": true, + "schema": {"type": "integer", "minimum": 1, "maximum": 100} + }], + "responses": {"200": {"description": "OK"}} + } + } + } + }`) + + doc, err := libopenapi.NewDocument(spec) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.Nil(t, errs) + + validator := NewParameterValidator(&v3Model.Model) + + // Error path 1: Missing required parameter (uses GetRenderedSchema) + req1, _ := http.NewRequest(http.MethodGet, "/test", nil) + _, errors1 := validator.ValidateQueryParams(req1) + require.NotEmpty(t, errors1) + require.NotEmpty(t, errors1[0].SchemaValidationErrors) + refSchema1 := errors1[0].SchemaValidationErrors[0].ReferenceSchema + + // Error path 2: Value outside range (uses formatJsonSchemaValidationError) + req2, _ := http.NewRequest(http.MethodGet, "/test?count=999", nil) + _, errors2 := validator.ValidateQueryParams(req2) + require.NotEmpty(t, errors2) + require.NotEmpty(t, errors2[0].SchemaValidationErrors) + refSchema2 := errors2[0].SchemaValidationErrors[0].ReferenceSchema + + // Both should be plain YAML with the same content + assert.Equal(t, refSchema1, refSchema2, + "ReferenceSchema should be consistent regardless of error path") +}