diff --git a/spec/fixtures/generated_proto2/TestOneofMessage.pb.cr b/spec/fixtures/generated_proto2/TestOneofMessage.pb.cr new file mode 100644 index 0000000..e85e91f --- /dev/null +++ b/spec/fixtures/generated_proto2/TestOneofMessage.pb.cr @@ -0,0 +1,25 @@ +#require "protobuf" + +module TestMessagesProto2 + + struct TestOneof1 + include Protobuf::Message + + contract do + optional :a, :int32, 1 + + optional :oo0_a, :int32, 2, oneof_index: 0 + optional :oo0_b, :int32, 3, oneof_index: 0 + optional :oo0_c, :int32, 4, oneof_index: 0 + + optional :b, :int32, 5 + + optional :oo1_a, :int32, 6, oneof_index: 1 + optional :oo1_b, :int32, 7, oneof_index: 1 + optional :oo1_c, :int32, 8, oneof_index: 1 + + oneof 0, "oo0" + oneof 1, "oo1" + end + end +end diff --git a/spec/fixtures/generated_proto3/TestOneofMessage.pb.cr b/spec/fixtures/generated_proto3/TestOneofMessage.pb.cr new file mode 100644 index 0000000..2a2fd35 --- /dev/null +++ b/spec/fixtures/generated_proto3/TestOneofMessage.pb.cr @@ -0,0 +1,25 @@ +#require "protobuf" + +module TestMessagesProto3 + + struct TestOneof1 + include Protobuf::Message + + contract do + optional :a, :int32, 1 + + optional :oo0_a, :int32, 2, oneof_index: 0 + optional :oo0_b, :int32, 3, oneof_index: 0 + optional :oo0_c, :int32, 4, oneof_index: 0 + + optional :b, :int32, 5 + + optional :oo1_a, :int32, 6, oneof_index: 1 + optional :oo1_b, :int32, 7, oneof_index: 1 + optional :oo1_c, :int32, 8, oneof_index: 1 + + oneof 0, "oo0" + oneof 1, "oo1" + end + end +end diff --git a/spec/protobuf/message_spec.cr b/spec/protobuf/message_spec.cr index 9ff0bc8..b2f7507 100644 --- a/spec/protobuf/message_spec.cr +++ b/spec/protobuf/message_spec.cr @@ -59,4 +59,96 @@ describe Protobuf::Message do test4 = TestMessagesProto2::Test4.new(d: [] of Int32) test4.to_protobuf.empty?.should be_true end + + it "handles oneof in proto2" do + # When multiple oneof values are populated + # by the ctor only the first survives + msg = TestMessagesProto2::TestOneof1.new( + a: 1, + oo0_a: 2, oo0_b: 3, oo0_c: 4, + b: 5, + oo1_a: 6, oo1_b: 7, oo1_c: 8 + ) + + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, 2, nil, nil, 5, 6, nil, nil]) + + # `msg.foo` tells us the string name + # of the member `foo` that is set + msg.oo0.should eq("oo0_a") + msg.oo1.should eq("oo1_a") + + # Setting values outside of oneof's leaves the oneof's untouched + msg.a = 1 + msg.b = 5 + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, 2, nil, nil, 5, 6, nil, nil]) + + # Setting a oneof value resets the others in the same oneof + msg.oo0_b = 3 + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, nil, 3, nil, 5, 6, nil, nil]) + msg.oo0.should eq("oo0_b") + + msg.oo1_b = 7 + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, nil, 3, nil, 5, nil, 7, nil]) + msg.oo0.should eq("oo0_b") + msg.oo1.should eq("oo1_b") + end + + it "handles oneof in proto2 and proto3" do + [TestMessagesProto2::TestOneof1, TestMessagesProto3::TestOneof1].each do |message_class| + # When multiple oneof values are populated + # by the ctor only the first survives + msg = message_class.new( + a: 1, + oo0_a: 2, oo0_b: 3, oo0_c: 4, + b: 5, + oo1_a: 6, oo1_b: 7, oo1_c: 8 + ) + + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, 2, nil, nil, 5, 6, nil, nil]) + + # `msg.foo` tells us the string name + # of the member `foo` that is set + msg.oo0.should eq("oo0_a") + msg.oo1.should eq("oo1_a") + + # Setting values outside of oneof's leaves the oneof's untouched + msg.a = 1 + msg.b = 5 + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, 2, nil, nil, 5, 6, nil, nil]) + + # Setting a oneof value resets the others in the same oneof + msg.oo0_b = 3 + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, nil, 3, nil, 5, 6, nil, nil]) + msg.oo0.should eq("oo0_b") + + msg.oo1_b = 7 + payload = [ msg.a, msg.oo0_a, msg.oo0_b, msg.oo0_c, msg.b, msg.oo1_a, msg.oo1_b, msg.oo1_c ] + payload.should eq([1, nil, 3, nil, 5, nil, 7, nil]) + msg.oo0.should eq("oo0_b") + msg.oo1.should eq("oo1_b") + + # Serialization and deserialization through protobuf + deser_msg = msg.class.from_protobuf(msg.to_protobuf) + msg.should eq(deser_msg) + + deser_msg.a.should eq(1) + deser_msg.oo0_a.should eq(nil) + deser_msg.oo0_b.should eq(3) + deser_msg.oo0_c.should eq(nil) + + deser_msg.oo1_a.should eq(nil) + deser_msg.oo1_b.should eq(7) + deser_msg.oo1_c.should eq(nil) + + deser_msg.oo0.should eq("oo0_b") + deser_msg.oo1.should eq("oo1_b") + end + end end diff --git a/src/protobuf/generator.cr b/src/protobuf/generator.cr index feb9636..2d54fd9 100644 --- a/src/protobuf/generator.cr +++ b/src/protobuf/generator.cr @@ -73,6 +73,33 @@ module Protobuf optional :default_value, :string, 7 optional :options, FieldOptions, 8 + + ## If set, gives the index of a oneof in the containing type's oneof_decl + ## list. This field is a member of that oneof. + optional :oneof_index, :int32, 9 + + ## If true, this is a proto3 "optional". When a proto3 field is optional, it + ## tracks presence regardless of field type. + ## + ## When proto3_optional is true, this field must be belong to a oneof to + ## signal to old proto3 clients that presence is tracked for this field. This + ## oneof is known as a "synthetic" oneof, and this field must be its sole + ## member (each proto3 optional field gets its own synthetic oneof). Synthetic + ## oneofs exist in the descriptor only, and do not generate any API. Synthetic + ## oneofs must be ordered after all "real" oneofs. + ## + ## For message fields, proto3_optional doesn't create any semantic change, + ## since non-repeated message fields always track presence. However it still + ## indicates the semantic detail of whether the user wrote "optional" or not. + ## This can be useful for round-tripping the .proto file. For consistency we + ## give message fields a synthetic oneof also, even though it is not required + ## to track presence. This is especially important because the parser can't + ## tell if a field is a message or an enum, so it must always create a + ## synthetic oneof. + ## + ## Proto2 optional fields do not set this flag, because they already indicate + ## optional with `LABEL_OPTIONAL`. + optional :proto3_optional, :bool, 17 end end @@ -84,6 +111,14 @@ module Protobuf end end + struct OneofDescriptorProto + include Protobuf::Message + + contract do + optional :name, :string, 1 + end + end + struct EnumValueDescriptorProto include Protobuf::Message @@ -184,6 +219,7 @@ module Protobuf repeated :extended, CodeGeneratorRequest::FieldDescriptorProto, 6 repeated :nested_type, CodeGeneratorRequest::DescriptorProto, 3 repeated :enum_type, CodeGeneratorRequest::EnumDescriptorProto, 4 + repeated :oneof_decl, CodeGeneratorRequest::OneofDescriptorProto, 8 end end @@ -236,6 +272,7 @@ module Protobuf end contract do + optional :supported_features, :uint64, 2 repeated :file, CodeGeneratorResponse::File, 15 end end @@ -258,7 +295,9 @@ module Protobuf content: generator.compile ) end - CodeGeneratorResponse.new(file: files) + # `supported_features` is explained here: + # https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/compiler/plugin.proto#L112 + CodeGeneratorResponse.new(file: files, supported_features: 1) end @package_name : String? @@ -329,10 +368,12 @@ module Protobuf syntax = @file.syntax.nil? ? "proto2" : @file.syntax - unless message_type.field.nil? + mt_field = message_type.field + unless mt_field.nil? puts "contract_of \"#{syntax}\" do" indent do - message_type.field.not_nil!.each { |f| field!(f, syntax) } unless message_type.field.nil? + mt_field.each { |f| field!(f, syntax) } # unless mt_field.nil? + message_type.oneof_decl.not_nil!.each_with_index { |oo, i| oneof!(oo, i) } unless message_type.oneof_decl.nil? end puts "end" end @@ -340,6 +381,18 @@ module Protobuf puts "end" end + def oneof!(oneof_decl, index) + # This feels wrong. We rely on the the leading underscore + # to detect synthetic oneofs. This works but there must be + # a better way (yet to be discovered). + # https://github.com/protocolbuffers/protobuf/blob/master/docs/implementing_proto3_presence.md#updating-a-code-generator + if oneof_decl.name.not_nil![0] == '_' + puts "synthetic_oneof #{index}, \"#{oneof_decl.name}\"" + else + puts "oneof #{index}, \"#{oneof_decl.name}\"" + end + end + def field!(field, syntax) met = case field.label when CodeGeneratorRequest::FieldDescriptorProto::Label::LABEL_OPTIONAL @@ -396,6 +449,7 @@ module Protobuf field_desc += ", packed: true" if field.options.not_nil!.packed end end + field_desc += ", oneof_index: #{field.oneof_index}" if field.oneof_index && field.proto3_optional != true puts field_desc end diff --git a/src/protobuf/message.cr b/src/protobuf/message.cr index 6a0fb49..8972675 100644 --- a/src/protobuf/message.cr +++ b/src/protobuf/message.cr @@ -1,8 +1,8 @@ module Protobuf module Message - macro contract_of (syntax, &blk) FIELDS = {} of Int32 => HashLiteral(Symbol, ASTNode) + ONEOFS = {} of Int32 => String? {{yield}} _generate_decoder {{syntax}} _generate_encoder {{syntax}} @@ -14,7 +14,7 @@ module Protobuf contract_of "proto2" {{blk}} end - macro _add_field(tag, name, pb_type, options = {} of Symbol => Bool) + macro _add_field(tag, name, pb_type, options = {} of Symbol => Bool, oneof_index = nil) {% t = ::Protobuf::PB_TYPE_MAP[pb_type] || pb_type FIELDS[tag] = { @@ -27,22 +27,58 @@ module Protobuf repeated: !!options[:repeated], default: options[:default], packed: !!options[:packed], + oneof_index: oneof_index } %} end - macro optional(name, type, tag, default = nil, repeated = false, packed = false) - _add_field({{tag.id}}, {{name}}, {{type}}, {optional: true, default: {{default}}, repeated: {{repeated}}, packed: {{packed}}}) + macro optional(name, type, tag, default = nil, repeated = false, packed = false, oneof_index = nil) + _add_field({{tag.id}}, {{name}}, {{type}}, + {optional: true, default: {{default}}, repeated: {{repeated}}, packed: {{packed}}}, oneof_index: {{oneof_index}}) end - macro required(name, type, tag, default = nil) - _add_field({{tag.id}}, {{name}}, {{type}}, {default: {{default}}}) + macro required(name, type, tag, default = nil, oneof_index = nil) + _add_field({{tag.id}}, {{name}}, {{type}}, {default: {{default}}}, oneof_index: {{oneof_index}}) end macro repeated(name, type, tag, packed = false) optional({{name}}, {{type}}, {{tag}}, nil, true, {{packed}}) end + macro synthetic_oneof(index, name) + # Synthetic oneofs don't get any special API, + # they are supposed to behave like proto2 optional fields. + # + # Leaving this macro in here as a placeholder regardless, + # in case special treatment will be needed later. + end + + macro oneof(index, name) + {% ONEOFS[index] = name %} + + @{{name.id}} : String? + def {{name.id}} + @{{name.id}} + end + + def {{name.id}}! + @{{name.id}}.not_nil! + end + + def {{name.id}}_value + {% for tag, field in FIELDS %} + {% if field[:oneof_index] == index %} + return @{{field[:name].id}} if @{{ONEOFS[index].id}} == {{ field[:name] }}.to_s + {% end %} + {% end %} + nil + end + + def {{name.id}}_value! + {{name.id}}_value.not_nil! + end + end + macro extensions(range) # puts "extensions: {{range.id}}" end @@ -126,6 +162,10 @@ module Protobuf @{{field[:name].id}} = (%var{tag}).as({{field[:cast_type]}}) {% end %} {% end %} + + {% for tag, field in FIELDS %} + self.{{field[:name].id}} = @{{field[:name].id}} + {% end %} end def initialize( @@ -140,6 +180,9 @@ module Protobuf {% end %} {% end %} ) + {% for tag, field in FIELDS %} + self.{{field[:name].id}} = @{{field[:name].id}} + {% end %} end end @@ -192,6 +235,27 @@ module Protobuf macro _generate_getters_setters {% for tag, field in FIELDS %} property {{field[:name].id}} : {{field[:cast_type]}} + def {{field[:name].id}}=(value : {{field[:cast_type]}}) + @{{field[:name].id}} = value + + # If this field is a member of a oneof.. + {% unless field[:oneof_index] == nil %} + unless value.nil? + # ..we set the oneof to the name of the this field.. + @{{ONEOFS[field[:oneof_index]].id}} = {{field[:name].id.stringify}} + # ..and clear the other members of the oneof + {% for neighbor_tag, neighbor_field in FIELDS %} + {% if neighbor_tag != tag && neighbor_field[:oneof_index] == field[:oneof_index] %} + @{{neighbor_field[:name].id}} = {{neighbor_field[:default]}} + {% end %} + {% end %} + end + {% end %} + end + + def {{field[:name].id}}! + @{{field[:name].id}}.not_nil! + end {% end %} end