diff --git a/elasticgraph-proto_ingestion/README.md b/elasticgraph-proto_ingestion/README.md index aef7b2f9c..eb0bcddbc 100644 --- a/elasticgraph-proto_ingestion/README.md +++ b/elasticgraph-proto_ingestion/README.md @@ -94,27 +94,93 @@ ElasticGraph.define_schema do |schema| end ``` +A custom scalar can also map to an externally defined proto type by passing `import:` with the +proto file that defines it, and `comment:` documents the expected format on each generated field +(useful when the proto type is wider than the ElasticGraph type): + +```ruby +# in config/schema/phone_number.rb + +ElasticGraph.define_schema do |schema| + schema.scalar_type "PhoneNumber" do |t| + t.mapping type: "keyword" + t.json_schema type: "string" + t.protobuf type: "string", comment: "E.164 phone number" + end +end +``` + +### Stable Field Numbers + +`schema_artifacts:dump` automatically reads and writes `proto_field_numbers.yaml` +in the schema artifacts directory. Existing numbers stay fixed even if field order +changes, and new fields get the next available numbers. Field numbers follow protobuf's +rules: they must be between 1 and 536,870,911, and the protobuf-reserved 19000-19999 +range is never allocated and is rejected in mappings. + +Alternatives inside generated interface and union `oneof` blocks use the same stable +message-field mappings, so adding or removing a concrete subtype does not renumber the +remaining alternatives. + +`schema.proto` always uses the public GraphQL field names. When a field uses a +different `name_in_index`, the sidecar YAML stores that override privately: + +```yaml +messages: + Widget: + fields: + id: 1 + display_name: + field_number: 2 + name_in_index: displayName +``` + +If a field is renamed with `field.renamed_from`, `elasticgraph-proto_ingestion` reuses the +existing field number under the new public field name. + +### Stable Enum Value Numbers + +Enum value numbers are pinned the same way, in an `enums` section of the sidecar. Existing +values keep their numbers when other values are added or removed, new values get the next +available numbers, and removed values keep their numbers reserved so they are never reused +(number `0` is always the generated `*_UNSPECIFIED` value): + +```yaml +enums: + WidgetColor: + values: + RED: 1 + BLUE: 2 +``` + ## Type Mappings The generated `schema.proto` uses these built-in scalar mappings: -| ElasticGraph Type | Protobuf Type | -|-------------------|------------| -| `Boolean` | `bool` | -| `Cursor` | `string` | -| `Date` | `string` | -| `DateTime` | `string` | -| `Float` | `double` | -| `ID` | `string` | -| `Int` | `int32` | -| `JsonSafeLong` | `int64` | -| `LocalTime` | `string` | -| `LongString` | `int64` | -| `String` | `string` | -| `TimeZone` | `string` | -| `Untyped` | `string` | +| ElasticGraph Type | Protobuf Type | +|-------------------|-----------------------------| +| `Boolean` | `bool` | +| `Cursor` | `string` | +| `Date` | `string` | +| `DateTime` | `google.protobuf.Timestamp` | +| `Float` | `double` | +| `ID` | `string` | +| `Int` | `int32` | +| `JsonSafeLong` | `int64` | +| `LocalTime` | `string` | +| `LongString` | `int64` | +| `String` | `string` | +| `TimeZone` | `string` | +| `Untyped` | `string` | Additionally: +- `DateTime` uses the [well-known `Timestamp` type](https://protobuf.dev/reference/protobuf/google.protobuf/#timestamp); + `schema.proto` imports `google/protobuf/timestamp.proto` automatically. Note that a `Timestamp` + is a UTC instant, so a publisher's original UTC offset is not preserved. +- `string`-typed temporal scalars (`Date`, `LocalTime`, `TimeZone`) are wider than the + ElasticGraph types they carry, so generated fields of these types document the expected format + in a comment (e.g. `// ISO 8601 date, e.g. "2024-11-25"`). Values are validated when events + are ingested, just as with JSON ingestion. - List types become `repeated` fields. - Lists of lists (e.g. `[[Float!]!]!`) are not supported because Protocol Buffers cannot represent them directly. Schema artifact generation raises an error identifying the unsupported field. 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 af7f76221..afe0837e1 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 @@ -45,6 +45,7 @@ def to_proto sections = [ %(syntax = "proto3";), "package #{@package_name};", + *render_imports(types), render_definitions(types) ] @@ -103,6 +104,14 @@ def render_definitions(types) .join("\n\n") end + def render_imports(types) + imports = types.filter_map do |type| + type.protobuf_import if type.respond_to?(:protobuf_import) + end.uniq.sort + + imports.empty? ? [] : [imports.map { |import| %(import "#{import}";) }.join("\n")] + end + def validate_unique_enum_value_prefixes(types) enum_type_by_prefix = {} # : ::Hash[::String, untyped] diff --git a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rb b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rb index d75d3908f..fd64cf730 100644 --- a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rb +++ b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rb @@ -74,7 +74,7 @@ def render_proto_message(schema, message_name, package_name) fields = proto_fields documentation = ProtoDocumentation.comment_lines_for(doc_comment).map { |line| "#{line}\n" }.join field_definitions = fields.map do |schema_field, field| - repeated, field_type = proto_field_type_for( + repeated, field_type, type_comment = proto_field_type_for( field.type, package_name: package_name, context_field_name: field.name @@ -87,6 +87,7 @@ def render_proto_message(schema, message_name, package_name) ) label = "repeated " if repeated line = " #{label}#{field_type} #{schema_field.name} = #{field_number};" + line += " // #{type_comment}" if type_comment field_documentation = ProtoDocumentation .comment_lines_for(schema_field.doc_comment, indent: " ") .map { |comment_line| "#{comment_line}\n" } @@ -152,7 +153,8 @@ def proto_field_type_for(type_ref, package_name:, context_field_name:) end proto_type = _ = base_type_ref.resolved - [list_depth == 1, proto_type.proto_type_reference(package_name)] + type_comment = (ScalarTypeExtension === proto_type) ? proto_type.protobuf_comment : nil + [list_depth == 1, proto_type.proto_type_reference(package_name), type_comment] end end end diff --git a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rb b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rb index 404258788..48e6c0041 100644 --- a/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rb +++ b/elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rb @@ -14,33 +14,45 @@ module SchemaDefinition module SchemaElements # Extends ScalarType with proto field type conversion. module ScalarTypeExtension - # Default protobuf types applied to ElasticGraph's built-in scalar types as they are constructed. - BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME = { - "Boolean" => "bool", - "Cursor" => "string", - "Date" => "string", - "DateTime" => "string", - "Float" => "double", - "ID" => "string", - "Int" => "int32", - "JsonSafeLong" => "int64", - "LocalTime" => "string", - "LongString" => "int64", - "String" => "string", - "TimeZone" => "string", - "Untyped" => "string" + # Default protobuf options applied to ElasticGraph's built-in scalar types as they are constructed. + BUILT_IN_SCALAR_PROTO_OPTIONS_BY_NAME = { + "Boolean" => {type: "bool"}, + "Cursor" => {type: "string"}, + "Date" => {type: "string", comment: %(ISO 8601 date, e.g. "2024-11-25")}, + "DateTime" => {type: "google.protobuf.Timestamp", import: "google/protobuf/timestamp.proto"}, + "Float" => {type: "double"}, + "ID" => {type: "string"}, + "Int" => {type: "int32"}, + "JsonSafeLong" => {type: "int64"}, + "LocalTime" => {type: "string", comment: %(ISO 8601 local time, e.g. "14:23:12")}, + "LongString" => {type: "int64"}, + "String" => {type: "string"}, + "TimeZone" => {type: "string", comment: %(IANA time zone identifier, e.g. "America/Los_Angeles")}, + "Untyped" => {type: "string"} }.freeze # Configured protobuf type (e.g. string, int64, bool). # @dynamic protobuf_type attr_reader :protobuf_type + # Proto file to import for the configured protobuf type, if it is externally defined. + # @dynamic protobuf_import + attr_reader :protobuf_import + + # Comment rendered on generated proto fields of this scalar type. + # @dynamic protobuf_comment + attr_reader :protobuf_comment + # Configures the protobuf type for this scalar type. # - # @param type [String] protobuf scalar type name + # @param type [String] protobuf type name + # @param import [String, nil] proto file to import for an externally defined type + # @param comment [String, nil] comment rendered on generated fields of this type # @return [void] - def protobuf(type:) + def protobuf(type:, import: nil, comment: nil) @protobuf_type = type + @protobuf_import = import + @protobuf_comment = comment end # Applies any built-in protobuf type, yields for further configuration, and validates the result. @@ -50,8 +62,12 @@ def protobuf(type:) # @raise [Errors::SchemaError] when a protobuf type is missing def initialize_proto_extension original_name = type_ref.with_reverted_override.name - if (proto_type = BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME[original_name]) - protobuf type: proto_type + if (proto_options = BUILT_IN_SCALAR_PROTO_OPTIONS_BY_NAME[original_name]) + protobuf( + type: proto_options.fetch(:type), + import: proto_options[:import], + comment: proto_options[:comment] + ) end yield 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 83b4f8b03..bc0e015d3 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 @@ -29,6 +29,7 @@ module ElasticGraph def proto_types: () -> ::Array[untyped] def render_definitions: (::Array[untyped] types) -> ::String + def render_imports: (::Array[untyped] types) -> ::Array[::String] def validate_unique_enum_value_prefixes: (::Array[untyped] types) -> void def previous_field_names_for: (::String, ::String) -> ::Array[::String] def renamed_public_field_names_by_type_name: () -> ::Hash[::String, ::Hash[::String, ::Array[::String]]] diff --git a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rbs b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rbs index 608f27a7d..41503a099 100644 --- a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rbs +++ b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rbs @@ -28,7 +28,7 @@ module ElasticGraph ::ElasticGraph::SchemaDefinition::SchemaElements::TypeReference, package_name: ::String, context_field_name: ::String - ) -> [bool, ::String] + ) -> [bool, ::String, ::String?] end end end diff --git a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rbs b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rbs index 94f0dcf15..b132b9777 100644 --- a/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rbs +++ b/elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rbs @@ -3,11 +3,13 @@ module ElasticGraph module SchemaDefinition module SchemaElements module ScalarTypeExtension: ::ElasticGraph::SchemaDefinition::SchemaElements::ScalarType - BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME: ::Hash[::String, ::String] + BUILT_IN_SCALAR_PROTO_OPTIONS_BY_NAME: ::Hash[::String, ::Hash[::Symbol, ::String]] attr_reader protobuf_type: ::String? + attr_reader protobuf_import: ::String? + attr_reader protobuf_comment: ::String? - def protobuf: (type: ::String) -> void + def protobuf: (type: ::String, ?import: ::String?, ?comment: ::String?) -> void def initialize_proto_extension: () { () -> void } -> void def proto_name: () -> ::String def proto_type_reference: (::String package_name) -> ::String diff --git a/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/api_extension_spec.rb b/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/api_extension_spec.rb index 44f0d986c..73cc94b29 100644 --- a/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/api_extension_spec.rb +++ b/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/api_extension_spec.rb @@ -32,10 +32,11 @@ module SchemaDefinition end it "maps every built-in scalar to a proto field type" do + built_in_scalar_options = SchemaElements::ScalarTypeExtension::BUILT_IN_SCALAR_PROTO_OPTIONS_BY_NAME field_types = [] proto = define_proto_schema do |s| s.object_type "Widget" do |t| - SchemaElements::ScalarTypeExtension::BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME.each_key do |type_name| + built_in_scalar_options.each_key do |type_name| t.field type_name.downcase, type_name end field_types = t.graphql_fields_by_name.values.map { |field| field.type.name } @@ -43,9 +44,10 @@ module SchemaDefinition end end - expect(field_types).to match_array(SchemaElements::ScalarTypeExtension::BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME.keys) - SchemaElements::ScalarTypeExtension::BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME.each.with_index(1) do |(type_name, proto_type), field_number| - expect(proto).to include("#{proto_type} #{type_name.downcase} = #{field_number};") + expect(field_types).to match_array(built_in_scalar_options.keys) + expect(proto).to include('import "google/protobuf/timestamp.proto";') + built_in_scalar_options.each.with_index(1) do |(type_name, options), field_number| + expect(proto).to include("#{options.fetch(:type)} #{type_name.downcase} = #{field_number};") end end diff --git a/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_spec.rb b/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_spec.rb index f41bfada1..289795595 100644 --- a/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_spec.rb +++ b/elasticgraph-proto_ingestion/spec/unit/elastic_graph/proto_ingestion/schema_definition/schema_spec.rb @@ -242,6 +242,95 @@ module SchemaDefinition expect(proto_type_def_from(proto, "Event")).to include("int64 occurred_at = 2;") end + it "maps `DateTime` fields to `google.protobuf.Timestamp`, importing its proto file once" do + proto = define_proto_schema do |s| + s.object_type "Event" do |t| + t.field "id", "ID" + t.field "created_at", "DateTime" + t.field "updated_at", "DateTime" + t.index "events" + end + end + + expect(proto).to eq(<<~PROTO) + syntax = "proto3"; + + package elasticgraph; + + import "google/protobuf/timestamp.proto"; + + message Event { + string id = 1; + google.protobuf.Timestamp created_at = 2; + google.protobuf.Timestamp updated_at = 3; + } + PROTO + end + + it "sorts and de-duplicates imports shared by distinct protobuf types" do + proto = define_proto_schema do |s| + s.scalar_type "FirstZType" do |t| + t.mapping type: "keyword" + t.protobuf type: "example.FirstZType", import: "z/types.proto" + end + + s.scalar_type "SecondZType" do |t| + t.mapping type: "keyword" + t.protobuf type: "example.SecondZType", import: "z/types.proto" + end + + s.scalar_type "AType" do |t| + t.mapping type: "keyword" + t.protobuf type: "example.AType", import: "a/types.proto" + end + + s.object_type "Event" do |t| + t.field "id", "ID" + t.field "first_z", "FirstZType" + t.field "second_z", "SecondZType" + t.field "a", "AType" + t.index "events" + end + end + + expect(proto.lines.grep(/\Aimport /).map(&:chomp)).to eq([ + %(import "a/types.proto";), + %(import "z/types.proto";) + ]) + end + + it "renders format comments on fields whose scalar type documents one" do + proto = define_proto_schema do |s| + s.object_type "Person" do |t| + t.field "id", "ID" + t.field "birth_date", "Date" + t.index "people" + end + end + + expect(proto).to include( + %(string birth_date = 2; // ISO 8601 date, e.g. "2024-11-25") + ) + end + + it "supports `import:` and `comment:` on custom scalar types" do + proto = define_proto_schema do |s| + s.scalar_type "Money" do |t| + t.mapping type: "keyword" + t.protobuf type: "myapp.types.Money", import: "myapp/types/money.proto", comment: "amount + currency" + end + + s.object_type "Order" do |t| + t.field "id", "ID" + t.field "total", "Money" + t.index "orders" + end + end + + expect(proto).to include('import "myapp/types/money.proto";') + expect(proto).to include("myapp.types.Money total = 2; // amount + currency") + end + it "assigns the lowest unused field number to a new field, rather than the number after the maximum used one" do # A gap below the maximum used number can only arise from a hand-edited artifact: # organic schema evolution never creates one, since removed fields keep their numbers