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..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 @@ -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,19 @@ 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.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}`." + 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|