From fd1699147911d258796e38ae5bb88ac4f2964a56 Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Fri, 27 Aug 2021 01:03:50 +0200 Subject: [PATCH 1/3] keep track of query params values for requests --- Makefile | 9 +- cmd/liveapichecker/main.go | 31 +++- go.mod | 1 + pkg/analyzer/analyzer_test.go | 109 +++++++++++ pkg/analyzer/coverage.go | 43 +++-- pkg/analyzer/coverage_queryparams.go | 217 ++++++++++++++++++++++ pkg/analyzer/coverage_queryparams_test.go | 42 +++++ pkg/analyzer/openapiv2checker.go | 16 ++ 8 files changed, 440 insertions(+), 28 deletions(-) create mode 100644 pkg/analyzer/analyzer_test.go create mode 100644 pkg/analyzer/coverage_queryparams.go create mode 100644 pkg/analyzer/coverage_queryparams_test.go create mode 100644 pkg/analyzer/openapiv2checker.go diff --git a/Makefile b/Makefile index 5d837ac..db47a09 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,13 @@ +VERSION ?= v0.3 docker: - docker build -t dhontecillas/liveapichecker:latest -t dhontecillas/liveapichecker:v0.1 . + docker build -t dhontecillas/liveapichecker:latest -t dhontecillas/liveapichecker:$(VERSION) . docker image prune --filter label=stage=builder dockeralpine: - docker build -f Dockerfile.alpine -t dhontecillas/liveapichecker-alpine:latest -t dhontecillas/liveapichecker-alpine:v0.1 . + docker build -f Dockerfile.alpine -t dhontecillas/liveapichecker-alpine:latest -t dhontecillas/liveapichecker-alpine:$(VERSION) . docker image prune --filter label=stage=builder + +dockerpushalpine: + docker push dhontecillas/liveapichecker-alpine:latest + docker push dhontecillas/liveapichecker-alpine:$(VERSION) diff --git a/cmd/liveapichecker/main.go b/cmd/liveapichecker/main.go index 070ef37..2716a57 100644 --- a/cmd/liveapichecker/main.go +++ b/cmd/liveapichecker/main.go @@ -29,14 +29,18 @@ func main() { v.AutomaticEnv() v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + fmt.Printf("*******************************************\n") + fmt.Printf("* Live API Checker v0.3 *\n") + fmt.Printf("*******************************************\n") + fileName := v.GetString(OpenAPIFileKey) if len(fileName) == 0 { - panic("cannot read OPENAPI_FILE filename") + panic(fmt.Sprintf("cannot read %d filename", OpenAPIFileKey)) } forwardTo := v.GetString(ForwardToKey) if len(forwardTo) == 0 { - panic("cannot read forward to") + panic(fmt.Sprintf("cannot read forward to config %s", ForwardToKey)) } fmt.Printf("checking %s against %s\n", fileName, forwardTo) @@ -50,9 +54,15 @@ func main() { var dumpCovFn func() outFile := v.GetString(ReportFileKey) if len(outFile) > 0 { + fmt.Printf("\nReport filename: %s\n", outFile) dumpCovFn = func() { + wd, _ := os.Getwd() + fmt.Printf("\ndumping results to %s (%s)\n", outFile, wd) covChecker.DumpResultsToFile(outFile) } + defer dumpCovFn() + } else { + fmt.Printf("\ncannot read report file name: %s\n", ReportFileKey) } var reportsSrv *http.Server reportsAddress := v.GetString(ReportServerAddress) @@ -79,13 +89,18 @@ func main() { signal.Notify(sigChan, os.Interrupt) <-sigChan + fmt.Printf("*******************************************\n") + fmt.Printf("* STARTING SHUTDOWN *\n") + fmt.Printf("* Live API Checker v0.3 *\n") + fmt.Printf("*******************************************\n") + + go shutdownServer(reportsSrv) shutdownServer(proxySrv) - shutdownServer(reportsSrv) - if dumpCovFn != nil { - fmt.Printf("post shutdown cleanup\n") - dumpCovFn() - } + fmt.Printf("*******************************************\n") + fmt.Printf("* SHUTDOWN FINISHED *\n") + fmt.Printf("* Live API Checker v0.3 *\n") + fmt.Printf("*******************************************\n") } func launchServer(hfn http.Handler, address string) *http.Server { @@ -96,7 +111,7 @@ func launchServer(hfn http.Handler, address string) *http.Server { go func() { if err := srv.ListenAndServe(); err != nil { if err == http.ErrServerClosed { - fmt.Printf("shutting down...\n") + fmt.Printf("shutting down... %#v\n", srv) } else { fmt.Printf("error %s\nSHUTTING DOWN", err.Error()) } diff --git a/go.mod b/go.mod index 9ffb2d2..363ad64 100644 --- a/go.mod +++ b/go.mod @@ -10,4 +10,5 @@ require ( github.com/go-openapi/spec v0.19.14 github.com/go-openapi/strfmt v0.19.11 github.com/spf13/viper v1.7.1 + github.com/stretchr/testify v1.7.0 // indirect ) diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go new file mode 100644 index 0000000..150c9d8 --- /dev/null +++ b/pkg/analyzer/analyzer_test.go @@ -0,0 +1,109 @@ +package analyzer_test + +var ( + testSwagger string = ` +swagger: "2.0" +info: + version: 1.0.0 + title: Swagger Petstore + license: + name: MIT +host: petstore.swagger.io +basePath: /v1 +schemes: + - http +consumes: + - application/json +produces: + - application/json +paths: + /pets: + get: + summary: List all pets + operationId: listPets + tags: + - pets + parameters: + - name: limit + in: query + description: How many items to return at one time (max 100) + required: false + type: integer + format: int32 + responses: + "200": + description: A paged array of pets + headers: + x-next: + type: string + description: A link to the next page of responses + schema: + $ref: '#/definitions/Pets' + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' + post: + summary: Create a pet + operationId: createPets + tags: + - pets + responses: + "201": + description: Null response + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' + /pets/{petId}: + get: + summary: Info for a specific pet + operationId: showPetById + tags: + - pets + parameters: + - name: petId + in: path + required: true + description: The id of the pet to retrieve + type: string + responses: + "200": + description: Expected response to a valid request + schema: + $ref: '#/definitions/Pets' + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' +definitions: + Pet: + type: "object" + required: + - id + - name + properties: + id: + type: integer + format: int64 + name: + type: string + tag: + type: string + Pets: + type: array + items: + $ref: '#/definitions/Pet' + Error: + type: "object" + required: + - code + - message + properties: + code: + type: integer + format: int32 + message: + type: string +` +) diff --git a/pkg/analyzer/coverage.go b/pkg/analyzer/coverage.go index f746dfa..8671df1 100644 --- a/pkg/analyzer/coverage.go +++ b/pkg/analyzer/coverage.go @@ -11,38 +11,53 @@ import ( "github.com/dhontecillas/liveapichecker/pkg/pathmatcher" "github.com/dhontecillas/liveapichecker/pkg/proxy" "github.com/go-openapi/loads" + "github.com/go-openapi/spec" ) // CoverageChecker uses an openapi specdoc and checks // recorded api calls to keep track of what endpoints // have been covered type CoverageChecker struct { + rwMutex sync.RWMutex + specDoc *loads.Document pathMatcher *pathmatcher.PathMatcher basePath string // covered is a map of path -> method -> status code -> covered - covered map[string]map[string]*EndpointCoverage - allEndpoints []*EndpointCoverage - rwMutex sync.RWMutex + covered map[string]map[string]*EndpointCoverage + + // allEndpoints []*EndpointCoverage + // reportNonMatchedRequests bool } // EndpointCoverage contains the information about // how an endpoint has been covered type EndpointCoverage struct { - Method string `json:"method"` - Path string `json:"path"` - StatusCodes map[int]bool `json:"statusCodes"` - UndocumentedStatusCodes map[int]bool `json:"undocumentedStatusCodes"` + Method string `json:"method"` + Path string `json:"path"` + StatusCodes map[int]bool `json:"statusCodes"` + UndocumentedStatusCodes map[int]bool `json:"undocumentedStatusCodes"` + Params *ParamsCoverage `json:"params"` } // NewEndpointCoverage creates a new EndpontCoverage data -func NewEndpointCoverage(method string, path string) *EndpointCoverage { +func NewEndpointCoverage(method string, path string, + opSpec *spec.Operation) *EndpointCoverage { + + /* + covVariants := NewFullCoverageMinVariants() + pcc, err := NewParamsCoverageChecker(method, path, opSpec, &covVariants) + if err != nil { + pc = &ParamsCoverage{} + } + */ return &EndpointCoverage{ Method: method, Path: path, StatusCodes: make(map[int]bool), UndocumentedStatusCodes: make(map[int]bool), + Params: nil, } } @@ -56,10 +71,6 @@ func NewCoverageChecker(specDoc *loads.Document) *CoverageChecker { // if not basePath is set, it might be set to . bp = "/" } - /* - fmt.Printf("HOST: %s\n", specDoc.Host()) - fmt.Printf("SPEC: %#v\n", specDoc.OrigSpec()) - */ covered := make(map[string]map[string]*EndpointCoverage) pMatcher := pathmatcher.NewPathMatcher() @@ -68,13 +79,11 @@ func NewCoverageChecker(specDoc *loads.Document) *CoverageChecker { for rePath, def := range mops { mU := strings.ToUpper(method) routePath := path.Join(bp, rePath) - ec := NewEndpointCoverage(mU, routePath) - // fmt.Printf("%s %s (bp: %s , p: %s)\n", method, routePath, bp, rePath) + ec := NewEndpointCoverage(mU, rePath, def) pMatcher.AddRoute(method, routePath) if def.Responses != nil { - for v, _ := range def.Responses.StatusCodeResponses { + for v := range def.Responses.StatusCodeResponses { ec.StatusCodes[v] = false - // fmt.Printf("%d -> %#v\n", v, r) } } if _, ok := covered[routePath]; !ok { @@ -96,8 +105,6 @@ func NewCoverageChecker(specDoc *loads.Document) *CoverageChecker { // ProcessRecordedResponse implements the RecordedRresponseProcessorHandler // interface, and updates the stats for received request func (cc *CoverageChecker) ProcessRecordedResponse(rwr *proxy.ResponseWriterRecorder) { - fmt.Printf("\nanalizing request\n") - reqPath := path.Clean(rwr.Req.URL.Path) matchedPath := cc.pathMatcher.LookupRoute(rwr.Req.Method, reqPath) if matchedPath == nil { diff --git a/pkg/analyzer/coverage_queryparams.go b/pkg/analyzer/coverage_queryparams.go new file mode 100644 index 0000000..cbbc93d --- /dev/null +++ b/pkg/analyzer/coverage_queryparams.go @@ -0,0 +1,217 @@ +package analyzer + +import ( + "fmt" + "net/http" + "sort" + "strings" + + "github.com/go-openapi/spec" +) + +const ( + ParamInQuery string = "query" + ParamInPath string = "path" + ParamInHeader string = "header" + ParamInBody string = "body" + ParamInForm string = "form" +) + +// FullCoverageMinVariants defines the number of different values +// each type of param must have to be considered fully covered. +type FullCoverageMinVariants struct { + ParamInQuery int + ParamInPath int + ParamInHeader int + ParamInBody int + ParamInForm int +} + +// FullCoverageMinVariantsFn is the function to overwrite config defaults +type FullCoverageMinVariantsFn func(cc *FullCoverageMinVariants) + +// MinVariansFor return the number of different values required to +// consider a parameter fully covered +func (f *FullCoverageMinVariants) MinVariantsFor(paramIn string) int { + switch paramIn { + case ParamInQuery: + return f.ParamInQuery + case ParamInPath: + return f.ParamInPath + case ParamInHeader: + return f.ParamInHeader + case ParamInBody: + return f.ParamInBody + case ParamInForm: + return f.ParamInForm + } + return 0 +} + +// NewFullCoverageMinVariants creates a configuration to define what type +// of params to include in the output result, and the number +func NewFullCoverageMinVariants(configFns ...FullCoverageMinVariantsFn) FullCoverageMinVariants { + conf := FullCoverageMinVariants{ + ParamInQuery: 2, + ParamInPath: 2, + ParamInHeader: 2, + ParamInBody: 2, + } + for _, cf := range configFns { + cf(&conf) + } + return conf +} + +// ParamCoverage reports the coverage for a given parameter for an endpoint +type ParamCoverage struct { + Name string `json:"name"` + FullCoverageMinVariants int `json:"full_coverage_min_variants"` + CoveredValues []string `json:"covered_values"` + // EmptyAllowed bool `json:"empty_allowed"` + // EmptyCovered bool `json:"empty_covered"` +} + +// ParamsCoverage creates holds the report for the coverage +// of the query params of an endpoint +type ParamsCoverage struct { + Params []ParamCoverage `json:"params"` +} + +type paramValues struct { + coveredSet map[string]bool + minVariants int + emptyAllowed bool +} + +type ParamsCoverageChecker struct { + byPlaceAndName map[string]map[string]paramValues +} + +func (pcc *ParamsCoverageChecker) recordInQuery(pvals map[string]paramValues, + r *http.Request) { + if len(pvals) == 0 { + return + } + qvals := r.URL.Query() + for k, v := range qvals { + if _, ok := pvals[k]; ok { + pvals[k].coveredSet[flattenStringSlice(v)] = true + } + } +} + +func (pcc *ParamsCoverageChecker) recordInPath(pvals map[string]paramValues, + r *http.Request) { + // TODO +} + +func (pcc *ParamsCoverageChecker) recordInHeader(pvals map[string]paramValues, + r *http.Request) { + // TODO +} + +func (pcc *ParamsCoverageChecker) recordInBody(pvals map[string]paramValues, + r *http.Request) { + // TODO +} + +func (pcc *ParamsCoverageChecker) recordInForm(pvals map[string]paramValues, + r *http.Request) { + // TODO +} + +func (pcc *ParamsCoverageChecker) Record(r *http.Request) { + pcc.recordInQuery(pcc.byPlaceAndName[ParamInQuery], r) + pcc.recordInPath(pcc.byPlaceAndName[ParamInPath], r) + pcc.recordInHeader(pcc.byPlaceAndName[ParamInHeader], r) + pcc.recordInBody(pcc.byPlaceAndName[ParamInBody], r) + pcc.recordInForm(pcc.byPlaceAndName[ParamInForm], r) +} + +func (pcc *ParamsCoverageChecker) Report() *ParamsCoverage { + np := pcc.NumParams() + if np == 0 { + return &ParamsCoverage{} + } + params := make([]ParamCoverage, 0, np) + for _, namedParams := range pcc.byPlaceAndName { + for name, paramValues := range namedParams { + covered := make([]string, 0, len(paramValues.coveredSet)) + for cv := range paramValues.coveredSet { + covered = append(covered, cv) + } + params = append(params, ParamCoverage{ + Name: name, + CoveredValues: covered, + }) + } + } + return &ParamsCoverage{ + Params: params, + } +} + +func (pcc *ParamsCoverageChecker) NumParams() int { + cnt := 0 + for _, namedParams := range pcc.byPlaceAndName { + cnt += len(namedParams) + } + return cnt +} + +// NewParamsCoverageChecker +func NewParamsCoverageChecker(opSpec *spec.Operation, + covVariants *FullCoverageMinVariants) (*ParamsCoverageChecker, error) { + + if opSpec == nil { + return nil, fmt.Errorf("nil spec.Operation") + } + + pcc := &ParamsCoverageChecker{ + byPlaceAndName: make(map[string]map[string]paramValues), + } + + for _, param := range opSpec.Parameters { + var minCovVariants int + if len(param.Enum) > 0 { + minCovVariants = len(param.Enum) + } else { + minCovVariants = covVariants.MinVariantsFor(param.In) + } + if _, ok := pcc.byPlaceAndName[param.In]; !ok { + pcc.byPlaceAndName[param.In] = make(map[string]paramValues) + } + pcc.byPlaceAndName[param.In][param.Name] = paramValues{ + coveredSet: make(map[string]bool), + minVariants: minCovVariants, + emptyAllowed: !param.Required, + } + } + return pcc, nil +} + +// flattenStringSlice joins different values for a single parameter +// using ',' +func flattenStringSlice(strs []string) string { + if len(strs) == 0 { + return "" + } + if len(strs) == 1 { + return strs[0] + } + var l int + for _, s := range strs { + l += len(s) + 1 + } + + sort.Strings(strs) + + sb := strings.Builder{} + sb.Grow(l) + for _, s := range strs { + sb.WriteString(s) + sb.WriteString(",") + } + return sb.String() +} diff --git a/pkg/analyzer/coverage_queryparams_test.go b/pkg/analyzer/coverage_queryparams_test.go new file mode 100644 index 0000000..6cfb5d6 --- /dev/null +++ b/pkg/analyzer/coverage_queryparams_test.go @@ -0,0 +1,42 @@ +package analyzer_test + +import ( + "net/http" + "testing" + + "github.com/dhontecillas/liveapichecker/pkg/analyzer" + "github.com/go-openapi/loads" + "github.com/stretchr/testify/require" +) + +func TestCoverageQueryParams(t *testing.T) { + r := require.New(t) + + doc, err := loads.Analyzed([]byte(testSwagger), "2.0") + + r.Nil(err, "swagger file") + + spec := doc.OrigSpec() + r.NotNil(spec, "spec not nil") + r.NotNil(spec.Paths, "spec.Paths not nil") + + op := spec.Paths.Paths["/pets"].Get + r.NotNil(op, "cannot find endpoint to test") + + fcmv := analyzer.FullCoverageMinVariants{} + pcc, err := analyzer.NewParamsCoverageChecker(op, &fcmv) + r.Nil(err, "cannot create NewParamsCoverageChecker") + + nParams := pcc.NumParams() + r.Equal(nParams, 1, "want 2 params, got %d", nParams) + + req, _ := http.NewRequest("GET", "https://example.com/pets/?limit=5", nil) + pcc.Record(req) + + report := pcc.Report() + r.NotNil(report) + + r.Equal(len(report.Params), 1, "should have covered the limit param") + r.Equal(report.Params[0].Name, "limit") + r.Equal(report.Params[0].CoveredValues[0], "5") +} diff --git a/pkg/analyzer/openapiv2checker.go b/pkg/analyzer/openapiv2checker.go new file mode 100644 index 0000000..654fbd5 --- /dev/null +++ b/pkg/analyzer/openapiv2checker.go @@ -0,0 +1,16 @@ +package analyzer + +/* +TODO: add checking of openapi v2 conformance + + "github.com/go-openapi/analysis" + "github.com/go-openapi/errors" + "github.com/go-openapi/loads" + "github.com/go-openapi/spec" + "github.com/go-openapi/strfmt" + + "github.com/go-openapi/runtime" + "github.com/go-openapi/runtime/logger" + "github.com/go-openapi/runtime/middleware/untyped" + "github.com/go-openapi/runtime/security" +*/ From 6d6d2515bda5ffc8e15ee5dd3a0d04dc4aac4d7f Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Fri, 27 Aug 2021 19:42:23 +0200 Subject: [PATCH 2/3] refactor report coverage format --- pkg/analyzer/coverage.go | 127 +++++++++++++++++----------- pkg/analyzer/coverage_test.go | 38 +++++++++ pkg/proxy/respcloneprocessor.go | 10 +-- pkg/proxy/responsewriterrecorder.go | 3 +- 4 files changed, 123 insertions(+), 55 deletions(-) create mode 100644 pkg/analyzer/coverage_test.go diff --git a/pkg/analyzer/coverage.go b/pkg/analyzer/coverage.go index 8671df1..dcdee55 100644 --- a/pkg/analyzer/coverage.go +++ b/pkg/analyzer/coverage.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path" + "sort" "strings" "sync" @@ -14,6 +15,49 @@ import ( "github.com/go-openapi/spec" ) +// EndpointCoverage contains the information about +// how an endpoint has been covered +type EndpointCoverage struct { + Method string `json:"method"` + Path string `json:"path"` + StatusCodes []int `json:"statusCodes"` + UndocumentedStatusCodes []int `json:"undocumentedStatusCodes"` + Params *ParamsCoverage `json:"params"` +} + +type endpointCovTracker struct { + path string + statusCodes map[int]bool + undocumentedStatusCodes map[int]bool + params *ParamsCoverageChecker +} + +// newEndpointCoverage creates a new EndpointCoverage data +func newEndpointCoverageTracker(specPath string, opSpec *spec.Operation) *endpointCovTracker { + covVariants := NewFullCoverageMinVariants() + pcc, err := NewParamsCoverageChecker(opSpec, &covVariants) + if err != nil { + pcc = &ParamsCoverageChecker{} + } + return &endpointCovTracker{ + path: specPath, + statusCodes: make(map[int]bool), + undocumentedStatusCodes: make(map[int]bool), + params: pcc, + } +} + +// Report returns the EndpointCoverage +func (e *endpointCovTracker) Report(method string) *EndpointCoverage { + return &EndpointCoverage{ + Method: method, + Path: e.path, + StatusCodes: intSetToSlice(e.statusCodes), + UndocumentedStatusCodes: intSetToSlice(e.undocumentedStatusCodes), + Params: e.params.Report(), + } +} + // CoverageChecker uses an openapi specdoc and checks // recorded api calls to keep track of what endpoints // have been covered @@ -25,42 +69,12 @@ type CoverageChecker struct { basePath string // covered is a map of path -> method -> status code -> covered - covered map[string]map[string]*EndpointCoverage + covered map[string]map[string]*endpointCovTracker // allEndpoints []*EndpointCoverage // reportNonMatchedRequests bool } -// EndpointCoverage contains the information about -// how an endpoint has been covered -type EndpointCoverage struct { - Method string `json:"method"` - Path string `json:"path"` - StatusCodes map[int]bool `json:"statusCodes"` - UndocumentedStatusCodes map[int]bool `json:"undocumentedStatusCodes"` - Params *ParamsCoverage `json:"params"` -} - -// NewEndpointCoverage creates a new EndpontCoverage data -func NewEndpointCoverage(method string, path string, - opSpec *spec.Operation) *EndpointCoverage { - - /* - covVariants := NewFullCoverageMinVariants() - pcc, err := NewParamsCoverageChecker(method, path, opSpec, &covVariants) - if err != nil { - pc = &ParamsCoverage{} - } - */ - return &EndpointCoverage{ - Method: method, - Path: path, - StatusCodes: make(map[int]bool), - UndocumentedStatusCodes: make(map[int]bool), - Params: nil, - } -} - // NewCoverageChecker creates a new CoverageChecker func NewCoverageChecker(specDoc *loads.Document) *CoverageChecker { bp := path.Clean(specDoc.BasePath()) @@ -72,22 +86,22 @@ func NewCoverageChecker(specDoc *loads.Document) *CoverageChecker { bp = "/" } - covered := make(map[string]map[string]*EndpointCoverage) + covered := make(map[string]map[string]*endpointCovTracker) pMatcher := pathmatcher.NewPathMatcher() ops := specDoc.Analyzer.Operations() for method, mops := range ops { - for rePath, def := range mops { + for specPath, def := range mops { mU := strings.ToUpper(method) - routePath := path.Join(bp, rePath) - ec := NewEndpointCoverage(mU, rePath, def) + routePath := path.Join(bp, specPath) + ec := newEndpointCoverageTracker(specPath, def) pMatcher.AddRoute(method, routePath) if def.Responses != nil { for v := range def.Responses.StatusCodeResponses { - ec.StatusCodes[v] = false + ec.statusCodes[v] = false } } if _, ok := covered[routePath]; !ok { - covered[routePath] = make(map[string]*EndpointCoverage) + covered[routePath] = make(map[string]*endpointCovTracker) } covered[routePath][mU] = ec } @@ -115,13 +129,25 @@ func (cc *CoverageChecker) ProcessRecordedResponse(rwr *proxy.ResponseWriterReco cc.rwMutex.Lock() defer cc.rwMutex.Unlock() e := cc.covered[matchedPath.Path][matchedPath.Method] - if _, ok := e.StatusCodes[rwr.StatusCode]; ok { - e.StatusCodes[rwr.StatusCode] = true + if _, ok := e.statusCodes[rwr.StatusCode]; ok { + e.statusCodes[rwr.StatusCode] = true } else { - e.UndocumentedStatusCodes[rwr.StatusCode] = true + e.undocumentedStatusCodes[rwr.StatusCode] = true } } +func (cc *CoverageChecker) Report() []*EndpointCoverage { + cc.rwMutex.RLock() + eps := make([]*EndpointCoverage, len(cc.covered)) + for method, pathMap := range cc.covered { + for _, covTrack := range pathMap { + eps = append(eps, covTrack.Report(method)) + } + } + cc.rwMutex.RUnlock() + return eps +} + // DumpResultsToJsonString returns a report of the coverage as // a JSON serialized string func (cc *CoverageChecker) DumpResultsToJSONString() (string, error) { @@ -129,14 +155,7 @@ func (cc *CoverageChecker) DumpResultsToJSONString() (string, error) { Endpoints []*EndpointCoverage `json:"endpoints"` } var r report - - cc.rwMutex.RLock() - for _, k := range cc.covered { - for _, j := range k { - r.Endpoints = append(r.Endpoints, j) - } - } - cc.rwMutex.RUnlock() + r.Endpoints = cc.Report() res, err := json.Marshal(r) if err != nil { @@ -174,3 +193,15 @@ func (cc *CoverageChecker) DumpResultsToFile(fileWithPath string) { } f.WriteString(string(res)) } + +func intSetToSlice(im map[int]bool) []int { + if len(im) == 0 { + return []int{} + } + is := make([]int, 0, len(im)) + for k := range im { + is = append(is, k) + } + sort.Ints(is) + return is +} diff --git a/pkg/analyzer/coverage_test.go b/pkg/analyzer/coverage_test.go new file mode 100644 index 0000000..dd5f68c --- /dev/null +++ b/pkg/analyzer/coverage_test.go @@ -0,0 +1,38 @@ +package analyzer_test + +import ( + "net/http" + "testing" + + "github.com/dhontecillas/liveapichecker/pkg/analyzer" + "github.com/dhontecillas/liveapichecker/pkg/proxy" + "github.com/go-openapi/loads" + "github.com/stretchr/testify/require" +) + +func TestCoverage(t *testing.T) { + r := require.New(t) + doc, err := loads.Analyzed([]byte(testSwagger), "2.0") + r.Nil(err, "swagger file") + + req, _ := http.NewRequest("GET", "https://example.com/pets/?limit=5", nil) + rwr := proxy.NewResponseWriterRecorder(req) + rwr.StatusCode = 200 + + cc := analyzer.NewCoverageChecker(doc) + cc.ProcessRecordedResponse(rwr) + report := cc.Report() + + r.Equal(len(report), 1, "want 1 params, got %d", len(report)) + + /* + pcc.Record(req) + + report := pcc.Report() + r.NotNil(report) + + r.Equal(len(report.Params), 1, "should have covered the limit param") + r.Equal(report.Params[0].Name, "limit") + r.Equal(report.Params[0].CoveredValues[0], "5") + */ +} diff --git a/pkg/proxy/respcloneprocessor.go b/pkg/proxy/respcloneprocessor.go index 15e6b44..5ef4048 100644 --- a/pkg/proxy/respcloneprocessor.go +++ b/pkg/proxy/respcloneprocessor.go @@ -14,10 +14,9 @@ type RecordedResponseProcessorHandler interface { // ParallelHandler implements an http.Handler middleware // that wraps another http.Handler and a provided type ParallelHandler struct { - handler http.Handler - parallelProcRunning bool - parallelProc chan *ResponseWriterRecorder - respProcessor RecordedResponseProcessorHandler + handler http.Handler + parallelProc chan *ResponseWriterRecorder + respProcessor RecordedResponseProcessorHandler } // NewParallelHandler creaete @@ -35,7 +34,7 @@ func NewParallelHandler(h http.Handler, // uses the original response writer as the primary one. // Once served, the recorder is sent to the paraller processor. func (p *ParallelHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - recRW := NewResponseWriterRecorder(req, p.parallelProc) + recRW := NewResponseWriterRecorder(req) dupRW := NewDupResponseWriter(rw, recRW) p.handler.ServeHTTP(dupRW, req) @@ -58,6 +57,7 @@ func (p *ParallelHandler) LaunchParallelProc() { select { case r := <-p.parallelProc: p.respProcessor.ProcessRecordedResponse(r) + // TODO: put here the signal to shutdown the parallelProc goroutine } } }() diff --git a/pkg/proxy/responsewriterrecorder.go b/pkg/proxy/responsewriterrecorder.go index f8739ac..fb7faac 100644 --- a/pkg/proxy/responsewriterrecorder.go +++ b/pkg/proxy/responsewriterrecorder.go @@ -17,8 +17,7 @@ type ResponseWriterRecorder struct { } // NewResponseWriterRecorder creates new ResponseWriterRecorder -func NewResponseWriterRecorder(req *http.Request, - onWriteFinishedChan chan<- *ResponseWriterRecorder) *ResponseWriterRecorder { +func NewResponseWriterRecorder(req *http.Request) *ResponseWriterRecorder { // clones the request, so it can fiddle with it in a separate goroutine // when main processing completes clonedReq := req.Clone(req.Context()) From 405f31420952b11702c3be2638999b1056cb1f52 Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Sat, 28 Aug 2021 11:31:55 +0200 Subject: [PATCH 3/3] fix not using a ptr for the struct holding the slice --- pkg/analyzer/coverage.go | 7 ++++--- pkg/analyzer/coverage_queryparams.go | 18 ++++++++--------- pkg/analyzer/coverage_test.go | 30 ++++++++++++++++++---------- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/pkg/analyzer/coverage.go b/pkg/analyzer/coverage.go index dcdee55..2cf0cd0 100644 --- a/pkg/analyzer/coverage.go +++ b/pkg/analyzer/coverage.go @@ -134,14 +134,15 @@ func (cc *CoverageChecker) ProcessRecordedResponse(rwr *proxy.ResponseWriterReco } else { e.undocumentedStatusCodes[rwr.StatusCode] = true } + e.params.Record(rwr.Req) } func (cc *CoverageChecker) Report() []*EndpointCoverage { cc.rwMutex.RLock() - eps := make([]*EndpointCoverage, len(cc.covered)) + eps := make([]*EndpointCoverage, 0, len(cc.covered)) for method, pathMap := range cc.covered { - for _, covTrack := range pathMap { - eps = append(eps, covTrack.Report(method)) + for path := range pathMap { + eps = append(eps, pathMap[path].Report(method)) } } cc.rwMutex.RUnlock() diff --git a/pkg/analyzer/coverage_queryparams.go b/pkg/analyzer/coverage_queryparams.go index cbbc93d..5e87df9 100644 --- a/pkg/analyzer/coverage_queryparams.go +++ b/pkg/analyzer/coverage_queryparams.go @@ -85,10 +85,10 @@ type paramValues struct { } type ParamsCoverageChecker struct { - byPlaceAndName map[string]map[string]paramValues + byPlaceAndName map[string]map[string]*paramValues } -func (pcc *ParamsCoverageChecker) recordInQuery(pvals map[string]paramValues, +func (pcc *ParamsCoverageChecker) recordInQuery(pvals map[string]*paramValues, r *http.Request) { if len(pvals) == 0 { return @@ -101,22 +101,22 @@ func (pcc *ParamsCoverageChecker) recordInQuery(pvals map[string]paramValues, } } -func (pcc *ParamsCoverageChecker) recordInPath(pvals map[string]paramValues, +func (pcc *ParamsCoverageChecker) recordInPath(pvals map[string]*paramValues, r *http.Request) { // TODO } -func (pcc *ParamsCoverageChecker) recordInHeader(pvals map[string]paramValues, +func (pcc *ParamsCoverageChecker) recordInHeader(pvals map[string]*paramValues, r *http.Request) { // TODO } -func (pcc *ParamsCoverageChecker) recordInBody(pvals map[string]paramValues, +func (pcc *ParamsCoverageChecker) recordInBody(pvals map[string]*paramValues, r *http.Request) { // TODO } -func (pcc *ParamsCoverageChecker) recordInForm(pvals map[string]paramValues, +func (pcc *ParamsCoverageChecker) recordInForm(pvals map[string]*paramValues, r *http.Request) { // TODO } @@ -169,7 +169,7 @@ func NewParamsCoverageChecker(opSpec *spec.Operation, } pcc := &ParamsCoverageChecker{ - byPlaceAndName: make(map[string]map[string]paramValues), + byPlaceAndName: make(map[string]map[string]*paramValues), } for _, param := range opSpec.Parameters { @@ -180,9 +180,9 @@ func NewParamsCoverageChecker(opSpec *spec.Operation, minCovVariants = covVariants.MinVariantsFor(param.In) } if _, ok := pcc.byPlaceAndName[param.In]; !ok { - pcc.byPlaceAndName[param.In] = make(map[string]paramValues) + pcc.byPlaceAndName[param.In] = make(map[string]*paramValues) } - pcc.byPlaceAndName[param.In][param.Name] = paramValues{ + pcc.byPlaceAndName[param.In][param.Name] = ¶mValues{ coveredSet: make(map[string]bool), minVariants: minCovVariants, emptyAllowed: !param.Required, diff --git a/pkg/analyzer/coverage_test.go b/pkg/analyzer/coverage_test.go index dd5f68c..2032a6d 100644 --- a/pkg/analyzer/coverage_test.go +++ b/pkg/analyzer/coverage_test.go @@ -10,12 +10,12 @@ import ( "github.com/stretchr/testify/require" ) -func TestCoverage(t *testing.T) { +func TestCoverageChecker(t *testing.T) { r := require.New(t) doc, err := loads.Analyzed([]byte(testSwagger), "2.0") r.Nil(err, "swagger file") - req, _ := http.NewRequest("GET", "https://example.com/pets/?limit=5", nil) + req, _ := http.NewRequest("GET", "https://petstore.swagger.io/v1/pets/?limit=5", nil) rwr := proxy.NewResponseWriterRecorder(req) rwr.StatusCode = 200 @@ -23,16 +23,24 @@ func TestCoverage(t *testing.T) { cc.ProcessRecordedResponse(rwr) report := cc.Report() - r.Equal(len(report), 1, "want 1 params, got %d", len(report)) + var getPetsEP *analyzer.EndpointCoverage = nil + for _, rep := range report { + if rep.Path == "/pets" { + getPetsEP = rep + break + } + } - /* - pcc.Record(req) + r.NotNil(getPetsEP) - report := pcc.Report() - r.NotNil(report) + nSC := len(getPetsEP.StatusCodes) + r.Equal(nSC, 1, "want 1 status code, got %d", nSC) - r.Equal(len(report.Params), 1, "should have covered the limit param") - r.Equal(report.Params[0].Name, "limit") - r.Equal(report.Params[0].CoveredValues[0], "5") - */ + r.NotNil(getPetsEP.Params) + params := getPetsEP.Params.Params + + r.Equal(len(params), 1, "should have covered the limit param") + r.Equal(params[0].Name, "limit") + r.Equal(len(params[0].CoveredValues), 1, "params %#v", params) + r.Equal(params[0].CoveredValues[0], "5") }