Skip to content

Commit 70454de

Browse files
vmarceauvmarceau
committed
fix: GetRenderedSchema returns YAML in all cases
1 parent 7271dd3 commit 70454de

2 files changed

Lines changed: 89 additions & 42 deletions

File tree

‎parameters/validate_parameter.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func buildJsonRender(schema *base.Schema) ([]byte, error) {
109109
return utils.ConvertYAMLtoJSON(renderedSchema)
110110
}
111111

112-
// GetRenderedSchema returns a JSON string representation of the schema for error messages.
112+
// GetRenderedSchema returns a YAML string representation of the schema for error messages.
113113
// It first checks the schema cache for a pre-rendered version, falling back to fresh rendering.
114114
// This avoids expensive re-rendering on each validation when the cache is available.
115115
func GetRenderedSchema(schema *base.Schema, opts *config.ValidationOptions) string {
@@ -120,15 +120,14 @@ func GetRenderedSchema(schema *base.Schema, opts *config.ValidationOptions) stri
120120
// Try cache lookup first
121121
if opts != nil && opts.SchemaCache != nil && schema.GoLow() != nil {
122122
hash := schema.GoLow().Hash()
123-
if cached, ok := opts.SchemaCache.Load(hash); ok && cached != nil && len(cached.RenderedJSON) > 0 {
124-
return string(cached.RenderedJSON)
123+
if cached, ok := opts.SchemaCache.Load(hash); ok && cached != nil && len(cached.RenderedInline) > 0 {
124+
return string(cached.RenderedInline)
125125
}
126126
}
127127

128-
// Cache miss - render fresh
128+
// Cache miss - render fresh as YAML
129129
rendered, _ := schema.RenderInline()
130-
schemaBytes, _ := json.Marshal(rendered)
131-
return string(schemaBytes)
130+
return string(rendered)
132131
}
133132

134133
// ValidateParameterSchema will validate a parameter against a raw object, or a blob of json/yaml.

‎parameters/validate_parameter_test.go‎

Lines changed: 84 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,6 @@ func Test_GetRenderedSchema_NilSchema(t *testing.T) {
846846

847847
// Test_GetRenderedSchema_NilOptions verifies GetRenderedSchema works without options.
848848
func Test_GetRenderedSchema_NilOptions(t *testing.T) {
849-
// Parse a document to get a properly initialized schema
850849
spec := []byte(`{
851850
"openapi": "3.1.0",
852851
"info": {"title": "Test", "version": "1.0.0"},
@@ -870,14 +869,16 @@ func Test_GetRenderedSchema_NilOptions(t *testing.T) {
870869
v3Model, errs := doc.BuildV3Model()
871870
require.Nil(t, errs)
872871

873-
// Get the parameter schema
874872
pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test")
875-
param := pathItem.Get.Parameters[0]
876-
schema := param.Schema.Schema()
873+
schema := pathItem.Get.Parameters[0].Schema.Schema()
877874

878-
// Call with nil options - should still render the schema (returns some representation)
875+
// Ground truth
876+
rendered, _ := schema.RenderInline()
877+
expected := string(rendered)
878+
879+
// With nil options should match ground truth
879880
result := GetRenderedSchema(schema, nil)
880-
assert.NotEmpty(t, result, "GetRenderedSchema should render schema even with nil options")
881+
assert.Equal(t, expected, result)
881882
}
882883

883884
// Test_GetRenderedSchema_CacheHit verifies GetRenderedSchema uses cached data when available.
@@ -905,23 +906,24 @@ func Test_GetRenderedSchema_CacheHit(t *testing.T) {
905906
v3Model, errs := doc.BuildV3Model()
906907
require.Nil(t, errs)
907908

908-
// Get the parameter schema
909909
pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test")
910-
param := pathItem.Get.Parameters[0]
911-
schema := param.Schema.Schema()
910+
schema := pathItem.Get.Parameters[0].Schema.Schema()
911+
912+
// Ground truth
913+
rendered, _ := schema.RenderInline()
914+
expected := string(rendered)
912915

913-
// Create options with cache and pre-populate with known value
916+
// Pre-populate cache with RenderedInline
914917
opts := config.NewValidationOptions()
915918
hash := schema.GoLow().Hash()
916-
testCachedJSON := []byte(`{"type":"integer","minimum":1,"cached":true}`)
917919
opts.SchemaCache.Store(hash, &cache.SchemaCacheEntry{
918-
Schema: schema,
919-
RenderedJSON: testCachedJSON,
920+
Schema: schema,
921+
RenderedInline: rendered,
920922
})
921923

922-
// GetRenderedSchema should return the cached value
924+
// Cache hit should match ground truth
923925
result := GetRenderedSchema(schema, opts)
924-
assert.Equal(t, string(testCachedJSON), result, "GetRenderedSchema should return cached JSON")
926+
assert.Equal(t, expected, result)
925927
}
926928

927929
// Test_GetRenderedSchema_NilCache verifies GetRenderedSchema works when cache is disabled.
@@ -949,22 +951,21 @@ func Test_GetRenderedSchema_NilCache(t *testing.T) {
949951
v3Model, errs := doc.BuildV3Model()
950952
require.Nil(t, errs)
951953

952-
// Get the parameter schema
953954
pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test")
954-
param := pathItem.Get.Parameters[0]
955-
schema := param.Schema.Schema()
955+
schema := pathItem.Get.Parameters[0].Schema.Schema()
956956

957-
// Create options with cache disabled
958-
opts := config.NewValidationOptions(config.WithSchemaCache(nil))
959-
require.Nil(t, opts.SchemaCache)
957+
// Ground truth
958+
rendered, _ := schema.RenderInline()
959+
expected := string(rendered)
960960

961-
// GetRenderedSchema should still work by rendering fresh (returns some representation)
961+
// With nil cache should match ground truth
962+
opts := config.NewValidationOptions(config.WithSchemaCache(nil))
962963
result := GetRenderedSchema(schema, opts)
963-
assert.NotEmpty(t, result, "GetRenderedSchema should render schema even with nil cache")
964+
assert.Equal(t, expected, result)
964965
}
965966

966-
// Test_GetRenderedSchema_CacheMiss verifies GetRenderedSchema renders fresh when cache entry has empty RenderedJSON.
967-
// This tests the code path where cache lookup succeeds but RenderedJSON is empty.
967+
// Test_GetRenderedSchema_CacheMiss verifies GetRenderedSchema renders fresh when cache entry has empty RenderedInline.
968+
// This tests the code path where cache lookup succeeds but RenderedInline is empty.
968969
func Test_GetRenderedSchema_CacheMiss(t *testing.T) {
969970
spec := []byte(`{
970971
"openapi": "3.1.0",
@@ -989,25 +990,72 @@ func Test_GetRenderedSchema_CacheMiss(t *testing.T) {
989990
v3Model, errs := doc.BuildV3Model()
990991
require.Nil(t, errs)
991992

992-
// Get the parameter schema
993993
pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test")
994-
param := pathItem.Get.Parameters[0]
995-
schema := param.Schema.Schema()
994+
schema := pathItem.Get.Parameters[0].Schema.Schema()
996995

997-
// Create options with cache enabled
998-
opts := config.NewValidationOptions()
999-
require.NotNil(t, opts.SchemaCache)
996+
// Ground truth
997+
rendered, _ := schema.RenderInline()
998+
expected := string(rendered)
1000999

1001-
// Store an entry with empty RenderedJSON to simulate cache miss scenario
1000+
// Store entry with empty RenderedInline to force cache miss
1001+
opts := config.NewValidationOptions()
10021002
hash := schema.GoLow().Hash()
10031003
opts.SchemaCache.Store(hash, &cache.SchemaCacheEntry{
1004-
Schema: schema,
1005-
RenderedJSON: nil, // Empty - should trigger fresh rendering
1004+
Schema: schema,
1005+
RenderedInline: nil, // Empty - should trigger fresh rendering
10061006
})
10071007

1008-
// GetRenderedSchema should render fresh since RenderedJSON is empty
1008+
// Cache miss should still match ground truth
10091009
result := GetRenderedSchema(schema, opts)
1010-
assert.NotEmpty(t, result, "GetRenderedSchema should render fresh when cached RenderedJSON is empty")
1010+
assert.Equal(t, expected, result)
1011+
}
1012+
1013+
// Test_GetRenderedSchema_Deterministic verifies that GetRenderedSchema returns the same
1014+
// output regardless of cache state (cache hit vs cache miss).
1015+
func Test_GetRenderedSchema_Deterministic(t *testing.T) {
1016+
spec := []byte(`{
1017+
"openapi": "3.1.0",
1018+
"info": {"title": "Test", "version": "1.0.0"},
1019+
"paths": {
1020+
"/test": {
1021+
"get": {
1022+
"parameters": [{
1023+
"name": "status",
1024+
"in": "query",
1025+
"schema": {"type": "string", "enum": ["active", "inactive"]}
1026+
}],
1027+
"responses": {"200": {"description": "OK"}}
1028+
}
1029+
}
1030+
}
1031+
}`)
1032+
1033+
doc, err := libopenapi.NewDocument(spec)
1034+
require.NoError(t, err)
1035+
1036+
v3Model, errs := doc.BuildV3Model()
1037+
require.Nil(t, errs)
1038+
1039+
pathItem := v3Model.Model.Paths.PathItems.GetOrZero("/test")
1040+
schema := pathItem.Get.Parameters[0].Schema.Schema()
1041+
1042+
// Ground truth
1043+
rendered, _ := schema.RenderInline()
1044+
expected := string(rendered)
1045+
1046+
// Cache miss path (no cache)
1047+
optsNoCache := config.NewValidationOptions(config.WithSchemaCache(nil))
1048+
resultMiss := GetRenderedSchema(schema, optsNoCache)
1049+
assert.Equal(t, expected, resultMiss)
1050+
1051+
// Cache hit path (pre-populated cache)
1052+
optsWithCache := config.NewValidationOptions()
1053+
hash := schema.GoLow().Hash()
1054+
optsWithCache.SchemaCache.Store(hash, &cache.SchemaCacheEntry{
1055+
RenderedInline: rendered,
1056+
})
1057+
resultHit := GetRenderedSchema(schema, optsWithCache)
1058+
assert.Equal(t, expected, resultHit)
10111059
}
10121060

10131061
// Test_ValidateSingleParameterSchema_CacheMissCompiledSchema tests the path where cache entry

0 commit comments

Comments
 (0)