Merge pull request #168 from cfallin/ruby-oneof

Support oneofs in MRI Ruby C extension.

Makefile.am

@@ -244,6 +244,7 @@ ruby_EXTRA_DIST=                                                             \
   ruby/ext/google/protobuf_c/defs.c                                          \
   ruby/ext/google/protobuf_c/encode_decode.c                                 \
   ruby/ext/google/protobuf_c/extconf.rb                                      \
+  ruby/ext/google/protobuf_c/map.c                                           \
   ruby/ext/google/protobuf_c/message.c                                       \
   ruby/ext/google/protobuf_c/protobuf.c                                      \
   ruby/ext/google/protobuf_c/protobuf.h                                      \
@@ -254,7 +255,10 @@ ruby_EXTRA_DIST=                                                             \
   ruby/google-protobuf.gemspec                                               \
   ruby/lib/google/protobuf.rb                                                \
   ruby/tests/basic.rb                                                        \
-  ruby/tests/stress.rb
+  ruby/tests/stress.rb                                                       \
+  ruby/tests/generated_code.proto                                            \
+  ruby/tests/generated_code.rb                                               \
+  ruby/tests/generated_code_test.rb
 
 all_EXTRA_DIST=$(java_EXTRA_DIST) $(python_EXTRA_DIST) $(ruby_EXTRA_DIST)
 

ruby/ext/google/protobuf_c/message.c

@@ -70,6 +70,35 @@ VALUE Message_alloc(VALUE klass) {
   return ret;
 }
 
