Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions index/rolodex_file_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,7 @@ func (l *LocalFS) OpenWithContext(ctx context.Context, name string) (fs.File, er
copiedCfg.Rolodex = l.rolodex
}

// Add this file to the context's indexing set to prevent deadlocks
// when circular references cause the same file to be looked up recursively.
indexingCtx := AddIndexingFile(ctx, name)

idx, _ := extractedFile.IndexWithContext(indexingCtx, &copiedCfg)
idx, _ := extractedFile.IndexWithContext(ctx, &copiedCfg)

if idx != nil {
NewResolver(idx)
Expand Down
103 changes: 103 additions & 0 deletions index/rolodex_nested_files_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package index

import (
"context"
"fmt"
"io"
"net/http"
"strconv"
"strings"
"sync/atomic"
"testing"
"time"

"github.com/pb33f/go-yaml"
"github.com/pb33f/testify/assert"
"github.com/pb33f/testify/require"
)

const nestedRemoteFilesBase = "https://nested.test/"

func nestedRemoteFileRef(n int) string {
return fmt.Sprintf("components:\n schemas:\n A:\n $ref: '%sf%d.yaml#/components/schemas/A'\n", nestedRemoteFilesBase, n)
}

// indexNestedRemoteFiles indexes a root document followed by a chain of remote files f1 to
// fDepth, where each file references the next, so each one is indexed while the ones before
// it still are. It returns the number of files fetched and the result of indexing.
func indexNestedRemoteFiles(depth int, lastFile string) (int64, error) {
var fetched atomic.Int64
cfg := CreateOpenAPIIndexConfig()
cfg.RemoteURLHandler = func(url string) (*http.Response, error) {
name := strings.TrimSuffix(strings.TrimPrefix(url, nestedRemoteFilesBase+"f"), ".yaml")
n, err := strconv.Atoi(name)
if err != nil {
return nil, fmt.Errorf("unexpected url %q", url)
}
fetched.Add(1)
doc := lastFile
if n < depth {
doc = nestedRemoteFileRef(n + 1)
}
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(doc)),
}, nil
}

var root yaml.Node
if err := yaml.Unmarshal([]byte(nestedRemoteFileRef(1)), &root); err != nil {
return 0, err
}

rolodex := NewRolodex(cfg)
remoteFS, err := NewRemoteFSWithConfig(cfg)
if err != nil {
return 0, err
}
rolodex.AddRemoteFS(nestedRemoteFilesBase, remoteFS)
rolodex.SetRootNode(&root)
err = rolodex.IndexTheRolodex(context.Background())
return fetched.Load(), err
}

const nestedRemoteLeaf = "components:\n schemas:\n A:\n type: string\n"

func TestRolodex_NestedRemoteFiles_IndexesEveryFile(t *testing.T) {
fetched, err := indexNestedRemoteFiles(50, nestedRemoteLeaf)
require.NoError(t, err)
assert.Equal(t, int64(50), fetched)
}

func TestRolodex_CircularRemoteFiles_DoNotDeadlock(t *testing.T) {
done := make(chan int64, 1)
go func() {
// The result of indexing depends on circular reference handling; only finishing matters here.
fetched, _ := indexNestedRemoteFiles(3, nestedRemoteFileRef(1))
done <- fetched
}()

select {
case fetched := <-done:
assert.Equal(t, int64(3), fetched)
case <-time.After(30 * time.Second):
t.Fatal("indexing circular remote files did not finish")
}
}

