diff --git a/document_examples_test.go b/document_examples_test.go index 33d89868..52c7a12b 100644 --- a/document_examples_test.go +++ b/document_examples_test.go @@ -700,9 +700,12 @@ components: $ref: './models/product.yaml' customer: $ref: 'https://example.com/schemas/customer.yaml' + warehouse: + $ref: 'https://example.com/schemas/warehouse.yaml#/components/schemas/Warehouse' required: - id - product + - warehouse Pet: $ref: './models/pet.yaml' ErrorResponse: @@ -799,9 +802,21 @@ components: assert.Nil(t, customerProxy.Schema(), "customer.Schema() should be nil (external ref not resolved)") } + // The "warehouse" property is an external ref with a URL fragment (issue #519 core bug) + warehouseProxy := orderSchema.Properties.GetOrZero("warehouse") + assert.NotNil(t, warehouseProxy, "Order.properties.warehouse should exist") + if warehouseProxy != nil { + assert.True(t, warehouseProxy.IsReference(), "warehouse should report IsReference()=true") + assert.Equal(t, "https://example.com/schemas/warehouse.yaml#/components/schemas/Warehouse", + warehouseProxy.GetReference(), + "warehouse GetReference() should return the full URL with fragment") + assert.Nil(t, warehouseProxy.Schema(), "warehouse.Schema() should be nil (external ref not resolved)") + } + // Verify required fields are preserved assert.Contains(t, orderSchema.Required, "id") assert.Contains(t, orderSchema.Required, "product") + assert.Contains(t, orderSchema.Required, "warehouse") } // ---- Check the Pet schema (top-level external $ref) ---- diff --git a/index/extract_refs.go b/index/extract_refs.go index 1f83c238..e2b7efea 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -813,6 +813,17 @@ func (index *SpecIndex) ExtractRefs(ctx context.Context, node, parent *yaml.Node // This function uses singleflight to deduplicate concurrent lookups for the same reference, // channel-based collection to avoid mutex contention during resolution, and sorts results // by input position for deterministic ordering. + +// isExternalReference checks whether a Reference originated from an external $ref. +// ref.Definition may have been transformed (e.g., HTTP URL with fragment becomes "#/fragment"), +// so we also check the original raw ref value. +func isExternalReference(ref *Reference) bool { + if ref == nil { + return false + } + return utils.IsExternalRef(ref.Definition) || utils.IsExternalRef(ref.RawRef) +} + func (index *SpecIndex) ExtractComponentsFromRefs(ctx context.Context, refs []*Reference) []*Reference { if len(refs) == 0 { return nil @@ -841,7 +852,7 @@ func (index *SpecIndex) ExtractComponentsFromRefs(ctx context.Context, refs []*R index.refLock.Unlock() } else { // If SkipExternalRefResolution is enabled, don't record errors for external refs - if index.config != nil && index.config.SkipExternalRefResolution && utils.IsExternalRef(ref.Definition) { + if index.config != nil && index.config.SkipExternalRefResolution && isExternalReference(ref) { continue } // Record error for definitive failure @@ -964,7 +975,7 @@ func (index *SpecIndex) ExtractComponentsFromRefs(ctx context.Context, refs []*R index.refLock.Unlock() } else { // If SkipExternalRefResolution is enabled, don't record errors for external refs - if index.config != nil && index.config.SkipExternalRefResolution && utils.IsExternalRef(ref.Definition) { + if index.config != nil && index.config.SkipExternalRefResolution && isExternalReference(ref) { continue } // Definitive failure - record error diff --git a/index/extract_refs_test.go b/index/extract_refs_test.go index 2f2ba8d9..dcf89edf 100644 --- a/index/extract_refs_test.go +++ b/index/extract_refs_test.go @@ -707,3 +707,7 @@ components: assert.NotNil(t, idx) assert.Greater(t, len(idx.GetAllReferences()), 0) } + +func TestSpecIndex_isExternalReference_Nil(t *testing.T) { + assert.False(t, isExternalReference(nil)) +} diff --git a/index/spec_index_test.go b/index/spec_index_test.go index ad346a1f..2eb15e89 100644 --- a/index/spec_index_test.go +++ b/index/spec_index_test.go @@ -1124,6 +1124,193 @@ components: "should not record errors for external refs when SkipExternalRefResolution is enabled") } +// Tests for issue #519: external refs with URL fragments should not produce errors +// when SkipExternalRefResolution is enabled. The bug was that ref.Definition gets +// transformed to just the fragment (e.g., "#/components/schemas/Warehouse") which +// looks local, so we must also check ref.RawRef. + +func TestSpecIndex_ExtractComponentsFromRefs_SkipExternalRef_HTTPFragment_Sequential(t *testing.T) { + yml := `openapi: 3.0.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + exists: + type: string` + + var rootNode yaml.Node + _ = yaml.Unmarshal([]byte(yml), &rootNode) + + config := CreateOpenAPIIndexConfig() + config.ExtractRefsSequentially = true + config.SkipExternalRefResolution = true + index := NewSpecIndexWithConfig(&rootNode, config) + + ref := &Reference{ + FullDefinition: "https://example.com/schemas/warehouse.yaml#/components/schemas/Warehouse", + Definition: "#/components/schemas/Warehouse", + RawRef: "https://example.com/schemas/warehouse.yaml#/components/schemas/Warehouse", + } + + result := index.ExtractComponentsFromRefs(context.Background(), []*Reference{ref}) + assert.Empty(t, result) + assert.Len(t, index.GetReferenceIndexErrors(), 0, + "should not record errors for external HTTP refs with fragments when SkipExternalRefResolution is enabled") +} + +func TestSpecIndex_ExtractComponentsFromRefs_SkipExternalRef_HTTPFragment_Async(t *testing.T) { + yml := `openapi: 3.0.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + exists: + type: string` + + var rootNode yaml.Node + _ = yaml.Unmarshal([]byte(yml), &rootNode) + + config := CreateOpenAPIIndexConfig() + config.ExtractRefsSequentially = false + config.SkipExternalRefResolution = true + index := NewSpecIndexWithConfig(&rootNode, config) + + ref := &Reference{ + FullDefinition: "https://example.com/schemas/warehouse.yaml#/components/schemas/Warehouse", + Definition: "#/components/schemas/Warehouse", + RawRef: "https://example.com/schemas/warehouse.yaml#/components/schemas/Warehouse", + } + + result := index.ExtractComponentsFromRefs(context.Background(), []*Reference{ref}) + assert.Empty(t, result) + assert.Len(t, index.GetReferenceIndexErrors(), 0, + "should not record errors for external HTTP refs with fragments when SkipExternalRefResolution is enabled") +} + +func TestSpecIndex_ExtractComponentsFromRefs_SkipExternalRef_RelativeFileFragment_Sequential(t *testing.T) { + yml := `openapi: 3.0.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + exists: + type: string` + + var rootNode yaml.Node + _ = yaml.Unmarshal([]byte(yml), &rootNode) + + config := CreateOpenAPIIndexConfig() + config.ExtractRefsSequentially = true + config.SkipExternalRefResolution = true + index := NewSpecIndexWithConfig(&rootNode, config) + + ref := &Reference{ + FullDefinition: "/abs/path/models/product.yaml#/components/schemas/Product", + Definition: "#/components/schemas/Product", + RawRef: "./models/product.yaml#/components/schemas/Product", + } + + result := index.ExtractComponentsFromRefs(context.Background(), []*Reference{ref}) + assert.Empty(t, result) + assert.Len(t, index.GetReferenceIndexErrors(), 0, + "should not record errors for relative file refs with fragments when SkipExternalRefResolution is enabled") +} + +func TestSpecIndex_ExtractComponentsFromRefs_SkipExternalRef_RelativeFileFragment_Async(t *testing.T) { + yml := `openapi: 3.0.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + exists: + type: string` + + var rootNode yaml.Node + _ = yaml.Unmarshal([]byte(yml), &rootNode) + + config := CreateOpenAPIIndexConfig() + config.ExtractRefsSequentially = false + config.SkipExternalRefResolution = true + index := NewSpecIndexWithConfig(&rootNode, config) + + ref := &Reference{ + FullDefinition: "/abs/path/models/product.yaml#/components/schemas/Product", + Definition: "#/components/schemas/Product", + RawRef: "./models/product.yaml#/components/schemas/Product", + } + + result := index.ExtractComponentsFromRefs(context.Background(), []*Reference{ref}) + assert.Empty(t, result) + assert.Len(t, index.GetReferenceIndexErrors(), 0, + "should not record errors for relative file refs with fragments when SkipExternalRefResolution is enabled") +} + +func TestSpecIndex_ExtractComponentsFromRefs_LocalMissingRef_StillErrors_Sequential(t *testing.T) { + yml := `openapi: 3.0.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + exists: + type: string` + + var rootNode yaml.Node + _ = yaml.Unmarshal([]byte(yml), &rootNode) + + config := CreateOpenAPIIndexConfig() + config.ExtractRefsSequentially = true + config.SkipExternalRefResolution = true + index := NewSpecIndexWithConfig(&rootNode, config) + + // Genuinely local ref that doesn't exist — should still produce an error + ref := &Reference{ + FullDefinition: "/abs/path/spec.yaml#/components/schemas/DoesNotExist", + Definition: "#/components/schemas/DoesNotExist", + RawRef: "#/components/schemas/DoesNotExist", + } + + result := index.ExtractComponentsFromRefs(context.Background(), []*Reference{ref}) + assert.Empty(t, result) + assert.Len(t, index.GetReferenceIndexErrors(), 1, + "should still record errors for genuinely local refs that don't exist") +} + +func TestSpecIndex_ExtractComponentsFromRefs_LocalMissingRef_StillErrors_Async(t *testing.T) { + yml := `openapi: 3.0.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + exists: + type: string` + + var rootNode yaml.Node + _ = yaml.Unmarshal([]byte(yml), &rootNode) + + config := CreateOpenAPIIndexConfig() + config.ExtractRefsSequentially = false + config.SkipExternalRefResolution = true + index := NewSpecIndexWithConfig(&rootNode, config) + + // Genuinely local ref that doesn't exist — should still produce an error + ref := &Reference{ + FullDefinition: "/abs/path/spec.yaml#/components/schemas/DoesNotExist", + Definition: "#/components/schemas/DoesNotExist", + RawRef: "#/components/schemas/DoesNotExist", + } + + result := index.ExtractComponentsFromRefs(context.Background(), []*Reference{ref}) + assert.Empty(t, result) + assert.Len(t, index.GetReferenceIndexErrors(), 1, + "should still record errors for genuinely local refs that don't exist") +} + func TestSpecIndex_FindComponent_WithACrazyAssPath(t *testing.T) { yml := `paths: /crazy/ass/references: