More hardening with digitla ocean

some ref handling was a bit strange, now it's rendering correctly. I have a feeling we will be back to the diff engine at some point soon, it's picking up some strange changes that are so deep in the model, I can't determine what is what, so we will wait for another set of triggers to appear.
This commit is contained in:
Dave Shanley
2023-03-25 10:49:33 -04:00
parent bc5a020c7a
commit 04eac2abe7
11 changed files with 140 additions and 61 deletions

View File

@@ -93,8 +93,6 @@ func (n *NodeBuilder) add(key string, i int) {
u++
}
}
default:
panic("not supported yet")
}
}
n.Nodes = append(n.Nodes, nodeEntry)
@@ -202,7 +200,6 @@ func (n *NodeBuilder) add(key string, i int) {
nodeEntry.Line = jk.GetKeyNode().Line
break
}
panic("this should not break.")
}
if nb.GetValueNode() != nil {
nodeEntry.Line = nb.GetValueNode().Line
@@ -256,10 +253,7 @@ func (n *NodeBuilder) Render() *yaml.Node {
node := n.Nodes[i]
n.AddYAMLNode(m, node)
}
if len(m.Content) > 0 {
return m
}
return nil
return m
}
// AddYAMLNode will add a new *yaml.Node to the parent node, using the tag, key and value provided.
@@ -366,14 +360,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *NodeEntry) *yaml.Nod
} else {
// this is a map, but it may be wrapped still.
bj := reflect.ValueOf(gh)
//yh := bj.Interface()
// if vg, jo := yh.(low.HasKeyNode); jo {
// fv := reflect.ValueOf(vg.GetKeyNode())
// orderedCollection = n.extractLowMapKeysWrapped(fv, x, orderedCollection, g)
// } else {
orderedCollection = n.extractLowMapKeysWrapped(bj, x, orderedCollection, g)
//}
}
} else {
// this is a map, without any low level details available (probably an extension map).
@@ -409,8 +396,6 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *NodeEntry) *yaml.Nod
}
if len(p.Content) > 0 {
valueNode = p
} else {
return parent
}
case reflect.Slice:
@@ -423,9 +408,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *NodeEntry) *yaml.Nod
sl := utils.CreateEmptySequenceNode()
skip := false
for i := 0; i < m.Len(); i++ {
sqi := m.Index(i).Interface()
// check if this is a reference.
if glu, ok := sqi.(GoesLowUntyped); ok {
if glu != nil {
@@ -506,8 +489,6 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *NodeEntry) *yaml.Nod
rawRender, _ := r.MarshalYAML()
if rawRender != nil {
valueNode = rawRender.(*yaml.Node)
} else {
return parent
}
} else {
@@ -551,8 +532,6 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *NodeEntry) *yaml.Nod
}
}
default:
panic("not supported yet")
}
if valueNode == nil {
return parent

View File

@@ -123,5 +123,10 @@ func ExtractExampleValue(exp *yaml.Node) any {
_ = exp.Decode(&m)
return m
}
if utils.IsNodeArray(exp) {
var m []interface{}
_ = exp.Decode(&m)
return m
}
return exp.Value
}

View File