func BenchmarkRolodex_NestedRemoteFiles(b *testing.B) {
for _, depth := range []int{10, 100, 500} {
b.Run(fmt.Sprintf("depth=%d", depth), func(b *testing.B) {
b.ReportAllocs()
for b.Loop() {
fetched, err := indexNestedRemoteFiles(depth, nestedRemoteLeaf)
if err != nil {
b.Fatal(err)
}
if fetched != int64(depth) {
b.Fatalf("fetched %d files, want %d", fetched, depth)
}
}
})
}
}
3 changes: 1 addition & 2 deletions index/rolodex_owned_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ func (r *Rolodex) ownIndexedFile(source RolodexFile) (*LocalFile, error) {
cfg.SpecAbsolutePath = location
cfg.SpecInfo = info
cfg.AvoidBuildIndex = true
ctx := AddIndexingFile(context.Background(), location)
idx := NewSpecIndexWithConfigAndContext(ctx, info.RootNode, &cfg)
idx := NewSpecIndexWithConfigAndContext(context.Background(), info.RootNode, &cfg)
resolver := NewResolver(idx)
resolver.IgnoreArray = cfg.IgnoreArrayCircularReferences
resolver.IgnorePoly = cfg.IgnorePolymorphicCircularReferences
Expand Down
10 changes: 3 additions & 7 deletions index/rolodex_remote_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ func (i *RemoteFS) OpenWithContext(ctx context.Context, remoteURL string) (fs.Fi
i.Files.Store(cacheKey, remoteFile)
i.releaseRemoteProcessingWaiter(processingWaiter, cacheKey, remoteFile, nil)

i.indexRemoteFile(ctx, remoteFile, copiedCfg, remoteParsedURL, remoteParsedURLOriginal)
i.indexRemoteFile(ctx, remoteFile, copiedCfg, remoteParsedURL)

return remoteFile, errors.Join(i.remoteErrors...)
}
Expand Down Expand Up @@ -800,13 +800,9 @@ func (i *RemoteFS) indexRemoteFile(
ctx context.Context,
remoteFile *RemoteFile,
copiedCfg *SpecIndexConfig,
remoteParsedURL, remoteParsedURLOriginal *url.URL,
remoteParsedURL *url.URL,
) {
indexingCtx := AddIndexingFile(ctx, remoteParsedURL.Path)
indexingCtx = AddIndexingFile(indexingCtx, remoteParsedURL.String())
indexingCtx = AddIndexingFile(indexingCtx, remoteParsedURLOriginal.String())

idx, idxError := remoteFile.Index(indexingCtx, copiedCfg)
idx, idxError := remoteFile.Index(ctx, copiedCfg)
if idxError != nil && idx == nil {
i.appendRemoteError(idxError)
remoteFile.signalIndexingComplete()
Expand Down
19 changes: 15 additions & 4 deletions index/search_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,20 @@ import (
type ContextKey string

const (
CurrentPathKey ContextKey = "currentPath"
FoundIndexKey ContextKey = "foundIndex"
RootIndexKey ContextKey = "currentIndex"
IndexingFilesKey ContextKey = "indexingFiles" // Tracks files being indexed in current call chain
CurrentPathKey ContextKey = "currentPath"
FoundIndexKey ContextKey = "foundIndex"
RootIndexKey ContextKey = "currentIndex"
// IndexingFilesKey holds the set used by AddIndexingFile and GetIndexingFiles.
//
// Deprecated: the library no longer tracks the files it is indexing.
IndexingFilesKey ContextKey = "indexingFiles"
)

// GetIndexingFiles returns the set of files currently being indexed in the call chain.
// Returns nil if not set.
//
// Deprecated: the library no longer tracks the files it is indexing, so this only
// returns what the caller added with AddIndexingFile.
func GetIndexingFiles(ctx context.Context) map[string]bool {
if v := ctx.Value(IndexingFilesKey); v != nil {
return v.(map[string]bool)
Expand All @@ -31,6 +37,8 @@ func GetIndexingFiles(ctx context.Context) map[string]bool {

// AddIndexingFile adds a file to the indexing set in the context.
// Returns a new context with the updated set.
//
// Deprecated: the library no longer reads this set when indexing.
func AddIndexingFile(ctx context.Context, filePath string) context.Context {
existing := GetIndexingFiles(ctx)
newSet := make(map[string]bool)
Expand All @@ -45,6 +53,9 @@ func AddIndexingFile(ctx context.Context, filePath string) context.Context {
// For HTTP URLs, it also checks if the PATH portion matches any indexed file,
// since the same file might be referenced with different hostnames (which get normalized
// to a common server).
//
// Deprecated: the library no longer tracks the files it is indexing, so this only
// sees what the caller added with AddIndexingFile.
func IsFileBeingIndexed(ctx context.Context, filePath string) bool {
files := GetIndexingFiles(ctx)
if files == nil {
Expand Down
10 changes: 10 additions & 0 deletions index/search_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,16 @@ func TestIsFileBeingIndexed_HTTPPathMatch(t *testing.T) {
assert.False(t, IsFileBeingIndexed(ctx, "https://example.com/other/file.yaml"))
}

func TestAddIndexingFile_ExtendsTheSetOfTheChildContextOnly(t *testing.T) {
parent := AddIndexingFile(context.Background(), "/tmp/specs/pet.yaml")
child := AddIndexingFile(parent, "/tmp/specs/cat.yaml")

assert.Equal(t, map[string]bool{"/tmp/specs/pet.yaml": true}, GetIndexingFiles(parent))
assert.Equal(t, map[string]bool{"/tmp/specs/pet.yaml": true, "/tmp/specs/cat.yaml": true}, GetIndexingFiles(child))
assert.True(t, IsFileBeingIndexed(child, "/tmp/specs/cat.yaml"))
assert.False(t, IsFileBeingIndexed(parent, "/tmp/specs/cat.yaml"))
}

func TestIsFileBeingIndexed_HTTPMatchesLocalFilename(t *testing.T) {
ctx := context.Background()

Expand Down
Loading