Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jruby: protobuf fields should be defined as methods #9202

Closed
HoneyryderChuck opened this issue Nov 8, 2021 · 6 comments
Closed

jruby: protobuf fields should be defined as methods #9202

HoneyryderChuck opened this issue Nov 8, 2021 · 6 comments
Assignees
Labels
bug jruby Issues unique to the JRuby interpreter ruby

Comments

@HoneyryderChuck
Copy link

What version of protobuf and what language are you using?
Version: 3.19.1
Language: Ruby (JRuby 9.3)

What operating system (Linux, Windows, ...) and version?

Darwin.

What did you do?

First, thanks a lot for reenabling jruby support 🙏

I have code which checks whether a certain field exists.I use the idiomatic .respond_to? in CRuby. Things aren't working the same way in JRuby however.

What did you expect to see

I'd expect the check to work. To compare, here's how it works in CRuby (v3.0):

# given a Google::Protobuf::Duration message
message.respond_to?(:seconds) #=> true
# although the method isn't defined
message.methods - Object.instance_methods #=>  [:to_f, :method_missing, :[], :[]=, :to_h, :to_json, :to_proto]

What did you see instead?

In Jruby, I don't get the same result for the respond_to? call, even if I can call .seconds.

message.respond_to?(:seconds) #=> false
message.seconds #=> 123

I suspect this happens because .seconds gets sent to method_missing, which then infers the value of the field; and the respective .respond_to_missing? method isn't defined, which makes my example break.

@elharo elharo added the ruby label Nov 8, 2021
@JasonLunn
Copy link
Contributor

Thanks for filing this issue, @HoneyryderChuck. Investigating.

@JasonLunn JasonLunn self-assigned this Feb 25, 2022
@JasonLunn JasonLunn added jruby Issues unique to the JRuby interpreter bug labels Feb 25, 2022
@JasonLunn
Copy link
Contributor

Progress update:

This turns out to be a known issue. There is already a test that covers this issue, and it is disabled on JRuby at the moment (and has been for over 5 years); see https://github.com/protocolbuffers/protobuf/blob/master/ruby/tests/basic.rb#L621

I've attempted to create a fix that would allow me to reenable the test for JRuby, but the issue with JRuby itself seems to still exist. Today I reduced the issue to a reproducible and repeatable failure care and opened #9668 for JRuby.

@JasonLunn
Copy link
Contributor

Marking this issue blocked until #9668 is resolved.

JasonLunn added a commit to JasonLunn/protobuf that referenced this issue Mar 25, 2022
…implemented by `method_missing` return true. Add regression tests.

Fix null pointer exceptions exposed by new unit tests.

Code cleanup: remove test code that was skipping several unit tests only on JRuby.

Fixes issue protocolbuffers#9202.
@JasonLunn JasonLunn removed the blocked label Mar 25, 2022
@JasonLunn
Copy link
Contributor

Workaround implemented in #9677

JasonLunn added a commit that referenced this issue Mar 28, 2022
All synthetic methods implemented by `method_missing` are now supported by `respond_to?`.

Fixes issue #9202.

* Fix null pointer exceptions exposed by new regression tests.
* Fix clear_ on oneofs so that it is safe to call repeatedly and so that respond_to? does not depend on whether the oneof is currently cleared.
* Code cleanup: reenable more tests on JRuby.
* Align JRuby behavior with CRuby by throwing a RuntimeError when attempting to assign to a oneof.
@JasonLunn
Copy link
Contributor

Fix merged.

@HoneyryderChuck
Copy link
Author

Thank you!

JasonLunn added a commit to JasonLunn/protobuf that referenced this issue Mar 31, 2022
All synthetic methods implemented by `method_missing` are now supported by `respond_to?`.

Fixes issue protocolbuffers#9202.

* Fix null pointer exceptions exposed by new regression tests.
* Fix clear_ on oneofs so that it is safe to call repeatedly and so that respond_to? does not depend on whether the oneof is currently cleared.
* Code cleanup: reenable more tests on JRuby.
* Align JRuby behavior with CRuby by throwing a RuntimeError when attempting to assign to a oneof.

(cherry picked from commit 8e7f936)
jorgbrown added a commit that referenced this issue Apr 2, 2022
Cherry pick JRuby changes that fix NPE during encoding (#9507) and implements respond_to? (#9202) into 3.20.x.
mkruskal-google added a commit that referenced this issue Apr 7, 2022
* Fix NPE during encoding and add regression test for issue 9507.

(cherry picked from commit 58e320a)

* Implement `respond_to?` in RubyMessage (#9677)

All synthetic methods implemented by `method_missing` are now supported by `respond_to?`.

Fixes issue #9202.

* Fix null pointer exceptions exposed by new regression tests.
* Fix clear_ on oneofs so that it is safe to call repeatedly and so that respond_to? does not depend on whether the oneof is currently cleared.
* Code cleanup: reenable more tests on JRuby.
* Align JRuby behavior with CRuby by throwing a RuntimeError when attempting to assign to a oneof.

(cherry picked from commit 8e7f936)

* Update protobuf version

* Merge pull request #9727 from mlocati/build-packaged-php-extension

Fix building packaged PHP extension

(cherry picked from commit 7f9901c)

* Update protobuf version

* Update changelogs for 3.20.1-rc1

Co-authored-by: Jason Lunn <jason.lunn@gmail.com>
Co-authored-by: Jorg Brown <jorg.brown@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug jruby Issues unique to the JRuby interpreter ruby
Projects
None yet
Development

No branches or pull requests

3 participants