@@ -124,6 +124,60 @@ value:
}
func TestExample_ExtractExampleValue_Map(t *testing.T) {
yml := `hot:
summer: nights
pizza: oven`
var idxNode yaml.Node
_ = yaml.Unmarshal([]byte(yml), &idxNode)
val := ExtractExampleValue(idxNode.Content[0])
if v, ok := val.(map[string]interface{}); ok {
if r, rok := v["hot"].(map[string]interface{}); rok {
assert.Equal(t, "nights", r["summer"])
assert.Equal(t, "oven", r["pizza"])
} else {
assert.Fail(t, "failed to decode correctly.")
}
} else {
assert.Fail(t, "failed to decode correctly.")
}
}
func TestExample_ExtractExampleValue_Slice(t *testing.T) {
yml := `- hot:
summer: nights
- hotter:
pizza: oven`
var idxNode yaml.Node
_ = yaml.Unmarshal([]byte(yml), &idxNode)
val := ExtractExampleValue(idxNode.Content[0])
if v, ok := val.([]interface{}); ok {
for w := range v {
if r, rok := v[w].(map[string]interface{}); rok {
for k := range r {
if k == "hotter" {
assert.Equal(t, "oven", r[k].(map[string]interface{})["pizza"])
}
if k == "hot" {
assert.Equal(t, "nights", r[k].(map[string]interface{})["summer"])
}
}
} else {
assert.Fail(t, "failed to decode correctly.")
}
}
} else {
assert.Fail(t, "failed to decode correctly.")
}
}
func TestExample_Hash(t *testing.T) {
left := `summary: hot

View File

@@ -215,10 +215,6 @@ func (s *Schema) Hash() [32]byte {
x = o.GetKeyNode().Value
l = o.GetKeyNode().Line
v = vo.MapIndex(k).Interface().(low.HasValueNodeUntyped).GetValueNode().Value
} else {
x = k.String()
l = 999
v = vo.MapIndex(k).Interface()
}
values = append(values, fmt.Sprintf("%d:%s:%s", l, x, low.GenerateHashString(v)))
}

View File

@@ -6,13 +6,14 @@ package v3
import (
"crypto/sha256"
"fmt"
"sort"
"strings"
"sync"
"github.com/pb33f/libopenapi/datamodel/low"
"github.com/pb33f/libopenapi/index"
"github.com/pb33f/libopenapi/utils"
"gopkg.in/yaml.v3"
"sort"
"strings"
"sync"
)
// PathItem represents a low-level OpenAPI 3+ PathItem object.
@@ -195,8 +196,9 @@ func (p *PathItem) Build(root *yaml.Node, idx *index.SpecIndex) error {
}
var op Operation
if ok, _, _ := utils.IsNodeRefValue(pathNode); ok {
opIsRef := false
var opRefVal string
if ok, _, ref := utils.IsNodeRefValue(pathNode); ok {
// According to OpenAPI spec the only valid $ref for paths is
// reference for the whole pathItem. Unfortunately, internet is full of invalid specs
// even from trusted companies like DigitalOcean where they tend to
@@ -206,8 +208,13 @@ func (p *PathItem) Build(root *yaml.Node, idx *index.SpecIndex) error {
// $ref: 'file.yaml'
// Check if that is the case and resolve such thing properly too.
opIsRef = true
opRefVal = ref
r, err := low.LocateRefNode(pathNode, idx)
if r != nil {
if r.Kind == yaml.DocumentNode {
r = r.Content[0]
}
pathNode = r
if r.Tag == "" {
// If it's a node from file, tag is empty
@@ -232,6 +239,10 @@ func (p *PathItem) Build(root *yaml.Node, idx *index.SpecIndex) error {
KeyNode: currentNode,
ValueNode: pathNode,
}
if opIsRef {
opRef.Reference = opRefVal
opRef.ReferenceNode = true
}
ops = append(ops, opRef)
@@ -260,8 +271,11 @@ func (p *PathItem) Build(root *yaml.Node, idx *index.SpecIndex) error {
opBuildChan := make(chan bool)
opErrorChan := make(chan error)
buildOpFunc := func(op low.NodeReference[*Operation], ch chan<- bool, errCh chan<- error) {
buildOpFunc := func(op low.NodeReference[*Operation], ch chan<- bool, errCh chan<- error, ref string) {
er := op.Value.Build(op.ValueNode, idx)
if ref != "" {
op.Value.Reference.Reference = ref
}
if er != nil {
errCh <- er
}
@@ -273,7 +287,11 @@ func (p *PathItem) Build(root *yaml.Node, idx *index.SpecIndex) error {
}
for _, op := range ops {
go buildOpFunc(op, opBuildChan, opErrorChan)
ref := ""
if op.ReferenceNode {
ref = op.Reference
}
go buildOpFunc(op, opBuildChan, opErrorChan, ref)
}
n := 0

View File

@@ -164,7 +164,7 @@ func (d *document) RenderAndReload() ([]byte, Document, *DocumentModel[v3high.Do
if err != nil {
return newBytes, nil, nil, []error{err}
}
newDoc, err := NewDocument(newBytes)
newDoc, err := NewDocumentWithConfiguration(newBytes, d.config)
if err != nil {
return newBytes, newDoc, nil, []error{err}
}

View File

@@ -4,6 +4,7 @@ package libopenapi
import (
"fmt"
"github.com/pb33f/libopenapi/datamodel"
"github.com/pb33f/libopenapi/datamodel/high/base"
"github.com/pb33f/libopenapi/what-changed/model"
"github.com/stretchr/testify/assert"
@@ -210,7 +211,6 @@ func TestDocument_RenderAndReload_ChangeCheck_Asana(t *testing.T) {
dat, newDoc, _, _ := doc.RenderAndReload()
assert.NotNil(t, dat)
//_ = os.WriteFile("asana_reloaded.yaml", dat, 0644)
// compare documents
compReport, errs := CompareDocuments(doc, newDoc)
@@ -218,22 +218,12 @@ func TestDocument_RenderAndReload_ChangeCheck_Asana(t *testing.T) {
// get flat list of changes.
flatChanges := compReport.GetAllChanges()
// remove everything that is an example set to "true" (asana has 45 of these). the lib converts this to a boolean.
var filtered []*model.Change
for i := range flatChanges {
if flatChanges[i].Property != "example" {
filtered = append(filtered, flatChanges[i])
}
}
assert.Nil(t, errs)
tc := compReport.TotalChanges()
bc := compReport.TotalBreakingChanges()
assert.Equal(t, 0, bc)
assert.Equal(t, 45, tc)
assert.Equal(t, 21, tc)
// there should be no other changes than the 519 example "true" re-renders
assert.Equal(t, 0, len(filtered))
// there are some properties re-rendered that trigger changes.
assert.Equal(t, 21, len(flatChanges))
}
@@ -544,5 +534,35 @@ components:
// render the document.
rend, _ := result.Model.Render()
assert.Len(t, rend, 644)
}
func TestDocument_OperationsAsRefs(t *testing.T) {
ae := `operationId: thisIsAnOperationId
summary: a test thing
description: this is a test, that does a test.`
_ = os.WriteFile("test-operation.yaml", []byte(ae), 0644)
var d = `openapi: "3.1"
paths:
/an/operation:
get:
$ref: test-operation.yaml`
doc, err := NewDocumentWithConfiguration([]byte(d), datamodel.NewOpenDocumentConfiguration())
if err != nil {
panic(err)
}
result, errs := doc.BuildV3Model()
if len(errs) > 0 {
panic(errs)
}
// render the document.
rend, _ := result.Model.Render()
assert.Equal(t, d, strings.TrimSpace(string(rend)))
}

View File

@@ -10,6 +10,7 @@ func CreateRefNode(ref string) *yaml.Node {
nodes := make([]*yaml.Node, 2)
nodes[0] = CreateStringNode("$ref")
nodes[1] = CreateStringNode(ref)
nodes[1].Style = yaml.SingleQuotedStyle
m.Content = nodes
return m
}

View File

@@ -142,6 +142,7 @@ parameters:
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 2, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 2)
assert.Equal(t, 2, extChanges.TotalBreakingChanges())
assert.Equal(t, Modified, extChanges.ParameterChanges[0].Changes[0].ChangeType)
assert.Equal(t, v3.InLabel, extChanges.ParameterChanges[0].Changes[0].Property)
@@ -182,6 +183,7 @@ parameters:
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, ObjectAdded, extChanges.Changes[0].ChangeType)
}
@@ -221,6 +223,7 @@ parameters:
// compare.
extChanges := ComparePathItems(&rDoc, &lDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, ObjectRemoved, extChanges.Changes[0].ChangeType)
}
@@ -332,6 +335,7 @@ parameters:
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 7, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 7)
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
}
func TestComparePathItem_V2_RemoveMethods(t *testing.T) {
@@ -371,6 +375,7 @@ parameters:
// compare.
extChanges := ComparePathItems(&rDoc, &lDoc)
assert.Equal(t, 7, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 7)
assert.Equal(t, 7, extChanges.TotalBreakingChanges())
}
@@ -485,6 +490,7 @@ x-thing: dang.`
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 12, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 12)
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
}
@@ -511,6 +517,7 @@ parameters:
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType)
assert.Equal(t, v3.ParametersLabel, extChanges.Changes[0].Property)
@@ -537,8 +544,9 @@ parameters:
_ = rDoc.Build(rNode.Content[0], nil)
// compare.
extChanges := ComparePathItems(&rDoc, &lDoc)
extChanges := ComparePathItemsV3(&rDoc, &lDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, PropertyRemoved, extChanges.Changes[0].ChangeType)
assert.Equal(t, v3.ParametersLabel, extChanges.Changes[0].Property)
@@ -581,6 +589,7 @@ trace:
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 8, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 8)
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
}
@@ -621,6 +630,7 @@ trace:
// compare.
extChanges := ComparePathItems(&rDoc, &lDoc)
assert.Equal(t, 8, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 8)
assert.Equal(t, 8, extChanges.TotalBreakingChanges())
}
@@ -653,5 +663,6 @@ func TestComparePathItem_V3_ChangeParam(t *testing.T) {
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
}

