diff --git a/cli/src/commands/grpc-service/commands/generate.ts b/cli/src/commands/grpc-service/commands/generate.ts index 6080bcdda1..493e3d219d 100644 --- a/cli/src/commands/grpc-service/commands/generate.ts +++ b/cli/src/commands/grpc-service/commands/generate.ts @@ -27,7 +27,6 @@ type CLIOptions = { customScalarMapping?: string; customScalarMappingFile?: string; maxDepth?: string; - prefixOperationType?: boolean; } & ProtoOptions; export default (opts: BaseCommandOptions) => { @@ -72,11 +71,6 @@ export default (opts: BaseCommandOptions) => { 'Maximum recursion depth for processing nested selections and fragments (default: 50). ' + 'Increase this if you have deeply nested queries or decrease to catch potential circular references earlier.', ); - command.option( - '--prefix-operation-type', - 'Prefix RPC method names with the operation type (Query/Mutation). Only applies with --with-operations. ' + - 'Subscriptions are not prefixed.', - ); command.action(generateCommandAction); return command; @@ -163,11 +157,6 @@ async function generateCommandAction(name: string, options: CLIOptions) { maxDepth = parsed; } - // Validate prefix-operation-type usage - if (options.prefixOperationType && !options.withOperations) { - spinner.warn('--prefix-operation-type flag is ignored when not using --with-operations'); - } - const languageOptions: ProtoOptions = { goPackage: options.goPackage, javaPackage: options.javaPackage, @@ -192,7 +181,6 @@ async function generateCommandAction(name: string, options: CLIOptions) { operationsDir: options.withOperations, customScalarMappings, maxDepth, - prefixOperationType: options.prefixOperationType, }); // Write the generated files @@ -244,7 +232,6 @@ type GenerationOptions = { operationsDir?: string; customScalarMappings?: Record; maxDepth?: number; - prefixOperationType?: boolean; }; /** @@ -335,7 +322,6 @@ function mergeProtoRoots(roots: protobuf.Root[], serviceName: string): protobuf. * @param lockFile - Path to the proto lock file * @param customScalarMappings - Custom scalar type mappings * @param maxDepth - Maximum recursion depth - * @param prefixOperationType - Whether to prefix operation types * @returns Generation result with proto content and lock data * @note All Query operations are automatically marked with NO_SIDE_EFFECTS idempotency level */ @@ -349,7 +335,6 @@ async function generateFromOperations( lockFile: string, customScalarMappings?: Record, maxDepth?: number, - prefixOperationType?: boolean, ): Promise { spinner.text = 'Reading operation files...'; const operationFiles = await readOperationFiles(operationsPath); @@ -359,8 +344,8 @@ async function generateFromOperations( // Load lock data for field number stability let currentLockData = await fetchLockData(lockFile); - // Process each operation file separately to maintain reversibility - // Collect the AST roots instead of proto strings + // Process each operation file separately, updating lock data sequentially for field number stability. + // Collect AST roots for merging rather than proto strings. const roots: protobuf.Root[] = []; for (const { filename, content } of operationFiles) { @@ -376,7 +361,6 @@ async function generateFromOperations( lockData: currentLockData, customScalarMappings, maxDepth, - prefixOperationType, }); // Keep the AST root instead of the string @@ -495,7 +479,6 @@ async function generateProtoAndMapping({ operationsDir, customScalarMappings, maxDepth, - prefixOperationType, }: GenerationOptions): Promise { const schema = await readFile(schemaFile, 'utf8'); const serviceName = upperFirst(camelCase(name)); @@ -518,7 +501,6 @@ async function generateProtoAndMapping({ lockFile, customScalarMappings, maxDepth, - prefixOperationType, ); } else { return generateFromSDL(schema, serviceName, spinner, packageName, languageOptions, lockFile); diff --git a/protographic/OPERATIONS_TO_PROTO.md b/protographic/OPERATIONS_TO_PROTO.md index ae850b016b..9951b6f943 100644 --- a/protographic/OPERATIONS_TO_PROTO.md +++ b/protographic/OPERATIONS_TO_PROTO.md @@ -716,6 +716,6 @@ Field number conflict in message X ## Limitations - **Named operations only**: All operations must have a name. Anonymous operations are not supported -- **Single operation per document**: Multiple operations in one document are not supported for proto reversibility -- **No root-level field aliases**: Field aliases at the root level break proto-to-GraphQL reversibility +- **Single operation per document**: Multiple operations in one document are not supported for deterministic proto schema generation +- **No root-level field aliases**: Field aliases at the root level break proto schema generation consistency (each GraphQL field must map to exactly one proto field name) - **Alpha status**: The API may change in future releases diff --git a/protographic/src/naming-conventions.ts b/protographic/src/naming-conventions.ts index 1faf41942f..5715449a5d 100644 --- a/protographic/src/naming-conventions.ts +++ b/protographic/src/naming-conventions.ts @@ -26,6 +26,14 @@ export function graphqlArgumentToProtoField(argName: string): string { return snakeCase(argName); } +/** + * Converts a proto field name (snake_case) to the expected protobuf JSON format (camelCase) + * This matches how protobuf JSON marshaling automatically converts field names + */ +export function protoFieldToProtoJSON(protoFieldName: string): string { + return camelCase(protoFieldName); +} + /** * Creates an operation method name from an operation type and field name */ diff --git a/protographic/src/operation-to-proto.ts b/protographic/src/operation-to-proto.ts index 28b19f2095..cec2f5efeb 100644 --- a/protographic/src/operation-to-proto.ts +++ b/protographic/src/operation-to-proto.ts @@ -104,7 +104,7 @@ export function compileOperationsToProto( const operationNames = namedOperations.map((op) => (op as OperationDefinitionNode).name!.value).join(', '); throw new Error( `Multiple operations found in document: ${operationNames}. ` + - 'Only a single named operation per document is supported for proto reversibility. ' + + 'Only a single named operation per document is supported for deterministic proto schema generation. ' + 'Please compile each operation separately.', ); } @@ -264,21 +264,37 @@ class OperationsToProtoVisitor { return; } - // 2. Validate no root-level field aliases (breaks reversibility) + // 2. Validate operation name starts with uppercase letter + // This follows protobuf RPC naming conventions while allowing flexibility + // Must start with uppercase letter, followed by letters/numbers + if (!/^[A-Z][a-zA-Z0-9]*$/.test(operationName)) { + const suggestedName = upperFirst(camelCase(operationName)); + throw new Error( + `Operation name "${operationName}" must start with an uppercase letter ` + + `and contain only letters and numbers. ` + + `Examples: GetUser, CreatePost, HRService, GETUSER. ` + + `Suggested name: "${suggestedName}". ` + + `This follows protobuf RPC naming conventions.`, + ); + } + + // 3. Validate no root-level field aliases (breaks proto schema consistency) if (node.selectionSet) { for (const selection of node.selectionSet.selections) { if (selection.kind === 'Field' && selection.alias) { throw new Error( `Root-level field alias "${selection.alias.value}: ${selection.name.value}" is not supported. ` + - 'Field aliases at the root level break proto-to-GraphQL reversibility. ' + + 'Field aliases at the root level break proto schema generation consistency. ' + + 'Each GraphQL field must map to exactly one proto field name. ' + 'Please remove the alias or use it only on nested fields.', ); } } } - // 3. Create method name from operation name, optionally prefixed with operation type - let methodName = upperFirst(camelCase(operationName)); + // 4. Create method name from operation name + // Use operation name as-is to ensure exact matching (no transformation) + let methodName = operationName; // Add operation type prefix if requested if (this.prefixOperationType) { diff --git a/protographic/src/operations/message-builder.ts b/protographic/src/operations/message-builder.ts index a0facb7742..262f6ca73b 100644 --- a/protographic/src/operations/message-builder.ts +++ b/protographic/src/operations/message-builder.ts @@ -78,10 +78,15 @@ export function buildMessageFromSelectionSet( parentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType, typeInfo: TypeInfo, options?: MessageBuilderOptions, + parentPath?: string, ): protobuf.Type { const message = new protobuf.Type(messageName); const fieldNumberManager = options?.fieldNumberManager; + // Build the full path for lock file tracking (e.g., "ParentMessage.ChildMessage") + // This is used for field number management to prevent collisions between nested messages + const fullPath = parentPath ? `${parentPath}.${messageName}` : messageName; + // First pass: collect all field names that will be in this message const fieldNames: string[] = []; const fieldSelections = new Map(); @@ -168,14 +173,15 @@ export function buildMessageFromSelectionSet( collectFields(selectionSet.selections, parentType, currentDepth); // Reconcile field order using lock manager if available + // Use fullPath for lock file operations to track nested message hierarchy let orderedFieldNames = fieldNames; if (fieldNumberManager && 'reconcileFieldOrder' in fieldNumberManager) { - orderedFieldNames = fieldNumberManager.reconcileFieldOrder(messageName, fieldNames); + orderedFieldNames = fieldNumberManager.reconcileFieldOrder(fullPath, fieldNames); } // Second pass: process fields in reconciled order // Pre-assign field numbers from lock data if available - assignFieldNumbersFromLockData(messageName, orderedFieldNames, fieldNumberManager); + assignFieldNumbersFromLockData(fullPath, orderedFieldNames, fieldNumberManager); for (const protoFieldName of orderedFieldNames) { const fieldData = fieldSelections.get(protoFieldName); @@ -184,7 +190,15 @@ export function buildMessageFromSelectionSet( ...options, _depth: currentDepth, }; - processFieldSelection(fieldData.selection, message, fieldData.type, typeInfo, fieldOptions, fieldNumberManager); + processFieldSelection( + fieldData.selection, + message, + fieldData.type, + typeInfo, + fieldOptions, + fieldNumberManager, + fullPath, + ); } } @@ -195,23 +209,24 @@ export function buildMessageFromSelectionSet( * Gets or assigns a field number for a proto field */ function getOrAssignFieldNumber( - message: protobuf.Type, + messagePath: string, protoFieldName: string, fieldNumberManager?: FieldNumberManager, + message?: protobuf.Type, ): number { - const existingFieldNumber = fieldNumberManager?.getFieldNumber(message.name, protoFieldName); + const existingFieldNumber = fieldNumberManager?.getFieldNumber(messagePath, protoFieldName); if (existingFieldNumber !== undefined) { return existingFieldNumber; } if (fieldNumberManager) { - const fieldNumber = fieldNumberManager.getNextFieldNumber(message.name); - fieldNumberManager.assignFieldNumber(message.name, protoFieldName, fieldNumber); + const fieldNumber = fieldNumberManager.getNextFieldNumber(messagePath); + fieldNumberManager.assignFieldNumber(messagePath, protoFieldName, fieldNumber); return fieldNumber; } - return message.fieldsArray.length + 1; + return message ? message.fieldsArray.length + 1 : 1; } /** @@ -292,6 +307,7 @@ function processFieldSelection( typeInfo: TypeInfo, options?: MessageBuilderOptions, fieldNumberManager?: FieldNumberManager, + messagePath?: string, ): void { const fieldName = field.name.value; @@ -332,6 +348,8 @@ function processFieldSelection( // Build nested message for object types const namedType = getNamedType(fieldType); if (isObjectType(namedType) || isInterfaceType(namedType) || isUnionType(namedType)) { + // Use clean nested message name (just the field name in PascalCase) + // The full path will be tracked separately for lock file operations const nestedMessageName = upperFirst(camelCase(fieldName)); const nestedOptions = { @@ -339,12 +357,16 @@ function processFieldSelection( _depth: (options?._depth ?? 0) + 1, }; + // Pass the current message path to build the full hierarchy for lock file tracking + const currentPath = messagePath || message.name; + const nestedMessage = buildMessageFromSelectionSet( nestedMessageName, field.selectionSet, namedType, typeInfo, nestedOptions, + currentPath, // Pass parent path for nested message tracking ); message.add(nestedMessage); @@ -373,7 +395,9 @@ function processFieldSelection( const { typeName, isRepeated } = resolveTypeNameAndRepetition(baseTypeName, protoTypeInfo, fieldType, options); - const fieldNumber = getOrAssignFieldNumber(message, protoFieldName, fieldNumberManager); + // Use the full message path for field number assignment to prevent collisions + const currentPath = messagePath || message.name; + const fieldNumber = getOrAssignFieldNumber(currentPath, protoFieldName, fieldNumberManager, message); const protoField = createProtoField(protoFieldName, fieldNumber, typeName, isRepeated, fieldDef, options); @@ -391,6 +415,7 @@ function processInlineFragment( typeInfo: TypeInfo, options?: MessageBuilderOptions, fieldNumberManager?: FieldNumberManager, + messagePath?: string, ): void { // Determine the type for this inline fragment let fragmentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType; @@ -421,12 +446,12 @@ function processInlineFragment( if (fragment.selectionSet) { for (const selection of fragment.selectionSet.selections) { if (selection.kind === 'Field') { - processFieldSelection(selection, message, fragmentType, typeInfo, options, fieldNumberManager); + processFieldSelection(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'InlineFragment') { // Nested inline fragment - processInlineFragment(selection, message, fragmentType, typeInfo, options, fieldNumberManager); + processInlineFragment(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'FragmentSpread') { - processFragmentSpread(selection, message, fragmentType, typeInfo, options, fieldNumberManager); + processFragmentSpread(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath); } } } @@ -443,6 +468,7 @@ function processFragmentSpread( typeInfo: TypeInfo, options?: MessageBuilderOptions, fieldNumberManager?: FieldNumberManager, + messagePath?: string, ): void { const fragmentName = spread.name.value; const fragments = options?.fragments; @@ -476,12 +502,12 @@ function processFragmentSpread( // Process the fragment's selection set with the resolved type for (const selection of fragmentDef.selectionSet.selections) { if (selection.kind === 'Field') { - processFieldSelection(selection, message, type, typeInfo, options, fieldNumberManager); + processFieldSelection(selection, message, type, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'InlineFragment') { - processInlineFragment(selection, message, type, typeInfo, options, fieldNumberManager); + processInlineFragment(selection, message, type, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'FragmentSpread') { // Nested fragment spread (fragment inside fragment) - processFragmentSpread(selection, message, type, typeInfo, options, fieldNumberManager); + processFragmentSpread(selection, message, type, typeInfo, options, fieldNumberManager, messagePath); } } } diff --git a/protographic/src/operations/proto-field-options.ts b/protographic/src/operations/proto-field-options.ts new file mode 100644 index 0000000000..89c99a6d4e --- /dev/null +++ b/protographic/src/operations/proto-field-options.ts @@ -0,0 +1,19 @@ +/** + * Protocol Buffer field option constants for ConnectRPC integration. + * + * These options are defined using `extend google.protobuf.FieldOptions` in generated proto files. + */ + +export interface ProtoFieldOption { + readonly fieldNumber: number; + readonly optionName: string; +} + +/** + * Maps protobuf fields to GraphQL variable names when they don't match the expected format. + * Field number 50001 is in the user-defined extension range. + */ +export const GRAPHQL_VARIABLE_NAME: ProtoFieldOption = { + fieldNumber: 50001, + optionName: '(graphql_variable_name)', +} as const; diff --git a/protographic/src/operations/proto-text-generator.ts b/protographic/src/operations/proto-text-generator.ts index c8f245a706..02c58c698a 100644 --- a/protographic/src/operations/proto-text-generator.ts +++ b/protographic/src/operations/proto-text-generator.ts @@ -1,6 +1,7 @@ import protobuf from 'protobufjs'; import { buildProtoOptions } from '../proto-options.js'; import { MethodWithIdempotency } from '../types.js'; +import { GRAPHQL_VARIABLE_NAME } from './proto-field-options.js'; /** * Helper to format indentation @@ -94,6 +95,13 @@ function generateHeader(root: protobuf.Root, options?: ProtoTextOptions): string // Imports const imports = new Set(); + // Check if any field uses graphql_variable_name option + const usesGraphQLVariableName = detectGraphQLVariableNameUsage(root); + if (usesGraphQLVariableName) { + // Import descriptor.proto for field option extension + imports.add('google/protobuf/descriptor.proto'); + } + // Only add wrapper types import if actually used if (detectWrapperTypeUsage(root)) { imports.add('google/protobuf/wrappers.proto'); @@ -112,6 +120,15 @@ function generateHeader(root: protobuf.Root, options?: ProtoTextOptions): string lines.push(''); } + // Add field option extension if used + if (usesGraphQLVariableName) { + lines.push('// Field option extension for GraphQL variable name mapping'); + lines.push('extend google.protobuf.FieldOptions {'); + lines.push(' string graphql_variable_name = 50001;'); + lines.push('}'); + lines.push(''); + } + // Options - use shared utility for standard options const protoOptions: string[] = buildProtoOptions( { @@ -285,7 +302,24 @@ export function formatField(field: protobuf.Field, options?: ProtoTextOptions, i // Build field line const repeated = field.repeated ? 'repeated ' : ''; - lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id};`)); + + // Check if field has options + if (field.options && Object.keys(field.options).length > 0) { + // Field with options - format with brackets + const optionsStr = Object.entries(field.options) + .map(([key, value]) => { + // The key already includes parentheses if it's an extension option + // e.g., "(graphql_variable_name)" + // Handle string values with quotes + const formattedValue = typeof value === 'string' ? `"${value}"` : value; + return `${key} = ${formattedValue}`; + }) + .join(', '); + lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id} [${optionsStr}];`)); + } else { + // Field without options + lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id};`)); + } return lines; } @@ -419,6 +453,43 @@ function messageUsesWrapperTypes(message: protobuf.Type): boolean { return false; } +/** + * Detects if any field in the root uses the graphql_variable_name option + */ +function detectGraphQLVariableNameUsage(root: protobuf.Root): boolean { + for (const nested of root.nestedArray) { + if (nested instanceof protobuf.Type) { + if (messageUsesGraphQLVariableName(nested)) { + return true; + } + } + } + return false; +} + +/** + * Recursively checks if a message or its nested messages use graphql_variable_name option + */ +function messageUsesGraphQLVariableName(message: protobuf.Type): boolean { + // Check fields in this message + for (const field of message.fieldsArray) { + if (field.options && field.options[GRAPHQL_VARIABLE_NAME.optionName]) { + return true; + } + } + + // Check nested messages recursively + for (const nested of message.nestedArray) { + if (nested instanceof protobuf.Type) { + if (messageUsesGraphQLVariableName(nested)) { + return true; + } + } + } + + return false; +} + /** * Helper to format method definitions for services */ diff --git a/protographic/src/operations/request-builder.ts b/protographic/src/operations/request-builder.ts index 7e5983230a..22d5b84337 100644 --- a/protographic/src/operations/request-builder.ts +++ b/protographic/src/operations/request-builder.ts @@ -18,7 +18,9 @@ import { graphqlArgumentToProtoField, createEnumUnspecifiedValue, graphqlEnumValueToProtoEnumValue, + protoFieldToProtoJSON, } from '../naming-conventions.js'; +import { GRAPHQL_VARIABLE_NAME } from './proto-field-options.js'; /** * Options for building request messages @@ -155,6 +157,18 @@ export function buildVariableField( field.repeated = true; } + // Add wundergraph.connectrpc.graphql_variable_name option if the GraphQL variable name doesn't match + // the expected protobuf JSON format (camelCase of snake_case field name) + const expectedProtoJSON = protoFieldToProtoJSON(protoFieldName); + if (variableName !== expectedProtoJSON) { + // Store the GraphQL variable name as a custom option + // This will be used by the handler to map proto JSON to GraphQL variables + if (!field.options) { + field.options = {}; + } + field.options[GRAPHQL_VARIABLE_NAME.optionName] = variableName; + } + return field; } diff --git a/protographic/tests/operations/fragments.test.ts b/protographic/tests/operations/fragments.test.ts index 26b388b461..d742496222 100644 --- a/protographic/tests/operations/fragments.test.ts +++ b/protographic/tests/operations/fragments.test.ts @@ -1263,7 +1263,7 @@ describe('Fragment Support', () => { message Child { message Child { string id = 1; - google.protobuf.StringValue value = 4; + google.protobuf.StringValue value = 2; } string id = 1; google.protobuf.StringValue name = 2; diff --git a/protographic/tests/operations/nested-message-field-numbering.test.ts b/protographic/tests/operations/nested-message-field-numbering.test.ts new file mode 100644 index 0000000000..6b7302d8d6 --- /dev/null +++ b/protographic/tests/operations/nested-message-field-numbering.test.ts @@ -0,0 +1,502 @@ +import { describe, expect, test } from 'vitest'; +import { compileOperationsToProto } from '../../src'; +import { expectValidProto } from '../util'; + +describe('Nested Message Field Numbering', () => { + test('should assign field numbers starting from 1 in a simple nested message', () => { + const schema = ` + type Query { + getEmployee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + forename: String + surname: String + } + `; + + const operation = ` + query GetEmployee { + getEmployee { + details { + forename + surname + } + } + } + `; + + const { proto, lockData } = compileOperationsToProto(operation, schema); + + expectValidProto(proto); + + expect(proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetEmployee(GetEmployeeRequest) returns (GetEmployeeResponse) {} + } + + message GetEmployeeRequest { + } + + message GetEmployeeResponse { + message GetEmployee { + message Details { + google.protobuf.StringValue forename = 1; + google.protobuf.StringValue surname = 2; + } + Details details = 1; + } + GetEmployee get_employee = 1; + } + " + `); + + // Verify lock file uses dot notation for nested message path + expect(lockData).toBeDefined(); + expect(lockData!.messages['GetEmployeeResponse.GetEmployee.Details']).toMatchInlineSnapshot(` + { + "fields": { + "forename": 1, + "surname": 2, + }, + } + `); + }); + + test('should not share field number counters between sibling nested messages with same name', () => { + const schema = ` + type Query { + getData: GetData + } + + type GetData { + userInfo: UserInfo + productInfo: ProductInfo + } + + type UserInfo { + details: UserDetails + } + + type UserDetails { + name: String + email: String + } + + type ProductInfo { + details: ProductDetails + } + + type ProductDetails { + title: String + price: Float + } + `; + + const operation = ` + query GetData { + getData { + userInfo { + details { + name + email + } + } + productInfo { + details { + title + price + } + } + } + } + `; + + const { proto, lockData } = compileOperationsToProto(operation, schema); + + expectValidProto(proto); + + expect(proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetData(GetDataRequest) returns (GetDataResponse) {} + } + + message GetDataRequest { + } + + message GetDataResponse { + message GetData { + message UserInfo { + message Details { + google.protobuf.StringValue name = 1; + google.protobuf.StringValue email = 2; + } + Details details = 1; + } + message ProductInfo { + message Details { + google.protobuf.StringValue title = 1; + google.protobuf.DoubleValue price = 2; + } + Details details = 1; + } + UserInfo user_info = 1; + ProductInfo product_info = 2; + } + GetData get_data = 1; + } + " + `); + + // Verify both Details messages have independent field numbering in lock file + // Each starts from 1 because they're tracked separately using full dot-notation paths + expect(lockData!.messages['GetDataResponse.GetData.UserInfo.Details']).toMatchInlineSnapshot(` + { + "fields": { + "email": 2, + "name": 1, + }, + } + `); + expect(lockData!.messages['GetDataResponse.GetData.ProductInfo.Details']).toMatchInlineSnapshot(` + { + "fields": { + "price": 2, + "title": 1, + }, + } + `); + }); + + test('should preserve existing field numbers when adding new fields to nested messages', () => { + const schema1 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + } + `; + + const operation1 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + } + } + } + } + `; + + const result1 = compileOperationsToProto(operation1, schema1); + + // Now add a field to the nested Details message + const schema2 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + email: String + } + `; + + const operation2 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + email + } + } + } + } + `; + + const result2 = compileOperationsToProto(operation2, schema2, { lockData: result1.lockData }); + + expectValidProto(result2.proto); + + expect(result2.proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetEmployee(GetEmployeeRequest) returns (GetEmployeeResponse) {} + } + + message GetEmployeeRequest { + } + + message GetEmployeeResponse { + message GetEmployee { + message Employee { + message Details { + google.protobuf.StringValue name = 1; + google.protobuf.StringValue email = 2; + } + Details details = 1; + } + Employee employee = 1; + } + GetEmployee get_employee = 1; + } + " + `); + + // Verify field numbers are stable: name keeps 1, email gets 2 + expect(result2.lockData!.messages['GetEmployeeResponse.GetEmployee.Employee.Details']).toMatchInlineSnapshot(` + { + "fields": { + "email": 2, + "name": 1, + }, + } + `); + }); + + test('should mark removed field numbers as reserved in nested messages', () => { + const schema1 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + email: String + phone: String + } + `; + + const operation1 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + email + phone + } + } + } + } + `; + + const result1 = compileOperationsToProto(operation1, schema1); + + // Remove the email field + const schema2 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + phone: String + } + `; + + const operation2 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + phone + } + } + } + } + `; + + const result2 = compileOperationsToProto(operation2, schema2, { lockData: result1.lockData }); + + expectValidProto(result2.proto); + + expect(result2.proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetEmployee(GetEmployeeRequest) returns (GetEmployeeResponse) {} + } + + message GetEmployeeRequest { + } + + message GetEmployeeResponse { + message GetEmployee { + message Employee { + message Details { + google.protobuf.StringValue name = 1; + google.protobuf.StringValue phone = 3; + } + Details details = 1; + } + Employee employee = 1; + } + GetEmployee get_employee = 1; + } + " + `); + + // Verify field number 2 (email) is reserved for backward compatibility + expect(result2.lockData!.messages['GetEmployeeResponse.GetEmployee.Employee.Details']).toMatchInlineSnapshot(` + { + "fields": { + "name": 1, + "phone": 3, + }, + "reservedNumbers": [ + 2, + ], + } + `); + }); + + test('should correctly track field numbers in deeply nested message hierarchies (5+ levels)', () => { + const schema = ` + type Query { + getDeep: Level1 + } + + type Level1 { + level2: Level2 + } + + type Level2 { + level3: Level3 + } + + type Level3 { + level4: Level4 + } + + type Level4 { + level5: Level5 + } + + type Level5 { + value: String + } + `; + + const operation = ` + query GetDeep { + getDeep { + level2 { + level3 { + level4 { + level5 { + value + } + } + } + } + } + } + `; + + const { proto, lockData } = compileOperationsToProto(operation, schema); + + expectValidProto(proto); + + expect(proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetDeep(GetDeepRequest) returns (GetDeepResponse) {} + } + + message GetDeepRequest { + } + + message GetDeepResponse { + message GetDeep { + message Level2 { + message Level3 { + message Level4 { + message Level5 { + google.protobuf.StringValue value = 1; + } + Level5 level_5 = 1; + } + Level4 level_4 = 1; + } + Level3 level_3 = 1; + } + Level2 level_2 = 1; + } + GetDeep get_deep = 1; + } + " + `); + + // Verify the deepest nested message uses full dot-notation path in lock file + expect(lockData!.messages['GetDeepResponse.GetDeep.Level2.Level3.Level4.Level5']).toMatchInlineSnapshot(` + { + "fields": { + "value": 1, + }, + } + `); + }); +}); diff --git a/protographic/tests/operations/operation-validation.test.ts b/protographic/tests/operations/operation-validation.test.ts index a93d436505..004882ab85 100644 --- a/protographic/tests/operations/operation-validation.test.ts +++ b/protographic/tests/operations/operation-validation.test.ts @@ -178,8 +178,8 @@ describe('Operation Validation', () => { }); }); - describe('Reversibility Considerations', () => { - test('should allow single operation for proto-to-graphql reversibility', () => { + describe('Proto Schema Consistency', () => { + test('should allow single operation for deterministic proto schema generation', () => { const operation = ` query GetUser { user { @@ -198,6 +198,168 @@ describe('Operation Validation', () => { }); }); + describe('Operation Name PascalCase Validation', () => { + test('should accept PascalCase operation names', () => { + const operation = ` + query GetUser { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).not.toThrow(); + }); + + test('should accept PascalCase with numbers', () => { + const operation = ` + query GetUser123 { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).not.toThrow(); + }); + + test('should reject camelCase operation names', () => { + const operation = ` + query getUser { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/Operation name "getUser" must start with an uppercase letter/); + }); + + test('should reject snake_case operation names', () => { + const operation = ` + query get_user { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/Operation name "get_user" must start with an uppercase letter/); + }); + + test('should accept all-UPPERCASE operation names', () => { + const operation = ` + query GETUSER { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).not.toThrow(); + }); + + test('should accept operation names with only uppercase and numbers', () => { + const operation = ` + query GET123USER { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).not.toThrow(); + }); + + test('should provide helpful error message for camelCase', () => { + const operation = ` + query getUserById { + user { + id + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/must start with an uppercase letter.*Examples: GetUser, CreatePost, HRService, GETUSER/); + }); + + test('should validate mutation operation names', () => { + const mutationSchema = ` + type Mutation { + createUser(name: String!): User + } + + type User { + id: ID! + name: String + } + `; + + const operation = ` + mutation createUser($name: String!) { + createUser(name: $name) { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, mutationSchema); + }).toThrow(/Operation name "createUser" must start with an uppercase letter/); + }); + + test('should validate subscription operation names', () => { + const subscriptionSchema = ` + type Query { + ping: String + } + + type Subscription { + messageAdded: Message + } + + type Message { + id: ID! + content: String + } + `; + + const operation = ` + subscription onMessageAdded { + messageAdded { + id + content + } + } + `; + + expect(() => { + compileOperationsToProto(operation, subscriptionSchema); + }).toThrow(/Operation name "onMessageAdded" must start with an uppercase letter/); + }); + }); + describe('Root-Level Field Aliases', () => { test('should reject root-level field aliases', () => { const operation = `