Skip to content

Commit 170323f

Browse files
committed
Reference external proto enum types
Some schemas already have canonical protobuf enum definitions, and generating duplicate enums in `schema.proto` makes consumers deal with parallel types. Passing `proto:` and `import:` to `external_proto_enum` imports the named proto file and references the existing enum type directly instead of generating a local definition. A referenced enum must have exactly one option-free source whose values match the ElasticGraph enum's values, so transformed or curated enums stay generated locally. Since the API lives on enum types, non-enum external references are impossible by construction rather than needing validation.
1 parent 7e7bbb1 commit 170323f

8 files changed

Lines changed: 473 additions & 32 deletions

File tree

config/site/support/doctest_helper.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,12 @@ module ElasticGraph
107107
extend ::RSpec::Mocks::ExampleMethods
108108

109109
# The examples source enum values from an app-defined proto enum class; provide one here.
110-
proto_enum_entry = ::Data.define(:name)
110+
proto_enum_entry = ::Data.define(:name, :number)
111111
currency_proto_enum = ::Class.new
112112
currency_proto_enum.define_singleton_method(:enums) do
113-
[:CURRENCY_UNKNOWN_DO_NOT_USE, :CURRENCY_USD, :CURRENCY_CAD].map { |name| proto_enum_entry.new(name: name) }
113+
[:CURRENCY_UNKNOWN_DO_NOT_USE, :CURRENCY_USD, :CURRENCY_CAD].each_with_index.map do |name, number|
114+
proto_enum_entry.new(name: name, number: number)
115+
end
114116
end
115117

116118
stub_const("MyApp::Protos::Currency", currency_proto_enum)

elasticgraph-proto_ingestion/README.md

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,12 +219,14 @@ In the examples below, this stand-in plays the role of your app's generated prot
219219
module MyApp
220220
module Protos
221221
class Currency
222-
EnumEntry = ::Data.define(:name)
222+
EnumEntry = ::Data.define(:name, :number)
223223
224224
def self.enums
225-
[:CURRENCY_UNKNOWN_DO_NOT_USE, :CURRENCY_USD, :CURRENCY_CAD].map do |name|
226-
EnumEntry.new(name: name)
227-
end
225+
[
226+
EnumEntry.new(name: :CURRENCY_UNKNOWN_DO_NOT_USE, number: 0),
227+
EnumEntry.new(name: :CURRENCY_USD, number: 1),
228+
EnumEntry.new(name: :CURRENCY_CAD, number: 2)
229+
]
228230
end
229231
end
230232
end
@@ -254,6 +256,33 @@ When an enum has one or more external sources, `elasticgraph-proto_ingestion` us
254256
as the source of the generated enum's values. When multiple sources are registered for the
255257
same enum, they must all resolve to the same value set.
256258

259+
### Referencing Existing Protobuf Types
260+
261+
For enums that exactly match a canonical proto enum, you can go further and reference the
262+
existing proto type instead of generating a duplicate local enum. Pass `proto:` and `import:`
263+
to `external_proto_enum`, and `schema.proto` will import the named file and use the external
264+
type name directly:
265+
266+
```ruby
267+
# in config/schema/currency.rb
268+
269+
ElasticGraph.define_schema do |schema|
270+
schema.enum_type "Currency" do |t|
271+
t.values "USD", "CAD"
272+
t.external_proto_enum MyApp::Protos::Currency,
273+
proto: "myapp.types.Currency",
274+
import: "myapp/types/currency.proto"
275+
end
276+
end
277+
```
278+
279+
A referenced enum must have exactly one option-free source whose values match the
280+
ElasticGraph enum's values; transformed, curated, or multi-source enums stay generated
281+
locally so value curation remains explicit. The source's enum entries must also expose
282+
`.number`, and those numbers must agree with any numbers previously pinned in
283+
`proto_field_numbers.yaml` — otherwise switching to the external type would silently
284+
reinterpret existing wire data.
285+
257286
## Type Mappings
258287

259288
The generated `schema.proto` uses these built-in scalar mappings:

elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,13 @@ def enum_value_numbers_for(enum_name, value_names)
157157
end
158158
end
159159

160+
# Returns previously pinned numbers for a protobuf enum.
161+
#
162+
# @api private
163+
def pinned_enum_value_numbers(enum_name)
164+
@proto_enum_value_numbers_by_enum[enum_name] || {}
165+
end
166+
160167
# Returns the label for a protobuf field under the configured syntax.
161168
#
162169
# @api private

elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rb

Lines changed: 86 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,33 @@ module EnumTypeExtension
2121
# Describes an external proto enum registered as the source of this enum's generated values.
2222
ExternalProtoEnumSource = ::Data.define(:proto_enum, :exclusions, :expected_extras, :name_transform)
2323

