Skip to content

Commit

Permalink
Remove lib state (go-openapi#134)
Browse files Browse the repository at this point in the history
* Removed the global state of the resolution cache

* the cache at the package level is only used for static entries (json schema, swagger schema)
* all subsequent utilizations of this cache are taken from a shallow clone of this initial cache
  which will therefore not mutate

Signed-off-by: Frederic BIDON <[email protected]>

* Added PathLoader to ExpandOptions

This allows to bypass package level settings.

In particular, go-openapi/loads can be changed to remove the subtle race conditions
when altering PathLoader at the package level from the Spec() function.

Signed-off-by: Frederic BIDON <[email protected]>

* * adapted package tests to avoid switching the package level variable
* ran tests with go-openapi/validate and go-openapi/loads

Signed-off-by: Frederic BIDON <[email protected]>
  • Loading branch information
fredbi authored Dec 2, 2020
1 parent a490b92 commit 10b968f
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 145 deletions.
43 changes: 40 additions & 3 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

package spec

import "sync"
import (
"sync"
)

// ResolutionCache a cache for resolving urls
type ResolutionCache interface {
Expand All @@ -27,6 +29,19 @@ type simpleCache struct {
store map[string]interface{}
}

func (s *simpleCache) ShallowClone() ResolutionCache {
store := make(map[string]interface{}, len(s.store))
s.lock.RLock()
for k, v := range s.store {
store[k] = v
}
s.lock.RUnlock()

return &simpleCache{
store: store,
}
}

// Get retrieves a cached URI
func (s *simpleCache) Get(uri string) (interface{}, bool) {
debugLog("getting %q from resolution cache", uri)
Expand All @@ -46,18 +61,40 @@ func (s *simpleCache) Set(uri string, data interface{}) {
}

var (
resCache ResolutionCache
// resCache is a package level cache for $ref resolution and expansion.
// It is initialized lazily by methods that have the need for it: no
// memory is allocated unless some expander methods are called.
//
// It is initialized with JSON schema and swagger schema,
// which do not mutate during normal operations.
//
// All subsequent utilizations of this cache are produced from a shallow
// clone of this initial version.
resCache *simpleCache
onceCache sync.Once

_ ResolutionCache = &simpleCache{}
)

// initResolutionCache initializes the URI resolution cache. To be wrapped in a sync.Once.Do call.
func initResolutionCache() {
resCache = defaultResolutionCache()
}

func defaultResolutionCache() ResolutionCache {
func defaultResolutionCache() *simpleCache {
return &simpleCache{store: map[string]interface{}{
"http://swagger.io/v2/schema.json": MustLoadSwagger20Schema(),
"http://json-schema.org/draft-04/schema": MustLoadJSONSchemaDraft04(),
}}
}

func cacheOrDefault(cache ResolutionCache) ResolutionCache {
onceCache.Do(initResolutionCache)

if cache != nil {
return cache
}

// get a shallow clone of the base cache with swagger and json schema
return resCache.ShallowClone()
}
53 changes: 35 additions & 18 deletions expander.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type ExpandOptions struct {
SkipSchemas bool
ContinueOnError bool
AbsoluteCircularRef bool
PathLoader func(string) (json.RawMessage, error) `json:"-"`
}

// ResolveRefWithBase resolves a reference against a context root with preservation of base path
Expand Down Expand Up @@ -81,6 +82,7 @@ func ResolveParameterWithBase(root interface{}, ref Ref, opts *ExpandOptions) (*
if err != nil {
return nil, err
}

specBasePath := ""
if opts != nil && opts.RelativeBase != "" {
specBasePath, _ = absPath(opts.RelativeBase)
Expand Down Expand Up @@ -126,10 +128,12 @@ func ResolveItems(root interface{}, ref Ref, opts *ExpandOptions) (*Items, error
if err != nil {
return nil, err
}

basePath := ""
if opts.RelativeBase != "" {
basePath = opts.RelativeBase
}

result := new(Items)
if err := resolver.Resolve(&ref, result, basePath); err != nil {
return nil, err
Expand All @@ -143,10 +147,12 @@ func ResolvePathItem(root interface{}, ref Ref, opts *ExpandOptions) (*PathItem,
if err != nil {
return nil, err
}

basePath := ""
if opts.RelativeBase != "" {
basePath = opts.RelativeBase
}

result := new(PathItem)
if err := resolver.Resolve(&ref, result, basePath); err != nil {
return nil, err
Expand All @@ -157,7 +163,7 @@ func ResolvePathItem(root interface{}, ref Ref, opts *ExpandOptions) (*PathItem,
// ExpandSpec expands the references in a swagger spec
func ExpandSpec(spec *Swagger, options *ExpandOptions) error {
resolver, err := defaultSchemaLoader(spec, options, nil, nil)
// Just in case this ever returns an error.
// just in case this ever returns an error
if resolver.shouldStopOnError(err) {
return err
}
Expand Down Expand Up @@ -212,28 +218,31 @@ func ExpandSpec(spec *Swagger, options *ExpandOptions) error {

const rootBase = ".root"

// baseForRoot loads in the cache the root document and produces a fake "root" base path entry
// baseForRoot loads in the cache the root document and produces a fake ".root" base path entry
// for further $ref resolution
//
// Setting the cache is optional and this parameter may safely be left to nil.
func baseForRoot(root interface{}, cache ResolutionCache) string {
if root == nil {
return ""
}

// cache the root document to resolve $ref's
if root != nil {
base, _ := absPath(rootBase)
normalizedBase := normalizeAbsPath(base)
debugLog("setting root doc in cache at: %s", normalizedBase)
if cache == nil {
onceCache.Do(initResolutionCache)
cache = resCache
}
cache.Set(normalizedBase, root)
return normalizedBase
}
return ""
base, _ := absPath(rootBase)
normalizedBase := normalizeAbsPath(base)
debugLog("setting root doc in cache at: %s", normalizedBase)
cache.Set(normalizedBase, root)

return normalizedBase
}

// ExpandSchema expands the refs in the schema object with reference to the root object
// go-openapi/validate uses this function
// notice that it is impossible to reference a json schema in a different file other than root
//
// Setting the cache is optional and this parameter may safely be left to nil.
func ExpandSchema(schema *Schema, root interface{}, cache ResolutionCache) error {
cache = cacheOrDefault(cache)
opts := &ExpandOptions{
// when a root is specified, cache the root as an in-memory document for $ref retrieval
RelativeBase: baseForRoot(root, cache),
Expand All @@ -245,12 +254,16 @@ func ExpandSchema(schema *Schema, root interface{}, cache ResolutionCache) error
return ExpandSchemaWithBasePath(schema, cache, opts)
}

// ExpandSchemaWithBasePath expands the refs in the schema object, base path configured through expand options
// ExpandSchemaWithBasePath expands the refs in the schema object, base path configured through expand options.
//
// Setting the cache is optional and this parameter may safely be left to nil.
func ExpandSchemaWithBasePath(schema *Schema, cache ResolutionCache, opts *ExpandOptions) error {
if schema == nil {
return nil
}

cache = cacheOrDefault(cache)

var basePath string
if opts.RelativeBase != "" {
basePath, _ = absPath(opts.RelativeBase)
Expand Down Expand Up @@ -314,7 +327,7 @@ func expandSchema(target Schema, parentRefs []string, resolver *schemaLoader, ba
basePath = normalizePaths(refPath, basePath)

// store found IDs for possible future reuse in $ref
resCache.Set(basePath, target)
resolver.cache.Set(basePath, target)
}

var t *Schema
Expand Down Expand Up @@ -529,15 +542,18 @@ func expandOperation(op *Operation, resolver *schemaLoader, basePath string) err
}

// ExpandResponseWithRoot expands a response based on a root document, not a fetchable document
//
// Setting the cache is optional and this parameter may safely be left to nil.
func ExpandResponseWithRoot(response *Response, root interface{}, cache ResolutionCache) error {
cache = cacheOrDefault(cache)
opts := &ExpandOptions{
RelativeBase: baseForRoot(root, cache),
SkipSchemas: false,
ContinueOnError: false,
// when no base path is specified, remaining $ref (circular) are rendered with an absolute path
AbsoluteCircularRef: false,
}
resolver, err := defaultSchemaLoader(root, opts, nil, nil)
resolver, err := defaultSchemaLoader(root, opts, cache, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -566,14 +582,15 @@ func ExpandResponse(response *Response, basePath string) error {

// ExpandParameterWithRoot expands a parameter based on a root document, not a fetchable document
func ExpandParameterWithRoot(parameter *Parameter, root interface{}, cache ResolutionCache) error {
cache = cacheOrDefault(cache)
opts := &ExpandOptions{
RelativeBase: baseForRoot(root, cache),
SkipSchemas: false,
ContinueOnError: false,
// when no base path is specified, remaining $ref (circular) are rendered with an absolute path
AbsoluteCircularRef: false,
}
resolver, err := defaultSchemaLoader(root, opts, nil, nil)
resolver, err := defaultSchemaLoader(root, opts, cache, nil)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 10b968f

Please sign in to comment.