From 0d84dcf7901f3728d26c04f2797f9708b7d7ea16 Mon Sep 17 00:00:00 2001 From: Andrey Pokhilko Date: Thu, 24 Feb 2022 18:10:50 +0300 Subject: [PATCH 1/2] Roll back some changes --- agent/pkg/oas/tree.go | 68 +----------------------------------- agent/pkg/oas/utils.go | 79 ------------------------------------------ 2 files changed, 1 insertion(+), 146 deletions(-) diff --git a/agent/pkg/oas/tree.go b/agent/pkg/oas/tree.go index 272872119..e9b73d3f4 100644 --- a/agent/pkg/oas/tree.go +++ b/agent/pkg/oas/tree.go @@ -217,46 +217,7 @@ func (n *Node) searchInConstants(pathChunk string) *Node { } func (n *Node) compact() { - // TODO: introduce and leverage "dirty" flag? - var param *Node - // find the param - for _, subnode := range n.children { - if subnode.constant != nil { - continue - } - - param = subnode - } - - if param != nil { - // take its regex - pRegex := param.pathParam.Schema.Pattern - if pRegex != nil { - newChildren := make([]*Node, 0) - - // compact the constants via regex - for _, subnode := range n.children { - if subnode.constant != nil { - if pRegex.Match([]byte(*subnode.constant)) { - param.merge(subnode) - continue - } - } - newChildren = append(newChildren, subnode) - } - - if len(n.children) != len(newChildren) { - logger.Log.Debugf("Shrinking children from %d to %d", len(n.children), len(newChildren)) - n.children = newChildren - n.compact() - } - } - } - - // recurse into next tree level - for _, subnode := range n.children { - subnode.compact() - } + // TODO } func (n *Node) listPaths() *openapi.Paths { @@ -345,30 +306,3 @@ func (n *Node) countParentParams() int { } return res } - -func (n *Node) merge(other *Node) { - if n.constant == nil && other.constant == nil { - // make sure the params will match by name later in merge - other.pathParam.Name = n.pathParam.Name - } - - if n.pathObj != nil && other.pathObj != nil { - mergePathObj(n.pathObj, other.pathObj) - } - - // TODO: if n is param and other is constant, could have added constant as an example - -outer: - for _, oChild := range other.children { - for _, nChild := range n.children { - matchedConst := oChild.constant != nil && oChild.constant == nChild.constant - matchedParam := oChild.constant == nil && nChild.constant == nil - if matchedConst || matchedParam { - // TODO: if both are params, could have merged their examples - nChild.merge(oChild) - continue outer - } - } - n.children = append(n.children, oChild) - } -} diff --git a/agent/pkg/oas/utils.go b/agent/pkg/oas/utils.go index 4205c29ef..59fb0849b 100644 --- a/agent/pkg/oas/utils.go +++ b/agent/pkg/oas/utils.go @@ -474,82 +474,3 @@ func intersectSliceWithMap(required []string, names map[string]struct{}) []strin } return required } - -func mergePathObj(po *openapi.PathObj, other *openapi.PathObj) { - // merge parameters - mergeParamLists(&po.Parameters, &other.Parameters) - - // merge ops - mergeOps(&po.Get, &other.Get) - mergeOps(&po.Put, &other.Put) - mergeOps(&po.Options, &other.Options) - mergeOps(&po.Patch, &other.Patch) - mergeOps(&po.Delete, &other.Delete) - mergeOps(&po.Head, &other.Head) - mergeOps(&po.Trace, &other.Trace) - mergeOps(&po.Post, &other.Post) -} - -func mergeParamLists(params **openapi.ParameterList, other **openapi.ParameterList) { - if *other == nil { - return - } - - if *params == nil { - *params = new(openapi.ParameterList) - } - -outer: - for _, o := range **other { - oParam, err := o.ResolveParameter(paramResolver) - if err != nil { - logger.Log.Warningf("Failed to resolve reference: %s", err) - continue - } - - for _, p := range **params { - param, err := p.ResolveParameter(paramResolver) - if err != nil { - logger.Log.Warningf("Failed to resolve reference: %s", err) - continue - } - - if param.In == oParam.In && param.Name == oParam.Name { - // TODO: merge examples? transfer schema pattern? - continue outer - } - } - - **params = append(**params, oParam) - } -} - -func mergeOps(op **openapi.Operation, other **openapi.Operation) { - if *other == nil { - return - } - - if *op == nil { - *op = *other - } else { - // merge parameters - mergeParamLists(&(*op).Parameters, &(*other).Parameters) - - // merge responses - mergeOpResponses(&(*op).Responses, &(*other).Responses) - - // merge request body - mergeOpReqBodies(&(*op).RequestBody, &(*other).RequestBody) - - // historical OpIDs - // merge kpis - } -} - -func mergeOpReqBodies(r *openapi.RequestBody, r2 *openapi.RequestBody) { - -} - -func mergeOpResponses(r *openapi.Responses, o *openapi.Responses) { - -} From ea5f96df377bad9d8159db03f4c14e30b541e2ed Mon Sep 17 00:00:00 2001 From: Andrey Pokhilko Date: Thu, 24 Feb 2022 18:13:58 +0300 Subject: [PATCH 2/2] Fix test --- .../oas/test_artifacts/params.har.spec.json | 162 ++++++++++++------ agent/pkg/oas/utils_test.go | 27 --- 2 files changed, 108 insertions(+), 81 deletions(-) diff --git a/agent/pkg/oas/test_artifacts/params.har.spec.json b/agent/pkg/oas/test_artifacts/params.har.spec.json index 7e984ff37..cb2081ec2 100644 --- a/agent/pkg/oas/test_artifacts/params.har.spec.json +++ b/agent/pkg/oas/test_artifacts/params.har.spec.json @@ -2,7 +2,7 @@ "openapi": "3.1.0", "info": { "title": "https://httpbin.org", - "description": "Mizu observed 18 entries (0 failed), at 0.11 hits/s, average response time is 0.18 seconds", + "description": "Mizu observed 19 entries (0 failed), at 0.10 hits/s, average response time is 0.17 seconds", "version": "1.0" }, "servers": [ @@ -63,7 +63,6 @@ } } }, - "x-sample-entry": 0, "x-last-seen-ts": 1567750581.74, "x-counters-total": { "entries": 2, @@ -82,7 +81,8 @@ "sumRT": 1.26, "sumDuration": 1 } - } + }, + "x-sample-entry": 0 } }, "/body-optional": { @@ -98,16 +98,6 @@ } } }, - "x-sample-entry": 0, - "x-counters-total": { - "entries": 3, - "failures": 0, - "firstSeen": 1567750581.74, - "lastSeen": 1567750581.75, - "sumRT": 0.00, - "sumDuration": 0.01 - }, - "x-last-seen-ts": 1567750581.75, "x-counters-per-source": { "": { "entries": 3, @@ -118,6 +108,16 @@ "sumDuration": 0.01 } }, + "x-last-seen-ts": 1567750581.75, + "x-sample-entry": 0, + "x-counters-total": { + "entries": 3, + "failures": 0, + "firstSeen": 1567750581.74, + "lastSeen": 1567750581.75, + "sumRT": 0.00, + "sumDuration": 0.01 + }, "requestBody": { "description": "Generic request body", "content": { @@ -141,15 +141,6 @@ } } }, - "x-sample-entry": 0, - "x-counters-total": { - "entries": 1, - "failures": 0, - "firstSeen": 1567750581.75, - "lastSeen": 1567750581.75, - "sumRT": 0.00, - "sumDuration": 0 - }, "x-counters-per-source": { "": { "entries": 1, @@ -160,7 +151,16 @@ "sumDuration": 0 } }, + "x-sample-entry": 0, "x-last-seen-ts": 1567750581.75, + "x-counters-total": { + "entries": 1, + "failures": 0, + "firstSeen": 1567750581.75, + "lastSeen": 1567750581.75, + "sumRT": 0.00, + "sumDuration": 0 + }, "requestBody": { "description": "Generic request body", "content": { @@ -187,7 +187,6 @@ } } }, - "x-sample-entry": 0, "x-last-seen-ts": 1567750582.74, "x-counters-total": { "entries": 1, @@ -207,6 +206,7 @@ "sumDuration": 0 } }, + "x-sample-entry": 0, "requestBody": { "description": "Generic request body", "content": { @@ -263,8 +263,8 @@ "sumDuration": 1 } }, - "x-sample-entry": 0, "x-last-seen-ts": 1567750581.74, + "x-sample-entry": 0, "x-counters-total": { "entries": 2, "failures": 0, @@ -319,6 +319,60 @@ } } }, + "/param-patterns/prefix-gibberish-fine/{prefixgibberishfineId}": { + "get": { + "tags": [ + "param-patterns" + ], + "summary": "/param-patterns/prefix-gibberish-fine/{prefixgibberishfineId}", + "description": "Mizu observed 1 entries (0 failed), at 0.00 hits/s, average response time is 0.00 seconds", + "operationId": "", + "responses": { + "200": { + "description": "Successful call with status 200", + "content": { + "": {} + } + } + }, + "x-last-seen-ts": 1567750582, + "x-counters-total": { + "entries": 1, + "failures": 0, + "firstSeen": 1567750582, + "lastSeen": 1567750582, + "sumRT": 0.00, + "sumDuration": 0 + }, + "x-counters-per-source": { + "": { + "entries": 1, + "failures": 0, + "firstSeen": 1567750582, + "lastSeen": 1567750582, + "sumRT": 0.00, + "sumDuration": 0 + } + }, + "x-sample-entry": 0 + }, + "parameters": [ + { + "name": "prefixgibberishfineId", + "in": "path", + "required": true, + "style": "simple", + "schema": { + "type": "string" + }, + "examples": { + "example #0": { + "value": "234324" + } + } + } + ] + }, "/param-patterns/{parampatternId}": { "get": { "tags": [ @@ -335,6 +389,7 @@ } } }, + "x-sample-entry": 0, "x-counters-per-source": { "": { "entries": 2, @@ -345,7 +400,6 @@ "sumDuration": 9.53e-7 } }, - "x-sample-entry": 0, "x-last-seen-ts": 1567750582.00, "x-counters-total": { "entries": 2, @@ -402,6 +456,7 @@ } } }, + "x-sample-entry": 0, "x-last-seen-ts": 1567750582.00, "x-counters-total": { "entries": 1, @@ -420,8 +475,7 @@ "sumRT": 0.00, "sumDuration": 0 } - }, - "x-sample-entry": 0 + } }, "parameters": [ { @@ -469,6 +523,15 @@ } } }, + "x-sample-entry": 0, + "x-counters-total": { + "entries": 1, + "failures": 0, + "firstSeen": 1567750582.00, + "lastSeen": 1567750582.00, + "sumRT": 0.00, + "sumDuration": 0 + }, "x-counters-per-source": { "": { "entries": 1, @@ -479,16 +542,7 @@ "sumDuration": 0 } }, - "x-sample-entry": 0, - "x-last-seen-ts": 1567750582.00, - "x-counters-total": { - "entries": 1, - "failures": 0, - "firstSeen": 1567750582.00, - "lastSeen": 1567750582.00, - "sumRT": 0.00, - "sumDuration": 0 - } + "x-last-seen-ts": 1567750582.00 }, "parameters": [ { @@ -616,7 +670,6 @@ } } }, - "x-last-seen-ts": 1567750579.74, "x-counters-total": { "entries": 1, "failures": 0, @@ -635,7 +688,8 @@ "sumDuration": 0 } }, - "x-sample-entry": 0 + "x-sample-entry": 0, + "x-last-seen-ts": 1567750579.74 }, "parameters": [ { @@ -670,8 +724,6 @@ } } }, - "x-sample-entry": 0, - "x-last-seen-ts": 1567750483.86, "x-counters-total": { "entries": 1, "failures": 0, @@ -689,7 +741,9 @@ "sumRT": 0.11, "sumDuration": 0 } - } + }, + "x-sample-entry": 0, + "x-last-seen-ts": 1567750483.86 }, "parameters": [ { @@ -726,16 +780,6 @@ } } }, - "x-sample-entry": 0, - "x-last-seen-ts": 1567750578.74, - "x-counters-total": { - "entries": 1, - "failures": 0, - "firstSeen": 1567750578.74, - "lastSeen": 1567750578.74, - "sumRT": 0.63, - "sumDuration": 0 - }, "x-counters-per-source": { "": { "entries": 1, @@ -745,6 +789,16 @@ "sumRT": 0.63, "sumDuration": 0 } + }, + "x-sample-entry": 0, + "x-last-seen-ts": 1567750578.74, + "x-counters-total": { + "entries": 1, + "failures": 0, + "firstSeen": 1567750578.74, + "lastSeen": 1567750578.74, + "sumRT": 0.63, + "sumDuration": 0 } }, "parameters": [ @@ -769,7 +823,7 @@ } }, "x-counters-total": { - "entries": 18, + "entries": 19, "failures": 0, "firstSeen": 1567750483.86, "lastSeen": 1567750582.74, @@ -778,7 +832,7 @@ }, "x-counters-per-source": { "": { - "entries": 18, + "entries": 19, "failures": 0, "firstSeen": 1567750483.86, "lastSeen": 1567750582.74, diff --git a/agent/pkg/oas/utils_test.go b/agent/pkg/oas/utils_test.go index 4813926de..ae4b21ef3 100644 --- a/agent/pkg/oas/utils_test.go +++ b/agent/pkg/oas/utils_test.go @@ -1,8 +1,6 @@ package oas import ( - "github.com/chanced/openapi" - "reflect" "testing" ) @@ -59,28 +57,3 @@ func TestStrRunes(t *testing.T) { t.Logf("Failed") } } - -func TestOpMerging(t *testing.T) { - testCases := []struct { - op1 *openapi.Operation - op2 *openapi.Operation - res *openapi.Operation - }{ - {nil, nil, nil}, - {&openapi.Operation{}, nil, &openapi.Operation{}}, - {nil, &openapi.Operation{}, &openapi.Operation{}}, - { - &openapi.Operation{OperationID: "op1"}, - &openapi.Operation{OperationID: "op2"}, - &openapi.Operation{OperationID: "op1"}, - }, - // has historicIds - } - for _, tc := range testCases { - mergeOps(&tc.op1, &tc.op2) - - if !reflect.DeepEqual(tc.op1, tc.res) { - t.Errorf("Does not match expected") - } - } -}