24-
# Sources this enum's generated proto values from an existing proto enum class.
24+
# Describes an existing proto enum type referenced instead of generating a local definition.
25+
ExternalProtoEnumReference = ::Data.define(:proto_name, :import)
26+
27+
# Sources this enum's generated proto values from an existing proto enum class. Passing
28+
# `proto:` and `import:` references that enum directly instead of generating a local enum.
2529
#
2630
# @return [void]
27-
def external_proto_enum(proto_enum, exclusions: [], expected_extras: [], name_transform: nil)
31+
def external_proto_enum(proto_enum, exclusions: [], expected_extras: [], name_transform: nil, proto: nil, import: nil)
2832
unless proto_enum.respond_to?(:enums)
2933
raise Errors::SchemaError, "`external_proto_enum` on `#{name}` must be given a proto enum class with `.enums`, " \
3034
"but got: #{proto_enum.inspect}."
3135
end
3236

37+
if proto || import
38+
unless proto.is_a?(String) && !proto.empty? && import.is_a?(String) && !import.empty?
39+
raise Errors::SchemaError, "`external_proto_enum` on `#{name}` must be given both `proto` and `import` " \
40+
"as non-empty Strings to reference an external proto enum type."
41+
end
42+
unless exclusions.empty? && expected_extras.empty? && name_transform.nil?
43+
raise Errors::SchemaError, "`external_proto_enum` on `#{name}` cannot combine `proto`/`import` with " \
44+
"`exclusions`, `expected_extras`, or `name_transform`; transformed or curated enums must stay generated locally."
45+
end
46+
47+
@external_proto_reference = ExternalProtoEnumReference.new(proto_name: proto, import: import)
48+
@proto_name = nil
49+
end
50+
3351
external_proto_enum_sources << ExternalProtoEnumSource.new(
3452
proto_enum: proto_enum,
3553
exclusions: exclusions.map(&:to_s),
@@ -46,6 +64,12 @@ def external_proto_enum_sources
4664
@external_proto_enum_sources ||= []
4765
end
4866

67+
# The external proto enum type referenced instead of generating a local enum, if configured.
68+
#
69+
# @dynamic external_proto_reference
70+
# @return [ExternalProtoEnumReference, nil]
71+
attr_reader :external_proto_reference
72+
4973
# Defines an enum value and immediately validates its protobuf name.
5074
#
5175
# @return [void]
@@ -70,6 +94,11 @@ def value(value_name, &block)
7094
#
7195
# @return [String]
7296
def to_proto(schema)
97+
if external_proto_reference
98+
validate_external_proto_reference(schema)
99+
return nil
100+
end
101+
73102
render_proto_enum(schema)
74103
end
75104

@@ -91,7 +120,14 @@ def referenced_proto_types
91120
#
92121
# @return [String]
93122
def proto_name
94-
@proto_name ||= Identifier.enum_name(name)
123+
@proto_name ||= external_proto_reference&.proto_name || Identifier.enum_name(name)
124+
end
125+
126+
# Returns the proto file imported for an externally referenced enum.
127+
#
128+
# @return [String, nil]
129+
def protobuf_import
130+
external_proto_reference&.import
95131
end
96132

97133
# @private
@@ -161,11 +197,57 @@ def proto_enum_value_names
161197

162198
def enum_value_names_from_source(source)
163199
name_transform = source.name_transform || :itself.to_proc
164-
mapped_values = source.proto_enum.enums.map { |enum_entry| name_transform.call(enum_entry.name.to_s).to_s }
200+
mapped_values = source.proto_enum.enums.map { |entry| name_transform.call(entry.name.to_s).to_s }
165201
(mapped_values - source.exclusions + source.expected_extras).uniq
166202
rescue => e
167203
raise Errors::SchemaError, "Failed loading external proto enum values for `#{name}` from `#{source.proto_enum}`: #{e.message}"
168204
end
205+
206+
def validate_external_proto_reference(schema)
207+
unless external_proto_enum_sources.one?
208+
raise Errors::SchemaError, "External proto enum `#{name}` must use exactly one `external_proto_enum` " \
209+
"source; multi-source enums cannot be safely referenced externally."
210+
end
211+
212+
numbers_by_value_name = enum_value_numbers_from_source(external_proto_enum_sources.first)
213+
external_names = numbers_by_value_name.keys.sort
214+
elasticgraph_names = values_by_name.keys.map(&:to_s).uniq.sort
215+
if external_names != elasticgraph_names
216+
raise Errors::SchemaError, "External proto enum `#{name}` values do not match the ElasticGraph enum values. " \
217+
"External values: #{external_names.join(", ")}. ElasticGraph values: #{elasticgraph_names.join(", ")}."
218+
end
219+
220+
schema.pinned_enum_value_numbers(Identifier.enum_name(name)).each do |value_name, pinned_number|
221+
external_number = numbers_by_value_name[value_name]
222+
if external_number && external_number != pinned_number
223+
raise Errors::SchemaError, "External proto enum `#{name}` assigns `#{value_name}` the number " \
224+
"#{external_number}, but previously dumped artifacts pin it to #{pinned_number}; referencing it would " \
225+
"silently reinterpret existing wire data."
226+
end
227+
228+
conflicting_name = numbers_by_value_name.find do |external_name, number|
229+
external_name != value_name && number == pinned_number
230+
end&.first
231+
if conflicting_name
232+
raise Errors::SchemaError, "External proto enum `#{name}` assigns `#{conflicting_name}` the number " \
233+
"#{pinned_number}, which previously dumped artifacts pin to `#{value_name}`; referencing it would " \
234+
"silently reinterpret existing wire data."
235+
end
236+
end
237+
end
238+
239+
def enum_value_numbers_from_source(source)
240+
entries = source.proto_enum.enums
241+
unless entries.all? { |entry| entry.respond_to?(:number) }
242+
raise Errors::SchemaError, "External proto enum `#{name}` cannot be referenced: its enum entries " \
243+
"must expose `.number` so its values can be verified against previously pinned numbers."
244+
end
245+
entries.to_h { |entry| [entry.name.to_s, entry.number] }
246+
rescue Errors::SchemaError
247+
raise
248+
rescue => e
249+
raise Errors::SchemaError, "Failed loading external proto enum values for `#{name}` from `#{source.proto_enum}`: #{e.message}"
250+
end
169251
end
170252
end
171253
end

elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema.rbs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ module ElasticGraph
4545
name_in_index: ::String
4646
) -> ::Integer
4747
def enum_value_numbers_for: (::String, ::Array[::String]) -> ::Hash[::String, ::Integer]
48+
def pinned_enum_value_numbers: (::String) -> ::Hash[::String, ::Integer]
4849
def field_label: (bool) -> ::String?
4950