+static VALUE which_oneof_field(MessageHeader* self, const upb_oneofdef* o) {
+  // If no fields in the oneof, always nil.
+  if (upb_oneofdef_numfields(o) == 0) {
+    return Qnil;
+  }
+  // Grab the first field in the oneof so we can get its layout info to find the
+  // oneof_case field.
+  upb_oneof_iter it;
+  upb_oneof_begin(&it, o);
+  assert(!upb_oneof_done(&it));
+  const upb_fielddef* first_field = upb_oneof_iter_field(&it);
+  assert(upb_fielddef_containingoneof(first_field) != NULL);
+
+  size_t case_ofs =
+      self->descriptor->layout->
+      fields[upb_fielddef_index(first_field)].case_offset;
+  uint32_t oneof_case = *((uint32_t*)(Message_data(self) + case_ofs));
+
+  if (oneof_case == ONEOF_CASE_NONE) {
+    return Qnil;
+  }
+
+  // oneof_case is a field index, so find that field.
+  const upb_fielddef* f = upb_oneofdef_itof(o, oneof_case);
+  assert(f != NULL);
+
+  return ID2SYM(rb_intern(upb_fielddef_name(f)));
+}
+
 /*
  * call-seq:
  *     Message.method_missing(*args)
@@ -82,6 +111,10 @@ VALUE Message_alloc(VALUE klass) {
  *
  *     msg.foo = 42
  *     puts msg.foo
+ *
+ * This method also provides read-only accessors for oneofs. If a oneof exists
+ * with name 'my_oneof', then msg.my_oneof will return a Ruby symbol equal to
+ * the name of the field in that oneof that is currently set, or nil if none.
  */
 VALUE Message_method_missing(int argc, VALUE* argv, VALUE _self) {
   MessageHeader* self;
@@ -104,6 +137,17 @@ VALUE Message_method_missing(int argc, VALUE* argv, VALUE _self) {
     name_len--;
   }
 
+  // Check for a oneof name first.
+  const upb_oneofdef* o = upb_msgdef_ntoo(self->descriptor->msgdef,
+                                          name, name_len);
+  if (o != NULL) {
+    if (setter) {
+      rb_raise(rb_eRuntimeError, "Oneof accessors are read-only.");
+    }
+    return which_oneof_field(self, o);
+  }
+
+  // Otherwise, check for a field with that name.
   const upb_fielddef* f = upb_msgdef_ntof(self->descriptor->msgdef,
                                           name, name_len);
 

ruby/ext/google/protobuf_c/protobuf.c

@@ -77,8 +77,10 @@ void Init_protobuf_c() {
   DescriptorPool_register(protobuf);
   Descriptor_register(protobuf);
   FieldDescriptor_register(protobuf);
+  OneofDescriptor_register(protobuf);
   EnumDescriptor_register(protobuf);
   MessageBuilderContext_register(internal);
+  OneofBuilderContext_register(internal);
   EnumBuilderContext_register(internal);
   Builder_register(internal);
   RepeatedField_register(protobuf);

ruby/ext/google/protobuf_c/protobuf.h

@@ -43,6 +43,7 @@ struct Descriptor;
 struct FieldDescriptor;
 struct EnumDescriptor;
 struct MessageLayout;
+struct MessageField;
 struct MessageHeader;
 struct MessageBuilderContext;
 struct EnumBuilderContext;
@@ -51,10 +52,13 @@ struct Builder;
 typedef struct DescriptorPool DescriptorPool;
 typedef struct Descriptor Descriptor;
 typedef struct FieldDescriptor FieldDescriptor;
+typedef struct OneofDescriptor OneofDescriptor;
 typedef struct EnumDescriptor EnumDescriptor;
 typedef struct MessageLayout MessageLayout;
+typedef struct MessageField MessageField;
 typedef struct MessageHeader MessageHeader;
 typedef struct MessageBuilderContext MessageBuilderContext;
+typedef struct OneofBuilderContext OneofBuilderContext;
 typedef struct EnumBuilderContext EnumBuilderContext;
 typedef struct Builder Builder;
 
@@ -120,6 +124,10 @@ struct FieldDescriptor {
   const upb_fielddef* fielddef;
 };
 
+struct OneofDescriptor {
+  const upb_oneofdef* oneofdef;
+};
+
 struct EnumDescriptor {
   const upb_enumdef* enumdef;
   VALUE module;  // begins as nil
@@ -130,6 +138,11 @@ struct MessageBuilderContext {
   VALUE builder;
 };
 
+struct OneofBuilderContext {
+  VALUE descriptor;
+  VALUE builder;
+};
+
 struct EnumBuilderContext {
   VALUE enumdesc;
 };
@@ -144,6 +157,7 @@ extern VALUE cDescriptor;
 extern VALUE cFieldDescriptor;
 extern VALUE cEnumDescriptor;
 extern VALUE cMessageBuilderContext;
+extern VALUE cOneofBuilderContext;
 extern VALUE cEnumBuilderContext;
 extern VALUE cBuilder;
 
@@ -175,6 +189,9 @@ VALUE Descriptor_name_set(VALUE _self, VALUE str);
 VALUE Descriptor_each(VALUE _self);
 VALUE Descriptor_lookup(VALUE _self, VALUE name);
 VALUE Descriptor_add_field(VALUE _self, VALUE obj);
+VALUE Descriptor_add_oneof(VALUE _self, VALUE obj);
+VALUE Descriptor_each_oneof(VALUE _self);
+VALUE Descriptor_lookup_oneof(VALUE _self, VALUE name);
 VALUE Descriptor_msgclass(VALUE _self);
 extern const rb_data_type_t _Descriptor_type;
 
@@ -199,6 +216,16 @@ VALUE FieldDescriptor_set(VALUE _self, VALUE msg_rb, VALUE value);
 upb_fieldtype_t ruby_to_fieldtype(VALUE type);
 VALUE fieldtype_to_ruby(upb_fieldtype_t type);
 
+void OneofDescriptor_mark(void* _self);
+void OneofDescriptor_free(void* _self);
+VALUE OneofDescriptor_alloc(VALUE klass);
+void OneofDescriptor_register(VALUE module);
+OneofDescriptor* ruby_to_OneofDescriptor(VALUE value);
+VALUE OneofDescriptor_name(VALUE _self);
+VALUE OneofDescriptor_name_set(VALUE _self, VALUE value);
+VALUE OneofDescriptor_add_field(VALUE _self, VALUE field);
+VALUE OneofDescriptor_each(VALUE _self, VALUE field);
+
 void EnumDescriptor_mark(void* _self);
 void EnumDescriptor_free(void* _self);
 VALUE EnumDescriptor_alloc(VALUE klass);
@@ -225,6 +252,17 @@ VALUE MessageBuilderContext_optional(int argc, VALUE* argv, VALUE _self);
 VALUE MessageBuilderContext_required(int argc, VALUE* argv, VALUE _self);
 VALUE MessageBuilderContext_repeated(int argc, VALUE* argv, VALUE _self);
 VALUE MessageBuilderContext_map(int argc, VALUE* argv, VALUE _self);
+VALUE MessageBuilderContext_oneof(VALUE _self, VALUE name);
+
+void OneofBuilderContext_mark(void* _self);
+void OneofBuilderContext_free(void* _self);
+VALUE OneofBuilderContext_alloc(VALUE klass);
+void OneofBuilderContext_register(VALUE module);
+OneofBuilderContext* ruby_to_OneofBuilderContext(VALUE value);
+VALUE OneofBuilderContext_initialize(VALUE _self,
+                                     VALUE descriptor,
+                                     VALUE builder);
+VALUE OneofBuilderContext_optional(int argc, VALUE* argv, VALUE _self);
 
 void EnumBuilderContext_mark(void* _self);
 void EnumBuilderContext_free(void* _self);
@@ -247,13 +285,22 @@ VALUE Builder_finalize_to_pool(VALUE _self, VALUE pool_rb);
 // Native slot storage abstraction.
 // -----------------------------------------------------------------------------
 
-#define NATIVE_SLOT_MAX_SIZE sizeof(void*)
+#define NATIVE_SLOT_MAX_SIZE sizeof(uint64_t)
 
 size_t native_slot_size(upb_fieldtype_t type);
 void native_slot_set(upb_fieldtype_t type,
                      VALUE type_class,
                      void* memory,
                      VALUE value);
+// Atomically (with respect to Ruby VM calls) either update the value and set a
+// oneof case, or do neither. If |case_memory| is null, then no case value is
+// set.
+void native_slot_set_value_and_case(upb_fieldtype_t type,
+                                    VALUE type_class,
+                                    void* memory,
+                                    VALUE value,
+                                    uint32_t* case_memory,
+                                    uint32_t case_number);
 VALUE native_slot_get(upb_fieldtype_t type,
                       VALUE type_class,
                       const void* memory);
@@ -275,6 +322,11 @@ VALUE field_type_class(const upb_fielddef* field);
 #define MAP_KEY_FIELD 1
 #define MAP_VALUE_FIELD 2
 
+// Oneof case slot value to indicate that no oneof case is set. The value `0` is
+// safe because field numbers are used as case identifiers, and no field can
+// have a number of 0.
+#define ONEOF_CASE_NONE 0
+
 // These operate on a map field (i.e., a repeated field of submessages whose
 // submessage type is a map-entry msgdef).
 bool is_map_field(const upb_fielddef* field);
@@ -384,9 +436,16 @@ VALUE Map_iter_value(Map_iter* iter);
 // Message layout / storage.
 // -----------------------------------------------------------------------------
 
+#define MESSAGE_FIELD_NO_CASE ((size_t)-1)
+
+struct MessageField {
+  size_t offset;
+  size_t case_offset;  // for oneofs, a uint32. Else, MESSAGE_FIELD_NO_CASE.
+};
+
 struct MessageLayout {
   const upb_msgdef* msgdef;
-  size_t* offsets;
+  MessageField* fields;
   size_t size;
 };
 

ruby/google-protobuf.gemspec

@@ -18,5 +18,7 @@ Gem::Specification.new do |s|
   s.files       = ["lib/google/protobuf.rb"] +
                   # extension C source
                   find_c_source("ext/google/protobuf_c")
-  s.test_files = `git ls-files -- tests`.split
+  s.test_files = ["tests/basic.rb",
+		  "tests/stress.rb",
+		  "tests/generated_code_test.rb"]
 end

ruby/tests/basic.rb

@@ -73,6 +73,15 @@ module BasicTest
       optional :key, :string, 1
       optional :value, :message, 2, "TestMessage2"
     end
+
+    add_message "OneofMessage" do
+      oneof :my_oneof do
+        optional :a, :string, 1
+        optional :b, :int32, 2
+        optional :c, :message, 3, "TestMessage2"
+        optional :d, :enum, 4, "TestEnum"
+      end
+    end
   end
 
   TestMessage = pool.lookup("TestMessage").msgclass
@@ -87,6 +96,7 @@ module BasicTest
     pool.lookup("MapMessageWireEquiv_entry1").msgclass
   MapMessageWireEquiv_entry2 =
     pool.lookup("MapMessageWireEquiv_entry2").msgclass
+  OneofMessage = pool.lookup("OneofMessage").msgclass
 
 # ------------ test cases ---------------
 
@@ -582,6 +592,91 @@ module BasicTest
                     "b" => TestMessage2.new(:foo => 2)}
     end
 
+    def test_oneof_descriptors
+      d = OneofMessage.descriptor
+      o = d.lookup_oneof("my_oneof")
+      assert o != nil
+      assert o.class == Google::Protobuf::OneofDescriptor
+      assert o.name == "my_oneof"
+      oneof_count = 0
+      d.each_oneof{ |oneof|
+        oneof_count += 1
+        assert oneof == o
+      }
+      assert oneof_count == 1
+      assert o.count == 4
+      field_names = o.map{|f| f.name}.sort
+      assert field_names == ["a", "b", "c", "d"]
+    end
+
+    def test_oneof
+      d = OneofMessage.new
+      assert d.a == nil
+      assert d.b == nil
+      assert d.c == nil
+      assert d.d == nil
+      assert d.my_oneof == nil
+
+      d.a = "hi"
+      assert d.a == "hi"
+      assert d.b == nil
+      assert d.c == nil
+      assert d.d == nil
+      assert d.my_oneof == :a
+
+      d.b = 42
+      assert d.a == nil
+      assert d.b == 42
+      assert d.c == nil
+      assert d.d == nil
+      assert d.my_oneof == :b
+
+      d.c = TestMessage2.new(:foo => 100)
+      assert d.a == nil
+      assert d.b == nil
+      assert d.c.foo == 100
+      assert d.d == nil
+      assert d.my_oneof == :c
+
+      d.d = :C
+      assert d.a == nil
+      assert d.b == nil
+      assert d.c == nil
+      assert d.d == :C
+      assert d.my_oneof == :d
+
+      d2 = OneofMessage.decode(OneofMessage.encode(d))
+      assert d2 == d
+
+      encoded_field_a = OneofMessage.encode(OneofMessage.new(:a => "string"))
+      encoded_field_b = OneofMessage.encode(OneofMessage.new(:b => 1000))
+      encoded_field_c = OneofMessage.encode(
+        OneofMessage.new(:c => TestMessage2.new(:foo => 1)))
+      encoded_field_d = OneofMessage.encode(OneofMessage.new(:d => :B))
+
+      d3 = OneofMessage.decode(
+        encoded_field_c + encoded_field_a + encoded_field_d)
+      assert d3.a == nil
+      assert d3.b == nil
+      assert d3.c == nil
+      assert d3.d == :B
+
+      d4 = OneofMessage.decode(
+        encoded_field_c + encoded_field_a + encoded_field_d +
+        encoded_field_c)
+      assert d4.a == nil
+      assert d4.b == nil
+      assert d4.c.foo == 1
+      assert d4.d == nil
+
+      d5 = OneofMessage.new(:a => "hello")
+      assert d5.a != nil
+      d5.a = nil
+      assert d5.a == nil
+      assert OneofMessage.encode(d5) == ''
+      assert d5.my_oneof == nil
+    end
+
     def test_enum_field
       m = TestMessage.new
       assert m.optional_enum == :Default
@@ -621,6 +716,14 @@ module BasicTest
       assert m.repeated_msg[0].object_id != m2.repeated_msg[0].object_id
     end
 
+    def test_eq
+      m = TestMessage.new(:optional_int32 => 42,
+                          :repeated_int32 => [1, 2, 3])
+      m2 = TestMessage.new(:optional_int32 => 43,
+                           :repeated_int32 => [1, 2, 3])
+      assert m != m2
+    end
+
     def test_enum_lookup
       assert TestEnum::A == 1
       assert TestEnum::B == 2

ruby/tests/generated_code.proto

+syntax = "proto3";
+
+package A.B.C;
+
+message TestMessage {
+  optional int32 optional_int32 = 1;
+  optional int64 optional_int64 = 2;
+  optional uint32 optional_uint32 = 3;
+  optional uint64 optional_uint64 = 4;
+  optional bool optional_bool = 5;
+  optional double optional_double = 6;
+  optional float optional_float = 7;
+  optional string optional_string = 8;
+  optional bytes optional_bytes = 9;
+  optional TestEnum optional_enum = 10;
+  optional TestMessage optional_msg = 11;
+
+  repeated int32 repeated_int32 = 21;
+  repeated int64 repeated_int64 = 22;
+  repeated uint32 repeated_uint32 = 23;
+  repeated uint64 repeated_uint64 = 24;
+  repeated bool repeated_bool = 25;
+  repeated double repeated_double = 26;
+  repeated float repeated_float = 27;
+  repeated string repeated_string = 28;
+  repeated bytes repeated_bytes = 29;
+  repeated TestEnum repeated_enum = 30;
+  repeated TestMessage repeated_msg = 31;
+
+  oneof my_oneof {
+    int32 oneof_int32 = 41;
+    int64 oneof_int64 = 42;
+    uint32 oneof_uint32 = 43;
+    uint64 oneof_uint64 = 44;
+    bool oneof_bool = 45;
+    double oneof_double = 46;
+    float oneof_float = 47;
+    string oneof_string = 48;
+    bytes oneof_bytes = 49;
+    TestEnum oneof_enum = 50;
+    TestMessage oneof_msg = 51;
+  }
+
+  map<int32, string> map_int32_string = 61;
+  map<int64, string> map_int64_string = 62;
+  map<uint32, string> map_uint32_string = 63;
+  map<uint64, string> map_uint64_string = 64;
+  map<bool, string> map_bool_string = 65;
+  map<string, string> map_string_string = 66;
+  map<string, TestMessage> map_string_msg = 67;
+  map<string, TestEnum> map_string_enum = 68;
+  map<string, int32> map_string_int32 = 69;
+  map<string, bool> map_string_bool = 70;
+
+  message NestedMessage {
+    optional int32 foo = 1;
+  }
+
+  optional NestedMessage nested_message = 80;
+}
+
+enum TestEnum {
+  Default = 0;
+  A = 1;
+  B = 2;
+  C = 3;
+}

ruby/tests/generated_code.rb

+# Generated by the protocol buffer compiler.  DO NOT EDIT!
+# source: generated_code.proto
+
+require 'google/protobuf'
+
+Google::Protobuf::DescriptorPool.generated_pool.build do
+  add_message "A.B.C.TestMessage" do
+    optional :optional_int32, :int32, 1
+    optional :optional_int64, :int64, 2
+    optional :optional_uint32, :uint32, 3
+    optional :optional_uint64, :uint64, 4
+    optional :optional_bool, :bool, 5
+    optional :optional_double, :double, 6
+    optional :optional_float, :float, 7
+    optional :optional_string, :string, 8
+    optional :optional_bytes, :string, 9
+    optional :optional_enum, :enum, 10, "A.B.C.TestEnum"
+    optional :optional_msg, :message, 11, "A.B.C.TestMessage"
+    repeated :repeated_int32, :int32, 21
+    repeated :repeated_int64, :int64, 22
+    repeated :repeated_uint32, :uint32, 23
+    repeated :repeated_uint64, :uint64, 24
+    repeated :repeated_bool, :bool, 25
+    repeated :repeated_double, :double, 26
+    repeated :repeated_float, :float, 27
+    repeated :repeated_string, :string, 28
+    repeated :repeated_bytes, :string, 29
+    repeated :repeated_enum, :enum, 30, "A.B.C.TestEnum"
+    repeated :repeated_msg, :message, 31, "A.B.C.TestMessage"
+    repeated :map_int32_string, :message, 61, "A.B.C.TestMessage.MapInt32StringEntry"
+    repeated :map_int64_string, :message, 62, "A.B.C.TestMessage.MapInt64StringEntry"
+    repeated :map_uint32_string, :message, 63, "A.B.C.TestMessage.MapUint32StringEntry"
+    repeated :map_uint64_string, :message, 64, "A.B.C.TestMessage.MapUint64StringEntry"
+    repeated :map_bool_string, :message, 65, "A.B.C.TestMessage.MapBoolStringEntry"
+    repeated :map_string_string, :message, 66, "A.B.C.TestMessage.MapStringStringEntry"
+    repeated :map_string_msg, :message, 67, "A.B.C.TestMessage.MapStringMsgEntry"
+    repeated :map_string_enum, :message, 68, "A.B.C.TestMessage.MapStringEnumEntry"
+    repeated :map_string_int32, :message, 69, "A.B.C.TestMessage.MapStringInt32Entry"
+    repeated :map_string_bool, :message, 70, "A.B.C.TestMessage.MapStringBoolEntry"
+    optional :nested_message, :message, 80, "A.B.C.TestMessage.NestedMessage"
+    oneof :my_oneof do
+      optional :oneof_int32, :int32, 41
+      optional :oneof_int64, :int64, 42
+      optional :oneof_uint32, :uint32, 43
+      optional :oneof_uint64, :uint64, 44
+      optional :oneof_bool, :bool, 45
+      optional :oneof_double, :double, 46
+      optional :oneof_float, :float, 47
+      optional :oneof_string, :string, 48
+      optional :oneof_bytes, :string, 49
+      optional :oneof_enum, :enum, 50, "A.B.C.TestEnum"
+      optional :oneof_msg, :message, 51, "A.B.C.TestMessage"
+    end
+  end
+  add_message "A.B.C.TestMessage.MapInt32StringEntry" do
+    optional :key, :int32, 1
+    optional :value, :string, 2
+  end
+  add_message "A.B.C.TestMessage.MapInt64StringEntry" do
+    optional :key, :int64, 1
+    optional :value, :string, 2
+  end
+  add_message "A.B.C.TestMessage.MapUint32StringEntry" do
+    optional :key, :uint32, 1
+    optional :value, :string, 2
+  end
+  add_message "A.B.C.TestMessage.MapUint64StringEntry" do
+    optional :key, :uint64, 1
+    optional :value, :string, 2
+  end
+  add_message "A.B.C.TestMessage.MapBoolStringEntry" do
+    optional :key, :bool, 1
+    optional :value, :string, 2
+  end
+  add_message "A.B.C.TestMessage.MapStringStringEntry" do
+    optional :key, :string, 1
+    optional :value, :string, 2
+  end
+  add_message "A.B.C.TestMessage.MapStringMsgEntry" do
+    optional :key, :string, 1
+    optional :value, :message, 2, "A.B.C.TestMessage"
+  end
+  add_message "A.B.C.TestMessage.MapStringEnumEntry" do
+    optional :key, :string, 1
+    optional :value, :enum, 2, "A.B.C.TestEnum"
+  end
+  add_message "A.B.C.TestMessage.MapStringInt32Entry" do
+    optional :key, :string, 1
+    optional :value, :int32, 2
+  end
+  add_message "A.B.C.TestMessage.MapStringBoolEntry" do
+    optional :key, :string, 1
+    optional :value, :bool, 2
+  end
+  add_message "A.B.C.TestMessage.NestedMessage" do
+    optional :foo, :int32, 1
+  end
+  add_enum "A.B.C.TestEnum" do
+    value :Default, 0
+    value :A, 1
+    value :B, 2
+    value :C, 3
+  end
+end
+
+module A
+  module B
+    module C
+      TestMessage = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage").msgclass
+      TestMessage::MapInt32StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapInt32StringEntry").msgclass
+      TestMessage::MapInt64StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapInt64StringEntry").msgclass
+      TestMessage::MapUint32StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapUint32StringEntry").msgclass
+      TestMessage::MapUint64StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapUint64StringEntry").msgclass
+      TestMessage::MapBoolStringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapBoolStringEntry").msgclass
+      TestMessage::MapStringStringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringStringEntry").msgclass
+      TestMessage::MapStringMsgEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringMsgEntry").msgclass
+      TestMessage::MapStringEnumEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringEnumEntry").msgclass
+      TestMessage::MapStringInt32Entry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringInt32Entry").msgclass
+      TestMessage::MapStringBoolEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringBoolEntry").msgclass
+      TestMessage::NestedMessage = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.NestedMessage").msgclass
+      TestEnum = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestEnum").enummodule
+    end
+  end
+end

ruby/tests/generated_code_test.rb

+#!/usr/bin/ruby
+
+# generated_code.rb is in the same directory as this test.
+$LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__)))
+
+require 'generated_code'
+require 'test/unit'
+
+class GeneratedCodeTest < Test::Unit::TestCase
+  def test_generated_msg
+    # just test that we can instantiate the message. The purpose of this test
+    # is to ensure that the output of the code generator is valid Ruby and
+    # successfully creates message definitions and classes, not to test every
+    # aspect of the extension (basic.rb is for that).
+    m = A::B::C::TestMessage.new()
+  end
+end

