From 4031c195f1254f06280c7f4e694368d83dc3de4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 6 Jan 2025 19:40:25 -0800 Subject: [PATCH] Fix ruby `has_...?` method (#19731) - Fixes #18807 Closes #19731 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/19731 from ntkme:fix-ruby-has 8bc2d0185ab5783a8ea4edab3c454a8120dea5ff PiperOrigin-RevId: 712734165 --- ruby/ext/google/protobuf_c/message.c | 3 ++- .../main/java/com/google/protobuf/jruby/RubyMessage.java | 6 ++++-- ruby/tests/basic_proto2.rb | 9 +++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index ac736fe9e7bc7..b909e75c8060d 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -362,7 +362,8 @@ static VALUE Message_field_accessor(VALUE _self, const upb_FieldDef* f, if (!upb_FieldDef_HasPresence(f)) { rb_raise(rb_eRuntimeError, "Field does not have presence."); } - return upb_Message_HasFieldByDef(Message_Get(_self, NULL), f); + return upb_Message_HasFieldByDef(Message_Get(_self, NULL), f) ? Qtrue + : Qfalse; case METHOD_WRAPPER_GETTER: { Message* self = ruby_to_Message(_self); if (upb_Message_HasFieldByDef(self->msg, f)) { diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index 2b533dfc45063..d073c54bb16a0 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -454,7 +454,9 @@ public IRubyObject methodMissing(ThreadContext context, IRubyObject[] args) { fieldDescriptor = descriptor.findFieldByName(methodName); if (fieldDescriptor != null && fieldDescriptor.hasPresence()) { - return fields.containsKey(fieldDescriptor) ? runtime.getTrue() : runtime.getFalse(); + return (fields.containsKey(fieldDescriptor) || builder.hasField(fieldDescriptor)) + ? runtime.getTrue() + : runtime.getFalse(); } } else if (methodName.endsWith(AS_VALUE_SUFFIX)) { @@ -952,7 +954,7 @@ protected IRubyObject hasField(ThreadContext context, FieldDescriptor fieldDescr if (!fieldDescriptor.hasPresence()) { throw context.runtime.newArgumentError("does not track presence"); } - return fields.containsKey(fieldDescriptor) + return (fields.containsKey(fieldDescriptor) || builder.hasField(fieldDescriptor)) ? context.runtime.getTrue() : context.runtime.getFalse(); } diff --git a/ruby/tests/basic_proto2.rb b/ruby/tests/basic_proto2.rb index 2112732664166..a14ab16591664 100755 --- a/ruby/tests/basic_proto2.rb +++ b/ruby/tests/basic_proto2.rb @@ -43,6 +43,15 @@ def test_has_field m = TestMessage.new(:optional_int32 => nil) refute m.has_optional_int32? + refute TestMessage.descriptor.lookup('optional_int32').has?(m) + + m = TestMessage.new(:optional_int32 => 0) + assert m.has_optional_int32? + assert TestMessage.descriptor.lookup('optional_int32').has?(m) + + m = TestMessage.decode(TestMessage.encode(m)) + assert m.has_optional_int32? + assert TestMessage.descriptor.lookup('optional_int32').has?(m) assert_raises NoMethodError do m.has_repeated_msg?