From 62c9e3cd9054b26b0f758bcf503e3beda45ce710 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Fri, 6 Mar 2026 21:08:15 +0100 Subject: [PATCH 1/4] test(cache): add comprehensive unit tests for cache functions Add 48+ new unit tests covering critical cache functions that previously only had indirect E2E coverage: - validateItemHasRequiredData: 22 subtests for nullable/non-nullable combinations, nested objects, arrays, type mismatches - normalize/denormalize round-trip: 8 subtests with CacheArgs suffix handling and alias combinations - computeArgSuffix: 7 subtests for xxhash determinism and edge cases - mergeEntityFields: 6 subtests for field merging semantics - L2 error resilience: 3 subtests for error handling and graceful fallthrough - Mutation L2 skip: 1 subtest verifying mutations bypass L2 reads All tests pass with race detector enabled. Co-Authored-By: Claude Haiku 4.5 --- v2/pkg/engine/resolve/cache_load_test.go | 295 +++++++++ v2/pkg/engine/resolve/l1_cache_test.go | 792 +++++++++++++++++++++++ 2 files changed, 1087 insertions(+) diff --git a/v2/pkg/engine/resolve/cache_load_test.go b/v2/pkg/engine/resolve/cache_load_test.go index 6cf0cb8c62..e0db13a2a4 100644 --- a/v2/pkg/engine/resolve/cache_load_test.go +++ b/v2/pkg/engine/resolve/cache_load_test.go @@ -1976,6 +1976,301 @@ func TestShadowMode_WithoutAnalytics(t *testing.T) { }) } +// ErrorLoaderCache wraps FakeLoaderCache but returns errors on Get/Set calls +// when configured to do so. Used for testing L2 error resilience. +type ErrorLoaderCache struct { + *FakeLoaderCache + getErr error + setErr error +} + +func (e *ErrorLoaderCache) Get(ctx context.Context, keys []string) ([]*CacheEntry, error) { + if e.getErr != nil { + return nil, e.getErr + } + return e.FakeLoaderCache.Get(ctx, keys) +} + +func (e *ErrorLoaderCache) Set(ctx context.Context, entries []*CacheEntry, ttl time.Duration) error { + if e.setErr != nil { + return e.setErr + } + return e.FakeLoaderCache.Set(ctx, entries, ttl) +} + +// buildProductEntityResponse creates a GraphQLResponse for a single product entity fetch. +// Used by error resilience and mutation skip tests to avoid repeating boilerplate. +func buildProductEntityResponse(rootDS, entityDS DataSource, cacheKeyTemplate CacheKeyTemplate, providesData *Object, operationType ast.OperationType) *GraphQLResponse { + rootOpName := "query" + rootFieldType := "Query" + rootFieldName := "product" + if operationType == ast.OperationTypeMutation { + rootOpName = "mutation" + rootFieldType = "Mutation" + rootFieldName = "updateUser" + } + + return &GraphQLResponse{ + Info: &GraphQLResponseInfo{OperationType: operationType}, + Fetches: Sequence( + SingleWithPath(&SingleFetch{ + FetchConfiguration: FetchConfiguration{ + DataSource: rootDS, + PostProcessing: PostProcessingConfiguration{SelectResponseDataPath: []string{"data"}}, + }, + InputTemplate: InputTemplate{Segments: []TemplateSegment{ + {Data: []byte(`{"method":"POST"}`), SegmentType: StaticSegmentType}, + }}, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + Info: &FetchInfo{ + DataSourceID: "ds", DataSourceName: "ds", + RootFields: []GraphCoordinate{{TypeName: rootFieldType, FieldName: rootFieldName}}, + OperationType: operationType, + }, + }, rootOpName), + SingleWithPath(&SingleFetch{ + FetchConfiguration: FetchConfiguration{ + DataSource: entityDS, + PostProcessing: PostProcessingConfiguration{SelectResponseDataPath: []string{"data", "_entities", "0"}}, + Caching: FetchCacheConfiguration{ + Enabled: true, + CacheName: "default", + TTL: 30 * time.Second, + CacheKeyTemplate: cacheKeyTemplate, + UseL1Cache: true, + }, + }, + InputTemplate: InputTemplate{Segments: []TemplateSegment{ + {Data: []byte(`{"method":"POST","url":"http://ds.service","body":{"query":"...","variables":{"representations":[`), SegmentType: StaticSegmentType}, + {SegmentType: VariableSegmentType, VariableKind: ResolvableObjectVariableKind, Renderer: NewGraphQLVariableResolveRenderer(&Object{ + Fields: []*Field{ + {Name: []byte("__typename"), Value: &String{Path: []string{"__typename"}}}, + {Name: []byte("id"), Value: &String{Path: []string{"id"}}}, + }, + })}, + {Data: []byte(`]}}}`), SegmentType: StaticSegmentType}, + }}, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + Info: &FetchInfo{ + DataSourceID: "ds", DataSourceName: "ds", + RootFields: []GraphCoordinate{{TypeName: "Product", FieldName: "name"}}, + OperationType: ast.OperationTypeQuery, ProvidesData: providesData, + }, + }, rootOpName+"."+rootFieldName, ObjectPath(rootFieldName)), + ), + Data: &Object{ + Fields: []*Field{{ + Name: []byte(rootFieldName), + Value: &Object{ + Path: []string{rootFieldName}, + Fields: []*Field{ + {Name: []byte("id"), Value: &String{Path: []string{"id"}}}, + {Name: []byte("name"), Value: &String{Path: []string{"name"}}}, + }, + }, + }}, + }, + } +} + +func TestL2CacheErrorResilience(t *testing.T) { + productCacheKeyTemplate := &EntityQueryCacheKeyTemplate{ + Keys: NewResolvableObjectVariable(&Object{ + Fields: []*Field{ + {Name: []byte("__typename"), Value: &String{Path: []string{"__typename"}}}, + {Name: []byte("id"), Value: &String{Path: []string{"id"}}}, + }, + }), + } + providesData := &Object{ + Fields: []*Field{ + {Name: []byte("name"), Value: &Scalar{}}, + }, + } + + t.Run("L2 Get error falls through to fetch", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + errorCache := &ErrorLoaderCache{ + FakeLoaderCache: NewFakeLoaderCache(), + getErr: assert.AnError, + } + + rootDS := NewMockDataSource(ctrl) + rootDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"product":{"__typename":"Product","id":"prod-1"}}}`), nil + }).Times(1) + + entityDS := NewMockDataSource(ctrl) + entityDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"_entities":[{"__typename":"Product","id":"prod-1","name":"Product One"}]}}`), nil + }).Times(1) + + response := buildProductEntityResponse(rootDS, entityDS, productCacheKeyTemplate, providesData, ast.OperationTypeQuery) + + loader := &Loader{caches: map[string]LoaderCache{"default": errorCache}} + ctx := NewContext(t.Context()) + ctx.ExecutionOptions.DisableSubgraphRequestDeduplication = true + ctx.ExecutionOptions.Caching.EnableL1Cache = true + ctx.ExecutionOptions.Caching.EnableL2Cache = true + + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + resolvable := NewResolvable(ar, ResolvableOptions{}) + err := resolvable.Init(ctx, nil, ast.OperationTypeQuery) + require.NoError(t, err) + + err = loader.LoadGraphQLResponseData(ctx, response, resolvable) + require.NoError(t, err) + + out := fastjsonext.PrintGraphQLResponse(resolvable.data, resolvable.errors) + assert.Equal(t, `{"data":{"product":{"__typename":"Product","id":"prod-1","name":"Product One"}}}`, out) + }) + + t.Run("L2 Set error does not fail request", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + errorCache := &ErrorLoaderCache{ + FakeLoaderCache: NewFakeLoaderCache(), + setErr: assert.AnError, + } + + rootDS := NewMockDataSource(ctrl) + rootDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"product":{"__typename":"Product","id":"prod-1"}}}`), nil + }).Times(1) + + entityDS := NewMockDataSource(ctrl) + entityDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"_entities":[{"__typename":"Product","id":"prod-1","name":"Product One"}]}}`), nil + }).Times(1) + + response := buildProductEntityResponse(rootDS, entityDS, productCacheKeyTemplate, providesData, ast.OperationTypeQuery) + + loader := &Loader{caches: map[string]LoaderCache{"default": errorCache}} + ctx := NewContext(t.Context()) + ctx.ExecutionOptions.DisableSubgraphRequestDeduplication = true + ctx.ExecutionOptions.Caching.EnableL1Cache = true + ctx.ExecutionOptions.Caching.EnableL2Cache = true + + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + resolvable := NewResolvable(ar, ResolvableOptions{}) + err := resolvable.Init(ctx, nil, ast.OperationTypeQuery) + require.NoError(t, err) + + err = loader.LoadGraphQLResponseData(ctx, response, resolvable) + require.NoError(t, err) + + out := fastjsonext.PrintGraphQLResponse(resolvable.data, resolvable.errors) + assert.Equal(t, `{"data":{"product":{"__typename":"Product","id":"prod-1","name":"Product One"}}}`, out) + }) + + t.Run("corrupted cache entry treated as miss", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + cache := NewFakeLoaderCache() + // Pre-populate cache with corrupted JSON + _ = cache.Set(t.Context(), []*CacheEntry{ + {Key: "Product:prod-1", Value: []byte(`{not valid json!!!}`)}, + }, 30*time.Second) + + rootDS := NewMockDataSource(ctrl) + rootDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"product":{"__typename":"Product","id":"prod-1"}}}`), nil + }).Times(1) + + entityDS := NewMockDataSource(ctrl) + entityDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"_entities":[{"__typename":"Product","id":"prod-1","name":"Product One"}]}}`), nil + }).Times(1) // Must fetch because cached entry is corrupted + + response := buildProductEntityResponse(rootDS, entityDS, productCacheKeyTemplate, providesData, ast.OperationTypeQuery) + + loader := &Loader{caches: map[string]LoaderCache{"default": cache}} + ctx := NewContext(t.Context()) + ctx.ExecutionOptions.DisableSubgraphRequestDeduplication = true + ctx.ExecutionOptions.Caching.EnableL1Cache = true + ctx.ExecutionOptions.Caching.EnableL2Cache = true + + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + resolvable := NewResolvable(ar, ResolvableOptions{}) + err := resolvable.Init(ctx, nil, ast.OperationTypeQuery) + require.NoError(t, err) + + err = loader.LoadGraphQLResponseData(ctx, response, resolvable) + require.NoError(t, err) + + out := fastjsonext.PrintGraphQLResponse(resolvable.data, resolvable.errors) + assert.Equal(t, `{"data":{"product":{"__typename":"Product","id":"prod-1","name":"Product One"}}}`, out) + }) +} + +func TestMutationSkipsL2Read(t *testing.T) { + t.Run("mutation operation type skips L2 read and always fetches", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + cache := NewFakeLoaderCache() + // Pre-populate cache with stale data + _ = cache.Set(t.Context(), []*CacheEntry{ + {Key: "Product:prod-1", Value: []byte(`{"__typename":"Product","id":"prod-1","name":"Old Name"}`)}, + }, 30*time.Second) + + userCacheKeyTemplate := &EntityQueryCacheKeyTemplate{ + Keys: NewResolvableObjectVariable(&Object{ + Fields: []*Field{ + {Name: []byte("__typename"), Value: &String{Path: []string{"__typename"}}}, + {Name: []byte("id"), Value: &String{Path: []string{"id"}}}, + }, + }), + } + providesData := &Object{ + Fields: []*Field{ + {Name: []byte("name"), Value: &Scalar{}}, + }, + } + + rootDS := NewMockDataSource(ctrl) + rootDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"updateUser":{"__typename":"Product","id":"prod-1"}}}`), nil + }).Times(1) + + entityDS := NewMockDataSource(ctrl) + entityDS.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, headers any, input []byte) ([]byte, error) { + return []byte(`{"data":{"_entities":[{"__typename":"Product","id":"prod-1","name":"New Name"}]}}`), nil + }).Times(1) // Must fetch fresh data despite cache having stale entry + + response := buildProductEntityResponse(rootDS, entityDS, userCacheKeyTemplate, providesData, ast.OperationTypeMutation) + + loader := &Loader{caches: map[string]LoaderCache{"default": cache}} + ctx := NewContext(t.Context()) + ctx.ExecutionOptions.DisableSubgraphRequestDeduplication = true + ctx.ExecutionOptions.Caching.EnableL1Cache = true + ctx.ExecutionOptions.Caching.EnableL2Cache = true + + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + resolvable := NewResolvable(ar, ResolvableOptions{}) + err := resolvable.Init(ctx, nil, ast.OperationTypeMutation) + require.NoError(t, err) + + err = loader.LoadGraphQLResponseData(ctx, response, resolvable) + require.NoError(t, err) + + out := fastjsonext.PrintGraphQLResponse(resolvable.data, resolvable.errors) + assert.Equal(t, `{"data":{"updateUser":{"__typename":"Product","id":"prod-1","name":"New Name"}}}`, out, "mutation should fetch fresh data, not use cached stale data") + }) +} + func TestWriteCanonicalJSON(t *testing.T) { canonicalize := func(input string) string { v, err := astjson.Parse(input) diff --git a/v2/pkg/engine/resolve/l1_cache_test.go b/v2/pkg/engine/resolve/l1_cache_test.go index 1f663cd70e..ad304d0e9a 100644 --- a/v2/pkg/engine/resolve/l1_cache_test.go +++ b/v2/pkg/engine/resolve/l1_cache_test.go @@ -1638,6 +1638,220 @@ func TestNormalizeForCache(t *testing.T) { resultJSON := string(result.MarshalTo(nil)) assert.Equal(t, `{"username":"Alice","__typename":"User"}`, resultJSON, "should normalize alias and preserve __typename") }) + + t.Run("with CacheArgs - appends arg suffix", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + item := mustParseJSON(ar, `{"friends":"value"}`) + result := loader.normalizeForCache(item, obj) + + suffix := loader.computeArgSuffix(field.CacheArgs) + resultJSON := string(result.MarshalTo(nil)) + assert.Equal(t, `{"friends`+suffix+`":"value"}`, resultJSON, "should append arg suffix to field name") + }) + + t.Run("with alias + CacheArgs - uses original name + arg suffix", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("myFriends"), + OriginalName: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + item := mustParseJSON(ar, `{"myFriends":"value"}`) + result := loader.normalizeForCache(item, obj) + + suffix := loader.computeArgSuffix(field.CacheArgs) + resultJSON := string(result.MarshalTo(nil)) + assert.Equal(t, `{"friends`+suffix+`":"value"}`, resultJSON, "should use original name + arg suffix") + }) +} + +func TestNormalizeDenormalizeRoundTrip(t *testing.T) { + t.Run("round-trip with CacheArgs preserves data", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + original := mustParseJSON(ar, `{"friends":"value"}`) + normalized := loader.normalizeForCache(original, obj) + denormalized := loader.denormalizeFromCache(normalized, obj) + + assert.Equal(t, `{"friends":"value"}`, string(denormalized.MarshalTo(nil))) + }) + + t.Run("round-trip with alias + CacheArgs preserves data", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("myFriends"), + OriginalName: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + original := mustParseJSON(ar, `{"myFriends":"value"}`) + normalized := loader.normalizeForCache(original, obj) + denormalized := loader.denormalizeFromCache(normalized, obj) + + assert.Equal(t, `{"myFriends":"value"}`, string(denormalized.MarshalTo(nil))) + }) + + t.Run("round-trip nested object with alias + CacheArgs", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + innerObj := &Object{ + HasAliases: true, + Fields: []*Field{ + {Name: []byte("n"), OriginalName: []byte("name"), Value: &Scalar{}}, + }, + } + field := &Field{ + Name: []byte("myFriends"), + OriginalName: []byte("friends"), + Value: innerObj, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + original := mustParseJSON(ar, `{"myFriends":{"n":"Alice"}}`) + normalized := loader.normalizeForCache(original, obj) + denormalized := loader.denormalizeFromCache(normalized, obj) + + assert.Equal(t, `{"myFriends":{"n":"Alice"}}`, string(denormalized.MarshalTo(nil))) + }) + + t.Run("round-trip array of objects with alias + CacheArgs", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + innerObj := &Object{ + HasAliases: true, + Fields: []*Field{ + {Name: []byte("n"), OriginalName: []byte("name"), Value: &Scalar{}}, + }, + } + arrNode := &Array{Item: innerObj} + field := &Field{ + Name: []byte("myFriends"), + OriginalName: []byte("friends"), + Value: arrNode, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + original := mustParseJSON(ar, `{"myFriends":[{"n":"Alice"},{"n":"Bob"}]}`) + normalized := loader.normalizeForCache(original, obj) + denormalized := loader.denormalizeFromCache(normalized, obj) + + assert.Equal(t, `{"myFriends":[{"n":"Alice"},{"n":"Bob"}]}`, string(denormalized.MarshalTo(nil))) + }) + + t.Run("round-trip preserves __typename with CacheArgs", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("myFriends"), + OriginalName: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + original := mustParseJSON(ar, `{"__typename":"User","myFriends":"value"}`) + normalized := loader.normalizeForCache(original, obj) + denormalized := loader.denormalizeFromCache(normalized, obj) + + // After round-trip, __typename should be preserved and field alias restored + result := denormalized + assert.Equal(t, `"User"`, string(result.Get("__typename").MarshalTo(nil))) + assert.Equal(t, `"value"`, string(result.Get("myFriends").MarshalTo(nil))) + }) + + t.Run("round-trip multiple fields with different CacheArgs", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5","b":"10"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field1 := &Field{ + Name: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + field2 := &Field{ + Name: []byte("id"), + Value: &Scalar{}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field1, field2}, + } + + original := mustParseJSON(ar, `{"friends":"Alice","id":"1"}`) + normalized := loader.normalizeForCache(original, obj) + denormalized := loader.denormalizeFromCache(normalized, obj) + + assert.Equal(t, `"Alice"`, string(denormalized.Get("friends").MarshalTo(nil))) + assert.Equal(t, `"1"`, string(denormalized.Get("id").MarshalTo(nil))) + }) } func TestDenormalizeFromCache(t *testing.T) { @@ -1680,6 +1894,59 @@ func TestDenormalizeFromCache(t *testing.T) { resultJSON := string(result.MarshalTo(nil)) assert.Equal(t, `{"userName":"Alice"}`, resultJSON, "should convert original name to alias") }) + + t.Run("with CacheArgs - looks up suffixed field name", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + // Cache stores data with suffixed key + suffix := loader.computeArgSuffix(field.CacheArgs) + cacheJSON := `{"friends` + suffix + `":"value"}` + cacheItem := mustParseJSON(ar, cacheJSON) + + result := loader.denormalizeFromCache(cacheItem, obj) + resultJSON := string(result.MarshalTo(nil)) + assert.Equal(t, `{"friends":"value"}`, resultJSON, "should map suffixed cache key back to query name") + }) + + t.Run("with alias + CacheArgs - maps suffixed original back to alias", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("myFriends"), + OriginalName: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{{ArgName: "first", VariableName: "a"}}, + } + obj := &Object{ + HasAliases: true, + Fields: []*Field{field}, + } + + // Cache stores: friends_ → value + suffix := loader.computeArgSuffix(field.CacheArgs) + cacheJSON := `{"friends` + suffix + `":"value"}` + cacheItem := mustParseJSON(ar, cacheJSON) + + result := loader.denormalizeFromCache(cacheItem, obj) + resultJSON := string(result.MarshalTo(nil)) + assert.Equal(t, `{"myFriends":"value"}`, resultJSON, "should map suffixed original name back to alias") + }) } func TestValidateFieldDataWithAliases(t *testing.T) { @@ -1811,3 +2078,528 @@ func mustParseJSON(a arena.Arena, jsonStr string) *astjson.Value { } return v } + +// --- P1: validateItemHasRequiredData unit tests --- + +func TestValidateItemHasRequiredData(t *testing.T) { + t.Run("nil item returns false", func(t *testing.T) { + loader := &Loader{} + obj := &Object{Fields: []*Field{{Name: []byte("id"), Value: &Scalar{}}}} + assert.False(t, loader.validateItemHasRequiredData(nil, obj)) + }) + + t.Run("all required scalar fields present", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("id"), Value: &Scalar{}}, + {Name: []byte("name"), Value: &Scalar{}}, + }, + } + item := mustParseJSON(ar, `{"id":"1","name":"Alice"}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("missing required field", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("id"), Value: &Scalar{}}, + {Name: []byte("name"), Value: &Scalar{}}, + }, + } + item := mustParseJSON(ar, `{"id":"1"}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("null value for non-nullable scalar", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("id"), Value: &Scalar{Nullable: false}}, + }, + } + item := mustParseJSON(ar, `{"id":null}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("null value for nullable scalar", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("email"), Value: &Scalar{Nullable: true}}, + }, + } + item := mustParseJSON(ar, `{"email":null}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("nested object with all fields", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + innerObj := &Object{ + Fields: []*Field{ + {Name: []byte("street"), Value: &Scalar{}}, + }, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("address"), Value: innerObj}, + }, + } + item := mustParseJSON(ar, `{"address":{"street":"Main St"}}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("nested object missing required field", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + innerObj := &Object{ + Fields: []*Field{ + {Name: []byte("street"), Value: &Scalar{}}, + {Name: []byte("city"), Value: &Scalar{}}, + }, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("address"), Value: innerObj}, + }, + } + item := mustParseJSON(ar, `{"address":{"street":"Main St"}}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("null for non-nullable object", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + innerObj := &Object{ + Nullable: false, + Fields: []*Field{{Name: []byte("street"), Value: &Scalar{}}}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("address"), Value: innerObj}, + }, + } + item := mustParseJSON(ar, `{"address":null}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("null for nullable object", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + innerObj := &Object{ + Nullable: true, + Fields: []*Field{{Name: []byte("street"), Value: &Scalar{}}}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("address"), Value: innerObj}, + }, + } + item := mustParseJSON(ar, `{"address":null}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("non-object value for object field", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + innerObj := &Object{ + Fields: []*Field{{Name: []byte("street"), Value: &Scalar{}}}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("address"), Value: innerObj}, + }, + } + item := mustParseJSON(ar, `{"address":"not-an-object"}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("array with all valid items", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{ + Item: &Scalar{}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":["a","b","c"]}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("array with invalid item - non-nullable scalar null", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{ + Item: &Scalar{Nullable: false}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":["a",null,"c"]}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("array with nullable items allows null", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{ + Item: &Scalar{Nullable: true}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":["a",null,"c"]}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("null for non-nullable array", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{ + Nullable: false, + Item: &Scalar{}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":null}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("null for nullable array", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{ + Nullable: true, + Item: &Scalar{}, + } + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":null}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("non-array value for array field", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{Item: &Scalar{}} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":"not-an-array"}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("empty array is valid", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{Item: &Scalar{}} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":[]}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("array of objects with valid items", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + itemObj := &Object{ + Fields: []*Field{ + {Name: []byte("id"), Value: &Scalar{}}, + }, + } + arr := &Array{Item: itemObj} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("items"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"items":[{"id":"1"},{"id":"2"}]}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("array of objects with invalid item", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + itemObj := &Object{ + Fields: []*Field{ + {Name: []byte("id"), Value: &Scalar{}}, + {Name: []byte("name"), Value: &Scalar{}}, + }, + } + arr := &Array{Item: itemObj} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("items"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"items":[{"id":"1","name":"ok"},{"id":"2"}]}`) + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("field with CacheArgs uses suffixed name for lookup", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"first":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + // Field has CacheArgs, so validation should look for "friends_" not "friends" + field := &Field{ + Name: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{ + {ArgName: "first", VariableName: "first"}, + }, + } + + // Compute expected suffixed name + suffix := loader.computeArgSuffix(field.CacheArgs) + expectedKey := "friends" + suffix + + // Item has the suffixed field name (as normalize would produce) + itemJSON := `{"` + expectedKey + `":"value"}` + item := mustParseJSON(ar, itemJSON) + + obj := &Object{Fields: []*Field{field}} + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("field with CacheArgs fails when only base name present", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"first":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + field := &Field{ + Name: []byte("friends"), + Value: &Scalar{}, + CacheArgs: []CacheFieldArg{ + {ArgName: "first", VariableName: "first"}, + }, + } + + // Item has only the base name "friends" without suffix + item := mustParseJSON(ar, `{"friends":"value"}`) + + obj := &Object{Fields: []*Field{field}} + assert.False(t, loader.validateItemHasRequiredData(item, obj)) + }) + + t.Run("array with nil Item spec is valid if array exists", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + arr := &Array{Item: nil} + obj := &Object{ + Fields: []*Field{ + {Name: []byte("tags"), Value: arr}, + }, + } + item := mustParseJSON(ar, `{"tags":["a","b"]}`) + assert.True(t, loader.validateItemHasRequiredData(item, obj)) + }) +} + +// --- P3: computeArgSuffix unit tests --- + +func TestComputeArgSuffix(t *testing.T) { + t.Run("single arg produces deterministic suffix", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + suffix1 := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "a"}}) + suffix2 := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "a"}}) + + assert.Equal(t, suffix1, suffix2, "same args should produce same suffix") + assert.Equal(t, 17, len(suffix1), "suffix should be _ + 16 hex chars") + assert.Equal(t, byte('_'), suffix1[0], "suffix should start with underscore") + }) + + t.Run("different values produce different suffixes", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"5","b":"10"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + suffix1 := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "a"}}) + suffix2 := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "b"}}) + + assert.NotEqual(t, suffix1, suffix2, "different values should produce different suffixes") + }) + + t.Run("null variable produces null in hash", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + // Variable "missing" doesn't exist, so argValue is nil → "null" written + suffix := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "missing"}}) + assert.Equal(t, 17, len(suffix), "should still produce valid suffix for null variable") + }) + + t.Run("null variable differs from string null", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":null,"b":"null"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + suffixNull := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "a"}}) + suffixMissing := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "missing"}}) + + // Both json null and missing variable produce "null" in the hash, + // so they should be equal + assert.Equal(t, suffixNull, suffixMissing, "json null and missing variable both hash as null") + }) + + t.Run("unsorted args get sorted before hashing", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"a":"1","b":"2"}`)) + loader := &Loader{jsonArena: ar, ctx: ctx} + + sorted := []CacheFieldArg{ + {ArgName: "alpha", VariableName: "a"}, + {ArgName: "beta", VariableName: "b"}, + } + unsorted := []CacheFieldArg{ + {ArgName: "beta", VariableName: "b"}, + {ArgName: "alpha", VariableName: "a"}, + } + + suffixSorted := loader.computeArgSuffix(sorted) + suffixUnsorted := loader.computeArgSuffix(unsorted) + + assert.Equal(t, suffixSorted, suffixUnsorted, "arg order should not affect suffix") + }) + + t.Run("RemapVariables applied before lookup", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx := NewContext(t.Context()) + ctx.Variables = astjson.MustParseBytes([]byte(`{"original":"42"}`)) + ctx.RemapVariables = map[string]string{"remapped": "original"} + loader := &Loader{jsonArena: ar, ctx: ctx} + + // "remapped" maps to "original" which has value "42" + suffixRemapped := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "remapped"}}) + // "original" has value "42" directly + suffixDirect := loader.computeArgSuffix([]CacheFieldArg{{ArgName: "first", VariableName: "original"}}) + + assert.Equal(t, suffixRemapped, suffixDirect, "remapped variable should produce same suffix as direct lookup") + }) + + t.Run("object arg produces deterministic hash regardless of key order", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + ctx1 := NewContext(t.Context()) + ctx1.Variables = astjson.MustParseBytes([]byte(`{"filter":{"name":"Alice","age":30}}`)) + loader1 := &Loader{jsonArena: ar, ctx: ctx1} + + ctx2 := NewContext(t.Context()) + ctx2.Variables = astjson.MustParseBytes([]byte(`{"filter":{"age":30,"name":"Alice"}}`)) + loader2 := &Loader{jsonArena: ar, ctx: ctx2} + + suffix1 := loader1.computeArgSuffix([]CacheFieldArg{{ArgName: "filter", VariableName: "filter"}}) + suffix2 := loader2.computeArgSuffix([]CacheFieldArg{{ArgName: "filter", VariableName: "filter"}}) + + assert.Equal(t, suffix1, suffix2, "object arg key order should not affect hash (canonical JSON)") + }) +} + +// --- P4: mergeEntityFields unit tests --- + +func TestMergeEntityFields(t *testing.T) { + t.Run("new field added to existing entity", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + + dst := mustParseJSON(ar, `{"id":"1","name":"Alice"}`) + src := mustParseJSON(ar, `{"id":"1","email":"alice@example.com"}`) + + loader.mergeEntityFields(dst, src) + + resultJSON := string(dst.MarshalTo(nil)) + assert.Equal(t, `{"id":"1","name":"Alice","email":"alice@example.com"}`, resultJSON) + }) + + t.Run("existing field preserved not overwritten", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + + dst := mustParseJSON(ar, `{"id":"1","name":"Alice"}`) + src := mustParseJSON(ar, `{"id":"1","name":"Bob"}`) + + loader.mergeEntityFields(dst, src) + + resultJSON := string(dst.MarshalTo(nil)) + assert.Equal(t, `{"id":"1","name":"Alice"}`, resultJSON, "existing field should not be overwritten") + }) + + t.Run("nil dst is no-op", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + src := mustParseJSON(ar, `{"id":"1"}`) + // Should not panic + loader.mergeEntityFields(nil, src) + }) + + t.Run("nil src is no-op", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + dst := mustParseJSON(ar, `{"id":"1"}`) + loader.mergeEntityFields(dst, nil) + resultJSON := string(dst.MarshalTo(nil)) + assert.Equal(t, `{"id":"1"}`, resultJSON, "dst should be unchanged") + }) + + t.Run("non-object type is no-op", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + dst := mustParseJSON(ar, `"string-value"`) + src := mustParseJSON(ar, `{"id":"1"}`) + // Should not panic + loader.mergeEntityFields(dst, src) + }) + + t.Run("multiple new and existing fields coexist", func(t *testing.T) { + ar := arena.NewMonotonicArena(arena.WithMinBufferSize(1024)) + loader := &Loader{jsonArena: ar} + + dst := mustParseJSON(ar, `{"id":"1","name":"Alice","age":30}`) + src := mustParseJSON(ar, `{"id":"1","email":"a@b.com","role":"admin","name":"Bob"}`) + + loader.mergeEntityFields(dst, src) + + result := dst + // Existing fields preserved + assert.Equal(t, `"1"`, string(result.Get("id").MarshalTo(nil))) + assert.Equal(t, `"Alice"`, string(result.Get("name").MarshalTo(nil))) + assert.Equal(t, `30`, string(result.Get("age").MarshalTo(nil))) + // New fields added + assert.Equal(t, `"a@b.com"`, string(result.Get("email").MarshalTo(nil))) + assert.Equal(t, `"admin"`, string(result.Get("role").MarshalTo(nil))) + }) +} From eea0c06d3b25991ad19fca9f3689d8a518b16168 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Fri, 6 Mar 2026 21:13:06 +0100 Subject: [PATCH 2/4] fix: add empty line after embedded field to satisfy linter Co-Authored-By: Claude Opus 4.6 --- v2/pkg/engine/resolve/cache_load_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/v2/pkg/engine/resolve/cache_load_test.go b/v2/pkg/engine/resolve/cache_load_test.go index e0db13a2a4..c70d7ae0d7 100644 --- a/v2/pkg/engine/resolve/cache_load_test.go +++ b/v2/pkg/engine/resolve/cache_load_test.go @@ -1980,6 +1980,7 @@ func TestShadowMode_WithoutAnalytics(t *testing.T) { // when configured to do so. Used for testing L2 error resilience. type ErrorLoaderCache struct { *FakeLoaderCache + getErr error setErr error } From 5b0b0c3093dd49a766edca4b665ae22f327d4a7b Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Fri, 6 Mar 2026 21:28:20 +0100 Subject: [PATCH 3/4] fix: apply gofmt formatting to websocket test files Co-Authored-By: Claude Opus 4.6 --- .../graphql_sse_handler_test.go | 16 ++++++++-------- .../graphql_tws_handler_test.go | 6 +++--- .../graphql_ws_handler_test.go | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_sse_handler_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_sse_handler_test.go index 181669e80d..c5c8cdbeef 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_sse_handler_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_sse_handler_test.go @@ -51,7 +51,7 @@ func TestGraphQLSubscriptionClientSubscribe_SSE(t *testing.T) { ctx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) @@ -91,7 +91,7 @@ func TestGraphQLSubscriptionClientSubscribe_SSE_RequestAbort(t *testing.T) { clientCancel() client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, t.Context(), - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) @@ -157,7 +157,7 @@ func TestGraphQLSubscriptionClientSubscribe_SSE_POST(t *testing.T) { ctx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) @@ -228,7 +228,7 @@ func TestGraphQLSubscriptionClientSubscribe_SSE_WithEvents(t *testing.T) { ctx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) @@ -294,7 +294,7 @@ func TestGraphQLSubscriptionClientSubscribe_SSE_Error(t *testing.T) { ctx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) @@ -397,7 +397,7 @@ func TestGraphQLSubscriptionClientSubscribe_SSE_Error_Without_Header(t *testing. ctx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) @@ -466,7 +466,7 @@ func TestGraphQLSubscriptionClientSubscribe_QueryParams(t *testing.T) { ctx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) @@ -607,7 +607,7 @@ func TestGraphQLSubscriptionClientSubscribe_SSE_Upstream_Dies(t *testing.T) { ctx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_tws_handler_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_tws_handler_test.go index 613cff7675..638b6a739d 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_tws_handler_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_tws_handler_test.go @@ -61,7 +61,7 @@ func TestWebsocketSubscriptionClient_GQLTWS(t *testing.T) { serverCtx, serverCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ).(*subscriptionClient) @@ -139,7 +139,7 @@ func TestWebsocketSubscriptionClientPing_GQLTWS(t *testing.T) { serverCtx, serverCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ).(*subscriptionClient) @@ -206,7 +206,7 @@ func TestWebsocketSubscriptionClientError_GQLTWS(t *testing.T) { clientCtx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_ws_handler_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_ws_handler_test.go index 76a8e0a6bb..7d3a843286 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_ws_handler_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_ws_handler_test.go @@ -72,7 +72,7 @@ func TestWebSocketSubscriptionClientInitIncludeKA_GQLWS(t *testing.T) { defer serverCancel() client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ).(*subscriptionClient) updater := &testSubscriptionUpdater{} @@ -136,7 +136,7 @@ func TestWebsocketSubscriptionClient_GQLWS(t *testing.T) { defer serverCancel() client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ).(*subscriptionClient) updater := &testSubscriptionUpdater{} @@ -197,7 +197,7 @@ func TestWebsocketSubscriptionClientErrorArray(t *testing.T) { defer serverCancel() clientCtx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) updater := &testSubscriptionUpdater{} @@ -254,7 +254,7 @@ func TestWebsocketSubscriptionClientErrorObject(t *testing.T) { defer serverCancel() clientCtx, clientCancel := context.WithCancel(context.Background()) client := NewGraphQLSubscriptionClient(http.DefaultClient, http.DefaultClient, serverCtx, - WithReadTimeout(100 * time.Millisecond), + WithReadTimeout(100*time.Millisecond), WithLogger(logger()), ) updater := &testSubscriptionUpdater{} From 464eb093e9095fcb7b6ef40aa45f41cf0565d13b Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Fri, 6 Mar 2026 21:31:38 +0100 Subject: [PATCH 4/4] fix(test): use real cache key format and propagate operationType in entity fetch Address CodeRabbit review feedback: - Use real cache key format (JSON with __typename and key) instead of "Product:prod-1" so tests validate actual cache hit/miss behavior - Propagate operationType to entity fetch FetchInfo so mutation tests exercise mutation-path behavior consistently - Add cache log assertion to verify corrupted entry is actually found Co-Authored-By: Claude Opus 4.6 --- v2/pkg/engine/resolve/cache_load_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/v2/pkg/engine/resolve/cache_load_test.go b/v2/pkg/engine/resolve/cache_load_test.go index c70d7ae0d7..4ec9a6016d 100644 --- a/v2/pkg/engine/resolve/cache_load_test.go +++ b/v2/pkg/engine/resolve/cache_load_test.go @@ -2055,7 +2055,7 @@ func buildProductEntityResponse(rootDS, entityDS DataSource, cacheKeyTemplate Ca Info: &FetchInfo{ DataSourceID: "ds", DataSourceName: "ds", RootFields: []GraphCoordinate{{TypeName: "Product", FieldName: "name"}}, - OperationType: ast.OperationTypeQuery, ProvidesData: providesData, + OperationType: operationType, ProvidesData: providesData, }, }, rootOpName+"."+rootFieldName, ObjectPath(rootFieldName)), ), @@ -2176,9 +2176,9 @@ func TestL2CacheErrorResilience(t *testing.T) { defer ctrl.Finish() cache := NewFakeLoaderCache() - // Pre-populate cache with corrupted JSON + // Pre-populate cache with corrupted JSON using the real key format _ = cache.Set(t.Context(), []*CacheEntry{ - {Key: "Product:prod-1", Value: []byte(`{not valid json!!!}`)}, + {Key: `{"__typename":"Product","key":{"id":"prod-1"}}`, Value: []byte(`{not valid json!!!}`)}, }, 30*time.Second) rootDS := NewMockDataSource(ctrl) @@ -2211,6 +2211,14 @@ func TestL2CacheErrorResilience(t *testing.T) { out := fastjsonext.PrintGraphQLResponse(resolvable.data, resolvable.errors) assert.Equal(t, `{"data":{"product":{"__typename":"Product","id":"prod-1","name":"Product One"}}}`, out) + + // Verify L2 cache was actually accessed (Get returned the corrupted entry, then Set wrote fresh data) + log := cache.GetLog() + assert.Equal(t, 3, len(log), "should have set (seed) + get (corrupted hit) + set (fresh data)") + assert.Equal(t, "set", log[0].Operation) + assert.Equal(t, "get", log[1].Operation) + assert.Equal(t, true, log[1].Hits[0], "L2 Get should find the seeded corrupted entry") + assert.Equal(t, "set", log[2].Operation) }) } @@ -2220,9 +2228,9 @@ func TestMutationSkipsL2Read(t *testing.T) { defer ctrl.Finish() cache := NewFakeLoaderCache() - // Pre-populate cache with stale data + // Pre-populate cache with stale data using the real key format _ = cache.Set(t.Context(), []*CacheEntry{ - {Key: "Product:prod-1", Value: []byte(`{"__typename":"Product","id":"prod-1","name":"Old Name"}`)}, + {Key: `{"__typename":"Product","key":{"id":"prod-1"}}`, Value: []byte(`{"__typename":"Product","id":"prod-1","name":"Old Name"}`)}, }, 30*time.Second) userCacheKeyTemplate := &EntityQueryCacheKeyTemplate{