5051
private

elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rbs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,24 @@ module ElasticGraph
1717
) -> instance
1818
end
1919

20+
class ExternalProtoEnumReference
21+
attr_reader proto_name: ::String
22+
attr_reader import: ::String
23+
24+
def self.new: (proto_name: ::String, import: ::String) -> instance
25+
end
26+
2027
@proto_name: ::String?
2128
@external_proto_enum_sources: ::Array[ExternalProtoEnumSource]?
29+
@external_proto_reference: ExternalProtoEnumReference?
2230

23-
def external_proto_enum: (untyped, ?exclusions: ::Array[::String | ::Symbol], ?expected_extras: ::Array[::String | ::Symbol], ?name_transform: untyped) -> void
31+
def external_proto_enum: (untyped, ?exclusions: ::Array[::String | ::Symbol], ?expected_extras: ::Array[::String | ::Symbol], ?name_transform: untyped, ?proto: ::String?, ?import: ::String?) -> void
2432
def external_proto_enum_sources: () -> ::Array[ExternalProtoEnumSource]
33+
attr_reader external_proto_reference: ExternalProtoEnumReference?
2534
def value: (::String) ?{ (::ElasticGraph::SchemaDefinition::SchemaElements::EnumValue & EnumValueExtension) -> void } -> void
2635
def proto_name: () -> ::String
27-
def to_proto: (Schema) -> ::String
36+
def protobuf_import: () -> ::String?
37+
def to_proto: (Schema) -> ::String?
2838
def proto_definition_kind: () -> :enum
2939
def referenced_proto_types: () -> ::Array[::ElasticGraph::SchemaDefinition::SchemaElements::graphQLType]
3040
def configure_derived_scalar_type: (::ElasticGraph::SchemaDefinition::SchemaElements::ScalarType) -> void
@@ -36,6 +46,8 @@ module ElasticGraph
3646
def proto_enum_value_name: (::String) -> ::String
3747
def proto_enum_value_names: () -> ::Array[::String]
3848
def enum_value_names_from_source: (ExternalProtoEnumSource) -> ::Array[::String]
49+
def validate_external_proto_reference: (Schema) -> void
50+
def enum_value_numbers_from_source: (ExternalProtoEnumSource) -> ::Hash[::String, ::Integer]
3951
end
4052
end
4153
end

0 commit comments

Comments
 (0)