From 038f11c750f7b0f0570a571491e3c74856da6111 Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sun, 19 Jul 2026 21:05:42 -0500 Subject: [PATCH 1/3] Validate protobuf enum prefix collisions --- .../schema_definition/schema.rb | 18 ++++++++ .../schema_elements/enum_type_extension.rb | 11 +++-- .../schema_definition/schema.rbs | 1 + .../schema_elements/enum_type_extension.rbs | 2 +- .../schema_edge_cases_spec.rb | 45 +++++++++++++++++++ 5 files changed, 72 insertions(+), 5 deletions(-) diff --git a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb index 1100ba5d9..967ac1f71 100644 --- a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb +++ b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb @@ -6,6 +6,7 @@ # # frozen_string_literal: true +require "elastic_graph/errors" require "elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension" require "elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension" require "elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension" @@ -31,6 +32,8 @@ def to_proto types = proto_types return "" if types.empty? + validate_unique_enum_value_prefixes(types) + sections = [ %(syntax = "proto3";), "package #{@package_name};", @@ -65,6 +68,21 @@ def render_definitions(types) .filter_map { |type| type.to_proto(@package_name) } .join("\n\n") end + + def validate_unique_enum_value_prefixes(types) + enum_type_by_prefix = {} # : ::Hash[::String, untyped] + + types.each do |type| + next unless type.is_a?(SchemaElements::EnumTypeExtension) + + if (existing_enum_type = enum_type_by_prefix[type.proto_enum_value_prefix]) + raise Errors::SchemaError, "Enum types `#{existing_enum_type.name}` and `#{type.name}` map to " \ + "duplicate protobuf enum value prefix `#{type.proto_enum_value_prefix}`." + end + + enum_type_by_prefix[type.proto_enum_value_prefix] = type + end + end end end end diff --git a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rb b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rb index e0275a647..da0884fd5 100644 --- a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rb +++ b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rb @@ -67,6 +67,13 @@ def proto_type_reference(package_name) ".#{package_name}.#{proto_name}" end + # Returns the package-level prefix applied to this enum's protobuf values. + # + # @return [String] + def proto_enum_value_prefix + @proto_enum_value_prefix ||= Support::Casing.to_upper_snake(name) + end + # @private def configure_derived_scalar_type(scalar_type) super @@ -107,10 +114,6 @@ def proto_zero_value_name @proto_zero_value_name ||= "#{proto_enum_value_prefix}_UNSPECIFIED" end - def proto_enum_value_prefix - @proto_enum_value_prefix ||= Support::Casing.to_upper_snake(name) - end - def values_by_proto_name @values_by_proto_name ||= {} end diff --git a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema.rbs b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema.rbs index 7ff135db9..ce7a1cdf7 100644 --- a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema.rbs +++ b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema.rbs @@ -18,6 +18,7 @@ module ElasticGraph def proto_types: () -> ::Array[untyped] def render_definitions: (::Array[untyped] types) -> ::String + def validate_unique_enum_value_prefixes: (::Array[untyped] types) -> void end end end diff --git a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rbs b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rbs index 406f72ab8..4987ddf7e 100644 --- a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rbs +++ b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rbs @@ -11,6 +11,7 @@ module ElasticGraph def value: (::String) ?{ (::ElasticGraph::SchemaDefinition::SchemaElements::EnumValue & EnumValueExtension) -> void } -> void def proto_name: () -> ::String def proto_type_reference: (::String package_name) -> ::String + def proto_enum_value_prefix: () -> ::String def to_proto: (::String package_name) -> ::String def referenced_proto_types: () -> ::Array[::ElasticGraph::SchemaDefinition::SchemaElements::graphQLType] def configure_derived_scalar_type: (::ElasticGraph::SchemaDefinition::SchemaElements::ScalarType) -> void @@ -20,7 +21,6 @@ module ElasticGraph def render_proto_enum: () -> ::String def proto_zero_value: () -> (::ElasticGraph::SchemaDefinition::SchemaElements::EnumValue & EnumValueExtension) def proto_zero_value_name: () -> ::String - def proto_enum_value_prefix: () -> ::String def values_by_proto_name: () -> ::Hash[::String, ::ElasticGraph::SchemaDefinition::SchemaElements::EnumValue & EnumValueExtension] end end diff --git a/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_edge_cases_spec.rb b/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_edge_cases_spec.rb index 0cd3434fc..ca265658c 100644 --- a/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_edge_cases_spec.rb +++ b/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_edge_cases_spec.rb @@ -49,6 +49,51 @@ module SchemaDefinition )) end + it "raises when emitted enums map to the same protobuf enum value prefix" do + expect { + define_proto_schema do |s| + s.enum_type "HttpStatus" do |t| + t.value "AVAILABLE" + end + + s.enum_type "HTTPStatus" do |t| + t.value "UNAVAILABLE" + end + + s.object_type "Endpoint" do |t| + t.field "id", "ID" + t.field "externalStatus", "HttpStatus" + t.field "internalStatus", "HTTPStatus" + t.index "endpoints" + end + end + }.to raise_error(Errors::SchemaError, a_string_including( + "Enum types `HttpStatus` and `HTTPStatus`", + "duplicate protobuf enum value prefix `HTTP_STATUS`" + )) + end + + it "allows colliding enum value prefixes when only one enum is emitted" do + proto = define_proto_schema do |s| + s.enum_type "HttpStatus" do |t| + t.value "AVAILABLE" + end + + s.enum_type "HTTPStatus" do |t| + t.value "UNAVAILABLE" + end + + s.object_type "Endpoint" do |t| + t.field "id", "ID" + t.field "status", "HttpStatus" + t.index "endpoints" + end + end + + expect(proto).to include("enum HttpStatus {") + expect(proto).not_to include("enum HTTPStatus {") + end + it "fully qualifies local type references so they do not conflict with contextual protobuf keywords" do proto = define_proto_schema do |s| s.enum_type "option" do |t| From d6bdc9537ea1ee14da739bec1ac3214a11ecb6bc Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Mon, 20 Jul 2026 14:58:16 -0500 Subject: [PATCH 2/3] Update elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb Co-authored-by: Myron Marston --- .../elastic_graph/proto_ingestion/schema_definition/schema.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb index 967ac1f71..e21573cf9 100644 --- a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb +++ b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb @@ -72,8 +72,7 @@ def render_definitions(types) def validate_unique_enum_value_prefixes(types) enum_type_by_prefix = {} # : ::Hash[::String, untyped] - types.each do |type| - next unless type.is_a?(SchemaElements::EnumTypeExtension) + types.grep(SchemaElements::EnumTypeExtension).each do |type| if (existing_enum_type = enum_type_by_prefix[type.proto_enum_value_prefix]) raise Errors::SchemaError, "Enum types `#{existing_enum_type.name}` and `#{type.name}` map to " \ From 27230af39d965b5264bd6197ff20a8d8a55c5f1a Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Mon, 20 Jul 2026 15:13:36 -0500 Subject: [PATCH 3/3] Fix enum prefix collision lint --- .../elastic_graph/proto_ingestion/schema_definition/schema.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb index e21573cf9..2458b982a 100644 --- a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb +++ b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb @@ -73,7 +73,6 @@ def validate_unique_enum_value_prefixes(types) enum_type_by_prefix = {} # : ::Hash[::String, untyped] types.grep(SchemaElements::EnumTypeExtension).each do |type| - if (existing_enum_type = enum_type_by_prefix[type.proto_enum_value_prefix]) raise Errors::SchemaError, "Enum types `#{existing_enum_type.name}` and `#{type.name}` map to " \ "duplicate protobuf enum value prefix `#{type.proto_enum_value_prefix}`."