fix: handle empty targetLocation in reference resolution for in-memory documents (#42)

Author: TristanSpeakEasy

jsonschema/oas3/resolution_test.go

@@ -670,27 +670,6 @@ $ref: "circular1.yaml"
 func TestJSONSchema_Resolve_Errors(t *testing.T) {
 	t.Parallel()
 
-	t.Run("missing root location", func(t *testing.T) {
-		t.Parallel()
-		ref := "external.yaml#/test" // Use external reference to trigger location validation
-		schema := createSchemaWithRef(ref)
-
-		opts := ResolveOptions{
-			RootDocument: NewMockResolutionTarget(),
-			// TargetLocation deliberately omitted to trigger the error
-		}
-
-		validationErrs, err := schema.Resolve(t.Context(), opts)
-
-		require.Error(t, err)
-		assert.Nil(t, validationErrs)
-		// The error can be either "target location is required" or "empty reference" depending on the implementation
-		assert.True(t,
-			strings.Contains(err.Error(), "target location is required") ||
-				strings.Contains(err.Error(), "empty reference"),
-			"Expected error about target location or empty reference, got: %s", err.Error())
-	})
-
 	t.Run("missing root document", func(t *testing.T) {
 		t.Parallel()
 		ref := "#/test"

references/resolution.go

@@ -80,7 +80,7 @@ func Resolve[T any](ctx context.Context, ref Reference, unmarshaler Unmarshal[T]
 		return nil, nil, errors.New("root document is required")
 	}
 	if opts.TargetLocation == "" {
-		return nil, nil, errors.New("target location is required")
+		opts.TargetLocation = "."
 	}
 	if opts.TargetDocument == nil {
 		return nil, nil, errors.New("target document is required")

references/resolution_cache.go

@@ -64,23 +64,25 @@ func (c *RefCache) Resolve(ref Reference, targetLocation string) (*AbsoluteRefer
 func resolveAbsoluteReferenceUncached(ref Reference, targetLocation string) (*AbsoluteReferenceResult, error) {
 	uri := ref.GetURI()
 
+	// Handle empty targetLocation by treating it as current directory
+	if targetLocation == "" {
+		targetLocation = "."
+	}
+
+	// Classify the target location once
+	classification, err := utils.ClassifyReference(targetLocation)
+	if err != nil {
+		return nil, err
+	}
+
 	// If the reference is empty, it's relative to the target document
 	if uri == "" {
-		classification, err := utils.ClassifyReference(targetLocation)
-		if err != nil {
-			return nil, err
-		}
 		return &AbsoluteReferenceResult{
 			AbsoluteReference: targetLocation,
 			Classification:    classification,
 		}, nil
 	}
 
-	classification, err := utils.ClassifyReference(targetLocation)
-	if err != nil {
-		return nil, err
-	}
-
 	// Check if the URI is already absolute - if so, use it as-is instead of joining
 	var absRef string
 	var finalClassification *utils.ReferenceClassification

references/resolution_cache_test.go

@@ -287,3 +287,206 @@ func BenchmarkRefCache_vs_Uncached(b *testing.B) {
 		}
 	})
 }
+
+// Tests for empty targetLocation bug fix
+func TestResolveAbsoluteReferenceUncached_EmptyTargetLocation_Success(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name                string
+		targetLocation      string
+		reference           string
+		expectedAbsoluteRef string
+		description         string
+	}{
+		{
+			name:                "empty_target_with_empty_reference",
+			targetLocation:      "",
+			reference:           "",
+			expectedAbsoluteRef: ".",
+			description:         "Empty target location should default to current directory",
+		},
+		{
+			name:                "empty_target_with_relative_reference",
+			targetLocation:      "",
+			reference:           "schemas/user.yaml",
+			expectedAbsoluteRef: "schemas/user.yaml",
+			description:         "Empty target location with relative reference should resolve relative to current directory",
+		},
+		{
+			name:                "empty_target_with_absolute_file_reference",
+			targetLocation:      "",
+			reference:           "/absolute/path/schema.yaml",
+			expectedAbsoluteRef: "/absolute/path/schema.yaml",
+			description:         "Empty target location with absolute file reference should use absolute path as-is",
+		},
+		{
+			name:                "empty_target_with_absolute_url_reference",
+			targetLocation:      "",
+			reference:           "https://example.com/schema.yaml",
+			expectedAbsoluteRef: "https://example.com/schema.yaml",
+			description:         "Empty target location with absolute URL reference should use URL as-is",
+		},
+		{
+			name:                "empty_target_with_fragment_reference",
+			targetLocation:      "",
+			reference:           "#/components/schemas/User",
+			expectedAbsoluteRef: ".",
+			description:         "Empty target location with fragment reference should resolve to current directory",
+		},
+		{
+			name:                "empty_target_with_uri_and_fragment_reference",
+			targetLocation:      "",
+			reference:           "schemas/user.yaml#/User",
+			expectedAbsoluteRef: "schemas/user.yaml",
+			description:         "Empty target location with URI and fragment should resolve URI relative to current directory",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ref := Reference(tt.reference)
+			result, err := resolveAbsoluteReferenceUncached(ref, tt.targetLocation)
+
+			require.NoError(t, err, tt.description)
+			require.NotNil(t, result)
+			assert.Equal(t, tt.expectedAbsoluteRef, result.AbsoluteReference, tt.description)
+			assert.NotNil(t, result.Classification, "Classification should not be nil")
+		})
+	}
+}
+
+func TestRefCache_Resolve_EmptyTargetLocation_Success(t *testing.T) {
+	t.Parallel()
+	cache := &RefCache{}
+
+	t.Run("empty_target_location_cached_correctly", func(t *testing.T) {
+		t.Parallel()
+		ref := Reference("schemas/user.yaml")
+
+		// First call with empty target location
+		result1, err1 := cache.Resolve(ref, "")
+		require.NoError(t, err1)
+		require.NotNil(t, result1)
+		assert.Equal(t, "schemas/user.yaml", result1.AbsoluteReference)
+
+		// Second call with same parameters should return cached result
+		result2, err2 := cache.Resolve(ref, "")
+		require.NoError(t, err2)
+		require.NotNil(t, result2)
+		assert.Equal(t, result1.AbsoluteReference, result2.AbsoluteReference)
+		assert.Equal(t, result1.Classification.Type, result2.Classification.Type)
+
+		// Results should be copies (not the same instance) to prevent mutation
+		assert.NotSame(t, result1, result2, "Cached results should be copies to prevent mutation")
+		assert.Equal(t, result1.AbsoluteReference, result2.AbsoluteReference, "But content should be identical")
+	})
+
+	t.Run("empty_vs_dot_target_location_different_cache_keys", func(t *testing.T) {
+		t.Parallel()
+		ref := Reference("test.yaml")
+
+		// Call with empty target location (internally becomes ".")
+		result1, err1 := cache.Resolve(ref, "")
+		require.NoError(t, err1)
+		require.NotNil(t, result1)
+
+		// Call with explicit "." target location
+		result2, err2 := cache.Resolve(ref, ".")
+		require.NoError(t, err2)
+		require.NotNil(t, result2)
+
+		// Both should have the same absolute reference result
+		assert.Equal(t, result1.AbsoluteReference, result2.AbsoluteReference)
+		assert.Equal(t, result1.Classification.Type, result2.Classification.Type)
+
+		// But they should be cached separately (different cache keys)
+		// This is expected behavior since the cache key includes the original targetLocation
+		assert.NotSame(t, result1, result2, "Different target locations should have separate cache entries")
+	})
+}
+
+//nolint:paralleltest // This test uses global cache and cannot be parallel
+func TestResolveAbsoluteReferenceCached_EmptyTargetLocation_Global(t *testing.T) {
+	// Clear global cache before test
+	ClearGlobalRefCache()
+
+	ref := Reference("schemas/user.yaml")
+
+	// Resolve using global function with empty target location
+	result1, err := ResolveAbsoluteReferenceCached(ref, "")
+	require.NoError(t, err)
+	assert.Equal(t, "schemas/user.yaml", result1.AbsoluteReference)
+
+	// Verify it's cached globally
+	stats := GetRefCacheStats()
+	assert.Equal(t, int64(1), stats.Size)
+
+	// Resolve again - should use cache
+	result2, err := ResolveAbsoluteReferenceCached(ref, "")
+	require.NoError(t, err)
+	assert.Equal(t, result1.AbsoluteReference, result2.AbsoluteReference)
+	assert.NotSame(t, result1, result2, "should return copies")
+
+	// Clean up
+	ClearGlobalRefCache()
+}
+
+func TestResolveAbsoluteReferenceUncached_EmptyTargetLocation_InMemoryDocuments(t *testing.T) {
+	t.Parallel()
+
+	// Test scenarios that represent in-memory documents (uploaded files, database content, etc.)
+	tests := []struct {
+		name        string
+		reference   string
+		expected    string
+		description string
+	}{
+		{
+			name:        "in_memory_document_self_reference",
+			reference:   "",
+			expected:    ".",
+			description: "In-memory document referencing itself should resolve to current directory",
+		},
+		{
+			name:        "in_memory_document_relative_schema",
+			reference:   "components/schemas/User.yaml",
+			expected:    "components/schemas/User.yaml",
+			description: "In-memory document with relative schema reference",
+		},
+		{
+			name:        "in_memory_document_json_pointer",
+			reference:   "#/components/schemas/User",
+			expected:    ".",
+			description: "In-memory document with JSON pointer reference",
+		},
+		{
+			name:        "in_memory_document_external_url",
+			reference:   "https://schemas.example.com/common.yaml",
+			expected:    "https://schemas.example.com/common.yaml",
+			description: "In-memory document referencing external URL should preserve absolute URL",
+		},
+		{
+			name:        "in_memory_document_uri_with_fragment",
+			reference:   "components/schemas/User.yaml#/properties/name",
+			expected:    "components/schemas/User.yaml",
+			description: "In-memory document with URI and fragment should resolve to the URI part only",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ref := Reference(tt.reference)
+			result, err := resolveAbsoluteReferenceUncached(ref, "")
+
+			require.NoError(t, err, tt.description)
+			require.NotNil(t, result)
+			assert.Equal(t, tt.expected, result.AbsoluteReference, tt.description)
+			assert.NotNil(t, result.Classification, "Classification should not be nil")
+		})
+	}
+}

references/resolution_test.go

@@ -377,18 +377,22 @@ func TestResolve_Caching(t *testing.T) {
 func TestResolve_Errors(t *testing.T) {
 	t.Parallel()
 
-	t.Run("missing root location", func(t *testing.T) {
+	t.Run("empty target location defaults to current directory", func(t *testing.T) {
 		t.Parallel()
+		root := NewMockResolutionTarget()
 		opts := ResolveOptions{
-			RootDocument: NewMockResolutionTarget(),
+			RootDocument:   root,
+			TargetDocument: root,
 		}
 
-		result, validationErrs, err := Resolve(t.Context(), Reference("#/test"), testPrimitiveUnmarshaler, opts)
+		result, validationErrs, err := Resolve(t.Context(), Reference(""), func(ctx context.Context, node *yaml.Node, skipValidation bool) (*MockResolutionTarget, []error, error) {
+			return root, nil, nil
+		}, opts)
 
-		require.Error(t, err)
+		require.NoError(t, err)
 		assert.Nil(t, validationErrs)
-		assert.Nil(t, result)
-		assert.Contains(t, err.Error(), "target location is required")
+		require.NotNil(t, result)
+		assert.Equal(t, ".", result.AbsoluteReference)
 	})
 
 	t.Run("missing root document", func(t *testing.T) {