src/Makefile.am

@@ -458,6 +458,7 @@ protobuf_test_SOURCES =                                        \
   google/protobuf/compiler/java/java_plugin_unittest.cc        \
   google/protobuf/compiler/java/java_doc_comment_unittest.cc   \
   google/protobuf/compiler/python/python_plugin_unittest.cc    \
+  google/protobuf/compiler/ruby/ruby_generator_unittest.cc     \
   $(COMMON_TEST_SOURCES)
 nodist_protobuf_test_SOURCES = $(protoc_outputs)
 

src/google/protobuf/compiler/ruby/ruby_generator.cc

@@ -100,15 +100,8 @@ std::string TypeName(const google::protobuf::FieldDescriptor* field) {
   }
 }
 
-void GenerateMessage(const google::protobuf::Descriptor* message,
+void GenerateField(const google::protobuf::FieldDescriptor* field,
                    google::protobuf::io::Printer* printer) {
-  printer->Print(
-    "add_message \"$name$\" do\n",
-    "name", message->full_name());
-  printer->Indent();
-
-  for (int i = 0; i < message->field_count(); i++) {
-    const FieldDescriptor* field = message->field(i);
   printer->Print(
     "$label$ :$name$, ",
     "label", LabelForField(field),
@@ -128,6 +121,41 @@ void GenerateMessage(const google::protobuf::Descriptor* message,
   } else {
     printer->Print("\n");
   }
+}
+
+void GenerateOneof(const google::protobuf::OneofDescriptor* oneof,
+                   google::protobuf::io::Printer* printer) {
+  printer->Print(
+      "oneof :$name$ do\n",
+      "name", oneof->name());
+  printer->Indent();
+
+  for (int i = 0; i < oneof->field_count(); i++) {
+    const FieldDescriptor* field = oneof->field(i);
+    GenerateField(field, printer);
+  }
+
+  printer->Outdent();
+  printer->Print("end\n");
+}
+
+void GenerateMessage(const google::protobuf::Descriptor* message,
+                     google::protobuf::io::Printer* printer) {
+  printer->Print(
+    "add_message \"$name$\" do\n",
+    "name", message->full_name());
+  printer->Indent();
+
+  for (int i = 0; i < message->field_count(); i++) {
+    const FieldDescriptor* field = message->field(i);
+    if (!field->containing_oneof()) {
+      GenerateField(field, printer);
+    }
+  }
+
+  for (int i = 0; i < message->oneof_decl_count(); i++) {
+    const OneofDescriptor* oneof = message->oneof_decl(i);
+    GenerateOneof(oneof, printer);
   }
 
   printer->Outdent();

src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc

+// Protocol Buffers - Google's data interchange format
+// Copyright 2014 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include <memory>
+
+#include <google/protobuf/compiler/ruby/ruby_generator.h>
+#include <google/protobuf/compiler/command_line_interface.h>
+#include <google/protobuf/io/zero_copy_stream.h>
+#include <google/protobuf/io/printer.h>
+
+#include <google/protobuf/testing/googletest.h>
+#include <gtest/gtest.h>
+#include <google/protobuf/testing/file.h>
+
+namespace google {
+namespace protobuf {
+namespace compiler {
+namespace ruby {
+namespace {
+
+string FindRubyTestDir() {
+  // Inspired by TestSourceDir() in src/google/protobuf/testing/googletest.cc.
+  string prefix = ".";
+  while (!File::Exists(prefix + "/ruby/tests")) {
+    if (!File::Exists(prefix)) {
+      GOOGLE_LOG(FATAL)
+          << "Could not find Ruby test directory. Please run tests from "
+             "somewhere within the protobuf source package.";
+    }
+    prefix += "/..";
+  }
+  return prefix + "/ruby/tests";
+}
+
+// This test is a simple golden-file test over the output of the Ruby code
+// generator. When we make changes to the Ruby extension and alter the Ruby code
+// generator to use those changes, we should (i) manually test the output of the
+// code generator with the extension, and (ii) update the golden output above.
+// Some day, we may integrate build systems between protoc and the language
+// extensions to the point where we can do this test in a more automated way.
+
+TEST(RubyGeneratorTest, GeneratorTest) {
+  string ruby_tests = FindRubyTestDir();
+
+  google::protobuf::compiler::CommandLineInterface cli;
+  cli.SetInputsAreProtoPathRelative(true);
+
+  ruby::Generator ruby_generator;
+  cli.RegisterGenerator("--ruby_out", &ruby_generator, "");
+
+  // Copy generated_code.proto to the temporary test directory.
+  string test_input;
+  GOOGLE_CHECK_OK(File::GetContents(
+      ruby_tests + "/generated_code.proto",
+      &test_input,
+      true));
+  GOOGLE_CHECK_OK(File::SetContents(
+      TestTempDir() + "/generated_code.proto",
+      test_input,
+      true));
+
+  // Invoke the proto compiler (we will be inside TestTempDir() at this point).
+  string ruby_out = "--ruby_out=" + TestTempDir();
+  string proto_path = "--proto_path=" + TestTempDir();
+  const char* argv[] = {
+    "protoc",
+    ruby_out.c_str(),
+    proto_path.c_str(),
+    "generated_code.proto",
+  };
+
+  EXPECT_EQ(0, cli.Run(4, argv));
+
+  // Load the generated output and compare to the expected result.
+  string output;
+  GOOGLE_CHECK_OK(File::GetContents(
+      TestTempDir() + "/generated_code.rb",
+      &output,
+      true));
+  string expected_output;
+  GOOGLE_CHECK_OK(File::GetContents(
+      ruby_tests + "/generated_code.rb",
+      &expected_output,
+      true));
+  EXPECT_EQ(expected_output, output);
+}
+
+}  // namespace
+}  // namespace ruby
+}  // namespace compiler
+}  // namespace protobuf
+}  // namespace google