Merge branch 'milinda/requestScoped-directive' into milinda/queryCache-directive

# Conflicts:
#	composition/src/router-configuration/types.ts
#	composition/src/v1/normalization/normalization-factory.ts
#	composition/src/v1/warnings/warnings.ts
#	connect-go/gen/proto/wg/cosmo/node/v1/node.pb.go
#	connect/src/wg/cosmo/node/v1/node_pb.ts
#	proto/wg/cosmo/node/v1/node.proto
#	router/gen/proto/wg/cosmo/node/v1/node.pb.go
#	shared/src/router-config/builder.ts

Author: SkArchon

composition/src/router-configuration/types.ts

@@ -87,7 +87,7 @@ export type RequiredFieldConfiguration = {
   disableEntityResolver?: boolean;
 };
 
-export type RequestScopedFieldConfig = {
+export type RequestScopedConfiguration = {
   fieldName: FieldName;
   typeName: TypeName;
   // L1 cache key used to store/lookup this field's value for the duration of a request.
@@ -186,7 +186,7 @@ export type EntityCachingConfiguration = {
   entityCacheConfigurations: Array<EntityCacheConfiguration>;
   // Attached to the Mutation/Subscription type's ConfigurationData from @openfed__cachePopulate.
   cachePopulateConfigurations: Array<CachePopulateConfig>;
-  requestScopedFields?: Array<RequestScopedFieldConfig>;
+  requestScopedConfigurations: Array<RequestScopedConfiguration>;
   // Attached to the Query type's ConfigurationData from @openfed__queryCache.
   queryCacheConfigurations?: Array<QueryCacheConfig>;
 };

composition/src/router-configuration/utils.ts

@@ -22,6 +22,7 @@ export function getOrInitializeEntityCaching(configurationData: ConfigurationDat
       cacheInvalidateConfigurations: [],
       entityCacheConfigurations: [],
       cachePopulateConfigurations: [],
+      requestScopedConfigurations: [],
     };
   }
   return configurationData.entityCaching;

composition/src/v1/normalization/normalization-factory.ts

@@ -217,8 +217,8 @@ import {
   type CachePopulateConfig,
   type EntityKeyMappingConfig,
   type FieldMappingConfig,
-  type RequestScopedFieldConfig,
   type QueryCacheConfig,
+  type RequestScopedConfiguration,
 } from '../../router-configuration/types';
 import { printTypeNode } from '@graphql-tools/merge';
 import {
@@ -229,10 +229,10 @@ import {
   fieldAlreadyProvidedWarning,
   invalidExternalFieldWarning,
   nonExternalConditionalFieldWarning,
+  requestScopedSingleFieldWarning,
   singleSubgraphInputFieldOneOfWarning,
   unimplementedInterfaceOutputTypeWarning,
   queryCacheReturnEntityMissingEntityCacheWarning,
-  requestScopedSingleFieldWarning,
 } from '../warnings/warnings';
 import { upsertDirectiveSchemaAndEntityDefinitions, upsertParentsAndChildren } from './walkers';
 import {
@@ -542,6 +542,7 @@ export class NormalizationFactory {
   referencedDirectiveNames = new Set<DirectiveName>();
   referencedTypeNames = new Set<TypeName>();
   renamedParentTypeName = '';
+  requestScopedFieldCoordsByL1Key = new Map<string, Array<string>>();
   schemaData: SchemaData;
   subgraphName: SubgraphName;
   unvalidatedExternalFieldCoords = new Set<string>();
@@ -5214,81 +5215,36 @@ export class NormalizationFactory {
     return [];
   }
 
-  extractRequestScopedFields() {
-    // Gather fields annotated with @openfed__requestScoped across all types in this subgraph.
-    // A field is both a reader and writer of the coordinate L1 — no receiver/provider.
-    // Fields with the same `key` share the same L1 entry: whichever is resolved first
-    // populates it, subsequent ones inject from it.
-    type Collected = {
-      typeName: string;
-      fieldName: string;
-      fieldCoords: string;
-      key: string;
-      l1Key: string;
-    };
-    const collected: Array<Collected> = [];
-
-    for (const [, parentData] of this.parentDefinitionDataByTypeName) {
-      if (parentData.kind !== Kind.OBJECT_TYPE_DEFINITION && parentData.kind !== Kind.INTERFACE_TYPE_DEFINITION) {
-        continue;
-      }
-      const typeName = getParentTypeName(parentData);
-      for (const [fieldName, fieldData] of parentData.fieldDataByName) {
-        const directives = fieldData.directivesByName.get(OPENFED_REQUEST_SCOPED);
-        if (!directives) {
-          continue;
-        }
-        // validateDirectives() (run earlier in normalize()) has already guaranteed a single, non-repeated
-        // @openfed__requestScoped with a required String `key`, so the generic ConstDirectiveNode can be
-        // narrowed to the precise typed node — mirroring handleComposeDirective()/ComposeDirectiveNode.
-        const directive = directives[0] as RequestScopedDirectiveNode;
-        const keyArg = directive.arguments.find((arg) => arg.name.value === KEY);
-        if (!keyArg) {
-          continue;
-        }
-
-        const fieldCoords = `${typeName}.${fieldName}`;
-        const key = keyArg.value.value;
-        const l1Key = `${this.subgraphName}.${key}`;
-        collected.push({ typeName, fieldName, fieldCoords, key, l1Key });
-      }
-    }
-
-    if (collected.length === 0) {
+  // Attaches a single field annotated with @openfed__requestScoped to its type's configurationData. A field
+  // is both a reader and writer of the coordinate L1 — no receiver/provider. Fields with the same `key` share
+  // the same L1 entry: whichever is resolved first populates it, subsequent ones inject from it.
+  extractRequestScopedConfig(fieldData: FieldData) {
+    const directives = fieldData.directivesByName.get(OPENFED_REQUEST_SCOPED);
+    if (!directives) {
       return;
     }
-
-    // Warn when a key is used on only one field — @openfed__requestScoped is meaningless
-    // unless >= 2 fields share the key (there'd be no second reader to benefit).
-    const coordsByKey = new Map<string, Array<string>>();
-    for (const c of collected) {
-      getValueOrDefault(coordsByKey, c.key, () => []).push(c.fieldCoords);
-    }
-    for (const [key, coordsList] of coordsByKey) {
-      if (coordsList.length === 1) {
-        this.warnings.push(
-          requestScopedSingleFieldWarning({
-            subgraphName: this.subgraphName,
-            key,
-            fieldCoords: coordsList[0],
-          }),
-        );
-      }
+    // validateDirectives() (run earlier in normalize()) has already guaranteed a single, non-repeated
+    // @openfed__requestScoped with a required String `key`, so the generic ConstDirectiveNode can be
+    // narrowed to the precise typed node — mirroring handleComposeDirective()/ComposeDirectiveNode.
+    const directive = directives[0] as RequestScopedDirectiveNode;
+    const keyArg = directive.arguments.find((arg) => arg.name.value === KEY);
+    if (!keyArg) {
+      return;
     }
 
-    // Group by type and attach to configurationData.
-    const byType = new Map<string, Array<RequestScopedFieldConfig>>();
-    for (const c of collected) {
-      const list = byType.get(c.typeName) ?? [];
-      list.push({ fieldName: c.fieldName, typeName: c.typeName, l1Key: c.l1Key });
-      byType.set(c.typeName, list);
-    }
-    for (const [typeName, fields] of byType) {
-      const configurationData = getValueOrDefault(this.configurationDataByTypeName, typeName, () =>
-        newConfigurationData(false, typeName),
-      );
-      getOrInitializeEntityCaching(configurationData).requestScopedFields = fields;
-    }
+    const config: RequestScopedConfiguration = {
+      fieldName: fieldData.name,
+      typeName: fieldData.renamedParentTypeName,
+      l1Key: `${this.subgraphName}.${keyArg.value.value}`,
+    };
+    const configurationData = getValueOrDefault(this.configurationDataByTypeName, fieldData.renamedParentTypeName, () =>
+      newConfigurationData(false, fieldData.renamedParentTypeName),
+    );
+    getOrInitializeEntityCaching(configurationData).requestScopedConfigurations.push(config);
+    // Track field coords per L1 key so the single-field warning can be emitted after the walk completes.
+    getValueOrDefault(this.requestScopedFieldCoordsByL1Key, config.l1Key, () => []).push(
+      `${config.typeName}.${config.fieldName}`,
+    );
   }
 
   addFieldNamesToConfigurationData(fieldDataByFieldName: Map<string, FieldData>, configurationData: ConfigurationData) {
@@ -5483,6 +5439,8 @@ export class NormalizationFactory {
                 this.extractCacheInvalidateConfig(fieldData);
                 this.extractCachePopulateConfig(fieldData);
               }
+
+              this.extractRequestScopedConfig(fieldData);
             } else if (fieldData.externalFieldDataBySubgraphName.get(this.subgraphName)?.isDefinedExternal) {
               externalInterfaceFieldNames.push(fieldName);
             }
@@ -5573,6 +5531,21 @@ export class NormalizationFactory {
         }
       }
     }
+    // @openfed__requestScoped is meaningless unless >= 2 fields share a key (there'd be no second reader to
+    // benefit), so warn for any key used on only one field across the subgraph. extractRequestScopedConfig()
+    // populated requestScopedFieldCoordsByL1Key during the type walk above.
+    for (const [l1Key, fieldCoordsList] of this.requestScopedFieldCoordsByL1Key) {
+      if (fieldCoordsList.length === 1) {
+        this.warnings.push(
+          requestScopedSingleFieldWarning({
+            subgraphName: this.subgraphName,
+            // l1Key is `${this.subgraphName}.${key}`, so strip the prefix to recover the original key.
+            key: l1Key.slice(this.subgraphName.length + 1),
+            fieldCoords: fieldCoordsList[0],
+          }),
+        );
+      }
+    }
     this.isSubgraphEventDrivenGraph = this.edfsDirectiveReferences.size > 0;
     // this is where @provides and @requires configurations are added to the ConfigurationData
     this.addValidConditionalFieldSetConfigurations();
@@ -5581,8 +5554,6 @@ export class NormalizationFactory {
     // @openfed__queryCache extraction and @openfed__is placement validation — must run after key field sets
     // are available (queryCache reads keyFieldSetDatasByTypeName to build argument→key mappings)
     this.processRootFieldCacheDirectives();
-    // this is where @openfed__requestScoped configurations are added to the ConfigurationData
-    this.extractRequestScopedFields();
     // Check that explicitly defined operations types are valid objects and that their fields are also valid
     for (const operationType of Object.values(OperationTypeNode)) {
       const operationTypeNode = this.schemaData.operationTypes.get(operationType);

composition/src/v1/warnings/params.ts

@@ -20,7 +20,7 @@ export type InvalidRepeatedComposedDirectiveWarningParams = {
 export type RequestScopedSingleFieldWarningParams = {
   subgraphName: SubgraphName;
   key: string;
-  fieldCoords: FieldName;
+  fieldCoords: string;
 };
 
 export type QueryCacheReturnEntityMissingEntityCacheWarningParams = {

composition/src/v1/warnings/warnings.ts

@@ -194,6 +194,22 @@ export function singleSubgraphInputFieldOneOfWarning({
   });
 }
 
+export function requestScopedSingleFieldWarning({
+  subgraphName,
+  key,
+  fieldCoords,
+}: RequestScopedSingleFieldWarningParams): Warning {
+  return new Warning({
+    message:
+      `@openfed__requestScoped(key: "${key}") is declared on only one field ("${fieldCoords}") in this subgraph.` +
+      ` The directive is meaningless unless at least 2 fields share the same key so that the second and` +
+      ` subsequent fields can be served from the per-request L1 cache populated by the first.`,
+    subgraph: {
+      name: subgraphName,
+    },
+  });
+}
+
 export function singleFederatedInputFieldOneOfWarning({
   fieldName,
   typeName,
@@ -239,22 +255,6 @@ export function invalidRepeatedComposedDirectiveWarning({
   });
 }
 
-export function requestScopedSingleFieldWarning({
-  subgraphName,
-  key,
-  fieldCoords,
-}: RequestScopedSingleFieldWarningParams): Warning {
-  return new Warning({
-    message:
-      `@openfed__requestScoped(key: "${key}") is declared on only one field ("${fieldCoords}") in this subgraph.` +
-      ` The directive is meaningless unless at least 2 fields share the same key so that the second` +
-      ` and subsequent fields can be served from the per-request L1 cache populated by the first.`,
-    subgraph: {
-      name: subgraphName,
-    },
-  });
-}
-
 export function queryCacheReturnEntityMissingEntityCacheWarning({
   subgraphName,
   fieldCoords,

composition/tests/v1/directives/request-scoped.test.ts

@@ -1,6 +1,5 @@
 import { describe, expect, test } from 'vitest';
 import {
-  DIRECTIVE_DEFINITION_BY_NAME,
   FIRST_ORDINAL,
   invalidDirectiveError,
   invalidRepeatedDirectiveErrorMessage,
@@ -12,9 +11,9 @@ import {
 } from '../../../src';
 import { createSubgraphWithDefaultName, normalizeSubgraphFailure, normalizeSubgraphSuccess } from '../../utils/utils';
 
-describe('@openfed__requestScoped', () => {
-  describe('registry', () => {
-    test('the directive is materialized in the normalized subgraph output', () => {
+describe('@openfed__requestScoped tests', () => {
+  describe('registry tests', () => {
+    test('that the directive is materialized in the normalized subgraph output', () => {
       const { directiveDefinitionByName } = normalizeSubgraphSuccess(
         createSubgraphWithDefaultName(`
           type Query {
@@ -32,8 +31,8 @@ describe('@openfed__requestScoped', () => {
     });
   });
 
-  describe('configuration extraction', () => {
-    test('≥ 2 fields sharing the same key produce a subgraph-prefixed l1Key and no warning', () => {
+  describe('configuration extraction tests', () => {
+    test('that ≥ 2 fields sharing the same key produce a subgraph-prefixed l1Key and no warning', () => {
       const result = normalizeSubgraphSuccess(
         createSubgraphWithDefaultName(`
           type Query {
@@ -47,16 +46,16 @@ describe('@openfed__requestScoped', () => {
         ROUTER_COMPATIBILITY_VERSION_ONE,
       );
       const config = result.configurationDataByTypeName.get('Query');
-      expect(config!.entityCaching?.requestScopedFields).toBeDefined();
-      expect(config!.entityCaching?.requestScopedFields).toHaveLength(2);
-      expect(config!.entityCaching!.requestScopedFields!.map((f) => f.l1Key)).toEqual([
+      expect(config!.entityCaching?.requestScopedConfigurations).toBeDefined();
+      expect(config!.entityCaching?.requestScopedConfigurations).toHaveLength(2);
+      expect(config!.entityCaching!.requestScopedConfigurations!.map((f) => f.l1Key)).toEqual([
         'subgraph-default-a.me',
         'subgraph-default-a.me',
       ]);
       expect(result.warnings).toHaveLength(0);
     });
 
-    test('works on a non-entity object type field', () => {
+    test('that it works on a non-entity object type field', () => {
       const result = normalizeSubgraphSuccess(
         createSubgraphWithDefaultName(`
           type Query {
@@ -67,13 +66,13 @@ describe('@openfed__requestScoped', () => {
         ROUTER_COMPATIBILITY_VERSION_ONE,
       );
       const config = result.configurationDataByTypeName.get('Query');
-      expect(config!.entityCaching?.requestScopedFields).toBeDefined();
-      expect(config!.entityCaching?.requestScopedFields).toHaveLength(2);
-      expect(config!.entityCaching!.requestScopedFields![0].fieldName).toBe('currentLocale');
-      expect(config!.entityCaching!.requestScopedFields![0].l1Key).toBe('subgraph-default-a.locale');
+      expect(config!.entityCaching?.requestScopedConfigurations).toBeDefined();
+      expect(config!.entityCaching?.requestScopedConfigurations).toHaveLength(2);
+      expect(config!.entityCaching!.requestScopedConfigurations![0].fieldName).toBe('currentLocale');
+      expect(config!.entityCaching!.requestScopedConfigurations![0].l1Key).toBe('subgraph-default-a.locale');
     });
 
-    test('a key declared on only one field still populates config but emits a warning', () => {
+    test('that a key declared on only one field still populates config but emits a warning', () => {
       const result = normalizeSubgraphSuccess(
         createSubgraphWithDefaultName(`
           type Query {
@@ -86,8 +85,8 @@ describe('@openfed__requestScoped', () => {
         ROUTER_COMPATIBILITY_VERSION_ONE,
       );
       const config = result.configurationDataByTypeName.get('Query');
-      expect(config!.entityCaching?.requestScopedFields).toHaveLength(1);
-      expect(config!.entityCaching!.requestScopedFields![0].l1Key).toBe('subgraph-default-a.lonely');
+      expect(config!.entityCaching?.requestScopedConfigurations).toHaveLength(1);
+      expect(config!.entityCaching!.requestScopedConfigurations![0].l1Key).toBe('subgraph-default-a.lonely');
       expect(result.warnings).toStrictEqual([
         requestScopedSingleFieldWarning({
           subgraphName: 'subgraph-default-a',
@@ -98,8 +97,8 @@ describe('@openfed__requestScoped', () => {
     });
   });
 
-  describe('validation', () => {
-    test('missing key argument is a failure', () => {
+  describe('validation tests', () => {
+    test('that a missing key argument is a failure', () => {
       const { errors } = normalizeSubgraphFailure(
         createSubgraphWithDefaultName(`
           type Query {
@@ -118,7 +117,7 @@ describe('@openfed__requestScoped', () => {
       );
     });
 
-    test('the directive is not repeatable — two on the same field fails', () => {
+    test('that the directive is not repeatable — two on the same field fails', () => {
       const { errors } = normalizeSubgraphFailure(
         createSubgraphWithDefaultName(`
           type Query {