View File

@@ -5,11 +5,14 @@ package model
import (
"github.com/pb33f/libopenapi/datamodel/low"
"github.com/pb33f/libopenapi/datamodel/low/v2"
"github.com/pb33f/libopenapi/datamodel/low/v3"
v3 "github.com/pb33f/libopenapi/datamodel/low/v3"
"reflect"
)
import (
"github.com/pb33f/libopenapi/datamodel/low/v2"
)
// ResponseChanges represents changes found between two Swagger or OpenAPI Response objects.
type ResponseChanges struct {
*PropertyChanges
@@ -23,7 +26,6 @@ type ResponseChanges struct {
// OpenAPI Response Properties.
ContentChanges map[string]*MediaTypeChanges `json:"content,omitempty" yaml:"content,omitempty"`
LinkChanges map[string]*LinkChanges `json:"links,omitempty" yaml:"links,omitempty"`
ServerChanges *ServerChanges `json:"server,omitempty" yaml:"server,omitempty"`
}
// GetAllChanges returns a slice of all changes made between RequestBody objects
@@ -39,9 +41,6 @@ func (r *ResponseChanges) GetAllChanges() []*Change {
if r.ExamplesChanges != nil {
changes = append(changes, r.ExamplesChanges.GetAllChanges()...)
}
if r.ServerChanges != nil {
changes = append(changes, r.ServerChanges.GetAllChanges()...)
}
for k := range r.HeadersChanges {
changes = append(changes, r.HeadersChanges[k].GetAllChanges()...)
}
@@ -66,9 +65,6 @@ func (r *ResponseChanges) TotalChanges() int {
if r.ExamplesChanges != nil {
c += r.ExamplesChanges.TotalChanges()
}
if r.ServerChanges != nil {
c += r.ServerChanges.TotalChanges()
}
for k := range r.HeadersChanges {
c += r.HeadersChanges[k].TotalChanges()
}
@@ -88,9 +84,6 @@ func (r *ResponseChanges) TotalBreakingChanges() int {
if r.SchemaChanges != nil {
c += r.SchemaChanges.TotalBreakingChanges()
}
if r.ServerChanges != nil {
c += r.ServerChanges.TotalBreakingChanges()
}
for k := range r.HeadersChanges {
c += r.HeadersChanges[k].TotalBreakingChanges()
}

View File

@@ -22,6 +22,8 @@ headers:
description: a header
examples:
bam: alam
server:
url: https://example.com
x-toot: poot`
right := left