From 8531113e17f3bbac9da60494c32ad72c7e26bd78 Mon Sep 17 00:00:00 2001 From: Tristan Cartledge Date: Tue, 3 Oct 2023 14:33:44 +0000 Subject: [PATCH] fix!: fixed handling of additionalProperties to handle the bool/json-schema nature better --- datamodel/high/base/schema.go | 118 ++++++++------- datamodel/high/base/schema_test.go | 129 ++++++++--------- datamodel/high/v3/document_test.go | 16 +-- datamodel/low/base/schema.go | 175 ++++++----------------- datamodel/low/base/schema_test.go | 37 +---- datamodel/low/v3/create_document_test.go | 27 +--- what-changed/model/schema.go | 90 +++++++----- what-changed/model/schema_test.go | 106 +++++++++++--- 8 files changed, 316 insertions(+), 382 deletions(-) diff --git a/datamodel/high/base/schema.go b/datamodel/high/base/schema.go index e10e04c..d421ca3 100644 --- a/datamodel/high/base/schema.go +++ b/datamodel/high/base/schema.go @@ -63,7 +63,7 @@ type Schema struct { // in 3.1 UnevaluatedProperties can be a Schema or a boolean // https://github.com/pb33f/libopenapi/issues/118 - UnevaluatedProperties *DynamicValue[*SchemaProxy, *bool] `json:"unevaluatedProperties,omitempty" yaml:"unevaluatedProperties,omitempty"` + UnevaluatedProperties *DynamicValue[*SchemaProxy, bool] `json:"unevaluatedProperties,omitempty" yaml:"unevaluatedProperties,omitempty"` // in 3.1 Items can be a Schema or a boolean Items *DynamicValue[*SchemaProxy, bool] `json:"items,omitempty" yaml:"items,omitempty"` @@ -72,35 +72,35 @@ type Schema struct { Anchor string `json:"$anchor,omitempty" yaml:"$anchor,omitempty"` // Compatible with all versions - Not *SchemaProxy `json:"not,omitempty" yaml:"not,omitempty"` - Properties map[string]*SchemaProxy `json:"properties,omitempty" yaml:"properties,omitempty"` - Title string `json:"title,omitempty" yaml:"title,omitempty"` - MultipleOf *float64 `json:"multipleOf,omitempty" yaml:"multipleOf,omitempty"` - Maximum *float64 `json:"maximum,omitempty" yaml:"maximum,omitempty"` - Minimum *float64 `json:"minimum,omitempty" yaml:"minimum,omitempty"` - MaxLength *int64 `json:"maxLength,omitempty" yaml:"maxLength,omitempty"` - MinLength *int64 `json:"minLength,omitempty" yaml:"minLength,omitempty"` - Pattern string `json:"pattern,omitempty" yaml:"pattern,omitempty"` - Format string `json:"format,omitempty" yaml:"format,omitempty"` - MaxItems *int64 `json:"maxItems,omitempty" yaml:"maxItems,omitempty"` - MinItems *int64 `json:"minItems,omitempty" yaml:"minItems,omitempty"` - UniqueItems *bool `json:"uniqueItems,omitempty" yaml:"uniqueItems,omitempty"` - MaxProperties *int64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` - MinProperties *int64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` - Required []string `json:"required,omitempty" yaml:"required,omitempty"` - Enum []any `json:"enum,omitempty" yaml:"enum,omitempty"` - AdditionalProperties any `json:"additionalProperties,omitempty" yaml:"additionalProperties,renderZero,omitempty"` - Description string `json:"description,omitempty" yaml:"description,omitempty"` - Default any `json:"default,omitempty" yaml:"default,renderZero,omitempty"` - Const any `json:"const,omitempty" yaml:"const,renderZero,omitempty"` - Nullable *bool `json:"nullable,omitempty" yaml:"nullable,omitempty"` - ReadOnly bool `json:"readOnly,omitempty" yaml:"readOnly,omitempty"` // https://github.com/pb33f/libopenapi/issues/30 - WriteOnly bool `json:"writeOnly,omitempty" yaml:"writeOnly,omitempty"` // https://github.com/pb33f/libopenapi/issues/30 - XML *XML `json:"xml,omitempty" yaml:"xml,omitempty"` - ExternalDocs *ExternalDoc `json:"externalDocs,omitempty" yaml:"externalDocs,omitempty"` - Example any `json:"example,omitempty" yaml:"example,omitempty"` - Deprecated *bool `json:"deprecated,omitempty" yaml:"deprecated,omitempty"` - Extensions map[string]any `json:"-" yaml:"-"` + Not *SchemaProxy `json:"not,omitempty" yaml:"not,omitempty"` + Properties map[string]*SchemaProxy `json:"properties,omitempty" yaml:"properties,omitempty"` + Title string `json:"title,omitempty" yaml:"title,omitempty"` + MultipleOf *float64 `json:"multipleOf,omitempty" yaml:"multipleOf,omitempty"` + Maximum *float64 `json:"maximum,omitempty" yaml:"maximum,omitempty"` + Minimum *float64 `json:"minimum,omitempty" yaml:"minimum,omitempty"` + MaxLength *int64 `json:"maxLength,omitempty" yaml:"maxLength,omitempty"` + MinLength *int64 `json:"minLength,omitempty" yaml:"minLength,omitempty"` + Pattern string `json:"pattern,omitempty" yaml:"pattern,omitempty"` + Format string `json:"format,omitempty" yaml:"format,omitempty"` + MaxItems *int64 `json:"maxItems,omitempty" yaml:"maxItems,omitempty"` + MinItems *int64 `json:"minItems,omitempty" yaml:"minItems,omitempty"` + UniqueItems *bool `json:"uniqueItems,omitempty" yaml:"uniqueItems,omitempty"` + MaxProperties *int64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` + MinProperties *int64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` + Required []string `json:"required,omitempty" yaml:"required,omitempty"` + Enum []any `json:"enum,omitempty" yaml:"enum,omitempty"` + AdditionalProperties *DynamicValue[*SchemaProxy, bool] `json:"additionalProperties,omitempty" yaml:"additionalProperties,renderZero,omitempty"` + Description string `json:"description,omitempty" yaml:"description,omitempty"` + Default any `json:"default,omitempty" yaml:"default,renderZero,omitempty"` + Const any `json:"const,omitempty" yaml:"const,renderZero,omitempty"` + Nullable *bool `json:"nullable,omitempty" yaml:"nullable,omitempty"` + ReadOnly bool `json:"readOnly,omitempty" yaml:"readOnly,omitempty"` // https://github.com/pb33f/libopenapi/issues/30 + WriteOnly bool `json:"writeOnly,omitempty" yaml:"writeOnly,omitempty"` // https://github.com/pb33f/libopenapi/issues/30 + XML *XML `json:"xml,omitempty" yaml:"xml,omitempty"` + ExternalDocs *ExternalDoc `json:"externalDocs,omitempty" yaml:"externalDocs,omitempty"` + Example any `json:"example,omitempty" yaml:"example,omitempty"` + Deprecated *bool `json:"deprecated,omitempty" yaml:"deprecated,omitempty"` + Extensions map[string]any `json:"-" yaml:"-"` low *base.Schema // Parent Proxy refers back to the low level SchemaProxy that is proxying this schema. @@ -211,29 +211,22 @@ func NewSchema(schema *base.Schema) *Schema { Value: schema.UnevaluatedItems.Value, }) } - // check if unevaluated properties is a schema - if !schema.UnevaluatedProperties.IsEmpty() && schema.UnevaluatedProperties.Value.IsA() { - s.UnevaluatedProperties = &DynamicValue[*SchemaProxy, *bool]{ - A: NewSchemaProxy( - &lowmodel.NodeReference[*base.SchemaProxy]{ + + var unevaluatedProperties *DynamicValue[*SchemaProxy, bool] + if !schema.UnevaluatedProperties.IsEmpty() { + if schema.UnevaluatedProperties.Value.IsA() { + unevaluatedProperties = &DynamicValue[*SchemaProxy, bool]{ + A: NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{ ValueNode: schema.UnevaluatedProperties.ValueNode, Value: schema.UnevaluatedProperties.Value.A, - }, - ), - N: 0, + KeyNode: schema.UnevaluatedProperties.KeyNode, + }), + } + } else { + unevaluatedProperties = &DynamicValue[*SchemaProxy, bool]{N: 1, B: schema.UnevaluatedProperties.Value.B} } } - - // check if unevaluated properties is a bool - if !schema.UnevaluatedProperties.IsEmpty() && schema.UnevaluatedProperties.Value.IsB() { - s.UnevaluatedProperties = &DynamicValue[*SchemaProxy, *bool]{ - B: schema.UnevaluatedProperties.Value.B, - N: 1, - } - } - - if !schema.UnevaluatedProperties.IsEmpty() { - } + s.UnevaluatedProperties = unevaluatedProperties s.Pattern = schema.Pattern.Value s.Format = schema.Format.Value @@ -248,19 +241,23 @@ func NewSchema(schema *base.Schema) *Schema { s.Type = append(s.Type, schema.Type.Value.B[i].Value) } } - if schema.AdditionalProperties.Value != nil { - if addPropSchema, ok := schema.AdditionalProperties.Value.(*base.SchemaProxy); ok { - s.AdditionalProperties = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{ - KeyNode: schema.AdditionalProperties.KeyNode, - ValueNode: schema.AdditionalProperties.ValueNode, - Value: addPropSchema, - }) - } else { - // TODO: check for slice and map types and unpack correctly. - s.AdditionalProperties = schema.AdditionalProperties.Value + var additionalProperties *DynamicValue[*SchemaProxy, bool] + if !schema.AdditionalProperties.IsEmpty() { + if schema.AdditionalProperties.Value.IsA() { + additionalProperties = &DynamicValue[*SchemaProxy, bool]{ + A: NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{ + ValueNode: schema.AdditionalProperties.ValueNode, + Value: schema.AdditionalProperties.Value.A, + KeyNode: schema.AdditionalProperties.KeyNode, + }), + } + } else { + additionalProperties = &DynamicValue[*SchemaProxy, bool]{N: 1, B: schema.AdditionalProperties.Value.B} } } + s.AdditionalProperties = additionalProperties + s.Description = schema.Description.Value s.Default = schema.Default.Value s.Const = schema.Const.Value @@ -423,7 +420,8 @@ func NewSchema(schema *base.Schema) *Schema { Value: schema.Items.Value.A, KeyNode: schema.Items.KeyNode, }, - )} + ), + } } else { items = &DynamicValue[*SchemaProxy, bool]{N: 1, B: schema.Items.Value.B} } diff --git a/datamodel/high/base/schema_test.go b/datamodel/high/base/schema_test.go index ccea88c..234adbc 100644 --- a/datamodel/high/base/schema_test.go +++ b/datamodel/high/base/schema_test.go @@ -111,7 +111,6 @@ func TestNewSchemaProxyRender(t *testing.T) { rice: $ref: '#/components/schemas/rice'` assert.Equal(t, desired, strings.TrimSpace(string(rend))) - } func TestNewSchemaProxy_WithObject(t *testing.T) { @@ -217,10 +216,7 @@ properties: type: number description: a number example: "2" - additionalProperties: - - chicken - - nugget - - soup + additionalProperties: false somethingB: type: object exclusiveMinimum: true @@ -241,8 +237,7 @@ properties: attribute: true x-pizza: love additionalProperties: - why: yes - thatIs: true + type: string additionalProperties: true required: - them @@ -315,12 +310,12 @@ $anchor: anchor` assert.Equal(t, "anchor", compiled.Anchor) wentLow := compiled.GoLow() - assert.Equal(t, 129, wentLow.AdditionalProperties.ValueNode.Line) + assert.Equal(t, 125, wentLow.AdditionalProperties.ValueNode.Line) assert.NotNil(t, compiled.GoLowUntyped()) // now render it out! schemaBytes, _ := compiled.Render() - assert.Len(t, schemaBytes, 3494) + assert.Len(t, schemaBytes, 3417) } func TestSchemaObjectWithAllOfSequenceOrder(t *testing.T) { @@ -504,7 +499,7 @@ required: [cake, fish]` assert.Equal(t, float64(334), compiled.Properties["somethingB"].Schema().ExclusiveMaximum.B) assert.Len(t, compiled.Properties["somethingB"].Schema().Properties["somethingBProp"].Schema().Type, 2) - assert.Equal(t, "nice", compiled.AdditionalProperties.(*SchemaProxy).Schema().Description) + assert.Equal(t, "nice", compiled.AdditionalProperties.A.Schema().Description) wentLow := compiled.GoLow() assert.Equal(t, 97, wentLow.AdditionalProperties.ValueNode.Line) @@ -556,7 +551,6 @@ func TestSchemaProxy_GoLow(t *testing.T) { spNil := NewSchemaProxy(nil) assert.Nil(t, spNil.GoLow()) assert.Nil(t, spNil.GoLowUntyped()) - } func getHighSchema(t *testing.T, yml string) *Schema { @@ -836,7 +830,6 @@ allOf: // now render it out, it should be identical. schemaBytes, _ := compiled.Render() assert.Equal(t, testSpec, string(schemaBytes)) - } func TestNewSchemaProxy_RenderSchemaWithMultipleObjectTypes(t *testing.T) { @@ -934,8 +927,7 @@ func TestNewSchemaProxy_RenderSchemaEnsurePropertyOrdering(t *testing.T) { attribute: true x-pizza: love additionalProperties: - why: yes - thatIs: true + type: string additionalProperties: true xml: name: XML Thing` @@ -989,60 +981,6 @@ func TestNewSchemaProxy_RenderSchemaCheckDiscriminatorMappingOrder(t *testing.T) assert.Equal(t, testSpec, strings.TrimSpace(string(schemaBytes))) } -func TestNewSchemaProxy_RenderSchemaCheckAdditionalPropertiesSlice(t *testing.T) { - testSpec := `additionalProperties: - - one - - two - - miss a few - - ninety nine - - hundred` - - var compNode yaml.Node - _ = yaml.Unmarshal([]byte(testSpec), &compNode) - - sp := new(lowbase.SchemaProxy) - err := sp.Build(nil, compNode.Content[0], nil) - assert.NoError(t, err) - - lowproxy := low.NodeReference[*lowbase.SchemaProxy]{ - Value: sp, - ValueNode: compNode.Content[0], - } - - schemaProxy := NewSchemaProxy(&lowproxy) - compiled := schemaProxy.Schema() - - // now render it out, it should be identical. - schemaBytes, _ := compiled.Render() - assert.Len(t, schemaBytes, 91) -} - -func TestNewSchemaProxy_RenderSchemaCheckAdditionalPropertiesSliceMap(t *testing.T) { - testSpec := `additionalProperties: - - nice: cake - - yummy: beer - - hot: coffee` - - var compNode yaml.Node - _ = yaml.Unmarshal([]byte(testSpec), &compNode) - - sp := new(lowbase.SchemaProxy) - err := sp.Build(nil, compNode.Content[0], nil) - assert.NoError(t, err) - - lowproxy := low.NodeReference[*lowbase.SchemaProxy]{ - Value: sp, - ValueNode: compNode.Content[0], - } - - schemaProxy := NewSchemaProxy(&lowproxy) - compiled := schemaProxy.Schema() - - // now render it out, it should be identical. - schemaBytes, _ := compiled.Render() - assert.Len(t, schemaBytes, 75) -} - func TestNewSchemaProxy_CheckDefaultBooleanFalse(t *testing.T) { testSpec := `default: false` @@ -1192,8 +1130,7 @@ unevaluatedProperties: true ` highSchema := getHighSchema(t, yml) - value := true - assert.EqualValues(t, &value, highSchema.UnevaluatedProperties.B) + assert.True(t, highSchema.UnevaluatedProperties.B) } func TestUnevaluatedPropertiesBoolean_False(t *testing.T) { @@ -1203,6 +1140,54 @@ unevaluatedProperties: false ` highSchema := getHighSchema(t, yml) - value := false - assert.EqualValues(t, &value, highSchema.UnevaluatedProperties.B) + assert.False(t, highSchema.UnevaluatedProperties.B) +} + +func TestUnevaluatedPropertiesBoolean_Unset(t *testing.T) { + yml := ` +type: number +` + highSchema := getHighSchema(t, yml) + + assert.Nil(t, highSchema.UnevaluatedProperties) +} + +func TestAdditionalProperties(t *testing.T) { + testSpec := `type: object +properties: + additionalPropertiesSimpleSchema: + type: object + additionalProperties: + type: string + additionalPropertiesBool: + type: object + additionalProperties: true + additionalPropertiesAnyOf: + type: object + additionalProperties: + anyOf: + - type: string + - type: array + items: + type: string +` + + var compNode yaml.Node + _ = yaml.Unmarshal([]byte(testSpec), &compNode) + + sp := new(lowbase.SchemaProxy) + err := sp.Build(nil, compNode.Content[0], nil) + assert.NoError(t, err) + + lowproxy := low.NodeReference[*lowbase.SchemaProxy]{ + Value: sp, + ValueNode: compNode.Content[0], + } + + schemaProxy := NewSchemaProxy(&lowproxy) + compiled := schemaProxy.Schema() + + assert.Equal(t, []string{"string"}, compiled.Properties["additionalPropertiesSimpleSchema"].Schema().AdditionalProperties.A.Schema().Type) + assert.Equal(t, true, compiled.Properties["additionalPropertiesBool"].Schema().AdditionalProperties.B) + assert.Equal(t, []string{"string"}, compiled.Properties["additionalPropertiesAnyOf"].Schema().AdditionalProperties.A.Schema().AnyOf[0].Schema().Type) } diff --git a/datamodel/high/v3/document_test.go b/datamodel/high/v3/document_test.go index 5c67cd8..bb09a4d 100644 --- a/datamodel/high/v3/document_test.go +++ b/datamodel/high/v3/document_test.go @@ -220,7 +220,7 @@ func TestNewDocument_Components_Schemas(t *testing.T) { d := h.Components.Schemas["Drink"] assert.Len(t, d.Schema().Required, 2) - assert.True(t, d.Schema().AdditionalProperties.(bool)) + assert.True(t, d.Schema().AdditionalProperties.B) assert.Equal(t, "drinkType", d.Schema().Discriminator.PropertyName) assert.Equal(t, "some value", d.Schema().Discriminator.Mapping["drink"]) assert.Equal(t, 516, d.Schema().Discriminator.GoLow().PropertyName.ValueNode.Line) @@ -377,7 +377,6 @@ func testBurgerShop(t *testing.T, h *Document, checkLines bool) { assert.Equal(t, 310, okResp.Links["LocateBurger"].GoLow().OperationId.ValueNode.Line) assert.Equal(t, 118, burgersOp.Post.Security[0].GoLow().Requirements.ValueNode.Line) } - } func TestStripeAsDoc(t *testing.T) { @@ -435,7 +434,6 @@ func TestDigitalOceanAsDocFromSHA(t *testing.T) { d := NewDocument(lowDoc) assert.NotNil(t, d) assert.Equal(t, 183, len(d.Paths.PathItems)) - } func TestPetstoreAsDoc(t *testing.T) { @@ -463,7 +461,6 @@ func TestCircularReferencesDoc(t *testing.T) { } func TestDocument_MarshalYAML(t *testing.T) { - // create a new document initTest() h := NewDocument(lowDoc) @@ -477,11 +474,9 @@ func TestDocument_MarshalYAML(t *testing.T) { highDoc := NewDocument(lDoc) testBurgerShop(t, highDoc, false) - } func TestDocument_MarshalIndention(t *testing.T) { - data, _ := os.ReadFile("../../../test_specs/single-definition.yaml") info, _ := datamodel.ExtractSpecInfo(data) @@ -495,11 +490,9 @@ func TestDocument_MarshalIndention(t *testing.T) { rendered = highDoc.RenderWithIndention(4) assert.NotEqual(t, string(data), strings.TrimSpace(string(rendered))) - } func TestDocument_MarshalIndention_Error(t *testing.T) { - data, _ := os.ReadFile("../../../test_specs/single-definition.yaml") info, _ := datamodel.ExtractSpecInfo(data) @@ -513,11 +506,9 @@ func TestDocument_MarshalIndention_Error(t *testing.T) { rendered = highDoc.RenderWithIndention(4) assert.NotEqual(t, string(data), strings.TrimSpace(string(rendered))) - } func TestDocument_MarshalJSON(t *testing.T) { - data, _ := os.ReadFile("../../../test_specs/petstorev3.json") info, _ := datamodel.ExtractSpecInfo(data) @@ -537,7 +528,6 @@ func TestDocument_MarshalJSON(t *testing.T) { } func TestDocument_MarshalYAMLInline(t *testing.T) { - // create a new document initTest() h := NewDocument(lowDoc) @@ -551,11 +541,9 @@ func TestDocument_MarshalYAMLInline(t *testing.T) { highDoc := NewDocument(lDoc) testBurgerShop(t, highDoc, false) - } func TestDocument_MarshalYAML_TestRefs(t *testing.T) { - // create a new document yml := `openapi: 3.1.0 paths: @@ -633,7 +621,6 @@ components: } func TestDocument_MarshalYAML_TestParamRefs(t *testing.T) { - // create a new document yml := `openapi: 3.1.0 paths: @@ -686,7 +673,6 @@ components: } func TestDocument_MarshalYAML_TestModifySchemas(t *testing.T) { - // create a new document yml := `openapi: 3.1.0 components: diff --git a/datamodel/low/base/schema.go b/datamodel/low/base/schema.go index a0c44b5..da9d194 100644 --- a/datamodel/low/base/schema.go +++ b/datamodel/low/base/schema.go @@ -3,7 +3,6 @@ package base import ( "crypto/sha256" "fmt" - "reflect" "sort" "strconv" "strings" @@ -100,7 +99,7 @@ type Schema struct { PatternProperties low.NodeReference[map[low.KeyReference[string]]low.ValueReference[*SchemaProxy]] PropertyNames low.NodeReference[*SchemaProxy] UnevaluatedItems low.NodeReference[*SchemaProxy] - UnevaluatedProperties low.NodeReference[*SchemaDynamicValue[*SchemaProxy, *bool]] + UnevaluatedProperties low.NodeReference[*SchemaDynamicValue[*SchemaProxy, bool]] Anchor low.NodeReference[string] // Compatible with all versions @@ -121,7 +120,7 @@ type Schema struct { Enum low.NodeReference[[]low.ValueReference[any]] Not low.NodeReference[*SchemaProxy] Properties low.NodeReference[map[low.KeyReference[string]]low.ValueReference[*SchemaProxy]] - AdditionalProperties low.NodeReference[any] + AdditionalProperties low.NodeReference[*SchemaDynamicValue[*SchemaProxy, bool]] Description low.NodeReference[string] ContentEncoding low.NodeReference[string] ContentMediaType low.NodeReference[string] @@ -189,53 +188,7 @@ func (s *Schema) Hash() [32]byte { d = append(d, fmt.Sprint(s.MinProperties.Value)) } if !s.AdditionalProperties.IsEmpty() { - - // check type of properties, if we have a low level map, we need to hash the values in a repeatable - // order. - to := reflect.TypeOf(s.AdditionalProperties.Value) - vo := reflect.ValueOf(s.AdditionalProperties.Value) - var values []string - switch to.Kind() { - case reflect.Slice: - for i := 0; i < vo.Len(); i++ { - vn := vo.Index(i).Interface() - - if jh, ok := vn.(low.HasValueUnTyped); ok { - vn = jh.GetValueUntyped() - fg := reflect.TypeOf(vn) - gf := reflect.ValueOf(vn) - - if fg.Kind() == reflect.Map { - for _, ky := range gf.MapKeys() { - hu := ky.Interface() - values = append(values, fmt.Sprintf("%s:%s", hu, low.GenerateHashString(gf.MapIndex(ky).Interface()))) - } - continue - } - values = append(values, fmt.Sprintf("%d:%s", i, low.GenerateHashString(vn))) - } - } - sort.Strings(values) - d = append(d, strings.Join(values, "||")) - - case reflect.Map: - for _, k := range vo.MapKeys() { - var x string - var l int - var v any - // extract key - if o, ok := k.Interface().(low.HasKeyNode); ok { - x = o.GetKeyNode().Value - l = o.GetKeyNode().Line - v = vo.MapIndex(k).Interface().(low.HasValueNodeUntyped).GetValueNode().Value - } - values = append(values, fmt.Sprintf("%d:%s:%s", l, x, low.GenerateHashString(v))) - } - sort.Strings(values) - d = append(d, strings.Join(values, "||")) - default: - d = append(d, low.GenerateHashString(s.AdditionalProperties.Value)) - } + d = append(d, low.GenerateHashString(s.AdditionalProperties.Value)) } if !s.Description.IsEmpty() { d = append(d, fmt.Sprint(s.Description.Value)) @@ -667,77 +620,24 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { } } - _, addPLabel, addPNode := utils.FindKeyNodeFullTop(AdditionalPropertiesLabel, root.Content) - if addPNode != nil { - if utils.IsNodeMap(addPNode) || utils.IsNodeArray(addPNode) { - // check if this is a reference, or an inline schema. - isRef, _, _ := utils.IsNodeRefValue(addPNode) - var sp *SchemaProxy - // now check if this object has a 'type' if so, it's a schema, if not... it's a random - // object, and we should treat it as a raw map. - if _, v := utils.FindKeyNodeTop(TypeLabel, addPNode.Content); v != nil { - sp = &SchemaProxy{ - kn: addPLabel, - vn: addPNode, - idx: idx, - } - } - if isRef { - _, vn := utils.FindKeyNodeTop("$ref", addPNode.Content) - sp = &SchemaProxy{ - kn: addPLabel, - vn: addPNode, - idx: idx, - isReference: true, - referenceLookup: vn.Value, - } - } - - // if this is a reference, or a schema, we're done. - if sp != nil { - s.AdditionalProperties = low.NodeReference[any]{Value: sp, KeyNode: addPLabel, ValueNode: addPNode} - } else { - - // if this is a map, collect all the keys and values. - if utils.IsNodeMap(addPNode) { - - addProps := make(map[low.KeyReference[string]]low.ValueReference[any]) - var label string - for g := range addPNode.Content { - if g%2 == 0 { - label = addPNode.Content[g].Value - continue - } else { - addProps[low.KeyReference[string]{Value: label, KeyNode: addPNode.Content[g-1]}] = low.ValueReference[any]{Value: addPNode.Content[g].Value, ValueNode: addPNode.Content[g]} - } - } - s.AdditionalProperties = low.NodeReference[any]{Value: addProps, KeyNode: addPLabel, ValueNode: addPNode} - } - - // if the node is an array, extract everything into a trackable structure - if utils.IsNodeArray(addPNode) { - var addProps []low.ValueReference[any] - - // if this is an array or maps, encode the map items correctly. - for i := range addPNode.Content { - if utils.IsNodeMap(addPNode.Content[i]) { - var prop map[string]any - _ = addPNode.Content[i].Decode(&prop) - addProps = append(addProps, - low.ValueReference[any]{Value: prop, ValueNode: addPNode.Content[i]}) - } else { - addProps = append(addProps, - low.ValueReference[any]{Value: addPNode.Content[i].Value, ValueNode: addPNode.Content[i]}) - } - } - - s.AdditionalProperties = low.NodeReference[any]{Value: addProps, KeyNode: addPLabel, ValueNode: addPNode} - } - } + // check additionalProperties type for schema or bool + addPropsIsBool := false + addPropsBoolValue := true + _, addPLabel, addPValue := utils.FindKeyNodeFullTop(AdditionalPropertiesLabel, root.Content) + if addPValue != nil { + if utils.IsNodeBoolValue(addPValue) { + addPropsIsBool = true + addPropsBoolValue, _ = strconv.ParseBool(addPValue.Value) } - if utils.IsNodeBoolValue(addPNode) { - b, _ := strconv.ParseBool(addPNode.Value) - s.AdditionalProperties = low.NodeReference[any]{Value: b, KeyNode: addPLabel, ValueNode: addPNode} + } + if addPropsIsBool { + s.AdditionalProperties = low.NodeReference[*SchemaDynamicValue[*SchemaProxy, bool]]{ + Value: &SchemaDynamicValue[*SchemaProxy, bool]{ + B: addPropsBoolValue, + N: 1, + }, + KeyNode: addPLabel, + ValueNode: addPValue, } } @@ -827,9 +727,9 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { } } if unevalIsBool { - s.UnevaluatedProperties = low.NodeReference[*SchemaDynamicValue[*SchemaProxy, *bool]]{ - Value: &SchemaDynamicValue[*SchemaProxy, *bool]{ - B: &unevalBoolValue, + s.UnevaluatedProperties = low.NodeReference[*SchemaDynamicValue[*SchemaProxy, bool]]{ + Value: &SchemaDynamicValue[*SchemaProxy, bool]{ + B: unevalBoolValue, N: 1, }, KeyNode: unevalLabel, @@ -838,7 +738,7 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { } var allOf, anyOf, oneOf, prefixItems []low.ValueReference[*SchemaProxy] - var items, not, contains, sif, selse, sthen, propertyNames, unevalItems, unevalProperties low.ValueReference[*SchemaProxy] + var items, not, contains, sif, selse, sthen, propertyNames, unevalItems, unevalProperties, addProperties low.ValueReference[*SchemaProxy] _, allOfLabel, allOfValue := utils.FindKeyNodeFullTop(AllOfLabel, root.Content) _, anyOfLabel, anyOfValue := utils.FindKeyNodeFullTop(AnyOfLabel, root.Content) @@ -852,6 +752,7 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { _, propNamesLabel, propNamesValue := utils.FindKeyNodeFullTop(PropertyNamesLabel, root.Content) _, unevalItemsLabel, unevalItemsValue := utils.FindKeyNodeFullTop(UnevaluatedItemsLabel, root.Content) _, unevalPropsLabel, unevalPropsValue := utils.FindKeyNodeFullTop(UnevaluatedPropertiesLabel, root.Content) + _, addPropsLabel, addPropsValue := utils.FindKeyNodeFullTop(AdditionalPropertiesLabel, root.Content) errorChan := make(chan error) allOfChan := make(chan schemaProxyBuildResult) @@ -867,6 +768,7 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { propNamesChan := make(chan schemaProxyBuildResult) unevalItemsChan := make(chan schemaProxyBuildResult) unevalPropsChan := make(chan schemaProxyBuildResult) + addPropsChan := make(chan schemaProxyBuildResult) totalBuilds := countSubSchemaItems(allOfValue) + countSubSchemaItems(anyOfValue) + @@ -921,6 +823,10 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { totalBuilds++ go buildSchema(unevalPropsChan, unevalPropsLabel, unevalPropsValue, errorChan, idx) } + if !addPropsIsBool && addPropsValue != nil { + totalBuilds++ + go buildSchema(addPropsChan, addPropsLabel, addPropsValue, errorChan, idx) + } completeCount := 0 for completeCount < totalBuilds { @@ -966,6 +872,9 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { case r := <-unevalPropsChan: completeCount++ unevalProperties = r.v + case r := <-addPropsChan: + completeCount++ + addProperties = r.v } } @@ -1056,14 +965,23 @@ func (s *Schema) Build(root *yaml.Node, idx *index.SpecIndex) error { } } if !unevalIsBool && !unevalProperties.IsEmpty() { - s.UnevaluatedProperties = low.NodeReference[*SchemaDynamicValue[*SchemaProxy, *bool]]{ - Value: &SchemaDynamicValue[*SchemaProxy, *bool]{ + s.UnevaluatedProperties = low.NodeReference[*SchemaDynamicValue[*SchemaProxy, bool]]{ + Value: &SchemaDynamicValue[*SchemaProxy, bool]{ A: unevalProperties.Value, }, KeyNode: unevalPropsLabel, ValueNode: unevalPropsValue, } } + if !addPropsIsBool && !addProperties.IsEmpty() { + s.AdditionalProperties = low.NodeReference[*SchemaDynamicValue[*SchemaProxy, bool]]{ + Value: &SchemaDynamicValue[*SchemaProxy, bool]{ + A: addProperties.Value, + }, + KeyNode: addPropsLabel, + ValueNode: addPropsValue, + } + } return nil } @@ -1219,8 +1137,7 @@ func buildSchema(schemas chan schemaProxyBuildResult, labelNode, valueNode *yaml v: *r.res, } } - } - if utils.IsNodeArray(valueNode) { + } else if utils.IsNodeArray(valueNode) { refBuilds := 0 results := make([]*low.ValueReference[*SchemaProxy], len(valueNode.Content)) @@ -1261,6 +1178,8 @@ func buildSchema(schemas chan schemaProxyBuildResult, labelNode, valueNode *yaml v: *r, } } + } else { + errors <- fmt.Errorf("build schema failed: unexpected node type: %s, line %d, col %d", valueNode.Tag, valueNode.Line, valueNode.Column) } } } diff --git a/datamodel/low/base/schema_test.go b/datamodel/low/base/schema_test.go index 81b9f1f..ac385c8 100644 --- a/datamodel/low/base/schema_test.go +++ b/datamodel/low/base/schema_test.go @@ -169,7 +169,7 @@ func Test_Schema(t *testing.T) { schErr := sch.Build(rootNode.Content[0], nil) assert.NoError(t, schErr) assert.Equal(t, "something object", sch.Description.Value) - assert.True(t, sch.AdditionalProperties.Value.(bool)) + assert.True(t, sch.AdditionalProperties.Value.B) assert.Len(t, sch.Properties.Value, 2) v := sch.FindProperty("somethingB") @@ -1172,30 +1172,7 @@ func TestExtractSchema_AdditionalPropertiesAsSchema(t *testing.T) { res, err := ExtractSchema(idxNode.Content[0], idx) - assert.NotNil(t, res.Value.Schema().AdditionalProperties.Value.(*SchemaProxy).Schema()) - assert.Nil(t, err) -} - -func TestExtractSchema_AdditionalPropertiesAsSchemaSlice(t *testing.T) { - yml := `components: - schemas: - Something: - additionalProperties: - - nice: rice` - - var iNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &iNode) - assert.NoError(t, mErr) - idx := index.NewSpecIndex(&iNode) - - yml = `$ref: '#/components/schemas/Something'` - - var idxNode yaml.Node - _ = yaml.Unmarshal([]byte(yml), &idxNode) - - res, err := ExtractSchema(idxNode.Content[0], idx) - - assert.NotNil(t, res.Value.Schema().AdditionalProperties.Value.([]low.ValueReference[interface{}])) + assert.NotNil(t, res.Value.Schema().AdditionalProperties.Value.A.Schema()) assert.Nil(t, err) } @@ -1244,7 +1221,7 @@ func TestExtractSchema_AdditionalProperties_Ref(t *testing.T) { _ = yaml.Unmarshal([]byte(yml), &idxNode) res, err := ExtractSchema(idxNode.Content[0], idx) - assert.NotNil(t, res.Value.Schema().AdditionalProperties.Value.(*SchemaProxy).Schema()) + assert.NotNil(t, res.Value.Schema().AdditionalProperties.Value.A.Schema()) assert.Nil(t, err) } @@ -1378,7 +1355,7 @@ func TestSchema_Hash_Equal(t *testing.T) { uniqueItems: 1 maxProperties: 10 minProperties: 1 - additionalProperties: anything + additionalProperties: true description: milky contentEncoding: rubber shoes contentMediaType: paper tiger @@ -1420,7 +1397,7 @@ func TestSchema_Hash_Equal(t *testing.T) { uniqueItems: 1 maxProperties: 10 minProperties: 1 - additionalProperties: anything + additionalProperties: true description: milky contentEncoding: rubber shoes contentMediaType: paper tiger @@ -1611,7 +1588,7 @@ func TestSchema_UnevaluatedPropertiesAsBool_DefinedAsTrue(t *testing.T) { res, _ := ExtractSchema(idxNode.Content[0], idx) assert.True(t, res.Value.Schema().UnevaluatedProperties.Value.IsB()) - assert.True(t, *res.Value.Schema().UnevaluatedProperties.Value.B) + assert.True(t, res.Value.Schema().UnevaluatedProperties.Value.B) assert.Equal(t, "571bd1853c22393131e2dcadce86894da714ec14968895c8b7ed18154b2be8cd", low.GenerateHashString(res.Value.Schema().UnevaluatedProperties.Value)) @@ -1636,7 +1613,7 @@ func TestSchema_UnevaluatedPropertiesAsBool_DefinedAsFalse(t *testing.T) { res, _ := ExtractSchema(idxNode.Content[0], idx) assert.True(t, res.Value.Schema().UnevaluatedProperties.Value.IsB()) - assert.False(t, *res.Value.Schema().UnevaluatedProperties.Value.B) + assert.False(t, res.Value.Schema().UnevaluatedProperties.Value.B) } func TestSchema_UnevaluatedPropertiesAsBool_Undefined(t *testing.T) { diff --git a/datamodel/low/v3/create_document_test.go b/datamodel/low/v3/create_document_test.go index 5f72764..636b23b 100644 --- a/datamodel/low/v3/create_document_test.go +++ b/datamodel/low/v3/create_document_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/pb33f/libopenapi/datamodel" - "github.com/pb33f/libopenapi/datamodel/low/base" "github.com/stretchr/testify/assert" ) @@ -52,7 +51,6 @@ func BenchmarkCreateDocument_Circular(b *testing.B) { } func BenchmarkCreateDocument_k8s(b *testing.B) { - data, _ := os.ReadFile("../../../test_specs/k8s.json") info, _ := datamodel.ExtractSpecInfo(data) @@ -69,7 +67,6 @@ func BenchmarkCreateDocument_k8s(b *testing.B) { } func TestCircularReferenceError(t *testing.T) { - data, _ := os.ReadFile("../../../test_specs/circular-tests.yaml") info, _ := datamodel.ExtractSpecInfo(data) circDoc, err := CreateDocumentFromConfig(info, &datamodel.DocumentConfiguration{ @@ -81,7 +78,6 @@ func TestCircularReferenceError(t *testing.T) { } func TestCircularReference_IgnoreArray(t *testing.T) { - spec := `openapi: 3.1.0 components: schemas: @@ -110,7 +106,6 @@ components: } func TestCircularReference_IgnorePoly(t *testing.T) { - spec := `openapi: 3.1.0 components: schemas: @@ -168,7 +163,6 @@ func BenchmarkCreateDocument_Petstore(b *testing.B) { } func TestCreateDocumentStripe(t *testing.T) { - data, _ := os.ReadFile("../../../test_specs/stripe.yaml") info, _ := datamodel.ExtractSpecInfo(data) d, err := CreateDocumentFromConfig(info, &datamodel.DocumentConfiguration{ @@ -303,7 +297,6 @@ func TestCreateDocument_Tags(t *testing.T) { // this is why we will need a higher level API to this model, this looks cool and all, but dude. assert.Equal(t, "now?", extension.Value.(map[string]interface{})["ok"].([]interface{})[0].(map[string]interface{})["what"]) } - } /// tag2 @@ -313,7 +306,6 @@ func TestCreateDocument_Tags(t *testing.T) { assert.Equal(t, "https://pb33f.io", doc.Tags.Value[1].Value.ExternalDocs.Value.URL.Value) assert.NotEmpty(t, doc.Tags.Value[1].Value.ExternalDocs.Value.URL.Value) assert.Len(t, doc.Tags.Value[1].Value.Extensions, 0) - } func TestCreateDocument_Paths(t *testing.T) { @@ -438,7 +430,6 @@ func TestCreateDocument_Paths(t *testing.T) { assert.NotNil(t, servers) assert.Len(t, servers, 1) assert.Equal(t, "https://pb33f.io", servers[0].Value.URL.Value) - } func TestCreateDocument_Components_Schemas(t *testing.T) { @@ -464,7 +455,6 @@ func TestCreateDocument_Components_Schemas(t *testing.T) { p := fries.Value.Schema().FindProperty("favoriteDrink") assert.Equal(t, "a frosty cold beverage can be coke or sprite", p.Value.Schema().Description.Value) - } func TestCreateDocument_Components_SecuritySchemes(t *testing.T) { @@ -493,7 +483,6 @@ func TestCreateDocument_Components_SecuritySchemes(t *testing.T) { readScope = oAuth.Flows.Value.AuthorizationCode.Value.FindScope("write:burgers") assert.NotNil(t, readScope) assert.Equal(t, "modify burgers and stuff", readScope.Value) - } func TestCreateDocument_Components_Responses(t *testing.T) { @@ -506,7 +495,6 @@ func TestCreateDocument_Components_Responses(t *testing.T) { assert.NotNil(t, dressingResponse.Value) assert.Equal(t, "all the dressings for a burger.", dressingResponse.Value.Description.Value) assert.Len(t, dressingResponse.Value.Content.Value, 1) - } func TestCreateDocument_Components_Examples(t *testing.T) { @@ -593,7 +581,6 @@ func TestCreateDocument_Component_Discriminator(t *testing.T) { assert.Nil(t, dsc.FindMappingValue("don't exist")) assert.NotNil(t, doc.GetExternalDocs()) assert.Nil(t, doc.FindSecurityRequirement("scooby doo")) - } func TestCreateDocument_CheckAdditionalProperties_Schema(t *testing.T) { @@ -601,11 +588,8 @@ func TestCreateDocument_CheckAdditionalProperties_Schema(t *testing.T) { components := doc.Components.Value d := components.FindSchema("Dressing") assert.NotNil(t, d.Value.Schema().AdditionalProperties.Value) - if n, ok := d.Value.Schema().AdditionalProperties.Value.(*base.SchemaProxy); ok { - assert.Equal(t, "something in here.", n.Schema().Description.Value) - } else { - assert.Fail(t, "should be a schema") - } + + assert.True(t, d.Value.Schema().AdditionalProperties.Value.IsA(), "should be a schema") } func TestCreateDocument_CheckAdditionalProperties_Bool(t *testing.T) { @@ -613,7 +597,7 @@ func TestCreateDocument_CheckAdditionalProperties_Bool(t *testing.T) { components := doc.Components.Value d := components.FindSchema("Drink") assert.NotNil(t, d.Value.Schema().AdditionalProperties.Value) - assert.True(t, d.Value.Schema().AdditionalProperties.Value.(bool)) + assert.True(t, d.Value.Schema().AdditionalProperties.Value.B) } func TestCreateDocument_Components_Error(t *testing.T) { @@ -667,7 +651,6 @@ components: AllowRemoteReferences: false, }) assert.Len(t, err, 1) - } func TestCreateDocument_Paths_Errors(t *testing.T) { @@ -776,8 +759,8 @@ func TestCreateDocument_YamlAnchor(t *testing.T) { assert.NotNil(t, jsonGet) // Should this work? It doesn't - //postJsonType := examplePath.GetValue().Post.GetValue().RequestBody.GetValue().FindContent("application/json") - //assert.NotNil(t, postJsonType) + // postJsonType := examplePath.GetValue().Post.GetValue().RequestBody.GetValue().FindContent("application/json") + // assert.NotNil(t, postJsonType) } func ExampleCreateDocument() { diff --git a/what-changed/model/schema.go b/what-changed/model/schema.go index 7d4b80c..17f9903 100644 --- a/what-changed/model/schema.go +++ b/what-changed/model/schema.go @@ -5,14 +5,14 @@ package model import ( "fmt" - "golang.org/x/exp/slices" "sort" "sync" + "golang.org/x/exp/slices" + "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/datamodel/low/base" v3 "github.com/pb33f/libopenapi/datamodel/low/v3" - "github.com/pb33f/libopenapi/utils" "gopkg.in/yaml.v3" ) @@ -24,16 +24,17 @@ import ( // PropertyChanges.Changes, and not in the AnyOfChanges property. type SchemaChanges struct { *PropertyChanges - DiscriminatorChanges *DiscriminatorChanges `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` - AllOfChanges []*SchemaChanges `json:"allOf,omitempty" yaml:"allOf,omitempty"` - AnyOfChanges []*SchemaChanges `json:"anyOf,omitempty" yaml:"anyOf,omitempty"` - OneOfChanges []*SchemaChanges `json:"oneOf,omitempty" yaml:"oneOf,omitempty"` - NotChanges *SchemaChanges `json:"not,omitempty" yaml:"not,omitempty"` - ItemsChanges *SchemaChanges `json:"items,omitempty" yaml:"items,omitempty"` - SchemaPropertyChanges map[string]*SchemaChanges `json:"properties,omitempty" yaml:"properties,omitempty"` - ExternalDocChanges *ExternalDocChanges `json:"externalDoc,omitempty" yaml:"externalDoc,omitempty"` - XMLChanges *XMLChanges `json:"xml,omitempty" yaml:"xml,omitempty"` - ExtensionChanges *ExtensionChanges `json:"extensions,omitempty" yaml:"extensions,omitempty"` + DiscriminatorChanges *DiscriminatorChanges `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` + AllOfChanges []*SchemaChanges `json:"allOf,omitempty" yaml:"allOf,omitempty"` + AnyOfChanges []*SchemaChanges `json:"anyOf,omitempty" yaml:"anyOf,omitempty"` + OneOfChanges []*SchemaChanges `json:"oneOf,omitempty" yaml:"oneOf,omitempty"` + NotChanges *SchemaChanges `json:"not,omitempty" yaml:"not,omitempty"` + ItemsChanges *SchemaChanges `json:"items,omitempty" yaml:"items,omitempty"` + SchemaPropertyChanges map[string]*SchemaChanges `json:"properties,omitempty" yaml:"properties,omitempty"` + ExternalDocChanges *ExternalDocChanges `json:"externalDoc,omitempty" yaml:"externalDoc,omitempty"` + XMLChanges *XMLChanges `json:"xml,omitempty" yaml:"xml,omitempty"` + ExtensionChanges *ExtensionChanges `json:"extensions,omitempty" yaml:"extensions,omitempty"` + AdditionalPropertiesChanges *SchemaChanges `json:"additionalProperties,omitempty" yaml:"additionalProperties,omitempty"` // 3.1 specifics IfChanges *SchemaChanges `json:"if,omitempty" yaml:"if,omitempty"` @@ -102,6 +103,9 @@ func (s *SchemaChanges) GetAllChanges() []*Change { if s.UnevaluatedPropertiesChanges != nil { changes = append(changes, s.UnevaluatedPropertiesChanges.GetAllChanges()...) } + if s.AdditionalPropertiesChanges != nil { + changes = append(changes, s.AdditionalPropertiesChanges.GetAllChanges()...) + } if s.SchemaPropertyChanges != nil { for n := range s.SchemaPropertyChanges { if s.SchemaPropertyChanges[n] != nil { @@ -185,6 +189,9 @@ func (s *SchemaChanges) TotalChanges() int { if s.UnevaluatedPropertiesChanges != nil { t += s.UnevaluatedPropertiesChanges.TotalChanges() } + if s.AdditionalPropertiesChanges != nil { + t += s.AdditionalPropertiesChanges.TotalChanges() + } if s.SchemaPropertyChanges != nil { for n := range s.SchemaPropertyChanges { if s.SchemaPropertyChanges[n] != nil { @@ -267,6 +274,9 @@ func (s *SchemaChanges) TotalBreakingChanges() int { if s.UnevaluatedPropertiesChanges != nil { t += s.UnevaluatedPropertiesChanges.TotalBreakingChanges() } + if s.AdditionalPropertiesChanges != nil { + t += s.AdditionalPropertiesChanges.TotalBreakingChanges() + } if s.DependentSchemasChanges != nil { for n := range s.DependentSchemasChanges { t += s.DependentSchemasChanges[n].TotalBreakingChanges() @@ -728,18 +738,36 @@ func checkSchemaPropertyChanges( New: rSchema, }) - // AdditionalProperties (only if not an object) - if !utils.IsNodeMap(lSchema.AdditionalProperties.ValueNode) && - !utils.IsNodeMap(rSchema.AdditionalProperties.ValueNode) { - props = append(props, &PropertyCheck{ - LeftNode: lSchema.AdditionalProperties.ValueNode, - RightNode: rSchema.AdditionalProperties.ValueNode, - Label: v3.AdditionalPropertiesLabel, - Changes: changes, - Breaking: false, - Original: lSchema, - New: rSchema, - }) + // AdditionalProperties + if lSchema.AdditionalProperties.Value != nil && rSchema.AdditionalProperties.Value != nil { + if lSchema.AdditionalProperties.Value.IsA() && rSchema.AdditionalProperties.Value.IsA() { + if !low.AreEqual(lSchema.AdditionalProperties.Value.A, rSchema.AdditionalProperties.Value.A) { + sc.AdditionalPropertiesChanges = CompareSchemas(lSchema.AdditionalProperties.Value.A, rSchema.AdditionalProperties.Value.A) + } + } else { + if lSchema.AdditionalProperties.Value.IsB() && rSchema.AdditionalProperties.Value.IsB() { + if lSchema.AdditionalProperties.Value.B != rSchema.AdditionalProperties.Value.B { + CreateChange(changes, Modified, v3.AdditionalPropertiesLabel, + lSchema.AdditionalProperties.ValueNode, rSchema.AdditionalProperties.ValueNode, true, + lSchema.AdditionalProperties.Value.B, rSchema.AdditionalProperties.Value.B) + } + } else { + CreateChange(changes, Modified, v3.AdditionalPropertiesLabel, + lSchema.AdditionalProperties.ValueNode, rSchema.AdditionalProperties.ValueNode, true, + lSchema.AdditionalProperties.Value.B, rSchema.AdditionalProperties.Value.B) + } + } + } + + // added AdditionalProperties + if lSchema.AdditionalProperties.Value == nil && rSchema.AdditionalProperties.Value != nil { + CreateChange(changes, ObjectAdded, v3.AdditionalPropertiesLabel, + nil, rSchema.AdditionalProperties.ValueNode, true, nil, rSchema.AdditionalProperties.Value) + } + // removed AdditionalProperties + if lSchema.AdditionalProperties.Value != nil && rSchema.AdditionalProperties.Value == nil { + CreateChange(changes, ObjectRemoved, v3.AdditionalPropertiesLabel, + lSchema.AdditionalProperties.ValueNode, nil, true, lSchema.AdditionalProperties.Value, nil) } // Description @@ -1108,20 +1136,6 @@ func checkSchemaPropertyChanges( // check extensions sc.ExtensionChanges = CompareExtensions(lSchema.Extensions, rSchema.Extensions) - // if additional properties is an object, then hash it - // AdditionalProperties (only if not an object) - if utils.IsNodeMap(lSchema.AdditionalProperties.ValueNode) || - utils.IsNodeMap(rSchema.AdditionalProperties.ValueNode) { - - lHash := low.GenerateHashString(lSchema.AdditionalProperties.ValueNode) - rHash := low.GenerateHashString(rSchema.AdditionalProperties.ValueNode) - if lHash != rHash { - CreateChange(changes, Modified, v3.AdditionalPropertiesLabel, - lSchema.AdditionalProperties.ValueNode, rSchema.AdditionalProperties.ValueNode, false, - lSchema.AdditionalProperties.Value, rSchema.AdditionalProperties.Value) - } - } - // check core properties CheckProperties(props) } diff --git a/what-changed/model/schema_test.go b/what-changed/model/schema_test.go index 10c3a54..fbdc835 100644 --- a/what-changed/model/schema_test.go +++ b/what-changed/model/schema_test.go @@ -5,13 +5,14 @@ package model import ( "fmt" + "testing" + "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/datamodel/low/base" v2 "github.com/pb33f/libopenapi/datamodel/low/v2" v3 "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/stretchr/testify/assert" - "testing" ) // These tests require full documents to be tested properly. schemas are perhaps the most complex @@ -158,7 +159,6 @@ components: } func test_BuildDoc(l, r string) (*v3.Document, *v3.Document) { - leftInfo, _ := datamodel.ExtractSpecInfo([]byte(l)) rightInfo, _ := datamodel.ExtractSpecInfo([]byte(r)) @@ -168,7 +168,6 @@ func test_BuildDoc(l, r string) (*v3.Document, *v3.Document) { } func test_BuildDocv2(l, r string) (*v2.Swagger, *v2.Swagger) { - leftInfo, _ := datamodel.ExtractSpecInfo([]byte(l)) rightInfo, _ := datamodel.ExtractSpecInfo([]byte(r)) @@ -278,7 +277,6 @@ components: assert.Equal(t, Modified, changes.Changes[0].ChangeType) assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) assert.Equal(t, "#/components/schemas/Yo", changes.Changes[0].Original) - } func TestCompareSchemas_InlineToRef(t *testing.T) { @@ -311,7 +309,6 @@ components: assert.Equal(t, Modified, changes.Changes[0].ChangeType) assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) assert.Equal(t, "#/components/schemas/Yo", changes.Changes[0].New) - } func TestCompareSchemas_Identical(t *testing.T) { @@ -1222,6 +1219,93 @@ components: assert.Equal(t, v3.UnevaluatedPropertiesLabel, changes.Changes[0].Property) } +func TestCompareSchemas_AdditionalProperties(t *testing.T) { + left := `openapi: 3.1 +components: + schemas: + OK: + additionalProperties: + type: string` + + right := `openapi: 3.1 +components: + schemas: + OK: + additionalProperties: + type: int` + + leftDoc, rightDoc := test_BuildDoc(left, right) + + // extract left reference schema and non reference schema. + lSchemaProxy := leftDoc.Components.Value.FindSchema("OK").Value + rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value + + changes := CompareSchemas(lSchemaProxy, rSchemaProxy) + assert.NotNil(t, changes) + assert.Equal(t, 1, changes.TotalChanges()) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, 1, changes.TotalBreakingChanges()) + assert.Equal(t, 1, changes.AdditionalPropertiesChanges.PropertyChanges.TotalChanges()) +} + +func TestCompareSchemas_AdditionalProperties_Added(t *testing.T) { + left := `openapi: 3.1 +components: + schemas: + OK: + type: string` + + right := `openapi: 3.1 +components: + schemas: + OK: + type: string + additionalProperties: + type: int` + + leftDoc, rightDoc := test_BuildDoc(left, right) + + // extract left reference schema and non reference schema. + lSchemaProxy := leftDoc.Components.Value.FindSchema("OK").Value + rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value + + changes := CompareSchemas(lSchemaProxy, rSchemaProxy) + assert.NotNil(t, changes) + assert.Equal(t, 1, changes.TotalChanges()) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, 1, changes.TotalBreakingChanges()) + assert.Equal(t, v3.AdditionalPropertiesLabel, changes.Changes[0].Property) +} + +func TestCompareSchemas_AdditionalProperties_Removed(t *testing.T) { + left := `openapi: 3.1 +components: + schemas: + OK: + type: string` + + right := `openapi: 3.1 +components: + schemas: + OK: + type: string + additionalProperties: + type: int` + + leftDoc, rightDoc := test_BuildDoc(left, right) + + // extract left reference schema and non reference schema. + lSchemaProxy := leftDoc.Components.Value.FindSchema("OK").Value + rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value + + changes := CompareSchemas(rSchemaProxy, lSchemaProxy) + assert.NotNil(t, changes) + assert.Equal(t, 1, changes.TotalChanges()) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, 1, changes.TotalBreakingChanges()) + assert.Equal(t, v3.AdditionalPropertiesLabel, changes.Changes[0].Property) +} + func TestCompareSchemas_UnevaluatedItems(t *testing.T) { left := `openapi: 3.1 components: @@ -1611,7 +1695,6 @@ components: assert.Equal(t, Modified, changes.AnyOfChanges[0].Changes[0].ChangeType) assert.Equal(t, "string", changes.AnyOfChanges[0].Changes[0].New) assert.Equal(t, "bool", changes.AnyOfChanges[0].Changes[0].Original) - } func TestCompareSchemas_OneOfModifyAndAddItem(t *testing.T) { @@ -1848,7 +1931,6 @@ components: assert.Equal(t, ObjectAdded, changes.Changes[0].ChangeType) assert.Equal(t, "0e563831440581c713657dd857a0ec3af1bd7308a43bd3cae9184f61d61b288f", low.HashToString(changes.Changes[0].NewObject.(*base.Discriminator).Hash())) - } func TestCompareSchemas_DiscriminatorRemove(t *testing.T) { @@ -1881,7 +1963,6 @@ components: assert.Equal(t, ObjectRemoved, changes.Changes[0].ChangeType) assert.Equal(t, "0e563831440581c713657dd857a0ec3af1bd7308a43bd3cae9184f61d61b288f", low.HashToString(changes.Changes[0].OriginalObject.(*base.Discriminator).Hash())) - } func TestCompareSchemas_ExternalDocsChange(t *testing.T) { @@ -1948,7 +2029,6 @@ components: assert.Equal(t, ObjectAdded, changes.Changes[0].ChangeType) assert.Equal(t, "2b7adf30f2ea3a7617ccf429a099617a9c03e8b5f3a23a89dba4b90f760010d7", low.HashToString(changes.Changes[0].NewObject.(*base.ExternalDoc).Hash())) - } func TestCompareSchemas_ExternalDocsRemove(t *testing.T) { @@ -1981,7 +2061,6 @@ components: assert.Equal(t, ObjectRemoved, changes.Changes[0].ChangeType) assert.Equal(t, "2b7adf30f2ea3a7617ccf429a099617a9c03e8b5f3a23a89dba4b90f760010d7", low.HashToString(changes.Changes[0].OriginalObject.(*base.ExternalDoc).Hash())) - } func TestCompareSchemas_AddExtension(t *testing.T) { @@ -2402,7 +2481,6 @@ components: assert.Equal(t, 1, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 1) assert.Equal(t, 1, changes.TotalBreakingChanges()) - } func TestCompareSchemas_SchemaAdditionalPropertiesCheck(t *testing.T) { @@ -2432,7 +2510,6 @@ components: assert.Equal(t, 1, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 1) assert.Equal(t, 0, changes.TotalBreakingChanges()) - } func TestCompareSchemas_Schema_DeletePoly(t *testing.T) { @@ -2466,7 +2543,6 @@ components: assert.Equal(t, 1, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 1) assert.Equal(t, 1, changes.TotalBreakingChanges()) - } func TestCompareSchemas_Schema_AddExamplesArray_AllOf(t *testing.T) { @@ -2499,7 +2575,6 @@ components: assert.Equal(t, 1, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 1) assert.Equal(t, 0, changes.TotalBreakingChanges()) - } func TestCompareSchemas_Schema_AddExampleMap_AllOf(t *testing.T) { @@ -2566,7 +2641,6 @@ components: assert.Equal(t, 1, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 1) assert.Equal(t, 0, changes.TotalBreakingChanges()) - } func TestCompareSchemas_Schema_AddExamplesMap(t *testing.T) { @@ -2601,7 +2675,6 @@ components: assert.Equal(t, 1, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 1) assert.Equal(t, 0, changes.TotalBreakingChanges()) - } func TestCompareSchemas_Schema_AddExamples(t *testing.T) { @@ -2661,7 +2734,6 @@ components: assert.Equal(t, 1, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 1) assert.Equal(t, 0, changes.TotalBreakingChanges()) - } func TestCompareSchemas_CheckIssue_170(t *testing.T) {